Keep address-review triage aligned with latest actionable feedback#2798
Keep address-review triage aligned with latest actionable feedback#2798
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughTwo workflow and command documentation files were updated to refine comment filtering and triage classification behavior. Reply comments now contribute to thread context rather than creating independent items, and classification criteria were expanded to distinguish actionable summaries from boilerplate content. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
✨ 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 tightens the triage logic in both the generic agents workflow ( Key improvements:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Fetch PR Comments] --> B{Has in_reply_to_id?}
B -->|Yes - OLD| X1["❌ Skip entirely"]
B -->|Yes - NEW| C["Use as context for parent thread\n(if it updates/narrows the concern)"]
C --> D[Parent top-level comment drives triage]
B -->|No| D
D --> E{Comment type?}
E -->|review_summary| F{Actionable content?}
F -->|No - boilerplate/status| G["SKIPPED\n(non-actionable summary)"]
F -->|Yes - OLD| X2["❌ Also SKIPPED as 'summary'"]
F -->|Yes - NEW| H["Classify normally\nMUST-FIX / DISCUSS"]
E -->|inline / issue| H
H --> I{Severity?}
I -->|Bug / security / regression| J[MUST-FIX → Todo]
I -->|Scope expansion / opinion| K[DISCUSS → Present to user]
I -->|Style / nit / speculative| G
Last reviewed commit: "Prevent triage guida..." |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| 4. Filter comments: | ||
| - Skip resolved threads. | ||
| - Skip replies where `in_reply_to_id` is set. | ||
| - Do not create standalone triage items from comments where `in_reply_to_id` is set, but use reply text as the latest thread context when it updates or narrows the unresolved concern. |
There was a problem hiding this comment.
The mechanics of "use reply text as the latest thread context" are left implicit. An agent executing this step may not know whether to:
- Append the reply to the parent comment's body before triaging
- Replace the parent's body with the reply
- Just note the reply alongside the parent
A more concrete formulation might help:
| - Do not create standalone triage items from comments where `in_reply_to_id` is set, but use reply text as the latest thread context when it updates or narrows the unresolved concern. | |
| - Do not create standalone triage items from comments where `in_reply_to_id` is set; instead, prepend the most recent reply text to the parent comment's body before triaging, so the triage reflects the current state of the concern rather than only the original wording. |
Also, the qualifier "when it updates or narrows the unresolved concern" leaves two edge cases unaddressed:
- A reply that withdraws the concern ("never mind, I see it now") — should the thread be auto-skipped?
- A reply that broadens the concern — the current wording would still fold it in, but it would be good to say so explicitly.
| - Verify reviewer claims locally before calling something `MUST-FIX`. | ||
| - If a claim is wrong, classify it as `SKIPPED` and say why. | ||
| - Preserve comment IDs and thread IDs for later replies and thread resolution. | ||
| - Treat actionable review summary bodies as normal feedback to classify (`MUST-FIX`/`DISCUSS` as appropriate); skip only boilerplate or status-only summaries. |
There was a problem hiding this comment.
The new bullet was added after Preserve comment IDs and thread IDs for later replies and thread resolution. Functionally it fits, but it reads slightly out of sequence: the rule about what to classify (actionable summaries as MUST-FIX/DISCUSS) would be easier to follow if it appeared before the subject-format rule at the bottom. Minor ordering nit only.
ReviewOverall: Solid, focused improvement. The change correctly moves from a blanket 'skip all replies' rule to a more nuanced context-aware one, and fixes the over-broad classification of review summary bodies as always-skipped. Both files are updated consistently, which is the right maintenance pattern. Two things worth addressing: 1. Vague mechanics for incorporating reply context (both files) The instruction to use reply text 'as the latest thread context when it updates or narrows the unresolved concern' leaves an AI executor without a concrete operation. Different agents may merge, append, replace, or just read the reply — producing different triage output. A more specific instruction (e.g. 'prepend the most recent reply to the parent comment body before triaging') would reduce ambiguity. See inline comment on line 69. Two edge cases are also unaddressed by the 'updates or narrows' qualifier:
2. Rule ordering in the triage step (minor) The new 'Treat actionable review summary bodies as normal feedback' bullet lands after the subject-format rule, slightly out of sequence with the other classification criteria. Moving it up would improve readability. See inline comment on line 85. What is good:
|
![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: c185ba88b4
ℹ️ 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".
| 4. Filter comments: | ||
| - Skip resolved threads. | ||
| - Skip replies where `in_reply_to_id` is set. | ||
| - Do not create standalone triage items from comments where `in_reply_to_id` is set, but use reply text as the latest thread context when it updates or narrows the unresolved concern. |
There was a problem hiding this comment.
This new rule relies on reply text to represent the current unresolved concern, but the workflow still fetches review comments without any created_at/updated_at field or other per-thread ordering metadata. In threads with multiple replies, the agent cannot tell whether it is looking at the reviewer’s latest narrowed concern or an older reply, so it can triage stale context instead of the thread’s actual remaining issue.
Useful? React with 👍 / 👎.
| - Verify reviewer claims locally before calling something `MUST-FIX`. | ||
| - If a claim is wrong, classify it as `SKIPPED` and say why. | ||
| - Preserve comment IDs and thread IDs for later replies and thread resolution. | ||
| - Treat actionable review summary bodies as normal feedback to classify (`MUST-FIX`/`DISCUSS` as appropriate); skip only boilerplate or status-only summaries. |
There was a problem hiding this comment.
Treating review summary bodies as normal actionable feedback becomes problematic because step 3 still pulls every non-empty body from /pulls/{PR_NUMBER}/reviews, including summaries from earlier review rounds. On a PR that had an early summary requesting changes and a later approval after fixes, this instruction will resurface the stale summary as a fresh MUST-FIX/DISCUSS item even though the workflow has no thread-resolution signal for review bodies. That makes the triage checklist regress to already-addressed feedback.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| - Skip comments belonging to already-resolved threads (match via `thread_id` and `is_resolved` from the GraphQL response) | ||
| - Skip comments where `in_reply_to_id` is set (these are replies, not top-level comments) | ||
| - Do not create standalone triage items from comments where `in_reply_to_id` is set, but use reply text as the latest thread context when it updates or narrows the unresolved concern |
There was a problem hiding this comment.
Filtering section still skips all summaries before triage
Medium Severity
The filtering section (line 87) tells the AI to skip "summaries" during comment filtering, but the new triage rule (line 112) says to classify actionable summaries as normal feedback. This creates an internal contradiction — actionable summaries get discarded in filtering before reaching triage. The corresponding line in .agents/workflows/address-review.md says only "skip status-only bot posts" (no mention of summaries), so the two files are not aligned despite the PR's stated goal of mirroring behavior.
Additional Locations (1)
## Summary - Stamp `### [16.5.0.rc.0]` version header with today's date - Add 10 new changelog entries for PRs merged since v16.4.0 - Fix incomplete PR 2818 entry (missing author link) ### New entries added **Added:** - `create-react-on-rails-app --pro` support (PR 2818) - Global prerender env override `REACT_ON_RAILS_PRERENDER_OVERRIDE` (PR 2816) - `react_on_rails:sync_versions` rake task (PR 2797) - Pro/RSC setup checks in `react_on_rails:doctor` (PR 2674) **Changed:** - [Pro] Canonical env var for worker count is now `RENDERER_WORKERS_COUNT` (PR 2611) **Improved:** - Smoother `create-react-on-rails-app` and install generator flows (PR 2650) - Pro upgrade hint after install (PR 2642) **Fixed:** - Preserve runtime env vars across `Bundler.with_unbundled_env` (PR 2836) - Fix doctor prerender check and ExecJS display for Pro/RSC apps (PR 2773) - Fix doctor false positives for custom layouts (PR 2612) ### Skipped PRs (not user-visible) Docs-only: #2845, #2842, #2826, #2830, #2820, #2809, #2803, #2785, #2801, #2791, #2789, #2788, #2772, #2778, #2780, #2784, #2671, #2676, #2662, #2657, #2669 CI/internal tooling: #2825, #2817, #2819, #2812, #2815, #2810, #2808, #2807, #2634, #2798, #2761, #2760, #2658, #2639, #2667, #2656 ## Test plan - [x] Verified version header and diff links are correct - [x] Verified all entries follow changelog formatting conventions - [x] Verified file ends with newline - [ ] After merge, run `rake release` to publish 16.5.0.rc.0 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only change updating `CHANGELOG.md` with a new `16.5.0.rc.0` section and compare links; no runtime code is modified. > > **Overview** > Adds a new `16.5.0.rc.0` (2026-03-25) section to `CHANGELOG.md`, consolidating recent PR entries under **Added/Changed/Improved/Fixed** and correcting the previously incomplete `--pro` CLI entry author attribution. > > Updates the bottom compare links so `[unreleased]` now compares from `v16.5.0.rc.0` and adds a link definition for `[16.5.0.rc.0]`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 481a71c. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - v16.5.0.rc.0 * **New Features** * Added sync_versions task for streamlined version management * Expanded doctor checks for Pro and RSC support * **Improvements** * Enhanced generator workflow and Pro upgrade guidance * Improved environment variable handling and preservation * **Bug Fixes** * Fixed detection issues with doctor tools and ExecJS/prerender functionality <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…2798) ## Summary - update reply filtering guidance so replies are not standalone triage items, but still update unresolved thread context - classify actionable review summary bodies as normal feedback and only skip non-actionable summaries - mirror the same behavior in both `.agents/workflows/address-review.md` and `.claude/commands/address-review.md` to keep Claude and non-Claude workflows aligned ## Testing - pnpm exec prettier --check .agents/workflows/address-review.md .claude/commands/address-review.md
…2798) ## Summary - update reply filtering guidance so replies are not standalone triage items, but still update unresolved thread context - classify actionable review summary bodies as normal feedback and only skip non-actionable summaries - mirror the same behavior in both `.agents/workflows/address-review.md` and `.claude/commands/address-review.md` to keep Claude and non-Claude workflows aligned ## Testing - pnpm exec prettier --check .agents/workflows/address-review.md .claude/commands/address-review.md
1 participant
Summary
.agents/workflows/address-review.mdand.claude/commands/address-review.mdto keep Claude and non-Claude workflows alignedTesting
Summary by CodeRabbit
Improvements