Skip to content

Commit a12d50e

Browse files
authored
fix(app): hydrate timeline message parents (anomalyco#35269)
1 parent 1b9b260 commit a12d50e

12 files changed

Lines changed: 984 additions & 42 deletions
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
import type { Page } from "@playwright/test"
2+
import { expectSessionTitle } from "../../utils/waits"
3+
import { mockOpenCodeServer } from "../../utils/mock-server"
4+
import { benchmark, expect, withBenchmarkPage } from "../benchmark"
5+
import { fixture } from "./session-timeline-stress.fixture"
6+
import { installStressSessionTabs, stressSessionHref } from "./timeline-test-helpers"
7+
import { measureSessionSwitch, waitForStableTimeline } from "./session-tab-switch-probe"
8+
9+
type ParentHydrationBenchmarkMode = "natural" | "candidate"
10+
11+
const mode = process.env.SESSION_PARENT_HYDRATION_BENCHMARK_MODE ?? "natural"
12+
if (mode !== "natural" && mode !== "candidate") throw new Error(`Unknown parent hydration benchmark mode: ${mode}`)
13+
const userID = "msg_parent_hydration_user"
14+
const user = {
15+
...fixture.messages[fixture.targetID][0]!,
16+
info: { ...fixture.messages[fixture.targetID][0]!.info, id: userID, time: { created: 1700001000000 } },
17+
parts: fixture.messages[fixture.targetID][0]!.parts.map((part, index) => ({
18+
...part,
19+
id: `prt_parent_hydration_user_${index}`,
20+
messageID: userID,
21+
})),
22+
}
23+
const assistantSeed = fixture.messages[fixture.targetID][3]!
24+
const assistants = Array.from({ length: 14 }, (_, index) => {
25+
const messageID = `msg_parent_hydration_${String(index).padStart(2, "0")}`
26+
return {
27+
...assistantSeed,
28+
info: {
29+
...assistantSeed.info,
30+
id: messageID,
31+
parentID: userID,
32+
time: { created: 1700001001000 + index * 1_000, completed: 1700001001500 + index * 1_000 },
33+
},
34+
parts: assistantSeed.parts.map((part, partIndex) => ({
35+
...part,
36+
id: `prt_parent_hydration_${String(index).padStart(2, "0")}_${partIndex}`,
37+
messageID,
38+
})),
39+
}
40+
})
41+
const messages = [user, ...assistants]
42+
const target = fixture.sessions.find((session) => session.id === fixture.targetID)!
43+
const lastID = userID
44+
const lastPartID = assistants.at(-1)!.parts.at(-1)!.id
45+
46+
benchmark("hydrates an orphaned latest turn after a cold session click", async ({ browser, report }, testInfo) => {
47+
benchmark.setTimeout(180_000)
48+
const results = [] as Awaited<ReturnType<typeof trial>>[]
49+
for (let run = 0; run < 5; run++) {
50+
results.push(
51+
await withBenchmarkPage(browser, `session-parent-hydration-${mode}-${run}`, (page) => trial(page, mode), testInfo),
52+
)
53+
}
54+
const timing = results.map((result) => result.metrics.firstCorrectObservedMs!).sort((a, b) => a - b)
55+
report(
56+
{
57+
results: results.map((result) => ({ ...result.metrics, historyGateCount: result.historyGateCount })),
58+
summary: {
59+
firstCorrectObservedMs: { min: timing[0], median: timing[2], max: timing.at(-1) },
60+
blankSamples: results.map((result) => result.metrics.blankSamples),
61+
requestCounts: {
62+
list: results.map((result) => result.requestCounts.list),
63+
parent: results.map((result) => result.requestCounts.parent),
64+
},
65+
historyGateCount: results.map((result) => result.historyGateCount),
66+
},
67+
},
68+
{ mode },
69+
)
70+
})
71+
72+
async function trial(page: Page, mode: ParentHydrationBenchmarkMode) {
73+
const requests: { type: "list" | "parent"; before?: string }[] = []
74+
const history = mode === "candidate" ? Promise.withResolvers<void>() : undefined
75+
let historyGates = 0
76+
await mockOpenCodeServer(page, {
77+
sessions: fixture.sessions.filter((session) => session.id === fixture.sourceID),
78+
provider: fixture.provider,
79+
directory: fixture.directory,
80+
project: fixture.project,
81+
messageDelay: 50,
82+
onMessages: (request) => {
83+
if (request.sessionID === fixture.targetID && request.phase === "start")
84+
requests.push({ type: "list", before: request.before })
85+
},
86+
beforeMessagesResponse: (request) => {
87+
if (mode !== "candidate" || request.sessionID !== fixture.targetID || !request.before) return Promise.resolve()
88+
historyGates++
89+
return history!.promise
90+
},
91+
onMessage: (request) => {
92+
if (request.sessionID === fixture.targetID && request.messageID === userID) requests.push({ type: "parent" })
93+
},
94+
message: (sessionID, messageID) => {
95+
if (sessionID !== fixture.targetID || messageID !== userID) return
96+
return user
97+
},
98+
pageMessages: (sessionID, limit, before) => {
99+
const items = sessionID === fixture.targetID ? messages : fixture.messages[fixture.sourceID]
100+
const end = before ? items.findIndex((message) => message.info.id === before) : items.length
101+
const start = Math.max(0, end - limit)
102+
return { items: items.slice(start, end), cursor: start > 0 ? items[start]!.info.id : undefined }
103+
},
104+
})
105+
await page.route(`**/session/${fixture.targetID}`, (route) =>
106+
route.fulfill({ status: 200, contentType: "application/json", body: JSON.stringify(target) }),
107+
)
108+
await installStressSessionTabs(page, { sessionIDs: [fixture.sourceID] })
109+
await page.goto(stressSessionHref(fixture.sourceID))
110+
await expectSessionTitle(page, fixture.expected.sourceTitle)
111+
await waitForStableTimeline(page, fixture.expected.sourceMessageIDs.at(-1)!)
112+
113+
const href = stressSessionHref(fixture.targetID)
114+
await page.evaluate(
115+
({ href, title }) => {
116+
const link = document.createElement("a")
117+
link.id = "parent-hydration-target"
118+
link.href = href
119+
link.textContent = title
120+
document.body.append(link)
121+
},
122+
{ href, title: target.title },
123+
)
124+
const metrics = await measureSessionSwitch(page, {
125+
destinationIDs: messages.map((message) => message.info.id),
126+
sourceIDs: fixture.messages[fixture.sourceID].map((message) => message.info.id),
127+
lastID,
128+
requiredPartID: lastPartID,
129+
requireBottomAnchor: false,
130+
href,
131+
switch: async () => {
132+
await page.locator("#parent-hydration-target").click()
133+
await expectSessionTitle(page, target.title)
134+
},
135+
}).finally(() => history?.resolve())
136+
expect(metrics.firstCorrectObservedMs).not.toBeNull()
137+
const requestCounts = {
138+
list: requests.filter((request) => request.type === "list").length,
139+
parent: requests.filter((request) => request.type === "parent").length,
140+
}
141+
if (mode === "candidate") {
142+
expect(requestCounts.parent).toBe(1)
143+
expect(historyGates).toBe(1)
144+
}
145+
return { metrics, requestCounts, historyGateCount: historyGates }
146+
}

packages/app/e2e/performance/timeline/session-tab-switch-metrics.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ export type SessionSwitchSample = {
44
source: string[]
55
hasVisibleRows: boolean
66
last: boolean
7+
requiredPartVisible?: boolean
8+
bottomAnchorRequired?: boolean
79
bottomErrorPx?: number
810
review?: {
911
fileHost: boolean
@@ -41,7 +43,8 @@ export function isCorrectDestination(sample: SessionSwitchSample) {
4143
sample.destination.length > 0 &&
4244
sample.source.length === 0 &&
4345
sample.last &&
44-
Math.abs(sample.bottomErrorPx ?? Infinity) <= 1
46+
sample.requiredPartVisible !== false &&
47+
(sample.bottomAnchorRequired === false || Math.abs(sample.bottomErrorPx ?? Infinity) <= 1)
4548
)
4649
}
4750

packages/app/e2e/performance/timeline/session-tab-switch-probe.ts

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,16 @@ type SessionSwitchProbe = {
88

99
async function installSessionSwitchProbe(
1010
page: Page,
11-
input: { destinationIDs: string[]; sourceIDs: string[]; lastID: string; href: string },
11+
input: {
12+
destinationIDs: string[]
13+
sourceIDs: string[]
14+
lastID: string
15+
requiredPartID?: string
16+
requireBottomAnchor?: boolean
17+
href: string
18+
},
1219
) {
13-
await page.evaluate(({ destinationIDs, sourceIDs, lastID, href }) => {
20+
await page.evaluate(({ destinationIDs, sourceIDs, lastID, requiredPartID, requireBottomAnchor, href }) => {
1421
const destination = new Set(destinationIDs)
1522
const source = new Set(sourceIDs)
1623
const samples: SessionSwitchSample[] = []
@@ -66,18 +73,36 @@ async function installSessionSwitchProbe(
6673
const rect = element.getBoundingClientRect()
6774
return rect.bottom > view.top && rect.top < view.bottom
6875
})
76+
const requiredPartVisible = requiredPartID
77+
? [...root.querySelectorAll<HTMLElement>("[data-timeline-part-id]")].some((element) => {
78+
if (element.dataset.timelinePartId !== requiredPartID) return false
79+
const rect = element.getBoundingClientRect()
80+
return rect.width > 0 && rect.height > 0 && rect.bottom > view.top && rect.top < view.bottom
81+
})
82+
: undefined
6983
const spacer = root.querySelector<HTMLElement>('[data-timeline-row="bottom-spacer"]')?.getBoundingClientRect()
7084
samples.push({
7185
observedAtMs,
7286
destination: visible.filter((id) => destination.has(id)),
7387
source: visible.filter((id) => source.has(id)),
7488
hasVisibleRows,
7589
last: visible.includes(lastID),
90+
requiredPartVisible,
91+
bottomAnchorRequired: requireBottomAnchor !== false,
7692
bottomErrorPx: spacer ? spacer.bottom - view.bottom : undefined,
7793
review,
7894
})
7995
} else {
80-
samples.push({ observedAtMs, destination: [], source: [], hasVisibleRows: false, last: false, review })
96+
samples.push({
97+
observedAtMs,
98+
destination: [],
99+
source: [],
100+
hasVisibleRows: false,
101+
last: false,
102+
requiredPartVisible: requiredPartID ? false : undefined,
103+
bottomAnchorRequired: requireBottomAnchor !== false,
104+
review,
105+
})
81106
}
82107
requestAnimationFrame(sample)
83108
}, 0)
@@ -117,7 +142,8 @@ async function waitForStableSessionSwitch(page: Page) {
117142
sample.destination.length > 0 &&
118143
sample.source.length === 0 &&
119144
sample.last &&
120-
Math.abs(sample.bottomErrorPx ?? Infinity) <= 1,
145+
sample.requiredPartVisible !== false &&
146+
(sample.bottomAnchorRequired === false || Math.abs(sample.bottomErrorPx ?? Infinity) <= 1),
121147
)
122148
)
123149
})
@@ -135,13 +161,27 @@ async function collectSessionSwitchResult(page: Page) {
135161

136162
export async function measureSessionSwitch(
137163
page: Page,
138-
input: { destinationIDs: string[]; sourceIDs: string[]; lastID: string; href: string; switch: () => Promise<void> },
164+
input: {
165+
destinationIDs: string[]
166+
sourceIDs: string[]
167+
lastID: string
168+
requiredPartID?: string
169+
requireBottomAnchor?: boolean
170+
href: string
171+
switch: () => Promise<void>
172+
},
139173
) {
140174
const { switch: run, ...probe } = input
141175
await installSessionSwitchProbe(page, probe)
142-
await run()
143-
await waitForStableSessionSwitch(page)
144-
return collectSessionSwitchResult(page)
176+
try {
177+
await run()
178+
await waitForStableSessionSwitch(page)
179+
return await collectSessionSwitchResult(page)
180+
} finally {
181+
await page.evaluate(() => {
182+
;(window as Window & { __sessionSwitchProbe?: SessionSwitchProbe }).__sessionSwitchProbe?.stop()
183+
})
184+
}
145185
}
146186

147187
export async function waitForStableTimeline(page: Page, lastID: string) {
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { expect, test } from "bun:test"
2+
import type { Page, Route } from "@playwright/test"
3+
import { mockOpenCodeServer } from "../../utils/mock-server"
4+
5+
test("applies message latency after a list response gate is released", async () => {
6+
const events: string[] = []
7+
const gate = Promise.withResolvers<void>()
8+
let handler: ((route: Route) => Promise<void>) | undefined
9+
const page = {
10+
route: (_url: string, callback: (route: Route) => Promise<void>) => {
11+
handler = callback
12+
return Promise.resolve()
13+
},
14+
} as unknown as Page
15+
await mockOpenCodeServer(page, {
16+
provider: {},
17+
directory: "C:/OpenCode",
18+
project: {},
19+
sessions: [{ id: "session" }],
20+
messageDelay: 25,
21+
beforeMessagesResponse: () => {
22+
events.push("before")
23+
return gate.promise
24+
},
25+
onMessages: (request) => events.push(request.phase),
26+
pageMessages: () => {
27+
events.push("page")
28+
return { items: [] }
29+
},
30+
})
31+
32+
const response = handler!({
33+
request: () => ({ url: () => "http://127.0.0.1:4096/session/session/message" }),
34+
fulfill: () => {
35+
events.push("fulfill")
36+
return Promise.resolve()
37+
},
38+
} as unknown as Route)
39+
expect(events).toEqual(["start", "before"])
40+
41+
const released = performance.now()
42+
gate.resolve()
43+
await response
44+
expect(performance.now() - released).toBeGreaterThanOrEqual(20)
45+
expect(events).toEqual(["start", "before", "page", "end", "fulfill"])
46+
})

packages/app/e2e/performance/unit/session-tab-switch-metrics.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,35 @@ test("reports missing correctness without throwing", () => {
5252
expect(result.firstCorrectObservedMs).toBeNull()
5353
expect(result.stableObservedMs).toBeNull()
5454
})
55+
56+
test("requires an explicitly tracked part to be visible", () => {
57+
const result = classifySessionSwitch([
58+
{
59+
observedAtMs: 16,
60+
destination: ["destination"],
61+
source: [],
62+
hasVisibleRows: true,
63+
last: true,
64+
requiredPartVisible: false,
65+
bottomErrorPx: 0,
66+
},
67+
])
68+
69+
expect(result.firstCorrectObservedMs).toBeNull()
70+
})
71+
72+
test("can measure content correctness without requiring a bottom anchor", () => {
73+
const result = classifySessionSwitch([
74+
{
75+
observedAtMs: 16,
76+
destination: ["destination"],
77+
source: [],
78+
hasVisibleRows: true,
79+
last: true,
80+
requiredPartVisible: true,
81+
bottomAnchorRequired: false,
82+
},
83+
])
84+
85+
expect(result.firstCorrectObservedMs).toBe(16)
86+
})
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { expect, test } from "bun:test"
2+
import type { Page } from "@playwright/test"
3+
import { measureSessionSwitch } from "../timeline/session-tab-switch-probe"
4+
5+
function testPage(waitFailure?: Error) {
6+
const stops: unknown[] = []
7+
const page = {
8+
evaluate: async (_callback: unknown, input?: unknown) => {
9+
if (input) return
10+
stops.push(undefined)
11+
},
12+
waitForFunction: async () => {
13+
if (waitFailure) throw waitFailure
14+
},
15+
} as unknown as Page
16+
return { page, stops }
17+
}
18+
19+
function input(run: () => Promise<void>) {
20+
return {
21+
destinationIDs: ["destination"],
22+
sourceIDs: ["source"],
23+
lastID: "destination",
24+
href: "/session/destination",
25+
switch: run,
26+
}
27+
}
28+
29+
test("stops sampling when the session switch fails", async () => {
30+
const failure = new Error("switch failed")
31+
const context = testPage()
32+
33+
await expect(measureSessionSwitch(context.page, input(async () => Promise.reject(failure)))).rejects.toBe(failure)
34+
35+
expect(context.stops).toHaveLength(1)
36+
})
37+
38+
test("stops sampling when the stable wait fails", async () => {
39+
const failure = new Error("stable wait failed")
40+
const context = testPage(failure)
41+
42+
await expect(measureSessionSwitch(context.page, input(async () => {}))).rejects.toBe(failure)
43+
44+
expect(context.stops).toHaveLength(1)
45+
})

0 commit comments

Comments
 (0)