📌 fix: Pin the PTC Trace Through Its Final Settle

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBRQUsPXSvDBnjrSDoXWHP
This commit is contained in:
Claude 2026-08-22 21:10:52 +00:00
parent 4bf68e18a4
commit 085a83fe5f
No known key found for this signature in database
2 changed files with 48 additions and 3 deletions

View file

@ -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(<Probe content="ab" active={false} expanded />);
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(<Probe content="ab" active={false} expanded />);
expect(state.writes).toHaveLength(0);
});
it('pins only the settling change, not the ones after it', () => {
const { rerender, state } = setup();
rerender(<Probe content="ab" active={false} expanded />);
expect(state.writes).toEqual([900]);
state.scrollHeight = 1200;
rerender(<Probe content="abc" active={false} expanded />);
expect(state.writes).toEqual([900]);
});
it('detaches when the user scrolls up beyond the follow threshold', () => {
const { rerender, pane, state } = setup();
state.scrollTop = 100;

View file

@ -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<T extends HTMLElement>(
content: string | readonly ReactNode[],
@ -42,6 +50,8 @@ export default function useFollowScroll<T extends HTMLElement>(
): { ref: RefObject<T>; onScroll: UIEventHandler<T> } {
const ref = useRef<T>(null);
const followRef = useRef(true);
const wasActiveRef = useRef(active);
const previousContentRef = useRef(content);
const onScroll = useCallback<UIEventHandler<T>>((event) => {
const el = event.currentTarget;
@ -49,7 +59,14 @@ export default function useFollowScroll<T extends HTMLElement>(
}, []);
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;