chore: add Claude Code skill for code review#372
Conversation
Greptile SummaryThis PR adds a new Claude Code skill ( The majority of issues raised in the prior review round have been addressed — pagination was added to the Key remaining concerns:
|
| Filename | Overview |
|---|---|
| .claude/skills/review-code/SKILL.md | New Claude Code skill for code review — well-structured with most prior review feedback incorporated; a few residual issues remain (parallel Step 6 dependency, .venv linter path in worktree, CLAUDE.md omitted from Step 2, gh pr checkout fallback pointing to a non-existent path), and the external-fork fallback condition is inaccurate. |
Sequence Diagram
sequenceDiagram
participant User
participant Skill as review-code skill
participant GH as GitHub CLI (gh)
participant Git as git
participant FS as File System (/tmp)
participant Ruff as .venv/bin/ruff
User->>Skill: /review-code [pr-number | instructions]
alt PR Mode
Skill->>GH: gh pr view <number> (details, commits)
Skill->>GH: gh pr diff <number> (diff + file list)
Skill->>GH: gh api pulls/<n>/comments --paginate (existing feedback)
Skill->>GH: gh api pulls/<n>/reviews --paginate (existing reviews)
Skill->>GH: gh repo view (owner/repo)
Skill->>Git: git fetch origin pull/<n>/head:pr-<n> --force
Skill->>FS: git worktree add /tmp/review-<n> pr-<n>
else Branch Mode
Skill->>Git: git fetch origin <base>
Skill->>Git: git branch --show-current
Skill->>Git: git log origin/<base>..HEAD
Skill->>Git: git diff origin/<base>..HEAD
Skill->>Git: git status --porcelain
end
Skill->>FS: Read AGENTS.md (project guidelines)
Skill->>FS: Read each changed file (Pass 1–3)
Skill->>Ruff: .venv/bin/ruff check <changed-files>
Skill->>FS: Write /tmp/review-<id>.md
Skill->>User: Display structured review (Critical / Warnings / Suggestions)
Last reviewed commit: 916ccf9
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
right now the skill ends at the summary, but the review is often just the starting point. could be nice to offer next steps, like:
- deep dive into a specific file
- check a specific concern in more detail
- install deps locally (
make install-dev) and write some additional smoke tests
(other suggestions moved to inline comments on the relevant lines)
.claude/skills/review-code/SKILL.md
Outdated
| @@ -0,0 +1,260 @@ | |||
| --- | |||
| name: review-code | |||
| description: Perform a thorough code review of the current branch or a GitHub PR by number. Use when the user asks to review code changes, audit a branch or PR, or wants a pre-merge quality check. | |||
There was a problem hiding this comment.
The last bet about "Use when..." is only needed with model invocation, right? Of course, having it work when you ask for a review is an option. Just noting that it is a bit inconsistent with the setting below.
|
i ran through review comments on the last ~60 merged PRs (thanks claude code!) to see what themes keep coming up. might be worth encoding the most frequent ones into the review passes so the skill catches them proactively. here are the top ones: testing (pass 3):
correctness (pass 1):
design (pass 2):
meta / ai-specific:
not all of these need to go in, but the testing ones especially come up a lot. could add them as bullet points under the relevant passes. |
.claude/skills/review-code/SKILL.md
Outdated
| git fetch origin pull/<number>/head:pr-<number> | ||
| git worktree add checkouts/review-<number> pr-<number> | ||
| # Run the rest of the review from checkouts/review-<number>; when done: git worktree remove checkouts/review-<number> |
There was a problem hiding this comment.
Repeated review of the same PR will fail due to stale local branch
git fetch origin pull/<number>/head:pr-<number> creates a local branch named pr-<number>. If the reviewer has previously run the skill for the same PR number (and the branch was not cleaned up), the fetch will fail with:
fatal: Cannot update existing branch 'pr-<number>' without --force
The branch cleanup instruction is only in an inline comment (# when done: git worktree remove ...), which is easy to overlook, and removing the worktree does not delete the local tracking branch.
Consider adding --force to the fetch and explicitly documenting cleanup of the local branch:
| git fetch origin pull/<number>/head:pr-<number> | |
| git worktree add checkouts/review-<number> pr-<number> | |
| # Run the rest of the review from checkouts/review-<number>; when done: git worktree remove checkouts/review-<number> | |
| git fetch origin pull/<number>/head:pr-<number> --force | |
| git worktree add checkouts/review-<number> pr-<number> | |
| # Cleanup when done: git worktree remove checkouts/review-<number> && git branch -d pr-<number> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .claude/skills/review-code/SKILL.md
Line: 46-48
Comment:
**Repeated review of the same PR will fail due to stale local branch**
`git fetch origin pull/<number>/head:pr-<number>` creates a local branch named `pr-<number>`. If the reviewer has previously run the skill for the same PR number (and the branch was not cleaned up), the fetch will fail with:
```
fatal: Cannot update existing branch 'pr-<number>' without --force
```
The branch cleanup instruction is only in an inline comment (`# when done: git worktree remove ...`), which is easy to overlook, and removing the worktree does not delete the local tracking branch.
Consider adding `--force` to the fetch and explicitly documenting cleanup of the local branch:
```suggestion
git fetch origin pull/<number>/head:pr-<number> --force
git worktree add checkouts/review-<number> pr-<number>
# Cleanup when done: git worktree remove checkouts/review-<number> && git branch -d pr-<number>
```
How can I resolve this? If you propose a fix, please make it concise.
nabinchha
dismissed stale reviews from andreatgretel and johnnygreco
via
f3b5e53
| # Cleanup when done: git worktree remove /tmp/review-<number> && git branch -D pr-<number> | ||
| ``` | ||
|
|
||
| If worktrees aren't suitable, you can use `gh pr checkout <number>` (this switches your current branch — only if you have no uncommitted work). Run the rest of the review from `/tmp/review-<number>`. |
There was a problem hiding this comment.
gh pr checkout fallback references the wrong working directory
gh pr checkout <number> checks out the PR branch directly into the current repository — it does not create any directory at /tmp/review-<number>. That path only exists when the worktree approach is used. If an agent follows this fallback path, the instruction to "Run the rest of the review from /tmp/review-<number>" will produce a "No such file or directory" error, and every subsequent relative path operation will fail.
The fallback should instruct the agent to continue from the repository root:
| If worktrees aren't suitable, you can use `gh pr checkout <number>` (this switches your current branch — only if you have no uncommitted work). Run the rest of the review from `/tmp/review-<number>`. | |
| If worktrees aren't suitable, you can use `gh pr checkout <number>` (this switches your current branch — only if you have no uncommitted work). Run the rest of the review from the repository root. When done, return to your original branch with `git checkout -`. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .claude/skills/review-code/SKILL.md
Line: 51
Comment:
**`gh pr checkout` fallback references the wrong working directory**
`gh pr checkout <number>` checks out the PR branch directly into the current repository — it does **not** create any directory at `/tmp/review-<number>`. That path only exists when the worktree approach is used. If an agent follows this fallback path, the instruction to "Run the rest of the review from `/tmp/review-<number>`" will produce a "No such file or directory" error, and every subsequent relative path operation will fail.
The fallback should instruct the agent to continue from the repository root:
```suggestion
If worktrees aren't suitable, you can use `gh pr checkout <number>` (this switches your current branch — only if you have no uncommitted work). Run the rest of the review from the repository root. When done, return to your original branch with `git checkout -`.
```
How can I resolve this? If you propose a fix, please make it concise.| 5. **Existing inline review comments**: `gh api repos/{owner}/{repo}/pulls/<number>/comments --paginate --jq '.[].body'` | ||
| 5b. **Existing PR-level reviews** (top-level review bodies from "Review changes"): `gh api repos/{owner}/{repo}/pulls/<number>/reviews --paginate --jq '.[].body'` | ||
| 6. **Repo info**: `gh repo view --json nameWithOwner -q '.nameWithOwner'` |
There was a problem hiding this comment.
Steps 5/5b depend on step 6's output but all are run in parallel
Steps 5 and 5b contain the literal placeholder {owner}/{repo} in the gh api URL, but that value is only available after step 6 (gh repo view --json nameWithOwner) completes. The instruction to run all six commands in parallel means the API calls would be constructed before the owner/repo is known.
In practice an LLM agent might infer the remote from the git context, but the skill should make this ordering dependency explicit to avoid silent failures on repos where the remote origin doesn't match the default GitHub host.
Consider sequencing step 6 first (it's instant), then running steps 1–5b in parallel with the owner/repo substituted in:
| 5. **Existing inline review comments**: `gh api repos/{owner}/{repo}/pulls/<number>/comments --paginate --jq '.[].body'` | |
| 5b. **Existing PR-level reviews** (top-level review bodies from "Review changes"): `gh api repos/{owner}/{repo}/pulls/<number>/reviews --paginate --jq '.[].body'` | |
| 6. **Repo info**: `gh repo view --json nameWithOwner -q '.nameWithOwner'` | |
| 0. **Repo info** (run first): `gh repo view --json nameWithOwner -q '.nameWithOwner'` | |
| Then run in parallel, substituting `<owner>/<repo>` from step 0: | |
| 1. PR details … | |
| … | |
| 5. Existing inline review comments: `gh api repos/<owner>/<repo>/pulls/<number>/comments --paginate --jq '.[].body'` | |
| 5b. Existing PR-level reviews: `gh api repos/<owner>/<repo>/pulls/<number>/reviews --paginate --jq '.[].body'` |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .claude/skills/review-code/SKILL.md
Line: 39-41
Comment:
**Steps 5/5b depend on step 6's output but all are run in parallel**
Steps 5 and 5b contain the literal placeholder `{owner}/{repo}` in the `gh api` URL, but that value is only available after step 6 (`gh repo view --json nameWithOwner`) completes. The instruction to run all six commands in parallel means the API calls would be constructed before the owner/repo is known.
In practice an LLM agent might infer the remote from the git context, but the skill should make this ordering dependency explicit to avoid silent failures on repos where the remote origin doesn't match the default GitHub host.
Consider sequencing step 6 first (it's instant), then running steps 1–5b in parallel with the owner/repo substituted in:
```suggestion
0. **Repo info** (run first): `gh repo view --json nameWithOwner -q '.nameWithOwner'`
Then run in parallel, substituting `<owner>/<repo>` from step 0:
1. PR details …
…
5. Existing inline review comments: `gh api repos/<owner>/<repo>/pulls/<number>/comments --paginate --jq '.[].body'`
5b. Existing PR-level reviews: `gh api repos/<owner>/<repo>/pulls/<number>/reviews --paginate --jq '.[].body'`
```
How can I resolve this? If you propose a fix, please make it concise.| ```bash | ||
| .venv/bin/ruff check <changed-files> | ||
| .venv/bin/ruff format --check <changed-files> |
There was a problem hiding this comment.
.venv/bin/ruff is a relative path that won't resolve inside the worktree
Step 5 uses .venv/bin/ruff (a relative path). When the agent is working from the worktree at /tmp/review-<number>, there is no .venv directory there — it lives in the original repository root. The command will fail with "No such file or directory".
Either use the absolute path to the venv or note that the linter should be invoked from the original repo root (not from the worktree):
| ```bash | |
| .venv/bin/ruff check <changed-files> | |
| .venv/bin/ruff format --check <changed-files> | |
| $(git rev-parse --show-toplevel)/.venv/bin/ruff check <changed-files> | |
| $(git rev-parse --show-toplevel)/.venv/bin/ruff format --check <changed-files> |
Alternatively, if uv is available globally this is avoided entirely with uv run ruff check <changed-files>, which discovers the project venv regardless of the current directory.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .claude/skills/review-code/SKILL.md
Line: 184-186
Comment:
**`.venv/bin/ruff` is a relative path that won't resolve inside the worktree**
Step 5 uses `.venv/bin/ruff` (a relative path). When the agent is working from the worktree at `/tmp/review-<number>`, there is no `.venv` directory there — it lives in the original repository root. The command will fail with "No such file or directory".
Either use the absolute path to the venv or note that the linter should be invoked from the original repo root (not from the worktree):
```suggestion
$(git rev-parse --show-toplevel)/.venv/bin/ruff check <changed-files>
$(git rev-parse --show-toplevel)/.venv/bin/ruff format --check <changed-files>
```
Alternatively, if `uv` is available globally this is avoided entirely with `uv run ruff check <changed-files>`, which discovers the project venv regardless of the current directory.
How can I resolve this? If you propose a fix, please make it concise.
| If worktrees aren't suitable, you can use `gh pr checkout <number>` (this switches your current branch — only if you have no uncommitted work). Run the rest of the review from `/tmp/review-<number>`. | ||
|
|
||
| If checkout isn't possible (e.g., external fork), use `gh api` to fetch file contents: | ||
|
|
||
| ```bash | ||
| gh api repos/{owner}/{repo}/contents/{path}?ref={head-branch} --jq '.content' | base64 --decode | ||
| ``` |
There was a problem hiding this comment.
External-fork fallback condition is inaccurate
The comment "If checkout isn't possible (e.g., external fork)" on line 51 and the gh api content-fetch fallback on lines 53–57 are presented as if external forks prevent the primary worktree path from working. That is not the case.
GitHub exposes a synthetic ref refs/pull/<number>/head for every open PR — including those from external forks — and that ref is fetchable via the upstream origin remote. This means:
git fetch origin pull/<number>/head:pr-<number> --force
git worktree add /tmp/review-<number> pr-<number>works equally well for same-repo branches and external forks, as long as the PR is open and the runner has network access to origin.
An agent reading this skill may skip the efficient worktree path and fall back to slow, per-file gh api calls for every external-fork PR unnecessarily.
The fallback condition should describe the cases where the worktree path genuinely fails (e.g. origin is not a GitHub remote, the PR ref has been garbage-collected after a very old close, or the runner has no git access) rather than citing "external fork":
| If worktrees aren't suitable, you can use `gh pr checkout <number>` (this switches your current branch — only if you have no uncommitted work). Run the rest of the review from `/tmp/review-<number>`. | |
| If checkout isn't possible (e.g., external fork), use `gh api` to fetch file contents: | |
| ```bash | |
| gh api repos/{owner}/{repo}/contents/{path}?ref={head-branch} --jq '.content' | base64 --decode | |
| ``` | |
| If the worktree approach fails for any reason (e.g. the runner has no direct `git` access to the remote, or the PR ref is no longer available), you can use `gh api` to fetch file contents directly: |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .claude/skills/review-code/SKILL.md
Line: 51-57
Comment:
**External-fork fallback condition is inaccurate**
The comment "If checkout isn't possible (e.g., external fork)" on line 51 and the `gh api` content-fetch fallback on lines 53–57 are presented as if external forks prevent the primary worktree path from working. That is not the case.
GitHub exposes a synthetic ref `refs/pull/<number>/head` for **every** open PR — including those from external forks — and that ref is fetchable via the upstream `origin` remote. This means:
```bash
git fetch origin pull/<number>/head:pr-<number> --force
git worktree add /tmp/review-<number> pr-<number>
```
works equally well for same-repo branches and external forks, as long as the PR is open and the runner has network access to `origin`.
An agent reading this skill may skip the efficient worktree path and fall back to slow, per-file `gh api` calls for every external-fork PR unnecessarily.
The fallback condition should describe the cases where the worktree path genuinely fails (e.g. `origin` is not a GitHub remote, the PR ref has been garbage-collected after a very old close, or the runner has no `git` access) rather than citing "external fork":
```suggestion
If the worktree approach fails for any reason (e.g. the runner has no direct `git` access to the remote, or the PR ref is no longer available), you can use `gh api` to fetch file contents directly:
```
How can I resolve this? If you propose a fix, please make it concise.3 participants
📋 Summary
Adds a new Claude Code skill (
review-code) that enables comprehensive code reviews of the current branch or a specific GitHub PR directly from the CLI.🔄 Changes
✨ Added
.claude/skills/review-code/SKILL.md— New skill definition with:AGENTS.mdruff🤖 Generated with AI