review-pr

name	review-pr
description	Address PR review comments — fetch all unresolved comments, fix the issues, validate with `uv run poe check`, commit, push, and reply to each comment on GitHub. Use when asked to handle PR review feedback.
argument-hint	[PR number]
allowed-tools	["Bash(gh api *)","Bash(gh pr *)","Bash(git status)","Bash(git diff *)","Bash(git log *)","Bash(git add *)","Bash(git commit *)","Bash(git push *)","Bash(git rebase *)","Bash(git stash *)","Bash(git fetch *)","Bash(git merge *)","Bash(uv run poe check)","Bash(uv run poe agent-check)","Bash(uv run poe fix)","Bash(./.claude/skills/review-pr/scripts/fetch-review-threads.sh*)"]

/review-pr — PR Review Comment Workflow

PURPOSE

Address every unresolved PR review comment: fix code, validate, commit, push, reply.

CRITICAL

• Rebase on the target branch before fixing — no exceptions — if the PR shows mergeStateStatus=BEHIND or git log HEAD..origin/<base> shows anything, the branch is stale. Rebase before reading comments or making code changes. Reasons: (a) some comments may already be obsolete after the base advanced and you'd waste time addressing them; (b) fixes pushed onto a stale branch trigger a new CI run against the still-stale base; (c) auto-merge stays blocked on BEHIND status even when CI is green and all threads are resolved. Same check applies before every --force-with-lease after a fixup. See Phase 1b.
• Fix first, reply after push — replies confirm the fix is live; never reply on stale code.
• Reply to every unresolved comment — none left without a response.
• No shortcuts — never --no-verify, noqa, type: ignore, or @pytest.mark.skip to make validation pass.
• Reply in thread, not on the PR — use POST /repos/{owner}/{repo}/pulls/{number}/comments/{comment_id}/replies (or the in_reply_to body param on the comments endpoint). Plain gh pr comment loses thread context.
• File issues for deferred work — every "acknowledged but deferring" reply must reference a GitHub issue (gh issue create first; include the number in the reply).
• Clean history via rebase — fold review fixes into target commits with fixup! + --autosquash. Don't squash everything into one; preserve meaningful boundaries.

STANDARD PATH

1. Identify the PR — $ARGUMENTS or auto-detect; capture base branch (Phase 1).
2. Check mergeability — resolve conflicts and CI failures BEFORE addressing comments (Phase 1b).
3. Fetch all comments — REST + GraphQL for resolved status (Phase 2).
4. Triage each unresolved comment — fix needed / already fixed / acknowledge (Phase 3).
5. Fix all issues — code changes + uv run poe check clean (Phase 4).
6. Commit, rebase, push — fixup commits + rebase --autosquash + push --force-with-lease (Phase 5).
7. Reply to every comment — using the correct replies endpoint (Phase 6).
8. Update PR description — reflect any scope changes (Phase 7).
9. Summary — counts, files changed, validation result (Phase 8).

See phase detail below.

EDGE CASES

• Comment has prior replies — check whether the issue was actually resolved. If not, fix it and add a new reply.
• Reviewer feedback contradicts project standards — acknowledge, explain the constraint, link CLAUDE.md or ADR. Don't dismiss; don't capitulate to a wrong direction.
• Cross-cutting fix — if a fix doesn't map to one original commit, create a standalone descriptive commit instead of forcing a fixup!.

Phase 1: Identify the PR

If $ARGUMENTS is provided and non-empty, use it as the PR number. Otherwise, auto-detect from the current branch:

gh pr view --json number,url,title,baseRefName

Determine the repo owner/name:

gh repo view --json nameWithOwner --jq '.nameWithOwner'

If no PR is found, tell the user and stop.

Store the PR's base branch (from baseRefName) — use origin/<baseRefName> as the rebase/log target throughout the workflow. Do not assume main.

Phase 1b: Sync branch with target + check CI

Check out the PR's head branch first — /review-pr is often invoked with an explicit PR number while the current checkout is a different branch (or main). Without this step the rebase-status check would compare the wrong HEAD:

# Determine the PR's head branch.
head_ref=$(gh pr view {number} --json headRefName --jq '.headRefName')

# Switch to it (gh pr checkout creates/updates the local branch).
gh pr checkout {number}

# Sanity-check: HEAD should now be on the PR's head branch.
test "$(git branch --show-current)" = "$head_ref"

Now check the PR's mergeability and rebase status against its base:

gh pr view {number} --json mergeable,mergeStateStatus,statusCheckRollup
git fetch origin <baseRefName>
git log HEAD..origin/<baseRefName> --oneline

If the branch is behind the target (commits appear in HEAD..origin/<base> OR mergeStateStatus=BEHIND):

Rebase before addressing any review comments. This step is mandatory (see CRITICAL).

# Rebase onto the latest base. (You're already on the PR's head branch
# from the gh pr checkout above.)
git rebase origin/<baseRefName>

# Handle uv.lock / other drift bundled in.
git status --short  # if uv.lock shows up, stage it into the rebased commit head
git add uv.lock  # if applicable
git commit --amend --no-edit  # or rebase --continue if mid-rebase

# Force-push the rebased branch.
git push --force-with-lease origin HEAD:refs/heads/<branch-name>

After the rebase, re-fetch the comment list — some comments may now reference deleted/moved lines and need to be re-classified or marked as resolved.

If there are merge conflicts (mergeable: CONFLICTING):

The rebase above may have surfaced conflicts. If so:

1. Resolve conflicts by reading each conflicted file, understanding both sides, and keeping the correct resolution (usually our branch's intent integrated with the base branch's changes).
2. After resolving, stage the files and continue the rebase:

   git add <resolved files>
   git rebase --continue
   

3. If the conflicts originated from an explicit git merge instead (e.g., GitHub's "Update branch" button created a merge commit on the remote), pull that merge first with git pull --no-rebase and resolve normally.

If CI is failing (mergeStateStatus: not SUCCESS):

1. Check what's failing:

   gh pr checks {number}
   

2. If it's a code issue (lint, type-check, test failure), fix it as part of Phase 4.
3. If it's an infrastructure issue (flaky CI, timeout), note it in the summary but don't block on it.

Always sync the branch and resolve conflicts/build failures before addressing review comments — review comments may no longer apply after a rebase + base advance.

Phase 2: Fetch all review comments

Fetch every review comment on the PR:

gh api repos/{owner}/{repo}/pulls/{number}/comments --paginate

Also fetch review threads to check resolved status — use the helper script (avoids inline GraphQL drift and paginates correctly through both reviewThreads and per-thread comments, so PRs with >100 threads or >100 comments per thread don't silently drop entries):

./.claude/skills/review-pr/scripts/fetch-review-threads.sh {owner} {repo} {number}

The script returns a JSON array of threads with isResolved plus the comments in each thread (id, databaseId, body, path, line, author).

Present a summary table of all comments:

#	Status	File:Line	Author	Comment (truncated)
1	unresolved	src/foo.py:42	reviewer	"Consider using..."
2	resolved	src/bar.py:10	reviewer	"Typo in variable..."

Skip resolved threads — only work on unresolved comments.

Phase 3: Triage each unresolved comment

For each unresolved comment:

1. Read the affected file and surrounding context
2. Check if already fixed — the code may have changed since the comment was posted
3. Classify the comment:
   • fix needed — code change required
   • already fixed — issue was addressed in a prior commit, just needs a reply
   • acknowledge — valid point but deferring, or explaining why current approach is correct

If a comment has prior replies, check whether the replies actually resolved the concern. If not, treat it as still needing action.

Phase 4: Fix all issues

Make all necessary code changes across affected files. Remember this repo's constraints:

• NEVER edit generated files (api/**/*.py, models/**/*.py, client.py)
• Use unwrap_unset(), unwrap_as(), is_success() helpers per CLAUDE.md patterns
• Resilience is at the transport layer — don't wrap API methods with retries

Then validate:

uv run poe check

This runs format + lint + type-check + tests. ALL must pass. If validation fails:

1. Run uv run poe fix for auto-fixable lint/format issues
2. Fix remaining issues manually
3. Re-run uv run poe check until clean

Phase 5: Commit, rebase, and push

The goal is a clean commit history — review fixes should be folded into the commits they logically belong to, not piled on as separate "fix review comments" commits. Don't squash everything into one commit; preserve meaningful commit boundaries.

Step 1: Stage and commit fixes

Stage only the files that were changed (never use git add -A or git add .):

git add <file1> <file2> ...

Create a temporary fixup commit:

git commit -m "$(cat <<'EOF'
fixup! <original commit subject line this fix belongs to>

- Description of fix 1
- Description of fix 2

Co-Authored-By: Claude Opus 4.6 (1M context) <[e-mail]>
EOF
)"

If fixes span multiple original commits, create multiple fixup commits — one per target commit.

If fixes don't clearly map to an existing commit (e.g., they address a cross-cutting concern), create a standalone commit with a descriptive message instead.

Step 2: Rebase to fold fixups into their target commits

git fetch origin <baseRefName>
git rebase --autosquash origin/<baseRefName>

This automatically reorders and squashes fixup! commits into their targets. After the rebase, verify with:

git log origin/<baseRefName>..HEAD --oneline

The result should be a clean set of logical commits — no "fix review comments" or "address feedback" commits.

Step 3: Force-push the rebased branch

Since the branch is already pushed, a force-push is required after rebasing:

git push --force-with-lease

Use --force-with-lease (not --force) to avoid overwriting changes pushed by others.

NEVER reply to comments before pushing. Replies confirm the fix is live.

Phase 6: Reply to each comment

After pushing, reply to every unresolved comment using:

gh api repos/{owner}/{repo}/pulls/{number}/comments/{comment_id}/replies \
  -X POST -f body='...'

Reply format by classification:

• Code fix: "Fixed — [describe what was changed]. [mention tests if added]"
• Already fixed: "This was already addressed in [commit hash] — [brief explanation]"
• Acknowledged/deferred: "Acknowledged — [reason for deferring]. Tracked in #NNN." (must include a GitHub issue link)

Reply to EVERY unresolved comment. None should be left without a response.

Phase 7: Update PR description

After fixing review comments (especially if the scope has changed), update the PR description to reflect the current state:

gh pr edit {number} --body "$(cat <<'EOF'
## Summary
<updated bullet points reflecting current scope>

## Test plan
<updated checklist>

🤖 Generated with [Claude Code](https://claude.com/claude-code)
EOF
)"

Review the commit log (git log origin/<baseRefName>..HEAD --oneline) to ensure the description covers all changes, not just the original PR scope.

Phase 8: Summary

Print a final summary for the user:

• Number of comments addressed (fixed / already fixed / acknowledged)
• Files changed
• Validation result (tests, lint, type-check)
• Any comments that couldn't be addressed, with explanation

Important Rules

• Fix first, reply after push — never reply before the code is pushed
• Reply to every comment — even if just acknowledging
• No shortcuts — uv run poe check must pass; never use --no-verify, noqa, or type: ignore
• Be specific in replies — describe what changed, don't just say "done"
• Clean history via rebase — fold review fixes into the commits they belong to using fixup! + --autosquash; don't pile on "fix review comments" commits, but also don't squash everything into one commit — preserve meaningful commit boundaries
• Evaluate existing replies — if a comment already has replies, check whether the issue was actually resolved; if not, fix it and add a new reply
• No ignores or suppressions — never use noqa, type: ignore, or skip tests to make validation pass
• File issues for deferred work — if a review comment identifies a valid issue that is out of scope or too large to fix in this PR, create a GitHub issue with gh issue create before replying. The reply MUST include the issue number. Never defer work without a tracking issue.