23 pull requests passed review. The branch they made had three bugs

Originally published on Hearso Writing on 2026-09-27. Hearso is the team trivia game I build and run; these posts are the engineering notes from building it.

Each of 23 pull requests passed review alone. Their 563-file parent was too big for the reviewer, so we read it whole and found 3 bugs between them.

Twenty-three pull requests passed review one at a time. The branch they built together still had three bugs, and none of those reviews was placed to see them.

We ship Hearso, a small real-time multiplayer trivia platform, in batches. Each piece of work is a pull request into a parent branch, each one is reviewed on its own, and the parent merges into develop once. The beta.9 batch was 23 of them.

I assumed the last step, a review of the parent, would be a formality. Every line in it had already been read. (It turned out to be the only review that could see the bugs, and it almost didn't happen.)

So the question here is simple: if every pull request in a branch passes review, is the branch reviewed?

The parent was 563 files, and our automated reviewer stops at 150. So on 2026-09-27 we read the whole branch locally instead: three reviewers, split by area, each asking only where one pull request's code met another's.

Short answer: no, the branch was not reviewed. The whole-branch read found 3 should-fix bugs and 7 nits that 23 green reviews had passed. Reading the 26 release-note fragments as one document found 8 claims overtaken inside the batch. And the review of our fixes found 2 more bugs, in the fixes.

It follows What we got wrong about checks that pass when nothing happened and Line coverage said done. Mutation testing disagreed.. Those were about checks that pass without checking. This one is about reviews that pass one piece at a time.

Per-PR review did its job, for what was inside each diff

The beta.9 batch was pull requests #468 to #491, merged between 2026-09-25 15:55Z and 2026-09-27 01:36Z (all times UTC). It carried a team feed, synthesized game music, a public results page, questions in the database, and two security sweeps.

Every one of the 23 was reviewed by our automated reviewer on its own head, and every finding was resolved before it merged. Many also had a local review before they were opened, and those found real bugs.

Here are two real examples:

• #490: a static scan accepted a guard whose result was thrown away. It passed any handler where the guard's name appeared before the first await, even refuseCrossSite(req); with its answer dropped. The scan now requires the refusal to be returned.
• #491: a join's "you are not in this room" rule (a 401) was decided when an answer arrived, not when the retry was sent. So a player who already held a seat could be shown a refusal. It is now decided at send time.

So per-PR review works. It is just answering a narrower question than it looks.

The parent was 563 files, and the reviewer stopped at 150

At 01:44Z on 2026-09-27 I opened the parent pull request, #492, into develop. The automated review skipped it on a label rule, so I asked for one by hand at 01:45:07Z. Eight seconds later it declined:

Review skipped: 563 files exceed the limit of 150.

That is the file limit on our plan.

Figure 1. Files changed per pull request in the beta.9 batch, n = 23 leaves plus the parent, 2026-09-25 to 2026-09-27 (UTC). Leaf counts are GitHub's; the parent's 563 is the reviewer's own count in its refusal.

Figure 1 is the whole problem in one picture. The largest leaf was 96 files (#483), the median 23 and the smallest 7 (#474), so every leaf fit. The parent did not, and it was the first time any reviewer had been asked to read those 23 changes as one.

A review of one diff cannot see its sibling

A per-PR review reads one diff against its base. It can tell you whether that diff is right. It cannot tell you whether the diff is still right next to a sibling, because the sibling is not what it was asked to read.

A parent branch makes this easy to miss. When #491 was reviewed, the music from #473 was already in its base. But it was not in #491's diff, so it was not what the review looked at. Nothing in our process ever put the two in front of one reader.

So the question for the whole branch was narrow on purpose. Not "is this code right?", which 23 reviews had asked. Instead: where does one pull request's code meet another's, and is it still right there?

How we read the whole branch

We chose a local review of the whole branch. The other option was to split the parent into slices under the limit, and each slice would still be only part of the branch.

• Three reviewers, split by area, each with one brief: security and the database; teams, boards and the inbox; rooms, the game and the public pages. By area rather than by pull request, so each reader held every pull request that touched the same screens.
• One question. Each brief asked only for what the per-PR reviews could not see: how the pull requests compose.
• A test per finding. Each behavioural fix has a test that fails with its fix reverted, one edge case per test.
• Controls for the migrations. Both new migrations ran on a throwaway Postgres 17. We checked the refused state before, the allowed state after, a case that must still be refused, and a second apply of each to prove it idempotent.

It found 3 should-fix issues and 7 nits. Nothing was a blocker, and the security area came back broadly clean.

Three bugs made of two correct parts

Figure 2. The three should-fix findings of the whole-branch review of the beta.9 parent, 2026-09-27 (UTC). Each row pairs two pull requests that were correct on their own; the last box is the fix in #493.

A settings form that kept the last team

Settings · Team seeds each field once from the page's read of the team. Switching team in the navigation reaches that page through router.refresh(), which re-renders the server part and keeps client components and their state.

After a switch from team A to team B, B's page showed A's name and rhythm and marked every field unsaved. A Save would have sent team A's schedule to team B.

The fix is one React key:

export function TeamSettingsForm(props: TeamSettingsFormProps) {
  return <TeamSettingsFormBody key={props.teamId} {...props} />;
}

A new key makes React mount a fresh form when the team changes. The key lives inside the exported component rather than at its one caller, so a later caller cannot forget it. The team feed already worked this way.

I got the history wrong at first. The review filed this bug under #487 and #488: #487 changed what counts as a changed field, and #488 moved the team switch onto a shared store. While writing this post I checked the base, and both ingredients were already on develop: fields seeded once, and a switch that refreshes in place. So this bug may be older than the two pull requests it was filed under. I have not checked whether it could be reached on main.

Music under "This room does not exist"

#473 added game music, which plays while the room's last known phase is a question or a reveal. The room page's dead ends, "This room does not exist" and "This room is open in another tab", are drawn by terminal(). It sets a fatal notice and leaves the last state where it was. #491 added one more way onto one of those screens.

Together, a room that vanished mid-question still read as "question" to the music. The loop played under the notice and started again whenever the tab came back into view. The fix adds one condition:

useGameMusic(
  !fatal && (state?.phase === "question" || state?.phase === "reveal"),
);

Clearing the state inside terminal() would also have worked. It would also have changed what every other effect sees at the moment a room dies, which is a wider change than the music needed.

An operator's id, handled three ways

Three pull requests store the id of the operator who used the operations console:

Each pull request was consistent with itself. Only the branch as a whole showed one kind of data treated as personal in one table and not in the next.

The fix adds a deletion step and a migration that redraws the CHECK and grants the update. The step reads before it writes. Code deploys before migrations here, and an UPDATE the role may not make is refused even when it matches no row, so a bare update would have failed every account deletion until the migration ran. ops_audit keeps the id on purpose, for audit integrity, and that is now written down. Its retention period is a follow-up.

Seven nits, and several were compositions too

• The privacy page did not mention the new public results page.
• Session replays recorded invite codes inside /join/ links.
• One new database function was the only one in the batch not revoked from the browser roles.
• The audio switches were announced "Mute music, pressed" while music played.
• A bare /r showed an unframed 404.
• Moving the weekly crown from Friday to Sunday would have re-announced weeks that players had already read.
• Good Company still went to an inviter who had left the team.

The Sunday crown is a good example of the pattern: the move came from one pull request, and the read markers written for the Friday crown came from earlier work.

The release notes had the same bug

Every pull request carries a changelog fragment, written with that pull request. For beta.9 there were 26. Read one at a time, each was true when it was written.

Read as one document, eight had been overtaken by a later pull request in the same batch. Three real examples:

• #489's fragment said the invite routes now used the same-origin helper, and #490 replaced that helper with one shared cross-site guard.
• #483 and #486 described a reaction's second tap two different ways.
• #484 and #485 described the weekend two different ways.

Then every "fixed" bullet was checked against main. Fourteen described faults introduced and repaired inside the batch, so nobody using a released version ever met them; the public notes claim none of them as fixes. Three more bullets overstated what main had been doing and were narrowed.

The fragments are per pull request; the release is their sum. It is the same problem as the code.

The fixes went back through review, and it found two more

A fix is new code, and new code can meet old code badly too. So all ten fixes went into one pull request into the parent, #493: 30 files, small enough for the vendor review. It read #493 at 03:36Z and found two issues, both valid:

• A migration guard with a gap. Two COMMENT ON COLUMN statements sat after the block that skips an absent table. On a database without that table, the migration would have failed on the comments, which is exactly what its guard was written to prevent.
• A browser test that could not fail for its reason. It checked a bare /r for its status and its words, and could pass with the results frame gone.

The second is the shape from the null-case post: a check that reads green without looking at the thing it exists for. We wrote it. The fix asserts the frame first, and it has a positive control: with the page file removed, the classic build fails on that assertion. Both threads were resolved at 03:42Z.

#493 merged into the parent at 03:44Z, as parent commit 3a4a72c8. Its two migrations were then applied to DEV and to production, in that order, and verified on both.

What the numbers say

Side by side, each read that ran after the per-PR reviews found something they were not placed to see.

Figure 3. Findings from reading the beta.9 batch as one change, 2026-09-27 (UTC): the local whole-branch review (3 + 7), the release-notes read of 26 fragments (8 + 14 + 3) and the vendor review of fix PR #493 (2).

Predictions vs outcome

Key takeaways

1. Review the whole branch as one change, as well as each pull request. The file limit forced it this time, but the bugs did not depend on the limit.
2. Split whole-branch reviewers by area, not by pull request, and ask one narrow question: where do the pull requests meet?
3. Turn every finding into a test that fails with its fix reverted, so the fix is proven, not just written.
4. Send the fixes back through a pull request small enough to review. Ours found two more bugs.
5. Read the release notes as one document, and check every "fixed" bullet against main, because a fragment is only true of its own pull request.
6. Treat a label like "#487 × #488" as a hypothesis. A cross-PR label is a claim about history; check the base before writing it down.

What I'd do next

• Plan the parent-level review from the start. We knew the batch was 23 pull requests before we opened the parent. We learned it was 563 files from the refusal. Counting the parent against the reviewer's limit as each leaf merges would have told us before the last one did.
• Read for composition while the batch grows, not once at the end. A pass after every few merges keeps each read small.
• Update a sibling's fragment in the pull request that changes it, so the notes stay one document all the way through.
• Check whether the settings bug was reachable on main.

For any team using parent or stacked branches, the short version: know your reviewer's file limit, count the parent against it, and schedule the whole-branch read before the last leaf merges.

Evidence

All times UTC.

• 23 pull requests, #468 to #491, with merge times and file counts: gh pr list on the beta.9 parent, state merged. The first merged 2026-09-25T15:55:43Z and the last 2026-09-27T01:36:15Z. The largest was #483 with 96 files, the smallest #474 with 7; the median of the 23 is 23.
• Each reviewed on its own head, every finding resolved: the body of PR #492.
• The refusal: #492 was created at 2026-09-27T01:44:40Z. Its first automated comment says the review was skipped on a label rule and names our plan. A manual request at 01:45:07Z got "Review skipped: 563 files exceed the limit of 150." at 01:45:15Z; the overnight handoff's 02:30Z entry quotes the same limit.
• Local per-PR findings: #490's notes for reviewers (the scan now requires the refusal to be returned); the overnight handoff's 00:51Z entry for #491 (the 401 rule is decided when the retry is sent).
• The whole-branch review: the body of PR #493 (three areas, 3 should-fix, 7 nits, a test per fix that failed with the fix reverted, both migrations on a throwaway Postgres 17 and applied twice); the handoff's 02:30Z entry.
• The key: commit 0de49267. The music: 4342c62c. The operator id: 85e8a589 and migration 20260927010200; the revoke is 20260927010100.
• Both form ingredients on the base: at b87906f1, the base of #492, team-settings-form.tsx seeds its fields with useState(initial.…), the navigation's switchTeam calls router.refresh(), and the room page defines terminal().
• Which PR added each operator-id table: migrations 20260925010200 (ops audit, #475), 20260925010300 (question batches, #476) and 20260926010100 (game assets, #483); the game-assets erasure step is commit 1b1fe829, merged with #483.
• The release notes: commit 24d38072, committed 2026-09-27T01:57:06Z: 26 fragments assembled, eight claims corrected, fourteen in-batch fixes restated, three narrowed.
• The vendor read of #493: the automated review at 2026-09-27T03:36:53Z on 6093da59, two actionable comments; fixed in b401cddc and ce940292; both threads resolved at 03:42Z per the overnight handoff.
• #493 merged, and its migrations applied: gh pr view 493 (merged 2026-09-27T03:44:34Z, merge commit 3a4a72c8); the handoff's 03:44Z entry (migrations 20260927010100 and 20260927010200 applied to DEV, then production, and verified on both).

Corrections to the first version of this post (2026-09-28): it said the review was skipped "eleven seconds" after #492 opened, quoted the refusal as "563 files, which is 413 over the limit of 150", and said usage-priced reviews read at most 300 files. The skip at 01:44:51Z was the label rule; the file-limit reply came at 01:45:15Z after a manual request, and its current text is quoted above. The "413" wording and the 300-file claim could not be re-verified, so they are gone. So is "GitHub counts 565", which cannot be re-checked now that later merges changed #492's file count.

Nothing above contains a credential, a token or a player's identity.