Audit shared D&D family conventions

This commit is contained in:
2026-08-08 22:08:24 +00:00
parent a22c1a7f59
commit 3589d33468

View File

@@ -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 4771, 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.
<!--
Finding template for later audit stages:
@@ -832,6 +865,87 @@ Finding template for later audit stages:
- **Grouping:** Independent, or the IDs with which this should be implemented
-->
## 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 | — | — | — |