Why most code reviews miss the bugs that matter

Most code reviews are a skim. The reviewer scrolls the diff, leaves a few comments about naming and a missing test, approves, and moves on. That catches typos and style drift — but the bugs that reach production are rarely typos. They're the inverted condition on an error path, the null that only appears on a cold cache, the validation that quietly narrowed, the guard that got deleted and never re-established.

Catching those takes a different posture. Not "does this look fine?" but "what input, state, or timing makes this line wrong?" This post lays out a structured way to review that consistently surfaces real defects, organized as a set of independent angles you run over the same diff.

Start with the diff, not the file

Before reading a single line, get the exact change under review:

git diff main...HEAD        # everything on this branch
git diff HEAD               # uncommitted working-tree changes

Reviews often run before the commit, so include the working tree. If new files are untracked, make sure they're in scope too — a reviewer who only reads the tracked diff misses half the change.

One subtlety: a bug can live on an unchanged line inside a touched function. If the diff modifies a function that already dereferenced a possibly-null value, the change re-exposes that bug — it's fair game. Always read the enclosing function, not just the highlighted hunk.

The eight angles

The core idea: instead of one fuzzy pass, run several focused passes, each with a single question. Independent angles catch different defects, and the overlap is where you gain confidence.

Angle 1 — Line by line

Read every hunk and ask, for each line: what makes this wrong? Keep a concrete checklist in your head:

• Inverted or wrong conditions (>= where > was meant)
• Off-by-one on a boundary the code doesn't exclude
• null/undefined dereference on a reachable path
• A missing await that makes a promise leak through
• Falsy-zero bugs — treating 0 or "" as "not provided"
• Copy-paste with the wrong variable left behind
• Errors swallowed in an empty catch

A classic falsy-zero example:

function applyDiscount(price, discount) {
  // BUG: discount of 0 is valid, but `||` treats it as missing
  const d = discount || 10;
  return price - d;
}

Here discount = 0 silently becomes 10. The fix is discount ?? 10, but you only catch it if you ask "what if this value is falsy-but-valid?"

Angle 2 — Removed-behavior auditor

For every line the diff deletes, name the invariant it enforced, then find where the new code re-establishes it. If you can't find it, that's your finding.

// before
function listItems(params) {
  if (!params.userId) throw new Error("userId required");
  return db.items.where({ userId: params.userId });
}

// after — the guard is gone
function listItems(params) {
  return db.items.where({ userId: params.userId });
}

The deleted guard meant "never query without a user scope." If nothing replaces it, a missing userId now returns every user's items. Deletions are where the most dangerous regressions hide, because nothing in the new code looks wrong — the wrongness is an absence.

Angle 3 — Cross-file tracer

For each function the change touches, find its callers and ask whether the change breaks them: a new precondition, a changed return shape, a new exception, a new ordering dependency.

If fetchOrder() used to return null for a missing order and now throws, every caller doing const o = fetchOrder(id); if (!o) ... is silently broken — the if never runs, the throw propagates. Grep for the symbol, read the call sites, confirm each still holds.

Angle 4 — Reuse

Does the new code reimplement something that already exists? A hand-rolled date formatter next to a shared formatDate util, a bespoke retry loop where the HTTP client already retries. Name the existing helper. Duplication isn't just ugly — two copies drift, and a fix lands in one.

Angle 5 — Simplification

Flag complexity the diff adds: state that's derivable from other state, deeply nested conditionals, dead code left behind after a refactor. Redundant state is a frequent offender:

// `count` is derivable — it can desync from `items`
const [items, setItems] = useState([]);
const [count, setCount] = useState(0);

// simpler: derive it
const count = items.length;

Two sources of truth for one fact is a bug waiting to happen.

Angle 6 — Efficiency

Look for wasted work the change introduces: repeated I/O in a loop, independent async calls run sequentially instead of concurrently, expensive work added to a hot path or to startup.

// sequential — each await blocks the next
const user = await getUser(id);
const orders = await getOrders(id);   // doesn't depend on `user`

// concurrent
const [user, orders] = await Promise.all([getUser(id), getOrders(id)]);

Also watch for long-lived objects that close over a large scope and keep it alive — a subtle memory leak.

Angle 7 — Altitude

Is the fix at the right depth, or is it a bandaid? A special case bolted onto shared infrastructure is a smell. If a list component breaks for one caller and the fix adds if (caller === 'X'), the real fix is usually to generalize the shared mechanism so no caller needs the special case. Special-casing accumulates; generalization pays down debt.

Angle 8 — Conventions

Check the change against the project's stated rules — the contributing guide, the lint config, any in-repo conventions doc. Only flag a violation you can quote: the exact rule and the exact line that breaks it. "This feels off" is not a review comment; "the style guide says error messages must not be hardcoded, and line 42 hardcodes English" is.

Verify before you report

Finding a candidate is half the job. The other half is not crying wolf. For each candidate, try to refute it from the code before you write it up:

• Refuted — the claim is factually wrong (quote the line), provably impossible (a type or constant rules it out), or already handled by a guard you missed.
• Plausible — the failing state is realistic: a concurrency race, a null on an error path, a falsy-zero, an off-by-one on an unchecked boundary. Keep these even if they're not certain.

Be biased toward keeping plausible bugs. A reviewer who only reports what they're 100% certain about misses the rare-but-real cases — and those are exactly the ones that reach production, because they're hard to reason about. The cost of a false positive is a minute of discussion; the cost of a false negative is an incident.

A lightweight checklist

You don't need a heavyweight process to apply this. Even solo, run these passes in order:

1. Get the full diff, including the working tree and new files.
2. Read each touched function in full, not just the hunk.
3. Pass once for correctness (lines, deletions, callers).
4. Pass once for cleanup (reuse, simplification, efficiency).
5. Pass once for altitude and conventions.
6. For each candidate, try to refute it; keep what survives.
7. Rank by severity — correctness always outranks cleanup.

The mindset that ties it together

Good review is adversarial, not ceremonial. You're not confirming the author's belief that the code works; you're actively constructing the input that breaks it. The angles are scaffolding for that mindset — each one is a different way to ask "what did you not think about?"

Approving a change should mean "I tried to break this and couldn't," not "I read it and nothing jumped out." The difference between those two sentences is most of the bugs that ship.