From 59a01b60c4a98c91b457200b06f92165f8b4c73b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sat, 5 Sep 2026 16:18:21 +0000 Subject: [PATCH] fix(web): preserve transcript position on task switch (#3405) --- .../task/chat/message-list-native-scroll.ts | 175 ++++++++++++++--- .../task/chat/message-list-native.test.tsx | 137 +++++++++++++- apps/web/e2e/helpers/ws-response-hold.ts | 105 +++++++++++ .../e2e/tests/chat/auto-scroll-toggle.spec.ts | 61 ++++++ .../chat/mobile-auto-scroll-toggle.spec.ts | 150 +++++++++++++++ apps/web/hooks/use-lazy-load-sentinel.test.ts | 78 ++++++++ apps/web/hooks/use-lazy-load-sentinel.ts | 29 ++- .../transcript-task-switch-continuity/plan.md | 178 ++++++++++++++++++ ...ask-01-stabilize-task-switch-transcript.md | 149 +++++++++++++++ .../ui/requirements/transcript-auto-scroll.md | 31 ++- .../system-design/transcript-auto-scroll.md | 78 +++++--- 11 files changed, 1103 insertions(+), 68 deletions(-) create mode 100644 apps/web/e2e/helpers/ws-response-hold.ts create mode 100644 docs/plans/transcript-task-switch-continuity/plan.md create mode 100644 docs/plans/transcript-task-switch-continuity/task-01-stabilize-task-switch-transcript.md diff --git a/apps/web/components/task/chat/message-list-native-scroll.ts b/apps/web/components/task/chat/message-list-native-scroll.ts index db30adbb9f..9cadcccd3b 100644 --- a/apps/web/components/task/chat/message-list-native-scroll.ts +++ b/apps/web/components/task/chat/message-list-native-scroll.ts @@ -26,6 +26,7 @@ import { scheduleClampedScrollRestore } from "./clamped-scroll-restore"; import { createDebugLogger, isDebug } from "@/lib/debug/log"; const paginationDebug = createDebugLogger("messages:pagination"); +const placementDebug = createDebugLogger("messages:scroll-placement"); // i18n-exempt: IntersectionObserver root-margin configuration, not user-facing copy. export const TRANSCRIPT_SENTINEL_ROOT_MARGIN = "200px 0px 0px 0px"; @@ -1090,7 +1091,9 @@ type InitialScrollApplyParams = { isVisibleRef: React.RefObject; isNearBottomRef: React.RefObject; envSwitchPlacementToken: number | null; + hasUnreadDivider: boolean; isProgrammaticScrollLocked: () => boolean; + phase: "provisional" | "final"; }; function markInitialScrollConsumed( @@ -1103,10 +1106,82 @@ function markInitialScrollConsumed( function completeEnvSwitchPlacement(envSwitchPlacementToken: number | null): void { if (envSwitchPlacementToken !== null) { + if (isDebug()) { + placementDebug("placement request completed", { token: envSwitchPlacementToken }); + } useDockviewStore.getState().completePendingChatInitialPlacement(envSwitchPlacementToken); } } +function isCurrentEnvSwitchPlacement( + pending: { sessionId: string; token: number } | null, + sessionId: string | null, + token: number | null, +): boolean { + if (token === null) return true; + return pending?.token === token && pending.sessionId === sessionId; +} + +function hasCompetingInitialScrollOwner(params: { + hasPendingLayoutRestore: boolean; + hasExplicitScrollTarget: boolean; + hasUnreadDivider: boolean; + isProgrammaticScrollLocked: () => boolean; +}): boolean { + return ( + params.hasPendingLayoutRestore || + params.hasExplicitScrollTarget || + params.hasUnreadDivider || + params.isProgrammaticScrollLocked() + ); +} + +function reportInitialPlacement( + phase: InitialScrollApplyParams["phase"], + element: HTMLDivElement, + sessionId: string | null, + token: number | null, + enabled: boolean, +): void { + if (!isDebug()) return; + placementDebug(`${phase} placement`, { + sessionId, + token, + owner: enabled ? "bottom" : "saved-position", + scrollTop: element.scrollTop, + scrollHeight: element.scrollHeight, + clientHeight: element.clientHeight, + }); +} + +function completeFinalEnvSwitchPlacement( + phase: InitialScrollApplyParams["phase"], + token: number | null, +): void { + if (phase === "final") completeEnvSwitchPlacement(token); +} + +function finishAppliedInitialPlacement(params: { + phase: InitialScrollApplyParams["phase"]; + enabled: boolean; + scrollTop: number; + element: HTMLDivElement; + envSwitchPlacementToken: number | null; + syncNearBottom: () => void; +}): void { + if (params.phase === "provisional") return; + if (params.enabled || params.scrollTop <= 0 || params.element.scrollTop >= params.scrollTop - 1) { + completeEnvSwitchPlacement(params.envSwitchPlacementToken); + return; + } + scheduleClampedScrollRestore({ + element: params.element, + targetScrollTop: params.scrollTop, + onApply: params.syncNearBottom, + onComplete: () => completeEnvSwitchPlacement(params.envSwitchPlacementToken), + }); +} + function applyInitialScrollPosition(params: InitialScrollApplyParams): void { const { element, @@ -1118,14 +1193,32 @@ function applyInitialScrollPosition(params: InitialScrollApplyParams): void { isVisibleRef, isNearBottomRef, envSwitchPlacementToken, + hasUnreadDivider, isProgrammaticScrollLocked, + phase, } = params; if (!isVisibleRef.current) return; const dockviewState = useDockviewStore.getState(); + if ( + !isCurrentEnvSwitchPlacement( + dockviewState.pendingChatInitialPlacement, + sessionId, + envSwitchPlacementToken, + ) + ) + return; const hasPendingLayoutRestore = dockviewState.pendingChatScrollTop !== null; const hasExplicitScrollTarget = sessionId !== null && dockviewState.scrollTarget?.sessionId === sessionId; - if (hasPendingLayoutRestore || hasExplicitScrollTarget || isProgrammaticScrollLocked()) { + if ( + hasCompetingInitialScrollOwner({ + hasPendingLayoutRestore, + hasExplicitScrollTarget, + hasUnreadDivider, + isProgrammaticScrollLocked, + }) + ) { + if (phase === "provisional") return; markInitialScrollConsumed(didInitialScroll, activationPendingRef); completeEnvSwitchPlacement(envSwitchPlacementToken); return; @@ -1141,9 +1234,11 @@ function applyInitialScrollPosition(params: InitialScrollApplyParams): void { savedScrollTop, scrollHeight: element.scrollHeight, }); - markInitialScrollConsumed(didInitialScroll, activationPendingRef); + if (phase === "final") { + markInitialScrollConsumed(didInitialScroll, activationPendingRef); + } if (scrollTop === null) { - completeEnvSwitchPlacement(envSwitchPlacementToken); + completeFinalEnvSwitchPlacement(phase, envSwitchPlacementToken); return; } @@ -1156,23 +1251,44 @@ function applyInitialScrollPosition(params: InitialScrollApplyParams): void { }; element.scrollTop = scrollTop; syncNearBottom(); - if (enabled || scrollTop <= 0 || element.scrollTop >= scrollTop - 1) { - completeEnvSwitchPlacement(envSwitchPlacementToken); - return; - } - scheduleClampedScrollRestore({ + reportInitialPlacement(phase, element, sessionId, envSwitchPlacementToken, enabled); + finishAppliedInitialPlacement({ + phase, + enabled, + scrollTop, element, - targetScrollTop: scrollTop, - onApply: syncNearBottom, - onComplete: () => completeEnvSwitchPlacement(envSwitchPlacementToken), + envSwitchPlacementToken, + syncNearBottom, }); } +function shouldSkipProvisionalPlacement( + phase: InitialScrollApplyParams["phase"], + hasUnreadDivider: boolean, + appliedToken: number | null, + currentToken: number | null, +): boolean { + return phase === "provisional" && (hasUnreadDivider || appliedToken === currentToken); +} + +function useInitialPlacementLatches(envSwitchPlacementToken: number | null) { + const didInitialScroll = useRef(false); + const lastTokenRef = useRef(null); + const provisionalTokenRef = useRef(null); + if (envSwitchPlacementToken !== null && envSwitchPlacementToken !== lastTokenRef.current) { + lastTokenRef.current = envSwitchPlacementToken; + provisionalTokenRef.current = null; + didInitialScroll.current = false; + } + return { didInitialScroll, provisionalTokenRef }; +} + function useInitialScrollPosition({ scrollRef, itemCount, sessionId, enabled, + hasUnreadDivider, isNearBottomRef, isVisible, historyRefreshPending, @@ -1184,6 +1300,7 @@ function useInitialScrollPosition({ itemCount: number; sessionId: string | null; enabled: boolean; + hasUnreadDivider: boolean; isNearBottomRef: React.RefObject; isVisible: boolean; historyRefreshPending: boolean; @@ -1192,15 +1309,8 @@ function useInitialScrollPosition({ isProgrammaticScrollLocked: () => boolean; }) { const storeApi = useAppStoreApi(); - const didInitialScroll = useRef(false); - const lastEnvSwitchPlacementTokenRef = useRef(null); - if ( - envSwitchPlacementToken !== null && - envSwitchPlacementToken !== lastEnvSwitchPlacementTokenRef.current - ) { - lastEnvSwitchPlacementTokenRef.current = envSwitchPlacementToken; - didInitialScroll.current = false; - } + const { didInitialScroll, provisionalTokenRef } = + useInitialPlacementLatches(envSwitchPlacementToken); const { isVisibleRef, activationPendingRef } = useActivationPending( isVisible, envSwitchPlacementToken, @@ -1209,8 +1319,9 @@ function useInitialScrollPosition({ if (!isVisible) { return; } - if (envSwitchPlacementToken !== null && (isRestoringLayout || historyRefreshPending)) return; + if (envSwitchPlacementToken !== null && isRestoringLayout) return; if (envSwitchPlacementToken !== null && itemCount === 0) { + if (historyRefreshPending) return; markInitialScrollConsumed(didInitialScroll, activationPendingRef); completeEnvSwitchPlacement(envSwitchPlacementToken); return; @@ -1219,7 +1330,19 @@ function useInitialScrollPosition({ const el = scrollRef.current; if (!el) return; - const applyInitialScroll = () => + const phase = + envSwitchPlacementToken !== null && historyRefreshPending ? "provisional" : "final"; + if ( + shouldSkipProvisionalPlacement( + phase, + hasUnreadDivider, + provisionalTokenRef.current, + envSwitchPlacementToken, + ) + ) { + return; + } + const applyInitialScroll = () => { applyInitialScrollPosition({ element: el, sessionId, @@ -1230,8 +1353,14 @@ function useInitialScrollPosition({ isVisibleRef, isNearBottomRef, envSwitchPlacementToken, + hasUnreadDivider, isProgrammaticScrollLocked, + phase, }); + if (phase === "provisional") { + provisionalTokenRef.current = envSwitchPlacementToken; + } + }; if (activationPendingRef.current) { return scheduleAfterPanelRestore(applyInitialScroll); @@ -1241,6 +1370,7 @@ function useInitialScrollPosition({ itemCount, sessionId, enabled, + hasUnreadDivider, isNearBottomRef, storeApi, isVisible, @@ -1351,6 +1481,7 @@ export function useNativeScrollManagement(params: NativeScrollManagementParams) itemCount: items.length, sessionId, enabled, + hasUnreadDivider, isNearBottomRef, isVisible, historyRefreshPending, diff --git a/apps/web/components/task/chat/message-list-native.test.tsx b/apps/web/components/task/chat/message-list-native.test.tsx index a16f74956f..3ab1b56df7 100644 --- a/apps/web/components/task/chat/message-list-native.test.tsx +++ b/apps/web/components/task/chat/message-list-native.test.tsx @@ -85,6 +85,7 @@ const HANDLE_RENDER_ERROR = "handle did not render"; const HARNESS_RENDER_ERROR = "harness did not render"; const NATIVE_SCROLL_MANAGEMENT_TEST_ID = "native-scroll-management-container"; const AUTO_SCROLL_CONTAINER_TEST_ID = "auto-scroll-container"; +const CACHED_MESSAGE_ID = "cached-message"; const TEST_MESSAGES = [{} as Message]; /** Always returns false: the harness never locks programmatic scrolling. */ const NEVER_LOCKED = () => false; @@ -278,6 +279,7 @@ function NativeScrollManagementHarness({ isVisible = true, enabled = false, historyRefreshPending = false, + hasUnreadDivider = false, }: { items: RenderItem[]; metrics?: NativeScrollMetrics; @@ -288,6 +290,7 @@ function NativeScrollManagementHarness({ isVisible?: boolean; enabled?: boolean; historyRefreshPending?: boolean; + hasUnreadDivider?: boolean; }) { const scrollRef = useRef(null); useNativeScrollMetrics(scrollRef, metrics); @@ -298,7 +301,7 @@ function NativeScrollManagementHarness({ isWorking: false, sessionId, enabled, - hasUnreadDivider: false, + hasUnreadDivider, messagesLoading: false, hasMore: true, isLoadingMore, @@ -354,7 +357,8 @@ describe("isElementInPreloadRegion", () => { // eslint-disable-next-line max-lines-per-function -- pagination invariants share one fixture and lifecycle. describe("useNativeScrollManagement transcript pagination", () => { // @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.11 - it("places an env-switched enabled transcript after layout and history settle", () => { + // @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.14 + it("places cached enabled history before refresh and reconciles after it settles", () => { const frames: Array = []; vi.stubGlobal("requestAnimationFrame", (callback: FrameRequestCallback) => { frames.push(callback); @@ -366,7 +370,7 @@ describe("useNativeScrollManagement transcript pagination", () => { try { const { rerender } = render( { expect(sharedSentinelCalls.at(-1)?.[2]).toBe(true); mockDockviewState.isRestoringLayout = false; + rerender( + , + ); + act(() => { + for (let frame = frames.shift(); frame; frame = frames.shift()) frame(0); + }); + + expect(scrollContainer.scrollTop).toBe(600); + expect(mockDockviewState.pendingChatInitialPlacement).toEqual({ + sessionId: "session-b", + token: 7, + }); + metrics.scrollHeight = 1400; rerender( { }); // @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.12 - it("restores only the incoming session offset after an env switch", () => { + // @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.14 + it("restores the incoming saved offset before refresh and reconciles afterward", () => { const frames: Array = []; vi.stubGlobal("requestAnimationFrame", (callback: FrameRequestCallback) => { frames.push(callback); @@ -414,7 +438,7 @@ describe("useNativeScrollManagement transcript pagination", () => { try { const { rerender } = render( { expect(scrollContainer.scrollTop).toBe(810); mockDockviewState.isRestoringLayout = false; + rerender( + , + ); + act(() => { + for (let frame = frames.shift(); frame; frame = frames.shift()) frame(0); + }); + + expect(scrollContainer.scrollTop).toBe(320); + expect(mockDockviewState.pendingChatInitialPlacement).toEqual({ + sessionId: "session-b", + token: 8, + }); + + metrics.scrollTop = 915; rerender( { } }); + it("leaves provisional placement to an active unread-divider target", () => { + const frames: Array = []; + vi.stubGlobal("requestAnimationFrame", (callback: FrameRequestCallback) => { + frames.push(callback); + return frames.length; + }); + const metrics = { scrollHeight: 900, scrollTop: 210, clientHeight: 400 }; + mockDockviewState.pendingChatInitialPlacement = { sessionId: "session-b", token: 9 }; + try { + render( + , + ); + act(() => { + for (let frame = frames.shift(); frame; frame = frames.shift()) frame(0); + }); + + expect(screen.getByTestId(NATIVE_SCROLL_MANAGEMENT_TEST_ID).scrollTop).toBe(210); + expect(mockDockviewState.pendingChatInitialPlacement).toEqual({ + sessionId: "session-b", + token: 9, + }); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("leaves final placement to an unread-divider target when refresh settles", () => { + const frames: Array = []; + vi.stubGlobal("requestAnimationFrame", (callback: FrameRequestCallback) => { + frames.push(callback); + return frames.length; + }); + const metrics = { scrollHeight: 900, scrollTop: 210, clientHeight: 400 }; + mockDockviewState.pendingChatInitialPlacement = { sessionId: "session-b", token: 10 }; + try { + const { rerender } = render( + , + ); + act(() => { + for (let frame = frames.shift(); frame; frame = frames.shift()) frame(0); + }); + + expect(metrics.scrollTop).toBe(900); + expect(mockDockviewState.pendingChatInitialPlacement).toEqual({ + sessionId: "session-b", + token: 10, + }); + + metrics.scrollTop = 480; + rerender( + , + ); + act(() => { + for (let frame = frames.shift(); frame; frame = frames.shift()) frame(0); + }); + + expect(metrics.scrollTop).toBe(480); + expect(mockDockviewState.pendingChatInitialPlacement).toBeNull(); + } finally { + vi.unstubAllGlobals(); + } + }); + // @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.13 it("does not defer a same-env session placement without an env-switch token", () => { const metrics = { scrollHeight: 900, scrollTop: 125, clientHeight: 400 }; render( void; + heldCount: () => number; + releaseHeldResponse: () => void; +}; + +function parseFrame(value: string): WireFrame | null { + try { + const parsed = JSON.parse(value) as unknown; + return typeof parsed === "object" && parsed !== null ? (parsed as WireFrame) : null; + } catch { + return null; + } +} + +function framePayload(frame: WireFrame | null): Record | null { + return typeof frame?.payload === "object" && frame.payload !== null + ? (frame.payload as Record) + : null; +} + +/** + * Holds one correlated latest-window `message.list` response while keeping the + * gateway socket and every unrelated frame live. Install before navigation, + * then arm immediately before the task switch under test. + */ +export async function routeMainWebSocketWithMessageListResponseHold( + page: Page, +): Promise { + let armedSessionId: string | null = null; + const requestIds = new Set(); + const heldFrames: string[] = []; + let releaseToClient: ((frames: readonly string[]) => void) | null = null; + + await page.routeWebSocket(/\/ws$/, (ws) => { + const server = ws.connectToServer(); + releaseToClient = (frames) => { + for (const frame of frames) ws.send(frame); + }; + ws.onMessage((message) => { + if (typeof message === "string" && armedSessionId) { + for (const part of message.split("\n")) { + const frame = parseFrame(part.trim()); + const payload = framePayload(frame); + if ( + frame?.type === "request" && + frame.action === "message.list" && + typeof frame.id === "string" && + payload?.session_id === armedSessionId && + payload.before === undefined + ) { + requestIds.add(frame.id); + } + } + } + server.send(message); + }); + server.onMessage((message) => { + if (typeof message !== "string" || requestIds.size === 0) { + ws.send(message); + return; + } + const kept: string[] = []; + for (const part of message.split("\n")) { + const trimmed = part.trim(); + const frame = parseFrame(trimmed); + if ( + frame?.type === "response" && + frame.action === "message.list" && + typeof frame.id === "string" && + requestIds.delete(frame.id) + ) { + heldFrames.push(trimmed); + } else if (trimmed) { + kept.push(part); + } + } + if (kept.length > 0) ws.send(kept.join("\n")); + }); + }); + + return { + holdNextLatestWindow: (sessionId) => { + armedSessionId = sessionId; + requestIds.clear(); + heldFrames.length = 0; + }, + heldCount: () => heldFrames.length, + releaseHeldResponse: () => { + const frames = heldFrames.splice(0); + armedSessionId = null; + requestIds.clear(); + releaseToClient?.(frames); + }, + }; +} diff --git a/apps/web/e2e/tests/chat/auto-scroll-toggle.spec.ts b/apps/web/e2e/tests/chat/auto-scroll-toggle.spec.ts index 91e8607c67..02758f8a89 100644 --- a/apps/web/e2e/tests/chat/auto-scroll-toggle.spec.ts +++ b/apps/web/e2e/tests/chat/auto-scroll-toggle.spec.ts @@ -7,6 +7,7 @@ import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; import { dwell } from "../../helpers/causal-waits"; import { waitForStableActiveSession } from "../../helpers/session-store"; +import { routeMainWebSocketWithMessageListResponseHold } from "../../helpers/ws-response-hold"; type E2EMessageStoreWindow = Window & { __KANDEV_E2E_STORE__?: { @@ -194,6 +195,18 @@ test.describe("Transcript auto-scroll toggle", () => { expect(envB).toBeTruthy(); expect(envA).not.toBe(envB); + const refreshHold = await routeMainWebSocketWithMessageListResponseHold(testPage); + const olderPageRequests: string[] = []; + testPage.on("request", (request) => { + const url = new URL(request.url()); + if ( + url.pathname === `/api/v1/task-sessions/${taskA.session_id}/messages` && + url.searchParams.has("before") + ) { + olderPageRequests.push(url.toString()); + } + }); + await testPage.goto(`/t/${taskA.id}`); const session = new SessionPage(testPage); await session.waitForLoad(); @@ -204,17 +217,41 @@ test.describe("Transcript auto-scroll toggle", () => { await waitForStableActiveSession(testPage, taskB.session_id); await session.showSessionContext(); await waitForOverflow(testPage); + refreshHold.holdNextLatestWindow(taskA.session_id); + olderPageRequests.length = 0; await session.sidebarTaskItem("Env switch transcript A").click(); await waitForStableActiveSession(testPage, taskA.session_id); await session.showSessionContext(); const list = chatList(testPage); await waitForOverflow(testPage); + await expect + .poll(refreshHold.heldCount, { + timeout: 5_000, + message: "returning task should issue a latest-window refresh", + }) + .toBe(1); + await expect + .poll(async () => list.evaluate((el) => el.scrollHeight - el.scrollTop - el.clientHeight), { + timeout: 2_000, + message: "cached enabled transcript should be at the bottom before refresh release", + }) + .toBeLessThan(10); + expect(olderPageRequests).toHaveLength(0); + + refreshHold.releaseHeldResponse(); await expect .poll(async () => list.evaluate((el) => el.scrollHeight - el.scrollTop - el.clientHeight), { timeout: 15_000, message: "enabled transcript should settle at the bottom after an environment switch", }) .toBeLessThan(10); + await dwell( + testPage, + 500, + "negative-assertion", + "negative assertion window for an unwanted older-page request after placement unlock", + ); + expect(olderPageRequests).toHaveLength(0); const targetScrollTop = await list.evaluate((el) => { el.scrollTop = Math.floor((el.scrollHeight - el.clientHeight) / 2); @@ -228,15 +265,39 @@ test.describe("Transcript auto-scroll toggle", () => { await session.sidebarTaskItem("Env switch transcript B").click(); await waitForStableActiveSession(testPage, taskB.session_id); + refreshHold.holdNextLatestWindow(taskA.session_id); + olderPageRequests.length = 0; await session.sidebarTaskItem("Env switch transcript A").click(); await waitForStableActiveSession(testPage, taskA.session_id); await session.showSessionContext(); + await expect + .poll(refreshHold.heldCount, { + timeout: 5_000, + message: "disabled return should issue a latest-window refresh", + }) + .toBe(1); + await expect + .poll(async () => Math.abs((await list.evaluate((el) => el.scrollTop)) - targetScrollTop), { + timeout: 2_000, + message: "cached disabled transcript should restore its position before refresh release", + }) + .toBeLessThanOrEqual(20); + expect(olderPageRequests).toHaveLength(0); + + refreshHold.releaseHeldResponse(); await expect .poll(async () => Math.abs((await list.evaluate((el) => el.scrollTop)) - targetScrollTop), { timeout: 15_000, message: "disabled transcript should restore its own offset after an environment switch", }) .toBeLessThanOrEqual(20); + await dwell( + testPage, + 500, + "negative-assertion", + "negative assertion window for pagination after disabled position restoration", + ); + expect(olderPageRequests).toHaveLength(0); await expect(toggle).toHaveAttribute("aria-pressed", "false"); }); diff --git a/apps/web/e2e/tests/chat/mobile-auto-scroll-toggle.spec.ts b/apps/web/e2e/tests/chat/mobile-auto-scroll-toggle.spec.ts index 0b3763a9fe..db5a7f003d 100644 --- a/apps/web/e2e/tests/chat/mobile-auto-scroll-toggle.spec.ts +++ b/apps/web/e2e/tests/chat/mobile-auto-scroll-toggle.spec.ts @@ -2,8 +2,31 @@ import { type Page } from "@playwright/test"; import { test, expect } from "../../fixtures/test-base"; import type { SeedData } from "../../fixtures/test-base"; import type { ApiClient } from "../../helpers/api-client"; +import { dwell } from "../../helpers/causal-waits"; import { waitForSessionDone } from "../../helpers/session"; import { SessionPage } from "../../pages/session-page"; +import { waitForStableActiveSession } from "../../helpers/session-store"; +import { routeMainWebSocketWithMessageListResponseHold } from "../../helpers/ws-response-hold"; +import { watchOlderMessageRequests } from "./message-pagination-helpers"; + +const MOBILE_END_TOLERANCE_PX = 10; + +function mobileOverflowScript(prefix: string): string { + return Array.from( + { length: 30 }, + (_, index) => + `e2e:message("${prefix} ${index + 1} - lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor incididunt ut labore et dolore magna aliqua")`, + ).join("\n"); +} + +async function switchMobileTask(testPage: Page, title: string): Promise { + await testPage.getByTestId("mobile-session-menu").tap(); + const sheet = testPage.getByRole("dialog", { name: "Tasks" }); + const taskRow = sheet.getByTestId("sidebar-task-item").filter({ hasText: title }); + await expect(taskRow).toBeVisible({ timeout: 15_000 }); + await taskRow.tap(); + await expect(sheet).not.toBeVisible({ timeout: 10_000 }); +} /** * Seed a task whose mock-agent script emits enough distinct messages to @@ -80,6 +103,133 @@ test.describe("Mobile transcript auto-scroll toggle", () => { .toBeLessThan(25); }); + test("task switch places cached history before its refresh settles", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(240_000); + const taskA = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Mobile cached transcript A", + seedData.agentProfileId, + { + description: mobileOverflowScript("Mobile environment A history"), + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + const taskB = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Mobile cached transcript B", + seedData.agentProfileId, + { + description: mobileOverflowScript("Mobile environment B history"), + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + if (!taskA.session_id || !taskB.session_id) { + throw new Error("createTaskWithAgent did not return both session ids"); + } + await waitForSessionDone(apiClient, taskA.id, taskA.session_id, "mobile task A should finish"); + await waitForSessionDone(apiClient, taskB.id, taskB.session_id, "mobile task B should finish"); + + const refreshHold = await routeMainWebSocketWithMessageListResponseHold(testPage); + const olderRequests = watchOlderMessageRequests(testPage, taskA.session_id); + await testPage.goto(`/t/${taskA.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await waitForStableActiveSession(testPage, taskA.session_id); + const list = session.activeChat().locator(".chat-message-list"); + await expect + .poll(async () => list.evaluate((element) => element.scrollHeight - element.clientHeight), { + timeout: 15_000, + message: "mobile task A transcript should overflow", + }) + .toBeGreaterThan(200); + + await switchMobileTask(testPage, "Mobile cached transcript B"); + await waitForStableActiveSession(testPage, taskB.session_id); + refreshHold.holdNextLatestWindow(taskA.session_id); + await switchMobileTask(testPage, "Mobile cached transcript A"); + await waitForStableActiveSession(testPage, taskA.session_id); + await expect.poll(refreshHold.heldCount, { timeout: 5_000 }).toBe(1); + await expect + .poll( + async () => + list.evaluate( + (element) => element.scrollHeight - element.scrollTop - element.clientHeight, + ), + { + timeout: 2_000, + message: "mobile cached transcript should be at the bottom before refresh release", + }, + ) + .toBeLessThan(MOBILE_END_TOLERANCE_PX); + refreshHold.releaseHeldResponse(); + await expect + .poll(async () => + list.evaluate((element) => element.scrollHeight - element.scrollTop - element.clientHeight), + ) + .toBeLessThan(MOBILE_END_TOLERANCE_PX); + await dwell( + testPage, + 500, + "negative-assertion", + "observe mobile pagination after first refresh release", + ); + expect(olderRequests).toHaveLength(0); + + const targetScrollTop = await list.evaluate((element) => { + element.scrollTop = Math.floor((element.scrollHeight - element.clientHeight) / 2); + element.dispatchEvent(new Event("scroll")); + return element.scrollTop; + }); + const toggle = session.chatStatusBar().getByTestId("auto-scroll-toggle-button"); + await toggle.tap(); + await expect(toggle).toHaveAttribute("aria-pressed", "false"); + + await switchMobileTask(testPage, "Mobile cached transcript B"); + await waitForStableActiveSession(testPage, taskB.session_id); + refreshHold.holdNextLatestWindow(taskA.session_id); + await switchMobileTask(testPage, "Mobile cached transcript A"); + await waitForStableActiveSession(testPage, taskA.session_id); + await expect.poll(refreshHold.heldCount, { timeout: 5_000 }).toBe(1); + await expect + .poll( + async () => + Math.abs((await list.evaluate((element) => element.scrollTop)) - targetScrollTop), + { + timeout: 2_000, + message: + "mobile cached transcript should restore its saved position before refresh release", + }, + ) + .toBeLessThanOrEqual(20); + refreshHold.releaseHeldResponse(); + await expect + .poll(async () => + Math.abs((await list.evaluate((element) => element.scrollTop)) - targetScrollTop), + ) + .toBeLessThanOrEqual(20); + await dwell( + testPage, + 500, + "negative-assertion", + "observe mobile pagination after second refresh release", + ); + expect(olderRequests).toHaveLength(0); + + const documentWidth = await testPage.evaluate(() => ({ + clientWidth: document.documentElement.clientWidth, + scrollWidth: document.documentElement.scrollWidth, + })); + expect(documentWidth.scrollWidth).toBeLessThanOrEqual(documentWidth.clientWidth); + }); + test("is reachable and toggles by touch", async ({ testPage, apiClient, seedData }) => { const session = await seedOverflowingTask( testPage, diff --git a/apps/web/hooks/use-lazy-load-sentinel.test.ts b/apps/web/hooks/use-lazy-load-sentinel.test.ts index dfb5877c40..cb2373e317 100644 --- a/apps/web/hooks/use-lazy-load-sentinel.test.ts +++ b/apps/web/hooks/use-lazy-load-sentinel.test.ts @@ -231,6 +231,7 @@ describe("useLazyLoadSentinel — scroller lifecycle", () => { }); }); +// eslint-disable-next-line max-lines-per-function -- eligibility, disarm, and stale-completion cases share one lifecycle fixture describe("useLazyLoadSentinel — re-arm, disarm, and stale completions", () => { it("retries when loading becomes eligible while the sentinel remains intersecting", async () => { const scrollRef = makeScrollRef(); @@ -256,6 +257,28 @@ describe("useLazyLoadSentinel — re-arm, disarm, and stale completions", () => await waitFor(() => expect(loadMore).toHaveBeenCalledTimes(2)); }); + // @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.13 + it("does not retry a blocked intersection that is outside current geometry", async () => { + const scrollRef = makeScrollRef(); + const loadMore = vi.fn(async () => 20); + const { result, rerender } = renderHook( + ({ blocked }: { blocked: boolean }) => + useLazyLoadSentinel(scrollRef, true, blocked, false, loadMore, { + rearmWhileIntersecting: true, + isCurrentGeometryEligible: () => false, + }), + { initialProps: { blocked: true } }, + ); + const node = document.createElement("div"); + act(() => result.current.sentinelRef(node)); + + fire(records[0], true, node); + rerender({ blocked: false }); + await act(async () => {}); + + expect(loadMore).not.toHaveBeenCalled(); + }); + it("re-arms only when enabled: unobserves before loading and re-observes after a positive result", async () => { const scrollRef = makeScrollRef(); const loadMore = vi.fn().mockResolvedValueOnce(20).mockResolvedValueOnce(0); @@ -730,6 +753,61 @@ describe("useLazyLoadSentinel — stale view handoff", () => { // intersection through the normal sentinel path. await waitFor(() => expect(loadB).toHaveBeenCalledTimes(1)); }); + + // @covers AC-UI-TRANSCRIPT-AUTO-SCROLL-001.13 + it("does not hand off a stale intersection that left current geometry", async () => { + const scrollRef = makeScrollRef(); + let activeView = "A"; + let geometryEligible = true; + let resolveA: (value: number) => void = () => {}; + const loadA = vi.fn( + () => + new Promise((resolve) => { + resolveA = resolve; + }), + ); + const loadB = vi.fn(async () => 0); + const { result, rerender } = renderHook( + ({ view }: { view: string }) => + useLazyLoadSentinel(scrollRef, true, false, false, view === "A" ? loadA : loadB, { + rearmWhileIntersecting: true, + isCurrentGeometryEligible: () => geometryEligible, + isRequestCurrent: () => activeView === view, + }), + { initialProps: { view: "A" } }, + ); + const node = document.createElement("div"); + act(() => result.current.sentinelRef(node)); + + fire(records[0], true, node); + activeView = "B"; + rerender({ view: "B" }); + fire(records[1], true, node); + geometryEligible = false; + + await act(async () => resolveA(20)); + await act(async () => {}); + + expect(loadB).not.toHaveBeenCalled(); + }); + + it("rejects a queued positive intersection when current geometry is ineligible", () => { + const scrollRef = makeScrollRef(); + let geometryEligible = true; + const loadMore = vi.fn(async () => 20); + const { result } = renderHook(() => + useLazyLoadSentinel(scrollRef, true, false, false, loadMore, { + isCurrentGeometryEligible: () => geometryEligible, + }), + ); + const node = document.createElement("div"); + act(() => result.current.sentinelRef(node)); + + geometryEligible = false; + fire(records[0], true, node); + + expect(loadMore).not.toHaveBeenCalled(); + }); }); describe("useLazyLoadSentinel — stale observers", () => { diff --git a/apps/web/hooks/use-lazy-load-sentinel.ts b/apps/web/hooks/use-lazy-load-sentinel.ts index 6ba8fb37c1..cdba69b560 100644 --- a/apps/web/hooks/use-lazy-load-sentinel.ts +++ b/apps/web/hooks/use-lazy-load-sentinel.ts @@ -131,7 +131,8 @@ function useSentinelObserver(opts: { !entry.isIntersecting || !hasMore || blocked || - (isLoadingMore && !joinInFlightWhileLoading) + (isLoadingMore && !joinInFlightWhileLoading) || + !isCurrentSentinelGeometryEligible(opts.refs) ) { return; } @@ -321,6 +322,21 @@ function useScrollPinnedToBottom(scrollRef: React.RefObject