Skip to content

Commit 94a7727

Browse files
committed
ENH: gh-triage-pr — support CodeRabbit alongside Greptile
Phase 3 recognised exactly one reviewer, greptile-apps[bot]. Any other review bot fell through is_bot() into "bot_other", a bucket the skill documents as "non-blocking, skip unless explicitly asked". On Project-MONAI/MONAI#9065 that put a genuine actionable CodeRabbit finding in the ignore pile; it was only acted on because the raw JSON was read by hand. The single GREPTILE_LOGIN constant becomes AI_REVIEW_PROVIDERS, keyed by bot login and carrying what differs per provider: how a review is requested, how one is forced for an already-reviewed head, and which in-repo file indicates the provider is configured. Findings are parsed per provider and normalised to P1/P2/P3, so CodeRabbit's Critical/Major/Minor maps onto the vocabulary the phase logic already speaks and one rule covers both. Two bugs surfaced while testing this against real PRs. The greptile parser never matched inline findings. Its pattern was alt="(P[123])"[^>]*>\s*\*\*([^*]+)\*\* but the badge is an <img> wrapped in an <a>, so a closing </a> sits between the badge and the bold title and \s* cannot span it. Most findings are inline, so Phase 3 has been running "address every P1/P2" against an empty list. InsightSoftwareConsortium/ITK#6777 reports 0 findings before this change and 2 P1s after. Provider detection read config files relative to the working directory, so triaging owner/repo#N from an unrelated checkout reported whatever that checkout happened to contain. It now queries the target repo. Unrecognised bots go to a new "bot_unknown" bucket rather than "bot_other". The two are documented differently on purpose: bot_other is ignorable, bot_unknown means nobody has classified this bot yet and it must be read before the phase can be called clean. That is the failure mode above, closed for the next review bot as well as this one. phase_3_ai_review also carries CodeRabbit's PR-level signals, merge_risk and failed_pre_merge_checks, which have no greptile equivalent and no inline comment to hang off. On Project-MONAI/MONAI#9067 merge_risk was "High" with zero inline findings — a credential-exposure issue in a workflow that would otherwise have been reported as Phase 3 clean. phase_3_greptile is retained as an alias so callers written against the old report keep working. ghtp_reply.py is untouched: replying and resolving are provider-agnostic. Verified against ITK#6714 and ITK#6777 (greptile) and MONAI#9065 and MONAI#9067 (coderabbit); test_ghtp_workstate.py still passes.
1 parent bd2b9d5 commit 94a7727

2 files changed

Lines changed: 335 additions & 119 deletions

File tree

skills/gh-triage-pr/SKILL.md

Lines changed: 116 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
---
22
name: gh-triage-pr
3-
version: 1.0.0
4-
purpose: 'Triage one or more GitHub pull requests in a strict priority order: (1) address human reviewer comments first, (2) fix CI failures second, (3) request and address a @greptileai draft review third — only after all draft CI builds are green, (4) finally recommend marking the PR ready for review.'
3+
version: 1.1.0
4+
purpose: 'Triage one or more GitHub pull requests in a strict priority order: (1) address human reviewer comments first, (2) fix CI failures second, (3) address the AI-reviewer findings for the repo (greptile or coderabbit) third — only after all CI builds are green, (4) finally recommend marking the PR ready for review.'
55
description: >-
66
Triage one or more GitHub pull requests in a strict priority order:
77
(1) address human reviewer comments first, (2) fix CI failures second,
8-
(3) request and address a @greptileai draft review third — only after
9-
all draft CI builds are green, (4) finally recommend marking the PR
8+
(3) address the AI-reviewer findings for the repo (greptile or
9+
coderabbit) third — only after all CI builds are green, (4) finally recommend marking the PR
1010
ready for review. Use this skill whenever the user says: "gh-triage-PR",
1111
"gh-triage-PR #NNNN", "gh-triage-PR my", "gh-triage-PR all-draft",
1212
"triage PR", "triage my PRs", "triage all drafts", "clean up my PRs",
@@ -74,7 +74,7 @@ If the user invokes `/gh-triage-pr` with no arguments or an ambiguous
7474
request, print this usage hint and ask what they'd like to triage:
7575

7676
```
77-
gh-triage-pr — Progressive PR triage (humans → CI → greptile → ready)
77+
gh-triage-pr — Progressive PR triage (humans → CI → AI review → ready)
7878
7979
Usage:
8080
/gh-triage-pr #6040 Triage one PR (current repo)
@@ -86,7 +86,7 @@ Usage:
8686
Phases (strict order — do not skip):
8787
1. Human comments Address reviewer feedback, fixup commits
8888
2. CI failures Fix red checks on HEAD
89-
3. Greptile review Request @greptileai review, address P1/P2
89+
3. AI review Address P1/P2 from greptile / coderabbit
9090
4. Metadata + ready Check title/body, recommend gh pr ready
9191
```
9292

@@ -106,8 +106,8 @@ Phases (strict order — do not skip):
106106

107107
These four phases are deterministic. Do not advance a phase until the
108108
prior phase is fully clean. This is the whole point of the skill —
109-
greptile's review is noisy and context-dependent, so we only ask for it
110-
after humans and CI are settled.
109+
AI review is noisy and context-dependent, so we only ask for it after
110+
humans and CI are settled.
111111

112112
```
113113
┌─────────────────────────────────────────────────────────────┐
@@ -119,10 +119,10 @@ after humans and CI are settled.
119119
│ Red checks on the current HEAD commit. │
120120
│ Do not proceed to Phase 3 with any red check. │
121121
├─────────────────────────────────────────────────────────────┤
122-
│ Phase 3 GREPTILE DRAFT REVIEW │
123-
Post "@greptileai review this draft before I make
124-
it official" and wait for its response. Address
125-
│ every P1/P2 finding. Iterate until clean. │
122+
│ Phase 3 AI REVIEW
123+
Whichever provider the repo runs (greptile,
124+
coderabbit, ...). Request or force a review, then
125+
address every P1/P2. Iterate until clean. │
126126
├─────────────────────────────────────────────────────────────┤
127127
│ Phase 4 PR METADATA + READY-FOR-REVIEW RECOMMENDATION │
128128
│ Re-read PR title + body + final diff. Recommend │
@@ -278,7 +278,7 @@ was done. Example reply bodies:
278278
After replying, resolve the thread via GraphQL (REST has no resolve
279279
endpoint). See `scripts/ghtp_reply.py` which handles both in one call.
280280

281-
### Format of PR bodies, comments, and greptile-review summaries
281+
### Format of PR bodies, comments, and AI-review summaries
282282

283283
**All text posted to GitHub must follow `~/.claude/rules/pr-message-format.md`.**
284284
Inline review replies via `ghtp_reply.py` are usually one-liners and don't
@@ -402,8 +402,14 @@ This prints a JSON report with three buckets:
402402
- `human_comments` — real-user comments, grouped by thread; each has
403403
`is_resolved`, `commit_line_blame` (if inline), and a `priority` hint
404404
- `ci_failures` — non-success checks on the HEAD commit
405-
- `greptile_findings` — greptile review findings (parsed from its
406-
markdown body), tagged P1/P2/P3 where greptile has indicated
405+
- `phase_3_ai_review` — findings from whichever AI reviewer the repo
406+
runs, normalised to P1/P2/P3 regardless of provider vocabulary. Also
407+
carries `providers_seen`, `blocking_findings` (unresolved P1/P2),
408+
and, where the provider supplies them, `merge_risk` and
409+
`failed_pre_merge_checks`
410+
- `bot_unknown` — bots matching no known provider or infra bot.
411+
**Not ignorable**: inspect these, then classify them in
412+
`AI_REVIEW_PROVIDERS` or `NON_BLOCKING_BOTS`
407413

408414
Also fetches the PR title, body, draft/ready state, and commit list.
409415

@@ -510,74 +516,88 @@ post a brief PR comment explaining the failure is a known flake before
510516
re-triggering. This prevents reviewers from thinking the PR has real
511517
issues.
512518

513-
### Step 4 — Phase 3: greptile draft review
519+
### Step 4 — Phase 3: AI review
514520

515521
Only after Phase 1 and Phase 2 are fully clean.
516522

517-
**Pre-check: Greptile availability**
518-
519-
Before requesting a review, verify Greptile is reachable:
520-
521-
```bash
522-
# Check if GREPTILE_API_KEY is set
523-
if [ "$GREPTILE_API_KEY" = "SKIP" ]; then
524-
echo "Greptile review skipped (GREPTILE_API_KEY=SKIP)"
525-
# Advance directly to Phase 4
526-
fi
527-
528-
if [ -z "$GREPTILE_API_KEY" ]; then
529-
echo "⚠ GREPTILE_API_KEY not set. Cannot run local review."
530-
echo ""
531-
echo "To configure:"
532-
echo " 1. Sign up at https://greptile.com"
533-
echo " 2. Get API key from https://app.greptile.com/settings/api"
534-
echo " 3. Add to ~/.zshrc: export GREPTILE_API_KEY=\"your-key\""
535-
echo " 4. Restart Claude Code"
536-
echo ""
537-
echo "To permanently skip: export GREPTILE_API_KEY=\"SKIP\""
538-
echo ""
539-
echo "Falling back to GitHub @greptileai bot comment..."
540-
fi
541-
```
542-
543-
**If `GREPTILE_API_KEY=SKIP`:** Skip Phase 3 entirely. Advance to Phase 4.
544-
545-
**If `GREPTILE_API_KEY` is set and MCP tools available:** Use the
546-
`trigger_code_review` MCP tool for a local review before posting to GitHub.
547-
This is faster and doesn't create noise on the PR.
548-
549-
**If MCP tools are not available:** Fall back to the GitHub bot approach:
550-
551-
1. Post the review-request comment:
552-
```bash
553-
gh pr comment "$NUM" --repo "$OWNER/$REPO" \
554-
--body "@greptileai review this draft before I make it official"
555-
```
556-
2. Wait for greptile to respond (typically 1-3 minutes). Poll via
557-
`ghtp_fetch.py` until the response arrives.
558-
559-
**Handling findings (both local and GitHub approaches):**
560-
561-
3. Address each P1/P2 finding using the same fixup pattern.
562-
4. Reply to each greptile inline comment in-thread (greptile threads
563-
can be resolved the same way as human threads).
564-
5. **ITK style conformance is always in scope** — fix naming, include
565-
order, assertion quality, `using` vs `typedef`, etc. regardless of
566-
Greptile priority level.
523+
Repos run different AI reviewers, and they differ in how a review is
524+
obtained, not just in output format. `ghtp_fetch.py` normalises the
525+
findings; this table covers the parts you have to drive by hand.
526+
527+
| | **Greptile** | **CodeRabbit** |
528+
|---|---|---|
529+
| login | `greptile-apps[bot]` | `coderabbitai[bot]` |
530+
| in-repo config || `.coderabbit.yaml` |
531+
| how a review starts | you request it | **automatic on every push** |
532+
| request comment | `@greptileai review this draft before I make it official` | none needed |
533+
| force a re-review | re-post the request | `@coderabbitai full review` |
534+
| severity vocabulary | `P1` / `P2` / `P3` badges | Critical / Major / Minor / Trivial |
535+
| extra PR-level signals || merge-risk rating, pre-merge checks |
536+
537+
`ghtp_fetch.py` maps CodeRabbit's Critical→P1, Major→P2, Minor/Trivial→P3
538+
and reports everything under `phase_3_ai_review`, so the rule below is one
539+
rule for all providers.
540+
541+
**Which provider is in play:** read `phase_3_ai_review.providers_seen`
542+
(who has actually commented) and `providers_configured_in_repo` (who the
543+
repo is configured for). A provider can be installed org-wide with no
544+
in-repo config, so `providers_seen` is the authority. Both may be present
545+
at once; handle every provider's findings.
546+
547+
**If `bot_unknown` is non-empty, stop and look.** That bucket means a bot
548+
commented that is neither a known reviewer nor a known infra bot. Read its
549+
comments before continuing, then add its login to `AI_REVIEW_PROVIDERS`
550+
(with a parser) or to `NON_BLOCKING_BOTS`. Leaving it unclassified is how
551+
a real finding gets lost.
552+
553+
**Procedure**
554+
555+
1. **Get a review for the current HEAD.**
556+
- *Greptile*: post the request comment, then poll `ghtp_fetch.py`
557+
until it responds (typically 1-3 minutes).
558+
- *CodeRabbit*: it has usually already reviewed. If it replies
559+
*"Already reviewed the last commit"* — which it does after a
560+
force-push it has seen — post `@coderabbitai full review` to force
561+
a fresh pass over the whole changeset.
562+
- Either way, confirm the review covers the current HEAD. A review of
563+
a pre-force-push commit is stale even when its inline comments carry
564+
forward.
565+
2. **Address every unresolved P1/P2** (`blocking_findings`) using the
566+
fixup pattern from Phase 1.
567+
3. **Reply in-thread and resolve** via `ghtp_reply.py --resolve`. This is
568+
provider-agnostic — thread IDs work the same for both.
569+
4. **Project style conformance is always in scope** regardless of the
570+
provider's priority level: naming, include order, assertion quality,
571+
`using` vs `typedef`.
572+
5. **False positives**: reply with a brief justification rather than
573+
changing code to silence the bot. When declining, cite evidence from
574+
the repo — measured prevalence of the pattern, or why the request is
575+
unreachable within the PR's scope — not just a preference.
567576
6. **Non-trivial or out-of-scope suggestions** (algorithm restructuring,
568-
new features, API redesign) → present to user for manual decision:
577+
new features, API redesign) → present to the user:
569578
```
570-
Greptile suggests (P2): "<suggestion>"
579+
<provider> suggests (P2): "<suggestion>"
571580
This appears out of scope for this PR. Options:
572581
1. Skip (recommended)
573582
2. Address as follow-up PR
574583
3. Implement now
575584
```
576-
7. If greptile flags a false positive, reply with a brief justification
577-
— do not force a fix just to silence it.
578-
8. Iterate: force-push, request another greptile review, address new
579-
findings, until greptile reports clean or only has P3/nitpicks you
580-
choose to accept.
585+
7. **CodeRabbit-only signals**, both PR-level and worth reading before
586+
declaring Phase 3 done:
587+
- `merge_risk` — its own blocking assessment. A "High" rating with no
588+
inline findings still deserves a look at the walkthrough.
589+
- `failed_pre_merge_checks` — gates visible on the PR page. Some are
590+
unreachable within a PR's scope (e.g. a docstring-coverage threshold
591+
that counts pre-existing functions); when so, say why in the thread
592+
rather than expanding scope to satisfy it.
593+
8. Iterate until there are no unresolved P1/P2 findings.
594+
595+
**Optional: Greptile local review.** If `$GREPTILE_API_KEY` is set and
596+
not `SKIP`, the `trigger_code_review` MCP tool runs a review locally
597+
without posting to the PR. If it is `SKIP`, skip the Greptile half of
598+
this phase. If it is unset, fall back to the GitHub bot comment above.
599+
There is no local-review equivalent for CodeRabbit.
600+
581601

582602
### Step 5 — Phase 4: PR metadata and ready-for-review
583603

@@ -586,7 +606,7 @@ This is faster and doesn't create noise on the PR.
586606
`gh pr edit "$NUM" --title "..."`.
587607
2. Re-read the PR body. Same check. Update if needed. **Any rewrite
588608
must follow the format in `~/.claude/rules/pr-message-format.md`
589-
and the "Format of PR bodies, comments, and greptile-review
609+
and the "Format of PR bodies, comments, and AI-review
590610
summaries" section above** — short visible summary, long-form
591611
analysis inside `<details>`, machine-readable provenance inside
592612
HTML comments. If the existing body is a pre-format wall of text,
@@ -681,24 +701,37 @@ Before making any commit in the PR branch:
681701

682702
## Bot classification
683703

684-
Treat these bots as non-blocking (skip unless explicitly asked):
704+
Three buckets, defined in `ghtp_fetch.py`:
705+
706+
**AI reviewers** (`AI_REVIEW_PROVIDERS`) — carry findings, handled in Phase 3:
707+
`greptile-apps[bot]`, `coderabbitai[bot]`.
708+
709+
**Infrastructure bots** (`NON_BLOCKING_BOTS`) — never carry findings, skip
710+
unless explicitly asked: `github-actions[bot]` (CI, surfaced in Phase 2),
711+
`codecov[bot]`, `dependabot[bot]`, `renovate[bot]`, `cla-bot[bot]`,
712+
`stale[bot]`, `pre-commit-ci[bot]`, `deepsource-autofix[bot]`.
713+
714+
**Unknown bots** (`bot_unknown`) — anything else ending in `[bot]`.
715+
**These are not ignorable.** A bot that is in neither list is one nobody
716+
has classified yet, which is exactly how a new review bot's findings get
717+
dropped. Read the comments, then add the login to one of the two lists.
685718

686-
- `greptile-apps[bot]` — handled explicitly in Phase 3
687-
- `github-actions[bot]` — CI status, surfaced in Phase 2
688-
- `codecov[bot]` — coverage comments, informational only
689-
- `dependabot[bot]`, `renovate[bot]` — usually not on your PRs
690-
- `cla-bot[bot]`, `stale[bot]` — process bots, ignore
719+
Everything not ending in `[bot]` is a human and belongs in Phase 1.
691720

692-
Everything else with `user.type == "User"` is a human and belongs in
693-
Phase 1.
721+
**Adding a provider:** add an entry to `AI_REVIEW_PROVIDERS` (login,
722+
`trigger`, `rereview`, `detect_files`), write a `parse_*_findings` body
723+
parser returning `{priority, severity, title}` with priority normalised to
724+
P1/P2/P3, and dispatch it from `parse_findings()`. Nothing in
725+
`ghtp_reply.py` needs changing — replying and resolving are provider-agnostic.
694726

695727
## Quality checks after triage
696728

697729
Before reporting success on a PR:
698730

699731
- [ ] All human threads marked resolved on GitHub
700732
- [ ] CI is green on the HEAD commit
701-
- [ ] Greptile review posted and all P1/P2 findings addressed
733+
- [ ] AI review covers the current HEAD and all P1/P2 findings are addressed
734+
- [ ] `bot_unknown` is empty (every commenting bot is classified)
702735
- [ ] PR title still accurate
703736
- [ ] PR body still accurate
704737
- [ ] Commit history is clean (no `fixup!` commits remain — autosquash ran)

0 commit comments

Comments
 (0)