diff --git a/docs/internal/overview.md b/docs/internal/overview.md index 2459da5..e14d3c3 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -17,7 +17,7 @@ contributor workflow and validation. | `examples/go-library/observe` | Demonstrates offline built-in HTTP retry and synchronous metadata observation across structured proposal and assessment runs using embedded sources, two profiles on one finite backend, and an application decision. It is not a public library package. | [Example program](../../examples/go-library/observe/main.go) | | `internal/backend` | Constructs each engine's immutable registry from maintained definitions and consumer additions, validates and defensively copies definitions through the shared JSON-value package, and consumes the LLM-owned OpenAI-compatible reserved request-field rule. | [Backend registry](../../internal/backend/registry.go) | | `internal/catalog` | Strictly validates imported immutable maintained backend and profile catalog assets before engine assembly uses them. | [Catalog adapter](../../internal/catalog/catalog.go), [internal sources](sources.md#profiles-and-built-ins) | -| `internal/configuration` | Loads keyed backend, profile, and prompt configuration through ConfigKit; records effective field presence, validates final values, and captures indexed prompt definitions and referenced message content. It is not yet connected to engine assembly. | [Typed loader](../../internal/configuration/load.go), [internal sources](sources.md#typed-configuration-loading) | +| `internal/configuration` | Loads keyed backend, profile, and prompt configuration through ConfigKit; records effective field presence, validates final values, captures indexed prompt definitions and referenced message content, and constructs immutable backend and presence-aware profile snapshots. It is not yet connected to engine assembly. | [Typed loader](../../internal/configuration/load.go), [internal sources](sources.md#typed-configuration-loading) | | `internal/capacity` | Owns engine-local bounded execution admission and FIFO model-generation permits for limited backend IDs, including cancellation-safe waiter removal and client wrapping. | [Internal capacity management](capacity.md) | | `internal/domain` | Defines internal framework values for requests, artifacts, prompt definitions, profiles, execution targets, rendering, generation, and validation, and owns source-neutral invariants for shared execution settings, OpenAI-compatible base endpoints, session identifiers, and output contracts. Source parsing, required fields, other source-specific normalization, defaulting, and boundary-specific error classification remain with their callers. | [Domain declarations](../../internal/domain/domain.go), [endpoint invariant](../../internal/domain/endpoint.go) | | `internal/engine` | Assembles external catalogs, Promptkit-owned profiles, repositories, registry, capacity, collaborators, repairer, and the one use-case runner for each engine instance. | [Engine assembly](../../internal/engine/engine.go), [internal sources](sources.md#profiles-and-built-ins), [internal runner](runner.md) | diff --git a/docs/internal/runner.md b/docs/internal/runner.md index 99c685a..f592830 100644 --- a/docs/internal/runner.md +++ b/docs/internal/runner.md @@ -53,8 +53,16 @@ explicit profile ID. Both then apply the ordinary source precedence, resolve a named backend, and construct the effective target from framework, backend, and profile values. -Inspection stops after the resulting endpoint and model are structurally -validated. It does not check credential availability or perform prompt, +The private ConfigKit loader can instead supply an already resolved, +presence-aware target to this boundary. Request overrides still apply afterward, +and preparation checks endpoint and model completeness only after those +overrides. This path remains internal until the engine assembly switches to the +loader; the existing public engine still uses the repository path above. + +For the existing repository path, inspection validates the resulting endpoint +and model. For the precomputed target path it can report an incomplete profile; +preparation remains responsible for checking completeness after request +overrides. Neither path checks credential availability or performs prompt, artifact, schema, rendering, admission, or model-client work. The root [`Engine.InspectProfile`](../../engine.go) GoDoc owns the public operation's exact contract. diff --git a/docs/internal/sources.md b/docs/internal/sources.md index 65f9e65..66d8a37 100644 --- a/docs/internal/sources.md +++ b/docs/internal/sources.md @@ -20,7 +20,11 @@ profile domain values. A lower-layer domain value replaced before the handoff is not separately validated. It also validates effective prompt fields, reads referenced message content using the contributing field's passive source context, checks static template syntax, and indexes immutable definitions by -exact ID and version. Its focused tests exercise real ConfigKit loading. +exact ID and version. The loader also builds an immutable backend registry from +the final definitions and indexes profile execution targets after inheritance, +applying the selected backend's defaults only then. These targets retain +explicit numeric-zero presence and null suppression for runtime execution. +Its focused tests exercise real ConfigKit loading. This loader is not yet wired into engine assembly. The public engine continues to use the source and repository behavior described below. The public diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 0e036ad..b54ba13 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -359,6 +359,8 @@ existing execution tests still work through the old assembly until cutover. ## Stage 3 — Make Execution Settings Presence-Aware +**Completed:** 2026-09-30. + Scope: domain settings, target resolution, wire-presence propagation, profile inspection. No new source discovery in execution packages. diff --git a/internal/backend/registry.go b/internal/backend/registry.go index 40a4a09..f65b5df 100644 --- a/internal/backend/registry.go +++ b/internal/backend/registry.go @@ -35,6 +35,23 @@ type Registry struct { backends map[string]domain.Backend } +// NewEffectiveRegistry stores already-validated effective backend definitions +// under their exact configuration keys. Configuration validation and +// normalization precede this ownership boundary. +func NewEffectiveRegistry(definitions map[string]domain.Backend) (*Registry, error) { + registry := &Registry{backends: make(map[string]domain.Backend, len(definitions))} + for id, definition := range definitions { + params, err := jsonvalue.CopyMap(definition.ExtraParams) + if err != nil { + return nil, fmt.Errorf("backend %q extra parameters: %w", id, err) + } + definition.ID = id + definition.ExtraParams = params + registry.backends[id] = definition + } + return registry, nil +} + // NewRegistry constructs a registry containing maintained definitions followed // by consumer additions. Every ID must be unique across both groups. func NewRegistry(maintained, additions []domain.Backend) (*Registry, error) { diff --git a/internal/backend/registry_test.go b/internal/backend/registry_test.go index 52f0d23..d933a0e 100644 --- a/internal/backend/registry_test.go +++ b/internal/backend/registry_test.go @@ -48,6 +48,29 @@ func TestRegistryIncludesMaintainedDefinitions(t *testing.T) { } } +func TestEffectiveRegistryKeepsLiteralIDsAndOwnsValues(t *testing.T) { + params := map[string]any{"nested": map[string]any{"enabled": true}} + registry, err := backend.NewEffectiveRegistry(map[string]domain.Backend{ + " local.v1 ": {Endpoint: validEndpoint, ExtraParams: params, ConcurrencyLimit: 2, QueueCapacitySet: true}, + }) + if err != nil { + t.Fatal(err) + } + params["nested"].(map[string]any)["enabled"] = false + selected, err := registry.GetBackend(" local.v1 ") + if err != nil || selected.ID != " local.v1 " || selected.ExtraParams["nested"].(map[string]any)["enabled"] != true { + t.Fatalf("effective registry changed identity or retained caller values: %+v, %v", selected, err) + } + if _, err := registry.GetBackend("local.v1"); !errors.Is(err, backend.ErrBackendNotFound) { + t.Fatalf("trimmed identity unexpectedly matched: %v", err) + } + selected.ExtraParams["nested"].(map[string]any)["enabled"] = false + again, _ := registry.GetBackend(" local.v1 ") + if again.ExtraParams["nested"].(map[string]any)["enabled"] != true { + t.Fatal("registry lookup exposed its stored values") + } +} + func TestRegistryNormalizesUniqueAdditionsAndIsolatesMutations(t *testing.T) { nested := map[string]int{"limit": 2} extraParams := map[string]any{ diff --git a/internal/configuration/load.go b/internal/configuration/load.go index 9e15aa4..496a5e4 100644 --- a/internal/configuration/load.go +++ b/internal/configuration/load.go @@ -9,6 +9,7 @@ import ( "strings" "gitea.maximumdirect.net/eric/ConfigKit" + "gitea.maximumdirect.net/eric/promptkit/internal/backend" "gitea.maximumdirect.net/eric/promptkit/internal/domain" "gitea.maximumdirect.net/eric/promptkit/internal/jsonvalue" "gitea.maximumdirect.net/eric/promptkit/internal/llm" @@ -89,11 +90,13 @@ const ( ) type loaded struct { - result *configkit.Result[document] - backends map[string]map[string]fieldState - profiles map[string]map[string]fieldState - prompts map[string]map[string]*promptEntry - ambiguous map[string]bool + result *configkit.Result[document] + backends map[string]map[string]fieldState + profiles map[string]map[string]fieldState + prompts map[string]map[string]*promptEntry + ambiguous map[string]bool + registry *backend.Registry + indexedProfiles map[string]domain.ExecutionProfile } var envName = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) @@ -129,6 +132,9 @@ func load(main configkit.Input, underlays []configkit.Underlay) (*loaded, error) if err := state.materializePrompts(); err != nil { return nil, fmt.Errorf("load configuration: %w", err) } + if err := state.buildExecutionProfiles(); err != nil { + return nil, fmt.Errorf("load configuration: %w", err) + } return state, nil } diff --git a/internal/configuration/targets.go b/internal/configuration/targets.go new file mode 100644 index 0000000..bbedb1a --- /dev/null +++ b/internal/configuration/targets.go @@ -0,0 +1,171 @@ +package configuration + +import ( + "context" + "fmt" + + "gitea.maximumdirect.net/eric/promptkit/internal/backend" + "gitea.maximumdirect.net/eric/promptkit/internal/defaults" + "gitea.maximumdirect.net/eric/promptkit/internal/domain" + "gitea.maximumdirect.net/eric/promptkit/internal/jsonvalue" + profilepkg "gitea.maximumdirect.net/eric/promptkit/internal/profile" +) + +func (state *loaded) buildExecutionProfiles() error { + definitions := make(map[string]domain.Backend, len(state.result.Config.Backends)) + for _, id := range sortedKeys(state.result.Config.Backends) { + config := state.result.Config.Backends[id] + params, err := jsonvalue.CopyMap(config.ExtraParams) + if err != nil { + return fmt.Errorf("backend %q extra parameters: %w", id, err) + } + definition := domain.Backend{ID: id, ExtraParams: params} + if config.Endpoint != nil { + definition.Endpoint = *config.Endpoint + } + if config.APIKeyEnv != nil { + definition.APIKeyEnv = *config.APIKeyEnv + } + if config.ConcurrencyLimit != nil { + definition.ConcurrencyLimit = *config.ConcurrencyLimit + } + if config.QueueCapacity != nil { + definition.QueueCapacity = *config.QueueCapacity + definition.QueueCapacitySet = definition.ConcurrencyLimit > 0 + } + definitions[id] = definition + } + registry, err := backend.NewEffectiveRegistry(definitions) + if err != nil { + return err + } + state.registry = registry + state.indexedProfiles = make(map[string]domain.ExecutionProfile, len(state.result.Config.Profiles)) + for _, id := range sortedKeys(state.result.Config.Profiles) { + config := state.result.Config.Profiles[id] + fields := state.profiles[id] + target := defaults.ExecutionTargetDefault() + if config.Backend != nil && *config.Backend != "" { + selected, err := registry.GetBackend(*config.Backend) + if err != nil { + return fmt.Errorf("profile %q backend: %w", id, err) + } + target.BackendID = selected.ID + target.Endpoint = selected.Endpoint + target.APIKeyEnv = selected.APIKeyEnv + target.ExtraParams = selected.ExtraParams + } + presence := domain.ExecutionTargetPresence{} + if fields["endpoint"] != fieldAbsent { + target.Endpoint = valueOrZero(config.Endpoint) + } + if fields["model"] != fieldAbsent { + target.Model = valueOrZero(config.Model) + } + if fields["temperature"] != fieldAbsent { + target.Temperature = valueOrZero(config.Temperature) + presence.Temperature = fields["temperature"] == fieldValue + } + if fields["top_p"] != fieldAbsent { + target.TopP = valueOrZero(config.TopP) + presence.TopP = fields["top_p"] == fieldValue + } + if fields["max_tokens"] != fieldAbsent { + target.MaxTokens = valueOrZero(config.MaxTokens) + presence.MaxTokens = fields["max_tokens"] == fieldValue + } + if fields["timeout_seconds"] == fieldValue { + target.TimeoutSeconds = valueOrZero(config.TimeoutSeconds) + presence.TimeoutSeconds = true + } + if fields["service_tier"] != fieldAbsent { + target.ServiceTier = valueOrZero(config.ServiceTier) + } + if fields["reasoning_effort"] != fieldAbsent { + target.ReasoningEffort = valueOrZero(config.ReasoningEffort) + } + if fields["api_key_env"] != fieldAbsent { + target.APIKeyEnv = valueOrZero(config.APIKeyEnv) + } + if fields["api_key_required"] != fieldAbsent { + target.APIKeyRequired = valueOrZero(config.APIKeyRequired) + } + if target.APIKeyRequired && fields["api_key_env"] == fieldAbsent { + target.APIKeyEnv = "" + } + switch fields["extra_params"] { + case fieldNull: + target.ExtraParams = nil + case fieldValue: + merged, err := overlayProviderMap(target.ExtraParams, config.ExtraParams) + if err != nil { + return fmt.Errorf("profile %q extra parameters: %w", id, err) + } + target.ExtraParams = merged + } + profile := domain.ExecutionProfile{ID: id, BackendID: target.BackendID, ResolvedTarget: &target, TargetPresence: presence} + state.indexedProfiles[id] = profile + } + return nil +} + +func valueOrZero[T any](value *T) T { + if value == nil { + var zero T + return zero + } + return *value +} + +// overlayProviderMap applies only the backend-to-profile JSON-object default +// merge. ConfigKit has already resolved source layering and inheritance. +func overlayProviderMap(lower, upper map[string]any) (map[string]any, error) { + merged, err := jsonvalue.CopyMap(lower) + if err != nil { + return nil, err + } + if merged == nil { + merged = make(map[string]any) + } + for key, value := range upper { + if child, ok := value.(map[string]any); ok { + if existing, ok := merged[key].(map[string]any); ok { + combined, err := overlayProviderMap(existing, child) + if err != nil { + return nil, err + } + merged[key] = combined + continue + } + } + copied, err := jsonvalue.Copy(value) + if err != nil { + return nil, err + } + merged[key] = copied + } + return merged, nil +} + +// GetProfile returns an independently owned indexed profile for the internal +// runner's lookup boundary. +func (state *loaded) GetProfile(ctx context.Context, id string) (*domain.ExecutionProfile, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + if state == nil { + return nil, fmt.Errorf("%w: %q", profilepkg.ErrProfileNotFound, id) + } + profile, found := state.indexedProfiles[id] + if !found { + return nil, fmt.Errorf("%w: %q", profilepkg.ErrProfileNotFound, id) + } + target := *profile.ResolvedTarget + params, err := jsonvalue.CopyMap(target.ExtraParams) + if err != nil { + return nil, err + } + target.ExtraParams = params + profile.ResolvedTarget = &target + return &profile, nil +} diff --git a/internal/configuration/targets_test.go b/internal/configuration/targets_test.go new file mode 100644 index 0000000..e75111c --- /dev/null +++ b/internal/configuration/targets_test.go @@ -0,0 +1,152 @@ +package configuration + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "reflect" + "testing" + + "gitea.maximumdirect.net/eric/promptkit/internal/defaults" + "gitea.maximumdirect.net/eric/promptkit/internal/domain" + "gitea.maximumdirect.net/eric/promptkit/internal/llm" +) + +func TestLoadBuildsEffectiveTargetsAfterInheritance(t *testing.T) { + state, err := load(mapping("effective-targets", map[string]any{ + "backends": map[string]any{ + "first": map[string]any{"endpoint": "https://first.example/v1", "api_key_env": "FIRST_KEY", + "extra_params": map[string]any{"source": "first", "nested": map[string]any{"old": true}}}, + "second": map[string]any{"endpoint": "https://second.example/v1", "api_key_env": "SECOND_KEY", + "concurrency_limit": 2, "queue_capacity": 0, + "extra_params": map[string]any{"source": "second", "nested": map[string]any{"backend": true}}}, + }, + "profiles": map[string]any{ + "base": map[string]any{"backend": "first", "model": "model", "temperature": 0.8, + "top_p": 0.9, "max_tokens": 100, "timeout_seconds": 30, "api_key_required": true, + "extra_params": map[string]any{"nested": map[string]any{"profile": true}}}, + "child": map[string]any{"extends": "base", "backend": "second", "temperature": 0.0, + "top_p": nil, "max_tokens": 0, "timeout_seconds": nil, "api_key_env": nil, + "api_key_required": false, + "extra_params": map[string]any{"nested": map[string]any{"child": nil}}}, + "direct-key": map[string]any{"backend": "second", "api_key_required": true}, + "named-key": map[string]any{"backend": "second", "api_key_required": true, "api_key_env": "CUSTOM_KEY"}, + "no-provider-map": map[string]any{"backend": "second", "extra_params": nil}, + "no-timeout": map[string]any{"backend": "second", "timeout_seconds": 0}, + "partial": map[string]any{}, + "endpoint-only": map[string]any{"endpoint": "https://standalone.example/v1"}, + "cleared-endpoint": map[string]any{"extends": "base", "endpoint": nil}, + }, + }), nil) + if err != nil { + t.Fatal(err) + } + selected, err := state.registry.GetBackend("second") + if err != nil || selected.QueueCapacity != 0 || !selected.QueueCapacitySet { + t.Fatalf("backend capacity was not preserved: %+v, %v", selected, err) + } + child, err := state.GetProfile(context.Background(), "child") + if err != nil { + t.Fatal(err) + } + if _, err := state.GetProfile(context.Background(), " child "); err == nil { + t.Fatal("profile lookup silently changed a literal ID") + } + target := child.ResolvedTarget + if target.BackendID != "second" || target.Endpoint != "https://second.example/v1" || target.Model != "model" || + target.Temperature != 0 || target.TopP != 0 || target.MaxTokens != 0 || + target.TimeoutSeconds != defaults.ExecutionDefaultTimeoutSeconds || target.APIKeyEnv != "" || target.APIKeyRequired { + t.Fatalf("effective child target is wrong: %+v", target) + } + if !child.TargetPresence.Temperature || !child.TargetPresence.MaxTokens || child.TargetPresence.TopP || child.TargetPresence.TimeoutSeconds { + t.Fatalf("numeric presence was not retained: %+v", child.TargetPresence) + } + wantParams := map[string]any{"source": "second", "nested": map[string]any{ + "backend": true, "profile": true, "child": nil, + }} + if !reflect.DeepEqual(target.ExtraParams, wantParams) { + t.Fatalf("backend/profile provider map overlay failed: %+v", target.ExtraParams) + } + if _, exists := target.ExtraParams["nested"].(map[string]any)["old"]; exists { + t.Fatal("previous backend derived parameter leaked into child") + } + for id, wantEnv := range map[string]string{"direct-key": "", "named-key": "CUSTOM_KEY"} { + profile, err := state.GetProfile(context.Background(), id) + if err != nil || profile.ResolvedTarget.APIKeyEnv != wantEnv { + t.Fatalf("credential source for %s: %+v, %v", id, profile, err) + } + } + noMap, _ := state.GetProfile(context.Background(), "no-provider-map") + if noMap.ResolvedTarget.ExtraParams != nil { + t.Fatalf("explicit null provider map did not suppress backend defaults: %+v", noMap.ResolvedTarget.ExtraParams) + } + noTimeout, _ := state.GetProfile(context.Background(), "no-timeout") + if noTimeout.ResolvedTarget.TimeoutSeconds != 0 || !noTimeout.TargetPresence.TimeoutSeconds { + t.Fatalf("explicit zero timeout was lost: %+v", noTimeout) + } + partial, _ := state.GetProfile(context.Background(), "partial") + if partial.ResolvedTarget.Endpoint != "" || partial.ResolvedTarget.Model != "" { + t.Fatalf("partial profile gained a runnable target: %+v", partial.ResolvedTarget) + } + endpointOnly, _ := state.GetProfile(context.Background(), "endpoint-only") + if endpointOnly.ResolvedTarget.Endpoint != "https://standalone.example/v1" || + endpointOnly.ResolvedTarget.BackendID != "" || endpointOnly.ResolvedTarget.Model != "" { + t.Fatalf("endpoint-only profile was not retained: %+v", endpointOnly.ResolvedTarget) + } + clearedEndpoint, _ := state.GetProfile(context.Background(), "cleared-endpoint") + if clearedEndpoint.ResolvedTarget.Endpoint != "" || clearedEndpoint.ResolvedTarget.BackendID != "first" { + t.Fatalf("explicit null recovered backend endpoint: %+v", clearedEndpoint.ResolvedTarget) + } + child.ResolvedTarget.ExtraParams["source"] = "mutated" + again, _ := state.GetProfile(context.Background(), "child") + if again.ResolvedTarget.ExtraParams["source"] != "second" { + t.Fatal("profile lookup exposed snapshot ownership") + } +} + +func TestLoadedZeroControlsReachBuiltInRequest(t *testing.T) { + payloads := make(chan map[string]any, 1) + server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + var payload map[string]any + if err := json.NewDecoder(request.Body).Decode(&payload); err != nil { + writer.WriteHeader(http.StatusBadRequest) + return + } + payloads <- payload + writer.Header().Set("Content-Type", "application/json") + _, _ = writer.Write([]byte(`{"choices":[{"message":{"content":"ok"}}]}`)) + })) + defer server.Close() + state, err := load(mapping("wire-controls", map[string]any{ + "backends": map[string]any{"local": map[string]any{"endpoint": server.URL + "/v1"}}, + "profiles": map[string]any{"direct": map[string]any{ + "backend": "local", "model": "example", "temperature": 0.0, "max_tokens": 0, "top_p": nil, + }}, + }), nil) + if err != nil { + t.Fatal(err) + } + profile, err := state.GetProfile(context.Background(), "direct") + if err != nil { + t.Fatal(err) + } + client, err := llm.NewOpenAICompatibleClient(llm.OpenAICompatibleConfig{}) + if err != nil { + t.Fatal(err) + } + _, err = client.Generate(context.Background(), domain.GenerateRequest{ + Prompt: domain.RenderedPrompt{Messages: []domain.RenderedMessage{{Role: "user", Content: "hello"}}}, + Target: *profile.ResolvedTarget, TargetPresence: profile.TargetPresence, + }) + if err != nil { + t.Fatal(err) + } + payload := <-payloads + if payload["temperature"] != float64(0) || payload["max_tokens"] != float64(0) { + t.Fatalf("explicit zeros were omitted from outbound request: %+v", payload) + } + if _, exists := payload["top_p"]; exists { + t.Fatalf("explicit null top_p was sent: %+v", payload) + } +} diff --git a/internal/domain/domain.go b/internal/domain/domain.go index b2dede8..4522e29 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -208,6 +208,10 @@ type ExecutionProfile struct { APIKeyEnv string `yaml:"api_key_env"` APIKeyRequired bool `yaml:"-" json:"-"` ExtraParams map[string]any `yaml:"extra_params"` + // ResolvedTarget is an execution-owned snapshot assembled from effective + // configuration and backend defaults. Legacy source repositories leave it nil. + ResolvedTarget *ExecutionTarget `yaml:"-" json:"-"` + TargetPresence ExecutionTargetPresence `yaml:"-" json:"-"` } // ExecutionTargetOverride represents per-request runtime setting overrides. @@ -224,8 +228,9 @@ type ExecutionTargetOverride struct { ExtraParams map[string]any `json:"extra_params,omitempty"` } -// ExecutionTargetPresence tracks which effective runtime fields came from an -// explicit request override even when the resolved value is a zero value. +// ExecutionTargetPresence tracks effective numeric values that must be sent +// even when they are zero. Values may come from configuration or a request +// override; explicit configuration null suppresses the corresponding bit. type ExecutionTargetPresence struct { Temperature bool MaxTokens bool diff --git a/internal/usecase/profile_inspection.go b/internal/usecase/profile_inspection.go index 3d84026..65c457e 100644 --- a/internal/usecase/profile_inspection.go +++ b/internal/usecase/profile_inspection.go @@ -36,6 +36,9 @@ func (r *Runner) resolveProfileSelection( } profileValue := *selectedProfile + if profileValue.ResolvedTarget != nil { + return &resolvedProfileSelection{id: normalizedID, profile: &profileValue}, nil + } profileValue.BackendID = strings.TrimSpace(profileValue.BackendID) var selectedBackend *domain.Backend @@ -92,9 +95,11 @@ func (r *Runner) InspectProfile( return nil, err } target, _ := resolveExecutionTarget(selection.backend, selection.profile, nil) - target, err = normalizeResolvedExecutionTarget(target) - if err != nil { - return nil, fmt.Errorf("%w: %w", ErrProfileLoad, err) + if selection.profile.ResolvedTarget == nil { + target, err = normalizeResolvedExecutionTarget(target) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrProfileLoad, err) + } } target.APIKey = "" diff --git a/internal/usecase/resolved_targets_test.go b/internal/usecase/resolved_targets_test.go new file mode 100644 index 0000000..43062bb --- /dev/null +++ b/internal/usecase/resolved_targets_test.go @@ -0,0 +1,85 @@ +package usecase + +import ( + "context" + "errors" + "testing" + + "gitea.maximumdirect.net/eric/promptkit/internal/defaults" + "gitea.maximumdirect.net/eric/promptkit/internal/domain" +) + +func TestInspectProfileAllowsIncompleteResolvedTarget(t *testing.T) { + profile := &domain.ExecutionProfile{ID: "partial", ResolvedTarget: &domain.ExecutionTarget{ + TimeoutSeconds: defaults.ExecutionDefaultTimeoutSeconds, + }} + runner := &Runner{profiles: staticExecutionProfileRepo{profile: profile}} + inspection, err := runner.InspectProfile(context.Background(), "partial") + if err != nil { + t.Fatalf("partial profile inspection failed: %v", err) + } + if inspection.EffectiveModelParams.Endpoint != "" || inspection.EffectiveModelParams.Model != "" { + t.Fatalf("inspection invented a runnable target: %+v", inspection) + } +} + +func TestPreparationCompletesResolvedTargetThroughRequestOverrides(t *testing.T) { + profile := &domain.ExecutionProfile{ID: "partial", ResolvedTarget: &domain.ExecutionTarget{ + TimeoutSeconds: defaults.ExecutionDefaultTimeoutSeconds, + Temperature: 0, ExtraParams: map[string]any{"configured": true}, + }, TargetPresence: domain.ExecutionTargetPresence{Temperature: true}} + runner := &Runner{ + promptDefs: staticPromptRepo{def: &domain.PromptDefinition{ID: "prompt", Version: "v1", Validation: domain.OutputContract{ + Format: domain.FormatText, ValidationMode: domain.ValidationNone, + }}}, + profiles: staticExecutionProfileRepo{profile: profile}, renderer: staticRenderer{}, + } + if _, err := runner.Prepare(context.Background(), domain.RunRequest{PromptID: "prompt", ProfileID: "partial"}); err == nil { + t.Fatal("incomplete target was accepted without overrides") + } + zero := 0.0 + prepared, err := runner.Prepare(context.Background(), domain.RunRequest{ + PromptID: "prompt", ProfileID: "partial", + Execution: &domain.ExecutionTargetOverride{ + Endpoint: "https://runtime.example/v1", Model: "runtime-model", Temperature: &zero, + ExtraParams: map[string]any{"request": true}, + }, + }) + if err != nil { + t.Fatalf("request did not complete partial target: %v", err) + } + if prepared.EffectiveModelParams.Endpoint != "https://runtime.example/v1" || + prepared.EffectiveModelParams.Model != "runtime-model" || !prepared.TargetPresence.Temperature || + prepared.EffectiveModelParams.ExtraParams["request"] != true { + t.Fatalf("request overrides were not applied: %+v", prepared) + } + if _, exists := prepared.EffectiveModelParams.ExtraParams["configured"]; exists { + t.Fatal("nonempty request provider map did not replace configured map") + } +} + +func TestResolvedTargetCredentialAvailability(t *testing.T) { + profile := &domain.ExecutionProfile{ID: "direct", ResolvedTarget: &domain.ExecutionTarget{ + Endpoint: "https://provider.example/v1", Model: "model", APIKeyRequired: true, + }} + runner := &Runner{ + promptDefs: staticPromptRepo{def: &domain.PromptDefinition{ID: "prompt", Version: "v1", Validation: domain.OutputContract{ + Format: domain.FormatText, ValidationMode: domain.ValidationNone, + }}}, + profiles: staticExecutionProfileRepo{profile: profile}, renderer: staticRenderer{}, + } + request := domain.RunRequest{PromptID: "prompt", ProfileID: "direct"} + if _, err := runner.Prepare(context.Background(), request); !errors.Is(err, ErrAPIKeyRequired) { + t.Fatalf("missing required direct key: %v", err) + } + request.APIKey = "direct-secret" + if _, err := runner.Prepare(context.Background(), request); err != nil { + t.Fatalf("direct key should satisfy requirement: %v", err) + } + request.APIKey = "" + t.Setenv("REQUEST_KEY", "runtime-secret") + request.Execution = &domain.ExecutionTargetOverride{APIKeyEnv: "REQUEST_KEY"} + if _, err := runner.Prepare(context.Background(), request); err != nil { + t.Fatalf("request environment override should satisfy requirement: %v", err) + } +} diff --git a/internal/usecase/runner.go b/internal/usecase/runner.go index 38b6bcd..41af2cc 100644 --- a/internal/usecase/runner.go +++ b/internal/usecase/runner.go @@ -620,9 +620,8 @@ func mergeExecutionTarget(base domain.ExecutionTarget, override domain.Execution return out } -func mergeExecutionTargetOverride(base domain.ExecutionTarget, override domain.ExecutionTargetOverride) (domain.ExecutionTarget, domain.ExecutionTargetPresence) { +func mergeExecutionTargetOverride(base domain.ExecutionTarget, presence domain.ExecutionTargetPresence, override domain.ExecutionTargetOverride) (domain.ExecutionTarget, domain.ExecutionTargetPresence) { out := base - var presence domain.ExecutionTargetPresence if override.Endpoint != "" { out.Endpoint = override.Endpoint } @@ -661,12 +660,21 @@ func mergeExecutionTargetOverride(base domain.ExecutionTarget, override domain.E } func resolveExecutionTarget(backendValue *domain.Backend, profileValue *domain.ExecutionProfile, override *domain.ExecutionTargetOverride) (domain.ExecutionTarget, domain.ExecutionTargetPresence) { + if profileValue != nil && profileValue.ResolvedTarget != nil { + out := *profileValue.ResolvedTarget + out.ExtraParams = copyExtraParams(out.ExtraParams) + presence := profileValue.TargetPresence + if override != nil { + return mergeExecutionTargetOverride(out, presence, *override) + } + return out, presence + } out := defaults.ExecutionTargetDefault() out = mergeExecutionTarget(out, backendToTarget(backendValue)) out = mergeExecutionTarget(out, executionProfileToTarget(profileValue)) var presence domain.ExecutionTargetPresence if override != nil { - out, presence = mergeExecutionTargetOverride(out, *override) + out, presence = mergeExecutionTargetOverride(out, presence, *override) } return out, presence } diff --git a/types.go b/types.go index 436ae29..e1a2016 100644 --- a/types.go +++ b/types.go @@ -573,18 +573,17 @@ type OpenAICompatibleProfileConfig struct { ExtraParams map[string]any } -// ExecutionTargetPresence tracks which numeric runtime settings were explicit -// request overrides, including explicit zero values. It has a stable JSON -// representation and is supplied to injected LLM clients so they can preserve -// omission semantics. +// ExecutionTargetPresence reports which effective numeric settings should be +// sent even when zero. It has a stable JSON representation and lets injected +// clients preserve omission semantics. type ExecutionTargetPresence struct { - // Temperature reports a non-nil ExecutionTargetOverride.Temperature. + // Temperature keeps a zero temperature explicit. Temperature bool `json:"temperature"` - // MaxTokens reports a non-nil ExecutionTargetOverride.MaxTokens. + // MaxTokens keeps a zero token maximum explicit. MaxTokens bool `json:"max_tokens"` - // TopP reports a non-nil ExecutionTargetOverride.TopP. + // TopP keeps a zero nucleus value explicit. TopP bool `json:"top_p"` - // TimeoutSeconds reports a non-nil ExecutionTargetOverride.TimeoutSeconds. + // TimeoutSeconds keeps a zero timeout explicit. TimeoutSeconds bool `json:"timeout_seconds"` } @@ -739,8 +738,8 @@ type GenerateRequest struct { Prompt RenderedPrompt `json:"prompt"` // Target contains effective model settings without the direct API key. Target ExecutionTarget `json:"target"` - // TargetPresence distinguishes inherited numeric zeros from explicit - // request overrides. + // TargetPresence distinguishes omitted or suppressed controls from explicit + // numeric values, including zero. TargetPresence ExecutionTargetPresence `json:"target_presence"` // StructuredOutput contains provider response constraints when requested. StructuredOutput *StructuredOutputSpec `json:"structured_output,omitempty"`