From 91fd33779e7b8f0c2db35aec50656cc437f3791a Mon Sep 17 00:00:00 2001 From: LukeParkerDev <10430890+Hona@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:04:08 +1000 Subject: [PATCH 1/3] fix(app): load capped review patches --- .../review-terminal-stacked.spec.ts | 88 +++++++++++++++---- packages/app/src/pages/session.tsx | 47 +++++++++- .../session/v2/review-diff-kinds.test.ts | 42 ++++++++- .../src/pages/session/v2/review-diff-kinds.ts | 18 ++++ .../src/pages/session/v2/review-panel-v2.tsx | 30 ++++++- 5 files changed, 203 insertions(+), 22 deletions(-) diff --git a/packages/app/e2e/regression/review-terminal-stacked.spec.ts b/packages/app/e2e/regression/review-terminal-stacked.spec.ts index 1ba8ed9474cf..71c1c28ef300 100644 --- a/packages/app/e2e/regression/review-terminal-stacked.spec.ts +++ b/packages/app/e2e/regression/review-terminal-stacked.spec.ts @@ -9,12 +9,19 @@ const title = "Review terminal stacked" const branchDiffs = [ fileDiff(".github/actions/setup-bun/action.yml", 7), ...Array.from({ length: 2_739 }, (_, index) => - fileDiff(`src/branch/generated-${String(index).padStart(4, "0")}.ts`, 100), + fileDiff( + `src/branch/d${String(Math.floor(index / 100)).padStart(5, "0")}/generated-${String(index).padStart(4, "0")}.ts`, + 100, + false, + ), ), ] test("keeps the review tree and terminal sized when both panels are open", async ({ page }) => { test.setTimeout(120_000) + const events: Array<{ directory: string; payload: Record }> = [] + let detailVersion = 1 + let detailFailures = 1 await page.setViewportSize({ width: 1400, height: 900 }) await mockOpenCodeServer(page, { directory, @@ -48,7 +55,10 @@ test("keeps the review tree and terminal sized when both panels are open", async time: { created: 1700000000000, updated: 1700000000000 }, }, ], + sessionStatus: { [sessionID]: { type: "idle" } }, pageMessages: () => ({ items: [] }), + events: () => events.splice(0, 1), + eventRetry: 16, }) await page.route(/\/vcs(?:\?.*)?$/, (route) => route.fulfill({ @@ -57,17 +67,25 @@ test("keeps the review tree and terminal sized when both panels are open", async body: JSON.stringify({ branch: "review-pane-performance", default_branch: "dev" }), }), ) - await page.route("**/vcs/diff**", (route) => - route.fulfill({ + await page.route("**/vcs/diff**", (route) => { + const url = new URL(route.request().url()) + const scope = url.searchParams.get("directory")?.replaceAll("\\", "/") + const detail = scope?.endsWith("/src/branch/d00027") + if (detail && detailFailures-- > 0) return route.fulfill({ status: 500, body: "retry detail" }) + return route.fulfill({ status: 200, contentType: "application/json", body: JSON.stringify( - new URL(route.request().url()).searchParams.get("mode") === "branch" - ? branchDiffs + url.searchParams.get("mode") === "branch" + ? detail + ? branchDiffs + .filter((diff) => diff.file.startsWith("src/branch/d00027/")) + .map((diff) => fileDiff(diff.file, diff.additions, true, detailVersion)) + : branchDiffs : Array.from({ length: 7 }, (_, index) => fileDiff(`src/git-${index}.ts`, 1)), ), - }), - ) + }) + }) await page.route("**/pty", (route) => route.fulfill({ status: 200, @@ -96,7 +114,7 @@ test("keeps the review tree and terminal sized when both panels are open", async await expect(page.getByRole("tab", { name: "Review 2740" })).toBeVisible() await page.keyboard.press("Control+Backquote") await expect(page.locator("#terminal-panel")).toBeVisible() - await expectTree(page, 2_745, "action.yml") + await expectTree(page, 2_773, "action.yml") await expectStackGeometry(page) const treeViewport = page.locator('#review-panel [data-slot="session-review-v2-sidebar-tree"] .scroll-view__viewport') @@ -113,41 +131,66 @@ test("keeps the review tree and terminal sized when both panels are open", async }) expect(bottomGap).toBeGreaterThanOrEqual(0) expect(bottomGap).toBeLessThanOrEqual(16) + const lazyDiff = page.waitForRequest((request) => { + const url = new URL(request.url()) + return ( + url.pathname === "/vcs/diff" && + url.searchParams.get("directory")?.replaceAll("\\", "/").endsWith("/src/branch/d00027") === true + ) + }) + await lastFile.click() + await lazyDiff + const preview = page.locator('[data-slot="session-review-v2-diff-scroll"]') + await expect(preview).toContainText("after-1") + detailVersion = 2 + events.push(statusEvent("busy")) + await expect(page.getByRole("button", { name: "Stop" })).toBeVisible() + const refreshedDiff = page.waitForRequest((request) => { + const url = new URL(request.url()) + return ( + url.pathname === "/vcs/diff" && + url.searchParams.get("directory")?.replaceAll("\\", "/").endsWith("/src/branch/d00027") === true + ) + }) + events.push(statusEvent("idle")) + await refreshedDiff + await expect(preview).toContainText("after-2") await selectMode(page, "Branch changes", "Git changes") await expectTree(page, 8, "git-0.ts") + await page.getByRole("button", { name: "git-0.ts" }).click() await selectMode(page, "Git changes", "Branch changes") - await expectTree(page, 2_745, "action.yml") + await expectTree(page, 2_773, "action.yml") const filter = page.getByRole("searchbox", { name: "Filter files" }) await filter.fill("generated-2738") await expectTree(page, 1, "generated-2738.ts") await filter.fill("") - await expectTree(page, 2_745, "action.yml") + await expectTree(page, 2_773, "action.yml") await page.getByRole("button", { name: "Toggle file tree" }).click() await expect(page.locator('[data-slot="session-review-v2-sidebar"]')).toHaveAttribute("aria-hidden", "true") await expect(page.locator('#review-panel [data-component="file-tree-v2"]')).toHaveCount(1) await page.getByRole("button", { name: "Toggle file tree" }).click() - await expectTree(page, 2_745, "action.yml") + await expectTree(page, 2_773, "action.yml") await page.keyboard.press("Control+Backquote") await expect(page.locator("#terminal-panel")).toHaveCount(0) - await expectTree(page, 2_745, "action.yml") + await expectTree(page, 2_773, "action.yml") await page.keyboard.press("Control+Backquote") await expect(page.locator("#terminal-panel")).toBeVisible() - await expectTree(page, 2_745, "action.yml") + await expectTree(page, 2_773, "action.yml") await page.getByRole("button", { name: "Toggle review" }).click() await expect(page.locator("#review-panel")).toHaveAttribute("aria-hidden", "true") await expect(page.locator('#review-panel [data-component="file-tree-v2"]')).toHaveCount(1) await page.getByRole("button", { name: "Toggle review" }).click() - await expectTree(page, 2_745, "action.yml") + await expectTree(page, 2_773, "action.yml") await page.setViewportSize({ width: 1_000, height: 700 }) - await expectTree(page, 2_745, "action.yml") + await expectTree(page, 2_773, "action.yml") await expectStackGeometry(page) await page.setViewportSize({ width: 1_000, height: 120 }) await page.setViewportSize({ width: 1_400, height: 900 }) - await expectTree(page, 2_745, "action.yml") + await expectTree(page, 2_773, "action.yml") await expectStackGeometry(page) }) @@ -201,12 +244,21 @@ function base64Encode(value: string) { return Buffer.from(value, "utf8").toString("base64").replace(/\+/g, "-").replace(/\//g, "_").replace(/=/g, "") } -function fileDiff(file: string, additions: number) { +function statusEvent(type: "busy" | "idle") { + return { + directory, + payload: { type: "session.status", properties: { sessionID, status: { type } } }, + } +} + +function fileDiff(file: string, additions: number, loaded = true, version = 1) { return { file, additions, deletions: 0, status: "modified", - patch: `diff --git a/${file} b/${file}\n--- a/${file}\n+++ b/${file}\n@@ -1 +1 @@\n-export const value = 'before'\n+export const value = 'after'\n`, + patch: loaded + ? `diff --git a/${file} b/${file}\n--- a/${file}\n+++ b/${file}\n@@ -1 +1 @@\n-export const value = 'before'\n+export const value = 'after-${version}'\n` + : `diff --git a/${file} b/${file}\n--- a/${file}\n+++ b/${file}`, } } diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx index 46bd907167a4..14ebad6fd131 100644 --- a/packages/app/src/pages/session.tsx +++ b/packages/app/src/pages/session.tsx @@ -1,4 +1,4 @@ -import type { Project, UserMessage } from "@opencode-ai/sdk/v2" +import type { Project, UserMessage, VcsFileDiff } from "@opencode-ai/sdk/v2" import { useDialog } from "@opencode-ai/ui/context/dialog" import { createQuery, skipToken, useMutation, useQueryClient } from "@tanstack/solid-query" import { @@ -76,6 +76,7 @@ import { SessionReviewEmptyChangesV2 } from "@opencode-ai/session-ui/v2/session- import { SessionReviewEmptyNoGitV2 } from "@opencode-ai/session-ui/v2/session-review-empty-no-git-v2" import { ReviewPanelV2 } from "@/pages/session/v2/review-panel-v2" import { createReviewPanelV2State } from "@/pages/session/v2/review-panel-v2-state" +import { reviewDiffDirectory, reviewDiffNeedsLoad, reviewRootDirectory } from "@/pages/session/v2/review-diff-kinds" import { TerminalPanel } from "@/pages/session/terminal-panel" import { TerminalPanelV2 } from "@/pages/session/terminal-panel-v2" import { useComposerCommands } from "@/pages/session/use-composer-commands" @@ -651,6 +652,46 @@ export default function Page() { if (store.changes === "git" || store.changes === "branch") return !vcsQuery.isPending return true } + const loadReviewDiff = async (file: string, version?: number): Promise => { + const mode = vcsMode() + if (!mode) return + const root = reviewRootDirectory(sync().project?.worktree ?? sdk().directory) + const directory = reviewDiffDirectory(root, file) + const source = reviewDiffs().find((diff) => diff.file === file) + const valid = (diff: VcsFileDiff | undefined) => { + if (!diff || !source) return + if (diff.additions !== source.additions || diff.deletions !== source.deletions) return + if (reviewDiffNeedsLoad(diff)) return + return diff + } + const request = (scope: string, context?: number) => + queryClient + .fetchQuery({ + queryKey: [serverSDK().scope, ...vcsKey(), mode, "directory", scope, context, version] as const, + staleTime: Number.POSITIVE_INFINITY, + retry: 2, + queryFn: () => + sdk() + .client.vcs.diff({ mode, directory: scope, context }) + .then((result) => result.data ?? []), + }) + .then((diffs) => diffs.find((diff) => diff.file === file)) + + if (directory !== root) { + try { + const scoped = valid(await request(directory)) + if (scoped) return scoped + } catch (error) { + console.debug("[session-review] failed to load scoped vcs diff", { mode, file, directory, error }) + } + } + try { + const bounded = valid(await request(root, 3)) + if (bounded) return bounded + } catch (error) { + console.debug("[session-review] failed to load bounded vcs diff", { mode, file, root, error }) + } + } const newSessionWorktree = createMemo(() => { if (store.newSessionWorktree === "create") return "create" @@ -1182,6 +1223,10 @@ export default function Page() { }, diffs: reviewDiffs, diffsReady: reviewReady, + get diffVersion() { + return vcsQuery.dataUpdatedAt + }, + loadDiff: loadReviewDiff, get activeFile() { return tree.activeDiff }, diff --git a/packages/app/src/pages/session/v2/review-diff-kinds.test.ts b/packages/app/src/pages/session/v2/review-diff-kinds.test.ts index ee37a2c8b4c9..2d069cdb186f 100644 --- a/packages/app/src/pages/session/v2/review-diff-kinds.test.ts +++ b/packages/app/src/pages/session/v2/review-diff-kinds.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test" -import { filterReviewFiles, reviewDiffKinds } from "./review-diff-kinds" +import { filterReviewFiles, reviewDiffDirectory, reviewDiffKinds, reviewDiffNeedsLoad } from "./review-diff-kinds" describe("reviewDiffKinds", () => { test("maps file and directory kinds", () => { @@ -28,3 +28,43 @@ describe("filterReviewFiles", () => { expect(filterReviewFiles(files, "")).toEqual(files) }) }) + +describe("reviewDiffNeedsLoad", () => { + test("loads changed files whose aggregate patch has no hunks", () => { + expect( + reviewDiffNeedsLoad({ + file: "src/a.ts", + additions: 1, + deletions: 0, + patch: "diff --git a/src/a.ts b/src/a.ts\n--- a/src/a.ts\n+++ b/src/a.ts", + }), + ).toBe(true) + }) + + test("keeps complete patches and empty changes", () => { + expect( + reviewDiffNeedsLoad({ + file: "src/a.ts", + additions: 1, + deletions: 0, + patch: "@@ -0,0 +1 @@\n+value", + }), + ).toBe(false) + expect(reviewDiffNeedsLoad({ file: "empty.txt", additions: 0, deletions: 0 })).toBe(false) + }) +}) + +describe("reviewDiffDirectory", () => { + test("scopes nested files to their parent directory", () => { + expect(reviewDiffDirectory("/repo", "src/lib/a.ts")).toBe("/repo/src/lib") + expect(reviewDiffDirectory("C:\\repo", "src/lib/a.ts")).toBe("C:\\repo\\src\\lib") + }) + + test("does not rescope root files", () => { + expect(reviewDiffDirectory("/repo/", "README.md")).toBe("/repo") + expect(reviewDiffDirectory("/", "README.md")).toBe("/") + expect(reviewDiffDirectory("C:\\", "README.md")).toBe("C:\\") + expect(reviewDiffDirectory("/", "src/a.ts")).toBe("/src") + expect(reviewDiffDirectory("C:\\", "src/a.ts")).toBe("C:\\src") + }) +}) diff --git a/packages/app/src/pages/session/v2/review-diff-kinds.ts b/packages/app/src/pages/session/v2/review-diff-kinds.ts index c288c357068d..49cec334bc47 100644 --- a/packages/app/src/pages/session/v2/review-diff-kinds.ts +++ b/packages/app/src/pages/session/v2/review-diff-kinds.ts @@ -12,6 +12,24 @@ export function filterRenderableDiff(value: SnapshotFileDiff | VcsFileDiff): val return typeof value.file === "string" } +export function reviewDiffNeedsLoad(diff: RenderDiff) { + if (diff.additions === 0 && diff.deletions === 0) return false + return !diff.patch || !/^@@ /m.test(diff.patch) +} + +export function reviewRootDirectory(root: string) { + return root === "/" || /^[A-Za-z]:[/\\]?$/.test(root) ? root : root.replace(/[/\\]+$/, "") +} + +export function reviewDiffDirectory(root: string, file: string) { + const path = normalizePath(file) + const index = path.lastIndexOf("/") + const separator = root.includes("\\") ? "\\" : "/" + const base = reviewRootDirectory(root) + if (index < 0) return base + return `${base.endsWith(separator) ? base : base + separator}${path.slice(0, index).replaceAll("/", separator)}` +} + export function reviewDiffKinds(diffs: RenderDiff[]) { const merge = (a: Kind | undefined, b: Kind) => { if (!a) return b diff --git a/packages/app/src/pages/session/v2/review-panel-v2.tsx b/packages/app/src/pages/session/v2/review-panel-v2.tsx index fcd52756ab08..0900f6d20181 100644 --- a/packages/app/src/pages/session/v2/review-panel-v2.tsx +++ b/packages/app/src/pages/session/v2/review-panel-v2.tsx @@ -1,4 +1,4 @@ -import { createMemo, createSignal, Show, type JSX } from "solid-js" +import { createEffect, createMemo, createSignal, onCleanup, Show, type JSX } from "solid-js" import type { SnapshotFileDiff, VcsFileDiff } from "@opencode-ai/sdk/v2" import { SESSION_REVIEW_V2_SIDEBAR_WIDTH_MAX, @@ -25,6 +25,7 @@ import { filterRenderableDiff, filterReviewFiles, reviewDiffKinds, + reviewDiffNeedsLoad, type RenderDiff, } from "@/pages/session/v2/review-diff-kinds" import type { ReviewPanelV2State } from "@/pages/session/v2/review-panel-v2-state" @@ -37,6 +38,8 @@ export type ReviewPanelV2Props = { empty?: JSX.Element diffs: () => ReviewDiff[] diffsReady: () => boolean + diffVersion?: number + loadDiff?: (path: string, version?: number) => Promise activeFile?: string onSelectFile: (path: string) => void diffStyle: SessionReviewDiffStyle @@ -74,7 +77,30 @@ export function ReviewPanelV2(props: ReviewPanelV2Props) { if (active && files.includes(active)) return active return files[0] }) - const activeItem = createMemo(() => diffs().find((diff) => diff.file === activeDiff())) + const sourceActiveItem = createMemo(() => diffs().find((diff) => diff.file === activeDiff())) + const [loadedDiff, setLoadedDiff] = createSignal<{ source: RenderDiff; version?: number; value: RenderDiff }>() + let loadToken = 0 + + createEffect(() => { + const diff = sourceActiveItem() + const load = props.loadDiff + const version = props.diffVersion + const token = ++loadToken + setLoadedDiff(undefined) + if (!diff || !load || !reviewDiffNeedsLoad(diff)) return + void load(diff.file, version).then((result) => { + if (token !== loadToken || result?.file !== diff.file) return + setLoadedDiff({ source: diff, version, value: result }) + }) + }) + onCleanup(() => loadToken++) + + const activeItem = createMemo(() => { + const source = sourceActiveItem() + const loaded = loadedDiff() + if (loaded && loaded.source === source && loaded.version === props.diffVersion) return loaded.value + return source + }) const readFile = async (path: string) => sdk() From d5d5f181bbff98b9689514f6e854140ffedd7d63 Mon Sep 17 00:00:00 2001 From: LukeParkerDev <10430890+Hona@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:16:13 +1000 Subject: [PATCH 2/3] refactor(app): use resource for review patches --- .../src/pages/session/v2/review-panel-v2.tsx | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/app/src/pages/session/v2/review-panel-v2.tsx b/packages/app/src/pages/session/v2/review-panel-v2.tsx index 0900f6d20181..b2e67815e5c2 100644 --- a/packages/app/src/pages/session/v2/review-panel-v2.tsx +++ b/packages/app/src/pages/session/v2/review-panel-v2.tsx @@ -1,4 +1,4 @@ -import { createEffect, createMemo, createSignal, onCleanup, Show, type JSX } from "solid-js" +import { createMemo, createResource, createSignal, Show, type JSX } from "solid-js" import type { SnapshotFileDiff, VcsFileDiff } from "@opencode-ai/sdk/v2" import { SESSION_REVIEW_V2_SIDEBAR_WIDTH_MAX, @@ -78,22 +78,17 @@ export function ReviewPanelV2(props: ReviewPanelV2Props) { return files[0] }) const sourceActiveItem = createMemo(() => diffs().find((diff) => diff.file === activeDiff())) - const [loadedDiff, setLoadedDiff] = createSignal<{ source: RenderDiff; version?: number; value: RenderDiff }>() - let loadToken = 0 - - createEffect(() => { + const detailSource = createMemo(() => { const diff = sourceActiveItem() const load = props.loadDiff - const version = props.diffVersion - const token = ++loadToken - setLoadedDiff(undefined) if (!diff || !load || !reviewDiffNeedsLoad(diff)) return - void load(diff.file, version).then((result) => { - if (token !== loadToken || result?.file !== diff.file) return - setLoadedDiff({ source: diff, version, value: result }) - }) + return { diff, load, version: props.diffVersion } + }) + const [loadedDiff] = createResource(detailSource, async ({ diff, load, version }) => { + const value = await load(diff.file, version) + if (value?.file !== diff.file) return + return { source: diff, version, value } }) - onCleanup(() => loadToken++) const activeItem = createMemo(() => { const source = sourceActiveItem() From bd71fec53e4d143ee912ff576e3d45884e8fe03d Mon Sep 17 00:00:00 2001 From: LukeParkerDev <10430890+Hona@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:37:41 +1000 Subject: [PATCH 3/3] fix(app): avoid suspending review preview --- packages/app/src/pages/session/v2/review-panel-v2.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/app/src/pages/session/v2/review-panel-v2.tsx b/packages/app/src/pages/session/v2/review-panel-v2.tsx index b2e67815e5c2..a202a1e39dae 100644 --- a/packages/app/src/pages/session/v2/review-panel-v2.tsx +++ b/packages/app/src/pages/session/v2/review-panel-v2.tsx @@ -92,6 +92,7 @@ export function ReviewPanelV2(props: ReviewPanelV2Props) { const activeItem = createMemo(() => { const source = sourceActiveItem() + if (loadedDiff.state !== "ready") return source const loaded = loadedDiff() if (loaded && loaded.source === source && loaded.version === props.diffVersion) return loaded.value return source