Skip to content

Commit 9853c35

Browse files
authored
fix(skill): address Claude review comments in /review skill (#543)
* fix(skill): ensure review skill addresses Claude's review comments, not just Greptile's * fix(skill): address Greptile P2 feedback on review skill - Use ANSI-C quoting ($'...') for body with newlines so \n is not passed literally to the GitHub API - Add issues endpoint to the reviewer checklist so issue-style commenters (e.g. @greptileai trigger responses) are not silently missed * fix(skill): add /reviews endpoint to Step 2b conflict-resolution guidance
1 parent ccafc60 commit 9853c35

1 file changed

Lines changed: 33 additions & 10 deletions

File tree

.claude/skills/review/SKILL.md

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ If `CONFLICTING`:
6363
2. **Do not assume which side to keep.** You must fully understand the context of both sides before resolving. If you don't know why a line was added — what feature it supports, what bug it fixes, what reviewer requested it — you cannot resolve the conflict correctly. Before touching any conflict:
6464
- Read the PR description and any linked issues (`gh pr view <number>`) to understand the PR's purpose and scope.
6565
- Check the PR's commit history (`git log --oneline origin/<base-branch>..HEAD -- <file>`) to understand *why* the conflicting line was changed on the PR side. Also check the base branch history (`git log --oneline HEAD..origin/<base-branch> -- <file>`) to understand *why* the base version exists.
66-
- Read Greptile and Claude review comments on the PR (`gh api repos/optave/codegraph/pulls/<number>/comments`, `gh api repos/optave/codegraph/issues/<number>/comments`) — a reviewer may have requested the change that caused the conflict.
66+
- Read Greptile and Claude review comments on the PR (`gh api repos/optave/codegraph/pulls/<number>/comments`, `gh api repos/optave/codegraph/pulls/<number>/reviews`, `gh api repos/optave/codegraph/issues/<number>/comments`) — a reviewer may have requested the change that caused the conflict.
6767
- Check what landed on main that introduced the other side (`git log --oneline HEAD..origin/<base-branch> -- <file>`) and read those PR descriptions too if needed.
6868
- Compare the PR's diff against its merge base (`git diff $(git merge-base origin/<base-branch> HEAD) HEAD -- <file>`) to see which side introduced an intentional change vs. which side carried stale code.
6969
- Only then choose the correct resolution. If the PR deliberately changed a line and main still has the old version, keep the PR's version. If main introduced a fix or new feature the PR doesn't have, keep main's version. If both sides made intentional changes, merge them together manually.
@@ -95,38 +95,61 @@ If any checks are failing:
9595

9696
### 2d. Gather all review comments
9797

98-
Fetch **all** review comments from both Claude and Greptile:
98+
Fetch **all** review comments from both Claude and Greptile. You MUST check all three endpoints — Claude's feedback often appears in the `/reviews` and `/comments` endpoints, not just issue comments:
9999

100100
```bash
101-
# PR review comments (inline code comments)
101+
# PR review comments (inline code comments — Claude and Greptile both use these)
102102
gh api repos/optave/codegraph/pulls/<number>/comments --paginate --jq '.[] | {id: .id, user: .user.login, body: .body, path: .path, line: .line, created_at: .created_at}'
103103

104-
# PR reviews (top-level review bodies)
104+
# PR reviews (top-level review bodies — Claude typically posts CHANGES_REQUESTED or COMMENT reviews here)
105105
gh api repos/optave/codegraph/pulls/<number>/reviews --paginate --jq '.[] | {id: .id, user: .user.login, body: .body, state: .state}'
106106

107-
# Issue-style comments (includes @greptileai trigger responses)
107+
# Issue-style comments (includes @greptileai trigger responses and general discussion)
108108
gh api repos/optave/codegraph/issues/<number>/comments --paginate --jq '.[] | {id: .id, user: .user.login, body: .body, created_at: .created_at}'
109109
```
110110

111-
### 2e. Address every comment
111+
**Important:** Go through the results from ALL three endpoints. Build a complete list of actionable items from every reviewer before starting fixes. Do not skip any reviewer's comments.
112+
113+
### 2e. Address every comment from EVERY reviewer
114+
115+
You must address comments from **all** reviewers — Claude (claude-code-review bot), Greptile, and any humans. Do not only address one reviewer's comments and skip another's. Process each reviewer's feedback systematically.
112116

113117
For **each** review comment — including minor suggestions, nits, style feedback, and optional improvements:
114118

115119
1. **Read the comment carefully.** Understand what the reviewer is asking for.
116120
2. **Read the relevant code** at the file and line referenced.
117121
3. **Make the change.** Even if the comment is marked as "nit" or "suggestion" or "minor" — address it. The goal is zero outstanding comments.
118122
4. **If you disagree** with a suggestion (e.g., it would introduce a bug or contradicts project conventions), do NOT silently ignore it. Reply to the comment explaining why you chose a different approach.
119-
5. **Reply to each comment** explaining what you did:
123+
5. **Reply to each comment** explaining what you did. The reply mechanism depends on where the comment lives:
124+
125+
**For inline PR review comments** (from Claude, Greptile, or humans — these have a `path` and `line`):
120126
```bash
121127
gh api repos/optave/codegraph/pulls/<number>/comments/<comment-id>/replies \
122128
-f body="Fixed — <brief description of what was changed>"
123129
```
124-
For issue-style comments, reply on the issue:
130+
131+
**For top-level PR review bodies** (Claude often leaves a summary review with `CHANGES_REQUESTED` or `COMMENT` state — these come from the `/reviews` endpoint and have no `path`):
132+
```bash
133+
# Reply on the PR conversation thread so the reviewer sees it
134+
gh api repos/optave/codegraph/issues/<number>/comments \
135+
-f body=$'Addressed Claude\'s review feedback:\n- <bullet per item addressed>'
136+
```
137+
138+
**For issue-style comments** (includes @greptileai trigger responses):
125139
```bash
126140
gh api repos/optave/codegraph/issues/<number>/comments \
127141
-f body="Addressed: <summary of changes made>"
128142
```
129143

144+
**Checklist before moving on:** After addressing all comments, verify you haven't missed a reviewer:
145+
```bash
146+
# List all unique reviewers who left comments
147+
gh api repos/optave/codegraph/pulls/<number>/comments --paginate --jq '[.[].user.login] | unique | .[]'
148+
gh api repos/optave/codegraph/pulls/<number>/reviews --paginate --jq '[.[].user.login] | unique | .[]'
149+
gh api repos/optave/codegraph/issues/<number>/comments --paginate --jq '[.[].user.login] | unique | .[]'
150+
# Confirm you addressed comments from EVERY reviewer listed
151+
```
152+
130153
### 2f. Commit and push fixes
131154

132155
After addressing all comments for a PR:
@@ -187,8 +210,8 @@ After processing all PRs, output a summary table:
187210

188211
- **Never rebase.** Always `git merge <base>` to resolve conflicts.
189212
- **Never force-push** unless fixing a commit message that fails commitlint. Amend + force-push is the only way to fix a pushed commit title (messages are part of the SHA). This is safe on feature branches. For all other problems, fix with a new commit.
190-
- **Address ALL comments**, even minor/nit/optional ones. Leave zero unaddressed.
191-
- **Always reply to comments** explaining what was done. Don't just fix silently.
213+
- **Address ALL comments from ALL reviewers** (Claude, Greptile, and humans), even minor/nit/optional ones. Leave zero unaddressed. Do not only respond to one reviewer and skip another.
214+
- **Always reply to comments** explaining what was done. Don't just fix silently. Every reviewer must see a reply on their feedback.
192215
- **Don't re-trigger Greptile if already approved.** If your last reply to a Greptile comment has a positive emoji reaction (👍, ✅, 🎉) from `greptileai`, it's already satisfied — skip re-triggering.
193216
- **Only re-trigger Claude** if you addressed Claude's feedback specifically.
194217
- **No co-author lines** in commit messages.

0 commit comments

Comments
 (0)