diff --git a/apps/web/src/domains/chat/components/chat-route-content.tsx b/apps/web/src/domains/chat/components/chat-route-content.tsx index 824bb5be20e..c6ec74b8fe2 100644 --- a/apps/web/src/domains/chat/components/chat-route-content.tsx +++ b/apps/web/src/domains/chat/components/chat-route-content.tsx @@ -1125,6 +1125,9 @@ export function ChatRouteContent({ const chatTranscriptProps: TranscriptProps = { items: transcriptItems, conversationId: activeConversationId, + hasMore: transcriptPagination.hasMore, + isLoadingOlder: transcriptPagination.isLoadingOlder, + onLoadOlder: loadOlder, assistantDisplayName: assistantName?.trim() || undefined, expandedToolCallIds: expandedToolCallIdsRef.current, onOpenRuleEditor: handleOpenRuleEditorForToolCall, diff --git a/apps/web/src/domains/chat/transcript/transcript-scroll.test.ts b/apps/web/src/domains/chat/transcript/transcript-scroll.test.ts index c9a844a1af1..4db8804624f 100644 --- a/apps/web/src/domains/chat/transcript/transcript-scroll.test.ts +++ b/apps/web/src/domains/chat/transcript/transcript-scroll.test.ts @@ -10,7 +10,10 @@ import { describe, expect, test } from "bun:test"; -import { attachSnapToLatest } from "@/domains/chat/transcript/transcript-scroll"; +import { + attachLoadOlderOnTop, + attachSnapToLatest, +} from "@/domains/chat/transcript/transcript-scroll"; type Listener = (...args: unknown[]) => void; @@ -213,3 +216,108 @@ describe("attachSnapToLatest", () => { stop(); }); }); + +// ---- attachLoadOlderOnTop -------------------------------------------------- + +describe("attachLoadOlderOnTop", () => { + test("fires onLoadOlder on the initial ResizeObserver tick when scrolled to top", () => { + installFakeResizeObserver(); + const container = createFakeElement(5000); + const content = createFakeElement(0); + container.scrollTop = 0; + let calls = 0; + + attachLoadOlderOnTop({ + container: container as unknown as HTMLElement, + content: content as unknown as HTMLElement, + onLoadOlder: () => { + calls += 1; + }, + }); + FakeResizeObserver.instances[0].fire(); + + expect(calls).toBe(1); + uninstallFakeResizeObserver(); + }); + + test("does NOT fire when scrolled past the 200px threshold", () => { + installFakeResizeObserver(); + const container = createFakeElement(5000); + const content = createFakeElement(0); + container.scrollTop = 1000; + let calls = 0; + + attachLoadOlderOnTop({ + container: container as unknown as HTMLElement, + content: content as unknown as HTMLElement, + onLoadOlder: () => { + calls += 1; + }, + }); + FakeResizeObserver.instances[0].fire(); + + expect(calls).toBe(0); + uninstallFakeResizeObserver(); + }); + + test("fires on every ResizeObserver tick while near the top (streaming, chain-load)", () => { + installFakeResizeObserver(); + const container = createFakeElement(5000); + const content = createFakeElement(0); + container.scrollTop = 100; + let calls = 0; + + attachLoadOlderOnTop({ + container: container as unknown as HTMLElement, + content: content as unknown as HTMLElement, + onLoadOlder: () => { + calls += 1; + }, + }); + + // Each RO tick (streaming chunk arriving, image load, etc.) + // triggers another check. The hook gates re-entry by tearing + // this down when isLoadingOlder flips true, so the attachable + // itself doesn't need to throttle. + FakeResizeObserver.instances[0].fire(); + FakeResizeObserver.instances[0].fire(); + FakeResizeObserver.instances[0].fire(); + expect(calls).toBe(3); + + uninstallFakeResizeObserver(); + }); + + test("teardown disconnects the observer", () => { + installFakeResizeObserver(); + const container = createFakeElement(5000); + const content = createFakeElement(0); + + const stop = attachLoadOlderOnTop({ + container: container as unknown as HTMLElement, + content: content as unknown as HTMLElement, + onLoadOlder: () => {}, + }); + stop(); + + expect(FakeResizeObserver.instances[0].disconnected).toBe(true); + uninstallFakeResizeObserver(); + }); + + test("returns an inert teardown when ResizeObserver is unavailable", () => { + // No installFakeResizeObserver — global is undefined. + const container = createFakeElement(5000); + const content = createFakeElement(0); + let calls = 0; + + const stop = attachLoadOlderOnTop({ + container: container as unknown as HTMLElement, + content: content as unknown as HTMLElement, + onLoadOlder: () => { + calls += 1; + }, + }); + expect(calls).toBe(0); + // Should not throw. + stop(); + }); +}); diff --git a/apps/web/src/domains/chat/transcript/transcript-scroll.ts b/apps/web/src/domains/chat/transcript/transcript-scroll.ts index 6ad6eb24fee..b92a9f3cbb0 100644 --- a/apps/web/src/domains/chat/transcript/transcript-scroll.ts +++ b/apps/web/src/domains/chat/transcript/transcript-scroll.ts @@ -8,9 +8,21 @@ // each utility's body so component files import them without // branching on the flag themselves. -import { useCallback, useRef, type MutableRefObject } from "react"; - -import { TRANSCRIPT_SCROLL_CONTROLLER_ENABLED } from "@/domains/chat/transcript/transcript-scroll-flag"; +import { + useCallback, + useEffect, + useRef, + type MutableRefObject, +} from "react"; + +import { + TRANSCRIPT_SCROLL_CONTROLLER_ENABLED, + getTranscriptScrollControllerEnabled, +} from "@/domains/chat/transcript/transcript-scroll-flag"; + +/** Pixel threshold for "near the top" — matches the value the + * deprecated hook used so user-facing behavior is preserved. */ +const NEAR_TOP_LOAD_OLDER_PX = 200; /** * Wire the transcript scroll container + content callback refs. @@ -40,6 +52,17 @@ import { TRANSCRIPT_SCROLL_CONTROLLER_ENABLED } from "@/domains/chat/transcript/ export function useTranscriptScrollOnAttach(args: { scrollContainerRef: MutableRefObject; contentRef: MutableRefObject; + /** Whether more older history is available. Used by the load-older + * effect to decide whether to attach a `ResizeObserver`. */ + hasMore?: boolean; + /** Whether an older-page fetch is currently in flight. When true, + * the effect tears down its `ResizeObserver`; when false again, + * the effect re-runs and a fresh observer's initial tick covers + * the chain-load case. */ + isLoadingOlder?: boolean; + /** Callback fired when the `ResizeObserver` detects the scroll + * position is near the top. */ + onLoadOlder?: () => void; }): { scrollContainerCallbackRef: (el: HTMLDivElement | null) => void; contentCallbackRef: (el: HTMLDivElement | null) => void; @@ -81,6 +104,38 @@ export function useTranscriptScrollOnAttach(args: { [args.scrollContainerRef, args.contentRef, teardown], ); + // Load-older wiring. A `useEffect` (not a state-mirror ref) is the + // right shape here: when `hasMore`, `isLoadingOlder`, or + // `onLoadOlder` change, the effect tears down and re-attaches with + // fresh closures — no `latestRef` pattern, no stale-snapshot bug + // shape from the deprecated hook. + // + // While `isLoadingOlder` is true the observer is intentionally + // detached: once it flips back to false the effect re-runs, a fresh + // observer's initial tick measures the post-prepend layout, and + // chain-loads continue automatically when the viewport is still + // underfilled. + const { hasMore, isLoadingOlder, onLoadOlder } = args; + useEffect(() => { + // Read the flag at effect-run time (not module-load). The effect + // is an early-return guard, not a hook dispatch site, so there's + // no rules-of-hooks concern with a dynamic check here. Side + // benefit: integration tests can flip the flag via `localStorage` + // without fighting module-import ordering. + if (!getTranscriptScrollControllerEnabled()) return; + if (!hasMore || isLoadingOlder || !onLoadOlder) return; + const container = args.scrollContainerRef.current; + const content = args.contentRef.current; + if (!container || !content) return; + return attachLoadOlderOnTop({ container, content, onLoadOlder }); + }, [ + args.scrollContainerRef, + args.contentRef, + hasMore, + isLoadingOlder, + onLoadOlder, + ]); + return { scrollContainerCallbackRef, contentCallbackRef }; } @@ -134,3 +189,51 @@ export function attachSnapToLatest(args: { return stop; } + +/** + * Trigger `onLoadOlder()` whenever a `ResizeObserver` tick on the + * content reports the scroll container is within + * `NEAR_TOP_LOAD_OLDER_PX` of the top. + * + * Pure imperative function — no React, no saved state, no anchor. + * The hook owns when to attach (only when `hasMore && !isLoadingOlder`) + * and tears down on prop change, so this function only needs to act + * on every observed tick. + * + * Covers, by construction: + * • **Initial chain-load** — `observe()` fires once with current + * measurements; if the freshly attached transcript is already + * near the top (typically because it's underfilled), older + * history is requested. + * • **Repeat chain-load** — when an older page lands, the parent + * flips `isLoadingOlder` false, the hook re-runs the effect, a + * fresh observer's initial tick measures the new layout, and the + * loop continues until the viewport is full or `hasMore` flips. + * • **Streaming-triggered detection** — any content height change + * while the user is near the top fires the observer. + * + * Does NOT cover, intentionally: + * • User scrolling up to the top with no other content change. + * Scroll events do not fire a `ResizeObserver`. Adding a scroll + * listener belongs to a separate PR (Trigger A in the migration + * spec). + */ +export function attachLoadOlderOnTop(args: { + container: HTMLElement; + content: HTMLElement; + onLoadOlder: () => void; +}): () => void { + const { container, content, onLoadOlder } = args; + + if (typeof ResizeObserver === "undefined") { + return () => {}; + } + + const observer = new ResizeObserver(() => { + if (container.scrollTop > NEAR_TOP_LOAD_OLDER_PX) return; + onLoadOlder(); + }); + observer.observe(content); + + return () => observer.disconnect(); +} diff --git a/apps/web/src/domains/chat/transcript/transcript.tsx b/apps/web/src/domains/chat/transcript/transcript.tsx index 1950ec48db7..c64eb15c8ec 100644 --- a/apps/web/src/domains/chat/transcript/transcript.tsx +++ b/apps/web/src/domains/chat/transcript/transcript.tsx @@ -41,6 +41,12 @@ export type RefreshOutcome = export interface TranscriptProps { items: TranscriptItem[]; conversationId: string | null; + /** Pagination state driving the imperative load-older + chain-load + * utilities. When the controller flag is OFF the deprecated hook + * still owns this; when ON, these props feed the attachables. */ + hasMore?: boolean; + isLoadingOlder?: boolean; + onLoadOlder?: () => void; assistantDisplayName?: string | null; onSecretSubmit: (requestId: string, value: string) => void; onConfirmationDecision: (requestId: string, decision: string) => void; @@ -152,14 +158,25 @@ export interface TranscriptHandle { export const Transcript = forwardRef( function Transcript(props, ref) { - const { items, conversationId, onPullRefresh, pullRefreshEnabled, ...rest } = - props; + const { + items, + conversationId, + hasMore, + isLoadingOlder, + onLoadOlder, + onPullRefresh, + pullRefreshEnabled, + ...rest + } = props; const scrollRef = useRef(null); const contentRef = useRef(null); const { scrollContainerCallbackRef, contentCallbackRef } = useTranscriptScrollOnAttach({ scrollContainerRef: scrollRef, contentRef, + hasMore, + isLoadingOlder, + onLoadOlder, }); const viewportMinHeight = useViewportMinHeight(scrollRef); diff --git a/apps/web/src/domains/chat/transcript/use-transcript-scroll-on-attach.test.tsx b/apps/web/src/domains/chat/transcript/use-transcript-scroll-on-attach.test.tsx new file mode 100644 index 00000000000..ff4ac830cd2 --- /dev/null +++ b/apps/web/src/domains/chat/transcript/use-transcript-scroll-on-attach.test.tsx @@ -0,0 +1,215 @@ +/** + * Integration test for `useTranscriptScrollOnAttach`'s load-older + * wiring. Mounts the hook through real React so the `useEffect` runs + * against the actual ref-attach lifecycle. + * + * Pure-function tests in `transcript-scroll.test.ts` cover + * `attachLoadOlderOnTop` against a fake DOM. This file proves the + * effect: + * • attaches the observer when refs are populated AND + * `hasMore && !isLoadingOlder`, + * • tears down + re-attaches when `isLoadingOlder` toggles, + * • tears down on unmount. + * + * The controller flag is toggled ON via `localStorage` before each + * test. `transcript-scroll.ts` reads it inside the effect at call + * time so this is order-independent against other tests. + */ + +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + test, +} from "bun:test"; +import { cleanup, render } from "@testing-library/react"; +import React, { useRef } from "react"; + +import { useTranscriptScrollOnAttach } from "@/domains/chat/transcript/transcript-scroll"; + +const STORAGE_KEY = "vellumDebug.flags.transcriptScrollController"; + +class FakeResizeObserver { + static instances: FakeResizeObserver[] = []; + observed: Element[] = []; + disconnected = false; + constructor(public callback: ResizeObserverCallback) { + FakeResizeObserver.instances.push(this); + } + observe(el: Element): void { + this.observed.push(el); + } + unobserve(): void {} + disconnect(): void { + this.disconnected = true; + } + fire(): void { + this.callback([], this as unknown as ResizeObserver); + } +} + +beforeAll(() => { + window.localStorage.setItem(STORAGE_KEY, "true"); +}); +afterAll(() => { + window.localStorage.removeItem(STORAGE_KEY); +}); +beforeEach(() => { + FakeResizeObserver.instances = []; + (globalThis as { ResizeObserver?: unknown }).ResizeObserver = + FakeResizeObserver; +}); +afterEach(() => { + cleanup(); + delete (globalThis as { ResizeObserver?: unknown }).ResizeObserver; +}); + +function Host(props: { + scrollHeight?: number; + scrollTop?: number; + hasMore?: boolean; + isLoadingOlder?: boolean; + onLoadOlder?: () => void; +}) { + const scrollRef = useRef(null); + const contentRef = useRef(null); + const { scrollContainerCallbackRef, contentCallbackRef } = + useTranscriptScrollOnAttach({ + scrollContainerRef: scrollRef, + contentRef, + hasMore: props.hasMore, + isLoadingOlder: props.isLoadingOlder, + onLoadOlder: props.onLoadOlder, + }); + return React.createElement( + "div", + { + ref: (el: HTMLDivElement | null) => { + if (el) { + Object.defineProperty(el, "scrollHeight", { + configurable: true, + value: props.scrollHeight ?? 5000, + }); + Object.defineProperty(el, "scrollTop", { + configurable: true, + writable: true, + value: props.scrollTop ?? 0, + }); + } + scrollContainerCallbackRef(el); + }, + }, + React.createElement("div", { ref: contentCallbackRef }), + ); +} + +describe("useTranscriptScrollOnAttach — load-older wiring", () => { + // NOTE: `attachSnapToLatest` (from PR #32239) is gated by the + // module-load const `TRANSCRIPT_SCROLL_CONTROLLER_ENABLED` which is + // resolved before any test runs, so it stays off here regardless of + // the localStorage seed. The new load-older effect reads the flag + // dynamically via `getTranscriptScrollControllerEnabled()` — that's + // what the localStorage seed enables. Result: only the load-older + // observer attaches in these tests, which is what we want to + // isolate. + + test("does NOT attach observer when hasMore is false", () => { + render( + React.createElement(Host, { + hasMore: false, + isLoadingOlder: false, + onLoadOlder: () => {}, + }), + ); + expect(FakeResizeObserver.instances.length).toBe(0); + }); + + test("does NOT attach observer while isLoadingOlder is true", () => { + render( + React.createElement(Host, { + hasMore: true, + isLoadingOlder: true, + onLoadOlder: () => {}, + }), + ); + expect(FakeResizeObserver.instances.length).toBe(0); + }); + + test("fires onLoadOlder on initial RO tick when at top + hasMore + !isLoadingOlder", () => { + let calls = 0; + render( + React.createElement(Host, { + hasMore: true, + isLoadingOlder: false, + scrollTop: 0, + onLoadOlder: () => { + calls += 1; + }, + }), + ); + expect(FakeResizeObserver.instances.length).toBe(1); + FakeResizeObserver.instances[0].fire(); + expect(calls).toBe(1); + }); + + test("tears down + re-attaches when isLoadingOlder toggles", () => { + let calls = 0; + const { rerender } = render( + React.createElement(Host, { + hasMore: true, + isLoadingOlder: false, + scrollTop: 0, + onLoadOlder: () => { + calls += 1; + }, + }), + ); + expect(FakeResizeObserver.instances.length).toBe(1); + const initialObserver = FakeResizeObserver.instances[0]; + + // Flip isLoadingOlder true — observer should disconnect. + rerender( + React.createElement(Host, { + hasMore: true, + isLoadingOlder: true, + scrollTop: 0, + onLoadOlder: () => { + calls += 1; + }, + }), + ); + expect(initialObserver.disconnected).toBe(true); + + // Flip back — a fresh observer should attach. + rerender( + React.createElement(Host, { + hasMore: true, + isLoadingOlder: false, + scrollTop: 0, + onLoadOlder: () => { + calls += 1; + }, + }), + ); + expect(FakeResizeObserver.instances.length).toBe(2); + FakeResizeObserver.instances[1].fire(); + expect(calls).toBe(1); + }); + + test("teardown disconnects the observer on unmount", () => { + const { unmount } = render( + React.createElement(Host, { + hasMore: true, + isLoadingOlder: false, + scrollTop: 0, + onLoadOlder: () => {}, + }), + ); + expect(FakeResizeObserver.instances.length).toBe(1); + unmount(); + expect(FakeResizeObserver.instances[0].disconnected).toBe(true); + }); +});