diff --git a/.claude/skills/alignment/SKILL.md b/.claude/skills/alignment/SKILL.md index 9becef6..bf9a4c8 100644 --- a/.claude/skills/alignment/SKILL.md +++ b/.claude/skills/alignment/SKILL.md @@ -129,7 +129,12 @@ Review all changes in scope. If changes span multiple patterns below, apply all 12. **Persistence safety**: records that may need migration carry a `version` field; reads apply migrations; destructive actions (delete, revert, reset) confirm; storage failures - warn rather than silently lose data. + warn rather than silently lose data. All IndexedDB writes go through `db.put` — a + hand-rolled `tx(…, 'readwrite')` or raw `objectStore.put`/`delete` outside `db.ts` bypasses + the quota normalization that mints `StorageQuotaError`, silently losing the fail-loud signal + (arch 02). A new per-record entity tier's write-through subscriber calls the shared + `wireEntityWriteThrough` helper (`orchestration/entity-persistence.ts`) instead of + re-implementing the prev/next diff loop. 13. **Documentation hygiene** (the docs are maintained artifacts; keep them at altitude): - **Self-containment**: documentation and comments must not add pointers that require an external repository to follow. Knowledge gets captured locally (`docs/spec/`, diff --git a/docs/IMPLEMENTATION-PLAN.md b/docs/IMPLEMENTATION-PLAN.md index 0edb51c..973017c 100644 --- a/docs/IMPLEMENTATION-PLAN.md +++ b/docs/IMPLEMENTATION-PLAN.md @@ -51,7 +51,7 @@ doc before implementing. | **M3** | Datasets ✅ | Named reusable data + reference resolution in preview | §05, §03F, §09B | | **M4** | Chart Builder ✅ | No-JSON chart composition from a dataset | §06 | | **M5** | Settings + Import/Export ✅ | Preferences + workspace backup/transfer | §07, §08, §09C | -| **M6** | Shell polish | Resize/toggle panes, routing, shortcuts, toasts, a11y, offline | §01, §10 | +| **M6** | Shell polish ✅ | Resize/toggle panes, routing, shortcuts, toasts, a11y, offline | §01, §10 | **MVP boundary = end of M1** (a genuinely usable single-user chart authoring loop). M1.5 makes it _look right_; M2 makes it _robust_; M3–M6 make it _complete_. @@ -428,10 +428,15 @@ through `services/transfer.ts` (→ `normalizeImport` / envelope build), no moda --- -## M6 · Shell polish & non-functional +## M6 · Shell polish & non-functional ✅ (done) **Goal:** the workspace feels finished and meets §10. +**Scope note — desktop/tablet, not phone.** Astrolabe is a desktop and (at best) +tablet tool; phones are out of scope. So the touch/installable surface we target is +**iPad add-to-home-screen**, not iPhone — which is why the one PNG icon is a 180×180 +`apple-touch-icon` and we don't chase phone-specific viewport/layout work. + - **Panes:** ~~drag-resize handles with min widths; widths persist~~ ✅ (pulled forward after M2). ~~Per-pane show/hide **toggle strip** + visibility persist + proportional redistribution on hide (§01A, §09D)~~ ✅. @@ -454,13 +459,13 @@ through `services/transfer.ts` (→ `normalizeImport` / envelope build), no moda IDB-transaction atomicity would require exposing a raw transaction from `db.ts` — see [arch 02](architecture/02-persistence.md)). - **A11y:** modal focus trap + return, labelled icon buttons, contrast in both themes (§10) — ✅ in place. -- **About** and **Donate** modals ~~(§01)~~ ✅ (Donate URL is a placeholder - `DONATE_URL` pending the real link). -- **Offline/installable:** the manifest now ships a full SVG icon set (favicon / maskable / - monochrome) + `theme_color`, and the SW precaches the shell — the app is **installable**. - **⏳ Remaining** — manual verification in a running/installed app (checklist: - [manual-verification.md](manual-verification.md)); iOS home-screen still wants a PNG - `apple-touch-icon` (logged residual). +- **About** and **Donate** modals ~~(§01)~~ ✅. The project solicits nothing for + itself — **Donate** redirects to Ukraine's defense (`savelife.in.ua`, the Come Back + Alive foundation), where the author forwards any project donations anyway. +- **Offline/installable:** the manifest ships a full SVG icon set (favicon / maskable / + monochrome) + a 180×180 `apple-touch-icon.png` (iPad add-to-home-screen) + `theme_color`, + and the SW precaches the shell — the app is **installable**. ✅ Manual verification in a + running/installed app passed (checklist: [manual-verification.md](manual-verification.md)). - **Council** — ~~seat **web.dev** for the PWA/offline/storage surfaces none of the seated members cover: the service-worker **update-available** prompt (`registerType: 'prompt'`), storage **persistence** (`navigator.storage.persist()`), and the quota **estimate** @@ -470,9 +475,10 @@ through `services/transfer.ts` (→ `normalizeImport` / envelope build), no moda [`/council`](../.claude/skills/council/SKILL.md) and [arch 10](architecture/10-interaction-and-feedback.md). -**Manual checks:** keyboard-only run-through; reload restores view from URL; -offline reload works; install as standalone; reduced-motion honored. **⏳ Still owed** -(plus visual verification of the new library/monitor/modals/busy-indicator surfaces). +**Manual checks:** ✅ keyboard-only run-through; reload restores view from URL; +offline reload works; install as standalone; reduced-motion honored — all verified in a +running/installed app, plus visual verification of the library/monitor/modals/busy-indicator +surfaces. --- @@ -487,7 +493,9 @@ offline reload works; install as standalone; reduced-motion honored. **⏳ Still boundary. M1–M6 can ship English-only with date formatting locale-aware (§10). Don't retrofit later if avoidable — keep user-facing strings centralized from M1. - **Versioning:** simplified semver `0.x.y`, `package.json` → `__APP_VERSION__` - (already wired). Bump per shippable milestone. + (already wired). **Not yet released publicly** — the working version stays pre-1.0 through + M0–M6; the **first public release will be `1.0.0`**, cut on the maintainer's signal (don't + auto-bump in the meantime). - **Docs trio:** keep `SOUL.md` / `AGENTS.md` / `CLAUDE.md` current as the app grows. --- @@ -528,20 +536,16 @@ is in the maintained plan, not the archive. **Chart Builder** (`exploration/chart-builder-enhancement-scope.md`): -- **Starter examples + open-in-builder (3B/3C)** — the next builder slice in the scope doc: - a starter-example front door that hydrates the builder from an existing spec (strict). +- **Open-in-builder + builder starter examples (3B/3C)** — the next builder slice in the + scope doc: hydrate the **builder** from an existing spec (strict spec→config parse), behind + a starter-example front door. Distinct from the shipped onboarding gallery + (`core/examples.ts` → `Onboarding.tsx`), which seeds a snippet straight into the editor, not + the builder — there is no spec→builder-config path yet. - **Transform-aware data inspector** — show resolved post-transform rows (`exploration/data-inspector-exploration.md`). - **Field-chip drag-and-drop** — click/keyboard-first shipped; drag deferred. - **Calculated-field autocomplete popup** — Monaco-style completion for expressions. -**Engineering follow-ups** (refactor candidates, not features): - -- **Entity-persistence subscriber consolidation** — snippet/dataset/theme/font each carry a - near-identical diff-prev/next write-through subscriber (`orchestration/*-persistence.ts`); - a generic `wireEntityPersistence(store, { save, delete })` would replace the four. Earned - by the four call sites; hold only if per-entity clarity is judged to outweigh the dedupe. - --- ## Architecture reference diff --git a/docs/architecture/02-persistence.md b/docs/architecture/02-persistence.md index 39400b9..3e0f5ed 100644 --- a/docs/architecture/02-persistence.md +++ b/docs/architecture/02-persistence.md @@ -426,26 +426,25 @@ export async function readOriginUsage(): Promise { ### Fail loudly, never silently lose data -When a write would exceed quota, IndexedDB rejects with a `QuotaExceededError`. The adapter must **propagate** this so the UI can tell the user to export and prune — it must never swallow the error and pretend the save succeeded. +When a write would exceed quota, IndexedDB rejects with a `QuotaExceededError`. Quota is +whole-origin, so it's normalized **once** at the single write path — `db.put` — into a typed +`StorageQuotaError`. Every typed adapter inherits fail-loud behavior without repeating the +check, and consumers branch on the type instead of sniffing a `DOMException`. ```ts -export async function saveSnippet(s: Snippet): Promise { - try { - await put('snippets', { ...s, version: CURRENT_SNIPPET_VERSION }); - } catch (err) { +// src/app/infrastructure/db.ts — the one write path +export const put = (store: string, value: T): Promise => + tx(store, 'readwrite', (s) => s.put(value)).catch((err: unknown) => { if (err instanceof DOMException && err.name === 'QuotaExceededError') { - // Surface to the user via the store; do NOT silently drop the write. - throw new StorageQuotaError( - 'Snippet storage is full. Export and remove snippets to free space.', - ); + throw new StorageQuotaError(); // never silently drop the write } throw err; - } -} + }); ``` > **Do:** surface quota warnings _before_ the budget is hit (the 80% threshold) and hard errors loudly when a write fails. -> **Don't:** wrap a save in a bare `try/catch {}` that logs and returns — that turns "your work wasn't saved" into a silent data-loss bug. The only thing the adapter may safely swallow is a _read_ failure, where falling back to defaults/empty is the correct behavior. +> **Don't:** wrap a save in a bare `try/catch {}` that logs and returns — that turns "your work wasn't saved" into a silent data-loss bug. The only thing that may safely swallow an error is a _read_ failure, where falling back to defaults/empty is the correct behavior. +> **Rule:** every write goes through `db.put`. An adapter that opens its own `tx(store, 'readwrite', …)` instead bypasses quota normalization and silently loses the typed `StorageQuotaError` — a regression the type system won't catch. **The adapter propagating is only half — a consumer must catch and surface it.** A fire-and-forget `void saveSnippet(n)` re-buries the very error the adapter took care to @@ -456,10 +455,12 @@ throw. Persistence write-backs are wired as store subscribers, so the surfacing `notify()` (`stores/NotificationStore`) → `Toaster`. Rules this encodes (spec §10 "told when a save fails"): never `void`-fire a persist without -a `.catch` that calls `notify(storageErrorNotification(op, err))`; the mapper splits -user-fixable (storage full → next step, no diagnostic) from not (blocked storage → plain -explanation **+** a reportable `detail`); and a blocked store at startup **warns and runs -in memory** rather than rejecting into the void. +a `.catch` that maps the error to a toast — `storageErrorNotification(op, err)` for snippets +(bespoke "your library" / "your changes" copy), `entityStorageErrorNotification(noun, op, err)` +for the other tiers (the same shape with the entity's own noun). The mapper splits user-fixable +(storage full → next step, no diagnostic) from not (blocked storage → plain explanation **+** a +reportable `detail`); and a blocked store at startup **warns and runs in memory** rather than +rejecting into the void. ### Multi-record writes (import): atomicity at the service boundary @@ -492,9 +493,9 @@ spec §08 "no partial import is committed" contract holds and the user gets an a 3. Add the object store in `openDB`'s `onupgradeneeded`, guarded by `contains(...)`; bump `DB_VERSION` only if you changed store _layout_. 4. Add a `migrate()` function and call it on every read. 5. Expose typed `load*/save*/ensure*` functions from one infrastructure module — and from _only_ there. -6. Add the app layer: a Zustand store whose low-level `add`/`update`/`remove` are the single mutation point for the collection, and a diffing **write-through subscriber** in `orchestration/` (the `dataset-persistence.ts` shape: compare the array against the previous snapshot, upsert changed records, delete missing ones, toast on failure). +6. Add the app layer: a Zustand store whose low-level `add`/`update`/`remove` are the single mutation point for the collection, and a **write-through subscriber** in `orchestration/` — a thin wrapper over the shared `wireEntityWriteThrough(store, select, { save, remove, onError })` helper (`entity-persistence.ts`), which diffs the array against the previous snapshot, upserts changed records, deletes missing ones, and toasts on failure. 7. Hydrate in `orchestration/startup.ts` and wire the subscriber **after** hydrate — wiring first would re-save every loaded record on each startup. -8. If the tier has a budget, hook it into the storage monitor and propagate `QuotaExceededError`. +8. Quota propagation is automatic (`db.put` throws `StorageQuotaError`) — just pass an `onError` that maps it via `entityStorageErrorNotification(noun, …)`. If the tier has a budget, also hook it into the storage monitor. 9. Test the adapter against `fake-indexeddb` / a localStorage stub; test the migration with fixtures from each historical version. -The stack for one entity is four files with fixed roles: `infrastructure/-store.ts` (typed IDB adapter) + `infrastructure/-migrations.ts` (read-time upgrade) + `stores/Store.ts` (in-memory collection + feature state) + `orchestration/-persistence.ts` (write-through), joined in `startup.ts`. Snippets, datasets, and custom themes each follow it. +The stack for one entity is four files with fixed roles: `infrastructure/-store.ts` (typed IDB adapter) + `infrastructure/-migrations.ts` (read-time upgrade) + `stores/Store.ts` (in-memory collection + feature state) + `orchestration/-persistence.ts` (write-through, a thin call to the shared `wireEntityWriteThrough`), joined in `startup.ts`. Snippets, datasets, custom themes, and user fonts each follow it. diff --git a/docs/ux-second-pass.md b/docs/ux-second-pass.md index c7a68ef..dca1515 100644 --- a/docs/ux-second-pass.md +++ b/docs/ux-second-pass.md @@ -8,9 +8,14 @@ record the resolution into the contract (`docs/architecture/09`+`10` and the rel ## Open -_(empty — the 2026-06-13 batch resolved all eleven open items; resolutions live in -spec §01B/§02/§03D, arch 09 §5 (header worked example), and arch 10 §5 (toggle-strip -pressed cue, SelectControl action row + trigger cap, no-affordance-for-unbuilt-features).)_ +- **Storage-full copy implies a per-tier budget, but quota is whole-origin** — the messages + say "snippet storage is full" / "dataset storage is full" and tell the user to delete that + entity's items, yet IndexedDB quota is shared across the whole origin. Per-tier framing is + more _actionable_ (deleting the tier you're saving into does free space) but misstates the + scope. Decide: keep the actionable per-tier framing, or switch to a whole-origin "Storage is + full — free space (the Storage monitor shows what's using it)". Affects both + `storageErrorNotification` and `entityStorageErrorNotification` in `services/storage-errors.ts` + and the import-quota copy in `services/transfer.ts`. ## Deferred (not design debts, revisit on demand) diff --git a/src/app/infrastructure/db.ts b/src/app/infrastructure/db.ts index 1a06d5b..339a0f4 100644 --- a/src/app/infrastructure/db.ts +++ b/src/app/infrastructure/db.ts @@ -21,6 +21,19 @@ export const DATASETS_STORE = 'datasets'; export const THEMES_STORE = 'themes'; export const FONTS_STORE = 'fonts'; +/** + * Raised when a write fails because the origin's storage budget is exhausted. + * Normalized from the raw `QuotaExceededError` DOMException in `put` (the one + * write path), so every typed adapter fails loud on quota uniformly and the UI + * can map it to an actionable "storage full" message rather than losing work. + */ +export class StorageQuotaError extends Error { + constructor(message = 'Storage is full. Export and remove items to free space.') { + super(message); + this.name = 'StorageQuotaError'; + } +} + /** Every object store the app expects — the open-time verification checklist. */ const EXPECTED_STORES = [SNIPPETS_STORE, DATASETS_STORE, THEMES_STORE, FONTS_STORE] as const; @@ -105,7 +118,14 @@ export const getAll = (store: string): Promise => tx(store, 'readonly', (s) => s.getAll() as IDBRequest); export const put = (store: string, value: T): Promise => - tx(store, 'readwrite', (s) => s.put(value)); + tx(store, 'readwrite', (s) => s.put(value)).catch((err: unknown) => { + // Quota is whole-origin and surfaces as a QuotaExceededError DOMException; + // normalize it here so every adapter throws a typed StorageQuotaError. + if (err instanceof DOMException && err.name === 'QuotaExceededError') { + throw new StorageQuotaError(); + } + throw err; + }); export const del = (store: string, key: IDBValidKey): Promise => tx(store, 'readwrite', (s) => s.delete(key)); diff --git a/src/app/infrastructure/snippet-store.ts b/src/app/infrastructure/snippet-store.ts index 38fab71..3fb63ca 100644 --- a/src/app/infrastructure/snippet-store.ts +++ b/src/app/infrastructure/snippet-store.ts @@ -2,22 +2,15 @@ * Snippet persistence adapter (docs/architecture/02). * * The typed seam between the snippet store and IndexedDB. Exposes plain async - * functions returning domain `Snippet` objects; migrates every record on read; - * fails loudly on quota so the UI can warn rather than silently lose work. + * functions returning domain `Snippet` objects; migrates every record on read. + * Quota failures fail loud as a `StorageQuotaError` — normalized once in `db.put` + * (see db.ts) — so the UI can warn rather than silently lose work. */ import { CURRENT_SNIPPET_VERSION, type Snippet } from '@core/snippet'; import { del, getAll, put, SNIPPETS_STORE } from './db'; import { migrateSnippet } from './snippet-migrations'; -/** Raised when a write fails because the snippet storage budget is exhausted. */ -export class StorageQuotaError extends Error { - constructor(message = 'Snippet storage is full. Export and remove snippets to free space.') { - super(message); - this.name = 'StorageQuotaError'; - } -} - /** Load every snippet, upgrading each record to the current shape. */ export async function loadSnippets(): Promise { const records = await getAll(SNIPPETS_STORE); @@ -26,14 +19,7 @@ export async function loadSnippets(): Promise { /** Persist a snippet at the current schema version. Propagates quota failures. */ export async function saveSnippet(snippet: Snippet): Promise { - try { - await put(SNIPPETS_STORE, { ...snippet, version: CURRENT_SNIPPET_VERSION }); - } catch (err) { - if (err instanceof DOMException && err.name === 'QuotaExceededError') { - throw new StorageQuotaError(); - } - throw err; - } + await put(SNIPPETS_STORE, { ...snippet, version: CURRENT_SNIPPET_VERSION }); } /** Permanently remove a snippet by id. */ diff --git a/src/app/orchestration/dataset-persistence.ts b/src/app/orchestration/dataset-persistence.ts index fdb037d..e3c9d90 100644 --- a/src/app/orchestration/dataset-persistence.ts +++ b/src/app/orchestration/dataset-persistence.ts @@ -1,54 +1,26 @@ /** * Dataset persistence wiring (docs/architecture/01 §5, spec §09E). * - * The dataset sibling of `persistence.ts`: a startup subscriber that diffs the - * `datasets` array against the previous snapshot and writes upserts/deletes - * through to the IndexedDB adapter. The store stays browser-free; failures - * surface as a toast rather than silent loss (spec §10). There is no debounced - * auto-save here — datasets change on explicit create/edit/delete, not per - * keystroke, so every change is a structural array edit. + * A startup subscriber that write-throughs the `datasets` array to the IndexedDB + * adapter via the shared `wireEntityWriteThrough` diff loop. The store stays + * browser-free; failures surface as a toast rather than silent loss (spec §10). + * There is no debounced auto-save here — datasets change on explicit create/edit/ + * delete, not per keystroke, so every change is a structural array edit. */ import { deleteDataset, saveDataset } from '../infrastructure/dataset-store'; +import { entityStorageErrorNotification } from '../services/storage-errors'; import { notify } from '../stores/NotificationStore'; import { useDatasetStore } from '../stores/DatasetStore'; +import { wireEntityWriteThrough } from './entity-persistence'; type Unsubscribe = () => void; -function datasetError(op: 'save' | 'delete', err: unknown) { - notify({ - kind: 'error', - title: op === 'delete' ? "Couldn't delete the dataset" : "Couldn't save the dataset", - message: - 'A storage error stopped Astrolabe from completing the last dataset change, so it may not ' + - 'survive a reload. If this keeps happening, your browser may be blocking local storage.', - detail: - err instanceof Error ? `Dataset ${op} failed: ${err.name}: ${err.message}` : String(err), - }); -} - /** Persist dataset upserts and deletions whenever the array changes. */ -function wireDatasetWriteThrough(): Unsubscribe { - let prevDatasets = useDatasetStore.getState().datasets; - return useDatasetStore.subscribe((s) => { - const next = s.datasets; - if (next === prevDatasets) return; - const prev = prevDatasets; - prevDatasets = next; - - for (const old of prev) { - if (!next.some((n) => n.id === old.id)) { - deleteDataset(old.id).catch((err) => datasetError('delete', err)); - } - } - for (const n of next) { - const old = prev.find((p) => p.id === n.id); - if (old !== n) saveDataset(n).catch((err) => datasetError('save', err)); - } +export function wireDatasetPersistence(): Unsubscribe { + return wireEntityWriteThrough(useDatasetStore, (s) => s.datasets, { + save: saveDataset, + remove: deleteDataset, + onError: (op, err) => notify(entityStorageErrorNotification('dataset', op, err)), }); } - -/** Wire dataset persistence subscribers. Returns a teardown that detaches them. */ -export function wireDatasetPersistence(): Unsubscribe { - return wireDatasetWriteThrough(); -} diff --git a/src/app/orchestration/entity-persistence.test.ts b/src/app/orchestration/entity-persistence.test.ts new file mode 100644 index 0000000..2b8ebad --- /dev/null +++ b/src/app/orchestration/entity-persistence.test.ts @@ -0,0 +1,119 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { wireEntityWriteThrough } from './entity-persistence'; + +/** + * Direct contract test for the shared write-through engine. The four + * `*-persistence.ts` wrappers exercise it transitively (see + * snippet-persistence.test.ts), but this is the engine four tiers depend on, so + * its diff/save/delete/error behavior is pinned here against a minimal store — + * including the number-id path the (string-id) snippet test never hits. + */ + +interface Item { + id: number; + v: number; +} +interface State { + items: Item[]; + other?: number; +} + +/** Minimal `StoreLike` the helper accepts, plus a `setState` for the test to drive. */ +function makeStore(initial: State) { + let state = initial; + const listeners = new Set<(s: State) => void>(); + return { + getState: () => state, + setState: (next: State) => { + state = next; + listeners.forEach((l) => l(state)); + }, + subscribe: (l: (s: State) => void) => { + listeners.add(l); + return () => listeners.delete(l); + }, + }; +} + +/** Flush microtasks so a `.catch` on a rejected io promise runs. */ +const flush = () => Promise.resolve().then(() => Promise.resolve()); + +let store: ReturnType; +let save: ReturnType; +let remove: ReturnType; +let onError: ReturnType; +let teardown: () => void; + +beforeEach(() => { + store = makeStore({ items: [{ id: 1, v: 1 }] }); + save = vi.fn().mockResolvedValue(undefined); + remove = vi.fn().mockResolvedValue(undefined); + onError = vi.fn(); + teardown = wireEntityWriteThrough(store, (s) => s.items, { save, remove, onError }); +}); + +afterEach(() => teardown()); + +describe('wireEntityWriteThrough', () => { + it('saves a newly added record (number id) and leaves untouched refs alone', () => { + const kept = store.getState().items[0]; + store.setState({ items: [kept, { id: 2, v: 1 }] }); + + expect(save).toHaveBeenCalledTimes(1); + expect(save).toHaveBeenCalledWith({ id: 2, v: 1 }); + expect(remove).not.toHaveBeenCalled(); + }); + + it('saves only the record whose reference changed', () => { + store.setState({ items: [{ id: 1, v: 2 }] }); + + expect(save).toHaveBeenCalledTimes(1); + expect(save).toHaveBeenCalledWith({ id: 1, v: 2 }); + }); + + it('removes a dropped record by id without re-saving the survivors', () => { + const one = store.getState().items[0]; + const two = { id: 2, v: 1 }; + store.setState({ items: [one, two] }); + save.mockClear(); + + store.setState({ items: [two] }); // drop 1, keep 2's reference + + expect(remove).toHaveBeenCalledTimes(1); + expect(remove).toHaveBeenCalledWith(1); + expect(save).not.toHaveBeenCalled(); + }); + + it('ignores state changes that do not touch the selected array', () => { + store.setState({ ...store.getState(), other: 5 }); + + expect(save).not.toHaveBeenCalled(); + expect(remove).not.toHaveBeenCalled(); + }); + + it('routes a rejected save to onError instead of losing it', async () => { + save.mockRejectedValueOnce(new Error('boom')); + const kept = store.getState().items[0]; + store.setState({ items: [kept, { id: 2, v: 1 }] }); + await flush(); + + expect(onError).toHaveBeenCalledWith('save', expect.any(Error)); + }); + + it('routes a rejected delete to onError', async () => { + remove.mockRejectedValueOnce(new Error('boom')); + const two = { id: 2, v: 1 }; + store.setState({ items: [store.getState().items[0], two] }); + store.setState({ items: [two] }); + await flush(); + + expect(onError).toHaveBeenCalledWith('delete', expect.any(Error)); + }); + + it('detaches on teardown', () => { + teardown(); + store.setState({ items: [...store.getState().items, { id: 9, v: 1 }] }); + + expect(save).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/orchestration/entity-persistence.ts b/src/app/orchestration/entity-persistence.ts new file mode 100644 index 0000000..e2acdb9 --- /dev/null +++ b/src/app/orchestration/entity-persistence.ts @@ -0,0 +1,64 @@ +/** + * Generic entity write-through (docs/architecture/01 §5). + * + * Every per-record store (snippets, datasets, custom themes, user fonts) persists + * the same way: watch its entity array, and on each change diff it against the + * previous snapshot by id reference — save what's new or changed, delete what's + * gone. Because store actions produce fresh object references for what they touch, + * a reference diff catches exactly the changed records (spec §09E one-record-per- + * entity tiers; a whole-store serializer like Zustand's `persist` middleware would + * rewrite every record on any change, so it doesn't fit). + * + * The per-entity `wire*Persistence` modules supply the store, the array selector, + * and the IndexedDB adapter + error toast; this owns the diff loop they shared. + */ + +type Unsubscribe = () => void; + +/** Minimal store surface this needs — satisfied by any Zustand store. */ +interface StoreLike { + getState: () => S; + subscribe: (listener: (state: S) => void) => Unsubscribe; +} + +export interface EntityWriteThrough { + /** Persist a new or changed record. */ + save: (entity: T) => Promise; + /** Remove a record that's no longer in the array. Id type follows the entity's. */ + remove: (id: T['id']) => Promise; + /** + * Surface a failed write — these promises are otherwise fire-and-forget, so a + * rejection becomes a toast rather than silent data loss (spec §10 → "told when + * a save fails rather than losing data silently"). + */ + onError: (op: 'save' | 'delete', err: unknown) => void; +} + +/** + * Persist upserts and deletions whenever a store's entity array changes. Returns + * a teardown that detaches the subscriber. Wire it AFTER the store is hydrated so + * the baseline is the loaded set — otherwise the first diff re-saves everything. + */ +export function wireEntityWriteThrough( + store: StoreLike, + select: (state: S) => readonly T[], + io: EntityWriteThrough, +): Unsubscribe { + let prev = select(store.getState()); + return store.subscribe((state) => { + const next = select(state); + if (next === prev) return; + const before = prev; + prev = next; + + for (const old of before) { + if (!next.some((n) => n.id === old.id)) { + io.remove(old.id).catch((err) => io.onError('delete', err)); + } + } + for (const n of next) { + const old = before.find((p) => p.id === n.id); + if (old !== n) io.save(n).catch((err) => io.onError('save', err)); // new or changed ref + } + }); +} diff --git a/src/app/orchestration/font-persistence.ts b/src/app/orchestration/font-persistence.ts index 3baf807..1948d2f 100644 --- a/src/app/orchestration/font-persistence.ts +++ b/src/app/orchestration/font-persistence.ts @@ -1,47 +1,25 @@ /** * User font persistence wiring (docs/architecture/01 §5; scope doc §4 → fonts). * - * The font sibling of `theme-persistence.ts`: a startup subscriber that diffs the - * `fonts` array against the previous snapshot and writes upserts/deletes through - * to the IndexedDB adapter. The store stays browser-free; failures surface as a - * toast rather than silent loss. Fonts change on explicit add/delete, so there is - * no debounce. + * A startup subscriber that write-throughs the `fonts` array to the IndexedDB + * adapter via the shared `wireEntityWriteThrough` diff loop. The store stays + * browser-free; failures surface as a toast rather than silent loss. Fonts change + * on explicit add/delete, so there is no debounce. */ import { deleteFont, saveFont } from '../infrastructure/font-store'; +import { entityStorageErrorNotification } from '../services/storage-errors'; import { notify } from '../stores/NotificationStore'; import { useFontStore } from '../stores/FontStore'; +import { wireEntityWriteThrough } from './entity-persistence'; type Unsubscribe = () => void; -function fontError(op: 'save' | 'delete', err: unknown) { - notify({ - kind: 'error', - title: op === 'delete' ? "Couldn't delete the font" : "Couldn't save the font", - message: - 'A storage error stopped Astrolabe from completing the last font change, so it may not ' + - 'survive a reload. If this keeps happening, your browser may be blocking local storage.', - detail: err instanceof Error ? `Font ${op} failed: ${err.name}: ${err.message}` : String(err), - }); -} - /** Persist font upserts and deletions whenever the array changes. */ export function wireFontPersistence(): Unsubscribe { - let prevFonts = useFontStore.getState().fonts; - return useFontStore.subscribe((s) => { - const next = s.fonts; - if (next === prevFonts) return; - const prev = prevFonts; - prevFonts = next; - - for (const old of prev) { - if (!next.some((n) => n.id === old.id)) { - deleteFont(old.id).catch((err) => fontError('delete', err)); - } - } - for (const n of next) { - const old = prev.find((p) => p.id === n.id); - if (old !== n) saveFont(n).catch((err) => fontError('save', err)); - } + return wireEntityWriteThrough(useFontStore, (s) => s.fonts, { + save: saveFont, + remove: deleteFont, + onError: (op, err) => notify(entityStorageErrorNotification('font', op, err)), }); } diff --git a/src/app/orchestration/snippet-persistence.test.ts b/src/app/orchestration/snippet-persistence.test.ts index 57fbaf9..038e22d 100644 --- a/src/app/orchestration/snippet-persistence.test.ts +++ b/src/app/orchestration/snippet-persistence.test.ts @@ -9,14 +9,15 @@ import { createSnippet, type Snippet } from '@core/snippet'; * no real database. */ -// Mock the adapter, but keep the real StorageQuotaError so the error-mapping -// path (instanceof check) behaves exactly as in production. +// Mock the adapter's writes. StorageQuotaError comes from the (unmocked) db +// module, so the error-mapping path (instanceof check) behaves as in production. vi.mock('../infrastructure/snippet-store', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, saveSnippet: vi.fn(), deleteSnippet: vi.fn() }; }); -import { saveSnippet, deleteSnippet, StorageQuotaError } from '../infrastructure/snippet-store'; +import { saveSnippet, deleteSnippet } from '../infrastructure/snippet-store'; +import { StorageQuotaError } from '../infrastructure/db'; import { useNotificationStore } from '../stores/NotificationStore'; import { useSnippetStore } from '../stores/SnippetStore'; import { AUTOSAVE_DEBOUNCE_MS, wireSnippetPersistence } from './snippet-persistence'; diff --git a/src/app/orchestration/snippet-persistence.ts b/src/app/orchestration/snippet-persistence.ts index c91f79a..54f2717 100644 --- a/src/app/orchestration/snippet-persistence.ts +++ b/src/app/orchestration/snippet-persistence.ts @@ -6,17 +6,19 @@ * through `infrastructure/`. Two subscribers: * * 1. Debounced auto-save: editor keystrokes (`draftText`) settle into a - * `commitDraft()` after a pause (spec §03B). - * 2. Write-through: any change to the `snippets` array is diffed against the - * previous snapshot and persisted/deleted. Because commits and structural - * edits produce fresh object references, a reference diff catches exactly - * what changed. + * `commitDraft()` after a pause (spec §03B). Snippet-specific — the other + * entity tiers change on explicit actions, not per keystroke. + * 2. Write-through: the shared `wireEntityWriteThrough` diff loop persists any + * change to the `snippets` array. Snippets use the richer + * `storageErrorNotification` (quota + load aware) rather than the generic + * entity toast. */ import { deleteSnippet, saveSnippet } from '../infrastructure/snippet-store'; import { storageErrorNotification } from '../services/storage-errors'; import { notify } from '../stores/NotificationStore'; import { useSnippetStore } from '../stores/SnippetStore'; +import { wireEntityWriteThrough } from './entity-persistence'; /** Delay before a settled edit is committed to the stored draft (spec §03B). */ export const AUTOSAVE_DEBOUNCE_MS = 400; @@ -35,28 +37,10 @@ function wireDraftAutoSave(): Unsubscribe { /** Persist snippet upserts and deletions whenever the array changes. */ function wireWriteThrough(): Unsubscribe { - let prevSnippets = useSnippetStore.getState().snippets; - return useSnippetStore.subscribe((s) => { - const next = s.snippets; - if (next === prevSnippets) return; - const prev = prevSnippets; - prevSnippets = next; - - // A failed write must surface, not vanish — these promises are otherwise - // fire-and-forget, so a rejection becomes a toast rather than silent loss - // (spec §10 → "told when a save fails rather than losing data silently"). - for (const old of prev) { - if (!next.some((n) => n.id === old.id)) { - deleteSnippet(old.id).catch((err) => notify(storageErrorNotification('delete', err))); - } - } - for (const n of next) { - const old = prev.find((p) => p.id === n.id); - if (old !== n) { - // new, or a changed reference - saveSnippet(n).catch((err) => notify(storageErrorNotification('save', err))); - } - } + return wireEntityWriteThrough(useSnippetStore, (s) => s.snippets, { + save: saveSnippet, + remove: deleteSnippet, + onError: (op, err) => notify(storageErrorNotification(op, err)), }); } diff --git a/src/app/orchestration/startup.ts b/src/app/orchestration/startup.ts index c4f0fb2..ff476a5 100644 --- a/src/app/orchestration/startup.ts +++ b/src/app/orchestration/startup.ts @@ -17,7 +17,10 @@ import { loadDatasets } from '../infrastructure/dataset-store'; import { loadCustomThemes } from '../infrastructure/theme-store'; import { loadFonts } from '../infrastructure/font-store'; import { registerFontAssets } from '../infrastructure/font-faces'; -import { storageErrorNotification } from '../services/storage-errors'; +import { + entityStorageErrorNotification, + storageErrorNotification, +} from '../services/storage-errors'; import { notify } from '../stores/NotificationStore'; import { useSnippetStore } from '../stores/SnippetStore'; import { useDatasetStore } from '../stores/DatasetStore'; @@ -53,7 +56,7 @@ export async function initApp(): Promise { try { datasets = await loadDatasets(); } catch (err) { - notify(storageErrorNotification('load', err)); + notify(entityStorageErrorNotification('dataset', 'load', err)); } // Custom chart themes (spec §04 → Chart theme). Same failure posture: the @@ -63,7 +66,7 @@ export async function initApp(): Promise { try { themes = await loadCustomThemes(); } catch (err) { - notify(storageErrorNotification('load', err)); + notify(entityStorageErrorNotification('theme', 'load', err)); } // User-uploaded font faces (scope doc §4). Same failure posture: a font that @@ -73,7 +76,7 @@ export async function initApp(): Promise { try { fonts = await loadFonts(); } catch (err) { - notify(storageErrorNotification('load', err)); + notify(entityStorageErrorNotification('font', 'load', err)); } useSnippetStore.getState().hydrate(snippets); diff --git a/src/app/orchestration/theme-persistence.ts b/src/app/orchestration/theme-persistence.ts index 9d24f5a..04f582f 100644 --- a/src/app/orchestration/theme-persistence.ts +++ b/src/app/orchestration/theme-persistence.ts @@ -1,47 +1,25 @@ /** * Custom theme persistence wiring (docs/architecture/01 §5; scope doc §4.4). * - * The theme sibling of `dataset-persistence.ts`: a startup subscriber that - * diffs the `themes` array against the previous snapshot and writes - * upserts/deletes through to the IndexedDB adapter. The store stays + * A startup subscriber that write-throughs the `themes` array to the IndexedDB + * adapter via the shared `wireEntityWriteThrough` diff loop. The store stays * browser-free; failures surface as a toast rather than silent loss. Themes * change on explicit save/delete, so there is no debounce. */ import { deleteCustomTheme, saveCustomTheme } from '../infrastructure/theme-store'; +import { entityStorageErrorNotification } from '../services/storage-errors'; import { notify } from '../stores/NotificationStore'; import { useCustomThemeStore } from '../stores/CustomThemeStore'; +import { wireEntityWriteThrough } from './entity-persistence'; type Unsubscribe = () => void; -function themeError(op: 'save' | 'delete', err: unknown) { - notify({ - kind: 'error', - title: op === 'delete' ? "Couldn't delete the theme" : "Couldn't save the theme", - message: - 'A storage error stopped Astrolabe from completing the last theme change, so it may not ' + - 'survive a reload. If this keeps happening, your browser may be blocking local storage.', - detail: err instanceof Error ? `Theme ${op} failed: ${err.name}: ${err.message}` : String(err), - }); -} - /** Persist theme upserts and deletions whenever the array changes. */ export function wireThemePersistence(): Unsubscribe { - let prevThemes = useCustomThemeStore.getState().themes; - return useCustomThemeStore.subscribe((s) => { - const next = s.themes; - if (next === prevThemes) return; - const prev = prevThemes; - prevThemes = next; - - for (const old of prev) { - if (!next.some((n) => n.id === old.id)) { - deleteCustomTheme(old.id).catch((err) => themeError('delete', err)); - } - } - for (const n of next) { - const old = prev.find((p) => p.id === n.id); - if (old !== n) saveCustomTheme(n).catch((err) => themeError('save', err)); - } + return wireEntityWriteThrough(useCustomThemeStore, (s) => s.themes, { + save: saveCustomTheme, + remove: deleteCustomTheme, + onError: (op, err) => notify(entityStorageErrorNotification('theme', op, err)), }); } diff --git a/src/app/services/storage-errors.test.ts b/src/app/services/storage-errors.test.ts index 6a20f15..ba41781 100644 --- a/src/app/services/storage-errors.test.ts +++ b/src/app/services/storage-errors.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { StorageQuotaError } from '../infrastructure/snippet-store'; -import { storageErrorNotification } from './storage-errors'; +import { StorageQuotaError } from '../infrastructure/db'; +import { entityStorageErrorNotification, storageErrorNotification } from './storage-errors'; describe('storageErrorNotification', () => { describe('a failure the user can fix (storage full)', () => { @@ -43,3 +43,43 @@ describe('storageErrorNotification', () => { }); }); }); + +describe('entityStorageErrorNotification', () => { + it('names the entity in the title, message, and diagnostic', () => { + const save = entityStorageErrorNotification('dataset', 'save', new Error('boom')); + expect(save.kind).toBe('error'); + expect(save.title).toBe("Couldn't save the dataset"); + expect(save.message).toContain('dataset change'); + expect(save.detail).toBe('Dataset save failed: Error: boom'); // capitalized noun in trace + + const del = entityStorageErrorNotification('font', 'delete', new Error('boom')); + expect(del.title).toBe("Couldn't delete the font"); + expect(del.detail).toContain('Font delete failed'); + }); + + it('detects a quota failure for any entity tier (not just snippets)', () => { + const n = entityStorageErrorNotification('dataset', 'save', new StorageQuotaError()); + expect(n.title).toBe('Storage full'); + expect(n.message.toLowerCase()).toContain('dataset'); + expect(n.message.toLowerCase()).toContain('delete'); + expect(n.detail).toBeUndefined(); // nothing to report — the fix is in the message + }); + + it('frames a load failure with the entity noun, not "snippets"', () => { + const n = entityStorageErrorNotification( + 'dataset', + 'load', + new DOMException('blocked', 'SecurityError'), + ); + expect(n.title).toBe("Couldn't open your datasets"); + expect(n.message).toContain('datasets'); + expect(n.message).not.toContain('snippet'); + expect(n.detail).toContain('load'); + expect(n.detail).toContain('SecurityError'); + }); + + it('describes a non-Error throw without crashing', () => { + const n = entityStorageErrorNotification('theme', 'save', 'weird string'); + expect(n.detail).toBe('weird string'); + }); +}); diff --git a/src/app/services/storage-errors.ts b/src/app/services/storage-errors.ts index 3792af0..835a81c 100644 --- a/src/app/services/storage-errors.ts +++ b/src/app/services/storage-errors.ts @@ -13,7 +13,7 @@ * that failed + the underlying error) so it can be reported and traced. */ -import { StorageQuotaError } from '../infrastructure/snippet-store'; +import { StorageQuotaError } from '../infrastructure/db'; import type { NotifyOptions } from '../stores/NotificationStore'; /** Which persistence operation failed — shapes both the wording and the trace. */ @@ -73,3 +73,58 @@ export function storageErrorNotification(op: StorageOp, err: unknown): NotifyOpt detail: diagnostic(op, err), }; } + +/** + * The storage-failure notification for the per-record entity tiers other than + * snippets (datasets, themes, fonts), parameterized by entity noun. Mirrors the + * snippet-flavored `storageErrorNotification` above — quota detection, the startup + * `load` failure, and the generic save/delete failure — but with the entity's own + * noun so a dataset error never reads "snippet". Snippets keep the bespoke function + * above (its "your library" / "your changes" framing is tuned to the silent + * save-on-edit model). `noun` is the lowercase singular ("dataset"). + */ +export function entityStorageErrorNotification( + noun: string, + op: StorageOp, + err: unknown, +): NotifyOptions { + const Noun = noun.charAt(0).toUpperCase() + noun.slice(1); + const plural = `${noun}s`; + const detail = + err instanceof Error ? `${Noun} ${op} failed: ${err.name}: ${err.message}` : String(err); + + // Storage full — the one failure the user can act on. No detail: the next step + // is in the message. (Quota is whole-origin; per-tier framing is a parked UX + // nuance — see docs/ux-second-pass.md.) + if (err instanceof StorageQuotaError) { + return { + kind: 'error', + title: 'Storage full', + message: + `This ${noun} couldn't be saved because ${noun} storage is full. ` + + `Delete ${plural} you no longer need to free space, then try again.`, + }; + } + + // Couldn't read this tier on startup — almost always blocked storage. + if (op === 'load') { + return { + kind: 'error', + title: `Couldn't open your ${plural}`, + message: + `Astrolabe couldn't open local storage, so your saved ${plural} aren't available yet. ` + + 'This usually means the browser is blocking storage — for example, private-browsing mode. ' + + 'Reload to try again.', + detail, + }; + } + + return { + kind: 'error', + title: op === 'delete' ? `Couldn't delete the ${noun}` : `Couldn't save the ${noun}`, + message: + `A storage error stopped Astrolabe from completing the last ${noun} change, so it may not ` + + 'survive a reload. If this keeps happening, your browser may be blocking local storage.', + detail, + }; +} diff --git a/src/app/services/transfer.test.ts b/src/app/services/transfer.test.ts index 0980515..8338cda 100644 --- a/src/app/services/transfer.test.ts +++ b/src/app/services/transfer.test.ts @@ -24,7 +24,8 @@ import { createCustomTheme } from '@core/custom-theme'; import { createDataset } from '@core/dataset'; import { createSnippet } from '@core/snippet'; import { downloadJson, readTextFile } from '../infrastructure/file-transfer'; -import { deleteSnippet, saveSnippet, StorageQuotaError } from '../infrastructure/snippet-store'; +import { deleteSnippet, saveSnippet } from '../infrastructure/snippet-store'; +import { StorageQuotaError } from '../infrastructure/db'; import { useCustomThemeStore } from '../stores/CustomThemeStore'; import { useDatasetStore } from '../stores/DatasetStore'; import { useNotificationStore } from '../stores/NotificationStore'; diff --git a/src/app/services/transfer.ts b/src/app/services/transfer.ts index da6a110..ff6066d 100644 --- a/src/app/services/transfer.ts +++ b/src/app/services/transfer.ts @@ -29,7 +29,8 @@ import { import { snippetSizeBytes } from '@core/snippet'; import { humanizeBytes } from '@core/storage-estimate'; import { downloadJson, readTextFile } from '../infrastructure/file-transfer'; -import { deleteSnippet, saveSnippet, StorageQuotaError } from '../infrastructure/snippet-store'; +import { deleteSnippet, saveSnippet } from '../infrastructure/snippet-store'; +import { StorageQuotaError } from '../infrastructure/db'; import { notify } from '../stores/NotificationStore'; import { useCustomThemeStore } from '../stores/CustomThemeStore'; import { useDatasetStore } from '../stores/DatasetStore';