| name | pr-review |
| description | Reviews Pull Requests or local diffs with an 8-agent fan-out covering static analysis, dead code, code smells + quality (naming, complexity, single-responsibility, magic numbers), language rules (C++/Python/CMake), architecture, simplification, performance (hot-path classification, allocations, locks, I/O), and undefined behaviour (signed overflow, lifetime, strict aliasing, data races, sanitizer coverage; C/C++/unsafe-Rust only). Use when the user asks to "review this PR", "review the diff", "audit this branch", "/pr-review", or when staging changes before push. |
PR Review Skill
Review Pull Requests or local changes with structured, thorough analysis.
**BE THOROUGH AND PICKY:**
- Review ENTIRE changed files, not just changed lines
- Report ALL issues found - do not skip or filter anything
- Apply programming skill rules strictly
- Check every function, class, and code block in changed files
Determine review target automatically:
- If user provides PR number or URL → Review that GitHub PR
- If no PR specified → Review local changes vs main branch (no questions asked)
Persist the review (opt-in): Do NOT write a markdown file by default. The report goes to chat output. Save the full markdown to .claude/pr-review-summaries/ ONLY when the user explicitly asks ("save the review", "write a summary file", "persist this", or equivalent). See Phase 4 for filename rules when saving.
Invoke relevant programming skills during review:
- C++ code →
programming-cpp, programming-cpp-design-patterns, programming-cpp-stl-algorithms
- Python code →
programming-python
- CMake files →
programming-cmake-best-practices
Workspace-level overrides. Workspaces may define additional
defaults in <workspace>/workflows/pr-review.md; that file
overrides anything in this skill (e.g. clone locations, project
routing, report destination paths).
Universal Hygiene Rules
Apply to every invocation of this skill. Full text in HYGIENE.md. Summary:
- Local-only by default: do NOT post to GitHub unless the user explicitly says "post" / "submit" / "comment on the PR".
- Fresh-eyes rule: when this skill runs inside a sub-agent, the brief is the only context - no project memory, no prior reviews, no conversation history.
- Required report sections: Header, Intent vs Implementation, Per-File Walkthrough, Findings by Severity, Static Analysis, Security Audit, Performance, API/ABI Compatibility, Documentation, Verdict (
APPROVE / REQUEST CHANGES / NEEDS DISCUSSION), Cleanup Confirmation. Full layout in REPORT_TEMPLATE.md.
- Local clone hygiene: record starting branch, stash if dirty, restore on exit via trap/finally - never leave the clone on a detached HEAD or PR branch.
Review Process
| Phase | Purpose |
|---|
| 0. Determine target | PR# / URL -> GitHub PR; nothing -> local diff vs main; "re-review" -> changes since last review |
| 1. Gather + read | Changed files, full diff, commit messages, CI status; read each file ONCE; build Data Package |
| 1.5. Spawn 8 agents in parallel | Static, Dead Code, Code Smells + Quality, Language Rules, Architecture (conditional), Simplify, Performance, UB Detection (C/C++/unsafe-Rust only) |
| 2. Aggregate | Merge findings, map to severity, deduplicate, sort, classify change type |
| 3. Review tests | Coverage, edge cases, naming, independence, assertions; suggest missing tests |
| 4. Final report | Severity-sorted, agent-sourced, with code fixes; chat-only by default, save .md only if asked |
Phase 0: Determine Review Target
Do NOT ask the user what to review. Determine automatically:
| User Input | Action |
|---|
PR number (e.g., 123, #123) | Review GitHub PR #123 |
PR URL (e.g., github.com/.../pull/123) | Review that GitHub PR |
| "re-review" or "review again" | Re-review mode (show only new changes) |
| Nothing / just "review" | Review local changes vs main branch |
GitHub PR
Invoke git-gh-client to verify gh is available, then fetch via gh pr view / gh pr diff / gh pr checks.
Local changes (default)
base=$(git rev-parse --verify main 2>/dev/null && echo main || echo master)
git diff --name-only "$base"...HEAD
git diff "$base"...HEAD
git log "$base"...HEAD --oneline
Re-review mode
Diff from the last reviewed SHA (git diff <last>...HEAD or gh pr view N --json commits,reviews). Focus on NEW changes; note which prior issues were addressed.
Phase 1: Gather Information + Read Files
Collect data ONCE and package for agents - minimize redundant file reads.
GitHub PR
Fetch via git-gh-client commands: gh pr view N --json ... for metadata, gh pr diff N for the diff, gh pr checks N for CI, gh api repos/{o}/{r}/pulls/N/comments + gh pr view N --json reviews for prior comments. Note CI failures in the report; do not block review on them unless the user asks.
Local diff (default)
git diff --name-only <base>...HEAD, git diff <base>...HEAD, git log <base>...HEAD --oneline.
1.1 Read each file ONCE
Read the full body (not just changed lines) with the Read tool. Reading once here vs N times across agents = N-fold token savings.
1.2 Identify languages
| Extension | Language | Used by |
|---|
.cpp / .hpp / .h / .cc | C++ | language-rules-agent, code-smells-agent |
.py | Python | language-rules-agent, code-smells-agent |
CMakeLists.txt / .cmake | CMake | language-rules-agent |
1.3 Build the Data Package
One package, passed to every spawned agent. Required sections: Files Changed (path + language), Full Diff, File Contents (full body, tagged with language), Commit Messages, PR Context (title, description, author, CI status - if GitHub).
Phase 1.5: Spawn Parallel Analysis Agents
Goal: Launch up to 8 specialized agents in parallel to analyze the packaged data from Phase 1.
**Use general-purpose agents** (not Explore agents) since they receive pre-loaded context.
All agents run in parallel - invoke all of them in a single tool call block.
Each agent has a unique identity, loads its skill, and maintains memory.
Lite mode gate
Before spawning all agents, check the diff scope. Agent IDs match the table in "Agent Identity & Memory System" below.
- Diff < 50 lines added/removed AND no logic changes (docs / comments / formatting / imports / type aliases only): spawn ONLY Agents 1 (
static-analysis-agent) and 2 (dead-code-agent). Skip 3, 4, 5, 6, 7, 8.
- Diff < 200 lines AND affects only one file: spawn Agents 1, 2, 3 (
code-smells-agent), 4 (language-rules-agent), 7 (performance-agent), 8 (ub-detection-agent if a C/C++/unsafe-Rust file changed). Skip 5 (architecture) and 6 (simplify).
- Otherwise: full 8-agent fan-out (Agent 5 still gated by the architectural-signal table below; Agent 8 still gated by the UB-trigger rule below).
UB-agent trigger (Agent 8). Spawn Agent 8 ONLY when at least one changed file matches *.c, *.cc, *.cpp, *.cxx, *.h, *.hpp, *.hxx, *.inl, *.ipp, *.tpp, OR contains unsafe { (Rust). Skip otherwise (pure Python / CMake / docs / shell diffs do not exercise UB classes).
Document the chosen mode and the spawned agent IDs in the final report's Header section.
Agent Identity & Memory System
Each agent has:
- Unique ID: Used in Agent tool
description field for identification
- Skill to Load: Agent invokes this skill using the Skill tool before analysis
- Memory File: Project-specific learnings persisted across reviews
| # | Agent ID | Skill to Load | Memory File |
|---|
| 1 | static-analysis-agent | static-analysis | agents/static-analysis.md |
| 2 | dead-code-agent | (none) | agents/dead-code.md |
| 3 | code-smells-agent | code-smells + load QUALITY.md from this skill | agents/code-smells.md |
| 4 | language-rules-agent | programming-cpp or programming-python | agents/language-rules.md |
| 5 | architecture-agent | architecture-analyze | agents/architecture.md |
| 6 | simplify-agent | simplify | agents/simplify.md |
| 7 | performance-agent | (none, loads PERFORMANCE.md from this skill) | agents/performance.md |
| 8 | ub-detection-agent | programming-cpp (for C++ Core Guidelines lifetime / type-safety rules) | agents/ub-detection.md |
Memory location: ~/.claude/projects/<project>/memory/agents/
The 8 Analysis Agents (purpose + memory topic)
| # | Agent ID | Purpose | Memory topic |
|---|
| 1 | static-analysis-agent | Run linters/tools (clang-tidy, ruff, etc.) on full file contents | Tool configs, false positives, suppressions |
| 2 | dead-code-agent | Unused code, comments, unreachable code, comment hygiene | Intentionally-unused code, reserved APIs |
| 3 | code-smells-agent | 22 smells + 4 quality dimensions per QUALITY.md | Project thresholds, quality-dim overrides |
| 4 | language-rules-agent | C++ / Python / CMake best practices per programming-* skills | Project conventions, intentional deviations |
| 5 | architecture-agent | Module boundaries, dependencies, testability (conditional - see signal table) | Module map, interfaces, dependency patterns |
| 6 | simplify-agent | Reuse, unnecessary complexity, verbose patterns per simplify skill | Project utilities, intentional verbosity |
| 7 | performance-agent | Hot-path classification + alloc/complexity/locks/IO per PERFORMANCE.md | Hot files, accepted alloc patterns, benchmarks |
| 8 | ub-detection-agent | Hunts undefined behaviour in C/C++/unsafe-Rust (signed overflow, lifetime, strict aliasing, data races, alignment, null deref, sanitizer-coverage check) | Sanitizer wiring in CI, vetted reinterpret_cast / union / bit_cast sites, confirmed-safe unsafe blocks |
Agent Execution Pattern
Spawn all selected agents in parallel using the Agent tool (one tool block, multiple Agent calls). For each spawn:
description: the agent ID from the table in "Agent Identity & Memory System"subagent_type: general-purpose- prompt body: contents of the matching file under
agents/prompts/ (see "Agent Prompt Templates" below) + the Data Package from Phase 1
Agent 5 only spawns when the architectural-signal table matches. Agent 8 only spawns when the UB-trigger rule above matches. Lite mode (above) further trims the set.
Output discipline (applies to EVERY agent)
Line-number discipline. NEVER cite a line number you have not actually read in the file body. Before emitting path:line, verify:
- The file content at that line was part of the Data Package passed to you in Phase 1.
- The line number is within the file's actual length. A finding citing line 660 in a 222-line file is fabricated; the underlying issue may be real but the citation is unactionable and erodes reviewer trust.
- If you remember the function/class name but not the exact line, cite
path:<function-name> or path:<line-N..line-M> covering the function's known range. Omit the specific line rather than guess.
Fabricated line numbers are WORSE than missing line numbers. The reviewer who follows a citation to line 660 of a 222-line file loses trust in every other finding from the agent.
Class-tag discipline. The Class / Issue Type column must use the agent's own vocabulary (UB:*, Perf:*, Lang:*, Smell:*, Dim N:*, Dead:*, Comment:*, Test:*, CMake:*, Arch:*, Simplify:*, Static:*). Do not tag a missing-virtual-destructor finding as Dead:* or a catch(...) as Comment:*.
Completeness discipline. When a checklist item says MUST flag, the agent emits ONE row per offending site - never collapses multiple violations of the same class into a single representative finding. Five using namespace std; instances in five files = five rows.
Conditional Architecture Analysis
Only run Architecture Agent if ANY of these signals present:
| Signal | Indicates Architecture |
|---|
| New directories created | New module/component |
| New/modified interfaces or abstract classes | API boundaries changing |
| Changes to factories, DI, object creation | Dependency structure changing |
New CMake targets (add_library, add_executable) | New build units |
| Changes across 5+ files in different modules | Cross-cutting change |
| New external dependencies | Integration points |
| Changes to base/core classes | Foundation shifting |
If no architectural signals → Skip Agent 5, run Agents 1-4, 6, 7, and 8 (8 only when its UB-trigger rule matches).
Agent Prompt Templates
Each agent's full prompt lives in its own file under agents/prompts/. The orchestrator passes the file's contents + Data Package to the spawned agent.
