Skip to content

explorer: skip tryEnterPointModeIfNeeded chase on non-applied loadRes (closes #193)#196

Merged
rdhyee merged 1 commit intoisamplesorg:mainfrom
rdhyee:fix/193-skip-chase-on-loadres-failure
May 10, 2026
Merged

explorer: skip tryEnterPointModeIfNeeded chase on non-applied loadRes (closes #193)#196
rdhyee merged 1 commit intoisamplesorg:mainfrom
rdhyee:fix/193-skip-chase-on-loadres-failure

Conversation

@rdhyee
Copy link
Copy Markdown
Contributor

@rdhyee rdhyee commented May 10, 2026

Summary

Closes #193. Stops tryEnterPointModeIfNeeded() from being invoked after a loadRes call returns false, so the user-facing failure phase message (Failed to load H3 res${target} — try zooming again.) no longer flickers before being overpainted by the chase's own Fetching sample index….

Root cause

After PR #191, three external loadRes call sites unconditionally chased with await tryEnterPointModeIfNeeded():

await loadRes(target, ...);
await tryEnterPointModeIfNeeded();   // unconditional

If loadRes failed (caught error → returned false), it painted Failed to load H3 res${target} — try zooming again. But the immediate chase, if the user is at point altitude, fired its own loadRes(8, ...) which painted Fetching sample index… over the failure. The user saw the failure message for ~1 frame.

If loadRes was superseded (stale → also false), the chase was technically harmless but redundant — the supersedor's own chase recovers the user.

Fix

Capture the return into applied and gate the chase:

const applied = await loadRes(target, ...);
if (applied) await tryEnterPointModeIfNeeded();

Applied at the three external call sites:

  1. Source-filter handler's mode === 'cluster' branch
  2. Camera handler's cluster-reload branch (targetMode === 'cluster' && mode !== 'cluster')
  3. Camera handler's cluster-mode resolution-change branch (else branch)

Documentation

Updates the tryEnterPointModeIfNeeded() doc-block (added in #194 / #195) to:

  • Show the new idiom: const applied = await loadRes(...); if (applied) await tryEnterPointModeIfNeeded();
  • Explain why we gate on applied (one bullet per false-stale and false-failed case)
  • Update the verification-grep instructions to point reviewers at the new shape

Test plan

Refs

🤖 Generated with Claude Code

…closes isamplesorg#193)

When a chase site's loadRes returns `false` (stale or failed), the
post-await chase would overpaint the user-facing phase message — most
visibly the "Failed to load H3 res${target} — try zooming again."
failure copy that flickers for a frame before being replaced by
"Fetching sample index…" from the chase's own loadRes(8).

Capture loadRes's return into `applied` at the three external chase
sites and gate the chase on `if (applied)`:

  - Source-filter handler's mode==='cluster' branch
  - Camera handler's cluster-reload branch (cluster→cluster transition)
  - Camera handler's cluster-mode resolution-change branch

Stale recovery is preserved: when our call returns 'false-stale', the
supersedor's own chase site handles recovery (per the invariant
documented in isamplesorg#194). Failed returns no longer paper over the failure
message.

Update the INVARIANT block on tryEnterPointModeIfNeeded() to reflect
the new idiom (`const applied = await loadRes(...); if (applied) await
tryEnterPointModeIfNeeded();`) and explain why we gate on `applied`.
The verification grep continues to work — it lists the four awaited
call sites; manual confirmation now checks for the
`const applied = ...; if (applied) await tryEnterPointModeIfNeeded();`
shape rather than the unconditional chase.

Closes isamplesorg#193.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rdhyee
Copy link
Copy Markdown
Contributor Author

rdhyee commented May 10, 2026

Codex review round 1 — LGTM

Findings: None. I don't see a blocking counterexample.

Verified against HEAD on fix/193-skip-chase-on-loadres-failure:

  1. The stale-recovery claim is sound for the current code. A stale loadRes return means a newer loadRes incremented loadResGen; the only live external superseding sites are the three edited sites, and each now chases when its own load applies. The helper's internal res8 load will not start while another loadRes is in flight because of the !loading guard.
  2. Yes, this fixes the failure-message flicker path. loadRes paints the failure message and returns false; the three external callers now skip tryEnterPointModeIfNeeded() on that false, so they no longer immediately trigger the helper's "Fetching sample index…" path.
  3. The edits are complete. rg "await\s+loadRes\(" explorer.qmd shows one internal helper call and three external call sites — all three external sites have the new const applied = ...; if (applied) await tryEnterPointModeIfNeeded(); shape.
  4. The doc block is consistent with the code.
  5. No new issue introduced. The only non-recovery case left is an actual newest-load failure, which is intentional so the user can read the failure message.

git diff --check upstream/main..HEAD passed.

LGTM, ready to merge.

Merging.

@rdhyee rdhyee merged commit 29c77f2 into isamplesorg:main May 10, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

explorer: smooth failure-message flicker when tryEnterPointModeIfNeeded chases a failed cluster reload

1 participant