Skip to content

fix(desktop): stop timeline cap from evicting scrolled-in history mid-read#1473

Open
tlongwell-block wants to merge 1 commit into
mainfrom
fix/timeline-cap-eviction
Open

fix(desktop): stop timeline cap from evicting scrolled-in history mid-read#1473
tlongwell-block wants to merge 1 commit into
mainfrom
fix/timeline-cap-eviction

Conversation

@tlongwell-block

Copy link
Copy Markdown
Collaborator

The bug (Tyler's "missing messages" report)

Sometimes all of June 12's messages show in #buzz-bugs; sometimes one or two; sometimes none.

Root cause: MAX_TIMELINE_MESSAGES = 2000 cap, applied inconsistently across merge paths. #buzz-bugs holds 2,770 kind-9 events since June 12 (measured via CLI pagination against the relay; the June-12 set itself is a stable 37 ids across 8+ repeated reads — the relay is fine). June 12 sits past the cap.

The flicker mechanism:

  1. Scrollback merges were uncapped (isOlderHistoryPage special case) — you can page back to June 12 and see all 37. Cache now holds 2,700+ content events.
  2. The next capped merge — any live event (mergeTimelineCacheMessages), or a fresh-history revalidation whose merge fell into the non-older branch of mergeTimelineHistoryMessages — trims the cache back to the newest 2,000, silently evicting June 12 while it's on screen.
  3. Where the cap boundary lands decides what survives: all 37, a couple, or none. Exactly the reported symptom.

Reproduced with the repo's own normalizeTimelineMessages: 37 "June 12" rows + 2,764 newer rows → 0 survivors.

Predates the current perf PRs — the cap is from #212; #1105 refined it to count content events; #1452's longer-lived caches (gcTime 60min + persisted snapshots) made caches big enough for people to hit it routinely.

The fix

Principle (per review discussion with Wren): no merge into a potentially-on-screen timeline applies the cap; the cap is enforced only at moments when nothing is rendered from the cache.

  • mergeTimelineHistoryMessages → sort + dedupe only. Drops the isOlderHistoryPage branch split entirely — older-page merges and newest-window revalidations now behave identically, so a background refetch over an expanded cache can no longer evict scrolled-in history (this covers the revalidation-without-live-event case).
  • mergeTimelineCacheMessages (live append / optimistic send) → same, now an alias of mergeMessages.
  • The DOM bound survives where it's safe:
    • cold snapshot paint (placeholderData) — already capped, unchanged;
    • new: trim-on-leave in useChannelSubscription's cleanup — leaving the channel re-applies normalizeTimelineMessages, so a long session can't grow caches unboundedly.

Net: while a channel is open its timeline only grows (bounded in practice by scrollback batches, 200/page × 3/pass); the moment you leave, it trims to 2,000 newest.

Validation

  • New regression test: 2,537-event expanded cache + fresh newest-page merge → all 37 old rows survive (red on main, green here).
  • pnpm --dir desktop test: 1495/0. pnpm check, pnpm --dir desktop typecheck: clean.
  • All existing cap tests (aux events don't consume the window, huddle starts count, capped old content) still pass — the capped normalizer itself is unchanged.

Fixes the merge-freeze blocker reported in #buzz-gui-performance.

…-read

A channel timeline cache holding more than MAX_TIMELINE_MESSAGES (2000)
content events — i.e. the user paged back to an old day in a busy
channel — was silently trimmed back to the newest 2000 by the next
capped merge: any live event, optimistic send, or fresh-history
revalidation. The rows being read vanished out from under the reader,
appearing as randomly missing messages for old dates (all present right
after scrollback, partially or fully gone seconds later).

Scrollback merges were already uncapped (isOlderHistoryPage special
case), which is what made the loss intermittent instead of consistent.

Fix: no merge into a potentially-on-screen timeline applies the cap.
- mergeTimelineHistoryMessages: sort + dedupe only (drops the
  isOlderHistoryPage split — both branches now behave identically).
- mergeTimelineCacheMessages (live/optimistic/single-event): same.
- The DOM bound is enforced where nothing is rendered from the cache:
  the cold-snapshot paint (already capped) and a new trim-on-leave in
  useChannelSubscription cleanup, which re-applies
  normalizeTimelineMessages when the user leaves the channel.

Regression test: expanded 2537-event cache + fresh newest-page
revalidation merge — all old rows survive.

Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants