Port improved address-review command from react_on_rails#970
Conversation
Upgrade from basic comment-to-todos workflow to a sophisticated triage system: - Classify comments as MUST-FIX, DISCUSS, or SKIPPED before creating todos - Only create todos for MUST-FIX items (correctness bugs, regressions, security) - Resolve review threads via GitHub GraphQL after addressing comments - Better bot comment handling: don't skip by default, deduplicate and filter - Verify factual claims locally before classifying as MUST-FIX - Present triage summary to user (by category and count) Co-Authored-By: Claude Haiku 4.5 <[email protected]>
WalkthroughThis PR expands the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![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: a41e8ff016
ℹ️ 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".
| - Deduplicate overlapping comments before classifying them. Keep one representative item for the underlying issue. | ||
| - Verify factual claims locally before classifying a comment as `MUST-FIX`. | ||
| - If a claim appears wrong, classify it as `SKIPPED` and note briefly why. | ||
| - Preserve the original review comment ID and thread ID when available so the command can reply to the correct place and resolve the correct thread later. |
There was a problem hiding this comment.
This workflow now depends on resolving review threads, but the fetched comment payloads in Step 3 only keep fields like id, path, body, and line, so there is no guaranteed threadId available to satisfy this new requirement. In the common /pulls/{PR_NUMBER}/comments path, the later GraphQL resolveReviewThread call cannot be formed from the retained data, which means addressed comments will often remain unresolved even when the user expects automatic resolution.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR ports an improved Key changes:
Issues to address:
Confidence Score: 3/5
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A(["/address-review PR_URL"]) --> B[Step 1: Detect repo\ngh repo view]
B --> C[Step 2: Parse input\nPR number / URL / comment ID]
C --> D{Input type?}
D -->|issue comment ID| E[Fetch single issue comment\nREST: issues/comments/ID]
D -->|review ID| F[Fetch review comments\nREST: pulls/PR/reviews/ID/comments]
D -->|PR number only| G[Fetch all PR review comments\nREST: pulls/PR/comments\n⚠ no --paginate flag]
E & F & G --> H[Filter: skip replies\ninclude bot comments]
H --> I[Step 4: Triage comments]
I --> J{Classify each comment}
J -->|correctness / security / regression| K["MUST-FIX ✅"]
J -->|scope-expanding / architectural opinion| L["DISCUSS 💬"]
J -->|style / nit / duplicate / factually wrong| M["SKIPPED ⏭"]
K --> N[Step 5: TodoWrite\nMUST-FIX items only]
N --> O[Step 6: Present triage\nMUST-FIX / DISCUSS / SKIPPED]
L & M --> O
O --> P{User selects items}
P --> Q[Step 7: Address selected items\ncode / docs / tests]
Q --> R[Reply to original comment\ngh api POST /replies]
R --> S{Thread ID available?\n⚠ not extracted by REST queries}
S -->|Yes + concern resolved| T[Resolve thread via GraphQL\nresolveReviewThread mutation]
S -->|No / pending / leave open| U[Leave thread open]
T & U --> V([Done])
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| Use GitHub GraphQL to resolve the thread: | ||
|
|
||
| ```bash | ||
| gh api graphql -f query='mutation($threadId:ID!) { resolveReviewThread(input:{threadId:$threadId}) { thread { id isResolved } } }' -f threadId="<THREAD_ID>" |
There was a problem hiding this comment.
Thread resolution lacks instructions to obtain thread ID
Medium Severity
The thread resolution feature in Step 7 uses resolveReviewThread with a <THREAD_ID>, but there's no way to obtain this value. The REST API calls in Step 3 don't extract node_id (the jq filters exclude it), and even if they did, GitHub's REST API returns comment node IDs, not PullRequestReviewThread node IDs — those are only available via GraphQL. Step 4 says to "Preserve the original review comment ID and thread ID when available," but the data was never fetched. The feature is non-functional as described.
Additional Locations (1)
| - **ALWAYS reply to comments after addressing them** to close the feedback loop | ||
| - Resolve the review thread after replying when the concern is actually addressed and a thread ID is available | ||
| - Default to real issues only. Do not spend a review cycle on optional polish unless the user explicitly asks for it | ||
| - Triage comments before creating todos. Only `MUST-FIX` items should become todos by default |
There was a problem hiding this comment.
Pagination limitation removed without adding pagination support
Medium Severity
The "Known Limitations" section previously warned that large PRs may require pagination (not currently handled). This warning was removed, but the API call on line 51 still lacks the --paginate flag, so gh api only returns the first page (30 items by default). For PRs with more than 30 review comments, the command silently drops comments, potentially missing MUST-FIX items.
description: Fetch GitHub PR review comments, triage them, create todos for must-fix items, reply to comments, and resolve addressed threadsFetch review comments from a GitHub PR in this repository, triage them, and create a todo list only for items worth addressing. InstructionsStep 1: Determine the RepositoryREPO=$(gh repo view --json nameWithOwner -q .nameWithOwner)If this command fails, ensure Step 2: Parse User InputExtract the PR number and optional review/comment ID from the user's message: Supported formats:
URL parsing:
Step 3: Fetch Review CommentsIf a specific issue comment ID is provided ( gh api repos/${REPO}/issues/comments/{COMMENT_ID} | jq '{body: .body, user: .user.login, html_url: .html_url}'If a specific review ID is provided ( gh api repos/${REPO}/pulls/{PR_NUMBER}/reviews/{REVIEW_ID}/comments | jq '[.[] | {id: .id, path: .path, body: .body, line: .line, start_line: .start_line, user: .user.login}]'If only PR number is provided (fetch all PR review comments): gh api repos/${REPO}/pulls/{PR_NUMBER}/comments | jq '[.[] | {id: .id, path: .path, body: .body, line: .line, start_line: .start_line, user: .user.login, in_reply_to_id: .in_reply_to_id}]'Filtering comments:
Error handling:
Step 4: Triage CommentsBefore creating any todos, classify every review comment into one of three categories:
Triage rules:
Step 5: Create Todo ListCreate a todo list with TodoWrite containing only the
Step 6: Present Triage to UserPresent the triage to the user - DO NOT automatically start addressing items:
Step 7: Address Items, Reply, and ResolveWhen addressing items, after completing each selected todo item, reply to the original review comment explaining how it was addressed. For issue comments (general PR comments): gh api repos/${REPO}/issues/{PR_NUMBER}/comments -X POST -f body="<response>"For PR review comments (file-specific, replying to a thread): gh api repos/${REPO}/pulls/{PR_NUMBER}/comments/{COMMENT_ID}/replies -X POST -f body="<response>"For standalone review comments (not in a thread): gh api repos/${REPO}/pulls/{PR_NUMBER}/comments -X POST -f body="<response>" -f commit_id="<COMMIT_SHA>" -f path="<FILE_PATH>" -f line=<LINE_NUMBER> -f side="RIGHT"Note: The response should briefly explain:
After posting the reply, resolve the review thread when all of the following are true:
Use GitHub GraphQL to resolve the thread: gh api graphql -f query='mutation($threadId:ID!) { resolveReviewThread(input:{threadId:$threadId}) { thread { id isResolved } } }' -f threadId="<THREAD_ID>"Do not resolve a thread if the fix is still pending, if you are unsure whether the reviewer concern is satisfied, or if the user asked to leave the thread open. If the user explicitly asks to close out a Example UsageExample OutputAfter fetching and triaging comments, present them like this: Important Notes
Known Limitations
|
### Summary Adds the v9.7.0 changelog section with release notes for all user-visible changes since v9.6.1: - **Added**: rspack v2 support (PR #975) - **Fixed**: Config exporter path traversal and annotation format validation (PR #914) - **Fixed**: `webpack-subresource-integrity` v5 named export handling (PR #978, fixes #972) Version diff links at the bottom of the file are updated accordingly. ### Pull Request checklist - [x] ~Add/update test to cover these changes~ - [x] ~Update documentation~ - [x] Update CHANGELOG file ### Other Information Non-user-visible PRs (#920, #965, #970, #971, #977, #979, #981, #982) were intentionally excluded per changelog policy. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only change updating `CHANGELOG.md`; no runtime code or dependency changes are introduced in this PR. > > **Overview** > Adds a new `v9.7.0` section to `CHANGELOG.md` documenting user-visible changes (rspack v2 support and two fixes around config export security/validation and `webpack-subresource-integrity` v5 exports). > > Updates the compare links at the bottom so `[Unreleased]` now compares from `v9.7.0`, and adds the new `[v9.7.0]` tag link. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8942a43. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added rspack v2 support * **Bug Fixes** * Improved security and validation handling <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
### Summary Adds the v9.7.0 changelog section with release notes for all user-visible changes since v9.6.1: - **Added**: rspack v2 support (PR #975) - **Fixed**: Config exporter path traversal and annotation format validation (PR #914) - **Fixed**: `webpack-subresource-integrity` v5 named export handling (PR #978, fixes #972) Version diff links at the bottom of the file are updated accordingly. ### Pull Request checklist - [x] ~Add/update test to cover these changes~ - [x] ~Update documentation~ - [x] Update CHANGELOG file ### Other Information Non-user-visible PRs (#920, #965, #970, #971, #977, #979, #981, #982) were intentionally excluded per changelog policy. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only change updating `CHANGELOG.md`; no runtime code or dependency changes are introduced in this PR. > > **Overview** > Adds a new `v9.7.0` section to `CHANGELOG.md` documenting user-visible changes (rspack v2 support and two fixes around config export security/validation and `webpack-subresource-integrity` v5 exports). > > Updates the compare links at the bottom so `[Unreleased]` now compares from `v9.7.0`, and adds the new `[v9.7.0]` tag link. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8942a43. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added rspack v2 support * **Bug Fixes** * Improved security and validation handling <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
1 participant
Summary
Upgraded the address-review custom command with a more sophisticated workflow from react_on_rails. Instead of creating todos for all comments, the command now triages review comments as MUST-FIX (correctness issues), DISCUSS (scope-expanding suggestions), or SKIPPED (style nits and non-actionable feedback), then creates todos only for must-fix items.
Additional improvements: bot comment handling, local factual verification, thread resolution via GraphQL, and category-based user presentation.
Pull Request checklist
- [ ] Add/update test to cover these changes- [ ] Update documentation- [ ] Update CHANGELOG fileOther Information
This is an internal tooling improvement with no impact on the published gem.
Note
Low Risk
Documentation/workflow-only change to an internal Claude command; main risk is behavioral mismatch with expectations (e.g., skipping non-must-fix items or resolving threads) rather than production impact.
Overview
Updates the
.claude/address-reviewcommand to triage review comments intoMUST-FIX,DISCUSS, andSKIPPED, and to create todos only for must-fix items (with deduping and guidance to locally verify claims before escalating).Adjusts bot-handling to keep actionable bot feedback while filtering duplicates/status posts, updates the user-facing output to show the triage breakdown, and adds instructions to reply to addressed comments and resolve review threads via GraphQL when appropriate.
Written by Cursor Bugbot for commit a41e8ff. Configure here.
Summary by CodeRabbit