feat(code-review): add thematic triage grouping#845
Conversation
|
@jblwilliams I'd like to see a few examples of output if you can provide it. |
![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: 8f1f19a0b4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| (A) Resolve by triage group — handle related findings together, with per-finding fallback | ||
| (B) Review each finding one by one — accept the recommendation or choose another action | ||
| (C) Auto-resolve with best judgment — apply per-finding fixes the agent can defend, surface the rest | ||
| (D) File a [TRACKER] ticket per finding without applying fixes | ||
| (E) Report only — take no further action |
There was a problem hiding this comment.
This routing menu defines five choices while the blocking question tools used here have strict option caps (the skill itself documents a 4-option cap for AskUserQuestion-style menus). In interactive runs, attempting to send all five as tool options will either fail the tool call or force an error-driven fallback every time, which breaks the intended primary UX path for a normal review. Split this into sequential questions or move one branch behind a follow-up prompt so the first question stays within the supported option limit.
Useful? React with 👍 / 👎.
| 1. Apply group fix | ||
| 2. Defer group — file one [TRACKER] ticket | ||
| 3. Split into findings | ||
| 4. Skip group | ||
| 5. Auto-resolve remaining groups with best judgment |
There was a problem hiding this comment.
The group walk-through path also defines five options for a single blocking question, which exceeds supported menu limits in the target question tools. When users choose grouped triage, this can cause the question call to error instead of rendering a selectable menu, so the new grouped workflow is unreliable in standard interactive sessions. Keep this question to the supported max (or split into a second prompt for the less-common branch).
Useful? React with 👍 / 👎.
|
@tmchow here is an example output of a review: https://gist.github.com/jblwilliams/43188df4885611ca97d41a85405e5a85 As a concrete example from this review in Group A, I was able to meaningfully have a follow-up discussion for managing sandbox permissions as a central theme instead of coming up on these different paper cuts piecemeal. That allows me to focus on designing a more robust solution because I can see all the potential issues that the current code could run into. Following this pattern helps think about code reviews as more architectural and that's where I like to spend a lot of my thought and focus in reviews now to understand classes of issues. |
|
Thanks. One thing to pause on is I'm doing a major change to ce-code-review so it'll no longer handle the auto fixing. I don't like the mixing of responsibilities and want it just to handle the review and report findings. This will support more orchestrated flows better. So I'm gonna get that merged in first then have you rebase off that as I think your work here is useful. |
|
@tmchow sounds good. any work in progress branch you have that I can look at/follow? |
2 participants
Summary
Adds optional thematic grouping to
ce-code-reviewso related findings are easier to triage together instead of as one long flat list.The existing severity tables and stable finding IDs stay as the source of truth, but reviews can now include a short
Triage Groupssection that explains which findings are related, why they matter as a theme, and what order they should be handled in.Why
On larger reviews, the finding count can easily get into the double digits. A lot of those issues are often related, and one design decision or fix can resolve several of them at once. Grouping gives developers more context about the shape of the feedback so they do not have to work through 20 or 30 individual findings as if each one were independent.
Changes
grouping:auto,grouping:off, andgrouping:always