diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md index d9830dc..251fd30 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -20,8 +20,9 @@ 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, and shared D&D reviews have found three High findings, -four Medium findings, and sixteen Low findings, with no production dependency +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 assembly, debug-to-cache coupling, model-visible credential material in the embedded LLM assets, or domain leakage across the Seriatim and generic module @@ -56,6 +57,10 @@ 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-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 | ## Findings @@ -849,6 +854,147 @@ Final cross-area ordering is pending synthesis. `go test ./internal/modules/dnd/shared/...`. - **Grouping:** Independent. +### NPC, Item, And Location Registries + +### DND-REG-001 — Reject reversed evidence ranges at the registry codec boundary + +- **Severity:** Low +- **Category:** Correctness +- **Evidence:** All three registry 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 + 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 + 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 + `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. + 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. + 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. +- **Grouping:** Independent. + +### DND-REG-002 — Keep NPC extraction evidence inside the current chunk + +- **Severity:** Low +- **Category:** Correctness +- **Evidence:** The NPC, item, and location extractors receive only the current + chunk and all three default extract chains run their family source-reference + validator. Item and location validators require a non-nil extraction chunk + and reject references whose endpoints are not in it + (`internal/modules/dnd/validate/itemregistry/source_refs/validator.go:37`–`55` + and `validate/locationregistry/source_refs/validator.go:37`–`55`). The NPC + validator at `validate/npcregistry/source_refs/validator.go:36`–`53` checks + only that a reference is valid somewhere in the complete source document; it + neither requires `req.Chunk` during extraction nor checks chunk membership. + Its focused tests include reversed, foreign, and out-of-document ranges but + no off-chunk range or missing-chunk case. +- **Impact:** If an NPC extraction response supplies a valid unit ID from + another chunk, the candidate can pass evidence validation despite the model + never receiving that passage. A name that also appears at the off-chunk range + can pass advisory relatedness without proving the current candidate, causing + duplicated or misattributed registry provenance across chunk results. The + model usually copies visible unit IDs, which limits the likelihood. +- **Recommendation:** Match the item/location extraction contract: require the + current chunk when `req.Stage` is extract and reject NPC references outside + that chunk. Keep whole-document validation for normalize and other non- + extraction validation calls. +- **Preserve:** Retain full-document source-ID/range validation, shape deferral, + bounded aggregate diagnostics, normalization without a chunk, direct factual + third-party mentions, and source-relatedness as an advisory check rather than + an identity gate. +- **Validation:** Add NPC source-reference cases for an existing off-chunk + range, a missing extraction chunk, an accepted in-chunk range, and normalize- + stage validation without a chunk; run the NPC registry validator, extractor, + and assembled pipeline tests. +- **Grouping:** Independent. + +### DND-REG-003 — Index item and location duplicate groups + +- **Severity:** Low +- **Category:** Efficiency +- **Evidence:** NPC preprocessing indexes normalized comparison names in a map, + but `comparisonNameGroups` in + `internal/modules/dnd/normalize/itemregistry/normalizer.go:242`–`259` scans + every previously formed group for each item and recomputes the first member's + comparison key. `exactDuplicateGroups` in + `normalize/locationregistry/normalizer.go:229`–`246` repeats that nested scan + and performs `reflect.DeepEqual` over canonical source-reference slices for + each candidate group. With distinct records, both paths are quadratic before + any LLM reconciliation request is built. +- **Impact:** Large merged registries spend avoidable deterministic CPU in + normalization, with the location cost also proportional to citation-list + comparisons. Normal registry sizes and the later LLM request dominate today, + so the issue is low severity. +- **Recommendation:** Preserve first-seen group order while maintaining a local + index from the complete duplicate identity to its group position. Use the + existing comparison key for items and an exact, collision-safe key over the + comparison name plus canonical source-reference sequence for locations; + verify equality on any hash collision rather than using lossy concatenation. +- **Preserve:** Item duplicates remain name-identity duplicates regardless of + evidence; locations collapse only equal comparison names with exactly equal + canonical evidence; group/member order, input-index provenance, warning + scopes, and later proposal-only semantic reconciliation remain unchanged. +- **Validation:** Add many-distinct and repeated-key cases that compare output + groups and warning order with current fixtures, plus a focused benchmark or + comparison-count hook demonstrating linear expected grouping work; run the + item and location normalizer tests. +- **Grouping:** Independent. + +### DND-REG-004 — Select location identity anchors without sorting + +- **Severity:** Low +- **Category:** Efficiency +- **Evidence:** Location identity depends only on the least valid source + reference. `earliestReference` nevertheless calls `canonicalReferences`, + which allocates a full copy, sorts it, and deduplicates it + (`internal/modules/dnd/locations/identity/identity.go:137`–`167`). + `ValidateRegistry` calls `earliestReference` once to check evidence and then + calls `DeriveID`, which invokes it again for every syntactically valid record + (lines 106–121). Normalization has already canonicalized each location's + references before deriving its ID, but pays the same second ordering pass. +- **Impact:** Each validation performs two `O(r log r)` allocations/sorts per + location to obtain one minimum, and normalization adds another sort after + source-order canonicalization. Citation counts are bounded in ordinary + transcripts, so this is a localized allocation and CPU issue. +- **Recommendation:** Implement `earliestReference` as a non-mutating linear + minimum selection using the exact current source-ID/start/end comparator, and + compute that anchor once per record during identity validation before ID + comparison. Do not reuse document-position `SourceRefOrder`, whose ordering + contract differs from durable location identity. +- **Preserve:** Keep order-independent IDs, filtering of malformed anchors, + lexicographic source-ID then numeric start/end selection, same-name/different- + anchor distinction, exact compact JSON hash input, and input non-mutation. +- **Validation:** Retain the permuted-reference and same-name/different-anchor + identity fixtures, add mixed valid/invalid and tie-order cases, and use an + allocation or benchmark assertion to demonstrate one linear pass per record; + run the location identity, registry, and normalizer tests. +- **Grouping:** Independent. +