tests: Playwright spec for tryEnterPointModeIfNeeded short-circuits (closes #192)

[rdhyee]

Summary

Closes #192. Adds tests/playwright/explorer-helper.spec.js covering the deterministically observable short-circuit invariants of tryEnterPointModeIfNeeded() (introduced in #191).

Coverage

Test	Status	What it asserts
boot to cluster mode reaches a done message	✓ passing	Phase-1 cluster load reaches "${n} clusters, ${m} samples." done state.
source-filter toggle at world altitude does NOT trigger Fetching sample index	✓ passing	Invariant (b) altitude short-circuit. The source-filter handler chases the helper after its own loadRes settles, and at world altitude the chase MUST bail at the altitude check without painting "Fetching sample index…".
deep-link to point altitude reaches a sample-points done message	⊘ skipped	Documented future work — see below.
source-filter toggle in point mode does NOT trigger Fetching sample index	⊘ skipped	Documented future work — see below.

Why two tests are skipped

Both require the Cesium camera at point altitude (alt < 120 km). In headless Chromium the scene.postRender event that applies the deep-link camera position doesn't drive the camera-changed handler the same way as a real WebGL display: the camera stays at world altitude and the point-mode logic never fires. (Confirmed during development — visited the alt=62054 deep-link in a headless browser, observed phase-message progression stopped at the cluster-mode done state and never advanced.)

The tests are kept (with detailed .skip() reasons inline) as living documentation. Two paths forward for a follow-up:

1. Run them in headed mode (npm run test:headed against the same preview server). Slower, requires display in CI, but the camera flies correctly.
2. Add a small test-only hook that exposes viewer on window (closure-private currently), so the spec can drive viewer.camera.flyTo() directly and the camera handler fires deterministically. This is a one-line change to explorer.qmd.

Either is out of scope for this PR — the issue's acceptance asks for "at least one Playwright spec covering items 1, 3, and 4," and the altitude-short-circuit test alone already covers an item-1 short-circuit invariant deterministically.

Approach

Cell-internal state (mode, currentRes, loading, viewer) is closure-private inside an OJS cell and is NOT exposed on window. The deterministically observable signal from outside the closure is #phaseMsg's textContent, which updatePhaseMsg(...) rewrites at every state transition. Tests install a MutationObserver on #phaseMsg, capture history during the action, then assert presence/absence of expected loading-message phrases.

Test plan

# Terminal 1: preview server
quarto preview explorer.qmd --no-browser --port 5860

# Terminal 2: tests
npm run test -- tests/playwright/explorer-helper.spec.js

Expected output:

✓ boot to cluster mode reaches a done message with cluster counts (2.1s)
✓ source-filter toggle at world altitude does NOT trigger Fetching sample index (4.0s)
- deep-link to point altitude reaches a sample-points done message
- source-filter toggle while in point mode does NOT trigger Fetching sample index
2 skipped, 2 passed (~7s)

• Both passing tests run in <10s combined locally
• Skipped tests have inline .skip() reason explaining why and how to enable
• Spec is purely additive — no production-code changes

Refs

• explorer: Playwright coverage for tryEnterPointModeIfNeeded() invariants #192 — this issue
• explorer: surface 'Fetching sample index…' during cold-cache boot→point-mode wait (#190 fix 2) #191 — introduces tryEnterPointModeIfNeeded()
• explorer: document the 'every loadRes caller chases with tryEnterPointModeIfNeeded' invariant #194 / explorer: document loadRes-chase invariant on tryEnterPointModeIfNeeded (closes #194) #195 — invariant doc-block this spec exercises
• explorer: smooth failure-message flicker when tryEnterPointModeIfNeeded chases a failed cluster reload #193 / explorer: skip tryEnterPointModeIfNeeded chase on non-applied loadRes (closes #193) #196 — the conditional-chase fix that makes the altitude-short-circuit assertion meaningful

🤖 Generated with Claude Code

[rdhyee]

Codex review round 1 — addressed in 11a06cd

Both blocking findings addressed.

Codex's pointer to the OJS runtime works

"A better available hook may be Quarto's OJS runtime: window._ojs.ojsConnector.mainModule.value('viewer') can likely retrieve viewer without production changes…"

Confirmed locally — the runtime does expose the viewer cell value. From there, viewer.camera.flyTo(...) drives Cesium directly. This eliminates the need for a production-side test hook.

Why the previous attempt didn't work (the real root cause)

Cesium's default requestRenderMode = true renders on demand only. A single setView + requestRender doesn't reliably fire camera.changed in headless chromium because the render loop suspends after idle. Setting scene.requestRenderMode = false keeps the loop running, and an animated flyTo produces a stream of postRender events that exceed percentageChanged and trigger the camera-changed handler.

(My earlier debug script "worked" because it ran during the still-warming-up render window. By the time the spec waited for cluster boot, the render loop had suspended.)

Result

All four tests pass:

✓ boot to cluster mode reaches a done message with cluster counts (2.0s)
✓ source-filter toggle at world altitude does NOT trigger Fetching sample index (5.6s)    [invariant b]
✓ flying camera into point altitude triggers helper and enters point mode (12.1s)         [camera-handler call site]
✓ source-filter toggle while in point mode does NOT trigger Fetching sample index (10.3s) [invariant a]

4 passed (30.4s)

Coverage now meets #192's acceptance: invariants (a) and (b) are deterministically exercised, plus an end-to-end smoke test of the camera-handler call site that drives the helper into a real point-mode entry. Invariant (c) (!loading short-circuit) still requires timing injection and is left for a future PR.

Also fixed

• Tightened the point-mode done-message assertion. The earlier substring match on "individual samples" was satisfied by the intermediate "Loading individual samples..." loading state. Now waits for the count-prefixed terminal form via regex.

Re-requesting review.

[rdhyee]

Codex review round 2 — addressed in b21f7e2

Codex's blocking finding

"waitForPhaseMessage() only checks the current #phaseMsg text. In the cluster test, CLUSTERS_DONE is already present before the toggle, so the post-toggle waits can resolve against stale text. … a future regression that paints Fetching sample index... after the reload/chase completes could be missed."

Confirmed locally: with the previous shape, the negative assertion would pass even if the chase call were removed entirely.

Fix

Three changes in the spec, all driven by Codex's recommendation to make waits transition-aware:

1. New waitForLoadingThenDone(page, loadingSub, donePattern) helper

Searches the captured _phaseHistory for loadingSub followed by a later entry matching donePattern (string or RegExp). Replaces the earlier waitForPhaseMessage(page, CLUSTERS_DONE) shape that could match pre-action text.

const CLUSTER_DONE_RE = /\d[\d,]*\s+clusters,/;
const POINT_DONE_RE = /\d[\d,]*\s+individual\s+samples/;

await toggleSourceFilter(page, 0);
await waitForLoadingThenDone(page, LOADING_H3_RES, CLUSTER_DONE_RE);

This guarantees the assertion runs against history that includes the new transition, not the captured-at-startup pre-action state.

2. New toggleSourceFilter(page, index) helper

Switched from Playwright's uncheck/check({force:true}) (which skip actionability but don't reliably fire change in this layout — input inside a <label>) to direct DOM dispatch:

cb.checked = !cb.checked;
cb.dispatchEvent(new Event('change', { bubbles: true }));

Plain click() works in isolation but is flaky when run after another test (Cesium widget transiently occludes the checkbox during its boot). Direct dispatch is robust under both conditions.

3. waitForClusterBoot now also awaits the zoomWatcher cell

The earlier sync point — phase-1 done message — fires before the source-filter / camera-changed event listeners are registered (those happen inside the zoomWatcher cell, which depends on phase1). Without this second wait, a test that dispatches change right after the done message can race the listener registration and silently no-op.

The fix uses Codex's own discovery — Quarto's OJS runtime exposes cell values:

await page.evaluate(async () => {
  return await window._ojs.ojsConnector.mainModule.value('zoomWatcher');
});

zoomWatcher returns "active" after registering all listeners; mainModule.value(...) resolves once the cell body completes. Reliable sync without adding a production hook.

Verification

Run twice end-to-end against quarto preview explorer.qmd:

✓ boot to cluster mode reaches a done message with cluster counts (2.5s)
✓ source-filter toggle at world altitude does NOT trigger Fetching sample index (8.1s)
✓ flying camera into point altitude triggers helper and enters point mode (12.3s)
✓ source-filter toggle while in point mode does NOT trigger Fetching sample index (15.5s)

4 passed (38.8s)

The negative assertions are now meaningful: a regression that paints Fetching sample index... after the toggle's loading→done cycle would now be visible in _phaseHistory and fail the test.

Re-requesting review.

[rdhyee]

Codex review round 3 — LGTM

No blocking issues found.

1. Yes. waitForLoadingThenDone() closes the round-2 weakness: it cannot pass on the initial already-painted done text because it requires a matching loading entry first, then a later done entry in _phaseHistory. A regression that paints Fetching sample index… after the observed toggle cycle should be captured by the phase observer and fail the final negative assertions.
2. No blocking new wait weakness. The one caveat is non-blocking: waitForLoadingThenDone() searches from the first matching loading entry, so it is not intrinsically scoped to "after this action." The current retoggle call sites compensate with a history-length guard, and that is sound with the current synchronous dispatchEvent path because the handlers paint their loading state before the first await.
3. The zoomWatcher sync point is sound for these tests. The cell registers the source-filter and camera handlers before returning "active". It could only hang if the OJS cell or one of its dependencies never resolves; in that case the test should fail by timeout rather than silently race listener registration.

LGTM, ready to merge.

Merging.