Sync address-review command and prompt from react_on_rails#23
Sync address-review command and prompt from react_on_rails#23
Conversation
Pulls in the latest versions from shakacode/react_on_rails, adding the `check all reviews` override, summary checkpoint comments with the `<!-- address-review-summary -->` marker, and scan-window cutoff logic so repeated runs only triage new feedback by default.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe address-review workflow adds a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Workflow
participant GitHubAPI
participant Repo
User->>Workflow: invoke /address-review <PR or URL> [optional "check all reviews"]
Workflow->>GitHubAPI: resolve repo/PR, fetch comments, reviews, review threads (with timestamps)
GitHubAPI-->>Workflow: return comments/reviews/threads (created_at/submitted_at)
Workflow->>Workflow: find latest <!-- address-review-summary --> (derive REVIEW_CUTOFF_AT)
alt CHECK_ALL_REVIEWS not set
Workflow->>Workflow: filter items by REVIEW_CUTOFF_AT (use latest thread activity)
end
Workflow->>User: present quick-action menu (f, f+i, d, r, m) with triage set
User->>Workflow: select actions (supports numeric/range)
Workflow->>GitHubAPI: post PR comments / thread replies / create follow-up issue as needed
GitHubAPI-->>Workflow: ack created comments/issues
Workflow->>GitHubAPI: post consolidated PR summary comment <!-- address-review-summary -->
GitHubAPI-->>Workflow: ack summary comment (new cutoff)
Workflow->>Repo: optionally push changes (after git push confirmation)
Workflow->>User: report merge-ready status (based on deferred MUST-FIXs)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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. Review rate limit: 0/1 reviews remaining, refill in 59 minutes and 56 seconds.Comment |
Code ReviewOverviewThis PR syncs the
The changes are well-structured and the incremental-scan idea is genuinely useful for iterative review cycles. Bugs1. Review summary objects are fetched with 2.
Minor
Positive notes
|
![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 One P1 defect: the summary checkpoint post will fail at runtime because commands/address-review.md — specifically Step 10 (line 399) and the cutoff filtering prose in Step 4 (lines 106-107). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["User: /address-review input"] --> B["Step 1-2: Resolve REPO + parse PR reference\nDetect check all reviews flag"]
B --> C{"CHECK_ALL_REVIEWS?"}
C -->|Yes| D["Scan full PR history"]
C -->|No| E["Fetch latest address-review-summary comment"]
E --> F{"Summary comment exists?"}
F -->|Yes| G["Set REVIEW_CUTOFF_AT = summary.created_at"]
F -->|No| D
G --> H["Step 4: Fetch review data (filtered after cutoff)"]
D --> H
H --> I["Step 5: Triage: MUST-FIX / DISCUSS / SKIPPED"]
I --> J["Step 6-7: Todo list + Quick-action menu"]
J --> K{"User chooses action"}
K -->|f or f+i| L["Fix MUST-FIX, commit, push with confirmation"]
K -->|d| M["Discuss items, user decides"]
K -->|r| N["Post rationale replies"]
K -->|m| O["Create follow-up issue, defer all items"]
L --> P["Step 9: Create follow-up issue (f+i/m only)"]
M --> P
N --> P
O --> P
P --> Q["Step 10: Post PR summary comment with marker"]
Q --> R["Step 11: Signal merge-ready or not"]
Reviews (1): Last reviewed commit: "Sync address-review command and prompt f..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: 34b9199003
ℹ️ 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}
Address-review would fail at runtime in two ways:
1. The summary checkpoint post used `gh api --input <file>`, which sends
the file verbatim as the HTTP body. The file held plain markdown, but
`POST /issues/{issue_number}/comments` requires JSON with a `body`
field, so every run returned a parse error and never created the
`<!-- address-review-summary -->` marker, disabling the cutoff
mechanism on subsequent runs. Switched both call sites to
`-F body=@<file>` so `gh` wraps the markdown into the expected JSON.
2. Review summaries were projected with `submitted_at`, but the
cutoff-filter prose uniformly refers to `created_at`. Renamed the
projection to `created_at: .submitted_at` in both command and prompt
so the filter logic works uniformly across issue comments, inline
threads, and review summaries.
Also incorporate two cleanup notes from the review:
- Wrap the two `mktemp` blocks in `trap ... EXIT` so temp files are
cleaned up even when a step aborts mid-script.
- Drop redundant outer parens in the Step 3 jq `select(...)`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks — all four items from this review are addressed in bd809c2:
|
|
Both findings are addressed in bd809c2:
|
Address-review summaryScan scope: full PR history (default cutoff — no prior summary comment existed) Mattered
Skipped
Next default scan starts after this comment. Say |
|
To use Codex here, create an environment for this repo. |
|
Tip For best results, initiate chat on the files or code changes.
The checkpoint comment is correctly placed to set the scan-window cutoff for future runs. The PR looks good to merge. [approve] 🧠 Learnings used |
|
The summary comment looks well-formed and the checkpoint tag is correctly placed. A few quick observations:
The checkpoint comment itself (this one) correctly includes No blocking issues from my end. Tip: You can customize Greptile's behavior for this repo with |
Code ReviewOverviewThis PR syncs
The design is well-thought-out. The cutoff-scan pattern is a genuine usability improvement and the IssuesBug: double In the shell templates, Step 9 sets trap 'rm -f "${issue_body_file}" "${summary_body_file}"' EXITor append to the existing trap with Bash-specific
The Step-ordering inconsistency between the two files
The cutoff-fetch command pipes through REVIEW_CUTOFF_AT=$(... | jq -r 'if . == null then "" else .created_at end')Minor observations
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
prompts/address-review.md (1)
65-65: Remove unnecessary outer parentheses in jq select expression.The
selectexpression has an extra set of parentheses:select(((.body // "") | contains(...))). The corresponding command incommands/address-review.md(line 57) usesselect((.body // "") | contains(...))without the outer pair. While both are valid jq syntax, the extra parentheses are unnecessary and create an inconsistency between the two files.♻️ Simplify jq select for consistency
- `gh api --paginate repos/${REPO}/issues/{PR_NUMBER}/comments | jq -s '[.[].[] | select(((.body // "") | contains("<!-- address-review-summary -->"))) | {id: .id, created_at: .created_at, html_url: .html_url}] | sort_by(.created_at) | last'` + `gh api --paginate repos/${REPO}/issues/{PR_NUMBER}/comments | jq -s '[.[].[] | select((.body // "") | contains("<!-- address-review-summary -->")) | {id: .id, created_at: .created_at, html_url: .html_url}] | sort_by(.created_at) | last'`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@prompts/address-review.md` at line 65, The jq select expression in the command snippet uses an unnecessary outer pair of parentheses around the select argument (the instance shown as select(((.body // "") | contains("<!-- address-review-summary -->")))); update that snippet to remove the extra outer parentheses so it becomes the simpler select((.body // "") | contains(...)) form matching the other file, ensuring consistency across commands/address-review.md and this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@prompts/address-review.md`:
- Line 65: The jq select expression in the command snippet uses an unnecessary
outer pair of parentheses around the select argument (the instance shown as
select(((.body // "") | contains("<!-- address-review-summary -->")))); update
that snippet to remove the extra outer parentheses so it becomes the simpler
select((.body // "") | contains(...)) form matching the other file, ensuring
consistency across commands/address-review.md and this file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6be5f009-8ff6-4afc-845b-e3e53958c89a
📒 Files selected for processing (2)
commands/address-review.mdprompts/address-review.md
- Fix trap overwrite in `f+i`: Step 10 now cleans up both `issue_body_file` and `summary_body_file` so the Step 9 temp file is not leaked (commands/address-review.md Step 10). - Replace bash-only `printf -v` with plain variable assignment in the follow-up issue template (commands/address-review.md Step 9). - Define bare `r` and bare `r all` handling in Action `r`: both are rejected with guidance instead of being silently ignored. - Null-guard the summary-cutoff `jq ... | last` snippet so an empty result yields `""` (not JSON `null`) and document the empty-cutoff = full-history semantics; mirrored in prompts/address-review.md. - Reorder Step 1/Step 2 in commands/address-review.md so input parsing runs before repo detection, matching prompts/address-review.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
All issues flagged in this review are now addressed in 987a697:
Minor observations (bare |
Address-review summaryScan scope: since previous summary at 2026-04-19T03:37:36Z (default cutoff) Mattered
Skipped
Next default scan starts after this comment. Say |
Code ReviewOverviewThis PR adds incremental triage to The overall design is solid and the workflow logic is well-specified. A few items below are worth addressing before merge. Issues
Step 10's template references
The phrase is documented only trailing the PR reference (
Positive observations
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@prompts/address-review.md`:
- Line 65: The jq select expression used when computing REVIEW_CUTOFF_AT
contains redundant outer parentheses; update the expression in the
REVIEW_CUTOFF_AT command to match the simplified form used in
commands/address-review.md by removing the extra parentheses around the select
argument so the `select(((.body // "") | contains("<!-- address-review-summary
-->")))` portion becomes the consistent simplified `select((.body // "") |
contains("<!-- address-review-summary -->"))` while keeping the surrounding gh
api and jq pipeline intact.
🪄 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: ef341e33-f9f4-48be-862a-bec45af312ba
📒 Files selected for processing (2)
commands/address-review.mdprompts/address-review.md
…all-reviews - Capture gh issue create output into FOLLOW_UP_URL in Step 9 so Step 10's summary comment can include the link (was previously unset). - Add a "Capture the gh issue create output into FOLLOW_UP_URL" rule line so followers don't miss the assignment. - Drop the redundant MUST_FIX_BLOCK alias and use MUST_FIX_SECTION directly in the empty check and for-loop. - Sync prompts/address-review.md jq select expression with commands/ by removing redundant outer parens. - Tighten check-all-reviews matching in both files: case-insensitive, trailing position only; warn on leading/embedded usage instead of silently falling through to a cutoff-filtered scan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address-review summaryScan scope: review activity since previous summary at 2026-04-19T10:02:30Z (default cutoff; no Mattered
Skipped
Next default scan starts after this comment. Say |
|
Tip For best results, initiate chat on the files or code changes.
The checkpoint is correctly placed and will serve as the next default scan cutoff. PR looks good to merge. [approve] 🧠 Learnings used |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 431f76c157
ℹ️ 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".
| ```bash | ||
| REVIEW_CUTOFF_AT=$( | ||
| gh api --paginate repos/${REPO}/issues/{PR_NUMBER}/comments \ | ||
| | jq -rs '[.[].[] | select((.body // "") | contains("<!-- address-review-summary -->")) | {id: .id, created_at: .created_at, html_url: .html_url}] | sort_by(.created_at) | last | if . == null then "" else .created_at end' |
There was a problem hiding this comment.
The new cutoff query treats any issue comment containing <!-- address-review-summary --> as a checkpoint, so a regular human comment (or a quoted snippet) with that marker can become the latest cutoff and silently exclude valid review feedback posted earlier. Because the default full-PR scan now starts after this computed timestamp, a stray marker comment can cause the workflow to miss actionable items until someone remembers to run check all reviews.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed:
FOLLOW_UP_URLmissing:-default causes unset variable error- Step 10 now uses
${FOLLOW_UP_URL:-}so unset follow-up URLs do not abort underset -u.
- Step 10 now uses
- ✅ Fixed: Bash-only
$'\n\n'breaks POSIX sh compatibility claim- Step 9 now uses a POSIX literal newline variable instead of bash-only ANSI-C quoting.
Or push these changes by commenting:
@cursor push 1b8214e498
Preview (1b8214e498)
diff --git a/commands/address-review.md b/commands/address-review.md
--- a/commands/address-review.md
+++ b/commands/address-review.md
@@ -342,10 +342,12 @@
echo "No deferred items found; skip follow-up issue creation."
else
SECTION_CONTENT=""
+ NL='
+'
for section in "${MUST_FIX_SECTION}" "${DISCUSS_SECTION}" "${SKIPPED_SECTION}"; do
[ -z "${section}" ] && continue
if [ -n "${SECTION_CONTENT}" ]; then
- SECTION_CONTENT="${SECTION_CONTENT}"$'\n\n'
+ SECTION_CONTENT="${SECTION_CONTENT}${NL}${NL}"
fi
SECTION_CONTENT="${SECTION_CONTENT}${section}"
done
@@ -405,7 +407,7 @@
printf '%s\n\n' "<bullets for must-fix/discuss outcomes, or - None.>"
printf '### Skipped\n'
printf '%s\n\n' "<bullets for skipped items, or - None.>"
- if [ -n "${FOLLOW_UP_URL}" ]; then
+ if [ -n "${FOLLOW_UP_URL:-}" ]; then
printf 'Follow-up issue: %s\n\n' "${FOLLOW_UP_URL}"
fi
printf 'Next default scan starts after this comment. Say `check all reviews` to rescan the full PR.\n'You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 431f76c. Configure here.
| printf '### Skipped\n' | ||
| printf '%s\n\n' "<bullets for skipped items, or - None.>" | ||
| if [ -n "${FOLLOW_UP_URL}" ]; then | ||
| printf 'Follow-up issue: %s\n\n' "${FOLLOW_UP_URL}" |
There was a problem hiding this comment.
FOLLOW_UP_URL missing :- default causes unset variable error
Medium Severity
Step 10 references ${FOLLOW_UP_URL} without the :- default-value guard, but FOLLOW_UP_URL is only conditionally assigned inside Step 9's else branch. When actions like f, d, r, or direct selection skip Step 9 entirely, or when Step 9 takes the "no deferred items" path, the variable is never set. Under set -u (common in production scripts), ${FOLLOW_UP_URL} on line 408 would abort the script. The same code block already demonstrates the correct defensive pattern with ${issue_body_file:-} on line 399, including an explanatory comment about why :- is needed for variables that may not be set — making this omission an inconsistent oversight.
Reviewed by Cursor Bugbot for commit 431f76c. Configure here.
| for section in "${MUST_FIX_SECTION}" "${DISCUSS_SECTION}" "${SKIPPED_SECTION}"; do | ||
| [ -z "${section}" ] && continue | ||
| if [ -n "${SECTION_CONTENT}" ]; then | ||
| SECTION_CONTENT="${SECTION_CONTENT}"$'\n\n' |
There was a problem hiding this comment.
Bash-only $'\n\n' breaks POSIX sh compatibility claim
Low Severity
The $'\n\n' ANSI-C quoting on this line is a bash-only feature that does not work in POSIX sh (e.g., dash, which is /bin/sh on Debian/Ubuntu). The PR discussion states that printf -v was replaced in Step 9 specifically so "the snippet works under /bin/sh as well as bash," but $'\n\n' in the same code block undermines that goal. Under dash, $'\n\n' produces the literal string $\n\n (dollar sign, backslash, n, backslash, n) instead of two newline characters, corrupting the section separator in the follow-up issue body.
Reviewed by Cursor Bugbot for commit 431f76c. Configure here.
1 participant
Summary
commands/address-review.mdandprompts/address-review.mdwith the latest versions from shakacode/react_on_rails.check all reviewsoverride phrase, PR summary checkpoint comments tagged with<!-- address-review-summary -->, and scan-window cutoff logic so repeated runs only triage feedback posted since the last summary.Test plan
/address-review <PR>on a PR with an existing summary comment and confirm it only surfaces new feedback./address-review <PR> check all reviewsand confirm it rescans the full PR history.Note
Medium Risk
Although this PR only updates Markdown workflow docs, it materially changes the
/address-reviewprocess (cutoff-based scanning, new actions, follow-up issue creation), which could cause reviewers/agents to miss older feedback if the cutoff/override is misunderstood.Overview
Updates the
/address-reviewcommand and companion prompt to support incremental review triage by default: full-PR scans now start after the latest checkpoint comment marked<!-- address-review-summary -->, with an explicitcheck all reviewsoverride to rescan history.Expands fetched data to include review summary bodies and timestamps/URLs, adds guidance for thread-activity-based filtering, and formalizes a quick-action menu (
f,f+i,d,r,m) with range selection plus new flows for follow-up issue creation and posting a consolidated PR summary checkpoint after actions.Reviewed by Cursor Bugbot for commit 987a697. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
f,f+i,d,r,m) with range-based selection and validation messagesBug Fixes / Improvements