Release hardening from the v0.2.5 panel

[neuromechanist]

Fixes from the four-agent review panel on release PR #272. Behavioural findings
first, one commit each; the documentation corrections are collected in a single
commit at the end.

This should merge before #272. It targets staging, so once it lands the
release PR picks it up automatically as part of the staging -> main
promotion; no second release PR is needed.

Findings and disposition

#	Severity	Finding	Disposition
1	Critical	GroupHandle.viewLevelsDegraded was computed in store.ts and read nowhere, so a view pyramid truncated by a connection failure was indistinguishable from a genuinely short recording	Fixed
2	Critical	An annotation persistence degrade never re-armed the beforeunload guard or repainted the "not being saved" banner	Fixed
3	High	PrefetchController.loop retried into an outage forever with only a static buffered bar as feedback	Fixed
4	Important	isPopoverOpen() had zero callers; in-dialog navigation silently discarded an unsaved annotation draft	Fixed
5	Medium	applyTransfer carried a manually-set gain across a recording swap with no modality check	Fixed
6	Low	Dialog close during an in-flight navigate mount had no disposer to call	Fixed
7	Low	The annotation export download() could fail silently	Fixed
8	Low	writeNavOrder's boolean return was discarded	Fixed
9-16	Docs	Stale comments, wrong bundle numbers, a missing sync comment, an unused local	Fixed

1. Degraded view pyramid is now visible (viewer.ts, BidsTree.astro)

The viewer's status line carries a · overview incomplete (connection problem)
suffix, and a standing note sits under the overview minimap saying it is a
connection problem rather than a short recording. Both are re-evaluated when
viewLevelsReady settles, so a group that degrades after first paint updates
without waiting for the reader's next interaction. The status line is now
composed from a held base string precisely so that late re-application is not a
string patch.

2. Persistence degrade is pushed, not polled (annotation-store.ts, annotation-ui.ts)

mutate() runs syncAll() synchronously and only then schedules the debounced
write, so a QuotaExceeded on the last write before a tab close was never
announced. AnnotationStore gains onPersistenceChange(listener), fired from
degrade(); the annotation layer subscribes before its first read and re-runs
syncBeforeUnload() + renderPanel(). A late subscriber is told immediately,
and a listener that throws does not stop the others.

Tested against fake-indexeddb (a platform shim, not a mock): open, force a
failure by closing the connection underneath, assert the callback fires exactly
once and that shouldWarnBeforeUnload's answer flips from false to true for a
signed-in annotator. Covered on both the save() and the load() path — the
latter is the realistic first contact, since the layer loads before it ever
saves.

3. Preloader circuit breaker (prefetch.ts, viewer.ts)

Four consecutive segment failures stop the walk and fire a new onStalled
callback. Four is documented in the constant's own comment: each of those
failures has already been through store.ts's retryingFetch (seven attempts
with backoff), so four in a row is the backend being down, while two would
abandon a recording over an unlucky cluster and ten would just be more retry
traffic. Any success resets the counter; an AbortError is a cancellation and
does not count. The viewer shows "Preload paused — network trouble" beside the
preload toggle, and any restart (toggle, cache cap, group switch) re-arms the
walk and clears the note.

Six new pure-logic tests in prefetch.test.ts cover N-1-then-success, an
alternating connection, N failures stopping with onStalled fired once,
coverage surviving the trip, aborts not counting, and start() re-arming.

4. Navigation refuses to discard an open draft (annotation-ui.ts, viewer.ts, [id].astro)

mountEegViewer gains an onAnnotations option that hands the page a
ViewerAnnotationHandle — the same seam onTransfer already uses. The layer
gains focusPopover(), so the DOM stays owned by the layer rather than reached
into from the page. navigateEegViewer, stepEegViewer and both the subject
and task select handlers refuse when a popover is open and flash + focus it
instead; the two select handlers additionally re-sync so the dropdown does not
sit on a value the viewer never moved to.

Deliberately no window.confirm: it blocks the page on an unstyleable dialog
and asks a question whose answer is almost always no. Refusing the navigation
and drawing attention to the popover already on screen says the same thing, and
Escape-or-Save-then-click works immediately. The flash is a two-pulse accent
ring with a prefers-reduced-motion static fallback, removed on a timer so it
never becomes a permanent state.

5. Manual gain does not cross a modality (viewer.ts)

ViewerTransferState gains an optional modality. applyTransfer keeps a
manually-set gain only when the incoming modality matches the new recording's
first group; otherwise it drops it and lets auto-scale measure. Same-modality
swaps behave exactly as before, which was the intended behaviour all along.

6. Dialog-close backstop ([id].astro)

navigateEegViewer nulls eegLive.destroy before awaiting the new mount, so
there is a window where the mount has published _eegvCleanup on the host but
has not returned its disposer. The close handler now takes
live.destroy ?? host._eegvCleanup, clears the host's copy, and calls it once —
one teardown whichever handle exists, never two.

7. Export download failures are visible (annotation-ui.ts)

The export is the escape hatch for annotations that live nowhere else, so a
throwing createObjectURL/click must not read as an inert button. Wrapped;
the failure renders as a role="alert" line directly under the export buttons
and clears on a later success.

8. writeNavOrder (viewer.ts)

Its boolean return exists to report "storage refused". Now warned, with the note
that the choice still applies to the current page.

Documentation (one commit)

• hed-vocab.ts, annotation-ui.ts: the bundle is ~341 KB and 1525 entries,
  not ~140 KB and 500, and it is no longer curated (website#269).
• HedPath, HedVocabEntry.path, extract-hed-vocab.mjs: the long form is the
  storage and lookup key; the SHORT form is what an export writes (website#268).
• labelForPath JSDoc: falls back to the derived short form, matching its own
  inline comment.
• render-dir-listing.ts: added the missing half of the CHEVRON_SVG /
  DIR_CHEVRON_SVG sync comment.
• ADR 0013: dated update note — search now reaches the full non-deprecated tag
  set of both schemas, curation narrowed to quick picks, decision unchanged.
• AGENTS.md: real test count (1395), and the eeg-viewer/ line now names
  recording nav, background preload and HED/SCORE annotation authoring.
• [id].astro: releaseEegViewer has two call sites, not one; and
  openRecordingInDialog can be reached with an inline viewer live, so the
  release there is not a no-op.
• extract-hed-vocab.mjs: dropped the unused byPath map. Regenerating the
  bundle after the change produces a byte-identical file.

Gates

bun run test (1402 passed, up from 1385: +10 in the first round, +7 in the
review round), bun run typecheck (0 errors), bun run lint, bun run build —
all green on the tip.

Review round

A second review panel raised ten more items, all fixed in the nine commits after
1d6615f. The substantial ones:

• The corner X bypassed the draft guard entirely. .close() fires only the
  non-cancelable close event, so the X never reached the cancel guard that
  protects Escape — the more natural exit destroyed the draft with no trace.
  DialogCloseButton now dispatches a cancelable pre-close event
  (src/lib/dialog-close.ts) that a page can veto. The non-detached close is
  deliberately not blocked: it moves the host back inline with the popover
  inside it, so nothing is lost.
• The "Preload paused" note could stick permanently. Its advertised recovery
  (toggle off and on) ran through updatePrefetchTarget, which early-returns
  when no view level is known — and the outage that trips the breaker is the same
  one that fails the interactive read supplying that level. The note is now
  derived from PrefetchController.stalled (which stop() clears too) and
  re-evaluated every render, exactly as the degraded-pyramid note already was.
• destroy() is now idempotent, closing the close-during-navigate race that
  fired it twice through two different handles.
• The draft guard fails open on an internal error, gainCarriesOver is
  extracted and tested, the degraded-overview copy no longer claims a
  cause it cannot know, and the ADR amendment pattern is written into
  .context/decisions/README.md.

Not addressed (out of scope, worth a follow-up)

Collapsing the originating tree row while an annotation draft is open still
discards it, via releaseEegViewer. That is an explicit "close this viewer"
gesture rather than a navigation or a dialog dismissal, and neither panel scoped
it; noting it rather than widening the change.

[cloudflare-workers-and-pages]

Deploying nemar-website with    Cloudflare Pages

Latest commit:	f0715ee
Status:	✅  Deploy successful!
Preview URL:	https://92490bc0.nemar-website.pages.dev
Branch Preview URL:	https://fix-v0-2-5-release-hardening.nemar-website.pages.dev

View logs

[neuromechanist]

Second panel: all ten addressed

Nine commits on top of the original nine. Gates green on the tip: bun run test
1402 passed (was 1395), bun run typecheck 0 errors, bun run lint, bun run build.

#	Finding	Disposition	Commit
1	Corner X bypasses the draft guard	Fixed as a pre-check	c1d19a4
2	"Preload paused" note gets permanently stuck	Fixed by deriving it	e5aee2b
3	destroy() not idempotent	Fixed in the closure	db4a741
4	Draft guard should fail open	Fixed	2c36d7c
5	Stale "only caller" comment	Fixed	f0715ee
6	Degraded copy overclaims a cause	Fixed	2b03d54
7	ADR amendment pattern uncodified	Fixed	6c2c945
8	Modality-gain predicate untested	Extracted + tested	3d0f4a8
9	Missing load()-first degrade case	Added	ebfaa98
10	PR body arithmetic + test count	Fixed (body edited, AGENTS.md → 1402)	f0715ee

1 — corner X (critical)

Confirmed: .close() fires only close, which is not cancelable and reports a
decision already taken, so the X went straight past the cancel guard Escape
respects.

Fixed as a pre-check, per the finding. DialogCloseButton now dispatches a
cancelable DIALOG_CLOSE_REQUEST_EVENT on the dialog before calling
.close(); [id].astro listens and calls preventDefault() when the guard
refuses. New one-constant module src/lib/dialog-close.ts carries the event
name and the rationale, following the NAV_ORDER_CHANGED_EVENT precedent — a
literal duplicated across a component and a page would disable the veto
silently on a typo.

I chose a cancelable event over a capture-phase stopPropagation in
[id].astro. Both work (capture-at-document always precedes bubble-at-document,
so ordering is deterministic), but the event is an explicit, discoverable seam
that reads as a veto instead of relying on propagation-phase trickery, and it
generalizes: any dialog with unsaved state can now refuse its own X. Dialogs
that do not listen are unaffected — no listener means the dispatch cannot be
cancelled.

On the non-detached branch: I checked whether it can drop a draft, and it
cannot, so it is deliberately not blocked. That branch does
anchor.replaceWith(live.host) — it moves the host back into the inline row and
never destroys the instance. The popover is a child of the viewer root inside
that host, and popState lives in the layer's closure, so both travel with the
move and the draft survives intact. Blocking there would refuse a gesture that
loses nothing. eegDraftBlocksDialogClose therefore gates on live.detached,
with the reasoning in its docstring.

2 — stuck preload note (high)

Confirmed, and the diagnosis was right about why: syncDegradedNote re-derives
on every render while syncPreloadNote read a latched local.

Fixed by removing the latch rather than only adding a re-derive. The note is now
computed from PrefetchController.stalled directly, which is the single source
of truth — and stop() now clears the breaker alongside start(), since a
stopped walk is not a stalled one. That closes the specific hole: the toggle-off
half of the advertised recovery goes through stop(), so the note clears even
when the toggle-on half early-returns for want of a view level.

Also added: syncPreloadNote() in renderImpl's read-failure branch (which
returns before updatePrefetchTarget) and in the success path beside
syncDegradedNote, and at both exits of updatePrefetchTarget. New test:
stop() retires the stall along with the walk.

3 — double destroy (important)

Applied as prescribed: if (disposed) return; in the closure, with a comment
naming the three handles that converge there. The [id].astro backstop comment
no longer claims "never two" on its own — it now says the closure guard is what
makes that true, because navigateEegViewer's superseded branch can still call
the same disposer afterwards and only the closure can see that.

4 — fail open (medium)

Done, with the reasoning recorded: a throwing guard would refuse every prev/next
and both dropdowns for the session, with the selects re-syncing away from the
user's pick and no gesture that recovers. One lost draft is the smaller failure.

6 — degraded copy

Agreed. ViewLevelDiscoveryError's own docs list retry-exhausted 5xx, an
expired-token 403 and decode errors, and the flag does not distinguish them.
Suffix is now · overview incomplete; the note is "Overview incomplete — some
zoom levels failed to load. Reload to try again." The docstring says why it
stays cause-free. The preload note keeps "network trouble" as you noted.

8 — gainCarriesOver

Extracted and exported, with six tests: same modality carries, three different
pairs drop, case-insensitive both ways, whitespace tolerated, and every
absent/empty combination treated as a match. That last group is the null-safety
convention made explicit — a transfer record written before the field existed
carries no modality, and absent is "unknown", not "different".

10 — arithmetic

You are right: 4 annotation-store tests plus 6 prefetch tests, not 5 + 6. The
first panel's round added +10 (1385 → 1395). This round adds +7 (6 for
gainCarriesOver, 1 for stop() clearing the breaker) plus the load-first
degrade case, landing at 1402. The PR body has been corrected and AGENTS.md now
reads 1402.

Still not addressed

Collapsing the originating tree row (a second click on it) while a draft is open
still destroys it via releaseEegViewer. That is an explicit "close this viewer"
gesture rather than a navigation, and it is outside both panels' scope; noting it
rather than expanding the change.