From 6fc9f195f1357977d6cfd496d126d0957337acf6 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 17 Mar 2026 14:26:53 -0400 Subject: [PATCH] fix: resolve chat message pagination scroll issues (#23169) ## Summary Fixes four interrelated issues that caused scroll position jumps and phantom scroll growth when paginating older chat messages. ## Changes ### 1. Removed client-side message windowing (`useMessageWindow`) There were two competing sentinel systems: server-side pagination and client-side windowing. The client windowing sentinel was nested deep inside the timeline with no explicit IntersectionObserver `root`, causing scroll position jumps when messages were prepended. Blink (coder/blink) has no client-side windowing. Removed it entirely; server pagination + `contentVisibility` handled performance. ### 2. Removed `contentVisibility: "auto"` from message sections Each section had `contentVisibility: "auto"` with `containIntrinsicSize: "1px 600px"`, causing the scroll region to grow/shrink as the browser swapped 600px placeholders for actual heights while scrolling. This created phantom scroll growth with no fetch involved. ### 3. Gated WebSocket on initial REST data The WebSocket `Subscribe` snapshot calls `GetChatMessagesByChatID` (no LIMIT) which returns every message when `afterMessageID` is 0. The WebSocket effect opened before the REST page resolved, so `lastMessageIdRef` was undefined, causing the server to replay the entire history and defeating pagination. Added `initialDataLoaded` guard so the socket waits for the first REST page. ### 4. Manual scroll position restoration Replaced unreliable CSS scroll anchoring in `flex-col-reverse` with a `ScrollAnchoredContainer` that snapshots `scrollHeight` before fetch and restores `scrollTop` via `useLayoutEffect` after render. Disabled browser scroll anchoring (`overflow-anchor: none`) to prevent conflicts. --- .../AgentsPage/AgentDetail/ChatContext.ts | 10 +- .../ConversationTimeline.stories.tsx | 110 ++++++++++--- .../AgentDetail/ConversationTimeline.tsx | 151 ++++++++---------- .../AgentsPage/AgentDetail/messageParsing.ts | 21 --- .../src/pages/AgentsPage/AgentDetail/types.ts | 5 - .../AgentDetail/useMessageWindow.ts | 55 ------- .../pages/AgentsPage/AgentDetailContent.tsx | 21 +-- site/src/pages/AgentsPage/AgentDetailView.tsx | 94 +++++++---- 8 files changed, 235 insertions(+), 232 deletions(-) delete mode 100644 site/src/pages/AgentsPage/AgentDetail/useMessageWindow.ts diff --git a/site/src/pages/AgentsPage/AgentDetail/ChatContext.ts b/site/src/pages/AgentsPage/AgentDetail/ChatContext.ts index 806f176675..65d89ee185 100644 --- a/site/src/pages/AgentsPage/AgentDetail/ChatContext.ts +++ b/site/src/pages/AgentsPage/AgentDetail/ChatContext.ts @@ -478,6 +478,13 @@ export const useChatStore = ( ? chatMessages[chatMessages.length - 1].id : undefined; + // True once the initial REST page has resolved for the current + // chat. The WebSocket effect gates on this so that + // lastMessageIdRef is populated before the socket opens; + // otherwise the server replays the entire message history as + // its snapshot, defeating pagination. + const initialDataLoaded = chatMessages !== undefined; + const updateSidebarChat = useCallback( (updater: (chat: TypesGen.Chat) => TypesGen.Chat) => { if (!chatID) { @@ -616,7 +623,7 @@ export const useChatStore = ( store.resetTransientState(); activeChatIDRef.current = chatID ?? null; - if (!chatID) { + if (!chatID || !initialDataLoaded) { return; } @@ -849,6 +856,7 @@ export const useChatStore = ( cancelScheduledStreamReset, chatID, clearChatErrorReason, + initialDataLoaded, scheduleStreamReset, setChatErrorReason, store, diff --git a/site/src/pages/AgentsPage/AgentDetail/ConversationTimeline.stories.tsx b/site/src/pages/AgentsPage/AgentDetail/ConversationTimeline.stories.tsx index f7b2f420eb..0aa4a2bcb9 100644 --- a/site/src/pages/AgentsPage/AgentDetail/ConversationTimeline.stories.tsx +++ b/site/src/pages/AgentsPage/AgentDetail/ConversationTimeline.stories.tsx @@ -1,19 +1,15 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import type * as TypesGen from "api/typesGenerated"; -import { createRef } from "react"; import { expect, fn, userEvent, within } from "storybook/test"; import { ConversationTimeline } from "./ConversationTimeline"; -import { - buildParsedMessageSections, - parseMessagesWithMergedTools, -} from "./messageParsing"; +import { parseMessagesWithMergedTools } from "./messageParsing"; // 1×1 solid coral (#FF6B6B) PNG encoded as base64. const TEST_PNG_B64 = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4n539HwAHFwLVF8kc1wAAAABJRU5ErkJggg=="; -const buildSections = (messages: TypesGen.ChatMessage[]) => - buildParsedMessageSections(parseMessagesWithMergedTools(messages)); +const buildMessages = (messages: TypesGen.ChatMessage[]) => + parseMessagesWithMergedTools(messages); const baseMessage = { chat_id: "story-chat", @@ -22,11 +18,9 @@ const baseMessage = { const defaultArgs: Omit< React.ComponentProps, - "parsedSections" + "parsedMessages" > = { isEmpty: false, - hasMoreMessages: false, - loadMoreSentinelRef: createRef(), hasStreamOutput: false, streamState: null, streamTools: [], @@ -53,7 +47,7 @@ type Story = StoryObj; export const UserMessageWithSingleImage: Story = { args: { ...defaultArgs, - parsedSections: buildSections([ + parsedMessages: buildMessages([ { ...baseMessage, id: 1, @@ -91,7 +85,7 @@ export const UserMessageWithSingleImage: Story = { export const UserMessageWithMultipleImages: Story = { args: { ...defaultArgs, - parsedSections: buildSections([ + parsedMessages: buildMessages([ { ...baseMessage, id: 1, @@ -128,7 +122,7 @@ export const UserMessageWithMultipleImages: Story = { export const UserMessageWithFileIdImage: Story = { args: { ...defaultArgs, - parsedSections: buildSections([ + parsedMessages: buildMessages([ { ...baseMessage, id: 1, @@ -160,7 +154,7 @@ export const UserMessageWithFileIdImage: Story = { export const UserMessageTextOnly: Story = { args: { ...defaultArgs, - parsedSections: buildSections([ + parsedMessages: buildMessages([ { ...baseMessage, id: 1, @@ -181,7 +175,7 @@ export const UserMessageTextOnly: Story = { export const AssistantMessageWithImage: Story = { args: { ...defaultArgs, - parsedSections: buildSections([ + parsedMessages: buildMessages([ { ...baseMessage, id: 1, @@ -214,7 +208,7 @@ export const AssistantMessageWithImage: Story = { export const UserMessageWithImagesAndFileRefs: Story = { args: { ...defaultArgs, - parsedSections: buildSections([ + parsedMessages: buildMessages([ { ...baseMessage, id: 1, @@ -249,8 +243,7 @@ export const UserMessageWithImagesAndFileRefs: Story = { export const UsageLimitExceeded: Story = { args: { ...defaultArgs, - loadMoreSentinelRef: { current: null }, - parsedSections: [], + parsedMessages: [], detailError: { kind: "usage-limit", message: @@ -274,8 +267,7 @@ export const UsageLimitExceeded: Story = { export const GenericErrorDoesNotShowUsageAction: Story = { args: { ...defaultArgs, - loadMoreSentinelRef: { current: null }, - parsedSections: [], + parsedMessages: [], detailError: { kind: "generic", message: "Provider request failed." }, onOpenAnalytics: fn(), subagentTitles: new Map(), @@ -294,7 +286,7 @@ export const GenericErrorDoesNotShowUsageAction: Story = { export const UserMessageWithInlineFileRef: Story = { args: { ...defaultArgs, - parsedSections: buildSections([ + parsedMessages: buildMessages([ { ...baseMessage, id: 1, @@ -336,7 +328,7 @@ export const UserMessageWithInlineFileRef: Story = { export const UserMessageWithMultipleInlineFileRefs: Story = { args: { ...defaultArgs, - parsedSections: buildSections([ + parsedMessages: buildMessages([ { ...baseMessage, id: 1, @@ -368,3 +360,77 @@ export const UserMessageWithMultipleInlineFileRefs: Story = { expect(canvas.getByText(/handler_test\.go/)).toBeInTheDocument(); }, }; + +/** + * Verifies the structural requirements for sticky user messages + * in the flat (section-less) message list: + * - Each user message renders a data-user-sentinel marker so + * the push-up logic can find the next user message via DOM + * traversal. + * - The user message container gets position:sticky. + * - Sentinels appear in the correct order (matching user + * message order). + */ +export const StickyUserMessageStructure: Story = { + args: { + ...defaultArgs, + parsedMessages: buildMessages([ + { + ...baseMessage, + id: 1, + role: "user", + content: [{ type: "text", text: "First prompt" }], + }, + { + ...baseMessage, + id: 2, + role: "assistant", + content: [{ type: "text", text: "First response" }], + }, + { + ...baseMessage, + id: 3, + role: "user", + content: [{ type: "text", text: "Second prompt" }], + }, + { + ...baseMessage, + id: 4, + role: "assistant", + content: [{ type: "text", text: "Second response" }], + }, + ]), + }, + play: async ({ canvasElement }) => { + // Each user message should produce a data-user-sentinel + // marker that the push-up scroll logic relies on. + const sentinels = canvasElement.querySelectorAll("[data-user-sentinel]"); + expect(sentinels.length).toBe(2); + + // Each sentinel should be immediately followed by a sticky + // container (the user message itself). + for (const sentinel of sentinels) { + const container = sentinel.nextElementSibling; + expect(container).not.toBeNull(); + const style = window.getComputedStyle(container!); + expect(style.position).toBe("sticky"); + } + + // Sentinels must appear in DOM order matching the message + // order so nextElementSibling traversal finds the correct + // next user message. + const allElements = Array.from( + canvasElement.querySelectorAll("[data-user-sentinel], [class*='sticky']"), + ); + const sentinelIndices = Array.from(sentinels).map((s) => + allElements.indexOf(s), + ); + // Sentinels should be in ascending DOM order. + expect(sentinelIndices[0]).toBeLessThan(sentinelIndices[1]); + + // Both user messages should be visible. + const canvas = within(canvasElement); + expect(canvas.getByText("First prompt")).toBeVisible(); + expect(canvas.getByText("Second prompt")).toBeVisible(); + }, +}; diff --git a/site/src/pages/AgentsPage/AgentDetail/ConversationTimeline.tsx b/site/src/pages/AgentsPage/AgentDetail/ConversationTimeline.tsx index 311c6b2dc4..6e99a0211f 100644 --- a/site/src/pages/AgentsPage/AgentDetail/ConversationTimeline.tsx +++ b/site/src/pages/AgentsPage/AgentDetail/ConversationTimeline.tsx @@ -23,7 +23,6 @@ import { Fragment, memo, type ReactNode, - type RefObject, useLayoutEffect, useRef, useState, @@ -37,7 +36,7 @@ import { useSmoothStreamingText } from "./SmoothText"; import type { MergedTool, ParsedMessageContent, - ParsedMessageSection, + ParsedMessageEntry, RenderBlock, StreamState, } from "./types"; @@ -695,9 +694,9 @@ const StickyUserMessage: FC<{ if (tooTall) { container.style.setProperty("--clip-h", `${fullHeight}px`); container.style.setProperty("--fade-opacity", "0"); + container.style.top = "0px"; return; } - const sentinelTop = sentinel.getBoundingClientRect().top; const scrolledPast = scrollerTop - sentinelTop; @@ -706,10 +705,10 @@ const StickyUserMessage: FC<{ // correct height immediately when isStuck flips. container.style.setProperty("--clip-h", `${fullHeight}px`); container.style.setProperty("--fade-opacity", "0"); + container.style.top = "0px"; return; } - - const visible = Math.max(fullHeight - scrolledPast, MIN_HEIGHT); + const visible = Math.max(fullHeight - scrolledPast - 48, MIN_HEIGHT); container.style.setProperty("--clip-h", `${visible}px`); // Only show the fade gradient once enough content is // clipped to be visually meaningful. @@ -717,6 +716,24 @@ const StickyUserMessage: FC<{ "--fade-opacity", visible < fullHeight - 8 ? "1" : "0", ); + + // Push-up effect: when the next user message's sentinel + // approaches the bottom of this sticky container, shift + // this container upward so it slides out of view — the + // same visual as the old section-boundary behavior. + let nextSentinel: Element | null = sentinel.nextElementSibling; + while (nextSentinel) { + if (nextSentinel.hasAttribute("data-user-sentinel")) { + break; + } + nextSentinel = nextSentinel.nextElementSibling; + } + if (nextSentinel) { + const nextY = nextSentinel.getBoundingClientRect().top - scrollerTop; + container.style.top = `${Math.min(0, nextY - visible)}px`; + } else { + container.style.top = "0px"; + } }; updateFnRef.current = update; @@ -786,12 +803,12 @@ const StickyUserMessage: FC<{ return ( <> -
+
; - parsedSections: readonly ParsedMessageSection[]; + parsedMessages: readonly ParsedMessageEntry[]; hasStreamOutput: boolean; streamState: StreamState | null; streamTools: readonly MergedTool[]; @@ -895,9 +910,7 @@ interface ConversationTimelineProps { export const ConversationTimeline: FC = ({ isEmpty, - hasMoreMessages, - loadMoreSentinelRef, - parsedSections, + parsedMessages, hasStreamOutput, streamState, streamTools, @@ -912,8 +925,8 @@ export const ConversationTimeline: FC = ({ savingMessageId, urlTransform, }) => { - const shouldRenderStreamInLastSection = - hasStreamOutput && parsedSections.length > 0; + const shouldRenderStreamAfterMessages = + hasStreamOutput && parsedMessages.length > 0; const isUsageLimitError = detailError?.kind === "usage-limit"; const showUsageAction = onOpenAnalytics !== undefined && isUsageLimitError; @@ -922,15 +935,13 @@ export const ConversationTimeline: FC = ({ const afterEditingMessageIds = new Set(); if (editingMessageId != null) { let found = false; - for (const section of parsedSections) { - for (const entry of section.entries) { - if (entry.message.id === editingMessageId) { - found = true; - continue; - } - if (found) { - afterEditingMessageIds.add(entry.message.id); - } + for (const entry of parsedMessages) { + if (entry.message.id === editingMessageId) { + found = true; + continue; + } + if (found) { + afterEditingMessageIds.add(entry.message.id); } } } @@ -943,66 +954,40 @@ export const ConversationTimeline: FC = ({
) : (
- {hasMoreMessages && ( -
- Loading earlier messages… -
+ {parsedMessages.map(({ message, parsed }) => + message.role === "user" ? ( + + ) : ( + + ), )} - {parsedSections.map((section, sectionIdx) => ( -
-
- {section.entries.map(({ message, parsed }) => - message.role === "user" ? ( - - ) : ( - - ), - )}{" "} - {shouldRenderStreamInLastSection && - sectionIdx === parsedSections.length - 1 && ( - - )} -
-
- ))} - {hasStreamOutput && parsedSections.length === 0 && ( + {shouldRenderStreamAfterMessages && ( + + )} + {hasStreamOutput && parsedMessages.length === 0 && ( { - const sections: ParsedMessageSection[] = []; - - for (const entry of parsedMessages) { - if (entry.message.role === "user") { - sections.push({ userEntry: entry, entries: [entry] }); - continue; - } - if (sections.length === 0) { - sections.push({ userEntry: null, entries: [entry] }); - continue; - } - sections[sections.length - 1].entries.push(entry); - } - - return sections; -}; diff --git a/site/src/pages/AgentsPage/AgentDetail/types.ts b/site/src/pages/AgentsPage/AgentDetail/types.ts index 8a1c5aee68..f8acfa1243 100644 --- a/site/src/pages/AgentsPage/AgentDetail/types.ts +++ b/site/src/pages/AgentsPage/AgentDetail/types.ts @@ -70,11 +70,6 @@ export type ParsedMessageEntry = { parsed: ParsedMessageContent; }; -export type ParsedMessageSection = { - userEntry: ParsedMessageEntry | null; - entries: ParsedMessageEntry[]; -}; - type StreamToolCall = { id: string; name: string; diff --git a/site/src/pages/AgentsPage/AgentDetail/useMessageWindow.ts b/site/src/pages/AgentsPage/AgentDetail/useMessageWindow.ts deleted file mode 100644 index 14d26fa2f0..0000000000 --- a/site/src/pages/AgentsPage/AgentDetail/useMessageWindow.ts +++ /dev/null @@ -1,55 +0,0 @@ -import type * as TypesGen from "api/typesGenerated"; -import { useEffect, useMemo, useRef, useState } from "react"; - -const DEFAULT_PAGE_SIZE = 50; - -type UseMessageWindowOptions = { - messages: readonly TypesGen.ChatMessage[]; - resetKey?: string; - pageSize?: number; -}; - -export const useMessageWindow = ({ - messages, - resetKey, - pageSize = DEFAULT_PAGE_SIZE, -}: UseMessageWindowOptions) => { - const [renderedMessageCount, setRenderedMessageCount] = useState(pageSize); - const loadMoreSentinelRef = useRef(null); - - useEffect(() => { - void resetKey; - setRenderedMessageCount(pageSize); - }, [resetKey, pageSize]); - - const hasMoreMessages = renderedMessageCount < messages.length; - const windowedMessages = useMemo(() => { - if (renderedMessageCount >= messages.length) { - return messages; - } - return messages.slice(messages.length - renderedMessageCount); - }, [messages, renderedMessageCount]); - - useEffect(() => { - const node = loadMoreSentinelRef.current; - if (!node || !hasMoreMessages) { - return; - } - const observer = new IntersectionObserver( - (entries) => { - if (entries[0]?.isIntersecting) { - setRenderedMessageCount((prev) => prev + pageSize); - } - }, - { rootMargin: "200px" }, - ); - observer.observe(node); - return () => observer.disconnect(); - }, [hasMoreMessages, pageSize]); - - return { - hasMoreMessages, - windowedMessages, - loadMoreSentinelRef, - }; -}; diff --git a/site/src/pages/AgentsPage/AgentDetailContent.tsx b/site/src/pages/AgentsPage/AgentDetailContent.tsx index b80a7fb7d7..8fa8c35336 100644 --- a/site/src/pages/AgentsPage/AgentDetailContent.tsx +++ b/site/src/pages/AgentsPage/AgentDetailContent.tsx @@ -25,12 +25,10 @@ import { import { ConversationTimeline } from "./AgentDetail/ConversationTimeline"; import { getLatestContextUsage } from "./AgentDetail/chatHelpers"; import { - buildParsedMessageSections, buildSubagentTitles, parseMessagesWithMergedTools, } from "./AgentDetail/messageParsing"; import { buildStreamTools } from "./AgentDetail/streamState"; -import { useMessageWindow } from "./AgentDetail/useMessageWindow"; import type { ChatDetailError } from "./usageLimitMessage"; import { useFileAttachments } from "./useFileAttachments"; @@ -42,7 +40,6 @@ const isChatMessage = ( interface AgentDetailTimelineProps { store: ChatStoreHandle; - chatID: string; persistedErrorReason: ChatDetailError | undefined; onOpenAnalytics?: () => void; onEditUserMessage?: ( @@ -57,7 +54,6 @@ interface AgentDetailTimelineProps { export const AgentDetailTimeline: FC = ({ store, - chatID, persistedErrorReason, onOpenAnalytics, onEditUserMessage, @@ -87,23 +83,14 @@ export const AgentDetailTimeline: FC = ({ () => buildStreamTools(streamState), [streamState], ); - const { hasMoreMessages, windowedMessages, loadMoreSentinelRef } = - useMessageWindow({ - messages, - resetKey: chatID, - }); const parsedMessages = useMemo( - () => parseMessagesWithMergedTools(windowedMessages), - [windowedMessages], + () => parseMessagesWithMergedTools(messages), + [messages], ); const subagentTitles = useMemo( () => buildSubagentTitles(parsedMessages), [parsedMessages], ); - const parsedSections = useMemo( - () => buildParsedMessageSections(parsedMessages), - [parsedMessages], - ); const detailError: ChatDetailError | undefined = (persistedErrorReason?.kind === "usage-limit" || chatStatus === "error" ? persistedErrorReason @@ -123,9 +110,7 @@ export const AgentDetailTimeline: FC = ({ return ( = ({ }} />
-
= ({ urlTransform={urlTransform} />
- {hasMoreMessages && ( - - )} -
+
= ({ }; /** - * Invisible sentinel that triggers loading older messages when it - * scrolls into view. Placed at the visual top of the flex-col-reverse - * container (which is the DOM bottom). + * Scroll container that uses flex-col-reverse for bottom-anchored chat + * layout. Handles loading older message pages via an IntersectionObserver + * sentinel and manually restores scroll position after new content + * renders — CSS scroll anchoring is unreliable in flex-col-reverse + * containers. */ -const MessagesPaginationSentinel: FC<{ - containerRef: RefObject; - isFetching: boolean; - onLoadMore: () => void; -}> = ({ containerRef, isFetching, onLoadMore }) => { +const ScrollAnchoredContainer: FC<{ + scrollContainerRef: RefObject; + isFetchingMoreMessages: boolean; + hasMoreMessages: boolean; + onFetchMoreMessages: () => void; + children: React.ReactNode; +}> = ({ + scrollContainerRef, + isFetchingMoreMessages, + hasMoreMessages, + onFetchMoreMessages, + children, +}) => { const sentinelRef = useRef(null); + const observerRef = useRef(null); + const isFetchingRef = useRef(isFetchingMoreMessages); + isFetchingRef.current = isFetchingMoreMessages; + const onFetchRef = useRef(onFetchMoreMessages); + onFetchRef.current = onFetchMoreMessages; + // Sentinel observer — triggers loading older messages. + // All changing values are read from refs so the observer + // is created once and never torn down / recreated, which + // would cause spurious intersection callbacks. useEffect(() => { const sentinel = sentinelRef.current; - const container = containerRef.current; + const container = scrollContainerRef.current; if (!sentinel || !container) return; const observer = new IntersectionObserver( ([entry]) => { - if (entry.isIntersecting && !isFetching) { - onLoadMore(); + if (entry.isIntersecting && !isFetchingRef.current) { + onFetchRef.current(); } }, { root: container, - rootMargin: "200px 0px 0px 0px", + rootMargin: "600px 0px 0px 0px", threshold: 0.01, }, ); + observerRef.current = observer; observer.observe(sentinel); - return () => observer.disconnect(); - }, [containerRef, isFetching, onLoadMore]); + return () => { + observer.disconnect(); + observerRef.current = null; + }; + }, [scrollContainerRef]); - return
; + // When a fetch completes, re-observe the sentinel to force + // the IntersectionObserver to re-evaluate. The observer only + // fires on state *changes* (entering/leaving), so if the + // sentinel stayed visible throughout the fetch it won't fire + // again on its own. + useEffect(() => { + if (isFetchingMoreMessages) return; + const sentinel = sentinelRef.current; + const observer = observerRef.current; + if (!sentinel || !observer) return; + observer.unobserve(sentinel); + observer.observe(sentinel); + }, [isFetchingMoreMessages]); + + return ( +
+ {children} + {hasMoreMessages &&
} +
+ ); };