refactor(pr-review-toolkit): simplify overlap sections and restructure report#65
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review 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 refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUpdates the review-pr plugin's overlap classification taxonomy (replacing possiblePlusOnes/partialOverlaps with overlaps/already_covered/relatedToExisting), adds commentId-based thread reply posting, removes action-plan/coverage-summary generation, rewrites SKILL.md's git preflight and posting instructions, and bumps the manifest version. ChangesReview Board Overlap & Posting Refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 refactors the pr-review-toolkit review board model and reporting flow to simplify overlap handling, streamline the board header content, and enable posting overlap findings as replies to existing review threads.
Changes:
- Consolidates overlap sections into
relatedToExistingand simplifies overlap/recommendation enums in the workflow schemas. - Restructures the SKILL report format and replaces the command-keyword interaction flow with contextual
AskUserQuestionoptions. - Adds support (and instructions) for posting overlap findings as thread replies using a numeric
commentId.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pr-review-toolkit/skills/review-pr/SKILL.md | Updates the board presentation, interaction flow, and adds guidance for posting overlap findings as thread replies. |
| pr-review-toolkit/skills/review-pr/review-pr.js | Updates schemas and routing to support the new overlap model (relatedToExisting) and includes commentId in thread/overlap data. |
| pr-review-toolkit/.claude-plugin/plugin.json | Bumps plugin version to 1.8.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e1cbfa4 to
0d82f39
Compare
…e report Collapse possiblePlusOnes/partialOverlaps into a single relatedToExisting section, simplify overlap and recommendation enums to binary (overlaps/already_covered), remove actionPlan and coverageSummary from the board schema, restructure the SKILL.md report layout with per-finding recommendation rationale, replace the 9-command keyword system with contextual AskUserQuestion options, and add thread reply posting for overlap findings via add_reply_to_pull_request_comment. Assisted-by: Claude:claude-opus-4-6
0d82f39 to
07fee48
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pr-review-toolkit/skills/review-pr/review-pr.js (1)
873-879: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve
commentIdwhen merging equal-strength overlaps.Equal-ranked statuses always keep the left overlap, so merging two
overlapsitems can drop acommentIdfound on the right item. That breaks the downstream thread-reply path forrelatedToExisting.Proposed fix
const leftStatus = (left && left.status) || 'none' const rightStatus = (right && right.status) || 'none' const selected = (order[rightStatus] > order[leftStatus]) ? right : left if (!selected) return { status: 'none', threadId: '', rationale: '' } + const fallback = selected === left ? right : left return { status: selected.status || 'none', - threadId: selected.threadId || '', - commentId: selected.commentId || undefined, + threadId: selected.threadId || (fallback && fallback.threadId) || '', + commentId: selected.commentId != null + ? selected.commentId + : fallback && fallback.commentId != null + ? fallback.commentId + : undefined, rationale: combineText(left && left.rationale, right && right.rationale) }🤖 Prompt for 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. In `@pr-review-toolkit/skills/review-pr/review-pr.js` around lines 873 - 879, The overlap merge logic in review-pr.js is dropping commentId when two items have equal-strength statuses because the tie always selects the left entry. Update the selection/merge behavior around the selected overlap object so that, when statuses are equal, any existing commentId from the right overlap is preserved if the left one is missing. Keep the fix localized to the overlap resolution path that returns status, threadId, and commentId so downstream relatedToExisting thread replies still have the right identifier.
🤖 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/.claude-plugin/plugin.json`:
- Line 3: The plugin manifest version is too low for a breaking change; update
the version in plugin.json to a new major release because the board field
removal and posting contract change are not backward compatible. Adjust the
manifest’s version field to the next major version so it clearly signals the
compatibility break.
In `@pr-review-toolkit/skills/review-pr/review-pr.js`:
- Around line 776-779: The overlap handling in overlapFromFinding() and
inferThreadOverlap() is short-circuiting recommendation-only overlaps before
thread-matching can run, so reply-capable findings can miss their existing
thread context. Update inferThreadOverlap() to keep processing when overlap
status is overlaps but threadId/commentId are still missing, and let the
matching logic infer a target thread before returning. Make sure the overlap
object construction around overlapFromFinding(), status handling, and the
related checks in inferThreadOverlap() and nearby overlap resolution paths still
preserve existing non-empty threadId/commentId when present.
---
Outside diff comments:
In `@pr-review-toolkit/skills/review-pr/review-pr.js`:
- Around line 873-879: The overlap merge logic in review-pr.js is dropping
commentId when two items have equal-strength statuses because the tie always
selects the left entry. Update the selection/merge behavior around the selected
overlap object so that, when statuses are equal, any existing commentId from the
right overlap is preserved if the left one is missing. Keep the fix localized to
the overlap resolution path that returns status, threadId, and commentId so
downstream relatedToExisting thread replies still have the right identifier.
🪄 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: 42a45c87-caa4-42ab-8338-c6830522158f
📒 Files selected for processing (4)
pr-review-toolkit/.claude-plugin/plugin.jsonpr-review-toolkit/README.mdpr-review-toolkit/skills/review-pr/SKILL.mdpr-review-toolkit/skills/review-pr/review-pr.js
…rning Tighten prose across all sections to bring estimated token count under the 3,000-token skillsaw warn threshold (~3,379 → ~2,450). Preserves all behavioral instructions including ad-hoc script prohibition and allowed-tools constraints. Assisted-by: Claude:claude-opus-4-6
07fee48 to
dc16454
Compare
Summary
possiblePlusOnesandpartialOverlapsinto a singlerelatedToExistingboard section, simplifying the overlap status enum tonone/overlaps/already_coveredand the recommendation enum from 6 to 5 valuesactionPlanandcoverageSummaryfrom the board schema — heading now derives counts from section array lengths and lists full reviewer agent names fromreviewMeta.selectedReviewersAskUserQuestionoptions that adapt to board state (recommended findings, only overlaps, nothing postable)add_reply_to_pull_request_comment— uses a new numericcommentIdfield inTHREAD_SCHEMAandBOARD_ITEM_SCHEMAto post as replies to existing threads instead of creating duplicate commentsTest plan
claude plugin validate ./pr-review-toolkitpossiblePlusOnes,partialOverlaps,possible_plus_one,partial_overlap,actionPlanForBoard,coverageSummary) — zero matchesreview-pr.jshas no syntax errors:node -c pr-review-toolkit/skills/review-pr/review-pr.js/review-pron a PR with existing review threads to verify overlap findings route torelatedToExistingand the contextual interaction flow works/review-pron a clean PR (no existing threads) to verify recommended findings display correctly with the new heading formatadd_reply_to_pull_request_commentwhen approvedSummary by CodeRabbit
New Features
Bug Fixes