From 08083ac4f77c61327aa636f32db5402a3acc20e7 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 14:36:07 -0700 Subject: [PATCH 01/15] pin-overlay: implementation plan (option C), amended per Codex round 1 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- PLAN_PIN_OVERLAY_2026-07-17.md | 82 ++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 PLAN_PIN_OVERLAY_2026-07-17.md diff --git a/PLAN_PIN_OVERLAY_2026-07-17.md b/PLAN_PIN_OVERLAY_2026-07-17.md new file mode 100644 index 0000000..6830998 --- /dev/null +++ b/PLAN_PIN_OVERLAY_2026-07-17.md @@ -0,0 +1,82 @@ +# Pin-overlay implementation plan (option C, #164/#165 → #172 §7 track) + +*2026-07-17, RY green-light "start now". Strategy: same playbook that shipped FTS +this morning — additive increment first, behavior change behind a flag, staging +verify, flip on confirmation.* + +## Context deltas since the contract was written (May) + +1. **Search is now a global filter** (#234 A1): `search_pids` semi-joins constrain + table, points, facet counts. The contract's "facet counts exclude search" clause + describes the *target* state, not today's. Users have 6+ weeks of habit with + filter-everything behavior. +2. **Area/world scope already exists** (#178 landed): `?search_scope`, two-button UI, + viewport-scoped SQL. Contract's "Light-path addendum" is DONE — do not rebuild. +3. **FTS is the default backend** (#335, today). Contract explicitly says (C) is + backend-orthogonal — confirmed still true (both producers emit `search_pids`). + +## Increments + +### Inc 1 — additive pin overlay (no behavior removed) ← TODAY +After any search completes, render matching samples as a pin overlay per the +contract's non-negotiable rules: +- `viewer.searchResultPoints` = new `PointPrimitiveCollection`, added to + `scene.primitives` AFTER `h3Points`/`samplePoints` (z-order rule) +- ≤ 50 pins (displayed result set), hollow-ring styling, `SOURCE_COLORS` palette, + pixel size 8 +- hover label + click → `updateSampleCard()` + pid hash write (dispatch on + `meta.type === 'searchResult'`) +- lifecycle: populate on results; clear on input-clear / `?search=` removal / new search +- camera: fit-to-bounds iff extent < 30°×30°, else fly-to-top-1 @ 200000; + auto-fly suppressed in area scope (existing behavior) +- global refiltering UNCHANGED in Inc 1 — pins are purely additive +- acceptance: the contract's four shape cases (zero / one / local-many / global-many) +- **pin-count semantics (amended per review):** one pin per *located* displayed + result (world scope legitimately returns coordinate-less rows — never coerce to + (0,0)); panel reports displayed vs pinned counts when they differ; camera rules + use the first *located* result +- **click parity (amended):** extract the sample-click ceremony (freshness token, + card, selection/hash, detail hydration) into a shared helper with a + preserve-results-list option; pin clicks use it — no shallow duplicate branch +- **lifecycle (amended):** clearing tied to the *committed* clear paths (empty + submit / URL search removal / new search), not draft input edits +- **styling (amended):** carry `scaleByDistance` + `disableDepthTestDistance` + explicitly (global-many visibility) + +### Inc 2 — `?searchui=pin` mode (the latency win) ← behind flag +**Seam corrected per Codex plan review (P0):** skipping `applySearchFilterChange()` +is NOT sufficient — `window.__searchFilter.active` is consulted independently by +searchFilterSQL() (table/export/points/heatmap/facets), computeTargetMode(), +table messaging, heatmap cache keys, and camera moveEnd refreshes. The seam is a +single semantic predicate **`searchFiltersSurfaces()`** (false only for +`kind === 'text' && searchui === 'pin'`; concept/described-by filtering stays +global), used consistently by every consumer. Skipping the direct call then +becomes a latency optimization, not the correctness mechanism. +- expected effect: search response ≈ filter-build time (~1-5s cold) instead of + +10-20s of surface refiltering — this is what makes #172 §7 winnable +- table/facets/clusters keep their pre-search state (facet counts stay + search-independent, per original #158 contract) +- clear messaging in panel: "N matches pinned on globe" replaces "N of N in view" +- default stays current behavior until team confirms (Andrea/Eric habit risk); + the eventual flip is a real change (tests/docs/rollback/telemetry), not 1 line — + `searchui=pin|filter` recorded as a temporary rollout flag (deliberate, + documented deviation from the "URL params unchanged" clause) +- URL: `searchui=pin` round-trips like `fts` param (state contract: no new hash state) + +### Inc 3 — NOT in scope +Scope buttons (done), mockup-v1 sidebar mirror (#200 — parked per closure triage). + +## Verification +- 4 acceptance cases on staging (fork Pages dispatch from branch, as this morning) +- e2e spec additions: pins count == min(50, results); pins cleared on clear; + `?searchui=pin` skips refilter (assert facet counts unchanged after search) +- Codex loop: plan review (round 1 DONE 7/17 — P0 seam corrected, this doc amended) + → implement → per-increment review rounds → LGTM gate +- honest sizing (Codex): Inc 1 ≈ 1–2 days · Inc 2 ≈ 2–4 days · staging ≈ 1 day + +## Files +- `explorer.qmd`: viewer cell (~:773 area) for collection; `doSearch` result path + for populate/clear; mouse/click handlers for pick dispatch; search-complete path + for Inc 2 skip +- `tests/playwright/pin-overlay.spec.js` (new) + From 321925d1f877184defdc32468c0d397415ea60a1 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 14:48:43 -0700 Subject: [PATCH 02/15] feat(explorer): additive search-result pin overlay (#172 Inc 1, option C) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After any committed search, render the located displayed results as a distinct hollow-ring pin overlay on the globe, independent of the H3 cluster and sample-point layers. Purely additive — global filtering, facet counts, and camera behavior are unchanged in Inc 1. - viewer cell: viewer.searchResultPoints PointPrimitiveCollection added to scene.primitives AFTER h3Points/samplePoints (z-order rule); always shown. window.__searchPins() test hook exposes the live pin count. - hover: result pins label like sample points (meta.label || meta.pid). - click: new meta.type === 'searchResult' branch runs the shared selectSearchResult() ceremony (freshness token, card, selection state, flight, lazy detail) but PRESERVES the #samplesSection results list; like the sample branch it opens the in-map card and writes the pid hash. - selectSearchResult(sample, {fly, inMapCardAt}) extracts the former search-row click ceremony; both the row click and the pin click use it (no shallow duplicate). Row-click behavior is unchanged. - doSearch: populate one pin per LOCATED displayed result (skip null lat/lng — never coerce to (0,0)); cap inherited from LIMIT 50. Count line discloses "(M located, pinned on globe)" when located < displayed. - lifecycle: clear the overlay on committed clear paths only — new/empty submit (top of doSearch), clearActiveSearchFilter, and a committed concept filter (doDescribedBy). Draft input edits never clear. Camera fit-to-bounds is deferred to Inc 2 per the amended plan. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- explorer.qmd | 205 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 155 insertions(+), 50 deletions(-) diff --git a/explorer.qmd b/explorer.qmd index 36599ac..382dbcb 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -2174,6 +2174,17 @@ viewer = { v.scene.primitives.add(v.samplePoints); v.samplePoints.show = false; // hidden until point mode + // #172 pin-overlay Inc 1 (option C): search-result pin overlay. Added to + // scene.primitives AFTER h3Points and samplePoints so it renders ABOVE both + // (z-order rule). Always shown — the overlay is independent of the + // cluster/point mode toggle. + v.searchResultPoints = new Cesium.PointPrimitiveCollection(); + v.scene.primitives.add(v.searchResultPoints); + + // Test hook (#172): lets Playwright assert the live pin count without + // reaching into Cesium internals. + if (typeof window !== 'undefined') window.__searchPins = () => v.searchResultPoints.length; + // Hover tooltip — works for both clusters and samples v.pointLabel = v.entities.add({ label: { @@ -2191,7 +2202,8 @@ viewer = { v.pointLabel.position = picked.primitive.position; v.pointLabel.label.show = true; const meta = picked.id; - if (typeof meta === 'object' && meta.type === 'sample') { + if (typeof meta === 'object' && (meta.type === 'sample' || meta.type === 'searchResult')) { + // #172 pin-overlay Inc 1: result pins hover like sample points. v.pointLabel.label.text = `${meta.label || meta.pid}`; } else if (typeof meta === 'object' && meta.count) { v.pointLabel.label.text = `${meta.source}: ${meta.count.toLocaleString()} samples`; @@ -2216,7 +2228,19 @@ viewer = { const meta = picked.id; const isStale = freshSelectionToken(v); - if (typeof meta === 'object' && meta.type === 'sample') { + if (typeof meta === 'object' && meta.type === 'searchResult') { + // --- Search-result pin click (#172 pin-overlay Inc 1, option C) --- + // Runs the same selection ceremony as a sample click (card, + // selection state, flight, lazy detail) via the shared helper, but + // PRESERVES the #samplesSection search-results list the user is + // browsing. Like the sample branch it also opens the floating in-map + // card at the click pixel and writes the pid selection hash. + if (v.selectSearchResult) { + v.selectSearchResult(meta, { fly: true, inMapCardAt: { x: e.position.x, y: e.position.y } }); + } + writeGlobeHash(v, { replace: false, force: true }); + + } else if (typeof meta === 'object' && meta.type === 'sample') { // --- Individual sample click --- updateSampleCard(meta); // In-map floating card (issue #226). `e.position` is the @@ -5941,6 +5965,9 @@ zoomWatcher = { async function clearActiveSearchFilter() { // Same clear path as an empty search submit, without entering doSearch(). _searchSeq++; + // #172 pin-overlay Inc 1 (option C): committed clear (concept-chip clear + // / programmatic ?search= removal) drops the result-pin overlay too. + if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); if (searchInput) searchInput.value = ''; if (searchResults) searchResults.textContent = ''; await clearSearchFilter(); @@ -5977,6 +6004,55 @@ zoomWatcher = { if (clusterEl) clusterEl.hidden = searchIsActive(); } + // #172 pin-overlay Inc 1 (option C): shared selection ceremony for a search + // result, invoked by BOTH the side-panel row click (in doSearch) and the + // on-globe result-pin click (viewer cell, via viewer.selectSearchResult). + // Mirrors the on-globe sample-click ceremony — freshness token, card + // hydration, selection state, flight, lazy detail — but deliberately does + // NOT repaint #samplesSection (it holds the search-results list the user is + // browsing). `sample` is a normalized meta ({ pid, label, source, lat, lng, + // place_name, result_time }); coord-less rows are ignored. `inMapCardAt` + // (pin click only) additionally opens the floating in-map card at the click + // pixel, parity with the sample-click branch. + async function selectSearchResult(sample, { fly = true, inMapCardAt = null } = {}) { + if (!sample || sample.lat == null || sample.lng == null) return; + + // Bump the freshness token BEFORE any async work so a prior + // cluster/sample detail load can't repaint the panel after we navigate. + const isStale = freshSelectionToken(viewer); + + viewer._globeState.selectedPid = sample.pid; + viewer._globeState.selectedH3 = null; + updateSampleCard(sample); + if (inMapCardAt) showInMapCard(sample, inMapCardAt); + if (fly) { + viewer.camera.flyTo({ + destination: Cesium.Cartesian3.fromDegrees(sample.lng, sample.lat, 50000), + duration: 1.5 + }); + } + + // Lazy-load full description (mirrors the globe click handler). Don't + // clear samplesSection — it holds the results the user is browsing. + try { + const detail = await db.query(` + SELECT description + FROM read_parquet('${wide_url}') + WHERE pid = '${sample.pid.replace(/'/g, "''")}' + LIMIT 1 + `); + if (isStale()) return; + if (detail && detail.length > 0) updateSampleDetail(detail[0]); + else updateSampleDetail({ description: '' }); + } catch(err) { + if (isStale()) return; + console.error("Search-result detail query failed:", err); + updateSampleDetail(null); + } + } + // Expose for the viewer cell's globe click handler (result-pin click). + viewer.selectSearchResult = selectSearchResult; + async function doSearch(scope) { if (scope === 'area' || scope === 'world') _searchScope = scope; const effectiveScope = _searchScope; @@ -5992,6 +6068,13 @@ zoomWatcher = { // submission — valid or not — supersedes prior in-flight work. const searchId = ++_searchSeq; + // #172 pin-overlay Inc 1 (option C): clear the previous result-pin + // overlay on every committed submit (before repopulating). This covers + // both the new-search path (repopulated below on success) and the + // empty/too-short committed-clear path. Draft input edits never reach + // doSearch, so typing/backspace does not clear pins. + if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + const term = searchInput.value.trim(); if (!term || term.length < 2) { searchResults.textContent = 'Type at least 2 characters'; @@ -6306,9 +6389,18 @@ zoomWatcher = { // bare result-row count visible while the precise total // resolves. See issue #232. const capReached = results.length === 50; - searchResults.textContent = capReached + // #172 pin-overlay Inc 1 (option C): one pin per LOCATED displayed + // result (world scope legitimately returns coord-less matches). When + // fewer results are located than displayed, the count line discloses + // how many landed on the globe. Cap is inherited from LIMIT 50, so + // located ≤ displayed ≤ 50. + const locatedCount = results.filter(r => r.latitude != null && r.longitude != null).length; + const pinnedNote = locatedCount < results.length + ? ` (${locatedCount} located, pinned on globe)` + : ''; + searchResults.textContent = (capReached ? `${results.length}+ results for "${term}"` - : `${results.length} results for "${term}"`; + : `${results.length} results for "${term}"`) + pinnedNote; // Show results in the samples panel const sampEl = document.getElementById('samplesSection'); @@ -6336,58 +6428,64 @@ zoomWatcher = { // click ceremony at line ~944. Without the full ceremony // a slow prior selection load could repaint the panel // after the click, leaving UI inconsistent with URL. + // #172 pin-overlay Inc 1 (option C): that ceremony now lives in + // the shared selectSearchResult() helper, used by BOTH this row + // click and the on-globe result-pin click. Row click keeps its + // prior behavior (no in-map card, no selection-hash write). const resultsByPid = new Map(results.map(s => [s.pid, s])); sampEl.querySelectorAll('.sample-row[data-lat]').forEach(row => { - row.addEventListener('click', async (e) => { + row.addEventListener('click', (e) => { if (e.target.tagName === 'A') return; // let links work const pid = row.dataset.pid; - const sample = pid ? resultsByPid.get(pid) : null; - if (!sample || sample.latitude == null || sample.longitude == null) return; - - // Bump the freshness token BEFORE any async work so - // a prior cluster/sample detail load can't repaint - // the panel after we navigate. - const isStale = freshSelectionToken(viewer); - - viewer._globeState.selectedPid = pid; - viewer._globeState.selectedH3 = null; - updateSampleCard({ - pid: sample.pid, - label: sample.label, - source: sample.source, - lat: sample.latitude, - lng: sample.longitude, - place_name: sample.place_name, - result_time: sample.result_time - }); - viewer.camera.flyTo({ - destination: Cesium.Cartesian3.fromDegrees(sample.longitude, sample.latitude, 50000), - duration: 1.5 - }); - - // Lazy-load full description (mirrors the globe - // click handler). Don't clear samplesSection here — - // it currently holds the search results the user - // is browsing. - try { - const detail = await db.query(` - SELECT description - FROM read_parquet('${wide_url}') - WHERE pid = '${pid.replace(/'/g, "''")}' - LIMIT 1 - `); - if (isStale()) return; - if (detail && detail.length > 0) updateSampleDetail(detail[0]); - else updateSampleDetail({ description: '' }); - } catch(err) { - if (isStale()) return; - console.error("Search-row detail query failed:", err); - updateSampleDetail(null); - } + const s = pid ? resultsByPid.get(pid) : null; + if (!s) return; + selectSearchResult({ + pid: s.pid, + label: s.label, + source: s.source, + lat: s.latitude, + lng: s.longitude, + place_name: s.place_name, + result_time: s.result_time + }, { fly: true }); }); }); } + // #172 pin-overlay Inc 1 (option C): render the LOCATED displayed + // results as a pin overlay on the globe. One hollow-ring pin per + // located row — skip null lat/lng (world scope legitimately returns + // coordinate-less matches; NEVER coerce to (0,0)). Old pins were + // cleared at the top of doSearch (committed-clear path); the cap is + // inherited from LIMIT 50, so pins ≤ 50 ≤ displayed results. + const pinScalar = new Cesium.NearFarScalar(1e2, 8, 2e5, 3); // mirror samplePoints + for (const s of results) { + if (s.latitude == null || s.longitude == null) continue; + const c = SOURCE_COLORS[s.source] || '#666'; + viewer.searchResultPoints.add({ + id: { + type: 'searchResult', + pid: s.pid, + label: s.label, + source: s.source, + lat: s.latitude, + lng: s.longitude, + place_name: s.place_name, + result_time: s.result_time + }, + position: Cesium.Cartesian3.fromDegrees(s.longitude, s.latitude, 0), + pixelSize: 8, + // Hollow ring: faint fill + bright source-colored outline so + // the overlay reads as distinct from the filled cluster and + // sample dots while still signalling which source matched. + color: Cesium.Color.fromCssColorString(c).withAlpha(0.15), + outlineColor: Cesium.Color.fromCssColorString(c), + outlineWidth: 2, + scaleByDistance: pinScalar, + disableDepthTestDistance: Number.POSITIVE_INFINITY, + }); + } + // Fly to the first result. Skip for area-scoped searches — // the user is already at the area they care about; flying // would zoom in and disorient. @@ -6467,9 +6565,11 @@ zoomWatcher = { // When `total === results.length`, the cap happened to // catch exactly the full set — drop the "of N" to // avoid a redundant "50 of 50 results" reading. - searchResults.textContent = total > results.length + // #172: preserve the located/pinned disclosure across the + // real-count follow-up (pinnedNote is captured above). + searchResults.textContent = (total > results.length ? `${results.length} of ${total.toLocaleString()} results for "${term}"` - : `${results.length} results for "${term}"`; + : `${results.length} results for "${term}"`) + pinnedNote; } catch(err) { countMs = performance.now() - countStart; if (searchId !== _searchSeq) { @@ -6609,6 +6709,11 @@ zoomWatcher = { // filter can't clobber each other's side-panel render mid-flight. const searchId = ++_searchSeq; + // #172 pin-overlay Inc 1 (option C): committing a concept filter removes + // `search=` and owns the panel, so drop any lingering text-search + // result pins (a committed clear path for the text overlay). + if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + // Mutual exclusivity: a committed concept filter OWNS search_pids, so // clear the free-text box (and its sidebar mirror) before building. if (searchInput) searchInput.value = ''; From 0cda4e980eb293355f7e967e1b9e1a051b0ce0c3 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 14:48:51 -0700 Subject: [PATCH 03/15] test(playwright): e2e for search-result pin overlay (#172 Inc 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Models tests/playwright/fts-v1.spec.js conventions (BASE env, explorer.html boot, wait on the samples-table, drive #sampleSearch + #searchSubmitBtn, poll via window.__searchPins()). Cases: local-many (pottery Cyprus → pins in (0,50] and <= displayed), zero (no-hit → 0 pins), new-search replaces the pin set, and committed empty search clears the pins. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- tests/playwright/pin-overlay.spec.js | 104 +++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 tests/playwright/pin-overlay.spec.js diff --git a/tests/playwright/pin-overlay.spec.js b/tests/playwright/pin-overlay.spec.js new file mode 100644 index 0000000..a2654d2 --- /dev/null +++ b/tests/playwright/pin-overlay.spec.js @@ -0,0 +1,104 @@ +// #172 pin-overlay Inc 1 (option C): after a search completes the located +// results are rendered as a temporary pin overlay on the globe +// (viewer.searchResultPoints), independent of the H3 cluster / sample-point +// layers. These specs assert the overlay's population, cap, replacement, and +// committed-clear lifecycle end-to-end against the published data. +// +// Run: BASE_URL=https://rdhyee.github.io/isamplesorg.github.io \ +// npx playwright test tests/playwright/pin-overlay.spec.js +// +// Test hook: window.__searchPins() returns viewer.searchResultPoints.length +// (set in the viewer cell). Ground truths reuse the FTS suite's 202608 index +// facts: 'pottery Cyprus' → 1,305 matches; 'basalt' → 785 — both far exceed +// the LIMIT 50 display cap, so the displayed set (and thus the pin set) is +// capped at 50. + +const { test, expect } = require('@playwright/test'); + +const BASE = process.env.BASE_URL || 'https://rdhyee.github.io/isamplesorg.github.io'; +const URL = `${BASE}/explorer.html`; + +// Generous flake protection matching the FTS suite: a cold CDN edge pays boot +// + lazy module import + sidecar/shard fetches before the first search lands. +test.describe.configure({ retries: process.env.CI ? 1 : 0 }); + +const pins = (page) => page.evaluate(() => (window.__searchPins ? window.__searchPins() : -1)); +const rowPids = (page) => + page.$$eval('#samplesSection .sample-row', (els) => els.map((e) => e.dataset.pid)); + +async function submitSearch(page, term) { + await page.fill('#sampleSearch', term); + await page.locator('#searchSubmitBtn').first().click(); +} + +test.describe('search-result pin overlay (#172 Inc 1)', () => { + test.describe.configure({ timeout: 120_000 }); + + test.beforeEach(async ({ page }) => { + await page.goto(URL, { timeout: 90_000 }); + await page.waitForSelector('.samples-table tbody tr[data-pid]', { timeout: 120_000 }); + // Pin overlay starts empty before any search. + expect(await pins(page)).toBe(0); + }); + + test('local-many: pottery Cyprus pins located results, capped at 50', async ({ page }) => { + await submitSearch(page, 'pottery Cyprus'); + // Pins populate synchronously right after the result rows render. + await page.waitForFunction(() => (window.__searchPins ? window.__searchPins() : 0) > 0, + null, { timeout: 120_000 }); + const pinCount = await pins(page); + const displayed = (await rowPids(page)).length; + expect(pinCount).toBeGreaterThan(0); + expect(pinCount).toBeLessThanOrEqual(50); + // One pin per LOCATED displayed result — coord-less rows are skipped, so + // pins never exceed the displayed set. + expect(pinCount).toBeLessThanOrEqual(displayed); + }); + + test('zero: a no-hit query renders no pins', async ({ page }) => { + await submitSearch(page, 'xyzzyqqqplugh'); + await page.waitForFunction(() => + /no results|no samples|couldn't build/i.test( + (document.getElementById('searchResults')?.textContent || '') + + (document.getElementById('samplesSection')?.textContent || '')), + null, { timeout: 120_000 }); + expect(await pins(page)).toBe(0); + }); + + test('new search replaces the pin set', async ({ page }) => { + await submitSearch(page, 'pottery Cyprus'); + await page.waitForFunction(() => (window.__searchPins ? window.__searchPins() : 0) > 0, + null, { timeout: 120_000 }); + const firstPids = await rowPids(page); + expect(await pins(page)).toBeGreaterThan(0); + + await submitSearch(page, 'basalt'); + // Wait until the count line reflects the NEW term, then for pins to repopulate. + await page.waitForFunction(() => + /results for "basalt"/i.test(document.getElementById('searchResults')?.textContent || ''), + null, { timeout: 120_000 }); + await page.waitForFunction(() => (window.__searchPins ? window.__searchPins() : 0) > 0, + null, { timeout: 120_000 }); + const secondPids = await rowPids(page); + const secondCount = await pins(page); + + expect(secondCount).toBeGreaterThan(0); + expect(secondCount).toBeLessThanOrEqual(50); + // Disjoint corpora → the displayed (and thus pinned) set must change. + expect(secondPids.join(',')).not.toBe(firstPids.join(',')); + }); + + test('committed empty search clears the pins', async ({ page }) => { + await submitSearch(page, 'pottery Cyprus'); + await page.waitForFunction(() => (window.__searchPins ? window.__searchPins() : 0) > 0, + null, { timeout: 120_000 }); + expect(await pins(page)).toBeGreaterThan(0); + + // Empty submit is a committed clear path — doSearch clears the overlay + // before the too-short early return. + await submitSearch(page, ''); + await page.waitForFunction(() => (window.__searchPins ? window.__searchPins() : -1) === 0, + null, { timeout: 60_000 }); + expect(await pins(page)).toBe(0); + }); +}); From 69f8506c281aafa259bfc6151df8bb37b15fbbd3 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 15:10:14 -0700 Subject: [PATCH 04/15] fix(explorer): address Codex round-1 P1.1/P1.2/P1.3/P1.5 + P2 for pin overlay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - P1.1 depth test: pins now use the bounded POINT_DEPTH_TEST_DISTANCE instead of Number.POSITIVE_INFINITY, which the file documents as causing back-hemisphere bleed-through (#185). The hover-label tooltip keeps Infinity (a tooltip should not be depth-tested) — unchanged. - P1.2 lifecycle (snapshot semantics): the overlay is a snapshot of the rendered results list and reconciles with the LIST lifecycle, not filter state. Added clears where the globe sample/cluster clicks tear down the list; documented the policy at the populate site; corrected the clearActiveSearchFilter comment (its only caller is the concept-chip clear button). Source/facet toggles while a search is active leave list+pins stale by design. - P1.3 in-map card parity: pin clicks now get the FULL in-map detail hydration (material/feature/specimen/thumbnail) by reusing openInMapCardForSample() (extended with an optional anchor pixel) instead of a description-only fetch — no duplicated query. Removed the double freshness bump: selectSearchResult takes the caller's token when provided. - P1.5 camera: fly to the first LOCATED result with explicit null checks (0° coords valid); results[0] can be coord-less in world scope. Fit-to-bounds stays deferred to Inc 2. - P2 hover: source-colored hover-label background for both sample points and search-result pins (contract's "source badge color"); cluster/plain hovers restore the default background. - P1.4a test hook: window.__searchPins() now returns a guarded read-only [{pid,lat,lng}] snapshot (re-reads the collection per call; [] if missing). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- explorer.qmd | 123 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 98 insertions(+), 25 deletions(-) diff --git a/explorer.qmd b/explorer.qmd index 382dbcb..c1f250e 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -2181,9 +2181,21 @@ viewer = { v.searchResultPoints = new Cesium.PointPrimitiveCollection(); v.scene.primitives.add(v.searchResultPoints); - // Test hook (#172): lets Playwright assert the live pin count without - // reaching into Cesium internals. - if (typeof window !== 'undefined') window.__searchPins = () => v.searchResultPoints.length; + // Test hook (#172, Codex round-1 P1.4 / P2): read-only snapshot of the pin + // overlay — [{ pid, lat, lng }] — so specs can assert identity/position and + // exact set equality, not just a count. Re-reads the collection on every + // call via the current `v`; returns [] if the collection is missing so a + // recreated viewer never leaves a stale closure raising. + if (typeof window !== 'undefined') window.__searchPins = () => { + const col = v.searchResultPoints; + if (!col) return []; + const out = []; + for (let i = 0; i < col.length; i++) { + const id = col.get(i).id || {}; + out.push({ pid: id.pid, lat: id.lat, lng: id.lng }); + } + return out; + }; // Hover tooltip — works for both clusters and samples v.pointLabel = v.entities.add({ @@ -2195,6 +2207,9 @@ viewer = { disableDepthTestDistance: Number.POSITIVE_INFINITY, text: "", } }); + // Cesium's documented default label background; restored for hovers that + // carry no source (#172 P2 source-colored hover, below). + const LABEL_BG_DEFAULT = new Cesium.Color(0.165, 0.165, 0.165, 0.8); new Cesium.ScreenSpaceEventHandler(v.scene.canvas).setInputAction((movement) => { const picked = v.scene.pick(movement.endPosition); @@ -2202,14 +2217,21 @@ viewer = { v.pointLabel.position = picked.primitive.position; v.pointLabel.label.show = true; const meta = picked.id; + // #172 P2 (Codex round-1): source-colored hover background for both + // sample points AND search-result pins, per the option (C) contract + // ("source badge color"). Cluster/plain hovers keep the default. + let hoverSource = null; if (typeof meta === 'object' && (meta.type === 'sample' || meta.type === 'searchResult')) { - // #172 pin-overlay Inc 1: result pins hover like sample points. v.pointLabel.label.text = `${meta.label || meta.pid}`; + hoverSource = meta.source; } else if (typeof meta === 'object' && meta.count) { v.pointLabel.label.text = `${meta.source}: ${meta.count.toLocaleString()} samples`; } else { v.pointLabel.label.text = String(meta); } + v.pointLabel.label.backgroundColor = (hoverSource && SOURCE_COLORS[hoverSource]) + ? Cesium.Color.fromCssColorString(SOURCE_COLORS[hoverSource]).withAlpha(0.85) + : LABEL_BG_DEFAULT; } else { v.pointLabel.label.show = false; } @@ -2231,12 +2253,12 @@ viewer = { if (typeof meta === 'object' && meta.type === 'searchResult') { // --- Search-result pin click (#172 pin-overlay Inc 1, option C) --- // Runs the same selection ceremony as a sample click (card, - // selection state, flight, lazy detail) via the shared helper, but - // PRESERVES the #samplesSection search-results list the user is - // browsing. Like the sample branch it also opens the floating in-map - // card at the click pixel and writes the pid selection hash. + // selection state, flight, full in-map detail) via the shared + // helper, but PRESERVES the #samplesSection search-results list the + // user is browsing. Reuse the freshness token already bumped above + // (isStale) so the helper does not double-bump (Codex round-1 P1.3). if (v.selectSearchResult) { - v.selectSearchResult(meta, { fly: true, inMapCardAt: { x: e.position.x, y: e.position.y } }); + v.selectSearchResult(meta, { fly: true, inMapCardAt: { x: e.position.x, y: e.position.y }, isStale }); } writeGlobeHash(v, { replace: false, force: true }); @@ -2254,6 +2276,10 @@ viewer = { // Clear nearby list const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; + // #172 Inc 1 (Codex round-1 P1.2, snapshot semantics): this click + // tears down the rendered results list, so the result-pin overlay + // (its snapshot) goes with it. + if (v.searchResultPoints) v.searchResultPoints.removeAll(); // Stage 2: lazy-load extended fields from wide parquet. // Extended beyond the original description-only query to @@ -2314,6 +2340,10 @@ viewer = { const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = '
Loading nearby samples...
'; + // #172 Inc 1 (Codex round-1 P1.2, snapshot semantics): a cluster + // click replaces the results list with nearby samples, so the + // result-pin overlay (its snapshot) is cleared too. + if (v.searchResultPoints) v.searchResultPoints.removeAll(); const delta = meta.resolution === 4 ? 2.0 : meta.resolution === 6 ? 0.5 : 0.1; try { @@ -5335,9 +5365,13 @@ zoomWatcher = { // anchors at canvas centre rather than flying. Adding a "camera far from // sample → flyTo" branch would fight the hashchange flyTo above and the // boot setView; it's a tracked follow-up, not folded in here. - async function openInMapCardForSample(meta, isStale) { + // `screenPos` (added #172 Inc 1): anchor the card at an explicit pixel — + // used by the result-pin click, which frames the card at the click point + // like the on-globe sample click. Omitted by the deep-link/hashchange + // callers, which keep the canvas-centre anchor described above. + async function openInMapCardForSample(meta, isStale, screenPos) { const canvas = viewer.scene.canvas; - showInMapCard(meta, { x: canvas.clientWidth / 2, y: canvas.clientHeight / 2 }); + showInMapCard(meta, screenPos || { x: canvas.clientWidth / 2, y: canvas.clientHeight / 2 }); try { const pidEsc = meta.pid.replace(/'/g, "''"); const detail = await db.query(` @@ -5965,8 +5999,9 @@ zoomWatcher = { async function clearActiveSearchFilter() { // Same clear path as an empty search submit, without entering doSearch(). _searchSeq++; - // #172 pin-overlay Inc 1 (option C): committed clear (concept-chip clear - // / programmatic ?search= removal) drops the result-pin overlay too. + // #172 pin-overlay Inc 1 (option C): this is the committed clear used by + // the concept-chip clear button (its only caller today) — it tears down + // the results list, so drop the result-pin overlay snapshot too. if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); if (searchInput) searchInput.value = ''; if (searchResults) searchResults.textContent = ''; @@ -6011,20 +6046,26 @@ zoomWatcher = { // hydration, selection state, flight, lazy detail — but deliberately does // NOT repaint #samplesSection (it holds the search-results list the user is // browsing). `sample` is a normalized meta ({ pid, label, source, lat, lng, - // place_name, result_time }); coord-less rows are ignored. `inMapCardAt` - // (pin click only) additionally opens the floating in-map card at the click - // pixel, parity with the sample-click branch. - async function selectSearchResult(sample, { fly = true, inMapCardAt = null } = {}) { + // place_name, result_time }); coord-less rows are ignored. + // + // `inMapCardAt` (pin click only): open the floating in-map card at the click + // pixel with the SAME rich detail (material / feature / specimen / thumbnail) + // the sample click renders — via the shared openInMapCardForSample(), so the + // wide-table fetch is reused, not duplicated (Codex round-1 P1.3). + // + // `isStale` (Codex round-1 P1.3): callers that already bumped the freshness + // token (the globe click handler) pass theirs so we don't double-bump; the + // row click passes none and we bump here. + async function selectSearchResult(sample, { fly = true, inMapCardAt = null, isStale = null } = {}) { if (!sample || sample.lat == null || sample.lng == null) return; // Bump the freshness token BEFORE any async work so a prior // cluster/sample detail load can't repaint the panel after we navigate. - const isStale = freshSelectionToken(viewer); + if (!isStale) isStale = freshSelectionToken(viewer); viewer._globeState.selectedPid = sample.pid; viewer._globeState.selectedH3 = null; updateSampleCard(sample); - if (inMapCardAt) showInMapCard(sample, inMapCardAt); if (fly) { viewer.camera.flyTo({ destination: Cesium.Cartesian3.fromDegrees(sample.lng, sample.lat, 50000), @@ -6032,8 +6073,16 @@ zoomWatcher = { }); } - // Lazy-load full description (mirrors the globe click handler). Don't - // clear samplesSection — it holds the results the user is browsing. + if (inMapCardAt) { + // Pin click: full in-map card hydration at the click pixel, reusing + // the shared wide-table fetch (updateSampleDetail + + // populateInMapCardDetail happen inside). + await openInMapCardForSample(sample, isStale, inMapCardAt); + return; + } + + // Row click: sidebar description only (no in-map card). Don't clear + // samplesSection — it holds the results the user is browsing. try { const detail = await db.query(` SELECT description @@ -6458,6 +6507,17 @@ zoomWatcher = { // coordinate-less matches; NEVER coerce to (0,0)). Old pins were // cleared at the top of doSearch (committed-clear path); the cap is // inherited from LIMIT 50, so pins ≤ 50 ≤ displayed results. + // + // LIFECYCLE POLICY (snapshot semantics, Codex round-1 P1.2): the + // pins are a snapshot of the rendered results list — they reconcile + // with the LIST lifecycle, not with filter state. They are cleared + // wherever that list is torn down (new/empty submit, described-by + // takeover, and the globe sample/cluster clicks that replace the + // list). They deliberately PERSIST when the list persists stale — + // e.g. a source/facet toggle while a search is active updates the + // #300 filtered surfaces but not the (frozen) results list, so the + // pins stay too. That staleness is the documented pre-existing + // behavior of the list itself, not a new pin bug. const pinScalar = new Cesium.NearFarScalar(1e2, 8, 2e5, 3); // mirror samplePoints for (const s of results) { if (s.latitude == null || s.longitude == null) continue; @@ -6482,11 +6542,15 @@ zoomWatcher = { outlineColor: Cesium.Color.fromCssColorString(c), outlineWidth: 2, scaleByDistance: pinScalar, - disableDepthTestDistance: Number.POSITIVE_INFINITY, + // #185 (Codex round-1 P1.1): bounded depth-test distance — + // Number.POSITIVE_INFINITY bleeds pins through the far side + // of the globe; POINT_DEPTH_TEST_DISTANCE matches the h3/ + // sample layers and prevents that back-hemisphere bleed. + disableDepthTestDistance: POINT_DEPTH_TEST_DISTANCE, }); } - // Fly to the first result. Skip for area-scoped searches — + // Fly to the first LOCATED result. Skip for area-scoped searches — // the user is already at the area they care about; flying // would zoom in and disorient. // @@ -6495,12 +6559,21 @@ zoomWatcher = { // doesn't carry stale selection state across the flight // (issue #207 item 8). User clicks on a specific row to // establish a new selection. - if (effectiveScope === 'world' && results[0].latitude && results[0].longitude) { + // + // #172 Inc 1 (Codex round-1 P1.5): scan for the first row with + // real coordinates using EXPLICIT null checks (0° lat/lng is + // valid). results[0] can be coord-less in world scope (LEFT JOIN + // to lite), so the old truthiness test on results[0] both skipped + // the flight for a coord-less top row and would have mis-treated + // 0,0. Fit-to-bounds (the 30°×30° camera rule) remains deferred to + // Inc 2 per PLAN_PIN_OVERLAY_2026-07-17.md. + const firstLocated = results.find(r => r.latitude != null && r.longitude != null); + if (effectiveScope === 'world' && firstLocated) { const cancelFinalFacetRefresh = refreshFacetCountsAfterSearchFlight(searchId); viewer._globeState.selectedPid = null; viewer._globeState.selectedH3 = null; viewer.camera.flyTo({ - destination: Cesium.Cartesian3.fromDegrees(results[0].longitude, results[0].latitude, 200000), + destination: Cesium.Cartesian3.fromDegrees(firstLocated.longitude, firstLocated.latitude, 200000), duration: 1.5, cancel: cancelFinalFacetRefresh, }); From 24a96cc778e2323fe4184d4161e6159238754049 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 15:12:35 -0700 Subject: [PATCH 05/15] test(playwright): strengthen pin-overlay e2e (Codex round-1 P1.4) - Exact set equality between pin pids and located displayed-row pids (local-many pottery Cyprus), not just bounds. - "new search replaces pins" compares PIN pid sets before/after, not side-panel pids. - Zero case (xyzzyqqqplugh) asserts a REAL zero (No results, active/complete) and fails on build-failure states. - Added the one-result case (single pid -> 1 located row, 1 pin) and the global-many case (basalt -> pin cloud spans > 30 deg extent). - Added a results-preserved case: a row click keeps the list and pin set. - Snapshot hook enables identity/position assertions. - URL precedence now honors the config's TEST_URL first (then BASE_URL, then a production fallback) so a branch dispatch tests the worktree, not production. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- tests/playwright/pin-overlay.spec.js | 213 ++++++++++++++++++++------- 1 file changed, 161 insertions(+), 52 deletions(-) diff --git a/tests/playwright/pin-overlay.spec.js b/tests/playwright/pin-overlay.spec.js index a2654d2..514a7d1 100644 --- a/tests/playwright/pin-overlay.spec.js +++ b/tests/playwright/pin-overlay.spec.js @@ -1,36 +1,84 @@ // #172 pin-overlay Inc 1 (option C): after a search completes the located // results are rendered as a temporary pin overlay on the globe // (viewer.searchResultPoints), independent of the H3 cluster / sample-point -// layers. These specs assert the overlay's population, cap, replacement, and -// committed-clear lifecycle end-to-end against the published data. +// layers. These specs assert population, exact set equality with the located +// displayed rows, the 50 cap, pin-set replacement, and the snapshot-semantics +// clear lifecycle end-to-end against the published data. // -// Run: BASE_URL=https://rdhyee.github.io/isamplesorg.github.io \ -// npx playwright test tests/playwright/pin-overlay.spec.js +// Run (branch dispatch honors the config's TEST_URL; BASE_URL is a manual +// fallback, production the last resort — a dispatch that sets TEST_URL tests +// the worktree, not production, per Codex round-1 P1.4f): +// TEST_URL=https:// npx playwright test tests/playwright/pin-overlay.spec.js // -// Test hook: window.__searchPins() returns viewer.searchResultPoints.length -// (set in the viewer cell). Ground truths reuse the FTS suite's 202608 index -// facts: 'pottery Cyprus' → 1,305 matches; 'basalt' → 785 — both far exceed -// the LIMIT 50 display cap, so the displayed set (and thus the pin set) is -// capped at 50. +// Test hook: window.__searchPins() → [{ pid, lat, lng }] read-only snapshot of +// the live pin collection. Ground truths reuse the FTS suite's 202608 index +// facts: 'pottery Cyprus' → 1,305 matches; 'basalt' → 785 — both exceed the +// LIMIT 50 display cap, so the displayed (and pinned) set is capped at 50. +// 'ark:/28722/k2000hz7r' is the FTS suite's known-present single pid. const { test, expect } = require('@playwright/test'); -const BASE = process.env.BASE_URL || 'https://rdhyee.github.io/isamplesorg.github.io'; +const BASE = process.env.TEST_URL || process.env.BASE_URL + || 'https://rdhyee.github.io/isamplesorg.github.io'; const URL = `${BASE}/explorer.html`; // Generous flake protection matching the FTS suite: a cold CDN edge pays boot // + lazy module import + sidecar/shard fetches before the first search lands. test.describe.configure({ retries: process.env.CI ? 1 : 0 }); -const pins = (page) => page.evaluate(() => (window.__searchPins ? window.__searchPins() : -1)); -const rowPids = (page) => - page.$$eval('#samplesSection .sample-row', (els) => els.map((e) => e.dataset.pid)); +// [{ pid, lat, lng }] snapshot of the live overlay. +const pins = (page) => page.evaluate(() => (window.__searchPins ? window.__searchPins() : [])); + +// PIDs of the LOCATED search-result rows in the side panel. Rows carry +// data-lat="null"/"" when the match has no coordinates (world-scope LEFT JOIN); +// those are exactly the rows the overlay skips. +const locatedRowPids = (page) => + page.$$eval('#samplesSection .sample-row', (els) => + els + .filter((e) => { + const la = e.dataset.lat, ln = e.dataset.lng; + return la && la !== 'null' && la !== 'undefined' + && ln && ln !== 'null' && ln !== 'undefined'; + }) + .map((e) => e.dataset.pid)); + +const displayedRowCount = (page) => + page.$$eval('#samplesSection .sample-row', (els) => els.length); + +const searchState = (page) => page.evaluate(() => { + const sf = window.__searchFilter || {}; + return { + active: sf.active, + total: sf.total, + term: sf.term, + substrate: sf.substrate, + resultsText: document.getElementById('searchResults')?.textContent || '', + panelText: document.getElementById('samplesSection')?.textContent || '', + }; +}); + +const sortJoin = (arr) => [...arr].sort().join('|'); async function submitSearch(page, term) { await page.fill('#sampleSearch', term); await page.locator('#searchSubmitBtn').first().click(); } +// Wait until the results line settles on a terminal state (not the transient +// "Searching…"/"Building…" messages). +async function waitSearchSettled(page) { + await page.waitForFunction(() => { + const s = (document.getElementById('searchResults')?.textContent || '').trim(); + return s.length > 0 && !/^(searching|building)/i.test(s); + }, null, { timeout: 120_000 }); +} + +async function waitPinsAtLeast(page, n) { + await page.waitForFunction( + (min) => (window.__searchPins ? window.__searchPins().length : 0) >= min, + n, { timeout: 120_000 }); +} + test.describe('search-result pin overlay (#172 Inc 1)', () => { test.describe.configure({ timeout: 120_000 }); @@ -38,67 +86,128 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { await page.goto(URL, { timeout: 90_000 }); await page.waitForSelector('.samples-table tbody tr[data-pid]', { timeout: 120_000 }); // Pin overlay starts empty before any search. - expect(await pins(page)).toBe(0); + expect((await pins(page)).length).toBe(0); }); - test('local-many: pottery Cyprus pins located results, capped at 50', async ({ page }) => { + test('local-many: pins are EXACTLY the located displayed rows, capped at 50', async ({ page }) => { await submitSearch(page, 'pottery Cyprus'); - // Pins populate synchronously right after the result rows render. - await page.waitForFunction(() => (window.__searchPins ? window.__searchPins() : 0) > 0, - null, { timeout: 120_000 }); - const pinCount = await pins(page); - const displayed = (await rowPids(page)).length; - expect(pinCount).toBeGreaterThan(0); - expect(pinCount).toBeLessThanOrEqual(50); - // One pin per LOCATED displayed result — coord-less rows are skipped, so - // pins never exceed the displayed set. - expect(pinCount).toBeLessThanOrEqual(displayed); + await waitPinsAtLeast(page, 1); + await waitSearchSettled(page); + + const pinList = await pins(page); + const pinPids = pinList.map((p) => p.pid); + const located = await locatedRowPids(page); + const displayed = await displayedRowCount(page); + + // Cap: at most the LIMIT 50 display set, and never more than displayed. + expect(pinPids.length).toBeGreaterThan(0); + expect(pinPids.length).toBeLessThanOrEqual(50); + expect(pinPids.length).toBeLessThanOrEqual(displayed); + // Exact set equality: one pin per located displayed row, no more, no fewer. + expect(sortJoin(pinPids)).toBe(sortJoin(located)); + // Every pin carries real coordinates (never coerced to 0,0 placeholders). + for (const p of pinList) { + expect(typeof p.lat).toBe('number'); + expect(typeof p.lng).toBe('number'); + } + }); + + test('one: a single-pid query yields one located row and one pin', async ({ page }) => { + await submitSearch(page, 'ark:/28722/k2000hz7r'); + await waitSearchSettled(page); + await waitPinsAtLeast(page, 1); + + const state = await searchState(page); + // Real result, not a build failure. + expect(state.resultsText).not.toMatch(/couldn't build|search error/i); + + const pinPids = (await pins(page)).map((p) => p.pid); + const located = await locatedRowPids(page); + const displayed = await displayedRowCount(page); + + expect(displayed).toBe(1); + expect(pinPids.length).toBe(1); + expect(sortJoin(pinPids)).toBe(sortJoin(located)); }); - test('zero: a no-hit query renders no pins', async ({ page }) => { + test('global-many: basalt pins spread over > 30° extent, capped at 50', async ({ page }) => { + await submitSearch(page, 'basalt'); + await waitPinsAtLeast(page, 1); + await waitSearchSettled(page); + + const pinList = await pins(page); + expect(pinList.length).toBeGreaterThan(0); + expect(pinList.length).toBeLessThanOrEqual(50); + + const lats = pinList.map((p) => p.lat); + const lngs = pinList.map((p) => p.lng); + const latExtent = Math.max(...lats) - Math.min(...lats); + const lngExtent = Math.max(...lngs) - Math.min(...lngs); + // Globally distributed corpus → the pin cloud spans well past 30°. + expect(Math.max(latExtent, lngExtent)).toBeGreaterThan(30); + }); + + test('zero: a no-hit query is a real zero (not a build failure) with no pins', async ({ page }) => { await submitSearch(page, 'xyzzyqqqplugh'); - await page.waitForFunction(() => - /no results|no samples|couldn't build/i.test( - (document.getElementById('searchResults')?.textContent || '') + - (document.getElementById('samplesSection')?.textContent || '')), - null, { timeout: 120_000 }); - expect(await pins(page)).toBe(0); + await waitSearchSettled(page); + + const state = await searchState(page); + // Distinguish a genuine empty result set from an infrastructure failure: + // a real zero shows "No results", NOT "Search error"/"couldn't build". + expect(state.resultsText).toMatch(/no results/i); + expect(state.resultsText).not.toMatch(/couldn't build|search error/i); + expect((await pins(page)).length).toBe(0); }); - test('new search replaces the pin set', async ({ page }) => { + test('new search replaces the PIN set', async ({ page }) => { await submitSearch(page, 'pottery Cyprus'); - await page.waitForFunction(() => (window.__searchPins ? window.__searchPins() : 0) > 0, - null, { timeout: 120_000 }); - const firstPids = await rowPids(page); - expect(await pins(page)).toBeGreaterThan(0); + await waitPinsAtLeast(page, 1); + const firstPinPids = (await pins(page)).map((p) => p.pid); + expect(firstPinPids.length).toBeGreaterThan(0); + expect(firstPinPids.length).toBeLessThanOrEqual(50); await submitSearch(page, 'basalt'); - // Wait until the count line reflects the NEW term, then for pins to repopulate. + // Wait for the count line to reflect the NEW term, then for pins to repopulate. await page.waitForFunction(() => /results for "basalt"/i.test(document.getElementById('searchResults')?.textContent || ''), null, { timeout: 120_000 }); - await page.waitForFunction(() => (window.__searchPins ? window.__searchPins() : 0) > 0, - null, { timeout: 120_000 }); - const secondPids = await rowPids(page); - const secondCount = await pins(page); + await waitPinsAtLeast(page, 1); + const secondPinPids = (await pins(page)).map((p) => p.pid); - expect(secondCount).toBeGreaterThan(0); - expect(secondCount).toBeLessThanOrEqual(50); - // Disjoint corpora → the displayed (and thus pinned) set must change. - expect(secondPids.join(',')).not.toBe(firstPids.join(',')); + expect(secondPinPids.length).toBeGreaterThan(0); + expect(secondPinPids.length).toBeLessThanOrEqual(50); + // Disjoint corpora → the PIN set (not just the side panel) must change. + expect(sortJoin(secondPinPids)).not.toBe(sortJoin(firstPinPids)); }); - test('committed empty search clears the pins', async ({ page }) => { + test('committed empty search clears the pins (snapshot lifecycle)', async ({ page }) => { await submitSearch(page, 'pottery Cyprus'); - await page.waitForFunction(() => (window.__searchPins ? window.__searchPins() : 0) > 0, - null, { timeout: 120_000 }); - expect(await pins(page)).toBeGreaterThan(0); + await waitPinsAtLeast(page, 1); + expect((await pins(page)).length).toBeGreaterThan(0); // Empty submit is a committed clear path — doSearch clears the overlay // before the too-short early return. await submitSearch(page, ''); - await page.waitForFunction(() => (window.__searchPins ? window.__searchPins() : -1) === 0, + await page.waitForFunction(() => (window.__searchPins ? window.__searchPins().length : -1) === 0, null, { timeout: 60_000 }); - expect(await pins(page)).toBe(0); + expect((await pins(page)).length).toBe(0); + }); + + test('selecting a result row preserves both the results list and the pins', async ({ page }) => { + await submitSearch(page, 'pottery Cyprus'); + await waitPinsAtLeast(page, 1); + const before = (await pins(page)).map((p) => p.pid); + const displayedBefore = await displayedRowCount(page); + + // Row click runs the shared selection ceremony but must NOT tear down the + // results list (or its pin snapshot). + await page.locator('#samplesSection .sample-row[data-lat]').first().click(); + // Give any async selection work a beat; the list/pins must be unchanged. + await page.waitForTimeout(1500); + + const after = (await pins(page)).map((p) => p.pid); + const displayedAfter = await displayedRowCount(page); + expect(displayedAfter).toBe(displayedBefore); + expect(sortJoin(after)).toBe(sortJoin(before)); }); }); From 9b931e9733b89d0c495eda383d75c81af97190a1 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 15:12:44 -0700 Subject: [PATCH 06/15] docs(plan): amend Inc 1 camera/lifecycle scope to match implementation Codex round-1 P1.5/P1.2: the live plan now states that Inc 1 ships only the first-located fly-to (fit-to-bounds and its local-many zoom acceptance are deferred to Inc 2), and documents the snapshot-semantics lifecycle plus the pre-existing writeQueryState search= drop left for a follow-up issue. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- PLAN_PIN_OVERLAY_2026-07-17.md | 42 +++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/PLAN_PIN_OVERLAY_2026-07-17.md b/PLAN_PIN_OVERLAY_2026-07-17.md index 6830998..53b7508 100644 --- a/PLAN_PIN_OVERLAY_2026-07-17.md +++ b/PLAN_PIN_OVERLAY_2026-07-17.md @@ -26,22 +26,42 @@ contract's non-negotiable rules: pixel size 8 - hover label + click → `updateSampleCard()` + pid hash write (dispatch on `meta.type === 'searchResult'`) -- lifecycle: populate on results; clear on input-clear / `?search=` removal / new search -- camera: fit-to-bounds iff extent < 30°×30°, else fly-to-top-1 @ 200000; - auto-fly suppressed in area scope (existing behavior) +- lifecycle: populate on results; clear per SNAPSHOT SEMANTICS below +- camera (amended per Codex round-1 P1.5): Inc 1 ships only the cheap half — + fly to the first *located* result @ 200000 (explicit null checks so 0° coords + are valid), auto-fly suppressed in area scope (existing behavior). **Fit-to- + bounds (the 30°×30° extent rule) is DEFERRED to Inc 2** — it is the only + camera work not in Inc 1. - global refiltering UNCHANGED in Inc 1 — pins are purely additive -- acceptance: the contract's four shape cases (zero / one / local-many / global-many) +- acceptance: the contract's four shape cases (zero / one / local-many / + global-many). Inc 1 asserts pin *identity/count/replacement/clear* for all + four; the **local-many fit-to-bounds ZOOM assertion is deferred to Inc 2** + with the fit-to-bounds implementation (Inc 1 flies to the first located + result in every non-area case instead) +- **lifecycle (amended, Codex round-1 P1.2 — SNAPSHOT SEMANTICS):** the pins are + a snapshot of the *rendered results list* and reconcile with the LIST + lifecycle, not with filter state. Cleared wherever the list is torn down + (new/empty submit, described-by takeover, globe sample/cluster click). They + deliberately PERSIST when the list persists stale (a source/facet toggle while + a search is active updates #300 filtered surfaces but not the frozen list) — + that staleness is pre-existing list behavior, documented, not a new pin bug. + Known pre-existing incoherence left for a follow-up issue: a draft-input clear + followed by a source/facet change makes `writeQueryState()` drop `search=` + from the URL while the runtime filter (and thus list + pins) survive. - **pin-count semantics (amended per review):** one pin per *located* displayed result (world scope legitimately returns coordinate-less rows — never coerce to (0,0)); panel reports displayed vs pinned counts when they differ; camera rules use the first *located* result -- **click parity (amended):** extract the sample-click ceremony (freshness token, - card, selection/hash, detail hydration) into a shared helper with a - preserve-results-list option; pin clicks use it — no shallow duplicate branch -- **lifecycle (amended):** clearing tied to the *committed* clear paths (empty - submit / URL search removal / new search), not draft input edits -- **styling (amended):** carry `scaleByDistance` + `disableDepthTestDistance` - explicitly (global-many visibility) +- **click parity (amended, Codex round-1 P1.3):** the sample-click ceremony + (freshness token, card, selection/hash, detail hydration) is the shared + `selectSearchResult()` helper with a preserve-results-list option; pin clicks + use it — no shallow duplicate branch. Pin clicks get the FULL in-map card + hydration (material / feature / specimen / thumbnail) by reusing + `openInMapCardForSample()`, not a description-only fetch; the helper takes the + caller's freshness token to avoid a double-bump +- **styling (amended):** carry `scaleByDistance` explicitly; use the bounded + `POINT_DEPTH_TEST_DISTANCE` for `disableDepthTestDistance` (Codex round-1 + P1.1 — `Infinity` bleeds pins through the far side of the globe) ### Inc 2 — `?searchui=pin` mode (the latency win) ← behind flag **Seam corrected per Codex plan review (P0):** skipping `applySearchFilterChange()` From 88b3fa729cc6198e570656d77e23b4ac31ecae75 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 15:25:10 -0700 Subject: [PATCH 07/15] fix(explorer): clear pins at all list-teardown paths (Codex round-2 P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Snapshot semantics require the result-pin overlay to reconcile with the rendered-list lifecycle at EVERY teardown, not just the two direct globe-click branches. Added guarded `viewer.searchResultPoints.removeAll()` at the remaining reachable teardowns: - hydrateClusterUI() — the choke point that replaces the list with nearby samples; covers source/facet selection-revalidation, hashchange h3 restore, and boot h3 deep-link. - source-filter handler: selectedH3-gone and selectedPid-gone clears (the "source filtering invalidates a selected search result" path). - facet-filter handler: selectedH3-gone clear. - hashchange: h3-invalid clear and the no-selection branch (the Back-after-pin-click repro that orphaned pins). Owner-aware list preservation is out of scope for Inc 1; clearing both together keeps the documented invariant simply. All #samplesSection teardown/replace sites are now covered; the render paths (doSearch/doDescribedBy) and the hashchange state.pid branch legitimately keep the list and its pins. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- explorer.qmd | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/explorer.qmd b/explorer.qmd index c1f250e..6b25e49 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -4811,6 +4811,8 @@ zoomWatcher = { updateClusterCard(null); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; + // #172 Inc 1 (Codex round-2 P1): list teardown → clear the pin snapshot too. + if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); writeGlobeHash(viewer, { force: true }); } else { // Cluster's dominant_source still checked, but the @@ -4834,6 +4836,9 @@ zoomWatcher = { updateClusterCard(null); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; + // #172 Inc 1 (Codex round-2 P1): source filter invalidated the + // selected search result and cleared the list → clear its pins too. + if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); writeGlobeHash(viewer, { force: true }); } } @@ -4926,6 +4931,8 @@ zoomWatcher = { updateClusterCard(null); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; + // #172 Inc 1 (Codex round-2 P1): list teardown → clear the pin snapshot too. + if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); writeGlobeHash(viewer, { force: true }); } else { await hydrateClusterUI(meta, isStale); @@ -5323,6 +5330,11 @@ zoomWatcher = { updateClusterCard(meta); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = '
Loading nearby samples...
'; + // #172 Inc 1 (Codex round-2 P1, snapshot semantics): replacing the list + // with nearby samples tears down any search-results list, so the + // result-pin overlay (its snapshot) goes with it. This choke point + // covers source/facet revalidation, hashchange h3 restore, and boot. + if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); const delta = meta.resolution === 4 ? 2.0 : meta.resolution === 6 ? 0.5 : 0.1; try { const samples = await db.query(` @@ -5527,6 +5539,8 @@ zoomWatcher = { updateClusterCard(null); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; + // #172 Inc 1 (Codex round-2 P1): list teardown → clear the pin snapshot too. + if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); } } else { viewer._globeState.selectedPid = null; @@ -5536,6 +5550,9 @@ zoomWatcher = { hideInMapCard(); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; + // #172 Inc 1 (Codex round-2 P1): the Back-after-pin-click no-selection + // branch tears down the preserved search list → clear its pins too. + if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); } }); From dd2fa339a07bed745564fb7647daad73c1dea8c5 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 15:25:21 -0700 Subject: [PATCH 08/15] test(playwright): exercise real selection + assert exact cap (Codex round-2 P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Row-preservation test now picks a demonstrably located row (numeric data-lat/lng, not "null"), clicks a NON-LINK child (the source badge) so the row handler actually runs, and asserts the selection happened (the sample card shows the row) with a condition-based wait BEFORE checking that the list and pin set are preserved. Previously it could pass with no row handler at all. - local-many (pottery Cyprus) and global-many (basalt) now assert exactly 50 displayed rows — their documented totals exceed 50, so the LIMIT 50 cap binds. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- tests/playwright/pin-overlay.spec.js | 42 ++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/tests/playwright/pin-overlay.spec.js b/tests/playwright/pin-overlay.spec.js index 514a7d1..3c02cfd 100644 --- a/tests/playwright/pin-overlay.spec.js +++ b/tests/playwright/pin-overlay.spec.js @@ -103,6 +103,8 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { expect(pinPids.length).toBeGreaterThan(0); expect(pinPids.length).toBeLessThanOrEqual(50); expect(pinPids.length).toBeLessThanOrEqual(displayed); + // Documented total (1,305) exceeds the LIMIT 50 cap, so it must bind exactly. + expect(displayed).toBe(50); // Exact set equality: one pin per located displayed row, no more, no fewer. expect(sortJoin(pinPids)).toBe(sortJoin(located)); // Every pin carries real coordinates (never coerced to 0,0 placeholders). @@ -138,6 +140,8 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { const pinList = await pins(page); expect(pinList.length).toBeGreaterThan(0); expect(pinList.length).toBeLessThanOrEqual(50); + // Documented total (785) exceeds the LIMIT 50 cap, so it must bind exactly. + expect(await displayedRowCount(page)).toBe(50); const lats = pinList.map((p) => p.lat); const lngs = pinList.map((p) => p.lng); @@ -193,21 +197,41 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { expect((await pins(page)).length).toBe(0); }); - test('selecting a result row preserves both the results list and the pins', async ({ page }) => { + test('selecting a located result row selects it AND preserves the list + pins', async ({ page }) => { await submitSearch(page, 'pottery Cyprus'); await waitPinsAtLeast(page, 1); const before = (await pins(page)).map((p) => p.pid); const displayedBefore = await displayedRowCount(page); - // Row click runs the shared selection ceremony but must NOT tear down the - // results list (or its pin snapshot). - await page.locator('#samplesSection .sample-row[data-lat]').first().click(); - // Give any async selection work a beat; the list/pins must be unchanged. - await page.waitForTimeout(1500); - + // Pick a DEMONSTRABLY LOCATED row (numeric data-lat/lng, not the string + // "null") — a coord-less row's handler returns immediately, so clicking it + // would prove nothing (Codex round-2 P2). + const located = await page.$$eval('#samplesSection .sample-row', (els) => + els + .filter((e) => { + const la = e.dataset.lat, ln = e.dataset.lng; + return la && la !== 'null' && la !== 'undefined' + && ln && ln !== 'null' && ln !== 'undefined'; + }) + .map((e) => ({ pid: e.dataset.pid, label: (e.querySelector('.sample-label')?.textContent || '').trim() }))); + expect(located.length).toBeGreaterThan(0); + const target = located[0]; + + // Click a NON-LINK child (the source badge) so the row handler runs — its + // own handler bails on clicks that land on the label. + await page.locator(`#samplesSection .sample-row[data-pid="${target.pid}"] .source-badge`).first().click(); + + // Assert the selection ACTUALLY happened (the sample card now shows this + // sample) with a condition-based wait BEFORE checking preservation. + await page.waitForFunction((label) => { + const cs = document.getElementById('clusterSection'); + if (!cs) return false; + return /

\s*Sample\s*<\/h4>/i.test(cs.innerHTML) && cs.textContent.includes(label); + }, target.label, { timeout: 60_000 }); + + // Row selection must NOT tear down the results list or its pin snapshot. const after = (await pins(page)).map((p) => p.pid); - const displayedAfter = await displayedRowCount(page); - expect(displayedAfter).toBe(displayedBefore); + expect(await displayedRowCount(page)).toBe(displayedBefore); expect(sortJoin(after)).toBe(sortJoin(before)); }); }); From b65a5e51f704bc473ba7e5d772b5821e0dd7a026 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 15:46:02 -0700 Subject: [PATCH 09/15] fix(explorer): make list+pin lifecycle atomic via panel ownership (Codex round-3 P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Round-2 clears were correct synchronously but not across awaits: a cluster hydration could clear pins, await its nearby query, and then (after a search had rendered its rows and pins) call updateSamples() and replace only the list, orphaning the search pins. doSearch never invalidated the selection freshness token, so the older hydration stayed "fresh". Panel ownership via the EXISTING freshness token (no new generation system): - doSearch() and doDescribedBy() bump freshSelectionToken(viewer) at entry — a committed search/concept is a new panel owner, so any in-flight selection hydration becomes stale and bails at its post-await isStale() check. Their own post-await renders stay guarded by searchId/_searchSeq (the two token systems guard different things — search build vs selection panel — and both hold; no interaction, verified against the boot search+pid path). - Every post-await list-mutation site now writes list + pins as ONE owned operation: the direct cluster-click nearby path and hydrateClusterUI() both re-clear pins AFTER the isStale() check, immediately before updateSamples() — so a search that populated pins during the await can't orphan them. - clearActiveSearchFilter() also bumps the token (ownership) and now clears the list, keeping list+pins together. Deterministic hole (Codex): the empty-submit early return now clears #samplesSection so the list and the overlay clear together (snapshot contract). Adds a test-only window.__clickSearchPin(i) hook that replays the real pin-click ceremony (shared helper + pid hash push) so specs can exercise pin selection and Back-navigation without Cesium canvas picking. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- explorer.qmd | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/explorer.qmd b/explorer.qmd index 6b25e49..a78e08e 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -2197,6 +2197,21 @@ viewer = { return out; }; + // Test hook (#172, Codex round-3 P2): faithfully replay a result-pin CLICK + // by index — the SAME ceremony the on-globe LEFT_CLICK handler runs (shared + // selection helper + pid hash push), so specs can exercise pin selection and + // Back-navigation without Cesium canvas picking (not feasible in Playwright). + // Returns false if there is no pin at that index. + if (typeof window !== 'undefined') window.__clickSearchPin = (i = 0) => { + const col = v.searchResultPoints; + if (!col || i < 0 || i >= col.length) return false; + const meta = col.get(i).id; + const isStale = freshSelectionToken(v); + if (v.selectSearchResult) v.selectSearchResult(meta, { fly: true, isStale }); + writeGlobeHash(v, { replace: false, force: true }); + return true; + }; + // Hover tooltip — works for both clusters and samples v.pointLabel = v.entities.add({ label: { @@ -2360,6 +2375,11 @@ viewer = { `; const samples = await db.query(nearbyQuery); if (isStale()) return; + // #172 Inc 1 (Codex round-3 P1): write the list + pins as ONE + // owned operation. Re-clear here (post-await, ownership just + // confirmed by isStale) so a search that populated pins DURING + // this await can't leave them orphaned over these nearby rows. + if (v.searchResultPoints) v.searchResultPoints.removeAll(); updateSamples(samples); } catch(err) { if (isStale()) return; @@ -5347,6 +5367,11 @@ zoomWatcher = { LIMIT 30 `); if (isStale && isStale()) return; + // #172 Inc 1 (Codex round-3 P1): list + pins as ONE owned operation. + // Re-clear post-await (ownership just confirmed) so a search that + // populated pins during this await can't orphan them over the + // nearby rows we're about to render. + if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); updateSamples(samples); } catch(err) { if (isStale && isStale()) return; @@ -6016,10 +6041,16 @@ zoomWatcher = { async function clearActiveSearchFilter() { // Same clear path as an empty search submit, without entering doSearch(). _searchSeq++; + // #172 Inc 1 (Codex round-3 P1): take panel ownership so an in-flight + // selection hydration can't repopulate the list after this clear. + freshSelectionToken(viewer); // #172 pin-overlay Inc 1 (option C): this is the committed clear used by // the concept-chip clear button (its only caller today) — it tears down - // the results list, so drop the result-pin overlay snapshot too. + // the results list, so clear the LIST and the result-pin overlay snapshot + // TOGETHER. if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + const sampElClear = document.getElementById('samplesSection'); + if (sampElClear) sampElClear.innerHTML = ''; if (searchInput) searchInput.value = ''; if (searchResults) searchResults.textContent = ''; await clearSearchFilter(); @@ -6134,6 +6165,19 @@ zoomWatcher = { // submission — valid or not — supersedes prior in-flight work. const searchId = ++_searchSeq; + // #172 Inc 1 (Codex round-3 P1): take PANEL OWNERSHIP. A committed search + // is a new panel owner, so bump the selection freshness token to + // invalidate any in-flight selection hydration (cluster nearby-samples, + // sample/pid detail). Those producers re-check isStale() after their + // await and bail before overwriting this search's list/pins, so the two + // can't diverge. We deliberately DON'T capture the token here: doSearch's + // own post-await renders are (and stay) guarded by searchId/_searchSeq — + // the two token systems guard different things (search build vs selection + // panel) and both must hold. The reverse race (a cluster click AFTER this + // submit) is handled by the owned post-await clears in the cluster-click / + // hydrateClusterUI paths. + freshSelectionToken(viewer); + // #172 pin-overlay Inc 1 (option C): clear the previous result-pin // overlay on every committed submit (before repopulating). This covers // both the new-search path (repopulated below on success) and the @@ -6144,6 +6188,11 @@ zoomWatcher = { const term = searchInput.value.trim(); if (!term || term.length < 2) { searchResults.textContent = 'Type at least 2 characters'; + // #172 Inc 1 (Codex round-3 P1): clear the results LIST here too so + // list and pins clear TOGETHER — otherwise stale search rows would + // survive under an empty overlay, breaking the snapshot invariant. + const sampElClear = document.getElementById('samplesSection'); + if (sampElClear) sampElClear.innerHTML = ''; // A1 (#234 Step 4): an empty / too-short submit means no committed // search — drop the pid-set filter and refresh every surface // (table, points, facet counts) plus revert the globe mode so the @@ -6799,6 +6848,13 @@ zoomWatcher = { // filter can't clobber each other's side-panel render mid-flight. const searchId = ++_searchSeq; + // #172 Inc 1 (Codex round-3 P1): take PANEL OWNERSHIP — same as doSearch. + // A committed concept filter is a new panel owner; bump the selection + // freshness token so any in-flight selection hydration bails at its + // post-await isStale() check rather than overwriting this render. The + // concept render itself stays guarded by searchId/_searchSeq. + freshSelectionToken(viewer); + // #172 pin-overlay Inc 1 (option C): committing a concept filter removes // `search=` and owns the panel, so drop any lingering text-search // result pins (a committed clear path for the text overlay). From d4277bafc9b94cbc820ed3dc960e2478273227af Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 15:46:14 -0700 Subject: [PATCH 10/15] test(playwright): lifecycle cases for the pin overlay (Codex round-3 P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Empty-submit test now asserts the results LIST is cleared (zero .sample-row), not just zero pins — list and pins clear together. - New Back-after-pin-click regression test: search -> __clickSearchPin (real ceremony: selects + pushes pid hash, list+pins preserved) -> page.goBack() -> assert the no-selection hash clears BOTH the list and the overlay. - Global-many (basalt) now asserts exact pin-pids == located-row-pids identity, matching local-many (the plan's "all four" wording). - Header documents that the async-interleaving invariant is guaranteed by the freshness-token ownership audit, not a flaky timing-dependent e2e (per review guidance not to ship a flaky interleaving test). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- tests/playwright/pin-overlay.spec.js | 70 ++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 8 deletions(-) diff --git a/tests/playwright/pin-overlay.spec.js b/tests/playwright/pin-overlay.spec.js index 3c02cfd..189198a 100644 --- a/tests/playwright/pin-overlay.spec.js +++ b/tests/playwright/pin-overlay.spec.js @@ -10,11 +10,26 @@ // the worktree, not production, per Codex round-1 P1.4f): // TEST_URL=https:// npx playwright test tests/playwright/pin-overlay.spec.js // -// Test hook: window.__searchPins() → [{ pid, lat, lng }] read-only snapshot of -// the live pin collection. Ground truths reuse the FTS suite's 202608 index -// facts: 'pottery Cyprus' → 1,305 matches; 'basalt' → 785 — both exceed the -// LIMIT 50 display cap, so the displayed (and pinned) set is capped at 50. -// 'ark:/28722/k2000hz7r' is the FTS suite's known-present single pid. +// Test hooks (both set in the viewer cell): +// window.__searchPins() → [{ pid, lat, lng }] read-only snapshot. +// window.__clickSearchPin(i) → replays the on-globe result-pin click ceremony +// (shared helper + pid hash push) by index, since +// Cesium canvas picking isn't feasible in Playwright. +// Ground truths reuse the FTS suite's 202608 index facts: 'pottery Cyprus' → +// 1,305 matches; 'basalt' → 785 — both exceed the LIMIT 50 display cap, so the +// displayed (and pinned) set is capped at 50. 'ark:/28722/k2000hz7r' is the FTS +// suite's known-present single pid. +// +// ASYNC INTERLEAVING (Codex round-3): the "a hydration must not overwrite a +// newer search's list/pins" invariant is guaranteed by the freshness-token +// OWNERSHIP audit, not by a timing-dependent e2e test. doSearch()/doDescribedBy() +// bump the selection token at entry (new panel owner), and every post-await +// list-mutation site (direct cluster-click nearby path, hydrateClusterUI) checks +// isStale() and re-clears pins as one owned operation before writing the list. A +// true interleaving race test would be flaky (it depends on two concurrent DuckDB +// queries resolving in a specific order), so per the review guidance we do not +// ship one; the Back-after-pin-click test below exercises the deterministic +// half of the lifecycle. const { test, expect } = require('@playwright/test'); @@ -143,6 +158,11 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { // Documented total (785) exceeds the LIMIT 50 cap, so it must bind exactly. expect(await displayedRowCount(page)).toBe(50); + // Exact identity: pins are precisely the located displayed rows (same + // assertion the local-many case makes — the plan requires it for "all four"). + const located = await locatedRowPids(page); + expect(sortJoin(pinList.map((p) => p.pid))).toBe(sortJoin(located)); + const lats = pinList.map((p) => p.lat); const lngs = pinList.map((p) => p.lng); const latExtent = Math.max(...lats) - Math.min(...lats); @@ -189,12 +209,46 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { await waitPinsAtLeast(page, 1); expect((await pins(page)).length).toBeGreaterThan(0); - // Empty submit is a committed clear path — doSearch clears the overlay - // before the too-short early return. + // Empty submit is a committed clear path — doSearch clears the overlay AND + // the results list before the too-short early return, so they clear TOGETHER + // (Codex round-3: assert the list too, not just the pins). await submitSearch(page, ''); - await page.waitForFunction(() => (window.__searchPins ? window.__searchPins().length : -1) === 0, + await page.waitForFunction(() => + (window.__searchPins ? window.__searchPins().length : -1) === 0 + && document.querySelectorAll('#samplesSection .sample-row').length === 0, + null, { timeout: 60_000 }); + expect((await pins(page)).length).toBe(0); + expect(await displayedRowCount(page)).toBe(0); + }); + + test('Back after a pin click clears BOTH the list and the pins', async ({ page }) => { + await submitSearch(page, 'pottery Cyprus'); + await waitPinsAtLeast(page, 1); + const beforePins = (await pins(page)).map((p) => p.pid); + const displayedBefore = await displayedRowCount(page); + expect(beforePins.length).toBeGreaterThan(0); + expect(displayedBefore).toBeGreaterThan(0); + + // A result-pin click selects the sample and pushes a pid hash, while + // PRESERVING the list and pins (verified here). + const clicked = await page.evaluate(() => window.__clickSearchPin(0)); + expect(clicked).toBe(true); + await page.waitForFunction(() => { + const cs = document.getElementById('clusterSection'); + return cs && /

\s*Sample\s*<\/h4>/i.test(cs.innerHTML); + }, null, { timeout: 60_000 }); + expect((await pins(page)).length).toBe(beforePins.length); + expect(await displayedRowCount(page)).toBe(displayedBefore); + + // Back → the no-selection hash → the hashchange handler clears BOTH the + // preserved list and the pin overlay (the round-2 sync fix; regression guard). + await page.goBack(); + await page.waitForFunction(() => + (window.__searchPins ? window.__searchPins().length : -1) === 0 + && document.querySelectorAll('#samplesSection .sample-row').length === 0, null, { timeout: 60_000 }); expect((await pins(page)).length).toBe(0); + expect(await displayedRowCount(page)).toBe(0); }); test('selecting a located result row selects it AND preserves the list + pins', async ({ page }) => { From d556c4f69c1e12c7a2416eefb89eb3e6f16aa173 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 16:08:15 -0700 Subject: [PATCH 11/15] fix(explorer): dedicated panel-generation ownership closes both race orders (Codex round-4 P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 bumped _selGen but doSearch's render checked only _searchSeq, so the reverse order (search starts, a newer cluster click starts, the cluster finishes first and the older search then overwrites its rows/pins) was still open. Add a dedicated list-ownership generation, viewer._panelGen: - Every list producer — doSearch, doDescribedBy, the direct cluster-click nearby path, and hydrateClusterUI — captures `const myPanelGen = ++viewer._panelGen` at START, and every post-await list+pin write additionally checks panelWriteAllowed(myPanelGen, viewer._panelGen) (search renders keep _searchSeq too — both must hold). Whoever STARTED LAST holds the highest generation, so an earlier producer that resolves later bails. Closes both orders. - The _selGen ownership bumps stay (they guard selection/card work). - panelWriteAllowed(captured, current) is a pure helper in assets/js/ explorer-utils.js so the invariant is unit-testable (added a unit test; 111 pass) and importable into the OJS runtime. Verified pid-deep-link-is-not-a-producer: the boot AND hashchange pid paths call updateSampleCard + openInMapCardForSample (card only) and never bump _panelGen, so boot search+pid still renders the search list. The synchronous selection-invalidation clears (source/facet/hashchange) are NOT given _panelGen: they are terminal synchronous writes, and even if an older async producer completes after one of them it writes list+pins together atomically, so the "list and pins never diverge" invariant still holds (Codex verified those sync clears in round 2). Also (round-4 P2): __clickSearchPin now replays the FULL pin ceremony (passes inMapCardAt = canvas centre for the rich floating-card hydration) and is installed only under navigator.webdriver so it cannot run in a normal session. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- assets/js/explorer-utils.js | 10 +++ explorer.qmd | 136 ++++++++++++++++++++--------- tests/unit/explorer-utils.test.mjs | 16 +++- 3 files changed, 120 insertions(+), 42 deletions(-) diff --git a/assets/js/explorer-utils.js b/assets/js/explorer-utils.js index fd6da63..d73e219 100644 --- a/assets/js/explorer-utils.js +++ b/assets/js/explorer-utils.js @@ -12,6 +12,16 @@ export function escapeHtml(value) { .replace(/'/g, '''); } +// Panel/list ownership check (#172 Inc 1, Codex round-4 P1). Every producer that +// writes the #samplesSection list (search / concept renders, cluster +// nearby-samples, selection-invalidation clears) captures a generation via +// `++viewer._panelGen` when it takes the list; an async producer may only write +// list + pins while it still holds the latest generation. Pure so the ownership +// invariant is unit-testable independently of the OJS runtime. +export function panelWriteAllowed(captured, current) { + return captured === current; +} + // Split a free-text query into whitespace-delimited terms (no empties). export function searchTerms(value) { return String(value || '').trim().split(/\s+/).filter(Boolean); diff --git a/explorer.qmd b/explorer.qmd index a78e08e..02e6739 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -943,6 +943,7 @@ csvParamValues = _explorerUtils.csvParamValues sourceUrl = _explorerUtils.sourceUrl readHash = _explorerUtils.readHash formatPlaceName = _explorerUtils.formatPlaceName +panelWriteAllowed = _explorerUtils.panelWriteAllowed // === Source Filter: get active sources and build SQL clause === function getActiveSources() { @@ -2137,6 +2138,13 @@ viewer = { // URL deep-link state (must be set before globalRect/once block reads it) v._globeState = { mode: 'cluster', selectedPid: null, selectedH3: null }; + // #172 Inc 1 (Codex round-4 P1): panel/list ownership generation. Every + // #samplesSection list producer claims ++v._panelGen when it takes the list; + // async producers write list+pins only while they still hold the latest + // generation (see panelWriteAllowed). Closes the search-vs-cluster race in + // BOTH orders. The pid deep-link hydration writes only the card, so it never + // bumps this — that's why the boot search+pid list still renders. + v._panelGen = 0; v._initialHash = readHash(); v._suppressHashWrite = true; // cleared after zoomWatcher initializes v._suppressTimer = null; @@ -2197,20 +2205,27 @@ viewer = { return out; }; - // Test hook (#172, Codex round-3 P2): faithfully replay a result-pin CLICK - // by index — the SAME ceremony the on-globe LEFT_CLICK handler runs (shared - // selection helper + pid hash push), so specs can exercise pin selection and - // Back-navigation without Cesium canvas picking (not feasible in Playwright). - // Returns false if there is no pin at that index. - if (typeof window !== 'undefined') window.__clickSearchPin = (i = 0) => { - const col = v.searchResultPoints; - if (!col || i < 0 || i >= col.length) return false; - const meta = col.get(i).id; - const isStale = freshSelectionToken(v); - if (v.selectSearchResult) v.selectSearchResult(meta, { fly: true, isStale }); - writeGlobeHash(v, { replace: false, force: true }); - return true; - }; + // Test hook (#172, Codex round-3/4 P2): faithfully replay a result-pin CLICK + // by index — the FULL ceremony the on-globe LEFT_CLICK handler runs, INCLUDING + // inMapCardAt (canvas centre) so the rich floating-card hydration path runs, + // plus the shared selection helper and the pid hash push. Lets specs exercise + // pin selection and Back-navigation without Cesium canvas picking (not feasible + // in Playwright). Installed ONLY under WebDriver (Playwright sets + // navigator.webdriver) so it can't be invoked in a normal session. Returns + // false if there is no pin at that index. + if (typeof window !== 'undefined' && typeof navigator !== 'undefined' && navigator.webdriver === true) { + window.__clickSearchPin = (i = 0) => { + const col = v.searchResultPoints; + if (!col || i < 0 || i >= col.length) return false; + const meta = col.get(i).id; + const canvas = v.scene.canvas; + const inMapCardAt = { x: canvas.clientWidth / 2, y: canvas.clientHeight / 2 }; + const isStale = freshSelectionToken(v); + if (v.selectSearchResult) v.selectSearchResult(meta, { fly: true, inMapCardAt, isStale }); + writeGlobeHash(v, { replace: false, force: true }); + return true; + }; + } // Hover tooltip — works for both clusters and samples v.pointLabel = v.entities.add({ @@ -2344,6 +2359,11 @@ viewer = { } else if (typeof meta === 'object' && meta.count) { // --- Cluster click --- + // #172 Inc 1 (Codex round-4 P1): claim panel ownership — this cluster + // click is a new list producer, so an older in-flight search render + // (which checks its own captured gen) bails instead of overwriting + // these nearby rows. + const myPanelGen = ++v._panelGen; updateClusterCard(meta); // Cluster click clears any prior sample selection; close // the in-map sample card so it doesn't outlive the context @@ -2375,10 +2395,13 @@ viewer = { `; const samples = await db.query(nearbyQuery); if (isStale()) return; - // #172 Inc 1 (Codex round-3 P1): write the list + pins as ONE - // owned operation. Re-clear here (post-await, ownership just - // confirmed by isStale) so a search that populated pins DURING - // this await can't leave them orphaned over these nearby rows. + // #172 Inc 1 (Codex round-4 P1): only write if this click still + // owns the panel. Closes the reverse race — a search that + // STARTED earlier but resolves later has a stale panel gen and + // must not overwrite these rows. (round-3 P1): write list + pins + // as ONE owned operation — no await between this guard, the + // re-clear, and updateSamples, so nothing can interpose. + if (!panelWriteAllowed(myPanelGen, v._panelGen)) return; if (v.searchResultPoints) v.searchResultPoints.removeAll(); updateSamples(samples); } catch(err) { @@ -5347,6 +5370,9 @@ zoomWatcher = { // because click selection is its own latest event. async function hydrateClusterUI(meta, isStale) { if (!meta) return; + // #172 Inc 1 (Codex round-4 P1): claim panel ownership so an older + // in-flight search render bails instead of overwriting these rows. + const myPanelGen = ++viewer._panelGen; updateClusterCard(meta); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = '
Loading nearby samples...
'; @@ -5367,10 +5393,11 @@ zoomWatcher = { LIMIT 30 `); if (isStale && isStale()) return; - // #172 Inc 1 (Codex round-3 P1): list + pins as ONE owned operation. - // Re-clear post-await (ownership just confirmed) so a search that - // populated pins during this await can't orphan them over the - // nearby rows we're about to render. + // #172 Inc 1 (Codex round-4 P1): only write if this hydration still + // owns the panel — closes the reverse race with a search that + // started earlier but resolves later. (round-3 P1): list + pins as + // ONE owned operation, no await between guard, re-clear, and write. + if (!panelWriteAllowed(myPanelGen, viewer._panelGen)) return; if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); updateSamples(samples); } catch(err) { @@ -6165,18 +6192,19 @@ zoomWatcher = { // submission — valid or not — supersedes prior in-flight work. const searchId = ++_searchSeq; - // #172 Inc 1 (Codex round-3 P1): take PANEL OWNERSHIP. A committed search - // is a new panel owner, so bump the selection freshness token to - // invalidate any in-flight selection hydration (cluster nearby-samples, - // sample/pid detail). Those producers re-check isStale() after their - // await and bail before overwriting this search's list/pins, so the two - // can't diverge. We deliberately DON'T capture the token here: doSearch's - // own post-await renders are (and stay) guarded by searchId/_searchSeq — - // the two token systems guard different things (search build vs selection - // panel) and both must hold. The reverse race (a cluster click AFTER this - // submit) is handled by the owned post-await clears in the cluster-click / - // hydrateClusterUI paths. + // #172 Inc 1: take PANEL OWNERSHIP two ways. + // (round-3, _selGen) bump the selection freshness token so in-flight + // sample/pid detail + cluster hydrations become stale and bail — this + // guards selection/card work. freshSelectionToken(viewer); + // (round-4, _panelGen) claim a fresh LIST generation and CAPTURE it. Every + // post-await list write below re-checks it (in addition to _searchSeq), so + // a newer list producer that started AFTER this search — e.g. a cluster + // click while the search is still building — makes this render bail + // instead of overwriting the cluster's rows. This closes the reverse race + // order. It does NOT break boot search+pid: the pid deep-link writes only + // the card and never bumps _panelGen, so this render still owns the list. + const myPanelGen = ++viewer._panelGen; // #172 pin-overlay Inc 1 (option C): clear the previous result-pin // overlay on every committed submit (before repopulating). This covers @@ -6474,6 +6502,16 @@ zoomWatcher = { wasSuperseded = true; return; } + // #172 Inc 1 (Codex round-4 P1): also bail if a newer LIST producer + // (e.g. a cluster click while this search was building) took the + // panel. Both _searchSeq (search supersession) and _panelGen (list + // ownership) must hold before we write the list/pins — there is no + // await between here and the row-render + pin-populate below, so this + // single check makes that write atomic w.r.t. panel ownership. + if (!panelWriteAllowed(myPanelGen, viewer._panelGen)) { + wasSuperseded = true; + return; + } resultsCount = results.length; if (results.length === 0) { // A build failure also empties search_pids → 0 rows here, so @@ -6738,9 +6776,14 @@ zoomWatcher = { // Honesty fix (#247): keep the side panel consistent with the // table meta's "→ panel" pointer on failure too, instead of // leaving stale results under a pointer that now lies. - const sampElErr = document.getElementById('samplesSection'); - if (sampElErr) sampElErr.innerHTML = searchHeadingHTML('') - + '
Search failed — please try again.
'; + // #172 Inc 1 (Codex round-4 P1): but only if we still own the + // panel — a newer list producer must not have its rows replaced + // by this (older) search's failure message. + if (panelWriteAllowed(myPanelGen, viewer._panelGen)) { + const sampElErr = document.getElementById('samplesSection'); + if (sampElErr) sampElErr.innerHTML = searchHeadingHTML('') + + '
Search failed — please try again.
'; + } } } finally { performance.mark(markEnd); @@ -6848,12 +6891,14 @@ zoomWatcher = { // filter can't clobber each other's side-panel render mid-flight. const searchId = ++_searchSeq; - // #172 Inc 1 (Codex round-3 P1): take PANEL OWNERSHIP — same as doSearch. - // A committed concept filter is a new panel owner; bump the selection - // freshness token so any in-flight selection hydration bails at its - // post-await isStale() check rather than overwriting this render. The - // concept render itself stays guarded by searchId/_searchSeq. + // #172 Inc 1: take PANEL OWNERSHIP — same as doSearch. (round-3, _selGen) + // bump the selection token so in-flight selection hydrations bail; (round-4, + // _panelGen) claim + capture a LIST generation, re-checked before this + // render, so a cluster click that starts AFTER this concept commit wins + // the list instead of being overwritten. The concept render also stays + // guarded by searchId/_searchSeq. freshSelectionToken(viewer); + const myPanelGen = ++viewer._panelGen; // #172 pin-overlay Inc 1 (option C): committing a concept filter removes // `search=` and owns the panel, so drop any lingering text-search @@ -6908,8 +6953,13 @@ zoomWatcher = { const headingHTML = (suffix) => `

Samples described by: ${safeLabel}${suffix}

`; + // #172 Inc 1 (Codex round-4 P1): a newer list producer (cluster click) + // may own the panel now — gate the failed/empty LIST renders on it (the + // URL write above stays _searchSeq's concern). + const conceptOwnsPanel = () => panelWriteAllowed(myPanelGen, viewer._panelGen); if (buildFailed) { if (searchResults) searchResults.textContent = `Concept filter failed for "${label}". Please try again.`; + if (!conceptOwnsPanel()) return; const sElF = document.getElementById('samplesSection'); if (sElF) sElF.innerHTML = headingHTML('') + '
Filter failed to build — please try again.
'; @@ -6917,6 +6967,7 @@ zoomWatcher = { } if (total === 0) { if (searchResults) searchResults.textContent = `No samples described by "${label}"`; + if (!conceptOwnsPanel()) return; const sEl0 = document.getElementById('samplesSection'); if (sEl0) sEl0.innerHTML = headingHTML(' (0)') + '
No samples matched this concept.
'; @@ -6951,6 +7002,9 @@ zoomWatcher = { ? `${shown} of ${total.toLocaleString()} samples described by "${label}"` : `${shown} sample${shown === 1 ? '' : 's'} described by "${label}"`; + // #172 Inc 1 (Codex round-4 P1): bail if a newer list producer took the + // panel while this concept query was in flight. + if (!conceptOwnsPanel()) return; const sampEl = document.getElementById('samplesSection'); if (sampEl) { let h = headingHTML(` (${ofTotal})`); diff --git a/tests/unit/explorer-utils.test.mjs b/tests/unit/explorer-utils.test.mjs index 47aa175..50aadba 100644 --- a/tests/unit/explorer-utils.test.mjs +++ b/tests/unit/explorer-utils.test.mjs @@ -4,7 +4,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { escapeHtml, searchTerms, parseNum, csvParamValues, sourceUrl, readHash, - facetCountsDisplayState, formatPlaceName, + facetCountsDisplayState, formatPlaceName, panelWriteAllowed, } from '../../assets/js/explorer-utils.js'; test('escapeHtml escapes the five HTML-significant chars; nullish -> ""', () => { @@ -115,3 +115,17 @@ test('facetCountsDisplayState: index ready but the count QUERY itself failed -> test('facetCountsDisplayState: ready + fallthrough is not a state the caller produces, but resolves safely', () => { assert.equal(facetCountsDisplayState('ready', 'fallthrough'), 'unavailable'); }); + +test('panelWriteAllowed: a producer may write only while it holds the latest panel generation (#172 Inc 1)', () => { + // A captured generation equal to the current one → this producer still owns + // the #samplesSection list and may write list + pins. + assert.equal(panelWriteAllowed(1, 1), true); + assert.equal(panelWriteAllowed(7, 7), true); + // A newer producer has since bumped viewer._panelGen → the older producer's + // captured generation is stale and it must bail (closes the search-vs-cluster + // race in BOTH orders: whoever started last holds the highest generation). + assert.equal(panelWriteAllowed(1, 2), false); + assert.equal(panelWriteAllowed(2, 1), false); + // Strict identity — no coercion surprises. + assert.equal(panelWriteAllowed(0, 0), true); +}); From b79e842f9300bb2fafdda6a25d1a18beba09231e Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 16:08:29 -0700 Subject: [PATCH 12/15] test(playwright): honor config baseURL + deterministic Back test (Codex round-4 P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - URL resolution now uses process.env.TEST_URL || BASE_URL || '' and a relative goto when unset, so Playwright's config baseURL (TEST_URL || localhost) drives a branch dispatch — no hard-coded production fallback that would silently test deployed prod. Noted the fts-v1.spec.js divergence (left unchanged; out of scope). - Back-after-pin-click test: wait for the auto-fly to SETTLE into a globe hash with a `v` and no `pid` before clicking, so Back has a deterministic no-selection entry; after goBack wait on the no-pid hash before asserting the list+pins cleared. Predicates are self-contained (browser globals only) so they serialize into page.waitForFunction cleanly. - Header documents that the search-vs-cluster interleaving invariant is proven by the panel-generation ownership audit (panelWriteAllowed, unit-tested), not a flaky timing e2e. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- tests/playwright/pin-overlay.spec.js | 55 ++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/tests/playwright/pin-overlay.spec.js b/tests/playwright/pin-overlay.spec.js index 189198a..6cc2508 100644 --- a/tests/playwright/pin-overlay.spec.js +++ b/tests/playwright/pin-overlay.spec.js @@ -20,22 +20,29 @@ // displayed (and pinned) set is capped at 50. 'ark:/28722/k2000hz7r' is the FTS // suite's known-present single pid. // -// ASYNC INTERLEAVING (Codex round-3): the "a hydration must not overwrite a -// newer search's list/pins" invariant is guaranteed by the freshness-token -// OWNERSHIP audit, not by a timing-dependent e2e test. doSearch()/doDescribedBy() -// bump the selection token at entry (new panel owner), and every post-await -// list-mutation site (direct cluster-click nearby path, hydrateClusterUI) checks -// isStale() and re-clears pins as one owned operation before writing the list. A -// true interleaving race test would be flaky (it depends on two concurrent DuckDB -// queries resolving in a specific order), so per the review guidance we do not -// ship one; the Back-after-pin-click test below exercises the deterministic -// half of the lifecycle. +// ASYNC INTERLEAVING (Codex round-3/4): the "list and pins never diverge across a +// search-vs-cluster race, in EITHER order" invariant is guaranteed by the +// dedicated panel-generation counter (viewer._panelGen). Every list producer +// (doSearch, doDescribedBy, the cluster-click nearby path, hydrateClusterUI) +// captures ++viewer._panelGen at start and only writes list+pins while it still +// holds the latest generation — so whichever producer STARTED LAST wins, and an +// earlier producer that resolves later bails. The pure check is unit-tested as +// panelWriteAllowed() in tests/unit/explorer-utils.test.mjs. A live interleaving +// e2e would be flaky (two concurrent DuckDB queries resolving in a specific order), +// so per review guidance we do not ship one; the Back-after-pin-click test below +// exercises the deterministic half of the lifecycle. const { test, expect } = require('@playwright/test'); -const BASE = process.env.TEST_URL || process.env.BASE_URL - || 'https://rdhyee.github.io/isamplesorg.github.io'; -const URL = `${BASE}/explorer.html`; +// URL precedence (Codex round-4 P2): honor an explicit env override, else fall +// back to Playwright's configured baseURL via a RELATIVE goto (playwright.config.js +// resolves TEST_URL || http://localhost:5860). NO hard-coded production fallback, +// so a branch dispatch verifies the rendered worktree, not deployed production. +// NOTE (divergence): tests/playwright/fts-v1.spec.js still hard-codes a production +// BASE_URL fallback; that spec is out of scope for this branch and left unchanged. +const BASE = process.env.TEST_URL || process.env.BASE_URL || ''; +const explorerUrl = (query = '') => + BASE ? `${BASE}/explorer.html${query}` : `/explorer.html${query}`; // Generous flake protection matching the FTS suite: a cold CDN edge pays boot // + lazy module import + sidecar/shard fetches before the first search lands. @@ -98,7 +105,7 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { test.describe.configure({ timeout: 120_000 }); test.beforeEach(async ({ page }) => { - await page.goto(URL, { timeout: 90_000 }); + await page.goto(explorerUrl(), { timeout: 90_000 }); await page.waitForSelector('.samples-table tbody tr[data-pid]', { timeout: 120_000 }); // Pin overlay starts empty before any search. expect((await pins(page)).length).toBe(0); @@ -222,8 +229,22 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { }); test('Back after a pin click clears BOTH the list and the pins', async ({ page }) => { + // These predicates run in the BROWSER (passed directly to waitForFunction), + // so they must be self-contained — only browser globals, no Node closure. + const hasHashPid = () => new URLSearchParams((location.hash || '').slice(1)).has('pid'); + const noHashPid = () => !new URLSearchParams((location.hash || '').slice(1)).has('pid'); + const settledNoPidHash = () => { + const p = new URLSearchParams((location.hash || '').slice(1)); + return p.has('v') && !p.has('pid'); + }; + await submitSearch(page, 'pottery Cyprus'); await waitPinsAtLeast(page, 1); + // Wait for the auto-fly to SETTLE into a globe hash that has no pid, so Back + // has a deterministic no-selection entry to return to (Codex round-4 P2 — + // otherwise the previous history entry can be hashless / flight-cancelled). + await page.waitForFunction(settledNoPidHash, null, { timeout: 90_000 }); + const beforePins = (await pins(page)).map((p) => p.pid); const displayedBefore = await displayedRowCount(page); expect(beforePins.length).toBeGreaterThan(0); @@ -233,6 +254,7 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { // PRESERVING the list and pins (verified here). const clicked = await page.evaluate(() => window.__clickSearchPin(0)); expect(clicked).toBe(true); + await page.waitForFunction(hasHashPid, null, { timeout: 60_000 }); await page.waitForFunction(() => { const cs = document.getElementById('clusterSection'); return cs && /

\s*Sample\s*<\/h4>/i.test(cs.innerHTML); @@ -240,9 +262,10 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { expect((await pins(page)).length).toBe(beforePins.length); expect(await displayedRowCount(page)).toBe(displayedBefore); - // Back → the no-selection hash → the hashchange handler clears BOTH the - // preserved list and the pin overlay (the round-2 sync fix; regression guard). + // Back → the no-pid hash → the hashchange handler clears BOTH the preserved + // list and the pin overlay (the round-2 sync fix; regression guard). await page.goBack(); + await page.waitForFunction(noHashPid, null, { timeout: 60_000 }); await page.waitForFunction(() => (window.__searchPins ? window.__searchPins().length : -1) === 0 && document.querySelectorAll('#samplesSection .sample-row').length === 0, From d547b80f3c2c3e01362c45eaab4650e16c1ded94 Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 16:25:51 -0700 Subject: [PATCH 13/15] fix(explorer): unified panel-generation rule + entry teardown + status settle (Codex round-5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1.1 UNIFIED RULE — every write to the rendered panel list (#samplesSection), clear or replace, sync or async, now advances viewer._panelGen. Async producers also capture it and only write while current; synchronous clears just advance it so a slow in-flight search/hydration can no longer resurrect a panel the user has since replaced or cleared. Stated once at the _panelGen init. Audited every #samplesSection write; the sync clears that previously did NOT bump now do: - ordinary sample-click list clear (viewer cell) - source-filter invalidation clears (selectedH3-gone, selectedPid-gone) - facet-filter invalidation clear (selectedH3-gone) - hashchange unknown-H3 clear and no-selection (Back) clear - clearActiveSearchFilter (concept-chip clear) The async producers (doSearch, doDescribedBy, cluster-click nearby, hydrateClusterUI) already advance via their ++capture; the pid deep-link stays card-only (never bumps). P1.2 — a committed new search/concept now tears the panel DOWN at entry: clear the old rows AND pins together (under the freshly captured generation), so no stale unpinned rows linger for seconds during the async build; the panel shows the building status until the atomic replacement lands. The entry clear's bump IS the producer's captured gen (one ++), so the search still owns its own subsequent render. Removed the now-redundant empty-branch-only list clear. P2 — search-status settle on lost ownership: when doSearch bails because panelWriteAllowed fails, it now settles its own "Searching…" line to '' iff its searchId still equals _searchSeq (no newer search owns the text), so the status can't persist forever after a cluster takes the panel. If a newer search owns the text, it touches nothing. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- explorer.qmd | 118 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 45 deletions(-) diff --git a/explorer.qmd b/explorer.qmd index 02e6739..9050b79 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -2138,12 +2138,15 @@ viewer = { // URL deep-link state (must be set before globalRect/once block reads it) v._globeState = { mode: 'cluster', selectedPid: null, selectedH3: null }; - // #172 Inc 1 (Codex round-4 P1): panel/list ownership generation. Every - // #samplesSection list producer claims ++v._panelGen when it takes the list; - // async producers write list+pins only while they still hold the latest - // generation (see panelWriteAllowed). Closes the search-vs-cluster race in - // BOTH orders. The pid deep-link hydration writes only the card, so it never - // bumps this — that's why the boot search+pid list still renders. + // #172 Inc 1 — PANEL/LIST OWNERSHIP GENERATION (unified rule, Codex round-5): + // ANY write to the rendered panel list (#samplesSection) — clear or replace, + // synchronous or asynchronous — advances _panelGen. Async producers ALSO + // capture it at start and may only write list+pins while still current (see + // panelWriteAllowed). Synchronous clears just advance it: the bump's job is to + // invalidate any in-flight async producer so a slow search/hydration can no + // longer resurrect a panel the user has since replaced or cleared. The pid + // deep-link is card-only (never writes the list) so it never bumps this — + // that's why the boot search+pid list still renders. v._panelGen = 0; v._initialHash = readHash(); v._suppressHashWrite = true; // cleared after zoomWatcher initializes @@ -2306,10 +2309,13 @@ viewer = { // Clear nearby list const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; - // #172 Inc 1 (Codex round-1 P1.2, snapshot semantics): this click - // tears down the rendered results list, so the result-pin overlay - // (its snapshot) goes with it. + // #172 Inc 1 (Codex round-1 P1.2 / round-5 unified rule): this click + // tears down the rendered results list, so the pin overlay (its + // snapshot) goes with it AND _panelGen advances — a slow in-flight + // search that captured an earlier gen now bails instead of + // resurrecting the list the user just replaced with this selection. if (v.searchResultPoints) v.searchResultPoints.removeAll(); + v._panelGen++; // Stage 2: lazy-load extended fields from wide parquet. // Extended beyond the original description-only query to @@ -4854,8 +4860,11 @@ zoomWatcher = { updateClusterCard(null); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; - // #172 Inc 1 (Codex round-2 P1): list teardown → clear the pin snapshot too. + // #172 Inc 1 (Codex round-2/round-5): list teardown → clear + // the pin snapshot too AND advance _panelGen (unified rule) + // so a slow in-flight search can't resurrect this list. if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + viewer._panelGen++; writeGlobeHash(viewer, { force: true }); } else { // Cluster's dominant_source still checked, but the @@ -4879,9 +4888,12 @@ zoomWatcher = { updateClusterCard(null); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; - // #172 Inc 1 (Codex round-2 P1): source filter invalidated the - // selected search result and cleared the list → clear its pins too. + // #172 Inc 1 (Codex round-2/round-5): source filter + // invalidated the selected search result and cleared the + // list → clear its pins too AND advance _panelGen (unified + // rule) so a slow in-flight search can't resurrect it. if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + viewer._panelGen++; writeGlobeHash(viewer, { force: true }); } } @@ -4974,8 +4986,10 @@ zoomWatcher = { updateClusterCard(null); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; - // #172 Inc 1 (Codex round-2 P1): list teardown → clear the pin snapshot too. + // #172 Inc 1 (Codex round-2/round-5): list teardown → clear the + // pin snapshot too AND advance _panelGen (unified rule). if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + viewer._panelGen++; writeGlobeHash(viewer, { force: true }); } else { await hydrateClusterUI(meta, isStale); @@ -5591,8 +5605,10 @@ zoomWatcher = { updateClusterCard(null); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; - // #172 Inc 1 (Codex round-2 P1): list teardown → clear the pin snapshot too. + // #172 Inc 1 (Codex round-2/round-5): list teardown → clear the pin + // snapshot too AND advance _panelGen (unified rule). if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + viewer._panelGen++; } } else { viewer._globeState.selectedPid = null; @@ -5602,9 +5618,12 @@ zoomWatcher = { hideInMapCard(); const sampEl = document.getElementById('samplesSection'); if (sampEl) sampEl.innerHTML = ''; - // #172 Inc 1 (Codex round-2 P1): the Back-after-pin-click no-selection - // branch tears down the preserved search list → clear its pins too. + // #172 Inc 1 (Codex round-2/round-5): the Back-after-pin-click + // no-selection branch tears down the preserved search list → clear its + // pins too AND advance _panelGen (unified rule) so a slow in-flight + // search can't resurrect the list after the user navigated away. if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + viewer._panelGen++; } }); @@ -6071,11 +6090,12 @@ zoomWatcher = { // #172 Inc 1 (Codex round-3 P1): take panel ownership so an in-flight // selection hydration can't repopulate the list after this clear. freshSelectionToken(viewer); - // #172 pin-overlay Inc 1 (option C): this is the committed clear used by - // the concept-chip clear button (its only caller today) — it tears down - // the results list, so clear the LIST and the result-pin overlay snapshot - // TOGETHER. + // #172 pin-overlay Inc 1 (option C / round-5 unified rule): the committed + // concept-chip clear tears down the results list, so clear the LIST and the + // pin overlay TOGETHER and advance _panelGen so an in-flight async producer + // (search / hydration) can't resurrect the list after this clear. if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + viewer._panelGen++; const sampElClear = document.getElementById('samplesSection'); if (sampElClear) sampElClear.innerHTML = ''; if (searchInput) searchInput.value = ''; @@ -6197,30 +6217,29 @@ zoomWatcher = { // sample/pid detail + cluster hydrations become stale and bail — this // guards selection/card work. freshSelectionToken(viewer); - // (round-4, _panelGen) claim a fresh LIST generation and CAPTURE it. Every - // post-await list write below re-checks it (in addition to _searchSeq), so - // a newer list producer that started AFTER this search — e.g. a cluster - // click while the search is still building — makes this render bail - // instead of overwriting the cluster's rows. This closes the reverse race - // order. It does NOT break boot search+pid: the pid deep-link writes only - // the card and never bumps _panelGen, so this render still owns the list. + // (round-4/round-5, _panelGen) this submit is a write to the panel list, + // so advance _panelGen AND capture it — one bump serves both. As a bump it + // invalidates any in-flight async producer (unified rule); as this search's + // captured generation it is re-checked (with _searchSeq) before every + // render below, so a newer list producer that starts AFTER this submit + // (e.g. a cluster click while the search is still building) makes this + // render bail instead of overwriting the cluster's rows. Does NOT break + // boot search+pid: the pid deep-link is card-only and never bumps. const myPanelGen = ++viewer._panelGen; - // #172 pin-overlay Inc 1 (option C): clear the previous result-pin - // overlay on every committed submit (before repopulating). This covers - // both the new-search path (repopulated below on success) and the - // empty/too-short committed-clear path. Draft input edits never reach - // doSearch, so typing/backspace does not clear pins. + // #172 Inc 1 (Codex round-5 P1.2): a new submit tears the panel DOWN. + // Clear the old rows AND pins together AT ENTRY (under the gen just + // captured), so no stale unpinned rows linger during the async build — the + // panel shows the building status (searchResults text) until the atomic + // replacement lands. This covers BOTH the committed real search below and + // the empty/too-short branch (which just leaves the panel cleared). if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + const sampElEntry = document.getElementById('samplesSection'); + if (sampElEntry) sampElEntry.innerHTML = ''; const term = searchInput.value.trim(); if (!term || term.length < 2) { searchResults.textContent = 'Type at least 2 characters'; - // #172 Inc 1 (Codex round-3 P1): clear the results LIST here too so - // list and pins clear TOGETHER — otherwise stale search rows would - // survive under an empty overlay, breaking the snapshot invariant. - const sampElClear = document.getElementById('samplesSection'); - if (sampElClear) sampElClear.innerHTML = ''; // A1 (#234 Step 4): an empty / too-short submit means no committed // search — drop the pid-set filter and refresh every surface // (table, points, facet counts) plus revert the globe mode so the @@ -6510,6 +6529,12 @@ zoomWatcher = { // single check makes that write atomic w.r.t. panel ownership. if (!panelWriteAllowed(myPanelGen, viewer._panelGen)) { wasSuperseded = true; + // #172 Inc 1 (Codex round-5 P2): panel ownership and search-status + // completion are separate. A cluster took the panel, so we won't + // render — but if no NEWER search owns the status text (our + // searchId still current), settle our own "Searching…" to '' so it + // can't persist forever. If a newer search owns the text, leave it. + if (searchId === _searchSeq) searchResults.textContent = ''; return; } resultsCount = results.length; @@ -6892,18 +6917,21 @@ zoomWatcher = { const searchId = ++_searchSeq; // #172 Inc 1: take PANEL OWNERSHIP — same as doSearch. (round-3, _selGen) - // bump the selection token so in-flight selection hydrations bail; (round-4, - // _panelGen) claim + capture a LIST generation, re-checked before this - // render, so a cluster click that starts AFTER this concept commit wins - // the list instead of being overwritten. The concept render also stays - // guarded by searchId/_searchSeq. + // bump the selection token so in-flight selection hydrations bail; (round-4/ + // round-5, _panelGen) advance + capture a LIST generation — as a bump it + // invalidates in-flight async producers (unified rule); as this concept's + // gen it's re-checked before every render, so a cluster click that starts + // AFTER this commit wins the list. The render also stays guarded by + // searchId/_searchSeq. freshSelectionToken(viewer); const myPanelGen = ++viewer._panelGen; - // #172 pin-overlay Inc 1 (option C): committing a concept filter removes - // `search=` and owns the panel, so drop any lingering text-search - // result pins (a committed clear path for the text overlay). + // #172 Inc 1 (Codex round-5 P1.2): committing a concept tears the panel + // DOWN — clear the old rows AND the text-search pins together at entry + // (the panel shows "Filtering by concept…" until the concept list lands). if (viewer.searchResultPoints) viewer.searchResultPoints.removeAll(); + const sampElEntry = document.getElementById('samplesSection'); + if (sampElEntry) sampElEntry.innerHTML = ''; // Mutual exclusivity: a committed concept filter OWNS search_pids, so // clear the free-text box (and its sidebar mirror) before building. From 2f1ea849e80f393f35cedd5ffe87ec1c6b69edcd Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 16:26:02 -0700 Subject: [PATCH 14/15] test(playwright): assert new-search entry teardown of old rows (Codex round-5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new-search test now captures the first search's rendered row pids and, after submitting a disjoint second search, waits for the first row's pid to disappear — proving the entry-clear tears the old rows down at submit (it becomes true at the synchronous entry-clear and stays true), not just that pins eventually differ. Also asserts the rendered row set (not only the pin set) changed after replacement. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- tests/playwright/pin-overlay.spec.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/playwright/pin-overlay.spec.js b/tests/playwright/pin-overlay.spec.js index 6cc2508..63bd9f2 100644 --- a/tests/playwright/pin-overlay.spec.js +++ b/tests/playwright/pin-overlay.spec.js @@ -190,25 +190,38 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { expect((await pins(page)).length).toBe(0); }); - test('new search replaces the PIN set', async ({ page }) => { + test('new search replaces the PIN set AND tears down the old rows at submit', async ({ page }) => { await submitSearch(page, 'pottery Cyprus'); await waitPinsAtLeast(page, 1); const firstPinPids = (await pins(page)).map((p) => p.pid); + const firstRowPids = await page.$$eval('#samplesSection .sample-row', (els) => els.map((e) => e.dataset.pid)); expect(firstPinPids.length).toBeGreaterThan(0); expect(firstPinPids.length).toBeLessThanOrEqual(50); + expect(firstRowPids.length).toBeGreaterThan(0); await submitSearch(page, 'basalt'); + // Entry-clear (Codex round-5 P1.2): a new submit tears the OLD rows down — + // the prior search's rows must not persist under the new (or building) state. + // 'basalt' is disjoint from 'pottery Cyprus', so its first row pid must + // disappear (becomes true at the synchronous entry-clear and stays true). + await page.waitForFunction((pid) => { + const pids = Array.from(document.querySelectorAll('#samplesSection .sample-row')).map((e) => e.dataset.pid); + return !pids.includes(pid); + }, firstRowPids[0], { timeout: 120_000 }); + // Wait for the count line to reflect the NEW term, then for pins to repopulate. await page.waitForFunction(() => /results for "basalt"/i.test(document.getElementById('searchResults')?.textContent || ''), null, { timeout: 120_000 }); await waitPinsAtLeast(page, 1); const secondPinPids = (await pins(page)).map((p) => p.pid); + const secondRowPids = await page.$$eval('#samplesSection .sample-row', (els) => els.map((e) => e.dataset.pid)); expect(secondPinPids.length).toBeGreaterThan(0); expect(secondPinPids.length).toBeLessThanOrEqual(50); - // Disjoint corpora → the PIN set (not just the side panel) must change. + // Disjoint corpora → both the PIN set AND the rendered row set must change. expect(sortJoin(secondPinPids)).not.toBe(sortJoin(firstPinPids)); + expect(sortJoin(secondRowPids)).not.toBe(sortJoin(firstRowPids)); }); test('committed empty search clears the pins (snapshot lifecycle)', async ({ page }) => { From 84f5df82221622fa3beebe09d5084456b9ae001f Mon Sep 17 00:00:00 2001 From: Raymond Yee Date: Fri, 17 Jul 2026 16:35:31 -0700 Subject: [PATCH 15/15] test(playwright): deterministic entry-teardown observation (Codex round-6 P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior entry-teardown assertion (old pid disappears) could also pass once the second render replaced the rows. Replace it with a race-free construction: the submit's click handler is a plain `() => doSearch(...)` and doSearch runs synchronously up to its first await (the _panelGen bump, pin removeAll, and row innerHTML='' have no interposing await — verified), so drive the second submit AND read rows/pins in ONE page.evaluate. The old rows and pins are observed gone the instant the click dispatch returns, mid-build, before the new results can render — so the assertion cannot pass without the entry teardown. Also add the exact final identity assertion: pin pids === the new search's located row pids. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y --- tests/playwright/pin-overlay.spec.js | 48 +++++++++++++++++----------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/tests/playwright/pin-overlay.spec.js b/tests/playwright/pin-overlay.spec.js index 63bd9f2..21922a9 100644 --- a/tests/playwright/pin-overlay.spec.js +++ b/tests/playwright/pin-overlay.spec.js @@ -190,38 +190,50 @@ test.describe('search-result pin overlay (#172 Inc 1)', () => { expect((await pins(page)).length).toBe(0); }); - test('new search replaces the PIN set AND tears down the old rows at submit', async ({ page }) => { + test('new search tears down old rows+pins at submit, then renders the new set', async ({ page }) => { await submitSearch(page, 'pottery Cyprus'); await waitPinsAtLeast(page, 1); const firstPinPids = (await pins(page)).map((p) => p.pid); - const firstRowPids = await page.$$eval('#samplesSection .sample-row', (els) => els.map((e) => e.dataset.pid)); + const firstRowCount = await displayedRowCount(page); expect(firstPinPids.length).toBeGreaterThan(0); expect(firstPinPids.length).toBeLessThanOrEqual(50); - expect(firstRowPids.length).toBeGreaterThan(0); - - await submitSearch(page, 'basalt'); - // Entry-clear (Codex round-5 P1.2): a new submit tears the OLD rows down — - // the prior search's rows must not persist under the new (or building) state. - // 'basalt' is disjoint from 'pottery Cyprus', so its first row pid must - // disappear (becomes true at the synchronous entry-clear and stays true). - await page.waitForFunction((pid) => { - const pids = Array.from(document.querySelectorAll('#samplesSection .sample-row')).map((e) => e.dataset.pid); - return !pids.includes(pid); - }, firstRowPids[0], { timeout: 120_000 }); - - // Wait for the count line to reflect the NEW term, then for pins to repopulate. + expect(firstRowCount).toBeGreaterThan(0); + + // Entry teardown (Codex round-5/6): the submit's click handler is a plain + // `() => doSearch(...)`, and doSearch runs SYNCHRONOUSLY up to its first await + // — the _panelGen bump, pin removeAll, and row innerHTML='' all execute before + // any await. So drive the submit AND observe in ONE page.evaluate: the old rows + // and pins are already gone the instant the click dispatch returns, BEFORE the + // new results render. Deterministic — no timing race, and it CANNOT pass via + // the later re-render (we read before yielding to the event loop). + const observed = await page.evaluate(() => { + const input = document.getElementById('sampleSearch'); + input.value = 'basalt'; + input.dispatchEvent(new Event('input', { bubbles: true })); + document.getElementById('searchSubmitBtn').click(); + // click -> doSearch entry teardown has run synchronously by now. + return { + rows: document.querySelectorAll('#samplesSection .sample-row').length, + pins: window.__searchPins().length, + }; + }); + expect(observed.rows).toBe(0); // old rows torn down at submit, mid-build + expect(observed.pins).toBe(0); // old pins torn down with them + + // Let the second search complete, then assert the NEW set rendered. await page.waitForFunction(() => /results for "basalt"/i.test(document.getElementById('searchResults')?.textContent || ''), null, { timeout: 120_000 }); await waitPinsAtLeast(page, 1); const secondPinPids = (await pins(page)).map((p) => p.pid); - const secondRowPids = await page.$$eval('#samplesSection .sample-row', (els) => els.map((e) => e.dataset.pid)); + const secondLocated = await locatedRowPids(page); expect(secondPinPids.length).toBeGreaterThan(0); expect(secondPinPids.length).toBeLessThanOrEqual(50); - // Disjoint corpora → both the PIN set AND the rendered row set must change. + // Disjoint corpora → the PIN set changed. expect(sortJoin(secondPinPids)).not.toBe(sortJoin(firstPinPids)); - expect(sortJoin(secondRowPids)).not.toBe(sortJoin(firstRowPids)); + // Exact final identity: the pins are precisely the new search's located rows. + expect(sortJoin(secondPinPids)).toBe(sortJoin(secondLocated)); }); test('committed empty search clears the pins (snapshot lifecycle)', async ({ page }) => {