coder · ibetitsmike · Jul 22, 2026
diff --git a/.agents/skills/deep-review/roles/frontend-reviewer.md b/.agents/skills/deep-review/roles/frontend-reviewer.md
@@ -6,6 +6,7 @@
 
 - Map every user-visible state: loading, polling, error, empty, abandoned, and the transitions between them. Find the gaps. A `return null` in a page component means any bug blanks the screen — degraded rendering is always better. Form state that vanishes on navigation is a lost route.
 - Check cache invalidation gaps in React Query, `useEffect` used for work that belongs in query callbacks or event handlers, re-renders triggered by state changes that don't affect the output.
+- Audit the diff against the FE rule contract in `.claude/docs/FRONTEND_PATTERNS.md` (FE1 to FE10) and cite rule IDs in findings.
 - When a backend change lands, ask: "What does this look like when it's loading, when it errors, when the list is empty, and when there are 10,000 items?"
 
 **Scope boundaries:** You review frontend code. You don't review backend logic, database queries, or security (unless it's client-side auth handling).
diff --git a/.agents/skills/frontend-review b/.agents/skills/frontend-review
@@ -0,0 +1 @@
+../../.claude/skills/frontend-review
diff --git a/.claude/skills/code-review/SKILL.md b/.claude/skills/code-review/SKILL.md
@@ -38,6 +38,9 @@ quality problems.
 - **Concurrency**: Race conditions, deadlocks, missing synchronization
 - **Resources**: Leaks, unclosed handles, missing cleanup
 - **Error handling**: Swallowed errors, missing validation, panic paths
+- **Frontend** (`site/src/`): audit against the FE rule IDs in
+  [Frontend Patterns](../../docs/FRONTEND_PATTERNS.md) and cite the rule ID
+  in findings (for example, "FE7: re-typed query key")
 
 ## What NOT to Comment On
 

diff --git a/.claude/skills/frontend-review/SKILL.md b/.claude/skills/frontend-review/SKILL.md
@@ -0,0 +1,94 @@
+---
+name: frontend-review
+description: Diff-scoped self-review of frontend changes under site/src against the FE pattern rules (FE1-FE10) before creating or updating a PR. Use whenever a branch diff touches site/src files.
+---
+
+# Frontend Review
+
+Audit the current branch diff against the frontend rule contract in
+[`.claude/docs/FRONTEND_PATTERNS.md`](../../docs/FRONTEND_PATTERNS.md) and fix
+every violation before the PR is created or updated. This is a self-review
+gate: its purpose is to catch the findings reviewers would otherwise post,
+before they see the PR.
+
+## When to run
+
+- Before creating a PR whose diff touches `site/src/`.
+- Before pushing significant new commits to an existing frontend PR.
+- Skip only when the diff touches no files under `site/src/`.
+
+## Workflow
+
+1. Collect the diff: `git diff --merge-base main -- site/src` (add
+   `site/e2e` if touched). Use `origin/main` instead when the checkout has an
+   `origin` remote and the local `main` may be stale. List the changed files.
+2. For each changed file, audit against each FE rule using the checklist
+   below. Read the full file when the diff alone cannot answer a check (for
+   example, whether a story's `play` function exercises the new behavior).
+3. Report results as a per-rule verdict table (see Output format). Every FAIL
+   must carry `file:line` and a one-line reason.
+4. Fix all FAIL findings with the smallest safe diff. Re-run the audit until
+   every rule passes or a remaining finding is explicitly justified.
+5. Include unresolved justifications in the PR description so reviewers see
+   them up front.
+
+## Per-rule diff checklist
+
+- **FE1 (Storybook coverage)**: Does any changed component or page alter
+  user-visible behavior? Then a changed or added `.stories.tsx` must exist,
+  and its `play` function must perform the new interaction (open the menu,
+  submit the form), not merely render. Interaction tests added to `.test.tsx`
+  files are a FAIL unless they cover pure logic.
+- **FE2 (types)**: Search the diff for `any`, `as unknown as`, non-null
+  assertions in any form (`x!.y`, `items[0]!`, `fn()!`, `value! as T`), and
+  new `as` casts. Check that API data uses types from `api/typesGenerated.ts`.
+- **FE3 (reuse/scope)**: For each new component, hook, or helper, search
+  `site/src/components/` and sibling folders for an existing equivalent.
+  Flag near-duplicates, hand-assembled versions of wrapped primitives, dead
+  branches, and unrelated changes bundled into the diff.
+- **FE4 (comments)**: Read every comment line the diff adds or edits. Flag
+  any comment that restates the identifier, assertion, or control flow.
+  Verify surviving comments are factually correct.
+- **FE5 (UI states)**: For each view rendering server data, confirm loading,
+  error, empty, and refetch handling. Flag form or selection state that a
+  background refetch would reset.
+- **FE6 (a11y)**: Flag interactive elements that are keyboard-unreachable,
+  `aria-label`s that replace visible label text, `aria-*` props that the
+  underlying primitive overwrites, and visually-hidden elements still in the
+  tab order.
+- **FE7 (react-query)**: Flag direct `API.*`/`fetch` calls in components,
+  string-literal query keys (must import the constant from `api/queries/`),
+  `isLoading || isFetching` patterns, missing invalidation on mutation paths
+  (including partial failure), and `mutateAsync` in `try/catch` with an empty
+  catch.
+- **FE8 (effects)**: For every added or modified `useEffect`, apply the
+  decision tree in FRONTEND_PATTERNS.md. Flag derived state via
+  `setState`-in-effect, fetches triggered by effects, new dependencies on
+  effects that own connections, and effects that only write refs nobody
+  reads.
+- **FE9 (fixtures)**: Flag inline entity literals that duplicate or deviate
+  from `Mock*` fixtures in `site/src/testHelpers/`, and shared pre-wired
+  query objects instead of per-story inline `{ key, data }` wiring.
+- **FE10 (test queries)**: Flag `querySelector`, class-name substring
+  matches, geometry assertions, `behavior: "smooth"` dependence, and
+  locale-less `toLocaleString()` in changed tests and stories.
+
+## Output format
+
+```
+FE1 PASS
+FE2 FAIL  site/src/pages/FooPage/FooPage.tsx:42  `as unknown as Workspace` cast
+FE3 PASS
+...
+```
+
+One line per rule. FAIL lines carry every finding (repeat the rule ID for
+multiple findings). After fixes, print the re-run table. The audit is done
+when all rules PASS or remaining FAILs have a written justification.
+
+## Notes
+
+- This audit does not replace `pnpm check`, `pnpm lint`, `pnpm format`, or
+  tests; run those too (see site/AGENTS.md Pre-PR Checklist).
+- Report findings in the current diff only. Do not refactor pre-existing
+  violations in untouched code; note them at most.
diff --git a/site/AGENTS.md b/site/AGENTS.md
@@ -273,6 +273,11 @@ Debug logs and pprof dumps use the same job name and commit SHA convention.
 3. `pnpm format` - Format code consistently
 4. `pnpm test` - Run affected unit tests
 5. Visual check in Storybook if component changes
+6. If the diff touches `site/src/`, run the `frontend-review` skill
+   (discovered from `.agents/skills/`; canonical copy at
+   [.claude/skills/frontend-review](../.claude/skills/frontend-review/SKILL.md)):
+   audit the diff against FE1 to FE10 and fix every FAIL before creating
+   the PR
 
 ## Migration (MUI → shadcn) (Emotion → Tailwind)