Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/renderer/components/thread/ChatPane/ChatPane.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ vi.mock("@tanstack/react-virtual", () => ({
getTotalSize: () => options.count * 96,
measure: vi.fn<() => void>(),
measureElement: vi.fn<(element: HTMLDivElement | null) => void>(),
resizeItem: vi.fn<(index: number, size: number) => void>(),
options: { measureElement: () => 96 },
scrollToIndex: (
index: number,
scrollOptions?: { align?: "auto" | "center" | "end" | "start" },
Expand Down
156 changes: 83 additions & 73 deletions src/renderer/components/thread/ChatPane/parts/MessageList.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { act, fireEvent, render, screen } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { useAppStore } from "@/renderer/state/appStore";
import { ChatPaneActionsContext, useChatPaneActions } from "../chatPaneActionsContext";
import {
ChatPaneActionsContext,
useChatPaneActions,
type ChatPaneActions,
} from "../chatPaneActionsContext";
import { MessageList } from "./MessageList";

type MockVirtualRow = {
Expand All @@ -15,6 +19,8 @@ type MockVirtualizer = {
getTotalSize: () => number;
measure: () => void;
measureElement: (element: HTMLDivElement | null) => void;
resizeItem: (index: number, size: number) => void;
options: { measureElement: (element: Element, entry: undefined, instance: unknown) => number };
scrollToIndex: (index: number, options?: { align?: "start" | "center" | "end" | "auto" }) => void;
shouldAdjustScrollPositionOnItemSizeChange?: (
item: { start: number; size: number },
Expand All @@ -34,13 +40,18 @@ const {
useVirtualizerMock,
measureMock,
measureElementMock,
resizeItemMock,
optionsMeasureElementMock,
scrollToIndexMock,
getVirtualItemsMock,
getTotalSizeMock,
} = vi.hoisted(() => ({
useVirtualizerMock: vi.fn<(options: MockVirtualizerOptions) => MockVirtualizer>(),
measureMock: vi.fn<() => void>(),
measureElementMock: vi.fn<(element: HTMLDivElement | null) => void>(),
resizeItemMock: vi.fn<(index: number, size: number) => void>(),
optionsMeasureElementMock:
vi.fn<(element: Element, entry: undefined, instance: unknown) => number>(),
scrollToIndexMock:
vi.fn<(index: number, options?: { align?: "start" | "center" | "end" | "auto" }) => void>(),
getVirtualItemsMock: vi.fn<() => MockVirtualRow[]>(),
Expand Down Expand Up @@ -77,11 +88,14 @@ describe("MessageList", () => {
{ key: "row-3", index: 2, start: 192 },
]);
getTotalSizeMock.mockReturnValue(384);
optionsMeasureElementMock.mockReturnValue(96);
useVirtualizerMock.mockReturnValue({
getVirtualItems: getVirtualItemsMock,
getTotalSize: getTotalSizeMock,
measure: measureMock,
measureElement: measureElementMock,
resizeItem: resizeItemMock,
options: { measureElement: optionsMeasureElementMock },
scrollToIndex: scrollToIndexMock,
});
});
Expand Down Expand Up @@ -140,78 +154,40 @@ describe("MessageList", () => {
expect(document.querySelector("[data-item-id='item-2']")).not.toHaveAttribute("style");
});

it("only adjusts scroll for measured rows fully above the viewport", () => {
const scrollElement = document.createElement("div");
scrollElement.scrollTop = 160;

render(
<MessageList
threadId="thread-1"
entries={makeEntries(["item-1", "item-2", "item-3", "item-4"])}
scrollElement={scrollElement}
/>,
);
it("never lets TanStack adjust scroll itself and compensates rows fully above the viewport on the next commit", () => {
const { scrollElement, shouldAdjust, commit } = renderCompensationList();

const virtualizer = useVirtualizerMock.mock.results[0]!.value;
const shouldAdjust = virtualizer.shouldAdjustScrollPositionOnItemSizeChange!;
expect(shouldAdjust({ start: 0, size: 80 }, 40, idleVirtualizer)).toBe(false);

const idleVirtualizer = { isScrolling: false, scrollDirection: null } as const;
expect(shouldAdjust({ start: 0, size: 80 }, 40, idleVirtualizer)).toBe(true);
expect(shouldAdjust({ start: 96, size: 100 }, 40, idleVirtualizer)).toBe(false);
commit();
expect(scrollElement.scrollTop).toBe(200);
});

it("does not adjust scroll for rows above the viewport during active upward scroll", () => {
const scrollElement = document.createElement("div");
scrollElement.scrollTop = 160;
it("does not compensate rows that overlap or sit below the viewport", () => {
const { scrollElement, shouldAdjust, commit } = renderCompensationList();

render(
<MessageList
threadId="thread-1"
entries={makeEntries(["item-1", "item-2", "item-3", "item-4"])}
scrollElement={scrollElement}
/>,
);

const virtualizer = useVirtualizerMock.mock.results[0]!.value;
const shouldAdjust = virtualizer.shouldAdjustScrollPositionOnItemSizeChange!;
expect(shouldAdjust({ start: 96, size: 100 }, 40, idleVirtualizer)).toBe(false);

expect(
shouldAdjust({ start: 0, size: 80 }, -40, {
isScrolling: true,
scrollDirection: "backward",
}),
).toBe(false);
commit();
expect(scrollElement.scrollTop).toBe(160);
});

it("does not adjust scroll for delayed row measurements after scrolling upward", () => {
const scrollElement = document.createElement("div");
scrollElement.scrollTop = 160;

render(
<MessageList
threadId="thread-1"
entries={makeEntries(["item-1", "item-2", "item-3", "item-4"])}
scrollElement={scrollElement}
/>,
);

const virtualizer = useVirtualizerMock.mock.results[0]!.value;
const shouldAdjust = virtualizer.shouldAdjustScrollPositionOnItemSizeChange!;
it("compensates rows above the viewport during active upward scroll", () => {
const { scrollElement, shouldAdjust, commit } = renderCompensationList();

scrollElement.scrollTop = 120;
fireEvent.scroll(scrollElement);

expect(
shouldAdjust({ start: 0, size: 80 }, -40, {
isScrolling: false,
scrollDirection: null,
isScrolling: true,
scrollDirection: "backward",
}),
).toBe(false);

commit();
expect(scrollElement.scrollTop).toBe(80);
});

it("adjusts streaming row height changes when bottom-sticky", () => {
const scrollElement = document.createElement("div");
scrollElement.scrollTop = 160;
it("compensates streaming row height changes when bottom-sticky", () => {
const actions = {
openProjectRelativePath: vi.fn<(path: string, lineNumber?: number) => void>(),
revealProjectFolderInTree: vi.fn<(path: string) => void>(),
Expand All @@ -221,26 +197,28 @@ describe("MessageList", () => {
projectLocation: { kind: "windows" as const, path: "C:\\repo" },
projectRootNames: new Set<string>(),
};
const { scrollElement, shouldAdjust, commit } = renderCompensationList(actions);

expect(shouldAdjust({ start: 96, size: 100 }, 24, idleVirtualizer)).toBe(false);

commit();
expect(scrollElement.scrollTop).toBe(184);
});

it("measures newly mounted rows synchronously so the size correction lands in the mount commit", () => {
const scrollElement = document.createElement("div");
optionsMeasureElementMock.mockReturnValue(132);

render(
<ChatPaneActionsContext.Provider value={actions}>
<MessageList
threadId="thread-1"
entries={makeEntries(["item-1", "item-2", "item-3", "item-4"])}
scrollElement={scrollElement}
/>
</ChatPaneActionsContext.Provider>,
<MessageList
threadId="thread-1"
entries={makeEntries(["item-1", "item-2", "item-3", "item-4"])}
scrollElement={scrollElement}
/>,
);

const virtualizer = useVirtualizerMock.mock.results[0]!.value;
const shouldAdjust = virtualizer.shouldAdjustScrollPositionOnItemSizeChange!;

expect(
shouldAdjust({ start: 96, size: 100 }, 24, {
isScrolling: false,
scrollDirection: null,
}),
).toBe(true);
expect(resizeItemMock).toHaveBeenCalledWith(1, 132);
expect(resizeItemMock).toHaveBeenCalledWith(2, 132);
});

it("registers TanStack scrollToIndex as the bottom scroll handler", () => {
Expand Down Expand Up @@ -479,3 +457,35 @@ describe("MessageList", () => {
function makeEntries(itemIds: readonly string[]) {
return itemIds.map((id) => ({ kind: "item" as const, id }));
}

const idleVirtualizer = { isScrolling: false, scrollDirection: null } as const;

/**
* Renders a MessageList wired for the scroll-compensation tests and returns
* the intercepted size-change predicate plus a `commit` that re-renders so the
* pending compensation layout effect applies.
*/
function renderCompensationList(actions?: ChatPaneActions) {
const scrollElement = document.createElement("div");
scrollElement.scrollTop = 160;
const entries = makeEntries(["item-1", "item-2", "item-3", "item-4"]);
// A fresh element per render: re-passing the identical element would let
// React bail out and skip the commit the compensation effect runs in.
const makeUi = () => {
const list = (
<MessageList threadId="thread-1" entries={entries} scrollElement={scrollElement} />
);
return actions ? (
<ChatPaneActionsContext.Provider value={actions}>{list}</ChatPaneActionsContext.Provider>
) : (
list
);
};
const { rerender } = render(makeUi());
const virtualizer = useVirtualizerMock.mock.results[0]!.value;
return {
scrollElement,
shouldAdjust: virtualizer.shouldAdjustScrollPositionOnItemSizeChange!,
commit: () => rerender(makeUi()),
};
}
67 changes: 34 additions & 33 deletions src/renderer/components/thread/ChatPane/parts/MessageList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ interface MessageListProps {

const CHAT_TRANSCRIPT_OVERSCAN = 8;
const DEFAULT_ROW_ESTIMATE_PX = 96;
const UPWARD_SCROLL_MEASUREMENT_SUPPRESSION_MS = 750;
const SKIP_REVERT_CONFIRM_PREF_KEY = "lightcode-chat-checkpoint-revert-skip-confirm";

// Intentionally not wrapped in `React.memo`: pane swaps preserve this fiber
Expand All @@ -65,8 +64,7 @@ export function MessageList({
const parentActions = useChatPaneActions();
const virtualSizeBoxRef = useRef<HTMLDivElement | null>(null);
const rowElementsRef = useRef(new Map<number, HTMLDivElement>());
const lastScrollTopRef = useRef(0);
const suppressUpwardSizeAdjustmentUntilRef = useRef(0);
const pendingScrollCompensationRef = useRef(0);
const [measureEpoch, setMeasureEpoch] = useState(0);
const [pendingRevertItemId, setPendingRevertItemId] = useState<string | null>(null);
const [dontAskAgain, setDontAskAgain] = useState(false);
Expand Down Expand Up @@ -97,42 +95,35 @@ export function MessageList({
}, [entries.length, parentActions, virtualizer]);

useLayoutEffect(() => {
if (!scrollElement) return;
lastScrollTopRef.current = scrollElement.scrollTop;

const handleScroll = () => {
const prevScrollTop = lastScrollTopRef.current;
const nextScrollTop = scrollElement.scrollTop;
lastScrollTopRef.current = nextScrollTop;
if (nextScrollTop < prevScrollTop) {
suppressUpwardSizeAdjustmentUntilRef.current =
performance.now() + UPWARD_SCROLL_MEASUREMENT_SUPPRESSION_MS;
}
};

scrollElement.addEventListener("scroll", handleScroll, { passive: true });
return () => scrollElement.removeEventListener("scroll", handleScroll);
}, [scrollElement]);

useLayoutEffect(() => {
// Adjust scroll only when the resized row is fully above the viewport.
// This is what keeps visible content anchored as overscan rows replace
// their estimated heights with real measurements — critical for smooth
// idle reading. During and shortly after upward scrolling, suppress the
// compensation so delayed row measurements do not pull the transcript.
virtualizer.shouldAdjustScrollPositionOnItemSizeChange = (item, _delta, instance) => {
// Compensate scroll when a row above the viewport (or any row while
// bottom-sticky) trades its estimated height for a real measurement —
// otherwise the estimate error shifts the visible content. Always return
// false so TanStack never calls scrollTo itself: that adjustment runs
// outside React's commit and paints one frame apart from the translateY /
// row-height change, which reads as flicker. Instead the delta is
// accumulated here and applied in the layout effect below, in the same
// commit (and therefore the same paint) as the DOM change it compensates.
virtualizer.shouldAdjustScrollPositionOnItemSizeChange = (item, delta) => {
if (!scrollElement) return false;
const now = performance.now();
if (instance.isScrolling && instance.scrollDirection === "backward") return false;
if (now <= suppressUpwardSizeAdjustmentUntilRef.current) return false;
if (parentActions?.isStickToBottom?.()) return true;
return item.start + item.size <= scrollElement.scrollTop;
if (parentActions?.isStickToBottom?.() || item.start + item.size <= scrollElement.scrollTop) {
pendingScrollCompensationRef.current += delta;
}
return false;
};
return () => {
virtualizer.shouldAdjustScrollPositionOnItemSizeChange = undefined;
};
}, [parentActions, scrollElement, virtualizer]);

// Intentionally dependency-free: runs after every commit, once row refs have
// synchronously measured newly mounted rows, so the scroll correction lands
// before the browser paints the row's real height.
useLayoutEffect(() => {
if (pendingScrollCompensationRef.current === 0 || !scrollElement) return;
scrollElement.scrollTop += pendingScrollCompensationRef.current;
pendingScrollCompensationRef.current = 0;
});

useLayoutEffect(() => {
const virtualSizeBox = virtualSizeBoxRef.current;
if (!virtualSizeBox) return;
Expand Down Expand Up @@ -182,10 +173,20 @@ export function MessageList({
(index: number, element: HTMLDivElement | null) => {
if (element) {
rowElementsRef.current.set(index, element);
virtualizer.measureElement(element);
// TanStack defers ref-time measurement to the next ResizeObserver
// frame while scrolling, but the row's real DOM height is already on
// screen this commit — a frame with the old scroll offset would paint
// shifted. Measure synchronously so the estimate correction (and the
// pending scroll compensation it accumulates) applies pre-paint.
virtualizer.resizeItem(
index,
virtualizer.options.measureElement(element, undefined, virtualizer),
);
} else {
rowElementsRef.current.delete(index);
virtualizer.measureElement(element);
}
virtualizer.measureElement(element);
},
[virtualizer],
);
Expand Down