---

name: code-review description: |- Use when reviewing code changes before committing or merging — staged diffs, branch diffs, or PR readiness checks. Triggers: 'review my code', 'check these changes', 'review this branch', 'give feedback on my diff', 'is this PR ready?'. Not for implementing features, running test suites, formatting-only passes, or building understanding of a change (use teach-me).

Code Review

Overview

Structured, actionable code review focused on correctness, bugs, style, and maintainability. Act as a senior engineer: thorough, pragmatic, impact-first.

When to Use

• Reviewing staged changes before committing.
• Reviewing branch changes before merging or opening a PR.
• Checking a diff for correctness, bugs, style, or structure.
• Final review of all changes on a feature branch.
• Quick sanity-check of a small changeset before pushing.

When NOT to use:

• Implementing new features from scratch.
• Running test suites (see writing-tests or equivalent).
• Refactoring without a prior review (review first, then refactor on request).
• Linting or formatting only — use a linter or formatter directly.

Invocation Notice

• Tell the user when this skill is running: code-review. Skip the notice when the user asked for the skill by name or slash command; spelling and spacing need not match. A phrase from this skill's own trigger list is not a name — naming the work is not naming the skill.

Constraints

• Do NOT modify code unless explicitly asked.
• Do NOT guess missing code or behavior; ask for missing context.
• Do NOT suggest adding suppressions (e.g., #pragma warning disable).
• Stay focused on review quality; avoid unrelated commentary.
• Work strictly from the provided diff or code context.

Quick Reference

| Area | Look for | | --------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | | Correctness & Logic | Wrong behavior, broken contracts, off-by-one | | Bugs & Edge Cases | Nil/null paths, boundary values, error propagation | | Code Quality & Style | Naming, readability, idiomatic usage | | Structure & Maintainability | Coupling, duplication, separation of concerns | | Data Representation | Structures that can hold invalid states, replicate information, or make every consumer enforce their rules | | Best Practices | Language/framework conventions, SOLID, DRY | | Test Adequacy | Wrong implementations the tests would still accept, missing tests for behavior changes, regression gaps | | Performance | N+1 queries, unbounded loops/fan-out, missing pagination, sync calls in hot paths, large allocations, missed async | | Security & Risk | OWASP Top 10: access control, injection, crypto, auth, supply chain, logging, error handling — use securing-code checklist | | Documentation | Misleading comments, missing doc for public API |

Common Mistakes

• Reviewing without full context — jumping to conclusions without reading the complete diff or understanding intent.
• Mixing review and edits — modifying code during the review phase instead of separating feedback from implementation.
• Nitpick overload — burying critical issues under dozens of style nitpicks.
• Ignoring intent — critiquing design choices without understanding the constraints or goals behind them.
• Incomplete scope — reviewing only one file when the change spans multiple files or modules.
• Searching near the last defect — a re-review that looks only at last review's findings and their neighbors finds one more class of defect per run; start from the partition list instead.

Workflow

Step 0 — Gate: Scope & Tools

Determine review scope and available tooling before doing any analysis.

Scope detection:

Identify whether this is a pre-commit or pre-merge review, then set the base ref accordingly.

Build the review packet first. Run scripts/build_review_packet.py (shipped in this skill's scripts/) to resolve scope deterministically and write one packet — diff, changed-file list, and source-of-truth — that every later step and every subagent reads by path instead of re-deriving the base ref and re-running git:

# Auto-detects staged vs. branch vs. worktree. Pass the spec so the packet
# carries what the change is judged against (see source-of-truth note below).
scripts/build_review_packet.py --intent "<what this change is for>" --include <spec-or-plan-if-any>

It prints JSON with packet_path, has_source_of_truth, branch, state_path, state_exists, stale_closed_partitions, and output_dir_is_tmp. Without --out, the packet goes to the first of .review/, ._scratch/, or .scratch/ that exists at the repo root, else $TMPDIR; when this run or the project instructions set a report destination (see Step 3), pass --out to put the packet beside it. Use packet_path as the deterministic scope baseline for the rest of the review and for every subagent dispatch — it fixes what changed so no one re-derives it, but it is a floor, not a ceiling: keep reading surrounding code, callers, history, and tests beyond the diff as the review demands. The raw git commands below are what the script automates; reach for them directly only when scoping a review the script's scope detection does not cover.

Review state. state_path names the file that keeps this branch's partition list across reviews (references/partition-discovery.md § 6). When output_dir_is_tmp is true, the state lands in $TMPDIR, which a later session or another sandbox may not see, so later reviews would start over. Ask the user once per repo: "May I create .review/ at the repo root for review files and state, with a .gitignore so git ignores it?" If the user agrees, create .review/ with a .gitignore that contains *, and rebuild the packet. If the user declines, or the run cannot ask, continue in $TMPDIR and say in the report that later reviews will not see this review's state.

Source of truth. If has_source_of_truth is false, the packet is a bare diff. A reviewer grading spec compliance from a bare diff will silently invent a spec and grade against it. When a spec exists, supply --intent/--include so the packet carries it; when it does not, judge only stated intent and say so — never infer a spec from the diff and grade against it.

Pre-commit (staged changes):

# List staged files
git diff --cached --name-status

# View staged diff
git --no-pager diff --cached

Pre-merge (branch vs. target):

# Resolve target base branch from origin/HEAD when available
BASE_BRANCH=$(git rev-parse --abbrev-ref origin/HEAD 2>/dev/null)
BASE_BRANCH=${BASE_BRANCH#origin/}
BASE_BRANCH=${BASE_BRANCH:-main}
MERGE_BASE=$(git merge-base "$BASE_BRANCH" HEAD)

# List changed files on this branch relative to the target base
git diff --name-status "$MERGE_BASE"...HEAD

# View full branch diff
git --no-pager diff "$MERGE_BASE"...HEAD

If the user does not specify scope, infer from context:

• On a feature branch with commits ahead of the target base branch → pre-merge review.
• Staged changes present → pre-commit review.
• Ask if ambiguous, especially when default branch detection fails.

Graph-enhanced tooling gate:

If the code-review-graph MCP plugin is available, probe and ensure freshness:

1. Call build_or_update_graph_tool() to run an incremental update.
2. Call list_graph_stats_tool() to verify the graph has nodes and check last_updated.
3. If either call fails or the graph is empty → proceed with the git-only path for the remainder of the review. Do not retry.

When the graph is available, follow references/code-review-graph-integration.md for enhanced analysis at each subsequent step. The reference doc specifies required and optional tool calls per phase, with decision criteria for when to use each.

Parallel subagent gate:

If the Agent tool is available and the change is non-trivial, plan to use the parallel-subagent path in Step 2. A change that touches a security boundary or that a spec drives takes the parallel path at any size. See references/parallel-subagent-review.md for the thresholds, dispatch pattern, and prompt templates. Otherwise continue with the single-agent path. These dispatch rules are purpose-built for code review and take precedence over any general subagent skill (e.g. subagent-patterns) when the work is a code review.

Step 1 — Triage & Context

Build a prioritized review plan before reading any code in detail.

Always (git-based):

• Review the diff to understand what changed and why.
• For medium/large or cross-module changes, write a short review plan focused on risk hotspots.

Git diagnostics (useful for unfamiliar codebases or large diffs — see references/git-diagnostics-before-review.md):

1. Check file churn and bug clusters to identify which changed files historically attract the most defects.
2. Review team composition and recent contributor activity for bus-factor context.
3. Look for firefighting patterns (reverts, hotfixes) that signal stability risks in the areas under review.

When graph is available (see references/code-review-graph-integration.md Phase 1):

• Call detect_changes_tool(base=<base>) for risk-scored, priority-ordered triage. Use its risk scores to order files highest-risk first.
• Call get_review_context_tool(base=<base>) for token-efficient structural context instead of reading entire changed files.
• Optionally call get_affected_flows_tool (change touches 2+ files), get_architecture_overview_tool (change spans 3+ directories), or get_impact_radius_tool (need raw blast-radius data) as the scope warrants.

Partition list: Build the list of domain features the changed code handles, before looking for defects (references/partition-discovery.md § 1). When state_exists is true, load the saved list first, reopen stale_closed_partitions, and add partitions for code the saved list does not reach. Do not take partitions from the last review's findings.

Triage output: Produce a prioritized file list, a ranked partition list, and a brief review plan identifying the highest-risk areas to focus on.

Step 2 — Structured Review (No Edits)

Pick the path set at the Step 0 parallel subagent gate.

Parallel-subagent path (when gate is active):

Follow references/parallel-subagent-review.md — dispatch one partition explorer per open partition alongside the spec-compliance, quality and correctness, test-adversary, and security lenses (plus architecture when warranted), run the explorers' distinguishing inputs, hunt variants of each confirmed defect, dispatch a devil's-advocate subagent for a challenge pass on what could not be run, then do final synthesis and tiebreak yourself. The security subagent is always dispatched; it uses the OWASP Top 10:2025 checklist from securing-code skill references. Security partitions come from the securing-code threat-modeling, OWASP Top 10:2025, and tier3 checklists whenever the change touches a security boundary. Include the architecture subagent when: pre-merge / pre-PR review, the user