diff --git a/src/renderer/src/components/native-chat/NativeChatMessageList.tsx b/src/renderer/src/components/native-chat/NativeChatMessageList.tsx index 108a188c4ab..f7462e07434 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageList.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.tsx @@ -256,10 +256,7 @@ export function NativeChatMessageList({ // Named so measurement can find the scroll root without depending on // which utility class happens to make it scroll. data-native-chat-scroll - // `overflow-anchor:none`: the transcript decides whether an offset - // it did not write is the reader moving, so the engine adjusting - // scrollTop under a settling row would read as a departure. The - // virtualizer does its own end anchoring, so this is redundant here. + // Browser anchoring would add unattributed movement beside the virtualizer's anchor. className="scrollbar-sleek relative h-full overflow-y-auto [overflow-anchor:none] [scrollbar-gutter:stable_both-edges]" // Why: `zoom` scales the chat transcript's text and layout together, // scoped to this pane so the rest of the app is untouched. It sits on diff --git a/src/renderer/src/components/native-chat/NativeChatMessageList.windowing.test.tsx b/src/renderer/src/components/native-chat/NativeChatMessageList.windowing.test.tsx index 37eed607a02..e4debc8e676 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageList.windowing.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.windowing.test.tsx @@ -472,11 +472,8 @@ describe('transcript with a hidden scroll root', () => { // arrive at their final height and are a different case; this is the one where // the row the reader is looking at keeps changing size underneath them. // -// Two mechanisms are supposed to hold the pin, and both are exercised here: the -// list's own resize observer on the transcript column (which re-runs -// `scrollToBottom` against the document) and the virtualizer's end anchor (which -// compensates `scrollTop` by the growth when the view was already at the end). -describe('a row growing in place while the view is pinned to the bottom', () => { +// Exercise the real virtualizer together with the transcript's follow owner. +describe('transcript follow ownership across growth and appends', () => { const TAIL_INDEX = TRANSCRIPT_LENGTH - 1 const GROWTH_STEPS = 24 const LINES_PER_STEP = 12 @@ -493,6 +490,13 @@ describe('a row growing in place while the view is pinned to the bottom', () => const transcript = Array.from({ length: TRANSCRIPT_LENGTH }, (_, index) => marker(index)) + function appendedTranscript(count: number): NativeChatMessage[] { + return [ + ...transcript, + ...Array.from({ length: count }, (_, index) => marker(TRANSCRIPT_LENGTH + index)) + ] + } + function tailHeightAt(step: number): number { return Math.max(ROW_PX, (1 + step * LINES_PER_STEP) * STREAM_LINE_PX) } @@ -645,50 +649,170 @@ describe('a row growing in place while the view is pinned to the bottom', () => expect(screen.getByRole('button', { name: /jump to latest/i })).toBeInTheDocument() }) - // The same refusal to drag a reader along, for one who only stepped off the - // end far enough to read the line above. That step is smaller than the pin's - // own slack, so the windowing layer still settles the offset once as the first - // growth lands; what must not happen is the transcript following from there. - it('stops following a reader who parked just above the latest message', () => { + it.each([0, 100])( + 'keeps a reader parked above a growing row with a %i px initial measurement delta', + (measurementDelta) => { + setMeasuredTail(4) + measuredRowHeights = measuredRowHeights.map((height, index) => + index === TAIL_INDEX ? height + measurementDelta : height + ) + const { container, rerender } = render(streamingList(4)) + paint(container) + const scroller = scrollRoot(container) + + const parkGapPx = NATIVE_CHAT_BOTTOM_THRESHOLD_PX - 8 + const parkedAt = scroller.scrollHeight - scroller.clientHeight - parkGapPx + scrollTranscript(container, parkedAt) + expect(distanceFromBottom(container)).toBe(parkGapPx) + // Not the "scrolled far away" case above: the latest message is still on + // screen, so there is nothing to offer a way back to yet. + expect(screen.queryByRole('button', { name: /jump to latest/i })).toBeNull() + + setMeasuredTail(5) + rerender(streamingList(5)) + paint(container) + expect(scroller.scrollTop).toBe(parkedAt) + + let previousDistance = distanceFromBottom(container) + for (let step = 6; step <= GROWTH_STEPS; step += 1) { + setMeasuredTail(step) + rerender(streamingList(step)) + paint(container) + + // The offset stops moving at all... + expect(scroller.scrollTop).toBe(parkedAt) + // ...so the end runs away from the reader instead of carrying them along. + const distance = distanceFromBottom(container) + expect(distance).toBeGreaterThan(previousDistance) + previousDistance = distance + } + + expect(previousDistance).toBeGreaterThan(VIEWPORT_PX) + expect(screen.getByRole('button', { name: /jump to latest/i })).toBeInTheDocument() + } + ) + + it('leaves a parked reader in place through repeated appends', () => { + const { container, rerender } = render(list(transcript)) + paint(container) + const scroller = scrollRoot(container) + const parkedAt = scroller.scrollHeight - scroller.clientHeight - 40 + scrollTranscript(container, parkedAt) + + for (let count = 1; count <= 8; count += 1) { + rerender(list(appendedTranscript(count))) + paint(container) + expect(scroller.scrollTop).toBe(parkedAt) + expect(windowState(container).indexes.length).toBeLessThan(TRANSCRIPT_LENGTH / 4) + } + expect(screen.getByRole('button', { name: /jump to latest/i })).toBeInTheDocument() + }) + + it('follows repeated appends until the reader detaches', () => { + const { container, rerender } = render(list(transcript)) + paint(container) + const scroller = scrollRoot(container) + for (let count = 1; count <= 8; count += 1) { + rerender(list(appendedTranscript(count))) + paint(container) + expect(distanceFromBottom(container)).toBeLessThanOrEqual(NATIVE_CHAT_FOLLOW_REARM_PX) + fireEvent.scroll(scroller) + } + + const parkedAt = scroller.scrollTop - 22 + scrollTranscript(container, parkedAt) + rerender(list(appendedTranscript(9))) + paint(container) + expect(scroller.scrollTop).toBe(parkedAt) + }) + + it('follows an empty transcript through underflow into scrollable output', () => { + const { container, rerender } = render(list([])) + paint(container) + expect(scrollRoot(container).scrollTop).toBe(0) + rerender(list(transcript.slice(0, 1))) + paint(container) + expect(scrollRoot(container).scrollTop).toBe(0) + fireEvent.scroll(scrollRoot(container)) + rerender(list(transcript)) + paint(container) + expect(distanceFromBottom(container)).toBeLessThanOrEqual(NATIVE_CHAT_FOLLOW_REARM_PX) + expect(windowState(container).indexes.length).toBeLessThan(TRANSCRIPT_LENGTH / 4) + }) + + it.each(['reader', 'jump'] as const)('rearms growth and append following via %s', (rearm) => { setMeasuredTail(4) const { container, rerender } = render(streamingList(4)) paint(container) const scroller = scrollRoot(container) - - const parkGapPx = NATIVE_CHAT_BOTTOM_THRESHOLD_PX - 8 - expect(parkGapPx).toBeGreaterThan(NATIVE_CHAT_FOLLOW_REARM_PX) - const parkedAt = scroller.scrollHeight - scroller.clientHeight - parkGapPx + fireEvent.scroll(scroller) + const parkedAt = scroller.scrollTop - 22 scrollTranscript(container, parkedAt) - expect(distanceFromBottom(container)).toBe(parkGapPx) - // Not the "scrolled far away" case above: the latest message is still on - // screen, so there is nothing to offer a way back to yet. - expect(screen.queryByRole('button', { name: /jump to latest/i })).toBeNull() - setMeasuredTail(5) rerender(streamingList(5)) paint(container) - // Bounded by the gap itself. Following would have tracked the whole growth. - const settledAt = scroller.scrollTop - expect(settledAt).toBeLessThanOrEqual(parkedAt + parkGapPx) + expect(scroller.scrollTop).toBe(parkedAt) - let previousDistance = distanceFromBottom(container) - for (let step = 6; step <= GROWTH_STEPS; step += 1) { + if (rearm === 'reader') { + scrollTranscript( + container, + scroller.scrollHeight - scroller.clientHeight - NATIVE_CHAT_FOLLOW_REARM_PX + ) + } else { + fireEvent.click(screen.getByRole('button', { name: /jump to latest/i })) + } + paint(container) + expect(screen.queryByRole('button', { name: /jump to latest/i })).toBeNull() + for (let step = 6; step <= 8; step += 1) { setMeasuredTail(step) rerender(streamingList(step)) paint(container) - - // The offset stops moving at all... - expect(scroller.scrollTop).toBe(settledAt) - // ...so the end runs away from the reader instead of carrying them along. - const distance = distanceFromBottom(container) - expect(distance).toBeGreaterThan(previousDistance) - previousDistance = distance + expect(distanceFromBottom(container)).toBeLessThanOrEqual(NATIVE_CHAT_FOLLOW_REARM_PX) + expect(windowState(container).indexes.length).toBeLessThan(TRANSCRIPT_LENGTH / 4) } + rerender(list([...transcriptAt(8), marker(TRANSCRIPT_LENGTH)])) + paint(container) + expect(distanceFromBottom(container)).toBeLessThanOrEqual(NATIVE_CHAT_FOLLOW_REARM_PX) + }) - expect(previousDistance).toBeGreaterThan(VIEWPORT_PX) + it('preserves the visible row anchor across prepends while detached', () => { + const { container, rerender } = render(list(transcript)) + paint(container) + const readingAt = 2000 + scrollTranscript(container, readingAt) + paint(container) + + const earlier = Array.from({ length: 10 }, (_, index) => marker(index - 10)) + rerender(list([...earlier, ...transcript])) + paint(container) + expect(scrollRoot(container).scrollTop).toBe(readingAt + earlier.length * ROW_PITCH_PX) + expect(windowState(container).indexes.length).toBeLessThan(TRANSCRIPT_LENGTH / 4) expect(screen.getByRole('button', { name: /jump to latest/i })).toBeInTheDocument() }) + it('compensates a measurement entirely above the viewport without reattaching', () => { + const { container, rerender } = render(list(transcript)) + paint(container) + const scroller = scrollRoot(container) + fireEvent.scroll(scroller) + const readingAt = 2000 + scrollTranscript(container, readingAt) + paint(container) + const aboveIndex = windowState(container).indexes[0]! + expect((aboveIndex + 1) * ROW_PITCH_PX).toBeLessThan(readingAt) + for (const growth of [100, 200]) { + measuredRowHeights = Array.from({ length: TRANSCRIPT_LENGTH }, (_, index) => + index === aboveIndex ? ROW_PX + growth : ROW_PX + ) + paint(container) + expect(scroller.scrollTop).toBe(readingAt + growth) + } + rerender(list(appendedTranscript(1))) + paint(container) + expect(scroller.scrollTop).toBe(readingAt + 200) + expect(windowState(container).indexes.length).toBeLessThan(TRANSCRIPT_LENGTH / 4) + }) + it('keeps following when a pin echo arrives after the document grows', () => { setMeasuredTail(0) const { container } = render(streamingList(0)) @@ -704,6 +828,8 @@ describe('a row growing in place while the view is pinned to the bottom', () => expect(scroller.scrollTop).toBe(pinnedAt) expect(screen.queryByRole('button', { name: /jump to latest/i })).toBeNull() + paint(container) + expect(distanceFromBottom(container)).toBeLessThanOrEqual(NATIVE_CHAT_FOLLOW_REARM_PX) }) it('settles a pending end reconcile after the reader keeps scrolling away', async () => { diff --git a/src/renderer/src/components/native-chat/native-chat-autoscroll.test.ts b/src/renderer/src/components/native-chat/native-chat-autoscroll.test.ts index 023e18571e4..0dc34b7eb30 100644 --- a/src/renderer/src/components/native-chat/native-chat-autoscroll.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-autoscroll.test.ts @@ -76,20 +76,20 @@ describe('nextFollowingEnd', () => { expect(nextFollowingEnd({ ...following, geometry: wellAway })).toBe(false) }) - it('does not re-attach a detached reader from an application write', () => { - expect(nextFollowingEnd({ following: false, programmatic: true, geometry: wellAway })).toBe( - false - ) - }) + it.each([0, NATIVE_CHAT_FOLLOW_REARM_PX, 400])( + 'does not reattach a detached reader from an application write %i px from the end', + (distance) => { + expect( + nextFollowingEnd({ following: false, programmatic: true, geometry: parkedAbove(distance) }) + ).toBe(false) + } + ) - // The park bug. The near-bottom band is slack for a streaming row's jitter; - // re-arming on it meant every step off the end smaller than that slack still - // counted as following, so the next chunk of stream took the reader back down. + // The jump affordance's wider band must not decide whether a reader follows. it('lets the reader park just inside the near-bottom band', () => { expect(NATIVE_CHAT_FOLLOW_REARM_PX).toBeLessThan(NATIVE_CHAT_BOTTOM_THRESHOLD_PX) const parked = parkedAbove(NATIVE_CHAT_BOTTOM_THRESHOLD_PX - 1) expect(nextFollowingEnd({ ...following, geometry: parked })).toBe(false) - // The pin and the affordance keep their own, wider band: only follow moved. expect(isNearBottom(parked)).toBe(true) expect(shouldShowJumpToLatest(false, parked)).toBe(false) }) diff --git a/src/renderer/src/components/native-chat/native-chat-autoscroll.ts b/src/renderer/src/components/native-chat/native-chat-autoscroll.ts index 0065686fcab..a8f2e54b22f 100644 --- a/src/renderer/src/components/native-chat/native-chat-autoscroll.ts +++ b/src/renderer/src/components/native-chat/native-chat-autoscroll.ts @@ -11,11 +11,7 @@ export type ScrollGeometry = { clientHeight: number } -/** Pixels from the bottom within which we treat the view as "at the bottom" and - * keep it pinned as content arrives. A small slack absorbs sub-pixel rounding - * and the height jitter of a streaming last message. Whether a reader's own - * scroll leaves us following is a separate, stricter question — see - * `NATIVE_CHAT_FOLLOW_REARM_PX`. */ +/** Hide the jump affordance while the latest output is still nearby. */ export const NATIVE_CHAT_BOTTOM_THRESHOLD_PX = 48 /** Distance in px from the bottom edge of the scroll range. */ @@ -23,8 +19,7 @@ export function distanceFromBottom(geometry: ScrollGeometry): number { return Math.max(0, geometry.scrollHeight - geometry.clientHeight - geometry.scrollTop) } -/** True when the viewport is close enough to the bottom that new content should - * keep it pinned (auto-scroll "attached"). */ +/** Whether the viewport is inside the requested distance from the bottom. */ export function isNearBottom( geometry: ScrollGeometry, threshold: number = NATIVE_CHAT_BOTTOM_THRESHOLD_PX @@ -45,20 +40,7 @@ export function shouldShowJumpToLatest( return distanceFromBottom(geometry) > threshold } -/** Pixels from the bottom within which a reader's own scroll still counts as - * following the end. - * - * Deliberately far stricter than `NATIVE_CHAT_BOTTOM_THRESHOLD_PX`, because the - * two answer different questions: that one asks whether the view is close - * enough to the end to keep pinning it there, this one asks whether the reader - * meant to stay. Sharing the wider band meant any step off the end smaller than - * it still counted as following, so every later chunk of stream carried the - * reader back down and there was no way to park just above the latest message. - * - * Sized to cover only what is not a decision: fractional-pixel and zoom - * rounding at the true end, at twice the epsilon the scroll marks already treat - * as offset noise, and well inside one line of prose so scrolling up a single - * line parks. */ +/** Allow bottom rounding noise without following a reader who moved up a line. */ export const NATIVE_CHAT_FOLLOW_REARM_PX = 4 export type FollowIntent = { diff --git a/src/renderer/src/components/native-chat/use-native-chat-transcript-window.options.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-transcript-window.options.test.tsx index da19a51a9c3..82166952670 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-transcript-window.options.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-transcript-window.options.test.tsx @@ -67,7 +67,7 @@ afterEach(() => { }) describe('native chat transcript virtualizer contract', () => { - it('configures prepend anchoring and matching bottom-follow behavior', () => { + it('retains prepend anchoring without independently following the end', () => { renderHook(() => useNativeChatTranscriptWindow({ scrollRef: { current: null }, @@ -78,8 +78,8 @@ describe('native chat transcript virtualizer contract', () => { expect(virtualizerMock.options.current).toMatchObject({ anchorTo: 'end', - followOnAppend: true, - scrollEndThreshold: 48 + followOnAppend: false, + scrollEndThreshold: -1 }) }) diff --git a/src/renderer/src/components/native-chat/use-native-chat-transcript-window.ts b/src/renderer/src/components/native-chat/use-native-chat-transcript-window.ts index d7cbde467df..30227bf47c5 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-transcript-window.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-transcript-window.ts @@ -1,14 +1,8 @@ // DOM windowing for the transcript: only the rows near the viewport are mounted, // the rest are reserved as estimated height. // -// Anchoring is the library's, not ours. `anchorTo: 'end'` captures the row at the -// current offset before a count change and re-resolves its position afterwards, -// which is what keeps a "load earlier" prepend from yanking the view; -// `followOnAppend` + `scrollEndThreshold` keep a reader who is already at the -// bottom pinned there as a turn streams. That pair decides on geometry alone, -// with no reader-intent flag behind it, so it can still settle the offset once -// for a reader parked inside its threshold; what stops them being followed from -// there on is the transcript's own follow state, not this option. +// The virtualizer owns visible-row anchoring; the transcript scroll hook owns +// end-follow intent. Geometry alone must never reattach a parked reader. // // Every measurement here ends up in the scroll container's own coordinate space, // which means `offsetTop` / `offsetHeight` rather than a bounding rect. The @@ -19,7 +13,6 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { elementScroll, useVirtualizer, type VirtualItem } from '@tanstack/react-virtual' import { createProgrammaticScrollMarks } from '@/hooks/programmatic-scroll-marks' -import { NATIVE_CHAT_BOTTOM_THRESHOLD_PX } from './native-chat-autoscroll' import { NATIVE_CHAT_ROW_GAP_PX } from './native-chat-row-height-estimate' import { nativeChatPinnedRowIndexes, nativeChatTranscriptRange } from './native-chat-pinned-rows' import type { NativeChatTranscriptSlot } from './native-chat-transcript-slots' @@ -138,8 +131,9 @@ export function useNativeChatTranscriptWindow({ gap: NATIVE_CHAT_ROW_GAP_PX, scrollMargin, anchorTo: 'end', - followOnAppend: true, - scrollEndThreshold: NATIVE_CHAT_BOTTOM_THRESHOLD_PX, + followOnAppend: false, + // Distances are nonnegative: disable geometry-only resize pinning, retaining prepend anchoring. + scrollEndThreshold: -1, // Every virtualizer write uses this public adapter, including measurement // adjustments and prepend anchoring, so scroll events have one provenance. scrollToFn: (offset, options, instance) => { @@ -167,6 +161,10 @@ export function useNativeChatTranscriptWindow({ } }) + // Growing a row that spans the viewport changes content below the reader's anchor. + virtualizer.shouldAdjustScrollPositionOnItemSizeChange = (item, _delta, instance) => + item.end <= (instance.scrollOffset ?? 0) + const finishReaderTakeover = useCallback(() => { if (readerTakeoverFrameRef.current !== null) { window.cancelAnimationFrame(readerTakeoverFrameRef.current)