Prepare the codebase audit plan

This commit is contained in:
2026-08-11 14:05:30 +00:00
parent 31f2ce3a09
commit ebf1602635
3 changed files with 706 additions and 21 deletions

View File

@@ -0,0 +1,635 @@
# Codebase Audit Sequence
## Purpose
This document defines the staged sequence for auditing Promptkit before further
feature development. The audit is intended to identify high-confidence
opportunities to improve correctness, efficiency, duplication, implementation
clarity, and test-suite quality without changing production behavior during the
review itself.
The audit findings belong in `audit.md`. A later, separate planning pass will
translate accepted findings into a staged remediation plan in
`implementation.md`. Neither this sequence nor the findings log owns current
behavior; the canonical sources identified by the
[documentation policy](../policy/documentation.md) remain authoritative.
Each stage below is deliberately scoped for one LLM coding-agent prompt. Run
the stages in order and do not combine them. A stage may discover a concern
outside its scope, but it should record that concern for the owning later stage
rather than expanding its own review.
## Governing Policies And Boundaries
Every stage must follow:
- the [development guide](../development.md), including its task-specific
reading guide;
- the [architecture policy](../policy/architecture.md), especially the public
facade, internal-package, dependency-direction, and consumer boundaries;
- the [testing policy](../policy/testing.md), including its risk-based,
behavior-oriented standard; and
- the [documentation policy](../policy/documentation.md), including canonical
ownership and the temporary nature of roadmap documents.
This is an audit, not an implementation pass:
- Do not change production code, tests, examples, fixtures, public contracts,
or current-state documentation.
- Limit repository edits to the audit artifacts explicitly authorized for the
stage.
- Do not silently repair an issue while investigating it.
- Do not treat coverage, complexity, similarity, lint, or graph output as a
finding without confirming the underlying behavior in source and tests.
- Do not recommend centralization merely because code looks similar. The code
must implement the same semantic rule, and consolidation must improve
ownership or reduce a credible drift risk.
- Do not recommend performance work without identifying a relevant execution
path and establishing a defensible cost model, measurement, or complexity
problem.
- Preserve unrelated working-tree changes. Record the audit baseline rather
than requiring an otherwise unrelated dirty tree to be cleaned.
## Finding Standard
Record each actionable finding in `audit.md` with:
- a stable ID in the form `SNN-FNN`, where the first number is the stage;
- category: correctness, efficiency, duplication, clarity, testing, or
contract-documentation consistency;
- severity: critical, high, medium, or low;
- confidence: confirmed, high, medium, or low;
- affected packages, files, symbols, and tests;
- the contract, invariant, policy, or maintenance concern at issue;
- concrete evidence and a concise explanation of the failure mode or cost;
- the recommended direction, without implementation-level sequencing;
- the verification or regression protection that remediation would require;
and
- status: accepted, deferred, rejected, superseded, or resolved.
Use **confirmed** confidence when the problem is reproduced or follows
unavoidably from a complete trace. Use **high** confidence when direct source
and test evidence establishes the problem but a safe reproduction is not
practical. Medium- and low-confidence concerns belong in a separate
observations section until a later stage confirms or rejects them; they must
not enter the remediation plan as if they were findings.
Severity describes impact, not implementation effort:
- **Critical:** credible data disclosure, data corruption, deadlock, unbounded
resource consumption, or a broadly unusable public contract.
- **High:** violation of an important public contract or invariant, a likely
concurrency or resource-lifecycle defect, or a failure with substantial
downstream impact.
- **Medium:** a real but narrower behavioral defect, meaningful avoidable cost,
duplicated policy with credible drift risk, or a material testing gap.
- **Low:** a bounded clarity, maintainability, or testing-friction problem with
a concrete improvement and little behavioral risk.
When a reviewed area yields no finding, record the important behavior or risk
that was inspected and found adequately implemented or tested. This coverage
ledger prevents later reviewers from mistaking silence for omission.
## Per-Stage Procedure
Unless a stage says otherwise, its single agent prompt should:
1. Read the required policies, focused internal documentation, production
files, and tests for that stage.
2. Use the code knowledge graph for symbol discovery, callers, callees, and
cross-package traces; confirm important conclusions against source.
3. Trace normal, boundary, and failure paths through the narrowest relevant
public or package contract.
4. Review correctness, meaningful runtime cost, semantic duplication,
responsibility clarity, and the value and ownership of tests in scope.
5. Run the narrowest existing tests needed to validate conclusions. Use
race-enabled or repeated focused tests when concurrency or nondeterminism is
in scope. Do not add permanent tests during the audit.
6. Add the stage result to `audit.md`: accepted findings, unresolved
observations, areas verified, commands run, and any handoff to a later
stage.
7. Recheck the working tree and confirm that only the authorized audit artifact
changed.
## Stage 0: Initialize The Audit And Establish The Baseline
Create `audit.md` and establish a reproducible starting point before reviewing
individual components.
Record:
- the audited commit, branch, Go version, module identity, and working-tree
state;
- unrelated pre-existing changes that all later stages must preserve;
- the implemented package and public-facade inventory;
- the baseline validation results; and
- the finding template, status vocabulary, and coverage ledger used by later
stages.
Refresh the code knowledge graph for the recorded commit. Run the repository's
ordinary tests, race tests, vet, build, maintained offline preparation example,
Go formatting check, Markdown link check, and repository-hygiene checks. Run
package coverage once as a diagnostic and record the result without defining a
coverage target or committing generated output. Measure coarse package test
duration only if it can be done without adding tooling or changing tests.
Compare the validation requirements stated by the testing policy, development
guide, and release procedure. Record a finding if their ownership or command
sets are materially inconsistent; do not edit those documents in this stage.
**Exit condition:** `audit.md` contains the baseline, ledger structure, and
validation result, and no component-level audit has begun.
## Stage 1: Public Values, Conversion, Errors, And Formatting
Review the root facade's public request, result, inspection, prepared-run, and
error values together with public-to-internal and internal-to-public
conversion. Scope the review to `doc.go`, `types.go`, `convert.go`, `errors.go`,
`capacity_error.go`, `formatting.go`, and `prepared_execution.go`, plus the
directly relevant portions of root tests.
Focus on:
- zero-value and nil behavior;
- defensive copying, aliasing, and immutable snapshots;
- lossless conversion and field precedence;
- error identity through `errors.Is` and `errors.As`;
- containment of internal representations;
- safe `String`, `GoString`, and diagnostic formatting;
- accidental disclosure of credentials, prompt content, generated content, or
other private state; and
- conversion or copying logic that represents the same rule in multiple
places.
Review only tests that own these value and boundary contracts. Defer engine
assembly, execution coordination, and transport behavior to their later
stages.
**Exit condition:** all root value-conversion and error-formatting paths have a
recorded audit result without evaluating engine orchestration.
## Stage 2: Public Configuration And Extension Adapters
Review the smaller public construction and extension surfaces in
`backends.go`, `profiles.go`, `artifact_reader.go`, `json.go`, and
`llm_adapter.go`, together with their directly relevant root and internal
adapter tests.
Focus on:
- validation performed at the public boundary;
- ownership and copying of caller-supplied maps, slices, filesystems, readers,
and clients;
- adapter error propagation and cancellation;
- consistency between convenience constructors and general configuration;
- whether extension interfaces are as narrow as their consumers require;
- whether public helpers duplicate internal policy or merely translate it;
and
- whether tests protect consumer-visible behavior rather than private adapter
choreography.
Do not review how `NewEngine` combines these values; that belongs to Stage 3.
**Exit condition:** every non-engine public configuration helper and adapter
has a recorded result and any assembly questions are handed to Stage 3.
## Stage 3: Engine Construction, Options, And Source Assembly
Review the construction and configuration portions of `engine.go` and the
corresponding tests in `engine_test.go`. Limit the scope to `NewEngine`, option
application, dependency defaults, backend registration, profile and prompt
source composition, fallback-profile placement, validator and client
selection, capacity-manager construction, and construction-time validation.
Focus on:
- deterministic option precedence;
- required versus optional dependencies;
- isolation between engine instances;
- freezing or copying consumer configuration at the correct boundary;
- correct dependency direction and absence of process-global mutable state;
- failure atomicity and useful public errors;
- consistency between configured backends and capacity policies; and
- assembly logic that is repeated or split across unclear owners.
Do not audit the runtime behavior of `Run`, `Prepare`, or inspection methods;
that belongs to Stage 4 and the internal use-case stages.
**Exit condition:** engine construction and source assembly are fully accounted
for, including tests, without expanding into runtime orchestration.
## Stage 4: Engine Operations And Root Contract Coverage
Review the remaining public methods in `engine.go` and their directly relevant
root tests, including the external-package contracts in
`public_contract_test.go` and `prepared_execution_contract_test.go` only where
they exercise the engine boundary under review.
Focus on:
- request translation and context propagation;
- ordinary run, preparation, inspection, and prepared-execution entry points;
- public error mapping and preservation of injected dependency errors;
- result and prepared-state ownership;
- consistency between method and package-level convenience functions;
- public behavior that is asserted redundantly in root internal tests and
external-package contract tests; and
- important public behavior that is tested only through internal packages.
Treat internal runner, transport, validation, and capacity mechanics as black
boxes in this stage. Hand questions about their implementation to their owning
later stages.
**Exit condition:** the public execution boundary and its contract-test
ownership are recorded without duplicating internal component audits.
## Stage 5: Internal Domain And JSON-Compatible Values
Review `internal/domain` and `internal/jsonvalue`, including all of their tests.
Focus on:
- domain invariants and invalid states;
- session normalization;
- prepared-run and schema immutability;
- deep-copy correctness for every supported JSON-compatible shape;
- numeric-type preservation and rejection policy;
- cycles, excessive nesting, unsupported values, and nil distinctions;
- avoidable repeated copying on execution paths; and
- whether generic value machinery has a single clear owner.
Trace important callers to confirm that these packages enforce the invariants
their consumers assume, but do not audit the callers' broader behavior.
**Exit condition:** shared value semantics and their test ownership are fully
recorded.
## Stage 6: Backend Registry, Defaults, And Built-In Profiles
Review `internal/backend`, `internal/defaults`, and
`internal/profile/builtin`, including their focused tests and the relevant
backend-policy traces into engine assembly and the LLM reserved-field rule.
Focus on:
- immutable registry construction and lookup;
- built-in versus consumer ID collision rules;
- endpoint, credential-environment, header, parameter, and concurrency
validation;
- defensive copies at registry boundaries;
- application-neutral default ownership;
- built-in profile/backend consistency;
- reserved request-field ownership without dependency inversion; and
- duplicated validation or default policy across public and internal layers.
Defer scheduling mechanics to Stage 15 and actual HTTP request construction to
Stage 14.
**Exit condition:** registry and default-policy correctness are recorded, with
transport and scheduling questions handed to their owning stages.
## Stage 7: File Discovery And Prompt Definitions
Review `internal/filecatalog` and `internal/promptdef`, including their tests
and fixtures. Read the framework format reference and internal source document
before evaluating behavior.
Focus on:
- deterministic discovery and duplicate handling;
- filesystem and `fs.FS` parity;
- root and relative-path normalization;
- strict YAML decoding and version selection;
- prompt ID, message, input, cache-control, and validation declarations;
- inline versus file-backed content rules;
- containment of referenced files where promised;
- malformed input and contextual error behavior;
- unnecessary repeated directory scans or file reads; and
- fixture and case duplication that does not protect distinct parser risks.
Do not audit rendering, artifact loading, profile loading, or schema validation
in this stage.
**Exit condition:** discovery and prompt-definition parsing have complete
findings and coverage-ledger entries.
## Stage 8: Profile Sources And Repository Composition
Review `internal/profile` excluding its built-in subpackage, including all
repository tests and profile fixtures. Read the profile format contract first.
Focus on:
- strict decoding and profile validation;
- filesystem and `fs.FS` parity;
- repository overlay and fallback precedence;
- distinction between absence and a malformed authoritative source;
- preservation of useful error identity and context;
- conversion to immutable execution profiles;
- duplicate IDs and deterministic selection;
- repeated parsing, validation, or copying; and
- whether tests at repository, engine, and public-contract layers have clear,
nonduplicative ownership.
Defer resolution of a profile with runtime overrides and backend definitions to
Stage 11.
**Exit condition:** profile-source and repository-composition behavior are
fully recorded.
## Stage 9: Artifact Loading And Prompt Rendering
Review `internal/artifact` and `internal/prompt`, including all focused tests.
Read the internal source document and format reference first.
Focus on:
- inline and file artifact ownership, metadata, hashing, and error behavior;
- copied versus shared byte storage;
- caller-selected path semantics and architecture-policy boundaries;
- template parsing and execution;
- artifact, variable, session, and cache-control rendering;
- missing, extra, nil, and malformed input behavior;
- deterministic output and safe diagnostics;
- unnecessary repeated reads, hashes, parses, or allocations on common paths;
and
- tests coupled to incidental template or struct implementation.
Do not audit the runner's decision about when rendering occurs.
**Exit condition:** input materialization and rendering are accounted for
through their package boundaries.
## Stage 10: Output Validation And Frozen Validation Plans
Review `internal/validate`, including all tests, schema fixtures used by the
root contract suite, and traces from preparation into frozen validation plans.
Read the format and internal source documents first.
Focus on:
- basic, JSON, and JSON Schema mode semantics;
- schema-path resolution and filesystem/`fs.FS` parity;
- schema compilation, transitive references, and source-lifetime independence;
- output normalization and preservation;
- malformed schema and malformed model-output errors;
- thread safety of reusable validators and prepared plans;
- expensive recompilation or copying on repeated execution; and
- whether parser, validator, runner, and public tests each own distinct risks.
Do not audit repair decisions or provider request construction.
**Exit condition:** validation behavior, plan lifetime, and focused test value
are fully recorded.
## Stage 11: Inspection And Execution-Target Resolution
Review `internal/usecase/profile_inspection.go`,
`internal/usecase/prompt_inspection.go`, and the preparation and target-
resolution portions of `internal/usecase/runner.go`, together with their
focused tests. Use graph traces to define the exact helper and call-path scope
before reviewing.
Focus on:
- prompt and profile selection;
- backend lookup and endpoint overrides;
- reasoning, session, and other runtime precedence;
- merge semantics for default, profile, backend, and per-run values;
- inspection fidelity versus actual execution;
- credential-name versus credential-value handling;
- prompt-definition and schema freezing during preparation;
- stable error identity and context; and
- duplicated resolution rules across inspection, preparation, and execution.
Do not review model invocation, repair execution, or prepared-handle lifecycle;
those belong to Stages 12 and 13.
**Exit condition:** all selection, merge, inspection, and preparation rules are
traced and recorded once.
## Stage 12: Ordinary Execution, Validation, And Repair Coordination
Review `internal/usecase/runner.go`, `internal/usecase/repairer.go`, and
`internal/usecase/capacity_error.go` only for the ordinary execution path after
preparation, together with the corresponding sections of `runner_test.go`.
Use the Stage 11 resolution result as an established input rather than
reauditing it.
Focus on:
- rendering, generation, validation, and optional repair transitions;
- context cancellation and dependency-error propagation;
- partial result and usage accounting;
- exact attempt count and repair eligibility;
- avoidance of unintended retries;
- capacity-error translation;
- cleanup and failure behavior on every exit path;
- repeated orchestration or request construction; and
- oversized tests, helpers, or case matrices that obscure distinct behavior.
Treat LLM transport and capacity scheduling as injected package contracts;
their mechanics belong to Stages 14 and 15.
**Exit condition:** the ordinary execution state machine and its test ownership
are fully recorded.
## Stage 13: Prepared Execution Lifecycle
Review `internal/usecase/prepared_execution.go`, its focused tests, and the
prepared-execution portions of the root facade and external contract tests.
Do not repeat the public value review from Stages 1 and 4 or the resolution
review from Stage 11.
Focus on:
- single-attempt or other lifecycle guarantees;
- concurrent use and synchronization;
- discard behavior and resource release;
- frozen source, target, credential, capacity, timing, and schema semantics;
- independence of returned details and results;
- context and error behavior;
- consistency between ordinary and prepared execution where promised;
- private-state containment in formatting; and
- redundant assertions across internal, root, and external-package tests.
Run focused race tests and repeated tests for lifecycle behavior where useful.
**Exit condition:** prepared execution has one complete lifecycle analysis and
a clear map of which test layer owns each guarantee.
## Stage 14: OpenAI-Compatible Transport
Review `internal/llm`, including all transport tests. Read the
OpenAI-compatible integration contract and internal LLM document first.
Focus on:
- request endpoint, headers, authentication, and JSON body construction;
- omission versus explicit zero-value behavior;
- reserved-field enforcement and extra-parameter collision handling;
- session ID and reasoning encoding;
- structured-output and cache-control translation;
- client and per-generation deadlines;
- cancellation, body closure, bounded response reads, and decode failures;
- non-success HTTP response behavior;
- response choices, usage, and malformed-success handling;
- wire-visible compatibility and safe error disclosure;
- unnecessary marshaling, copying, or buffering; and
- whether the large transport test file can be simplified without losing
protocol-risk coverage.
Use `httptest`-based existing tests; do not contact a live provider.
**Exit condition:** every outbound and inbound wire path has a recorded result,
including focused test ownership.
## Stage 15: Capacity, Admission, And Concurrency
Review `internal/capacity`, its tests, `capacity_contract_test.go`, and the
integration points already identified in engine and use-case stages. Read the
internal capacity document first.
Focus on:
- bounded run admission and queue-capacity enforcement;
- per-backend limited and unlimited scheduling;
- FIFO behavior and cancellation-safe waiter removal;
- permit release on success, error, panic-relevant boundaries, and
cancellation;
- goroutine, timer, and waiter lifecycle;
- starvation, deadlock, race, and engine-isolation risks;
- lock scope and meaningful contention or allocation costs;
- preservation of injected-client concurrency where promised;
- relational testing of configured limits rather than duplicated defaults;
and
- duplication between internal concurrency tests and public contract tests.
Run focused ordinary, race-enabled, and repeated tests. Repetition must remain
bounded and diagnostic; a test that passes many times is not proof of
correctness without a source-level synchronization analysis.
**Exit condition:** concurrency invariants have both a source trace and a
test-ownership assessment.
## Stage 16: Repository-Wide Test Strategy And Maintained Examples
Perform a suite-level review after every component has been audited. Review
the testing policy, test inventory, fixtures, external-package root tests,
`architecture_test.go`, and both maintained examples. Use the component-stage
coverage ledger instead of repeating every individual test assertion.
Construct a risk-to-owner matrix for:
- public compatibility and error identity;
- parsing, validation, and serialization;
- immutability and data integrity;
- external wire behavior;
- cancellation, failure propagation, and recovery;
- concurrency and resource lifecycle; and
- representative assembled consumer workflows.
Identify only evidence-backed cases of:
- consequential behavior with no credible test owner;
- the same semantic rule asserted redundantly at several layers;
- tests coupled to private helpers, internal constants, exact noncontractual
wording, or collaborator choreography;
- low-value or obsolete cases whose lifetime cost exceeds their protection;
- missing failure, cancellation, race, or boundary coverage;
- nondeterminism, shared state, environment dependence, fixed ports, or test
ordering assumptions;
- helpers and fixtures whose complexity is not justified; and
- maintained examples that duplicate one another without protecting distinct
workflows.
Use coverage and timing only to direct attention. Do not propose tests solely
to raise percentages or remove tests solely to shorten the suite.
**Exit condition:** every important risk has a named test owner or an accepted
finding, and every proposed test deletion or consolidation states what
protection remains.
## Stage 17: Cross-Cutting Duplication, Efficiency, And Architecture Review
Review the codebase as a whole using the completed component findings, graph
traces, complexity signals, similarity signals, and package dependency map.
Do not reopen settled package behavior without new cross-cutting evidence.
Focus on:
- one semantic policy implemented by multiple packages;
- repeated public/internal transformations with credible drift risk;
- interfaces broader than their actual consumers;
- responsibilities split across packages or concentrated in the facade
contrary to the architecture policy;
- repeated parsing, copying, schema compilation, request construction, or
source traversal on important paths;
- avoidable lock contention or serial work supported by the concurrency audit;
- abstractions that add indirection without enforcing a boundary; and
- discrepancies between implemented package responsibilities and their
canonical architecture or internal documentation.
For each possible consolidation, state why the code represents one rule, which
package should own it, and why the resulting dependency direction remains
valid. For each efficiency finding, state the path frequency, input scale,
complexity or measurement evidence, and the benchmark or invariant needed to
verify a remediation.
**Exit condition:** all cross-cutting opportunities are either accepted with
high confidence, retained as explicitly lower-confidence observations, or
rejected with a short rationale.
## Stage 18: Consolidate And Close The Audit
Perform a findings-only synthesis. Do not change code and do not write the
remediation plan yet.
- Recheck every accepted finding against the final audited tree.
- Merge duplicates and mark superseded IDs without erasing their history.
- Separate shared root causes from downstream symptoms.
- Confirm that every accepted item is confirmed or high confidence.
- Confirm that severity describes impact rather than effort.
- Reject speculative cleanup, coverage-driven test work, and centralization
without a clear owner or drift risk.
- Record dependencies and a recommended remediation order.
- Distinguish behavioral fixes, safe refactors, performance work, test gaps,
test consolidation, and documentation synchronization.
- Add an audit summary stating what was reviewed, what validation ran, the
accepted finding counts by category and severity, and any residual
uncertainty.
- Re-run baseline validation if audit-only investigation could have affected
repository state, and confirm that only authorized roadmap files differ from
the recorded baseline.
The recommended ordering should place correctness, data-integrity,
resource-lifecycle, and concurrency defects first; policy duplication and
missing protection for consequential behavior next; then clarity, test
consolidation, and demonstrated efficiency improvements. Actual implementation
stages must be decided in the later `implementation.md` planning pass, where
files, dependencies, acceptance criteria, and validation can be made
decision-complete.
**Exit condition:** `audit.md` is a complete, internally consistent input to a
separate remediation-planning prompt, with no code or test changes mixed into
the audit.
## Completion Criteria
The audit is complete only when:
- every production component and public boundary appears in the coverage
ledger;
- every test file and maintained example has been reviewed at its owning stage
or in the suite-wide stage;
- important cross-package paths have been traced end to end;
- concurrency-sensitive behavior has received source and race-test review;
- every accepted finding meets the evidence and confidence standard;
- lower-confidence observations are visibly separated from remediation
candidates;
- proposed test additions, deletions, and consolidations are justified against
the testing policy;
- proposed simplifications identify a durable responsibility owner;
- proposed efficiency work has a relevant cost model or measurement plan; and
- the repository remains unchanged except for the authorized audit roadmap
artifacts.