PR Review

Review a pull request by diffing the branch against its base, verifying acceptance criteria from the task spec, and producing a structured verdict.

Workflow

START
  │
  ▼
┌──────────────────────────────┐
│ 1. Identify PR & Task        │ ◄── PR number/branch + task spec
└──────────┬───────────────────┘
           │
           ▼
┌──────────────────────────────┐
│ 2. Gather Diff & Context     │ ◄── git diff, file list, commit log
└──────────┬───────────────────┘
           │
           ▼
┌──────────────────────────────┐
│ 3. Acceptance Criteria Audit │ ◄── Check each criterion against diff
└──────────┬───────────────────┘
           │
           ▼
┌──────────────────────────────┐
│ 4. Test Coverage Audit       │ ◄── Identify missing/weak tests
└──────────┬───────────────────┘
           │
           ▼
┌──────────────────────────────┐
│ 5. Code Quality Review       │ ◄── Patterns, bugs, style, security
└──────────┬───────────────────┘
           │
           ▼
┌──────────────────────────────┐
│ 6. Generate Verdict          │ ◄── APPROVE / REQUEST CHANGES
└──────────────────────────────┘

Step 1: Identify PR & Task

Determine what to review:

Find the task spec:

• •Check the PR body for a task reference (e.g., docs/tasks/task-3.1.2.md)
• •Infer from branch name (e.g., feature/task-3.1.2-login-form → docs/tasks/task-3.1.2.md)
• •If no individual task doc, fall back to the matching section in docs/TASKS.md
• •If no task spec found, ask the user — but still proceed with a general review if they don't have one

Read the task spec and extract:

• •Acceptance criteria — the checklist to verify
• •Files expected to change — compare against actual diff
• •Testing instructions — what should be covered

Step 2: Gather Diff & Context

Collect everything needed for review:

# Full diff against base branch
git diff <base>...<branch>

# List of changed files
git diff --name-status <base>...<branch>

# Commit history on the branch
git log --oneline <base>...<branch>

Read each changed file in full to understand context around the diff (not just the diff hunks). Also read:

• •docs/ARCHITECTURE.md — to verify changes follow architectural patterns
• •Existing test files adjacent to changed code — to understand testing conventions

Step 3: Acceptance Criteria Audit

For each acceptance criterion from the task spec, determine:

For each criterion, cite the specific file and code that satisfies it (or explain what's missing).

Step 4: Test Coverage Audit

Review the test files in the diff. Check for:

Coverage gaps:

• •Acceptance criteria with no corresponding test
• •Happy path tested but error/edge cases missing
• •New branches (if/else, switch, try/catch) without test coverage
• •New public functions/methods without tests

Test quality:

• •Tests that only assert toBeDefined() or similar weak assertions
• •Tests that mock so heavily they don't test real behavior
• •Missing setup/teardown that could cause test pollution
• •Tests that would pass even if the implementation were wrong

If coverage reports are available (--coverage output), reference actual coverage numbers. Otherwise assess from reading the test code.

Step 5: Code Quality Review

Scan for issues across these categories:

Correctness:

• •Off-by-one errors, null/undefined handling, race conditions
• •Missing error handling at system boundaries (API calls, file I/O, user input)
• •Logic that doesn't match the task spec's described behavior

Security (OWASP top 10):

• •Unsanitized user input (XSS, injection)
• •Secrets or credentials in code
• •Missing auth/authorization checks

Patterns & conventions:

• •Deviations from docs/ARCHITECTURE.md (wrong directory, wrong abstraction)
• •Inconsistency with existing code conventions in the repo
• •Dead code, unused imports, leftover debug statements

Scope:

• •Changes to files not in the task spec (intentional or accidental?)
• •Unrelated refactors mixed into the PR
• •Over-engineering beyond what the task requires

Keep findings actionable. Don't nitpick style issues that a linter would catch.

Step 6: Generate Verdict

Produce a structured review report:

# PR Review: <PR title>

**Branch:** `<branch>` → `<base>`
**Task:** <task ID and title>
**Verdict:** APPROVE | REQUEST CHANGES

## Acceptance Criteria

| # | Criterion | Status | Evidence |
|---|-----------|--------|----------|
| 1 | <criterion> | MET / PARTIAL / NOT MET / UNCLEAR | <file:line or explanation> |
| 2 | <criterion> | ... | ... |

**Summary:** X/Y criteria met.

## Test Coverage

### Missing Tests
- <what's not tested and should be>

### Weak Tests
- <tests that exist but don't assert meaningful behavior>

### Test Quality Notes
- <any positive observations or general notes>

## Code Review Findings

### Blocking (must fix)
- **[file:line]** <description of issue>

### Non-blocking (suggestions)
- **[file:line]** <description of suggestion>

### Positive Notes
- <things done well worth calling out>

## Scope Check
- **Expected files:** <from task spec>
- **Actual changed files:** <from diff>
- **Unexpected changes:** <any files changed outside task scope, or "None">

## Manual QA Checklist
- [ ] <visual/interactive checks from task spec that can't be verified from code>

Verdict Rules

APPROVE when:

• •All acceptance criteria are MET (UNCLEAR is acceptable if it requires visual QA)
• •No blocking code review findings
• •Tests exist for all criteria with reasonable coverage

REQUEST CHANGES when:

• •Any acceptance criterion is NOT MET or PARTIAL
• •Blocking code quality issues found
• •Critical test coverage gaps (acceptance criteria with zero tests)

Always explain the verdict in one sentence after the status.