From d42e2095d7366a24e65fa56dd8048792043dc49f Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 10 Nov 2026 17:18:30 +0000 Subject: [PATCH] Resolve pipeline settings and collection selections --- docs/roadmap/implementation.md | 5 + internal/cli/process_command.go | 35 ++++ .../cli/process_command_integration_test.go | 27 +++ internal/cli/process_inputs_test.go | 30 ++++ internal/core/config/config.go | 46 +++-- internal/core/config/resolve.go | 73 ++++++++ internal/core/config/resolve_test.go | 160 ++++++++++++++++++ 7 files changed, 358 insertions(+), 18 deletions(-) create mode 100644 internal/core/config/resolve.go create mode 100644 internal/core/config/resolve_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 70d2f70..ff9086c 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -425,6 +425,11 @@ explicit and absent main configuration. Test integration, not native grammar aga ## Stage 10: Resolve stage settings and native CLI selection inputs +**Completed.** ConfigKit records root and per-entry defaults, explicit clears, +validation-profile fallback, and dependencies. Typed JSON list flags feed root +overrides. Vocabulary derivation and the ordered runner remain assigned below; +focused loader and command checks passed with a controlled runner boundary. + Own `internal/core/config` resolution and the matching process flag bindings. Use [stage scope](glossary-stages.md#configuration-scope--resolved), [clearing](glossary-stages.md#compound-values-and-explicit-clearing--resolved), and diff --git a/internal/cli/process_command.go b/internal/cli/process_command.go index 34711fb..a3dd75c 100644 --- a/internal/cli/process_command.go +++ b/internal/cli/process_command.go @@ -1,6 +1,7 @@ package cli import ( + "fmt" "strconv" commandkit "go.maximumdirect.net/commandkit" @@ -73,6 +74,20 @@ func processCommand() commandkit.Command { } })) } + for _, item := range []struct { + name string + path ck.Path + }{ + {"glossaries", ck.Keys("glossaries")}, {"protected-terms", ck.Keys("protected_terms")}, + } { + name, path := item.name, item.path + flags = append(flags, commandkit.ValueFlag(commandkit.FlagMeta{Name: name, Usage: "JSON array of collection names, or null."}, (*any)(nil), parseCollectionNames, + func(in *processInput, value commandkit.Supplied[any]) { + if value.Set { + in.Overrides = append(in.Overrides, ck.Override{Path: path, Value: value.Value, Flag: "--" + name}) + } + })) + } flags = append(flags, commandkit.BoolFlag(commandkit.FlagMeta{Name: "reporting", Usage: "Enable ordinary reports."}, false, func(in *processInput, v commandkit.Supplied[bool]) { if v.Set { @@ -83,3 +98,23 @@ func processCommand() commandkit.Command { Arguments: []commandkit.Argument[processInput]{commandkit.ValueArgument(commandkit.ArgumentMeta{Name: "transcript", Usage: "Transcript JSON file."}, true, stringValue, func(in *processInput, v commandkit.Supplied[string]) { in.Transcript = v.Value })}, }) } + +func parseCollectionNames(raw string) (any, error) { + value, err := commandkit.ParseCLIJSON(raw) + if err != nil { + return nil, err + } + if value == nil { + return nil, nil + } + names, ok := value.([]any) + if !ok { + return nil, fmt.Errorf("expected a JSON array of collection names or null") + } + for _, name := range names { + if _, ok := name.(string); !ok { + return nil, fmt.Errorf("collection names must be strings") + } + } + return names, nil +} diff --git a/internal/cli/process_command_integration_test.go b/internal/cli/process_command_integration_test.go index 86b33ca..35ffe5b 100644 --- a/internal/cli/process_command_integration_test.go +++ b/internal/cli/process_command_integration_test.go @@ -56,3 +56,30 @@ func TestProcessUsesResolvedReportingMode(t *testing.T) { }) } } + +func TestProcessCollectionFlags(t *testing.T) { + dir := t.TempDir() + for name, contents := range map[string]string{ + "config.yml": "version: 3\npipeline:\n stages: []\nreporting:\n enabled: false\nglossary_collections:\n party: {entries: []}\n", + "transcript.json": `[{"speaker":"Alice","start":0,"end":1,"text":"hello"}]`, + } { + if err := os.WriteFile(filepath.Join(dir, name), []byte(contents), 0644); err != nil { + t.Fatal(err) + } + } + app, err := buildApplication() + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + value string + status int + }{{`["party"]`, 0}, {`[]`, 0}, {`null`, 0}, {`[1]`, 2}, {`"party"`, 2}} { + var out, stderr bytes.Buffer + args := []string{"process", "--config", filepath.Join(dir, "config.yml"), "--glossaries", tc.value, filepath.Join(dir, "transcript.json")} + outcome := app.Execute(context.Background(), args, commandkit.Streams{Out: &out, Err: &stderr}) + if outcome.Status != tc.status { + t.Fatalf("value=%q status=%d out=%q err=%q", tc.value, outcome.Status, out.String(), stderr.String()) + } + } +} diff --git a/internal/cli/process_inputs_test.go b/internal/cli/process_inputs_test.go index 5a6d789..94e716a 100644 --- a/internal/cli/process_inputs_test.go +++ b/internal/cli/process_inputs_test.go @@ -76,3 +76,33 @@ func TestPrepareProcessInitializesReportsAndToleratesDebugLoss(t *testing.T) { t.Fatalf("output published early: %v", err) } } + +func TestCollectionFlagJSON(t *testing.T) { + for _, tc := range []struct { + raw string + valid bool + null bool + size int + }{ + {`["party","spells"]`, true, false, 2}, {`[]`, true, false, 0}, {`null`, true, true, 0}, + {`"party"`, false, false, 0}, {`[1]`, false, false, 0}, {`["x"] true`, false, false, 0}, + } { + result, err := parseCollectionNames(tc.raw) + if (err == nil) != tc.valid { + t.Fatalf("%s error=%v", tc.raw, err) + } + if !tc.valid { + continue + } + if tc.null { + if result != nil { + t.Fatalf("null=%v", result) + } + continue + } + list, ok := result.([]any) + if !ok || len(list) != tc.size { + t.Fatalf("%s result=%v", tc.raw, result) + } + } +} diff --git a/internal/core/config/config.go b/internal/core/config/config.go index 1c08dea..397b310 100644 --- a/internal/core/config/config.go +++ b/internal/core/config/config.go @@ -11,7 +11,6 @@ import ( ) const ( - DefaultModulesCSV = "glossary,homophones,glossary,spoken_word,grammar" DefaultTranscriptDescriptionMaxChars = 500 DefaultValidationMaxPromptTokens = 2048 ) @@ -142,6 +141,7 @@ func Load(input ck.Input, overlay string, overrides ...ck.Override) (*ck.Result[ } return nil }}, + Resolve: resolveSettings, Validate: validate, }, }) @@ -168,8 +168,33 @@ func validate(c *Config, v *ck.Validation) { check(c.LLM.TransientRetries >= 0, ck.Keys("llm", "transient_retries"), "retry budget must be non-negative") check(c.LLM.RepairAttempts >= 0 && c.LLM.RepairAttempts <= 3, ck.Keys("llm", "repair_attempts"), "repair attempts must be between zero and three") check(c.LLM.OperationTimeout > 0, ck.Keys("llm", "operation_timeout"), "operation timeout must be positive") - for i, stage := range c.Pipeline.Stages { - check(modulecatalog.IsSupported(stage.Module), ck.Keys("pipeline", "stages").Index(i).Key("module"), "unsupported module key") + check(c.LLM.ValidationProfile == nil || *c.LLM.ValidationProfile != "", ck.Keys("llm", "validation_profile"), "validation profile must not be empty; use null to clear it") + validateSelections := func(values []string, path ck.Path) { + for i, name := range values { + _, exists := c.GlossaryCollections[name] + check(exists, path.Index(i), "unknown glossary collection") + } + } + validateSelections(c.Glossaries, ck.Keys("glossaries")) + validateSelections(c.ProtectedTerms, ck.Keys("protected_terms")) + ids := make(map[string]bool) + for i, item := range c.Pipeline.Stages { + path := ck.Keys("pipeline", "stages").Index(i) + check(modulecatalog.IsSupported(item.Module), path.Key("module"), "unsupported module key") + if item.ID != nil { + check(*item.ID != "", path.Key("id"), "id must not be empty") + check(!ids[*item.ID], path.Key("id"), "id must be unique") + ids[*item.ID] = true + } + validateSelections(item.Glossaries, path.Key("glossaries")) + validateSelections(item.ProtectedTerms, path.Key("protected_terms")) + check(item.ConfidenceThreshold >= 0 && item.ConfidenceThreshold <= 1, path.Key("confidence_threshold"), "confidence threshold must be between zero and one") + check(item.LLM.ValidationProfile == nil || *item.LLM.ValidationProfile != "", path.Key("llm").Key("validation_profile"), "validation profile must not be empty; use null to clear it") + check(item.LLM.TransientRetries >= 0, path.Key("llm").Key("transient_retries"), "retry budget must be non-negative") + check(item.LLM.RepairAttempts >= 0 && item.LLM.RepairAttempts <= 3, path.Key("llm").Key("repair_attempts"), "repair attempts must be between zero and three") + check(item.LLM.OperationTimeout > 0, path.Key("llm").Key("operation_timeout"), "operation timeout must be positive") + check(item.Validation.MaxPromptTokens > 0, path.Key("validation").Key("max_prompt_tokens"), "validation prompt budget must be positive") + check(len(strings.TrimSpace(item.Context.Description)) <= DefaultTranscriptDescriptionMaxChars, path.Key("context").Key("description"), fmt.Sprintf("description must be at most %d bytes after trimming", DefaultTranscriptDescriptionMaxChars)) } check(outputschema.IsSupported(c.Output.Schema), ck.Keys("output", "schema"), "unsupported output schema") check(c.Validation.MaxPromptTokens > 0, ck.Keys("validation", "max_prompt_tokens"), "validation prompt budget must be positive") @@ -185,18 +210,3 @@ func validate(c *Config, v *ck.Validation) { } check(len(strings.TrimSpace(c.Context.Description)) <= DefaultTranscriptDescriptionMaxChars, ck.Keys("context", "description"), fmt.Sprintf("description must be at most %d bytes after trimming", DefaultTranscriptDescriptionMaxChars)) } - -// ParseModulesCSV is used only for an explicitly supplied --modules override. -func ParseModulesCSV(raw string) ([]string, error) { - if raw == "" { - return []string{}, nil - } - parts := strings.Split(raw, ",") - for i := range parts { - parts[i] = strings.TrimSpace(parts[i]) - if parts[i] == "" { - return nil, fmt.Errorf("modules list contains an empty value") - } - } - return parts, nil -} diff --git a/internal/core/config/resolve.go b/internal/core/config/resolve.go new file mode 100644 index 0000000..6aae9cf --- /dev/null +++ b/internal/core/config/resolve.go @@ -0,0 +1,73 @@ +package config + +import ck "go.maximumdirect.net/configkit" + +func cloneSelection(value *string) *string { + if value == nil { + return nil + } + copied := *value + return &copied +} + +func cloneNames(value []string) []string { + if value == nil { + return nil + } + return append([]string{}, value...) +} + +// resolveSettings records root-derived defaults after all explicit assignments. +func resolveSettings(c *Config, r *ck.Resolution) error { + glossaries := ck.Keys("glossaries") + protection := ck.Keys("protected_terms") + ck.Default(r, protection, &c.ProtectedTerms, cloneNames(c.Glossaries), "protection defaults to selected glossaries", glossaries) + rootValidation := ck.Keys("llm", "validation_profile") + rootValidationInfo, _ := r.EntryInfo(rootValidation) + inheritedValidation := r.IsExplicit(rootValidation) && !rootValidationInfo.Null && c.LLM.ValidationProfile != nil + for i := range c.Pipeline.Stages { + item := &c.Pipeline.Stages[i] + base := ck.Keys("pipeline", "stages").Index(i) + ck.Default(r, base.Key("glossaries"), &item.Glossaries, cloneNames(c.Glossaries), "inherit root glossaries", glossaries) + ck.Default(r, base.Key("protected_terms"), &item.ProtectedTerms, cloneNames(c.ProtectedTerms), "inherit root protection", protection) + threshold := ck.Keys("thresholds", item.Module) + var value float64 + switch item.Module { + case "glossary": + value = c.Thresholds.Glossary + case "homophones": + value = c.Thresholds.Homophones + case "spoken_word": + value = c.Thresholds.SpokenWord + case "grammar": + value = c.Thresholds.Grammar + } + ck.Default(r, base.Key("confidence_threshold"), &item.ConfidenceThreshold, value, "inherit module threshold", threshold) + llm := base.Key("llm") + rootLLM := ck.Keys("llm") + proposal := llm.Key("proposal_profile") + ck.Default(r, proposal, &item.LLM.ProposalProfile, cloneSelection(c.LLM.ProposalProfile), "inherit proposal profile", rootLLM.Key("proposal_profile")) + ck.Default(r, llm.Key("transient_retries"), &item.LLM.TransientRetries, c.LLM.TransientRetries, "inherit retry budget", rootLLM.Key("transient_retries")) + ck.Default(r, llm.Key("repair_attempts"), &item.LLM.RepairAttempts, c.LLM.RepairAttempts, "inherit repair budget", rootLLM.Key("repair_attempts")) + ck.Default(r, llm.Key("operation_timeout"), &item.LLM.OperationTimeout, c.LLM.OperationTimeout, "inherit operation timeout", rootLLM.Key("operation_timeout")) + ck.Default(r, base.Key("validation").Key("max_prompt_tokens"), &item.Validation.MaxPromptTokens, c.Validation.MaxPromptTokens, "inherit validation budget", ck.Keys("validation", "max_prompt_tokens")) + ck.Default(r, base.Key("context").Key("description"), &item.Context.Description, c.Context.Description, "inherit description", ck.Keys("context", "description")) + validation := llm.Key("validation_profile") + info, _ := r.EntryInfo(validation) + if r.IsExplicit(validation) && info.Null { + item.LLM.ValidationProfile = cloneSelection(item.LLM.ProposalProfile) + r.RecordDerivedDefault(validation, "validation defaults to proposal profile", proposal) + } else if inheritedValidation { + ck.Default(r, validation, &item.LLM.ValidationProfile, cloneSelection(c.LLM.ValidationProfile), "inherit validation profile", rootValidation) + } else { + ck.Default(r, validation, &item.LLM.ValidationProfile, cloneSelection(item.LLM.ProposalProfile), "validation defaults to proposal profile", proposal) + } + } + if r.IsExplicit(rootValidation) && rootValidationInfo.Null { + c.LLM.ValidationProfile = cloneSelection(c.LLM.ProposalProfile) + r.RecordDerivedDefault(rootValidation, "validation defaults to proposal profile", ck.Keys("llm", "proposal_profile")) + } else { + ck.Default(r, rootValidation, &c.LLM.ValidationProfile, cloneSelection(c.LLM.ProposalProfile), "validation defaults to proposal profile", ck.Keys("llm", "proposal_profile")) + } + return nil +} diff --git a/internal/core/config/resolve_test.go b/internal/core/config/resolve_test.go new file mode 100644 index 0000000..643f116 --- /dev/null +++ b/internal/core/config/resolve_test.go @@ -0,0 +1,160 @@ +package config + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + ck "go.maximumdirect.net/configkit" +) + +func loadResolution(t *testing.T, text string, overrides ...ck.Override) (*ck.Result[Config], error) { + t.Helper() + file := filepath.Join(t.TempDir(), "config.yml") + if err := os.WriteFile(file, []byte(text), 0644); err != nil { + t.Fatal(err) + } + return Load(ck.Input{Source: ck.File(file)}, "", overrides...) +} + +func TestResolutionCopiesRootOverridesAndHonorsClears(t *testing.T) { + document := `glossary_collections: + party: {entries: []} + spells: {entries: []} +glossaries: [party] +llm: + proposal_profile: A + repair_attempts: 2 +context: + description: root +pipeline: + stages: + - module: glossary + glossaries: [party] + llm: + proposal_profile: B + repair_attempts: 0 + context: + description: "" + - module: grammar + protected_terms: null + - module: grammar + glossaries: [] +` + result, err := loadResolution(t, document, ck.Override{Path: ck.Keys("glossaries"), Value: []any{"spells"}, Flag: "--glossaries"}) + if err != nil { + t.Fatal(err) + } + cfg := result.Config + if !reflect.DeepEqual(cfg.ProtectedTerms, []string{"spells"}) || !reflect.DeepEqual(cfg.Pipeline.Stages[0].Glossaries, []string{"party"}) || !reflect.DeepEqual(cfg.Pipeline.Stages[1].Glossaries, []string{"spells"}) || len(cfg.Pipeline.Stages[2].Glossaries) != 0 { + t.Fatalf("resolved: %+v", cfg) + } + if cfg.Pipeline.Stages[1].ProtectedTerms != nil || cfg.Pipeline.Stages[0].LLM.RepairAttempts != 0 || cfg.Pipeline.Stages[0].Context.Description != "" { + t.Fatalf("clears lost: %+v", cfg.Pipeline.Stages) + } + if *cfg.Pipeline.Stages[0].LLM.ValidationProfile != "B" || *cfg.Pipeline.Stages[1].LLM.ValidationProfile != "A" { + t.Fatalf("fallback: %+v", cfg.Pipeline.Stages) + } + cfg.Pipeline.Stages[0].ProtectedTerms[0] = "changed" + if cfg.ProtectedTerms[0] != "spells" || cfg.Pipeline.Stages[2].ProtectedTerms[0] != "spells" { + t.Fatal("selections share mutable storage") + } + info, ok := result.Explain(ck.Keys("pipeline", "stages").Index(0).Key("protected_terms")) + if !ok || len(info.History) == 0 { + t.Fatal("missing resolution evidence") + } +} + +func TestValidationProfileFallback(t *testing.T) { + for _, tc := range []struct{ name, root, local, want string }{ + {"proposal override", "", "proposal_profile: B", "B"}, + {"inherited selection", "validation_profile: V", "proposal_profile: B", "V"}, + {"explicit null", "validation_profile: V", "proposal_profile: B\n validation_profile: null", "B"}, + {"root null", "validation_profile: null", "proposal_profile: B", "B"}, + {"stage selection", "", "validation_profile: W", "W"}, + } { + t.Run(tc.name, func(t *testing.T) { + doc := "llm:\n proposal_profile: A\n" + if tc.root != "" { + doc += " " + tc.root + "\n" + } + doc += "pipeline:\n stages:\n - module: grammar\n llm:\n " + tc.local + "\n" + result, err := loadResolution(t, doc) + if err != nil { + t.Fatal(err) + } + value := result.Config.Pipeline.Stages[0].LLM.ValidationProfile + if value == nil || *value != tc.want { + t.Fatalf("profile=%v want=%s", value, tc.want) + } + }) + } +} + +func TestResolvedSettingsValidation(t *testing.T) { + for _, tc := range []struct{ name, text string }{ + {"unknown root collection", "glossaries: [missing]\n"}, + {"unknown local collection", "pipeline:\n stages:\n - module: grammar\n protected_terms: [missing]\n"}, + {"empty id", "pipeline:\n stages:\n - {module: grammar, id: ''}\n"}, + {"duplicate id", "pipeline:\n stages:\n - {module: grammar, id: same}\n - {module: grammar, id: same}\n"}, + {"empty root validation profile", "llm:\n validation_profile: ''\n"}, + {"empty local validation profile", "pipeline:\n stages:\n - module: grammar\n llm:\n validation_profile: ''\n"}, + {"invalid local threshold", "pipeline:\n stages:\n - {module: grammar, confidence_threshold: 1.1}\n"}, + {"invalid local timeout", "pipeline:\n stages:\n - module: grammar\n llm: {operation_timeout: 0s}\n"}, + {"invalid local batch budget", "pipeline:\n stages:\n - module: grammar\n validation: {max_prompt_tokens: 0}\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := loadResolution(t, tc.text); err == nil { + t.Fatal("invalid settings accepted") + } + }) + } + if _, err := loadResolution(t, "pipeline:\n stages:\n - {module: grammar, id: 'a/b not slug'}\n"); err != nil { + t.Fatal(err) + } +} + +func TestResolutionRecordsFieldDependenciesAndExplicitNull(t *testing.T) { + doc := `glossary_collections: + party: {entries: []} +glossaries: [party] +llm: + proposal_profile: A +pipeline: + stages: + - module: grammar + llm: + proposal_profile: B + validation_profile: null +` + result, err := loadResolution(t, doc) + if err != nil { + t.Fatal(err) + } + base := ck.Keys("pipeline", "stages").Index(0) + for _, tc := range []struct{ path, dependency ck.Path }{ + {ck.Keys("protected_terms"), ck.Keys("glossaries")}, + {base.Key("glossaries"), ck.Keys("glossaries")}, + {base.Key("llm").Key("validation_profile"), base.Key("llm").Key("proposal_profile")}, + } { + found := false + for _, change := range result.Metadata().History(tc.path) { + if change.Origin.Operation != ck.ChangeDerivedDefault { + continue + } + for _, dependency := range change.Dependencies { + if dependency.String() == tc.dependency.String() { + found = true + } + } + } + if !found { + t.Errorf("missing %s dependency on %s", tc.path.String(), tc.dependency.String()) + } + } + info, ok := result.Metadata().Lookup(base.Key("llm").Key("validation_profile")) + if !ok || !info.Explicit { + t.Fatalf("explicit null lost: %+v", info) + } +}