Explorer: search-result pin overlay, Inc 1 (option C, #172 track)#337
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
…nc 1, option C)
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
…Inc 1) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
… overlay - 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 (isamplesorg#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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
- 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
…ound-2 P2) - 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
…dex round-3 P1) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
- 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
…orders (Codex round-4 P1) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
…ex round-4 P2) - 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
…s settle (Codex round-5) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
… round-5) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
…nd-6 P2) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y
rdhyee
added
the
CC+Codex+LGTM
Contributor
Author
Live acceptance: ALL PASS
Console: 0 errors across the whole run. Not exercised live: pin-click and Back-after-pin-click (the Ready for merge on Raymond's call. |
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BLUF (plain English)
After a search, the Explorer now drops pins on the globe showing where your matches are — one hollow ring per located result, colored by source, capped at the 50 displayed results. Click a pin and it behaves exactly like clicking a sample. This is Increment 1 of the "option C" search UI the team adopted in May (#164/#165): purely additive — search still filters the table/facets exactly as before. The speed-focused Increment 2 (search stops refiltering every surface, behind a
searchui=pinflag) comes next; this PR deliberately does not change any filtering behavior. The #172 gate stays open, as does #165.What's in it
viewer.searchResultPoints— third primitive collection (above clusters and sample points), hollow-ring styling per the EXPLORER_STATE.md option-C contract; hover + click parity via a sharedselectSearchResult()helper (row clicks and pin clicks use the same ceremony)viewer._panelGen; async producers capture at start and may only write if still current. 11 audited sites; purepanelWriteAllowed()helper inassets/js/explorer-utils.jswith a unit test. This closes a pre-existing race family (search rows vs nearby-samples rows interleaving across awaits) that pins made visible.tests/playwright/pin-overlay.spec.js: 8 tests — four contract shape cases (zero/one/local-many/global-many with exact pin↔row identity), pin-set replacement, committed-clear, row-click preservation with real-selection assertion, Back-after-pin-click regression (via anavigator.webdriver-gated hook that replays the real ceremony), deterministic same-tick entry-teardown observation; spec honors PlaywrightbaseURL(no production fallback)PLAN_PIN_OVERLAY_2026-07-17.md(amended through the rounds)Review trail (6 Codex rounds, all findings addressed)
__searchFilter.activehas 7 independent consumers) — corrected in plan before any code_panelGenEach round also verified: OJS cell compilation,
quarto render, 111 node unit tests, 40 frontend-derived tests,git diff --check.Known follow-ups (not in this PR)
writeQueryStateincoherence (draft-cleared input + facet toggle dropssearch=from URL while the runtime filter survives) — issue to be filedfts-v1.spec.jsstill has a hard-coded production URL fallback — separate small fixsearchui=pinmode via asearchFiltersSurfaces()predicate + fit-to-bounds cameraVerification before merge
Live staging run (fork Pages dispatch from this branch) + browser acceptance of the four shape cases and Back-after-pin-click. Will be posted in comments.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Nohddj7oXLnkJ4P9Cibg8Y