Review Pull Request
Mode: $ARGUMENTS
If mode is one of the following, adjust the review:
- BUGS: Focus only on logical or other bugs
- SECURITY: Focus only on security issues
- PERFORMANCE: Focus only on performance issues
Approval standard
Approve when the change definitely improves overall code health, even if it isn't perfect. Perfect code doesn't exist — don't block a change because it isn't how you would have written it. If it improves the codebase and follows its conventions, approve.
If the change is too large to review well (~1000+ lines), asking the author to split it is a valid review outcome. Suggest a strategy: stack (small change, next one based on it), by file group, horizontal (shared code first, then consumers), or vertical (one end-to-end slice per PR).
Workflow
- Analyze the diff and pre-loaded PR context
- Read changed files to understand full context
- Review based on mode (or all categories if no mode set)
- Provide structured feedback
Review criteria
Apply all axes (or narrow to the mode above):
- Correctness: Logic bugs, off-by-ones, race conditions, unhandled states, missing error paths
- Readability: Functions <50 lines, nesting <3 levels, no dead code/unused imports
- Security: No exposed secrets, no
any, no unvalidated external data
- Immutability: No push/pop/splice/direct mutation
- Patterns: Consistent with codebase conventions, no reinvented wheels
- Performance: Unnecessary re-renders, O(n²) where O(n) works, missing memoization
- Code smells: match the diff against the baseline in smells.md — always judgement calls, and the repo's documented style overrides the baseline
Output format
Group by severity:
- Critical - must fix before merge (bugs, security vulnerabilities)
- Suggestions - improvements worth considering
- Nit - minor and optional; the author may ignore (formatting, naming taste)
- FYI - informational only, no action needed
- Positives - good patterns to call out
If you have one structural problem and ten nits, the structural problem is the review — lead with it.
Use file:line references for all findings. Include suggested fix for each critical issue.
Rules
- Review ALL changed files, not just the latest commit
- Be specific — label true nitpicks as Nit rather than dropping or inflating them
- Dependency upgrades (when
package.json/lockfile is in the diff): one dependency per change — a bulk bump that breaks hides which package did it; verify against the changelog, not the version number; review the lockfile diff (one direct bump pulls dozens of transitive changes); flag hand-edited lockfiles
Common Rationalizations
| Excuse |
Rebuttal |
| "Too small to review" |
Small changes cause big bugs — review everything |
| "It's just a refactor" |
Refactors break behavior silently — verify contracts preserved |
| "Tests pass so it's fine" |
Tests don't catch readability, security, or design issues |
| "I'll clean it up later" |
Later never comes — fix now or it ships as-is |
Verification
Error Handling
- If
gh pr view fails → run gh auth status to verify authentication; ask user for PR number if not on a PR branch
- If a changed file is deleted in the PR → skip reading it; note it was removed
- If diff is too large → prioritize changed files with highest risk (auth, payments, data mutation)
1---2name: code-review3description: Review a GitHub Pull Request for bugs, security, performance, and code quality. Use when user asks to review a PR or wants pull request feedback. Don't use for reviewing local uncommitted changes, creating new PRs, or merging branches.4---56# Review Pull Request78Mode: $ARGUMENTS910If mode is one of the following, adjust the review:1112- BUGS: Focus only on logical or other bugs13- SECURITY: Focus only on security issues14- PERFORMANCE: Focus only on performance issues1516## Approval standard1718Approve when the change definitely improves overall code health, even if it isn't perfect. Perfect code doesn't exist — don't block a change because it isn't how you would have written it. If it improves the codebase and follows its conventions, approve.1920If the change is too large to review well (~1000+ lines), asking the author to split it is a valid review outcome. Suggest a strategy: stack (small change, next one based on it), by file group, horizontal (shared code first, then consumers), or vertical (one end-to-end slice per PR).2122## Workflow23241. Analyze the diff and pre-loaded PR context252. Read changed files to understand full context263. Review based on mode (or all categories if no mode set)274. Provide structured feedback2829## Review criteria3031Apply all axes (or narrow to the mode above):3233- **Correctness**: Logic bugs, off-by-ones, race conditions, unhandled states, missing error paths34- **Readability**: Functions <50 lines, nesting <3 levels, no dead code/unused imports35- **Security**: No exposed secrets, no `any`, no unvalidated external data36- **Immutability**: No push/pop/splice/direct mutation37- **Patterns**: Consistent with codebase conventions, no reinvented wheels38- **Performance**: Unnecessary re-renders, O(n²) where O(n) works, missing memoization39- **Code smells**: match the diff against the baseline in [smells.md](references/smells.md) — always judgement calls, and the repo's documented style overrides the baseline4041## Output format4243Group by severity:4445- **Critical** - must fix before merge (bugs, security vulnerabilities)46- **Suggestions** - improvements worth considering47- **Nit** - minor and optional; the author may ignore (formatting, naming taste)48- **FYI** - informational only, no action needed49- **Positives** - good patterns to call out5051If you have one structural problem and ten nits, the structural problem _is_ the review — lead with it.5253Use `file:line` references for all findings. Include suggested fix for each critical issue.5455## Rules5657- Review ALL changed files, not just the latest commit58- Be specific — label true nitpicks as **Nit** rather than dropping or inflating them59- **Dependency upgrades** (when `package.json`/lockfile is in the diff): one dependency per change — a bulk bump that breaks hides which package did it; verify against the changelog, not the version number; review the lockfile diff (one direct bump pulls dozens of transitive changes); flag hand-edited lockfiles6061## Common Rationalizations6263| Excuse | Rebuttal |64| ------------------------- | -------------------------------------------------------------- |65| "Too small to review" | Small changes cause big bugs — review everything |66| "It's just a refactor" | Refactors break behavior silently — verify contracts preserved |67| "Tests pass so it's fine" | Tests don't catch readability, security, or design issues |68| "I'll clean it up later" | Later never comes — fix now or it ships as-is |6970## Verification7172- [ ] Every changed file reviewed (not just the diff summary)73- [ ] No critical issue left without a suggested fix74- [ ] Security concerns flagged with specific fix75- [ ] Feedback grouped by severity, not file order76- [ ] Verdict stated against the approval standard — improves code health, or blocked with a reason7778## Error Handling7980- If `gh pr view` fails → run `gh auth status` to verify authentication; ask user for PR number if not on a PR branch81- If a changed file is deleted in the PR → skip reading it; note it was removed82- If diff is too large → prioritize changed files with highest risk (auth, payments, data mutation)
helderberto