diff --git a/docs/config.md b/docs/config.md index 361dd20..d15117a 100644 --- a/docs/config.md +++ b/docs/config.md @@ -387,8 +387,8 @@ production validators do not call the LLM and must not set `llm_profile`. | `normalize/dnd/combat-turns/invariants` | deterministic | Rejects normalized combat-turn identity, evidence-order, and chronology violations. | | `extract/dnd/npc-interactions/shape` | deterministic | Rejects malformed D&D NPC-interaction-list artifacts. | | `extract/dnd/npc-interactions/registry` | deterministic | Rejects interaction names absent from the supplied NPC registry. | -| `extract/dnd/npc-interactions/source_refs` | deterministic | Rejects missing or invalid D&D interaction source references. | -| `extract/dnd/npc-interactions/source_relatedness` | deterministic | Emits warnings when an interaction name is not found near cited source text. | +| `extract/dnd/npc-interactions/source_refs` | deterministic | Rejects missing, invalid, or extract-chunk-external D&D interaction source references. | +| `extract/dnd/npc-interactions/source_relatedness` | deterministic | Emits bounded warnings when an interaction name is not found in its cited source text. | | `normalize/dnd/npc-interactions/invariants` | deterministic | Rejects normalized interaction identity, evidence-order, and chronology violations. | The production default chain for `dnd/spells` is used for both its extract and @@ -460,9 +460,9 @@ normalize: - generic/valid_json - extract/dnd/npc-interactions/shape - extract/dnd/npc-interactions/registry + - normalize/dnd/npc-interactions/invariants - extract/dnd/npc-interactions/source_refs - generic/valid_json_schema - - normalize/dnd/npc-interactions/invariants - extract/dnd/npc-interactions/source_relatedness ``` diff --git a/docs/integrations/dnd-npc-interaction-artifacts.md b/docs/integrations/dnd-npc-interaction-artifacts.md index 85192e7..dee7ecc 100644 --- a/docs/integrations/dnd-npc-interaction-artifacts.md +++ b/docs/integrations/dnd-npc-interaction-artifacts.md @@ -33,23 +33,57 @@ array may be empty. Each item has exactly `name`, `kind`, and `source_refs`: `name` is the canonical display name from the required NPC registry. `source_refs` contains one or more current-source ranges with required `source_id`, `start_unit_id`, and `end_unit_id`; unit IDs are positive integers. +During extraction, every range must be wholly contained in the current accepted +chunk. This prevents a candidate from citing valid units that were not presented +to that extraction call. Unknown fields are rejected. ## Interaction Categories `kind` is exactly one of: -- `mentioned`: the named NPC is referenced without stronger participation. -- `noncombat_presence`: the NPC is present in the current scene without a - dialogue or combat classification. -- `dialogue`: the NPC participates in spoken interaction. -- `combat_ally`: the NPC participates in combat aligned with the party. -- `combat_opponent`: the NPC participates in combat against the party. -- `other`: a transcript-supported interaction outside the bounded categories. +| 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 a fallback +for uncertain classification. When activities overlap, active combat +participation outranks dialogue, presence, and mention; dialogue outranks +non-combat presence and mention; and non-combat presence outranks mention. +Combat alignment is not resolved by precedence: a meaningful change between +ally and opponent creates separate occurrences. These categories do not encode summaries, relationships, state, motives, or unobserved events. +## Occurrence Boundaries And Ordering + +One occurrence represents one NPC, one kind, and one locally coherent passage +within one accepted chunk. Repeated evidence belongs to the same occurrence +only while it supports the same uninterrupted activity. A kind change, combat +alignment change, intervening scene or meaningful absence, or transition from +mention to presence starts a new occurrence. Occurrences never span chunks, and +merge or normalization never semantically combines nearby, overlapping, or +cross-chunk records. + +Normalization orders records by: + +1. earliest valid source-document position; +2. the NPC identity comparison key; +3. the exact canonical NPC display name; +4. interaction kind in lexical order; and +5. the complete canonical source-reference sequence, ordered by source ID and + the source-document positions of each range's start and end. + +Only records with identical canonical names, kinds, and complete valid evidence +sequences are duplicates. Different categories, ranges, or separately grounded +occurrences remain separate. + ## Evidence, Registry, And Normalization The registry proves only the canonical NPC identity. Its source references are @@ -59,11 +93,8 @@ classification. The extractor receives a names-only registry projection such as `{"npcs":[{"name":"Mira Thorn"}]}`. The normalizer uses the full immutable -registry for exact canonical-name lookup. It orders source references, -stable-sorts occurrences by their earliest source-document position, and -collapses only exact duplicates with the same canonical name, kind, and complete -valid evidence. Different categories, distinct ranges, and separately grounded -occurrences remain separate; no semantic merge is performed. +registry for exact canonical-name lookup. It canonicalizes source references +and applies the ordering and exact-duplicate rules above. ## Production Pipeline @@ -102,8 +133,10 @@ copying registry names, source ranges, or payload content into the manifest. The default extract chain is `generic/valid_json`, interaction shape, registry, and source-reference validation, `generic/valid_json_schema`, then warning-only -source relatedness. The normalize chain adds normalized invariants after schema -validation and before relatedness. The codec metadata contains only +source relatedness. The normalize chain runs normalized invariants after +registry validation and before source-reference and schema validation, followed +by relatedness. Normalizer and relatedness warnings are bounded and end with an +omission summary when necessary. The codec metadata contains only `interaction_count`. Extractor metadata identifies its prompt and private response schema; component-local checkpoint identities include the names-only registry projection where relevant. Generated registry identity stays in diff --git a/docs/internal/modules.md b/docs/internal/modules.md index fa33a2d..f7608d0 100644 --- a/docs/internal/modules.md +++ b/docs/internal/modules.md @@ -299,10 +299,19 @@ evidence, identity, and transcript material, then maps private model records to references are never reused as interaction evidence. The private response schema carries only name, bounded interaction kind, and source-unit ranges; deterministic validators own registry membership, source validity, and -relatedness. Prompt, schema, mapping, and the names-only registry projection +relatedness. Extract-stage source validation additionally requires every cited +range to be wholly contained in the current materialized chunk. Prompt, schema, +mapping, and the names-only registry projection participate in checkpoint identity, while generated producer identity remains framework provenance. +The domain-owned `internal/modules/dnd/npcinteractions` package defines +canonical source-reference and occurrence ordering, valid-evidence eligibility, +and collision-safe exact identity. The interaction normalizer and normalized +invariants validator both consume those rules, so their production and checking +paths cannot drift. Normalizer and relatedness warning lists use the shared D&D +diagnostic cap and emit a final omission-summary warning when truncated. + ### `internal/modules/dnd/normalize/npcs` The NPC normalizer performs deterministic identity-aware consolidation in diff --git a/docs/internal/overview.md b/docs/internal/overview.md index 3d1238b..1d50257 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -97,6 +97,7 @@ Configuration. The implemented module packages are: | `internal/modules/dnd/extract/npcs` | Maps private structured model output to canonical source-grounded D&D NPC lists. | | `internal/modules/dnd/extract/combatturns` | Maps private structured model output to source-grounded D&D combat-turn candidates and preserves chronology and invalid candidate values for validators. | | `internal/modules/dnd/extract/npcinteractions` | Maps private structured model output to current-source NPC interaction candidates grounded by a required registry. | +| `internal/modules/dnd/npcinteractions` | Owns canonical source-reference ordering, occurrence ordering, valid-evidence checks, and exact interaction identity shared by normalization and invariant validation. | | `internal/modules/dnd/normalize/combatturns` | Canonicalizes and orders merged combat turns, applies exact NPC identity matches, and collapses only exact valid-evidence duplicates. | | `internal/modules/dnd/normalize/npcinteractions` | Canonicalizes required-registry names, orders interaction occurrences, and collapses only exact valid-evidence duplicates. | | `internal/modules/dnd/validate/combatturns` | Provides deterministic shape, source-reference, source-relatedness, and normalized-invariant validation for the production combat chains. | diff --git a/internal/modules/dnd/normalize/npcinteractions/normalizer.go b/internal/modules/dnd/normalize/npcinteractions/normalizer.go index 89ea765..26ad37e 100644 --- a/internal/modules/dnd/normalize/npcinteractions/normalizer.go +++ b/internal/modules/dnd/normalize/npcinteractions/normalizer.go @@ -5,13 +5,12 @@ import ( "context" "fmt" "sort" - "strconv" - "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" + interactionmodel "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcinteractions" "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" @@ -20,13 +19,14 @@ import ( const ( Key = "dnd/npc-interactions" - normalizationPolicy = "dnd.npc_interactions.normalize.v1" + normalizationPolicy = "dnd.npc_interactions.normalize.v2" NormalizationPolicy = normalizationPolicy ReasonCodeNameCanonicalized = "npc_interaction_name_canonicalized" ReasonCodeSourceRefsNormalized = "source_references_normalized" ReasonCodeInteractionsReordered = "npc_interactions_reordered" ReasonCodeDuplicateCollapsed = "duplicate_npc_interaction_collapsed" + ReasonCodeWarningsOmitted = "npc_interaction_normalization_warnings_omitted" ) const ( @@ -125,8 +125,6 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize type normalizedRecord struct { interaction dnd.NPCInteraction inputIndex int - earliest int - hasEvidence bool } type nameCanonicalization struct { @@ -143,8 +141,7 @@ func normalizeList(input dnd.NPCInteractionList, doc *source.SourceDocument, reg warnings := make([]contracts.Warning, 0) for index, inputInteraction := range input.Interactions { interaction, nameChange, refsChanged := normalizeInteraction(inputInteraction, doc, registry) - earliest, hasEvidence := earliestSourcePosition(doc, interaction) - records[index] = normalizedRecord{interaction: interaction, inputIndex: index, earliest: earliest, hasEvidence: hasEvidence} + records[index] = normalizedRecord{interaction: interaction, inputIndex: index} if nameChange != nil { warnings = append(warnings, contracts.Warning{ Scope: interactionScope(index), @@ -163,7 +160,9 @@ func normalizeList(input dnd.NPCInteractionList, doc *source.SourceDocument, reg } } - sort.SliceStable(records, func(left, right int) bool { return recordLess(doc, records[left], records[right]) }) + sort.SliceStable(records, func(left, right int) bool { + return interactionmodel.Less(doc, records[left].interaction, records[right].interaction) + }) for position, record := range records { if position == record.inputIndex { continue @@ -177,7 +176,8 @@ func normalizeList(input dnd.NPCInteractionList, doc *source.SourceDocument, reg output, duplicateWarnings := collapseDuplicates(records, doc) warnings = append(warnings, duplicateWarnings...) - return dnd.NPCInteractionList{Interactions: output}, warnings + return dnd.NPCInteractionList{Interactions: output}, + diagnostics.LimitWarnings(warnings, "npc_interactions", ReasonCodeWarningsOmitted) } func normalizeInteraction(input dnd.NPCInteraction, doc *source.SourceDocument, registry *npcregistry.Registry) (dnd.NPCInteraction, *nameCanonicalization, bool) { @@ -189,8 +189,8 @@ func normalizeInteraction(input dnd.NPCInteraction, doc *source.SourceDocument, if input.Name != output.Name { nameChange = &nameCanonicalization{from: input.Name, to: output.Name} } - output.SourceRefs = canonicalizeSourceRefs(doc, input.SourceRefs) - return output, nameChange, !sourceRefsEqual(input.SourceRefs, output.SourceRefs) + output.SourceRefs = interactionmodel.CanonicalizeSourceRefs(doc, input.SourceRefs) + return output, nameChange, !interactionmodel.SourceRefsEqual(input.SourceRefs, output.SourceRefs) } func cloneInteraction(input dnd.NPCInteraction) dnd.NPCInteraction { @@ -201,107 +201,6 @@ func cloneInteraction(input dnd.NPCInteraction) dnd.NPCInteraction { return output } -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 -} - -func sourceRefsEqual(left, right []source.SourceRef) bool { - if (left == nil) != (right == nil) || len(left) != len(right) { - return false - } - for index := range left { - if left[index] != right[index] { - return false - } - } - return true -} - -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 -} - -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 -} - -func recordLess(doc *source.SourceDocument, left, right normalizedRecord) bool { - if left.hasEvidence != right.hasEvidence { - return left.hasEvidence - } - if left.hasEvidence && left.earliest != right.earliest { - return left.earliest < right.earliest - } - leftKey := identity.ComparisonKey(left.interaction.Name) - rightKey := identity.ComparisonKey(right.interaction.Name) - if leftKey != rightKey { - return leftKey < rightKey - } - if left.interaction.Name != right.interaction.Name { - return left.interaction.Name < right.interaction.Name - } - if left.interaction.Kind != right.interaction.Kind { - return left.interaction.Kind < right.interaction.Kind - } - return sourceRefsLess(doc, left.interaction.SourceRefs, right.interaction.SourceRefs) -} - -func sourceRefsLess(doc *source.SourceDocument, 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 len(left) < len(right) -} - type duplicateGroup struct { retainedIndex int removed []int @@ -315,11 +214,11 @@ func collapseDuplicates(records []normalizedRecord, doc *source.SourceDocument) groups := make([]duplicateGroup, 0) groupByKey := make(map[string]int) for index, record := range records { - key, eligible := duplicateKey(record.interaction, doc) - if !eligible { + if !interactionmodel.ValidSourceRefs(doc, record.interaction.SourceRefs) { keep[index] = true continue } + key := interactionmodel.ExactIdentity(record.interaction) groupIndex, exists := groupByKey[key] if !exists { groupByKey[key] = len(groups) @@ -344,37 +243,6 @@ func collapseDuplicates(records []normalizedRecord, doc *source.SourceDocument) return output, warnings } -func duplicateKey(interaction dnd.NPCInteraction, doc *source.SourceDocument) (string, bool) { - if len(interaction.SourceRefs) == 0 { - return "", false - } - for _, ref := range interaction.SourceRefs { - if source.ValidateRef(doc, ref) != nil { - return "", false - } - } - var key strings.Builder - writeKeyString(&key, interaction.Name) - writeKeyString(&key, string(interaction.Kind)) - for _, ref := range interaction.SourceRefs { - writeKeyString(&key, ref.SourceID) - writeKeyInt(&key, ref.StartUnitID) - writeKeyInt(&key, ref.EndUnitID) - } - return key.String(), true -} - -func writeKeyString(builder *strings.Builder, value string) { - builder.WriteString(strconv.Itoa(len(value))) - builder.WriteByte(':') - builder.WriteString(value) -} - -func writeKeyInt(builder *strings.Builder, value int) { - builder.WriteString(strconv.Itoa(value)) - builder.WriteByte(';') -} - func duplicateWarning(retainedIndex int, removed []int) contracts.Warning { issues := make([]string, len(removed)) for index, removedIndex := range removed { diff --git a/internal/modules/dnd/normalize/npcinteractions/normalizer_test.go b/internal/modules/dnd/normalize/npcinteractions/normalizer_test.go index 9768e1d..c6312eb 100644 --- a/internal/modules/dnd/normalize/npcinteractions/normalizer_test.go +++ b/internal/modules/dnd/normalize/npcinteractions/normalizer_test.go @@ -11,6 +11,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" npccodec "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/codec/npcs" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/diagnostics" ) func TestNormalizeCanonicalizesAndClones(t *testing.T) { @@ -135,6 +136,35 @@ func TestNormalizerContractAndDeterministicWarnings(t *testing.T) { } } +func TestNormalizeBoundsWarnings(t *testing.T) { + count := diagnostics.MaxWarnings + 5 + doc := &source.SourceDocument{ID: "session", Units: make([]source.SourceUnit, count)} + input := dnd.NPCInteractionList{Interactions: make([]dnd.NPCInteraction, count)} + for index := range doc.Units { + doc.Units[index].ID = index + 1 + unitID := count - index + input.Interactions[index] = interaction( + "Ária", + dnd.NPCInteractionKindDialogue, + source.SourceRef{SourceID: doc.ID, StartUnitID: unitID, EndUnitID: unitID}, + ) + } + normalizer, err := New(Options{}, npcReferences(t)) + if err != nil { + t.Fatal(err) + } + result, err := normalizer.Normalize(context.Background(), contracts.TypedNormalizeRequest[dnd.NPCInteractionList]{ + Source: doc, MergeOutput: contracts.MergeArtifact[dnd.NPCInteractionList]{Value: input}, + }) + if err != nil { + t.Fatal(err) + } + if len(result.Warnings) != diagnostics.MaxWarnings || + result.Warnings[len(result.Warnings)-1].ReasonCode != ReasonCodeWarningsOmitted { + t.Fatalf("warnings = %#v", result.Warnings) + } +} + func interaction(name string, kind dnd.NPCInteractionKind, ref source.SourceRef) dnd.NPCInteraction { return dnd.NPCInteraction{Name: name, Kind: kind, SourceRefs: []source.SourceRef{ref}} } diff --git a/internal/modules/dnd/npcinteractions/canonical.go b/internal/modules/dnd/npcinteractions/canonical.go new file mode 100644 index 0000000..66883ea --- /dev/null +++ b/internal/modules/dnd/npcinteractions/canonical.go @@ -0,0 +1,166 @@ +// Package npcinteractions owns canonical ordering and exact-identity rules for +// D&D NPC interaction artifacts. +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" +) + +// 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 { + if (left == nil) != (right == nil) || len(left) != len(right) { + return false + } + for index := range left { + if left[index] != right[index] { + return false + } + } + 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) + if leftHasEvidence != rightHasEvidence { + return leftHasEvidence + } + if leftHasEvidence && leftPosition != rightPosition { + return leftPosition < rightPosition + } + leftKey := identity.ComparisonKey(left.Name) + rightKey := identity.ComparisonKey(right.Name) + if leftKey != rightKey { + return leftKey < rightKey + } + if left.Name != right.Name { + return left.Name < right.Name + } + 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 +} + +// ValidSourceRefs reports whether an interaction has non-empty, valid +// current-document evidence. +func ValidSourceRefs(doc *source.SourceDocument, refs []source.SourceRef) bool { + if len(refs) == 0 { + return false + } + for _, ref := range refs { + if source.ValidateRef(doc, ref) != nil { + return false + } + } + return true +} + +// ExactIdentity returns a collision-safe key over every durable interaction +// field. Callers decide whether the record is eligible for duplicate handling. +func ExactIdentity(interaction dnd.NPCInteraction) string { + var key strings.Builder + writeKeyString(&key, interaction.Name) + writeKeyString(&key, string(interaction.Kind)) + for _, ref := range interaction.SourceRefs { + writeKeyString(&key, ref.SourceID) + writeKeyInt(&key, ref.StartUnitID) + writeKeyInt(&key, ref.EndUnitID) + } + return key.String() +} + +func sourceRefsLess(doc *source.SourceDocument, 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 len(left) < len(right) +} + +func writeKeyString(builder *strings.Builder, value string) { + builder.WriteString(strconv.Itoa(len(value))) + builder.WriteByte(':') + builder.WriteString(value) +} + +func writeKeyInt(builder *strings.Builder, value int) { + builder.WriteString(strconv.Itoa(value)) + builder.WriteByte(';') +} diff --git a/internal/modules/dnd/shared/diagnostics/diagnostics.go b/internal/modules/dnd/shared/diagnostics/diagnostics.go index e9f616b..4bc81a9 100644 --- a/internal/modules/dnd/shared/diagnostics/diagnostics.go +++ b/internal/modules/dnd/shared/diagnostics/diagnostics.go @@ -6,10 +6,13 @@ import ( "fmt" "strconv" "strings" + + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" ) const ( MaxIssues = 20 + MaxWarnings = 20 MaxDisplayedRunes = 128 MaxMessageBytes = 4096 ) @@ -37,6 +40,28 @@ func Aggregate(prefix string, issues []string) string { return aggregateMessage(prefix, displayed, len(issues)-len(displayed)) } +// LimitWarnings returns at most MaxWarnings warnings, reserving the final +// position for a deterministic omission summary when truncation is required. +func LimitWarnings(warnings []contracts.Warning, scope, reasonCode string) []contracts.Warning { + if warnings == nil { + return nil + } + if len(warnings) <= MaxWarnings { + bounded := make([]contracts.Warning, len(warnings)) + copy(bounded, warnings) + return bounded + } + displayed := MaxWarnings - 1 + bounded := make([]contracts.Warning, displayed, MaxWarnings) + copy(bounded, warnings[:displayed]) + bounded = append(bounded, contracts.Warning{ + Scope: scope, + ReasonCode: reasonCode, + Message: fmt.Sprintf("%d additional warning(s) omitted", len(warnings)-displayed), + }) + return bounded +} + func aggregateMessage(prefix string, issues []string, omitted int) string { message := prefix + ": " + strings.Join(issues, ", ") if omitted > 0 { diff --git a/internal/modules/dnd/shared/diagnostics/diagnostics_test.go b/internal/modules/dnd/shared/diagnostics/diagnostics_test.go index 23214eb..fdecfa7 100644 --- a/internal/modules/dnd/shared/diagnostics/diagnostics_test.go +++ b/internal/modules/dnd/shared/diagnostics/diagnostics_test.go @@ -2,9 +2,12 @@ package diagnostics import ( "fmt" + "reflect" "strings" "testing" "unicode/utf8" + + "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" ) func TestAggregateEnforcesByteBudgetAndReportsOmissions(t *testing.T) { @@ -26,3 +29,25 @@ func TestAggregateEnforcesByteBudgetAndReportsOmissions(t *testing.T) { t.Fatalf("Aggregate() = %q, want %q", message, wantOmitted) } } + +func TestLimitWarningsBoundsOutputAndReportsOmissions(t *testing.T) { + warnings := make([]contracts.Warning, MaxWarnings+3) + for index := range warnings { + warnings[index] = contracts.Warning{ReasonCode: fmt.Sprintf("warning-%d", index)} + } + before := append([]contracts.Warning(nil), warnings...) + + got := LimitWarnings(warnings, "records", "warnings_omitted") + if len(got) != MaxWarnings { + t.Fatalf("LimitWarnings() count = %d, want %d", len(got), MaxWarnings) + } + summary := got[len(got)-1] + wantOmitted := len(warnings) - (MaxWarnings - 1) + if summary.Scope != "records" || summary.ReasonCode != "warnings_omitted" || + summary.Message != fmt.Sprintf("%d additional warning(s) omitted", wantOmitted) { + t.Fatalf("summary = %#v", summary) + } + if !reflect.DeepEqual(warnings, before) { + t.Fatal("LimitWarnings() mutated its input") + } +} diff --git a/internal/modules/dnd/validate/npcinteractions/invariants/validator.go b/internal/modules/dnd/validate/npcinteractions/invariants/validator.go index a11f3da..36114e1 100644 --- a/internal/modules/dnd/validate/npcinteractions/invariants/validator.go +++ b/internal/modules/dnd/validate/npcinteractions/invariants/validator.go @@ -5,14 +5,12 @@ import ( "context" "fmt" "sort" - "strconv" - "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/npcs/identity" + 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/diagnostics" interactionshape "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/validate/npcinteractions/shape" @@ -78,7 +76,7 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.NPCInteractionList]) (contracts.ValidationResult, error) { - if interactionshape.Validate(req.Value) != nil || !sourceRefsValid(req.Source, req.Value) { + if interactionshape.Validate(req.Value) != nil || !allSourceRefsValid(req.Source, req.Value) { return contracts.ValidationResult{Approved: true}, nil } if v == nil || v.npcResolver == nil { @@ -102,12 +100,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq }, nil } -func sourceRefsValid(doc *source.SourceDocument, value dnd.NPCInteractionList) bool { +func allSourceRefsValid(doc *source.SourceDocument, value dnd.NPCInteractionList) bool { for _, interaction := range value.Interactions { - for _, ref := range interaction.SourceRefs { - if source.ValidateRef(doc, ref) != nil { - return false - } + if !interactionmodel.ValidSourceRefs(doc, interaction.SourceRefs) { + return false } } return true @@ -123,7 +119,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 sourceRefLess(doc, current, previous) { + if interactionmodel.SourceRefLess(doc, 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)) @@ -132,13 +128,13 @@ func issuesFor(doc *source.SourceDocument, value dnd.NPCInteractionList, npcRegi } if !sort.SliceIsSorted(value.Interactions, func(left, right int) bool { - return interactionLess(doc, value.Interactions[left], value.Interactions[right]) + return interactionmodel.Less(doc, value.Interactions[left], value.Interactions[right]) }) { issues = append(issues, "interactions are not in canonical order") } seen := make(map[string]int) for index, interaction := range value.Interactions { - key := duplicateKey(interaction) + key := interactionmodel.ExactIdentity(interaction) if previous, ok := seen[key]; ok { issues = append(issues, fmt.Sprintf("interactions[%d] duplicates interaction %d", index, previous)) continue @@ -148,105 +144,6 @@ func issuesFor(doc *source.SourceDocument, value dnd.NPCInteractionList, npcRegi return issues } -func interactionLess(doc *source.SourceDocument, left, right dnd.NPCInteraction) bool { - leftPosition, leftHasEvidence := earliestSourcePosition(doc, left) - rightPosition, rightHasEvidence := earliestSourcePosition(doc, right) - if leftHasEvidence != rightHasEvidence { - return leftHasEvidence - } - if leftHasEvidence && leftPosition != rightPosition { - return leftPosition < rightPosition - } - leftKey := identity.ComparisonKey(left.Name) - rightKey := identity.ComparisonKey(right.Name) - if leftKey != rightKey { - return leftKey < rightKey - } - if left.Name != right.Name { - return left.Name < right.Name - } - if left.Kind != right.Kind { - return left.Kind < right.Kind - } - return sourceRefsLess(doc, left.SourceRefs, right.SourceRefs) -} - -func sourceRefsLess(doc *source.SourceDocument, 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 len(left) < len(right) -} - -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 -} - -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 -} - -func duplicateKey(interaction dnd.NPCInteraction) string { - var key strings.Builder - writeKeyString(&key, interaction.Name) - writeKeyString(&key, string(interaction.Kind)) - for _, ref := range interaction.SourceRefs { - writeKeyString(&key, ref.SourceID) - writeKeyInt(&key, ref.StartUnitID) - writeKeyInt(&key, ref.EndUnitID) - } - return key.String() -} - -func writeKeyString(builder *strings.Builder, value string) { - builder.WriteString(strconv.Itoa(len(value))) - builder.WriteByte(':') - builder.WriteString(value) -} - -func writeKeyInt(builder *strings.Builder, value int) { - builder.WriteString(strconv.Itoa(value)) - builder.WriteByte(';') -} - func Spec() pipeline.ValidatorSpec { return pipeline.ValidatorSpec{Key: Key, ExecutionClass: contracts.ExecutionClassDeterministic} } diff --git a/internal/modules/dnd/validate/npcinteractions/source_refs/validator.go b/internal/modules/dnd/validate/npcinteractions/source_refs/validator.go index 7c3c3e9..c3486c3 100644 --- a/internal/modules/dnd/validate/npcinteractions/source_refs/validator.go +++ b/internal/modules/dnd/validate/npcinteractions/source_refs/validator.go @@ -16,7 +16,7 @@ import ( const ( Key = "extract/dnd/npc-interactions/source_refs" ReasonCode = "invalid_npc_interaction_source_refs" - policy = "dnd.npc_interactions.validator.source_refs.v1" + policy = "dnd.npc_interactions.validator.source_refs.v2" ) type Options struct{} @@ -35,6 +35,9 @@ func (v *Validator) CheckpointFingerprints() []pipeline.CheckpointFingerprint { } func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationRequest[dnd.NPCInteractionList]) (contracts.ValidationResult, error) { + if req.Stage == string(pipeline.StageExtract) && req.Chunk == nil { + return contracts.ValidationResult{}, fmt.Errorf("NPC interaction source-reference validator requires the current extraction chunk") + } if interactionshape.Validate(req.Value) != nil { return contracts.ValidationResult{Approved: true}, nil } @@ -43,6 +46,13 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq for refIndex, ref := range interaction.SourceRefs { if err := source.ValidateRef(req.Source, ref); err != nil { issues = append(issues, fmt.Sprintf("interactions[%d].source_refs[%d]: %s", interactionIndex, refIndex, diagnostics.Truncate(err.Error()))) + continue + } + if req.Stage == string(pipeline.StageExtract) && !chunkContainsRef(req.Chunk, ref) { + issues = append(issues, fmt.Sprintf( + "interactions[%d].source_refs[%d]: source reference is outside the current extraction chunk", + interactionIndex, refIndex, + )) } } } @@ -56,6 +66,19 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq }, 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/npcinteractions/source_refs/validator_test.go b/internal/modules/dnd/validate/npcinteractions/source_refs/validator_test.go index f5c2792..6851826 100644 --- a/internal/modules/dnd/validate/npcinteractions/source_refs/validator_test.go +++ b/internal/modules/dnd/validate/npcinteractions/source_refs/validator_test.go @@ -29,6 +29,49 @@ func TestValidatorOwnsCurrentSourceUnitAndRangeValidation(t *testing.T) { } } +func TestValidatorRejectsDocumentValidEvidenceOutsideCurrentExtractionChunk(t *testing.T) { + doc := document() + chunk := &source.Chunk{ + ID: "chunk-0", + SourceID: doc.ID, + Units: append([]source.SourceUnit(nil), doc.Units[:2]...), + } + value := validList() + result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCInteractionList]{ + Stage: string(pipeline.StageExtract), + Source: doc, + Chunk: chunk, + Value: value, + }) + if err != nil || !result.Approved { + t.Fatalf("contained evidence = %#v, %v", result, err) + } + + value.Interactions[0].SourceRefs = []source.SourceRef{{ + SourceID: doc.ID, StartUnitID: 2, EndUnitID: 3, + }} + result, err = New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCInteractionList]{ + 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("out-of-chunk evidence = %#v, %v", result, err) + } +} + +func TestValidatorRequiresChunkDuringExtractValidation(t *testing.T) { + _, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCInteractionList]{ + Stage: string(pipeline.StageExtract), + Source: document(), + Value: validList(), + }) + if err == nil || !strings.Contains(err.Error(), "requires the current extraction chunk") { + t.Fatalf("Validate() error = %v", err) + } +} + func TestValidatorDefersShapeAndDoesNotMutate(t *testing.T) { malformed := dnd.NPCInteractionList{Interactions: []dnd.NPCInteraction{{Name: "Mira Thorn"}}} result, err := New(Options{}).Validate(context.Background(), request(document(), malformed)) @@ -55,7 +98,7 @@ func request(doc *source.SourceDocument, value dnd.NPCInteractionList) contracts } func document() *source.SourceDocument { - return &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 1}, {ID: 2}}} + return &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 1}, {ID: 2}, {ID: 3}}} } func validList() dnd.NPCInteractionList { diff --git a/internal/modules/dnd/validate/npcinteractions/source_relatedness/validator.go b/internal/modules/dnd/validate/npcinteractions/source_relatedness/validator.go index f5a0886..02d6fdf 100644 --- a/internal/modules/dnd/validate/npcinteractions/source_relatedness/validator.go +++ b/internal/modules/dnd/validate/npcinteractions/source_relatedness/validator.go @@ -16,7 +16,8 @@ import ( const ( Key = "extract/dnd/npc-interactions/source_relatedness" WarningReasonCode = "npc_interaction_not_near_source" - policy = "dnd.npc_interactions.validator.source_relatedness.v1" + OmittedReasonCode = "npc_interaction_relatedness_warnings_omitted" + policy = "dnd.npc_interactions.validator.source_relatedness.v2" ) type Options struct{} @@ -57,7 +58,10 @@ func (v *Validator) Validate(_ context.Context, req contracts.TypedValidationReq Message: fmt.Sprintf("NPC interaction name %s was not found in cited source text", diagnostics.Quote(interaction.Name)), }) } - return contracts.ValidationResult{Approved: true, Warnings: warnings}, nil + return contracts.ValidationResult{ + Approved: true, + Warnings: diagnostics.LimitWarnings(warnings, "npc_interactions", OmittedReasonCode), + }, nil } func Spec() pipeline.ValidatorSpec { diff --git a/internal/modules/dnd/validate/npcinteractions/source_relatedness/validator_test.go b/internal/modules/dnd/validate/npcinteractions/source_relatedness/validator_test.go index e410de0..9eb812f 100644 --- a/internal/modules/dnd/validate/npcinteractions/source_relatedness/validator_test.go +++ b/internal/modules/dnd/validate/npcinteractions/source_relatedness/validator_test.go @@ -10,6 +10,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/diagnostics" ) func TestValidatorUsesOnlyCurrentTranscriptAndWarnsOncePerInteraction(t *testing.T) { @@ -49,3 +50,26 @@ func TestValidatorDefersMalformedShapeAndInvalidRanges(t *testing.T) { t.Fatal(err) } } + +func TestValidatorBoundsWarnings(t *testing.T) { + count := diagnostics.MaxWarnings + 5 + interactions := make([]dnd.NPCInteraction, count) + for index := range interactions { + interactions[index] = dnd.NPCInteraction{ + Name: "Missing NPC", + Kind: dnd.NPCInteractionKindMentioned, + SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: 1, EndUnitID: 1}}, + } + } + result, err := New(Options{}).Validate(context.Background(), contracts.TypedValidationRequest[dnd.NPCInteractionList]{ + Source: &source.SourceDocument{ID: "session", Units: []source.SourceUnit{{ID: 1, Text: "The party waits."}}}, + Value: dnd.NPCInteractionList{Interactions: interactions}, + }) + if err != nil || !result.Approved { + t.Fatalf("Validate() = %#v, %v", result, err) + } + if len(result.Warnings) != diagnostics.MaxWarnings || + result.Warnings[len(result.Warnings)-1].ReasonCode != OmittedReasonCode { + t.Fatalf("warnings = %#v", result.Warnings) + } +}