From 2ca55b479d5fb23da1f910e82fbea45aababd983 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Thu, 27 Aug 2026 13:10:27 +1000 Subject: [PATCH] fix(app): reduce tab switch rendering work (#45428) --- packages/app/e2e/performance/README.md | 9 + .../scroll-interaction.spec.ts | 61 ++++++- .../session-tab-switch-benchmark.spec.ts | 150 ++++++---------- .../timeline/session-tab-switch-metrics.ts | 3 +- .../timeline/session-tab-switch-probe.spec.ts | 67 +++++++ .../timeline/session-tab-switch-probe.ts | 61 ++++--- .../unit/session-tab-switch-metrics.test.ts | 9 + packages/app/src/session/screen-layout.ts | 3 +- packages/app/src/session/screen.tsx | 14 +- .../app/src/session/timeline/virtualizer.tsx | 165 +++++++++++------- .../component-tests/markdown.fixture.tsx | 36 ++++ .../component-tests/markdown.spec.ts | 120 +++++++++++++ .../session-ui/src/components/markdown.tsx | 39 +++-- 13 files changed, 511 insertions(+), 226 deletions(-) create mode 100644 packages/app/e2e/performance/timeline/session-tab-switch-probe.spec.ts create mode 100644 packages/session-ui/component-tests/markdown.fixture.tsx create mode 100644 packages/session-ui/component-tests/markdown.spec.ts diff --git a/packages/app/e2e/performance/README.md b/packages/app/e2e/performance/README.md index 26494e89b8f..7164abcd86f 100644 --- a/packages/app/e2e/performance/README.md +++ b/packages/app/e2e/performance/README.md @@ -79,6 +79,15 @@ Benchmarks do not assert machine-dependent performance budgets. Streaming proces Committed smoke and regression tests continue to own correctness coverage for pagination, tab paint, context resize, collapse state, and composer spacing. +Tab-switch timing starts at `mousedown`, when mouse-selected tabs actually navigate, with a `click` fallback for keyboard activation. The probe excludes hidden/transparent content and intersects answers with their virtual-row clip and viewport. The tab workload requires the destination's final answer to be visible with Markdown ready. These results are not directly comparable to older click-start, geometry-only measurements. `stableObservedMs` includes confirmation across three correct samples; `firstCorrectObservedMs` is the first sample meeting all content and geometry checks. Neither is a compositor presentation timestamp. + +Each tab scenario reports one sample, including its raw observations. Use Playwright's `--repeat-each=5` for repeated measurements. Cached scenarios warm the destination at the same panel width before leaving it; a separate resized scenario validates reuse after opening the review pane changes that width. + +```sh +bunx playwright test --config e2e/performance/playwright.config.ts \ + timeline/session-tab-switch-benchmark.spec.ts --repeat-each=5 +``` + ## Retained renderer memory Run the catalog workload against the production app bundle: diff --git a/packages/app/e2e/performance/timeline-stability/scroll-interaction.spec.ts b/packages/app/e2e/performance/timeline-stability/scroll-interaction.spec.ts index 7c85528bd5c..6758ef017e2 100644 --- a/packages/app/e2e/performance/timeline-stability/scroll-interaction.spec.ts +++ b/packages/app/e2e/performance/timeline-stability/scroll-interaction.spec.ts @@ -126,13 +126,19 @@ test("keeps moving upward while drag-selecting above the timeline", async ({ pag ) }) }) - const textBox = await text.boundingBox() + const textBox = await text.evaluate((element) => { + const range = document.createRange() + range.selectNodeContents(element) + const rect = range.getClientRects()[0] + return rect ? { x: rect.x, y: rect.y, width: rect.width, height: rect.height } : null + }) const scrollBox = await scroller.boundingBox() expect(textBox).not.toBeNull() expect(scrollBox).not.toBeNull() if (!textBox || !scrollBox) return - await page.mouse.move(textBox.x + textBox.width - 10, textBox.y + textBox.height / 2) + // Start on a text line, not the empty right edge or gap between wrapped lines. + await page.mouse.move(textBox.x + Math.min(20, textBox.width / 2), textBox.y + textBox.height / 2) await page.mouse.down() await page.mouse.move(textBox.x + 20, scrollBox.y - 120, { steps: 30 }) @@ -195,6 +201,45 @@ test("does not pull a keyboard-scrolled user during shell remeasurement", async await reportVisualStability(testInfo, "keyboard-during-resize", trace, anchorPlan(regions)) }) +test("keeps an older answer selected while scrolling within the interaction buffer", async ({ page }) => { + await setupTimeline(page, { + messages: history(80), + viewport: { width: 1400, height: 700 }, + reducedMotion: true, + }) + const scroller = page.locator(".scroll-view__viewport", { has: page.locator("[data-timeline-row]") }) + const answer = page.getByText("History 78.", { exact: false }) + await expect(answer).toBeVisible() + await expect + .poll(() => + answer.evaluate((element) => element.closest('[data-component="markdown"]')?.hasAttribute("data-markdown-ready")), + ) + .toBe(true) + const textBox = await answer.evaluate((element) => { + const range = document.createRange() + range.selectNodeContents(element) + const rect = range.getClientRects()[0] + return { x: rect.x, y: rect.y, width: rect.width, height: rect.height } + }) + const scrollBox = await scroller.boundingBox() + expect(scrollBox).not.toBeNull() + if (!scrollBox) return + await page.mouse.move(textBox.x + Math.min(180, textBox.width - 2), textBox.y + textBox.height / 2) + await page.mouse.down() + await page.mouse.move(textBox.x + 2, textBox.y + textBox.height / 2, { steps: 30 }) + await page.mouse.up() + await expect.poll(() => page.evaluate(() => window.getSelection()?.toString())).toContain("History 78.") + await page.mouse.move(scrollBox.x + scrollBox.width / 2, scrollBox.y + scrollBox.height / 2) + await page.mouse.wheel(0, -450) + await expect + .poll(() => scroller.evaluate((element) => element.scrollHeight - element.clientHeight - element.scrollTop)) + .toBeGreaterThan(400) + await expect(answer).toHaveCount(1) + await expect.poll(() => page.evaluate(() => window.getSelection()?.toString())).toContain("History 78.") + await page.getByRole("heading", { name: "Timeline visual stability" }).click() + await expect.poll(() => page.evaluate(() => window.getSelection()?.isCollapsed)).toBe(true) +}) + test("tracks keyboard scrolling from a focused timeline descendant", async ({ page }, testInfo) => { const shellID = "prt_descendant_keyboard_01_shell" const timeline = await setupTimeline(page, { @@ -259,12 +304,16 @@ test("does not claim keyboard scrolling owned by a nested scrollable", async ({ const before = await scroller.evaluate((element) => element.scrollTop) const nestedBefore = await nested.evaluate((element) => element.scrollTop) await nested.press("PageUp") - await page.waitForTimeout(300) + await expect.poll(() => nested.evaluate((element) => element.scrollTop)).toBeLessThan(nestedBefore) expect(await scroller.evaluate((element) => element.scrollTop)).toBe(before) - expect(await nested.evaluate((element) => element.scrollTop)).toBeLessThan(nestedBefore) - await nested.evaluate((element) => (element.scrollTop = 0)) - await scroller.evaluate((element) => (element.scrollTop = Math.min(300, element.scrollHeight - element.clientHeight))) + await nested.evaluate((element) => element.scrollTo({ top: 0, behavior: "instant" })) + await expect.poll(() => nested.evaluate((element) => element.scrollTop)).toBe(0) + await scroller.evaluate((element) => { + element.dispatchEvent(new WheelEvent("wheel", { bubbles: true, cancelable: true, deltaY: -1 })) + element.scrollTo({ top: Math.min(300, element.scrollHeight - element.clientHeight), behavior: "instant" }) + }) + await expect.poll(() => scroller.evaluate((element) => element.scrollTop)).toBeLessThan(500) const boundaryBefore = await scroller.evaluate((element) => element.scrollTop) expect(boundaryBefore).toBeGreaterThan(0) await nested.press("PageUp") diff --git a/packages/app/e2e/performance/timeline/session-tab-switch-benchmark.spec.ts b/packages/app/e2e/performance/timeline/session-tab-switch-benchmark.spec.ts index eff2c4a1d96..b06031eb218 100644 --- a/packages/app/e2e/performance/timeline/session-tab-switch-benchmark.spec.ts +++ b/packages/app/e2e/performance/timeline/session-tab-switch-benchmark.spec.ts @@ -11,115 +11,73 @@ import { } from "./timeline-test-helpers" import { measureSessionSwitch, waitForStableTimeline } from "./session-tab-switch-probe" -type Result = Awaited> +const scenarios = [ + { cached: false, review: false, resized: false }, + { cached: false, review: true, resized: false }, + { cached: true, review: false, resized: false }, + { cached: true, review: true, resized: false }, + { cached: true, review: true, resized: true }, +] -benchmark( - "benchmarks 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-${reviewPane}-${mode}-${run}`, - (page) => trial(page, mode, reviewPane), - testInfo, - ), - ) +scenarios.forEach((scenario) => { + const name = `tab switch: ${scenario.cached ? "cached" : "unmounted"}, review ${scenario.review ? "open" : "closed"}${scenario.resized ? ", resized" : ""}` + benchmark(name, async ({ browser, report }, testInfo) => { + const result = await withBenchmarkPage( + browser, + name, + async (page) => { + await mockStressTimeline(page, { vcsDiff: createReviewDiffs() }) + await installTimelineSettings(page) + await installStressSessionTabs(page) + await page.goto(stressSessionHref(fixture.sourceID)) + await expectSessionTitle(page, fixture.expected.sourceTitle) + await waitForStableTimeline(page, fixture.expected.sourceMessageIDs.at(-1)!) + if (scenario.review && !scenario.resized) await openReviewPane(page) + if (scenario.cached) { + await switchSession(page, fixture.targetID, fixture.expected.targetTitle) + const answer = page.locator(`[data-timeline-part-id="${fixture.expected.targetPartIDs.at(-1)}"]`) + await expect(answer.locator('[data-component="markdown"]')).toHaveAttribute("data-markdown-ready", "") + await expect + .poll(() => + answer.evaluate((element) => element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true })), + ) + .toBe(true) + await waitForStableTimeline(page, fixture.expected.targetMessageIDs.at(-1)!) + await switchSession(page, fixture.sourceID, fixture.expected.sourceTitle) } - } - } - report({ results, summary: summarizeReviewPane(results) }, { runs, reviewDiffs: createReviewDiffs().length }) - }, -) + if (scenario.resized) await openReviewPane(page) + await waitForStableTimeline(page, fixture.expected.sourceMessageIDs.at(-1)!) -async function trial(page: Page, mode: "cold" | "hot", reviewPane: "closed" | "open") { - const reviewDiffs = createReviewDiffs() - await mockStressTimeline(page, { vcsDiff: reviewDiffs }) - await installTimelineSettings(page) - await installStressSessionTabs(page) - if (mode === "hot") { - await page.goto(stressSessionHref(fixture.targetID)) - await expectSessionTitle(page, fixture.expected.targetTitle) - await waitForStableTimeline(page, fixture.expected.targetMessageIDs.at(-1)!) - await switchSession(page, fixture.sourceID, fixture.expected.sourceTitle) - } else { - await page.goto(stressSessionHref(fixture.sourceID)) - await expectSessionTitle(page, fixture.expected.sourceTitle) - } - await waitForStableTimeline(page, fixture.expected.sourceMessageIDs.at(-1)!) - if (reviewPane === "open") { - await openReviewPane(page) - await waitForStableTimeline(page, fixture.expected.sourceMessageIDs.at(-1)!) - } - - const destinationIDs = fixture.messages[fixture.targetID].map((message) => message.id) - const sourceIDs = fixture.messages[fixture.sourceID].map((message) => message.id) - const lastID = fixture.expected.targetMessageIDs.at(-1)! - const href = stressSessionHref(fixture.targetID) - const result = await measureSessionSwitch(page, { - destinationIDs, - sourceIDs, - lastID, - href, - switch: () => switchSession(page, fixture.targetID, fixture.expected.targetTitle), - }) - return result -} - -function summarize(results: Record<"cold" | "hot", Result[]>) { - const stats = (values: (number | null)[]) => { - const sorted = values.filter((value): value is number => value !== null).sort((a, b) => a - b) - return { - min: sorted[0] ?? null, - median: sorted[Math.floor(sorted.length / 2)] ?? null, - max: sorted.at(-1) ?? null, - missing: values.length - sorted.length, - } - } - return Object.fromEntries( - Object.entries(results).map(([mode, values]) => [ - mode, - { - firstDestinationObservedMs: stats(values.map((value) => value.firstDestinationObservedMs)), - firstCorrectObservedMs: stats(values.map((value) => value.firstCorrectObservedMs)), - stableObservedMs: stats(values.map((value) => value.stableObservedMs)), + return measureSessionSwitch(page, { + destinationIDs: fixture.messages[fixture.targetID].map((message) => message.id), + sourceIDs: fixture.messages[fixture.sourceID].map((message) => message.id), + lastID: fixture.expected.targetMessageIDs.at(-1)!, + requiredPartID: fixture.expected.targetPartIDs.at(-1), + href: stressSessionHref(fixture.targetID), + switch: () => switchSession(page, fixture.targetID, fixture.expected.targetTitle), + }) }, - ]), - ) -} - -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[]>), - ]), - ) -} + testInfo, + ) + expect(result.unknownSamples).toBe(0) + expect(result.wrongDestinationSamples).toBe(0) + if (scenario.cached) expect(result.blankSamples).toBe(0) + report(result, { ...scenario, inputEvent: "mousedown", requireReadyAnswer: true }) + }) +}) 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() + const tab = page.locator(`[data-slot="titlebar-tabs"] a[href="${stressSessionHref(sessionID)}"]`) + await expect(tab).toHaveCount(1) 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() + await expect(page.locator("#review-panel")).toBeVisible() await page.waitForFunction(() => { - const panel = document.querySelector("#review-panel") - const text = panel?.textContent ?? "" + const text = document.querySelector("#review-panel")?.textContent ?? "" return text.includes("generated-000.ts") && text.includes("+3") }) } diff --git a/packages/app/e2e/performance/timeline/session-tab-switch-metrics.ts b/packages/app/e2e/performance/timeline/session-tab-switch-metrics.ts index 5298b9b0b7d..4d2d6508f3e 100644 --- a/packages/app/e2e/performance/timeline/session-tab-switch-metrics.ts +++ b/packages/app/e2e/performance/timeline/session-tab-switch-metrics.ts @@ -20,9 +20,10 @@ export function classifySessionSwitch(samples: SessionSwitchSample[]) { const firstCorrect = samples.findIndex(isCorrectDestination) const stable = samples.findIndex((_, index) => isStableSessionSwitch(samples.slice(index, index + 3))) return { + samples, firstDestinationObservedMs: samples[firstDestination]?.observedAtMs ?? null, firstCorrectObservedMs: samples[firstCorrect]?.observedAtMs ?? null, - stableObservedMs: samples[stable + 2]?.observedAtMs ?? null, + stableObservedMs: stable < 0 ? null : samples[stable + 2].observedAtMs, wrongDestinationSamples: samples .slice(firstDestination) .filter((sample) => sample.destination.length > 0 && !sample.last).length, diff --git a/packages/app/e2e/performance/timeline/session-tab-switch-probe.spec.ts b/packages/app/e2e/performance/timeline/session-tab-switch-probe.spec.ts new file mode 100644 index 00000000000..2f2f8666f65 --- /dev/null +++ b/packages/app/e2e/performance/timeline/session-tab-switch-probe.spec.ts @@ -0,0 +1,67 @@ +import { benchmark, expect } from "../benchmark" +import { measureSessionSwitch } from "./session-tab-switch-probe" +import type { SessionSwitchSample } from "./session-tab-switch-metrics" + +benchmark("starts at mousedown and excludes hidden or unfinished destination content", async ({ page, report }) => { + await page.setContent(` + Destination +
+
+
Destination answer
+
+
+ `) + await page.evaluate(() => { + document.querySelector("a")!.addEventListener("mousedown", () => { + const row = document.querySelector("[data-message-id]")! + row.dataset.messageId = "destination" + row.style.visibility = "hidden" + }) + }) + const result = await measureSessionSwitch(page, { + destinationIDs: ["destination"], + sourceIDs: ["source"], + lastID: "destination", + requiredPartID: "answer", + requireBottomAnchor: false, + href: "/session/destination", + switch: async () => { + // No click is dispatched: the probe must observe the event that activates tabs. + await page.getByRole("link", { name: "Destination" }).dispatchEvent("mousedown", { button: 0 }) + await page.waitForFunction(() => { + const host = window as Window & { __sessionSwitchProbe?: { samples: SessionSwitchSample[] } } + return host.__sessionSwitchProbe?.samples.some((sample) => !sample.hasVisibleRows) + }) + await page.locator("[data-message-id]").evaluate((row) => row.style.removeProperty("visibility")) + await page.waitForFunction(() => { + const host = window as Window & { __sessionSwitchProbe?: { samples: SessionSwitchSample[] } } + return host.__sessionSwitchProbe?.samples.some( + (sample) => sample.destination.length > 0 && sample.requiredPartVisible === false, + ) + }) + const beforeClip = await page.evaluate(() => { + const row = document.querySelector("[data-timeline-key]")! + row.style.cssText = "height:10px;position:relative;overflow:clip" + const answer = row.querySelector("[data-timeline-part-id]")! + answer.style.cssText = "position:absolute;top:30px;width:150px" + answer.querySelector('[data-component="markdown"]')!.setAttribute("data-markdown-ready", "") + return ( + (window as Window & { __sessionSwitchProbe?: { samples: SessionSwitchSample[] } }).__sessionSwitchProbe + ?.samples.length ?? 0 + ) + }) + await page.waitForFunction((count) => { + const host = window as Window & { __sessionSwitchProbe?: { samples: SessionSwitchSample[] } } + return host.__sessionSwitchProbe?.samples.slice(count).some((sample) => sample.requiredPartVisible === false) + }, beforeClip) + await page.locator("[data-timeline-key]").evaluate((row) => { + row.style.height = "100px" + }) + }, + }) + expect(result.blankSamples).toBeGreaterThan(0) + expect(result.firstCorrectObservedMs).not.toBeNull() + expect(result.stableObservedMs).not.toBeNull() + expect(result.firstCorrectObservedMs).toBeGreaterThan(result.firstDestinationObservedMs!) + report(result) +}) diff --git a/packages/app/e2e/performance/timeline/session-tab-switch-probe.ts b/packages/app/e2e/performance/timeline/session-tab-switch-probe.ts index 5dcfc3ac6b3..24a0ac7a359 100644 --- a/packages/app/e2e/performance/timeline/session-tab-switch-probe.ts +++ b/packages/app/e2e/performance/timeline/session-tab-switch-probe.ts @@ -25,7 +25,7 @@ async function installSessionSwitchProbe( let running = true const reviewLevels: Record = { panel: "#review-panel", - tabs: '#review-panel [data-component="tabs"]', + 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"]', @@ -37,7 +37,6 @@ async function installSessionSwitchProbe( if (!running || started === undefined) return setTimeout(() => { if (!running || started === undefined) return - const observedAtMs = performance.now() - started const reviewPanel = document.querySelector("#review-panel") const reviewFile = reviewPanel?.querySelector('[data-component="file"][data-mode="diff"]') const initialReviewFile = initialReviewNodes.file @@ -63,26 +62,30 @@ async function installSessionSwitchProbe( ) if (root) { const view = root.getBoundingClientRect() - const visible = [...root.querySelectorAll("[data-message-id]")] - .filter((element) => { - const rect = element.getBoundingClientRect() - return rect.bottom > view.top && rect.top < view.bottom - }) - .map((element) => element.dataset.messageId!) - const hasVisibleRows = [...root.querySelectorAll("[data-timeline-key]")].some((element) => { + const inViewport = (element: HTMLElement) => { + if (!element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true })) return false const rect = element.getBoundingClientRect() - return rect.bottom > view.top && rect.top < view.bottom - }) + const clip = element.closest("[data-timeline-key]")?.getBoundingClientRect() ?? view + return ( + Math.min(rect.bottom, clip.bottom, view.bottom) > Math.max(rect.top, clip.top, view.top) && + Math.min(rect.right, clip.right, view.right) > Math.max(rect.left, clip.left, view.left) + ) + } + const visible = [...root.querySelectorAll("[data-message-id]")] + .filter(inViewport) + .map((element) => element.dataset.messageId!) + const hasVisibleRows = [...root.querySelectorAll("[data-timeline-key]")].some(inViewport) const requiredPartVisible = requiredPartID ? [...root.querySelectorAll("[data-timeline-part-id]")].some((element) => { if (element.dataset.timelinePartId !== requiredPartID) return false - const rect = element.getBoundingClientRect() - return rect.width > 0 && rect.height > 0 && rect.bottom > view.top && rect.top < view.bottom + if (!element.textContent?.trim()) return false + if (element.querySelector('[data-component="markdown"]:not([data-markdown-ready])')) return false + return inViewport(element) }) : undefined const spacer = root.querySelector('[data-timeline-row="bottom-spacer"]')?.getBoundingClientRect() samples.push({ - observedAtMs, + observedAtMs: performance.now() - started, destination: visible.filter((id) => destination.has(id)), source: visible.filter((id) => source.has(id)), hasVisibleRows, @@ -94,7 +97,7 @@ async function installSessionSwitchProbe( }) } else { samples.push({ - observedAtMs, + observedAtMs: performance.now() - started, destination: [], source: [], hasVisibleRows: false, @@ -107,23 +110,25 @@ async function installSessionSwitchProbe( requestAnimationFrame(sample) }, 0) } - document.addEventListener( - "click", - (event) => { - 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 }, - ) + const start = (event: MouseEvent) => { + if (started !== undefined || event.button !== 0) return + 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) + } + // Tabs activate on mousedown; click alone misses the synchronous navigation work. + document.addEventListener("mousedown", start, true) + document.addEventListener("click", start, true) ;(window as Window & { __sessionSwitchProbe?: SessionSwitchProbe }).__sessionSwitchProbe = { samples, stop: () => { running = false + document.removeEventListener("mousedown", start, true) + document.removeEventListener("click", start, true) }, } }, input) diff --git a/packages/app/e2e/performance/unit/session-tab-switch-metrics.test.ts b/packages/app/e2e/performance/unit/session-tab-switch-metrics.test.ts index 4b824d9dfb0..481ee177a25 100644 --- a/packages/app/e2e/performance/unit/session-tab-switch-metrics.test.ts +++ b/packages/app/e2e/performance/unit/session-tab-switch-metrics.test.ts @@ -53,6 +53,15 @@ test("reports missing correctness without throwing", () => { expect(result.stableObservedMs).toBeNull() }) +test("does not report stability for only two correct samples", () => { + const result = classifySessionSwitch([ + { observedAtMs: 16, destination: ["destination"], source: [], hasVisibleRows: true, last: true, bottomErrorPx: 0 }, + { observedAtMs: 32, destination: ["destination"], source: [], hasVisibleRows: true, last: true, bottomErrorPx: 0 }, + ]) + expect(result.firstCorrectObservedMs).toBe(16) + expect(result.stableObservedMs).toBeNull() +}) + test("requires an explicitly tracked part to be visible", () => { const result = classifySessionSwitch([ { diff --git a/packages/app/src/session/screen-layout.ts b/packages/app/src/session/screen-layout.ts index 0e7e0f36de6..2709a070608 100644 --- a/packages/app/src/session/screen-layout.ts +++ b/packages/app/src/session/screen-layout.ts @@ -8,7 +8,7 @@ import type { SessionModel } from "./model" import { sessionPanelLayout } from "./session-panel-layout" import { clampSessionPanelWidth, sessionPanelWidthMax } from "./session-panel-width" -export function createSessionScreenLayout(session: SessionModel, serverScope: string) { +export function createSessionScreenLayout(session: SessionModel) { const layout = useLayout() const settings = useSettings() const size = createSizing() @@ -92,7 +92,6 @@ export function createSessionScreenLayout(session: SessionModel, serverScope: st centered: createMemo(() => session.isDesktop()), files: { open: fileTreeOpen }, panel: { - key: createMemo(() => (session.identity.params.id ? `${serverScope}\0${session.identity.params.id}` : undefined)), max: panelMax, ref: (element: HTMLDivElement) => { row = element diff --git a/packages/app/src/session/screen.tsx b/packages/app/src/session/screen.tsx index ece28d4f32b..b893920c967 100644 --- a/packages/app/src/session/screen.tsx +++ b/packages/app/src/session/screen.tsx @@ -4,7 +4,6 @@ import createPresence from "solid-presence" import { ResizeHandle } from "@opencode-ai/ui/resize-handle" import { SessionHeader } from "@/session/header/session-header" import { useLayout } from "@/shell/state/layout" -import { useServerSDK } from "@/runtime/server/client" import { useSettings } from "@/settings/model" import { MessageTimeline } from "@/session/timeline/message-timeline" import type { SessionModel } from "@/session/model" @@ -23,10 +22,9 @@ import { SessionIdentityHeader } from "./session-identity-header" export function SessionScreen(props: { session: SessionModel }) { const session = props.session const layout = useLayout() - const serverSDK = useServerSDK() const settings = useSettings() const isDesktop = session.isDesktop - const screen = createSessionScreenLayout(session, serverSDK.scope) + const screen = createSessionScreenLayout(session) const timeline = createSessionTimelineInteraction(session) const messagesReady = timeline.ready const [store, setStore] = createStore({ @@ -177,12 +175,10 @@ export function SessionScreen(props: { session: SessionModel }) { width: screen.panel.width(), }} > - - {(_) => ( - - {sessionPanelContent()} - - )} + + + {sessionPanelContent()} + diff --git a/packages/app/src/session/timeline/virtualizer.tsx b/packages/app/src/session/timeline/virtualizer.tsx index 566f67f5b54..5fff7876e26 100644 --- a/packages/app/src/session/timeline/virtualizer.tsx +++ b/packages/app/src/session/timeline/virtualizer.tsx @@ -1,8 +1,15 @@ -import { createVirtualizer, defaultRangeExtractor, elementScroll, type VirtualItem } from "@tanstack/solid-virtual" +import { + createVirtualizer, + defaultRangeExtractor, + elementScroll, + type Range, + type VirtualItem, +} from "@tanstack/solid-virtual" import { isScrollKeyTarget, scrollKey, scrollKeyOwner, ScrollView } from "@opencode-ai/ui/scroll-view" import { TimelineRow } from "@opencode-ai/session-ui/timeline/projection" import { useLanguage } from "@/runtime/i18n/language" import { + batch, createEffect, createMemo, createSignal, @@ -67,11 +74,12 @@ export function createTimelineVirtualizer(input: Input) { const coldBottomMount = !initialMeasurements?.length && input.pinned() const [listRoot, setListRoot] = createSignal() const [toolOpen, setToolOpen] = createStore>(cached?.toolOpen ?? {}) - const [renderOverscan, setRenderOverscan] = createSignal(initialMeasurements?.length || coldBottomMount ? 6 : 20) + const [overscan, setOverscan] = createSignal(2) const rows = input.projection.rows const rowByKey = input.projection.rowByKey const knownKeys = new Set(rows().map(TimelineRow.key)) const addedKeys = new Set() + const measuredElements = new WeakSet() let touchStart: number | undefined let pointerHeld = false let maxScroll = 0 @@ -91,14 +99,19 @@ export function createTimelineVirtualizer(input: Input) { initialOffset: () => (input.pinned() ? Number.MAX_SAFE_INTEGER : 0), initialMeasurementsCache: initialMeasurements, estimateSize: () => fallbackItemSize, - // Do not replace this with TanStack's default measurer: without a ResizeObserver entry, - // it returns the cached height instead of reading the element (TanStack/virtual#1183). - // Restored sessions, deferred tools, and rewrapped content can then keep stale heights; - // our fixed-height, overflow-clipped rows will hide their content. Keep observer entries - // on the cheap precomputed path, but make explicit measurements read the real height. - measureElement: (element, entry) => { + // A newly observed element gets a real ResizeObserver box before paint. Reuse + // its snapshot on attachment, but later explicit measurements must read layout + // so deferred/rewrapped content cannot keep stale, clipped heights (TanStack/virtual#1183). + measureElement: (element, entry, instance) => { + const initial = !measuredElements.has(element) + measuredElements.add(element) const box = entry?.borderBoxSize[0] - return box ? Math.round(box.blockSize) : element.offsetHeight + if (box) return Math.round(box.blockSize) + if (initial) { + const size = instance.itemSizeCache.get(instance.options.getItemKey(instance.indexFromElement(element))) + if (size !== undefined) return size + } + return element.offsetHeight }, scrollToFn: (offset, options, instance) => { if (virtualContent) virtualContent.style.height = `${instance.getTotalSize()}px` @@ -130,41 +143,59 @@ export function createTimelineVirtualizer(input: Input) { return input.showHeader() ? 64 : 0 }, paddingEnd: 64, - rangeExtractor: (range) => { + get rangeExtractor() { const id = input.projection.activeMessageID() const active = id ? (input.projection.messageLastRowIndex().get(id) ?? -1) : -1 - const indexes = defaultRangeExtractor({ ...range, overscan: renderOverscan() }) - return filterVirtualIndexes( - [...new Set([...indexes, ...(active < 0 ? [] : [active])])].sort((a, b) => a - b), - range.count, - ) + const buffer = overscan() + return (range: Range) => { + const indexes = defaultRangeExtractor({ ...range, overscan: buffer }) + return filterVirtualIndexes( + [...new Set([...indexes, ...(active < 0 ? [] : [active])])].sort((a, b) => a - b), + range.count, + ) + } }, }) const resizeItem = virtualizer.resizeItem - let resizeAnchorScheduled = false - // Rows measure asynchronously, so the last row can still hold its estimate when TanStack - // reconciles the end. Coalesce one correction per measurement batch, before paint. - const anchorResizedBottom = () => { - if (resizeAnchorScheduled) return - resizeAnchorScheduled = true + const pendingSizes = new Map() + let resizeScheduled = false + // Read the whole measurement delivery before committing reactive row sizes. + // Otherwise each row can render and force layout before the next is measured. + virtualizer.resizeItem = (index, size) => { + const row = rows()[index] + if (!row) return + const key = TimelineRow.key(row) + if (virtualizer.itemSizeCache.get(key) === size) { + pendingSizes.delete(index) + return + } + pendingSizes.set(index, { key, size }) + if (resizeScheduled) return + resizeScheduled = true queueMicrotask(() => { - resizeAnchorScheduled = false + resizeScheduled = false + if (!pendingSizes.size) return + const sizes = [...pendingSizes] + pendingSizes.clear() + batch(() => { + sizes.forEach(([index, value]) => { + const row = rows()[index] + if (row && TimelineRow.key(row) === value.key) resizeItem(index, value.size) + }) + }) if (!input.pinned()) return - virtualizer.scrollToEnd() + const root = listRoot() + // Reopening a settled scroll-to-end operation can fight subsequent keyboard scrolling. + if (root && Math.abs(root.scrollHeight - root.clientHeight - root.scrollTop) > endEpsilon) + virtualizer.scrollToEnd() }) } - virtualizer.resizeItem = (index, size) => { - resizeItem(index, size) - if (listRoot() && input.pinned()) anchorResizedBottom() - } + onCleanup(() => pendingSizes.clear()) virtualizer.shouldAdjustScrollPositionOnItemSizeChange = (item, _delta, instance) => { // Prepended rows can resize more than once as deferred content mounts. Keep // compensating while they remain entirely above the visible content fold. if (addedKeys.has(String(item.key))) - return ( - item.end <= - (instance.scrollOffset ?? 0) + instance.scrollAdjustments + instance.options.scrollMargin - ) + return item.end <= (instance.scrollOffset ?? 0) + instance.scrollAdjustments + instance.options.scrollMargin const first = instance.range?.startIndex return first !== undefined && item.index < first } @@ -185,50 +216,41 @@ export function createTimelineVirtualizer(input: Input) { }) }) + let settleFrame: number | undefined let overscanFrame: number | undefined + let overscanTimer: number | undefined + const expandOverscan = () => { + overscanFrame = requestAnimationFrame(() => { + overscanFrame = undefined + // Let the visible rows paint before building the normal interaction buffer. + overscanTimer = window.setTimeout(() => { + overscanTimer = undefined + setOverscan(20) + }, 0) + }) + } const pendingMeasurements = () => virtualizer.getVirtualItems().some((item) => !virtualizer.itemSizeCache.has(item.key)) const settleColdBottom = () => { if (input.pinned()) virtualizer.scrollToEnd() if (virtualContent?.querySelector(pendingMarkdown) || pendingMeasurements()) { - overscanFrame = requestAnimationFrame(settleColdBottom) + settleFrame = requestAnimationFrame(settleColdBottom) return } - overscanFrame = requestAnimationFrame(() => { + settleFrame = requestAnimationFrame(() => { if (input.pinned()) virtualizer.scrollToEnd() if (virtualContent?.querySelector(pendingMarkdown) || pendingMeasurements()) { settleColdBottom() return } - overscanFrame = undefined - const content = virtualContent - if (!content) return - if (window.matchMedia("(prefers-reduced-motion: reduce)").matches) { - content.style.removeProperty("visibility") - return - } - const animation = ["animate-in", "fade-in", "duration-150"] - const clearAnimation = (event: AnimationEvent) => { - if (event.target !== content) return - content.removeEventListener("animationend", clearAnimation) - content.removeEventListener("animationcancel", clearAnimation) - content.classList.remove(...animation) - } - content.addEventListener("animationend", clearAnimation) - content.addEventListener("animationcancel", clearAnimation) - content.classList.add(...animation) - content.style.removeProperty("visibility") + settleFrame = undefined + virtualContent?.style.removeProperty("visibility") + expandOverscan() }) } onMount(() => { - overscanFrame = requestAnimationFrame(() => { - if (renderOverscan() < 20) setRenderOverscan(20) - if (!coldBottomMount) { - overscanFrame = undefined - return - } - settleColdBottom() - }) + if (coldBottomMount) settleFrame = requestAnimationFrame(settleColdBottom) + if (!coldBottomMount) expandOverscan() }) let measuredSessionKey = input.sessionKey() @@ -255,11 +277,13 @@ export function createTimelineVirtualizer(input: Input) { const handleListWheel = (event: WheelEvent & { currentTarget: HTMLDivElement }) => { input.onUserScroll(event.target) if (event.deltaY < 0) input.onUnpin() + setOverscan(20) } const handleListTouchStart = (event: TouchEvent) => { input.onUserScroll(event.target) touchStart = event.touches[0]?.clientY + setOverscan(20) } const handleListTouchMove = (event: TouchEvent & { currentTarget: HTMLDivElement }) => { @@ -276,14 +300,19 @@ export function createTimelineVirtualizer(input: Input) { const handleListPointerDown = (event: PointerEvent & { currentTarget: HTMLDivElement }) => { input.onUserScroll(event.target) pointerHeld = true - const release = () => { - pointerHeld = false - window.removeEventListener("pointerup", release) - window.removeEventListener("pointercancel", release) - } - window.addEventListener("pointerup", release) - window.addEventListener("pointercancel", release) + setOverscan(20) } + const releasePointer = () => { + pointerHeld = false + } + onMount(() => { + window.addEventListener("pointerup", releasePointer) + window.addEventListener("pointercancel", releasePointer) + }) + onCleanup(() => { + window.removeEventListener("pointerup", releasePointer) + window.removeEventListener("pointercancel", releasePointer) + }) const handleListKeyDown = (event: KeyboardEvent & { currentTarget: HTMLDivElement }) => { const key = scrollKey(event) @@ -292,6 +321,7 @@ export function createTimelineVirtualizer(input: Input) { if (scrollKeyOwner(event.currentTarget, event.target, key) !== event.currentTarget) return input.onUserScroll(event.currentTarget) if (upwardKeys.has(key)) input.onUnpin() + setOverscan(20) } // Following resumes by arriving at the end, either by scrolling there or by content shrinking @@ -414,7 +444,6 @@ export function createTimelineVirtualizer(input: Input) { {props.header}
{ virtualContent = element input.setContentRef(element) @@ -446,7 +475,9 @@ export function createTimelineVirtualizer(input: Input) { cache.delete(ownerSessionKey) cache.set(ownerSessionKey, { measurements: virtualizer.takeSnapshot(), toolOpen: { ...toolOpen } }) while (cache.size > 16) cache.delete(cache.keys().next().value!) + if (settleFrame !== undefined) cancelAnimationFrame(settleFrame) if (overscanFrame !== undefined) cancelAnimationFrame(overscanFrame) + if (overscanTimer !== undefined) window.clearTimeout(overscanTimer) input.setScrollRef(undefined) input.setRevealMessage?.(() => {}) input.setScrollToEnd?.(() => {}) diff --git a/packages/session-ui/component-tests/markdown.fixture.tsx b/packages/session-ui/component-tests/markdown.fixture.tsx new file mode 100644 index 00000000000..c31096f901b --- /dev/null +++ b/packages/session-ui/component-tests/markdown.fixture.tsx @@ -0,0 +1,36 @@ +import { createSignal, Show } from "solid-js" +import { render } from "solid-js/web" +import { Markdown } from "../src/components/markdown" +import { preloadMarkdown } from "../src/components/markdown-cache" + +export async function mountMarkdown(options: { text: string; streaming?: boolean; cached?: boolean }) { + if (options.cached) await preloadMarkdown(options.text, "markdown-test") + const host = document.createElement("div") + host.dataset.testid = "markdown-fixture" + document.body.appendChild(host) + render(() => { + const [text, setText] = createSignal(options.text) + const [streaming, setStreaming] = createSignal(options.streaming ?? false) + const [visible, setVisible] = createSignal(true) + return ( + <> +