Learn to Review PRs Faster

A 59-file PR landed in my inbox this week. My first reaction was the same as yours would be: absolutely not.

Then I reviewed it in about twenty minutes. Not by reading faster — by reading less. Specifically, by reading 5 files instead of 59. Here’s how.

The PR

PR #25166: “Fix Bazel baseline: conversation gazelle errors and test_config_parser sandbox failure.” Ryan (squad lead) authored it. 59 files changed, +302/-53. Three distinct fixes bundled together:

1. The conversation package had empty # gazelle:ignore BUILD files in subdirectories — no py_library targets, so Gazelle couldn’t see the source files. When it tried to walk in, it hit circular imports and crashed. Fix: give each subdirectory a real py_library(srcs=glob(["*.py"])) with no deps, and let the parent aggregate everything.

2. test_config_parser used a relative path to .github/ci/configs/test_exec_modes.yml. Works fine when you run pytest from the repo root. Doesn’t exist in a Bazel sandbox. Fix: fall back to TEST_SRCDIR runfiles path and export the file via exports_files.

3. Bonus cleanup: ~15 stale deps removed (typing_extensions, mai_nano, python_forward_compat).

That’s three problems, three fixes, and a spring cleaning — totaling 59 files. Sounds terrifying. It’s not.

Why top-to-bottom fails

Most people review PRs the way they read code: open the diff, start at the first file, scroll down. For a 10-file PR, this works. You build context as you go, file by file, and by the end you have a mental model.

For a 59-file PR, this falls apart. You’re 20 files in, you’ve seen the same py_library stanza 12 times, your eyes glaze over, and when you finally hit the file that actually matters — the sandbox fix, the .bazelignore change — you’re too fatigued to notice the subtle thing that’s wrong.

Top-to-bottom review scales linearly with file count. The whole trick is making it scale with conceptual complexity instead.

Story first, code second

Before I opened a single diff, I read the PR description. Not skimmed — read. What was broken? Why? What’s different now?

This is the step most people skip, and it’s the highest-leverage thing you can do. A good PR description (and this one was good) gives you the narrative: Gazelle was crashing on conversation because there were no targets to resolve against. The sandbox couldn’t find a CI config file because relative paths don’t survive sandboxing.

Once you have the story, every file in the diff has a role. You’re not discovering what the PR does by reading code. You already know what it does. Now you’re verifying that the code does what the story says.

That’s a completely different cognitive task. Discovery is hard. Verification is easy.

Triage: 59 files into 3 buckets

Here’s the actual file breakdown of this PR:

17 files are literally the same file copied into different directories. Once you’ve read one, you’ve read all of them. You don’t need to diff each one — you need to verify the pattern is correct, then confirm it’s applied consistently. That’s a 30-second scan, not a 17-file review.

~15 more files are mechanical dep removal. typing_extensions got removed from a bunch of targets. The only question is: “is this dep actually unused?” If the PR passes CI (it did), the answer is yes. Skim for anything weird, move on.

That leaves about 5 files where actual thinking is required.

Build a reading order

Not all 5 files are equal, and the order you read them matters. Here’s the order I used:

1. conversation/BUILD.bazel (the parent) — This is the architectural decision. How does the parent aggregate child packages? What’s the py_library structure? Read this first because it’s the “why.”

2. One child BUILD file (any of the 17) — Verify the template. py_library(name = "subpackage_name", srcs = glob(["*.py"])). No deps. Makes sense — the parent handles cross-cutting dependencies.

3. ci/src/ci/test_config_parser.py — The sandbox fix. Does the TEST_SRCDIR fallback look right? Is the path construction correct?

4. .bazelignore — What got un-ignored? .github/ci/configs/ — needed so Bazel can export the config file as a runfile.

5. SKILL.md — Process improvement. “Verify baseline first” got hardened. Good to know for future PRs.

By file 3, I had the full mental model. The last two were confirmatory reads — I already knew what to expect.

Draw the before and after

For the conversation fix, I sketched the architecture change mentally (you could do this on paper or a whiteboard):

Before:

conversation/
├── BUILD.bazel          (parent py_library — deps declared here)
├── __init__.py
├── subdir_a/
│   ├── BUILD.bazel      (empty — just # gazelle:ignore)
│   └── *.py             (invisible to Bazel)
├── subdir_b/
│   ├── BUILD.bazel      (empty)
│   └── *.py             (invisible)

Gazelle tries to resolve imports from subdir_a. No targets exist. It looks elsewhere, finds circular paths, crashes.

After:

conversation/
├── BUILD.bazel          (parent — aggregates child targets via deps)
├── __init__.py
├── subdir_a/
│   ├── BUILD.bazel      (py_library with srcs=glob, NO deps)
│   └── *.py             (now tracked!)
├── subdir_b/
│   ├── BUILD.bazel      (py_library with srcs=glob, NO deps)
│   └── *.py             (now tracked!)

Each subdirectory owns its sources. Parent aggregates. Gazelle can resolve imports because targets exist. No circular deps because children declare no deps — all dependency edges live at the parent level.

Once I drew this, the “no deps on children” pattern was obvious. And the one potential issue — @pypi//argparse appearing in a BUILD file — jumped out immediately. argparse is stdlib since Python 2.7. You don’t need a PyPI dep for it. (I flagged it.)

The real insight

The reason this works isn’t about speed-reading or being clever. It’s about when you bring in context.

Reading code cold — without knowing what it’s trying to do — is like solving a jigsaw puzzle without the box lid. Every piece is a mystery. You’re simultaneously discovering the picture AND assembling it.

Reading code after building a mental model is like solving the puzzle with the box lid. You know the picture. You’re just checking that the pieces fit. When a piece doesn’t fit — wrong dep, incorrect path, missing edge case — it’s visually obvious. It looks wrong because you know what right looks like.

That’s the shift. The review itself isn’t faster. The preparation makes the review trivially easy.

A checklist you can steal

For your next large PR:

1. Read the description. Not skim. Read. What broke? What’s different? Why?
2. Look at the file list before opening any diffs. Categorize: template/mechanical/meaningful.
3. Count the meaningful files. If a 59-file PR has 5 meaningful files, you have a 5-file review.
4. Pick a reading order. Start with the file that explains the architecture. Read children/copies only enough to verify the pattern.
5. Sketch the before/after. Doesn’t have to be fancy. Even “before: broken because X, after: works because Y” in your head is enough.
6. Now open the diffs. You’ll be surprised how fast it goes when you already know what you’re looking for.

The 59-file PR that looked like an hour of work took twenty minutes. Not because I’m fast. Because 54 of those files didn’t need more than a glance.

The meta-lesson

Large PRs aren’t inherently harder to review. They’re harder to review if you treat every file as equally important. The moment you triage — the moment you realize that 17 files are the same template and 15 more are mechanical cleanup — the PR collapses from “overwhelming” to “manageable.”

The best code reviewers I’ve worked with don’t read more carefully. They build better mental models before they start reading. The code review is the last step, not the first.

Build the map. Then navigate.