From 7fadfc6578fb32b1930887cfa9f81c4236ed1943 Mon Sep 17 00:00:00 2001 From: sabraman Date: Tue, 17 Mar 2026 13:07:03 +0300 Subject: [PATCH 1/3] Stabilize chat timeline row measurement --- apps/web/src/components/ChatView.browser.tsx | 389 ++++++++++++++++++ apps/web/src/components/ChatView.tsx | 2 + .../src/components/chat/MessagesTimeline.tsx | 174 +++++++- 3 files changed, 560 insertions(+), 5 deletions(-) diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index faecc7f51..d7ba945c7 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -2,12 +2,14 @@ import "../index.css"; import { + CheckpointRef, ORCHESTRATION_WS_METHODS, type MessageId, type OrchestrationReadModel, type ProjectId, type ServerConfig, type ThreadId, + type TurnId, type WsWelcomePayload, WS_CHANNELS, WS_METHODS, @@ -32,6 +34,7 @@ const PROJECT_ID = "project-1" as ProjectId; const NOW_ISO = "2026-03-04T12:00:00.000Z"; const BASE_TIME_MS = Date.parse(NOW_ISO); const ATTACHMENT_SVG = ""; +const BUTTON_POSITION_TOLERANCE_PX = 6; interface WsRequestEnvelope { id: string; @@ -86,6 +89,7 @@ interface UserRowMeasurement { interface MountedChatView { cleanup: () => Promise; + host: HTMLElement; measureUserRow: (targetMessageId: MessageId) => Promise; setViewport: (viewport: ViewportSpec) => Promise; router: ReturnType; @@ -237,6 +241,120 @@ function createSnapshotForTargetUser(options: { }; } +function createSnapshotWithVirtualizedAssistantDiffSummary(options?: { + pairCount?: number; + overlapPairIndex?: number; +}): { + assistantMessageId: MessageId; + snapshot: OrchestrationReadModel; + userMessageId: MessageId; +} { + const pairCount = options?.pairCount ?? 5; + const overlapPairIndex = options?.overlapPairIndex ?? 0; + const turnId = "turn-overlap-virtualized" as TurnId; + const assistantMessageId = "msg-assistant-overlap-target" as MessageId; + const userMessageId = "msg-user-overlap-next" as MessageId; + const messages: Array = []; + + for (let index = 0; index < pairCount; index += 1) { + const isOverlapPair = index === overlapPairIndex; + messages.push( + createUserMessage({ + id: (isOverlapPair ? "msg-user-overlap-anchor" : `msg-user-overlap-${index}`) as MessageId, + text: isOverlapPair ? "anchor prompt" : `filler user message ${index}`, + offsetSeconds: messages.length * 3, + }), + ); + messages.push( + createAssistantMessage({ + id: isOverlapPair ? assistantMessageId : (`msg-assistant-overlap-${index}` as MessageId), + text: isOverlapPair + ? "assistant response with a large changed files summary" + : `assistant filler ${index}`, + offsetSeconds: messages.length * 3, + }), + ); + } + + const overlapNextUserMessageIndex = overlapPairIndex * 2 + 2; + messages[overlapNextUserMessageIndex] = createUserMessage({ + id: userMessageId, + text: "user message immediately after the virtualized assistant row", + offsetSeconds: overlapNextUserMessageIndex * 3, + }); + + const files = Array.from({ length: 18 }, (_, index) => { + const directory = `packages/feature-${Math.floor(index / 3) + 1}`; + const nestedDirectory = `${directory}/deep-${(index % 3) + 1}`; + return { + path: `${nestedDirectory}/file-${index + 1}.ts`, + kind: "modified", + additions: 10 + index, + deletions: index % 4, + }; + }); + + return { + assistantMessageId, + userMessageId, + snapshot: { + snapshotSequence: 1, + projects: [ + { + id: PROJECT_ID, + title: "Project", + workspaceRoot: "/repo/project", + defaultModel: "gpt-5", + scripts: [], + createdAt: NOW_ISO, + updatedAt: NOW_ISO, + deletedAt: null, + }, + ], + threads: [ + { + id: THREAD_ID, + projectId: PROJECT_ID, + title: "Browser test thread", + model: "gpt-5", + interactionMode: "default", + runtimeMode: "full-access", + branch: "main", + worktreePath: null, + latestTurn: null, + createdAt: NOW_ISO, + updatedAt: NOW_ISO, + deletedAt: null, + messages, + activities: [], + proposedPlans: [], + checkpoints: [ + { + turnId, + checkpointTurnCount: 1, + checkpointRef: CheckpointRef.makeUnsafe("checkpoint-overlap-1"), + status: "ready", + files, + assistantMessageId, + completedAt: isoAt(30), + }, + ], + session: { + threadId: THREAD_ID, + status: "ready", + providerName: "codex", + runtimeMode: "full-access", + activeTurnId: null, + lastError: null, + updatedAt: NOW_ISO, + }, + }, + ], + updatedAt: NOW_ISO, + }, + }; +} + function buildFixture(snapshot: OrchestrationReadModel): TestFixture { return { snapshot, @@ -564,6 +682,118 @@ async function waitForImagesToLoad(scope: ParentNode): Promise { await waitForLayout(); } +async function waitForMessageRow(options: { + host: HTMLElement; + messageId: MessageId; + role: "assistant" | "user"; +}): Promise { + return waitForElement( + () => + options.host.querySelector( + `[data-message-id="${options.messageId}"][data-message-role="${options.role}"]`, + ), + `Unable to locate ${options.role} message row ${options.messageId}.`, + ); +} + +async function scrollElementIntoView(element: HTMLElement): Promise { + element.scrollIntoView({ block: "center" }); + await waitForLayout(); +} + +async function waitForMessageRowButton(options: { + host: HTMLElement; + messageId: MessageId; + role: "assistant" | "user"; + label: string; +}): Promise { + let button: HTMLButtonElement | null = null; + await vi.waitFor( + () => { + const row = options.host.querySelector( + `[data-message-id="${options.messageId}"][data-message-role="${options.role}"]`, + ); + button = + row && + (Array.from(row.querySelectorAll("button")).find( + (candidate) => candidate.textContent?.trim() === options.label, + ) as HTMLButtonElement | null); + expect( + button, + `Unable to locate "${options.label}" button in ${options.role} message row ${options.messageId}.`, + ).toBeTruthy(); + }, + { + timeout: 8_000, + interval: 16, + }, + ); + if (!button) { + throw new Error( + `Unable to locate "${options.label}" button in ${options.role} message row ${options.messageId}.`, + ); + } + return button; +} + +async function waitForNoVerticalOverlap(options: { + host: HTMLElement; + previousAssistantMessageId: MessageId; + nextUserMessageId: MessageId; +}): Promise<{ + assistantRenderedInVirtualizedRegion: boolean; + nextUserRenderedInVirtualizedRegion: boolean; +}> { + let assistantRenderedInVirtualizedRegion = false; + let nextUserRenderedInVirtualizedRegion = false; + + await vi.waitFor( + async () => { + await waitForLayout(); + const assistantRow = await waitForMessageRow({ + host: options.host, + messageId: options.previousAssistantMessageId, + role: "assistant", + }); + const nextUserRow = await waitForMessageRow({ + host: options.host, + messageId: options.nextUserMessageId, + role: "user", + }); + + const assistantRect = assistantRow.getBoundingClientRect(); + const nextUserRect = nextUserRow.getBoundingClientRect(); + assistantRenderedInVirtualizedRegion = + assistantRow.closest("[data-index]") instanceof HTMLElement; + nextUserRenderedInVirtualizedRegion = + nextUserRow.closest("[data-index]") instanceof HTMLElement; + + expect( + assistantRect.bottom, + "Expected the previous assistant row to end before the following user row starts.", + ).toBeLessThanOrEqual(nextUserRect.top + 1); + }, + { + timeout: 8_000, + interval: 16, + }, + ); + + return { assistantRenderedInVirtualizedRegion, nextUserRenderedInVirtualizedRegion }; +} + +async function clickButtonByText(host: HTMLElement, label: string): Promise { + const button = await waitForElement( + () => + Array.from(host.querySelectorAll("button")).find( + (candidate) => candidate.textContent?.trim() === label, + ) as HTMLButtonElement | null, + `Unable to find "${label}" button.`, + ); + button.click(); + await waitForLayout(); +} + async function measureUserRow(options: { host: HTMLElement; targetMessageId: MessageId; @@ -664,6 +894,7 @@ async function mountChatView(options: { await screen.unmount(); host.remove(); }, + host, measureUserRow: async (targetMessageId: MessageId) => measureUserRow({ host, targetMessageId }), setViewport: async (viewport: ViewportSpec) => { await setViewport(viewport); @@ -889,6 +1120,164 @@ describe("ChatView timeline estimator parity (full app)", () => { }, ); + it("keeps the last virtualized assistant diff summary from overlapping the next user row", async () => { + const overlapFixture = createSnapshotWithVirtualizedAssistantDiffSummary(); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: overlapFixture.snapshot, + }); + + try { + const initialMeasurement = await waitForNoVerticalOverlap({ + host: mounted.host, + previousAssistantMessageId: overlapFixture.assistantMessageId, + nextUserMessageId: overlapFixture.userMessageId, + }); + + expect(initialMeasurement.assistantRenderedInVirtualizedRegion).toBe(true); + expect(initialMeasurement.nextUserRenderedInVirtualizedRegion).toBe(false); + + await clickButtonByText(mounted.host, "Collapse all"); + await waitForNoVerticalOverlap({ + host: mounted.host, + previousAssistantMessageId: overlapFixture.assistantMessageId, + nextUserMessageId: overlapFixture.userMessageId, + }); + + await clickButtonByText(mounted.host, "Expand all"); + await waitForNoVerticalOverlap({ + host: mounted.host, + previousAssistantMessageId: overlapFixture.assistantMessageId, + nextUserMessageId: overlapFixture.userMessageId, + }); + } finally { + await mounted.cleanup(); + } + }); + + it("keeps adjacent virtualized rows from overlapping when an assistant diff summary is tall", async () => { + const overlapFixture = createSnapshotWithVirtualizedAssistantDiffSummary({ + pairCount: 12, + overlapPairIndex: 2, + }); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: overlapFixture.snapshot, + }); + + try { + const measurement = await waitForNoVerticalOverlap({ + host: mounted.host, + previousAssistantMessageId: overlapFixture.assistantMessageId, + nextUserMessageId: overlapFixture.userMessageId, + }); + + expect(measurement.assistantRenderedInVirtualizedRegion).toBe(true); + expect(measurement.nextUserRenderedInVirtualizedRegion).toBe(true); + } finally { + await mounted.cleanup(); + } + }); + + it("keeps the virtualized diff-summary button position stable enough after collapse", async () => { + const overlapFixture = createSnapshotWithVirtualizedAssistantDiffSummary(); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: overlapFixture.snapshot, + }); + + try { + const measurement = await waitForNoVerticalOverlap({ + host: mounted.host, + previousAssistantMessageId: overlapFixture.assistantMessageId, + nextUserMessageId: overlapFixture.userMessageId, + }); + + expect(measurement.assistantRenderedInVirtualizedRegion).toBe(true); + + const collapseButton = await waitForMessageRowButton({ + host: mounted.host, + messageId: overlapFixture.assistantMessageId, + role: "assistant", + label: "Collapse all", + }); + await scrollElementIntoView(collapseButton); + const collapseButtonTop = collapseButton.getBoundingClientRect().top; + + collapseButton.click(); + await waitForLayout(); + + const expandButton = await waitForMessageRowButton({ + host: mounted.host, + messageId: overlapFixture.assistantMessageId, + role: "assistant", + label: "Expand all", + }); + await scrollElementIntoView(expandButton); + const expandButtonTop = expandButton.getBoundingClientRect().top; + + expect(Math.abs(expandButtonTop - collapseButtonTop)).toBeLessThanOrEqual( + BUTTON_POSITION_TOLERANCE_PX, + ); + } finally { + await mounted.cleanup(); + } + }); + + it("keeps the virtualized diff-summary button position stable enough after expand", async () => { + const overlapFixture = createSnapshotWithVirtualizedAssistantDiffSummary(); + const mounted = await mountChatView({ + viewport: DEFAULT_VIEWPORT, + snapshot: overlapFixture.snapshot, + }); + + try { + const measurement = await waitForNoVerticalOverlap({ + host: mounted.host, + previousAssistantMessageId: overlapFixture.assistantMessageId, + nextUserMessageId: overlapFixture.userMessageId, + }); + + expect(measurement.assistantRenderedInVirtualizedRegion).toBe(true); + + const collapseButton = await waitForMessageRowButton({ + host: mounted.host, + messageId: overlapFixture.assistantMessageId, + role: "assistant", + label: "Collapse all", + }); + await scrollElementIntoView(collapseButton); + collapseButton.click(); + await waitForLayout(); + + const expandButton = await waitForMessageRowButton({ + host: mounted.host, + messageId: overlapFixture.assistantMessageId, + role: "assistant", + label: "Expand all", + }); + await scrollElementIntoView(expandButton); + const expandButtonTop = expandButton.getBoundingClientRect().top; + + expandButton.click(); + await waitForLayout(); + + const nextCollapseButton = await waitForMessageRowButton({ + host: mounted.host, + messageId: overlapFixture.assistantMessageId, + role: "assistant", + label: "Collapse all", + }); + const nextCollapseButtonTop = nextCollapseButton.getBoundingClientRect().top; + + expect(Math.abs(nextCollapseButtonTop - expandButtonTop)).toBeLessThanOrEqual( + BUTTON_POSITION_TOLERANCE_PX, + ); + } finally { + await mounted.cleanup(); + } + }); + it("opens the project cwd for draft threads without a worktree path", async () => { useComposerDraftStore.setState({ draftThreadsByThreadId: { diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 52637695e..ee867bb88 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -1530,6 +1530,8 @@ export default function ChatView({ threadId }: ChatViewProps) { ); if (!trigger || !scrollContainer.contains(trigger)) return; if (trigger.closest("[data-scroll-anchor-ignore]")) return; + const virtualizedRow = trigger.closest("[data-index]"); + if (virtualizedRow && virtualizedRow.closest('[data-timeline-root="true"]')) return; pendingInteractionAnchorRef.current = { element: trigger, diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index e30801041..876e416bc 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -236,7 +236,22 @@ export const MessagesTimeline = memo(function MessagesTimeline({ if (row.kind === "working") return 40; return estimateTimelineMessageHeight(row.message, { timelineWidthPx }); }, - measureElement: measureVirtualElement, + measureElement: (element, entry, instance) => { + const index = instance.indexFromElement(element); + const row = rows[index]; + const nextSize = measureVirtualElement(element, entry, instance); + if (!row) { + return nextSize; + } + + const previousSize = measuredHeightByRowIdRef.current.get(row.id); + if (previousSize !== undefined && Math.abs(nextSize - previousSize) <= 1) { + return previousSize; + } + + measuredHeightByRowIdRef.current.set(row.id, nextSize); + return nextSize; + }, useAnimationFrameWithResizeObserver: true, overscan: 8, }); @@ -245,17 +260,66 @@ export const MessagesTimeline = memo(function MessagesTimeline({ rowVirtualizer.measure(); }, [rowVirtualizer, timelineWidthPx]); useEffect(() => { - rowVirtualizer.shouldAdjustScrollPositionOnItemSizeChange = (_item, _delta, instance) => { + rowVirtualizer.shouldAdjustScrollPositionOnItemSizeChange = (item, _delta, instance) => { const viewportHeight = instance.scrollRect?.height ?? 0; const scrollOffset = instance.scrollOffset ?? 0; const remainingDistance = instance.getTotalSize() - (scrollOffset + viewportHeight); - return remainingDistance > AUTO_SCROLL_BOTTOM_THRESHOLD_PX; + const changedItemStartsAboveViewport = item.start < scrollOffset; + return changedItemStartsAboveViewport && remainingDistance > AUTO_SCROLL_BOTTOM_THRESHOLD_PX; }; return () => { rowVirtualizer.shouldAdjustScrollPositionOnItemSizeChange = undefined; }; }, [rowVirtualizer]); const pendingMeasureFrameRef = useRef(null); + const measuredHeightByRowIdRef = useRef(new Map()); + const virtualRowElementsByIdRef = useRef(new Map()); + const virtualRowRefCallbacksByIdRef = useRef( + new Map void>(), + ); + const dirtyVirtualRowIdsRef = useRef(new Set()); + const previousHeightSignatureByRowIdRef = useRef(new Map()); + + const clearVirtualRowMeasurementTracking = useCallback((rowId: string) => { + virtualRowElementsByIdRef.current.delete(rowId); + }, []); + + const clearAllVirtualRowMeasurementTracking = useCallback(() => { + const trackedRowIds = new Set(virtualRowElementsByIdRef.current.keys()); + for (const rowId of trackedRowIds) { + clearVirtualRowMeasurementTracking(rowId); + } + }, [clearVirtualRowMeasurementTracking]); + + const getVirtualRowRef = useCallback( + (rowId: string) => { + const cachedCallback = virtualRowRefCallbacksByIdRef.current.get(rowId); + if (cachedCallback) { + return cachedCallback; + } + + const callback = (element: HTMLDivElement | null) => { + const previousElement = virtualRowElementsByIdRef.current.get(rowId); + if (previousElement === element) { + return; + } + + clearVirtualRowMeasurementTracking(rowId); + + if (!element) { + return; + } + + virtualRowElementsByIdRef.current.set(rowId, element); + rowVirtualizer.measureElement(element); + }; + + virtualRowRefCallbacksByIdRef.current.set(rowId, callback); + return callback; + }, + [clearVirtualRowMeasurementTracking, rowVirtualizer], + ); + const onTimelineImageLoad = useCallback(() => { if (pendingMeasureFrameRef.current !== null) return; pendingMeasureFrameRef.current = window.requestAnimationFrame(() => { @@ -269,8 +333,9 @@ export const MessagesTimeline = memo(function MessagesTimeline({ if (frame !== null) { window.cancelAnimationFrame(frame); } + clearAllVirtualRowMeasurementTracking(); }; - }, []); + }, [clearAllVirtualRowMeasurementTracking]); const virtualRows = rowVirtualizer.getVirtualItems(); const nonVirtualizedRows = rows.slice(virtualizedRowCount); @@ -284,6 +349,105 @@ export const MessagesTimeline = memo(function MessagesTimeline({ })); }, []); + const virtualizedHeightSignatures = useMemo(() => { + const nextSignatures = new Map(); + for (const row of rows.slice(0, virtualizedRowCount)) { + if (row.kind === "work") { + nextSignatures.set(row.id, `work:${row.id}:${row.groupedEntries.length}`); + continue; + } + + if (row.kind === "proposed-plan") { + nextSignatures.set( + row.id, + `proposed-plan:${row.id}:${row.proposedPlan.planMarkdown.length}`, + ); + continue; + } + + if (row.kind === "working") { + nextSignatures.set(row.id, `working:${row.id}`); + continue; + } + + const diffSummary = + row.message.role === "assistant" + ? turnDiffSummaryByAssistantMessageId.get(row.message.id) + : undefined; + const diffSummaryTurnId = diffSummary?.turnId ?? ""; + const diffSummaryFileCount = diffSummary?.files.length ?? 0; + const allDirectoriesExpanded = + diffSummary === undefined + ? "" + : String(allDirectoriesExpandedByTurnId[diffSummary.turnId] ?? true); + + nextSignatures.set( + row.id, + [ + "message", + row.id, + row.message.role, + row.message.text.length, + row.message.streaming ? 1 : 0, + row.message.attachments?.length ?? 0, + row.showCompletionDivider ? 1 : 0, + diffSummary ? 1 : 0, + diffSummaryTurnId, + diffSummaryFileCount, + allDirectoriesExpanded, + ].join(":"), + ); + } + return nextSignatures; + }, [ + rows, + virtualizedRowCount, + turnDiffSummaryByAssistantMessageId, + allDirectoriesExpandedByTurnId, + ]); + + useLayoutEffect(() => { + const previousSignatures = previousHeightSignatureByRowIdRef.current; + const dirtyRowIds = dirtyVirtualRowIdsRef.current; + + for (const [rowId, signature] of virtualizedHeightSignatures) { + if (previousSignatures.get(rowId) !== signature) { + dirtyRowIds.add(rowId); + } + } + + for (const rowId of previousSignatures.keys()) { + if (!virtualizedHeightSignatures.has(rowId)) { + previousSignatures.delete(rowId); + virtualRowElementsByIdRef.current.delete(rowId); + virtualRowRefCallbacksByIdRef.current.delete(rowId); + measuredHeightByRowIdRef.current.delete(rowId); + dirtyRowIds.delete(rowId); + } + } + + previousHeightSignatureByRowIdRef.current = new Map(virtualizedHeightSignatures); + }, [virtualizedHeightSignatures]); + + useLayoutEffect(() => { + if (virtualizedRowCount === 0) return; + rowVirtualizer.measure(); + }, [rowVirtualizer, virtualizedRowCount]); + + useLayoutEffect(() => { + const dirtyRowIds = Array.from(dirtyVirtualRowIdsRef.current); + if (dirtyRowIds.length === 0) return; + + for (const rowId of dirtyRowIds) { + const element = virtualRowElementsByIdRef.current.get(rowId); + if (!element) { + continue; + } + rowVirtualizer.measureElement(element); + } + dirtyVirtualRowIdsRef.current.clear(); + }, [rowVirtualizer, virtualizedHeightSignatures]); + const renderRowContent = (row: TimelineRow) => (
From 175ac8f0796fdb39e0afee6f6a966999547622d2 Mon Sep 17 00:00:00 2001 From: sabraman Date: Tue, 17 Mar 2026 15:43:08 +0300 Subject: [PATCH 2/3] Improve initial estimate for assistant diff summaries --- .../src/components/chat/MessagesTimeline.tsx | 47 ++++++++++++++----- apps/web/src/lib/turnDiffTree.test.ts | 19 +++++++- apps/web/src/lib/turnDiffTree.ts | 18 +++++++ 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index 876e416bc..3d0423c32 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -8,7 +8,7 @@ import { import { deriveTimelineEntries, formatElapsed } from "../../session-logic"; import { AUTO_SCROLL_BOTTOM_THRESHOLD_PX } from "../../chat-scroll"; import { type TurnDiffSummary } from "../../types"; -import { summarizeTurnDiffStats } from "../../lib/turnDiffTree"; +import { countVisibleTurnDiffTreeNodes, summarizeTurnDiffStats } from "../../lib/turnDiffTree"; import ChatMarkdown from "../ChatMarkdown"; import { BotIcon, @@ -39,6 +39,9 @@ import { formatTimestamp } from "../../timestampFormat"; const MAX_VISIBLE_WORK_LOG_ENTRIES = 6; const ALWAYS_UNVIRTUALIZED_TAIL_ROWS = 8; +const ASSISTANT_COMPLETION_DIVIDER_HEIGHT_PX = 48; +const ASSISTANT_DIFF_SUMMARY_BASE_HEIGHT_PX = 74; +const ASSISTANT_DIFF_TREE_NODE_HEIGHT_PX = 24; interface MessagesTimelineProps { hasMessages: boolean; @@ -222,6 +225,30 @@ export const MessagesTimeline = memo(function MessagesTimeline({ minimum: 0, maximum: rows.length, }); + const [allDirectoriesExpandedByTurnId, setAllDirectoriesExpandedByTurnId] = useState< + Record + >({}); + const onToggleAllDirectories = useCallback((turnId: TurnId) => { + setAllDirectoriesExpandedByTurnId((current) => ({ + ...current, + [turnId]: !(current[turnId] ?? true), + })); + }, []); + const diffSummaryHeightByAssistantMessageId = useMemo(() => { + const nextHeights = new Map(); + for (const [assistantMessageId, turnSummary] of turnDiffSummaryByAssistantMessageId) { + const visibleNodeCount = countVisibleTurnDiffTreeNodes( + turnSummary.files, + allDirectoriesExpandedByTurnId[turnSummary.turnId] ?? true, + ); + nextHeights.set( + assistantMessageId, + ASSISTANT_DIFF_SUMMARY_BASE_HEIGHT_PX + + visibleNodeCount * ASSISTANT_DIFF_TREE_NODE_HEIGHT_PX, + ); + } + return nextHeights; + }, [allDirectoriesExpandedByTurnId, turnDiffSummaryByAssistantMessageId]); const rowVirtualizer = useVirtualizer({ count: virtualizedRowCount, @@ -234,7 +261,14 @@ export const MessagesTimeline = memo(function MessagesTimeline({ if (row.kind === "work") return 112; if (row.kind === "proposed-plan") return estimateTimelineProposedPlanHeight(row.proposedPlan); if (row.kind === "working") return 40; - return estimateTimelineMessageHeight(row.message, { timelineWidthPx }); + let estimatedHeight = estimateTimelineMessageHeight(row.message, { timelineWidthPx }); + if (row.showCompletionDivider) { + estimatedHeight += ASSISTANT_COMPLETION_DIVIDER_HEIGHT_PX; + } + if (row.message.role === "assistant") { + estimatedHeight += diffSummaryHeightByAssistantMessageId.get(row.message.id) ?? 0; + } + return estimatedHeight; }, measureElement: (element, entry, instance) => { const index = instance.indexFromElement(element); @@ -339,15 +373,6 @@ export const MessagesTimeline = memo(function MessagesTimeline({ const virtualRows = rowVirtualizer.getVirtualItems(); const nonVirtualizedRows = rows.slice(virtualizedRowCount); - const [allDirectoriesExpandedByTurnId, setAllDirectoriesExpandedByTurnId] = useState< - Record - >({}); - const onToggleAllDirectories = useCallback((turnId: TurnId) => { - setAllDirectoriesExpandedByTurnId((current) => ({ - ...current, - [turnId]: !(current[turnId] ?? true), - })); - }, []); const virtualizedHeightSignatures = useMemo(() => { const nextSignatures = new Map(); diff --git a/apps/web/src/lib/turnDiffTree.test.ts b/apps/web/src/lib/turnDiffTree.test.ts index 6389fc3ee..6f52386a7 100644 --- a/apps/web/src/lib/turnDiffTree.test.ts +++ b/apps/web/src/lib/turnDiffTree.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; -import { buildTurnDiffTree, summarizeTurnDiffStats } from "./turnDiffTree"; +import { + buildTurnDiffTree, + countVisibleTurnDiffTreeNodes, + summarizeTurnDiffStats, +} from "./turnDiffTree"; describe("summarizeTurnDiffStats", () => { it("sums only files with numeric additions/deletions", () => { @@ -166,3 +170,16 @@ describe("buildTurnDiffTree", () => { expect(directoryNodes.map((node) => node.path).toSorted()).toEqual([" a", "a"]); }); }); + +describe("countVisibleTurnDiffTreeNodes", () => { + it("counts only top-level rows when collapsed and all rendered rows when expanded", () => { + const files = [ + { path: "apps/web/src/index.ts", additions: 2, deletions: 1 }, + { path: "apps/web/src/app.ts", additions: 1, deletions: 0 }, + { path: "README.md", additions: 3, deletions: 1 }, + ]; + + expect(countVisibleTurnDiffTreeNodes(files, false)).toBe(2); + expect(countVisibleTurnDiffTreeNodes(files, true)).toBe(5); + }); +}); diff --git a/apps/web/src/lib/turnDiffTree.ts b/apps/web/src/lib/turnDiffTree.ts index cd9bfc831..2b2b6b88d 100644 --- a/apps/web/src/lib/turnDiffTree.ts +++ b/apps/web/src/lib/turnDiffTree.ts @@ -170,3 +170,21 @@ export function buildTurnDiffTree(files: ReadonlyArray): Tur return toTreeNodes(root); } + +export function countVisibleTurnDiffTreeNodes( + files: ReadonlyArray, + expanded: boolean, +): number { + return countVisibleNodes(buildTurnDiffTree(files), expanded); +} + +function countVisibleNodes(nodes: ReadonlyArray, expanded: boolean): number { + let count = 0; + for (const node of nodes) { + count += 1; + if (expanded && node.kind === "directory") { + count += countVisibleNodes(node.children, expanded); + } + } + return count; +} From b91483e30d5580656cd604680480df7867659918 Mon Sep 17 00:00:00 2001 From: sabraman Date: Tue, 17 Mar 2026 16:02:22 +0300 Subject: [PATCH 3/3] Keep assistant diff-summary rows out of virtualization --- apps/web/src/components/ChatView.browser.tsx | 28 ++++++++--- .../src/components/chat/MessagesTimeline.tsx | 49 +++++++------------ apps/web/src/lib/turnDiffTree.test.ts | 19 +------ apps/web/src/lib/turnDiffTree.ts | 18 ------- 4 files changed, 38 insertions(+), 76 deletions(-) diff --git a/apps/web/src/components/ChatView.browser.tsx b/apps/web/src/components/ChatView.browser.tsx index d7ba945c7..f8e3401f8 100644 --- a/apps/web/src/components/ChatView.browser.tsx +++ b/apps/web/src/components/ChatView.browser.tsx @@ -115,6 +115,18 @@ function createBaseServerConfig(): ServerConfig { }, ], availableEditors: [], + telegram: { + transport: "threaded-private", + mode: "disabled", + hasBotToken: false, + chatId: null, + chatTitle: null, + botUsername: null, + hasTopicsEnabled: null, + allowsUserCreatedTopics: null, + setupExpiresAt: null, + errorMessage: null, + }, }; } @@ -1134,7 +1146,7 @@ describe("ChatView timeline estimator parity (full app)", () => { nextUserMessageId: overlapFixture.userMessageId, }); - expect(initialMeasurement.assistantRenderedInVirtualizedRegion).toBe(true); + expect(initialMeasurement.assistantRenderedInVirtualizedRegion).toBe(false); expect(initialMeasurement.nextUserRenderedInVirtualizedRegion).toBe(false); await clickButtonByText(mounted.host, "Collapse all"); @@ -1155,7 +1167,7 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); - it("keeps adjacent virtualized rows from overlapping when an assistant diff summary is tall", async () => { + it("keeps assistant diff-summary rows and following rows out of the virtualized region", async () => { const overlapFixture = createSnapshotWithVirtualizedAssistantDiffSummary({ pairCount: 12, overlapPairIndex: 2, @@ -1172,14 +1184,14 @@ describe("ChatView timeline estimator parity (full app)", () => { nextUserMessageId: overlapFixture.userMessageId, }); - expect(measurement.assistantRenderedInVirtualizedRegion).toBe(true); - expect(measurement.nextUserRenderedInVirtualizedRegion).toBe(true); + expect(measurement.assistantRenderedInVirtualizedRegion).toBe(false); + expect(measurement.nextUserRenderedInVirtualizedRegion).toBe(false); } finally { await mounted.cleanup(); } }); - it("keeps the virtualized diff-summary button position stable enough after collapse", async () => { + it("keeps the diff-summary button position stable enough after collapse", async () => { const overlapFixture = createSnapshotWithVirtualizedAssistantDiffSummary(); const mounted = await mountChatView({ viewport: DEFAULT_VIEWPORT, @@ -1193,7 +1205,7 @@ describe("ChatView timeline estimator parity (full app)", () => { nextUserMessageId: overlapFixture.userMessageId, }); - expect(measurement.assistantRenderedInVirtualizedRegion).toBe(true); + expect(measurement.assistantRenderedInVirtualizedRegion).toBe(false); const collapseButton = await waitForMessageRowButton({ host: mounted.host, @@ -1224,7 +1236,7 @@ describe("ChatView timeline estimator parity (full app)", () => { } }); - it("keeps the virtualized diff-summary button position stable enough after expand", async () => { + it("keeps the diff-summary button position stable enough after expand", async () => { const overlapFixture = createSnapshotWithVirtualizedAssistantDiffSummary(); const mounted = await mountChatView({ viewport: DEFAULT_VIEWPORT, @@ -1238,7 +1250,7 @@ describe("ChatView timeline estimator parity (full app)", () => { nextUserMessageId: overlapFixture.userMessageId, }); - expect(measurement.assistantRenderedInVirtualizedRegion).toBe(true); + expect(measurement.assistantRenderedInVirtualizedRegion).toBe(false); const collapseButton = await waitForMessageRowButton({ host: mounted.host, diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index 3d0423c32..8e1054a0a 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -8,7 +8,7 @@ import { import { deriveTimelineEntries, formatElapsed } from "../../session-logic"; import { AUTO_SCROLL_BOTTOM_THRESHOLD_PX } from "../../chat-scroll"; import { type TurnDiffSummary } from "../../types"; -import { countVisibleTurnDiffTreeNodes, summarizeTurnDiffStats } from "../../lib/turnDiffTree"; +import { summarizeTurnDiffStats } from "../../lib/turnDiffTree"; import ChatMarkdown from "../ChatMarkdown"; import { BotIcon, @@ -39,9 +39,6 @@ import { formatTimestamp } from "../../timestampFormat"; const MAX_VISIBLE_WORK_LOG_ENTRIES = 6; const ALWAYS_UNVIRTUALIZED_TAIL_ROWS = 8; -const ASSISTANT_COMPLETION_DIVIDER_HEIGHT_PX = 48; -const ASSISTANT_DIFF_SUMMARY_BASE_HEIGHT_PX = 74; -const ASSISTANT_DIFF_TREE_NODE_HEIGHT_PX = 24; interface MessagesTimelineProps { hasMessages: boolean; @@ -184,8 +181,18 @@ export const MessagesTimeline = memo(function MessagesTimeline({ }, [timelineEntries, completionDividerBeforeEntryId, isWorking, activeTurnStartedAt]); const firstUnvirtualizedRowIndex = useMemo(() => { + const firstDiffSummaryRowIndex = rows.findIndex( + (row) => + row.kind === "message" && + row.message.role === "assistant" && + turnDiffSummaryByAssistantMessageId.has(row.message.id), + ); const firstTailRowIndex = Math.max(rows.length - ALWAYS_UNVIRTUALIZED_TAIL_ROWS, 0); - if (!activeTurnInProgress) return firstTailRowIndex; + let firstUnvirtualizedIndex = firstTailRowIndex; + if (firstDiffSummaryRowIndex >= 0) { + firstUnvirtualizedIndex = Math.min(firstUnvirtualizedIndex, firstDiffSummaryRowIndex); + } + if (!activeTurnInProgress) return firstUnvirtualizedIndex; const turnStartedAtMs = typeof activeTurnStartedAt === "string" ? Date.parse(activeTurnStartedAt) : Number.NaN; @@ -205,21 +212,21 @@ export const MessagesTimeline = memo(function MessagesTimeline({ ); } - if (firstCurrentTurnRowIndex < 0) return firstTailRowIndex; + if (firstCurrentTurnRowIndex < 0) return firstUnvirtualizedIndex; for (let index = firstCurrentTurnRowIndex - 1; index >= 0; index -= 1) { const previousRow = rows[index]; if (!previousRow || previousRow.kind !== "message") continue; if (previousRow.message.role === "user") { - return Math.min(index, firstTailRowIndex); + return Math.min(index, firstUnvirtualizedIndex); } if (previousRow.message.role === "assistant" && !previousRow.message.streaming) { break; } } - return Math.min(firstCurrentTurnRowIndex, firstTailRowIndex); - }, [activeTurnInProgress, activeTurnStartedAt, rows]); + return Math.min(firstCurrentTurnRowIndex, firstUnvirtualizedIndex); + }, [activeTurnInProgress, activeTurnStartedAt, rows, turnDiffSummaryByAssistantMessageId]); const virtualizedRowCount = clamp(firstUnvirtualizedRowIndex, { minimum: 0, @@ -234,21 +241,6 @@ export const MessagesTimeline = memo(function MessagesTimeline({ [turnId]: !(current[turnId] ?? true), })); }, []); - const diffSummaryHeightByAssistantMessageId = useMemo(() => { - const nextHeights = new Map(); - for (const [assistantMessageId, turnSummary] of turnDiffSummaryByAssistantMessageId) { - const visibleNodeCount = countVisibleTurnDiffTreeNodes( - turnSummary.files, - allDirectoriesExpandedByTurnId[turnSummary.turnId] ?? true, - ); - nextHeights.set( - assistantMessageId, - ASSISTANT_DIFF_SUMMARY_BASE_HEIGHT_PX + - visibleNodeCount * ASSISTANT_DIFF_TREE_NODE_HEIGHT_PX, - ); - } - return nextHeights; - }, [allDirectoriesExpandedByTurnId, turnDiffSummaryByAssistantMessageId]); const rowVirtualizer = useVirtualizer({ count: virtualizedRowCount, @@ -261,14 +253,7 @@ export const MessagesTimeline = memo(function MessagesTimeline({ if (row.kind === "work") return 112; if (row.kind === "proposed-plan") return estimateTimelineProposedPlanHeight(row.proposedPlan); if (row.kind === "working") return 40; - let estimatedHeight = estimateTimelineMessageHeight(row.message, { timelineWidthPx }); - if (row.showCompletionDivider) { - estimatedHeight += ASSISTANT_COMPLETION_DIVIDER_HEIGHT_PX; - } - if (row.message.role === "assistant") { - estimatedHeight += diffSummaryHeightByAssistantMessageId.get(row.message.id) ?? 0; - } - return estimatedHeight; + return estimateTimelineMessageHeight(row.message, { timelineWidthPx }); }, measureElement: (element, entry, instance) => { const index = instance.indexFromElement(element); diff --git a/apps/web/src/lib/turnDiffTree.test.ts b/apps/web/src/lib/turnDiffTree.test.ts index 6f52386a7..6389fc3ee 100644 --- a/apps/web/src/lib/turnDiffTree.test.ts +++ b/apps/web/src/lib/turnDiffTree.test.ts @@ -1,10 +1,6 @@ import { describe, expect, it } from "vitest"; -import { - buildTurnDiffTree, - countVisibleTurnDiffTreeNodes, - summarizeTurnDiffStats, -} from "./turnDiffTree"; +import { buildTurnDiffTree, summarizeTurnDiffStats } from "./turnDiffTree"; describe("summarizeTurnDiffStats", () => { it("sums only files with numeric additions/deletions", () => { @@ -170,16 +166,3 @@ describe("buildTurnDiffTree", () => { expect(directoryNodes.map((node) => node.path).toSorted()).toEqual([" a", "a"]); }); }); - -describe("countVisibleTurnDiffTreeNodes", () => { - it("counts only top-level rows when collapsed and all rendered rows when expanded", () => { - const files = [ - { path: "apps/web/src/index.ts", additions: 2, deletions: 1 }, - { path: "apps/web/src/app.ts", additions: 1, deletions: 0 }, - { path: "README.md", additions: 3, deletions: 1 }, - ]; - - expect(countVisibleTurnDiffTreeNodes(files, false)).toBe(2); - expect(countVisibleTurnDiffTreeNodes(files, true)).toBe(5); - }); -}); diff --git a/apps/web/src/lib/turnDiffTree.ts b/apps/web/src/lib/turnDiffTree.ts index 2b2b6b88d..cd9bfc831 100644 --- a/apps/web/src/lib/turnDiffTree.ts +++ b/apps/web/src/lib/turnDiffTree.ts @@ -170,21 +170,3 @@ export function buildTurnDiffTree(files: ReadonlyArray): Tur return toTreeNodes(root); } - -export function countVisibleTurnDiffTreeNodes( - files: ReadonlyArray, - expanded: boolean, -): number { - return countVisibleNodes(buildTurnDiffTree(files), expanded); -} - -function countVisibleNodes(nodes: ReadonlyArray, expanded: boolean): number { - let count = 0; - for (const node of nodes) { - count += 1; - if (expanded && node.kind === "directory") { - count += countVisibleNodes(node.children, expanded); - } - } - return count; -}