# Review Code

> Run a multi-reviewer audit of your own branch, diff, or explicit paths and return a prioritized, deduplicated report on correctness, security, performance, design, and tests. Use for "review my changes", "review this branch/diff", "find bugs", or "audit these files". Use review-pr for others' PRs, auto-review-code to apply fixes, and review-plan for plans.

- Skill: `sontek/review-code` (Agent Skill, multi-file: 3 files)
- Install (CLI): `npx skillmds@latest add sontek/review-code`
- Raw SKILL.md: https://api.skillmd.com/api/skills/sontek/review-code/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: sontek (https://skillmd.com/u/sontek)
- Updated: 2026-09-10
- Page: https://skillmd.com/skills/sontek/review-code

---


# Review Code

Fan out to the specialized review agents in isolated context, coalesce their findings, and return one prioritized report. The skill resolves scope and selects reviewers; the agents read the code and produce the findings. Use this on code you're working on, to read or hand to `auto-review-code` for fixes. For comments on someone else's PR, use `review-pr` (human-toned comments + approval-to-post).

## When to invoke

- "Review my changes", "review this branch", "review this diff", "find bugs in this code"
- Pre-merge review of your own feature branch
- Audit a specific set of files or directories for quality / correctness issues

Don't use for:

- Leaving review comments on someone else's PR — use `review-pr`
- Auto-applying fixes in a loop — use `auto-review-code`
- Reviewing a planning document — use `review-plan`
- Reviewing skills under `plugins/sontek-skills/skills/` — use `review-skill`

## Modes

Pick one before invoking. If the caller didn't specify, default to `branch`.

- **`branch` (default)** — Review the current branch's changes vs. the main branch. Agents flag only issues introduced by the diff.
- **`paths`** — Review the current state of an explicit list of files or directories, regardless of git history. Requires a path list from the caller — do not default to whole-repo; ask if it's missing.

## Process

### 1. Resolve scope

- **`branch` mode:** determine the base branch (default `main`; check `git symbolic-ref refs/remotes/origin/HEAD` or an explicit override in conversation). Diff range `<base>...HEAD`. Collect changed files: `git diff --name-only <base>...HEAD`.
- **`paths` mode:** take the explicit file/directory list from the caller.

### 2. Pick reviewers from the changed files

Select reviewers per the shared rules in [references/reviewer-selection.md](references/reviewer-selection.md) — the single source of truth that `review-pr` reads from too, so the two skills' rosters can't drift. Always dispatch **`code-reviewer`** and **`security-auditor`**; add a specialist (`perf-reviewer`, `iac-reviewer`, `sql-reviewer`, the Django reviewers, `gha-security-reviewer`) only when the changed files touch its domain. The full domain table and the co-firing/overlap rules (e.g. `perf-reviewer` co-firing with `django-perf-reviewer` / `sql-reviewer`, where the overlap is the backstop) live in that reference.

### 3. Ground the review

Light pass only — do NOT review the code yourself.

- Read the branch summary / PR description if available (`gh pr view --json title,body` when a PR exists).
- **Claims audit.** When the PR description states concrete numbers, defaults, or behavioral claims ("defaults to 8 GB", "lowers the timeout to 30s", "now retries 3 times"), spot-check them against the diff and forward any mismatch to the agents as a candidate finding — the description may describe an earlier revision. Don't audit prose intent, only checkable claims.
- **Author-declared risk areas.** Scan the description for the author's own pointers at risk — "worth a careful look", "I'm not sure about", "the tricky/risky part is", "needs a close look", "double-check the X logic". Forward each, verbatim, to the agents as a **mandatory deep-dive target** — this is where the author already suspects a defect, and it's the highest-yield place to look (a senior reviewer reads these first, and it's a large part of why a whole-diff bot lands findings here). Distinct from the claims audit: claims are checkable facts; risk areas are the author's unease about correctness.
- Capture any caller "don't flag this, it's intentional" notes verbatim.
- Note whether `REVIEW_GUIDELINES.md` exists (the agents load it; just confirm it's there).

### 4. Dispatch in parallel

Invoke every selected agent in a **single message with multiple Task calls** so they run concurrently. Use `subagent_type: sontek-skills:<agent-name>`. Each prompt is self-contained (isolated context): include the mode, the base branch + diff range (or path list), the changed-file list, the grounding from step 3, the caller's "don't flag X" notes verbatim, and a pointer to `REVIEW_GUIDELINES.md` if present. Tell each agent to follow its own rubric and output format.

### 5. Coalesce, normalize, and report

Merge the raw findings into one deduplicated report:

- **Dedup / corroborate.** Same file within ±3 lines on the same root issue → one entry; note when multiple reviewers raised it (corroboration raises confidence and ordering).
- **Drop noise.** In `branch` mode, drop findings outside the diff.
- **Normalize severity to one ruler.** Each agent uses its own scale; map them onto a shared `P0`–`P3` band so the report sorts cleanly and downstream tooling (e.g. `auto-review-code`) can triage uniformly. This is scale-mapping, not second-guessing — keep each agent's native label in the line.
- **Sweep for gaps before you finalize.** The finders run in isolated context and each sees only its own angle, so a defect that falls *between* them — or one the diff re-exposes in an untouched line of a touched function — is the dominant miss (this skill's documented weak spot is recall, not false positives). After dedup, dispatch **one** fresh `code-reviewer` agent (`subagent_type: sontek-skills:code-reviewer`) as a gap sweep: give it the diff scope **and the consolidated findings list so far**, and tell it to re-read the diff and enclosing functions looking ONLY for defects not already on the list — not to re-derive or re-confirm what's there. Fold any genuinely new candidates into the list; they go through the verify step below like any other finding. A clean sweep that finds nothing new is the expected outcome on a tight diff — it must not pad to look thorough. This is the recall complement to the verifier's precision; together they mirror find → sweep → verify.
- **Verify before reporting — don't just consolidate.** A sub-agent finding is a hypothesis, not a fact, and finders self-validate in the *same* context that produced the finding, so they confirm their own catch. After dedup, dispatch the **`finding-verifier`** agent (`subagent_type: sontek-skills:finding-verifier`) once on the consolidated list — pass each finding's fingerprint, claimed mechanism, claimed consequence, and the diff scope, but **not** the finders' reasoning (the asymmetry is the point: it's told to *refute*, fresh). It returns CONFIRMED / PLAUSIBLE / REFUTED per finding. **Drop REFUTED;** keep CONFIRMED and PLAUSIBLE. For a PLAUSIBLE finding carrying a `needs_confirmation` question, phrase that finding as a question in the report rather than asserting it. This is verification, not re-grading: don't invent findings of your own, don't re-rank by personal taste, and don't refute a finding yourself — the verifier owns the refute call, and it's biased to keep ("PLAUSIBLE by default") so a real low-severity hit survives into the nudge band instead of being suppressed.
- **Preserve the P3 nudge band.** Each reviewer separates its gating findings from a low-severity "Minor / nudges" band (code-reviewer's P3 section, the other reviewers' advisory hits). Carry those through verbatim into the report's own Minor / nudges section — don't fold them into the gating list and don't drop them. A validated P3 a reviewer already surfaced (a new branch the diff shipped untested, a bare primitive where a codebase alias exists) is signal the author can scan and dismiss in one line; suppressing it is what makes a real finding look like a coverage miss. This is distinct from *inventing* low-value nits — see "What this review optimizes for".

The IaC and SQL reviewers already emit `P0`–`P3`, so their bands carry over directly.

| Source severity | Normalized |
|---|---|
| code-reviewer `[P0]`; security / GHA **Critical**; access bypass enabling cross-user/tenant data read or write; sql-reviewer `[P0]` (injection reachable from untrusted input) | **P0** |
| code-reviewer `[P1]`; security / GHA **High**; other IDOR / access-control gaps; Django perf **CRITICAL** (N+1, unbounded queryset); perf-reviewer `[P1]` (blocking I/O on async path, algo blow-up, per-item network loop, unbounded memory); iac / sql-reviewer `[P1]` (apply-failure, migration/DDL or transaction corruption) | **P1** |
| code-reviewer `[P2]`; security / GHA **Medium** or "Needs verification"; Django perf **HIGH** (missing index, write loop); perf-reviewer `[P2]` (missing app cache, misused concurrency); iac / sql-reviewer `[P2]` | **P2** |
| code-reviewer `[P3]`; perf-reviewer `[P3]` (micro-inefficiency); iac / sql-reviewer `[P3]`; anything advisory | **P3** |

Carry "Needs verification" findings through at their normalized band with the verification question intact — don't silently drop them.

Output has two finding bands plus the callouts. **Findings (P0–P2)** is the gating "fix before merge" list, grouped by normalized priority (P0 first), one finding per line, anchored and fingerprint-shaped:

```
[P0] path/to/file.py:42 | security | sql-injection-in-search — security-auditor (Critical, HIGH confidence)
[P1] path/to/views.py:88 | access  | idor-order-detail        — django-access-reviewer (High)
```

**Minor / nudges (P3)** is a trailing bullet list of the validated low-severity hits the reviewers surfaced (step 5's "Preserve the P3 nudge band"). Always emit it when any reviewer reported one; never drop it to make the report look tighter:

```
[P3] path/to/MessageBubble.tsx:170 | testing | inline-branch-untested — code-reviewer (P3)
```

The `file:line | category | slug` portion is a stable fingerprint downstream tools rely on. Keep `branch`-mode Human Reviewer Callouts as a final trailing section. For follow-up ("explain #2", "go deeper on the security findings"), re-invoke the relevant agent rather than answering from your own judgment.

## What this review optimizes for

The goal is catching the **P1 class** — runtime crashes, broken deploys/migrations, and security-boundary regressions — not driving any external review bot to zero findings. Don't *invent* low-value findings to look thorough: a duplicated constant, a mutable default that already has a guard, an unnecessary formatter-skip comment add noise, and bots produce false positives and retract findings too. But "don't pad" means don't manufacture nits — it is **not** license to drop a validated low-severity finding a reviewer actually surfaced. Those belong in the Minor / nudges band, where the author scans and dismisses them in one line; silently suppressing them is what makes a real finding look like a coverage gap (the failure mode this skill was bitten by). Spend the gating Findings band's attention on what would actually break in production, and let the nudge band carry the rest.