mirror of
https://github.com/getpaseo/paseo.git
synced 2026-11-03 07:03:18 +00:00
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 <anhle12892@gmail.com> Co-authored-by: ykurilov <110710320+ykurilov@users.noreply.github.com>
This commit is contained in:
co-authored by
Kevin Le
ykurilov
parent
d2c372d78b
commit
b6511ea5f9
@@ -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"]);
|
||||
});
|
||||
|
||||
@@ -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) => <MeasuredTestRow item={rowItem} />;
|
||||
const duplicate = userMessage(2);
|
||||
const historyVirtualized = [
|
||||
userMessage(0),
|
||||
userMessage(1),
|
||||
{ ...duplicate, text: "grown" },
|
||||
duplicate,
|
||||
userMessage(3),
|
||||
];
|
||||
const renderRow = (rowItem: StreamItem) => (
|
||||
<MeasuredTestRow
|
||||
item={rowItem}
|
||||
measuredHeight={rowItem.kind === "plugin" && rowItem.sourceId === "message-2" ? 120 : 24}
|
||||
/>
|
||||
);
|
||||
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<HTMLElement>("[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)",
|
||||
|
||||
@@ -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<string>();
|
||||
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);
|
||||
|
||||
@@ -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([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user