From 0512995931d08a88748c15cf871d944e15b235a3 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 4 Jul 2026 13:24:27 +0000 Subject: [PATCH] Allow JSON-compatible extra params --- internal/adapter/http/dto.go | 20 ++++----- internal/adapter/http/handler.go | 13 +----- internal/adapter/http/handler_test.go | 53 ++++++++++++++++++++++++ internal/format/prepared_run.go | 18 ++++++++- internal/format/prepared_run_test.go | 50 ++++++++++++++++++++++- internal/profile/repository_test.go | 58 +++++++++++++++++++++++++++ internal/usecase/runner_test.go | 36 +++++++++++++++++ 7 files changed, 224 insertions(+), 24 deletions(-) diff --git a/internal/adapter/http/dto.go b/internal/adapter/http/dto.go index d63767e..693034f 100644 --- a/internal/adapter/http/dto.go +++ b/internal/adapter/http/dto.go @@ -21,16 +21,16 @@ 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"` - 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,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"` } type runResponseDTO struct { diff --git a/internal/adapter/http/handler.go b/internal/adapter/http/handler.go index 0047f05..1052670 100644 --- a/internal/adapter/http/handler.go +++ b/internal/adapter/http/handler.go @@ -137,7 +137,7 @@ func executionTargetOverrideFromModelOverrideDTO(dto *modelOverrideRequestDTO) * ServiceTier: dto.ServiceTier, ReasoningEffort: dto.ReasoningEffort, APIKeyEnv: dto.APIKeyEnv, - ExtraParams: stringMapToAnyMap(dto.ExtraParams), + ExtraParams: dto.ExtraParams, } } @@ -156,17 +156,6 @@ 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 2d49c41..39fdfb2 100644 --- a/internal/adapter/http/handler_test.go +++ b/internal/adapter/http/handler_test.go @@ -250,6 +250,50 @@ func TestHandlerModelOverrideMapsAllSupportedExecutionFields(t *testing.T) { } } +func TestHandlerModelOverrideAcceptsJSONCompatibleExtraParams(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"}, + }} + h := NewHandler(r) + + reqBody := `{ + "prompt_id": "prompt-1", + "inputs": {"transcript": {"type": "file", "uri": "./t.md"}}, + "model": { + "extra_params": { + "string_value": "enabled", + "number_value": 42, + "boolean_value": true, + "object_value": {"nested": "value", "count": 2}, + "array_value": ["first", 3, false] + } + } + }` + req := httptest.NewRequest(http.MethodPost, "/v1/runs", bytes.NewBufferString(reqBody)) + 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 execution override in run request") + } + want := map[string]any{ + "string_value": "enabled", + "number_value": float64(42), + "boolean_value": true, + "object_value": map[string]any{"nested": "value", "count": float64(2)}, + "array_value": []any{"first", float64(3), false}, + } + if !reflect.DeepEqual(r.last.Execution.ExtraParams, want) { + t.Fatalf("unexpected mapped extra_params:\ngot=%#v\nwant=%#v", r.last.Execution.ExtraParams, want) + } +} + func TestHandlerModelOverrideExplicitZeroTemperatureMapsAsPresent(t *testing.T) { r := &fakeRunner{result: &domain.RunResult{ Artifact: domain.Artifact{Body: []byte("ok")}, @@ -336,6 +380,8 @@ func TestHandlerResponseMetadataModelParamsIncludesAllSupportedFields(t *testing APIKeyEnv: "SCRIPTORIUM_API_KEY", ExtraParams: map[string]any{ "provider_option": "on", + "number_value": 42, + "object_value": map[string]any{"nested": "value"}, }, }, }} @@ -390,6 +436,13 @@ func TestHandlerResponseMetadataModelParamsIncludesAllSupportedFields(t *testing if extraParams["provider_option"] != "on" { t.Fatalf("unexpected extra_params.provider_option: %#v", extraParams["provider_option"]) } + if extraParams["number_value"] != float64(42) { + t.Fatalf("unexpected extra_params.number_value: %#v", extraParams["number_value"]) + } + objectValue, ok := extraParams["object_value"].(map[string]any) + if !ok || objectValue["nested"] != "value" { + t.Fatalf("unexpected extra_params.object_value: %#v", extraParams["object_value"]) + } } func TestHandlerInvalidJSON(t *testing.T) { diff --git a/internal/format/prepared_run.go b/internal/format/prepared_run.go index 5610180..904191f 100644 --- a/internal/format/prepared_run.go +++ b/internal/format/prepared_run.go @@ -126,7 +126,11 @@ func (textPreparedRunFormatter) Format(prepared *domain.PreparedRun) ([]byte, er } sort.Strings(keys) for _, k := range keys { - fmt.Fprintf(&b, " %s: %s\n", k, target.ExtraParams[k]) + renderedValue, err := formatExtraParamTextValue(target.ExtraParams[k]) + if err != nil { + return nil, fmt.Errorf("failed to format extra_params.%s: %w", k, err) + } + fmt.Fprintf(&b, " %s: %s\n", k, renderedValue) } } @@ -175,3 +179,15 @@ func (textPreparedRunFormatter) Format(prepared *domain.PreparedRun) ([]byte, er return b.Bytes(), nil } + +func formatExtraParamTextValue(value any) (string, error) { + if s, ok := value.(string); ok { + return s, nil + } + + b, err := json.Marshal(value) + if err != nil { + return "", err + } + return string(b), nil +} diff --git a/internal/format/prepared_run_test.go b/internal/format/prepared_run_test.go index 31f78e7..36fe5c2 100644 --- a/internal/format/prepared_run_test.go +++ b/internal/format/prepared_run_test.go @@ -49,6 +49,36 @@ func TestTextFormatterIncludesPreparedRunDetails(t *testing.T) { } } +func TestTextFormatterRendersExtraParamsDeterministically(t *testing.T) { + prepared := samplePreparedRun() + prepared.EffectiveModelParams.ExtraParams = map[string]any{ + "z_string": "enabled", + "b_number": 42, + "a_object": map[string]any{ + "nested": "value", + "count": 2, + }, + "c_array": []any{"first", 3, false}, + } + + out, err := FormatPreparedRun(prepared, PreparedRunFormatText) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + s := string(out) + + want := strings.Join([]string{ + " extra_params:", + " a_object: {\"count\":2,\"nested\":\"value\"}", + " b_number: 42", + " c_array: [\"first\",3,false]", + " z_string: enabled", + }, "\n") + if !strings.Contains(s, want) { + t.Fatalf("expected deterministic extra_params block %q, got:\n%s", want, s) + } +} + func TestTextFormatterDoesNotIncludeResolvedAPIKeyValue(t *testing.T) { const secret = "super-secret-api-key" t.Setenv("SCRIPTORIUM_API_KEY", secret) @@ -130,6 +160,12 @@ func TestTextFormatterOmitsEmptyCacheControlTTL(t *testing.T) { func TestJSONFormatterEmitsValidJSONAndIncludesPreparedRunFields(t *testing.T) { prepared := samplePreparedRun() prepared.SessionID = "session-123" + prepared.EffectiveModelParams.ExtraParams = map[string]any{ + "number": 42, + "nested": map[string]any{ + "enabled": true, + }, + } out, err := FormatPreparedRun(prepared, PreparedRunFormatJSON) if err != nil { @@ -156,9 +192,21 @@ func TestJSONFormatterEmitsValidJSONAndIncludesPreparedRunFields(t *testing.T) { if decoded["session_id"] != "session-123" { t.Fatalf("expected session_id in json output, got %#v", decoded["session_id"]) } - if _, ok := decoded["effective_model_params"]; !ok { + modelParams, ok := decoded["effective_model_params"].(map[string]any) + if !ok { t.Fatalf("expected effective_model_params in json output, got %#v", decoded) } + extraParams, ok := modelParams["extra_params"].(map[string]any) + if !ok { + t.Fatalf("expected extra_params in json output, got %#v", modelParams["extra_params"]) + } + if extraParams["number"] != float64(42) { + t.Fatalf("unexpected numeric extra param in json output: %#v", extraParams["number"]) + } + nested, ok := extraParams["nested"].(map[string]any) + if !ok || nested["enabled"] != true { + t.Fatalf("unexpected nested extra param in json output: %#v", extraParams["nested"]) + } if _, ok := decoded["input_hashes"]; !ok { t.Fatalf("expected input_hashes in json output, got %#v", decoded) } diff --git a/internal/profile/repository_test.go b/internal/profile/repository_test.go index 1ecb0ce..b970560 100644 --- a/internal/profile/repository_test.go +++ b/internal/profile/repository_test.go @@ -2,6 +2,7 @@ package profile import ( "context" + "encoding/json" "errors" "os" "path/filepath" @@ -85,6 +86,63 @@ temperature: 0.1 } }) + t.Run("valid profile with JSON-compatible extra params", func(t *testing.T) { + writeProfileTestFile(t, filepath.Join(tmpDir, "json-extra-params.yaml"), ` +id: json-extra-params +endpoint: http://localhost:8000/v1 +model: nested-model +extra_params: + string_value: enabled + number_value: 42 + boolean_value: true + object_value: + nested: value + count: 2 + array_value: + - first + - 3 + - false +`) + + p, err := repo.GetProfile(ctx, "json-extra-params") + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + var got map[string]any + encoded, err := json.Marshal(p.ExtraParams) + if err != nil { + t.Fatalf("expected extra_params to marshal as JSON, got %v", err) + } + if err := json.Unmarshal(encoded, &got); err != nil { + t.Fatalf("expected extra_params JSON to decode, got %v", err) + } + + if got["string_value"] != "enabled" { + t.Fatalf("unexpected string extra param: %#v", got["string_value"]) + } + if got["number_value"] != float64(42) { + t.Fatalf("unexpected number extra param: %#v", got["number_value"]) + } + if got["boolean_value"] != true { + t.Fatalf("unexpected boolean extra param: %#v", got["boolean_value"]) + } + objectValue, ok := got["object_value"].(map[string]any) + if !ok { + t.Fatalf("expected object extra param, got %#v", got["object_value"]) + } + if objectValue["nested"] != "value" || objectValue["count"] != float64(2) { + t.Fatalf("unexpected object extra param: %#v", objectValue) + } + arrayValue, ok := got["array_value"].([]any) + if !ok { + t.Fatalf("expected array extra param, got %#v", got["array_value"]) + } + if len(arrayValue) != 3 || arrayValue[0] != "first" || arrayValue[1] != float64(3) || arrayValue[2] != false { + t.Fatalf("unexpected array extra param: %#v", arrayValue) + } + }) + t.Run("duplicate profile IDs fail as ambiguous", func(t *testing.T) { writeProfileTestFile(t, filepath.Join(tmpDir, "duplicate-profile-a.yaml"), ` id: duplicate-profile diff --git a/internal/usecase/runner_test.go b/internal/usecase/runner_test.go index 36a941a..582deed 100644 --- a/internal/usecase/runner_test.go +++ b/internal/usecase/runner_test.go @@ -862,6 +862,42 @@ func TestRunnerRunSuccessful(t *testing.T) { } } +func TestRunnerRunPassesExtraParamsToGenerateRequestTarget(t *testing.T) { + extraParams := map[string]any{ + "string_value": "enabled", + "number_value": 42, + "boolean_value": true, + "object_value": map[string]any{"nested": "value"}, + "array_value": []any{"first", 3, false}, + } + promptRepo := &fakePromptRepo{def: promptDef(domain.FormatText, domain.ValidationNone, 0)} + execRepo := &fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{ + "exec": { + ID: "exec", + Endpoint: "http://profile/v1", + Model: "profile-model", + ExtraParams: extraParams, + }, + }} + llmClient := &fakeLLM{resp: &domain.GenerateResponse{Content: "ok"}} + runner := NewRunner(promptRepo, execRepo, defaultArtifactReader(), defaultRenderer(), llmClient, nil) + + res, err := runner.Run(context.Background(), domain.RunRequest{ + PromptID: "p", + ProfileID: "exec", + Inputs: singleInputRef(), + }) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if !reflect.DeepEqual(res.EffectiveModelParams.ExtraParams, extraParams) { + t.Fatalf("expected run result extra_params to match profile values, got %#v", res.EffectiveModelParams.ExtraParams) + } + if !reflect.DeepEqual(llmClient.lastReq.Target.ExtraParams, extraParams) { + t.Fatalf("expected generate request extra_params to match profile values, got %#v", llmClient.lastReq.Target.ExtraParams) + } +} + func TestRunnerRunAndPrepareResolveSameProfileAndEffectiveSettings(t *testing.T) { promptRepo := &fakePromptRepo{def: promptDef(domain.FormatMarkdown, domain.ValidationBasic, 0)} execRepo := &fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{"exec": defaultExecutionProfile()}}