From 917d150279cfc500a61ec834c089ef7657d34942 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 29 Aug 2026 01:24:45 +0000 Subject: [PATCH] Improve D&D validation reliability --- .../schemas/dnd_combat_turns_llm.v1.json | 3 +- .../schemas/dnd_enemy_events_llm.v1.json | 5 +- .../schemas/dnd_npc_occurrences_llm.v1.json | 3 +- .../dnd_scene_descriptions_llm.v1.json | 3 +- docs/internal/dnd.md | 38 ++- docs/roadmap/dnd-validation-reliability.md | 240 ++++++++++++++++++ .../dnd/extract/combatturns/extractor_test.go | 4 +- .../dnd/extract/combatturns/schema_test.go | 26 +- .../dnd/extract/enemyevents/extractor_test.go | 4 +- .../dnd/extract/enemyevents/schema_test.go | 24 +- .../extract/npcoccurrences/extractor_test.go | 6 +- .../dnd/extract/npcoccurrences/schema_test.go | 24 +- .../scenedescriptions/extractor_test.go | 6 +- .../extract/scenedescriptions/schema_test.go | 17 +- .../dnd/shared/diagnostics/diagnostics.go | 74 ++++++ .../shared/diagnostics/diagnostics_test.go | 31 +++ .../modules/dnd/shared/source_ref_coverage.go | 48 ++++ .../dnd/shared/source_ref_coverage_test.go | 69 +++++ .../validate/combatturns/shape/validator.go | 28 +- .../combatturns/source_refs/validator.go | 51 +--- .../enemyevents/engagements/validator.go | 33 ++- .../enemyevents/engagements/validator_test.go | 21 ++ .../validate/enemyevents/shape/validator.go | 26 +- .../enemyevents/source_refs/validator.go | 56 ++-- .../itemoccurrences/registry/validator.go | 16 +- .../itemoccurrences/shape/validator.go | 57 +---- .../itemoccurrences/source_refs/validator.go | 53 ++-- .../itemregistry/identity/validator.go | 32 ++- .../itemregistry/identity/validator_test.go | 5 +- .../validate/itemregistry/shape/validator.go | 37 ++- .../itemregistry/source_refs/validator.go | 39 +-- .../source_refs/validator_test.go | 11 + .../locationoccurrences/registry/validator.go | 16 +- .../locationoccurrences/shape/validator.go | 27 +- .../source_refs/validator.go | 28 +- .../locationregistry/identity/validator.go | 32 ++- .../identity/validator_test.go | 5 +- .../locationregistry/shape/validator.go | 37 ++- .../locationregistry/source_refs/validator.go | 39 +-- .../npcoccurrences/registry/validator.go | 16 +- .../npcoccurrences/shape/validator.go | 27 +- .../npcoccurrences/source_refs/validator.go | 25 +- .../npcregistry/identity/validator.go | 32 ++- .../npcregistry/identity/validator_test.go | 5 +- .../validate/npcregistry/shape/validator.go | 37 ++- .../npcregistry/shape/validator_test.go | 17 +- .../npcregistry/source_refs/validator.go | 59 ++--- .../npcregistry/source_refs/validator_test.go | 16 +- .../scenedescriptions/shape/validator.go | 79 +++--- .../dnd/validate/spells/catalog/validator.go | 66 ++--- .../validate/spells/catalog/validator_test.go | 62 +---- .../dnd/validate/spells/shape/validator.go | 62 ++++- .../validate/spells/shape/validator_test.go | 29 +++ .../validate/spells/source_refs/validator.go | 51 ++-- .../spells/source_refs/validator_test.go | 8 + 55 files changed, 1300 insertions(+), 565 deletions(-) create mode 100644 docs/roadmap/dnd-validation-reliability.md create mode 100644 internal/modules/dnd/shared/source_ref_coverage.go create mode 100644 internal/modules/dnd/shared/source_ref_coverage_test.go diff --git a/assets/dnd/combat-turns/schemas/dnd_combat_turns_llm.v1.json b/assets/dnd/combat-turns/schemas/dnd_combat_turns_llm.v1.json index f3e8af90..29bb6476 100644 --- a/assets/dnd/combat-turns/schemas/dnd_combat_turns_llm.v1.json +++ b/assets/dnd/combat-turns/schemas/dnd_combat_turns_llm.v1.json @@ -16,7 +16,8 @@ "type": "string" }, "turn_kind": { - "type": "string" + "type": "string", + "enum": ["turn", "reaction", "legendary_action", "lair_action", "other"] }, "source_refs": { "type": "array", diff --git a/assets/dnd/enemy-events/schemas/dnd_enemy_events_llm.v1.json b/assets/dnd/enemy-events/schemas/dnd_enemy_events_llm.v1.json index f574075f..bbbf8ad9 100644 --- a/assets/dnd/enemy-events/schemas/dnd_enemy_events_llm.v1.json +++ b/assets/dnd/enemy-events/schemas/dnd_enemy_events_llm.v1.json @@ -13,7 +13,10 @@ "required": ["name", "kind", "source_refs"], "properties": { "name": {"type": "string"}, - "kind": {"type": "string"}, + "kind": { + "type": "string", + "enum": ["engaged", "killed", "fled", "captured", "incapacitated"] + }, "source_refs": { "type": "array", "items": { diff --git a/assets/dnd/npc-occurrences/schemas/dnd_npc_occurrences_llm.v1.json b/assets/dnd/npc-occurrences/schemas/dnd_npc_occurrences_llm.v1.json index fd0a230b..6e23f0bf 100644 --- a/assets/dnd/npc-occurrences/schemas/dnd_npc_occurrences_llm.v1.json +++ b/assets/dnd/npc-occurrences/schemas/dnd_npc_occurrences_llm.v1.json @@ -16,7 +16,8 @@ "type": "string" }, "kind": { - "type": "string" + "type": "string", + "enum": ["mentioned", "noncombat_presence", "dialogue", "combat_ally", "combat_opponent", "other"] }, "source_refs": { "type": "array", diff --git a/assets/dnd/scene-descriptions/schemas/dnd_scene_descriptions_llm.v1.json b/assets/dnd/scene-descriptions/schemas/dnd_scene_descriptions_llm.v1.json index 20ff4655..bbcb6f95 100644 --- a/assets/dnd/scene-descriptions/schemas/dnd_scene_descriptions_llm.v1.json +++ b/assets/dnd/scene-descriptions/schemas/dnd_scene_descriptions_llm.v1.json @@ -6,7 +6,8 @@ "required": ["kind", "title", "summary"], "properties": { "kind": { - "type": "string" + "type": "string", + "enum": ["combat", "narrative", "recap", "meta"] }, "title": { "type": "string" diff --git a/docs/internal/dnd.md b/docs/internal/dnd.md index 068aada3..32d3c3ca 100644 --- a/docs/internal/dnd.md +++ b/docs/internal/dnd.md @@ -209,12 +209,30 @@ wrong-direction rejection, execution failure, producer-correction success, added calls, latency, and token use together. A structurally successful provider run alone is not evidence that the validator should become a default. -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. +Every producer-correctable D&D rejection describes all currently detectable +corrections in transcript-grounded domain terms, using contextual names, +model-owned artifact fields, and source segment ranges when useful. Validators +collect independent record defects in one pass so one retry does not merely +reveal the next issue. Shared D&D diagnostic helpers keep repeated rules and +record descriptions stable, de-duplicated, and bounded; each artifact family +continues to own the semantic rule and its prose. + +Operator diagnostics and model guidance are separate products of the same +assessment. Operator messages may use typed paths, reason details, and opaque +application identities. Correction guidance must not copy those messages or +ask the model to reproduce durable entity IDs, hashes, validator module keys, +reason codes, or Go field paths. A registry-normalization rejection instead +speaks in terms of the duplicate-group proposal response the normalizer can +actually revise. Normalization-only deterministic invariants retain useful +operator detail but do not imply that a model controls derived ordering or +identity. Only bounded actionable semantic guidance is eligible for a +correction prompt. + +Private LLM schemas use simple enums for closed categorical fields when the +provider-compatible shape can express the rule directly. Deterministic typed +validators retain the same checks as defense in depth and for non-LLM +producers. Private schemas keep every property required and avoid optional +properties, `uniqueItems`, and conditional cross-field logic. Item-occurrence shape validation groups repeated holder mistakes by occurrence kind and gives the producer the required JSON null/non-null relationship. It @@ -228,6 +246,14 @@ the same comparison identity within one scene-scoped result. Normalization may combine results from distinct scenes, so it intentionally does not apply that rule. Configuration owns the exact validator key and chain position. +Extraction source-reference validators share one full-span chunk-containment +policy. After ordinary reference validity succeeds, the policy resolves both +endpoints through document order and requires every source unit in the +inclusive range to be present in the current chunk. It does not assume numeric +unit-ID ordering, mutate input, or weaken wrong-source and unresolved-reference +validation. Scene descriptions remain separate because their validator owns an +exact one-scene range contract rather than general extraction containment. + Normalizers are deterministic for spells, combat turns, item occurrences, NPC occurrences, scene descriptions, enemy events, and location occurrences. They canonicalize display values and evidence, use source-document order for stable diff --git a/docs/roadmap/dnd-validation-reliability.md b/docs/roadmap/dnd-validation-reliability.md new file mode 100644 index 00000000..8d5e23a8 --- /dev/null +++ b/docs/roadmap/dnd-validation-reliability.md @@ -0,0 +1,240 @@ +# D&D Validation Reliability Hardening + +## Purpose + +Improve the correctness and efficiency of D&D validation retries by giving the +producer complete, contextual, semantically useful correction guidance; moving +simple closed-value checks into compatible LLM-facing schemas; and removing +duplicated source-range containment logic. The work should make one retry more +likely to repair every detectable defect without weakening deterministic +validation or exposing internal identifiers to a model. + +## Current State And Findings + +The validation architecture is coherent and the major safety boundaries are in +place. Validators run through the framework-managed retry protocol, model-facing +guidance is distinct from reason codes and operator diagnostics, schemas use +provider-compatible required fields, and D&D extraction adapters canonically +order safely resolvable source-reference endpoints before validation. The +review nevertheless found four opportunities to make that architecture more +consistent and effective. + +### Generic correction guidance + +Most rejecting D&D validators produce detailed operator-facing messages but +pair them with a fixed, general correction sentence. The framework correctly +constructs retry text from `CorrectionGuidance` alone, so the model does not see +the internal diagnostic—and therefore often does not learn which records were +wrong or how each record must change. Item-occurrence shape validation and the +combat-semantics validator already demonstrate the intended contextual pattern. + +Passing the operator message through unchanged is not an acceptable fix. +Operator messages may contain Go paths, array indexes that do not reliably map +back to canonicalized model output, reason codes, validator terminology, or +opaque application identities. Those details are useful for diagnosis but are +not appropriate model context. + +### Missing closed-value schema constraints + +Four private LLM response schemas describe categorical fields as unconstrained +strings even though their prompts, durable artifact schemas, and deterministic +validators define closed value sets: + +- `combat-turns`: `turn_kind`; +- `enemy-events`: `kind`; +- `npc-occurrences`: `kind`; and +- `scene-descriptions`: `kind`. + +This defers an inexpensive structural check until the outer semantic-validation +loop. Item and location occurrences already use the preferable private-schema +enum pattern. + +### First-defect validation + +Spell shape validation returns after the first invalid cast field, and enemy +engagement validation returns after the first duplicated engagement subject. +When multiple defects exist, each retry can therefore reveal only one of them. +That needlessly consumes the bounded producer-attempt budget and makes +correction less reliable for smaller models. + +### Duplicated source-range containment + +Nine extraction source-reference validators independently determine whether a +citation belongs to the current chunk. Five validate every source unit spanned +by a reference, while four check only the endpoints. Those behaviors are +equivalent for today's contiguous materialized chunks, but the duplicated +implementations and different apparent contracts create a drift risk. +Scene-description validation has a separate exact-one-scene range contract and +should remain specialized. + +## Target End State + +- Every producer-correctable D&D rejection supplies bounded contextual guidance + that identifies all affected records using transcript-grounded names, source + ranges, and response fields as appropriate, explains each semantic defect, + and requests one complete corrected replacement. +- Model-facing guidance never contains reason codes, validator keys, Go field + paths, opaque or hash-derived entity IDs, or raw internal error text. + Operator diagnostics remain detailed and separately available. +- Every detectable issue in a candidate is collected in one validator pass, + subject to the repository's established diagnostic bounds. A retry is not + spent merely to reveal the next defect. +- Closed categorical fields are constrained by compatible enums in the private + LLM schemas. Deterministic typed validators retain the same rules as + defense-in-depth and as protection for non-LLM producers and later stages. +- One D&D-owned helper implements full-span chunk containment for extraction + evidence. All applicable source-reference validators use it and no + module-local endpoint-only or coverage implementation remains. +- Validator results remain immutable, deterministic, bounded, and suitable for + the existing feedback-aware replacement-request protocol. Retry budgets, + warning policy, durable artifact schemas, and accepted output semantics do + not change. + +## Required Work + +### Contextual correction guidance + +Audit each rejecting D&D validator according to the stage that can actually +correct its result: + +- Extraction validators should build model-facing issue descriptions from the + candidate and its transcript context. Shape, registry-membership, identity, + catalog, engagement, and source-reference validators should name the + contextual artifact and cited range where useful, state the invalid value or + relationship in plain language, and state the required replacement shape. +- LLM-backed NPC, item, and location registry reconciliation should continue to + translate its typed proposal issues through the shared category-to-prose + renderer, including any domain-specific supplement. +- Normalization-only deterministic invariant validators should retain useful + operator diagnostics. They should not claim that a producer can directly + repair deterministically derived ordering, identity, or normalization state. + If such a rejection can reach a feedback-capable producer, guidance must be + expressed only in terms of the source candidate that producer controls. +- LLM-backed semantic validators may continue using their verdict explanation + when it is bounded and semantically meaningful. + +Introduce a small D&D-shared diagnostic utility only for demonstrated common +mechanics such as stable grouping, de-duplication, contextual source-range +rendering, and bounded correction aggregation. Domain validators must continue +to own the meaning and prose of their rules. The generic pipeline must not gain +D&D knowledge, and operator `Message` values must never be mechanically copied +into `CorrectionGuidance`. + +Update validator policy fingerprints wherever correction behavior changes so +checkpoints created under generic feedback are not reused as though the policy +were identical. + +### Private-schema enum guardrails + +Add the existing supported value sets to the four private LLM schemas: + +- `assets/dnd/combat-turns/schemas/dnd_combat_turns_llm.v1.json`; +- `assets/dnd/enemy-events/schemas/dnd_enemy_events_llm.v1.json`; +- `assets/dnd/npc-occurrences/schemas/dnd_npc_occurrences_llm.v1.json`; and +- `assets/dnd/scene-descriptions/schemas/dnd_scene_descriptions_llm.v1.json`. + +Use the exact values owned by the corresponding durable contract and typed +domain constants. Keep all object properties required, retain current nullable +types where present, and do not introduce `uniqueItems`, optional properties, +or conditional schema logic. Revise schema tests that currently accept unknown +values, and rely on computed asset fingerprints to invalidate incompatible +LLM-output checkpoints. Keep deterministic enum validation in place. + +### Complete per-attempt issue collection + +Refactor spell shape and enemy engagement validation to inspect the complete +candidate and collect every detectable violation before returning. Build the +operator diagnostic and contextual correction request from the same evaluated +issue set while preserving their different audiences. De-duplicate repeated +semantic instructions, retain enough contextual identification for every +affected record, and use the established bounded diagnostic behavior rather +than an unbounded error string. + +Review the surrounding D&D validators while applying the contextual-guidance +change. Remove any additional accidental early exits that prevent independent +candidate defects from being reported together, but retain immediate returns +for request-level prerequisites whose absence makes further inspection unsafe +or meaningless. + +### Shared full-span chunk containment + +Add one helper under `internal/modules/dnd/shared` that determines whether a +source reference's complete document-ordered span is contained by a chunk. The +helper must: + +- validate source identity; +- resolve endpoints through `source.DocumentIndex` rather than numeric-ID + assumptions; +- require every unit in the inclusive span to be present in the chunk; +- handle nil or unresolved inputs without panic; +- avoid mutating the source, chunk, or reference; and +- leave source-reference validity and error wording to the consuming validator. + +Use the helper from the spell, NPC-registry, NPC-occurrence, item-registry, +item-occurrence, location-registry, location-occurrence, combat-turn, and +enemy-event source-reference validators. Keep the scene-description exact-range +validator separate because it enforces a materially different contract. + +## Testing And Documentation + +Add lean offline behavioral coverage at the narrowest stable boundary: + +- correction tests should prove that multiple contextual defects produce one + actionable, bounded request and that internal identifiers and diagnostic + syntax are absent; they should not snapshot exact prose or message length; +- schema tests should prove rejection of representative unsupported categorical + values and acceptance of the supported sets without duplicating every + provider behavior; +- spell-shape and engagement tests should prove that independent defects are + reported together; +- the shared containment helper should own the full case matrix, including + partial spans, non-monotonic unit IDs, wrong sources, unresolved endpoints, + and nil inputs; consuming validators need only enough coverage to prove they + use the common policy; and +- existing validator tests should be simplified when the shared helper makes + module-local cases redundant. + +Update `docs/internal/dnd.md` to document the durable conventions for +contextual correction guidance, complete issue collection, private-schema enum +guardrails, and shared full-span extraction containment. Update +`docs/internal/pipeline.md` only if framework behavior changes; the intended +work applies its existing contract and should normally require only a link or +no change. No new ADR is required because ADR-0014 and the architecture policy +already decide the separation between semantic guidance and internal +diagnostics. Create an ADR only if implementation requires changing that +framework-level decision. + +## Non-Goals + +- Passing operator diagnostics or internal errors directly to an LLM. +- Exposing durable IDs, hashes, reason codes, validator keys, or raw provider + responses in correction text. +- Changing stage retry counts, PromptKit structural-repair budgets, validator + failure policy, or warning classification. +- Silently repairing domain-semantic defects or weakening deterministic + validators after adding schema guardrails. +- Adding provider-sensitive schema constructs beyond simple enums. +- Moving D&D-specific behavior into the generic pipeline framework. +- Generalizing the scene-description exact-range contract into the shared + extraction containment helper. + +## Acceptance Criteria + +- Every producer-correctable D&D rejection reviewed in this work gives the next + attempt all currently detectable, actionable corrections in contextual prose. +- No model-facing correction request contains an opaque entity ID, hash, + validator key, reason code, Go-style field path, or unfiltered operator error. +- Spell shape and enemy engagement validation aggregate independent defects in + one pass and keep outputs immutable. +- The four private LLM schemas reject unsupported categorical values, retain + provider-compatible required-only shapes, and leave durable v1 contracts + unchanged. +- All nine applicable extraction source-reference validators use one D&D-shared + full-span containment policy; scene descriptions retain their specialized + exact-range check. +- Validator and asset fingerprints change wherever their effective policy + changes, preventing reuse of stale checkpoints. +- Canonical internal documentation records the conventions future D&D + validators must follow. +- Focused tests, `go test ./...`, `go vet ./...`, and + `go build ./cmd/notarius` pass in a supported development environment. diff --git a/internal/modules/dnd/extract/combatturns/extractor_test.go b/internal/modules/dnd/extract/combatturns/extractor_test.go index b47e85f2..75db4c1a 100644 --- a/internal/modules/dnd/extract/combatturns/extractor_test.go +++ b/internal/modules/dnd/extract/combatturns/extractor_test.go @@ -66,7 +66,7 @@ func TestExtractMapsAndOrdersCombatTurnsBySourcePosition(t *testing.T) { func TestExtractPreservesInvalidCandidatesForValidators(t *testing.T) { client := &fakeCombatTurnsLLMClient{response: extractionResponse{CombatTurns: []combatTurnResponse{ { - Actor: " ", TurnKind: "unsupported", + Actor: " ", TurnKind: "turn", SourceRefs: []combatSourceRefResponse{{StartUnitID: 99, EndUnitID: 0}}, }, }}} @@ -75,7 +75,7 @@ func TestExtractPreservesInvalidCandidatesForValidators(t *testing.T) { t.Fatalf("Extract() error = %v, want nil for candidate values", err) } turn := result.Value.CombatTurns[0] - if turn.Actor != " " || turn.TurnKind != "unsupported" { + if turn.Actor != " " || turn.TurnKind != "turn" { t.Fatalf("invalid turn fields = %#v, want preserved candidate values", turn) } if turn.SourceRefs[0] != (source.SourceRef{SourceID: "session-alpha", StartUnitID: 99}) { diff --git a/internal/modules/dnd/extract/combatturns/schema_test.go b/internal/modules/dnd/extract/combatturns/schema_test.go index 568a5e4a..302e7f16 100644 --- a/internal/modules/dnd/extract/combatturns/schema_test.go +++ b/internal/modules/dnd/extract/combatturns/schema_test.go @@ -7,6 +7,8 @@ import ( "testing" "github.com/santhosh-tekuri/jsonschema/v6" + + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" ) func TestLoadResponseSchemaUsesPrivateCombatShape(t *testing.T) { @@ -37,7 +39,7 @@ func TestLoadResponseSchemaUsesPrivateCombatShape(t *testing.T) { } } -func TestResponseSchemaLeavesSemanticConstraintsToDeterministicValidators(t *testing.T) { +func TestResponseSchemaLeavesNonCategoricalSemanticsToDeterministicValidators(t *testing.T) { schema, err := loadResponseSchema() if err != nil { t.Fatal(err) @@ -45,7 +47,6 @@ func TestResponseSchemaLeavesSemanticConstraintsToDeterministicValidators(t *tes semanticCandidate := validCombatResponse() turn := semanticCandidate["combat_turns"].([]any)[0].(map[string]any) turn["actor"] = "" - turn["turn_kind"] = "unsupported" ref := turn["source_refs"].([]any)[0].(map[string]any) ref["start_unit_id"] = 0 ref["end_unit_id"] = -1 @@ -54,7 +55,7 @@ func TestResponseSchemaLeavesSemanticConstraintsToDeterministicValidators(t *tes t.Fatal(err) } if err := validateJSONSchema(content, schema.JSONSchema); err != nil { - t.Fatalf("private schema rejected validator-owned semantics: %v", err) + t.Fatalf("private schema rejected non-categorical validator-owned semantics: %v", err) } turn["source_refs"] = []any{} content, err = json.Marshal(semanticCandidate) @@ -79,6 +80,7 @@ func TestResponseSchemaRetainsStructuralBoundary(t *testing.T) { {name: "wrong actor type", mutate: func(turn map[string]any) { turn["actor"] = 1 }}, {name: "unknown field", mutate: func(turn map[string]any) { turn["unexpected"] = true }}, {name: "missing source refs", mutate: func(turn map[string]any) { delete(turn, "source_refs") }}, + {name: "unsupported turn kind", mutate: func(turn map[string]any) { turn["turn_kind"] = "unsupported" }}, } { t.Run(test.name, func(t *testing.T) { candidate := validCombatResponse() @@ -94,6 +96,24 @@ func TestResponseSchemaRetainsStructuralBoundary(t *testing.T) { } } +func TestResponseSchemaAcceptsEverySupportedTurnKind(t *testing.T) { + schema, err := loadResponseSchema() + if err != nil { + t.Fatal(err) + } + for _, kind := range []string{string(dnd.CombatTurnKindTurn), string(dnd.CombatTurnKindReaction), string(dnd.CombatTurnKindLegendaryAction), string(dnd.CombatTurnKindLairAction), string(dnd.CombatTurnKindOther)} { + candidate := validCombatResponse() + candidate["combat_turns"].([]any)[0].(map[string]any)["turn_kind"] = kind + content, err := json.Marshal(candidate) + if err != nil { + t.Fatal(err) + } + if err := validateJSONSchema(content, schema.JSONSchema); err != nil { + t.Fatalf("supported turn kind %q was rejected: %v", kind, err) + } + } +} + func TestResponseSchemaJSONIsMutationSafe(t *testing.T) { first, err := loadResponseSchema() if err != nil { diff --git a/internal/modules/dnd/extract/enemyevents/extractor_test.go b/internal/modules/dnd/extract/enemyevents/extractor_test.go index 7f23b774..5f77266e 100644 --- a/internal/modules/dnd/extract/enemyevents/extractor_test.go +++ b/internal/modules/dnd/extract/enemyevents/extractor_test.go @@ -59,14 +59,14 @@ func TestExtractMapsEnemyEventsInSourceOrder(t *testing.T) { func TestExtractPreservesSemanticCandidatesAndResponseOwnership(t *testing.T) { client := &fakeEnemyEventsLLMClient{response: extractionResponse{Events: []enemyEventResponse{{ - Name: " ", Kind: "unsupported", SourceRefs: []enemySourceRefResponse{{StartUnitID: 99, EndUnitID: 0}}, + Name: " ", Kind: "engaged", SourceRefs: []enemySourceRefResponse{{StartUnitID: 99, EndUnitID: 0}}, }}}} result, err := newEnemyExtractor(t, client).Extract(context.Background(), enemyExtractionRequest(t)) if err != nil { t.Fatal(err) } event := result.Value.Events[0] - if event.Name != " " || event.Kind != "unsupported" || event.SourceRefs[0] != (source.SourceRef{SourceID: "combat-session", StartUnitID: 99}) { + if event.Name != " " || event.Kind != "engaged" || event.SourceRefs[0] != (source.SourceRef{SourceID: "combat-session", StartUnitID: 99}) { t.Fatalf("semantic candidate = %#v", event) } result.Value.Events[0].SourceRefs[0].StartUnitID = 7 diff --git a/internal/modules/dnd/extract/enemyevents/schema_test.go b/internal/modules/dnd/extract/enemyevents/schema_test.go index adf91523..9f093eb8 100644 --- a/internal/modules/dnd/extract/enemyevents/schema_test.go +++ b/internal/modules/dnd/extract/enemyevents/schema_test.go @@ -7,6 +7,8 @@ import ( "testing" "github.com/santhosh-tekuri/jsonschema/v6" + + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" ) func TestResponseSchemaDefinesPrivateStructuralBoundary(t *testing.T) { @@ -28,13 +30,12 @@ func TestResponseSchemaDefinesPrivateStructuralBoundary(t *testing.T) { semantic := validEnemyResponse() event := semantic["events"].([]any)[0].(map[string]any) event["name"] = "" - event["kind"] = "unsupported" ref := event["source_refs"].([]any)[0].(map[string]any) ref["start_unit_id"] = 0 ref["end_unit_id"] = -1 content, err = json.Marshal(semantic) if err != nil || validateEnemySchema(content, schema.JSONSchema) != nil { - t.Fatalf("validator-owned semantics were rejected: %v", err) + t.Fatalf("non-categorical validator-owned semantics were rejected: %v", err) } } @@ -46,6 +47,7 @@ func TestResponseSchemaRejectsInvalidStructure(t *testing.T) { for _, mutate := range []func(map[string]any){ func(event map[string]any) { delete(event, "name") }, func(event map[string]any) { event["kind"] = 1 }, + func(event map[string]any) { event["kind"] = "unsupported" }, func(event map[string]any) { event["unexpected"] = true }, func(event map[string]any) { event["source_refs"].([]any)[0].(map[string]any)["source_id"] = "session" }, } { @@ -70,6 +72,24 @@ func TestResponseSchemaRejectsInvalidStructure(t *testing.T) { } } +func TestResponseSchemaAcceptsEverySupportedKind(t *testing.T) { + schema, err := loadResponseSchema() + if err != nil { + t.Fatal(err) + } + for _, kind := range []string{string(dnd.EnemyEventKindEngaged), string(dnd.EnemyEventKindKilled), string(dnd.EnemyEventKindFled), string(dnd.EnemyEventKindCaptured), string(dnd.EnemyEventKindIncapacitated)} { + candidate := validEnemyResponse() + candidate["events"].([]any)[0].(map[string]any)["kind"] = kind + content, err := json.Marshal(candidate) + if err != nil { + t.Fatal(err) + } + if err := validateEnemySchema(content, schema.JSONSchema); err != nil { + t.Fatalf("supported enemy-event kind %q was rejected: %v", kind, err) + } + } +} + func validEnemyResponse() map[string]any { return map[string]any{"events": []any{map[string]any{ "name": "Ashfang", "kind": "engaged", "source_refs": []any{map[string]any{"start_unit_id": 1, "end_unit_id": 1}}, diff --git a/internal/modules/dnd/extract/npcoccurrences/extractor_test.go b/internal/modules/dnd/extract/npcoccurrences/extractor_test.go index 623ee1ac..1974d426 100644 --- a/internal/modules/dnd/extract/npcoccurrences/extractor_test.go +++ b/internal/modules/dnd/extract/npcoccurrences/extractor_test.go @@ -24,7 +24,7 @@ func TestExtractMapsEveryKindAndOrdersBySourcePosition(t *testing.T) { {Name: "Speaker", Kind: "dialogue", SourceRefs: append(occurrenceRefs(2, 2), occurrenceRefs(2, 2)...)}, {Name: "Present", Kind: "noncombat_presence", SourceRefs: occurrenceRefs(7, 2)}, {Name: "Mentioned", Kind: "mentioned", SourceRefs: occurrenceRefs(10, 10)}, - {Name: "Invalid", Kind: "unsupported", SourceRefs: occurrenceRefs(0, 0)}, + {Name: "Invalid", Kind: "other", SourceRefs: occurrenceRefs(0, 0)}, }}} references := requiredRegistryReferences(t, "Mentioned", "Speaker", "Present", "Ally", "Opponent", "Other", "Invalid") req := extractionRequest() @@ -44,7 +44,7 @@ func TestExtractMapsEveryKindAndOrdersBySourcePosition(t *testing.T) { dnd.NPCOccurrenceKindCombatAlly, dnd.NPCOccurrenceKindCombatOpponent, dnd.NPCOccurrenceKindOther, - "unsupported", + dnd.NPCOccurrenceKindOther, }) { t.Fatalf("occurrence kinds = %#v", got) } @@ -57,7 +57,7 @@ func TestExtractMapsEveryKindAndOrdersBySourcePosition(t *testing.T) { if id := result.Value.Occurrences[0].NPCID; id != identity.DeriveID("Mentioned") { t.Fatalf("durable NPC ID = %q, want registry identity", id) } - if invalid := result.Value.Occurrences[6]; invalid.Name != "Invalid" || invalid.Kind != "unsupported" || !reflect.DeepEqual(invalid.SourceRefs, []source.SourceRef{{SourceID: "session-alpha"}}) { + if invalid := result.Value.Occurrences[6]; invalid.Name != "Invalid" || invalid.Kind != dnd.NPCOccurrenceKindOther || !reflect.DeepEqual(invalid.SourceRefs, []source.SourceRef{{SourceID: "session-alpha"}}) { t.Fatalf("invalid candidate = %#v, want preserved values with current source identity", invalid) } if len(client.requests) != 1 { diff --git a/internal/modules/dnd/extract/npcoccurrences/schema_test.go b/internal/modules/dnd/extract/npcoccurrences/schema_test.go index 6b9e400c..0a3f1643 100644 --- a/internal/modules/dnd/extract/npcoccurrences/schema_test.go +++ b/internal/modules/dnd/extract/npcoccurrences/schema_test.go @@ -7,6 +7,8 @@ import ( "testing" "github.com/santhosh-tekuri/jsonschema/v6" + + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" ) func TestResponseSchemaOwnsOnlyPrivateStructuralContract(t *testing.T) { @@ -29,7 +31,6 @@ func TestResponseSchemaOwnsOnlyPrivateStructuralContract(t *testing.T) { semanticCandidate := validOccurrenceResponse() occurrence := semanticCandidate["occurrences"].([]any)[0].(map[string]any) occurrence["name"] = "" - occurrence["kind"] = "unsupported" ref := occurrence["source_refs"].([]any)[0].(map[string]any) ref["start_unit_id"] = 0 ref["end_unit_id"] = -1 @@ -38,12 +39,13 @@ func TestResponseSchemaOwnsOnlyPrivateStructuralContract(t *testing.T) { t.Fatal(err) } if err := validateJSONSchema(content, schema.JSONSchema); err != nil { - t.Fatalf("schema rejected validator-owned semantics: %v", err) + t.Fatalf("schema rejected non-categorical validator-owned semantics: %v", err) } for _, mutate := range []func(map[string]any){ func(record map[string]any) { delete(record, "name") }, func(record map[string]any) { record["kind"] = 1 }, + func(record map[string]any) { record["kind"] = "unsupported" }, func(record map[string]any) { record["npc_id"] = "npc:sha256:opaque" }, func(record map[string]any) { record["unexpected"] = true }, func(record map[string]any) { @@ -79,6 +81,24 @@ func TestResponseSchemaIsDefensiveAndContentSafe(t *testing.T) { } } +func TestResponseSchemaAcceptsEverySupportedKind(t *testing.T) { + schema, err := loadResponseSchema() + if err != nil { + t.Fatal(err) + } + for _, kind := range []string{string(dnd.NPCOccurrenceKindMentioned), string(dnd.NPCOccurrenceKindNoncombatPresence), string(dnd.NPCOccurrenceKindDialogue), string(dnd.NPCOccurrenceKindCombatAlly), string(dnd.NPCOccurrenceKindCombatOpponent), string(dnd.NPCOccurrenceKindOther)} { + candidate := validOccurrenceResponse() + candidate["occurrences"].([]any)[0].(map[string]any)["kind"] = kind + content, err := json.Marshal(candidate) + if err != nil { + t.Fatal(err) + } + if err := validateJSONSchema(content, schema.JSONSchema); err != nil { + t.Fatalf("supported NPC-occurrence kind %q was rejected: %v", kind, err) + } + } +} + func validOccurrenceResponse() map[string]any { return map[string]any{"occurrences": []any{map[string]any{ "name": "Mira Thorn", "kind": "dialogue", diff --git a/internal/modules/dnd/extract/scenedescriptions/extractor_test.go b/internal/modules/dnd/extract/scenedescriptions/extractor_test.go index 3ce7326d..8923d7e3 100644 --- a/internal/modules/dnd/extract/scenedescriptions/extractor_test.go +++ b/internal/modules/dnd/extract/scenedescriptions/extractor_test.go @@ -75,18 +75,18 @@ func TestExtractReturnsSemanticallyInvalidResponseForDeterministicValidation(t * if err != nil { t.Fatalf("loadResponseSchema() error = %v", err) } - if err := validateJSONSchema(t, map[string]any{"kind": "unrecognized", "title": " ", "summary": ""}, schema.JSONSchema); err != nil { + if err := validateJSONSchema(t, map[string]any{"kind": "narrative", "title": " ", "summary": ""}, schema.JSONSchema); err != nil { t.Fatalf("semantic candidate rejected by private schema: %v", err) } client := &fakeSceneDescriptionsLLMClient{response: extractionResponse{ - Kind: dnd.SceneKind("unrecognized"), Title: " ", Summary: "", + Kind: dnd.SceneKindNarrative, Title: " ", Summary: "", }} result, err := newExtractor(t, client).Extract(context.Background(), extractionRequest()) if err != nil { t.Fatalf("Extract() error = %v, want nil", err) } scene := result.Value.Scenes[0] - if scene.Kind != dnd.SceneKind("unrecognized") || scene.Title != "" || scene.Summary != "" { + if scene.Kind != dnd.SceneKindNarrative || scene.Title != "" || scene.Summary != "" { t.Fatalf("scene = %#v, want semantic candidates returned for deterministic validation", scene) } } diff --git a/internal/modules/dnd/extract/scenedescriptions/schema_test.go b/internal/modules/dnd/extract/scenedescriptions/schema_test.go index 756fd070..86f1aba9 100644 --- a/internal/modules/dnd/extract/scenedescriptions/schema_test.go +++ b/internal/modules/dnd/extract/scenedescriptions/schema_test.go @@ -7,6 +7,8 @@ import ( "testing" "github.com/santhosh-tekuri/jsonschema/v6" + + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" ) func TestLoadResponseSchemaUsesStrictPrivateSceneDescriptionContract(t *testing.T) { @@ -30,7 +32,7 @@ func TestLoadResponseSchemaUsesStrictPrivateSceneDescriptionContract(t *testing. {name: "unknown framework field", response: map[string]any{"kind": "combat", "title": "Ambush", "summary": "Bandits strike.", "id": "assigned-later"}}, {name: "unknown application field", response: map[string]any{"kind": "combat", "title": "Ambush", "summary": "Bandits strike.", "source_ref": map[string]any{}}}, {name: "collection is not allowed", response: map[string]any{"kind": "combat", "title": "Ambush", "summary": "Bandits strike.", "scenes": []any{}}}, - {name: "unsupported kind", response: map[string]any{"kind": "interlude", "title": "Ambush", "summary": "Bandits strike."}, valid: true}, + {name: "unsupported kind", response: map[string]any{"kind": "interlude", "title": "Ambush", "summary": "Bandits strike."}}, {name: "empty title", response: map[string]any{"kind": "combat", "title": "", "summary": "Bandits strike."}, valid: true}, {name: "empty summary", response: map[string]any{"kind": "combat", "title": "Ambush", "summary": ""}, valid: true}, {name: "wrong kind type", response: map[string]any{"kind": 7, "title": "Ambush", "summary": "Bandits strike."}}, @@ -64,6 +66,19 @@ func TestResponseSchemaIsMutationSafeAndDiagnosticsRedactContent(t *testing.T) { } } +func TestResponseSchemaAcceptsEverySupportedKind(t *testing.T) { + schema, err := loadResponseSchema() + if err != nil { + t.Fatal(err) + } + for _, kind := range []string{string(dnd.SceneKindCombat), string(dnd.SceneKindNarrative), string(dnd.SceneKindRecap), string(dnd.SceneKindMeta)} { + response := map[string]any{"kind": kind, "title": "Title", "summary": "Summary"} + if err := validateJSONSchema(t, response, schema.JSONSchema); err != nil { + t.Fatalf("supported scene kind %q was rejected: %v", kind, err) + } + } +} + func validateJSONSchema(t *testing.T, instance map[string]any, schemaContent []byte) error { t.Helper() content, err := json.Marshal(instance) diff --git a/internal/modules/dnd/shared/diagnostics/diagnostics.go b/internal/modules/dnd/shared/diagnostics/diagnostics.go index 362bd583..ef69e75d 100644 --- a/internal/modules/dnd/shared/diagnostics/diagnostics.go +++ b/internal/modules/dnd/shared/diagnostics/diagnostics.go @@ -7,6 +7,7 @@ import ( "strconv" "strings" + "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" frameworkdiagnostics "gitea.maximumdirect.net/eric/notarius/internal/framework/diagnostics" ) @@ -25,6 +26,79 @@ type Finding struct { Message string } +// Corrections collects domain-owned model instructions and contextual record +// descriptions without coupling them to operator-facing diagnostics. Rules are +// emitted before affected records so the bounded result remains useful even +// when a large candidate exceeds the display budget. +type Corrections struct { + groups []correctionGroup + indexes map[string]int +} + +type correctionGroup struct { + rule string + records []string + recordsSeen map[string]struct{} +} + +// Add records one semantic rule and, when non-empty, one contextual record to +// which it applies. key is local grouping state and is never rendered. +func (c *Corrections) Add(key, rule, record string) { + if c.indexes == nil { + c.indexes = make(map[string]int) + } + index, ok := c.indexes[key] + if !ok { + index = len(c.groups) + c.indexes[key] = index + c.groups = append(c.groups, correctionGroup{rule: rule, recordsSeen: make(map[string]struct{})}) + } + if record == "" { + return + } + group := &c.groups[index] + if _, seen := group.recordsSeen[record]; seen { + return + } + group.recordsSeen[record] = struct{}{} + group.records = append(group.records, record) +} + +// Guidance returns one bounded correction request. It deliberately renders +// neither grouping keys nor operator diagnostics. +func (c Corrections) Guidance(prefix string) string { + issues := make([]string, 0, len(c.groups)*2) + for _, group := range c.groups { + issues = append(issues, group.rule) + } + for _, group := range c.groups { + issues = append(issues, group.records...) + } + return Aggregate(prefix, issues) +} + +// SourceRange describes cited transcript positions without exposing source +// identities or application entity IDs. +func SourceRange(refs []source.SourceRef) string { + if len(refs) == 0 { + return "without a cited source range" + } + description := SourceRefRange(refs[0]) + if len(refs) > 1 { + description += fmt.Sprintf(" (first of %d cited ranges)", len(refs)) + } + return description +} + +// SourceRefRange describes one transcript range without exposing its source +// identity. +func SourceRefRange(ref source.SourceRef) string { + if ref.StartUnitID == ref.EndUnitID { + return "at source unit " + strconv.Itoa(ref.StartUnitID) + } + return fmt.Sprintf("at source units %d-%d", ref.StartUnitID, ref.EndUnitID) +} + // DataQualityResult converts accepted source-quality findings into bounded, // locally grouped advisories. These findings do not indicate process // degradation. diff --git a/internal/modules/dnd/shared/diagnostics/diagnostics_test.go b/internal/modules/dnd/shared/diagnostics/diagnostics_test.go index 2c863c47..eb0e195d 100644 --- a/internal/modules/dnd/shared/diagnostics/diagnostics_test.go +++ b/internal/modules/dnd/shared/diagnostics/diagnostics_test.go @@ -6,9 +6,40 @@ import ( "testing" "unicode/utf8" + "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" ) +func TestCorrectionsGroupRulesBeforeContextAndHideKeys(t *testing.T) { + var corrections Corrections + corrections.Add("internal-kind", "Use a supported kind.", "Affected goblin at source unit 4.") + corrections.Add("internal-kind", "Use a supported kind.", "Affected ogre at source units 8-9.") + corrections.Add("internal-kind", "Use a supported kind.", "Affected goblin at source unit 4.") + corrections.Add("name", "Provide a contextual name.", "Affected unnamed record at source unit 12.") + + guidance := corrections.Guidance("Correct every record") + if strings.Contains(guidance, "internal-kind") { + t.Fatalf("Guidance() exposed grouping key: %q", guidance) + } + if strings.Count(guidance, "Use a supported kind.") != 1 || strings.Count(guidance, "Affected goblin") != 1 { + t.Fatalf("Guidance() did not de-duplicate rules and records: %q", guidance) + } + if strings.Index(guidance, "Use a supported kind.") > strings.Index(guidance, "Affected goblin") { + t.Fatalf("Guidance() = %q, want rules before records", guidance) + } +} + +func TestSourceRangeUsesOnlyTranscriptPositions(t *testing.T) { + refs := []source.SourceRef{ + {SourceID: "opaque-source", StartUnitID: 8, EndUnitID: 10}, + {SourceID: "opaque-source", StartUnitID: 12, EndUnitID: 12}, + } + got := SourceRange(refs) + if strings.Contains(got, "opaque-source") || !strings.Contains(got, "8-10") || !strings.Contains(got, "first of 2") { + t.Fatalf("SourceRange() = %q", got) + } +} + func TestAggregateEnforcesByteBudgetAndReportsOmissions(t *testing.T) { issues := make([]string, MaxIssues) for index := range issues { diff --git a/internal/modules/dnd/shared/source_ref_coverage.go b/internal/modules/dnd/shared/source_ref_coverage.go new file mode 100644 index 00000000..c6dcfa75 --- /dev/null +++ b/internal/modules/dnd/shared/source_ref_coverage.go @@ -0,0 +1,48 @@ +package shared + +import "gitea.maximumdirect.net/eric/notarius/internal/core/source" + +// ChunkCoverage is an immutable snapshot of the source units materialized in +// one extraction chunk. +type ChunkCoverage struct { + sourceID string + unitIDs map[int]struct{} +} + +// NewChunkCoverage snapshots chunk without retaining or mutating it. +func NewChunkCoverage(chunk *source.Chunk) ChunkCoverage { + if chunk == nil { + return ChunkCoverage{} + } + coverage := ChunkCoverage{ + sourceID: chunk.SourceID, + unitIDs: make(map[int]struct{}, len(chunk.Units)), + } + for _, unit := range chunk.Units { + coverage.unitIDs[unit.ID] = struct{}{} + } + return coverage +} + +// Contains reports whether ref identifies a valid inclusive span in index and +// every unit in that document-ordered span is present in the chunk snapshot. +func (c ChunkCoverage) Contains(index source.DocumentIndex, doc *source.SourceDocument, ref source.SourceRef) bool { + documentID, ok := index.DocumentID() + if !ok || doc == nil || doc.ID != documentID || c.sourceID == "" || ref.SourceID != c.sourceID || ref.SourceID != documentID { + return false + } + start, startOK := index.Position(ref.StartUnitID) + end, endOK := index.Position(ref.EndUnitID) + if !startOK || !endOK || start > end { + return false + } + for position := start; position <= end; position++ { + if position >= len(doc.Units) { + return false + } + if _, found := c.unitIDs[doc.Units[position].ID]; !found { + return false + } + } + return true +} diff --git a/internal/modules/dnd/shared/source_ref_coverage_test.go b/internal/modules/dnd/shared/source_ref_coverage_test.go new file mode 100644 index 00000000..05b5ca96 --- /dev/null +++ b/internal/modules/dnd/shared/source_ref_coverage_test.go @@ -0,0 +1,69 @@ +package shared + +import ( + "testing" + + "gitea.maximumdirect.net/eric/notarius/internal/core/source" +) + +func TestChunkCoverageRequiresCompleteDocumentOrderedSpan(t *testing.T) { + doc := coverageDocument(30, 10, 20, 40) + index := source.NewDocumentIndex(doc) + + for _, test := range []struct { + name string + chunk *source.Chunk + ref source.SourceRef + want bool + }{ + {name: "complete non-monotonic span", chunk: coverageChunk(doc.ID, 30, 10, 20), ref: coverageRef(doc.ID, 30, 20), want: true}, + {name: "single unit", chunk: coverageChunk(doc.ID, 10), ref: coverageRef(doc.ID, 10, 10), want: true}, + {name: "missing middle unit", chunk: coverageChunk(doc.ID, 30, 20), ref: coverageRef(doc.ID, 30, 20)}, + {name: "only endpoints", chunk: coverageChunk(doc.ID, 30, 40), ref: coverageRef(doc.ID, 30, 40)}, + {name: "reversed", chunk: coverageChunk(doc.ID, 30, 10), ref: coverageRef(doc.ID, 10, 30)}, + {name: "wrong source", chunk: coverageChunk(doc.ID, 30), ref: coverageRef("other", 30, 30)}, + {name: "missing endpoint", chunk: coverageChunk(doc.ID, 30), ref: coverageRef(doc.ID, 30, 999)}, + {name: "nil chunk", ref: coverageRef(doc.ID, 30, 30)}, + } { + t.Run(test.name, func(t *testing.T) { + coverage := NewChunkCoverage(test.chunk) + if got := coverage.Contains(index, doc, test.ref); got != test.want { + t.Fatalf("Contains() = %t, want %t", got, test.want) + } + }) + } + if NewChunkCoverage(coverageChunk(doc.ID, 30)).Contains(source.DocumentIndex{}, nil, coverageRef(doc.ID, 30, 30)) { + t.Fatal("Contains() with nil document and zero index = true, want false") + } +} + +func TestChunkCoverageDoesNotRetainMutableChunkState(t *testing.T) { + doc := coverageDocument(1, 2) + chunk := coverageChunk(doc.ID, 1, 2) + coverage := NewChunkCoverage(chunk) + chunk.SourceID = "changed" + chunk.Units[0].ID = 99 + if !coverage.Contains(source.NewDocumentIndex(doc), doc, coverageRef(doc.ID, 1, 2)) { + t.Fatal("Contains() changed after mutating source chunk") + } +} + +func coverageDocument(ids ...int) *source.SourceDocument { + doc := &source.SourceDocument{ID: "session", Units: make([]source.SourceUnit, len(ids))} + for index, id := range ids { + doc.Units[index] = source.SourceUnit{ID: id} + } + return doc +} + +func coverageChunk(sourceID string, ids ...int) *source.Chunk { + chunk := &source.Chunk{SourceID: sourceID, Units: make([]source.SourceUnit, len(ids))} + for index, id := range ids { + chunk.Units[index] = source.SourceUnit{ID: id} + } + return chunk +} + +func coverageRef(sourceID string, start, end int) source.SourceRef { + return source.SourceRef{SourceID: sourceID, StartUnitID: start, EndUnitID: end} +} diff --git a/internal/modules/dnd/validate/combatturns/shape/validator.go b/internal/modules/dnd/validate/combatturns/shape/validator.go index 07516a58..33714169 100644 --- a/internal/modules/dnd/validate/combatturns/shape/validator.go +++ b/internal/modules/dnd/validate/combatturns/shape/validator.go @@ -14,7 +14,7 @@ import ( const ( Key = "extract/dnd/combat-turns/shape" ReasonCode = "invalid_combat_turn_shape" - policy = "dnd.combat_turns.validator.shape.v1" + policy = "dnd.combat_turns.validator.shape.v2" ) type Options struct{} @@ -33,38 +33,48 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.CombatTurnList]) (contracts.ValidationResult, error) { - if err := Validate(req.Value); err != nil { - return rejection(err.Error()), nil + issues, corrections := assess(req.Value) + if len(issues) != 0 { + return rejection( + diagnostics.Aggregate("invalid combat turn shape", issues), + corrections.Guidance("Correct every rejected combat turn and return the complete replacement combat-turn list"), + ), nil } return contracts.ValidationResult{Approved: true}, nil } func Validate(value dnd.CombatTurnList) error { - issues := issuesFor(value) + issues, _ := assess(value) if len(issues) == 0 { return nil } return fmt.Errorf("%s", diagnostics.Aggregate("invalid combat turn shape", issues)) } -func issuesFor(value dnd.CombatTurnList) []string { +func assess(value dnd.CombatTurnList) ([]string, diagnostics.Corrections) { + var corrections diagnostics.Corrections if value.CombatTurns == nil { - return []string{"combat_turns must be present"} + corrections.Add("list", "Return a `combat_turns` array; use an empty array when the scene contains no combat turns.", "") + return []string{"combat_turns must be present"}, corrections } issues := make([]string, 0) for turnIndex, turn := range value.CombatTurns { prefix := fmt.Sprintf("combat_turns[%d]", turnIndex) + record := fmt.Sprintf("Affected %s for actor %s %s.", diagnostics.Quote(string(turn.TurnKind)), diagnostics.Quote(strings.TrimSpace(turn.Actor)), diagnostics.SourceRange(turn.SourceRefs)) if strings.TrimSpace(turn.Actor) == "" { issues = append(issues, prefix+".actor must not be empty: "+diagnostics.Quote(turn.Actor)) + corrections.Add("actor", "Provide the contextual combatant name for every combat turn.", record) } if !validTurnKind(turn.TurnKind) { issues = append(issues, prefix+".turn_kind is unsupported: "+diagnostics.Quote(string(turn.TurnKind))) + corrections.Add("kind", "Set `turn_kind` to exactly one of `turn`, `reaction`, `legendary_action`, `lair_action`, or `other`.", record) } if len(turn.SourceRefs) == 0 { issues = append(issues, prefix+".source_refs must contain at least one reference") + corrections.Add("source-refs", "Provide at least one transcript source range that directly supports every combat turn.", record) } } - return issues + return issues, corrections } func validTurnKind(value dnd.CombatTurnKind) bool { @@ -99,6 +109,6 @@ 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, CorrectionGuidance: "Return a complete combat-turn list with every required field present, valid combatant names, and valid source references."} +func rejection(message, guidance string) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: guidance} } diff --git a/internal/modules/dnd/validate/combatturns/source_refs/validator.go b/internal/modules/dnd/validate/combatturns/source_refs/validator.go index f2e6455f..454c8d9b 100644 --- a/internal/modules/dnd/validate/combatturns/source_refs/validator.go +++ b/internal/modules/dnd/validate/combatturns/source_refs/validator.go @@ -8,6 +8,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" combatshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/combatturns/shape" ) @@ -15,7 +16,7 @@ import ( const ( Key = "extract/dnd/combat-turns/source_refs" ReasonCode = "invalid_combat_turn_source_refs" - policy = "dnd.combat_turns.validator.source_refs.v2" + policy = "dnd.combat_turns.validator.source_refs.v3" ) type Options struct{} @@ -40,11 +41,11 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq if err := combatshape.Validate(req.Value); err != nil { return contracts.ValidationResult{Approved: true}, nil } - var coverage *chunkCoverage + var coverage shared.ChunkCoverage if req.Stage == string(pipeline.StageExtract) { - coverage = newChunkCoverage(req.Chunk) + coverage = shared.NewChunkCoverage(req.Chunk) } - issues := sourceRefIssues(source.NewDocumentIndex(req.Source), req.Source, coverage, req.Value) + issues, corrections := sourceRefIssues(source.NewDocumentIndex(req.Source), req.Source, coverage, req.Stage == string(pipeline.StageExtract), req.Value) if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } @@ -52,54 +53,28 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq 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.", + CorrectionGuidance: corrections.Guidance("Correct every rejected combat-turn citation and return the complete replacement combat-turn list"), }, nil } -func sourceRefIssues(index source.DocumentIndex, doc *source.SourceDocument, coverage *chunkCoverage, value dnd.CombatTurnList) []string { +func sourceRefIssues(index source.DocumentIndex, doc *source.SourceDocument, coverage shared.ChunkCoverage, checkCoverage bool, value dnd.CombatTurnList) ([]string, diagnostics.Corrections) { issues := make([]string, 0) + var corrections diagnostics.Corrections for turnIndex, turn := range value.CombatTurns { for refIndex, ref := range turn.SourceRefs { + record := fmt.Sprintf("Affected %s for actor %s, citing %s.", diagnostics.Quote(string(turn.TurnKind)), diagnostics.Quote(turn.Actor), diagnostics.SourceRefRange(ref)) if err := index.ValidateRef(ref); err != nil { issues = append(issues, fmt.Sprintf("combat_turns[%d].source_refs[%d]: %s", turnIndex, refIndex, diagnostics.Truncate(err.Error()))) + corrections.Add("valid-range", "Use positive source range endpoints that occur in the supplied transcript, with the earlier unit first.", record) continue } - if coverage != nil && !coverage.contains(doc, ref) { + if checkCoverage && !coverage.Contains(index, doc, ref) { issues = append(issues, fmt.Sprintf("combat_turns[%d].source_refs[%d]: source reference is outside the current extraction chunk", turnIndex, refIndex)) + corrections.Add("chunk-range", "Use only source ranges wholly contained in the supplied extraction chunk.", record) } } } - return issues -} - -type chunkCoverage struct { - sourceID string - unitIDs map[int]struct{} -} - -func newChunkCoverage(chunk *source.Chunk) *chunkCoverage { - coverage := &chunkCoverage{sourceID: chunk.SourceID, unitIDs: make(map[int]struct{}, len(chunk.Units))} - for _, unit := range chunk.Units { - coverage.unitIDs[unit.ID] = struct{}{} - } - return coverage -} - -func (coverage *chunkCoverage) contains(doc *source.SourceDocument, ref source.SourceRef) bool { - if coverage == nil || doc == nil || ref.SourceID != coverage.sourceID { - return false - } - start, startOK := source.UnitIndex(doc, ref.StartUnitID) - end, endOK := source.UnitIndex(doc, ref.EndUnitID) - if !startOK || !endOK || start > end { - return false - } - for position := start; position <= end; position++ { - if _, found := coverage.unitIDs[doc.Units[position].ID]; !found { - return false - } - } - return true + return issues, corrections } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/enemyevents/engagements/validator.go b/internal/modules/dnd/validate/enemyevents/engagements/validator.go index 266955cf..bab9f161 100644 --- a/internal/modules/dnd/validate/enemyevents/engagements/validator.go +++ b/internal/modules/dnd/validate/enemyevents/engagements/validator.go @@ -16,7 +16,7 @@ import ( const ( Key = "extract/dnd/enemy-events/engagements" ReasonCode = "duplicate_enemy_engagement" - policy = "dnd.enemy_events.validator.engagements.v1" + policy = "dnd.enemy_events.validator.engagements.v2" ) type Options struct{} @@ -38,7 +38,9 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq if enemyeventshape.Validate(req.Value) != nil { return contracts.ValidationResult{Approved: true}, nil } - seen := make(map[string]struct{}) + seen := make(map[string]dnd.EnemyEvent) + issues := make([]string, 0) + var corrections diagnostics.Corrections for _, event := range req.Value.Events { if event.Kind != dnd.EnemyEventKindEngaged { continue @@ -47,17 +49,24 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq if identity == "" { continue } - if _, found := seen[identity]; found { - return contracts.ValidationResult{ - Approved: false, - ReasonCode: ReasonCode, - 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 + if first, found := seen[identity]; found { + issues = append(issues, fmt.Sprintf("subject %s has more than one engagement in one combat scene", diagnostics.Quote(event.Name))) + corrections.Add( + "duplicate-subject", + "Return at most one `engaged` event for each contextual enemy name within this combat scene; keep the source ranges together on that one event.", + fmt.Sprintf("Affected enemy %s: first engagement %s; additional engagement %s.", diagnostics.Quote(event.Name), diagnostics.SourceRange(first.SourceRefs), diagnostics.SourceRange(event.SourceRefs)), + ) + continue } - seen[identity] = struct{}{} + seen[identity] = event + } + if len(issues) != 0 { + return contracts.ValidationResult{ + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("duplicate enemy engagement", issues), + CorrectionGuidance: corrections.Guidance("Correct every duplicate enemy engagement and return the complete replacement event list"), + }, nil } return contracts.ValidationResult{Approved: true}, nil } diff --git a/internal/modules/dnd/validate/enemyevents/engagements/validator_test.go b/internal/modules/dnd/validate/enemyevents/engagements/validator_test.go index 6855f242..35b909eb 100644 --- a/internal/modules/dnd/validate/enemyevents/engagements/validator_test.go +++ b/internal/modules/dnd/validate/enemyevents/engagements/validator_test.go @@ -66,6 +66,27 @@ func TestValidatorRegistersStrictOptionsAndPolicy(t *testing.T) { } } +func TestValidatorReportsEveryDuplicateSubjectWithContext(t *testing.T) { + value := events( + event("Ashfang", dnd.EnemyEventKindEngaged, 1), + event("Ashfang", dnd.EnemyEventKindEngaged, 2), + event("Briar", dnd.EnemyEventKindEngaged, 3), + event("Briar", dnd.EnemyEventKindEngaged, 4), + ) + result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.EnemyEventList]{Value: value}) + if err != nil || result.Approved { + t.Fatalf("Validate() = %#v, %v; want rejection", result, err) + } + for _, want := range []string{"Ashfang", "Briar", "source unit 1", "source unit 2", "source unit 3", "source unit 4"} { + if !strings.Contains(result.CorrectionGuidance, want) { + t.Fatalf("CorrectionGuidance = %q, want %q", result.CorrectionGuidance, want) + } + } + if strings.Contains(result.CorrectionGuidance, "events[") || strings.Contains(result.CorrectionGuidance, ReasonCode) { + t.Fatalf("CorrectionGuidance exposed internal diagnostics: %q", result.CorrectionGuidance) + } +} + func events(values ...dnd.EnemyEvent) dnd.EnemyEventList { return dnd.EnemyEventList{Events: values} } func event(name string, kind dnd.EnemyEventKind, unitID int) dnd.EnemyEvent { diff --git a/internal/modules/dnd/validate/enemyevents/shape/validator.go b/internal/modules/dnd/validate/enemyevents/shape/validator.go index 8105804d..e91b8a61 100644 --- a/internal/modules/dnd/validate/enemyevents/shape/validator.go +++ b/internal/modules/dnd/validate/enemyevents/shape/validator.go @@ -16,7 +16,7 @@ import ( const ( Key = "extract/dnd/enemy-events/shape" ReasonCode = "invalid_enemy_event_shape" - policy = "dnd.enemy_events.validator.shape.v1" + policy = "dnd.enemy_events.validator.shape.v2" ) type Options struct{} @@ -35,38 +35,50 @@ 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(), CorrectionGuidance: "Return a complete enemy-event list with every required field present, valid contextual NPC names, supported event values, and valid source references."}, nil + issues, corrections := assess(req.Value) + if len(issues) != 0 { + return contracts.ValidationResult{ + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid enemy event shape", issues), + CorrectionGuidance: corrections.Guidance("Correct every rejected enemy event and return the complete replacement event list"), + }, nil } return contracts.ValidationResult{Approved: true}, nil } func Validate(value dnd.EnemyEventList) error { - issues := issuesFor(value) + issues, _ := assess(value) if len(issues) == 0 { return nil } return fmt.Errorf("%s", diagnostics.Aggregate("invalid enemy event shape", issues)) } -func issuesFor(value dnd.EnemyEventList) []string { +func assess(value dnd.EnemyEventList) ([]string, diagnostics.Corrections) { + var corrections diagnostics.Corrections if value.Events == nil { - return []string{"events must be present"} + corrections.Add("list", "Return an `events` array; use an empty array when the combat scene establishes no enemy events.", "") + return []string{"events must be present"}, corrections } issues := make([]string, 0) for eventIndex, event := range value.Events { prefix := fmt.Sprintf("events[%d]", eventIndex) + record := fmt.Sprintf("Affected %s event for enemy %s %s.", diagnostics.Quote(string(event.Kind)), diagnostics.Quote(strings.TrimSpace(event.Name)), diagnostics.SourceRange(event.SourceRefs)) if strings.TrimSpace(event.Name) == "" { issues = append(issues, prefix+".name must not be empty: "+diagnostics.Quote(event.Name)) + corrections.Add("name", "Select a nonblank contextual enemy name from the supplied NPC registry for every event.", record) } if !enemyeventmodel.SupportedKind(event.Kind) { issues = append(issues, prefix+".kind is unsupported: "+diagnostics.Quote(string(event.Kind))) + corrections.Add("kind", "Set `kind` to exactly one of `engaged`, `killed`, `fled`, `captured`, or `incapacitated`.", record) } if len(event.SourceRefs) == 0 { issues = append(issues, prefix+".source_refs must contain at least one reference") + corrections.Add("source-refs", "Provide at least one transcript source range that directly supports every enemy event.", record) } } - return issues + return issues, corrections } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/enemyevents/source_refs/validator.go b/internal/modules/dnd/validate/enemyevents/source_refs/validator.go index 5e52c0a5..2b71f713 100644 --- a/internal/modules/dnd/validate/enemyevents/source_refs/validator.go +++ b/internal/modules/dnd/validate/enemyevents/source_refs/validator.go @@ -9,6 +9,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" enemyeventshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/enemyevents/shape" ) @@ -16,7 +17,7 @@ import ( const ( Key = "extract/dnd/enemy-events/source_refs" ReasonCode = "invalid_enemy_event_source_refs" - policy = "dnd.enemy_events.validator.source_refs.v1" + policy = "dnd.enemy_events.validator.source_refs.v2" ) type Options struct{} @@ -42,61 +43,40 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } index := source.NewDocumentIndex(req.Source) - var coverage *chunkCoverage + var coverage shared.ChunkCoverage if req.Stage == string(pipeline.StageExtract) { - coverage = newChunkCoverage(req.Chunk) + coverage = shared.NewChunkCoverage(req.Chunk) } - issues := sourceRefIssues(index, req.Source, coverage, req.Value) + issues, corrections := sourceRefIssues(index, req.Source, coverage, req.Stage == string(pipeline.StageExtract), req.Value) 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), CorrectionGuidance: "Return enemy events whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each event."}, nil + return contracts.ValidationResult{ + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid enemy event source references", issues), + CorrectionGuidance: corrections.Guidance("Correct every rejected enemy-event citation and return the complete replacement event list"), + }, nil } -func sourceRefIssues(index source.DocumentIndex, doc *source.SourceDocument, coverage *chunkCoverage, value dnd.EnemyEventList) []string { +func sourceRefIssues(index source.DocumentIndex, doc *source.SourceDocument, coverage shared.ChunkCoverage, checkCoverage bool, value dnd.EnemyEventList) ([]string, diagnostics.Corrections) { issues := make([]string, 0) + var corrections diagnostics.Corrections for eventIndex, event := range value.Events { for refIndex, ref := range event.SourceRefs { + record := fmt.Sprintf("Affected %s event for enemy %s, citing %s.", diagnostics.Quote(string(event.Kind)), diagnostics.Quote(event.Name), diagnostics.SourceRefRange(ref)) if err := index.ValidateRef(ref); err != nil { issues = append(issues, fmt.Sprintf("events[%d].source_refs[%d]: %s", eventIndex, refIndex, diagnostics.Truncate(err.Error()))) + corrections.Add("valid-range", "Use positive source range endpoints that occur in the supplied transcript, with the earlier unit first.", record) continue } - if coverage != nil && !coverage.contains(doc, ref) { + if checkCoverage && !coverage.Contains(index, doc, ref) { issues = append(issues, fmt.Sprintf("events[%d].source_refs[%d]: source reference is outside the current extraction chunk", eventIndex, refIndex)) + corrections.Add("chunk-range", "Use only source ranges wholly contained in the supplied extraction chunk.", record) } } } - return issues -} - -type chunkCoverage struct { - sourceID string - unitIDs map[int]struct{} -} - -func newChunkCoverage(chunk *source.Chunk) *chunkCoverage { - coverage := &chunkCoverage{sourceID: chunk.SourceID, unitIDs: make(map[int]struct{}, len(chunk.Units))} - for _, unit := range chunk.Units { - coverage.unitIDs[unit.ID] = struct{}{} - } - return coverage -} - -func (coverage *chunkCoverage) contains(doc *source.SourceDocument, ref source.SourceRef) bool { - if coverage == nil || doc == nil || ref.SourceID != coverage.sourceID { - return false - } - start, startOK := source.UnitIndex(doc, ref.StartUnitID) - end, endOK := source.UnitIndex(doc, ref.EndUnitID) - if !startOK || !endOK || start > end { - return false - } - for position := start; position <= end; position++ { - if _, found := coverage.unitIDs[doc.Units[position].ID]; !found { - return false - } - } - return true + return issues, corrections } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/itemoccurrences/registry/validator.go b/internal/modules/dnd/validate/itemoccurrences/registry/validator.go index 5d432f55..2460041c 100644 --- a/internal/modules/dnd/validate/itemoccurrences/registry/validator.go +++ b/internal/modules/dnd/validate/itemoccurrences/registry/validator.go @@ -16,7 +16,7 @@ import ( const ( Key = "extract/dnd/item-occurrences/registry" ReasonCode = "invalid_item_occurrence_registry" - policy = "dnd.item_occurrences.validator.registry.v1" + policy = "dnd.item_occurrences.validator.registry.v2" ) type Options struct{} @@ -76,27 +76,33 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{}, fmt.Errorf("resolve item registry: %w", err) } if !registry.Bound() { - return rejection([]string{"item registry reference is required"}), nil + var corrections diagnostics.Corrections + corrections.Add("registry", "Use only contextual item names from the supplied item registry; omit an occurrence that cannot be matched unambiguously.", "") + return rejection([]string{"item registry reference is required"}, corrections), nil } issues := make([]string, 0) + var corrections diagnostics.Corrections for index, occurrence := range req.Value.Occurrences { + record := fmt.Sprintf("Affected %s occurrence for item %s %s.", diagnostics.Quote(string(occurrence.Kind)), diagnostics.Quote(occurrence.Name), diagnostics.SourceRange(occurrence.SourceRefs)) item, ok := registry.LookupID(occurrence.ItemID) if !ok { issues = append(issues, fmt.Sprintf("occurrences[%d].item_id is not in the item registry: %s", index, diagnostics.Quote(occurrence.ItemID))) + corrections.Add("registry", "Use only contextual item names from the supplied item registry; omit an occurrence that cannot be matched unambiguously.", record) continue } if occurrence.Name != item.Name { issues = append(issues, fmt.Sprintf("occurrences[%d] does not match registry item %s", index, diagnostics.Quote(occurrence.ItemID))) + corrections.Add("canonical-name", "Use the exact contextual item name supplied by the registry.", record+" Use registry name "+diagnostics.Quote(item.Name)+".") } } if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return rejection(issues), nil + return rejection(issues, corrections), nil } -func rejection(issues []string) contracts.ValidationResult { - 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 rejection(issues []string, corrections diagnostics.Corrections) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid item occurrence registry", issues), CorrectionGuidance: corrections.Guidance("Correct every rejected item occurrence and return the complete replacement occurrence list")} } 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 44ee6256..bb0e85d2 100644 --- a/internal/modules/dnd/validate/itemoccurrences/shape/validator.go +++ b/internal/modules/dnd/validate/itemoccurrences/shape/validator.go @@ -16,7 +16,7 @@ import ( const ( Key = "extract/dnd/item-occurrences/shape" ReasonCode = "invalid_item_occurrence_shape" - policy = "dnd.item_occurrences.shape.v2" + policy = "dnd.item_occurrences.shape.v3" ) type Options struct{} @@ -57,20 +57,12 @@ func Validate(value dnd.ItemOccurrenceList) error { } type validationAssessment struct { - operatorIssues []string - correctionGroups []correctionGroup - groupIndexes map[string]int -} - -type correctionGroup struct { - label string - rule string - records []string - recordsSeen map[string]struct{} + operatorIssues []string + corrections diagnostics.Corrections } func assess(value dnd.ItemOccurrenceList) validationAssessment { - assessment := validationAssessment{groupIndexes: make(map[string]int)} + var assessment validationAssessment if value.Occurrences == nil { assessment.operatorIssues = append(assessment.operatorIssues, "occurrences must be present") assessment.addCorrection("occurrences", "item-occurrence list", "Return an `occurrences` array; use an empty array when the transcript establishes no occurrences.", "") @@ -107,20 +99,10 @@ func assess(value dnd.ItemOccurrenceList) validationAssessment { } func (assessment *validationAssessment) addCorrection(key, label, rule, record string) { - index, ok := assessment.groupIndexes[key] - if !ok { - index = len(assessment.correctionGroups) - assessment.groupIndexes[key] = index - assessment.correctionGroups = append(assessment.correctionGroups, correctionGroup{label: label, rule: rule, recordsSeen: make(map[string]struct{})}) - } if record != "" { - group := &assessment.correctionGroups[index] - if _, seen := group.recordsSeen[record]; seen { - return - } - group.recordsSeen[record] = struct{}{} - group.records = append(group.records, record) + record = "Affected " + label + " record: " + record } + assessment.corrections.Add(key, rule, record) } func (assessment validationAssessment) operatorMessage() string { @@ -128,16 +110,7 @@ func (assessment validationAssessment) operatorMessage() string { } func (assessment validationAssessment) correctionGuidance() string { - issues := make([]string, 0, len(assessment.correctionGroups)+len(assessment.operatorIssues)) - for _, group := range assessment.correctionGroups { - issues = append(issues, group.rule) - } - for _, group := range assessment.correctionGroups { - for _, record := range group.records { - issues = append(issues, "Affected "+group.label+" record: "+record) - } - } - return diagnostics.Aggregate("Correct every rejected item occurrence and return the complete replacement list", issues) + return assessment.corrections.Guidance("Correct every rejected item occurrence and return the complete replacement list") } func holderOperatorIssue(occurrence dnd.ItemOccurrence) string { @@ -159,7 +132,7 @@ func holderExpectation(kind dnd.ItemOccurrenceKind) string { case dnd.ItemOccurrenceKindConsumed: return "from present and to absent" case dnd.ItemOccurrenceKindTransferred: - return "distinct named non-party holders" + return "distinct named party-member holders" default: return "a supported holder combination" } @@ -188,19 +161,7 @@ func occurrenceContext(occurrence dnd.ItemOccurrence) string { if name == "" { context = "item with a blank contextual name" } - if len(occurrence.SourceRefs) == 0 { - context += " without a cited source range" - } else { - ref := occurrence.SourceRefs[0] - if ref.StartUnitID == ref.EndUnitID { - context += fmt.Sprintf(" at source unit %d", ref.StartUnitID) - } else { - context += fmt.Sprintf(" at source units %d-%d", ref.StartUnitID, ref.EndUnitID) - } - if len(occurrence.SourceRefs) > 1 { - context += fmt.Sprintf(" (first of %d cited ranges)", len(occurrence.SourceRefs)) - } - } + context += " " + diagnostics.SourceRange(occurrence.SourceRefs) return context + " with from " + holderDisplay(occurrence.From) + " and to " + holderDisplay(occurrence.To) } diff --git a/internal/modules/dnd/validate/itemoccurrences/source_refs/validator.go b/internal/modules/dnd/validate/itemoccurrences/source_refs/validator.go index 203505ef..8ce1475e 100644 --- a/internal/modules/dnd/validate/itemoccurrences/source_refs/validator.go +++ b/internal/modules/dnd/validate/itemoccurrences/source_refs/validator.go @@ -9,6 +9,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" itemoccurrenceshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/itemoccurrences/shape" ) @@ -16,7 +17,7 @@ import ( const ( Key = "extract/dnd/item-occurrences/source_refs" ReasonCode = "invalid_item_occurrence_source_references" - policy = "dnd.item_occurrences.source_refs.v1" + policy = "dnd.item_occurrences.source_refs.v2" ) type Options struct{} @@ -42,59 +43,35 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } index := source.NewDocumentIndex(req.Source) - var coverage *chunkCoverage + var coverage shared.ChunkCoverage if req.Stage == string(pipeline.StageExtract) { - coverage = newChunkCoverage(req.Chunk) + coverage = shared.NewChunkCoverage(req.Chunk) } issues := make([]string, 0) + var corrections diagnostics.Corrections for occurrenceIndex, occurrence := range req.Value.Occurrences { for refIndex, ref := range occurrence.SourceRefs { + record := fmt.Sprintf("Affected %s occurrence for item %s, citing %s.", diagnostics.Quote(string(occurrence.Kind)), diagnostics.Quote(occurrence.Name), diagnostics.SourceRefRange(ref)) if err := index.ValidateRef(ref); err != nil { issues = append(issues, fmt.Sprintf("occurrences[%d].source_refs[%d]: %s", occurrenceIndex, refIndex, diagnostics.Truncate(err.Error()))) + corrections.Add("valid-range", "Use positive source range endpoints that occur in the supplied transcript, with the earlier unit first.", record) continue } - if coverage != nil && !coverage.contains(req.Source, ref) { + if req.Stage == string(pipeline.StageExtract) && !coverage.Contains(index, req.Source, ref) { issues = append(issues, fmt.Sprintf("occurrences[%d].source_refs[%d]: source reference is outside the current extraction chunk", occurrenceIndex, refIndex)) + corrections.Add("chunk-range", "Use only source ranges wholly contained in the supplied extraction chunk.", record) } } } 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), 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 { - sourceID string - unitIDs map[int]struct{} -} - -func newChunkCoverage(chunk *source.Chunk) *chunkCoverage { - coverage := &chunkCoverage{ - sourceID: chunk.SourceID, - unitIDs: make(map[int]struct{}, len(chunk.Units)), - } - for _, unit := range chunk.Units { - coverage.unitIDs[unit.ID] = struct{}{} - } - return coverage -} - -func (coverage *chunkCoverage) contains(doc *source.SourceDocument, ref source.SourceRef) bool { - if coverage == nil || doc == nil || ref.SourceID != coverage.sourceID { - return false - } - start, startOK := source.UnitIndex(doc, ref.StartUnitID) - end, endOK := source.UnitIndex(doc, ref.EndUnitID) - if !startOK || !endOK || start > end { - return false - } - for position := start; position <= end; position++ { - if _, found := coverage.unitIDs[doc.Units[position].ID]; !found { - return false - } - } - return true + return contracts.ValidationResult{ + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid item occurrence source references", issues), + CorrectionGuidance: corrections.Guidance("Correct every rejected item-occurrence citation and return the complete replacement occurrence list"), + }, nil } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/itemregistry/identity/validator.go b/internal/modules/dnd/validate/itemregistry/identity/validator.go index 961a2371..9830f026 100644 --- a/internal/modules/dnd/validate/itemregistry/identity/validator.go +++ b/internal/modules/dnd/validate/itemregistry/identity/validator.go @@ -14,9 +14,10 @@ import ( ) const ( - Key = "normalize/dnd/item-registry/identity" - ReasonCode = "invalid_item_identity" - policy = domainidentity.Policy + Key = "normalize/dnd/item-registry/identity" + ReasonCode = "invalid_item_identity" + policy = domainidentity.Policy + correctionPolicy = "dnd.item_registry.validator.identity.v2" ) type Options struct{} @@ -31,7 +32,7 @@ func (v *Validator) ExecutionClass() contracts.ExecutionClass { return contracts.ExecutionClassDeterministic } func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { - return []pipeline.CheckpointFingerprint{{Name: "policy", Value: policy}} + return []pipeline.CheckpointFingerprint{{Name: "policy", Value: policy}, {Name: "correction_policy", Value: correctionPolicy}} } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.ItemRegistry]) (contracts.ValidationResult, error) { @@ -43,10 +44,31 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } issues := make([]string, len(identityIssues)) + var corrections diagnostics.Corrections for index, issue := range identityIssues { issues[index] = fmt.Sprintf("items[%d] %s: %s", issue.RecordIndex, issue.Code, diagnostics.Quote(issue.Value)) + if issue.RecordIndex < 0 || issue.RecordIndex >= len(req.Value.Items) { + continue + } + item := req.Value.Items[issue.RecordIndex] + corrections.Add(string(issue.Code), itemIdentityCorrection(issue.Code), fmt.Sprintf("Affected item %s %s.", diagnostics.Quote(item.Name), diagnostics.SourceRange(item.SourceRefs))) + } + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid item identity", issues), CorrectionGuidance: corrections.Guidance("Correct the duplicate-group proposals and return the complete replacement proposal response")}, nil +} + +func itemIdentityCorrection(code domainidentity.IssueCode) string { + switch code { + case domainidentity.IssueEmptyCanonicalName: + return "Select a canonical proposal member with a nonblank, transcript-supported item name." + case domainidentity.IssueDuplicateCanonical: + return "Put duplicate mentions of the same item in one proposal group and select one transcript-supported canonical member." + case domainidentity.IssueInvalidID, domainidentity.IssueIDMismatch: + return "Revise the proposal so its canonical item member has a valid transcript-supported name; Notarius derives durable identity without model input." + case domainidentity.IssueDuplicateID: + return "Do not use proposals that collapse distinct items into one canonical identity; group only records that describe the same item." + default: + return "Revise the duplicate-group proposal so every canonical item is transcript-supported and distinct." } - 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/identity/validator_test.go b/internal/modules/dnd/validate/itemregistry/identity/validator_test.go index 7f18bd57..8979d37a 100644 --- a/internal/modules/dnd/validate/itemregistry/identity/validator_test.go +++ b/internal/modules/dnd/validate/itemregistry/identity/validator_test.go @@ -21,6 +21,9 @@ func TestValidatorRejectsInvalidAndDuplicateItemIdentity(t *testing.T) { if err != nil || result.Approved || result.ReasonCode != ReasonCode || !strings.Contains(result.Message, "invalid_id") || !strings.Contains(result.Message, "duplicate_canonical_identity") { t.Fatalf("Validate() = %#v, %v; want identity rejection", result, err) } + if !strings.Contains(result.CorrectionGuidance, "duplicate-group proposals") || !strings.Contains(result.CorrectionGuidance, "Rope") || strings.Contains(result.CorrectionGuidance, "wrong") || strings.Contains(result.CorrectionGuidance, "items[") { + t.Fatalf("CorrectionGuidance = %q, want contextual proposal guidance without IDs or operator paths", result.CorrectionGuidance) + } } func TestValidatorRegistersPolicy(t *testing.T) { @@ -31,7 +34,7 @@ func TestValidatorRegistersPolicy(t *testing.T) { if got, ok := registry.Spec(Key); !ok || got != Spec() || got.ExecutionClass != contracts.ExecutionClassDeterministic { t.Fatalf("registered spec = %#v, present = %t", got, ok) } - if got := New(Options{}).CheckpointFingerprints(); len(got) != 1 || got[0].Value != domainidentity.Policy { + if got := New(Options{}).CheckpointFingerprints(); len(got) != 2 || got[0].Value != domainidentity.Policy || got[1].Name != "correction_policy" || got[1].Value != correctionPolicy { t.Fatalf("fingerprints = %#v", got) } if _, err := DecodeOptions(map[string]any{"unexpected": true}); err == nil { diff --git a/internal/modules/dnd/validate/itemregistry/shape/validator.go b/internal/modules/dnd/validate/itemregistry/shape/validator.go index 4dce38fe..e3d807c5 100644 --- a/internal/modules/dnd/validate/itemregistry/shape/validator.go +++ b/internal/modules/dnd/validate/itemregistry/shape/validator.go @@ -15,7 +15,7 @@ import ( const ( Key = "extract/dnd/item-registry/shape" ReasonCode = "invalid_item_shape" - policy = "dnd.item_registry.validator.shape.v1" + policy = "dnd.item_registry.validator.shape.v2" ) type Options struct{} @@ -34,39 +34,58 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.ItemRegistry]) (contracts.ValidationResult, error) { - issues := issuesFor(req.Value) + normalization := req.Stage == string(pipeline.StageNormalize) + issues, corrections := assess(req.Value, normalization) if len(issues) > 0 { - return rejection(diagnostics.Aggregate("invalid item shape", issues)), nil + prefix := "Correct every rejected item and return the complete replacement registry" + if normalization { + prefix = "Correct the duplicate-group proposals and return the complete replacement proposal response" + } + return rejection(diagnostics.Aggregate("invalid item shape", issues), corrections.Guidance(prefix)), nil } return contracts.ValidationResult{Approved: true}, nil } func Validate(value dnd.ItemRegistry) error { - issues := issuesFor(value) + issues, _ := assess(value, false) if len(issues) == 0 { return nil } return fmt.Errorf("%s", diagnostics.Aggregate("invalid item shape", issues)) } -func issuesFor(value dnd.ItemRegistry) []string { +func assess(value dnd.ItemRegistry, normalization bool) ([]string, diagnostics.Corrections) { + var corrections diagnostics.Corrections + nameRule := "Provide one nonblank, transcript-supported item name for every registry entry." + refRule := "Provide at least one transcript source range that directly supports every named item." + listRule := "Return an `items` array; use an empty array when the transcript establishes no named items." + if normalization { + nameRule = "Revise the duplicate-group proposals so every selected canonical item has a nonblank, transcript-supported name." + refRule = "Revise the duplicate-group proposals so every selected canonical item preserves direct transcript evidence." + listRule = "Revise the duplicate-group proposals so normalization retains the complete item candidate registry." + } if value.Items == nil { - return []string{"items must be present"} + corrections.Add("list", listRule, "") + return []string{"items must be present"}, corrections } issues := make([]string, 0) for index, item := range value.Items { prefix := fmt.Sprintf("items[%d]", index) + record := "Affected item " + diagnostics.Quote(strings.TrimSpace(item.Name)) + " " + diagnostics.SourceRange(item.SourceRefs) + "." if strings.TrimSpace(item.ID) == "" { issues = append(issues, prefix+".id must not be empty") + corrections.Add("name", nameRule, record) } if strings.TrimSpace(item.Name) == "" { issues = append(issues, prefix+".name must not be empty") + corrections.Add("name", nameRule, record) } if len(item.SourceRefs) == 0 { issues = append(issues, prefix+".source_refs must not be empty") + corrections.Add("source-refs", refRule, record) } } - return issues + return issues, corrections } func Spec() pipeline.ValidatorSpec { @@ -92,6 +111,6 @@ 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, CorrectionGuidance: "Return a complete item registry containing only properly named items with valid source references."} +func rejection(message, guidance string) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: guidance} } diff --git a/internal/modules/dnd/validate/itemregistry/source_refs/validator.go b/internal/modules/dnd/validate/itemregistry/source_refs/validator.go index 3d16777c..9e035899 100644 --- a/internal/modules/dnd/validate/itemregistry/source_refs/validator.go +++ b/internal/modules/dnd/validate/itemregistry/source_refs/validator.go @@ -9,6 +9,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" itemshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/itemregistry/shape" ) @@ -16,7 +17,7 @@ import ( const ( Key = "extract/dnd/item-registry/source_refs" ReasonCode = "invalid_item_source_refs" - policy = "dnd.item_registry.validator.source_refs.v1" + policy = "dnd.item_registry.validator.source_refs.v2" ) type Options struct{} @@ -42,35 +43,37 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } index := source.NewDocumentIndex(req.Source) + coverage := shared.NewChunkCoverage(req.Chunk) issues := make([]string, 0) + var corrections diagnostics.Corrections + normalization := req.Stage == string(pipeline.StageNormalize) + validRangeRule := "Use positive source range endpoints that occur in the supplied transcript, with the earlier unit first." + guidancePrefix := "Correct every rejected item citation and return the complete replacement item registry" + if normalization { + validRangeRule = "Revise the duplicate-group proposals so every selected canonical item preserves valid transcript evidence; do not reproduce application IDs." + guidancePrefix = "Correct the duplicate-group proposals and return the complete replacement proposal response" + } for itemIndex, item := range req.Value.Items { for refIndex, ref := range item.SourceRefs { + record := fmt.Sprintf("Affected item %s, citing %s.", diagnostics.Quote(item.Name), diagnostics.SourceRefRange(ref)) if err := index.ValidateRef(ref); err != nil { issues = append(issues, fmt.Sprintf("items[%d].source_refs[%d]: %s", itemIndex, refIndex, diagnostics.Truncate(err.Error()))) + corrections.Add("valid-range", validRangeRule, record) continue } - if req.Stage == string(pipeline.StageExtract) && !chunkContainsRef(req.Chunk, ref) { + if req.Stage == string(pipeline.StageExtract) && !coverage.Contains(index, req.Source, ref) { issues = append(issues, fmt.Sprintf("items[%d].source_refs[%d]: source reference is outside the current extraction chunk", itemIndex, refIndex)) + corrections.Add("chunk-range", "Use only source ranges wholly contained in the supplied extraction chunk.", record) } } } if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return rejection(diagnostics.Aggregate("invalid item source references", issues)), nil -} - -func chunkContainsRef(chunk *source.Chunk, ref source.SourceRef) bool { - if chunk == nil || ref.SourceID != chunk.SourceID { - return false - } - startFound := false - endFound := false - for _, unit := range chunk.Units { - startFound = startFound || unit.ID == ref.StartUnitID - endFound = endFound || unit.ID == ref.EndUnitID - } - return startFound && endFound + return rejection( + diagnostics.Aggregate("invalid item source references", issues), + corrections.Guidance(guidancePrefix), + ), nil } func Spec() pipeline.ValidatorSpec { @@ -96,6 +99,6 @@ 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, CorrectionGuidance: "Return registry items whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each named item."} +func rejection(message, guidance string) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: guidance} } diff --git a/internal/modules/dnd/validate/itemregistry/source_refs/validator_test.go b/internal/modules/dnd/validate/itemregistry/source_refs/validator_test.go index b1edc315..fea66cdb 100644 --- a/internal/modules/dnd/validate/itemregistry/source_refs/validator_test.go +++ b/internal/modules/dnd/validate/itemregistry/source_refs/validator_test.go @@ -47,6 +47,17 @@ func TestValidatorDefersMalformedShapeAndRegisters(t *testing.T) { } } +func TestValidatorRequiresEveryUnitInCitedSpanToBeInChunk(t *testing.T) { + doc := &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 30}, {ID: 10}, {ID: 20}}} + value := validItemRegistry() + value.Items[0].SourceRefs = []source.SourceRef{{SourceID: doc.ID, StartUnitID: 30, EndUnitID: 20}} + chunk := &source.Chunk{SourceID: doc.ID, Units: []source.SourceUnit{{ID: 30}, {ID: 20}}} + result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.ItemRegistry]{Stage: string(pipeline.StageExtract), Source: doc, Chunk: chunk, Value: value}) + if err != nil || result.Approved || !strings.Contains(result.Message, "outside the current extraction chunk") { + t.Fatalf("Validate() = %#v, %v; want missing-middle-unit rejection", result, err) + } +} + func request(doc *source.SourceDocument, value dnd.ItemRegistry) contracts.TypedValidationRequest[dnd.ItemRegistry] { return contracts.TypedValidationRequest[dnd.ItemRegistry]{Source: doc, Value: value} } diff --git a/internal/modules/dnd/validate/locationoccurrences/registry/validator.go b/internal/modules/dnd/validate/locationoccurrences/registry/validator.go index 1891356d..5d0d722a 100644 --- a/internal/modules/dnd/validate/locationoccurrences/registry/validator.go +++ b/internal/modules/dnd/validate/locationoccurrences/registry/validator.go @@ -16,7 +16,7 @@ import ( const ( Key = "extract/dnd/location-occurrences/registry" ReasonCode = "invalid_location_occurrence_registry" - policy = "dnd.location_occurrences.validator.registry.v1" + policy = "dnd.location_occurrences.validator.registry.v2" ) type Options struct{} @@ -76,27 +76,33 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{}, fmt.Errorf("resolve location registry: %w", err) } if !registry.Bound() { - return rejection([]string{"location registry reference is required"}), nil + var corrections diagnostics.Corrections + corrections.Add("registry", "Use only proper contextual location names from the supplied location registry; omit an occurrence that cannot be matched unambiguously.", "") + return rejection([]string{"location registry reference is required"}, corrections), nil } issues := make([]string, 0) + var corrections diagnostics.Corrections for index, occurrence := range req.Value.Occurrences { + record := fmt.Sprintf("Affected %s occurrence for location %s %s.", diagnostics.Quote(string(occurrence.Kind)), diagnostics.Quote(occurrence.Name), diagnostics.SourceRange(occurrence.SourceRefs)) location, ok := registry.Lookup(occurrence.LocationID) if !ok { issues = append(issues, fmt.Sprintf("occurrences[%d].location_id is not in the location registry: %s", index, diagnostics.Quote(occurrence.LocationID))) + corrections.Add("registry", "Use only proper contextual location names from the supplied location registry; omit an occurrence that cannot be matched unambiguously.", record) continue } if occurrence.Name != location.Name { issues = append(issues, fmt.Sprintf("occurrences[%d] does not match registry location %s", index, diagnostics.Quote(occurrence.LocationID))) + corrections.Add("canonical-name", "Use the exact proper location name supplied by the registry.", record+" Use registry name "+diagnostics.Quote(location.Name)+".") } } if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return rejection(issues), nil + return rejection(issues, corrections), nil } -func rejection(issues []string) contracts.ValidationResult { - 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 rejection(issues []string, corrections diagnostics.Corrections) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: diagnostics.Aggregate("invalid location occurrence registry", issues), CorrectionGuidance: corrections.Guidance("Correct every rejected location occurrence and return the complete replacement occurrence list")} } 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 fdb574a4..89049c02 100644 --- a/internal/modules/dnd/validate/locationoccurrences/shape/validator.go +++ b/internal/modules/dnd/validate/locationoccurrences/shape/validator.go @@ -15,7 +15,7 @@ import ( const ( Key = "extract/dnd/location-occurrences/shape" ReasonCode = "invalid_location_occurrence_shape" - policy = "dnd.location_occurrences.validator.shape.v1" + policy = "dnd.location_occurrences.validator.shape.v2" ) type Options struct{} @@ -34,41 +34,54 @@ 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(), CorrectionGuidance: "Return a complete location-occurrence list with every required field present, valid contextual location names, supported occurrence kinds, and valid source references."}, nil + issues, corrections := assess(req.Value) + if len(issues) != 0 { + return contracts.ValidationResult{ + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid location occurrence shape", issues), + CorrectionGuidance: corrections.Guidance("Correct every rejected location occurrence and return the complete replacement occurrence list"), + }, nil } return contracts.ValidationResult{Approved: true}, nil } func Validate(value dnd.LocationOccurrenceList) error { - issues := issuesFor(value) + issues, _ := assess(value) if len(issues) == 0 { return nil } return fmt.Errorf("%s", diagnostics.Aggregate("invalid location occurrence shape", issues)) } -func issuesFor(value dnd.LocationOccurrenceList) []string { +func assess(value dnd.LocationOccurrenceList) ([]string, diagnostics.Corrections) { + var corrections diagnostics.Corrections if value.Occurrences == nil { - return []string{"occurrences must be present"} + corrections.Add("list", "Return an `occurrences` array; use an empty array when the transcript establishes no location occurrences.", "") + return []string{"occurrences must be present"}, corrections } issues := make([]string, 0) for index, occurrence := range value.Occurrences { prefix := fmt.Sprintf("occurrences[%d]", index) + record := fmt.Sprintf("Affected %s occurrence for location %s %s.", diagnostics.Quote(string(occurrence.Kind)), diagnostics.Quote(strings.TrimSpace(occurrence.Name)), diagnostics.SourceRange(occurrence.SourceRefs)) if strings.TrimSpace(occurrence.LocationID) == "" { issues = append(issues, prefix+".location_id must not be empty") + corrections.Add("name", "Select a nonblank proper location name from the supplied registry for every occurrence.", record) } if strings.TrimSpace(occurrence.Name) == "" { issues = append(issues, prefix+".name must not be empty") + corrections.Add("name", "Select a nonblank proper location name from the supplied registry for every occurrence.", record) } if !validKind(occurrence.Kind) { issues = append(issues, prefix+".kind is unsupported: "+diagnostics.Quote(string(occurrence.Kind))) + corrections.Add("kind", "Set `kind` to exactly one of `visited`, `planned`, `recalled`, or `mentioned`.", record) } if len(occurrence.SourceRefs) == 0 { issues = append(issues, prefix+".source_refs must contain at least one reference") + corrections.Add("source-refs", "Provide at least one transcript source range that directly supports every location occurrence.", record) } } - return issues + return issues, corrections } func validKind(value dnd.LocationOccurrenceKind) bool { diff --git a/internal/modules/dnd/validate/locationoccurrences/source_refs/validator.go b/internal/modules/dnd/validate/locationoccurrences/source_refs/validator.go index 20e4f58c..dc72870e 100644 --- a/internal/modules/dnd/validate/locationoccurrences/source_refs/validator.go +++ b/internal/modules/dnd/validate/locationoccurrences/source_refs/validator.go @@ -9,6 +9,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" occurrenceshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/locationoccurrences/shape" ) @@ -16,7 +17,7 @@ import ( const ( Key = "extract/dnd/location-occurrences/source_refs" ReasonCode = "invalid_location_occurrence_source_refs" - policy = "dnd.location_occurrences.validator.source_refs.v1" + policy = "dnd.location_occurrences.validator.source_refs.v2" ) type Options struct{} @@ -42,35 +43,34 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } index := source.NewDocumentIndex(req.Source) + coverage := shared.NewChunkCoverage(req.Chunk) issues := make([]string, 0) + var corrections diagnostics.Corrections for occurrenceIndex, occurrence := range req.Value.Occurrences { for refIndex, ref := range occurrence.SourceRefs { + record := fmt.Sprintf("Affected %s occurrence for location %s, citing %s.", diagnostics.Quote(string(occurrence.Kind)), diagnostics.Quote(occurrence.Name), diagnostics.SourceRefRange(ref)) if err := index.ValidateRef(ref); err != nil { issues = append(issues, fmt.Sprintf("occurrences[%d].source_refs[%d]: %s", occurrenceIndex, refIndex, diagnostics.Truncate(err.Error()))) + corrections.Add("valid-range", "Use positive source range endpoints that occur in the supplied transcript, with the earlier unit first.", record) continue } - if req.Stage == string(pipeline.StageExtract) && !chunkContainsRef(req.Chunk, ref) { + if req.Stage == string(pipeline.StageExtract) && !coverage.Contains(index, req.Source, ref) { issues = append(issues, fmt.Sprintf("occurrences[%d].source_refs[%d]: source reference is outside the current extraction chunk", occurrenceIndex, refIndex)) + corrections.Add("chunk-range", "Use only source ranges wholly contained in the supplied extraction chunk.", record) } } } 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), CorrectionGuidance: "Return location occurrences whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each occurrence."}, nil + return contracts.ValidationResult{ + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid location occurrence source references", issues), + CorrectionGuidance: corrections.Guidance("Correct every rejected location-occurrence citation and return the complete replacement occurrence list"), + }, nil } -func chunkContainsRef(chunk *source.Chunk, ref source.SourceRef) bool { - if chunk == nil || ref.SourceID != chunk.SourceID { - return false - } - startFound, endFound := false, false - for _, unit := range chunk.Units { - startFound = startFound || unit.ID == ref.StartUnitID - endFound = endFound || unit.ID == ref.EndUnitID - } - return startFound && endFound -} func Spec() pipeline.ValidatorSpec { return pipeline.ValidatorSpec{Key: Key, ExecutionClass: contracts.ExecutionClassDeterministic} } diff --git a/internal/modules/dnd/validate/locationregistry/identity/validator.go b/internal/modules/dnd/validate/locationregistry/identity/validator.go index 99f1e34a..3e8bb559 100644 --- a/internal/modules/dnd/validate/locationregistry/identity/validator.go +++ b/internal/modules/dnd/validate/locationregistry/identity/validator.go @@ -14,9 +14,10 @@ import ( ) const ( - Key = "normalize/dnd/location-registry/identity" - ReasonCode = "invalid_location_identity" - policy = domainidentity.Policy + Key = "normalize/dnd/location-registry/identity" + ReasonCode = "invalid_location_identity" + policy = domainidentity.Policy + correctionPolicy = "dnd.location_registry.validator.identity.v2" ) type Options struct{} @@ -31,7 +32,7 @@ func (v *Validator) ExecutionClass() contracts.ExecutionClass { return contracts.ExecutionClassDeterministic } func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { - return []pipeline.CheckpointFingerprint{{Name: "policy", Value: policy}} + return []pipeline.CheckpointFingerprint{{Name: "policy", Value: policy}, {Name: "correction_policy", Value: correctionPolicy}} } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.LocationRegistry]) (contracts.ValidationResult, error) { @@ -43,17 +44,38 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } issues := make([]string, len(identityIssues)) + var corrections diagnostics.Corrections for index, issue := range identityIssues { issues[index] = fmt.Sprintf("locations[%d] %s: %s", issue.RecordIndex, issue.Code, diagnostics.Quote(issue.Value)) + if issue.RecordIndex < 0 || issue.RecordIndex >= len(req.Value.Locations) { + continue + } + location := req.Value.Locations[issue.RecordIndex] + corrections.Add(string(issue.Code), locationIdentityCorrection(issue.Code), fmt.Sprintf("Affected location %s %s.", diagnostics.Quote(location.Name), diagnostics.SourceRange(location.SourceRefs))) } return contracts.ValidationResult{ 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.", + CorrectionGuidance: corrections.Guidance("Correct the duplicate-group proposals and return the complete replacement proposal response"), }, nil } +func locationIdentityCorrection(code domainidentity.IssueCode) string { + switch code { + case domainidentity.IssueEmptyCanonicalName: + return "Select a canonical proposal member with a nonblank, transcript-supported proper location name." + case domainidentity.IssueMissingEvidence: + return "Select a canonical proposal member that has direct transcript evidence; do not discard all supported evidence for a location." + case domainidentity.IssueInvalidID, domainidentity.IssueIDMismatch: + return "Revise the proposal so its canonical location member has a valid name and evidence anchor; Notarius derives durable identity without model input." + case domainidentity.IssueDuplicateID: + return "Do not use proposals that collapse distinct locations or evidence anchors into one canonical identity; group only records that describe the same location." + default: + return "Revise the duplicate-group proposal so every canonical location is transcript-supported and distinct." + } +} + func Spec() pipeline.ValidatorSpec { return pipeline.ValidatorSpec{Key: Key, ExecutionClass: contracts.ExecutionClassDeterministic} } diff --git a/internal/modules/dnd/validate/locationregistry/identity/validator_test.go b/internal/modules/dnd/validate/locationregistry/identity/validator_test.go index 9e18610c..df6bcc65 100644 --- a/internal/modules/dnd/validate/locationregistry/identity/validator_test.go +++ b/internal/modules/dnd/validate/locationregistry/identity/validator_test.go @@ -47,10 +47,13 @@ func TestValidatorDefersShapeAndRejectsDerivationAndDuplicateID(t *testing.T) { t.Fatalf("message %q missing %q", result.Message, want) } } + if !strings.Contains(result.CorrectionGuidance, "duplicate-group proposals") || !strings.Contains(result.CorrectionGuidance, "Gate") || strings.Contains(result.CorrectionGuidance, "not-an-id") || strings.Contains(result.CorrectionGuidance, "locations[") { + t.Fatalf("CorrectionGuidance = %q, want contextual proposal guidance without IDs or operator paths", result.CorrectionGuidance) + } } func TestValidatorRegistersIdentityPolicy(t *testing.T) { - if got := New(Options{}).CheckpointFingerprints(); len(got) != 1 || got[0].Value != domainidentity.Policy { + if got := New(Options{}).CheckpointFingerprints(); len(got) != 2 || got[0].Value != domainidentity.Policy || got[1].Name != "correction_policy" || got[1].Value != correctionPolicy { t.Fatalf("fingerprints = %#v", got) } registry := pipeline.NewValidatorRegistry() diff --git a/internal/modules/dnd/validate/locationregistry/shape/validator.go b/internal/modules/dnd/validate/locationregistry/shape/validator.go index 931fdbad..90343224 100644 --- a/internal/modules/dnd/validate/locationregistry/shape/validator.go +++ b/internal/modules/dnd/validate/locationregistry/shape/validator.go @@ -15,7 +15,7 @@ import ( const ( Key = "extract/dnd/location-registry/shape" ReasonCode = "invalid_location_shape" - policy = "dnd.location_registry.validator.shape.v1" + policy = "dnd.location_registry.validator.shape.v2" ) type Options struct{} @@ -34,39 +34,58 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.LocationRegistry]) (contracts.ValidationResult, error) { - issues := issuesFor(req.Value) + normalization := req.Stage == string(pipeline.StageNormalize) + issues, corrections := assess(req.Value, normalization) if len(issues) > 0 { - return rejection(diagnostics.Aggregate("invalid location shape", issues)), nil + prefix := "Correct every rejected location and return the complete replacement registry" + if normalization { + prefix = "Correct the duplicate-group proposals and return the complete replacement proposal response" + } + return rejection(diagnostics.Aggregate("invalid location shape", issues), corrections.Guidance(prefix)), nil } return contracts.ValidationResult{Approved: true}, nil } func Validate(value dnd.LocationRegistry) error { - issues := issuesFor(value) + issues, _ := assess(value, false) if len(issues) == 0 { return nil } return fmt.Errorf("%s", diagnostics.Aggregate("invalid location shape", issues)) } -func issuesFor(value dnd.LocationRegistry) []string { +func assess(value dnd.LocationRegistry, normalization bool) ([]string, diagnostics.Corrections) { + var corrections diagnostics.Corrections + nameRule := "Provide one nonblank, transcript-supported proper location name for every registry entry." + refRule := "Provide at least one transcript source range that directly supports every named location." + listRule := "Return a `locations` array; use an empty array when the transcript establishes no named locations." + if normalization { + nameRule = "Revise the duplicate-group proposals so every selected canonical location has a nonblank, transcript-supported proper name." + refRule = "Revise the duplicate-group proposals so every selected canonical location preserves direct transcript evidence." + listRule = "Revise the duplicate-group proposals so normalization retains the complete location candidate registry." + } if value.Locations == nil { - return []string{"locations must be present"} + corrections.Add("list", listRule, "") + return []string{"locations must be present"}, corrections } issues := make([]string, 0) for index, location := range value.Locations { prefix := fmt.Sprintf("locations[%d]", index) + record := "Affected location " + diagnostics.Quote(strings.TrimSpace(location.Name)) + " " + diagnostics.SourceRange(location.SourceRefs) + "." if strings.TrimSpace(location.ID) == "" { issues = append(issues, prefix+".id must not be empty") + corrections.Add("name", nameRule, record) } if strings.TrimSpace(location.Name) == "" { issues = append(issues, prefix+".name must not be empty") + corrections.Add("name", nameRule, record) } if len(location.SourceRefs) == 0 { issues = append(issues, prefix+".source_refs must not be empty") + corrections.Add("source-refs", refRule, record) } } - return issues + return issues, corrections } func Spec() pipeline.ValidatorSpec { @@ -92,6 +111,6 @@ 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, CorrectionGuidance: "Return a complete location registry containing only properly named locations with valid source references."} +func rejection(message, guidance string) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: guidance} } diff --git a/internal/modules/dnd/validate/locationregistry/source_refs/validator.go b/internal/modules/dnd/validate/locationregistry/source_refs/validator.go index c4b8ed5b..9ae617f9 100644 --- a/internal/modules/dnd/validate/locationregistry/source_refs/validator.go +++ b/internal/modules/dnd/validate/locationregistry/source_refs/validator.go @@ -9,6 +9,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" locationshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/locationregistry/shape" ) @@ -16,7 +17,7 @@ import ( const ( Key = "extract/dnd/location-registry/source_refs" ReasonCode = "invalid_location_source_refs" - policy = "dnd.location_registry.validator.source_refs.v2" + policy = "dnd.location_registry.validator.source_refs.v3" ) type Options struct{} @@ -42,35 +43,37 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } index := source.NewDocumentIndex(req.Source) + coverage := shared.NewChunkCoverage(req.Chunk) issues := make([]string, 0) + var corrections diagnostics.Corrections + normalization := req.Stage == string(pipeline.StageNormalize) + validRangeRule := "Use positive source range endpoints that occur in the supplied transcript, with the earlier unit first." + guidancePrefix := "Correct every rejected location citation and return the complete replacement location registry" + if normalization { + validRangeRule = "Revise the duplicate-group proposals so every selected canonical location preserves valid transcript evidence; do not reproduce application IDs." + guidancePrefix = "Correct the duplicate-group proposals and return the complete replacement proposal response" + } for locationIndex, location := range req.Value.Locations { for refIndex, ref := range location.SourceRefs { + record := fmt.Sprintf("Affected location %s, citing %s.", diagnostics.Quote(location.Name), diagnostics.SourceRefRange(ref)) if err := index.ValidateRef(ref); err != nil { issues = append(issues, fmt.Sprintf("locations[%d].source_refs[%d]: %s", locationIndex, refIndex, diagnostics.Truncate(err.Error()))) + corrections.Add("valid-range", validRangeRule, record) continue } - if req.Stage == string(pipeline.StageExtract) && !chunkContainsRef(req.Chunk, ref) { + if req.Stage == string(pipeline.StageExtract) && !coverage.Contains(index, req.Source, ref) { issues = append(issues, fmt.Sprintf("locations[%d].source_refs[%d]: source reference is outside the current extraction chunk", locationIndex, refIndex)) + corrections.Add("chunk-range", "Use only source ranges wholly contained in the supplied extraction chunk.", record) } } } if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return rejection(diagnostics.Aggregate("invalid location source references", issues)), nil -} - -func chunkContainsRef(chunk *source.Chunk, ref source.SourceRef) bool { - if chunk == nil || ref.SourceID != chunk.SourceID { - return false - } - startFound := false - endFound := false - for _, unit := range chunk.Units { - startFound = startFound || unit.ID == ref.StartUnitID - endFound = endFound || unit.ID == ref.EndUnitID - } - return startFound && endFound + return rejection( + diagnostics.Aggregate("invalid location source references", issues), + corrections.Guidance(guidancePrefix), + ), nil } func Spec() pipeline.ValidatorSpec { @@ -96,6 +99,6 @@ 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, CorrectionGuidance: "Return registry locations whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each proper location name."} +func rejection(message, guidance string) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: guidance} } diff --git a/internal/modules/dnd/validate/npcoccurrences/registry/validator.go b/internal/modules/dnd/validate/npcoccurrences/registry/validator.go index 7db8b66e..dd69ab9f 100644 --- a/internal/modules/dnd/validate/npcoccurrences/registry/validator.go +++ b/internal/modules/dnd/validate/npcoccurrences/registry/validator.go @@ -16,7 +16,7 @@ import ( const ( Key = "extract/dnd/npc-occurrences/registry" ReasonCode = "invalid_npc_occurrence_registry" - policy = "dnd.npc_occurrences.validator.registry.v1" + policy = "dnd.npc_occurrences.validator.registry.v2" ) type Options struct{} @@ -84,31 +84,37 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{}, fmt.Errorf("resolve NPC registry: %w", err) } if !npcRegistry.Bound() { - return rejection([]string{"NPC registry reference is required"}), nil + var corrections diagnostics.Corrections + corrections.Add("registry", "Use only contextual NPC names from the supplied NPC registry; omit an occurrence that cannot be matched unambiguously.", "") + return rejection([]string{"NPC registry reference is required"}, corrections), nil } issues := make([]string, 0) + var corrections diagnostics.Corrections for index, occurrence := range req.Value.Occurrences { + record := fmt.Sprintf("Affected %s occurrence for NPC %s %s.", diagnostics.Quote(string(occurrence.Kind)), diagnostics.Quote(occurrence.Name), diagnostics.SourceRange(occurrence.SourceRefs)) canonical, ok := npcRegistry.LookupID(occurrence.NPCID) if !ok { issues = append(issues, fmt.Sprintf("occurrences[%d].npc_id is not in the NPC registry: %s", index, diagnostics.Quote(occurrence.NPCID))) + corrections.Add("registry", "Use only contextual NPC names from the supplied NPC registry; omit an occurrence that cannot be matched unambiguously.", record) continue } if occurrence.Name != canonical.Name { issues = append(issues, fmt.Sprintf("occurrences[%d].name does not match npc_id: %s", index, diagnostics.Quote(occurrence.Name))) + corrections.Add("canonical-name", "Use the exact contextual NPC name supplied by the registry.", record+" Use registry name "+diagnostics.Quote(canonical.Name)+".") } } if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return rejection(issues), nil + return rejection(issues, corrections), nil } -func rejection(issues []string) contracts.ValidationResult { +func rejection(issues []string, corrections diagnostics.Corrections) contracts.ValidationResult { return contracts.ValidationResult{ 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.", + CorrectionGuidance: corrections.Guidance("Correct every rejected NPC occurrence and return the complete replacement occurrence list"), } } diff --git a/internal/modules/dnd/validate/npcoccurrences/shape/validator.go b/internal/modules/dnd/validate/npcoccurrences/shape/validator.go index 07936e1c..18b5faf8 100644 --- a/internal/modules/dnd/validate/npcoccurrences/shape/validator.go +++ b/internal/modules/dnd/validate/npcoccurrences/shape/validator.go @@ -15,7 +15,7 @@ import ( const ( Key = "extract/dnd/npc-occurrences/shape" ReasonCode = "invalid_npc_occurrence_shape" - policy = "dnd.npc_occurrences.validator.shape.v1" + policy = "dnd.npc_occurrences.validator.shape.v2" ) type Options struct{} @@ -34,41 +34,54 @@ 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(), CorrectionGuidance: "Return a complete NPC-occurrence list with every required field present, valid contextual NPC names, supported interaction kinds, and valid source references."}, nil + issues, corrections := assess(req.Value) + if len(issues) != 0 { + return contracts.ValidationResult{ + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid NPC occurrence shape", issues), + CorrectionGuidance: corrections.Guidance("Correct every rejected NPC occurrence and return the complete replacement occurrence list"), + }, nil } return contracts.ValidationResult{Approved: true}, nil } func Validate(value dnd.NPCOccurrenceList) error { - issues := issuesFor(value) + issues, _ := assess(value) if len(issues) == 0 { return nil } return fmt.Errorf("%s", diagnostics.Aggregate("invalid NPC occurrence shape", issues)) } -func issuesFor(value dnd.NPCOccurrenceList) []string { +func assess(value dnd.NPCOccurrenceList) ([]string, diagnostics.Corrections) { + var corrections diagnostics.Corrections if value.Occurrences == nil { - return []string{"occurrences must be present"} + corrections.Add("list", "Return an `occurrences` array; use an empty array when the transcript establishes no NPC occurrences.", "") + return []string{"occurrences must be present"}, corrections } issues := make([]string, 0) for index, occurrence := range value.Occurrences { prefix := fmt.Sprintf("occurrences[%d]", index) + record := fmt.Sprintf("Affected %s occurrence for NPC %s %s.", diagnostics.Quote(string(occurrence.Kind)), diagnostics.Quote(strings.TrimSpace(occurrence.Name)), diagnostics.SourceRange(occurrence.SourceRefs)) if strings.TrimSpace(occurrence.NPCID) == "" { issues = append(issues, prefix+".npc_id must not be empty: "+diagnostics.Quote(occurrence.NPCID)) + corrections.Add("name", "Select a nonblank contextual NPC name from the supplied registry for every occurrence.", record) } if strings.TrimSpace(occurrence.Name) == "" { issues = append(issues, prefix+".name must not be empty: "+diagnostics.Quote(occurrence.Name)) + corrections.Add("name", "Select a nonblank contextual NPC name from the supplied registry for every occurrence.", record) } if !validKind(occurrence.Kind) { issues = append(issues, prefix+".kind is unsupported: "+diagnostics.Quote(string(occurrence.Kind))) + corrections.Add("kind", "Set `kind` to exactly one of `mentioned`, `noncombat_presence`, `dialogue`, `combat_ally`, `combat_opponent`, or `other`.", record) } if len(occurrence.SourceRefs) == 0 { issues = append(issues, prefix+".source_refs must contain at least one reference") + corrections.Add("source-refs", "Provide at least one transcript source range that directly supports every NPC occurrence.", record) } } - return issues + return issues, corrections } func validKind(value dnd.NPCOccurrenceKind) bool { diff --git a/internal/modules/dnd/validate/npcoccurrences/source_refs/validator.go b/internal/modules/dnd/validate/npcoccurrences/source_refs/validator.go index 47010a42..b1feee91 100644 --- a/internal/modules/dnd/validate/npcoccurrences/source_refs/validator.go +++ b/internal/modules/dnd/validate/npcoccurrences/source_refs/validator.go @@ -9,6 +9,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" occurrenceshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcoccurrences/shape" ) @@ -16,7 +17,7 @@ import ( const ( Key = "extract/dnd/npc-occurrences/source_refs" ReasonCode = "invalid_npc_occurrence_source_refs" - policy = "dnd.npc_occurrences.validator.source_refs.v2" + policy = "dnd.npc_occurrences.validator.source_refs.v3" ) type Options struct{} @@ -42,18 +43,23 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } index := source.NewDocumentIndex(req.Source) + coverage := shared.NewChunkCoverage(req.Chunk) issues := make([]string, 0) + var corrections diagnostics.Corrections for occurrenceIndex, occurrence := range req.Value.Occurrences { for refIndex, ref := range occurrence.SourceRefs { + record := fmt.Sprintf("Affected %s occurrence for NPC %s, citing %s.", diagnostics.Quote(string(occurrence.Kind)), diagnostics.Quote(occurrence.Name), diagnostics.SourceRefRange(ref)) if err := index.ValidateRef(ref); err != nil { issues = append(issues, fmt.Sprintf("occurrences[%d].source_refs[%d]: %s", occurrenceIndex, refIndex, diagnostics.Truncate(err.Error()))) + corrections.Add("valid-range", "Use positive source range endpoints that occur in the supplied transcript, with the earlier unit first.", record) continue } - if req.Stage == string(pipeline.StageExtract) && !chunkContainsRef(req.Chunk, ref) { + if req.Stage == string(pipeline.StageExtract) && !coverage.Contains(index, req.Source, ref) { issues = append(issues, fmt.Sprintf( "occurrences[%d].source_refs[%d]: source reference is outside the current extraction chunk", occurrenceIndex, refIndex, )) + corrections.Add("chunk-range", "Use only source ranges wholly contained in the supplied extraction chunk.", record) } } } @@ -64,23 +70,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq 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.", + CorrectionGuidance: corrections.Guidance("Correct every rejected NPC-occurrence citation and return the complete replacement occurrence list"), }, nil } -func chunkContainsRef(chunk *source.Chunk, ref source.SourceRef) bool { - if chunk == nil || ref.SourceID != chunk.SourceID { - return false - } - startFound := false - endFound := false - for _, unit := range chunk.Units { - startFound = startFound || unit.ID == ref.StartUnitID - endFound = endFound || unit.ID == ref.EndUnitID - } - return startFound && endFound -} - func Spec() pipeline.ValidatorSpec { return pipeline.ValidatorSpec{Key: Key, ExecutionClass: contracts.ExecutionClassDeterministic} } diff --git a/internal/modules/dnd/validate/npcregistry/identity/validator.go b/internal/modules/dnd/validate/npcregistry/identity/validator.go index 06db3bf6..60f2e2be 100644 --- a/internal/modules/dnd/validate/npcregistry/identity/validator.go +++ b/internal/modules/dnd/validate/npcregistry/identity/validator.go @@ -14,9 +14,10 @@ import ( ) const ( - Key = "normalize/dnd/npc-registry/identity" - ReasonCode = "invalid_npc_identity" - policy = domainidentity.Policy + Key = "normalize/dnd/npc-registry/identity" + ReasonCode = "invalid_npc_identity" + policy = domainidentity.Policy + correctionPolicy = "dnd.npc_registry.validator.identity.v2" ) type Options struct{} @@ -33,7 +34,7 @@ func (v *Validator) ExecutionClass() contracts.ExecutionClass { } func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { - return []pipeline.CheckpointFingerprint{{Name: "policy", Value: policy}} + return []pipeline.CheckpointFingerprint{{Name: "policy", Value: policy}, {Name: "correction_policy", Value: correctionPolicy}} } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.NPCRegistry]) (contracts.ValidationResult, error) { @@ -46,18 +47,39 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq } issues := make([]string, len(identityIssues)) + var corrections diagnostics.Corrections for index, issue := range identityIssues { location := fmt.Sprintf("npcs[%d]", issue.RecordIndex) issues[index] = fmt.Sprintf("%s %s: %s", location, issue.Code, diagnostics.Quote(issue.Value)) + if issue.RecordIndex < 0 || issue.RecordIndex >= len(req.Value.NPCs) { + continue + } + npc := req.Value.NPCs[issue.RecordIndex] + corrections.Add(string(issue.Code), npcIdentityCorrection(issue.Code), fmt.Sprintf("Affected NPC %s %s.", diagnostics.Quote(npc.Name), diagnostics.SourceRange(npc.SourceRefs))) } return contracts.ValidationResult{ 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.", + CorrectionGuidance: corrections.Guidance("Correct the duplicate-group proposals and return the complete replacement proposal response"), }, nil } +func npcIdentityCorrection(code domainidentity.IssueCode) string { + switch code { + case domainidentity.IssueEmptyCanonicalName: + return "Select a canonical proposal member with a nonblank, transcript-supported proper NPC name." + case domainidentity.IssueDuplicateCanonical: + return "Put duplicate mentions of the same NPC in one proposal group and select one transcript-supported canonical member." + case domainidentity.IssueInvalidID, domainidentity.IssueIDMismatch: + return "Revise the proposal so its canonical NPC member has a valid transcript-supported name; Notarius derives durable identity without model input." + case domainidentity.IssueDuplicateID: + return "Do not use proposals that collapse distinct NPCs into one canonical identity; group only records that describe the same NPC." + default: + return "Revise the duplicate-group proposal so every canonical NPC is transcript-supported and distinct." + } +} + func Spec() pipeline.ValidatorSpec { return pipeline.ValidatorSpec{Key: Key, ExecutionClass: contracts.ExecutionClassDeterministic} } diff --git a/internal/modules/dnd/validate/npcregistry/identity/validator_test.go b/internal/modules/dnd/validate/npcregistry/identity/validator_test.go index 0527dea0..9fc1b1c7 100644 --- a/internal/modules/dnd/validate/npcregistry/identity/validator_test.go +++ b/internal/modules/dnd/validate/npcregistry/identity/validator_test.go @@ -21,7 +21,7 @@ func TestValidatorContractAndRegistration(t *testing.T) { if _, err := DecodeOptions(map[string]any{"unexpected": true}); err == nil || !strings.Contains(err.Error(), "unknown option") { t.Fatalf("DecodeOptions() error = %v, want unknown option error", err) } - if got := New(Options{}).CheckpointFingerprints(); len(got) != 1 || got[0].Name != "policy" || got[0].Value != policy { + if got := New(Options{}).CheckpointFingerprints(); len(got) != 2 || got[0].Name != "policy" || got[0].Value != policy || got[1].Name != "correction_policy" || got[1].Value != correctionPolicy { t.Fatalf("fingerprints = %#v, want identity policy", got) } @@ -65,6 +65,9 @@ func TestValidatorDefersShapeAndRejectsIdentityIssues(t *testing.T) { t.Fatalf("identity message %q missing %q", result.Message, want) } } + if !strings.Contains(result.CorrectionGuidance, "duplicate-group proposals") || !strings.Contains(result.CorrectionGuidance, value.NPCs[0].Name) || strings.Contains(result.CorrectionGuidance, value.NPCs[0].ID) || strings.Contains(result.CorrectionGuidance, "npcs[") { + t.Fatalf("CorrectionGuidance = %q, want contextual proposal guidance without IDs or operator paths", result.CorrectionGuidance) + } } func TestValidatorBoundsUnicodeDiagnostics(t *testing.T) { diff --git a/internal/modules/dnd/validate/npcregistry/shape/validator.go b/internal/modules/dnd/validate/npcregistry/shape/validator.go index dd27735a..20d73ef2 100644 --- a/internal/modules/dnd/validate/npcregistry/shape/validator.go +++ b/internal/modules/dnd/validate/npcregistry/shape/validator.go @@ -14,7 +14,7 @@ import ( const ( Key = "extract/dnd/npc-registry/shape" ReasonCode = "invalid_npc_shape" - policy = "dnd.npc_registry.validator.shape.v1" + policy = "dnd.npc_registry.validator.shape.v2" ) type Options struct{} @@ -33,39 +33,58 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.NPCRegistry]) (contracts.ValidationResult, error) { - issues := issuesFor(req.Value) + normalization := req.Stage == string(pipeline.StageNormalize) + issues, corrections := assess(req.Value, normalization) if len(issues) > 0 { - return rejection(diagnostics.Aggregate("invalid NPC shape", issues)), nil + prefix := "Correct every rejected NPC and return the complete replacement registry" + if normalization { + prefix = "Correct the duplicate-group proposals and return the complete replacement proposal response" + } + return rejection(diagnostics.Aggregate("invalid NPC shape", issues), corrections.Guidance(prefix)), nil } return contracts.ValidationResult{Approved: true}, nil } func Validate(value dnd.NPCRegistry) error { - issues := issuesFor(value) + issues, _ := assess(value, false) if len(issues) == 0 { return nil } return fmt.Errorf("%s", diagnostics.Aggregate("invalid NPC shape", issues)) } -func issuesFor(value dnd.NPCRegistry) []string { +func assess(value dnd.NPCRegistry, normalization bool) ([]string, diagnostics.Corrections) { + var corrections diagnostics.Corrections + nameRule := "Provide one nonblank, transcript-supported proper NPC name for every registry entry." + refRule := "Provide at least one transcript source range that directly supports every named NPC." + listRule := "Return an `npcs` array; use an empty array when the transcript establishes no named NPCs." + if normalization { + nameRule = "Revise the duplicate-group proposals so every selected canonical NPC has a nonblank, transcript-supported proper name." + refRule = "Revise the duplicate-group proposals so every selected canonical NPC preserves direct transcript evidence." + listRule = "Revise the duplicate-group proposals so normalization retains the complete NPC candidate registry." + } issues := make([]string, 0) if value.NPCs == nil { - return []string{"npcs must be present"} + corrections.Add("list", listRule, "") + return []string{"npcs must be present"}, corrections } for index, npc := range value.NPCs { prefix := fmt.Sprintf("npcs[%d]", index) + record := "Affected NPC " + diagnostics.Quote(strings.TrimSpace(npc.Name)) + " " + diagnostics.SourceRange(npc.SourceRefs) + "." if strings.TrimSpace(npc.ID) == "" { issues = append(issues, prefix+".id must not be empty") + corrections.Add("name", nameRule, record) } if strings.TrimSpace(npc.Name) == "" { issues = append(issues, prefix+".name must not be empty") + corrections.Add("name", nameRule, record) } if len(npc.SourceRefs) == 0 { issues = append(issues, prefix+".source_refs must not be empty") + corrections.Add("source-refs", refRule, record) } } - return issues + return issues, corrections } func Spec() pipeline.ValidatorSpec { @@ -91,6 +110,6 @@ 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, CorrectionGuidance: "Return a complete NPC registry containing only properly named NPCs with valid source references."} +func rejection(message, guidance string) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: guidance} } diff --git a/internal/modules/dnd/validate/npcregistry/shape/validator_test.go b/internal/modules/dnd/validate/npcregistry/shape/validator_test.go index cbe0be85..e72084da 100644 --- a/internal/modules/dnd/validate/npcregistry/shape/validator_test.go +++ b/internal/modules/dnd/validate/npcregistry/shape/validator_test.go @@ -50,7 +50,7 @@ func TestValidatorBoundsDiagnosticsAndQuotesUnicode(t *testing.T) { } func TestValidatorSpecCheckpointAndRegistration(t *testing.T) { - if got := New(Options{}).CheckpointFingerprints(); len(got) != 1 || got[0].Name != "policy" || got[0].Value != "dnd.npc_registry.validator.shape.v1" { + if got := New(Options{}).CheckpointFingerprints(); len(got) != 1 || got[0].Name != "policy" || got[0].Value != policy { t.Fatalf("CheckpointFingerprints() = %#v, want local policy", got) } if spec := Spec(); spec.Key != Key || spec.ExecutionClass != contracts.ExecutionClassDeterministic { @@ -74,6 +74,21 @@ func TestValidatorDoesNotMutateValue(t *testing.T) { } } +func TestValidatorUsesProposalGuidanceDuringNormalization(t *testing.T) { + value := validNPCRegistry() + value.NPCs[0].ID = "npc:sha256:opaque" + value.NPCs[0].Name = "" + req := requestWithValue(value) + req.Stage = string(pipeline.StageNormalize) + result, err := New(Options{}).Validate(context.Background(), req) + if err != nil || result.Approved { + t.Fatalf("Validate() = %#v, %v; want rejection", result, err) + } + if !strings.Contains(result.CorrectionGuidance, "duplicate-group proposals") || strings.Contains(result.CorrectionGuidance, value.NPCs[0].ID) || strings.Contains(result.CorrectionGuidance, "npcs[") { + t.Fatalf("CorrectionGuidance = %q, want proposal guidance without IDs or operator paths", result.CorrectionGuidance) + } +} + func requestWithValue(value dnd.NPCRegistry) contracts.TypedValidationRequest[dnd.NPCRegistry] { return contracts.TypedValidationRequest[dnd.NPCRegistry]{Value: value} } diff --git a/internal/modules/dnd/validate/npcregistry/source_refs/validator.go b/internal/modules/dnd/validate/npcregistry/source_refs/validator.go index 22d6e180..9619c072 100644 --- a/internal/modules/dnd/validate/npcregistry/source_refs/validator.go +++ b/internal/modules/dnd/validate/npcregistry/source_refs/validator.go @@ -8,6 +8,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" npcshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcregistry/shape" ) @@ -15,7 +16,7 @@ import ( const ( Key = "extract/dnd/npc-registry/source_refs" ReasonCode = "invalid_npc_source_refs" - policy = "dnd.npc_registry.validator.source_refs.v2" + policy = "dnd.npc_registry.validator.source_refs.v3" ) type Options struct{} @@ -41,56 +42,40 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } index := source.NewDocumentIndex(req.Source) - var coverage *chunkCoverage + var coverage shared.ChunkCoverage if req.Stage == string(pipeline.StageExtract) { - coverage = newChunkCoverage(req.Chunk) + coverage = shared.NewChunkCoverage(req.Chunk) } issues := make([]string, 0) + var corrections diagnostics.Corrections + normalization := req.Stage == string(pipeline.StageNormalize) + validRangeRule := "Use positive source range endpoints that occur in the supplied transcript, with the earlier unit first." + guidancePrefix := "Correct every rejected NPC citation and return the complete replacement NPC registry" + if normalization { + validRangeRule = "Revise the duplicate-group proposals so every selected canonical NPC preserves valid transcript evidence; do not reproduce application IDs." + guidancePrefix = "Correct the duplicate-group proposals and return the complete replacement proposal response" + } for npcIndex, npc := range req.Value.NPCs { for refIndex, ref := range npc.SourceRefs { + record := fmt.Sprintf("Affected NPC %s, citing %s.", diagnostics.Quote(npc.Name), diagnostics.SourceRefRange(ref)) if err := index.ValidateRef(ref); err != nil { issues = append(issues, fmt.Sprintf("npcs[%d].source_refs[%d]: %s", npcIndex, refIndex, diagnostics.Truncate(err.Error()))) + corrections.Add("valid-range", validRangeRule, record) continue } - if coverage != nil && !coverage.contains(req.Source, ref) { + if req.Stage == string(pipeline.StageExtract) && !coverage.Contains(index, req.Source, ref) { issues = append(issues, fmt.Sprintf("npcs[%d].source_refs[%d]: source reference is outside the current extraction chunk", npcIndex, refIndex)) + corrections.Add("chunk-range", "Use only source ranges wholly contained in the supplied extraction chunk.", record) } } } if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } - return rejection(diagnostics.Aggregate("invalid NPC source references", issues)), nil -} - -type chunkCoverage struct { - sourceID string - unitIDs map[int]struct{} -} - -func newChunkCoverage(chunk *source.Chunk) *chunkCoverage { - coverage := &chunkCoverage{sourceID: chunk.SourceID, unitIDs: make(map[int]struct{}, len(chunk.Units))} - for _, unit := range chunk.Units { - coverage.unitIDs[unit.ID] = struct{}{} - } - return coverage -} - -func (coverage *chunkCoverage) contains(doc *source.SourceDocument, ref source.SourceRef) bool { - if coverage == nil || doc == nil || ref.SourceID != coverage.sourceID { - return false - } - start, startOK := source.UnitIndex(doc, ref.StartUnitID) - end, endOK := source.UnitIndex(doc, ref.EndUnitID) - if !startOK || !endOK || start > end { - return false - } - for position := start; position <= end; position++ { - if _, found := coverage.unitIDs[doc.Units[position].ID]; !found { - return false - } - } - return true + return rejection( + diagnostics.Aggregate("invalid NPC source references", issues), + corrections.Guidance(guidancePrefix), + ), nil } func Spec() pipeline.ValidatorSpec { @@ -116,6 +101,6 @@ 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, CorrectionGuidance: "Return registry NPCs whose source references identify valid transcript ranges within the supplied extraction chunk and directly support each proper NPC name."} +func rejection(message, guidance string) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: guidance} } diff --git a/internal/modules/dnd/validate/npcregistry/source_refs/validator_test.go b/internal/modules/dnd/validate/npcregistry/source_refs/validator_test.go index c474d314..c09f7cc1 100644 --- a/internal/modules/dnd/validate/npcregistry/source_refs/validator_test.go +++ b/internal/modules/dnd/validate/npcregistry/source_refs/validator_test.go @@ -70,6 +70,20 @@ func TestValidatorDefersMalformedShape(t *testing.T) { } } +func TestValidatorUsesProposalGuidanceDuringNormalization(t *testing.T) { + value := validNPCRegistry() + value.NPCs[0].SourceRefs = []source.SourceRef{{SourceID: "session", StartUnitID: 99, EndUnitID: 99}} + req := requestWithValue(validDocument(), value) + req.Stage = string(pipeline.StageNormalize) + result, err := New(Options{}).Validate(context.Background(), req) + if err != nil || result.Approved { + t.Fatalf("Validate() = %#v, %v; want rejection", result, err) + } + if !strings.Contains(result.CorrectionGuidance, "duplicate-group proposals") || !strings.Contains(result.CorrectionGuidance, "Mira Thorn") || strings.Contains(result.CorrectionGuidance, "npcs[") { + t.Fatalf("CorrectionGuidance = %q, want contextual proposal guidance", result.CorrectionGuidance) + } +} + func TestValidatorBoundsDiagnosticsAndHandlesMissingDocument(t *testing.T) { value := validNPCRegistry() value.NPCs[0].SourceRefs = make([]source.SourceRef, 24) @@ -86,7 +100,7 @@ func TestValidatorBoundsDiagnosticsAndHandlesMissingDocument(t *testing.T) { } func TestValidatorSpecCheckpointAndRegistration(t *testing.T) { - if got := New(Options{}).CheckpointFingerprints(); len(got) != 1 || got[0].Name != "policy" || got[0].Value != "dnd.npc_registry.validator.source_refs.v2" { + if got := New(Options{}).CheckpointFingerprints(); len(got) != 1 || got[0].Name != "policy" || got[0].Value != policy { t.Fatalf("CheckpointFingerprints() = %#v, want local policy", got) } if Spec().ExecutionClass != contracts.ExecutionClassDeterministic { diff --git a/internal/modules/dnd/validate/scenedescriptions/shape/validator.go b/internal/modules/dnd/validate/scenedescriptions/shape/validator.go index 32502dca..0e1b02ec 100644 --- a/internal/modules/dnd/validate/scenedescriptions/shape/validator.go +++ b/internal/modules/dnd/validate/scenedescriptions/shape/validator.go @@ -15,7 +15,7 @@ import ( const ( Key = "extract/dnd/scene-descriptions/shape" ReasonCode = "invalid_scene_description_shape" - policy = "dnd.scene_descriptions.validator.shape.v1" + policy = "dnd.scene_descriptions.validator.shape.v2" ) type Options struct{} @@ -34,8 +34,14 @@ 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(), 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 + issues, corrections := assess(req.Value, req.Stage == string(pipeline.StageExtract)) + if len(issues) != 0 { + return contracts.ValidationResult{ + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("invalid scene description shape", issues), + CorrectionGuidance: corrections.Guidance("Correct the rejected scene description and return the complete replacement response"), + }, nil } return contracts.ValidationResult{Approved: true}, nil } @@ -47,38 +53,53 @@ func ValidateForStage(value dnd.SceneDescriptionList, stage string) error { } func validate(value dnd.SceneDescriptionList, exactlyOne bool) error { - issues := make([]string, 0) - if value.Scenes == nil { - issues = append(issues, "scenes must be present") - } else if len(value.Scenes) == 0 { - issues = append(issues, "scenes must not be empty") - } else if exactlyOne && len(value.Scenes) != 1 { - issues = append(issues, "extraction must contain exactly one scene") - } - for index, scene := range value.Scenes { - prefix := fmt.Sprintf("scenes[%d]", index) - if strings.TrimSpace(scene.ID) == "" || scene.ID != strings.TrimSpace(scene.ID) { - issues = append(issues, prefix+".id must be non-empty and trimmed") - } - if !ValidKind(scene.Kind) { - issues = append(issues, prefix+".kind is unsupported: "+diagnostics.Quote(string(scene.Kind))) - } - if strings.TrimSpace(scene.Title) == "" || scene.Title != strings.TrimSpace(scene.Title) { - issues = append(issues, prefix+".title must be non-empty and trimmed") - } - if strings.TrimSpace(scene.Summary) == "" || scene.Summary != strings.TrimSpace(scene.Summary) { - issues = append(issues, prefix+".summary must be non-empty and trimmed") - } - if strings.TrimSpace(scene.SourceRef.SourceID) == "" || scene.SourceRef.SourceID != strings.TrimSpace(scene.SourceRef.SourceID) || scene.SourceRef.StartUnitID <= 0 || scene.SourceRef.EndUnitID <= 0 { - issues = append(issues, prefix+".source_ref must have a trimmed source ID and positive unit IDs") - } - } + issues, _ := assess(value, exactlyOne) if len(issues) == 0 { return nil } return fmt.Errorf("%s", diagnostics.Aggregate("invalid scene description shape", issues)) } +func assess(value dnd.SceneDescriptionList, exactlyOne bool) ([]string, diagnostics.Corrections) { + issues := make([]string, 0) + var corrections diagnostics.Corrections + if value.Scenes == nil { + issues = append(issues, "scenes must be present") + corrections.Add("scene", "Return one scene description for the supplied extraction chunk.", "") + } else if len(value.Scenes) == 0 { + issues = append(issues, "scenes must not be empty") + corrections.Add("scene", "Return one scene description for the supplied extraction chunk.", "") + } else if exactlyOne && len(value.Scenes) != 1 { + issues = append(issues, "extraction must contain exactly one scene") + corrections.Add("scene", "Return exactly one scene description for the supplied extraction chunk.", "") + } + for index, scene := range value.Scenes { + prefix := fmt.Sprintf("scenes[%d]", index) + record := fmt.Sprintf("Affected scene titled %s with kind %s, citing %s.", diagnostics.Quote(strings.TrimSpace(scene.Title)), diagnostics.Quote(string(scene.Kind)), diagnostics.SourceRefRange(scene.SourceRef)) + if strings.TrimSpace(scene.ID) == "" || scene.ID != strings.TrimSpace(scene.ID) { + issues = append(issues, prefix+".id must be non-empty and trimmed") + corrections.Add("scene", "Return exactly one scene description for the supplied extraction chunk.", record) + } + if !ValidKind(scene.Kind) { + issues = append(issues, prefix+".kind is unsupported: "+diagnostics.Quote(string(scene.Kind))) + corrections.Add("kind", "Set `kind` to exactly one of `combat`, `narrative`, `recap`, or `meta`.", record) + } + if strings.TrimSpace(scene.Title) == "" || scene.Title != strings.TrimSpace(scene.Title) { + issues = append(issues, prefix+".title must be non-empty and trimmed") + corrections.Add("title", "Provide a concise, nonblank, trimmed title grounded in the supplied scene transcript.", record) + } + if strings.TrimSpace(scene.Summary) == "" || scene.Summary != strings.TrimSpace(scene.Summary) { + issues = append(issues, prefix+".summary must be non-empty and trimmed") + corrections.Add("summary", "Provide a concise, nonblank, trimmed summary grounded in the supplied scene transcript.", record) + } + if strings.TrimSpace(scene.SourceRef.SourceID) == "" || scene.SourceRef.SourceID != strings.TrimSpace(scene.SourceRef.SourceID) || scene.SourceRef.StartUnitID <= 0 || scene.SourceRef.EndUnitID <= 0 { + issues = append(issues, prefix+".source_ref must have a trimmed source ID and positive unit IDs") + corrections.Add("scene", "Return exactly one scene description for the supplied extraction chunk.", record) + } + } + return issues, corrections +} + func ValidKind(value dnd.SceneKind) bool { switch value { case dnd.SceneKindCombat, dnd.SceneKindNarrative, dnd.SceneKindRecap, dnd.SceneKindMeta: diff --git a/internal/modules/dnd/validate/spells/catalog/validator.go b/internal/modules/dnd/validate/spells/catalog/validator.go index 6431528f..8a7c88e0 100644 --- a/internal/modules/dnd/validate/spells/catalog/validator.go +++ b/internal/modules/dnd/validate/spells/catalog/validator.go @@ -5,19 +5,19 @@ import ( "fmt" "strings" + "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" spellcatalog "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/spells/catalog" spellshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/spells/shape" ) const ( - Key = "extract/dnd/spells/catalog" - ReasonCode = "unknown_spell" - maxIssues = 20 - maxDisplayedNameRunes = 128 - maxMessageBytes = 4096 + Key = "extract/dnd/spells/catalog" + ReasonCode = "unknown_spell" + policy = "dnd.spells.validator.catalog.v2" ) type Options struct{} @@ -54,7 +54,10 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { if v == nil { return nil } - return []pipeline.CheckpointFingerprint{{Name: "effective_catalog", Value: v.catalog.Digest()}} + return []pipeline.CheckpointFingerprint{ + {Name: "policy", Value: policy}, + {Name: "effective_catalog", Value: v.catalog.Digest()}, + } } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.SpellList]) (contracts.ValidationResult, error) { @@ -69,7 +72,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq continue } if _, ok := v.catalog.Lookup(name); !ok { - unknown = append(unknown, unknownSpell{index: index, name: name}) + unknown = append(unknown, unknownSpell{index: index, name: name, caster: spell.Caster, sourceRefs: spell.SourceRefs}) } } if len(unknown) == 0 { @@ -79,42 +82,29 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq } type unknownSpell struct { - index int - name string + index int + name string + caster string + sourceRefs []source.SourceRef } func rejection(unknown []unknownSpell) contracts.ValidationResult { - limit := len(unknown) - if limit > maxIssues { - limit = maxIssues + issues := make([]string, 0, len(unknown)) + var corrections diagnostics.Corrections + for _, item := range unknown { + issues = append(issues, fmt.Sprintf("spell_casts[%d].spell %s", item.index, diagnostics.Quote(item.name))) + corrections.Add( + "recognized-spell", + "Use a recognized D&D spell name supported by the transcript, or omit the record when the evidence does not establish a spell cast.", + fmt.Sprintf("Affected spell %s by caster %s %s.", diagnostics.Quote(item.name), diagnostics.Quote(strings.TrimSpace(item.caster)), diagnostics.SourceRange(item.sourceRefs)), + ) } - issues := make([]string, 0, limit) - for _, item := range unknown[:limit] { - issue := fmt.Sprintf("spell_casts[%d].spell %q", item.index, truncateDisplayedName(item.name)) - candidate := rejectionMessage(append(issues, issue), len(unknown)-len(issues)-1) - if len(candidate) > maxMessageBytes { - break - } - issues = append(issues, issue) + return contracts.ValidationResult{ + Approved: false, + ReasonCode: ReasonCode, + Message: diagnostics.Aggregate("unknown spell names", issues), + CorrectionGuidance: corrections.Guidance("Correct every unrecognized spell and return the complete replacement spell-cast list"), } - message := rejectionMessage(issues, len(unknown)-len(issues)) - 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 { - runes := []rune(name) - if len(runes) <= maxDisplayedNameRunes { - return name - } - return string(runes[:maxDisplayedNameRunes-1]) + "…" -} - -func rejectionMessage(issues []string, omitted int) string { - message := "unknown spell names: " + strings.Join(issues, ", ") - if omitted > 0 { - message += fmt.Sprintf("; %d additional issue(s) omitted", omitted) - } - return message } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/spells/catalog/validator_test.go b/internal/modules/dnd/validate/spells/catalog/validator_test.go index 26ac397b..3ddf9b0e 100644 --- a/internal/modules/dnd/validate/spells/catalog/validator_test.go +++ b/internal/modules/dnd/validate/spells/catalog/validator_test.go @@ -2,11 +2,9 @@ package catalog import ( "context" - "fmt" "reflect" "strings" "testing" - "unicode/utf8" "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" @@ -36,7 +34,7 @@ func TestValidatorCheckpointFingerprintUsesEffectiveCatalogDigest(t *testing.T) t.Fatal(err) } fingerprints := validator.CheckpointFingerprints() - if len(fingerprints) != 1 || fingerprints[0].Name != "effective_catalog" || fingerprints[0].Value != validator.catalog.Digest() { + if len(fingerprints) != 2 || fingerprints[0].Name != "policy" || fingerprints[0].Value != policy || fingerprints[1].Name != "effective_catalog" || fingerprints[1].Value != validator.catalog.Digest() { t.Fatalf("checkpoint fingerprints = %#v, want effective catalog digest %q", fingerprints, validator.catalog.Digest()) } } @@ -60,59 +58,13 @@ func TestValidatorRejectsMultipleUnknownCastsInStableOrder(t *testing.T) { 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) } -} - -func TestValidatorBoundsUnknownCastMessage(t *testing.T) { - value := dnd.SpellList{SpellCasts: make([]dnd.SpellCast, 22)} - for index := range value.SpellCasts { - value.SpellCasts[index] = validCast(fmt.Sprintf("Unknown Spell %02d", index)) + for _, want := range []string{"Unknown First", "Unknown Second", "source unit 1", "complete replacement"} { + if !strings.Contains(result.CorrectionGuidance, want) { + t.Fatalf("CorrectionGuidance = %q, want %q", result.CorrectionGuidance, want) + } } - validator, err := New(Options{}) - if err != nil { - t.Fatalf("New() error = %v, want nil", err) - } - result, err := validator.Validate(context.Background(), validationRequest(value)) - if err != nil { - t.Fatalf("Validate() error = %v, want nil", err) - } - if result.Approved || result.ReasonCode != ReasonCode { - t.Fatalf("Validate() = %#v, want unknown-spell rejection", result) - } - if got := strings.Count(result.Message, "spell_casts["); got != maxIssues { - t.Fatalf("message includes %d issues, want %d: %q", got, maxIssues, result.Message) - } - if !strings.Contains(result.Message, "2 additional issue(s) omitted") || strings.Contains(result.Message, "Unknown Spell 21") { - t.Fatalf("message = %q, want bounded diagnostics", result.Message) - } -} - -func TestValidatorBoundsUnknownSpellNamesAndTotalMessage(t *testing.T) { - longName := strings.Repeat("火", maxDisplayedNameRunes+100) + "\n\t" - value := dnd.SpellList{SpellCasts: make([]dnd.SpellCast, maxIssues)} - for index := range value.SpellCasts { - value.SpellCasts[index] = validCast(fmt.Sprintf("%s-%d", longName, index)) - } - validator, err := New(Options{}) - if err != nil { - t.Fatal(err) - } - result, err := validator.Validate(context.Background(), validationRequest(value)) - if err != nil { - t.Fatal(err) - } - if result.Approved || len([]byte(result.Message)) > maxMessageBytes || !strings.Contains(result.Message, "…") { - t.Fatalf("message length/content = %d/%q, want bounded message with truncation", len([]byte(result.Message)), result.Message) - } - if !strings.Contains(result.Message, "additional issue(s) omitted") { - t.Fatalf("message = %q, want byte-budget omitted count", result.Message) - } - displayed := strings.Count(result.Message, "spell_casts[") - wantOmitted := fmt.Sprintf("%d additional issue(s) omitted", len(value.SpellCasts)-displayed) - if !strings.Contains(result.Message, wantOmitted) { - t.Fatalf("message = %q, want omitted count %q", result.Message, wantOmitted) - } - if !utf8.ValidString(result.Message) { - t.Fatal("bounded message is not valid UTF-8") + if strings.Contains(result.CorrectionGuidance, "spell_casts[") || strings.Contains(result.CorrectionGuidance, ReasonCode) { + t.Fatalf("CorrectionGuidance exposed internal diagnostics: %q", result.CorrectionGuidance) } } diff --git a/internal/modules/dnd/validate/spells/shape/validator.go b/internal/modules/dnd/validate/spells/shape/validator.go index a445a5a1..b0114f8a 100644 --- a/internal/modules/dnd/validate/spells/shape/validator.go +++ b/internal/modules/dnd/validate/spells/shape/validator.go @@ -8,12 +8,13 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" ) const ( Key = "extract/dnd/spells/shape" ReasonCode = "invalid_spell_shape" - policy = "dnd.spells.validator.shape.v1" + policy = "dnd.spells.validator.shape.v2" ) type Options struct{} @@ -31,28 +32,69 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { return []pipeline.CheckpointFingerprint{{Name: "policy", Value: policy}} } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.SpellList]) (contracts.ValidationResult, error) { - if err := Validate(req.Value); err != nil { - return rejection(err.Error()), nil + assessment := assess(req.Value) + if len(assessment.issues) != 0 { + return rejection(assessment.message(), assessment.corrections.Guidance("Correct every rejected spell cast and return the complete replacement list")), nil } return contracts.ValidationResult{Approved: true}, nil } func Validate(value dnd.SpellList) error { + assessment := assess(value) + if len(assessment.issues) == 0 { + return nil + } + return fmt.Errorf("%s", assessment.message()) +} + +type validationAssessment struct { + issues []string + corrections diagnostics.Corrections +} + +func assess(value dnd.SpellList) validationAssessment { + var assessment validationAssessment if value.SpellCasts == nil { - return fmt.Errorf("spell_casts must be present") + assessment.issues = append(assessment.issues, "spell_casts must be present") + assessment.corrections.Add("list", "Return a `spell_casts` array; use an empty array when the transcript establishes no spell casts.", "") + return assessment } for index, spell := range value.SpellCasts { + context := spellContext(spell) if strings.TrimSpace(spell.Caster) == "" { - return fmt.Errorf("spell_casts[%d].caster must not be empty", index) + assessment.issues = append(assessment.issues, fmt.Sprintf("spell_casts[%d].caster must not be empty", index)) + assessment.corrections.Add("caster", "Provide the contextual caster name for every spell cast.", context) } if strings.TrimSpace(spell.Spell) == "" { - return fmt.Errorf("spell_casts[%d].spell must not be empty", index) + assessment.issues = append(assessment.issues, fmt.Sprintf("spell_casts[%d].spell must not be empty", index)) + assessment.corrections.Add("spell", "Provide the transcript-supported spell name for every spell cast.", context) } if len(spell.SourceRefs) == 0 { - return fmt.Errorf("spell_casts[%d].source_refs must not be empty", index) + assessment.issues = append(assessment.issues, fmt.Sprintf("spell_casts[%d].source_refs must not be empty", index)) + assessment.corrections.Add("source-refs", "Provide at least one transcript source range that directly supports every spell cast.", context) } } - return nil + return assessment +} + +func (assessment validationAssessment) message() string { + return diagnostics.Aggregate("invalid spell shape", assessment.issues) +} + +func spellContext(spell dnd.SpellCast) string { + name := strings.TrimSpace(spell.Spell) + if name == "" { + name = "blank spell name" + } else { + name = "spell " + diagnostics.Quote(name) + } + caster := strings.TrimSpace(spell.Caster) + if caster == "" { + caster = "blank caster name" + } else { + caster = "caster " + diagnostics.Quote(caster) + } + return "Affected cast: " + name + " with " + caster + " " + diagnostics.SourceRange(spell.SourceRefs) + "." } func Spec() pipeline.ValidatorSpec { @@ -74,6 +116,6 @@ func DecodeOptions(options map[string]any) (Options, error) { return Options{}, nil } 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, CorrectionGuidance: "Return a complete spell-cast list with a nonblank spell name and caster plus valid source references for every cast."} +func rejection(message, guidance string) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: guidance} } diff --git a/internal/modules/dnd/validate/spells/shape/validator_test.go b/internal/modules/dnd/validate/spells/shape/validator_test.go index 13191080..86716228 100644 --- a/internal/modules/dnd/validate/spells/shape/validator_test.go +++ b/internal/modules/dnd/validate/spells/shape/validator_test.go @@ -2,6 +2,7 @@ package shape import ( "context" + "strings" "testing" "gitea.maximumdirect.net/eric/notarius/internal/core/source" @@ -54,6 +55,30 @@ func TestValidatorRejectsMissingRequiredSpellFields(t *testing.T) { } } +func TestValidatorReportsAllSpellDefectsWithContextualGuidance(t *testing.T) { + value := dnd.SpellList{SpellCasts: []dnd.SpellCast{ + {Caster: "", Spell: "Fire Bolt", SourceRefs: refsAt(4)}, + {Caster: "Aria", Spell: "", SourceRefs: nil}, + }} + result, err := New(Options{}).Validate(context.Background(), requestWithValue(value)) + if err != nil || result.Approved { + t.Fatalf("Validate() = %#v, %v; want rejection", result, err) + } + for _, want := range []string{"spell_casts[0].caster", "spell_casts[1].spell", "spell_casts[1].source_refs"} { + if !strings.Contains(result.Message, want) { + t.Fatalf("Message = %q, want %q", result.Message, want) + } + } + for _, want := range []string{"Fire Bolt", "source unit 4", "Aria", "complete replacement"} { + if !strings.Contains(result.CorrectionGuidance, want) { + t.Fatalf("CorrectionGuidance = %q, want %q", result.CorrectionGuidance, want) + } + } + if strings.Contains(result.CorrectionGuidance, "spell_casts[") { + t.Fatalf("CorrectionGuidance exposed operator path: %q", result.CorrectionGuidance) + } +} + func TestValidatorSpecCheckpointAndRegistration(t *testing.T) { if got := New(Options{}).CheckpointFingerprints(); len(got) != 1 || got[0].Name != "policy" || got[0].Value != policy { t.Fatalf("CheckpointFingerprints() = %#v, want local policy", got) @@ -77,3 +102,7 @@ func requestWithValue(value dnd.SpellList) contracts.TypedValidationRequest[dnd. func validSpellList() dnd.SpellList { return dnd.SpellList{SpellCasts: []dnd.SpellCast{{Caster: "Aria", Spell: "Cure Wounds", SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}}}} } + +func refsAt(unitID int) []source.SourceRef { + return []source.SourceRef{{SourceID: "session", StartUnitID: unitID, EndUnitID: unitID}} +} diff --git a/internal/modules/dnd/validate/spells/source_refs/validator.go b/internal/modules/dnd/validate/spells/source_refs/validator.go index f1f59a41..70822ab7 100644 --- a/internal/modules/dnd/validate/spells/source_refs/validator.go +++ b/internal/modules/dnd/validate/spells/source_refs/validator.go @@ -8,6 +8,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" spellshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/spells/shape" ) @@ -15,7 +16,7 @@ import ( const ( Key = "extract/dnd/spells/source_refs" ReasonCode = "invalid_source_refs" - policy = "dnd.spells.validator.source_refs.v2" + policy = "dnd.spells.validator.source_refs.v3" ) type Options struct{} @@ -40,57 +41,35 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq return contracts.ValidationResult{Approved: true}, nil } index := source.NewDocumentIndex(req.Source) - var coverage *chunkCoverage + var coverage shared.ChunkCoverage if req.Stage == string(pipeline.StageExtract) { - coverage = newChunkCoverage(req.Chunk) + coverage = shared.NewChunkCoverage(req.Chunk) } issues := make([]string, 0) + var corrections diagnostics.Corrections for spellIndex, spell := range req.Value.SpellCasts { for refIndex, ref := range spell.SourceRefs { + record := fmt.Sprintf("Affected spell %s by caster %s, citing %s.", diagnostics.Quote(spell.Spell), diagnostics.Quote(spell.Caster), diagnostics.SourceRefRange(ref)) if err := index.ValidateRef(ref); err != nil { issues = append(issues, fmt.Sprintf("spell_casts[%d].source_refs[%d]: %s", spellIndex, refIndex, diagnostics.Truncate(err.Error()))) + corrections.Add("valid-range", "Use positive source range endpoints that occur in the supplied transcript, with the earlier unit first.", record) continue } - if coverage != nil && !coverage.contains(req.Source, ref) { + if req.Stage == string(pipeline.StageExtract) && !coverage.Contains(index, req.Source, ref) { issues = append(issues, fmt.Sprintf("spell_casts[%d].source_refs[%d]: source reference is outside the current extraction chunk", spellIndex, refIndex)) + corrections.Add("chunk-range", "Use only source ranges wholly contained in the supplied extraction chunk.", record) } } } if len(issues) > 0 { - return rejection(diagnostics.Aggregate("invalid spell source references", issues)), nil + return rejection( + diagnostics.Aggregate("invalid spell source references", issues), + corrections.Guidance("Correct every rejected spell citation and return the complete replacement spell-cast list"), + ), nil } return contracts.ValidationResult{Approved: true}, nil } -type chunkCoverage struct { - sourceID string - unitIDs map[int]struct{} -} - -func newChunkCoverage(chunk *source.Chunk) *chunkCoverage { - coverage := &chunkCoverage{sourceID: chunk.SourceID, unitIDs: make(map[int]struct{}, len(chunk.Units))} - for _, unit := range chunk.Units { - coverage.unitIDs[unit.ID] = struct{}{} - } - return coverage -} - -func (coverage *chunkCoverage) contains(doc *source.SourceDocument, ref source.SourceRef) bool { - if coverage == nil || doc == nil || ref.SourceID != coverage.sourceID { - return false - } - start, startOK := source.UnitIndex(doc, ref.StartUnitID) - end, endOK := source.UnitIndex(doc, ref.EndUnitID) - if !startOK || !endOK || start > end { - return false - } - for position := start; position <= end; position++ { - if _, found := coverage.unitIDs[doc.Units[position].ID]; !found { - return false - } - } - return true -} func Spec() pipeline.ValidatorSpec { return pipeline.ValidatorSpec{Key: Key, ExecutionClass: contracts.ExecutionClassDeterministic} } @@ -110,6 +89,6 @@ func DecodeOptions(options map[string]any) (Options, error) { return Options{}, nil } 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, CorrectionGuidance: "Return spell casts whose source references identify valid transcript ranges within the supplied extraction chunk and directly support the named spell and caster."} +func rejection(message, guidance string) contracts.ValidationResult { + return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: message, CorrectionGuidance: guidance} } diff --git a/internal/modules/dnd/validate/spells/source_refs/validator_test.go b/internal/modules/dnd/validate/spells/source_refs/validator_test.go index 13f0fc7e..fca58bc8 100644 --- a/internal/modules/dnd/validate/spells/source_refs/validator_test.go +++ b/internal/modules/dnd/validate/spells/source_refs/validator_test.go @@ -33,6 +33,14 @@ func TestValidatorRejectsInvalidSourceRefs(t *testing.T) { if result.ReasonCode != ReasonCode { t.Fatalf("ReasonCode = %q, want %q", result.ReasonCode, ReasonCode) } + for _, want := range []string{"Cure Wounds", "Aria", "source unit 99", "complete replacement"} { + if !strings.Contains(result.CorrectionGuidance, want) { + t.Fatalf("CorrectionGuidance = %q, want %q", result.CorrectionGuidance, want) + } + } + if strings.Contains(result.CorrectionGuidance, "spell_casts[") || strings.Contains(result.CorrectionGuidance, ReasonCode) { + t.Fatalf("CorrectionGuidance exposed internal diagnostics: %q", result.CorrectionGuidance) + } } func TestValidatorRejectsMissingSourceDocument(t *testing.T) {