From 42c0eb1df9464f897f6bda6d4b52e0dd1bb56d0b Mon Sep 17 00:00:00 2001 From: "@taltas" <6816042+taltas@users.noreply.github.com> Date: Mon, 31 Aug 2026 18:55:12 +0000 Subject: [PATCH 1/6] perf(webview): reduce retained chat state --- webview-ui/src/App.tsx | 2 + webview-ui/src/__tests__/App.spec.tsx | 35 +++++++- webview-ui/src/components/chat/ChatView.tsx | 5 +- .../chat/__tests__/ChatView.spec.tsx | 5 +- .../src/context/ExtensionStateContext.tsx | 58 ++++++++++++-- .../__tests__/ExtensionStateContext.spec.tsx | 80 +++++++++++++++++++ .../src/utils/__tests__/stringCache.spec.ts | 42 ++++++++++ webview-ui/src/utils/stringCache.ts | 61 ++++++++++++++ 8 files changed, 279 insertions(+), 9 deletions(-) create mode 100644 webview-ui/src/utils/__tests__/stringCache.spec.ts create mode 100644 webview-ui/src/utils/stringCache.ts diff --git a/webview-ui/src/App.tsx b/webview-ui/src/App.tsx index b1fbf82999..0c527e4e5b 100644 --- a/webview-ui/src/App.tsx +++ b/webview-ui/src/App.tsx @@ -55,6 +55,7 @@ const App = () => { useThemeFixtureProbe() const { + currentTaskId, didHydrateState, showWelcome, settingsImportedAt, @@ -260,6 +261,7 @@ const App = () => { /> )} ({ vscode: { postMessage: vi.fn(), @@ -28,8 +30,9 @@ vi.mock("@src/utils/TelemetryClient", () => ({ vi.mock("@src/components/chat/ChatView", () => ({ __esModule: true, default: function ChatView({ isHidden }: { isHidden: boolean }) { + const [instance] = React.useState(() => ++nextChatViewInstance) return ( -
+
Chat View
) @@ -160,6 +163,7 @@ vi.mock("process.env", () => ({ describe("App", () => { beforeEach(() => { vi.clearAllMocks() + nextChatViewInstance = 0 window.removeEventListener("message", () => {}) // Set up default mock return value @@ -205,6 +209,35 @@ describe("App", () => { expect(chatView.getAttribute("data-hidden")).toBe("false") }, 10000) + it("remounts chat on task transitions but preserves it across tab changes", async () => { + const state = { + didHydrateState: true, + showWelcome: false, + shouldShowAnnouncement: false, + experiments: {}, + language: "en", + telemetrySetting: "enabled", + currentTaskId: "task-a" as string | undefined, + } + mockUseExtensionState.mockImplementation(() => state) + + const { rerender } = render() + const firstInstance = screen.getByTestId("chat-view").getAttribute("data-instance") + + act(() => triggerMessage("settingsButtonClicked")) + expect(await screen.findByTestId("settings-view")).toBeInTheDocument() + expect(screen.getByTestId("chat-view").getAttribute("data-instance")).toBe(firstInstance) + + state.currentTaskId = "task-b" + rerender() + const secondInstance = screen.getByTestId("chat-view").getAttribute("data-instance") + expect(secondInstance).not.toBe(firstInstance) + + state.currentTaskId = undefined + rerender() + expect(screen.getByTestId("chat-view").getAttribute("data-instance")).not.toBe(secondInstance) + }) + it("shows welcome view when setup is incomplete", () => { mockUseExtensionState.mockReturnValue({ didHydrateState: true, diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 988c98ba30..8b578aa2fa 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -1510,7 +1510,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction `${messageOrGroup.ts}-${index}`, + (index: number, messageOrGroup: ClineMessage) => + `${messageOrGroup.ts}-${index}-${messageOrGroup.partial ? "partial" : "full"}`, [], ) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index 6b2fa177c9..cd75b21577 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -708,7 +708,10 @@ describe("ChatView - Virtualization Configuration", () => { expect(mockVirtuosoState.lastConfig?.defaultItemHeight).toBe(180) expect(mockVirtuosoState.lastConfig?.increaseViewportBy).toEqual({ top: 600, bottom: 800 }) - expect(mockVirtuosoState.lastConfig?.computeItemKey?.(1, { type: "say", ts: rowTs })).toBe(`${rowTs}-1`) + expect(mockVirtuosoState.lastConfig?.computeItemKey?.(1, { type: "say", ts: rowTs })).toBe(`${rowTs}-1-full`) + expect(mockVirtuosoState.lastConfig?.computeItemKey?.(1, { type: "say", ts: rowTs, partial: true })).toBe( + `${rowTs}-1-partial`, + ) }) }) diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 377c8eb721..0518995319 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -1,4 +1,4 @@ -import React, { createContext, useCallback, useEffect, useState } from "react" +import React, { createContext, useCallback, useEffect, useRef, useState } from "react" import { type ProviderSettings, @@ -33,6 +33,7 @@ import { experimentDefault } from "@roo/experiments" import { vscode } from "@src/utils/vscode" import { convertTextMateToHljs } from "@src/utils/textMateToHljs" +import { StringCache } from "@src/utils/stringCache" export interface ExtensionStateContextType extends ExtensionState { historyPreviewCollapsed?: boolean // Add the new state property @@ -283,9 +284,22 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode initialState?: ExtensionStateProviderInitialState }> = ({ children, initialState }) => { - const [state, setState] = useState(() => - mergeExtensionState(createInitialExtensionState(), initialState ?? {}), - ) + const [historyCache] = useState(() => new StringCache()) + const [messageCache] = useState(() => new StringCache((value) => !("partial" in value) || value.partial !== true)) + const currentTaskIdRef = useRef(initialState?.currentTaskId) + const clineMessagesSeqRef = useRef(initialState?.clineMessagesSeq) + + const [state, setState] = useState(() => { + const mergedState = mergeExtensionState(createInitialExtensionState(), initialState ?? {}) + for (const clineMessage of mergedState.clineMessages) { + messageCache.intern(clineMessage) + } + historyCache.intern(mergedState.taskHistory) + if (mergedState.currentTaskItem) { + historyCache.intern(mergedState.currentTaskItem) + } + return mergedState + }) const [didHydrateState, setDidHydrateState] = useState(false) const [showWelcome, setShowWelcome] = useState(false) @@ -341,6 +355,37 @@ export const ExtensionStateContextProvider: React.FC<{ switch (message.type) { case "state": { const newState = message.state ?? {} + if (Object.prototype.hasOwnProperty.call(newState, "currentTaskId")) { + if (newState.currentTaskId !== currentTaskIdRef.current) { + messageCache.clear() + currentTaskIdRef.current = newState.currentTaskId + } + } + + const messagesAreStale = + newState.clineMessages !== undefined && + newState.clineMessagesSeq !== undefined && + clineMessagesSeqRef.current !== undefined && + newState.clineMessagesSeq <= clineMessagesSeqRef.current + + if (newState.clineMessages && !messagesAreStale) { + for (const clineMessage of newState.clineMessages) { + messageCache.intern(clineMessage) + } + } + if ( + newState.clineMessagesSeq !== undefined && + (clineMessagesSeqRef.current === undefined || + newState.clineMessagesSeq > clineMessagesSeqRef.current) + ) { + clineMessagesSeqRef.current = newState.clineMessagesSeq + } + if (newState.taskHistory) { + historyCache.intern(newState.taskHistory) + } + if (newState.currentTaskItem) { + historyCache.intern(newState.currentTaskItem) + } setState((prevState) => mergeExtensionState(prevState, newState)) setShowWelcome(!checkExistKey(newState.apiConfiguration, newState.zooCodeIsAuthenticated)) setDidHydrateState(true) @@ -409,6 +454,7 @@ export const ExtensionStateContextProvider: React.FC<{ // worth noting it will never be possible for a more up-to-date message to be sent here or in normal messages post since the presentAssistantContent function uses lock const lastIndex = findLastIndex(prevState.clineMessages, (msg) => msg.ts === clineMessage.ts) if (lastIndex !== -1) { + messageCache.intern(clineMessage) const newClineMessages = [...prevState.clineMessages] newClineMessages[lastIndex] = clineMessage return { ...prevState, clineMessages: newClineMessages } @@ -469,6 +515,7 @@ export const ExtensionStateContextProvider: React.FC<{ case "taskHistoryUpdated": { // Efficiently update just the task history without replacing entire state if (message.taskHistory !== undefined) { + historyCache.intern(message.taskHistory) setState((prevState) => ({ ...prevState, taskHistory: message.taskHistory!, @@ -481,6 +528,7 @@ export const ExtensionStateContextProvider: React.FC<{ if (!item) { break } + historyCache.intern(item) setState((prevState) => { const existingIndex = prevState.taskHistory.findIndex((h) => h.id === item.id) let nextHistory: typeof prevState.taskHistory @@ -503,7 +551,7 @@ export const ExtensionStateContextProvider: React.FC<{ } } }, - [setListApiConfigMeta], + [historyCache, messageCache, setListApiConfigMeta], ) useEffect(() => { diff --git a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx index 23ac911585..44f26ae5e9 100644 --- a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx +++ b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx @@ -14,6 +14,7 @@ import { } from "@roo-code/types" import { ExtensionStateContextProvider, useExtensionState, mergeExtensionState } from "../ExtensionStateContext" +import { StringCache } from "@/utils/stringCache" const TestComponent = () => { const { allowedCommands, setAllowedCommands, soundEnabled, showRooIgnoredFiles, setShowRooIgnoredFiles } = @@ -103,6 +104,85 @@ const InitialStateTestComponent = () => { } describe("ExtensionStateContext", () => { + it("clears the task-scoped message cache only on explicit task transitions", () => { + const clearSpy = vi.spyOn(StringCache.prototype, "clear") + render( + + + , + ) + clearSpy.mockClear() + + act(() => { + window.dispatchEvent( + new MessageEvent("message", { data: { type: "state", state: { currentTaskId: "task-a" } } }), + ) + window.dispatchEvent(new MessageEvent("message", { data: { type: "state", state: { language: "en" } } })) + }) + expect(clearSpy).not.toHaveBeenCalled() + + act(() => { + window.dispatchEvent( + new MessageEvent("message", { data: { type: "state", state: { currentTaskId: undefined } } }), + ) + window.dispatchEvent( + new MessageEvent("message", { data: { type: "state", state: { currentTaskId: "task-b" } } }), + ) + }) + expect(clearSpy).toHaveBeenCalledTimes(2) + }) + + it("does not intern messages from stale sequenced state updates", () => { + const internSpy = vi.spyOn(StringCache.prototype, "intern") + render( + + + , + ) + internSpy.mockClear() + const staleMessage = { ts: 1, type: "say", say: "text", text: "stale" } as ClineMessage + const currentMessage = { ts: 2, type: "say", say: "text", text: "current" } as ClineMessage + + act(() => { + window.dispatchEvent( + new MessageEvent("message", { + data: { type: "state", state: { clineMessages: [staleMessage], clineMessagesSeq: 4 } }, + }), + ) + }) + expect(internSpy).not.toHaveBeenCalledWith(staleMessage) + + act(() => { + window.dispatchEvent( + new MessageEvent("message", { + data: { type: "state", state: { clineMessages: [currentMessage], clineMessagesSeq: 6 } }, + }), + ) + }) + expect(internSpy).toHaveBeenCalledWith(currentMessage) + }) + + it("does not intern an update for a message outside the current task", () => { + const currentMessage = { ts: 1, type: "say", say: "text", text: "current" } as ClineMessage + const unknownMessage = { ts: 2, type: "say", say: "text", text: "late" } as ClineMessage + const internSpy = vi.spyOn(StringCache.prototype, "intern") + vi.spyOn(console, "warn").mockImplementation(() => undefined) + render( + + + , + ) + internSpy.mockClear() + + act(() => { + window.dispatchEvent( + new MessageEvent("message", { data: { type: "messageUpdated", clineMessage: unknownMessage } }), + ) + }) + + expect(internSpy).not.toHaveBeenCalledWith(unknownMessage) + }) + it("initializes with empty allowedCommands array", () => { render( diff --git a/webview-ui/src/utils/__tests__/stringCache.spec.ts b/webview-ui/src/utils/__tests__/stringCache.spec.ts new file mode 100644 index 0000000000..e699c898c8 --- /dev/null +++ b/webview-ui/src/utils/__tests__/stringCache.spec.ts @@ -0,0 +1,42 @@ +import { StringCache } from "../stringCache" + +describe("StringCache", () => { + it("interns strings recursively without retaining duplicate or empty values", () => { + const cache = new StringCache() + const value = { + label: "shared", + nested: [{ label: "shared" }, { label: "other" }], + empty: "", + count: 1, + } + + expect(cache.intern(value)).toBe(value) + expect(cache.size).toBe(2) + + cache.intern({ label: "shared", another: "other" }) + expect(cache.size).toBe(2) + }) + + it("skips values rejected by its filter while interning accepted siblings", () => { + const cache = new StringCache((value) => !("partial" in value) || value.partial !== true) + const partial = { partial: true, text: "streaming", nested: { text: "transient" } } + const complete = { partial: false, text: "complete", nested: { partial: true, text: "still-complete" } } + + cache.intern(partial) + expect(cache.size).toBe(0) + + cache.intern(complete) + expect(cache.size).toBe(2) + }) + + it("clears interned values and can be reused", () => { + const cache = new StringCache() + cache.intern({ text: "first" }) + + cache.clear() + expect(cache.size).toBe(0) + + cache.intern({ text: "second" }) + expect(cache.size).toBe(1) + }) +}) diff --git a/webview-ui/src/utils/stringCache.ts b/webview-ui/src/utils/stringCache.ts new file mode 100644 index 0000000000..1fe34e7cca --- /dev/null +++ b/webview-ui/src/utils/stringCache.ts @@ -0,0 +1,61 @@ +export type StringCacheFilter = (value: object) => boolean + +/** + * Rebinds strings in deserialized values to canonical instances held by this cache. + * Values are mutated in place to avoid allocating another object tree per message. + */ +export class StringCache { + private readonly cache = new Map() + + constructor(private readonly shouldIntern?: StringCacheFilter) {} + + intern(value: T): T { + if (value !== null && typeof value === "object" && this.shouldIntern?.(value) === false) { + return value + } + + return this.internValue(value) as T + } + + clear(): void { + this.cache.clear() + } + + get size(): number { + return this.cache.size + } + + private internValue(value: unknown): unknown { + if (typeof value === "string") { + if (value.length === 0) { + return value + } + + const existing = this.cache.get(value) + if (existing !== undefined) { + return existing + } + + this.cache.set(value, value) + return value + } + + if (Array.isArray(value)) { + for (let index = 0; index < value.length; index++) { + value[index] = this.internValue(value[index]) + } + return value + } + + if (value === null || typeof value !== "object") { + return value + } + + const record = value as Record + for (const key of Object.keys(record)) { + record[key] = this.internValue(record[key]) + } + + return value + } +} From b9a6ba03cb93c0f92f06cfb00c82ae7d488db7cb Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Tue, 27 Oct 2026 03:50:56 +0000 Subject: [PATCH 2/6] fix(webview): bound string caches and guard task identity updates --- webview-ui/src/App.tsx | 2 - webview-ui/src/__tests__/App.spec.tsx | 6 +- webview-ui/src/components/chat/ChatView.tsx | 5 +- .../chat/__tests__/ChatView.spec.tsx | 27 ++- .../src/context/ExtensionStateContext.tsx | 47 +++- .../__tests__/ExtensionStateContext.spec.tsx | 220 +++++++++++++++++- .../src/utils/__tests__/stringCache.spec.ts | 27 +++ webview-ui/src/utils/stringCache.ts | 20 +- 8 files changed, 330 insertions(+), 24 deletions(-) diff --git a/webview-ui/src/App.tsx b/webview-ui/src/App.tsx index e467115837..3e05ce64da 100644 --- a/webview-ui/src/App.tsx +++ b/webview-ui/src/App.tsx @@ -55,7 +55,6 @@ const App = () => { useThemeFixtureProbe() const { - currentTaskId, didHydrateState, showWelcome, settingsImportedAt, @@ -272,7 +271,6 @@ const App = () => { /> )} { expect(chatView.getAttribute("data-hidden")).toBe("false") }, 10000) - it("remounts chat on task transitions but preserves it across tab changes", async () => { + it("preserves chat state across task and tab changes", async () => { const state = { didHydrateState: true, showWelcome: false, @@ -232,11 +232,11 @@ describe("App", () => { state.currentTaskId = "task-b" rerender() const secondInstance = screen.getByTestId("chat-view").getAttribute("data-instance") - expect(secondInstance).not.toBe(firstInstance) + expect(secondInstance).toBe(firstInstance) state.currentTaskId = undefined rerender() - expect(screen.getByTestId("chat-view").getAttribute("data-instance")).not.toBe(secondInstance) + expect(screen.getByTestId("chat-view").getAttribute("data-instance")).toBe(secondInstance) }) it.each(["sidebar", "editor"])( diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index c0a52439aa..bb690c1f03 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -1518,7 +1518,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction - `${messageOrGroup.ts}-${index}-${messageOrGroup.partial ? "partial" : "full"}`, + (index: number, messageOrGroup: ClineMessage) => `${messageOrGroup.ts}-${index}`, [], ) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index f47786a3be..7fa4e81131 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -6,6 +6,7 @@ import { mockVscodePostMessage, renderWithExtensionState, waitFor, + screen, act, fireEvent, } from "@/utils/test-utils" @@ -49,6 +50,8 @@ vi.mock("use-sound", () => ({ }), })) +let nextRowInstance = 0 + // Mock components that use ESM dependencies vi.mock("../ChatRow", () => ({ default: function MockChatRow({ @@ -60,6 +63,7 @@ vi.mock("../ChatRow", () => ({ onSuggestionClick?: (suggestion: SuggestionItem, event?: React.MouseEvent) => void isFollowUpAnswered?: boolean }) { + const [instance] = React.useState(() => ++nextRowInstance) if (message.type === "ask" && message.ask === "followup" && message.text) { try { const followUp = JSON.parse(message.text) as { suggest?: SuggestionItem[] } @@ -81,7 +85,11 @@ vi.mock("../ChatRow", () => ({ } } - return
{JSON.stringify(message)}
+ return ( +
+ {JSON.stringify(message)} +
+ ) }, })) @@ -721,6 +729,19 @@ describe("ChatView - Virtualization Configuration", () => { mockVirtuosoState.lastConfig = null }) + it("preserves the row instance when a partial message completes", async () => { + renderChatView() + const task: ClineMessage = { type: "say", say: "task", ts: 1, text: "Task" } + const partial: ClineMessage = { type: "say", say: "text", ts: 2, text: "Streaming", partial: true } + mockPostMessage({ clineMessages: [task, partial] }) + const row = await screen.findByTestId("chat-row") + const instance = row.getAttribute("data-instance") + expect(instance).not.toBeNull() + mockPostMessage({ clineMessages: [task, { ...partial, text: "Completed", partial: false }] }) + await waitFor(() => expect(screen.getByTestId("chat-row")).toHaveTextContent("Completed")) + expect(screen.getByTestId("chat-row").getAttribute("data-instance")).toBe(instance) + }) + it("keeps the off-screen render buffer tight for chat rows", async () => { renderChatView() @@ -750,9 +771,9 @@ describe("ChatView - Virtualization Configuration", () => { expect(mockVirtuosoState.lastConfig?.defaultItemHeight).toBe(180) expect(mockVirtuosoState.lastConfig?.increaseViewportBy).toEqual({ top: 600, bottom: 800 }) - expect(mockVirtuosoState.lastConfig?.computeItemKey?.(1, { type: "say", ts: rowTs })).toBe(`${rowTs}-1-full`) + expect(mockVirtuosoState.lastConfig?.computeItemKey?.(1, { type: "say", ts: rowTs })).toBe(`${rowTs}-1`) expect(mockVirtuosoState.lastConfig?.computeItemKey?.(1, { type: "say", ts: rowTs, partial: true })).toBe( - `${rowTs}-1-partial`, + `${rowTs}-1`, ) }) }) diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 2b2bf67689..7c352a2193 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -12,6 +12,7 @@ import { type OrganizationAllowList, type CloudOrganizationMembership, type ExtensionMessage, + type ClineMessage, type ExtensionState, type MarketplaceInstalledMetadata, type SkillMetadata, @@ -186,6 +187,12 @@ export const mergeExtensionState = (prevState: ExtensionState, newState: Partial ) { rest.clineMessages = prevState.clineMessages rest.clineMessagesSeq = prevState.clineMessagesSeq + rest.currentTaskId = prevState.currentTaskId + } else if (newState.clineMessages !== undefined) { + rest.currentTaskId = newState.currentTaskId + } else { + // Pushes without messages cannot establish task identity. + rest.currentTaskId = prevState.currentTaskId } // Note that we completely replace the previous apiConfiguration and customSupportPrompts objects @@ -291,6 +298,7 @@ export const ExtensionStateContextProvider: React.FC<{ }> = ({ children, initialState }) => { const [historyCache] = useState(() => new StringCache()) const [messageCache] = useState(() => new StringCache((value) => !("partial" in value) || value.partial !== true)) + const messagesRef = useRef(initialState?.clineMessages ?? []) const currentTaskIdRef = useRef(initialState?.currentTaskId) const clineMessagesSeqRef = useRef(initialState?.clineMessagesSeq) @@ -360,12 +368,6 @@ export const ExtensionStateContextProvider: React.FC<{ switch (message.type) { case "state": { const newState = message.state ?? {} - if (Object.prototype.hasOwnProperty.call(newState, "currentTaskId")) { - if (newState.currentTaskId !== currentTaskIdRef.current) { - messageCache.clear() - currentTaskIdRef.current = newState.currentTaskId - } - } const messagesAreStale = newState.clineMessages !== undefined && @@ -373,10 +375,31 @@ export const ExtensionStateContextProvider: React.FC<{ clineMessagesSeqRef.current !== undefined && newState.clineMessagesSeq <= clineMessagesSeqRef.current + if (!messagesAreStale && newState.clineMessages !== undefined) { + if (newState.currentTaskId !== currentTaskIdRef.current) { + messageCache.clear() + messagesRef.current = [] + currentTaskIdRef.current = newState.currentTaskId + } + } + if (newState.clineMessages && !messagesAreStale) { + const previousMessages = new Map(messagesRef.current.map((item) => [item.ts, item])) for (const clineMessage of newState.clineMessages) { - messageCache.intern(clineMessage) + const previous = previousMessages.get(clineMessage.ts) + if ( + previous && + Object.keys(clineMessage).length === Object.keys(previous).length && + Object.entries(clineMessage).every( + ([key, value]) => previous[key as keyof ClineMessage] === value, + ) + ) { + Object.assign(clineMessage, previous) + } else { + messageCache.intern(clineMessage) + } } + messagesRef.current = newState.clineMessages } if ( newState.clineMessagesSeq !== undefined && @@ -386,6 +409,7 @@ export const ExtensionStateContextProvider: React.FC<{ clineMessagesSeqRef.current = newState.clineMessagesSeq } if (newState.taskHistory) { + historyCache.clear() historyCache.intern(newState.taskHistory) } if (newState.currentTaskItem) { @@ -455,11 +479,17 @@ export const ExtensionStateContextProvider: React.FC<{ } case "messageUpdated": { const clineMessage = message.clineMessage! + const messageIndex = messagesRef.current.findIndex((item) => item.ts === clineMessage.ts) + if (messageIndex !== -1) { + messageCache.intern(clineMessage) + messagesRef.current = messagesRef.current.map((item, index) => + index === messageIndex ? clineMessage : item, + ) + } setState((prevState) => { // worth noting it will never be possible for a more up-to-date message to be sent here or in normal messages post since the presentAssistantContent function uses lock const lastIndex = findLastIndex(prevState.clineMessages, (msg) => msg.ts === clineMessage.ts) if (lastIndex !== -1) { - messageCache.intern(clineMessage) const newClineMessages = [...prevState.clineMessages] newClineMessages[lastIndex] = clineMessage return { ...prevState, clineMessages: newClineMessages } @@ -520,6 +550,7 @@ export const ExtensionStateContextProvider: React.FC<{ case "taskHistoryUpdated": { // Efficiently update just the task history without replacing entire state if (message.taskHistory !== undefined) { + historyCache.clear() historyCache.intern(message.taskHistory) setState((prevState) => ({ ...prevState, diff --git a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx index 929702607a..f6b4e2ce3a 100644 --- a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx +++ b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx @@ -7,6 +7,7 @@ import { type ExperimentId, type ExtensionState, type ClineMessage, + type ExtensionMessage, type MarketplaceItem, type MarketplaceInstalledMetadata, type RouterModels, @@ -113,13 +114,207 @@ const InitialStateTestComponent = () => { } describe("ExtensionStateContext", () => { + afterEach(() => vi.restoreAllMocks()) + it.each(["initial", "state", "messageUpdated", "taskHistoryUpdated", "taskHistoryItemUpdated"] as const)( + "interns accepted payloads through %s", + (entry) => { + const internSpy = vi.spyOn(StringCache.prototype, "intern") + const payload: ClineMessage = { ts: 1, type: "say", say: "text", text: "accepted" } + const item = { id: "task-a", number: 1, ts: 1, task: "history", tokensIn: 0, tokensOut: 0, totalCost: 0 } + render( + + + , + ) + if (entry === "initial") { + expect(internSpy).toHaveBeenCalledWith(payload) + const cache = internSpy.mock.contexts[0] + expect(cache).toBeInstanceOf(StringCache) + if (!(cache instanceof StringCache)) throw new Error("Missing cache") + expect(typeof cache["shouldIntern"]).toBe("function") + expect(cache.size).toBeGreaterThan(0) + return + } + const caches = internSpy.mock.contexts.filter((cache): cache is StringCache => cache instanceof StringCache) + const targetCache = caches.find( + (cache) => (cache["shouldIntern"] !== undefined) === (entry === "state" || entry === "messageUpdated"), + ) + if (!targetCache) throw new Error("Missing target cache") + const sizeBefore = targetCache.size + internSpy.mockClear() + const messages: Record, ExtensionMessage> = { + state: { type: "state", state: { clineMessages: [{ ...payload, text: "changed" }] } }, + messageUpdated: { type: "messageUpdated", clineMessage: { ...payload, text: "updated" } }, + taskHistoryUpdated: { type: "taskHistoryUpdated", taskHistory: [item] }, + taskHistoryItemUpdated: { type: "taskHistoryItemUpdated", taskHistoryItem: item }, + } + act(() => window.dispatchEvent(new MessageEvent("message", { data: messages[entry] }))) + expect(internSpy.mock.contexts).toEqual([targetCache]) + expect(targetCache.size).toBeGreaterThan(sizeBefore) + expect(internSpy).toHaveBeenCalledWith( + entry === "state" + ? expect.objectContaining({ text: "changed" }) + : entry === "messageUpdated" + ? { ...payload, text: "updated" } + : entry === "taskHistoryUpdated" + ? [item] + : item, + ) + }, + ) + + it.each(["task switch", "message update"] as const)( + "keeps the message mirror synchronized after %s", + (scenario) => { + const internSpy = vi.spyOn(StringCache.prototype, "intern") + const payload: ClineMessage = { ts: 1, type: "say", say: "text", text: "seed" } + render( + + + , + ) + const dispatch = (data: ExtensionMessage) => + act(() => window.dispatchEvent(new MessageEvent("message", { data }))) + if (scenario === "task switch") { + internSpy.mockClear() + dispatch({ type: "state", state: { currentTaskId: "task-b", clineMessages: [{ ...payload }] } }) + expect(internSpy).toHaveBeenCalledWith(payload) + } else { + const updated = { ...payload, text: "updated" } + dispatch({ type: "messageUpdated", clineMessage: updated }) + internSpy.mockClear() + dispatch({ type: "state", state: { currentTaskId: "task-a", clineMessages: [{ ...updated }] } }) + expect(internSpy).not.toHaveBeenCalledWith(updated) + } + }, + ) + + it("ignores task identity in pushes without messages", () => { + const initial = { ...createInitialExtensionState(), currentTaskId: "task-a" } + expect(mergeExtensionState(initial, { currentTaskId: "old" }).currentTaskId).toBe("task-a") + const internSpy = vi.spyOn(StringCache.prototype, "intern") + const payload: ClineMessage = { ts: 1, type: "say", say: "text", text: "seed" } + render( + + + , + ) + const cache = internSpy.mock.contexts[0] + if (!(cache instanceof StringCache)) throw new Error("Missing cache") + const size = cache.size + act(() => + window.dispatchEvent( + new MessageEvent("message", { data: { type: "state", state: { currentTaskId: "old" } } }), + ), + ) + expect(cache.size).toBe(size) + internSpy.mockClear() + act(() => + window.dispatchEvent( + new MessageEvent("message", { + data: { type: "state", state: { currentTaskId: "task-a", clineMessages: [{ ...payload }] } }, + }), + ), + ) + expect(internSpy).not.toHaveBeenCalledWith(payload) + }) + + it("does not retain partial messages or re-intern unchanged snapshot messages", () => { + const internSpy = vi.spyOn(StringCache.prototype, "intern") + const payload: ClineMessage = { ts: 1, type: "say", say: "text", text: "streaming", partial: true } + render( + + + , + ) + const cache = internSpy.mock.contexts[0] + if (!(cache instanceof StringCache)) throw new Error("Missing message cache") + expect(cache?.size).toBe(0) + internSpy.mockClear() + act(() => + window.dispatchEvent( + new MessageEvent("message", { data: { type: "state", state: { clineMessages: [{ ...payload }] } } }), + ), + ) + expect(internSpy).not.toHaveBeenCalledWith(payload) + expect(cache?.size).toBe(0) + }) + + it("clears a serialized absent task id and preserves task identity on stale snapshots", () => { + const initial = { ...createInitialExtensionState(), currentTaskId: "task-a", clineMessagesSeq: 5 } + const cleared = mergeExtensionState( + initial, + JSON.parse(JSON.stringify({ currentTaskId: undefined, clineMessages: [], clineMessagesSeq: 6 })), + ) + expect(cleared.currentTaskId).toBeUndefined() + const stale = mergeExtensionState(initial, { + currentTaskId: "old-task", + clineMessages: [], + clineMessagesSeq: 4, + }) + expect(stale.currentTaskId).toBe("task-a") + }) + + it("preserves the message cache on stale task pushes and rebuilds replaced history", () => { + const internSpy = vi.spyOn(StringCache.prototype, "intern") + render( + + + , + ) + const messageCache = internSpy.mock.contexts[0] + const historyCache = internSpy.mock.contexts[1] + if (!(messageCache instanceof StringCache) || !(historyCache instanceof StringCache)) + throw new Error("Missing caches") + const messageSize = messageCache.size + historyCache.intern("removed-history") + act(() => + window.dispatchEvent( + new MessageEvent("message", { + data: { type: "state", state: { currentTaskId: "old", clineMessages: [], clineMessagesSeq: 4 } }, + }), + ), + ) + expect(messageCache.size).toBe(messageSize) + act(() => + window.dispatchEvent( + new MessageEvent("message", { data: { type: "taskHistoryUpdated", taskHistory: [] } }), + ), + ) + expect(historyCache.size).toBe(0) + expect(messageCache.size).toBe(messageSize) + historyCache.intern("removed-again") + act(() => + window.dispatchEvent(new MessageEvent("message", { data: { type: "state", state: { taskHistory: [] } } })), + ) + expect(historyCache.size).toBe(0) + }) + it("clears the task-scoped message cache only on explicit task transitions", () => { + const internSpy = vi.spyOn(StringCache.prototype, "intern") const clearSpy = vi.spyOn(StringCache.prototype, "clear") render( - + , ) + const messageCache = internSpy.mock.contexts[0] + const historyCache = internSpy.mock.contexts[1] + if (!(messageCache instanceof StringCache) || !(historyCache instanceof StringCache)) { + throw new Error("Missing provider caches") + } + expect(messageCache?.size).toBeGreaterThan(0) + historyCache?.intern("history-seed") + const historySize = historyCache?.size clearSpy.mockClear() act(() => { @@ -132,13 +327,21 @@ describe("ExtensionStateContext", () => { act(() => { window.dispatchEvent( - new MessageEvent("message", { data: { type: "state", state: { currentTaskId: undefined } } }), + new MessageEvent("message", { + data: JSON.parse( + JSON.stringify({ type: "state", state: { currentTaskId: undefined, clineMessages: [] } }), + ), + }), ) window.dispatchEvent( - new MessageEvent("message", { data: { type: "state", state: { currentTaskId: "task-b" } } }), + new MessageEvent("message", { + data: { type: "state", state: { currentTaskId: "task-b", clineMessages: [] } }, + }), ) }) expect(clearSpy).toHaveBeenCalledTimes(2) + expect(messageCache?.size).toBe(0) + expect(historyCache?.size).toBe(historySize) }) it("does not intern messages from stale sequenced state updates", () => { @@ -155,7 +358,7 @@ describe("ExtensionStateContext", () => { act(() => { window.dispatchEvent( new MessageEvent("message", { - data: { type: "state", state: { clineMessages: [staleMessage], clineMessagesSeq: 4 } }, + data: { type: "state", state: { clineMessages: [staleMessage], clineMessagesSeq: 5 } }, }), ) }) @@ -169,6 +372,15 @@ describe("ExtensionStateContext", () => { ) }) expect(internSpy).toHaveBeenCalledWith(currentMessage) + internSpy.mockClear() + act(() => { + window.dispatchEvent( + new MessageEvent("message", { + data: { type: "state", state: { clineMessages: [staleMessage], clineMessagesSeq: 6 } }, + }), + ) + }) + expect(internSpy).not.toHaveBeenCalledWith(staleMessage) }) it("does not intern an update for a message outside the current task", () => { diff --git a/webview-ui/src/utils/__tests__/stringCache.spec.ts b/webview-ui/src/utils/__tests__/stringCache.spec.ts index e699c898c8..7ef523d91d 100644 --- a/webview-ui/src/utils/__tests__/stringCache.spec.ts +++ b/webview-ui/src/utils/__tests__/stringCache.spec.ts @@ -1,6 +1,33 @@ import { StringCache } from "../stringCache" describe("StringCache", () => { + it.each([ + { entries: 2, characters: 1000, values: ["a", "b", "c"], retained: ["b", "c"] }, + { entries: 2, characters: 3, values: ["abc"], retained: ["abc"] }, + { entries: 0, characters: 1000, values: ["a"], retained: [] }, + ])( + "honors independent limits: $entries entries, $characters characters", + ({ entries, characters, values, retained }) => { + const cache = new StringCache(undefined, entries, characters) + cache.intern(values) + expect([...cache["cache"].keys()]).toEqual(retained) + }, + ) + + it("bounds entry count and retained characters, skipping oversized strings", () => { + const cache = new StringCache(undefined, 2, 6) + cache.intern(["aaa", "bbb", "ccc"]) + expect(cache.size).toBe(2) + expect([...cache["cache"].keys()]).toEqual(["bbb", "ccc"]) + cache.intern("oversized") + expect(cache.size).toBe(2) + cache.intern("dddd") + expect([...cache["cache"].keys()]).toEqual(["dddd"]) + cache.clear() + cache.intern(["aaa", "bbb"]) + expect(cache.size).toBe(2) + }) + it("interns strings recursively without retaining duplicate or empty values", () => { const cache = new StringCache() const value = { diff --git a/webview-ui/src/utils/stringCache.ts b/webview-ui/src/utils/stringCache.ts index 1fe34e7cca..f969525bf1 100644 --- a/webview-ui/src/utils/stringCache.ts +++ b/webview-ui/src/utils/stringCache.ts @@ -2,12 +2,19 @@ export type StringCacheFilter = (value: object) => boolean /** * Rebinds strings in deserialized values to canonical instances held by this cache. + * FIFO eviction bounds both entry overhead and retained UTF-16 characters. * Values are mutated in place to avoid allocating another object tree per message. */ export class StringCache { private readonly cache = new Map() - constructor(private readonly shouldIntern?: StringCacheFilter) {} + private retainedCharacters = 0 + + constructor( + private readonly shouldIntern?: StringCacheFilter, + private readonly maxEntries = 10_000, + private readonly maxCharacters = 2_000_000, + ) {} intern(value: T): T { if (value !== null && typeof value === "object" && this.shouldIntern?.(value) === false) { @@ -19,6 +26,7 @@ export class StringCache { clear(): void { this.cache.clear() + this.retainedCharacters = 0 } get size(): number { @@ -36,7 +44,17 @@ export class StringCache { return existing } + if (this.maxEntries <= 0 || value.length > this.maxCharacters) { + return value + } + while (this.cache.size >= this.maxEntries || this.retainedCharacters + value.length > this.maxCharacters) { + const oldest = this.cache.keys().next().value + if (oldest === undefined) break + this.cache.delete(oldest) + this.retainedCharacters -= oldest.length + } this.cache.set(value, value) + this.retainedCharacters += value.length return value } From d8832b273cb6335de8372036f038d37369e09c7e Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Tue, 27 Oct 2026 20:51:52 +0000 Subject: [PATCH 3/6] test(webview): add real VS Code heap benchmark and results --- .gitignore | 1 + apps/vscode-e2e/package.json | 5 +- apps/vscode-e2e/playwright-heap.config.ts | 16 ++ apps/vscode-e2e/src/heap/README.md | 60 ++++++ apps/vscode-e2e/src/heap/RESULTS.md | 72 +++++++ apps/vscode-e2e/src/heap/electron.heap.ts | 204 ++++++++++++++++++++ apps/vscode-e2e/src/heap/prepare.ts | 82 ++++++++ apps/vscode-e2e/src/heap/sceneController.ts | 112 +++++++++++ apps/vscode-e2e/src/heap/summarize.ts | 90 +++++++++ apps/vscode-e2e/src/heap/workload.ts | 32 +++ 10 files changed, 673 insertions(+), 1 deletion(-) create mode 100644 apps/vscode-e2e/playwright-heap.config.ts create mode 100644 apps/vscode-e2e/src/heap/README.md create mode 100644 apps/vscode-e2e/src/heap/RESULTS.md create mode 100644 apps/vscode-e2e/src/heap/electron.heap.ts create mode 100644 apps/vscode-e2e/src/heap/prepare.ts create mode 100644 apps/vscode-e2e/src/heap/sceneController.ts create mode 100644 apps/vscode-e2e/src/heap/summarize.ts create mode 100644 apps/vscode-e2e/src/heap/workload.ts diff --git a/.gitignore b/.gitignore index 3961778d5e..1d4b255cea 100644 --- a/.gitignore +++ b/.gitignore @@ -27,6 +27,7 @@ AGENTS.local.md .vscode-test/ apps/vscode-e2e/playwright-report-electron/ apps/vscode-e2e/test-results-electron/ +apps/vscode-e2e/heap-results/ # Docs docs/_site/ diff --git a/apps/vscode-e2e/package.json b/apps/vscode-e2e/package.json index 738d315357..678b45b857 100644 --- a/apps/vscode-e2e/package.json +++ b/apps/vscode-e2e/package.json @@ -15,7 +15,10 @@ "themes:test": "pnpm --filter @roo-code/types build && rimraf out && tsc -p tsconfig.json && node --test ./out/theme-fixtures/fixtures.test.js", "visual:build": "pnpm themes:build", "test:visual:electron": "pnpm visual:build && playwright test -c playwright-electron.config.ts", - "clean": "rimraf out .turbo" + "clean": "rimraf out .turbo", + "heap:build": "pnpm -w bundle && tsc -p tsconfig.json && node out/heap/prepare.js", + "test:heap": "playwright test -c playwright-heap.config.ts", + "heap:summarize": "node out/heap/summarize.js" }, "devDependencies": { "@playwright/test": "1.62.1", diff --git a/apps/vscode-e2e/playwright-heap.config.ts b/apps/vscode-e2e/playwright-heap.config.ts new file mode 100644 index 0000000000..ef43a70c37 --- /dev/null +++ b/apps/vscode-e2e/playwright-heap.config.ts @@ -0,0 +1,16 @@ +import path from "path" +import { defineConfig } from "@playwright/test" + +const results = process.env.HEAP_BENCHMARK_DIR ?? path.resolve(__dirname, "heap-results") +export default defineConfig({ + testDir: "./src/heap", + testMatch: "electron.heap.ts", + outputDir: path.join(results, "runs"), + fullyParallel: false, + workers: 1, + repeatEach: Number(process.env.HEAP_REPEATS ?? 3), + timeout: 5 * 60_000, + expect: { timeout: 90_000 }, + projects: [{ name: "base" }, { name: "candidate" }], + reporter: [["list"], ["json", { outputFile: path.join(results, "playwright.json") }]], +}) diff --git a/apps/vscode-e2e/src/heap/README.md b/apps/vscode-e2e/src/heap/README.md new file mode 100644 index 0000000000..16647ca9c0 --- /dev/null +++ b/apps/vscode-e2e/src/heap/README.md @@ -0,0 +1,60 @@ +# Webview heap benchmark + +See [RESULTS.md](./RESULTS.md) for the recorded comparison and its retention findings. + +This opt-in diagnostic runs real VS Code, the extension host, task-history hydration, +extension/webview messaging and the production React webview. The model is a local +mock. It does not call a paid API or mutate your normal VS Code profile. + +```sh +pnpm --dir apps/vscode-e2e heap:build +xvfb-run -a pnpm --dir apps/vscode-e2e test:heap +pnpm --dir apps/vscode-e2e heap:summarize +``` + +`heap:build` creates isolated base and candidate webview builds in `heap-results/builds`. +The extension host is identical for both. Base replaces the three production webview +modules changed by PR #1466 with their versions at `git merge-base HEAD origin/main`. +Override that reference with `HEAP_BASE_REF`. Source maps are disabled for both builds +to reduce build memory; production minification is preserved. Source files in your +checkout are untouched. + +Each repetition creates two tasks through the exported API, evicts them, and stops +VS Code. With no extension host alive, it replaces the large task's persisted UI +transcript with 602 deterministic messages: +300 completed command/output pairs plus a title and completion. Each output contains +256 log lines (roughly 17 KiB). `repeated` uses 32 distinct output texts; `unique` uses 300. The isolated profile is restarted, and the normal task-resume path reads and posts +the transcript. This represents a +reproducible large saved task, not a recording of a human or live model session. + +The driver waits for task-resume events, extension readiness files and rendered +completion markers. It requests a normal full-state refresh after each resume. It +then waits for fonts and two animation frames, forces GC through CDP attached to the +Zoo Code content frame, records `Runtime.getHeapUsage`, and streams a heap snapshot +to disk. This happens with no task loaded, with the large task loaded, and after +switching to a small task. A final snapshot can expose strings retained after the +task transition. This measures the webview renderer isolate, not extension-host heap +or whole-process RSS. External HTTP traffic is blocked (only the local mock model is reachable). +Playwright tracing is disabled to avoid retaining captured DOM +snapshots and message text during measurement. + +The default is three fresh sessions per workload and variant (12 sessions). There is +no pass/fail memory threshold. Inspect distributions, baseline-adjusted differences, +and retaining paths before drawing conclusions. The summarizer reports median heap +sizes and counts large output string nodes in the loaded and switched snapshots. +It does not infer cache ownership solely from a surviving string. + +Artifacts are ignored by git under `apps/vscode-e2e/heap-results/`: build provenance, +per-run `measurement.json`, three `.heapsnapshot` files, `summary.json` and `summary.md`. +Load snapshots in Chrome DevTools' Memory panel for retaining-path analysis. Temporary +profiles, workspaces and extension directories are removed on both success and failure; +failure logs and screenshots are copied to the run artifacts first. + +Optional environment variables (use the same `HEAP_BENCHMARK_DIR` for all three commands): + +- `HEAP_BENCHMARK_DIR`: absolute artifact/build directory. +- `HEAP_BASE_REF`: base commit or ref used by the comparison build. +- `HEAP_REPEATS`: repetitions per workload and variant (default 3). +- `HEAP_VSCODE_EXECUTABLE`: existing VS Code executable; otherwise download the pinned version. + +For a smoke run: `HEAP_REPEATS=1 ... test:heap --project=candidate --grep=repeated`. diff --git a/apps/vscode-e2e/src/heap/RESULTS.md b/apps/vscode-e2e/src/heap/RESULTS.md new file mode 100644 index 0000000000..714d215522 --- /dev/null +++ b/apps/vscode-e2e/src/heap/RESULTS.md @@ -0,0 +1,72 @@ +# Webview heap results — 2026-10-27 + +Measured with real VS Code 1.100.0 under Xvfb, using CDP forced GC on the Zoo Code +webview renderer target. These are deterministic saved-task workloads loaded through +the real extension host and task-resume path, not an organic human/model session. +The extension host, dependencies and styles are identical between variants. Both +webview builds use production minification without source maps. External network +requests are blocked except for the local mock model used to create the seed tasks. + +- Base webview modules: `97228c7ca9b032e5fe466e57e24fe72fbf405192`. +- Candidate webview modules: `b9a6ba03cb93c0f92f06cfb00c82ae7d488db7cb`. +- Three fresh measurement sessions per workload and variant: 12 successful samples. +- Each large task has 602 UI messages, including 300 command/output pairs. Repeated + uses 32 distinct large output strings; unique uses 300. Each output is 16.4–16.9 KiB. + A full-state refresh after + hydration exercises reuse across successive deserialized state snapshots. + +Median forced-GC used heap, in MiB: + +| Workload | Variant | Empty | Large task loaded | Loaded − empty | After switching to small task | +| -------- | --------- | ----: | ----------------: | -------------: | ----------------------------: | +| Repeated | Base | 26.46 | 41.42 | 14.71 | 36.19 | +| Repeated | Candidate | 26.57 | 31.75 | 5.19 | 31.85 | +| Unique | Base | 26.78 | 41.58 | 14.80 | 36.27 | +| Unique | Candidate | 26.57 | 36.63 | 10.07 | 36.26 | + +Loaded heap is approximately **9.67 MiB lower (23%)** for repeated outputs and +**4.95 MiB lower (12%)** for unique outputs. Baseline-adjusted task growth falls by +about 9.52 MiB and 4.73 MiB respectively. The unique workload still benefits from +sharing unchanged message strings across full-state pushes. These results do not +measure CPU overhead, image-rich messages, or continuously streamed task execution. + +Loaded-heap ranges across the three sessions: + +| Workload | Base MiB | Candidate MiB | +| -------- | ----------- | ------------- | +| Repeated | 40.96–41.42 | 31.75–32.23 | +| Unique | 41.14–41.59 | 36.62–36.64 | + +## Task-switch retention + +The candidate's StringCache maps retain **zero old output strings after switching** +in all six candidate samples. Before switching, the message cache contains 32 +output strings in the repeated case and 115 in the unique case, consistent with its +character budget. The history cache contains no tool outputs. + +However, old task output strings remain reachable elsewhere in **both** variants: +300 output string nodes on base, versus 32 repeated or 300 unique on candidate. +A shortest strong retaining path inspected in both switched snapshots runs through: + +```text +webview Window → DOM node → __reactProps… → children → props +→ onJumpToPreviousCheckpoint → closure contexts → message array → message.text +``` + +This confirms cache cleanup, but does **not** establish complete task-memory release. +The callback/React-props retaining path needs a separate investigation; this benchmark +only measures the observed post-switch state and does not establish indefinite growth. +No forced remounts or additional product behavior changes were introduced here. + +## Evidence and reproduction + +See [README.md](./README.md) for commands and methodology. The ignored +`apps/vscode-e2e/heap-results/` directory contains the 36 successful snapshots, +per-run measurements, build provenance, generated summary, cache-map inventory, +and representative base/candidate retaining paths under `analysis/`. + +The final sample set consists of 11 successful matrix cases and a successful narrow +rerun of the first base case after fixing the diagnostic's resume-readiness check. +The failed setup attempt has no measurement record and is excluded from the summary; +its failure artifacts remain available. Package type checks and ESLint pass with no +new suppressions. No memory threshold is imposed on CI. diff --git a/apps/vscode-e2e/src/heap/electron.heap.ts b/apps/vscode-e2e/src/heap/electron.heap.ts new file mode 100644 index 0000000000..40b2004343 --- /dev/null +++ b/apps/vscode-e2e/src/heap/electron.heap.ts @@ -0,0 +1,204 @@ +import fs from "fs/promises" +import { closeSync, openSync, readFileSync, writeSync } from "fs" +import os from "os" +import path from "path" +import { LLMock } from "@copilotkit/aimock" +import { downloadAndUnzipVSCode } from "@vscode/test-electron" +import { _electron, expect, test, type CDPSession, type ElectronApplication, type Frame } from "@playwright/test" +import { createTranscript, LARGE_TASK_MARKER, SMALL_TASK_MARKER, type Workload } from "./workload" + +const packageRoot = path.resolve(__dirname, "../..") +const results = process.env.HEAP_BENCHMARK_DIR ?? path.join(packageRoot, "heap-results") +const packageJson = JSON.parse(readFileSync(path.join(packageRoot, "package.json"), "utf8")) as { + devDependencies: Record +} + +async function waitForFile(file: string): Promise { + await expect + .poll( + () => + fs.access(file).then( + () => true, + () => false, + ), + { timeout: 120_000 }, + ) + .toBe(true) +} + +async function capture(client: CDPSession, file: string) { + await client.send("HeapProfiler.collectGarbage") + const usage = await client.send("Runtime.getHeapUsage") + const descriptor = openSync(file, "w") + const receive = ({ chunk }: { chunk: string }) => { + writeSync(descriptor, chunk) + } + client.on("HeapProfiler.addHeapSnapshotChunk", receive) + try { + await client.send("HeapProfiler.takeHeapSnapshot", { reportProgress: false }) + } finally { + client.off("HeapProfiler.addHeapSnapshotChunk", receive) + closeSync(descriptor) + } + return usage +} + +for (const workload of ["repeated", "unique"] satisfies Workload[]) { + test(`webview retained heap: ${workload} tool outputs`, async ({ browserName: _browserName }, testInfo) => { + const temporaryRoot = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-heap-")) + const workspace = path.join(temporaryRoot, "workspace") + const userData = path.join(temporaryRoot, "user-data") + const extensions = path.join(temporaryRoot, "extensions") + await Promise.all([workspace, userData, extensions].map((directory) => fs.mkdir(directory))) + const mock = new LLMock({ port: 0 }) + for (const marker of [SMALL_TASK_MARKER, "HEAP_TRANSCRIPT_SEED"]) { + mock.addFixture({ + match: { userMessage: marker }, + response: { + toolCalls: [ + { + name: "attempt_completion", + arguments: JSON.stringify({ result: `${marker}_COMPLETED` }), + id: `call_${marker}`, + }, + ], + }, + }) + } + let app: ElectronApplication | undefined + let client: CDPSession | undefined + try { + await mock.start() + const executablePath = + process.env.HEAP_VSCODE_EXECUTABLE ?? + (await downloadAndUnzipVSCode({ version: packageJson.devDependencies["@types/vscode"] })) + const launch = (phase: "seed" | "measure") => + _electron.launch({ + executablePath, + args: [ + workspace, + `--extensionDevelopmentPath=${path.join(results, "builds", testInfo.project.name, "src")}`, + `--extensionTestsPath=${path.join(packageRoot, "out/heap/sceneController")}`, + `--user-data-dir=${userData}`, + `--extensions-dir=${extensions}`, + "--disable-workspace-trust", + "--skip-welcome", + "--skip-release-notes", + "--disable-updates", + "--disable-gpu", + "--force-device-scale-factor=1", + ], + env: { + ...process.env, + AIMOCK_URL: mock.url, + HEAP_RUN_DIR: temporaryRoot, + HEAP_WORKLOAD: workload, + HEAP_PHASE: phase, + HTTP_PROXY: "http://127.0.0.1:9", + HTTPS_PROXY: "http://127.0.0.1:9", + ALL_PROXY: "http://127.0.0.1:9", + NO_PROXY: "127.0.0.1,localhost", + }, + locale: "en-US", + timezoneId: "UTC", + timeout: 60_000, + }) + + app = await launch("seed") + await waitForFile(path.join(temporaryRoot, "seed.ready")) + await fs.writeFile(path.join(temporaryRoot, "seed.close"), "close") + await app.close() + app = undefined + // No extension host is alive while replacing the persisted transcript. + const seed = JSON.parse(await fs.readFile(path.join(temporaryRoot, "seed.ready"), "utf8")) as { + largeTaskId: string + } + await fs.writeFile( + path.join(temporaryRoot, "storage/tasks", seed.largeTaskId, "ui_messages.json"), + JSON.stringify(createTranscript(workload)), + ) + app = await launch("measure") + const page = await app.firstWindow() + await app.evaluate(({ BrowserWindow }) => + BrowserWindow.getAllWindows()[0]?.setBounds({ x: 0, y: 0, width: 1200, height: 800 }), + ) + await waitForFile(path.join(temporaryRoot, "empty.ready")) + let content: Frame | undefined + await expect + .poll(() => { + const parent = page + .frames() + .find((frame) => frame.url().includes("extensionId=ZooCodeOrganization.zoo-code")) + content = parent?.childFrames().find((frame) => frame.url().includes("/fake.html")) + return Boolean(content) + }) + .toBe(true) + if (!content) throw new Error("Zoo Code webview content frame not found") + // The content iframe may share its CDP target with the outer webview. + let targetFrame: Frame | null = content + while (targetFrame) { + try { + client = await app.context().newCDPSession(targetFrame) + break + } catch (error) { + if (!(error instanceof Error) || !error.message.includes("does not have a separate CDP session")) + throw error + targetFrame = targetFrame.parentFrame() + } + } + if (!client) throw new Error("No CDP target found for the webview") + await client.send("HeapProfiler.enable") + const target = await client.send("Target.getTargetInfo") + expect(target.targetInfo.url).toContain("vscode-webview") + const stages: Record = {} + for (const stage of ["empty", "loaded", "switched"]) { + await waitForFile(path.join(temporaryRoot, `${stage}.ready`)) + if (stage !== "empty") { + await expect( + content + .getByText(stage === "loaded" ? LARGE_TASK_MARKER : `${SMALL_TASK_MARKER}_COMPLETED`, { + exact: false, + }) + .first(), + ).toBeVisible() + } + await content.evaluate(async () => { + await document.fonts.ready + await new Promise((resolve) => + requestAnimationFrame(() => requestAnimationFrame(() => resolve())), + ) + }) + stages[stage] = await capture(client, testInfo.outputPath(`${stage}.heapsnapshot`)) + await fs.writeFile(path.join(temporaryRoot, `${stage}.continue`), "continue") + } + await waitForFile(path.join(temporaryRoot, "done")) + const measurement = { + variant: testInfo.project.name, + workload, + repeat: testInfo.repeatEachIndex, + vscode: packageJson.devDependencies["@types/vscode"], + target, + stages, + } + await fs.writeFile(testInfo.outputPath("measurement.json"), JSON.stringify(measurement, null, 2)) + await testInfo.attach("measurement", { body: JSON.stringify(measurement), contentType: "application/json" }) + } catch (error) { + if (app) + await ( + await app.firstWindow() + ) + .screenshot({ path: testInfo.outputPath("failure.png") }) + .catch(() => undefined) + await fs + .cp(path.join(userData, "logs"), testInfo.outputPath("logs"), { recursive: true }) + .catch(() => undefined) + throw error + } finally { + await fs.writeFile(path.join(temporaryRoot, "close"), "close").catch(() => undefined) + await client?.detach().catch(() => undefined) + await app?.close().catch(() => undefined) + await mock.stop().catch(() => undefined) + await fs.rm(temporaryRoot, { recursive: true, force: true }) + } + }) +} diff --git a/apps/vscode-e2e/src/heap/prepare.ts b/apps/vscode-e2e/src/heap/prepare.ts new file mode 100644 index 0000000000..152ca17580 --- /dev/null +++ b/apps/vscode-e2e/src/heap/prepare.ts @@ -0,0 +1,82 @@ +import fs from "fs/promises" +import path from "path" +import { execFileSync } from "child_process" + +async function main() { + const repositoryRoot = path.resolve(__dirname, "../../../..") + const results = process.env.HEAP_BENCHMARK_DIR ?? path.join(repositoryRoot, "apps/vscode-e2e/heap-results") + const base = + process.env.HEAP_BASE_REF ?? + execFileSync("git", ["merge-base", "HEAD", "origin/main"], { cwd: repositoryRoot, encoding: "utf8" }).trim() + await fs.mkdir(results, { recursive: true }) + for (const variant of ["base", "candidate"]) { + const root = path.join(results, "builds", variant) + const extension = path.join(root, "src") + await fs.mkdir(extension, { recursive: true }) + await fs.cp(path.join(repositoryRoot, "webview-ui"), path.join(root, "webview-ui"), { + recursive: true, + filter: (source) => + ![ + "node_modules", + "coverage", + "coverage-ct", + "test-results", + "playwright-report", + "monocart-report", + ].includes(path.basename(source)), + }) + const link = async (source: string, target: string) => { + await fs.rm(target, { recursive: true, force: true }) + await fs.symlink(source, target) + } + await link(path.join(repositoryRoot, "node_modules"), path.join(root, "node_modules")) + await link(path.join(repositoryRoot, "webview-ui/node_modules"), path.join(root, "webview-ui/node_modules")) + for (const name of ["dist", "node_modules", "assets", "shared", "integrations"]) + await link(path.join(repositoryRoot, "src", name), path.join(extension, name)) + for (const name of (await fs.readdir(path.join(repositoryRoot, "src"))).filter( + (entry) => entry.startsWith("package") && entry.endsWith(".json"), + )) { + await fs.copyFile(path.join(repositoryRoot, "src", name), path.join(extension, name)) + } + await fs.mkdir(path.join(extension, "webview-ui"), { recursive: true }) + await link(path.join(repositoryRoot, "src/webview-ui/audio"), path.join(extension, "webview-ui/audio")) + if (variant === "base") { + // Hold the host, styles and unrelated code constant; compare the three + // production webview modules changed by this PR with their base versions. + for (const file of ["App.tsx", "components/chat/ChatView.tsx", "context/ExtensionStateContext.tsx"]) { + const contents = execFileSync("git", ["show", `${base}:webview-ui/src/${file}`], { + cwd: repositoryRoot, + }) + await fs.writeFile(path.join(root, "webview-ui/src", file), contents) + } + } + // Source maps do not affect runtime behavior and are omitted in both builds + // to keep this diagnostic usable on memory-constrained runners. + const viteConfig = path.join(root, "webview-ui/vite.config.ts") + await fs.writeFile( + viteConfig, + (await fs.readFile(viteConfig, "utf8")).replace("sourcemap: true,", "sourcemap: false,"), + ) + execFileSync("pnpm", ["exec", "vite", "build"], { + cwd: path.join(root, "webview-ui"), + stdio: "inherit", + env: { ...process.env, RAYON_NUM_THREADS: "2", NODE_OPTIONS: "--max-old-space-size=2048" }, + }) + } + await fs.writeFile( + path.join(results, "builds.json"), + JSON.stringify( + { + base, + candidate: execFileSync("git", ["rev-parse", "HEAD"], { cwd: repositoryRoot, encoding: "utf8" }).trim(), + results, + }, + null, + 2, + ), + ) +} +void main().catch((error: unknown) => { + console.error(error) + process.exitCode = 1 +}) diff --git a/apps/vscode-e2e/src/heap/sceneController.ts b/apps/vscode-e2e/src/heap/sceneController.ts new file mode 100644 index 0000000000..4652237d40 --- /dev/null +++ b/apps/vscode-e2e/src/heap/sceneController.ts @@ -0,0 +1,112 @@ +import fs from "fs/promises" +import path from "path" +import * as vscode from "vscode" +import { providerIdentifiers, RooCodeEventName, type ClineMessage, type RooCodeTestAPI } from "@roo-code/types" +import { SMALL_TASK_MARKER } from "./workload" + +async function poll(capture: () => Promise, description: string): Promise { + const deadline = Date.now() + 120_000 + while (Date.now() < deadline) { + if (await capture()) return + await new Promise((resolve) => setTimeout(resolve, 100)) + } + throw new Error(`Timed out waiting for ${description}`) +} + +export async function run(): Promise { + const runDirectory = process.env.HEAP_RUN_DIR + const aimockUrl = process.env.AIMOCK_URL + if (!runDirectory || !aimockUrl) throw new Error("HEAP_RUN_DIR and AIMOCK_URL are required") + // The isolated host only needs the local model. Keep background catalogs and + // authentication traffic from adding timing-dependent data to the webview. + const originalFetch = globalThis.fetch + globalThis.fetch = async (input, init) => { + const url = typeof input === "string" || input instanceof URL ? String(input) : input.url + return new URL(url).origin === new URL(aimockUrl).origin + ? originalFetch(input, init) + : new Response("{}", { status: 503, statusText: "Heap benchmark: external network disabled" }) + } + try { + await runScene(runDirectory, aimockUrl) + } finally { + globalThis.fetch = originalFetch + } +} + +async function runScene(runDirectory: string, aimockUrl: string): Promise { + const storage = path.join(runDirectory, "storage") + await vscode.workspace + .getConfiguration("zoo-code") + .update("customStoragePath", storage, vscode.ConfigurationTarget.Global) + const extension = vscode.extensions.getExtension("ZooCodeOrganization.zoo-code") + if (!extension) throw new Error("Zoo Code extension not found") + const api = extension.isActive ? extension.exports : await extension.activate() + await vscode.commands.executeCommand("zoo-code.SidebarProvider.focus") + await poll(async () => api.isReady(), "webview activation") + const configuration = { + apiProvider: providerIdentifiers.openrouter, + lastShownAnnouncementId: api.getLatestAnnouncementId(), + openRouterApiKey: "mock-key", + openRouterModelId: "openai/gpt-4.1", + openRouterBaseUrl: `${aimockUrl}/v1`, + telemetrySetting: "disabled" as const, + enableCheckpoints: false, + } + await api.setConfiguration(configuration) + const exists = (name: string) => + fs.access(path.join(runDirectory, name)).then( + () => true, + () => false, + ) + if (process.env.HEAP_PHASE === "seed") { + const createTask = async (marker: string) => { + const taskId = await api.startNewTask({ + text: marker, + configuration: { mode: "ask", autoApprovalEnabled: true }, + }) + await poll(() => api.hasTaskUiMessageText(taskId, `${marker}_COMPLETED`), marker) + await api.clearCurrentTask() + return taskId + } + const smallTaskId = await createTask(SMALL_TASK_MARKER) + const largeTaskId = await createTask("HEAP_TRANSCRIPT_SEED") + await fs.writeFile(path.join(runDirectory, "seed.ready"), JSON.stringify({ largeTaskId, smallTaskId })) + await poll(() => exists("seed.close"), "seed shutdown") + return + } + const { largeTaskId, smallTaskId } = JSON.parse( + await fs.readFile(path.join(runDirectory, "seed.ready"), "utf8"), + ) as { largeTaskId: string; smallTaskId: string } + const resume = async (taskId: string) => { + let ready = false + const listener = (event: { taskId: string; message: ClineMessage }) => { + if ( + event.taskId === taskId && + (event.message.ask === "resume_completed_task" || event.message.ask === "resume_task") && + event.message.partial !== true + ) + ready = true + } + api.on(RooCodeEventName.Message, listener) + try { + await api.resumeTask(taskId) + await poll(async () => ready, `resumed task ${taskId}`) + // Exercise a normal full-state refresh after hydration, and after the + // switch let React commit the small task again before measuring retention. + await api.setConfiguration(configuration) + } finally { + api.off(RooCodeEventName.Message, listener) + } + } + const stage = async (name: string, action: () => Promise) => { + await action() + await fs.writeFile(path.join(runDirectory, `${name}.ready`), JSON.stringify({ largeTaskId, smallTaskId })) + await poll(() => exists(`${name}.continue`), `${name} measurement`) + } + if (api.getCurrentTaskStack().length) throw new Error("Measurement must begin without an active task") + await stage("empty", async () => {}) + await stage("loaded", () => resume(largeTaskId)) + await stage("switched", () => resume(smallTaskId)) + await fs.writeFile(path.join(runDirectory, "done"), "done") + await poll(() => exists("close"), "benchmark shutdown") +} diff --git a/apps/vscode-e2e/src/heap/summarize.ts b/apps/vscode-e2e/src/heap/summarize.ts new file mode 100644 index 0000000000..e4751e7dc8 --- /dev/null +++ b/apps/vscode-e2e/src/heap/summarize.ts @@ -0,0 +1,90 @@ +import fs from "fs/promises" +import path from "path" +import { glob } from "glob" + +interface Snapshot { + snapshot: { meta: { node_fields: string[]; node_types: Array } } + nodes: number[] + strings: string[] +} +interface Measurement { + variant: string + workload: string + repeat: number + stages: Record<"empty" | "loaded" | "switched", { usedSize: number }> +} + +async function outputRetention(file: string) { + const snapshot = JSON.parse(await fs.readFile(file, "utf8")) as Snapshot + const fields = snapshot.snapshot.meta.node_fields + const typeIndex = fields.indexOf("type") + const nameIndex = fields.indexOf("name") + const sizeIndex = fields.indexOf("self_size") + const types = snapshot.snapshot.meta.node_types[typeIndex] + if (!Array.isArray(types) || nameIndex < 0 || sizeIndex < 0) throw new Error(`Unsupported snapshot: ${file}`) + let count = 0 + let bytes = 0 + for (let offset = 0; offset < snapshot.nodes.length; offset += fields.length) { + const type = snapshot.nodes[offset + typeIndex] + const name = snapshot.nodes[offset + nameIndex] + const size = snapshot.nodes[offset + sizeIndex] + if (type === undefined || name === undefined || size === undefined) throw new Error(`Malformed node in ${file}`) + if (types[type] === "string" && snapshot.strings[name]?.startsWith("heap-tool-output-")) { + count++ + bytes += size + } + } + return { count, bytes } +} + +const median = (values: number[]) => { + const sorted = [...values].sort((a, b) => a - b) + const middle = Math.floor(sorted.length / 2) + const upper = sorted[middle] + if (upper === undefined) throw new Error("Cannot take the median of an empty sample") + return sorted.length % 2 ? upper : ((sorted[middle - 1] ?? upper) + upper) / 2 +} +const mib = (bytes: number) => (bytes / 1024 / 1024).toFixed(2) + +async function main() { + const results = process.env.HEAP_BENCHMARK_DIR ?? path.resolve(__dirname, "../../heap-results") + const rows = [] + for (const file of await glob("runs/**/measurement.json", { cwd: results, absolute: true })) { + const measurement = JSON.parse(await fs.readFile(file, "utf8")) as Measurement + const loadedOutputs = await outputRetention(path.join(path.dirname(file), "loaded.heapsnapshot")) + const switchedOutputs = await outputRetention(path.join(path.dirname(file), "switched.heapsnapshot")) + rows.push({ ...measurement, loadedOutputs, switchedOutputs }) + } + if (!rows.length) throw new Error("No heap measurements found") + const lines = [ + "# VS Code webview heap comparison", + "", + "Forced-GC renderer heap, measured through CDP on the Zoo Code webview target. The extension host is held constant. Both builds omit source maps. The workload is a deterministic persisted transcript, not a recording of a human session.", + "", + "Each transcript contains 602 messages: 300 command/output pairs with roughly 17 KiB outputs. Repeated uses 32 distinct outputs; unique uses 300. Snapshots are taken after task-history hydration and after switching to a small task. Values below are medians across fresh VS Code sessions.", + "", + "| Workload | Variant | Runs | Empty MiB | Loaded MiB | Loaded − empty MiB | Switched MiB | Output string nodes loaded / switched |", + "| --- | --- | ---: | ---: | ---: | ---: | ---: | --- |", + ] + for (const workload of ["repeated", "unique"]) { + for (const variant of ["base", "candidate"]) { + const group = rows.filter((row) => row.workload === workload && row.variant === variant) + if (!group.length) continue + lines.push( + `| ${workload} | ${variant} | ${group.length} | ${mib(median(group.map((r) => r.stages.empty.usedSize)))} | ${mib(median(group.map((r) => r.stages.loaded.usedSize)))} | ${mib(median(group.map((r) => r.stages.loaded.usedSize - r.stages.empty.usedSize)))} | ${mib(median(group.map((r) => r.stages.switched.usedSize)))} | ${median(group.map((r) => r.loadedOutputs.count))} / ${median(group.map((r) => r.switchedOutputs.count))} |`, + ) + } + } + lines.push( + "", + "Raw per-run measurements and string-node counts are in summary.json. The matching .heapsnapshot files can be loaded in Chrome DevTools. These are renderer-isolate measurements, not whole-process RSS or extension-host heap. String-node counts are diagnostic evidence; inspect retaining paths before attributing surviving strings to a particular cache.", + "", + ) + await fs.writeFile(path.join(results, "summary.json"), JSON.stringify(rows, null, 2)) + await fs.writeFile(path.join(results, "summary.md"), lines.join("\n")) + console.log(lines.join("\n")) +} +void main().catch((error: unknown) => { + console.error(error) + process.exitCode = 1 +}) diff --git a/apps/vscode-e2e/src/heap/workload.ts b/apps/vscode-e2e/src/heap/workload.ts new file mode 100644 index 0000000000..feba2fde78 --- /dev/null +++ b/apps/vscode-e2e/src/heap/workload.ts @@ -0,0 +1,32 @@ +import type { ClineMessage } from "@roo-code/types" + +export type Workload = "repeated" | "unique" +export const LARGE_TASK_MARKER = "HEAP_LARGE_TASK_READY" +export const SMALL_TASK_MARKER = "HEAP_SMALL_TASK_READY" + +/** A completed transcript loaded by the real task-history and webview paths. */ +export function createTranscript(workload: Workload): ClineMessage[] { + const messages: ClineMessage[] = [{ ts: 1, type: "say", say: "text", text: "Heap benchmark task" }] + for (let index = 0; index < 300; index++) { + const outputId = workload === "repeated" ? index % 32 : index + const output = + `heap-tool-output-${outputId}\n` + + Array.from( + { length: 256 }, + (_, line) => `log ${outputId}: line ${line}: deterministic tool output for memory measurement\n`, + ).join("") + messages.push( + { + ts: 2 + index * 2, + type: "ask", + ask: "command", + text: `cat output-${index}.log`, + isAnswered: true, + partial: false, + }, + { ts: 3 + index * 2, type: "say", say: "command_output", text: output, partial: false }, + ) + } + messages.push({ ts: 602, type: "say", say: "completion_result", text: LARGE_TASK_MARKER, partial: false }) + return messages +} From 134b68410c3554af3f53eba6a8bcd92f5d991a02 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Tue, 27 Oct 2026 20:54:58 +0000 Subject: [PATCH 4/6] test(webview): strengthen message state and completion coverage --- .../chat/__tests__/ChatView.spec.tsx | 5 ++- .../__tests__/ExtensionStateContext.spec.tsx | 39 +++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx index 7fa4e81131..42edc94b9f 100644 --- a/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ChatView.spec.tsx @@ -737,7 +737,10 @@ describe("ChatView - Virtualization Configuration", () => { const row = await screen.findByTestId("chat-row") const instance = row.getAttribute("data-instance") expect(instance).not.toBeNull() - mockPostMessage({ clineMessages: [task, { ...partial, text: "Completed", partial: false }] }) + await dispatchExtensionMessage({ + type: "messageUpdated", + clineMessage: { ...partial, text: "Completed", partial: false }, + }) await waitFor(() => expect(screen.getByTestId("chat-row")).toHaveTextContent("Completed")) expect(screen.getByTestId("chat-row").getAttribute("data-instance")).toBe(instance) }) diff --git a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx index f6b4e2ce3a..5129f0e4ba 100644 --- a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx +++ b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx @@ -25,11 +25,20 @@ import { import { StringCache } from "@/utils/stringCache" const TestComponent = () => { - const { allowedCommands, setAllowedCommands, soundEnabled, showRooIgnoredFiles, setShowRooIgnoredFiles } = - useExtensionState() + const { + allowedCommands, + setAllowedCommands, + soundEnabled, + showRooIgnoredFiles, + setShowRooIgnoredFiles, + clineMessages, + currentTaskId, + } = useExtensionState() return (
+
{JSON.stringify(clineMessages)}
+
{currentTaskId ?? "no-task"}
{JSON.stringify(allowedCommands)}
{JSON.stringify(soundEnabled)}
{JSON.stringify(showRooIgnoredFiles)}
@@ -43,6 +52,10 @@ const TestComponent = () => { ) } +const expectMessages = (messages: ClineMessage[]) => { + expect(JSON.parse(screen.getByTestId("cline-messages").textContent!)).toEqual(messages) +} + const RulesTestComponent = () => { const { rules } = useExtensionState() @@ -168,8 +181,13 @@ describe("ExtensionStateContext", () => { (scenario) => { const internSpy = vi.spyOn(StringCache.prototype, "intern") const payload: ClineMessage = { ts: 1, type: "say", say: "text", text: "seed" } + const oldTaskOnly: ClineMessage = { ts: 2, type: "say", say: "text", text: "old-task-only" } render( - + , ) @@ -179,17 +197,22 @@ describe("ExtensionStateContext", () => { internSpy.mockClear() dispatch({ type: "state", state: { currentTaskId: "task-b", clineMessages: [{ ...payload }] } }) expect(internSpy).toHaveBeenCalledWith(payload) + expectMessages([payload]) + expect(screen.getByTestId("current-task-id")).toHaveTextContent("task-b") } else { const updated = { ...payload, text: "updated" } dispatch({ type: "messageUpdated", clineMessage: updated }) + expectMessages([updated]) internSpy.mockClear() dispatch({ type: "state", state: { currentTaskId: "task-a", clineMessages: [{ ...updated }] } }) expect(internSpy).not.toHaveBeenCalledWith(updated) + expectMessages([updated]) } }, ) it("ignores task identity in pushes without messages", () => { + // Settings, telemetry and mode refreshes do not switch tasks; their snapshots can arrive late. const initial = { ...createInitialExtensionState(), currentTaskId: "task-a" } expect(mergeExtensionState(initial, { currentTaskId: "old" }).currentTaskId).toBe("task-a") const internSpy = vi.spyOn(StringCache.prototype, "intern") @@ -208,6 +231,8 @@ describe("ExtensionStateContext", () => { ), ) expect(cache.size).toBe(size) + expectMessages([payload]) + expect(screen.getByTestId("current-task-id")).toHaveTextContent("task-a") internSpy.mockClear() act(() => window.dispatchEvent( @@ -217,6 +242,8 @@ describe("ExtensionStateContext", () => { ), ) expect(internSpy).not.toHaveBeenCalledWith(payload) + expectMessages([payload]) + expect(screen.getByTestId("current-task-id")).toHaveTextContent("task-a") }) it("does not retain partial messages or re-intern unchanged snapshot messages", () => { @@ -242,6 +269,7 @@ describe("ExtensionStateContext", () => { it("clears a serialized absent task id and preserves task identity on stale snapshots", () => { const initial = { ...createInitialExtensionState(), currentTaskId: "task-a", clineMessagesSeq: 5 } + // JSON serialization intentionally drops undefined, reproducing the extension transport. const cleared = mergeExtensionState( initial, JSON.parse(JSON.stringify({ currentTaskId: undefined, clineMessages: [], clineMessagesSeq: 6 })), @@ -328,6 +356,7 @@ describe("ExtensionStateContext", () => { act(() => { window.dispatchEvent( new MessageEvent("message", { + // Keep the JSON round trip: structuredClone would preserve the absent-task key. data: JSON.parse( JSON.stringify({ type: "state", state: { currentTaskId: undefined, clineMessages: [] } }), ), @@ -363,6 +392,7 @@ describe("ExtensionStateContext", () => { ) }) expect(internSpy).not.toHaveBeenCalledWith(staleMessage) + expectMessages([]) act(() => { window.dispatchEvent( @@ -372,6 +402,7 @@ describe("ExtensionStateContext", () => { ) }) expect(internSpy).toHaveBeenCalledWith(currentMessage) + expectMessages([currentMessage]) internSpy.mockClear() act(() => { window.dispatchEvent( @@ -381,6 +412,7 @@ describe("ExtensionStateContext", () => { ) }) expect(internSpy).not.toHaveBeenCalledWith(staleMessage) + expectMessages([currentMessage]) }) it("does not intern an update for a message outside the current task", () => { @@ -402,6 +434,7 @@ describe("ExtensionStateContext", () => { }) expect(internSpy).not.toHaveBeenCalledWith(unknownMessage) + expectMessages([currentMessage]) }) it("initializes with empty allowedCommands array", () => { From 2ff4aa6c9b53ffef24304d5ae6168d46a6aaf299 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Wed, 28 Oct 2026 03:55:07 +0000 Subject: [PATCH 5/6] Address webview memory optimization review feedback --- apps/vscode-e2e/AGENTS.md | 4 + apps/vscode-e2e/src/heap/README.md | 4 +- apps/vscode-e2e/src/heap/RESULTS.md | 15 ++- apps/vscode-e2e/src/heap/electron.heap.ts | 3 +- apps/vscode-e2e/src/heap/prepare.ts | 15 +-- .../src/context/ExtensionStateContext.tsx | 59 ++++---- .../__tests__/ExtensionStateContext.spec.tsx | 126 +++++++++++++++++- .../src/utils/__tests__/stringCache.spec.ts | 10 ++ 8 files changed, 187 insertions(+), 49 deletions(-) diff --git a/apps/vscode-e2e/AGENTS.md b/apps/vscode-e2e/AGENTS.md index 5195e6f3d3..421194f766 100644 --- a/apps/vscode-e2e/AGENTS.md +++ b/apps/vscode-e2e/AGENTS.md @@ -278,3 +278,7 @@ Always restore the default OpenRouter config in `suiteTeardown` so subsequent su ## Programmatic fixtures (regex matching) For requests that can't be matched by a stable substring (e.g. "starts with `` but not preceded by a user message"), add a programmatic fixture in `src/runTest.ts` using `mock.addFixture()` with a `RegExp` match. These are only available in replay mode and are not recorded. + +## Webview heap diagnostic + +The opt-in suite under `src/heap/` measures retained renderer heap through real VS Code task hydration and switching. See [src/heap/README.md](src/heap/README.md) for build, run, and summarize commands. Keep base and candidate host builds identical, use isolated profiles, and preserve cleanup on failure. Heap snapshots and generated measurements stay in the git-ignored `heap-results/` directory. This diagnostic has no CI memory threshold and does not replace focused webview tests. diff --git a/apps/vscode-e2e/src/heap/README.md b/apps/vscode-e2e/src/heap/README.md index 16647ca9c0..27ca98cc46 100644 --- a/apps/vscode-e2e/src/heap/README.md +++ b/apps/vscode-e2e/src/heap/README.md @@ -13,8 +13,8 @@ pnpm --dir apps/vscode-e2e heap:summarize ``` `heap:build` creates isolated base and candidate webview builds in `heap-results/builds`. -The extension host is identical for both. Base replaces the three production webview -modules changed by PR #1466 with their versions at `git merge-base HEAD origin/main`. +The extension host is identical for both. Base replaces `webview-ui/src/context/ExtensionStateContext.tsx` +with its version at `git merge-base HEAD origin/main`. Override that reference with `HEAP_BASE_REF`. Source maps are disabled for both builds to reduce build memory; production minification is preserved. Source files in your checkout are untouched. diff --git a/apps/vscode-e2e/src/heap/RESULTS.md b/apps/vscode-e2e/src/heap/RESULTS.md index 714d215522..cb3fb31f69 100644 --- a/apps/vscode-e2e/src/heap/RESULTS.md +++ b/apps/vscode-e2e/src/heap/RESULTS.md @@ -15,6 +15,9 @@ requests are blocked except for the local mock model used to create the seed tas A full-state refresh after hydration exercises reuse across successive deserialized state snapshots. +These hashes identify the historical measured builds, not the current branch head. +The measurements below have not been rerun for subsequent review fixes. + Median forced-GC used heap, in MiB: | Workload | Variant | Empty | Large task loaded | Loaded − empty | After switching to small task | @@ -24,10 +27,17 @@ Median forced-GC used heap, in MiB: | Unique | Base | 26.78 | 41.58 | 14.80 | 36.27 | | Unique | Candidate | 26.57 | 36.63 | 10.07 | 36.26 | +`Loaded − empty` is the median of each session’s loaded-minus-empty difference; +it need not equal the difference of the two column medians. + Loaded heap is approximately **9.67 MiB lower (23%)** for repeated outputs and **4.95 MiB lower (12%)** for unique outputs. Baseline-adjusted task growth falls by about 9.52 MiB and 4.73 MiB respectively. The unique workload still benefits from -sharing unchanged message strings across full-state pushes. These results do not +sharing unchanged message strings across full-state pushes via `Object.assign`, +even after FIFO eviction. The cache can retain only about 120 strings of 16 KiB +within its 2M-character budget. Repeated outputs fit within that budget and benefit +from cache interning as well. These mechanisms explain the observed difference, +but this comparison does not isolate their individual contributions. These results do not measure CPU overhead, image-rich messages, or continuously streamed task execution. Loaded-heap ranges across the three sessions: @@ -60,7 +70,8 @@ No forced remounts or additional product behavior changes were introduced here. ## Evidence and reproduction -See [README.md](./README.md) for commands and methodology. The ignored +See [README.md](./README.md) for commands and methodology. The raw artifacts are local, git-ignored evidence and are not distributed with this +report; reproducing the run is required to independently inspect them. The ignored `apps/vscode-e2e/heap-results/` directory contains the 36 successful snapshots, per-run measurements, build provenance, generated summary, cache-map inventory, and representative base/candidate retaining paths under `analysis/`. diff --git a/apps/vscode-e2e/src/heap/electron.heap.ts b/apps/vscode-e2e/src/heap/electron.heap.ts index 40b2004343..19e658766e 100644 --- a/apps/vscode-e2e/src/heap/electron.heap.ts +++ b/apps/vscode-e2e/src/heap/electron.heap.ts @@ -44,7 +44,8 @@ async function capture(client: CDPSession, file: string) { } for (const workload of ["repeated", "unique"] satisfies Workload[]) { - test(`webview retained heap: ${workload} tool outputs`, async ({ browserName: _browserName }, testInfo) => { + test(`webview retained heap: ${workload} tool outputs`, async () => { + const testInfo = test.info() const temporaryRoot = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-heap-")) const workspace = path.join(temporaryRoot, "workspace") const userData = path.join(temporaryRoot, "user-data") diff --git a/apps/vscode-e2e/src/heap/prepare.ts b/apps/vscode-e2e/src/heap/prepare.ts index 152ca17580..3c99bdf25a 100644 --- a/apps/vscode-e2e/src/heap/prepare.ts +++ b/apps/vscode-e2e/src/heap/prepare.ts @@ -41,14 +41,13 @@ async function main() { await fs.mkdir(path.join(extension, "webview-ui"), { recursive: true }) await link(path.join(repositoryRoot, "src/webview-ui/audio"), path.join(extension, "webview-ui/audio")) if (variant === "base") { - // Hold the host, styles and unrelated code constant; compare the three - // production webview modules changed by this PR with their base versions. - for (const file of ["App.tsx", "components/chat/ChatView.tsx", "context/ExtensionStateContext.tsx"]) { - const contents = execFileSync("git", ["show", `${base}:webview-ui/src/${file}`], { - cwd: repositoryRoot, - }) - await fs.writeFile(path.join(root, "webview-ui/src", file), contents) - } + // Hold the host, styles and unrelated code constant; compare only the + // state context module with its base version. + const file = "context/ExtensionStateContext.tsx" + const contents = execFileSync("git", ["show", `${base}:webview-ui/src/${file}`], { + cwd: repositoryRoot, + }) + await fs.writeFile(path.join(root, "webview-ui/src", file), contents) } // Source maps do not affect runtime behavior and are omitted in both builds // to keep this diagnostic usable on memory-constrained runners. diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 7c352a2193..61e23ebfcd 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -159,6 +159,30 @@ export interface ExtensionStateContextType extends ExtensionState { export const ExtensionStateContext = createContext(undefined) +const messagesAreStale = (incoming: Partial, currentSequence: number | undefined): boolean => + incoming.clineMessages !== undefined && + incoming.clineMessagesSeq !== undefined && + currentSequence !== undefined && + incoming.clineMessagesSeq <= currentSequence + +function internMessages(messages: ClineMessage[], previousMessages: ClineMessage[], cache: StringCache): void { + const previousByTimestamp = new Map(previousMessages.map((item) => [item.ts, item])) + for (const message of messages) { + const previous = previousByTimestamp.get(message.ts) + if ( + previous && + Object.keys(message).length === Object.keys(previous).length && + Object.entries(message).every(([key, value]) => previous[key as keyof ClineMessage] === value) + ) { + // Keep the incoming object, but rebind equal strings to the previous snapshot's + // instances, including strings that have already left the bounded FIFO cache. + Object.assign(message, previous) + } else { + cache.intern(message) + } + } +} + export const mergeExtensionState = (prevState: ExtensionState, newState: Partial) => { const { customModePrompts: prevCustomModePrompts, experiments: prevExperiments, ...prevRest } = prevState @@ -179,12 +203,7 @@ export const mergeExtensionState = (prevState: ExtensionState, newState: Partial // concurrent state pushes. If a stale push arrives after a newer one, its clineMessages // would overwrite the newer messages. The sequence number prevents this by only applying // clineMessages when the incoming seq is strictly greater than the last applied seq. - if ( - newState.clineMessagesSeq !== undefined && - prevState.clineMessagesSeq !== undefined && - newState.clineMessagesSeq <= prevState.clineMessagesSeq && - newState.clineMessages !== undefined - ) { + if (messagesAreStale(newState, prevState.clineMessagesSeq)) { rest.clineMessages = prevState.clineMessages rest.clineMessagesSeq = prevState.clineMessagesSeq rest.currentTaskId = prevState.currentTaskId @@ -369,13 +388,9 @@ export const ExtensionStateContextProvider: React.FC<{ case "state": { const newState = message.state ?? {} - const messagesAreStale = - newState.clineMessages !== undefined && - newState.clineMessagesSeq !== undefined && - clineMessagesSeqRef.current !== undefined && - newState.clineMessagesSeq <= clineMessagesSeqRef.current + const staleMessages = messagesAreStale(newState, clineMessagesSeqRef.current) - if (!messagesAreStale && newState.clineMessages !== undefined) { + if (!staleMessages && newState.clineMessages !== undefined) { if (newState.currentTaskId !== currentTaskIdRef.current) { messageCache.clear() messagesRef.current = [] @@ -383,22 +398,8 @@ export const ExtensionStateContextProvider: React.FC<{ } } - if (newState.clineMessages && !messagesAreStale) { - const previousMessages = new Map(messagesRef.current.map((item) => [item.ts, item])) - for (const clineMessage of newState.clineMessages) { - const previous = previousMessages.get(clineMessage.ts) - if ( - previous && - Object.keys(clineMessage).length === Object.keys(previous).length && - Object.entries(clineMessage).every( - ([key, value]) => previous[key as keyof ClineMessage] === value, - ) - ) { - Object.assign(clineMessage, previous) - } else { - messageCache.intern(clineMessage) - } - } + if (newState.clineMessages && !staleMessages) { + internMessages(newState.clineMessages, messagesRef.current, messageCache) messagesRef.current = newState.clineMessages } if ( @@ -479,7 +480,7 @@ export const ExtensionStateContextProvider: React.FC<{ } case "messageUpdated": { const clineMessage = message.clineMessage! - const messageIndex = messagesRef.current.findIndex((item) => item.ts === clineMessage.ts) + const messageIndex = findLastIndex(messagesRef.current, (item) => item.ts === clineMessage.ts) if (messageIndex !== -1) { messageCache.intern(clineMessage) messagesRef.current = messagesRef.current.map((item, index) => diff --git a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx index 5129f0e4ba..22ba7c37fb 100644 --- a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx +++ b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx @@ -33,10 +33,14 @@ const TestComponent = () => { setShowRooIgnoredFiles, clineMessages, currentTaskId, + taskHistory, + currentTaskItem, } = useExtensionState() return (
+
{JSON.stringify(taskHistory)}
+
{JSON.stringify(currentTaskItem)}
{JSON.stringify(clineMessages)}
{currentTaskId ?? "no-task"}
{JSON.stringify(allowedCommands)}
@@ -56,6 +60,16 @@ const expectMessages = (messages: ClineMessage[]) => { expect(JSON.parse(screen.getByTestId("cline-messages").textContent!)).toEqual(messages) } +type InternSpy = { mock: { contexts: readonly unknown[] } } + +const cacheFor = (spy: InternSpy, kind: "message" | "history"): StringCache => { + const cache = spy.mock.contexts.find( + (value) => value instanceof StringCache && (value["shouldIntern"] !== undefined) === (kind === "message"), + ) + if (!(cache instanceof StringCache)) throw new Error(`Missing ${kind} cache`) + return cache +} + const RulesTestComponent = () => { const { rules } = useExtensionState() @@ -128,6 +142,84 @@ const InitialStateTestComponent = () => { describe("ExtensionStateContext", () => { afterEach(() => vi.restoreAllMocks()) + it("interns initial history and current task items and subsequent state pushes", () => { + const internSpy = vi.spyOn(StringCache.prototype, "intern") + const item = { id: "task-a", number: 1, ts: 1, task: "history", tokensIn: 0, tokensOut: 0, totalCost: 0 } + const currentTaskItem = { ...item, id: "task-b", task: "current" } + const taskHistory = [item] + render( + + + , + ) + const cache = cacheFor(internSpy, "history") + expect(internSpy).toHaveBeenCalledWith(taskHistory) + expect(internSpy).toHaveBeenCalledWith(currentTaskItem) + expect(cache.size).toBe(4) + expect(JSON.parse(screen.getByTestId("task-history").textContent!)).toEqual(taskHistory) + expect(JSON.parse(screen.getByTestId("current-task-item").textContent!)).toEqual(currentTaskItem) + + internSpy.mockClear() + const updated = { ...currentTaskItem, task: "updated" } + act(() => + window.dispatchEvent( + new MessageEvent("message", { + data: { type: "state", state: { currentTaskItem: updated } }, + }), + ), + ) + expect(internSpy).toHaveBeenCalledWith(updated) + expect(internSpy.mock.contexts).toEqual([cache]) + expect(cache.size).toBe(5) + expect(JSON.parse(screen.getByTestId("current-task-item").textContent!)).toEqual(updated) + }) + + it("does not restore a key dropped by a snapshot message", () => { + const payload: ClineMessage = { ts: 1, type: "say", say: "text", text: "seed", partial: false } + const internSpy = vi.spyOn(StringCache.prototype, "intern") + render( + + + , + ) + internSpy.mockClear() + const updated: ClineMessage = { ts: 1, type: "say", say: "text", text: "seed" } + act(() => + window.dispatchEvent( + new MessageEvent("message", { data: { type: "state", state: { clineMessages: [updated] } } }), + ), + ) + expect(JSON.parse(screen.getByTestId("cline-messages").textContent!)[0]).not.toHaveProperty("partial") + expectMessages([{ ts: 1, type: "say", say: "text", text: "seed" }]) + expect(internSpy).toHaveBeenCalledWith(updated) + }) + + it("updates the last duplicate timestamp in both the message mirror and rendered state", () => { + const first: ClineMessage = { ts: 1, type: "say", say: "text", text: "first" } + const last = { ...first, text: "last" } + const updated = { ...last, text: "updated" } + const internSpy = vi.spyOn(StringCache.prototype, "intern") + render( + + + , + ) + act(() => + window.dispatchEvent( + new MessageEvent("message", { data: { type: "messageUpdated", clineMessage: updated } }), + ), + ) + expectMessages([first, updated]) + internSpy.mockClear() + act(() => + window.dispatchEvent( + new MessageEvent("message", { data: { type: "state", state: { clineMessages: [{ ...updated }] } } }), + ), + ) + expect(internSpy).not.toHaveBeenCalledWith(updated) + expectMessages([updated]) + }) + it.each(["initial", "state", "messageUpdated", "taskHistoryUpdated", "taskHistoryItemUpdated"] as const)( "interns accepted payloads through %s", (entry) => { @@ -141,7 +233,7 @@ describe("ExtensionStateContext", () => { ) if (entry === "initial") { expect(internSpy).toHaveBeenCalledWith(payload) - const cache = internSpy.mock.contexts[0] + const cache = cacheFor(internSpy, "message") expect(cache).toBeInstanceOf(StringCache) if (!(cache instanceof StringCache)) throw new Error("Missing cache") expect(typeof cache["shouldIntern"]).toBe("function") @@ -164,6 +256,9 @@ describe("ExtensionStateContext", () => { act(() => window.dispatchEvent(new MessageEvent("message", { data: messages[entry] }))) expect(internSpy.mock.contexts).toEqual([targetCache]) expect(targetCache.size).toBeGreaterThan(sizeBefore) + if (entry === "taskHistoryUpdated" || entry === "taskHistoryItemUpdated") { + expect(JSON.parse(screen.getByTestId("task-history").textContent!)).toEqual([item]) + } expect(internSpy).toHaveBeenCalledWith( entry === "state" ? expect.objectContaining({ text: "changed" }) @@ -222,7 +317,7 @@ describe("ExtensionStateContext", () => { , ) - const cache = internSpy.mock.contexts[0] + const cache = cacheFor(internSpy, "message") if (!(cache instanceof StringCache)) throw new Error("Missing cache") const size = cache.size act(() => @@ -254,7 +349,7 @@ describe("ExtensionStateContext", () => { , ) - const cache = internSpy.mock.contexts[0] + const cache = cacheFor(internSpy, "message") if (!(cache instanceof StringCache)) throw new Error("Missing message cache") expect(cache?.size).toBe(0) internSpy.mockClear() @@ -295,8 +390,8 @@ describe("ExtensionStateContext", () => { , ) - const messageCache = internSpy.mock.contexts[0] - const historyCache = internSpy.mock.contexts[1] + const messageCache = cacheFor(internSpy, "message") + const historyCache = cacheFor(internSpy, "history") if (!(messageCache instanceof StringCache) || !(historyCache instanceof StringCache)) throw new Error("Missing caches") const messageSize = messageCache.size @@ -309,6 +404,23 @@ describe("ExtensionStateContext", () => { ), ) expect(messageCache.size).toBe(messageSize) + const clearSpy = vi.spyOn(StringCache.prototype, "clear") + act(() => + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: "state", + state: { + currentTaskId: "task-a", + clineMessages: [{ ts: 1, type: "say", say: "text", text: "seed" }], + clineMessagesSeq: 6, + }, + }, + }), + ), + ) + expect(clearSpy).not.toHaveBeenCalled() + expect(messageCache.size).toBe(messageSize) act(() => window.dispatchEvent( new MessageEvent("message", { data: { type: "taskHistoryUpdated", taskHistory: [] } }), @@ -335,8 +447,8 @@ describe("ExtensionStateContext", () => { , ) - const messageCache = internSpy.mock.contexts[0] - const historyCache = internSpy.mock.contexts[1] + const messageCache = cacheFor(internSpy, "message") + const historyCache = cacheFor(internSpy, "history") if (!(messageCache instanceof StringCache) || !(historyCache instanceof StringCache)) { throw new Error("Missing provider caches") } diff --git a/webview-ui/src/utils/__tests__/stringCache.spec.ts b/webview-ui/src/utils/__tests__/stringCache.spec.ts index 7ef523d91d..c8c6da0a7c 100644 --- a/webview-ui/src/utils/__tests__/stringCache.spec.ts +++ b/webview-ui/src/utils/__tests__/stringCache.spec.ts @@ -14,6 +14,14 @@ describe("StringCache", () => { }, ) + it("does not refresh FIFO order on a cache hit", () => { + const cache = new StringCache(undefined, 2) + cache.intern(["first", "second"]) + cache.intern("first") + cache.intern("third") + expect([...cache["cache"].keys()]).toEqual(["second", "third"]) + }) + it("bounds entry count and retained characters, skipping oversized strings", () => { const cache = new StringCache(undefined, 2, 6) cache.intern(["aaa", "bbb", "ccc"]) @@ -35,6 +43,8 @@ describe("StringCache", () => { nested: [{ label: "shared" }, { label: "other" }], empty: "", count: 1, + nullable: null, + optional: undefined, } expect(cache.intern(value)).toBe(value) From 31e260a9feac5a21c6216c03474d8f20c3a9521c Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Wed, 28 Oct 2026 23:13:22 +0000 Subject: [PATCH 6/6] fix(webview): keep task item with pinned task id --- .../src/context/ExtensionStateContext.tsx | 14 ++++++--- .../__tests__/ExtensionStateContext.spec.tsx | 31 +++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 61e23ebfcd..41f94b5ff5 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -203,15 +203,21 @@ export const mergeExtensionState = (prevState: ExtensionState, newState: Partial // concurrent state pushes. If a stale push arrives after a newer one, its clineMessages // would overwrite the newer messages. The sequence number prevents this by only applying // clineMessages when the incoming seq is strictly greater than the last applied seq. + const keepTaskIdentity = + messagesAreStale(newState, prevState.clineMessagesSeq) || newState.clineMessages === undefined if (messagesAreStale(newState, prevState.clineMessagesSeq)) { rest.clineMessages = prevState.clineMessages rest.clineMessagesSeq = prevState.clineMessagesSeq + } + if (keepTaskIdentity) { + // Pushes without fresh messages cannot establish task identity. Keep the task item with + // the pinned ID when it describes another task; same-task item updates still apply. + if (newState.currentTaskId !== prevState.currentTaskId) { + rest.currentTaskItem = prevState.currentTaskItem + } rest.currentTaskId = prevState.currentTaskId - } else if (newState.clineMessages !== undefined) { - rest.currentTaskId = newState.currentTaskId } else { - // Pushes without messages cannot establish task identity. - rest.currentTaskId = prevState.currentTaskId + rest.currentTaskId = newState.currentTaskId } // Note that we completely replace the previous apiConfiguration and customSupportPrompts objects diff --git a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx index 22ba7c37fb..424255245a 100644 --- a/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx +++ b/webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx @@ -306,6 +306,37 @@ describe("ExtensionStateContext", () => { }, ) + it("keeps the task item with the pinned task id unless it updates the same task", () => { + const item = (id: string, task: string) => ({ + id, + number: 1, + ts: 1, + task, + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + }) + const initial = { + ...createInitialExtensionState(), + currentTaskId: "task-a", + currentTaskItem: item("task-a", "a"), + clineMessagesSeq: 5, + } + const other = { currentTaskId: "task-b", currentTaskItem: item("task-b", "b") } + const sameTask = { currentTaskId: "task-a", currentTaskItem: item("task-a", "a2") } + const stale = { clineMessages: [], clineMessagesSeq: 4 } + + for (const extra of [{}, stale]) { + expect(mergeExtensionState(initial, { ...other, ...extra }).currentTaskItem).toBe(initial.currentTaskItem) + expect(mergeExtensionState(initial, { ...sameTask, ...extra }).currentTaskItem).toBe( + sameTask.currentTaskItem, + ) + } + expect(mergeExtensionState(initial, { ...other, clineMessages: [], clineMessagesSeq: 6 }).currentTaskItem).toBe( + other.currentTaskItem, + ) + }) + it("ignores task identity in pushes without messages", () => { // Settings, telemetry and mode refreshes do not switch tasks; their snapshots can arrive late. const initial = { ...createInitialExtensionState(), currentTaskId: "task-a" }