From 2350a31d5fc387f11317f0e2308d529f3fc80bec Mon Sep 17 00:00:00 2001 From: Noah Lindner Date: Sat, 25 Jul 2026 09:05:27 -0400 Subject: [PATCH] sweep: catch the files variant of GitHub's too-large diff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #49 only matched "diff exceeded the maximum number of lines". GitHub 406s on TWO limits — 20000 lines and 300 files — so #8338 and #8241 (both over the file cap) stayed in the retry loop after the fix, while #8308 and #8121 correctly dropped out. Match gh's stable `PullRequest.diff too_large` code, which both variants carry, and keep the prose patterns as a fallback. Both real payloads are pinned in the test. --- src/review-sweep.test.ts | 13 +++++++++---- src/review-sweep.ts | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/review-sweep.test.ts b/src/review-sweep.test.ts index baf6abe..73addc3 100644 --- a/src/review-sweep.test.ts +++ b/src/review-sweep.test.ts @@ -283,12 +283,17 @@ test('isRateLimited flags plan exhaustion, not ordinary failures', () => { expect(isRateLimited('the diff had no reviewable changes')).toBe(false) }) -// GitHub 406s on diffs past 20k lines, so those PRs can never be fetched. Reading that as an ordinary gh failure -// re-fetched them every 60s forever (11k wasted attempts on bevyl before this was caught). +// GitHub 406s on diffs past 20k lines OR 300 files, so those PRs can never be fetched. Reading that as an ordinary +// gh failure re-fetched them every 60s forever (11k wasted attempts on bevyl before this was caught). Both real +// payloads below are copied verbatim from the live failures — the FILES variant was missed on the first pass and +// kept #8338/#8241 looping, so both stay pinned here. test('isDiffTooLarge flags the GitHub size refusal, not ordinary gh failures', () => { - const real = + const tooManyLines = 'could not find pull request diff: HTTP 406: Sorry, the diff exceeded the maximum number of lines (20000) (https://api.github.com/repos/acme/widgets/pulls/8121)\nPullRequest.diff too_large' - expect(isDiffTooLarge(real)).toBe(true) + const tooManyFiles = + "could not find pull request diff: HTTP 406: Sorry, the diff exceeded the maximum number of files (300). Consider using 'List pull requests files' API or locally cloning the repository instead. (https://api.github.com/repos/acme/widgets/pulls/8338)\nPullRequest.diff too_large" + expect(isDiffTooLarge(tooManyLines)).toBe(true) + expect(isDiffTooLarge(tooManyFiles)).toBe(true) expect(isDiffTooLarge('gh: HTTP 502 Bad Gateway')).toBe(false) expect(isDiffTooLarge('could not find pull request diff: HTTP 404')).toBe(false) expect(isDiffTooLarge('gh: exceeded your quota')).toBe(false) // a quota wall is transient; this is not it diff --git a/src/review-sweep.ts b/src/review-sweep.ts index 12ed963..706d336 100755 --- a/src/review-sweep.ts +++ b/src/review-sweep.ts @@ -253,12 +253,15 @@ export function priorReviewThread(comments: Comment[]): string { return thread.length > MEMORY_BYTE_CAP ? thread.slice(-MEMORY_BYTE_CAP) : thread // keep the most recent context } -// GitHub's diff endpoint hard-refuses anything past this with a 406, so a big enough PR can't even be MEASURED. -// It is GitHub's limit, not ours — DIFF_LINE_CAP can be set above it but never reached. -const GH_DIFF_MAX_LINES = 20_000 +// GitHub's diff endpoint 406s past EITHER of these, so a big enough PR can't even be MEASURED. They are GitHub's +// limits, not ours — DIFF_LINE_CAP can be set above the line one but never reached. +const GH_DIFF_LIMITS = '20000-line / 300-file' -/** A `gh pr diff` failure that is GitHub's size refusal rather than a transient error. Retrying can never fix it. */ -export const isDiffTooLarge = (output: string): boolean => /diff exceeded the maximum number of lines/i.test(output) +/** A `gh pr diff` failure that is GitHub's size refusal rather than a transient error — retrying can never fix it. + * Matches on gh's stable `too_large` code first: the prose differs per limit (lines vs files) and can be reworded, + * but both variants carry the code. Missing one variant is what kept #8338/#8241 looping after the first fix. */ +export const isDiffTooLarge = (output: string): boolean => + /PullRequest\.diff too_large|diff exceeded the maximum number of (lines|files)/i.test(output) // The RUNNER fetches the diff (not codex) so codex needs no network or gh — it reviews the diff straight from the // prompt, sandboxed. Never treat a failed read as "0 lines" (a silent under-cap that would auto-review something it @@ -1164,7 +1167,7 @@ async function reviewOne(cfg: Config, ref: string, post: boolean): Promise if (!read.ok) { console.error( read.reason === 'too-large' - ? `stupify review: ${slug}#${number} is over GitHub's ${GH_DIFF_MAX_LINES}-line diff API limit, so gh can't return it and there's nothing to review. Split the PR.` + ? `stupify review: ${slug}#${number} is over GitHub's ${GH_DIFF_LIMITS} diff API limit, so gh can't return it and there's nothing to review. Split the PR.` : `stupify review: couldn't fetch the diff for ${slug}#${number}.`, ) process.exit(1) @@ -1347,10 +1350,10 @@ async function main(): Promise { if (!read.ok && read.reason === 'too-large') { // Terminal: gh will never hand us this diff, so there is nothing to retry and nothing to measure. Say so // plainly — the old wording promised a retry that could not possibly succeed. - const why = `diff over GitHub's ${GH_DIFF_MAX_LINES}-line API limit — gh can't return it, so it can't be reviewed; split the PR` + const why = `diff over GitHub's ${GH_DIFF_LIMITS} API limit — gh can't return it, so it can't be reviewed; split the PR` log(`skip #${pr.number} — ${why}`) skipStatusPr(cfg, status, pr, 'skipped', why) - setCommitStatus(cfg, commitStatuses, pr, 'success', `diff over GitHub's ${GH_DIFF_MAX_LINES}-line API limit; split the PR to get a review`) + setCommitStatus(cfg, commitStatuses, pr, 'success', `diff over GitHub's ${GH_DIFF_LIMITS} API limit; split the PR to get a review`) continue } if (!read.ok) {