fix(site/src/api/queries): keep live messages when an older chat page resolves - #29444
Draft
jscottmiller wants to merge 3 commits into
Draft
jscottmiller wants to merge 3 commits into
jscottmiller wants to merge 3 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Assistant messages that stream in over the chat WebSocket while an older-page history request (
GET /api/experimental/chats/{chat}/messages?before_id=...) is in flight disappear from the transcript until the page is reloaded. With a 10s delay on that request, a 24-step turn rendered 20 of 24 step rows onmain; the missing messages were exactly those created between the request starting and its response. Messages streamed during the fetch should merge like any other live message, so the transcript matches a reload once the page lands.The transcript history is a TanStack infinite query and live messages are written into its cache with
setQueryData. TanStack'sinfiniteQueryBehaviorsnapshots the existing pages whenfetchNextPagestarts and writes[...snapshot, newPage]when the request resolves, discarding every cache write made in between.useChatStore's sync effect then sees those IDs missing from the fetched set, classifies them as stale (the edit-truncation path) and removes them from the store as well.Fix
chatMessagesForInfiniteScrollgets astructuralSharingmerge,mergeChatMessagesPages. When a result only appends pages under the cached page params, it keeps the current cached pages and appends the new ones; a cache whose last page already hashas_more: falseis left untouched; every other write shape falls through toreplaceEqualDeepas before.useChatStoreis unchanged.Coverage: a Vitest drives a real
InfiniteQueryObserverover the query with a deferredbefore_idpage and an upsert mid-flight (fails onmain), plus direct tests of the merge branches.Surfaced by the collapse-assistant-steps UAT report (internal repo), issue I1: https://github.com/coder/scott-misc/blob/main/uat/collapse-steps-uat/README.md
Follow-ups (pre-existing, not changed here)
editChatMessage.onMutatecallscancelChatMessages, whose revert restores the pre-fetch query state wholesale, so an edit started during an in-flight older-page fetch can drop WebSocket-upserted messages through a different path (state revert, notsetData). Recovery currently relies on the post-edit refresh.history_resetpath inuseChatStoredoes not cancel an in-flight older-page fetch. Thehas_moreguard makes the resolving page a no-op; cancelling the fetch there would be the tidier fix.Root-cause note
Mechanism
The transcript's message history is a TanStack infinite query (
chatMessagesForInfiniteScroll,site/src/api/queries/chats.ts). Live WebSocket messages are written to the store directly and also into the infinite cache viaupsertChatMessages->queryClient.setQueryData(useChatStore.ts,upsertCacheMessages).TanStack's
infiniteQueryBehavior.onFetchcapturesoldPages = context.state.data.pageswhenfetchNextPagestarts, and when the page request resolves it writes{ pages: [...oldPages, newPage] }as the query data (@tanstack/query-core@5.82.0,src/infiniteQueryBehavior.ts). AnysetQueryDatawrite made while the page request was in flight is therefore overwritten: page 0 reverts to its pre-fetch snapshot.The store then loses the messages too.
useChatStore's sync effect compares the flattened cache messages against the previous sync (lastSyncedMessagesRef). The WS upsert had already been synced intoprevIDs; after the pagination result lands those IDs are absent from the fetched set but present inprevIDs, sohasStaleEntriesis true and the effect callsstore.replaceMessages(chatMessages), which removes them from the store. This path exists to handle edit truncation.Trigger conditions
has_moretrue), so the client issuesGET .../messages?before_id=...(fetchNextPage).messagestream event arrives between the start of that request and its response.Backend ruled out
before_idpages only contain ids below the cursor, so the new messages could never appear in that response. The WS delivered them (the UAT saw them render and then vanish), and a reload shows them, so they are persisted. Nothing server-side needs to change.Confirmation
infiniteQueryBehavior.ts(oldPages captured at fetch start) anduseChatStore.ts(stale-entry replace).site/src/api/queries/chats.test.ts: seed a two-page cache, startfetchNextPageon anInfiniteQueryObserverwith a deferredgetChatMessages, callupsertChatMessageswhile pending, resolve; on unfixed code page 0 no longer contains the upserted message.Generated by Coder Agents on behalf of @jscottmiller.