Choosing the right altitude to fix a bug found in review, not just the fastest patch

In code review, caught a null-safety bug that would have crashed three production components, and fixed the defect class — not the three instances — by proposing a shared data-layer helper to the PR's author instead of patching each render site myself; one of 100+ reviews I did that semester.

Context

Most of my case studies are about work I owned end to end; this one is evidence of a different, equally important capability — raising the quality bar on someone else's work without taking it over. Code review is where a lot of real engineering judgment is invisible: catching a defect that isn't obviously wrong, deciding to fix the general case instead of the instance, and choosing to suggest rather than rewrite so the author keeps ownership and the lesson. This is part of a sustained practice — over 100 reviews across front end, back end, and tests in a single half, alongside pair programming on critical work — not a one-off catch.

Problem

A teammate's pull request rendered a list of items after filtering it, but a nearby piece of logic in the same component still referred to the length of the original, unfiltered array. Both lines were locally correct — neither was wrong on its own — but they encoded an assumption that the two values would always agree. They wouldn't: the moment the filter actually removed an item, the two derived values would diverge, and three components shared this exact pattern.

Constraints

—The bug was invisible line by line. Nothing in the diff was individually wrong; the defect only exists in the *relationship* between two values derived from the same source — exactly the kind of bug a fast, diff-focused review misses.

—The fast fix was tempting and insufficient. Patching the three known call sites would have made the visible symptom disappear without removing the underlying defect class — the next component built the same way would reintroduce it.

—It wasn't my code. Proposing a deeper structural change to someone else's pull request risks either being ignored (too soft) or taking over their work (too heavy) — getting the balance right is itself the skill.

Decision

I recognized the shape of the bug before deciding what to do about it: two values derived from the same source, read independently, with no guarantee they'd stay in sync — the same kind of problem I've solved architecturally elsewhere by giving a derived value a single computation path.

—Traced the pattern to all three affected components, not just the one in the diff, so the fix could address the actual defect class.

—Proposed a shared helper at the data layer — one place that derives the value both pieces of logic need, so nothing downstream can read two different answers from the same source again.

—Suggested it to the author instead of implementing it myself. Leaving the change in their hands cost an extra review round, but kept their ownership of the PR intact and taught the pattern rather than silently overriding their work.

Trade-offs

—A shared data-layer helper over patching the three render sites. Fixing all three instances is faster; giving them one shared source removes the defect class for any future consumer too — worth the extra design step for a pattern that had already repeated three times.

—Suggesting the fix over implementing it myself. Writing the fix directly would have been quicker and guaranteed my preferred outcome, but it would have taken the PR away from its author. Proposing it and letting them apply it cost a review cycle and produced a teammate who understood the pattern, not just a merged fix.

—Reviewing the invariant over reviewing the diff. Reasoning about what the two derived values were supposed to guarantee together — rather than checking each line in isolation — is slower per review but is the only way this class of bug gets caught before production.

Impact

—Prevented a crash in three production components before it shipped.

—Removed the underlying defect class, not just the three known instances, via a shared data-layer helper any future consumer now goes through.

—Kept the fix owned by its original author, reinforcing the pattern for them rather than substituting my judgment for theirs.

—Part of a review practice recognized in performance feedback for the clarity of its questions and documentation — over 100 reviews in a single half-year, across front end, back end, and tests.

Lessons Learned

Reusable engineering knowledge I carry forward from this:

—A value with two independent readers is a bug waiting for the day they diverge. The same principle that governs cross-service data consistency applies just as much inside a single component.

—In review, fix the class when you can see it, not just the instance in front of you. Three repeats of the same pattern is a signal that patching the visible one will leave the other two to fail later.

—Suggest, don't take over. A review that fixes the pattern and hands the implementation back teaches it; a review that silently rewrites the PR doesn't.

—Review what the code assumes, not just what it changed. The bug was only visible when reasoning about the invariant the two lines were supposed to share.

Evidence

—Caught and redirected a null-safety divergence bug across three components before release, via a proposed shared abstraction rather than a direct rewrite.

—Part of a sustained review practice: 100+ code reviews in a single half (up from 18 the cycle before), across front end, back end, and tests, plus pair programming on critical deliveries.

—Recognized in performance feedback for the clarity of review questions and PR documentation.

—Source (private): consolidated career knowledge base and performance-review evidence.