From 37bed4f64c9ba64acd7f1108f9f339048932a96e Mon Sep 17 00:00:00 2001
From: NeilClack <cnclack@pm.me>
Date: Tue, 7 Jul 2026 22:29:13 -0500
Subject: [PATCH] Update AGENTS.md with detailed Svelte 5 gotchas, testing
 patterns, and admin form architecture

Covers select binding workaround, / sync pattern,
Playwright locator API, test patterns, dirty-state submit, and
Super-only proclamation restrictions.
---
 AGENTS.md | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 241 insertions(+), 4 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index e1233f0..86549ef 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -27,15 +27,17 @@ Requires `.env` with at least `DATABASE_URL`, `BETTER_AUTH_SECRET`, `BETTER_AUTH
 
 ## Testing quirks
 
-- **Two Vitest projects**: `client` (Playwright/Chromium headless, `src/**/*.svelte.{test,spec}.{js,ts}`) and `server` (node env, everything else). Tests co-located beside source files.
-- **`expect: { requireAssertions: true }`** — every test must assert something.
+- **Two Vitest projects**: `client` (Playwright/Chromium headless, `src/**/*.svelte.{test,spec}.{js,ts}`) and `server` (node env, everything else — `src/**/*.{test,spec}.{js,ts}` excluding `.svelte.*`). Tests co-located beside source files.
+- **`expect: { requireAssertions: true }`** — every test must assert something. No assertions = test fails.
 - **All route tests mock `$lib/db`** (Prisma client) and `$lib/auth-client`/`$lib/auth` via `vi.mock()`. Stub factories in `src/lib/test-utils.ts`.
 - **No `.env.test`** — mocking `$lib/db` makes real DB unnecessary.
+- **Mock `$app/forms`** for any component using `use:enhance` — provide `enhance: () => () => {}` (action → destroy fn).
+- **Mock TipTap** for client tests rendering ProclamationForm — the Svelte 5 component mock pattern is a function `($anchor: Element) => { …; $anchor.replaceWith(div); }`.
 
 ## Architecture notes
 
 - **Route groups**: `(protected)/` requires auth + profile; `(user)/` is user-facing; `admin/` requires roleId <= 3 (STAFF+).
-- **Svelte-Admin**: custom config-driven CRUD in `src/lib/svelte-admin/`. Models registered in `registry.ts`. Dynamic routes `[model]/[id]`.
+- **Admin forms**: Each route has its own form component (e.g. `ProclamationForm.svelte`, `CategoryForm.svelte`). The generic `AdminForm.svelte` / `AdminPage.svelte` were **deleted**. Registry config (`registry.ts`) still provides metadata, list fields, search fields, relation options, etc., but forms are hand-written per route.
 - **Prisma client** generated to `src/lib/generated/prisma/` (not `node_modules/.prisma/client`). Lazy singleton via Proxy in `src/lib/db.ts`.
 - **Better-Auth** server instance in `src/lib/auth.ts`, browser client in `src/lib/auth-client.ts`. Session fetched in `src/hooks.server.ts`.
 - **Roles**: integer hierarchy (SUPER=1, ADMIN=2, STAFF=3, USER=4). `canAccessAdmin` checks roleId <= 3.
@@ -44,7 +46,242 @@ Requires `.env` with at least `DATABASE_URL`, `BETTER_AUTH_SECRET`, `BETTER_AUTH
 ## Formatting conventions
 
 - Tabs, single quotes, no trailing commas, 100 print width (enforced by Prettier).
-- Svelte 5 runes only (`$state`, `$derived`, `$effect`, `$props`). No legacy stores.
+- Svelte 5 runes only (`$state`, `$derived`, `$effect`, `$props`). No legacy stores (`writable`, `derived`, etc.).
+
+---
+
+## Svelte 5 Runes Gotchas & Patterns
+
+### `$state` initialization vs `$effect` sync
+
+State variables declared with `$state('')` are set once on component mount. To keep them in sync with a changing `$props()` value, use a **separate `$effect`**:
+
+```svelte
+<script lang="ts">
+let { record = {} as Record<string, unknown> } = $props();
+
+let name = $state('');
+let freq = $state('');
+
+// Sync local state when prop changes
+$effect(() => {
+	name = String(record.name ?? '');
+	freq = String(record.solfeggioFrequencyId ?? '');
+});
+</script>
+```
+
+This is the **only correct pattern** for form fields that edit an existing record. Do NOT initialize `$state` directly from the prop — that only captures the initial value and won't re-sync when the parent changes the record (e.g. switching from one row to another in a modal).
+
+### `$effect` ordering matters
+
+Effects run in declaration order. If you have multiple effects, the first one's writes are applied before the second one runs. This matters for the `dirty` flag:
+
+```svelte
+let dirty = $state(false);
+
+// Runs first — resets dirty on mount / record change
+$effect(() => {
+	dirty = false;
+});
+
+// Runs second — syncs field values
+$effect(() => {
+	name = String(record.name ?? '');
+	// …
+});
+```
+
+Both effects run synchronously during mount, so by the time the browser paints, all state is consistent.
+
+### `bind:value` on `<select>` is broken with `$effect`-driven state
+
+**DO NOT USE** `bind:value={stateVar}` on `<select>` elements when the state is set from a `$effect`. Svelte 5's select binding does not respond to `$state` changes triggered inside `$effect`. Instead:
+
+**Use `selected` on `<option>` + `onchange` on `<select>`:**
+
+```svelte
+<select name="solfeggioFrequencyId" onchange={(e) => freq = (e.target as HTMLSelectElement).value}>
+	<option value="">-- Select --</option>
+	{#each options as opt (opt.value)}
+		<option value={opt.value} selected={String(opt.value) === freq}>{opt.label}</option>
+	{/each}
+</select>
+```
+
+This pattern applies to ALL `<select>` elements in form components. The `value={stateVar}` prop directive also does not work reliably — always use `selected` on options.
+
+### `bind:value` on `<input>` works fine
+
+Unlike `<select>`, `bind:value` on `<input type="text">` works correctly with `$effect`-driven state. Feel free to use it for text inputs.
+
+### `$props()` destructuring
+
+Always destructure `$props()` with defaults:
+
+```svelte
+let {
+	record = {} as Record<string, unknown>,
+	fieldErrors = {} as Record<string, string>,
+	isSuper = false,
+}: { record?: Record<string, unknown>; … } = $props();
+```
+
+The type annotation on the destructuring object is REQUIRED for Svelte 5 to infer prop types. Default values are used when the parent doesn't pass that prop.
+
+### Component mocking for client tests
+
+When mocking a Svelte 5 component in `vi.mock`, use a function that takes `($anchor, props)`:
+
+```ts
+vi.mock('$lib/components/tiptap/TipTap.svelte', () => ({
+	default: ($anchor: Element, _props: Record<string, unknown>) => {
+		const div = document.createElement('div');
+		div.setAttribute('data-testid', 'tiptap-mock');
+		$anchor.replaceWith(div);
+	}
+}));
+```
+
+---
+
+## Testing Patterns & Gotchas
+
+### What tests to write
+
+For every admin route, write BOTH:
+
+1. **Server test** (`page.server.spec.ts`): Test the `load` function and all `actions` (upsert, delete). Cover role-based access control (Super vs non-Super content stripping, rejection of unauthorized mutations). Mock `$lib/db` and `$lib/svelte-admin/crud`.
+
+2. **Client test** (`Component.svelte.spec.ts`): Test the form component rendering in Playwright/Chromium. Cover:
+   - All fields render
+   - Pre-fill on edit (name input, select values)
+   - Conditional field visibility (e.g. content hidden for non-Super)
+   - Submit button disabled/enabled by dirty state
+   - No visual regression testing — only functional assertions.
+
+### Available Playwright/Vitest Browser Locator API
+
+These methods are available on `page` from `vitest/browser`:
+
+| Method | Works? | Example |
+|--------|--------|---------|
+| `getByRole` | ✅ | `page.getByRole('textbox', { name: 'Name' })` |
+| `getByText` | ✅ | `page.getByText('Content')` |
+| `getByPlaceholder` | ✅ | `page.getByPlaceholder('Email Address')` |
+| `getByAltText` | ✅ | `page.getByAltText('Sign In with Google')` |
+| `getByTestId` | ✅ | `page.getByTestId('tiptap-mock')` |
+| `getByLabel` | ❌ | Not available in this version |
+| `getByDisplayValue` | ❌ | Not available in this version |
+
+For label-identified elements, use `getByRole` with `name` option:
+- Text inputs → `getByRole('textbox', { name: /^Name$/ })`
+- Selects → `getByRole('combobox', { name: /^Frequency$/ })`
+- Buttons → `getByRole('button', { name: 'Save' })`
+
+**Always use regex `{ name: /^Exact$/ }`** for role queries to avoid strict-mode violations from substring matches (e.g. `'Category'` matching both `'Category'` and `'Sub-Category'`).
+
+### Querying select values in client tests
+
+Since `bind:value` on selects doesn't work with `$effect`-driven state, and we use `selected` on `<option>` instead, you can check the select's value directly:
+
+```ts
+const el = await page.getByRole('combobox', { name: /^Frequency$/ }).element() as HTMLSelectElement;
+expect(el.value).toBe('2');
+```
+
+The `.element()` call returns the underlying DOM element. This works synchronously because `selected` is set during render.
+
+### Server test patterns
+
+**Always call `vi.clearAllMocks()` in `beforeEach`.**
+
+Mock `$lib/db` with the exact Prisma model methods the code under test uses:
+
+```ts
+vi.mock('$lib/db', () => ({
+	prisma: {
+		proclamation: {
+			findMany: vi.fn(),
+			findUnique: vi.fn(),
+			create: vi.fn(),
+			update: vi.fn(),
+			delete: vi.fn()
+		},
+		proclamationCategory: { findMany: vi.fn() }
+	}
+}));
+```
+
+Mock `$lib/svelte-admin/crud` when the route uses custom upsert/delete logic:
+
+```ts
+vi.mock('$lib/svelte-admin/crud', () => ({
+	loadRecords: vi.fn(),
+	deleteRecord: vi.fn(),
+	superFormErrorsToFieldErrors: vi.fn(() => ({ error: '', fieldErrors: {} }))
+}));
+```
+
+Use `createLocals(mockUser({ roleId: 4 }) as any)` to simulate different user roles.
+Use `createFormRequest({ name: 'test' })` for form submissions.
+Use `.catch((e) => e)` + `isHttpError()` for error assertions.
+
+### `createLocals` nuances
+
+- `createLocals()` (no arg) → unauthenticated (session is `undefined`).
+- `createLocals(mockUser() as any)` → authenticated with the mock user object.
+- Cast to `any` when passing to load/action functions: `{ locals: createLocals(mockUser()) } as any`.
+
+### `vi.mock` hoisting
+
+All `vi.mock()` calls must be at the top level of the test file. Values needed inside `vi.mock` factories must use `vi.hoisted()`:
+
+```ts
+const { mockFn } = vi.hoisted(() => ({ mockFn: vi.fn() }));
+vi.mock('$lib/some-module', () => ({ default: mockFn }));
+```
+
+Imports of the mocked module must come AFTER `vi.mock()` calls.
+
+---
+
+## Admin Form Architecture
+
+### Per-route form components
+
+Each admin route under `admin/[model]/` has its own form component (e.g. `ProclamationForm.svelte`). These components:
+- Receive `record`, `fieldErrors`, `formError`, `relationOptionsData`, and model-specific extras (e.g. `categories`, `isSuper`) as props.
+- Use `$state` + `$effect` for field values (see "Svelte 5 Runes Gotchas" above).
+- Use `selected` on `<option>` for selects.
+- Use `oninput` on the `<form>` to track dirty state: `<form oninput={markDirty}>`.
+- Submit via `action="?/upsert"` with SvelteKit's `use:enhance`.
+
+### Server actions
+
+Routes with custom requirements (like proclamation's Super-only restriction) override the generic `upsertRecord`/`deleteRecord` with their own actions. The generic functions in `crud.ts` are still available and used by simpler routes.
+
+### Super-only proclamation restriction
+
+- **Load**: content field is stripped from records for non-Super users before returning.
+- **Upsert**: returns `{ success: false, error: 'Only Super admins can edit proclamations' }` for non-Super.
+- **Delete**: same early return for non-Super; delegates to `deleteRecord` for Super.
+- **Content field in form**: wrapped in `{#if isSuper}` block — hidden from non-Super users.
+- **Row click / New button**: disabled for non-Super in `+page.svelte`.
+
+### Dirty-state submit pattern
+
+When editing an existing record (`record.id` is truthy), the submit button is disabled until the user modifies at least one field. The form tracks dirtiness with `let dirty = $state(false)` and a `markDirty()` handler:
+
+```svelte
+<form oninput={markDirty}>
+	<button disabled={!!record?.id && !dirty}>Save</button>
+</form>
+```
+
+The `$effect` resets `dirty = false` on mount (when the record prop changes).
+
+---
 
 ## Generated / build artifacts