Files
narratio/docs/roadmap/audit-plan.md

21 KiB

Codebase Audit Plan

Status: proposed

Purpose

This audit will evaluate Narratio for correctness, efficiency, maintainability, and test-suite value. It will identify defects and credible risks, duplicated or near-duplicated behavior, code that can be made smaller or more idiomatic, and complex code whose remaining invariants need focused explanation.

The audit is investigative. It should produce evidence-backed findings and a prioritized remediation backlog, not make opportunistic production changes as it proceeds. The Audit Sequence assigns this scope to concrete execution stages.

Authoritative Baseline

Review implemented behavior against its canonical owner rather than treating the current implementation or tests as the specification:

Where code, tests, and documentation disagree, record the disagreement. Do not assume which one is wrong until the canonical contract and caller expectations have been traced.

Audit Principles

  1. Review correctness before cleanup. A shorter implementation is not an improvement if it weakens a state transition, safety check, or external contract.
  2. Trace behavior across boundaries. Narratio's most important properties often emerge from the interaction of application orchestration, stages, manifests, artifact resolution, filesystem operations, and adapters.
  3. Distinguish repeated syntax from repeated policy. Extract a helper only when the behavior has one stable owner and the shared abstraction makes that ownership clearer. Similar stage code may be intentionally explicit.
  4. Prefer narrow, idiomatic Go over generic frameworks. In particular, proposed refactors must preserve the explicit canonical stage sequence and must not turn Narratio into a workflow engine or a second configuration system for downstream tools.
  5. Optimize credible work. Flag repeated I/O, hashing, serialization, remote calls, subprocess work, allocation, or poor asymptotic behavior when the relevant path can matter. Require a benchmark or workload argument for performance changes whose benefit is not evident.
  6. Treat comments as explanations of intent. Recommend comments for invariants, ordering constraints, non-obvious failure policy, or security reasoning—not as narration of ordinary Go or a substitute for simplifying code.
  7. Judge tests as a suite. A test can be locally reasonable and still add no marginal protection, while a compact test can be inadequate for a consequential cross-component failure.

Evidence And Finding Standard

Begin from a cleanly identified revision and record toolchain and platform assumptions. Use the code knowledge graph to find ownership, callers, callees, similarity candidates, high-complexity functions, and weakly protected boundaries. Confirm every candidate by reading the implementation, its focused tests, and the applicable contract. Text search and static analysis supplement the graph for literals, configuration, generated files, and patterns that are not modeled reliably.

Each finding should record:

  • category: correctness defect, correctness risk, duplication, simplification, efficiency, architectural boundary, comment/clarity, or test-suite issue;
  • source locations and the affected contract or invariant;
  • concrete evidence and a realistic failure or maintenance scenario;
  • impact, likelihood, confidence, and estimated remediation scope separately;
  • the smallest plausible improvement and its intended owner;
  • tests that already protect the behavior, tests that should change or be added, and tests that may become redundant; and
  • dependencies on, or conflicts with, other findings.

Do not report a metric alone as a finding. Complexity, similarity, coverage, fan-in, file size, and test count are prioritization signals that require manual confirmation. Consolidate findings that share one root cause.

Cross-Cutting Review Lenses

Correctness And Pipeline Semantics

Construct an explicit lifecycle matrix for every stage outcome: first run, already-succeeded skip, self-skip, failure, interruption, forced replacement, non-resumable result, and successful rerun. Trace how each outcome changes the session manifest, invocation manifest, downstream stage state, artifacts, diagnostics, and cleanup eligibility.

Across the pipeline, verify:

  • the registry exposes one deterministic canonical order;
  • each stage's declared inputs, outputs, configuration, adapters, and manifest effects agree with its implementation and focused documentation;
  • inputs are resolved through manifest and artifact contracts rather than incidental directory contents;
  • run-local outputs are fully validated before canonical materialization;
  • failure, cancellation, or process interruption cannot advertise partial work as successful;
  • force and changed outcomes invalidate exactly the intended succeeded downstream work;
  • repeated execution is idempotent where promised, and ordering is stable wherever maps, directory reads, remote listings, or dependency graphs are involved;
  • session, campaign, run, source, checksum, contract, and external provenance identities cannot be confused across runs; and
  • errors preserve useful causes and do not expose secrets or private content.

Use fault-oriented reasoning at durability boundaries: fail immediately before and after manifest saves, canonical renames, external process completion, uploads, current-manifest publication, the current-run commit marker, restore manifest installation, and cleanup. Determine which state is authoritative and whether the next invocation recovers safely.

Duplication And Helper Ownership

Search for exact and semantic duplication in production and tests, including:

  • repeated stage setup, input resolution, output validation, run-local materialization, metadata construction, and error adaptation;
  • repeated manifest create/load/save and session/run transition handling;
  • repeated adapter construction, timeout parsing, command execution, generated configuration, log handling, and output checks;
  • repeated source-ID, destination, remote-key, and path validation policy;
  • repeated sorting, deduplication, checksum, copy, and atomic-write mechanics; and
  • repeated test fixtures and assertions that encode the same policy at several layers.

For each candidate, decide whether it is coincidental similarity, a repeated mechanism, or duplicated policy. Recommend extraction only when the helper can have a clear package owner, a narrow contract, and callers that become easier to understand. Prefer an unexported local helper when sharing is package-local. Do not create a broad utility package, force unlike stage results into one data model, or move policy into storage/file-operation helpers.

Initial similarity and complexity signals should seed, but not predetermine, inspection of the single-stage command wrappers, session/run manifest persistence pairs, adapter constructors, Scriptorium operations, stage fakes, and common stage materialization paths.

Simplification, Go Idioms, And Efficiency

Review long or branch-heavy functions for separable decisions, state transitions, or data transformations. Pay particular attention to orchestration, configuration validation, artifact dependency resolution, resume verification, restore/previous-cache planning, and analyze/publish selection logic. A useful refactor should reduce cognitive load while leaving the important ordering visible.

Check for:

  • unnecessary nesting, defensive branches made unreachable by earlier validation, repeated normalization, and overly wide parameter lists;
  • interfaces defined for hypothetical extensibility rather than a demonstrated consumer boundary;
  • manual slice, map, string, error, and filesystem logic with a clearer standard library form;
  • incorrect or inconsistent errors.Is/errors.As, wrapping, context propagation, deferred cleanup, response-body closure, process waiting, and goroutine/channel ownership;
  • redundant filesystem scans, stat/checksum passes, whole-file buffering, copying, YAML/JSON round trips, sorting, remote listings, downloads, uploads, or adapter initialization;
  • linear searches nested in loops and repeated dependency or artifact lookup that should use an indexed map or a single planning pass;
  • unbounded concurrency, leaked work after cancellation, serialized independent work, and nondeterministic result collection; and
  • obsolete dependencies, portability assumptions, and platform-sensitive path or atomic-rename behavior.

Keep correctness and diagnosability ahead of micro-optimization. When a simpler algorithm changes performance characteristics, specify the representative input size and validation method.

Comments And Local Explanation

Review high fan-in, high-complexity, security-sensitive, and commit-boundary code after likely simplifications have been identified. Add a comment recommendation when a maintainer needs to know why:

  • state transitions or persistence operations occur in a specific order;
  • a stale record intentionally retains data while another transition clears it;
  • a path is checked more than once to resist traversal, symlink replacement, or time-of-check/time-of-use hazards;
  • an artifact is accepted only with particular manifest, checksum, contract, or provenance evidence;
  • a partial operation is intentionally not rolled back;
  • a remote pointer or local manifest must be installed last; or
  • concurrency, cancellation, compatibility, or downstream-tool behavior makes an apparently simpler approach unsafe.

Prefer a named helper, typed state, or smaller control flow when that removes the need for explanation. Check existing comments for stale claims as well as missing rationale.

Test Suite Against The Canonical Policy

Build a risk-to-test matrix rather than auditing tests file by file in isolation. For each important behavior, identify its proper owner—parser, validator, domain package, adapter, orchestrator, CLI, integration, or end to end—and identify all tests that claim to protect it.

Evaluate:

  • protection of data integrity, destructive operations, compatibility, security, concurrency, idempotency, recovery, and partial failure;
  • manifest transitions, force/invalidation, resume validation, atomic materialization, publish commit order, restore install order, and cleanup gates as assembled behaviors;
  • realistic HTTP, subprocess, filesystem, and object-store boundary behavior, including cancellation and malformed responses;
  • whether higher-level tests intentionally sample lower-level behavior or redundantly reproduce its full policy;
  • whether tests assert durable outcomes or private constants, exact error text, incidental paths, call choreography, or oversized snapshots;
  • whether real fast collaborators could replace elaborate doubles, and whether stateful fakes are realistic enough for the risk they protect;
  • fixture/helper duplication, oversized test cases, and setup that obscures the behavior under test without introducing a heavyweight test framework;
  • deterministic, offline, credential-free, order-independent execution and safe handling of environment and process-global state;
  • focused fuzz candidates in parsing, normalization, source IDs, remote/local path mapping, manifest decoding, and configuration boundaries; and
  • the presence and value of a small number of representative assembled workflows.

Use coverage only to locate unexpectedly weak consequential branches. Also inspect packages with extensive coverage for redundant tests and refactoring friction. For every proposed addition, deletion, or consolidation, state the realistic defect and marginal confidence involved.

The audit baseline should include the repository's canonical commands plus targeted diagnostic runs where supported:

go test ./...
go test -race ./...
go vet ./...
go build ./cmd/narratio

Use focused repeated or shuffled runs to investigate state leakage and flakiness, and collect package/branch coverage for diagnosis. Review continuous integration to determine whether the appropriate offline validation is enforced; do not turn coverage percentage into a gate merely for this audit.

Area-By-Area Inspection Map

Area Primary locations What to inspect
Process and application boundary cmd/narratio, internal/app Command dispatch, configuration selection, production composition, secret loading, lock lifetime, object-store initialization, context/error propagation, and separation of CLI reporting from orchestration policy. Review operator commands for consistent current-state authority and shared read-only mechanics.
Stage registry and runner internal/stage/placeholders.go, internal/stage/stage.go, internal/app/planner.go, internal/app/runner.go, internal/app/run_stage.go Canonical order, action decisions, resume/force/self-skip/failure transitions, downstream invalidation, session/run manifest consistency, resource lifecycle, cleanup triggering, and opportunities to decompose the runner without hiding its state machine.
Configuration internal/config Strict decoding, discovery and precedence, centralized defaults, normalization, templating, validation order, unknown fields, empty-value behavior, secret references, cross-field constraints, path confinement, deterministic errors, duplicated validator policy, and compatibility with maintained examples.
Prepare and audio internal/stage/prepare.go, internal/audio, internal/previouscache Local/S3 exclusivity, cache and spool identity, partial downloads, checksum/reuse policy, previous-session required/optional planning, deterministic input records, clearing semantics, traversal safety, and avoiding repeated remote or filesystem work.
Transcript stages internal/stage/transcribe.go, merge.go, polish.go, normalize.go, trim.go, render.go Contract parity across similar stages, bounded concurrency and cancellation, deterministic speaker/input ordering, run-local validation and canonical promotion, report/diagnostic classification, disabled behavior, and narrow opportunities for shared mechanics.
Extraction internal/stage/extract.go, extract_resume.go, internal/adapters/notarius, internal/fileops/directory.go External receipt and lane validation, configuration fingerprint limits, immutable promotion, symlink/root replacement defenses, provenance and checksum checks, immediate and cross-invocation reuse, obsolete versus unsafe outcomes, failure residue, and whether dense verification logic can be clarified without weakening it.
Analyze and artifact dependencies internal/stage/analyze.go, internal/artifacts, internal/artifactpolicy Source-family validation, runtime catalog state, enabled/selected/reused distinctions, topological ordering and cycle handling, required/optional inputs, local-only previous sources, deterministic metadata, repeated lookup/scanning, and ownership shared with config and publish.
Publish and cleanup internal/stage/publish.go, internal/app/post_publish_cleanup.go, internal/app/cleanup_targets.go Prerequisite success, output selection, locks, required/optional behavior, exclusion rules, deterministic upload set, retry/idempotency implications, current-manifest then commit-marker ordering, metadata gates, and destructive path confinement.
Manifest state internal/manifest Validation and backward compatibility, atomic persistence, timestamps, session/run identity, transition truth table, clearing versus retaining payload, create/load/save duplication, failure during dual-manifest updates, and whether state mutation has a single owner.
Artifacts, paths, and policy internal/artifacts, internal/artifactpolicy, internal/pathsafe Canonical helper coverage, ad hoc reconstruction by callers, source-ID ownership, manifest-first resolution, extraction/current-state identity, destination normalization, stable ordering, typed missing-state errors, symlink/traversal defenses, and duplicate policy across config/stages/app.
Restore internal/app/restore*.go, internal/previouscache, internal/audio Remote authority, confined mapping, deterministic plan actions, local conflict and force behavior, dry-run purity, temp-file installation, manifest-last ordering, partial failure/retry behavior, report accuracy, cache reuse, and shared current-state mechanics.
File operations internal/fileops, internal/pathsafe, local-store code in internal/artifacts Atomic-write and promotion guarantees, permissions, close/sync/rename error handling, temp cleanup, same-filesystem assumptions, replacement policy, regular-file-only traversal, symlink and root-swap resistance, lock cleanup, and portability.
External adapters and storage internal/adapters, internal/audio Transport isolation, shared subprocess mechanics versus adapter-specific policy, command/config duplication, quoting and working directories, timeouts/cancellation, stdout/stderr separation, HTTP body and retry behavior, S3 pagination/streaming/not-found mapping, credential independence, and external error adaptation.
Shared models and diagnostics internal/artifactmodel, internal/contracts, internal/logging Serialization and validation invariants, unnecessary conversions, ownership of shared types, stable diagnostic structure, redaction, and whether small shared packages remain cohesive.
Tests, examples, and automation all *_test.go, examples/, .woodpecker/ Risk ownership, semantic duplication, fixture cost, policy-coupled assertions, realistic boundary tests, end-to-end sufficiency, default-suite isolation, example validation, diagnostic coverage, flakiness, runtime cost, and enforcement of canonical validation.

Narratio-Specific Cross-Boundary Scenarios

In addition to package-local review, trace these complete scenarios because a modular pipeline can look correct within every package while violating an end-to-end invariant:

  1. A stage succeeds, its result becomes non-resumable, the rerun fails, and a later invocation decides what remains usable.
  2. An upstream forced or changed outcome interacts with already-succeeded, self-skipped, and disabled downstream stages.
  3. Extraction produces a valid immutable bundle, then configuration or transitive Notarius inputs change before analyze or publish.
  4. Previous-session state is published, restored or prepared into the local cache, and consumed by analyze without an unintended remote read.
  5. Publish fails at each upload boundary, especially between current manifest and current-run pointer, followed by status, restore, and retry.
  6. Restore encounters identical files, conflicting files, unsafe remote keys, cache hits, and a failure immediately before manifest installation.
  7. Automatic or manual cleanup is requested after skipped, failed, locked, partially uploaded, and fully committed publish outcomes.
  8. Cancellation reaches bounded transcription work, HTTP requests, subprocesses, object storage, and manifest reporting without leaks or false success.
  9. A configured artifact is disabled, unselected, reused, generated from another artifact, sourced from extraction, or sourced from a previous session, then filtered for publish.
  10. The same session is invoked concurrently, including lock contention and cleanup/release failures.

Completion Criteria

The audit is complete when:

  • every area in the inspection map has been reviewed against its canonical contracts and focused tests;
  • the stage lifecycle matrix and cross-boundary scenarios have explicit conclusions;
  • duplication candidates have been classified rather than merely counted;
  • simplification and performance recommendations explain their correctness constraints and expected benefit;
  • comment recommendations identify the non-obvious rationale to preserve;
  • the test suite has a risk-based sufficiency assessment, including gaps, redundancy, durability, execution properties, and automation;
  • findings are deduplicated, evidence-backed, and ranked by risk and dependency; and
  • unresolved questions and intentionally accepted risks are recorded rather than silently omitted.

Execution

The Audit Sequence is the canonical owner of execution order, stage boundaries, checkpoints, validation, and audit deliverables. This document remains the canonical owner of audit scope, review criteria, and the finding standard.