From c40eacf9a8324bd19d4bc2255fd7dd854b7a98fb Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Tue, 16 Jun 2026 10:22:28 -0700 Subject: [PATCH] #208 PR4b: shared settled-camera tail via reconcileSettledCamera() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the shared settled-camera reconciliation both globe listeners run once the camera comes to rest — the cluster "Samples in View" stat refresh + the URL-hash write (writeGlobeHash) — into reconcileSettledCamera(v), and call it from BOTH camera.changed and moveEnd (#208 smell 1b). This gives moveEnd the same cluster-stat refresh camera.changed already did, closing the sub-10%-pan gap: a small cluster-mode drag fired moveEnd (which updated the URL via #204) but NOT camera.changed (debounced away by percentageChanged=0.1), leaving the "Samples in View" count stale. Scope is deliberately minimal (Codex Q3 / REFACTOR_PR4_PLAN.md §3): NOT the full handler merge. Mode-transition + resolution-reload stays camera.changed- only; facet/heatmap/point-exit stays moveEnd-only; #262 stays a separate tracked sibling. reconcileSettledCamera is a local fn (closes over getMode/currentRes/countInViewport), not top-level like writeGlobeHash. Behavior: - camera.changed: behavior-neutral (same order — cluster-stat then hash). - moveEnd: adds the cluster-stat refresh (point mode skips it via the getMode()==='cluster' guard, so point mode is unchanged). The stat read is synchronous, guarded by _clusterData, and writes no mode/selection/URL/ facet/heatmap state. Verification: smoke 4 + characterization 7 + url-roundtrip 5 all green (behavior-neutral URL contract from both handlers preserved); render clean; Codex review of the diff found no blocking issues. A dedicated headless regression for the moveEnd cluster-stat refresh proved unreliable (OJS cell re-evaluation yields multiple viewer instances in the harness; the one reachable at interaction time often has no camera listeners) — documented inline in url-roundtrip.spec.js rather than shipped flaky. Co-Authored-By: Claude Opus 4.8 (1M context) --- explorer.qmd | 57 +++++++++++++++++++------- tests/playwright/url-roundtrip.spec.js | 15 +++++++ 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/explorer.qmd b/explorer.qmd index 73c8c0ee..4c4d9bae 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -3679,6 +3679,37 @@ zoomWatcher = { document.getElementById('contextFilterBody').addEventListener('change', handleFacetFilterChange); document.getElementById('objectTypeFilterBody').addEventListener('change', handleFacetFilterChange); + // --- Shared settled-camera tail (#208 smell 1b) --- + // The single reconciliation entry point both settled-camera listeners run + // once the camera has come to rest: refresh the cluster-mode "Samples in + // View" stat for the current viewport, then write the URL hash. Extracted + // so `moveEnd` runs the SAME cluster-stat refresh that `camera.changed` + // does — closing the sub-10%-pan gap where a small drag in cluster mode + // updated the URL (moveEnd fires) but left the "Samples in View" count + // stale (camera.changed's `percentageChanged=0.1` debounce didn't fire). + // + // Scope is deliberately minimal (Codex Q3, REFACTOR_PR4_PLAN.md §3): this + // touches NEITHER the mode-transition / resolution-reload logic (which + // stays camera.changed-only) NOR the facet/heatmap/point-exit logic (which + // stays moveEnd-only). The cluster-stat read is synchronous and cheap + // (counts already-loaded `_clusterData` against the padded viewport) and + // no-ops unless we're in cluster mode with data loaded. + function reconcileSettledCamera(v) { + // Cluster-mode viewport count only — point mode shows its own count via + // loadViewportSamples(). Padded bbox so the cluster "Samples in View" + // stat matches the samples table row total (issue #221 round 2). + if (getMode() === 'cluster' && v._clusterData) { + const inView = countInViewport(paddedViewportBounds(VIEWPORT_PAD_FACTOR)); + const total = v._clusterTotal; + if (total) { + updateStats(`H3 Res${currentRes}`, `${inView.clusters.toLocaleString()} / ${total.clusters.toLocaleString()}`, inView.samples.toLocaleString(), null, 'Clusters in View / Loaded', 'Samples in View'); + } + } + // _suppressHashWrite-gated default — the hashchange-flight path stays + // unaffected (same gate the two raw writers honored before). + writeGlobeHash(v); + } + // --- Camera change handler --- let timer = null; viewer.camera.changed.addEventListener(() => { @@ -3756,20 +3787,11 @@ zoomWatcher = { } } - // Update viewport cluster count (cluster mode only; point mode - // already shows viewport count). Padded bbox so the cluster - // "Samples in View" stat matches the samples table row total - // (issue #221 round 2). - if (getMode() === 'cluster' && viewer._clusterData) { - const inView = countInViewport(paddedViewportBounds(VIEWPORT_PAD_FACTOR)); - const total = viewer._clusterTotal; - if (total) { - updateStats(`H3 Res${currentRes}`, `${inView.clusters.toLocaleString()} / ${total.clusters.toLocaleString()}`, inView.samples.toLocaleString(), null, 'Clusters in View / Loaded', 'Samples in View'); - } - } - - // Update URL hash (replaceState for continuous movement) - writeGlobeHash(viewer); + // Settled-camera tail: cluster "Samples in View" stat (cluster + // mode only; point mode already shows viewport count) + URL-hash + // replaceState for continuous movement. Shared with `moveEnd` via + // reconcileSettledCamera() (#208 smell 1b). + reconcileSettledCamera(viewer); }, 600); }); viewer.camera.percentageChanged = 0.1; @@ -3833,7 +3855,12 @@ zoomWatcher = { }); viewer.camera.moveEnd.addEventListener(() => { - writeGlobeHash(viewer); + // Settled-camera tail shared with `camera.changed` (#208 smell 1b): + // URL-hash write + cluster "Samples in View" refresh. moveEnd fires on + // every discrete settle including sub-10% pans that `camera.changed` + // debounces away, so routing through here keeps the cluster stat in + // lockstep with the URL (which moveEnd already kept fresh via #204). + reconcileSettledCamera(viewer); // B1: viewport-aware facet counts. Bouncing through refreshFacetCounts // reuses the existing 250ms debounce + facetCountsReqId stale-guard, // so bursts of moveEnd (drag-pan, wheel-zoom) coalesce into one query diff --git a/tests/playwright/url-roundtrip.spec.js b/tests/playwright/url-roundtrip.spec.js index 55bfdb6e..d7184b36 100644 --- a/tests/playwright/url-roundtrip.spec.js +++ b/tests/playwright/url-roundtrip.spec.js @@ -298,4 +298,19 @@ test.describe('Explorer URL state round-trip (issue #209)', () => { const s = await snapshot(page); expect(s.selectedH3).toBeNull(); }); + + // NOTE (PR4b, #208 smell 1b): a dedicated headless regression for the new + // `moveEnd` cluster "Samples in View" refresh was attempted but proved + // unreliable. The explorer OJS cell re-evaluates repeatedly during headless + // boot, so `_ojs...value('viewer')` returns different `viewer` instances + // across calls — the one reachable at interaction time frequently has zero + // camera listeners attached, so a forced `moveEnd.raiseEvent()` (or `flyTo`) + // never reaches reconcileSettledCamera. A flaky test being worse than none, + // PR4b instead rests on: (1) the existing url-roundtrip + characterization + // suite proving behavior-neutrality of the shared URL write from BOTH the + // camera.changed and moveEnd handlers; (2) the change being mechanically + // trivial — `moveEnd` now invokes the IDENTICAL cluster-stat block that + // `camera.changed` already ran (covered by cluster-mode boot in the + // characterization specs); (3) a manual probe confirming a settled cluster + // camera writes " | Samples in View" via the shared tail. });