diff --git a/docs/roadmap/dnd-npc-interactions.md b/docs/roadmap/dnd-npc-interactions.md deleted file mode 100644 index 34adc4a..0000000 --- a/docs/roadmap/dnd-npc-interactions.md +++ /dev/null @@ -1,200 +0,0 @@ -# D&D NPC Interactions - -Status: Implemented - -The current durable and configuration contract is documented in the -[D&D NPC interaction artifact](../integrations/dnd-npc-interaction-artifacts.md). - -## Purpose - -The normalized NPC registry intentionally answers only who was identified in a -session. It does not answer whether an NPC was merely mentioned, participated -in dialogue, or fought alongside or against the party. Add a separate, -ordered NPC-interaction artifact for that occurrence-level information rather -than expanding the identity registry. - -This feature preserves the minimal-extractor policy: the model identifies one -bounded kind of interaction and its supporting transcript evidence. It does -not summarize the interaction, infer relationships, or maintain NPC state. - -## Desired End State - -A D&D pipeline can run NPC extraction first and supply its accepted normalized -registry to a later `dnd/npc-interactions` lane. The later lane emits an ordered -list of evidenced interaction occurrences involving registry NPCs. - -The production identities should be: - -- extractor and normalizer key: `dnd/npc-interactions`; -- artifact kind: `dnd/npc-interaction-list`; -- durable schema ID: `notarius.dnd.npc_interactions`; -- durable schema name: `notarius_dnd_npc_interactions_v1`; -- durable schema version: `v1`; and -- media type: `application/json`. - -The lane must use the existing D&D module organization, shared prompt assets, -typed artifact pipeline, codec boundary, registration pattern, and default -validator composition. - -## Artifact Contract - -The durable payload is an object containing an `interactions` array. The array -may be empty. Each interaction contains exactly: - -- `name`: the canonical NPC name from the supplied registry; -- `kind`: one value from the bounded interaction vocabulary; and -- `source_refs`: one or more current-transcript ranges supporting both the NPC - identity and the classified interaction. - -Every object rejects unknown fields. The model-facing response should contain -only the corresponding name, kind, and source-unit range candidates. Notarius -attaches the current source identity deterministically; the model must not -reproduce it. - -Do not add a separate interaction ID in the durable contract. Stable ordering, -the canonical NPC name, the bounded kind, and exact evidence ranges are enough -to identify and audit an occurrence for the present use cases. Revisit durable -cross-artifact identity only with a concrete consumer requirement. - -## Interaction Vocabulary - -Use this closed vocabulary: - -| Kind | Meaning | -| --- | --- | -| `mentioned` | The NPC is referred to, but is not established as present or communicating in the evidenced passage. | -| `noncombat_presence` | The NPC is present and relevant to the passage but does not meaningfully participate in dialogue or combat. | -| `dialogue` | The NPC speaks, responds, or is directly engaged in a meaningful non-combat exchange. | -| `combat_ally` | The NPC actively participates in combat on the party's side. | -| `combat_opponent` | The NPC actively participates in combat against the party. | -| `other` | The transcript clearly establishes a direct NPC occurrence that fits none of the preceding kinds. | - -`other` is a residual category for positively evidenced activity, not an -escape hatch for uncertain classification. Omit a candidate when the -transcript does not support one category. - -When activities overlap within one occurrence, apply this precedence: - -1. active combat participation outranks dialogue, presence, and mention; -2. dialogue outranks non-combat presence and mention; -3. non-combat presence outranks mention; and -4. `other` applies only when none of the defined categories describes the - evidenced activity. - -Combat alignment is not resolved by precedence. An NPC cannot be both a combat -ally and combat opponent in one occurrence; split the record when its alignment -meaningfully changes. - -## Occurrence Boundaries And Ordering - -An occurrence represents one NPC, one interaction kind, and one locally -coherent transcript passage. Combine repeated evidence only while it supports -the same uninterrupted activity. Create separate occurrences when: - -- the interaction kind changes; -- combat alignment changes; -- a scene or meaningful absence separates repeated activity; or -- the NPC is first mentioned and later becomes present. - -Extraction remains chunk-scoped, so an occurrence must not span accepted chunk -boundaries. Merge and normalization must not semantically combine occurrences -from different chunks. - -Order the durable list by the earliest valid source position. Use canonical -name, interaction kind, and canonical source ranges as deterministic -tie-breakers. Remove only exact duplicate records; do not use model judgment to -collapse nearby occurrences. - -## NPC Grounding And Evidence - -The extractor declares the existing `npcs` reference slot and requires a bound, -accepted NPC registry. The normal same-run configuration binds the normalized -output of an earlier NPC lane through an ordered generated reference. Existing -framework support for a compatible external registry may remain available, but -must not weaken the artifact contract. - -Present the registry's names-only projection to the model. An emitted name must -match one canonical registry name exactly after the registry's established -lookup rules are applied, and the durable artifact retains that canonical -display name. Do not copy NPC IDs or registry source references into an -interaction. - -The registry establishes available identity, not occurrence. Every interaction -must cite current transcript ranges that independently support both the NPC and -the selected kind. Campaign references and generated artifacts may disambiguate -a name, but they never become interaction evidence. - -If no registry NPC has an evidenced interaction, the correct artifact is an -empty list. A missing, rejected, or incompatible required registry handoff is a -pipeline dependency failure rather than a request to extract ungrounded names. - -## Prompt And Model Boundary - -Follow the established D&D prompt ordering and cache-boundary policy. Stable -shared instructions, lane instructions, campaign references, and the NPC -names-only projection precede the variable transcript. Factor wording shared -with the spell and combat lanes into the existing shared asset pattern rather -than creating nearly identical package-local messages. - -The prompt must: - -- define the closed vocabulary and precedence rules; -- ask for occurrences involving only supplied registry NPCs; -- distinguish mention from presence and dialogue; -- require current-transcript evidence for identity and kind; -- forbid summaries, relationship inference, sentiment, aliases, and invented - names; and -- permit an empty result. - -Keep the private JSON Schema structural. Deterministic code owns canonical-name -resolution, enum enforcement at the durable boundary, source-range validation, -ordering, exact deduplication, and evidence invariants. - -## Validation And Quality - -Provide production validators and default chains at extraction and -normalization boundaries consistent with the existing D&D artifacts. The -append-only merge does not require a separate default validator chain. -Deterministic validation must reject: - -- missing or extra fields; -- empty or unrecognized NPC names; -- names absent from the supplied registry; -- unknown interaction kinds; -- empty, malformed, out-of-source, or reversed evidence ranges; and -- records whose evidence comes from a reference rather than the current - transcript. - -Any relatedness validator should remain warning-only unless evaluation -demonstrates a reliable deterministic rejection rule. Diagnostics must be -bounded and must not leak reference contents. - -Evaluate the lane on a small human-reviewed transcript set that includes every -category, transitions between categories, multiple occurrences for one NPC, -mentions followed by appearances, alignment changes, repeated evidence across -chunks, and empty output. Review category agreement, evidence sufficiency, -ordering, duplicate behavior, and reliability on the smaller models the -application is intended to support. Treat model-output evaluation as a human -development aid, not a brittle deterministic test oracle. - -## Documentation Outcomes - -When implemented, document the durable artifact in `docs/integrations/`, add -the selectable module and validator contracts to configuration documentation, -update the current module and pipeline internals, and provide a maintained -ordered-pipeline example showing NPC extraction followed by interaction -extraction. Future behavior must remain in this roadmap until it exists. - -## Non-Goals - -This scope does not: - -- add occurrence fields to the normalized NPC registry; -- summarize dialogue, combat, or NPC behavior; -- infer disposition, relationships, factions, motives, or persistent state; -- identify player characters or anonymous groups as NPCs; -- add scene participants or duplicate scene-description responsibilities; -- reconcile NPC aliases or perform LLM-assisted semantic deduplication; -- derive interaction records from registry evidence; or -- introduce a DAG, concurrent cross-lane reconciliation, or a new reference - mechanism. diff --git a/internal/modules/dnd/normalize/combatturns/normalizer.go b/internal/modules/dnd/normalize/combatturns/normalizer.go index 3e31e59..5066f20 100644 --- a/internal/modules/dnd/normalize/combatturns/normalizer.go +++ b/internal/modules/dnd/normalize/combatturns/normalizer.go @@ -14,12 +14,13 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" npcregistry "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/registry" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" ) const ( Key = "dnd/combat-turns" - normalizationPolicy = "dnd.combat_turns.normalize.v1" + normalizationPolicy = "dnd.combat_turns.normalize.v2" NormalizationPolicy = normalizationPolicy ReasonCodeActorCanonicalized = "combat_actor_canonicalized" @@ -109,7 +110,8 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize if err != nil { return contracts.TypedNormalizeResult[dnd.CombatTurnList]{}, normalizerErrorf("resolve NPC registry: %w", err) } - value, warnings := normalizeList(req.MergeOutput.Value, req.Source, npcRegistry) + order := shared.NewSourceRefOrder(req.Source) + value, warnings := normalizeList(req.MergeOutput.Value, req.Source, order, npcRegistry) return contracts.TypedNormalizeResult[dnd.CombatTurnList]{Value: value, Warnings: warnings}, nil } @@ -125,7 +127,7 @@ type actorCanonicalization struct { to string } -func normalizeList(input dnd.CombatTurnList, doc *source.SourceDocument, registry *npcregistry.Registry) (dnd.CombatTurnList, []contracts.Warning) { +func normalizeList(input dnd.CombatTurnList, doc *source.SourceDocument, order shared.SourceRefOrder, registry *npcregistry.Registry) (dnd.CombatTurnList, []contracts.Warning) { if input.CombatTurns == nil { return dnd.CombatTurnList{}, nil } @@ -133,8 +135,8 @@ func normalizeList(input dnd.CombatTurnList, doc *source.SourceDocument, registr records := make([]normalizedRecord, len(input.CombatTurns)) warnings := make([]contracts.Warning, 0) for index, inputTurn := range input.CombatTurns { - turn, actorChange, refsChanged := normalizeTurn(inputTurn, registry) - earliest, hasEvidence := earliestSourcePosition(doc, turn) + turn, actorChange, refsChanged := normalizeTurn(inputTurn, order, registry) + earliest, hasEvidence := order.EarliestValid(turn.SourceRefs) records[index] = normalizedRecord{ turn: turn, inputIndex: index, @@ -185,7 +187,7 @@ func normalizeList(input dnd.CombatTurnList, doc *source.SourceDocument, registr return dnd.CombatTurnList{CombatTurns: output}, warnings } -func normalizeTurn(input dnd.CombatTurn, registry *npcregistry.Registry) (dnd.CombatTurn, *actorCanonicalization, bool) { +func normalizeTurn(input dnd.CombatTurn, order shared.SourceRefOrder, registry *npcregistry.Registry) (dnd.CombatTurn, *actorCanonicalization, bool) { output := cloneCombatTurn(input) output.Actor = identity.NormalizeDisplay(input.Actor) @@ -198,7 +200,7 @@ func normalizeTurn(input dnd.CombatTurn, registry *npcregistry.Registry) (dnd.Co actorChange = &actorCanonicalization{from: input.Actor, to: output.Actor} } - canonicalRefs, _, _ := canonicalizeSourceRefs(input.SourceRefs) + canonicalRefs, _, _ := canonicalizeSourceRefs(order, input.SourceRefs) output.SourceRefs = canonicalRefs refsChanged := !sourceRefsEqual(input.SourceRefs, output.SourceRefs) return output, actorChange, refsChanged @@ -225,62 +227,9 @@ func sourceRefsEqual(left, right []source.SourceRef) bool { return true } -func canonicalizeSourceRefs(input []source.SourceRef) ([]source.SourceRef, bool, int) { - if input == nil { - return nil, false, 0 - } - - canonical := make([]source.SourceRef, len(input)) - copy(canonical, input) - sort.SliceStable(canonical, func(left, right int) bool { - return sourceRefLess(canonical[left], canonical[right]) - }) - - orderChanged := false - for index := range input { - if input[index] != canonical[index] { - orderChanged = true - break - } - } - - unique := make([]source.SourceRef, 0, len(canonical)) - for _, ref := range canonical { - if len(unique) == 0 || unique[len(unique)-1] != ref { - unique = append(unique, ref) - } - } - return unique, orderChanged, len(input) - len(unique) -} - -func sourceRefLess(left, right source.SourceRef) bool { - if left.SourceID != right.SourceID { - return left.SourceID < right.SourceID - } - if left.StartUnitID != right.StartUnitID { - return left.StartUnitID < right.StartUnitID - } - return left.EndUnitID < right.EndUnitID -} - -func earliestSourcePosition(doc *source.SourceDocument, turn dnd.CombatTurn) (int, bool) { - if doc == nil { - return 0, false - } - earliest := 0 - found := false - for _, ref := range turn.SourceRefs { - if source.ValidateRef(doc, ref) != nil { - continue - } - index, ok := source.UnitIndex(doc, ref.StartUnitID) - if !ok || (found && index >= earliest) { - continue - } - earliest = index - found = true - } - return earliest, found +func canonicalizeSourceRefs(order shared.SourceRefOrder, input []source.SourceRef) ([]source.SourceRef, bool, int) { + canonical := order.Canonicalize(input) + return canonical, !sourceRefsEqual(input, canonical), len(input) - len(canonical) } type duplicateGroup struct { diff --git a/internal/modules/dnd/normalize/combatturns/normalizer_test.go b/internal/modules/dnd/normalize/combatturns/normalizer_test.go index 0eeaae2..838f37a 100644 --- a/internal/modules/dnd/normalize/combatturns/normalizer_test.go +++ b/internal/modules/dnd/normalize/combatturns/normalizer_test.go @@ -122,6 +122,34 @@ func TestNormalizeOrdersBySourcePositionAndCollapsesExactDuplicates(t *testing.T } } +func TestNormalizeUsesDocumentOrderForReferencesAndChronology(t *testing.T) { + doc := &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 30}, {ID: 10}}} + input := dnd.CombatTurnList{CombatTurns: []dnd.CombatTurn{ + validTurn("Borin", source.SourceRef{SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}), + validTurn("Aria", source.SourceRef{SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}), + {Actor: "Goblin", TurnKind: dnd.CombatTurnKindTurn, SourceRefs: []source.SourceRef{ + {SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}, + {SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}, + {SourceID: doc.ID, StartUnitID: 999, EndUnitID: 999}, + }}, + }} + normalizer, err := New(Options{}) + if err != nil { + t.Fatalf("New() error = %v", err) + } + result, err := normalizer.Normalize(context.Background(), contracts.TypedNormalizeRequest[dnd.CombatTurnList]{Source: doc, MergeOutput: contracts.MergeArtifact[dnd.CombatTurnList]{Value: input}}) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + if got := result.Value.CombatTurns; got[0].Actor != "Aria" || got[1].Actor != "Goblin" || got[2].Actor != "Borin" { + t.Fatalf("turn order = %#v, want source chronology", got) + } + refs := result.Value.CombatTurns[1].SourceRefs + if refs[0].StartUnitID != 30 || refs[1].StartUnitID != 10 || refs[2].StartUnitID != 999 { + t.Fatalf("turn source refs = %#v, want source-document order and invalid fallback", refs) + } +} + func TestNormalizePreservesStableOrderForEqualEvidencePositions(t *testing.T) { doc := &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 50}}} input := dnd.CombatTurnList{CombatTurns: []dnd.CombatTurn{ diff --git a/internal/modules/dnd/normalize/npcinteractions/normalizer.go b/internal/modules/dnd/normalize/npcinteractions/normalizer.go index 26ad37e..4f5cbd8 100644 --- a/internal/modules/dnd/normalize/npcinteractions/normalizer.go +++ b/internal/modules/dnd/normalize/npcinteractions/normalizer.go @@ -118,7 +118,8 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize if !registry.Bound() { return contracts.TypedNormalizeResult[dnd.NPCInteractionList]{}, normalizerErrorf("NPC registry reference is required") } - value, warnings := normalizeList(req.MergeOutput.Value, req.Source, registry) + order := shared.NewSourceRefOrder(req.Source) + value, warnings := normalizeList(req.MergeOutput.Value, req.Source, order, registry) return contracts.TypedNormalizeResult[dnd.NPCInteractionList]{Value: value, Warnings: warnings}, nil } @@ -132,7 +133,7 @@ type nameCanonicalization struct { to string } -func normalizeList(input dnd.NPCInteractionList, doc *source.SourceDocument, registry *npcregistry.Registry) (dnd.NPCInteractionList, []contracts.Warning) { +func normalizeList(input dnd.NPCInteractionList, doc *source.SourceDocument, order shared.SourceRefOrder, registry *npcregistry.Registry) (dnd.NPCInteractionList, []contracts.Warning) { if input.Interactions == nil { return dnd.NPCInteractionList{}, nil } @@ -140,7 +141,7 @@ func normalizeList(input dnd.NPCInteractionList, doc *source.SourceDocument, reg records := make([]normalizedRecord, len(input.Interactions)) warnings := make([]contracts.Warning, 0) for index, inputInteraction := range input.Interactions { - interaction, nameChange, refsChanged := normalizeInteraction(inputInteraction, doc, registry) + interaction, nameChange, refsChanged := normalizeInteraction(inputInteraction, order, registry) records[index] = normalizedRecord{interaction: interaction, inputIndex: index} if nameChange != nil { warnings = append(warnings, contracts.Warning{ @@ -161,7 +162,7 @@ func normalizeList(input dnd.NPCInteractionList, doc *source.SourceDocument, reg } sort.SliceStable(records, func(left, right int) bool { - return interactionmodel.Less(doc, records[left].interaction, records[right].interaction) + return interactionmodel.Less(order, records[left].interaction, records[right].interaction) }) for position, record := range records { if position == record.inputIndex { @@ -180,7 +181,7 @@ func normalizeList(input dnd.NPCInteractionList, doc *source.SourceDocument, reg diagnostics.LimitWarnings(warnings, "npc_interactions", ReasonCodeWarningsOmitted) } -func normalizeInteraction(input dnd.NPCInteraction, doc *source.SourceDocument, registry *npcregistry.Registry) (dnd.NPCInteraction, *nameCanonicalization, bool) { +func normalizeInteraction(input dnd.NPCInteraction, order shared.SourceRefOrder, registry *npcregistry.Registry) (dnd.NPCInteraction, *nameCanonicalization, bool) { output := cloneInteraction(input) if canonical, ok := registry.Lookup(identity.NormalizeDisplay(input.Name)); ok { output.Name = canonical.Name @@ -189,7 +190,7 @@ func normalizeInteraction(input dnd.NPCInteraction, doc *source.SourceDocument, if input.Name != output.Name { nameChange = &nameCanonicalization{from: input.Name, to: output.Name} } - output.SourceRefs = interactionmodel.CanonicalizeSourceRefs(doc, input.SourceRefs) + output.SourceRefs = order.Canonicalize(input.SourceRefs) return output, nameChange, !interactionmodel.SourceRefsEqual(input.SourceRefs, output.SourceRefs) } diff --git a/internal/modules/dnd/normalize/npcinteractions/normalizer_test.go b/internal/modules/dnd/normalize/npcinteractions/normalizer_test.go index c6312eb..a25eed5 100644 --- a/internal/modules/dnd/normalize/npcinteractions/normalizer_test.go +++ b/internal/modules/dnd/normalize/npcinteractions/normalizer_test.go @@ -15,7 +15,7 @@ import ( ) func TestNormalizeCanonicalizesAndClones(t *testing.T) { - doc := testDocument() + doc := &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 30}, {ID: 10}}} normalizer, err := New(Options{}, npcReferences(t)) if err != nil { t.Fatalf("New() error = %v", err) @@ -30,7 +30,7 @@ func TestNormalizeCanonicalizesAndClones(t *testing.T) { t.Fatalf("Normalize() error = %v", err) } got := result.Value.Interactions[0] - if got.Name != "Ária" || !reflect.DeepEqual(got.SourceRefs, []source.SourceRef{{SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}, {SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}}) { + if got.Name != "Ária" || !reflect.DeepEqual(got.SourceRefs, []source.SourceRef{{SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}, {SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}}) { t.Fatalf("normalized interaction = %#v", got) } if !hasWarning(result.Warnings, ReasonCodeNameCanonicalized) || !hasWarning(result.Warnings, ReasonCodeSourceRefsNormalized) { diff --git a/internal/modules/dnd/normalize/npcs/normalizer.go b/internal/modules/dnd/normalize/npcs/normalizer.go index 74f53e2..3a2979b 100644 --- a/internal/modules/dnd/normalize/npcs/normalizer.go +++ b/internal/modules/dnd/normalize/npcs/normalizer.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "reflect" - "sort" "strconv" "strings" @@ -14,12 +13,13 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" ) const ( Key = "dnd/npcs" - normalizationPolicy = "dnd.npcs.normalize.v1" + normalizationPolicy = "dnd.npcs.normalize.v2" NormalizationPolicy = normalizationPolicy ReasonCodeNPCFieldsNormalized = "npc_fields_normalized" @@ -69,7 +69,8 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize if err := ctx.Err(); err != nil { return contracts.TypedNormalizeResult[dnd.NPCList]{}, normalizerErrorf("context error before normalize: %w", err) } - value, warnings := normalizeList(req.MergeOutput.Value) + order := shared.NewSourceRefOrder(req.Source) + value, warnings := normalizeList(req.MergeOutput.Value, order) return contracts.TypedNormalizeResult[dnd.NPCList]{Value: value, Warnings: warnings}, nil } @@ -77,14 +78,14 @@ type normalizedRecord struct { npc dnd.NPC } -func normalizeList(input dnd.NPCList) (dnd.NPCList, []contracts.Warning) { +func normalizeList(input dnd.NPCList, order shared.SourceRefOrder) (dnd.NPCList, []contracts.Warning) { if input.NPCs == nil { return dnd.NPCList{}, nil } records := make([]normalizedRecord, len(input.NPCs)) warnings := make([]contracts.Warning, 0) for index, inputNPC := range input.NPCs { - npc, fieldsChanged, referencesChanged := normalizeRecord(inputNPC) + npc, fieldsChanged, referencesChanged := normalizeRecord(inputNPC, order) records[index] = normalizedRecord{npc: npc} if fieldsChanged { warnings = append(warnings, contracts.Warning{Scope: npcScope(index), ReasonCode: ReasonCodeNPCFieldsNormalized, Message: fmt.Sprintf("input index %d: NPC name normalized for %s", index, diagnostics.Quote(inputNPC.Name))}) @@ -100,7 +101,7 @@ func normalizeList(input dnd.NPCList) (dnd.NPCList, []contracts.Warning) { groups := canonicalNameGroups(records) output := dnd.NPCList{NPCs: make([]dnd.NPC, 0, len(groups))} for _, members := range groups { - consolidated, referencesChanged := consolidate(records, members) + consolidated, referencesChanged := consolidate(records, members, order) retainedIndex := members[0] output.NPCs = append(output.NPCs, consolidated) if referencesChanged { @@ -113,10 +114,10 @@ func normalizeList(input dnd.NPCList) (dnd.NPCList, []contracts.Warning) { return output, warnings } -func normalizeRecord(input dnd.NPC) (dnd.NPC, bool, bool) { +func normalizeRecord(input dnd.NPC, order shared.SourceRefOrder) (dnd.NPC, bool, bool) { output := cloneNPC(input) output.Name = identity.NormalizeDisplay(input.Name) - output.SourceRefs, _, _ = canonicalizeSourceRefs(input.SourceRefs) + output.SourceRefs, _, _ = canonicalizeSourceRefs(order, input.SourceRefs) output.ID = identity.DeriveID(output.Name) return output, input.Name != output.Name, !reflect.DeepEqual(input.SourceRefs, output.SourceRefs) } @@ -143,39 +144,20 @@ func canonicalNameGroups(records []normalizedRecord) [][]int { return groups } -func consolidate(records []normalizedRecord, members []int) (dnd.NPC, bool) { +func consolidate(records []normalizedRecord, members []int, order shared.SourceRefOrder) (dnd.NPC, bool) { output := cloneNPC(records[members[0]].npc) originalRefs := cloneSourceRefs(output.SourceRefs) for _, member := range members[1:] { output.SourceRefs = append(output.SourceRefs, records[member].npc.SourceRefs...) } - output.SourceRefs, _, _ = canonicalizeSourceRefs(output.SourceRefs) + output.SourceRefs, _, _ = canonicalizeSourceRefs(order, output.SourceRefs) output.ID = identity.DeriveID(output.Name) return output, !reflect.DeepEqual(originalRefs, output.SourceRefs) } -func canonicalizeSourceRefs(input []source.SourceRef) ([]source.SourceRef, bool, int) { - if input == nil { - return nil, false, 0 - } - canonical := cloneSourceRefs(input) - sort.SliceStable(canonical, func(left, right int) bool { - if canonical[left].SourceID != canonical[right].SourceID { - return canonical[left].SourceID < canonical[right].SourceID - } - if canonical[left].StartUnitID != canonical[right].StartUnitID { - return canonical[left].StartUnitID < canonical[right].StartUnitID - } - return canonical[left].EndUnitID < canonical[right].EndUnitID - }) - orderChanged := !reflect.DeepEqual(input, canonical) - unique := make([]source.SourceRef, 0, len(canonical)) - for _, ref := range canonical { - if len(unique) == 0 || unique[len(unique)-1] != ref { - unique = append(unique, ref) - } - } - return unique, orderChanged, len(input) - len(unique) +func canonicalizeSourceRefs(order shared.SourceRefOrder, input []source.SourceRef) ([]source.SourceRef, bool, int) { + canonical := order.Canonicalize(input) + return canonical, !reflect.DeepEqual(input, canonical), len(input) - len(canonical) } func cloneSourceRefs(input []source.SourceRef) []source.SourceRef { diff --git a/internal/modules/dnd/normalize/npcs/normalizer_test.go b/internal/modules/dnd/normalize/npcs/normalizer_test.go index a795fd4..98f2dfa 100644 --- a/internal/modules/dnd/normalize/npcs/normalizer_test.go +++ b/internal/modules/dnd/normalize/npcs/normalizer_test.go @@ -61,6 +61,23 @@ func TestNormalizeNamesEvidenceAndIDs(t *testing.T) { } } +func TestNormalizeOrdersEvidenceBySourceDocumentPosition(t *testing.T) { + doc := &source.SourceDocument{ID: "source", Units: []source.SourceUnit{{ID: 30}, {ID: 10}}} + input := dnd.NPCList{NPCs: []dnd.NPC{{Name: "Lady Ash", SourceRefs: []source.SourceRef{ + {SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}, + {SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}, + {SourceID: doc.ID, StartUnitID: 999, EndUnitID: 999}, + }}}} + result, err := New(Options{}).Normalize(context.Background(), normalizeRequestWithSource(input, doc)) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + got := result.Value.NPCs[0].SourceRefs + if got[0].StartUnitID != 30 || got[1].StartUnitID != 10 || got[2].StartUnitID != 999 { + t.Fatalf("source refs = %#v, want source-document order followed by invalid reference", got) + } +} + func TestNormalizeConsolidatesCanonicalNamesOnlyAndUnionsEvidence(t *testing.T) { input := dnd.NPCList{NPCs: []dnd.NPC{ {Name: " Captain Vale ", SourceRefs: []source.SourceRef{{SourceID: "a", StartUnitID: 1, EndUnitID: 1}}}, @@ -115,6 +132,12 @@ func normalizeRequest(value dnd.NPCList) contracts.TypedNormalizeRequest[dnd.NPC return contracts.TypedNormalizeRequest[dnd.NPCList]{MergeOutput: contracts.MergeArtifact[dnd.NPCList]{Value: value}} } +func normalizeRequestWithSource(value dnd.NPCList, doc *source.SourceDocument) contracts.TypedNormalizeRequest[dnd.NPCList] { + request := normalizeRequest(value) + request.Source = doc + return request +} + func hasWarning(warnings []contracts.Warning, reason, scope string) bool { for _, warning := range warnings { if warning.ReasonCode == reason && warning.Scope == scope { diff --git a/internal/modules/dnd/normalize/spells/normalizer.go b/internal/modules/dnd/normalize/spells/normalizer.go index 72e0468..0e3dd4a 100644 --- a/internal/modules/dnd/normalize/spells/normalizer.go +++ b/internal/modules/dnd/normalize/spells/normalizer.go @@ -3,7 +3,6 @@ package spells import ( "context" "fmt" - "sort" "strconv" "strings" @@ -11,11 +10,16 @@ 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" spellcatalog "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/spells/catalog" "golang.org/x/text/cases" ) -const Key = "dnd/spells" +const ( + Key = "dnd/spells" + normalizationPolicy = "dnd.spells.normalize.v1" + NormalizationPolicy = normalizationPolicy +) const ( ReasonCodeSpellNameCanonicalized = "spell_name_canonicalized" @@ -65,9 +69,10 @@ func (n *Normalizer) ManifestMetadata() map[string]any { return nil } return map[string]any{ - "catalog_base_id": n.effectiveCatalog.BaseID(), - "catalog_digest": n.effectiveCatalog.Digest(), - "catalog_overlay_ids": append([]string(nil), n.effectiveCatalog.OverlayIDs()...), + "catalog_base_id": n.effectiveCatalog.BaseID(), + "catalog_digest": n.effectiveCatalog.Digest(), + "catalog_overlay_ids": append([]string(nil), n.effectiveCatalog.OverlayIDs()...), + "normalization_policy": normalizationPolicy, } } @@ -75,7 +80,10 @@ func (n *Normalizer) CheckpointFingerprints() []pipeline.CheckpointFingerprint { if n == nil { return nil } - return []pipeline.CheckpointFingerprint{{Name: "effective_catalog", Value: n.effectiveCatalog.Digest()}} + return []pipeline.CheckpointFingerprint{ + {Name: "effective_catalog", Value: n.effectiveCatalog.Digest()}, + {Name: "normalization_policy", Value: normalizationPolicy}, + } } func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalizeRequest[dnd.SpellList]) (contracts.TypedNormalizeResult[dnd.SpellList], error) { @@ -89,13 +97,14 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize return contracts.TypedNormalizeResult[dnd.SpellList]{}, normalizerErrorf("context error before normalize: %w", err) } - value, warnings := normalizeSpellList(req.MergeOutput.Value, n.effectiveCatalog) + order := shared.NewSourceRefOrder(req.Source) + value, warnings := normalizeSpellList(req.MergeOutput.Value, n.effectiveCatalog, order) value, duplicateWarnings := collapseDuplicateSpellCasts(value, req.Source, n.effectiveCatalog) warnings = append(warnings, duplicateWarnings...) return contracts.TypedNormalizeResult[dnd.SpellList]{Value: value, Warnings: warnings}, nil } -func normalizeSpellList(input dnd.SpellList, catalog spellcatalog.EffectiveCatalog) (dnd.SpellList, []contracts.Warning) { +func normalizeSpellList(input dnd.SpellList, catalog spellcatalog.EffectiveCatalog, order shared.SourceRefOrder) (dnd.SpellList, []contracts.Warning) { var warnings []contracts.Warning if input.SpellCasts == nil { return dnd.SpellList{}, nil @@ -123,7 +132,7 @@ func normalizeSpellList(input dnd.SpellList, catalog spellcatalog.EffectiveCatal }) } - canonicalRefs, orderChanged, duplicateCount := canonicalizeSourceRefs(inputCast.SourceRefs) + canonicalRefs, orderChanged, duplicateCount := canonicalizeSourceRefs(order, inputCast.SourceRefs) cast.SourceRefs = canonicalRefs if orderChanged || duplicateCount > 0 { warnings = append(warnings, contracts.Warning{ @@ -147,42 +156,21 @@ func cloneSpellCast(input dnd.SpellCast) dnd.SpellCast { return output } -func canonicalizeSourceRefs(input []source.SourceRef) ([]source.SourceRef, bool, int) { - if input == nil { - return nil, false, 0 - } - - canonical := make([]source.SourceRef, len(input)) - copy(canonical, input) - sort.SliceStable(canonical, func(left, right int) bool { - return sourceRefLess(canonical[left], canonical[right]) - }) - - orderChanged := false - for index := range input { - if input[index] != canonical[index] { - orderChanged = true - break - } - } - - unique := make([]source.SourceRef, 0, len(canonical)) - for _, ref := range canonical { - if len(unique) == 0 || unique[len(unique)-1] != ref { - unique = append(unique, ref) - } - } - return unique, orderChanged, len(input) - len(unique) +func canonicalizeSourceRefs(order shared.SourceRefOrder, input []source.SourceRef) ([]source.SourceRef, bool, int) { + canonical := order.Canonicalize(input) + return canonical, !sourceRefsEqual(input, canonical), len(input) - len(canonical) } -func sourceRefLess(left, right source.SourceRef) bool { - if left.SourceID != right.SourceID { - return left.SourceID < right.SourceID +func sourceRefsEqual(left, right []source.SourceRef) bool { + if (left == nil) != (right == nil) || len(left) != len(right) { + return false } - if left.StartUnitID != right.StartUnitID { - return left.StartUnitID < right.StartUnitID + for index := range left { + if left[index] != right[index] { + return false + } } - return left.EndUnitID < right.EndUnitID + return true } type duplicateGroup struct { diff --git a/internal/modules/dnd/normalize/spells/normalizer_test.go b/internal/modules/dnd/normalize/spells/normalizer_test.go index 1c90da5..5837741 100644 --- a/internal/modules/dnd/normalize/spells/normalizer_test.go +++ b/internal/modules/dnd/normalize/spells/normalizer_test.go @@ -106,12 +106,12 @@ func TestIdentityAndMetadataAreDefensive(t *testing.T) { } fingerprints := normalizer.CheckpointFingerprints() - if len(fingerprints) != 1 || fingerprints[0].Name != "effective_catalog" || fingerprints[0].Value != normalizer.effectiveCatalog.Digest() { - t.Fatalf("fingerprints = %#v, want effective catalog fingerprint", fingerprints) + if len(fingerprints) != 2 || fingerprints[0].Name != "effective_catalog" || fingerprints[0].Value != normalizer.effectiveCatalog.Digest() || fingerprints[1] != (pipeline.CheckpointFingerprint{Name: "normalization_policy", Value: normalizationPolicy}) { + t.Fatalf("fingerprints = %#v, want catalog and normalization policy fingerprints", fingerprints) } fingerprints[0].Name = "changed" fingerprints[0].Value = "changed" - if got := normalizer.CheckpointFingerprints(); len(got) != 1 || got[0].Name != "effective_catalog" || got[0].Value != normalizer.effectiveCatalog.Digest() { + if got := normalizer.CheckpointFingerprints(); len(got) != 2 || got[0].Name != "effective_catalog" || got[0].Value != normalizer.effectiveCatalog.Digest() || got[1] != (pipeline.CheckpointFingerprint{Name: "normalization_policy", Value: normalizationPolicy}) { t.Fatalf("fingerprints were not defensive: %#v", got) } @@ -210,6 +210,26 @@ func TestNormalizeSortsAndDeduplicatesExactSourceReferences(t *testing.T) { } } +func TestNormalizeOrdersReferencesBySourceDocumentPosition(t *testing.T) { + doc := &source.SourceDocument{ID: "source", Units: []source.SourceUnit{{ID: 30}, {ID: 10}}} + input := dnd.SpellList{SpellCasts: []dnd.SpellCast{{ + Spell: "Cure Wounds", + SourceRefs: []source.SourceRef{ + {SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}, + {SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}, + {SourceID: doc.ID, StartUnitID: 999, EndUnitID: 999}, + }, + }}} + result, err := newNormalizer(t).Normalize(context.Background(), normalizeRequestWithSource(input, doc)) + if err != nil { + t.Fatalf("Normalize() error = %v, want nil", err) + } + got := result.Value.SpellCasts[0].SourceRefs + if got[0].StartUnitID != 30 || got[1].StartUnitID != 10 || got[2].StartUnitID != 999 { + t.Fatalf("normalized refs = %#v, want source-document order followed by invalid reference", got) + } +} + func TestNormalizePreservesNilEmptyAndAdjacentOrOverlappingReferences(t *testing.T) { normalizer := newNormalizer(t) input := dnd.SpellList{SpellCasts: []dnd.SpellCast{ diff --git a/internal/modules/dnd/npcinteractions/canonical.go b/internal/modules/dnd/npcinteractions/canonical.go index 66883ea..7925bf8 100644 --- a/internal/modules/dnd/npcinteractions/canonical.go +++ b/internal/modules/dnd/npcinteractions/canonical.go @@ -3,34 +3,15 @@ package npcinteractions import ( - "sort" "strconv" "strings" "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) -// CanonicalizeSourceRefs returns a cloned, document-ordered, de-duplicated -// source-reference list. -func CanonicalizeSourceRefs(doc *source.SourceDocument, input []source.SourceRef) []source.SourceRef { - if input == nil { - return nil - } - canonical := append([]source.SourceRef(nil), input...) - sort.SliceStable(canonical, func(left, right int) bool { - return SourceRefLess(doc, canonical[left], canonical[right]) - }) - unique := make([]source.SourceRef, 0, len(canonical)) - for _, ref := range canonical { - if len(unique) == 0 || unique[len(unique)-1] != ref { - unique = append(unique, ref) - } - } - return unique -} - // SourceRefsEqual reports whether two source-reference lists have identical // representations and values. func SourceRefsEqual(left, right []source.SourceRef) bool { @@ -45,39 +26,10 @@ func SourceRefsEqual(left, right []source.SourceRef) bool { return true } -// SourceRefLess orders references by source identity and then by the source -// document positions of their endpoints. Invalid endpoints sort after valid -// endpoints and fall back to their literal IDs for deterministic diagnostics. -func SourceRefLess(doc *source.SourceDocument, left, right source.SourceRef) bool { - if left.SourceID != right.SourceID { - return left.SourceID < right.SourceID - } - leftStart, leftStartOK := source.UnitIndex(doc, left.StartUnitID) - rightStart, rightStartOK := source.UnitIndex(doc, right.StartUnitID) - if leftStartOK != rightStartOK { - return leftStartOK - } - if leftStartOK && leftStart != rightStart { - return leftStart < rightStart - } - if left.StartUnitID != right.StartUnitID { - return left.StartUnitID < right.StartUnitID - } - leftEnd, leftEndOK := source.UnitIndex(doc, left.EndUnitID) - rightEnd, rightEndOK := source.UnitIndex(doc, right.EndUnitID) - if leftEndOK != rightEndOK { - return leftEndOK - } - if leftEndOK && leftEnd != rightEnd { - return leftEnd < rightEnd - } - return left.EndUnitID < right.EndUnitID -} - // Less defines the canonical order for NPC interaction occurrences. -func Less(doc *source.SourceDocument, left, right dnd.NPCInteraction) bool { - leftPosition, leftHasEvidence := EarliestSourcePosition(doc, left) - rightPosition, rightHasEvidence := EarliestSourcePosition(doc, right) +func Less(order shared.SourceRefOrder, left, right dnd.NPCInteraction) bool { + leftPosition, leftHasEvidence := order.EarliestValid(left.SourceRefs) + rightPosition, rightHasEvidence := order.EarliestValid(right.SourceRefs) if leftHasEvidence != rightHasEvidence { return leftHasEvidence } @@ -95,25 +47,7 @@ func Less(doc *source.SourceDocument, left, right dnd.NPCInteraction) bool { if left.Kind != right.Kind { return left.Kind < right.Kind } - return sourceRefsLess(doc, left.SourceRefs, right.SourceRefs) -} - -// EarliestSourcePosition returns the earliest valid cited position. -func EarliestSourcePosition(doc *source.SourceDocument, interaction dnd.NPCInteraction) (int, bool) { - found := false - earliest := 0 - for _, ref := range interaction.SourceRefs { - if source.ValidateRef(doc, ref) != nil { - continue - } - position, ok := source.UnitIndex(doc, ref.StartUnitID) - if !ok || (found && position >= earliest) { - continue - } - earliest = position - found = true - } - return earliest, found + return sourceRefsLess(order, left.SourceRefs, right.SourceRefs) } // ValidSourceRefs reports whether an interaction has non-empty, valid @@ -144,12 +78,12 @@ func ExactIdentity(interaction dnd.NPCInteraction) string { return key.String() } -func sourceRefsLess(doc *source.SourceDocument, left, right []source.SourceRef) bool { +func sourceRefsLess(order shared.SourceRefOrder, left, right []source.SourceRef) bool { for index := 0; index < len(left) && index < len(right); index++ { if left[index] == right[index] { continue } - return SourceRefLess(doc, left[index], right[index]) + return order.Less(left[index], right[index]) } return len(left) < len(right) } diff --git a/internal/modules/dnd/shared/source_ref_order.go b/internal/modules/dnd/shared/source_ref_order.go new file mode 100644 index 0000000..5685788 --- /dev/null +++ b/internal/modules/dnd/shared/source_ref_order.go @@ -0,0 +1,107 @@ +package shared + +import ( + "sort" + "strings" + + "gitea.maximumdirect.net/eric/notarius/internal/core/source" +) + +// SourceRefOrder provides a stable snapshot of a source document's unit +// ordering for source-reference comparison and canonicalization. +type SourceRefOrder struct { + sourceID string + positions map[int]int +} + +// NewSourceRefOrder captures the source identity and unit positions from doc. +func NewSourceRefOrder(doc *source.SourceDocument) SourceRefOrder { + if doc == nil { + return SourceRefOrder{} + } + positions := make(map[int]int, len(doc.Units)) + for position, unit := range doc.Units { + if _, exists := positions[unit.ID]; !exists { + positions[unit.ID] = position + } + } + return SourceRefOrder{sourceID: doc.ID, positions: positions} +} + +// Less orders references by source identity, then document positions when +// available, and finally literal endpoint IDs. +func (o SourceRefOrder) Less(left, right source.SourceRef) bool { + if left.SourceID != right.SourceID { + return left.SourceID < right.SourceID + } + positions := o.positionsFor(left.SourceID) + if lessEndpoint(positions, left.StartUnitID, right.StartUnitID) { + return true + } + if lessEndpoint(o.positions, right.StartUnitID, left.StartUnitID) { + return false + } + return lessEndpoint(positions, left.EndUnitID, right.EndUnitID) +} + +// EarliestValid returns the earliest document position among valid refs. +func (o SourceRefOrder) EarliestValid(refs []source.SourceRef) (int, bool) { + if len(refs) == 0 || o.sourceID == "" { + return 0, false + } + found := false + earliest := 0 + for _, ref := range refs { + if ref.SourceID != o.sourceID || strings.TrimSpace(ref.SourceID) != ref.SourceID || ref.StartUnitID <= 0 || ref.EndUnitID <= 0 { + continue + } + start, startOK := o.positions[ref.StartUnitID] + end, endOK := o.positions[ref.EndUnitID] + if !startOK || !endOK || start > end { + continue + } + if !found || start < earliest { + earliest = start + found = true + } + } + return earliest, found +} + +// Canonicalize returns an owned, stable-sorted, exactly de-duplicated copy of +// refs. It deliberately preserves invalid references for diagnostics. +func (o SourceRefOrder) Canonicalize(refs []source.SourceRef) []source.SourceRef { + if refs == nil { + return nil + } + canonical := append([]source.SourceRef{}, refs...) + sort.SliceStable(canonical, func(left, right int) bool { + return o.Less(canonical[left], canonical[right]) + }) + unique := make([]source.SourceRef, 0, len(canonical)) + for _, ref := range canonical { + if len(unique) == 0 || unique[len(unique)-1] != ref { + unique = append(unique, ref) + } + } + return unique +} + +func (o SourceRefOrder) positionsFor(sourceID string) map[int]int { + if o.sourceID == "" || sourceID != o.sourceID { + return nil + } + return o.positions +} + +func lessEndpoint(positions map[int]int, left, right int) bool { + leftPosition, leftOK := positions[left] + rightPosition, rightOK := positions[right] + if leftOK != rightOK { + return leftOK + } + if leftOK && leftPosition != rightPosition { + return leftPosition < rightPosition + } + return left < right +} diff --git a/internal/modules/dnd/shared/source_ref_order_test.go b/internal/modules/dnd/shared/source_ref_order_test.go new file mode 100644 index 0000000..d5e8614 --- /dev/null +++ b/internal/modules/dnd/shared/source_ref_order_test.go @@ -0,0 +1,81 @@ +package shared + +import ( + "reflect" + "testing" + + "gitea.maximumdirect.net/eric/notarius/internal/core/source" +) + +func TestSourceRefOrderUsesDocumentOrderAndLiteralFallbacks(t *testing.T) { + doc := unitRefSourceDocument(30, 10, 20) + order := NewSourceRefOrder(doc) + + refs := []source.SourceRef{ + {SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}, + {SourceID: doc.ID, StartUnitID: 999, EndUnitID: 999}, + {SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}, + {SourceID: "other", StartUnitID: 20, EndUnitID: 20}, + {SourceID: "other", StartUnitID: 10, EndUnitID: 10}, + } + got := order.Canonicalize(refs) + want := []source.SourceRef{ + {SourceID: "other", StartUnitID: 10, EndUnitID: 10}, + {SourceID: "other", StartUnitID: 20, EndUnitID: 20}, + {SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}, + {SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}, + {SourceID: doc.ID, StartUnitID: 999, EndUnitID: 999}, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("Canonicalize() = %#v, want %#v", got, want) + } + if order.Less(want[0], want[0]) { + t.Fatal("Less(ref, ref) = true, want false") + } +} + +func TestSourceRefOrderCanonicalizePreservesRepresentationAndOwnership(t *testing.T) { + order := NewSourceRefOrder(unitRefSourceDocument(3, 1, 2)) + if got := order.Canonicalize(nil); got != nil { + t.Fatalf("Canonicalize(nil) = %#v, want nil", got) + } + empty := []source.SourceRef{} + if got := order.Canonicalize(empty); got == nil || len(got) != 0 { + t.Fatalf("Canonicalize(empty) = %#v, want owned empty slice", got) + } + input := []source.SourceRef{ + {SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 2}, + {SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 2}, + {SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 3}, + } + got := order.Canonicalize(input) + if len(got) != 2 || got[0] == got[1] { + t.Fatalf("Canonicalize() = %#v, want exact duplicate removed but distinct range retained", got) + } + got[0].StartUnitID = 99 + if input[0].StartUnitID == 99 { + t.Fatal("Canonicalize() output aliases input") + } +} + +func TestSourceRefOrderSnapshotAndEarliestValid(t *testing.T) { + doc := unitRefSourceDocument(30, 10, 20) + order := NewSourceRefOrder(doc) + doc.Units[0].ID, doc.Units[1].ID = 10, 30 + refs := []source.SourceRef{ + {SourceID: doc.ID, StartUnitID: 20, EndUnitID: 20}, + {SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}, + {SourceID: "other", StartUnitID: 1, EndUnitID: 1}, + {SourceID: doc.ID, StartUnitID: 999, EndUnitID: 999}, + {SourceID: doc.ID, StartUnitID: 20, EndUnitID: 30}, + } + if position, ok := order.EarliestValid(refs); !ok || position != 1 { + t.Fatalf("EarliestValid() = %d, %t, want 1, true", position, ok) + } + if position, ok := order.EarliestValid([]source.SourceRef{{SourceID: doc.ID, StartUnitID: 20, EndUnitID: 30}}); ok || position != 0 { + t.Fatalf("EarliestValid(invalid) = %d, %t, want 0, false", position, ok) + } + if position, ok := (SourceRefOrder{}).EarliestValid(refs); ok || position != 0 { + t.Fatalf("zero EarliestValid() = %d, %t, want 0, false", position, ok) + } +} diff --git a/internal/modules/dnd/validate/combatturns/invariants/validator.go b/internal/modules/dnd/validate/combatturns/invariants/validator.go index d7f27e6..c2d65ff 100644 --- a/internal/modules/dnd/validate/combatturns/invariants/validator.go +++ b/internal/modules/dnd/validate/combatturns/invariants/validator.go @@ -12,6 +12,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" + "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" ) @@ -19,7 +20,7 @@ import ( const ( Key = "normalize/dnd/combat-turns/invariants" ReasonCode = "invalid_combat_turn_normalization" - policy = "dnd.combat_turns.validator.normalized.v1" + policy = "dnd.combat_turns.validator.normalized.v2" ) type Options struct{} @@ -50,14 +51,14 @@ func Validate(doc *source.SourceDocument, value dnd.CombatTurnList) error { if combatshape.Validate(value) != nil || !sourceRefsValid(doc, value) { return nil } - issues := issuesFor(doc, value) + issues := issuesFor(shared.NewSourceRefOrder(doc), value) if len(issues) == 0 { return nil } return fmt.Errorf("%s", diagnostics.Aggregate("invalid combat turn normalization", issues)) } -func issuesFor(doc *source.SourceDocument, value dnd.CombatTurnList) []string { +func issuesFor(order shared.SourceRefOrder, value dnd.CombatTurnList) []string { issues := make([]string, 0) seenIdentity := make(map[string]int) previousPosition := -1 @@ -69,14 +70,14 @@ func issuesFor(doc *source.SourceDocument, value dnd.CombatTurnList) []string { for refIndex := 1; refIndex < len(turn.SourceRefs); refIndex++ { previous := turn.SourceRefs[refIndex-1] current := turn.SourceRefs[refIndex] - if sourceRefLess(current, previous) { + if order.Less(current, previous) { issues = append(issues, fmt.Sprintf("%s.source_refs are not in canonical order at index %d", prefix, refIndex)) } else if current == previous { issues = append(issues, fmt.Sprintf("%s.source_refs[%d] duplicates the previous reference", prefix, refIndex)) } } - position, ok := earliestSourcePosition(doc, turn) + position, ok := order.EarliestValid(turn.SourceRefs) if !ok { continue } @@ -107,36 +108,6 @@ func sourceRefsValid(doc *source.SourceDocument, value dnd.CombatTurnList) bool return true } -func sourceRefLess(left, right source.SourceRef) bool { - if left.SourceID != right.SourceID { - return left.SourceID < right.SourceID - } - if left.StartUnitID != right.StartUnitID { - return left.StartUnitID < right.StartUnitID - } - return left.EndUnitID < right.EndUnitID -} - -func earliestSourcePosition(doc *source.SourceDocument, turn dnd.CombatTurn) (int, bool) { - if doc == nil { - return 0, false - } - earliest := 0 - found := false - for _, ref := range turn.SourceRefs { - if source.ValidateRef(doc, ref) != nil { - continue - } - index, ok := source.UnitIndex(doc, ref.StartUnitID) - if !ok || (found && index >= earliest) { - continue - } - earliest = index - found = true - } - return earliest, found -} - func duplicateKey(turn dnd.CombatTurn) (string, bool) { if len(turn.SourceRefs) == 0 { return "", false diff --git a/internal/modules/dnd/validate/combatturns/invariants/validator_test.go b/internal/modules/dnd/validate/combatturns/invariants/validator_test.go index 8fc9658..912b403 100644 --- a/internal/modules/dnd/validate/combatturns/invariants/validator_test.go +++ b/internal/modules/dnd/validate/combatturns/invariants/validator_test.go @@ -57,6 +57,24 @@ func TestValidateRejectsOwnedNormalizedInvariants(t *testing.T) { } } +func TestValidateUsesSourceDocumentOrder(t *testing.T) { + doc := &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 30}, {ID: 10}}} + value := dnd.CombatTurnList{CombatTurns: []dnd.CombatTurn{ + {Actor: "Aria", TurnKind: dnd.CombatTurnKindTurn, SourceRefs: []source.SourceRef{{SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}, {SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}}}, + }} + if err := Validate(doc, value); err == nil || !strings.Contains(err.Error(), "not in canonical order") { + t.Fatalf("Validate() error = %v, want document-order reference rejection", err) + } + + value = dnd.CombatTurnList{CombatTurns: []dnd.CombatTurn{ + {Actor: "Aria", TurnKind: dnd.CombatTurnKindTurn, SourceRefs: []source.SourceRef{{SourceID: doc.ID, StartUnitID: 10, EndUnitID: 10}}}, + {Actor: "Borin", TurnKind: dnd.CombatTurnKindTurn, SourceRefs: []source.SourceRef{{SourceID: doc.ID, StartUnitID: 30, EndUnitID: 30}}}, + }} + if err := Validate(doc, value); err == nil || !strings.Contains(err.Error(), "out of chronological order") { + t.Fatalf("Validate() error = %v, want document-order chronology rejection", err) + } +} + func TestValidatorDefersShapeAndSourceReferenceFailures(t *testing.T) { doc := invariantDocument() shapeInvalid := normalizedList() diff --git a/internal/modules/dnd/validate/npcinteractions/invariants/validator.go b/internal/modules/dnd/validate/npcinteractions/invariants/validator.go index 36114e1..064d561 100644 --- a/internal/modules/dnd/validate/npcinteractions/invariants/validator.go +++ b/internal/modules/dnd/validate/npcinteractions/invariants/validator.go @@ -12,6 +12,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" interactionmodel "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcinteractions" npcregistry "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/registry" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" interactionshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcinteractions/shape" ) @@ -89,7 +90,7 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq if !npcRegistry.Bound() { return contracts.ValidationResult{Approved: false, ReasonCode: ReasonCode, Message: "invalid NPC interaction normalization: NPC registry reference is required"}, nil } - issues := issuesFor(req.Source, req.Value, npcRegistry) + issues := issuesFor(req.Source, shared.NewSourceRefOrder(req.Source), req.Value, npcRegistry) if len(issues) == 0 { return contracts.ValidationResult{Approved: true}, nil } @@ -109,7 +110,7 @@ func allSourceRefsValid(doc *source.SourceDocument, value dnd.NPCInteractionList return true } -func issuesFor(doc *source.SourceDocument, value dnd.NPCInteractionList, npcRegistry *npcregistry.Registry) []string { +func issuesFor(doc *source.SourceDocument, order shared.SourceRefOrder, value dnd.NPCInteractionList, npcRegistry *npcregistry.Registry) []string { issues := make([]string, 0) for index, interaction := range value.Interactions { prefix := fmt.Sprintf("interactions[%d]", index) @@ -119,7 +120,7 @@ func issuesFor(doc *source.SourceDocument, value dnd.NPCInteractionList, npcRegi for refIndex := 1; refIndex < len(interaction.SourceRefs); refIndex++ { previous := interaction.SourceRefs[refIndex-1] current := interaction.SourceRefs[refIndex] - if interactionmodel.SourceRefLess(doc, current, previous) { + if order.Less(current, previous) { issues = append(issues, fmt.Sprintf("%s.source_refs are not in canonical order at index %d", prefix, refIndex)) } else if current == previous { issues = append(issues, fmt.Sprintf("%s.source_refs[%d] duplicates the previous reference", prefix, refIndex)) @@ -128,7 +129,7 @@ func issuesFor(doc *source.SourceDocument, value dnd.NPCInteractionList, npcRegi } if !sort.SliceIsSorted(value.Interactions, func(left, right int) bool { - return interactionmodel.Less(doc, value.Interactions[left], value.Interactions[right]) + return interactionmodel.Less(order, value.Interactions[left], value.Interactions[right]) }) { issues = append(issues, "interactions are not in canonical order") } diff --git a/internal/modules/integration/dnd_npcs_runner_test.go b/internal/modules/integration/dnd_npcs_runner_test.go index a9d7c43..76e718a 100644 --- a/internal/modules/integration/dnd_npcs_runner_test.go +++ b/internal/modules/integration/dnd_npcs_runner_test.go @@ -95,7 +95,7 @@ func TestRunnerProcessesSeriatimInputWithProductionDNDNPCPipeline(t *testing.T) t.Fatalf("manifest lane = %#v, want NPC production composition", lane) } normalizerMetadata, ok := lane.Metadata["normalizer"].(map[string]any) - if !ok || normalizerMetadata["identity_policy"] != identity.Policy || normalizerMetadata["normalization_policy"] != "dnd.npcs.normalize.v1" { + if !ok || normalizerMetadata["identity_policy"] != identity.Policy || normalizerMetadata["normalization_policy"] != "dnd.npcs.normalize.v2" { t.Fatalf("normalizer metadata = %#v, want identity and normalization policies", lane.Metadata) } var npcOutputFile *contracts.OutputFile