Epic: signal viewer workbench (phases 1-5) - #270
Merged
Conversation
* Recognize .mefd/.ds dirs as viewer recordings Directory-keyed recordings (MEF3 .mefd, CTF .ds) render as hybrid rows: the name is the same eeg preview button a signal file gets, the chevron still expands member files. Extensionless directory recordings (4D/BTi) are upgraded at zarr-annotate time from the index's store paths. Dir recordings get a browse fallback instead of a download link. Tested: bun run test (1085 passing, new signalDirExt + renderer cases), typecheck, lint, build all green. * Address review: sibling toggle, BTi failures, copy Review findings on PR #257, all addressed: - Hybrid dir rows no longer nest the viewer button in <summary> (inconsistent AT exposure); expand toggle and viewer button are sibling <button>s, lazy expansion driven by a data-dir-toggle delegate sharing loadDirChildren with the details path. - Dir-recording fallback no longer links to raw listing JSON; copy points at the row's expand arrow. - Extensionless (BTi) upgrades also fire for index failures so a known-unconvertible recording shows its reason. - upgradeDirRecordingRow warns on structural-assumption breaks. - og-image.png build artifact dropped from the diff; chunk-reveal path covered by a test. Tested: 1087 vitest, typecheck, lint, build green.
* eeg-viewer: thread AbortSignal, export level readers 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). * viewer: capped background preload of recordings 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). * prefetch: batch progress/yield over a warm cache restart 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). * store/viewer: abort-safe retries, level-checked cache hits 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).
* Add BIDS recording navigation helpers Pure module behind website#253: parse sub/ses/task/acq/run/recording entities out of zarr index store paths (files and .mefd/.ds/BTi directories alike), order the list three ways for the prev/next control, enumerate subjects and tasks for the dropdowns, and resolve a (subject, task) pick to a recording. Paths that parse to nothing keep file order instead of scrambling. Also holds the persisted iteration order, read through an injected Storage so it is testable. Tested: 51 new vitest cases (1127 total), typecheck, lint, build green. * Add viewer nav-order setting and transfer The gear popover gains one section, "Next moves through", persisting the prev/next iteration order and announcing changes on a bubbling event so the dialog chrome can relabel its controls without polling storage. `transfer`/`onTransfer` let a caller carry window length, manual gain, channel zoom, filters and the display toggles into the next recording. Window position, bad channels and group index deliberately do not travel; the notch re-defaults from the new recording's PowerLineFrequency unless the user set it themselves. Tested: typecheck, lint, build, full vitest suite green. * Navigate recordings from the viewer dialog Subject and task dropdowns plus prev/next in the enlarged viewer's header row swap the mounted recording in place, reusing the eegSeq supersession the tree-click path uses so rapid navigation cannot strand a mount or a WebGL context (website#208). Settings carry over; the window resets to the start of the new recording. Neighbours are warmed with the same prefetch a hovered tree row gets. The first navigation collapses the inline row the viewer came from and the dialog owns the instance from then on -- the target recording often sits in a directory the tree never rendered, so re-anchoring is not available. The inline panel gains no controls. Tested: typecheck, lint, build, full vitest suite green. * Record ADR 0012 on viewer row detachment Why navigating inside the dialog collapses the originating inline row instead of re-anchoring under the new recording, and why the alternatives lost. * Keep focus when a step button goes disabled Stepping onto the first or last recording disabled the button under the user's focus, dropping focus to the body; hand it to the opposite button. Tested: typecheck and lint green. * Simplify the selectRecording relaxation Same behaviour, expressed as two candidate lookups picked by `prefer` instead of a key-dispatched find. Tested: full vitest suite, typecheck and lint green. * Walk subjects before sessions in subjects order "Subjects first" promises the subject moves fastest, but sorting ses before sub walked one subject's sessions before reaching the next subject, which is what runs order already does. Also pins what happens to recordings that tie on every tracked entity (BIDS split files): they keep source order, a dropdown pick lands on the first, prev/next reaches the rest. Review findings 3a and 3b on PR #258. Tested: 8 new vitest cases, full suite green. * Offer the right fallback for directory recordings renderUnavailable always linked "Download the file", which for a .mefd/.ds/BTi recording (website#252) resolves to raw directory listing JSON. Thread `dirRecording` through ViewerOptions from both mount sites and reuse the page's own wording. The nav path has no tree row to read data-dir-recording from, so `isDirRecordingName` decides from the name. Also stops the gear from showing a filter that never runs: a cutoff at or above the new recording's Nyquist, whether transferred or taken from the declared PowerLineFrequency, now reads as off. Review findings 4 and 5 on PR #258. Tested: 3 new vitest cases, full suite, typecheck, lint green. * Explain a failed navigation instead of going blank A throw outside mountEegViewer's own try/catch (DOM build, WebGL setup, transfer) left the dialog naming the target while the host held a half-built viewer and no message: the previous instance is torn down as the mount's first act, so there is nothing to fall back to. Render the failure with the same fallback the inline path offers, and put the title and dropdowns back on the recording the user came from. Prev/next also now update aria-label alongside title, so a screen reader hears which recording is next rather than the static markup label. Review findings 1 and 2 on PR #258. Tested: typecheck, lint, full suite green.
* Add firstRecording nav helper for the View data button Picks the first recording in a given iteration order, reusing orderedRecordings. Backs website#260 (dataset page "View data" button), which needs to resolve a starting recording without a tree click. * Add hidden View data button to the dataset action bar SSR hidden next to Download; the dataset page's hydration script reveals it once the Zarr index resolves with at least one store (website#260). * Wire View data button to open the enlarge dialog directly Clicking opens the first recording in the user's current nav order straight into the enlarge dialog, detached from birth (ADR 0012): no originating tree row, so closing the dialog destroys the instance. Reuses the existing eegLive/eegSeq machinery, mountEegViewer, and the recording-nav dropdowns/prev-next rather than duplicating them. A mount failure renders an explicit error in the dialog, mirroring navigateEegViewer's failure state. eegLive.btn/slot become nullable to represent a viewer with no originating row; resetEegRow and the dialog close handler are updated to treat null as a no-op, matching behavior for every existing row-opened viewer exactly. * Fix View data dialog aria-busy, prefetch, and aria-label gaps Review follow-ups on the View data button (website#260): - openRecordingInDialog never toggled aria-busy on its host, unlike openEegViewerInline and navigateEegViewer. Set it right after the host is inserted and clear it in a finally, mirroring navigateEegViewer. - The target recording's own store metadata was never warmed before the mount -- only its neighbours were, via prefetchAdjacentRecordings. Warm it the same way prefetchZarrButton does for a tree row. - setEegDialogTitle only ever updated the visible heading; the dialog's own aria-label stayed the static "Signal viewer" from EegViewerDialog.astro, so a screen reader never heard which recording was open. This was a pre-existing gap across all three entry points (inline enlarge, in-dialog navigation, and now View data); fixed here since all three funnel through this one setter. * Add firstRecording coverage for the subjects nav order Only "runs" and "file" were covered for firstRecording; add the missing "subjects" case.
* Add HED vocabulary extraction script Parses base HED 8.4.0 and the HED-SCORE 2.1.0 library schema from a local hed-schemas checkout into a committed JSON bundle (488 tags, 138 KB): the whole SCORE clinical EEG vocabulary plus curated base-HED subtrees for artifacts, event categories, temporal markers and vigilance state. SCORE paths carry the `sc:` namespace prefix so an exported HED string is usable as-is with `"HEDVersion": ["8.4.0", "sc:score_2.1.0"]`. Hand-rolled hedxml scanner rather than an XML dependency; the script header documents how to regenerate. Biome skips the generated bundle. Tested: script runs clean, resolves all 26 quick picks, rejects duplicates. * Add HED vocabulary loading and fuzzy search `loadHedVocab` is the only reference to the JSON bundle, so Vite splits it into its own chunk and the viewer's initial bundle never carries it; the fetch happens when annotation mode is first entered. Search is hand-rolled scoring over the 488-entry list: exact, prefix, hyphen word-boundary, substring, path, description, shortest-tag tiebreak. Multi-word queries are a conjunction scored by their weakest term. Tested: 28 vitest cases against the real bundle, including chunk-size and prefix invariants. * Add annotation model with BIDS TSV export Two disjoint kinds: TimeAnnotation (an events.tsv row, duration 0 for a point marker) and ChannelAnnotation (a channels.tsv row with status and status_description). Pure module - construction with clamping, sorting, upsert/remove, windowing, overlap detection, greedy lane assignment for drawing overlapping spans, and both serializers. Output is deterministic: total sort order, 0.1 ms rounding without float noise, n/a for missing, tabs and newlines stripped from free text at the point it enters the model. Tested: 64 vitest cases, including byte-identical output for a set built in a different order. * Add guarded IndexedDB annotation storage Keyed by dataset id, dataset version and recording path (a JSON tuple, so no separator a BIDS path could contain). Not keyed by user: the issue requires persistence regardless of sign-in. Every IndexedDB call is guarded. A synchronous throw on open (Firefox private windows), an absent factory, or a later quota failure degrades to an in-memory store for the session with `persistent` false, which the viewer surfaces as "download these". Records read back are re-validated through the model's constructors rather than trusted. Tested: 22 vitest cases; the no-IndexedDB fallback is the real path under node, not a simulated one. * Add annotation mode to the signal viewer A pencil toggle beside the topomap arms the tool. On the trace, a click drops a zero-duration event marker and a horizontal drag makes a span, both opening a focus-trapped popover with fuzzy HED search, grouped quick picks and a free-text note. In the gutter, channel selection stays the viewer's existing bad-channel click, which the panel then annotates as a set; the two exports are separate downloads. The layer is self-contained: an overlay canvas above the chrome canvas, a popover and a panel, reading geometry the renderer already computed. It never owns the arrow keys, and pointer handlers are capture-phase on the scope so gutter clicks still reach the existing handler. Annotations restore on mount and flush on teardown, so a recording swap or an enlarge round trip keeps them. Anonymous or unpersisted work arms a beforeunload confirm and an in-panel notice. Tested: driven end to end in Chromium against the dev server on on005514, both themes, inline and enlarged - marker, drag, edit, delete, search, quick picks, channel marks, both downloads, restore across a remount. * Add ADR for the annotation data model Records why annotations are two BIDS kinds rather than one type with optional fields, why storage is local-only in v1, and what that forecloses (channel-scoped time ranges, cross-device access, sharing). * Close the annotation store on an early teardown A mount can be destroyed while openAnnotationStore is still in flight — a recording swap, or the enlarge dialog closing. The resolved connection was then unreachable: destroy() flushes through `store`, which is still null, so the IDBDatabase stayed open for the life of the page. Close it at the point the layer learns it is gone. * Never write a negative zero into an annotation TSV formatSeconds rounds before printing, and a value just below zero rounds to -0, which stringifies as the cell "-0" — a value no reader of an onset column expects. Return "0" for a rounded zero of either sign. * Name the channel export for merging, not replacing The channel annotations downloaded as <stem>_channels.tsv, which is the name of the recording's real channels table while being deliberately not one: it lists only the annotated channels and omits the type/units columns BIDS requires. Dropped into a dataset under that name it would delete every unannotated channel's row. Rename it <stem>_channels-annotations.tsv. * Place the annotation popover beside what it annotates Three changes from live design feedback, plus two fixes in the same code. Placement. The popover opened at a fixed offset under the toolbar, which put it on top of the very span the annotator had just dragged out. It is now placed adjacent to its selection: right of it, else left, else below, else above, else — only when nothing clears it — at the right-centre of the trace. Coordinates are relative to the viewer root, so the same arithmetic serves the inline and the enlarged mount. Bounds are the *visible* box (root box intersected with the viewport and any clipping ancestor), because inside the enlarge dialog the viewer root is taller than the dialog that clips it, and placing against the root put the footer below the dialog's edge. It follows the selection across a frame (scrubbing, channel zoom) and a window resize. Pinned footer. The popover no longer scrolls as a whole: its body does, and Save/Cancel sit in a footer that cannot leave the frame however long the vocabulary list is. Enter now saves, once the draft carries a tag or a note, except where a control owns Enter itself (a textarea's newline, a button's activation) or an IME is mid-composition. Escape still cancels. Channel entry. With annotate mode on, clicking a channel label opens the channel popover for that channel directly rather than only toggling the bad mark — the popover's status field decides that mark on save. Its quick picks are the artifact vocabulary (base HED's artifact tree plus SCORE's terms for what the artifact cost the recording), derived from the bundle rather than curated a second time. Outside annotate mode the marking gesture is unchanged. Escape and the enlarge dialog. Whether a cancelled keydown suppresses a modal dialog's own close request is engine-dependent, so one Escape could have closed the popover and ejected the enlarged viewer behind it. The layer now guards the surrounding dialog's `cancel` while its popover is open (and briefly after, since the close request can arrive once it has closed), and exposes isPopoverOpen for the same question asked from outside. Overlay caching. The lane assignment and the channel index were rebuilt on every drawOverlay, which runs on every pointer move of a drag. Both are now cached on the set's identity, the visible-window filter is shared with the hit test, and drag repaints are coalesced to one per frame. * Cover the annotation layer's helpers and its storage The placement arithmetic decides whether the popover covers the signal being annotated, which a screenshot review catches unreliably; it and the other pure helpers are exported precisely so they can be driven without a browser. 26 tests over placement (each candidate, the clamps, bounds that do not start at the viewer's origin), the time/pixel mapping, Enter's submit rule, the unsaved-work predicate and the colour blend. Storage gains a real IndexedDB. fake-indexeddb is a platform shim rather than a mock of anything here — the actual W3C algorithms under Node, as jsdom is a real DOM — so nothing in annotation-store.ts is stubbed: the tests drive the genuine open-and-upgrade, a round trip through a second connection, key separation, and the degrade-to-memory path through a genuine write failure. Until now only the no-IndexedDB fallback was covered, because Node has none.
A mouse click on a quick-pick chip or search result left focus on that button, where Enter re-activates the button (ENTER_OWNING_TAGS defers to it for keyboard users) instead of saving - so the natural mouse flow drag -> click tag -> Return never saved. Mouse activations (detail > 0) now hand focus back to the search input; keyboard activations (detail 0) keep focus on the chip. Tested: typecheck, lint, 1347 vitest; reported by Yahya in live QA.
Three live-QA fixes from Yahya: - 'Annotate 1 marked channel' now opens the existing annotation prefilled (tags/status/comment) instead of a blank draft, mirroring the gutter click's edit-not-replace behavior; multi-channel sets stay a fresh draft describing the set. - Popover footer pills no longer wrap mid-word into circles (white-space nowrap + flex-shrink 0). - Annotation list scroll cap raised 132px -> 260px for long scoring sessions. Tested: typecheck, lint, 1347 vitest.
Yahya's QA round 4: - New pure hedShortForm(): leaf tag with the sc: library prefix carried over. formatHed() now writes short form to events.tsv/channels TSV (HED's annotation default; validators expand via HEDVersion). - Panel rows and chips show short form even before the lazy vocab loads (labelForPath falls back to hedShortForm, not the raw path — how long paths leaked into the UI); the long form moves to hover titles on rows and chosen chips. - Annotation list becomes a responsive grid: 2-3 columns of short rows on a wide panel, collapsing to one, row-major. Tested: typecheck, lint, 1350 vitest (serializer expectations updated to short form; new hedShortForm cases).
Yahya's QA round 5: the search box only reached a curated 488-tag subset, so Building/Left/Right found nothing and 'sleep' missed tags. The bundle now carries every non-deprecated tag of both schemas (1525 tags: 1129 base HED + 396 SCORE; 341 KB raw, 68 KB gzipped, still popover-lazy). Curation now lives only in the quick-picks. Active searches hide the quick-pick groups so results are the only list on screen; clearing the box restores them. Tested: typecheck, lint, build, 1353 vitest (new whole-schema coverage cases; size cap raised to 500 KB with rationale).
This was referenced Sep 1, 2026
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements epic #256 — the viewer goes from preview to workbench. Every phase was implemented in its own worktree, independently reviewed by a Sonnet agent, had all findings fixed, and merged with green CI; five live-QA rounds from Yahya followed on the annotation tool.
Phases
.mefd/.dshybrid rows (sibling toggle + viewer buttons for AT safety), extensionless (BTi) recordings upgraded from the zarr index, honest browse fallback. Implements Viewer: directory-format recordings (.mefd/.ds/BTi) never get a viewer row #252.recording-navmodule, settings transfer across recordings, "Next moves through" gear setting, ADR 0012. Implements Viewer: navigate between recordings (next run/task/subject) with entity dropdowns #253.Live-QA rounds (merged after review)
#266 Return-to-save focus, #267 bulk-edit prefill + footer pills + list height, #268 HED short form with long-form hover + multi-column list, #269 whole-schema vocabulary search (1,525 tags, 68 KB gzipped, popover-lazy).
Test plan
Follow-up in flight: a transfer-efficiency audit (reader-side optimizations) will arrive as its own PR to staging.