code-review

Code review

Produce a focused, high-signal review of pending changes. Avoid drive-by nits; flag things that actually matter.

What "the changes" means

Resolve the review target in this order:

1. If the user passed a PR number or URL, fetch that PR's diff and metadata.
2. Else, if the working tree is on a feature branch, diff against the merge-base with the default branch (main or master).
3. Else, if there are uncommitted changes, review those.
4. If you can't determine a target, ask the user once before doing anything else.

For PRs, also pull: title, description, linked issues, and the list of files touched. For local diffs, run git log against the merge-base to see commit messages — they often reveal intent the diff doesn't.

How to read the diff

• Read the whole changed file when a hunk is non-trivial — diffs hide context (function being modified, surrounding invariants, callers).
• For each touched symbol, grep the repo for callers and tests. A change that looks local often isn't.
• Check the test files in the diff: do they actually exercise the new behavior, or just compile?
• If a config, schema, migration, or public API changed, check for backwards-compatibility implications.

What to look for

In rough priority order — stop reporting once signal drops:

1. Correctness bugs — off-by-one, wrong condition, missing await, swapped arguments, race conditions, null/undefined handling, error swallowing.
2. Security — injection (SQL, shell, HTML), auth/authz gaps, secret exposure, unsafe deserialization, SSRF, path traversal, missing input validation at trust boundaries.
3. Data safety — destructive operations without guards, migrations that can't be rolled back, schema changes without backfill, queries that lock or scan large tables.
4. Test coverage of the new behavior — not "is there a test" but "does the test fail if the implementation regresses?" Note missing edge cases (empty, error path, concurrency).
5. API / contract changes — breaking changes to public functions, types, HTTP routes, CLI flags, config keys, env vars; missing changelog or version bump where the project tracks them.
6. Performance traps — N+1 queries, accidental quadratic loops, large allocations in hot paths, sync I/O on a request thread.
7. Clarity — naming that misleads, dead code, half-finished refactors, comments that lie, magic numbers without explanation.
8. Consistency with the codebase — does it follow patterns already used here? If it diverges, is there a stated reason?

Skip: bikeshedding on style the formatter handles, restating what the diff already shows, "consider adding a comment" without saying what the comment should explain.

Output format

Group findings by severity. Use these exact labels:

• Blocker — must fix before merge (correctness, security, data loss).
• Should fix — meaningful issue, fix unless there's a reason not to.
• Nit — minor, optional. Cap at 3 total nits across the whole review; if you have more, you're being noisy.
• Question — something you can't determine from the diff and need the author to clarify.

For each finding:

- **<Severity>** `path/to/file.ext:LINE` — <one-sentence problem>. <One- to two-sentence explanation or suggested fix.>

Always include the file path and line number so the user can jump to it. If the finding spans multiple lines, cite the start line.

End with a one-paragraph Summary: overall verdict (ship / needs work / blocked), what the change does well, and the top 1–3 things to address.

When there's nothing to flag

Say so plainly. "Reviewed N files / M lines. No blockers or should-fixes. Two nits below if you care." Don't manufacture findings to look thorough.

Things to avoid

• Don't propose large refactors unrelated to the diff.
• Don't repeat the same finding in multiple places — cite once, link other locations.
• Don't quote large blocks of the diff back at the user; they wrote it.
• Don't comment on generated files, lockfiles, or vendored code unless something there is actually wrong.
• Don't post the review to GitHub unless the user explicitly asked.