fix(web): preserve Chat scroll position across Chat/Comments tab switches (#790) (#841)

* fix(web): preserve Chat scroll position across Chat/Comments tab switches (#790)

The chat-log <div> in ChatPane is conditionally rendered (the inner
`{tab === 'chat' ? <>...</> : null}` branch). When the user switches
to Comments and back, the chat-log is unmounted and remounted; the
remounted element starts at scrollTop=0, and the initial-bottom-scroll
effect skips because didInitialScrollRef.current is already true from
the original mount. Result: the conversation view jumps to the top
instead of preserving the user's reading position.

Replaced the empty-deps scroll listener with a tab-keyed effect that:
1. Captures scrollTop in the existing onScroll handler so the saved
   position is always current.
2. On every mount of the chat-log (when tab becomes 'chat'), restores
   the saved scrollTop on the next animation frame so layout finishes
   before the scroll write lands.

The existing scrolledFromBottom signal that drives the jump-to-bottom
button is folded into the same handler and now correctly re-attaches
on every chat-log remount, fixing a secondary issue where that listener
would silently stop firing after a tab toggle.

* fix(web): preserve bottom-pinned chat across off-tab streaming and snapshot on unmount

Round 1 saved an absolute scrollTop, so a user who left Chat while
pinned to the bottom came back above any new messages that streamed
in while Comments was open. Save a discriminated state instead:
{ pinnedToBottom: true } when the user was within 50px of the bottom,
otherwise { scrollTop }. On remount, pinned state snaps to the new
scrollHeight so bottom-followers stay pinned; non-pinned state
restores the absolute offset.

Also snapshot the final scroll state in the effect cleanup before
removing the listener, so programmatic scrolls or layout shifts
right before unmount don't leave the ref stale.

Adds tests/components/chat-scroll-preservation.test.tsx covering
both branches.

* fix(web): clear saved chat scroll state on conversation switch

The savedChatScrollRef persisted across conversation changes, so
switching to Comments while on conversation A and then switching
to conversation B would, on returning to Chat, restore A's
scrollTop instead of starting fresh at the bottom.

Reset the ref alongside didInitialScrollRef when activeConversationId
changes. Added a third test covering the cross-conversation case.

* fix(web): scroll new conversation to its bottom when conv switch happened off-tab

When activeConversationId changed while the user was on the Comments
tab, the conversation-reset effect cleared didInitialScrollRef and
the saved scroll ref, but the initial-bottom-scroll effect couldn't
do anything because logRef.current was null. Returning to Chat then
left the new conversation at scrollTop: 0 instead of its initial
bottom.

Add `tab` to the initial-scroll deps so the effect re-runs when the
chat-log remounts, picks up the cleared didInitialScrollRef state,
and scrolls the fresh conversation to its scrollHeight.

Updated the cross-conversation test to assert the new conversation
lands at its bottom (1000), not at scrollTop: 0.

* fix(web): resync jump-to-latest button when restoring saved chat scroll position

The rAF restore branch wrote scrollTop but never refreshed
scrolledFromBottom, so a user who left Chat ~60px from the bottom
and returned to find new messages stacked underneath would land
hundreds of pixels above the latest turn while the jump-to-latest
button stayed hidden until they manually scrolled.

Recompute the distance and update scrolledFromBottom inside the
restore rAF, mirroring what onScroll already does. Adds a test that
asserts the jump-to-latest button is visible immediately after a
non-pinned restore over a grown scrollHeight.

---------

Co-authored-by: Nagendhra <nagendhra405@gmail.com>

apps/web/src/components/ChatPane.tsx

@@ -162,6 +162,9 @@ export function ChatPane({
 
   useEffect(() => {
     didInitialScrollRef.current = false;
+    // A new conversation should land at the bottom (its own initial
+    // scroll), not inherit the previous conversation's saved position.
+    savedChatScrollRef.current = null;
   }, [activeConversationId]);
 
   useEffect(() => {
@@ -172,7 +175,13 @@ export function ChatPane({
       el.scrollTop = el.scrollHeight;
       setScrolledFromBottom(false);
     });
-  }, [activeConversationId, messages.length]);
+    // `tab` is in the deps so that switching conversations while
+    // Comments is open doesn't strand the new conversation at scrollTop:
+    // 0. The activeConversationId-reset effect above clears
+    // didInitialScrollRef while the chat-log is unmounted; this effect
+    // then re-runs when the user returns to Chat and the element is
+    // available, scrolling the new conversation to its initial bottom.
+  }, [activeConversationId, messages.length, tab]);
 
   useEffect(() => {
     const el = logRef.current;
@@ -186,19 +195,72 @@ export function ChatPane({
     }
   }, [messages, error]);
 
+  // Saved chat-log scroll state, preserved across tab switches. The
+  // chat-log <div> is conditionally rendered so it unmounts when the
+  // user switches to Comments. On remount it would default to
+  // scrollTop: 0 and the initial-bottom-scroll effect skips because
+  // didInitialScrollRef is already true. We capture either the absolute
+  // scrollTop or a "pinned to bottom" flag while Chat is visible, so
+  // bottom-followers stay pinned even when new messages stream in
+  // off-tab. Issue #790.
+  const savedChatScrollRef = useRef<
+    { pinnedToBottom: true } | { pinnedToBottom: false; scrollTop: number } | null
+  >(null);
   useEffect(() => {
+    if (tab !== 'chat') return;
     const el = logRef.current;
     if (!el) return;
+
+    // Restore previously-saved position on remount. Defer to the next
+    // frame so the conditional <> contents finish layout before the
+    // scrollTop write lands.
+    const saved = savedChatScrollRef.current;
+    if (saved !== null) {
+      requestAnimationFrame(() => {
+        const target = logRef.current;
+        if (!target) return;
+        if (saved.pinnedToBottom) {
+          target.scrollTop = target.scrollHeight;
+        } else {
+          target.scrollTop = saved.scrollTop;
+        }
+        // Resync the jump-to-latest affordance with the restored
+        // position. Without this, a user who left Chat ~60px from the
+        // bottom and returns to find new messages stacked underneath
+        // would land hundreds of pixels above the latest turn while
+        // scrolledFromBottom remained false until they scrolled.
+        const distance =
+          target.scrollHeight - target.scrollTop - target.clientHeight;
+        setScrolledFromBottom(distance > 120);
+      });
+    }
+
+    function snapshot(target: HTMLDivElement) {
+      const distance =
+        target.scrollHeight - target.scrollTop - target.clientHeight;
+      savedChatScrollRef.current =
+        distance < 50
+          ? { pinnedToBottom: true }
+          : { pinnedToBottom: false, scrollTop: target.scrollTop };
+    }
+
     function onScroll() {
       const target = logRef.current;
       if (!target) return;
+      snapshot(target);
       const distance =
         target.scrollHeight - target.scrollTop - target.clientHeight;
       setScrolledFromBottom(distance > 120);
     }
     el.addEventListener('scroll', onScroll);
-    return () => el.removeEventListener('scroll', onScroll);
-  }, []);
+    return () => {
+      // Capture final scroll state before unmount; the ref normally
+      // tracks via onScroll, but programmatic scrolls or layout shifts
+      // right before unmount can leave it stale.
+      snapshot(el);
+      el.removeEventListener('scroll', onScroll);
+    };
+  }, [tab]);
 
   // Close the conversation history dropdown on outside click / Escape.
   useEffect(() => {

apps/web/tests/components/chat-scroll-preservation.test.tsx

@@ -0,0 +1,263 @@
+// @vitest-environment jsdom
+
+import { act, cleanup, fireEvent, render, screen } from '@testing-library/react';
+import { afterEach, describe, expect, it } from 'vitest';
+import { ChatPane } from '../../src/components/ChatPane';
+import type { ChatMessage } from '../../src/types';
+
+// jsdom does not run a layout engine, so scrollHeight, clientHeight, and
+// scrollTop are zero by default. The scroll-preservation effect derives
+// "near bottom" from those, so we drive them explicitly per test.
+function mockScrollGeometry(
+  el: HTMLElement,
+  geom: { scrollHeight: number; clientHeight: number; scrollTop: number },
+) {
+  Object.defineProperty(el, 'scrollHeight', {
+    configurable: true,
+    get: () => geom.scrollHeight,
+  });
+  Object.defineProperty(el, 'clientHeight', {
+    configurable: true,
+    get: () => geom.clientHeight,
+  });
+  let scrollTop = geom.scrollTop;
+  Object.defineProperty(el, 'scrollTop', {
+    configurable: true,
+    get: () => scrollTop,
+    set: (v: number) => {
+      scrollTop = v;
+    },
+  });
+  return {
+    setScrollTop(v: number) {
+      scrollTop = v;
+      fireEvent.scroll(el);
+    },
+    setScrollHeight(v: number) {
+      Object.defineProperty(el, 'scrollHeight', {
+        configurable: true,
+        get: () => v,
+      });
+      geom.scrollHeight = v;
+    },
+    getScrollTop() {
+      return scrollTop;
+    },
+  };
+}
+
+function chatPaneEl(messages: ChatMessage[], activeConversationId: string | null) {
+  return (
+    <ChatPane
+      messages={messages}
+      streaming={false}
+      error={null}
+      projectId="project-1"
+      projectFiles={[]}
+      onEnsureProject={async () => 'project-1'}
+      onSend={() => {}}
+      onStop={() => {}}
+      conversations={[]}
+      activeConversationId={activeConversationId}
+      onSelectConversation={() => {}}
+      onDeleteConversation={() => {}}
+    />
+  );
+}
+
+function renderChatPane(messages: ChatMessage[], activeConversationId: string | null = null) {
+  return render(chatPaneEl(messages, activeConversationId));
+}
+
+const sampleMessages: ChatMessage[] = [
+  { id: 'u1', role: 'user', content: 'first request', createdAt: Date.now() },
+  { id: 'a1', role: 'assistant', content: 'first reply', createdAt: Date.now() },
+  { id: 'u2', role: 'user', content: 'second request', createdAt: Date.now() },
+  { id: 'a2', role: 'assistant', content: 'second reply', createdAt: Date.now() },
+];
+
+function getChatLog(): HTMLElement {
+  const el = document.querySelector('.chat-log');
+  if (!el) throw new Error('chat-log not found');
+  return el as HTMLElement;
+}
+
+function flushFrame() {
+  return act(async () => {
+    await new Promise((resolve) => requestAnimationFrame(() => resolve(null)));
+  });
+}
+
+async function switchTab(name: 'Chat' | 'Comments') {
+  const tab = screen.getByRole('tab', { name });
+  await act(async () => {
+    fireEvent.click(tab);
+  });
+}
+
+describe('chat scroll preservation across tab switches', () => {
+  afterEach(() => {
+    cleanup();
+  });
+
+  it('restores absolute scrollTop when user was scrolled up', async () => {
+    renderChatPane(sampleMessages);
+    const log = getChatLog();
+    const ctl = mockScrollGeometry(log, {
+      scrollHeight: 1000,
+      clientHeight: 400,
+      scrollTop: 0,
+    });
+
+    // User scrolls up to 200 (well above bottom: distance=400).
+    ctl.setScrollTop(200);
+
+    await switchTab('Comments');
+    await switchTab('Chat');
+    await flushFrame();
+
+    const restored = getChatLog();
+    expect(restored.scrollTop).toBe(200);
+  });
+
+  it('snaps to new scrollHeight when user was pinned to bottom and new content arrived off-tab', async () => {
+    renderChatPane(sampleMessages);
+    const log = getChatLog();
+    const ctl = mockScrollGeometry(log, {
+      scrollHeight: 1000,
+      clientHeight: 400,
+      scrollTop: 600,
+    });
+
+    // User is pinned at bottom (distance = 1000 - 600 - 400 = 0 < 50).
+    ctl.setScrollTop(600);
+
+    await switchTab('Comments');
+
+    // While off-tab, new messages would normally grow scrollHeight. We
+    // simulate that, then re-render so the chat-log remounts at the new
+    // size.
+    ctl.setScrollHeight(1500);
+
+    await switchTab('Chat');
+
+    // Re-mount picks up a fresh element; carry the new geometry into it.
+    const remounted = getChatLog();
+    Object.defineProperty(remounted, 'scrollHeight', {
+      configurable: true,
+      get: () => 1500,
+    });
+    Object.defineProperty(remounted, 'clientHeight', {
+      configurable: true,
+      get: () => 400,
+    });
+    let remountedTop = 0;
+    Object.defineProperty(remounted, 'scrollTop', {
+      configurable: true,
+      get: () => remountedTop,
+      set: (v: number) => {
+        remountedTop = v;
+      },
+    });
+
+    await flushFrame();
+
+    // Bottom-pinned user lands at scrollHeight, not at the old offset.
+    expect(remountedTop).toBe(1500);
+  });
+
+  it('reveals the jump-to-latest button when restored position is no longer near bottom', async () => {
+    renderChatPane(sampleMessages);
+    const log = getChatLog();
+    const ctl = mockScrollGeometry(log, {
+      scrollHeight: 1000,
+      clientHeight: 400,
+      scrollTop: 0,
+    });
+
+    // User leaves Chat ~60px from the bottom (distance = 1000 - 540 - 400 = 60).
+    ctl.setScrollTop(540);
+
+    await switchTab('Comments');
+
+    // While off-tab, new messages stack underneath. scrollHeight grows
+    // dramatically; the saved absolute scrollTop is now hundreds of
+    // pixels above the latest turn.
+    ctl.setScrollHeight(2000);
+
+    await switchTab('Chat');
+
+    // Carry the new geometry into the remounted element so the
+    // distance-from-bottom calc inside the rAF restore can see it.
+    const remounted = getChatLog();
+    let remountedTop = 0;
+    Object.defineProperty(remounted, 'scrollHeight', {
+      configurable: true,
+      get: () => 2000,
+    });
+    Object.defineProperty(remounted, 'clientHeight', {
+      configurable: true,
+      get: () => 400,
+    });
+    Object.defineProperty(remounted, 'scrollTop', {
+      configurable: true,
+      get: () => remountedTop,
+      set: (v: number) => {
+        remountedTop = v;
+      },
+    });
+
+    await flushFrame();
+
+    // Restored to old offset (540), but distance = 2000 - 540 - 400 = 1060
+    // is well past the 120px threshold, so the jump-to-latest button
+    // must be visible immediately, not hidden until the user scrolls.
+    expect(remountedTop).toBe(540);
+    expect(screen.getByRole('button', { name: /jump to latest/i })).toBeTruthy();
+  });
+
+  it('lands new conversation at its own bottom when switching conversations off-tab', async () => {
+    const { rerender } = render(chatPaneEl(sampleMessages, 'conv-A'));
+    const log = getChatLog();
+    const ctl = mockScrollGeometry(log, {
+      scrollHeight: 1000,
+      clientHeight: 400,
+      scrollTop: 0,
+    });
+
+    // User scrolls up in conversation A and switches to Comments.
+    ctl.setScrollTop(150);
+    await switchTab('Comments');
+
+    // While off-tab the active conversation changes to B. Returning to
+    // Chat must land at conversation B's own initial bottom, not at
+    // scrollTop: 0 and not at conversation A's saved offset.
+    rerender(chatPaneEl(sampleMessages, 'conv-B'));
+    await switchTab('Chat');
+
+    const remounted = getChatLog();
+    let remountedTop = 0;
+    Object.defineProperty(remounted, 'scrollHeight', {
+      configurable: true,
+      get: () => 1000,
+    });
+    Object.defineProperty(remounted, 'clientHeight', {
+      configurable: true,
+      get: () => 400,
+    });
+    Object.defineProperty(remounted, 'scrollTop', {
+      configurable: true,
+      get: () => remountedTop,
+      set: (v: number) => {
+        remountedTop = v;
+      },
+    });
+
+    await flushFrame();
+
+    // Saved state was cleared and the initial-bottom-scroll effect
+    // re-runs with `tab` in its deps, so the new conversation lands at
+    // its own scrollHeight rather than the browser default 0.
+    expect(remountedTop).toBe(1000);
+  });
+});