diff --git a/docs/integrations/dnd-item-occurrence-artifacts.md b/docs/integrations/dnd-item-occurrence-artifacts.md index b7325c1..471ec2c 100644 --- a/docs/integrations/dnd-item-occurrence-artifacts.md +++ b/docs/integrations/dnd-item-occurrence-artifacts.md @@ -27,11 +27,13 @@ an earlier normalized `dnd/item-registry` artifact. The registry is immutable for an operation and contributes only its ordered `{id,name}` projection after the shared evidence message. It is never occurrence evidence. -Each occurrence must use one exact registry ID/name pair. Extraction omits an -unknown ID or mismatched name rather than creating an item. Normalization -canonicalizes a recognized name by ID, preserves unknown values for the -registry validator, and the registry validator rejects unknown or mismatched -pairs. +Each occurrence must use one exact registry ID/name pair. An extraction response +with an unknown ID or mismatched name is rejected as invalid model output; the +configured pipeline may retry it and never accepts a partial artifact. +Normalization and validation remain defense in depth for artifacts entering +through other boundaries: normalization canonicalizes a recognized name by ID, +preserves unknown values for the registry validator, and the registry validator +rejects unknown or mismatched pairs. ## Wire shape diff --git a/docs/internal/dnd.md b/docs/internal/dnd.md index 796f0f5..cdf6037 100644 --- a/docs/internal/dnd.md +++ b/docs/internal/dnd.md @@ -160,6 +160,11 @@ These projections are guidance only and never event evidence. The following differences are intentional and should remain explicit when a shared helper changes. +Shared D&D text comparison is identified by `dnd.text_comparison.v1`. Any +semantic change requires an explicit policy-version review for every affected +identity, mapping, normalization, and validator policy; helper source is not a +checkpoint fingerprint. + | Lane | Intentional behavior | | --- | --- | | Spells | May use a spell-catalog overlay and optional NPC grounding; the catalog validator supplies domain-specific semantic checks. | diff --git a/internal/modules/dnd/extract/itemoccurrences/canonicalize.go b/internal/modules/dnd/extract/itemoccurrences/canonicalize.go index 11c09ea..cbd37ba 100644 --- a/internal/modules/dnd/extract/itemoccurrences/canonicalize.go +++ b/internal/modules/dnd/extract/itemoccurrences/canonicalize.go @@ -1,6 +1,7 @@ package itemoccurrences import ( + "fmt" "sort" "gitea.maximumdirect.net/eric/notarius/internal/core/source" @@ -15,12 +16,15 @@ type orderedItemOccurrenceResponse struct { hasEvidence bool } -func canonicalizeResponse(response *extractionResponse, order shared.SourceRefOrder, sourceID string) { +func canonicalizeResponse(response *extractionResponse, order shared.SourceRefOrder, sourceID string, registry *itemregistry.Registry) error { if response == nil { - return + return nil } ordered := make([]orderedItemOccurrenceResponse, len(response.Occurrences)) for index := range response.Occurrences { + if err := validateRegistryPair(index, response.Occurrences[index], registry); err != nil { + return err + } earliest, hasEvidence := canonicalizeItemOccurrence(&response.Occurrences[index], order, sourceID) ordered[index] = orderedItemOccurrenceResponse{ value: response.Occurrences[index], @@ -40,6 +44,7 @@ func canonicalizeResponse(response *extractionResponse, order shared.SourceRefOr for index := range ordered { response.Occurrences[index] = ordered[index].value } + return nil } func canonicalizeItemOccurrence(occurrence *itemOccurrenceResponse, order shared.SourceRefOrder, sourceID string) (int, bool) { @@ -51,16 +56,23 @@ func canonicalizeItemOccurrence(occurrence *itemOccurrenceResponse, order shared return order.EarliestValid(refs) } -func canonicalItemOccurrenceList(response extractionResponse, sourceID string, registry *itemregistry.Registry) dnd.ItemOccurrenceList { +func validateRegistryPair(index int, occurrence itemOccurrenceResponse, registry *itemregistry.Registry) error { + item, found := registry.LookupID(occurrence.ItemID) + if !found { + return fmt.Errorf("occurrences[%d].item_id is not in the item registry", index) + } + if item.Name != occurrence.Name { + return fmt.Errorf("occurrences[%d].name does not match item_id", index) + } + return nil +} + +func canonicalItemOccurrenceList(response extractionResponse, sourceID string) dnd.ItemOccurrenceList { if response.Occurrences == nil { return dnd.ItemOccurrenceList{} } occurrences := make([]dnd.ItemOccurrence, 0, len(response.Occurrences)) for _, occurrence := range response.Occurrences { - item, found := registry.LookupID(occurrence.ItemID) - if !found || item.Name != occurrence.Name { - continue - } occurrences = append(occurrences, dnd.ItemOccurrence{ ItemID: occurrence.ItemID, Name: occurrence.Name, diff --git a/internal/modules/dnd/extract/itemoccurrences/extractor.go b/internal/modules/dnd/extract/itemoccurrences/extractor.go index 2de9970..d0e76d2 100644 --- a/internal/modules/dnd/extract/itemoccurrences/extractor.go +++ b/internal/modules/dnd/extract/itemoccurrences/extractor.go @@ -109,6 +109,7 @@ func (e *Extractor) ManifestMetadata() map[string]any { "response_schema_version": SchemaVersion, "response_schema_sha256": e.responseSchemaSHA, "mapping_policy": mappingPolicy, + "comparison_policy": shared.TextComparisonPolicy, } seeded := e.itemResolver.Seeded() if seeded.Bound() { @@ -126,6 +127,7 @@ func (e *Extractor) CheckpointFingerprints() []pipeline.CheckpointFingerprint { {Name: "prompt", Value: e.promptSHA}, {Name: "response_schema", Value: e.responseSchemaSHA}, {Name: "mapping_policy", Value: mappingPolicy}, + {Name: "comparison_policy", Value: shared.TextComparisonPolicy}, {Name: "item_registry", Value: e.itemResolver.Seeded().ProjectionDigest()}, } } @@ -159,8 +161,10 @@ func (e *Extractor) Extract(ctx context.Context, req contracts.TypedExtractionRe }, &response); err != nil { return contracts.TypedExtractionResult[dnd.ItemOccurrenceList]{}, extractorErrorf("complete structured output: %w", err) } - canonicalizeResponse(&response, order, req.Source.ID) - return contracts.TypedExtractionResult[dnd.ItemOccurrenceList]{Value: canonicalItemOccurrenceList(response, req.Source.ID, registry)}, nil + if err := canonicalizeResponse(&response, order, req.Source.ID, registry); err != nil { + return contracts.TypedExtractionResult[dnd.ItemOccurrenceList]{}, extractorErrorf("map item occurrence response: %w", err) + } + return contracts.TypedExtractionResult[dnd.ItemOccurrenceList]{Value: canonicalItemOccurrenceList(response, req.Source.ID)}, nil } func ModuleSpec() pipeline.ModuleSpec { diff --git a/internal/modules/dnd/extract/itemoccurrences/extractor_test.go b/internal/modules/dnd/extract/itemoccurrences/extractor_test.go index e8bd657..0f19ac6 100644 --- a/internal/modules/dnd/extract/itemoccurrences/extractor_test.go +++ b/internal/modules/dnd/extract/itemoccurrences/extractor_test.go @@ -2,6 +2,7 @@ package itemoccurrences import ( "context" + "strings" "testing" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" @@ -12,8 +13,6 @@ func TestExtractGroundsOccurrencesInRequiredRegistry(t *testing.T) { id := itemidentity.DeriveID("Torch") client := &fakeItemOccurrencesLLMClient{response: extractionResponse{Occurrences: []itemOccurrenceResponse{ {ItemID: id, Name: "Torch", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)}, - {ItemID: "unknown", Name: "Unknown", Kind: "lost", From: "party", SourceRefs: responseRefs(2, 2)}, - {ItemID: "torch", Name: "Lantern", Kind: "lost", From: "party", SourceRefs: responseRefs(3, 3)}, }}} req := extractionRequest() req.References = itemRegistryReferences(t) @@ -30,6 +29,56 @@ func TestExtractGroundsOccurrencesInRequiredRegistry(t *testing.T) { } } +func TestExtractRejectsInvalidRegistryPairs(t *testing.T) { + id := itemidentity.DeriveID("Torch") + for _, test := range []struct { + name string + occurrence itemOccurrenceResponse + wantError string + }{ + { + name: "unknown item ID", + occurrence: itemOccurrenceResponse{ItemID: "unknown", Name: "Torch", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)}, + wantError: "occurrences[0].item_id is not in the item registry", + }, + { + name: "mismatched item name", + occurrence: itemOccurrenceResponse{ItemID: id, Name: "Lantern", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)}, + wantError: "occurrences[0].name does not match item_id", + }, + } { + t.Run(test.name, func(t *testing.T) { + client := &fakeItemOccurrencesLLMClient{response: extractionResponse{Occurrences: []itemOccurrenceResponse{test.occurrence}}} + req := extractionRequest() + req.References = itemRegistryReferences(t) + result, err := newExtractor(t, client, req.References).Extract(context.Background(), req) + if err == nil || !strings.Contains(err.Error(), test.wantError) { + t.Fatalf("Extract() error = %v, want %q", err, test.wantError) + } + if len(result.Value.Occurrences) != 0 { + t.Fatalf("Extract() returned partial occurrences: %#v", result.Value.Occurrences) + } + }) + } +} + +func TestExtractRejectsResponseWithInvalidRegistryPairAfterValidOccurrence(t *testing.T) { + id := itemidentity.DeriveID("Torch") + client := &fakeItemOccurrencesLLMClient{response: extractionResponse{Occurrences: []itemOccurrenceResponse{ + {ItemID: id, Name: "Torch", Kind: "lost", From: "party", SourceRefs: responseRefs(1, 1)}, + {ItemID: "unknown", Name: "Unknown", Kind: "lost", From: "party", SourceRefs: responseRefs(2, 2)}, + }}} + req := extractionRequest() + req.References = itemRegistryReferences(t) + result, err := newExtractor(t, client, req.References).Extract(context.Background(), req) + if err == nil || !strings.Contains(err.Error(), "occurrences[1].item_id") { + t.Fatalf("Extract() error = %v, want occurrence index and item ID", err) + } + if len(result.Value.Occurrences) != 0 { + t.Fatalf("Extract() returned partial occurrences: %#v", result.Value.Occurrences) + } +} + func TestExtractRequiresItemRegistry(t *testing.T) { _, err := newExtractor(t, &fakeItemOccurrencesLLMClient{}).Extract(context.Background(), extractionRequest()) if err == nil { diff --git a/internal/modules/dnd/itemoccurrences/itemoccurrences.go b/internal/modules/dnd/itemoccurrences/itemoccurrences.go index 5a40f1e..68c0fef 100644 --- a/internal/modules/dnd/itemoccurrences/itemoccurrences.go +++ b/internal/modules/dnd/itemoccurrences/itemoccurrences.go @@ -8,7 +8,6 @@ import ( "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" ) @@ -34,7 +33,7 @@ func DisplayValue(value string) string { return strings.TrimSpace(value) } // ComparisonKey returns the shared D&D Unicode- and case-insensitive key for // a trimmed item or holder display value. -func ComparisonKey(value string) string { return identity.ComparisonKey(DisplayValue(value)) } +func ComparisonKey(value string) string { return shared.ComparisonKey(DisplayValue(value)) } // HolderPresent reports whether value is a nonblank optional holder. func HolderPresent(value string) bool { return DisplayValue(value) != "" } diff --git a/internal/modules/dnd/items/identity/identity.go b/internal/modules/dnd/items/identity/identity.go index c4c7929..fc62f32 100644 --- a/internal/modules/dnd/items/identity/identity.go +++ b/internal/modules/dnd/items/identity/identity.go @@ -8,10 +8,8 @@ import ( "fmt" "strings" - "golang.org/x/text/cases" - "golang.org/x/text/unicode/norm" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) const ( @@ -51,17 +49,7 @@ func NormalizeDisplay(value string) string { // ComparisonKey returns the stable key used for item-type identity comparisons. func ComparisonKey(value string) string { - value = norm.NFKC.String(value) - value = strings.Map(func(r rune) rune { - switch r { - case '\u2018', '\u2019', '\u02bc': - return '\'' - default: - return r - } - }, value) - value = strings.Join(strings.Fields(value), " ") - return cases.Fold().String(value) + return shared.ComparisonKey(value) } // DeriveID returns the deterministic ID for an item type. Empty identity keys diff --git a/internal/modules/dnd/items/identity/identity_test.go b/internal/modules/dnd/items/identity/identity_test.go index 4470905..12d8873 100644 --- a/internal/modules/dnd/items/identity/identity_test.go +++ b/internal/modules/dnd/items/identity/identity_test.go @@ -9,29 +9,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" ) -func TestComparisonKeyNormalizesSupportedEquivalences(t *testing.T) { - tests := []struct { - name string - left string - right string - }{ - {name: "case", left: "Silver Key", right: "sILVER kEY"}, - {name: "compatibility", left: "Silver Key", right: "Silver Key"}, - {name: "whitespace", left: " Silver\u2003Key ", right: "Silver Key"}, - {name: "apostrophe", left: "Healer’s Kit", right: "healer's kit"}, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - if ComparisonKey(test.left) != ComparisonKey(test.right) { - t.Fatalf("ComparisonKey(%q) = %q, ComparisonKey(%q) = %q", test.left, ComparisonKey(test.left), test.right, ComparisonKey(test.right)) - } - }) - } - if ComparisonKey("Silver Key") == ComparisonKey("Gold Key") { - t.Fatal("different item types received the same comparison key") - } -} - func TestDeriveIDUsesExactCompactIdentityBytes(t *testing.T) { const wantID = "item:sha256:2f211c60b9bdcf3a6dd64086cc4c05df5b3357ae02cb462f7ef8988d9bd2fa4f" const wantIdentity = `["dnd.item_registry.identity.v1","silver key"]` diff --git a/internal/modules/dnd/locations/identity/identity.go b/internal/modules/dnd/locations/identity/identity.go index bada3aa..14c3e72 100644 --- a/internal/modules/dnd/locations/identity/identity.go +++ b/internal/modules/dnd/locations/identity/identity.go @@ -10,8 +10,7 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" - "golang.org/x/text/cases" - "golang.org/x/text/unicode/norm" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) const ( @@ -51,17 +50,7 @@ func NormalizeDisplay(value string) string { // ComparisonKey returns the stable key used for location identity comparisons. func ComparisonKey(value string) string { - value = norm.NFKC.String(value) - value = strings.Map(func(r rune) rune { - switch r { - case '\u2018', '\u2019', '\u02bc': - return '\'' - default: - return r - } - }, value) - value = strings.Join(strings.Fields(value), " ") - return cases.Fold().String(value) + return shared.ComparisonKey(value) } // DeriveID derives a location ID from name and the earliest canonical evidence diff --git a/internal/modules/dnd/locations/identity/identity_test.go b/internal/modules/dnd/locations/identity/identity_test.go index ae03575..ebff1f4 100644 --- a/internal/modules/dnd/locations/identity/identity_test.go +++ b/internal/modules/dnd/locations/identity/identity_test.go @@ -15,17 +15,6 @@ func TestNormalizeDisplayOnlyChangesWhitespace(t *testing.T) { } } -func TestComparisonKeyNormalizesUnicodeWhitespaceAndApostrophes(t *testing.T) { - for _, pair := range [][2]string{ - {" Caf\u00e9\u2003d\u2019Or ", "cafe\u0301 d'Or"}, - {"\uff34\uff48\uff45\u00a0\uff34\uff41\uff56\uff45\uff52\uff4e", "the tavern"}, - } { - if left, right := ComparisonKey(pair[0]), ComparisonKey(pair[1]); left != right { - t.Fatalf("ComparisonKey(%q) = %q, want value equal to %q", pair[0], left, pair[1]) - } - } -} - func TestDeriveIDUsesDocumentedCompactJSONInput(t *testing.T) { refs := []source.SourceRef{ {SourceID: "session-alpha", StartUnitID: 9, EndUnitID: 9}, diff --git a/internal/modules/dnd/normalize/itemoccurrences/normalizer.go b/internal/modules/dnd/normalize/itemoccurrences/normalizer.go index dc6c0f4..54eb9c7 100644 --- a/internal/modules/dnd/normalize/itemoccurrences/normalizer.go +++ b/internal/modules/dnd/normalize/itemoccurrences/normalizer.go @@ -68,7 +68,7 @@ func (n *Normalizer) ManifestMetadata() map[string]any { if n == nil || n.itemResolver == nil { return nil } - metadata := map[string]any{"normalization_policy": normalizationPolicy} + metadata := map[string]any{"normalization_policy": normalizationPolicy, "comparison_policy": shared.TextComparisonPolicy} seeded := n.itemResolver.Seeded() if seeded.Bound() { metadata["item_registry_digest"] = seeded.Digest() @@ -83,6 +83,7 @@ func (n *Normalizer) CheckpointFingerprints() []pipeline.CheckpointFingerprint { } return []pipeline.CheckpointFingerprint{ {Name: "normalization_policy", Value: normalizationPolicy}, + {Name: "comparison_policy", Value: shared.TextComparisonPolicy}, {Name: "item_registry", Value: n.itemResolver.Seeded().ProjectionDigest()}, } } diff --git a/internal/modules/dnd/npcs/identity/identity.go b/internal/modules/dnd/npcs/identity/identity.go index 31c4b90..c4d2b78 100644 --- a/internal/modules/dnd/npcs/identity/identity.go +++ b/internal/modules/dnd/npcs/identity/identity.go @@ -9,10 +9,8 @@ import ( "fmt" "strings" - "golang.org/x/text/cases" - "golang.org/x/text/unicode/norm" - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared" ) const ( @@ -52,17 +50,7 @@ func NormalizeDisplay(value string) string { // ComparisonKey returns the stable key used for NPC identity comparisons. func ComparisonKey(value string) string { - value = norm.NFKC.String(value) - value = strings.Map(func(r rune) rune { - switch r { - case '\u2018', '\u2019', '\u02bc': - return '\'' - default: - return r - } - }, value) - value = strings.Join(strings.Fields(value), " ") - return cases.Fold().String(value) + return shared.ComparisonKey(value) } // DeriveID returns the deterministic ID for a canonical NPC name. Empty diff --git a/internal/modules/dnd/npcs/identity/identity_test.go b/internal/modules/dnd/npcs/identity/identity_test.go index 342fdeb..4b6f9fa 100644 --- a/internal/modules/dnd/npcs/identity/identity_test.go +++ b/internal/modules/dnd/npcs/identity/identity_test.go @@ -9,30 +9,6 @@ import ( "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd" ) -func TestComparisonKeyNormalizesSupportedEquivalences(t *testing.T) { - tests := []struct { - name string - left string - right string - }{ - {name: "case", left: "Captain Vale", right: "cAPtAiN vALE"}, - {name: "compatibility", left: "Ally", right: "Ally"}, - {name: "whitespace", left: " Mira\u2003Thorn ", right: "Mira Thorn"}, - {name: "apostrophe", left: "O’Rin", right: "o'Rin"}, - {name: "modifier apostrophe", left: "OʼRin", right: "o'Rin"}, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - if ComparisonKey(test.left) != ComparisonKey(test.right) { - t.Fatalf("ComparisonKey(%q) = %q, ComparisonKey(%q) = %q", test.left, ComparisonKey(test.left), test.right, ComparisonKey(test.right)) - } - }) - } - if ComparisonKey("Mira Thorn") == ComparisonKey("Mira Thorne") { - t.Fatal("different names received the same comparison key") - } -} - func TestNormalizeDisplayOnlyChangesWhitespace(t *testing.T) { if got := NormalizeDisplay(" O’Rin\u2003Thorn "); got != "O’Rin Thorn" { t.Fatalf("NormalizeDisplay() = %q", got) diff --git a/internal/modules/dnd/shared/matching.go b/internal/modules/dnd/shared/matching.go index ebe1fb0..d307605 100644 --- a/internal/modules/dnd/shared/matching.go +++ b/internal/modules/dnd/shared/matching.go @@ -3,14 +3,12 @@ package shared import ( "strings" "unicode" - - "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/npcs/identity" ) // NormalizedTokens returns identity-normalized alphanumeric tokens from a // D&D value. func NormalizedTokens(value string) []string { - value = identity.ComparisonKey(value) + value = ComparisonKey(value) if value == "" { return nil } diff --git a/internal/modules/dnd/shared/matching_test.go b/internal/modules/dnd/shared/matching_test.go index de9d990..979685d 100644 --- a/internal/modules/dnd/shared/matching_test.go +++ b/internal/modules/dnd/shared/matching_test.go @@ -2,6 +2,32 @@ package shared import "testing" +func TestComparisonKeyNormalizesDNDText(t *testing.T) { + if TextComparisonPolicy != "dnd.text_comparison.v1" { + t.Fatalf("TextComparisonPolicy = %q", TextComparisonPolicy) + } + for _, test := range []struct { + name string + input string + want string + }{ + {name: "case", input: "Captain Vale", want: "captain vale"}, + {name: "compatibility", input: "Ally", want: "ally"}, + {name: "whitespace", input: " Mira\u2003Thorn ", want: "mira thorn"}, + {name: "apostrophes", input: "O’Rin and OʼRin", want: "o'rin and o'rin"}, + {name: "unicode composition", input: " Cafe\u0301\u2003d’Or ", want: "café d'or"}, + } { + t.Run(test.name, func(t *testing.T) { + if got := ComparisonKey(test.input); got != test.want { + t.Fatalf("ComparisonKey(%q) = %q, want %q", test.input, got, test.want) + } + }) + } + if ComparisonKey("Mira Thorn") == ComparisonKey("Mira Thorne") { + t.Fatal("different names received the same comparison key") + } +} + func TestContainsTokenSequence(t *testing.T) { tests := []struct { name string diff --git a/internal/modules/dnd/shared/text_comparison.go b/internal/modules/dnd/shared/text_comparison.go new file mode 100644 index 0000000..e23af93 --- /dev/null +++ b/internal/modules/dnd/shared/text_comparison.go @@ -0,0 +1,28 @@ +package shared + +import ( + "strings" + + "golang.org/x/text/cases" + "golang.org/x/text/unicode/norm" +) + +// TextComparisonPolicy identifies the shared D&D text-comparison semantics. +// A semantic change requires a new value and a review of every dependent +// identity, mapping, normalization, and validator policy. +const TextComparisonPolicy = "dnd.text_comparison.v1" + +// ComparisonKey returns the D&D Unicode- and case-insensitive comparison key. +func ComparisonKey(value string) string { + value = norm.NFKC.String(value) + value = strings.Map(func(r rune) rune { + switch r { + case '\u2018', '\u2019', '\u02bc': + return '\'' + default: + return r + } + }, value) + value = strings.Join(strings.Fields(value), " ") + return cases.Fold().String(value) +}