fix(pr-review-toolkit): recover truncated file pages - #57
Conversation
Collect changed-file metadata when patch bodies are truncated, then recover only incomplete page ranges with single-file reads. This keeps large PR manifests complete without retrying the full manifest at smaller page sizes. Assisted-by: Codex:gpt-5
|
Warning Review limit reached
More reviews will be available in 50 minutes and 22 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe plugin manifest version changes to 1.6.2. The PR review skill now reads file manifests with fixed-size pages, detects incomplete pages, retries missing single-file slots, and returns completion metadata after recovery. ChangesManifest recovery and version metadata
Sequence Diagram(s)sequenceDiagram
participant collectCompleteFileManifest
participant collectFilePages
participant collectFilePage
participant filePageResultIssues
participant missingSingleFilePages
collectCompleteFileManifest->>collectFilePages: fetch pages with FILE_PAGE_SIZE
collectFilePages->>collectFilePage: collect each page in parallel
collectCompleteFileManifest->>filePageResultIssues: compare expected vs actual file counts
filePageResultIssues-->>collectCompleteFileManifest: incomplete page slots
collectCompleteFileManifest->>collectFilePages: retry missing slots with perPage=1
collectCompleteFileManifest->>missingSingleFilePages: check remaining single-file gaps
missingSingleFilePages-->>collectCompleteFileManifest: remaining missing slots or complete manifest
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR reworks the changed-file manifest collection in the pr-review-toolkit workflow (review-pr.js) to better handle large PRs whose file pages get truncated. Previously, when a page came back incomplete the whole manifest was re-fetched at progressively smaller page sizes (10 → 5 → 1). The new approach fetches all pages at the full page size first, then surgically recovers only the truncated page ranges with perPage=1 single-file reads, preserving each recovered page's metadata. The agent prompt is also updated so file metadata is still returned even when the patch body is truncated.
Changes:
- Replaces the whole-manifest size-reducing retry loop with a "complete-first, then recover-only-incomplete-ranges" strategy in
collectCompleteFileManifest, adding helpers (filePageResultIssues,filePageIssueSummary,filePageNumbersForIssues,missingSingleFilePages). - Tags each page result with its
perPagesomergeFilePagesuses per-page values, and replacesFILE_RETRY_PAGE_SIZESwithFILE_SINGLE_PAGE_RETRIES. - Updates the collector prompt to keep file metadata when patch bodies are truncated, and bumps the plugin to
1.6.2.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pr-review-toolkit/skills/review-pr/review-pr.js | Implements truncation-tolerant manifest collection with targeted single-file recovery and updated collector prompt. |
| pr-review-toolkit/.claude-plugin/plugin.json | Bumps plugin version to 1.6.2. |
I verified that the removed functions (validateFilePageResults, collectFileManifestAttempt) and the removed FILE_RETRY_PAGE_SIZES constant have no remaining references, that mergeFilePages correctly dedups recovered/primary pages by path, and that the page→single-file slot mapping is correct. I noted two minor nits (a misleading "retrying" log message on the final attempt, and two unused hardcoded return fields).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pr-review-toolkit/skills/review-pr/review-pr.js`:
- Around line 1276-1299: The merge still includes suspect primary page results
even after recovery succeeds, so bad or stale entries can leak into the final
file list. Update the recovery flow in review-pr.js around primaryIssues,
recoveryResults, and mergeFilePages so that any page flagged by
filePageResultIssues is excluded from the primary set once its slots are
recovered. Keep only unflagged primaryResults plus the recovered results when
building pageResults, using the existing helpers filePageResultIssues,
filePageNumbersForIssues, and mergeFilePages to locate the logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 85668bba-2849-4f72-83ef-65e0ea3048c5
📒 Files selected for processing (2)
pr-review-toolkit/.claude-plugin/plugin.jsonpr-review-toolkit/skills/review-pr/review-pr.js
Exclude suspect primary page results after successful single-file recovery, avoid misleading final-attempt retry logs, and drop unused manifest status fields. Assisted-by: Codex:gpt-5
Summary
perPage=1reads and preserve recovered page metadatapr-review-toolkitto1.6.2Validation
rtk node --check pr-review-toolkit/skills/review-pr/review-pr.jsrtk claude plugin validate ./pr-review-toolkitrtk claude plugin validate .rtk uvx skillsaw --strictrtk git diff --checkSummary by CodeRabbit
New Features
Bug Fixes