diff --git a/.changeset/load-initial-diff-previews.md b/.changeset/load-initial-diff-previews.md new file mode 100644 index 00000000000..d6fdf22dd54 --- /dev/null +++ b/.changeset/load-initial-diff-previews.md @@ -0,0 +1,5 @@ +--- +"kilo-code": patch +--- + +Load expanded diff previews consistently when opening a review. diff --git a/packages/kilo-vscode/tests/unit/diff-preview-request.test.ts b/packages/kilo-vscode/tests/unit/diff-preview-request.test.ts new file mode 100644 index 00000000000..ba61eeee9be --- /dev/null +++ b/packages/kilo-vscode/tests/unit/diff-preview-request.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it } from "bun:test" +import path from "node:path" + +const WEBVIEW = path.resolve(import.meta.dir, "../../webview-ui") +const PASS = "DIFF_PREVIEW_REQUEST_PASS" +const FAIL = "DIFF_PREVIEW_REQUEST_FAIL:" + +const SCRIPT = ` + const { createEffect, createRoot, createSignal, on } = await import("solid-js") + const { createDiffRequests } = await import("./diff-viewer/diff-requests.ts") + + const summary = { + file: "src/app.ts", + before: "", + after: "", + patch: "", + additions: 1, + deletions: 0, + status: "modified", + tracked: true, + generatedLike: false, + summarized: true, + stamp: "1:1", + } + + const fail = (reason) => { + console.log("${FAIL}" + reason) + process.exit(2) + } + + const requested = [] + const [key, setKey] = createSignal("review-1") + const [diffs, setDiffs] = createSignal([summary]) + const [open, setOpen] = createSignal([]) + const dispose = createRoot((dispose) => { + let initialized + createEffect( + on( + () => [key(), diffs()], + ([next, items]) => { + if (next === initialized || items.length === 0) return + initialized = next + setOpen(items.map((item) => item.file)) + }, + ), + ) + createDiffRequests({ + key, + diffs, + open, + loading: () => undefined, + send: () => (file) => requested.push(file), + }) + return dispose + }) + + await new Promise((resolve) => setTimeout(resolve, 0)) + if (requested.length !== 1 || requested[0] !== summary.file) { + fail("initial summarized diff requested " + JSON.stringify(requested)) + } + + setDiffs([{ ...summary }]) + await new Promise((resolve) => setTimeout(resolve, 0)) + if (requested.length !== 1) { + fail("unchanged summarized diff requested again " + JSON.stringify(requested)) + } + + setOpen([]) + await new Promise((resolve) => setTimeout(resolve, 0)) + setOpen([summary.file]) + await new Promise((resolve) => setTimeout(resolve, 0)) + if (requested.length !== 2) { + fail("reopened summarized diff did not retry " + JSON.stringify(requested)) + } + + setKey("review-2") + await new Promise((resolve) => setTimeout(resolve, 0)) + if (requested.length !== 3) { + fail("new review did not request the same diff token " + JSON.stringify(requested)) + } + dispose() + + const blocked = [] + const [loading, setLoading] = createSignal(new Set([summary.file])) + const disposeBlocked = createRoot((dispose) => { + createDiffRequests({ + key: () => "review-3", + diffs, + open: () => [summary.file], + loading, + send: () => (file) => blocked.push(file), + }) + return dispose + }) + + await new Promise((resolve) => setTimeout(resolve, 0)) + if (blocked.length !== 0) fail("requested a diff that was already loading") + setLoading(new Set()) + await new Promise((resolve) => setTimeout(resolve, 0)) + if (blocked.length !== 1 || blocked[0] !== summary.file) { + fail("did not request after existing loading state cleared " + JSON.stringify(blocked)) + } + disposeBlocked() + console.log("${PASS}") +` + +describe("diff preview detail requests", () => { + it("requests detail when summarized diffs are present on first render", () => { + const result = Bun.spawnSync(["bun", "--conditions=browser", "-e", SCRIPT], { + cwd: WEBVIEW, + stdout: "pipe", + stderr: "pipe", + }) + const output = result.stdout.toString() + result.stderr.toString() + const logic = output.indexOf(FAIL) + + if (logic !== -1) { + expect.unreachable( + output + .slice(logic + FAIL.length) + .split("\n")[0] + ?.trim(), + ) + } + expect(result.exitCode, output).toBe(0) + expect(output).toContain(PASS) + }) +}) diff --git a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx index 0fe46a5aeec..12897d8426b 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx @@ -59,7 +59,8 @@ import { VirtualDiffList } from "../diff-viewer/VirtualDiffList" import { treeOrder } from "../diff-viewer/file-tree-utils" import { isMarkdownFile, MarkdownDiffView } from "../diff-viewer/MarkdownDiffView" import { ImageDiffView } from "../diff-viewer/ImageDiffView" -import { createDiffRows, diffToken } from "../diff-viewer/diff-state" +import { createDiffRows } from "../diff-viewer/diff-state" +import { createDiffRequests } from "../diff-viewer/diff-requests" // --- Data model --- @@ -136,7 +137,6 @@ export const DiffPanel: Component = (props) => { // collapse state while adding and removing files from live summaries. let initializedKey: string | undefined let known = new Set() - const requested = new Map() // Reorder diffs to match the file-tree's depth-first visual order so // scrolling through the accordion matches the tree grouping. @@ -240,7 +240,6 @@ export const DiffPanel: Component = (props) => { on( () => props.sessionKey, () => { - requested.clear() setDraft(null) draftMeta = null setEditing(null) @@ -251,32 +250,13 @@ export const DiffPanel: Component = (props) => { ), ) - const request = (diff: WorktreeFileDiff) => { - if (!props.onRequestDiff || props.loadingFiles?.has(diff.file)) return - if (!isDiffExpandable(diff) || diff.summarized !== true) return - const value = diffToken(diff) - if (requested.get(diff.file) === value) return - requested.set(diff.file, value) - props.onRequestDiff(diff.file) - } - - createEffect( - on( - () => [open(), props.diffs] as const, - ([next]) => { - const files = new Set(next) - for (const file of requested.keys()) { - if (!files.has(file)) requested.delete(file) - } - for (const file of next) { - const diff = props.diffs.find((item) => item.file === file) - if (!diff || diff.kind === "image") continue - request(diff) - } - }, - { defer: true }, - ), - ) + const request = createDiffRequests({ + key: () => props.sessionKey, + diffs: () => props.diffs, + open, + loading: () => props.loadingFiles, + send: () => props.onRequestDiff, + }) // --- CRUD --- diff --git a/packages/kilo-vscode/webview-ui/diff-viewer/FullScreenDiffView.tsx b/packages/kilo-vscode/webview-ui/diff-viewer/FullScreenDiffView.tsx index 09cbe615b28..03d3b0b3034 100644 --- a/packages/kilo-vscode/webview-ui/diff-viewer/FullScreenDiffView.tsx +++ b/packages/kilo-vscode/webview-ui/diff-viewer/FullScreenDiffView.tsx @@ -60,7 +60,8 @@ import { DiffEndMarker } from "./DiffEndMarker" import { VirtualDiffList } from "./VirtualDiffList" import { isMarkdownFile, MarkdownDiffView } from "./MarkdownDiffView" import { ImageDiffView } from "./ImageDiffView" -import { createDiffRows, diffToken } from "./diff-state" +import { createDiffRows } from "./diff-state" +import { createDiffRequests } from "./diff-requests" type DiffStyle = "unified" | "split" @@ -144,7 +145,6 @@ export const FullScreenDiffView: Component = (props) => // collapse state while adding and removing files from live summaries. let initializedKey: string | undefined let known = new Set() - const requested = new Map() let rootRef: HTMLDivElement | undefined const [scroller, setScroller] = createSignal() const [virtualizer, setVirtualizer] = createSignal() @@ -246,7 +246,6 @@ export const FullScreenDiffView: Component = (props) => on( () => props.sessionKey, () => { - requested.clear() setDraft(null) draftMeta = null setEditing(null) @@ -257,32 +256,13 @@ export const FullScreenDiffView: Component = (props) => ), ) - const request = (diff: WorktreeFileDiff) => { - if (!props.onRequestDiff || props.loadingFiles?.has(diff.file)) return - if (!isDiffExpandable(diff) || diff.summarized !== true) return - const value = diffToken(diff) - if (requested.get(diff.file) === value) return - requested.set(diff.file, value) - props.onRequestDiff(diff.file) - } - - createEffect( - on( - () => [open(), props.diffs] as const, - ([next]) => { - const files = new Set(next) - for (const file of requested.keys()) { - if (!files.has(file)) requested.delete(file) - } - for (const file of next) { - const diff = props.diffs.find((item) => item.file === file) - if (!diff || diff.kind === "image") continue - request(diff) - } - }, - { defer: true }, - ), - ) + const request = createDiffRequests({ + key: () => props.sessionKey, + diffs: () => props.diffs, + open, + loading: () => props.loadingFiles, + send: () => props.onRequestDiff, + }) // --- CRUD --- diff --git a/packages/kilo-vscode/webview-ui/diff-viewer/diff-requests.ts b/packages/kilo-vscode/webview-ui/diff-viewer/diff-requests.ts new file mode 100644 index 00000000000..904ceca13fc --- /dev/null +++ b/packages/kilo-vscode/webview-ui/diff-viewer/diff-requests.ts @@ -0,0 +1,55 @@ +import { createEffect, on, type Accessor } from "solid-js" +import type { WorktreeFileDiff } from "../src/types/messages" +import { isDiffExpandable } from "./diff-open-policy" +import { diffToken } from "./diff-state" + +interface DiffRequestOptions { + key: Accessor + diffs: Accessor + open: Accessor + loading: Accessor | undefined> + send: Accessor<((file: string) => void) | undefined> +} + +export function createDiffRequests(opts: DiffRequestOptions) { + const requested = new Map() + + createEffect( + on( + opts.key, + () => { + requested.clear() + }, + { defer: true }, + ), + ) + + const request = (diff: WorktreeFileDiff) => { + const send = opts.send() + if (!send || opts.loading()?.has(diff.file)) return + if (!isDiffExpandable(diff) || diff.summarized !== true) return + const value = diffToken(diff) + if (requested.get(diff.file) === value) return + requested.set(diff.file, value) + send(diff.file) + } + + createEffect( + on( + () => [opts.open(), opts.diffs(), opts.loading()] as const, + ([open, diffs]) => { + const files = new Set(open) + for (const file of requested.keys()) { + if (!files.has(file)) requested.delete(file) + } + for (const file of open) { + const diff = diffs.find((item) => item.file === file) + if (!diff || diff.kind === "image") continue + request(diff) + } + }, + ), + ) + + return request +}