From da3472fe5125baf5756355c5f3f8fe85f3cbcee7 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 27 Oct 2026 12:32:24 +0100 Subject: [PATCH] fix(app): preserve the reader when older pages extend tool groups Resolve grouped calls and folded turns through one presentation-owned row lookup. Keep retained group indexing off the streaming path and verify item-page group replacement against its failing baseline. --- .../browser/agent-timeline-pagination.spec.ts | 30 +++++ .../owned-subscriptions-old-daemon.spec.ts | 4 +- .../support/helpers/agent-timeline-gate.ts | 26 +++- .../support/helpers/timeline-pagination.ts | 120 +++++++++++++++++- .../app/src/agent-stream/presentation.test.ts | 47 +++++++ packages/app/src/agent-stream/presentation.ts | 61 +++++++-- .../app/src/agent-stream/reading-anchor.ts | 5 +- packages/app/src/agent-stream/strategy.ts | 3 +- .../agent-stream/turn-collapse/model.test.ts | 9 +- .../src/agent-stream/turn-collapse/model.ts | 18 +-- packages/app/src/agent-stream/view.tsx | 2 +- 11 files changed, 282 insertions(+), 43 deletions(-) diff --git a/packages/app/e2e/browser/agent-timeline-pagination.spec.ts b/packages/app/e2e/browser/agent-timeline-pagination.spec.ts index f2772b0b1b..57ee050fa4 100644 --- a/packages/app/e2e/browser/agent-timeline-pagination.spec.ts +++ b/packages/app/e2e/browser/agent-timeline-pagination.spec.ts @@ -27,6 +27,10 @@ import { seedRealisticOlderTimeline, sendLiveTurnBeforeHydration, expectTimelineViewportAnchoredAfterPrepend, + expectToolCallGroupPositionPreserved, + rememberToolCallGroupPosition, + seedItemPagedToolGroupTimeline, + useChatDisplay, userNavigatesTimelineToHistoryStartWithKeyboard, userScrollsTimelineToHistoryStart, } from "../support/helpers/timeline-pagination"; @@ -100,6 +104,32 @@ test.describe("Agent timeline pagination", () => { } }); + // Hosts paged by items can end a page inside a tool run. The older page extends the group + // the reader is on, which changes the row hosting it; the reader must stay with that group + // rather than land at history start and drain every older page. + test("keeps the reading position when an older item page extends the tool-call group being read", async ({ + page, + }) => { + test.setTimeout(120_000); + await useChatDisplay(page, "collapseToolCalls"); + const agent = await seedItemPagedToolGroupTimeline(); + try { + const history = await holdOlderHistoryPages(page, agent, { itemPagedHost: true }); + await openAgentTimeline(page, agent); + await expectTimelinePromptVisible(page, agent.newestPrompt); + + await userScrollsTimelineToHistoryStart(page); + await history.expectRequestedPages(1); + const reading = await rememberToolCallGroupPosition(page, agent.loadedCommands); + + history.releasePage(1); + await expectToolCallGroupPositionPreserved(page, agent.totalCommands, reading); + await history.expectSettledWithRequestedPages(1); + } finally { + await agent.cleanup(); + } + }); + test("loads older history when the user navigates to history start with the keyboard", async ({ page, }) => { diff --git a/packages/app/e2e/browser/owned-subscriptions-old-daemon.spec.ts b/packages/app/e2e/browser/owned-subscriptions-old-daemon.spec.ts index f735f71098..5f1f36e409 100644 --- a/packages/app/e2e/browser/owned-subscriptions-old-daemon.spec.ts +++ b/packages/app/e2e/browser/owned-subscriptions-old-daemon.spec.ts @@ -162,7 +162,9 @@ test("keeps the reader on the turn that folds when an older item page completes await agent.client.waitForFinish(agent.agentId, 30_000); } await published.seedBrowser(page, { chatDisplay: "collapseTurns" }); - const history = await holdOlderHistoryPages(page, agent, published.daemon.port); + const history = await holdOlderHistoryPages(page, agent, { + daemonPort: published.daemon.port, + }); await openAgentTimeline(page, agent, serverId); await expectTimelinePromptVisible(page, prompts.at(-1)!); diff --git a/packages/app/e2e/support/helpers/agent-timeline-gate.ts b/packages/app/e2e/support/helpers/agent-timeline-gate.ts index 408bd8af24..89a1d8c364 100644 --- a/packages/app/e2e/support/helpers/agent-timeline-gate.ts +++ b/packages/app/e2e/support/helpers/agent-timeline-gate.ts @@ -338,11 +338,32 @@ export async function delayAgentOlderTimelineResponse( return delayAgentTimelineResponse(page, agentId, "before"); } +export interface OlderTimelinePagesOptions { + daemonPort?: number; + /** + * Hide whole-turn paging from the app so it reads this host with item pages, as it reads + * daemons released before whole-turn pages. An item page can end inside a turn or a tool run. + */ + itemPagedHost?: boolean; +} + +function withoutWholeTurnPages(message: WebSocketMessage): WebSocketMessage { + const sessionMessage = getSessionMessage(message); + const payload = sessionMessage ? getPayload(sessionMessage) : null; + if (sessionMessage?.type !== "status" || payload?.status !== "server_info") return message; + const envelope = parseWebSocketJson(message) as { + message: { payload: { features?: Record } }; + }; + delete envelope.message.payload.features?.wholeTurnTimeline; + return JSON.stringify(envelope); +} + export async function holdAgentOlderTimelinePages( page: Page, agentId: string, - daemonPort?: number, + options: OlderTimelinePagesOptions = {}, ): Promise { + const { daemonPort } = options; let requestCount = 0; let responseCount = 0; let repeatedEntryCount = 0; @@ -376,7 +397,8 @@ export async function holdAgentOlderTimelinePages( } server.send(message); }); - server.onMessage((message) => { + server.onMessage((received) => { + const message = options.itemPagedHost ? withoutWholeTurnPages(received) : received; const sessionMessage = getSessionMessage(message); const payload = sessionMessage ? getPayload(sessionMessage) : null; if ( diff --git a/packages/app/e2e/support/helpers/timeline-pagination.ts b/packages/app/e2e/support/helpers/timeline-pagination.ts index b152585d5a..e575f4f0d9 100644 --- a/packages/app/e2e/support/helpers/timeline-pagination.ts +++ b/packages/app/e2e/support/helpers/timeline-pagination.ts @@ -1,4 +1,4 @@ -import { expect, type TestInfo, type Page } from "@playwright/test"; +import { expect, type Locator, type TestInfo, type Page } from "@playwright/test"; import { buildAgentRoute, seedMockAgentWorkspace, type MockAgentWorkspace } from "./mock-agent"; import { readReplicaCache } from "./replica-cache-storage"; import { @@ -9,6 +9,7 @@ import { type AgentTimelineResponseGate, type BootstrapTimelineGate, type OlderTimelinePagesGate, + type OlderTimelinePagesOptions, } from "./agent-timeline-gate"; export { holdDaemonHydration }; @@ -35,6 +36,19 @@ interface RealisticOlderTimelineAgent extends MockAgentWorkspace { newestPrompt: string; } +export interface ItemPagedToolGroupAgent extends MockAgentWorkspace { + newestPrompt: string; + /** Commands in one run, more than an item page holds. */ + totalCommands: number; + /** Commands of that run the initial tail loads, so the size of the group as first shown. */ + loadedCommands: number; +} + +interface ToolCallGroupPositionSnapshot { + top: number; +} + +const ITEM_PAGE_SIZE = 40; const PROMPT_PREFIX = "timeline-pagination-turn"; const LIVE_BEFORE_HYDRATION_PROMPT = "timeline live before authoritative hydration"; const HISTORY_START_THRESHOLD_PX = 96; @@ -126,6 +140,93 @@ export async function seedRealisticOlderTimeline(): Promise { + const agent = await seedMockAgentWorkspace({ + repoPrefix: "timeline-item-page-tool-group-", + title: "Item page tool group regression", + }); + const olderTurns = 10; + const newerTurns = 12; + const totalCommands = 50; + // A short turn is its prompt and one reply; the run's turn ends with one reply. + const loadedCommands = ITEM_PAGE_SIZE - newerTurns * 2 - 1; + const shortTurnPrompt = (label: string, index: number) => + `item-page-${label}-turn-${index}: emit 1 coalesced agent stream updates`; + const sendTurn = async (prompt: string) => { + await agent.client.sendAgentMessage(agent.agentId, prompt); + await agent.client.waitForFinish(agent.agentId, 15_000); + }; + + for (let index = 0; index < olderTurns; index += 1) { + await sendTurn(shortTurnPrompt("older", index)); + } + await sendTurn(`emit ${totalCommands} tool agent stream updates`); + for (let index = 0; index < newerTurns; index += 1) { + await sendTurn(shortTurnPrompt("newer", index)); + } + + return { + ...agent, + newestPrompt: shortTurnPrompt("newer", newerTurns - 1), + totalCommands, + loadedCommands, + }; +} + +export async function useChatDisplay( + page: Page, + chatDisplay: "full" | "collapseToolCalls" | "collapseTurns", +): Promise { + await page.addInitScript((value) => { + localStorage.setItem("@paseo:app-settings", JSON.stringify({ chatDisplay: value })); + }, chatDisplay); +} + +function toolCallGroup(page: Page, commandCount: number): Locator { + const timeline = page.locator('[data-testid="agent-chat-scroll"]:visible').first(); + return timeline + .getByTestId("tool-call-group") + .getByRole("button", { name: `Ran ${commandCount} commands`, exact: true }); +} + +export async function rememberToolCallGroupPosition( + page: Page, + commandCount: number, +): Promise { + const group = toolCallGroup(page, commandCount); + await expect(group).toBeVisible(); + const box = await group.boundingBox(); + if (!box) { + throw new Error(`Expected a rendered tool-call group of ${commandCount} commands`); + } + return { top: box.y }; +} + +/** The group the reader was on keeps its place on screen, now counting the older page's calls. */ +export async function expectToolCallGroupPositionPreserved( + page: Page, + commandCount: number, + before: ToolCallGroupPositionSnapshot, +): Promise { + const group = toolCallGroup(page, commandCount); + await expect(group).toBeVisible(); + await expect + .poll( + async () => { + const box = await group.boundingBox(); + return box ? Math.abs(box.y - before.top) : Number.POSITIVE_INFINITY; + }, + { message: `the group of ${commandCount} commands keeps its viewport position` }, + ) + .toBeLessThanOrEqual(2); +} + export async function scrollThroughOlderHistoryPages( page: Page, pageCount: number, @@ -316,13 +417,14 @@ export async function holdNextOlderTimelinePage( export async function holdOlderHistoryPages( page: Page, agent: MockAgentWorkspace, - daemonPort?: number, + options?: OlderTimelinePagesOptions, ): Promise { const gate: OlderTimelinePagesGate = await holdAgentOlderTimelinePages( page, agent.agentId, - daemonPort, + options, ); + const loader = page.getByTestId("load-older-history-spinner"); return { async expectRequestedPages(count) { await gate.waitForRequestCount(count); @@ -330,7 +432,17 @@ export async function holdOlderHistoryPages( }, async expectSettledWithRequestedPages(count) { await waitForTimelineGeometryToSettle(page); - expect(gate.getRequestCount()).toBe(count); + // The loader stays until the prepend settles and the viewport decides whether to + // keep loading. Once it hides, no further page loads without new user intent. + await expect + .poll( + async () => ({ + requestedPages: gate.getRequestCount(), + loading: await loader.isVisible(), + }), + { message: `older history settles after ${count} requested page(s)` }, + ) + .toEqual({ requestedPages: count, loading: false }); }, expectNoRepeatedEntries() { expect(gate.getRepeatedEntryCount()).toBe(0); diff --git a/packages/app/src/agent-stream/presentation.test.ts b/packages/app/src/agent-stream/presentation.test.ts index 0cfc793228..f7d146d92a 100644 --- a/packages/app/src/agent-stream/presentation.test.ts +++ b/packages/app/src/agent-stream/presentation.test.ts @@ -15,6 +15,7 @@ import { QueryClient } from "@tanstack/react-query"; import { describe, expect, it } from "vitest"; import { buildAgentStreamRenderModel } from "./model"; import { createStreamPresentation } from "./presentation"; +import { createReadingAnchor } from "./reading-anchor"; const runtime = { paseo: {}, @@ -609,8 +610,54 @@ it("opening a turn reveals individual calls rather than another tool group", () }); expect(result.tail.map((item) => item.id)).toEqual(tail.map((item) => item.id)); expect(result.groupsByHostId.size).toBe(0); + expect(result.readingRowReplacements.get("tool-two")).toBeUndefined(); }); +it.each(["collapseToolCalls", "collapseTurns"] as const)( + "preserves the reader when a prepend extends a group in %s", + (display) => { + const present = createStreamPresentation(); + const anchor = createReadingAnchor(); + const calls = [toolCall("one", "Bash"), toolCall("two", "Bash")]; + const input = { + ...presentationOptions, + transform: undefined, + display, + head: [], + isTurnActive: false, + }; + const partial = present({ ...input, tail: calls.slice(1) }); + anchor.reconcile(0, [{ id: partial.tail[0]!.id, top: 48, height: 200 }]); + const completed = present({ ...input, tail: calls }); + const groupId = completed.tail[0]!.id; + expect(anchor.getRowId(completed.readingRowReplacements)).toBe(groupId); + expect( + anchor.reconcile( + 0, + [{ id: groupId, top: 448, height: 200 }], + false, + completed.readingRowReplacements, + ), + ).toBe(400); + expect(anchor.getRowId()).toBe(groupId); + const timestamp = new Date(); + const folded = present({ + ...input, + display: "collapseTurns", + tail: [ + { kind: "user_message", id: "prompt", text: "Work", timestamp }, + ...calls, + { kind: "assistant_message", id: "reply", text: "Done", timestamp }, + ], + }); + // Both the original partial group and its replacement resolve directly to the turn. + expect(folded.readingRowReplacements.get(calls[1]!.id)).toBe("prompt"); + expect(anchor.getRowId(folded.readingRowReplacements)).toBe("prompt"); + const opened = present({ ...input, tail: calls, display: "full" }); + expect(opened.readingRowReplacements.get(calls[1]!.id)).toBeUndefined(); + }, +); + it("presents received agent messages as attributed bubbles with the original turn identity", () => { const call = toolCall("received", "agent_message"); if (call.payload.source !== "agent") throw new Error("Expected agent tool"); diff --git a/packages/app/src/agent-stream/presentation.ts b/packages/app/src/agent-stream/presentation.ts index 5138cc1647..b449cb63cc 100644 --- a/packages/app/src/agent-stream/presentation.ts +++ b/packages/app/src/agent-stream/presentation.ts @@ -11,6 +11,7 @@ import { } from "@/tool-calls/detail-level/projection"; import type { StreamItem, UserMessageItem } from "@/types/stream"; import { collapseTurns, type TurnCollapse } from "./turn-collapse/model"; +import type { ReadingRowReplacements } from "./reading-anchor"; interface PresentationInput { tail: StreamItem[]; @@ -26,6 +27,22 @@ interface PresentationInput { export interface StreamPresentation extends ToolCallDetailProjection { turns: Omit; + readingRowReplacements: ReadingRowReplacements; +} + +function groupedRowHosts( + rows: readonly StreamItem[], + groups: ToolCallDetailProjection["groupsByHostId"], +): ReadonlyMap { + const hosts = new Map(); + for (const row of rows) { + const group = groups.get(row.id); + if (!group) continue; + for (const call of group.run.calls) { + if (call.id !== row.id) hosts.set(call.id, row.id); + } + } + return hosts; } const NO_COLLAPSED_TURNS: StreamPresentation["turns"] = collapseTurns({ @@ -61,6 +78,7 @@ export function createStreamPresentation() { let preparedAutoExpandReasoning: boolean | undefined; let preparedLevel: ToolCallDetailLevel | undefined; let preparedHistory: PreparedToolCallHistory | null = null; + let historyGroupHosts: ReadonlyMap = new Map(); let collapsedSource: StreamItem[] | undefined; let collapsedIsLatestTurnOpen = false; let collapsedExpandedHostIds: ReadonlySet | undefined; @@ -89,7 +107,7 @@ export function createStreamPresentation() { function collapse( projection: ToolCallDetailProjection, input: PresentationInput, - ): StreamPresentation { + ): Omit { if (input.display !== "collapseTurns") { return { ...projection, turns: NO_COLLAPSED_TURNS }; } @@ -157,19 +175,42 @@ export function createStreamPresentation() { preparedAutoExpandReasoning !== input.autoExpandReasoning ) { preparedHistory = prepareToolCallHistory(level, historyRows, input.autoExpandReasoning); + historyGroupHosts = preparedHistory + ? groupedRowHosts(preparedHistory.grouped.tail, preparedHistory.grouped.groupsByHostId) + : new Map(); preparedTail = historyRows; preparedLevel = level; preparedAutoExpandReasoning = input.autoExpandReasoning; } - return collapse( - projectToolCallDetailLevel({ - level, - tail: historyRows, - head, - preparedHistory, - isTurnActive: input.isTurnActive, - }), - input, + const projection = projectToolCallDetailLevel({ + level, + tail: historyRows, + head, + preparedHistory, + isTurnActive: input.isTurnActive, + }); + const result = collapse(projection, input); + // Only the trailing retained group can absorb head rows. Reuse the retained + // index across streaming updates rather than walking the full history again. + const last = projection.tail.at(-1); + const liveGroupHosts = groupedRowHosts( + last ? [last, ...projection.head] : projection.head, + projection.groupsByHostId, ); + const retainedGroupHosts = historyGroupHosts; + return { + ...result, + readingRowReplacements: { + get(id) { + if (result.turns.nestedItemIds.has(id)) return undefined; + const groupHost = liveGroupHosts.get(id) ?? retainedGroupHosts.get(id) ?? id; + return ( + result.turns.hostIdByHiddenRowId.get(id) ?? + result.turns.hostIdByHiddenRowId.get(groupHost) ?? + (groupHost === id ? undefined : groupHost) + ); + }, + }, + }; }; } diff --git a/packages/app/src/agent-stream/reading-anchor.ts b/packages/app/src/agent-stream/reading-anchor.ts index 132ab16eda..aa915c6ae7 100644 --- a/packages/app/src/agent-stream/reading-anchor.ts +++ b/packages/app/src/agent-stream/reading-anchor.ts @@ -6,7 +6,10 @@ interface RowGeometry { // A row must clear the reading line before the next row takes ownership. const READING_POSITION_OFFSET_PX = 8; -const NO_REPLACEMENTS: ReadonlyMap = new Map(); +export interface ReadingRowReplacements { + get(id: string): string | undefined; +} +const NO_REPLACEMENTS: ReadingRowReplacements = new Map(); // Content coordinates survive user scrolling. Layout commits replace the geometry; // scroll events reuse it, without another DOM measurement pass. diff --git a/packages/app/src/agent-stream/strategy.ts b/packages/app/src/agent-stream/strategy.ts index 1db6a3c1c9..364eba1043 100644 --- a/packages/app/src/agent-stream/strategy.ts +++ b/packages/app/src/agent-stream/strategy.ts @@ -1,6 +1,7 @@ import type { ComponentType, ReactElement, ReactNode, RefObject } from "react"; import type { StyleProp, ViewStyle } from "react-native"; import type { StreamItem } from "@/types/stream"; +import type { ReadingRowReplacements } from "./reading-anchor"; import { continuesResponse } from "./turn-membership"; import type { StreamHistoryBoundary, StreamRenderSegments } from "./model"; import type { @@ -79,7 +80,7 @@ export interface StreamRenderInput { historyRowRevision?: StreamHistoryRowRevision; liveHeadRowRevision?: unknown; /** Presentation rows replaced by another row after history is folded. */ - readingRowReplacements?: ReadonlyMap; + readingRowReplacements?: ReadingRowReplacements; boundary: StreamHistoryBoundary; renderers: StreamSegmentRenderers; listEmptyComponent: ReactNode; diff --git a/packages/app/src/agent-stream/turn-collapse/model.test.ts b/packages/app/src/agent-stream/turn-collapse/model.test.ts index d525f47d45..eca6d8757c 100644 --- a/packages/app/src/agent-stream/turn-collapse/model.test.ts +++ b/packages/app/src/agent-stream/turn-collapse/model.test.ts @@ -48,12 +48,9 @@ function collapse(tail: StreamItem[], options: Partial = {}) const ids = (items: StreamItem[]) => items.map((item) => item.id); describe("collapseTurns", () => { - it("maps a partial turn's former group host to the completed turn host", () => { - const result = collapse([user("u1"), shell("earlier-call"), reply("final")], { - getGroupedCalls: () => [shell("earlier-call"), shell("previous-group-host")], - }); - expect(result.hostIdByHiddenRowId.get("previous-group-host")).toBe("u1"); - expect(result.hostIdByHiddenRowId.get("earlier-call")).toBe("u1"); + it("maps hidden rows only while their turn is closed", () => { + const result = collapse([user("u1"), shell("c1"), reply("final")]); + expect(result.hostIdByHiddenRowId.get("c1")).toBe("u1"); const expanded = collapse([user("u1"), shell("c1"), reply("final")], { expandedHostIds: new Set(["u1"]), }); diff --git a/packages/app/src/agent-stream/turn-collapse/model.ts b/packages/app/src/agent-stream/turn-collapse/model.ts index fe7dfb81d3..a5985e600f 100644 --- a/packages/app/src/agent-stream/turn-collapse/model.ts +++ b/packages/app/src/agent-stream/turn-collapse/model.ts @@ -63,22 +63,6 @@ function staysVisibleWhenCollapsed(item: StreamItem): boolean { return isTurnTrigger(item) || item.kind === "compaction"; } -function recordHiddenRows( - items: readonly StreamItem[], - hostId: string, - getGroupedCalls: CollapseTurnsInput["getGroupedCalls"], - hosts: Map, -): void { - for (const item of items) { - if (staysVisibleWhenCollapsed(item)) continue; - hosts.set(item.id, hostId); - // A prepend can change which call hosts a group before the turn folds. - if (item.kind === "tool_call" || item.kind === "thought") { - for (const call of getGroupedCalls(item.id) ?? []) hosts.set(call.id, hostId); - } - } -} - function findLastAssistantIndex(tail: StreamItem[], start: number, end: number): number { for (let index = end - 1; index >= start; index -= 1) { if (tail[index]?.kind === "assistant_message") return index; @@ -121,7 +105,6 @@ export function collapseTurns(input: CollapseTurnsInput): TurnCollapse { return; } const expanded = input.expandedHostIds.has(host.id); - if (!expanded) recordHiddenRows(folded, host.id, input.getGroupedCalls, hostIdByHiddenRowId); turnsByHostId.set(host.id, { hostId: host.id, expanded, summary }); const revealed = expanded ? folded.flatMap((item) => @@ -134,6 +117,7 @@ export function collapseTurns(input: CollapseTurnsInput): TurnCollapse { if (staysVisibleWhenCollapsed(item)) continue; hostIdByFoldedMessageId.set(getStreamItemMessageId(item), host.id); if (expanded) nestedItemIds.add(item.id); + else hostIdByHiddenRowId.set(item.id, host.id); } output.push( ...(expanded ? revealed : folded.filter((item) => staysVisibleWhenCollapsed(item))), diff --git a/packages/app/src/agent-stream/view.tsx b/packages/app/src/agent-stream/view.tsx index 82ae2ab718..f70052d820 100644 --- a/packages/app/src/agent-stream/view.tsx +++ b/packages/app/src/agent-stream/view.tsx @@ -1203,7 +1203,7 @@ const AgentStreamViewComponent = forwardRef {streamRenderStrategy.render({ - readingRowReplacements: presentation.turns.hostIdByHiddenRowId, + readingRowReplacements: presentation.readingRowReplacements, agentId, segments: renderModel.segments, historyRowRevision,