pr-review

What this skill does

## Table of Contents

- [Core Principle](#core-principle)
- [When to Use](#when-to-use)
- [Scope Classification Framework](#scope-classification-framework)
- [Classification Examples](#classification-examples)
- [Workflow](#workflow)
- [Phase 1: Establish Scope Baseline](#phase-1-establish-scope-baseline)
- [Phase 2: Gather Changes](#phase-2-gather-changes)
- [Phase 3: Requirements Validation](#phase-3-requirements-validation)
- [Phase 1.5: Version Validation (MANDATORY)](#phase-15-version-validation-mandatory)
- [Phase 4: Code Review with Scope Context](#phase-4-code-review-with-scope-context)
- [Phase 4.5: Additive Bias Audit](#phase-45-additive-bias-audit)
- [Phase 5: Backlog Triage](#phase-5-backlog-triage)
- [Phase 6: Generate Report](#phase-6-generate-report)
- [Phase 7: Knowledge Capture](#phase-7-knowledge-capture)
- [Quality Gates](#quality-gates)
- [Anti-Patterns to Avoid](#anti-patterns-to-avoid)
- [Don't: Scope Creep Review](#dont-scope-creep-review)
- [Don't: Perfect is Enemy of Good](#dont-perfect-is-enemy-of-good)
- [Don't: Blocking on Style](#dont-blocking-on-style)
- [Don't: Reviewing Unchanged Code](#dont-reviewing-unchanged-code)
- [Integration with Other Tools](#integration-with-other-tools)
- [Exit Criteria](#exit-criteria)


# Scope-Focused PR Review

Review pull/merge requests with discipline: validate against original requirements, prevent scope creep, and route out-of-scope findings to issues on the detected platform.

**Platform detection is automatic** via `leyline:git-platform`. Use `gh` for GitHub, `glab` for GitLab. Check session context for `git_platform:`.

## Core Principle

**A PR review validates scope compliance, not code perfection.**

The goal is to validate the implementation meets its stated requirements without introducing regressions. Improvements beyond the scope belong in future PRs.

## When To Use

- Before merging any feature branch
- When reviewing PRs from teammates
- To validate your own work before requesting review
- To generate a backlog of improvements discovered during review

## When NOT To Use

- Preparing PRs - use pr-prep instead
- Deep code
  review - use pensive:unified-review
- Preparing PRs - use pr-prep instead
- Deep code
  review - use pensive:unified-review

## Scope Classification Framework

Every finding must be classified:

| Category | Definition | Action |
|----------|------------|--------|
| **BLOCKING** | Bug, security issue, or regression introduced by this change | Must fix before merge |
| **IN-SCOPE** | Issue directly related to stated requirements | Should address in this PR |
| **SUGGESTION** | Improvement within changed code, not required | Author decides |
| **BACKLOG** | Good idea but outside PR scope | Create GitHub issue |
| **IGNORE** | Nitpick, style preference, or not worth tracking | Skip entirely |

### Classification Examples

**BLOCKING:**
- Null pointer exception in new code path
- SQL injection in new endpoint
- Breaking change to public API without migration
- Test that was passing now fails

**IN-SCOPE:**
- Missing error handling specified in requirements
- Feature doesn't match spec behavior
- Incomplete implementation of planned functionality

**SUGGESTION:**
- Better variable name in changed function
- Slightly more efficient algorithm
- Additional edge case test

**BACKLOG:**
- Refactoring opportunity in adjacent code
- "While we're here" improvements
- Technical debt in files touched but not changed
- Features sparked by seeing the code

**IGNORE:**
- Personal style preferences
- Theoretical improvements with no practical impact
- Premature optimization suggestions

## Workflow

### Phase 1: Establish Scope Baseline

Before looking at ANY code, understand what this PR is supposed to accomplish.

**Note:** Version validation (Phase 1.5) runs AFTER scope establishment but BEFORE code review. See `modules/version-validation.md` for details.

**Search for scope artifacts in order:**

1. **Plan file**: Most authoritative (check spec-kit locations first, then root)
   ```bash
   # Spec-kit feature plans (preferred - structured implementation blueprints)
   find specs -name "plan.md" -type f 2>/dev/null | head -1 | xargs cat 2>/dev/null | head -100
   # Legacy/alternative locations
   ls docs/plans/ 2>/dev/null
   # Root plan.md (may be Claude Plan Mode artifact from v2.0.51+)
   cat plan.md 2>/dev/null | head -100
   ```
   **Verification:** Run the command with `--help` flag to verify availability.

2. **Spec file**: Requirements definition (check spec-kit locations first)
   ```bash
   find specs -name "spec.md" -type f 2>/dev/null | head -1 | xargs cat 2>/dev/null | head -100
   cat spec.md 2>/dev/null | head -100
   ```
   **Verification:** Run the command with `--help` flag to verify availability.

3. **Tasks file**: Implementation checklist (check spec-kit locations first)
   ```bash
   find specs -name "tasks.md" -type f 2>/dev/null | head -1 | xargs cat 2>/dev/null
   cat tasks.md 2>/dev/null
   ```
   **Verification:** Run the command with `--help` flag to verify availability.

4. **PR/MR description**: Author's intent
   ```bash
   # GitHub
   gh pr view <number> --json body --jq '.body'
   # GitLab
   glab mr view <number> --json description --jq '.description'
   ```
   **Verification:** Run the command with `--help` flag to verify availability.

5. **Commit messages**: Incremental decisions
   ```bash
   # GitHub
   gh pr view <number> --json commits --jq '.commits[].messageHeadline'
   # GitLab
   glab mr view <number> --json commits
   ```
   **Verification:** Run the command with `--help` flag to verify availability.

**Output:** A clear statement of scope:
> "This PR implements [feature X] as specified in plan.md. The requirements are:
> 1. [requirement]
> 2. [requirement]
> 3. [requirement]"

If no scope artifacts exist, flag this as a process issue but continue with PR description as the baseline.

### Phase 2: Gather Changes

```bash
# GitHub
gh pr diff <number> --name-only
gh pr diff <number>
gh pr view <number> --json additions,deletions,changedFiles,commits

# GitLab
glab mr diff <number>
glab mr view <number>
```
**Verification:** Run the command with `--help` flag to verify availability.

### Phase 3: Requirements Validation

Before detailed code review, check scope coverage:

- [ ] Each requirement has corresponding implementation
- [ ] No requirements are missing
- [ ] Implementation doesn't exceed requirements (overengineering signal)

### Phase 1.5: Version Validation (MANDATORY)

**Run version validation checks BEFORE code review.**

See `modules/version-validation.md` for detailed validation procedures.

**Quick reference:**
1. Check if bypass requested (`--skip-version-check`, label, or PR marker)
2. Detect if version files changed in PR diff
3. If changed, run project-specific validations:
   - Claude marketplace: Check marketplace.json vs plugin.json versions
   - Python: Check pyproject.toml vs __version__
   - Node: Check package.json vs package-lock.json
   - Rust: Check Cargo.toml vs Cargo.lock
4. Validate CHANGELOG has entry for new version
5. Check README/docs for version references
6. Classify findings as BLOCKING (or WAIVED if bypassed)

**All version mismatches are BLOCKING unless explicitly waived by maintainer.**

### Phase 3.5: PR Hygiene Checks

Before diving into code, run the PR hygiene checks from
`modules/pr-hygiene.md`:

1. **Atomicity check**: Does this PR contain one logical
   change? Flag mixed commit types (feat, refactor, and fix),
   formatting commits bundled with logic, or changes spanning
   unrelated subsystems. Large PRs get 30% defect detection
   vs 75% for focused ones.

2. **Agent curation check**: Does the code show signs of
   iterative AI generation without a cleanup pass? Look for
   redundant implementations, premature abstractions, incomplete
   refactors, and scope drift.

3. **Self-review signals**: Are there unsquashed fixup commits,
   debug statements, or commented-out code tha