diff --git a/docs/internal/dnd.md b/docs/internal/dnd.md index af53933a..d3c0b2ef 100644 --- a/docs/internal/dnd.md +++ b/docs/internal/dnd.md @@ -134,6 +134,13 @@ relatedness validators report advisory evidence concerns. The configured order is documented in [Configuration](../config.md#production-validator-keys-and-default-chains). +Every D&D rejection describes the correction in transcript-grounded domain +terms, using contextual names, artifact fields, and source segment ranges when +useful. The guidance must not ask the model to reproduce durable entity IDs, +hashes, validator module keys, or reason codes. Those identifiers remain in +ordinary validation provenance; only the actionable semantic guidance is +eligible for the correction prompt. + Enemy-event extraction additionally rejects a second `engaged` observation for the same comparison identity within one scene-scoped result. Normalization may combine results from distinct scenes, so it intentionally does not apply that diff --git a/docs/internal/modules.md b/docs/internal/modules.md index cb84cac7..a28a11e4 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -45,6 +45,15 @@ they need, register each leaf implementation, and add any family-owned assets or default validator chains. They return contextual errors so production composition fails at startup rather than at the first run. +A validator that returns a completed rejection must supply two separate +bounded values: a stable `ReasonCode` for provenance and actionable +`CorrectionGuidance` for the producer. Guidance identifies the semantic defect +and the constraints on one complete corrected replacement. It must not contain +validator keys, diagnostic paths, opaque application IDs, or other internal +identifiers. An operator-facing `Message` may explain the same event, but the +framework never copies it into a model request. Missing or invalid guidance is +a validator contract failure. + An artifact family can register an optional typed evidence projector alongside its codec. The projector returns defensive copies of the artifact's direct generic source references and must use the codec's exact Go type. It does not @@ -131,6 +140,8 @@ its domain prompt. 4. Register the module through its typed registry helper and add it to the owning family registrar. Add a default validator chain only when that family owns the behavior; otherwise require an explicit compatible chain. + Every rejection path in a validator must provide actionable correction + guidance while retaining its stable internal reason code. 5. Update the selectable-key and chain reference in [Configuration](../config.md#production-module-keys), the applicable integration contract, and focused tests. Keep the configuration document diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index 03b0119f..c8ead3e0 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -143,7 +143,11 @@ immutable candidate; it does not regenerate the producer or alter the validator request. Rejections stop that validator, while other configured validators still run. The executor retains ordered results, bounded deduplicated correction guidance from rejections, and only the final exhausted -failure outcome for each validator. +failure outcome for each validator. The correction builder keeps first +occurrence order, omits internal reason codes, validator names, and operator +messages, and requests one complete replacement. Missing guidance or an +oversized aggregate is a framework contract error; guidance is never inferred +or truncated. The runner applies the binding's retry policy around a stage operation and its complete validation chain. It preserves warnings only from the final accepted @@ -163,12 +167,14 @@ settles the semantic policy immediately. Structural-output errors alone use the structural policy, and validation failure without rejection settles the validator-failure policy without regenerating the producer. -Chunk planning uses this state machine for generated plans. A rejected -automatic cache hit is not model material and therefore falls through to a -fresh initial generation; it neither receives a correction nor overwrites the -stored record. Only a newly generated, completely validated plan is published -to the chunk-plan store. Rejected plans never advance, and validation-incomplete -plans remain unpublishable. +Chunk planning uses this state machine for generated plans. A rejected or +validation-incomplete automatic cache hit is not model material and therefore +falls through to a fresh initial generation at producer attempt one; it neither +receives a correction, consumes retry budget, promotes cached-candidate +warnings, nor overwrites the stored record. An incomplete cache validation +under `fail_run` terminates instead. Only a newly generated, completely +validated plan is published to the chunk-plan store. Rejected plans never +advance, and validation-incomplete plans remain unpublishable. After terminal lane work, the runner assembles manifest provenance, normalized artifacts, rejections, warnings, and an optional accepted chunk map. When an @@ -204,7 +210,12 @@ The runner writes successful checkpoint artifacts only after complete accepted validation. Chunk plans follow the same rule for publication. A rejection, invalid structured response, or incomplete validation is never reusable state; the current run may still hand off an otherwise valid `warn_continue` result -according to its terminal policy. Attempt debug records retain safe kind, +according to its terminal policy. The runner carries private reuse eligibility +through extract, merge, normalize, and generated-reference handoff. Any stage +derived from incomplete validation skips both checkpoint lookup and all +checkpoint publication even when that stage's own validation completes. +External references and fully validated generated references remain eligible. +Attempt debug records retain safe kind, validator, repair-usage, policy, and terminal-decision provenance. Full assistant and correction content remains confined to the requested detailed LLM trace. diff --git a/docs/policy/architecture.md b/docs/policy/architecture.md index 1de3f86f..b95d2ed8 100644 --- a/docs/policy/architecture.md +++ b/docs/policy/architecture.md @@ -184,6 +184,13 @@ the configured `warn_continue` policy, advance a structurally valid candidate with explicit incomplete-validation provenance. Validators report findings; they do not choose candidate disposition. +A completed rejection supplies a stable reason code for internal provenance +and bounded actionable correction guidance for the candidate producer. Reason +codes, validator keys, and operator-facing messages remain diagnostic data; +they are not model instructions. The framework constructs model-facing retry +text only from the semantic guidance and fails the contract rather than +inventing or truncating missing guidance. + Default validator chains are production composition policy and are registered centrally by stage and module. Configuration may replace a stage-local default, including with an explicitly empty chain. Configured validator order is @@ -270,7 +277,10 @@ collaborator interfaces and never physical roots. Only accepted, completely validated producer output is reusable checkpoint or chunk-plan state. Rejected, structurally invalid, and validation-incomplete results cannot become cache or checkpoint inputs, even when a -`warn_continue` result is allowed to advance in the current run. +`warn_continue` result is allowed to advance in the current run. This +ineligibility follows derived merge and normalize results and generated +references for the remainder of the run: current-run handoff remains allowed, +but no dependent cache or checkpoint may be loaded or published. Writes are atomic where practical. Paths for writes, moves, overwrites, and deletion must be narrow and explicit. Notarius never automatically deletes diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index b6f49736..00000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,731 +0,0 @@ -# Feedback-Aware Validation Retry Implementation Plan - -## Status - -Ready for implementation. This plan implements the target state defined by -[Feedback-Aware Stage Validation Retries](validation-retries.md). Its stages -are ordered dependencies, and each stage is intentionally scoped for one -gpt-5.6-terra implementation prompt. - -The feature roadmap owns product intent and durable policy. This document owns -implementation order, concrete boundaries, and stage-level verification. If a -conflict is discovered, preserve the feature roadmap and the repository -policies, stop the affected stage, and revise this plan rather than silently -choosing a different architecture. - -## Settled Decisions - -The implementation agent must treat these decisions as fixed: - -- PromptKit v0.9.0 is the minimum and exact supported PromptKit release for - this work. Notarius uses `RunRequest.AppendedMessages`; it does not create - paired correction manifests or bypass PromptKit preparation and execution. -- A correction request reconstructs the ordinary initial request and appends - exactly two messages: the latest exact producer response as `assistant`, - followed by one deterministic aggregate correction request as `user`. - Earlier correction turns never accumulate. -- A correction-capable producer supplies the exact single LLM response that - directly controlled the candidate. The initial protocol does not synthesize - a model-facing projection of a compound artifact. -- The only supported producer correction protocol is - `single_response_v1`. An empty protocol means unsupported. Configuration - rejects semantic-retry workflows for LLM-backed compound producers or any - other producer unable to satisfy `single_response_v1`. -- The existing producer binding `retries` value is the sole outer stage - budget. PromptKit repair attempts and validator execution retries are - independent budgets. -- Terminal policy is configured as pipeline defaults with field-by-field - producer-binding overrides. Validators never own candidate disposition. -- Application defaults are `fail_run` for exhausted producer structural - failure, `fail_run` for exhausted semantic rejection, and `warn_continue` - for exhausted validator execution failure. -- All applicable validators run sequentially in configured order. Rejections - and validator failures are aggregated; failures are not represented to the - producer as candidate defects. -- Deterministic producers do not consume retries after rejection. An - LLM-backed attempt that took a deterministic fast path and produced no model - response is likewise not correctable for that candidate. -- Known rejected or structurally invalid output never advances. A candidate - accepted under `validator_failure: warn_continue` advances with explicit - incomplete-validation provenance but is not checkpointed. -- The D&D combat-scene validator and broader warning-system redesign remain - out of scope. - -## Cross-Stage Implementation Rules - -Apply these rules in every stage: - -1. Read the feature roadmap, `docs/development.md`, all `docs/policy/` - documents, and the focused current documentation named by the stage before - editing. -2. Inspect current code and tests rather than assuming paths or private helper - layouts. Preserve unrelated work and existing public ordering, - cancellation, scheduler, checkpoint, and sensitive-data invariants. -3. Use transport-neutral types outside `internal/framework/llm`. PromptKit - types must not escape the adapter boundary. -4. Keep correction material attempt-local. Do not put raw assistant responses - or correction text in ordinary errors, warnings, manifests, receipts, - checkpoints, caches, or default debug summaries. -5. Add the smallest durable tests that protect the stage's contracts. Do not - duplicate PromptKit's own prepared-execution, copying, hashing, capacity, - role-validation, or structural-repair test matrix. -6. Use `gofmt` on changed Go files. Run the focused checks listed for the - stage and fix failures before stopping. Do not proceed into the next stage - in the same prompt. -7. Do not delete this plan or the feature roadmap during implementation. They - are retired only after a final review confirms the complete target state. - -## Contract Names And Bounds - -Use one vocabulary consistently across packages. Exact private helper names -may follow local conventions, but the following contracts and values are not -open for redesign: - -- `CorrectionProtocol` has only the empty unsupported value and - `single_response_v1`. -- An application-owned `SemanticCorrection` carries owned - `AssistantResponse []byte` and `UserGuidance string` values. It is exposed - on chunk, typed extract, typed merge, typed normalize, and structured - completion requests as an optional pointer. -- An application-owned `ModelCandidate` carries the owned exact response bytes - and `CorrectionProtocol`. It is exposed on the corresponding producer - results as an optional pointer and is never serialized as part of a durable - artifact. -- `ValidationResult` gains optional `CorrectionGuidance`. Operator-facing - `Message` is not copied into this field implicitly. -- Each reason code is valid UTF-8, nonblank after trimming, and at most 128 - bytes. Each validator-supplied correction guidance value is valid UTF-8, - nonblank after trimming, and at most 4 KiB. -- The exact defective assistant response may be at most 1 MiB. The aggregate - user correction message may be at most 64 KiB. The combined appended - content may therefore be at most 1,114,112 bytes. Reject invalid UTF-8, - empty/whitespace-only content, or over-limit content; never truncate it. -- Invalid producer-owned correction material is a framework contract error. - It is not a validator failure and cannot be converted to `reject_output` or - `warn_continue`. -- Aggregate correction entries are ordered by validator order, contain the - stable reason code plus guidance, and collapse only byte-identical duplicate - `(reason_code, correction_guidance)` pairs while retaining first occurrence. - A rejection without guidance receives bounded generic guidance based only on - its reason code. -- Terminal actions are closed enums: structural failure and semantic - rejection accept `fail_run` or `reject_output`; validator failure accepts - `warn_continue` or `fail_run`. - -## Stage 1 ✅ — Adopt PromptKit v0.9.0 - -### Goal - -Upgrade the dependency without changing Notarius stage behavior, prove the -existing integration remains compatible, and make the new upstream request -primitive available to later stages. - -### Work - -- Update `go.mod` and `go.sum` to PromptKit v0.9.0 with `go get` and - `go mod tidy`. Accept the catalog modules selected transitively by PromptKit; - do not import or register them directly. -- Review every Notarius `promptkit.RunRequest` literal and retain keyed form. - Verify production prompt roles are limited to `system` and `user`. -- Update the conservative built-in-profile checkpoint marker in - `internal/framework/llm/promptkit_profile_fingerprint.go` from v0.8.0 to - v0.9.0. Do not duplicate the separately versioned catalog module versions. -- Update `docs/integrations/pkg-promptkit.md` only to describe the newly - implemented dependency version and unchanged current integration boundary. - Include the strict role vocabulary and external catalog ownership, but do - not document Notarius correction behavior yet. - -### Verification - -- Run `go test ./internal/framework/llm ./internal/cli`. -- Run `go test ./...`, `go test -race ./internal/framework/llm`, `go vet ./...`, - and `go build ./cmd/notarius`. - -### Exit Criteria - -Notarius builds and passes its suite on PromptKit v0.9.0, its compatibility -document and checkpoint marker name v0.9.0, and no Notarius package directly -depends on either upstream catalog module. This stage is one Terra prompt. - -## Stage 2 ✅ — Record The Architecture And Add Transport-Neutral Contracts - -### Goal - -Record the durable decision and introduce owned correction/candidate types -without changing runtime retry behavior. - -### Work - -- Add ADR-0014 under `docs/adr/` using the repository ADR format. Record the - three distinct retry budgets, complete validator aggregation, the fresh - two-message correction protocol, exact-response producer capability, - non-recursive validator failure handling, terminal-policy ownership and - defaults, checkpoint conservatism, and sensitive-data constraints. -- Add `CorrectionProtocol`, `SemanticCorrection`, and `ModelCandidate` to the - domain-neutral framework contracts, with constructors/clone helpers that - validate the settled bounds and defensively copy bytes. -- Add optional correction input to `ChunkRequest`, - `TypedExtractionRequest`, `TypedMergeRequest`, and - `TypedNormalizeRequest`. -- Add optional model-candidate output to `ChunkPlanResult`, - `TypedExtractionResult`, `TypedMergeResult`, and - `TypedNormalizeResult`. -- Add `CorrectionGuidance` to `ValidationResult` and centralize validation of - reason codes and corrective guidance at the framework boundary. -- Update type-erasure adapters, stored fakes, and clone paths so the new values - retain ownership and cannot alias caller buffers. Do not declare production - module capabilities or use the fields in the runner yet. - -### Verification - -- Add contract-level tests for accepted values, UTF-8 and size rejection, - defensive copying, nil behavior, and typed-erasure preservation. -- Run `go test ./internal/framework/contracts ./internal/framework/pipeline`. -- Run `go test ./...`. - -### Exit Criteria - -The generic contracts can safely carry correction input and exact candidate -material across every producer stage, no PromptKit type crosses the boundary, -and existing runtime behavior is unchanged. This stage is one Terra prompt. - -## Stage 3 ✅ — Add Validation Policy Configuration And Resolution - -### Goal - -Implement strict configuration, inheritance, identity, and provenance for -terminal validation policy before the runner consumes it. - -### Work - -- Add optional `validation_policy` to a pipeline profile and to chunk, - extract, merge, and normalize producer bindings. Use pointer-backed override - fields so omission is distinguishable from an explicit value. -- Reject an explicitly null policy object, null policy fields, duplicate or - unknown fields, and values outside the settled enums. Keep the current file - configuration version. -- Reject `validation_policy` on input, output, and validator bindings. Reject - a binding-level explicit `producer_structural_failure` value on a - deterministic producer; a pipeline-level structural default remains valid - because a pipeline may contain LLM-backed producers. -- Resolve each policy field independently in binding, pipeline, application - default order. Store one detached concrete effective policy for chunk and - for each lane's extract, merge, and normalize stage; do not leave runtime - inheritance to the runner. -- Include both configured overrides and effective policies at their existing - appropriate configuration/provenance boundaries. Include the effective - values in resolved-pipeline digest and checkpoint identity. Preserve - deterministic serialization and redaction. -- Update `examples/dnd-minimal/config.yml` and - `examples/dnd-complete/config.yml` only if their current shape must change to - remain valid; do not add redundant explicit defaults merely to demonstrate - the feature. - -### Verification - -- Add focused parsing tests for omission versus null, unknown/duplicate - fields, invalid placements, and invalid enums. -- Add resolution tests for field-by-field inheritance and mixed overrides. -- Add digest, clone, JSON/YAML round-trip, and redacted-effective-config tests. -- Run `go test ./internal/core/config ./internal/framework/pipeline ./internal/cli`. - -### Exit Criteria - -Every producer stage receives one immutable effective policy, invalid -configuration fails before execution, and policy changes invalidate resolved -identity. The runner still follows its old behavior until later stages. This -stage is one Terra prompt. - -## Stage 4 ✅ — Declare And Validate Producer Correction Capability - -### Goal - -Make correction support an explicit module contract and prepare the framework -to reject impossible workflows. - -### Work - -- Extend `pipeline.ModuleSpec` with `CorrectionProtocol`; normalize, clone, - validate, and include it in relevant registry/spec fingerprints. -- Permit `single_response_v1` only for LLM-backed chunk, extract, merge, or - normalize modules. Reject it for deterministic, input, output, or validator - specs. Empty remains the default unsupported value. -- Carry the selected protocol into resolved and prepared producer metadata and - checkpoint fingerprints. -- Reject positive `retries` on deterministic validator bindings during - resolution. LLM-backed validator retries remain valid and independent of - producer capability. - -### Verification - -- Add spec normalization/clone tests, invalid stage/class combinations, - validator retry-class tests, and preparation tests for supported and - unsupported producer workflows. -- Run `go test ./internal/framework/pipeline ./internal/core/config`. - -### Exit Criteria - -Capability is explicit, fingerprinted, and testable, while existing production -pipelines remain executable pending their migrations. No dormant feature gate -or unused preparation check is introduced. This stage is one Terra prompt. - -## Stage 5 — Adapt Corrections Through PromptKit ✅ - -### Goal - -Map the transport-neutral correction contract onto PromptKit v0.9.0 without -changing ordinary requests. - -### Work - -- Add optional `SemanticCorrection` to `StructuredCompletionRequest` and its - debug-safe request representation. -- In `PromptKitClient.CompleteStructured`, validate and defensively copy the - correction, then map it to exactly two `promptkit.RenderedMessage` values in - `RunRequest.AppendedMessages`, using `promptkit.RoleAssistant` followed by - `promptkit.RoleUser`. -- Leave `AppendedMessages` nil for an ordinary request. Do not allow callers to - choose other roles through the Notarius contract. -- Ensure request formatting and default debug summaries expose only safe - counts/digests. The existing explicitly requested detailed PromptKit trace - may contain complete effective messages and must retain its existing - sensitive-data treatment. -- Preserve the same prompt identity, inputs, variables, session, profile, - execution overrides, repair attempts, scheduler, and prepared-execution - path for corrected calls. - -### Verification - -- Add focused adapter tests proving an ordinary request is unchanged and a - corrected request has the ordinary rendered prefix plus exactly the two - supplied messages in order with exact content. -- Test invalid/oversized correction rejection before preparation and absence - of raw content from ordinary formatting/errors. -- Add one representative test showing PromptKit structural repair remains - available on a request with appended messages; do not reproduce upstream's - complete repair suite. -- Run `go test ./internal/framework/llm ./internal/framework/pipeline` and - `go test -race ./internal/framework/llm`. - -### Exit Criteria - -Notarius has one safe, tested adapter path for fresh corrected requests, and no -stage invokes it yet. This stage is one Terra prompt. - -## Stage 6 — Migrate The Scene Chunker And Foundational Extractors ✅ - -### Goal - -Make the scene chunker and the foundational direct D&D extractors satisfy -`single_response_v1`. - -### Work - -- Update `dnd/scenes`, `dnd/npc-registry`, `dnd/item-registry`, - `dnd/location-registry`, `dnd/scene-descriptions`, and `dnd/spells` producer - implementations to pass request correction to their structured completion - and return an owned copy of the successful response's exact validated raw - bytes as `ModelCandidate`. -- Declare `single_response_v1` in each corresponding module spec. -- Do not serialize normalized artifacts to fabricate candidate material. Keep - artifact parsing, deterministic identity attachment, evidence validation, - warnings, and durable schemas unchanged. -- Update shared D&D extraction helpers only where the behavior is genuinely - common; keep domain prompt ownership in each module. - -### Verification - -- Use representative package-level tests to prove exact-response propagation, - correction forwarding, and input/result ownership. Update spec tests for the - declared protocol without multiplying the same behavior test across all six - modules. -- Run the affected module tests and `go test ./internal/modules/dnd/...`. - -### Exit Criteria - -All named producers truthfully advertise and implement the exact-response -protocol, with unchanged ordinary extraction behavior. This stage is one -Terra prompt. - -## Stage 7 — Migrate Downstream D&D Extractors ✅ - -### Goal - -Complete `single_response_v1` support for the remaining direct D&D extraction -producers. - -### Work - -- Migrate `dnd/npc-occurrences`, `dnd/item-occurrences`, - `dnd/location-occurrences`, `dnd/combat-turns`, and `dnd/enemy-events` using - the same contract as Stage 6. -- Preserve their contextual-name and deterministic-ID rules. Correction - messages must never introduce opaque registry IDs or ask the model to copy - them. -- Preserve combat-scene gating, reference projections, and prompt-cache prefix - ordering. The correction pair is appended after the complete existing - request and never changes stable prompt assets. - -### Verification - -- Add or adapt focused tests for correction forwarding and exact raw-response - retention at the shared boundary, plus one registry-grounded extractor case - proving no opaque IDs enter correction material. -- Run the affected module tests, `go test ./internal/modules/dnd/...`, and the - assembled D&D CLI contract tests. - -### Exit Criteria - -Every direct production D&D LLM extractor implements the same correction -protocol without changing artifact semantics. This stage is one Terra prompt. - -## Stage 8 — Migrate Semantic Reconciliation Normalizers ✅ - -### Goal - -Support correction for the current single-proposal reconciliation path and -activate capability enforcement for all eligible production producers. - -### Work - -- Extend `semanticreconcile.Request` to accept `SemanticCorrection` and its - result to expose the exact validated proposal response as owned - `ModelCandidate` when an LLM call actually occurred. -- Forward correction through the shared engine's ordinary structured - completion request. Preserve request-local integer candidate handles, - proposal validation, typed application policies, and fallback behavior. -- Update the NPC-, item-, and location-registry normalizers to carry the exact - proposal material through their typed result and declare - `single_response_v1`. -- Mark deterministic skip/limit/fallback outcomes as having no model candidate. - If such a candidate is later rejected, it is not correctable and must not - consume a retry merely because its module execution class is LLM-backed. -- Audit all production LLM-backed producer specs. Add the preparation rule that - an LLM-backed producer with a non-empty validator chain and `retries > 0` - must declare `single_response_v1`. Any genuine multi-response producer - remains unsupported and a configured validator-backed semantic retry for it - must fail preparation. - -### Verification - -- Add engine tests for initial and corrected requests, exact proposal bytes, - deterministic no-call outcomes, invalid proposal outcomes, and ownership. -- Add representative normalizer and preparation tests for supported, - unsupported, and no-validator/no-retry configurations. -- Run `go test ./internal/framework/semanticreconcile ./internal/framework/pipeline ./internal/modules/dnd/... ./internal/cli`. - -### Exit Criteria - -All eligible production LLM producers implement the declared protocol, -unsupported workflows fail before source parsing, and ordinary operational -retries remain allowed when semantic correction cannot occur. This stage is -one Terra prompt. - -## Stage 9 — Build Complete Validator-Chain Execution ✅ - -### Goal - -Replace first-result validation with one reusable, deterministic executor that -aggregates the complete chain and isolates validator retries. - -### Work - -- Introduce an internal immutable `validationReport` model with one ordered - invocation record per validator: approved, rejected, failed, or skipped; - validator attempt count; safe reason/message metadata; warnings; and bounded - correction guidance. -- Implement one shared sequential executor around stage-specific invocation - closures. It must continue after rejection and isolated execution failure, - retry an LLM-backed failed validator against the same immutable candidate up - to its binding budget, and stop retrying after a contract-valid approval or - rejection. -- Reconstruct the same ordinary validator request for each validator retry. - Do not append feedback to validator prompts and do not regenerate the - producer candidate. -- Preserve only one final warning for an exhausted validator failure; retain - individual attempt details for debug. Keep warnings from completed validators - in configured order. -- Build aggregate correction guidance from all rejections using the settled - ordering, deduplication, generic fallback, and bounds. Validator failures, - skips, warnings, and operator messages must not enter it. -- Retain `skipped` as a framework-owned outcome for an otherwise selected - validator whose runtime prerequisites are unavailable. Module construction, - type incompatibility, candidate cloning failure, cancellation, and debug - persistence failure remain framework errors rather than skips. - -### Verification - -- Add table-driven behavior tests for all approved, multiple rejections, - rejection plus failure, failure only, skipped, retry success, retry - exhaustion, immutable candidate reuse, deterministic ordering, duplicate - guidance, and bounds. -- Prove deterministic validators cannot receive positive retry budgets and - LLM validator retry counts do not consume producer attempts. -- Run `go test ./internal/framework/pipeline` and - `go test -race ./internal/framework/pipeline`. - -### Exit Criteria - -The shared executor returns a complete ordered report without deciding whether -the producer candidate advances. Existing stage callers may still adapt the -report through their old disposition path until the next stages. This stage is -one Terra prompt. - -## Stage 10 — Implement The Generic Producer Attempt State Machine ✅ - -### Goal - -Replace the boolean retry helper with one explicit domain-neutral state -machine used by later stage integrations. - -### Work - -- Refactor `runWithRetry` into an attempt engine that accepts producer and - complete-validation closures and returns a terminal result plus ordered - attempt provenance. Keep stage-specific artifact handling outside it. -- Classify attempts as initial, operational-error retry, structural retry, - module-requested retry, or semantic correction. Preserve one total producer - budget of `retries + 1` attempts. -- On semantic rejection, correct only when another attempt exists and the - current candidate contains valid `single_response_v1` material. Construct a - fresh `SemanticCorrection` from that latest response and aggregate feedback. -- Apply outcome precedence and effective terminal policy exactly as the - feature roadmap specifies. Ordinary producer errors remain framework errors - after retries. Only `ErrInvalidStructuredOutput` uses the structural-failure - policy; `reject_output` records no usable artifact. -- A deterministic or no-model candidate applies semantic terminal policy - immediately without spending an ineffective retry. -- With rejection plus validator failure, use rejection guidance for a - correction while recording incomplete validation. With failure only, apply - validator-failure policy without regenerating the producer. -- Keep cancellation and debug-persistence failures immediately terminal. - Abandoned-attempt warnings must not be promoted. - -### Verification - -- Test the state machine through stable behavior with fake closures: budget - accounting, fresh correction history, latest-response replacement, all - terminal actions, failure precedence, deterministic candidates, - cancellation, and warning promotion. -- Do not assert private helper call choreography or exact correction prose. -- Run `go test ./internal/framework/pipeline` and its race-enabled suite. - -### Exit Criteria - -One tested state machine owns attempt budgets and disposition, but no stage is -partially migrated. This stage is one Terra prompt. - -## Stage 11 — Integrate Chunk Validation And Correction ✅ - -### Goal - -Move chunk generation, validation, cache reuse, and retry disposition onto the -new state machine. - -### Work - -- Adapt chunk candidate materialization and both chunk/serialized validator - targets to the complete-chain executor. -- Pass correction only to a generated chunker attempt and retain the exact raw - chunker response associated with the materialized plan. -- Preserve chunk-plan cache identity and mode semantics. A rejected automatic - cache hit is unusable for the invocation and falls through to generated - attempt one without pretending the cache hit has model material. Publish a - newly generated plan only after complete accepted validation. -- Apply chunk effective terminal policy. A terminal chunk rejection prevents - lane execution; a framework error still prevents output encoding. -- Preserve annotation materialization, deterministic plan canonicalization, - attempt debug paths, scheduler use, and checkpoint behavior. - -### Verification - -- Add focused tests for corrected plan acceptance, multi-validator feedback, - rejected cache hit regeneration, cache non-overwrite, incomplete-validation - non-publication, policy outcomes, cancellation, and debug failure. -- Run `go test ./internal/framework/pipeline ./internal/modules/dnd/chunk/... ./internal/cli` and the pipeline race tests. - -### Exit Criteria - -Chunk is the first complete production stage using feedback-aware retries, with -cache and terminal behavior matching the roadmap. This stage is one Terra -prompt. - -## Stage 12 ✅ — Integrate Per-Chunk Extraction - -### Goal - -Apply the state machine independently to every concurrent extraction job. - -### Work - -- Adapt typed and serialized extract validation to the complete-chain executor - and use the extract binding's effective policy. -- Keep correction state local to one lane/chunk job. Rebuild the same initial - extraction request with correction attached only on a semantic retry. -- Preserve the run-wide worker pool, scheduled LLM client, chunk-first/lane- - second public ordering, stable error selection, cancellation, and lane - continuation rules. -- Ensure `reject_output` records the terminal chunk-scoped rejection without - advancing it to merge. `warn_continue` is available only for validator - failure with no semantic rejection and marks the accepted extract as - validation-incomplete. - -### Verification - -- Add representative extraction tests for correction success, exhausted - rejection, validator failure policy, independent concurrent jobs, stable - ordering, cancellation, and warning promotion. -- Add one assembled D&D pipeline test using fakes—not a live provider—to prove - a rejected direct extraction is corrected using the exact prior response. -- Run `go test ./internal/framework/pipeline ./internal/modules/dnd/... ./internal/cli` and relevant race tests. - -### Exit Criteria - -Every extraction job has an isolated bounded correction conversation and -continues to obey existing concurrency and ordering contracts. This stage is -one Terra prompt. - -## Stage 13 ✅ — Integrate Merge And Normalize - -### Goal - -Complete stage coverage and reconcile semantic correction with the existing -normalizer fallback retry directive. - -### Work - -- Adapt typed and serialized merge and normalize validation to the shared - executor and their respective effective policies. -- Keep merge and normalize serial within a lane and reuse the exact same - accepted upstream artifacts, references, source input, profile, and session - on every attempt. -- Fold `NormalizeRetry` into the one producer attempt state machine. It consumes - the same remaining stage budget, retains its validated safe fallback when - exhausted, and never overrides a semantic rejection of that fallback or a - later candidate. -- Do not send correction to deterministic mergers/normalizers. A rejected - deterministic candidate applies terminal policy immediately. -- Preserve typed erasure checks, codec boundaries, generated handoffs, and - lane continuation/cancellation behavior. - -### Verification - -- Add one representative merge path and normalize paths for correction - success, deterministic rejection, `NormalizeRetry` success/exhaustion, - fallback rejection, validator failure, and terminal policies. -- Run `go test ./internal/framework/pipeline ./internal/framework/semanticreconcile ./internal/modules/dnd/... ./internal/cli` and relevant race tests. - -### Exit Criteria - -Chunk, extract, merge, and normalize all use the same attempt and validation -semantics, with no nested normalizer retry loop. This stage is one Terra -prompt. - -## Stage 14 ✅ — Finalize Provenance, Checkpoints, Debug, And Durable Results - -### Goal - -Make the new state machine auditable and safe without leaking correction -content or allowing degraded output to become reusable state. - -### Work - -- Extend internal attempt/debug records with attempt kind, PromptKit repair - count and usage, ordered validator outcomes and attempt counts, aggregate - reason codes, validation completeness, effective policy, and terminal - decision. -- Keep raw assistant responses and complete correction messages only in the - existing explicitly requested detailed trace. Default summaries contain - counts, digests, identities, and bounded safe fields. -- Add one reusable durable validation summary with the JSON fields - `status`, `rejecting_validators`, `reason_codes`, - `incomplete_validators`, `producer_attempt_count`, and `terminal_action`. - `status` is one of `complete`, `rejected`, or `incomplete`; lists preserve - configured order and omit duplicates after their first occurrence. Embed or - project this summary at the manifest, rejection, and run-result boundaries - that already expose the affected stage outcome. Preserve the existing - singular rejection fields as the first configured rejection for downstream - continuity; do not put raw guidance or responses in the summary. -- Emit one genuine, bounded, deterministically ordered warning per validator - whose execution budget is exhausted under `warn_continue`; do not emit a - warning merely because a later correction succeeded. -- Checkpoint only accepted and completely validated results. Never write or - reuse rejected, structurally invalid, or validation-incomplete producer - output. Ensure correction protocol and effective policy participate in - fingerprints. -- Confirm cache, resume, generated-reference handoff, output encoding, and CLI - result publication cannot treat a rejected or incomplete checkpoint as - accepted. - -### Verification - -- Add focused manifest, receipt, warning, debug, redaction, checkpoint reuse, - generated-handoff, and sensitive-content tests at their canonical owners. -- Test that corrected success is auditable but quiet, warn-continue output is - never checkpointed, and raw content is absent from ordinary durable files. -- Run `go test ./internal/framework/pipeline ./internal/framework/checkpoint ./internal/framework/artifacts ./internal/cli` and relevant race tests. - -### Exit Criteria - -Every terminal path has accurate bounded provenance, no sensitive correction -content leaks by default, and only completely validated output is reusable. -This stage is one Terra prompt. - -## Stage 15 ✅ — Update Canonical Documentation And Perform Final Verification - -### Goal - -Document the implemented contracts in their canonical homes and prove the -repository is ready for review. - -### Work - -- Update `docs/policy/architecture.md` with durable validator aggregation, - retry-budget separation, producer correction capability, terminal-policy, - checkpoint, and sensitive-data invariants. Link to ADR-0014 for rationale. -- Update `docs/config.md` with the exact `validation_policy` schema, enum - values, defaults, inheritance, placement restrictions, retry-budget - meanings, and invalid combinations. -- Update `docs/operations.md` with costs, terminal outcomes, warnings, debug - sensitivity, retry exhaustion, resume/cache consequences, and recovery. -- Update `docs/internal/pipeline.md`, `docs/internal/llm.md`, and - `docs/internal/modules.md` with implemented mechanics and focused test - routing. Keep public configuration definitions in `docs/config.md`. -- Complete `docs/integrations/pkg-promptkit.md` for v0.9.0 appended messages, - supported roles, application-owned bounds, and the external catalog - boundary. Update the run-result and affected output/subprocess integration - contracts for any durable fields added in Stage 14. -- Update maintained examples only to demonstrate implemented behavior. Use a - minimal policy override in the complete D&D example if it materially aids - operators; keep the minimal example minimal. Ensure all links point to - canonical owners and remove stale v0.8.0 claims outside historical release - notes. -- Do not create a release note until an actual release is prepared. - -### Verification - -- Run `gofmt` on all changed Go files. -- Run `go test ./...`, `go test -race ./...`, `go vet ./...`, and - `go build ./cmd/notarius`. -- Run the repository's example/config validation tests and documentation link - checks. If no standalone link checker exists, verify changed relative links - and record that manual check in the implementation report. -- Use targeted searches to confirm no current documentation still claims - PromptKit v0.8.0, no production correction-capable spec is missing its - protocol, no PromptKit type escaped `internal/framework/llm`, and no raw - correction content is serialized outside explicit detailed debug data. -- Review `git diff --check` and `git status --short`; do not include generated - binaries, temporary files, or unrelated work. - -### Exit Criteria - -All roadmap acceptance criteria are satisfied, canonical documentation matches -the code, maintained examples validate, the complete ordinary and race-enabled -test suites pass, and the worktree contains only intentional implementation -changes. This stage is one Terra prompt. - -## Open Questions - -None. PromptKit transport, producer representation, terminal-policy ownership, -bounds, retry budgets, outcome precedence, and checkpoint treatment are all -settled by the feature roadmap and this plan. diff --git a/docs/roadmap/validation-retries.md b/docs/roadmap/validation-retries.md deleted file mode 100644 index 552198ad..00000000 --- a/docs/roadmap/validation-retries.md +++ /dev/null @@ -1,609 +0,0 @@ -# Feedback-Aware Stage Validation Retries - -## Status - -Proposed. This is the active feature roadmap for the next Notarius work set. -Its design decisions are settled. Current behavior remains authoritative until -this roadmap is implemented and the corresponding ADR and canonical -documentation are updated. - -## Purpose - -Make validation an effective corrective boundary around LLM-produced stage -candidates. When deterministic or LLM-backed validators reject a structurally -valid candidate, Notarius should give the producing model the complete, -ordered validation feedback and use the stage's existing retry budget to ask -for a corrected replacement. The feature must distinguish semantic rejection -from producer failure and validator execution failure, preserve the boundary -between PromptKit repair and Notarius stage retries, and remain safe under -concurrency, cancellation, caching, checkpoints, and sensitive input. - -This work is domain-neutral. It establishes the framework behavior required by -future LLM-backed validators such as D&D combat-scene review, but it does not -add that validator. - -## User Intent - -- A stage candidate should be evaluated by every applicable configured - validator before Notarius decides whether to retry or terminate. -- A semantic retry should be materially more useful than repeating the same - request. The producing model should see its latest defective response and - all actionable semantic feedback. -- PromptKit's bounded structural repair and Notarius's stage retry loop are - separate. Each stage attempt receives its own complete PromptKit repair - budget; PromptKit repair never consumes or replenishes the stage budget. -- Deterministic rejection, semantic rejection, producer structural failure, - and validator execution failure are different outcomes and must not be - collapsed into one generic error path. -- The default posture is strict for known-invalid producer output and tolerant - but visible when a validator itself cannot make a decision. -- Corrective prompts must not expose opaque application identifiers, secrets, - or unbounded diagnostic content merely because those values exist in an - internal artifact or operator-facing error. - -## Current State - -The current runner already provides useful foundations: - -- chunk, extract, merge, and normalize producer bindings have one `retries` - value interpreted as additional stage attempts; -- `runWithRetry` retries producer errors and semantic rejections within that - budget; -- PromptKit performs bounded structural repair inside each structured - completion; -- validator targets, execution classes, profile selection, repair policy, - attempts, debug scopes, checkpoint identity, and deterministic public - ordering are already explicit; and -- the structured-completion response retains the model's validated raw bytes - and PromptKit repair metadata. - -The current behavior is not yet the desired corrective workflow: - -- the runner repeats the ordinary producer request after rejection and does - not pass the previous model response or validator feedback; -- validation stops at the first rejection or execution failure, so later - applicable validators do not contribute findings; -- validator execution failure is immediately a framework error rather than a - configurable incomplete-validation outcome; -- `ValidationResult.Message` currently serves operator diagnostics and does - not define separately bounded model-facing guidance; -- typed stage results do not carry the exact model response needed for the - next correction attempt; -- validator-binding `retries` values participate in resolved configuration but - are not used to retry a failed validator against the same candidate; and -- Notarius still pins PromptKit v0.8, while PromptKit v0.9.0 now provides the - append-only request-message API needed for application-owned correction - attempts. - -## Target End State - -For chunk, extract, merge, and normalize stages, Notarius owns one explicit -candidate-attempt state machine: - -1. The producer creates one candidate using the ordinary request. An - LLM-backed producer may use PromptKit structural repair internally. -2. The framework establishes one immutable validation candidate and runs every - applicable validator sequentially in configured order. -3. The framework aggregates approvals, warnings, semantic rejections, - execution failures, and skipped-validator diagnostics without allowing one - validator to mutate the candidate seen by another. -4. A candidate with one or more semantic rejections is never accepted. If the - LLM-backed producer has another stage attempt available, Notarius rebuilds - the complete original prompt and appends the latest defective assistant - response followed by one application-owned correction message containing - every actionable rejection. It then requests one complete replacement - candidate. -5. A producer error consumes the same stage attempt budget under the existing - retry rules, but semantic correction material is used only when a - structurally valid candidate was actually rejected. -6. A validator execution failure is retried, when configured, against the same - immutable candidate. It never regenerates the producer candidate by itself. -7. When budgets are exhausted, the configured terminal policies decide - whether the run fails, a rejected output is recorded, or a structurally - valid candidate advances with explicitly incomplete validation. - -The first attempt remains byte-for-byte the ordinary prompt rendered from the -selected prompt definition. Every correction attempt starts from that same -ordinary prompt rather than from the prior correction conversation. It appends -exactly two messages: - -- an `assistant` message containing the producer-supplied exact defective - response for the latest candidate; and -- a `user` message containing deterministic, bounded, application-owned - correction guidance and asking for one complete replacement response. - -The session ID, prompt ID and version, selected profile, reasoning settings, -structured-output contract, repair budget, named inputs, variables, references, -and reusable prompt prefix remain unchanged across stage attempts. - -## Architectural Ownership - -### PromptKit - -PromptKit continues to own prompt loading and rendering, profile resolution, -backend admission, provider generation, structural validation, and bounded -structural repair within one completion. A PromptKit repair conversation is -private to that completion and is not exposed as a Notarius stage attempt. - -PromptKit v0.9.0 owns the mechanical operation of appending explicitly supplied -messages to a normally rendered prompt before creating the immutable prepared -execution. `RunRequest.AppendedMessages` preserves the original rendered -messages as an exact prefix, validates and defensively copies additions, -includes the complete sequence in prepared details and the rendered-prompt -hash, and runs it through the ordinary generation and structural-repair path. -PromptKit does not impose message-count, byte-size, token, or context-window -limits and permits empty content, so Notarius retains its stricter -application-owned correction validation and bounds. - -### Notarius Framework - -The framework owns stage budgets, immutable candidate preparation, complete -validator-chain execution, result aggregation, outcome precedence, correction -message construction, terminal policy, public ordering, checkpoint effects, -manifest summaries, warnings, and debug lifecycle. - -The framework must remain domain-neutral. It may format stable reason codes and -validator-supplied corrective guidance, but it must not infer D&D or other -domain rules from artifact JSON. - -### Producers And Artifact Families - -The producing module owns prompt selection, prompt inputs, typed decoding, and -the model-facing representation that corresponds to its candidate. An -LLM-backed producer that supports feedback-aware correction must return the -exact response material that the model should see as its prior assistant turn. -It must not substitute a normalized artifact containing deterministically -attached UUIDs or other opaque application identity. - -Artifact-family validators own semantic decisions and domain-specific -corrective guidance. Operator-facing explanation and model-facing correction -are separate contract fields even when their concise text happens to match. - -## Validation Outcome Model - -Each validator invocation produces one of four framework outcomes: - -| Outcome | Meaning | Effect | -| --- | --- | --- | -| Approved | The validator completed and accepted the whole candidate. | Retain its warnings and continue the chain. | -| Rejected | The validator completed and found a semantic defect in the candidate. | Record the finding, continue the chain, and make the candidate ineligible for acceptance. | -| Failed | The validator could not return a usable decision because of an internal, transport, generation, structural-output, or result-invariant failure. | Retry that validator when eligible, then record incomplete validation and continue the chain unless cancellation or framework integrity prevents it. | -| Skipped | Runtime prerequisites for an otherwise selected validator cannot be satisfied. | Record a deterministic incomplete-validation diagnostic and continue; do not invent a semantic decision. | - -Configured validator order controls invocation order and aggregate feedback -order. Execution remains sequential initially. The framework must continue -after a rejection and after an isolated validator failure when it can safely -prepare the remaining validator requests. Cancellation, inability to preserve -an immutable candidate, debug persistence failure, or another framework -integrity failure remains immediately terminal. - -### Outcome Precedence - -For one candidate, apply this precedence: - -1. A producer structural failure means no acceptable candidate exists and - cannot be converted into validator approval. -2. Any completed semantic rejection makes the candidate rejected, even when - another validator failed or was skipped. -3. With no semantic rejection, a validator failure or skip makes validation - incomplete and invokes the validator-failure policy. -4. Only a structurally valid candidate with no rejection and either complete - validation or an explicit `warn_continue` decision may advance. - -Do not turn a known rejection into acceptance through a permissive -validator-failure policy. Do not turn a structurally invalid response into a -rejected-but-usable artifact. - -## Corrective Feedback Contract - -`ValidationResult` should gain a separately bounded, optional model-facing -correction field. A rejecting production validator should provide: - -- a stable reason code suitable for aggregation and provenance; -- an operator-facing message suitable for ordinary diagnostics; and -- concise corrective guidance that explains the violated rule without asking - the model to reproduce opaque identity or leaking unrelated source data. - -The framework constructs one deterministic correction message from all -rejections in validator order. Each entry identifies the stable reason code -and corrective guidance. Duplicate identical entries may be collapsed while -preserving first occurrence; distinct findings must not be discarded merely -to shorten the message. If a validator rejects without model-facing guidance, -the framework uses a generic reason-code-based correction rather than copying -the operator message automatically. - -Warnings, validator failures, skipped diagnostics, provider messages, stack -traces, debug paths, and sensitive values are not corrective guidance. They may -be recorded through their proper diagnostic channels but must not be presented -to the producer as candidate defects. - -The framework must validate UTF-8, role, non-empty content, and -application-owned size limits before constructing the correction request. Oversized or -invalid correction material is a framework-owned inability to perform a -feedback retry; it must never be silently truncated into a misleading or -syntactically defective assistant response. - -## Producer Correction Contracts - -Introduce application-owned, defensively copied correction contracts at the -framework boundary: - -- chunk, typed extraction, typed merge, and typed normalize results can carry - optional model-facing candidate material associated with their returned - value; -- the corresponding requests can carry an optional correction containing the - latest assistant material and aggregated guidance; -- `StructuredCompletionRequest` can carry the two bounded appended messages - without importing PromptKit types into module or pipeline contracts; and -- the PromptKit adapter translates those application-owned messages into - `RunRequest.AppendedMessages` using `promptkit.RoleAssistant` and - `promptkit.RoleUser` before preparation. - -A semantic correction always supplies exactly two appended messages: the -latest defective response as `assistant`, followed by the aggregate correction -request as `user`. The framework does not expose the other PromptKit-supported -roles through this contract and does not accumulate messages from earlier -stage attempts. PromptKit preserves message content exactly, but Notarius must -reject empty content and enforce its own per-message and aggregate byte limits -before the adapter is called. - -Correction material is attempt-local sensitive data. It is not part of the -artifact schema, checkpoint value, cache key, durable output bundle, ordinary -error, or configuration summary. The policy and capability that affect -execution do participate in resolved pipeline and checkpoint identity. - -LLM-backed modules selected with both `retries > 0` and a non-empty validator -chain must declare whether they can produce and consume correction material. -Preparation must reject a pipeline that could request feedback-aware semantic -retries from an LLM-backed producer without that capability. An LLM-backed -producer with no validators may continue to use its retry budget for -operational failures without declaring semantic-correction capability. - -A correction-capable producer must supply the exact single LLM response that -directly controlled the candidate being validated. Direct D&D chunk and -extraction producers expose their exact structured response. The shared -semantic-reconciliation path exposes its exact proposal response through its -typed normalizers without turning request-local batch handles into durable -identity. Deterministic transformations after that response are permitted only -when the validated candidate remains directly traceable to it. - -A producer whose candidate combines multiple LLM responses is not -correction-capable under this initial protocol. It may continue to use ordinary -operational retries when no semantic correction can occur, but configuration -must reject a validator-backed retry workflow for it. Supporting compound -producers later requires a separately reviewed multi-response protocol; the -framework must not synthesize an assistant message by serializing the final -typed artifact. - -Deterministic producers do not receive correction material. A deterministic -candidate rejected by validation immediately applies the terminal semantic -rejection policy without consuming retries that cannot change the result. - -## Retry Budgets - -### Producer Stage Budget - -The existing producer binding `retries` field remains the sole outer stage -budget. `retries: N` means at most `N` additional complete producer attempts -after the initial attempt. Producer operational errors, producer structural -failures, module-requested normalize retries, and semantic corrections all -draw from this same budget. Do not add a separate semantic retry counter. - -Every LLM-backed producer attempt receives the configured PromptKit -`structured_output_repair_attempts` value independently. Notarius does not -decrement that value across stage attempts. - -### Validator Budget - -Use the existing `retries` field on an LLM-backed validator binding for -additional attempts to obtain a usable decision about the same immutable -candidate. A completed approval or rejection is terminal for that validator -and does not consume another validator attempt. A validator retry reconstructs -the same ordinary validator prompt; it does not append semantic feedback about -the validator's prior failed judgment and does not create a recursive -Notarius correction loop. - -Reject a positive validator `retries` value on a deterministic validator at -configuration resolution because repeating the same pure decision cannot -improve it. Validator retries do not consume the producer stage budget. - -## PromptKit v0.9.0 Adoption - -The target end state pins PromptKit v0.9.0 for correction requests. The -resolved dependency graph includes its independently versioned -OpenRouter and Rakestrawhome catalog modules through ordinary Go module -resolution; Notarius must not import or register those catalogs directly. -PromptKit continues to own their built-in backend and profile IDs, source -precedence, credentials, and capacity behavior. - -Notarius's PromptKit compatibility documentation and built-in-profile -checkpoint marker identify v0.9.0 rather than v0.8.0. The PromptKit release -identity remains the conservative checkpoint identity for the exact catalog -versions selected by that release; Notarius should not duplicate upstream -catalog module versions in a second hand-maintained marker. - -PromptKit v0.9.0 restricts text-chat roles to `developer`, `system`, `user`, and -`assistant`. Maintained Notarius prompt definitions already use only `system` -and `user`; correction requests add only `assistant` and `user`. PromptKit -`RunRequest` literals remain keyed. These compatibility conditions must remain -covered by the ordinary production-asset and adapter checks without adding a -brittle inventory test that merely counts prompt messages or literals. - -## Terminal Policy Configuration - -Add an optional `validation_policy` object at pipeline scope and on chunk, -extract, merge, and normalize producer bindings: - -```yaml -validation_policy: - producer_structural_failure: fail_run - semantic_rejection: fail_run - validator_failure: warn_continue -``` - -The binding object overrides individual pipeline values; resolution is -field-by-field in binding, pipeline, application-default order. Omitted values -inherit rather than replacing the complete object. Explicit null, unknown -fields, and unknown enum values are invalid. The effective policy is resolved -and detached before execution, appears in redacted effective configuration and -run provenance, and participates in the resolved pipeline digest and checkpoint -identity. - -The initial enum values and defaults are: - -- `producer_structural_failure`: `fail_run` by default; `reject_output` may - retain a terminal rejection and final raw candidate for debug, but may not - advance or publish an invalid artifact; -- `semantic_rejection`: `fail_run` by default after stage attempts are - exhausted; `reject_output` records the aggregate rejection and allows - unrelated work to complete without advancing that candidate; and -- `validator_failure`: `warn_continue` by default, which advances a - structurally valid and otherwise unrejected candidate with explicit - incomplete-validation provenance and one genuine warning; `fail_run` - terminates the run. - -Producer structural policy applies only to LLM-backed producers. Semantic and -validator-failure policies apply to any validated producer. Input and output -bindings do not accept `validation_policy`, and validator bindings do not own -terminal policy; they own only their decision and their own operational retry -budget. Candidate disposition belongs to the chunk, extract, merge, or -normalize producer binding after its complete validator chain has run. - -Keep the current file-configuration version. The syntax is strictly -decodable without a version change. The project is pre-v1, but the behavior -and output changes should still be called out in the next release note and -downstream documentation. - -## Stage-Specific Behavior - -### Chunk - -A generated chunk plan is structurally validated and materialized before the -validator chain runs. Semantic feedback applies to the exact raw chunker -response associated with that plan. - -When an automatically reused chunk-plan record is rejected by the current -validator chain, treat the record as unusable for this invocation and enter -ordinary generation at attempt one. A cache hit is not a new model attempt and -does not supply model-facing assistant material. Do not overwrite the cached -record until a newly generated plan is accepted. Refresh and bypass modes -retain their existing publication rules. - -### Extract - -Each chunk-scoped extraction job owns its own attempt state and correction -conversation. One rejected chunk candidate does not cancel unrelated chunks or -lanes unless terminal policy converts it into a framework error. Deterministic -public ordering remains chunk-first and lane-second regardless of concurrent -completion. - -### Merge And Normalize - -Merge and normalize remain serial within a lane. A correction attempt receives -the same accepted upstream artifacts and references as the initial attempt. -The existing safe-fallback `NormalizeRetry` mechanism must be reconciled with -the shared attempt state rather than layered into a second retry loop: it uses -the same stage budget, retains its documented fallback behavior, and cannot -override a known validator rejection. - -The initial feature supports one exact producer-supplied assistant response per -candidate attempt. Future multi-request normalization or batching must define -which response directly represents the candidate, or supply a new explicitly -reviewed correction protocol, before it can claim feedback-aware correction. - -## LLM-Backed Validators - -An LLM-backed validator uses the same scheduled PromptKit client, selected -profile, session, timeout, and structural-repair policy as other LLM-backed -modules. PromptKit may structurally repair its response inside one validator -attempt. - -- A contract-valid validator response is its decision; Notarius does not ask a - second LLM to judge that judgment. -- A structurally invalid final validator response, transport failure, or - deterministic violation of the validator-result contract is a validator - execution failure. -- Validator execution retries reuse the immutable producer candidate and do - not regenerate it. -- Exhaustion invokes `validator_failure` policy and emits a genuine warning - under `warn_continue`. - -This feature supplies the generic execution model only. It does not register a -production LLM-backed validator or change a D&D default validator chain. - -## Provenance, Diagnostics, And Sensitive Data - -Attempt debug output should make the state machine auditable. When debug is -enabled, record: - -- producer attempt number and whether it was initial, error retry, module - retry, or semantic correction; -- PromptKit repair count and cumulative usage for every completion; -- each validator's configured-order outcome and validator attempt count; -- aggregate rejection codes and the bounded correction message; -- effective terminal policy and the decision it produced; and -- whether validation was complete, rejected, or incomplete. - -Raw assistant responses and correction messages belong only in explicitly -requested detailed debug traces, following existing allowlisted content-file, -redaction, permission, and retention rules. Ordinary errors, CLI output, -warnings, manifests, checkpoints, caches, and run receipts contain identities, -counts, bounded safe summaries, and reason codes—not raw source or model -content. - -The durable run manifest and rejection summaries should record enough -structured information to distinguish: - -- the number and kinds of producer attempts; -- completed semantic rejection and all rejecting validator identities; -- incomplete validation and failed or skipped validator identities; -- the effective terminal policy and terminal result; and -- successful use of a correction attempt without treating it as a warning. - -Warnings from abandoned producer attempts must not be promoted. Warnings from -the accepted attempt remain eligible. A warn-and-continue validator failure -produces one bounded, deterministically ordered warning per affected validator -after its retry budget is exhausted; detailed repeated failures stay in debug -provenance. - -## Checkpoints, Caches, Concurrency, And Cancellation - -- Effective validation policy, producer correction capability/protocol - version, validator chain, validator retry budgets, and prompt assets must all - affect checkpoint identity. -- Only accepted, completely validated stage outputs may be checkpointed or - reused. Rejected, structurally invalid, and validation-incomplete outputs - accepted under a permissive policy must not be written as reusable stage - checkpoints. This conservative rule avoids treating a transient validator - outage as durable validation success; a future checkpoint-status contract may - revisit it explicitly. -- Correction attempts use the same run-wide scheduler and worker bounds as - initial completions. No retry path may bypass provider admission. -- A scheduled permit covers the complete PromptKit operation, including its - internal structural repair, and is reacquired normally for a later Notarius - stage attempt. -- Parent cancellation dominates producer, validator, retry, debug, cache, and - checkpoint work. Cancellation never becomes a rejection, warning, or - incomplete-validation acceptance. -- Framework errors retain deterministic selection and cancellation behavior - across concurrently executing chunks and lanes. - -## Architecture Record And Canonical Documentation - -The target end state includes an accepted ADR that records: - -- the separation between PromptKit structural repair, producer stage attempts, - and validator execution retries; -- the complete validator-chain aggregation rule and outcome precedence; -- the fresh reconstruction plus two-message correction protocol; -- module ownership of model-facing candidate material; -- deterministic producer and non-recursive validator behavior; -- default fail-closed and fail-open terminal policies; and -- provenance, cache, identity, and sensitive-data constraints. - -The canonical owners describe the implemented behavior without duplicating -one another: - -- `docs/policy/architecture.md` for durable validation and retry invariants; -- `docs/config.md` for fields, values, precedence, defaults, and validation; -- `docs/operations.md` for costs, failure behavior, warnings, debug handling, - and recovery; -- `docs/internal/pipeline.md` for the attempt state machine, aggregation, - checkpoint behavior, and concurrency; -- `docs/internal/llm.md` for appended correction messages and the distinction - from PromptKit repair; -- `docs/internal/modules.md` for producer and validator contracts; -- `docs/integrations/pkg-promptkit.md` for PromptKit v0.9.0, - `RunRequest.AppendedMessages`, supported message roles, application-owned - bounds, and the independently versioned upstream catalog boundary; and -- affected output and subprocess integration documents for durable validation - status and rejection summaries. - -Until this target state is implemented, canonical current-state documentation -continues to describe the existing behavior. - -## Testing Strategy - -Tests should protect observable state-machine behavior rather than private -helper layout or exact prose. The target test suite includes: - -- contract tests proving the first request is unchanged and a correction - request contains the same initial messages plus exactly one assistant and one - user message; -- behavioral runner tests for all-approved, multiple-rejection, - rejection-plus-failure, failure-only, skipped, retry-success, and each - terminal policy outcome; -- one representative path for chunk, extract, merge, and normalize, without - duplicating the complete state matrix at every stage; -- proof that all validators see immutable equivalent candidates and execute in - configured order after an earlier rejection or isolated failure; -- proof that validator retries reuse the candidate and do not consume producer - retries; -- proof that deterministic rejection does not repeat the producer; -- focused PromptKit-adapter tests proving that application-owned correction - messages map to the two intended PromptKit roles without content leakage; -- config parsing, precedence, invalid-placement, round-trip, redaction, and - digest tests for effective policy; -- checkpoint and chunk-cache tests for rejected, corrected, incomplete, and - accepted outcomes; -- warning, manifest, receipt, debug, and sensitive-content tests at their - canonical boundaries; and -- a small assembled D&D pipeline test proving a rejected direct extraction can - be corrected without a live provider. - -Tests remain offline and deterministic. Do not reproduce PromptKit's internal -message-copying, rendered-hash, prepared-execution, capacity, or repair suite. -One representative adapter or assembled-run test should prove that PromptKit -structural repair remains usable after Notarius appends semantic-correction -messages. Do not snapshot full prompts or error prose, assert private -constants, or multiply equivalent tests across every D&D artifact family. - -## Acceptance Criteria - -- Every applicable validator runs in configured order and contributes one - explicit outcome before candidate disposition. -- Multiple semantic rejections produce one bounded, deterministic correction - request containing all actionable findings. -- Correction attempts reconstruct the exact ordinary prompt and append only - the latest defective assistant response and one correction message. -- Notarius pins PromptKit v0.9.0 and routes correction messages through - `RunRequest.AppendedMessages`; it does not maintain paired correction prompt - manifests or bypass PromptKit's normal execution path. -- Every correction-capable LLM producer exposes the exact single response that - directly controlled its candidate. Configuration rejects semantic retries - for compound producers that cannot satisfy that contract. -- The existing producer `retries` value is the only producer-stage budget; - PromptKit structural repair and validator execution retries remain separate. -- Deterministic producers are not repeated after semantic rejection. -- Validator execution failure is never described to the producer as a - candidate defect and never creates recursive semantic validation. -- Default terminal behavior is `fail_run` for structural failure and semantic - rejection, and `warn_continue` with explicit incomplete validation for - validator failure. -- Terminal policy resolves field by field from producer-binding override to - pipeline default to application default; individual validators do not own - candidate disposition. -- Permissive policy never advances known rejected or structurally invalid - output. -- Raw model responses and correction content are confined to model requests and - explicitly requested debug traces. -- Checkpoint, cache, manifest, warning, concurrency, cancellation, and - deterministic-ordering invariants remain intact. -- Canonical architecture, configuration, operations, internal, integration, - and ADR documentation accurately describe the implemented behavior. -- Focused, full, and race-enabled Go tests; vet; builds; example validation; - formatting; link checks; and repository hygiene checks pass. - -## Non-Goals - -- Adding the D&D combat-scene semantic validator. -- Redesigning the warning taxonomy beyond the warnings required for validator - failure and retry outcomes. -- Concurrent validator execution. -- Unbounded or accumulating conversational history. -- A second semantic retry counter. -- Recursive LLM judgment of LLM-validator decisions. -- Provider-specific retry policy or bypassing PromptKit. -- General workflow graphs or new pipeline stages. -- Large-collection reconciliation batching or a generic multi-response - correction protocol. diff --git a/internal/cli/assembled_spell_pipeline_contract_test.go b/internal/cli/assembled_spell_pipeline_contract_test.go index fca9d23b..608a4537 100644 --- a/internal/cli/assembled_spell_pipeline_contract_test.go +++ b/internal/cli/assembled_spell_pipeline_contract_test.go @@ -124,8 +124,8 @@ func TestAssembledSpellPipelineCorrectsRejectedDirectExtraction(t *testing.T) { t.Fatalf("run output = %#v, want corrected accepted spell output", output) } correction := extractor.correctionSnapshot() - if correction == nil || string(correction.AssistantResponse) != `{"spell":"Mysterious Burst"}` || correction.UserGuidance != "use a known spell name" { - t.Fatalf("extract correction = %#v, want exact rejected model response and validator guidance", correction) + if correction == nil || string(correction.AssistantResponse) != `{"spell":"Mysterious Burst"}` || !strings.Contains(correction.UserGuidance, "use a known spell name") || !strings.Contains(correction.UserGuidance, "complete corrected replacement") || strings.Contains(correction.UserGuidance, "unknown_spell") || strings.Contains(correction.UserGuidance, "spell is not in the catalog") { + t.Fatalf("extract correction = %#v, want exact rejected model response and semantic replacement guidance only", correction) } } diff --git a/internal/framework/contracts/correction.go b/internal/framework/contracts/correction.go index f10e28b3..1880aeb1 100644 --- a/internal/framework/contracts/correction.go +++ b/internal/framework/contracts/correction.go @@ -126,11 +126,17 @@ func (candidate ModelCandidate) Validate() error { } func ValidateValidationResult(result ValidationResult) error { + if !result.Approved && result.ReasonCode == "" { + return errors.New("validation rejection reason code must not be empty") + } if result.ReasonCode != "" { if err := validateBoundedText(result.ReasonCode, MaxValidationReasonCodeBytes, "validation reason code", false); err != nil { return err } } + if !result.Approved && result.CorrectionGuidance == "" { + return errors.New("validation rejection correction guidance must not be empty") + } if result.CorrectionGuidance != "" { if err := validateBoundedText(result.CorrectionGuidance, MaxValidationCorrectionGuidanceBytes, "validation correction guidance", false); err != nil { return err diff --git a/internal/framework/contracts/correction_test.go b/internal/framework/contracts/correction_test.go index afb178ca..04e856a6 100644 --- a/internal/framework/contracts/correction_test.go +++ b/internal/framework/contracts/correction_test.go @@ -59,13 +59,21 @@ func TestCorrectionContractsRejectInvalidContent(t *testing.T) { {"unsupported protocol", func() error { _, err := NewModelCandidate([]byte("response"), "multiple_responses"); return err }}, {"missing candidate protocol", func() error { _, err := NewModelCandidate([]byte("response"), ""); return err }}, {"blank candidate response", func() error { _, err := NewModelCandidate([]byte(" "), CorrectionProtocolSingleResponseV1); return err }}, - {"oversized reason code", func() error { return ValidateValidationResult(ValidationResult{ReasonCode: tooLongReason}) }}, - {"blank reason code", func() error { return ValidateValidationResult(ValidationResult{ReasonCode: " \t"}) }}, + {"missing rejection reason code", func() error { + return ValidateValidationResult(ValidationResult{CorrectionGuidance: "Correct the response."}) + }}, + {"missing rejection guidance", func() error { return ValidateValidationResult(ValidationResult{ReasonCode: "invalid"}) }}, + {"oversized reason code", func() error { + return ValidateValidationResult(ValidationResult{ReasonCode: tooLongReason, CorrectionGuidance: "Correct the response."}) + }}, + {"blank reason code", func() error { + return ValidateValidationResult(ValidationResult{ReasonCode: " \t", CorrectionGuidance: "Correct the response."}) + }}, {"invalid correction guidance utf8", func() error { - return ValidateValidationResult(ValidationResult{CorrectionGuidance: string([]byte{0xff})}) + return ValidateValidationResult(ValidationResult{ReasonCode: "invalid", CorrectionGuidance: string([]byte{0xff})}) }}, {"oversized correction guidance", func() error { - return ValidateValidationResult(ValidationResult{CorrectionGuidance: tooLongValidationGuidance}) + return ValidateValidationResult(ValidationResult{ReasonCode: "invalid", CorrectionGuidance: tooLongValidationGuidance}) }}, } { t.Run(test.name, func(t *testing.T) { diff --git a/internal/framework/pipeline/checkpoint.go b/internal/framework/pipeline/checkpoint.go index 0aa4e417..70d1f64e 100644 --- a/internal/framework/pipeline/checkpoint.go +++ b/internal/framework/pipeline/checkpoint.go @@ -89,6 +89,7 @@ const ( CheckpointReasonReused CheckpointReasonCode = "checkpoint_reused" CheckpointReasonAcceptedArtifactReused CheckpointReasonCode = "accepted_artifact_reused" CheckpointReasonRecomputeStep CheckpointReasonCode = "recompute_step" + CheckpointReasonValidationIncompleteLineage CheckpointReasonCode = "validation_incomplete_lineage" ) type CheckpointDecision struct { @@ -160,6 +161,8 @@ func checkpointDecisionDetail(reasonCode CheckpointReasonCode) string { return "accepted normalized artifact is reusable" case CheckpointReasonRecomputeStep: return "selected step requires execution" + case CheckpointReasonValidationIncompleteLineage: + return "checkpoint reuse is disabled by validation-incomplete input lineage" default: return "checkpoint decision" } diff --git a/internal/framework/pipeline/evidence_output_test.go b/internal/framework/pipeline/evidence_output_test.go index caacbd13..d80b6e48 100644 --- a/internal/framework/pipeline/evidence_output_test.go +++ b/internal/framework/pipeline/evidence_output_test.go @@ -114,7 +114,7 @@ func TestRunnerEvidenceContextOmitsAbsentAndRejectedLanes(t *testing.T) { prepared.Steps[1].lanes[0].mergeValidators.validators = []preparedValidator{{ resolved: ResolvedValidator{Binding: Binding("reject"), Target: ValidatorTargetTyped, ArtifactKind: "test/notes"}, typedValidate: func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { - return contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "not accepted"}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "not accepted", CorrectionGuidance: "return an acceptable candidate"}, nil }, }} prepared.Steps[1].lanes[0].resolved.MergeValidationPolicy.SemanticRejection = SemanticRejectionRejectOutput diff --git a/internal/framework/pipeline/handoff.go b/internal/framework/pipeline/handoff.go index 069a730c..b4ea2be6 100644 --- a/internal/framework/pipeline/handoff.go +++ b/internal/framework/pipeline/handoff.go @@ -41,6 +41,54 @@ func operationReferenceSet(input RunInput, target ResolvedReferenceTarget) contr return CloneReferenceSet(target.ReferenceSet) } +// referenceTargetReuseEligible reports whether every generated artifact in a +// stage's reference set descends exclusively from fully validated work. Static +// references and callers that do not supply lineage metadata are reusable. +func referenceTargetReuseEligible(input RunInput, target ResolvedReferenceTarget) bool { + if input.referenceReuseEligibility == nil { + return true + } + eligible, ok := input.referenceReuseEligibility[keyForReferenceTarget(target)] + return !ok || eligible +} + +func laneReferencesReuseEligible(input RunInput, lane ResolvedArtifactLane) bool { + return referenceTargetReuseEligible(input, lane.ExtractReferences) && + referenceTargetReuseEligible(input, lane.MergeReferences) && + referenceTargetReuseEligible(input, lane.NormalizeReferences) +} + +// buildStepReferenceReuseEligibility carries validation completeness alongside +// generated references without exposing the internal lineage flag in artifact +// payloads. A target becomes ineligible when any generated input is ineligible. +func buildStepReferenceReuseEligibility(step PreparedPipelineStep, outputs map[generatedOutputKey]bool) map[referenceTargetKey]bool { + eligibility := make(map[referenceTargetKey]bool) + for _, prepared := range step.lanes { + lane := prepared.resolved + for _, target := range []ResolvedReferenceTarget{lane.ExtractReferences, lane.MergeReferences, lane.NormalizeReferences} { + generated := false + eligible := true + for _, binding := range target.Bindings { + if binding.Artifact == nil { + continue + } + generated = true + producer := generatedOutputKeyFor(binding.Artifact.Step, binding.Artifact.Lane) + if reusable, ok := outputs[producer]; ok && !reusable { + eligible = false + } + } + if generated { + eligibility[keyForReferenceTarget(target)] = eligible + } + } + } + if len(eligibility) == 0 { + return nil + } + return eligibility +} + // buildStepReferenceSets resolves every generated binding for a step before // any lane in that step is allowed to start. Each returned set is a fresh // operation-time view; prepared reference sets are never modified. diff --git a/internal/framework/pipeline/producer_attempts.go b/internal/framework/pipeline/producer_attempts.go index d1550aa3..e6e61953 100644 --- a/internal/framework/pipeline/producer_attempts.go +++ b/internal/framework/pipeline/producer_attempts.go @@ -232,7 +232,12 @@ func runProducerAttempts(ctx context.Context, config producerAttemptConfig, prod } } if number < attemptLimit && correctionCandidate != nil && correctionCandidate.Protocol == contracts.CorrectionProtocolSingleResponseV1 { - correction, err = contracts.NewSemanticCorrection(correctionCandidate.Response, report.CorrectionGuidance()) + correctionRequest, guidanceErr := report.CorrectionRequest() + if guidanceErr != nil { + provenance = append(provenance, producerAttemptProvenance{Number: number, Kind: kind, Outcome: producerAttemptFailed, Validation: report}) + return failedProducerAttempt(provenance), fmt.Errorf("construct semantic correction request: %w", guidanceErr) + } + correction, err = contracts.NewSemanticCorrection(correctionCandidate.Response, correctionRequest) if err != nil { provenance = append(provenance, producerAttemptProvenance{Number: number, Kind: kind, Outcome: producerAttemptFailed, Validation: report}) return failedProducerAttempt(provenance), fmt.Errorf("construct semantic correction: %w", err) diff --git a/internal/framework/pipeline/runner.go b/internal/framework/pipeline/runner.go index cf7a3722..7d281a04 100644 --- a/internal/framework/pipeline/runner.go +++ b/internal/framework/pipeline/runner.go @@ -61,10 +61,11 @@ type RunInput struct { // single worker so direct framework callers retain deterministic behavior. ExtractWorkers int - pipeline ResolvedPipeline - llmClient contracts.StructuredLLMClient - stepID string - references map[referenceTargetKey]contracts.ReferenceSet + pipeline ResolvedPipeline + llmClient contracts.StructuredLLMClient + stepID string + references map[referenceTargetKey]contracts.ReferenceSet + referenceReuseEligibility map[referenceTargetKey]bool } type RunOutput struct { @@ -76,6 +77,8 @@ type RunOutput struct { OutputFiles []contracts.OutputFile `json:"-"` CheckpointEvents []CheckpointEvent `json:"checkpoint_events,omitempty"` ValidationSummaries []artifacts.ValidationSummary `json:"validation_summaries,omitempty"` + + normalizeReuseEligibility map[generatedOutputKey]bool } func (r *Runner) Run(ctx context.Context, input RunInput) (output RunOutput, err error) { @@ -425,6 +428,7 @@ func (r *Runner) runPreparedSteps(ctx context.Context, input RunInput, checkpoin return fmt.Errorf("prepare generated references for pipeline step %q: %w", step.ID, err) } stepInput.references = stepReferences + stepInput.referenceReuseEligibility = buildStepReferenceReuseEligibility(step, output.normalizeReuseEligibility) output.Manifest.References = append(output.Manifest.References, referenceProvenance...) laneOutput, laneErr := r.runLanes(ctx, stepInput, step, checkpoints, loader, doc, sourceInput, sessionID, chunks) if err := mergeLaneOutput(output, laneOutput); err != nil { diff --git a/internal/framework/pipeline/runner_attempt_debug_test.go b/internal/framework/pipeline/runner_attempt_debug_test.go index 777f3f86..71bd9929 100644 --- a/internal/framework/pipeline/runner_attempt_debug_test.go +++ b/internal/framework/pipeline/runner_attempt_debug_test.go @@ -237,7 +237,7 @@ func TestRunnerRecordsDistinctRetryAttemptsAndPromotesAcceptedWarningsOnly(t *te resolved: ResolvedValidator{Binding: Binding("retry-check"), Target: ValidatorTargetTyped, ArtifactKind: "test/notes"}, typedValidate: func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { validatorCalls++ - return contracts.ValidationResult{Approved: validatorCalls > 1, ReasonCode: "retry", Message: "retry candidate"}, nil + return contracts.ValidationResult{Approved: validatorCalls > 1, ReasonCode: "retry", Message: "retry candidate", CorrectionGuidance: "return an acceptable candidate"}, nil }, } switch stage { @@ -280,11 +280,12 @@ func TestRunnerRecordsDistinctRetryAttemptsAndPromotesAcceptedWarningsOnly(t *te func TestRunnerAttemptDebugRepresentsFailuresAndFinalRejection(t *testing.T) { tests := []struct { - name string - configure func(*PreparedPipeline) - path string - wantError string - wantBody string + name string + configure func(*PreparedPipeline) + path string + wantError string + wantBody string + attemptError bool }{ { name: "merge module error", @@ -293,8 +294,9 @@ func TestRunnerAttemptDebugRepresentsFailuresAndFinalRejection(t *testing.T) { return erasedTypedResult{}, errors.New("merge exploded") } }, - path: "merge/notes/attempt-01.json", - wantError: "merge exploded", + path: "merge/notes/attempt-01.json", + wantError: "merge exploded", + attemptError: true, }, { name: "normalize validator error", @@ -318,7 +320,7 @@ func TestRunnerAttemptDebugRepresentsFailuresAndFinalRejection(t *testing.T) { prepared.Steps[0].lanes[0].mergeValidators.validators = []preparedValidator{{ resolved: ResolvedValidator{Binding: Binding("reject-check"), Target: ValidatorTargetTyped, ArtifactKind: "test/notes"}, typedValidate: func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { - return contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "not accepted"}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "not accepted", CorrectionGuidance: "return an acceptable candidate"}, nil }, }} }, @@ -332,8 +334,9 @@ func TestRunnerAttemptDebugRepresentsFailuresAndFinalRejection(t *testing.T) { return erasedTypedResult{Value: "wrong artifact type"}, nil } }, - path: "normalize/notes/attempt-01.json", - wantError: "serialize normalize candidate", + path: "normalize/notes/attempt-01.json", + wantError: "serialize normalize candidate", + attemptError: true, }, } @@ -345,9 +348,15 @@ func TestRunnerAttemptDebugRepresentsFailuresAndFinalRejection(t *testing.T) { output, runErr := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Debug: debug}) envelope := debug.envelope(t, tc.path) if tc.wantError != "" { - if runErr == nil || !strings.Contains(runErr.Error(), tc.wantError) || !strings.Contains(envelope.Error, tc.wantError) { + if runErr == nil || !strings.Contains(runErr.Error(), tc.wantError) { t.Fatalf("run error = %v, envelope error = %q; want %q", runErr, envelope.Error, tc.wantError) } + if tc.attemptError && !strings.Contains(envelope.Error, tc.wantError) { + t.Fatalf("attempt envelope error = %q, want %q", envelope.Error, tc.wantError) + } + if !tc.attemptError && envelope.Error != "" { + t.Fatalf("settled validator failure attempt error = %q, want empty", envelope.Error) + } } else if runErr != nil { t.Fatalf("Run() error = %v, want nil rejection outcome", runErr) } diff --git a/internal/framework/pipeline/runner_candidate_encoding_test.go b/internal/framework/pipeline/runner_candidate_encoding_test.go index 47ae57da..7e907c52 100644 --- a/internal/framework/pipeline/runner_candidate_encoding_test.go +++ b/internal/framework/pipeline/runner_candidate_encoding_test.go @@ -160,7 +160,7 @@ func setCandidateValidator(prepared *PreparedPipeline, target ModuleStage, appro validator := preparedValidator{ resolved: ResolvedValidator{Binding: Binding("candidate-check"), Target: ValidatorTargetTyped, ArtifactKind: "test/notes"}, typedValidate: func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { - return contracts.ValidationResult{Approved: approved, ReasonCode: "candidate_rejected", Message: "candidate rejected by validator"}, nil + return contracts.ValidationResult{Approved: approved, ReasonCode: "candidate_rejected", Message: "candidate rejected by validator", CorrectionGuidance: "return an acceptable candidate"}, nil }, } switch target { diff --git a/internal/framework/pipeline/runner_chunk_plan.go b/internal/framework/pipeline/runner_chunk_plan.go index cd9aeb19..cca20478 100644 --- a/internal/framework/pipeline/runner_chunk_plan.go +++ b/internal/framework/pipeline/runner_chunk_plan.go @@ -72,10 +72,9 @@ func (r *Runner) runChunkPlan(ctx context.Context, input RunInput, doc *source.S result.setValidation(report.Warnings(), nil, err) return result, err } - if report.FirstRejection() == nil { - if incomplete := firstIncompleteValidation(report); incomplete != nil && input.pipeline.ChunkValidationPolicy.ValidatorFailure == ValidatorFailureFailRun { - return result, validatorFailureError(*incomplete) - } + rejection := report.FirstRejection() + incomplete := firstIncompleteValidation(report) + if rejection == nil && incomplete == nil { if err := result.setCandidate(record, "reused"); err != nil { return result, fmt.Errorf("clone reused chunk plan record: %w", err) } @@ -85,18 +84,21 @@ func (r *Runner) runChunkPlan(ctx context.Context, input RunInput, doc *source.S result.accepted = true result.setValidation(report.Warnings(), nil, nil) cachedTerminal := producerAttemptTerminal{Action: producerTerminalAccepted, Validation: report} - if firstIncompleteValidation(report) != nil { - result.summary.ValidationStatus = "incomplete" - cachedTerminal.Action = producerTerminalIncompleteAccepted - cachedTerminal.ValidationIncomplete = true - } summary := validationSummary(cachedTerminal, StageChunk, "", "", chunker.Key(), "", 0) result.validation = &summary return result, nil } + if incomplete != nil && input.pipeline.ChunkValidationPolicy.ValidatorFailure == ValidatorFailureFailRun { + failure := validatorFailureError(*incomplete) + result.setValidation(report.Warnings(), nil, failure) + failedTerminal := producerAttemptTerminal{Action: producerTerminalFailed, Validation: report, ValidationIncomplete: true} + summary := validationSummary(failedTerminal, StageChunk, "", "", chunker.Key(), "", 0) + result.validation = &summary + return result, failure + } // A cache hit is not model material. Its rejection is discarded and - // generation begins with the ordinary initial request below. - result.setValidation(report.Warnings(), chunkRejection(report, 1, chunker.Key()), nil) + // generation begins with the ordinary initial request below. Warnings + // from this discarded candidate are intentionally not promoted. } result.lookup = ChunkPlanDecision{Status: ChunkPlanInvalid, Reason: chunkPlanLookupReason(ChunkPlanInvalid)} result.summary.LookupStatus = "invalid" @@ -176,11 +178,6 @@ func (r *Runner) runChunkPlan(ctx context.Context, input RunInput, doc *source.S if validationErr != nil { return report, candidate.terminal.record(payload, validationErr) } - if report.FirstRejection() == nil && input.pipeline.ChunkValidationPolicy.ValidatorFailure == ValidatorFailureFailRun { - if failure := firstIncompleteValidation(report); failure != nil { - return report, candidate.terminal.record(payload, validatorFailureError(*failure)) - } - } return report, candidate.terminal.record(payload, nil) }) terminalSummary := validationSummary(terminal, StageChunk, "", "", chunker.Key(), "", 0) diff --git a/internal/framework/pipeline/runner_chunk_plan_test.go b/internal/framework/pipeline/runner_chunk_plan_test.go index 07666376..33142562 100644 --- a/internal/framework/pipeline/runner_chunk_plan_test.go +++ b/internal/framework/pipeline/runner_chunk_plan_test.go @@ -293,7 +293,7 @@ func TestRunnerOmitsChunkMapForRejectedPlan(t *testing.T) { prepared.output = encoder prepared.chunkValidators.validators = []preparedValidator{{ resolved: ResolvedValidator{Binding: Binding("reject/chunk"), Target: ValidatorTargetChunk}, - chunk: &countingChunkValidator{result: contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "not accepted"}}, + chunk: &countingChunkValidator{result: contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "not accepted", CorrectionGuidance: "return an acceptable chunk plan"}}, }} output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input")}) if err != nil { @@ -351,6 +351,92 @@ func TestRunnerDoesNotPublishValidationIncompleteChunkPlan(t *testing.T) { } } +func TestRunnerRegeneratesValidationIncompleteChunkPlanHit(t *testing.T) { + prepared, plan := preparedTerminalDebugPipeline(t) + prepared.resolved.ChunkValidationPolicy.ValidatorFailure = ValidatorFailureWarnContinue + calls := 0 + prepared.chunker = terminalChunker{key: prepared.resolved.Chunk.Module, plan: plan, calls: &calls} + validatorCalls := 0 + prepared.chunkValidators.validators = []preparedValidator{{ + resolved: ResolvedValidator{Binding: Binding("cache-then-generated"), Target: ValidatorTargetChunk}, + chunk: chunkValidationFunc{name: "cache-then-generated", validate: func(context.Context, contracts.ChunkValidationRequest) (contracts.ValidationResult, error) { + validatorCalls++ + if validatorCalls == 1 { + return contracts.ValidationResult{}, errors.New("cached candidate could not be validated") + } + return contracts.ValidationResult{Approved: true}, nil + }}, + }} + record := chunkPlanRecord(t, prepared, plan) + record.Warnings = []contracts.Warning{{Scope: "stored", ReasonCode: "old", Message: "discarded stored warning"}} + store := &recordingChunkPlanStore{record: record, decision: ChunkPlanDecision{Status: ChunkPlanHit}} + debug := newCapturedDebugRecorder() + + output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), ChunkCacheMode: ChunkCacheAuto, ChunkPlans: store, Debug: debug}) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if calls != 1 || validatorCalls != 2 || store.saves != 1 { + t.Fatalf("calls = producer %d validator %d saves %d, want 1 2 1", calls, validatorCalls, store.saves) + } + if output.ChunkPlan == nil || output.ChunkPlan.Action != "generated" || output.ChunkPlan.LookupStatus != "invalid" || output.ChunkPlan.ValidationStatus != "approved" { + t.Fatalf("chunk plan summary = %#v", output.ChunkPlan) + } + if len(output.Warnings) != 0 { + t.Fatalf("warnings = %#v, want discarded cache warnings omitted", output.Warnings) + } + assertAttemptEnvelopeSequence(t, debug, "chunk", 1) +} + +func TestRunnerKeepsStoredPlanWhenCacheAndGeneratedValidationAreIncomplete(t *testing.T) { + prepared, plan := preparedTerminalDebugPipeline(t) + prepared.resolved.ChunkValidationPolicy.ValidatorFailure = ValidatorFailureWarnContinue + calls := 0 + prepared.chunker = terminalChunker{key: prepared.resolved.Chunk.Module, plan: plan, calls: &calls} + validator := &countingChunkValidator{err: errors.New("validator unavailable")} + prepared.chunkValidators.validators = []preparedValidator{{resolved: ResolvedValidator{Binding: Binding(validator.Name()), Target: ValidatorTargetChunk}, chunk: validator}} + record := chunkPlanRecord(t, prepared, plan) + record.Warnings = []contracts.Warning{{Scope: "stored", ReasonCode: "old", Message: "discarded stored warning"}} + store := &recordingChunkPlanStore{record: record, decision: ChunkPlanDecision{Status: ChunkPlanHit}} + + output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), ChunkCacheMode: ChunkCacheAuto, ChunkPlans: store}) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if calls != 1 || validator.calls != 2 || store.saves != 0 { + t.Fatalf("calls = producer %d validator %d saves %d, want 1 2 0", calls, validator.calls, store.saves) + } + if output.ChunkPlan == nil || output.ChunkPlan.Action != "generated" || output.ChunkPlan.LookupStatus != "invalid" || output.ChunkPlan.ValidationStatus != "incomplete" { + t.Fatalf("chunk plan summary = %#v", output.ChunkPlan) + } + if len(output.Warnings) != 1 || output.Warnings[0].ReasonCode != "validator_execution_incomplete" { + t.Fatalf("warnings = %#v, want only generated incomplete warning", output.Warnings) + } + if !reflect.DeepEqual(store.record, record) { + t.Fatal("discarded incomplete candidates mutated the stored record") + } +} + +func TestRunnerFailsOnValidationIncompleteChunkPlanHitUnderFailRun(t *testing.T) { + prepared, plan := preparedTerminalDebugPipeline(t) + calls := 0 + prepared.chunker = terminalChunker{key: prepared.resolved.Chunk.Module, plan: plan, calls: &calls} + validator := &countingChunkValidator{err: errors.New("validator unavailable")} + prepared.chunkValidators.validators = []preparedValidator{{resolved: ResolvedValidator{Binding: Binding(validator.Name()), Target: ValidatorTargetChunk}, chunk: validator}} + store := &recordingChunkPlanStore{record: chunkPlanRecord(t, prepared, plan), decision: ChunkPlanDecision{Status: ChunkPlanHit}} + + output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), ChunkCacheMode: ChunkCacheAuto, ChunkPlans: store}) + if err == nil || !strings.Contains(err.Error(), "validator unavailable") { + t.Fatalf("Run() error = %v, want cached validation failure", err) + } + if calls != 0 || validator.calls != 1 || store.saves != 0 { + t.Fatalf("calls = producer %d validator %d saves %d, want 0 1 0", calls, validator.calls, store.saves) + } + if output.ChunkPlan == nil || output.ChunkPlan.ValidationStatus != "error" { + t.Fatalf("chunk plan summary = %#v", output.ChunkPlan) + } +} + func TestRunnerRetainsChunkMapAfterLaneRejection(t *testing.T) { prepared, _ := preparedTerminalDebugPipeline(t) encoder := &capturingChunkMapOutput{} @@ -358,7 +444,7 @@ func TestRunnerRetainsChunkMapAfterLaneRejection(t *testing.T) { prepared.Steps[0].lanes[0].mergeValidators.validators = []preparedValidator{{ resolved: ResolvedValidator{Binding: Binding("reject/lane"), Target: ValidatorTargetTyped, ArtifactKind: "test/notes"}, typedValidate: func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { - return contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "not accepted"}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "not accepted", CorrectionGuidance: "return an acceptable candidate"}, nil }, }} output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input")}) @@ -392,7 +478,7 @@ func TestRunnerChunkMapRequestDoesNotAliasStoredPlan(t *testing.T) { func TestRunnerChunkPlanManifestRetainsCandidateOnRejection(t *testing.T) { prepared, _ := preparedTerminalDebugPipeline(t) - validator := &countingChunkValidator{result: contracts.ValidationResult{Approved: false, ReasonCode: "policy", Message: "no"}} + validator := &countingChunkValidator{result: contracts.ValidationResult{Approved: false, ReasonCode: "policy", Message: "no", CorrectionGuidance: "return a policy-compliant chunk plan"}} prepared.chunkValidators.validators = []preparedValidator{{resolved: ResolvedValidator{Binding: Binding(validator.Name()), Target: ValidatorTargetChunk}, chunk: validator}} output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), ChunkCacheMode: ChunkCacheRefresh, ChunkPlans: &recordingChunkPlanStore{}}) if err != nil { @@ -508,7 +594,7 @@ func TestRunnerAutoHitValidatesOnceWithoutRegenerationOrMutation(t *testing.T) { wantReject bool }{ {name: "warning", result: contracts.ValidationResult{Approved: true, Warnings: []contracts.Warning{{Scope: "current", ReasonCode: "observed", Message: "current warning"}}}}, - {name: "rejection", result: contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "rejected hit"}, wantReject: true}, + {name: "rejection", result: contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "rejected hit", CorrectionGuidance: "return an acceptable chunk plan"}, wantReject: true}, {name: "error", validatorErr: errors.New("validator failed"), wantError: "validator failed"}, } for _, tc := range tests { @@ -573,7 +659,7 @@ func TestRunnerPublishesOnlyAcceptedGeneratedPlans(t *testing.T) { wantReject bool }{ {name: "module error", moduleErr: errors.New("generation failed"), wantCalls: 2}, - {name: "validator rejection", validator: contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "rejected generated plan"}, wantCalls: 1, wantReject: true}, + {name: "validator rejection", validator: contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "rejected generated plan", CorrectionGuidance: "return an acceptable chunk plan"}, wantCalls: 1, wantReject: true}, {name: "cancellation", cancel: true}, } { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/framework/pipeline/runner_concurrency_test.go b/internal/framework/pipeline/runner_concurrency_test.go index 9ba8ccd9..0f3aa247 100644 --- a/internal/framework/pipeline/runner_concurrency_test.go +++ b/internal/framework/pipeline/runner_concurrency_test.go @@ -568,7 +568,7 @@ func TestRunnerKeepsSuccessfulChunksWhenAnotherExtractIsRejected(t *testing.T) { validator := &prepared.Steps[0].lanes[0].extractValidators.validators[0] validator.typedValidate = func(_ context.Context, _ any, target typedValidationTarget) (contracts.ValidationResult, error) { if target.chunk != nil && target.chunk.Index == 0 { - return contracts.ValidationResult{Approved: false, ReasonCode: "expected_rejection", Message: "rejected by test"}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: "expected_rejection", Message: "rejected by test", CorrectionGuidance: "return an acceptable candidate"}, nil } return contracts.ValidationResult{Approved: true}, nil } diff --git a/internal/framework/pipeline/runner_concurrent.go b/internal/framework/pipeline/runner_concurrent.go index d9c50ab5..1eae3610 100644 --- a/internal/framework/pipeline/runner_concurrent.go +++ b/internal/framework/pipeline/runner_concurrent.go @@ -21,6 +21,7 @@ type laneExtractState struct { prepared preparedLaneExecutor deps []CheckpointFingerprint decision CheckpointDecision + reuseEligible bool values []erasedExtractArtifact serialized []CheckpointArtifact warnings []contracts.Warning @@ -42,6 +43,7 @@ type finalizedExtractResults struct { incomplete []int validationSummaries []artifacts.ValidationSummary decision CheckpointDecision + reuseEligible bool } func loadExtract(loader CheckpointLoader, stepID, laneID, moduleKey string, deps []CheckpointFingerprint) (ExtractCheckpoint, CheckpointDecision) { @@ -123,6 +125,9 @@ func initializeLaneStates(input RunInput, step PreparedPipelineStep, checkpoints return states, err } if input.CheckpointPolicy.requiresReusable(input.stepID, prepared.resolved.ID) && !input.CheckpointPolicy.forced(input.stepID, prepared.resolved.ID) { + if !laneReferencesReuseEligible(input, prepared.resolved) { + return states, fmt.Errorf("required reusable checkpoint unavailable for step %q lane %q: generated input lineage is validation-incomplete", input.stepID, prepared.resolved.ID) + } state, err := hydrateRequiredLane(input, loader, doc, i, prepared) states[i] = state if err != nil { @@ -134,7 +139,7 @@ func initializeLaneStates(input RunInput, step PreparedPipelineStep, checkpoints if err != nil { return states, err } - if !state.decision.Reused { + if !state.decision.Reused && state.reuseEligible { if err := checkpointExtractRunning(checkpoints, input.stepID, prepared.resolved.ID, prepared.resolved.Extract.Module, state.deps); err != nil { return states, fmt.Errorf("write extract checkpoint for lane %q: %w", prepared.resolved.ID, err) } @@ -303,7 +308,9 @@ func (e *laneEngine) handleExtractResult(result extractJobResult) { if result.err != nil { state.failed = true e.runErrors = append(e.runErrors, orderedRunError{stage: 0, lane: state.index, chunk: result.chunkIndex, err: result.err}) - _ = checkpointExtractFailed(e.checkpoints, e.input.stepID, state.prepared.resolved.ID, state.prepared.resolved.Extract.Module, state.deps, result.err) + if state.reuseEligible { + _ = checkpointExtractFailed(e.checkpoints, e.input.stepID, state.prepared.resolved.ID, state.prepared.resolved.Extract.Module, state.deps, result.err) + } e.cancel() } else { state.results[result.chunkIndex] = result @@ -341,7 +348,7 @@ func hydrateRequiredLane(input RunInput, loader CheckpointLoader, doc *source.So } resolution, err := resolveCheckpointDecision(&local, loader, input.CheckpointPolicy, StageNormalize, input.stepID, lane.ID, lane.Normalize.Module, decision, typed.codec, []CheckpointArtifact{checkpoint.Output}) decision = resolution.decision - state := &laneExtractState{index: index, prepared: prepared, decision: decision, terminal: true, output: local} + state := &laneExtractState{index: index, prepared: prepared, decision: decision, reuseEligible: true, terminal: true, output: local} if err != nil { return state, err } @@ -354,6 +361,7 @@ func hydrateRequiredLane(input RunInput, loader CheckpointLoader, doc *source.So SourceID: doc.ID, Artifact: contracts.CloneSerializedArtifact(hydrated.Artifact), }) + local.normalizeReuseEligibility = map[generatedOutputKey]bool{generatedOutputKeyFor(input.stepID, lane.ID): true} state.output = local return state, nil } @@ -387,7 +395,12 @@ func prepareLaneExtract(input RunInput, loader CheckpointLoader, doc *source.Sou } extractReferences := operationReferenceSet(input, lane.ExtractReferences) deps := append(digestFingerprints("chunks", digest), generatedReferenceDependencies(extractReferences)...) - state := &laneExtractState{index: index, prepared: prepared, deps: normalizeCheckpointFingerprints(deps), remaining: len(chunks), results: make(map[int]extractJobResult, len(chunks))} + state := &laneExtractState{index: index, prepared: prepared, deps: normalizeCheckpointFingerprints(deps), reuseEligible: referenceTargetReuseEligible(input, lane.ExtractReferences), remaining: len(chunks), results: make(map[int]extractJobResult, len(chunks))} + if !state.reuseEligible { + state.decision = checkpointDecision(CheckpointDecisionExecuted, CheckpointReasonValidationIncompleteLineage) + recordCheckpointEvent(output, loader, string(StageExtract), input.stepID, lane.ID, lane.Extract.Module, state.decision) + return state, nil + } cp, decision := loadExtract(loader, input.stepID, lane.ID, lane.Extract.Module, state.deps) resolution, err := resolveCheckpointDecision(output, loader, input.CheckpointPolicy, StageExtract, input.stepID, lane.ID, lane.Extract.Module, decision, typed.codec, cp.Outputs) if err != nil { @@ -460,14 +473,6 @@ func (r *Runner) runExtractJob(ctx context.Context, input RunInput, doc *source. if validationErr != nil { return report, candidate.terminal.record(payload, validationErr) } - if report.FirstRejection() != nil { - return report, candidate.terminal.record(payload, nil) - } - if report.FirstRejection() == nil && lane.ExtractValidationPolicy.ValidatorFailure == ValidatorFailureFailRun { - if failure := firstIncompleteValidation(report); failure != nil { - return report, candidate.terminal.record(payload, validatorFailureError(*failure)) - } - } return report, candidate.terminal.record(payload, nil) }) result.err = err @@ -540,7 +545,10 @@ func finalizeLaneExtract(checkpoints CheckpointRecorder, stepID string, state *l sort.SliceStable(state.serialized, func(i, j int) bool { return state.serialized[i].ChunkIndex < state.serialized[j].ChunkIndex }) sort.SliceStable(state.rejected, func(i, j int) bool { return state.rejected[i].ChunkIndex < state.rejected[j].ChunkIndex }) sort.Ints(state.incomplete) - if !state.decision.Reused && len(state.incomplete) == 0 { + if len(state.incomplete) > 0 { + state.reuseEligible = false + } + if !state.decision.Reused && state.reuseEligible { if err := recordExtract(checkpoints, stepID, lane.ID, lane.Extract.Module, state.deps, state.serialized, state.rejected, state.warnings); err != nil { return fmt.Errorf("write extract checkpoint for lane %q: %w", lane.ID, err) } @@ -559,6 +567,7 @@ func (r *Runner) continueLane(ctx context.Context, input RunInput, checkpoints C incomplete: state.incomplete, validationSummaries: state.validationSummaries, decision: state.decision, + reuseEligible: state.reuseEligible, } local.Warnings = append(local.Warnings, cloneWarnings(results.warnings)...) local.Rejected = append(local.Rejected, cloneRejectedOutputs(results.rejected)...) @@ -634,6 +643,14 @@ func mergeLaneOutput(dst *RunOutput, src RunOutput) error { dst.Warnings = append(dst.Warnings, cloneWarnings(src.Warnings)...) dst.CheckpointEvents = append(dst.CheckpointEvents, src.CheckpointEvents...) dst.ValidationSummaries = append(dst.ValidationSummaries, cloneValidationSummaries(src.ValidationSummaries)...) + if len(src.normalizeReuseEligibility) > 0 { + if dst.normalizeReuseEligibility == nil { + dst.normalizeReuseEligibility = make(map[generatedOutputKey]bool, len(src.normalizeReuseEligibility)) + } + for key, eligible := range src.normalizeReuseEligibility { + dst.normalizeReuseEligibility[key] = eligible + } + } for i := range dst.Manifest.ArtifactLanes { for j := range src.Manifest.ArtifactLanes { if dst.Manifest.ArtifactLanes[i].ID == src.Manifest.ArtifactLanes[j].ID && dst.Manifest.ArtifactLanes[i].StepID == src.Manifest.ArtifactLanes[j].StepID && src.Manifest.ArtifactLanes[j].Metadata != nil { diff --git a/internal/framework/pipeline/runner_extract_correction_test.go b/internal/framework/pipeline/runner_extract_correction_test.go index f3f5904d..1778005d 100644 --- a/internal/framework/pipeline/runner_extract_correction_test.go +++ b/internal/framework/pipeline/runner_extract_correction_test.go @@ -65,8 +65,8 @@ func TestRunnerCorrectsExtractCandidatesIndependentlyPerChunk(t *testing.T) { } for index := 0; index < 2; index++ { correction := corrections[index] - if correction == nil || string(correction.AssistantResponse) != fmt.Sprintf("initial-response-%d", index) || correction.UserGuidance != fmt.Sprintf("correct chunk %d", index) { - t.Fatalf("chunk %d correction = %#v, want its exact initial response and guidance", index, correction) + if correction == nil || string(correction.AssistantResponse) != fmt.Sprintf("initial-response-%d", index) || !strings.Contains(correction.UserGuidance, fmt.Sprintf("correct chunk %d", index)) || !strings.Contains(correction.UserGuidance, "complete corrected replacement") || strings.Contains(correction.UserGuidance, "incorrect_extract") || strings.Contains(correction.UserGuidance, "candidate needs correction") { + t.Fatalf("chunk %d correction = %#v, want its exact initial response and semantic replacement guidance only", index, correction) } } terminal := string(debug.json["extract/notes/chunk-000001/terminal.json"]) diff --git a/internal/framework/pipeline/runner_extract_handoff_test.go b/internal/framework/pipeline/runner_extract_handoff_test.go index 0fe750d0..92be174e 100644 --- a/internal/framework/pipeline/runner_extract_handoff_test.go +++ b/internal/framework/pipeline/runner_extract_handoff_test.go @@ -230,7 +230,7 @@ func TestRunnerPromotesOnlyAcceptedExtractRetryWarnings(t *testing.T) { validatorCalls := 0 prepared.Steps[0].lanes[0].extractValidators.validators[0].typedValidate = func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { validatorCalls++ - return contracts.ValidationResult{Approved: validatorCalls == 2, ReasonCode: "retry", Message: "retry extract"}, nil + return contracts.ValidationResult{Approved: validatorCalls == 2, ReasonCode: "retry", Message: "retry extract", CorrectionGuidance: "return an acceptable extract"}, nil } debug := newCapturedDebugRecorder() output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Debug: debug}) diff --git a/internal/framework/pipeline/runner_normalize_retry_test.go b/internal/framework/pipeline/runner_normalize_retry_test.go index c334ab78..a88b172a 100644 --- a/internal/framework/pipeline/runner_normalize_retry_test.go +++ b/internal/framework/pipeline/runner_normalize_retry_test.go @@ -70,7 +70,7 @@ func TestRunnerHandlesRetryableNormalizeFallbacks(t *testing.T) { validator: &preparedValidator{ resolved: ResolvedValidator{Binding: Binding("reject-final-fallback"), Target: ValidatorTargetTyped, ArtifactKind: "test/notes"}, typedValidate: func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { - return contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "reject fallback"}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "reject fallback", CorrectionGuidance: "return an acceptable normalized artifact"}, nil }, }, wantCalls: 2, diff --git a/internal/framework/pipeline/runner_rejection_warnings_test.go b/internal/framework/pipeline/runner_rejection_warnings_test.go index 46d40d28..8aab1e68 100644 --- a/internal/framework/pipeline/runner_rejection_warnings_test.go +++ b/internal/framework/pipeline/runner_rejection_warnings_test.go @@ -50,7 +50,7 @@ func TestRunnerPromotesOnlyTerminalRejectionWarnings(t *testing.T) { return contracts.ValidationResult{Approved: true, Warnings: []contracts.Warning{{Scope: fmt.Sprintf("validator-%d", attempts), ReasonCode: "validator", Message: "validator warning"}}} } reject := func() contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "rejected"} + return contracts.ValidationResult{Approved: false, ReasonCode: "rejected", Message: "rejected", CorrectionGuidance: "return an acceptable candidate"} } debug := newCapturedDebugRecorder() recorder := &extractCaptureRecorder{CheckpointRecorder: NoopCheckpointRecorder()} diff --git a/internal/framework/pipeline/runner_reuse_lineage_test.go b/internal/framework/pipeline/runner_reuse_lineage_test.go new file mode 100644 index 00000000..c071c9ed --- /dev/null +++ b/internal/framework/pipeline/runner_reuse_lineage_test.go @@ -0,0 +1,238 @@ +package pipeline + +import ( + "context" + "fmt" + "strings" + "testing" + + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" +) + +type reuseLineageCheckpointSpy struct { + CheckpointLoader + CheckpointRecorder + loads map[string]int + writes map[string]int + forbid map[string]struct{} +} + +func newReuseLineageCheckpointSpy() *reuseLineageCheckpointSpy { + return &reuseLineageCheckpointSpy{ + CheckpointLoader: NoopCheckpointLoader(), + CheckpointRecorder: NoopCheckpointRecorder(), + loads: make(map[string]int), + writes: make(map[string]int), + forbid: make(map[string]struct{}), + } +} + +func (s *reuseLineageCheckpointSpy) Enabled() bool { return true } + +func (s *reuseLineageCheckpointSpy) load(stage, laneID string) { + key := stage + "/" + laneID + if _, forbidden := s.forbid[key]; forbidden { + panic("checkpoint lookup crossed validation-incomplete lineage: " + key) + } + s.loads[key]++ +} + +func (s *reuseLineageCheckpointSpy) write(stage, action, laneID string) { + s.writes[stage+"/"+action+"/"+laneID]++ +} + +func (s *reuseLineageCheckpointSpy) Extract(laneID, _ string, _ []CheckpointFingerprint) (ExtractCheckpoint, CheckpointDecision) { + s.load("extract", laneID) + return ExtractCheckpoint{}, NewCheckpointDecision(CheckpointDecisionExecuted, CheckpointReasonMissing) +} + +func (s *reuseLineageCheckpointSpy) Merge(laneID, _ string, _ []CheckpointFingerprint) (MergeCheckpoint, CheckpointDecision) { + s.load("merge", laneID) + return MergeCheckpoint{}, NewCheckpointDecision(CheckpointDecisionExecuted, CheckpointReasonMissing) +} + +func (s *reuseLineageCheckpointSpy) Normalize(laneID, _ string, _ []CheckpointFingerprint) (NormalizeCheckpoint, CheckpointDecision) { + s.load("normalize", laneID) + return NormalizeCheckpoint{}, NewCheckpointDecision(CheckpointDecisionExecuted, CheckpointReasonMissing) +} + +func (s *reuseLineageCheckpointSpy) ExtractRunning(laneID, _ string, _ []CheckpointFingerprint) error { + s.write("extract", "running", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) ExtractSucceeded(laneID, _ string, _ []CheckpointFingerprint, _ []CheckpointArtifact, _ []contracts.RejectedOutput, _ []contracts.Warning) error { + s.write("extract", "succeeded", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) ExtractFailed(laneID, _ string, _ []CheckpointFingerprint, _ error) error { + s.write("extract", "failed", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) MergeRunning(laneID, _ string, _ []CheckpointFingerprint) error { + s.write("merge", "running", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) MergeSucceeded(laneID, _ string, _ []CheckpointFingerprint, _ CheckpointArtifact, _ []contracts.Warning) error { + s.write("merge", "succeeded", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) MergeRejected(laneID, _ string, _ []CheckpointFingerprint, _ contracts.RejectedOutput) error { + s.write("merge", "rejected", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) MergeFailed(laneID, _ string, _ []CheckpointFingerprint, _ error) error { + s.write("merge", "failed", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) NormalizeRunning(laneID, _ string, _ []CheckpointFingerprint) error { + s.write("normalize", "running", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) NormalizeSucceeded(laneID, _ string, _ []CheckpointFingerprint, _ CheckpointArtifact, _ []contracts.Warning) error { + s.write("normalize", "succeeded", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) NormalizeRejected(laneID, _ string, _ []CheckpointFingerprint, _ contracts.RejectedOutput) error { + s.write("normalize", "rejected", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) NormalizeFailed(laneID, _ string, _ []CheckpointFingerprint, _ error) error { + s.write("normalize", "failed", laneID) + return nil +} + +func (s *reuseLineageCheckpointSpy) writesFor(stage, laneID string) int { + total := 0 + needle := stage + "/" + suffix := "/" + laneID + for key, count := range s.writes { + if strings.HasPrefix(key, needle) && strings.HasSuffix(key, suffix) { + total += count + } + } + return total +} + +func unavailableTypedValidator(kind contracts.ArtifactKind) preparedValidator { + return preparedValidator{ + resolved: ResolvedValidator{Binding: Binding("typed/check"), Target: ValidatorTargetTyped, ArtifactKind: kind}, + typedValidate: func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { + return contracts.ValidationResult{}, fmt.Errorf("validator unavailable") + }, + } +} + +func TestValidationIncompleteExtractDisablesDownstreamCheckpointIO(t *testing.T) { + prepared := preparedAttemptDebugPipeline(t) + lane := &prepared.Steps[0].lanes[0] + lane.resolved.ExtractValidationPolicy.ValidatorFailure = ValidatorFailureWarnContinue + installExtractOperation(prepared, 0, func(_ context.Context, request contracts.TypedExtractionRequest) (erasedTypedResult, error) { + return erasedTypedResult{Value: typedValueForLane(0, request.Chunk.Index)}, nil + }) + lane.extractValidators.validators[0].typedValidate = func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { + return contracts.ValidationResult{}, fmt.Errorf("validator unavailable") + } + spy := newReuseLineageCheckpointSpy() + + output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Checkpoint: spy, Checkpoints: spy}) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if len(output.NormalizeOutputs) != 1 { + t.Fatalf("normalize outputs = %d, want current-run handoff", len(output.NormalizeOutputs)) + } + if spy.loads["merge/notes"] != 0 || spy.loads["normalize/notes"] != 0 { + t.Fatalf("downstream loads = %#v, want none", spy.loads) + } + if spy.writesFor("merge", "notes") != 0 || spy.writesFor("normalize", "notes") != 0 { + t.Fatalf("downstream writes = %#v, want none", spy.writes) + } + if spy.writes["extract/succeeded/notes"] != 0 { + t.Fatalf("extract writes = %#v, want no reusable success", spy.writes) + } +} + +func TestValidationIncompleteMergeDisablesNormalizeCheckpointIO(t *testing.T) { + prepared := preparedAttemptDebugPipeline(t) + lane := &prepared.Steps[0].lanes[0] + lane.resolved.MergeValidationPolicy.ValidatorFailure = ValidatorFailureWarnContinue + installExtractOperation(prepared, 0, func(_ context.Context, request contracts.TypedExtractionRequest) (erasedTypedResult, error) { + return erasedTypedResult{Value: typedValueForLane(0, request.Chunk.Index)}, nil + }) + lane.mergeValidators.validators = []preparedValidator{unavailableTypedValidator(lane.resolved.ArtifactKind)} + spy := newReuseLineageCheckpointSpy() + + output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Checkpoint: spy, Checkpoints: spy}) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if len(output.NormalizeOutputs) != 1 { + t.Fatalf("normalize outputs = %d, want current-run handoff", len(output.NormalizeOutputs)) + } + if spy.loads["merge/notes"] != 1 || spy.loads["normalize/notes"] != 0 { + t.Fatalf("loads = %#v, want merge lookup only", spy.loads) + } + if spy.writes["merge/succeeded/notes"] != 0 || spy.writesFor("normalize", "notes") != 0 { + t.Fatalf("writes = %#v, want no reusable merge or normalize state", spy.writes) + } +} + +func TestValidationIncompleteNormalizeIsNotPublished(t *testing.T) { + prepared := preparedAttemptDebugPipeline(t) + lane := &prepared.Steps[0].lanes[0] + lane.resolved.NormalizeValidationPolicy.ValidatorFailure = ValidatorFailureWarnContinue + installExtractOperation(prepared, 0, func(_ context.Context, request contracts.TypedExtractionRequest) (erasedTypedResult, error) { + return erasedTypedResult{Value: typedValueForLane(0, request.Chunk.Index)}, nil + }) + lane.normalizeValidators.validators = []preparedValidator{unavailableTypedValidator(lane.resolved.ArtifactKind)} + spy := newReuseLineageCheckpointSpy() + + output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Checkpoint: spy, Checkpoints: spy}) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if len(output.NormalizeOutputs) != 1 || spy.writes["normalize/succeeded/notes"] != 0 { + t.Fatalf("output/writes = %d / %#v, want in-memory output without normalize publication", len(output.NormalizeOutputs), spy.writes) + } +} + +func TestGeneratedReferenceFromIncompleteValidationDisablesDependentCheckpointIO(t *testing.T) { + input, _, _ := handoffFixture(t, codecNotes{Items: []string{"producer"}}) + prepared := input.Prepared + producer := &prepared.Steps[0].lanes[0] + consumer := &prepared.Steps[1].lanes[0] + producer.resolved.NormalizeValidationPolicy.ValidatorFailure = ValidatorFailureWarnContinue + producer.normalizeValidators.validators = []preparedValidator{unavailableTypedValidator(producer.resolved.ArtifactKind)} + referenceItems := 0 + consumer.typed.extract = func(_ context.Context, _ any, request contracts.TypedExtractionRequest) (erasedTypedResult, error) { + referenceItems = len(request.References.Slots["producer-output"].Items) + return erasedTypedResult{Value: codecScore{Value: 1}}, nil + } + spy := newReuseLineageCheckpointSpy() + for _, stage := range []string{"extract", "merge", "normalize"} { + spy.forbid[stage+"/score"] = struct{}{} + } + + output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Checkpoint: spy, Checkpoints: spy}) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if referenceItems != 1 || len(output.NormalizeOutputs) != 2 { + t.Fatalf("current-run handoff = items %d outputs %d, want one generated item and two outputs", referenceItems, len(output.NormalizeOutputs)) + } + for _, stage := range []string{"extract", "merge", "normalize"} { + if spy.writesFor(stage, "score") != 0 { + t.Fatalf("dependent writes = %#v, want none for %s", spy.writes, stage) + } + } +} diff --git a/internal/framework/pipeline/runner_semantic_correction_test.go b/internal/framework/pipeline/runner_semantic_correction_test.go index 4ff5105e..9d5ae186 100644 --- a/internal/framework/pipeline/runner_semantic_correction_test.go +++ b/internal/framework/pipeline/runner_semantic_correction_test.go @@ -3,6 +3,7 @@ package pipeline import ( "context" "fmt" + "strings" "testing" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" @@ -61,8 +62,8 @@ func TestRunnerCorrectsRejectedMergeAndNormalizeCandidates(t *testing.T) { if err != nil { t.Fatalf("Run() error = %v, want nil", err) } - if correction == nil || string(correction.AssistantResponse) != `{"items":["invalid"]}` || correction.UserGuidance != "produce the accepted value" { - t.Fatalf("%s correction = %#v, want exact rejected response and guidance", target, correction) + if correction == nil || string(correction.AssistantResponse) != `{"items":["invalid"]}` || !strings.Contains(correction.UserGuidance, "produce the accepted value") || !strings.Contains(correction.UserGuidance, "complete corrected replacement") || strings.Contains(correction.UserGuidance, "invalid_candidate") || strings.Contains(correction.UserGuidance, "candidate needs correction") { + t.Fatalf("%s correction = %#v, want exact rejected response and semantic replacement guidance only", target, correction) } if len(output.Rejected) != 0 || len(output.NormalizeOutputs) != 1 { t.Fatalf("run output = %#v, want corrected accepted output", output) @@ -84,7 +85,7 @@ func TestRunnerRejectsDeterministicMergeCandidateWithoutCorrection(t *testing.T) lane.mergeValidators.validators = []preparedValidator{{ resolved: ResolvedValidator{Binding: Binding("reject"), Target: ValidatorTargetTyped, ArtifactKind: "test/notes"}, typedValidate: func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { - return contracts.ValidationResult{Approved: false, ReasonCode: "invalid_candidate", Message: "not accepted"}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: "invalid_candidate", Message: "not accepted", CorrectionGuidance: "return an acceptable candidate"}, nil }, }} diff --git a/internal/framework/pipeline/runner_terminal_debug_test.go b/internal/framework/pipeline/runner_terminal_debug_test.go index f754f1d8..52440a2e 100644 --- a/internal/framework/pipeline/runner_terminal_debug_test.go +++ b/internal/framework/pipeline/runner_terminal_debug_test.go @@ -157,10 +157,11 @@ func TestRunnerRecordsChunkTerminalOutcomes(t *testing.T) { validator terminalChunkValidator wantError string wantRejection bool + attemptError bool }{ {name: "accepted", validator: terminalChunkValidator{result: contracts.ValidationResult{Approved: true}}}, - {name: "module error", moduleError: errors.New("chunk module failed"), wantError: "chunk module failed"}, - {name: "validator rejection", validator: terminalChunkValidator{result: contracts.ValidationResult{Approved: false, ReasonCode: "chunk_rejected", Message: "chunk rejected"}}, wantRejection: true}, + {name: "module error", moduleError: errors.New("chunk module failed"), wantError: "chunk module failed", attemptError: true}, + {name: "validator rejection", validator: terminalChunkValidator{result: contracts.ValidationResult{Approved: false, ReasonCode: "chunk_rejected", Message: "chunk rejected", CorrectionGuidance: "return an acceptable chunk plan"}}, wantRejection: true}, {name: "validator error", validator: terminalChunkValidator{err: errors.New("chunk validator failed")}, wantError: "chunk validator failed"}, } for _, tc := range tests { @@ -174,9 +175,15 @@ func TestRunnerRecordsChunkTerminalOutcomes(t *testing.T) { assertAttemptEnvelopeSequence(t, debug, "chunk", 1) envelope := debug.envelope(t, "chunk/attempt-01.json") if tc.wantError != "" { - if err == nil || !strings.Contains(err.Error(), tc.wantError) || !strings.Contains(envelope.Error, tc.wantError) { + if err == nil || !strings.Contains(err.Error(), tc.wantError) { t.Fatalf("Run() error = %v, attempt error = %q; want %q", err, envelope.Error, tc.wantError) } + if tc.attemptError && !strings.Contains(envelope.Error, tc.wantError) { + t.Fatalf("attempt error = %q, want %q", envelope.Error, tc.wantError) + } + if !tc.attemptError && envelope.Error != "" { + t.Fatalf("settled validator failure attempt error = %q, want empty", envelope.Error) + } } else if err != nil { t.Fatalf("Run() error = %v, want nil", err) } @@ -300,12 +307,13 @@ func TestRunnerRecordsExtractTerminalOutcomes(t *testing.T) { candidateFail bool finalFail bool wantError string + attemptError bool }{ {name: "terminal rejection", reject: true}, - {name: "module error", moduleError: errors.New("extract module failed"), wantError: "extract module failed"}, + {name: "module error", moduleError: errors.New("extract module failed"), wantError: "extract module failed", attemptError: true}, {name: "validator error", validatorErr: errors.New("extract validator failed"), wantError: "extract validator failed"}, - {name: "candidate codec error", candidateFail: true, wantError: "serialize extract candidate"}, - {name: "final codec error", finalFail: true, wantError: "serialize accepted extract output"}, + {name: "candidate codec error", candidateFail: true, wantError: "serialize extract candidate", attemptError: true}, + {name: "final codec error", finalFail: true, wantError: "serialize accepted extract output", attemptError: true}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -333,7 +341,7 @@ func TestRunnerRecordsExtractTerminalOutcomes(t *testing.T) { prepared.Steps[0].lanes[0].extractValidators.validators = []preparedValidator{{ resolved: ResolvedValidator{Binding: Binding("terminal/extract-validator"), Target: ValidatorTargetTyped, ArtifactKind: "test/notes"}, typedValidate: func(context.Context, any, typedValidationTarget) (contracts.ValidationResult, error) { - return contracts.ValidationResult{Approved: !tc.reject, ReasonCode: "extract_rejected", Message: "extract rejected"}, tc.validatorErr + return contracts.ValidationResult{Approved: !tc.reject, ReasonCode: "extract_rejected", Message: "extract rejected", CorrectionGuidance: "return an acceptable extract"}, tc.validatorErr }, }} debug := newCapturedDebugRecorder() @@ -343,9 +351,15 @@ func TestRunnerRecordsExtractTerminalOutcomes(t *testing.T) { attemptPath := "extract/notes/chunk-000001/attempt-01.json" envelope := debug.envelope(t, attemptPath) if tc.wantError != "" { - if err == nil || !strings.Contains(err.Error(), tc.wantError) || !strings.Contains(envelope.Error, tc.wantError) { + if err == nil || !strings.Contains(err.Error(), tc.wantError) { t.Fatalf("Run() error = %v, attempt error = %q; want %q", err, envelope.Error, tc.wantError) } + if tc.attemptError && !strings.Contains(envelope.Error, tc.wantError) { + t.Fatalf("attempt error = %q, want %q", envelope.Error, tc.wantError) + } + if !tc.attemptError && envelope.Error != "" { + t.Fatalf("settled validator failure attempt error = %q, want empty", envelope.Error) + } } else { if err != nil { t.Fatalf("Run() error = %v, want nil rejection", err) diff --git a/internal/framework/pipeline/runner_typed.go b/internal/framework/pipeline/runner_typed.go index c488b90a..376a7693 100644 --- a/internal/framework/pipeline/runner_typed.go +++ b/internal/framework/pipeline/runner_typed.go @@ -193,6 +193,7 @@ type mergeStageResult struct { serialized CheckpointArtifact terminal bool validationIncomplete bool + reuseEligible bool } type mergeAttemptValue struct { @@ -223,12 +224,16 @@ func (r *Runner) continueTypedLane(ctx context.Context, input RunInput, checkpoi if merged.terminal { return nil } - normalized, err := r.runNormalizeStage(ctx, input, checkpoints, loader, doc, sourceInput, sessionID, prepared, merged.artifact, merged.serialized, output) + normalized, err := r.runNormalizeStage(ctx, input, checkpoints, loader, doc, sourceInput, sessionID, prepared, merged.artifact, merged.serialized, merged.reuseEligible, output) if err != nil { return &laneRunError{stage: StageNormalize, err: err} } if normalized.accepted { output.NormalizeOutputs = append(output.NormalizeOutputs, contracts.SerializedOutput{StepID: input.stepID, LaneID: lane.ID, NormalizerKey: lane.Normalize.Module, SourceID: doc.ID, Artifact: contracts.CloneSerializedArtifact(normalized.serialized.Artifact)}) + if output.normalizeReuseEligibility == nil { + output.normalizeReuseEligibility = make(map[generatedOutputKey]bool) + } + output.normalizeReuseEligibility[generatedOutputKeyFor(input.stepID, lane.ID)] = normalized.reuseEligible } return nil } @@ -241,9 +246,14 @@ func (r *Runner) runMergeStage(ctx context.Context, input RunInput, checkpoints mergeInputs[i] = contracts.ExtractArtifact[any]{LaneID: value.LaneID, ExtractorKey: value.ExtractorKey, SourceID: value.SourceID, ChunkID: value.ChunkID, ChunkIndex: value.ChunkIndex, ChunkRef: value.ChunkRef, Value: value.Value} } mergeReferences := operationReferenceSet(input, lane.MergeReferences) + stageResult.reuseEligible = extracts.reuseEligible && referenceTargetReuseEligible(input, lane.MergeReferences) mergeDeps := append(artifactCheckpointDigests(extracts.serialized), generatedReferenceDependencies(mergeReferences)...) mergeDeps = normalizeCheckpointFingerprints(mergeDeps) - mergeCP, mergeDecision := loadMerge(loader, input.stepID, lane.ID, lane.Merge.Module, mergeDeps) + var mergeCP MergeCheckpoint + mergeDecision := checkpointDecision(CheckpointDecisionExecuted, CheckpointReasonValidationIncompleteLineage) + if stageResult.reuseEligible { + mergeCP, mergeDecision = loadMerge(loader, input.stepID, lane.ID, lane.Merge.Module, mergeDeps) + } mergeResolution, err := resolveCheckpointDecision(output, loader, input.CheckpointPolicy, StageMerge, input.stepID, lane.ID, lane.Merge.Module, mergeDecision, typed.codec, []CheckpointArtifact{mergeCP.Output}) if err != nil { return stageResult, err @@ -261,8 +271,10 @@ func (r *Runner) runMergeStage(ctx context.Context, input RunInput, checkpoints mergeWarnings = cloneWarnings(mergeCP.Warnings) output.Warnings = append(output.Warnings, mergeWarnings...) } else { - if err := checkpointMergeRunning(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps); err != nil { - return stageResult, err + if stageResult.reuseEligible { + if err := checkpointMergeRunning(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps); err != nil { + return stageResult, err + } } terminalResult, runErr := runProducerAttempts(ctx, producerAttemptConfig{Retries: lane.Merge.Retries, Policy: lane.MergeValidationPolicy, AllowStructuralRetry: lane.MergeExecutionClass == contracts.ExecutionClassLLMBacked}, func(attemptCtx context.Context, request producerAttemptRequest) (producerAttemptOutput, error) { started := time.Now().UTC() @@ -295,23 +307,19 @@ func (r *Runner) runMergeStage(ctx context.Context, input RunInput, checkpoints if validationErr != nil { return report, candidate.terminal.record(payload, validationErr) } - if report.FirstRejection() != nil { - return report, candidate.terminal.record(payload, nil) - } - if lane.MergeValidationPolicy.ValidatorFailure == ValidatorFailureFailRun { - if failure := firstIncompleteValidation(report); failure != nil { - return report, candidate.terminal.record(payload, validatorFailureError(*failure)) - } - } - return report, nil + return report, candidate.terminal.record(payload, nil) }) terminalSummary := validationSummary(terminalResult, StageMerge, input.stepID, lane.ID, lane.Merge.Module, "", 0) if debugErr := writeProducerTerminalDebug(input.Debug, path.Join("merge", fileio.EncodePathComponent(lane.ID), "terminal.json"), terminalResult, lane.MergeValidationPolicy, terminalSummary); debugErr != nil { - _ = checkpointMergeFailed(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, debugErr) + if stageResult.reuseEligible { + _ = checkpointMergeFailed(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, debugErr) + } return stageResult, debugErr } if runErr != nil { - _ = checkpointMergeFailed(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, runErr) + if stageResult.reuseEligible { + _ = checkpointMergeFailed(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, runErr) + } return stageResult, runErr } if terminalResult.Action == producerTerminalRejected { @@ -324,8 +332,10 @@ func (r *Runner) runMergeStage(ctx context.Context, input RunInput, checkpoints output.ValidationSummaries = append(output.ValidationSummaries, artifacts.CloneValidationSummary(terminalSummary)) output.Warnings = append(output.Warnings, terminalResult.Warnings...) output.Rejected = append(output.Rejected, *rejected) - if err := checkpointMergeRejected(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, *rejected); err != nil { - return stageResult, err + if stageResult.reuseEligible { + if err := checkpointMergeRejected(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, *rejected); err != nil { + return stageResult, err + } } stageResult.terminal = true return stageResult, nil @@ -338,19 +348,26 @@ func (r *Runner) runMergeStage(ctx context.Context, input RunInput, checkpoints payload := map[string]any{"output": debugCheckpointArtifact(candidate.candidate), "warnings": debugWarningEnvelopes(terminalResult.Warnings), "rejection": debugRejectedOutputPtr(nil)} if encodeErr != nil { attemptErr := fmt.Errorf("serialize accepted merge output for lane %q: %w", lane.ID, encodeErr) - _ = checkpointMergeFailed(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, attemptErr) + if stageResult.reuseEligible { + _ = checkpointMergeFailed(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, attemptErr) + } return stageResult, candidate.terminal.record(payload, attemptErr) } if debugErr := candidate.terminal.record(payload, nil); debugErr != nil { - _ = checkpointMergeFailed(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, debugErr) + if stageResult.reuseEligible { + _ = checkpointMergeFailed(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, debugErr) + } return stageResult, debugErr } merged, serializedMerge = candidate.artifact, stored mergeWarnings = cloneWarnings(terminalResult.Warnings) output.ValidationSummaries = append(output.ValidationSummaries, terminalSummary) stageResult.validationIncomplete = terminalResult.ValidationIncomplete + if stageResult.validationIncomplete { + stageResult.reuseEligible = false + } output.Warnings = append(output.Warnings, mergeWarnings...) - if !stageResult.validationIncomplete { + if stageResult.reuseEligible { if err := recordMerge(checkpoints, input.stepID, lane.ID, lane.Merge.Module, mergeDeps, serializedMerge, mergeWarnings); err != nil { return stageResult, err } @@ -365,18 +382,24 @@ func (r *Runner) runMergeStage(ctx context.Context, input RunInput, checkpoints } type normalizeStageResult struct { - serialized CheckpointArtifact - warnings []contracts.Warning - accepted bool + serialized CheckpointArtifact + warnings []contracts.Warning + accepted bool + reuseEligible bool } -func (r *Runner) runNormalizeStage(ctx context.Context, input RunInput, checkpoints CheckpointRecorder, loader CheckpointLoader, doc *source.SourceDocument, sourceInput contracts.LLMInputMaterial, sessionID string, prepared preparedLaneExecutor, merged erasedMergeArtifact, serializedMerge CheckpointArtifact, output *RunOutput) (normalizeStageResult, error) { +func (r *Runner) runNormalizeStage(ctx context.Context, input RunInput, checkpoints CheckpointRecorder, loader CheckpointLoader, doc *source.SourceDocument, sourceInput contracts.LLMInputMaterial, sessionID string, prepared preparedLaneExecutor, merged erasedMergeArtifact, serializedMerge CheckpointArtifact, upstreamReuseEligible bool, output *RunOutput) (normalizeStageResult, error) { var stageResult normalizeStageResult lane, typed := prepared.resolved, prepared.typed normalizeReferences := operationReferenceSet(input, lane.NormalizeReferences) + stageResult.reuseEligible = upstreamReuseEligible && referenceTargetReuseEligible(input, lane.NormalizeReferences) normalizeDeps := append(artifactCheckpointDigests([]CheckpointArtifact{serializedMerge}), generatedReferenceDependencies(normalizeReferences)...) normalizeDeps = normalizeCheckpointFingerprints(normalizeDeps) - normalizeCP, normalizeDecision := loadNormalize(loader, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps) + var normalizeCP NormalizeCheckpoint + normalizeDecision := checkpointDecision(CheckpointDecisionExecuted, CheckpointReasonValidationIncompleteLineage) + if stageResult.reuseEligible { + normalizeCP, normalizeDecision = loadNormalize(loader, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps) + } normalizeResolution, err := resolveCheckpointDecision(output, loader, input.CheckpointPolicy, StageNormalize, input.stepID, lane.ID, lane.Normalize.Module, normalizeDecision, typed.codec, []CheckpointArtifact{normalizeCP.Output}) if err != nil { return stageResult, err @@ -392,8 +415,10 @@ func (r *Runner) runNormalizeStage(ctx context.Context, input RunInput, checkpoi normalizeWarnings = cloneWarnings(normalizeCP.Warnings) output.Warnings = append(output.Warnings, normalizeWarnings...) } else { - if err := checkpointNormalizeRunning(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps); err != nil { - return stageResult, err + if stageResult.reuseEligible { + if err := checkpointNormalizeRunning(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps); err != nil { + return stageResult, err + } } terminalResult, runErr := runProducerAttempts(ctx, producerAttemptConfig{Retries: lane.Normalize.Retries, Policy: lane.NormalizeValidationPolicy, AllowStructuralRetry: lane.NormalizeExecutionClass == contracts.ExecutionClassLLMBacked}, func(attemptCtx context.Context, request producerAttemptRequest) (producerAttemptOutput, error) { started := time.Now().UTC() @@ -444,23 +469,19 @@ func (r *Runner) runNormalizeStage(ctx context.Context, input RunInput, checkpoi if validationErr != nil { return report, candidate.terminal.record(payload, validationErr) } - if report.FirstRejection() != nil { - return report, candidate.terminal.record(payload, nil) - } - if lane.NormalizeValidationPolicy.ValidatorFailure == ValidatorFailureFailRun { - if failure := firstIncompleteValidation(report); failure != nil { - return report, candidate.terminal.record(payload, validatorFailureError(*failure)) - } - } - return report, nil + return report, candidate.terminal.record(payload, nil) }) terminalSummary := validationSummary(terminalResult, StageNormalize, input.stepID, lane.ID, lane.Normalize.Module, "", 0) if debugErr := writeProducerTerminalDebug(input.Debug, path.Join("normalize", fileio.EncodePathComponent(lane.ID), "terminal.json"), terminalResult, lane.NormalizeValidationPolicy, terminalSummary); debugErr != nil { - _ = checkpointNormalizeFailed(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, debugErr) + if stageResult.reuseEligible { + _ = checkpointNormalizeFailed(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, debugErr) + } return stageResult, debugErr } if runErr != nil { - _ = checkpointNormalizeFailed(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, runErr) + if stageResult.reuseEligible { + _ = checkpointNormalizeFailed(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, runErr) + } return stageResult, runErr } if terminalResult.Action == producerTerminalRejected { @@ -473,8 +494,10 @@ func (r *Runner) runNormalizeStage(ctx context.Context, input RunInput, checkpoi output.ValidationSummaries = append(output.ValidationSummaries, artifacts.CloneValidationSummary(terminalSummary)) output.Warnings = append(output.Warnings, terminalResult.Warnings...) output.Rejected = append(output.Rejected, *rejected) - if err := checkpointNormalizeRejected(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, *rejected); err != nil { - return stageResult, err + if stageResult.reuseEligible { + if err := checkpointNormalizeRejected(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, *rejected); err != nil { + return stageResult, err + } } return stageResult, nil } @@ -489,18 +512,25 @@ func (r *Runner) runNormalizeStage(ctx context.Context, input RunInput, checkpoi } if encodeErr != nil { attemptErr := fmt.Errorf("serialize accepted normalize output for lane %q: %w", lane.ID, encodeErr) - _ = checkpointNormalizeFailed(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, attemptErr) + if stageResult.reuseEligible { + _ = checkpointNormalizeFailed(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, attemptErr) + } return stageResult, candidate.terminal.record(payload, attemptErr) } if debugErr := candidate.terminal.record(payload, nil); debugErr != nil { - _ = checkpointNormalizeFailed(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, debugErr) + if stageResult.reuseEligible { + _ = checkpointNormalizeFailed(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, debugErr) + } return stageResult, debugErr } serializedNormalize = stored normalizeWarnings = cloneWarnings(terminalResult.Warnings) output.ValidationSummaries = append(output.ValidationSummaries, terminalSummary) output.Warnings = append(output.Warnings, normalizeWarnings...) - if !terminalResult.ValidationIncomplete { + if terminalResult.ValidationIncomplete { + stageResult.reuseEligible = false + } + if stageResult.reuseEligible { if err := recordNormalize(checkpoints, input.stepID, lane.ID, lane.Normalize.Module, normalizeDeps, serializedNormalize, normalizeWarnings); err != nil { return stageResult, err } @@ -616,20 +646,6 @@ func (r *Runner) validateTypedReport(ctx context.Context, codec artifactCodecEnt return report, nil } -func (r *Runner) validateTypedArtifact(ctx context.Context, codec artifactCodecEntry, target typedValidationTarget, chain preparedValidatorChain, attempt int, debug DebugRecorder) ([]contracts.Warning, *contracts.RejectedOutput, error) { - report, err := r.validateTypedReport(ctx, codec, target, chain, attempt, debug) - if err != nil { - return report.Warnings(), nil, err - } - if failure := report.FirstFailure(); failure != nil { - return report.Warnings(), nil, validatorFailureError(*failure) - } - if rejected := typedRejection(report, target, attempt); rejected != nil { - return report.Warnings(), rejected, nil - } - return report.Warnings(), nil, nil -} - func typedRejection(report validationReport, target typedValidationTarget, attempt int) *contracts.RejectedOutput { rejection := report.FirstRejection() if rejection == nil { diff --git a/internal/framework/pipeline/validation_executor.go b/internal/framework/pipeline/validation_executor.go index 1d570386..0b9ad842 100644 --- a/internal/framework/pipeline/validation_executor.go +++ b/internal/framework/pipeline/validation_executor.go @@ -18,7 +18,7 @@ const ( validationSkipped validationOutcome = "skipped" ) -const defaultCorrectionGuidance = "Correct the candidate to satisfy the validator requirements." +const correctionRequestIntroduction = "The previous response failed semantic validation. Return one complete corrected replacement response, not a patch, explanation, or commentary.\n\nCorrect all of the following:\n" // validationRecord captures the settled result of one configured validator. // Its fields remain private so reports cannot expose mutable warning storage. @@ -82,33 +82,39 @@ func (report validationReport) FirstFailure() *validationRecord { return nil } -func (report validationReport) CorrectionGuidance() string { +func (report validationReport) CorrectionRequest() (string, error) { seen := make(map[string]struct{}) parts := make([]string, 0, len(report.records)) - used := 0 for _, record := range report.records { if record.outcome != validationRejected { continue } guidance := strings.TrimSpace(record.correctionGuidance) if guidance == "" { - guidance = defaultCorrectionGuidance + return "", fmt.Errorf("validator %q rejected output without correction guidance", record.validatorName) } if _, exists := seen[guidance]; exists { continue } - separator := 0 - if len(parts) > 0 { - separator = 1 - } - if used+separator+len(guidance) > contracts.MaxCorrectionGuidanceBytes { - break - } seen[guidance] = struct{}{} parts = append(parts, guidance) - used += separator + len(guidance) } - return strings.Join(parts, "\n") + if len(parts) == 0 { + return "", errors.New("validation report contains no correction guidance") + } + var builder strings.Builder + builder.WriteString(correctionRequestIntroduction) + for index, guidance := range parts { + fmt.Fprintf(&builder, "%d. %s", index+1, guidance) + if index+1 < len(parts) { + builder.WriteByte('\n') + } + } + request := builder.String() + if len(request) > contracts.MaxCorrectionGuidanceBytes { + return "", fmt.Errorf("aggregate correction request exceeds maximum length of %d bytes", contracts.MaxCorrectionGuidanceBytes) + } + return request, nil } type validationInvocation struct { @@ -184,15 +190,11 @@ func executeValidationChain(ctx context.Context, chain preparedValidatorChain, i report.records = append(report.records, validationRecord{validatorName: binding.Module, outcome: validationApproved, attemptCount: attempt, warnings: cloneWarnings(invocation.result.Warnings), diagnosticPath: invocation.result.DiagnosticArtifactPath}) break } - reason := invocation.result.ReasonCode - if reason == "" { - reason = "output_rejected" - } message := invocation.result.Message if message == "" { message = "output rejected" } - report.records = append(report.records, validationRecord{validatorName: binding.Module, outcome: validationRejected, attemptCount: attempt, reasonCode: reason, message: message, diagnosticPath: invocation.result.DiagnosticArtifactPath, warnings: cloneWarnings(invocation.result.Warnings), correctionGuidance: invocation.result.CorrectionGuidance}) + report.records = append(report.records, validationRecord{validatorName: binding.Module, outcome: validationRejected, attemptCount: attempt, reasonCode: invocation.result.ReasonCode, message: message, diagnosticPath: invocation.result.DiagnosticArtifactPath, warnings: cloneWarnings(invocation.result.Warnings), correctionGuidance: invocation.result.CorrectionGuidance}) break } } diff --git a/internal/framework/pipeline/validation_executor_test.go b/internal/framework/pipeline/validation_executor_test.go index c6cf7321..c288a592 100644 --- a/internal/framework/pipeline/validation_executor_test.go +++ b/internal/framework/pipeline/validation_executor_test.go @@ -17,7 +17,7 @@ func TestExecuteValidationChainSettlesEveryValidatorInOrder(t *testing.T) { invoke validationInvoker want []validationOutcome wantCalls []string - wantGuidance string + wantGuidance []string wantWarnings []string }{ { @@ -41,7 +41,7 @@ func TestExecuteValidationChainSettlesEveryValidatorInOrder(t *testing.T) { }), want: []validationOutcome{validationRejected, validationRejected, validationRejected}, wantCalls: []string{"shape:1", "refs:1", "coverage:1"}, - wantGuidance: "repair shape\nrepair references", + wantGuidance: []string{"repair shape", "repair references"}, }, { name: "rejection and failure both settle", @@ -52,7 +52,7 @@ func TestExecuteValidationChainSettlesEveryValidatorInOrder(t *testing.T) { }), want: []validationOutcome{validationRejected, validationFailed}, wantCalls: []string{"shape:1", "remote:1"}, - wantGuidance: "repair shape", + wantGuidance: []string{"repair shape"}, }, { name: "failure only has no correction guidance", @@ -103,8 +103,19 @@ func TestExecuteValidationChainSettlesEveryValidatorInOrder(t *testing.T) { if !reflect.DeepEqual(outcomes, test.want) || !reflect.DeepEqual(calls, test.wantCalls) { t.Fatalf("outcomes = %#v calls = %#v, want %#v %#v", outcomes, calls, test.want, test.wantCalls) } - if guidance := report.CorrectionGuidance(); guidance != test.wantGuidance { - t.Fatalf("CorrectionGuidance() = %q, want %q", guidance, test.wantGuidance) + if len(test.wantGuidance) > 0 { + guidance, err := report.CorrectionRequest() + if err != nil { + t.Fatalf("CorrectionRequest() error = %v", err) + } + if !strings.Contains(guidance, "complete corrected replacement") { + t.Fatalf("CorrectionRequest() = %q, want complete replacement instruction", guidance) + } + for _, item := range test.wantGuidance { + if strings.Count(guidance, item) != 1 { + t.Fatalf("CorrectionRequest() = %q, want one occurrence of %q", guidance, item) + } + } } warnings := report.Warnings() var warningCodes []string @@ -139,7 +150,7 @@ func TestExecuteValidationChainKeepsInvocationInputsImmutable(t *testing.T) { } } -func TestValidationReportBoundsGuidanceAndOwnsRecords(t *testing.T) { +func TestValidationReportRejectsOversizedGuidanceAndOwnsRecords(t *testing.T) { guidance := strings.Repeat("x", contracts.MaxValidationCorrectionGuidanceBytes) report := validationReport{records: make([]validationRecord, 17)} for index := range report.records { @@ -148,8 +159,8 @@ func TestValidationReportBoundsGuidanceAndOwnsRecords(t *testing.T) { report.records[index] = validationRecord{validatorName: "validator", outcome: validationRejected, correctionGuidance: string(unique)} } report.records[0].warnings = []contracts.Warning{{ReasonCode: "warning"}} - if got := report.CorrectionGuidance(); len(got) > contracts.MaxCorrectionGuidanceBytes || !strings.Contains(got, string([]byte{byte('a')})) || strings.Contains(got, string([]byte{byte('q')})) { - t.Fatalf("CorrectionGuidance() length = %d, want bounded ordered guidance", len(got)) + if _, err := report.CorrectionRequest(); err == nil { + t.Fatal("CorrectionRequest() error = nil, want aggregate overflow error") } records := report.Records() records[0].warnings[0].ReasonCode = "changed" diff --git a/internal/modules/dnd/validate/combatturns/invariants/validator.go b/internal/modules/dnd/validate/combatturns/invariants/validator.go index c314fe54..618d170e 100644 --- a/internal/modules/dnd/validate/combatturns/invariants/validator.go +++ b/internal/modules/dnd/validate/combatturns/invariants/validator.go @@ -40,7 +40,7 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.CombatTurnList]) (contracts.ValidationResult, error) { if err := Validate(req.Source, req.Value); err != nil { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error()}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error(), CorrectionGuidance: "Return combat turns in transcript order, without duplicate turns, using consistent contextual combatant names and evidence for each turn."}, nil } return contracts.ValidationResult{Approved: true}, nil } diff --git a/internal/modules/dnd/validate/combatturns/shape/validator.go b/internal/modules/dnd/validate/combatturns/shape/validator.go index 84b911b6..07516a58 100644 --- a/internal/modules/dnd/validate/combatturns/shape/validator.go +++ b/internal/modules/dnd/validate/combatturns/shape/validator.go @@ -100,5 +100,5 @@ func DecodeOptions(options map[string]any) (Options, error) { func validateOptions(options map[string]any) error { _, err := DecodeOptions(options); return err } func rejection(message string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: "Return a complete combat-turn list with every required field present, valid combatant names, and valid source references."} } diff --git a/internal/modules/dnd/validate/combatturns/shape/validator_test.go b/internal/modules/dnd/validate/combatturns/shape/validator_test.go index 446bf6a7..1e24dc83 100644 --- a/internal/modules/dnd/validate/combatturns/shape/validator_test.go +++ b/internal/modules/dnd/validate/combatturns/shape/validator_test.go @@ -41,6 +41,9 @@ func TestValidateRejectsEveryOwnedShapeBoundary(t *testing.T) { if err != nil || result.Approved || result.ReasonCode != ReasonCode { t.Fatalf("Validator result = %#v, %v; want shape rejection", result, err) } + if err := contracts.ValidateValidationResult(result); err != nil { + t.Fatalf("ValidateValidationResult() error = %v for %#v", err, result) + } }) } } diff --git a/internal/modules/dnd/validate/combatturns/source_refs/validator.go b/internal/modules/dnd/validate/combatturns/source_refs/validator.go index d8a9d337..f2e6455f 100644 --- a/internal/modules/dnd/validate/combatturns/source_refs/validator.go +++ b/internal/modules/dnd/validate/combatturns/source_refs/validator.go @@ -49,9 +49,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } return contracts.ValidationResult{ - Approved: false, - ReasonCode: ReasonCode, - Message: diagnostics.Aggregate("invalid combat turn source references", issues), + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid combat turn source references", issues), + CorrectionGuidance: "Return combat turns whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each turn.", }, nil } diff --git a/internal/modules/dnd/validate/combatturns/source_refs/validator_test.go b/internal/modules/dnd/validate/combatturns/source_refs/validator_test.go index a6467c06..6b7076d1 100644 --- a/internal/modules/dnd/validate/combatturns/source_refs/validator_test.go +++ b/internal/modules/dnd/validate/combatturns/source_refs/validator_test.go @@ -42,6 +42,9 @@ func TestValidatorRejectsInvalidSourceIdentityExistenceAndOrder(t *testing.T) { if err != nil || result.Approved || result.ReasonCode != ReasonCode || !strings.Contains(result.Message, test.want) { t.Fatalf("Validate() = %#v, %v; want source-reference rejection containing %q", result, err, test.want) } + if err := contracts.ValidateValidationResult(result); err != nil { + t.Fatalf("ValidateValidationResult() error = %v for %#v", err, result) + } }) } } diff --git a/internal/modules/dnd/validate/correction_contract_test.go b/internal/modules/dnd/validate/correction_contract_test.go new file mode 100644 index 00000000..64331cf1 --- /dev/null +++ b/internal/modules/dnd/validate/correction_contract_test.go @@ -0,0 +1,180 @@ +package validate_test + +import ( + "context" + "strings" + "testing" + + "gitea.maximumdirect.net/eric/notarius/internal/core/source" + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + npccodec "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/codec/npcregistry" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" + npcregistry "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/registry" + combatshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/combatturns/shape" + combatsourcerefs "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/combatturns/source_refs" + combatsourcerelatedness "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/combatturns/source_relatedness" + npcregistryvalidator "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcoccurrences/registry" + npcidentity "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcregistry/identity" + spellcatalog "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/spells/catalog" +) + +func TestRepresentativeProductionValidatorResultsSatisfyCorrectionContract(t *testing.T) { + document := &source.SourceDocument{ + ID: "session", Kind: "transcript", Format: "application/json", Digest: "sha256:session", + Units: []source.SourceUnit{{ID: 1, Kind: "message", Text: "Mira Thorn waits while Aria acts."}}, + } + npcReferences := npcRegistryReferences(t, "Mira Thorn") + npcRegistryValidator, err := npcregistryvalidator.New(npcregistryvalidator.Options{}, npcReferences) + if err != nil { + t.Fatal(err) + } + unknownNPCID := "npc:sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + + tests := []struct { + name string + validate func() (contracts.ValidationResult, error) + wantApproved bool + wantWarningCount int + }{ + { + name: "shape rejection", + validate: func() (contracts.ValidationResult, error) { + return combatshape.New(combatshape.Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.CombatTurnList]{ + Value: dnd.CombatTurnList{}, + }) + }, + }, + { + name: "source reference rejection", + validate: func() (contracts.ValidationResult, error) { + return combatsourcerefs.New(combatsourcerefs.Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.CombatTurnList]{ + Source: document, + Value: dnd.CombatTurnList{CombatTurns: []dnd.CombatTurn{{ + Actor: "Aria", TurnKind: dnd.CombatTurnKindTurn, + SourceRefs: []source.SourceRef{{SourceID: "other", StartUnitID: 1, EndUnitID: 1}}, + }}}, + }) + }, + }, + { + name: "registry identity rejection", + validate: func() (contracts.ValidationResult, error) { + return npcidentity.New(npcidentity.Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCRegistry]{ + Value: dnd.NPCRegistry{NPCs: []dnd.NPC{{ + ID: "not-an-id", Name: "Mira Thorn", + SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}, + }}}, + }) + }, + }, + { + name: "catalog membership rejection", + validate: func() (contracts.ValidationResult, error) { + validator, buildErr := spellcatalog.New(spellcatalog.Options{}) + if buildErr != nil { + return contracts.ValidationResult{}, buildErr + } + return validator.Validate(context.Background(), contracts.TypedValidationRequest[dnd.SpellList]{ + Value: dnd.SpellList{SpellCasts: []dnd.SpellCast{{ + Caster: "Aria", Spell: "Definitely Not A Spell", + SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}, + }}}, + }) + }, + }, + { + name: "registry membership rejection", + validate: func() (contracts.ValidationResult, error) { + return npcRegistryValidator.Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCOccurrenceList]{ + References: npcReferences, + Value: dnd.NPCOccurrenceList{Occurrences: []dnd.NPCOccurrence{{ + NPCID: unknownNPCID, Name: "Mira Thorn", Kind: dnd.NPCOccurrenceKindDialogue, + SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}, + }}}, + }) + }, + }, + { + name: "source relatedness advisory", + validate: func() (contracts.ValidationResult, error) { + return combatsourcerelatedness.New(combatsourcerelatedness.Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.CombatTurnList]{ + Source: document, + Value: dnd.CombatTurnList{CombatTurns: []dnd.CombatTurn{{ + Actor: "Unmentioned Actor", TurnKind: dnd.CombatTurnKindTurn, + SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}, + }}}, + }) + }, + wantApproved: true, + wantWarningCount: 1, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := test.validate() + if err != nil { + t.Fatalf("Validate() error = %v", err) + } + if result.Approved != test.wantApproved { + t.Fatalf("Validate() approved = %t, want %t: %#v", result.Approved, test.wantApproved, result) + } + if len(result.Warnings) != test.wantWarningCount { + t.Fatalf("Validate() warnings = %d, want %d: %#v", len(result.Warnings), test.wantWarningCount, result) + } + if err := contracts.ValidateValidationResult(result); err != nil { + t.Fatalf("ValidateValidationResult() error = %v for %#v", err, result) + } + if !result.Approved && strings.TrimSpace(result.CorrectionGuidance) == "" { + t.Fatalf("rejection lacks actionable correction guidance: %#v", result) + } + }) + } +} + +func TestRegistryCorrectionGuidanceUsesContextualNamesNotOpaqueIDs(t *testing.T) { + references := npcRegistryReferences(t, "Mira Thorn") + validator, err := npcregistryvalidator.New(npcregistryvalidator.Options{}, references) + if err != nil { + t.Fatal(err) + } + opaqueID := "npc:sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" + result, err := validator.Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCOccurrenceList]{ + References: references, + Value: dnd.NPCOccurrenceList{Occurrences: []dnd.NPCOccurrence{{ + NPCID: opaqueID, Name: "Mira Thorn", Kind: dnd.NPCOccurrenceKindDialogue, + SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}, + }}}, + }) + if err != nil || result.Approved { + t.Fatalf("Validate() = %#v, %v; want rejection", result, err) + } + if !strings.Contains(result.CorrectionGuidance, "contextual NPC names") { + t.Fatalf("correction guidance = %q, want contextual-name instruction", result.CorrectionGuidance) + } + if strings.Contains(result.CorrectionGuidance, opaqueID) || strings.Contains(result.CorrectionGuidance, "npc:sha256:") { + t.Fatalf("correction guidance leaked opaque durable ID: %q", result.CorrectionGuidance) + } +} + +func npcRegistryReferences(t *testing.T, name string) contracts.ReferenceSet { + t.Helper() + content, err := npccodec.New().Encode(dnd.NPCRegistry{NPCs: []dnd.NPC{{ + ID: identity.DeriveID(name), Name: name, + SourceRefs: []source.SourceRef{{SourceID: "prior-session", StartUnitID: 1, EndUnitID: 1}}, + }}}) + if err != nil { + t.Fatal(err) + } + return contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{ + npcregistry.ReferenceSlot: { + Slot: contracts.ReferenceSlot{Name: npcregistry.ReferenceSlot}, + Items: []contracts.ReferenceItem{{ + SlotName: npcregistry.ReferenceSlot, + MediaType: npccodec.MediaType, + Content: content, + }}, + }, + }} +} diff --git a/internal/modules/dnd/validate/enemyevents/engagements/validator.go b/internal/modules/dnd/validate/enemyevents/engagements/validator.go index 3678d2ad..266955cf 100644 --- a/internal/modules/dnd/validate/enemyevents/engagements/validator.go +++ b/internal/modules/dnd/validate/enemyevents/engagements/validator.go @@ -54,6 +54,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq Message: diagnostics.Aggregate("duplicate enemy engagement", []string{ fmt.Sprintf("subject %s has more than one engagement in one combat scene", diagnostics.Quote(event.Name)), }), + CorrectionGuidance: "Return at most one engagement event for each contextual enemy name within the same combat scene.", }, nil } seen[identity] = struct{}{} diff --git a/internal/modules/dnd/validate/enemyevents/invariants/validator.go b/internal/modules/dnd/validate/enemyevents/invariants/validator.go index f3d02859..27d6f662 100644 --- a/internal/modules/dnd/validate/enemyevents/invariants/validator.go +++ b/internal/modules/dnd/validate/enemyevents/invariants/validator.go @@ -90,13 +90,13 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{}, fmt.Errorf("resolve NPC registry: %w", err) } if !registry.Bound() { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: "invalid enemy event normalization: NPC registry reference is required"}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: "invalid enemy event normalization: NPC registry reference is required", CorrectionGuidance: "Return enemy events using contextual NPC names that match the supplied NPC registry."}, nil } issues := issuesFor(shared.NewSourceRefOrderFromIndex(index), req.Value, registry) if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid enemy event normalization", issues)}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid enemy event normalization", issues), CorrectionGuidance: "Return enemy events in canonical transcript order, without duplicate events, using contextual NPC names that match the supplied registry and evidence."}, nil } func allSourceRefsValid(index source.DocumentIndex, value dnd.EnemyEventList) bool { diff --git a/internal/modules/dnd/validate/enemyevents/shape/validator.go b/internal/modules/dnd/validate/enemyevents/shape/validator.go index 6f8e5e90..8105804d 100644 --- a/internal/modules/dnd/validate/enemyevents/shape/validator.go +++ b/internal/modules/dnd/validate/enemyevents/shape/validator.go @@ -36,7 +36,7 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.EnemyEventList]) (contracts.ValidationResult, error) { if err := Validate(req.Value); err != nil { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error()}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error(), CorrectionGuidance: "Return a complete enemy-event list with every required field present, valid contextual NPC names, supported event values, and valid source references."}, nil } return contracts.ValidationResult{Approved: true}, nil } diff --git a/internal/modules/dnd/validate/enemyevents/source_refs/validator.go b/internal/modules/dnd/validate/enemyevents/source_refs/validator.go index 0c3f0b04..5e52c0a5 100644 --- a/internal/modules/dnd/validate/enemyevents/source_refs/validator.go +++ b/internal/modules/dnd/validate/enemyevents/source_refs/validator.go @@ -50,7 +50,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid enemy event source references", issues)}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid enemy event source references", issues), CorrectionGuidance: "Return enemy events whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each event."}, nil } func sourceRefIssues(index source.DocumentIndex, doc *source.SourceDocument, coverage *chunkCoverage, value dnd.EnemyEventList) []string { diff --git a/internal/modules/dnd/validate/itemoccurrences/invariants/validator.go b/internal/modules/dnd/validate/itemoccurrences/invariants/validator.go index c601830e..b28a1078 100644 --- a/internal/modules/dnd/validate/itemoccurrences/invariants/validator.go +++ b/internal/modules/dnd/validate/itemoccurrences/invariants/validator.go @@ -38,7 +38,7 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.ItemOccurrenceList]) (contracts.ValidationResult, error) { if err := Validate(req.Source, req.Value); err != nil { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error()}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error(), CorrectionGuidance: "Return item occurrences in canonical transcript order, without duplicate occurrences, using consistent contextual item names and valid event transitions."}, nil } return contracts.ValidationResult{Approved: true}, nil } diff --git a/internal/modules/dnd/validate/itemoccurrences/registry/validator.go b/internal/modules/dnd/validate/itemoccurrences/registry/validator.go index 2585df73..5d432f55 100644 --- a/internal/modules/dnd/validate/itemoccurrences/registry/validator.go +++ b/internal/modules/dnd/validate/itemoccurrences/registry/validator.go @@ -96,7 +96,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq } func rejection(issues []string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid item occurrence registry", issues)} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid item occurrence registry", issues), CorrectionGuidance: "Return item occurrences using contextual item names that match an item in the supplied registry; omit occurrences that cannot be matched unambiguously."} } func Spec() pipeline.ValidatorSpec { return pipeline.ValidatorSpec{Key: Key, ExecutionClass: contracts.ExecutionClassDeterministic} diff --git a/internal/modules/dnd/validate/itemoccurrences/shape/validator.go b/internal/modules/dnd/validate/itemoccurrences/shape/validator.go index db5a1400..02684217 100644 --- a/internal/modules/dnd/validate/itemoccurrences/shape/validator.go +++ b/internal/modules/dnd/validate/itemoccurrences/shape/validator.go @@ -36,7 +36,7 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.ItemOccurrenceList]) (contracts.ValidationResult, error) { if err := Validate(req.Value); err != nil { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error()}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error(), CorrectionGuidance: "Return a complete item-occurrence list with every required field present, valid contextual item names, supported event kinds and holder combinations, positive quantities, and valid source references."}, nil } return contracts.ValidationResult{Approved: true}, nil } diff --git a/internal/modules/dnd/validate/itemoccurrences/source_refs/validator.go b/internal/modules/dnd/validate/itemoccurrences/source_refs/validator.go index 344d0dc4..203505ef 100644 --- a/internal/modules/dnd/validate/itemoccurrences/source_refs/validator.go +++ b/internal/modules/dnd/validate/itemoccurrences/source_refs/validator.go @@ -61,7 +61,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid item occurrence source references", issues)}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid item occurrence source references", issues), CorrectionGuidance: "Return item occurrences whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each occurrence."}, nil } type chunkCoverage struct { diff --git a/internal/modules/dnd/validate/itemregistry/identity/validator.go b/internal/modules/dnd/validate/itemregistry/identity/validator.go index 17d35479..961a2371 100644 --- a/internal/modules/dnd/validate/itemregistry/identity/validator.go +++ b/internal/modules/dnd/validate/itemregistry/identity/validator.go @@ -46,7 +46,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq for index, issue := range identityIssues { issues[index] = fmt.Sprintf("items[%d] %s: %s", issue.RecordIndex, issue.Code, diagnostics.Quote(issue.Value)) } - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid item identity", issues)}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid item identity", issues), CorrectionGuidance: "Return one canonical registry entry per distinct properly named item, combining duplicate mentions under the same transcript-supported name."}, nil } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/itemregistry/shape/validator.go b/internal/modules/dnd/validate/itemregistry/shape/validator.go index cb7cdd20..4dce38fe 100644 --- a/internal/modules/dnd/validate/itemregistry/shape/validator.go +++ b/internal/modules/dnd/validate/itemregistry/shape/validator.go @@ -93,5 +93,5 @@ func DecodeOptions(options map[string]any) (Options, error) { func validateOptions(options map[string]any) error { _, err := DecodeOptions(options); return err } func rejection(message string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: "Return a complete item registry containing only properly named items with valid source references."} } diff --git a/internal/modules/dnd/validate/itemregistry/source_refs/validator.go b/internal/modules/dnd/validate/itemregistry/source_refs/validator.go index 93236a6e..3d16777c 100644 --- a/internal/modules/dnd/validate/itemregistry/source_refs/validator.go +++ b/internal/modules/dnd/validate/itemregistry/source_refs/validator.go @@ -97,5 +97,5 @@ func DecodeOptions(options map[string]any) (Options, error) { func validateOptions(options map[string]any) error { _, err := DecodeOptions(options); return err } func rejection(message string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: "Return registry items whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each named item."} } diff --git a/internal/modules/dnd/validate/locationoccurrences/invariants/validator.go b/internal/modules/dnd/validate/locationoccurrences/invariants/validator.go index b8a2fc12..980dd4d7 100644 --- a/internal/modules/dnd/validate/locationoccurrences/invariants/validator.go +++ b/internal/modules/dnd/validate/locationoccurrences/invariants/validator.go @@ -50,7 +50,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid location occurrence normalization", issues)}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid location occurrence normalization", issues), CorrectionGuidance: "Return location occurrences in canonical transcript order, without duplicates, using consistent proper location names and supported occurrence kinds."}, nil } func allSourceRefsValid(index source.DocumentIndex, value dnd.LocationOccurrenceList) bool { diff --git a/internal/modules/dnd/validate/locationoccurrences/registry/validator.go b/internal/modules/dnd/validate/locationoccurrences/registry/validator.go index 1c81f133..1891356d 100644 --- a/internal/modules/dnd/validate/locationoccurrences/registry/validator.go +++ b/internal/modules/dnd/validate/locationoccurrences/registry/validator.go @@ -96,7 +96,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq } func rejection(issues []string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid location occurrence registry", issues)} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid location occurrence registry", issues), CorrectionGuidance: "Return location occurrences using proper contextual location names that match a location in the supplied registry; omit occurrences that cannot be matched unambiguously."} } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/locationoccurrences/shape/validator.go b/internal/modules/dnd/validate/locationoccurrences/shape/validator.go index d80a776c..fdb574a4 100644 --- a/internal/modules/dnd/validate/locationoccurrences/shape/validator.go +++ b/internal/modules/dnd/validate/locationoccurrences/shape/validator.go @@ -35,7 +35,7 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.LocationOccurrenceList]) (contracts.ValidationResult, error) { if err := Validate(req.Value); err != nil { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error()}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error(), CorrectionGuidance: "Return a complete location-occurrence list with every required field present, valid contextual location names, supported occurrence kinds, and valid source references."}, nil } return contracts.ValidationResult{Approved: true}, nil } diff --git a/internal/modules/dnd/validate/locationoccurrences/source_refs/validator.go b/internal/modules/dnd/validate/locationoccurrences/source_refs/validator.go index 77330280..20e4f58c 100644 --- a/internal/modules/dnd/validate/locationoccurrences/source_refs/validator.go +++ b/internal/modules/dnd/validate/locationoccurrences/source_refs/validator.go @@ -57,7 +57,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid location occurrence source references", issues)}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid location occurrence source references", issues), CorrectionGuidance: "Return location occurrences whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each occurrence."}, nil } func chunkContainsRef(chunk *source.Chunk, ref source.SourceRef) bool { diff --git a/internal/modules/dnd/validate/locationregistry/identity/validator.go b/internal/modules/dnd/validate/locationregistry/identity/validator.go index e68d77d6..99f1e34a 100644 --- a/internal/modules/dnd/validate/locationregistry/identity/validator.go +++ b/internal/modules/dnd/validate/locationregistry/identity/validator.go @@ -47,8 +47,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq issues[index] = fmt.Sprintf("locations[%d] %s: %s", issue.RecordIndex, issue.Code, diagnostics.Quote(issue.Value)) } return contracts.ValidationResult{ - Approved: false, ReasonCode: ReasonCode, - Message: diagnostics.Aggregate("invalid location identity", issues), + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid location identity", issues), + CorrectionGuidance: "Return one canonical registry entry per distinct proper location name, combining duplicate mentions of the same location.", }, nil } diff --git a/internal/modules/dnd/validate/locationregistry/shape/validator.go b/internal/modules/dnd/validate/locationregistry/shape/validator.go index 1a4e66d1..931fdbad 100644 --- a/internal/modules/dnd/validate/locationregistry/shape/validator.go +++ b/internal/modules/dnd/validate/locationregistry/shape/validator.go @@ -93,5 +93,5 @@ func DecodeOptions(options map[string]any) (Options, error) { func validateOptions(options map[string]any) error { _, err := DecodeOptions(options); return err } func rejection(message string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: "Return a complete location registry containing only properly named locations with valid source references."} } diff --git a/internal/modules/dnd/validate/locationregistry/source_refs/validator.go b/internal/modules/dnd/validate/locationregistry/source_refs/validator.go index 438bc5d1..c4b8ed5b 100644 --- a/internal/modules/dnd/validate/locationregistry/source_refs/validator.go +++ b/internal/modules/dnd/validate/locationregistry/source_refs/validator.go @@ -97,5 +97,5 @@ func DecodeOptions(options map[string]any) (Options, error) { func validateOptions(options map[string]any) error { _, err := DecodeOptions(options); return err } func rejection(message string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: "Return registry locations whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each proper location name."} } diff --git a/internal/modules/dnd/validate/npcoccurrences/invariants/validator.go b/internal/modules/dnd/validate/npcoccurrences/invariants/validator.go index 565282f8..2cb8d294 100644 --- a/internal/modules/dnd/validate/npcoccurrences/invariants/validator.go +++ b/internal/modules/dnd/validate/npcoccurrences/invariants/validator.go @@ -92,16 +92,17 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{}, fmt.Errorf("resolve NPC registry: %w", err) } if !npcRegistry.Bound() { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: "invalid NPC occurrence normalization: NPC registry reference is required"}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: "invalid NPC occurrence normalization: NPC registry reference is required", CorrectionGuidance: "Return NPC occurrences using contextual NPC names that match the supplied NPC registry."}, nil } issues := issuesFor(shared.NewSourceRefOrderFromIndex(index), req.Value, npcRegistry) if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } return contracts.ValidationResult{ - Approved: false, - ReasonCode: ReasonCode, - Message: diagnostics.Aggregate("invalid NPC occurrence normalization", issues), + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid NPC occurrence normalization", issues), + CorrectionGuidance: "Return NPC occurrences in canonical transcript order, without duplicates, using contextual NPC names that match the supplied registry and supported interaction kinds.", }, nil } diff --git a/internal/modules/dnd/validate/npcoccurrences/registry/validator.go b/internal/modules/dnd/validate/npcoccurrences/registry/validator.go index 096b700f..7db8b66e 100644 --- a/internal/modules/dnd/validate/npcoccurrences/registry/validator.go +++ b/internal/modules/dnd/validate/npcoccurrences/registry/validator.go @@ -105,9 +105,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq func rejection(issues []string) contracts.ValidationResult { return contracts.ValidationResult{ - Approved: false, - ReasonCode: ReasonCode, - Message: diagnostics.Aggregate("invalid NPC occurrence registry", issues), + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid NPC occurrence registry", issues), + CorrectionGuidance: "Return NPC occurrences using contextual NPC names that match an NPC in the supplied registry; omit occurrences that cannot be matched unambiguously.", } } diff --git a/internal/modules/dnd/validate/npcoccurrences/registry/validator_test.go b/internal/modules/dnd/validate/npcoccurrences/registry/validator_test.go index 4555aa5d..48ed1f89 100644 --- a/internal/modules/dnd/validate/npcoccurrences/registry/validator_test.go +++ b/internal/modules/dnd/validate/npcoccurrences/registry/validator_test.go @@ -29,6 +29,12 @@ func TestValidatorRecognizesExactRegistryPairsAndRejectsUnknownIDs(t *testing.T) if err != nil || result.Approved || result.ReasonCode != ReasonCode || !strings.Contains(result.Message, "occurrences[0].npc_id") { t.Fatalf("unknown result = %#v, %v", result, err) } + if err := contracts.ValidateValidationResult(result); err != nil { + t.Fatalf("ValidateValidationResult() error = %v for %#v", err, result) + } + if !strings.Contains(result.CorrectionGuidance, "contextual NPC names") || strings.Contains(result.CorrectionGuidance, value.Occurrences[0].NPCID) { + t.Fatalf("correction guidance = %q, want contextual names without opaque IDs", result.CorrectionGuidance) + } value = validList("Mira Thorn") value.Occurrences[0].Name = "Hooded Guard" result, err = validator.Validate(context.Background(), request(references, value)) diff --git a/internal/modules/dnd/validate/npcoccurrences/shape/validator.go b/internal/modules/dnd/validate/npcoccurrences/shape/validator.go index 099ae1b1..07936e1c 100644 --- a/internal/modules/dnd/validate/npcoccurrences/shape/validator.go +++ b/internal/modules/dnd/validate/npcoccurrences/shape/validator.go @@ -35,7 +35,7 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.NPCOccurrenceList]) (contracts.ValidationResult, error) { if err := Validate(req.Value); err != nil { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error()}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error(), CorrectionGuidance: "Return a complete NPC-occurrence list with every required field present, valid contextual NPC names, supported interaction kinds, and valid source references."}, nil } return contracts.ValidationResult{Approved: true}, nil } diff --git a/internal/modules/dnd/validate/npcoccurrences/source_refs/validator.go b/internal/modules/dnd/validate/npcoccurrences/source_refs/validator.go index 49f0954b..47010a42 100644 --- a/internal/modules/dnd/validate/npcoccurrences/source_refs/validator.go +++ b/internal/modules/dnd/validate/npcoccurrences/source_refs/validator.go @@ -61,9 +61,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } return contracts.ValidationResult{ - Approved: false, - ReasonCode: ReasonCode, - Message: diagnostics.Aggregate("invalid NPC occurrence source references", issues), + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid NPC occurrence source references", issues), + CorrectionGuidance: "Return NPC occurrences whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each occurrence.", }, nil } diff --git a/internal/modules/dnd/validate/npcregistry/identity/validator.go b/internal/modules/dnd/validate/npcregistry/identity/validator.go index 5ec022a9..06db3bf6 100644 --- a/internal/modules/dnd/validate/npcregistry/identity/validator.go +++ b/internal/modules/dnd/validate/npcregistry/identity/validator.go @@ -51,9 +51,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq issues[index] = fmt.Sprintf("%s %s: %s", location, issue.Code, diagnostics.Quote(issue.Value)) } return contracts.ValidationResult{ - Approved: false, - ReasonCode: ReasonCode, - Message: diagnostics.Aggregate("invalid NPC identity", issues), + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid NPC identity", issues), + CorrectionGuidance: "Return one canonical registry entry per distinct properly named NPC, combining duplicate mentions under the same transcript-supported name.", }, nil } diff --git a/internal/modules/dnd/validate/npcregistry/identity/validator_test.go b/internal/modules/dnd/validate/npcregistry/identity/validator_test.go index 0fd85e12..0527dea0 100644 --- a/internal/modules/dnd/validate/npcregistry/identity/validator_test.go +++ b/internal/modules/dnd/validate/npcregistry/identity/validator_test.go @@ -57,6 +57,9 @@ func TestValidatorDefersShapeAndRejectsIdentityIssues(t *testing.T) { if err != nil || result.Approved || result.ReasonCode != ReasonCode { t.Fatalf("identity result = %#v, error = %v, want rejection", result, err) } + if err := contracts.ValidateValidationResult(result); err != nil { + t.Fatalf("ValidateValidationResult() error = %v for %#v", err, result) + } for _, want := range []string{"invalid_id", "duplicate_canonical_identity", "duplicate_id", "npcs[0]", "npcs[1]"} { if !strings.Contains(result.Message, want) { t.Fatalf("identity message %q missing %q", result.Message, want) diff --git a/internal/modules/dnd/validate/npcregistry/shape/validator.go b/internal/modules/dnd/validate/npcregistry/shape/validator.go index a4fcb44e..dd27735a 100644 --- a/internal/modules/dnd/validate/npcregistry/shape/validator.go +++ b/internal/modules/dnd/validate/npcregistry/shape/validator.go @@ -92,5 +92,5 @@ func DecodeOptions(options map[string]any) (Options, error) { func validateOptions(options map[string]any) error { _, err := DecodeOptions(options); return err } func rejection(message string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: "Return a complete NPC registry containing only properly named NPCs with valid source references."} } diff --git a/internal/modules/dnd/validate/npcregistry/source_refs/validator.go b/internal/modules/dnd/validate/npcregistry/source_refs/validator.go index 15057441..22d6e180 100644 --- a/internal/modules/dnd/validate/npcregistry/source_refs/validator.go +++ b/internal/modules/dnd/validate/npcregistry/source_refs/validator.go @@ -117,5 +117,5 @@ func DecodeOptions(options map[string]any) (Options, error) { func validateOptions(options map[string]any) error { _, err := DecodeOptions(options); return err } func rejection(message string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: "Return registry NPCs whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each proper NPC name."} } diff --git a/internal/modules/dnd/validate/scenedescriptions/invariants/validator.go b/internal/modules/dnd/validate/scenedescriptions/invariants/validator.go index 4b93a638..a934e617 100644 --- a/internal/modules/dnd/validate/scenedescriptions/invariants/validator.go +++ b/internal/modules/dnd/validate/scenedescriptions/invariants/validator.go @@ -48,8 +48,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } return contracts.ValidationResult{ - Approved: false, ReasonCode: ReasonCode, - Message: diagnostics.Aggregate("invalid scene description normalization", issues), + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid scene description normalization", issues), + CorrectionGuidance: "Return scene descriptions in canonical source order, with one unique scene per source range and no duplicate scene identity.", }, nil } diff --git a/internal/modules/dnd/validate/scenedescriptions/shape/validator.go b/internal/modules/dnd/validate/scenedescriptions/shape/validator.go index 343e1d26..32502dca 100644 --- a/internal/modules/dnd/validate/scenedescriptions/shape/validator.go +++ b/internal/modules/dnd/validate/scenedescriptions/shape/validator.go @@ -35,7 +35,7 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.SceneDescriptionList]) (contracts.ValidationResult, error) { if err := ValidateForStage(req.Value, req.Stage); err != nil { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error()}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: err.Error(), CorrectionGuidance: "Return a complete scene-description list with the required number of scenes, supported scene kinds, nonblank titles and summaries, and valid source references."}, nil } return contracts.ValidationResult{Approved: true}, nil } diff --git a/internal/modules/dnd/validate/scenedescriptions/source_refs/validator.go b/internal/modules/dnd/validate/scenedescriptions/source_refs/validator.go index 9f0942b7..a58b06f3 100644 --- a/internal/modules/dnd/validate/scenedescriptions/source_refs/validator.go +++ b/internal/modules/dnd/validate/scenedescriptions/source_refs/validator.go @@ -62,8 +62,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } return contracts.ValidationResult{ - Approved: false, ReasonCode: ReasonCode, - Message: diagnostics.Aggregate("invalid scene description source references", issues), + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid scene description source references", issues), + CorrectionGuidance: "Return exactly one scene description for the supplied chunk, using that chunk's scene identity and complete source range.", }, nil } diff --git a/internal/modules/dnd/validate/spells/catalog/validator.go b/internal/modules/dnd/validate/spells/catalog/validator.go index c90f3c32..6431528f 100644 --- a/internal/modules/dnd/validate/spells/catalog/validator.go +++ b/internal/modules/dnd/validate/spells/catalog/validator.go @@ -98,7 +98,7 @@ func rejection(unknown []unknownSpell) contracts.ValidationResult { issues = append(issues, issue) } message := rejectionMessage(issues, len(unknown)-len(issues)) - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: "Use recognized D&D spell names supported by the supplied transcript evidence, and omit any candidate that is not a spell cast."} } func truncateDisplayedName(name string) string { diff --git a/internal/modules/dnd/validate/spells/catalog/validator_test.go b/internal/modules/dnd/validate/spells/catalog/validator_test.go index 861cc1e0..26ac397b 100644 --- a/internal/modules/dnd/validate/spells/catalog/validator_test.go +++ b/internal/modules/dnd/validate/spells/catalog/validator_test.go @@ -54,6 +54,9 @@ func TestValidatorRejectsMultipleUnknownCastsInStableOrder(t *testing.T) { if result.Approved || result.ReasonCode != ReasonCode { t.Fatalf("Validate() = %#v, want unknown-spell rejection", result) } + if err := contracts.ValidateValidationResult(result); err != nil { + t.Fatalf("ValidateValidationResult() error = %v for %#v", err, result) + } if want := `spell_casts[0].spell "Unknown First", spell_casts[2].spell "Unknown Second"`; !strings.Contains(result.Message, want) { t.Fatalf("message = %q, want %q", result.Message, want) } diff --git a/internal/modules/dnd/validate/spells/shape/validator.go b/internal/modules/dnd/validate/spells/shape/validator.go index ab4107a2..a445a5a1 100644 --- a/internal/modules/dnd/validate/spells/shape/validator.go +++ b/internal/modules/dnd/validate/spells/shape/validator.go @@ -75,5 +75,5 @@ func DecodeOptions(options map[string]any) (Options, error) { } func validateOptions(options map[string]any) error { _, err := DecodeOptions(options); return err } func rejection(message string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: "Return a complete spell-cast list with a nonblank spell name and caster plus valid source references for every cast."} } diff --git a/internal/modules/dnd/validate/spells/source_refs/validator.go b/internal/modules/dnd/validate/spells/source_refs/validator.go index 0837c275..f1f59a41 100644 --- a/internal/modules/dnd/validate/spells/source_refs/validator.go +++ b/internal/modules/dnd/validate/spells/source_refs/validator.go @@ -111,5 +111,5 @@ func DecodeOptions(options map[string]any) (Options, error) { } func validateOptions(options map[string]any) error { _, err := DecodeOptions(options); return err } func rejection(message string) contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: "Return spell casts whose source references identify valid transcript ranges within the supplied extraction chunk and directly support the named spell and caster."} } diff --git a/internal/modules/generic/validate/always_reject/validator.go b/internal/modules/generic/validate/always_reject/validator.go index 342f4517..f9c6573a 100644 --- a/internal/modules/generic/validate/always_reject/validator.go +++ b/internal/modules/generic/validate/always_reject/validator.go @@ -18,7 +18,7 @@ func NewChunk(Options) *ChunkValidator { return &ChunkValidator{} } func NewTyped[T any](Options) *TypedValidator[T] { return &TypedValidator[T]{} } func rejection() contracts.ValidationResult { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: "output rejected by always-reject validator"} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: "output rejected by always-reject validator", CorrectionGuidance: "Return one complete replacement response that satisfies the configured validation requirements."} } func (v *ChunkValidator) Name() string { return Key } func (v *ChunkValidator) ExecutionClass() contracts.ExecutionClass { diff --git a/internal/modules/generic/validate/valid_json/validator.go b/internal/modules/generic/validate/valid_json/validator.go index 9133217f..1f14cc0e 100644 --- a/internal/modules/generic/validate/valid_json/validator.go +++ b/internal/modules/generic/validate/valid_json/validator.go @@ -31,7 +31,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.SerializedValidati func validate(content []byte) contracts.ValidationResult { if !json.Valid(content) { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCodeInvalidJSON, Message: "payload is not valid JSON"} + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCodeInvalidJSON, Message: "payload is not valid JSON", CorrectionGuidance: "Return one complete replacement response encoded as valid JSON."} } return contracts.ValidationResult{Approved: true} } diff --git a/internal/modules/generic/validate/valid_json_schema/validator.go b/internal/modules/generic/validate/valid_json_schema/validator.go index 7b5b58f0..a8a755fe 100644 --- a/internal/modules/generic/validate/valid_json_schema/validator.go +++ b/internal/modules/generic/validate/valid_json_schema/validator.go @@ -56,14 +56,14 @@ func (v *Validator) Validate(_ context.Context, req contracts.SerializedValidati } instance, err := jsonschema.UnmarshalJSON(bytes.NewReader(req.Content)) if err != nil { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCodeInvalidJSON, Message: "payload is not valid JSON"}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCodeInvalidJSON, Message: "payload is not valid JSON", CorrectionGuidance: "Return one complete replacement response encoded as valid JSON."}, nil } schema, err := v.compiledSchema(req.Schema) if err != nil { return contracts.ValidationResult{}, err } if err := schema.Validate(instance); err != nil { - return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCodeSchemaInvalid, Message: "payload does not conform to response schema"}, nil + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCodeSchemaInvalid, Message: "payload does not conform to response schema", CorrectionGuidance: "Return one complete JSON replacement that conforms exactly to the supplied response schema."}, nil } return contracts.ValidationResult{Approved: true}, nil }