From b70d9f77e37f88ad7fcdcff1b4e1b48a6e8864cb Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 9 Aug 2026 02:26:49 +0000 Subject: [PATCH] Improve D&D registry normalization efficiency --- .../dnd/locations/identity/identity.go | 56 +++++++++---------- .../dnd/normalize/itemregistry/normalizer.go | 17 ++---- .../normalize/itemregistry/normalizer_test.go | 14 +++++ .../normalize/locationregistry/normalizer.go | 37 +++++++++--- .../locationregistry/normalizer_test.go | 18 ++++++ 5 files changed, 93 insertions(+), 49 deletions(-) diff --git a/internal/modules/dnd/locations/identity/identity.go b/internal/modules/dnd/locations/identity/identity.go index 83aa4c6..2ce687b 100644 --- a/internal/modules/dnd/locations/identity/identity.go +++ b/internal/modules/dnd/locations/identity/identity.go @@ -5,7 +5,6 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" - "sort" "strings" "gitea.maximumdirect.net/eric/notarius/internal/core/source" @@ -58,9 +57,13 @@ func ComparisonKey(value string) string { // available, leaving validation to report the problem instead of manufacturing // an ID. func DeriveID(name string, refs []source.SourceRef) string { - comparisonName := ComparisonKey(name) anchor, ok := earliestReference(refs) - if comparisonName == "" || !ok { + return deriveIDFromAnchor(name, anchor, ok) +} + +func deriveIDFromAnchor(name string, anchor source.SourceRef, hasAnchor bool) string { + comparisonName := ComparisonKey(name) + if comparisonName == "" || !hasAnchor { return "" } @@ -111,13 +114,14 @@ func ValidateRegistry(locations []dnd.Location) []Issue { if ComparisonKey(location.Name) == "" { issues = append(issues, Issue{Code: IssueEmptyCanonicalName, RecordIndex: recordIndex, Value: location.Name}) } - if _, ok := earliestReference(location.SourceRefs); !ok { + anchor, hasAnchor := earliestReference(location.SourceRefs) + if !hasAnchor { issues = append(issues, Issue{Code: IssueMissingEvidence, RecordIndex: recordIndex}) } if !IsValidID(location.ID) { issues = append(issues, Issue{Code: IssueInvalidID, RecordIndex: recordIndex, Value: location.ID}) - } else if expected := DeriveID(location.Name, location.SourceRefs); location.ID != expected { + } else if expected := deriveIDFromAnchor(location.Name, anchor, hasAnchor); location.ID != expected { issues = append(issues, Issue{Code: IssueIDMismatch, RecordIndex: recordIndex, Value: location.ID}) } if location.ID != "" { @@ -135,36 +139,28 @@ func ValidateRegistry(locations []dnd.Location) []Issue { } func earliestReference(refs []source.SourceRef) (source.SourceRef, bool) { - canonical := canonicalReferences(refs) - if len(canonical) == 0 { - return source.SourceRef{}, false + var earliest source.SourceRef + found := false + for _, ref := range refs { + if !validIdentityReference(ref) { + continue + } + if !found || referenceLess(ref, earliest) { + earliest = ref + found = true + } } - return canonical[0], true + return earliest, found } -func canonicalReferences(refs []source.SourceRef) []source.SourceRef { - canonical := make([]source.SourceRef, 0, len(refs)) - for _, ref := range refs { - if validIdentityReference(ref) { - canonical = append(canonical, ref) - } +func referenceLess(left, right source.SourceRef) bool { + if left.SourceID != right.SourceID { + return left.SourceID < right.SourceID } - sort.Slice(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 - }) - unique := canonical[:0] - for _, ref := range canonical { - if len(unique) == 0 || unique[len(unique)-1] != ref { - unique = append(unique, ref) - } + if left.StartUnitID != right.StartUnitID { + return left.StartUnitID < right.StartUnitID } - return unique + return left.EndUnitID < right.EndUnitID } func validIdentityReference(ref source.SourceRef) bool { diff --git a/internal/modules/dnd/normalize/itemregistry/normalizer.go b/internal/modules/dnd/normalize/itemregistry/normalizer.go index 8c0192a..e634579 100644 --- a/internal/modules/dnd/normalize/itemregistry/normalizer.go +++ b/internal/modules/dnd/normalize/itemregistry/normalizer.go @@ -241,20 +241,15 @@ func normalizeRecord(input dnd.Item, order shared.SourceRefOrder) (dnd.Item, boo func comparisonNameGroups(records []normalizedRecord) [][]int { groups := make([][]int, 0, len(records)) + groupPositions := make(map[string]int, len(records)) for index, record := range records { key := identity.ComparisonKey(record.item.Name) - found := false - for groupIndex, members := range groups { - first := records[members[0]] - if identity.ComparisonKey(first.item.Name) == key { - groups[groupIndex] = append(groups[groupIndex], index) - found = true - break - } - } - if !found { - groups = append(groups, []int{index}) + if groupIndex, found := groupPositions[key]; found { + groups[groupIndex] = append(groups[groupIndex], index) + continue } + groupPositions[key] = len(groups) + groups = append(groups, []int{index}) } return groups } diff --git a/internal/modules/dnd/normalize/itemregistry/normalizer_test.go b/internal/modules/dnd/normalize/itemregistry/normalizer_test.go index a4272f4..2def3a6 100644 --- a/internal/modules/dnd/normalize/itemregistry/normalizer_test.go +++ b/internal/modules/dnd/normalize/itemregistry/normalizer_test.go @@ -85,6 +85,20 @@ func TestNormalizeConsolidatesEqualNamesAcrossEvidenceWithoutMutation(t *testing } } +func BenchmarkComparisonNameGroupsManyDistinct(b *testing.B) { + records := make([]normalizedRecord, 1_000) + for index := range records { + records[index] = normalizedRecord{item: dnd.Item{Name: "Item " + strconv.Itoa(index)}} + } + b.ReportAllocs() + b.ResetTimer() + for iteration := 0; iteration < b.N; iteration++ { + if groups := comparisonNameGroups(records); len(groups) != len(records) { + b.Fatalf("group count = %d, want %d", len(groups), len(records)) + } + } +} + func TestNormalizeAppliesSafeAliasProposal(t *testing.T) { doc := semanticDocument() input := dnd.ItemRegistry{Items: []dnd.Item{ diff --git a/internal/modules/dnd/normalize/locationregistry/normalizer.go b/internal/modules/dnd/normalize/locationregistry/normalizer.go index ea23bdb..9eb8d41 100644 --- a/internal/modules/dnd/normalize/locationregistry/normalizer.go +++ b/internal/modules/dnd/normalize/locationregistry/normalizer.go @@ -3,6 +3,7 @@ package locationregistry import ( "context" + "encoding/json" "errors" "fmt" "reflect" @@ -228,24 +229,44 @@ func normalizeRecord(input dnd.Location, order shared.SourceRefOrder) (dnd.Locat func exactDuplicateGroups(records []normalizedRecord) [][]int { groups := make([][]int, 0, len(records)) + groupPositions := make(map[string][]int, len(records)) for index, record := range records { - key := identity.ComparisonKey(record.location.Name) - found := false - for groupIndex, members := range groups { - first := records[members[0]] - if identity.ComparisonKey(first.location.Name) == key && reflect.DeepEqual(first.location.SourceRefs, record.location.SourceRefs) { + key := exactDuplicateKey(record.location) + comparisonName := identity.ComparisonKey(record.location.Name) + matched := false + for _, groupIndex := range groupPositions[key] { + first := records[groups[groupIndex][0]] + if identity.ComparisonKey(first.location.Name) == comparisonName && reflect.DeepEqual(first.location.SourceRefs, record.location.SourceRefs) { groups[groupIndex] = append(groups[groupIndex], index) - found = true + matched = true break } } - if !found { - groups = append(groups, []int{index}) + if matched { + continue } + groupPositions[key] = append(groupPositions[key], len(groups)) + groups = append(groups, []int{index}) } return groups } +type duplicateKey struct { + ComparisonName string `json:"comparison_name"` + SourceRefs []source.SourceRef `json:"source_refs"` +} + +func exactDuplicateKey(location dnd.Location) string { + encoded, err := json.Marshal(duplicateKey{ + ComparisonName: identity.ComparisonKey(location.Name), + SourceRefs: location.SourceRefs, + }) + if err != nil { + panic("encode location duplicate key") + } + return string(encoded) +} + func cloneLocation(input dnd.Location) dnd.Location { input.SourceRefs = cloneSourceRefs(input.SourceRefs) return input diff --git a/internal/modules/dnd/normalize/locationregistry/normalizer_test.go b/internal/modules/dnd/normalize/locationregistry/normalizer_test.go index 665862a..4263ccc 100644 --- a/internal/modules/dnd/normalize/locationregistry/normalizer_test.go +++ b/internal/modules/dnd/normalize/locationregistry/normalizer_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "reflect" + "strconv" "strings" "testing" @@ -60,6 +61,23 @@ func TestNormalizePreparesOnlyExactDuplicatesAndRetainsSameNameAndNestedPlaces(t } } +func BenchmarkExactDuplicateGroupsManyDistinct(b *testing.B) { + records := make([]normalizedRecord, 1_000) + for index := range records { + records[index] = normalizedRecord{location: dnd.Location{ + Name: "Location " + strconv.Itoa(index), + SourceRefs: []source.SourceRef{{SourceID: "session", StartUnitID: index + 1, EndUnitID: index + 1}}, + }} + } + b.ReportAllocs() + b.ResetTimer() + for iteration := 0; iteration < b.N; iteration++ { + if groups := exactDuplicateGroups(records); len(groups) != len(records) { + b.Fatalf("group count = %d, want %d", len(groups), len(records)) + } + } +} + func TestNormalizeAppliesSafeAliasGroupAndUsesContextualInputs(t *testing.T) { client := &recordingLocationNormalizerClient{response: `{"duplicate_groups":[{"members":["candidate-000001","candidate-000002"],"canonical":"candidate-000002"}]}`} doc := semanticDocument()