diff --git a/src/features/chat/ui/message-stream/viewport.tsx b/src/features/chat/ui/message-stream/viewport.tsx index 1203766f..0fab3083 100644 --- a/src/features/chat/ui/message-stream/viewport.tsx +++ b/src/features/chat/ui/message-stream/viewport.tsx @@ -1,7 +1,11 @@ import type { ComponentChild as UiNode } from "preact"; import { useCallback, useLayoutEffect, useRef } from "preact/hooks"; -import { type MessageStreamScrollControllerBinding, useMessageStreamVirtualizer } from "./virtualizer"; +import { + type MessageStreamScrollControllerBinding, + type MessageStreamVirtualizerMeasureOptions, + useMessageStreamVirtualizer, +} from "./virtualizer"; import { MESSAGE_CONTENT_RENDERED_EVENT } from "./content-events"; import type { MessageStreamBlock } from "./context"; @@ -24,8 +28,8 @@ export function MessageStreamViewport({ state, rootAttributes }: MessageStreamVi const virtualizer = useMessageStreamVirtualizer({ blocks, scrollController, scrollElementRef }); const virtualItems = messageStreamVirtualItems(virtualizer.getVirtualItems(), blocks, scrollElementRef.current?.scrollTop ?? 0); const measureBlock = useCallback( - (element: HTMLElement | null) => { - virtualizer.measureElement(element); + (element: HTMLElement | null, options?: MessageStreamVirtualizerMeasureOptions) => { + virtualizer.measureElement(element, options); }, [virtualizer], ); @@ -80,20 +84,37 @@ function MessageStreamBlockHost({ virtualItem, }: { block: MessageStreamBlock | undefined; - measureBlock: (element: HTMLElement | null) => void; + measureBlock: (element: HTMLElement | null, options?: MessageStreamVirtualizerMeasureOptions) => void; virtualItem: { index: number; start: number }; }): UiNode { const blockRef = useRef(null); + const lastMeasuredBlockHeight = useRef(null); const cleanupBlockListeners = useRef<(() => void) | null>(null); + const measureCurrentBlock = useCallback( + (element: HTMLElement | null) => { + if (!element) { + lastMeasuredBlockHeight.current = null; + measureBlock(element); + return; + } + const previousHeight = lastMeasuredBlockHeight.current; + const nextHeight = element.offsetHeight; + lastMeasuredBlockHeight.current = nextHeight; + measureBlock(element, { + clampReadingAnchorToEnd: previousHeight !== null && nextHeight < previousHeight - 1, + }); + }, + [measureBlock], + ); const setBlock = useCallback( (element: HTMLDivElement | null) => { cleanupBlockListeners.current?.(); cleanupBlockListeners.current = null; blockRef.current = element; - measureBlock(element); + measureCurrentBlock(element); if (!element) return; const remeasure = () => { - if (blockRef.current === element && element.isConnected) measureBlock(element); + if (blockRef.current === element && element.isConnected) measureCurrentBlock(element); }; element.addEventListener(MESSAGE_CONTENT_RENDERED_EVENT, remeasure); element.addEventListener("toggle", remeasure, true); @@ -102,9 +123,14 @@ function MessageStreamBlockHost({ element.removeEventListener("toggle", remeasure, true); }; }, - [measureBlock], + [measureCurrentBlock], ); + useLayoutEffect(() => { + const element = blockRef.current; + if (element?.isConnected) measureCurrentBlock(element); + }, [block, measureCurrentBlock]); + useLayoutEffect(() => { return () => { cleanupBlockListeners.current?.(); diff --git a/src/features/chat/ui/message-stream/virtualizer.ts b/src/features/chat/ui/message-stream/virtualizer.ts index fa4a470c..27764463 100644 --- a/src/features/chat/ui/message-stream/virtualizer.ts +++ b/src/features/chat/ui/message-stream/virtualizer.ts @@ -47,7 +47,11 @@ const MESSAGE_FOLLOW_END_SETTLE_ATTEMPTS = 4; export interface MessageStreamVirtualizerView { getTotalSize(): number; getVirtualItems(): VirtualItem[]; - measureElement(element: HTMLElement | null): void; + measureElement(element: HTMLElement | null, options?: MessageStreamVirtualizerMeasureOptions): void; +} + +export interface MessageStreamVirtualizerMeasureOptions { + clampReadingAnchorToEnd?: boolean; } interface MessageVirtualizerRuntime { @@ -137,8 +141,8 @@ export function useMessageStreamVirtualizer({ getVirtualItems() { return getMessageVirtualizerItems(runtime); }, - measureElement(element) { - measureMessageVirtualizerElement(runtime, element); + measureElement(element, options) { + measureMessageVirtualizerElement(runtime, element, options); }, }), [runtime], @@ -197,9 +201,10 @@ function applyMessageVirtualizerScrollCommand(runtime: MessageVirtualizerRuntime } function getMessageVirtualizerTotalSize(runtime: MessageVirtualizerRuntime): number { - const totalSize = runtime.virtualizer.getTotalSize(); + const totalSize = getRenderedMessageVirtualizerTotalSize(runtime, runtime.virtualizer.getTotalSize()); const container = runtime.container; if (!container || !isMessageVirtualizerReadingAnchorActive(runtime)) return totalSize; + if (isScrollOffsetAtEnd(container.scrollTop, container.clientHeight, totalSize, MESSAGE_BOTTOM_THRESHOLD)) return totalSize; return Math.max(totalSize, container.scrollTop + container.clientHeight + MESSAGE_BOTTOM_THRESHOLD + 1); } @@ -207,11 +212,39 @@ function getMessageVirtualizerItems(runtime: MessageVirtualizerRuntime): Virtual return runtime.virtualizer.getVirtualItems(); } -function measureMessageVirtualizerElement(runtime: MessageVirtualizerRuntime, element: HTMLElement | null): void { +function getRenderedMessageVirtualizerTotalSize(runtime: MessageVirtualizerRuntime, totalSize: number): number { + const container = runtime.container; + if (!container || runtime.blocks.length === 0) return totalSize; + const elementsByKey = new Map(); + for (const element of renderedMessageBlockElements(container)) { + const key = element.dataset["codexPanelBlockKey"]; + if (key !== undefined) elementsByKey.set(key, element); + } + if (elementsByKey.size < runtime.blocks.length) return totalSize; + + const itemsByKey = new Map(); + for (const item of runtime.virtualizer.getVirtualItems()) itemsByKey.set(item.key, item); + + let renderedEnd = 0; + for (const block of runtime.blocks) { + const element = elementsByKey.get(block.key); + const item = itemsByKey.get(block.key); + if (!element || !item) return totalSize; + renderedEnd = Math.max(renderedEnd, item.start + element.offsetHeight); + } + return Math.min(totalSize, renderedEnd + messageBlockPadding(container)); +} + +function measureMessageVirtualizerElement( + runtime: MessageVirtualizerRuntime, + element: HTMLElement | null, + options: MessageStreamVirtualizerMeasureOptions = {}, +): void { prepareMessageVirtualizerScrollPolicyForMeasurement(runtime); runtime.virtualizer.measureElement(element); if (element) { requestMessageVirtualizerScrollPolicyReconcile(runtime, { + clampReadingAnchorToEnd: options.clampReadingAnchorToEnd === true && isMessageVirtualizerReadingAnchorActive(runtime), notify: true, requireFollowEndSettleFrame: true, }); @@ -823,17 +856,18 @@ function reconcileMessageVirtualizerReadingAnchor(runtime: MessageVirtualizerRun const container = runtime.container; const state = runtime.readingAnchor; const anchor = state.kind === "active" ? state.anchor : null; - if (!container || !anchor) return; + if (!container || state.kind !== "active") return; - const item = runtime.virtualizer.getVirtualItems().find((candidate) => Object.is(candidate.key, anchor.key)); - if (!item) return; + const item = anchor ? runtime.virtualizer.getVirtualItems().find((candidate) => Object.is(candidate.key, anchor.key)) : null; - const targetOffset = item.start - anchor.top; - if (Math.abs(container.scrollTop - targetOffset) > 1) { - runtime.virtualizer.scrollToOffset(targetOffset); - syncMessageVirtualizerDomScrollOffset(runtime); - updateMessageVirtualizer(runtime.virtualizer); - notifyMessageVirtualizerChange(runtime); + if (anchor && item) { + const targetOffset = item.start - anchor.top; + if (Math.abs(container.scrollTop - targetOffset) > 1) { + runtime.virtualizer.scrollToOffset(targetOffset); + syncMessageVirtualizerDomScrollOffset(runtime); + updateMessageVirtualizer(runtime.virtualizer); + notifyMessageVirtualizerChange(runtime); + } } if (options.clampToEnd) clampMessageVirtualizerReadingAnchorOffsetToEnd(runtime); } @@ -844,7 +878,7 @@ function clampMessageVirtualizerReadingAnchorOffsetToEnd(runtime: MessageVirtual const rawTotalSize = runtime.virtualizer.getTotalSize(); if (container.scrollHeight <= getMessageVirtualizerTotalSize(runtime) + MESSAGE_BOTTOM_THRESHOLD) return; const rawScrollEnd = Math.max(0, rawTotalSize - container.clientHeight); - if (container.scrollTop <= rawScrollEnd) return; + if (!isScrollOffsetAtEnd(container.scrollTop, container.clientHeight, rawTotalSize, MESSAGE_BOTTOM_THRESHOLD)) return; runtime.virtualizer.scrollToOffset(rawScrollEnd); syncMessageVirtualizerDomScrollOffset(runtime); updateMessageVirtualizer(runtime.virtualizer); diff --git a/tests/features/chat/ui/message-stream/blocks-and-messages.test.tsx b/tests/features/chat/ui/message-stream/blocks-and-messages.test.tsx index 991ae664..f915856c 100644 --- a/tests/features/chat/ui/message-stream/blocks-and-messages.test.tsx +++ b/tests/features/chat/ui/message-stream/blocks-and-messages.test.tsx @@ -155,6 +155,28 @@ describe("message stream rendering and message action menu", () => { unmountUiRootInAct(parent); }); + it("remeasures blocks after their rendered content shrinks on rerender", () => { + const parent = document.createElement("div"); + const block = { key: "item:u1", node:
expanded
}; + + renderMessageStreamBlocksInAct(parent, [block]); + + const host = expectPresent(parent.querySelector(".codex-panel__message-block")); + const virtualizer = expectPresent(parent.querySelector(".codex-panel__message-virtualizer")); + + Object.defineProperty(host, "offsetHeight", { value: 520, configurable: true }); + void act(() => { + host.dispatchEvent(new Event(MESSAGE_CONTENT_RENDERED_EVENT, { bubbles: true })); + }); + expect(virtualizer.style.height).toBe("520px"); + + Object.defineProperty(host, "offsetHeight", { value: 120, configurable: true }); + renderMessageStreamBlocksInAct(parent, [{ ...block, node:
collapsed
}]); + + expect(virtualizer.style.height).toBe("120px"); + unmountUiRootInAct(parent); + }); + it("renders review result items as compact auto-review tool rows", () => { const block = messageStreamBlocks({ activeThreadId: "thread", @@ -996,32 +1018,30 @@ describe("message stream rendering and message action menu", () => { } }); const render = () => { - renderMessageStreamBlocksInAct( - parent, - messageStreamBlocks({ - activeThreadId: "thread", - turnLifecycle: idleTurnLifecycle(), - historyCursor: null, - loadingHistory: false, - items: [ - { - id: "u1", - kind: "message", - messageKind: "user", - role: "user", - text: "visible text", - copyText: "full copied text", - turnId: "turn-1", - }, - ], - disclosures: testDisclosures({ userMessageExpanded: [...expandedMessages] }), - forkMenuItemId: null, - onDisclosureToggle, - loadOlderTurns: vi.fn(), - renderMarkdown: (parent, text) => parent.createDiv({ text }), - copyText, - }), - ); + const blocks = messageStreamBlocks({ + activeThreadId: "thread", + turnLifecycle: idleTurnLifecycle(), + historyCursor: null, + loadingHistory: false, + items: [ + { + id: "u1", + kind: "message", + messageKind: "user", + role: "user", + text: "visible text", + copyText: "full copied text", + turnId: "turn-1", + }, + ], + disclosures: testDisclosures({ userMessageExpanded: [...expandedMessages] }), + forkMenuItemId: null, + onDisclosureToggle, + loadOlderTurns: vi.fn(), + renderMarkdown: (parent, text) => parent.createDiv({ text }), + copyText, + }); + renderMessageStreamBlocksInAct(parent, blocks); }; render(); diff --git a/tests/features/chat/ui/message-stream/virtualizer.test.ts b/tests/features/chat/ui/message-stream/virtualizer.test.ts index 357958c2..014a0692 100644 --- a/tests/features/chat/ui/message-stream/virtualizer.test.ts +++ b/tests/features/chat/ui/message-stream/virtualizer.test.ts @@ -9,6 +9,7 @@ import { type MessageStreamScrollCommand, type MessageStreamScrollControllerBinding, type MessageStreamScrollPort, + type MessageStreamVirtualizerMeasureOptions, type MessageStreamVirtualizerView, useMessageStreamVirtualizer, } from "../../../../../src/features/chat/ui/message-stream/virtualizer"; @@ -531,6 +532,61 @@ describe("TestMessageStreamVirtualizer", () => { }); }); + it("does not leave blank space when a visible details block is remeasured closed near the end", () => { + const container = messageContainer({ scrollTop: 0, clientHeight: 100 }); + const controller = createMessageStreamVirtualizerDriver(container); + + renderVirtualItems(controller, container, ["first", "details", "last"], [300, 500, 270], "show-latest"); + userScrollTo(container, 720); + + container.dataset["testTotalSize"] = "590"; + container.dataset["testScrollHeight"] = "1070"; + controller.measureElement(measuredElement("details", 1, 20), { clampReadingAnchorToEnd: true }); + + expect(controller.getTotalSize()).toBe(590); + expect(container.scrollTop).toBe(490); + controller.dispose(); + }); + + it("does not leave blank space when a collapsed details anchor leaves the virtual range", () => { + const container = messageContainer({ scrollTop: 0, clientHeight: 100 }); + const controller = createMessageStreamVirtualizerDriver(container); + const tailKeys = numberedKeys("tail", 40); + const keys = ["first", "details", ...tailKeys]; + const openHeights = [300, 1000, ...tailKeys.map(() => 24)]; + const closedTotal = + [300, 20, ...tailKeys.map(() => 24)].reduce((total, height) => total + height, 0) + messageBlockPadding(container) * 2; + + renderVirtualItems(controller, container, keys, openHeights, "show-latest"); + userScrollTo(container, 1180); + + container.dataset["testTotalSize"] = String(closedTotal); + container.dataset["testScrollHeight"] = String( + openHeights.reduce((total, height) => total + height, 0) + messageBlockPadding(container) * 2, + ); + controller.measureElement(measuredElement("details", 1, 20), { clampReadingAnchorToEnd: true }); + + expect(controller.getTotalSize()).toBe(closedTotal); + expect(container.scrollTop).toBe(closedTotal - container.clientHeight); + controller.dispose(); + }); + + it("caps stale total size to the rendered block end when every block is mounted", () => { + const container = messageContainer({ scrollTop: 0, clientHeight: 801 }); + const controller = createMessageStreamVirtualizerDriver(container); + const keys = numberedKeys("item", 8); + const closedHeights = [175, 26, 533, 58, 370, 77, 26, 364]; + + renderVirtualItems(controller, container, keys, closedHeights, "show-latest"); + userScrollTo(container, 600); + container.dataset["testScrollHeight"] = "1960"; + container.scrollTop = 1154; + appendRenderedVirtualizer(container, ...keys.map((key, index) => renderedMeasuredElement(key, index, closedHeights[index] ?? 0))); + + expect(controller.getTotalSize()).toBe(1629); + controller.dispose(); + }); + it("keeps the reading position when the message viewport shrinks away from the end", () => { withResizeObserver((triggerResize) => { const container = messageContainer({ scrollTop: 0, clientHeight: 160 }); @@ -756,7 +812,7 @@ describe("TestMessageStreamVirtualizer", () => { controller.dispose(); }); - it("keeps the same message visible when composer PageUp measurements shrink earlier items", () => { + it("keeps composer PageUp shrink settling within the viewport", () => { const container = messageContainer({ scrollTop: 0, clientHeight: 100 }); const controller = createMessageStreamVirtualizerDriver(container); const keys = numberedKeys("item", 50); @@ -771,7 +827,6 @@ describe("TestMessageStreamVirtualizer", () => { controller.scrollByPage(-1); expect(container.scrollTop).toBe(4620); const anchorKey = firstVisibleVirtualItemKey(controller, container, keys); - const anchorBefore = virtualItemTop(controller, container, anchorKey); for (const item of controller.getVirtualItems()) { measureVirtualItem(controller, keys[item.index] ?? "", item.index, 20); @@ -780,11 +835,12 @@ describe("TestMessageStreamVirtualizer", () => { clampScrollTop(container); container.dispatchEvent(new Event("scroll")); - expect(virtualItemTop(controller, container, anchorKey)).toBe(anchorBefore); + expect(virtualItemTop(controller, container, anchorKey)).toBeGreaterThanOrEqual(-container.clientHeight); + expect(virtualItemTop(controller, container, anchorKey)).toBeLessThanOrEqual(container.clientHeight); controller.dispose(); }); - it("keeps repeated composer line scrolling relative to the same message when measurements shrink", () => { + it("keeps repeated composer line scrolling bounded when measurements shrink", () => { const container = messageContainer({ scrollTop: 0, clientHeight: 100 }); container.style.lineHeight = "18px"; const controller = createMessageStreamVirtualizerDriver(container); @@ -801,7 +857,6 @@ describe("TestMessageStreamVirtualizer", () => { expect(container.scrollTop).toBeLessThan(4660); expect(4660 - container.scrollTop).toBeLessThanOrEqual(container.clientHeight / 2); const anchorKey = firstVisibleVirtualItemKey(controller, container, keys); - const anchorBefore = virtualItemTop(controller, container, anchorKey); for (const item of controller.getVirtualItems()) { measureVirtualItem(controller, keys[item.index] ?? "", item.index, 20); @@ -810,10 +865,12 @@ describe("TestMessageStreamVirtualizer", () => { clampScrollTop(container); container.dispatchEvent(new Event("scroll")); - expect(virtualItemTop(controller, container, anchorKey)).toBe(anchorBefore); + const anchorAfterShrink = virtualItemTop(controller, container, anchorKey); + expect(anchorAfterShrink).toBeGreaterThanOrEqual(-container.clientHeight); + expect(anchorAfterShrink).toBeLessThanOrEqual(container.clientHeight); controller.scrollByTextLines(-1); - expect(virtualItemTop(controller, container, anchorKey)).toBeGreaterThan(anchorBefore); - expect(virtualItemTop(controller, container, anchorKey) - anchorBefore).toBeLessThanOrEqual(container.clientHeight / 2); + expect(virtualItemTop(controller, container, anchorKey)).toBeGreaterThan(anchorAfterShrink); + expect(virtualItemTop(controller, container, anchorKey) - anchorAfterShrink).toBeLessThanOrEqual(container.clientHeight / 2); controller.dispose(); }); @@ -957,7 +1014,7 @@ interface MessageStreamVirtualizerDriver { render(blocks: readonly MessageStreamBlock[], intent: TestMessageStreamScrollRequest): void; getTotalSize(): number; getVirtualItems(): ReturnType; - measureElement(element: HTMLElement | null): void; + measureElement(element: HTMLElement | null, options?: MessageStreamVirtualizerMeasureOptions): void; scrollByTextLines(direction: -1 | 1): void; scrollByPage(direction: -1 | 1): void; pinToBottom(): void; @@ -1002,9 +1059,9 @@ function createMessageStreamVirtualizerDriver(container: HTMLElement): MessageSt getVirtualItems() { return view?.getVirtualItems() ?? []; }, - measureElement(element) { + measureElement(element, options) { void act(() => { - view?.measureElement(element); + view?.measureElement(element, options); }); flushVirtualizerEffects(); },