Michel Packmind Engineer Review

3. Run the checklist

Read references/engineer-checklist.md and apply the Cross-cutting section plus every layer section the diff touched. Skip sections for layers the diff doesn't touch — don't pad the report with N/A items.

Each checklist entry gives you the concern, the signal (how to spot it by reading the diff/code), and a real example from past reviews. Follow the signal: grep the changed files, open neighbouring files for context, trace a command field from frontend gateway → controller body → use case, look for the matching event subscriber, and so on. A finding is only worth reporting if you can point at the specific code.

Confidence discipline — match how the team actually reviews:

• State a finding plainly only when the diff makes it certain (e.g. a controller that never catches a domain error it can throw → guaranteed 500).
• When it depends on runtime behavior you can't observe statically (cache invalidation, a job that swallows an error, a toast duration), report it but mark it needs confirmation and say how to verify it.
• When it's a genuine design question (is this the right role? should this be a dedicated use case?), pose it as a question rather than asserting a defect.
• Don't manufacture findings to look thorough. Zero findings is a valid, good outcome — say QC ✅.

Scope calibration — the issue almost always describes more than any single diff contains (UI copy, an endpoint, analytics, ordering). Before flagging "the issue asks for X but I don't see it," decide whether the diff under review is meant to be the complete implementation or a slice of a larger feature still landing in sibling commits/PRs. If you reviewed the full feature span (step 1) and a stated behavior is genuinely absent, that's a real gap — flag it. If you can't tell whether it's deferred, raise it as an open question ("Is X handled elsewhere / in a follow-up?"), not a HIGH defect. Severity reflects shipped behavior: missing wiring in a deliberate slice is a scope note; missing wiring in something claimed done is a real finding. Don't inflate "not here yet" into "broken."

For a large diff (many files across several layers), you may fan out: launch one review subagent per touched layer (subagent_type: general-purpose), each given the issue summary, that layer's checklist section, and the relevant changed files, then merge their findings. For a small diff, review inline.

4. Write the report

Write to engineer-review-<issue-number>.md at the repo root (or engineer-review.md if there's no issue number) — unless the caller specified a different output path, in which case use that. Use this structure:

# Engineer Review — #<issue> <title>

**Issue**: #<number> | **Branch**: <branch> | **Base**: <short-sha> | **Files changed**: <N>
**Layers touched**: <list>

## Verdict

<One of:

- `QC ✅` — nothing to flag.
- `LGTM otherwise ✅, <N> point(s) below` — minor/non-blocking findings only.
- `<N> finding(s) to address` — at least one blocking finding.>

## Findings

#### [HIGH|MEDIUM|LOW] <short title>

- [ ] **Category**: <category from the checklist, e.g. "Authorization scoping">
- **File**: `path/to/file.ts:line`
- **What**: <what's wrong / the question, in the reviewer's voice>
- **Why it matters**: <user-visible or maintainability consequence>
- **Suggested check/fix**: <concrete next step; a fix suggestion or how to confirm>
- **Confidence**: certain | needs confirmation (static review)

<repeat per finding, ordered by severity>

## Open questions

<Design/scope questions that aren't defects — "Should this be Admin-only?", "Is this the expected
behavior?". Omit the section if there are none.>

---

_Static review only — no code was executed. Findings marked "needs confirmation" should be reproduced
before acting. Automated checks (lint, build, e2e) are out of scope here by design._

Severity guidance

• HIGH — wrong behavior reaching users, a security/authorization gap, data loss, or a guaranteed crash (unmapped domain error → 500, non-admin reaching an admin action, dropped command field on the wire).
• MEDIUM — incorrect-but-recoverable behavior, missing feedback, drift/duplication that will bite later, missing required tests, a type that hides a real contract.
• LOW / Not blocking — copy polish, accessibility nice-to-haves, minor consistency, dead code.

Conventions to mirror

• Findings start as unchecked - [ ] — the team checks them off (- [x]) as they resolve.
• Prefix genuinely optional items with "Not blocking but…".
• It's fine — encouraged — to end with QC ✅ or LGTM otherwise ✅ when warranted. Don't hedge a clean review into sounding alarming.

5. Print a summary

After writing the file, print to the console: the verdict, finding counts by severity, the count needing confirmation, and the report path.