Review diff — pre-PR code review

origin/main

Before You Start

• CLAUDE.md

  — the project's coding conventions and "Things to Know." The review is FROM this perspective.
• Any scope-level

  CLAUDE.md

  under subdirectories touched by the diff — scope-specific rules override root.
• Recent ADRs (

  docs/adr/

  ,

  docs/decisions/

  , or

  docs/arc42/decisions/

  ) — the why behind recent architectural rules.

Step 1: gather the diff

git fetch origin main
git diff origin/main...HEAD
git diff origin/main...HEAD --stat
git log origin/main..HEAD --oneline

Step 2: read affected CLAUDE.md files

CLAUDE.md

git diff origin/main...HEAD --name-only | awk -F/ '{print $1"/"$2}' | sort -u | while read d; do
  [ -f "$d/CLAUDE.md" ] && echo "READ: $d/CLAUDE.md"
done

Step 3: review checklist

Architecture and boundaries

• Code is in the correct scope (shared code in the project's designated shared library — common locations are

  src/lib/

  ,

  src/shared/

  , or a monorepo's core package — and scope-specific code in its scope).
• No cross-scope imports where the project's rule forbids them (check root

  CLAUDE.md

  for the dependency-direction rule).
• Layering respected (controller → service → repository or equivalent).
• New public APIs match the existing pattern — read an adjacent file for comparison.

Coding conventions

CLAUDE.md

• Use of typed errors vs generic

  catch (e)

  .
• Dependency-injection conventions.
• Async/sync discipline (e.g., no blocking I/O in async code paths).
• Naming patterns for new public symbols.

Security (OWASP-style pass)

• No string concatenation in SQL, shell, or logging.
• Auth and permission checks present on new endpoints.
• Secrets are not hardcoded; use the project's secrets mechanism.
• Input validated at boundaries (API params, form submissions, webhook payloads).
• Untrusted input not interpolated into templates or

  eval

  -like constructs.

Testing

• New code has corresponding tests.
• Tests assert something meaningful — not just "no exception."
• External services (DB, HTTP APIs, message brokers) are isolated or mocked appropriately.
• Test markers/tags (slow, integration, flaky) applied where the project uses them.

Performance (when relevant)

• No N+1 queries introduced.
• No loops with per-iteration network calls where a batch would do.
• Hot paths don't allocate unnecessarily.

Step 4: produce the review

file.ts:42

## Review of {branch} ({N} files, {+X -Y} lines)

## Critical (must fix before merge)
- `src/api/users.ts:42` — SQL injection via `getUserByName`: user input is concatenated into
  the query. Use the parameterized `db.query(..., [params])` form — see `src/api/orders.ts:78`
  for the pattern.

## Important (should fix)
- `src/services/payment.ts:15` — new service does not extend `ServiceBase`. Convention per
  root `CLAUDE.md` is that all services extend it for logging + error-wrapping.

## Suggestions (nice to have)
- `src/lib/utils.ts:22` — `formatDate` duplicates logic already in `src/lib/dates.ts:8`.
  Consider re-using or removing the duplicate.

## What looks good
- Error handling in `src/api/auth.ts` correctly uses `AppError` subclasses.
- New tests in `tests/integration/users.spec.ts` cover both happy path and permission denial.

Rules for the review output

• Be specific: file:line, not "somewhere."
• Be constructive: state the fix, not just the problem.
• Be honest: if the code is good, say so in What looks good.
• Skip nitpicks that the formatter or linter catches (principle 03 — don't duplicate tooling).
• Don't suggest adding backwards-compatibility unless the user asked.

Verify

# Did you miss any files in the diff?
git diff origin/main...HEAD --name-only | wc -l
# Your review should touch a substantial fraction of these (not necessarily every one —
# formatter-only changes and pure test additions may not need flagging).

Common Mistakes

CLAUDE.md