From e2ac337780a24f0fab810daa89200eeff80ba0d3 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 12 May 2026 16:22:11 -0500 Subject: [PATCH] =?UTF-8?q?fix(hud):=20chat=20panel=20auto-scrolls=20on=20?= =?UTF-8?q?streaming=20token=20append=20=E2=80=94=20closes=20#51?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ChatPanel now pins to the bottom as streaming tokens arrive, so the user no longer has to manually scroll while the agent is speaking (B-06). Heuristic: - Compute a content signal = total char-length across all chat events. Token deltas append to the same text part (stable event id), so a length-based signal — not array-length — is what changes on every streamed chunk. - A useLayoutEffect keyed on that signal re-measures distanceFromBottom at the moment the content mutates. If the user is within 24 px of the bottom we scroll to scrollHeight via scrollTo({ behavior: 'smooth' }). - If the user scrolled up to re-read, the pin is broken (distance > 24 px) and we leave them alone. The pin auto-re-arms the next time they scroll back to the bottom — no event listener required, since we re-measure DOM on every content change. - scrollTo is feature-detected (JSDOM lacks it; fall back to direct scrollTop assignment so production never throws). Scope: tactical, pure-DOM. The full fix lands in M-7 (v0.8) once the API surface emits turnTextComplete and the HUD hydrates from Turn.listTurns; this is the consumer-side scroll behavior that fits in v0.1. Tests: 4 new vitest cases in ChatPanel.autoscroll.test.tsx covering: - AS1: new event when pinned -> scrollTo top=scrollHeight. - AS2: streaming token append (same event id) -> scrollTo per token. - AS3: user scrolled up -> no scrollTo. - AS4: user scrolled back to bottom -> next append resumes scroll. DOM-layout properties (scrollHeight/clientHeight/scrollTop) are stubbed via Object.defineProperty because JSDOM does not lay out. Empirical pixel-level scroll verification is deferred to the M-7 typed-API work per the issue's own scope note. Co-Authored-By: Claude Opus 4.7 --- App/Resources/webview/index.html | 2 +- webview/packages/hud/src/chat/ChatPanel.tsx | 79 +++++++ .../hud/tests/ChatPanel.autoscroll.test.tsx | 213 ++++++++++++++++++ 3 files changed, 293 insertions(+), 1 deletion(-) create mode 100644 webview/packages/hud/tests/ChatPanel.autoscroll.test.tsx diff --git a/App/Resources/webview/index.html b/App/Resources/webview/index.html index 3526801..68b478e 100644 --- a/App/Resources/webview/index.html +++ b/App/Resources/webview/index.html @@ -5,7 +5,7 @@ Jarvis HUD - +
diff --git a/webview/packages/hud/src/chat/ChatPanel.tsx b/webview/packages/hud/src/chat/ChatPanel.tsx index a56aad8..e13617c 100644 --- a/webview/packages/hud/src/chat/ChatPanel.tsx +++ b/webview/packages/hud/src/chat/ChatPanel.tsx @@ -1,19 +1,98 @@ +import { useLayoutEffect, useRef } from 'react' import { useJarvisStore } from '../store' import { ChatEventRouter } from './ChatEventRouter' import './chat-panel.css' +/** + * Hysteresis (in px) for the "is user at the bottom?" check. As long as the + * scroll position is within this distance of the bottom edge, the panel is + * considered pinned and we auto-scroll on every new token. A small value is + * deliberate: streaming tokens grow scrollHeight by ~one line at a time, and + * we want the very first user-initiated scroll-up to break the pin. + */ +const BOTTOM_THRESHOLD_PX = 24 + +/** + * Sum the text length of all chat events. This is the streaming-progress + * signal the auto-scroll effect depends on. Using char-count means the + * effect re-fires on every appended token even though `chatEvents.length` + * stays constant during streaming (tokens append to the same text part). + * + * Cheap to compute (O(n) over chat events, each typically a short string) + * and avoids inventing a new bus message just to drive scroll. + */ +function totalContentSignal( + events: ReturnType['chatEvents'], +): number { + let total = events.length // event count contributes too (non-text events) + for (const ev of events) { + if (ev.kind === 'text') total += ev.text.length + else if (ev.kind === 'tool-call') { + // Status changes (running -> completed/failed) and result text growth + // should also trigger a scroll-pin refresh. `result` is typed + // `unknown` (tool outputs are arbitrary JSON), so coerce to string. + total += ev.status.length + if (ev.result !== undefined) total += String(ev.result).length + if (ev.error) total += ev.error.length + } else if (ev.kind === 'error') { + total += ev.message.length + } + } + return total +} + /** * ChatPanel — flat, chronological list of ChatEvents. role="log" + * aria-live="polite" announces streamed content to screen readers. * + * Auto-scroll behavior (B-06 / #51 tactical fix): + * - When new content arrives (new event OR a token appended to an + * existing text part), scroll to the bottom — but only if the user + * is already pinned within BOTTOM_THRESHOLD_PX of the bottom edge. + * - If the user has scrolled up to re-read, leave them alone. The + * pin auto-re-arms as soon as they scroll back down. + * + * The full fix lands in M-7 (v0.8) once the API surface emits + * `turnTextComplete` and we hydrate from `Turn.listTurns`; this is the + * pure-DOM tactical layer that fits in v0.1. + * * No virtualization in P3 (RESEARCH §A8); P8 adds `@tanstack/react-virtual` * if profiling shows need. Each child is keyed by event.id so React reuses * DOM nodes across streaming updates. */ export function ChatPanel() { const chatEvents = useJarvisStore((s) => s.chatEvents) + const panelRef = useRef(null) + + // The content signal is `totalContentSignal(chatEvents)` — a number that + // changes on every appended token, status transition, or new event. + // useLayoutEffect runs synchronously after DOM mutation but before paint, + // so the user never sees the intermediate (pre-scroll) frame. + // + // We re-measure `distanceFromBottom` at the moment the content changed + // (rather than tracking pin state in a separate scroll handler). The + // browser always exposes current scroll position via getter properties, + // so this stays accurate even as the user scrolls without our handler + // observing it. + const contentSignal = totalContentSignal(chatEvents) + useLayoutEffect(() => { + const el = panelRef.current + if (!el) return + const distanceFromBottom = el.scrollHeight - el.scrollTop - el.clientHeight + if (distanceFromBottom > BOTTOM_THRESHOLD_PX) return + // `scrollTo` is missing on JSDOM Elements (and on a few legacy + // engines). Fall back to direct scrollTop assignment so production + // never throws if the engine ships without it. + if (typeof el.scrollTo === 'function') { + el.scrollTo({ top: el.scrollHeight, behavior: 'smooth' }) + } else { + el.scrollTop = el.scrollHeight + } + }, [contentSignal]) + return (
number }, +) { + Object.defineProperty(el, 'scrollHeight', { + configurable: true, + get: () => opts.scrollHeight, + }) + Object.defineProperty(el, 'clientHeight', { + configurable: true, + get: () => opts.clientHeight, + }) + Object.defineProperty(el, 'scrollTop', { + configurable: true, + get: opts.scrollTopGetter, + set: () => { + /* mutation handled by stubScrollTo */ + }, + }) +} + +function getPanel(container: HTMLElement): HTMLElement { + const el = container.querySelector('[role="log"]') as HTMLElement | null + if (!el) throw new Error('chat-panel not found') + return el +} + +describe('ChatPanel auto-scroll on streaming append (B-06 / #51)', () => { + beforeEach(() => resetStore()) + + it('AS1: scrolls to bottom when a new chat event is appended and user is at bottom', () => { + // Start empty so we can attach stubs to the panel element before any + // streaming-driven layout queries happen. + const { container } = render() + const panel = getPanel(container) + + const scrollToSpy = vi.fn() + panel.scrollTo = scrollToSpy as unknown as typeof panel.scrollTo + + // User is at the bottom: scrollTop + clientHeight == scrollHeight. + stubLayout(panel, { + scrollHeight: 1000, + clientHeight: 200, + scrollTopGetter: () => 800, + }) + + act(() => { + useJarvisStore.setState({ + chatEvents: [ + { id: 'e1', kind: 'text', text: 'hello', role: 'assistant', turnId: 't1' }, + ] as ChatEvent[], + }) + }) + + expect(scrollToSpy).toHaveBeenCalled() + const arg = scrollToSpy.mock.calls.at(-1)?.[0] as ScrollToOptions + expect(arg.top).toBe(1000) + }) + + it('AS2: scrolls on streaming token append (same event, growing text)', () => { + const base: ChatEvent = { + id: 'turn:t1:assistant', + kind: 'text', + text: '', + role: 'assistant', + turnId: 't1', + } + act(() => { + useJarvisStore.setState({ chatEvents: [base] }) + }) + + const { container, rerender } = render() + const panel = getPanel(container) + const scrollToSpy = vi.fn() + panel.scrollTo = scrollToSpy as unknown as typeof panel.scrollTo + + // Simulate a growing scrollHeight as tokens stream in. + let scrollHeight = 1000 + Object.defineProperty(panel, 'scrollHeight', { + configurable: true, + get: () => scrollHeight, + }) + Object.defineProperty(panel, 'clientHeight', { + configurable: true, + get: () => 200, + }) + Object.defineProperty(panel, 'scrollTop', { + configurable: true, + get: () => scrollHeight - 200, // pinned to bottom + set: () => { + /* noop */ + }, + }) + + scrollToSpy.mockClear() + for (const delta of ['h', 'e', 'l', 'l', 'o']) { + scrollHeight += 20 + act(() => { + useJarvisStore.getState().appendTokenToLastText('turn:t1:assistant', delta) + }) + rerender() + } + + // Once per token append. + expect(scrollToSpy.mock.calls.length).toBeGreaterThanOrEqual(5) + const lastArg = scrollToSpy.mock.calls.at(-1)?.[0] as ScrollToOptions + expect(lastArg.top).toBe(scrollHeight) + }) + + it('AS3: does NOT scroll when user has scrolled up away from the bottom', () => { + const { container } = render() + const panel = getPanel(container) + const scrollToSpy = vi.fn() + panel.scrollTo = scrollToSpy as unknown as typeof panel.scrollTo + + // User scrolled up: 500 px below the bottom (way beyond the small + // hysteresis threshold used by the auto-scroll effect). + stubLayout(panel, { + scrollHeight: 1000, + clientHeight: 200, + scrollTopGetter: () => 300, // 1000 - 200 - 300 = 500 px from bottom + }) + + act(() => { + useJarvisStore.setState({ + chatEvents: [ + { id: 'e1', kind: 'text', text: 'noisy', role: 'assistant', turnId: 't1' }, + ] as ChatEvent[], + }) + }) + + expect(scrollToSpy).not.toHaveBeenCalled() + }) + + it('AS4: resumes auto-scroll once the user scrolls back to the bottom', () => { + const { container } = render() + const panel = getPanel(container) + const scrollToSpy = vi.fn() + panel.scrollTo = scrollToSpy as unknown as typeof panel.scrollTo + + // Start scrolled up — `scrollTop` is mutable here so we can simulate + // the user scrolling back to the bottom between content updates. + // The implementation re-measures DOM at the moment of each content + // change, so no manual `scroll` event dispatch is needed for pin + // state to update. + let scrollTop = 300 + stubLayout(panel, { + scrollHeight: 1000, + clientHeight: 200, + scrollTopGetter: () => scrollTop, + }) + + act(() => { + useJarvisStore.setState({ + chatEvents: [ + { id: 'e1', kind: 'text', text: 'first', role: 'assistant', turnId: 't1' }, + ] as ChatEvent[], + }) + }) + expect(scrollToSpy).not.toHaveBeenCalled() + + // User scrolled back to the bottom. Next content update re-measures + // distanceFromBottom = 0 and resumes auto-scroll. + scrollTop = 800 + + act(() => { + useJarvisStore.setState({ + chatEvents: [ + { id: 'e1', kind: 'text', text: 'first', role: 'assistant', turnId: 't1' }, + { id: 'e2', kind: 'text', text: 'second', role: 'assistant', turnId: 't1' }, + ] as ChatEvent[], + }) + }) + + expect(scrollToSpy).toHaveBeenCalled() + }) +}) -- 2.54.0