perf(sessions): page archived sidebar rows - #5200
santastabber wants to merge 1 commit into
Conversation
🔬 Gate certification — RED ⛔ (correct paging, but a SILENT search-archived gap — reproduced)Certified head: What I ran (isolated worktree
|
| Gate | Result |
|---|---|
| Codex (reproduce) | SHIP ONLY WITH FIXES — SILENT: filtered archived rows past page 1 unreachable |
| Opus (full review) | COMMENT — no blockers; different minor nits (didn't catch the search gap) |
Full pytest suite (-p no:xdist, rebased) |
11105 passed, 0 failed (green — but no test covers search+archived+page-2, so it masks the gap) |
| PR's own + touched tests | 30 passed |
node --check sessions.js |
OK |
⛔ Blocking (SILENT, reproduced) — search/filter over archived silently drops matches past page 1
_sessionListQueryString() (sessions.js:1849-1851) sets archived_limit whenever _showArchived is on — but not gated on filter state. Meanwhile filtering is client-side, and the archived "Load more" control is shown only if(_showArchived && !archivePagingFilterActive) where archivePagingFilterActive = Boolean(searchQueryRaw || _activeProject) (sessions.js:~6476). So when "Show archived" + a search or project filter are both active:
- the server caps archived rows at the first page (
archived_limit), - the client filters only that first page,
- and the load-more that would fetch the rest is hidden → there is no way to reach archived sessions matching the search/filter beyond page 1.
Before this PR all archived rows were loaded, so search found them all. This is a silent search-correctness regression ("my archived session isn't in search results" with no indication) — exactly the existing-flow-reliability class Nathan prioritizes. Moderate likelihood (power users with large archives who search/filter with archived shown).
Fix-spec (Codex, correct): do not send archived_limit when a search/project filter is active (($('sessionSearch').value||'').trim() || _activeProject) — fetch the full archived set for client-side filtering — OR make the load-more filter-aware so filtered archived pagination can complete. Add a test for "search with archived shown finds a match on archived page 2."
What's correct (the paging core is good — keep it)
- ✅ Visible/active rows always render regardless of archived paging (
scoped = visible_rows_for_page + archived[offset:offset+limit]) — test-covered. - ✅ Cache key includes normalized
archived_limit/archived_offset(route_session_list_cache.py) — no stale cross-page serving; dedicated test asserts the key varies. No WAL-sidecar/state.db staleness regression. - ✅ Bounds enforced (
archived_limit≤ 2000,archived_offset≤ 200000; negatives clamped). No perf: /api/sessions ships 465 rows (342KB) when the sidebar displays 48 #4766 partition regression (30 tests pass). - ✅ Pagination slice is off-by-one-clean; client re-sort makes server reordering safe (Opus verified).
Opus's non-blocking nits (fold in with the fix)
-
2000 archived "Load more" soft dead-end (button stays visible but server clamps to 2000 → same rows; extremely rare) — hide the button once the cap is hit.
- Error-fallback mismatch: bad-type
archived_limit→ cache-key falls back toNone, payload to0(unreachable via HTTP; align them). - Redundant
... or 0onarchived_offset. 4. No CHANGELOG (maintainer adds at merge).
Recommendation to the next agent
Bounce for the search-under-filter archived fix (Codex's #1), then re-gate. The paging perf work is correct and well-tested; the gap is that archived paging + client-side filtering interact badly (filtered matches past page 1 become unreachable). Fix is targeted (gate archived_limit on no-active-filter, or filter-aware pagination) + a regression test. Fold in Opus's >2000 dead-end + fallback-mismatch nits while there. I applied gate-fail; the search gap is a reproduced SILENT regression, not a nit. Cert valid only at sha:f6d174fb9c71.
Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy/close. Cert valid only at sha:f6d174fb9c71; a new push invalidates it → re-gate.
f6d174f to
7099455
Compare
|
Updated this PR to address the review/gate feedback. Changes in the latest head:
Verification on the updated head:
|
|
Re-gated the convergence — the gaps from the first gate read are addressed (paging disabled under search/project filters at the The remaining issue (SILENT — archived rows unreachable): the archived-paging suppression is applied when Fix-spec (Codex, sound):
The |
|
Updated again for the latest gate comment. Additional fixes in this head:
Verification on the updated head:
|
7099455 to
7ff5218
Compare
…ch refetch (#5200, @santastabber) Release v0.51.757 — page archived sidebar rows + archived-search refetch (#5200, @santastabber)
|
Shipped in v0.51.757 (via #5241). Thanks @santastabber — archived sidebar rows are now paged (no more loading thousands at once), and the search-input gap I flagged is fixed: with Show-archived on, starting a search transparently refetches without the archive cap so title/id matches beyond the first archived page are reachable, restoring normal paging when search clears. Converged from the gate-fail; re-gated Codex SAFE (no refetch thrash — render-generation guard; default paging + 2000-row cap intact), full suite 11075 passed. Closes #5200. |
Summary
archived_limitquery parameter so the initial archive toggle no longer fetches/renders the entire archived historyVerification
python3 -m py_compile api/routes.py api/route_session_list_cache.pynode --check static/sessions.jsgit diff --check./scripts/test.sh tests/test_session_list_long_history_perf.py tests/test_sidebar_session_partition.py tests/test_issue4766_sidebar_source_pushdown.py tests/test_session_sidebar_cache.py -q→ 47 passedgitleaks git --no-banner --redact --timeout 90 --report-format json --report-path /tmp/gitleaks-webui-archive.json --log-opts <base>..<head> .→ 0 findingstrufflehog git file://<temp-clone> --since-commit <base> --branch HEAD --max-depth=1 --include-paths <changed-paths> --json --no-update --force-skip-archives --force-skip-binaries --detector-timeout 10s→ 0 findingscoderabbit review --agent --type committed --base origin/master→ 0 findings after addressing one minor local finding