Add OPTIONAL triage tier and fix recommendations to /address-review#3161
Add OPTIONAL triage tier and fix recommendations to /address-review#3161
Conversation
WalkthroughThe review triage adds an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
Greptile SummaryThis PR refines the Confidence Score: 5/5Safe to merge — documentation-only change with no runtime impact, all internal logic is consistent. All findings are P2 or absent. The OPTIONAL tier, new quick actions, Recommendation requirement, and updated templates are internally consistent across every section of the file. The example output correctly reflects the new four-tier numbering, and edge cases (zero OPTIONAL items for f+o, dynamic menu visibility, range validation) are explicitly handled. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Fetch & Triage Review Comments] --> B{Classify each comment}
B -->|Correctness bugs, security,\nmissing tests| C[MUST-FIX]
B -->|Expands scope,\narchitectural opinion| D[DISCUSS]
B -->|Nits, style, docs,\nspeculative suggestions| E[OPTIONAL]
B -->|Duplicates, incorrect claims,\nstatus noise| F[SKIPPED]
C --> G[Create Todos with\nRecommendation sketch]
D --> H[Present to user\nwith reason]
E --> I[Surface for opt-in\nnever auto-address]
F --> J[List with short reason]
G & H & I & J --> K[Present Quick-Action Menu]
K --> L{User chooses}
L -->|f| M[Fix MUST-FIX\nthen prompt OPTIONAL/DISCUSS]
L -->|f+i| N[Fix MUST-FIX\ncreate follow-up issue\nfor DISCUSS+OPTIONAL]
L -->|f+o| O[Fix MUST-FIX\n+ address OPTIONAL inline]
L -->|d| P[Discuss selected items]
L -->|o| Q[Address selected\nOPTIONAL inline]
L -->|r| R[Reply with rationale\nto SKIPPED/OPTIONAL/DISCUSS]
L -->|m| S[Skip code changes\nfollow-up issue for all]
M & N & O & P & Q & R & S --> T[Post PR Summary Comment]
T --> U[Signal Merge Readiness]
Reviews (1): Last reviewed commit: "Add OPTIONAL triage tier and fix recomme..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.claude/commands/address-review.md (2)
207-207: Consider: Clarify dynamic menu logic for all actions.Line 207 states that
f+oandoshould only show when OPTIONAL items exist, and describes generating dynamic descriptions using actual item numbers forf,f+i, andf+o. However, it doesn't specify whether actions likeforf+ishould be hidden when there are no MUST-FIX items, or how to describe them when the set is empty.For example, if a scan finds only OPTIONAL items (no MUST-FIX), should the menu show:
f — (nothing to fix, skip to optional/discuss prompt)?- Or hide
fentirely and only showo,r,m?🔍 Suggested clarification
Consider adding explicit rules for empty categories:
-**Dynamic menu**: Generate `f`, `f+i`, and `f+o` descriptions dynamically using actual item numbers and deferred targets from the current triage set (e.g., "Fix `#1`, `#3`" or "Fix `#1` plus inline `#4-6`" instead of "Fix must-fix items"). Only show `f+o` and `o` when there is at least one `OPTIONAL` item. When there are no `DISCUSS`, `OPTIONAL`, or `SKIPPED` items, only show `f` and direct item selection. +**Dynamic menu**: Generate `f`, `f+i`, and `f+o` descriptions dynamically using actual item numbers and deferred targets from the current triage set (e.g., "Fix `#1`, `#3`" or "Fix `#1` plus inline `#4-6`" instead of "Fix must-fix items"). Only show `f+o` and `o` when there is at least one `OPTIONAL` item. When there are no `MUST-FIX` items, start `f` descriptions with "Skip to optional/discuss handling" instead of listing fix item numbers. When there are no `DISCUSS`, `OPTIONAL`, or `SKIPPED` items, only show `f` and direct item selection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/address-review.md at line 207, Clarify the dynamic menu rules: update the logic that builds the menu entries for actions "f", "f+i", "f+o", and "o" so that entries are only shown when they have applicable targets in the current triage set (use actual item numbers for descriptions), hide "f" and "f+i" entirely when there are no MUST-FIX items, show "f+o" and "o" only when at least one OPTIONAL item exists, and when DISCUSS/OPTIONAL/SKIPPED are all empty reduce the menu to only "f" and direct item selection; reference the triage generation code that constructs menu labels and the enums/constants OPTIONAL, MUST-FIX, DISCUSS, SKIPPED to implement these presence checks and dynamic label formatting.
127-130: Minor: Align OPTIONAL tier definitions for consistency.Lines 128 and 145 both define the OPTIONAL tier, but use slightly different wording:
- Line 128: "changelog wording" vs. Line 145: "changelog polish"
- Line 128: "comment/documentation/naming requests" vs. Line 145: "documentation/comment/naming suggestions"
While these convey the same intent, using identical phrasing in both locations would reduce ambiguity and make the tier boundaries clearer for users implementing the workflow.
📝 Proposed alignment
Pick one canonical definition and use it in both places. For example, align line 128 to match line 145:
-- Treat as optional improvements (OPTIONAL, not SKIPPED): style nits, speculative suggestions, changelog wording, test-shape preferences, comment/documentation/naming requests, and "could consider" feedback that would improve the PR but isn't a blocker +- Treat as optional improvements (OPTIONAL, not SKIPPED): nits, style preferences, documentation/comment/naming suggestions, test-shape preferences, speculative "consider" suggestions, and changelog polish that would improve the PR but isn't a blockerAlso applies to: 145-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/address-review.md around lines 127 - 130, The OPTIONAL tier definitions are worded differently in two places (one uses "changelog wording" and "comment/documentation/naming requests", the other uses "changelog polish" and "documentation/comment/naming suggestions"); make both OPTIONAL entries identical by choosing a canonical phrasing and replacing the alternate instance so the two blocks match exactly—for example, change both to: "changelog polish, documentation/comment/naming suggestions, style nits, speculative suggestions, test-shape preferences, and 'could consider' feedback" and update both OPTIONAL blocks in .claude/commands/address-review.md to that exact string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.claude/commands/address-review.md:
- Line 207: Clarify the dynamic menu rules: update the logic that builds the
menu entries for actions "f", "f+i", "f+o", and "o" so that entries are only
shown when they have applicable targets in the current triage set (use actual
item numbers for descriptions), hide "f" and "f+i" entirely when there are no
MUST-FIX items, show "f+o" and "o" only when at least one OPTIONAL item exists,
and when DISCUSS/OPTIONAL/SKIPPED are all empty reduce the menu to only "f" and
direct item selection; reference the triage generation code that constructs menu
labels and the enums/constants OPTIONAL, MUST-FIX, DISCUSS, SKIPPED to implement
these presence checks and dynamic label formatting.
- Around line 127-130: The OPTIONAL tier definitions are worded differently in
two places (one uses "changelog wording" and "comment/documentation/naming
requests", the other uses "changelog polish" and "documentation/comment/naming
suggestions"); make both OPTIONAL entries identical by choosing a canonical
phrasing and replacing the alternate instance so the two blocks match
exactly—for example, change both to: "changelog polish,
documentation/comment/naming suggestions, style nits, speculative suggestions,
test-shape preferences, and 'could consider' feedback" and update both OPTIONAL
blocks in .claude/commands/address-review.md to that exact string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a4bdb705-4d1c-4f0a-85c6-944e707d0d25
📒 Files selected for processing (1)
.claude/commands/address-review.md
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON, but it could not run because on-demand usage is turned off. To enable Bugbot Autofix, turn on on-demand usage and set a spend limit in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1ff4673. Configure here.
|
Code Review for PR 3161 - OPTIONAL tier and Recommendation sketches. See inline comments for details. |
|
Code Review: OPTIONAL tier + Recommendation sketches Overall: Well-structured improvement. Surfacing nits as OPTIONAL instead of silently dropping them to SKIPPED is the right call, and requiring a concrete Recommendation sketch per MUST-FIX item adds real value. A few issues worth addressing. Bug: OPTIONAL items block merge-ready signal (contradicts their purpose) In Action f (steps 8-9) and Step 11, OPTIONAL items gate the merge-ready signal alongside DISCUSS:
This contradicts the OPTIONAL definition: items that would genuinely improve the PR but are NOT blockers. A user choosing f should get merge-ready without explicitly declining every nit. Suggested fix: remove OPTIONAL from the merge-ready gate in f steps 8-9 and Step 11. Keep surfacing them in f step 5 for visibility, but do not require their resolution before signalling readiness. (Inline comments on the specific lines follow.) Inconsistency: f+o step 4 references f's OPTIONAL prompt f+o step 4 says to handle DISCUSS and SKIPPED items using the same prompts as f. But f step 5 is now a prompt about OPTIONAL items. In f+o, optional items are already fully addressed in step 2, so the reference to f's flow would re-present them redundantly. Should clarify: "Handle DISCUSS and SKIPPED items using f steps 6-7 (skip step 5; optional items already addressed)." Gap: Global file not updated The PR description claims both the global ~/.claude/commands/address-review.md and the project-level file are updated. The diff only modifies the project-level .claude/commands/address-review.md. The global file still uses the three-tier system. Users running /address-review outside this repository will get inconsistent triage behavior. Should be documented as a known gap or tracked as a follow-up. Minor: d action description does not mention OPTIONAL The d action description only handles DISCUSS items; declined items become SKIPPED. A user typing d3 where item 3 is OPTIONAL will get confusing behavior. Worth adding: "For OPTIONAL items, use o instead." |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
@coderabbitai — auto-generated walkthrough acknowledged, no action required. I (Claude Code) am skipping this as a non-actionable summary comment. |
|
@greptile-apps — confidence-5 summary with no findings requiring changes. I (Claude Code) am skipping this as a non-actionable summary comment. |
|
@claude — pointer-only comment ("See inline comments for details"). I (Claude Code) addressed all referenced inline review comments; no separate action needed for this post. |
|
@claude — the points in this review summary are duplicates of the inline comments on lines 224, 225, 240, 248, and 496 plus the cursor[bot] .agents/workflows sync. All six have been addressed in commit 1aa69d1 and their threads resolved. I (Claude Code) am skipping a separate reply here to avoid duplication. |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings used |
|
That classification is correct. This PR only modifies |
/address-review summary — commit 1aa69d1Scan window: default cutoff (this run covered all unresolved review feedback through commit 1ff4673). MatteredMUST-FIX (4, all addressed in 1aa69d1):
DISCUSS (1, addressed in 1aa69d1):
OPTIONAL (1, addressed inline in 1aa69d1):
Skipped
Merge readinessAll review threads resolved. PR is merge-ready. Future full-PR scans should start after this comment unless you say |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings used |
/address-review summary — commit 1aa69d1Scan window: default cutoff (this run covered all unresolved review feedback through commit 1ff4673). MatteredMUST-FIX (4, all addressed in 1aa69d1):
DISCUSS (1, addressed in 1aa69d1):
OPTIONAL (1, addressed inline in 1aa69d1):
Skipped
Merge readinessAll review threads resolved. PR is merge-ready. Future full-PR scans should start after this comment unless you say |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings used |
|
Clean summary — all 4 MUST-FIX items resolved, the DISCUSS sync addressed, and the OPTIONAL clarification for PR looks merge-ready from a review-resolution standpoint. |
|
The summary is accurate — all 6 review threads are resolved and the categorizations match what I see in the thread history. The four MUST-FIX items, one DISCUSS, and one OPTIONAL were all handled in commit PR is merge-ready. ✓ |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.claude/commands/address-review.md (1)
191-200: Avoid contradictory quick-action instructions forf+o/o.Line 191–200 presents
f+oandounconditionally, but Line 207 says those options must be shown only whenOPTIONALitems exist. Please make the menu template explicitly conditional to avoid agents showing invalid actions.Suggested doc adjustment
Quick actions: f — ... f+i — ... - f+o — Fix must-fix + address all optional items inline in the same PR d — ... - o — Address specific optional items inline (e.g., "o6,7"). Bare "o" presents all OPTIONAL items for selection. r — ... m — ... +If OPTIONAL items exist, also show: + f+o — Fix must-fix + address all optional items inline in the same PR + o — Address specific optional items inline (e.g., "o6,7"). Bare "o" presents all OPTIONAL items for selection.Also applies to: 207-207
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/address-review.md around lines 191 - 200, The quick-actions menu currently always includes the `f+o` and `o` options; update the template that renders the "Quick actions:" block so `f+o` and `o` are only displayed when there are OPTIONAL items present (e.g., check the OPTIONAL list/count before rendering those lines), and ensure any helper or conditional that builds this menu (the section producing the `f+o` / `o` entries) is gated by that same OPTIONAL existence check so agents won’t see invalid actions..agents/workflows/address-review.md (1)
130-137: Make static quick-actions consistent with dynamic visibility rules.Line 130–137 always lists
f+o/o, but Line 143 says they should appear only when optional items exist. This ambiguity can lead to invalid prompts in runs with zero optional items.Suggested doc adjustment
Quick actions: f — ... f+i — ... - f+o — Fix must-fix + address all optional items inline in the same PR d — ... - o — Address specific optional items inline (e.g., "o6,7"). Bare "o" presents all OPTIONAL items. r — ... m — ... +Only when OPTIONAL count > 0, also include: + f+o — Fix must-fix + address all optional items inline in the same PR + o — Address specific optional items inline (e.g., "o6,7"). Bare "o" presents all OPTIONAL items.Also applies to: 143-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/workflows/address-review.md around lines 130 - 137, In the "Quick actions" section the static entries 'f+o' and 'o' conflict with the dynamic visibility rule that says those actions should only be shown when optional items exist; update the content under the "Quick actions" heading to either remove 'f+o' and 'o' from the always-visible list or make their display conditional so they only appear when optional items are present, and add a short clarifying note that these options are shown only if optional review items exist (ensure you update both the static list and the other statement that references dynamic visibility so they match).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.agents/workflows/address-review.md:
- Around line 130-137: In the "Quick actions" section the static entries 'f+o'
and 'o' conflict with the dynamic visibility rule that says those actions should
only be shown when optional items exist; update the content under the "Quick
actions" heading to either remove 'f+o' and 'o' from the always-visible list or
make their display conditional so they only appear when optional items are
present, and add a short clarifying note that these options are shown only if
optional review items exist (ensure you update both the static list and the
other statement that references dynamic visibility so they match).
In @.claude/commands/address-review.md:
- Around line 191-200: The quick-actions menu currently always includes the
`f+o` and `o` options; update the template that renders the "Quick actions:"
block so `f+o` and `o` are only displayed when there are OPTIONAL items present
(e.g., check the OPTIONAL list/count before rendering those lines), and ensure
any helper or conditional that builds this menu (the section producing the `f+o`
/ `o` entries) is gated by that same OPTIONAL existence check so agents won’t
see invalid actions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b329b084-7774-4b66-b70d-78286e59a342
📒 Files selected for processing (2)
.agents/workflows/address-review.md.claude/commands/address-review.md
![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: 1aa69d1437
ℹ️ 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".
Code Review: Add OPTIONAL triage tier to /address-reviewOverviewThis PR introduces a four-tier triage model ( The Recommendation sketch requirement for Issues FoundMinor: Heading inconsistency between prose and shell script template (
|
Introduce a four-tier triage (MUST-FIX, DISCUSS, OPTIONAL, SKIPPED) so improvement-worthy nits get surfaced instead of being silently dropped, and reserve SKIPPED for duplicates, incorrect claims, and non-actionable noise. MUST-FIX todos now include a concrete Recommendation sketch verified against current code. Project variant also adds `f+o` and `o` quick actions plus OPTIONAL sections in the follow-up issue and summary comment templates. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- Remove OPTIONAL from merge-ready gates in actions f and direct-selection; OPTIONAL items are surfaced for opt-in but never block merge-readiness, matching their definition as non-blocking improvements. - Fix f+o step 4 to skip f's OPTIONAL prompt since optional items are already addressed, avoiding redundant re-prompting. - Add note to action d clarifying that OPTIONAL items use o instead. - Sync .agents/workflows/address-review.md with the OPTIONAL tier, the f+o and o quick actions, and the Recommendation sketch requirement so non-Claude-Code agents follow the same 4-tier workflow. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- Update .agents/workflows/address-review.md f+i behavior to match .claude version: for trivial skipped items excluded from the follow-up issue, ask before posting rationale replies (default no replies unless opted in) - Rename OPTIONAL section heading from "Optional improvements" to "Optional items" in shell template so it matches the prose docs and parallels "Discuss items" / "Skipped items (non-trivial)" - Tighten d and o actions in both files: reject mismatched-tier item numbers with explicit redirect messages instead of proceeding Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
justin808
force-pushed
the
jg/update-address-review
branch
from
1aa69d1 to
fb9fc64
Compare
|
@claude — rationale reply to the code-review summary comment above:
I (Claude Code) am posting this as a rationale reply rather than editing the PR further. |
/address-review summary — commit fb9fc64Scan window: default cutoff (this run covered all unresolved review feedback through commit b316a2b, after rebasing onto MatteredMUST-FIX (all addressed in commit fb9fc64):
Optional
Skipped
Notes
|
|
To use Codex here, create an environment for this repo. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.claude/commands/address-review.md (2)
217-218: Minor readability polish: vary repeated sentence openings.These list items repeatedly start with “If…”, which makes the section slightly harder to scan. Quick rewording would improve flow.
Also applies to: 232-232
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/address-review.md around lines 217 - 218, The two consecutive checklist items beginning with "If..." in .claude/commands/address-review.md are repetitive and hurt readability; reword the second sentence of the first item and/or the first sentence of the second item so they vary their openings and improve scanability while preserving meaning—locate the list block that contains the lines "Address all `MUST-FIX` items..." and "If local changes exist, commit..." and change one or both sentence starts to alternatives like "When there are local changes, commit and then ask..." or "When no local changes exist, skip commit/push..." so the items no longer both begin with "If...".
561-561: Clarify “never auto-address OPTIONAL” to avoid conflict withf+osemantics.This line can read as absolute, while
f+ointentionally auto-addresses OPTIONAL after explicit user opt-in. Consider narrowing wording to “never auto-address OPTIONAL in defaultfflow.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/address-review.md at line 561, Update the sentence that currently reads "never auto-address OPTIONAL" to clarify scope: change it to say "never auto-address OPTIONAL in the default `f` flow" (or similar) so it does not conflict with the `f+o` behavior; reference the terms `OPTIONAL`, `SKIPPED`, and the `f+o`/`f` semantics in the sentence so readers understand AUTO-addressing is only disabled for the default `f` path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.claude/commands/address-review.md:
- Around line 217-218: The two consecutive checklist items beginning with
"If..." in .claude/commands/address-review.md are repetitive and hurt
readability; reword the second sentence of the first item and/or the first
sentence of the second item so they vary their openings and improve scanability
while preserving meaning—locate the list block that contains the lines "Address
all `MUST-FIX` items..." and "If local changes exist, commit..." and change one
or both sentence starts to alternatives like "When there are local changes,
commit and then ask..." or "When no local changes exist, skip commit/push..." so
the items no longer both begin with "If...".
- Line 561: Update the sentence that currently reads "never auto-address
OPTIONAL" to clarify scope: change it to say "never auto-address OPTIONAL in the
default `f` flow" (or similar) so it does not conflict with the `f+o` behavior;
reference the terms `OPTIONAL`, `SKIPPED`, and the `f+o`/`f` semantics in the
sentence so readers understand AUTO-addressing is only disabled for the default
`f` path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 626874e2-7658-4317-bc83-da3481db979d
📒 Files selected for processing (2)
.agents/workflows/address-review.md.claude/commands/address-review.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb9fc64d41
ℹ️ 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".
| f — Fix #N, then confirm whether to reply/resolve skipped items before deciding discuss items | ||
| f+i — Fix #N, create follow-up issue for discuss/non-trivial skipped items, reply/resolve trivial skipped rest | ||
| f — Fix #N, then prompt for optional/discuss handling and skipped rationale replies | ||
| f+i — Fix #N, create follow-up issue for discuss/optional/non-trivial skipped items, reply/resolve trivial skipped rest |
There was a problem hiding this comment.
f+i skipped-thread replies opt-in in output template
The f+i quick-action text here still says to reply/resolve trivial skipped rest, but Step 8 now defines skipped replies as opt-in with a default of no reply. Because this output block is part of the prompt contract, assistants following it may post and resolve duplicate/status-noise comments automatically, which contradicts the new SKIPPED semantics and creates avoidable review churn.
Useful? React with 👍 / 👎.
Code Review: Add OPTIONAL triage tierOverall assessment: solid improvement. The 4-tier system is more expressive than the previous 3-tier model, the mandatory Recommendation on MUST-FIX items is a great UX addition, and the OPTIONAL-does-not-block-merge-readiness rule is correct design. Both files are updated consistently. What works well: Tighter SKIPPED definition - reserving SKIPPED for factually incorrect/duplicate/noise comments while promoting well-intentioned nits to OPTIONAL reduces the silent-drop problem and gives users an explicit opt-in path. Recommendation requirement on MUST-FIX - forcing the agent to read actual code before proposing a fix prevents hallucinated patches and makes todos immediately actionable. Dynamic menu with conditional f+o/o - only surfacing the optional-item actions when OPTIONAL items actually exist keeps the UI clean. OPTIONAL section in follow-up issues and PR summary - closes the information gap where this feedback was previously silently discarded. Merge-ready signal clarity - OPTIONAL items explicitly not blocking is unambiguous and correct. Issues:
|
| - **`f`**: Fix all must-fix items (if none exist, skip fix phase). If local changes exist, commit, ask for push confirmation, then push; if no local changes, skip commit/push and continue decision flow. Then reply/resolve addressed must-fix threads. If `OPTIONAL` items exist, present them and prompt me to choose: `o <nums>` to address inline, `f+i` to defer to a follow-up issue, or `r all optional + resolve` to decline and close (do not auto-address or auto-resolve optional items in `f`). If skipped items exist, ask for explicit confirmation before posting rationale replies/resolving skipped threads. Keep discuss items for an explicit follow-up decision (`d`, `f+i`, or `r all discuss + resolve`). Tell me the PR is merge-ready after `DISCUSS` items are resolved or explicitly deferred; `OPTIONAL` items do not block merge-readiness. | ||
| - **`f+i`**: Same must-fix handling as `f`, plus create a follow-up GitHub issue bundling discuss, optional, and non-trivial skipped items (in distinct sections). For trivial skipped items excluded from the follow-up issue (duplicates, factually incorrect suggestions, status noise), ask whether to post rationale replies and resolve those threads; default is no replies unless I opt in. For general PR comments and review summary bodies (which have no thread), the reply alone is sufficient. If there are no deferred items, skip issue creation and behave like `f`. No additional commit is needed unless later steps introduce local changes. | ||
| - **`f+o`**: Same must-fix handling as `f`, plus address all `OPTIONAL` items inline in the same PR (make the code change, reply, resolve each thread). If optional fixes require a separate commit to keep the must-fix commit atomic, commit them and ask for push confirmation before pushing the additional commit. Then handle `DISCUSS` and `SKIPPED` items using `f`'s prompts for those tiers (skip the optional-items prompt; optional is already done). Tell me the PR is merge-ready once all selected work is pushed and `DISCUSS` items are resolved or explicitly deferred. If there are zero `OPTIONAL` items, behave like `f` and note that `f+o` had nothing additional to do. | ||
| - **`d`**: Present requested items with full context, ask for a decision on each. Bare `d` presents all DISCUSS items. Approved → fix like must-fix (use the same commit/push-before-reply ordering as `f` when code changes occur). Declined → optionally reply with rationale. Note: `d` only accepts `DISCUSS` item numbers. If any selected number refers to an `OPTIONAL`, `MUST-FIX`, or `SKIPPED` item, do not proceed — respond with "Item N is {tier} — use `{o|f|r}` instead" for each mismatched number and ask for a corrected selection. |
There was a problem hiding this comment.
The {o|f|r} placeholder is ambiguous — an LLM may render it literally rather than resolve it to the single correct action for the mismatched tier. The right action is deterministic: OPTIONAL → o, MUST-FIX → f, SKIPPED → r. Suggest making the guidance explicit:
| - **`d`**: Present requested items with full context, ask for a decision on each. Bare `d` presents all DISCUSS items. Approved → fix like must-fix (use the same commit/push-before-reply ordering as `f` when code changes occur). Declined → optionally reply with rationale. Note: `d` only accepts `DISCUSS` item numbers. If any selected number refers to an `OPTIONAL`, `MUST-FIX`, or `SKIPPED` item, do not proceed — respond with "Item N is {tier} — use `{o|f|r}` instead" for each mismatched number and ask for a corrected selection. | |
| - **`d`**: Present requested items with full context, ask for a decision on each. Bare `d` presents all DISCUSS items. Approved → fix like must-fix (use the same commit/push-before-reply ordering as `f` when code changes occur). Declined → optionally reply with rationale. Note: `d` only accepts `DISCUSS` item numbers. If any selected number refers to a different tier, do not proceed — respond with "Item N is OPTIONAL — use `o` instead", "Item N is MUST-FIX — use `f` instead", or "Item N is SKIPPED — use `r` instead" as appropriate, and ask for a corrected selection. |
| - **`f+i`**: Same must-fix handling as `f`, plus create a follow-up GitHub issue bundling discuss, optional, and non-trivial skipped items (in distinct sections). For trivial skipped items excluded from the follow-up issue (duplicates, factually incorrect suggestions, status noise), ask whether to post rationale replies and resolve those threads; default is no replies unless I opt in. For general PR comments and review summary bodies (which have no thread), the reply alone is sufficient. If there are no deferred items, skip issue creation and behave like `f`. No additional commit is needed unless later steps introduce local changes. | ||
| - **`f+o`**: Same must-fix handling as `f`, plus address all `OPTIONAL` items inline in the same PR (make the code change, reply, resolve each thread). If optional fixes require a separate commit to keep the must-fix commit atomic, commit them and ask for push confirmation before pushing the additional commit. Then handle `DISCUSS` and `SKIPPED` items using `f`'s prompts for those tiers (skip the optional-items prompt; optional is already done). Tell me the PR is merge-ready once all selected work is pushed and `DISCUSS` items are resolved or explicitly deferred. If there are zero `OPTIONAL` items, behave like `f` and note that `f+o` had nothing additional to do. | ||
| - **`d`**: Present requested items with full context, ask for a decision on each. Bare `d` presents all DISCUSS items. Approved → fix like must-fix (use the same commit/push-before-reply ordering as `f` when code changes occur). Declined → optionally reply with rationale. Note: `d` only accepts `DISCUSS` item numbers. If any selected number refers to an `OPTIONAL`, `MUST-FIX`, or `SKIPPED` item, do not proceed — respond with "Item N is {tier} — use `{o|f|r}` instead" for each mismatched number and ask for a corrected selection. | ||
| - **`o`**: Present requested items with full context. Bare `o` presents all `OPTIONAL` items for selection. For each selected optional item, treat it the same as a must-fix: make the code change, run relevant checks, reply, and resolve the thread. Use the same commit/push-before-reply ordering as `f`. For optional items I decline, offer a rationale reply via `r <nums>`. Note: `o` only accepts `OPTIONAL` item numbers. If any selected number refers to a `DISCUSS`, `MUST-FIX`, or `SKIPPED` item, do not proceed — respond with "Item N is {tier} — use `{d|f|r}` instead" for each mismatched number and ask for a corrected selection. |
There was a problem hiding this comment.
Same issue as the d action above — {d|f|r} is ambiguous. For o, the tier-to-action mapping is: DISCUSS → d, MUST-FIX → f, SKIPPED → r.
| - **`o`**: Present requested items with full context. Bare `o` presents all `OPTIONAL` items for selection. For each selected optional item, treat it the same as a must-fix: make the code change, run relevant checks, reply, and resolve the thread. Use the same commit/push-before-reply ordering as `f`. For optional items I decline, offer a rationale reply via `r <nums>`. Note: `o` only accepts `OPTIONAL` item numbers. If any selected number refers to a `DISCUSS`, `MUST-FIX`, or `SKIPPED` item, do not proceed — respond with "Item N is {tier} — use `{d|f|r}` instead" for each mismatched number and ask for a corrected selection. | |
| - **`o`**: Present requested items with full context. Bare `o` presents all `OPTIONAL` items for selection. For each selected optional item, treat it the same as a must-fix: make the code change, run relevant checks, reply, and resolve the thread. Use the same commit/push-before-reply ordering as `f`. For optional items I decline, offer a rationale reply via `r <nums>`. Note: `o` only accepts `OPTIONAL` item numbers. If any selected number refers to a different tier, do not proceed — respond with "Item N is DISCUSS — use `d` instead", "Item N is MUST-FIX — use `f` instead", or "Item N is SKIPPED — use `r` instead" as appropriate, and ask for a corrected selection. |
| ### Action `d` — Discuss items | ||
|
|
||
| Present the requested items with full context and ask the user for a decision on each. If the user enters bare `d` with no item numbers, present all `DISCUSS` items. After the user decides, treat approved items as `MUST-FIX` (fix, reply, resolve) and declined items as `SKIPPED` (optionally reply with rationale if the user asks). For approved items that produce local changes, use the same commit/push-before-reply ordering as action `f`. After handling requested `d` items, re-offer the quick-action menu for remaining unaddressed items. | ||
| Present the requested items with full context and ask the user for a decision on each. If the user enters bare `d` with no item numbers, present all `DISCUSS` items. After the user decides, treat approved items as `MUST-FIX` (fix, reply, resolve) and declined items as `SKIPPED` (optionally reply with rationale if the user asks). For approved items that produce local changes, use the same commit/push-before-reply ordering as action `f`. After handling requested `d` items, re-offer the quick-action menu for remaining unaddressed items. Note: `d` only accepts `DISCUSS` item numbers. If any selected number refers to an `OPTIONAL`, `MUST-FIX`, or `SKIPPED` item, do not proceed — respond with "Item N is {tier} — use `{o|f|r}` instead" for each mismatched number and ask for a corrected selection. |
There was a problem hiding this comment.
Same ambiguity as in the workflow file: {o|f|r} is a template placeholder that an LLM may render literally. Since the right action per tier is deterministic, spell it out:
| Present the requested items with full context and ask the user for a decision on each. If the user enters bare `d` with no item numbers, present all `DISCUSS` items. After the user decides, treat approved items as `MUST-FIX` (fix, reply, resolve) and declined items as `SKIPPED` (optionally reply with rationale if the user asks). For approved items that produce local changes, use the same commit/push-before-reply ordering as action `f`. After handling requested `d` items, re-offer the quick-action menu for remaining unaddressed items. Note: `d` only accepts `DISCUSS` item numbers. If any selected number refers to an `OPTIONAL`, `MUST-FIX`, or `SKIPPED` item, do not proceed — respond with "Item N is {tier} — use `{o|f|r}` instead" for each mismatched number and ask for a corrected selection. | |
| Present the requested items with full context and ask the user for a decision on each. If the user enters bare `d` with no item numbers, present all `DISCUSS` items. After the user decides, treat approved items as `MUST-FIX` (fix, reply, resolve) and declined items as `SKIPPED` (optionally reply with rationale if the user asks). For approved items that produce local changes, use the same commit/push-before-reply ordering as action `f`. After handling requested `d` items, re-offer the quick-action menu for remaining unaddressed items. Note: `d` only accepts `DISCUSS` item numbers. If any selected number refers to a different tier, do not proceed — respond with "Item N is OPTIONAL — use `o` instead", "Item N is MUST-FIX — use `f` instead", or "Item N is SKIPPED — use `r` instead" as appropriate, and ask for a corrected selection. |
|
|
||
| ### Action `o` — Address optional items inline | ||
|
|
||
| Present the requested items with full context. If the user enters bare `o` with no item numbers, present all `OPTIONAL` items for selection. For each selected optional item, treat it the same as a `MUST-FIX`: make the code change, run relevant checks, reply, and resolve the thread. Use the same commit/push-before-reply ordering as action `f`. For optional items the user declines, offer a rationale reply via `r <nums>`. After handling requested `o` items, re-offer the quick-action menu for remaining unaddressed items. Note: `o` only accepts `OPTIONAL` item numbers. If any selected number refers to a `DISCUSS`, `MUST-FIX`, or `SKIPPED` item, do not proceed — respond with "Item N is {tier} — use `{d|f|r}` instead" for each mismatched number and ask for a corrected selection. |
There was a problem hiding this comment.
Same issue — {d|f|r} should be spelled out per tier for the o action (DISCUSS → d, MUST-FIX → f, SKIPPED → r):
| Present the requested items with full context. If the user enters bare `o` with no item numbers, present all `OPTIONAL` items for selection. For each selected optional item, treat it the same as a `MUST-FIX`: make the code change, run relevant checks, reply, and resolve the thread. Use the same commit/push-before-reply ordering as action `f`. For optional items the user declines, offer a rationale reply via `r <nums>`. After handling requested `o` items, re-offer the quick-action menu for remaining unaddressed items. Note: `o` only accepts `OPTIONAL` item numbers. If any selected number refers to a `DISCUSS`, `MUST-FIX`, or `SKIPPED` item, do not proceed — respond with "Item N is {tier} — use `{d|f|r}` instead" for each mismatched number and ask for a corrected selection. | |
| Present the requested items with full context. If the user enters bare `o` with no item numbers, present all `OPTIONAL` items for selection. For each selected optional item, treat it the same as a `MUST-FIX`: make the code change, run relevant checks, reply, and resolve the thread. Use the same commit/push-before-reply ordering as action `f`. For optional items the user declines, offer a rationale reply via `r <nums>`. After handling requested `o` items, re-offer the quick-action menu for remaining unaddressed items. Note: `o` only accepts `OPTIONAL` item numbers. If any selected number refers to a different tier, do not proceed — respond with "Item N is DISCUSS — use `d` instead", "Item N is MUST-FIX — use `f` instead", or "Item N is SKIPPED — use `r` instead" as appropriate, and ask for a corrected selection. |
| 1. Address all `MUST-FIX` items as in `f` (self-review gate, commit, push confirmation). | ||
| 2. Address all `OPTIONAL` items inline in the same PR: make the code change, reply, and resolve the thread for each. Treat each optional item the same as a selected must-fix once `f+o` is chosen. | ||
| 3. If optional fixes require a separate commit (e.g., to keep the must-fix commit atomic), commit them and ask for push confirmation before pushing the additional commit. | ||
| 4. Handle `DISCUSS` and `SKIPPED` items using `f` steps 6–7 (skip step 5; optional items are already addressed above). |
There was a problem hiding this comment.
This numbered cross-reference to Action f is fragile — if f's steps are renumbered in a future edit, this silently becomes wrong. Consider a description-based reference instead:
| 4. Handle `DISCUSS` and `SKIPPED` items using `f` steps 6–7 (skip step 5; optional items are already addressed above). | |
| 4. Handle `DISCUSS` and `SKIPPED` items the same way as in action `f`: prompt for explicit confirmation before posting skipped-item rationale replies, then prompt for a DISCUSS decision (`d`, `f+i`, or `r all discuss + resolve`). Skip the OPTIONAL-items prompt since optional work is already addressed above. |
…3161) ### Summary Updates the `/address-review` slash command (both the global `~/.claude/commands/address-review.md` and the project-level `.claude/commands/address-review.md`) so reviewer feedback gets triaged into four tiers instead of three: `MUST-FIX`, `DISCUSS`, `OPTIONAL`, and `SKIPPED`. The new `OPTIONAL` tier captures well-intentioned nits and improvement suggestions that used to be silently dropped into `SKIPPED`, which is now reserved for duplicates, factually incorrect claims, and non-actionable noise. `MUST-FIX` todos now carry a concrete `Recommendation` sketch (file/line, code snippet, or approach) verified against the current code — not a guessed patch. The project variant also adds `f+o` (fix must-fix + address optional inline) and `o` (address selected optional items) quick actions, plus `OPTIONAL` sections in the follow-up issue template and PR summary comment. ### Pull Request checklist - ~[ ] Add/update test to cover these changes~ (slash-command markdown, no runtime tests) - ~[ ] Update documentation~ (this PR is the documentation) - ~[ ] Update CHANGELOG file~ (tooling-only change, not user-facing) ### Other Information No runtime code is affected; this only changes Claude Code command instructions under `.claude/commands/`. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: documentation/workflow instruction changes only, with no runtime code paths affected. Main risk is behavioral confusion if the new `OPTIONAL` tier/action prompts are misapplied by users or agents. > > **Overview** > **Expands `/address-review` triage from 3 tiers to 4** by introducing `OPTIONAL` for non-blocking improvements and narrowing `SKIPPED` to duplicates/incorrect/noise, across both the reusable workflow prompt and the Claude command. > > **Requires `MUST-FIX` items to include a concrete `Recommendation` fix sketch**, and updates the displayed triage output and quick-action menu to include optional selection, range support, and dynamic menu text. > > **Adds explicit handling for optional work in the action flows** (new `o` and `f+o`, optional sections in follow-up issues and PR summary checkpoints, and updated merge-ready signaling where `OPTIONAL` never blocks merge readiness). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit fb9fc64. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an OPTIONAL review tier for non-blocking improvements, shown in the UI and selection menu. * New actions to address optional items inline (o) and combined fix+optional (f+o). * Must-fix todos now include a concrete, verified Recommendation. * **Changes** * Quick-action menu, selection/range syntax and conditional options updated to surface OPTIONAL. * Follow-up issues and PR summaries now include Optional sections; OPTIONAL no longer blocks merge readiness. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
…ging' into jg/3122-rolling-deploy-adapter * origin/jg/3122-unify-renderer-cache-staging: (39 commits) fix(specs): boot dummy specs without readline and drop redundant pnpm workspace (#3190) docs: add RSC migration success stories page (#1985) (#3162) Fix Bencher reporting permanently broken on pushes to main (#3148) docs: add example migrations guide (#3126) docs: remove defunct guavapass.com reference (#3199) chore: remove redundant --rsc-pro install generator flag (#3105) ci: warn (don't fail) on Bencher main regression (#3168) test: enable RSpec --profile to surface slowest package tests (#3176) fix(node-renderer): expose performance in VM context when supportModules (#3158) docs: remove stale immediate_hydration references (#3139) (#3159) docs: restore absolute URL for node-renderer testing example (#3179) Bump Rspack dependencies to v2 (^2.0.0-0) (#3084) chore: remove obsolete webpack <5.106.0 pin (#3175) Move Node Renderer entry point to renderer/ directory (#3165) docs: address RSC pitfalls review follow-ups (#3155) (#3156) docs: remove fabricated DevConsole reference, link verified RSC tools (#2527) (#3163) Scaffold CI workflow and build scripts for first-run consistency (#3097) Add OPTIONAL triage tier and fix recommendations to /address-review (#3161) chore: sync Gemfile.lock with term-ansicolor 1.11.3 (#3164) Simplify the docs sidebar and Pro landing pages (#3119) ...
1 participant
Summary
Updates the
/address-reviewslash command (both the global~/.claude/commands/address-review.mdand the project-level.claude/commands/address-review.md) so reviewer feedback gets triaged into four tiers instead of three:MUST-FIX,DISCUSS,OPTIONAL, andSKIPPED. The newOPTIONALtier captures well-intentioned nits and improvement suggestions that used to be silently dropped intoSKIPPED, which is now reserved for duplicates, factually incorrect claims, and non-actionable noise.MUST-FIXtodos now carry a concreteRecommendationsketch (file/line, code snippet, or approach) verified against the current code — not a guessed patch. The project variant also addsf+o(fix must-fix + address optional inline) ando(address selected optional items) quick actions, plusOPTIONALsections in the follow-up issue template and PR summary comment.Pull Request checklist
[ ] Add/update test to cover these changes(slash-command markdown, no runtime tests)[ ] Update documentation(this PR is the documentation)[ ] Update CHANGELOG file(tooling-only change, not user-facing)Other Information
No runtime code is affected; this only changes Claude Code command instructions under
.claude/commands/.Note
Low Risk
Low risk: documentation/workflow instruction changes only, with no runtime code paths affected. Main risk is behavioral confusion if the new
OPTIONALtier/action prompts are misapplied by users or agents.Overview
Expands
/address-reviewtriage from 3 tiers to 4 by introducingOPTIONALfor non-blocking improvements and narrowingSKIPPEDto duplicates/incorrect/noise, across both the reusable workflow prompt and the Claude command.Requires
MUST-FIXitems to include a concreteRecommendationfix sketch, and updates the displayed triage output and quick-action menu to include optional selection, range support, and dynamic menu text.Adds explicit handling for optional work in the action flows (new
oandf+o, optional sections in follow-up issues and PR summary checkpoints, and updated merge-ready signaling whereOPTIONALnever blocks merge readiness).Reviewed by Cursor Bugbot for commit fb9fc64. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Changes