pr-reviewer

What this skill does

# Local Review

Perform systematic review with actionable, validated feedback only.
Use this skill as an explicit local self-review step before handoff, not as a generic replacement for native PR review tools.
Run it before `/done` when a coding session produced changes worth checking.

## Reference Files

| File | Read When |
|------|-----------|
| `references/severity-rubric.md` | Default: choosing severity labels and filtering weak findings |
| `references/comment-examples.md` | Before producing a local review report |
| `references/review-surfaces.md` | When deciding whether the work stays in local self-review or should hand off to PR-specific workflows |
| `references/security-checklist.md` | When the diff touches auth, input handling, external APIs, file uploads, or environment configuration — and always in Security audit mode (whole-codebase) |
| `references/performance-checklist.md` | When the diff touches data fetching, rendering, images, dependencies, or bundle-affecting imports |
| `references/structural-quality-rubric.md` | When the user asks for a structural quality review, thermo-nuclear review, deep code quality audit, or when reviewing large diffs that touch module boundaries |
| `references/ai-slop-patterns.md` | When the user asks to deslop, clean up AI code, remove slop, or when reviewing AI-assisted code changes |

## Scope
- Default target: staged or uncommitted local changes
- Secondary target: current branch diff against base when the working tree is clean or the user asks for branch review
- Explicit PR requests are secondary: keep the same review criteria, but treat them as a handoff path rather than the main workflow
- Keep the skill focused on concrete bugs, missing validation/tests that clearly matter, and repository instruction-file compliance
- Do not use this skill for inbound PR comments or thread resolution; use `pr-comments` for that

## Security audit mode (whole-codebase)

Triggered by an explicit security ask ("security audit", "find vulnerabilities", "deepsec", "security review", "threat model", "audit for security"). This mode **overrides the default diff scope** — it proactively sweeps the relevant subsystem or the whole repo by vulnerability class, not just changed lines. Everything else about the skill is unchanged: it stays report-only and uses the same three-tier output.

- **Scope:** the subsystem the user names, or the whole codebase if unscoped. Diff status is irrelevant here — review code as it stands, not just what changed.
- **Always load `references/security-checklist.md`** and walk the threat-model lens and vulnerability-class sweep it defines, rather than waiting for a diff to touch a sensitive area.
- **Walk by vulnerability class, not by file** — for each OWASP-aligned class, search the codebase for the pattern (e.g. query construction, auth checks, deserialization, secret handling) and confirm each hit against the actual code before reporting.
- **High-signal bar still applies** — report only concrete, exploitable findings with a path/line and a plausible attack; never speculative "could be risky" items. Drop anything you can't tie to a real exploit path.
- **Output:** the same `Must fix before push` / `Should fix soon` / `Ready for handoff` tiers, with each finding naming the vulnerability class, the location, and the exploit path.

## Workflow

Copy this checklist to track progress:

```text
Review progress:
- [ ] Discover local review target
- [ ] Gather context and scoped instruction files
- [ ] Choose the local review path
- [ ] Validate findings
- [ ] Produce the review report
```

1. **Discover the review target**:
   - If staged or unstaged changes exist, review those first
   - Otherwise review the current branch diff against its base
   - Only switch to a PR handoff summary when the user explicitly points at an existing PR
   - Record the current branch and changed files so the report is grounded in the local session
2. **Gather context**:
   - Capture the change intent from the session, recent commits, or the user's request
   - Load relevant repository instruction files (`AGENTS.md` / `CLAUDE.md` as applicable, including any in nested package/MFE directories whose code is in the diff)
   - Apply only in-scope instruction-file rules for the changed paths
   - Run the project's lint, type check, and test commands (from `package.json` scripts) to capture current status — note pre-existing failures so they are distinguishable from regressions caused by the change. Include the lint/type-check/test status in the report so the reader knows the baseline at review time
3. **Choose the local review path**:
   - Local self-review is the default: current diff/branch with a local report in chat
   - Existing PR requests are secondary: apply the same validation bar, then produce a concise handoff summary instead of changing the main workflow
   - For large changes, shard by subsystem and keep the final report consolidated
   - Optional, non-trivial diffs only: if an independent review CLI is already installed (e.g. `codex exec`, `droid exec`), you may run it read-only as a *different-model* second opinion and fold any validated findings into the same report. This hedges against Claude reviewing Claude-authored code. Skip silently if none is installed — never add it as a dependency
   - Treat external-engine output as **advisory**: validate every finding against the actual diff before including it, and drop anything this skill would not have flagged on its own
4. **Validate issues**:
   - Re-check exact lines before reporting
   - Keep only high-confidence issues; drop speculative or duplicate items
   - Confirm each issue still applies to the latest diff and maps to a changed line
   - Collapse multiple comments that share the same root cause into one finding
5. **Produce the report**:
   - Default output: a local review report in chat
   - Organize findings into `Must fix before push`, `Should fix soon`, and `Ready for handoff`
   - Do not post inline comments, resolve threads, or handle inbound review feedback from this skill
   - Hand off inbound PR feedback to `pr-comments`

## High signal only

Flag only when certain:
- Code will fail to compile (syntax, types, imports)
- Code will produce incorrect behavior (clear logic or state errors)
- Code introduces a concrete security risk with direct exploit path — load `references/security-checklist.md` for the three-tier classification when the diff touches auth, input handling, external APIs, or environment configuration
- Code introduces a measurable performance regression — load `references/performance-checklist.md` for common bottleneck patterns when the diff touches data fetching, rendering, images, or dependencies
- Changed behavior is clearly missing a necessary regression or validation test, including: a new component or hook shipped with no co-located test file, or an existing test where every assertion is a render-only presence check (`expect(getByText(...)).toBeInTheDocument()`) with no user interaction or branch coverage
- Bug fix without a failing test that reproduces it first (Prove-It Pattern: if the fix is correct, a test for the bug should fail before and pass after)
- Test code over-abstracts shared setup to the point where individual tests are unreadable without tracing helpers (prefer DAMP — Descriptive And Meaningful Phrases — over DRY in test code)
- Lint, type check, or tests fail as a result of the change (distinguish from pre-existing failures captured in step 2)
- Unambiguous instruction-file violation (quote rule, verify scope)
- YAGNI violation: code adds abstractions, config systems, or extension points not justified by a current requirement (three similar lines is better than a premature abstraction)
- KISS violation: implementation is more complex than the problem demands — a simpler approach exists that achieves the same result
- Code exhibits AI-generated patterns (over-commenting, unnecessary wrapping, type bypas