diff --git a/site/src/pages/AgentsPage/components/ChatVirtualList/ChatVirtualList.stories.tsx b/site/src/pages/AgentsPage/components/ChatVirtualList/ChatVirtualList.stories.tsx index ed4e6e0d07..4d72df06ab 100644 --- a/site/src/pages/AgentsPage/components/ChatVirtualList/ChatVirtualList.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatVirtualList/ChatVirtualList.stories.tsx @@ -1,37 +1,75 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { type FC, useRef, useState } from "react"; import { expect, userEvent, waitFor, within } from "storybook/test"; -import { ChatVirtualList } from "./ChatVirtualList"; +import { ChatVirtualList, type VirtualItem } from "./ChatVirtualList"; +import type { MessageKind } from "./heightCache"; -type Item = { id: number; height: number }; +type DemoItem = { id: string; kind: MessageKind; height: number }; -const StoryHarness: FC<{ hasMoreMessages?: boolean }> = ({ - hasMoreMessages = false, -}) => { +const makeItems = (count: number, height = 220): DemoItem[] => + Array.from({ length: count }, (_, i) => ({ + id: `m-${i}`, + kind: "assistant" as MessageKind, + height, + })); + +const StoryHarness: FC<{ + initialCount?: number; + hasMoreMessages?: boolean; + itemHeight?: number; +}> = ({ initialCount = 40, hasMoreMessages = false, itemHeight = 220 }) => { const scrollContainerRef = useRef(null); const scrollToBottomRef = useRef<(() => void) | null>(null); - const nextId = useRef(40); - const olderId = useRef(-1); - const [items, setItems] = useState(() => - Array.from({ length: 40 }, (_, i) => ({ id: i, height: 120 })), + const [items, setItems] = useState(() => + makeItems(initialCount, itemHeight), ); const [fetchCount, setFetchCount] = useState(0); + // Derive new ids purely from the previous list so the updater stays pure. + // React StrictMode double-invokes updaters; a mutable counter would advance + // twice and make the appended/prepended ids unpredictable. const append = () => - setItems((prev) => [...prev, { id: nextId.current++, height: 200 }]); + setItems((prev) => { + const nextIndex = + prev.reduce((max, it) => { + const n = it.id.startsWith("m-") ? Number(it.id.slice(2)) : -1; + return Number.isNaN(n) ? max : Math.max(max, n); + }, -1) + 1; + return [ + ...prev, + { id: `m-${nextIndex}`, kind: "assistant", height: itemHeight }, + ]; + }); const prepend = () => - setItems((prev) => [ - ...Array.from({ length: 5 }, () => ({ - id: olderId.current--, + setItems((prev) => { + const minOld = prev.reduce((min, it) => { + const n = it.id.startsWith("old-") ? Number(it.id.slice(4)) : 0; + return Number.isNaN(n) ? min : Math.min(min, n); + }, 0); + const older = Array.from({ length: 5 }, (_, i) => ({ + id: `old-${minOld - 1 - i}`, + kind: "user" as MessageKind, height: 120, - })), - ...prev, - ]); + })); + return [...older, ...prev]; + }); const growFive = () => setItems((prev) => - prev.map((it) => (it.id === 5 ? { ...it, height: 320 } : it)), + prev.map((it) => + it.id === "m-5" ? { ...it, height: it.height + 160 } : it, + ), ); + const byId = new Map(items.map((it) => [it.id, it])); + const renderItem = (item: VirtualItem) => { + const demo = byId.get(item.id); + return ( +
+ {item.id} +
+ ); + }; + return (
@@ -47,23 +85,14 @@ const StoryHarness: FC<{ hasMoreMessages?: boolean }> = ({ {fetchCount}
setFetchCount((count) => count + 1)} - messageCount={items.length} - > - {items.map((it) => ( -
- message {it.id} -
- ))} - + />
); }; @@ -76,45 +105,80 @@ export default meta; type Story = StoryObj; -const offsetFromScrollerTop = ( - scroller: HTMLElement, - el: HTMLElement, -): number => - el.getBoundingClientRect().top - scroller.getBoundingClientRect().top; - const distanceFromBottom = (scroller: HTMLElement): number => scroller.scrollHeight - scroller.scrollTop - scroller.clientHeight; +// visibleAnchorId returns the id of the first rendered item intersecting the +// viewport top, picked dynamically so assertions do not depend on which items +// the window chose to render. +const visibleAnchorId = ( + canvasElement: HTMLElement, + scroller: HTMLElement, +): string => { + const top = scroller.getBoundingClientRect().top; + for (const el of canvasElement.querySelectorAll("[data-chat-item-id]")) { + if (el.getBoundingClientRect().bottom > top + 5) { + const id = el.getAttribute("data-chat-item-id"); + if (id) { + return id; + } + } + } + throw new Error("no visible item"); +}; + +const offsetOfId = ( + canvasElement: HTMLElement, + scroller: HTMLElement, + id: string, +): number => { + const el = canvasElement.querySelector(`[data-chat-item-id="${id}"]`); + if (!el) { + return Number.NaN; + } + return el.getBoundingClientRect().top - scroller.getBoundingClientRect().top; +}; + +// settleWindow waits until the window has re-rendered around the current scroll +// position, i.e. a real item sits at the viewport top. The window recompute is +// async, so picking a reference item before settling races against it and may +// observe a stale window from the previous scroll position. +const settleWindow = async ( + canvasElement: HTMLElement, + scroller: HTMLElement, +): Promise => { + await waitFor(() => { + const id = visibleAnchorId(canvasElement, scroller); + expect(Math.abs(offsetOfId(canvasElement, scroller, id))).toBeLessThan(400); + }); +}; + export const RendersAndOverflows: Story = { render: () => , play: async ({ canvasElement }) => { const canvas = within(canvasElement); const scroller = canvas.getByTestId("scroll-container"); - await expect(scroller.scrollHeight).toBeGreaterThan(scroller.clientHeight); + await waitFor(() => + expect(scroller.scrollHeight).toBeGreaterThan(scroller.clientHeight), + ); }, }; -export const AnchorStaysStableOnAboveViewportGrowth: Story = { - render: () => , +export const RendersOnlyWindow: Story = { + render: () => , play: async ({ canvasElement }) => { const canvas = within(canvasElement); const scroller = canvas.getByTestId("scroll-container"); - scroller.scrollTop = 1500; - scroller.dispatchEvent(new Event("scroll")); - await waitFor(() => expect(scroller.scrollTop).toBe(1500)); - const before = offsetFromScrollerTop( - scroller, - canvas.getByTestId("msg-15"), - ); - await userEvent.click(canvas.getByTestId("grow-5")); await waitFor(() => - expect( - Math.abs( - offsetFromScrollerTop(scroller, canvas.getByTestId("msg-15")) - - before, - ), - ).toBeLessThanOrEqual(2), + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), ); + // Only the near-viewport window is rendered, not all 200 items, and a + // far-above item is not mounted once the window has converged. + await waitFor(() => expect(canvas.queryByTestId("m-2")).toBeNull()); + const rendered = canvasElement.querySelectorAll("[data-chat-item]"); + await expect(rendered.length).toBeLessThanOrEqual(40); + const topSpacer = canvasElement.querySelector("[data-virtual-spacer]"); + await expect((topSpacer as HTMLElement).style.height).not.toBe("0px"); }, }; @@ -138,6 +202,9 @@ export const DoesNotYankWhenScrolledUp: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); const scroller = canvas.getByTestId("scroll-container"); + await waitFor(() => + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), + ); scroller.scrollTop = 800; scroller.dispatchEvent(new Event("scroll")); await waitFor(() => expect(scroller.scrollTop).toBe(800)); @@ -153,6 +220,9 @@ export const FetchesOlderNearTop: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); const scroller = canvas.getByTestId("scroll-container"); + await waitFor(() => + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), + ); scroller.scrollTop = 0; scroller.dispatchEvent(new Event("scroll")); await waitFor(() => @@ -163,35 +233,14 @@ export const FetchesOlderNearTop: Story = { }, }; -export const PrependKeepsViewportStable: Story = { - render: () => , - play: async ({ canvasElement }) => { - const canvas = within(canvasElement); - const scroller = canvas.getByTestId("scroll-container"); - scroller.scrollTop = 1500; - scroller.dispatchEvent(new Event("scroll")); - await waitFor(() => expect(scroller.scrollTop).toBe(1500)); - const before = offsetFromScrollerTop( - scroller, - canvas.getByTestId("msg-20"), - ); - await userEvent.click(canvas.getByTestId("prepend")); - await waitFor(() => - expect( - Math.abs( - offsetFromScrollerTop(scroller, canvas.getByTestId("msg-20")) - - before, - ), - ).toBeLessThanOrEqual(2), - ); - }, -}; - export const ScrollToBottomButtonWorks: Story = { render: () => , play: async ({ canvasElement }) => { const canvas = within(canvasElement); const scroller = canvas.getByTestId("scroll-container"); + await waitFor(() => + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), + ); scroller.scrollTop = 100; scroller.dispatchEvent(new Event("scroll")); const button = await canvas.findByRole("button", { @@ -203,3 +252,138 @@ export const ScrollToBottomButtonWorks: Story = { ); }, }; + +export const NoJumpOnPrepend: Story = { + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const scroller = canvas.getByTestId("scroll-container"); + await waitFor(() => + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), + ); + scroller.scrollTop = 20000; + scroller.dispatchEvent(new Event("scroll")); + await waitFor(() => expect(scroller.scrollTop).toBe(20000)); + await settleWindow(canvasElement, scroller); + const ref = visibleAnchorId(canvasElement, scroller); + const before = offsetOfId(canvasElement, scroller, ref); + await userEvent.click(canvas.getByTestId("prepend")); + await waitFor(() => + expect( + Math.abs(offsetOfId(canvasElement, scroller, ref) - before), + ).toBeLessThanOrEqual(3), + ); + }, +}; + +export const StreamingGrowthAboveViewportNoJump: Story = { + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const scroller = canvas.getByTestId("scroll-container"); + await waitFor(() => + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), + ); + // Put m-5 (offset 1100) into the top overscan, above the viewport top. + scroller.scrollTop = 1800; + scroller.dispatchEvent(new Event("scroll")); + await waitFor(() => expect(scroller.scrollTop).toBe(1800)); + await settleWindow(canvasElement, scroller); + const ref = visibleAnchorId(canvasElement, scroller); + const before = offsetOfId(canvasElement, scroller, ref); + // m-5 grows above the viewport; the visible reference must not move. + await userEvent.click(canvas.getByTestId("grow-5")); + await waitFor(() => + expect( + Math.abs(offsetOfId(canvasElement, scroller, ref) - before), + ).toBeLessThanOrEqual(3), + ); + }, +}; + +export const NoJumpScrollingThroughWrongEstimates: Story = { + // Real height 120 differs sharply from the assistant seed 220, so items + // entering the top of the window reconcile estimate to measured. Scrolling up + // by a delta must move a tracked item by exactly that delta. + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const scroller = canvas.getByTestId("scroll-container"); + await waitFor(() => + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), + ); + scroller.scrollTop = 10000; + scroller.dispatchEvent(new Event("scroll")); + await waitFor(() => expect(scroller.scrollTop).toBe(10000)); + await settleWindow(canvasElement, scroller); + const ref = visibleAnchorId(canvasElement, scroller); + const before = offsetOfId(canvasElement, scroller, ref); + scroller.scrollTop = 9500; + scroller.dispatchEvent(new Event("scroll")); + await waitFor(() => + expect( + Math.abs(offsetOfId(canvasElement, scroller, ref) - (before + 500)), + ).toBeLessThanOrEqual(5), + ); + }, +}; + +export const OnlyWindowMounts: Story = { + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const scroller = canvas.getByTestId("scroll-container"); + await waitFor(() => + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), + ); + // The newest item is mounted at the bottom. + await waitFor(() => expect(canvas.queryByTestId("m-999")).not.toBeNull()); + // DOM node count stays bounded by the window, independent of the 1000-item + // list size: a render-all implementation would mount far more than this. + const rendered = canvasElement.querySelectorAll("[data-chat-item]"); + await expect(rendered.length).toBeLessThanOrEqual(40); + }, +}; + +export const AppendKeepsPinned: Story = { + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const scroller = canvas.getByTestId("scroll-container"); + await waitFor(() => + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), + ); + await userEvent.click(canvas.getByTestId("append")); + // The appended item mounts and the scroller stays pinned to the bottom. + await waitFor(() => expect(canvas.queryByTestId("m-1000")).not.toBeNull()); + await waitFor(() => + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), + ); + }, +}; + +export const ScrollRecyclesOffscreen: Story = { + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const scroller = canvas.getByTestId("scroll-container"); + await waitFor(() => + expect(distanceFromBottom(scroller)).toBeLessThanOrEqual(16), + ); + // Bring m-100 into the window. + scroller.scrollTop = 22000; + scroller.dispatchEvent(new Event("scroll")); + await waitFor(() => expect(canvas.queryByTestId("m-100")).not.toBeNull()); + // Scroll far below m-100 so it falls outside the overscan window. + scroller.scrollTop = 40000; + scroller.dispatchEvent(new Event("scroll")); + // The offscreen row leaves the DOM (true recycling) and the top spacer + // grows to reserve the space of every item now scrolled above the window, + // including m-100's slot at offset 22000. + await waitFor(() => expect(canvas.queryByTestId("m-100")).toBeNull()); + const topSpacer = canvasElement.querySelector("[data-virtual-spacer]"); + await expect( + Number.parseInt((topSpacer as HTMLElement).style.height, 10), + ).toBeGreaterThan(22000); + }, +}; diff --git a/site/src/pages/AgentsPage/components/ChatVirtualList/ChatVirtualList.tsx b/site/src/pages/AgentsPage/components/ChatVirtualList/ChatVirtualList.tsx index 26a615e57e..77581bb88f 100644 --- a/site/src/pages/AgentsPage/components/ChatVirtualList/ChatVirtualList.tsx +++ b/site/src/pages/AgentsPage/components/ChatVirtualList/ChatVirtualList.tsx @@ -6,34 +6,94 @@ import { useEffect, useLayoutEffect, useRef, + useState, } from "react"; +import { + createHeightCache, + type HeightCache, + type MessageKind, +} from "./heightCache"; import { ScrollToBottomButton } from "./ScrollToBottomButton"; import { useScrollAnchor } from "./useScrollAnchor"; +import { computeWindow, cumulativeOffsets } from "./windowMath"; +export type VirtualItem = { id: string; kind: MessageKind }; + +// Render this many pixels above and below the viewport, matching the +// @pierre/diffs Virtualizer overscan so fast scrolls do not blank. +const VIRTUAL_OVERSCAN = 1000; const LOAD_MORE_MARGIN = "600px 0px 0px 0px"; type ChatVirtualListProps = { + items: ReadonlyArray; + renderItem: (item: VirtualItem) => ReactNode; scrollContainerRef: RefObject; scrollToBottomRef: RefObject<(() => void) | null>; isFetchingMoreMessages: boolean; hasMoreMessages: boolean; onFetchMoreMessages: () => void; - messageCount: number; - children: ReactNode; +}; + +// The cached height sizes this item's spacer once it scrolls out of the +// window, so leaving the window is layout-neutral. +const MeasuredItem: FC<{ + item: VirtualItem; + renderItem: (item: VirtualItem) => ReactNode; + cache: HeightCache; + onMeasured: () => void; +}> = ({ item, renderItem, cache, onMeasured }) => { + const ref = useRef(null); + useEffect(() => { + const el = ref.current; + if (!el) { + return; + } + const observer = new ResizeObserver(() => { + const height = el.getBoundingClientRect().height; + if (height !== cache.get(item.id)) { + cache.record(item.id, item.kind, height); + onMeasured(); + } + }); + observer.observe(el); + return () => observer.disconnect(); + }, [item, cache, onMeasured]); + + return ( +
+ {renderItem(item)} +
+ ); }; export const ChatVirtualList: FC = ({ + items, + renderItem, scrollContainerRef, scrollToBottomRef, isFetchingMoreMessages, hasMoreMessages, onFetchMoreMessages, - messageCount, - children, }) => { - const { scrollerRef, contentRef, atBottom, scrollToBottom, maintainPin } = - useScrollAnchor(); + const { + scrollerRef, + contentRef, + atBottom, + scrollToBottom, + captureAnchor, + restoreAnchor, + } = useScrollAnchor(); const topSentinelRef = useRef(null); + // Lazy useState initializer keeps one cache instance for the component's life + // without reading a ref during render, which the React Compiler forbids. + const [cache] = useState(() => createHeightCache()); + + const [scrollTop, setScrollTop] = useState(0); + const [viewportHeight, setViewportHeight] = useState(0); + const [cacheVersion, setCacheVersion] = useState(0); + const bumpCacheVersion = useCallback(() => { + setCacheVersion((value) => value + 1); + }, []); const setScroller = useCallback( (element: HTMLDivElement | null) => { @@ -44,10 +104,102 @@ export const ChatVirtualList: FC = ({ [scrollerRef, scrollContainerRef, scrollToBottomRef, scrollToBottom], ); - // biome-ignore lint/correctness/useExhaustiveDependencies(messageCount): messageCount is an intentional trigger so a new message pins to the bottom; maintainPin reads refs and must re-run when the count changes. + const heights = items.map((item) => cache.estimate(item.id, item.kind)); + const offsets = cumulativeOffsets(heights); + const { start, end, topPad, bottomPad } = computeWindow({ + offsets, + scrollTop, + // Until the scroller is measured, fall back to a 1px viewport so the first + // window stays small (overscan only) instead of mounting the whole list. + viewportHeight: viewportHeight || 1, + overscan: VIRTUAL_OVERSCAN, + }); + const visible = end >= start ? items.slice(start, end + 1) : []; + + // Mirror the live scroll position into state so the window recomputes as the + // reader scrolls. The anchor is captured by the layout effect below, after the + // new window commits, never here. A teleport scroll therefore records a real + // rendered item instead of a stale one from the previous window. + useEffect(() => { + const scroller = scrollerRef.current; + if (!scroller) { + return; + } + let frame = 0; + const onScroll = () => { + if (frame) { + return; + } + frame = requestAnimationFrame(() => { + frame = 0; + setScrollTop(scroller.scrollTop); + }); + }; + scroller.addEventListener("scroll", onScroll, { passive: true }); + return () => { + scroller.removeEventListener("scroll", onScroll); + if (frame) { + cancelAnimationFrame(frame); + } + }; + }, [scrollerRef]); + + // A resize counts as a content change below, so the anchor (or bottom pin) + // is preserved across it. + useEffect(() => { + const scroller = scrollerRef.current; + if (!scroller) { + return; + } + const observer = new ResizeObserver(() => { + setViewportHeight(scroller.clientHeight); + }); + observer.observe(scroller); + setViewportHeight(scroller.clientHeight); + return () => observer.disconnect(); + }, [scrollerRef]); + + // Single scrollTop owner. After every commit, classify the change by diffing + // the committed inputs against the previous commit: + // - content changed (items, measured heights, or viewport): the DOM shifted + // under the reader, so restore the captured anchor (or re-pin to the + // bottom), then re-capture at the settled position. + // - only scrollTop changed (a deliberate scroll): just re-capture; restoring + // here would fight the scroll. + // restore writes scroller.scrollTop, which the scroll listener turns into the + // next scrollTop state, so a correction converges across frames. + const prevCommitRef = useRef<{ + scrollTop: number; + items: ReadonlyArray; + cacheVersion: number; + viewportHeight: number; + } | null>(null); useLayoutEffect(() => { - maintainPin(); - }, [messageCount, maintainPin]); + const prev = prevCommitRef.current; + prevCommitRef.current = { scrollTop, items, cacheVersion, viewportHeight }; + if (!prev) { + restoreAnchor(); + captureAnchor(); + return; + } + const contentChanged = + items !== prev.items || + cacheVersion !== prev.cacheVersion || + viewportHeight !== prev.viewportHeight; + if (contentChanged) { + restoreAnchor(); + captureAnchor(); + } else if (scrollTop !== prev.scrollTop) { + captureAnchor(); + } + }, [ + scrollTop, + viewportHeight, + items, + cacheVersion, + restoreAnchor, + captureAnchor, + ]); useEffect(() => { const sentinel = topSentinelRef.current; @@ -85,7 +237,17 @@ export const ChatVirtualList: FC = ({ >
- {children} +
+ {visible.map((item) => ( + + ))} +
{ + it("uses a provided seed when a kind has no samples", () => { + const cache = createHeightCache({ assistant: 200 }); + expect(cache.estimate("a1", "assistant")).toBe(200); + }); + + it("falls back to a built-in seed when none is provided", () => { + const cache = createHeightCache(); + expect(cache.estimate("u1", "user")).toBe(80); + }); + + it("averages recorded heights per kind", () => { + const cache = createHeightCache(); + cache.record("a1", "assistant", 100); + cache.record("a2", "assistant", 300); + expect(cache.estimate("a3", "assistant")).toBe(200); + }); + + it("returns the measured height for a known id via get and estimate", () => { + const cache = createHeightCache(); + cache.record("a1", "assistant", 512); + expect(cache.get("a1")).toBe(512); + expect(cache.estimate("a1", "assistant")).toBe(512); + }); + + it("updates the average when an id is re-recorded, without double counting", () => { + const cache = createHeightCache(); + cache.record("a1", "assistant", 100); + cache.record("a1", "assistant", 200); + expect(cache.estimate("a2", "assistant")).toBe(200); + }); + + it("returns undefined from get for an unknown id", () => { + const cache = createHeightCache(); + expect(cache.get("nope")).toBeUndefined(); + }); + + it("keeps averages per kind independent", () => { + const cache = createHeightCache(); + cache.record("u1", "user", 60); + cache.record("a1", "assistant", 400); + expect(cache.estimate("u2", "user")).toBe(60); + expect(cache.estimate("a2", "assistant")).toBe(400); + }); +}); diff --git a/site/src/pages/AgentsPage/components/ChatVirtualList/heightCache.ts b/site/src/pages/AgentsPage/components/ChatVirtualList/heightCache.ts new file mode 100644 index 0000000000..91d2ef480a --- /dev/null +++ b/site/src/pages/AgentsPage/components/ChatVirtualList/heightCache.ts @@ -0,0 +1,62 @@ +// Type-aware height model for the windowing renderer. Pure mutable state, read +// on render and updated from measurement. No DOM or React here. + +export type MessageKind = "user" | "assistant" | "tool" | "diff" | "other"; + +export type HeightCache = { + // get returns the measured height for an id, or undefined if never measured. + get(id: string): number | undefined; + // estimate returns the measured height if known, else the running average for + // the kind, else the seed. Used to size not-yet-measured items. + estimate(id: string, kind: MessageKind): number; + // record stores an id's measured height and folds it into the kind average. + record(id: string, kind: MessageKind, height: number): void; +}; + +// Seeds only affect the first paint of a kind before any of its messages have +// been measured. They are intentionally rough. +const DEFAULT_SEEDS: Record = { + user: 80, + assistant: 220, + tool: 140, + diff: 320, + other: 160, +}; + +export function createHeightCache( + seeds?: Partial>, +): HeightCache { + const seed: Record = { ...DEFAULT_SEEDS, ...seeds }; + const measured = new Map(); + const totals = new Map(); + + const kindAverage = (kind: MessageKind): number | undefined => { + const total = totals.get(kind); + if (!total || total.count === 0) { + return undefined; + } + return total.sum / total.count; + }; + + return { + get(id) { + return measured.get(id)?.height; + }, + estimate(id, kind) { + return measured.get(id)?.height ?? kindAverage(kind) ?? seed[kind]; + }, + record(id, kind, height) { + const previous = measured.get(id); + const total = totals.get(kind) ?? { sum: 0, count: 0 }; + // Replace the previous sample so re-measuring never double counts. + if (previous && previous.kind === kind) { + total.sum += height - previous.height; + } else { + total.sum += height; + total.count += 1; + } + totals.set(kind, total); + measured.set(id, { kind, height }); + }, + }; +} diff --git a/site/src/pages/AgentsPage/components/ChatVirtualList/useScrollAnchor.stories.tsx b/site/src/pages/AgentsPage/components/ChatVirtualList/useScrollAnchor.stories.tsx new file mode 100644 index 0000000000..43d366186c --- /dev/null +++ b/site/src/pages/AgentsPage/components/ChatVirtualList/useScrollAnchor.stories.tsx @@ -0,0 +1,129 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { type FC, useLayoutEffect, useState } from "react"; +import { expect, userEvent, waitFor, within } from "storybook/test"; +import { useScrollAnchor } from "./useScrollAnchor"; + +// Exercises the capture/restore primitive in isolation, without the windowing +// renderer, so a failure here points at the anchor hook rather than the layout +// math. + +const ROWS = 40; +const ROW_HEIGHT = 100; +const SPACER_HEIGHT = 50; + +const AnchorHarness: FC = () => { + const { scrollerRef, contentRef, captureAnchor, restoreAnchor } = + useScrollAnchor(); + const [heights, setHeights] = useState>({}); + const [version, setVersion] = useState(0); + + const mutate = (changes: Record) => { + // Record the item at the viewport top before the heights change, then + // restore it once the mutation commits. This mirrors how the container + // brackets a content mutation around the layout effect. + captureAnchor(); + setHeights((prev) => ({ ...prev, ...changes })); + setVersion((value) => value + 1); + }; + + // biome-ignore lint/correctness/useExhaustiveDependencies(version): version is the layout-mutation trigger; restore reads the DOM and must run after each mutation commits. + useLayoutEffect(() => { + restoreAnchor(); + }, [version, restoreAnchor]); + + return ( +
+
+ + +
+
+
+
+ {Array.from({ length: ROWS }, (_, i) => ( +
+ row {i} +
+ ))} +
+
+
+ ); +}; + +const meta: Meta = { + title: "pages/AgentsPage/ChatVirtualList/useScrollAnchor", + component: AnchorHarness, +}; +export default meta; + +type Story = StoryObj; + +const offsetFromTop = (scroller: HTMLElement, el: HTMLElement): number => + el.getBoundingClientRect().top - scroller.getBoundingClientRect().top; + +// Scroll so row 20 sits at the viewport top: spacer + 20 rows. +const SCROLL_TO_ROW_20 = SPACER_HEIGHT + 20 * ROW_HEIGHT; + +export const AnchorSurvivesAboveMutation: Story = { + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const scroller = canvas.getByTestId("scroll-container"); + scroller.scrollTop = SCROLL_TO_ROW_20; + scroller.dispatchEvent(new Event("scroll")); + await waitFor(() => expect(scroller.scrollTop).toBe(SCROLL_TO_ROW_20)); + const before = offsetFromTop(scroller, canvas.getByTestId("row-20")); + await userEvent.click(canvas.getByTestId("grow-above")); + await waitFor(() => + expect( + Math.abs( + offsetFromTop(scroller, canvas.getByTestId("row-20")) - before, + ), + ).toBeLessThanOrEqual(2), + ); + }, +}; + +export const AnchorSurvivesZeroNetMutation: Story = { + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const scroller = canvas.getByTestId("scroll-container"); + scroller.scrollTop = SCROLL_TO_ROW_20; + scroller.dispatchEvent(new Event("scroll")); + await waitFor(() => expect(scroller.scrollTop).toBe(SCROLL_TO_ROW_20)); + const before = offsetFromTop(scroller, canvas.getByTestId("row-20")); + // Grows a row above the anchor and shrinks a row below it by the same + // amount: total height is unchanged, so a content ResizeObserver never + // fires, yet the anchor moved. Only capture/restore corrects this. + await userEvent.click(canvas.getByTestId("zero-net")); + await waitFor(() => + expect( + Math.abs( + offsetFromTop(scroller, canvas.getByTestId("row-20")) - before, + ), + ).toBeLessThanOrEqual(2), + ); + }, +}; diff --git a/site/src/pages/AgentsPage/components/ChatVirtualList/useScrollAnchor.ts b/site/src/pages/AgentsPage/components/ChatVirtualList/useScrollAnchor.ts index caeabc2ad7..65af8a19b5 100644 --- a/site/src/pages/AgentsPage/components/ChatVirtualList/useScrollAnchor.ts +++ b/site/src/pages/AgentsPage/components/ChatVirtualList/useScrollAnchor.ts @@ -7,34 +7,65 @@ import { } from "react"; import { correctedScrollTop, isAtBottom } from "./anchorMath"; -type Anchor = { el: HTMLElement; offset: number }; +// The anchor is keyed by a stable item id so it survives windowing remounts. +// The element ref is a fast path; if it has been unmounted we re-find by id. +type Anchor = { id: string | null; el: HTMLElement; offset: number }; -// findTopAnchor returns the first child intersecting the viewport top, together -// with its current offset from the scroller top. That child is what we keep -// fixed when content above it changes height. +const ITEM_SELECTOR = "[data-chat-item]"; + +// findTopAnchor returns the first rendered item intersecting the viewport top. +// Spacers and other non-item children are ignored so the anchor is always a +// real element whose offset we can preserve. function findTopAnchor( scroller: HTMLElement, content: HTMLElement, ): Anchor | null { const scrollerTop = scroller.getBoundingClientRect().top; - for (const child of content.children) { + for (const child of content.querySelectorAll(ITEM_SELECTOR)) { if (!(child instanceof HTMLElement)) { continue; } const rect = child.getBoundingClientRect(); if (rect.bottom > scrollerTop) { - return { el: child, offset: rect.top - scrollerTop }; + return { + id: child.getAttribute("data-chat-item-id"), + el: child, + offset: rect.top - scrollerTop, + }; } } return null; } +function resolveAnchorElement( + content: HTMLElement, + anchor: Anchor, +): HTMLElement | null { + if (anchor.el.isConnected) { + return anchor.el; + } + if (anchor.id == null) { + return null; + } + const found = content.querySelector( + `[data-chat-item-id="${CSS.escape(anchor.id)}"]`, + ); + return found instanceof HTMLElement ? found : null; +} + type ScrollAnchor = { scrollerRef: RefObject; contentRef: RefObject; atBottom: boolean; scrollToBottom: () => void; - maintainPin: () => void; + // captureAnchor records the item currently at the viewport top. Callers run + // it after every committed render so the anchor always reflects the latest + // settled layout. + captureAnchor: () => void; + // restoreAnchor re-pins to the bottom when sticky, otherwise moves the + // captured anchor element back to its recorded viewport offset. It is the + // single owner of scrollTop correction and is idempotent. + restoreAnchor: () => void; }; export function useScrollAnchor(): ScrollAnchor { @@ -55,14 +86,42 @@ export function useScrollAnchor(): ScrollAnchor { setAtBottom(true); }, []); - // maintainPin keeps the viewport pinned to the bottom while the user has not - // scrolled away. Callers run this in a layout effect on content changes so - // new messages pin synchronously, independent of ResizeObserver timing. - const maintainPin = useCallback(() => { + const captureAnchor = useCallback(() => { const scroller = scrollerRef.current; - if (scroller && stickRef.current) { - scroller.scrollTop = scroller.scrollHeight; + const content = contentRef.current; + if (!scroller || !content) { + return; } + anchorRef.current = stickRef.current + ? null + : findTopAnchor(scroller, content); + }, []); + + const restoreAnchor = useCallback(() => { + const scroller = scrollerRef.current; + const content = contentRef.current; + if (!scroller || !content) { + return; + } + if (stickRef.current) { + scroller.scrollTop = scroller.scrollHeight; + return; + } + const anchor = anchorRef.current; + if (!anchor) { + return; + } + const el = resolveAnchorElement(content, anchor); + if (!el) { + return; + } + const offset = + el.getBoundingClientRect().top - scroller.getBoundingClientRect().top; + scroller.scrollTop = correctedScrollTop( + scroller.scrollTop, + anchor.offset, + offset, + ); }, []); useEffect(() => { @@ -76,10 +135,8 @@ export function useScrollAnchor(): ScrollAnchor { const onScroll = () => { const scrollTop = scroller.scrollTop; // A real user scroll always moves scrollTop. Layout-induced scroll - // events keep the same scrollTop, so processing them would recapture - // the anchor against an already-mutated layout and make the resize - // correction a no-op. Our own correction does move scrollTop, so it - // still re-establishes a fresh anchor afterward. + // events keep the same scrollTop; ignoring them avoids flipping the + // stick state when our own correction nudges the position. if (scrollTop === lastScrollTopRef.current) { return; } @@ -90,7 +147,6 @@ export function useScrollAnchor(): ScrollAnchor { scroller.clientHeight, ); stickRef.current = bottom; - anchorRef.current = bottom ? null : findTopAnchor(scroller, content); if (frame) { return; } @@ -100,26 +156,12 @@ export function useScrollAnchor(): ScrollAnchor { }); }; - // ResizeObserver notifications run between layout and paint, so correcting - // scrollTop here keeps the viewport stable without a visible jump. Safari - // has no native scroll anchoring, which is why we must do this ourselves. + // The ResizeObserver is a fallback trigger for async intrinsic growth + // (streaming markdown, image load, late highlight) that happens without a + // React render. Windowing, measurement, and prepend are driven explicitly + // by the container's layout effect instead. const resize = new ResizeObserver(() => { - if (stickRef.current) { - scroller.scrollTop = scroller.scrollHeight; - return; - } - const anchor = anchorRef.current; - if (!anchor) { - return; - } - const offset = - anchor.el.getBoundingClientRect().top - - scroller.getBoundingClientRect().top; - scroller.scrollTop = correctedScrollTop( - scroller.scrollTop, - anchor.offset, - offset, - ); + restoreAnchor(); }); scroller.addEventListener("scroll", onScroll, { passive: true }); @@ -131,7 +173,14 @@ export function useScrollAnchor(): ScrollAnchor { cancelAnimationFrame(frame); } }; - }, []); + }, [restoreAnchor]); - return { scrollerRef, contentRef, atBottom, scrollToBottom, maintainPin }; + return { + scrollerRef, + contentRef, + atBottom, + scrollToBottom, + captureAnchor, + restoreAnchor, + }; } diff --git a/site/src/pages/AgentsPage/components/ChatVirtualList/windowMath.test.ts b/site/src/pages/AgentsPage/components/ChatVirtualList/windowMath.test.ts new file mode 100644 index 0000000000..dfabe8a832 --- /dev/null +++ b/site/src/pages/AgentsPage/components/ChatVirtualList/windowMath.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; +import { computeWindow, cumulativeOffsets } from "./windowMath"; + +describe("cumulativeOffsets", () => { + it("returns N+1 offsets with the total last", () => { + expect(cumulativeOffsets([100, 200, 50])).toEqual([0, 100, 300, 350]); + }); + it("returns [0] for an empty list", () => { + expect(cumulativeOffsets([])).toEqual([0]); + }); +}); + +describe("computeWindow", () => { + const overscan = 1000; + + it("renders the whole list when it fits the window", () => { + const offsets = cumulativeOffsets([100, 200, 50]); + expect( + computeWindow({ offsets, scrollTop: 0, viewportHeight: 1000, overscan }), + ).toEqual({ start: 0, end: 2, topPad: 0, bottomPad: 0 }); + }); + + it("windows the middle of a long list with overscan", () => { + // 100 items of 100px each, total 10000. + const offsets = cumulativeOffsets(Array.from({ length: 100 }, () => 100)); + const win = computeWindow({ + offsets, + scrollTop: 5000, + viewportHeight: 500, + overscan, + }); + // windowTop = 4000, windowBottom = 6500. + expect(win.start).toBe(40); + expect(win.end).toBe(64); + expect(win.topPad).toBe(4000); + expect(win.bottomPad).toBe(10000 - 6500); + }); + + it("clamps the top: scrollTop 0 gives start 0 and no top pad", () => { + const offsets = cumulativeOffsets(Array.from({ length: 100 }, () => 100)); + const win = computeWindow({ + offsets, + scrollTop: 0, + viewportHeight: 500, + overscan, + }); + expect(win.start).toBe(0); + expect(win.topPad).toBe(0); + }); + + it("clamps the bottom: at max scroll, end is last and no bottom pad", () => { + const offsets = cumulativeOffsets(Array.from({ length: 100 }, () => 100)); + const win = computeWindow({ + offsets, + scrollTop: 10000 - 500, + viewportHeight: 500, + overscan, + }); + expect(win.end).toBe(99); + expect(win.bottomPad).toBe(0); + }); + + it("returns an empty window for an empty list", () => { + expect( + computeWindow({ + offsets: [0], + scrollTop: 0, + viewportHeight: 500, + overscan, + }), + ).toEqual({ start: 0, end: -1, topPad: 0, bottomPad: 0 }); + }); +}); diff --git a/site/src/pages/AgentsPage/components/ChatVirtualList/windowMath.ts b/site/src/pages/AgentsPage/components/ChatVirtualList/windowMath.ts new file mode 100644 index 0000000000..52a6a8ae80 --- /dev/null +++ b/site/src/pages/AgentsPage/components/ChatVirtualList/windowMath.ts @@ -0,0 +1,66 @@ +// Pure layout math for the windowing renderer. Kept free of DOM and React so it +// can be unit tested and reused by the container without re-rendering. + +// cumulativeOffsets returns N+1 entries: the running top offset of each item, +// with the final entry holding the total height. +export function cumulativeOffsets(heights: readonly number[]): number[] { + const offsets = new Array(heights.length + 1); + offsets[0] = 0; + for (let i = 0; i < heights.length; i++) { + offsets[i + 1] = offsets[i] + heights[i]; + } + return offsets; +} + +type WindowInput = { + offsets: readonly number[]; + scrollTop: number; + viewportHeight: number; + overscan: number; +}; + +type Window = { + start: number; + end: number; + topPad: number; + bottomPad: number; +}; + +// computeWindow returns the inclusive [start, end] item range to render plus the +// spacer heights that reserve the unrendered ranges. The range covers the +// viewport expanded by `overscan` on each side. An empty list yields an empty +// range (end = -1) and zero pads. +export function computeWindow({ + offsets, + scrollTop, + viewportHeight, + overscan, +}: WindowInput): Window { + const count = offsets.length - 1; + const total = offsets[count]; + if (count <= 0) { + return { start: 0, end: -1, topPad: 0, bottomPad: 0 }; + } + + const windowTop = scrollTop - overscan; + const windowBottom = scrollTop + viewportHeight + overscan; + + // First item whose bottom edge is past the window top. + let start = 0; + while (start < count - 1 && offsets[start + 1] <= windowTop) { + start++; + } + + // Last item whose top edge is before the window bottom. + let end = start; + while (end < count - 1 && offsets[end + 1] < windowBottom) { + end++; + } + + return { + start, + end, + topPad: offsets[start], + bottomPad: total - offsets[end + 1], + }; +}