Audit

---
name: audit
description: >-
  Reviewing code that already exists for structural problems: responsibilities that have drifted
  together, names that no longer describe what they name, dependencies pointing the wrong way,
  and duplication that has hardened into divergence.
---

# Audit: Reading Code That Already Works

Everything you are looking at ships and passes its tests. That is the constraint. You are not
hunting for bugs; you are finding the places where the structure will make the *next* change
expensive or risky, and saying which ones are worth paying to fix now.

That framing matters, because a review that lists everything imperfect is indistinguishable
from a review that found nothing important.

## Read in this order

**1. The public surface.** Exported functions, classes and endpoints. This is what other code
depends on, so problems here have the largest blast radius and the highest migration cost.

**2. The names.** Read them without reading the bodies. Any name that turns out to be wrong
once you read the implementation is a finding, because every future reader will make the same
mistake you just did.

**3. The imports.** They show the dependency direction at a glance. Business logic importing a
web framework, an ORM or a mail client is the single most reliable structural smell.

**4. The longest functions.** Not because length is the problem, but because that is where
several responsibilities usually ended up together.

**5. The tests.** Tests are a readout of the design. Heavy mocking means tangled dependencies.
Long setup means complicated construction. Absent tests for the core logic usually means the
core logic is hard to reach.

## What to look for

**Responsibilities that drifted together.** Describe the unit in one sentence. If you need
"and", note where the seam is: which parts change for different reasons, and on whose schedule.

**Names that have gone stale.** A function called `validate_user` that also creates a session.
A `TempFile` class that outlives the request. A `config` parameter that is actually a feature
flag. The code changed, the name did not, and the name is what the next reader trusts.

**Dependency direction.** Which module could not be tested, reused or moved without dragging
something heavy behind it? That is coupling with a cost you can name.

**Duplication that has diverged.** Two copies that started identical and have drifted are worse
than either one alone, because a fix applied to one leaves the other wrong. Note which is
correct.

**Ambient state.** Module-level singletons, globals, and implicit context. Each makes a unit's
behaviour depend on something not visible at its call site.

**Boolean parameters at call sites.** `render(rows, True, False)` cannot be read. Note it where
the call sites are numerous enough for the confusion to be real.

**Comments explaining what the code does.** Usually a sign the code does not say it. The
comment is not the finding; what forced it is.

## Ranking, which is most of the value

An unordered list of twenty observations moves nothing. Rank by expected cost of leaving it,
which is roughly *how often this code changes* multiplied by *how badly the structure hurts
when it does*.

- **High:** misleading names on widely-called functions; business logic that cannot be tested without infrastructure; diverged duplication where one copy is wrong.
- **Medium:** long multi-responsibility functions in code that changes regularly; ambient state; unclear module boundaries.
- **Low:** long functions in stable code nobody touches; naming that is imperfect but unambiguous; local inconsistency in a file that is otherwise clear.
- **Not a finding:** formatting, brace style, import order, and anything a tool should own. Raising these spends the reader's attention and teaches them to skim the rest.

## Say what you did not flag

A review that lists only problems reads as though everything unmentioned was examined and
approved. Name what you looked at and considered acceptable, and why:

> The `reports/` package has three near-identical builders. I left them: they render for
> different audiences and have changed independently twice, so the duplication is carrying real
> variation rather than hiding it.

That sentence is worth more than three more findings, because it tells the reader what your
threshold was.

## What good output looks like

- **Anchored to lines.** `orders.py:142, apply_discount` is reviewable; "the discount logic" is not.
- **Problem before fix.** State what the structure costs, then what you would do.
- **Ordered by cost**, with the ordering visible, so the reader can stop partway and still have covered the important ones.
- **Sized honestly.** If three findings matter, give three. Padding to a round number buries them.
- **With the shape shown.** A short before-and-after communicates faster than a paragraph.

## What to avoid

**Rewriting to taste.** Code that is consistent and clear but not your style is not a finding.
Local consistency beats individual improvement.

**Findings that are true everywhere.** "Consider adding tests" and "reduce coupling" apply to
every codebase and therefore inform nothing about this one.

**Reviewing the domain instead of the code.** Whether the discount rules are correct is a
question for whoever owns them; whether they are legible and changeable is yours.

**Proposing a rewrite.** A large restructuring cannot be reviewed, so it will be merged on
trust. Propose the smallest change that removes the problem you named.

**Counting lines as complexity.** A hundred-line function doing one thing in sequence is easier
to follow than five short ones threading state between them.