diff --git a/engine_test.go b/engine_test.go index 45bf9b3..1d92987 100644 --- a/engine_test.go +++ b/engine_test.go @@ -274,30 +274,183 @@ func TestGenerateRequestFormattingRedactsDirectAPIKey(t *testing.T) { } } -func TestPreparePreservesExplicitZeroExecutionOverrides(t *testing.T) { - engine := newContractEngine(t) - zeroFloat := 0.0 - zeroInt := 0 - - prepared, err := engine.Prepare(context.Background(), scriptorium.RunRequest{ - PromptID: frameworkMarkdownSummaryPromptID, - Inputs: map[string]scriptorium.ArtifactRef{ - "transcript": scriptorium.File(frameworkTranscriptPath), - "glossary": scriptorium.File(frameworkGlossaryPath), - }, - Execution: &scriptorium.ExecutionTargetOverride{ - Temperature: &zeroFloat, - MaxTokens: &zeroInt, - TopP: &zeroFloat, - TimeoutSeconds: &zeroInt, - }, - }) - if err != nil { - t.Fatalf("expected prepare to succeed, got %v", err) +func TestEngineExecutionSettingPrecedence(t *testing.T) { + floatPointer := func(value float64) *float64 { + return &value } - target := prepared.EffectiveModelParams - if target.Temperature != 0 || target.MaxTokens != 0 || target.TopP != 0 || target.TimeoutSeconds != 0 { - t.Fatalf("expected explicit zero overrides in effective target, got %+v", target) + intPointer := func(value int) *int { + return &value + } + + defaultsProfile := executionProfileFixture{ + id: "settings-defaults", + endpoint: "http://profile-defaults.test/v1", + model: "profile-defaults-model", + serviceTier: "profile-defaults-tier", + reasoningEffort: "profile-defaults-reasoning", + apiKeyEnv: "SCRIPTORIUM_PRECEDENCE_DEFAULTS", + extraParamSource: "profile-defaults", + } + profileSettings := executionProfileFixture{ + id: "settings-profile", + endpoint: "http://profile-settings.test/v1", + model: "profile-settings-model", + temperature: 0.31, + maxTokens: 311, + topP: 0.61, + timeoutSeconds: 71, + serviceTier: "profile-settings-tier", + reasoningEffort: "profile-settings-reasoning", + apiKeyEnv: "SCRIPTORIUM_PRECEDENCE_PROFILE", + extraParamSource: "profile-settings", + } + requestProfile := executionProfileFixture{ + id: "settings-request", + endpoint: "http://profile-request.test/v1", + model: "profile-request-model", + temperature: 0.29, + maxTokens: 299, + topP: 0.59, + timeoutSeconds: 79, + serviceTier: "profile-request-tier", + reasoningEffort: "profile-request-reasoning", + apiKeyEnv: "SCRIPTORIUM_PRECEDENCE_REQUEST_PROFILE", + extraParamSource: "profile-request", + } + zeroOverrideProfile := executionProfileFixture{ + id: "settings-zero", + endpoint: "http://profile-zero.test/v1", + model: "profile-zero-model", + temperature: 0.43, + maxTokens: 433, + topP: 0.73, + timeoutSeconds: 83, + serviceTier: "profile-zero-tier", + reasoningEffort: "profile-zero-reasoning", + apiKeyEnv: "SCRIPTORIUM_PRECEDENCE_ZERO", + extraParamSource: "profile-zero", + } + + requestTarget := scriptorium.ExecutionTarget{ + Endpoint: "http://request-settings.test/v1", + Model: "request-settings-model", + Temperature: 0.87, + MaxTokens: 877, + TopP: 0.97, + TimeoutSeconds: 177, + ServiceTier: "request-settings-tier", + ReasoningEffort: "request-settings-reasoning", + APIKeyEnv: "SCRIPTORIUM_PRECEDENCE_REQUEST", + ExtraParams: map[string]any{"source": "request-settings"}, + } + zeroOverrideTarget := executionTargetFromProfileFixture(zeroOverrideProfile) + zeroOverrideTarget.Temperature = 0 + zeroOverrideTarget.MaxTokens = 0 + zeroOverrideTarget.TopP = 0 + zeroOverrideTarget.TimeoutSeconds = 0 + + tests := []struct { + name string + profile executionProfileFixture + override *scriptorium.ExecutionTargetOverride + want scriptorium.ExecutionTarget + wantPresence scriptorium.ExecutionTargetPresence + }{ + { + name: "framework defaults fill zero-valued profile settings", + profile: defaultsProfile, + want: scriptorium.ExecutionTarget{ + Endpoint: defaultsProfile.endpoint, + Model: defaultsProfile.model, + Temperature: 0, + MaxTokens: 0, + TopP: 1, + TimeoutSeconds: 600, + ServiceTier: defaultsProfile.serviceTier, + ReasoningEffort: defaultsProfile.reasoningEffort, + APIKeyEnv: defaultsProfile.apiKeyEnv, + ExtraParams: map[string]any{"source": defaultsProfile.extraParamSource}, + }, + }, + { + name: "profile settings replace framework defaults", + profile: profileSettings, + want: executionTargetFromProfileFixture(profileSettings), + }, + { + name: "request settings replace profile settings", + profile: requestProfile, + override: &scriptorium.ExecutionTargetOverride{ + Endpoint: requestTarget.Endpoint, + Model: requestTarget.Model, + Temperature: floatPointer(requestTarget.Temperature), + MaxTokens: intPointer(requestTarget.MaxTokens), + TopP: floatPointer(requestTarget.TopP), + TimeoutSeconds: intPointer(requestTarget.TimeoutSeconds), + ServiceTier: requestTarget.ServiceTier, + ReasoningEffort: requestTarget.ReasoningEffort, + APIKeyEnv: requestTarget.APIKeyEnv, + ExtraParams: requestTarget.ExtraParams, + }, + want: requestTarget, + wantPresence: scriptorium.ExecutionTargetPresence{Temperature: true, MaxTokens: true, TopP: true, TimeoutSeconds: true}, + }, + { + name: "explicit request zero replaces profile settings", + profile: zeroOverrideProfile, + override: &scriptorium.ExecutionTargetOverride{ + Temperature: floatPointer(0), + MaxTokens: intPointer(0), + TopP: floatPointer(0), + TimeoutSeconds: intPointer(0), + }, + want: zeroOverrideTarget, + wantPresence: scriptorium.ExecutionTargetPresence{Temperature: true, MaxTokens: true, TopP: true, TimeoutSeconds: true}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(tt.profile.apiKeyEnv, "set") + if tt.override != nil && tt.override.APIKeyEnv != "" { + t.Setenv(tt.override.APIKeyEnv, "set") + } + + profileDir := t.TempDir() + writeExecutionProfileFixture(t, profileDir, tt.profile) + fake := &fakeLLMClient{response: &scriptorium.GenerateResponse{Content: "ok"}} + engine, err := scriptorium.NewEngine(scriptorium.Config{ + PromptDir: frameworkPromptDir, + ProfileDir: profileDir, + SchemaDir: frameworkSchemaDir, + }, scriptorium.WithLLMClient(fake)) + if err != nil { + t.Fatalf("construct engine: %v", err) + } + + _, err = engine.Run(context.Background(), scriptorium.RunRequest{ + PromptID: frameworkMarkdownSummaryPromptID, + ProfileID: tt.profile.id, + Inputs: map[string]scriptorium.ArtifactRef{ + "transcript": scriptorium.Inline("Nia labels the archive."), + "glossary": scriptorium.Inline("archive: A catalogued collection."), + }, + Execution: tt.override, + }) + if err != nil { + t.Fatalf("run engine: %v", err) + } + if len(fake.requests) != 1 { + t.Fatalf("expected one generation request, got %d", len(fake.requests)) + } + got := fake.requests[0] + if !reflect.DeepEqual(got.Target, tt.want) { + t.Fatalf("unexpected effective target:\ngot=%#v\nwant=%#v", got.Target, tt.want) + } + if got.TargetPresence != tt.wantPresence { + t.Fatalf("unexpected target presence: got=%+v want=%+v", got.TargetPresence, tt.wantPresence) + } + }) } } @@ -419,25 +572,17 @@ func TestEngineRunWithDirectorySourcesAndFileInputs(t *testing.T) { func TestRunPassesPreparedRequestToInjectedLLMClient(t *testing.T) { const directKey = "direct-injected-key" fake := &fakeLLMClient{ - response: &scriptorium.GenerateResponse{Content: "ok"}, + response: &scriptorium.GenerateResponse{Content: `{"events":[{"title":"Archive labelled"}]}`}, } engine := newContractEngineWithOptions(t, frameworkSchemaDir, scriptorium.WithLLMClient(fake)) - zeroFloat := 0.0 - zeroInt := 0 _, err := engine.Run(context.Background(), scriptorium.RunRequest{ - PromptID: frameworkMarkdownSummaryPromptID, + PromptID: frameworkStructuredEventsPromptID, APIKey: directKey, Inputs: map[string]scriptorium.ArtifactRef{ "transcript": scriptorium.Inline("Rin opens the gate."), "glossary": scriptorium.Inline("gate: A guarded passage."), }, - Execution: &scriptorium.ExecutionTargetOverride{ - Temperature: &zeroFloat, - MaxTokens: &zeroInt, - TopP: &zeroFloat, - TimeoutSeconds: &zeroInt, - }, }) if err != nil { t.Fatalf("expected run to succeed, got %v", err) @@ -449,14 +594,8 @@ func TestRunPassesPreparedRequestToInjectedLLMClient(t *testing.T) { if len(req.Prompt.Messages) != 2 || !strings.Contains(req.Prompt.Messages[1].Content, "Rin opens the gate.") { t.Fatalf("expected rendered prompt in generate request, got %+v", req.Prompt) } - if req.Target.Model != "contract-fast-model" || req.Target.Temperature != 0 || req.Target.MaxTokens != 0 || req.Target.TopP != 0 || req.Target.TimeoutSeconds != 0 { - t.Fatalf("unexpected effective target: %+v", req.Target) - } - if !req.TargetPresence.Temperature || !req.TargetPresence.MaxTokens || !req.TargetPresence.TopP || !req.TargetPresence.TimeoutSeconds { - t.Fatalf("expected explicit zero target presence, got %+v", req.TargetPresence) - } - if req.StructuredOutput != nil { - t.Fatalf("did not expect structured output for markdown prompt: %+v", req.StructuredOutput) + if req.StructuredOutput == nil || req.StructuredOutput.Type != scriptorium.StructuredOutputJSONSchema || req.StructuredOutput.JSONSchema == nil { + t.Fatalf("expected structured output handoff, got %+v", req.StructuredOutput) } if req.APIKey != directKey { t.Fatalf("expected direct key on injected generate request") @@ -470,6 +609,78 @@ func TestRunPassesPreparedRequestToInjectedLLMClient(t *testing.T) { } } +func TestEngineRunPropagatesCallerCancellation(t *testing.T) { + started := make(chan struct{}) + transport := roundTripFunc(func(req *http.Request) (*http.Response, error) { + close(started) + <-req.Context().Done() + return nil, req.Context().Err() + }) + engine, err := scriptorium.NewEngine(scriptorium.Config{ + PromptDir: frameworkPromptDir, + ProfileDir: frameworkProfileDir, + SchemaDir: frameworkSchemaDir, + HTTPClient: &http.Client{Transport: transport}, + }) + if err != nil { + t.Fatalf("construct engine: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + result := make(chan error, 1) + go func() { + _, err := engine.Run(ctx, scriptorium.RunRequest{ + PromptID: frameworkMarkdownSummaryPromptID, + Inputs: map[string]scriptorium.ArtifactRef{ + "transcript": scriptorium.Inline("Nia labels the archive."), + "glossary": scriptorium.Inline("archive: A catalogued collection."), + }, + }) + result <- err + }() + + <-started + cancel() + if err := <-result; !errors.Is(err, scriptorium.ErrLLMGenerate) { + t.Fatalf("expected ErrLLMGenerate after caller cancellation, got %v", err) + } +} + +func TestRunRejectsReservedExtraParamsBeforeProviderCall(t *testing.T) { + called := false + transport := roundTripFunc(func(*http.Request) (*http.Response, error) { + called = true + return nil, errors.New("provider should not be called") + }) + engine, err := scriptorium.NewEngine(scriptorium.Config{ + PromptDir: frameworkPromptDir, + ProfileDir: frameworkProfileDir, + SchemaDir: frameworkSchemaDir, + HTTPClient: &http.Client{Transport: transport}, + }) + if err != nil { + t.Fatalf("construct engine: %v", err) + } + + _, err = engine.Run(context.Background(), scriptorium.RunRequest{ + PromptID: frameworkMarkdownSummaryPromptID, + Inputs: map[string]scriptorium.ArtifactRef{ + "transcript": scriptorium.Inline("Nia labels the archive."), + "glossary": scriptorium.Inline("archive: A catalogued collection."), + }, + Execution: &scriptorium.ExecutionTargetOverride{ + ExtraParams: map[string]any{"model": "collision"}, + }, + }) + if !errors.Is(err, scriptorium.ErrInvalidRequest) { + t.Fatalf("expected ErrInvalidRequest, got %v", err) + } + if called { + t.Fatal("expected reserved provider parameter to fail before the provider call") + } +} + func TestRunUsesDirectAPIKeyWithDefaultLLMClient(t *testing.T) { const directKey = "direct-public-key" const missingEnv = "SCRIPTORIUM_PUBLIC_DIRECT_MISSING" @@ -702,6 +913,18 @@ func TestPublicErrorsSupportErrorsIs(t *testing.T) { client: &fakeLLMClient{err: llmErr}, want: scriptorium.ErrLLMGenerate, }, + { + name: "nil llm response", + req: scriptorium.RunRequest{ + PromptID: frameworkMarkdownSummaryPromptID, + Inputs: map[string]scriptorium.ArtifactRef{ + "transcript": scriptorium.Inline("Rin opens the gate."), + "glossary": scriptorium.Inline("gate: A guarded passage."), + }, + }, + client: &fakeLLMClient{}, + want: scriptorium.ErrLLMGenerate, + }, { name: "validation runtime failure", req: scriptorium.RunRequest{ @@ -2057,6 +2280,67 @@ func publicSchemaJSON() string { }` } +type executionProfileFixture struct { + id string + endpoint string + model string + temperature float64 + maxTokens int + topP float64 + timeoutSeconds int + serviceTier string + reasoningEffort string + apiKeyEnv string + extraParamSource string +} + +func executionTargetFromProfileFixture(profile executionProfileFixture) scriptorium.ExecutionTarget { + return scriptorium.ExecutionTarget{ + Endpoint: profile.endpoint, + Model: profile.model, + Temperature: profile.temperature, + MaxTokens: profile.maxTokens, + TopP: profile.topP, + TimeoutSeconds: profile.timeoutSeconds, + ServiceTier: profile.serviceTier, + ReasoningEffort: profile.reasoningEffort, + APIKeyEnv: profile.apiKeyEnv, + ExtraParams: map[string]any{"source": profile.extraParamSource}, + } +} + +func writeExecutionProfileFixture(t *testing.T, dir string, profile executionProfileFixture) { + t.Helper() + data := fmt.Sprintf(`id: %s +endpoint: %s +model: %s +temperature: %g +max_tokens: %d +top_p: %g +timeout_seconds: %d +service_tier: %s +reasoning_effort: %s +api_key_env: %s +extra_params: + source: %q +`, + profile.id, + profile.endpoint, + profile.model, + profile.temperature, + profile.maxTokens, + profile.topP, + profile.timeoutSeconds, + profile.serviceTier, + profile.reasoningEffort, + profile.apiKeyEnv, + profile.extraParamSource, + ) + if err := os.WriteFile(filepath.Join(dir, profile.id+".yaml"), []byte(data), 0o644); err != nil { + t.Fatalf("write execution profile fixture: %v", err) + } +} + type fakeLLMClient struct { response *scriptorium.GenerateResponse err error