From ef9e2fa11ee57523bde53ac336fd959a2dfbc76f Mon Sep 17 00:00:00 2001 From: ApolloBot Date: Fri, 22 May 2026 23:02:28 +0000 Subject: [PATCH 1/4] fix(chat): kick load-older from items effect when viewport can't scroll --- .../chat/transcript/use-transcript-scroll.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts b/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts index 86a54115cdd..f8c5601a989 100644 --- a/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts +++ b/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts @@ -394,6 +394,42 @@ export function useTranscriptScroll( ) { setShowScrollToLatest(classification.showScrollToLatest); } + + // Underfilled-viewport kick. The scroll-driven path in + // `handleScroll` is the only other place that fires `onLoadOlder`, + // and it requires a scroll event to do so. When the initial page + // (or a still-too-short prepend) leaves `scrollHeight === + // clientHeight`, the user can't scroll, no scroll event ever + // fires, and the transcript gets stuck at the top with + // `hasMore=true` and `shouldLoadOlder=true` forever. Kick a load + // directly from the items effect to recover. + // + // The classification already gates on `hasMore && !isLoadingOlder + // && hasConversation`, so this won't double-fire or spam after + // history is exhausted. As subsequent pages land the effect re- + // runs and either kicks again (still underfilled) or stops + // (overflow achieved, or `hasMore=false`). + // + // Anchor handling: during the auto-pin window (conversation + // switch / "Go to Newest" / user submit) the content resize + // observer will re-pin to latest after the prepend lands, so + // saving an anchor here would just be overridden by — and fight + // with — auto-pin. Outside the window we save the anchor exactly + // like `handleScroll` does so the reader's row stays visually + // stable across the prepend. + if (classification.shouldLoadOlder) { + if (!shouldAutoPinRef.current) { + const firstItem = latestRef.current.items[0]; + if (firstItem) { + savedAnchorRef.current = { + key: firstItem.key, + scrollTop: el.scrollTop, + scrollHeight: el.scrollHeight, + }; + } + } + latestRef.current.onLoadOlder(); + } } }, [items, conversationKey, transcriptRef]); From 9ab8e1f1b31fdf6f7cd514bf1e530367e5eef236 Mon Sep 17 00:00:00 2001 From: ApolloBot Date: Fri, 22 May 2026 23:03:44 +0000 Subject: [PATCH 2/4] test(chat): regression for items-effect underfilled-viewport kick Captures the exact scenario from the bug report (scrollHeight === clientHeight, scrollTop=0, hasMore=true, isLoadingOlder=false, shouldLoadOlder=true) and locks in the dispatch contract for the items effect's load-older kick: it fires when classify says so, and stops cleanly once isLoadingOlder=true, hasMore=false, or the viewport overflows the 200 px threshold. --- .../transcript/use-transcript-scroll.test.ts | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts b/apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts index 3ee469e61db..9c4649576c9 100644 --- a/apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts +++ b/apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts @@ -454,3 +454,87 @@ describe("integration — handleScroll-style dispatch via pure helpers", () => { expect(handle.calls.scrollToLatest.length).toBe(0); }); }); + +// Regression for the items-effect underfilled-viewport kick. Captured +// scrollState from the bug report (2026-05-22): +// +// { scrollTop: 0, scrollHeight: 1370, clientHeight: 1370, +// hasMore: true, isLoadingOlder: false, itemCount: 2, +// shouldLoadOlder: true, diagnosis: "NEAR TOP ... but NOT loading" } +// +// scrollHeight === clientHeight means the user CANNOT scroll. handleScroll +// can never fire, so without an items-effect kick the load-older path +// stays gated forever even though `shouldLoadOlder=true`. These tests +// lock in the precise classification and the dispatch contract. +describe("integration — items-effect dispatch on underfilled viewport", () => { + test("scrollHeight === clientHeight with hasMore=true classifies as shouldLoadOlder=true", () => { + const c = classifyScrollPosition( + { scrollTop: 0, scrollHeight: 1370, clientHeight: 1370 }, + { hasMore: true, isLoadingOlder: false, hasConversation: true }, + ); + expect(c.shouldLoadOlder).toBe(true); + // Underfilled also reads as pinned-to-latest (distanceFromBottom=0 + // is well under the 64 px threshold) — the items effect needs to be + // willing to load AND keep `isPinned=true`. + expect(c.isPinned).toBe(true); + expect(c.distanceFromBottom).toBe(0); + }); + + test("items-effect-style dispatch fires onLoadOlder when classify says so", () => { + // Mirrors the hook's items-effect path: read DOM metrics, classify, + // and if `shouldLoadOlder` is true, call `onLoadOlder()`. This is + // the call site the bug was missing — `handleScroll` had it, + // the items effect did not. + const onLoadOlder = mock(() => {}); + const c = classifyScrollPosition( + { scrollTop: 0, scrollHeight: 1370, clientHeight: 1370 }, + { hasMore: true, isLoadingOlder: false, hasConversation: true }, + ); + if (c.shouldLoadOlder) onLoadOlder(); + expect(onLoadOlder).toHaveBeenCalledTimes(1); + }); + + test("items-effect dispatch does NOT fire when a load is already in flight", () => { + // After the first kick lands, `isLoadingOlder` flips to true. The + // next items change (e.g. an unrelated re-render) must NOT fire a + // second kick — `classification.shouldLoadOlder` is the only gate + // and it must reflect that. + const onLoadOlder = mock(() => {}); + const c = classifyScrollPosition( + { scrollTop: 0, scrollHeight: 1370, clientHeight: 1370 }, + { hasMore: true, isLoadingOlder: true, hasConversation: true }, + ); + if (c.shouldLoadOlder) onLoadOlder(); + expect(onLoadOlder).not.toHaveBeenCalled(); + }); + + test("items-effect dispatch stops kicking once viewport overflows", () => { + // After enough older pages prepend, scrollHeight exceeds + // clientHeight + LOAD_OLDER_THRESHOLD_PX (200 px). The cascade + // terminates: classify reports shouldLoadOlder=false and the kick + // does not fire. + const onLoadOlder = mock(() => {}); + const c = classifyScrollPosition( + // scrollTop bumped well below the 200 px threshold (the auto-pin + // window will have re-pinned to latest, so scrollTop sits near + // the bottom of an overflowing viewport). + { scrollTop: 5000, scrollHeight: 6370, clientHeight: 1370 }, + { hasMore: true, isLoadingOlder: false, hasConversation: true }, + ); + if (c.shouldLoadOlder) onLoadOlder(); + expect(onLoadOlder).not.toHaveBeenCalled(); + }); + + test("items-effect dispatch stops kicking once hasMore=false", () => { + // Conversation has fewer messages than fit in the viewport AND no + // older history exists. The cascade must terminate cleanly with no + // further calls. + const onLoadOlder = mock(() => {}); + const c = classifyScrollPosition( + { scrollTop: 0, scrollHeight: 1370, clientHeight: 1370 }, + { hasMore: false, isLoadingOlder: false, hasConversation: true }, + ); + if (c.shouldLoadOlder) onLoadOlder(); + expect(onLoadOlder).not.toHaveBeenCalled(); + }); +}); From 39e1a8114fba0929ed5faa266aac91d586b3ed4f Mon Sep 17 00:00:00 2001 From: "vellum-apollo-bot[bot]" <242025090+vellum-apollo-bot[bot]@users.noreply.github.com> Date: Sat, 23 May 2026 14:42:44 +0000 Subject: [PATCH 3/4] chore(chat): trim verbose comment on underfilled-viewport kick --- .../chat/transcript/use-transcript-scroll.ts | 24 ++----------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts b/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts index f8c5601a989..5b229fbbcf6 100644 --- a/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts +++ b/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts @@ -395,28 +395,8 @@ export function useTranscriptScroll( setShowScrollToLatest(classification.showScrollToLatest); } - // Underfilled-viewport kick. The scroll-driven path in - // `handleScroll` is the only other place that fires `onLoadOlder`, - // and it requires a scroll event to do so. When the initial page - // (or a still-too-short prepend) leaves `scrollHeight === - // clientHeight`, the user can't scroll, no scroll event ever - // fires, and the transcript gets stuck at the top with - // `hasMore=true` and `shouldLoadOlder=true` forever. Kick a load - // directly from the items effect to recover. - // - // The classification already gates on `hasMore && !isLoadingOlder - // && hasConversation`, so this won't double-fire or spam after - // history is exhausted. As subsequent pages land the effect re- - // runs and either kicks again (still underfilled) or stops - // (overflow achieved, or `hasMore=false`). - // - // Anchor handling: during the auto-pin window (conversation - // switch / "Go to Newest" / user submit) the content resize - // observer will re-pin to latest after the prepend lands, so - // saving an anchor here would just be overridden by — and fight - // with — auto-pin. Outside the window we save the anchor exactly - // like `handleScroll` does so the reader's row stays visually - // stable across the prepend. + // Underfilled viewport: no scroll event can fire, so kick load-older + // from here. Skip the anchor save during auto-pin (resize observer re-pins). if (classification.shouldLoadOlder) { if (!shouldAutoPinRef.current) { const firstItem = latestRef.current.items[0]; From e021b2ce54654a849a2b2d8f07caa2eecfeb6704 Mon Sep 17 00:00:00 2001 From: "vellum-apollo-bot[bot]" <242025090+vellum-apollo-bot[bot]@users.noreply.github.com> Date: Sat, 23 May 2026 15:11:04 +0000 Subject: [PATCH 4/4] fix(chat): read loading flags from closure to address Codex P1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The items-effect classification was reading hasMore/isLoadingOlder/ conversationKey/items from latestRef, but latestRef is refreshed by a useEffect that runs AFTER this useLayoutEffect. On the prepend render (isLoadingOlder: true → false), the stale ref kept the gate as true, shouldLoadOlder evaluated to false, and the kick was silently skipped — stalling conversations that need multiple older pages to overflow. Switch to closure reads and add the flags to the dep array so the effect also re-runs on loading-state transitions. Add a regression test that walks the false→true→false sequence and asserts a second kick fires when the viewport is still underfilled. --- .../transcript/use-transcript-scroll.test.ts | 37 +++++++++++++++++++ .../chat/transcript/use-transcript-scroll.ts | 34 ++++++++++------- 2 files changed, 58 insertions(+), 13 deletions(-) diff --git a/apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts b/apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts index 9c4649576c9..207ed538099 100644 --- a/apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts +++ b/apps/web/src/domains/chat/transcript/use-transcript-scroll.test.ts @@ -537,4 +537,41 @@ describe("integration — items-effect dispatch on underfilled viewport", () => if (c.shouldLoadOlder) onLoadOlder(); expect(onLoadOlder).not.toHaveBeenCalled(); }); + + test("chain-load: false→true→false sequence kicks again when still underfilled", () => { + // Codex P1 regression. The dispatch site must observe the CURRENT + // render's isLoadingOlder flag, not a stale ref. Sequence: + // 1. Initial: isLoadingOlder=false, items underfill → kick. + // 2. Loading: isLoadingOlder=true, same items → no kick. + // 3. Prepended: isLoadingOlder=false, items grew but still + // underfill → MUST kick again to chain-load. + // If the dispatch reads a stale flag from the previous render, step 3 + // sees isLoadingOlder=true and silently skips the kick, stranding + // conversations that need multiple older pages to overflow. + const onLoadOlder = mock(() => {}); + + // Step 1: initial, underfilled + let c = classifyScrollPosition( + { scrollTop: 0, scrollHeight: 1370, clientHeight: 1370 }, + { hasMore: true, isLoadingOlder: false, hasConversation: true }, + ); + if (c.shouldLoadOlder) onLoadOlder(); + expect(onLoadOlder).toHaveBeenCalledTimes(1); + + // Step 2: loading in flight + c = classifyScrollPosition( + { scrollTop: 0, scrollHeight: 1370, clientHeight: 1370 }, + { hasMore: true, isLoadingOlder: true, hasConversation: true }, + ); + if (c.shouldLoadOlder) onLoadOlder(); + expect(onLoadOlder).toHaveBeenCalledTimes(1); + + // Step 3: prepend landed but still underfilled + c = classifyScrollPosition( + { scrollTop: 0, scrollHeight: 2200, clientHeight: 2200 }, + { hasMore: true, isLoadingOlder: false, hasConversation: true }, + ); + if (c.shouldLoadOlder) onLoadOlder(); + expect(onLoadOlder).toHaveBeenCalledTimes(2); + }); }); diff --git a/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts b/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts index 5b229fbbcf6..bcbd6b516d5 100644 --- a/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts +++ b/apps/web/src/domains/chat/transcript/use-transcript-scroll.ts @@ -369,8 +369,10 @@ export function useTranscriptScroll( // browser does NOT fire a scroll event when scrollHeight grows under // a stationary scrollTop (the streaming case), so without this the // "Go to Newest" pill would only surface once the user touched the - // scroll wheel. Reading via the latestRef avoids putting the - // hasMore/isLoadingOlder/conversationKey flags in the dep array. + // scroll wheel. Read flags from closure (not latestRef) so the + // prepend render sees the just-flipped isLoadingOlder=false instead + // of the previous render's snapshot; latestRef is refreshed by a + // later useEffect that hasn't fired yet at this point. const el = transcriptRef.current?.getScrollElement(); if (el) { const classification = classifyScrollPosition( @@ -380,18 +382,15 @@ export function useTranscriptScroll( clientHeight: el.clientHeight, }, { - hasMore: latestRef.current.hasMore, - isLoadingOlder: latestRef.current.isLoadingOlder, - hasConversation: latestRef.current.conversationKey !== null, + hasMore, + isLoadingOlder, + hasConversation: conversationKey !== null, }, ); - if (classification.isPinned !== latestRef.current.isPinnedToLatest) { + if (classification.isPinned !== isPinnedToLatest) { setIsPinnedToLatest(classification.isPinned); } - if ( - classification.showScrollToLatest !== - latestRef.current.showScrollToLatest - ) { + if (classification.showScrollToLatest !== showScrollToLatest) { setShowScrollToLatest(classification.showScrollToLatest); } @@ -399,7 +398,7 @@ export function useTranscriptScroll( // from here. Skip the anchor save during auto-pin (resize observer re-pins). if (classification.shouldLoadOlder) { if (!shouldAutoPinRef.current) { - const firstItem = latestRef.current.items[0]; + const firstItem = items[0]; if (firstItem) { savedAnchorRef.current = { key: firstItem.key, @@ -408,10 +407,19 @@ export function useTranscriptScroll( }; } } - latestRef.current.onLoadOlder(); + onLoadOlder(); } } - }, [items, conversationKey, transcriptRef]); + }, [ + items, + conversationKey, + transcriptRef, + hasMore, + isLoadingOlder, + onLoadOlder, + isPinnedToLatest, + showScrollToLatest, + ]); // ----------------------------------------------------------------------- // Container resize re-pin. When the scroll container resizes (e.g. the