explorer: collapse dual mode state to single source of truth (#208 step 1)

[rdhyee]

Step 1 of the #208 state-management refactor. Foundational.

Problem

The zoomWatcher OJS cell carried explorer mode in two places:

• closure-private `let mode = 'cluster';`
• public `viewer._globeState.mode`

Both were updated together (`mode = 'point'; viewer._globeState.mode = 'point';`), but the duplication caused transient sync mismatches during boot. The URL round-trip harness captures this: `waitReadyAndPointMode` succeeds against `_globeState.mode` then a snapshot moments later sees a brief cluster state — the harness's iter 1 "mode A=cluster B=point" anomaly.

Codex's retrospective on PR #203 (item 7) recommended making `_globeState.mode` the single source of truth and wrapping transitions in one setter.

What this changes

• `getMode()` getter reads from `viewer._globeState.mode`. All 10 former bare-`mode` reads in the zoomWatcher cell go through it: `tryEnterPointModeIfNeeded`, source-filter handler, facet-filter handler, camera-changed handler (`targetMode` fallback + two mode transitions + stats guard), hashchange post-flight reconciliation (both branches).
• `setExplorerMode(next)` setter is the only writer. Used by `enterPointMode` and `exitPointMode`, which previously did the redundant `mode = X; viewer._globeState.mode = X;` pair.
• Closure `let mode = 'cluster';` removed.

20 insertions, 16 deletions. No behavioral changes intended — just collapses the duplication.

Verification

• `tests/playwright/url_roundtrip_investigation.js` — 5/6 iterations green on localhost.
• The iter 1 anomaly ("mode A=cluster B=point" on no-op flight) is the same pre-existing boot-transition timing Codex flagged in the original retrospective; it's neither caused nor worsened by this refactor. Centralized camera reconciliation in step 2 will fix it.

Risks

• Refactor that touches every mode-check site. Each call site verified individually; grep confirms no residual bare-`mode` reads outside the unrelated `setView` function's parameter, comments, and string literals.
• `getMode()` adds a function-call indirection on every mode check. Negligible cost; readability win.
• Boot mode anomaly NOT fixed by this PR. It's foundational for step 2 (route boot + camera + hashchange through one `reconcileCameraState` path), which IS the fix.

Sequencing reminder

Per Codex's recommended sequence (in #208):

1. ✅ Collapse `mode` to single source — THIS PR
2. Introduce `writeGlobeHash` + `setExplorerMode` and migrate the writers
3. Route both `camera.changed` and `camera.moveEnd` through one `reconcileCameraState(reason)` settled-camera path

Each step gets its own PR with Codex review.

Test plan

• url_roundtrip_investigation.js 5/6 green (iter 1 pre-existing anomaly)
• Manual smoke on deploy preview: cluster→point→cluster transitions, source filter toggle, hashchange back/forward
• Codex review

🤖 Generated with Claude Code

[rdhyee]

Codex review — round 1

Verbatim Codex review (no shared context). LGTM, ready to merge in a single round.

One factual correction noted: Codex counted 11 reads (not 10 as the PR description says). The commit message lists "10 former bare-mode reads in the zoomWatcher cell" — minor count error, doesn't change anything material.

---

Summary

No blocking issues found. The refactor does collapse runtime mode state to viewer._globeState.mode, and I found no missed code-path reads/writes inside zoomWatcher.

One correction: the author’s count is off. I count 11 reads + 2 writes, not 10 reads + 2 writes.

Per-ask response

1. Completeness

Complete. In explorer.qmd, old runtime mode reads were all replaced with getMode().

I count 11 reads:

• tryEnterPointModeIfNeeded: 2
• source filter handler: 1
• facet filter handler: 2
• camera handler: 4
• hashchange reconciliation: 2

Writes are both routed through setExplorerMode() in enterPointMode and exitPointMode.

2. Initialization Gap

Fine. explorer.qmd initializes _globeState before zoomWatcher exists, so routing that through setExplorerMode() is not available without moving/reshaping ownership. If future setter side effects matter, initialization should probably become a separate explicit initialization API, not call into this cell.

3. Behavioral Preservation

Preserved for #alt=8000&mode=point. Before: let mode = 'cluster' existed at zoomWatcher entry. After: getMode() reads viewer._globeState.mode, initialized to 'cluster' before zoomWatcher runs. First boot call therefore sees the same initial value.

4. External Writes

I found no external _globeState.mode writes. Across explorer.qmd, mode is initialized once as part of _globeState, then written only through setExplorerMode().

5. PR #214 Spec Interaction

Confirmed. Specs polling viewer._globeState.mode now observe the same state that zoomWatcher branches on. Under the old code, tests could observe public mode while closure-private mode temporarily disagreed.

6. getMode() Cost

No concern. It is one function created at cell init and called from event-driven handlers, not a tight render loop. The camera handler is debounced.

7. Other

No subtle ordering or OJS reactivity issue found. Since this is mutation of a property on an existing object, it does not depend on OJS re-running cells; the relevant consumers are event handlers and buildHash(viewer) reads.

Verdict

LGTM, ready to merge