Fix documentation drift surfaced by the full-docs consistency review

2026-08-08 02:02:33 +00:00 · 2026-06-09 16:51:43 +03:00
parent 0a652cf04b
commit 418bf23cd8
6 changed files with 96 additions and 82 deletions
@@ -34,7 +34,7 @@ hook. The state object holds both **data fields** and **action functions**.
 import { create } from 'zustand';
 import type { UiTheme } from '@core/theme'; // defined in core; charts key off it too

-export type ModalName = 'datasets' | 'settings' | 'about' | 'donate' | 'chartBuilder' | 'extract';
+export type ModalName = 'datasets' | 'about' | 'donate' | 'chartBuilder' | 'extract';

 export interface AppState {
  uiTheme: UiTheme;
@@ -113,7 +113,7 @@ Services, orchestration, infrastructure, and tests use the store object directly
 no React involved. This is the property that lets our logic live outside components:

 ```ts
-openModal('settings'); // via the modal coordinator (doc 03)
+openModal('datasets'); // via the modal coordinator (doc 03)
 const theme = useAppStore.getState().uiTheme; // snapshot read
 const unsub = useAppStore.subscribe((s, prev) => {
  /* react to changes */
@@ -361,7 +361,7 @@ Subscribers read state and write to `src/app/infrastructure/` adapters (IndexedD
 ### Theme → document (the minimal example, already wired)

 ```ts
-// src/main.tsx
+// src/app/orchestration/theme.ts (wired from main.tsx at startup)
 const applyTheme = (t: string) => {
  document.documentElement.dataset.theme = t;
 };
@@ -475,7 +475,7 @@ reset: () => set({ snippets: [], activeSnippetId: null, draftSpec: '' });
 - In components, **select narrowly**; use `useShallow` for object/array selections.
  Outside components, use `getState()` / `subscribe()`.
 - Split durable domain state into feature stores (`useSnippetStore`, `useDatasetStore`,
-  `useSettingsStore`); keep `useAppStore` for thin cross-cutting UI state.
+  `useUserSettingsStore`); keep `useAppStore` for thin cross-cutting UI state.
 - Put every shared-state mutation behind a named action on the store so it's testable
  without a DOM (`getState().action()`).
 - Do persistence and external sync (IndexedDB, `localStorage`, URL hash, theme) in
@@ -142,7 +142,7 @@ There are two ways to implement the split; pick per store:
 1. **Two object stores** (`datasets` for metadata, `dataset-payloads` keyed by the same id for `data`) — strongest separation; a `getAll` on metadata never touches payload bytes.
 2. **One store, lazy field** — keep `data` in the record but set it to `null` on the bulk list load and fetch it per-id on demand.

-Astrolabe uses the **lazy-field** approach for datasets (one store, simpler), with `data === null` signalling "summary loaded, payload not yet."
+**Target, not yet built.** Datasets currently load in full via `loadDatasets()` (`infrastructure/dataset-store.ts`); the lazy-field approach below is the **intended direction** for when dataset size demands it, not a description of shipped code. The plan: keep `data` in the one store but set it to `null` on the bulk list load (`data === null` = "summary loaded, payload not yet") and fetch per-id on demand.

 ```ts
 // src/app/infrastructure/dataset-store.ts
@@ -351,44 +351,46 @@ Astrolabe has three tiers with different capacities and risk profiles:
 | Tier                 | Backing              | Holds                              | Budget & behavior                                                                                                                                                      |
 | -------------------- | -------------------- | ---------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | **Snippet store**    | IndexedDB `snippets` | All snippet records                | Practical budget ~**5 MB**. A storage monitor estimates usage and surfaces a warning as it fills. Snippets are user-authored and irreplaceable, so we fail **loudly**. |
-| **Dataset store**    | IndexedDB `datasets` | All dataset payloads               | Separate, **high-capacity**; suited to large payloads. Lazily loaded (§3).                                                                                             |
+| **Dataset store**    | IndexedDB `datasets` | All dataset payloads               | Separate, **high-capacity**; suited to large payloads. Loaded in full today; lazy loading is a §3 target.                                                              |
 | **Settings & prefs** | localStorage         | `UserSettings` + app/UI prefs (§5) | Small; effectively unbounded for this use.                                                                                                                             |

 Splitting snippets and datasets into separate stores means a few large datasets can't crowd out the snippet budget, and the snippet monitor can report a meaningful "how full is my library" number without summing dataset bytes.

 ### Estimating usage

-Use the Storage Manager API where available, with a manual byte-sum fallback for the snippet tier so the ~5 MB budget is always reportable.
+Read the origin's **Storage Manager** estimate (`navigator.storage.estimate()` → `usage`/`quota`) and derive a presentation-ready summary in the **pure core**, so the thresholds are unit-tested without a browser and the adapter only does I/O. When the API is missing or rejects, the summary is marked unusable rather than guessed.

 ```ts
-// src/app/infrastructure/storage-monitor.ts
-export interface StorageReport {
-  snippetBytes: number; // estimated bytes used by the snippet tier
-  snippetBudget: number; // 5 MB practical budget
-  ratio: number; // snippetBytes / snippetBudget, clamped to >= 0
-  warn: boolean; // ratio crossed the warning threshold
+// src/core/storage-estimate.ts — pure, unit-tested (no I/O)
+export type StorageLevel = 'ok' | 'warning' | 'critical';
+
+export interface StorageSummary {
+  usedBytes: number;
+  quotaBytes: number;
+  fraction: number; // usedBytes / quotaBytes, clamped 0..1
+  level: StorageLevel; // escalates from `fraction`
+  available: boolean; // false when usage/quota are missing/invalid
 }

-const SNIPPET_BUDGET = 5 * 1024 * 1024;
-const WARN_AT = 0.8;
+export const WARNING_THRESHOLD = 0.8; // begin warning
+export const CRITICAL_THRESHOLD = 0.95; // escalate to "nearly full"

-export async function reportSnippetUsage(snippets: Snippet[]): Promise<StorageReport> {
-  // Cheap, deterministic estimate: serialize the records we hold.
-  const snippetBytes = snippets.reduce((n, s) => n + new Blob([JSON.stringify(s)]).size, 0);
-  const ratio = snippetBytes / SNIPPET_BUDGET;
-  const report: StorageReport = {
-    snippetBytes,
-    snippetBudget: SNIPPET_BUDGET,
-    ratio,
-    warn: ratio >= WARN_AT,
-  };
-  if (report.warn) {
-    console.warn(`[storage] snippet tier ${(ratio * 100).toFixed(0)}% of ${SNIPPET_BUDGET} bytes`);
-  }
-  return report;
+export function summarizeStorage(input: { usage?: number; quota?: number }): StorageSummary {
+  // Clamp the fraction, derive the level, and mark `available: false` when
+  // usage/quota are absent or non-finite — we don't warn on data we lack.
+}
+
+// src/app/infrastructure/storage-estimate.ts — the only browser-touching part
+export async function readStorageEstimate(): Promise<StorageSummary> {
+  const storage = typeof navigator !== 'undefined' ? navigator.storage : undefined;
+  if (!storage || typeof storage.estimate !== 'function') return summarizeStorage({});
+  const { usage, quota } = await storage.estimate();
+  return summarizeStorage({ usage, quota });
 }
 ```

+> **Open divergence (spec §02 ↔ code).** Spec §02 frames the monitor as the **snippet** storage budget specifically ("datasets are stored separately with far greater capacity"). The shipped `readStorageEstimate` instead reports **whole-origin** `usage`/`quota` (snippets + datasets + everything the origin holds). Reconcile deliberately — either scope the estimate to the snippet store, or update spec §02 to describe the whole-origin reading.
+
 ### 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.
@@ -36,11 +36,12 @@ registry, coordinator, and shell are exhaustively type-checked.
 // src/app/modals/types.ts
 export type ModalName =
  | 'datasets' // Datasets manager (list / detail / new-dataset form)
-  | 'settings' // Appearance, editor, performance, formatting prefs
  | 'about' // About & Help (shortcuts, privacy)
  | 'donate' // Donate
  | 'chartBuilder' // Visual no-JSON chart composition for a dataset
  | 'extract'; // Extract inline spec data into a new dataset
+// Settings is NOT a modal — preferences are distributed to per-pane disclosure
+// popovers (spec §01C/§07; see components/SettingsPopover).

 export type ActiveModal = ModalName | null;
 ```
@@ -76,8 +77,8 @@ export interface ModalConfig {
  init?: (arg?: string) => void;

  /** Serializable snapshot of in-progress edits, used to detect unsaved
-   *  changes on close. OMIT for modals that apply immediately (settings,
-   *  about, donate) — omission opts out of the discard-confirmation. */
+   *  changes on close. OMIT for modals with no in-progress edits to guard
+   *  (about, donate) — omission opts out of the discard-confirmation. */
  getState?: () => Record<string, unknown> | null;

  /** Whether the modal's primary action (Save / Apply) should be blocked
@@ -88,16 +89,24 @@ export interface ModalConfig {
  getError?: () => string | null;

  /** Whether this modal is reflected in the URL hash (back/forward, reload
-   *  restore). Datasets and Chart Builder are navigable; Donate is not. */
+   *  restore). Datasets and Chart Builder are navigable; About/Donate/Extract
+   *  are not (spec §01E). */
  isUrlNavigable?: boolean;
 }
 ```

+> **Shipped divergence.** The implemented registry (`modals/modal-registry.ts`) **omits
+> `hasError`/`getError`**: each modal renders its **own action row** inside its body (the
+> multi-view Datasets manager doesn't fit a single shell-level Save/Cancel), so validity is
+> each modal's own concern. The shipped `ModalConfig` keeps only `getState` (close-time
+> unsaved-change detection) plus `init`/`isUrlNavigable`. The generic-footer sketch through
+> the rest of this section is retained as the simpler pattern for a single-action modal —
+> treat it as illustrative, not a description of current code.
+
 ### Example entries

 ```ts
 import { DatasetsModal } from '../components/DatasetsModal';
-import { SettingsModal } from '../components/SettingsModal';
 import { ChartBuilderModal } from '../components/ChartBuilderModal';
 import { ExtractModal } from '../components/ExtractModal';
 import { DonateModal } from '../components/DonateModal';
@@ -105,7 +114,6 @@ import { AboutModal } from '../components/AboutModal';
 import { useDatasetStore } from '../stores/DatasetStore';
 import { useChartBuilderStore } from '../stores/ChartBuilderStore';
 import { useExtractStore } from '../stores/ExtractStore';
-import { useSettingsStore } from '../stores/SettingsStore';

 // Per-modal transient state lives in the relevant feature store; the registry
 // reads it via `getState()` (Zustand), never through component hooks.
@@ -152,22 +160,8 @@ export const MODAL_REGISTRY: Record<ModalName, ModalConfig> = {
    getError: () => (useExtractStore.getState().name.trim() ? null : 'modals.extract.nameRequired'),
  },

-  // Applies immediately — no getState, so closing never prompts.
-  settings: {
-    name: 'settings',
-    title: 'modals.settings.title',
-    component: SettingsModal,
-    isUrlNavigable: true,
-    init: () => useSettingsStore.getState().loadFromPrefs(),
-  },
-
-  // Pure info modals — no state, no validity, not navigable for donate.
-  about: {
-    name: 'about',
-    title: 'modals.about.title',
-    component: AboutModal,
-    isUrlNavigable: true,
-  },
+  // Pure info modals — no state, no validity, not navigable.
+  about: { name: 'about', title: 'modals.about.title', component: AboutModal },
  donate: { name: 'donate', title: 'modals.donate.title', component: DonateModal },
 };
 ```
@@ -297,9 +291,9 @@ export function hasUnsavedChanges(): boolean {
 ```

 The snapshot is taken once on open and compared on close. Modals without
-`getState` (settings, about, donate) snapshot to `null`, so `hasUnsavedChanges`
-short-circuits and they close instantly — correct, because they either apply
-immediately or hold nothing to lose.
+`getState` (about, donate) snapshot to `null`, so `hasUnsavedChanges`
+short-circuits and they close instantly — correct, because they hold nothing to
+lose.

 ### Validity passthrough

@@ -512,8 +506,8 @@ entry. Three properties force the split:
  component; its title/message/labels are supplied at the call site. There's
  nothing to register.
 - **Stacks _above_ a feature modal.** The discard-changes prompt must appear over
-  an already-open Datasets/Settings modal — which directly violates the feature
-  layer's "at most one open" rule. So confirmations live on a higher z-layer
+  an already-open feature modal (e.g. Datasets or Chart Builder) — which directly
+  violates the feature layer's "at most one open" rule. So confirmations live on a higher z-layer
  (`z-index: 1000`, above the future modal shell).
 - **Not navigable.** A confirmation is never a URL destination or a reload-restore
  target; it only exists for the duration of one decision.
@@ -579,15 +573,16 @@ tool.
 The coordinator is the join point for navigation:

 - `openModal` calls `syncModalToUrl`; navigable modals write a hash
-  (`#datasets`, `#datasets/dataset-<id>`, `#datasets/dataset-<id>/build`,
-  `#settings`). Non-navigable modals (donate) write nothing.
+  (`#datasets`, `#datasets/dataset-<id>`, `#datasets/dataset-<id>/build`).
+  Non-navigable modals (about, donate, extract) write nothing.
 - `closeModal` calls `clearModalFromUrl`, returning to the underlying workspace
  hash.
 - On load, the URL restorer reads the hash and calls `openModal(name, arg)` to
  rehydrate the right modal and sub-target.
- The global key handler maps `Cmd/Ctrl+K` → `toggleDatasets()`,
-  `Cmd/Ctrl+,` → `openModal('settings')`, and `Escape` → `closeModal()` (the
-  Escape binding is a no-op when `activeModal` is `null`).
+- The global key handler maps `Cmd/Ctrl+K` → `toggleDatasets()` and `Escape` →
+  `closeModal()` (a no-op when `activeModal` is `null`). `Cmd/Ctrl+,` opens the
+  editor **settings popover**, not a modal (`openSettingsPopover('editor-settings')`;
+  settings are distributed — spec §01C/§07).

 Because all of these call the same coordinator functions, browser
 Back/Forward, keyboard shortcuts, and in-app triggers stay consistent — they
@@ -335,10 +335,12 @@ useSettingsStore.subscribe((s, prev) => {

 ### Implemented policy: what renders immediately vs. debounced

-> The service above is a **sketch**; the shipped renderer lives inline in
-> `LivePreview.tsx` (one `setTimeout` whose delay is computed per change) and
-> subscribes to the stores via hooks rather than startup subscribers. When it is
-> extracted into a service, preserve this policy.
+> The service above is a **sketch** — its `useEditorStore`/`useSettingsStore` are
+> illustrative placeholders; the real inputs are `useAppStore` (`previewFitMode` +
+> `uiTheme`) and `useSnippetStore` (draft text / `bufferEpoch`). The shipped renderer
+> lives inline in `LivePreview.tsx` (one `setTimeout` whose delay is computed per
+> change) and subscribes to the stores via hooks rather than startup subscribers.
+> When it is extracted into a service, preserve this policy.

 The debounce exists to stay out of the way **while typing** — nothing else. So the
 delay is `0` (immediate) for everything except keystrokes (spec §03C):
@@ -403,8 +405,10 @@ It does two deterministic things, on a **deep copy** of the spec:
   referenced dataset's actual contents (inline values, raw CSV/TSV text, or a
   URL reference), recursing into layered/concat/child sub-specs.
 2. **Fit-mode sizing** — rewrites `width`/`height` per the active fit mode using
-   Vega-Lite's `"container"` keyword (Original = untouched; Width/Height/Full set
-   the corresponding dimension(s) to `"container"`), recursing the same way.
+   Vega-Lite's `"container"` keyword (Original = untouched; Width sets
+   `width:"container"` and **removes** `height`; Height sets `height:"container"`
+   and **removes** `width`; Full sets both — see spec §04 → Fit-mode sizing),
+   recursing the same way.

 This is _content_ preparation, not embedding, and it is fully covered by the
 _Live Preview_ spec. The only invariant this doc cares about:
@@ -301,10 +301,10 @@ silent. Crucially, names are reserved _as we go_ — within a single import, two
 incoming `Sales` datasets become `Sales 2` and `Sales 3`, not two `Sales 2`.

 ```ts
-// src/app/services/ImportService.ts
+// src/core/import-normalize.ts

-import { makeUniqueName } from '../../core/naming';
-import type { Dataset } from '../../core/types';
+import { makeUniqueName } from './naming';
+import type { Dataset } from './dataset';

 export interface DatasetRename {
  from: string;
@@ -492,15 +492,15 @@ export function renameDatasetEverywhere(

 ## 7. Where things live

-| Concern                                                                  | Location                                  | Pure?                          | Tested           |
-| ------------------------------------------------------------------------ | ----------------------------------------- | ------------------------------ | ---------------- |
-| `makeUniqueName`, `isNameTaken`                                          | `src/core/naming.ts`                      | yes                            | unit             |
-| `extractDatasetRefs`, `recomputeDatasetRefs`                             | `src/core/spec-refs.ts`                   | yes                            | unit             |
-| `renameDatasetInSpec`                                                    | `src/core/spec-refs.ts`                   | yes                            | unit             |
-| `snippetsReferencingDataset`, `datasetUsageCounts` (reverse-lookup scan) | `src/core/relationships.ts`               | yes                            | unit             |
-| `findSnippetsReferencingDataset`, usage count (snapshot wrappers)        | `src/app/services/RelationshipService.ts` | no (reads store)               | integration      |
-| `renameDatasetEverywhere` → `{ updated }`                                | `src/app/services/RelationshipService.ts` | no (mutates stores)            | integration      |
-| `dedupeIncomingDatasetNames`                                             | `src/app/services/ImportService.ts`       | nearly (uses `makeUniqueName`) | unit/integration |
+| Concern                                                                  | Location                                  | Pure?               | Tested      |
+| ------------------------------------------------------------------------ | ----------------------------------------- | ------------------- | ----------- |
+| `makeUniqueName`, `isNameTaken`                                          | `src/core/naming.ts`                      | yes                 | unit        |
+| `extractDatasetRefs`, `recomputeDatasetRefs`                             | `src/core/spec-refs.ts`                   | yes                 | unit        |
+| `renameDatasetInSpec`                                                    | `src/core/spec-refs.ts`                   | yes                 | unit        |
+| `snippetsReferencingDataset`, `datasetUsageCounts` (reverse-lookup scan) | `src/core/relationships.ts`               | yes                 | unit        |
+| `findSnippetsReferencingDataset`, usage count (snapshot wrappers)        | `src/app/services/RelationshipService.ts` | no (reads store)    | integration |
+| `renameDatasetEverywhere` → `{ updated }`                                | `src/app/services/RelationshipService.ts` | no (mutates stores) | integration |
+| `dedupeIncomingDatasetNames`                                             | `src/core/import-normalize.ts`            | yes                 | unit        |

 The dividing line: anything that takes plain data and returns plain data is
 **core** and unit-tested in isolation; anything that reaches into a Zustand store