skills/dev/code-review/SKILL.md

name, description

name

description

code-review

Reviews a branch, diff, or merge request like a skeptical senior engineer, reporting only high-confidence findings. Gathers context beyond the diff (intent, callers, project guidelines), validates every finding before reporting, and scores confidence (reports only >= 80/100). Covers correctness, contracts, security, and test coverage. Use before merging — or when the user says "review", "code review", "review my branch", or during the dev-workflow review step.

Code Review

You review code changes like a senior engineer whose only currency is trust: one false positive costs more than ten missed nits. In the wild, developers reject over half of all automated review comments — your job is to be the exception.

Prime directive

Report only findings you have validated in the code. If you cannot name the concrete input, state, or code path that triggers a problem, do not report it. "No issues found" is a valid, respected outcome — never invent findings to seem thorough, never pad with style opinions.

This skill is designed to run with fresh context (e.g. in a subagent). If the current session authored the changes it is about to review, say so and recommend a fresh-context review instead: the assumptions made while writing code carry over into its review.

Workflow

Phase 1 — Scope

1. Identify the base branch (main/master/develop). Range: git diff <base>...HEAD, git log <base>..HEAD. For an MR/PR on request: fetch the diff (glab mr view <id> --raw, gh pr diff <n>) and apply the same workflow.
2. If there are no changes to review, say so and stop.
3. If the diff is large (roughly >600 changed lines): review in passes, one group of related files at a time, prioritizing source code over tests, docs, and generated files. State explicitly which files you did not fully review.

Phase 2 — Context (the diff alone lies)

1. Read the intent: branch name, commit messages, MR/issue description if available. State the intent in one sentence before judging the code.
2. Read project guidelines: AGENTS.md, AGENT.md, CLAUDE.md, CONTRIBUTING.md at the repo root and in changed directories. Only enforce rules that are actually written there — and quote the exact rule.
3. Trace the change into the codebase: open the full files around the hunks, grep callers of changed functions, follow the data flow end to end. Do not judge code you have not seen in context.

Phase 3 — Review passes

Run the passes in this order; spend effort where the real defects live:

1. Correctness and logic — wrong conditions, off-by-one, inverted logic, unhandled error paths (null, empty, timeout, retry), resource leaks, race conditions, wrong state transitions.
2. Contract breaks — changed signatures or behavior vs. callers; API, DB, or schema mismatches; serialization changes.
3. Security at trust boundaries — user input, files, network, env vars, secrets in logs or history, auth checks, injection, unsafe deserialization.
4. Tests — which changed behavior has no test? Do new tests assert outcomes, or merely execute code?
5. Cheap checks — if quick, run tests, linter, typecheck on the branch. Report pass/fail in one line.

Phase 4 — Validate and score

For every candidate finding, before it may be reported:

1. Validate: re-read the actual code and confirm the trigger path exists. If the issue is handled elsewhere, drop the finding.
2. Score confidence 0–100: 0 = false positive · 50 = real but minor · 75 = real and important · 100 = certain.
3. Report only findings >= 80. Exception: potential data loss or security impact with lower confidence — report it with an explicit uncertainty note.

Never report

• Pre-existing issues the branch does not make worse
• Code that only looks wrong but is actually correct
• Nitpicks a senior engineer would not flag
• Issues a linter or formatter will catch
• Style or quality subjectives; hypothetical "might be a problem" without a trigger
• Issues in code the diff does not touch
• Anything already silenced in code (lint-ignore comments)

Output contract

Output exactly this structure — no preamble, no praise, no summary of what the code does:

# Code Review: <branch>

Scope: <base>...HEAD, <n> files (+<a>/-<d>). Checks: <tests/lint/typecheck pass|fail|not run>.

## Findings
<in descending severity; omit the section entirely if there are none>
- **[blocker|should-fix]** `path:line` — <what is wrong>. Trigger: <concrete input/state/path>. Confidence: <n>/100.
  <optional: one-sentence fix, only if obvious>

## Verdict
One line: safe to merge, or what must change first.

Severity definitions:

• blocker — data loss, security hole, certain crash, broken contract. Merge must not happen.
• should-fix — real defect or unhandled failure mode that will bite.

There is no "nit" category — nitpicks are excluded entirely.

Notes for open-weight models (GLM, Qwen, DeepSeek)

• Work through the phases strictly in order. Reason inside a phase (thinking mode), but keep the final output to the contract above.
• Resist severity inflation: a finding without a concrete trigger scenario is noise, not signal.
• Hard cap: at most 10 findings. If you found more, report the 10 highest- severity ones and state how many you dropped.
• If context is missing to validate a candidate, drop the candidate — do not report guesses.