sweep: stop retrying diffs GitHub refuses to serve

src/review-sweep.test.ts

@@ -5,7 +5,7 @@
 import { expect, test } from 'bun:test'
 import { createVerify, generateKeyPairSync } from 'node:crypto'
 import { join } from 'node:path'
-import { type Config, appJwt, commitStatusDescription, commitStatusForSweepResult, dismissedFindings, diffRightLines, finalCodexMessage, FIXED_TOKEN, isFixedReview, isNoopReview, isRateLimited, NOOP_TOKEN, parseFindings, type Pr, pidAlive, priorReviewThread, reviewPrompt, stablePrefix, STILL_NOTE, stripSignoff } from './review-sweep'
+import { type Config, appJwt, commitStatusDescription, commitStatusForSweepResult, dismissedFindings, diffRightLines, finalCodexMessage, FIXED_TOKEN, isDiffTooLarge, isFixedReview, isNoopReview, isRateLimited, NOOP_TOKEN, parseFindings, type Pr, pidAlive, priorReviewThread, reviewPrompt, stablePrefix, STILL_NOTE, stripSignoff } from './review-sweep'
 
 const REVIEW_DIR = join(import.meta.dir, '..', '.review') // the real spec/rubric/corpus shipped in this repo
 const THIS_PR = '===== THIS PR' // the boundary between the cached prefix and the per-PR tail
@@ -283,6 +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).
+test('isDiffTooLarge flags the GitHub size refusal, not ordinary gh failures', () => {
+  const real =
+    '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)
+  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
+})
+
 // codex sometimes says a convergence token as its final message without writing the review file — the transcript's
 // final message (last `codex` line to its `tokens used` footer) is the only part allowed to stand in for the file.
 test('finalCodexMessage extracts the last codex message, not the transcript', () => {

src/review-sweep.ts

@@ -253,12 +253,23 @@ 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
+
+/** 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)
+
 // The RUNNER fetches the diff (not codex) so codex needs no network or gh — it reviews the diff straight from the
-// prompt, sandboxed. null = gh failed; the caller skips and retries next sweep rather than treating an unreadable
-// diff as "0 lines" (a silent under-cap that would auto-review something it never measured).
-function getDiff(cfg: Config, number: number): string | null {
+// prompt, sandboxed. Never treat a failed read as "0 lines" (a silent under-cap that would auto-review something it
+// never measured) — and keep the two failure modes apart: 'unreadable' is transient and worth retrying, but
+// 'too-large' is terminal, and conflating them is what left oversized PRs re-fetched every 60s forever.
+type DiffRead = { ok: true; diff: string } | { ok: false; reason: 'unreadable' | 'too-large' }
+
+function getDiff(cfg: Config, number: number): DiffRead {
   const r = exec('gh', ['pr', 'diff', String(number), '--repo', cfg.slug])
-  return r.ok ? r.stdout : null
+  if (r.ok) return { ok: true, diff: r.stdout }
+  return { ok: false, reason: isDiffTooLarge(r.combined) ? 'too-large' : 'unreadable' }
 }
 
 const diffLineCount = (diff: string): number => (diff ? diff.split('\n').length - (diff.endsWith('\n') ? 1 : 0) : 0)
@@ -1149,11 +1160,16 @@ async function reviewOne(cfg: Config, ref: string, post: boolean): Promise<void>
     /* the marker just won't carry a real SHA — harmless for a one-off */
   }
   const pr: Pr = { number, headRefOid: meta.headRefOid ?? '', isDraft: false, author: { login: '', is_bot: false }, labels: [], title: meta.title ?? '', body: meta.body ?? '' }
-  const diff = getDiff(cfg, number)
-  if (diff === null) {
-    console.error(`stupify review: couldn't fetch the diff for ${slug}#${number}.`)
+  const read = getDiff(cfg, number)
+  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: couldn't fetch the diff for ${slug}#${number}.`,
+    )
     process.exit(1)
   }
+  const diff = read.diff
   console.error(`reviewing ${slug}#${number} …`) // progress on stderr; stdout stays just the review
   const r = await runReview(cfg, pr, '', diff) // no memory: a manual review is always a fresh, full take
   if (r.kind === 'limit' || r.kind === 'fail') {
@@ -1327,13 +1343,23 @@ async function main(): Promise<void> {
     }
 
     // Fetch the diff once, here in the runner — codex reviews it from the prompt with no network/gh of its own.
-    const diff = getDiff(cfg, pr.number)
-    if (diff === null) {
+    const read = getDiff(cfg, pr.number)
+    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`
+      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`)
+      continue
+    }
+    if (!read.ok) {
       log(`skip #${pr.number} — couldn't read its diff from gh; will retry next sweep`)
       skipStatusPr(cfg, status, pr, 'skipped', "couldn't read diff from gh; will retry next sweep")
       setCommitStatus(cfg, commitStatuses, pr, 'error', "couldn't read PR diff; retrying next sweep")
       continue
     }
+    const diff = read.diff
     const lines = diffLineCount(diff)
     // auto-scope only: skip oversized diffs UNLESS the PR carries the review label (the documented force-include).
     // (label-scope means you already opted in, so size never gates there.)