Frontend Review

Frontend Review

Audit the current branch diff against the frontend rule contract in .claude/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 .test.tsx covers the changed 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 (behavior and visual coverage): Does any changed component or page alter frontend behavior? The behavior should be covered by an existing or new Vitest test, and the new visual states by Storybook stories. Behavior tests should use Vitest, React Testing Library and userEvent. A play function may only drive state setup the screenshot needs (open the menu, type the text); flag assertions in a play function and behavior covered only by a story, and note that a valuable assertion belongs in a Vitest test. In the test, queries locate the element to interact with; the assertion must be the non-visual outcome (callback, request, state). An outcome assertion on what the DOM renders (toBeVisible, toBeInTheDocument, geometry, attribute presence) is flagged, hard stop: the story's screenshot already covers it. Flag tests written only to expand coverage when equivalent coverage already exists. Flag stories marked parameters.pixel.exclude: true that lack a stated reason or an equivalent test: an excluded story is never screenshotted, so it must justify why it is excluded and how its behavior is covered.
• 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. Flag new React hooks that an existing hook, a plain function, or component state could replace; several new single-use hooks in one diff is a FAIL.
• 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-labels 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. Flag any Object.defineProperty replacement of a browser global in tests or stories: unit tests stub with vi.stubGlobal, stories mock existing globals with spyOn from storybook/test.
• 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.