refactor: tags, presets and custom themes read through plain getters (U5) (@miodec)

docs/specs/solid-monkeytype/roadmap.md

@@ -112,8 +112,8 @@ Serial; each step needs the previous one. Verify each in the browser (`__mt.chec
 | [ ] | U2 | M · PR | Activity calendar (user store already holds plain activity data; the per-year cache already sits in `ActivityCalendar.tsx`): the calendar becomes a Solid component; delete the imperative test-activity modules | U1 (data shape) | Can share a PR with U1 |
 | [ ] | U3 | M · PR | Remove the `userLoaded` auth event (was snapshot-updated) (PSA, last-signed-out-result modal read `getUser()`); PSA's `authEvent` subscription (`Psa.init`, started by boot) follows the auth signal; auth becomes a signal the router reads, with the user-data load as state (loading/loaded/failed) plus `reloadUserData()` (reruns `loadUserData()`) replacing the event's promise; delete the auth bus | U1 | |
 | [x] | U4 | S · commit per area | Readers move from the `DB.getSnapshot()` shim to `getUser()`, by area; the last PR deletes the shim and the snapshot code in `db.ts` | U1 | Done in 0012c6f0f: shim deleted, readers import `getUser()`; also renamed `Snapshot`→`UserData`, `SnapshotResult`→`NormalizedLocalResult` (now in `collections/results.ts`), `SnapshotInitError`→`UserDataLoadError`, `snapshotUpdated`→`userLoaded`, `window.snapshot`→`window.userData` |
-| [ ] | U5 | M · PR | Tags, presets and custom themes get the `getUser()` pattern: plain getters backed by a module-level reactive mirror, replacing their live-query hooks (keep `gcTime: Infinity`). Rewrite the non-test `__nonReactive` callers as Solid code reading the getters: commandline tag/preset lists, the custom-themes check in config metadata, the tag PB helpers in `collections/tags.ts` (`getLocalTagPB`, `saveLocalTagPB`) | none | Test-page callers go with T6a/T6c |
-| [ ] | U6 | S · commit | Delete `__nonReactive` from every module once no caller is left | U5, T6a, T6c | Pure deletion: a caller still left means a step missed it |
+| [x] | U5 | M · PR | Tags, presets and custom themes get the `getUser()` pattern: plain getters backed by a module-level reactive mirror, replacing their live-query hooks (keep `gcTime: Infinity`). Rewrite the non-test `__nonReactive` callers as Solid code reading the getters: commandline tag/preset lists, the custom-themes check in config metadata, the tag PB helpers in `collections/tags.ts` (`getLocalTagPB`, `saveLocalTagPB`) | none | Done in [#8564](https://github.com/monkeytypegame/monkeytype/pull/8564): a module-level live query per collection behind plain getters, plus a session download status (`getTagsStatus()` etc.: loading/loaded/failed) and `reloadCustomThemes()`. Tags keep `__nonReactive` for `test-logic` and `result-pb` |
+| [ ] | U6 | S · commit | Delete `__nonReactive` from every module once no caller is left | U5, T6a, T6c | Pure deletion: a caller still left means a step missed it. Since U5 only the tags module (test-page callers) and `states/test`/results keep one |
 
 ## Track EP: engine prep (small, parallel)
 

docs/specs/solid-monkeytype/spec.md

@@ -143,6 +143,9 @@ Users should see no behaviour change, except where a bug at today's seams disapp
 - **Auth**: auth state is a signal (unknown, signed in, signed out), which the router's auth context and UI read. The user-data load sits next to it as state (loading, loaded, failed with an error) plus a `reloadUserData()` command; the router's auth context reads it for the blocking user-data step (user, tags, presets) and its retry, instead of receiving a promise through an event. The auth bus (both its auth-state-changed and snapshot-updated events) is deleted.
 - **Server data**: Query for read-only server data, DB collections for user-owned lists (results, tags, presets, connections, ape keys, inbox, custom themes, filter presets). The user is a store (above), not a collection.
   - Small collections (tags, presets, custom themes) each export plain getters (eg. `getTags()`, `getActiveTags()`) backed by a module-level reactive mirror of the collection, reactive in components and synchronous elsewhere, the same read shape as `getUser()`. They replace both `__nonReactive` and the separate live-query hooks for these collections, are documented as "returns only what's loaded, never fetches", and keep `gcTime: Infinity`.
+    - The mirror is a module-level live query (`useLiveQuery` in a root that lives as long as the page). Getters read its `.state` (single-row getters like `getTag(id)` track only their row) and never call the query itself, which suspends: the getters never suspend.
+    - Since nothing suspends, each collection exports its session download status (`getTagsStatus()`, `getPresetsStatus()`, `getCustomThemesStatus()`: loading, loaded or failed, like the user-data load), from `createSessionDownload`; UI that needs a loading state switches on that status, not `Suspense`.
+    - A collection's rows outlive sign out (the user query reset doesn't clear them), so the mirror, the status and the blocking `waitFor…Ready` calls wait for the download since the current sign in, which the module starts on sign in. A failed download settles the wait (sign in doesn't hang) and sets the status to failed; UI with a loading state shows the failure with a retry (`reloadCustomThemes()`, like `reloadUserData()`).
   - `__nonReactive` exists only for old-style code. It is not replaced one-for-one: each caller is rewritten as Solid code (or moved into the engine) as part of its own step, and `__nonReactive` is deleted once the last caller is gone. Reading a getter outside a tracking scope, as the engine does at finish, is ordinary Solid usage, not an escape hatch. Sign-in already waits for tags and presets inside the blocking user-data step, so they're loaded before any test can start; no extra blocking data is added.
   - Large reads (all results) use the async `*Once` form and stay off hot paths.
 - **UI state**: module-level signals and stores grouped by feature in the states area, created with the existing helpers. No plain `let` state that UI reads. Engine state is not here (see below).
@@ -286,6 +289,11 @@ From commandline metadata (C5): generated config commands use the settings label
 - Renamed options: key tips, out of focus warning and caps lock warning show/hide (were on/off); error sound 4 "missed punch" (was "punch miss"); layout "default" (was "off"); random theme "favorite" (was "fav"); pace caret "tag pb" (was "tagPb").
 - Kept commandline labels: blind mode on/off (settings shows a blank "on" for button height), quote length, sound volume, font family and keymap layout (no settings label of their own to share).
 
+From user data (U5):
+
+- The settings custom themes list shows a loading circle until the custom themes load, and "Failed to load custom themes" with a retry button if the download fails (before: an empty list either way). Custom themes aren't part of the blocking sign-in step.
+- After signing out and back in, tags, presets and custom themes show nothing (and the sign-in wait for tags and presets holds) until the new session's download lands; before, the previous session's rows showed, reported as loaded.
+
 ## Settled decisions (grilling, 2026-10-24)
 
 | # | Decision |
@@ -317,7 +325,7 @@ From commandline metadata (C5): generated config commands use the settings label
 | Q25 | U1 adds the user store and converts all writers; a read-only `DB.getSnapshot()` shim stays until readers move, then is deleted |
 | Q26 | Quote ratings stay in the user as an optimistic action; #7717 closed |
 | Q27 | One `getUser()` read: reactive in components, synchronous elsewhere |
-| Q28 | Same one-getter pattern for tags, presets and custom themes (replaces their live-query hooks); results stay async. `__nonReactive` goes by rewriting its old-style callers, then deleting it |
+| Q28 | Same one-getter pattern for tags, presets and custom themes (replaces their live-query hooks); results stay async. `__nonReactive` goes by rewriting its old-style callers, then deleting it. Amended in U5: the mirror is a module-level live query, plus a session download status per collection (loading/loaded/failed) since the getters never suspend |
 | Q29 | Auth bus deleted entirely; the user-data load is state (loading/loaded/failed) plus `reloadUserData()` beside the auth signal, read by the router |
 | Q30 | The user is one Solid store in `states/user.ts`, not a collection: no sync source, the collection needed a mirror store anyway, TanStack optimistic transactions hide concurrent direct writes. Same API (`getUser()`, named writes, hand-rolled optimistic actions with field-level rollback) |
 

frontend/__tests__/collections/custom-themes.spec.ts

@@ -1,21 +1,40 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
+import { createEffect, createRoot } from "solid-js";
 
 vi.mock("../../src/ts/ape", () => ({
   default: {
     users: {
       getCustomThemes: vi.fn(async () => ({
         status: 200,
-        body: { data: [] },
+        body: {
+          data: [
+            { _id: "c2", name: "zebra", colors: [] },
+            { _id: "c1", name: "mine", colors: [] },
+          ],
+        },
       })),
+      deleteCustomTheme: vi.fn(async () => ({ status: 200, body: {} })),
     },
   },
 }));
 
 import Ape from "../../src/ts/ape";
-import "../../src/ts/collections/custom-themes";
+import {
+  getCustomThemesStatus,
+  reloadCustomThemes,
+  deleteCustomTheme,
+  getCustomTheme,
+  getCustomThemes,
+} from "../../src/ts/collections/custom-themes";
 import { queryClient } from "../../src/ts/queries/index";
 import { setUserId } from "../../src/ts/states/core";
 
+const isLoaded = (): boolean => getCustomThemesStatus() === "loaded";
+const hasFailed = (): boolean => getCustomThemesStatus() === "failed";
+
+const names = (themes: { name: string }[]): string[] =>
+  themes.map((it) => it.name);
+
 /** Lets a queued effect update run (a set signal flushes on the next turn). */
 async function flush(): Promise<void> {
   await new Promise((resolve) => setTimeout(resolve, 0));
@@ -65,4 +84,93 @@ describe("customThemes collection", () => {
     await waitFor(hasDownloaded, "the customThemes query to fetch");
     expect(Ape.users.getCustomThemes).toHaveBeenCalled();
   });
+
+  it("is empty while signed out", () => {
+    expect(getCustomThemes()).toEqual([]);
+  });
+
+  it("returns the loaded themes sorted by name", async () => {
+    setUserId("uid");
+    await waitFor(() => getCustomThemes().length === 2, "the themes to load");
+    expect(names(getCustomThemes())).toEqual(["mine", "zebra"]);
+    expect(getCustomTheme("c2")?.name).toEqual("zebra");
+  });
+
+  it("reads a delete synchronously and re-runs effects", async () => {
+    setUserId("uid");
+    await waitFor(() => getCustomThemes().length === 2, "the themes to load");
+    const seen: string[][] = [];
+    const dispose = createRoot((dispose) => {
+      createEffect(() => seen.push(names(getCustomThemes())));
+      return dispose;
+    });
+
+    const deleted = deleteCustomTheme({ themeId: "c2" });
+
+    expect(getCustomTheme("c2")).toBeUndefined();
+    expect(seen.at(-1)).toEqual(["mine"]);
+    await deleted;
+    dispose();
+  });
+
+  it("empties on sign out", async () => {
+    setUserId("uid");
+    await waitFor(() => getCustomThemes().length > 0, "the themes to load");
+    setUserId(null);
+    expect(getCustomThemes()).toEqual([]);
+  });
+
+  it("reports loaded only once signed in and downloaded", async () => {
+    expect(getCustomThemesStatus()).toBe("loading");
+    setUserId("uid");
+    await waitFor(isLoaded, "the themes to load");
+    setUserId(null);
+    expect(getCustomThemesStatus()).toBe("loading");
+  });
+
+  it("drops the previous user's themes until the new ones load", async () => {
+    setUserId("uid");
+    await waitFor(isLoaded, "the themes to load");
+    setUserId(null);
+
+    let release = (): void => undefined;
+    vi.mocked(Ape.users.getCustomThemes).mockImplementationOnce(
+      async () =>
+        new Promise((resolve) => {
+          release = () =>
+            resolve({
+              status: 200,
+              body: { data: [{ _id: "c3", name: "other", colors: [] }] },
+            } as never);
+        }),
+    );
+    setUserId("uid2");
+    await flush();
+
+    expect(getCustomThemes()).toEqual([]);
+    expect(getCustomThemesStatus()).toBe("loading");
+
+    release();
+    await waitFor(isLoaded, "the new themes to load");
+    expect(names(getCustomThemes())).toEqual(["other"]);
+  });
+
+  it("reports a failed download, and loads on reload", async () => {
+    // no retries: the failure is final at once
+    queryClient.setQueryDefaults(["user", "customThemes"], { retry: false });
+    vi.mocked(Ape.users.getCustomThemes).mockRejectedValueOnce(
+      new Error("offline"),
+    );
+    setUserId("uid4");
+    await waitFor(hasFailed, "the download to fail");
+
+    expect(getCustomThemesStatus()).toBe("failed");
+
+    reloadCustomThemes();
+    expect(getCustomThemesStatus()).toBe("loading");
+    await waitFor(isLoaded, "the themes to load on reload");
+    expect(getCustomThemesStatus()).not.toBe("failed");
+    expect(getCustomThemes()).toHaveLength(2);
+    queryClient.setQueryDefaults(["user", "customThemes"], {});
+  });
 });

frontend/__tests__/collections/presets.spec.ts

@@ -1,48 +1,100 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { createEffect, createRoot } from "solid-js";
 
 vi.mock("../../src/ts/ape", () => ({
   default: {
     presets: {
       get: vi.fn(async () => ({
         status: 200,
-        body: { data: [{ _id: "p1", name: "my_preset", config: {} }] },
+        body: {
+          data: [
+            { _id: "p2", name: "zebra", config: {} },
+            { _id: "p1", name: "my_preset", config: {} },
+          ],
+        },
       })),
+      delete: vi.fn(async () => ({ status: 200, body: {} })),
     },
   },
 }));
 
 import {
-  __nonReactive,
+  getPresetsStatus,
+  deletePreset,
+  getPreset,
+  getPresets,
   waitForPresetsReady,
 } from "../../src/ts/collections/presets";
 import { setUserId } from "../../src/ts/states/core";
 
-/**
- * The commandline reads presets through `__nonReactive`, which doesn't
- * subscribe. Loaded only by `waitForPresetsReady` in the login flow, the
- * collection has no subscriber outside the settings page, so it must not be
- * garbage collected (TanStack DB's default collection gcTime is 5 min).
- */
+const names = (presets: { name: string }[]): string[] =>
+  presets.map((it) => it.name);
+
 describe("presets collection", () => {
   beforeEach(() => {
     vi.useFakeTimers({ shouldAdvanceTime: true });
   });
 
   afterEach(() => {
     vi.useRealTimers();
+    setUserId(null);
+  });
+
+  it("is empty while signed out", () => {
+    expect(getPresets()).toEqual([]);
   });
 
-  it("keeps its data with no subscriber", async () => {
+  it("returns the loaded presets sorted by name", async () => {
+    setUserId("uid");
+    await waitForPresetsReady();
+    expect(names(getPresets())).toEqual(["my preset", "zebra"]);
+    expect(getPreset("p2")?.name).toEqual("zebra");
+  });
+
+  /**
+   * The commandline reads presets outside a tracking scope. Loaded only by
+   * `waitForPresetsReady` in the login flow, the collection must not be
+   * garbage collected (TanStack DB's default collection gcTime is 5 min).
+   */
+  it("keeps its data over time", async () => {
     setUserId("uid");
     await waitForPresetsReady();
-    expect(__nonReactive.getPresets().map((it) => it.name)).toEqual([
-      "my preset",
-    ]);
 
     await vi.advanceTimersByTimeAsync(10 * 60_000);
 
-    expect(__nonReactive.getPresets().map((it) => it.name)).toEqual([
-      "my preset",
-    ]);
+    expect(names(getPresets())).toEqual(["my preset", "zebra"]);
+  });
+
+  it("reads a delete synchronously and re-runs effects", async () => {
+    setUserId("uid");
+    await waitForPresetsReady();
+    const seen: string[][] = [];
+    const dispose = createRoot((dispose) => {
+      createEffect(() => seen.push(names(getPresets())));
+      return dispose;
+    });
+
+    const deleted = deletePreset({ presetId: "p2" });
+
+    expect(getPreset("p2")).toBeUndefined();
+    expect(seen.at(-1)).toEqual(["my preset"]);
+    await deleted;
+    dispose();
+  });
+
+  it("empties on sign out", async () => {
+    setUserId("uid");
+    await waitForPresetsReady();
+    setUserId(null);
+    expect(getPresets()).toEqual([]);
+  });
+
+  it("reports loaded only once signed in and downloaded", async () => {
+    expect(getPresetsStatus()).toBe("loading");
+    setUserId("uid");
+    await waitForPresetsReady();
+    expect(getPresetsStatus()).toBe("loaded");
+    setUserId(null);
+    expect(getPresetsStatus()).toBe("loading");
   });
 });