Finalize codebase audit synthesis
This commit is contained in:
@@ -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.
|
||||
|
||||
<!--
|
||||
Finding template for later audit stages:
|
||||
|
||||
### AREA-001 — Concise title
|
||||
|
||||
- **Severity:** High, Medium, or Low
|
||||
- **Category:** Correctness, Efficiency, Duplication, Simplicity, Test Quality,
|
||||
or Documentation/Comments
|
||||
- **Evidence:** Exact files, symbols, call paths, and observed behavior
|
||||
- **Impact:** The concrete risk or cost
|
||||
- **Recommendation:** A bounded implementation direction
|
||||
- **Preserve:** Invariants and contracts that remediation must retain
|
||||
- **Validation:** Focused checks that would demonstrate success
|
||||
- **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
|
||||
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 |
|
||||
|
||||
Reference in New Issue
Block a user