feat(app): v2 review panel overhaul (#31882)

Co-authored-by: LukeParkerDev <10430890+Hona@users.noreply.github.com>
This commit is contained in:
Aarav Sareen
2026-07-02 13:11:58 +05:30
committed by GitHub
parent 0b5420391e
commit de915b7205
35 changed files with 3438 additions and 214 deletions
@@ -2,7 +2,13 @@ import type { Page } from "@playwright/test"
import { expectSessionTitle } from "../../utils/waits"
import { benchmark, expect, withBenchmarkPage } from "../benchmark"
import { fixture } from "./session-timeline-stress.fixture"
import { installStressSessionTabs, mockStressTimeline, stressSessionHref } from "./timeline-test-helpers"
import {
createReviewDiffs,
installStressSessionTabs,
installTimelineSettings,
mockStressTimeline,
stressSessionHref,
} from "./timeline-test-helpers"
import { measureSessionSwitch, waitForStableTimeline } from "./session-tab-switch-probe"
type Result = Awaited<ReturnType<typeof measureSessionSwitch>>
@@ -20,8 +26,41 @@ benchmark("benchmarks cold and hot session tab switching", async ({ browser, rep
report({ results, summary: summarize(results) })
})
async function trial(page: Page, mode: "cold" | "hot") {
await mockStressTimeline(page)
benchmark(
"benchmarks v2 session tab switching with and without the review pane",
async ({ browser, report }, testInfo) => {
benchmark.setTimeout(360_000)
const runs = Number(process.env.SESSION_TAB_SWITCH_RUNS ?? 5)
const results = {
closed: { cold: [] as Result[], hot: [] as Result[] },
open: { cold: [] as Result[], hot: [] as Result[] },
}
for (const reviewPane of ["closed", "open"] as const) {
for (const mode of ["cold", "hot"] as const) {
for (let run = 0; run < runs; run++) {
results[reviewPane][mode].push(
await withBenchmarkPage(
browser,
`session-tab-switch-v2-${reviewPane}-${mode}-${run}`,
(page) => trial(page, mode, { newLayoutDesigns: true, reviewPane }),
testInfo,
),
)
}
}
}
report({ results, summary: summarizeReviewPane(results) }, { runs, reviewDiffs: createReviewDiffs().length })
},
)
async function trial(
page: Page,
mode: "cold" | "hot",
options?: { newLayoutDesigns?: boolean; reviewPane?: "closed" | "open" },
) {
const reviewDiffs = options?.newLayoutDesigns ? createReviewDiffs() : undefined
await mockStressTimeline(page, { vcsDiff: reviewDiffs })
if (options?.newLayoutDesigns) await installTimelineSettings(page)
await installStressSessionTabs(page)
if (mode === "hot") {
await page.goto(stressSessionHref(fixture.targetID))
@@ -33,6 +72,10 @@ async function trial(page: Page, mode: "cold" | "hot") {
await expectSessionTitle(page, fixture.expected.sourceTitle)
}
await waitForStableTimeline(page, fixture.expected.sourceMessageIDs.at(-1)!)
if (options?.reviewPane === "open") {
await openReviewPane(page)
await waitForStableTimeline(page, fixture.expected.sourceMessageIDs.at(-1)!)
}
const destinationIDs = fixture.messages[fixture.targetID].map((message) => message.info.id)
const sourceIDs = fixture.messages[fixture.sourceID].map((message) => message.info.id)
@@ -70,6 +113,15 @@ function summarize(results: Record<"cold" | "hot", Result[]>) {
)
}
function summarizeReviewPane(results: Record<"closed" | "open", Record<"cold" | "hot", Result[]>>) {
return Object.fromEntries(
Object.entries(results).map(([reviewPane, values]) => [
reviewPane,
summarize(values as Record<"cold" | "hot", Result[]>),
]),
)
}
async function switchSession(page: Page, sessionID: string, title: string) {
const href = stressSessionHref(sessionID)
const tab = page.locator(`[data-slot="titlebar-tabs"] a[href="${href}"]`).first()
@@ -77,3 +129,16 @@ async function switchSession(page: Page, sessionID: string, title: string) {
await tab.click()
await expectSessionTitle(page, title)
}
async function openReviewPane(page: Page) {
await page.getByRole("button", { name: "Toggle review" }).click()
const panel = page.locator("#review-panel")
await expect(panel).toBeVisible()
// Text-based readiness works across review implementations; the legacy list mounts
// diff viewers lazily while V2 mounts the active preview eagerly.
await page.waitForFunction(() => {
const panel = document.querySelector<HTMLElement>("#review-panel")
const text = panel?.textContent ?? ""
return text.includes("generated-000.ts") && text.includes("+3")
})
}
@@ -5,6 +5,12 @@ export type SessionSwitchSample = {
hasVisibleRows: boolean
last: boolean
bottomErrorPx?: number
review?: {
fileHost: boolean
fileHostReplaced: boolean
header: string
replacedLevels: string[]
}
}
export function classifySessionSwitch(samples: SessionSwitchSample[]) {
@@ -23,6 +29,10 @@ export function classifySessionSwitch(samples: SessionSwitchSample[]) {
(sample) => sample.hasVisibleRows && sample.destination.length === 0 && sample.source.length === 0,
).length,
sourceSamples: samples.filter((sample) => sample.source.length > 0).length,
reviewFileHostMissingSamples: samples.filter((sample) => sample.review && !sample.review.fileHost).length,
reviewFileHostReplacedSamples: samples.filter((sample) => sample.review?.fileHostReplaced).length,
reviewHeaders: [...new Set(samples.flatMap((sample) => (sample.review ? [sample.review.header] : [])))],
reviewReplacedLevels: [...new Set(samples.flatMap((sample) => sample.review?.replacedLevels ?? []))],
}
}
@@ -16,11 +16,41 @@ async function installSessionSwitchProbe(
const samples: SessionSwitchSample[] = []
let started: number | undefined
let running = true
const reviewLevels: Record<string, string> = {
panel: "#review-panel",
tabs: '#review-panel [data-component="tabs"]',
body: '#review-panel [data-slot="session-review-v2-body"]',
review: '#review-panel [data-component="session-review-v2"]',
preview: '#review-panel [data-slot="session-review-v2-preview"]',
scroll: '#review-panel [data-slot="session-review-v2-diff-scroll"]',
file: '#review-panel [data-component="file"][data-mode="diff"]',
}
const initialReviewNodes: Record<string, Element | null> = {}
const sample = () => {
if (!running || started === undefined) return
setTimeout(() => {
if (!running || started === undefined) return
const observedAtMs = performance.now() - started
const reviewPanel = document.querySelector<HTMLElement>("#review-panel")
const reviewFile = reviewPanel?.querySelector('[data-component="file"][data-mode="diff"]')
const initialReviewFile = initialReviewNodes.file
const replacedLevels = Object.entries(reviewLevels).flatMap(([name, selector]) => {
const initial = initialReviewNodes[name]
if (!initial) return []
const current = document.querySelector(selector)
return current && current !== initial ? [name] : []
})
const review = reviewPanel
? {
fileHost: !!reviewFile,
fileHostReplaced: !!initialReviewFile && !!reviewFile && reviewFile !== initialReviewFile,
header:
reviewPanel
.querySelector<HTMLElement>('[data-slot="session-review-v2-file-header"]')
?.textContent?.trim() ?? "",
replacedLevels,
}
: undefined
const root = [...document.querySelectorAll<HTMLElement>(".scroll-view__viewport")].find((element) =>
element.querySelector("[data-timeline-row]"),
)
@@ -44,9 +74,10 @@ async function installSessionSwitchProbe(
hasVisibleRows,
last: visible.includes(lastID),
bottomErrorPx: spacer ? spacer.bottom - view.bottom : undefined,
review,
})
} else {
samples.push({ observedAtMs, destination: [], source: [], hasVisibleRows: false, last: false })
samples.push({ observedAtMs, destination: [], source: [], hasVisibleRows: false, last: false, review })
}
requestAnimationFrame(sample)
}, 0)
@@ -57,6 +88,9 @@ async function installSessionSwitchProbe(
const link = event.target instanceof Element ? event.target.closest("a") : undefined
if (link?.getAttribute("href") !== href) return
started = performance.now()
for (const [name, selector] of Object.entries(reviewLevels)) {
initialReviewNodes[name] = document.querySelector(selector)
}
requestAnimationFrame(sample)
},
{ capture: true, once: true },
@@ -93,36 +93,53 @@ const assistantMessage = {
parts: [editPart],
}
export async function setupTimelineBenchmark(page: Page, options: { historyTurns: number; eventBatch: number }) {
export async function setupTimelineBenchmark(
page: Page,
options: {
historyTurns: number
eventBatch: number
newLayoutDesigns?: boolean
vcsDiff?: unknown[]
turnDiffs?: unknown[]
},
) {
const events: EventPayload[] = []
let eventBatch = options.eventBatch
const currentUserMessage = options.turnDiffs
? { ...userMessage, info: { ...userMessage.info, summary: { diffs: options.turnDiffs } } }
: userMessage
await mockOpenCodeServer(page, {
directory,
project: project(),
provider: provider(),
sessions: [session()],
vcsDiff: options.vcsDiff,
pageMessages: () => ({
items: [
...Array.from({ length: options.historyTurns }, (_, index) => performanceTurn(index)).flat(),
userMessage,
currentUserMessage,
assistantMessage,
],
}),
events: () => events.splice(0, eventBatch),
eventRetry: 16,
})
await page.addInitScript(() => {
localStorage.setItem(
"settings.v3",
JSON.stringify({
general: {
editToolPartsExpanded: true,
shellToolPartsExpanded: true,
showReasoningSummaries: true,
},
}),
)
})
await page.addInitScript(
(input) => {
localStorage.setItem(
"settings.v3",
JSON.stringify({
general: {
newLayoutDesigns: input.newLayoutDesigns,
editToolPartsExpanded: true,
shellToolPartsExpanded: true,
showReasoningSummaries: true,
},
}),
)
},
{ newLayoutDesigns: options.newLayoutDesigns ?? false },
)
await page.setViewportSize({ width: 1366, height: 768 })
const scroller = page.locator(".scroll-view__viewport", { has: page.locator("[data-timeline-row]") })
const text = page.locator(`[data-timeline-part-id="${textPartID}"]`).first()
@@ -1,3 +1,4 @@
import type { Page } from "@playwright/test"
import { benchmark, benchmarkDiagnostics, expect } from "../benchmark"
import {
buildInitialStreamEvent,
@@ -6,80 +7,300 @@ import {
textPartID,
} from "./session-timeline-benchmark.fixture"
import { startTimelineProfile } from "./session-timeline-profile"
import { createReviewDiffs } from "./timeline-test-helpers"
import {
collectTimelineStreamMetrics,
installTimelineStreamProbe,
startTimelineStreamProbe,
} from "./session-timeline-stream-probe"
type TimelineStreamOptions = {
newLayoutDesigns?: boolean
reviewDiffs?: boolean
reviewPane?: boolean
}
type ReviewPaneSample = {
observedAtMs: number
panelVisible: boolean
header: string
diffViewers: number
diffLines: number
codeBlocks: number
ready: boolean
}
type ReviewPaneProbe = {
samples: ReviewPaneSample[]
start: () => void
stop: () => void
}
const reviewReadyStreak = 3
benchmark.describe("performance: session timeline streaming", () => {
benchmark("streams assistant text without remounting or oscillating", async ({ page, report }) => {
benchmark.setTimeout(480_000)
const cpuThrottle = Number(process.env.TIMELINE_CPU_THROTTLE ?? 30)
const deltaCount = Number(process.env.TIMELINE_DELTA_COUNT ?? 160)
const historyTurns = Number(process.env.TIMELINE_HISTORY_TURNS ?? 320)
const eventBatch = Number(process.env.TIMELINE_EVENT_BATCH ?? 1)
const minimal = process.env.TIMELINE_MINIMAL === "1"
const profileCPU = process.env.TIMELINE_CPU_PROFILE === "1"
const profileVisual = !minimal && profileCPU && process.env.TIMELINE_VISUAL_PROFILE !== "0"
benchmark.setTimeout(Number(process.env.TIMELINE_COMPLETION_TIMEOUT_MS ?? 420_000) + 60_000)
const result = await runTimelineStreamBenchmark(page, {})
report(result.metrics, result.context)
})
benchmark("streams assistant text in v2 with review pane closed", async ({ page, report }) => {
benchmark.setTimeout(Number(process.env.TIMELINE_COMPLETION_TIMEOUT_MS ?? 420_000) + 60_000)
const result = await runTimelineStreamBenchmark(page, { newLayoutDesigns: true })
report(result.metrics, result.context)
})
benchmark("streams assistant text in v2 with review diffs and pane closed", async ({ page, report }) => {
benchmark.setTimeout(Number(process.env.TIMELINE_COMPLETION_TIMEOUT_MS ?? 420_000) + 60_000)
const result = await runTimelineStreamBenchmark(page, { newLayoutDesigns: true, reviewDiffs: true })
report(result.metrics, result.context)
})
benchmark("streams assistant text in v2 with review pane open", async ({ page, report }) => {
benchmark.setTimeout(Number(process.env.TIMELINE_COMPLETION_TIMEOUT_MS ?? 420_000) + 60_000)
const result = await runTimelineStreamBenchmark(page, { newLayoutDesigns: true, reviewPane: true })
report(result.metrics, result.context)
})
})
benchmark.describe("performance: review pane", () => {
benchmark("loads v2 review diffs and switches active files", async ({ page, report }) => {
benchmark.setTimeout(240_000)
const historyTurns = Number(process.env.REVIEW_PANE_HISTORY_TURNS ?? 72)
const diffs = createReviewDiffs()
const fixture = await setupTimelineBenchmark(page, {
historyTurns,
eventBatch,
eventBatch: 1,
newLayoutDesigns: true,
vcsDiff: diffs,
})
fixture.transport.enqueue(buildInitialStreamEvent(deltaCount))
const contentStart = performance.now()
fixture.transport.enqueue(buildInitialStreamEvent(1))
await expect(fixture.text).toBeVisible()
await expect(fixture.text).toContainText("Implementation plan")
const initialContentObservedMs = performance.now() - contentStart
await fixture.scrollToBottom()
await fixture.waitForStableGeometry()
const profile = await startTimelineProfile(page, { cpuThrottle, profileCPU })
await installTimelineStreamProbe(page, { textPartID, finalIndex: deltaCount, profileVisual, minimal })
const deltas = buildStreamDeltaEvents(deltaCount)
await startTimelineStreamProbe(page)
fixture.transport.enqueue(deltas)
await page.waitForFunction(
(finalIndex) =>
(
window as Window & {
__timelineStreamBenchmark?: { applied: { index: number }[] }
}
).__timelineStreamBenchmark?.applied.some((value) => value.index === finalIndex),
deltaCount,
{ timeout: 420_000 },
)
await expect(fixture.text).toContainText("benchmark-complete")
await expect(fixture.text).toContainText("Streaming")
await fixture.waitForStableGeometry()
const metrics = await collectTimelineStreamMetrics(page, {
textPartID,
finalIndex: deltaCount,
navigations: benchmarkDiagnostics(page).navigations,
})
const delivered = deltas.length - fixture.transport.pendingCount()
await profile.stop()
const open = await measureReviewPaneLoad(page, diffs[0]!.file)
const switches = []
for (const diff of diffs.slice(1, 4)) switches.push(await measureReviewNextFile(page, diff.file))
report(
{
endToEndInitialContentObservedMs: initialContentObservedMs,
...metrics,
deliveredDeltas: delivered,
pendingDeltas: fixture.transport.pendingCount(),
open,
switches,
},
{
cpuThrottle,
profileCPU,
profileVisual,
minimal,
queuedDeltas: deltas.length,
historyTurns,
eventBatch,
reviewDiffs: diffs.length,
},
)
await profile.reset()
})
})
async function runTimelineStreamBenchmark(page: Page, options: TimelineStreamOptions) {
const completionTimeoutMs = Number(process.env.TIMELINE_COMPLETION_TIMEOUT_MS ?? 420_000)
const cpuThrottle = Number(process.env.TIMELINE_CPU_THROTTLE ?? 30)
const deltaCount = Number(process.env.TIMELINE_DELTA_COUNT ?? 160)
const historyTurns = Number(process.env.TIMELINE_HISTORY_TURNS ?? 320)
const eventBatch = Number(process.env.TIMELINE_EVENT_BATCH ?? 1)
const minimal = process.env.TIMELINE_MINIMAL === "1"
const profileCPU = process.env.TIMELINE_CPU_PROFILE === "1"
const profileVisual = !minimal && profileCPU && process.env.TIMELINE_VISUAL_PROFILE !== "0"
const diffs = options.reviewDiffs || options.reviewPane ? createReviewDiffs() : undefined
const fixture = await setupTimelineBenchmark(page, {
historyTurns,
eventBatch,
newLayoutDesigns: options.newLayoutDesigns,
// Turn diffs exercise timeline data cost; the pane-open scenario serves the same
// diffs through the default git mode so it works across review implementations.
turnDiffs: options.reviewDiffs ? diffs : undefined,
vcsDiff: options.reviewPane ? diffs : undefined,
})
fixture.transport.enqueue(buildInitialStreamEvent(deltaCount))
const contentStart = performance.now()
await expect(fixture.text).toBeVisible()
await expect(fixture.text).toContainText("Implementation plan")
const initialContentObservedMs = performance.now() - contentStart
await fixture.scrollToBottom()
await fixture.waitForStableGeometry()
const reviewPane = options.reviewPane && diffs ? await measureReviewPaneLoad(page, diffs[0]!.file) : undefined
if (reviewPane) await fixture.waitForStableGeometry()
const profile = await startTimelineProfile(page, { cpuThrottle, profileCPU })
await installTimelineStreamProbe(page, { textPartID, finalIndex: deltaCount, profileVisual, minimal })
const deltas = buildStreamDeltaEvents(deltaCount)
await startTimelineStreamProbe(page)
fixture.transport.enqueue(deltas)
await page.waitForFunction(
(finalIndex) =>
(
window as Window & {
__timelineStreamBenchmark?: { applied: { index: number }[] }
}
).__timelineStreamBenchmark?.applied.some((value) => value.index === finalIndex),
deltaCount,
{ timeout: completionTimeoutMs },
)
await expect(fixture.text).toContainText("benchmark-complete")
await expect(fixture.text).toContainText("Streaming")
await fixture.waitForStableGeometry()
const metrics = await collectTimelineStreamMetrics(page, {
textPartID,
finalIndex: deltaCount,
navigations: benchmarkDiagnostics(page).navigations,
})
const delivered = deltas.length - fixture.transport.pendingCount()
await profile.stop()
const result = {
metrics: {
endToEndInitialContentObservedMs: initialContentObservedMs,
...metrics,
deliveredDeltas: delivered,
pendingDeltas: fixture.transport.pendingCount(),
reviewPane: reviewPane ?? null,
},
context: {
cpuThrottle,
profileCPU,
profileVisual,
minimal,
queuedDeltas: deltas.length,
historyTurns,
eventBatch,
newLayoutDesigns: options.newLayoutDesigns === true,
reviewPane: options.reviewPane === true ? "open" : "closed",
reviewDiffs: diffs?.length ?? 0,
},
}
await profile.reset()
return result
}
async function measureReviewPaneLoad(page: Page, file: string) {
// Default git mode reads the mocked /vcs/diff data, so opening the pane is enough
// and the flow works across review pane implementations.
await installReviewPaneProbe(page, { file })
await startReviewPaneProbe(page)
await page.getByRole("button", { name: "Toggle review" }).click()
await expect(page.locator("#review-panel")).toBeVisible()
return collectReviewPaneProbe(page)
}
async function measureReviewNextFile(page: Page, file: string) {
await installReviewPaneProbe(page, { file })
await startReviewPaneProbe(page)
await page.getByRole("button", { name: "Next file" }).click()
return collectReviewPaneProbe(page)
}
async function installReviewPaneProbe(page: Page, input: { file: string }) {
await page.evaluate((input) => {
const samples: ReviewPaneSample[] = []
const basename = input.file.split(/[\\/]/).at(-1) ?? input.file
let started: number | undefined
let running = true
const paneState = () => {
const panel = document.querySelector<HTMLElement>("#review-panel")
const review = panel?.querySelector<HTMLElement>('[data-component="session-review-v2"]')
const rect = (review ?? panel)?.getBoundingClientRect()
const text = panel?.textContent ?? ""
const previewHeader = panel?.querySelector<HTMLElement>(
'[data-slot="session-review-v2-file-header"]',
)?.textContent
const header = previewHeader ?? text
const viewers = panel ? [...panel.querySelectorAll<HTMLElement>('[data-component="file"][data-mode="diff"]')] : []
const codeBlocks = panel?.querySelectorAll("code").length ?? 0
const diffLines = viewers.reduce(
(sum, viewer) =>
sum +
(viewer.shadowRoot?.querySelectorAll("[data-line]").length ?? viewer.querySelectorAll("[data-line]").length),
0,
)
const panelVisible =
!!panel && panel.getAttribute("aria-hidden") !== "true" && !!rect && rect.width > 0 && rect.height > 0
return {
panelVisible,
header: header.slice(0, 500),
diffViewers: viewers.length,
diffLines,
codeBlocks,
ready:
panelVisible &&
header.includes(basename) &&
(viewers.length > 0 || text.includes("+3") || diffLines > 0 || codeBlocks > 0),
}
}
const sample = () => {
if (!running || started === undefined) return
requestAnimationFrame(() => {
setTimeout(() => {
if (!running || started === undefined) return
samples.push({ observedAtMs: performance.now() - started, ...paneState() })
if (performance.now() - started < 10_000) sample()
}, 0)
})
}
;(window as Window & { __reviewPaneProbe?: ReviewPaneProbe }).__reviewPaneProbe = {
samples,
start: () => {
started = performance.now()
performance.mark("opencode.review-pane.click")
sample()
},
stop: () => {
running = false
},
}
}, input)
}
async function startReviewPaneProbe(page: Page) {
await page.evaluate(() => {
;(window as Window & { __reviewPaneProbe?: ReviewPaneProbe }).__reviewPaneProbe!.start()
})
}
async function collectReviewPaneProbe(page: Page) {
await page.waitForFunction((streak) => {
const samples = (window as Window & { __reviewPaneProbe?: ReviewPaneProbe }).__reviewPaneProbe?.samples
if (!samples) return false
return samples.some((_, index) => {
const stable = samples.slice(index, index + streak)
return stable.length === streak && stable.every((sample) => sample.ready)
})
}, reviewReadyStreak)
const samples = await page.evaluate(() => {
const probe = (window as Window & { __reviewPaneProbe?: ReviewPaneProbe }).__reviewPaneProbe!
probe.stop()
return probe.samples
})
return { summary: summarizeReviewPaneSamples(samples), samples }
}
function summarizeReviewPaneSamples(samples: ReviewPaneSample[]) {
const firstReady = samples.find((sample) => sample.ready)
const stableIndex = samples.findIndex((_, index) => {
const stable = samples.slice(index, index + reviewReadyStreak)
return stable.length === reviewReadyStreak && stable.every((sample) => sample.ready)
})
return {
samples: samples.length,
firstReadyObservedMs: firstReady?.observedAtMs ?? null,
stableReadyObservedMs: stableIndex === -1 ? null : samples[stableIndex + reviewReadyStreak - 1]!.observedAtMs,
notReadySamples: samples.filter((sample) => !sample.ready).length,
maxDiffViewers: Math.max(0, ...samples.map((sample) => sample.diffViewers)),
maxDiffLines: Math.max(0, ...samples.map((sample) => sample.diffLines)),
maxCodeBlocks: Math.max(0, ...samples.map((sample) => sample.codeBlocks)),
}
}
@@ -21,7 +21,10 @@ export async function installTimelineSettings(page: Page) {
export function mockStressTimeline(
page: Page,
input?: { onMessages?: (input: { sessionID: string; before?: string; phase: "start" | "end" }) => void },
input?: {
onMessages?: (input: { sessionID: string; before?: string; phase: "start" | "end" }) => void
vcsDiff?: unknown[]
},
) {
return mockOpenCodeServer(page, {
sessions: fixture.sessions,
@@ -30,6 +33,7 @@ export function mockStressTimeline(
project: fixture.project,
pageMessages,
onMessages: input?.onMessages,
vcsDiff: input?.vcsDiff,
})
}
@@ -78,3 +82,53 @@ export function stressDraftHref(draftID: string) {
function stressServer() {
return `http://${process.env.PLAYWRIGHT_SERVER_HOST ?? "127.0.0.1"}:${process.env.PLAYWRIGHT_SERVER_PORT ?? "4096"}`
}
export function createReviewDiffs() {
return Array.from({ length: Number(process.env.REVIEW_PANE_DIFF_COUNT ?? 72) }, (_, index) => {
const lines = index % 3 === 0 ? 300 : index % 3 === 1 ? 120 : 38
const file = `src/review/generated-${String(index).padStart(3, "0")}.ts`
const before = reviewSource(index, lines)
const after = before
.replace(`value_${index}_4`, `updated_${index}_4`)
.replace(
`value_${index}_${Math.max(8, Math.floor(lines / 2))}`,
`updated_${index}_${Math.max(8, Math.floor(lines / 2))}`,
)
.replace(`value_${index}_${lines - 4}`, `updated_${index}_${lines - 4}`)
return {
file,
patch: reviewPatch(file, before, after),
additions: 3,
deletions: 3,
status: "modified" as const,
}
})
}
function reviewSource(seed: number, lines: number) {
return Array.from(
{ length: lines },
(_, index) => `export const value_${seed}_${index} = "${reviewWords(seed + index, index % 5 === 0 ? 180 : 42)}"`,
).join("\n")
}
function reviewPatch(file: string, before: string, after: string) {
const beforeLines = before.split("\n")
const afterLines = after.split("\n")
return [
`diff --git a/${file} b/${file}`,
`--- a/${file}`,
`+++ b/${file}`,
`@@ -1,${beforeLines.length} +1,${afterLines.length} @@`,
...beforeLines.flatMap((line, index) => {
const next = afterLines[index]!
if (line === next) return [` ${line}`]
return [`-${line}`, `+${next}`]
}),
].join("\n")
}
function reviewWords(seed: number, length: number) {
const words = ["alpha", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel", "india", "juliet"]
return Array.from({ length: Math.ceil(length / 7) }, (_, index) => words[(seed + index * 3) % words.length]).join(" ")
}
@@ -0,0 +1,202 @@
import { expect, test, type Page } from "@playwright/test"
import { base64Encode } from "@opencode-ai/core/util/encode"
import { mockOpenCodeServer } from "../utils/mock-server"
import { expectAppVisible, expectSessionTitle } from "../utils/waits"
const directory = "C:/OpenCode/ReviewImageFlashRegression"
const sessionID = "ses_review_image_flash_regression"
const title = "Review image flash regression"
const imageFile = "assets/preview.png"
test("clicking an image file in the v2 review pane does not blank the panel", async ({ page }) => {
await openReview(page)
await installReviewFlashProbe(page)
await page.getByRole("button", { name: /preview\.png/ }).click()
await waitForReviewFlashProbe(page, 400)
const trace = await collectReviewFlashProbe(page)
const bad = trace.samples.filter((sample) => sample.blank || sample.blackCenter)
expect(trace.samples.length).toBeGreaterThan(0)
expect(
bad,
JSON.stringify({ bad: bad.slice(0, 8), first: trace.samples.slice(0, 8), last: trace.samples.slice(-4) }, null, 2),
).toEqual([])
})
async function openReview(page: Page) {
await page.setViewportSize({ width: 960, height: 900 })
await page.addInitScript(() => {
localStorage.setItem("settings.v3", JSON.stringify({ general: { newLayoutDesigns: true } }))
})
await mockOpenCodeServer(page, {
directory,
project: {
id: "proj_review_image_flash_regression",
worktree: directory,
vcs: "git",
name: "review-image-flash-regression",
time: { created: 1700000000000, updated: 1700000000000 },
sandboxes: [],
},
provider: { all: [], connected: [], default: {} },
sessions: [
{
id: sessionID,
slug: "review-image-flash-regression",
projectID: "proj_review_image_flash_regression",
directory,
title,
version: "dev",
time: { created: 1700000000000, updated: 1700000000000 },
},
],
vcsDiff: [
{
file: "src/example.ts",
additions: 1,
deletions: 1,
status: "modified",
patch:
"diff --git a/src/example.ts b/src/example.ts\n--- a/src/example.ts\n+++ b/src/example.ts\n@@ -1 +1 @@\n-export const value = 'before'\n+export const value = 'after'\n",
},
{
file: imageFile,
patch: "",
additions: 1,
deletions: 0,
status: "added",
},
],
fileContent: async (path) => {
if (path !== imageFile) return undefined
await new Promise((resolve) => setTimeout(resolve, 250))
return {
type: "binary",
content: "iVBORw0KGgo=",
encoding: "base64",
mimeType: "image/png",
}
},
fileList: (path) => {
if (!path) {
return [
{ name: "assets", path: "assets", absolute: `${directory}/assets`, type: "directory", ignored: false },
{ name: "src", path: "src", absolute: `${directory}/src`, type: "directory", ignored: false },
]
}
if (path === "assets") {
return [
{
name: "preview.png",
path: imageFile,
absolute: `${directory}/${imageFile}`,
type: "file",
ignored: false,
},
]
}
if (path === "src") {
return [
{
name: "example.ts",
path: "src/example.ts",
absolute: `${directory}/src/example.ts`,
type: "file",
ignored: false,
},
]
}
return []
},
pageMessages: () => ({
items: [
{
info: {
id: "msg_review_image_flash_regression",
sessionID,
role: "user",
time: { created: 1700000000000 },
summary: { diffs: [] },
agent: "build",
model: { providerID: "opencode", modelID: "test" },
},
parts: [
{
id: "prt_review_image_flash_regression",
sessionID,
messageID: "msg_review_image_flash_regression",
type: "text",
text: "Review this change.",
},
],
},
],
}),
})
await page.goto(`/${base64Encode(directory)}/session/${sessionID}`)
await expectSessionTitle(page, title)
await page.getByRole("button", { name: "Toggle review" }).click()
await expectAppVisible(page.locator('#review-panel [data-component="session-review-v2"]'))
await expectAppVisible(page.getByRole("button", { name: /preview\.png/ }))
}
async function installReviewFlashProbe(page: Page) {
await page.evaluate(() => {
const samples: Array<{
observedAtMs: number
blank: boolean
blackCenter: boolean
text: string
background: string
}> = []
const startedAt = performance.now()
const sample = () => {
const panel = document.querySelector<HTMLElement>('#review-panel [data-component="session-review-v2"]')
const rect = panel?.getBoundingClientRect()
const center = rect
? document.elementFromPoint(rect.left + rect.width / 2, rect.top + rect.height / 2)
: undefined
const background = center instanceof Element ? getComputedStyle(center).backgroundColor : ""
samples.push({
observedAtMs: performance.now() - startedAt,
blank: !panel || panel.textContent?.trim().length === 0,
blackCenter: background === "rgb(0, 0, 0)",
text: panel?.textContent?.trim().slice(0, 80) ?? "",
background,
})
if (performance.now() - startedAt < 500) requestAnimationFrame(sample)
}
document.addEventListener(
"click",
(event) => {
const target = event.target instanceof Element ? event.target : undefined
if (!target?.closest('[data-slot="file-tree-v2-row"]')) return
requestAnimationFrame(sample)
},
{ capture: true, once: true },
)
;(window as Window & { __reviewImageFlash?: { samples: typeof samples; startedAt: number } }).__reviewImageFlash = {
samples,
startedAt,
}
})
}
async function waitForReviewFlashProbe(page: Page, durationMs: number) {
await page.waitForFunction((durationMs) => {
const state = (window as Window & { __reviewImageFlash?: { samples: unknown[]; startedAt: number } })
.__reviewImageFlash
return !!state && state.samples.length > 0 && performance.now() - state.startedAt >= durationMs
}, durationMs)
}
async function collectReviewFlashProbe(page: Page) {
return page.evaluate(() => {
return (window as Window & { __reviewImageFlash?: { samples: unknown[]; startedAt: number } }).__reviewImageFlash!
}) as Promise<{
startedAt: number
samples: Array<{ observedAtMs: number; blank: boolean; blackCenter: boolean; text: string; background: string }>
}>
}
+6
View File
@@ -17,6 +17,8 @@ export interface MockServerConfig {
todos?: (sessionID: string) => unknown[]
permissions?: unknown[] | (() => unknown[])
questions?: unknown[] | (() => unknown[])
fileList?: (path: string) => unknown | Promise<unknown>
fileContent?: (path: string) => unknown | Promise<unknown>
sessionStatus?: unknown
}
@@ -56,6 +58,10 @@ export async function mockOpenCodeServer(page: Page, config: MockServerConfig) {
return json(route, typeof config.questions === "function" ? config.questions() : (config.questions ?? []))
if (path === "/session/status") return json(route, config.sessionStatus ?? {})
if (path === "/vcs/diff" && config.vcsDiff) return json(route, config.vcsDiff)
if (path === "/file" && config.fileList)
return json(route, await config.fileList(url.searchParams.get("path") ?? ""))
if (path === "/file/content" && config.fileContent)
return json(route, await config.fileContent(url.searchParams.get("path") ?? ""))
if (emptyObject.has(path)) return json(route, {})
if (emptyList.has(path)) return json(route, [])
if (path in staticRoutes) return json(route, staticRoutes[path])