# Review Pr

> Perform a comprehensive code review of a pull request

- Skill: `florianbruniaux/review-pr` (Agent Skill)
- Install (CLI): `npx skillmds@latest add florianbruniaux/review-pr`
- Raw SKILL.md: https://api.skillmd.com/api/skills/florianbruniaux/review-pr/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Productivity
- Author: FlorianBruniaux (https://skillmd.com/u/florianbruniaux)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/florianbruniaux/review-pr

---


# 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

```markdown
## 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:

```bash
# 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.

```bash
# 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](../../guide/ultimate-guide.md#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):

| If diff contains... | Then check... |
|---------------------|---------------|
| Database queries | Indexes, N+1 patterns, query optimization |
| API endpoints | Auth middleware, input validation, rate limiting |
| Authentication logic | Password hashing, session management, CSRF tokens |
| File uploads | Size limits, MIME validation, storage security |
| Date/time operations | Timezone handling, DST edge cases |
| External API calls | Timeout configs, retry logic, error handling |
| Environment variables | Presence in .env.example, validation at startup |

### 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](https://gist.github.com/patyearone/c9a091b97e756f5ed361f7514d88ef0b) (Jan 2026)
- Anti-hallucination patterns: [Méthode Aristote](https://github.com/FlorianBruniaux) code review system

$ARGUMENTS