pr-review

name	pr-review
description	Review PR changes against a base branch, focusing on regressions, missing edge cases, serious code quality problems, and other fundamental issues. Use when you want a thorough code review of your changes before merging.

PR Review Skill

Overview

Review all changes (commits + staged changes) that differ from a base branch. Focus on catching serious issues that could cause problems in production rather than stylistic nitpicks.

Workflow

Step 1: Determine Base Branch

Ask the user which branch to compare against:

Which branch should I compare against?
1. main
2. master
3. develop
4. Other (specify)

If the user doesn't specify, default to checking for main first, then master.

Step 2: Gather Changes

Run these commands to understand the full scope of changes:

# Get the merge base (common ancestor)
git merge-base <base-branch> HEAD

# Show all commits since diverging from base
git log --oneline <base-branch>..HEAD

# Get full diff of all changes
git diff <base-branch>...HEAD

# Check for any staged changes not yet committed
git diff --cached

# List all changed files
git diff --name-only <base-branch>...HEAD

Step 3: Analyze Changes

For each changed file, read the full file context (not just the diff) to understand:

• What the code is supposed to do
• How the changes fit into the broader system
• What edge cases might exist

Step 4: Review Focus Areas

Focus your review on these critical areas, in order of importance:

1. Regressions

• Does this change break existing functionality?
• Are there callers of modified functions that might break?
• Do modified APIs maintain backward compatibility?
• Are there tests that should be updated but weren't?

2. Missing Edge Cases

• What happens with null/undefined/empty inputs?
• What happens at boundary conditions (0, negative, max values)?
• What happens with concurrent access?
• What happens if external services fail?
• What happens with malformed data?

3. Security Issues

• SQL injection, XSS, command injection vulnerabilities
• Authentication/authorization bypasses
• Sensitive data exposure (logs, errors, responses)
• Insecure dependencies or configurations

4. Data Integrity

• Race conditions that could corrupt data
• Missing transactions where atomicity is needed
• Inconsistent state if operations partially fail
• Missing validation before database writes

5. Error Handling

• Are errors caught and handled appropriately?
• Do error messages leak sensitive information?
• Is there proper cleanup on failure (connections, files, locks)?
• Are errors logged with sufficient context for debugging?

6. Performance Concerns

• N+1 queries or unnecessary database calls
• Missing indexes for new query patterns
• Memory leaks or unbounded growth
• Blocking operations in async contexts

7. Logic Errors

• Off-by-one errors
• Incorrect boolean logic
• Wrong comparison operators
• Incorrect null/undefined handling

Step 5: Output Format

Present findings in this structure:

## PR Review: [brief description of changes]

**Base Branch:** <branch>
**Commits Reviewed:** <count>
**Files Changed:** <count>

### Critical Issues
*Must be fixed before merging*

1. **[File:Line] Issue Title**
   - **Problem:** Clear description of the issue
   - **Impact:** What could go wrong
   - **Suggestion:** How to fix it

### Warnings
*Should be addressed, but may be acceptable with justification*

1. **[File:Line] Issue Title**
   - **Problem:** Description
   - **Risk:** Potential consequences
   - **Suggestion:** Recommended fix

### Suggestions
*Nice to have improvements*

1. **[File:Line] Issue Title**
   - **Observation:** What could be better
   - **Suggestion:** Improvement idea

### Summary
- X critical issues found
- Y warnings found
- Z suggestions

**Recommendation:** [APPROVE / REQUEST CHANGES / NEEDS DISCUSSION]

Guidelines

DO Focus On

• Logic errors that cause incorrect behavior
• Security vulnerabilities
• Data corruption risks
• Breaking changes to APIs
• Missing error handling that could crash the app
• Race conditions and concurrency bugs

DO NOT Focus On

• Style preferences (naming, formatting)
• Minor refactoring opportunities
• Adding comments or documentation
• Test coverage (unless critical paths are untested)
• Performance micro-optimizations

Be Specific

• Always reference exact file paths and line numbers
• Show the problematic code snippet
• Explain WHY something is a problem, not just WHAT
• Provide concrete fix suggestions, not vague advice

Consider Context

• Understand the purpose of the change before critiquing
• Consider whether edge cases are actually possible in this context
• Weigh the severity against the likelihood of occurrence
• Acknowledge when trade-offs are reasonable

False Positives

If you're uncertain whether something is actually an issue:

• Mark it as a question rather than a definite problem
• Explain your concern and ask for clarification
• Don't flag things just to have something to say