notarius/docs/roadmap/audit-plan.md

# Codebase Audit Plan

## Purpose

This document defines a repository-wide audit of Notarius for correctness,
efficiency, maintainability, and clarity. The audit should identify concrete
improvements without treating abstraction, fewer lines, or higher test coverage
as goals in themselves.

The audit is intentionally separate from implementation. Its findings should
be evidence-backed and sufficiently specific to support a later remediation
roadmap, but the audit should not modify production code, tests, assets, or
current-behavior documentation.

## Governing Principles

The audit must preserve the architecture and testing policies in
`docs/policy/architecture.md` and `docs/policy/testing.md`.

In particular:

- Notarius remains a fixed, staged pipeline rather than a general workflow
  engine.
- Generic framework packages must remain domain-neutral, and production
  modules must not acquire CLI or physical-state responsibilities.
- Typed artifact boundaries, exact codec compatibility, deterministic ordering,
  whole-output validation, and generated-reference provenance are correctness
  properties, not incidental complexity to be optimized away.
- The root `assets` package remains a content-only dependency leaf.
- Shared helpers should protect demonstrated common semantics. Similar-looking
  code with different ownership, error policy, identity rules, or type contracts
  should remain separate.
- Tests should protect durable behavior and meaningful risks. The audit should
  not recommend tests merely to increase coverage or freeze implementation
  details.
- Efficiency claims must distinguish measured or structurally credible costs
  from cosmetic line-count reductions. Optimizing local CPU work that is
  insignificant beside an LLM call is low priority unless it also simplifies
  correctness or applies to large inputs.

## Audit Questions

Every audited area should be examined through the following questions.

### Correctness

- Are documented architecture invariants enforced at the correct boundary?
- Can invalid configuration, incompatible artifact types, malformed references,
  or unavailable dependencies reach execution when they could be rejected
  during resolution or preparation?
- Are nil, empty, absent, rejected, failed, and canceled states distinguished
  consistently?
- Are stored or returned slices, maps, byte slices, options, metadata, source
  documents, references, and artifacts defensively owned where required?
- Are public ordering, selected errors, warnings, and checkpoint decisions
  deterministic regardless of map or goroutine completion order?
- Do cancellation, retry, validation, and partial-work semantics match their
  documented ownership?
- Do checkpoint and chunk-plan identities include every semantic dependency and
  exclude scheduling-only or diagnostic state?
- Can auxiliary references accidentally become source evidence, or can
  generated references bypass codec, schema, provenance, or step-order checks?
- Can provider-specific values, credentials, or source content escape through
  errors, manifests, debug summaries, cache state, or logs?
- Do schemas, codecs, candidate decoders, normalizers, and validators agree on
  the exact durable contract without silently accepting incompatible shapes?

### Duplication And Shared Mechanics

- Which exact or near-duplicate implementations express the same invariant and
  failure policy?
- Has duplicated code already drifted in naming, nil handling, canonicalization,
  metadata, fingerprints, validation, or diagnostics?
- Would a helper have a natural owner and a smaller, clearer contract than the
  duplicated callers?
- Can an extraction preserve static typing and package ownership, or would it
  require reflection, `any`, callbacks with many policy parameters, or a
  domain-neutral package importing domain concepts?
- Is repeated code required by a small interface adapter or typed registration
  boundary and therefore clearer when left explicit?

As a default heuristic, prioritize a shared helper when identical semantics
appear in three or more production sites, or in two sites where divergence
would create a meaningful correctness risk. Do not use that heuristic as a
quota: one substantial duplicate may warrant extraction, while widespread
one-line interface methods may not.

### Simplicity And Idiomatic Go

- Does a function combine orchestration, policy, transformation, persistence,
  and reporting that could be separated along existing ownership boundaries?
- Are repeated scans, sorts, conversions, clones, encodes, or decodes doing work
  that can safely occur once?
- Are intermediate representations necessary, or can a value be validated,
  canonicalized, and mapped in one comprehensible pass?
- Are maps, sets, stable sorts, generics, standard-library helpers, and error
  wrapping used idiomatically?
- Are abstractions earning their complexity, or are interfaces, option layers,
  wrappers, aliases, compatibility paths, and private types left over after a
  completed migration?
- Are there unreachable error branches, redundant fingerprints or digests,
  duplicated sources of truth, or accessors used only by tests?
- Can a smaller implementation preserve exact observable behavior and safety
  properties?

### Explanatory Comments

Comments should be recommended where the code is necessarily complex because
it preserves a non-obvious invariant. Good candidates include:

- concurrency coordination, cancellation, and stable error selection;
- checkpoint identity, reuse, forced recomputation, and dependency invalidation;
- typed erasure and restoration at framework boundaries;
- generated-reference ordering and provenance;
- canonicalization and identity resolution where registry evidence differs
  from occurrence evidence;
- prompt ordering or input identity required for backend caching; and
- path confinement, atomic publication, redaction, or terminal error precedence.

Recommend comments that explain *why* a step or ordering constraint exists and
what would break if it changed. Do not recommend comments that narrate syntax,
repeat a function name, duplicate current-behavior documentation, or preserve
implementation history.

### Tests

- Is each consequential invariant protected at the narrowest stable boundary?
- Are concurrency, cancellation, retries, recovery, compatibility, path safety,
  and data-integrity behavior credibly exercised?
- Do higher-level contract tests duplicate lower-level cases without adding
  integration confidence?
- Are tests coupled to private constants, helper shape, exact prose, full error
  strings, or collaborator choreography rather than behavior?
- Can repetitive fixtures or fakes be simplified without creating a test
  framework more complex than the tests?
- Would a focused fuzz test, race test, or package-level invariant test protect
  a realistic risk better than several example tests?

## Evidence And Finding Standards

Static metrics and textual similarity are discovery aids, not findings. A long
function may be a clear linear coordinator; identical methods may be useful
typed adapters. Every reported finding must include:

1. a concise title and severity;
2. exact files and symbols;
3. the observed behavior or structural evidence;
4. the correctness, efficiency, maintenance, or comprehension impact;
5. a concrete recommended direction;
6. important invariants the remediation must preserve;
7. focused validation that would demonstrate success; and
8. whether the recommendation is independent or should be grouped with another
   finding.

Use these severities:

- **High:** a credible risk of corrupt output, unsafe state handling, secret
  exposure, stale reuse, deadlock, nondeterminism, or violated external
  contract.
- **Medium:** a plausible behavioral defect, meaningful wasted work on common
  paths, or complexity/duplication likely to cause future correctness drift.
- **Low:** a contained simplification, small efficiency improvement, dead code,
  naming issue, or missing explanation with no current behavioral failure.

The audit should explicitly record examined areas with no findings. This makes
coverage visible and prevents later agents from repeatedly rediscovering the
same safe design.

## Repository Areas

### 1. Architecture And Dependency Boundaries

Inspect `docs/policy/architecture.md`, `docs/adr/`, `docs/internal/overview.md`,
package imports, module registrars, and the CLI composition root.

Look for:

- framework or core code depending on production modules;
- modules depending on CLI, physical roots, or provider-specific types;
- domain knowledge placed in generic helpers;
- duplicated registries or composition policy outside the owning registrar;
- abstractions that turn the fixed pipeline into an implicit general graph; and
- current code that no longer matches an accepted ADR or documented invariant.

Graph-reported cross-layer calls must be traced before being classified because
tests and interface implementations can resemble dependency inversions without
creating a production import violation.

### 2. Configuration And CLI Composition

Inspect `internal/core/config`, `internal/cli`, configuration parsing and
redaction tests, profile construction, session derivation, catalog assembly,
reference overrides, run-result handling, terminal reporting, and maintained
example contract tests.

Pay particular attention to the currently dense paths around
`runPipelineCommand`, configuration profile validation, selected reference
targets, recomputation policy, and option normalization. Determine whether
their complexity reflects necessary composition or mixed responsibilities that
can be separated without moving policy into the framework.

Verify:

- file, environment, CLI, pipeline, binding, and prompt-default precedence;
- consistent strict option and unknown-field handling;
- session identity independence from references and pipeline-local changes;
- effective profile and runtime fingerprint consistency;
- redaction before errors or debug/manifest boundaries;
- output publication only after framework success; and
- one guarded terminalization path that preserves the primary failure.

### 3. Pipeline Resolution, Preparation, And Typed Registries

Inspect `internal/framework/pipeline/profile.go`, `prepare.go`, registry files,
`options.go`, `references.go`, `handoff.go`, `construction.go`, typed contracts,
and their focused tests.

This area deserves a dedicated pass because the current graph identifies
`ResolvePipeline`, generated-binding validation, reference-target resolution,
and generated-reference construction as high-complexity or high-fan-in code.

Verify:

- static failures occur before source parsing;
- selected and unselected lanes do not contaminate each other's requirements;
- stage defaults and overrides have one canonical resolution path;
- typed registration and private erasure cannot panic or accept near-matching
  artifact types;
- generated bindings reject cycles, forward references, ambiguity, wrong kinds,
  and missing accepted normalized producers;
- materialized reference bytes and options are cloned and bounded; and
- resolved composition and prepared fingerprints include the complete semantic
  policy exactly once.

Compare input, chunker, extractor, merger, normalizer, output, validator, codec,
evidence-projector, and validator-chain registries for shared mechanics and
intentional differences. Repeated typed registration code is a candidate only
if a helper can retain useful compile-time guarantees and stage-specific
diagnostics.

### 4. Pipeline Execution, Validation, Retry, And Concurrency

Inspect `runner*.go`, `typed_execution.go`, `runner_typed.go`,
`runner_concurrent.go`, validation-chain execution, normalize retry behavior,
synchronized collaborators, and the concurrency, cancellation, retry, debug,
and checkpoint tests.

Trace complete paths rather than reviewing helper files in isolation:

- source and chunk-plan selection through chunk validation;
- deterministic chunk-first/lane-second dispatch;
- lane extraction through merge and normalize continuations;
- rejection versus framework-error propagation;
- cancellation before dispatch, while queued, and while running;
- retry attempts and warning retention;
- stable error selection after concurrent completion;
- checkpoint hydration back into typed execution; and
- output suppression after a framework error.

Look for goroutine leaks, unbounded work, lock-order risks, double release or
double recording, races on shared result state, unnecessary serialization,
and repeated canonicalization. Comments are especially valuable here when they
explain ordering or cancellation invariants that are not apparent from local
control flow.

### 5. State, Checkpoints, Chunk Plans, Debugging, And File Safety

Inspect `internal/framework/checkpoint`, `chunkplan`, `chunkmap`, `debug`,
`evidencecontext`, `internal/core/fileio`, `debugbundle`, and their CLI
composition.

Verify:

- narrow path validation and symlink-resistant confinement;
- atomic writes and recoverable explicit cleanup;
- separation of checkpoint recording, resume loading, chunk-plan caching, and
  debug capture;
- canonical encoding before content identity is trusted;
- complete but non-secret checkpoint fingerprints;
- correct ordinary-resume and selective-recompute behavior;
- producer dependency invalidation across ordered steps;
- immutable hydration and no aliasing with stored bytes;
- debug data never influencing execution or reuse; and
- terminal persistence failures never obscuring the primary error.

Review the repeated extract/merge/normalize recorder and loader methods, path
component validators in multiple state packages, and clone/encode/decode paths.
Determine which repetition is a clear stage adapter and which can share a
private primitive without weakening reason-code ownership or diagnostics.

### 6. LLM Runtime, Prompt Filesystems, And Assets

Inspect `internal/framework/llm`, `promptfs`, the PromptKit integration,
scheduler, profile-source construction, prompt/schema registries, root
`assets`, module prompt manifests, and relevant D&D shared assets.

Verify:

- every provider call passes through the shared scheduler and cancellation
  removes queued calls safely;
- PromptKit and Notarius concurrency limits compose as documented;
- profile inspection and runtime use identical source precedence;
- session IDs, profile-source fingerprints, prompt fingerprints, and schema
  fingerprints reflect the intended semantic inputs;
- secrets and provider-specific error types do not cross the boundary;
- prompt inputs and private outputs do not expose opaque entity IDs;
- prompt ordering, stable prefixes, and cache controls remain intentional;
- schema loaders and filesystem adapters validate once and return defensive
  data; and
- the root assets package contains no business logic.

Compare the LLM asset registry and prompt-filesystem adapters for duplicated
filesystem behavior. Review repeated prompt/schema loader and metadata code in
module packages, but reject an extraction that would centralize domain prompt
ownership or make unrelated assets share one invalidation boundary.

### 7. Generic And Seriatim Modules

Inspect `internal/modules/generic` and `internal/modules/seriatim`, including
module specs, option decoding, chunk planning, input translation, validators,
output encoding, evidence-context publication, registration, and tests.

Verify that:

- external Seriatim details end at the input boundary;
- generic chunking and output remain domain-neutral;
- chunk plans and source units preserve source-addressed invariants;
- output logical names are safe and deterministic;
- output options do not bypass preparation-time compatibility checks; and
- option decoding is strict, small, and consistent with configuration
  validation.

The graph flags generic integer option parsing and JSON output policy decoding
as relatively complex. Examine whether that is inherent strict decoding or an
opportunity for a smaller typed parser with equally precise diagnostics.

### 8. D&D Domain Model, Codecs, And Shared Helpers

Inspect `internal/modules/dnd` domain types, codecs, candidate decoders,
identity packages, registries, shared source-reference helpers, diagnostics,
registry resolution, entity reconciliation, mergers, registrar, and assets.

Compare all ten current artifact families. Build a convention matrix covering:

- module specs and execution classes;
- constructor and option behavior;
- manifest metadata and checkpoint fingerprints;
- response-schema loading and private-versus-durable types;
- source-reference conversion, canonicalization, ordering, and deduplication;
- nil versus present-empty output;
- codecs and strict JSON behavior;
- registry lookup, identity derivation, immutable projections, and resolution;
- normalizer retry/fallback behavior;
- validators and default chains; and
- registration, prompt assets, and documentation ownership.

The graph reports many exact similarities among codec `Decode` methods,
fingerprint/metadata methods, registry extractors, identity helpers, occurrence
normalizers, and validators. Treat these as a prioritized review list, not an
instruction to create one generic D&D engine. A worthwhile helper must preserve
domain-specific identity, evidence, kind ordering, validation, diagnostics,
and artifact typing.

### 9. D&D Extraction And Normalization Flows

Trace each lane end to end rather than auditing only similarly named files:

- spells;
- NPC registry and NPC occurrences;
- combat turns and enemy events;
- item registry and item occurrences;
- scene descriptions; and
- location registry and location occurrences.

For registry/occurrence pairs, verify the complete semantic boundary: the model
uses contextual evidence, deterministic code attaches opaque identity, registry
evidence does not become occurrence evidence, and unresolved or ambiguous
selections fail according to lane policy.

Review whether any lane resolves or canonicalizes the same entity, source
reference, or response twice; constructs unnecessary intermediate response
forms; performs repeated sorts or scans; or retains transitional paths. Compare
registry normalizers and occurrence normalizers for genuinely identical
mechanics, while keeping currency, same-name location, NPC ambiguity, spell
catalog, scene eligibility, and combat-specific policy with their owners.

### 10. Test Suite And Comment Coverage

Review the tests associated with every preceding area after understanding the
production contracts. This should be a cross-cutting pass, not a request to add
tests for every flagged function.

Identify:

- consequential unprotected invariants;
- duplicated policy assertions across layers;
- brittle tests coupled to internal constants, prompt prose, or private helper
  shape;
- oversized test harnesses and repeated fixtures that obscure intent;
- race-sensitive code not exercised under `-race`;
- parsers, canonicalizers, and path handlers where fuzzing would address a real
  input-space risk; and
- complex production code whose tests reveal an unclear ownership boundary.

Also identify necessarily complex symbols that lack a concise invariant-level
comment. Comment recommendations should name the exact symbol and the fact the
comment should explain; “add more comments” is not an actionable finding.

## Efficiency Evaluation

The audit should consider both runtime and maintenance efficiency.

For runtime efficiency, examine algorithmic behavior relative to realistic
input dimensions: source units, chunks, lanes, references, artifacts, registry
records, checkpoint files, and prompt assets. Prioritize repeated full-input
passes, nested linear lookup, unnecessary JSON round trips, repeated hashing,
large defensive copies at adjacent ownership boundaries, and serialization on
concurrent hot paths. Preserve a defensive copy when it establishes ownership;
removing it solely to reduce allocation is not an improvement.

For maintenance efficiency, prioritize repeated policy, parallel type systems,
duplicated error classification, scattered defaults, and migrations that left
two ways to perform the same operation. Boilerplate is costly only when it can
drift or obscures the semantic core. Small explicit typed adapters can be more
maintainable than a generic abstraction.

Do not recommend caching, pooling, concurrency, or a benchmark without naming
the workload and risk it addresses. Add a benchmark only when a proposed
optimization concerns a repeatable local path and the result would influence
the decision.

## Audit Method

Each area should use the same method:

1. Read its architecture/internal documentation and focused tests.
2. Map public/package contracts and trace the main call paths.
3. Inspect high-fan-in, high-cognitive-complexity, nested-loop, and repeated-
   conversion symbols.
4. Review exact and near-duplicate code side by side, including callers and
   failure semantics.
5. Check dependency direction, ownership, aliasing, deterministic order,
   cancellation, and error classification.
6. Compare tests with the risks owned at that layer.
7. Record findings and inspected-with-no-finding areas before moving on.
8. Run focused read-only validation when it can confirm or refute a suspected
   problem.

Prefer the repository knowledge graph for symbol discovery, call tracing, and
similarity candidates. Use textual search for literals, diagnostics, config
keys, asset content, and stale names. Read complete implementations and tests
before reporting a metric-derived candidate.

## Execution Sequence

This document owns audit scope, questions, evidence standards, and the quality
bar. [Staged Codebase Audit Sequence](audit-sequence.md) is the sole canonical
owner of prompt order, stage boundaries, per-stage reading, validation commands,
and acceptance criteria. Do not derive or maintain a second sequence here.

The audit is executed as bounded prompts and writes its accumulated findings to
`docs/roadmap/audit.md`. Later stages must build on and reconcile earlier
evidence rather than concatenate independent reports. Implementation and
roadmap retirement remain separate work after maintainers review the completed
audit.

## Baseline And Validation

Before the first audit stage, record the commit under review and require a clean
worktree. Refresh the code knowledge graph so renamed or deleted code does not
produce false findings. Run the normal offline baseline:

```sh
go test ./...
go vet ./...
go build ./cmd/notarius
git diff --check
```

Run `go test -race` for packages with concurrency or mutable shared state,
especially `internal/framework/pipeline`, `internal/framework/llm`, state
packages, and D&D registries. A repository-wide race run is appropriate for
final verification if its cost remains reasonable.

Optional diagnostic commands should be used only when relevant:

- `go test -count=1` to rule out cache-masked failures;
- `go test -shuffle=on` to detect order coupling;
- focused fuzzing for existing or newly justified fuzz targets; and
- focused benchmarks or profiles for a specific efficiency finding.

The audit itself should not change tests to make the baseline pass. Record any
pre-existing failure and distinguish it from an audit finding.

## Deliverable Quality Bar

The completed audit should:

- cover every repository area listed above;
- distinguish defects from refactoring opportunities and comment requests;
- distinguish credible performance costs from aesthetic simplification;
- identify intentional duplication that should remain explicit;
- avoid recommendations that violate dependency direction or weaken typing;
- cite exact evidence and preserve named invariants for every finding;
- consolidate root causes rather than report many symptoms;
- rank independent work so a later implementation plan can stage it safely;
- recommend no code change whose expected benefit is smaller than its added
  abstraction or test-maintenance cost; and
- leave implementation and roadmap retirement to later work.

## Open Questions

None are required to begin the audit. If a later stage cannot determine whether
behavior is intentional from code, tests, policies, ADRs, or current
documentation, it should record the uncertainty and a recommended resolution
rather than silently treating preference as a defect.