docs: architecture audit — deepening opportunities for fdbck

Per-screen + per-server-helper audit. Identifies six deepening opportunities ranked by leverage: T1: §3.A per-question-type module bundle (highest leverage; closes a real server-side date_ranked_choice validation gap; unblocks i18n) T2: §3.C withOwnedInstance wrapper, §3.D findExistingSubmission helper, §3.F testability gaps T3: §3.B ChatPanel + FormEditor component extraction, §3.E feedback_instances repository Each candidate explained against the deletion test (LANGUAGE.md vocab): modules, depth, locality, leverage, seams, adapters. Tier-1 is the only load-bearing refactor; tier-2 are small independent wins; tier-3 is medium-cost cleanup that gets easier after tier-1. Anti-scope listed: no new deps, no CSS split, no real-time migration, no auth tier on /f/[slug], no port/adapter for fdb (one adapter = hypothetical seam, per LANGUAGE.md). 5 open questions for m at the end before any code lands. Design only — no source files touched. Awaiting m's pick before any coder shift.
2026-05-06 17:03:30 +02:00
parent 7f600a5279
commit c13d84d0f3
1 changed files with 734 additions and 0 deletions
--- a/docs/plans/architecture-improvements.md
+++ b/docs/plans/architecture-improvements.md
@@ -0,0 +1,734 @@
+# fdbck — architecture audit
+
+**Status:** design proposal, no code touched.
+**Branch:** `mai/cronus/architecture-audit` off main (`bc278a8`).
+**Author:** cronus, 2026-05-06.
+**Scope:** identify *deepening opportunities* — refactors that turn shallow
+modules into deep ones, with the aim of better testability and AI-navigability.
+
+m's brief: small codebase (~3 900 lines across 1 stylesheet, 3 components,
+6 routes, 12 server helpers, schemas, 8 API endpoints), perfect candidate for
+a focused architecture pass before the codebase calcifies.
+
+The doc is structured as: §1 quick survey, §2 friction findings (the seven
+coverage areas), §3 deepening opportunities with the "deletion test" applied,
+§4 a 1-pager prioritised proposal list. m picks; I implement what's chosen.
+
+---
+
+## 1. Survey
+
+### Map of the territory
+
+```
+src/
+├── app.d.ts
+├── hooks.server.ts            ← auth + public-scope policy gate (40L)
+├── lib/
+│   ├── components/
+│   │   ├── FormBuilder.svelte (438L) ← per-question-type editor cards
+│   │   ├── Icon.svelte        (97L)
+│   │   └── Results.svelte     (375L) ← per-question-type result charts
+│   ├── schemas.ts             (160L) ← every zod schema lives here
+│   ├── styles/
+│   │   └── feedback.css       (1500+L) ← tokens + every component
+│   └── server/
+│       ├── auth.ts, errors.ts, fdb.ts, feedback.ts, public-scope.ts,
+│       │ rate-limit.ts, request-context.ts, response.ts, results.ts,
+│       │ shlink.ts, supabase.ts (≈800L total across 11 files)
+│       └── (3 .test.ts files: rate-limit, public-scope, results)
+└── routes/
+    ├── +page.svelte, login/, /f/[slug]/+page.svelte (865L)
+    ├── admin/feedback/+page.svelte, [id]/+page.svelte (693L), new/+page.svelte
+    └── api/
+        ├── auth/{sign-in,sign-out}
+        ├── public/feedback/[slug]/{posts, results, submit, +}
+        └── admin/feedback/{+, [id]/+, [id]/{export, share, posts/[post_id]/hide}}
+```
+
+### Decisions inherited from `docs/plans/feedback-feature.md` (not up for revision)
+
+- **Slug = access token.** 32-char base62 (~190 bits), no auth on participant
+  side. Don't propose adding auth; m chose Security-by-Obscurity.
+- **In-memory rate-limiter.** Single Node instance; explicit "if scaled
+  horizontally, replace with Redis." Don't propose Redis until that scale.
+- **JSON form-definition.** No drag-drop form-builder. Visual editor exists
+  but JSON paste is the canonical interchange. (See `m/fdbck#1` history.)
+- **Polling at 3s for chat / 5s for admin refresh / 5s for live results.**
+  Real-time is "trivial migration when needed" but not in scope.
+- **Public-scope policy gate** is a security architecture decision (see
+  `flexsiebels#59`); `requireAuth` / `markVisibilityFiltered` /
+  `markDbAccessed` need to keep being called from every endpoint that
+  touches the DB. Don't propose dissolving the gate.
+
+### What's healthy already
+
+- Server modules are correctly sliced (auth / errors / rate-limit / public-scope
+  / request-context / fdb-schema-accessor / supabase-client / results
+  aggregation / shlink). Each file has one job and a small interface.
+- The discriminated-union `FeedbackQuestionSchema` is a sane *data* model.
+  The trouble is its *behavioural* fan-out (see §3.A).
+- The public-scope gate is genuinely deep: small interface
+  (`requireAuth` / `markVisibilityFiltered`), large guarantee (anonymous
+  responses can't accidentally leak DB data). It was clearly a model.
+- Versioning logic in `results.ts` is well-isolated: pure functions
+  (`todayVersion`, `nextVersionToday`, `nextVersion`) tested in
+  `results.test.ts`. Good seam.
+- Honeypot + rate-limit + closed-instance gates are consistent across
+  POST endpoints — same shape every time, no drift.
+
+---
+
+## 2. Friction findings
+
+For each, I name the modules involved, describe the friction, apply the
+deletion test ("if this disappeared, where would the complexity go?"), and
+note implications for tests, locality, and i18n.
+
+### 2.A — The question-type fan-out (the biggest)
+
+A "question type" lives, today, as a discriminated-union *strip* across the
+following files:
+
+| Concern | File | Lines |
+| --- | --- | --- |
+| zod schema branch | `lib/schemas.ts` | 23–64 |
+| default question stub | `FormBuilder.svelte` `defaultQuestion` | 64–88 |
+| editor card branches | `FormBuilder.svelte` template | 250–416 |
+| participant input branches | `routes/f/[slug]/+page.svelte` | 634–757 |
+| participant answer summary | `routes/f/[slug]/+page.svelte` `summariseSubmittedAnswer` | 326–362 |
+| participant required-validation | `routes/f/[slug]/+page.svelte` `submitForm` | 220–235 |
+| admin submissions-cell summary | `routes/admin/feedback/[id]/+page.svelte` `summarizeAnswer` | 297–312 |
+| results aggregation | `lib/server/results.ts` `emptyStats` / `ingest` / `finalise` | 105–250 |
+| results chart branches | `lib/components/Results.svelte` template | 200–360 |
+| public-results sanitization | `lib/server/results.ts` `publicResults` | 252–263 |
+| server submit required-validation | `api/public/feedback/[slug]/submit/+server.ts` | 37–47 |
+| CSV column expansion | `api/admin/feedback/[id]/export/+server.ts` | 105–130 |
+
+That's **twelve places** for one concept. m's commit history confirms the
+pain: m/fdbck#1 (`date_ranked_choice`) needed schema + builder + participant
+renderer + results aggregator + Results.svelte chart + CSV expansion, and the
+strip even diverged: the server-side required-validation in `submit/+server.ts`
+quietly skips `date_ranked_choice` (it only checks for empty-string /
+empty-array, not "no options rated"), so the gate is only enforced
+client-side. That's a real bug-shape bug, not just untidy code.
+
+Two small navigation papercuts inside this strip:
+
+- `FormBuilder.svelte`'s `setScaleLabel` is misnamed — it only handles
+  `date_ranked_choice`'s nested `scale.{min,max}_label` (early-returns on
+  every other type), while the standalone `scale` question's labels use
+  inline handlers. Half-grown helper from the m/fdbck#1 add.
+- `summarizeAnswer` (admin) and `summariseSubmittedAnswer` (participant) are
+  near-homophones with different signatures (`(v)` vs. `(q, v)`) and
+  divergent locales (`Yes/No` vs. `Ja/Nein`) and divergent date_ranked_choice
+  rendering (`"3.4 avg (5 rated)"` vs. per-option list). Same job, two code
+  paths.
+
+**Deletion test:** delete any single one of these twelve callers, the
+complexity reappears immediately at the other eleven. They all earn keep.
+But they don't have to live in twelve files.
+
+### 2.B — Page-level `.svelte` files have grown into kitchen sinks
+
+| File | Lines | Concerns it juggles |
+| --- | ---: | --- |
+| `routes/f/[slug]/+page.svelte` | 865 | localStorage session + display name; chat polling; per-user color hashing; 3 different time formatters; compose autosize + IME-aware Enter; single-submission server-load + client-retry; live-results polling; per-question-type input branches; per-type answer summary; submit; reset-and-resubmit |
+| `routes/admin/feedback/[id]/+page.svelte` | 693 | 4-tab mode; status pill optimistic toggle; share strip + replace; click-outside menu; full edit form (title/desc/toggles/visual/JSON); form-version pill; polling refresh; CSV/JSON export; submissions table; chat moderator |
+| `routes/admin/feedback/new/+page.svelte` | 281 | Title/desc/toggles + the same Visual/JSON segmented editor as `[id]/+page.svelte` (literal copy of `editForm` / `editFormJson` / `switchEditMode` / `ensureBuilderForm` from the detail page) |
+
+Both detail pages of the admin flow share a question-authoring sub-tree
+that's been *copy-pasted*. The participant page has a chat sub-tree that
+mirrors (loosely) the admin chat moderator UI but with bubbles instead of
+boxes.
+
+**Deletion test:** if we delete the participant chat block (≈ 200 lines), the
+behaviour vanishes. If we extract a `<ChatPanel slug sessionId
+isMine={...}>` component, the same behaviour survives and now has *one*
+place that knows how to fetch / post / autosize / IME / colorize / scroll.
+Concentrates ~250 lines into one file with a small props interface. Deep.
+
+Same for `<FormEditor bind:value bind:editMode>` — the Visual/JSON segmented
+editor used in two places. ~80 lines saved on the create page; the edit-tab
+on the detail page also shrinks. Already partially modularised via
+`FormBuilder.svelte` (the visual mode), but the `editFormJson` /
+`syncJsonFromVisual` / `syncVisualFromJson` / `switchEditMode` helpers are
+*not* in `FormBuilder.svelte` — they sit at page level twice.
+
+### 2.C — The `requireAuth → ownerOf → try/catch` pattern is shallow but copied
+
+Every admin API endpoint opens with the same 4–6 lines:
+
+```ts
+const err = requireAuth(locals.userId);
+if (err) return err;
+try {
+  const inst = await getInstanceById(params.id);
+  if (!inst) return notFound('Instance not found');
+  if (inst.owner_user_id !== locals.userId) return unauthorized('Not your instance');
+  // ... actual handler ...
+} catch (e) {
+  return handleApiError(e, '<context-tag>');
+}
+```
+
+Affected: `/api/admin/feedback/[id]/+server.ts` (defines `ownerOf` helper —
+only used inside this file), `/api/admin/feedback/[id]/posts/[post_id]/hide`,
+`/api/admin/feedback/[id]/share`, `/api/admin/feedback/[id]/export`. The list
+endpoint `/api/admin/feedback/+server.ts` has the auth half but not the
+ownership half (lists by `owner_user_id = userId`).
+
+The `ownerOf` helper *exists* in one file and the others copy its body
+inline. Adding a fifth admin-instance endpoint means re-opening that strip
+again.
+
+**Deletion test:** delete the inline 6-liners and replace with a
+`withOwnedInstance(handler)` wrapper. Complexity moves into one wrapper,
+then disappears as new endpoints are added. The wrapper is genuinely deep
+because:
+
+```
+interface: handler({ inst, params, request, locals }) → Response
+implementation: 30 lines of auth + ownership + error handling
+leverage: every admin-instance endpoint is N lines shorter
+```
+
+Today's `ownerOf` is a *partial* version of this — it returns
+`{ ok, response | inst }` but each endpoint still writes the destructure +
+the `try/catch` around it.
+
+### 2.D — "Find existing submission" appears in three places
+
+The single-submission gate looks up "did this client submit before?" in
+three different code paths with three different shapes:
+
+| Caller | Lookup keys | Shape returned |
+| --- | --- | --- |
+| `routes/f/[slug]/+page.server.ts` (server load) | IP + UA only | `previousSubmission \| null` |
+| `api/public/feedback/[slug]/+server.ts` (client retry GET) | session_id OR IP+UA | `previousSubmission \| null` |
+| `api/public/feedback/[slug]/submit/+server.ts` (gate on POST) | session_id OR IP+UA | `existing \| null` (then 409) |
+
+Same `select`, same `eq` columns, same ordering. Three near-identical
+PostgREST queries.
+
+**Deletion test:** delete the inline triple and replace with
+`findExistingSubmission(instId, { sessionId, ip, ua })` returning a
+`SubmissionLookup | null`. The complexity (the IP+UA fallback rule, the
+session_id-first ordering, the `maybeSingle()` behaviour) concentrates in
+one helper. Deep.
+
+### 2.E — Schema ergonomics
+
+`schemas.ts` is 160 lines and currently fine. It already uses
+`z.discriminatedUnion('type', [...])` with a base extended per type. m
+asked whether breaking it into per-question-type files would help future
+adds.
+
+My read: **per-question-type files are useful only as part of §2.A** (a
+per-type module that bundles schema + UI parts + server logic together).
+Splitting just the schema is a paper-shuffle and would *worsen* navigation
+because you'd lose the at-a-glance discriminated-union view. The right
+unit is "everything about question type X in one file," not "everything
+about question type X's *schema* in one file."
+
+The non-question schemas in this file (`FeedbackInstance{Create,Update}`,
+`FeedbackSubmission`, `FeedbackPost`, `FeedbackPostHide`, `SignIn`,
+`ShareCreate`) belong together — they're API request bodies. They could
+live in `lib/api-schemas.ts` while question schemas move to per-type
+modules. Mild win, not load-bearing.
+
+### 2.F — Testability gaps
+
+Today: `rate-limit.test.ts` (4 cases), `public-scope.test.ts` (full table of
+allow/block decisions), `results.test.ts` (aggregation + version stamping).
+
+Untested:
+- **Slug generation.** `generateSlug` returns 32 base62 chars. Not tested
+  for length, alphabet, or birthday-collision behaviour over a large pool.
+- **`stampVersion`.** `nextVersion` (pure) is tested; the wiring that
+  reads `feedback_submissions.form_snapshot.version` and *picks* the next
+  one is not. This is the entry point that future edits will most likely
+  break. Needs an integration test with a fake fdb client.
+- **Per-question-type validation roundtrips.** Each branch of
+  `FeedbackQuestionSchema` accepts a valid example and rejects bad shapes.
+  Catches drift like the `date_ranked_choice` server-side gap.
+- **Server-side required-question gating.** Submit handler with a missing
+  required answer of every type. Would have caught the
+  `date_ranked_choice` gap.
+- **CSV column expansion.** Per-type. The `[opt_id]` suffix path is
+  fragile and only exercised against the export endpoint manually.
+- **Public-results sanitization.** `publicResults` strips text answers;
+  one-line test would lock the contract.
+- **Single-submission gate.** The IP+UA + session_id matrix has at least
+  6 paths (cleared-storage / new-IP / different-UA / first-submit /
+  resubmit-attempt / closed-instance). Today only tested by m clicking.
+- **`findExistingSubmission`** (when extracted, §2.D) — 6-row table test.
+- **`withOwnedInstance`** (when extracted, §2.C) — auth-fail / not-found /
+  not-yours / pass-through.
+
+If §3.A's per-type module bundle lands, each type's module gets a small
+unit-test file and most of these gaps close per-type. The other gaps are
+cross-cutting (slug, stampVersion, withOwnedInstance, findExistingSubmission)
+and need their own tests.
+
+### 2.G — i18n future (m/fdbck#3)
+
+Hardcoded strings live inline:
+
+- **German** in `/f/[slug]/+page.svelte`: "Anonym", "Sende …", "Absenden",
+  "Ja", "Nein", "Diese Feedback-Sitzung ist geschlossen — neue Beiträge sind
+  nicht mehr möglich.", "Live-Feedback", "Fragebogen", "Live-Ergebnisse",
+  "Antwort gesendet", "Noch eine Antwort senden", "Du hast bereits
+  abgesendet…", "Bitte beantworte: {label}", "Bitte bewerte mindestens eine
+  Option: {label}", "passt nicht", "passt super", "Beitrag entfernt",
+  "(von Moderation entfernt — du siehst es noch)", "Nachricht…", "Senden"
+- **English** on admin pages: "Forms", "New form", "Save", "Edit",
+  "Delete", "Open", "Closed", "Copy link", "Replace short link", "Yes",
+  "No", "responses", "messages", "anonymous", every menu item, every
+  banner.
+
+Today's architecture **hurts** i18n because each per-question-type strip
+(§2.A) carries inline strings that an i18n migration would need to find
+and rewrite *separately* in each strip. After §2.A's per-type module bundle:
+each type owns its strings and can take the locale through one chokepoint
+(`registry[q.type].render(q, answer, ctx)` where `ctx` carries `t()`).
+Same string moves once, not 12 times.
+
+The participant footer string `fdbck.msbls.de` is fine as branding; the
+trust-evoking copy on the participant page is German because that's the
+audience. Rewriting all of it for i18n is m/fdbck#3's job — but the
+*architecture* shouldn't multiply the work by 12.
+
+### 2.H — AI-navigability for a coder picking up cold
+
+A new agent (or a returning future-you) reading this codebase needs to
+answer "how does a question type work?" The current answer is "open these
+8–12 files in this order." That's the high-water mark of friction.
+
+After §2.A: "open `lib/questions/<type>.ts`." One file per type tells the
+whole story.
+
+Other navigability friction:
+
+- The `parseFormDefinition` quirk (supabase-js with `.schema('fdbck')`
+  returning JSONB as encoded strings) is documented in `feedback.ts:46`
+  and applied inside `getInstanceBy{Slug,Id}`. But the admin list endpoint
+  `/api/admin/feedback/+server.ts` and the detail endpoint
+  `/api/admin/feedback/[id]/+server.ts` GET both **don't** call it on the
+  rows they return. Two of these routes happen to render via Svelte which
+  doesn't care; one renders via JSON which means the browser receives a
+  JSON-encoded string for `form_definition` and has to `JSON.parse` it.
+  Inconsistency. A thin Repository wrapper that *always* normalises would
+  make this invisible.
+- `void q;` at line 219 of `results.ts` is a "tell the linter the
+  parameter is intentionally unused" hack. The function takes `q` but the
+  switch statement only ever needs `s`. Either drop `q` from the
+  signature or use it for a defensive check.
+- `(s as ScaleStats & { _sum?: number })._sum` cast pattern (results.ts:162,
+  211) — using a private temp on the public stats object during
+  aggregation, then `delete`ing in `finalise`. Functional, but readers
+  pause. A separate `Map<qid, sum>` or a `WIPStats` type used during
+  aggregation and converted to public `Stats` in `finalise` would be
+  clearer. Low priority.
+
+### 2.I — CSS coupling (low priority, mentioned for completeness)
+
+`feedback.css` is now 1500+ lines, single file. Tokens + viewport reset +
+header + banner + section + question + option + scale + button + chat
+(admin) + bubble (participant) + tabs + builder + results + cards + menus
+ status pill + segment + share strip + closed line + compose + summary +
+date-ranked + per-question density + dark-mode overrides for each cluster.
+
+Splitting into `tokens.css`, `forms.css`, `chat.css`, `admin-list.css`,
+`admin-detail.css` would help locate styles. But the file *reads* fine
+today — clear comment-banner sections separate clusters. Defer until it
+hurts.
+
+---
+
+## 3. Deepening opportunities
+
+For each candidate, I name the modules, restate the problem in one
+sentence, sketch the solution in plain English, and explain the benefits
+in terms of locality + leverage + tests.
+
+### 3.A — Per-question-type module bundle
+
+**Files:** `lib/schemas.ts` (lines 23–64), `lib/components/FormBuilder.svelte`,
+`lib/components/Results.svelte`, `lib/server/results.ts`,
+`routes/f/[slug]/+page.svelte`, `routes/admin/feedback/[id]/+page.svelte`,
+`api/public/feedback/[slug]/submit/+server.ts`,
+`api/admin/feedback/[id]/export/+server.ts`.
+
+**Problem:** every question type is implemented as a 12-place strip; m/fdbck#1
+demonstrated the cost (and shipped a server-side validation gap as a
+side-effect). Adding a new type touches 8–12 files in the same review.
+
+**Solution:** introduce `lib/questions/` with one file per type
+(`scale.ts`, `single-choice.ts`, `multi-choice.ts`, `boolean.ts`,
+`short-text.ts`, `long-text.ts`, `date-ranked-choice.ts`) plus a
+`registry.ts`. Each module exports a value satisfying:
+
+```ts
+interface QuestionTypeModule<T extends FeedbackQuestion['type']> {
+  type: T;
+  schema: ZodType<Extract<FeedbackQuestion, { type: T }>>;
+  defaultStub(): Extract<FeedbackQuestion, { type: T }>;
+  isAnswerEmpty(question, answer): boolean;
+  emptyStats(question): QuestionStats;
+  ingest(stats, question, answer, createdAt): void;
+  finalise(stats): void;
+  sanitizeForPublic(stats): QuestionStats;
+  csvColumns(question): { header: string; key: string; optId?: string }[];
+  csvCellFor(question, answer, col): string;
+  // UI:
+  BuilderEditor: SvelteComponent;
+  ParticipantInput: SvelteComponent;
+  AdminCellSummary(question, answer, locale): string;
+  ResultsBlock: SvelteComponent;
+}
+```
+
+The schema discriminated-union becomes
+`z.discriminatedUnion('type', QUESTION_MODULES.map(m => m.schema))`. The
+participant page becomes `<ParticipantInput {q} bind:answer />`. The admin
+table cell becomes `registry[q.type].AdminCellSummary(q, a, locale)`. The
+aggregator becomes `registry[q.type].ingest(...)`. The CSV expansion
+becomes `for col of registry[q.type].csvColumns(q): out.push(...)`. The
+required-validation gate (server + client) reduces to
+`if (q.required && registry[q.type].isAnswerEmpty(q, answers[q.id])) ...`
+— ONE rule, two callers, no drift.
+
+**Benefits:**
+
+- **Locality**: a new type = one file + one line in `registry.ts`. Today's
+  12-place strip becomes one place.
+- **Leverage**: the registry's interface is small (~10 named exports per
+  type) but the implementation behind it covers the whole app's
+  per-type behaviour.
+- **Tests**: every module gets a colocated `<type>.test.ts`. The empty-
+  answer rule, the schema roundtrip, the aggregation, the CSV expansion
+  all become unit-testable in isolation. The server-side gap that exists
+  today closes by construction (one `isAnswerEmpty`, two callers).
+- **i18n** (m/fdbck#3 prep): each module's `AdminCellSummary` and the
+  ResultsBlock take a locale via context; one chokepoint per type.
+- **AI-navigability**: "how does a `boolean` question work?" → open
+  `lib/questions/boolean.ts`. Done.
+
+**Cost:** non-trivial rewrite. Probably 1 commit per question type
+(7 commits) plus a CSS-friendly slot system if the BuilderEditor /
+ParticipantInput / ResultsBlock components are pulled out. The Svelte
+side may need a `<svelte:component this={registry[q.type].ParticipantInput}>`
+pattern — Svelte 5 handles this cleanly. **Largest deepening on this list.**
+
+### 3.B — `<ChatPanel>` + `<FormEditor>` extraction
+
+**Files:** `routes/f/[slug]/+page.svelte` (the chat half),
+`routes/admin/feedback/[id]/+page.svelte` (the chat-tab + the edit-tab),
+`routes/admin/feedback/new/+page.svelte` (the edit-tab clone).
+
+**Problem:** participant page is 865 lines doing 9 concerns; admin detail
+is 693 doing 9 concerns; the new-form page literally copy-pastes the
+edit-tab's `editForm` / `editFormJson` / `switchEditMode` helpers from
+the detail page.
+
+**Solution:**
+
+1. **`<ChatPanel slug={inst.slug} sessionId chatStatus role="participant|moderator" />`**
+   — owns: `posts` state, polling, `postChat`, `fetchPosts`, autosize,
+   IME-aware Enter, color hashing, time formatting, scroll-to-bottom.
+   Used in both `/f/[slug]` (participant role) and `/admin/feedback/[id]`
+   Chat tab (moderator role with extra hide button). One component,
+   two render variants.
+2. **`<FormEditor bind:value={editForm} bind:json={editFormJson} bind:mode={editMode} />`**
+   — owns: visual / JSON segmented toggle; `syncJsonFromVisual`,
+   `syncVisualFromJson`; the `<FormBuilder>` mount; the JSON textarea +
+   Insert-sample button. Used in both `/admin/feedback/new` and
+   `/admin/feedback/[id]` Edit tab.
+
+**Benefits:**
+
+- Page files shrink to ~400 / ~450 / ~150 lines.
+- The chat fetch/post protocol (lastSeenAt cursor, deduping by `id`,
+  optimistic append) lives in one place. Today it's *almost* duplicated
+  between participant and admin Chat tab — admin polls via the detail
+  endpoint instead of `/posts`, so the de-dup logic is participant-only,
+  but the bubble vs. box renderer is the only real difference.
+- Tests: `<ChatPanel>` becomes mountable in component tests. Today the
+  chat behaviour is untestable through its current interface (it's
+  inlined in a 865-line page).
+
+**Cost:** medium. Extract + rewire both pages. Independent of §3.A.
+
+### 3.C — `withOwnedInstance` admin-route wrapper
+
+**Files:** `api/admin/feedback/[id]/+server.ts` (defines `ownerOf`),
+`api/admin/feedback/[id]/posts/[post_id]/hide/+server.ts`,
+`api/admin/feedback/[id]/share/+server.ts`,
+`api/admin/feedback/[id]/export/+server.ts`.
+
+**Problem:** every admin-instance endpoint opens with the same 6-line
+auth + ownership + try/catch preamble. The `ownerOf` helper exists in
+one file and is duplicated inline in three others.
+
+**Solution:** lift the preamble into a wrapper:
+
+```ts
+// lib/server/admin-route.ts
+export function withOwnedInstance<P>(
+  handler: (ctx: { inst: FeedbackInstance; event: RequestEvent<P> }) => Promise<Response>,
+  context: string,  // for handleApiError
+): RequestHandler<P> {
+  return async (event) => {
+    const err = requireAuth(event.locals.userId);
+    if (err) return err;
+    try {
+      const inst = await getInstanceById(event.params.id);
+      if (!inst) return notFound('Instance not found');
+      if (inst.owner_user_id !== event.locals.userId) return unauthorized('Not your instance');
+      return await handler({ inst, event });
+    } catch (e) {
+      return handleApiError(e, context);
+    }
+  };
+}
+```
+
+Each endpoint becomes:
+
+```ts
+export const POST = withOwnedInstance(async ({ inst, event }) => {
+  const body = await parseBody(event.request, FeedbackPostHideSchema);
+  // ... only the actual work ...
+  return json({ post: data });
+}, 'admin feedback post hide');
+```
+
+**Benefits:**
+
+- **Locality**: the auth+ownership rule is one place; new endpoints can't
+  forget it.
+- **Tests**: `withOwnedInstance` itself becomes testable (no auth, no
+  instance, wrong owner, happy path). Today every endpoint re-tests this
+  by inspection.
+- **AI-navigability**: a coder reading the handler sees only the
+  endpoint-specific logic; the security envelope is implicit.
+
+**Cost:** small. One helper, four call-site rewrites. Independent of §3.A
+and §3.B.
+
+### 3.D — `findExistingSubmission` extraction
+
+**Files:** `routes/f/[slug]/+page.server.ts` (lines 17–38),
+`api/public/feedback/[slug]/+server.ts` (lines 24–70),
+`api/public/feedback/[slug]/submit/+server.ts` (lines 55–93).
+
+**Problem:** the single-submission lookup query (session_id OR IP+UA,
+ordered by created_at desc, limit 1) is inlined three times. The
+participant load path uses IP+UA-only by design; the API GET and submit
+paths use both.
+
+**Solution:** add a helper to `lib/server/feedback.ts`:
+
+```ts
+export interface SubmissionLookup {
+  submitted_at: string;
+  display_name: string | null;
+  answers: Record<string, unknown>;
+}
+
+export async function findExistingSubmission(
+  instanceId: string,
+  by: { sessionId?: string; ip?: string; ua?: string },
+): Promise<SubmissionLookup | null> {
+  // session_id first (if provided), then IP+UA fallback (if both provided),
+  // mirrors today's three call sites.
+}
+```
+
+**Benefits:**
+
+- **Locality**: the priority rule (session-id-first, IP+UA-fallback) lives
+  in one place. Today changing it requires touching three queries.
+- **Tests**: one matrix test — `(sessionId yes/no) × (ip yes/no) ×
+  (ua yes/no) × (db row matches yes/no)`.
+
+**Cost:** small. Independent of everything.
+
+### 3.E — Repository for `feedback_instances` row normalisation
+
+**Files:** `lib/server/feedback.ts` (`parseFormDefinition`),
+`api/admin/feedback/+server.ts` (skips it),
+`api/admin/feedback/[id]/+server.ts` (skips it, returns the raw row to JSON).
+
+**Problem:** supabase-js with `.schema('fdbck')` returns `form_definition`
+JSONB as a JSON-encoded *string* instead of a decoded object. Two
+callers normalise via `parseFormDefinition`; two callers don't, leaking
+encoded strings into JSON responses where the browser has to
+double-decode.
+
+**Solution:** wrap fdb-query of `feedback_instances` in a small repository
+that always normalises:
+
+```ts
+// lib/server/feedback-repo.ts
+export async function findInstanceBySlug(slug): Promise<FeedbackInstance | null>;
+export async function findInstanceById(id): Promise<FeedbackInstance | null>;
+export async function listInstancesForOwner(userId): Promise<(FeedbackInstance & { counts })[]>;
+export async function updateInstance(id, patch): Promise<FeedbackInstance>;
+export async function deleteInstance(id): Promise<void>;
+export async function insertInstance(input): Promise<FeedbackInstance>;
+```
+
+Every endpoint goes through this repo; no caller writes raw `fdb().from(...)`
+queries against `feedback_instances`. The string-decoding wart hides
+inside the repo.
+
+**Benefits:**
+
+- **Locality**: the supabase-js JSONB quirk lives in one place. The
+  current "remember to call `parseFormDefinition`" trap goes away.
+- **Tests**: repo methods are mockable. Today the admin endpoints are
+  hard to unit-test because they speak SQL directly.
+- **Adapter signal**: today there's *one* implementation
+  (`getSupabaseAdmin().schema('fdbck')`). The repo creates a hypothetical
+  seam (no second adapter today). **One adapter = hypothetical seam, not
+  yet a real seam** per LANGUAGE.md — so this is a lower-priority "set
+  up the seam, don't over-design it." Don't introduce a port/adapter
+  pattern; just collapse the inline queries into typed repo methods.
+
+**Cost:** medium. Many call sites. Probably 2–3 commits.
+
+### 3.F — Closing the testability gaps
+
+Already covered point-by-point in §2.F. Recommended unit tests:
+
+| Test | File | Lines (rough) |
+| --- | --- | --- |
+| `generateSlug` length / alphabet / collision | `lib/server/feedback.test.ts` | ~30 |
+| `findExistingSubmission` matrix | `lib/server/feedback.test.ts` | ~80 (after §3.D) |
+| `withOwnedInstance` 4 paths | `lib/server/admin-route.test.ts` | ~60 (after §3.C) |
+| `parseFormDefinition` round-trip + already-decoded passthrough | `lib/server/feedback.test.ts` | ~15 |
+| `stampVersion` integration with fake fdb | `lib/server/feedback.test.ts` | ~80 |
+| Per-type schema accept-valid / reject-bad | per-type `lib/questions/<type>.test.ts` | ~20 each (after §3.A) |
+| Per-type empty-answer / required-violation | per-type | ~15 each (after §3.A) |
+| Per-type CSV expansion roundtrip | per-type | ~20 each (after §3.A) |
+| `publicResults` strips text answers | `lib/server/results.test.ts` (extend) | ~15 |
+| Honeypot drops submission silently | `routes/api/public/feedback/[slug]/submit/+server.test.ts` | ~30 |
+
+The biggest payoff is the per-type tests post-§3.A, because they sit
+beside the implementation and don't need integration scaffolding.
+
+---
+
+## 4. Prioritised proposals (1-pager)
+
+m picks one or more; cronus then implements per shift. Each proposal lists
+its dependencies on others.
+
+### Tier 1 — biggest leverage, biggest commit
+
+1. **§3.A — Per-question-type module bundle.** Highest leverage on this
+   list. Closes the existing server-side `date_ranked_choice` validation
+   gap by construction. Sets up i18n cleanly. Roughly 1 commit per type
+   (7 commits) plus a registry-and-shape commit; medium-large cost. **My
+   recommendation: do this first** if m has appetite for a focused 1–2-
+   shift refactor. Everything else gets easier afterwards.
+
+### Tier 2 — independent, small commits
+
+2. **§3.C — `withOwnedInstance` wrapper.** Small, localised, removes a
+   real copy-paste. Independent of §3.A. ~1 commit.
+3. **§3.D — `findExistingSubmission` helper.** Small, localised, removes
+   3-place duplication. Independent of §3.A. ~1 commit.
+4. **§3.F — testability gaps that don't depend on §3.A.** Specifically:
+   `generateSlug`, `stampVersion`, `publicResults` strip-text, honeypot.
+   ~1 commit per topic. Independent of all the above.
+
+### Tier 3 — more invasive, less urgent
+
+5. **§3.B — `<ChatPanel>` + `<FormEditor>` extraction.** Medium-cost
+   component refactor. Real win for the 865/693-line page files but the
+   two-call-sites-each ratio means the leverage is moderate. Independent
+   of §3.A. **Note:** the post-§3.A registry approach changes how
+   `<FormEditor>` is built (because the visual/JSON branches inside it
+   now consume `registry`), so wait until after §3.A to do §3.B.
+6. **§3.E — `feedback_instances` repository.** Medium-cost rewire. Wins
+   are: hides the JSONB-decoding quirk in one place, sets up the
+   testability seam for admin endpoints. Currently has *one* concrete
+   implementation, so per LANGUAGE.md this is a hypothetical seam —
+   don't go full ports-and-adapters, just collapse inline queries into
+   typed repo methods. Independent of §3.A.
+
+### Anti-scope (NOT proposed)
+
+- **No new dependencies.** Same constraint as the redesign work.
+- **No CSS split** (§2.I). The single feedback.css reads fine today.
+- **No real-time / WebSocket migration.** Polling is the documented
+  decision; defer until it actually hurts.
+- **No auth tier on participant pages.** Slug-as-token is the documented
+  decision.
+- **No proper i18n shipped here.** Just *prepare* for it via §3.A's
+  per-type modules. m/fdbck#3 is a separate ticket.
+- **No port/adapter pattern for fdb.** One adapter = hypothetical seam,
+  per LANGUAGE.md. §3.E is a "collapse inline queries," not a "design
+  for a swap-out we won't do."
+
+---
+
+## 5. Open questions for m before implementing anything
+
+1. **Tier 1 vs. Tier 2 first?** §3.A is the highest leverage but biggest
+   commit. §3.C / §3.D / §3.F are all tiny and independent. Picking up
+   the tier-2 wins first is honest progress with low risk; tier-1 is the
+   real architectural payoff. **My recommendation: §3.C + §3.D + §3.F's
+   easy tests in one shift to clear quick debt; then §3.A in a focused
+   shift afterwards.**
+2. **Component test runner.** We have `bun test ./*.test.ts` for server-
+   side units but no Svelte component test runner configured. After
+   §3.A, the per-type Svelte parts (`BuilderEditor`, `ParticipantInput`,
+   `ResultsBlock`) want component-level tests. Do we add one (e.g.
+   `@testing-library/svelte` + jsdom)? **Recommend: only if §3.A lands;
+   otherwise skip.**
+3. **Naming.** `lib/questions/scale.ts` vs. `lib/question-types/scale.ts`
+   vs. `lib/q/scale.ts`. I'd default to `lib/questions/`. Confirm.
+4. **Registry shape.** A flat `Record<type, QuestionTypeModule>`
+   centralised in `lib/questions/registry.ts`, vs. each module
+   self-registering on import. Prefer the explicit registry — it's the
+   one place that lists every type, ordering matters for the
+   "+ Add question" buttons in `FormBuilder`.
+5. **`setScaleLabel` rename + the `(s as ScaleStats & { _sum?: number })`
+   cleanup** — these are tiny papercut fixes (§2.A nav-friction and
+   §2.H ugly-cast). Do them as a "code-quality" sub-commit at the start
+   of §3.A's branch, or as a separate trivial commit? Either is fine.
+   **Recommend: bundle into the §3.A branch as the very first commit so
+   the diff is calmer afterwards.**
+
+---
+
+## 6. What's not on this list and why
+
+- **`results.ts` aggregation logic.** Already deep, well-tested,
+  well-named. The `(s as X & { _sum })` pattern is ugly but isolated.
+  Don't refactor without §3.A pulling it apart.
+- **`public-scope.ts` policy gate.** Genuinely deep. Don't touch.
+- **`auth.ts` + `request-context.ts`.** Small, tested via
+  `public-scope.test.ts` indirectly. Don't touch.
+- **`shlink.ts` / share endpoint.** External-API adapter, single caller,
+  small surface. Don't touch.
+- **`Icon.svelte`.** A 14-icon switch statement; trivially extendable.
+  Fine.
+- **`feedback.css`.** Long but readable. §2.I.
+
+---
+
+## TL;DR (for the head's nudge)
+
+The single biggest deepening opportunity is **§3.A — bundle each question
+type into its own module** (schema + UI parts + server logic). It closes a
+real validation gap, sets up i18n cleanly, makes adding a type a
+one-file-plus-one-line change instead of a 12-place strip, and unlocks
+clean per-type unit tests. Everything else on this list is either a small
+independent win (`withOwnedInstance`, `findExistingSubmission`, the slug /
+stampVersion / publicResults tests) or downstream of §3.A (§3.B `<ChatPanel>`
+extraction, §3.F per-type tests).