# Skill: PR review

## Description

Review the current pull request or git diff for bugs, style issues,
and missing test coverage. Produce a structured report with severity
levels that the developer can act on immediately.

Invoke this skill with: /review-pr
Or by asking: "Review this PR" or "Review my changes"

### Returns

A markdown report with these sections, in this order:

- A header with the PR identifier (or branch name) and overall severity
- Critical issues: bugs, security problems, or data-loss risks (one per item, with file:line and a one-line fix suggestion)
- Should fix: style, test coverage, or maintainability issues
- Nice to have: opinion-driven suggestions
- Files changed: a per-file summary of what was touched

Plus structured metadata for downstream skills: counts per severity, the list of files reviewed, and a `blocking` boolean derived from "any critical issues present." A composing skill (a PR commenter, a CI gate) can read these fields directly.

## When to use

- The user opens or pushes to a PR and asks for a review
- The user runs `/review-pr` or asks "Review this PR"
- The user wants a pre-merge sanity check before requesting human review

## When not to use

- The PR is purely a vendor update or auto-generated lockfile change. The skill will produce noise on machine-edited code.
- You don't have read access to the diff or the source files. Without the full file context, the review is guessing.
- The diff exceeds tens of thousands of lines (a major refactor or a vendored library import). Sampling instead of reviewing every file is fine, but tell the user explicitly that you sampled.
- The codebase is in a language the agent has weak coverage on. Generating plausible-looking review comments on a language you can't read is worse than declining.

## Steps

### 1. Gather the diff

First, determine what to review:

- If on a branch with an open PR, run `gh pr diff` to get the PR diff.
- If no PR exists, run `git diff main...HEAD` to compare against the
  main branch (adjust the base branch name if the project uses a
  different default).
- If there are only unstaged changes, run `git diff` for those.

Store the full diff output. Also run `git diff --stat main...HEAD`
(or the equivalent) to get a summary of changed files and line counts.

### 2. Identify the changed files

From the diff stat output, build a list of every file that was added
or modified. For each file, note:

- The file path
- The number of lines added and removed
- Whether this is a new file or a modification

Skip files that are purely generated (lock files, build output,
vendored dependencies). Focus review effort on source code, tests,
and configuration.

### 3. Read the full content of changed files

For each changed source file, read the complete file (not just the
diff hunks). You need the full context to evaluate whether new code
fits the existing style and patterns. If a file is very large
(over 1000 lines), focus on the changed sections plus 50 lines of
surrounding context.

### 4. Check for common issues

Review each changed file against this checklist. Only flag issues
that actually appear in the diff. Do not flag pre-existing issues
in unchanged code.

**Critical issues (bugs and security):**

- Unhandled errors: new async calls without try/catch or .catch(),
  new operations that can throw without error handling
- Security problems: hardcoded secrets, SQL injection vectors,
  unsanitized user input, missing authentication checks
- Data loss risks: destructive operations without confirmation,
  missing null checks on database results
- Race conditions: shared mutable state without synchronization,
  async operations that assume ordering

**Warnings (quality and maintainability):**

- Functions longer than 50 lines (suggest extraction)
- Deeply nested logic (more than 3 levels of nesting)
- Console.log, console.debug, or print statements left in
  production code (not in test files)
- Commented-out code blocks (should be removed or explained)
- TODO/FIXME/HACK comments without associated issue numbers
- Magic numbers or strings that should be named constants
- Duplicated logic that appears in more than one place in the diff

**Suggestions (style and best practices):**

- Naming that doesn't match the project's existing conventions
  (check surrounding code for patterns)
- Missing TypeScript types where the project uses strict typing
- Inconsistent formatting compared to surrounding code
- Missing JSDoc or docstrings on public functions (if the project
  uses them elsewhere)
- Opportunities to simplify: ternaries that could be early returns,
  complex conditionals that could be extracted to named functions

### 5. Check test coverage

For every new or modified source file, check whether corresponding
tests exist:

- Look in common test directories: `__tests__/`, `test/`, `tests/`,
  `spec/`, or colocated `.test.*` / `.spec.*` files.
- If the changed file has new exported functions or classes, verify
  that tests exist for them.
- If tests exist but don't cover the new code paths, flag it.
- If no test file exists at all for a new source file, flag that as
  a warning.

Do NOT flag missing tests for:

- Configuration files
- Type definition files
- Migration files
- Files that are purely wiring or glue code with no logic

### 6. Check style consistency

Look at the existing code in the repository (not just the diff) to
identify the project's patterns:

- Naming conventions (camelCase vs snake_case, prefixes, etc.)
- Import ordering and grouping
- Error handling patterns (do they use Result types? Custom error
  classes? Simple try/catch?)
- Comment style and density

Flag any places where the new code deviates from established patterns.

### 7. Produce the review

Output the review in this exact format:

```
## PR Review

**Files reviewed:** [count]
**Issues found:** [critical count] critical, [warning count] warnings, [suggestion count] suggestions

### Critical

- **[file:line]** [description of the issue]
  *Why:* [one sentence explaining the risk]
  *Fix:* [concrete suggestion for how to fix it]

### Warnings

- **[file:line]** [description of the issue]
  *Why:* [one sentence explaining why this matters]
  *Fix:* [concrete suggestion]

### Suggestions

- **[file:line]** [description]
  *Fix:* [concrete suggestion]

### Test coverage

- [file]: [covered / partially covered / no tests]
  [If gaps exist, describe what's missing]

### Summary

[2-3 sentences: overall impression of the changes, the most
important thing to address, and anything that looks particularly
good]
```

If a section has no items, include the heading with "None found."
beneath it.

## Rules

- Only flag issues in changed code. Do not review unchanged lines.
- Be specific. Every issue must reference a file name and line number.
- Be actionable. Every issue must include a concrete fix suggestion.
- Do not rewrite the code for the developer. Describe what to change.
- If you are unsure whether something is a real issue, classify it
  as a suggestion, not a warning or critical.
- Keep the tone professional and constructive. No snark.