Skip to content

Commit 192ae6d

Browse files
committed
fix(messages): immediate re-pin auto-scroll on thread switch
1 parent 284c685 commit 192ae6d

4 files changed

Lines changed: 95 additions & 28 deletions

File tree

memory/decisions.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,3 +281,10 @@ Type: preference
281281
Event: User requested both TCP and Orbit provider options be visibly marked as in progress.
282282
Action: Updated provider selector labels to `TCP (wip)` and `Orbit (wip)` in settings.
283283
Rule: Keep both remote provider labels marked `(wip)` until user requests removal after production-readiness.
284+
285+
## 2026-02-07 21:06
286+
Context: Conversation auto-scroll regression (jump before re-pin)
287+
Type: decision
288+
Event: Smooth, delayed auto-scroll on streaming/appended messages caused visible upward jump then repin in the conversation view.
289+
Action: Updated `Messages` auto-scroll paths to immediate container-bottom pinning (`scrollTop = scrollHeight`) and removed delayed smooth auto-scroll behavior for automatic updates.
290+
Rule: Automatic message pinning should be immediate and non-animated; reserve smooth scrolling for explicit user-initiated navigation only.

memory/mistakes.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,13 @@ Rule: Do not auto-run desktop-only diagnostics on mobile surfaces; gate by platf
111111
Root cause: Auto-refresh effect was scoped by provider/mode only and assumed desktop capabilities.
112112
Fix applied: Updated `src/features/settings/components/SettingsView.tsx` effect logic and added `src/utils/platformPaths.test.ts` coverage for mobile detection.
113113
Prevention rule: For any auto-run settings helper, explicitly classify desktop-only vs cross-platform behavior before wiring useEffect refreshes.
114+
115+
## 2026-02-07 21:09
116+
Context: Messages auto-scroll regression follow-up (thread switch)
117+
Type: mistake
118+
Event: Converting the auto-scroll effect to `useLayoutEffect` introduced an ordering bug where thread switches could skip initial re-pin if the previous thread was scrolled up.
119+
Action: Switched thread-change `autoScrollRef` reset to `useLayoutEffect`, added `threadId` to auto-scroll layout effect dependencies, and added a regression test for thread-switch re-pin behavior.
120+
Rule: When converting effects between `useEffect` and `useLayoutEffect`, preserve ordering guarantees for dependent refs across thread/navigation boundaries.
121+
Root cause: `autoScrollRef.current = true` still ran in `useEffect` after the new layout scroll pass, so first render on thread switch could evaluate stale `false`.
122+
Fix applied: Updated `src/features/messages/components/Messages.tsx` hook ordering/dependencies and added `re-pins to bottom on thread switch even when previous thread was scrolled up` in `src/features/messages/components/Messages.test.tsx`.
123+
Prevention rule: For scroll/anchor refs, pair layout-timing ref resets with layout-timing consumers and add regression coverage for cross-thread transitions.

src/features/messages/components/Messages.test.tsx

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,4 +716,59 @@ describe("Messages", () => {
716716
expect(screen.getByText("5 tool calls")).toBeTruthy();
717717
});
718718
});
719+
720+
it("re-pins to bottom on thread switch even when previous thread was scrolled up", () => {
721+
const items: ConversationItem[] = [
722+
{
723+
id: "msg-shared",
724+
kind: "message",
725+
role: "assistant",
726+
text: "Shared tail",
727+
},
728+
];
729+
730+
const { container, rerender } = render(
731+
<Messages
732+
items={items}
733+
threadId="thread-1"
734+
workspaceId="ws-1"
735+
isThinking={false}
736+
openTargets={[]}
737+
selectedOpenAppId=""
738+
/>,
739+
);
740+
741+
const messagesNode = container.querySelector(".messages.messages-full");
742+
expect(messagesNode).toBeTruthy();
743+
const scrollNode = messagesNode as HTMLDivElement;
744+
745+
Object.defineProperty(scrollNode, "clientHeight", {
746+
configurable: true,
747+
value: 200,
748+
});
749+
Object.defineProperty(scrollNode, "scrollHeight", {
750+
configurable: true,
751+
value: 600,
752+
});
753+
scrollNode.scrollTop = 100;
754+
fireEvent.scroll(scrollNode);
755+
756+
Object.defineProperty(scrollNode, "scrollHeight", {
757+
configurable: true,
758+
value: 900,
759+
});
760+
761+
rerender(
762+
<Messages
763+
items={items}
764+
threadId="thread-2"
765+
workspaceId="ws-1"
766+
isThinking={false}
767+
openTargets={[]}
768+
selectedOpenAppId=""
769+
/>,
770+
);
771+
772+
expect(scrollNode.scrollTop).toBe(900);
773+
});
719774
});

src/features/messages/components/Messages.tsx

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react";
1+
import {
2+
memo,
3+
useCallback,
4+
useEffect,
5+
useLayoutEffect,
6+
useMemo,
7+
useRef,
8+
useState,
9+
} from "react";
210
import { createPortal } from "react-dom";
311
import { convertFileSrc } from "@tauri-apps/api/core";
412
import Brain from "lucide-react/dist/esm/icons/brain";
@@ -1161,19 +1169,20 @@ export const Messages = memo(function Messages({
11611169
};
11621170

11631171
const requestAutoScroll = useCallback(() => {
1164-
if (!bottomRef.current) {
1165-
return;
1166-
}
11671172
const container = containerRef.current;
11681173
const shouldScroll =
11691174
autoScrollRef.current || (container ? isNearBottom(container) : true);
11701175
if (!shouldScroll) {
11711176
return;
11721177
}
1173-
bottomRef.current.scrollIntoView({ behavior: "smooth", block: "end" });
1178+
if (container) {
1179+
container.scrollTop = container.scrollHeight;
1180+
return;
1181+
}
1182+
bottomRef.current?.scrollIntoView({ block: "end" });
11741183
}, [isNearBottom]);
11751184

1176-
useEffect(() => {
1185+
useLayoutEffect(() => {
11771186
autoScrollRef.current = true;
11781187
}, [threadId]);
11791188
const toggleExpanded = useCallback((id: string) => {
@@ -1289,34 +1298,20 @@ export const Messages = memo(function Messages({
12891298
[],
12901299
);
12911300

1292-
useEffect(() => {
1293-
if (!bottomRef.current) {
1294-
return undefined;
1295-
}
1301+
useLayoutEffect(() => {
12961302
const container = containerRef.current;
12971303
const shouldScroll =
12981304
autoScrollRef.current ||
12991305
(container ? isNearBottom(container) : true);
13001306
if (!shouldScroll) {
1301-
return undefined;
1307+
return;
13021308
}
1303-
let raf1 = 0;
1304-
let raf2 = 0;
1305-
const target = bottomRef.current;
1306-
raf1 = window.requestAnimationFrame(() => {
1307-
raf2 = window.requestAnimationFrame(() => {
1308-
target.scrollIntoView({ behavior: "smooth", block: "end" });
1309-
});
1310-
});
1311-
return () => {
1312-
if (raf1) {
1313-
window.cancelAnimationFrame(raf1);
1314-
}
1315-
if (raf2) {
1316-
window.cancelAnimationFrame(raf2);
1317-
}
1318-
};
1319-
}, [scrollKey, isThinking, isNearBottom]);
1309+
if (container) {
1310+
container.scrollTop = container.scrollHeight;
1311+
return;
1312+
}
1313+
bottomRef.current?.scrollIntoView({ block: "end" });
1314+
}, [scrollKey, isThinking, isNearBottom, threadId]);
13201315

13211316
const groupedItems = useMemo(() => buildToolGroups(visibleItems), [visibleItems]);
13221317

0 commit comments

Comments
 (0)