From 726f028a8f04c84b18e54c2c3490e64a9e97dc27 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Thu, 14 May 2026 22:07:01 +0800 Subject: [PATCH 1/5] fix(ui): defer default-open tool details --- .../src/components/basic-tool-defer.test.ts | 18 ++++++++ packages/ui/src/components/basic-tool.tsx | 42 +++++++++---------- .../components/message-part-registry.test.ts | 17 +++++++- .../components/message-part/tools/bash.tsx | 1 + 4 files changed, 55 insertions(+), 23 deletions(-) create mode 100644 packages/ui/src/components/basic-tool-defer.test.ts diff --git a/packages/ui/src/components/basic-tool-defer.test.ts b/packages/ui/src/components/basic-tool-defer.test.ts new file mode 100644 index 000000000..bcc32a13e --- /dev/null +++ b/packages/ui/src/components/basic-tool-defer.test.ts @@ -0,0 +1,18 @@ +import { expect, test } from "bun:test" +import { basicToolInitialReady } from "./basic-tool" + +test("deferred default-open tools do not mount details immediately", () => { + expect(basicToolInitialReady({ defaultOpen: true, defer: true })).toBe(false) +}) + +test("non-deferred default-open tools keep the previous immediate details behavior", () => { + expect(basicToolInitialReady({ defaultOpen: true })).toBe(true) + expect(basicToolInitialReady({ defaultOpen: true, defer: false })).toBe(true) +}) + +test("closed tools start without mounted details", () => { + expect(basicToolInitialReady({ defaultOpen: false, defer: true })).toBe(false) + expect(basicToolInitialReady({ defaultOpen: false })).toBe(false) + expect(basicToolInitialReady({ defer: true })).toBe(false) + expect(basicToolInitialReady({})).toBe(false) +}) diff --git a/packages/ui/src/components/basic-tool.tsx b/packages/ui/src/components/basic-tool.tsx index 817163965..33c062508 100644 --- a/packages/ui/src/components/basic-tool.tsx +++ b/packages/ui/src/components/basic-tool.tsx @@ -41,10 +41,15 @@ export interface BasicToolProps { const SPRING = { type: "spring" as const, visualDuration: 0.35, bounce: 0 } +export function basicToolInitialReady(props: { defaultOpen?: boolean; defer?: boolean }) { + if (props.defer) return false + return props.defaultOpen ?? false +} + export function BasicTool(props: BasicToolProps) { const [state, setState] = createStore({ open: props.defaultOpen ?? false, - ready: props.defaultOpen ?? false, + ready: basicToolInitialReady(props), }) const open = () => state.open const ready = () => state.ready @@ -64,27 +69,22 @@ export function BasicTool(props: BasicToolProps) { if (props.forceOpen) setState("open", true) }) - createEffect( - on( - open, - (value) => { - if (!props.defer) return - if (!value) { - cancel() - setState("ready", false) - return - } + createEffect(() => { + if (!props.defer) return + if (!open()) { + cancel() + setState("ready", false) + return + } + if (ready()) return - cancel() - frame = requestAnimationFrame(() => { - frame = undefined - if (!open()) return - setState("ready", true) - }) - }, - { defer: true }, - ), - ) + cancel() + frame = requestAnimationFrame(() => { + frame = undefined + if (!open()) return + setState("ready", true) + }) + }) // Animated height for collapsible open/close let contentRef: HTMLDivElement | undefined diff --git a/packages/ui/src/components/message-part-registry.test.ts b/packages/ui/src/components/message-part-registry.test.ts index 11e0ed87a..c4b3dc5e7 100644 --- a/packages/ui/src/components/message-part-registry.test.ts +++ b/packages/ui/src/components/message-part-registry.test.ts @@ -46,6 +46,10 @@ function readMessagePartSources() { ].join("\n") } +function readToolSource(file: string) { + return readFileSync(join(MESSAGE_PART_DIR, "tools", file), "utf8") +} + test("message-part internals do not import through the facade", () => { const offenders = sourceFiles(MESSAGE_PART_DIR) .map((file) => ({ @@ -109,12 +113,21 @@ test("part and tool side-effect barrels cover every registered renderer", () => test("split keeps hidden tools and deferred heavy tool bodies explicit", () => { const source = readMessagePartSources() - const deferCount = [...source.matchAll(/\bdefer\b/g)].length expect(source).toContain('export const HIDDEN_TOOLS = new Set(["todowrite"])') expect(source).toContain('if (tool === "edit" || tool === "write" || tool === "apply_patch") return edit') expect(source).toContain("defaultOpen={completed()}") - expect(deferCount).toBe(4) + + const deferredHeavyTools = { + "bash.tsx": 1, + "edit.tsx": 1, + "write.tsx": 1, + "apply-patch.tsx": 2, + } as const + + for (const [file, count] of Object.entries(deferredHeavyTools)) { + expect([...readToolSource(file).matchAll(/\bdefer\b/g)].length).toBe(count) + } }) test("review hardening keeps routing, clipboard, url, and write guards explicit", () => { diff --git a/packages/ui/src/components/message-part/tools/bash.tsx b/packages/ui/src/components/message-part/tools/bash.tsx index c70a73bdf..de86ee344 100644 --- a/packages/ui/src/components/message-part/tools/bash.tsx +++ b/packages/ui/src/components/message-part/tools/bash.tsx @@ -70,6 +70,7 @@ ToolRegistry.register({
From 1a6183171ab6a5da8d09263b0f1d5a8f6f5e21d8 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Thu, 14 May 2026 22:47:47 +0800 Subject: [PATCH 2/5] test(ui): cover deferred tool detail mounting --- .../src/components/basic-tool-defer.test.ts | 120 +++++++++++++++++- packages/ui/src/components/basic-tool.tsx | 21 ++- .../fixtures/basic-tool-render.fixture.tsx | 38 ++++++ 3 files changed, 171 insertions(+), 8 deletions(-) create mode 100644 packages/ui/test/fixtures/basic-tool-render.fixture.tsx diff --git a/packages/ui/src/components/basic-tool-defer.test.ts b/packages/ui/src/components/basic-tool-defer.test.ts index bcc32a13e..5a08c438f 100644 --- a/packages/ui/src/components/basic-tool-defer.test.ts +++ b/packages/ui/src/components/basic-tool-defer.test.ts @@ -1,10 +1,128 @@ -import { expect, test } from "bun:test" +import { afterAll, beforeAll, beforeEach, expect, test } from "bun:test" +import { GlobalRegistrator } from "@happy-dom/global-registrator" +import { createRequire } from "module" +import { createServer, type ViteDevServer } from "vite" +import solidPlugin from "vite-plugin-solid" import { basicToolInitialReady } from "./basic-tool" +const require = createRequire(import.meta.url) +const solidWeb = require.resolve("solid-js/web/dist/web.js") +const solidCore = require.resolve("solid-js/dist/solid.js") +const solidStore = require.resolve("solid-js/store/dist/store.js") + +let server: ViteDevServer | undefined +let rafCallbacks: Array = [] +let registeredDom = false + +const flushAnimationFrames = () => { + const callbacks = rafCallbacks + rafCallbacks = [] + callbacks.forEach((callback) => callback?.(performance.now())) +} + +beforeAll(async () => { + if (typeof document === "undefined" || typeof window === "undefined") { + GlobalRegistrator.register() + registeredDom = true + } + window.requestAnimationFrame = globalThis.requestAnimationFrame = ((callback: FrameRequestCallback) => { + rafCallbacks.push(callback) + return rafCallbacks.length + }) as typeof requestAnimationFrame + window.cancelAnimationFrame = globalThis.cancelAnimationFrame = ((id: number) => { + rafCallbacks[id - 1] = undefined + }) as typeof cancelAnimationFrame + + server = await createServer({ + root: new URL("../..", import.meta.url).pathname, + configFile: false, + plugins: [solidPlugin({ solid: { generate: "dom" } })], + resolve: { + alias: { + "solid-js/web": solidWeb, + "solid-js/store": solidStore, + "solid-js": solidCore, + }, + }, + server: { middlewareMode: true }, + appType: "custom", + logLevel: "silent", + ssr: { noExternal: ["@kobalte/core", "solid-js"] }, + }) +}) + +beforeEach(() => { + document.body.textContent = "" + rafCallbacks = [] +}) + +afterAll(async () => { + await server?.close() + if (registeredDom) GlobalRegistrator.unregister() +}) + +async function loadFixture(): Promise { + if (!server) throw new Error("Vite server not initialized") + return (await server.ssrLoadModule( + "/test/fixtures/basic-tool-render.fixture.tsx", + )) as typeof import("../../test/fixtures/basic-tool-render.fixture") +} + test("deferred default-open tools do not mount details immediately", () => { expect(basicToolInitialReady({ defaultOpen: true, defer: true })).toBe(false) }) +test("deferred default-open tools do not resolve details until the next frame", async () => { + const { mountBasicTool } = await loadFixture() + const tool = mountBasicTool({ defaultOpen: true, defer: true }) + + await Promise.resolve() + + expect(tool.detailsRenderCount()).toBe(0) + expect(tool.details()).toBeNull() + + flushAnimationFrames() + await Promise.resolve() + + expect(tool.detailsRenderCount()).toBeGreaterThan(0) + expect(tool.details()?.textContent).toBe("details") + + tool.dispose() +}) + +test("non-deferred default-open tools keep rendering details synchronously", async () => { + const { mountBasicTool } = await loadFixture() + const tool = mountBasicTool({ defaultOpen: true, defer: false }) + + expect(tool.detailsRenderCount()).toBeGreaterThan(0) + expect(tool.details()?.textContent).toBe("details") + + tool.dispose() +}) + +test("deferred tools reset details when closed", async () => { + const { mountBasicTool } = await loadFixture() + const tool = mountBasicTool({ defaultOpen: false, defer: true }) + + expect(tool.detailsRenderCount()).toBe(0) + expect(tool.details()).toBeNull() + + tool.trigger()?.click() + await Promise.resolve() + flushAnimationFrames() + await Promise.resolve() + + expect(tool.detailsRenderCount()).toBeGreaterThan(0) + expect(tool.details()?.textContent).toBe("details") + + tool.trigger()?.click() + await Promise.resolve() + + expect(tool.details()).toBeNull() + + tool.dispose() +}) + test("non-deferred default-open tools keep the previous immediate details behavior", () => { expect(basicToolInitialReady({ defaultOpen: true })).toBe(true) expect(basicToolInitialReady({ defaultOpen: true, defer: false })).toBe(true) diff --git a/packages/ui/src/components/basic-tool.tsx b/packages/ui/src/components/basic-tool.tsx index 33c062508..cc8bef7e1 100644 --- a/packages/ui/src/components/basic-tool.tsx +++ b/packages/ui/src/components/basic-tool.tsx @@ -1,4 +1,4 @@ -import { createEffect, For, on, onCleanup, Show, type JSX } from "solid-js" +import { createEffect, For, on, onCleanup, Show, untrack, type JSX } from "solid-js" import { animate, type AnimationPlaybackControls } from "motion" import { useI18n } from "../context/i18n" import { createStore } from "solid-js/store" @@ -54,6 +54,13 @@ export function BasicTool(props: BasicToolProps) { const open = () => state.open const ready = () => state.ready const pending = () => props.status === "pending" || props.status === "running" + const hasDetails = () => { + if (props.hideDetails || props.locked || pending()) return false + if (props.defer) return true + return !!props.children + } + const shouldRenderDetails = () => !props.hideDetails && (props.defer || !!props.children) + const detailsReady = () => !props.defer || ready() let frame: number | undefined @@ -76,7 +83,7 @@ export function BasicTool(props: BasicToolProps) { setState("ready", false) return } - if (ready()) return + if (untrack(ready)) return cancel() frame = requestAnimationFrame(() => { @@ -189,7 +196,7 @@ export function BasicTool(props: BasicToolProps) {
{triggerInfo()}
- +
@@ -219,7 +226,7 @@ export function BasicTool(props: BasicToolProps) { )} - +
- {props.children} + {props.children}
- + - {props.children} + {props.children} diff --git a/packages/ui/test/fixtures/basic-tool-render.fixture.tsx b/packages/ui/test/fixtures/basic-tool-render.fixture.tsx new file mode 100644 index 000000000..e669be657 --- /dev/null +++ b/packages/ui/test/fixtures/basic-tool-render.fixture.tsx @@ -0,0 +1,38 @@ +import { render } from "solid-js/web" +import { BasicTool } from "../../src/components/basic-tool" + +function Details(props: { onRender: () => void }) { + props.onRender() + return details +} + +export function mountBasicTool(props: { defaultOpen?: boolean; defer?: boolean }) { + let detailsRenderCount = 0 + const host = document.createElement("div") + document.body.append(host) + + const disposeRoot = render( + () => ( + +
detailsRenderCount++} /> + + ), + host, + ) + + return { + host, + detailsRenderCount: () => detailsRenderCount, + details: () => host.querySelector("[data-testid='basic-tool-details']"), + trigger: () => host.querySelector("[data-slot='collapsible-trigger']") as HTMLElement | null, + dispose: () => { + disposeRoot() + host.remove() + }, + } +} From 248b77b5bd47a6c5a66623f5320373e0f95b82da Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Thu, 14 May 2026 22:49:31 +0800 Subject: [PATCH 3/5] fix(ui): keep animated deferred details through close --- packages/ui/src/components/basic-tool.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/ui/src/components/basic-tool.tsx b/packages/ui/src/components/basic-tool.tsx index cc8bef7e1..6d4a69703 100644 --- a/packages/ui/src/components/basic-tool.tsx +++ b/packages/ui/src/components/basic-tool.tsx @@ -61,6 +61,7 @@ export function BasicTool(props: BasicToolProps) { } const shouldRenderDetails = () => !props.hideDetails && (props.defer || !!props.children) const detailsReady = () => !props.defer || ready() + const closingAnimatedDetails = () => props.animated && !open() let frame: number | undefined @@ -80,7 +81,7 @@ export function BasicTool(props: BasicToolProps) { if (!props.defer) return if (!open()) { cancel() - setState("ready", false) + if (!props.animated) setState("ready", false) return } if (untrack(ready)) return @@ -115,6 +116,10 @@ export function BasicTool(props: BasicToolProps) { } else { contentRef.style.overflow = "hidden" heightAnim = animate(contentRef, { height: "0px" }, SPRING) + void heightAnim.finished.then(() => { + if (!contentRef || open()) return + if (props.defer) setState("ready", false) + }) } }, { defer: true }, @@ -234,7 +239,10 @@ export function BasicTool(props: BasicToolProps) { style={{ height: initialOpen ? "auto" : "0px", overflow: initialOpen ? "visible" : "hidden", + "pointer-events": closingAnimatedDetails() ? "none" : undefined, }} + aria-hidden={closingAnimatedDetails() ? "true" : undefined} + inert={closingAnimatedDetails() ? true : undefined} > {props.children} From a4e6ccede77dbce2856fb09ffb62a73dbabb9830 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Thu, 14 May 2026 23:26:02 +0800 Subject: [PATCH 4/5] test(app): add heavy bash perf probe --- packages/app/e2e/perf/perf-probe.spec.ts | 121 ++++++++++++++++++++++- packages/app/e2e/perf/profiles.test.ts | 9 ++ packages/app/e2e/perf/profiles.ts | 2 + 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 packages/app/e2e/perf/profiles.test.ts diff --git a/packages/app/e2e/perf/perf-probe.spec.ts b/packages/app/e2e/perf/perf-probe.spec.ts index 027a41d92..076f28ca0 100644 --- a/packages/app/e2e/perf/perf-probe.spec.ts +++ b/packages/app/e2e/perf/perf-probe.spec.ts @@ -2,7 +2,7 @@ import fs from "node:fs/promises" import path from "node:path" import { raw } from "../../../opencode/test/lib/llm-server" import { test, expect } from "../fixtures" -import { waitTerminalFocusIdle, withSession } from "../actions" +import { cleanupSession, waitSessionIdle, waitSessionSaved, waitTerminalFocusIdle, withSession } from "../actions" import { promptSelector, sessionMessageItemSelector, @@ -11,6 +11,7 @@ import { terminalSelector, } from "../selectors" import { sessionPath, terminalToggleKey } from "../utils" +import type { createSdk } from "../utils" import { installPerfProbe, resetPerfProbe, snapshotPerfProbe, summarizeScenarioRuns } from "./probe" import { applyPerfProfile, readPerfProfile, shouldRunScenario, type PerfScenarioName } from "./profiles" import { seedTimelineRecomputeSession } from "./timeline-fixture" @@ -37,8 +38,22 @@ const longMarkdown = [ ...Array.from({ length: 80 }, (_, index) => `Paragraph ${index + 1}: ${"streaming markdown content ".repeat(8)}`), ].join("\n") +const heavyBashCommand = + 'node -e \'for (let i = 0; i < 900; i++) console.log(String(i).padStart(4, "0") + " " + "heavy bash output ".repeat(8))\'' + const scenarioResults: ReturnType[] = [] +type PerfSdk = ReturnType +type PerfProject = { + directory: string + url: string + sdk: PerfSdk + trackSession: (sessionID: string) => void +} +type PerfLlm = { + tool: (name: string, input: unknown) => Promise +} + const chatChunk = (delta: Record, input?: { finish?: string; usage?: { input: number; output: number } }) => ({ id: "chatcmpl-test", object: "chat.completion.chunk", @@ -149,6 +164,82 @@ async function scrollTimelineTo(page: Parameters[0], t expect(found).toBe(true) } +async function enableShellToolPartsExpanded(page: Parameters[0]) { + const apply = () => { + const raw = localStorage.getItem("settings.v3") + const current = (() => { + if (!raw) return {} + try { + return JSON.parse(raw) as Record + } catch { + return {} + } + })() + const general = current.general && typeof current.general === "object" ? current.general : {} + localStorage.setItem( + "settings.v3", + JSON.stringify({ + ...current, + general: { + ...general, + shellToolPartsExpanded: true, + }, + }), + ) + } + + await page.addInitScript(apply) + await page.evaluate(apply) +} + +async function seedHeavyBashSession(input: { + project: PerfProject + llm: PerfLlm + run: number +}) { + const session = await input.project.sdk.session + .create({ + title: `perf heavy bash ${Date.now()}-${input.run}`, + permission: [{ permission: "bash", pattern: "*", action: "allow" }], + }) + .then((result) => result.data) + if (!session?.id) throw new Error("Session create did not return an id") + input.project.trackSession(session.id) + + await input.llm.tool("bash", { + command: heavyBashCommand, + description: "Prints heavy deterministic output", + }) + await input.project.sdk.session.promptAsync({ + sessionID: session.id, + agent: "build", + parts: [{ type: "text", text: "Run the heavy bash perf fixture." }], + }) + await waitSessionIdle(input.project.sdk, session.id, 90_000) + await waitSessionSaved(input.project.directory, session.id, 90_000, input.project.url) + + await expect + .poll( + async () => { + const messages = await input.project.sdk.session.messages({ sessionID: session.id, limit: 20 }) + return (messages.data ?? []).some((message) => + message.parts.some( + (part) => + part.type === "tool" && + part.tool === "bash" && + part.state.status === "completed" && + typeof part.state.output === "string" && + part.state.output.includes("heavy bash output"), + ), + ) + }, + { timeout: 30_000 }, + ) + .toBe(true) + + return session +} + function skipUnlessScenario(name: PerfScenarioName) { test.skip(!shouldRunScenario(PERF_PROFILE, name), `${PERF_PROFILE} profile does not run ${name}`) } @@ -262,6 +353,34 @@ test.describe("PR0.1 perf probe baseline", () => { scenarioResults.push(summarizeScenarioRuns({ branch: perfBranch, profile: PERF_PROFILE, scenario: "tool-call-expand", runs })) }) + test("tool-default-open-heavy-bash emits a 3-run JSON baseline", async ({ page, project, llm }) => { + skipUnlessScenario("tool-default-open-heavy-bash") + await installPerfProbe(page) + await applyPerfProfile(page, PERF_PROFILE) + await project.open() + await enableShellToolPartsExpanded(page) + + const runs = [] + for (let run = 0; run < 3; run += 1) { + const session = await seedHeavyBashSession({ project, llm, run }) + try { + await page.goto(sessionPath(project.directory, session.id)) + const trigger = page.locator('[data-slot="collapsible-trigger"]').filter({ has: page.locator('[data-component="tool-trigger"]') }).first() + await expect(trigger).toBeVisible({ timeout: 30_000 }) + await expect(trigger).toHaveAttribute("aria-expanded", "true") + await settleFrames(page, 2) + runs.push(await snapshotPerfProbe(page)) + } finally { + await cleanupSession({ sdk: project.sdk, sessionID: session.id }).catch(() => undefined) + } + if (run < 2) await cooldownAfterRun(page) + } + + scenarioResults.push( + summarizeScenarioRuns({ branch: perfBranch, profile: PERF_PROFILE, scenario: "tool-default-open-heavy-bash", runs }), + ) + }) + test("terminal-side-panel-open emits a 3-run JSON baseline", async ({ page, project }) => { skipUnlessScenario("terminal-side-panel-open") await installPerfProbe(page) diff --git a/packages/app/e2e/perf/profiles.test.ts b/packages/app/e2e/perf/profiles.test.ts new file mode 100644 index 000000000..06fc6cf30 --- /dev/null +++ b/packages/app/e2e/perf/profiles.test.ts @@ -0,0 +1,9 @@ +import { expect, test } from "bun:test" +import { shouldRunScenario, type PerfScenarioName } from "./profiles" + +test("default profile runs heavy default-open bash perf coverage", () => { + const scenario = "tool-default-open-heavy-bash" as PerfScenarioName + + expect(shouldRunScenario("default", scenario)).toBe(true) + expect(shouldRunScenario("low-end", scenario)).toBe(false) +}) diff --git a/packages/app/e2e/perf/profiles.ts b/packages/app/e2e/perf/profiles.ts index cf06f1313..4c751e633 100644 --- a/packages/app/e2e/perf/profiles.ts +++ b/packages/app/e2e/perf/profiles.ts @@ -5,6 +5,7 @@ export type PerfScenarioName = | "homepage-cold" | "session-streaming-long" | "tool-call-expand" + | "tool-default-open-heavy-bash" | "terminal-side-panel-open" | "session-scroll-reading" | "session-timeline-recompute" @@ -13,6 +14,7 @@ const defaultScenarios = new Set([ "homepage-cold", "session-streaming-long", "tool-call-expand", + "tool-default-open-heavy-bash", "terminal-side-panel-open", "session-scroll-reading", ]) From 2dcc23ea9ffe3c902520a96746379b1bec87cf83 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Thu, 14 May 2026 23:36:53 +0800 Subject: [PATCH 5/5] test(app): keep perf profile unit out of playwright --- packages/app/e2e/perf/{profiles.test.ts => profiles.unit.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/app/e2e/perf/{profiles.test.ts => profiles.unit.ts} (100%) diff --git a/packages/app/e2e/perf/profiles.test.ts b/packages/app/e2e/perf/profiles.unit.ts similarity index 100% rename from packages/app/e2e/perf/profiles.test.ts rename to packages/app/e2e/perf/profiles.unit.ts