Skip to content

Commit 1bd1f72

Browse files
HonaHona
authored
fix(session-ui): cancel abandoned completed Markdown parse jobs (#46764)
1 parent 311e32d commit 1bd1f72

19 files changed

Lines changed: 755 additions & 28 deletions

File tree

‎bun.lock‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/app/e2e/performance/markdown/README.md‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Timeline Preload Lifetime
2+
3+
This manual benchmark uses the production app, restored session tabs, the real
4+
`MessageTimeline` preload, and the real Markdown worker. Only API data and result
5+
delivery timing are fixture-owned. It does not connect to a running OpenCode
6+
service or send prompts.
7+
8+
From `packages/app`, set absolute `MARKDOWN_APP_BUILD_DIR` and
9+
`MARKDOWN_RESULTS_DIR` artifact paths, then run:
10+
11+
```sh
12+
bun run build
13+
# Copy dist into MARKDOWN_APP_BUILD_DIR before editing production source.
14+
bun --bun x playwright test --config e2e/performance/markdown/playwright.config.ts --repeat-each 20
15+
```
16+
17+
Each isolated sample restores two sessions with one user message and one completed
18+
assistant text part each. The cold target has a realistic recovery review with
19+
either two TypeScript fences (typical) or 36 fences (large). The source has a short
20+
completed answer. Target data is prefetched before selection, but its Markdown is
21+
not parsed until the target is selected.
22+
23+
The app's service-worker generator reads `dist`, so use the normal build output
24+
and freeze a copy, rather than overriding Vite's build output directory.
25+
26+
The real worker result is held after admission. The test selects the original
27+
session again and releases the held result only after the abandoned timeline row
28+
detaches and the selected answer reports production Markdown readiness. This
29+
exercises both the timeline preload and the nested Markdown consumer, including
30+
the case where either one would otherwise keep a shared parse alive.
31+
32+
Destination readiness and post-disposal result settlement are separate metrics.
33+
The latter is a MessageChannel task after the result's promise microtasks drain.
34+
The DOMParser probe counts actual DOMPurify input containing the abandoned answer
35+
after disposal, in characters. CDP reports renderer task/script time and JS heap.
36+
These are not worker CPU, Electron process RAM, or ungated tab-switch measurements.
37+
In particular, this gate releases the result after destination readiness and must
38+
not be used to claim a destination-readiness gain from skipping sanitization.
39+
40+
`MARKDOWN_ASSERT_DISPOSAL=1` enables the no-obsolete-sanitization assertion. Use
41+
`MARKDOWN_RETAINED=1` only in separate post-GC runs. The repository trace collector
42+
is available through `OPENCODE_PERFORMANCE_TRACE_DIR`, and `MARKDOWN_SCREENSHOT`
43+
captures final output after timing. Run serially, preserve frozen builds, and keep
44+
all results outside Git.

‎packages/app/e2e/performance/markdown/lifetime.bench.ts‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
import type { SessionMessageInfo } from "@opencode-ai/client/promise"
2+
import { benchmark, expect } from "../benchmark"
3+
import { mockOpenCodeServer } from "../../utils/mock-server"
4+
import { fixture } from "../timeline/session-timeline-stress.fixture"
5+
import { installStressSessionTabs, installTimelineSettings, stressSessionHref } from "../timeline/timeline-test-helpers"
6+
import { completedAnswer } from "../../../../session-ui/performance/markdown-lifetime/answer"
7+
import { installMarkdownGate } from "./probe"
8+
9+
for (const size of ["typical", "large"]) {
10+
benchmark(`timeline preload disposal: ${size}`, async ({ page, report }) => {
11+
const answer = completedAnswer(size === "typical" ? 2 : 36)
12+
const errors: string[] = []
13+
page.on("pageerror", (error) => errors.push(error.message))
14+
const messages: Record<string, SessionMessageInfo[]> = Object.fromEntries(
15+
[fixture.sourceID, fixture.targetID].map((id) => [
16+
id,
17+
[
18+
{
19+
id: `msg_1_${id}_user`,
20+
type: "user",
21+
time: { created: 1700000000000 },
22+
text: "Review the recovery boundary.",
23+
},
24+
{
25+
id: `msg_2_${id}_assistant`,
26+
type: "assistant",
27+
time: { created: 1700000001000, completed: 1700000008000 },
28+
model: { id: "claude-opus-4-6", providerID: "opencode" },
29+
agent: "build",
30+
cost: 0.01,
31+
tokens: { input: 100, output: 200, reasoning: 0, cache: { read: 0, write: 0 } },
32+
finish: "stop",
33+
content: [
34+
{
35+
type: "text",
36+
text:
37+
id === fixture.targetID
38+
? answer
39+
: "## Current destination\n\nThe selected session is ready.\n\n```typescript\nconst current = { ready: true }\n```",
40+
},
41+
],
42+
},
43+
] satisfies SessionMessageInfo[],
44+
]),
45+
)
46+
await mockOpenCodeServer(page, {
47+
sessions: fixture.sessions.filter((session) => session.id !== fixture.childID),
48+
provider: fixture.provider,
49+
directory: fixture.directory,
50+
project: fixture.project,
51+
pageMessages: (id) => ({ items: messages[id] ?? [] }),
52+
})
53+
await installTimelineSettings(page)
54+
await installStressSessionTabs(page)
55+
const targetPart = `msg_2_${fixture.targetID}_assistant:text:0`
56+
const sourcePart = `msg_2_${fixture.sourceID}_assistant:text:0`
57+
await installMarkdownGate(page, { answer, targetPart, sourcePart, href: stressSessionHref(fixture.sourceID) })
58+
const prefetched = page.waitForResponse((response) =>
59+
new URL(response.url()).pathname.endsWith(`/session/${fixture.targetID}/message`),
60+
)
61+
await page.goto(stressSessionHref(fixture.sourceID))
62+
await prefetched
63+
const source = page.locator(`[data-timeline-part-id="${sourcePart}"] [data-component="markdown"]`)
64+
await expect(source).toHaveAttribute("data-markdown-ready", "")
65+
await page.locator(`[data-slot="titlebar-tabs"] a[href="${stressSessionHref(fixture.targetID)}"]`).click()
66+
await page.waitForFunction(() => Reflect.get(window, "markdownGate").held)
67+
await expect(page.locator(`[data-timeline-part-id="${targetPart}"]`)).toBeAttached()
68+
const cdp = await page.context().newCDPSession(page)
69+
await cdp.send("Performance.enable")
70+
const before = await cdp.send("Performance.getMetrics")
71+
await page.evaluate(() => Reflect.get(window, "markdownGate").arm())
72+
await page.locator(`[data-slot="titlebar-tabs"] a[href="${stressSessionHref(fixture.sourceID)}"]`).click()
73+
await expect(source).toHaveAttribute("data-markdown-ready", "")
74+
await expect(source.getByRole("heading", { name: "Current destination" })).toBeVisible()
75+
await expect(page.locator(`[data-timeline-part-id="${targetPart}"]`)).toHaveCount(0)
76+
await page.waitForFunction(() => Reflect.get(window, "markdownGate").settled > 0)
77+
const after = await cdp.send("Performance.getMetrics")
78+
const stats = await page.evaluate(() => {
79+
const value = Reflect.get(window, "markdownGate")
80+
return {
81+
admitted: value.admitted,
82+
responses: value.responses,
83+
started: value.started,
84+
ready: value.ready,
85+
released: value.released,
86+
settled: value.settled,
87+
sanitizeCalls: value.sanitizeCalls,
88+
sanitizeChars: value.sanitizeChars,
89+
}
90+
})
91+
expect(stats.admitted).toBe(1)
92+
expect(stats.responses).toBe(1)
93+
expect(stats.ready).toBeGreaterThan(stats.started)
94+
expect(stats.settled).toBeGreaterThan(stats.released)
95+
expect(errors).toEqual([])
96+
if (process.env.MARKDOWN_ASSERT_DISPOSAL === "1") expect(stats.sanitizeCalls).toBe(0)
97+
const value = (data: typeof after, name: string) => data.metrics.find((item) => item.name === name)!.value
98+
const retained = process.env.MARKDOWN_RETAINED === "1"
99+
if (retained) await cdp.send("HeapProfiler.collectGarbage")
100+
report(
101+
{
102+
...stats,
103+
destinationReadyMs: stats.ready - stats.started,
104+
releasedSettledMs: stats.settled - stats.released,
105+
taskMs: (value(after, "TaskDuration") - value(before, "TaskDuration")) * 1000,
106+
scriptMs: (value(after, "ScriptDuration") - value(before, "ScriptDuration")) * 1000,
107+
usedHeapBytes: (await cdp.send("Runtime.getHeapUsage")).usedSize,
108+
},
109+
{
110+
size,
111+
retained,
112+
answerBytes: Buffer.byteLength(answer),
113+
messagesPerSession: 2,
114+
partsPerAnswer: 1,
115+
fences: size === "typical" ? 2 : 36,
116+
browser: page.context().browser()!.version(),
117+
transport: "playwright-route",
118+
build: process.env.MARKDOWN_APP_BUILD_DIR,
119+
},
120+
)
121+
if (process.env.MARKDOWN_SCREENSHOT)
122+
await page.screenshot({ path: `${process.env.MARKDOWN_SCREENSHOT}/timeline-${size}.png` })
123+
await cdp.detach()
124+
})
125+
}

‎packages/app/e2e/performance/markdown/playwright.config.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { defineConfig } from "@playwright/test"
2+
import { fileURLToPath } from "node:url"
3+
4+
process.env.PLAYWRIGHT_PORT = "6199"
5+
process.env.PLAYWRIGHT_SERVER_PORT = "6199"
6+
process.env.PLAYWRIGHT_SERVER_HOST = "127.0.0.1"
7+
8+
export default defineConfig({
9+
testDir: ".",
10+
testMatch: "*.bench.ts",
11+
outputDir: process.env.MARKDOWN_RESULTS_DIR,
12+
workers: 1,
13+
retries: 0,
14+
timeout: 60_000,
15+
expect: { timeout: 15_000 },
16+
reporter: [["line"]],
17+
use: { baseURL: "http://127.0.0.1:6199", viewport: { width: 1280, height: 900 }, serviceWorkers: "block" },
18+
webServer: {
19+
cwd: fileURLToPath(new URL("../../..", import.meta.url)),
20+
command: `bun run serve -- --host 127.0.0.1 --port 6199 --strictPort --outDir "${process.env.MARKDOWN_APP_BUILD_DIR}"`,
21+
url: "http://127.0.0.1:6199",
22+
reuseExistingServer: false,
23+
},
24+
})

‎packages/app/e2e/performance/markdown/probe.ts‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import type { Page } from "@playwright/test"
2+
import type {
3+
MarkdownWorkerRequest,
4+
MarkdownWorkerResponse,
5+
} from "../../../../session-ui/src/components/markdown-worker-protocol"
6+
7+
export async function installMarkdownGate(
8+
page: Page,
9+
input: { answer: string; sourcePart: string; targetPart: string; href: string },
10+
) {
11+
await page.addInitScript(({ answer, sourcePart, targetPart, href }) => {
12+
const stats = {
13+
admitted: 0,
14+
responses: 0,
15+
held: false,
16+
started: 0,
17+
ready: 0,
18+
released: 0,
19+
settled: 0,
20+
sanitizeCalls: 0,
21+
sanitizeChars: 0,
22+
arm: () => {
23+
armed = true
24+
},
25+
}
26+
let armed = false
27+
let id: number | undefined
28+
let release: (() => void) | undefined
29+
const descriptor = Object.getOwnPropertyDescriptor(Worker.prototype, "onmessage")!
30+
const post = Worker.prototype.postMessage
31+
Object.defineProperty(Worker.prototype, "onmessage", {
32+
configurable: true,
33+
get: descriptor.get,
34+
set(callback: (event: MessageEvent<MarkdownWorkerResponse>) => void) {
35+
descriptor.set!.call(this, (event: MessageEvent<MarkdownWorkerResponse>) => {
36+
if (event.data.type === "parse" && event.data.id === id) {
37+
stats.responses++
38+
stats.held = true
39+
release = () => callback.call(this, event)
40+
return
41+
}
42+
callback.call(this, event)
43+
})
44+
},
45+
})
46+
Worker.prototype.postMessage = function (request: MarkdownWorkerRequest) {
47+
if (request.type === "parse" && request.text === answer) {
48+
id = request.id
49+
stats.admitted++
50+
}
51+
post.call(this, request)
52+
}
53+
const parse = DOMParser.prototype.parseFromString
54+
DOMParser.prototype.parseFromString = function (text, type) {
55+
if (stats.released && String(text).includes("Recovery implementation review")) {
56+
stats.sanitizeCalls++
57+
stats.sanitizeChars += String(text).length
58+
}
59+
return parse.call(this, text, type)
60+
}
61+
document.addEventListener(
62+
"mousedown",
63+
(event) => {
64+
if (!armed || stats.started) return
65+
const target = event.target instanceof Element ? event.target.closest("a") : undefined
66+
if (target?.getAttribute("href") !== href) return
67+
stats.started = performance.now()
68+
},
69+
true,
70+
)
71+
// The app can retain the outgoing view until the destination is ready. Release
72+
// only after its actual row detaches, rather than assuming click means dispose.
73+
new MutationObserver(() => {
74+
if (!stats.started || stats.released) return
75+
const current = document.querySelector(`[data-timeline-part-id="${sourcePart}"] [data-markdown-ready]`)
76+
if (!current) return
77+
stats.ready ||= performance.now()
78+
if (document.querySelector(`[data-timeline-part-id="${targetPart}"]`)) return
79+
stats.released = performance.now()
80+
performance.mark("markdown-timeline-disposed")
81+
release!()
82+
release = undefined
83+
const channel = new MessageChannel()
84+
channel.port1.onmessage = () => {
85+
stats.settled = performance.now()
86+
channel.port1.close()
87+
channel.port2.close()
88+
}
89+
channel.port2.postMessage(null)
90+
}).observe(document, { childList: true, subtree: true, attributes: true })
91+
Object.defineProperty(window, "markdownGate", { value: stats })
92+
}, input)
93+
}

‎packages/app/src/session/timeline/message-timeline.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createEffect, createMemo, createSignal, For, on, Show, type Accessor, type JSX } from "solid-js"
1+
import { createEffect, createMemo, createSignal, For, on, onCleanup, Show, type Accessor, type JSX } from "solid-js"
22
import { createStore } from "solid-js/store"
33
import { createAnimatedPresence } from "@/runtime/animated-presence"
44
import type { SessionUserActions } from "@opencode-ai/session-ui/actions"
@@ -342,8 +342,11 @@ export function MessageTimeline(props: MessageTimelineProps) {
342342
if (message?.type === "assistant" && message.time.completed !== undefined) {
343343
const content = Timeline.resolveContent(message, tail.group.ref.partID)
344344
// Start the required worker job while the rest of the selected view is constructed.
345-
if (content?.type === "text" && content.text.trim())
346-
void preloadMarkdown(content.text, tail.group.ref.partID).catch(() => undefined)
345+
if (content?.type === "text" && content.text.trim()) {
346+
const preload = new AbortController()
347+
onCleanup(() => preload.abort())
348+
void preloadMarkdown(content.text, tail.group.ref.partID, preload.signal).catch(() => undefined)
349+
}
347350
}
348351
}
349352
return (

‎packages/session-ui/component-tests/markdown.fixture.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ export {
1313
touchCachedMarkdown,
1414
} from "../src/components/markdown-cache"
1515
export { renderMermaidSvg } from "../src/components/markdown-mermaid"
16+
export { MarkdownWorkerDisposedError } from "../src/components/markdown-worker"
17+
export { preloadMarkdown }
1618

1719
export async function mountMarkdown(options: {
1820
text: string

0 commit comments

Comments
 (0)