perf: virtualize charts/grids/editors, lazy-hydrate notebook cells

[emrberk]

Summary

Makes large notebooks remain responsive as they grow. Cells outside the viewport no longer keep their full editor, chart, result grid, and refresh workload active, reducing render work, memory use, and background queries. With these limits in place, the notebook cell cap increases from 50 to 200.

What changes

• Nearby cells render normally; distant cells use lightweight placeholders. Focused, running, and recently edited cells stay active, and full content returns when the user scrolls back.
• Offscreen chart refreshes pause and resume when needed, so notebooks with many live charts do not continuously query and render in the background.
• Saved query results can be released from memory and loaded again on demand. Cell sizing and run status remain available while the result is unloaded.

Validation

Typecheck, lint, build, and all unit tests pass.

Closes #576.

[emrberk]

Code Review — PR #589

refactor: move chart refresh out of the component (WIP draft, closes #576)
Repo: questdb/ui · Review level: 3 (full mission-critical pass — all 13 agents incl. fresh-context adversarial, per-finding verification)
Scope: 82 files, +9,341/−1,683. A large Notebook-subsystem refactor: chart auto-refresh moved into an imperative engine, cell virtualization (mount/retain bands + placeholders), and on-demand IndexedDB↔memory result hydration.

---

Quality gate

All green — no findings.

Check	Result
yarn typecheck	✓ pass
yarn lint	✓ pass
yarn build	✓ pass
yarn test:unit	✓ 1715/1715 pass

Headline

The data-integrity core is clean. Four independent passes (query/data, async/cancellation, React correctness, cross-context) each returned NO FINDINGS, corroborating a manual trace: the memory→disk release path is fail-safe by construction — a cell's in-memory result is dropped only when its exact array identity is in the persisted WeakSet, populated solely after a confirmed Dexie write. No total-data-loss bug exists. No Critical findings. The confirmed issues are UX, a11y, edge-case durability, and one intended tradeoff worth an explicit sign-off.

---

Issues

#	Issue	Category	Severity	Location
1	Failed snapshot load → permanent shimmer, no error/retry	State & context	Moderate	in-diff
2	Wide results silently shrink after deep scroll-away/return	Query & data integrity	Moderate (needs discussion)	in-diff
3	Chart entry animation ignores prefers-reduced-motion	Styling & theming	Moderate	in-diff
4	Keyboard focus dropped when released cell hydrates	Accessibility & UX	Moderate (narrow)	in-diff
5	Find-in-page can't locate off-screen cell text	Accessibility & UX	Moderate (inherent tradeoff)	in-diff
6	useAdaptivePoll hook wiring never render-tested	Test coverage	Moderate	in-diff
7	mountedCellNodes singleton has zero unit coverage	Test coverage	Moderate	in-diff
8	Grid scroll-restore fails on deep (releasing) scroll	Query & data integrity	Minor	in-diff
9	Non-autorefresh draw chart can stick on "can't classify SQL"	Async / data	Minor	in-diff
10	draw→grid switch drops the pending throttled chart frame	Persistence	Minor	in-diff
11	capResultBytes stringifies whole dataset every ≤10s snapshot	Performance	Minor	in-diff
12	echarts full re-init + animation replay on series-count growth	Performance	Minor	in-diff
13	computedLayout memo recomputes on every cell mutation	Performance	Minor	in-diff
14	Duplicated query-match predicate (drift risk)	Structure & types	Minor	in-diff
15	as unknown as Layout double-cast severs type-checking	Structure & types	Minor	in-diff
16	Shimmer geometry/perf nits (3 bundled)	Styling & theming	Minor	in-diff
17	No IntersectionObserver fallback	Browser compat	Minor (near-unreachable)	in-diff
18	persistCellSnapshot failure + duplicate-flush paths untested	Test coverage	Minor	in-diff

Detail

#1 — Failed snapshot load leaves a run cell shimmering forever, with no error or retry — Moderate

isExpectingResult (notebookUtils.ts:1112) excludes only "missing", so a "failed" hydration status keeps expectingResult=true → CellBottomContent.tsx:81-83 renders <GridShimmer> indefinitely; no component branches on "failed". cellResultHydration.ts:187-199 sets "failed" after MAX_LOAD_RETRIES=2 when loadCellSnapshot (Dexie .get, no catch) rejects.

• Repro: Run a cell so it persists a snapshot; leave it at rest on-screen in the retain band; make IndexedDB reads fail 3× within ~1.5s (corruption / forced connection close / storage fault). The cell shows an infinite loading skeleton — no error, no retry — and an at-rest cell gets no further band event to recover; only a reload helps (and won't if IDB stays down). Under a whole-DB fault every run cell shimmers at once.
• Fix: Treat "failed" like "missing" in isExpectingResult (collapse the reserved area), or add a "failed" branch in CellBottomContent with a "Couldn't load saved result — Retry" affordance calling resultHydration.request (which already re-serves "failed").

#2 — Wide (> NOTEBOOK_BYTE_CAP) results shrink to a truncated prefix merely by scrolling away and back — Moderate (needs product sign-off)

onSnapshotPersisted marks the uncapped in-memory array releasable (useCellExecution.ts:120-122), but persistCellSnapshot.ts:36 saves the byte-capped copy. After release + re-hydrate (cellResultHydration.ts:170-178) only the truncated prefix remains (with a "truncated" badge) until a re-run. The author documents this as deliberate (persistCellSnapshot.ts:19-26).

• Repro: Run a query whose result exceeds the byte cap (within the row cap); scroll it past the retain band so it releases; scroll back → fewer rows than before.
• Assessment: Real behavioral change vs. pre-PR (cells never released before). The badge means it isn't fully silent and a re-run recovers, but "scrolling reduces the rows you were viewing" deserves explicit product confirmation. Not a blocker on its own.

#3 — The new chart entry animation ignores prefers-reduced-motion, while the shimmer honors it — Moderate

ShimmerBar.tsx:48 stills its sweep under @media (prefers-reduced-motion: reduce); ChartRenderer.tsx / chartEntryAnimation.ts (both new) have no matchMedia guard, so the echarts entry animation plays on every notebook open with draw cells (and replays on series-count remounts — see #12).

• Fix: Force animationDuration: 0 in the animate-entry decision when matchMedia("(prefers-reduced-motion: reduce)").matches.

#4 — Keyboard focus is dropped when focus enters a released cell and the toolbar right-slot swaps on hydration — Moderate (narrow reachability)

A released cell has cell.result == null → view === "none" → header renders CellRunDrawToggles (Cell.tsx:428). Focusing it pins + hydrates asynchronously; when the snapshot lands, view flips none→grid and the slot swaps to CellViewToggle/CellWideActions (Cell.tsx:445-469), unmounting the focused Run/Draw button → focus resets to document.body.

• Repro: Shift+Tab up into an off-screen (released) cell; focus lands on a header button; ~tens of ms later hydration swaps the slot and focus is lost.
• Fix: Keep a stable right-slot control across none↔grid, or move focus to the replacement control on swap.

#5 — Find-in-page (Ctrl+F) can no longer locate SQL/result text in off-screen cells — Moderate (inherent virtualization tradeoff)

Placeholdered cells render shimmers, not real text, so the browser reports "not found" for text present in the buffer. This is the direct consequence of the virtualization #576 asked for — flagging so it's a conscious decision. Optional mitigation: render visually-hidden editor text + top-N rows inside the placeholder.

#6 / #7 — Two Moderate test-coverage gaps on the cross-context seam

• add web-console package #6: useAdaptivePoll was rewritten (loop extracted to runAdaptivePollLoop), but the hook itself is never renderHook-ed; the ~400 lines under "useAdaptivePoll core logic" test local reimplementations that would pass even if useAdaptivePoll.ts were deleted (pre-existing; the PR added only 2 real runAdaptivePollLoop tests). The effect wiring — abort-prior-loop on key change, currentInterval reset, unmount cleanup, enabled toggle — is untested, and the out-of-diff TableDetailsDrawer polls monitoring at 200 ms through it. Fix: renderHook test with fake timers covering those transitions.
• clean web console #7: mountedCellNodes.ts (module-level MutationObserver+RAF singleton) feeds the entire band pipeline yet has no unit test. A wrong mayChangeCellSet skip would silently stop detecting cell add/remove. Fix: unit test with faked observers covering diff output, container switch, refcount start/stop, late-join.

#8 — Grid scroll position is not restored on a deep (releasing) scroll — the PR's own last commit only half-works — Minor

resultGridViewportStore keys saved viewports on runToken = result.timestamp. A live run stamps timestamp = T1; the snapshot saves savedAt = T2; re-hydration restores timestamp: snapshot.savedAt (cellResultHydration.ts:175), so after a release the token changes T1→T2, replaceResult clears the saved viewport, and load misses → scroll lost. A shallow scroll (placeholder swap, result retained) keeps T1 and restores correctly. Same root cause changes the "ran at" time shown in StatusNotification from run-time to save-time.

• Fix: Preserve the original run timestamp through the snapshot (keep timestamp as the stable run token distinct from savedAt), so the viewport token survives release.

#9 — A non-auto-refresh draw chart can get stuck on "can't classify SQL" — Minor

If validateWithGlobals throws (network blip) during a fetch, chartRefreshEngine.ts:575-583 sets classifyBlock:{kind:"failed"} with settledKey=queriesKey; ensureData then early-returns without fetching (:426-429) and applySql never clears classifyBlock. Auto-refresh cells self-heal next tick, but an autoRefresh===false cell has no poll and stays on the error until manual refresh.

• Fix: Clear classifyBlock in applySql, or re-attempt classification on reveal.

#10 — draw→grid mode switch drops the pending throttled chart frame — Minor

removeEntry re-queues the pending snapshot only for reason==="teardown" (chartRefreshEngine.ts:366-371); modeExited drops it, so a reload shows a frame up to SNAPSHOT_THROTTLE_MS (10s) older. Recoverable by re-run.

• Fix: Also re-queue for reason==="modeExited" (same rows).

#11 — capResultBytes JSON-stringifies the whole dataset every snapshot just to measure size — Minor

notebookUtils.ts:243 stringifies unconditionally before the size check; the chart engine now saves every ≤10s per visible live draw cell. Several live charts → periodic multi-ms main-thread bursts, all discarded when under the cap.

• Fix: Cheap size estimate (rows × sampled-row width) as a fast-path guard.

#12 — echarts fully re-inits (and replays the entry animation) when a live partitioned chart's series count grows — Minor

structuralKey includes seriesTypes.join("|"), so a new partition value changes the key and remounts <ReactECharts>. Nuance from verification: the key/remount is pre-existing (not a regression), and the PR actually adds zoom restoration (handleChartReady re-applies zoomWindowRef), so "loses dataZoom" is a false positive. The one new effect is the entry-animation replay on those remounts (firstInstanceDoneRef is already true → suppressEntryAnimation is false).

• Fix: Exclude pure series-count growth from the key, or setOption(opt, { replaceMerge: ['series'] }).

#13–#17 — Minor structure / perf / styling / robustness

• web console automate npm publish #13 computedLayout memo (index.tsx:420-460) recomputes on every cells identity change; bounded (≤200) and layoutKey guards rgl re-render, so low impact.
• release(web-console): 0.0.2 (test PR does not actually publish version) #14 resultMatchesQueries (drawCanvasUtils.ts:67) and snapshotResultsMatchQueries (notebookUtils.ts:690) independently implement the same normalize-and-compare core, then diverge — drift risk. Fix: share the per-index comparison.
• DO NOT MERGE: test publish script #15 currentLayout as unknown as Layout (index.tsx:464) severs type-checking at the rgl boundary. Fix: single as Layout or align the computed element type.
• release(web-console): 0.0.2 #16 Three shimmer nits: GridShimmer hand-copies unexported reserved-geometry constants from notebookUtils (drift → silhouette mismatch); the generic no-result shimmer omits the tab strip so released multi-query cells flash ~40px blank on rehydrate; EditorShimmer interpolates per-line px width into styled-components (SC class-cache growth). Fix: export the constants, reserve tab-strip space, use style={{ width }}.
• docs(web-console): add changelog #17 useCellBandObservers early-returns with no IntersectionObserver, leaving the notebook all-placeholder — but the browser baseline requires IO for Monaco anyway, so effectively unreachable. Borderline drop; kept as a defensive note.

#18 — Minor coverage

persistCellSnapshot's failure→false contract (the release-gate guard) is only exercised indirectly; the flush-pending-frame-before-duplicate path isn't asserted.

---

False-positives

Findings raised during the review that verification dismissed:

• Perf — "chart loses the dataZoom window on series-count change." The PR adds zoomWindow + handleChartReady re-dispatch, which restores zoom across the remount. Only the animation replay (update readmes #12) survives.
• State/architecture — "unmemoized context provider values re-render all consumers." All five Notebook context values are stable: stateValue (useMemo), the action proxy (useMemo [] over a live ref), and the three engine instances (useMemo [bufferId] / []). Verified in NotebookProvider.tsx.
• Lifecycle — "engines capture a stale bufferId on buffer switch." <Notebook> is keyed by activeBuffer.id (src/scenes/Editor/index.tsx:492), so the whole subtree — provider + engines — remounts per buffer; bufferId never changes in place.
• Persistence — "new snapshot fields need a Dexie migration." db.ts is unchanged; notebook_results (v9) pre-exists; activeResultIndex/script are optional and every reader defaults them.
• Cross-context — "released cell.result (transient undefined) breaks AI/headless/controller readers." The persisted buffer already stored result as undefined (carrying lastRunStatus), so background readers never depended on it; getCellRunStatus and the run-outcome paths guard correctly (every callsite walked).
• Async — "ChartRefreshEngine.destroy omits listeners.clear() → leak." Child consumers unsubscribe before parent teardown and no notify fires after entries empty; harmless.

---

Summary

Verdict: approve in principle — no blockers found in the critical path; address the confirmed Moderates before marking the draft ready. This is a high-quality, unusually well-defended refactor: the release/re-hydrate data-loss surface, async cancellation, React hook correctness, and the cross-context blast radius are all clean, and the quality gate passes fully.

• No regressions in data integrity. The one durability tradeoff (init react components package #2, wide-result truncation on release) is deliberate and badge-visible but warrants an explicit product decision. The genuine bugs are a UX dead-end under IndexedDB fault (wip test for error range #1), a partial failure of the PR's own scroll-restore feature (run web console #8), and a11y gaps (disabled buttons #3, add test for query run when cursor position is next to ending semicolon #4, Console autocomplete tests #5).
• Verification tally: 22 draft findings verified/kept; 6 dropped as false positives (1 agent-produced + 5 architectural claims pre-cleared in the surface map). The four correctness-focused agents (query/data, async, React, cross-context) produced zero findings.
• In-diff vs out-of-diff: all 22 confirmed findings are in-diff. Zero out-of-diff is a genuine result, not an underrun: the cross-context pass walked every exported-symbol callsite (useAdaptivePoll→TableDetailsDrawer, the notebookResults store consumers, all cell.result readers, the changed types) with explicit SAFE verdicts, and the biggest contract change (results becoming transiently undefined) is safe precisely because the persisted-buffer contract already carried result: undefined.
• Draft status: As a WIP draft the PR is expected to keep iterating; the title also undersells the change (it's virtualization + hydration, not just chart-refresh relocation) and the description is a bare "WIP to close Notebooks: virtualize cell rendering (and/or infinite scroll) to cut memory and network usage #576" — worth expanding to end-user impact (faster notebooks, lower memory/network) before it's marked ready.