From 9a92212632ef9532f2862f1f3e70bb67d7f92020 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 25 Aug 2026 19:47:42 +0000 Subject: [PATCH] Add structured output repair configuration --- docs/roadmap/implementation.md | 2 +- internal/core/config/config.go | 8 + internal/core/config/file_config.go | 109 +++++++++--- .../core/config/file_config_contract_test.go | 162 ++++++++++++++++++ internal/core/config/validation.go | 16 ++ internal/framework/pipeline/profile.go | 77 +++++++-- 6 files changed, 326 insertions(+), 48 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 8f403f38..e1f4084e 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -545,7 +545,7 @@ git diff --check - Existing request-contract tests remain concise and pass. - A final call-site audit finds no silent production omission. -## Stage 8: Add The Public Repair Configuration Contract +## Stage 8: Add The Public Repair Configuration Contract ✅ ### Goal diff --git a/internal/core/config/config.go b/internal/core/config/config.go index dd949500..bfd33dfc 100644 --- a/internal/core/config/config.go +++ b/internal/core/config/config.go @@ -116,6 +116,10 @@ func (c *ConcurrencyConfig) recomputeStageWorkerDefaults() { func clonePipelineProfile(in pipeline.PipelineProfile) pipeline.PipelineProfile { out := in + if in.StructuredOutputRepairAttempts != nil { + value := *in.StructuredOutputRepairAttempts + out.StructuredOutputRepairAttempts = &value + } out.Input = cloneModuleBinding(in.Input) out.Chunk = cloneModuleBinding(in.Chunk) out.Output = cloneModuleBinding(in.Output) @@ -196,6 +200,10 @@ func cloneReferenceSource(in pipeline.ReferenceSource) pipeline.ReferenceSource func cloneModuleBinding(in pipeline.ModuleBinding) pipeline.ModuleBinding { out := in + if in.StructuredOutputRepairAttempts != nil { + value := *in.StructuredOutputRepairAttempts + out.StructuredOutputRepairAttempts = &value + } if len(in.Options) > 0 { out.Options = cloneOptions(in.Options) } diff --git a/internal/core/config/file_config.go b/internal/core/config/file_config.go index 909b6cc8..084ab6f3 100644 --- a/internal/core/config/file_config.go +++ b/internal/core/config/file_config.go @@ -35,23 +35,27 @@ type FilePromptKitLocalBackendConfig struct { } type FilePipelineProfile struct { - 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:"-"` + LLMProfile *string `yaml:"llm_profile,omitempty"` + StructuredOutputRepairAttempts *int `yaml:"structured_output_repair_attempts,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 { + if err := validateStructuredOutputRepairAttemptsNode(node, "pipeline profile"); err != nil { + return err + } type plainFilePipelineProfile FilePipelineProfile var decoded plainFilePipelineProfile seen, err := decodeKnownMapping(node, &decoded, map[string]struct{}{ - "llm_profile": {}, "input": {}, "chunk": {}, "artifacts": {}, "steps": {}, "output": {}, "references": {}, + "llm_profile": {}, "structured_output_repair_attempts": {}, "input": {}, "chunk": {}, "artifacts": {}, "steps": {}, "output": {}, "references": {}, }, "pipeline profile") if err != nil { return err @@ -147,12 +151,13 @@ type FileDebugConfig struct { } type fileModuleBinding struct { - Module string - LLMProfile string - Retries int - Options map[string]any - References map[string]fileReferenceSource - Validators pipeline.ValidatorOverride + Module string + LLMProfile string + StructuredOutputRepairAttempts *int + Retries int + Options map[string]any + References map[string]fileReferenceSource + Validators pipeline.ValidatorOverride } type fileReferenceSource struct { @@ -264,6 +269,12 @@ func (b *fileModuleBinding) UnmarshalYAML(node *yaml.Node) error { if b.LLMProfile == "" { return fmt.Errorf("llm_profile must not be empty when set") } + case "structured_output_repair_attempts": + attempts, err := parseStructuredOutputRepairAttempts(valueNode, "module binding") + if err != nil { + return err + } + b.StructuredOutputRepairAttempts = attempts case "retries": var retries int if err := valueNode.Decode(&retries); err != nil { @@ -304,15 +315,56 @@ func (b *fileModuleBinding) UnmarshalYAML(node *yaml.Node) error { func (b fileModuleBinding) toPipelineBinding() pipeline.ModuleBinding { return pipeline.ModuleBinding{ - Module: strings.TrimSpace(b.Module), - LLMProfile: strings.TrimSpace(b.LLMProfile), - Retries: b.Retries, - Options: cloneOptions(b.Options), - References: fileReferenceSourcesToPipeline(b.References), - Validators: b.Validators, + Module: strings.TrimSpace(b.Module), + LLMProfile: strings.TrimSpace(b.LLMProfile), + StructuredOutputRepairAttempts: cloneStructuredOutputRepairAttempts(b.StructuredOutputRepairAttempts), + Retries: b.Retries, + Options: cloneOptions(b.Options), + References: fileReferenceSourcesToPipeline(b.References), + Validators: cloneValidatorOverride(b.Validators), } } +func validateStructuredOutputRepairAttemptsNode(node *yaml.Node, context string) error { + if node.Kind != yaml.MappingNode { + return fmt.Errorf("%s must be an object", context) + } + for i := 0; i < len(node.Content); i += 2 { + if node.Content[i].Value != "structured_output_repair_attempts" { + continue + } + if _, err := parseStructuredOutputRepairAttempts(node.Content[i+1], context); err != nil { + return err + } + } + return nil +} + +func parseStructuredOutputRepairAttempts(node *yaml.Node, context string) (*int, error) { + if node.Tag == "!!null" { + return nil, fmt.Errorf("%s structured_output_repair_attempts must not be null", context) + } + if node.Kind != yaml.ScalarNode || node.Tag != "!!int" { + return nil, fmt.Errorf("%s structured_output_repair_attempts must be an integer", context) + } + var attempts int + if err := node.Decode(&attempts); err != nil { + return nil, fmt.Errorf("%s structured_output_repair_attempts must be an integer: %w", context, err) + } + if attempts < 0 || attempts > 3 { + return nil, fmt.Errorf("%s structured_output_repair_attempts must be between zero and three", context) + } + return &attempts, nil +} + +func cloneStructuredOutputRepairAttempts(attempts *int) *int { + if attempts == nil { + return nil + } + value := *attempts + return &value +} + func LoadFileConfig(path string) (FileConfig, error) { data, err := os.ReadFile(path) if err != nil { @@ -518,11 +570,12 @@ func (c *Config) applyFileConfigWithLookup(fileCfg FileConfig, lookup func(strin return err } profile := pipeline.PipelineProfile{ - ID: pipelineID, - LLMProfile: llmProfile, - Input: filePipeline.Input.toPipelineBinding(), - Artifacts: make(map[string]pipeline.ArtifactLaneProfile, len(filePipeline.Artifacts)), - References: fileReferenceSourcesToPipeline(filePipeline.References), + ID: pipelineID, + LLMProfile: llmProfile, + StructuredOutputRepairAttempts: cloneStructuredOutputRepairAttempts(filePipeline.StructuredOutputRepairAttempts), + Input: filePipeline.Input.toPipelineBinding(), + Artifacts: make(map[string]pipeline.ArtifactLaneProfile, len(filePipeline.Artifacts)), + References: fileReferenceSourcesToPipeline(filePipeline.References), } if filePipeline.Chunk != nil { profile.Chunk = filePipeline.Chunk.toPipelineBinding() diff --git a/internal/core/config/file_config_contract_test.go b/internal/core/config/file_config_contract_test.go index 0f01c5eb..9092ebd3 100644 --- a/internal/core/config/file_config_contract_test.go +++ b/internal/core/config/file_config_contract_test.go @@ -138,6 +138,168 @@ pipelines: } } +func TestStructuredOutputRepairAttemptsFileConfigurationPreservesPresenceAndOwnership(t *testing.T) { + const pipelineYAML = `version: 4 +pipelines: + main: + %s + input: input + artifacts: + lane: + extract: + module: extract + structured_output_repair_attempts: 2 + validators: + - module: validator + structured_output_repair_attempts: 3 +` + + t.Run("omitted pipeline value remains absent", func(t *testing.T) { + file := parseFileConfig(t, fmt.Sprintf(pipelineYAML, "")) + if got := file.Pipelines["main"].StructuredOutputRepairAttempts; got != nil { + t.Fatalf("file pipeline repair attempts = %v, want nil", *got) + } + cfg := Default() + if err := cfg.ApplyFileConfig(file); err != nil { + t.Fatal(err) + } + if got := cfg.Pipelines["main"].StructuredOutputRepairAttempts; got != nil { + t.Fatalf("pipeline repair attempts = %v, want nil", *got) + } + if got := cfg.Pipelines["main"].Input.StructuredOutputRepairAttempts; got != nil { + t.Fatalf("scalar input binding repair attempts = %v, want nil", *got) + } + }) + + t.Run("zero is explicit and survives configuration boundaries", func(t *testing.T) { + file := parseFileConfig(t, fmt.Sprintf(pipelineYAML, "structured_output_repair_attempts: 0")) + cfg := Default() + if err := cfg.ApplyFileConfig(file); err != nil { + t.Fatal(err) + } + profile := cfg.Pipelines["main"] + if profile.StructuredOutputRepairAttempts == nil || *profile.StructuredOutputRepairAttempts != 0 { + t.Fatalf("pipeline repair attempts = %v, want explicit zero", profile.StructuredOutputRepairAttempts) + } + lane := profile.Artifacts["lane"] + if lane.Extract.StructuredOutputRepairAttempts == nil || *lane.Extract.StructuredOutputRepairAttempts != 2 { + t.Fatalf("extract repair attempts = %v, want 2", lane.Extract.StructuredOutputRepairAttempts) + } + if lane.Extract.Validators.Validators[0].StructuredOutputRepairAttempts == nil || *lane.Extract.Validators.Validators[0].StructuredOutputRepairAttempts != 3 { + t.Fatalf("validator repair attempts = %v, want 3", lane.Extract.Validators.Validators[0].StructuredOutputRepairAttempts) + } + + *file.Pipelines["main"].StructuredOutputRepairAttempts = 1 + if got := *cfg.Pipelines["main"].StructuredOutputRepairAttempts; got != 0 { + t.Fatalf("applied config aliased file configuration: got %d, want 0", got) + } + fileProfile := file.Pipelines["main"] + fileLane := fileProfile.Artifacts["lane"] + *fileLane.Extract.StructuredOutputRepairAttempts = 1 + *fileLane.Extract.Validators.Validators[0].StructuredOutputRepairAttempts = 1 + fileProfile.Artifacts["lane"] = fileLane + file.Pipelines["main"] = fileProfile + configuredLane := cfg.Pipelines["main"].Artifacts["lane"] + if got := *configuredLane.Extract.StructuredOutputRepairAttempts; got != 2 { + t.Fatalf("configured extract aliased file configuration: got %d, want 2", got) + } + if got := *configuredLane.Extract.Validators.Validators[0].StructuredOutputRepairAttempts; got != 3 { + t.Fatalf("configured validator aliased file configuration: got %d, want 3", got) + } + cloned := cloneConfig(cfg) + *cloned.Pipelines["main"].StructuredOutputRepairAttempts = 1 + if got := *cfg.Pipelines["main"].StructuredOutputRepairAttempts; got != 0 { + t.Fatalf("cloned config aliased source configuration: got %d, want 0", got) + } + redacted := cfg.Redacted() + *redacted.Pipelines["main"].StructuredOutputRepairAttempts = 1 + if got := *cfg.Pipelines["main"].StructuredOutputRepairAttempts; got != 0 { + t.Fatalf("redacted config aliased source configuration: got %d, want 0", got) + } + summary := cfg.RedactedSummaryPayload().(Config) + if summary.Pipelines["main"].StructuredOutputRepairAttempts == nil || *summary.Pipelines["main"].StructuredOutputRepairAttempts != 0 { + t.Fatalf("redacted summary pipeline repair attempts = %v, want explicit zero", summary.Pipelines["main"].StructuredOutputRepairAttempts) + } + + 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) + } + roundTrippedProfile := roundTripped.Pipelines["main"] + if roundTrippedProfile.StructuredOutputRepairAttempts == nil || *roundTrippedProfile.StructuredOutputRepairAttempts != 0 { + t.Fatalf("round-tripped pipeline repair attempts = %v, want explicit zero", roundTrippedProfile.StructuredOutputRepairAttempts) + } + roundTrippedLane := roundTrippedProfile.Artifacts["lane"] + if roundTrippedLane.Extract.StructuredOutputRepairAttempts == nil || *roundTrippedLane.Extract.StructuredOutputRepairAttempts != 2 { + t.Fatalf("round-tripped extract repair attempts = %v, want 2", roundTrippedLane.Extract.StructuredOutputRepairAttempts) + } + if roundTrippedLane.Extract.Validators.Validators[0].StructuredOutputRepairAttempts == nil || *roundTrippedLane.Extract.Validators.Validators[0].StructuredOutputRepairAttempts != 3 { + t.Fatalf("round-tripped validator repair attempts = %v, want 3", roundTrippedLane.Extract.Validators.Validators[0].StructuredOutputRepairAttempts) + } + }) +} + +func TestStructuredOutputRepairAttemptsFileConfigurationRejectsInvalidValues(t *testing.T) { + const pipelineYAML = `version: 4 +pipelines: + main: + input: input + artifacts: + lane: + extract: extract + %s +` + const bindingYAML = `version: 4 +pipelines: + main: + input: + module: input + %s + artifacts: + lane: + extract: extract +` + + for _, tt := range []struct { + name string + source string + want string + }{ + {name: "null pipeline value", source: "structured_output_repair_attempts: null", want: "pipeline profile structured_output_repair_attempts must not be null"}, + {name: "fractional pipeline value", source: "structured_output_repair_attempts: 1.5", want: "pipeline profile structured_output_repair_attempts must be an integer"}, + {name: "quoted pipeline value", source: "structured_output_repair_attempts: '1'", want: "pipeline profile structured_output_repair_attempts must be an integer"}, + {name: "out of range pipeline value", source: "structured_output_repair_attempts: 4", want: "pipeline profile structured_output_repair_attempts must be between zero and three"}, + } { + t.Run(tt.name, func(t *testing.T) { + _, err := ParseFileConfigYAML([]byte(fmt.Sprintf(pipelineYAML, tt.source))) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Fatalf("ParseFileConfigYAML() error = %v, want %q", err, tt.want) + } + }) + } + + for _, tt := range []struct { + name string + source string + want string + }{ + {name: "null binding value", source: "structured_output_repair_attempts: null", want: "module binding structured_output_repair_attempts must not be null"}, + {name: "noninteger binding value", source: "structured_output_repair_attempts: true", want: "module binding structured_output_repair_attempts must be an integer"}, + {name: "out of range binding value", source: "structured_output_repair_attempts: -1", want: "module binding structured_output_repair_attempts must be between zero and three"}, + } { + t.Run(tt.name, func(t *testing.T) { + _, err := ParseFileConfigYAML([]byte(fmt.Sprintf(bindingYAML, tt.source))) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Fatalf("ParseFileConfigYAML() error = %v, want %q", err, tt.want) + } + }) + } +} + func TestFileModuleBindingRejectsExplicitEmptyLLMProfile(t *testing.T) { const configYAML = `version: 4 pipelines: diff --git a/internal/core/config/validation.go b/internal/core/config/validation.go index a015d719..2d879c59 100644 --- a/internal/core/config/validation.go +++ b/internal/core/config/validation.go @@ -116,6 +116,9 @@ func validatePipelineProfiles(profiles map[string]pipeline.PipelineProfile) erro if profile.ID != "" && strings.TrimSpace(profile.ID) != id { return fmt.Errorf("pipeline %q profile id %q does not match map key", id, profile.ID) } + if err := validateStructuredOutputRepairAttempts(fmt.Sprintf("pipeline %q", id), profile.StructuredOutputRepairAttempts); err != nil { + return err + } if err := validateBinding(id, "", "input", profile.Input, false); err != nil { return err } @@ -195,6 +198,9 @@ func validateBinding( binding pipeline.ModuleBinding, referencesAllowed bool, ) error { + if err := validateStructuredOutputRepairAttempts(referenceContext(pipelineID, laneID, slot), binding.StructuredOutputRepairAttempts); err != nil { + return err + } if err := validateBindingLLMProfile(pipelineID, laneID, slot, binding); err != nil { return err } @@ -219,6 +225,13 @@ func validateBinding( return validateReferenceMapForContext(pipelineID, laneID, slot, binding.References, true) } +func validateStructuredOutputRepairAttempts(context string, attempts *int) error { + if attempts != nil && (*attempts < 0 || *attempts > 3) { + return fmt.Errorf("%s structured_output_repair_attempts must be between zero and three", context) + } + return nil +} + func validateValidatorOverride(pipelineID string, laneID string, slot string, override pipeline.ValidatorOverride) error { if !override.Set { return nil @@ -230,6 +243,9 @@ func validateValidatorOverride(pipelineID string, laneID string, slot string, ov } for i, validator := range override.Validators { context := fmt.Sprintf("%s validators[%d]", referenceContext(pipelineID, laneID, slot), i) + if err := validateStructuredOutputRepairAttempts(context, validator.StructuredOutputRepairAttempts); err != nil { + return err + } if strings.TrimSpace(validator.Module) == "" { return fmt.Errorf("%s module must not be empty", context) } diff --git a/internal/framework/pipeline/profile.go b/internal/framework/pipeline/profile.go index b20c8fcd..a24324c5 100644 --- a/internal/framework/pipeline/profile.go +++ b/internal/framework/pipeline/profile.go @@ -81,19 +81,21 @@ type ValidatorOverride struct { func (binding ModuleBinding) MarshalJSON() ([]byte, error) { type moduleBindingJSON struct { - Module string `json:"module"` - LLMProfile string `json:"llm_profile,omitempty"` - Retries int `json:"retries,omitempty"` - Options map[string]any `json:"options,omitempty"` - References map[string]ReferenceSource `json:"references,omitempty"` - Validators *[]ModuleBinding `json:"validators,omitempty"` + Module string `json:"module"` + LLMProfile string `json:"llm_profile,omitempty"` + StructuredOutputRepairAttempts *int `json:"structured_output_repair_attempts,omitempty"` + Retries int `json:"retries,omitempty"` + Options map[string]any `json:"options,omitempty"` + References map[string]ReferenceSource `json:"references,omitempty"` + Validators *[]ModuleBinding `json:"validators,omitempty"` } out := moduleBindingJSON{ - Module: binding.Module, - LLMProfile: binding.LLMProfile, - Retries: binding.Retries, - Options: binding.Options, - References: binding.References, + Module: binding.Module, + LLMProfile: binding.LLMProfile, + StructuredOutputRepairAttempts: binding.StructuredOutputRepairAttempts, + Retries: binding.Retries, + Options: binding.Options, + References: binding.References, } if binding.Validators.Set { validators := cloneModuleBindings(binding.Validators.Validators) @@ -102,6 +104,42 @@ func (binding ModuleBinding) MarshalJSON() ([]byte, error) { return json.Marshal(out) } +func (binding *ModuleBinding) UnmarshalJSON(data []byte) error { + if binding == nil { + return fmt.Errorf("module binding must not be nil") + } + type moduleBindingJSON struct { + Module string `json:"module"` + LLMProfile string `json:"llm_profile,omitempty"` + StructuredOutputRepairAttempts *int `json:"structured_output_repair_attempts,omitempty"` + Retries int `json:"retries,omitempty"` + Options map[string]any `json:"options,omitempty"` + References map[string]ReferenceSource `json:"references,omitempty"` + Validators json.RawMessage `json:"validators,omitempty"` + } + var decoded moduleBindingJSON + if err := json.Unmarshal(data, &decoded); err != nil { + return err + } + out := ModuleBinding{ + Module: decoded.Module, + LLMProfile: decoded.LLMProfile, + StructuredOutputRepairAttempts: decoded.StructuredOutputRepairAttempts, + Retries: decoded.Retries, + Options: decoded.Options, + References: decoded.References, + } + if len(decoded.Validators) > 0 && string(decoded.Validators) != "null" { + var validators []ModuleBinding + if err := json.Unmarshal(decoded.Validators, &validators); err != nil { + return fmt.Errorf("decode module binding validators: %w", err) + } + out.Validators = ValidatorOverride{Set: true, Validators: validators} + } + *binding = cloneModuleBinding(out) + return nil +} + type ArtifactLaneProfile struct { Extract ModuleBinding `json:"extract"` Merge ModuleBinding `json:"merge,omitempty"` @@ -117,14 +155,15 @@ type PipelineStepProfile struct { } type PipelineProfile struct { - ID string `json:"id"` - LLMProfile string `json:"llm_profile,omitempty"` - Input ModuleBinding `json:"input"` - Chunk ModuleBinding `json:"chunk,omitempty"` - Artifacts map[string]ArtifactLaneProfile `json:"artifacts"` - Steps []PipelineStepProfile `json:"steps,omitempty"` - Output ModuleBinding `json:"output,omitempty"` - References map[string]ReferenceSource `json:"references,omitempty"` + ID string `json:"id"` + LLMProfile string `json:"llm_profile,omitempty"` + StructuredOutputRepairAttempts *int `json:"structured_output_repair_attempts,omitempty"` + Input ModuleBinding `json:"input"` + Chunk ModuleBinding `json:"chunk,omitempty"` + Artifacts map[string]ArtifactLaneProfile `json:"artifacts"` + Steps []PipelineStepProfile `json:"steps,omitempty"` + Output ModuleBinding `json:"output,omitempty"` + References map[string]ReferenceSource `json:"references,omitempty"` } type ResolveOptions struct {