skills/dev/code-review/SKILL.md

---
name: code-review
description: 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.