diff --git a/EXPLORER_STATE.md b/EXPLORER_STATE.md index 923fd9c..ebd6b39 100644 --- a/EXPLORER_STATE.md +++ b/EXPLORER_STATE.md @@ -111,8 +111,15 @@ across cells without becoming a separate OJS reactive value. | `viewer.h3Points` | Cesium `PointPrimitiveCollection` | `:815` | cluster mode rendering | | | `viewer.samplePoints` | Cesium `PointPrimitiveCollection` | `:818` | point mode rendering | | | `viewer.pointLabel` | Cesium label entity | `:823` | mouse-move handler (`:836-848`) | hover tooltip | +| `viewer._selGen` | int | bumped by every `freshSelectionToken(viewer)` call (top-level helper, see invariant below) | snapshot captured by each handler that mutates selection | freshness counter; see invariant below | | `window.refreshSamplesTable` | `() => Promise` | `:1238` | external (debug / Playwright) | not used by other cells; safe to keep or remove | +### Async-selection invariant + +Any async work that updates `viewer._globeState`, the URL hash, or the side-panel DOM **must check freshness after every await**. The `freshSelectionToken(viewer)` helper (defined at top level alongside `readHash` / `buildHash` so both the viewer-cell click handler and the zoomWatcher-cell handlers can reach it) is the primitive: each user-input event handler that touches selection (cluster/sample click, hashchange, source-filter toggle, boot deep-link) calls it once at start to bump `_selGen` and capture an `isStale()` closure; every subsequent await is followed by `if (isStale()) return;` before any state/URL/DOM mutation. Pass `isStale` into nested helpers (`hydrateClusterUI`'s second param) so their internal awaits also bail before touching the DOM. + +This invariant exists because there's no central "selection store" — selection state lives in `_globeState`, the URL hash, and the side-panel DOM, and four different paths (click, hashchange, filter, boot) write to all three. Without the freshness check, a slow earlier handler can repaint the side panel for a selection the user has already moved off of. Issue #187 has the post-mortem on the 6-round Codex review that motivated extracting the primitive. + ### `_urlParamsHydrated` — confirmed gone Grep for `_urlParamsHydrated` in `explorer.qmd` returns no hits. The flag from diff --git a/explorer.qmd b/explorer.qmd index 686d23a..0566aa0 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -649,6 +649,30 @@ function buildHash(v) { return '#' + params.toString(); } +// === Selection freshness primitive === +// +// Async work that updates `viewer._globeState`, the URL hash, or the side +// panel must check freshness after every await. A user input (click, +// hashchange, source-filter toggle, boot deep-link) bumps `_selGen`; any +// in-flight async work whose generation no longer matches must NOT mutate +// anything that an interactive newer event has already moved. +// +// Usage: +// const isStale = freshSelectionToken(viewer); +// await someWork(); +// if (isStale()) return; +// updateDOM(); +// +// Pass `isStale` into helpers (see hydrateClusterUI) so their internal +// awaits also bail before touching DOM. Top-level so both the viewer-cell +// click handler and the zoomWatcher-cell handlers can reach it. See issue +// #187 for the post-mortem that motivated extracting this primitive. +function freshSelectionToken(v) { + v._selGen = (v._selGen || 0) + 1; + const gen = v._selGen; + return () => gen !== v._selGen; +} + // === Helpers: update DOM imperatively (no OJS reactivity) === function updateStats(phase, points, samples, time, pointsLabel, samplesLabel) { const s = (id, v) => { const e = document.getElementById(id); if (e) e.textContent = v; }; @@ -885,11 +909,14 @@ viewer = { } }, Cesium.ScreenSpaceEventType.MOUSE_MOVE); - // Click handler — routes to cluster card or sample card + // Click handler — routes to cluster card or sample card. + // Uses freshSelectionToken so a slow detail/nearby fetch doesn't repaint + // the side panel after the user has clicked a different sample/cluster. new Cesium.ScreenSpaceEventHandler(v.scene.canvas).setInputAction(async (e) => { const picked = v.scene.pick(e.position); if (!Cesium.defined(picked) || !picked.primitive || !picked.id) return; const meta = picked.id; + const isStale = freshSelectionToken(v); if (typeof meta === 'object' && meta.type === 'sample') { // --- Individual sample click --- @@ -909,12 +936,14 @@ viewer = { WHERE pid = '${meta.pid.replace(/'/g, "''")}' LIMIT 1 `); + if (isStale()) return; if (detail && detail.length > 0) { updateSampleDetail(detail[0]); } else { updateSampleDetail({ description: '' }); } } catch(err) { + if (isStale()) return; console.error("Detail query failed:", err); updateSampleDetail(null); } @@ -943,8 +972,10 @@ viewer = { LIMIT 30 `; const samples = await db.query(nearbyQuery); + if (isStale()) return; updateSamples(samples); } catch(err) { + if (isStale()) return; console.error("Sample query failed:", err); if (sampEl) sampEl.innerHTML = '
Query failed — try again.
'; } @@ -1305,6 +1336,10 @@ zoomWatcher = { let requestId = 0; // stale-request guard // clusterDataCache stored on viewer._clusterData (set by phase1 and loadRes) + // freshSelectionToken(viewer) is defined at top level (alongside readHash / + // buildHash) so the viewer cell's click handler and this cell's handlers + // can both reach it. See issue #187. + // Hysteresis thresholds to avoid flicker const ENTER_POINT_ALT = 120000; // 120 km → enter point mode const EXIT_POINT_ALT = 180000; // 180 km → exit point mode @@ -1699,8 +1734,7 @@ zoomWatcher = { // any in-flight selection lookup AND re-validate the current selection // (cluster or sample) under the new filter — if it's filtered out, drop // it from runtime state and the URL so the side panel matches the globe. - viewer._selGen = (viewer._selGen || 0) + 1; - const filterSelGen = viewer._selGen; + const isStale = freshSelectionToken(viewer); try { updateSourceLegendState(); writeQueryState(); @@ -1714,11 +1748,11 @@ zoomWatcher = { refreshFacetCounts(); // Re-validate selection (only if no newer filter change has fired). - if (filterSelGen === viewer._selGen) { + if (!isStale()) { const sel = viewer._globeState; if (sel.selectedH3) { const meta = await fetchClusterByH3(sel.selectedH3); - if (filterSelGen !== viewer._selGen) return; + if (isStale()) return; if (!meta) { sel.selectedH3 = null; updateClusterCard(null); @@ -1731,7 +1765,7 @@ zoomWatcher = { // source-filtered too — re-run it under the new filter // so the panel doesn't show stale rows from unchecked // sources (or miss newly-checked ones). - await hydrateClusterUI(meta, () => filterSelGen !== viewer._selGen); + await hydrateClusterUI(meta, isStale); } } else if (sel.selectedPid) { const safe = sel.selectedPid.replace(/'/g, "''"); @@ -1741,7 +1775,7 @@ zoomWatcher = { ${sourceFilterSQL('source')} LIMIT 1 `); - if (filterSelGen !== viewer._selGen) return; + if (isStale()) return; if (!stillVisible || stillVisible.length === 0) { sel.selectedPid = null; updateClusterCard(null); @@ -1930,7 +1964,7 @@ zoomWatcher = { window.addEventListener('hashchange', async () => { // Bump the selection generation BEFORE any early-return so even // hashchanges that lack lat/lng invalidate stale async work. - viewer._selGen = (viewer._selGen || 0) + 1; + const isStale = freshSelectionToken(viewer); const state = readHash(); if (state.lat == null || state.lng == null) return; @@ -1958,9 +1992,8 @@ zoomWatcher = { // EXPLORER_CLUSTER_URL_PROPOSAL §4). Both branches do an `await` against // a remote parquet, so a fast back/forward could race: an older fetch // resolves AFTER a newer hash has applied, and would otherwise repaint - // the side panel with stale data. `_selGen` is bumped at the very top - // of this handler; we capture it here and check after each await. - const selGen = viewer._selGen; + // the side panel with stale data. `isStale` is the freshness token + // captured at the top of this handler; we check it after each await. if (state.pid) { viewer._globeState.selectedPid = state.pid; viewer._globeState.selectedH3 = null; @@ -1971,7 +2004,7 @@ zoomWatcher = { WHERE pid = '${state.pid.replace(/'/g, "''")}' LIMIT 1 `); - if (selGen !== viewer._selGen) return; + if (isStale()) return; if (sample && sample.length > 0) { const s = sample[0]; updateSampleCard({ @@ -1987,10 +2020,10 @@ zoomWatcher = { viewer._globeState.selectedPid = null; viewer._globeState.selectedH3 = state.h3.toLowerCase(); const meta = await fetchClusterByH3(state.h3); - if (selGen !== viewer._selGen) return; + if (isStale()) return; if (meta) { viewer._globeState.selectedH3 = meta.h3_cell; // canonical lowercase - await hydrateClusterUI(meta, () => selGen !== viewer._selGen); + await hydrateClusterUI(meta, isStale); } else { // Unknown / malformed h3 — clear the side panel rather than // leaving prior content stranded. @@ -2337,14 +2370,13 @@ zoomWatcher = { // Sample mode wins if both &pid= and &h3= are present (see EXPLORER_CLUSTER_URL_PROPOSAL §4). // The boot path runs once, but the hashchange listener is already registered // by this point — back/forward or a manual hash edit during the boot await - // could supersede this lookup. Use the same `_selGen` token the hashchange - // handler uses; bumping it here also invalidates any in-flight lookups. + // could supersede this lookup. Capture the same freshness token the + // hashchange handler uses; bumping it here also invalidates any in-flight + // lookups. // Wrap in try/finally so `_suppressHashWrite = false` always runs even if // a stale early-return aborts the deep-link work — otherwise a no-lat/lng // hashchange during boot could leave hash writes suppressed forever. - viewer._selGen = (viewer._selGen || 0) + 1; - const bootSelGen = viewer._selGen; - const isBootStale = () => bootSelGen !== viewer._selGen; + const isStale = freshSelectionToken(viewer); const ih = viewer._initialHash; try { if (ih.pid) { @@ -2357,7 +2389,7 @@ zoomWatcher = { WHERE pid = '${ih.pid.replace(/'/g, "''")}' LIMIT 1 `); - if (isBootStale()) return "active"; + if (isStale()) return "active"; if (sample && sample.length > 0) { const s = sample[0]; updateSampleCard({ @@ -2370,7 +2402,7 @@ zoomWatcher = { WHERE pid = '${ih.pid.replace(/'/g, "''")}' LIMIT 1 `); - if (isBootStale()) return "active"; + if (isStale()) return "active"; if (detail && detail.length > 0) updateSampleDetail(detail[0]); else updateSampleDetail({ description: '' }); } @@ -2380,10 +2412,10 @@ zoomWatcher = { } else if (ih.h3) { viewer._globeState.selectedH3 = ih.h3.toLowerCase(); const meta = await fetchClusterByH3(ih.h3); - if (isBootStale()) return "active"; + if (isStale()) return "active"; if (meta) { viewer._globeState.selectedH3 = meta.h3_cell; // canonical lowercase - await hydrateClusterUI(meta, isBootStale); + await hydrateClusterUI(meta, isStale); } else { // Unknown / malformed h3, OR filtered out by ?sources=. Drop it // from runtime state so buildHash() doesn't keep emitting it.