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 8d9f3ffa83b..e4debc8e676 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageList.windowing.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.windowing.test.tsx @@ -12,7 +12,10 @@ import { projectStructuredItemsToNativeChat } from '../../../../shared/structure import type { NativeChatMessage } from '../../../../shared/native-chat-types' import type { NativeChatLiveSession } from './use-native-chat-live-session' import { NativeChatMessageList } from './NativeChatMessageList' -import { NATIVE_CHAT_BOTTOM_THRESHOLD_PX } from './native-chat-autoscroll' +import { + NATIVE_CHAT_BOTTOM_THRESHOLD_PX, + NATIVE_CHAT_FOLLOW_REARM_PX +} from './native-chat-autoscroll' import { estimateNativeChatRowHeight, NATIVE_CHAT_ROW_GAP_PX, @@ -469,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 @@ -490,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) } @@ -642,6 +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() }) + 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) + fireEvent.scroll(scroller) + const parkedAt = scroller.scrollTop - 22 + scrollTranscript(container, parkedAt) + setMeasuredTail(5) + rerender(streamingList(5)) + paint(container) + expect(scroller.scrollTop).toBe(parkedAt) + + 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) + 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) + }) + + 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)) @@ -657,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 fa23669e931..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 @@ -5,13 +5,23 @@ import { nextFollowingEnd, shouldLoadEarlier, shouldShowJumpToLatest, - NATIVE_CHAT_BOTTOM_THRESHOLD_PX + NATIVE_CHAT_BOTTOM_THRESHOLD_PX, + NATIVE_CHAT_FOLLOW_REARM_PX } from './native-chat-autoscroll' const atBottom = { scrollTop: 952, scrollHeight: 1000, clientHeight: 48 } const scrolledUp = { scrollTop: 0, scrollHeight: 1000, clientHeight: 48 } const noOverflow = { scrollTop: 0, scrollHeight: 48, clientHeight: 48 } +/** A view parked exactly `distance` px above the end of the same document. */ +function parkedAbove(distance: number): { + scrollTop: number + scrollHeight: number + clientHeight: number +} { + return { scrollTop: 952 - distance, scrollHeight: 1000, clientHeight: 48 } +} + describe('distanceFromBottom', () => { it('is zero at the exact bottom and never negative', () => { expect(distanceFromBottom(atBottom)).toBe(0) @@ -48,7 +58,8 @@ describe('shouldShowJumpToLatest', () => { // The browser reports application writes as ordinary scroll events. Explicit // marks distinguish their delayed echoes from reader movement after growth. describe('nextFollowingEnd', () => { - const following = { following: true, programmatic: false, atEnd: true } + const following = { following: true, programmatic: false, geometry: parkedAbove(0) } + const wellAway = parkedAbove(400) it('follows when the reader reaches the end', () => { expect(nextFollowingEnd(following)).toBe(true) @@ -58,15 +69,44 @@ describe('nextFollowingEnd', () => { // the end runs away from an offset the transcript itself pinned. That is not a // reader leaving, and treating it as one strands them mid-transcript. it('keeps following when a delayed application scroll arrives after growth', () => { - expect(nextFollowingEnd({ ...following, programmatic: true, atEnd: false })).toBe(true) + expect(nextFollowingEnd({ ...following, programmatic: true, geometry: wellAway })).toBe(true) }) it('treats an unmarked offset away from the end as the reader leaving', () => { - expect(nextFollowingEnd({ ...following, atEnd: false })).toBe(false) + 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, atEnd: false })).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 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) + expect(isNearBottom(parked)).toBe(true) + expect(shouldShowJumpToLatest(false, parked)).toBe(false) + }) + + it('re-arms at the band and not one pixel past it', () => { + const detached = { following: false, programmatic: false } + expect( + nextFollowingEnd({ ...detached, geometry: parkedAbove(NATIVE_CHAT_FOLLOW_REARM_PX) }) + ).toBe(true) + expect( + nextFollowingEnd({ ...detached, geometry: parkedAbove(NATIVE_CHAT_FOLLOW_REARM_PX + 1) }) + ).toBe(false) + }) + + // Sub-pixel and zoom rounding put the true end a fraction short of exact. + it('holds follow through rounding noise at the end', () => { + expect(nextFollowingEnd({ ...following, geometry: parkedAbove(1.5) })).toBe(true) }) }) 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 f07aeeb6671..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,9 +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. */ +/** 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. */ @@ -21,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 @@ -43,22 +40,26 @@ export function shouldShowJumpToLatest( return distanceFromBottom(geometry) > threshold } +/** Allow bottom rounding noise without following a reader who moved up a line. */ +export const NATIVE_CHAT_FOLLOW_REARM_PX = 4 + export type FollowIntent = { following: boolean /** Whether the scroll event matches an offset the application registered. */ programmatic: boolean - atEnd: boolean + geometry: ScrollGeometry } /** Whether the transcript should still follow the end after this offset. * * Application writes preserve intent even when their delayed events arrive - * after the end moved. Reader events detach away from the end and reattach at it. */ + * after the end moved. Reader events detach away from the end and reattach at + * it — against the re-arm band, never the wider near-bottom one. */ export function nextFollowingEnd(intent: FollowIntent): boolean { if (intent.programmatic) { return intent.following } - return intent.atEnd + return isNearBottom(intent.geometry, NATIVE_CHAT_FOLLOW_REARM_PX) } /** Distance from the top within which the transcript pages in older history. */ diff --git a/src/renderer/src/components/native-chat/use-native-chat-transcript-scroll.ts b/src/renderer/src/components/native-chat/use-native-chat-transcript-scroll.ts index 0f690bce57e..c54cbf54e85 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-transcript-scroll.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-transcript-scroll.ts @@ -21,7 +21,6 @@ import { type UIEventHandler } from 'react' import { - isNearBottom, nextFollowingEnd, shouldLoadEarlier, shouldShowJumpToLatest, @@ -89,7 +88,7 @@ export function useNativeChatTranscriptScroll({ const following = nextFollowingEnd({ following: followingRef.current, programmatic, - atEnd: isNearBottom(geometry) + geometry }) followingRef.current = following if (!programmatic) { 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 24b63e1fbae..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,11 +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. +// 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 @@ -16,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' @@ -135,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) => { @@ -164,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)