Skip to content

Commit afe9d60

Browse files
authored
fix(chat-pane): keep transcript anchored during row sizing (#153)
- replace scroll-event suppression with explicit resize compensation for virtual rows - update MessageList tests and ChatPane virtualizer mocks for the new measurement API
1 parent d205dcb commit afe9d60

3 files changed

Lines changed: 119 additions & 106 deletions

File tree

src/renderer/components/thread/ChatPane/ChatPane.test.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ vi.mock("@tanstack/react-virtual", () => ({
4141
getTotalSize: () => options.count * 96,
4242
measure: vi.fn<() => void>(),
4343
measureElement: vi.fn<(element: HTMLDivElement | null) => void>(),
44+
resizeItem: vi.fn<(index: number, size: number) => void>(),
45+
options: { measureElement: () => 96 },
4446
scrollToIndex: (
4547
index: number,
4648
scrollOptions?: { align?: "auto" | "center" | "end" | "start" },

src/renderer/components/thread/ChatPane/parts/MessageList.test.tsx

Lines changed: 83 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { act, fireEvent, render, screen } from "@testing-library/react";
22
import { beforeEach, describe, expect, it, vi } from "vitest";
33
import { useAppStore } from "@/renderer/state/appStore";
4-
import { ChatPaneActionsContext, useChatPaneActions } from "../chatPaneActionsContext";
4+
import {
5+
ChatPaneActionsContext,
6+
useChatPaneActions,
7+
type ChatPaneActions,
8+
} from "../chatPaneActionsContext";
59
import { MessageList } from "./MessageList";
610

711
type MockVirtualRow = {
@@ -15,6 +19,8 @@ type MockVirtualizer = {
1519
getTotalSize: () => number;
1620
measure: () => void;
1721
measureElement: (element: HTMLDivElement | null) => void;
22+
resizeItem: (index: number, size: number) => void;
23+
options: { measureElement: (element: Element, entry: undefined, instance: unknown) => number };
1824
scrollToIndex: (index: number, options?: { align?: "start" | "center" | "end" | "auto" }) => void;
1925
shouldAdjustScrollPositionOnItemSizeChange?: (
2026
item: { start: number; size: number },
@@ -34,13 +40,18 @@ const {
3440
useVirtualizerMock,
3541
measureMock,
3642
measureElementMock,
43+
resizeItemMock,
44+
optionsMeasureElementMock,
3745
scrollToIndexMock,
3846
getVirtualItemsMock,
3947
getTotalSizeMock,
4048
} = vi.hoisted(() => ({
4149
useVirtualizerMock: vi.fn<(options: MockVirtualizerOptions) => MockVirtualizer>(),
4250
measureMock: vi.fn<() => void>(),
4351
measureElementMock: vi.fn<(element: HTMLDivElement | null) => void>(),
52+
resizeItemMock: vi.fn<(index: number, size: number) => void>(),
53+
optionsMeasureElementMock:
54+
vi.fn<(element: Element, entry: undefined, instance: unknown) => number>(),
4455
scrollToIndexMock:
4556
vi.fn<(index: number, options?: { align?: "start" | "center" | "end" | "auto" }) => void>(),
4657
getVirtualItemsMock: vi.fn<() => MockVirtualRow[]>(),
@@ -77,11 +88,14 @@ describe("MessageList", () => {
7788
{ key: "row-3", index: 2, start: 192 },
7889
]);
7990
getTotalSizeMock.mockReturnValue(384);
91+
optionsMeasureElementMock.mockReturnValue(96);
8092
useVirtualizerMock.mockReturnValue({
8193
getVirtualItems: getVirtualItemsMock,
8294
getTotalSize: getTotalSizeMock,
8395
measure: measureMock,
8496
measureElement: measureElementMock,
97+
resizeItem: resizeItemMock,
98+
options: { measureElement: optionsMeasureElementMock },
8599
scrollToIndex: scrollToIndexMock,
86100
});
87101
});
@@ -140,78 +154,40 @@ describe("MessageList", () => {
140154
expect(document.querySelector("[data-item-id='item-2']")).not.toHaveAttribute("style");
141155
});
142156

143-
it("only adjusts scroll for measured rows fully above the viewport", () => {
144-
const scrollElement = document.createElement("div");
145-
scrollElement.scrollTop = 160;
146-
147-
render(
148-
<MessageList
149-
threadId="thread-1"
150-
entries={makeEntries(["item-1", "item-2", "item-3", "item-4"])}
151-
scrollElement={scrollElement}
152-
/>,
153-
);
157+
it("never lets TanStack adjust scroll itself and compensates rows fully above the viewport on the next commit", () => {
158+
const { scrollElement, shouldAdjust, commit } = renderCompensationList();
154159

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

158-
const idleVirtualizer = { isScrolling: false, scrollDirection: null } as const;
159-
expect(shouldAdjust({ start: 0, size: 80 }, 40, idleVirtualizer)).toBe(true);
160-
expect(shouldAdjust({ start: 96, size: 100 }, 40, idleVirtualizer)).toBe(false);
162+
commit();
163+
expect(scrollElement.scrollTop).toBe(200);
161164
});
162165

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

167-
render(
168-
<MessageList
169-
threadId="thread-1"
170-
entries={makeEntries(["item-1", "item-2", "item-3", "item-4"])}
171-
scrollElement={scrollElement}
172-
/>,
173-
);
174-
175-
const virtualizer = useVirtualizerMock.mock.results[0]!.value;
176-
const shouldAdjust = virtualizer.shouldAdjustScrollPositionOnItemSizeChange!;
169+
expect(shouldAdjust({ start: 96, size: 100 }, 40, idleVirtualizer)).toBe(false);
177170

178-
expect(
179-
shouldAdjust({ start: 0, size: 80 }, -40, {
180-
isScrolling: true,
181-
scrollDirection: "backward",
182-
}),
183-
).toBe(false);
171+
commit();
172+
expect(scrollElement.scrollTop).toBe(160);
184173
});
185174

186-
it("does not adjust scroll for delayed row measurements after scrolling upward", () => {
187-
const scrollElement = document.createElement("div");
188-
scrollElement.scrollTop = 160;
189-
190-
render(
191-
<MessageList
192-
threadId="thread-1"
193-
entries={makeEntries(["item-1", "item-2", "item-3", "item-4"])}
194-
scrollElement={scrollElement}
195-
/>,
196-
);
197-
198-
const virtualizer = useVirtualizerMock.mock.results[0]!.value;
199-
const shouldAdjust = virtualizer.shouldAdjustScrollPositionOnItemSizeChange!;
175+
it("compensates rows above the viewport during active upward scroll", () => {
176+
const { scrollElement, shouldAdjust, commit } = renderCompensationList();
200177

201178
scrollElement.scrollTop = 120;
202-
fireEvent.scroll(scrollElement);
203-
204179
expect(
205180
shouldAdjust({ start: 0, size: 80 }, -40, {
206-
isScrolling: false,
207-
scrollDirection: null,
181+
isScrolling: true,
182+
scrollDirection: "backward",
208183
}),
209184
).toBe(false);
185+
186+
commit();
187+
expect(scrollElement.scrollTop).toBe(80);
210188
});
211189

212-
it("adjusts streaming row height changes when bottom-sticky", () => {
213-
const scrollElement = document.createElement("div");
214-
scrollElement.scrollTop = 160;
190+
it("compensates streaming row height changes when bottom-sticky", () => {
215191
const actions = {
216192
openProjectRelativePath: vi.fn<(path: string, lineNumber?: number) => void>(),
217193
revealProjectFolderInTree: vi.fn<(path: string) => void>(),
@@ -221,26 +197,28 @@ describe("MessageList", () => {
221197
projectLocation: { kind: "windows" as const, path: "C:\\repo" },
222198
projectRootNames: new Set<string>(),
223199
};
200+
const { scrollElement, shouldAdjust, commit } = renderCompensationList(actions);
201+
202+
expect(shouldAdjust({ start: 96, size: 100 }, 24, idleVirtualizer)).toBe(false);
203+
204+
commit();
205+
expect(scrollElement.scrollTop).toBe(184);
206+
});
207+
208+
it("measures newly mounted rows synchronously so the size correction lands in the mount commit", () => {
209+
const scrollElement = document.createElement("div");
210+
optionsMeasureElementMock.mockReturnValue(132);
224211

225212
render(
226-
<ChatPaneActionsContext.Provider value={actions}>
227-
<MessageList
228-
threadId="thread-1"
229-
entries={makeEntries(["item-1", "item-2", "item-3", "item-4"])}
230-
scrollElement={scrollElement}
231-
/>
232-
</ChatPaneActionsContext.Provider>,
213+
<MessageList
214+
threadId="thread-1"
215+
entries={makeEntries(["item-1", "item-2", "item-3", "item-4"])}
216+
scrollElement={scrollElement}
217+
/>,
233218
);
234219

235-
const virtualizer = useVirtualizerMock.mock.results[0]!.value;
236-
const shouldAdjust = virtualizer.shouldAdjustScrollPositionOnItemSizeChange!;
237-
238-
expect(
239-
shouldAdjust({ start: 96, size: 100 }, 24, {
240-
isScrolling: false,
241-
scrollDirection: null,
242-
}),
243-
).toBe(true);
220+
expect(resizeItemMock).toHaveBeenCalledWith(1, 132);
221+
expect(resizeItemMock).toHaveBeenCalledWith(2, 132);
244222
});
245223

246224
it("registers TanStack scrollToIndex as the bottom scroll handler", () => {
@@ -479,3 +457,35 @@ describe("MessageList", () => {
479457
function makeEntries(itemIds: readonly string[]) {
480458
return itemIds.map((id) => ({ kind: "item" as const, id }));
481459
}
460+
461+
const idleVirtualizer = { isScrolling: false, scrollDirection: null } as const;
462+
463+
/**
464+
* Renders a MessageList wired for the scroll-compensation tests and returns
465+
* the intercepted size-change predicate plus a `commit` that re-renders so the
466+
* pending compensation layout effect applies.
467+
*/
468+
function renderCompensationList(actions?: ChatPaneActions) {
469+
const scrollElement = document.createElement("div");
470+
scrollElement.scrollTop = 160;
471+
const entries = makeEntries(["item-1", "item-2", "item-3", "item-4"]);
472+
// A fresh element per render: re-passing the identical element would let
473+
// React bail out and skip the commit the compensation effect runs in.
474+
const makeUi = () => {
475+
const list = (
476+
<MessageList threadId="thread-1" entries={entries} scrollElement={scrollElement} />
477+
);
478+
return actions ? (
479+
<ChatPaneActionsContext.Provider value={actions}>{list}</ChatPaneActionsContext.Provider>
480+
) : (
481+
list
482+
);
483+
};
484+
const { rerender } = render(makeUi());
485+
const virtualizer = useVirtualizerMock.mock.results[0]!.value;
486+
return {
487+
scrollElement,
488+
shouldAdjust: virtualizer.shouldAdjustScrollPositionOnItemSizeChange!,
489+
commit: () => rerender(makeUi()),
490+
};
491+
}

src/renderer/components/thread/ChatPane/parts/MessageList.tsx

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ interface MessageListProps {
4747

4848
const CHAT_TRANSCRIPT_OVERSCAN = 8;
4949
const DEFAULT_ROW_ESTIMATE_PX = 96;
50-
const UPWARD_SCROLL_MEASUREMENT_SUPPRESSION_MS = 750;
5150
const SKIP_REVERT_CONFIRM_PREF_KEY = "lightcode-chat-checkpoint-revert-skip-confirm";
5251

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

9997
useLayoutEffect(() => {
100-
if (!scrollElement) return;
101-
lastScrollTopRef.current = scrollElement.scrollTop;
102-
103-
const handleScroll = () => {
104-
const prevScrollTop = lastScrollTopRef.current;
105-
const nextScrollTop = scrollElement.scrollTop;
106-
lastScrollTopRef.current = nextScrollTop;
107-
if (nextScrollTop < prevScrollTop) {
108-
suppressUpwardSizeAdjustmentUntilRef.current =
109-
performance.now() + UPWARD_SCROLL_MEASUREMENT_SUPPRESSION_MS;
110-
}
111-
};
112-
113-
scrollElement.addEventListener("scroll", handleScroll, { passive: true });
114-
return () => scrollElement.removeEventListener("scroll", handleScroll);
115-
}, [scrollElement]);
116-
117-
useLayoutEffect(() => {
118-
// Adjust scroll only when the resized row is fully above the viewport.
119-
// This is what keeps visible content anchored as overscan rows replace
120-
// their estimated heights with real measurements — critical for smooth
121-
// idle reading. During and shortly after upward scrolling, suppress the
122-
// compensation so delayed row measurements do not pull the transcript.
123-
virtualizer.shouldAdjustScrollPositionOnItemSizeChange = (item, _delta, instance) => {
98+
// Compensate scroll when a row above the viewport (or any row while
99+
// bottom-sticky) trades its estimated height for a real measurement —
100+
// otherwise the estimate error shifts the visible content. Always return
101+
// false so TanStack never calls scrollTo itself: that adjustment runs
102+
// outside React's commit and paints one frame apart from the translateY /
103+
// row-height change, which reads as flicker. Instead the delta is
104+
// accumulated here and applied in the layout effect below, in the same
105+
// commit (and therefore the same paint) as the DOM change it compensates.
106+
virtualizer.shouldAdjustScrollPositionOnItemSizeChange = (item, delta) => {
124107
if (!scrollElement) return false;
125-
const now = performance.now();
126-
if (instance.isScrolling && instance.scrollDirection === "backward") return false;
127-
if (now <= suppressUpwardSizeAdjustmentUntilRef.current) return false;
128-
if (parentActions?.isStickToBottom?.()) return true;
129-
return item.start + item.size <= scrollElement.scrollTop;
108+
if (parentActions?.isStickToBottom?.() || item.start + item.size <= scrollElement.scrollTop) {
109+
pendingScrollCompensationRef.current += delta;
110+
}
111+
return false;
130112
};
131113
return () => {
132114
virtualizer.shouldAdjustScrollPositionOnItemSizeChange = undefined;
133115
};
134116
}, [parentActions, scrollElement, virtualizer]);
135117

118+
// Intentionally dependency-free: runs after every commit, once row refs have
119+
// synchronously measured newly mounted rows, so the scroll correction lands
120+
// before the browser paints the row's real height.
121+
useLayoutEffect(() => {
122+
if (pendingScrollCompensationRef.current === 0 || !scrollElement) return;
123+
scrollElement.scrollTop += pendingScrollCompensationRef.current;
124+
pendingScrollCompensationRef.current = 0;
125+
});
126+
136127
useLayoutEffect(() => {
137128
const virtualSizeBox = virtualSizeBoxRef.current;
138129
if (!virtualSizeBox) return;
@@ -182,10 +173,20 @@ export function MessageList({
182173
(index: number, element: HTMLDivElement | null) => {
183174
if (element) {
184175
rowElementsRef.current.set(index, element);
176+
virtualizer.measureElement(element);
177+
// TanStack defers ref-time measurement to the next ResizeObserver
178+
// frame while scrolling, but the row's real DOM height is already on
179+
// screen this commit — a frame with the old scroll offset would paint
180+
// shifted. Measure synchronously so the estimate correction (and the
181+
// pending scroll compensation it accumulates) applies pre-paint.
182+
virtualizer.resizeItem(
183+
index,
184+
virtualizer.options.measureElement(element, undefined, virtualizer),
185+
);
185186
} else {
186187
rowElementsRef.current.delete(index);
188+
virtualizer.measureElement(element);
187189
}
188-
virtualizer.measureElement(element);
189190
},
190191
[virtualizer],
191192
);

0 commit comments

Comments
 (0)