mirror of
https://github.com/anomalyco/opencode.git
synced 2026-07-09 22:58:28 +00:00
fix(app): load capped review patches (#35633)
This commit is contained in:
parent
62de2b3695
commit
824455e2f1
5 changed files with 199 additions and 22 deletions
|
|
@ -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,
|
||||
|
|
@ -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}`,
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
@ -75,6 +75,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"
|
||||
|
|
@ -644,6 +645,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"
|
||||
|
|
@ -1175,6 +1216,10 @@ export default function Page() {
|
|||
},
|
||||
diffs: reviewDiffs,
|
||||
diffsReady: reviewReady,
|
||||
get diffVersion() {
|
||||
return vcsQuery.dataUpdatedAt
|
||||
},
|
||||
loadDiff: loadReviewDiff,
|
||||
get activeFile() {
|
||||
return tree.activeDiff
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
@ -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<RenderDiff | undefined>
|
||||
activeFile?: string
|
||||
onSelectFile: (path: string) => void
|
||||
diffStyle: SessionReviewDiffStyle
|
||||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue