From d28d1062e01671ea44e802663d423c749e37dab9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 9 Aug 2026 02:36:53 +0000 Subject: [PATCH] Reuse canonical item occurrence evidence --- .../dnd/itemoccurrences/itemoccurrences.go | 48 ++++++++++++++++--- .../itemoccurrences/itemoccurrences_test.go | 14 ++++++ .../normalize/itemoccurrences/normalizer.go | 32 ++++++++----- .../normalize/npcoccurrences/normalizer.go | 14 +----- .../npcoccurrences/normalizer_test.go | 2 +- .../itemoccurrences/invariants/validator.go | 26 +++++++--- 6 files changed, 97 insertions(+), 39 deletions(-) diff --git a/internal/modules/dnd/itemoccurrences/itemoccurrences.go b/internal/modules/dnd/itemoccurrences/itemoccurrences.go index 68c0fef..342c208 100644 --- a/internal/modules/dnd/itemoccurrences/itemoccurrences.go +++ b/internal/modules/dnd/itemoccurrences/itemoccurrences.go @@ -93,6 +93,14 @@ func ValidSourceRefs(index source.DocumentIndex, refs []source.SourceRef) bool { // comparable through SourceRefOrder's literal fallback so malformed candidates // are still safe to sort and diagnose. func Less(order shared.SourceRefOrder, left, right dnd.ItemOccurrence) bool { + left.SourceRefs = order.Canonicalize(left.SourceRefs) + right.SourceRefs = order.Canonicalize(right.SourceRefs) + return CanonicalLess(order, left, right) +} + +// CanonicalLess defines the canonical occurrence order without modifying or +// canonicalizing source references. Callers must provide canonical evidence. +func CanonicalLess(order shared.SourceRefOrder, left, right dnd.ItemOccurrence) bool { leftPosition, leftHasEvidence := order.EarliestValid(left.SourceRefs) rightPosition, rightHasEvidence := order.EarliestValid(right.SourceRefs) if leftHasEvidence != rightHasEvidence { @@ -122,12 +130,20 @@ func Less(order shared.SourceRefOrder, left, right dnd.ItemOccurrence) bool { if less, decided := optionalQuantityLess(left.Quantity, right.Quantity); decided { return less } - return sourceRefsLess(order, order.Canonicalize(left.SourceRefs), order.Canonicalize(right.SourceRefs)) + return sourceRefsLess(order, left.SourceRefs, right.SourceRefs) } // ExactEqual reports whether occurrences are exact duplicates after their display // fields and evidence have been canonicalized for the supplied source order. func ExactEqual(order shared.SourceRefOrder, left, right dnd.ItemOccurrence) bool { + left.SourceRefs = order.Canonicalize(left.SourceRefs) + right.SourceRefs = order.Canonicalize(right.SourceRefs) + return CanonicalExactEqual(left, right) +} + +// CanonicalExactEqual reports whether canonical occurrences are exact +// duplicates without modifying or canonicalizing their source references. +func CanonicalExactEqual(left, right dnd.ItemOccurrence) bool { if left.ItemID != right.ItemID || DisplayValue(left.Name) != DisplayValue(right.Name) || left.Kind != right.Kind || DisplayValue(left.From) != DisplayValue(right.From) || DisplayValue(left.To) != DisplayValue(right.To) || (left.Quantity == nil) != (right.Quantity == nil) { @@ -136,13 +152,21 @@ func ExactEqual(order shared.SourceRefOrder, left, right dnd.ItemOccurrence) boo if left.Quantity != nil && *left.Quantity != *right.Quantity { return false } - return SourceRefsEqual(order.Canonicalize(left.SourceRefs), order.Canonicalize(right.SourceRefs)) + return SourceRefsEqual(left.SourceRefs, right.SourceRefs) } // ExactIdentity returns a collision-safe duplicate key after display and // evidence canonicalization. It is intended for callers that have already // decided the occurrence is eligible for duplicate handling. func ExactIdentity(order shared.SourceRefOrder, occurrence dnd.ItemOccurrence) string { + occurrence.SourceRefs = order.Canonicalize(occurrence.SourceRefs) + return CanonicalExactIdentity(occurrence) +} + +// CanonicalExactIdentity returns a collision-safe duplicate key without +// modifying or canonicalizing source references. Callers must provide +// canonical evidence. +func CanonicalExactIdentity(occurrence dnd.ItemOccurrence) string { var key strings.Builder writeKeyString(&key, occurrence.ItemID) writeKeyString(&key, DisplayValue(occurrence.Name)) @@ -155,14 +179,24 @@ func ExactIdentity(order shared.SourceRefOrder, occurrence dnd.ItemOccurrence) s key.WriteByte('1') writeKeyInt(&key, *occurrence.Quantity) } - for _, ref := range order.Canonicalize(occurrence.SourceRefs) { - writeKeyString(&key, ref.SourceID) - writeKeyInt(&key, ref.StartUnitID) - writeKeyInt(&key, ref.EndUnitID) - } + writeKeySourceRefs(&key, occurrence.SourceRefs) return key.String() } +func writeKeySourceRefs(builder *strings.Builder, refs []source.SourceRef) { + if refs == nil { + builder.WriteByte('0') + return + } + builder.WriteByte('1') + writeKeyInt(builder, len(refs)) + for _, ref := range refs { + writeKeyString(builder, ref.SourceID) + writeKeyInt(builder, ref.StartUnitID) + writeKeyInt(builder, ref.EndUnitID) + } +} + func optionalStringLess(left, right string) (bool, bool) { leftPresent, rightPresent := HolderPresent(left), HolderPresent(right) if leftPresent != rightPresent { diff --git a/internal/modules/dnd/itemoccurrences/itemoccurrences_test.go b/internal/modules/dnd/itemoccurrences/itemoccurrences_test.go index 9885055..207f483 100644 --- a/internal/modules/dnd/itemoccurrences/itemoccurrences_test.go +++ b/internal/modules/dnd/itemoccurrences/itemoccurrences_test.go @@ -108,6 +108,14 @@ func TestLessAndExactEqualityCanonicalizeEvidence(t *testing.T) { if ExactIdentity(order, first) != ExactIdentity(order, second) { t.Fatal("ExactIdentity() differs for canonical duplicates") } + canonicalFirst := first + canonicalFirst.SourceRefs = order.Canonicalize(first.SourceRefs) + if !CanonicalExactEqual(canonicalFirst, second) || CanonicalExactIdentity(canonicalFirst) != CanonicalExactIdentity(second) { + t.Fatal("canonical helpers did not recognize canonical duplicates") + } + if CanonicalExactEqual(first, second) || CanonicalExactIdentity(first) == CanonicalExactIdentity(second) { + t.Fatal("canonical helpers repaired noncanonical source references") + } quantity := 1 differentQuantity := second @@ -132,6 +140,12 @@ func TestSourceReferenceHelpers(t *testing.T) { if ValidSourceRefs(source.NewDocumentIndex(doc), []source.SourceRef{{SourceID: "other", StartUnitID: 10, EndUnitID: 20}}) { t.Fatal("ValidSourceRefs() = true, want invalid source identifier rejection") } + nilEvidence := dnd.ItemOccurrence{ItemID: "item"} + emptyEvidence := nilEvidence + emptyEvidence.SourceRefs = []source.SourceRef{} + if CanonicalExactIdentity(nilEvidence) == CanonicalExactIdentity(emptyEvidence) { + t.Fatal("CanonicalExactIdentity() conflated nil and empty source references") + } } func TestLessSortsMalformedReferencesDeterministically(t *testing.T) { diff --git a/internal/modules/dnd/normalize/itemoccurrences/normalizer.go b/internal/modules/dnd/normalize/itemoccurrences/normalizer.go index 54eb9c7..76f0d8c 100644 --- a/internal/modules/dnd/normalize/itemoccurrences/normalizer.go +++ b/internal/modules/dnd/normalize/itemoccurrences/normalizer.go @@ -113,6 +113,7 @@ func (n *Normalizer) Normalize(ctx context.Context, req contracts.TypedNormalize type normalizedRecord struct { occurrence dnd.ItemOccurrence + identity string inputIndex int } @@ -125,7 +126,7 @@ func normalizeList(input dnd.ItemOccurrenceList, index source.DocumentIndex, ord warnings := make([]contracts.Warning, 0) for index, inputOccurrence := range input.Occurrences { occurrence, changedFields, found, refsChanged := normalizeOccurrence(inputOccurrence, order, registry) - records[index] = normalizedRecord{occurrence: occurrence, inputIndex: index} + records[index] = normalizedRecord{occurrence: occurrence, identity: itemoccurrencemodel.CanonicalExactIdentity(occurrence), inputIndex: index} if len(changedFields) != 0 { warnings = append(warnings, contracts.Warning{ Scope: occurrenceScope(index), @@ -153,7 +154,7 @@ func normalizeList(input dnd.ItemOccurrenceList, index source.DocumentIndex, ord } sort.SliceStable(records, func(left, right int) bool { - return itemoccurrencemodel.Less(order, records[left].occurrence, records[right].occurrence) + return itemoccurrencemodel.CanonicalLess(order, records[left].occurrence, records[right].occurrence) }) for position, record := range records { if position == record.inputIndex { @@ -166,7 +167,7 @@ func normalizeList(input dnd.ItemOccurrenceList, index source.DocumentIndex, ord }) } - output, duplicateWarnings := collapseDuplicates(records, index, order) + output, duplicateWarnings := collapseDuplicates(records, index) warnings = append(warnings, duplicateWarnings...) return dnd.ItemOccurrenceList{Occurrences: output}, diagnostics.LimitWarnings(warnings, "item_occurrences", ReasonCodeWarningsOmitted) } @@ -208,28 +209,35 @@ func cloneOccurrence(input dnd.ItemOccurrence) dnd.ItemOccurrence { } type duplicateGroup struct { - retainedIndex int - removed []int + retainedIndex int + retainedRecord int + removed []int } -func collapseDuplicates(records []normalizedRecord, index source.DocumentIndex, order shared.SourceRefOrder) ([]dnd.ItemOccurrence, []contracts.Warning) { +func collapseDuplicates(records []normalizedRecord, index source.DocumentIndex) ([]dnd.ItemOccurrence, []contracts.Warning) { if len(records) == 0 { return make([]dnd.ItemOccurrence, 0), nil } keep := make([]bool, len(records)) groups := make([]duplicateGroup, 0) - groupByKey := make(map[string]int) + groupByKey := make(map[string][]int) for recordIndex, record := range records { if !itemoccurrencemodel.ValidSourceRefs(index, record.occurrence.SourceRefs) { keep[recordIndex] = true continue } - key := itemoccurrencemodel.ExactIdentity(order, record.occurrence) - groupIndex, exists := groupByKey[key] - if !exists { - groupByKey[key] = len(groups) - groups = append(groups, duplicateGroup{retainedIndex: record.inputIndex}) + groupIndex := -1 + for _, candidate := range groupByKey[record.identity] { + if itemoccurrencemodel.CanonicalExactEqual(records[groups[candidate].retainedRecord].occurrence, record.occurrence) { + groupIndex = candidate + break + } + } + if groupIndex < 0 { + groupIndex = len(groups) + groupByKey[record.identity] = append(groupByKey[record.identity], groupIndex) + groups = append(groups, duplicateGroup{retainedIndex: record.inputIndex, retainedRecord: recordIndex}) keep[recordIndex] = true continue } diff --git a/internal/modules/dnd/normalize/npcoccurrences/normalizer.go b/internal/modules/dnd/normalize/npcoccurrences/normalizer.go index 45aceee..80a5efc 100644 --- a/internal/modules/dnd/normalize/npcoccurrences/normalizer.go +++ b/internal/modules/dnd/normalize/npcoccurrences/normalizer.go @@ -35,13 +35,6 @@ const ( var requiredCapabilities = []string{"merged"} var providedCapabilities = []string{"normalized"} -var referenceSlotDescriptions = shared.ReferenceSlotDescriptions{ - Glossary: "Optional campaign glossary reference material used only for occurrence disambiguation.", - Party: "Optional party roster reference material used only for occurrence disambiguation.", - Players: "Optional player list reference material used only for occurrence disambiguation.", - Roster: "Deprecated alias for party roster reference material used only for occurrence disambiguation.", -} - var _ contracts.Normalizer[dnd.NPCOccurrenceList] = (*Normalizer)(nil) var _ contracts.ManifestMetadataProvider = (*Normalizer)(nil) var _ pipeline.CheckpointFingerprintProvider = (*Normalizer)(nil) @@ -250,17 +243,14 @@ func duplicateWarning(retainedIndex int, removed []int) contracts.Warning { func occurrenceScope(index int) string { return fmt.Sprintf("occurrences[%d]", index) } func referenceSlots() []contracts.ReferenceSlot { - slots := shared.ReferenceSlots(referenceSlotDescriptions) - slots = append(slots, contracts.ReferenceSlot{ + return []contracts.ReferenceSlot{{ Name: NPCRegistryReferenceSlot, Description: "Required normalized NPC registry used only for occurrence identity grounding, never as occurrence evidence.", Required: true, AcceptedMediaTypes: []string{"application/json"}, AcceptedArtifactKinds: []contracts.ArtifactKind{dnd.NPCRegistryKind}, MaxBytes: NPCRegistryMaxBytes, - }) - sort.Slice(slots, func(left, right int) bool { return slots[left].Name < slots[right].Name }) - return slots + }} } func ModuleSpec() pipeline.ModuleSpec { diff --git a/internal/modules/dnd/normalize/npcoccurrences/normalizer_test.go b/internal/modules/dnd/normalize/npcoccurrences/normalizer_test.go index 9a39a43..771d79f 100644 --- a/internal/modules/dnd/normalize/npcoccurrences/normalizer_test.go +++ b/internal/modules/dnd/normalize/npcoccurrences/normalizer_test.go @@ -126,7 +126,7 @@ func TestNormalizerContractAndDeterministicWarnings(t *testing.T) { if err != nil { t.Fatal(err) } - if spec := ModuleSpec(); spec.Key != Key || spec.Stage != pipeline.StageNormalize || spec.ArtifactKind != dnd.NPCOccurrenceListKind || len(spec.ReferenceSlots) == 0 { + if spec := ModuleSpec(); spec.Key != Key || spec.Stage != pipeline.StageNormalize || spec.ArtifactKind != dnd.NPCOccurrenceListKind || len(spec.ReferenceSlots) != 1 || spec.ReferenceSlots[0].Name != NPCRegistryReferenceSlot || !spec.ReferenceSlots[0].Required { t.Fatalf("ModuleSpec() = %#v", spec) } if metadata := normalizer.ManifestMetadata(); metadata["normalization_policy"] != normalizationPolicy || metadata["npc_registry_digest"] == "" || metadata["npc_count"] != 2 { diff --git a/internal/modules/dnd/validate/itemoccurrences/invariants/validator.go b/internal/modules/dnd/validate/itemoccurrences/invariants/validator.go index c16d244..c601830 100644 --- a/internal/modules/dnd/validate/itemoccurrences/invariants/validator.go +++ b/internal/modules/dnd/validate/itemoccurrences/invariants/validator.go @@ -62,9 +62,11 @@ func Validate(doc *source.SourceDocument, value dnd.ItemOccurrenceList) error { func issuesFor(order shared.SourceRefOrder, value dnd.ItemOccurrenceList) []string { issues := make([]string, 0) - seenIdentity := make(map[string]int) + canonicalEvidence := make([]bool, len(value.Occurrences)) + seenIdentity := make(map[string][]int) for occurrenceIndex, occurrence := range value.Occurrences { prefix := fmt.Sprintf("occurrences[%d]", occurrenceIndex) + canonicalEvidence[occurrenceIndex] = true if occurrence.Name != itemoccurrencemodel.DisplayValue(occurrence.Name) { issues = append(issues, prefix+".name is not display-normalized") } @@ -79,19 +81,29 @@ func issuesFor(order shared.SourceRefOrder, value dnd.ItemOccurrenceList) []stri current := occurrence.SourceRefs[refIndex] if order.Less(current, previous) { issues = append(issues, fmt.Sprintf("%s.source_refs are not in canonical order at index %d", prefix, refIndex)) + canonicalEvidence[occurrenceIndex] = false } else if current == previous { issues = append(issues, fmt.Sprintf("%s.source_refs[%d] duplicates the previous reference", prefix, refIndex)) + canonicalEvidence[occurrenceIndex] = false } } - key := itemoccurrencemodel.ExactIdentity(order, occurrence) - if previous, exists := seenIdentity[key]; exists { - issues = append(issues, fmt.Sprintf("%s duplicates item occurrence %d under normalized identity", prefix, previous)) - } else { - seenIdentity[key] = occurrenceIndex + if canonicalEvidence[occurrenceIndex] { + key := itemoccurrencemodel.CanonicalExactIdentity(occurrence) + for _, previous := range seenIdentity[key] { + if itemoccurrencemodel.CanonicalExactEqual(value.Occurrences[previous], occurrence) { + issues = append(issues, fmt.Sprintf("%s duplicates item occurrence %d under normalized identity", prefix, previous)) + break + } + } + seenIdentity[key] = append(seenIdentity[key], occurrenceIndex) } } for occurrenceIndex := 1; occurrenceIndex < len(value.Occurrences); occurrenceIndex++ { - if itemoccurrencemodel.Less(order, value.Occurrences[occurrenceIndex], value.Occurrences[occurrenceIndex-1]) { + less := itemoccurrencemodel.Less + if canonicalEvidence[occurrenceIndex] && canonicalEvidence[occurrenceIndex-1] { + less = itemoccurrencemodel.CanonicalLess + } + if less(order, value.Occurrences[occurrenceIndex], value.Occurrences[occurrenceIndex-1]) { issues = append(issues, fmt.Sprintf("occurrences[%d] is out of canonical order", occurrenceIndex)) } }