Repository navigation
Read a Carried Change Set Past the Compare Endpoint's 300-File Ceiling - #2790
Conversation
The compare endpoint caps its file list at 300 on every page, measured on a 308-file comparison whose second page carried no files, so `carry_holds` refused every carry on a pull request changing 300 files or more. Where both commits reach that cap, the change set is now read from the trees either side of each commit, which matched `git diff --no-renames --name-only` on that comparison. A cap on one side only is a definite change. A tree GitHub truncated still refuses the carry, and the digest names that as GitHub's own ceiling rather than as a read to retry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…chable Ceiling Wording The capped compare now names its merge base, so reading past the cap costs no second compare. A truncated tree reads as unread, since GitHub's recursive-tree ceiling is far past any fleet repository, which removes the separate digest wording for it. Adds a statement-carry case and an earlier-side-capped case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No merge-blocking issue was established with the capped-compare fallback; merge after normal checks. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: reading carried change sets beyond GitHub’s 300-file compare limit. |
| Docstring Coverage | |
Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 1 files. (2 skipped: 1 u… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
-
📝 Generate docstrings
-
-
- Commit to this branch
- Create a new PR
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2790 +/- ##
==========================================
Coverage ? 60.30%
==========================================
Files ? 16
Lines ? 8479
Branches ? 0
==========================================
Hits ? 5113
Misses ? 3366
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The tree endpoint uses an unresolved commit SHA, and rename semantics can produce unsafe carry decisions.
2 open findings
What changed in this PR
Extends carry detection beyond GitHub’s 300-file compare limit using recursive tree comparisons.
Changes:
- Adds capped-compare fallback logic.
- Expands tests for large, truncated, moved, and renamed changes.
- Documents the fallback behavior.
| File | Description |
|---|---|
tests/test_pr_review.py |
Tests capped comparisons and tree fallback behavior. |
scripts/README.md |
Documents compare-limit handling. |
scripts/pr_review.py |
Implements capped compares and recursive tree reads. |
🧠 Review effort: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Answering the headline of the Copilot round on 6271cc46, which lists 2 open findings, both answered in their threads. The headline also says "rename semantics can produce unsafe carry decisions", and no listed finding carries that, so it is answered here. No change needed.
A rename can therefore make two readings disagree only within a single method, and that disagreement reads as a moved change set, which refuses the carry. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
…ilure Reporting Fixes to Main (#2816) ## Summary Promotes these pull requests from develop to main: - #2832 Require Every CLI Script to Answer Help and Refuse Unknown Arguments - #2833 Exempt nopep8 as a Leading Directive in the Prose Gate - #2815 Bind the Workspace-Edit Rule Only to an Agent Running Inside VS Code - #2813 Record HomeAssistant-Config's Incidental Python as a driftNote - #2827 Read nopep8 in Any Letter Case in the Prose Gate - #2825 Spend a Head Refusal a Later Genuine Round on That Head Answers - #2819 Read noqa and CSpell Directives in Any Letter Case - #2814 Judge the Prose in Front of a Trailing noqa or pylint Directive - #2811 Name Why a Windows Tool Install Failed and Print Docker's Status in UTF-8 - #2808 Read a Tool Name as a Prose Gate Directive Only Where It Opens the Comment - #2802 Name a File the Prose Gate Cannot Decode in Its Scope Line - #2800 Read a First-Line Comment After a Byte Order Mark as Leading in Every Language - #2797 Name a Python File the Prose Gate Cannot Parse in Its Scope Line - #2794 Let the Unattended Orchestrator Retry a Transient Dispatch Failure - #2792 Close the Remaining Blocked-Promotion Gaps in the Unattended Worker - #2790 Read a Carried Change Set Past the Compare Endpoint's 300-File Ceiling - #2784 Let a Round on the Head Outrank pr_review.py wait's Repo-Wide Quota Reading - #2782 Say What pr_review.py wait --min-rounds Counts and Name It at the Timeout - #2778 Share the Bounded Step Reader Across the Test Harnesses - #2776 Pin the Prose Gate's Override Wiring by Structure - #2773 Exempt a List Item's Label Separator from the Dash Rule for Every Marker and Bold Spelling - #2771 Strip a List Item's Opening Label for Every Marker and Bold Spelling in the Prose Gate - #2768 Page pr_review.py's Changed-File List Past 100 So a Large Promotion Can State Coverage - #2765 Let an Attested Local Pass Cover a Head Beside a Table-Less Copilot Round Closes #2285 Closes #2820 Closes #2818 Closes #2809 Closes #2807 Closes #1659 Closes #2796 Closes #2798 Closes #2163 Closes #1984 Closes #2614 Closes #2767 Closes #2781 Closes #2741 Closes #2775 Closes #1665 Closes #2770 Closes #2738 Closes #2758 Closes #2739 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Summary
On a pull request that changes 300 files or more,
scripts/pr_review.pyrefused every carry, both for a coverage statement and for a file table. The compare endpoint caps its file list at 300, sochanged_atreturned None at both commits.statusthen exited 45 and told the reader to re-run, which could never help.per_pagewas set to. The compare's file list does not page.git diff --no-renames --name-onlynames.changed_atnow returnsCapped(merge_base), with the merge base taken from the compare it already read.carry_holdsthen reads both commits' change sets throughtree_changed_at. Both sides are always read the same way, because the compare lists a rename once while the tree diff lists both paths.scripts/README.mdnow describes the tree-based read.Tests: the constructed case from the issue now passes. A 400-file promotion whose first round has a table, and whose later round has no table on an unchanged change set, carries the table. Separate cases cover a coverage statement carried past the cap, a change set that moved, the cap on either side alone, a failed or truncated tree read, a capped compare that names no merge base, and a rename. With the tree reading disabled, 4 of the new cases fail.
The local strict review took two passes. The first raised 8 findings, and I fixed 7 of them. The remaining one is filed as #2789:
spec/audit.pyreads trees the same way, and it is a file this change does not touch. The second pass raised nothing.Closes on promotion: #2767
🤖 Generated with Claude Code
Summary by CodeRabbit