diff --git a/EXPLORER_STATE.md b/EXPLORER_STATE.md index 6567ba9..3a537d8 100644 --- a/EXPLORER_STATE.md +++ b/EXPLORER_STATE.md @@ -197,7 +197,12 @@ lookup. After Codex review on #165, we committed to a sharper third framing: data sources), but the *UI surface* gains a temporary point-overlay visualization of the matching samples. -### What "search" does today (`doSearch` at `:1782-1859`) +### What "search" did as of this document (`doSearch` at `:1782-1859`) — **HISTORICAL** + +> ⚠️ **Superseded by #234 and #340.** The paragraph below describes the original option-(B)/(C) +> behavior and is kept for provenance. It is NO LONGER accurate: search now DOES filter the map +> and table (#234 made it a global filter), and facet counts are no longer left at their +> unfiltered values — under an active search they render `(—)` (#340). See §7. Reads `#sampleSearch.value`. Runs an ILIKE-based DuckDB query against `lite_url` over `label` and `place_name`, with `sourceFilterSQL()` and @@ -620,10 +625,34 @@ The cross-filter rule (codified by Codex in #158, restated here): | dimension | counts respect it? | rationale | |-----------|--------------------|-----------| | other facet selections | **YES** | counts answer "if I add this value, how many samples would match all OTHER active filters plus this one"; that's the drill-out signal users want | -| viewport (camera bounds) | **NO** | counts are global. Viewport-scoped counts would couple facet UI to camera state, contradict the "facets describe the dataset" reading, and require re-querying on every camera change | -| `?search=` text query | **NO** | option (C); search renders a side panel + result-pin overlay, but does not alter facet counts | +| viewport (camera bounds) | **YES** (superseded #158) | counts ARE viewport-scoped when a bbox is active — `updateCrossFilteredCounts` always applies `viewerBboxSQL(...)` outside global view. The original "NO" below was overtaken by #234/#237 | +| `?search=` text query | **UNAVAILABLE** (superseded #158) | search does not produce cross-filtered counts; while a search is committed every count renders the `(—)` dash. See the #340 note below | | ~~view mode (globe vs table)~~ | _moot — mockup-v1 (#200) removed the Globe/Table toggle; both surfaces are permanent_ | — | +> **#340 (2026-08-04, grant closeout) — search makes counts UNAVAILABLE, not baseline.** +> The viewport and search rows were originally recorded as "NO" (the #158 contract). Both +> were already false in code by mid-2026: #234/#304/#305 retrofitted viewport- and +> search-awareness into the count paths without updating this table. The rows above have been +> corrected; this note records why. +> +> Eric Kansa re-reported the visible symptom in #340 — under `?search=pottery+Cyprus` +> (1,305 hits) every facet count still showed the unfiltered global total. +> +> The cause was NOT a missing predicate. The search-aware path exists, but the recompute +> **never reaches a terminal repaint**: nothing calls `applyFacetCounts` / +> `markFacetCountsUnavailable` / `markFacetCountsPending`, and `markFacetCountsRecomputing()` +> only adds a CSS class, so the previously-painted global numbers stay on screen. Instrumented +> live on prod: all 60 `.facet-count` elements still carried `.recomputing` 15s+ after the +> search completed cleanly, and a forced camera move did not clear it. The precise stall point +> was **not** diagnosed — see PLAN_305 Phase 3 for the post-grant probe list. +> +> Search-aware cross-filtered counts are **PLAN_305 Phase 3, deferred past the grant**. +> `refreshFacetCounts()` therefore bails to `markFacetCountsUnavailable()` whenever +> `searchIsActive()` — which also covers the concept / described-by filters, since they share +> the `search_pids` mechanism. This is the Honesty rule from PLAN_305 (never show baseline +> under an active filter) and answers Eric's own ask in #304: *"make sure the user interface +> does NOT show inaccurate numbers."* Clearing the search restores real counts. + Exposed via `applyFacetCounts(facetKey, countsMap)` (`:551-567`): - `countsMap = null` ⇒ render the baseline counts from `viewer._baselineCounts`. @@ -636,6 +665,13 @@ is selected; the general path (`:1586-1604`) issues four parallel `GROUP BY` queries against `facets_url`, one per dimension, each excluding its own active values from the WHERE. +> ⚠️ **Line references and the "four parallel queries" description above are stale.** +> Since #304/#305 the constrained full-tree path runs through the `sample_facet_index` +> bitmask index (`applyMaskIndexCounts`, a single `UNION ALL` histogram), with the tree +> cube and the legacy per-dimension queries kept as fast-path/fallback. Read +> `updateCrossFilteredCounts()` in `explorer.qmd` for the current routing; `PLAN_305_facet_counts.md` +> is the design record. + --- ## 8. Out of scope diff --git a/PLAN_305_facet_counts.md b/PLAN_305_facet_counts.md index 22f4c7a..4eb7d63 100644 --- a/PLAN_305_facet_counts.md +++ b/PLAN_305_facet_counts.md @@ -104,6 +104,33 @@ stale-guard check** (`facetCountsReqId`); never partially repaint dimensions. membership count implementation. (Immediate unification would balloon the first fix into viewport + search + flat-mode + stale-update refactoring — defer.) +> **STATUS 2026-08-04 (grant closeout) — Phase 3 SEARCH half is NOT done; now explicitly disabled.** +> Viewport landed. The search half was wired into `applyMaskIndexCounts`, but in the #340 +> reproduction the count recompute **never reaches a terminal repaint** under an active +> search — nothing calls `applyFacetCounts` / `markFacetCountsUnavailable` / +> `markFacetCountsPending`, so `markFacetCountsRecomputing()`'s class-only update leaves the +> previously-painted global baseline on screen (issue #340, reported by Eric Kansa). +> +> **What was actually instrumented** (prod, 2026-08-04): 60/60 `.facet-count` elements still +> carried `.recomputing` 15s+ after the search completed cleanly (1,305 hits, no console +> error), and a forced camera move — a fresh `refreshFacetCounts()` request — did not clear +> it. Reproduced on both deep-link and interactively-typed search, with all three facet trees +> confirmed rendered. **Not** established: the precise hang point. We did not prove the SQL +> itself never resolves, only that no repaint ever happened. +> +> Because the grant period has ended we did not diagnose further. Top Risk #1 in this doc +> (WASM latency / connection starvation) remains the leading suspect. Instead +> `refreshFacetCounts()` now short-circuits to `markFacetCountsUnavailable()` whenever +> `searchIsActive()`, honoring the **Honesty rule** above (never baseline under an active +> filter). Note `searchIsActive()` also covers the concept / described-by producer, so those +> filters show the dash too — consistent, since they share the `search_pids` mechanism. +> +> **To finish Phase 3 post-grant:** remove that short-circuit, then determine where the +> search-constrained count path stalls. Suggested first probes: `window.__facetIndexStatus`, +> in-flight DuckDB query count at the time of the stall, whether the search-constrained form +> of the `UNION ALL` in `applyMaskIndexCounts` resolves in isolation, and whether serializing +> it against concurrent search/table/point queries makes it settle. + ### Phase 4 — Verification + cleanup - Unit-test predicate generation (exclude-self, zero-source states). - Data validator gates from Phase 1, run in CI. diff --git a/explorer.qmd b/explorer.qmd index 1923b60..02f7b59 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -1612,9 +1612,21 @@ function markFacetCountsUnavailable(facetKeys = null) { el.classList.add('count-unavailable'); el.title = 'Count unavailable for this filter combination'; }); + clearStaleZeroRows(key); } } +// #340: `.zero` is set by applyFacetCounts from a PREVIOUS, authoritative +// count. Once a count becomes unknown (unavailable OR still loading), leaving +// the row greyed-out as "0" asserts a number we just admitted we don't have — +// the same misleading-number problem in a different costume. A later real +// count restores the correct state via applyFacetCounts' classList.toggle. +function clearStaleZeroRows(facetKey) { + document.querySelectorAll(`.facet-row[data-facet="${facetKey}"].zero`).forEach(row => { + row.classList.remove('zero'); + }); +} + // #313 P0: the facet index (sample_facet_index) is still loading — distinct // from markFacetCountsUnavailable's "this session genuinely can't compute it" // dash. On a slow connection the cold-boot preflight can take ~20-80s; until @@ -1631,6 +1643,7 @@ function markFacetCountsPending(facetKeys = null) { el.classList.remove('count-unavailable'); // NOT the "—" dash — this is honest "still loading" el.removeAttribute('title'); }); + clearStaleZeroRows(key); // #340: same invariant — unknown never implies zero } } @@ -4426,7 +4439,30 @@ zoomWatcher = { function refreshFacetCounts() { clearTimeout(facetCountsDebounce); const myReq = ++facetCountsReqId; + // #340 (grant closeout): under a committed search the count recompute + // NEVER settles — verified live on prod 2026-08-04. The search+bbox arm + // of applyMaskIndexCounts leaves its promise pending, so nothing ever + // calls applyFacetCounts / markFacetCountsUnavailable / -Pending, and + // markFacetCountsRecomputing()'s class-only repaint leaves the PREVIOUS + // (unfiltered, global) numbers on screen indefinitely. That is exactly + // the misleading baseline #304 forbids and Eric asked us to never show. + // + // Search-aware cross-filtered counts are PLAN_305 Phase 3, which is not + // being finished in the grant period. So we stop attempting them and + // apply the honesty rule directly: an active search means counts are + // unavailable. Bailing BEFORE the debounce also invalidates in-flight + // requests via ++facetCountsReqId above, so a late-settling older query + // cannot overwrite the dash. Clearing the search takes the normal path + // again and restores real counts. + if (searchIsActive()) { + markFacetCountsUnavailable(); + return; + } facetCountsDebounce = setTimeout(() => { + // Re-check at FIRE time, not just schedule time: search can go active + // during the 250 ms window (the producers call us on activation, but + // this keeps the invariant local and true regardless of caller). + if (searchIsActive()) { markFacetCountsUnavailable(); return; } updateCrossFilteredCounts(myReq); }, 250); } @@ -5944,6 +5980,11 @@ zoomWatcher = { const total = cnt.length ? Number(cnt[0].n) : 0; if (token !== _searchFilterToken) return false; window.__searchFilter = { active: true, term, token, total, kind: 'text' }; + // #340: paint the unavailable dash the instant search goes active, and + // invalidate any refresh that was scheduled while it was still inactive + // (that one would otherwise fire mid-flight and re-enter the never- + // settling search+bbox count path). See refreshFacetCounts(). + refreshFacetCounts(); window.a1dbg?.('search-build-end', { term, token, total }); return true; } finally { @@ -5985,6 +6026,7 @@ zoomWatcher = { const total = cnt.length ? Number(cnt[0].n) : 0; if (token !== _searchFilterToken) return false; window.__searchFilter = { active: true, term: label, token, total, kind: 'concept', uri }; + refreshFacetCounts(); // #340: see buildSearchFilter / refreshFacetCounts() window.a1dbg?.('search-build-end', { term: label, token, total, kind: 'concept' }); return true; } finally { @@ -6149,6 +6191,7 @@ zoomWatcher = { ignoredCommon: plan.ignoredCommon, expectedBytes: plan.expectedBytes, }; + refreshFacetCounts(); // #340: see buildSearchFilter / refreshFacetCounts() window.a1dbg?.('substrate-search-end', { term, token, total, mode: plan.mode, ignored: plan.ignoredCommon }); return true;