Incremental reactivity via @preact/signals-core + architecture decisions (part of #88)

.github/ISSUE_TEMPLATE/roadmap.md

@@ -0,0 +1,23 @@
+---
+name: Roadmap item
+about: A planned phase / tracked feature with a spec (links to roadmap #68)
+labels: enhancement
+---
+
+### Goal
+<!-- What this delivers and why it's on the roadmap. -->
+
+### Files / scope
+<!-- Modules expected to change (src/core, src/ui, …); rough LOC; non-goals. -->
+
+### Key implementation
+<!-- Approach: injected seams, pure-logic split, migration order. -->
+
+### Acceptance criteria
+<!-- Testable exit conditions per slice. Coverage gate stays 100/100/100/100. -->
+
+### Re-evaluation trigger
+<!-- When to reconsider the approach. -->
+
+### Tracking
+<!-- Parent roadmap meta-issue (currently #68); related ADR / issues. -->

.github/PULL_REQUEST_TEMPLATE.md

@@ -8,3 +8,4 @@
 - [ ] Layers kept honest: pure logic in `src/core/`, network in `src/net/` (injected fetch), DOM in `src/ui/`
 - [ ] No new runtime dependency (or it's a deliberate, justified addition — see CONTRIBUTING)
 - [ ] README / `CHANGELOG.md` (`[Unreleased]`) updated if behavior or the deployed surface changed
+- [ ] Reconciled affected tracked work (roadmap #68, the issue body, ADR/CHANGELOG) if this change reshaped it

CHANGELOG.md

@@ -16,6 +16,15 @@ auto-generated per-PR notes; this file is the curated, human-readable history.
   Chromium/Firefox/Safari are supported (Safari verified green on CI); the full
   browser/ClickHouse/IdP matrix is tracked in #71. (#69)
 
+### Changed
+- State reactivity now uses `@preact/signals-core` (the third bundled runtime
+  dependency), adopted incrementally per
+  [ADR-0001](docs/ADR-0001-reactivity.md): the tab list, side panel, run state
+  (`running`/`resultView`), and the library title repaint via signal `effect`s
+  instead of manual render calls. No user-facing behavior change. A Preact
+  schema-panel spike was evaluated and **rejected** — the app stays
+  framework-free (ADR-0001 addendum). (#88)
+
 ## [0.1.5] - 2026-06-29
 
 ### Added

CLAUDE.md

@@ -1,8 +1,8 @@
 # Contributor guide — altinity-sql-browser
 
 A modular ES-module SPA that builds to one self-contained HTML file served from
-ClickHouse. No framework; runtime deps are rare and deliberate (currently two,
-both bundled — see hard rule 4). Quality is held by tests.
+ClickHouse. No framework; runtime deps are rare and deliberate (currently three,
+all bundled — see hard rule 4). Quality is held by tests.
 
 ## Hard rules
 
@@ -22,16 +22,32 @@ both bundled — see hard rule 4). Quality is held by tests.
    (see README "Configuring OAuth").
 4. **The build is esbuild only; runtime deps are rare and deliberate.** Source
    files are the tested files; esbuild bundles `src/main.js` → `dist/sql.html`.
-   There are **two** bundled runtime dependencies — **Chart.js** (the Chart
-   result view) and **@dagrejs/dagre** (the EXPLAIN pipeline-graph layout) — both
-   inlined into the artifact, so the page still makes zero third-party requests.
+   There are **three** bundled runtime dependencies — **Chart.js** (the Chart
+   result view), **@dagrejs/dagre** (the EXPLAIN pipeline-graph layout), and
+   **@preact/signals-core** (the reactivity primitive — see
+   `docs/ADR-0001-reactivity.md`) — all inlined into the artifact, so the page
+   still makes zero third-party requests.
    Adding *another* runtime dependency is a deliberate decision (it grows the
    single served file) — don't do it casually. When a feature needs a library,
    keep the testable logic pure in `src/core/` (chart axis/role/pivot math in
    `src/core/chart-data.js`; DOT→positions in `src/core/dot-layout.js`, both
    100%-covered) and make the library call an **injected seam** (`app.Chart` /
    `app.Dagre`, like the fetch/crypto seams) so the DOM wrapper stays fully tested
    rather than dropping below the coverage gate.
+5. **No UI framework; signals for state, imperative adapters for islands.** State
+   reactivity is `@preact/signals-core` (`signal`/`effect`/`computed`/`batch`),
+   migrated slice-by-slice (ADR-0001). **No React/Preact/Solid** — a Preact spike
+   on the schema panel (`spike/preact-schema`, ADR-0001 addendum) confirmed a
+   component model removes the in-place-mutation pain but buys a second render
+   paradigm the roadmap doesn't justify. The hard, third-party, or
+   high-frequency-pointer surfaces (the editor, the EXPLAIN/schema graphs,
+   Chart.js, result-grid resize/sort) stay **imperative behind an injected seam** —
+   signals coordinate state, they don't own every mousemove. CodeMirror 6 is the
+   pre-approved next runtime dep, behind an `EditorPort` seam, to land when
+   schema-aware autocomplete (#84) does (#21). When a *second* consumer of a
+   complex UI pattern appears, extract a shared primitive (e.g. `EditorPort`,
+   `GraphSurface`, a result-view registry, `Drawer`) rather than copy it — but
+   don't build a primitive speculatively for a single caller.
 
 ## How to add a result view / panel / feature
 
@@ -57,3 +73,20 @@ Touch these in one change:
 
 Pure-by-construction modules, injected side-effect seams, per-file coverage
 thresholds, and a single ClickHouse-served artifact built by esbuild.
+
+## Working discipline
+
+- **Surface out-of-scope findings, don't bury them.** Spot a real bug, data
+  inconsistency, deprecated API, or future footgun outside the current task →
+  open an issue labeled `inbox` (file:line + why deferred) and tell the user.
+  High signal only, not style nits.
+- **Reconcile forward work after a substantive change.** A change to behavior,
+  schema, or a settled decision can stale tracked work. In the same commit,
+  reconcile what it reshaped: the roadmap meta-issue (currently #68) — re-check
+  or re-scope the track it touches; the affected issue's body (Goal/Acceptance);
+  the relevant ADR addendum and `CHANGELOG.md` `[Unreleased]`; and any issue it
+  obsoletes (close via "Closes #N" in the PR). Flag it if the rework is large.
+  (Trivial typo/comment changes exempt.)
+- **Convert friction into memory.** If a task needed retried commits or hit an
+  unexpected failure (test/env/scope surprise), save a memory so the next
+  session doesn't repeat it.

THIRD-PARTY-NOTICES.md

@@ -1,7 +1,7 @@
 # Third-party notices
 
 The Altinity SQL Browser is licensed under Apache-2.0 (see `LICENSE`). The built
-single-file artifact (`dist/sql.html`) inlines the two runtime dependencies
+single-file artifact (`dist/sql.html`) inlines the three runtime dependencies
 below; this file reproduces their MIT license texts as required, and the same
 notices are embedded as a comment at the top of the built artifact.
 
@@ -32,3 +32,17 @@ Permission is hereby granted, free of charge, to any person obtaining a copy of
 The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
 
 THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+---
+
+## @preact/signals-core — v1.14.3
+
+The MIT License (MIT)
+
+Copyright (c) 2022-present Preact Team
+
+Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

build/build.mjs

@@ -2,7 +2,7 @@
 // is inlined (with the stylesheet) into build/template.html → dist/sql.html.
 //
 // esbuild is the only build-time tool; the bundled runtime dependencies are
-// Chart.js and @dagrejs/dagre (inlined, not fetched). The output is a self-contained HTML file
+// Chart.js, @dagrejs/dagre, and @preact/signals-core (inlined, not fetched). The output is a self-contained HTML file
 // that installs into any ClickHouse cluster's user_files and is served by an
 // <http_handlers> static rule — it still makes zero third-party requests.
 
@@ -52,8 +52,8 @@ async function main() {
   const styles = await readFile(resolve(root, 'src/styles.css'), 'utf8');
   const template = await readFile(resolve(here, 'template.html'), 'utf8');
 
-  // The two runtime deps (Chart.js, dagre) are MIT and inlined into the bundle,
-  // so the artifact must carry their notices. esbuild strips legal comments
+  // The runtime deps (Chart.js, dagre, @preact/signals-core) are MIT and inlined
+  // into the bundle, so the artifact must carry their notices. esbuild strips legal comments
   // (legalComments: 'none'), so embed THIRD-PARTY-NOTICES.md as a leading HTML
   // comment — sanitized so its text can't close the comment early.
   const notices = (await readFile(resolve(root, 'THIRD-PARTY-NOTICES.md'), 'utf8')).replace(/--+>?/g, '-');

docs/ADR-0001-reactivity.md

@@ -0,0 +1,154 @@
+# ADR-0001: Reactivity — incremental signals, not a framework
+
+- **Status:** Accepted — adopted on `spike/signals-core` (#88)
+- **Date:** 2026-06-29 (adopted 2026-06-30)
+- **Context tracking:** roadmap #68; adoption #88
+- **Evidence:** `spike/signals` (hand-rolled primitive + two converted slices),
+  `spike/signals-core` (the chosen primitive; tabs/sidePanel, then the
+  `resultView`+`running` and `libraryName`+`libraryDirty` scalar slices)
+
+## Context
+
+The app is a framework-free ES-module SPA: pure logic in `src/core/` (100%
+covered), an injected-seam controller (`createApp(env)`), and a hand-rolled
+hyperscript render layer in `src/ui/`. As feature count grows (16+ open issues),
+the recurring pain is **manual render invalidation**: state is mutated and then
+the dependent views are repainted by hand (e.g. `tabs.js`'s old `refresh()`
+called `renderTabs` + editorSync + `renderResults` + `updateSaveBtn`). Forgetting
+a dependent → stale UI. This is an *absence-of-reactivity* problem, distinct from
+a *component-model* problem.
+
+Three hard constraints frame any solution: a single self-contained HTML artifact
+with zero third-party requests (CLAUDE.md rule 4 — deliberate deps only), the
+per-file 100/100/100/100 coverage gate (rule 1), and the injected-seam layering
+(rule 2).
+
+## Decision
+
+Adopt **`@preact/signals-core`** (`signal()` / `effect()` / `computed()` /
+`batch()` / `untracked()`) and migrate state **one slice at a time**, reading
+and writing through `.value`. An accessor helper (like `activeTab()`) is
+*optional* — used where it usefully contains reader churn, skipped where the
+churn is trivially mechanical (see the Consequences guideline). **Do not** adopt
+a UI framework (React/Preact/Solid).
+
+A hand-rolled ~70-line primitive was prototyped first and works (`spike/signals`),
+but `@preact/signals-core` was chosen over it: same `.value` API (so the
+migration code is identical and the choice is reversible in ~5 lines), but
+maintained, glitch-free (correct topological/diamond updates), and with a lazy
+memoized `computed()` — none of which the naive synchronous prototype provides.
+It costs one small, zero-transitive-dependency package and **+1.4 KB gzip** to
+the artifact (vs +0.45 KB hand-rolled), in exchange for not owning ~70 lines +
+~180 test lines of correctness-critical reactive code. Per CLAUDE.md rule 4 this
+is a deliberate dependency; it is still inlined, so the artifact makes zero
+third-party requests.
+
+## Options considered (all measured)
+
+| Option | artifact Δ (gzip) | Verdict |
+|---|---|---|
+| **@preact/signals-core** | **+1.4 KB** | **Chosen** — maintained, glitch-free, `computed`; same `.value` API; we own no reactive code |
+| Hand-rolled `signal.js` | +0.45 KB | Viable (`spike/signals`) — zero deps, but we own + 100%-cover it; naive (not glitch-free), no `computed` |
+| Preact + @preact/signals | +7.3 KB (`spike/preact`) | Rejected — its only edge over signals-core is auto DOM-diffing of large lists, which we don't need |
+| Solid.js | ~+7 KB | Rejected — compiler/plugin + ecosystem cost; same unneeded big-list benefit |
+| React (proper) | ~+45 KB | Rejected — heaviest dep, vDOM-on-big-tables liability, mismatch with the single-file ethos |
+
+Decisive factor: we do **not** need a virtualized / large-list renderer (no
+10k-row grid requirement), which is the one thing a vDOM framework buys over a
+bare signals core. The pain is invalidation, which signals solve directly — and
+between the two signals options, a maintained, glitch-free core beats owning the
+primitive for ~1 KB.
+
+## Evidence (spike)
+
+Two slices converted end-to-end with the full suite + per-file coverage gate
+green and a real `npm run build` (1033 tests on the hand-rolled branch; 1022 on
+`spike/signals-core`, which drops the 11 primitive-internal tests):
+
+- **tabs** (`tabs` + `activeTabId`) — *has* an `activeTab()` accessor → 16 callers
+  insulated; low mechanical churn, but one behavioral test relocation (the
+  repaint responsibility moved from `tabs.js` to a `createApp()` effect).
+- **sidePanel** — *no* accessor → every reader changed, but the churn was purely
+  mechanical `.value` and concentrated in the panel's own test file; no assertion
+  rewrites.
+
+Both branches share the slice conversions verbatim. Moving from the hand-rolled
+primitive to `@preact/signals-core` was a 5-line diff (import swaps + deleting
+`src/core/signal.js` and its test), demonstrating the API parity and that the
+choice is reversible.
+
+## Consequences
+
+- Mutating a converted slice is just `state.x.value = …`; an `effect()` repaints.
+  Manual `refresh()` lists are deleted.
+- **Migration is monotonic**: a slice keeps some direct render calls until its
+  co-dependent slices are also signals. No slice un-converts another.
+- Per-slice cost is mostly mechanical `.value` edits in that slice's tests.
+- **Guideline (not a rule):** add an accessor helper (like `activeTab()`) for a
+  slice with many scattered readers to contain churn; slices with few or
+  localized readers convert fine with bare `.value`. Validated by the
+  `sidePanel` slice (no helper — "worst case") and the `resultView`/`running`
+  and `libraryName`/`libraryDirty` scalar slices, all helper-free.
+- Adding `@preact/signals-core` makes **three** bundled runtime deps. On adoption
+  the dependency count was updated in CLAUDE.md rule 4, THIRD-PARTY-NOTICES.md,
+  and `build/build.mjs` comments (all done).
+- Re-evaluate Preact/Solid only if the UI later grows many interdependent
+  components with rich local state, or a genuine large-list render need appears.
+
+## Addendum — Preact spike on the schema panel (#88, branch `spike/preact-schema`)
+
+The scalar slices (`resultView`/`running`, `libraryName`/`libraryDirty`) fit
+signals-core cleanly. The **schema panel** does not: `state.schema` was a nested
+array mutated *in place* (`db.expanded`, an `expandedTables` Set, `tb.columns`
+flipping null→'loading'→array), and signals only react to reference changes — so
+a signals-core conversion would just relocate the manual-invalidation pain into
+`schema.value = [...]` bumps. That makes it the fair test of whether a
+component+vDOM model earns adoption. We built it as Preact components
+(`SchemaTree → DbRow → TableRow → ColumnRow`) to gather evidence — explicitly
+re-opening this ADR's "no framework" line for *complex panels only*.
+
+**What the spike proved (the thesis holds):**
+- The in-place-mutation anti-pattern is **gone**. Per-row expand state and lazy
+  columns are component-local `useState`; the `expandedTables` Set and
+  `db.expanded` were deleted; `loadColumns` became `fetchColumns` (returns the
+  columns for local state, still caches to `tb.columns` for autocomplete — a data
+  cache now decoupled from rendering). `state.schema`/`schemaError`/`schemaFilter`
+  are signals the tree reads via `@preact/signals` auto-tracking; no manual
+  `renderSchema()` calls remain.
+- **The 100/100/100/100 per-file coverage gate is achievable** on the component
+  file (`src/ui/schema.js`) with tests ported to preact's `render` + `act()`.
+- Signal→component auto-tracking works under happy-dom (load and filter repaint
+  the mounted tree).
+
+**What it cost (measured / observed):**
+- **Bundle: +6.8 KB gzip** (148,044 → 154,873 B; raw 457,892 → 474,440) — close
+  to this ADR's +5.9 KB estimate. ~+4.6% of the artifact, still zero third-party
+  requests (inlined).
+- **A second paradigm.** signals-core `effect()`s drive the rest of `createApp`
+  (tabs/results/title); the schema tree is Preact. Two render models coexist —
+  the main ongoing cost.
+- **Integration seams with the imperative layer:** `Icon.*()` returns live SVG
+  DOM nodes that Preact can't embed, so a `ref`-mounter bridges them; preact's
+  `h` vs the project's hand-rolled `h`; `loadColumns` had to be reshaped.
+- **Test friction:** scenarios staged by poking global state (`expandedTables`,
+  `tb.columns`) now require driving interactions; every interaction needs `act()`;
+  the double-click detector forced a fake-timer gap between open/collapse clicks.
+  Coverage still reaches 100%, but assertions shifted from state to DOM/behaviour.
+- **LOC:** `src/ui/schema.js` 149 → 185 (+36), plus `preact` + `@preact/signals`
+  deps (would make **five** bundled runtime deps; CLAUDE.md rule 4 +
+  THIRD-PARTY-NOTICES would need updating *iff* adopted — not done on the spike).
+- Used preact's `h()` (zero build/test config); JSX would improve ergonomics
+  further but needs esbuild + vitest jsx config — not required to evaluate the
+  component model.
+
+**Recommendation: do not adopt Preact now; stay signals-core.** The component
+model genuinely removes the anti-pattern and is fully testable, but the costs
+(a second render paradigm app-wide, +6.8 KB, the icon/`h`/columns integration
+seams) are paid across the whole app to benefit **one** panel. That matches this
+ADR's original reasoning — the need is invalidation, not a component model, and
+there is no large-list requirement. Keep the schema panel as a **documented
+imperative exception** (or, if its `tb.columns`/expand churn becomes a real
+maintenance drag, convert it with signals-core using *replaced* Set/Map-valued
+signals rather than in-place mutation). The `spike/preact-schema` branch stands
+as the evidence; re-open the decision only when several complex, interdependent,
+rich-local-state panels are actually on the roadmap (per the trigger above).

package.json

@@ -22,6 +22,7 @@
   },
   "dependencies": {
     "@dagrejs/dagre": "^3.0.0",
+    "@preact/signals-core": "^1.14.3",
     "chart.js": "^4.5.1"
   }
 }

src/main.js

@@ -61,7 +61,7 @@ export async function bootstrap(app, env) {
     catch { shared = { sql: '', chart: null }; }
   }
   if (shared.sql) {
-    const t0 = app.state.tabs[0];
+    const t0 = app.state.tabs.value[0];
     t0.sql = shared.sql;
     t0.name = 'Shared query';
     if (shared.chart && shared.chart.cfg) {