Skip to content

feat(conversation): add useHistoryWindow and ConversationEarlier - #122

Merged
TheGreatAxios merged 7 commits into
conversation/04-messagefrom
conversation/06-history-window
Oct 28, 2026
Merged

TheGreatAxios merged 7 commits into
conversation/04-messagefrom
conversation/06-history-window

Conversation

@pratikbuilds

@pratikbuilds pratikbuilds commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

useHistoryWindow keeps a window of the latest items, adds earlier ones as the reader nears the top, freezes while the reader has scrolled away and trims back once they return to the end. It works with history the host pages in asynchronously, and the item in view stays put within a pixel as earlier items load. ConversationEarlier marks the top with a loading state and a retry when a page brings nothing. The doc comment records why this windows rather than virtualizes, and its ceiling.

Stacked on #121: review only this commit. Part of #94.

Notes for review

  • stalled only applies while the window starts at the first held item (the only place the host is asked); a failed or empty page never blocks revealing items already held above the window.
  • A page that settles while the chat is hidden under still lands (no unmount-only guard).
  • Retry keeps focus: the marker takes focus while loading, returns it to the button if the retry stalls again, and hands it to the log when the last page lands and the marker goes away.
  • The hook's own captureAnchor/restoreAnchor are covered by a cancel-out test (append + trim of equal height, so nothing resizes); it drifts 63px without them.
  • The hook tests mount a small host component instead of using renderHook (src/test/render-hook.tsx), because renderHook has no wrapper option and the hook has to run inside a <Conversation> next to a mounted ConversationContent. They still go only through the hook's public API.

Verification

  • This commit passes bun run typecheck and bun run lint on its own (the stack is checked commit by commit).
  • At the top of the stack: bun run check, bun run build, bun run dep-guard, bun run contrast-test and bun run test:e2e (Playwright specs and the Storybook axe sweep in light and dark) pass.

Closes #100

useHistoryWindow keeps a window of the latest items, adds earlier ones
as the reader nears the top, freezes while the reader has scrolled away
and trims back once they return to the end. It works with history the
host pages in asynchronously, and the item in view stays put within a
pixel as earlier items load. ConversationEarlier marks the top with a
loading state and a retry when a page brings nothing. The doc comment
records why this windows rather than virtualizes, and its ceiling.

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated multi-agent review: independent review + second-agent validation; only confirmed items listed. The hook itself is well-built and the windowing-ceiling doc is real; one verified race below.

Verified (executed harness running the real hook + real engine)

  • The render-phase trim reads the rAF-throttled context atBottom: an append committing between the reader's scroll-up and the engine's per-frame pass removes the first shown item while the reader is scrolled up — violating the plan's hard constraint "never removes items while the reader is scrolled up" (src/hooks/use-history-window.ts:156-162 vs src/ui/conversation.tsx:389-399; a deterministic ordering harness reproduces it, and a frame-first control run proves nothing is removed when ordering is safe). Fix shape: gate the trim on a fresh position signal — trim in the subscribeScroll listener when snapshot.distanceFromEnd <= threshold, or read the last snapshot's distance from a ref in the render-phase trim. The suggested regression test currently fails on this code.

Minor

  • e2e/history-window.spec.ts:3-51 duplicates e2e/conversation.spec.ts:3-44's harness helpers line-for-line — extract a shared e2e/ helper before the next PR copies it.
  • A sync throw from onLoadEarlier is settled then rethrown out of reachStart(), which runs inside the marker's IntersectionObserver callback → uncaught exception in an observer callback (use-history-window.ts:246-253). Document, or route the rethrow through settled state only.
  • nits: the hook harness differs from the plan's renderHook prescription without a stated deviation (add a PR-body note); conversation-earlier.tsx:92-101 hand-writes the ref-merge idiom.

@pratikbuilds
pratikbuilds force-pushed the conversation/06-history-window branch from 255751e to 4dfa1be Compare October 27, 2026 13:54
@pratikbuilds

pratikbuilds commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. Here's what happened to each point.

Fixed

1. A new message could remove the oldest one while the reader was scrolled up (the verified race)

  • The problem: this was slightly wider than the trim alone. A message arriving in the same frame the reader started scrolling up made the window slide, because its "at the bottom" signal was one frame old.
  • The fix: a new message is now added with nothing removed. A layout effect then trims the window as before, but it first checks whether the view has moved up since the last frame, and if so leaves everything in place.
  • Tests: a unit test, "an item that lands before the frame the reader left in removes nothing". In a real-browser run, the first message used to change every time and now stays put.

2. The two e2e specs had duplicate helpers (minor)

  • The fix: they now share e2e/conversation-helpers.ts.

3. A loader that throws crashed inside the observer (minor)

  • The fix: if onLoadEarlier throws, the window stalls the same way it does for a rejected promise, and the error is reported once with reportError.

Not changed, and why

  • The hand-written ref merge: the shared helper is private to another PR's file, so I left this one.
  • Not using renderHook: I'll add a note about this to the PR description.

Commit: fix(conversation): hold the history window until the view is read

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review · Request changes

Adds useHistoryWindow and ConversationEarlier. The window holds its tail while a scroll has not been read yet, except when the trim anchor disappears in the same update that grows the list.

Findings

  • src/hooks/use-history-window.ts:72 — A missing through id slides the window during render, before the scroll pass can freeze it. When start.kind === "latest" and start.through is gone, startIndex returns items.length - windowSize. The layout effect at :204-211 only skips a later setState; it does not undo that fallback, and the next snapshot freezes the slid start. With 300 items, a window of 120 trimmed through r299, and a scroll that has moved but not yet been read, replacing r299 and appending r300 in one update drops r180. The pinned-start path already keeps the first surviving id of the previous window (:78-80). through should do the same when it is missing, instead of jumping to the new tail.

@pratikbuilds
pratikbuilds force-pushed the conversation/06-history-window branch from 4dfa1be to 202d6df Compare October 28, 2026 08:41
The window remembers the last item it was trimmed through. When that
id vanished in the same update as an append (an optimistic id swapped
for the server's, plus the reply), the window slid by one during
render. If the reader had just scrolled up, the next frame froze the
slid start and the window's first item was gone.

A missing trim id now falls back as a missing pin does: to the first
item of the last window that still exists. At the bottom, the next
commit trims back, as it does after a plain append.
@pratikbuilds
pratikbuilds force-pushed the conversation/06-history-window branch from 202d6df to e66abb5 Compare October 28, 2026 11:28
@pratikbuilds

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. It's fixed.

1. A missing through id slid the window during render

  • The fix: when the item the window was trimmed through is gone, the start falls back the same way a missing pinned id does. It goes to the first item of the previous window that still exists, and only goes to the latest windowSize items when none does. At the bottom, the next commit trims back, as it does after a plain append.
  • Tests:
    • "an id swapped at the end as the reader leaves removes nothing" failed before the fix: the window started at r181 instead of r180.
    • "an id swapped at the end at the bottom still slides the window" checks that the bottom case still trims to 120 items.

Commit: fix(conversation): keep the window start when the last id is swapped

Only a scroll the reader made froze the window's start. A move the page
made, such as a glide to an item, focus or find in page, left the start
following the latest items, so the next appended item slid the window
and unmounted the item the view had just been moved to.

The window now freezes on any scroll snapshot that moved the view
towards the top and ended away from the end. Content growing below a
view that stays put leaves scrollTop unchanged, so the window still
slides then.
The window start's fallback, used when the item it is kept by is gone,
read the previous window from a ref during render. The ref was written
by a layout effect, so it went stale while Activity hid the host: a
start grown while hidden whose item was then deleted fell back to the
window from before the host was hidden.

The last slice now lives in the hook's state, recorded during render as
atBottom already is, and the scroll listener reads the window's first
item in the handler rather than in a state updater.
The onLoadEarlier doc said the hook never reports a failure. A loader
that throws synchronously is reported through reportError, so that it
does not escape the marker's observer callback; only a rejection is
left to the host.
ConversationEarlier read ref from its props, which React 19 passes but
React 18 strips, so a ref never reached the marker on React 18, the
lower end of the peer range. It is now wrapped in forwardRef.
@pratikbuilds
pratikbuilds force-pushed the conversation/06-history-window branch from e66abb5 to 78de4e3 Compare October 28, 2026 14:47
@pratikbuilds

Copy link
Copy Markdown
Collaborator Author

Round 3 fixes

A fresh review pass over the stack turned up the points below; each is fixed in a new commit at the end of this branch, and the stack is rebased onto it.

  • 0ef713e: Only the reader's own scroll froze the window, so after a scrollToItem jump, a focus move or find-in-page, the next new item could trim the item the reader jumped to. Any move up away from the end now freezes it; content growing below a still view still slides.
  • e43ba72: startIndex read previousWindow.current during render. The last slice now lives in the hook's state, and the scroll listener no longer reads a ref inside a state updater.
  • 2e3be38: The onLoadEarlier doc now says a synchronous throw is reported through reportError and a rejection is left to the host.
  • 78de4e3: React 18 refs: components that read ref from props only receive it on React 19, while the peer range allows ^18.2.0. They are now wrapped in forwardRef, so a host ref reaches the element on both; the peer range is unchanged. Covers ConversationEarlier.

bun run check and dep-guard pass on every branch of the stack, and the full e2e suite passes at the top of the stack (163/163, including the stories axe sweep). Each behaviour fix comes with a test that fails without it; the forwardRef tests can only guard, since the suite runs on React 19.

@TheGreatAxios
TheGreatAxios removed this pull request from stack #130 October 28, 2026 21:38
@TheGreatAxios
TheGreatAxios added this pull request to stack #139 October 28, 2026 21:38
@TheGreatAxios
TheGreatAxios force-pushed the conversation/06-history-window branch from 78de4e3 to d1672ad Compare October 28, 2026 21:42
@TheGreatAxios
TheGreatAxios dismissed their stale review October 28, 2026 21:49

Superseded by the completed stacked review; blocking fixes were addressed and the rebased stack is being landed.

@corbits-triage

Copy link
Copy Markdown
Contributor

Triage: In progress

Triage is waiting for CI to finish before it decides. This comment shows the verdict once it has.

@TheGreatAxios
TheGreatAxios merged commit 361e789 into main Oct 28, 2026
4 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add useHistoryWindow so long chats load without jumping

2 participants