The Code Review Habits That Catch Bugs

"LGTM" vs. a Review That Actually Stops Bugs

Code review looks like one activity on the surface, but there are really two kinds. One is "I can read the code and nothing feels off, so I'll approve it." The other is "I'm reading this while actively imagining how it breaks in production." The first kind still catches style nits and naming suggestions, but bugs slip through. The gap between the two isn't experience — it's a difference in how the diff is read.

Don't Read Top to Bottom — Follow the Data

A pull request diff is ordered by file and by change, not by risk. Reading it top to bottom is enough to follow the author's intent, but not enough to reason about blast radius. Reviewers who catch bugs read the same diff in a different order:

1. Find the entry point — where does the data for this change originate? A form submission, an API request, an event handler.
2. Trace every transformation — validation, mapping, persistence. At each step, ask: could this be null here? Does the type actually hold?
3. Check the exit — the response, the DB write, the rendered UI. Does what comes out still match what went in?

Reading "entry → transform → exit" lets you review the path the data travels, not just the lines that changed — even when the diff spans multiple files.

Five Spots Worth Checking Every Time

Even under time pressure, mechanically checking these five areas catches a disproportionate number of real bugs.

1. Boundary values

First/last array elements, zero/one/max item counts, empty string vs. null. It's common to "verify with 3 sample records" and never try 0 or 1,000.

2. Error handling

Is a try/catch silently swallowing failures? Is there any line describing what the user sees on failure? A diff with 3 lines of happy path and 0 lines of error path is a red flag.

3. Async ordering

What happens when two async operations race? Does setState fire after a component unmounts? Can a double-click trigger a duplicate submission?

4. Authorization

Does the API's filtering condition match the UI's visibility condition? "Hidden on screen but still returned by the API" is a classic miss.

5. Backward compatibility

Does a column type change or a response shape change break existing data or older clients? Does the deploy still assume the migration runs first?

A Real Example: Caught at the Boundary

One team added a single line to a coupon discount calculation: discountedPrice = price - coupon.amount. The happy-path tests passed with three sample prices (¥1,000, ¥3,000, ¥5,000). A reviewer checking boundary values asked what happens when coupon.amount exceeds price — and reproduced a negative price. The author had assumed a coupon would always be smaller than the price, but nothing in the code enforced that assumption. Without the habit of mechanically checking boundaries, this likely would have shipped and shown negative prices in production before anyone noticed.

Writing the Comment

Once you spot something suspicious, phrasing it as a question rather than a verdict keeps the review moving.

• ❌ "This is a bug" — forces the author to reconstruct your reasoning, adding a round trip.
• ✅ "What happens here if the array is empty — is there a test for that?" — lets the author verify and fix it themselves.

Putting This Into Practice with Bugoon

It's common to spot something suspicious during review that can't be reproduced on the spot. Bugoon captures screenshots, annotations, and recorded interaction steps directly in the browser and turns them into a bug report you can file as a GitHub Issue — so a boundary-value concern flagged during review can be reproduced in a staging environment, documented, and handed to the implementer without switching tools. Keeping review-driven findings and user-reported bugs on the same kanban board also helps avoid the "I thought we agreed on that" problem that review comments alone tend to create.