React code review standards

# React code review standards

Follow these standards when writing or changing code in this repository. Code review flags changes that break them.

## Remove Unnecessary useEffect and Derived State in React

Applies to `**/*.tsx`, `**/*.jsx`, `**/use*.ts`, `**/use*.js`.

Review React components and hooks for effects that duplicate render logic or chain state updates.

Flag changes that:
- Call a state setter inside useEffect with a value computed only from props or other state, such as a filtered list or a formatted label; compute it during render instead, with useMemo if it is expensive.
- Mirror a prop into local state with useEffect(() => setValue(prop), [prop]) instead of reading the prop directly or making the component controlled.
- Reset local state in an effect when an ID prop changes, where rendering the component with key={id} would remount it with fresh state.
- Chain effects where one effect sets state only to trigger another effect, so a single change cascades through several renders.
- Run user-triggered work, such as submitting a form, sending a click analytics event, or showing a toast, in an effect that watches a flag set by an event handler instead of in the handler.
- Notify a parent through an onChange prop inside an effect after local state changes, instead of calling it in the event handler that updates the state.

Do not flag:
- Effects that synchronize with systems outside React, such as subscriptions, timers, DOM measurement, third-party widgets, or network requests.
- State seeded once from a prop on purpose, such as an editable draft initialized from initialValue, when later prop changes should not overwrite it.
- setState calls that the project's ESLint config already reports as errors through react-hooks/set-state-in-effect, part of the eslint-plugin-react-hooks 7 recommended preset.

## Use Stable Keys Instead of Array Index in React Lists

Applies to `**/*.tsx`, `**/*.jsx`.

Review React list rendering for keys that do not identify the same item across renders.

Flag changes that:
- Use the map index as the key (key={index}, key={i}) on lists that can be sorted, filtered, or have items inserted or removed before the end, especially when rows hold local state, uncontrolled inputs, or focus.
- Generate keys during render with Math.random(), Date.now(), crypto.randomUUID(), or a counter, which remounts every row on each render.
- Build keys from fields that are not unique, such as a name, title, or date, so two items with the same value collide.
- Include editable fields in the key (key={item.name}, key={`${item.id}-${item.updatedAt}`}), so editing a row remounts it and drops its state and focus.
- Combine the index with other data (key={`${item.type}-${index}`}), which still changes when the order changes.

Do not flag:
- Index keys on lists that never reorder, filter, or change length and hold no state, such as a fixed set of labels or skeleton placeholders.
- IDs generated once when an item is created, such as crypto.randomUUID() called in the handler that adds an unsaved row.
- Composite keys built from stable fields, such as `${orderId}-${lineNumber}`, when no single ID exists.

## Require Effect Cleanup and Race-Safe Data Fetching in React

Applies to `**/*.tsx`, `**/*.jsx`, `**/use*.ts`, `**/use*.js`.

Review React effects for missing cleanup and for async results that can arrive after they are stale.

Flag changes that:
- Call addEventListener, setInterval, setTimeout, a store or socket subscribe, or an observer such as ResizeObserver inside useEffect or useLayoutEffect without returning a cleanup that removes, clears, unsubscribes, or disconnects it.
- Open a WebSocket, EventSource, or BroadcastChannel in an effect without closing it in cleanup.
- Remove a listener with a different function reference than the one added, such as a new inline arrow function, so the original listener stays attached.
- Fetch in an effect and set state from the response without an AbortController or an ignore flag, when the effect depends on values that change, such as an ID, search term, or route param.
- Set error state when a request fails because the cleanup aborted it, which shows a failure message for a request the user replaced.
- Await a request in an effect and then navigate, show a toast, or call a parent callback without checking that the effect is still current, which can redirect a user who already left the view.

Do not flag:
- Effects that leave nothing running, such as setting document.title or focusing an element.
- Data fetching through a library or framework that handles cancellation and stale responses, such as TanStack Query, SWR, or a route loader.
- Fetches in effects with an empty dependency array that only set state, where no newer request can race the first one.

## Prevent Unnecessary Re-renders from Unstable Values in React

Applies to `**/*.tsx`, `**/*.jsx`.

Review React context providers, memoized components, and effect dependencies for values that get a new identity on every render.

Flag changes that:
- Pass an object or array literal, or an inline function, as the value of a context provider consumed across much of the tree (auth, theme, app state, data clients) without useMemo or useCallback.
- Put fast-changing state, such as input text, pointer position, or a ticking timer, in the same context value as slow-changing data, so every consumer re-renders on each change; split the context instead.
- Pass new object, array, or inline function props to a component wrapped in memo, which defeats its shallow prop comparison.
- Pass a new object or function from a parent to a child that lists it as a useEffect dependency, rerunning the child's effect, and any fetch or subscription in it, on every parent render.

Do not flag:
- Code compiled by React Compiler (babel-plugin-react-compiler, or a framework setting such as reactCompiler in next.config), which memoizes values and callbacks automatically, unless the component opts out with "use no memo".
- Inline objects or callbacks passed to DOM elements or to components that are not memoized and are cheap to render.
- Primitive context values, or providers near the leaves with a handful of consumers.

## Handle Loading, Empty, and Error States in React

Applies to `**/*.tsx`, `**/*.jsx`.

Review React components that render fetched or async data for missing loading, empty, and error states.

Flag changes that:
- Render data from useQuery, useSWR, or a fetch in an effect without a loading state, leaving a blank area or a layout jump.
- Ignore the error from a data hook (error, isError) or a rejected request, so a failure renders as an empty or stale view with no message or retry.
- Map over a list with no empty-state branch when the list can be empty, such as search results, filtered tables, or a new account's records.
- Show the empty state while the first request is still pending, such as "No results" before any data has loaded.
- Suspend with use() or a suspense-enabled query without a Suspense fallback and an error boundary above it.
- Submit a form or run a mutation without a pending state (isPending from useTransition or useActionState, pending from useFormStatus) or a visible failure message, allowing double submits and silent failures.

Do not flag:
- Components that receive resolved data as props when a parent or route already handles loading and errors.
- Routes where framework files cover these states, such as Next.js loading.tsx and error.tsx.
- Background refetches that intentionally keep showing the previous data.

## Enforce React and Next.js Production Guardrails

Review React and Next.js changes for production-safe component boundaries, routing, and rendering behavior.

Flag changes that:
- Move pages or layouts to "use client" without a clear need for browser APIs, hooks, or event handlers.
- Put protected route checks only in middleware when the page or layout also needs server-side authorization.
- Use direct DOM APIs in React components instead of refs or framework APIs.
- Introduce hydration-prone code, such as reading browser-only state during server render.
- Add raw inline styles or hard-coded visual values when project design tokens or shared components exist.

## Enforce Accessibility Standards (WCAG 2.1 AA)

Check for proper alt text on images, logical heading hierarchy (one h1, no skipped levels), ARIA attributes on interactive elements, keyboard navigation support, and screen reader compatibility. Ensure all UI components meet WCAG 2.1 AA standards.

## Enforce Pragmatic Test Coverage

Be reasonable when evaluating tests. Flag changes when important behavior is untested, but do not require tests for every export, type, config, metadata file, or mechanical change.

Flag PRs that:
- Change core business logic without covering the main success and failure paths.
- Add UI behavior without testing the user-visible interaction or state change.
- Add tests that only assert rendering, existence, snapshots, or that a function returns any value.
- Skip, mute, or weaken existing tests without a clear reason.
- Mock so much of the system that the test no longer validates the behavior the PR claims to change.

## Flag Security Vulnerabilities

Scan for security flaws: hardcoded secrets, SQL injection, XSS, weak auth, exposed data, poor crypto, unsafe deserialization, insecure HTTP calls in production, and more. Ensure all network requests use HTTPS/TLS. Identify attack vectors and security violations to maintain robust defenses.

## Enforce Strict Maintainability Standards

Review changes for structural maintainability regressions. Behavior-correct code can still violate this rule if it makes the codebase harder to understand, extend, or decompose.

Be ambitious about structural simplification. Look for reframings that delete whole branches, helpers, modes, conditionals, or layers instead of adding more of them. Prefer solutions that remove complexity rather than rearrange it.

Flag PRs that:
- Push a file from under 1,000 lines to over 1,000 without a strong reason or clear organization. Prefer extracting helpers, subcomponents, modules, or local abstractions first.
- Add ad-hoc conditionals, scattered special cases, one-off booleans, nullable modes, or flags into already busy shared flows instead of a dedicated abstraction, helper, state machine, policy object, or separate module.
- Introduce thin wrappers, identity abstractions, or pass-through helpers that add indirection without reducing complexity.
- Use brittle, ad-hoc, or "magic" mechanisms that hide simple data-shape assumptions.
- Rely on unnecessary casts, any, unknown, or optional parameters when a clearer type boundary or explicit model would simplify control flow.
- Use silent fallbacks to paper over unclear invariants instead of making the boundary explicit.
- Duplicate existing helpers or put logic in the wrong layer when a canonical utility or module already owns the concept.
- Leak feature-specific logic into general-purpose or shared paths.
- Copy-paste logic instead of extracting a shared helper or pure function.
- Handle narrow edge cases in the middle of an already busy function.
- Refactor in ways that move complexity around without reducing the number of concepts a reader must hold in their head.
- Add "temporary" branching that is likely to become permanent debt.
- Serialize obviously independent work or leave related updates in a non-atomic shape when a cleaner structure is available.

Prioritize findings in this order:
1. Structural code-quality regressions
2. Missed opportunities for dramatic simplification
3. Spaghetti or branching complexity increases
4. Boundary, abstraction, or type-contract problems
5. File-size and decomposition concerns
6. Modularity and legibility concerns

Do not flag cosmetic nits when a larger structural issue is present. Prefer a smaller number of high-conviction findings over a long list of minor notes. Only report violations on changed lines with clear evidence from the diff.

## Enforce i18n for User-Facing Strings

All user-visible text must be wrapped in translation function calls (t(), useTranslation, etc.). This includes button labels, headings, error messages, and placeholder text. Raw string literals in JSX or template strings should be replaced with translation keys. Exception: Technical identifiers and logging messages.

## Flag AI Slop and Fabricated Changes

Catch AI-generated changes that look finished but aren't, such as placeholder or fabricated behavior, claims the code doesn't back up, and references that don't resolve.
Be conservative. Flag only obvious, evidence-backed low-quality AI output in the diff.

Flag changes that:
- Ship placeholder, mock, demo, fake-data, temporary debug, or filler text to production paths without a clear reason.
- Make claims in docs, comments, config, or PR text that contradict the implementation. Distinguish claims about current behavior or verification from documented plans or explicit statements that a capability is not supported.
- Reference files, commands, routes, functions, env vars, or config keys that do not resolve in their intended environment. Check the documented setup, including aliases, generated definitions, and externally supplied inputs.
- Add duplicated, patchwork, or overengineered code where a smaller cohesive solution would clearly work.
- Add narrating comments that restate obvious code instead of explaining intent or constraints.
- Change consequential behavior without meaningful verification, add tests that bypass the behavior they claim to cover, or weaken assertions to permit a regression. For coverage concerns, identify a realistic failure the verification would miss; consider existing shared tests and relevant type or static checks.

Do not flag:
- Generated files, fixtures, snapshots, migrations, vendored code, or mechanical renames unless one of the rules above clearly applies.
- Explicitly labeled scaffolding with concrete follow-up guidance, provided unfinished behavior is isolated from active production use. A TODO does not excuse fabricated results in a live path.
- Well-scoped AI-assisted code that is cohesive and tested.