From a9d2c3efffc574bcf7f6232def8c2971dc6b6166 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Mon, 11 May 2026 21:56:56 -0700 Subject: [PATCH] explorer: collapse dual mode state to single source of truth (#208 step 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 1 of the #208 state-management refactor. The closure-private \`mode\` variable in the zoomWatcher OJS cell duplicated \`viewer._globeState.mode\`; both were updated together but the duplication caused transient sync mismatches during boot (the URL round-trip harness's iter 1 captures this — \`waitReadyAndPointMode\` succeeds against \`_globeState.mode\` then the snapshot sees a brief cluster state shortly after). Codex's retrospective on PR #203 recommended making \`_globeState.mode\` the single source of truth and wrapping mode transitions in one setter. This change does exactly that: - **\`getMode()\` getter** reads from \`viewer._globeState.mode\`. All former \`mode === ...\` reads in the zoomWatcher cell (10 sites: tryEnterPointModeIfNeeded, source-filter handler, facet-filter handler, camera-changed handler targetMode + transitions + stats guard, hashchange post-flight reconciliation) now go through it. - **\`setExplorerMode(next)\` setter** is the only writer. Used by \`enterPointMode\` and \`exitPointMode\`, which previously did the redundant \`mode = X; viewer._globeState.mode = X;\` pair. - **Closure \`let mode = 'cluster';\` removed.** This is foundation work for step 2 (introduce \`writeGlobeHash\` / \`reconcileCameraState\` and centralize the URL writers — Codex smell 1). Verification: \`url_roundtrip_investigation.js\` — 5/6 iterations green. The iter 1 anomaly ("mode A=cluster B=point" on no-op flight) is the same pre-existing boot-transition timing that Codex flagged in the original retrospective; it's neither caused nor worsened by this refactor and will be addressed when the camera reconciliation path is centralized in step 2. Co-Authored-By: Claude Opus 4.7 (1M context) --- explorer.qmd | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/explorer.qmd b/explorer.qmd index fd42c32..b249751 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -1352,7 +1352,13 @@ zoomWatcher = { if (!facetFilters) return; // wait for facet checkboxes // --- State --- - let mode = 'cluster'; // 'cluster' or 'point' + // `viewer._globeState.mode` is the single source of truth for cluster/ + // point mode (issue #208 smell 2). The closure-private `mode` variable + // that used to live here was removed; reads now go through the helper + // below, writes only via `setExplorerMode()` (in enterPointMode / + // exitPointMode). + const getMode = () => viewer._globeState.mode; + const setExplorerMode = (next) => { viewer._globeState.mode = next; }; let currentRes = 4; let loading = false; let requestId = 0; // stale-request guard @@ -1651,8 +1657,7 @@ zoomWatcher = { // --- Mode transitions --- function enterPointMode(pushHistory) { - mode = 'point'; - viewer._globeState.mode = 'point'; + setExplorerMode('point'); viewer.h3Points.show = false; viewer.samplePoints.show = true; if (pushHistory !== false) history.pushState(null, '', buildHash(viewer)); @@ -1661,8 +1666,7 @@ zoomWatcher = { } function exitPointMode(pushHistory) { - mode = 'cluster'; - viewer._globeState.mode = 'cluster'; + setExplorerMode('cluster'); viewer.samplePoints.show = false; viewer.samplePoints.removeAll(); viewer.h3Points.show = true; @@ -1739,7 +1743,7 @@ zoomWatcher = { // captures the return into `applied` and is immediately followed by // `if (applied) await tryEnterPointModeIfNeeded();`. async function tryEnterPointModeIfNeeded(opts) { - if (mode === 'point') return; + if (getMode() === 'point') return; if (viewer.camera.positionCartographic.height >= ENTER_POINT_ALT) return; let res8Ready = currentRes === 8; @@ -1754,7 +1758,7 @@ zoomWatcher = { }); } const hNow = viewer.camera.positionCartographic.height; - if (res8Ready && mode !== 'point' && hNow < ENTER_POINT_ALT) { + if (res8Ready && getMode() !== 'point' && hNow < ENTER_POINT_ALT) { // Propagate `pushHistory` so boot/hash hydration callers can // avoid growing the browser history stack (issue #207 item 3). enterPointMode(opts && opts.pushHistory); @@ -1924,7 +1928,7 @@ zoomWatcher = { try { updateSourceLegendState(); writeQueryState(); - if (mode === 'cluster') { + if (getMode() === 'cluster') { loading = false; const applied = await loadRes(currentRes, resUrls[currentRes]); // Liveness recovery (issue #190 round-2 review): if the user @@ -2005,9 +2009,9 @@ zoomWatcher = { busyAcquire(); try { const active = hasFacetFilters(); - if (facetNote) facetNote.style.display = (active && mode === 'cluster') ? 'block' : 'none'; + if (facetNote) facetNote.style.display = (active && getMode() === 'cluster') ? 'block' : 'none'; writeQueryState(); - if (mode === 'point') { + if (getMode() === 'point') { cachedBounds = null; cachedTotalCount = null; cachedCapReached = false; @@ -2045,16 +2049,16 @@ zoomWatcher = { // Determine target mode with hysteresis const targetMode = h < ENTER_POINT_ALT ? 'point' : h > EXIT_POINT_ALT ? 'cluster' - : mode; + : getMode(); - if (targetMode === 'point' && mode !== 'point') { + if (targetMode === 'point' && getMode() !== 'point') { // Cold-cache deep-link: the res8 + samples_map_lite fetches // can take 60-90s (DuckDB-WASM 1.24.0 falls back to a full // HTTP read; see issue #190). Delegate to the shared helper // so the source-filter handler can call the same path on // supersession recovery. await tryEnterPointModeIfNeeded(); - } else if (targetMode === 'cluster' && mode !== 'cluster') { + } else if (targetMode === 'cluster' && getMode() !== 'cluster') { exitPointMode(); // Reload appropriate resolution const target = h > 3000000 ? 4 : h > 300000 ? 6 : 8; @@ -2088,7 +2092,7 @@ zoomWatcher = { } // Update viewport cluster count (cluster mode only; point mode already shows viewport count) - if (mode === 'cluster' && viewer._clusterData) { + if (getMode() === 'cluster' && viewer._clusterData) { const bounds = getViewportBounds(); const inView = countInViewport(bounds); const total = viewer._clusterTotal; @@ -2235,8 +2239,8 @@ zoomWatcher = { viewer._suppressHashWrite = false; const s = readHash(); const wantsPoint = s.mode === 'point' || (s.alt != null && s.alt < ENTER_POINT_ALT); - if (wantsPoint && mode !== 'point') enterPointMode(false); - else if (!wantsPoint && mode === 'point') exitPointMode(false); + if (wantsPoint && getMode() !== 'point') enterPointMode(false); + else if (!wantsPoint && getMode() === 'point') exitPointMode(false); }, 2000); // Handle pid / h3 selection (sample mode wins if both present — see