diff --git a/docs/cli.md b/docs/cli.md index 2ed830b..f29277b 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -39,7 +39,7 @@ pipeline ID and **--input** are required. | **--debug** | Retain a debug bundle for this run. | | **--debug-dir path** | Override the debug-bundle root. Requires **--debug**. | | **--only lane-a,lane-b** | Run only the selected comma-separated artifact lanes when that selection is valid for the configured pipeline. | -| **--llm-profile id** | Override effective LLM-capable module bindings with one configured profile. | +| **--llm-profile id** | Highest-precedence configured profile for selected LLM-backed bindings and validators; it replaces binding and [pipeline](config.md#pipelines) defaults. | | **--session-id id** | Supply a non-empty prompt session identifier to LLM-backed module calls. | | **--reasoning-effort value** | Replace the selected PromptKit profile's reasoning effort for every LLM-backed call in this run. The value must be non-empty and the flag may be specified only once. | | **--clear-reasoning-effort** | Clear reasoning effort inherited from the selected PromptKit profile for every LLM-backed call in this run. | diff --git a/docs/config.md b/docs/config.md index bcc43b4..c946558 100644 --- a/docs/config.md +++ b/docs/config.md @@ -197,6 +197,7 @@ pipelines: | Field | Type | Default | Rules | | --- | --- | --- | --- | +| **llm_profile** | string | none | Optional non-empty default PromptKit profile ID for selected LLM-backed bindings and validators. | | **input** | module binding | none | Required. | | **chunk** | module binding | **generic** | Optional. | | **output** | module binding | **json** | Optional. | @@ -210,6 +211,11 @@ needs a unique non-empty **id**, an **artifacts** map, and may have **references**. A lane ID must not appear more than once in a pipeline, including across explicit steps. +For each selected LLM-backed binding or validator, profile selection uses the +run-level **--llm-profile** value first, then the binding's **llm_profile**, +then the pipeline's **llm_profile**, and finally the PromptKit default. +Deterministic bindings do not receive these defaults or run overrides. + A lane has these fields: | Field | Type | Default | Rules | @@ -246,7 +252,7 @@ extract: | Binding field | Type | Default | Rules | | --- | --- | --- | --- | | **module** | string | none | Required for an object binding. Must be a registered compatible key. | -| **llm_profile** | string | none | Optional non-empty PromptKit profile ID. | +| **llm_profile** | string | none | Optional non-empty PromptKit profile ID for an LLM-backed binding. It overrides the pipeline default unless the run supplies **--llm-profile**. | | **retries** | integer | 0 | Non-negative additional attempts for chunk, extract, merge, and normalize bindings. | | **options** | object | none | Must satisfy the selected module. | | **references** | map | none | Valid only on chunk, extract, merge, and normalize bindings. | @@ -256,7 +262,8 @@ Omitting **validators** uses the registered chain. **validators: []** selects an empty chain; a non-empty list replaces the chain in the listed order. Validator bindings accept only **module**, **llm_profile**, and **options**. They reject **references**, **retries**, and nested **validators**. Deterministic -validators reject an explicit **llm_profile**. +validators reject an explicit **llm_profile**. Deterministic module bindings +also reject an explicit **llm_profile**. The **json** output module accepts optional **include_chunk_map** and **evidence_context** settings: diff --git a/internal/cli/command_contract_test.go b/internal/cli/command_contract_test.go index 8475958..70c5c35 100644 --- a/internal/cli/command_contract_test.go +++ b/internal/cli/command_contract_test.go @@ -154,6 +154,23 @@ func TestConfigValidateResolvesPipelineAndChecksSelection(t *testing.T) { } } +func TestConfigValidatePipelineDefaultProfileIsOffline(t *testing.T) { + configPath := writeCommandConfigContent(t, `version: 4 +pipelines: + demo: + llm_profile: dnd-extraction + input: seriatim + artifacts: + spells: + extract: dnd/spells +`) + var stdout, stderr bytes.Buffer + code := RunWithOptions([]string{"config", "validate", "--config", configPath, "--pipeline", "demo"}, &stdout, &stderr, Options{}) + if code != 0 || !strings.Contains(stdout.String(), "valid for pipeline \"demo\"") || stderr.Len() != 0 { + t.Fatalf("code=%d stdout=%q stderr=%q", code, stdout.String(), stderr.String()) + } +} + func TestPipelinesListSortsNormalizedIDsInTextAndJSON(t *testing.T) { configPath := writeCommandConfig(t, " zeta ", "alpha") options := commandContractOptions(t) diff --git a/internal/cli/run.go b/internal/cli/run.go index 5b87d98..3c361d6 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -954,11 +954,22 @@ func effectiveLLMProfileIDs(resolved pipeline.ResolvedPipeline) []string { seen[id] = struct{}{} } } - add(resolved.Chunk) + if resolved.InputExecutionClass == contracts.ExecutionClassLLMBacked { + add(resolved.Input) + } + if resolved.ChunkExecutionClass == contracts.ExecutionClassLLMBacked { + add(resolved.Chunk) + } for _, lane := range resolved.AllArtifactLanes() { - add(lane.Extract) - add(lane.Merge) - add(lane.Normalize) + if lane.ExtractExecutionClass == contracts.ExecutionClassLLMBacked { + add(lane.Extract) + } + if lane.MergeExecutionClass == contracts.ExecutionClassLLMBacked { + add(lane.Merge) + } + if lane.NormalizeExecutionClass == contracts.ExecutionClassLLMBacked { + add(lane.Normalize) + } } for _, chain := range resolved.ValidatorChains { for _, validator := range chain.Validators { @@ -967,6 +978,9 @@ func effectiveLLMProfileIDs(resolved pipeline.ResolvedPipeline) []string { } } } + if resolved.OutputExecutionClass == contracts.ExecutionClassLLMBacked { + add(resolved.Output) + } ids := make([]string, 0, len(seen)) for id := range seen { ids = append(ids, id) diff --git a/internal/cli/run_contract_test.go b/internal/cli/run_contract_test.go index ff1a922..7301729 100644 --- a/internal/cli/run_contract_test.go +++ b/internal/cli/run_contract_test.go @@ -318,6 +318,24 @@ func TestRunLLMProfileOverrideAndValidationUseInjectedBoundaries(t *testing.T) { t.Fatalf("code=%d stdout=%q stderr=%q factoryCalls=%d", code, stdout.String(), stderr.String(), factoryCalls) } }) + + t.Run("pipeline default is rejected before factory access", func(t *testing.T) { + roots := newStateTestRoots(t) + profileDir := writeRunContractProfiles(t, "configured-profile") + prependRunContractConfig(t, roots, fmt.Sprintf("promptkit:\n profile_dir: %q\n", profileDir)) + replaceStateTestConfigLine(t, roots.config, " sample:\n", " sample:\n llm_profile: missing-profile\n") + factoryCalls := 0 + opts := newStateTestHarness().options() + opts.LLMClientFactory = func(context.Context, config.Config, string, LLMRuntimeOverrides) (contracts.StructuredLLMClient, []artifacts.LLMProfileManifest, error) { + factoryCalls++ + return nil, nil, nil + } + var stdout, stderr bytes.Buffer + code := RunWithOptions([]string{"run", "sample", "--config", roots.config, "--input", roots.input, "--chunk_cache", "bypass"}, &stdout, &stderr, opts) + if code != 1 || !strings.Contains(stderr.String(), "not configured") || factoryCalls != 0 || stdout.Len() != 0 { + t.Fatalf("code=%d stdout=%q stderr=%q factoryCalls=%d", code, stdout.String(), stderr.String(), factoryCalls) + } + }) } func TestRunReasoningEffortOverrideReachesFactory(t *testing.T) { @@ -445,24 +463,30 @@ func TestReasoningEffortOverrideSeparatesCheckpointIdentities(t *testing.T) { func TestEffectiveLLMProfileIDsAreSortedDeduplicatedAndLLMOnly(t *testing.T) { resolved := pipeline.ResolvedPipeline{ - Input: pipeline.ModuleBinding{LLMProfile: "input-profile"}, - Chunk: pipeline.ModuleBinding{LLMProfile: " zeta "}, + Input: pipeline.ModuleBinding{LLMProfile: "input-profile"}, + InputExecutionClass: contracts.ExecutionClassLLMBacked, + Chunk: pipeline.ModuleBinding{LLMProfile: " zeta "}, + ChunkExecutionClass: contracts.ExecutionClassLLMBacked, Steps: []pipeline.ResolvedPipelineStep{{ ID: "default", ArtifactLanes: []pipeline.ResolvedArtifactLane{{ - Extract: pipeline.ModuleBinding{LLMProfile: "alpha"}, - Merge: pipeline.ModuleBinding{LLMProfile: "zeta"}, - Normalize: pipeline.ModuleBinding{LLMProfile: " gamma "}, + Extract: pipeline.ModuleBinding{LLMProfile: "alpha"}, + ExtractExecutionClass: contracts.ExecutionClassLLMBacked, + Merge: pipeline.ModuleBinding{LLMProfile: "deterministic-merge"}, + MergeExecutionClass: contracts.ExecutionClassDeterministic, + Normalize: pipeline.ModuleBinding{LLMProfile: " gamma "}, + NormalizeExecutionClass: contracts.ExecutionClassLLMBacked, }}, }}, ValidatorChains: []pipeline.ResolvedValidatorChain{{Validators: []pipeline.ResolvedValidator{ {Binding: pipeline.ModuleBinding{LLMProfile: "deterministic-profile"}, ExecutionClass: contracts.ExecutionClassDeterministic}, {Binding: pipeline.ModuleBinding{LLMProfile: "beta"}, ExecutionClass: contracts.ExecutionClassLLMBacked}, }}}, - Output: pipeline.ModuleBinding{LLMProfile: "output-profile"}, + Output: pipeline.ModuleBinding{LLMProfile: "output-profile"}, + OutputExecutionClass: contracts.ExecutionClassLLMBacked, } got := effectiveLLMProfileIDs(resolved) - want := []string{"alpha", "beta", "gamma", "zeta"} + want := []string{"alpha", "beta", "gamma", "input-profile", "output-profile", "zeta"} if strings.Join(got, ",") != strings.Join(want, ",") { t.Fatalf("effective profiles = %#v, want %#v", got, want) } diff --git a/internal/core/config/effective_config_contract_test.go b/internal/core/config/effective_config_contract_test.go index 51f322d..efcee44 100644 --- a/internal/core/config/effective_config_contract_test.go +++ b/internal/core/config/effective_config_contract_test.go @@ -242,6 +242,25 @@ func TestEffectiveConfigLLMProfileOverrideChangesDigestAndOverridesValidators(t } } +func TestEffectiveConfigPipelineLLMProfileIsInheritedWithoutMutatingConfig(t *testing.T) { + profile := effectiveProfile() + profile.LLMProfile = " configured-profile " + effective, err := resolveEffectiveProfile(t, profile, ResolveInput{}) + if err != nil { + t.Fatalf("Resolve() error = %v", err) + } + if got := effective.Config.Pipelines["main"].LLMProfile; got != " configured-profile " { + t.Fatalf("effective config pipeline llm profile = %q, want preserved programmatic value", got) + } + resolved := effective.ResolvedPipeline + if got := resolved.Chunk.LLMProfile; got != "configured-profile" { + t.Fatalf("resolved chunk profile = %q, want inherited profile", got) + } + if got := resolved.Steps[0].ArtifactLanes[0].Extract.LLMProfile; got != "configured-profile" { + t.Fatalf("resolved extract profile = %q, want inherited profile", got) + } +} + func TestEffectiveConfigValidatorOverridesRemainDistinctAndOrdered(t *testing.T) { tests := []struct { name string diff --git a/internal/core/config/file_config.go b/internal/core/config/file_config.go index 510496e..f3c5024 100644 --- a/internal/core/config/file_config.go +++ b/internal/core/config/file_config.go @@ -34,21 +34,23 @@ type FilePromptKitLocalBackendConfig struct { } type FilePipelineProfile struct { - Input fileModuleBinding `yaml:"input"` - Chunk *fileModuleBinding `yaml:"chunk,omitempty"` - Artifacts map[string]FileArtifactLaneProfile `yaml:"artifacts,omitempty"` - Steps []FilePipelineStepProfile `yaml:"steps,omitempty"` - Output *fileModuleBinding `yaml:"output,omitempty"` - References map[string]fileReferenceSource `yaml:"references,omitempty"` - artifactsSet bool `yaml:"-"` - stepsSet bool `yaml:"-"` + LLMProfile *string `yaml:"llm_profile,omitempty"` + Input fileModuleBinding `yaml:"input"` + Chunk *fileModuleBinding `yaml:"chunk,omitempty"` + Artifacts map[string]FileArtifactLaneProfile `yaml:"artifacts,omitempty"` + Steps []FilePipelineStepProfile `yaml:"steps,omitempty"` + Output *fileModuleBinding `yaml:"output,omitempty"` + References map[string]fileReferenceSource `yaml:"references,omitempty"` + artifactsSet bool `yaml:"-"` + stepsSet bool `yaml:"-"` + llmProfileSet bool `yaml:"-"` } func (p *FilePipelineProfile) UnmarshalYAML(node *yaml.Node) error { type plainFilePipelineProfile FilePipelineProfile var decoded plainFilePipelineProfile seen, err := decodeKnownMapping(node, &decoded, map[string]struct{}{ - "input": {}, "chunk": {}, "artifacts": {}, "steps": {}, "output": {}, "references": {}, + "llm_profile": {}, "input": {}, "chunk": {}, "artifacts": {}, "steps": {}, "output": {}, "references": {}, }, "pipeline profile") if err != nil { return err @@ -56,6 +58,7 @@ func (p *FilePipelineProfile) UnmarshalYAML(node *yaml.Node) error { *p = FilePipelineProfile(decoded) _, p.artifactsSet = seen["artifacts"] _, p.stepsSet = seen["steps"] + _, p.llmProfileSet = seen["llm_profile"] return nil } @@ -492,6 +495,13 @@ func (c *Config) applyFileConfigWithLookup(fileCfg FileConfig, lookup func(strin for _, pipelineID := range pipelineIDs { filePipeline := fileCfg.Pipelines[rawPipelineIDs[pipelineID]] + llmProfile := "" + if filePipeline.llmProfileSet || filePipeline.LLMProfile != nil { + if filePipeline.LLMProfile == nil || strings.TrimSpace(*filePipeline.LLMProfile) == "" { + return fmt.Errorf("pipeline %q llm_profile must not be empty when set", pipelineID) + } + llmProfile = strings.TrimSpace(*filePipeline.LLMProfile) + } hasSteps := filePipeline.stepsSet || filePipeline.Steps != nil laneIDs, rawLaneIDs, err := normalizedMapKeys(filePipeline.Artifacts, fmt.Sprintf("pipeline %q artifact lane id", pipelineID)) if err != nil { @@ -499,6 +509,7 @@ func (c *Config) applyFileConfigWithLookup(fileCfg FileConfig, lookup func(strin } profile := pipeline.PipelineProfile{ ID: pipelineID, + LLMProfile: llmProfile, Input: filePipeline.Input.toPipelineBinding(), Artifacts: make(map[string]pipeline.ArtifactLaneProfile, len(filePipeline.Artifacts)), References: fileReferenceSourcesToPipeline(filePipeline.References), diff --git a/internal/core/config/file_config_contract_test.go b/internal/core/config/file_config_contract_test.go index e3559fb..d4a7af0 100644 --- a/internal/core/config/file_config_contract_test.go +++ b/internal/core/config/file_config_contract_test.go @@ -2,6 +2,7 @@ package config import ( "encoding/json" + "fmt" "os" "path/filepath" "reflect" @@ -49,6 +50,72 @@ func TestFileConfigMinimalVersion4AppliesOverDefaults(t *testing.T) { } } +func TestFilePipelineLLMProfileIsPresenceAwareAndDetached(t *testing.T) { + const pipelineYAML = `version: 4 +pipelines: + main: + %s + input: input + artifacts: + lane: + extract: extract +` + + t.Run("omitted", func(t *testing.T) { + file := parseFileConfig(t, fmt.Sprintf(pipelineYAML, "")) + if file.Pipelines["main"].LLMProfile != nil || file.Pipelines["main"].llmProfileSet { + t.Fatalf("parsed pipeline profile = %#v, want omitted llm profile", file.Pipelines["main"]) + } + cfg := Default() + if err := cfg.ApplyFileConfig(file); err != nil { + t.Fatal(err) + } + if got := cfg.Pipelines["main"].LLMProfile; got != "" { + t.Fatalf("pipeline llm profile = %q, want empty", got) + } + }) + + t.Run("trimmed and detached", func(t *testing.T) { + file := parseFileConfig(t, fmt.Sprintf(pipelineYAML, "llm_profile: ' configured-profile '")) + cfg := Default() + if err := cfg.ApplyFileConfig(file); err != nil { + t.Fatal(err) + } + if got := cfg.Pipelines["main"].LLMProfile; got != "configured-profile" { + t.Fatalf("pipeline llm profile = %q, want trimmed value", got) + } + *file.Pipelines["main"].LLMProfile = "changed-profile" + if got := cfg.Pipelines["main"].LLMProfile; got != "configured-profile" { + t.Fatalf("effective config aliases parsed file: %q", got) + } + if got := cloneConfig(cfg).Pipelines["main"].LLMProfile; got != "configured-profile" { + t.Fatalf("cloned pipeline llm profile = %q", got) + } + data, err := json.Marshal(cfg) + if err != nil { + t.Fatal(err) + } + var roundTripped Config + if err := json.Unmarshal(data, &roundTripped); err != nil { + t.Fatal(err) + } + if got := roundTripped.Pipelines["main"].LLMProfile; got != "configured-profile" { + t.Fatalf("round-tripped pipeline llm profile = %q", got) + } + }) + + for _, value := range []string{"''", "' '", "null"} { + t.Run("explicit empty "+value, func(t *testing.T) { + file := parseFileConfig(t, fmt.Sprintf(pipelineYAML, "llm_profile: "+value)) + cfg := Default() + err := cfg.ApplyFileConfig(file) + if err == nil || !strings.Contains(err.Error(), `pipeline "main" llm_profile must not be empty`) { + t.Fatalf("ApplyFileConfig() error = %v, want explicit-empty rejection", err) + } + }) + } +} + func TestFilePromptKitProfileSourcesSurviveConfigBoundaries(t *testing.T) { tests := []struct { name string