Every pull request was reviewed. The branch they made was not.

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.

This is a sequel of learnings. 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.

What happened

Hearso is a small real-time multiplayer trivia platform. We ship in batches. Each piece of work is a pull request into a parent branch, and the parent merges into develop once.

The beta.9 batch was 23 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. In #490, a static scan accepted a guard whose result was thrown away. In #491, a join rule was decided when an answer arrived instead of when the retry was sent, so a player who already held a seat could be shown a refusal. Per-PR review works for what is inside a diff.

At 01:44Z on 2026-09-27 I opened the parent pull request, #492, into develop. Eleven seconds later the review was skipped: "This PR contains 563 files, which is 413 over the limit of 150." That is the file limit on our plan. The same message says a higher plan raises the limit and that usage-priced reviews read at most 300 files.

Figure 1. Files changed per pull request in the beta.9 batch, 2026-09-25 to 2026-09-27 (UTC). Leaf counts are GitHub's. The parent's 563 is the reviewer's own count, after its path filters leave out a lockfile and one SVG (GitHub counts 565).

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

What a review of one diff cannot see

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 the 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 ran the whole-branch review

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 23 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.

The three bugs

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.

The review filed this one under #487 and #488. #487 changed how the form decides what counts as changed; #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. #483's game_assets blanks it when that account is deleted. #476's question_batches stored it with no deletion step. Its migration also made erasing it impossible: a CHECK required every upload batch to have an uploader, and the server role could only read the table. #475's ops_audit keeps it.

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.

Figure 2. The three should-fix findings of the whole-branch review, 2026-09-27 (UTC). Each row pairs two parts that were correct on their own; the green line is the fix in #493.

Seven nits

• 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.

Several of these are compositions too. The Sunday crown came from one pull request, and the read markers written for the Friday crown came from earlier work.

The release notes had the same shape

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. #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 that were 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

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 more 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. 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.

Figure 3. What each read found, in time order, 2026-09-25 to 2026-09-27 (UTC). The release-notes commit is dated 01:56Z; the whole-branch findings were recorded by 02:30Z.

Key takeaways

• 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.
• Split reviewers by area, not by pull request, and ask one narrow question: where do the pull requests meet?
• Turn every finding into a test that fails with its fix reverted.
• Send the fixes back through a pull request small enough to review. That caught two more.
• Read the release notes as one document, and check every "fixed" bullet against main.

What we would 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.
• Treat "#487 and #488" as a hypothesis. A cross-PR label is a claim about history. Check the base before writing it down.
• Update a sibling's fragment in the pull request that changes it, so the notes stay one document all the way through.

For any team using parent or stacked branches, the short version is this. A per-PR review answers "is this diff right?". Only a read of the whole branch answers "do these diffs agree?". 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 and the smallest #474 with 7.
• Each reviewed on its own head: the body of PR #492.
• The skipped review: the automated reviewer's comment on #492 at 2026-09-27T01:44:51Z, eleven seconds after the pull request was created at 01:44:40Z. It says "563 files, which is 413 over the limit of 150", names our plan, and says usage-priced reviews support at most 300 files. GitHub counts 565; the comment lists package-lock.json and one SVG as excluded by path filters.
• Local per-PR findings: #490's body (the scan now requires the refusal to be returned); the overnight handoff of 2026-09-26, the 00:51Z entry for #491 (the 401 rule 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 fails with the fix reverted, both migrations on a throwaway Postgres 17 and applied twice).
• 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 on the release branch: 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 overnight handoff's 03:44Z entry (migrations 20260927010100 and 20260927010200 applied to DEV, then production, and verified on both).

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