Repository navigation
feat(pr-review-cli): count byte-identical changed files as read once - #947
Conversation
A diff in scope over one review's 300000-byte read budget was reviewed by one model step and always concluded partial. Each model leg now splits it at file boundaries, each file whole, into at most 4 chunks with claude-code-plugins' .github/scripts/pr-review-main/split-diff.py, read from that repository's default-branch tip with the review plugin, and reviews each chunk in its own run-claude-cli step, in order. - Split step: runs only over the budget, into a chunk directory that must be new, refuses a symlinked diff, and outputs the chunk count and each chunk's file count. A refused or failed split reviews unsplit, which the extract step keeps partial. - Compose: one prompt per chunk naming its patch and the manifest, with the summary and comment caps divided by the chunk count so the combined review still fits one environment string. - Chunk k runs only when chunk k-1 succeeded and handed nothing on. The job's handoff, lane class and outcome report follow the last chunk that ran; a credit stop in any chunk is recorded. - Extract: complete only when every chunk's step succeeded and returned a well-formed review; findings, comments and files read are the chunks' sums, each chunk's files read capped at the files the job counted in it, and each chunk file must be a regular file within the budget. A failed, skipped or verdict-less chunk is incomplete, so the status check is never success before every chunk returns. - Job limits grow by the three extra chunk steps, their timing steps and the split step. Refs: melodic-software/claude-code-plugins#11575 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The plugin step now also checks out .github/scripts/pr-review-main for split-diff.py; collect-settled-threads.test.cjs pinned the old set. Refs: melodic-software/claude-code-plugins#11575 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A head that commits .claude-lane/chunks failed the split step silently; it now warns and takes one review, which stays partial when over budget. Refs: melodic-software/claude-code-plugins#11575 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A chunked review was told to report any reference to a file another chunk holds, which flagged valid caller-helper pairs split by the partition. The model now reads the other file's changes as context and reports only a verified incompatibility, in all three credential legs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nk-oversize # Conflicts: # .github/scripts/pr-review-cli.test.cjs # .github/workflows/pr-review-cli.yml
A pull request that syncs one file into many places changed files the model rightly reads once, so its files_read fell below the files in scope and the review was published as partial. The full-diff step groups changed, non-deleted files by post-image blob (git diff --raw --no-abbrev) into .claude-lane/identical.txt and outputs the number of extra copies. The prompt names the file as untrusted data and tells the model to read each group's first path and not count its copies; the extract step credits the copies, capping the model's count first. Only one review of the whole pull request gets the note and the credit: incremental and chunked reviews keep copies at 0. A planted file or any failure groups nothing. All three legs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…entical-files # Conflicts: # .github/workflows/pr-review-cli.yml
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
claude-security-review reviewed No security issues found in the incremental change since efefa63 ( What I read. The full-diff step now groups a pull request's identical changes. It keys on both modes and both blobs from Checks.
I did not run the awk program, because the sandbox refused the awk-version check. No trust-control widening, secret or token exposure, or untrusted text reaching a credentialed lane. No manifest items were hit. critical-findings: 0 |
|
claude-review reviewed Incremental review of 1d31b87 (since efefa63)No findings. I reviewed
I could not run PR context: the body states no issue to close ( |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efefa636fd
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Grouping by post-image blob alone let different changes converge: two files edited to the same content from different pre-images, or a control file and a harmless file both truncated to empty, grouped together and the copy's change was never read while coverage reported it complete. The full-diff step now keys each group on the whole raw-diff record (both modes, both blobs, status), and only modifications of regular files (100644/100755 on both sides) group: additions, deletions, renames, copies, type changes, symlinks and submodules never do. A path matching a hold-paths glob, matched as the scope step matches docs-only-paths, never joins a group in any position, so the TRUSTED PATHS instruction is never undercut. The same change lands in all three legs, and the prompt now says copies are files whose whole change is identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No related issue: a review-lane defect on melodic-software/claude-code-plugins#11791, assigned by the GitHub Support hub.
Summary
claude-code-plugins#11791 changed 19 files: 3 real files, 8 byte-identical copies of one shared library, and 8 changelog fragments. The review read 11 of them, correctly skipping the 8 identical copies. It then failed as
partial, because coverage countsfiles_readagainst every changed file, so the PR stalled.Fix
In all three credential legs:
git diff --raw --no-abbrev) and writes.claude-lane/identical.txt, one group per line with the first path followed by its copies. It outputscopies.files_read.read = min(read, changed - copies) + copies.It fails closed: an unreadable listing, a planted
identical.txt, or a malformed count givescopies=0, which is today's behavior. Incremental and chunked reviews get no dedupe. For incremental reviews, the groups would come from the whole PR, not the scope. For chunked reviews, the splitter lives in claude-code-plugins and does not keep a group in one chunk.Verification
node --test .github/scripts/pr-review-cli.test.cjs: 53/53, including a real-git grouping fixture and cases for the prompt line and the coverage credit (11 read + 8 copies = complete; 10 + 8 = partial; over-reports are capped).node --test .github/scripts/pr-review-cli-api-calls.test.cjs: 10/10.Related
🤖 Generated with Claude Code