Skip to content

Fix search & navigation bugs: pagination, scroll, stats, and zero-result handling#71

Merged
wesm merged 16 commits intomainfrom
fast-search-quirks
Feb 5, 2026
Merged

Fix search & navigation bugs: pagination, scroll, stats, and zero-result handling#71
wesm merged 16 commits intomainfrom
fast-search-quirks

Conversation

@wesm
Copy link
Owner

@wesm wesm commented Feb 5, 2026 •
edited
Loading

Summary

Comprehensive fixes for search, navigation, and pagination bugs in the TUI. Multiple serious issues were discovered during QA and fixed incrementally.

Fixes #42, #69.

Search engine fixes

  • Stale search stats: Header showed zero size/attachments after the first search because GetTotalStats ignored SearchQuery — fixed to include search filter in stats queries
  • Temp table race: DuckDB search created a new temp table per query without cleanup — refactored to single-scan materialization with proper lifecycle
  • Repeated Parquet scans: Each pagination call re-scanned Parquet files — added temp table caching across pagination calls, reusing materialized results
  • Cached stats mutation: UI could corrupt cached TotalStats pointer — now returns a defensive copy

TUI crash and rendering fixes

  • Zero-result search crash: Fast search returning no results broke the TUI (search bar disappeared, required force quit) — fixed early return to preserve search UI, added "No results found" indicator
  • Stats not updating on subsequent searches: After the first search, only message count updated when typing more — fixed replaceSearchResults to prioritize fresh stats over stale drill-down stats

Navigation and scroll fixes

  • Cursor off-bottom: Cursor could scroll onto the info/notification line because ensureCursorVisible used pageSize but views render pageSize-1 data rows — fixed to use visible row count
  • Fast search pagination broken: navigateList early return prevented maybeLoadMoreSearchResults from ever firing — moved pagination check before the early return
  • Message list pagination missing: Non-search message lists were hard-capped at 500 with no way to load more — added infinite-scroll pagination (500-item pages, auto-loads when cursor nears end)

Code quality

  • Cache key uses JSON encoding to prevent delimiter collisions
  • SQLite/DuckDB count queries are best-effort (return -1 on failure instead of aborting)
  • dropSearchCache uses context.Background() for cleanup that must not be skipped
  • Pagination gated on navigation intent (not cursor change) so pressing down at last item still triggers load

Test plan

  • TestSearchCacheKeyFor — 7 table-driven cases for cache key construction
  • TestSearchFastWithStats_CacheHitSkipsRescan — verifies no re-materialization on cache hit
  • TestSearchFastWithStats_CacheInvalidatedOnNewSearch — verifies cache invalidation
  • TestZeroSearchResultsRendersSearchBar — 3 subtests for zero-result rendering
  • TestSearchStatsUpdateOnSubsequentSearch — stats refresh on re-search
  • TestSearchStatsUpdateOnDeleteKey — stats refresh on backspace
  • TestDrillDownStatsPreservedWhenSearchHasNoStats — drill-down stats preservation
  • TestFreshStatsOverrideDrillDownStats — fresh stats take priority
  • TestFastSearchPaginationTriggersOnNavigation — 5 subtests including cursor-at-end edge case
  • TestMessageListPaginationTriggersOnNavigation — 6 subtests for non-search pagination
  • TestDetailNavigationFromThreadView — updated scroll offset expectations
  • All existing tests pass, lint clean

🤖 Generated with Claude Code

wesm and others added 9 commits February 5, 2026 08:14
@wesm @claude
Fix search header showing zero size and attachments
feeb7cc 
When searching from top-level (no drill-down), the header metrics showed
correct message count but zero size and zero attachments. The root cause
was replaceSearchResults creating contextStats with only MessageCount,
leaving TotalSize and AttachmentCount at their zero values.

Fix: call GetTotalStats with the search query during initial search
execution to fetch accurate aggregate stats (size, attachment count),
and use them in replaceSearchResults when no drill-down stats exist.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Refactor fast search to single-scan temp table materialization
e356ecf 
Replace 3-4 separate Parquet scans in loadSearchWithOffset with a single
SearchFastWithStats call that materializes matching messages into a DuckDB
temp table, then reuses it for count, pagination, and stats. The temp
table stores denormalized message data (including sender info), so later
phases never re-read msg Parquet — only small page-scoped label/attachment
lookups remain.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Fix SQLite GetTotalStats ignoring SearchQuery and prevent temp table …
69917eb 
…races

Two fixes:

1. SQLiteEngine.GetTotalStats now respects opts.SearchQuery by parsing it
   with search.Parse() and applying conditions via buildSearchQueryParts().
   Previously it returned global stats regardless of search filters. When
   search joins are present, a subquery pattern avoids duplicate counting
   from 1:N joins.

2. DuckDB SearchFastWithStats now uses unique temp table names (via atomic
   counter) to prevent concurrent goroutines from clobbering each other's
   temp tables. The old fixed name "_search_matches" could be dropped or
   replaced by an overlapping search goroutine.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Cache search temp table across pagination to avoid repeated Parquet s…
b2f3bdd 
…cans

SearchFastWithStats now keeps the materialized temp table alive between
calls with the same conditions+args (e.g. page down/scroll). On cache hit,
Phase 1 (Parquet scan) is skipped entirely and the page is served from the
cached in-memory table. A new search invalidates the old cache automatically.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Fix TUI breaking when fast search returns zero results
6b15f80 
When a search returned no matches, the early return in messageListView()
skipped the entire rendering including the search bar, making it impossible
to see or edit the query. Now the early return only fires for non-search
empty states; search contexts render the full layout with a "No results
found" indicator, the search bar, and "(0 results)" in the info line.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Address review findings across search refactor commits
1116002 
Fixes from reviews #4602, #4603, #4604:

- Cache key: use JSON encoding instead of delimiter-based concatenation
  to prevent ambiguous collisions (e.g. args containing commas/pipes)
- SQLite SearchFastWithStats: make count best-effort (log + use -1)
  instead of aborting the entire search on count failure
- DuckDB SearchFastWithStats: same best-effort count behavior
- source_id: remove COALESCE(source_id, 0) in temp table so NULL
  source_ids don't inflate COUNT(DISTINCT) account stats
- dropSearchCache: always use context.Background() so cleanup succeeds
  even when the caller's context is canceled, preventing leaked temp
  tables on the single DuckDB connection
- Stats retry: if computeSearchStats fails (transient error), retry on
  next cache-hit pagination instead of permanently caching nil stats
- SQLite attachment stats: only use IN subquery when search joins are
  present; use direct JOIN when no search is active (faster path)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Add tests for zero search results rendering
5815354 
Covers the three key scenarios from review #4605:
- Inline search active with zero results: search bar visible, "No results found" shown
- Completed search with zero results: "(0 results)" in info line, query visible
- Non-search empty state: "No messages" shown without search UI

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Add tests for search cache key, cache hit/miss, and empty state UI
faf7776 
Addresses review testing gaps from #4606 and #4607:

- TestSearchCacheKeyFor: table-driven tests verifying JSON cache keys
  avoid collisions from delimiters, type differences, and special chars
- TestSearchFastWithStats_CacheHitSkipsRescan: paginates the same search
  twice, asserts no new temp table is created and count/stats are stable
- TestSearchFastWithStats_CacheInvalidatedOnNewSearch: changes the search
  query and asserts cache is invalidated (new temp table created)
- Non-search empty state: negative assertions for search bar prefix
  "[Fast]/", "(0 results)" to fully cover "without search UI"

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Fix search stats not updating on subsequent searches
40e87f6 
When the user modified a search query (typing more or pressing delete),
only MessageCount updated in the header — TotalSize and AttachmentCount
stayed stale from the first search. The bug was in replaceSearchResults()
where hasDrillDownStats incorrectly treated stats from a previous search
as drill-down stats, preventing fresh stats from being applied.

Fix: when msg.stats != nil (fresh stats from SearchFastWithStats), always
use them. Only fall back to preserving existing stats when no fresh stats
are available (e.g. deep/FTS search which doesn't compute aggregates).

Adds 4 regression tests covering:
- Subsequent search updates all stats fields
- Delete key (broadening search) updates all stats fields
- Drill-down stats preserved when search has no fresh stats
- Fresh stats override even existing drill-down stats

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm
Copy link
Owner Author

wesm commented Feb 5, 2026

Pagination has some issues, so I'm working on those now

wesm and others added 2 commits February 5, 2026 09:44
@wesm @claude
Fix cursor off-bottom and fast search pagination bugs
3d2e5f2 
Fix two issues: (1) cursor could scroll onto the info/notification line
because ensureCursorVisible used pageSize as the visible window, but
views only render pageSize-1 data rows. (2) Fast search pagination
never triggered because navigateList's early return exited before
maybeLoadMoreSearchResults could fire.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Address review findings: cached stats mutation, pagination gating, tests
785ae1c 
- Return a copy of TotalStats from searchPageFromCache to prevent UI
  mutations from corrupting the cache (review #4610)
- Gate maybeLoadMoreSearchResults behind a cursor-changed check so
  non-navigation handled keys don't trigger unnecessary I/O (#4612)
- Add TestFastSearchPaginationTriggersOnNavigation with 4 subtests
  covering: near-end triggers load, far-from-end skips, all-loaded
  skips, and deep-mode skips (#4612)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm wesm mentioned this pull request Feb 5, 2026
Closed
wesm and others added 2 commits February 5, 2026 09:55
@wesm @claude
Fix pagination gate: trigger on navigation intent, not cursor change
66d59f9 
Remove the cursorMoved gate for maybeLoadMoreSearchResults since it
blocked pagination when cursor was already at the last loaded item
(pressing down couldn't move cursor, so pagination never triggered).
navigateList only returns handled=true for navigation keys, so using
handled as the gate is sufficient. Add test for cursor-at-end case.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Add infinite-scroll pagination for non-search message lists
af9ef07 
Previously, message lists were capped at 500 results with no way to
load more. This adds pagination mirroring the fast search approach:
messages load in 500-item pages, and navigating within 20 rows of the
end triggers loading the next page (appended to the existing list).

Also addresses review #4613: remove overly-strict cursorMoved gate
that blocked pagination when cursor was already at the last item.

Changes:
- Extract buildMessageFilter from loadMessages for reuse
- Add loadMessagesWithOffset with append mode support
- Add maybeLoadMoreMessages (mirrors maybeLoadMoreSearchResults)
- Add msgListOffset/msgListLoadingMore state fields
- Handle append flag in messagesLoadedMsg/handleMessagesLoaded
- Reset pagination state on fresh message list entry points
- Add TestMessageListPaginationTriggersOnNavigation (6 subtests)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm wesm changed the title Fix fast search bugs: stale stats, zero-result crash, and pagination perf Fix search & navigation bugs: pagination, scroll, stats, and zero-result handling Feb 5, 2026
wesm and others added 3 commits February 5, 2026 10:08
@wesm @claude
Fix pagination state in breadcrumbs, add end-of-data detection
38adda4 
Move msgListOffset/msgListLoadingMore into viewState so breadcrumb
snapshots preserve pagination position across navigation. Add
msgListComplete flag set when an append returns zero messages,
preventing infinite empty load requests when total is an exact
multiple of page size.

- Move pagination fields from Model to viewState (review #4615 medium)
- Add msgListComplete end-of-data flag (review #4615 low)
- Reset msgListComplete on fresh loads and new list entry points
- Add TestMessageListPaginationBreadcrumbRestore (2 subtests)
- Add empty-append and fresh-load-reset subtests

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Reset stale msgListLoadingMore on breadcrumb restore
77cc516 
When a load-more pagination request is in-flight and the user navigates
forward (e.g. to detail view), the breadcrumb snapshot captures
msgListLoadingMore=true. The in-flight request becomes stale and gets
discarded, so goBack must clear the flag to allow fresh pagination.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm @claude
Add hidden --no-sqlite-scanner flag for cross-platform QA
6916da7 
Allows disabling the DuckDB sqlite_scanner extension on Linux/macOS to
exercise the direct SQLite fallback code path (normally Windows-only).
Added to both tui and mcp commands as a hidden flag.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@wesm wesm merged commit 2305d72 into main Feb 5, 2026
2 checks passed
@wesm wesm mentioned this pull request Feb 5, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TUI: the search field disappears

1 participant

@wesm