fix(review): a PR too big for GitHub to diff is reviewed anyway - #46
Conversation
GitHub refuses to render a diff past 300 changed files — `gh pr diff` comes back HTTP 406 — so a large PR failed the review before the agent ever started, with a raw API error where the draft should be. The diff is now re-assembled from the `pulls/N/files` API when that happens: the per-file hunks with their `diff --git`/`---`/`+++` headers restored, so it parses exactly like `gh pr diff` output and the line anchoring reads it the same way. A file whose patch GitHub withholds (binary, or too large) keeps its headers and carries a line saying so, and a change past the files API's own 3000-file cap is stated too — an unexplained gap would read as "nothing changed here". Only a 406 falls back; every other gh failure still fails the run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a robust fallback for fetching PR diffs when GitHub refuses to render them (notably when a PR changes >300 files), so Cerber can still produce a review instead of failing early.
Changes:
- Add
fetchPrDifffallback: on the “diff too large” failure, fetch per-file patches viapulls/{n}/filesand reassemble a unified diff. - Introduce
assembleDiff(files)to reconstructdiff --git/---/+++headers and add explicit notes when patches are withheld or truncated. - Add test coverage for diff reconstruction and the fallback behavior; update SPEC to describe the 406-only fallback normatively.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/core/gh.ts | Implements the 406 fallback path, fetchPrFiles, and assembleDiff unified diff reconstruction. |
| src/core/gh.test.ts | Adds tests validating reconstruction, parsing/anchoring behavior, and fallback conditions. |
| SPEC.md | Updates the run sequence specification to require the 406-only fallback and to describe truncation/withheld-patch notes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const message = err instanceof Error ? err.message : String(err); | ||
| if (!/too_large|exceeded the maximum number of files/i.test(message)) throw err; | ||
| const files = await fetchPrFiles(ref); |
There was a problem hiding this comment.
You are right that code and spec disagreed, and I fixed that on the spec side rather than the code side — reasoning, so you can push back if you disagree.
The condition the fallback actually means is "GitHub declined to render this diff because it is too big". GitHub signals that with the too_large error code; 406 is Not Acceptable, a general status it can return for unrelated reasons. Matching too_large matches the meaning, matching 406 matches the transport — so requiring the status would be the one that eventually falls back on something this was never for.
The theoretical hole you name — some other gh pr diff failure whose message happens to contain too_large — needs GitHub to emit that token for a different refusal, which it does not today. And the blast radius if it ever did is small: the fallback immediately calls the files API, which fails the same way for a genuine auth or not-found error, so the run still fails rather than reviewing a fabricated diff. There is a test pinning that an auth failure throws instead of falling back.
So the code is unchanged in behaviour, but it now says why in a comment, and SPEC.md §11.2 no longer claims "a 406 is the only diff failure that falls back" — it says the too_large signature is, and that the match is deliberately not on the status. ea6457e.
There was a problem hiding this comment.
For the record, since this merged with the thread still open: the guard is unchanged, matching GitHub's too_large signature rather than the 406 status, for the reason above — 406 is Not Acceptable generally, too_large is emitted for this refusal and nothing else.
Left unresolved deliberately rather than closed off, because it records a real design choice someone may want to revisit, not a loose end. If the status check is wanted after all it is a one-line change on top of c50b803.
A pure rename changes no lines and carries no patch — the same shape a binary file arrives in from the files API. Counting lines alone reported the move as "Binary files … differ", which is wrong, and the more alarming of the two ways to be wrong. It now ends at its `rename from`/`rename to` lines with no hunks, which is exactly what git emits for a file that only moved. The binary line also names the side an added or removed file does not have `/dev/null`, as git does, instead of repeating the path on both sides. Spec and code no longer disagree about the guard either: the fallback matches GitHub's `too_large` signature, not the bare 406 status, because 406 is "Not Acceptable" generally while `too_large` is emitted for this refusal and nothing else. SPEC.md now says that rather than "a 406". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The files API returns at most 3000 files, so a diff assembled from it carried a note saying anything beyond that was missing. But a PR can change exactly 3000 files and lose nothing, and there is nothing on hand to tell that apart from a PR that was cut short. The note now reports the doubt — what the API returned, and what would follow if the PR is bigger — instead of asserting a truncation that may not have happened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 0.27.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
The problem
Open a large PR in the cockpit and there is no review — just a raw GitHub error
where the draft should be:
GitHub refuses to render a diff past 300 changed files.
gh pr diffis the veryfirst thing a run does, so the review died before the agent started: no summary,
no chapters, no verdict, and a status of
failedcarrying an API message thereader has to decode themselves. Re-review does the same thing again. The PR is
simply un-reviewable by cerber, and it is exactly the kind of PR where a review
would have earned its keep.
The fix
On a 406 — and only a 406 — the diff is re-assembled from the
pulls/N/filesAPI, which answers for ten times as many files. That endpointhands back the hunks per file but drops the headers around them, so cerber puts
them back:
diff --git,---,+++, andrename from/rename towhere afile moved. The result parses exactly like
gh pr diffoutput, which is thepoint —
splitDiffByFile, the line anchoring, the chapter file lists and thesend path all read it without knowing which way it was fetched.
Two gaps are stated rather than left silent, because an unexplained one reads as
"nothing changed here":
big, come back with no patch at all. They keep their headers and carry a line
saying which case it is and how many lines it covers. The note sits outside
any hunk, so nothing can mistake it for a changed line.
Every other
ghfailure still fails the run. A fallback that swallowed an autherror would turn "you are logged out" into a review of an empty diff.
Technical details
src/core/gh.ts—fetchPrDiffgains the fallback;assembleDiffis a purefunction over the API rows, so the header reconstruction is testable without
a network. Old and new paths are derived from
status, not guessed from thefilename, so an added file gets
--- /dev/nulland a deletion is stillattributed to the path it removed — the same way
gh pr diffattributes it.too_large/ "exceeded the maximum number of files"rather than on the exit code, which every
ghfailure shares.SPEC.md§11.2 — the run sequence now says this normatively, including thata 406 is the only diff failure that falls back.
Nothing about the prompt changes. A diff this size was already truncated at
300,000 characters with the agent told to read the rest in the checkout, and
that is still what happens — the fix is about the review existing at all, not
about how much of a large diff reaches the model.
Verification
pnpm typecheck,pnpm test(523 passing) andpnpm buildall green.10 new tests cover header reconstruction, added/removed/renamed files,
withheld patches, the 3000-file note, and that an auth failure still throws
instead of falling back.
reconstructed diff, 366 files, 45,777 anchorable lines, and a finished review
with chapters, comments and a verdict where there had been an error string.
Worth knowing
Reviewing a 366-file PR leans much harder on the source checkout than usual,
since only the first 300,000 characters of the diff reach the prompt. That
threshold is unchanged here and may deserve its own look — this PR's claim is
narrower: a PR GitHub won't diff is reviewed rather than failed.