shoc-frontend-new/ARCHITECTURE_AND_CODE_QUALITY.md

# ARCHITECTURE_AND_CODE_QUALITY.md — MUST / MUST NOT conventions

Mandatory React/TypeScript conventions for this repository. Each rule lists how
it is enforced. Detailed rationale for rendering and typography lives in
[docs/FRONTEND_MAINTAINABILITY.md](docs/FRONTEND_MAINTAINABILITY.md). When a rule
says "MUST", it is enforced by lint, build, or `npm run governance`; "SHOULD"
means it is a review-enforced convention backed by the
[PR review contract](REVIEW_AND_PR_FRAMEWORK.md).

## Conditional rendering

- **MUST** use `&&` or the `Text` `when` prop for one-sided conditions; never
  `cond ? <Element/> : null`. (ESLint `no-restricted-syntax`.)
- **MUST** make the left operand of `&&` in JSX entirely boolean. Coerce presence
  with `Boolean(value)` (or `Boolean(a || b)`); preserve numeric/empty-string
  semantics and TypeScript narrowing with `count > 0`, `value != null`.
  `{count && <X/>}` renders `0` and is rejected by the type-aware
  `seahaven/no-non-boolean-jsx-and` rule. (See FRONTEND_MAINTAINABILITY.md for
  the approved guard forms.)
- **MUST NOT** widen typed element-slot props (`icon`, `action`, `actions`) back
  to `ReactNode`; coerce with `Boolean(prop)` before `&&`.

## Typography and feedback

- **MUST** use `Text` from `@/components/ui/text` for paragraphs, headings,
  descriptions, labels, captions, code, and async feedback. Raw `<p>` and
  `<h1>`–`<h6>` are rejected. (ESLint `no-restricted-syntax`.)
- **MUST** use `Text variant="error"` for the `vp-error` styling; the
  `vp-error` class on any non-`Text` element is rejected
  (`seahaven/no-vp-error-outside-text`). Do not compose `vp-error` dynamically to
  bypass the static check.
- **MUST NOT** pass `component`, `role`, or `aria-live` to `Text` — they are
  omitted from `TextProps` so the variant contract (element, tone, live region)
  cannot be overridden. Use `as` and `tone`.
- **SHOULD** keep the `feedback` (polite `status`) region mounted and toggle text
  with `when`; mount `error` (assertive `alert`) on demand.

## Forms and mutations

- **MUST** use the libraries already established: **React Hook Form** for field
  registration/lifecycle, **Zod** for validation and inferred types, **TanStack
  Query** for server reads and mutations (pending/error state, invalidation,
  retries).
- **MUST NOT** add **TanStack Form** alongside React Hook Form for one form. A
  second form convention increases cognitive and dependency cost. Replacing
  React Hook Form requires an approved repo-wide migration ADR with measured
  benefits, a codemod/migration plan, and removal of the superseded dependency.
- **SHOULD** keep file upload selection/validation in a focused component and
  drive upload progress/errors/retry through a TanStack Query mutation, not in a
  route-sized page.

## State ownership (bounded)

- **MUST** keep query loading/error/empty state adjacent to the query result;
  mutation pending/error state in the component that initiated the mutation.
- **MUST** compose focused state components instead of accumulating unrelated
  booleans in a page. Route pages coordinate sections and navigation; reusable
  sections own their interaction details.
- **SHOULD** render errors inline with accessible feedback (`Text
variant="error"`); reserve toasts for cross-page outcomes.

## Data access

- **MUST** keep TanStack Query keys **stable and descriptive** (a consistent
  entity + identity tuple, co-located with the query). Unstable or ad-hoc keys
  break caching and invalidation. (Review-enforced; a future lint rule is
  tracked as a gap.)
- **MUST NOT** materialize large server collections into client state and then
  filter/sort them in the component when the server (or a memoized, virtualized
  layer) should own it. Prefer server-side filtering/pagination; if client-side
  is required, memoize and avoid re-filtering on every render. (Review-enforced.)
- **MUST** invalidate the right query keys after a mutation so caches do not
  show stale data.

## Security

- **MUST NOT** surface raw server error payloads, stack traces, or internal
  identifiers to end users. Map server errors to a safe user-facing message
  (e.g. `Text variant="error"`); log the full detail only to controlled channels.
  (Review-enforced.)
- **MUST NOT** bake secrets, tokens, or account-specific values into the build.
  Only `VITE_*` build-time vars are allowed, and they are baked into the bundle —
  never put a secret in a `VITE_` variable.

## Hooks, performance, and Big-O

- **MUST** satisfy `react-hooks` recommended rules (including `exhaustive-deps`)
  under the zero-warnings gate.
- **SHOULD** avoid `O(n²)` or worse work inside render/hot paths; memoize derived
  data, key lists stably, and prefer server-side filtering for large sets
  (reviewers flag algorithmic complexity — see REVIEW_AND_PR_FRAMEWORK.md).
- **SHOULD** keep renders pure; derive expensive values with `useMemo`/`useCallback`
  only when measured to matter (no speculative memoization).

## Maintainability (measured thresholds)

Enforced by `npm run governance` ([QUALITY_GATES.md](QUALITY_GATES.md)).
These are conservative code-shape proxies for review focus. File length is not
proof of a god object, cyclomatic complexity is not runtime Big-O, and neither
substitutes for behavior tests, profiling, query-plan evidence, or reviewer
judgment.

| Metric                | Threshold   | How enforced                                                                               |
| --------------------- | ----------- | ------------------------------------------------------------------------------------------ |
| File length (godfile) | ≤ 500 lines | Whole-repo ratchet: any file over the cap not in `scripts/governance-baseline.json` fails. |
| Cyclomatic complexity | ≤ 20        | Changed-file ESLint `complexity`.                                                          |
| Function length       | ≤ 150 lines | Changed-file ESLint `max-lines-per-function` (`skipComments`).                             |
| Parameters            | ≤ 4         | Changed-file ESLint `max-params`.                                                          |
| Nesting depth         | ≤ 4         | Changed-file ESLint `max-depth`.                                                           |

**Why these are changed-file ratchets, not whole-repo errors:** measured against
the current codebase, the maintainability thresholds surface **72 violations
across ~51 legacy files** (45 function-length, 24 complexity, 3 params), and
**5 files exceed 500 lines**. Applying them repo-wide at `error` would block
legacy without a migration. Instead:

- The **5 oversized files** are explicit grandfathered debt with frozen
  per-file caps in `scripts/governance-baseline.json`. They may shrink but never
  grow; new entries and cap increases fail.
- The **complexity/function-length/params/depth** rules apply to **changed**
  TS/TSX on a PR/push, so new and modified code must comply while untouched
  legacy is not blocked. Bring legacy into compliance when you next touch it.

These thresholds are starting ratchets: tighten them (lower caps, fewer baseline
entries) as debt is paid down. Never raise them to make a change pass.