# Cami Design Engineer

> Senior design-engineer code review of front-end code — component composition, design-system fidelity, state and data flow, behavior changes and regressions, cross-file completeness, accessibility, i18n, performance, security, TypeScript. Use when asked to review front-end, React, or UI code, to check a branch or PR for regressions, before handing a project to a tech team, or to make a prototype ship-ready.

- Skill: `pawlakcamille/cami-design-engineer` (Agent Skill)
- Install (CLI): `npx skillmds@latest add pawlakcamille/cami-design-engineer`
- Raw SKILL.md: https://api.skillmd.com/api/skills/pawlakcamille/cami-design-engineer/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Web & Frontend
- Author: PawlakCamille (https://skillmd.com/u/pawlakcamille)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/pawlakcamille/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 add`ed 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.

| Dimension | Reference | Load when |
| --- | --- | --- |
| Component Composition | `../cami-design/references/composition.md` | Component shape, prop surface, state location, compound patterns |
| Design System Fidelity | `../cami-design/references/ds-fidelity.md` | New or modified styled components, raw color/spacing values, DS imports, public API changes |
| State & Data Flow | `../cami-design/references/state.md` | `useState`, `useEffect`, async work, shared data fetching, state changing owner |
| Behavior Diff | `../cami-design/references/behavior-diff.md` | The diff modifies an existing conditional, early return, default value, dependency array, or the signature or return shape of a shared symbol |
| Cross-file Completeness | `../cami-design/references/cross-file-completeness.md` | The diff adds a union member — variant, status, tab, plan tier, role, feature flag — or moves/renames a module |
| A11y Implementation | `../cami-design/references/a11y-implementation.md` | Any interactive element, form, modal, image, custom widget |
| Internationalization | `../cami-design/references/i18n.md` | User-facing strings, dates, numbers, `aria-label`/`alt` text — when `package.json` has an i18n dependency (`i18next`, `next-intl`, `react-intl`…) or the repo has locale files |
| Performance & Rendering | `../cami-design/references/perf.md` | Lists, memoization, animations, heavy state, hot handlers, loading skeletons |
| Security Spot-Check | `../cami-design/references/security.md` | `dangerouslySetInnerHTML`, external links, clipboard/file/camera APIs, logged or persisted values |
| Type Safety & Code Clarity | `../cami-design/references/typing.md` | TypeScript annotations, file naming, comments, magic numbers |

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`.