Skip to content

Fix false timeline catch-up indicators and scroll shifts - #5068

Merged
ymichael merged 2 commits into
mainfrom
bb/timeline-shift-on-unread-thr_u3dmswjps5
Oct 6, 2026
Merged

ymichael merged 2 commits into
mainfrom
bb/timeline-shift-on-unread-thr_u3dmswjps5

Conversation

@ymichael

@ymichael ymichael commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Human comments

What was wrong

Opening an unread thread could show “Loading latest messages…” even when its cached timeline already included the notified events. Catch-up state tracked whether an event arrived while the timeline was inactive, rather than comparing the notification with the loaded snapshot. Delayed/coalesced notifications could therefore mark current data as behind. The loading row also remained visible for at least 320 ms and occupied timeline space; collapsing it moved visible messages by 44 px in the browser reproduction.

What changed

  • Include metadata.timelineSequence in event-append notifications and preserve the highest sequence through server coalescing and client debouncing.
  • Compare the known event sequence with the cached timeline's maxSeq. A response acknowledges only events through its own sequence, so an older response cannot clear a newer pending change. Notifications without a sequence still refresh the cache without claiming that messages are missing.
  • Replace the inline loading row with an overlay. Reveal it only after the timeline remains behind for one second; fast catch-ups stay silent. Hide it immediately when caught up, with no minimum visible time. Switching threads or starting another catch-up resets the timer. Initial loads retain the loading skeleton.
  • Document catch-up behavior in docs/timeline-pagination.md.

The additive notification field is on the server-to-client realtime contract. Server/host-daemon wire payloads are unchanged, so no daemon protocol bump is needed. Existing CLI/SDK timeline and notification surfaces carry the change without new commands or settings.

How you verified

  • The initial sequence/layout verification passed 274 focused tests: app cache/controller/surface (116), server hub/event writes/ingestion (54), database events (96), and domain notification contracts (8). Ran through Turbo with the affected test-file filters.

  • pnpm exec turbo run lint typecheck --filter=@bb/app --filter=@bb/server --filter=@bb/db --filter=@bb/domain passed; existing app lint warnings remain. git diff --check passed.

  • Regression tests failed against the old implementation for already-covered notifications, premature stale-state clearing, and the indicator remaining visible after catch-up.

  • Built and launched pnpm start:worktree; verified healthy server/daemon and created two real provider conversations through the branch-built BB CLI. A cached sequence-39 timeline showed loading while its actual sequence-52 response was held, then displayed the reply and removed loading in the same observed DOM update.

  • Observed a real coalesced sequence-60 notification arrive after sequence 64 was loaded. Reopening with the refresh held for 650 ms showed no loading indicator. Cold-load skeleton, reload persistence, CLI event pagination, and read-state checks also passed.

  • Desktop Chromium measurements with unchanged messages: indicator removal moved the last message 44 px before, 0 px after, repeated three times. Native/mobile engines were not tested.

  • Follow-up reveal-delay verification: all four timeline-surface tests plus app lint/typecheck passed. Tests cover a 900 ms catch-up staying silent, the 999/1000 ms reveal boundary, immediate hiding, and timer resets. Rebuilt the worktree server and observed first visibility at approximately 1.05 seconds, unchanged message position, and hiding approximately 7 ms after releasing the real response.

AGENT GENERATED

@ymichael
ymichael merged commit 502e497 into main Oct 6, 2026
41 checks passed
@ymichael
ymichael deleted the bb/timeline-shift-on-unread-thr_u3dmswjps5 branch October 6, 2026 22:39
ymichael added a commit that referenced this pull request Oct 7, 2026
## Human comments

## What was wrong

#5068 fixed when "Loading latest messages…" shows and hides, but it also
moved the indicator. The row at the end of the timeline became a sticky
pill, fixed 8px below the top of the scroll area against the right edge
of the reading column. Because it floated over the column with no
reserved space, it covered message text in the middle of the pane. The
move was unintentional.

## What changed

- `ThreadTimelineSurface.tsx`: removes the sticky overlay wrapper. The
indicator is again a shimmering `TimelineStatusIndicator` row inside a
`HeightTransition`, after the timeline rows and before the working
indicator.
- #5068's show/hide behavior is unchanged. The flag is still based on
comparing event sequences. The row appears only after the timeline has
been behind for one second and disappears as soon as it catches up. The
timer restarts when you switch threads or a new catch-up begins.
- `docs/timeline-pagination.md`: the catch-up paragraph now describes a
row at the end of the timeline instead of an overlay.

Tradeoff: since it's a row again, removing it can shift the messages,
which #5068 avoided. The one-second delay means this only happens on
slow catch-ups.

## How you verified

- `pnpm exec turbo run typecheck lint --filter=@bb/app` passed; the
existing lint warnings are unchanged.
- `ThreadTimelineSurface.test.tsx` (4 tests, including the reveal delay,
immediate hide, and per-thread timer reset) passed through Turbo.
- In the `thread/timeline/Catch-up indicator` Ladle story with "Catching
up" on, the row renders under the last reply, above the composer and
aligned with the message text. It no longer overlaps the conversation.

> AGENT GENERATED

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant