|
| 1 | +--- |
| 2 | +name: pr-review |
| 3 | +description: Review code changes on the current branch for quality, bugs, performance, and security |
| 4 | +disable-model-invocation: true |
| 5 | +argument-hint: "[optional: LINEAR-TICKET-ID]" |
| 6 | +allowed-tools: Read, Grep, Glob, Bash(git diff:*), Bash(git log:*), Bash(git show:*), Bash(git branch:*), Bash(gh pr:*), Bash(gh api:*), Bash(~/.claude/scripts/fetch-github-pr.sh:*), Bash(~/.claude/scripts/fetch-sentry-data.sh:*), Bash(~/.claude/scripts/fetch-slack-thread.sh:*) |
| 7 | +--- |
| 8 | + |
| 9 | +# Code Review |
| 10 | + |
| 11 | +You are reviewing code changes on the current branch. Your review must be based on the **current state of the code right now**, not on anything you've seen earlier in this conversation. |
| 12 | + |
| 13 | +## CRITICAL: Always Use Fresh Data |
| 14 | + |
| 15 | +**IGNORE any file contents, diffs, or line numbers you may have seen earlier in this conversation.** They may be stale. You MUST re-fetch everything from scratch using the commands below. |
| 16 | + |
| 17 | +## Step 1: Get the Current Diff and PR Context |
| 18 | + |
| 19 | +Run ALL of these commands to get a fresh view: |
| 20 | + |
| 21 | +```bash |
| 22 | +# The authoritative diff -- only review what's in HERE |
| 23 | +git diff main...HEAD |
| 24 | + |
| 25 | +# Recent commits on this branch |
| 26 | +git log --oneline main..HEAD |
| 27 | + |
| 28 | +# PR description and comments |
| 29 | +gh pr view --json number,title,body,comments,reviews,reviewRequests |
| 30 | +``` |
| 31 | + |
| 32 | +Also fetch PR review comments (inline code comments): |
| 33 | + |
| 34 | +```bash |
| 35 | +# Get the PR number |
| 36 | +PR_NUMBER=$(gh pr view --json number -q '.number') |
| 37 | + |
| 38 | +# Fetch all review comments (inline comments on specific lines) |
| 39 | +gh api repos/{owner}/{repo}/pulls/$PR_NUMBER/comments --jq '.[] | {path: .path, line: .line, body: .body, user: .user.login, created_at: .created_at}' |
| 40 | + |
| 41 | +# Fetch review-level comments (general review comments) |
| 42 | +gh api repos/{owner}/{repo}/pulls/$PR_NUMBER/reviews --jq '.[] | {state: .state, body: .body, user: .user.login}' |
| 43 | +``` |
| 44 | + |
| 45 | +## Step 2: Understand Context from PR Comments |
| 46 | + |
| 47 | +Before reviewing, read through the PR comments and review comments. Note **who** said what (by username). |
| 48 | + |
| 49 | +- **Already-addressed feedback**: If a reviewer pointed out an issue and the author has already fixed it (the fix is visible in the current diff), do NOT re-raise it. |
| 50 | +- **Ongoing discussions**: Note any unresolved threads -- your review should take these into account. |
| 51 | +- **Previous approvals/requests for changes**: Understand what reviewers have already looked at. |
| 52 | + |
| 53 | +**IMPORTANT**: Your review is YOUR independent review. Do not take credit for or reference other reviewers' findings as if they were yours. If another reviewer already flagged something, you can note "as [reviewer] pointed out" but do not present their feedback as your own prior review. Your verdict should be based solely on your own analysis of the current code. |
| 54 | + |
| 55 | +## Step 3: Get Requirements Context |
| 56 | + |
| 57 | +Check if a Linear ticket ID was provided as an argument ($ARGUMENTS). If not, try to extract it from the branch name (pattern: `{username}/{linear-ticket}-{title}`). |
| 58 | + |
| 59 | +If a Linear ticket is found: |
| 60 | +- Use Linear MCP tools (`get_issue`) to get the issue details and comments |
| 61 | +- **Check for a parent ticket**: If the issue has a parent issue, fetch the parent too. Our pattern is to have a parent ticket with project-wide requirements and sub-tickets for specific tasks (often one per repo/PR). The parent ticket will contain the full scope of the project, while the sub-ticket scopes what this specific PR should cover. Use both to assess completeness — the PR should fulfill the sub-ticket's scope, and that scope should be a reasonable subset of the parent's backend-related requirements. |
| 62 | +- Look for Sentry links in the description/comments; if found, use Sentry MCP tools to get error details |
| 63 | +- Assess whether the changes fulfill the ticket requirements |
| 64 | + |
| 65 | +If no ticket is found, check the PR description for context on what the changes are meant to accomplish. |
| 66 | + |
| 67 | +## Step 4: Review the Code |
| 68 | + |
| 69 | +Review ONLY the changed lines (from `git diff main...HEAD`). Do not comment on unchanged code. |
| 70 | + |
| 71 | +**When referencing code, always use the file path and quote the actual code snippet** rather than citing line numbers, since line numbers shift as the branch evolves. |
| 72 | + |
| 73 | +### Code Quality |
| 74 | +- Is the code well-structured and maintainable? |
| 75 | +- Does it follow CLAUDE.md conventions? (import grouping, error handling with lib/errors, naming, alphabetization, etc.) |
| 76 | +- Any AI-generated slop? (excessive comments, unnecessary abstractions, over-engineering) |
| 77 | + |
| 78 | +### Performance |
| 79 | +- N+1 queries, inefficient loops, missing indexes for new queries |
| 80 | +- Unbuffered writes in hot paths (especially ClickHouse) |
| 81 | +- Missing LIMIT clauses on potentially large result sets |
| 82 | + |
| 83 | +### Bugs |
| 84 | +- Nil pointer risks (especially on struct pointer params and optional relations) |
| 85 | +- Functions returning `nil, nil` (violates convention) |
| 86 | +- Missing error handling |
| 87 | +- Race conditions in concurrent code paths |
| 88 | + |
| 89 | +### Security |
| 90 | +- Hardcoded secrets or sensitive data exposure |
| 91 | +- Missing input validation on service request structs |
| 92 | + |
| 93 | +### Tests |
| 94 | +- Are there tests for the new/changed code? |
| 95 | +- Do the tests cover edge cases and error paths? |
| 96 | +- Are test assertions specific (not just "no error")? |
| 97 | + |
| 98 | +## Step 5: Present the Review |
| 99 | + |
| 100 | +Structure your review as: |
| 101 | + |
| 102 | +``` |
| 103 | +## Summary |
| 104 | +[1-2 sentences: what this PR does and overall assessment] |
| 105 | +
|
| 106 | +## Requirements Check |
| 107 | +[Does the PR fulfill the Linear ticket / PR description requirements? Any gaps?] |
| 108 | +
|
| 109 | +## Issues |
| 110 | +### Critical (must fix before merge) |
| 111 | +- [blocking issues] |
| 112 | +
|
| 113 | +### Suggestions (nice to have) |
| 114 | +- [non-blocking improvements] |
| 115 | +
|
| 116 | +## Prior Review Activity |
| 117 | +[Summarize what other reviewers have flagged, attributed by name. Note which of their concerns have been addressed in the current code and which remain open.] |
| 118 | +
|
| 119 | +## Verdict |
| 120 | +[LGTM / Needs changes / Needs discussion -- based on YOUR analysis, not other reviewers' findings] |
| 121 | +``` |
| 122 | + |
| 123 | +## Guidelines |
| 124 | + |
| 125 | +- Be concise. Don't pad with praise or filler. |
| 126 | +- Only raise issues that matter. Don't nitpick formatting (that's what linters are for). |
| 127 | +- Quote code snippets rather than referencing line numbers. |
| 128 | +- If PR comments show a discussion was already resolved, don't reopen it. |
| 129 | +- If you're unsure about something, flag it as a question rather than a definitive issue. |
0 commit comments