# Eng Code Review

> Review a diff or pull request for correctness, security, and performance, returning specific, severity-ranked, fix-ready findings anchored to file and line.

- Skill: `matrixx0070/eng-code-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add matrixx0070/eng-code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/matrixx0070/eng-code-review/raw
- Safety review: pending (external: skill-scanner PASS, skillspector PASS)
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: Matrixx0070 (https://skillmd.com/u/matrixx0070)
- Updated: 2026-09-08
- Page: https://skillmd.com/skills/matrixx0070/eng-code-review

---


## When to use

Any time you are handed a diff, PR, or patch to assess before merge. Review the change, but read enough surrounding context to judge its blast radius.

**Not for:** whole-codebase audits, style bikeshedding a formatter already handles, or rewriting the author's approach when the existing one is correct. Review what changed, not what you'd have written.

## Method

1. **Understand intent first.** Read the description and diff; restate what the change should do. If intent is unclear, that is finding #1 — stop and ask.
2. **Correctness.** Trace edge cases: empty/null inputs, off-by-one, timezone/locale, concurrency and races, error paths, partial failure, idempotency. If the change touches shared state or async flow, escalate scrutiny here.
3. **Security.** Check injection (SQL/command/template), missing authz, unvalidated input, secrets in code, unsafe deserialization, SSRF, and leaky error messages. If the diff touches an auth or trust boundary, treat every input as hostile.
4. **Performance.** Hunt N+1 queries, unbounded loops/allocations, missing indexes, synchronous work in hot paths, chatty network calls.
5. **Design & maintainability.** Naming, duplication, dead code, test coverage of the new logic, backward compatibility.
6. **Rank each finding** Critical / Major / Minor / Nit. Quote `path:line`, explain the concrete risk, propose a fix.
7. **Set the verdict from the worst finding**: any Critical or unaddressed Major ⇒ Request-changes or Block; only Nits ⇒ Approve-with-nits.

Prioritize findings that cause bugs, breaches, or outages over stylistic preference.

## Example

```
Finding (Critical) — src/api/orders.ts:88
  `db.query("... WHERE id=" + req.params.id)` — SQL injection via `id`.
  Why: `id` is unvalidated user input concatenated into SQL.
  Fix: use a parameterized query: db.query("... WHERE id=$1", [id]).
```

## Pitfalls

- **Approving because it "looks fine"** without tracing the error and concurrency paths.
- **Vague findings** ("this could be cleaner") with no line, risk, or suggested fix — unactionable noise.
- **Nit-flooding** that buries the one Critical under twenty style comments.
- **Missing the diff's blast radius** — a changed shared helper affects every caller; check them.

## Output format

```
Summary: <1-2 lines: intent + overall verdict>
Verdict: Approve | Approve-with-nits | Request-changes | Block
Findings:
  [Critical] path:line — issue — why it matters — suggested fix
  [Major]    path:line — ...
  [Minor]    path:line — ...
  [Nit]      path:line — ...
Questions for author: <blocking unknowns, if any>
```