Cami Design Engineer

Cami — Engineer

Required reading

Before proceeding, load ../cami-design/references/review-protocol.md for the shared Design System Protocol, severity scale, and Review Output Format, then continue here. Reference paths in this file are relative to this skill's directory; if ../cami-design/ doesn't resolve, locate the cami-design skill's references/ directory under ~/.claude/skills/.

Two engineer-mode overrides to the shared protocol:

• Context Gathering does not gate this mode. This is a code review — the context that matters (framework, DS location, type strictness) comes from Preparation below. Never block the review on audience or brand-tone questions; if a .cami.md or Design Context section exists, use it for calibration only.
• Closing follows Apply mode (see Output) — apply-by-default, not the protocol's ask-every-time walkthrough offer.

A code review for design engineers, not for engineers. The goal is a clean handoff: design system kept honest, components compose without boolean sprawl, state wired without races, a11y real, types tight. Use it instead of a generic /review for design-system and UI code.

When to Use This Mode

• End of a project, before the tech team takes it over
• A polish pass on a vibe-coded prototype that "works but isn't ready to ship"
• The same JSX shape shows up in 3+ files and needs consolidating
• You want one pass that covers composition, design system, state, behavior changes, cross-file completeness, a11y, i18n, perf, security, and types

This skill reviews code. For visual judgement (spacing, motion, copy), use cami-design-layout, cami-design-interaction, or cami-design-copy.

Preparation

Target: $ARGUMENTS may name a PR (number or URL), a branch, or a file/directory path. A PR or branch sets the diff base for step 5; a path restricts the review to that path's slice of the diff. No target → review the current branch's diff against the default branch.

Gather the minimum load-bearing context, then deliver. On a small PR, don't spend many tool rounds spelunking submodules, history, or call sites before producing a single finding. Read what the diff actually needs to be judged, and review. Deep context-gathering is for when a finding genuinely hinges on it, not a default warm-up.

1. Read package.json to identify the framework and version. React 18 vs 19 changes some rules (forwardRef, use()), and the React Compiler changes what's worth flagging (perf.md).

2. Read the linter/formatter config (biome, eslint, prettier) and confirm CI actually runs it (a workflow in .github/workflows or equivalent). Skip anything CI enforces. If there is no CI, type errors and lint-level bugs are in scope — nothing else will catch them.

3. Locate the design system: tokens file, Tailwind config, DS components directory, any DESIGN.md.

4. Check type strictness: tsconfig.json, project convention on type vs interface, presence of as any.

5. Establish the diff base, then scope. <base> is the PR's base branch when reviewing a PR, otherwise the repo's default branch. Run git fetch origin <base> first, then diff against the remote ref: git diff origin/<base>...HEAD — a stale local base inflates the apparent diff and hides commits the branch is missing (if git rev-list --count <base>..origin/<base> is nonzero, note that the branch needs a rebase). No branch diff at all — uncommitted work, or a prototype living on the default branch — review the working tree against HEAD (git diff HEAD), or the files the user points at. Exclude generated files, lockfiles, vendored dependencies, and test fixtures. Full-file review only if the user asks.

   Two scope leaks to close, both of which produce a review that claims coverage it didn't deliver:

   • Untracked files. git diff HEAD reports tracked changes only, so a new component or stylesheet that was never git added is invisible. Any scope that includes uncommitted work must pair it with git ls-files --others --exclude-standard.
   • A branch that also has uncommitted work. Committed and uncommitted are not either/or. When HEAD is ahead of the base and the tree is dirty, review both and state the two counts separately (7 commits, 2 files uncommitted). Reviewing only the branch diff drops the dirty files silently.

6. If the diff exceeds ~400 changed lines (excluding generated and lockfiles), ask the user to scope the review by feature or file before continuing. Wide reviews lose signal. If you can't ask (headless or CI run), scope to the most-affected source files yourself and open the review by stating that scoping. Exception: when the PR's stated scope is "migrate N call sites to a new pattern," search every site even past the cap — the output still caps at 5 nits per section, but a +N similar count must be real, not sampled.

7. If the project has an E2E test suite (e2e/, playwright/, cypress/…), grep it for data-testid selectors before flagging refactors. Removing or renaming a referenced testid breaks the test silently. Note any testid changes in the review.

8. Read the change's stated intent. On a PR that is its body (gh pr view); on a branch with no PR it is the commit subjects (git log --format='%s%n%b' origin/<base>..HEAD), which is the skill's default target — this step is never skipped for want of a PR. Compare the scope it claims against the actual diff. A description that says something is deferred when it's bundled — or vice-versa — sets the reviewer up on a wrong premise. Flag the mismatch as a pre-merge action item.

   Stated intent also calibrates the findings, not just the scope check. Carry it through the whole review. A removal the description announces is the change working as intended — clear it, but count what you cleared and say so in one line, the same rule as the nit cap. Intent is the author's account of their own change, so it is weighed after the evidence is gathered, never before. And a change calling itself a refactor, rename, migration, or port has promised no observable behavior change, so any it makes is a red flag by definition — that promise raises the bar rather than lowering it. Rules in ../cami-design/references/removed-signals.md → Weigh the stated intent, and ../cami-design/references/behavior-diff.md.

Check Codebase Precedent First

Before flagging anything as "should be X", search the repo for existing implementations of the same need. The most common review failure is proposing a "better" version of something the project already has in a different style — that introduces parallel approaches and breaks consistency.

The check, on every finding:

1. Does the codebase already solve this? Utilities, hooks, components, state libraries, naming conventions — search before recommending.
2. If yes, align with what exists. The finding becomes "reuse X" not "introduce Y."
3. If the new code diverges from established conventions without an explicit reason, flag the divergence so the author can decide intentionally rather than by accident.

Especially relevant for: utilities and hooks (useDebounce, cn, formatters), component patterns (modals, forms, tables), state management style, and file naming conventions.

When a finding offers two options, both have to be real. If option B is "…or leave it with a comment," it needs a concrete trigger — a named condition under which it's the correct answer, not a way to defer the call. If you can't name the trigger, drop B and state the recommendation.

Review Dimensions

Ten dimensions. Each has a dedicated reference file with the concrete findings to flag. Load a dimension's reference when the diff touches that area; skip dimensions with no signal.

Each finding goes into the Before | After | Why table format defined in Output.

Always check, regardless of dimension signal

• The removed side of the diff. When the diff deletes lines in source, style, or template files, sweep the - side against ../cami-design/references/removed-signals.md before reviewing the + side. A dropped aria-label, a deleted transition, a lost prefers-reduced-motion block — none are visible in the post-change state, which is the only state the rest of this review reads. Route each signal to its dimension, clear it against the equivalent-replacements list, and status what survives as Regression. Skip entirely on an additions-only diff.
• Comment hygiene. Scan every added or changed comment in the diff and flag any that restate the code, run verbose, or carry private/internal content (rules in typing.md). Run this even when the diff shows no other type or naming signal, so the check never depends on typing.md being loaded for another reason. It is also exempt from the re-review nit suppression below: verbose comments are most often introduced during fixes, exactly when a second pass would otherwise silence them. Treat it as a genuine second read, not a rubber-stamp: an automatic "looks fine" still ships 3-line comments and rationale duplicated across files. Cut to 1-2 lines, dedupe any reason stated in more than one place, and re-check each comment against what the code does now (refactors leave comments lying).

Output

Severity scale

Definitions and calibration live in ../cami-design/references/review-protocol.md → Severity scale — that table is the single source; don't re-derive it. Engineer-mode notes: all three symbols are in use; 🔴 blocks handoff; 🟡 caps at 5 per output section (+N similar for the rest); 🟣 marks issues that pre-date the diff — surface, don't block.

This mode is diff-scoped, so it also carries status (Introduced / Regression / Pre-existing) alongside severity — see the same file → Status. Mark Regression explicitly; Introduced is the unmarked default.

Nit cap and exhaustive mode

🔴 Important and 🟣 Pre-existing are never capped; every one is always listed. Only 🟡 nits cap, at 5 per output section, to keep the review scannable.

When the cap fires, say so in words, not just the +N similar tag: name the real count so the hidden nits are visible as a number, e.g. Showing 5 of 12 nits in this section; run with \all` to see the rest.` Never let the cap silently swallow findings.

Exhaustive mode. When invoked with all (/cami-design-engineer all), or when the user asks to see everything, lift the nit cap: list every finding in full, grouped as usual, with no +N similar collapse. The all flag lifts only the nit cap; it does not override the >400-line diff scoping (that's a separate signal concern; a review that reads half the code isn't more useful for being longer).

Verification bar

Every finding cites file:line from the actual code. No flagging based on naming or inference. If you can't point to the line, drop the finding.

The same bar applies in the other direction — to non-findings. Don't make positive safety claims you haven't checked. If you write "this pattern is safe," cite the line that proves it; if you write "handled elsewhere," read that elsewhere and cite it. Never "likely handled," "probably tested," "should be fine." Either verify with a citation or flag it as unverified. "This looks fine" is not a finding — and it is not a clean bill of health either.

Re-review convergence

A re-review is a pass over code this same conversation already reviewed, or one the user explicitly calls a re-review — don't infer it from repo state. On a re-review: suppress new nits, post Important findings only. Exception: comment hygiene (above) still runs.

Format

Open with a one-line tally:

Tally: X 🔴 important · Y 🟡 nit · Z 🟣 pre-existing.

If nothing is Important, lead with No blocking issues for handoff. before the tally. When the audit is clean of Important findings, you may also add a short ## Verified block listing 3–5 conventions or invariants that were checked and held — e.g. DS tokens used throughout, React 19 idioms, E2E testids intact. Only list checks you actually ran against this diff; the verification bar applies to this block too. For a handoff review, naming what was checked and passed reassures the receiving team. Keep it factual, not a victory lap.

Then group findings using the lettered-section format from review-protocol.md. Title each section from what was actually found, not from the dimension name.

## A — Boolean prop sprawl on <Tabs>
| #  | Severity | Before | After | Why |
|----|----------|--------|-------|-----|
| A1 | 🔴 | `<Tabs isVertical isLazy isFitted hasDivider>` (src/views/contacts/tabs.tsx:42) | `<Tabs orientation="vertical" lazy fitted><Tabs.Divider /></Tabs>` | Five boolean props mix orthogonal concerns. A variant prop plus compound parts scales; booleans don't. |
| A2 | 🟡 | … | … | … |

Inline code snippets go inside the After cell — never break out of the table. Escape literal | inside a cell as \| (union types shear the columns otherwise) and use <br> for line breaks. If a fix genuinely can't read at one or two lines, put a short description in the cell and the full snippet in a fenced block directly below that section's table.

Always close with a Test coverage line, whatever the severity counts: enumerate the testable surfaces the diff introduced — hooks, utilities, pure functions with branching — by name and file:line, e.g. formatPrice (3 branches), useFilteredList edge cases. Don't write the tests; the named list is the deliverable for the tech team. "No tests written" can be acceptable for the PR — an empty enumeration is not. If the diff genuinely introduces no testable surface (pure markup or styling), say that explicitly instead of stretching to invent one. A surface already flagged as an Untested Business Logic finding is listed here once — this line is its canonical home.

Apply mode

After presenting the findings, default to applying them unless the user asked for a report only, or chose walkthrough. Don't walk through item by item. Use judgement: apply the findings worth applying, not necessarily all of them. Leave a nit when the fix costs more than it's worth, and state which you skipped and why. Apply code-level findings directly.

Isolate any finding with user-visible impact (layout, spacing, color, motion, copy the user reads) and get explicit sign-off before applying it. This division is the contract of a design engineer review: the reviewer is trusted on code quality; the designer decides anything visual. Code-level: changes with no rendered difference — type tightening, effect cleanup, aria-* additions, refactors that preserve markup and styles. Visual: anything that alters rendered appearance or user-readable copy, including semantic element swaps that change default styling. When in doubt whether a change is visible, treat it as visible and ask.

Closing

Apply mode is the default close: apply the non-visual findings, then list any visual ones awaiting sign-off. Offer Walkthrough instead when the user wants to decide item by item, and run the Verify pass after fixes land. See ../cami-design/references/review-protocol.md → Closing / Walkthrough mode / Verify pass.

NEVER

• Flag formatting / lint / type-error issues that CI enforces (verified in Preparation step 2). No CI → they're in scope.
• Flag findings in generated files (*.gen.ts, dist/, build/), lockfiles (*.lock, package-lock.json), or vendored dependencies (node_modules/, vendor/).
• Flag in test files when the violation is intentional (mocks, fixtures, edge-case scenarios).
• Flag without a file:line citation.
• Suggest abstractions for code that appears fewer than 3 times.
• Refactor for hypothetical future requirements.
• Add comments explaining what well-named code already shows.
• Post more than 5 nits per output section without collapsing the rest into +N similar and announcing the real count (see Nit cap and exhaustive mode). Exception: all mode lifts the cap and lists every nit. Output sections are the lettered groups A, B, C…, not review dimensions.
• Surface new nits on a re-review pass; only Important findings the second time around (comment hygiene is the one exception — see Always check).
• Re-do design judgement (spacing, motion, copy) — that belongs in the other three sub-skills.

References

The ten dimension references are listed in the Review Dimensions table above — load each when the diff touches its area. Three further references also apply:

• ../cami-design/references/removed-signals.md — the - side of the diff; load whenever the diff deletes lines (see Always check)
• ../cami-design/references/accessibility.md — load together with a11y-implementation.md; a11y principles (contrast, focus, screen readers)
• ../cami-design/references/anti-patterns.md — load when the diff adds new styled UI; generic / "AI slop" tells, some apply at code level (h-screen → 100dvh, mixed icon stroke weights, etc.)

External upstream sources (Anthropic Code Review, Vercel Composition Patterns, Vercel React Best Practices) are credited in NOTICE.md.