Table v2: server-side pagination + stale-while-loading#218
Merged
Conversation
Replace the up-front "load up to N rows; paginate in JS" model with
true server-side pagination via DuckDB LIMIT/OFFSET.
Architecture:
- One DuckDB query per page (LIMIT TABLE_PAGE_SIZE OFFSET p*size).
- One COUNT(*) query per filter change (cached until filters change
again — page navigation does not refire it).
- ORDER BY pid so "Page N is always the same N rows" is actually true.
- Stale-while-loading: existing rendered rows stay visible (dimmed to
60% opacity via #tableContainer.is-loading) while the new page or
count is fetched in the DuckDB Web Worker. aria-busy="true" on the
container signals to screen readers.
- A small CSS-only spinner appears in #tableMeta during loads. Honors
prefers-reduced-motion (animation: none).
- pageGen counter invalidates in-flight callbacks when a newer load
starts, similar to the freshSelectionToken pattern.
UI removed:
- #tableControls + #maxSamples input. Capping at 25K-100K rows was a
relic of the toggle-mode UI where the table was an opt-in surface;
with server-side paging the user pages through whatever the filter
set is, no client-side memory pressure.
- TABLE_DEFAULT_MAX, TABLE_MIN_MAX, TABLE_MAX_MAX constants and
getTableMaxSamples() / clampTableMaxSamples() helpers — all
unused.
Pager UX:
- Prev/Next buttons disabled during in-flight loads to prevent the
user queueing a second click while a query is in flight.
- Meta line shows "Loading samples matching filters..." then
"{N} samples match the current filters." once count returns.
- If page-data returns before count (smaller query, usually does),
pager shows "Page N" without total; updates to "Page N of M
(start-end of total)" when count lands.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex flagged two blockers + recommendations on bf49003: 1. Stale tests in test_globe.py — both test_has_max_samples_input and test_has_view_toggle expected DOM elements that no longer exist (#maxSamples removed in table v2; view toggle removed in PR isamplesorg#200). Replaced with a test_has_permanent_table assertion that confirms the new contract: tableContainer + samplesTable + pager present, no #maxSamples, no view-toggle buttons. 2. loadPage() error path was swallowed — the inner setMeta error message was being overwritten by setMeta(summaryText()) in the orchestrator. Reworked: loadCount and loadPage now return true/false; refreshAll/refreshPage only call setMeta(summary) on success, and surface specific error meta otherwise. Three distinct error meta lines now: page-failed, count-failed-only, page-failed-during-page-change. Plus Codex recommendations folded in: - AND pid IS NOT NULL guard on the page query — belt-and-suspenders for the ORDER BY pid determinism contract. - setMetaLoading() now clears #tablePageInfo too, so a filter change doesn't leave stale "Page 3 of 12 (200-300 of 1,200)" text visible while the new filter set is being counted. - EXPLORER_STATE.md §3 updated to reflect rowsByPid → pageRowsByPid (now scoped per page); #maxSamples removed from the input-source paragraph; new "Table v2 addendum" added to §6 covering pagination contract, determinism, stale-while-loading, race protection, and error handling. - Two new Playwright tests in explorer-map-overlay.spec.js: - server-side pagination: Next button advances to Page 2 with different rows, no #maxSamples element - filter change: aria-busy flips true→false, pager text updates to reflect the new filtered total Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three blockers from round 2:
1. Page-query failure left the OLD DOM visible while pageRowsByPid
was empty — rows looked clickable but did nothing. Added a
lastPageFailed flag; renderTable() swaps the table body for an
explicit sentinel row ("Page query failed. Adjust filters or click
Previous/Next to retry.") when the flag is on. Pager text is also
cleared in the failure branch. Reset on the next successful
loadPage so a recovery returns to normal rendering.
2. Next button stayed enabled when totalRows == null (count failure)
even though the click handler bailed immediately — confusing UX.
updatePagerEdges() now disables Next whenever totalRows == null,
regardless of currentPage.
3. COUNT(*) query didn't have the WHERE pid IS NOT NULL guard. With
ORDER BY pid + IS NOT NULL on the page query only, totalRows
could overcount and the pager could enable Next past the last
non-null page. Added the matching predicate to the count query
for consistency.
Plus updates to EXPLORER_STATE.md table v2 addendum: the determinism
note now mentions BOTH queries carry the null filter; the error
handling section now describes all three failure modes (page-failed,
count-failed-only, both-failed) and the explicit sentinel table
state.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 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.
Follow-up to #200. The user asked: "do we need to set max samples to 25000 — seems like we don't need to cap things but just set a page size, right?" With the samples table now permanent (not toggle-gated), server-side pagination is the cleaner model.
Summary
LIMIT TABLE_PAGE_SIZE OFFSET page*size.COUNT(*)query per filter change; not refired on page change.ORDER BY pidplusWHERE pid IS NOT NULL(on both queries) — "Page N is always the same N rows" actually true.#tableContainer.is-loading) while the new query runs in the DuckDB Web Worker. CSS-only spinner in#tableMeta.aria-busy="true"for screen readers.prefers-reduced-motionhonored.pageGencounter invalidates in-flight callbacks when a newer load starts.#maxSamplesinput +getTableMaxSamples()/clampTableMaxSamples()removed.Codex review trail
Three rounds to LGTM:
tests/test_globe.pyreferences to#maxSamples+ view toggle;loadPageerror meta was being overwritten by success summary; missingIS NOT NULLguard on COUNT.pageRowsByPid→ silent click no-ops; Next button stayed enabled whentotalRows == null; addendum overstated page-failure handling.Test plan
explorer-map-overlay.spec.js:#maxSampleselementtests/test_globe.pyrewritten —test_has_max_samples_input+test_has_view_togglereplaced withtest_has_permanent_tableState-contract delta (
EXPLORER_STATE.md§6 "Table v2 addendum")rowsByPid: Map→pageRowsByPid: Map, scoped to current page only#tableContainer[aria-busy]+#tableContainer.is-loadingare new state-marker surfaces#maxSamples,getTableMaxSamples(),clampTableMaxSamples()removed from the state inventory🤖 Generated with Claude Code