---
name: code-review
description: >
  Review existing code for bugs, security problems and risky design: scope the
  review (git diff or files), outline and lint with a helper script, read
  function by function, verify each suspected bug with a concrete failing input,
  and write a ranked REVIEW.md. Use when the task asks to review, audit, check,
  or find bugs/problems in code - not for writing new features.
---

# Code review

The goal is a short list of REAL problems the author would want to fix, each
with evidence. Ten verified findings beat fifty guesses. A review does not
change the code unless the task explicitly asks for fixes.

## 1. Scope and groundwork (one command)
```
python <skill-dir>/review_prep.py                 # git: changed files; else all source files
python <skill-dir>/review_prep.py a.py b.js       # only these files
python <skill-dir>/review_prep.py --base main     # diff against a branch/commit
```
`<skill-dir>` is the absolute directory at the top of this text. Per file you
get: changed line ranges, an outline (`start-end  def name`), compiler/linter
output and hints. Linter output and hints are LEADS, not findings - many are
harmless in context. `NOT CHECKED` means no compiler was available: say so in
the report.

## 2. Understand intent before judging
Read what the code is supposed to do: the task text, README/AGENTS.md, module
docstrings, the tests. A "bug" is a difference between intent and behaviour -
without knowing the intent you can only report style.

## 3. Read function by function
- Use the outline: `read_file(path, start_line=a, end_line=b)` for ONE function
  at a time. Never read a whole large file; never re-read a range you have.
- Order: changed ranges first, then the functions they call and the callers
  of changed functions (`search_files("name\\(")`), then hotspots: input
  parsing, file/network I/O, error handling, concurrency, anything that builds
  shell commands, SQL, HTML or paths from input.
- For each function ask the questions in [checklist.md](./checklist.md).
  Load it once at the start: `load_skill("code-review", "checklist.md")`.

## 4. Verify every finding
For each suspected problem, write down a concrete failure scenario:
**input/state -> what happens -> what should happen**. Then:
- If it is cheap, PROVE it: a 1-5 line repro with `run_command`
  (`python -c "from calc import average; average([])"`, a tiny test file,
  `node -e ...`). Mark it **CONFIRMED** and quote the output.
- If a repro is not practical (needs network, GUI, a race), mark it
  **PLAUSIBLE** and state the exact preconditions.
- If you cannot state a concrete scenario, it is not a finding - drop it or
  list it under "Questions for the author".
Never put repro files into the project's source tree: use a `_review/` folder
and mention it in the report.

## 5. Report: write REVIEW.md
```markdown
# Review: <scope>   (files, base, date)
Checked with: <linters/compilers that ran; what was NOT CHECKED>

## Findings (most severe first)
### 1. [HIGH] <one-line claim>  - `file.py:42` - CONFIRMED
**Scenario:** `average([])` -> ZeroDivisionError is swallowed, returns None;
callers format None as "None %" (report.py:88).
**Evidence:** `python -c ...` -> `None`
**Fix:** raise ValueError for empty input (or return 0.0 if that is the intent).

## Questions for the author
- ...

## Not reviewed
- vendor/, generated files, ...
```
Severity: **HIGH** = wrong results, data loss, crash on normal input, security
hole. **MEDIUM** = crash/misbehaviour on edge cases, resource leaks, silent
error swallowing. **LOW** = maintainability problems that will likely cause
bugs later. Pure style (naming, formatting, import order) only if asked.

At most ~10 findings; if there are more, keep the most severe and say how many
smaller ones exist. End your answer with the count per severity and the path
to REVIEW.md - do not paste the whole report into the answer.

## Pitfalls
- Reporting a linter line verbatim without reading the code around it.
- "Could be None" without showing a call path where it IS None.
- Suggesting a rewrite instead of the smallest fix.
- Flagging framework conventions as bugs (read how the codebase does it elsewhere).
- Running out of steps on a big codebase: after the changed ranges and
  hotspots, stop and list the rest under "Not reviewed".