diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md index 7ec55cf..987b9e4 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -18,55 +18,101 @@ change only roadmap audit documents do not change that production target. ## Executive Summary -Pending final synthesis. The initial baseline is healthy. The architecture, -configuration/CLI, pipeline composition, reference/handoff, runtime, state, -LLM, generic/Seriatim, shared D&D, registry-family, occurrence-family, -spell/scene-family, and combat/enemy-family reviews have found three High -findings, seven Medium findings, and twenty-three 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. +The frozen production target is healthy enough to build and pass its complete +fresh, race, vet, and shuffled test suites, but the audit found 33 actionable +issues: three High, seven Medium, and twenty-three Low. The principal risks are +typed-validator aliasing that can mutate accepted stage output (RUN-002), raw +provider errors escaping the LLM adapter (LLM-001), work dispatch after +cancellation (RUN-001), silently ignored configuration documents (CFGCLI-001), +lossy state-path identities (STATE-001), and D&D prompt/durable-boundary +contract mismatches (DND-OCC-001, DND-SCENE-001, and DND-COMBAT-001). + +The recurring remediation themes are narrow boundary checks, explicit value +ownership, cancellation and error containment, indexing repeated scans, reuse +of compiled or canonical values, and removal of unreachable helpers. Those +changes should preserve the explicit typed registries, ordered runner and CLI +lifecycles, family-owned D&D policy, canonical generated-reference codec +boundary, source/evidence separation, bounded dual worker pools, and exact +module prompt manifests documented below. No independent Test Quality finding +survived final review: missing regression cases are owned by the corresponding +production defects. RUN-004 is the only independent comment defect that meets +the repository's exact-invariant standard. + +The review found no production dependency inversion, unbounded framework +worker pool, completion-order-dependent result assembly, debug-to-cache +coupling, model-visible credential material in embedded LLM assets, or domain +leakage across the Seriatim and generic module boundaries. + +## Recommended Remediation Work Sets + +These are bounded sets of related ownership changes, not a required delivery +sequence. Sets without shared IDs can be implemented and reviewed +independently. + +- **Runtime containment:** RUN-001, RUN-002, RUN-003, LLM-001, and LLM-002. + Keep cancellation, candidate ownership, terminal warnings, and outward error + chains aligned across the provider and pipeline runtime boundaries. +- **Strict input and identity boundaries:** CFGCLI-001, CFGCLI-002, PIPE-001, + REF-001, REF-003, STATE-001, and LLM-003. Reject ambiguous or oversized input + before allocation or normalization can erase the distinction. +- **Construction and handoff efficiency:** PIPE-002, REF-002, STATE-002, and + MOD-002. Remove only copies, scans, or compilation repeated inside an already + established ownership boundary. +- **Physical and virtual publication infrastructure:** STATE-003 and LLM-004. + Reuse the existing confined writer and read-only in-memory filesystem + mechanics without combining their higher-level policies. +- **D&D evidence and durable contracts:** DND-REG-001, DND-REG-002, + DND-OCC-001, DND-SCENE-001, and DND-COMBAT-001. Align evidence range, + prompt/schema, catalog projection, and durable codec enforcement at the + family-owned boundaries. +- **D&D normalization cost and diagnostics:** DND-REG-003, DND-REG-004, + DND-OCC-003, and DND-SCENE-002. Index repeated identity work, reuse canonical + evidence, and bound warning output without changing ordering or fallback. +- **Focused cleanup and documentation:** ARCH-001, RUN-004, MOD-001, MOD-003, + DND-CORE-001, and DND-OCC-002. These are independent documentation, + reachability, or unused-input changes with narrow validation surfaces. ## Finding Index -Final cross-area ordering is pending synthesis. +Findings are ordered by severity, then by correctness risk, dependency +boundary, and expected remediation value. Stable IDs preserve the owning audit +area even when related findings are grouped for implementation. | ID | Severity | Category | Title | | --- | --- | --- | --- | -| ARCH-001 | Low | Documentation/Comments | Repair broken ADR cross-references | +| RUN-002 | High | Correctness | Isolate typed validator values from stage output | +| LLM-001 | High | Correctness | Keep raw provider errors inside the adapter | +| RUN-001 | High | Correctness | Check cancellation at unguarded dispatch boundaries | | CFGCLI-001 | Medium | Correctness | Reject additional YAML documents | +| RUN-003 | Medium | Correctness | Preserve warnings from the terminal rejected attempt | +| STATE-001 | Medium | Correctness | Preserve distinct identities in state paths | +| DND-OCC-001 | Medium | Correctness | Align item occurrence evidence fields with the shared prompt | +| DND-SCENE-001 | Medium | Correctness | Expose catalog aliases to spell extraction | +| DND-COMBAT-001 | Medium | Correctness | Enforce enemy-event semantics at the durable codec boundary | +| REF-001 | Medium | Efficiency | Bound reference reads before allocating the file | | CFGCLI-002 | Low | Correctness | Reject a blank command-level LLM profile | | PIPE-001 | Low | Correctness | Reject normalized module-reference collisions in the resolver | -| PIPE-002 | Low | Efficiency | Clone construction inputs once per builder boundary | -| REF-001 | Medium | Efficiency | Bound reference reads before allocating the file | -| REF-002 | Low | Efficiency | Index accepted outputs once per ordered handoff | | REF-003 | Low | Correctness | Include canonical size in generated-reference fingerprints | -| RUN-001 | High | Correctness | Check cancellation at unguarded dispatch boundaries | -| RUN-002 | High | Correctness | Isolate typed validator values from stage output | -| RUN-003 | Medium | Correctness | Preserve warnings from the terminal rejected attempt | -| RUN-004 | Low | Documentation/Comments | Document the lane collector's liveness invariant | -| STATE-001 | Medium | Correctness | Preserve distinct identities in state paths | -| STATE-002 | Low | Efficiency | Remove redundant post-decode clones from canonical codecs | -| STATE-003 | Low | Duplication | Publish output through the confined file writer | -| LLM-001 | High | Correctness | Keep raw provider errors inside the adapter | | LLM-002 | Low | Correctness | Recheck cancellation after scheduler admission | | LLM-003 | Low | Correctness | Reject duplicate virtual prompt names | -| LLM-004 | Low | Duplication | Share the read-only in-memory filesystem mechanics | -| 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 | | DND-REG-001 | Low | Correctness | Reject reversed evidence ranges at durable D&D codec boundaries | | DND-REG-002 | Low | Correctness | Keep extraction evidence inside the current chunk | +| DND-SCENE-002 | Low | Correctness | Make D&D normalization and advisory diagnostics complete and bounded | +| PIPE-002 | Low | Efficiency | Clone construction inputs once per builder boundary | +| REF-002 | Low | Efficiency | Index accepted outputs once per ordered handoff | +| STATE-002 | Low | Efficiency | Remove redundant post-decode clones from canonical codecs | +| MOD-002 | Low | Efficiency | Reuse compiled response schemas within a prepared validator | | DND-REG-003 | Low | Efficiency | Index duplicate groups across D&D families | | 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 | -| DND-SCENE-001 | Medium | Correctness | Expose catalog aliases to spell extraction | -| DND-SCENE-002 | Low | Correctness | Make D&D normalization and advisory diagnostics complete and bounded | -| DND-COMBAT-001 | Medium | Correctness | Enforce enemy-event semantics at the durable codec boundary | +| STATE-003 | Low | Duplication | Publish output through the confined file writer | +| LLM-004 | Low | Duplication | Share the read-only in-memory filesystem mechanics | +| MOD-001 | Low | Simplicity | Narrow generic integer option decoding | +| MOD-003 | Low | Simplicity | Remove unreachable JSON metadata clone helpers | +| DND-CORE-001 | Low | Simplicity | Remove the unused lossy unit-reference constructor | +| DND-OCC-002 | Low | Simplicity | Remove unused campaign references from NPC occurrence normalization | +| ARCH-001 | Low | Documentation/Comments | Repair broken ADR cross-references | +| RUN-004 | Low | Documentation/Comments | Document the lane collector's liveness invariant | ## Findings @@ -1305,25 +1351,9 @@ Final cross-area ordering is pending synthesis. - **Grouping:** Coordinate with DND-REG-001; that finding owns the common forward-range gap, while this finding owns enemy-event schema enforcement. - - ## D&D Convention Matrix -The matrix records the current production convention before the lane-specific +The matrix records the frozen production convention after 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 @@ -1426,11 +1456,10 @@ mechanics that recur without erasing those distinctions. not merging the two module identities or requiring a particular chain. - `internal/modules/dnd/register.registerModules`, `registerEvidence`, `registerValidators`, and `registerDefaultChains` use explicit typed - registration lists. At this architectural pass, that repetition preserves - artifact Go types, module-specific validator order, and registrar-owned - production policy. Later D&D stages may evaluate individual shared - mechanics, but should not replace these lists with a dynamically typed - registration engine. + registration lists. That repetition preserves artifact Go types, + module-specific validator order, and registrar-owned production policy. + Individual shared mechanics do not justify replacing these lists with a + dynamically typed registration engine. - Combat-turn and enemy-event extractors deliberately own separate explicit scene gates. Both consult the same immutable eligibility registry before any LLM call, but the enemy lane must then resolve three required source-free @@ -1585,6 +1614,32 @@ mechanics that recur without erasing those distinctions. not combining catalog composition with prompting or weakening collision checks. +## Cross-Cutting Test And Comment Review + +Every finding was re-read against its cited production owner, immediate +callers, and the narrowest maintained test boundary. The uncovered cases are +specific consequences of the production defects—for example second-document +parsing, post-cancellation dispatch, validator mutation, terminal rejected +warnings, state-identity collisions, provider-error redaction, prompt-name +collisions, evidence bounds, and durable enemy-event semantics—so their +regression tests remain in each finding's **Validation** clause rather than +becoming duplicate Test Quality findings. + +The focused-test inventories in the configuration, CLI, pipeline, state, LLM, +module, and D&D internal guides agree with the observed ownership. Contract and +matrix tests appropriately span files where the invariant is cross-component; +leaf codec, validator, resolver, scheduler, and filesystem tests own local +rules. No assertion-only duplication, private-helper coupling, flaky timing +dependency, or test fixture elaborate enough to justify a standalone finding +survived the review. + +Comments were judged only where code cannot express an ordering, concurrency, +or ownership invariant. Existing explanations around CLI/runner lifecycle, +operation-time cloning, cache publication, prompt order, scene coverage, and +typed erasure are specific and still accurate. RUN-004 remains because the +collector's non-obvious liveness proof depends on its caller draining every +lane channel; no other missing or stale comment met that bar. + ## Areas Reviewed Without Findings ### Architecture And Dependency Boundaries @@ -2390,6 +2445,14 @@ mechanics that recur without erasing those distinctions. | 2026-08-08 | Combat and enemy durable codec tests | `go test ./internal/modules/dnd/codec/combatturns ./internal/modules/dnd/codec/enemyevents` | Pass | | 2026-08-08 | Assembled combat and generated-reference integration tests | `go test ./internal/modules/integration/...` | Pass | | 2026-08-08 | Generated combat/enemy handoff and publication tests | `go test ./internal/cli` | Pass | +| 2026-08-08 | Audit target integrity before final synthesis | `git diff --quiet 92e89076a268089e703978fb9d7176200e93344c..HEAD -- . ':(exclude)docs/roadmap/**'` | Pass; production target unchanged | +| 2026-08-08 | Cross-cutting finding, test, and comment review | Complete finding reread against cited production owners and callers, focused-test inventories, complexity/comment hotspots, intentional-complexity entries, and coverage matrix | All 33 findings retained with distinct root causes; no new finding, contradiction, duplicate Test Quality issue, or unsupported comment issue found | +| 2026-08-08 | Fresh full test suite | `go test -count=1 ./...` | Pass | +| 2026-08-08 | Full race suite | `go test -race ./...` | Pass | +| 2026-08-08 | Full static analysis | `go vet ./...` | Pass | +| 2026-08-08 | Production build | `go build ./cmd/notarius` | Pass | +| 2026-08-08 | Final whitespace | `git diff --check` | Pass | +| 2026-08-08 | Full shuffled test suite | `go test -shuffle=on ./...` | Pass | ## Coverage Matrix @@ -2408,4 +2471,4 @@ mechanics that recur without erasing those distinctions. | NPC, item, and location occurrences | Reviewed | NPC/item/location occurrence integration contracts and deterministic opaque-ID ADR; extractors, private response models/schemas, semantic registry projections and resolvers, canonicalizers, normalizers, validator chains, codecs, prompt assets, and generated/external registry handoff tests for all three families | Target-integrity check, scoped graph architecture/complexity/search/traces, full family comparison, required extractor/normalizer/validator tests, generated CLI handoff tests | DND-REG-001, DND-OCC-001, DND-OCC-002, DND-OCC-003 | | Spells, scene chunking, and scene descriptions | Reviewed | Spell, overlay, scene-description, and accepted chunk-map integration contracts; D&D scene chunker and prompt/schema assets; framework plan canonicalization, validation, materialization, accepted chunk-map serialization/publication and production tests; scene-description extractor, normalizer, validators, immutable eligibility registry, codec, prompts, schemas, and tests; spell extractor, effective/base/overlay catalogs, NPC grounding, canonicalizer, normalizer, validators, codec, prompts, schemas, checkpoint/retry/CLI tests | Target-integrity check, scoped graph architecture/complexity/symbol/call review, full family comparison, required chunker/extractor/normalizer/validator/registry/catalog tests | PIPE-002, DND-REG-001, DND-REG-002, DND-SCENE-001, DND-SCENE-002 | | Combat turns and enemy events | Reviewed | Combat/enemy integration contracts and D&D internals; extractors, scene/NPC resolvers, compact combat/opponent projections, canonicalizers, normalizers, shared enemy ordering/identity helpers, shape/source/relatedness/engagement/invariant validators, codecs, prompt assets, private schemas, focused and assembled integration tests, generated-reference CLI contracts, and maintained complete example | Target-integrity check, scoped graph architecture/complexity/symbol/call review, full family comparison, required extractor/normalizer/validator/helper tests, codec tests, assembled integration tests, generated CLI handoff/publication tests | DND-REG-001, DND-REG-002, DND-REG-003, DND-SCENE-002, DND-COMBAT-001 | -| Test ownership, comments, and final synthesis | Pending | — | — | — | +| Test ownership, comments, and final synthesis | Reviewed | Complete audit report; testing and documentation policies; focused-test inventories in configuration, CLI, pipeline, state, LLM, modules, and D&D internal guides; every finding's cited production owner, callers, tests, comments, and intentional-complexity entries | Target-integrity check, graph symbol/caller/complexity review, complete fresh and race test suites, vet, production build, whitespace check, shuffled suite | No new IDs; all 33 findings revalidated, including RUN-004 as the only independent comment defect |