From b6511ea5f97e2298dd839b65f710bcf66a05e03e Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Sun, 1 Nov 2026 22:28:17 +0000 Subject: [PATCH] fix(plugins): keep transformed replies distinct in chat history Replacement IDs are local to their source message. Reusing a stable output ID across replies aliased the row and virtualizer caches, reproducing a packaged desktop React #185 crash. Scope row identity at projection and retain the original prompt identity for navigation. Build on the contributor regression, exercising transformed source rows instead of assigning occurrence keys in the virtualizer. Co-authored-by: Kevin Le Co-authored-by: ykurilov <110710320+ykurilov@users.noreply.github.com> --- .../app/src/agent-stream/presentation.test.ts | 4 +- .../src/agent-stream/strategy-web.test.tsx | 49 ++++++++----- .../app/src/agent-stream/strategy-web.tsx | 14 +--- .../src/plugins/timeline/projection.test.ts | 69 +++++++++++++++++-- .../app/src/plugins/timeline/projection.ts | 4 +- 5 files changed, 102 insertions(+), 38 deletions(-) diff --git a/packages/app/src/agent-stream/presentation.test.ts b/packages/app/src/agent-stream/presentation.test.ts index ce3c0352ff..1d6dbf6492 100644 --- a/packages/app/src/agent-stream/presentation.test.ts +++ b/packages/app/src/agent-stream/presentation.test.ts @@ -200,7 +200,9 @@ describe("stream presentation through installed plugins", () => { const source = rows(harness.source()); expect(source).toMatchObject([{ kind: "user_message", id: "prompt-1" }]); - expect(rendered).toMatchObject([{ kind: "plugin", id: "probe-plugin/prompt-1/0" }]); + expect(rendered).toMatchObject([ + { kind: "plugin", sourceId: "prompt-1", pluginItemId: "prompt-1/0" }, + ]); // The chat outline jumps to the source id; the row it lands on is the plugin's. expect(rendered.map(getStreamItemMessageId)).toEqual(["prompt-1"]); }); diff --git a/packages/app/src/agent-stream/strategy-web.test.tsx b/packages/app/src/agent-stream/strategy-web.test.tsx index 9e0fd81523..25e29a30c0 100644 --- a/packages/app/src/agent-stream/strategy-web.test.tsx +++ b/packages/app/src/agent-stream/strategy-web.test.tsx @@ -10,6 +10,7 @@ import { RetainedPanelActivity } from "@/components/retained-panel"; import { reduceStreamUpdate, type StreamItem } from "@/types/stream"; import type { StreamRenderInput, StreamSegmentRenderers, StreamViewportHandle } from "./strategy"; import { createWebStreamStrategy } from "./strategy-web"; +import { projectPluginTimelineItems } from "@/plugins/timeline/projection"; vi.hoisted(() => { Object.defineProperty(window, "matchMedia", { @@ -55,10 +56,11 @@ function createRenderers(onRowRender: () => void): StreamSegmentRenderers { }; } -function MeasuredTestRow({ item }: { item: StreamItem }) { +function MeasuredTestRow({ item, measuredHeight }: { item: StreamItem; measuredHeight?: number }) { const [loaded, setLoaded] = React.useState(false); const load = React.useCallback(() => setLoaded(true), []); - const height = item.kind === "user_message" && item.text === "grown" ? 120 : 24; + const height = + measuredHeight ?? (item.kind === "user_message" && item.text === "grown" ? 120 : 24); const measure = React.useCallback( (node: HTMLDivElement | null) => { if (!node?.parentElement) return; @@ -252,20 +254,33 @@ describe("createWebStreamStrategy", () => { expect(rowRenderCount.mock.calls.length).toBeLessThanOrEqual(historyVirtualized.length); }); - it("settles row measurement when two history rows share an id", () => { + it("settles row measurement when a plugin reuses output IDs across source messages", () => { const strategy = createWebStreamStrategy({ isMobileBreakpoint: true }); container = document.createElement("div"); document.body.appendChild(container); root = createRoot(container); - const renderRow = (rowItem: StreamItem) => ; - const duplicate = userMessage(2); - const historyVirtualized = [ - userMessage(0), - userMessage(1), - { ...duplicate, text: "grown" }, - duplicate, - userMessage(3), - ]; + const renderRow = (rowItem: StreamItem) => ( + + ); + const historyVirtualized = projectPluginTimelineItems( + [userMessage(0), userMessage(1), userMessage(2), userMessage(3), userMessage(4)], + ({ sourceId }) => + sourceId === "message-2" || sourceId === "message-3" + ? [ + { + type: "plugin", + id: "part-0", + pluginId: "split", + kind: "part", + version: 1, + data: null, + }, + ] + : undefined, + ); const input: StreamRenderInput = { agentId: "agent", segments: { historyVirtualized, historyMounted: [], liveHead: [] }, @@ -296,13 +311,9 @@ describe("createWebStreamStrategy", () => { act(() => root?.render(strategy.render(input))); const rows = [...container.querySelectorAll("[data-history-row-id]")]; - expect(rows.map((row) => row.dataset.historyRowId)).toEqual([ - "message-0", - "message-1", - "message-2", - "message-2", - "message-3", - ]); + expect(rows.map((row) => row.dataset.historyRowId)).toEqual( + historyVirtualized.map((row) => row.id), + ); expect(rows.map((row) => row.style.transform)).toEqual([ "translateY(0px)", "translateY(24px)", diff --git a/packages/app/src/agent-stream/strategy-web.tsx b/packages/app/src/agent-stream/strategy-web.tsx index f63b34c1c9..9dae42bcc0 100644 --- a/packages/app/src/agent-stream/strategy-web.tsx +++ b/packages/app/src/agent-stream/strategy-web.tsx @@ -443,18 +443,6 @@ function WebStreamViewport(props: StreamRenderInput & { isMobileBreakpoint: bool unsubscribe?.(); }; }, []); - // The virtualizer caches measured sizes by key. Two rows with one key overwrite - // each other's size on every measurement pass, so the layout never settles and - // React aborts with "Maximum update depth exceeded". A repeated id gets a suffix. - const historyVirtualizedKeys = useMemo(() => { - const used = new Set(); - return segments.historyVirtualized.map((row) => { - let key = row.id; - for (let repeat = 1; used.has(key); repeat++) key = `${row.id}:${repeat}`; - used.add(key); - return key; - }); - }, [segments.historyVirtualized]); const contentWidth = contentRef.current?.getBoundingClientRect().width; const rowVirtualizer = useVirtualizer({ count: segments.historyVirtualized.length, @@ -464,7 +452,7 @@ function WebStreamViewport(props: StreamRenderInput & { isMobileBreakpoint: bool height: scrollContainerRef.current?.clientHeight ?? 0, }, getScrollElement: () => scrollContainerRef.current, - getItemKey: (index: number) => historyVirtualizedKeys[index] ?? index, + getItemKey: (index: number) => segments.historyVirtualized[index]?.id ?? index, estimateSize: (index: number) => { const row = segments.historyVirtualized[index]; const measured = row && measuredRowHeights.current.get(row.id); diff --git a/packages/app/src/plugins/timeline/projection.test.ts b/packages/app/src/plugins/timeline/projection.test.ts index 2dc95ed339..8b28bea4d2 100644 --- a/packages/app/src/plugins/timeline/projection.test.ts +++ b/packages/app/src/plugins/timeline/projection.test.ts @@ -14,6 +14,65 @@ function thought(text: string, status: "loading" | "ready" = "loading"): StreamI } describe("plugin timeline projection", () => { + it("keeps repeated plugin output IDs distinct across source messages", () => { + const transform: TimelineItemTransform = ({ item }) => [ + { + type: "plugin", + id: "part-0", + pluginId: "split", + kind: "part", + version: 1, + data: { text: item.type === "assistant_message" ? item.text : "" }, + }, + ]; + const first: StreamItem = { + kind: "assistant_message", + id: "message-1", + text: "Short", + timestamp: new Date("2026-01-01T00:00:00.000Z"), + }; + const second: StreamItem = { ...first, id: "message-2", text: "A much longer reply" }; + const rows = projectPluginTimelineItems([first, second], transform); + expect(new Set(rows.map((row) => row.id)).size).toBe(2); + expect(rows).toMatchObject([ + { sourceId: "message-1", pluginItemId: "part-0", data: { text: "Short" } }, + { sourceId: "message-2", pluginItemId: "part-0", data: { text: "A much longer reply" } }, + ]); + const updated = projectPluginTimelineItems( + [{ ...second, text: "Updated reply" }], + transform, + "streaming", + ); + expect(updated[0].id).toBe(rows[1].id); + const completed = projectPluginTimelineItems([{ ...second, text: "Final reply" }], transform); + expect(completed[0].id).toBe(rows[1].id); + const prepended = projectPluginTimelineItems( + [{ ...first, id: "older-message" }, first, second], + transform, + ); + expect(prepended.slice(1).map((row) => row.id)).toEqual(rows.map((row) => row.id)); + }); + + it("keeps source and output ID separators unambiguous", () => { + const rows = projectPluginTimelineItems( + [ + { ...thought("First"), id: "message/a" }, + { ...thought("Second"), id: "message" }, + ], + ({ sourceId }) => [ + { + type: "plugin", + id: sourceId === "message/a" ? "b" : "a/b", + pluginId: "split", + kind: "part", + version: 1, + data: null, + }, + ], + ); + expect(rows[0].id).not.toBe(rows[1].id); + }); + it("does not expose mutable tool detail from stream state", () => { const detail = { type: "read" as const, filePath: "/repo/original.ts" }; const source: StreamItem = { @@ -58,7 +117,8 @@ describe("plugin timeline projection", () => { expect(projectPluginTimelineItems([thought("First")], transform)).toMatchObject([ { kind: "plugin", - id: "inline-thinking/thought-1/0", + sourceId: "thought-1", + pluginItemId: "thought-1/0", data: { text: "First", phase: "streaming" }, }, ]); @@ -145,10 +205,11 @@ describe("plugin timeline projection", () => { ]); const filtered = projectPluginTimelineItems([source], () => []); - expect(exploded.map((item) => item.id)).toEqual([ - "split/thought-1/left", - "split/thought-1/right", + expect(exploded).toMatchObject([ + { sourceId: "thought-1", pluginItemId: "thought-1/left" }, + { sourceId: "thought-1", pluginItemId: "thought-1/right" }, ]); + expect(new Set(exploded.map((item) => item.id)).size).toBe(2); expect(filtered).toEqual([]); }); }); diff --git a/packages/app/src/plugins/timeline/projection.ts b/packages/app/src/plugins/timeline/projection.ts index 0750bdb66a..4857e35ac5 100644 --- a/packages/app/src/plugins/timeline/projection.ts +++ b/packages/app/src/plugins/timeline/projection.ts @@ -103,7 +103,9 @@ function transformSourceItem( return transformed.map((pluginItem) => { const projected: PluginTimelineStreamItem = { kind: "plugin", - id: `${pluginItem.pluginId}/${pluginItem.id}`, + // Output IDs are local to a source item. Encode each part so separators in + // source or output IDs cannot alias another replacement. + id: JSON.stringify([pluginItem.pluginId, item.id, pluginItem.id]), sourceId: item.id, timestamp: item.timestamp, pluginId: pluginItem.pluginId,