explorer: persist selected cluster identity in URL via &h3=<cell> (Phase 1)#186
Merged
rdhyee merged 7 commits intoisamplesorg:mainfrom May 9, 2026
Merged
Conversation
Phase 1 of EXPLORER_CLUSTER_URL_PROPOSAL.md (isamplesorg#182). Cluster selection now round-trips through the URL hash, complementing the existing &pid= for samples. Use case: share or bookmark a specific cluster you clicked, and have collaborators land on the same H3 cell with side-panel populated. Encoding: #h3=843f6d3ffffffff H3 cell index in canonical 15-char hex (no 0x prefix). The cell index encodes its own resolution; no separate &res= or &cluster_source= field needed. The existing &sources= filter (already URL-persisted) covers the only filter that affects cluster aggregation — material/context/object_type filters can't, per the comment at :1706-1710. Mechanics: - h3_cell carried into the runtime cluster .id at both add() sites (:992, :1335) as a hex string via row.h3_cell.toString(16). The parquet column is UBIGINT; converting to hex once at ingestion keeps the URL representation canonical. - _globeState.selectedH3 added; mutated by cluster-click (:923) and cleared by sample-click (:895) for mutual exclusion. Same pattern as selectedPid. - readHash parses h3 (:626); buildHash emits h3 when set (:645). - fetchClusterByH3 helper at :1791 looks up the row across res4/res6/res8 parquets via UNION ALL. DuckDB-WASM doesn't accept 0x... literals, so hex is converted to decimal in JS via BigInt and CAST AS UBIGINT in SQL. - hydrateClusterUI helper at :1827 mirrors the cluster-click side-panel + nearby-samples query, called from both the boot deep-link (:2266) and the back/forward hashchange handler (:1899). - Mutual-exclusion at hydration time: &pid= wins if both are present, per the proposal §4. EXPLORER_STATE.md §2 updated with the new h3 row. Verified locally: - URL #h3=843f6d3ffffffff (a known res4 cell with 151,334 OpenContext samples in central Turkey) round-trips: side panel shows 'Selected Cluster / OpenContext / H3 res4 / 151,334 samples / 37.6619, 32.8334' with 30 nearby samples loaded. - Empty hash + hash without h3/pid both load without errors. Closes Phase 1 of isamplesorg#182. Phase 2 (unified &sel=) deferred unless a third selection type appears. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five fixes from Codex review:
1. Race-safe hash hydration (BLOCKER)
Both pid and h3 hashchange branches now use a monotonic `viewer._selGen`
token, bumped per hashchange and rechecked after every await. Fast
back/forward across pid/h3/empty no longer lets stale fetch results
repaint the side panel.
2. Strict h3 validation
Replaced `replace(/[^0-9a-fA-F]/g, '')` with `/^[0-9a-f]{15}$/i.test()`
over a lowercased input. Reject malformed input rather than silently
strip — `h3=xxx843f...` no longer becomes a different lookup key.
3. Canonical lowercase normalization
After successful lookup, runtime `selectedH3` is set from the parquet
row's `h3_cell.toString(16)` (always lowercase), not the raw URL token.
Subsequent `buildHash` writes always emit canonical form regardless of
what the user typed. Boot deep-link applies the same normalization.
4. Resolution routing instead of UNION ALL
Canonical H3 cells encode resolution in the 2nd hex char (after the
leading-zero strip). `RES_TO_H3_URL[parseInt(lower[1], 16)]` picks the
right parquet directly — one fetch instead of three on every &h3=
load.
5. Mutual-exclusion in buildHash
Changed independent `if`s for `selectedPid` / `selectedH3` to `else if`,
making the runtime invariant load-bearing in one place.
Also: unknown / malformed h3 now actively clears the cluster card and
nearby-samples list, matching the empty-hash and missing-pid paths
(previously left stale content).
Verified locally:
- Uppercase #h3=843F6D3FFFFFFFF — hydrates, then runtime canonicalizes.
- Unknown well-formed cell #h3=843ffffffffffff — side panel clears, no
errors.
- Non-hex #h3=zzz_NOT_HEX_zzz — silent reject, no JS errors.
- Known #h3=843f6d3ffffffff — round-trips identically.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Pushed
Verified locally:
Ready for re-review. |
…earlier Codex's second review found the previous race fix was incomplete: hydrateClusterUI has its own internal `await db.query(...)` for the nearby- samples list, then calls updateSamples(samples). The hashchange-handler- side selGen check happened only AFTER hydrateClusterUI returned, so a stale fetch INSIDE hydrateClusterUI could still repaint the side panel with samples for an older h3 selection. Fix: hydrateClusterUI now accepts an optional `isStale` predicate and checks it after its inner await, before updateSamples (and before the catch-path's "Query failed" message). The hashchange caller passes `() => selGen !== viewer._selGen`. The cluster-click and boot-deep-link callers leave it undefined — clicks are user-serialized and there's only one boot, so no race possible there. Also (Codex non-blocking nits): - Bump `_selGen` at the very top of the hashchange handler, before the lat/lng early return — so even hashchanges that lack lat/lng invalidate any in-flight stale work. - Reject non-cell H3 modes (`lower[0] !== '8'`) in fetchClusterByH3 — defensive guard against edges/vertices/etc. ever ending up in `&h3=`. Verified locally: known-good `#h3=843f6d3ffffffff` round-trips identically (151,334 OpenContext samples, 30 nearby rendered). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Pushed
Known-good Ready for re-review. |
rdhyee
commented
May 9, 2026
Contributor
Author
rdhyee
left a comment
rdhyee
left a comment
There was a problem hiding this comment.
Findings:
-
P2: Boot deep-link hydration can still race with a later hashchange. explorer.qmd around the boot deep-link path still calls fetchClusterByH3(ih.h3) and then hydrateClusterUI(meta) without the freshness predicate. The hashchange listener is registered earlier, so a slow initial &h3= lookup can be superseded by browser back/forward or a manual hash change, then the boot path can finish later and repaint the side panel with the original stale cluster. The same _selGen guard should cover the boot path too, or boot should abort if location.hash no longer matches the initial ih.h3.
-
P2: fetchClusterByH3 bypasses the active source filter. The lookup query reads WHERE h3_cell = ... but does not include sourceFilterSQL("dominant_source"). Since this PR contract says ?sources= is the one cluster-affecting filter, an &h3= URL whose active sources excludes that cluster can still hydrate a cluster card for a dot that is not visible under the current filter state. That can also produce a mismatched panel: unfiltered cluster card/count/source, but nearby samples filtered by sourceFilterSQL("source") inside hydrateClusterUI.
The hydrateClusterUI internal freshness check from comment 4413059240 fixes the second-round race I had flagged inside the nearby-samples query, but the boot path and source-filter consistency still need tightening.
…ency Two P2 findings from Codex's third pass on PR isamplesorg#186: 1. Boot deep-link could still race with a later hashchange. The hashchange listener registers earlier in the same OJS cell, so a slow initial &h3= or &pid= lookup can be superseded by browser back/forward (or a manual hash edit) during the await — the boot path would then finish later and repaint stale data. Apply the same _selGen guard to the boot path: bump the token at boot start, capture bootSelGen, define isBootStale = () => bootSelGen !== viewer._selGen, and check it after every await (pid lookup, wide-parquet description fetch, h3 lookup, and inside hydrateClusterUI via the existing isStale-predicate parameter). 2. fetchClusterByH3 bypassed the active source filter. The cluster lookup did `WHERE h3_cell = ?` without sourceFilterSQL — so an &h3= URL whose dominant_source is currently unchecked in ?sources= would still hydrate a cluster card for a dot the user can't see on the globe. Worse, hydrateClusterUI's nearby-samples query DOES apply source filter, producing a mismatched panel: full unfiltered cluster card with a filtered-down samples list. Add sourceFilterSQL('dominant_source') to the lookup; an excluded source now returns null and the side panel stays empty (matching what the globe shows). Verified locally: - ?sources=SESAR,GEOME,SMITHSONIAN#h3=843f6d3ffffffff (the cluster's dominant_source OPENCONTEXT is excluded) → side panel stays empty. - ?sources= default (all checked) #h3=843f6d3ffffffff → hydrates as before with 151,334 OpenContext samples and 30 nearby rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Pushed `90d3fdf` addressing Codex's v3 findings:
Verified locally:
Ready for fourth-round review. |
…inally
Two findings from Codex's fourth review:
1. Source-filter changes don't invalidate selection state.
When the user unchecked the source for an already-hydrated cluster
(or sample), the globe correctly hid the dot but the side panel and
`&h3=` / `&pid=` URL stayed stale. Source filter changes also raced
against in-flight selection lookups since they didn't bump `_selGen`.
Fix: in the source-filter change handler (`:1690`), bump `_selGen`
immediately, then after the existing globe-data reload, re-validate
the current selection under the new filter:
- Cluster (selectedH3): re-run fetchClusterByH3 (already honors
sourceFilterSQL after v3); if returns null, clear selectedH3,
cluster card, samples list, and rewrite the URL via replaceState.
- Sample (selectedPid): probe lite_url with the same source filter;
if no match, clear selectedPid + side panel + URL.
Both branches re-check `_selGen` after the await to bail if a newer
filter change has fired.
2. Boot's stale-abort early-returns skipped `_suppressHashWrite = false`.
A no-lat/lng hashchange during boot's awaits could leave hash writes
suppressed forever (the lat/lng path clears it via _suppressTimer; a
stale-aborted boot leaves it set with no later cleanup).
Fix: wrap the boot deep-link block in try/finally; move the
`_suppressHashWrite = false` assignment into the finally so it runs
on every path, including stale-abort early returns.
Verified locally:
- Load #h3=843f6d3ffffffff (OpenContext cluster); side panel hydrates.
- Uncheck OPENCONTEXT in the source filter → `&h3=` drops from URL,
cluster card returns to empty state, samples list clears, ?sources=
written with the remaining 3 sources. Globe also re-renders without
OpenContext clusters.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Pushed `ebd7978` addressing Codex's v4 findings:
Verified locally: load `#h3=843f6d3ffffffff` (OpenContext cluster) → side panel hydrates. Uncheck OPENCONTEXT in legend → `&h3=` drops from URL, cluster card empties, samples list clears, `?sources=` rewrites with the other 3 sources. Ready for fifth-round review. |
…n filter
Two issues from Codex's fifth review:
1. (P2 NEW) Selected cluster surviving the filter wasn't being rehydrated.
When the user toggled a non-cluster source (e.g. unchecked SESAR while
the selected cluster's dominant_source = OPENCONTEXT), the cluster
stayed in URL but the nearby-samples list could now show stale rows
from unchecked sources or miss newly-checked ones (hydrateClusterUI's
nearby query uses sourceFilterSQL('source')).
Fix: in the source-filter handler's revalidate branch, when meta is
truthy (cluster still valid), call hydrateClusterUI(meta, isStale) to
refresh the side panel under the new filter — not just leave it.
2. (UBIGINT precision regression — surfaced by testing #1) DuckDB-WASM
returns h3_cell (UBIGINT > 2^53) as a JS Number, which loses precision
on .toString(16). Boot worked because the SQL WHERE matched at the
parquet level, but `selectedH3 = meta.h3_cell` (lossy roundtrip)
stored a corrupted hex; subsequent revalidations against the corrupted
key would never match and the panel would clear. The bug was latent
in PR-as-of-ebd7978; the rehydrate branch above made it visible.
Fix: SQL SELECT now CASTs h3_cell to VARCHAR (decimal string), and JS
converts to hex via BigInt(decString).toString(16) — no precision
loss. Applied at the two cluster-render sites (phase1, loadRes).
fetchClusterByH3's return now uses the validated input `lower` as the
canonical hex so the helper is also lossless.
`to_hex()` in DuckDB-WASM doesn't exist (tried first, errored
"Catalog Error: Scalar Function with name to_hex does not exist!" —
the VARCHAR cast + JS BigInt is portable across versions).
Verified locally:
- Boot at #h3=843f6d3ffffffff hydrates correctly.
- Uncheck SESAR (OPENCONTEXT survives): cluster card unchanged, samples
re-rendered with only OpenContext rows, &h3= preserved.
- Uncheck OPENCONTEXT (cluster's own source): card + samples cleared,
&h3= dropped from URL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Pushed `0ee44b7` addressing Codex v5 P2 + a latent precision-loss bug it surfaced. Codex v5 P2: cluster surviving filter wasn't being rehydrated. Latent UBIGINT precision-loss bug (surfaced by testing #1). Fix: SQL `SELECT CAST(h3_cell AS VARCHAR)` returns a decimal string; JS converts via `BigInt(decString).toString(16)` — no precision loss. Applied at the two cluster-render sites (`phase1`, `loadRes`); `fetchClusterByH3`'s return now uses the validated input `lower` as the canonical hex. (Tried `to_hex()` first; DuckDB-WASM returns "Catalog Error: Scalar Function with name to_hex does not exist!".) Verified locally:
Ready for sixth-round review. |
Codex's sixth review only finding (P3, non-runtime): the EXPLORER_STATE.md
description still reflected the original v1 implementation:
- "regex `[^0-9a-fA-F]` strip" → now strict `/^[0-9a-f]{15}$/i` reject-not-strip
- "UNION ALL across all 3 parquets" → now resolution-routed via RES_TO_H3_URL
- Missing: cell-mode guard (`lower[0] === '8'`)
- Missing: source filter applied (sourceFilterSQL('dominant_source'))
- Missing: UBIGINT precision-loss workaround (CAST AS VARCHAR + BigInt)
- Missing: source-filter change re-validation
- Missing: _selGen race guard
Updated the h3 row to describe current behavior so future URL-state work
finds accurate docs.
This was referenced May 9, 2026
Closed
rdhyee
added a commit
to rdhyee/isamplesorg.github.io
that referenced
this pull request
May 9, 2026
Codex's review of PR isamplesorg#188 caught a P2 inconsistency: EXPLORER_STATE.md's async-selection invariant claimed coverage of cluster/sample click handlers, but freshSelectionToken() was defined inside the zoomWatcher cell — the click handler lives in the earlier viewer cell and couldn't see it. The click handler still had two unguarded awaits (sample detail load, cluster nearby-samples load) where a slow earlier click could repaint the side panel after the user clicked a different sample/cluster. Fix: move `freshSelectionToken(v)` to top-level OJS scope alongside `readHash` / `buildHash` so both cells can reach it. Take the viewer as a parameter (rather than closing over it) since the function is now defined before the viewer cell runs. Apply `isStale()` checks at the click handler's two await sites: sample-detail post-await and pre-DOM-write, cluster-nearby same. Doc adjusted to reflect the top-level definition. Behavior-preserving regression check: identical results to PR isamplesorg#186's verification suite (boot at #h3=, unchecked-non-cluster source, unchecked cluster source). Refs isamplesorg#187 (post-mortem), isamplesorg#188 (origin refactor PR — Codex's review of that diff surfaced this). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rdhyee
added a commit
that referenced
this pull request
May 9, 2026
#188) * explorer: extract freshSelectionToken() primitive (closes #187 step 1) Six rounds of Codex review on PR #186 surfaced the same class of bug eight times: async work mutating selection state without checking that a newer user event had already superseded it. The duct-tape was a `_selGen` counter inlined at three call sites with slightly different shapes (filter handler, hashchange handler, boot deep-link). Each site bumped the counter, captured the snapshot, and rolled its own stale-check arrow. Extract the pattern as `freshSelectionToken()` in zoomWatcher (~10 lines): bumps `viewer._selGen`, returns an `isStale()` closure. The four call sites now just do: const isStale = freshSelectionToken(); ... await someWork(); if (isStale()) return; — with `isStale` passed into hydrateClusterUI's existing predicate parameter for nested-await freshness. Behavior-preserving refactor: verified locally that the three round-trips from PR #186's verification (boot at #h3=, uncheck non-cluster source, uncheck cluster's own source) produce identical results. Also documents the async-selection invariant explicitly in EXPLORER_STATE.md §4 (a new row for `viewer._selGen` plus an "Async-selection invariant" subsection) so future selection-touching code finds the rule before tripping the wire. Per #187 conversation between Claude Code and Codex: Step 2 of that issue (consolidating selection mutations into a `selectSample` / `selectCluster` / `clearSelection` controller) is YAGNI-deferred until the next feature actually has to touch click + boot + hashchange + filter + URL paths. A wholesale state-machine rewrite is not justified. Refs #187, #186. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * explorer: extend freshness primitive to viewer-cell click handler Codex's review of PR #188 caught a P2 inconsistency: EXPLORER_STATE.md's async-selection invariant claimed coverage of cluster/sample click handlers, but freshSelectionToken() was defined inside the zoomWatcher cell — the click handler lives in the earlier viewer cell and couldn't see it. The click handler still had two unguarded awaits (sample detail load, cluster nearby-samples load) where a slow earlier click could repaint the side panel after the user clicked a different sample/cluster. Fix: move `freshSelectionToken(v)` to top-level OJS scope alongside `readHash` / `buildHash` so both cells can reach it. Take the viewer as a parameter (rather than closing over it) since the function is now defined before the viewer cell runs. Apply `isStale()` checks at the click handler's two await sites: sample-detail post-await and pre-DOM-write, cluster-nearby same. Doc adjusted to reflect the top-level definition. Behavior-preserving regression check: identical results to PR #186's verification suite (boot at #h3=, unchecked-non-cluster source, unchecked cluster source). Refs #187 (post-mortem), #188 (origin refactor PR — Codex's review of that diff surfaced this). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: sync _selGen row in EXPLORER_STATE.md to top-level helper location Codex's P3 nit: the table row still said "in zoomWatcher cell" while the helper is now top-level. The invariant text below was already correct; this just aligns the row. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 of
EXPLORER_CLUSTER_URL_PROPOSAL.md(merged in #182). Cluster selection now round-trips through the URL hash, complementing the existing&pid=for samples.Use case: share or bookmark a specific cluster you clicked, have collaborators land on the same H3 cell with the side-panel populated. Specifically aimed at debugging conversations: "tell me about this dot, why does it disappear when I zoom to res 6?".
Encoding
H3 cell index in canonical 15-char hex (no
0xprefix). The cell index encodes its own resolution; no separate&res=or&cluster_source=field needed.&sources=(already URL-persisted) covers the only filter that affects cluster aggregation — material/context/object_type filters can't, per the comment at:1706-1710.Mechanics
row.h3_cell.toString(16)at bothadd()sites (:992,:1335). Parquet stores UBIGINT; we convert to hex once at ingestion._globeState.selectedH3:923); cleared by sample-click (:895) for mutual exclusion. MirrorsselectedPid.buildHashwrites&h3=whenselectedH3non-null (:645).readHashparsesparams.get('h3')(:626).fetchClusterByH3(:1791) UNIONs across res4/res6/res8 withWHERE h3_cell = CAST('${decimal}' AS UBIGINT). DuckDB-WASM doesn't accept0x...integer literals, so hex → decimal conversion happens in JS via BigInt.hydrateClusterUI(:1827) mirrors the cluster-click side-panel + nearby-samples query. Called from both boot deep-link (:2266) andhashchange(:1899).&pid=wins if both&pid=and&h3=are present, per proposal §4.EXPLORER_STATE.md §2updated with the newh3row.Verified locally
quarto render+ Playwright. Test URL (a known res4 cell with 151,334 OpenContext samples in central Turkey):Result:
Selected Clustercardh3/pidThe cluster-click → URL emission path uses the same
meta.h3_cellfield thatadd()populates; verified by static review (couldn't reliably synthesize Cesium picks via Playwright in this run).Test plan
https://isamples.org/explorer.html#v=1&lat=37.66&lng=32.83&alt=2000000&h3=843f6d3ffffffff— side panel populates without clicking anything&h3=<cell>; reload → side panel re-populates&pid=and not&h3=(mutual exclusion)&pid=clears,&h3=set&h3=value (cell not in any parquet) — page loads cleanly without errorsRefs #182 (proposal),
EXPLORER_STATE.md, #163.🤖 Generated with Claude Code