Skip to content

fix(pr-pipeline): verify a diff of up to 12 chunks instead of stopping at 4 - #11646

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/pr-verify-large-diffs
Oct 31, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
fix/pr-verify-large-diffs

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #11463, #11000, #11169

Summary

pr-verify stopped any diff that needed more than 4 chunks of 300000 bytes (about 1.2 MB) with no
verdict. #11169 (a 1.33 MB diff, 5 chunks) has no verdict for that reason
(run 41219253935).
This raises the ceiling to 12 chunks, enough for a 3 MB diff. Every chunk still needs a counted
verdict, and the final judge still has to run.

Fix

  • What bounded the count: each judging job unrolls one run-claude-cli judge step per chunk,
    and a uses: step cannot loop. Each job had 4 slots, and its 125-minute timeout held five
    22-minute judge steps. seal-lane-output and open-lane-output (MAX_CHUNKS=6) are not used
    by pr-verify, so they set no bound.
  • Each job (key, subscription, fallback) now has 12 chunk slots, each identical to the earlier
    slots apart from its chunk number. The job timeout is 13 × 22 + 15 = 301 minutes, under
    GitHub's 6-hour job limit. The workflow is about 266 KB, under the 500 KB file limit.
  • The ceiling is set once, as MAX_CHUNKS=${MAX_CHUNKS:-12} in chunk-diff.sh. No step sets its
    own, and workflow.test.mjs derives the slot count from it. A workflow-level env: was ruled
    out because probe/parity.test.mjs requires that none reach the CLI.
  • write-verdict.mjs and the prompt step now accept chunk counts above 9 (chunk-10 and up).
  • Unchanged: a file over 300000 bytes is still named, and a diff past 12 chunks still stops with
    its byte count and needs N chunks, more than 12. Neither leaves a verdict, so size alone never
    writes a fail.
  • pr-review-main is not changed. It shares split-diff.py, which already takes the ceiling as
    an argument, but its own slots and the cap it passes are its own (feat(pr-pipeline): review an oversize main-run diff in chunks instead of holding it for a person #11000).

Verification

  • New tests, red before the change:
    • chunk-diff.test.mjs: a 3 MB diff under the workflow's limits splits into 12 chunks with no
      stop. A diff past the ceiling stops, naming its size and the ceiling.
    • write-verdict.test.mjs: a 12-chunk head passes, fails on chunk 11's fail, and gets no
      verdict when chunk 11 leaves none.
    • workflow.test.mjs: the ceiling is set once, and 12 prompts are written for 12 chunks.
  • Existing tests changed only where they pinned 4 slots or single-digit ids:
    • SLOTS, chainOf, the Array(5) reason counts and the Fail without a verdict conditions
      now derive from the ceiling.
    • (?:-\d|-spec) becomes (?:-\d+|-spec).
    • The MAX_CHUNKS step-env assertion now checks that no step sets it.
  • Suites passed: pr-verify (238 tests), pr-review-main, reconcile, pr-unblock
    workflow-gates, seal-lane-output, open-lane-output, upload-lane-transcript and
    resolve-config (1051 tests), and pr-merge (770 tests).
  • Gates passed: actionlint, zizmor, runner-policy, ShellCheck, check-line-citations,
    check-artifact-retention, check-lane-coverage, markdownlint and typos.
  • Smoke run of chunk-diff.sh on real diffs: feat: land the rest of the UI course migration: interface design guidance, build-generator and eval cases #11169 gives 5 chunks, and a 3.05 MB range of main
    gives 11 chunks with an 8-part manifest.

Related

🤖 Generated with Claude Code

…eeds

pr-verify stopped any diff over 4 chunks of 300000 bytes (about 1.2 MB)
with no verdict. The bound was the judge steps each job unrolls: a
run-claude-cli step cannot loop, and the job timeout held five of them.

Each judging job now has 12 chunk slots, the job timeout holds them and
the final judge (301 minutes, under the 6-hour job limit), and the
ceiling is set once, as MAX_CHUNKS in chunk-diff.sh, which the workflow
tests hold every job's slots to. 12 chunks hold a 3 MB diff; a diff past
them still stops with its size and the chunk count named, and a file
over one chunk is still named. write-verdict.mjs and the prompt step
now read chunk counts past 9. Every chunk must still return a counted
verdict, and the final judge must still run, or the head gets none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-31T17:07:01.533219Z 6f0e56e PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

This PR body does not yet satisfy the issue-linkage contract:

  • Missing a native closing keyword (Closes/Fixes/Resolves #N). If this PR references an issue it must not close, put "Refs: #N" (or "Relates to: #N") on its own line. If it relates to no GitHub issue at all, state "No linked issue" (or "No related issue:") in the body instead.

Edit the body and this comment updates itself on the next run.

@github-actions github-actions Bot added the needs-issue-linkage Pull request body has no issue linkage line; set by the PR-contract check, advisory. label Oct 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 6f0e56ea7fe182aa84c10b33f0cb0f9a33f12d41: 0 finding(s), 0 as inline comments.

No security issues found.

critical-findings: 0

I read all 8 changed files in .claude-lane/pr.diff and the PR context. pr-verify.yml and write-verdict.mjs are on the trusted-path list. I checked the bulk of the repeated workflow slots, but not every one of the roughly 4,600 changed lines of pr-verify.yml.

What I checked

  • Chunk-gating expressions (key job). I read the final versions of "Write the chunk results", "Write the verdict" and "Fail without a verdict" in full. The "Write the chunk results" step needs every judge step up to the chunk count to have succeeded and no chunk-fail-N output set to true, for chunks 1 to 12. "Write the verdict" fires on any counted fail, or on all chunks plus the whole-spec judge succeeding. "Fail without a verdict" requires every *-leg-N check, 1 to 12 and spec, to be non-success. The 4-slot version's fail-closed structure is kept with no slot dropped or mis-numbered. The subscription job's "Fail without a verdict" condition reads the same.
  • Not read line by line. I did not read every slot in the subscription and fallback jobs. workflow.test.mjs (changed in this PR) checks that each job's slots repeat the first slot apart from the chunk number. SLOTS, chainOf and legsClear are derived from MAX_CHUNKS in chunk-diff.sh.
  • Judge-step permissions. Each new judge slot has the same tool lockdown as the earlier ones. That is tools: Read, an allow rule of Read(./pr-verify-data/**), and denies for .base, Bash, Edit, Write, web tools, Agent and mcp__*. The whole-spec judge keeps its Read(./pr-verify-data/diff/**) deny, so it still cannot read the diff.
  • Trigger and token scope. Job permissions: are unchanged (contents/pull-requests/actions: read). The if: gate (main ref, a pull_request run, a same-repo head) is unchanged. Neither secrets nor trust scope widen.
  • Count validation. The write-verdict.mjs regex changes (^chunk-[1-9][0-9]?$, ^[1-9][0-9]?$) and the prompt step's CHUNKS =~ ^[1-9][0-9]?$ check admit 1 to 99. The chunk count comes from chunk-diff.sh, which now defaults MAX_CHUNKS to 12, so the real ceiling is 12. The directory count must still equal CHUNKS, so a mismatch still throws. No other single-digit assumption remains in the non-test pr-verify scripts.
  • Removed MAX_CHUNKS: "4" env. The "Write the diff" step no longer sets a cap, and the script default of 12 applies. The new test pins that no step sets one. No untrusted input reaches that variable, since the step env comes from the workflow file.
  • Oversize handling. A file over 300000 bytes, or a diff past 12 chunks, still stops with no verdict. Size alone cannot write a pass or a fail.
  • Instruction-surface deletions. The diff removes none. Only comments, tests and workflow slots change.
  • Plugin manifests. The diff touches none.

Spend (not a hold): a head can now cost up to 12 chunk judge runs plus the whole-spec judge, where it was 4 plus one. Job timeouts rise from 125 to 301 minutes. This spends prepaid API credits and subscription usage only. No paid plan, billing setting or billed resource changes.

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 6f0e56ea7fe182aa84c10b33f0cb0f9a33f12d41: 0 finding(s), 0 as inline comments.

No findings. I read the full diff in .claude-lane/pr.diff and the PR context. I ran nothing (no shell here), so everything below comes from reading the diff, not from running tests or the workflow.

What I checked

  • Slots are consistent. Every steps.judge-N, key-leg-N, sub-leg-N and chunk-fail-N reference for N=5..12 appears in each place the earlier four did. That covers the job outputs, the "Write the chunk results" if, the "Write the verdict" if, the EXECUTION_FILES and LOG_FILES lists, the leg-check logs, and the "Fail without a verdict" if. I did not find a slot that was left out.
  • Chunk-number patterns accept two digits. The ^[1-9][0-9]?$ patterns in write-verdict.mjs:527 and :529 and in the prompt step (pr-verify.yml:327, :2082, :3802) all take 10 to 12. A search of .github/scripts/pr-verify found no other single-digit chunk pattern outside tests. The manifest sub(/^chunk [0-9]+ / in chunk-diff.sh:68 already handles several digits.
  • The ceiling is set in one place. MAX_CHUNKS=${MAX_CHUNKS:-12} in chunk-diff.sh is the only default. The workflow step no longer sets it. workflow.test.mjs reads the number from the script, so the slot count follows it. In chunk-diff.test.mjs, MAX_CHUNKS: undefined in the spawn env is skipped by Node, so the script's own default applies.
  • Job timeout. 301 minutes is 13×22+15, under GitHub's 360-minute job limit. The longest expression I found (the "Write the chunk results" if) is far below the expression length limit.
  • Oversize behaviour is unchanged. A file over 300000 bytes, or a diff needing more than 12 chunks, still stops with no verdict, so size alone never writes a fail. The new tests cover both stops.
  • The old limit is still described only where it applies. pr-review-main and its docs still say 4 chunks, which matches the PR's statement that it is out of scope.
  • Docs and changelog fragment. The README and the .changes fragment (bump: patch) match the code. The README says a diff needing more than 12 chunks is "over about 3.6 MB", while the fragment and the PR text say 12 chunks hold a 3 MB diff. Since files are packed whole, the actual threshold depends on how they pack, so this is not a defect.
  • Trusted paths. pr-verify.yml and write-verdict.mjs are trusted-path matches. I looked at what they gate. The change adds no permissions, secrets, triggers or checkout changes, and no new inputs reach a run: script. The key, subscription and fallback jobs keep their separate credentials.

Spend: this change raises how many prepaid-credit judge runs one large head can use, up to 12 chunk judges plus the final spec judge per leg, each capped at 22 minutes. This is prepaid credit or subscription usage only, so it is not a hold.

Settled threads and closing issues: the PR context lists no closing-issue sections and no settled-threads file was named. The PR body cites #11463, #11000 and #11169 as "Refs". I did not read them.

Everything in the PR body about suites run, gates passed and smoke runs is the author's claim. I did not confirm any of it.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 31, 2026
Merged via the queue into main with commit 5cce729 Oct 31, 2026
87 of 88 checks passed
@kyle-sexton
kyle-sexton deleted the fix/pr-verify-large-diffs branch October 31, 2026 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-issue-linkage Pull request body has no issue linkage line; set by the PR-contract check, advisory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant