review-pr

name: review-pr description: Perform a comprehensive code review of a pull request argument-hint: "[PR_number|URL]" effort: high disable-model-invocation: true

Review Pull Request

Perform a comprehensive code review of a pull request.

Instructions

1. Get PR information: gh pr view $ARGUMENTS --json title,body,files,additions,deletions
2. Review each changed file
3. Provide structured feedback

Review Checklist

Code Quality

• Code is readable and well-organized
• Functions are appropriately sized
• No code duplication
• Meaningful variable/function names

Functionality

• Logic is correct
• Edge cases handled
• Error handling is comprehensive
• No obvious bugs

Security

• No hardcoded secrets
• Input validation present
• No injection vulnerabilities
• Authorization checks in place

Testing

• Tests added for new code
• Existing tests still pass
• Edge cases tested

Documentation

• Code is self-documenting or commented
• README updated if needed
• API changes documented

Output Format

## PR Review: #[number] - [title]

### Summary
[1-2 sentence overview]

### Approval Status
[ ] Approved
[ ] Approved with suggestions
[ ] Changes requested

### Findings

#### Critical (Must Fix)
- [ ] [Issue description] - `file:line`

#### Suggestions (Should Consider)
- [ ] [Improvement] - `file:line`

#### Nitpicks (Optional)
- [ ] [Minor suggestion] - `file:line`

### Positive Highlights
- [What's done well]

### Questions
- [Clarifications needed]

Usage

/review-pr 123
/review-pr https://github.com/owner/repo/pull/123

Advanced: Multi-Agent Review

For production-grade reviews requiring specialized perspectives and anti-hallucination safeguards.

Pre-flight Check

Before reviewing, check if this is a follow-up pass to avoid repeating suggestions:

# Detect if Claude already reviewed this PR
git log --oneline -10 | grep "Co-Authored-By: Claude"

If detected, note: "This appears to be a follow-up pass. I'll focus on new issues and avoid repeating previous suggestions."

Scope Drift Detection

Cross-reference the PR diff against the original plan to catch unintended changes.

# Detect current branch
BRANCH=$(git branch --show-current)

# Search for a plan file associated with this branch
ls ~/.claude/plans/ 2>/dev/null | grep -i "$BRANCH" | head -3

# Files actually changed in this PR
git diff --stat origin/main...HEAD | head -30

If a plan file exists for this branch:

1. Read the plan file: what was the stated scope?
2. Compare stated scope vs actual git diff --stat
3. Flag files changed that were NOT mentioned in the plan

Output format:

SCOPE DRIFT CHECK
─────────────────────────────────────────
Plan scope:    [what the plan said would change]
Actual diff:   [files actually changed]
Drift:         [files changed outside plan scope, if any]
Verdict:       IN SCOPE / DRIFT DETECTED

If no plan file exists: note "No plan file found for this branch, skipping scope drift check."

Multi-Agent Specialization

Launch 3 parallel specialized agents (see Split Role Sub-Agents):

Agent 1: Consistency Auditor

Focus: DRY violations, duplicate logic, pattern inconsistencies
Check for:
- Duplicated code blocks (>5 lines similar)
- Inconsistent naming conventions
- Pattern violations (if project uses X pattern, enforce it)

Agent 2: SOLID Principles Analyst

Focus: Single Responsibility Principle violations, complexity
Check for:
- Functions >50 lines (likely doing too much)
- Nested conditionals >3 levels deep
- Cyclomatic complexity >10
- Mixed concerns in single component

Agent 3: Defensive Code Auditor

Focus: Silent failures, masked bugs, hidden fallbacks, LLM output trust boundary
Check for:
- Empty catch blocks: try { } catch (e) { } // swallows error
- Silent fallbacks: return data || DEFAULT // hides missing data
- Unchecked null/undefined: user.name without validation
- Ignored promise rejections: async fn without .catch()

LLM Output Trust Boundary (especially relevant in AI-assisted codebases):
- LLM-generated values (emails, URLs, names, IDs) written to DB or passed to
  downstream functions without format validation; add lightweight guards
  (email regex, URL parsing, .trim()) before persisting
- Structured tool output (arrays, objects from AI tools) accepted without
  type/shape checks before database writes or rendering
- AI-generated SQL or code strings executed without sanitization

Anti-Hallucination Rules

Verify before asserting:

• Use Grep or Glob to verify patterns before recommending them
• If suggesting "use existing UserService pattern", confirm UserService exists first
• Never claim "project uses X" without checking actual codebase

Occurrence rule:

• Pattern with >10 occurrences = established (Suggestion level)
• Pattern with <3 occurrences = not established (Can Skip or ask maintainer)
• Read full file context, not just diff lines

Uncertainty markers:

• Use "❓ To verify:" when unsure about project conventions
• Use "💡 Consider:" for optional improvements
• Use "🔴 Must fix:" only for critical bugs/security

Reconciliation

After agents report findings:

1. Deduplicate: Remove overlapping suggestions across agents
2. Prioritize existing patterns: If codebase uses pattern X, recommend X (not ideal pattern Y)
3. Mark skipped suggestions: "Skipping [suggestion] because project uses [alternative pattern]"
4. Track reasoning: Document why suggestion was kept or skipped

Severity Classification

🔴 Must Fix (Blockers)
- Security vulnerabilities
- Data loss risks
- Breaking changes without migration
- Silent failures masking bugs

🟡 Should Fix (Improvements)
- SOLID violations causing maintenance issues
- DRY violations (>3 duplicates)
- Performance bottlenecks (N+1 queries)
- Missing error handling for critical paths

🟢 Can Skip (Nice-to-haves)
- Style inconsistencies (if no linter)
- Minor naming improvements
- Overly nested code (if <3 levels)
- Documentation gaps (if code self-documenting)

Fix-First Heuristic

Determine whether to auto-fix each finding or surface it for user decision.

AUTO-FIX (apply without asking):          ASK (needs human judgment):
├─ Dead code / unused variables            ├─ Security changes (auth, XSS, injection)
├─ N+1 queries (missing eager loading)     ├─ Race conditions
├─ Stale comments contradicting code       ├─ Design decisions
├─ Magic numbers → named constants         ├─ Large fixes (>20 lines changed)
├─ Missing import / path mismatches        ├─ Enum completeness
├─ Variables assigned but never read       ├─ Anything removing functionality
└─ Obvious version/doc mismatches          └─ User-visible behavior changes

Rule: If a senior engineer would apply the fix in 30 seconds without discussion, it's AUTO-FIX. If reasonable engineers could disagree, it's ASK.

After agents report findings:

1. Apply all AUTO-FIX items immediately with minimal targeted edits
2. Batch all ASK items into a single user decision (not one question per item)

Auto-Fix Loop (Optional)

For automated convergence:

Review → Identify issues → Fix → Re-review → Repeat until minimal changes

Safeguards:
- Max 3 iterations to prevent infinite loops
- Run tsc/lint check before each iteration
- Skip auto-fix for protected files (package.json, migrations, etc.)

Example prompt:

Review this PR with auto-fix enabled:
1. Review using 3 agents above
2. Fix all 🔴 Must Fix issues
3. Re-review to verify fixes
4. Repeat for 🟡 Should Fix (max 2 more iterations)
5. Stop when only 🟢 Can Skip remain

Conditional Context Loading

Load additional context based on diff content (stack-agnostic):

Integration with Existing Tools

SE-CoVe Plugin: Use for general fact-checking of review claims (complementary to anti-hallucination rules above)

Worktrunk: For codebase-wide pattern analysis before suggesting changes

AST-grep: For structural pattern matching (e.g., find all similar try/catch blocks)

Sources

• Base template: Claude Code Ultimate Guide
• Multi-agent review: Pat Cullen (Jan 2026)
• Anti-hallucination patterns: Méthode Aristote code review system

$ARGUMENTS