From 35b7051dcfa4e1813c49e5ed8c23d5557229fd0a Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:37:14 +0800 Subject: [PATCH 01/11] fix(ui): bottom-align short VP content so blank space is at top (#9300) In VP mode, when the conversation fits within the viewport, content was top-aligned, leaving a blank gap between the last message and the composer. Bottom-align short bottom-stuck content (blank space at the top, latest message directly above the composer), matching standard chat TUIs. Only applies while sticking to the bottom with content that fits; overflow and scrolled-away cases are unchanged. Updates the three tests that pinned the previous top-aligned/collapsed layout to the new bottom-aligned expectation. Fixes #9300 --- .../shared/VirtualizedList.test.tsx | 25 ++++++++++++++++--- .../ui/components/shared/VirtualizedList.tsx | 14 ++++++++++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index dc7399e4575..eb74053f4b0 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -174,7 +174,12 @@ describe('', () => { await act(async () => {}); const frame = lastFrame() ?? ''; - expect(frame.split('\n')).toEqual([ + // Short bottom-stuck content is bottom-aligned (#9300): blank rows at the + // top, the five items pinned to the bottom of the 20-row container. + const lines = frame.split('\n'); + expect(lines.length).toBe(20); + expect(lines.slice(0, 15).every((l) => l.trim() === '')).toBe(true); + expect(lines.slice(15)).toEqual([ 'item-0', 'item-1', 'item-2', @@ -301,7 +306,9 @@ describe('', () => { />, ); await act(async () => {}); - expect(lastFrame()?.split('\n')).toEqual(['confirm']); + // Bottom-aligned (#9300): the single confirmation sits at the container + // bottom with blank rows above. + expect(lastFrame()?.split('\n')).toEqual(['', '', '', '', 'confirm']); const longConfirmationItems = [ { id: -1, label: ['confirm', 'line 2', 'line 3'].join('\n') }, @@ -326,7 +333,14 @@ describe('', () => { expect( frames.slice(frameCountBeforeLongContent).map((frame) => frame.trimEnd()), ).not.toContain('confirm'); - expect(lastFrame()?.split('\n')).toEqual(['confirm', 'line 2', 'line 3']); + // Bottom-aligned (#9300): 3-line content pinned to the 5-row bottom. + expect(lastFrame()?.split('\n')).toEqual([ + '', + '', + 'confirm', + 'line 2', + 'line 3', + ]); }); it('targetScrollIndex anchors to that index on first usable render', () => { @@ -1132,10 +1146,13 @@ describe(' VP collapsed thought groups', () => { const lines = (harness.lastFrame() ?? '').split('\n'); expect(lines.some((l) => l.includes('c1 thought line 0'))).toBe(false); expect(lines.filter((l) => l.trim() !== '').length).toBeLessThanOrEqual(6); - expect(lines.length).toBeLessThanOrEqual(8); // Collapsed state must still render the head summary, not an empty // window (the assertions above also hold for a blank frame). expect(lines.some((l) => l.includes('Thought for 1m 41s'))).toBe(true); + // Bottom-aligned (#9300): the released height becomes blank space at the + // TOP, and the collapsed summary sits at the bottom (last row is content, + // not a gap between content and the composer). + expect(lines[lines.length - 1]!.trim()).not.toBe(''); }); it('does not lock the render window when a tall thought collapses off-screen', async () => { diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index 84425c56ba3..da2896b2e6a 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -543,6 +543,17 @@ function VirtualizedList( maxScroll, ); + // Bottom-align short content while stuck to the bottom (#9300): when the + // whole conversation fits in the viewport, push it down so the latest + // message sits directly above the composer and any blank space is at the + // TOP (standard chat-TUI behavior), instead of top-aligning and leaving a + // gap between the last message and the composer. Zero whenever content + // overflows (maxScroll > 0) or the user has scrolled away from the bottom. + const bottomAlignGap = + isStickingToBottom && maxScroll === 0 + ? Math.max(0, scrollableContainerHeight - totalHeight) + : 0; + // The render window must cover what the viewport actually paints, so // it is computed from clampedScrollTop, not the anchor-based // actualScrollTop. While bottom-stuck the viewport pins to maxScroll, @@ -953,7 +964,7 @@ function VirtualizedList( // `containerHeight`, so scrolling is unaffected. const rootHeight = props.containerHeight !== undefined - ? fullHeightMeasurementPending + ? fullHeightMeasurementPending || bottomAlignGap > 0 ? props.containerHeight : Math.min(props.containerHeight, totalHeight) : '100%'; @@ -973,6 +984,7 @@ function VirtualizedList( flexDirection="column" marginTop={-clampedScrollTop} > + {renderedItems} From e4ab66807bdea31fd6ad52ca5f625a610362ab36 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com> Date: Mon, 17 Aug 2026 06:49:06 +0000 Subject: [PATCH 02/11] fix(ui): align VP design comment and tests with bottom-aligned layout (#9305) Co-authored-by: Qwen-Coder --- .../shared/VirtualizedList.test.tsx | 10 +++++---- .../ui/components/shared/VirtualizedList.tsx | 21 +++++++++++-------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index eb74053f4b0..26f026e1f98 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -145,7 +145,7 @@ describe('', () => { expect(frame.endsWith('item-19')).toBe(true); }); - it('collapses a short bottom-stuck list below the container height', async () => { + it('bottom-aligns a short bottom-stuck list within the container height', async () => { const { lastFrame, rerender } = render( data={makeItems(5)} @@ -273,7 +273,7 @@ describe('', () => { expect(lastFrame() ?? '').toContain('thinking line 29'); }); - it('collapses after measuring changed content at full viewport height', async () => { + it('bottom-aligns changed content after measuring it at full viewport height', async () => { const liveItems = [{ id: -1, label: 'live' }]; const { frames, lastFrame, rerender } = render( @@ -1150,8 +1150,10 @@ describe(' VP collapsed thought groups', () => { // window (the assertions above also hold for a blank frame). expect(lines.some((l) => l.includes('Thought for 1m 41s'))).toBe(true); // Bottom-aligned (#9300): the released height becomes blank space at the - // TOP, and the collapsed summary sits at the bottom (last row is content, - // not a gap between content and the composer). + // TOP, and the collapsed summary sits at the bottom of the full 40-row + // container (last row is content, not a gap between content and the + // composer). + expect(lines.length).toBe(40); expect(lines[lines.length - 1]!.trim()).not.toBe(''); }); diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index da2896b2e6a..495d7de734c 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -951,15 +951,18 @@ function VirtualizedList( ]); // The host passes `containerHeight` as the *maximum* viewport height (the - // room available between the header and the composer). Pinning the root box - // to that height unconditionally left a tall empty gap below short content - // and pushed the composer far down the screen — the legacy path - // instead grows with its content. Collapse to `totalHeight` whenever the - // content fits so the composer sits right beneath the conversation. A - // caller can request one full-height measurement pass while content changes - // shape under a stable item key; otherwise a stale cached total can clip - // the new content before it is measured. The root collapses again after the - // measurement so short content does not leave a gap above the composer. + // room available between the header and the composer). While the list is + // not bottom-aligned, collapse to `totalHeight` whenever the content fits: + // pinning the root box to the full height unconditionally would leave a + // tall empty gap below short content and push the composer far down the + // screen, while the legacy path grows with its content. While a + // bottom-stuck conversation has room to spare (#9300), keep the full + // `containerHeight` instead so `bottomAlignGap` can push it down: the + // blank rows render ABOVE the content and the latest message sits right + // above the composer. A caller can request one full-height measurement + // pass while content changes shape under a stable item key; otherwise a + // stale cached total can clip the new content before it is measured. The + // collapse/bottom-align rule above applies again after the measurement. // `scrollableContainerHeight` (the scroll math) still uses the full // `containerHeight`, so scrolling is unaffected. const rootHeight = From a5403de647ecad633da9778d289c868fb19b1f1c Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com> Date: Thu, 20 Aug 2026 00:24:40 +0000 Subject: [PATCH 03/11] fix(ui): stop re-stick from bottom-aligning top-anchored VP lists (#9305) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-stick effect treated "content fits" as "was at the bottom" and flipped isStickingToBottom to true on every commit of a fitting list. While fitting that flag used to be layout-invisible, but #9300's bottomAlignGap now keys on it, so the flip silently overrode the host's explicit top anchor: every banner-only VP session (MainContent mounts initialScrollIndex={0} for it) rendered the banner bottom-aligned above the composer with a blank viewport above, and a scrolled-away list whose content shrank to fit was yanked to the bottom on the follow-up heights-prune commit. Re-stick only from a real bottom position: previously overflowing with the viewport at the bottom pixels. The collapse cascade still re-engages sticking (its clamped scrollTop sits at the bottom pixels while the content is still overflowing), and growth auto-scroll keeps the broader wasAtBottom, so banner-only → first-message still snaps to the bottom-stuck layout. Adds the regression test for a fitting initialScrollIndex={0} mount and a discriminating test for the isStickingToBottom gate that kills the previously surviving maxScroll-only mutant. --- .../shared/VirtualizedList.test.tsx | 89 +++++++++++++++++++ .../ui/components/shared/VirtualizedList.tsx | 12 ++- 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index 26f026e1f98..1aa0b78cbda 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -188,6 +188,95 @@ describe('', () => { ]); }); + it('keeps a fitting top-anchored mount (initialScrollIndex 0) top-aligned', async () => { + // MainContent mounts the banner-only VP session with + // `initialScrollIndex={0}` (top-anchored). The mount-time re-stick must + // not override that explicit anchor and bottom-align the content: + // bottom-alignment is reserved for bottom-stuck conversations (#9300). + const { lastFrame, rerender } = render( + + data={makeItems(3)} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={0} + containerHeight={20} + width={40} + showScrollbar={false} + />, + ); + + rerender( + + data={makeItems(3)} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={0} + containerHeight={20} + width={40} + showScrollbar={false} + />, + ); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'item-0', + 'item-1', + 'item-2', + ]); + }); + + it('does not bottom-align a scrolled-away list when content shrinks to fit', async () => { + // Discriminates the `isStickingToBottom` gate of `bottomAlignGap`: the + // user scrolled away from the bottom, then content shrinks in place + // below the viewport. The frame must collapse top-aligned around the + // content the user is reading, not grow to the full container height + // with blank rows above it (#9305 review R4-2). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + let items = makeItems(20); + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollTo(0); + }); + rerender(); + await act(async () => {}); + + items = makeItems(3); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'item-0', + 'item-1', + 'item-2', + ]); + }); + it('reports zero-height shrink so collapsed items leave no blank gap', async () => { // Mirrors VP thought groups: the head renders a 1-line summary when // collapsed while continuations render nothing (zero height). The zero diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index 495d7de734c..1169ae2fcd3 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -415,7 +415,17 @@ function VirtualizedList( prevTotalHeight.current - prevContainerHeight.current - 1; const wasAtBottom = contentPreviouslyFit || wasScrolledToBottomPixels; - if (wasAtBottom && actualScrollTop >= prevScrollTop.current) { + // Fitting alone is not a bottom signal for re-sticking: a top-anchored + // mount (initialScrollIndex 0, e.g. the banner-only VP session) fits + // too, and flipping sticking there would let `bottomAlignGap` + // bottom-align content the host anchored to the top (#9300). Re-stick + // only from a real bottom position: previously overflowing with the + // viewport at the bottom pixels. + if ( + !contentPreviouslyFit && + wasScrolledToBottomPixels && + actualScrollTop >= prevScrollTop.current + ) { if (!isStickingToBottom) { setIsStickingToBottom(true); } From 23988d6883d37492e5623e6559c2ca28325c3575 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com> Date: Thu, 20 Aug 2026 03:59:00 +0000 Subject: [PATCH 04/11] fix(ui): stop scroll and shrink paths from re-engaging VP sticking (#9305) --- .../shared/VirtualizedList.test.tsx | 112 ++++++++++++++++++ .../ui/components/shared/VirtualizedList.tsx | 45 ++++++- 2 files changed, 155 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index 1aa0b78cbda..5392830f341 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -275,6 +275,118 @@ describe('', () => { 'item-1', 'item-2', ]); + + // A scroll attempt while everything fits is positionally a no-op and + // must not flip sticking either way: engaging it would bottom-align + // the fitting content (destroying the top-aligned state pinned above), + // releasing it would drop auto-follow (#9305 review R5-2). + for (const scroll of [ + () => listRef!.scrollBy(1), + () => listRef!.scrollBy(-1), + () => listRef!.scrollTo(0), + ]) { + act(scroll); + rerender(); + await act(async () => {}); + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'item-0', + 'item-1', + 'item-2', + ]); + } + }); + + it('top-aligns the banner-only state when a stuck list collapses without remount', async () => { + // /clear does not remount the list (no key on ScrollableList in + // MainContent), so sticking from the bottom-stuck rest state is still + // set when the data collapses to the banner alone. The host mounts that + // state top-anchored (initialScrollIndex 0), so the in-place collapse + // must render the same top-aligned frame instead of bottom-aligning the + // banner under a blank viewport (#9305 review R5-1). + let items = makeItems(20); + + const renderList = () => ( + + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + + const { lastFrame, rerender } = render(renderList()); + rerender(renderList()); + await act(async () => {}); + + items = [{ id: 999, label: 'banner' }]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual(['banner']); + }); + + it('does not re-stick a scrolled-away list that shrinks to fit in two steps', async () => { + // The first shrink re-anchors a scrolled-away viewport, parking it + // exactly at the new bottom; the re-stick gate must not read that + // content-driven clamp as the user being at the bottom and flip + // sticking back on when the next shrink fits (#9305 review R5-3). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + let items = makeItems(20); + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + items = makeItems(12); + rerender(); + rerender(); + await act(async () => {}); + + items = makeItems(8); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'item-0', + 'item-1', + 'item-2', + 'item-3', + 'item-4', + 'item-5', + 'item-6', + 'item-7', + ]); }); it('reports zero-height shrink so collapsed items leave no blank gap', async () => { diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index 1169ae2fcd3..4d717411834 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -406,6 +406,15 @@ function VirtualizedList( const prevTotalHeight = useRef(totalHeight); const prevScrollTop = useRef(actualScrollTop); const prevContainerHeight = useRef(scrollableContainerHeight); + // Set by the re-anchor branch when it clamps a scrolled-away viewport to + // the new bottom, holding the anchor the clamp installed. While the + // current anchor still sits where the clamp parked it, that position is + // content-driven, so the re-stick gate must not read it as the user + // having scrolled to the bottom. Any scroll moves the anchor, and the + // mismatch clears the mark. + const reAnchorClampMark = useRef<{ index: number; offset: number } | null>( + null, + ); useLayoutEffect(() => { const contentPreviouslyFit = @@ -420,8 +429,18 @@ function VirtualizedList( // too, and flipping sticking there would let `bottomAlignGap` // bottom-align content the host anchored to the top (#9300). Re-stick // only from a real bottom position: previously overflowing with the - // viewport at the bottom pixels. + // viewport at the bottom pixels. A position installed by the re-anchor + // clamp is not a user-driven bottom either (#9305): suppress the flip + // while the anchor still matches the clamp mark. + const clampParked = + reAnchorClampMark.current !== null && + scrollAnchor.index === reAnchorClampMark.current.index && + scrollAnchor.offset === reAnchorClampMark.current.offset; + if (reAnchorClampMark.current !== null && !clampParked) { + reAnchorClampMark.current = null; + } if ( + !clampParked && !contentPreviouslyFit && wasScrolledToBottomPixels && actualScrollTop >= prevScrollTop.current @@ -460,8 +479,18 @@ function VirtualizedList( actualScrollTop > totalHeight - scrollableContainerHeight) && data.length > 0 ) { + if (data.length <= 1 && isStickingToBottom) { + // Collapse to the host's non-end-anchored state (MainContent mounts + // banner-only data with initialScrollIndex 0, e.g. after /clear): + // the followed conversation is gone, so drop the carried sticking + // instead of bottom-aligning the remnant under a blank viewport. + setIsStickingToBottom(false); + } const newScrollTop = Math.max(0, totalHeight - scrollableContainerHeight); const newAnchor = getAnchorForScrollTop(newScrollTop, offsets); + if (!isStickingToBottom) { + reAnchorClampMark.current = newAnchor; + } if ( scrollAnchor.index !== newAnchor.index || scrollAnchor.offset !== newAnchor.offset @@ -754,11 +783,18 @@ function VirtualizedList( ref, () => ({ scrollBy: (delta: number) => { + const maxScroll = Math.max(0, totalHeight - scrollableContainerHeight); + if (maxScroll === 0) { + // Nothing to scroll: the attempt is positionally a no-op and must + // not flip sticking either way. Engaging it would bottom-align a + // top-anchored list whose content fits; releasing it would drop a + // stuck conversation's auto-follow. + return; + } if (delta < 0) { setIsStickingToBottom(false); } const currentScrollTop = getScrollTop(); - const maxScroll = Math.max(0, totalHeight - scrollableContainerHeight); const actualCurrent = Math.min(currentScrollTop, maxScroll); const newScrollTop = Math.max(0, actualCurrent + delta); // Reaching the bottom must use the same live-recomputing end anchor as @@ -782,6 +818,11 @@ function VirtualizedList( }, scrollTo: (offset: number) => { const maxScroll = Math.max(0, totalHeight - scrollableContainerHeight); + if (maxScroll === 0) { + // Same no-op rule as scrollBy: a scroll attempt on fitting content + // must not flip sticking either way. + return; + } if (offset >= maxScroll || offset === SCROLL_TO_ITEM_END) { setIsStickingToBottom(true); setPendingScrollTop(Number.MAX_SAFE_INTEGER); From 05c073f94b50a79c1e20941993752b8e0c321c26 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Thu, 20 Aug 2026 09:33:26 +0000 Subject: [PATCH 05/11] fix(ui): keep clamp-parked VP positions from re-engaging sticking (#9305) Co-authored-by: Qwen-Coder --- .../shared/VirtualizedList.test.tsx | 158 ++++++++++++++++++ .../ui/components/shared/VirtualizedList.tsx | 20 ++- 2 files changed, 175 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index 5392830f341..15e1c2853cc 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -284,6 +284,7 @@ describe('', () => { () => listRef!.scrollBy(1), () => listRef!.scrollBy(-1), () => listRef!.scrollTo(0), + () => listRef!.scrollToEnd(), ]) { act(scroll); rerender(); @@ -330,6 +331,143 @@ describe('', () => { expect((lastFrame() ?? '').split('\n')).toEqual(['banner']); }); + it('keeps sticking released when an overflowing banner-only list is resized', async () => { + // The collapse drop queues setIsStickingToBottom(false), but the + // mark-install still reads the stale render-time flag on that render. + // When the single remaining item overflows the container (a tall + // banner in a small terminal), the missing clamp mark lets the next + // effect trigger — here a terminal resize — read the parked position + // as the user being at the bottom and re-engage sticking, + // bottom-pinning the top-anchored banner (#9305 review R6-1). The + // banner sits at index 0 like AppHeader so its height is cached + // before the collapse, as in the real /clear lifecycle. + const banner = Array.from({ length: 15 }, (_, i) => `b${i}`).join('\n'); + let items: Item[] = [{ id: 999, label: banner }, ...makeItems(5)]; + let height = 10; + + const renderList = () => ( + + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={height} + width={40} + showScrollbar={false} + /> + ); + + const { lastFrame, rerender } = render(renderList()); + rerender(renderList()); + await act(async () => {}); + + items = [{ id: 999, label: banner }]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + // Collapse parks the viewport at the banner's bottom (top clipped). + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'b5', + 'b6', + 'b7', + 'b8', + 'b9', + 'b10', + 'b11', + 'b12', + 'b13', + 'b14', + ]); + + height = 8; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + // The resize must not re-engage sticking from the parked position: + // the anchor holds and the frame is not bottom-pinned to b7..b14. + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'b5', + 'b6', + 'b7', + 'b8', + 'b9', + 'b10', + 'b11', + 'b12', + ]); + }); + + it('does not yank the viewport to the first post-clear message', async () => { + // Combined R6-1 + R6-2: after the /clear-style collapse drops + // sticking and parks the viewport inside an overflowing banner, the + // first new message must not read that parked position as the user + // being at the bottom: the growth branch would snap the anchor to + // the end and latch auto-follow back on (#9305 reviews R6-1, R6-2). + // The banner sits at index 0 like AppHeader so its height is cached + // before the collapse, as in the real /clear lifecycle. + const banner = Array.from({ length: 12 }, (_, i) => `b${i}`).join('\n'); + let items: Item[] = [{ id: 999, label: banner }, ...makeItems(5)]; + + const renderList = () => ( + + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + + const { lastFrame, rerender } = render(renderList()); + rerender(renderList()); + await act(async () => {}); + + items = [{ id: 999, label: banner }]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'b2', + 'b3', + 'b4', + 'b5', + 'b6', + 'b7', + 'b8', + 'b9', + 'b10', + 'b11', + ]); + + items = [ + { id: 999, label: banner }, + { id: 0, label: 'message' }, + ]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'b2', + 'b3', + 'b4', + 'b5', + 'b6', + 'b7', + 'b8', + 'b9', + 'b10', + 'b11', + ]); + }); + it('does not re-stick a scrolled-away list that shrinks to fit in two steps', async () => { // The first shrink re-anchors a scrolled-away viewport, parking it // exactly at the new bottom; the re-stick gate must not read that @@ -387,6 +525,26 @@ describe('', () => { 'item-6', 'item-7', ]); + + // The parked position must not read as the user being at the bottom + // when the list grows either: the next message arriving must not yank + // the anchor to the end and re-engage sticking (#9305 review R6-2). + items = makeItems(9); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'item-0', + 'item-1', + 'item-2', + 'item-3', + 'item-4', + 'item-5', + 'item-6', + 'item-7', + 'item-8', + ]); }); it('reports zero-height shrink so collapsed items leave no blank gap', async () => { diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index 4d717411834..bfc07f65b7c 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -458,7 +458,10 @@ function VirtualizedList( if ( shouldAutoScroll && - ((listGrew && (isStickingToBottom || wasAtBottom)) || + // The clamp-parked position is content-driven, not the user being at + // the bottom, so growth must not auto-follow from it either (#9305 + // review R6-2). + ((listGrew && (isStickingToBottom || (wasAtBottom && !clampParked))) || (isStickingToBottom && containerChanged)) ) { const newIndex = data.length > 0 ? data.length - 1 : 0; @@ -479,7 +482,8 @@ function VirtualizedList( actualScrollTop > totalHeight - scrollableContainerHeight) && data.length > 0 ) { - if (data.length <= 1 && isStickingToBottom) { + const droppingSticking = data.length <= 1 && isStickingToBottom; + if (droppingSticking) { // Collapse to the host's non-end-anchored state (MainContent mounts // banner-only data with initialScrollIndex 0, e.g. after /clear): // the followed conversation is gone, so drop the carried sticking @@ -488,7 +492,11 @@ function VirtualizedList( } const newScrollTop = Math.max(0, totalHeight - scrollableContainerHeight); const newAnchor = getAnchorForScrollTop(newScrollTop, offsets); - if (!isStickingToBottom) { + // Install the mark on the drop render too: isStickingToBottom is the + // stale render-time flag (the drop only queued its update), and + // without the mark the next effect trigger re-sticks from the parked + // position before the release is visible here (#9305 review R6-1). + if (!isStickingToBottom || droppingSticking) { reAnchorClampMark.current = newAnchor; } if ( @@ -840,6 +848,12 @@ function VirtualizedList( } }, scrollToEnd: () => { + const maxScroll = Math.max(0, totalHeight - scrollableContainerHeight); + if (maxScroll === 0) { + // Same no-op rule as scrollBy/scrollTo: a scroll attempt on + // fitting content must not flip sticking either way. + return; + } setIsStickingToBottom(true); setPendingScrollTop(Number.MAX_SAFE_INTEGER); if (data.length > 0) { From 5e38caff9bea29993d3aaf0e8f8c20c098681814 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Thu, 20 Aug 2026 12:46:58 +0000 Subject: [PATCH 06/11] fix(ui): install the VP clamp mark only when the clamp moved the anchor (#9305) --- .../shared/VirtualizedList.test.tsx | 112 ++++++++++++++++++ .../ui/components/shared/VirtualizedList.tsx | 20 ++-- 2 files changed, 125 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index 15e1c2853cc..2304c861e96 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -468,6 +468,118 @@ describe('', () => { ]); }); + it('re-engages auto-follow when a fitting top-anchored mount grows to overflow', async () => { + // MainContent mounts the banner-only session top-anchored + // (`length <= 1 ? 0 : SCROLL_TO_ITEM_END`). While the banner fits, + // the re-anchor clamp moves nothing, so no clamp mark may be + // installed: while content fits, nothing can move the anchor off a + // mark, and `clampParked` would suppress the growth auto-follow + // branch forever — the first streamed reply would then render below + // the fold (#9305 review R8-1). Growth must re-engage sticking once + // the conversation overflows the viewport. + let items: Item[] = [{ id: 999, label: 'banner' }]; + + const renderList = () => ( + + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={0} + containerHeight={5} + width={40} + showScrollbar={false} + /> + ); + + const { lastFrame, rerender } = render(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual(['banner']); + + items = [{ id: 999, label: 'banner' }, ...makeItems(8)]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'item-3', + 'item-4', + 'item-5', + 'item-6', + 'item-7', + ]); + + // Sticking stays engaged: further growth keeps following. + items = [{ id: 999, label: 'banner' }, ...makeItems(14)]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'item-9', + 'item-10', + 'item-11', + 'item-12', + 'item-13', + ]); + }); + + it('re-engages auto-follow when the first post-clear message overflows a fitting banner', async () => { + // Fitting-remnant counterpart of the R6 combined test above: the + // /clear collapse drops sticking and re-anchors to {0,0}, but the + // remnant FITS — the re-stick gate is already blocked by + // `!contentPreviouslyFit`, so no clamp mark may be installed. A + // mark here reads as `clampParked` on every later growth and + // suppresses the auto-follow branch, killing follow for the first + // conversation after /clear (#9305 review R8-1). The overflowing + // remnant keeps its park (pinned by the R6 tests). + let items = makeItems(20); + + const renderList = () => ( + + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + + const { lastFrame, rerender } = render(renderList()); + rerender(renderList()); + await act(async () => {}); + + items = [{ id: 999, label: 'banner' }]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual(['banner']); + + items = [{ id: 999, label: 'banner' }, ...makeItems(12)]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'item-2', + 'item-3', + 'item-4', + 'item-5', + 'item-6', + 'item-7', + 'item-8', + 'item-9', + 'item-10', + 'item-11', + ]); + }); + it('does not re-stick a scrolled-away list that shrinks to fit in two steps', async () => { // The first shrink re-anchors a scrolled-away viewport, parking it // exactly at the new bottom; the re-stick gate must not read that diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index bfc07f65b7c..30ae3a4ff39 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -492,17 +492,23 @@ function VirtualizedList( } const newScrollTop = Math.max(0, totalHeight - scrollableContainerHeight); const newAnchor = getAnchorForScrollTop(newScrollTop, offsets); - // Install the mark on the drop render too: isStickingToBottom is the - // stale render-time flag (the drop only queued its update), and - // without the mark the next effect trigger re-sticks from the parked - // position before the release is visible here (#9305 review R6-1). - if (!isStickingToBottom || droppingSticking) { - reAnchorClampMark.current = newAnchor; - } if ( scrollAnchor.index !== newAnchor.index || scrollAnchor.offset !== newAnchor.offset ) { + // Install the mark on the drop render too: isStickingToBottom is the + // stale render-time flag (the drop only queued its update), and + // without the mark the next effect trigger re-sticks from the parked + // position before the release is visible here (#9305 review R6-1). + // Only when the clamp moved the anchor, and on the drop path only + // while the remnant still overflows: while content fits, nothing + // can move the anchor off a mark, so one installed at rest reads as + // clampParked forever and suppresses growth auto-follow, defending + // nothing — the re-stick gate is already blocked by + // `!contentPreviouslyFit` (#9305 review R8-1). + if (!isStickingToBottom || (droppingSticking && newScrollTop > 0)) { + reAnchorClampMark.current = newAnchor; + } setScrollAnchor(newAnchor); } } else if (data.length === 0) { From 047021aa52180f71183ff27f506c8d3f116b873e Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Thu, 20 Aug 2026 14:43:00 +0000 Subject: [PATCH 07/11] fix(ui): let VP auto-follow re-engage after scrolled-away fitting-banner collapse (#9305) --- .../shared/VirtualizedList.test.tsx | 80 +++++++++++++++++++ .../ui/components/shared/VirtualizedList.tsx | 18 +++-- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index 2304c861e96..60646fa21bb 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -580,6 +580,86 @@ describe('', () => { ]); }); + it('re-engages auto-follow when a scrolled-away collapse to a fitting banner grows', async () => { + // Scrolled-away counterpart of the post-clear re-follow test above: + // sticking was already released before the collapse re-anchors, but the + // fitting banner must not get a clamp mark through the released arm + // either — while content fits, nothing can move the anchor off a mark, + // so one installed here reads as `clampParked` on every later growth + // and suppresses the auto-follow branch, leaving the first post-clear + // reply below the fold (#9305 review R10-1). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + let items: Item[] = makeItems(20); + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + // Scrolled-away rest state: sticking released, viewport five rows up. + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'item-5', + 'item-6', + 'item-7', + 'item-8', + 'item-9', + 'item-10', + 'item-11', + 'item-12', + 'item-13', + 'item-14', + ]); + + items = [{ id: 999, label: 'banner' }]; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual(['banner']); + + items = [{ id: 999, label: 'banner' }, ...makeItems(12)]; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'item-2', + 'item-3', + 'item-4', + 'item-5', + 'item-6', + 'item-7', + 'item-8', + 'item-9', + 'item-10', + 'item-11', + ]); + }); + it('does not re-stick a scrolled-away list that shrinks to fit in two steps', async () => { // The first shrink re-anchors a scrolled-away viewport, parking it // exactly at the new bottom; the re-stick gate must not read that diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index 30ae3a4ff39..774f1aa0b11 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -500,13 +500,17 @@ function VirtualizedList( // stale render-time flag (the drop only queued its update), and // without the mark the next effect trigger re-sticks from the parked // position before the release is visible here (#9305 review R6-1). - // Only when the clamp moved the anchor, and on the drop path only - // while the remnant still overflows: while content fits, nothing - // can move the anchor off a mark, so one installed at rest reads as - // clampParked forever and suppresses growth auto-follow, defending - // nothing — the re-stick gate is already blocked by - // `!contentPreviouslyFit` (#9305 review R8-1). - if (!isStickingToBottom || (droppingSticking && newScrollTop > 0)) { + // While the remnant overflows, always install it — a scroll can move + // the anchor off the mark and end the suppression. A fitting remnant + // keeps it only through the released arm and only when real content + // remains: that park must not re-stick on the next trigger (#9305 + // review R5-3). A fitting banner-only remnant gets no mark on either + // arm: while content fits, nothing can move the anchor off a mark, + // so one installed at rest reads as clampParked forever and kills + // growth auto-follow, defending nothing — the re-stick gate is + // already blocked by `!contentPreviouslyFit` (#9305 reviews R8-1, + // R10-1). + if (newScrollTop > 0 || (!isStickingToBottom && data.length > 1)) { reAnchorClampMark.current = newAnchor; } setScrollAnchor(newAnchor); From fa7f5c9693b29473b4d3eb824f047e4fe65526b0 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Thu, 20 Aug 2026 20:53:16 +0000 Subject: [PATCH 08/11] fix(ui): close resize edges that defeat the VP sticking drop and clamp mark (#9305) --- .../shared/VirtualizedList.test.tsx | 357 ++++++++++++++++++ .../ui/components/shared/VirtualizedList.tsx | 26 +- 2 files changed, 378 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index 60646fa21bb..c8b429eb9ed 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -739,6 +739,363 @@ describe('', () => { ]); }); + it('does not preempt the sticking drop when a shrink and resize land in one render', async () => { + // Ink coalesces rapid state updates, so a terminal resize can land in + // the same render as the /clear-style shrink to the banner. The + // container-change arm must not evaluate on the stale render-time + // sticking flag and preempt the drop branch there: the END anchor it + // would install makes the drop unreachable again, so the first + // post-clear message yanks the viewport and re-latches follow — the + // R6-2 regression this PR removes (#9305 review R11-1). + const banner = Array.from({ length: 15 }, (_, i) => `b${i}`).join('\n'); + let items: Item[] = [{ id: 999, label: banner }, ...makeItems(5)]; + let height = 10; + + const renderList = () => ( + + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={height} + width={40} + showScrollbar={false} + /> + ); + + const { lastFrame, rerender } = render(renderList()); + rerender(renderList()); + await act(async () => {}); + + // Shrink to the banner and resize the container in one batched render. + items = [{ id: 999, label: banner }]; + height = 8; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + // The collapse parks the viewport at the banner's bottom, sticking + // released. + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'b7', + 'b8', + 'b9', + 'b10', + 'b11', + 'b12', + 'b13', + 'b14', + ]); + + // The first post-clear message must not yank the viewport to the end + // nor re-latch auto-follow. + items = [ + { id: 999, label: banner }, + { id: 0, label: 'message' }, + ]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'b7', + 'b8', + 'b9', + 'b10', + 'b11', + 'b12', + 'b13', + 'b14', + ]); + + // Follow must stay off: once the content fits again the frame renders + // top-aligned, not bottom-aligned under a stuck viewport. + height = 20; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + ...Array.from({ length: 15 }, (_, i) => `b${i}`), + 'message', + ]); + }); + + it('does not yank the first message after a resize overflows a fitting banner by one row', async () => { + // After the /clear collapse a fitting banner installs no mark (R8-1 / + // R10-1). Shrinking the terminal so the banner overflows by exactly + // one row must not let the -1 bottom tolerance read the parked TOP of + // that one-row scroll range as the bottom pixels: the first new + // message would then snap the anchor to END and latch follow from a + // position the user never scrolled to (#9305 review R11-4). + const banner = Array.from({ length: 10 }, (_, i) => `b${i}`).join('\n'); + let items: Item[] = [{ id: 999, label: banner }, ...makeItems(5)]; + let height = 12; + + const renderList = () => ( + + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={height} + width={40} + showScrollbar={false} + /> + ); + + const { lastFrame, rerender } = render(renderList()); + rerender(renderList()); + await act(async () => {}); + + // Collapse to the fitting banner: top-anchored, no mark. + items = [{ id: 999, label: banner }]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'b0', + 'b1', + 'b2', + 'b3', + 'b4', + 'b5', + 'b6', + 'b7', + 'b8', + 'b9', + ]); + + // Resize so the banner overflows by exactly one row: the park stays + // at the top with sticking off. + height = 9; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'b0', + 'b1', + 'b2', + 'b3', + 'b4', + 'b5', + 'b6', + 'b7', + 'b8', + ]); + + // The first new message must not yank the viewport nor latch follow. + items = [ + { id: 999, label: banner }, + { id: 0, label: 'message' }, + ]; + rerender(renderList()); + rerender(renderList()); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'b0', + 'b1', + 'b2', + 'b3', + 'b4', + 'b5', + 'b6', + 'b7', + 'b8', + ]); + }); + + it('re-engages auto-follow when a scrolled-away resize-to-fit grows past the viewport', async () => { + // Scrolled away in an overflowing conversation, then the terminal + // grows until the content fits: the clamp parks at {0,0} and the + // released arm installs the mark. While the content fits that park is + // correct, but once growth crosses the fit boundary the user — who + // could see everything while it fit — must get auto-follow back + // instead of every new message rendering below the fold (#9305 + // review R11-6). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + let items = makeItems(20); + let height = 10; + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={height} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + // Enlarge the terminal until the content fits: top-parked at {0,0}. + height = 25; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 20 }, (_, i) => `item-${i}`), + ); + + // Growth past the viewport re-engages follow. + items = makeItems(28); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 25 }, (_, i) => `item-${i + 3}`), + ); + }); + + it('re-engages auto-follow when a scrolled-away shrink-to-fit grows past the viewport', async () => { + // Second entrance of the released-arm mark: the user scrolls away, the + // list shrinks in place until it fits, and the park at {0,0} installs + // the mark. Growth crossing the fit boundary must re-engage follow + // (#9305 review R11-6). The still-fitting growth right after the park + // stays suppressed (pinned by the R6-2 test above). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + let items = makeItems(20); + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + items = makeItems(8); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 8 }, (_, i) => `item-${i}`), + ); + + items = makeItems(14); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 10 }, (_, i) => `item-${i + 4}`), + ); + }); + + it('re-engages auto-follow across a wholesale dataset replacement', async () => { + // /resume swaps the history in one batched commit and ScrollableList + // carries no key, so the list state — including the clamp mark — + // survives the swap. Switching into a short fitting session re-anchors + // to {0,0} and the released arm installs the mark on the NEW dataset; + // the mark must not kill growth follow there: the session's first + // streaming reply past the viewport must follow, not render below the + // fold (#9305 review R11-6). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + let items = makeItems(30); + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + // In-place session switch to a short fitting session (2+ items). + items = Array.from({ length: 6 }, (_, i) => ({ + id: 100 + i, + label: `it-${100 + i}`, + })); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 6 }, (_, i) => `it-${100 + i}`), + ); + + // The new session streams past the viewport: follow must re-engage. + items = Array.from({ length: 15 }, (_, i) => ({ + id: 100 + i, + label: `it-${100 + i}`, + })); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 10 }, (_, i) => `it-${105 + i}`), + ); + }); + it('reports zero-height shrink so collapsed items leave no blank gap', async () => { // Mirrors VP thought groups: the head renders a 1-line summary when // collapsed while continuations render nothing (zero height). The zero diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index 774f1aa0b11..051402313dd 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -419,9 +419,12 @@ function VirtualizedList( useLayoutEffect(() => { const contentPreviouslyFit = prevTotalHeight.current <= prevContainerHeight.current; + const prevMaxScroll = prevTotalHeight.current - prevContainerHeight.current; + // The -1 tolerance must not span the whole scroll range: a viewport + // parked at the TOP of a one-row range would otherwise read as "at the + // bottom pixels" and yank the first growth to END (#9305 review R11-4). const wasScrolledToBottomPixels = - prevScrollTop.current >= - prevTotalHeight.current - prevContainerHeight.current - 1; + prevMaxScroll > 1 && prevScrollTop.current >= prevMaxScroll - 1; const wasAtBottom = contentPreviouslyFit || wasScrolledToBottomPixels; // Fitting alone is not a bottom signal for re-sticking: a top-anchored @@ -460,9 +463,22 @@ function VirtualizedList( shouldAutoScroll && // The clamp-parked position is content-driven, not the user being at // the bottom, so growth must not auto-follow from it either (#9305 - // review R6-2). - ((listGrew && (isStickingToBottom || (wasAtBottom && !clampParked))) || - (isStickingToBottom && containerChanged)) + // review R6-2) — except growth that crosses the fit boundary: while + // the content fit, the user could see everything, so follow must come + // back once it overflows instead of every new message rendering below + // the fold (#9305 review R11-6). + ((listGrew && + (isStickingToBottom || + (wasAtBottom && + (!clampParked || + (contentPreviouslyFit && + totalHeight > scrollableContainerHeight))))) || + // A shrink landing in the same render must reach the drop/re-anchor + // branch below, not be preempted here on the stale render-time + // sticking flag (#9305 review R11-1). + (isStickingToBottom && + containerChanged && + data.length >= prevDataLength.current)) ) { const newIndex = data.length > 0 ? data.length - 1 : 0; if ( From c7e46dca7fbb101b93685771d24ba5a0206a9426 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Fri, 21 Aug 2026 20:15:19 +0000 Subject: [PATCH 09/11] fix(ui): re-engage VP follow on any fit-boundary crossing, not just length growth (#9305) --- .../shared/VirtualizedList.test.tsx | 139 ++++++++++++++++++ .../ui/components/shared/VirtualizedList.tsx | 21 ++- 2 files changed, 149 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index c8b429eb9ed..594655ef47a 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -1096,6 +1096,145 @@ describe('', () => { ); }); + it('re-engages auto-follow when parked fitting content grows taller in place', async () => { + // Streaming reply shape: useGeminiStream updates one pending item per + // chunk and MainContent maps it at a constant array position, so the + // reply crosses the container height at constant data.length. The + // fit-boundary crossing must re-engage follow from the clamp-parked, + // previously-fitting state for any growth shape, not only data.length + // increases (#9305 review R14-1). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + let items = makeItems(20); + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + // Shrink in place until the content fits: clamp-parked at {0,0}. + items = makeItems(8); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 8 }, (_, i) => `item-${i}`), + ); + + // In-place height growth that still fits must not yank the park. + items = [...makeItems(7), { id: 7, label: 'tok-0\ntok-1' }]; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + ...Array.from({ length: 7 }, (_, i) => `item-${i}`), + 'tok-0', + 'tok-1', + ]); + + // The streamed reply crosses the container height at constant + // data.length: follow must re-engage. + items = [...makeItems(7), { id: 7, label: 'tok-0\ntok-1\ntok-2\ntok-3' }]; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + ...Array.from({ length: 6 }, (_, i) => `item-${i + 1}`), + 'tok-0', + 'tok-1', + 'tok-2', + 'tok-3', + ]); + }); + + it('re-engages auto-follow when a container shrink overflows parked fitting content', async () => { + // Second crossing shape: the resize-to-fit park at {0,0} carries the + // mark, then the terminal shrinks back below the content height. The + // previously-fitting state overflows without any data.length change; + // afterwards contentPreviouslyFit is false and the parked scrollTop is + // not near the grown max scroll, so no other gate can bring follow + // back — the crossing must re-engage it (#9305 review R14-1). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + const items = makeItems(20); + let height = 10; + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={height} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + // Enlarge the terminal until the content fits: top-parked at {0,0}. + height = 25; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 20 }, (_, i) => `item-${i}`), + ); + + // Shrink the terminal back below the content height: follow must + // re-engage. + height = 10; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 10 }, (_, i) => `item-${i + 10}`), + ); + }); + it('reports zero-height shrink so collapsed items leave no blank gap', async () => { // Mirrors VP thought groups: the head renders a 1-line summary when // collapsed while continuations render nothing (zero height). The zero diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index 051402313dd..919bb0fe55f 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -462,17 +462,16 @@ function VirtualizedList( if ( shouldAutoScroll && // The clamp-parked position is content-driven, not the user being at - // the bottom, so growth must not auto-follow from it either (#9305 - // review R6-2) — except growth that crosses the fit boundary: while - // the content fit, the user could see everything, so follow must come - // back once it overflows instead of every new message rendering below - // the fold (#9305 review R11-6). - ((listGrew && - (isStickingToBottom || - (wasAtBottom && - (!clampParked || - (contentPreviouslyFit && - totalHeight > scrollableContainerHeight))))) || + // the bottom, so growth must not auto-follow from it (#9305 review + // R6-2) — except a parked state that previously fit crossing into + // overflow by any shape: length growth, in-place height growth, or a + // container shrink. While the content fit, the user could see + // everything, so follow must come back once it overflows instead of + // content rendering below the fold (#9305 reviews R11-6, R14-1). + ((listGrew && (isStickingToBottom || (wasAtBottom && !clampParked))) || + (clampParked && + contentPreviouslyFit && + totalHeight > scrollableContainerHeight) || // A shrink landing in the same render must reach the drop/re-anchor // branch below, not be preempted here on the stale render-time // sticking flag (#9305 review R11-1). From 839a87329b5e47b7107efa5c0e78a9edef141e19 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Sat, 22 Aug 2026 05:37:25 +0000 Subject: [PATCH 10/11] fix(ui): keep VP follow alive across dataset swaps and banner-only overflow (#9305) --- .../shared/VirtualizedList.test.tsx | 245 ++++++++++++++++++ .../ui/components/shared/VirtualizedList.tsx | 60 +++-- 2 files changed, 289 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index 594655ef47a..2880e404216 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -1096,6 +1096,251 @@ describe('', () => { ); }); + it('does not re-latch sticking when a banner-only remnant overflows after a fitting park', async () => { + // The released shrink-to-fit park installs the mark keyed to the first + // remnant item — the banner, which survives the /clear collapse at the + // same {0,0} anchor, so the mark still validates and survives too. A + // terminal shrink that then overflows the banner must not let the + // fit→overflow crossing re-latch sticking on the banner-only remnant: + // there is no conversation to follow, and the engaged flag would + // bottom-align the banner under a blank viewport and yank the first + // post-clear message (#9305 review R17-1). + const banner = Array.from({ length: 3 }, (_, i) => `b${i}`).join('\n'); + let items: Item[] = [{ id: 999, label: banner }, ...makeItems(20)]; + let height = 12; + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={height} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + // Shrink in place until the remnant fits: the released park at {0,0} + // installs the mark keyed to the banner. + items = [{ id: 999, label: banner }, ...makeItems(8)]; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'b0', + 'b1', + 'b2', + ...Array.from({ length: 8 }, (_, i) => `item-${i}`), + ]); + + // /clear collapses to the banner alone; it still fits. + items = [{ id: 999, label: banner }]; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual(['b0', 'b1', 'b2']); + + // Shrink the terminal past the banner height: sticking must stay + // released and the frame stays top-aligned, not bottom-pinned b1..b2. + height = 2; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual(['b0', 'b1']); + + // The first post-clear message must not yank the viewport nor latch + // follow. + items = [ + { id: 999, label: banner }, + { id: 0, label: 'message' }, + ]; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual(['b0', 'b1']); + }); + + it('re-engages auto-follow when a released swap lands in an overflowing session', async () => { + // /resume swaps the whole dataset in one commit and ScrollableList + // carries no key, so the carried anchor lands out of range and the drop + // branch parks the viewport at the new session's live bottom. That park + // must not install the clamp mark: the user never scrolled there, and + // the mark would suppress every re-follow path, leaving the new + // session's first streamed messages below the fold until a manual + // scroll (#9305 review R17-2). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + let items = makeItems(30); + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + // In-place session switch to an overflowing session: the carried + // anchor is out of range, the drop branch parks at the new bottom. + items = Array.from({ length: 12 }, (_, i) => ({ + id: 100 + i, + label: `it-${100 + i}`, + })); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 10 }, (_, i) => `it-${102 + i}`), + ); + + // The new session streams: follow must re-engage. + items = Array.from({ length: 13 }, (_, i) => ({ + id: 100 + i, + label: `it-${100 + i}`, + })); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 10 }, (_, i) => `it-${103 + i}`), + ); + + items = Array.from({ length: 14 }, (_, i) => ({ + id: 100 + i, + label: `it-${100 + i}`, + })); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 10 }, (_, i) => `it-${104 + i}`), + ); + }); + + it('invalidates the clamp mark when a dataset swap keeps the anchor in range', async () => { + // The mark identifies the parked item, not just coordinates: a /resume + // swap whose carried anchor stays in range and inside the new max + // scroll bypasses the drop branch entirely, and a coordinate-only mark + // would survive the swap and suppress follow in the new session + // (#9305 review R17-3). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + let items = makeItems(20); + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + // Shrink in place: the drop branch parks at the new bottom and + // installs the mark keyed to the parked item. + items = makeItems(12); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 10 }, (_, i) => `item-${i + 2}`), + ); + + // In-place session switch whose items sit under the carried anchor: + // the anchor stays in range inside the new max scroll, so no drop + // render fires. The stale mark must not survive the swap. + items = Array.from({ length: 20 }, (_, i) => ({ + id: 100 + i, + label: `it-${100 + i}`, + })); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 10 }, (_, i) => `it-${110 + i}`), + ); + + // The new session streams: follow must work. + items = Array.from({ length: 21 }, (_, i) => ({ + id: 100 + i, + label: `it-${100 + i}`, + })); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 10 }, (_, i) => `it-${111 + i}`), + ); + }); + it('re-engages auto-follow when parked fitting content grows taller in place', async () => { // Streaming reply shape: useGeminiStream updates one pending item per // chunk and MainContent maps it at a constant array position, so the diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index 919bb0fe55f..dccfafe0ad2 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -407,14 +407,18 @@ function VirtualizedList( const prevScrollTop = useRef(actualScrollTop); const prevContainerHeight = useRef(scrollableContainerHeight); // Set by the re-anchor branch when it clamps a scrolled-away viewport to - // the new bottom, holding the anchor the clamp installed. While the - // current anchor still sits where the clamp parked it, that position is - // content-driven, so the re-stick gate must not read it as the user - // having scrolled to the bottom. Any scroll moves the anchor, and the - // mismatch clears the mark. - const reAnchorClampMark = useRef<{ index: number; offset: number } | null>( - null, - ); + // the new bottom, holding the anchor the clamp installed plus the key of + // the item parked there. While the current anchor still sits where the + // clamp parked it, that position is content-driven, so the re-stick gate + // must not read it as the user having scrolled to the bottom. Any scroll + // moves the anchor, and the mismatch clears the mark; a wholesale dataset + // swap (/resume — ScrollableList carries no key) changes the key at the + // mark's index and clears it the same way (#9305 review R17-3). + const reAnchorClampMark = useRef<{ + index: number; + offset: number; + key: string; + } | null>(null); useLayoutEffect(() => { const contentPreviouslyFit = @@ -435,11 +439,15 @@ function VirtualizedList( // viewport at the bottom pixels. A position installed by the re-anchor // clamp is not a user-driven bottom either (#9305): suppress the flip // while the anchor still matches the clamp mark. + const clampMark = reAnchorClampMark.current; + const clampMarkItem = clampMark ? data[clampMark.index] : undefined; const clampParked = - reAnchorClampMark.current !== null && - scrollAnchor.index === reAnchorClampMark.current.index && - scrollAnchor.offset === reAnchorClampMark.current.offset; - if (reAnchorClampMark.current !== null && !clampParked) { + clampMark !== null && + clampMarkItem !== undefined && + scrollAnchor.index === clampMark.index && + scrollAnchor.offset === clampMark.offset && + keyExtractor(clampMarkItem, clampMark.index) === clampMark.key; + if (clampMark !== null && !clampParked) { reAnchorClampMark.current = null; } if ( @@ -468,9 +476,13 @@ function VirtualizedList( // container shrink. While the content fit, the user could see // everything, so follow must come back once it overflows instead of // content rendering below the fold (#9305 reviews R11-6, R14-1). + // Banner-only remnants never cross back: there is no conversation to + // follow, and the surviving banner carries the park mark across the + // collapse (#9305 review R17-1). ((listGrew && (isStickingToBottom || (wasAtBottom && !clampParked))) || (clampParked && contentPreviouslyFit && + data.length > 1 && totalHeight > scrollableContainerHeight) || // A shrink landing in the same render must reach the drop/re-anchor // branch below, not be preempted here on the stale render-time @@ -524,9 +536,24 @@ function VirtualizedList( // so one installed at rest reads as clampParked forever and kills // growth auto-follow, defending nothing — the re-stick gate is // already blocked by `!contentPreviouslyFit` (#9305 reviews R8-1, - // R10-1). - if (newScrollTop > 0 || (!isStickingToBottom && data.length > 1)) { - reAnchorClampMark.current = newAnchor; + // R10-1). An out-of-range carried anchor with a multi-item remnant + // is a wholesale dataset swap (/resume — ScrollableList carries no + // key) or a truncation below the anchor: the park lands on content + // the user never scrolled, and a mark there would keep follow dead + // in the new dataset; the banner-only collapse keeps its mark + // (#9305 review R17-2). + const swapEntry = scrollAnchor.index >= data.length && data.length > 1; + const markItem = data[newAnchor.index]; + if ( + markItem !== undefined && + ((newScrollTop > 0 && !swapEntry) || + (newScrollTop === 0 && !isStickingToBottom && data.length > 1)) + ) { + reAnchorClampMark.current = { + index: newAnchor.index, + offset: newAnchor.offset, + key: keyExtractor(markItem, newAnchor.index), + }; } setScrollAnchor(newAnchor); } @@ -541,7 +568,8 @@ function VirtualizedList( prevScrollTop.current = actualScrollTop; prevContainerHeight.current = scrollableContainerHeight; }, [ - data.length, + data, + keyExtractor, totalHeight, actualScrollTop, scrollableContainerHeight, From 6c1ada41bed30e2cfd98d8b6f140fbf06bcff875 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Sat, 22 Aug 2026 10:19:48 +0000 Subject: [PATCH 11/11] fix(ui): reset VP scroll state on session swap, heal park-mark follow (#9305) --- .../src/ui/components/MainContent.test.tsx | 46 +++++ .../cli/src/ui/components/MainContent.tsx | 7 + .../shared/VirtualizedList.test.tsx | 189 +++++++++++++++++- .../ui/components/shared/VirtualizedList.tsx | 53 +++-- 4 files changed, 264 insertions(+), 31 deletions(-) diff --git a/packages/cli/src/ui/components/MainContent.test.tsx b/packages/cli/src/ui/components/MainContent.test.tsx index 8b9a89e59e4..c0eed7f962b 100644 --- a/packages/cli/src/ui/components/MainContent.test.tsx +++ b/packages/cli/src/ui/components/MainContent.test.tsx @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { useEffect } from 'react'; import type React from 'react'; import { describe, expect, it, vi } from 'vitest'; import { render } from 'ink-testing-library'; @@ -27,6 +28,7 @@ const staticItemsSpy = vi.fn(); const historyItemDisplayPropsSpy = vi.fn(); const appHeaderSpy = vi.fn(); const scrollableListPropsSpy = vi.fn(); +const scrollableListMountSpy = vi.fn(); // Records every render's props so tests can assert layout props // (e.g. the pending-region maxHeight backstop) without coupling to ink's // Yoga internals. @@ -113,6 +115,9 @@ vi.mock('./shared/ScrollableList.js', async () => { renderItem: (info: { item: { id: number }; index: number }) => unknown; }) => { scrollableListPropsSpy(props); + useEffect(() => { + scrollableListMountSpy(); + }, []); // Drive renderItem once per item so historyItemDisplayPropsSpy fires — // mirrors what the real VirtualizedList does for the visible window. return ( @@ -884,6 +889,47 @@ describe('', () => { expect(lastFrame()).toMatch(/VP_ITEM:1[\s\S]*VP_ITEM:2/); }); + it('remounts the VP list when the session id changes (/clear, /resume) (#9305)', () => { + // /clear and /resume each start a new session (startNewSession swaps + // the sessionId), and the whole dataset is replaced with it. The VP + // list must remount on that boundary so carried scroll state — park + // marks, sticking, the anchor — resets by construction instead of + // leaking into the new session (#9305 review R18-1). Same-session + // re-renders must NOT remount: scroll state survives ordinary + // streaming updates. + scrollableListMountSpy.mockClear(); + + const vpState = (sessionId: string) => + createUIState({ + useTerminalBuffer: true, + history: [{ id: 1, type: 'user', text: 'hello' }], + sessionStats: { + sessionId, + lastPromptTokenCount: 0, + } as UIState['sessionStats'], + }); + const wrap = (uiState: UIState) => ( + + + + + + + + + + ); + + const { rerender } = render(wrap(vpState('session-A'))); + expect(scrollableListMountSpy).toHaveBeenCalledTimes(1); + + rerender(wrap(vpState('session-A'))); + expect(scrollableListMountSpy).toHaveBeenCalledTimes(1); + + rerender(wrap(vpState('session-B'))); + expect(scrollableListMountSpy).toHaveBeenCalledTimes(2); + }); + // Shared fixtures for the #9420 collapse tests. A tool batch renders // twice transiently — the committed history copy plus the live pending // copy — and both copies carry the same scheduler-minted `batchId`. diff --git a/packages/cli/src/ui/components/MainContent.tsx b/packages/cli/src/ui/components/MainContent.tsx index 45279aa35f3..c2d9d984b96 100644 --- a/packages/cli/src/ui/components/MainContent.tsx +++ b/packages/cli/src/ui/components/MainContent.tsx @@ -140,6 +140,7 @@ export const MainContent = ({ footerRef }: MainContentProps) => { staticAreaMaxItemHeight, availableTerminalHeight, historyRemountKey, + sessionStats, } = uiState; // Filter out items whose display is suppressed (e.g. /history collapse). @@ -497,6 +498,12 @@ export const MainContent = ({ footerRef }: MainContentProps) => { return ( ', () => { ); }); - it('invalidates the clamp mark when a dataset swap keeps the anchor in range', async () => { - // The mark identifies the parked item, not just coordinates: a /resume - // swap whose carried anchor stays in range and inside the new max - // scroll bypasses the drop branch entirely, and a coordinate-only mark - // would survive the swap and suppress follow in the new session - // (#9305 review R17-3). + it('keeps follow alive when a replacement lands under the carried anchor', async () => { + // A wholesale replacement whose items sit under the carried anchor + // bypasses the drop branch entirely. In production this shape remounts + // by session key (MainContent), but the component must still hold: the + // park mark — now positional, no item key — must not suppress follow + // in the replaced dataset; growth from a live-bottom park re-engages + // (#9305 reviews R17-3, R18-1). type RefShape = VirtualizedListRef; let listRef: RefShape | null = null; let items = makeItems(20); @@ -1312,9 +1313,10 @@ describe('', () => { Array.from({ length: 10 }, (_, i) => `item-${i + 2}`), ); - // In-place session switch whose items sit under the carried anchor: - // the anchor stays in range inside the new max scroll, so no drop - // render fires. The stale mark must not survive the swap. + // Replacement whose items sit under the carried anchor: the anchor + // stays in range, so no drop render fires. The park sat at the live + // bottom of a multi-item remnant, so the arrival of the taller dataset + // re-engages follow (not the old gate latch from a key-cleared mark). items = Array.from({ length: 20 }, (_, i) => ({ id: 100 + i, label: `it-${100 + i}`, @@ -2288,6 +2290,175 @@ describe('', () => { expect(listRef!.getScrollIndex()).toBe(24); }); }); + + it('keeps the park mark across a re-key of the parked item (R18-1 F2)', async () => { + // The park mark must survive a key transition of the item it sits on: + // pending items re-key on commit (p-N → h-N), and the old key-based + // validation cleared the mark while the user still sat at the parked + // position. With the mark gone, the fit→overflow crossing arm (which + // requires the parked signal) dies and the shrunken content clips + // instead of re-engaging follow (#9305 review R18-1). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + let items = makeItems(20); + let height = 10; + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={height} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-5); + }); + rerender(); + await act(async () => {}); + + // Shrink in place until the remnant fits: the released park at {0,0} + // installs the positional mark. + items = makeItems(8); + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 8 }, (_, i) => `item-${i}`), + ); + + // Re-key the parked item (pending → commit re-key keeps the position). + items = [{ id: 900, label: 'item-0' }, ...makeItems(8).slice(1)]; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 8 }, (_, i) => `item-${i}`), + ); + + // The container shrink crosses the fitting remnant into overflow: the + // parked signal must survive the re-key so follow re-engages. + height = 6; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual( + Array.from({ length: 6 }, (_, i) => `item-${i + 2}`), + ); + }); + + it('re-engages follow when growth lands on a live-bottom park (R18-1 F7)', async () => { + // An in-place shrink of a scrolled-away (bottom-pixels) viewport parks + // it at the live bottom and installs the park mark. The park is not a + // user scroll, so the very next render must not latch sticking — but + // once new content arrives the park is the bottom of a live multi-item + // conversation, and follow must come back. The old mark suppressed + // every re-follow path forever: the whole next reply streamed below the + // fold until a manual scroll (#9305 review R18-1). + type RefShape = VirtualizedListRef; + let listRef: RefShape | null = null; + const tall = (n: number, from = 0): Item[] => + Array.from({ length: n }, (_, i) => ({ + id: from + i, + label: `X${from + i}-0\nX${from + i}-1\nX${from + i}-2`, + })); + let items = tall(12); + + function Wrapper() { + const ref = useRef(null); + if (ref.current) listRef = ref.current; + return ( + + ref={ref} + data={items} + renderItem={renderItem} + estimatedItemHeight={estimatedItemHeight} + keyExtractor={keyExtractor} + initialScrollIndex={SCROLL_TO_ITEM_END} + containerHeight={10} + width={40} + showScrollbar={false} + /> + ); + } + + const { lastFrame, rerender } = render(); + rerender(); + await act(async () => {}); + + act(() => { + listRef!.scrollBy(-1); + }); + rerender(); + await act(async () => {}); + + // In-place shrink below the viewport (new keys: cached heights do not + // shrink under a stable key): the re-anchor clamp parks the viewport + // at the live bottom of the remnant. + items = [ + ...Array.from({ length: 8 }, (_, i) => ({ + id: 100 + i, + label: `x${i}`, + })), + ...tall(1, 8), + ...Array.from({ length: 3 }, (_, i) => ({ + id: 109 + i, + label: `x${9 + i}`, + })), + ]; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'x4', + 'x5', + 'x6', + 'x7', + 'X8-0', + 'X8-1', + 'X8-2', + 'x9', + 'x10', + 'x11', + ]); + + // Growth must re-engage follow from the live-bottom park. + items = [...items, { id: 120, label: 'g0' }]; + rerender(); + rerender(); + await act(async () => {}); + + expect((lastFrame() ?? '').split('\n')).toEqual([ + 'x5', + 'x6', + 'x7', + 'X8-0', + 'X8-1', + 'X8-2', + 'x9', + 'x10', + 'x11', + 'g0', + ]); + }); }); // Hoisted to module scope so every harness rerender hands VirtualizedList diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index dccfafe0ad2..651db24dc4b 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -407,17 +407,23 @@ function VirtualizedList( const prevScrollTop = useRef(actualScrollTop); const prevContainerHeight = useRef(scrollableContainerHeight); // Set by the re-anchor branch when it clamps a scrolled-away viewport to - // the new bottom, holding the anchor the clamp installed plus the key of - // the item parked there. While the current anchor still sits where the - // clamp parked it, that position is content-driven, so the re-stick gate - // must not read it as the user having scrolled to the bottom. Any scroll - // moves the anchor, and the mismatch clears the mark; a wholesale dataset - // swap (/resume — ScrollableList carries no key) changes the key at the - // mark's index and clears it the same way (#9305 review R17-3). + // the new bottom, holding the anchor the clamp installed. While the + // current anchor still sits where the clamp parked it, that position is + // content-driven, so the re-stick gate must not read it as the user + // having scrolled to the bottom. Any scroll moves the anchor, and the + // mismatch clears the mark. The mark deliberately carries no item key: + // pending items re-key on commit and the banner key is constant, so a + // key comparison cleared (or kept) the mark for the wrong reasons while + // the user still sat at the parked position (#9305 review R18-1). + // Dataset swaps (/clear, /resume) never reach the mark because + // MainContent keys the list by session and remounts it. `allowFollow` + // records whether the park landed in a live multi-item conversation, + // where growth may re-engage follow from a live-bottom park; a + // banner-only remnant must stay released (#9305 reviews R6-2, R17-1). const reAnchorClampMark = useRef<{ index: number; offset: number; - key: string; + allowFollow: boolean; } | null>(null); useLayoutEffect(() => { @@ -440,13 +446,11 @@ function VirtualizedList( // clamp is not a user-driven bottom either (#9305): suppress the flip // while the anchor still matches the clamp mark. const clampMark = reAnchorClampMark.current; - const clampMarkItem = clampMark ? data[clampMark.index] : undefined; const clampParked = clampMark !== null && - clampMarkItem !== undefined && + data[clampMark.index] !== undefined && scrollAnchor.index === clampMark.index && - scrollAnchor.offset === clampMark.offset && - keyExtractor(clampMarkItem, clampMark.index) === clampMark.key; + scrollAnchor.offset === clampMark.offset; if (clampMark !== null && !clampParked) { reAnchorClampMark.current = null; } @@ -475,15 +479,22 @@ function VirtualizedList( // overflow by any shape: length growth, in-place height growth, or a // container shrink. While the content fit, the user could see // everything, so follow must come back once it overflows instead of - // content rendering below the fold (#9305 reviews R11-6, R14-1). + // content rendering below the fold (#9305 reviews R11-6, R14-1). A + // park at the live bottom of a multi-item remnant re-engages on + // growth too: it IS the bottom of a live conversation, and the mark + // would otherwise suppress follow forever (#9305 review R18-1). // Banner-only remnants never cross back: there is no conversation to - // follow, and the surviving banner carries the park mark across the - // collapse (#9305 review R17-1). + // follow (#9305 review R17-1). ((listGrew && (isStickingToBottom || (wasAtBottom && !clampParked))) || (clampParked && contentPreviouslyFit && data.length > 1 && totalHeight > scrollableContainerHeight) || + (clampMark !== null && + clampParked && + clampMark.allowFollow && + wasScrolledToBottomPixels && + totalHeight > prevTotalHeight.current) || // A shrink landing in the same render must reach the drop/re-anchor // branch below, not be preempted here on the stale render-time // sticking flag (#9305 review R11-1). @@ -537,11 +548,10 @@ function VirtualizedList( // growth auto-follow, defending nothing — the re-stick gate is // already blocked by `!contentPreviouslyFit` (#9305 reviews R8-1, // R10-1). An out-of-range carried anchor with a multi-item remnant - // is a wholesale dataset swap (/resume — ScrollableList carries no - // key) or a truncation below the anchor: the park lands on content - // the user never scrolled, and a mark there would keep follow dead - // in the new dataset; the banner-only collapse keeps its mark - // (#9305 review R17-2). + // is a truncation below the anchor (dataset swaps remount the list + // by session key, #9305 review R18-1): the park lands on content + // the user never scrolled, and a mark there would keep follow dead; + // the banner-only collapse keeps its mark (#9305 review R17-2). const swapEntry = scrollAnchor.index >= data.length && data.length > 1; const markItem = data[newAnchor.index]; if ( @@ -552,7 +562,7 @@ function VirtualizedList( reAnchorClampMark.current = { index: newAnchor.index, offset: newAnchor.offset, - key: keyExtractor(markItem, newAnchor.index), + allowFollow: data.length > 1, }; } setScrollAnchor(newAnchor); @@ -569,7 +579,6 @@ function VirtualizedList( prevContainerHeight.current = scrollableContainerHeight; }, [ data, - keyExtractor, totalHeight, actualScrollTop, scrollableContainerHeight,