Sync address-review command with react_on_rails upstream#22
Sync address-review command with react_on_rails upstream#22
Conversation
Brings over significant upstream improvements from
shakacode/react_on_rails `.claude/commands/address-review.md`:
- Summary cutoff system using `<!-- address-review-summary -->` markers
for incremental review passes; default to only new feedback
- `check all reviews` override to rescan full PR history
- Quick-action menu (f / f+i / d / r / m) with range syntax (e.g. 1,3-5)
replacing free-text item selection
- Follow-up GitHub issue creation (Step 9) bundling deferred items
- Consolidated PR summary checkpoint comment posted after each action
(Step 10) to anchor the next cutoff
- Explicit merge-ready signal (Step 11)
- Fetch standalone review summary bodies from /pulls/{N}/reviews
- Timestamp-based filtering via created_at/submitted_at
- Require push confirmation before running git push
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughUpdates Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Workflow
participant GitHub
participant IssueTracker
User->>Workflow: Invoke /address-review (optional full repo URL)
Workflow->>GitHub: Parse repo, fetch PR, fetch latest <!-- address-review-summary --> comment
GitHub-->>Workflow: return prior checkpoint (if any) with timestamp
Workflow->>GitHub: fetch reviews, review comments, issue comments (include created_at/submitted_at)
GitHub-->>Workflow: return review & comment metadata
Workflow->>Workflow: apply REVIEW_CUTOFF_AT (unless check-all override), filter/triage items
Workflow->>User: present quick-action menu (f, f+i, d, r, m) and item ranges
User->>Workflow: choose actions / chains
Workflow->>GitHub: perform actions (post comments, resolve threads, anchor replies)
alt follow-up needed
Workflow->>IssueTracker: create follow-up issue (bundle selected items)
IssueTracker-->>Workflow: return follow-up URL
end
Workflow->>GitHub: post exactly one `<!-- address-review-summary -->` PR comment with outcomes and follow-up URL
GitHub-->>User: visible PR summary checkpoint
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 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Greptile SummaryThis PR syncs
Confidence Score: 4/5Safe to merge after fixing the gh api --input JSON encoding bug in Step 10; everything else is additive and well-structured. One P1 bug: the summary comment POST in Step 10 will fail because raw markdown is sent where JSON is required, breaking the entire cutoff mechanism for future runs. The rest of the additions are solid and faithfully replicate upstream intent. commands/address-review.md — line 399 (gh api --input encoding bug) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["/address-review input"] --> B{Contains 'check all reviews'?}
B -- yes --> C[Set CHECK_ALL_REVIEWS flag]
B -- no --> D[Parse PR number / URL / anchor]
C --> D
D --> E{Anchor type?}
E -- issuecomment --> F[Fetch single issue comment]
E -- pullrequestreview --> G[Fetch review + inline comments]
E -- none/PR only --> H[Fetch latest summary comment]
H --> I{Summary found AND not CHECK_ALL_REVIEWS?}
I -- yes --> J[Set REVIEW_CUTOFF_AT]
I -- no --> K[Full history scan]
J --> L[Fetch all reviews + comments]
K --> L
F --> M[Triage Comments]
G --> M
L --> M
M --> N[Present triage + Quick-action menu]
N --> O{User action}
O -- f --> P[Fix MUST-FIX, commit, push confirm, reply, resolve]
O -- f+i --> Q[Fix MUST-FIX + create follow-up issue]
O -- d --> R[Discuss selected items]
O -- r --> S[Post rationale replies]
O -- m --> T[Create follow-up issue, defer all]
P --> U[Post PR summary comment with marker]
Q --> U
R --> U
S --> U
T --> U
U --> V[Merge-ready signal]
Reviews (1): Last reviewed commit: "Sync address-review command with react_o..." | Re-trigger Greptile |
Code Review: Sync address-review command with react_on_rails upstreamOverviewThis PR substantially expands Bug (must fix)Step 10 —
# Replace:
gh api repos/${REPO}/issues/${PR_NUMBER}/comments -X POST --input "${summary_body_file}"
# With:
gh pr comment "${PR_NUMBER}" --repo "${REPO}" --body-file "${summary_body_file}"(Step 9's Minor issuesPlaceholder vs. shell-variable inconsistency (see inline comment on line 57) Throughout the file,
printf -v DISCUSS_SECTION '### Discuss items\n%s\n' "${DISCUSS_ITEMS}"
Strengths
SummaryOne concrete bug (Step 10 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
commands/address-review.md (2)
57-58: Use one placeholder convention in command snippets.The doc mixes
{PR_NUMBER}and${PR_NUMBER}styles in executable snippets. Standardizing to shell variable form in runnable examples will reduce copy/paste errors during execution.Proposed normalization example
-gh api --paginate repos/${REPO}/issues/{PR_NUMBER}/comments | jq -s '...' +gh api --paginate repos/${REPO}/issues/${PR_NUMBER}/comments | jq -s '...' -gh api repos/${REPO}/issues/comments/{COMMENT_ID} | jq '{...}' +gh api repos/${REPO}/issues/comments/${COMMENT_ID} | jq '{...}'Also applies to: 73-74, 80-84, 92-99, 255-263, 271-273, 399-400
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commands/address-review.md` around lines 57 - 58, The command snippets in commands/address-review.md mix two placeholder conventions ({PR_NUMBER} vs ${PR_NUMBER}); update all executable examples (including the snippet showing gh api --paginate repos/${REPO}/issues/{PR_NUMBER}/comments and the other occurrences around the ranges noted) to use the shell variable form ${PR_NUMBER} (and ${REPO} where applicable) for consistency and to make snippets directly runnable; search for occurrences of "{PR_NUMBER}" and replace them with "${PR_NUMBER}" (and likewise normalize any "{REPO}" to "${REPO}") so all examples use the same shell variable convention.
199-201: Minor readability polish: reduce repeated “If …” sentence starts.LanguageTool is right here; a quick reword improves flow in procedural steps without changing meaning.
Also applies to: 213-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commands/address-review.md` around lines 199 - 201, Revise the three procedural bullets (the numbered steps shown as items 1–3 in the snippet) to remove repetitive sentence-initial "If ..." phrasing by merging conditional clauses or rephrasing to lead with the action, e.g., convert "If there are no X, skip Y" patterns into "Skip Y when there are no X" or combine conditions and actions into single sentences; apply the same rewrite pattern to the similar block referenced (items covering lines 213–215) so the steps read more smoothly while preserving exact meaning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@commands/address-review.md`:
- Around line 369-370: The current sentence ambiguously says "after any chosen
action or completed action chain" and may trigger posting the consolidated PR
comment after intermediate steps (e.g., after `d` before `f+i`), so update the
wording to state that the consolidated PR comment (which sets the next default
review cutoff) must be posted only once at the final stable stopping point of
the current run — i.e., after the user completes the full action or action chain
(`f`, `f+i`, `d`, `r`, `m`, or direct item selection), not after intermediate or
partial actions; revise the sentence to explicitly require posting at the end of
the complete flow.
---
Nitpick comments:
In `@commands/address-review.md`:
- Around line 57-58: The command snippets in commands/address-review.md mix two
placeholder conventions ({PR_NUMBER} vs ${PR_NUMBER}); update all executable
examples (including the snippet showing gh api --paginate
repos/${REPO}/issues/{PR_NUMBER}/comments and the other occurrences around the
ranges noted) to use the shell variable form ${PR_NUMBER} (and ${REPO} where
applicable) for consistency and to make snippets directly runnable; search for
occurrences of "{PR_NUMBER}" and replace them with "${PR_NUMBER}" (and likewise
normalize any "{REPO}" to "${REPO}") so all examples use the same shell variable
convention.
- Around line 199-201: Revise the three procedural bullets (the numbered steps
shown as items 1–3 in the snippet) to remove repetitive sentence-initial "If
..." phrasing by merging conditional clauses or rephrasing to lead with the
action, e.g., convert "If there are no X, skip Y" patterns into "Skip Y when
there are no X" or combine conditions and actions into single sentences; apply
the same rewrite pattern to the similar block referenced (items covering lines
213–215) so the steps read more smoothly while preserving exact meaning.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb1f2436-ce88-4d70-b5e8-ad5599fcdbd3
📒 Files selected for processing (1)
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: fc442cc459
ℹ️ 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".
![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 2 potential issues.
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 fc442cc. Configure here.
- Step 10: replace `gh api --input` (which sends raw markdown) with `gh pr comment --body-file`; the API expected JSON, so the previous call would 422 and break the cutoff mechanism.
- Step 4: split the ambiguous "created after" cutoff bullet into explicit per-type rules — issue comments and inline threads use `created_at`; review summaries use `submitted_at`.
- Normalize shell snippets to use `${PR_NUMBER}`/`${REVIEW_ID}`/`${COMMENT_ID}`/`${THREAD_ID}` consistently so examples are directly runnable.
- Step 10 timing: require posting the checkpoint only at the final stable stopping point, not after intermediate steps in a chain.
- Readability polish on Action `f`/`f+i` steps.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code ReviewThis is a substantial, well-structured sync from the upstream What's good
Issues and suggestions1. The cutoff rules say "If no summary comment exists, scan the full PR history" but never say 2. Step 1 code block executes unconditionally The prose correctly says "Otherwise, detect the repository from the current checkout" — but the code block below shows # Only if REPO was not already extracted from a URL:
REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner)3. Step 8 correctly forbids 4.
5. Missing The spec says "detect the exact phrase Minor nits
SummaryThis is a solid sync. The four substantive issues above are the most worth addressing before merge: the |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
commands/address-review.md (1)
214-214: Optional wording polish for readability.This section has repetitive sentence openings (“For …”). Consider varying one sentence start to improve scanability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@commands/address-review.md` at line 214, The bulleted guideline starting with "For trivial `SKIPPED` items that are not included in the follow-up issue..." repeats the "For ..." opening and hurts scanability; rephrase one of the repeated sentences in this section (e.g., change the opening of the fourth bullet or the neighboring sentence that also begins with "For ...") to vary the sentence start while preserving meaning—target the sentence that contains "For trivial `SKIPPED` items that are not included in the follow-up issue (duplicates, factually incorrect suggestions, status noise), still post rationale replies and resolve those threads." and rewrite it to something like "When an item is a trivial `SKIPPED` case (duplicates, factually incorrect suggestions, or status noise), post a rationale reply and resolve the thread." to improve readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@commands/address-review.md`:
- Around line 92-93: The jq filter used in the gh api command that builds review
objects (the pipeline starting with gh api --paginate ... | jq -s '[.[].[] |
select((.body // "") != "") | {id: .id, type: "review_summary", body: .body,
state: .state, user: .user.login, submitted_at: .submitted_at, html_url:
.html_url}]') should exclude non-submitted reviews: update the select to also
require reviews be submitted (e.g. .submitted_at != null) and not in PENDING
state (e.g. .state != "PENDING") before mapping into the review_summary object
so only actionable, submitted reviews are included.
---
Nitpick comments:
In `@commands/address-review.md`:
- Line 214: The bulleted guideline starting with "For trivial `SKIPPED` items
that are not included in the follow-up issue..." repeats the "For ..." opening
and hurts scanability; rephrase one of the repeated sentences in this section
(e.g., change the opening of the fourth bullet or the neighboring sentence that
also begins with "For ...") to vary the sentence start while preserving
meaning—target the sentence that contains "For trivial `SKIPPED` items that are
not included in the follow-up issue (duplicates, factually incorrect
suggestions, status noise), still post rationale replies and resolve those
threads." and rewrite it to something like "When an item is a trivial `SKIPPED`
case (duplicates, factually incorrect suggestions, or status noise), post a
rationale reply and resolve the thread." to improve readability.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9a30df0-8b81-4646-bd21-8da8d912c895
📒 Files selected for processing (1)
commands/address-review.md
| gh api --paginate repos/${REPO}/pulls/${PR_NUMBER}/reviews | jq -s '[.[].[] | select((.body // "") != "") | {id: .id, type: "review_summary", body: .body, state: .state, user: .user.login, submitted_at: .submitted_at, html_url: .html_url}]' | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether current PR has review objects that are non-submitted but would be included by body-only filtering.
gh api --paginate "repos/${REPO}/pulls/${PR_NUMBER}/reviews" \
| jq -s '[.[].[] | select((.body // "") != "") | {id, state, submitted_at, html_url}]'Repository: shakacode/claude-code-commands-skills-agents
Length of output: 580
🌐 Web query:
GitHub API pull request reviews states submitted_at documentation
💡 Result:
In the GitHub REST API, pull request reviews have a 'state' field that can be PENDING, APPROVED, CHANGES_REQUESTED, COMMENT, or DISMISSED. The 'submitted_at' field is a date-time string present only for submitted reviews (not PENDING). PENDING reviews lack submitted_at as they are not submitted. In the GitHub GraphQL API, the PullRequestReview object has a 'state' enum (PENDING, COMMENTED, APPROVED, CHANGES_REQUESTED, DISMISSED) and 'submittedAt' field indicating submission time. Official documentation: https://docs.github.com/en/rest/pulls/reviews (REST API, up-to-date as of API version 2026-03-10).
Citations:
- 1: https://docs.github.com/rest/pulls/reviews
- 2: https://docs.github.com/en/rest/pulls/reviews?apiVersion=2026-03-10
- 3: https://docs.github.com/en/rest/pulls/reviews
- 4: https://developer.github.com/v3/pulls/reviews
Exclude non-submitted reviews from summary triage input.
This query can pull PENDING draft reviews that have body text but have not been submitted yet. Filter to submitted reviews (non-null submitted_at and excluding state: "PENDING") before classifying as actionable.
Suggested adjustment
-gh api --paginate repos/${REPO}/pulls/${PR_NUMBER}/reviews | jq -s '[.[].[] | select((.body // "") != "") | {id: .id, type: "review_summary", body: .body, state: .state, user: .user.login, submitted_at: .submitted_at, html_url: .html_url}]'
+gh api --paginate repos/${REPO}/pulls/${PR_NUMBER}/reviews | jq -s '[.[].[]
+ | select((.body // "") != "")
+ | select(.submitted_at != null)
+ | select(.state != "PENDING")
+ | {id: .id, type: "review_summary", body: .body, state: .state, user: .user.login, submitted_at: .submitted_at, html_url: .html_url}
+]'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@commands/address-review.md` around lines 92 - 93, The jq filter used in the
gh api command that builds review objects (the pipeline starting with gh api
--paginate ... | jq -s '[.[].[] | select((.body // "") != "") | {id: .id, type:
"review_summary", body: .body, state: .state, user: .user.login, submitted_at:
.submitted_at, html_url: .html_url}]') should exclude non-submitted reviews:
update the select to also require reviews be submitted (e.g. .submitted_at !=
null) and not in PENDING state (e.g. .state != "PENDING") before mapping into
the review_summary object so only actionable, submitted reviews are included.
|
|
||
| - If a summary comment exists and `CHECK_ALL_REVIEWS` is false, set `REVIEW_CUTOFF_AT` to that comment's `created_at` timestamp. | ||
| - Use exact timestamps in user-facing status updates, for example: "Scanning review activity after 2026-04-01T20:14:33Z." | ||
| - If no summary comment exists, scan the full PR history. |
There was a problem hiding this comment.
Missing explicit reset: when the jq query returns null (no summary comment exists), REVIEW_CUTOFF_AT is never set to empty. An LLM with retained state from a prior execution could carry a stale value forward. Suggest adding a line here:
| - If no summary comment exists, scan the full PR history. | |
| - If a summary comment exists and `CHECK_ALL_REVIEWS` is false, set `REVIEW_CUTOFF_AT` to that comment's `created_at` timestamp. | |
| - Otherwise (no summary comment, or `CHECK_ALL_REVIEWS` is true), set `REVIEW_CUTOFF_AT` to empty so no cutoff is applied. |
|
|
||
| ```bash | ||
| REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner) | ||
| ``` |
There was a problem hiding this comment.
The prose says "Otherwise, detect the repository from the current checkout" but the code block below shows gh repo view unconditionally with no guard. An LLM may run it even when REPO was already extracted from a URL. Suggest making the fallback explicit in the code block:
| ``` | |
| If the user input is a full GitHub URL, extract `org/repo` from the URL and use that as `REPO`. | |
| Otherwise, detect the repository from the current checkout: |
And in the code block, add a comment like # Fallback: only run if REPO was not extracted from a URL to clarify intent.
| f — Fix must-fix items, then confirm whether to reply/resolve skipped items before deciding discuss items | ||
| f+i — Fix must-fix + create follow-up issue for discuss/non-trivial skipped items | ||
| d — Discuss specific items before deciding (e.g., "d2,4"). Bare "d" presents all DISCUSS items. | ||
| r — Reply with rationale to items (e.g., "r3,5", "r7-9", "r all skipped", "r all discuss"); add `+ resolve` to also resolve those threads |
There was a problem hiding this comment.
The r all must-fix rejection (defined in Step 8) is invisible here. A user who types r all must-fix will get a surprising refusal with no prior indication. Adding a short note prevents confusion:
| r — Reply with rationale to items (e.g., "r3,5", "r7-9", "r all skipped", "r all discuss"); add `+ resolve` to also resolve those threads | |
| r — Reply with rationale to items (e.g., "r3,5", "r7-9", "r all skipped", "r all discuss"); add `+ resolve` to also resolve those threads (not applicable to must-fix items — use `f` or `f+i` instead) |
|
|
||
| DISCUSS_SECTION="" | ||
| if [ -n "${DISCUSS_ITEMS}" ]; then | ||
| printf -v DISCUSS_SECTION '### Discuss items\n%s\n' "${DISCUSS_ITEMS}" |
There was a problem hiding this comment.
printf -v varname is a bash-only built-in; it silently fails or errors in sh/dash. Since the broader script uses bash-specific constructs ($'\n\n', ${VAR%/*}), the whole snippet implicitly requires bash, but it's not stated. Either add a #!/usr/bin/env bash note at the top of the Step 9 snippet or switch to POSIX-compatible assignment:
| printf -v DISCUSS_SECTION '### Discuss items\n%s\n' "${DISCUSS_ITEMS}" | |
| DISCUSS_SECTION=$(printf '### Discuss items\n%s\n' "${DISCUSS_ITEMS}") |
Same applies to the SKIPPED_SECTION assignment two lines below.
Address-review summaryScan scope: full history (first run on this PR — no prior summary cutoff). MatteredMUST-FIX (addressed in 5d7b1b2):
OPTIONAL (addressed in 5d7b1b2):
Skipped
No follow-up issue was created; every item either landed in 5d7b1b2 or was a duplicate/bot-summary with no new signal. Next default scan starts after this comment. Say |
|
Superseded by #23, which includes this sync plus the companion prompts/address-review.md file and runtime bug fixes flagged in review here. |
1 participant
Summary
Brings
commands/address-review.mdin sync with the significantly evolved upstream version atshakacode/react_on_rails/.claude/commands/address-review.md.Key additions: summary cutoff system with
<!-- address-review-summary -->markers for incremental review passes,check all reviewsoverride, quick-action menu (f/f+i/d/r/m) with range syntax (1,3-5), follow-up GitHub issue creation for deferred items, consolidated PR summary checkpoint comment after each action, explicit merge-ready signal, review summary body fetching, timestamp-based filtering, and push confirmation.The other react_on_rails commands (
run-ci.md,update-changelog.md) are repo-specific and were not ported.Test plan
/address-review <PR URL>on a PR with existing review comments and verify the quick-action menu renders with dynamic item numbers<!-- address-review-summary -->cutoffcheck all reviewsand confirm full history is rescannedf+ipath and confirm a follow-up issue is created and linked in thread replies🤖 Generated with Claude Code
Note
Medium Risk
Although this is documentation, it significantly changes the
/address-reviewautomation workflow (scan cutoffs, reply/resolve behavior, follow-up issue creation, merge-ready signaling), so misunderstandings could lead to missed feedback or incorrect thread resolution.Overview
Updates the
/address-reviewcommand spec to support incremental review passes by defaulting full-PR scans to only comments after the latest<!-- address-review-summary -->checkpoint, with acheck all reviewsoverride and timestamp-based filtering that considers thread activity.Expands fetched data to include review summary bodies and adds required timestamps/URLs in API payloads, plus new execution guidance: a quick-action menu (
f/f+i/d/r/m) with range selection, optional follow-up issue creation for deferred items, and a single end-of-run PR summary checkpoint + explicit merge-ready/non-ready signal.Reviewed by Cursor Bugbot for commit 5d7b1b2. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
check all reviewsoverride, persistent PR summary checkpoint, and explicit merge-readiness signaling