Skip to content

Commit a8bc8ad

Browse files
Apply PR #31882: feat(app): v2 review panel overhaul
2 parents 0d44806 + 7f661ec commit a8bc8ad

64 files changed

Lines changed: 6475 additions & 243 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 107 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { Page } from "@playwright/test"
22
import { expectSessionTitle } from "../../utils/waits"
33
import { benchmark, expect, withBenchmarkPage } from "../benchmark"
44
import { fixture } from "./session-timeline-stress.fixture"
5-
import { installStressSessionTabs, mockStressTimeline, stressSessionHref } from "./timeline-test-helpers"
5+
import { installStressSessionTabs, installTimelineSettings, mockStressTimeline, stressSessionHref } from "./timeline-test-helpers"
66
import { measureSessionSwitch, waitForStableTimeline } from "./session-tab-switch-probe"
77

88
type Result = Awaited<ReturnType<typeof measureSessionSwitch>>
@@ -20,8 +20,38 @@ benchmark("benchmarks cold and hot session tab switching", async ({ browser, rep
2020
report({ results, summary: summarize(results) })
2121
})
2222

23-
async function trial(page: Page, mode: "cold" | "hot") {
24-
await mockStressTimeline(page)
23+
benchmark("benchmarks v2 session tab switching with and without the review pane", async ({ browser, report }, testInfo) => {
24+
benchmark.setTimeout(360_000)
25+
const runs = Number(process.env.SESSION_TAB_SWITCH_RUNS ?? 5)
26+
const results = {
27+
closed: { cold: [] as Result[], hot: [] as Result[] },
28+
open: { cold: [] as Result[], hot: [] as Result[] },
29+
}
30+
for (const reviewPane of ["closed", "open"] as const) {
31+
for (const mode of ["cold", "hot"] as const) {
32+
for (let run = 0; run < runs; run++) {
33+
results[reviewPane][mode].push(
34+
await withBenchmarkPage(
35+
browser,
36+
`session-tab-switch-v2-${reviewPane}-${mode}-${run}`,
37+
(page) => trial(page, mode, { newLayoutDesigns: true, reviewPane }),
38+
testInfo,
39+
),
40+
)
41+
}
42+
}
43+
}
44+
report({ results, summary: summarizeReviewPane(results) }, { runs, reviewDiffs: createReviewDiffs().length })
45+
})
46+
47+
async function trial(
48+
page: Page,
49+
mode: "cold" | "hot",
50+
options?: { newLayoutDesigns?: boolean; reviewPane?: "closed" | "open" },
51+
) {
52+
const reviewDiffs = options?.newLayoutDesigns ? createReviewDiffs() : undefined
53+
await mockStressTimeline(page, { vcsDiff: reviewDiffs })
54+
if (options?.newLayoutDesigns) await installTimelineSettings(page)
2555
await installStressSessionTabs(page)
2656
if (mode === "hot") {
2757
await page.goto(stressSessionHref(fixture.targetID))
@@ -33,6 +63,10 @@ async function trial(page: Page, mode: "cold" | "hot") {
3363
await expectSessionTitle(page, fixture.expected.sourceTitle)
3464
}
3565
await waitForStableTimeline(page, fixture.expected.sourceMessageIDs.at(-1)!)
66+
if (options?.reviewPane === "open") {
67+
await openReviewPane(page)
68+
await waitForStableTimeline(page, fixture.expected.sourceMessageIDs.at(-1)!)
69+
}
3670

3771
const destinationIDs = fixture.messages[fixture.targetID].map((message) => message.info.id)
3872
const sourceIDs = fixture.messages[fixture.sourceID].map((message) => message.info.id)
@@ -70,10 +104,80 @@ function summarize(results: Record<"cold" | "hot", Result[]>) {
70104
)
71105
}
72106

107+
function summarizeReviewPane(results: Record<"closed" | "open", Record<"cold" | "hot", Result[]>>) {
108+
return Object.fromEntries(
109+
Object.entries(results).map(([reviewPane, values]) => [reviewPane, summarize(values as Record<"cold" | "hot", Result[]>)]),
110+
)
111+
}
112+
73113
async function switchSession(page: Page, sessionID: string, title: string) {
74114
const href = stressSessionHref(sessionID)
75115
const tab = page.locator(`[data-slot="titlebar-tabs"] a[href="${href}"]`).first()
76116
await expect(tab).toBeVisible()
77117
await tab.click()
78118
await expectSessionTitle(page, title)
79119
}
120+
121+
async function openReviewPane(page: Page) {
122+
await page.getByRole("button", { name: "Toggle review" }).click()
123+
const panel = page.locator("#review-panel")
124+
await expect(panel).toBeVisible()
125+
// Text-based readiness works across review implementations; the legacy list mounts
126+
// diff viewers lazily while V2 mounts the active preview eagerly.
127+
await page.waitForFunction(() => {
128+
const panel = document.querySelector<HTMLElement>("#review-panel")
129+
const text = panel?.textContent ?? ""
130+
return text.includes("generated-000.ts") && text.includes("+3")
131+
})
132+
}
133+
134+
function createReviewDiffs() {
135+
return Array.from({ length: Number(process.env.REVIEW_PANE_DIFF_COUNT ?? 72) }, (_, index) => {
136+
const lines = index % 3 === 0 ? 300 : index % 3 === 1 ? 120 : 38
137+
const file = `src/review/generated-${String(index).padStart(3, "0")}.ts`
138+
const before = reviewSource(index, lines)
139+
const after = before
140+
.replace(`value_${index}_4`, `updated_${index}_4`)
141+
.replace(
142+
`value_${index}_${Math.max(8, Math.floor(lines / 2))}`,
143+
`updated_${index}_${Math.max(8, Math.floor(lines / 2))}`,
144+
)
145+
.replace(`value_${index}_${lines - 4}`, `updated_${index}_${lines - 4}`)
146+
return {
147+
file,
148+
patch: reviewPatch(file, before, after),
149+
additions: 3,
150+
deletions: 3,
151+
status: "modified" as const,
152+
}
153+
})
154+
}
155+
156+
function reviewSource(seed: number, lines: number) {
157+
return Array.from(
158+
{ length: lines },
159+
(_, index) =>
160+
`export const value_${seed}_${index} = "${reviewWords(seed + index, index % 5 === 0 ? 180 : 42)}"`,
161+
).join("\n")
162+
}
163+
164+
function reviewPatch(file: string, before: string, after: string) {
165+
const beforeLines = before.split("\n")
166+
const afterLines = after.split("\n")
167+
return [
168+
`diff --git a/${file} b/${file}`,
169+
`--- a/${file}`,
170+
`+++ b/${file}`,
171+
`@@ -1,${beforeLines.length} +1,${afterLines.length} @@`,
172+
...beforeLines.flatMap((line, index) => {
173+
const next = afterLines[index]!
174+
if (line === next) return [` ${line}`]
175+
return [`-${line}`, `+${next}`]
176+
}),
177+
].join("\n")
178+
}
179+
180+
function reviewWords(seed: number, length: number) {
181+
const words = ["alpha", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel", "india", "juliet"]
182+
return Array.from({ length: Math.ceil(length / 7) }, (_, index) => words[(seed + index * 3) % words.length]).join(" ")
183+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ export type SessionSwitchSample = {
55
hasVisibleRows: boolean
66
last: boolean
77
bottomErrorPx?: number
8+
review?: {
9+
fileHost: boolean
10+
fileHostReplaced: boolean
11+
header: string
12+
replacedLevels: string[]
13+
}
814
}
915

1016
export function classifySessionSwitch(samples: SessionSwitchSample[]) {
@@ -23,6 +29,12 @@ export function classifySessionSwitch(samples: SessionSwitchSample[]) {
2329
(sample) => sample.hasVisibleRows && sample.destination.length === 0 && sample.source.length === 0,
2430
).length,
2531
sourceSamples: samples.filter((sample) => sample.source.length > 0).length,
32+
reviewFileHostMissingSamples: samples.filter((sample) => sample.review && !sample.review.fileHost).length,
33+
reviewFileHostReplacedSamples: samples.filter((sample) => sample.review?.fileHostReplaced).length,
34+
reviewHeaders: [...new Set(samples.flatMap((sample) => (sample.review ? [sample.review.header] : [])))],
35+
reviewReplacedLevels: [
36+
...new Set(samples.flatMap((sample) => sample.review?.replacedLevels ?? [])),
37+
],
2638
}
2739
}
2840

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

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,48 @@ async function installSessionSwitchProbe(
1212
) {
1313
await page.evaluate(({ destinationIDs, sourceIDs, lastID, href }) => {
1414
const destination = new Set(destinationIDs)
15-
const source = new Set(sourceIDs)
16-
const samples: SessionSwitchSample[] = []
17-
let started: number | undefined
18-
let running = true
19-
const sample = () => {
20-
if (!running || started === undefined) return
21-
setTimeout(() => {
15+
const source = new Set(sourceIDs)
16+
const samples: SessionSwitchSample[] = []
17+
let started: number | undefined
18+
let running = true
19+
const reviewLevels: Record<string, string> = {
20+
panel: "#review-panel",
21+
tabs: '#review-panel [data-component="tabs-v2"]',
22+
body: "#review-panel .session-review-v2-panel-body",
23+
review: '#review-panel [data-component="session-review-v2"]',
24+
preview: '#review-panel [data-slot="session-review-v2-preview"]',
25+
scroll: '#review-panel [data-slot="session-review-v2-diff-scroll"]',
26+
file: '#review-panel [data-component="file"][data-mode="diff"]',
27+
}
28+
const initialReviewNodes: Record<string, Element | null> = {}
29+
const sample = () => {
2230
if (!running || started === undefined) return
23-
const observedAtMs = performance.now() - started
24-
const root = [...document.querySelectorAll<HTMLElement>(".scroll-view__viewport")].find((element) =>
25-
element.querySelector("[data-timeline-row]"),
26-
)
31+
setTimeout(() => {
32+
if (!running || started === undefined) return
33+
const observedAtMs = performance.now() - started
34+
const reviewPanel = document.querySelector<HTMLElement>("#review-panel")
35+
const reviewFile = reviewPanel?.querySelector('[data-component="file"][data-mode="diff"]')
36+
const initialReviewFile = initialReviewNodes.file
37+
const replacedLevels = Object.entries(reviewLevels).flatMap(([name, selector]) => {
38+
const initial = initialReviewNodes[name]
39+
if (!initial) return []
40+
const current = document.querySelector(selector)
41+
return current && current !== initial ? [name] : []
42+
})
43+
const review = reviewPanel
44+
? {
45+
fileHost: !!reviewFile,
46+
fileHostReplaced: !!initialReviewFile && !!reviewFile && reviewFile !== initialReviewFile,
47+
header:
48+
reviewPanel
49+
.querySelector<HTMLElement>('[data-slot="session-review-v2-file-header"]')
50+
?.textContent?.trim() ?? "",
51+
replacedLevels,
52+
}
53+
: undefined
54+
const root = [...document.querySelectorAll<HTMLElement>(".scroll-view__viewport")].find((element) =>
55+
element.querySelector("[data-timeline-row]"),
56+
)
2757
if (root) {
2858
const view = root.getBoundingClientRect()
2959
const visible = [...root.querySelectorAll<HTMLElement>("[data-message-id]")]
@@ -37,16 +67,17 @@ async function installSessionSwitchProbe(
3767
return rect.bottom > view.top && rect.top < view.bottom
3868
})
3969
const spacer = root.querySelector<HTMLElement>('[data-timeline-row="bottom-spacer"]')?.getBoundingClientRect()
40-
samples.push({
41-
observedAtMs,
42-
destination: visible.filter((id) => destination.has(id)),
43-
source: visible.filter((id) => source.has(id)),
44-
hasVisibleRows,
45-
last: visible.includes(lastID),
46-
bottomErrorPx: spacer ? spacer.bottom - view.bottom : undefined,
47-
})
70+
samples.push({
71+
observedAtMs,
72+
destination: visible.filter((id) => destination.has(id)),
73+
source: visible.filter((id) => source.has(id)),
74+
hasVisibleRows,
75+
last: visible.includes(lastID),
76+
bottomErrorPx: spacer ? spacer.bottom - view.bottom : undefined,
77+
review,
78+
})
4879
} else {
49-
samples.push({ observedAtMs, destination: [], source: [], hasVisibleRows: false, last: false })
80+
samples.push({ observedAtMs, destination: [], source: [], hasVisibleRows: false, last: false, review })
5081
}
5182
requestAnimationFrame(sample)
5283
}, 0)
@@ -57,6 +88,9 @@ async function installSessionSwitchProbe(
5788
const link = event.target instanceof Element ? event.target.closest("a") : undefined
5889
if (link?.getAttribute("href") !== href) return
5990
started = performance.now()
91+
for (const [name, selector] of Object.entries(reviewLevels)) {
92+
initialReviewNodes[name] = document.querySelector(selector)
93+
}
6094
requestAnimationFrame(sample)
6195
},
6296
{ capture: true, once: true },

packages/app/e2e/performance/timeline/session-timeline-benchmark.fixture.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,36 +93,50 @@ const assistantMessage = {
9393
parts: [editPart],
9494
}
9595

96-
export async function setupTimelineBenchmark(page: Page, options: { historyTurns: number; eventBatch: number }) {
96+
export async function setupTimelineBenchmark(
97+
page: Page,
98+
options: {
99+
historyTurns: number
100+
eventBatch: number
101+
newLayoutDesigns?: boolean
102+
vcsDiff?: unknown[]
103+
turnDiffs?: unknown[]
104+
},
105+
) {
97106
const events: EventPayload[] = []
98107
let eventBatch = options.eventBatch
108+
const currentUserMessage = options.turnDiffs
109+
? { ...userMessage, info: { ...userMessage.info, summary: { diffs: options.turnDiffs } } }
110+
: userMessage
99111
await mockOpenCodeServer(page, {
100112
directory,
101113
project: project(),
102114
provider: provider(),
103115
sessions: [session()],
116+
vcsDiff: options.vcsDiff,
104117
pageMessages: () => ({
105118
items: [
106119
...Array.from({ length: options.historyTurns }, (_, index) => performanceTurn(index)).flat(),
107-
userMessage,
120+
currentUserMessage,
108121
assistantMessage,
109122
],
110123
}),
111124
events: () => events.splice(0, eventBatch),
112125
eventRetry: 16,
113126
})
114-
await page.addInitScript(() => {
127+
await page.addInitScript((input) => {
115128
localStorage.setItem(
116129
"settings.v3",
117130
JSON.stringify({
118131
general: {
132+
newLayoutDesigns: input.newLayoutDesigns,
119133
editToolPartsExpanded: true,
120134
shellToolPartsExpanded: true,
121135
showReasoningSummaries: true,
122136
},
123137
}),
124138
)
125-
})
139+
}, { newLayoutDesigns: options.newLayoutDesigns ?? false })
126140
await page.setViewportSize({ width: 1366, height: 768 })
127141
const scroller = page.locator(".scroll-view__viewport", { has: page.locator("[data-timeline-row]") })
128142
const text = page.locator(`[data-timeline-part-id="${textPartID}"]`).first()

0 commit comments

Comments
 (0)