feat: paginate chat messages endpoint with cursor-based infinite scroll#23083
feat: paginate chat messages endpoint with cursor-based infinite scroll#23083
Conversation
kylecarbs
force-pushed
the
kylecarbs/paginate-chat-messages
branch
from
09bfacd to
0b9192f
Compare
…roll Adds cursor-based pagination to the chat messages endpoint using before_id/limit query params. The API returns messages in DESC order (newest first) with a has_more boolean for page detection via the LIMIT N+1 pattern. Frontend switches from useQuery to useInfiniteQuery with an IntersectionObserver sentinel at the visual top of the flex-col-reverse scroll container. When the user scrolls up near the oldest loaded message, the next page of older messages is fetched and prepended. The flex-col-reverse CSS handles scroll anchoring natively — no manual scrollTop adjustment needed. Key changes: - New GetChatMessagesByChatIDPaginated SQL query with keyset cursor - Handler parses before_id/limit params, fetches limit+1 for has_more - Queued messages only returned on first page (no cursor) - SDK client accepts ChatMessagesPaginationOptions - Frontend uses chatMessagesInfinite with useInfiniteQuery - MessagesPaginationSentinel component with IntersectionObserver - Pages flattened and sorted by ID for chronological display
- Fix dbauthz panic: implement auth check for GetChatMessagesByChatIDPaginated matching the pattern from GetChatMessagesByChatID (fetch parent chat to verify access). - Fix ChatContext setQueryData crash: updateChatQueuedMessages now writes to InfiniteData<ChatMessagesResponse> shape instead of plain ChatMessagesResponse, updating pages[0].queued_messages. - Fix replaceMessages dropping WebSocket messages: switch from store.replaceMessages() to per-message upsertDurableMessage() so REST page loads merge into the store without discarding messages delivered by the WebSocket that aren't in REST yet. - Fix unstable onFetchMoreMessages: pass fetchNextPage directly instead of an inline arrow to avoid rebuilding the IntersectionObserver every render. - Remove dead chatMessages export superseded by chatMessagesInfinite.
- Add #nosec G115 comment for int->int32 conversion (limit is validated to 1-200, so limit+1=201 fits in int32) - Add dbauthz test for GetChatMessagesByChatIDPaginated matching the existing GetChatMessagesByChatID test pattern - Fix ChatContext.test.tsx: seed query cache with InfiniteData shape and read back from pages[0] to match the new useInfiniteQuery cache structure
The chatMessages query is now useInfiniteQuery, so the cache
must be seeded with { pages: [...], pageParams: [...] } instead
of a plain ChatMessagesResponse.
kylecarbs
force-pushed
the
kylecarbs/paginate-chat-messages
branch
from
ee7a5c3 to
f899eb2
Compare
| ORDER BY | ||
| id DESC | ||
| LIMIT | ||
| COALESCE(NULLIF(@limit_val::int, 0), 50); |
There was a problem hiding this comment.
I would consider updating or replacing the previous GetChatMessagesByChatID, or do we have a use-case for keeping it around?
There was a problem hiding this comment.
The old GetChatMessagesByChatID is still used in three internal server-side callers (chatd.go x2, subagent.go) that use the AfterID parameter to fetch messages after a known ID for streaming replay and subagent context. Different use case from the paginated endpoint — kept it.
| WHERE | ||
| chat_id = @chat_id::uuid | ||
| AND CASE | ||
| WHEN @before_id::bigint > 0 THEN id < @before_id::bigint |
There was a problem hiding this comment.
Considering this is non-standard reverse pagination, I wonder if we should make this obvious in the name?
There was a problem hiding this comment.
Good point. Open to suggestions — maybe GetChatMessagesByChatIDDescPaginated or GetChatMessagesByChatIDReversePaginated? The current name at least differentiates it from the non-paginated version.
There was a problem hiding this comment.
I think Desc in the name makes sense, either as proposed or suffix 👍🏻
There was a problem hiding this comment.
Renamed to GetChatMessagesByChatIDDescPaginated.
kylecarbs
changed the title
- Use httpapi.NewQueryParamParser() for before_id/limit parsing instead of manual strconv, matching the ParsePagination pattern - Rename chatMessagesInfinite -> chatMessagesForInfiniteScroll - Use flatMap instead of for-loop for page flattening - Use .at(-1) instead of bracket indexing for last page
Documentation CheckUpdates Needed
Automated review via Coder Tasks |
mafredri
left a comment
mafredri
left a comment
There was a problem hiding this comment.
BE looks good. FE looks good to me, but I'm not @DanielleMaywood.
|
With regards to frontend changes, I have nothing blocking. I think our infinite query cache logic is getting quite gnarly though so I might take a look in the future into how we can simplify this. |
kylecarbs
changed the title
Makes the reverse sort order obvious in the query name per review feedback.
kylecarbs
force-pushed
the
kylecarbs/paginate-chat-messages
branch
from
63d53cc to
5ecb36f
Compare
3 participants
Adds cursor-based pagination to the chat messages endpoint.
Backend
GetChatMessagesByChatIDPaginatedSQL query: returns messages inid DESCorder with abefore_idkeyset cursor and configurablelimit?before_id=N&limit=Nquery params, uses theLIMIT N+1trick to sethas_morewithout a separate COUNT queryChatMessagesPaginationOptionsFrontend
getChatMessagesfromuseQuerytouseInfiniteQuerywith cursor chaining viagetNextPageParamidascending for chronological displayMessagesPaginationSentinelcomponent usesIntersectionObserver(200px rootMargin prefetch) inside the existingflex-col-reversescroll containerflex-col-reversehandles scroll anchoring natively when older messages are prepended — no manualscrollTopadjustment needed (same pattern as coder/blink)Why cursor-based instead of offset/limit
Offset-based pagination breaks when new messages arrive while paginating backward (offsets shift, causing duplicates or missed messages). The
before_idcursor is stable regardless of inserts — each page is deterministic.