From f9e8afa2c3db968824135e678afc2c1ed5530258 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 25 Aug 2026 09:41:15 +0000 Subject: [PATCH] Enforce bounded output repair contracts --- docs/roadmap/implementation.md | 2 + internal/domain/output_contract.go | 8 +++ internal/domain/output_contract_test.go | 23 ++++++- internal/promptdef/repository_test.go | 32 ++++++++++ internal/usecase/output_contract_test.go | 2 + internal/usecase/runner_test.go | 2 +- internal/validate/standard_validator.go | 5 +- internal/validate/standard_validator_test.go | 37 ++++++++++++ output_contract_contract_test.go | 63 ++++++++++++++------ 9 files changed, 149 insertions(+), 25 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index a701cc1..d575bca 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -178,6 +178,8 @@ Stage 1 is complete when every input path accepts only a coherent zero-to-three budget, invalid contracts fail before model work, and validators no longer misreport a configured budget as completed repair work. +**Status:** Complete. + ## Stage 2: Distinguish Explicit Empty Content From A Malformed Envelope ### Objective diff --git a/internal/domain/output_contract.go b/internal/domain/output_contract.go index a6919cf..76ef23c 100644 --- a/internal/domain/output_contract.go +++ b/internal/domain/output_contract.go @@ -6,6 +6,8 @@ import ( "strings" ) +const maxOutputRepairAttempts = 3 + // ValidateOutputContract validates source-neutral output-contract invariants. func ValidateOutputContract(contract OutputContract) error { switch contract.Format { @@ -26,5 +28,11 @@ func ValidateOutputContract(contract OutputContract) error { if contract.RepairAttempts < 0 { return errors.New("repair_attempts must be greater than or equal to 0") } + if contract.RepairAttempts > maxOutputRepairAttempts { + return fmt.Errorf("repair_attempts must be less than or equal to %d", maxOutputRepairAttempts) + } + if contract.ValidationMode == ValidationNone && contract.RepairAttempts > 0 { + return errors.New("repair_attempts requires basic, json, or json_schema validation") + } return nil } diff --git a/internal/domain/output_contract_test.go b/internal/domain/output_contract_test.go index b9f541a..70086b4 100644 --- a/internal/domain/output_contract_test.go +++ b/internal/domain/output_contract_test.go @@ -30,9 +30,28 @@ func TestValidateOutputContract(t *testing.T) { }}, {name: "empty validation mode", change: func(c *OutputContract) { c.ValidationMode = "" }, wantErr: "validation mode"}, {name: "unsupported validation mode", change: func(c *OutputContract) { c.ValidationMode = ValidationMode("unknown") }, wantErr: "validation mode"}, - {name: "negative repair attempts", change: func(c *OutputContract) { c.RepairAttempts = -1 }, wantErr: "repair_attempts"}, + {name: "negative repair attempts", change: func(c *OutputContract) { + c.ValidationMode = ValidationBasic + c.RepairAttempts = -1 + }, wantErr: "repair_attempts"}, {name: "zero repair attempts", change: func(c *OutputContract) { c.RepairAttempts = 0 }}, - {name: "positive repair attempts", change: func(c *OutputContract) { c.RepairAttempts = 1 }}, + {name: "one repair attempt", change: func(c *OutputContract) { + c.ValidationMode = ValidationBasic + c.RepairAttempts = 1 + }}, + {name: "maximum repair attempts", change: func(c *OutputContract) { + c.ValidationMode = ValidationJSON + c.RepairAttempts = 3 + }}, + {name: "too many repair attempts", change: func(c *OutputContract) { + c.ValidationMode = ValidationJSONSchema + c.SchemaPath = "schema.json" + c.RepairAttempts = 4 + }, wantErr: "repair_attempts"}, + {name: "none validation with repair attempts", change: func(c *OutputContract) { + c.ValidationMode = ValidationNone + c.RepairAttempts = 1 + }, wantErr: "repair_attempts"}, {name: "json schema empty path", change: func(c *OutputContract) { c.ValidationMode = ValidationJSONSchema c.SchemaPath = "" diff --git a/internal/promptdef/repository_test.go b/internal/promptdef/repository_test.go index 4b12132..2b2709a 100644 --- a/internal/promptdef/repository_test.go +++ b/internal/promptdef/repository_test.go @@ -892,6 +892,38 @@ output: format: text validation_mode: none repair_attempts: -1 +`, + wantErr: true, + wantDiagnostic: "repair_attempts", + }, + { + name: "repair attempts above maximum", + definition: ` +id: normalization-rule +version: "1" +messages: + - role: user + content: test +output: + format: text + validation_mode: basic + repair_attempts: 4 +`, + wantErr: true, + wantDiagnostic: "repair_attempts", + }, + { + name: "none validation with repair attempts", + definition: ` +id: normalization-rule +version: "1" +messages: + - role: user + content: test +output: + format: text + validation_mode: none + repair_attempts: 1 `, wantErr: true, wantDiagnostic: "repair_attempts", diff --git a/internal/usecase/output_contract_test.go b/internal/usecase/output_contract_test.go index 17cb080..a6e5873 100644 --- a/internal/usecase/output_contract_test.go +++ b/internal/usecase/output_contract_test.go @@ -106,6 +106,8 @@ func TestRunnerPreparationRejectsInvalidOutputContractsBeforeCompletion(t *testi {name: "unsupported format", override: domain.OutputContract{Format: "binary", ValidationMode: domain.ValidationNone}}, {name: "empty validation mode", override: domain.OutputContract{Format: domain.FormatText}}, {name: "negative repair attempts", override: domain.OutputContract{Format: domain.FormatText, ValidationMode: domain.ValidationNone, RepairAttempts: -1}}, + {name: "repair attempts above maximum", override: domain.OutputContract{Format: domain.FormatText, ValidationMode: domain.ValidationBasic, RepairAttempts: 4}}, + {name: "none validation with repair attempts", override: domain.OutputContract{Format: domain.FormatText, ValidationMode: domain.ValidationNone, RepairAttempts: 1}}, {name: "json schema without path", override: domain.OutputContract{Format: domain.FormatJSON, ValidationMode: domain.ValidationJSONSchema}}, } diff --git a/internal/usecase/runner_test.go b/internal/usecase/runner_test.go index 2315fe8..0861b76 100644 --- a/internal/usecase/runner_test.go +++ b/internal/usecase/runner_test.go @@ -2202,7 +2202,7 @@ func TestRunnerRepairStateMachine(t *testing.T) { { name: "successful repair stops below larger budget", mode: domain.ValidationJSON, - budget: 4, + budget: 3, validationResults: []domain.ValidationResult{ failed(domain.ValidationJSON, "candidate zero"), failed(domain.ValidationJSON, "candidate one"), diff --git a/internal/validate/standard_validator.go b/internal/validate/standard_validator.go index 1018e23..f56d15d 100644 --- a/internal/validate/standard_validator.go +++ b/internal/validate/standard_validator.go @@ -174,9 +174,8 @@ func validateArtifact(ctx context.Context, artifact *domain.Artifact, contract d } res := domain.ValidationResult{ - Mode: contract.ValidationMode, - SchemaPath: contract.SchemaPath, - RepairAttempts: contract.RepairAttempts, + Mode: contract.ValidationMode, + SchemaPath: contract.SchemaPath, } if artifact == nil { diff --git a/internal/validate/standard_validator_test.go b/internal/validate/standard_validator_test.go index 1247ce7..72209cb 100644 --- a/internal/validate/standard_validator_test.go +++ b/internal/validate/standard_validator_test.go @@ -64,6 +64,43 @@ func TestStandardValidatorBasicFailureEmpty(t *testing.T) { } } +func TestStandardValidatorReportsZeroRepairAttempts(t *testing.T) { + v := NewStandardValidator("") + + for _, tc := range []struct { + name string + body string + contract domain.OutputContract + }{ + { + name: "passed basic validation", + body: "answer", + contract: domain.OutputContract{ + ValidationMode: domain.ValidationBasic, + RepairAttempts: 3, + }, + }, + { + name: "failed JSON validation", + body: `{"answer":`, + contract: domain.OutputContract{ + ValidationMode: domain.ValidationJSON, + RepairAttempts: 3, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + res, err := v.Validate(context.Background(), &domain.Artifact{Body: []byte(tc.body)}, tc.contract) + if err != nil { + t.Fatalf("validate: %v", err) + } + if res.RepairAttempts != 0 { + t.Fatalf("repair attempts = %d, want 0", res.RepairAttempts) + } + }) + } +} + func TestStandardValidatorJSONSuccess(t *testing.T) { v := NewStandardValidator("") diff --git a/output_contract_contract_test.go b/output_contract_contract_test.go index 738db8d..9051a75 100644 --- a/output_contract_contract_test.go +++ b/output_contract_contract_test.go @@ -22,27 +22,52 @@ func TestPreparationRejectsInvalidOutputContractWithPublicError(t *testing.T) { t.Fatalf("construct engine: %v", err) } - req := promptkit.RunRequest{ - PromptID: "prompt", - Validation: &promptkit.OutputContract{ - Format: promptkit.OutputFormat("binary"), - ValidationMode: promptkit.ValidationNone, + for _, tc := range []struct { + name string + contract promptkit.OutputContract + }{ + { + name: "unsupported format", + contract: promptkit.OutputContract{ + Format: promptkit.OutputFormat("binary"), + ValidationMode: promptkit.ValidationNone, + }, }, - } + { + name: "repair attempts above maximum", + contract: promptkit.OutputContract{ + Format: promptkit.FormatText, + ValidationMode: promptkit.ValidationBasic, + RepairAttempts: 4, + }, + }, + { + name: "none validation with repair attempts", + contract: promptkit.OutputContract{ + Format: promptkit.FormatText, + ValidationMode: promptkit.ValidationNone, + RepairAttempts: 1, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + req := promptkit.RunRequest{PromptID: "prompt", Validation: &tc.contract} - prepared, err := engine.Prepare(context.Background(), req) - if prepared != nil { - t.Fatalf("expected no partial prepared run, got %+v", prepared) - } - if !errors.Is(err, promptkit.ErrInvalidRequest) { - t.Fatalf("prepare error = %v, want ErrInvalidRequest", err) - } + prepared, err := engine.Prepare(context.Background(), req) + if prepared != nil { + t.Fatalf("expected no partial prepared run, got %+v", prepared) + } + if !errors.Is(err, promptkit.ErrInvalidRequest) { + t.Fatalf("prepare error = %v, want ErrInvalidRequest", err) + } - preparedExecution, err := engine.PrepareExecution(context.Background(), req) - if preparedExecution != nil { - t.Fatalf("expected no partial prepared execution, got %+v", preparedExecution) - } - if !errors.Is(err, promptkit.ErrInvalidRequest) { - t.Fatalf("prepare execution error = %v, want ErrInvalidRequest", err) + preparedExecution, err := engine.PrepareExecution(context.Background(), req) + if preparedExecution != nil { + t.Fatalf("expected no partial prepared execution, got %+v", preparedExecution) + } + if !errors.Is(err, promptkit.ErrInvalidRequest) { + t.Fatalf("prepare execution error = %v, want ErrInvalidRequest", err) + } + }) } }