Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 70 additions & 18 deletions packages/app/e2e/regression/review-terminal-stacked.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> }> = []
let detailVersion = 1
let detailFailures = 1
await page.setViewportSize({ width: 1400, height: 900 })
await mockOpenCodeServer(page, {
directory,
Expand Down Expand Up @@ -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({
Expand All @@ -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,
Expand Down Expand Up @@ -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')
Expand All @@ -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)
})

Expand Down Expand Up @@ -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}`,
}
}
47 changes: 46 additions & 1 deletion packages/app/src/pages/session.tsx
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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<VcsFileDiff | undefined> => {
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"
Expand Down Expand Up @@ -1182,6 +1223,10 @@ export default function Page() {
},
diffs: reviewDiffs,
diffsReady: reviewReady,
get diffVersion() {
return vcsQuery.dataUpdatedAt
},
loadDiff: loadReviewDiff,
get activeFile() {
return tree.activeDiff
},
Expand Down
42 changes: 41 additions & 1 deletion packages/app/src/pages/session/v2/review-diff-kinds.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down Expand Up @@ -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")
})
})
18 changes: 18 additions & 0 deletions packages/app/src/pages/session/v2/review-diff-kinds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 24 additions & 2 deletions packages/app/src/pages/session/v2/review-panel-v2.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createMemo, createSignal, 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,
Expand All @@ -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"
Expand All @@ -37,6 +38,8 @@ export type ReviewPanelV2Props = {
empty?: JSX.Element
diffs: () => ReviewDiff[]
diffsReady: () => boolean
diffVersion?: number
loadDiff?: (path: string, version?: number) => Promise<RenderDiff | undefined>
activeFile?: string
onSelectFile: (path: string) => void
diffStyle: SessionReviewDiffStyle
Expand Down Expand Up @@ -74,7 +77,26 @@ 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 detailSource = createMemo(() => {
const diff = sourceActiveItem()
const load = props.loadDiff
if (!diff || !load || !reviewDiffNeedsLoad(diff)) return
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 }
})

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
})

const readFile = async (path: string) =>
sdk()
Expand Down
Loading