Skip to content

explorer: heatmap mutual-exclusivity (phase 3) + self-refresh loop fix (#233) #242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rdhyee merged 2 commits into from
May 28, 2026
Merged

explorer: heatmap mutual-exclusivity (phase 3) + self-refresh loop fix (#233) #242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0d2747
explorer: heatmap mutually exclusive with marker layers (#233 phase 3)
rdhyee May 28, 2026
8759bcf
explorer: stop heatmap self-refresh loop with tolerance dedupe (#233)
rdhyee May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 152 additions & 18 deletions explorer.qmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,20 +888,26 @@ function hasFacetFilters() {
// takes effect at neighborhood zoom") explains the cluster-mode honesty
// gap: H3 summary parquets only carry `dominant_source`, so material /
// context / object_type filters cannot affect cluster counts. Invariant:
// visible ⇔ (any facet active) ∧ (mode === 'cluster')
// Call sites that mutate either side of the conjunction MUST call this
// to keep DOM and state in agreement:
// visible ⇔ (any facet active) ∧ (mode === 'cluster') ∧ (heatmap off)
// Call sites that mutate any conjunct MUST call this to keep DOM and
// state in agreement:
// - facetFilters cell (URL deep-link restore, #234 step 1)
// - handleFacetFilterChange (user toggles a facet checkbox)
// - enterPointMode / exitPointMode (mode transitions)
// `viewer` resolves late at call time per ojs reactive scoping, so this
// helper is safe to define before the viewer cell runs.
// - heatmapToggle change handler (#233 phase 3)
// The heatmap conjunct: when the heatmap overlay is on, it IS the
// filter-aware density view (dots are generated from filtered data, not
// pre-baked dominant_source), so the apology is false and must be hidden
// (#233). The toggle state is read straight off the DOM rather than via
// heatmapEnabled() so this helper stays reachable from cells that run
// before the viewer cell defines that function.
function syncFacetNote() {
const el = document.getElementById('facetNote');
if (!el) return;
const active = hasFacetFilters();
const inCluster = (typeof viewer !== 'undefined') && viewer._globeState?.mode === 'cluster';
el.style.display = (active && inCluster) ? 'block' : 'none';
const heatOn = document.getElementById('heatmapToggle')?.checked === true;
el.style.display = (active && inCluster && !heatOn) ? 'block' : 'none';
}

function escSql(value) {
Expand Down Expand Up @@ -2562,11 +2568,31 @@ zoomWatcher = {
}
}

// Single source of truth for marker-layer visibility (#233 phase 3).
//
// Cluster dots (h3Points) and individual sample points (samplePoints)
// are mutually exclusive with the heatmap overlay. When the heatmap is
// on it IS the density view, so painting the cluster dots on top of it
// produces the dots-vs-hotspots disagreement RY flagged 2026-05-27 —
// two layers telling contradictory spatial stories at once. Rule:
// heatmap on ⇒ both marker collections hidden (heatmap stands alone)
// heatmap off ⇒ show whichever collection the altitude-driven mode
// calls for (cluster→h3Points, point→samplePoints)
// Aside from the one-time initializer (samplePoints starts hidden at
// creation), this is the ONLY place that writes `.show` on the two
// collections. Call it from every site that flips the mode or the
// heatmap toggle.
function applyLayerVisibility() {
const heat = heatmapEnabled();
const mode = getMode();
viewer.h3Points.show = !heat && mode === 'cluster';
viewer.samplePoints.show = !heat && mode === 'point';
}

// --- Mode transitions ---
function enterPointMode(pushHistory) {
setExplorerMode('point');
viewer.h3Points.show = false;
viewer.samplePoints.show = true;
applyLayerVisibility();
if (pushHistory !== false) history.pushState(null, '', buildHash(viewer));
// #facetNote is only meaningful in cluster mode (#234 step 1).
syncFacetNote();
Expand All @@ -2576,9 +2602,8 @@ zoomWatcher = {

function exitPointMode(pushHistory) {
setExplorerMode('cluster');
viewer.samplePoints.show = false;
viewer.samplePoints.removeAll();
viewer.h3Points.show = true;
applyLayerVisibility();
if (pushHistory !== false) history.pushState(null, '', buildHash(viewer));
// Returning to cluster mode: surface the honesty note if any
// facet filter is active (#234 step 1).
Expand Down Expand Up @@ -2857,6 +2882,17 @@ zoomWatcher = {
let heatmapReqId = 0;
let heatmapDebounce = null;
let heatmapLastKey = null;
// Last SUCCESSFULLY-rendered view + filter (#233 loop fix, 2026-05-28).
// refreshHeatmap() dedupes against these with a tolerance so that
// sub-meaningful viewport jitter does NOT schedule a re-render. Set on
// render success; cleared in clearHeatmap(), the render-error path, and
// refreshHeatmap()'s no-bounds path. DELIBERATELY NOT cleared on
// moveStart (unlike heatmapLastKey): retaining the last-success snapshot
// across a moveStart/moveEnd cycle is what lets terrain/inertia jitter
// be measured against the committed overlay and ignored, instead of
// re-arming the loop. See heatmapViewMeaningfullyChanged().
let heatmapLastBounds = null;
let heatmapLastFilterHash = null;
const HEATMAP_CANVAS_SIZE = 512;

function heatmapEnabled() {
Expand Down Expand Up @@ -2949,10 +2985,63 @@ zoomWatcher = {
return `${bbox}:${heatmapFilterHash()}`;
}

// Normalized center + span of a viewport rectangle, antimeridian-aware
// (mirrors renderHeatmap's `eastNorm` shift). Used by the tolerance
// dedupe below.
function heatmapViewMetrics(b) {
const eastNorm = b.west > b.east ? b.east + 360 : b.east;
return {
cLat: (b.north + b.south) / 2,
cLng: (b.west + eastNorm) / 2,
spanLat: Math.max(1e-9, b.north - b.south),
spanLng: Math.max(1e-9, eastNorm - b.west),
};
}

// Did the view change enough to warrant a fresh heatmap render?
//
// #233 loop fix (RY 2026-05-28): the heatmap re-renders on `moveEnd`,
// keyed off `computeViewRectangle`. But that rectangle is sensitive to
// tiny camera-height changes, and with Cesium World Terrain enabled the
// camera height keeps settling for a beat after a move (terrain-collision
// adjustment as tiles stream in) — plus inertial drift after mouse-up.
// Each tiny settle fired `moveEnd` → re-render → the render's frame
// nudged terrain again → another `moveEnd`: a self-sustaining refresh
// loop with no user input. Exact-key equality (`toFixed(4)`, ~11 m) let
// that jitter through.
//
// Fix: only re-render when the view center or span moved by more than a
// small fraction of the current span. Real pans/zooms blow past this;
// terrain-settle / inertial micro-jitter stays under it and is ignored.
const HEATMAP_VIEW_TOLERANCE = 0.02; // 2% of span
// Shortest angular distance between two longitudes (degrees), result in
// [0, 180]. Without this, a wrapped bbox near the antimeridian (cLng≈180
// via the eastNorm +360 shift) compared to a settled non-wrapped bbox
// (cLng≈-177) would read as a ~357° "move" and wrongly pass the
// meaningful-change test — preserving the loop exactly at the dateline.
// Codex review 2026-05-28.
function lngDelta(a, b) {
let d = Math.abs(a - b) % 360;
return d > 180 ? 360 - d : d;
}
function heatmapViewMeaningfullyChanged(curr, prev) {
if (!prev) return true;
const a = heatmapViewMetrics(curr);
const b = heatmapViewMetrics(prev);
const tolLat = a.spanLat * HEATMAP_VIEW_TOLERANCE;
const tolLng = a.spanLng * HEATMAP_VIEW_TOLERANCE;
return Math.abs(a.cLat - b.cLat) > tolLat
|| lngDelta(a.cLng, b.cLng) > tolLng
|| Math.abs(a.spanLat - b.spanLat) > tolLat
|| Math.abs(a.spanLng - b.spanLng) > tolLng;
}

function clearHeatmap() {
++heatmapReqId;
clearTimeout(heatmapDebounce);
heatmapLastKey = null;
heatmapLastBounds = null;
heatmapLastFilterHash = null;
setHeatmapStatus('');
if (heatmapImageryLayer) {
viewer.imageryLayers.remove(heatmapImageryLayer, true);
Expand Down Expand Up @@ -3064,6 +3153,11 @@ zoomWatcher = {
}
heatmapImageryLayer = nextLayer;
heatmapLastKey = key; // success-only — see refreshHeatmap()
// Snapshot the view + filter this render committed to, for the
// tolerance dedupe in refreshHeatmap() (#233 loop fix). Same
// success-only discipline as heatmapLastKey.
heatmapLastBounds = bounds;
heatmapLastFilterHash = heatmapFilterHash();
const refreshedAt = Date.now();
// With SQL pre-aggregation, every sample in the bbox is counted
// into its pixel cell — no more arbitrary LIMIT cap. `capped` is
Expand All @@ -3085,9 +3179,11 @@ zoomWatcher = {
if (myReq !== heatmapReqId) return;
console.warn('Heatmap refresh failed:', err);
setHeatmapStatus('Heatmap unavailable for this view.');
// Clear dedupe key so a retry on the same (viewport, filter)
// Clear dedupe state so a retry on the same (viewport, filter)
// actually re-attempts the render. Codex round-1 review of #240.
heatmapLastKey = null;
heatmapLastBounds = null;
heatmapLastFilterHash = null;
// Codex round-2 review of #240: also remove the prior imagery
// layer on failure. Without this, the user sees the OLD heatmap
// density alongside the "Heatmap unavailable" status — a UI lie
Expand Down Expand Up @@ -3123,6 +3219,8 @@ zoomWatcher = {
++heatmapReqId;
clearTimeout(heatmapDebounce);
heatmapLastKey = null;
heatmapLastBounds = null;
heatmapLastFilterHash = null;
if (heatmapImageryLayer) {
viewer.imageryLayers.remove(heatmapImageryLayer, true);
heatmapImageryLayer = null;
Expand All @@ -3138,13 +3236,41 @@ zoomWatcher = {
return;
}
const key = heatmapKey(bounds);
if (key === heatmapLastKey) return;
// NOTE: heatmapLastKey is set ONLY after a successful layer swap in
// renderHeatmap(), and cleared on error/cancellation. Setting it here
// would let a moveStart-cancellation between the debounce schedule
// and the actual render leave the dedupe key set without a render
// having happened — the next moveEnd then early-returns and the
// overlay is wedged. Codex round-1 review of PR #240.
// Tolerance dedupe (#233 loop fix, RY 2026-05-28). Re-render only on
// a filter change OR a meaningful view change. Sub-meaningful
// viewport jitter (terrain-collision height settling, inertial
// micro-drift) keeps the same filter and stays within the view
// tolerance, so it is ignored here instead of driving a render that
// would nudge terrain and sustain the loop. The previous exact-key
// equality (`toFixed(4)`, ~11 m) let that jitter through.
const filterHash = heatmapFilterHash();
if (filterHash === heatmapLastFilterHash
&& !heatmapViewMeaningfullyChanged(bounds, heatmapLastBounds)) {
// View only jittered (terrain settle / inertia); the existing
// overlay is still correct. A moveStart may have flipped the
// status to "waiting for camera" — restore the truthful
// "rendered" line so it doesn't get stuck mid-loop. Gate on
// heatmapLastBounds (set only on a successful render) rather than
// a truthy count, so a valid ZERO-sample overlay also restores
// its status instead of sticking at "waiting" (Codex review).
if (heatmapLastBounds) {
const n = Number(viewer._heatmapOverlay?.lastPointCount || 0);
setHeatmapStatus(`Heatmap rendered from ${n.toLocaleString()} samples.`);
}
// Test-only signal: count tolerance skips so the regression test
// can PROVE the skip branch ran (a real moveEnd reached here),
// rather than relying on the vacuous "lastRefreshAt unchanged"
// assertion (Codex review 2026-05-28).
viewer._heatmapSkips = (viewer._heatmapSkips || 0) + 1;
return;
}
// NOTE: heatmapLast{Key,Bounds,FilterHash} are set ONLY after a
// successful layer swap in renderHeatmap(), and cleared on
// error/cancellation. Setting them here would let a moveStart-
// cancellation between the debounce schedule and the actual render
// leave the dedupe state set without a render having happened — the
// next moveEnd then early-returns and the overlay is wedged. Codex
// round-1 review of PR #240.
clearTimeout(heatmapDebounce);
const myReq = ++heatmapReqId;
heatmapDebounce = setTimeout(() => {
Expand All @@ -3158,8 +3284,16 @@ zoomWatcher = {
} else {
clearHeatmap();
}
// #233 phase 3: heatmap is mutually exclusive with the marker
// layers, so flip dot visibility and re-evaluate the facet apology
// note (which is meaningless once the heatmap shows filtered
// density directly). Both helpers read live state, so order vs the
// refresh/clear above does not matter.
applyLayerVisibility();
syncFacetNote();
});
viewer._heatmapOverlay = { enabled: false, layer: null, lastRefreshAt: 0, lastPointCount: 0, lastKey: null };
viewer._heatmapSkips = 0; // tolerance-dedupe skip counter (test signal)

// --- Busy-flag depth counter (#173 review round 2) ---
//
Expand Down
Loading
Loading