You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Port improved address-review command from react_on_rails (#970)
### 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 file~
### Other Information
This is an internal tooling improvement with no impact on the published
gem.
<!-- CURSOR_SUMMARY -->
---
> [!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-review` command to **triage review
comments** into `MUST-FIX`, `DISCUSS`, and `SKIPPED`, 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.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
a41e8ff. 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
* **Documentation**
* Updated PR review addressing workflow with new triage system
categorizing review comments (MUST-FIX, DISCUSS, SKIPPED)
* Enhanced process for addressing items, replying to comments, and
resolving review threads
* Improved comment filtering and deduplication logic
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Claude Haiku 4.5 <[email protected]>
- Skip comments where `in_reply_to_id` is set (these are replies, not top-level comments)
57
-
- Skip bot-generated comments (check if `user` ends with `[bot]`)
57
+
- Do not skip bot-generated comments by default. Many actionable review comments in this repository come from bots.
58
+
- Deduplicate repeated bot comments and skip bot status posts, summaries, and acknowledgments that do not require a code or documentation change
59
+
- Treat as actionable by default only: correctness bugs, regressions, missing tests, and clear inconsistencies with adjacent code
60
+
- Treat as non-actionable by default: style nits, speculative suggestions, changelog wording, duplicate bot comments, and "could consider" feedback unless the user explicitly asks for polish work
58
61
- Focus on actionable feedback, not acknowledgments or thank-you messages
- If the API returns 403, check authentication with `gh auth status`
64
67
- If the response is empty, inform the user no review comments were found
65
68
66
-
## Step 4: Create Todo List
69
+
## Step 4: Triage Comments
67
70
68
-
Parse the response and create a todo list with TodoWrite containing:
71
+
Before creating any todos, classify every review comment into one of three categories:
69
72
70
-
- One todo per actionable review comment/suggestion
73
+
-`MUST-FIX`: correctness bugs, regressions, security issues, missing tests that could hide a real bug, and clear inconsistencies with adjacent code that would likely block merge
74
+
-`DISCUSS`: reasonable suggestions that expand scope, architectural opinions that are not clearly right or wrong, and comments where the reviewer claim may be correct but needs a user decision
- Deduplicate overlapping comments before classifying them. Keep one representative item for the underlying issue.
80
+
- Verify factual claims locally before classifying a comment as `MUST-FIX`.
81
+
- If a claim appears wrong, classify it as `SKIPPED` and note briefly why.
82
+
- 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.
83
+
84
+
## Step 5: Create Todo List
85
+
86
+
Create a todo list with TodoWrite containing **only the `MUST-FIX` items**:
87
+
88
+
- One todo per must-fix comment or deduplicated issue
71
89
- For file-specific comments: `"{file}:{line} - {comment_summary} (@{username})"` (content)
72
-
- For general comments: Parse the comment body and extract actionable items
90
+
- For general comments: Parse the comment body and extract the must-fix action
73
91
- Format activeForm: `"Addressing {brief description}"`
74
92
- All todos should start with status: `"pending"`
75
93
76
-
## Step 5: Present to User
94
+
## Step 6: Present Triage to User
77
95
78
-
Present the todos to the user - **DO NOT automatically start addressing them**:
96
+
Present the triage to the user - **DO NOT automatically start addressing items**:
79
97
80
-
- Show a summary of how many actionable items were found
81
-
- List the todos clearly
82
-
- Wait for the user to tell you which ones to address
98
+
-`MUST-FIX ({count})`: list the todos created
99
+
-`DISCUSS ({count})`: list items needing user choice, with a short reason
100
+
-`SKIPPED ({count})`: list skipped comments with a short reason, including duplicates and factually incorrect suggestions
101
+
- Wait for the user to tell you which items to address
102
+
- If useful, suggest replying with a brief rationale on declined items, but do not do so unless the user asks
83
103
84
-
## Step 6: Address Itemsand Reply
104
+
## Step 7: Address Items, Reply, and Resolve
85
105
86
-
When addressing items, after completing each todo item, reply to the original review comment explaining how it was addressed.
106
+
When addressing items, after completing each selected todo item, reply to the original review comment explaining how it was addressed.
87
107
88
108
**For issue comments (general PR comments):**
89
109
@@ -111,6 +131,22 @@ The response should briefly explain:
111
131
- Which commit(s) contain the fix
112
132
- Any relevant details or decisions made
113
133
134
+
After posting the reply, resolve the review thread when all of the following are true:
135
+
136
+
- The comment belongs to a review thread and you have the thread ID
137
+
- The concern was actually addressed in code, tests, or documentation, or it was explicitly declined with a clear explanation approved by the user
138
+
- The thread is not already resolved
139
+
140
+
Use GitHub GraphQL to resolve the thread:
141
+
142
+
```bash
143
+
gh api graphql -f query='mutation($threadId:ID!) { resolveReviewThread(input:{threadId:$threadId}) { thread { id isResolved } } }' -f threadId="<THREAD_ID>"
144
+
```
145
+
146
+
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.
147
+
148
+
If the user explicitly asks to close out a `DISCUSS` or `SKIPPED` item, reply with the rationale and resolve the thread only when the conversation is actually complete.
149
+
114
150
# Example Usage
115
151
116
152
```text
@@ -122,16 +158,24 @@ The response should briefly explain:
122
158
123
159
# Example Output
124
160
125
-
After fetching comments, present them like this:
161
+
After fetching and triaging comments, present them like this:
126
162
127
163
```text
128
-
Found 3 actionable review comments:
164
+
Found 5 review comments. Triage:
165
+
166
+
MUST-FIX (1):
167
+
1. ⬜ src/helper.rb:45 - Missing nil guard causes a crash on empty input (@reviewer1)
168
+
169
+
DISCUSS (1):
170
+
2. src/config.rb:12 - Extract this to a shared config constant (@reviewer1)
171
+
Reason: reasonable suggestion, but it expands scope
129
172
130
-
1. ⬜ src/helper.rb:45 - Add error handling for nil case (@reviewer1)
131
-
2. ⬜ src/config.rb:12 - Consider using constant instead of magic number (@reviewer1)
132
-
3. ⬜ General comment - Update documentation to reflect API changes (@reviewer2)
0 commit comments