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

Co-authored-by: LukeParkerDev <10430890+Hona@users.noreply.github.com>

Author: arvsrn

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

@@ -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,10 +113,32 @@ 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()
   await expect(tab).toBeVisible()
   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")
+  })
+}

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

@@ -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 ?? []))],
   }
 }
 

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

@@ -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 },

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

@@ -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()