From de1ae896b312aaf33a0d3b1ba7fbea1674c072e4 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 2 Aug 2026 05:25:03 +0000 Subject: [PATCH] Add comparison profile preflight --- internal/app/prompt_inspection.go | 106 +++++++++++++++++++++---- internal/app/prompt_inspection_test.go | 92 +++++++++++++++++++++ 2 files changed, 184 insertions(+), 14 deletions(-) diff --git a/internal/app/prompt_inspection.go b/internal/app/prompt_inspection.go index e060fde..eedf186 100644 --- a/internal/app/prompt_inspection.go +++ b/internal/app/prompt_inspection.go @@ -5,6 +5,7 @@ import ( "os" "strings" + "gitea.maximumdirect.net/eric/weatherreporter/internal/comparison" "gitea.maximumdirect.net/eric/weatherreporter/internal/config" "gitea.maximumdirect.net/eric/weatherreporter/internal/promptexec" "gitea.maximumdirect.net/eric/weatherreporter/internal/report" @@ -39,6 +40,32 @@ type PromptExecutionsInspectionRequest struct { LookupEnv func(string) (string, bool) } +// ComparisonInspectionRequest contains the explicit profile selection for one +// resolved prompt comparison. It intentionally has no configured profile field. +type ComparisonInspectionRequest struct { + Resolved report.Resolved + ProfileIDs []string + Executor promptexec.Executor + LookupEnv func(string) (string, bool) +} + +// ComparisonInspectionResult contains the safe, shared prompt identity and +// ordered effective profile identities for a comparison. +type ComparisonInspectionResult struct { + PromptID string + PromptVersion string + PromptHash string + Profiles []ComparisonProfileInspection +} + +// ComparisonProfileInspection contains one requested profile's safe effective +// execution identity. +type ComparisonProfileInspection struct { + ProfileID string + BackendID string + ModelName string +} + // InspectPromptExecution validates the exact prompt and profile needed for a // report before collection, execution, or durable writes begin. func InspectPromptExecution(ctx context.Context, req PromptInspectionRequest) (PromptInspectionResult, error) { @@ -64,21 +91,9 @@ func InspectPromptExecutions(ctx context.Context, req PromptExecutionsInspection profiles := map[string]promptexec.ProfileInspection{} for _, resolved := range req.Resolved { definition := resolved.Definition - if strings.TrimSpace(definition.PromptID) == "" || strings.TrimSpace(definition.PromptVersion) == "" { - return nil, promptexec.NewError(promptexec.InvalidConfiguration, "report prompt identity is incomplete", nil) - } - inspection, err := req.Executor.InspectPrompt(ctx, definition.PromptID, definition.PromptVersion) + inspection, err := inspectPromptContract(ctx, req.Executor, definition) if err != nil { - return nil, promptInspectionError("prompt inspection failed", err) - } - if inspection.PromptID != definition.PromptID || inspection.PromptVersion != definition.PromptVersion { - return nil, promptexec.NewError(promptexec.InvalidConfiguration, "prompt inspection did not return the requested prompt version", nil) - } - if !validPromptInput(inspection.Inputs) { - return nil, promptexec.NewError(promptexec.InvalidConfiguration, "prompt must declare exactly one required application/yaml data_package input", nil) - } - if !validPromptOutput(definition, inspection.Output) { - return nil, promptexec.NewError(promptexec.InvalidConfiguration, "prompt must declare the report JSON Schema output contract", nil) + return nil, err } profileID := req.Promptkit.Profile if profileID == "" { @@ -103,6 +118,61 @@ func InspectPromptExecutions(ctx context.Context, req PromptExecutionsInspection return results, nil } +// InspectComparisonExecution validates one exact prompt and every explicitly +// requested profile before collection or model execution. Profiles are +// inspected sequentially in request order. +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) + } + if req.Executor == nil { + return ComparisonInspectionResult{}, promptexec.NewError(promptexec.InvalidConfiguration, "prompt executor is required", nil) + } + + inspection, err := inspectPromptContract(ctx, req.Executor, req.Resolved.Definition) + if err != nil { + return ComparisonInspectionResult{}, comparisonInspectionError("comparison prompt inspection failed", err) + } + result := ComparisonInspectionResult{ + PromptID: inspection.PromptID, + PromptVersion: inspection.PromptVersion, + PromptHash: inspection.PromptHash, + Profiles: make([]ComparisonProfileInspection, 0, len(req.ProfileIDs)), + } + 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) + } + result.Profiles = append(result.Profiles, ComparisonProfileInspection{ + ProfileID: profile.ProfileID, + BackendID: profile.BackendID, + ModelName: profile.ModelName, + }) + } + return result, nil +} + +func inspectPromptContract(ctx context.Context, executor promptexec.Executor, definition report.Definition) (promptexec.PromptInspection, error) { + if strings.TrimSpace(definition.PromptID) == "" || strings.TrimSpace(definition.PromptVersion) == "" { + return promptexec.PromptInspection{}, promptexec.NewError(promptexec.InvalidConfiguration, "report prompt identity is incomplete", nil) + } + inspection, err := executor.InspectPrompt(ctx, definition.PromptID, definition.PromptVersion) + if err != nil { + return promptexec.PromptInspection{}, promptInspectionError("prompt inspection failed", err) + } + if inspection.PromptID != definition.PromptID || inspection.PromptVersion != definition.PromptVersion { + return promptexec.PromptInspection{}, promptexec.NewError(promptexec.InvalidConfiguration, "prompt inspection did not return the requested prompt version", nil) + } + if !validPromptInput(inspection.Inputs) { + return promptexec.PromptInspection{}, promptexec.NewError(promptexec.InvalidConfiguration, "prompt must declare exactly one required application/yaml data_package input", nil) + } + if !validPromptOutput(definition, inspection.Output) { + return promptexec.PromptInspection{}, promptexec.NewError(promptexec.InvalidConfiguration, "prompt must declare the report JSON Schema output contract", nil) + } + return inspection, nil +} + func inspectPromptProfile(ctx context.Context, executor promptexec.Executor, profileID string, lookupEnv func(string) (string, bool)) (promptexec.ProfileInspection, error) { profile, err := executor.InspectProfile(ctx, profileID) if err != nil { @@ -140,3 +210,11 @@ func promptInspectionError(operation string, err error) error { } return promptexec.NewError(promptexec.InvalidConfiguration, operation, err) } + +func comparisonInspectionError(operation string, err error) error { + category := promptexec.CategoryOf(err) + if category == "" { + category = promptexec.InvalidConfiguration + } + return promptexec.NewError(category, operation, err) +} diff --git a/internal/app/prompt_inspection_test.go b/internal/app/prompt_inspection_test.go index 15ecbb9..59e041d 100644 --- a/internal/app/prompt_inspection_test.go +++ b/internal/app/prompt_inspection_test.go @@ -3,6 +3,7 @@ package app import ( "context" "errors" + "reflect" "strings" "testing" "time" @@ -131,6 +132,95 @@ func TestInspectPromptExecutionsReusesEffectiveProfile(t *testing.T) { } } +func TestInspectComparisonExecutionPreservesOrderedExplicitProfiles(t *testing.T) { + resolved := inspectionResolved(t) + executor := &inspectionExecutor{ + prompt: validPromptInspection(resolved.Definition), + profiles: map[string]promptexec.ProfileInspection{ + "weather-light": {ProfileID: "weather-light", BackendID: "local", ModelName: "light-model"}, + "weather-deep": {ProfileID: "weather-deep", BackendID: "cloud", ModelName: "deep-model"}, + }, + } + profileIDs := []string{"weather-light", "weather-deep"} + result, err := InspectComparisonExecution(context.Background(), ComparisonInspectionRequest{ + Resolved: resolved, ProfileIDs: profileIDs, Executor: executor, + }) + if err != nil { + t.Fatalf("InspectComparisonExecution() error = %v", err) + } + if result.PromptID != resolved.Definition.PromptID || result.PromptVersion != resolved.Definition.PromptVersion || result.PromptHash != "prompt-hash" { + t.Fatalf("prompt result = %#v", result) + } + if !reflect.DeepEqual(executor.profileRequests, profileIDs) || len(executor.promptRequests) != 1 || executor.executeRequests != 0 { + t.Fatalf("prompt/profile/execute requests = %#v/%#v/%d", executor.promptRequests, executor.profileRequests, executor.executeRequests) + } + wantProfiles := []ComparisonProfileInspection{ + {ProfileID: "weather-light", BackendID: "local", ModelName: "light-model"}, + {ProfileID: "weather-deep", BackendID: "cloud", ModelName: "deep-model"}, + } + if !reflect.DeepEqual(result.Profiles, wantProfiles) { + t.Fatalf("profiles = %#v, want %#v", result.Profiles, wantProfiles) + } +} + +func TestInspectComparisonExecutionRejectsInvalidProfilesBeforeInspection(t *testing.T) { + resolved := inspectionResolved(t) + for _, profileIDs := range [][]string{ + {"weather-light"}, + {"weather-light", " \t"}, + {"weather-light", "weather-light"}, + } { + t.Run(strings.Join(profileIDs, ","), func(t *testing.T) { + executor := &inspectionExecutor{prompt: validPromptInspection(resolved.Definition)} + _, err := InspectComparisonExecution(context.Background(), ComparisonInspectionRequest{ + Resolved: resolved, ProfileIDs: profileIDs, Executor: executor, + }) + if err == nil || promptexec.CategoryOf(err) != promptexec.InvalidRequest { + t.Fatalf("error/category = %v/%q, want invalid request", err, promptexec.CategoryOf(err)) + } + if len(executor.promptRequests) != 0 || len(executor.profileRequests) != 0 || executor.executeRequests != 0 { + t.Fatalf("invalid profile selection performed prompt/profile/execution work: %#v/%#v/%d", executor.promptRequests, executor.profileRequests, executor.executeRequests) + } + }) + } +} + +func TestInspectComparisonExecutionStopsAtFirstProfileFailure(t *testing.T) { + resolved := inspectionResolved(t) + executor := &inspectionExecutor{ + prompt: validPromptInspection(resolved.Definition), + profiles: map[string]promptexec.ProfileInspection{ + "weather-light": {ProfileID: "weather-light", BackendID: "local", ModelName: "light-model"}, + "missing-key": {ProfileID: "missing-key", APIKeyEnv: "PROMPT_API_KEY"}, + "weather-deep": {ProfileID: "weather-deep", BackendID: "cloud", ModelName: "deep-model"}, + }, + } + _, err := InspectComparisonExecution(context.Background(), ComparisonInspectionRequest{ + Resolved: resolved, ProfileIDs: []string{"weather-light", "missing-key", "weather-deep"}, Executor: executor, + LookupEnv: func(string) (string, bool) { return "", false }, + }) + if err == nil || promptexec.CategoryOf(err) != promptexec.MissingCredential { + t.Fatalf("error/category = %v/%q, want missing credential", err, promptexec.CategoryOf(err)) + } + 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) + } +} + +func TestInspectComparisonExecutionStopsBeforeProfileInspectionWhenPromptFails(t *testing.T) { + resolved := inspectionResolved(t) + executor := &inspectionExecutor{promptErr: promptexec.NewError(promptexec.PromptNotFound, "prompt is unavailable", nil)} + _, err := InspectComparisonExecution(context.Background(), ComparisonInspectionRequest{ + Resolved: resolved, ProfileIDs: []string{"weather-light", "weather-deep"}, Executor: executor, + }) + if err == nil || promptexec.CategoryOf(err) != promptexec.PromptNotFound || !strings.Contains(err.Error(), "comparison prompt") { + t.Fatalf("error/category = %v/%q, want prompt-context prompt not found", err, promptexec.CategoryOf(err)) + } + if len(executor.promptRequests) != 1 || len(executor.profileRequests) != 0 || executor.executeRequests != 0 { + t.Fatalf("prompt/profile/execute requests = %#v/%#v/%d", executor.promptRequests, executor.profileRequests, executor.executeRequests) + } +} + type inspectionPromptRequest struct { id string version string @@ -143,6 +233,7 @@ type inspectionExecutor struct { promptErr error promptRequests []inspectionPromptRequest profileRequests []string + executeRequests int } func (e *inspectionExecutor) InspectPrompt(_ context.Context, id string, version string) (promptexec.PromptInspection, error) { @@ -166,6 +257,7 @@ func (e *inspectionExecutor) InspectProfile(_ context.Context, id string) (promp } func (e *inspectionExecutor) Execute(context.Context, promptexec.ExecuteRequest, promptexec.PreparationCallback) (*promptexec.Execution, error) { + e.executeRequests++ return nil, errors.New("unexpected execution") }