From 4da9360d748ecd3cf6f66bfdbaa3b024a06cac60 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 28 Aug 2026 18:25:00 +0000 Subject: [PATCH] Improve semantic reconciliation retries --- ...feedback-aware-module-requested-retries.md | 76 +++++++++++ docs/internal/dnd.md | 29 +++-- docs/internal/llm.md | 8 +- docs/internal/modules.md | 10 ++ docs/internal/pipeline.md | 19 ++- docs/policy/architecture.md | 21 +++- docs/roadmap/future.md | 2 +- .../semantic-reconciliation-retry-feedback.md | 117 +++++++++++++++++ .../framework/contracts/typed_pipeline.go | 10 +- .../framework/pipeline/normalize_retry.go | 9 ++ .../framework/pipeline/normalizer_registry.go | 1 + .../pipeline/normalizer_registry_test.go | 8 +- .../framework/pipeline/producer_attempts.go | 36 +++++- .../pipeline/producer_attempts_test.go | 73 +++++++++++ .../pipeline/runner_normalize_retry_test.go | 107 +++++++++++++++- internal/framework/pipeline/runner_typed.go | 4 +- .../framework/semanticreconcile/identity.go | 2 +- .../framework/semanticreconcile/proposal.go | 118 +++++++++++++++++- .../semanticreconcile/proposal_test.go | 68 ++++++++++ .../dnd/normalize/itemregistry/normalizer.go | 38 ++++-- .../normalize/itemregistry/normalizer_test.go | 8 +- .../normalize/locationregistry/normalizer.go | 30 +++-- .../locationregistry/normalizer_test.go | 8 +- .../dnd/normalize/npcregistry/normalizer.go | 30 +++-- .../npcregistry/semantic_normalizer_test.go | 8 +- 25 files changed, 771 insertions(+), 69 deletions(-) create mode 100644 docs/adr/0016-feedback-aware-module-requested-retries.md create mode 100644 docs/roadmap/semantic-reconciliation-retry-feedback.md diff --git a/docs/adr/0016-feedback-aware-module-requested-retries.md b/docs/adr/0016-feedback-aware-module-requested-retries.md new file mode 100644 index 00000000..d29c3edd --- /dev/null +++ b/docs/adr/0016-feedback-aware-module-requested-retries.md @@ -0,0 +1,76 @@ +# ADR-0016: Use feedback-aware module-requested retries + +**Status:** Accepted +**Date:** 2026-08-28 + +## Context + +An LLM-backed module can discover that a structurally valid model response is +unsafe while translating it into a typed candidate, before the ordinary +validator chain runs. Semantic registry reconciliation is the first such case: +the shared core can identify invalid duplicate-group proposals, and typed +application can reject a domain-incompatible group while preserving a safe +partial result. Repeating the original request without the rejected response or +corrective explanation gives the model no information with which to improve. + +The existing feedback-aware validation mechanism already establishes the safe +correction protocol, but module-owned retry directives also carry internal +reason codes, operator messages, and fallback diagnostics. Those values are not +model instructions. Some module retry conditions, including exhausted +structured-output repair, also have no valid exact response to append. + +## Decision + +An LLM-backed normalizer may attach optional, bounded model-facing correction +guidance to a module-requested retry. Guidance is a separate contract field; the +framework never derives it from a reason code, operator message, diagnostic, or +error. A feedback-capable directive must include the exact model candidate that +controlled the safe fallback and must use `single_response_v1`. + +The artifact-neutral producer-attempt state machine constructs the correction +from that exact latest response and the supplied guidance. The producer rebuilds +its complete ordinary request and appends the response as one assistant message +and the guidance as one user message. Earlier attempts do not accumulate, and +the attempt consumes the same configured stage retry budget as operational, +structural, validation, and feedback-free module retries. + +A module retry without guidance remains valid and starts a fresh attempt. This +is the required behavior when no valid model candidate exists, including after +structured-output repair is exhausted. If feedback is supplied without a valid +supported candidate, the framework fails the module contract rather than +retrying blindly or inventing guidance. + +After retry exhaustion, the normalizer's safe fallback continues through its +validator chain. Safe semantic groups may remain applied, unsafe groups remain +separate, and bounded fallback diagnostics may explain the process degradation. +Exact responses and correction text remain attempt-local and are excluded from +ordinary errors, warnings, manifests, receipts, caches, and checkpoints. + +This decision extends, rather than supersedes, +[ADR-0014](0014-feedback-aware-validation-retries.md): both correction sources +use the same transport-neutral payload, replacement-request semantics, outer +retry budget, and sensitive-data boundary. + +## Alternatives considered + +- Continue blind module retries. This preserves a smaller contract but wastes + the module's deterministic diagnosis and commonly repeats the same defect. +- Convert module safety checks into validators. Typed reconciliation must apply + only safe proposal groups and retain a fallback before validation; moving + artifact-owned translation and application policy into validators would blur + stage ownership. +- Copy the retry reason or operator message into the model request. Those values + are written for provenance and humans, can contain opaque internal labels, + and do not reliably describe a correct replacement. +- Require feedback for every module retry. Structural failures may have no + valid exact candidate, so this would either prevent useful fresh retries or + fabricate prior-response material. + +## Consequences + +The normalize retry contract and generic producer-attempt directive gain an +optional correction-guidance field and candidate-pairing validation. Modules +that use it must provide semantically meaningful bounded prose and an exact +candidate. Registry reconciliation maintains separate operator and model +renderers, and policy fingerprints change so checkpoints created under blind +retry behavior are not reused. diff --git a/docs/internal/dnd.md b/docs/internal/dnd.md index 17892de8..068aada3 100644 --- a/docs/internal/dnd.md +++ b/docs/internal/dnd.md @@ -29,13 +29,17 @@ The D&D registrar registers the family’s artifact codecs, extractors, typed append-order mergers, normalizers, validators, prompt assets, fallback LLM profile asset, and default validator chains. Each extractor and normalizer has a stable module spec, explicit execution class, strict option decoding, and a -typed builder. Scene chunking, every extractor, and NPC, location, and item-registry -normalization are registered as `llm_backed`; the remaining current D&D mergers -and normalizers are `deterministic`. The metadata is available to catalog inspection and -resolved-pipeline debug data and determines which selected bindings inherit the -pipeline profile. The registry normalizers use `single_response_v1`, forwarding -corrections to their reconciliation completion and retaining the accepted raw -proposal only as an owned model candidate. Configuration remains the canonical owner of the exact keys, +typed builder. Scene chunking, every extractor, and NPC, location, and +item-registry normalization are registered as `llm_backed`; the remaining +current D&D mergers and normalizers are `deterministic`. The metadata is +available to catalog inspection and resolved-pipeline debug data and determines +which selected bindings inherit the pipeline profile. The registry normalizers +use `single_response_v1`, forwarding corrections to their reconciliation +completion and retaining the accepted raw proposal only as an owned model +candidate. When deterministic proposal safety or typed application rejects a +group, they provide separate model-facing prose for a corrective module retry; +internal issue categories, reason codes, and operator messages remain +diagnostic-only. Configuration remains the canonical owner of the exact keys, profile precedence, and validator order. Private structured-LLM response schemas are deliberately minimal. They reject @@ -117,6 +121,17 @@ presentation, and final ephemeral generic transcript windows. These orders and cache controls are prompt behavior; change them only through the owning manifest and prompt declaration. +NPC, item, and location registry reconciliation translate shared proposal +safety categories into bounded prose that references only the response-local +duplicate-group ordinals and candidate handles. Item reconciliation appends its +typed rule that currency may be consolidated only with aliases of the same +denomination and never with non-currency items. The next normalize attempt +receives that prose with the exact defective proposal under the shared +replacement-request protocol. Structurally invalid output has no valid proposal +candidate and receives a fresh feedback-free attempt instead. If the stage +budget is exhausted, safe groups stay applied, unsafe groups stay separate, and +one fallback warning summarizes the final defect without raw model content. + ## Evidence, Candidates, And Normalization The current transcript is the only durable evidence source. Extractors assign diff --git a/docs/internal/llm.md b/docs/internal/llm.md index 95ac3630..08bad59a 100644 --- a/docs/internal/llm.md +++ b/docs/internal/llm.md @@ -247,8 +247,12 @@ redaction boundary. Structural repair does not replace pipeline retry behavior: a binding's configured retry count reruns its complete stage attempt after an operational or structural error, module-requested retry, or actionable semantic rejection. -The pipeline owns attempt lifecycle, validation chains, and retry diagnostics; -see [Pipeline Internals](pipeline.md#validation-retries-and-output) and the +An actionable module-requested retry and a validator rejection both use the +same correction payload when the producer exposes an exact latest response; +feedback-free module retries reconstruct the ordinary request without appended +messages. The pipeline owns attempt lifecycle, validation chains, and retry +diagnostics; see +[Pipeline Internals](pipeline.md#validation-retries-and-output) and the [binding reference](../config.md#module-bindings-and-validators). ## Timeout Ownership diff --git a/docs/internal/modules.md b/docs/internal/modules.md index b8e7531b..cb0a2f4b 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -115,6 +115,16 @@ eligible producers. Deterministic skip, limit, and fallback outcomes carry no model candidate, so a later rejection applies terminal policy without spending an ineffective semantic retry. +When proposal assessment or typed group application rejects a structurally +valid group, the normalizer may return its safe partial value with a +module-requested retry. A feedback-capable directive supplies bounded +model-facing correction guidance separately from operator diagnostics and +retains the exact proposal response as its candidate. The shared stage retry +mechanism appends that response and guidance to a fresh complete request. A +structurally invalid completion has no valid candidate and therefore requests a +feedback-free fresh attempt. On exhaustion, only the final safe fallback and +its bounded process diagnostic advance to validation. + The core supplies a conservative generic prompt and the single private response schema. A domain prompt may substitute its semantic instructions but mounts the core-owned protocol and candidate/transcript presentation assets. diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index 4655da5c..3d353dd9 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -154,18 +154,25 @@ complete validation chain. It preserves terminal diagnostics only from the final or rejected attempt, plus one fixed validation-incomplete warning per validator whose execution budget was exhausted under `warn_continue`. Cancellation stops retries. Normalizer-specific retry directives consume this same budget and validate any -final safe fallback through the normalizer chain. +final safe fallback through the normalizer chain. A directive may carry bounded +correction guidance only when it also exposes the exact latest +`single_response_v1` candidate. The state machine then uses the same replacement +request shape as validator correction. A directive without guidance clears any +prior correction and starts a fresh attempt, which preserves structural retry +behavior when no valid response exists. The artifact-neutral producer-attempt state machine owns that shared budget, attempt provenance, semantic-correction material, and terminal-policy selection. It accepts producer and complete-validation closures, so artifact materialization, cache handling, checkpoints, and debug output stay at the operation boundary. It distinguishes operational, structural, module-requested, -and semantic retries. A semantic retry is available only for a valid latest -`single_response_v1` candidate; a deterministic or no-model rejection instead -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. +and validator-semantic retries. Model feedback from either semantic source is +available only for a valid latest `single_response_v1` candidate. A +deterministic or no-model validator rejection instead settles the semantic +policy immediately, while a feedback-free module directive remains an ordinary +fresh retry. 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 or validation-incomplete automatic cache hit is not model material and therefore diff --git a/docs/policy/architecture.md b/docs/policy/architecture.md index 9bb1407d..c07ea8db 100644 --- a/docs/policy/architecture.md +++ b/docs/policy/architecture.md @@ -201,6 +201,13 @@ 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. +An LLM-backed module may also request a feedback-aware retry when its own +deterministic translation or typed safety policy rejects a structurally valid +model response. It must supply model-facing guidance separately from its +reason code, operator message, and fallback diagnostics, together with the +exact `single_response_v1` candidate. A feedback-free module retry remains +valid when no exact candidate exists. + 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 @@ -222,12 +229,14 @@ the two budgets must remain separate. An LLM-backed producer can participate in semantic correction only when it declares `single_response_v1` and returns the exact one response that directly -controlled its candidate. On an actionable rejection, the framework rebuilds -the ordinary request and appends only the latest defective response as an -`assistant` message plus one aggregated `user` correction message. This is a -fresh replacement request, not a growing conversation. The retry budgets, -terminal policy, and sensitive-data rationale are recorded in -[ADR-0014](../adr/0014-feedback-aware-validation-retries.md). +controlled its candidate. On an actionable validator rejection or +feedback-capable module retry, the framework rebuilds the ordinary request and +appends only the latest defective response as an `assistant` message plus one +aggregated `user` correction message. This is a fresh replacement request, not +a growing conversation. The retry budgets, terminal policy, and sensitive-data +rationale are recorded in +[ADR-0014](../adr/0014-feedback-aware-validation-retries.md) and +[ADR-0016](../adr/0016-feedback-aware-module-requested-retries.md). When a model selects an application entity, callers must supply a contextual selection and deterministically attach the opaque application identity whenever diff --git a/docs/roadmap/future.md b/docs/roadmap/future.md index 3f555d9e..d8220705 100644 --- a/docs/roadmap/future.md +++ b/docs/roadmap/future.md @@ -15,7 +15,7 @@ diagnostics. The remaining near-term work applies those completed foundations to domain review and empirical evaluation. The active D&D reliability work is defined by -[D&D Item-Occurrence Holder Reliability](item-occurrence-holder-reliability.md). +[Feedback-Aware Semantic-Reconciliation Retries](semantic-reconciliation-retry-feedback.md). ## Near-Term D&D Pipeline diff --git a/docs/roadmap/semantic-reconciliation-retry-feedback.md b/docs/roadmap/semantic-reconciliation-retry-feedback.md new file mode 100644 index 00000000..d8185d13 --- /dev/null +++ b/docs/roadmap/semantic-reconciliation-retry-feedback.md @@ -0,0 +1,117 @@ +# Feedback-Aware Semantic-Reconciliation Retries + +## Purpose + +Improve semantic registry reconciliation for smaller models by turning the +existing module-requested retry into a bounded corrective exchange. Preserve +the current deterministic safety checks, shared retry budget, and safe fallback +when correction is unsuccessful. + +## Problem + +NPC, item, and location registry normalizers can reject unsafe duplicate-group +proposals and retain a safe partial result. They currently ask the pipeline for +another attempt, but the next attempt receives the original prompt without the +defective response or an explanation of the defect. The normalizers also build +their retry messages from internal issue-category strings, which are suitable +for diagnostics but not useful model instructions. If all attempts are +exhausted, the warning reports the omitted-group count without explaining the +final defect or confirming that affected candidates were preserved separately. + +## Target State + +- A normalizer retry directive may carry model-facing correction guidance + separately from its stable reason code, operator message, and fallback + diagnostics. +- When the directive includes correction guidance and a valid + `single_response_v1` model candidate, the shared producer-attempt state + machine reconstructs the complete original prompt and appends the exact + defective response as an assistant message followed by one user correction + message. Earlier failed attempts do not accumulate. +- This correction consumes the existing normalize-stage retry budget. It does + not add a module-local loop or alter PromptKit's independent structured-output + repair budget. +- A retry caused by structurally invalid output remains a fresh stage attempt + without semantic feedback when no valid model candidate is available. +- Internal issue categories remain available for operator and debug + diagnostics, but are never sent to a model. A shared renderer translates + every known category into concise prose that explains the defect and the + required valid shape. +- Item reconciliation appends its domain rule when a proposal attempts to + combine different currency denominations or currency with a non-currency + item. NPC and location reconciliation use only the shared protocol guidance. +- On final exhaustion, safe proposal groups remain applied, unsafe groups + remain separate, and one process warning gives a bounded summary of the final + defect and explicitly states that the affected candidates were preserved + separately. + +## Required Work + +1. Extend `contracts.NormalizeRetry` and its clone, validation, and debug paths + with an optional, bounded model-facing correction-guidance field. Keep + reason codes and operator messages out of model input. Require guidance to + be paired with a valid `single_response_v1` model candidate; preserve the + existing feedback-free directive for structural retry cases. +2. Extend the generic producer-attempt state machine so a feedback-capable + module retry constructs `SemanticCorrection` from the latest exact response + and the supplied guidance. Preserve attempt provenance, cancellation, + terminal policy, diagnostic handling, byte-faithful response provenance, + and the rule that only the latest response is included. +3. Add a model-facing renderer to `internal/framework/semanticreconcile`. + Implement it with a typed lookup table keyed by `IssueCategory` and an + authoritative list of all categories, rather than a positional array. + Render response-local duplicate-group numbers in human-friendly form, + de-duplicate repeated guidance, enforce a size bound, and return an error for + any unmapped category instead of exposing its raw value. Retain + `IssueDetails` for operator/debug diagnostics. +4. Update the NPC, item, and location registry normalizers to populate the new + correction guidance for semantically invalid proposals. Add the item-only + currency guidance after the shared prose when typed application rejects a + group. Do not add guidance to the invalid-structured-output path when no + exact valid candidate exists. +5. Improve exhausted-fallback diagnostics across all three registries. Describe + the final proposal failure accurately, state that affected candidates were + preserved separately, and keep warning samples bounded and free of raw model + responses. +6. Update the shared semantic-reconciliation and affected normalizer policy + fingerprints so checkpoints produced under blind retry behavior are not + reused. Record the durable module-retry correction decision in a new ADR, + and update the architecture, pipeline, LLM-runtime, and D&D internal docs in + their canonical scopes when the behavior is implemented. +7. Add lean offline behavioral coverage for corrective module retries, exact + latest-response use, feedback-free structural retries, contract validation, + complete category-to-prose coverage, domain-specific item guidance, bounded + messages, safe final fallback, and warning content. Avoid exact prose + snapshots, prompt-length assertions, live providers, and redundant tests of + PromptKit internals. + +## Non-Goals + +- Changing normalize retry counts, PromptKit repair settings, or pipeline + terminal policies. +- Changing the reconciliation prompt protocol, response schema, registry + artifact schemas, or request-local candidate IDs. +- Reintroducing provider-sensitive schema constraints such as `uniqueItems`. +- Automatically applying an unsafe duplicate proposal or treating a fallback + warning as an extraction-quality warning. +- Persisting raw model responses or correction text in ordinary warnings, + receipts, manifests, caches, or checkpoints. + +## Acceptance Criteria + +- A semantic proposal rejected by an NPC, item, or location registry normalizer + receives the exact defective response and actionable prose on the next + normalize attempt, using only the configured stage retry budget. +- Every `IssueCategory` has non-empty model-facing guidance, no raw category or + reason-code string reaches the model, and an unmapped future category fails + closed. +- An invalid item consolidation additionally tells the model to keep currency + denominations distinct and not mix currency with non-currency items. +- Structural-output failures without a valid model candidate retain the current + fresh-retry behavior. +- After retry exhaustion, safe consolidations remain applied, affected + candidates remain separate, and the single fallback warning explains both + the final cause and preservation behavior without including raw model output. +- Checkpoint identities change where policy changed, canonical documentation + and a new ADR describe the implemented behavior, and focused tests plus + `go test ./...`, `go vet ./...`, and `go build ./cmd/notarius` pass. diff --git a/internal/framework/contracts/typed_pipeline.go b/internal/framework/contracts/typed_pipeline.go index 7572a86d..6f08a3bd 100644 --- a/internal/framework/contracts/typed_pipeline.go +++ b/internal/framework/contracts/typed_pipeline.go @@ -102,11 +102,12 @@ type TypedNormalizeResult[T any] struct { ModelCandidate *ModelCandidate } -// Normalize retry diagnostic limits bound module-provided values before the -// framework persists them in debug artifacts. +// Normalize retry limits bound module-provided control and diagnostic text +// before the framework consumes or records it. const ( - MaxNormalizeRetryReasonCodeBytes = 128 - MaxNormalizeRetryMessageBytes = 4096 + MaxNormalizeRetryReasonCodeBytes = 128 + MaxNormalizeRetryMessageBytes = 4096 + MaxNormalizeRetryCorrectionGuidanceBytes = 4096 ) // NormalizeRetry asks the framework to retry normalization while retaining a @@ -114,6 +115,7 @@ const ( type NormalizeRetry struct { ReasonCode string Message string + CorrectionGuidance string FallbackDiagnostics []ProducerDiagnostic } diff --git a/internal/framework/pipeline/normalize_retry.go b/internal/framework/pipeline/normalize_retry.go index 47416fcb..62ee17cd 100644 --- a/internal/framework/pipeline/normalize_retry.go +++ b/internal/framework/pipeline/normalize_retry.go @@ -30,5 +30,14 @@ func validateNormalizeRetry(retry *contracts.NormalizeRetry) error { if len(retry.Message) > contracts.MaxNormalizeRetryMessageBytes { return errors.New("normalize retry directive message exceeds maximum length") } + if !utf8.ValidString(retry.CorrectionGuidance) { + return errors.New("normalize retry directive correction guidance has invalid UTF-8") + } + if retry.CorrectionGuidance != "" && strings.TrimSpace(retry.CorrectionGuidance) == "" { + return errors.New("normalize retry directive correction guidance is blank") + } + if len(retry.CorrectionGuidance) > contracts.MaxNormalizeRetryCorrectionGuidanceBytes { + return errors.New("normalize retry directive correction guidance exceeds maximum length") + } return nil } diff --git a/internal/framework/pipeline/normalizer_registry.go b/internal/framework/pipeline/normalizer_registry.go index 07d2653d..7bb91867 100644 --- a/internal/framework/pipeline/normalizer_registry.go +++ b/internal/framework/pipeline/normalizer_registry.go @@ -101,6 +101,7 @@ func cloneNormalizeRetry(retry *contracts.NormalizeRetry) *contracts.NormalizeRe return &contracts.NormalizeRetry{ ReasonCode: retry.ReasonCode, Message: retry.Message, + CorrectionGuidance: retry.CorrectionGuidance, FallbackDiagnostics: contracts.CloneProducerDiagnostics(retry.FallbackDiagnostics), } } diff --git a/internal/framework/pipeline/normalizer_registry_test.go b/internal/framework/pipeline/normalizer_registry_test.go index 0ae79029..16a3aa2f 100644 --- a/internal/framework/pipeline/normalizer_registry_test.go +++ b/internal/framework/pipeline/normalizer_registry_test.go @@ -19,8 +19,9 @@ func (n retryingNotesNormalizer) Normalize(_ context.Context, req contracts.Type func TestNormalizerRegistryErasureClonesRetryDirective(t *testing.T) { retry := &contracts.NormalizeRetry{ - ReasonCode: "retryable", - Message: "safe fallback available", + ReasonCode: "retryable", + Message: "safe fallback available", + CorrectionGuidance: "return a complete corrected proposal", } registry := NewNormalizerRegistry() if err := RegisterNormalizer(registry, ModuleSpec{Key: "test/retry-normalize", Stage: StageNormalize, ExecutionClass: contracts.ExecutionClassDeterministic, ArtifactKind: "test/notes"}, func() (contracts.Normalizer[codecNotes], error) { @@ -41,7 +42,8 @@ func TestNormalizerRegistryErasureClonesRetryDirective(t *testing.T) { t.Fatalf("normalize() error = %v", err) } retry.Message = "mutated" - if result.Retry == nil || result.Retry.Message != "safe fallback available" { + retry.CorrectionGuidance = "mutated guidance" + if result.Retry == nil || result.Retry.Message != "safe fallback available" || result.Retry.CorrectionGuidance != "return a complete corrected proposal" { t.Fatalf("erased retry result = %#v, want independent retry data", result) } } diff --git a/internal/framework/pipeline/producer_attempts.go b/internal/framework/pipeline/producer_attempts.go index 3359dd87..4bb03131 100644 --- a/internal/framework/pipeline/producer_attempts.go +++ b/internal/framework/pipeline/producer_attempts.go @@ -52,6 +52,7 @@ type producerAttemptRequest struct { // the current value as a safe fallback if its shared budget is exhausted. // Artifact-specific adapters are responsible for validating and populating it. type producerRetryDirective struct { + CorrectionGuidance string FallbackDiagnostics []contracts.ProducerDiagnostic } @@ -59,7 +60,10 @@ func (directive *producerRetryDirective) clone() *producerRetryDirective { if directive == nil { return nil } - return &producerRetryDirective{FallbackDiagnostics: contracts.CloneProducerDiagnostics(directive.FallbackDiagnostics)} + return &producerRetryDirective{ + CorrectionGuidance: directive.CorrectionGuidance, + FallbackDiagnostics: contracts.CloneProducerDiagnostics(directive.FallbackDiagnostics), + } } // producerAttemptOutput is intentionally artifact-neutral. Value remains @@ -204,10 +208,21 @@ func runProducerAttempts(ctx context.Context, config producerAttemptConfig, prod return failedProducerAttempt(provenance), err } if output.Retry != nil && number < attemptLimit { + correction, err = moduleRetryCorrection(output) + if err != nil { + provenance = append(provenance, producerAttemptProvenance{Number: number, Kind: kind, Outcome: producerAttemptFailed}) + return failedProducerAttempt(provenance), err + } provenance = append(provenance, producerAttemptProvenance{Number: number, Kind: kind, Outcome: producerAttemptRetried}) - kind, correction = producerAttemptModuleRetry, nil + kind = producerAttemptModuleRetry continue } + if output.Retry != nil && output.Retry.CorrectionGuidance != "" { + if _, err := moduleRetryCorrection(output); err != nil { + provenance = append(provenance, producerAttemptProvenance{Number: number, Kind: kind, Outcome: producerAttemptFailed}) + return failedProducerAttempt(provenance), err + } + } if output.Retry != nil { output.Diagnostics = append(output.Diagnostics, contracts.CloneProducerDiagnostics(output.Retry.FallbackDiagnostics)...) } @@ -269,6 +284,23 @@ func runProducerAttempts(ctx context.Context, config producerAttemptConfig, prod return failedProducerAttempt(provenance), errors.New("producer attempt budget was not exhausted deterministically") } +func moduleRetryCorrection(output producerAttemptOutput) (*contracts.SemanticCorrection, error) { + if output.Retry == nil || output.Retry.CorrectionGuidance == "" { + return nil, nil + } + if output.Candidate == nil { + return nil, errors.New("module retry correction guidance requires a model candidate") + } + if output.Candidate.Protocol != contracts.CorrectionProtocolSingleResponseV1 { + return nil, fmt.Errorf("module retry correction guidance requires protocol %q", contracts.CorrectionProtocolSingleResponseV1) + } + correction, err := contracts.NewSemanticCorrection(output.Candidate.Response, output.Retry.CorrectionGuidance) + if err != nil { + return nil, fmt.Errorf("construct module retry semantic correction: %w", err) + } + return correction, nil +} + func validateProducerAttemptDiagnostics(output producerAttemptOutput) error { if err := contracts.ValidateProducerDiagnostics(output.Diagnostics); err != nil { return fmt.Errorf("producer returned invalid diagnostics: %w", err) diff --git a/internal/framework/pipeline/producer_attempts_test.go b/internal/framework/pipeline/producer_attempts_test.go index f02f369f..8e3c9f2b 100644 --- a/internal/framework/pipeline/producer_attempts_test.go +++ b/internal/framework/pipeline/producer_attempts_test.go @@ -218,6 +218,9 @@ func TestRunProducerAttemptsUsesModuleRetryBudgetAndFallback(t *testing.T) { calls := 0 terminal, err := runProducerAttempts(context.Background(), producerAttemptConfig{Retries: 1, Policy: DefaultValidationPolicy()}, func(_ context.Context, request producerAttemptRequest) (producerAttemptOutput, error) { calls++ + if request.Correction != nil { + t.Fatalf("feedback-free module retry correction = %#v, want nil", request.Correction) + } if request.Number == 1 { return producerAttemptOutput{Value: "fallback", Retry: &producerRetryDirective{}}, nil } @@ -234,6 +237,58 @@ func TestRunProducerAttemptsUsesModuleRetryBudgetAndFallback(t *testing.T) { } }) + t.Run("feedback retry", func(t *testing.T) { + const ( + defective = `{"duplicate_groups":[{"candidate_ids":[1,99],"canonical_candidate_id":1}]}` + guidance = "Use only candidate IDs from the supplied candidate list. Return one complete corrected response." + ) + var observed *contracts.SemanticCorrection + terminal, err := runProducerAttempts(context.Background(), producerAttemptConfig{Retries: 1, Policy: DefaultValidationPolicy()}, func(_ context.Context, request producerAttemptRequest) (producerAttemptOutput, error) { + if request.Number == 1 { + return producerAttemptOutput{Value: "safe fallback", Candidate: attemptCandidate(t, defective), Retry: &producerRetryDirective{CorrectionGuidance: guidance}}, nil + } + observed = request.Correction + return producerAttemptOutput{Value: "corrected"}, nil + }, approveAttempt) + if err != nil { + t.Fatalf("runProducerAttempts() error = %v", err) + } + if terminal.Action != producerTerminalAccepted || terminal.Value != "corrected" { + t.Fatalf("terminal = %#v, want corrected accepted value", terminal) + } + if observed == nil || string(observed.AssistantResponse) != defective || observed.UserGuidance != guidance { + t.Fatalf("module retry correction = %#v, want exact latest response and guidance", observed) + } + if got := attemptKinds(terminal.Provenance); !reflect.DeepEqual(got, []producerAttemptKind{producerAttemptInitial, producerAttemptModuleRetry}) { + t.Fatalf("attempt kinds = %v", got) + } + }) + + t.Run("feedback-free retry clears prior correction", func(t *testing.T) { + terminal, err := runProducerAttempts(context.Background(), producerAttemptConfig{Retries: 2, Policy: DefaultValidationPolicy()}, func(_ context.Context, request producerAttemptRequest) (producerAttemptOutput, error) { + switch request.Number { + case 1: + return producerAttemptOutput{Value: "first fallback", Candidate: attemptCandidate(t, "first defective response"), Retry: &producerRetryDirective{CorrectionGuidance: "Correct the first response."}}, nil + case 2: + if request.Correction == nil || string(request.Correction.AssistantResponse) != "first defective response" { + t.Fatalf("second attempt correction = %#v", request.Correction) + } + return producerAttemptOutput{Value: "second fallback", Retry: &producerRetryDirective{}}, nil + case 3: + if request.Correction != nil { + t.Fatalf("third attempt retained stale correction %#v", request.Correction) + } + return producerAttemptOutput{Value: "accepted"}, nil + default: + t.Fatalf("unexpected producer attempt %d", request.Number) + return producerAttemptOutput{}, nil + } + }, approveAttempt) + if err != nil || terminal.Action != producerTerminalAccepted || terminal.Value != "accepted" { + t.Fatalf("terminal = %#v, error = %v", terminal, err) + } + }) + t.Run("fallback", func(t *testing.T) { fallbackDiagnostic := contracts.ProducerDiagnostic{Disposition: contracts.DiagnosticDispositionWarning, Category: contracts.DiagnosticCategoryFallback, ReasonCode: "fallback", OccurrenceCount: 1, Samples: []contracts.DiagnosticSample{{Scope: "fallback", Message: "fallback warning"}}} terminal, err := runProducerAttempts(context.Background(), producerAttemptConfig{Policy: DefaultValidationPolicy()}, func(context.Context, producerAttemptRequest) (producerAttemptOutput, error) { @@ -248,6 +303,24 @@ func TestRunProducerAttemptsUsesModuleRetryBudgetAndFallback(t *testing.T) { }) } +func TestRunProducerAttemptsRejectsModuleCorrectionWithoutModelCandidate(t *testing.T) { + producerCalls := 0 + validatorCalls := 0 + terminal, err := runProducerAttempts(context.Background(), producerAttemptConfig{Retries: 1, Policy: DefaultValidationPolicy()}, func(context.Context, producerAttemptRequest) (producerAttemptOutput, error) { + producerCalls++ + return producerAttemptOutput{Value: "safe", Retry: &producerRetryDirective{CorrectionGuidance: "Return a complete corrected response."}}, nil + }, func(context.Context, producerAttemptOutput) (validationReport, error) { + validatorCalls++ + return validationReport{}, nil + }) + if err == nil || !strings.Contains(err.Error(), "requires a model candidate") { + t.Fatalf("runProducerAttempts() error = %v, want model-candidate contract failure", err) + } + if terminal.Action != producerTerminalFailed || producerCalls != 1 || validatorCalls != 0 { + t.Fatalf("terminal = %#v, producer calls = %d, validator calls = %d", terminal, producerCalls, validatorCalls) + } +} + func TestRunProducerAttemptsRejectsInvalidDiagnosticsWithoutRetry(t *testing.T) { tests := []struct { name string diff --git a/internal/framework/pipeline/runner_normalize_retry_test.go b/internal/framework/pipeline/runner_normalize_retry_test.go index aa0daf9a..522fc365 100644 --- a/internal/framework/pipeline/runner_normalize_retry_test.go +++ b/internal/framework/pipeline/runner_normalize_retry_test.go @@ -153,6 +153,60 @@ func TestRunnerHandlesRetryableNormalizeFallbacks(t *testing.T) { } } +func TestRunnerForwardsModuleRequestedNormalizeCorrection(t *testing.T) { + const ( + defective = `{"duplicate_groups":[{"candidate_ids":[1,99],"canonical_candidate_id":1}]}` + guidance = "Duplicate group 1 must use only supplied candidate IDs. Return one complete corrected response." + ) + prepared := preparedAttemptDebugPipeline(t) + lane := &prepared.Steps[0].lanes[0] + lane.resolved.Normalize.Retries = 1 + var observed *contracts.SemanticCorrection + calls := 0 + lane.typed.normalize = func(_ context.Context, _ any, request contracts.TypedNormalizeRequest[any]) (erasedTypedResult, error) { + calls++ + if request.Correction != nil { + clone, err := contracts.CloneSemanticCorrection(request.Correction) + if err != nil { + return erasedTypedResult{}, err + } + observed = clone + } + if calls == 1 { + return erasedTypedResult{ + Value: codecNotes{Items: []string{"safe fallback"}}, + ModelCandidate: attemptCandidate(t, defective), + Retry: &contracts.NormalizeRetry{ + ReasonCode: "semantic_proposal_invalid", + Message: "operator-facing proposal diagnostic", + CorrectionGuidance: guidance, + }, + }, nil + } + return erasedTypedResult{Value: codecNotes{Items: []string{"corrected"}}}, nil + } + debug := newCapturedDebugRecorder() + output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Debug: debug}) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if calls != 2 || observed == nil || string(observed.AssistantResponse) != defective || observed.UserGuidance != guidance { + t.Fatalf("normalize calls = %d correction = %#v", calls, observed) + } + if len(output.NormalizeOutputs) != 1 || len(output.Rejected) != 0 { + t.Fatalf("run output = %#v, want corrected accepted output", output) + } + var retryDebug strings.Builder + for _, name := range debug.names() { + if strings.HasPrefix(name, "normalize/notes/attempt-") && strings.HasSuffix(name, ".json") { + retryDebug.Write(debug.json[name]) + } + } + if !strings.Contains(retryDebug.String(), `"correction_available":true`) || strings.Contains(retryDebug.String(), guidance) || strings.Contains(retryDebug.String(), defective) { + t.Fatalf("retry debug = %s, want safe correction metadata without content", retryDebug.String()) + } +} + func TestRunnerValidatesNormalizeRetryDiagnostics(t *testing.T) { const ( reasonSentinel = "reason-diagnostic-sentinel" @@ -160,9 +214,11 @@ func TestRunnerValidatesNormalizeRetryDiagnostics(t *testing.T) { ) reasonOverLimit := strings.Repeat("r", contracts.MaxNormalizeRetryReasonCodeBytes-len(reasonSentinel)) + reasonSentinel + "x" messageOverLimit := strings.Repeat("m", contracts.MaxNormalizeRetryMessageBytes-len(messageSentinel)) + messageSentinel + "x" + guidanceOverLimit := strings.Repeat("g", contracts.MaxNormalizeRetryCorrectionGuidanceBytes-len(messageSentinel)) + messageSentinel + "x" tests := []struct { name string retry contracts.NormalizeRetry + candidate *contracts.ModelCandidate wantError string hiddenValues []string }{ @@ -173,6 +229,15 @@ func TestRunnerValidatesNormalizeRetryDiagnostics(t *testing.T) { Message: strings.Repeat("m", contracts.MaxNormalizeRetryMessageBytes), }, }, + { + name: "accepts correction guidance byte limit with candidate", + retry: contracts.NormalizeRetry{ + ReasonCode: reasonSentinel, + Message: messageSentinel, + CorrectionGuidance: strings.Repeat("g", contracts.MaxNormalizeRetryCorrectionGuidanceBytes), + }, + candidate: attemptCandidate(t, `{"duplicate_groups":[]}`), + }, { name: "rejects oversized reason code", retry: contracts.NormalizeRetry{ @@ -209,6 +274,46 @@ func TestRunnerValidatesNormalizeRetryDiagnostics(t *testing.T) { wantError: "message has invalid UTF-8", hiddenValues: []string{reasonSentinel, messageSentinel}, }, + { + name: "rejects oversized correction guidance", + retry: contracts.NormalizeRetry{ + ReasonCode: reasonSentinel, + Message: messageSentinel, + CorrectionGuidance: guidanceOverLimit, + }, + wantError: "correction guidance exceeds maximum length", + hiddenValues: []string{reasonSentinel, messageSentinel}, + }, + { + name: "rejects invalid correction guidance UTF-8", + retry: contracts.NormalizeRetry{ + ReasonCode: reasonSentinel, + Message: messageSentinel, + CorrectionGuidance: messageSentinel + string([]byte{0xff}), + }, + wantError: "correction guidance has invalid UTF-8", + hiddenValues: []string{reasonSentinel, messageSentinel}, + }, + { + name: "rejects blank correction guidance", + retry: contracts.NormalizeRetry{ + ReasonCode: reasonSentinel, + Message: messageSentinel, + CorrectionGuidance: " \t\n ", + }, + wantError: "correction guidance is blank", + hiddenValues: []string{reasonSentinel, messageSentinel}, + }, + { + name: "rejects correction guidance without candidate", + retry: contracts.NormalizeRetry{ + ReasonCode: reasonSentinel, + Message: messageSentinel, + CorrectionGuidance: "Return one complete corrected response.", + }, + wantError: "requires a model candidate", + hiddenValues: []string{reasonSentinel, messageSentinel}, + }, { name: "rejects blank reason code", retry: contracts.NormalizeRetry{ @@ -233,7 +338,7 @@ func TestRunnerValidatesNormalizeRetryDiagnostics(t *testing.T) { t.Run(tc.name, func(t *testing.T) { prepared := preparedAttemptDebugPipeline(t) prepared.Steps[0].lanes[0].typed.normalize = func(context.Context, any, contracts.TypedNormalizeRequest[any]) (erasedTypedResult, error) { - return erasedTypedResult{Value: codecNotes{Items: []string{"safe"}}, Retry: &tc.retry}, nil + return erasedTypedResult{Value: codecNotes{Items: []string{"safe"}}, Retry: &tc.retry, ModelCandidate: tc.candidate}, nil } debug := newCapturedDebugRecorder() output, err := New().Run(context.Background(), RunInput{Prepared: prepared, RawInput: []byte("input"), Debug: debug}) diff --git a/internal/framework/pipeline/runner_typed.go b/internal/framework/pipeline/runner_typed.go index 24118923..72679347 100644 --- a/internal/framework/pipeline/runner_typed.go +++ b/internal/framework/pipeline/runner_typed.go @@ -451,8 +451,8 @@ func (r *Runner) runNormalizeStage(ctx context.Context, input RunInput, checkpoi return producerAttemptOutput{}, terminal.record(map[string]any{"output": debugCheckpointArtifact(serializedCandidate)}, fmt.Errorf("normalize lane %q returned invalid retry directive: %w", lane.ID, err)) } anotherAttempt := request.Number <= lane.Normalize.Retries - attemptValue.retry = map[string]any{"reason_code": result.Retry.ReasonCode, "message": result.Retry.Message, "another_attempt": anotherAttempt, "fallback_accepted": !anotherAttempt} - directive = &producerRetryDirective{FallbackDiagnostics: contracts.CloneProducerDiagnostics(result.Retry.FallbackDiagnostics)} + attemptValue.retry = map[string]any{"reason_code": result.Retry.ReasonCode, "message": result.Retry.Message, "correction_available": result.Retry.CorrectionGuidance != "", "another_attempt": anotherAttempt, "fallback_accepted": !anotherAttempt} + directive = &producerRetryDirective{CorrectionGuidance: result.Retry.CorrectionGuidance, FallbackDiagnostics: contracts.CloneProducerDiagnostics(result.Retry.FallbackDiagnostics)} if anotherAttempt { payload := map[string]any{"output": debugCheckpointArtifact(serializedCandidate), "retry": attemptValue.retry} if debugErr := terminal.record(payload, nil); debugErr != nil { diff --git a/internal/framework/semanticreconcile/identity.go b/internal/framework/semanticreconcile/identity.go index 86bf35c4..cdc78e6e 100644 --- a/internal/framework/semanticreconcile/identity.go +++ b/internal/framework/semanticreconcile/identity.go @@ -13,7 +13,7 @@ import ( ) // Policy identifies the framework-owned reconciliation and assessment rules. -const Policy = "semantic_reconciliation.v1" +const Policy = "semantic_reconciliation.v2" var _ contracts.ManifestMetadataProvider = (*Engine)(nil) var _ pipeline.CheckpointFingerprintProvider = (*Engine)(nil) diff --git a/internal/framework/semanticreconcile/proposal.go b/internal/framework/semanticreconcile/proposal.go index f9d9a5bb..a33fc115 100644 --- a/internal/framework/semanticreconcile/proposal.go +++ b/internal/framework/semanticreconcile/proposal.go @@ -3,6 +3,10 @@ package semanticreconcile import ( "fmt" "sort" + "strings" + "unicode/utf8" + + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" ) // ProposalResponse is the complete private structured response contract. @@ -31,6 +35,28 @@ const ( IssueOverlappingMember IssueCategory = "overlapping_member" ) +var allIssueCategories = []IssueCategory{ + IssueMemberNonPositive, + IssueMemberUnknown, + IssueRepeatedMember, + IssueFewerThanTwoMembers, + IssueCanonicalNonPositive, + IssueCanonicalUnknown, + IssueCanonicalNotMember, + IssueOverlappingMember, +} + +var issueCorrectionProse = map[IssueCategory]string{ + IssueMemberNonPositive: "Use only positive candidate IDs from the supplied candidate list.", + IssueMemberUnknown: "Remove every candidate ID that is not present in the supplied candidate list.", + IssueRepeatedMember: "List each candidate ID at most once within the duplicate group.", + IssueFewerThanTwoMembers: "Include at least two distinct candidate IDs, or omit the duplicate group.", + IssueCanonicalNonPositive: "Choose a positive canonical_candidate_id from the supplied candidate list.", + IssueCanonicalUnknown: "Choose canonical_candidate_id from the supplied candidate list.", + IssueCanonicalNotMember: "Make canonical_candidate_id one of the candidate_ids in the same duplicate group.", + IssueOverlappingMember: "Place each candidate ID in at most one duplicate group.", +} + // Issue identifies an unsafe proposal category at its original response group // index without prescribing caller diagnostic text. type Issue struct { @@ -38,8 +64,8 @@ type Issue struct { Category IssueCategory } -// IssueDetails renders stable, domain-neutral proposal diagnostics for an -// adapter's retry message. +// IssueDetails renders stable, domain-neutral proposal diagnostics for +// operators and debug records. Its internal categories are not model guidance. func IssueDetails(issues []Issue) []string { details := make([]string, len(issues)) for index, issue := range issues { @@ -48,6 +74,94 @@ func IssueDetails(issues []Issue) []string { return details } +// CorrectionDetails translates proposal issues into stable model-facing prose. +// The response-local group ordinals help the model find the defective group in +// the exact response appended to the correction request. +func CorrectionDetails(issues []Issue) ([]string, error) { + groupsByCategory := make(map[IssueCategory][]int) + seen := make(map[IssueCategory]map[int]struct{}) + for _, issue := range issues { + if issue.GroupIndex < 0 { + return nil, fmt.Errorf("semantic reconciliation issue group index must not be negative") + } + if _, exists := issueCorrectionProse[issue.Category]; !exists { + return nil, fmt.Errorf("semantic reconciliation issue category %q has no correction guidance", issue.Category) + } + if seen[issue.Category] == nil { + seen[issue.Category] = make(map[int]struct{}) + } + if _, exists := seen[issue.Category][issue.GroupIndex]; exists { + continue + } + seen[issue.Category][issue.GroupIndex] = struct{}{} + groupsByCategory[issue.Category] = append(groupsByCategory[issue.Category], issue.GroupIndex) + } + + details := make([]string, 0, len(groupsByCategory)) + for _, category := range allIssueCategories { + groups := groupsByCategory[category] + if len(groups) == 0 { + continue + } + sort.Ints(groups) + details = append(details, fmt.Sprintf("%s: %s", correctionGroupLabel(groups), issueCorrectionProse[category])) + } + return details, nil +} + +// CorrectionGuidance builds one bounded request for a complete corrected +// proposal. Additional details let a typed owner append a domain rule without +// weakening or duplicating the shared protocol guidance. +func CorrectionGuidance(issues []Issue, additionalDetails ...string) (string, error) { + details, err := CorrectionDetails(issues) + if err != nil { + return "", err + } + for _, detail := range additionalDetails { + if !utf8.ValidString(detail) { + return "", fmt.Errorf("semantic reconciliation additional correction detail has invalid UTF-8") + } + detail = strings.TrimSpace(detail) + if detail == "" { + return "", fmt.Errorf("semantic reconciliation additional correction detail must not be blank") + } + details = append(details, detail) + } + if len(details) == 0 { + return "", fmt.Errorf("semantic reconciliation correction guidance requires at least one detail") + } + + parts := make([]string, 0, len(details)+2) + parts = append(parts, "The previous semantic-duplicate proposal was invalid.") + parts = append(parts, details...) + parts = append(parts, "Return one complete corrected JSON response that follows the original instructions; do not return a patch or commentary.") + guidance := strings.Join(parts, " ") + if len(guidance) > contracts.MaxNormalizeRetryCorrectionGuidanceBytes { + return "", fmt.Errorf("semantic reconciliation correction guidance exceeds maximum length") + } + return guidance, nil +} + +func correctionGroupLabel(groupIndexes []int) string { + const maximumDisplayedGroups = 12 + displayed := groupIndexes + if len(displayed) > maximumDisplayedGroups { + displayed = displayed[:maximumDisplayedGroups] + } + ordinals := make([]string, len(displayed)) + for index, groupIndex := range displayed { + ordinals[index] = fmt.Sprintf("%d", groupIndex+1) + } + if len(groupIndexes) == 1 { + return "Duplicate group " + ordinals[0] + } + label := "Duplicate groups " + strings.Join(ordinals, ", ") + if omitted := len(groupIndexes) - len(displayed); omitted > 0 { + label += fmt.Sprintf(", and %d additional affected group(s)", omitted) + } + return label +} + // PlanGroup identifies one validated group using original candidate positions. type PlanGroup struct { memberPositions []int diff --git a/internal/framework/semanticreconcile/proposal_test.go b/internal/framework/semanticreconcile/proposal_test.go index ae76b62e..0e97efae 100644 --- a/internal/framework/semanticreconcile/proposal_test.go +++ b/internal/framework/semanticreconcile/proposal_test.go @@ -2,9 +2,11 @@ package semanticreconcile import ( "reflect" + "strings" "testing" "gitea.maximumdirect.net/eric/notarius/internal/core/source" + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" ) func TestAssessProducesAStableOriginalPositionPlan(t *testing.T) { @@ -50,6 +52,72 @@ func TestIssueDetailsPreservesIssueOrder(t *testing.T) { } } +func TestCorrectionGuidanceCoversEveryIssueCategoryWithoutExposingInternalLabels(t *testing.T) { + if len(issueCorrectionProse) != len(allIssueCategories) { + t.Fatalf("correction prose entries = %d, categories = %d", len(issueCorrectionProse), len(allIssueCategories)) + } + issues := make([]Issue, len(allIssueCategories)) + seen := make(map[IssueCategory]struct{}, len(allIssueCategories)) + for index, category := range allIssueCategories { + if _, duplicate := seen[category]; duplicate { + t.Fatalf("duplicate authoritative issue category %q", category) + } + seen[category] = struct{}{} + if strings.TrimSpace(issueCorrectionProse[category]) == "" { + t.Fatalf("issue category %q has no model-facing prose", category) + } + issues[index] = Issue{GroupIndex: index, Category: category} + } + + details, err := CorrectionDetails(issues) + if err != nil { + t.Fatalf("CorrectionDetails() error = %v", err) + } + guidance, err := CorrectionGuidance(issues) + if err != nil { + t.Fatalf("CorrectionGuidance() error = %v", err) + } + if len(details) != len(allIssueCategories) || !strings.Contains(guidance, "Duplicate group 1") || !strings.Contains(guidance, "complete corrected JSON response") || len(guidance) > contracts.MaxNormalizeRetryCorrectionGuidanceBytes { + t.Fatalf("correction details = %#v guidance = %q", details, guidance) + } + for _, category := range allIssueCategories { + if strings.Contains(guidance, string(category)) { + t.Fatalf("model guidance exposed internal category %q: %q", category, guidance) + } + } +} + +func TestCorrectionDetailsDeduplicatesAndBoundsAffectedGroupLists(t *testing.T) { + issues := make([]Issue, 0, 257) + for group := 0; group < 256; group++ { + issues = append(issues, Issue{GroupIndex: group, Category: IssueMemberUnknown}) + } + issues = append(issues, Issue{GroupIndex: 0, Category: IssueMemberUnknown}) + + details, err := CorrectionDetails(issues) + if err != nil { + t.Fatalf("CorrectionDetails() error = %v", err) + } + if len(details) != 1 || !strings.Contains(details[0], "additional affected group") || strings.Count(details[0], "Duplicate groups") != 1 { + t.Fatalf("CorrectionDetails() = %#v, want one bounded grouped detail", details) + } + guidance, err := CorrectionGuidance(issues) + if err != nil || len(guidance) > contracts.MaxNormalizeRetryCorrectionGuidanceBytes { + t.Fatalf("CorrectionGuidance() = %q, %v", guidance, err) + } +} + +func TestCorrectionDetailsRejectsUnknownOrInvalidIssues(t *testing.T) { + for _, issues := range [][]Issue{ + {{GroupIndex: 0, Category: "future_unmapped_category"}}, + {{GroupIndex: -1, Category: IssueMemberUnknown}}, + } { + if details, err := CorrectionDetails(issues); err == nil || details != nil { + t.Fatalf("CorrectionDetails(%#v) = %#v, %v; want fail-closed error", issues, details, err) + } + } +} + func TestAssessRejectsEveryUnsafeLocalGroupShape(t *testing.T) { preparation := proposalPreparation(t) tests := []struct { diff --git a/internal/modules/dnd/normalize/itemregistry/normalizer.go b/internal/modules/dnd/normalize/itemregistry/normalizer.go index 081c3db7..be820148 100644 --- a/internal/modules/dnd/normalize/itemregistry/normalizer.go +++ b/internal/modules/dnd/normalize/itemregistry/normalizer.go @@ -23,7 +23,7 @@ const ( Key = "dnd/item-registry" PromptID = "dnd.item_registry.normalize" PromptVersion = "v1" - normalizationPolicy = "dnd.item_registry.normalize.v3" + normalizationPolicy = "dnd.item_registry.normalize.v4" NormalizationPolicy = normalizationPolicy ReasonCodeItemFieldsNormalized = "item_fields_normalized" @@ -129,7 +129,7 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize case semanticreconcile.SkippedInsufficientCandidates: return normalizationResult(deterministic, findings, nil, nil) case semanticreconcile.SkippedLimitExceeded: - return fallbackResult(deterministic, findings, nil, semanticFallbackFinding(-1)) + return fallbackResult(deterministic, findings, nil, semanticFallbackFinding(-1, "the candidate set exceeded the configured semantic-reconciliation limits")) case semanticreconcile.RetryableInvalidStructuredOutput: return n.invalidStructuredResult(deterministic, findings) case semanticreconcile.Complete, semanticreconcile.RetryableDiscardedProposalGroups: @@ -150,22 +150,38 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize } func (n *Normalizer) invalidStructuredResult(value dnd.ItemRegistry, findings []diagnostics.Finding) (contracts.TypedNormalizeResult[dnd.ItemRegistry], error) { - return retryResultWithFallback(value, findings, nil, nil, ReasonCodeItemSemanticRetryProposalInvalid, "semantic proposal requires retry: invalid structured output", semanticFallbackFinding(-1)) + return retryResultWithFallback(value, findings, nil, nil, ReasonCodeItemSemanticRetryProposalInvalid, "", "semantic proposal requires retry: invalid structured output", semanticFallbackFinding(-1, "the final model response did not satisfy the structured-output contract")) } func retryResult(value dnd.ItemRegistry, findings, advisoryFindings []diagnostics.Finding, reconciliation semanticreconcile.Result, rejectedGroups int) (contracts.TypedNormalizeResult[dnd.ItemRegistry], error) { - details := semanticreconcile.IssueDetails(reconciliation.Issues()) + issues := reconciliation.Issues() + operatorDetails := semanticreconcile.IssueDetails(issues) + correctionDetails, err := semanticreconcile.CorrectionDetails(issues) + if err != nil { + return contracts.TypedNormalizeResult[dnd.ItemRegistry]{}, normalizerErrorf("build semantic correction details: %w", err) + } + var additionalGuidance []string if rejectedGroups > 0 { - details = append(details, "currency may only be consolidated with aliases of one denomination") + const currencyGuidance = "Consolidate currency only with aliases of the same denomination; keep different denominations separate and do not combine currency with non-currency items." + operatorDetails = append(operatorDetails, "currency may only be consolidated with aliases of one denomination") + correctionDetails = append(correctionDetails, currencyGuidance) + additionalGuidance = append(additionalGuidance, currencyGuidance) + } + correctionGuidance, err := semanticreconcile.CorrectionGuidance(issues, additionalGuidance...) + if err != nil { + return contracts.TypedNormalizeResult[dnd.ItemRegistry]{}, normalizerErrorf("build semantic correction guidance: %w", err) } discardedGroups := reconciliation.DiscardedGroupCount() + rejectedGroups - return retryResultWithFallback(value, findings, advisoryFindings, reconciliation.ModelCandidate(), ReasonCodeItemSemanticRetryProposalInvalid, diagnostics.Aggregate("semantic proposal requires retry", details), semanticFallbackFinding(discardedGroups)) + return retryResultWithFallback(value, findings, advisoryFindings, reconciliation.ModelCandidate(), ReasonCodeItemSemanticRetryProposalInvalid, correctionGuidance, diagnostics.Aggregate("semantic proposal requires retry", operatorDetails), semanticFallbackFinding(discardedGroups, correctionDetails...)) } -func semanticFallbackFinding(discarded int) diagnostics.Finding { - message := "semantic proposal could not be applied" +func semanticFallbackFinding(discarded int, details ...string) diagnostics.Finding { + message := "semantic reconciliation could not be completed; all affected candidates were preserved separately" if discarded >= 0 { - message = fmt.Sprintf("%d proposal group(s) omitted after semantic proposal retry exhaustion", discarded) + message = fmt.Sprintf("%d proposal group(s) omitted from the final semantic proposal; all affected candidates were preserved separately", discarded) + } + if len(details) > 0 { + message = diagnostics.Aggregate(message, details) } return diagnostics.Finding{Scope: "items", ReasonCode: ReasonCodeItemSemanticReconciliationExhausted, Message: message} } @@ -196,7 +212,7 @@ func fallbackResult(value dnd.ItemRegistry, findings, advisoryFindings []diagnos return result, nil } -func retryResultWithFallback(value dnd.ItemRegistry, findings, advisoryFindings []diagnostics.Finding, candidate *contracts.ModelCandidate, reasonCode, message string, fallback diagnostics.Finding) (contracts.TypedNormalizeResult[dnd.ItemRegistry], error) { +func retryResultWithFallback(value dnd.ItemRegistry, findings, advisoryFindings []diagnostics.Finding, candidate *contracts.ModelCandidate, reasonCode, correctionGuidance, message string, fallback diagnostics.Finding) (contracts.TypedNormalizeResult[dnd.ItemRegistry], error) { result, err := normalizationResult(value, findings, advisoryFindings, candidate) if err != nil { return contracts.TypedNormalizeResult[dnd.ItemRegistry]{}, err @@ -205,7 +221,7 @@ func retryResultWithFallback(value dnd.ItemRegistry, findings, advisoryFindings if err != nil { return contracts.TypedNormalizeResult[dnd.ItemRegistry]{}, normalizerErrorf("collect fallback diagnostic: %w", err) } - result.Retry = &contracts.NormalizeRetry{ReasonCode: reasonCode, Message: message, FallbackDiagnostics: fallbackGroups} + result.Retry = &contracts.NormalizeRetry{ReasonCode: reasonCode, Message: message, CorrectionGuidance: correctionGuidance, FallbackDiagnostics: fallbackGroups} return result, nil } diff --git a/internal/modules/dnd/normalize/itemregistry/normalizer_test.go b/internal/modules/dnd/normalize/itemregistry/normalizer_test.go index 006e8de0..41ecd669 100644 --- a/internal/modules/dnd/normalize/itemregistry/normalizer_test.go +++ b/internal/modules/dnd/normalize/itemregistry/normalizer_test.go @@ -289,7 +289,10 @@ func TestNormalizeAppliesIndependentGroupAndCountsAllOmissions(t *testing.T) { if !hasDiagnostic(result.Diagnostics, ReasonCodeDuplicateItemCollapsed, contracts.DiagnosticDispositionObservation) || !hasDiagnostic(result.Diagnostics, ReasonCodeItemSemanticProposalInvalid, contracts.DiagnosticDispositionAdvisory) { t.Fatalf("diagnostics = %#v, want accepted and guarded-group diagnostics", result.Diagnostics) } - if len(result.Retry.FallbackDiagnostics) != 1 || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "2 proposal group(s)") { + if !strings.Contains(result.Retry.CorrectionGuidance, "aliases of the same denomination") || !strings.Contains(result.Retry.CorrectionGuidance, "do not combine currency with non-currency items") || !strings.Contains(result.Retry.CorrectionGuidance, "supplied candidate list") || strings.Contains(result.Retry.CorrectionGuidance, "member_unknown") || strings.Contains(result.Retry.CorrectionGuidance, ReasonCodeItemSemanticRetryProposalInvalid) { + t.Fatalf("correction guidance = %q, want shared and item-specific prose only", result.Retry.CorrectionGuidance) + } + if len(result.Retry.FallbackDiagnostics) != 1 || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "2 proposal group(s)") || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "preserved separately") || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "same denomination") { t.Fatalf("retry = %#v, want one guarded and one malformed group counted", result.Retry) } } @@ -321,6 +324,9 @@ func TestNormalizeRetryFallbackErrorsAndIdempotence(t *testing.T) { if err != nil || invalid.Retry == nil || invalid.Retry.ReasonCode != ReasonCodeItemSemanticRetryProposalInvalid { t.Fatalf("invalid result = %#v, %v", invalid, err) } + if invalid.Retry.CorrectionGuidance != "" || !strings.Contains(invalid.Retry.FallbackDiagnostics[0].Samples[0].Message, "structured-output contract") { + t.Fatalf("invalid structured retry = %#v, want feedback-free retry with final-cause fallback", invalid.Retry) + } _, err = newNormalizer(t, &recordingNormalizerClient{err: errors.New("provider unavailable")}).Normalize(context.Background(), normalizeRequestWithSource(input, doc)) if err == nil || !strings.Contains(err.Error(), "provider unavailable") { t.Fatalf("provider error = %v", err) diff --git a/internal/modules/dnd/normalize/locationregistry/normalizer.go b/internal/modules/dnd/normalize/locationregistry/normalizer.go index 59168d03..6dceb131 100644 --- a/internal/modules/dnd/normalize/locationregistry/normalizer.go +++ b/internal/modules/dnd/normalize/locationregistry/normalizer.go @@ -24,7 +24,7 @@ const ( Key = "dnd/location-registry" PromptID = "dnd.location_registry.normalize" PromptVersion = "v1" - normalizationPolicy = "dnd.location_registry.normalize.v3" + normalizationPolicy = "dnd.location_registry.normalize.v4" NormalizationPolicy = normalizationPolicy ReasonCodeLocationFieldsNormalized = "location_fields_normalized" @@ -129,7 +129,7 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize case semanticreconcile.SkippedInsufficientCandidates: return normalizationResult(deterministic, findings, nil) case semanticreconcile.SkippedLimitExceeded: - return fallbackResult(deterministic, findings, semanticFallbackFinding(-1)) + return fallbackResult(deterministic, findings, semanticFallbackFinding(-1, "the candidate set exceeded the configured semantic-reconciliation limits")) case semanticreconcile.RetryableInvalidStructuredOutput: return n.invalidStructuredResult(deterministic, findings) case semanticreconcile.Complete, semanticreconcile.RetryableDiscardedProposalGroups: @@ -149,17 +149,29 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize } func (n *Normalizer) invalidStructuredResult(value dnd.LocationRegistry, findings []diagnostics.Finding) (contracts.TypedNormalizeResult[dnd.LocationRegistry], error) { - return retryResultWithFallback(value, findings, nil, "semantic proposal requires retry: invalid structured output", semanticFallbackFinding(-1)) + return retryResultWithFallback(value, findings, nil, "", "semantic proposal requires retry: invalid structured output", semanticFallbackFinding(-1, "the final model response did not satisfy the structured-output contract")) } func retryResult(value dnd.LocationRegistry, findings []diagnostics.Finding, reconciliation semanticreconcile.Result) (contracts.TypedNormalizeResult[dnd.LocationRegistry], error) { - return retryResultWithFallback(value, findings, reconciliation.ModelCandidate(), diagnostics.Aggregate("semantic proposal requires retry", semanticreconcile.IssueDetails(reconciliation.Issues())), semanticFallbackFinding(reconciliation.DiscardedGroupCount())) + issues := reconciliation.Issues() + correctionDetails, err := semanticreconcile.CorrectionDetails(issues) + if err != nil { + return contracts.TypedNormalizeResult[dnd.LocationRegistry]{}, normalizerErrorf("build semantic correction details: %w", err) + } + correctionGuidance, err := semanticreconcile.CorrectionGuidance(issues) + if err != nil { + return contracts.TypedNormalizeResult[dnd.LocationRegistry]{}, normalizerErrorf("build semantic correction guidance: %w", err) + } + return retryResultWithFallback(value, findings, reconciliation.ModelCandidate(), correctionGuidance, diagnostics.Aggregate("semantic proposal requires retry", semanticreconcile.IssueDetails(issues)), semanticFallbackFinding(reconciliation.DiscardedGroupCount(), correctionDetails...)) } -func semanticFallbackFinding(discarded int) diagnostics.Finding { - message := "semantic proposal could not be applied" +func semanticFallbackFinding(discarded int, details ...string) diagnostics.Finding { + message := "semantic reconciliation could not be completed; all affected candidates were preserved separately" if discarded >= 0 { - message = fmt.Sprintf("%d proposal group(s) omitted after semantic proposal retry exhaustion", discarded) + message = fmt.Sprintf("%d proposal group(s) omitted from the final semantic proposal; all affected candidates were preserved separately", discarded) + } + if len(details) > 0 { + message = diagnostics.Aggregate(message, details) } return diagnostics.Finding{Scope: "locations", ReasonCode: ReasonCodeLocationSemanticReconciliationExhausted, Message: message} } @@ -185,7 +197,7 @@ func fallbackResult(value dnd.LocationRegistry, findings []diagnostics.Finding, return result, nil } -func retryResultWithFallback(value dnd.LocationRegistry, findings []diagnostics.Finding, candidate *contracts.ModelCandidate, message string, fallback diagnostics.Finding) (contracts.TypedNormalizeResult[dnd.LocationRegistry], error) { +func retryResultWithFallback(value dnd.LocationRegistry, findings []diagnostics.Finding, candidate *contracts.ModelCandidate, correctionGuidance, message string, fallback diagnostics.Finding) (contracts.TypedNormalizeResult[dnd.LocationRegistry], error) { result, err := normalizationResult(value, findings, candidate) if err != nil { return contracts.TypedNormalizeResult[dnd.LocationRegistry]{}, err @@ -194,7 +206,7 @@ func retryResultWithFallback(value dnd.LocationRegistry, findings []diagnostics. if err != nil { return contracts.TypedNormalizeResult[dnd.LocationRegistry]{}, normalizerErrorf("collect fallback diagnostic: %w", err) } - result.Retry = &contracts.NormalizeRetry{ReasonCode: ReasonCodeLocationSemanticProposalInvalid, Message: message, FallbackDiagnostics: fallbackGroups} + result.Retry = &contracts.NormalizeRetry{ReasonCode: ReasonCodeLocationSemanticProposalInvalid, Message: message, CorrectionGuidance: correctionGuidance, FallbackDiagnostics: fallbackGroups} return result, nil } diff --git a/internal/modules/dnd/normalize/locationregistry/normalizer_test.go b/internal/modules/dnd/normalize/locationregistry/normalizer_test.go index d5f9eea9..e884a781 100644 --- a/internal/modules/dnd/normalize/locationregistry/normalizer_test.go +++ b/internal/modules/dnd/normalize/locationregistry/normalizer_test.go @@ -128,7 +128,10 @@ func TestNormalizeRejectsUnsafeAndOverlappingGroupsWithoutLosingCandidates(t *te if err != nil || result.Retry == nil || len(result.Value.Locations) != 3 || !strings.Contains(result.Retry.Message, "overlapping_member") { t.Fatalf("Normalize() = %#v, %v; want safe retry fallback", result, err) } - if len(result.Retry.FallbackDiagnostics) != 1 || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "2 proposal group") { + if !strings.Contains(result.Retry.CorrectionGuidance, "Duplicate groups 1, 2") || !strings.Contains(result.Retry.CorrectionGuidance, "at most one duplicate group") || strings.Contains(result.Retry.CorrectionGuidance, "overlapping_member") { + t.Fatalf("correction guidance = %q, want model-facing overlap explanation", result.Retry.CorrectionGuidance) + } + if len(result.Retry.FallbackDiagnostics) != 1 || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "2 proposal group") || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "preserved separately") { t.Fatalf("fallback diagnostics = %#v", result.Retry.FallbackDiagnostics) } } @@ -188,6 +191,9 @@ func TestNormalizeHandlesRetryFallbackAndErrors(t *testing.T) { if err != nil || invalid.Retry == nil || invalid.Retry.ReasonCode != ReasonCodeLocationSemanticProposalInvalid { t.Fatalf("invalid result = %#v, %v", invalid, err) } + if invalid.Retry.CorrectionGuidance != "" || !strings.Contains(invalid.Retry.FallbackDiagnostics[0].Samples[0].Message, "structured-output contract") || !strings.Contains(invalid.Retry.FallbackDiagnostics[0].Samples[0].Message, "preserved separately") { + t.Fatalf("invalid structured retry = %#v, want feedback-free retry and informative fallback", invalid.Retry) + } _, err = newNormalizer(t, &recordingLocationNormalizerClient{err: errors.New("provider unavailable")}).Normalize(context.Background(), normalizeRequestWithSource(input, doc)) if err == nil || !strings.Contains(err.Error(), "provider unavailable") { t.Fatalf("provider error = %v", err) diff --git a/internal/modules/dnd/normalize/npcregistry/normalizer.go b/internal/modules/dnd/normalize/npcregistry/normalizer.go index c4b64ffb..c641313f 100644 --- a/internal/modules/dnd/normalize/npcregistry/normalizer.go +++ b/internal/modules/dnd/normalize/npcregistry/normalizer.go @@ -23,7 +23,7 @@ const ( Key = "dnd/npc-registry" PromptID = "dnd.npc_registry.normalize" PromptVersion = "v1" - normalizationPolicy = "dnd.npc_registry.normalize.v5" + normalizationPolicy = "dnd.npc_registry.normalize.v6" NormalizationPolicy = normalizationPolicy ReasonCodeNPCFieldsNormalized = "npc_fields_normalized" @@ -128,7 +128,7 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize case semanticreconcile.SkippedInsufficientCandidates: return normalizationResult(deterministic, findings, nil) case semanticreconcile.SkippedLimitExceeded: - return fallbackResult(deterministic, findings, semanticFallbackFinding(-1)) + return fallbackResult(deterministic, findings, semanticFallbackFinding(-1, "the candidate set exceeded the configured semantic-reconciliation limits")) case semanticreconcile.RetryableInvalidStructuredOutput: return n.invalidStructuredResult(deterministic, findings) case semanticreconcile.Complete, semanticreconcile.RetryableDiscardedProposalGroups: @@ -148,17 +148,29 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize } func (n *Normalizer) invalidStructuredResult(value dnd.NPCRegistry, findings []diagnostics.Finding) (contracts.TypedNormalizeResult[dnd.NPCRegistry], error) { - return retryResultWithFallback(value, findings, nil, "semantic proposal requires retry: invalid structured output", semanticFallbackFinding(-1)) + return retryResultWithFallback(value, findings, nil, "", "semantic proposal requires retry: invalid structured output", semanticFallbackFinding(-1, "the final model response did not satisfy the structured-output contract")) } func retryResult(value dnd.NPCRegistry, findings []diagnostics.Finding, reconciliation semanticreconcile.Result) (contracts.TypedNormalizeResult[dnd.NPCRegistry], error) { - return retryResultWithFallback(value, findings, reconciliation.ModelCandidate(), diagnostics.Aggregate("semantic proposal requires retry", semanticreconcile.IssueDetails(reconciliation.Issues())), semanticFallbackFinding(reconciliation.DiscardedGroupCount())) + issues := reconciliation.Issues() + correctionDetails, err := semanticreconcile.CorrectionDetails(issues) + if err != nil { + return contracts.TypedNormalizeResult[dnd.NPCRegistry]{}, normalizerErrorf("build semantic correction details: %w", err) + } + correctionGuidance, err := semanticreconcile.CorrectionGuidance(issues) + if err != nil { + return contracts.TypedNormalizeResult[dnd.NPCRegistry]{}, normalizerErrorf("build semantic correction guidance: %w", err) + } + return retryResultWithFallback(value, findings, reconciliation.ModelCandidate(), correctionGuidance, diagnostics.Aggregate("semantic proposal requires retry", semanticreconcile.IssueDetails(issues)), semanticFallbackFinding(reconciliation.DiscardedGroupCount(), correctionDetails...)) } -func semanticFallbackFinding(discardedGroups int) diagnostics.Finding { - message := "semantic proposal could not be applied" +func semanticFallbackFinding(discardedGroups int, details ...string) diagnostics.Finding { + message := "semantic reconciliation could not be completed; all affected candidates were preserved separately" if discardedGroups >= 0 { - message = fmt.Sprintf("%d proposal group(s) omitted after semantic proposal retry exhaustion", discardedGroups) + message = fmt.Sprintf("%d proposal group(s) omitted from the final semantic proposal; all affected candidates were preserved separately", discardedGroups) + } + if len(details) > 0 { + message = diagnostics.Aggregate(message, details) } return diagnostics.Finding{Scope: "npcs", ReasonCode: ReasonCodeNPCSemanticReconciliationExhausted, Message: message} } @@ -184,7 +196,7 @@ func fallbackResult(value dnd.NPCRegistry, findings []diagnostics.Finding, fallb return result, nil } -func retryResultWithFallback(value dnd.NPCRegistry, findings []diagnostics.Finding, candidate *contracts.ModelCandidate, message string, fallback diagnostics.Finding) (contracts.TypedNormalizeResult[dnd.NPCRegistry], error) { +func retryResultWithFallback(value dnd.NPCRegistry, findings []diagnostics.Finding, candidate *contracts.ModelCandidate, correctionGuidance, message string, fallback diagnostics.Finding) (contracts.TypedNormalizeResult[dnd.NPCRegistry], error) { result, err := normalizationResult(value, findings, candidate) if err != nil { return contracts.TypedNormalizeResult[dnd.NPCRegistry]{}, err @@ -193,7 +205,7 @@ func retryResultWithFallback(value dnd.NPCRegistry, findings []diagnostics.Findi if err != nil { return contracts.TypedNormalizeResult[dnd.NPCRegistry]{}, normalizerErrorf("collect fallback diagnostic: %w", err) } - result.Retry = &contracts.NormalizeRetry{ReasonCode: ReasonCodeNPCSemanticProposalInvalid, Message: message, FallbackDiagnostics: fallbackGroups} + result.Retry = &contracts.NormalizeRetry{ReasonCode: ReasonCodeNPCSemanticProposalInvalid, Message: message, CorrectionGuidance: correctionGuidance, FallbackDiagnostics: fallbackGroups} return result, nil } diff --git a/internal/modules/dnd/normalize/npcregistry/semantic_normalizer_test.go b/internal/modules/dnd/normalize/npcregistry/semantic_normalizer_test.go index 5e7867eb..e4a9a6ee 100644 --- a/internal/modules/dnd/normalize/npcregistry/semantic_normalizer_test.go +++ b/internal/modules/dnd/normalize/npcregistry/semantic_normalizer_test.go @@ -115,10 +115,13 @@ func TestNormalizeUnsafeProposalReturnsSafeRetryFallback(t *testing.T) { if result.Retry.ReasonCode != ReasonCodeNPCSemanticProposalInvalid || !strings.Contains(result.Retry.Message, "group 1: member_unknown") { t.Fatalf("retry = %#v, want bounded invalid-proposal diagnostics", result.Retry) } + if !strings.Contains(result.Retry.CorrectionGuidance, "Duplicate group 2") || !strings.Contains(result.Retry.CorrectionGuidance, "supplied candidate list") || strings.Contains(result.Retry.CorrectionGuidance, "member_unknown") || strings.Contains(result.Retry.CorrectionGuidance, ReasonCodeNPCSemanticProposalInvalid) { + t.Fatalf("correction guidance = %q, want contextual prose without internal labels", result.Retry.CorrectionGuidance) + } if len(result.Value.NPCs) != 2 || result.Value.NPCs[0].Name != "Mira Thorn" || result.Value.NPCs[1].Name != "Captain Vale" { t.Fatalf("fallback NPCs = %#v, want independently safe group applied", result.Value.NPCs) } - if len(result.Retry.FallbackDiagnostics) != 1 || result.Retry.FallbackDiagnostics[0].ReasonCode != ReasonCodeNPCSemanticReconciliationExhausted || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "1 proposal group") { + if len(result.Retry.FallbackDiagnostics) != 1 || result.Retry.FallbackDiagnostics[0].ReasonCode != ReasonCodeNPCSemanticReconciliationExhausted || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "1 proposal group") || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "preserved separately") || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "supplied candidate list") { t.Fatalf("fallback diagnostics = %#v, want exact omitted-group warning", result.Retry.FallbackDiagnostics) } } @@ -134,6 +137,9 @@ func TestNormalizeInvalidStructuredOutputAndOperationalErrorsRemainDistinct(t *t if err != nil || result.Retry == nil || result.Retry.ReasonCode != ReasonCodeNPCSemanticProposalInvalid || len(result.Value.NPCs) != 2 { t.Fatalf("invalid structured result = %#v, %v; want deterministic retry fallback", result, err) } + if result.Retry.CorrectionGuidance != "" || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "structured-output contract") || !strings.Contains(result.Retry.FallbackDiagnostics[0].Samples[0].Message, "preserved separately") { + t.Fatalf("invalid structured retry = %#v, want feedback-free retry and informative fallback", result.Retry) + } operational := errors.New("provider unavailable") if _, err := newNormalizer(t, &recordingNPCNormalizerClient{err: operational}).Normalize(context.Background(), normalizeRequestWithSource(input, doc)); !errors.Is(err, operational) || errors.Is(err, contracts.ErrInvalidStructuredOutput) { t.Fatalf("operational completion error = %v, want ordinary error", err)