From b3b23fb381c657851c3b8be1508fe6c4a0ed484d Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 25 Aug 2026 19:52:41 +0000 Subject: [PATCH] Migrate comparison bundles to v2 --- docs/integrations/comparison-bundle.md | 13 +++-- docs/operations.md | 4 +- docs/roadmap/implementation.md | 2 + internal/app/comparison.go | 7 +++ internal/cli/result.go | 9 ++++ internal/comparison/comparison.go | 11 +++- internal/comparison/comparison_test.go | 9 +++- internal/comparison/publish.go | 73 +++++++++++++++++++++++++- internal/comparison/publish_test.go | 2 +- 9 files changed, 120 insertions(+), 10 deletions(-) diff --git a/docs/integrations/comparison-bundle.md b/docs/integrations/comparison-bundle.md index 8cb10e3..0f5ed29 100644 --- a/docs/integrations/comparison-bundle.md +++ b/docs/integrations/comparison-bundle.md @@ -9,7 +9,7 @@ and retention belong to the [operations guide](../operations.md). ## Version And Layout The current and only supported manifest schema version is -`weatherreporter.comparison.v1`. A bundle directory contains exactly these +`weatherreporter.comparison.v2`. A bundle directory contains exactly these regular, non-symlinked files: ```text @@ -51,7 +51,7 @@ this order: ```text position, profileId, backendId, modelName, status, validationStatus, -reportPath, error +repairAttempts, reportPath, error ``` `startedAt` and `finishedAt` are nonzero UTC timestamps, and the latter is not @@ -66,12 +66,15 @@ contiguous from one, profile IDs are distinct and nonblank, and `succeeded + failed == total`. A successful result has `status: "succeeded"`, `validationStatus: "passed"`, +and a non-negative `repairAttempts` count, a `reportPath` exactly equal to the canonical `NN-profile-slug.md` filename for its position, total, and logical profile ID, and no `error`. A failed result has `status: "failed"`, no `reportPath`, and an `error` object with nonblank `category` and `message`. Its validation status is absent, `failed`, or `skipped`. Error messages are valid UTF-8 and no longer than 1,024 bytes. -`backendId` and `validationStatus` are omitted when unavailable. +`backendId` and `validationStatus` are omitted when unavailable. A failed +result with a completed validation (`failed` or `skipped`) must retain its +non-negative `repairAttempts`; early operational failures omit both fields. Every successful Markdown file is declared by exactly one successful result. The directory contains no extra entries. Consumers can therefore verify the @@ -88,7 +91,9 @@ case-variant, or duplicate fields; multiple JSON values; extra entries; symlinks; and future or otherwise unsupported versions. Treat a bundle that fails recognition as an ordinary directory, not as a compatible bundle. -When replacing a recognized bundle, cancellation observed before the new +Only v2 is recognized as a replaceable bundle; v1 is unsupported and must be +moved or removed before a replacement at the same destination. When replacing +a recognized bundle, cancellation observed before the new bundle is installed preserves the prior bundle rather than committing the replacement. diff --git a/docs/operations.md b/docs/operations.md index 652f647..890bb0a 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -122,7 +122,9 @@ The destination is preflighted before prompt inspection and collection, then rechecked immediately before an atomic publish. A missing or empty directory is usable. A nonempty directory can be replaced only when `--replace` is given and it is recognized as a current Weatherreporter comparison bundle; ordinary -directories, symlinks, and unsafe destinations are rejected. Cancellation and +directories, symlinks, and unsafe destinations are rejected. Existing v1 +bundles are not recognized for replacement: move or remove them first. +Cancellation and all failures before publication preserve an existing bundle, including a cancellation observed while a replacement is being prepared. If guarded restoration cannot complete, the error names the retained sibling bundle for diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 42bac61..be0e112 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -386,6 +386,8 @@ GOWORK=off go test -race -count=1 ./internal/adapters/promptkit ./internal/app ### Stage 7: Migrate Comparison Bundles To v2 +Status: Complete. + Purpose: preserve repair activity in the profile-evaluation artifact and make the strict durable schema change explicit. diff --git a/internal/app/comparison.go b/internal/app/comparison.go index e4ceaaa..8858d8b 100644 --- a/internal/app/comparison.go +++ b/internal/app/comparison.go @@ -59,6 +59,7 @@ type ComparisonProfileResult struct { ModelName string Status string ValidationStatus promptexec.ValidationStatus + RepairAttempts *int ReportPath string LLMDebugPath string Error *comparison.SafeError @@ -197,6 +198,9 @@ func copyComparisonOutcomes(result *ComparisonResult, outcomes []comparisonProfi Position: outcome.Position, ProfileID: outcome.ProfileID, BackendID: outcome.BackendID, ModelName: outcome.ModelName, Status: outcome.Status, ValidationStatus: outcome.ValidationStatus, LLMDebugPath: outcome.LLMDebugPath, Error: outcome.Error, } + if outcome.RepairAttempts != nil { + profile.RepairAttempts = repairAttemptsPointer(*outcome.RepairAttempts) + } if published && outcome.Status == comparison.StatusSucceeded { profile.ReportPath = filepath.Join(result.OutputDirectory, outcome.ReportPath) } @@ -226,6 +230,9 @@ func comparisonBundle(result *ComparisonResult, dataPackage []byte, outcomes []c Position: outcome.Position, ProfileID: outcome.ProfileID, BackendID: outcome.BackendID, ModelName: outcome.ModelName, Status: outcome.Status, ValidationStatus: string(outcome.ValidationStatus), Error: outcome.Error, } + if outcome.RepairAttempts != nil { + manifestResult.RepairAttempts = repairAttemptsPointer(*outcome.RepairAttempts) + } if outcome.Status == comparison.StatusSucceeded { manifestResult.ReportPath = outcome.ReportPath bundle.Reports = append(bundle.Reports, comparison.BundleReport{Position: outcome.Position, Path: outcome.ReportPath, Markdown: outcome.Markdown}) diff --git a/internal/cli/result.go b/internal/cli/result.go index c9afc10..aa1cfcf 100644 --- a/internal/cli/result.go +++ b/internal/cli/result.go @@ -107,6 +107,7 @@ type comparisonProfileSummary struct { ModelName string `json:"modelName"` Status string `json:"status"` ValidationStatus string `json:"validationStatus,omitempty"` + RepairAttempts *int `json:"repairAttempts,omitempty"` ReportPath string `json:"reportPath,omitempty"` LLMDebugPath string `json:"llmDebugPath,omitempty"` Error *comparison.SafeError `json:"error,omitempty"` @@ -167,6 +168,11 @@ func newGenerateNotificationSummary(result *app.NotificationResult) *generateNot return summary } +func repairAttemptsCopy(value int) *int { + copy := value + return © +} + func newBatchSummary(result *app.BatchResult, err error) batchSummary { summary := batchSummary{Command: commandRun} if result == nil { @@ -208,6 +214,9 @@ func newComparisonSummary(result *app.ComparisonResult, err error) comparisonSum Status: profile.Status, ValidationStatus: string(profile.ValidationStatus), ReportPath: profile.ReportPath, LLMDebugPath: profile.LLMDebugPath, Error: profile.Error, }) + if profile.RepairAttempts != nil { + summary.Results[len(summary.Results)-1].RepairAttempts = repairAttemptsCopy(*profile.RepairAttempts) + } } summary.Status = comparisonSummaryStatus(result, err) if err != nil { diff --git a/internal/comparison/comparison.go b/internal/comparison/comparison.go index 39fd598..cd281ad 100644 --- a/internal/comparison/comparison.go +++ b/internal/comparison/comparison.go @@ -15,7 +15,7 @@ import ( const ( // SchemaVersion identifies the supported comparison manifest schema. - SchemaVersion = "weatherreporter.comparison.v1" + SchemaVersion = "weatherreporter.comparison.v2" // ManifestFilename is the canonical name of a comparison manifest. ManifestFilename = "comparison.json" @@ -71,6 +71,7 @@ type Result struct { ModelName string `json:"modelName"` Status string `json:"status"` ValidationStatus string `json:"validationStatus,omitempty"` + RepairAttempts *int `json:"repairAttempts,omitempty"` ReportPath string `json:"reportPath,omitempty"` Error *SafeError `json:"error,omitempty"` } @@ -276,11 +277,14 @@ func (manifest Manifest) Validate() error { if strings.TrimSpace(result.ModelName) == "" { return fmt.Errorf("result %d has a blank model name", result.Position) } + if result.RepairAttempts != nil && *result.RepairAttempts < 0 { + return fmt.Errorf("result %d has negative repair attempts", result.Position) + } switch result.Status { case StatusSucceeded: succeeded++ - if result.ValidationStatus != "passed" { + if result.ValidationStatus != "passed" || result.RepairAttempts == nil { return fmt.Errorf("successful result %d did not pass validation", result.Position) } expectedPath, err := ReportFilename(result.Position, manifest.Total, result.ProfileID) @@ -302,6 +306,9 @@ func (manifest Manifest) Validate() error { if err := result.Error.validate(); err != nil { return fmt.Errorf("failed result %d: %w", result.Position, err) } + if result.ValidationStatus != "" && result.RepairAttempts == nil { + return fmt.Errorf("failed result %d has validation without repair provenance", result.Position) + } default: return fmt.Errorf("result %d has unsupported status %q", result.Position, result.Status) } diff --git a/internal/comparison/comparison_test.go b/internal/comparison/comparison_test.go index 5468d3e..f57d9a8 100644 --- a/internal/comparison/comparison_test.go +++ b/internal/comparison/comparison_test.go @@ -118,7 +118,7 @@ func TestManifestEncodingAndRoundTrip(t *testing.T) { t.Fatalf("EncodeManifest() error = %v", err) } want := "{\n" + - " \"schemaVersion\": \"weatherreporter.comparison.v1\",\n" + + " \"schemaVersion\": \"weatherreporter.comparison.v2\",\n" + " \"comparisonId\": \"comparison_daily-2026-08-24\",\n" + " \"startedAt\": \"2026-08-24T12:00:00Z\",\n" + " \"finishedAt\": \"2026-08-24T12:01:00Z\",\n" + @@ -146,6 +146,7 @@ func TestManifestEncodingAndRoundTrip(t *testing.T) { " \"modelName\": \"gpt-5-mini\",\n" + " \"status\": \"succeeded\",\n" + " \"validationStatus\": \"passed\",\n" + + " \"repairAttempts\": 0,\n" + " \"reportPath\": \"01-weather-light.md\"\n" + " },\n" + " {\n" + @@ -309,6 +310,7 @@ func validManifest() Manifest { ModelName: "gpt-5-mini", Status: StatusSucceeded, ValidationStatus: "passed", + RepairAttempts: intPtr(0), ReportPath: "01-weather-light.md", }, { @@ -321,3 +323,8 @@ func validManifest() Manifest { }, } } + +func intPtr(value int) *int { + copy := value + return © +} diff --git a/internal/comparison/publish.go b/internal/comparison/publish.go index c587edd..15ac811 100644 --- a/internal/comparison/publish.go +++ b/internal/comparison/publish.go @@ -388,6 +388,7 @@ var resultFields = map[string]jsonValueValidator{ "modelName": nil, "status": nil, "validationStatus": nil, + "repairAttempts": validateRepairAttemptsJSON, "reportPath": nil, "error": validateSafeErrorJSON, } @@ -430,7 +431,7 @@ func validateResultsJSON(decoder *json.Decoder) error { return fmt.Errorf("results must be an array") } for decoder.More() { - if err := validateJSONObject(decoder, resultFields); err != nil { + if err := validateOrderedJSONObject(decoder, resultFields, []string{"position", "profileId", "backendId", "modelName", "status", "validationStatus", "repairAttempts", "reportPath", "error"}); err != nil { return err } } @@ -444,6 +445,65 @@ func validateResultsJSON(decoder *json.Decoder) error { return nil } +func validateOrderedJSONObject(decoder *json.Decoder, fields map[string]jsonValueValidator, order []string) error { + token, err := decoder.Token() + if err != nil { + return err + } + if delimiter, ok := token.(json.Delim); !ok || delimiter != '{' { + return fmt.Errorf("manifest value must be an object") + } + seen := make(map[string]struct{}, len(fields)) + last := -1 + for decoder.More() { + token, err := decoder.Token() + if err != nil { + return err + } + name, ok := token.(string) + if !ok { + return fmt.Errorf("manifest field name is invalid") + } + validator, known := fields[name] + if !known { + return fmt.Errorf("manifest field %q is not canonical", name) + } + if _, duplicate := seen[name]; duplicate { + return fmt.Errorf("manifest field %q is duplicated", name) + } + position := -1 + for index, candidate := range order { + if candidate == name { + position = index + break + } + } + if position <= last { + return fmt.Errorf("manifest field %q is out of canonical order", name) + } + seen[name] = struct{}{} + last = position + if validator == nil { + var value json.RawMessage + if err := decoder.Decode(&value); err != nil { + return err + } + continue + } + if err := validator(decoder); err != nil { + return err + } + } + token, err = decoder.Token() + if err != nil { + return err + } + if delimiter, ok := token.(json.Delim); !ok || delimiter != '}' { + return fmt.Errorf("manifest object has an invalid terminator") + } + return nil +} + func validateSafeErrorJSON(decoder *json.Decoder) error { token, err := decoder.Token() if err != nil { @@ -458,6 +518,17 @@ func validateSafeErrorJSON(decoder *json.Decoder) error { return validateJSONObjectBody(decoder, safeErrorFields) } +func validateRepairAttemptsJSON(decoder *json.Decoder) error { + var value int + if err := decoder.Decode(&value); err != nil { + return err + } + if value < 0 { + return fmt.Errorf("repairAttempts must not be negative") + } + return nil +} + func validateJSONObject(decoder *json.Decoder, fields map[string]jsonValueValidator) error { token, err := decoder.Token() if err != nil { diff --git a/internal/comparison/publish_test.go b/internal/comparison/publish_test.go index d9a34cb..f4c26bc 100644 --- a/internal/comparison/publish_test.go +++ b/internal/comparison/publish_test.go @@ -1077,7 +1077,7 @@ func testBundleWithReports(t *testing.T, count int) LogicalBundle { } manifest.Results[i] = Result{ Position: position, ProfileID: profileID, ModelName: "gpt-5-mini", - Status: StatusSucceeded, ValidationStatus: "passed", ReportPath: path, + Status: StatusSucceeded, ValidationStatus: "passed", RepairAttempts: intPtr(0), ReportPath: path, } reports[i] = BundleReport{Position: position, Path: path, Markdown: []byte("# Daily\n")} }