Reviewing AI-written code
What to cut before it merges.
Coding agents now write a large share of new code. They're fast, thorough and good at following explicit rules, and they have recognizable habits that work against readability. Reviewing their output is mostly about subtraction.
RP15
Review AI-written code for what to delete.
An agent rarely leaves something out. It tends to add: extra defensiveness, extra generality, extra explanation. The fastest review question is "what here could go?"
Common habits, and the fix
| Habit | Looks like | Fix |
|---|---|---|
| Narrated comments | Comments restating each line, numbered step comments, essays in docstrings | Keep only why comments (chapter 6) |
| History in code | "Previously…", "This replaces…", references to PRs, tickets and design-doc sections | Move it to the PR description |
| Citations as justification | "Matches how framework X does it (issue #12345)" | Justify with a measurement or a requirement, or drop the claim |
| Defensive wrappers | try/except Exception around everything, getattr(x, "y", None) on objects whose type is known, re-validating already-validated input | Fail loudly at boundaries; trust inside (chapter 7) |
| Speculative generality | Protocols with one implementation, option flags nobody sets, "basic" and "advanced" interfaces, plugin registries | Inline until the second case (chapter 9) |
| Helpers that mutate | _admit(request, tokenizer, prompts, generated, budgets) filling in the caller's dicts | Make it a method on the owner (chapter 4) |
| Parallel state | Several dicts keyed by the same id, updated together in every branch | One object per entity (chapter 3) |
| Concurrency by default | Extra threads, executors, streams or pipelining "for performance" with no profile | Remove, or require a timeline (chapter 8) |
| Giant PRs | 1,000+ lines landing at once, design and implementation together | Ask for a short design note first, then small PRs |
| Style noise | Markdown bold and em-dashes in docstrings, emoji in logs | Plain text |
A review checklist
- Read the main loop or entry point first. Can you say what the change does from that alone? (RP02)
- For each new piece of state: who owns it? Is it duplicated anywhere? (RP04)
- For each new class or protocol: how many implementations exist today? (RP13)
- For each
except: what exactly is caught, and is this the layer that should decide? (RP10) - For each thread, task, stream or cache: where's the profile? (RP12)
- For each comment: does it say why? (RP09)
- Do the test names, read alone, describe the behaviour? (RP14)
Prevention: tell the agent up front
Agents follow written instructions well. Put the rules where they'll be read, e.g. AGENTS.md or CLAUDE.md at the repo root:
## Code style
Follow the Readable Python booklet: <link to this site>
- The top-level loop is a list of named steps; one class, one job.
- Every piece of state has one owner. No parallel dicts. Enums for status.
- Functions don't mutate their arguments. At most 4 parameters.
- Comments explain why only. No history, PR or design-doc references in code.
- Catch specific exceptions only, at the layer that handles them.
- No new threads, CUDA streams, caches or pipelining without a profile in the PR.
- No abstraction (Protocol, registry, option flag) until a second implementation exists.
- Keep PRs under ~400 lines; propose a design note first for anything larger.Then review against the same list. When the instructions and the review agree, the agent's next attempt is usually right.