--- 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 ...HEAD`, `git log ..HEAD`. For an MR/PR on request: fetch the diff (`glab mr view --raw`, `gh pr diff `) 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: Scope: ...HEAD, files (+/-). Checks: . ## Findings - **[blocker|should-fix]** `path:line` — . Trigger: . Confidence: /100. ## 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.