| name | pr-review |
| description | Review pull requests. Read-only analysis — does NOT modify code, build, deploy, or run tests.
Use when: review PR, code review, check branch changes.
|
PR Review — Strictly Read-Only Review Methodology
Constraints
- Do NOT edit, create, or delete any files in the repository
- Do NOT make commits or push changes
- Do NOT run builds or tests
- Do NOT deploy anything
- Do NOT fix issues — only describe them and suggest fixes in comments
- ONLY read, analyze, and post review comments
Trust but Verify — BLOCKING Rule 🔴
Do NOT trust the developer's completion summary in PR comments. You MUST:
- Read the full code diff (every changed file, every changed line)
- Check each claim in the developer's comment — is it actually implemented in the code?
- If a claim is not verifiable from the diff, flag it as Critical severity
- Do NOT approve if the diff does not match the developer's stated work
This rule is BLOCKING — violating it means approving changes you haven't verified, which defeats the purpose of review.
Review Checklist
Evaluate every change against the following criteria:
- Correctness: Does the code do what the spec/PR description says? Does it actually fix the issue?
- Design: Is the approach reasonable? Are there simpler, more maintainable alternatives?
- Code quality: Readability, naming, error handling, consistent style with the surrounding codebase
- Tests: Are there tests for the changes? Do they actually test the behavior? Are edge cases covered?
- Edge cases: Missing error handling, boundary conditions, null/empty inputs, concurrent access
- Project conventions: Does the code follow the project's own rules from AGENTS.md / CLAUDE.md / README.md?
- Coding principles (check against
coding-principles skill):
- Simplicity First (P2): Flag overcomplication — code beyond what was asked, unnecessary abstractions
- Surgical Changes (P3): Flag unnecessary changes — "improvements" to unrelated code, style changes
- Goal-Driven Execution (P4): Check goal traceability — every changed line should trace to user's request
- Documentation: CHANGELOG.md updated for user-facing changes; DESIGN.md updated for architecture changes (if applicable)
- Commit structure: Does each commit map to one step from the Implementation Plan? Are commit messages clear?
Severity Classification
Categorize each finding by severity:
- Critical 🔴: security issues, data loss, crashes, trust-but-verify violations
- High 🟠: design principle violations, missing error handling, broken functionality
- Medium 🟡: missing tests, inconsistent naming, dead code
- Low 🟢: documentation gaps, style suggestions, minor improvements
For each finding, use this format:
**[SEVERITY]** `file:line` — description
Suggestion: how to fix
Overall Verdict
- Approve: no Critical or High issues
- Request Changes: Critical or High issues found
- Comment (approve with suggestions): only Medium/Low issues
