Code Review Pass

A review is finished when a person can act on every finding without asking you a follow-up question.

Process

1. Get the actual diff. Reviewing from a description reviews the description. If you cannot see the change, ask for it.
2. Read the tests first. They state the intended behavior. If they do not tell you what the change is supposed to do, that is finding number one.
3. Read enough context around each change to know what the surrounding code assumes. A changed line is not reviewable alone.

4. Review along five axes, in this order. Order matters because it determines what you still have attention for.

   Correctness — Does it do what it claims? Empty, one, many, boundary, error path, concurrent, and the retry. Do the tests assert the behavior, or do they assert that the implementation is the implementation?

   Security — Untrusted input, server-side authorization, injection sinks, secrets, error messages that leak. See references/security-checklist.md. Anything on a trust boundary routes to security-hardening for depth.

   Architecture — Does it fit the patterns already here? Is the dependency direction right? Is a boundary being punched through for convenience?

   Readability — Naming, control flow, function size. A comment explaining what the code does is usually a name that should have been better.

   Performance — N+1, work in a loop that belongs outside it, an unbounded query, a missing timeout.

5. Rank by leverage. Lead with what would hurt in production. A style note above a missing authorization check makes the whole review skimmable.
6. Name one specific thing done well. Not flattery: it shows you read the change.
7. Give a fix with every finding. "This is fragile" is not actionable.

Severity

Do not inflate severity to be heard. Do not pad with suggestions to look thorough.

What to look for that tools do not catch

• A test that asserts the implementation rather than the behavior.
• Error handling that catches and continues, leaving the system in a half-state.
• A new code path with no test, hidden inside a diff that has many tests.
• An interface change with a caller in the repository that was not updated.
• A comment that no longer matches the code beneath it.
• A migration with no rollback.
• Authorization checked in the handler for one path and not for the sibling path added in the same diff.

Output

## Review: <target>

**Verdict:** approve | approve with changes | request changes
**Strength:** <one specific thing done well>

### Critical
- `path:line` — <problem>. <fix>.

### Important
### Suggestions

### Not reviewed
<what you could not see, and why it matters>

Verification

• Every finding has a file:line and a concrete fix.
• Findings cover more than one axis, or you say why the others did not apply.
• The verdict matches the findings: an open Critical means request changes.
• You state what you did not review. A silent gap reads as coverage.

Red flags