diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md index 251fd30..04ad210 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -20,10 +20,10 @@ 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, generic/Seriatim, shared D&D, and registry-family reviews have found three -High findings, four Medium findings, and twenty Low findings, with no -production dependency -inversion, unbounded framework worker pool, completion-order-dependent result +LLM, generic/Seriatim, shared D&D, registry-family, and occurrence-family +reviews have found three High findings, five Medium findings, and twenty-two +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. @@ -57,10 +57,13 @@ Final cross-area ordering is pending synthesis. | 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 | -| DND-REG-001 | Low | Correctness | Reject reversed evidence ranges at the registry codec boundary | +| DND-REG-001 | Low | Correctness | Reject reversed evidence ranges at durable D&D codec boundaries | | DND-REG-002 | Low | Correctness | Keep NPC extraction evidence inside the current chunk | | DND-REG-003 | Low | Efficiency | Index item and location duplicate groups | | DND-REG-004 | Low | Efficiency | Select location identity anchors without sorting | +| DND-OCC-001 | Medium | Correctness | Align item occurrence evidence fields with the shared prompt | +| DND-OCC-002 | Low | Simplicity | Remove unused campaign references from NPC occurrence normalization | +| DND-OCC-003 | Low | Efficiency | Reuse canonical item occurrence evidence | ## Findings @@ -856,46 +859,56 @@ Final cross-area ordering is pending synthesis. ### NPC, Item, And Location Registries -### DND-REG-001 — Reject reversed evidence ranges at the registry codec boundary +### DND-REG-001 — Reject reversed evidence ranges at durable D&D codec boundaries - **Severity:** Low - **Category:** Correctness -- **Evidence:** All three registry integration contracts require that a source - range's start not follow its end +- **Evidence:** The three registry and three occurrence integration contracts + require that a source range's start not follow its end (`docs/integrations/dnd-npc-registry-artifacts.md:32`–`34`, `dnd-item-registry-artifacts.md:33`–`35`, and - `dnd-location-registry-artifacts.md:34`–`36`). The three durable codec + `dnd-location-registry-artifacts.md:34`–`36`; and + `dnd-npc-occurrence-artifacts.md:33`–`36`, + `dnd-item-occurrence-artifacts.md:41`–`43`, and + `dnd-location-occurrence-artifacts.md:33`–`35`). All six durable codec validators check only that both endpoints are positive (`internal/modules/dnd/codec/npcregistry/codec.go:96`–`106`, `codec/itemregistry/codec.go:97`–`107`, and - `codec/locationregistry/codec.go:97`–`107`). Direct registry references are + `codec/locationregistry/codec.go:97`–`107`; and + `codec/npcoccurrences/codec.go:99`–`109`, + `codec/itemoccurrences/codec.go:117`–`127`, and + `codec/locationoccurrences/codec.go:100`–`110`). Direct registry references are decoded and identity-checked by `npcs/registry.loadRegistry`, `items/registry.loadRegistry`, and `locations/registry.loadRegistry`, but do not pass through the generated-output source-reference validators. A durable - artifact with `{start_unit_id: 2, end_unit_id: 1}` and otherwise valid - identity is therefore accepted; for locations, `validIdentityReference` at + registry or occurrence artifact with + `{start_unit_id: 2, end_unit_id: 1}` and otherwise valid fields is therefore + accepted; for locations, `validIdentityReference` at `internal/modules/dnd/locations/identity/identity.go:170`–`171` also treats that reversed range as a valid identity anchor. -- **Impact:** An externally supplied registry can be accepted as approved even - though its evidence cannot denote the documented forward source interval. +- **Impact:** An externally supplied registry or occurrence list can be + accepted as approved even though its evidence cannot denote the documented + forward source interval. NPC and item prompt projections then hide the malformed provenance, while a location can derive and retain a durable ID from it; only later consumers that happen to construct contextual grounding against the same source may reject it. Generated pipeline outputs remain protected by their source- reference validator chains, which limits current exposure. - **Recommendation:** Add the order-independent structural condition - `start_unit_id <= end_unit_id` to each registry codec's durable validation, - and make location identity reject reversed anchors as a defense in depth. + `start_unit_id <= end_unit_id` to all six reviewed codecs' durable + validation, and make location identity reject reversed anchors as a defense + in depth. Keep document membership and current-chunk coverage in the existing validators, where the source document is available. - **Preserve:** Retain positive exact source-unit identifiers, strict unknown- field/trailing-value rejection, location identity's earliest canonical anchor policy, and the separation between source-independent durable shape validation and source-dependent evidence validation. -- **Validation:** Add codec encode/decode cases for reversed ranges in all three - registry families, a location identity case that refuses a reversed-only - anchor, and direct registry resolver cases proving malformed referenced JSON - is rejected; run the three codec, identity, and registry package suites. +- **Validation:** Add codec encode/decode cases for reversed ranges in all six + reviewed families, a location identity case that refuses a reversed-only + anchor, and direct registry and occurrence reference cases proving malformed + referenced JSON is rejected; run the codec, identity, registry, occurrence, + and assembled CLI suites. - **Grouping:** Independent. ### DND-REG-002 — Keep NPC extraction evidence inside the current chunk @@ -995,6 +1008,111 @@ Final cross-area ordering is pending synthesis. run the location identity, registry, and normalizer tests. - **Grouping:** Independent. +### NPC, Item, And Location Occurrences + +### DND-OCC-001 — Align item occurrence evidence fields with the shared prompt + +- **Severity:** Medium +- **Category:** Correctness +- **Evidence:** The shared model-visible evidence instruction tells every D&D + extractor to return integer `start_unit_id` and `end_unit_id` fields + (`assets/dnd/shared/prompts/common-dnd-extraction-evidence.md:1`–`4`). NPC and + location occurrence extraction follow that convention, but the item + occurrence private response DTO instead requires `start_segment` and + `end_segment` + (`internal/modules/dnd/extract/itemoccurrences/model.go:16`–`18`), and its + strict structured-output schema requires the same incompatible names + (`assets/dnd/item-occurrences/schemas/dnd_item_occurrences_llm.v1.json:20`–`29`). + The representative generated pipeline fixture must likewise emit the + segment-named fields to pass + (`internal/cli/dnd_enemy_events_contract_test.go:318`–`334`). +- **Impact:** The prompt and enforced response schema give the model + contradictory instructions at one structured-output boundary. A model that + follows the shared evidence instruction produces missing required fields and + disallowed unknown fields, causing an otherwise valid item occurrence + candidate to be rejected and retried. Provider-side schema steering may + conceal the mismatch for some models, but cannot make the prompt contract + coherent. +- **Recommendation:** Rename the item occurrence private response fields and + schema properties to `start_unit_id` and `end_unit_id`, update the adapter + and fixtures, and verify that the affected response-schema fingerprint + changes. Add an assembled-prompt contract proving every evidence field named + by the item response schema matches the shared instruction. +- **Preserve:** Keep source IDs out of model output, inject the current source + identity locally, retain exact positive transcript unit IDs, strict + structured decoding, current-chunk evidence validation, and the unchanged + durable occurrence wire shape. +- **Validation:** Retain item extractor malformed-response and citation tests, + update the generated multi-family CLI fixture to use unit-named endpoints, + add the prompt/schema alignment assertion, and run the item occurrence and + CLI suites. +- **Grouping:** Independent. + +### DND-OCC-002 — Remove unused campaign references from NPC occurrence normalization + +- **Severity:** Low +- **Category:** Simplicity +- **Evidence:** NPC occurrence normalization declares the optional campaign + `glossary`, `party`, `players`, and deprecated `roster` slots as material used + for occurrence disambiguation + (`internal/modules/dnd/normalize/npcoccurrences/normalizer.go:38`–`43`) and + includes them beside the required registry in `ReferenceSlots` (lines + 252–263). Construction retains only an NPC registry resolver (lines 51–67), + and normalization resolves and uses only that registry plus the source + document (lines 99–123). Item and location occurrence normalizers expose + only their required family registry. +- **Impact:** Bindings that cannot affect NPC normalization are nevertheless + admitted into resolved profiles, materialized, recorded in manifests and + dependency identities, and can invalidate checkpoints when their content + changes. The descriptions also imply a disambiguation behavior the + deterministic normalizer does not implement. +- **Recommendation:** Remove the four campaign slots and their descriptions + from the NPC occurrence normalizer, leaving only required `npc_registry`. + Keep campaign context on extraction, where it is model-visible and can + actually influence selection. +- **Preserve:** Retain exact registry ID/name validation and canonicalization, + operation-time registry overrides, source-reference ordering, exact duplicate + collapse, bounded warnings, and all extraction reference slots. +- **Validation:** Update module-spec, reference-binding, manifest, and + checkpoint dependency tests to require exactly the registry slot during + normalization; prove extraction still accepts campaign context; run NPC + occurrence and assembled CLI tests. +- **Grouping:** Independent. + +### DND-OCC-003 — Reuse canonical item occurrence evidence + +- **Severity:** Low +- **Category:** Efficiency +- **Evidence:** Item normalization canonicalizes each occurrence's evidence + once in `normalizeOccurrence` + (`internal/modules/dnd/normalize/itemoccurrences/normalizer.go:174`–`195`), + then sorts those records through `itemoccurrences.Less` (lines 155–157) and + builds duplicate keys through `ExactIdentity` (lines 220–229). `Less` + canonicalizes both reference slices again at its final tie-breaker + (`internal/modules/dnd/itemoccurrences/itemoccurrences.go:95`–`125`), while + `ExactIdentity` canonicalizes each slice again at lines 145–163. The + invariants validator first verifies canonical order and uniqueness, then + calls both helpers again + (`internal/modules/dnd/validate/itemoccurrences/invariants/validator.go:65`–`95`). +- **Impact:** Tie-heavy merged lists repeatedly allocate, sort, and deduplicate + evidence that the normalizer has already canonicalized; normalized artifact + validation repeats the same work. Transcript and LLM costs dominate ordinary + runs, so this is a localized low-severity deterministic cost. +- **Recommendation:** Add canonical-input comparator and identity-key paths, or + precompute canonical sort/dedup keys per normalized record, and use them only + after the caller has established canonical evidence. Retain the current + defensive public helpers for arbitrary or malformed candidates that still + need literal-fallback ordering. +- **Preserve:** Keep exact canonical ordering, nil and invalid-reference + diagnostic behavior, holder and quantity distinctions, collision-safe + identity keys, stable input provenance for warnings, and non-mutating helper + contracts. +- **Validation:** Compare optimized normalization and invariant results against + the existing permuted/duplicate/invalid evidence fixtures; add a tie-heavy + benchmark or canonicalization-count hook showing one evidence canonicalization + per normalized record; run item occurrence normalize/validator tests. +- **Grouping:** Independent. +