explorer: facet counts use padded viewport, matching the table (#234)#245
Open
rdhyee wants to merge 1 commit into
Open
explorer: facet counts use padded viewport, matching the table (#234)#245rdhyee wants to merge 1 commit into
rdhyee wants to merge 1 commit into
Conversation
…lesorg#234) Facet-legend counts were computed over the EXACT viewport (pad 0) in updateCrossFilteredCounts, while every other "in view" surface — the samples-table COUNT (loadCount), the point-mode sample loader, the "samples in view" stat, and the heatmap — pads by VIEWPORT_PAD_FACTOR (0.3). A matching sample sitting in the 30% pad margin was counted by the table but not the legend, so the legend read one (or more) low. RY hit it 2026-05-28: material=mineral at a Cyprus deep-zoom showed 13 on the legend but "14 samples match the current filters" in the table. Verified against the live data: the facet query returns 13 at pad 0 and 14 at pad 0.3 (== the table). Fix: align the facet-count bbox to VIEWPORT_PAD_FACTOR. The heatmap hit the identical mismatch earlier and was moved to the padded contract in the isamplesorg#241 follow-up; this aligns the last stray surface. (The durable fix — one shared "in view" bbox source so these can't drift again — is tracked on isamplesorg#234.) Tests (tests/playwright/facet-viewport.spec.js): - New coherence regression: active-material legend count == table "N match" count at the exact repro view (backreference assertion). - Hardened flyToAndSettle: the padded (slower) facet query reliably exposed a pre-existing race — the helper waited only for `.recomputing` to clear, which can be a transient before the new counts apply. It now keys on the full recompute cycle (`.recomputing` appear → clear) plus a two-consecutive-equal-reads stability gate, so it's correct even for moves that leave counts unchanged. Confirms there is no real product stale-overwrite race (the restore settles on global). - Full B1 suite green 2x each (10/10). Provenance: Claude; empirically verified (DuckDB 13/14 + Playwright); Codex 2-round review (resolved the unchanged-counts hang in the helper). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rdhyee
added a commit
that referenced
this pull request
Jun 18, 2026
…#292) The Material tree shipped with STATIC global baseline counts; this makes them live when the user zooms in. Map/table filtering was already live — this is the legend. - describeCrossFilters: Material participates via materialSelection() (minimal nodes) ONLY when zoomed (!isGlobalView); at/near global it stays baseline. - buildCrossFilterWhere: Material cross-filters OTHER dims via a membership pid-subquery (concept_uri IN selected) — a selected parent matches its subtree. - updateCrossFilteredCounts: cube disabled in tree mode (no tree nodes in the cube); Material own-counts at global → baseline (instant); zoomed → membership query COUNT(DISTINCT pid) per concept_uri, scoped by bbox (lite JOIN) + other dims (facets_v3 pid-subquery) + search. GLOBAL GATE (perf): the membership COUNT(DISTINCT) is a near-full scan at global/ large views and starved the single DuckDB-WASM connection (samples-table query timed out). At true-global the baseline IS the correct global count, so we use it (instant); live counts engage when zoomed, where the bbox prunes the scan. Verified (202608): earthmaterial 4,091,133 global → 25,988 at Cyprus; legend(node) == table(node filter) = 26,310 (coherence #245); parent>=child in-viewport; source⇄ material cross-filter both directions. 8 facet-tree specs + smoke green; render clean. Codex: no blocking findings (verified cross-filter via direct DuckDB queries). Accepted residual (documented): at global view WITH another filter/search, Material counts show baseline (instant, slightly stale) — a precomputed facet_tree_cross_filter cube is the follow-up if live global cross-filtered material counts are wanted. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Facet-legend counts were computed over the exact viewport (pad 0) in
updateCrossFilteredCounts, while every other "in view" surface — the samples-tableCOUNT(loadCount), the point-mode sample loader, the "samples in view" stat, and the heatmap — pads the viewport byVIEWPORT_PAD_FACTOR(0.3). A sample sitting in that 30% margin was counted by the table but not the facet legend, so the legend read one (or more) low.Reproduction:
/explorer.html?material=…/mineral#v=1&lat=35.0900&lng=32.8900&alt=50000&mode=point→ the table says "14 samples match the current filters" but the mineral facet shows 13.Verified against live data (DuckDB queries at that exact view): the facet query returns 13 at pad 0 and 14 at pad 0.3 (== the table). Exactly one mineral sample lives in the pad margin.
Fix
One line in
updateCrossFilteredCounts— align the facet-count bbox to the shared contract:The heatmap hit the identical mismatch earlier and was moved to the padded contract in the #241 follow-up; this aligns the last stray surface.
Tests (
tests/playwright/facet-viewport.spec.js)==table "N match" count at the repro view (backreference assertion —14/14passes,13/14fails).flyToAndSettle: the padded (slower) facet query reliably exposed a pre-existing race — the helper waited only for.recomputingto clear, which can be a transient before the new counts apply. It now keys on the full recompute cycle (.recomputingappear → clear) plus a two-consecutive-equal-reads stability gate, so it's correct even for moves that leave counts unchanged. This also confirms there is no real product stale-overwrite race — the restore settles correctly on global.Provenance
Claude; empirically verified (DuckDB 13/14 + Playwright); Codex 2-round review (resolved an unchanged-counts hang in the test helper). Deployed to the rdhyee fork staging and verified live (mineral legend now reads 14, matching the table).
🤖 Generated with Claude Code