From 049a5feadb059ef84a6aa50529cde2d9e57ecb59 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 4 Jul 2026 13:20:39 +0000 Subject: [PATCH] Make request execution overrides presence-aware --- internal/adapter/cli/run.go | 25 ++- internal/adapter/cli/run_test.go | 29 +++ internal/adapter/http/dto.go | 28 +-- internal/adapter/http/handler.go | 21 +- internal/adapter/http/handler_test.go | 86 +++++++- internal/domain/domain.go | 58 +++-- internal/llm/openai_compatible_client_test.go | 2 +- internal/usecase/runner.go | 88 ++++++-- internal/usecase/runner_test.go | 201 +++++++++++++++--- 9 files changed, 434 insertions(+), 104 deletions(-) diff --git a/internal/adapter/cli/run.go b/internal/adapter/cli/run.go index 0099ca5..a3de22a 100644 --- a/internal/adapter/cli/run.go +++ b/internal/adapter/cli/run.go @@ -513,18 +513,25 @@ func buildRunRequestFromConfig(cfg *runConfig) (domain.RunRequest, error) { inputs[name] = domain.ArtifactRef{Type: domain.ArtifactRefFile, URI: path} } - var modelOverride *domain.ExecutionTarget + var modelOverride *domain.ExecutionTargetOverride if cfg.llmBaseURLSet || cfg.modelSet || cfg.temperatureSet || cfg.maxTokensSet || cfg.topPSet || cfg.apiKeyEnvSet || cfg.timeoutSet { - modelOverride = &domain.ExecutionTarget{ - Endpoint: cfg.llmBaseURL, - Model: cfg.model, - Temperature: cfg.temperature, - MaxTokens: cfg.maxTokens, - TopP: cfg.topP, - APIKeyEnv: cfg.apiKeyEnv, + modelOverride = &domain.ExecutionTargetOverride{ + Endpoint: cfg.llmBaseURL, + Model: cfg.model, + APIKeyEnv: cfg.apiKeyEnv, + } + if cfg.temperatureSet { + modelOverride.Temperature = &cfg.temperature + } + if cfg.maxTokensSet { + modelOverride.MaxTokens = &cfg.maxTokens + } + if cfg.topPSet { + modelOverride.TopP = &cfg.topP } if cfg.timeoutSet { - modelOverride.TimeoutSeconds = int(cfg.timeout.Seconds()) + timeoutSeconds := int(cfg.timeout.Seconds()) + modelOverride.TimeoutSeconds = &timeoutSeconds } } diff --git a/internal/adapter/cli/run_test.go b/internal/adapter/cli/run_test.go index 7b7896a..3ba41cc 100644 --- a/internal/adapter/cli/run_test.go +++ b/internal/adapter/cli/run_test.go @@ -747,6 +747,35 @@ func TestRenderCommandDefaultFormatTextIncludesPreparedDetailsAndNoSecrets(t *te } } +func TestRenderCommandExplicitZeroTemperatureReachesEffectiveSettings(t *testing.T) { + lib := newCLITestLibrary(t) + inputPath := lib.writeInputFile(t, "transcript.md", "hello transcript") + + writePromptFile(t, lib.promptDir, "prompt.render", "local-default") + profile := `id: local-default +endpoint: http://127.0.0.1:1/v1 +model: profile-model +temperature: 0.7 +` + if err := os.WriteFile(filepath.Join(lib.profileDir, "local-default.yaml"), []byte(profile), 0o644); err != nil { + t.Fatalf("failed to write profile fixture: %v", err) + } + + code, stdout, stderr := runCLICommand(t, renderCommand, []string{ + "--prompt-dir", lib.promptDir, + "--profile-dir", lib.profileDir, + "--prompt", "prompt.render", + "--input", "transcript=" + inputPath, + "--temperature", "0", + }) + if code != ExitOK { + t.Fatalf("expected ExitOK, got %d stderr=%q", code, stderr) + } + if !strings.Contains(stdout, "\n temperature: 0\n") { + t.Fatalf("expected explicit zero temperature in effective settings, got:\n%s", stdout) + } +} + func TestRenderCommandSucceedsWithPromptAndProfileDirsFromConfig(t *testing.T) { lib := newCLITestLibrary(t) inputPath := lib.writeInputFile(t, "transcript.md", "hello transcript") diff --git a/internal/adapter/http/dto.go b/internal/adapter/http/dto.go index 2113b48..d63767e 100644 --- a/internal/adapter/http/dto.go +++ b/internal/adapter/http/dto.go @@ -23,10 +23,10 @@ type inputRefDTO struct { type modelOverrideRequestDTO struct { Endpoint string `json:"endpoint,omitempty"` Model string `json:"model,omitempty"` - Temperature float64 `json:"temperature,omitempty"` - MaxTokens int `json:"max_tokens,omitempty"` - TopP float64 `json:"top_p,omitempty"` - TimeoutSeconds int `json:"timeout_seconds,omitempty"` + Temperature *float64 `json:"temperature,omitempty"` + MaxTokens *int `json:"max_tokens,omitempty"` + TopP *float64 `json:"top_p,omitempty"` + TimeoutSeconds *int `json:"timeout_seconds,omitempty"` ServiceTier string `json:"service_tier,omitempty"` ReasoningEffort string `json:"reasoning_effort,omitempty"` APIKeyEnv string `json:"api_key_env,omitempty"` @@ -70,16 +70,16 @@ type metadataDTO struct { } type modelParamsDTO struct { - Endpoint string `json:"endpoint"` - Model string `json:"model"` - Temperature float64 `json:"temperature"` - MaxTokens int `json:"max_tokens"` - TopP float64 `json:"top_p"` - TimeoutSeconds int `json:"timeout_seconds"` - ServiceTier string `json:"service_tier,omitempty"` - ReasoningEffort string `json:"reasoning_effort,omitempty"` - APIKeyEnv string `json:"api_key_env,omitempty"` - ExtraParams map[string]string `json:"extra_params,omitempty"` + Endpoint string `json:"endpoint"` + Model string `json:"model"` + Temperature float64 `json:"temperature"` + MaxTokens int `json:"max_tokens"` + TopP float64 `json:"top_p"` + TimeoutSeconds int `json:"timeout_seconds"` + ServiceTier string `json:"service_tier,omitempty"` + ReasoningEffort string `json:"reasoning_effort,omitempty"` + APIKeyEnv string `json:"api_key_env,omitempty"` + ExtraParams map[string]any `json:"extra_params,omitempty"` } type tokenUsageDTO struct { diff --git a/internal/adapter/http/handler.go b/internal/adapter/http/handler.go index 73a4b73..0047f05 100644 --- a/internal/adapter/http/handler.go +++ b/internal/adapter/http/handler.go @@ -61,9 +61,9 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } - var model *domain.ExecutionTarget + var model *domain.ExecutionTargetOverride if req.Model != nil { - model = executionTargetFromModelOverrideDTO(req.Model) + model = executionTargetOverrideFromModelOverrideDTO(req.Model) } res, err := h.runner.Run(r.Context(), domain.RunRequest{ @@ -123,11 +123,11 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, resp) } -func executionTargetFromModelOverrideDTO(dto *modelOverrideRequestDTO) *domain.ExecutionTarget { +func executionTargetOverrideFromModelOverrideDTO(dto *modelOverrideRequestDTO) *domain.ExecutionTargetOverride { if dto == nil { return nil } - return &domain.ExecutionTarget{ + return &domain.ExecutionTargetOverride{ Endpoint: dto.Endpoint, Model: dto.Model, Temperature: dto.Temperature, @@ -137,7 +137,7 @@ func executionTargetFromModelOverrideDTO(dto *modelOverrideRequestDTO) *domain.E ServiceTier: dto.ServiceTier, ReasoningEffort: dto.ReasoningEffort, APIKeyEnv: dto.APIKeyEnv, - ExtraParams: dto.ExtraParams, + ExtraParams: stringMapToAnyMap(dto.ExtraParams), } } @@ -156,6 +156,17 @@ func modelParamsDTOFromExecutionTarget(target domain.ExecutionTarget) modelParam } } +func stringMapToAnyMap(src map[string]string) map[string]any { + if len(src) == 0 { + return nil + } + out := make(map[string]any, len(src)) + for k, v := range src { + out[k] = v + } + return out +} + func mapValidation(v domain.ValidationResult) validationDTO { return validationDTO{ Status: string(v.Status), diff --git a/internal/adapter/http/handler_test.go b/internal/adapter/http/handler_test.go index c05bfa4..2d49c41 100644 --- a/internal/adapter/http/handler_test.go +++ b/internal/adapter/http/handler_test.go @@ -147,7 +147,7 @@ func TestHandlerPostRunsSuccessWithExplicitProfileID(t *testing.T) { if r.last.Execution == nil || r.last.Execution.Model != "gpt-x" { t.Fatalf("expected model override, got %#v", r.last.Execution) } - if r.last.Execution.TimeoutSeconds != 120 { + if r.last.Execution.TimeoutSeconds == nil || *r.last.Execution.TimeoutSeconds != 120 { t.Fatalf("expected timeout_seconds override 120, got %#v", r.last.Execution) } if r.last.Execution.ServiceTier != "flex" { @@ -228,20 +228,92 @@ func TestHandlerModelOverrideMapsAllSupportedExecutionFields(t *testing.T) { got := r.last.Execution if got.Endpoint != "http://override/v1" || got.Model != "override-model" || - got.Temperature != 0.6 || - got.MaxTokens != 250 || - got.TopP != 0.85 || - got.TimeoutSeconds != 33 || got.ServiceTier != "flex" || got.ReasoningEffort != "medium" || got.APIKeyEnv != "SCRIPTORIUM_API_KEY" { t.Fatalf("unexpected mapped execution target: %+v", got) } - if !reflect.DeepEqual(got.ExtraParams, map[string]string{"provider_option": "on"}) { + if got.Temperature == nil || *got.Temperature != 0.6 { + t.Fatalf("unexpected mapped temperature: %#v", got.Temperature) + } + if got.MaxTokens == nil || *got.MaxTokens != 250 { + t.Fatalf("unexpected mapped max_tokens: %#v", got.MaxTokens) + } + if got.TopP == nil || *got.TopP != 0.85 { + t.Fatalf("unexpected mapped top_p: %#v", got.TopP) + } + if got.TimeoutSeconds == nil || *got.TimeoutSeconds != 33 { + t.Fatalf("unexpected mapped timeout_seconds: %#v", got.TimeoutSeconds) + } + if !reflect.DeepEqual(got.ExtraParams, map[string]any{"provider_option": "on"}) { t.Fatalf("unexpected mapped extra_params: %#v", got.ExtraParams) } } +func TestHandlerModelOverrideExplicitZeroTemperatureMapsAsPresent(t *testing.T) { + r := &fakeRunner{result: &domain.RunResult{ + Artifact: domain.Artifact{Body: []byte("ok")}, + Validation: domain.ValidationResult{Status: domain.ValidationPassed, Mode: domain.ValidationBasic, IsValid: true}, + EffectiveModelParams: domain.ExecutionTarget{Endpoint: "http://llm/v1", Model: "m1", Temperature: 0}, + }} + h := NewHandler(r) + + req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{ + "prompt_id": "prompt-1", + "inputs": {"transcript": {"type": "file", "uri": "./t.md"}}, + "model": {"temperature": 0} + }`)) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d body=%s", w.Code, w.Body.String()) + } + if r.last.Execution == nil || r.last.Execution.Temperature == nil { + t.Fatalf("expected temperature override to be present, got %#v", r.last.Execution) + } + if *r.last.Execution.Temperature != 0 { + t.Fatalf("expected zero temperature override, got %v", *r.last.Execution.Temperature) + } +} + +func TestHandlerModelOverrideOmittedTemperatureMapsAsAbsent(t *testing.T) { + r := &fakeRunner{result: &domain.RunResult{ + Artifact: domain.Artifact{Body: []byte("ok")}, + Validation: domain.ValidationResult{Status: domain.ValidationPassed, Mode: domain.ValidationBasic, IsValid: true}, + EffectiveModelParams: domain.ExecutionTarget{Endpoint: "http://llm/v1", Model: "m1", Temperature: 0.7}, + }} + h := NewHandler(r) + + req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(`{ + "prompt_id": "prompt-1", + "inputs": {"transcript": {"type": "file", "uri": "./t.md"}}, + "model": {"model": "override-model"} + }`)) + w := httptest.NewRecorder() + h.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d body=%s", w.Code, w.Body.String()) + } + if r.last.Execution == nil { + t.Fatal("expected model override") + } + if r.last.Execution.Temperature != nil { + t.Fatalf("expected omitted temperature to remain absent, got %#v", r.last.Execution.Temperature) + } + + var resp map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("invalid JSON response: %v", err) + } + metadata := resp["metadata"].(map[string]any) + params := metadata["model_params"].(map[string]any) + if params["temperature"] != 0.7 { + t.Fatalf("expected effective profile/default temperature in response, got %#v", params["temperature"]) + } +} + func TestHandlerResponseMetadataModelParamsIncludesAllSupportedFields(t *testing.T) { r := &fakeRunner{result: &domain.RunResult{ Artifact: domain.Artifact{ @@ -262,7 +334,7 @@ func TestHandlerResponseMetadataModelParamsIncludesAllSupportedFields(t *testing ServiceTier: "priority", ReasoningEffort: "high", APIKeyEnv: "SCRIPTORIUM_API_KEY", - ExtraParams: map[string]string{ + ExtraParams: map[string]any{ "provider_option": "on", }, }, diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 38f770f..a3fab44 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -65,7 +65,7 @@ type RunRequest struct { ProfileID string Inputs map[string]ArtifactRef Vars map[string]string - Execution *ExecutionTarget + Execution *ExecutionTargetOverride Validation *OutputContract Metadata map[string]string } @@ -159,31 +159,45 @@ type PromptMessageTemplate struct { // ExecutionProfile describes how and where to execute a model. type ExecutionProfile struct { - ID string `yaml:"id"` - Endpoint string `yaml:"endpoint"` - Model string `yaml:"model"` - Temperature float64 `yaml:"temperature"` - MaxTokens int `yaml:"max_tokens"` - TopP float64 `yaml:"top_p"` - TimeoutSeconds int `yaml:"timeout_seconds"` - ServiceTier string `yaml:"service_tier"` - ReasoningEffort string `yaml:"reasoning_effort"` - APIKeyEnv string `yaml:"api_key_env"` - ExtraParams map[string]string `yaml:"extra_params"` + ID string `yaml:"id"` + Endpoint string `yaml:"endpoint"` + Model string `yaml:"model"` + Temperature float64 `yaml:"temperature"` + MaxTokens int `yaml:"max_tokens"` + TopP float64 `yaml:"top_p"` + TimeoutSeconds int `yaml:"timeout_seconds"` + ServiceTier string `yaml:"service_tier"` + ReasoningEffort string `yaml:"reasoning_effort"` + APIKeyEnv string `yaml:"api_key_env"` + ExtraParams map[string]any `yaml:"extra_params"` +} + +// ExecutionTargetOverride represents per-request runtime setting overrides. +type ExecutionTargetOverride struct { + Endpoint string `json:"endpoint,omitempty"` + Model string `json:"model,omitempty"` + Temperature *float64 `json:"temperature,omitempty"` + MaxTokens *int `json:"max_tokens,omitempty"` + TopP *float64 `json:"top_p,omitempty"` + TimeoutSeconds *int `json:"timeout_seconds,omitempty"` + ServiceTier string `json:"service_tier,omitempty"` + ReasoningEffort string `json:"reasoning_effort,omitempty"` + APIKeyEnv string `json:"api_key_env,omitempty"` + ExtraParams map[string]any `json:"extra_params,omitempty"` } // ExecutionTarget represents effective model runtime settings for a run. type ExecutionTarget struct { - Endpoint string `yaml:"endpoint" json:"endpoint"` - Model string `yaml:"model" json:"model"` - Temperature float64 `yaml:"temperature" json:"temperature"` - MaxTokens int `yaml:"max_tokens" json:"max_tokens"` - TopP float64 `yaml:"top_p" json:"top_p"` - TimeoutSeconds int `yaml:"timeout_seconds" json:"timeout_seconds"` - ServiceTier string `yaml:"service_tier" json:"service_tier"` - ReasoningEffort string `yaml:"reasoning_effort" json:"reasoning_effort"` - APIKeyEnv string `yaml:"api_key_env" json:"api_key_env"` - ExtraParams map[string]string `yaml:"extra_params" json:"extra_params"` + Endpoint string `yaml:"endpoint" json:"endpoint"` + Model string `yaml:"model" json:"model"` + Temperature float64 `yaml:"temperature" json:"temperature"` + MaxTokens int `yaml:"max_tokens" json:"max_tokens"` + TopP float64 `yaml:"top_p" json:"top_p"` + TimeoutSeconds int `yaml:"timeout_seconds" json:"timeout_seconds"` + ServiceTier string `yaml:"service_tier" json:"service_tier"` + ReasoningEffort string `yaml:"reasoning_effort" json:"reasoning_effort"` + APIKeyEnv string `yaml:"api_key_env" json:"api_key_env"` + ExtraParams map[string]any `yaml:"extra_params" json:"extra_params"` } // OutputContract defines the requirements for the output artifact. diff --git a/internal/llm/openai_compatible_client_test.go b/internal/llm/openai_compatible_client_test.go index 28b3c19..b685312 100644 --- a/internal/llm/openai_compatible_client_test.go +++ b/internal/llm/openai_compatible_client_test.go @@ -438,7 +438,7 @@ func TestOpenAICompatibleClientOmitsReasoningEffortAndExtraParams(t *testing.T) Target: domain.ExecutionTarget{ Model: "model", ReasoningEffort: "high", - ExtraParams: map[string]string{ + ExtraParams: map[string]any{ "provider_option": "on", }, }, diff --git a/internal/usecase/runner.go b/internal/usecase/runner.go index 42c3c96..7a4affd 100644 --- a/internal/usecase/runner.go +++ b/internal/usecase/runner.go @@ -187,7 +187,10 @@ func (r *Runner) Prepare(ctx context.Context, req domain.RunRequest) (*domain.Pr return nil, fmt.Errorf("%w: %w", ErrProfileLoad, err) } - effectiveModel := resolveExecutionTarget(execProfile, req.Execution) + effectiveModel, err := resolveExecutionTarget(execProfile, req.Execution) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrInvalidRequest, err) + } if strings.TrimSpace(effectiveModel.Endpoint) == "" { return nil, fmt.Errorf("%w: execution endpoint is required", ErrInvalidRequest) } @@ -355,22 +358,69 @@ func mergeExecutionTarget(base domain.ExecutionTarget, override domain.Execution out.APIKeyEnv = override.APIKeyEnv } if len(override.ExtraParams) > 0 { - cp := make(map[string]string, len(override.ExtraParams)) - for k, v := range override.ExtraParams { - cp[k] = v - } - out.ExtraParams = cp + out.ExtraParams = copyExtraParams(override.ExtraParams) } return out } -func resolveExecutionTarget(profileValue *domain.ExecutionProfile, override *domain.ExecutionTarget) domain.ExecutionTarget { +func mergeExecutionTargetOverride(base domain.ExecutionTarget, override domain.ExecutionTargetOverride) (domain.ExecutionTarget, error) { + out := base + if override.Endpoint != "" { + out.Endpoint = override.Endpoint + } + if override.Model != "" { + out.Model = override.Model + } + if override.Temperature != nil { + if *override.Temperature < 0 || *override.Temperature > 2 { + return domain.ExecutionTarget{}, errors.New("temperature must be between 0 and 2") + } + out.Temperature = *override.Temperature + } + if override.MaxTokens != nil { + if *override.MaxTokens < 0 { + return domain.ExecutionTarget{}, errors.New("max_tokens must be greater than or equal to 0") + } + out.MaxTokens = *override.MaxTokens + } + if override.TopP != nil { + if *override.TopP < 0 || *override.TopP > 1 { + return domain.ExecutionTarget{}, errors.New("top_p must be between 0 and 1") + } + out.TopP = *override.TopP + } + if override.TimeoutSeconds != nil { + if *override.TimeoutSeconds < 0 { + return domain.ExecutionTarget{}, errors.New("timeout_seconds must be greater than or equal to 0") + } + out.TimeoutSeconds = *override.TimeoutSeconds + } + if strings.TrimSpace(override.ServiceTier) != "" { + out.ServiceTier = override.ServiceTier + } + if strings.TrimSpace(override.ReasoningEffort) != "" { + out.ReasoningEffort = override.ReasoningEffort + } + if strings.TrimSpace(override.APIKeyEnv) != "" { + out.APIKeyEnv = override.APIKeyEnv + } + if len(override.ExtraParams) > 0 { + out.ExtraParams = copyExtraParams(override.ExtraParams) + } + return out, nil +} + +func resolveExecutionTarget(profileValue *domain.ExecutionProfile, override *domain.ExecutionTargetOverride) (domain.ExecutionTarget, error) { out := defaults.ExecutionTargetDefault() out = mergeExecutionTarget(out, executionProfileToTarget(profileValue)) if override != nil { - out = mergeExecutionTarget(out, *override) + var err error + out, err = mergeExecutionTargetOverride(out, *override) + if err != nil { + return domain.ExecutionTarget{}, err + } } - return out + return out, nil } func validateAPIKeyEnv(apiKeyEnv string) error { @@ -388,13 +438,6 @@ func executionProfileToTarget(p *domain.ExecutionProfile) domain.ExecutionTarget if p == nil { return domain.ExecutionTarget{} } - cp := map[string]string(nil) - if len(p.ExtraParams) > 0 { - cp = make(map[string]string, len(p.ExtraParams)) - for k, v := range p.ExtraParams { - cp[k] = v - } - } return domain.ExecutionTarget{ Endpoint: p.Endpoint, Model: p.Model, @@ -405,10 +448,21 @@ func executionProfileToTarget(p *domain.ExecutionProfile) domain.ExecutionTarget ServiceTier: p.ServiceTier, ReasoningEffort: p.ReasoningEffort, APIKeyEnv: p.APIKeyEnv, - ExtraParams: cp, + ExtraParams: copyExtraParams(p.ExtraParams), } } +func copyExtraParams(src map[string]any) map[string]any { + if len(src) == 0 { + return nil + } + cp := make(map[string]any, len(src)) + for k, v := range src { + cp[k] = v + } + return cp +} + func resolveOutputContract(def *domain.PromptDefinition, override *domain.OutputContract) domain.OutputContract { contract := def.Validation if contract.Format == "" { diff --git a/internal/usecase/runner_test.go b/internal/usecase/runner_test.go index d5fad34..36a941a 100644 --- a/internal/usecase/runner_test.go +++ b/internal/usecase/runner_test.go @@ -172,7 +172,7 @@ func TestRunnerPrepareWithExplicitProfileSelection(t *testing.T) { "transcript": {Type: domain.ArtifactRefFile, URI: "a://t"}, "glossary": {Type: domain.ArtifactRefFile, URI: "a://g"}, }, - Execution: &domain.ExecutionTarget{Endpoint: "http://override/v1", Model: "m", Temperature: 0.3, TimeoutSeconds: 90}, + Execution: &domain.ExecutionTargetOverride{Endpoint: "http://override/v1", Model: "m", Temperature: float64Ptr(0.3), TimeoutSeconds: intPtr(90)}, }) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -272,11 +272,11 @@ func TestRunnerPrepareRuntimeOverrideBeatsSelectedProfileValue(t *testing.T) { PromptID: "p", ProfileID: "exec", Inputs: singleInputRef(), - Execution: &domain.ExecutionTarget{ + Execution: &domain.ExecutionTargetOverride{ Endpoint: "http://override/v1", Model: "override-model", - Temperature: 0.7, - TimeoutSeconds: 30, + Temperature: float64Ptr(0.7), + TimeoutSeconds: intPtr(30), ServiceTier: "flex", }, }) @@ -294,6 +294,135 @@ func TestRunnerPrepareRuntimeOverrideBeatsSelectedProfileValue(t *testing.T) { } } +func TestRunnerPrepareRequestNumericOverridePresence(t *testing.T) { + tests := []struct { + name string + override *domain.ExecutionTargetOverride + wantTemperature float64 + wantMaxTokens int + wantTopP float64 + wantTimeoutSecs int + }{ + { + name: "omitted preserves profile values", + override: &domain.ExecutionTargetOverride{}, + wantTemperature: 0.7, + wantMaxTokens: 321, + wantTopP: 0.8, + wantTimeoutSecs: 45, + }, + { + name: "explicit zero temperature", + override: &domain.ExecutionTargetOverride{Temperature: float64Ptr(0)}, + wantTemperature: 0, + wantMaxTokens: 321, + wantTopP: 0.8, + wantTimeoutSecs: 45, + }, + { + name: "explicit zero max tokens", + override: &domain.ExecutionTargetOverride{MaxTokens: intPtr(0)}, + wantTemperature: 0.7, + wantMaxTokens: 0, + wantTopP: 0.8, + wantTimeoutSecs: 45, + }, + { + name: "explicit zero top p", + override: &domain.ExecutionTargetOverride{TopP: float64Ptr(0)}, + wantTemperature: 0.7, + wantMaxTokens: 321, + wantTopP: 0, + wantTimeoutSecs: 45, + }, + { + name: "explicit zero timeout", + override: &domain.ExecutionTargetOverride{TimeoutSeconds: intPtr(0)}, + wantTemperature: 0.7, + wantMaxTokens: 321, + wantTopP: 0.8, + wantTimeoutSecs: 0, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + runner := NewRunner( + &fakePromptRepo{def: promptDef(domain.FormatText, domain.ValidationNone, 0)}, + &fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{ + "exec": { + ID: "exec", + Endpoint: "http://profile/v1", + Model: "profile-model", + Temperature: 0.7, + MaxTokens: 321, + TopP: 0.8, + TimeoutSeconds: 45, + }, + }}, + defaultArtifactReader(), + defaultRenderer(), + &fakeLLM{forbid: true}, + nil, + ) + + prepared, err := runner.Prepare(context.Background(), domain.RunRequest{ + PromptID: "p", + ProfileID: "exec", + Inputs: singleInputRef(), + Execution: tc.override, + }) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + got := prepared.EffectiveModelParams + if got.Temperature != tc.wantTemperature || + got.MaxTokens != tc.wantMaxTokens || + got.TopP != tc.wantTopP || + got.TimeoutSeconds != tc.wantTimeoutSecs { + t.Fatalf("unexpected effective numeric settings: %+v", got) + } + }) + } +} + +func TestRunnerPrepareInvalidRequestNumericOverridesFail(t *testing.T) { + tests := []struct { + name string + override *domain.ExecutionTargetOverride + }{ + {name: "temperature below range", override: &domain.ExecutionTargetOverride{Temperature: float64Ptr(-0.1)}}, + {name: "temperature above range", override: &domain.ExecutionTargetOverride{Temperature: float64Ptr(2.1)}}, + {name: "max tokens below range", override: &domain.ExecutionTargetOverride{MaxTokens: intPtr(-1)}}, + {name: "top p below range", override: &domain.ExecutionTargetOverride{TopP: float64Ptr(-0.1)}}, + {name: "top p above range", override: &domain.ExecutionTargetOverride{TopP: float64Ptr(1.1)}}, + {name: "timeout below range", override: &domain.ExecutionTargetOverride{TimeoutSeconds: intPtr(-1)}}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + runner := NewRunner( + &fakePromptRepo{def: promptDef(domain.FormatText, domain.ValidationNone, 0)}, + &fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{"exec": defaultExecutionProfile()}}, + defaultArtifactReader(), + defaultRenderer(), + &fakeLLM{forbid: true}, + nil, + ) + + _, err := runner.Prepare(context.Background(), domain.RunRequest{ + PromptID: "p", + ProfileID: "exec", + Inputs: singleInputRef(), + Execution: tc.override, + }) + if !errors.Is(err, ErrInvalidRequest) { + t.Fatalf("expected ErrInvalidRequest, got %v", err) + } + }) + } +} + func TestRunnerPrepareSelectedProfileBeatsBuiltInDefault(t *testing.T) { promptRepo := &fakePromptRepo{def: promptDef(domain.FormatText, domain.ValidationNone, 0)} execRepo := &fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{ @@ -693,7 +822,7 @@ func TestRunnerRunSuccessful(t *testing.T) { "transcript": {Type: domain.ArtifactRefFile, URI: "a://t"}, "glossary": {Type: domain.ArtifactRefFile, URI: "a://g"}, }, - Execution: &domain.ExecutionTarget{Endpoint: "http://override/v1", Model: "m", Temperature: 0.3, TimeoutSeconds: 90}, + Execution: &domain.ExecutionTargetOverride{Endpoint: "http://override/v1", Model: "m", Temperature: float64Ptr(0.3), TimeoutSeconds: intPtr(90)}, }) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -749,7 +878,7 @@ func TestRunnerRunAndPrepareResolveSameProfileAndEffectiveSettings(t *testing.T) Inputs: map[string]domain.ArtifactRef{ "transcript": {Type: domain.ArtifactRefFile, URI: "a://t"}, }, - Execution: &domain.ExecutionTarget{Endpoint: "http://override/v1", Model: "m", Temperature: 0.3, TimeoutSeconds: 90}, + Execution: &domain.ExecutionTargetOverride{Endpoint: "http://override/v1", Model: "m", Temperature: float64Ptr(0.3), TimeoutSeconds: intPtr(90)}, } prepared, err := runner.Prepare(context.Background(), req) @@ -877,11 +1006,11 @@ func TestRunnerRunExplicitRuntimeOverrideBeatsSelectedProfileValue(t *testing.T) PromptID: "p", ProfileID: "exec", Inputs: singleInputRef(), - Execution: &domain.ExecutionTarget{ + Execution: &domain.ExecutionTargetOverride{ Endpoint: "http://override/v1", Model: "override-model", - Temperature: 0.7, - TimeoutSeconds: 30, + Temperature: float64Ptr(0.7), + TimeoutSeconds: intPtr(30), ServiceTier: "flex", }, }) @@ -1016,7 +1145,7 @@ func TestRunnerRunRuntimeAPIKeyEnvOverrideWorks(t *testing.T) { PromptID: "p", ProfileID: "exec", Inputs: singleInputRef(), - Execution: &domain.ExecutionTarget{APIKeyEnv: envName}, + Execution: &domain.ExecutionTargetOverride{APIKeyEnv: envName}, }) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -1041,7 +1170,7 @@ func TestRunnerRunRuntimeAPIKeyEnvOverrideBeatsProfile(t *testing.T) { PromptID: "p", ProfileID: "exec", Inputs: singleInputRef(), - Execution: &domain.ExecutionTarget{APIKeyEnv: runtimeEnv}, + Execution: &domain.ExecutionTargetOverride{APIKeyEnv: runtimeEnv}, }) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -1182,7 +1311,7 @@ func TestRunnerRunStructuredRepairRemainsBoundedAndUsesEffectiveModelSettings(t PromptID: "p", ProfileID: "exec", Inputs: singleInputRef(), - Execution: &domain.ExecutionTarget{Endpoint: "http://override/v1", Model: "override-model", TimeoutSeconds: 22}, + Execution: &domain.ExecutionTargetOverride{Endpoint: "http://override/v1", Model: "override-model", TimeoutSeconds: intPtr(22)}, }) if err != nil { t.Fatalf("expected no error, got %v", err) @@ -1269,7 +1398,7 @@ func TestExecutionProfileToTargetPopulatesAllFieldsAndCopiesExtraParams(t *testi ServiceTier: "priority", ReasoningEffort: "medium", APIKeyEnv: "SCRIPTORIUM_API_KEY", - ExtraParams: map[string]string{ + ExtraParams: map[string]any{ "provider_option": "on", }, } @@ -1308,12 +1437,15 @@ func TestResolveExecutionTargetProfileValuesPopulateAllSupportedFields(t *testin ServiceTier: "priority", ReasoningEffort: "low", APIKeyEnv: "PROFILE_KEY", - ExtraParams: map[string]string{ + ExtraParams: map[string]any{ "profile_option": "enabled", }, } - target := resolveExecutionTarget(profileValue, nil) + target, err := resolveExecutionTarget(profileValue, nil) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } if target.Endpoint != profileValue.Endpoint || target.Model != profileValue.Model || target.Temperature != profileValue.Temperature || @@ -1342,32 +1474,35 @@ func TestResolveExecutionTargetRuntimeOverridesBeatProfileForAllOverrideableFiel ServiceTier: "priority", ReasoningEffort: "medium", APIKeyEnv: "PROFILE_KEY", - ExtraParams: map[string]string{ + ExtraParams: map[string]any{ "profile_only": "yes", }, } - override := &domain.ExecutionTarget{ + override := &domain.ExecutionTargetOverride{ Endpoint: "http://override/v1", Model: "override-model", - Temperature: 0.9, - MaxTokens: 111, - TopP: 0.5, - TimeoutSeconds: 30, + Temperature: float64Ptr(0.9), + MaxTokens: intPtr(111), + TopP: float64Ptr(0.5), + TimeoutSeconds: intPtr(30), ServiceTier: "flex", ReasoningEffort: "high", APIKeyEnv: "RUNTIME_KEY", - ExtraParams: map[string]string{ + ExtraParams: map[string]any{ "runtime_only": "yes", }, } - target := resolveExecutionTarget(profileValue, override) + target, err := resolveExecutionTarget(profileValue, override) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } if target.Endpoint != override.Endpoint || target.Model != override.Model || - target.Temperature != override.Temperature || - target.MaxTokens != override.MaxTokens || - target.TopP != override.TopP || - target.TimeoutSeconds != override.TimeoutSeconds || + target.Temperature != *override.Temperature || + target.MaxTokens != *override.MaxTokens || + target.TopP != *override.TopP || + target.TimeoutSeconds != *override.TimeoutSeconds || target.ServiceTier != override.ServiceTier || target.ReasoningEffort != override.ReasoningEffort || target.APIKeyEnv != override.APIKeyEnv { @@ -1411,12 +1546,12 @@ func TestMergeExecutionTargetEmptyStringOverridesDoNotErase(t *testing.T) { func TestMergeExecutionTargetEmptyExtraParamsDoesNotErase(t *testing.T) { base := domain.ExecutionTarget{ - ExtraParams: map[string]string{ + ExtraParams: map[string]any{ "keep": "value", }, } override := domain.ExecutionTarget{ - ExtraParams: map[string]string{}, + ExtraParams: map[string]any{}, } merged := mergeExecutionTarget(base, override) @@ -1492,6 +1627,14 @@ func singleInputRef() map[string]domain.ArtifactRef { return map[string]domain.ArtifactRef{"transcript": {Type: domain.ArtifactRefFile, URI: "a://ok"}} } +func float64Ptr(v float64) *float64 { + return &v +} + +func intPtr(v int) *int { + return &v +} + func newMinimalRunner(promptRepo *fakePromptRepo, execRepo *fakeExecutionProfileRepo) *Runner { return NewRunner( promptRepo,