Start a code review by identifying the promised behavior and the most consequential way the patch could be wrong. Then inspect the implementation and evidence against that contract. A convincing summary and a green test run are useful inputs, but neither tells you whether the tests exercised the risk you care about.
Three passes give the review a practical order: behavior and scope; contracts, risks, and tests; then maintainability and cleanup. The aim is to reach a clear accept, revise, or split decision.
First pass: map the behavior and scope
Read the bug report and changed-file list before following the diff from top to bottom. State the intended change in one sentence. Identify which files control that behavior and which changes appear unrelated.
Suppose a fourteen-file pull request claims to fix refund totals but also edits interface copy and reorders imports. Inspect the calculation and data-selection path before commenting on naming. Ask why each additional change belongs in this patch. Verify that the import-only files contain no behavioral edits before treating them as harmless.
A small bug fix that unexpectedly touches session handling deserves an explanation and separate scrutiny. If the patch has expanded beyond an agreed boundary, request a split or a narrower attempt. Preserve a clean reproduction test when it remains useful; reviewing unrelated refactors together with the repair increases the work needed to establish correctness.
Second pass: compare old and new contracts
Shorter code can change behavior. Write down representative inputs and expected outputs, especially where defaults, missing fields, ordering, or error handling matter.
Consider a feature-flag contract with three levels: a user override, an organization default, and a global default. An explicit false at the user level must win over an organization’s true. If all three are absent, the result is false.
This rewrite breaks that contract:
return user.override || org.default || false;
It treats an explicit false as though it were absent and drops the global default. If the contract defines absence as exactly null or undefined, the expression can preserve those distinctions:
return user.override ?? org.default ?? global.default ?? false;
The presence rule must be established before choosing the operator. An empty string or another sentinel may require different handling in a different contract.
Use cases that distinguish the implementations:
| User override | Organization default | Global default | Expected |
|---|---|---|---|
false |
true |
true |
false |
| absent | false |
true |
false |
| absent | absent | true |
true |
| absent | absent | absent | false |
These are expected outcomes for the stated contract, not evidence that a particular repository has passed the checks.
Trace authorization separately
For an export or download, inspect caller identity, tenant scope, resource ownership, and permission for the action. A logged-in caller is not automatically entitled to every resource. Permission to view a record does not automatically establish permission to export a collection.
Check whether the query constrains the records to the authorized scope and whether object lookups respect the same boundary. Inspect exported fields, logs, and filenames for sensitive information. Require negative cases for another tenant’s records or another customer’s identifier, plus a valid authorized case.
Keep findings distinct. Missing authorization and sensitive data in a log need separate explanations and checks. Audit evidence should establish who performed the action using appropriate metadata without copying private record contents into the log.
Ask what each test would catch
A test that asserts a helper was called may say little about the contract. For an invoice notification, useful checks might concern the recipient, payload, send count, and absence of a send after failed authorization.
Interaction assertions are valuable when the interaction itself is part of the contract. Pair them with outcome checks where practical. Avoid tying the test only to private helper names that could change without affecting behavior.
For an actual reproducible defect, require a check that fails for the relevant reason before the fix and passes afterward. For an exercise that only rewrites a weak test, require a discriminating check against a plausible regression; do not invent an original product failure that was never established.
Match the evidence to the risk. A visual-only spacing change may need a screenshot of the affected state. A payment-state repair can require duplicate-delivery, replay, and database-state checks. The same “unit tests passed” note does not answer both questions.
Third pass: review the cost of maintaining it
Once behavior and material risks are understood, assess design, naming, dependencies, and cleanup. Ask whether a new abstraction removes duplication that exists today. For a new library, compare the existing dependencies and platform APIs, then consider maintenance, license, security posture, and relevant package or bundle impact.
Keep feedback specific: identify the location, failing condition, consequence, and required proof. “Preserve explicit false overrides; the organization default currently wins when it should not” gives the author a concrete repair target.
Finish with a verdict and its limits. Record blocking findings, evidence inspected, checks still missing, and the next owner. Approve only the change whose behavior and remaining risk you can explain.
The Agentic Developer Interview provides practice cards for three-pass reviews, fallback regressions, security boundaries, and test quality.
The Kindle edition is free from 23 to 25 October 2026 (US Pacific time).