Skip to content

Commit 761dfe9

Browse files
improvement(review-pr): add thread management to review skill
Adds Step 2 to the review skill covering all existing Claude Code threads before the diff analysis runs. This fixes the three problems observed on active PRs (e.g. #4820: 15 review submissions, 14 inline comments, many duplicates from claude[bot]): 1. Duplicate comments: SKIP SET built from active threads prevents re-posting the same issue at the same path+line on every synchronize event. 2. Stale outdated threads: threads whose code has changed are auto-resolved via resolveReviewThread GraphQL mutation. 3. Human replies ignored: threads are classified by conversation history: - no_human_reply → skip set only - author_asked_question → Claude answers inline via in_reply_to - justified → accepted silently (substantive technical reasoning) - unjustified_dismissal → challenge reply asking for reasoning; stricter language for security/correctness issues - fix_claimed_but_code_unchanged → reply noting issue persists Dedup uses {path, line, body_excerpt} comparison so a genuinely different issue at the same path+line can still be raised. Pagination handles PRs with more than 100 review threads. Aligned with scality/agent-hub#26 (central workflow) and ring#6462. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 025aae8 commit 761dfe9

File tree

1 file changed

+49
-5
lines changed

1 file changed

+49
-5
lines changed

.claude/skills/review-pr/SKILL.md

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,50 @@ gh pr view <number> --repo <owner/repo> --json title,body,headRefOid,author,file
3232
gh pr diff <number> --repo <owner/repo>
3333
```
3434

35-
2. **Read changed files** to understand the full context around each change (not just the diff hunks).
35+
2. **Manage existing Claude Code threads** (run before reviewing the diff):
3636

37-
3. **Analyze the changes** against these criteria:
37+
Fetch all review threads with full pagination — loop until `hasNextPage` is false, merging nodes across pages:
38+
39+
```bash
40+
gh api graphql -f query='{repository(owner:"<owner>",name:"<repo>"){pullRequest(number:<number>){reviewThreads(first:100){pageInfo{hasNextPage endCursor}nodes{id isResolved isOutdated resolvedBy{login}comments(first:20){nodes{databaseId author{login}body path line}}}}}}}'
41+
```
42+
43+
For pagination add `after:"<endCursor>"` to `reviewThreads(first:100, after:"<endCursor>")` and repeat until `hasNextPage` is false.
44+
45+
For each thread whose **first comment** contains `— Claude Code`:
46+
47+
**OUTDATED** (`isOutdated=true`, `isResolved=false`) → resolve silently, then skip to next thread:
48+
```bash
49+
gh api graphql -f query='mutation{resolveReviewThread(input:{threadId:"<id>"}){thread{isResolved}}}'
50+
```
51+
52+
**ACTIVE** — read ALL comments, find human replies (`author.login != "claude[bot]"`), classify:
53+
54+
- **`no_human_reply`** — no human comments, not resolved → add `{path, line, body_excerpt}` to **SKIP SET**. No further action.
55+
56+
- **`author_asked_question`** — human replied with a direct question (asking for clarification, why it's a problem, what the fix is) → answer in the thread. Add to SKIP SET.
57+
```bash
58+
gh api -X POST -H "Accept: application/vnd.github+json" "repos/<owner/repo>/pulls/<number>/comments" -f body="<answer><br><br>— Claude Code" -F in_reply_to=<databaseId_of_first_comment> -f commit_id="<headRefOid>"
59+
```
60+
61+
- **`justified`** — human reply contains substantive technical reasoning (why issue isn't applicable, acknowledged trade-off, relevant constraint) → accept silently. Add to SKIP SET. Optionally resolve thread if still open.
62+
63+
- **`unjustified_dismissal`** — thread resolved by human with no reply, OR reply is dismissive with no technical reasoning (`"done"`, `"ok"`, `"wontfix"` with nothing substantive) AND flagged code still unchanged in new diff → reply in thread (do NOT post new top-level comment). Add to SKIP SET.
64+
```bash
65+
gh api -X POST -H "Accept: application/vnd.github+json" "repos/<owner/repo>/pulls/<number>/comments" -f body="Issue still present in latest commit. Could you share your reasoning or clarify how it's been mitigated?<br><br>— Claude Code" -F in_reply_to=<databaseId_of_first_comment> -f commit_id="<headRefOid>"
66+
```
67+
For security/correctness issues add: `"This is a security/correctness concern — a brief explanation helps reviewers and future contributors."` Scale to severity: minor issues may be accepted silently.
68+
69+
- **`fix_claimed_but_code_unchanged`** — human claimed fix but code unchanged in new diff → reply in thread. Add to SKIP SET.
70+
```bash
71+
gh api -X POST -H "Accept: application/vnd.github+json" "repos/<owner/repo>/pulls/<number>/comments" -f body="Flagged code still appears at this location in the latest commit. Could you double-check, or clarify if the fix is elsewhere?<br><br>— Claude Code" -F in_reply_to=<databaseId_of_first_comment> -f commit_id="<headRefOid>"
72+
```
73+
74+
All replies: collaborative not adversarial. 1-2 sentences. Name the specific construct.
75+
76+
3. **Read changed files** to understand the full context around each change (not just the diff hunks).
77+
78+
4. **Analyze the changes** against these criteria:
3879
3980
| Area | What to check |
4081
|------|---------------|
@@ -55,13 +96,16 @@ gh pr diff <number> --repo <owner/repo>
5596
| Test coverage | New logic paths have corresponding unit tests; changed functions have updated tests; no test files deleted without replacement, no orphan test that would be never run |
5697
| Test impact | Changes to shared code (`map.jinja`, `constants.py`, CRD types, API types) are reflected in dependent tests across all affected layers |
5798
| BDD / integration | New user-facing behavior has matching behave scenarios in `tests/`; modified Salt orchestrations have updated integration steps |
58-
4. **Deliver your review:**
99+
100+
5. **Deliver your review:**
59101
60102
### If CI mode: post to GitHub
61103
62104
#### Part A: Inline file comments
63105
64-
For each specific issue, post a comment on the exact file and line:
106+
For each issue found, **check the SKIP SET from step 2 first**. The SKIP SET contains `{path, line, body_excerpt}` entries. If an entry exists with the same `path` and `line` AND the `body_excerpt` covers the same root issue → skip. A genuinely different issue at the same path+line may still be posted.
107+
108+
Otherwise post a comment on the exact file and line:
65109
66110
```bash
67111
gh api -X POST -H "Accept: application/vnd.github+json" "repos/<owner/repo>/pulls/<number>/comments" -f body="Your comment<br><br>— Claude Code" -f path="path/to/file" -F line=<line_number> -f side="RIGHT" -f commit_id="<headRefOid>"
@@ -104,7 +148,7 @@ Do not describe or summarize the PR. For each issue, state the problem on one li
104148
- <suggestion>
105149
```
106150
107-
If no issues: just say "LGTM". End with: `Review by Claude Code`
151+
If no new issues and no thread replies needed: just say "LGTM". End with: `Review by Claude Code`
108152
109153
### If local mode: output the review as text
110154

0 commit comments

Comments
 (0)