explorer: surface 'Fetching sample index…' during cold-cache boot→point-mode wait (#190 fix 2)

[rdhyee]

Summary

Implements fix 2 from #190 — surfaces a coherent loading message during the cold-cache boot→point-mode gap so deep-links to a point-mode altitude don't look broken during the 60-90s wait. Three review rounds also tightened underlying race / liveness bugs in loadRes that the longer user-visible window made unsafe.

Independent of fix 1 (move off Quarto's bundled DuckDB-WASM 1.24.0), which is blocked on UBIGINT representation changes in newer wasm builds (see spike result comment).

Phase-message sequence (cold-cache deep-link, alt < 120 km)

Before:

1. Loading H3 res8... — internal jargon, fires ~immediately, persists ~85 s
2. 175,653 clusters, X samples. Zoom closer for individual samples. — flashes for one frame; misleading because the user is already zoomed in
3. Loading individual samples... — fires from inside loadViewportSamples
4. 47 individual samples. Click one for details.

After:

1. Fetching sample index… — fires ~immediately, persists during the cold-cache wait
2. Loading individual samples...
3. 47 individual samples. Click one for details.

On failure: Failed to fetch the sample index — try zooming out and back in. (instead of Failed to load H3 res8 — try zooming again.).

Implementation (final state)

1. New loadRes opts (round 1)

• opts.loadingMsg — override the default "Loading H3 res${res}..." text.
• opts.suppressDoneMsg — skip the interim "... Zoom closer for individual samples." done message that would otherwise flash before point mode overwrites it.
• opts.errorMsg — override the default "Failed to load H3 res${res} — try zooming again." failure copy.

2. Hardened loadRes race contract (round 2)

• Returns true only when fresh data was applied and currentRes = res; false on stale-generation supersession or caught failure. Callers that gate follow-up work on the cluster resolution being ready (e.g. before enterPointMode) must use the return value.
• catch block now has the same generation guard as the success path, so a stale failure can't overwrite UI state owned by a newer in-flight call.
• finally only releases the loading busy-flag if the call is still the current generation, so a stale call's cleanup can't clear the flag while a newer load is still in flight.

3. Idempotent tryEnterPointModeIfNeeded() helper (round 3 + 5bb5a78)

Centralizes the cluster→point transition. Called from four loadRes sites that could be in-flight when the user crosses below ENTER_POINT_ALT:

• The camera handler's targetMode === 'point' && mode !== 'point' branch (the original cold-cache deep-link path).
• The camera handler's two cluster-resolution-reload branches (5bb5a78).
• The source-filter handler's mode === 'cluster' branch, after its own loadRes(currentRes, ...) settles.

The helper short-circuits if already in point mode or above ENTER_POINT_ALT, and re-checks both after its own (potentially long) loadRes await. Closes the supersession liveness gap where a source-filter toggle during the boot wait would supersede the camera handler's pending res8 load and strand the user in cluster mode at point altitude until they nudged the camera.

Stats

+101/-12 across four commits, all in explorer.qmd. No tests added (deterministic parts of tryEnterPointModeIfNeeded would benefit from Playwright coverage — tracked as a follow-up).

Test plan

• Cold-cache deep-link to point mode — fresh browser profile / cache disabled. Visit https://isamples.org/explorer.html#v=1&lat=34.9954&lng=33.7052&alt=62054&heading=360.0&mode=point&h3=882da6b2e1fffff and confirm Fetching sample index… appears and persists until sample dots render, with no Loading H3 res8... or Zoom closer for individual samples. flashes in between.
• Wider deep-link (alt > 180 km, cluster mode) — confirm phase messages are unchanged (still Loading H3 res${res}... → ${count} clusters, ${samples} samples. Zoom in for finer detail.).
• Source-filter toggle while in cluster mode (warm cache) — confirm Loading H3 res${res}... still appears (default loadRes message), no spurious Fetching sample index… flicker because the helper short-circuits at the altitude check.
• Source-filter toggle during cold-cache boot wait (the round-3 race) — toggle a source filter while Fetching sample index… is showing. Confirm: phase message switches to Loading H3 res${currentRes}... while the source-filter's loadRes runs, then back to Fetching sample index… for the recovery loadRes(8), then point mode is entered. Do not end up in cluster mode at point altitude.
• Source-filter toggle while in point mode — confirm Loading individual samples... still appears.
• Camera zoom from cluster → point mode (warm cache) — confirm transition still works.
• Cluster-mode resolution change while crossing into point altitude (round-3.5 race, 5bb5a78) — start in cluster mode, fast-zoom past res4→res6→res8 thresholds straight into point altitude. Confirm point mode is entered without requiring a manual camera nudge.

Follow-ups (post-merge)

Three follow-up issues filed:

1. explorer: Playwright coverage for tryEnterPointModeIfNeeded() invariants #192 — Playwright coverage for tryEnterPointModeIfNeeded invariants — short-circuit at mode === 'point' / above ENTER_POINT_ALT, source-filter→cluster chase path, supersession recovery.
2. explorer: smooth failure-message flicker when tryEnterPointModeIfNeeded chases a failed cluster reload #193 — Smooth the rare failure-message flicker when a chased tryEnterPointModeIfNeeded() call paints Fetching sample index… over a just-painted Failed to load H3 res${target} from a failed cluster reload.
3. explorer: document the 'every loadRes caller chases with tryEnterPointModeIfNeeded' invariant #194 — Document the "every loadRes caller chases with tryEnterPointModeIfNeeded" invariant near the helper definition, to make the supersession-recovery contract self-evident for future contributors adding new loadRes sites.

Refs

• explorer: 60–90 s 'no dots' window on cold-cache deep-link to point mode (DuckDB-WASM 1.24.0 falls back to full HTTP read) #190 — parent issue (DuckDB-WASM 1.24.0 falls back to full HTTP read; this PR ships the UX-side mitigation independently of the runtime version bump)
• Spike comment — why the version bump can't ship as a one-liner today

🤖 Generated with Claude Code

[rdhyee]

Codex review (gpt-5.4)

Asked Codex to review focusing on correctness, race interaction with loadResGen, the failure path, and other entry points to loadRes(8). Findings worth tracking:

1. Medium — enterPointMode() can run after a stale or failed res8 load

await loadRes(8, ...) at the new camera branch returns normally for both stale results (explorer.qmd:1374 if (gen !== loadResGen) return) and caught failures (explorer.qmd:1418-1420). During the 60-90s cold-cache wait, a source-filter change calls loadRes(currentRes, resUrls[currentRes]) (explorer.qmd:1754-1756) — and currentRes is still 4 at that point because the res8 load hasn't completed yet. That bumps loadResGen, the in-flight res8 request becomes stale and silently returns, the camera handler resumes, and enterPointMode() fires anyway — possibly with currentRes !== 8.

Same shape if the user zooms back out during the wait: no post-await altitude/mode recheck.

Suggested fix: have loadRes return true only when it applied fresh data (false for stale/failed), then gate enterPointMode() on that return value plus a fresh altitude check.

2. Medium/Low — stale failures can overwrite newer UI state

The success path inside loadRes has if (gen !== loadResGen) return, but the catch block at L1418-1420 does not. A stale res8 request that rejects after a newer loadRes has already written its phase message will still write "Failed to load H3 res8 — try zooming again.", clobbering the newer state.

Fix: add the same generation guard inside catch.

3. Low — failure copy isn't coherent with the new user-facing message

A user who only ever saw "Fetching sample index…" will find "Failed to load H3 res8 — try zooming again." abrupt and technical (and in the new branch it's also likely to be immediately overwritten by enterPointMode() → loadViewportSamples(), masking the failure entirely).

Fix: add opts.errorMsg (e.g. "Failed to fetch the sample index — try zooming out and back in."), and avoid entering point mode when the prerequisite load failed (folds into finding 1).

Nit

The new comments say "boot→point-mode path," but the override applies to any cluster-to-point transition where currentRes !== 8 (including manual zoom-in). The behavior is fine; the comment is just narrower than the code.

What Codex confirmed is OK

The suppressDoneMsg opt by itself doesn't introduce a "stuck on Fetching sample index…" path — any newer loadRes that supersedes the in-flight one will rewrite the phase message at its own start. The bug is that supersession isn't communicated back to the camera handler, not the message itself.

---

Findings 1 and 2 are real but are pre-existing conditions in the camera-handler / loadRes interaction that this PR makes more visible by lengthening the user-visible "Fetching sample index…" window. Suggest: ship this PR as-is for the UX win, file a follow-up to harden the loadRes return-value contract + catch generation guard. Happy to do either in this PR or a follow-up — preference?

[rdhyee]

Round 2: addressed all three Codex findings in e0f70ef

Finding	Fix
1. Medium — enterPointMode() runs after stale/failed res8 load	loadRes now returns true only when it applied fresh data; false on stale-generation supersession or caught failure. Camera handler gates enterPointMode() on the return value plus a post-await re-check of altitude (hNow < ENTER_POINT_ALT) and mode !== 'point'. Closes the race where a source-filter change during the 60-90s wait supersedes the res8 load and the camera handler would otherwise force point mode with currentRes !== 8.
2. Medium/Low — stale failures overwrite newer UI	Added if (gen !== loadResGen) return false; at the top of loadRes's catch. While I was there: same shape on finally (if (gen === loadResGen) loading = false;) — a pre-existing variant of the same bug, where a stale call's finally could clear the loading busy-flag while a newer load was still in flight.
3. Low — failure copy mismatch	Added opts.errorMsg. Camera-handler boot path now passes 'Failed to fetch the sample index — try zooming out and back in.' so the error reads coherently with 'Fetching sample index…'.
Nit	Widened the inline comment to acknowledge the override messages apply to any cluster→point transition where currentRes !== 8, not just cold boot. The copy reads sensibly for both.

What the user sees now (cold-cache deep-link, three race scenarios)

Scenario	Phase messages
Happy path	Fetching sample index… → Loading individual samples... → 47 individual samples. Click one for details.
Source-filter changed during wait	Fetching sample index… → (filter handler's own Loading H3 res4... → ... clusters, ... samples. Zoom in for finer detail.) — point mode is not entered (correct: res8 isn't loaded under the new filter); next camera movement re-evaluates.
User zooms back out during wait	Fetching sample index… → load completes → altitude re-check fails → stays in cluster mode at the new altitude.
Load fails	Fetching sample index… → Failed to fetch the sample index — try zooming out and back in. (the previous copy Failed to load H3 res8 … is preserved as the default for callers without errorMsg).

Diff for round 2 is +49/-13, all in explorer.qmd.

[rdhyee]

Codex round-2 review

Confirmed clean

• A. loadRes return contract — correct: true only after fresh data is applied and currentRes = res; stale and caught failures return false.
• B. catch / finally gen guards — correct. The guarded finally does not leave loading stuck unless the current-generation db.query itself never resolves, which isn't a new issue.
• C. Post-await re-check — safely covers zoom-back-out and already-in-point-mode, prevents bad entry on supersession.
• D. No new correctness issues.
• E. Ignoring the return value at the cluster-reload loadRes call sites is OK — they don't immediately depend on "this exact load applied" before doing unsafe follow-up work.

Residual finding (Medium/Low) — supersession can leave point transition stalled

In the source-filter supersession case, the source-filter handler reloads currentRes (still likely 4), supersedes the pending res8 load, and then no camera event necessarily fires again. End state: camera still below ENTER_POINT_ALT, mode === 'cluster', currentRes === 4, loading === false. The unsafe point entry is fixed, but the user may need to nudge the camera to retry point mode.

The bad-state-entry is gone (which was the actual correctness bug). What remains is a liveness gap: if a user toggles the source filter during the 60-90s cold-cache wait, they'll stay in cluster mode at point altitude until they pan/zoom the camera even a tiny bit.

Codex's suggested fix shapes:

1. Have the source-filter handler re-run camera-mode reconciliation after its loadRes settles (call the camera-handler's setTimeout body, or factor it into a function and call it).
2. Or change loadRes's return to a richer applied | stale | failed status so the camera handler can distinguish supersession (retry-worthy) from failure (don't retry).

Recommendation

The pre-existing PR was scoped to the cold-cache UX fix; the supersession liveness gap is a marginal scenario (user changes source filter during the boot wait) on a code path that has had similar ergonomic gaps for a while. Suggest:

• Ship explorer: surface 'Fetching sample index…' during cold-cache boot→point-mode wait (#190 fix 2) #191 as-is — round-2 already closes the actual correctness bug Codex flagged.
• Open a follow-up issue for the supersession-retry liveness gap, with shape (1) above as the cheaper option.

Open to either path. If we keep round-2 as the final state of this PR, the residual finding gets a tracking issue. If we want to land it here, I'd take shape (1) — call the camera-mode reconciliation from the source-filter handler's finally after its loadRes returns, gated on whether mode/altitude actually warrant point mode.

[rdhyee]

Round 3: closes the supersession liveness gap in 1f109e0

Addressed Codex's round-2 residual finding by factoring the cluster→point transition into an idempotent helper, then calling it from the source-filter handler too:

async function tryEnterPointModeIfNeeded() {
    if (mode === 'point') return;
    if (viewer.camera.positionCartographic.height >= ENTER_POINT_ALT) return;

    let res8Ready = currentRes === 8;
    if (!res8Ready && !loading) {
        res8Ready = await loadRes(8, h3_res8_url, {
            loadingMsg: 'Fetching sample index…',
            suppressDoneMsg: true,
            errorMsg: 'Failed to fetch the sample index — try zooming out and back in.',
        });
    }
    const hNow = viewer.camera.positionCartographic.height;
    if (res8Ready && mode !== 'point' && hNow < ENTER_POINT_ALT) {
        enterPointMode();
    }
}

Call sites

1. Camera-changed handler — the original boot path. The targetMode === 'point' && mode !== 'point' branch shrinks from ~40 lines to a single await tryEnterPointModeIfNeeded().
2. Source-filter handler's mode === 'cluster' branch — new call after its own loadRes(currentRes, ...) settles. This is what closes the liveness gap: when a source-filter toggle during the cold-cache boot wait supersedes the camera handler's pending res8 load, the source-filter handler now drives the transition forward instead of stranding the user.

Walkthrough of the previously-broken supersession path

Step	State
Camera handler starts loadRes(8, ...) (gen=1)	currentRes=4, loading=true, mode='cluster'
User toggles source filter mid-wait
Source-filter handler: loading=false; await loadRes(4, ...) (gen=2) — supersedes gen=1
Camera's gen=1 awaits resume; gen !== loadResGen → returns false	res8Ready=false, no enterPointMode() ✓
Source-filter's gen=2 settles → currentRes=4, loading=false
Round-2 end state (broken): camera still <ENTER_POINT_ALT, mode='cluster', no further events. User stranded.
Round-3: source-filter handler calls tryEnterPointModeIfNeeded() — height check passes, currentRes !== 8 → fires fresh loadRes(8, ...) (gen=3) under new filter, then enterPointMode() on success. ✓

Other paths considered

• handleFacetFilterChange (material/context/object_type) — doesn't touch loadRes in cluster mode (the parquets only carry dominant_source; these filters surface an explanatory note instead). No supersession risk → no change needed.
• Source-filter handler's mode === 'point' branch — calls loadViewportSamples, not loadRes. Already in point mode; helper would short-circuit anyway.

Stats

Round 3 diff: +54/-39 (net +15 lines; the camera-handler block shrinks by ~30 lines, replaced by the ~30-line helper + 2-line delegation).

The full PR (rounds 1+2+3) is now +88/-12 in explorer.qmd.

[rdhyee]

Codex follow-up review + fix

Found one remaining Medium/Low liveness gap after Round 3: tryEnterPointModeIfNeeded() returns immediately when another loadRes() is already in flight. Round 3 fixed the source-filter supersession path because the source-filter handler calls the helper after its own loadRes() settles, but camera-initiated cluster reloads did not have the same follow-up.

Repro shape:

1. Camera handler starts a cluster loadRes(8) while still above ENTER_POINT_ALT.
2. User continues below ENTER_POINT_ALT while that load is in flight.
3. The point-mode camera handler fires, sees loading === true, and tryEnterPointModeIfNeeded() returns without awaiting or starting res8.
4. The original camera cluster load settles, but no reconciliation reruns, so the user can remain in cluster mode at point altitude until another camera nudge.

Applied fix in 5bb5a78: after camera cluster-resolution loadRes(target, ...) calls settle, call tryEnterPointModeIfNeeded() just like the source-filter recovery path. The helper is idempotent and re-checks altitude/mode, so it is a no-op unless the camera is now below the point threshold.

Validation:

• quarto render explorer.qmd
• git diff --check

Working tree is clean and the fix is pushed to the PR branch.

[rdhyee]

Code Review (final state, post-round 3 + 5bb5a78)

Branch: rdhyee:fix/190-loading-message-boot-to-point-mode → isamplesorg:main
Final state: 4 commits, +101/-12, all in explorer.qmd. Note 5bb5a78 "reconcile point mode after cluster reloads" extended tryEnterPointModeIfNeeded() to the camera handler's two cluster-reload branches as well.

Overview

Implements fix 2 of #190: surfaces a coherent loading message during the cold-cache 60-90s boot→point-mode wait so deep-links don't look broken, while hardening the underlying loadRes race contract that this newly-visible window made unsafe.

Three logical layers landed together:

1. UX: Fetching sample index… / Failed to fetch the sample index… copy via new loadRes opts.
2. Race contract: loadRes returns true only when fresh data was applied; gen-guards on both catch and finally.
3. Liveness recovery: idempotent tryEnterPointModeIfNeeded() called from the camera handler (point + both cluster-reload branches) and the source-filter handler, so any superseded res8 load retries automatically rather than stranding the user.

Correctness — looks solid

• loadRes return contract: true only after currentRes = res; false on stale or caught failure. The catch and finally gen-guards prevent stale calls from clobbering newer UI state or the loading busy-flag.
• tryEnterPointModeIfNeeded: short-circuits cheaply when not applicable (mode/altitude); re-checks both after the long await — same defensive pattern round 2 established for the camera handler.
• All four loadRes sites that could be in-flight when point altitude is entered now call tryEnterPointModeIfNeeded() afterwards (camera→point, camera→cluster-reload×2, source-filter→cluster). Supersession recovery is comprehensive.

Edge cases worth flagging

• tryEnterPointModeIfNeeded short-circuits silently when currentRes !== 8 && loading: if some other in-flight load is what's blocking, this call does nothing. That's safe because that other load's call site is also followed by tryEnterPointModeIfNeeded() — but the safety relies on the invariant that every loadRes caller chases with the helper. Worth a one-line comment near the !loading guard noting that invariant so future contributors adding new loadRes sites know.
• Concurrent tryEnterPointModeIfNeeded calls (e.g. camera→point branch and source-filter handler racing): both can pass the !loading check simultaneously, both call loadRes(8, ...), the second supersedes via loadResGen. Result is correct (second wins, only its enterPointMode() fires); the user might briefly see two Fetching sample index… paint cycles. Acceptable.
• Loading-after-failure copy: if a cluster reload fails (returns false but isn't stale), the camera handler still chases with tryEnterPointModeIfNeeded(), which can paint Fetching sample index… immediately over the just-painted Failed to load H3 res${target} — try zooming again.. Rare; the failure message effectively flickers. Not blocking.

Style / project conventions

• Comment density and "see issue #N / round-N review" inline pointers match the surrounding explorer.qmd style.
• Helper placement (right after enterPointMode / exitPointMode) groups mode-transition logic well.
• Naming (tryEnterPointModeIfNeeded, opts.loadingMsg, opts.errorMsg, opts.suppressDoneMsg) is descriptive.
• The doc-blocks above loadRes and tryEnterPointModeIfNeeded both reference the issue and the round-N review that motivated them — consistent and self-explaining.

Suggestions (non-blocking)

1. Document the "every loadRes caller chases with tryEnterPointModeIfNeeded" invariant near the helper definition — e.g. // Invariant: every loadRes call site that could be in-flight when the user crosses below ENTER_POINT_ALT must call this helper afterwards. Makes the supersession-recovery contract self-evident for future additions.
2. Test coverage gap: no Playwright test exercises any of this. The cold-cache 60-90s scenario is hard to automate, but the deterministic parts could be — e.g. mock loadRes to verify (a) helper short-circuits at mode === 'point', (b) source-filter handler in cluster mode chases with the helper. Worth a follow-up issue rather than blocking this PR.
3. Trailing-comma consistency on the opts object literals across the helper / camera handler / source-filter call sites — minor nit.

Risks

• Test plan in the PR body covers the right scenarios for manual verification; worth running the source-filter-during-boot-wait scenario in particular before merge since that's the specific race round 3 closes.
• No backwards-compat concerns — loadRes's new third opts arg is optional with {} default; existing callers unaffected.
• No security or performance implications — no SQL changes, no input handling, helper is O(1) when short-circuiting.

Verdict

Ready to merge modulo a manual smoke-test of the source-filter-during-cold-cache-wait scenario. The three review rounds tightened a real correctness bug (round 2) and a real liveness gap (round 3) that had nothing to do with the original UX-message ambition — solid surfacing-by-fix-attempt outcome.

Suggest a small follow-up for: (a) Playwright coverage of tryEnterPointModeIfNeeded short-circuit and chase invariants, (b) the residual flicker if a chase paints over a failure message.

[rdhyee]

Codex independent review

I re-reviewed the final PR state after 5bb5a78 and agree with the prior merge recommendation.

Findings

No blocking findings.

What I verified:

• PR is open, not draft, mergeable, and clean against main.
• The only check, say-hello, passed.
• Local branch HEAD matches PR head 5bb5a78.
• git diff --check passes.
• quarto render explorer.qmd passes and leaves the working tree clean.
• The changed async paths in explorer.qmd are internally consistent: the production loadRes call sites are now the helper itself, the source-filter cluster path, and the two camera cluster reload branches. I did not find another loadRes caller that violates the new recovery invariant.

Review notes

The loadRes return contract now correctly distinguishes applied fresh data from stale or failed loads, and the generation guards in catch / finally close the stale UI and busy-flag clobbering risks. tryEnterPointModeIfNeeded() is a reasonable idempotent reconciliation point, and the added calls after source-filter and camera cluster reloads cover the liveness gaps discussed in the earlier review rounds.

The remaining risks are non-blocking:

• No automated coverage exercises the deterministic parts of tryEnterPointModeIfNeeded.
• A rare cluster-load failure can still have its failure copy quickly overwritten by a follow-up Fetching sample index... chase.
• The PR body is now slightly stale where it says other loadRes callers are unchanged; later commits intentionally changed the source-filter and camera reload paths.

Verdict

Ready to merge. Suggested follow-ups after merge: add focused Playwright coverage for the helper/chase invariant, and optionally smooth the rare failure-message flicker.