From 29fcad6e9bf5a30620575d2d0372e95b2366807c Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 4 Aug 2026 18:29:17 +0000 Subject: [PATCH] Improve D&D registry caching and retire the completed roadmap --- docs/roadmap/implementation.md | 432 ------------------ docs/roadmap/location.md | 337 -------------- .../dnd/locations/registry/registry_test.go | 78 +--- .../locationoccurrences/normalizer_test.go | 40 +- .../dnd/npcs/registry/registry_test.go | 109 +---- .../modules/dnd/register/register_test.go | 10 +- .../dnd/shared/registryresolver/resolver.go | 46 +- 7 files changed, 96 insertions(+), 956 deletions(-) delete mode 100644 docs/roadmap/implementation.md delete mode 100644 docs/roadmap/location.md diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index 25a2e87..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,432 +0,0 @@ -# D&D Location Tracking Implementation Plan - -## Objective - -Complete the target state in [D&D Location Tracking](location.md) by correcting -the behavioral and maintainability issues found in the post-implementation -audit. The location registry and occurrence lanes are already implemented and -production-selectable; the remaining work restores one NPC normalization -contract, aligns location-occurrence classification with the roadmap, narrows a -module's declared references, and factors demonstrated registry-resolution -mechanics into a D&D-shared helper. - -This is an ordered implementation plan for a `gpt-5.6-terra` coding agent. -Stages 1 through 14 are complete and must not be reimplemented. Implement one -new stage per prompt, beginning with Stage 15, and complete each stage's focused -validation before proceeding. - -All remaining stages must follow: - -- [Architecture Policy](../policy/architecture.md); -- [Testing Policy](../policy/testing.md); -- [Documentation Policy](../policy/documentation.md); -- [D&D Module Internals](../internal/dnd.md); and -- the durable policy decisions in [the feature roadmap](location.md). - -Use behavior-level tests. Do not add tests that freeze exact prompt wording, -message counts, source layout, shared-prefix length, or other incidental -implementation details. Prompts and canonical documentation must agree, but -that agreement is a review responsibility rather than a reason to add a -change-detector test. - -## Completed Work Summary - -Stages 1 through 14 delivered the following production state: - -| Completed stage | Result | -| --- | --- | -| Stage 1 | Added location and location-occurrence domain types plus evidence-anchored location identity. | -| Stage 2 | Added strict `v1` durable codecs and JSON Schemas for both artifact kinds. | -| Stage 3 | Added shared, opaque-key D&D entity-reconciliation context and proposal safety infrastructure. | -| Stage 4 | Migrated NPC normalization to the shared reconciliation machinery. | -| Stage 5 | Added evidence-grounded `dnd/locations` extraction. | -| Stage 6 | Added conservative LLM-assisted location normalization. | -| Stage 7 | Added immutable location-registry resolution and source-free `{id, name}` projection. | -| Stage 8 | Added registry-grounded `dnd/location-occurrences` extraction. | -| Stage 9 | Added deterministic location-occurrence normalization. | -| Stage 10 | Added location shape, identity, evidence, and advisory validators. | -| Stage 11 | Added location-occurrence shape, registry, invariant, evidence, and advisory validators. | -| Stage 12 | Registered both lanes, their assets, evidence projectors, default profiles, and validator chains. | -| Stage 13 | Added the ordered generated-reference handoff to the complete D&D example and integration coverage. | -| Stage 14 | Published the current integration, configuration, internal, output, and top-level documentation and ran final verification. | - -The post-implementation audit confirmed that the feature's durable types, -identity derivation, codecs, registration, validators, generated handoff, -evidence publication, maintained example, and integration coverage are in -place. At the audit baseline, `go test ./...`, focused race tests, `go vet -./...`, `go build ./cmd/notarius`, and `git diff --check` all passed. - -## Stage 15: Restore NPC Canonical-Name Selection Semantics - -### Goal - -Restore the NPC-specific canonical display-name policy that was weakened during -the shared reconciliation migration, without changing the shared proposal -contract or any durable artifact shape. - -### Work - -- Update the NPC normalization task asset under - `internal/modules/dnd/normalize/npcs/assets/prompts/` so canonical selection - again applies all of these rules: - - choose only a supplied candidate from an evidence-supported duplicate - group; - - prefer a complete, stable proper name over an abbreviation; and - - prefer an unadorned proper name over that name plus a contextual class, - role, title, or relationship descriptor unless the transcript establishes - the descriptor as part of the person's name. -- Preserve the opaque-key shared reconciliation instructions, prompt-message - ordering, response schema, retry/fallback behavior, and deterministic group - application. Do not move NPC-specific naming policy into - `shared/entityreconcile`. -- Update the canonical NPC artifact contract in - `docs/integrations/dnd-npc-artifacts.md` with the observable canonical-name - selection rule. Keep `docs/internal/dnd.md` focused on normalization - mechanics and link to the integration contract rather than maintaining a - competing definition; add or adjust a link only if needed for clarity. -- Allow the prompt fingerprint and any derived manifest/checkpoint metadata to - change through the existing asset-fingerprint mechanism. Do not manually - pin a digest. -- Retain existing behavior tests for opaque candidate selection, evidence - union, ID derivation, retry/fallback, and non-mutation. Add a test only if it - exercises deterministic code behavior; do not assert prompt prose. - -### Acceptance Criteria - -- The rendered NPC normalization prompt contains one unambiguous NPC-owned - policy that distinguishes proper names from contextual descriptors. -- `Captain Aria` is not preferred over `Aria` solely because it is longer when - `Captain` is only a contextual title, while an established full name may - still outrank an abbreviation. -- No module key, schema, identity policy, response schema, validator chain, or - shared reconciliation asset changes. -- Canonical current documentation, rather than this implementation plan, owns - the restored externally observable rule. -- `go test ./internal/modules/dnd/normalize/npcs/... ./internal/modules/dnd/shared/entityreconcile/... ./internal/modules/dnd/register/...` - passes offline. -- `git diff --check` reports no errors. - -### Prompt Size - -Small and suitable for one implementation prompt. - -## Stage 16: Correct Hypothetical Location-Occurrence Classification - -### Goal - -Make the location-occurrence prompt and durable documentation implement the -roadmap's distinction between an explicit speculative mention and an inferred -or unsupported occurrence. - -### Work - -- Update the location-occurrence task asset under - `internal/modules/dnd/extract/locationoccurrences/assets/prompts/` to encode - these exact semantic outcomes: - - explicit non-actionable speculation or a mere hypothetical reference to a - registry location is `mentioned`; - - an explicit proposal, intention, or agreement to travel is `planned`; - - a mere hypothetical or speculation is not `planned`; and - - an inferred, unstated, uncertain, or unsupported place or occurrence is - omitted. -- Preserve the documented precedence `visited`, `planned`, `recalled`, then - `mentioned`, including the ability to return multiple independently - evidenced facts from one passage. -- Update the `mentioned` definition in - `docs/integrations/dnd-location-occurrence-artifacts.md` to explicitly include - non-actionable speculation. Keep durable category semantics in that - integration contract and link from internal documentation rather than - duplicating the taxonomy elsewhere. -- Do not change the occurrence enum, durable schema, normalizer, validators, or - generated-reference contract. -- Allow existing prompt fingerprinting to record the asset change. Do not add - an exact-text or exact-digest test. - -### Acceptance Criteria - -- The prompt no longer instructs the model to omit every hypothetical - reference. -- An explicit statement such as “What if we went to Moon Gate?” is eligible as - `mentioned` when it is supported by a narrow transcript range, but is not a - `planned` occurrence without an actual proposal or intention. -- An unstated location inferred only from surrounding events remains omitted. -- The feature roadmap, prompt, and canonical integration contract agree on all - four categories and their precedence. -- `go test ./internal/modules/dnd/extract/locationoccurrences/... ./internal/modules/dnd/register/...` - passes offline. -- `git diff --check` reports no errors. - -### Prompt Size - -Small and suitable for one implementation prompt. - -## Stage 17: Narrow Location-Occurrence Normalizer References - -### Goal - -Make the deterministic normalizer advertise exactly the reference material it -uses. - -### Work - -- Change `internal/modules/dnd/normalize/locationoccurrences` so its - `ReferenceSlots` and `ModuleSpec` expose only the required `locations` slot. -- Keep that slot's existing contract unchanged: exactly one - `dnd/location-list` artifact, `application/json`, at most 1,048,576 bytes, - required at extraction and normalization. -- Remove the normalizer's unused `party`, `roster`, `players`, and `glossary` - declarations and any imports or description tables made dead by that change. - Do not remove those optional slots from the LLM-backed occurrence extractor, - which actually presents campaign material to the model. -- Add or revise focused module-contract tests to compare the normalizer's - complete declared slot set, including requiredness, media type, artifact kind, - and size. This is a stable module contract test, not a source-layout test. -- Verify that pipeline-level campaign reference defaults continue to bind only - to eligible targets and that the complete example still resolves. Current - configuration documentation already states that `locations` is required by - both occurrence stages; update it only if repository inspection finds a claim - that optional campaign references are consumed by the normalizer. -- Do not change normalization output, registry fingerprints, warnings, or - checkpoint handling for the required generated registry. - -### Acceptance Criteria - -- Catalog and `ModuleSpec` inspection report exactly one normalizer reference - slot: required `locations`. -- The extractor continues to accept its existing optional campaign context and - required location registry. -- Binding optional campaign references no longer implies that deterministic - occurrence normalization consumes them. -- The complete configuration and generated location handoff still resolve and - run through existing tests. -- `go test ./internal/modules/dnd/normalize/locationoccurrences/... ./internal/modules/dnd/register/... ./internal/modules/integration/... ./internal/cli/...` - passes offline. -- `git diff --check` reports no errors. - -### Prompt Size - -Small and suitable for one implementation prompt. - -## Stage 18: Add Shared Registry-Resolution Infrastructure - -### Goal - -Introduce a focused D&D-shared primitive for the demonstrated immutable -registry-resolution mechanics, without changing any production consumer in the -same prompt. - -### Work - -- Add `internal/modules/dnd/shared/registryresolver` as a domain-shared package; - do not put this helper in the generic framework. -- Provide an idiomatic generic resolver driven by callbacks or a small adapter - so it can operate on immutable domain registry views without importing NPC, - location, scene-description, or codec packages. -- The resolver core must own the behavior currently duplicated by the NPC and - location resolvers: - - validate and retain one construction-time seeded view; - - treat a declared generated-reference placeholder with zero construction- - time items as absent only during construction; - - use the seeded view when an operation does not supply the target slot; - - resolve an operation-time override when the target slot is present; - - memoize valid immutable views by a raw key and by semantic identity; - - reuse the seeded view when bound state and semantic identity match; - - synchronize concurrent resolution; and - - never retain caller-owned reference maps, item content, or mutable values. -- Raw cache identity must normalize the media-type spelling and hash the actual - item content. Do not trust a caller-supplied digest as proof that different - bytes are equal. -- Provide a narrow helper for resolving an optional single reference item that - validates the configured slot name, exact item count when bound, parsed media - type, and maximum size. Return owned bytes or require immediate decode so the - helper cannot retain caller-owned content. -- Keep artifact decoding, semantic identity validation, canonical encoding, - prompt projection, lookup indexes, clone policy, and domain-specific error - context in the owning registry packages. -- Keep errors bounded and content-free. Do not include raw reference bytes, - origin paths, or campaign data in diagnostics. -- Add focused tests for absent construction state, zero-item generated - placeholders, seeded fallback, operation overrides, malformed cardinality, - media types, size limits, raw reuse, semantic reuse, caller-supplied digest - mismatch, concurrent access, loader failure, and non-retention/non-mutation. - -### Acceptance Criteria - -- The shared package depends only on generic source/framework contracts and Go - standard-library packages, not concrete D&D artifact or codec packages. -- The helper owns only demonstrated resolution, validation, caching, and - ownership mechanics; it does not become a universal artifact registry. -- Invalid values and loader errors are never cached. -- Concurrent callers resolving identical bytes receive the same prepared - immutable view without races. -- Existing production registry packages remain unchanged in this stage. -- `go test -race ./internal/modules/dnd/shared/registryresolver/...` passes - offline. -- `git diff --check` reports no errors. - -### Prompt Size - -Medium and suitable for one implementation prompt. Keep production migrations -out of this stage so the generic ownership and concurrency contract can be -reviewed independently. - -## Stage 19: Migrate The NPC Registry To The Shared Resolver - -### Goal - -Make the established NPC registry the first production consumer of the shared -resolver while preserving all public and operational behavior. - -### Work - -- Refactor `internal/modules/dnd/npcs/registry` to compose the Stage 18 helper - for construction/operation selection, single-item validation, raw caching, - semantic caching, and synchronization. -- Preserve the existing package API used by extractors, normalizers, - validators, manifests, and tests, including `NewResolver`, `Resolve`, - `Seeded`, `Bound`, `NPCs`, `List`, `CanonicalBytes`, `Digest`, - `ProjectionDigest`, `Count`, `PromptInput`, `CanonicalName`, and `Matches`. -- Keep NPC-owned behavior in the NPC package: - - strict approved NPC codec use; - - NPC identity validation; - - canonical durable JSON and names-only prompt projection; - - comparison-key lookup; - - defensive NPC/source-reference cloning; and - - NPC-specific redacted error context. -- Preserve the exact empty projection, semantic and projection digest behavior, - seeded/generated resolution semantics, manifest metadata, checkpoint - fingerprints, and source-free prompt input. -- Remove superseded NPC-private resolver/cache/media/digest code only after the - shared helper supplies the same behavior. Do not migrate locations or scene - descriptions in this stage. -- Update focused tests as needed to verify behavior through the NPC package, - including malformed static and operation-time references, raw/semantic cache - reuse, concurrent resolution, defensive copies, prompt projection, and - downstream grounding. - -### Acceptance Criteria - -- Existing NPC consumers compile without API changes. -- NPC registry output bytes, prompt projection, lookup behavior, digests, - reference limits, errors, manifests, and checkpoints retain their established - semantics. -- The NPC package no longer contains a second implementation of resolver - selection, caching, or synchronization. -- `go test -race ./internal/modules/dnd/npcs/registry/... ./internal/modules/dnd/extract/npcinteractions/... ./internal/modules/dnd/normalize/npcinteractions/... ./internal/modules/dnd/validate/npcinteractions/...` - passes offline. -- `go test ./internal/modules/dnd/register/...` passes offline. -- `git diff --check` reports no errors. - -### Prompt Size - -Medium and suitable for one implementation prompt. - -## Stage 20: Migrate The Location Registry To The Shared Resolver - -### Goal - -Remove the newly introduced NPC/location resolver duplication while preserving -the location identity and generated-handoff contracts. - -### Work - -- Refactor `internal/modules/dnd/locations/registry` to compose the Stage 18 - helper for the same resolver, reference-item validation, cache, and - synchronization responsibilities migrated in Stage 19. -- Preserve the existing package API, including `NewResolver`, `Resolve`, - `Seeded`, `Bound`, `Locations`, `List`, `CanonicalBytes`, `Digest`, - `ProjectionDigest`, `Count`, `PromptInput`, `Lookup`, and `Matches`. -- Keep location-owned behavior in the location package: - - strict approved location codec use; - - evidence-anchored identity validation; - - canonical durable JSON and ordered source-free `{id, name}` projection; - - exact ID lookup and exact ID/name matching; - - defensive location/source-reference cloning; and - - location-specific redacted error context. -- Preserve absent/empty/bound distinctions, same-name locations with distinct - IDs, seeded/generated resolution, semantic and projection digests, manifest - metadata, checkpoint fingerprints, and the 1 MiB limit. -- Remove superseded location-private resolver/cache/media/digest code only - after parity is established. -- Do not migrate the scene-description eligibility registry in this stage. It - has a different projection and matching contract; including it is not needed - to eliminate the exact NPC/location duplication identified by the audit. -- Update focused and integration tests as needed for static and generated - references, malformed inputs, raw/semantic reuse, concurrency, defensive - copies, source-free projections, same-name ID lookup, normalizer grounding, - validator grounding, and handoff checkpoint invalidation. - -### Acceptance Criteria - -- Existing location consumers compile without API changes. -- Location registry output bytes, projection, identity validation, lookup, - digests, reference errors, manifests, and checkpoints retain their - established semantics. -- NPC and location registries share one implementation of construction/ - operation resolution, raw and semantic caching, and synchronization. -- No registry evidence or provenance appears in the occurrence prompt - projection. -- `go test -race ./internal/modules/dnd/locations/registry/... ./internal/modules/dnd/extract/locationoccurrences/... ./internal/modules/dnd/normalize/locationoccurrences/... ./internal/modules/dnd/validate/locationoccurrences/... ./internal/modules/integration/...` - passes offline. -- `git diff --check` reports no errors. - -### Prompt Size - -Medium and suitable for one implementation prompt. - -## Stage 21: Final Regression And Documentation Verification - -### Goal - -Verify the remediations as one coherent production state without expanding the -feature or prematurely retiring its roadmap documents. - -### Work - -- Run `gofmt` on all changed Go files and inspect the final diff for unrelated - changes, stale private resolver implementations, unused imports, and - accidental artifact-contract changes. -- Re-read the affected prompt assets and canonical integration documents to - verify that: - - NPC canonical-name selection retains the proper-name/title distinction; - - explicit non-actionable speculation maps to `mentioned`; - - inferred, uncertain, and unsupported occurrences remain omitted; and - - prompt changes did not reorder or duplicate shared cacheable prompt assets. -- Inspect module catalog output or registration tests to confirm the - location-occurrence normalizer declares only `locations`, while its extractor - retains the intended optional campaign slots. -- Confirm the shared resolver has exactly the NPC and location consumers - planned here and that domain-specific codecs, identity, projections, lookups, - and error context remain in their owning packages. -- Run the maintained complete example/configuration loading path and all - repository-wide validation. -- Leave `docs/roadmap/location.md` and this implementation plan in place for the - user's separate roadmap-retirement step. Do not delete them in this stage. - -### Acceptance Criteria - -- `go test ./internal/modules/dnd/...` passes. -- `go test ./internal/modules/integration/... ./internal/cli/...` passes. -- `go test -race ./internal/modules/dnd/shared/registryresolver/... ./internal/modules/dnd/npcs/registry/... ./internal/modules/dnd/locations/registry/...` passes. -- `go test ./...` passes. -- `go vet ./...` passes. -- `go build ./cmd/notarius` passes. -- Maintained configuration/example validation passes through its repository - test target. -- `git diff --check` reports no errors. -- Current documentation contains no stale contradiction about hypothetical - location mentions or NPC canonical-name selection. -- The worktree contains only the intended remediation changes and the roadmap - documents remain available for later retirement. - -### Prompt Size - -Small-to-medium and suitable for one implementation prompt. - -## Open Questions - -None. The feature roadmap and audit findings determine the required behavioral -semantics, module reference contract, abstraction boundary, migration order, -and validation expectations. The scene-description eligibility registry is -explicitly outside this remediation because its projection and matching -semantics differ from the duplicated NPC/location grounding registries. diff --git a/docs/roadmap/location.md b/docs/roadmap/location.md deleted file mode 100644 index 4e3d34d..0000000 --- a/docs/roadmap/location.md +++ /dev/null @@ -1,337 +0,0 @@ -# D&D Location Tracking - -## Purpose - -Add evidence-grounded D&D location tracking without turning a single extractor -into both an entity registry and an event classifier. The target design follows -the established NPC pattern: one lane identifies canonical location records and -a later lane records how the party related to those locations in the transcript. - -This roadmap defines the desired end state and policy choices. The ordered work -needed to reach that state is in [the implementation plan](implementation.md). - -## User Intent - -- Record locations the party visits or that the session otherwise discusses. -- Distinguish current physical presence from plans, recollections, and ordinary - mentions. -- Preserve transcript evidence for every durable record. -- Reconcile aliases and repeated appearances conservatively. -- Keep distinct places separate when they happen to share a generic name. -- Keep the schemas minimal. Location description, hierarchy, participants, and - narrative analysis belong in other artifacts or deterministic joins. - -## Target Capability - -The D&D module family will have two new lanes: - -1. `dnd/locations` produces a session-scoped registry of physical places. -2. `dnd/location-occurrences` consumes the normalized location registry and - produces an ordered list of source-grounded relationships between the party - and those places. - -The normalized location artifact is handed to the occurrence lane through a -required generated reference named `locations`. The occurrence lane must use -that registry for identity grounding, but the current transcript remains its -only evidence source. - -## Durable Artifact Contracts - -Both contracts remain at `v1`; Notarius is pre-release and does not need a -compatibility layer for these new artifacts. - -### Location registry - -The location lane uses: - -- artifact kind: `dnd/location-list` -- module key: `dnd/locations` -- schema ID: `notarius.dnd.locations` -- schema name: `notarius_dnd_locations_v1` -- media type: `application/json` -- root member: `locations` - -Each location contains exactly: - -| Field | Type | Meaning | -| --- | --- | --- | -| `id` | string | Deterministic, session-scoped canonical location identity. | -| `name` | string | Evidence-grounded display name or transcript-established label. | -| `source_refs` | non-empty source-reference array | Current-transcript evidence that identifies the place. | - -Locations are physical or spatial places: planes, regions, settlements, -districts, buildings, rooms, landmarks, routes, and geographic features. A -generic label such as `the tavern` is permitted only when the transcript uses -it for a specific place. The extractor must not invent a qualifier merely to -distinguish that place from another place with the same label. - -The registry does not contain type, parent, description, summary, coordinates, -participants, visit status, or occurrence data. Parent and child places are -separate identities when the transcript identifies both; nesting alone is not -a reason to merge them. - -### Location occurrences - -The occurrence lane uses: - -- artifact kind: `dnd/location-occurrence-list` -- module key: `dnd/location-occurrences` -- schema ID: `notarius.dnd.location_occurrences` -- schema name: `notarius_dnd_location_occurrences_v1` -- media type: `application/json` -- root member: `occurrences` - -Each occurrence contains exactly: - -| Field | Type | Meaning | -| --- | --- | --- | -| `location_id` | string | An exact ID from the consumed normalized location registry. | -| `name` | string | The canonical display name associated with `location_id`. | -| `kind` | enum | `visited`, `planned`, `recalled`, or `mentioned`. | -| `source_refs` | non-empty source-reference array | Current-transcript evidence for both the place and the classified occurrence. | - -`location_id` is required even though existing NPC interactions currently use -name-only grounding. Locations can legitimately share the same display name, -so a name alone cannot provide an unambiguous cross-artifact link. The name is -retained for readable standalone output and must exactly match the registry -record selected by the ID after normalization. - -## Identity Policy - -Location identity is conservative and scoped to one source document. It is not -a campaign-wide or cross-session world identity. - -Display normalization trims surrounding whitespace and collapses internal -Unicode whitespace. Comparison normalization uses the existing D&D entity -rules: Unicode NFKC normalization, normalized apostrophes, collapsed -whitespace, and Unicode case folding. - -The canonical ID is: - -~~~text -location:sha256: -~~~ - -The digest input is the UTF-8 encoding of compact JSON for this five-element -array: - -~~~text -["dnd.locations.identity.v1", comparison_name, source_id, start_unit_id, end_unit_id] -~~~ - -The source values come from the earliest reference after canonical reference -sorting and exact deduplication. Compact JSON array encoding is part of the -identity contract: it avoids delimiter ambiguity and must not be replaced -without changing the policy version. A blank comparison name or missing valid -source reference produces no manufactured ID and remains a validation error. - -Including the evidence anchor prevents two unrelated places called `the -tavern` from receiving the same ID. When semantic normalization safely groups -aliases or repeated appearances, it first chooses an existing canonical display -name and unions the evidence; it then derives the final ID from that name and -the earliest unioned reference. - -The normalizer may merge records only when transcript evidence clearly shows -that they denote the same physical place. It must not merge records solely -because: - -- their comparison names are equal; -- they are near one another in the transcript; -- one is spatially nested inside the other; or -- their labels are both generic. - -Distinct normalized records may therefore have the same comparison name, but -their IDs must be unique and correctly derived. Exact duplicates with the same -comparison name and canonical evidence may be collapsed deterministically. - -## Occurrence Semantics - -Each occurrence has one kind: - -- `visited`: current-session gameplay establishes that one or more party - members are physically present at the location, including an arrival, - continuing presence, or departure. -- `planned`: the party explicitly proposes, intends, or agrees to future travel - to the location. Mere hypotheticals or speculation are not plans. -- `recalled`: the transcript explicitly recounts or recaps the party being at - the location before the current session's live events. -- `mentioned`: the location is explicitly referenced but the occurrence does - not meet a stronger definition. This includes lore, directions, third-party - activity, non-actionable speculation, and out-of-character discussion. - -An inferred but unstated place produces no location or occurrence. Uncertainty -is handled by conservative omission rather than an `uncertain` enum value. - -For one occurrence supported by overlapping evidence, classification precedence -is `visited`, then `planned`, then `recalled`, then `mentioned`; `mentioned` is -the fallback. A passage may produce multiple records when it independently -supports separate facts, such as recalling an earlier visit while planning a -return. Exact duplicates with the same ID, kind, and canonical evidence are -collapsed. Different kinds or independently supported evidence remain. - -Output is ordered by earliest evidence in source-document order, then by -`location_id`, `name`, kind order (`visited`, `planned`, `recalled`, -`mentioned`), and the remaining canonical reference sequence. - -## Extraction, Normalization, And Evidence - -### Location registry lane - -The extractor is LLM-backed and follows the shared D&D extraction prompt and -input conventions. It emits names and source ranges through a private response -schema; deterministic mapping supplies the current source ID and derives -candidate IDs. Campaign references may disambiguate terminology but never -become durable evidence. - -The merger uses the typed append-order convention. The normalizer is LLM-backed: -it deterministically prepares names and evidence, then asks the model only for -duplicate groups. The model may identify groups and choose a canonical member, -but it may not create, delete, rewrite, or directly replace durable records. -Code validates the proposal, applies non-overlapping safe groups, unions -evidence, derives final IDs, orders output, and emits bounded warnings. - -Malformed, unknown, overlapping, or ambiguous proposal groups are rejected. -The normalizer uses the existing bounded retry behavior and falls back to the -safe deterministic candidate set if no usable proposal is obtained. - -### Location occurrence lane - -The extractor is LLM-backed and requires exactly one validated `locations` -reference. The prompt projection contains only ordered `{id, name}` pairs; it -omits registry evidence and reference provenance. The model must copy both -values from one projected record and cite current-transcript source ranges for -the occurrence. - -The occurrence normalizer is deterministic. It canonicalizes names by exact -registry ID, normalizes evidence and ordering, and removes exact duplicates. -Unknown IDs and mismatched ID/name pairs remain inspectable validation failures -rather than being guessed or silently reassigned. - -The occurrence lane cannot add a missing location to the registry. If the -location extractor omitted a place, the correct behavior is to omit its -occurrence and improve the upstream extraction later. - -## Shared Entity Reconciliation - -Adding a second LLM-assisted entity registry demonstrates a concrete shared -need in the D&D domain. The existing NPC normalization context-window and -proposal-safety logic will move to -`internal/modules/dnd/shared/entityreconcile` and serve both NPC and location -normalizers. - -The shared package owns: - -- deterministic opaque candidate keys; -- source-window construction and canonical prompt materials; -- a common private duplicate-group response contract; -- validation of unknown, repeated, overlapping, malformed, or ineligible - candidate keys; and -- immutable assessment results identifying safe groups. - -It does not call the LLM, choose domain-specific canonical names, derive -durable IDs, mutate domain artifacts, or format domain warnings. Those -responsibilities remain in each normalizer. - -NPC normalization will migrate to the shared key-based proposal contract -without changing its durable NPC behavior. Its prompt and private response -schema fingerprints are expected to change, so stale NPC normalization -checkpoints will invalidate normally. - -The two normalizers will reuse an exactly identical shared reconciliation -instruction asset and private response schema. Module-owned task text will -continue to define the different NPC and location identity rules. This keeps -shared prompt content identical without pretending the two domains have the -same semantic merge policy. - -This helper is intentionally D&D-specific. It does not implement the broader -domain-neutral replacement-element normalizer still described in -[future work](future.md). - -## Reference Contract And Pipeline Placement - -The generated reference slot is named `locations` and accepts exactly one JSON -artifact of kind `dnd/location-list`, with the established 1 MiB limit. It is -required by both extraction and normalization for -`dnd/location-occurrences`. Static file bindings remain valid where the -framework permits them, but the maintained complete example uses a generated -same-run artifact. - -The complete D&D pipeline places `locations` in the first descriptive step -alongside the independent NPC, item-event, and scene-description lanes. It -places `location-occurrences` in the next step and binds the accepted normalized -location artifact from the first step. The occurrence lane has no mandatory -NPC or scene-description dependency. - -No current downstream lane is changed to consume location artifacts. Future -narrative reports or joins may use the canonical IDs after defining their own -contracts. - -## Validation And Production Defaults - -The location registry receives production validators for: - -- required shape and supported ID syntax; -- current-document and current-chunk source ranges; -- normalized identity derivation and ID uniqueness; and -- advisory source relatedness. - -The location occurrence artifact receives production validators for: - -- required shape and the four supported kinds; -- registry membership and exact ID/name correspondence; -- normalized ordering and exact-duplicate invariants; -- current-document and current-chunk source ranges; and -- advisory source relatedness. - -Validators remain immutable and diagnostic. Durable JSON Schema validation -stays in the production chains after semantic shape and source-reference -checks, consistent with the existing D&D lanes. - -## Documentation End State - -Implementation will add canonical integration documents for both durable -artifacts and update current-state documentation to cover: - -- both module and artifact keys; -- the `locations` generated-reference slot; -- production validators and default chains; -- D&D family composition, reconciliation, identity, and grounding behavior; -- the complete maintained pipeline example; and -- JSON output and evidence-context support. - -After the feature is implemented, the fulfilled Location Extraction section is -removed from `future.md`. Historical implementation narration remains in -version control rather than current documentation. - -## Out Of Scope - -- Campaign-wide or cross-session canonical location IDs. -- A location ontology, hierarchy, map, coordinates, or containment graph. -- Location descriptions, summaries, participants, ownership, or encounter - analysis. -- Inferring a location that the transcript does not identify. -- Automatically creating registry records from occurrence output. -- Changing NPC-interaction artifacts to use NPC IDs. -- Making other lanes consume location references. -- A generic domain-neutral LLM deduplication framework. -- Long-term artifact-version migration machinery. - -## Acceptance Criteria - -- Both durable contracts are minimal, strict, versioned, and registered. -- Location IDs are deterministic under the documented policy and do not force - same-named places to collapse. -- Alias and repeat reconciliation is proposal-only, conservatively validated, - and safe on retry exhaustion. -- Occurrences use one of the four defined kinds and carry an unambiguous - registry ID/name pair plus current-transcript evidence. -- Missing, malformed, oversized, or incompatible `locations` references fail - through the established configuration or operation boundaries. -- Both lanes have typed mergers, normalizers, evidence projectors, validators, - default chains, prompt/profile metadata, and registration coverage consistent - with the D&D family. -- NPC normalization retains its durable behavior after adopting the shared - reconciliation helper. -- The maintained complete example loads and exercises the generated handoff. -- Focused D&D and integration tests pass offline, and current documentation - describes only implemented behavior once the work is complete. diff --git a/internal/modules/dnd/locations/registry/registry_test.go b/internal/modules/dnd/locations/registry/registry_test.go index b6cce41..9b0b648 100644 --- a/internal/modules/dnd/locations/registry/registry_test.go +++ b/internal/modules/dnd/locations/registry/registry_test.go @@ -2,9 +2,7 @@ package registry import ( "bytes" - "fmt" "strings" - "sync" "testing" "gitea.maximumdirect.net/eric/notarius/internal/core/source" @@ -100,26 +98,19 @@ func TestResolveRejectsInvalidReferenceInputs(t *testing.T) { } } -func TestResolverValidatesStaticAndOperationReferences(t *testing.T) { +func TestResolverHandlesConstructionAndOperationReferences(t *testing.T) { placeholder, err := NewResolver(referenceSet()) if err != nil || placeholder.Seeded().Bound() { t.Fatalf("generated placeholder = %#v, %v; want unbound seed", placeholder, err) } validContent := encodeList(t, registryFixture()) - invalidSets := []contracts.ReferenceSet{ - referenceSet(item([]byte(`{"locations":[`))), - referenceSet(contracts.ReferenceItem{MediaType: "text/plain", Content: validContent}), - referenceSet(item(make([]byte, MaxBytes+1))), - referenceSet(item(validContent), item(validContent)), + malformed := referenceSet(item([]byte(`{"locations":[`))) + if _, err := NewResolver(malformed); err == nil { + t.Fatal("NewResolver(malformed) error = nil") } - 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) - } + if _, err := placeholder.Resolve(malformed); err == nil { + t.Fatal("Resolve(malformed) error = nil") } staticContent := append([]byte(nil), validContent...) @@ -137,9 +128,12 @@ func TestResolverValidatesStaticAndOperationReferences(t *testing.T) { if seeded.Seeded().Count() != 2 || seeded.Seeded().CanonicalBytes()[0] != '{' { t.Fatalf("seeded registry retained construction references: %#v", seeded.Seeded()) } + if fallback, err := seeded.Resolve(contracts.ReferenceSet{}); err != nil || fallback != seeded.Seeded() { + t.Fatalf("fallback = %#v, %v; want seeded registry", fallback, err) + } } -func TestResolverCachesEquivalentRegistriesConcurrentlyAndIgnoresCallerDigest(t *testing.T) { +func TestResolverReusesEquivalentCanonicalRegistries(t *testing.T) { resolver, err := NewResolver(referenceSet()) if err != nil { t.Fatal(err) @@ -153,57 +147,11 @@ func TestResolverCachesEquivalentRegistriesConcurrentlyAndIgnoresCallerDigest(t if err != nil { t.Fatal(err) } - second, err := resolver.Resolve(firstSet) - if err != nil || first != second { - t.Fatalf("raw cache Resolve() = %p, %p, %v", first, second, err) - } spaced := append([]byte("\n "), content...) spaced = append(spaced, '\n') - third, err := resolver.Resolve(referenceSet(contracts.ReferenceItem{MediaType: "APPLICATION/JSON; charset=utf-8", Content: spaced})) - if err != nil || third != first { - t.Fatalf("semantic cache Resolve() = %p, %p, %v", first, third, err) - } - - var group sync.WaitGroup - errs := make(chan error, 24) - for range 24 { - group.Add(1) - go func() { - defer group.Done() - resolved, err := resolver.Resolve(firstSet) - if err != nil || resolved != first { - errs <- fmt.Errorf("resolved %p, want %p: %w", resolved, first, err) - } - }() - } - group.Wait() - close(errs) - for err := range errs { - t.Error(err) - } - - sharedDigest := "sha256:" + strings.Repeat("0", 64) - firstItem := contracts.ReferenceItem{MediaType: locationcodec.MediaType, Content: content, Digest: sharedDigest} - otherList := registryFixture() - otherList.Locations[0].Name = "Moon Gate" - otherList.Locations[0].ID = identity.DeriveID(otherList.Locations[0].Name, otherList.Locations[0].SourceRefs) - otherContent := encodeList(t, otherList) - otherItem := contracts.ReferenceItem{MediaType: locationcodec.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) - } - - firstSet.Slots[ReferenceSlot].Items[0].Content[0] = '[' - if got, ok := first.Lookup(registryFixture().Locations[0].ID); !ok || got.Name != "The Tavern" { - t.Fatalf("cached registry retained caller bytes: %#v, %t", got, 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) + second, err := resolver.Resolve(referenceSet(contracts.ReferenceItem{MediaType: "APPLICATION/JSON; charset=utf-8", Content: spaced})) + if err != nil || second != first { + t.Fatalf("equivalent canonical registry = %p / %p, %v", first, second, err) } } diff --git a/internal/modules/dnd/normalize/locationoccurrences/normalizer_test.go b/internal/modules/dnd/normalize/locationoccurrences/normalizer_test.go index f41951d..f5eab31 100644 --- a/internal/modules/dnd/normalize/locationoccurrences/normalizer_test.go +++ b/internal/modules/dnd/normalize/locationoccurrences/normalizer_test.go @@ -101,20 +101,8 @@ func TestNormalizerContractsRequiredRegistryAndWarningBounds(t *testing.T) { if spec.Key != Key || spec.Stage != pipeline.StageNormalize || spec.ExecutionClass != contracts.ExecutionClassDeterministic || spec.ArtifactKind != dnd.LocationOccurrenceListKind { t.Fatalf("ModuleSpec() = %#v", spec) } - wantSlots := []contracts.ReferenceSlot{{ - Name: LocationRegistryReferenceSlot, - Description: "Required normalized location registry used only for location identity grounding, never as occurrence evidence.", - Required: true, - AcceptedMediaTypes: []string{"application/json"}, - AcceptedArtifactKinds: []contracts.ArtifactKind{dnd.LocationListKind}, - MaxBytes: LocationRegistryMaxBytes, - }} - if !reflect.DeepEqual(spec.ReferenceSlots, wantSlots) { - t.Fatalf("ModuleSpec().ReferenceSlots = %#v, want %#v", spec.ReferenceSlots, wantSlots) - } - if got := normalizer.ReferenceSlots(); !reflect.DeepEqual(got, wantSlots) { - t.Fatalf("ReferenceSlots() = %#v, want %#v", got, wantSlots) - } + assertLocationRegistryReferenceSlots(t, "ModuleSpec", spec.ReferenceSlots) + assertLocationRegistryReferenceSlots(t, "Normalizer", normalizer.ReferenceSlots()) registry := pipeline.NewNormalizerRegistry() if err := Register(registry); err != nil { t.Fatal(err) @@ -123,9 +111,7 @@ func TestNormalizerContractsRequiredRegistryAndWarningBounds(t *testing.T) { if !ok { t.Fatalf("registry missing %q", Key) } - if !reflect.DeepEqual(registeredSpec.ReferenceSlots, wantSlots) { - t.Fatalf("registered reference slots = %#v, want %#v", registeredSpec.ReferenceSlots, wantSlots) - } + assertLocationRegistryReferenceSlots(t, "registered ModuleSpec", registeredSpec.ReferenceSlots) if _, err := DecodeOptions(map[string]any{"unexpected": true}); err == nil { t.Fatal("DecodeOptions() accepted unknown options") } @@ -153,6 +139,26 @@ func TestNormalizerContractsRequiredRegistryAndWarningBounds(t *testing.T) { } } +func assertLocationRegistryReferenceSlots(t *testing.T, owner string, slots []contracts.ReferenceSlot) { + t.Helper() + if len(slots) != 1 { + t.Fatalf("%s reference slots = %#v, want one location registry", owner, slots) + } + got := slots[0] + description := got.Description + got.Description = "" + want := contracts.ReferenceSlot{ + Name: LocationRegistryReferenceSlot, + Required: true, + AcceptedMediaTypes: []string{"application/json"}, + AcceptedArtifactKinds: []contracts.ArtifactKind{dnd.LocationListKind}, + MaxBytes: LocationRegistryMaxBytes, + } + if !reflect.DeepEqual(got, want) || strings.TrimSpace(description) == "" { + t.Fatalf("%s location registry slot = %#v, want contract %#v with a nonempty description", owner, slots[0], want) + } +} + func newNormalizer(t *testing.T, references ...contracts.ReferenceSet) *Normalizer { t.Helper() normalizer, err := New(Options{}, references...) diff --git a/internal/modules/dnd/npcs/registry/registry_test.go b/internal/modules/dnd/npcs/registry/registry_test.go index 99ad6cb..3be019e 100644 --- a/internal/modules/dnd/npcs/registry/registry_test.go +++ b/internal/modules/dnd/npcs/registry/registry_test.go @@ -2,10 +2,8 @@ package registry import ( "bytes" - "fmt" "reflect" "strings" - "sync" "testing" "gitea.maximumdirect.net/eric/notarius/internal/core/source" @@ -114,17 +112,27 @@ func TestResolveRejectsMalformedOrUnsupportedRegistryInput(t *testing.T) { func TestResolverReusesEquivalentCanonicalRegistries(t *testing.T) { set := listReferenceSet(t, registryFixture()) - resolver, err := NewResolver(set) + resolver, err := NewResolver(contracts.ReferenceSet{}) if err != nil { t.Fatal(err) } - resolved, err := resolver.Resolve(set) - if err != nil || resolved != resolver.Seeded() { - t.Fatalf("Resolve() = %p, %v; seeded %p", resolved, err, resolver.Seeded()) + first, err := resolver.Resolve(set) + if err != nil { + t.Fatal(err) + } + content := set.Slots[ReferenceSlot].Items[0].Content + equivalent := append([]byte("\n "), content...) + equivalent = append(equivalent, '\n') + second, err := resolver.Resolve(referenceSet(contracts.ReferenceItem{ + MediaType: "APPLICATION/JSON; charset=utf-8", + Content: equivalent, + })) + if err != nil || second != first { + t.Fatalf("equivalent canonical registry = %p / %p, %v", first, second, err) } } -func TestResolverValidatesStaticAndOperationReferences(t *testing.T) { +func TestResolverHandlesConstructionAndOperationReferences(t *testing.T) { placeholder, err := NewResolver(referenceSet()) if err != nil || placeholder.Seeded().Bound() { t.Fatalf("generated placeholder = %#v, %v; want unbound seed", placeholder, err) @@ -132,22 +140,12 @@ func TestResolverValidatesStaticAndOperationReferences(t *testing.T) { 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}, - ), + malformed := referenceSet(contracts.ReferenceItem{MediaType: npccodec.MediaType, Content: []byte(`{"npcs":[`)}) + if _, err := NewResolver(malformed); err == nil { + t.Fatal("NewResolver(malformed) error = nil") } - 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) - } + if _, err := placeholder.Resolve(malformed); err == nil { + t.Fatal("Resolve(malformed) error = nil") } staticContent := append([]byte(nil), validContent...) @@ -161,71 +159,8 @@ func TestResolverValidatesStaticAndOperationReferences(t *testing.T) { 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) + if fallback, err := seeded.Resolve(contracts.ReferenceSet{}); err != nil || fallback != seeded.Seeded() { + t.Fatalf("fallback = %#v, %v; want seeded registry", fallback, err) } } diff --git a/internal/modules/dnd/register/register_test.go b/internal/modules/dnd/register/register_test.go index 480570c..dcdd78a 100644 --- a/internal/modules/dnd/register/register_test.go +++ b/internal/modules/dnd/register/register_test.go @@ -426,7 +426,7 @@ func TestRegisterAddsDNDFamily(t *testing.T) { if len(occurrenceExtractSpec.ReferenceSlots) != 5 || len(occurrenceNormalizeSpec.ReferenceSlots) != 1 { t.Fatalf("location occurrence reference slots = %#v / %#v, want extractor campaign context and normalizer registry only", occurrenceExtractSpec.ReferenceSlots, occurrenceNormalizeSpec.ReferenceSlots) } - if !locationRegistrySlot.Required || !reflect.DeepEqual(locationRegistrySlot.AcceptedMediaTypes, []string{"application/json"}) || !reflect.DeepEqual(locationRegistrySlot.AcceptedArtifactKinds, []contracts.ArtifactKind{dnd.LocationListKind}) || locationRegistrySlot.MaxBytes != 1048576 || !reflect.DeepEqual(locationRegistrySlot, occurrenceNormalizeRegistrySlot) { + if !locationRegistrySlot.Required || !reflect.DeepEqual(locationRegistrySlot.AcceptedMediaTypes, []string{"application/json"}) || !reflect.DeepEqual(locationRegistrySlot.AcceptedArtifactKinds, []contracts.ArtifactKind{dnd.LocationListKind}) || locationRegistrySlot.MaxBytes != 1048576 || !sameReferenceSlotContract(locationRegistrySlot, occurrenceNormalizeRegistrySlot) { t.Fatalf("location registry slots disagree: %#v / %#v", occurrenceExtractSpec.ReferenceSlots, occurrenceNormalizeSpec.ReferenceSlots) } for _, name := range []string{"party", "roster", "players", "glossary"} { @@ -445,7 +445,7 @@ func TestRegisterAddsDNDFamily(t *testing.T) { } extractRegistrySlot := referenceSlot(interactionExtractSpec.ReferenceSlots, "npcs") normalizeRegistrySlot := referenceSlot(interactionNormalizeSpec.ReferenceSlots, "npcs") - if !extractRegistrySlot.Required || !reflect.DeepEqual(extractRegistrySlot.AcceptedArtifactKinds, []contracts.ArtifactKind{dnd.NPCListKind}) || !reflect.DeepEqual(extractRegistrySlot, normalizeRegistrySlot) { + if !extractRegistrySlot.Required || !reflect.DeepEqual(extractRegistrySlot.AcceptedArtifactKinds, []contracts.ArtifactKind{dnd.NPCListKind}) || !sameReferenceSlotContract(extractRegistrySlot, normalizeRegistrySlot) { t.Fatalf("NPC interaction registry slots disagree: %#v / %#v", interactionExtractSpec.ReferenceSlots, interactionNormalizeSpec.ReferenceSlots) } } @@ -508,6 +508,12 @@ func referenceSlot(slots []contracts.ReferenceSlot, name string) contracts.Refer return contracts.ReferenceSlot{} } +func sameReferenceSlotContract(first, second contracts.ReferenceSlot) bool { + first.Description = "" + second.Description = "" + return reflect.DeepEqual(first, second) +} + func TestAppendNPCListsPreservesOrderAndArrayPresence(t *testing.T) { tests := []struct { name string diff --git a/internal/modules/dnd/shared/registryresolver/resolver.go b/internal/modules/dnd/shared/registryresolver/resolver.go index a6aaff4..d436717 100644 --- a/internal/modules/dnd/shared/registryresolver/resolver.go +++ b/internal/modules/dnd/shared/registryresolver/resolver.go @@ -28,6 +28,13 @@ type Item struct { Content []byte } +// validatedItem borrows its content from the supplied reference set. Callers +// must copy content before passing it to a callback that may retain it. +type validatedItem struct { + mediaType string + content []byte +} + // Config supplies the domain-owned operations needed to prepare immutable // registry views. Absent and Load must return values whose mutable state is not // exposed to callers. Load receives owned bytes and may retain them. Errors @@ -111,7 +118,7 @@ func (r *Resolver[V]) Resolve(references contracts.ReferenceSet) (V, error) { return r.seeded.value, nil } - item, _, err := resolveOptionalSingleItem(references, r.config.Reference, r.mediaType) + item, _, err := validateOptionalSingleItem(references, r.config.Reference, r.mediaType) if err != nil { var zero V return zero, err @@ -124,7 +131,7 @@ func (r *Resolver[V]) Resolve(references contracts.ReferenceSet) (V, error) { return cached.value, nil } - resolved, err := r.load(item.Content) + resolved, err := r.load(append([]byte(nil), item.content...)) if err != nil { var zero V return zero, err @@ -147,23 +154,30 @@ func (r *Resolver[V]) Resolve(references contracts.ReferenceSet) (V, error) { // ResolveOptionalSingleItem validates and copies one optional registry item. // A missing slot returns present=false. A present slot must contain exactly one // item, even when it represents an operation-time generated reference. -func ResolveOptionalSingleItem(references contracts.ReferenceSet, spec ReferenceSpec) (item Item, present bool, err error) { +func ResolveOptionalSingleItem(references contracts.ReferenceSet, spec ReferenceSpec) (Item, bool, error) { normalized, mediaType, err := normalizeReferenceSpec(spec) if err != nil { return Item{}, false, err } - return resolveOptionalSingleItem(references, normalized, mediaType) + item, present, err := validateOptionalSingleItem(references, normalized, mediaType) + if err != nil || !present { + return Item{}, present, err + } + return Item{ + MediaType: item.mediaType, + Content: append([]byte(nil), item.content...), + }, true, nil } func (r *Resolver[V]) resolveUncached(references contracts.ReferenceSet) (preparedView[V], error) { - item, present, err := resolveOptionalSingleItem(references, r.config.Reference, r.mediaType) + item, present, err := validateOptionalSingleItem(references, r.config.Reference, r.mediaType) if err != nil { return preparedView[V]{}, err } if !present { return r.absent() } - return r.load(item.Content) + return r.load(append([]byte(nil), item.content...)) } func (r *Resolver[V]) absent() (preparedView[V], error) { @@ -221,32 +235,32 @@ func normalizeReferenceSpec(spec ReferenceSpec) (ReferenceSpec, string, error) { return spec, mediaType, nil } -func resolveOptionalSingleItem(references contracts.ReferenceSet, spec ReferenceSpec, acceptedMediaType string) (Item, bool, error) { +func validateOptionalSingleItem(references contracts.ReferenceSet, spec ReferenceSpec, acceptedMediaType string) (validatedItem, bool, error) { slot, present := references.Slots[spec.SlotName] if !present { - return Item{}, false, nil + return validatedItem{}, false, nil } if len(slot.Items) != 1 { - return Item{}, true, fmt.Errorf("reference slot %q must contain exactly one item", spec.SlotName) + return validatedItem{}, true, fmt.Errorf("reference slot %q must contain exactly one item", spec.SlotName) } item := slot.Items[0] mediaType, _, err := mime.ParseMediaType(item.MediaType) if err != nil { - return Item{}, true, fmt.Errorf("reference slot %q item media type is invalid", spec.SlotName) + return validatedItem{}, true, fmt.Errorf("reference slot %q item media type is invalid", spec.SlotName) } mediaType = strings.ToLower(mediaType) if !strings.EqualFold(mediaType, acceptedMediaType) { - return Item{}, true, fmt.Errorf("reference slot %q item media type must be %s", spec.SlotName, acceptedMediaType) + return validatedItem{}, true, fmt.Errorf("reference slot %q item media type must be %s", spec.SlotName, acceptedMediaType) } if int64(len(item.Content)) > spec.MaxBytes { - return Item{}, true, fmt.Errorf("reference slot %q item is %d bytes, limit %d", spec.SlotName, len(item.Content), spec.MaxBytes) + return validatedItem{}, true, fmt.Errorf("reference slot %q item is %d bytes, limit %d", spec.SlotName, len(item.Content), spec.MaxBytes) } - return Item{MediaType: mediaType, Content: append([]byte(nil), item.Content...)}, true, nil + return validatedItem{mediaType: mediaType, content: item.Content}, true, nil } -func rawReferenceKey(item Item) string { - sum := sha256.Sum256(item.Content) - return item.MediaType + "\x00sha256:" + hex.EncodeToString(sum[:]) +func rawReferenceKey(item validatedItem) string { + sum := sha256.Sum256(item.content) + return item.mediaType + "\x00sha256:" + hex.EncodeToString(sum[:]) } func sameIdentity[V any](first, second preparedView[V]) bool {