diff --git a/docs/internal/app-orchestration.md b/docs/internal/app-orchestration.md index e965221..88ddf34 100644 --- a/docs/internal/app-orchestration.md +++ b/docs/internal/app-orchestration.md @@ -30,7 +30,9 @@ construction to the prepared-report flow. It does not accept a notifier. Once the destination is resolved, the partial result retains its absolute output directory even when later preflight, debug initialization, inspection, collection, or preparation fails. Every initialized result is finalized with a -finished timestamp. Artifact paths are added only after publication commits. +finished timestamp. If prompt inspection succeeds before a later profile +inspection fails, the partial result retains the resolved prompt ID, version, +and hash. Artifact paths are added only after publication commits. The comparison execution core starts each inspected profile independently, keeps results in selection order, and waits for all started work. Independent diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 4312e21..cad7f8a 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -16,7 +16,7 @@ internally coherent. ## Implementation Rules -Apply these rules in every remaining stage: +The following rules governed every implementation stage: - Read `docs/development.md`, the task-specific documents it identifies, all files under `docs/policy/`, and the feature roadmap before changing code. diff --git a/internal/app/comparison.go b/internal/app/comparison.go index 605e925..d3dcced 100644 --- a/internal/app/comparison.go +++ b/internal/app/comparison.go @@ -65,12 +65,17 @@ type ComparisonProfileResult struct { Error *comparison.SafeError } -var publishComparison = comparison.Publish +type comparisonPublisher func(context.Context, comparison.DestinationPlan, comparison.LogicalBundle) (comparison.PublicationResult, error) // CompareDetailed assembles, executes, and atomically publishes a comparison -// bundle. Profile failures publish a complete partial bundle; all other -// failures leave the destination untouched. +// bundle. Profile failures publish a complete partial bundle. Failures before +// commit leave the destination untouched; a post-commit cleanup failure leaves +// the new bundle installed and returns its artifact paths with an error. func CompareDetailed(ctx context.Context, req ComparisonRequest) (*ComparisonResult, error) { + return compareDetailed(ctx, req, comparison.Publish) +} + +func compareDetailed(ctx context.Context, req ComparisonRequest, publish comparisonPublisher) (*ComparisonResult, error) { if err := comparison.ValidateProfileIDs(req.ProfileIDs); err != nil { return nil, err } @@ -117,10 +122,10 @@ func CompareDetailed(ctx context.Context, req ComparisonRequest) (*ComparisonRes inspection, err := InspectComparisonExecution(ctx, ComparisonInspectionRequest{ Resolved: resolved, ProfileIDs: req.ProfileIDs, Executor: req.Executor, LookupEnv: os.LookupEnv, }) + result.PromptID, result.PromptVersion, result.PromptHash = inspection.PromptID, inspection.PromptVersion, inspection.PromptHash if err != nil { return result, err } - result.PromptID, result.PromptVersion, result.PromptHash = inspection.PromptID, inspection.PromptVersion, inspection.PromptHash collection, err := collectWeather(ctx, req.Config, req.Collector) if err != nil { @@ -148,7 +153,7 @@ func CompareDetailed(ctx context.Context, req ComparisonRequest) (*ComparisonRes if err != nil { return result, fmt.Errorf("re-preflight comparison destination: %w", err) } - publication, err := publishComparison(ctx, publicationPlan, bundle) + publication, err := publish(ctx, publicationPlan, bundle) if publication.Committed { result.OutputDirectory = publicationPlan.Target result.ManifestPath = filepath.Join(publicationPlan.Target, comparison.ManifestFilename) diff --git a/internal/app/comparison_test.go b/internal/app/comparison_test.go index 09e8ba5..7109790 100644 --- a/internal/app/comparison_test.go +++ b/internal/app/comparison_test.go @@ -90,18 +90,16 @@ func TestCompareDetailedRetainsCommittedPathsWhenBackupCleanupFails(t *testing.T bundle := generationBundle(t) backupPath := filepath.Join(t.TempDir(), ".comparison-daily.backup-retained") cleanupCause := errors.New("backup cleanup failed") - originalPublish := publishComparison - publishComparison = func(context.Context, comparison.DestinationPlan, comparison.LogicalBundle) (comparison.PublicationResult, error) { + publish := func(context.Context, comparison.DestinationPlan, comparison.LogicalBundle) (comparison.PublicationResult, error) { return comparison.PublicationResult{Committed: true, RetainedBackupPath: backupPath}, &comparison.PublicationCleanupError{RetainedBackupPath: backupPath, Err: cleanupCause} } - t.Cleanup(func() { publishComparison = originalPublish }) - result, err := CompareDetailed(context.Background(), ComparisonRequest{ + result, err := compareDetailed(context.Background(), ComparisonRequest{ Config: comparisonConfig(), Report: ReportDaily, ProfileIDs: []string{"weather-light", "weather-deep"}, WorkingDir: t.TempDir(), Date: generationTime("2026-05-29T12:00:00-05:00"), Clock: timeutil.FixedClock{Time: generationTime("2026-05-29T08:30:00-05:00")}, Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, - }) + }, publish) var cleanupErr *comparison.PublicationCleanupError if result == nil || !errors.As(err, &cleanupErr) || !errors.Is(err, cleanupCause) || cleanupErr.RetainedBackupPath != backupPath || !filepath.IsAbs(result.ManifestPath) || !filepath.IsAbs(result.DataPackagePath) { t.Fatalf("CompareDetailed() result/error = %#v/%v", result, err) @@ -133,12 +131,13 @@ func TestCompareDetailedPreflightsBeforePromptOrCollection(t *testing.T) { func TestCompareDetailedFinalizesUnpublishedFailures(t *testing.T) { for _, test := range []struct { - name string - prepare func(t *testing.T, outputDirectory string) - debugDir string - executor *generationExecutor - collector *generationCollector - wantPrompt bool + name string + prepare func(t *testing.T, outputDirectory string) + debugDir string + executor *generationExecutor + collector *generationCollector + wantPrompt bool + wantCollection bool }{ { name: "destination preflight", @@ -163,16 +162,26 @@ func TestCompareDetailedFinalizesUnpublishedFailures(t *testing.T) { wantPrompt: false, }, { - name: "collection", - executor: &generationExecutor{}, - collector: &generationCollector{err: errors.New("collection failed")}, + name: "profile preflight", + executor: &generationExecutor{profileInspectErrors: map[string]error{ + "weather-deep": promptexec.NewError(promptexec.MissingCredential, "profile credential is unavailable", errors.New("unsafe cause")), + }}, + collector: &generationCollector{}, wantPrompt: true, }, { - name: "preparation", - executor: &generationExecutor{}, - collector: &generationCollector{bundle: &weatherdata.Bundle{}}, - wantPrompt: true, + name: "collection", + executor: &generationExecutor{}, + collector: &generationCollector{err: errors.New("collection failed")}, + wantPrompt: true, + wantCollection: true, + }, + { + name: "preparation", + executor: &generationExecutor{}, + collector: &generationCollector{bundle: &weatherdata.Bundle{}}, + wantPrompt: true, + wantCollection: true, }, } { t.Run(test.name, func(t *testing.T) { @@ -199,6 +208,9 @@ func TestCompareDetailedFinalizesUnpublishedFailures(t *testing.T) { if (result.PromptID != "") != test.wantPrompt || (result.PromptHash != "") != test.wantPrompt { t.Fatalf("prompt identity = %q/%q, want resolved=%t", result.PromptID, result.PromptHash, test.wantPrompt) } + if collector.called != test.wantCollection || test.executor.executeCalls != 0 { + t.Fatalf("collection/execution = %t/%d, want collection=%t and no execution", collector.called, test.executor.executeCalls, test.wantCollection) + } }) } } diff --git a/internal/app/generation_test.go b/internal/app/generation_test.go index 63580c4..8848b59 100644 --- a/internal/app/generation_test.go +++ b/internal/app/generation_test.go @@ -35,18 +35,19 @@ func (c *generationCollector) Run(context.Context, collect.Request) (*collect.Re } type generationExecutor struct { - called bool - executeCalls int - promptInspections int - profileInspections int - inspectErr error - executeErr error - executeErrors map[string]error - beforeExecute func(promptexec.ExecuteRequest) - cancelBeforeReturn context.CancelFunc - validation promptexec.ValidationStatus - rawOutput []byte - failedPrompt string + called bool + executeCalls int + promptInspections int + profileInspections int + inspectErr error + profileInspectErrors map[string]error + executeErr error + executeErrors map[string]error + beforeExecute func(promptexec.ExecuteRequest) + cancelBeforeReturn context.CancelFunc + validation promptexec.ValidationStatus + rawOutput []byte + failedPrompt string } var generationExecutorMu sync.Mutex @@ -65,6 +66,9 @@ func (e *generationExecutor) InspectProfile(_ context.Context, id string) (promp generationExecutorMu.Lock() defer generationExecutorMu.Unlock() e.profileInspections++ + if err := e.profileInspectErrors[id]; err != nil { + return promptexec.ProfileInspection{}, err + } return promptexec.ProfileInspection{ProfileID: id, BackendID: "fixture", ModelName: "fixture-model"}, nil } func (e *generationExecutor) Execute(_ context.Context, req promptexec.ExecuteRequest, callback promptexec.PreparationCallback) (*promptexec.Execution, error) { diff --git a/internal/app/prompt_inspection.go b/internal/app/prompt_inspection.go index eedf186..bf8b67a 100644 --- a/internal/app/prompt_inspection.go +++ b/internal/app/prompt_inspection.go @@ -120,7 +120,8 @@ func InspectPromptExecutions(ctx context.Context, req PromptExecutionsInspection // InspectComparisonExecution validates one exact prompt and every explicitly // requested profile before collection or model execution. Profiles are -// inspected sequentially in request order. +// inspected sequentially in request order. If a profile fails, the returned +// partial result retains the prompt identity and successfully inspected prefix. func InspectComparisonExecution(ctx context.Context, req ComparisonInspectionRequest) (ComparisonInspectionResult, error) { if err := comparison.ValidateProfileIDs(req.ProfileIDs); err != nil { return ComparisonInspectionResult{}, promptexec.NewError(promptexec.InvalidRequest, "comparison profile selection is invalid", err) @@ -142,7 +143,7 @@ func InspectComparisonExecution(ctx context.Context, req ComparisonInspectionReq for _, profileID := range req.ProfileIDs { profile, err := inspectPromptProfile(ctx, req.Executor, profileID, req.LookupEnv) if err != nil { - return ComparisonInspectionResult{}, comparisonInspectionError("comparison profile inspection failed", err) + return result, comparisonInspectionError("comparison profile inspection failed", err) } result.Profiles = append(result.Profiles, ComparisonProfileInspection{ ProfileID: profile.ProfileID, diff --git a/internal/app/prompt_inspection_test.go b/internal/app/prompt_inspection_test.go index 59e041d..5727a1e 100644 --- a/internal/app/prompt_inspection_test.go +++ b/internal/app/prompt_inspection_test.go @@ -195,7 +195,7 @@ func TestInspectComparisonExecutionStopsAtFirstProfileFailure(t *testing.T) { "weather-deep": {ProfileID: "weather-deep", BackendID: "cloud", ModelName: "deep-model"}, }, } - _, err := InspectComparisonExecution(context.Background(), ComparisonInspectionRequest{ + result, err := InspectComparisonExecution(context.Background(), ComparisonInspectionRequest{ Resolved: resolved, ProfileIDs: []string{"weather-light", "missing-key", "weather-deep"}, Executor: executor, LookupEnv: func(string) (string, bool) { return "", false }, }) @@ -205,6 +205,9 @@ func TestInspectComparisonExecutionStopsAtFirstProfileFailure(t *testing.T) { if !reflect.DeepEqual(executor.profileRequests, []string{"weather-light", "missing-key"}) || len(executor.promptRequests) != 1 || executor.executeRequests != 0 { t.Fatalf("prompt/profile/execute requests = %#v/%#v/%d", executor.promptRequests, executor.profileRequests, executor.executeRequests) } + if result.PromptID != resolved.Definition.PromptID || result.PromptVersion != resolved.Definition.PromptVersion || result.PromptHash == "" || len(result.Profiles) != 1 || result.Profiles[0].ProfileID != "weather-light" { + t.Fatalf("partial inspection result = %#v", result) + } } func TestInspectComparisonExecutionStopsBeforeProfileInspectionWhenPromptFails(t *testing.T) {