# Engineering Review — 2026-07-02

> Point-in-time record. Full-project engineering review requested by the maintainer, with a
> specific question attached: _"as we work session to session, we may forget to look back
> and see the bigger picture — the end-of-session skill checks aim to mitigate it, but I'm
> not sure to what extent it is successful."_
>
> **Method:** five independent clean-context reviewers (code quality, test suite,
> documentation, build/tooling/delivery, cross-session coherence), each required to cite
> file/line/commit evidence, followed by an adversarial verification wave that reproduced
> every load-bearing claim (rebuilt the bundle, re-ran greps and git archaeology, hand-walked
> the flagged logic, re-checked the live site). Findings below are only those that survived
> verification; where a reviewer's number was wrong, the corrected number is used.
> Reviewed state: working tree at `c5e4c4c` plus the uncommitted spec-params/editor WIP.

## Verdict

The codebase itself is in excellent shape — the architecture contract holds under grep, not
just in prose, and the per-session review machinery demonstrably works at the session scale.
The problems are concentrated at two horizons the per-session view cannot see: **delivery**
(one measurable production defect: the marketing landing executes 1.3 MB gzipped of Monaco +
Vega it never uses) and **look-back**. The maintainer's fear is confirmed, mechanically: no
instrument — on-demand or scheduled — owns the whole-project view, so coherence work happens
only when a session happens to collide with it. (Maintainer clarification after the first
draft: the eng-council sweep and the ux-second-pass batch were designed as on-demand
session guardrails, not periodic instruments — which sharpens the finding rather than
softening it: the on-demand instruments have each fired once, and nothing at all runs on a
cadence.)

## Scorecard

| Dimension               | Grade | One-line summary                                                                      |
| ----------------------- | ----- | ------------------------------------------------------------------------------------- |
| Code quality            | A     | Zero `any`/suppressions, layering verifies by grep, error discipline is real          |
| Test suite              | A−    | 1,366 tests, uniform harness, deterministic; one committed coverage hole that matters |
| Documentation           | A−    | Symbol-level accuracy at scale; drift is localized fossils plus one governance gap    |
| Build / delivery        | B     | Strong local gates; landing bundle defect shipped because no gate measures output     |
| Cross-session coherence | B−    | A− for per-session machinery, D for look-back machinery                               |

## Critical findings (all independently verified)

### C1. The landing and /learn/ eagerly load and execute Monaco (940 kB gz) and Vega (285 kB gz)

Verified on the live site and reproduced from a clean build. `dist/index.html` modulepreloads
both chunks and the landing entry's static import graph executes them
(`main-*.js` ends with bare `import"./vega-*.js";import"./monaco-*.js"`). `/learn/` is worse:
both arrive as direct `<script type="module">` tags. Landing eager JS today: ~4.7 MB raw /
~1.30 MB gzip; the intended payload is ~255 kB raw / ~81 kB gzip — a 16× reduction available.

The source-level lazy-loading discipline is correct (`LandingChart.tsx` dynamic-imports
chart-renderer; nothing in `src/landing`/`src/learn` mentions Monaco). Two bundling-level
causes defeat it:

- **Preload-helper placement.** The object-form `manualChunks` in `vite.config.ts` leads
  Rollup to emit Vite's `__vitePreload` helper _inside_ the `monaco` chunk;
  `LandingChart-*.js` begins `import{_ as h}from"./monaco-*.js"`, so every chunk that uses
  `import()` statically depends on all of Monaco. (Attribution to the object form
  specifically is plausible-but-untested; the observed mechanism is fully reproduced.)
- **`vega-scale` edge.** `Landing.tsx` → `@core/vega-themes` → `theme-controls.ts:27`
  `import { scheme } from 'vega-scale'` merges into the `vega` manual chunk, dragging all of
  Vega into the landing's static graph. The comment at `theme-controls.ts:21-22` ("keeps the
  umbrella vega out of core") is true at source level and defeated by chunking. The
  `examples` chunk has a second, independent edge into the vega chunk.

**Fix:** break the `vega-scale` edge (lazy-import or move scheme resolution off the
landing-reachable path); switch to function-form `manualChunks` and confirm the helper lands
in a shared micro-chunk; and — the durable part — add a ~10-line post-build assertion that
`dist/index.html` and `dist/learn/index.html` reference neither `monaco-*` nor `vega-*`.
That assertion is the missing gate for this entire regression class.

### C2. The project's operating regime is recorded nowhere the machinery reads

The 2026-06-10 phase shift — spec follows code; docs/spec is no longer the authoritative
contract — appears in exactly one repo location: `docs/exploration/chart-builder-enhancement-scope.md:673`,
inside the directory whose charter says nothing there is kept current. Meanwhile CLAUDE.md
("authoritative behavioral specification… This is the contract"), AGENTS.md ("Spec is the
contract"), SOUL.md, arch 00 ("the spec wins"), and `.claude/skills/alignment/SKILL.md`
rules #3/#17 all still assert the old regime. Two reviewers independently converged on this,
and the verification wave reproduced the greps.

This is the worst coherence defect because it corrupts the corrective machinery itself: the
wrap-up protocol's whole design is clean-context subagents judging against the recorded
contract — and the recorded contract is wrong. **Fix:** one paragraph in CLAUDE.md/AGENTS.md
stating the regime (spec is descriptive, kept current with code; on conflict fix the spec),
softened phrasing in `docs/spec/README.md` and arch 00, and an update to the alignment skill.

### C3. The spec stopped absorbing new behavior at a verifiable cutover (2026-06-21 → 06-25)

Adjudicated timeline, commit-dated: through `d76a7a5` (06-21) every feature landed with its
spec section in the same commit (Theme Builder, FontAsset, data inspector, onboarding, URL
datasets — all specified at full fidelity). From `c19857b` (06-25) onward the discipline
inverted: 12 of 13 feature commits through 06-30 updated `docs/architecture/` in-commit and
**zero** touched `docs/spec/`. Unspecified user-facing surfaces at HEAD:

- the entire **/learn/** section (`grep -ril learn docs/spec/` → nothing);
- the **composition wireframe** (drag reorder / pull-out / stack / Simplify — zero matches);
- **editor transform actions and CodeLens scaffolds** (wrap/simplify/add-view, add-transform);
- the **multi-view Data Inspector view picker** (`docs/spec/04-live-preview.md:84` references
  "the chosen view's table" — the chooser it refers to is specified nowhere);
- the current uncommitted params-scaffolding work continues the pattern (arch 08 only).

Even under the spec-follows-code regime this is debt: the "then amend the spec to match"
half of the bargain has not happened for ten days of features. Root cause is structural, not
negligence: `/doc-update` flushes what a session remembers and `/alignment` reviews diffs —
no instrument owns "the spec describes the product." Alignment rule #17 covers removed/moved
surfaces only, not never-specified additions.

## Significant findings

### S1. No instrument owns the big picture on any cadence

- **Eng-council sweep:** once, 2026-06-12 (`0e225d7`). Since then non-test source grew
  ~**+76%** (16.8k → 29.7k ts/tsx LOC; ~21.3k → ~37k all-source). `docs/codebase-metrics.md`,
  whose stated purpose is the trend, holds two same-day rows. Specific unswept accretion: the
  six-module `spec-*` service family plus `editor-snippet`/`editor-cursor-lens` (~1,900 LOC,
  mostly post-sweep) — which alignment rule #16 already notes "has grown by copy-paste twice."
- **Batched council pass over `docs/ux-second-pass.md`:** once, 2026-06-13 (`92bfe88`,
  76→25 lines — the design worked). The file has regrown 25→48; oldest open item is from
  06-16.
- **TODO gardening:** never. Lifecycle is bimodal — 56 added / 43 removed over history
  (~71–77% resolution, usually within 0–3 days, at least 10 by dedicated commits), but every
  TODO that survived its first week is still alive. Oldest two are from 06-12:
  `src/app/modals/ModalCoordinator.ts:36` (a **latent init-ordering bug**, not polish) and
  `ChartBuilderModal.tsx:524`. The open set is increasingly the hard/ambiguous residue no
  session claims.

**Fix direction (maintainer to ratify):** give each instrument a trigger — re-sweep when
source LOC grows ~25% past the last metrics row; batch council pass when ux-second-pass open
items exceed a count or age past two weeks; a spec-reconciliation clause in `/doc-update`
("did this session add user-facing behavior? name the spec section or write it"); TODO
gardening as a standing sweep agenda item.

### S2. The scaffold insertion math is untested, duplicated, and validity-critical

Verified: `runAddTransform` (`spec-transform-scaffold.ts:130-146`, committed `c5e4c4c`) and
`runAddParam` (`spec-param-scaffold.ts:109-142`, WIP) compute insert offsets, indentation,
and the trailing-comma decision; a bug here writes **invalid JSON into the user's editor**.
No test exercises the composition — no `SpecEditor.test.tsx` exists, so there is no indirect
path either. The create-property block is a verbatim 7-line duplicate between the two files;
the completion providers share ~35 structurally identical lines. Hand-walking the current
logic found **no live bug** — this is a coverage hole, not a defect — but a future inversion
of the ternary or offset drift ships silently. The repo's own TODOs
(`spec-param-scaffold.ts:33`, `spec-params.test.ts:6`) already point at the fix: extract a
neutral core module (working name `spec-snippet`) computing
`(text, host offset, existing-array?) → {insertOffset, snippetText}` and table-test it over
empty-host / host-with-following-key / compact-one-line / host-as-last-property.

Related: `spec-transform-actions.ts` (663 lines, largest service, no test file) carries pure
unexported helpers with real branching — `reindent`, `defaultFacet`, `defaultRepeat` (:106,
:123, :136) — one layer short of tested core, mitigated by `buildNext` being a thin
dispatcher over tested core wrappers.

### S3. Documentation drift cluster (6/6 claims verified)

All in otherwise highly accurate docs; each is a fossil a maintainer would act on:

1. **5 MB budget fossil.** `docs/spec/09-data-model.md` §E and `08-import-export.md` describe
   a budget-with-fill-warnings storage monitor that spec 02/10, arch 02 §6, and the code all
   contradict (the monitor is a composition breakdown; the only 5 MB logic is the import
   pre-check in `transfer.ts:42`). Arch 02:445's "80% threshold" Do-line contradicts the same
   doc's line 425 and matches nothing in code.
2. **"Not installable" is false.** Arch 10:625 claims the manifest ships no icons; four icons
   ship (`vite.config.ts:85-89`, files in `public/`), and manual-verification actively tests
   install.
3. **The ajv layer was never built.** Arch 08 presents it as planned-M2 and claims "we
   deliberately do better: map ajv errors to editor positions" — no such module exists; ajv
   isn't even in `package.json`. The current WIP on arch 08 does not touch this.
4. **Wrong store name in three docs.** Arch 01/03/04 name `useSettingsPopoverStore` /
   `openSettingsPopover`; the code is `usePopoverStore` / `openPopover`
   (`PopoverStore.ts:22,30`, called from `EventRouter.ts:96`). Grep by the documented name
   finds nothing.
5. **Arch 03's "closed union" is stale.** Five modals listed; `modals/types.ts` has six
   (`themeBuilder`). Same doc's add-a-modal checklist instructs `hasError`/`getError` on
   `ModalConfig` — fields the real type doesn't have; following it fails typecheck.
6. **Spec 09's UserSettings omits `ui.dataInspectorOpen`/`ui.dataInspectorHeight`**, which
   spec 04 mandates and `settings-store.ts:158-183` persists (they're also absent from
   `core/settings.ts`'s type — the code half of the same gap).

### S4. Delivery gates never observe build output, and caching is misconfigured

- All quality gates live in the (excellent) pre-commit hook — lint-staged + full typecheck +
  the whole test suite — but nothing anywhere runs or inspects `vite build`. Cloudflare's
  build fails safe on compile errors (previous deploy stays live) but silently; the escape
  class is green-but-wrong output, which is exactly how C1 shipped. Cheapest durable fix: the
  C1 post-build assertion in the `build` script; optionally a minimal CI workflow
  (typecheck + lint + test + build + assertion) to cover `--no-verify` and web edits.
- Hashed `/assets/*` are served `cache-control: max-age=14400, must-revalidate` (Cloudflare
  Pages default; no `public/_headers` exists). Content-hashed files are the textbook
  `max-age=31536000, immutable` case; today every returning visitor revalidates a 3.6 MB
  chunk every 4 hours. HTML cache headers are correct.

### S5. Module-size outliers in the chart builder

`src/core/chart-builder.ts` (1,851 lines: catalog + rules + smart defaults + assembly/parse)
and `ChartBuilderModal.tsx` (1,653 lines, ~25 private subcomponents; 2.1× the next-largest
component). Internally cohesive, well-documented — maintainability risk, not defect. Split on
next substantive touch (`chart-builder/{catalog,rules,defaults,assemble}.ts`; hoist the
modal's sections into sibling files).

### S6. Smaller verified items

- `noUncheckedIndexedAccess` is off in a parser-heavy codebase (jsonc-parser node walking in
  `spec-params.ts`, `spec-cursor.ts`, `spec-insert.ts`) that is exactly its target class.
  Enable and sweep; if parser noise proves disproportionate, record the rejection.
- `ajv` is imported by `spec-params.test.ts:1` but undeclared — a phantom transitive
  dependency; add it to devDependencies (found during verification).
- `npm audit`: monaco 0.54.0 bundles a vulnerable DOMPurify (moderate; fix is 0.55.1, flagged
  breaking — plan deliberately); esbuild advisory is dev-only/Windows-only.
- Landing/learn light-dark toggle duplicated since 06-25, TODO'd in both copies and reworded
  06-28 instead of extracted — a breadcrumb treated as the deliverable.

## Minor findings

- Per-editor CodeLens providers register on the global `'json'` language
  (`editor-cursor-lens.ts:83`) while closing over one editor's cursor — safe with today's
  single editor (verified: one `monaco.editor.create`), latent cross-wiring if a second JSON
  editor appears. One guard line: `if (model !== editor.getModel()) return`.
- One avoidable non-null assertion in the WIP: `spec-transform-scaffold.ts:176`.
- `navigator.platform` (deprecated) in `EventRouter.ts:38`.
- Hooks are untested as a class; `useFocusTrap` (a11y-load-bearing, shared by all overlays)
  is the one worth a cheap happy-dom test. `ModalCoordinator`'s open/close/replace sequencing
  likewise (~5 tests).
- No coverage reporting exists; for a philosophy that is explicitly proportional ("core
  hardest"), nothing measures the proportion. `@vitest/coverage-v8` scoped to
  `src/core` + `src/app/{stores,services}`, no thresholds, visibility only.
- localStorage write failures are console-only (`settings-store.ts:80`, `ux-prefs.ts:80`) —
  judged an acceptable, explicit low-stakes swallow; noted so the asymmetry with the
  IndexedDB path stays a decision.
- `/learn/` is reachable only from the landing; nothing in the app links it. Deliberate or
  forgotten is unrecorded — which is itself the gap.
- Doc/housekeeping nits: pre-commit hook is stronger than AGENTS.md documents (says eslint;
  runs typecheck + tests too — drift in the good direction); PWA precache carries ~150–200 kB
  of landing/learn assets the `/app/`-scoped SW can never serve; spec 00 still calls settings
  a modal; spec 02 overstates the "imported" tag (only foreign/older shapes are tagged — spec
  08 has it right); `docs/embedding-vega-lite.md` (447 lines) is orphaned — index it with a
  charter line or move it; `core/settings.ts:64` and `HeaderControls.tsx:2` cite the old
  pre-move path of the theming scope doc; IMPLEMENTATION-PLAN files a completed item under
  "Next"; empty `ops/` directory; `vega-themes` pinned exact with no recorded rationale;
  manual-verification.md lacks checks for the shipped theming/export surface (Theme Builder
  gallery, font upload incl. variable fonts, PNG/SVG export, clipboard).

## Strengths (verified, and worth keeping deliberate)

1. **The architecture contract verifies by grep, both directions.** No browser APIs, React,
   or Monaco in `src/core/`; storage APIs confined to `infrastructure/`; landing/learn import
   no stores/orchestration/components.
2. **TypeScript rigor at the top of the distribution.** Zero `any`, zero
   `@ts-ignore`/`@ts-expect-error`, exactly one non-null assertion across ~30k lines;
   type-aware ESLint (incl. `no-floating-promises`) passes clean; every intentionally
   unawaited promise is an explicit `void` with a comment.
3. **Error-handling discipline is real.** Every swallowed catch inspected carries a rationale
   naming the sanctioned fallback; real failures route to notifications with actionable copy;
   quota errors are normalized and surfaced.
4. **Concurrency/resource engineering above typical app code.** `LivePreview`'s generation
   token + render mutex (with reasoning written down), full Monaco disposable cleanup, the
   finalize-before-unsubscribe edge handled in chart-renderer.
5. **The test suite is engineered, not accumulated.** 1,366 tests / 8.5s, zero snowflake
   harnesses across all 17 component test files, injected clocks and fake timers keyed to
   exported constants, fresh `IDBFactory` per test (including interrupted-upgrade
   self-healing), tests that assert contracts with the why inline
   (`spec-refs.test.ts`, `chart-renderer.test.ts`).
6. **Session-scale coherence is solved.** All 12 stores share one canonical shape and cite
   their canonical sibling in doc comments; modals go through one registry/coordinator/shell;
   subtraction happens (`PreviewStore` deleted when obsoleted). The "written by strangers"
   failure mode is absent.
7. **The breadcrumb→fix pipeline works.** At least 10 TODOs resolved by dedicated commits
   (quota rollback, hash routing, splitter ARIA, field-name escaping…); the eng-council→law
   loop is real (sweep findings became alignment rules #15–#17; skills keep evolving).
8. **Docs are accurate at symbol level at scale** — of ~35 spot-checked claims (constants,
   record shapes, hash grammar, keyboard maps, API surfaces), nearly all exact — and the
   "shipped divergence" fencing discipline keeps pedagogical sketches from lying.
9. **PWA configuration is best-practice**: `registerType: 'prompt'` properly consumed with a
   durable update toast, SW scope narrowed to `/app/`, sophisticated font precache strategy
   with reasoning in the config.

## Recommended sequence

1. **Record the regime** (C2): CLAUDE.md/AGENTS.md paragraph + alignment-skill update +
   soften spec README/arch 00. Smallest fix, unblocks every future clean-context review.
   _Done same day: CLAUDE.md, AGENTS.md, SOUL.md, spec README, and alignment rules #3/#17
   now state the spec-follows-code regime; #17 extended to cover never-specified additions._
2. **Fix the landing bundle and add the post-build assertion** (C1): the only
   production-visible defect, and the assertion closes the gate gap (S4) for free. Add
   `public/_headers` for immutable assets while in there.
   _Done same day: `schemeColors` split into `core/scheme-colors.ts`; function-form
   `manualChunks` with explicit homes for the preload helper and the shared light packages
   (vega-themes, vega-expression, vega-util, json-stringify-pretty-compact — Rollup was
   absorbing each into the nearest vendor chunk); `scripts/check-light-entries.mjs` gates
   every build; `public/_headers` ships immutable caching. Landing eager JS measured
   ~98 kB gzipped, down from ~1,300 kB; all three entries smoke-tested on the production
   build (charts + Monaco render, zero console errors)._
3. **Spec back-fill sprint** (C3): specify /learn/, the wireframe, editor actions/scaffolds,
   and the view picker; add the spec-reconciliation clause to `/doc-update`.
4. **Give the periodic instruments triggers** (S1) and run the overdue ones: an eng-council
   re-sweep (which naturally absorbs S5's splits and the `spec-*` family consolidation), a
   ux-second-pass batch pass, and TODO gardening — starting with `ModalCoordinator.ts:36`.
5. **Extract and test the scaffold insertion core** (S2) before the params WIP is committed —
   the TODOs already name the module.
6. **Doc drift batch** (S3 + minors): one session, mostly deletions of fossils.

---

## Addendum: subtraction audit (same day)

Follow-up requested by the maintainer: how much of the codebase is duplication, boilerplate,
or unnecessary wrapping — with the explicit instruction that **deliberate architecture layers
are not exempt**; "less LoC = less surface for bugs" outranks ceremony. Two auditors (a
jscpd-quantified duplication pass and a wrapper/indirection pass with the contract layers on
trial); the largest verbatim-copy claims were independently re-diffed before inclusion.

### The numbers

- **Production duplication is low: ~0.9%** by jscpd at min-tokens 50 (268 duplicated lines
  over ~29.5k production TS/TSX; roughly double counting renamed semantic twins). Test-file
  duplication is ~5.4% but is the documented harness convention — benign. CSS Modules are
  the highest-duplication format at 5.4% and deserve one deliberate pass.
- **Pure ceremony is ~1.5% of non-test LoC.** The plumbing layers (infrastructure +
  orchestration + modals + hooks, ~2.8k lines, ~9% of src) are overwhelmingly load-bearing:
  `db.ts`'s promise wrapping/self-healing/quota normalization, `url-hash.ts`'s total-parse
  grammar, `remote-data.ts`'s size-cap and error classification, the modal coordinator's
  snapshot/confirm/URL lifecycle all earn their lines on inspection. No generic machinery
  with a single instantiation was found — `editor-cursor-lens` has three real consumers,
  `wireEntityWriteThrough` four, and every modal-registry field varies across entries.
- **Confidently deletable today, behavior identical: ~450–500 LoC**, plus ~100–150 more
  behind design decisions.

The diagnosis, verbatim from the audit because it generalizes: **the abstractions are
right; the residue is photocopied instantiation files and hand-unrolled adapter pairs that
the abstractions should have absorbed.** Nothing calls for architectural change — only
tidying inside the architecture. Notably, one suspicion from the main review was stale: the
orchestration wirers were already consolidated behind `wireEntityWriteThrough`; the leftover
per-entity files are the residue that consolidation should have deleted.

### The cut list (verified; ranked by value)

Do right away — all mechanical, most protected by existing tests:

1. **Per-preference persistence chain** (~90–110 LoC; the compounding win). Five
   near-identical load/save pairs in `settings-store.ts:116-184`, four identical 10-line
   init/wire pairs in `orchestration/preferences.ts`, same shape again in
   `orchestration/settings.ts`/`snippet-sort.ts`, ten sequential calls in `main.tsx`.
   Collapse: a `uiSlicePref(key, validate, fallback)` adapter factory + a
   `wireStorePref(store, selector, save)` orchestration helper. Every future preference then
   costs ~8 lines instead of ~30 across four files. `theme.ts` (FOUC ordering) and
   `panes.ts` (debounce) stay bespoke — they carry real variation.
2. **Entity-adapter photocopies** (~55–60 LoC). `snippet-store.ts` / `dataset-store.ts` /
   `theme-store.ts` / `font-store.ts` are the identical 28-line load/save/delete triple
   differing only in store name, migrate fn, and version constant (re-diffed: confirmed).
   Collapse: `makeEntityAdapter<T>(storeName, version, migrate)` in `db.ts` + four 3-line
   instantiations. Migration files stay — real per-entity logic.
3. **Wirer residue files** (~55 LoC). `dataset-persistence.ts` / `theme-persistence.ts` /
   `font-persistence.ts` are 25-line modules whose body is one 6-line
   `wireEntityWriteThrough` call. Move the calls into `entity-persistence.ts` or
   `startup.ts` (their only caller). `snippet-persistence.ts` stays — the debounced
   autosave is real logic.
4. **`editor.addAction` ceremony** (~55 LoC). Thirteen copies of the same 6-line
   registration object across `spec-transform-actions.ts:524-585` and
   `spec-config-actions.ts:210-235` → one table + `.map(addAction)`.
5. **Scaffold-twin merge** (~40–50 LoC; overlaps main-review S2 and shares its fix). The
   completion prologue, suggestion mapping, and create-property block are verbatim between
   `spec-transform-scaffold.ts` and `spec-param-scaffold.ts`. Collapse into
   `editor-snippet.ts` helpers; the pure indent math moves to the TODO'd core module, where
   it also becomes testable.
6. **Core jsonc-helper copies** (~25–30 LoC). `objectKeys` byte-identical between
   `spec-params.ts:85-93` and `spec-data-transforms.ts:103-111` (re-diffed: confirmed);
   `paramsArrayNode`/`transformArrayNode` identical modulo key string; two more shared
   shapes. Extract/share — both sides have strong tests.
7. **localStorage adapter shell** (~28 LoC). `readRaw`/`writeRaw`/`available` identical
   between `settings-store.ts` and `ux-prefs.ts` modulo type name and log tag (re-diffed:
   confirmed) → `jsonLocalRecord<T>(key, tag)` in the same layer.
8. **Batch of micro-cuts** (~90 LoC): `UrlStateSync`'s argument-ignoring
   `syncModalToUrl`/`clearModalFromUrl` wrappers (delete when fixing the
   `ModalCoordinator.ts:36` ordering TODO they paper over); `DatasetStore`'s
   thrice-repeated save epilogue; `readTextFile`/`readBinaryFile` 1:1 renames of `File`
   methods; `initPersistentStorage` pass-through; `modals/types.ts` folded into the
   registry; `SpecEditor`'s seven subscribe/dispose pairs → array; the landing's four-times
   repeated `shot` block + TODO'd `UiTheme` redeclaration; `Icon.tsx`'s four-times copied
   panel frame; one dead CSS rule (`ChartBuilderModal.module.css:213`).

Needs design thought (~100–150 LoC more): the splitter triple (`ResizeHandle` /
`PaneSplitHandle` / `InspectorSplitHandle` share a near-verbatim `role="separator"` block —
worthwhile but untested a11y surface, do with manual keyboard verification); the theme-panel
Color+Size+Weight row groups (60–100 LoC vs greppability of accessible names); the modal
master-detail CSS block; the landing/learn theme-toggle hook (blocked on where a shared
React hook may live under the entry-isolation rule — needs a maintainer call).

Adjudicated and acquitted (challenged per the widened mandate, earn their lines): the modal
registry/coordinator/shell trio (every registry field varies; flattest honest version saves
only ~35 lines); the Zustand store shapes (a generic collection-slice helper would cost more
in typing than the ~60 lines it saves and obscure real per-store invariants); core purity
shims (minimal — the altitude problem runs the other way, app services holding pure logic);
the non-photocopy infrastructure adapters (real normalization, migration, error mapping).

### Wrap-up follow-ups (recorded from the params-feature review passes, same day)

The params WIP was closed through the full wrap-up protocol (alignment + eng-council, both
clean-context). Fixed in that pass: an `isUnitNode` bug (nested layer containers offered
schema-invalid selection params), the S2 extraction (`core/spec-snippet.ts`, insertion math
now table-tested applied-and-reparsed), spec §03 scaffolding coverage, the transforms
catalog upgraded to real-schema validation, `ajv` declared. Deferred with recorded homes:

- `src/core/lesson-parse.ts` exports five unused types (knip) — verify and unexport. Same
  check for `LensFactory` in `editor-cursor-lens.ts:24` (exported, consumed only in-module).
- `spec-transform-actions.ts` has an internal ~8-line clone (~:269-276 vs ~:346-353) —
  fold into the existing 663-LOC consolidation item.
- The Ajv compile boilerplate now lives in two test files; a third schema-validating
  catalog test should extract a shared helper — and at that point promote the rule
  "seeded editor catalogs validate against the bundled VL schema, not just JSON.parse"
  to an `/alignment` check.
- A small knip config (ignore `@fontsource/*`, `marked` false positives) would make
  future dead-export sweeps one command.