explorer: add camera.moveEnd URL-write backstop (closes #204) by rdhyee · Pull Request #205 · isamplesorg/isamplesorg.github.io

rdhyee · 2026-05-11T19:39:24Z

Closes #204. Follow-up to #203.

Problem

#203 fixed the await-blocking case for Bug A but the harness against the deployed site revealed a residual: small camera moves below Cesium's percentageChanged = 0.1 threshold don't fire camera.changed, so the URL write is never scheduled. A user who pans a small distance and immediately copies the URL gets stale state. Empirically reproduced on iter 2 of url_roundtrip_investigation.js (Δlat ≈ 0.02° pan only, no zoom).

Fix

Add viewer.camera.moveEnd as a parallel URL-write trigger. It fires once per discrete settled camera move regardless of magnitude — covers drag-pan mouseup, wheel-stop, flight-complete on flyTo. The existing camera.changed handler keeps its role (mode transitions, resolution loads); this just adds a lightweight URL-only backstop.

The _suppressHashWrite gate already handles the hashchange-flight path, so no fight with back/forward navigation.

Verification

tests/playwright/url_roundtrip_investigation.js against localhost on this branch — all 6 iterations pass (cf. iter 2 still failing on the post-#203 deploy).

Risks

Double writes for big camera moves: camera.changed and moveEnd will both fire for above-threshold moves. Both are replaceState (no history-entry growth) and write the same value at settle, so the second write is idempotent. Acceptable cost for the small-move coverage.
**flyTo-triggered writes**: moveEndfires on programmatic flights too. The_suppressHashWrite flag is set during hashchange flights (explorer.qmd:2139`), so back/forward hydration paths are still respected. Other flights (click handlers, source-filter rebuild) intentionally want the URL updated — same as today's behavior.

Test plan

Harness passes all 6 iterations against localhost (Quarto preview, this branch)
Smoke on deploy preview: small pan only → copy URL → paste → matches
Codex review (optional but recommended)

🤖 Generated with Claude Code

isamplesorg#203 fixed the await-blocking case for Bug A but a residual remained: small camera moves below Cesium's `percentageChanged` threshold (set to 0.1 at line 2043) don't fire `camera.changed`, so the debounced callback containing the URL write never runs. A user who pans a small distance and immediately copies the URL gets stale state. Empirically confirmed via `url_roundtrip_investigation.js` against the post-isamplesorg#203 deploy: iter 2 (Δlat ≈ 0.02° pan-only, no zoom) had Context A at the new camera position but `location.href` still at the original deep-link, so Context B opened the stale URL. Fix: add `viewer.camera.moveEnd` as a parallel URL-write trigger. `moveEnd` fires once per discrete settled camera move regardless of magnitude (mouseup on drag-pan, wheel-stop on zoom, flight-complete on flyTo), so every position the user can reach gets captured. Keeps `camera.changed` for its existing role (mode transitions, resolution loads); just adds a lightweight URL-only backstop. The `_suppressHashWrite` gate already handles the hashchange-flight path correctly, so no fight with back/forward navigation. Verification: harness now passes all 6 iterations against localhost with this fix (post-isamplesorg#203 it failed on iter 2). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

#206) (#210) * explorer: report real Samples-in-View count when cap is reached (closes #206) In point mode, the viewport sample query was capped at POINT_BUDGET = 5000 with no ORDER BY. The "Samples in View" stat box and phase message both displayed `cachedData.length`, which equals the budget in any region dense enough to exceed it. Cyprus deep-link (`#v=1&lat=34.9957&lng=33.6798&alt=15212&mode=point`) showed exactly 5,000 — real count from DuckDB-WASM against the lite parquet: 23,421 samples in the same viewport. Counter understated the density by ~5x. Two changes, both localized to `loadViewportSamples`: 1. **Real count**: after the LIMIT POINT_BUDGET data fetch, run a `count(*)` against the same WHERE clause when we hit the cap. Stat box `sSamples` now displays the true count; `sPoints` shows the rendered count under a "Samples Rendered" label so the user sees both numbers at a glance. Phase message becomes "23,422 samples in view (showing 5,000 — zoom in for more). Click one for details." when capped; unchanged when not. Stale-guard applied between the two queries. The count query only runs when `data.length >= POINT_BUDGET`. Below the cap the displayed and real counts are identical, so a second round-trip would be pure waste. 2. **`ORDER BY pid`** on the data query, so the POINT_BUDGET-worth of rows the LIMIT returns is deterministic across browsers and sessions. Codex's recommendation 4 from the #203 retrospective. Verification: - Smoke test against localhost confirms "Samples Rendered: 5,000 / Samples in View: 23,422" for the Cyprus URL (cf. matching ±0.13° DuckDB count from the OC investigation). - url_roundtrip_investigation.js still passes all 6 iterations against localhost (no regression in #203/#205 URL state work). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fixup: address Codex review on #206 (stale guard, cache stats, stable labels) Three changes from the Codex review of PR #210: 1. **Stale guard in count-query catch**: the `try/catch` around the real-count query could mutate state on a stale request if the count failed AFTER a newer request started. Added `if (myReqId !== requestId) return` at the top of the catch, before the warning log. 2. **Cache-hit stats**: when `isWithinCache(bounds)` short-circuits the fetch, the stat boxes were left whatever they were from the previous load (the original loose behavior, but now noticeable because the count info matters). Cached `totalCount` and `capReached` alongside bounds + data, and re-apply on cache hits. 3. **Always-on stable labels**: replaced the conditional label flip ("Samples in View" ⇄ "Samples Rendered") with stable labels — sPoints always says "Samples Rendered", sSamples always says "Samples in View". When the cap isn't reached, both boxes show the same number; Codex flagged the label flip as more confusing than the redundancy. All four `cachedBounds = null` reset sites also clear the new `cachedTotalCount` / `cachedCapReached` fields so they don't leak between mode transitions. Verified locally: - Cyprus smoke test still reads `Samples Rendered: 5,000 / Samples in View: 23,422`. - url_roundtrip_investigation.js all 6 iterations green. Follow-up issue #211 filed (Codex's other suggestion: tighten count to the visible viewport rather than the 30%-padded fetch box). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* explorer: URL state contract micro-fixes (closes #207) 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) <noreply@anthropic.com> * fixup: address Codex review on #207 (hashchange parity + full search-row hydration) 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

) (#214) * tests: Playwright spec for explorer URL state round-trip (closes #209) CI-safe spec set extracted from `tests/playwright/url_roundtrip_investigation.js` (which remains as a manual diagnostic). Five tests covering the URL state contract slices fixed in PRs #203, #205, #210, #212: 1. **Point-mode deep-link with `mode=point` enters point mode** — regression for Bug B fix in #203 (when heading is omitted, the URL must still hydrate into point mode). 2. **Low-alt deep-link WITHOUT `mode=point` enters point mode** — regression for #207 item 4 (boot should enter point mode based on altitude alone, not just the URL's mode flag). 3. **Sub-threshold pan updates URL hash via `moveEnd`** — regression for #205 (Cesium's `camera.changed` suppresses small moves below `percentageChanged` threshold; `moveEnd` is the backstop). 4. **URL round-trips across browser contexts** — combined regression for #203 + #205. Context A pans + zooms after settle, captures URL; Context B opens the captured URL and arrives at the same camera + mode within tight tolerance. 5. **`h3` hashchange with invalid cell clears `_globeState.selectedH3`** — regression for #207 item 6 (asymmetry between boot and hashchange null-result branches). Search-result row click selection (#207 item 8) deferred — needs working FTS data and depends on substrate the spec shouldn't have to set up. Tests use `waitForPointModeSettled` (waits for the phase message's "Click one for details." text) instead of `waitForMode` for the no-`mode=point` cases — `waitForMode` can match a transient mode flip during the boot transition (the dual-mode-state anomaly being addressed structurally in #208). `test.setTimeout(360000)` per describe block — cold-cache point-mode boot can take 60–90s per #190, and the round-trip test opens multiple fresh contexts each paying the cost. Verified: all 5 tests pass against localhost in ~1m 24s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * tests: address Codex round-1 review on URL roundtrip specs Per Codex review on PR #214: - Replace fixed `waitForTimeout` sleeps with `waitForFunction` polling URL hash in tests 3 (sub-threshold pan) and 4 (cross-context round-trip). Fixes thin timing margin (0.5s) over flight + moveEnd debounce. - Wrap context-creation in try/finally so ctxA/ctxB get closed on mid-test assertion failure (test 4). - Drop unused `before` snapshot in test 3. - Move 6-minute `test.setTimeout(360000)` from describe-level default (180s) to per-test override on the round-trip test only. Single broken test no longer waits 6 minutes to fail in CI. - Add `waitForBootSettled(page)` helper waiting for `viewer._suppressHashWrite === false` — `_globeState.mode` is initialized early (explorer.qmd:871) while the hashchange listener is registered late (explorer.qmd:2210); waitForMode alone isn't enough. - Strengthen test 5 with the boot-settle wait so the hashchange listener exists before driving the test hashchange. - Add comment in `snapshot()` noting `_globeState.selectedH3`/`selectedPid` field coupling. - Update header: change `closes #209` to `addresses #209` (PR explicitly defers several #209 checklist items to a follow-up). Remove the internally-inconsistent "don't try cold-cache deep-links" sentence that contradicted the timeout comment's cold-cache rationale. Kept (with rationale in code comments): - `waitForPointModeSettled` matches "Click one for details" rather than the count phrase Codex suggested — count phrase misses the cap-reached done-state branch (explorer.qmd:1611) which says "samples in view ..." with no "individual" word. "Click one for details" is the common denominator across both done-state branches. - Test 5 h3-hashchange remains a hashchange-driven test rather than the valid→invalid round-trip Codex proposed. The original assertion DOES catch the regression: line 2272 unconditionally overwrites selectedH3 from the URL on hashchange, so the only way the post-handler value is null is the explicit null-result branch at line 2285. Codex's "trivially true" framing missed that line 2272 always fires first. (Tried seeding selectedH3 before the hashchange to make the test even stronger; the seed survived the handler unexpectedly — suspected OJS reactivity around mainModule.value('viewer') returning a different reference than the handler's closure. Documented in test comment.) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * tests: address Codex round-2 review on URL roundtrip specs Codex round 2 sharpened the round-1 concern on Test 5 and was right: booting with `&h3=<invalid>` runs the boot deep-link's own null-result branch (explorer.qmd:2728) which sets `selectedH3=null` BEFORE the test even drives a hashchange. So the post-mutation `selectedH3 === null` wait is satisfied by boot, not by the hashchange handler's null-clear branch at line 2285 (the regression we want to guard). Fix: gate the assertion on a handler-only side effect. The hashchange handler's `camera.flyTo` (explorer.qmd:2220-2227) rotates `camera.heading` to the URL's heading value — only the handler produces this rotation. Wait for heading to reach ~5° before asserting `selectedH3 === null`. This proves the handler executed past line 2272 (which would write a non-null selectedH3 from the URL) AND reached line 2285 (the null-clear). Also addresses Codex's altitude nit on Test 4: `waitForHashLatLng` now takes an optional `alt` so the round-trip assertion checks lat + lng + alt together, matching what `buildHash` actually writes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Codex round-3 spotted a pre-existing doc gap: zoomWatcher registers both viewer.camera.changed AND viewer.camera.moveEnd (added in #205 for sub-threshold pan settling), but §5 listed only camera.changed. Adding moveEnd to both the event-listeners and the URL-writes cells. Not strictly mockup-v1 scope but landing while we're here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

rdhyee merged commit de3fab1 into isamplesorg:main May 11, 2026
1 check passed

rdhyee mentioned this pull request May 12, 2026

explorer: URL state contract micro-fixes (closes #207) #212

Merged

4 tasks

This was referenced May 12, 2026

tests: Playwright spec for explorer URL state round-trip (addresses #209) #214

Merged

explorer: round-trip specs for remaining URL state surfaces (follow-up to #209) #215

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

explorer: add camera.moveEnd URL-write backstop (closes #204)#205

explorer: add camera.moveEnd URL-write backstop (closes #204)#205
rdhyee merged 1 commit into
isamplesorg:mainfrom
rdhyee:fix/204-camera-moveend-url-write

rdhyee commented May 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

rdhyee commented May 11, 2026

Problem

Fix

Verification

Risks

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant