Phase 3: capped background preload of recordings - #259
Conversation
readWindow/readLevel0/readViewLevel take an optional AbortSignal forwarded to zarr.get, and readLevel0/readViewLevel are now exported so a caller can target a specific pyramid level directly instead of readWindow's pixel-width heuristic. Both are additive (default undefined signal, new exports) with no behavior change to existing call sites. Prep for the website#254 background preloader, which needs to read an explicit level and abort cleanly on teardown. Also adds windowDataBytes, a pure byte-accounting helper for WindowData. Tested: bun run test (1109 passed), bun run typecheck (0 errors).
Adds an optional "Preload full recording" gear toggle (off by default) for clinicians who want the whole recording resident so paging/scrubbing never waits on a fetch. src/lib/eeg-viewer/prefetch.ts is a pure module with no zarr/DOM dependency: - ByteCappedLRUCache<T>: byte-accounted LRU with two insert modes. put() (interactive path) evicts LRU entries to fit; putIfRoom() (background scheduler) never evicts, failing once full so the walk stops rather than displacing what the viewport is showing. - outwardOrder/segmentIndexForTime: pure helpers for the segment grid. - PrefetchController<T>: walks the recording outward from the playhead one segment at a time, yielding to interactive reads (depth-counted) and to document.hidden, aborting cleanly via AbortController on stop(). viewer.ts wires it up: - Segments are windowLengthS-wide reads at whatever pyramid level the interactive path last actually rendered (lastWinLevel), fetched via the newly-exported store.ts readLevel0/readViewLevel rather than re-deriving readWindow's pixel-width heuristic for off-screen windows. - The interactive read path opportunistically serves an exact grid-aligned window from the cache (Page back/forward, Home/End land on the grid; an arbitrary scrub position falls through to a normal network read, same as preload off). - Both settings (enabled, cache cap: 250/500/1000 MB) persist in localStorage under nemar:eeg-preload / nemar:eeg-preload-cap-mb, guarded with try/catch. - The minimap gets a thin buffered-region strip (like a video player's buffer bar) shading cached segments. - Preload pauses on visibilitychange (tab hidden) and stop()s on viewer destroy, matching the abort discipline from website#208. Tested: bun run test (1142 passed, 33 new in prefetch.test.ts covering LRU eviction/cap enforcement, outward scheduling order, priority yielding, hidden-pause, abort, and stop-when-full), bun run typecheck (0 errors), bun run lint (clean), bun run build (clean; the one build failure seen locally was api.nemar.org intermittently 500ing on an unrelated catalog page during OG image generation, confirmed by direct curl, not this change).
A restart over an already-warm cache (cache-cap change, or switching back to a previously-visited group/level) hit cache.has(key) for every segment with no await in between: the while(this.yielding) guard is a no-op when nothing is contending, so a multi-hour recording's worth of segments could confirm in one synchronous microtask, firing onProgress (a canvas redraw downstream) on every single one -- a real main-thread freeze rather than a fast no-op. The cache-hit branch now batches: it counts confirmed hits and only flushes progress + awaits idle() every PROGRESS_BATCH_SIZE (32) segments, plus a final partial flush at the end of the walk and before every early-return (halted-full, superseded). The fetch path is unchanged -- a real network call plus the existing idle() await already yields every iteration, so it never needed batching. Tested: bun run test (new case asserts idleCalls > 5 and far fewer onProgress calls than segments across a 300-segment fully-cached restart, confirming batched yielding rather than one synchronous burst).
Three fixes from review: - retryingFetch no longer retries an already-aborted request. A caller's AbortController firing (background preloader stop()/destroy()) was previously treated as a transient failure and retried through up to ~10s of backoff, keeping the controller and up to the full cache cap alive well past teardown for a response nothing would use. Both the resolved-transient-status branch and the catch block now check request.signal?.aborted and propagate immediately instead. - The interactive opportunistic cache-hit path (website#254) trusted lastWinLevel -- the level the *previous* frame rendered -- without re-deriving it for the CURRENT plotWidth. A resize with no remount (e.g. the Enlarge dialog) could serve a stale-level cached window and then reassign lastWinLevel back to it, self-reinforcing. Added store.ts#chooseWindowLevel, a pure function mirroring readWindow's own level-selection heuristic without fetching, and the cache check now only serves a hit when it agrees with lastWinLevel; otherwise it falls through to a real read (which corrects lastWinLevel for next time). Confirmed the filtered/forceLevel0 path was already structurally isolated from the cache (the check lives only inside the `!hasFilters(filters)` branch). - windowDataBytes had zero test coverage despite being the byte-accounting foundation for the whole cache. Added line/band/mixed/empty cases. Also exported retryingFetch and added coverage for the abort-vs-transient distinction, and added chooseWindowLevel coverage (natural pick, forceLevel0 override, the LEVEL0_MAX_SAMPLES boundary, and no-pyramid fallback). Tested: bun run test (1122 passed, all new cases green), bun run typecheck (0 errors), bun run lint (clean), bun run build (clean).
|
Addressed all four findings from review, in commits 5d8880d and 621fb74. 1. CRITICAL — synchronous burst over a warm cache (prefetch.ts loop). Fixed. 2. IMPORTANT — retryingFetch retrying an aborted request (store.ts). Fixed. 3. IMPORTANT — stale-level cache hit after a resize (viewer.ts). Fixed. 4. IMPORTANT — windowDataBytes had zero coverage (store.ts). Fixed. All four gates green on the pushed tip ( |
ceedd33
into
feature/issue-256-epic-viewer-workbench
1 participant
Summary
Part of epic #256. Implements #254.
Adds an optional, off-by-default "Preload full recording" gear toggle. When
enabled, a low-priority background scheduler walks the recording outward
from the current playhead at whatever pyramid level the interactive path is
currently rendering, filling a byte-capped in-memory LRU cache so paging and
scrubbing elsewhere in the recording become instant once the walk has
reached that region — the clinician scenario in the issue (reviewing a whole
recording, wanting paging/scrubbing to never wait on a fetch).
src/lib/eeg-viewer/prefetch.ts(new, pure module — no zarr/DOMdependency):
ByteCappedLRUCache<T>: byte-accounted LRU with two insert modes.put()(would-be interactive use) evicts LRU entries to fit.putIfRoom()(the background scheduler) never evicts — it fails once the cache is
full so the walk stops rather than evicting what the viewport is
showing, per the issue's memory-cap requirement.
outwardOrder/segmentIndexForTime: pure scheduling helpers.PrefetchController<T>: walks the recording's segment grid outward fromthe playhead, yields to interactive reads (depth-counted, so a fast
scrub holds it the whole time) and to
document.hidden, and abortscleanly via
AbortControlleronstop().src/lib/eeg-viewer/store.ts: additive-only.readWindowtakes anoptional trailing
AbortSignalforwarded tozarr.get;readLevel0andreadViewLevelare now exported (previously private) so the preloader cantarget an explicit level directly instead of re-deriving
readWindow'spixel-width heuristic for a window that isn't on screen. No existing call
site changes behavior. Also adds
windowDataBytes, a pure byte-accountinghelper.
src/lib/eeg-viewer/viewer.ts:windowLengthS-wide reads atlastWinLevel(the level theinteractive path actually rendered last), fetched via the newly-exported
level readers.
grid-aligned window from the cache before falling back to a network
read — Page back/forward and Home/End move in exact multiples of the
window length, so they hit the grid; an arbitrary scrub position
usually doesn't, and falls through to a normal read, identical to
preload being off. See "Known limitations" below.
coordination note below) with the toggle and a 250 MB / 500 MB / 1 GB
cache-limit select. Both persist in
localStorageundernemar:eeg-preload/nemar:eeg-preload-cap-mb, every read/writeguarded with try/catch.
overview minimap (like a video player's buffer bar), shading segments
the walk has already cached. This was cheap with the current markup
(the minimap already redraws every render and on group switch), so it's
included rather than skipped.
visibilitychange(tab hidden) and the controller'sstop()is wired into the viewer's existingdestroy()cleanup path —same abort discipline eeg-viewer: superseded and closed mounts leave zarr fetches in flight #208 asks for, applied to this mount's own
background reads (network-level abort for the interactive path is
still open in eeg-viewer: superseded and closed mounts leave zarr fetches in flight #208 itself; this PR doesn't touch that).
Design decisions / known limitations
the
zarr.FetchStoreitself (so any overlapping interactive read gets abyte-level cache hit regardless of alignment) would need the store's
generic type threaded through
store.tsmore broadly than the issue's"minimal changes to store.ts" allows, and
zarr.FetchStorehas privatefields that prevent a duck-typed substitute from type-checking. Caching
decoded
WindowDataper fixed-width segment is the smaller, additivechange; the tradeoff is that only exact grid-aligned interactive reads
are cache hits (see above) rather than every overlapping read.
cache on the interactive path — its span is shorter than
windowLengthSby construction, which the exact-match check treats as"not a grid window." Correctness is unaffected (it just falls through to
a normal read); a future refinement could special-case it.
grouped("Preload", ...)block appendedlast, to stay out of the way of Viewer: navigate between recordings (next run/task/subject) with entity dropdowns #253's concurrent gear-menu work.
Test plan
bun run test— 1142 passed (33 new, all inprefetch.test.ts):LRU eviction order/recency, cap enforcement (
putvsputIfRoom),setCapacitygrow/shrink, outward scheduling order and edge clamping,exact-grid
segmentIndexForTime, priority yielding (depth-counted),hidden-pause, clean abort via
stop(), stop-when-full, and asuperseded-retarget race (stale in-flight fetch discarded). Uses a
fake transport as the network boundary (real-shape byte-accounted
payloads), not a mock of business logic.
bun run typecheck— 0 errors.bun run lint— clean.bun run build— clean. One transient failure seen locally wasapi.nemar.orgintermittently 500ing on an unrelated catalogpagination offset during OG-image generation (confirmed independently
via direct
curlagainst production, unrelated to this branch); aretry built clean.
done in this pass (no interactive browser available in this environment);
the pure scheduling/cache logic is covered by the unit tests above, and
the store.ts/viewer.ts wiring is additive and type-checked. Recommend a
/design-review-style pass against test.nemar.org after this lands onstaging, specifically: toggling preload on a long recording, watching the
buffered strip grow, confirming Page back/forward feels instant once
buffered, and confirming the tab-hidden pause (Network tab quiets down
when backgrounded).
Coordination
Per the task brief: #253 (recording navigation) is also adding a gear-menu
item concurrently. This PR's menu addition is a single appended group, so a
rebase should be low-risk, but flagging in case of a conflict on the
menuconstruction block in
viewer.ts.