From e0d053874e4cbef1cfe56515ac404ddbd6938ebf Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Mon, 11 May 2026 21:15:23 -0700 Subject: [PATCH 1/2] explorer: URL state contract micro-fixes (closes #207) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four small, independent fixes addressing the URL state contract gaps surfaced by Codex's retrospective on PRs #203 + #205. Each is bounded to its own call site. **Item 4 — low-altitude-without-`mode=point` round-trip.** #203's early URL write can produce URLs like `alt=8000` *without* `mode=point` if the user copies during a pending point-mode transition. Boot only force-entered point mode when the URL had `mode=point`, so the round- trip landed in cluster mode at low altitude — not what the user saw. Boot now also enters point mode when `ih.alt < ENTER_POINT_ALT`, regardless of `ih.mode`. `tryEnterPointModeIfNeeded()` short-circuits above the altitude threshold so cluster deep-links are unaffected. **Item 6 — `h3` hashchange null-result asymmetry.** When `fetchClusterByH3(state.h3)` returns null in the hashchange handler, the UI was cleared but `viewer._globeState.selectedH3` was left set to the invalid value. Boot path already cleared it on the same condition; hashchange now matches. Prevents `buildHash()` from re-emitting the invalid h3 on subsequent camera moves. **Item 8 — selection state with search-result flights.** Search- result row clicks flew the camera but didn't update selection, so the URL captured at flight settle carried whatever prior pid/h3 was selected. Two changes: row click now sets `selectedPid` to the clicked sample's pid (treating the click as a selection, matching on-globe sample clicks); the auto-flight to first result on world- scope searches clears prior selection (treating the auto-pan as a nudge, not an explicit selection). **Item 3 — boot/hash hydration shouldn't grow history.** `tryEnter` `PointModeIfNeeded()` ultimately called `enterPointMode()` with default `pushHistory`, so boot would `pushState` a history entry. Threaded an `opts.pushHistory` option through the helper; boot now passes `{ pushHistory: false }`. Cluster deep-links unaffected (helper short-circuits before that path). Camera-handler call sites keep the default (pushState) since those are user-driven movements. Verified locally: - `url_roundtrip_investigation.js` all 6 iterations green - Low-alt no-`mode=point` URL (`#alt=8000` with no mode param) now enters point mode and shows the real 23k count (post-#206 + #207 item 4). Co-Authored-By: Claude Opus 4.7 (1M context) --- explorer.qmd | 66 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/explorer.qmd b/explorer.qmd index b3cb2d8..f0c2533 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -1738,7 +1738,7 @@ zoomWatcher = { // and one per external caller). For each external match, confirm it // captures the return into `applied` and is immediately followed by // `if (applied) await tryEnterPointModeIfNeeded();`. - async function tryEnterPointModeIfNeeded() { + async function tryEnterPointModeIfNeeded(opts) { if (mode === 'point') return; if (viewer.camera.positionCartographic.height >= ENTER_POINT_ALT) return; @@ -1755,7 +1755,9 @@ zoomWatcher = { } const hNow = viewer.camera.positionCartographic.height; if (res8Ready && mode !== 'point' && hNow < ENTER_POINT_ALT) { - enterPointMode(); + // Propagate `pushHistory` so boot/hash hydration callers can + // avoid growing the browser history stack (issue #207 item 3). + enterPointMode(opts && opts.pushHistory); } } @@ -2269,8 +2271,13 @@ zoomWatcher = { viewer._globeState.selectedH3 = meta.h3_cell; // canonical lowercase await hydrateClusterUI(meta, isStale); } else { - // Unknown / malformed h3 — clear the side panel rather than - // leaving prior content stranded. + // Unknown / malformed h3, OR filtered out by ?sources= — + // clear the side panel rather than leaving prior content + // stranded, AND drop the h3 from runtime state so + // buildHash() doesn't keep emitting it (issue #207 item 6: + // restores symmetry with the boot deep-link path which + // already does this). + viewer._globeState.selectedH3 = null; updateClusterCard(null); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; @@ -2490,7 +2497,12 @@ zoomWatcher = { } sampEl.innerHTML = h; - // Click search result → fly to it + // Click search result → fly to it AND set it as the + // selected sample (issue #207 item 8). Without the + // selection write, the URL captured at flight settle + // would carry whatever prior pid/h3 was selected, + // misrepresenting the user's actual view in any paste- + // elsewhere flow. sampEl.querySelectorAll('.sample-row[data-lat]').forEach(row => { row.addEventListener('click', (e) => { if (e.target.tagName === 'A') return; // let links work @@ -2498,6 +2510,12 @@ zoomWatcher = { const lng = parseFloat(row.dataset.lng); const pid = row.dataset.pid; if (!isNaN(lat) && !isNaN(lng)) { + // Update selection BEFORE the flight so that + // moveEnd's buildHash captures the new pid. + if (pid) { + viewer._globeState.selectedPid = pid; + viewer._globeState.selectedH3 = null; + } viewer.camera.flyTo({ destination: Cesium.Cartesian3.fromDegrees(lng, lat, 50000), duration: 1.5 @@ -2510,7 +2528,15 @@ zoomWatcher = { // Fly to the first result. Skip for area-scoped searches — // the user is already at the area they care about; flying // would zoom in and disorient. + // + // Auto-flight is a NUDGE, not an explicit selection — clear + // any prior selectedPid/selectedH3 so the resulting URL + // doesn't carry stale selection state across the flight + // (issue #207 item 8). User clicks on a specific row to + // establish a new selection. if (effectiveScope === 'world' && results[0].latitude && results[0].longitude) { + viewer._globeState.selectedPid = null; + viewer._globeState.selectedH3 = null; viewer.camera.flyTo({ destination: Cesium.Cartesian3.fromDegrees(results[0].longitude, results[0].latitude, 200000), duration: 1.5 @@ -2672,17 +2698,27 @@ zoomWatcher = { viewer._suppressHashWrite = false; } - // Deep-link mode hydration (issue #201, Bug B). The boot path positions - // the camera via `camera.setView`, which in some cases (notably when the - // URL omits `heading` — i.e. heading defaults to 0 — at point altitudes) - // does not raise `camera.changed`, so the camera-changed handler never - // runs and the URL's `mode=point` is silently ignored. Drive the mode - // transition explicitly here so the boot is independent of whether - // `setView` happened to fire the event. `tryEnterPointModeIfNeeded()` + // Deep-link mode hydration (issue #201 Bug B + issue #207 item 4). + // The boot path positions the camera via `camera.setView`, which in + // some cases (notably when the URL omits `heading` — i.e. heading + // defaults to 0 — at point altitudes) does not raise `camera.changed`, + // so the camera-changed handler never runs and the URL's `mode=point` + // is silently ignored. Drive the mode transition explicitly here so + // the boot is independent of whether `setView` happened to fire the + // event. + // + // Trigger on EITHER `ih.mode === 'point'` OR low altitude (< ENTER_POINT_ALT). + // The altitude branch closes the loophole introduced by #203's early + // URL write: a user copying mid-transition can produce a URL like + // `alt=8000` *without* `mode=point`, which previously round-tripped + // as cluster mode at low altitude. Now boot enters point mode for + // any URL whose altitude says it should. `tryEnterPointModeIfNeeded()` // short-circuits if alt >= ENTER_POINT_ALT or we're already in point - // mode, so this is a no-op for cluster deep-links. - if (ih.mode === 'point') { - await tryEnterPointModeIfNeeded(); + // mode, so this is a no-op for cluster deep-links at cluster altitude. + if (ih.mode === 'point' || (ih.alt != null && ih.alt < ENTER_POINT_ALT)) { + // pushHistory: false — boot should reconcile state without adding + // a history entry (issue #207 item 3). + await tryEnterPointModeIfNeeded({ pushHistory: false }); } return "active"; From 5176967444ba53c504fbf504ad5365a8c392da5f Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Mon, 11 May 2026 21:26:27 -0700 Subject: [PATCH 2/2] fixup: address Codex review on #207 (hashchange parity + full search-row hydration) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues from the Codex review of PR #212: 1. **Hashchange handler now uses altitude-authoritative predicate** (matches boot's #207 item 4 fix). Without this, navigating back/ forward through a `#alt=8000` URL with no `mode=point` would re-enter cluster mode via the 2s post-flight setTimeout — boot would enter point mode but hashchange would force exit it. Same predicate used in both places now. 2. **Search-result row click does the full sample-selection ceremony**, not just selectedPid + flyTo. Matches on-globe sample click at line ~944: freshness token bump, updateSampleCard with the sample's metadata, flyTo, lazy detail load with stale guards in both success and catch paths. Without this, a slow prior selection's detail query could repaint the panel after the click, leaving the UI inconsistent with `_globeState` and the URL. samplesSection is NOT cleared (unlike the globe click handler) because it holds the search results the user is browsing. Verified: url_roundtrip_investigation.js — all 6 iterations green. Co-Authored-By: Claude Opus 4.7 (1M context) --- explorer.qmd | 82 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 59 insertions(+), 23 deletions(-) diff --git a/explorer.qmd b/explorer.qmd index f0c2533..fd42c32 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -2226,12 +2226,17 @@ zoomWatcher = { duration: 1.5, }); - // After flight settles, force mode and clear suppress flag + // After flight settles, force mode and clear suppress flag. + // Use the same altitude-authoritative predicate as the boot path + // (issue #207 item 4 → Codex follow-up): without this, back/forward + // through a `#alt=8000` URL with no `mode=point` would exit point + // mode here even though boot would have entered it. viewer._suppressTimer = setTimeout(() => { viewer._suppressHashWrite = false; const s = readHash(); - if (s.mode === 'point' && mode !== 'point') enterPointMode(false); - else if (s.mode !== 'point' && mode === 'point') exitPointMode(false); + 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); }, 2000); // Handle pid / h3 selection (sample mode wins if both present — see @@ -2497,29 +2502,60 @@ zoomWatcher = { } sampEl.innerHTML = h; - // Click search result → fly to it AND set it as the - // selected sample (issue #207 item 8). Without the - // selection write, the URL captured at flight settle - // would carry whatever prior pid/h3 was selected, - // misrepresenting the user's actual view in any paste- - // elsewhere flow. + // Click search result → treat as a full selection event + // (issue #207 item 8 → Codex follow-up): freshness token + // bump, sample-card hydration, selection state, flight, + // and lazy detail load — matching the on-globe sample + // click ceremony at line ~944. Without the full ceremony + // a slow prior selection load could repaint the panel + // after the click, leaving UI inconsistent with URL. + const resultsByPid = new Map(results.map(s => [s.pid, s])); sampEl.querySelectorAll('.sample-row[data-lat]').forEach(row => { - row.addEventListener('click', (e) => { + row.addEventListener('click', async (e) => { if (e.target.tagName === 'A') return; // let links work - const lat = parseFloat(row.dataset.lat); - const lng = parseFloat(row.dataset.lng); const pid = row.dataset.pid; - if (!isNaN(lat) && !isNaN(lng)) { - // Update selection BEFORE the flight so that - // moveEnd's buildHash captures the new pid. - if (pid) { - viewer._globeState.selectedPid = pid; - viewer._globeState.selectedH3 = null; - } - viewer.camera.flyTo({ - destination: Cesium.Cartesian3.fromDegrees(lng, lat, 50000), - duration: 1.5 - }); + const sample = pid ? resultsByPid.get(pid) : null; + if (!sample || sample.latitude == null || sample.longitude == null) return; + + // Bump the freshness token BEFORE any async work so + // a prior cluster/sample detail load can't repaint + // the panel after we navigate. + const isStale = freshSelectionToken(viewer); + + viewer._globeState.selectedPid = pid; + viewer._globeState.selectedH3 = null; + updateSampleCard({ + pid: sample.pid, + label: sample.label, + source: sample.source, + lat: sample.latitude, + lng: sample.longitude, + place_name: sample.place_name, + result_time: sample.result_time + }); + viewer.camera.flyTo({ + destination: Cesium.Cartesian3.fromDegrees(sample.longitude, sample.latitude, 50000), + duration: 1.5 + }); + + // Lazy-load full description (mirrors the globe + // click handler). Don't clear samplesSection here — + // it currently holds the search results the user + // is browsing. + try { + const detail = await db.query(` + SELECT description + FROM read_parquet('${wide_url}') + WHERE pid = '${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("Search-row detail query failed:", err); + updateSampleDetail(null); } }); });