feat(review-pr): semantic contextualization against existing review threads - #51
Conversation
|
Warning Review limit reached
More reviews will be available in 51 minutes and 5 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 refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. 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 (3)
📝 WalkthroughWalkthroughAdds a "Contextualize" phase to the Changesreview-pr Contextualize Phase
Sequence Diagram(s)sequenceDiagram
participant User
participant Skill as review-pr skill
participant ThreadAgent as Thread-fetch agent
participant ClassifyAgent as Classification agent
participant VerifyAgent as Verification agent
User->>Skill: invoke review-pr
rect rgba(100, 149, 237, 0.5)
note over Skill: Phase: Analyze
Skill->>Skill: run sub-agents (code-reviewer, silent-failure-hunter, etc.)
Skill->>Skill: collect allFindings
end
rect rgba(60, 179, 113, 0.5)
note over Skill: Phase: Contextualize
Skill->>ThreadAgent: fetch existing review threads (THREAD_SCHEMA)
ThreadAgent-->>Skill: threads[]
alt threads exist
Skill->>ClassifyAgent: classify findings (CLASSIFICATION_SCHEMA)
ClassifyAgent-->>Skill: enriched findings with status/matchedThreadId/delta
Skill->>Skill: compute reviewMeta counts
opt current user has resolved threads
Skill->>VerifyAgent: verify resolutions (VERIFICATION_SCHEMA)
VerifyAgent-->>Skill: threadVerifications[]
end
end
Skill-->>User: findings + threadVerifications + reviewMeta
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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.
Actionable comments posted: 3
🤖 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 647-669: The classification records are being applied directly to
classMap without validation, which can cause findings to be mislabeled with
dedupe or partial_overlap status even when contextual fields are missing. Before
the classifications.forEach loop populates classMap, add a normalization step to
ensure each classification record contains all required contextual fields
(matchedThreadId, existingCoverage, delta, adjustedSeverity, adjustedConfidence)
with appropriate defaults when missing, so that dedupe status is only assigned
when sufficient evidence exists in the enriched findings map.
- Around line 551-580: The code contains top-level await statements (such as the
await calls to agent() in the Contextualize phase around line 557) and return
statements (around lines 569 and 677) that are not inside any function, which
causes JavaScript parse errors. Wrap the entire execution code block—starting
from before the phase definitions and ending after all the return statements—in
an async function or an immediately-invoked async function expression (IIFE) to
allow these statements to be valid at the function scope rather than module
scope.
In `@pr-review-toolkit/skills/review-pr/SKILL.md`:
- Around line 101-112: The `newIssueDescription` field in the
threadVerifications example is set to null, but the workflow schema defines this
field as a string type. Update the example in the threadVerifications array to
either provide an actual string value for newIssueDescription or omit the field
entirely to match the schema contract and avoid confusion.
🪄 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: 9e72b1fe-5707-4062-827d-2fc94798cd61
📒 Files selected for processing (3)
pr-review-toolkit/.claude-plugin/plugin.jsonpr-review-toolkit/skills/review-pr/SKILL.mdpr-review-toolkit/skills/review-pr/review-pr.js
There was a problem hiding this comment.
Pull request overview
This PR evolves the pr-review-toolkit plugin's review-pr skill from a deterministic string-match dedup step into a model-driven Contextualize workflow phase. After the existing parallel review agents produce findings, three new agents run: a fetch agent pulls all existing review threads (with replies) plus the authenticated user's login, a classify agent labels each finding new/duplicate/partial_overlap against those threads, and a verify agent (only when the current user has resolved threads from a prior review) judges whether those threads were adequately addressed. The skill's SKILL.md is updated with the new return schema and a standardized presentation template, and the agent prompts are trimmed of non-essential sections.
Changes:
- Adds a
Contextualizephase toreview-pr.jswith three new structured-output schemas (THREAD_SCHEMA,CLASSIFICATION_SCHEMA,VERIFICATION_SCHEMA) and enrichment/aggregation logic returningfindings,threadVerifications, andreviewMeta. - Rewrites
SKILL.mdPhase 3 to document the new return object and a numbered, category-grouped presentation template; removesget_mefromallowed-toolsand rewrites the data-processing constraint. - Trims agent prompts (removes "When to invoke", "Structured Output Requirements", tone/personality sections), aligns severity wording to the schema enum, and bumps the plugin version to 1.3.0.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pr-review-toolkit/skills/review-pr/review-pr.js |
Adds the Contextualize phase, new schemas, fetch/classify/verify agents, and result enrichment; trims agent prompts and fixes severity wording. |
pr-review-toolkit/skills/review-pr/SKILL.md |
Documents new return JSON, adds presentation template, renames Phase 3, removes get_me from allowed-tools, updates constraints. |
pr-review-toolkit/.claude-plugin/plugin.json |
Version bump 1.2.0 → 1.3.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
feeb224 to
56a4073
Compare
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pr-review-toolkit/skills/review-pr/review-pr.js (1)
36-66: 💤 Low valueConsider adding
requiredarrays to nested schema items for consistency.The PR objectives mention adding
requiredarrays to all new schemas.CLASSIFICATION_SCHEMAandVERIFICATION_SCHEMAboth specifyrequiredat the item level, butTHREAD_SCHEMAdoes not define required fields forthreadsitems orrepliesitems.The current code handles missing fields gracefully (line 615's filter treats undefined
isResolved/authoras falsy), so this is non-blocking. However, adding explicit required fields would make the contract clearer and align with the other schemas.♻️ Optional: Add required arrays for consistency
properties: { id: { type: 'string' }, file: { type: 'string' }, line: { type: 'number' }, author: { type: 'string' }, body: { type: 'string' }, isResolved: { type: 'boolean' }, replies: { type: 'array', items: { type: 'object', properties: { author: { type: 'string' }, body: { type: 'string' } - } + }, + required: ['author', 'body'] } } - } + }, + required: ['id', 'author', 'isResolved'] } },🤖 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 36 - 66, Add `required` arrays to the nested schema definitions in THREAD_SCHEMA for consistency with CLASSIFICATION_SCHEMA and VERIFICATION_SCHEMA. Specifically, add a `required` property to the items schema of the `threads` array (after the properties definition) to specify which fields are mandatory for each thread object, and add a `required` property to the items schema of the `replies` array to specify which fields are mandatory for each reply object. This will make the schema contract explicit and align with the pattern used in other schemas.
🤖 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.
Nitpick comments:
In `@pr-review-toolkit/skills/review-pr/review-pr.js`:
- Around line 36-66: Add `required` arrays to the nested schema definitions in
THREAD_SCHEMA for consistency with CLASSIFICATION_SCHEMA and
VERIFICATION_SCHEMA. Specifically, add a `required` property to the items schema
of the `threads` array (after the properties definition) to specify which fields
are mandatory for each thread object, and add a `required` property to the items
schema of the `replies` array to specify which fields are mandatory for each
reply object. This will make the schema contract explicit and align with the
pattern used in other schemas.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a0fabd0d-6d70-47f1-8549-d434ae17fc7d
📒 Files selected for processing (3)
pr-review-toolkit/.claude-plugin/plugin.jsonpr-review-toolkit/skills/review-pr/SKILL.mdpr-review-toolkit/skills/review-pr/review-pr.js
🚧 Files skipped from review as they are similar to previous changes (1)
- pr-review-toolkit/.claude-plugin/plugin.json
56a4073 to
0c95b1e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 93-116: The required array in the VERIFICATION_SCHEMA constant
(inside the items object of the verifications array) is missing three critical
fields that the Phase 3 presentation depends on. Update the required array to
include 'file', 'originalConcern', and 'newIssueIntroduced' in addition to the
existing required fields 'threadId', 'resolution', 'assessment', and
'isAdequate' to ensure all necessary data is validated before processing.
In `@pr-review-toolkit/skills/review-pr/SKILL.md`:
- Around line 123-130: The presentation template displays findings using a
format that always includes the line number (file:line), but the documentation
acknowledges that line numbers may be absent for file-level or PR-level
findings. Update the template sections in the New, Partial, and Duplicate
sections (around lines 168-199) to conditionally render the line number only
when it exists. If line is present, show the file:line format; if line is
absent, show only the file path to avoid rendering invalid locations like
path/to/file:undefined.
🪄 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: a0c3b910-6258-479f-8412-17c2ab997637
📒 Files selected for processing (3)
pr-review-toolkit/.claude-plugin/plugin.jsonpr-review-toolkit/skills/review-pr/SKILL.mdpr-review-toolkit/skills/review-pr/review-pr.js
🚧 Files skipped from review as they are similar to previous changes (1)
- pr-review-toolkit/.claude-plugin/plugin.json
…xisting review threads Replace the deterministic string-match dedup with a Contextualize workflow phase that uses model-driven agents to: - Fetch all existing review threads with full reply chains - Classify each finding as new/duplicate/partial_overlap with rescored confidence for partial overlaps based on the delta - Verify resolved threads from previous reviews (fixed, pushed back, or unaddressed) and flag incomplete fixes or new issues Add standardized presentation template for Phase 3 with sections for new findings, partial overlaps, duplicates, thread verifications, and positive observations. Assisted-by: Claude:claude-opus-4-6
0c95b1e to
b7bc82b
Compare
Summary
new,duplicate(fully covered), orpartial_overlap(rescores based on what we found that others missed)requiredarrays to all new schemas, renamesisReReviewtohasOwnResolvedThreadsTest plan
claude plugin validate ./pr-review-toolkitnpx markdownlint-cli2onpr-review-toolkit/**/*.md/pr-review-toolkit:review-pron a PR with no existing comments — verify all findings showstatus: "new"Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores