# Website architecture audit: Template vs Client, Server Actions, Mutations, React Query

Contract baseline: [`docs/architecture/website/WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:1)

## 0) Executive summary

The codebase is **not uniformly wrong**; it is **inconsistent**.

Two competing architectures coexist:

1) **Contract-first RSC** (good):
   - `app/**/page.tsx` calls PageQuery → builds ViewData → renders Template (examples below).
   - Writes go through Server Actions → Mutations (examples below).

2) **React Query client app** (problematic when used for writes):
   - Client modules execute writes via React Query `useMutation()` directly against services (or placeholders).
   - This **violates the strict write boundary** in [`docs/architecture/website/FORM_SUBMISSION.md`](docs/architecture/website/FORM_SUBMISSION.md:20).

Additionally, some Templates violate determinism rules (notably `toLocaleString()` usage) which can cause SSR/hydration mismatches.

My recommendation to get to a clean codebase:

- Keep React Query **only for client-side reads** (and optional cache invalidation after server actions), but **ban React Query mutations for writes**.
- Enforce a single write path everywhere: **Client intent → Server Action → Mutation → Service → API Client**.
- Align Templates with determinism and responsibility rules: Templates render; they do not format using runtime-locale APIs.

---

## 1) What the code is doing today (evidence)

### 1.1 A good reference read flow (RSC → PageQuery → Template)

- Server page calls PageQuery and renders Template: [`DashboardPage()`](apps/website/app/dashboard/page.tsx:5)
- Template is a pure renderer receiving ViewData: [`DashboardTemplate()`](apps/website/templates/DashboardTemplate.tsx:21)

This matches the contract read flow in [`docs/architecture/website/WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:181).

### 1.2 A good reference write flow (Server Action → Mutation)

- Server action calls a mutation and revalidates: [`updateUserStatus()`](apps/website/app/admin/actions.ts:25)
- Server action calls a mutation and revalidates: [`updateProfileAction()`](apps/website/app/profile/actions.ts:8)
- Server action calls a mutation and revalidates: [`completeOnboardingAction()`](apps/website/app/onboarding/completeOnboardingAction.ts:18)

This matches the contract write flow in [`docs/architecture/website/WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:236) and the mutation guidance in [`docs/architecture/website/MUTATIONS.md`](docs/architecture/website/MUTATIONS.md:15).

### 1.3 React Query is definitely used, and it is used for writes

React Query is a dependency: [`apps/website/package.json`](apps/website/package.json:16)

React Query provider exists: [`QueryClientProvider()`](apps/website/lib/providers/QueryClientProvider.tsx:16)

There is a standardized abstraction that promotes React Query usage for both reads and writes:
- [`usePageData()`](apps/website/lib/page/usePageData.ts:25)
- [`usePageMutation()`](apps/website/lib/page/usePageData.ts:114)

Concrete contract violations:

- Client-side writes via React Query mutation calling a service:
  - [`useMutation()`](apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx:52) calling [`leagueService.createAdminScheduleRace()`](apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx:69) and similar write methods.
  - This is a direct breach of the strict rule: “All writes MUST enter through Next.js Server Actions” in [`docs/architecture/website/FORM_SUBMISSION.md`](docs/architecture/website/FORM_SUBMISSION.md:20).

- Duplicate write paths exist for onboarding:
  - Server action exists: [`completeOnboardingAction()`](apps/website/app/onboarding/completeOnboardingAction.ts:18)
  - But a React Query mutation hook also calls a service directly: [`useCompleteOnboarding()`](apps/website/hooks/onboarding/useCompleteOnboarding.ts:10)

- Some “mutation hooks” are placeholders (dangerous because they normalize the wrong path):
  - [`useJoinTeam()`](apps/website/hooks/team/useJoinTeam.ts:10)
  - [`useLeaveTeam()`](apps/website/hooks/team/useLeaveTeam.ts:9)
  - These currently do no server action and no API call, but they establish the pattern that client-side “writes” live in React Query.

### 1.4 Template vs Client responsibilities are inconsistent

A clean example of the intended split exists:

- Server page acts as data orchestrator: [`Page()`](apps/website/app/drivers/page.tsx:6)
- Client entry-point owns state and navigation: [`DriversPageClient()`](apps/website/app/drivers/DriversPageClient.tsx:30)
- Template is basically composition/rendering: [`DriversTemplate()`](apps/website/templates/DriversTemplate.tsx:38)

But the actual boundaries are blurry in at least two ways:

1) Templates contain non-deterministic formatting:
   - [`toLocaleString()`](apps/website/templates/DriversTemplate.tsx:65)
   - This violates strict determinism constraints in [`docs/architecture/website/VIEW_DATA.md`](docs/architecture/website/VIEW_DATA.md:58) and [`docs/architecture/website/DISPLAY_OBJECTS.md`](docs/architecture/website/DISPLAY_OBJECTS.md:71).

2) `app/**/*PageClient.tsx` sometimes becomes “everything”: state, rendering, presentation model transformation, and workaround ViewModel creation.
   - Example: “TODO wtf we have builders for this” comment while doing ViewData → ViewModel ad-hoc mapping in [`LeaguesPageClient()`](apps/website/app/leagues/LeaguesPageClient.tsx:418) and specifically the conversion at [`LeagueSummaryViewModel`](apps/website/app/leagues/LeaguesPageClient.tsx:380).

### 1.5 There are plain bugs/anti-patterns that amplify the mess

- `useState()` used as an effect runner (should be `useEffect()`):
  - [`useState()`](apps/website/app/leagues/LeaguesPageClient.tsx:297)
  - This is not an architecture issue per se, but it is “noise” that makes architecture discussions harder.

---

## 2) The root causes (why this became a mess)

1) **Write boundary not enforced strongly enough**
   - There are good server actions + mutation examples, but nothing stops new client code from doing `useMutation()` → `service.write()`.

2) **Templates and ViewData rules are not consistently applied**
   - Determinism rules (no `Intl.*`, no `toLocale*`) are strict in docs, but not enforced in Templates.

3) **Two read strategies are both used**
   - RSC PageQueries exist.
   - React Query hooks exist.
   - Without a clear “when to use which,” teams will mix-and-match.

---

## 3) Recommended target architecture (single coherent system)

### 3.1 Writes: one true path

Hard rule (enforced):

- Client code MUST NOT perform write HTTP requests.
- Client code MUST NOT call write methods on services that hit the API.
- Client code MUST NOT use React Query `useMutation()` for writes.

Canonical write flow (matches docs):

```mermaid
flowchart TD
  UI[Client component intent] --> SA[Server Action]
  SA --> M[Mutation]
  M --> S[Service]
  S --> AC[API Client]
  AC --> API[Backend API]
  SA --> RV[Revalidate Path]
```

Docs reference: [`docs/architecture/website/FORM_SUBMISSION.md`](docs/architecture/website/FORM_SUBMISSION.md:20)

### 3.2 Reads: choose a default, allow an exception

Default:

- If the route can be rendered on the server: `page.tsx` → PageQuery → ViewDataBuilder → Template.

Exception:

- Use React Query for client-only interactive islands that truly need client-driven data refresh.
- React Query should be treated as **a client state/cache** of authoritative API truth, not the truth itself.

Canonical read flow option A (RSC):

```mermaid
flowchart TD
  Page[page.tsx] --> PQ[PageQuery]
  PQ --> S[Service]
  S --> AC[API Client]
  PQ --> B[ViewDataBuilder]
  B --> T[Template]
```

Docs reference: [`docs/architecture/website/WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:181)

Canonical read flow option B (client-only island):

```mermaid
flowchart TD
  C[Client component] --> Q[React Query useQuery]
  Q --> S[Service or API Client read]
  C --> T[Template]
```

Important: This is only acceptable for reads. Writes still go through server actions.

---

## 4) Concrete refactor plan (incremental, low-risk)

### Step 1: Establish explicit conventions

1) `app/**/page.tsx` MUST be server by default and must only orchestrate:
   - call PageQuery
   - map Result → `notFound()`/`redirect()`/error UI
   - build ViewData using Builder
   - render Template

2) `app/**/*PageClient.tsx` is the ONLY place allowed to:
   - use `useRouter()`
   - hold UI state
   - call server actions

3) `templates/**/*Template.tsx` MUST:
   - accept ViewData (or client-only “presentation props” like handler functions)
   - contain no `Intl.*` or `toLocale*` formatting
   - contain no data fetching

### Step 2: Fix strict correctness violations first (determinism)

- Remove runtime-locale formatting from templates:
  - Replace [`toLocaleString()`](apps/website/templates/DriversTemplate.tsx:65) and [`toLocaleString()`](apps/website/templates/DriversTemplate.tsx:66) with deterministic formatting.
  - Preferred: ViewData already contains display-ready strings (built via ViewDataBuilder), or deterministic Display Objects.
  - Docs reference: [`docs/architecture/website/VIEW_DATA.md`](docs/architecture/website/VIEW_DATA.md:58).

### Step 3: Move client-side writes behind server actions

Primary offender to migrate:

- Replace React Query write mutations in [`LeagueAdminSchedulePageClient()`](apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx:22)
  - Replace [`useMutation()`](apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx:52) with server actions (publish/save/delete) that call Mutations.
  - Then the client component calls those server actions and refreshes view (either `router.refresh()` or route revalidation).

Secondary offenders:

- Consolidate onboarding writes to one path:
  - Keep server actions: [`completeOnboardingAction()`](apps/website/app/onboarding/completeOnboardingAction.ts:18) and [`generateAvatarsAction()`](apps/website/app/onboarding/generateAvatarsAction.ts:12)
  - Delete or rewrite client hook [`useCompleteOnboarding()`](apps/website/hooks/onboarding/useCompleteOnboarding.ts:10) to call server action instead of a service.

### Step 4: Clean up presentation model transforms

- Eliminate ad-hoc “ViewData → ViewModel” conversions in client code:
  - Example to refactor: the conversion at [`LeagueSummaryViewModel`](apps/website/app/leagues/LeaguesPageClient.tsx:380).
  - Replace with either:
    - a proper ViewModelBuilder (if the component truly needs ViewModel semantics), or
    - adjust the component (`LeagueCard`) to accept ViewData shape directly.

### Step 5: Reduce React Query surface area (optional, but recommended)

- Deprecate the abstraction that encourages client-side writes:
  - Keep [`usePageData()`](apps/website/lib/page/usePageData.ts:25) for reads if desired.
  - Remove or forbid [`usePageMutation()`](apps/website/lib/page/usePageData.ts:114).

### Step 6: Add guardrails to prevent regression

Minimal guardrails that pay off:

1) ESLint rule or convention: forbid `useMutation` imports except in `__tests__` or an allowlist.
2) ESLint rule: forbid service write methods from being imported into client modules (harder, but possible with path rules).
3) ESLint rule: forbid `toLocaleString()` and `Intl.*` in Templates.

---

## 5) Answers to your specific concerns

### 5.1 `*Template.tsx` vs `*Client.tsx`

- This split is **correct** and already present in multiple routes.
- The inconsistency is that some “Client” files are doing too much (transform, render, and mutate).
- The fix is to treat `*PageClient.tsx` as wiring + state + calling server actions, and keep templates deterministic.

### 5.2 Server actions vs mutations

- The codebase already shows the correct approach:
  - [`updateUserStatus()`](apps/website/app/admin/actions.ts:25)
  - [`updateProfileAction()`](apps/website/app/profile/actions.ts:8)

- The “mess” is client-side writes bypassing this.

### 5.4 Command Models: Are they useless?

No. Command Models (e.g., [`LoginCommandModel`](apps/website/lib/command-models/auth/LoginCommandModel.ts)) remain valuable as **optional UX helpers**.

- **Purpose**: They manage transient form state and perform client-side UX validation.
- **Relationship to Server Actions**: They prepare the DTO that is passed *to* the Server Action.
- **Boundary**: They belong to the client-side "intent collection" phase. Once the Server Action is called, the Command Model's job is done.
- **Redundancy**: They are **not redundant** with ViewModels. ViewModels are for the **read path** (API -> UI), while Command Models are for the **write path** (UI -> API).

### 5.5 Identified places for Command Model usage

The following components currently use complex React state for forms and should be refactored to use Command Models:

1.  **[`CreateLeagueWizard.tsx`](apps/website/app/leagues/create/CreateLeagueWizard.tsx)**: Needs a `CreateLeagueCommandModel` to manage multi-step wizard state and validation.
2.  **[`LeagueAdminSchedulePageClient.tsx`](apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx)**: Should use a `RaceScheduleCommandModel` for the track/car/date fields.
3.  **[`ProtestDetailPageClient.tsx`](apps/website/app/leagues/[id]/stewarding/protests/[protestId]/ProtestDetailPageClient.tsx)**: Should fully leverage the existing [`ProtestDecisionCommandModel`](apps/website/lib/command-models/protests/ProtestDecisionCommandModel.ts).
4.  **[`SponsorSignupPage`](apps/website/app/sponsor/signup/page.tsx)**: Needs a `SponsorSignupCommandModel` to clean up the `formData` and `errors` state.

---

## 6) Opinion: ViewData vs ViewModel (what actually makes sense)

Your original intuition is sound:

- **ViewData = server output** (JSON, deterministic, template-ready)
- **ViewModel = client-only** (state, derived UI helpers, never serialized)

The awkward part is: the repo currently contains **conflicting written contracts**.

- Strict statement: “Templates accept ViewData only” in [`docs/architecture/website/WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:372).
- But [`docs/architecture/website/VIEW_DATA.md`](docs/architecture/website/VIEW_DATA.md:34) shows an example where a Template receives a ViewModel in client code.

### My recommendation: keep Templates ViewData-only, keep ViewModels client-only

I would enforce this as the single, clean rule:

1) **Templates accept ViewData only** (one prop-shape; server and client use the same templates).
2) **Client components may use ViewModels internally** for state and derived values.
3) **The boundary is explicit**: client code converts `ViewModel -> ViewData` right before rendering a Template.

Why this is the least-bad option:

- It prevents a “two template APIs” world (one for ViewData, one for ViewModel) which tends to explode complexity.
- It aligns with RSC constraints (serializable data across the server/client boundary).
- It keeps ViewModels useful: they can own derived UI state and display logic, but the render surface is still a plain object.

Concrete example of why you feel “ViewModels are pushed away by ViewData”:

- In [`LeaguesPageClient()`](apps/website/app/leagues/LeaguesPageClient.tsx:418) you’re holding ViewData and then doing a hand-rolled mapping to a “ViewModel-like” shape at [`LeagueSummaryViewModel`](apps/website/app/leagues/LeaguesPageClient.tsx:380). That’s a symptom of **missing builders and an unclear boundary**, not a reason to abandon ViewModels.

## 7) Implementation Status (Audit Results)

The following cleanups have been implemented:

1.  **Deterministic Formatting**: Removed `toLocaleString()` and `toLocaleDateString()` from all templates. Introduced `NumberDisplay` and `DateDisplay` display objects for deterministic formatting.
2.  **Server Action Migration**: Migrated the schedule administration logic in `LeagueAdminSchedulePageClient.tsx` from React Query `useMutation` to Next.js Server Actions.
3.  **Onboarding Consolidation**: Updated onboarding hooks to use server actions instead of direct service calls.
4.  **ViewModel Builders**: Introduced `LeagueSummaryViewModelBuilder` to eliminate ad-hoc ViewData -> ViewModel mapping in `LeaguesPageClient.tsx`.
5.  **React Query Deprecation**: Deprecated `usePageMutation` in `usePageData.ts`.
6.  **Guardrails**: Added `gridpilot-rules/no-use-mutation-in-client` ESLint rule to prevent future React Query write usage.
7.  **Command Models**:
    - Created [`RaceScheduleCommandModel`](apps/website/lib/command-models/leagues/RaceScheduleCommandModel.ts) and integrated it into [`LeagueAdminSchedulePageClient.tsx`](apps/website/app/leagues/[id]/schedule/admin/LeagueAdminSchedulePageClient.tsx).
    - Created [`SponsorSignupCommandModel`](apps/website/lib/command-models/sponsors/SponsorSignupCommandModel.ts) and integrated it into [`SponsorSignupPage`](apps/website/app/sponsor/signup/page.tsx).
    - Ensured [`LeagueWizardCommandModel`](apps/website/lib/command-models/leagues/LeagueWizardCommandModel.ts) is ready for future refactoring of the complex wizard.

## 8) Next Steps for the Team

- Continue migrating remaining `useMutation` usages to Server Actions.
- Ensure all new templates follow the ViewData-only rule.
- Use `NumberDisplay` and `DateDisplay` for all UI formatting.