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.
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. 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
|
| The user's input is: $ARGUMENTS | ||
|
|
||
| ## Step 2: Parse User Input | ||
| First, detect whether the request includes the exact phrase `check all reviews`. |
There was a problem hiding this comment.
The spec says "exact phrase" but doesn't define whether matching is case-sensitive or whether the phrase must appear trailing. A user typing Check All Reviews or check all reviews 12345 (leading) would silently fall through to a cutoff-filtered scan instead of seeing an error or override. Consider adding:
| First, detect whether the request includes the exact phrase `check all reviews`. | |
| First, detect whether the request includes the phrase `check all reviews` (case-insensitive, trailing position only). |
Or, if leading position should also be supported, document that explicitly and note it in the supported-formats list below.
| 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 |
There was a problem hiding this comment.
${FOLLOW_UP_URL} is referenced here but is never explicitly assigned in Step 9. The Step 9 rules say "Return the issue URL to the user" but give no instruction to capture the gh issue create output into this variable. An LLM following the template could skip the follow-up URL line silently.
Step 9 should include an explicit assignment, e.g.:
FOLLOW_UP_URL=$(gh issue create --repo "${REPO}" --title "..." --body-file "${issue_body_file}")…so Step 10 can reliably reference it.
| EOF | ||
| )" | ||
|
|
||
| MUST_FIX_BLOCK="${MUST_FIX_SECTION}" |
There was a problem hiding this comment.
MUST_FIX_BLOCK is set to the same value as MUST_FIX_SECTION immediately after it's assigned, and is only used in the for-loop and the empty check below. The extra alias adds indirection without benefit — the for-loop and check could reference "${MUST_FIX_SECTION}" directly.
| MUST_FIX_BLOCK="${MUST_FIX_SECTION}" | |
| # (remove MUST_FIX_BLOCK; use MUST_FIX_SECTION directly in the for-loop and empty check) |
| - If `CHECK_ALL_REVIEWS` is true, ignore the cutoff and scan the full PR history. | ||
| - If the input is a specific review URL or specific issue-comment URL, fetch that exact target even if it predates the latest summary comment. | ||
| - Fetch the latest summary comment before collecting review data. Use a null-safe extraction so an empty result becomes an empty string (not JSON `null`): | ||
| `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.
Minor inconsistency with the commands/ version: this uses double parens select(((.body // "") | contains(...))) while commands/address-review.md (line 59) uses single parens select((.body // "") | contains(...)). Both are valid jq, but keeping them identical reduces divergence between the two files.
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
| - If `CHECK_ALL_REVIEWS` is true, ignore the cutoff and scan the full PR history. | ||
| - If the input is a specific review URL or specific issue-comment URL, fetch that exact target even if it predates the latest summary comment. | ||
| - Fetch the latest summary comment before collecting review data. Use a null-safe extraction so an empty result becomes an empty string (not JSON `null`): | ||
| `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.
Inconsistent jq syntax with commands/address-review.md.
The select expression has redundant outer parentheses:
select(((.body // "") | contains("<!-- address-review-summary -->")))
The commands file (line 59) has the simplified version:
select((.body // "") | contains("<!-- address-review-summary -->"))
Both work identically, but per the PR objectives mentioning "removed redundant jq parentheses," these two files should be consistent.
🔧 Proposed fix for consistency
- `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')`
+ `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')`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `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')` | |
| `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')` |
🤖 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 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.
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 987a697. Configure here.
| printf 'Original PR: https://github.com/%s/pull/%s\n' "${REPO}" "${PR_NUMBER}" | ||
| } > "${issue_body_file}" | ||
|
|
||
| gh issue create --repo "${REPO}" --title "Follow-up: Review feedback from PR #${PR_NUMBER}" --body-file "${issue_body_file}" |
There was a problem hiding this comment.
Follow-up issue URL never captured into variable
Medium Severity
Step 9's gh issue create output (the new issue URL) is never captured into FOLLOW_UP_URL, but Step 10's summary-comment template checks ${FOLLOW_UP_URL} and conditionally prints it. Since the variable is never assigned, the condition is always false and the summary comment will never include the follow-up issue link. This breaks the stated requirement to "Mention any follow-up issue URL that was created" and means the f+i and m actions silently omit the reference from the checkpoint comment.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 987a697. 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