From 3589d3346802a7b12b7b66ebca594be92b1d95c7 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 8 Aug 2026 22:08:24 +0000 Subject: [PATCH] Audit shared D&D family conventions --- docs/roadmap/audit.md | 183 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 177 insertions(+), 6 deletions(-) diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md index b90f307..d9830dc 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -20,11 +20,12 @@ change only roadmap audit documents do not change that production target. Pending final synthesis. The initial baseline is healthy. The architecture, configuration/CLI, pipeline composition, reference/handoff, runtime, state, -LLM, and generic/Seriatim reviews have found three High findings, four Medium -findings, and fifteen Low findings, with no production dependency inversion, -unbounded framework worker pool, completion-order-dependent result assembly, -debug-to-cache coupling, model-visible credential material in the embedded LLM -assets, or domain leakage across the Seriatim and generic module boundaries. +LLM, generic/Seriatim, and shared D&D reviews have found three High findings, +four Medium findings, and sixteen Low findings, with no production dependency +inversion, unbounded framework worker pool, completion-order-dependent result +assembly, debug-to-cache coupling, model-visible credential material in the +embedded LLM assets, or domain leakage across the Seriatim and generic module +boundaries. ## Finding Index @@ -54,6 +55,7 @@ Final cross-area ordering is pending synthesis. | MOD-001 | Low | Simplicity | Narrow generic integer option decoding | | MOD-002 | Low | Efficiency | Reuse compiled response schemas within a prepared validator | | MOD-003 | Low | Simplicity | Remove unreachable JSON metadata clone helpers | +| DND-CORE-001 | Low | Simplicity | Remove the unused lossy unit-reference constructor | ## Findings @@ -816,6 +818,37 @@ Final cross-area ordering is pending synthesis. confirm no production caller is removed with graph and compiler checks. - **Grouping:** Independent. +### Shared D&D Types, Codecs, And Family Mechanics + +### DND-CORE-001 — Remove the unused lossy unit-reference constructor + +- **Severity:** Low +- **Category:** Simplicity +- **Evidence:** `internal/modules/dnd/shared/unit_refs.go:24`–`27` exports + `UnitRefFromString`, discards the error from `parseUnitRefNumber`, and returns + a zero-valued `UnitRef` for every blank, whitespace-padded, non-integer, or + non-positive value. Knowledge-graph inbound-call tracing and a scoped source + search found no production or test caller. The live external boundary is + `UnitRef.UnmarshalJSON` at lines 47–71, which calls the same parser and + correctly propagates its error; `UnitRefFromInt` remains used by that decoder + and focused tests. +- **Impact:** The exported helper advertises a supported string-construction + path whose only distinguishing behavior is to erase invalid-input context. + A future caller could turn malformed model output or test data into a later, + less specific `must be positive` error, while the dead API adds maintenance + surface beside the authoritative strict decoder. +- **Recommendation:** Delete `UnitRefFromString`. If a non-JSON string boundary + later needs construction, expose a parsing function that returns + `(UnitRef, error)` and reuse `parseUnitRefNumber` without discarding errors. +- **Preserve:** Keep string-or-integer JSON compatibility, exact positive source + unit IDs rather than ordinal fallback, original string-versus-number JSON + rendering, and the contextual checks in `ResolveUnitID`. +- **Validation:** Confirm graph and compiler checks find no removed caller; + retain the string/integer/malformed JSON and exact-ID tests in + `internal/modules/dnd/shared/unit_refs_test.go`; run + `go test ./internal/modules/dnd/shared/...`. +- **Grouping:** Independent. + +## D&D Convention Matrix + +The matrix records the current production convention before the lane-specific +reviews. `E`, `M`, and `N` mean extract, merge, and normalize. Every extractor +is LLM-backed, every merger is the deterministic typed `appendorder` merger, +and every family also registers the typed deterministic `noop` normalizer as an +explicit alternate. `B` is the ordered default chain +`valid_json → shape → source_refs → valid_json_schema → source_relatedness`; +notations such as `B + identity` name the check inserted after `shape` and +before `source_refs`. `C` means the optional campaign-context slots `glossary`, +`party`, `players`, and deprecated `roster`. Every durable codec is +family-owned, uses media type `application/json`, embeds its own v1 durable +schema, publishes count metadata, and has a registered direct-source-reference +evidence projector. + +| Family | Durable kind / Go type | Modules and execution class | Default validator chains (E / N) | Reference dependencies (E / N) | Codec, prompt, and schema ownership | Documented exception and later confirmation | +| --- | --- | --- | --- | --- | --- | --- | +| Spells | `dnd/spell-list` / `dnd.SpellList` | `dnd/spells` (LLM) → typed `appendorder` (deterministic) → `dnd/spells` (deterministic) | `B + catalog` / `B + catalog` | `C` plus optional `spell_catalog` and `npc_registry` / optional `spell_catalog` | `codec/spells`; extractor owns its prompt and private response schema; normalizer has no prompt | Catalog overlay and NPC caster grounding are optional and never evidence; confirm spell/catalog and scene-family behavior in the spells/scenes review. | +| NPC registry | `dnd/npc-registry` / `dnd.NPCRegistry` | `dnd/npc-registry` (LLM) → typed `appendorder` (deterministic) → `dnd/npc-registry` (LLM) | `B` / `B + identity` | `C` / none | `codec/npcregistry`; extractor and normalizer own prompts; extractor owns its response schema, while normalize uses the shared private entity-reconciliation schema | Normalization is proposal-only LLM reconciliation; confirm identity, merge safety, and same-name policy in the registry review. | +| Combat turns | `dnd/combat-turn-list` / `dnd.CombatTurnList` | `dnd/combat-turns` (LLM) → typed `appendorder` (deterministic) → `dnd/combat-turns` (deterministic) | `B` / `B + invariants` | `C`, optional `npc_registry`, required `scene_descriptions` / optional `npc_registry` | `codec/combatturns`; extractor owns its prompt and private response schema; normalizer has no prompt | Scene descriptions gate LLM execution and NPC grounding is not evidence; confirm gate and empty-result semantics in the combat/enemy review. | +| Item occurrences | `dnd/item-occurrence-list` / `dnd.ItemOccurrenceList` | `dnd/item-occurrences` (LLM) → typed `appendorder` (deterministic) → `dnd/item-occurrences` (deterministic) | `B + registry` / `B + registry + invariants` | `C` plus required `item_registry` / required `item_registry` | `codec/itemoccurrences`; extractor owns its prompt and private response schema; normalizer has no prompt | Registry identity grounds current-transcript facts but never supplies evidence; confirm quantities, holders, and registry projection in the occurrence review. | +| Item registry | `dnd/item-registry` / `dnd.ItemRegistry` | `dnd/item-registry` (LLM) → typed `appendorder` (deterministic) → `dnd/item-registry` (LLM) | `B` / `B + identity` | `C` / none | `codec/itemregistry`; extractor and normalizer own prompts; extractor owns its response schema, while normalize uses the shared private entity-reconciliation schema | Normalization is proposal-only and preserves denominations/types rather than inventing instances; confirm identity and merge policy in the registry review. | +| NPC occurrences | `dnd/npc-occurrence-list` / `dnd.NPCOccurrenceList` | `dnd/npc-occurrences` (LLM) → typed `appendorder` (deterministic) → `dnd/npc-occurrences` (deterministic) | `B + registry` / `B + registry + invariants` | `C` plus required `npc_registry` / `C` plus required `npc_registry` | `codec/npcoccurrences`; extractor owns its prompt and private response schema; normalizer has no prompt | Registry provenance cannot become occurrence evidence and `mentioned` remains a factual category; confirm category/identity handling in the occurrence review. | +| Scene descriptions | `dnd/scene-description-list` / `dnd.SceneDescriptionList` | `dnd/scene-descriptions` (LLM) → typed `appendorder` (deterministic) → `dnd/scene-descriptions` (deterministic) | `B` / `B + invariants` | optional `glossary`, `party`, and `players` / none | `codec/scenedescriptions`; extractor owns its prompt and private response schema; normalizer has no prompt | Each record has one `source_ref` rather than a slice; the separate `dnd/scenes` LLM chunker owns the full-transcript scene prompt. Confirm scene IDs, ordering, and classification in the spells/scenes review. | +| Enemy events | `dnd/enemy-event-list` / `dnd.EnemyEventList` | `dnd/enemy-events` (LLM) → typed `appendorder` (deterministic) → `dnd/enemy-events` (deterministic) | `B + engagements` / `B + invariants` | `C` plus required `npc_registry`, `scene_descriptions`, `combat_turns`, and `npc_occurrences` / required `npc_registry` | `codec/enemyevents`; extractor owns its prompt and private response schema; normalizer has no prompt | Four generated artifacts ground extraction without becoming event evidence; durable decode separately proves required JSON-field presence. Confirm combat gating, engagement uniqueness, and observation ordering in the combat/enemy review. | +| Location registry | `dnd/location-registry` / `dnd.LocationRegistry` | `dnd/location-registry` (LLM) → typed `appendorder` (deterministic) → `dnd/location-registry` (LLM) | `B` / `B + identity` | `C` / none | `codec/locationregistry`; extractor and normalizer own prompts; extractor owns its response schema, while normalize uses the shared private entity-reconciliation schema | Normalization is proposal-only and same-name locations require contextual evidence; confirm identity and merge policy in the registry review. | +| Location occurrences | `dnd/location-occurrence-list` / `dnd.LocationOccurrenceList` | `dnd/location-occurrences` (LLM) → typed `appendorder` (deterministic) → `dnd/location-occurrences` (deterministic) | `B + registry` / `B + registry + invariants` | `C` plus required `location_registry` / required `location_registry` | `codec/locationoccurrences`; extractor owns its prompt and private response schema; normalizer has no prompt | Registry grounding cannot become evidence and speculation remains distinct from unsupported inference; confirm category and identity handling in the occurrence review. | + +Shared convention review classified the codec surface as safe typed adapters, +not a missing artifact-codec framework. `candidatejson` is the natural owner for +single-value JSON encoding and strict decoding (including unknown-field and +trailing-value rejection), while each codec retains exact Go type, kind, +metadata, embedded durable schema, and artifact-specific required-field or +semantic validation. Candidate encode/decode clones are limited to the two +families with retained pointer/slice ownership (`ItemOccurrenceList` and +`EnemyEventList`); schema bytes are owned because every `embed.FS.ReadFile` +call returns a fresh byte slice. The generic candidate helper intentionally +does not absorb family validation or durable-schema policy. + +Source-reference responsibilities are likewise separated by semantics: +`SourceRefOrder` owns document-position ordering, exact equality-based +deduplication, invalid-reference preservation for diagnostics, and nil versus +owned-empty behavior; `UnitRef.UnmarshalJSON` owns strict model-facing string +or integer parsing; `CitationResolver` owns current-document range expansion; +family validators own admissibility and evidence rules; and evidence projectors +copy only direct artifact references. Extractor-local response adapters remain +typed because their private response DTOs differ; replacing them with +reflection or callbacks would hide field ownership without consolidating +policy. DND-CORE-001 records the one dead constructor outside these live paths. + +Registration is explicit and complete for all ten codecs, extractors, mergers, +normalizers, noop alternates, evidence projectors, validator capabilities, and +default chains, plus the separate scene chunker. Prompt registration covers +every LLM extractor, the scene chunker, the three LLM registry normalizers, the +shared reconciliation schema, and the fallback profile. The registrar validates +every registry and the asset registry before mutation, registers in a fixed +order, and wraps failures with the exact D&D component name; a later failure +can leave the caller-supplied registries partially populated, but production +composition treats registration failure as terminal and no rollback contract +is documented. + +The shared registry resolver correctly distinguishes absent construction views, +empty generated-reference placeholders, operation-time overrides, raw-content +cache identity, and semantic identity while retaining owned loaded values. The +entity-reconciliation helper keeps durable IDs out of prompts, rejects invalid +or colliding selectors and overlapping groups, bounds transcript context, and +returns owned safe groups. Shared comparison policy is versioned, and shared +diagnostics bound displayed issues and warning counts. Domain-specific identity, +merge, category, and eligibility decisions remain assigned to the subsequent +registry, occurrence, scene/spell, and combat/enemy reviews. + +Finally, repeated `ManifestMetadata` and `CheckpointFingerprints` methods remain +module-local because their exact policy names, prompt/schema hashes, reference +projection digests, and ordering are checkpoint semantics. Small `Register`, +`New`, and `DecodeOptions` methods preserve typed registry construction, +module-local error context, dependency validation, and strict option surfaces; +parameterizing them would trade visible policy for callback-heavy helpers. The +shared prompt manifest/hash, candidate JSON, registry resolver, diagnostics, +source ordering, and entity reconciliation packages already own the exact +mechanics that recur without erasing those distinctions. + ## Intentional Complexity And Duplication To Preserve - `internal/modules/generic/register.Register`, @@ -1469,6 +1583,60 @@ Finding template for later audit stages: example contracts prove Seriatim input through generic JSON publication and opt-in chunk-map provenance. +### Shared D&D Types, Codecs, And Family Mechanics + +- **Durable family inventory:** `internal/modules/dnd/types.go`, the ten codec + packages, and D&D registration agree on ten exact kind/type variants. Each + has one LLM extractor, one typed append-order merger, a concrete normalizer, + a typed noop alternate, an evidence projector, artifact-specific validators, + and extract/normalize default chains. The convention matrix above records the + three intentionally LLM-backed registry normalizers and every reference + dependency; no missing or extra durable family registration was found. +- **Codec boundary:** All family codecs publish `application/json`, codec-local + v1 durable schemas and count metadata, strictly decode through the shared + candidate JSON helper, reject unknown fields and trailing values, and apply + family required-value validation only at durable encode/decode. Ownership + clones are present for the two mutable nested shapes that need them, and + schema reads return owned bytes. The common adapter surface is already at its + natural owner; a further generic codec would have to parameterize the exact + behavior the typed boundary is meant to expose. +- **Reference mechanics:** Shared campaign slots return owned media-type and + slot declarations; source ordering uses document positions with literal-ID + fallback, stable exact deduplication, and diagnostic preservation of invalid + refs. Strict unit-reference JSON parsing, current-document citation + expansion, typed family canonicalization, validator admissibility, and direct + evidence projection remain distinct owners. DND-CORE-001 records the only + dead/lossy API found in this layer. +- **Shared registry and reconciliation mechanics:** The generic registry + resolver validates one typed JSON artifact, bounds it by the family-declared + maximum, copies content at retention boundaries, and caches raw and semantic + identities behind a mutex. Entity reconciliation renders source-free + selectors and bounded transcript windows, keeps durable IDs model-invisible, + discards unsafe or overlapping proposals, and returns defensive copies. + Diagnostics and Unicode comparison policy are shared and bounded/versioned; + registry-specific identity and merge outcomes remain deferred. +- **Registration and assets:** The registrar validates all collaborators before + mutation and gives every failed component contextual identity. All ten + durable schemas, fourteen family prompt manifests, the shared reconciliation + schema, evidence projectors, generic validator capabilities, typed + always-accept/reject capabilities, default chains, and the fallback profile + are registered. Sequential registration is intentionally fail-stop rather + than transactional; production discards the composition on any error. +- **Repeated method classification:** Module-local metadata and fingerprint + methods enumerate semantic cache/checkpoint inputs; constructors and + registrars preserve exact dependencies and typed error context; strict empty + option decoders make supported surfaces locally visible. Existing shared + helpers already consolidate prompt assets, candidate JSON, reference slots, + comparison, diagnostics, resolution, and reconciliation. No additional + callback- or reflection-driven helper reduced demonstrated drift. +- **Deferred lane questions:** Registry identity and reconciliation policy is + assigned to the registry review; occurrence categories and registry + projections to the occurrence review; spell catalog, scene chunking, scene + IDs, and scene classification to the spells/scenes review; and combat gates, + engagement uniqueness, collective labels, and enemy observation ordering to + the combat/enemy review. This area therefore remains `Revisit` until those + reviews confirm the matrix's documented exceptions. + ## Validation Record | Date | Scope | Command or check | Result | @@ -1512,6 +1680,9 @@ Finding template for later audit stages: | 2026-08-08 | Published source-artifact codec tests | `go test ./internal/framework/chunkmap ./internal/framework/evidencecontext` | Pass | | 2026-08-08 | Composed evidence/output contracts | Focused `go test` runs in `./internal/framework/pipeline` and `./internal/cli` for evidence preparation/publication, maintained minimal JSON output, and production chunk-map provenance | Pass | | 2026-08-08 | Generic and Seriatim import boundaries | Direct `go list` import-edge audit plus scoped transcript-adapter import search | Pass; no D&D dependency in generic/Seriatim packages and no production import of the transcript adapter outside Seriatim registration | +| 2026-08-08 | Audit target integrity before shared D&D review | `git diff --quiet 92e89076a268089e703978fb9d7176200e93344c..HEAD -- . ':(exclude)docs/roadmap/**'` | Pass; production target unchanged | +| 2026-08-08 | Shared D&D graph/code review | Scoped architecture, exact symbol reads, inbound traces, and complete implementation/test inspection across ten durable types/codecs, candidate JSON, source-reference utilities, resolver/reconciliation/diagnostics, typed mergers, evidence, registration, default chains, shared assets, and representative family prompt/schema owners | DND-CORE-001 recorded; family registration, codec ownership, reference/evidence separation, shared reconciliation safety, and typed adapter boundaries otherwise confirmed | +| 2026-08-08 | Required shared D&D tests | `go test ./internal/modules/dnd/codec/... ./internal/modules/dnd/shared/... ./internal/modules/dnd/register` | Pass | ## Coverage Matrix @@ -1525,7 +1696,7 @@ Finding template for later audit stages: | State, checkpoints, debugging, and file safety | Reviewed | State and operations docs plus architecture state/security policy; CLI output, cache-root, checkpoint identity, recomputation, debug allocation, and terminal owners; core file I/O, debug bundle, and source digest/clone helpers; framework checkpoint, chunk-plan, chunk-map, debug, evidence-context implementations and focused tests | Target-integrity check, graph architecture/complexity/call traces, state collaborator race tests, CLI tests | STATE-001, STATE-002, STATE-003 | | LLM runtime, prompt filesystems, and assets | Reviewed | LLM internal/integration/configuration/operations docs and related ADRs; scheduled client, scheduler, PromptKit adapter/profile inspector, profile/source fingerprints, redaction, debug, asset registry, schema loader, promptfs implementations and focused tests; root assets, fallback profile, all fourteen module manifests and prompt YAML files, shared/model-visible fragments, private response schemas, and representative composed loaders | Target-integrity check, graph architecture/symbol/call review, content scan and exact prompt-order comparison, focused race tests, composed CLI and D&D prompt-cache tests | LLM-001, LLM-002, LLM-003, LLM-004 | | Generic and Seriatim modules | Reviewed | Internal module guide; Seriatim, JSON output, chunk-map, and evidence-context integration contracts; all implementation/tests under `internal/modules/generic/` and `internal/modules/seriatim/`; framework evidence preparation/output paths and focused tests; maintained CLI example and production output contracts | Target-integrity check, graph architecture/symbol/call/caller review, direct import map, required module and codec tests, focused composed evidence/output tests | MOD-001, MOD-002, MOD-003 | -| Shared D&D types, codecs, and family mechanics | Pending | — | — | — | +| Shared D&D types, codecs, and family mechanics | Revisit | D&D internal/module docs and all D&D integration contracts; root durable types; all ten codec packages and candidate JSON; shared references, ordering, citations, inputs, comparison, diagnostics, registry resolver, entity reconciliation, and assets; typed merger/evidence/default-chain/fallback/family registration; representative module asset declarations and focused tests | Target-integrity check, scoped graph architecture/search/traces, ten-family convention matrix, required codec/shared/register tests | DND-CORE-001 | | NPC, item, and location registries | Pending | — | — | — | | NPC, item, and location occurrences | Pending | — | — | — | | Spells, scene chunking, and scene descriptions | Pending | — | — | — |