From 085a83fe5f85072ed0c2a316fee24733c7cf6b59 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 21:10:52 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=8C=20fix:=20Pin=20the=20PTC=20Trace?= =?UTF-8?q?=20Through=20Its=20Final=20Settle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fourth Codex pass on `4bf68e1`, one P2 finding. `useFollowScroll` returned early whenever `active` was false, so the one change it most needed to follow was the one it skipped. A failing inner call settles by appending its error line in the same commit that clears the last running row: the content grows and the stream ends together, and the pin that would have revealed that line never fired. On an expanded, bottom-pinned pane the failure — the row a reader most wants — stayed below the fold. The falling edge of `active` now pins too, but only when the content changed with it. Ending a stream on its own still leaves the pane where the reader left it, which is what the existing contract promises and what the sibling code and command panes rely on; a reader who has scrolled up is untouched either way. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP --- .../Parts/__tests__/useFollowScroll.spec.tsx | 28 +++++++++++++++++++ .../Messages/Content/Parts/useFollowScroll.ts | 23 +++++++++++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/useFollowScroll.spec.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/useFollowScroll.spec.tsx index 8573d53b64..4a996c7ca9 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/useFollowScroll.spec.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/useFollowScroll.spec.tsx @@ -112,6 +112,34 @@ describe('useFollowScroll', () => { expect(state.writes).toEqual([900]); }); + /** A failing PTC call settles by appending its error line in the same + * commit that clears the last running row, so the growth and the end of + * the stream arrive together. */ + it('pins one last time when the change that ends the stream also adds content', () => { + const { rerender, state } = setup(); + state.scrollHeight = 1200; + rerender(); + expect(state.writes).toEqual([1200]); + }); + + it('does not pin the settling change for a reader who scrolled up', () => { + const { rerender, pane, state } = setup(); + state.scrollTop = 100; + fireEvent.scroll(pane); + state.scrollHeight = 1200; + rerender(); + expect(state.writes).toHaveLength(0); + }); + + it('pins only the settling change, not the ones after it', () => { + const { rerender, state } = setup(); + rerender(); + expect(state.writes).toEqual([900]); + state.scrollHeight = 1200; + rerender(); + expect(state.writes).toEqual([900]); + }); + it('detaches when the user scrolls up beyond the follow threshold', () => { const { rerender, pane, state } = setup(); state.scrollTop = 100; diff --git a/client/src/components/Chat/Messages/Content/Parts/useFollowScroll.ts b/client/src/components/Chat/Messages/Content/Parts/useFollowScroll.ts index 2cf0d6ca4b..37c3c820d4 100644 --- a/client/src/components/Chat/Messages/Content/Parts/useFollowScroll.ts +++ b/client/src/components/Chat/Messages/Content/Parts/useFollowScroll.ts @@ -32,8 +32,16 @@ const FOLLOW_THRESHOLD_PX = 40; * would open at the tail. Gated, it opens reading from the top, while an * expand mid-stream pins immediately. The pin's own scroll event measures * distance zero and keeps the attached state, so programmatic scrolls - * need no special-casing. Finished panes (`active === false`) are never - * scrolled. + * need no special-casing. + * + * A finished pane (`active === false`) is never scrolled, with one + * exception: the very change that ends the stream. That change often adds + * content — a failing PTC call settles by appending its error line in the + * same commit that clears the last running row — and gating on `active` + * alone would drop exactly the pin that content needs, leaving the final + * line below the fold. So the falling edge of `active` still pins, but + * only when the content changed with it; ending the stream on its own + * leaves the pane where the reader left it. */ export default function useFollowScroll( content: string | readonly ReactNode[], @@ -42,6 +50,8 @@ export default function useFollowScroll( ): { ref: RefObject; onScroll: UIEventHandler } { const ref = useRef(null); const followRef = useRef(true); + const wasActiveRef = useRef(active); + const previousContentRef = useRef(content); const onScroll = useCallback>((event) => { const el = event.currentTarget; @@ -49,7 +59,14 @@ export default function useFollowScroll( }, []); useLayoutEffect(() => { - if (!active || !expanded || !followRef.current) { + const settling = wasActiveRef.current && !Object.is(previousContentRef.current, content); + previousContentRef.current = content; + wasActiveRef.current = active; + + if (!expanded || !followRef.current) { + return; + } + if (!active && !settling) { return; } const el = ref.current;