From 7f28899730e9d4f78f2dbc85304fda94bfaa3ae6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 4 Aug 2026 13:26:06 +0000 Subject: [PATCH] Migrate NPC registry to shared resolver --- .../modules/dnd/npcs/registry/registry.go | 144 ++++++------------ .../dnd/npcs/registry/registry_test.go | 111 +++++++++++++- .../dnd/shared/registryresolver/resolver.go | 4 +- 3 files changed, 156 insertions(+), 103 deletions(-) diff --git a/internal/modules/dnd/npcs/registry/registry.go b/internal/modules/dnd/npcs/registry/registry.go index 95e38d4..e0ecc9e 100644 --- a/internal/modules/dnd/npcs/registry/registry.go +++ b/internal/modules/dnd/npcs/registry/registry.go @@ -7,9 +7,6 @@ import ( "encoding/hex" "encoding/json" "fmt" - "mime" - "strings" - "sync" "gitea.maximumdirect.net/eric/notarius/internal/core/source" "gitea.maximumdirect.net/eric/notarius/internal/framework/contracts" @@ -17,6 +14,7 @@ import ( 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" + "gitea.maximumdirect.net/eric/notarius/internal/modules/dnd/shared/registryresolver" ) const ( @@ -37,39 +35,20 @@ type Registry struct { lookupByKey map[string]int } -// Resolver retains only the validated construction-time registry and immutable -// canonical registries keyed by their semantic digest. Operation references -// are resolved on demand; caller-owned reference bytes are never retained. +// Resolver selects and memoizes immutable NPC registry views. type Resolver struct { - seeded *Registry - - mu sync.Mutex - cache map[string]*Registry - rawCache map[string]*Registry + resolver *registryresolver.Resolver[*Registry] } // NewResolver validates the optional construction-time NPC reference and // prepares the operation-time registry cache. A malformed static reference // therefore fails before any operation starts. func NewResolver(references contracts.ReferenceSet) (*Resolver, error) { - seeded, err := Resolve(constructionReferences(references)) + resolver, err := registryresolver.New(registryResolverConfig(), references) if err != nil { return nil, err } - return &Resolver{seeded: seeded, cache: make(map[string]*Registry), rawCache: make(map[string]*Registry)}, nil -} - -func constructionReferences(references contracts.ReferenceSet) contracts.ReferenceSet { - slot, ok := references.Slots[ReferenceSlot] - if !ok || len(slot.Items) > 0 { - return references - } - cloned := contracts.ReferenceSet{Slots: make(map[string]contracts.ResolvedReferenceSlot, len(references.Slots))} - for name, value := range references.Slots { - cloned.Slots[name] = value - } - delete(cloned.Slots, ReferenceSlot) - return cloned + return &Resolver{resolver: resolver}, nil } // Seeded returns the immutable construction-time registry. Its accessors are @@ -78,7 +57,7 @@ func (r *Resolver) Seeded() *Registry { if r == nil { return nil } - return r.seeded + return r.resolver.Seeded() } // Resolve returns the effective registry for one operation. An operation @@ -86,90 +65,57 @@ func (r *Resolver) Seeded() *Registry { // matching that registry reuses it; other canonical registries are cached by // digest for concurrent chunk operations. func (r *Resolver) Resolve(references contracts.ReferenceSet) (*Registry, error) { - if r == nil { + if r == nil || r.resolver == nil { return Resolve(references) } - if _, ok := references.Slots[ReferenceSlot]; !ok { - return r.seeded, nil - } - - slot := references.Slots[ReferenceSlot] - rawKey := "" - if len(slot.Items) == 1 { - rawKey = strings.ToLower(strings.TrimSpace(slot.Items[0].MediaType)) + "\x00" + semanticDigest(slot.Items[0].Content) - } - - r.mu.Lock() - defer r.mu.Unlock() - if rawKey != "" { - if cached, ok := r.rawCache[rawKey]; ok { - return cached, nil - } - } - resolved, err := Resolve(references) - if err != nil { - return nil, err - } - if sameRegistryIdentity(r.seeded, resolved) { - if rawKey != "" { - r.rawCache[rawKey] = r.seeded - } - return r.seeded, nil - } - - if cached, ok := r.cache[resolved.Digest()]; ok { - if rawKey != "" { - r.rawCache[rawKey] = cached - } - return cached, nil - } - r.cache[resolved.Digest()] = resolved - if rawKey != "" { - r.rawCache[rawKey] = resolved - } - return resolved, nil -} - -func sameRegistryIdentity(first, second *Registry) bool { - if first == nil || second == nil { - return first == second - } - return first.bound == second.bound && first.digest == second.digest + return r.resolver.Resolve(references) } // Resolve prepares the optional NPC registry reference. An absent slot // produces the exact empty prompt input and no semantic registry identity. func Resolve(references contracts.ReferenceSet) (*Registry, error) { - slot, ok := references.Slots[ReferenceSlot] - if !ok { - content := []byte(emptyPrompt) - projectionDigest := semanticDigest(content) - return &Registry{ - list: dnd.NPCList{NPCs: []dnd.NPC{}}, - canonical: append([]byte(nil), content...), - projectionDigest: projectionDigest, - promptInput: contracts.NewLLMInputMaterial(ReferenceSlot, npccodec.MediaType, content, projectionDigest, ""), - lookupByKey: map[string]int{}, - }, nil - } - if len(slot.Items) != 1 { - return nil, fmt.Errorf("reference slot %q must contain exactly one item", ReferenceSlot) - } - - item := slot.Items[0] - mediaType, _, err := mime.ParseMediaType(item.MediaType) + item, present, err := registryresolver.ResolveOptionalSingleItem(references, npcReferenceSpec()) if err != nil { - return nil, fmt.Errorf("reference slot %q item media type is invalid", ReferenceSlot) + return nil, err } - if !strings.EqualFold(mediaType, npccodec.MediaType) { - return nil, fmt.Errorf("reference slot %q item media type must be %s", ReferenceSlot, npccodec.MediaType) - } - if len(item.Content) > MaxBytes { - return nil, fmt.Errorf("reference slot %q item is %d bytes, limit %d", ReferenceSlot, len(item.Content), MaxBytes) + if !present { + return emptyRegistry(), nil } + return loadRegistry(item.Content) +} +func registryResolverConfig() registryresolver.Config[*Registry] { + return registryresolver.Config[*Registry]{ + Reference: npcReferenceSpec(), + Absent: func() (*Registry, error) { + return emptyRegistry(), nil + }, + Load: loadRegistry, + SemanticIdentity: func(registry *Registry) string { + return registry.Digest() + }, + } +} + +func npcReferenceSpec() registryresolver.ReferenceSpec { + return registryresolver.ReferenceSpec{SlotName: ReferenceSlot, AcceptedMediaType: npccodec.MediaType, MaxBytes: MaxBytes} +} + +func emptyRegistry() *Registry { + content := []byte(emptyPrompt) + projectionDigest := semanticDigest(content) + return &Registry{ + list: dnd.NPCList{NPCs: []dnd.NPC{}}, + canonical: append([]byte(nil), content...), + projectionDigest: projectionDigest, + promptInput: contracts.NewLLMInputMaterial(ReferenceSlot, npccodec.MediaType, content, projectionDigest, ""), + lookupByKey: map[string]int{}, + } +} + +func loadRegistry(referenceContent []byte) (*Registry, error) { codec := npccodec.New() - value, err := codec.Decode(item.Content) + value, err := codec.Decode(referenceContent) if err != nil { return nil, fmt.Errorf("decode NPC registry: invalid approved NPC JSON") } diff --git a/internal/modules/dnd/npcs/registry/registry_test.go b/internal/modules/dnd/npcs/registry/registry_test.go index 9912398..99ad6cb 100644 --- a/internal/modules/dnd/npcs/registry/registry_test.go +++ b/internal/modules/dnd/npcs/registry/registry_test.go @@ -2,8 +2,10 @@ package registry import ( "bytes" + "fmt" "reflect" "strings" + "sync" "testing" "gitea.maximumdirect.net/eric/notarius/internal/core/source" @@ -122,6 +124,111 @@ func TestResolverReusesEquivalentCanonicalRegistries(t *testing.T) { } } +func TestResolverValidatesStaticAndOperationReferences(t *testing.T) { + placeholder, err := NewResolver(referenceSet()) + if err != nil || placeholder.Seeded().Bound() { + t.Fatalf("generated placeholder = %#v, %v; want unbound seed", placeholder, err) + } + + valid := listReferenceSet(t, registryFixture()) + validContent := valid.Slots[ReferenceSlot].Items[0].Content + invalidSets := []contracts.ReferenceSet{ + referenceSet(contracts.ReferenceItem{MediaType: npccodec.MediaType, Content: []byte(`{"npcs":[`)}), + referenceSet(contracts.ReferenceItem{MediaType: "text/plain", Content: validContent}), + referenceSet(contracts.ReferenceItem{MediaType: npccodec.MediaType, Content: make([]byte, MaxBytes+1)}), + referenceSet( + contracts.ReferenceItem{MediaType: npccodec.MediaType, Content: validContent}, + contracts.ReferenceItem{MediaType: npccodec.MediaType, Content: validContent}, + ), + } + for index, references := range invalidSets { + if _, err := NewResolver(references); err == nil { + t.Fatalf("NewResolver(invalid %d) error = nil", index) + } + if _, err := placeholder.Resolve(references); err == nil { + t.Fatalf("Resolve(invalid %d) error = nil", index) + } + } + + staticContent := append([]byte(nil), validContent...) + staticReferences := referenceSet(contracts.ReferenceItem{MediaType: npccodec.MediaType, Content: staticContent}) + seeded, err := NewResolver(staticReferences) + if err != nil { + t.Fatal(err) + } + staticContent[0] = '[' + delete(staticReferences.Slots, ReferenceSlot) + if seeded.Seeded().Count() != 2 || seeded.Seeded().CanonicalBytes()[0] != '{' { + t.Fatalf("seeded registry retained construction references: %#v", seeded.Seeded()) + } +} + +func TestResolverCachesEquivalentRegistriesConcurrentlyAndIgnoresCallerDigest(t *testing.T) { + resolver, err := NewResolver(referenceSet()) + if err != nil { + t.Fatal(err) + } + content := listReferenceSet(t, registryFixture()).Slots[ReferenceSlot].Items[0].Content + references := referenceSet(contracts.ReferenceItem{MediaType: npccodec.MediaType, Content: content}) + first, err := resolver.Resolve(references) + if err != nil { + t.Fatal(err) + } + second, err := resolver.Resolve(references) + if err != nil || second != first { + t.Fatalf("raw reuse = %p / %p, %v", first, second, err) + } + spaced := append([]byte("\n "), content...) + spaced = append(spaced, '\n') + equivalent, err := resolver.Resolve(referenceSet(contracts.ReferenceItem{MediaType: "APPLICATION/JSON; charset=utf-8", Content: spaced})) + if err != nil || equivalent != first { + t.Fatalf("semantic reuse = %p / %p, %v", first, equivalent, err) + } + + const callers = 24 + var group sync.WaitGroup + errors := make(chan error, callers) + for range callers { + group.Add(1) + go func() { + defer group.Done() + resolved, err := resolver.Resolve(references) + if err != nil || resolved != first { + errors <- fmt.Errorf("resolved %p, want %p: %w", resolved, first, err) + } + }() + } + group.Wait() + close(errors) + for err := range errors { + t.Error(err) + } + + sharedDigest := "sha256:" + strings.Repeat("0", 64) + firstItem := contracts.ReferenceItem{MediaType: npccodec.MediaType, Content: content, Digest: sharedDigest} + otherList := registryFixture() + otherList.NPCs[0].Name = "The Greencloak" + otherList.NPCs[0].ID = identity.DeriveID(otherList.NPCs[0].Name) + otherContent := listReferenceSet(t, otherList).Slots[ReferenceSlot].Items[0].Content + otherItem := contracts.ReferenceItem{MediaType: npccodec.MediaType, Content: otherContent, Digest: sharedDigest} + byDigestFirst, err := resolver.Resolve(referenceSet(firstItem)) + if err != nil { + t.Fatal(err) + } + byDigestOther, err := resolver.Resolve(referenceSet(otherItem)) + if err != nil || byDigestFirst == byDigestOther || byDigestFirst.Digest() == byDigestOther.Digest() { + t.Fatalf("caller digest aliased different registries: %p / %p, %v", byDigestFirst, byDigestOther, err) + } + + content[0] = '[' + if npc, ok := first.Lookup("Mira Thorn"); !ok || npc.Name != "Mira Thorn" { + t.Fatalf("resolved registry retained operation bytes: %#v, %t", npc, ok) + } + if fallback, err := resolver.Resolve(contracts.ReferenceSet{}); err != nil || fallback != resolver.Seeded() || fallback.Bound() { + t.Fatalf("fallback = %#v, %v; want unbound seed", fallback, err) + } +} + func registryFixture() dnd.NPCList { return dnd.NPCList{NPCs: []dnd.NPC{ {ID: identity.DeriveID("Mira Thorn"), Name: "Mira Thorn", SourceRefs: []source.SourceRef{{SourceID: "session-alpha", StartUnitID: 1, EndUnitID: 2}}}, @@ -147,8 +254,8 @@ func listReferenceSet(t *testing.T, list dnd.NPCList) contracts.ReferenceSet { return referenceSet(contracts.ReferenceItem{SlotName: ReferenceSlot, MediaType: npccodec.MediaType, Content: content}) } -func referenceSet(item contracts.ReferenceItem) contracts.ReferenceSet { - return contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{ReferenceSlot: {Items: []contracts.ReferenceItem{item}}}} +func referenceSet(items ...contracts.ReferenceItem) contracts.ReferenceSet { + return contracts.ReferenceSet{Slots: map[string]contracts.ResolvedReferenceSlot{ReferenceSlot: {Items: items}}} } func TestProjectionIsStableForEquivalentNormalizedRegistries(t *testing.T) { diff --git a/internal/modules/dnd/shared/registryresolver/resolver.go b/internal/modules/dnd/shared/registryresolver/resolver.go index bca9109..a6aaff4 100644 --- a/internal/modules/dnd/shared/registryresolver/resolver.go +++ b/internal/modules/dnd/shared/registryresolver/resolver.go @@ -169,7 +169,7 @@ func (r *Resolver[V]) resolveUncached(references contracts.ReferenceSet) (prepar func (r *Resolver[V]) absent() (preparedView[V], error) { value, err := r.config.Absent() if err != nil { - return preparedView[V]{}, fmt.Errorf("prepare absent reference slot %q: %w", r.slotName, err) + return preparedView[V]{}, err } return preparedView[V]{value: value, identity: r.config.SemanticIdentity(value)}, nil } @@ -177,7 +177,7 @@ func (r *Resolver[V]) absent() (preparedView[V], error) { func (r *Resolver[V]) load(content []byte) (preparedView[V], error) { value, err := r.config.Load(content) if err != nil { - return preparedView[V]{}, fmt.Errorf("load reference slot %q: %w", r.slotName, err) + return preparedView[V]{}, err } identity := strings.TrimSpace(r.config.SemanticIdentity(value)) if identity == "" {