Code Review Mastery

Stage 3 · Testing, Quality & Craft · B.U.I.L.D. letter: D

You wrote the code. You tested it. It works on your machine. So why does it need a second pair of eyes? Because the most dangerous bugs are the ones that look obviously correct to the person who wrote them.

---

⚠️ The vibe trap

You asked your AI to build a feature, it delivered something clean-looking, the tests passed, and you merged. Three weeks later a colleague (or a user, or a security researcher) finds the flaw you both missed. Vibe coding is powerful — but shipping AI-generated code without reviewing it is like signing a contract you didn't read. You're the professional in this loop. The AI is your very fast junior developer, and it needs the same review every junior developer needs. Skipping review — of your own code or your AI's — isn't moving fast, it's accumulating debt you'll pay later with interest.

---

🤔 Why Code Review Exists

Review does three things that tests alone cannot:

1. Catches bugs at zero cost — before they reach users, before they compound into architecture problems.
2. Spreads knowledge — the reviewer learns the domain; the author learns new patterns; the team converges on shared standards.
3. Raises the bar — knowing someone will read your code changes how you write it, every time.

These benefits apply equally whether the code was written by you, by a teammate, or generated by an AI assistant. The diff doesn't know who produced it.

Without review:                   With review:
  Author → Merge → Prod             Author → PR → Reviewer → Merge → Prod
  Bugs found: in prod               Bugs found: in the PR (free to fix)
  Knowledge: siloed                 Knowledge: shared
  Standards: drift                  Standards: enforced gently, consistently

Mental model: Review is not a quality gate that blocks progress. It is the cheapest debugging session you will ever run, staffed by someone who hasn't lost the ability to see your code clearly.

Common mistake: Treating review as bureaucracy to get through as fast as possible. A two-minute rubber-stamp review catches roughly zero bugs.

---

💬 Giving Review Kindly and Usefully

The goal of a review comment is to make the code better, not to demonstrate that you spotted something. Every comment should be actionable, specific, and respectful — because you are commenting on the code, never on the person.

Good vs. bad review comments — real examples:

# --- BLOCKING ISSUE ---

BAD:  "This is wrong."
GOOD: "This function returns undefined when `items` is empty — the
       caller on line 47 dereferences the result without a null check,
       which will throw at runtime. Suggest: return an empty array []
       as the fallback, or add a guard in the caller."

# --- SUGGESTION (non-blocking) ---

BAD:  "Why did you write it like this?"
GOOD: "nit: `getUserById(id)` could be renamed `findUser(id)` to match
       the naming convention we use for nullable lookups elsewhere in
       this module. Not a blocker — happy to discuss."

# --- PRAISE (yes, praise) ---

GOOD: "Nice — extracting `formatCurrency` into its own util makes
       this so much easier to follow. "

Distinguish comment types explicitly. Reviewers who don't label their comments force authors to guess what is blocking and what is optional. Use prefixes:

• blocking: — must fix before merge
• suggestion: or nit: — would be better, author's call
• question: — you want to understand something before approving
• praise: — good pattern worth naming

Common mistake: Writing every comment at the same urgency level, turning ten nits into ten reasons the PR feels overwhelming.

---

📥 Receiving Review Without Ego

Your code is not you. This is the hardest thing to internalize, and the most important.

When a reviewer flags something, the useful responses are: fix it, explain why you disagree, or ask a clarifying question. The useless response is defensiveness.

# Reviewer leaves this comment:
# "suggestion: This loop rebuilds the filtered list on every render.
#  Could memoize with useMemo to avoid unnecessary work."

BAD author response (internal or external):
  "It works fine. Premature optimization."

BETTER author response:
  "Good catch — added useMemo. I left a comment explaining the
   dependency array so future readers understand why it's scoped
   this way."

ALSO FINE author response:
  "I benchmarked this — the list never exceeds ~20 items and the
   parent re-renders only on user interaction. I'd rather keep it
   simple for now. Can we revisit if profiling shows it matters?"

Mental model: A review comment is a gift — someone read your code carefully enough to have a thought about it. You don't have to accept every gift, but you should acknowledge it.

Common mistake: Responding to every comment with "done" even when you disagreed and didn't change anything. That's not resolution — that's confusion waiting to happen.

---

🔍 What to Actually Look For

When you are the reviewer, work through this checklist mentally before you comment:

CODE REVIEW RUBRIC — What to examine

CORRECTNESS
  [ ] Does the logic handle the happy path?
  [ ] Does it handle empty input, null, undefined, zero, negative numbers?
  [ ] Are error paths handled (try/catch, Promise rejections, HTTP errors)?
  [ ] Does it do what the PR description claims?

SECURITY
  [ ] Is user input sanitized before use in queries, HTML, or shell commands?
  [ ] Are secrets hardcoded anywhere in the diff?
  [ ] Are permissions/auth checks in place for new endpoints?

TESTS
  [ ] Are new behaviors covered by tests?
  [ ] Are edge cases tested, not just the happy path?
  [ ] Do tests actually fail when the code is broken?

NAMING & READABILITY
  [ ] Do names say what the thing IS, not how it was made?
  [ ] Is a comment explaining WHY, not restating what the code does?
  [ ] Could you understand this function without reading the rest of the file?

EDGE CASES
  [ ] What happens at scale (large data sets, many users, concurrent requests)?
  [ ] What happens if a network call or database write fails mid-operation?

Common mistake: Only reviewing for style. Style matters, but a beautifully formatted function that deletes the wrong row is still a disaster.

---

📦 Small PRs Review Better

A 30-line PR can be reviewed in five minutes and approved with confidence. A 1,200-line PR gets a rubber stamp because nobody has three hours to trace every path.

# BAD: one PR that does everything
+ feat: add user profile, redesign settings page, fix password reset,
+       update email templates, add admin role, migrate old avatars

# GOOD: five focused PRs reviewed and merged in sequence
+ feat: add user profile model and DB migration
+ feat: add profile read/write API endpoints
+ feat: add profile UI component
+ fix: password reset token expiry was not being checked
+ feat: add admin role with scoped permissions

The small-PR habit also applies to AI-generated code. If you prompt your AI to "build the whole auth system," review each piece as a separate logical unit even if you merge them together. Reviewing a diff in chunks is reviewing at all. Reviewing it as a 2,000-line wall is not.

Mental model: A PR is a unit of reasoning, not a unit of time. You should be able to hold the entire change in your head at once.

Common mistake: Treating PR size as a reflection of effort. Smaller is harder to write and easier to review — that's a compliment to the author, not a criticism.

---

🛠️ Your Mission

Pull up a recent change — something you shipped in the last week, whether you wrote it by hand or with AI help. Open the diff (GitHub, GitLab, or git diff main) and go through it using the Code Review Rubric above. Leave real comments in the style you practiced here — labeled blocking:, suggestion:, question:, or praise:. At least one comment should be a genuine question you cannot immediately answer. At least one should be praise.

If you have a teammate, teammate AI, or even your past self, trade reviews. Give the feedback you would want to receive.

---

✅ You're Done When…

• You can recite the five categories of the Code Review Rubric (Correctness, Security, Tests, Naming, Edge Cases) without looking
• You've written at least four labeled review comments (blocking / suggestion / question / praise) on a real diff
• You can explain the difference between a blocking issue and a nit, and why the distinction matters
• You've received at least one piece of review feedback and responded without defensiveness — fix, explain, or ask
• You've broken a recent large change (or a planned one) into at least two smaller, reviewable units

---

➡️ Next: Documentation That Survives. Build It Right, Or Don't Build It At All. 🏛️