From ea74bda3996d55ace921a80b96e3ddb65636652b Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Tue, 25 Aug 2026 12:17:33 +0200 Subject: [PATCH] perf(agent-manager): optimize worktree diff loading --- .changeset/fast-agent-manager-diffs.md | 5 + .../src/agent-manager/AgentManagerProvider.ts | 2 +- .../kilo-vscode/src/agent-manager/GitOps.ts | 13 +- .../src/agent-manager/local-diff.ts | 125 ++++++++++++++---- .../src/agent-manager/semaphore.ts | 33 +++-- .../kilo-vscode/src/diff/SourceController.ts | 16 ++- .../kilo-vscode/src/diff/sources/catalog.ts | 13 +- .../kilo-vscode/src/diff/sources/worktree.ts | 16 ++- .../tests/unit/agent-manager-arch.test.ts | 2 + .../unit/agent-manager-worktree-diffs.test.ts | 11 ++ .../kilo-vscode/tests/unit/local-diff.test.ts | 30 +++++ .../kilo-vscode/tests/unit/semaphore.test.ts | 16 +++ .../tests/unit/source-controller.test.ts | 30 +++++ .../agent-manager/AgentManagerApp.tsx | 99 ++++++++------ .../webview-ui/agent-manager/DiffPanel.tsx | 11 +- .../agent-manager/DiffPanelCache.tsx | 110 +++++++++++++++ .../agent-manager/agent-manager.css | 18 +++ .../webview-ui/agent-manager/revert-file.ts | 14 +- .../agent-manager/review-composers.ts | 24 ++++ .../agent-manager/worktree-diffs.ts | 34 ++++- .../webview-ui/diff-viewer/diff-requests.ts | 2 +- 21 files changed, 519 insertions(+), 105 deletions(-) create mode 100644 .changeset/fast-agent-manager-diffs.md create mode 100644 packages/kilo-vscode/webview-ui/agent-manager/DiffPanelCache.tsx create mode 100644 packages/kilo-vscode/webview-ui/agent-manager/review-composers.ts diff --git a/.changeset/fast-agent-manager-diffs.md b/.changeset/fast-agent-manager-diffs.md new file mode 100644 index 0000000000..8a51a2f6c9 --- /dev/null +++ b/.changeset/fast-agent-manager-diffs.md @@ -0,0 +1,5 @@ +--- +"kilo-code": patch +--- + +Load Agent Manager worktree diffs faster and keep warmed reviews visible when switching worktrees. diff --git a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts index 99eab29e3e..68e24959d1 100644 --- a/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts +++ b/packages/kilo-vscode/src/agent-manager/AgentManagerProvider.ts @@ -212,7 +212,7 @@ export class AgentManagerProvider implements Disposable { log: (msg) => this.log(msg), }) const local = createLocalDiff(this.gitOps, (...args) => this.log(...args)) - this.diffCatalog = new DiffSourceCatalog(this.connectionService) + this.diffCatalog = new DiffSourceCatalog(this.connectionService, local) this.diffs = new WorktreeDiffController({ getState: () => this.getStateManager(), getRoot: () => this.getRoot(), diff --git a/packages/kilo-vscode/src/agent-manager/GitOps.ts b/packages/kilo-vscode/src/agent-manager/GitOps.ts index 8b14d97277..e31ebc00c7 100644 --- a/packages/kilo-vscode/src/agent-manager/GitOps.ts +++ b/packages/kilo-vscode/src/agent-manager/GitOps.ts @@ -44,6 +44,7 @@ interface ExecOptions { env?: NodeJS.ProcessEnv stdin?: string timeout?: number + signal?: AbortSignal } export interface ExecResult { @@ -594,12 +595,12 @@ export class GitOps { * suitable for callers that need to tolerate legitimate failures (e.g. * `merge-base` on an orphan branch, `ls-files --error-unmatch`). */ - execGit(args: string[], cwd: string, options?: { stdin?: string }): Promise { + execGit(args: string[], cwd: string, options?: { stdin?: string; signal?: AbortSignal }): Promise { return this.exec(args, cwd, options) } - execGitBuffer(args: string[], cwd: string): Promise { - return this.execBuffer(args, cwd) + execGitBuffer(args: string[], cwd: string, options?: { signal?: AbortSignal }): Promise { + return this.execBuffer(args, cwd, options) } private async exec(args: string[], cwd: string, options?: ExecOptions): Promise { @@ -616,7 +617,7 @@ export class GitOps { return { code: 1, stdout: Buffer.alloc(0), stderr: "GitOps disposed" } } const invoke = () => this.invoke(cmd, args, cwd, options) - return this.semaphore ? this.semaphore.run(invoke) : invoke() + return this.semaphore ? this.semaphore.run(invoke, options?.signal) : invoke() } private executable(): Promise { @@ -642,7 +643,7 @@ export class GitOps { } private invoke(cmd: string, args: string[], cwd: string, options?: ExecOptions): Promise { - if (this.controller.signal.aborted) { + if (this.controller.signal.aborted || options?.signal?.aborted) { return Promise.resolve({ code: 1, stdout: Buffer.alloc(0), stderr: "GitOps disposed" }) } @@ -664,6 +665,7 @@ export class GitOps { : undefined this.controller.signal.addEventListener("abort", abort, { once: true }) + options?.signal?.addEventListener("abort", abort, { once: true }) child.stdout?.on("data", (chunk: Buffer) => out.push(chunk)) child.stderr?.on("data", (chunk: Buffer) => err.push(chunk)) @@ -673,6 +675,7 @@ export class GitOps { child.on("close", (code) => { if (timeout) clearTimeout(timeout) this.controller.signal.removeEventListener("abort", abort) + options?.signal?.removeEventListener("abort", abort) resolve({ code: code ?? 1, stdout: Buffer.concat(out), diff --git a/packages/kilo-vscode/src/agent-manager/local-diff.ts b/packages/kilo-vscode/src/agent-manager/local-diff.ts index e643e86627..83eff8b4f7 100644 --- a/packages/kilo-vscode/src/agent-manager/local-diff.ts +++ b/packages/kilo-vscode/src/agent-manager/local-diff.ts @@ -166,16 +166,16 @@ function statusFromCode(code: string): Status { } async function list(git: GitOps, dir: string, anc: string, log?: Log): Promise { - const nameStatus = await git.execGit( - ["-c", "core.quotepath=false", "diff", "--name-status", "--no-renames", anc], - dir, - ) + const [nameStatus, counts, untracked] = await Promise.all([ + git.execGit(["-c", "core.quotepath=false", "diff", "--name-status", "--no-renames", anc], dir), + numstat(git, dir, anc), + git.execGit(["ls-files", "--others", "--exclude-standard"], dir), + ]) if (nameStatus.code !== 0) { log?.("git diff --name-status failed", { code: nameStatus.code, stderr: nameStatus.stderr.trim() }) return [] } - const counts = await numstat(git, dir, anc) const result: Meta[] = [] const seen = new Set() @@ -201,7 +201,6 @@ async function list(git: GitOps, dir: string, anc: string, log?: Log): Promise }>() + const generations = new Map() + const details = new Map() + const pending = new Map }>() + let bytes = 0 + + const remember = (id: string, value: WorktreeDiffEntry) => { + const size = + (value.before?.length ?? 0) + + (value.after?.length ?? 0) + + (value.patch?.length ?? 0) + + (value.image?.before?.data?.length ?? 0) + + (value.image?.after?.data?.length ?? 0) + const current = details.get(id) + if (current) bytes -= current.bytes + details.delete(id) + details.set(id, { value, bytes: size }) + bytes += size + while (details.size > 128 || bytes > 64 * 1024 * 1024) { + const key = details.keys().next().value! + bytes -= details.get(key)!.bytes + details.delete(key) + } + } return { summary: async (dir: string, base: string): Promise => { const id = `${dir}\0${base}` + const generation = (generations.get(id) ?? 0) + 1 + generations.set(id, generation) const anc = await ancestor(git, dir, base, log) if (!anc) { - states.delete(id) + if (generations.get(id) === generation) states.delete(id) return [] } const items = await list(git, dir, anc, log) + if (generations.get(id) !== generation) return items.map(summarize) states.delete(id) states.set(id, { anc, metas: new Map(items.map((item) => [item.file, item])) }) if (states.size > 8) states.delete(states.keys().next().value!) return items.map(summarize) }, - file: async (dir: string, base: string, file: string): Promise => { + file: async (dir: string, base: string, file: string, signal?: AbortSignal): Promise => { const state = states.get(`${dir}\0${base}`) if (!state) return diffFile(git, dir, base, file, log) const meta = state.metas.get(file) if (!meta) return null - return materialize(git, dir, state.anc, meta, log) + const id = `${dir}\0${base}\0${state.anc}\0${file}\0${meta.stamp}` + const cached = details.get(id) + if (cached) { + remember(id, cached.value) + return cached.value + } + const current = pending.get(id) + if (current && !current.signal?.aborted) return current.work + const work = materialize(git, dir, state.anc, meta, log, signal) + pending.set(id, { signal, work }) + work.then( + (value) => { + if (pending.get(id)?.work !== work) return + pending.delete(id) + remember(id, value) + }, + () => { + if (pending.get(id)?.work === work) pending.delete(id) + }, + ) + return work }, } } @@ -343,8 +388,8 @@ async function detailMeta(git: GitOps, dir: string, anc: string, file: string): } } -async function blobSize(git: GitOps, dir: string, anc: string, file: string): Promise { - const result = await git.execGit(["cat-file", "-s", `${anc}:${file}`], dir) +async function blobSize(git: GitOps, dir: string, anc: string, file: string, signal?: AbortSignal): Promise { + const result = await git.execGit(["cat-file", "-s", `${anc}:${file}`], dir, { signal }) if (result.code !== 0) return 0 return parseInt(result.stdout.trim(), 10) || 0 } @@ -356,8 +401,14 @@ async function fileSize(dir: string, file: string): Promise { return stat?.size ?? 0 } -async function readBlob(git: GitOps, dir: string, ref: string, file: string): Promise { - const result = await git.execGitBuffer(["show", `${ref}:${file}`], dir) +async function readBlob( + git: GitOps, + dir: string, + ref: string, + file: string, + signal?: AbortSignal, +): Promise { + const result = await git.execGitBuffer(["show", `${ref}:${file}`], dir, { signal }) return result.code === 0 ? result.stdout : undefined } @@ -369,9 +420,16 @@ async function readFile(dir: string, file: string): Promise return readImageFile(full) } -async function readBefore(git: GitOps, dir: string, anc: string, file: string, status: Status): Promise { +async function readBefore( + git: GitOps, + dir: string, + anc: string, + file: string, + status: Status, + signal?: AbortSignal, +): Promise { if (status === "added") return "" - const result = await git.execGit(["show", `${anc}:${file}`], dir) + const result = await git.execGit(["show", `${anc}:${file}`], dir, { signal }) return result.code === 0 ? result.stdout : "" } @@ -386,10 +444,17 @@ async function readAfter(dir: string, file: string, status: Status): Promise "") } -async function unifiedPatch(git: GitOps, dir: string, anc: string, file: string): Promise { +async function unifiedPatch( + git: GitOps, + dir: string, + anc: string, + file: string, + signal?: AbortSignal, +): Promise { const result = await git.execGit( ["-c", "core.quotepath=false", "diff", "--no-ext-diff", "--no-renames", anc, "--", file], dir, + { signal }, ) return result.code === 0 ? result.stdout : "" } @@ -418,15 +483,26 @@ export async function diffFile( return materialize(git, dir, anc, meta, log) } -async function materialize(git: GitOps, dir: string, anc: string, meta: Meta, log?: Log): Promise { +async function materialize( + git: GitOps, + dir: string, + anc: string, + meta: Meta, + log?: Log, + signal?: AbortSignal, +): Promise { const mime = imageMime(meta.file) if (meta.binary && !mime) return summarize(meta) - const beforeBytes = meta.status === "added" ? 0 : await blobSize(git, dir, anc, meta.file) - const afterBytes = meta.status === "deleted" ? 0 : await fileSize(dir, meta.file) + const [beforeBytes, afterBytes] = await Promise.all([ + meta.status === "added" ? 0 : blobSize(git, dir, anc, meta.file, signal), + meta.status === "deleted" ? 0 : fileSize(dir, meta.file), + ]) if (mime) { const image = await loadImage( meta.file, - meta.status === "added" ? undefined : { bytes: beforeBytes, read: () => readBlob(git, dir, anc, meta.file) }, + meta.status === "added" + ? undefined + : { bytes: beforeBytes, read: () => readBlob(git, dir, anc, meta.file, signal) }, meta.status === "deleted" ? undefined : { bytes: afterBytes, read: () => readFile(dir, meta.file) }, ) return { ...summarize(meta), summarized: false, image } @@ -444,9 +520,12 @@ async function materialize(git: GitOps, dir: string, anc: string, meta: Meta, lo return summarize(meta) } - const before = await readBefore(git, dir, anc, meta.file, meta.status) - const after = await readAfter(dir, meta.file, meta.status) - const patch = meta.tracked ? await unifiedPatch(git, dir, anc, meta.file) : buildUntrackedPatch(meta.file, after) + const [before, after, tracked] = await Promise.all([ + readBefore(git, dir, anc, meta.file, meta.status, signal), + readAfter(dir, meta.file, meta.status), + meta.tracked ? unifiedPatch(git, dir, anc, meta.file, signal) : Promise.resolve(""), + ]) + const patch = meta.tracked ? tracked : buildUntrackedPatch(meta.file, after) const additions = meta.status === "added" && meta.additions === 0 && !meta.tracked ? linesOf(after) : meta.additions return { file: meta.file, diff --git a/packages/kilo-vscode/src/agent-manager/semaphore.ts b/packages/kilo-vscode/src/agent-manager/semaphore.ts index 37f3ab9cdd..a5e52d3eaf 100644 --- a/packages/kilo-vscode/src/agent-manager/semaphore.ts +++ b/packages/kilo-vscode/src/agent-manager/semaphore.ts @@ -7,12 +7,12 @@ */ export class Semaphore { private running = 0 - private readonly pending: (() => void)[] = [] + private readonly pending: { resolve: () => void; abort?: () => void }[] = [] constructor(private readonly limit: number) {} - async run(fn: () => Promise): Promise { - await this.acquire() + async run(fn: () => Promise, signal?: AbortSignal): Promise { + await this.acquire(signal) try { return await fn() } finally { @@ -20,22 +20,33 @@ export class Semaphore { } } - private acquire(): Promise { + private acquire(signal?: AbortSignal): Promise { + if (signal?.aborted) return Promise.reject(signal.reason) if (this.running < this.limit) { this.running++ return Promise.resolve() } - return new Promise((resolve) => { - this.pending.push(() => { - this.running++ - resolve() - }) + return new Promise((resolve, reject) => { + const item = { + resolve: () => { + if (item.abort) signal?.removeEventListener("abort", item.abort) + this.running++ + resolve() + }, + abort: undefined as (() => void) | undefined, + } + item.abort = () => { + const index = this.pending.indexOf(item) + if (index !== -1) this.pending.splice(index, 1) + reject(signal?.reason) + } + signal?.addEventListener("abort", item.abort, { once: true }) + this.pending.push(item) }) } private release(): void { this.running-- - const next = this.pending.shift() - if (next) next() + this.pending.shift()?.resolve() } } diff --git a/packages/kilo-vscode/src/diff/SourceController.ts b/packages/kilo-vscode/src/diff/SourceController.ts index 11a51ed344..1d454fdf8d 100644 --- a/packages/kilo-vscode/src/diff/SourceController.ts +++ b/packages/kilo-vscode/src/diff/SourceController.ts @@ -180,6 +180,13 @@ export class SourceController { this.send(this.messages.diffFile(source, file, null)) return } + // Yield once so a worktree switch can advance the epoch before queued + // detail work enters the shared Git semaphore. + await new Promise((resolve) => setTimeout(resolve, 0)) + if (this.epoch !== epoch || this.active !== source) { + this.send(this.messages.diffFile(source, file, null)) + return + } const diff = await source.fetchFile(file).catch(() => null) // Discard stale content after disposal/swap, but still complete the request // so consumers can clear per-file loading state. @@ -239,10 +246,15 @@ export class SourceController { private startPolling(source: DiffSource, epoch: number): void { this.stopPolling() + let busy = false this.interval = setInterval(async () => { + if (busy) return + busy = true // Self-cancel when the tick reports the source is done - const keep = await this.runFetch(source, epoch, false) - if (!keep) this.stopPolling() + const keep = await this.runFetch(source, epoch, false).finally(() => { + busy = false + }) + if (!keep && this.epoch === epoch && this.active === source) this.stopPolling() }, DIFF_POLL_INTERVAL_MS) } diff --git a/packages/kilo-vscode/src/diff/sources/catalog.ts b/packages/kilo-vscode/src/diff/sources/catalog.ts index 8eb2d0ffdf..1cfb414a8c 100644 --- a/packages/kilo-vscode/src/diff/sources/catalog.ts +++ b/packages/kilo-vscode/src/diff/sources/catalog.ts @@ -18,6 +18,12 @@ import { import { TURN_PREFIX, createTurnDiffSource, type TurnDiffFetch } from "./turn" import { STAGED_DESCRIPTOR, STAGED_SOURCE_ID, createStagedDiffSource } from "./staged" import { UNSTAGED_DESCRIPTOR, UNSTAGED_SOURCE_ID, createUnstagedDiffSource } from "./unstaged" +import type { WorktreeDiffEntry } from "../../agent-manager/types" + +export interface LocalDiffSource { + summary: (dir: string, base: string) => Promise + file: (dir: string, base: string, file: string, signal?: AbortSignal) => Promise +} export interface WorkspaceBranchesResult { branches: BranchListItem[] @@ -68,7 +74,10 @@ export class DiffSourceCatalog implements vscode.Disposable { private branchGit: GitOps | undefined private branchOutput: vscode.OutputChannel | undefined - constructor(private readonly connection: KiloConnectionService) {} + constructor( + private readonly connection: KiloConnectionService, + private readonly local?: LocalDiffSource, + ) {} listAvailable(ctx: PanelContext): DiffSourceDescriptor[] { if (ctx.hidePicker) return [] @@ -96,6 +105,8 @@ export class DiffSourceCatalog implements vscode.Disposable { ...opts, baseBranchOverride: ctx.baseBranchOverride, baseBranch: ctx.baseBranch, + summary: this.local?.summary, + file: this.local?.file, }) } diff --git a/packages/kilo-vscode/src/diff/sources/worktree.ts b/packages/kilo-vscode/src/diff/sources/worktree.ts index 89e750a1c8..4aa8b690f1 100644 --- a/packages/kilo-vscode/src/diff/sources/worktree.ts +++ b/packages/kilo-vscode/src/diff/sources/worktree.ts @@ -45,6 +45,8 @@ export interface WorktreeDiffSourceOptions { /** Shared GitOps / log so sources don't each spawn their own channel. */ git?: GitOps log?: (...args: unknown[]) => void + summary?: (dir: string, base: string) => Promise + file?: (dir: string, base: string, file: string, signal?: AbortSignal) => Promise } /** @@ -57,6 +59,7 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}): const output = opts.git ? undefined : vscode.window.createOutputChannel("Kilo Diff: Workspace") const log = opts.log ?? ((...args: unknown[]) => appendOutput(output!, "WorktreeDiffSource", ...args)) const git = opts.git ?? new GitOps({ log }) + const controller = new AbortController() const root = (): string | undefined => { const dir = opts.dir?.() @@ -101,7 +104,9 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}): } const status: StatusResolver = async (current, file) => { - const entry = await diffFile(git, current.directory, current.baseBranch, file, log) + const entry = opts.file + ? await opts.file(current.directory, current.baseBranch, file) + : await diffFile(git, current.directory, current.baseBranch, file, log) return entry?.status } @@ -112,7 +117,9 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}): const current = await resolveTarget() if (!current) return { diffs: [] } - const entries = await diffSummary(git, current.directory, current.baseBranch, log) + const entries = opts.summary + ? await opts.summary(current.directory, current.baseBranch) + : await diffSummary(git, current.directory, current.baseBranch, log) const diffs = entries.map(toDiffFile) log(`Diff: ${diffs.length} file(s)`) return { diffs } @@ -124,7 +131,9 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}): if (!current) return null try { - const entry = await diffFile(git, current.directory, current.baseBranch, file, log) + const entry = opts.file + ? await opts.file(current.directory, current.baseBranch, file, controller.signal) + : await diffFile(git, current.directory, current.baseBranch, file, log) if (!entry) return null return toDiffFile(entry) } catch (err) { @@ -152,6 +161,7 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}): // owned by the caller. if (!opts.git) git.dispose() output?.dispose() + controller.abort() target = undefined }, } diff --git a/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts b/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts index fff186c189..39751504d6 100644 --- a/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts +++ b/packages/kilo-vscode/tests/unit/agent-manager-arch.test.ts @@ -29,6 +29,8 @@ const TSX_FILES = [ path.join(ROOT, "webview-ui/agent-manager/ProjectSelect.tsx"), path.join(ROOT, "webview-ui/agent-manager/sortable-tab.tsx"), path.join(ROOT, "webview-ui/agent-manager/DiffPanel.tsx"), + path.join(ROOT, "webview-ui/agent-manager/DiffPanelCache.tsx"), + path.join(ROOT, "webview-ui/agent-manager/review-composers.ts"), path.join(ROOT, "webview-ui/documents/DocumentPanel.tsx"), path.join(ROOT, "webview-ui/diff-viewer/FullScreenDiffView.tsx"), path.join(ROOT, "webview-ui/diff-viewer/ImageDiffView.tsx"), diff --git a/packages/kilo-vscode/tests/unit/agent-manager-worktree-diffs.test.ts b/packages/kilo-vscode/tests/unit/agent-manager-worktree-diffs.test.ts index 5d2e5bc3e1..16bd99d63f 100644 --- a/packages/kilo-vscode/tests/unit/agent-manager-worktree-diffs.test.ts +++ b/packages/kilo-vscode/tests/unit/agent-manager-worktree-diffs.test.ts @@ -65,11 +65,22 @@ describe("createWorktreeDiffs", () => { withDiffs((diffs) => { diffs.onWorktreeDiffLoading({ type: "agentManager.worktreeDiffLoading", sessionId: "s1", loading: true }) expect(diffs.diffLoading()).toBe(true) + expect(diffs.diffLoadingFor(() => "s1")).toBe(true) + diffs.onWorktreeDiff({ type: "agentManager.worktreeDiff", sessionId: "s1", diffs: [] }) + expect(diffs.diffLoadingFor(() => "s1")).toBe(false) diffs.onWorktreeDiffLoading({ type: "agentManager.worktreeDiffLoading", sessionId: "s1", loading: false }) expect(diffs.diffLoading()).toBe(false) }) }) + it("keeps loading isolated to its composite diff id", () => { + withDiffs((diffs) => { + diffs.onWorktreeDiffLoading({ type: "agentManager.worktreeDiffLoading", sessionId: "s1#branch", loading: true }) + expect(diffs.diffLoadingFor(() => "s1#branch")).toBe(true) + expect(diffs.diffLoadingFor(() => "s2#branch")).toBe(false) + }) + }) + it("requestDiffFile marks a file pending, posts once, and ignores repeats", () => { withDiffs((diffs, sent) => { diffs.requestDiffFile("s1", "a.ts") diff --git a/packages/kilo-vscode/tests/unit/local-diff.test.ts b/packages/kilo-vscode/tests/unit/local-diff.test.ts index 68f8e8f89b..1e947e34ae 100644 --- a/packages/kilo-vscode/tests/unit/local-diff.test.ts +++ b/packages/kilo-vscode/tests/unit/local-diff.test.ts @@ -352,6 +352,36 @@ describe("diffFile", () => { }) }) + it("reuses cached detail while the summary stamp is unchanged", async () => { + await withRepo(async (dir, base) => { + await fs.writeFile(path.join(dir, "seed.txt"), "seed\ncached\n") + const local = createLocalDiff(git()) + await local.summary(dir, base) + + const first = await local.file(dir, base, "seed.txt") + const second = await local.file(dir, base, "seed.txt") + + expect(second).toBe(first) + }) + }) + + it("invalidates cached detail after the summary stamp changes", async () => { + await withRepo(async (dir, base) => { + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfirst\n") + const local = createLocalDiff(git()) + await local.summary(dir, base) + const first = await local.file(dir, base, "seed.txt") + + await new Promise((resolve) => setTimeout(resolve, 5)) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nsecond value\n") + await local.summary(dir, base) + const second = await local.file(dir, base, "seed.txt") + + expect(second).not.toBe(first) + expect(second?.after).toBe("seed\nsecond value\n") + }) + }) + it("does not materialize binary detail from a cached summary", async () => { await withRepo(async (dir, base) => { await fs.writeFile(path.join(dir, "tone.wav"), Buffer.from([0x52, 0x49, 0x46, 0x46, 0x00, 0x01, 0x02, 0x03])) diff --git a/packages/kilo-vscode/tests/unit/semaphore.test.ts b/packages/kilo-vscode/tests/unit/semaphore.test.ts index 697ba146b2..dc64b8b0e5 100644 --- a/packages/kilo-vscode/tests/unit/semaphore.test.ts +++ b/packages/kilo-vscode/tests/unit/semaphore.test.ts @@ -69,6 +69,22 @@ describe("Semaphore", () => { expect(order).toEqual([1, 2, 3]) }) + it("removes an aborted task from the pending queue", async () => { + const sem = new Semaphore(1) + let release: () => void = () => {} + const first = sem.run(() => new Promise((resolve) => (release = resolve))) + const controller = new AbortController() + const aborted = sem.run(async () => "aborted", controller.signal) + const next = sem.run(async () => "next") + + controller.abort(new Error("cancelled")) + await expect(aborted).rejects.toThrow("cancelled") + release() + + expect(await next).toBe("next") + await first + }) + it("allows full concurrency when limit exceeds task count", async () => { const sem = new Semaphore(10) let running = 0 diff --git a/packages/kilo-vscode/tests/unit/source-controller.test.ts b/packages/kilo-vscode/tests/unit/source-controller.test.ts index 3c14910e70..c08673ced3 100644 --- a/packages/kilo-vscode/tests/unit/source-controller.test.ts +++ b/packages/kilo-vscode/tests/unit/source-controller.test.ts @@ -346,6 +346,36 @@ describe("SourceController.requestFile", () => { controller.stop() }) + it("does not start queued detail work after the source changes", async () => { + let details = 0 + const workspace: DiffSource = { + descriptor: WORKSPACE_DESC, + async fetch() { + return { diffs: [] } + }, + async fetchFile() { + details++ + return null + }, + } + const session: DiffSource = { + descriptor: SESSION_DESC, + async fetch() { + return { diffs: [] } + }, + } + const { controller } = make({ workspace, "session:s1": session }) + + controller.setContext({ workspaceRoot: "/repo", sessionId: "s1" }) + await controller.activate("workspace") + const request = controller.requestFile("foo.ts") + await controller.activate("session:s1") + await request + + expect(details).toBe(0) + controller.stop() + }) + it("posts null when a pending fetchFile result is invalidated by stop", async () => { let release: () => void = () => {} const workspace: DiffSource = { diff --git a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx index ff426db114..a0a2ffeb9d 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx @@ -162,14 +162,14 @@ import { import { createEmbeddedTerminalReader } from "./terminal/output" import { focusCurrentTab, renderTab, renderTerminalLayer, renderNewTabButton } from "./tab-rendering" import { useTabScroll } from "./tab-scroll" -import { DiffPanel } from "./DiffPanel" +import { DiffPanelCache } from "./DiffPanelCache" import { PRPanelHost } from "./pr/PRPanelHost" import { createRevertFile } from "./revert-file" import { FullScreenDiffView } from "../diff-viewer/FullScreenDiffView" import { createApplyToLocal } from "./apply-to-local" import { createWorktreeDiffs, wireDiffId } from "./worktree-diffs" import type { ReviewComment } from "../diff-viewer/review-comments" -import { clearReviewComposer, createReviewComposer } from "../diff-viewer/review-annotations" +import { createReviewComposers } from "./review-composers" import type { SidebarSearchMenuRef } from "./SidebarSearchMenu" import { createSidebarSearch, type SidebarSearchItem } from "./sidebar-search" import { randomColor } from "./section-colors" @@ -309,6 +309,7 @@ const AgentManagerContent: Component = () => { let pendingSidebarWidth: number | undefined const [history, setHistory] = createSignal(false) const [sidePanel, setSidePanel] = createSignal(null) + const [diffMounted, setDiffMounted] = createSignal(false) const diffOpen = () => sidePanel() === SidePanel.Diff const prOpen = () => sidePanel() === SidePanel.PR const activePR = createMemo(() => { @@ -319,6 +320,7 @@ const AgentManagerContent: Component = () => { return { pr, selected, wt: worktrees().find((w) => w.id === selected) } }) const diffs = createWorktreeDiffs(vscode, activeProjectId) + createEffect(on(activeProjectId, diffs.reset, { defer: true })) const diffDatas = diffs.diffDatas const diffLoading = diffs.diffLoading const setDiffLoading = diffs.setDiffLoading @@ -330,7 +332,7 @@ const AgentManagerContent: Component = () => { setReviewActive(false) setSidePanel(SidePanel.Terminal) } - const reviewComposer = createReviewComposer() + const composers = createReviewComposers(currentProjectId) const reviewState = createReviewState() const reviewOpenByContext = reviewState.open const setReviewOpenByContext = reviewState.setOpen @@ -524,7 +526,6 @@ const AgentManagerContent: Component = () => { setPendingDelete(null) } createEffect(on(selection, () => cancelPendingDelete(), { defer: true })) - createEffect(on(selection, () => clearReviewComposer(reviewComposer), { defer: true })) createEffect( on( selection, @@ -1752,9 +1753,12 @@ const AgentManagerContent: Component = () => { } const diffFileLoadingForCurrent = createMemo(() => diffs.diffFileLoadingFor(diffScopeId)) + const diffLoadingForCurrent = createMemo(() => diffs.diffLoadingFor(diffScopeId)) const revertCtl = createRevertFile(diffScopeId, diffCtx, () => review.scope(), vscode, showToast, t, activeProjectId) + createEffect(() => diffOpen() && setDiffMounted(true)) + const handleShowKeyboardShortcuts = () => { const categories = buildShortcutCategories(kb(), t) dialog.show(() => ( @@ -2603,7 +2607,9 @@ const AgentManagerContent: Component = () => { mounted while a side terminal is alive — hidden via .am-side-host-hidden (absolute + opacity), never unmounted, so xterm render loops keep streaming. */} - 0 || subagents.tabs().length > 0}> + 0 || subagents.tabs().length > 0} + >
{ />
- - metrics.track("send_review_comments", "side_review")} - onClose={metrics.click("side_review_close", "side_review", () => setSidePanel(null))} - onExpand={ - selection() !== null - ? metrics.click("fullscreen_review", "side_review", openReviewTab, { action: "open" }) - : undefined - } - onRequestDiff={requestDiffFile} - onOpenFile={(file, line) => { - const id = diffCtx() - if (id) - vscode.postMessage({ type: "agentManager.openFile", sessionId: id, filePath: file, line }) - }} - onOpenDocument={documentInspector.open} - onRevertFile={metrics.use("revert_file", "side_review", revertCtl.revert)} - revertingFiles={revertCtl.reverting()} - activeTerminalId={terms.activeId()} - /> - + diffOpen() && !history() && !reviewActive()} + data={diffDatas} + loading={(key) => diffs.diffLoadingFor(() => key)} + loadingFiles={(key) => diffs.diffFileLoadingFor(() => key)} + notice={(key) => diffNotices()[key]} + comments={(ctx) => + readReviewComments(reviewCommentsByContext(), currentProjectId() ?? "single", ctx) + } + setComments={(ctx, comments) => + setReviewCommentsByContext((prev) => + setReviewComments(prev, currentProjectId() ?? "single", ctx, comments), + ) + } + composer={composers.get} + lead={() => diffScopeControls(true)} + canRevert={scopeCapabilities(review.scope()).revert} + diffStyle={diffStyle.style()} + onDiffStyleChange={setSharedDiffStyle} + markdownRender={markdown.render()} + onMarkdownRenderChange={markdown.update} + onSendClick={() => metrics.track("send_review_comments", "side_review")} + onClose={metrics.click("side_review_close", "side_review", () => setSidePanel(null))} + onExpand={ + selection() !== null + ? metrics.click("fullscreen_review", "side_review", openReviewTab, { action: "open" }) + : undefined + } + onRequestDiff={diffs.requestDiffFile} + onOpenFile={(ctx, file, line) => + vscode.postMessage({ type: "agentManager.openFile", sessionId: ctx, filePath: file, line }) + } + onOpenDocument={documentInspector.open} + onRevertFile={(key, ctx, file) => { + metrics.track("revert_file", "side_review") + revertCtl.revertFor(key, ctx, review.scope(), file) + }} + revertingFiles={revertCtl.revertingFor} + activeTerminalId={terms.activeId()} + /> {
{ canComment={scopeCapabilities(review.scope()).comments} comments={reviewComments()} onCommentsChange={setReviewCommentsForSelection} - composer={reviewComposer} + composer={composers.get(`${activeProjectId() ?? "single"}\0${diffScopeId() ?? ""}`)} onSendAll={closeReviewTab} onSendClick={() => metrics.track("send_review_comments", "fullscreen_review")} diffStyle={diffStyle.style()} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx index 9513aacae6..803cbd45ea 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanel.tsx @@ -8,7 +8,6 @@ import { DiffChanges } from "@kilocode/kilo-ui/diff-changes" import { Icon } from "@kilocode/kilo-ui/icon" import { Button } from "@kilocode/kilo-ui/button" import { IconButton } from "@kilocode/kilo-ui/icon-button" -import { Spinner } from "@kilocode/kilo-ui/spinner" import { Tooltip, TooltipKeybind } from "@kilocode/kilo-ui/tooltip" import type { DiffLineAnnotation, AnnotationSide, SelectedLineRange } from "@pierre/diffs" import type { WorktreeFileDiff } from "../src/types/messages" @@ -74,6 +73,7 @@ const DIFF_NOTICE_KEYS: Record = { interface DiffPanelProps { diffs: WorktreeFileDiff[] loading: boolean + active?: boolean loadingFiles?: Set sessionId?: string sessionKey?: string @@ -264,7 +264,7 @@ export const DiffPanel: Component = (props) => { diffs: () => props.diffs, open, loading: () => props.loadingFiles, - send: () => props.onRequestDiff, + send: () => (props.active === false ? undefined : props.onRequestDiff), }) // --- CRUD --- @@ -327,6 +327,7 @@ export const DiffPanel: Component = (props) => { on( () => [props.diffs, comments()] as const, ([diffs, current]) => { + if (props.active === false) return const valid = sanitizeReviewComments(current, diffs) if (valid.length !== current.length) { setComments(valid) @@ -553,7 +554,6 @@ export const DiffPanel: Component = (props) => {
- {t("session.review.loadingChanges")}
@@ -714,10 +714,7 @@ export const DiffPanel: Component = (props) => { fallback={
Diff preview loads on demand.}> - <> - - Loading diff... - + Loading diff...
} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanelCache.tsx b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanelCache.tsx new file mode 100644 index 0000000000..e6890125d8 --- /dev/null +++ b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanelCache.tsx @@ -0,0 +1,110 @@ +import { For, createEffect, createMemo, createSignal, type Accessor, type Component, type JSX } from "solid-js" +import type { WorktreeFileDiff } from "../src/types/messages" +import type { ReviewComment } from "../diff-viewer/review-comments" +import type { ReviewComposer } from "../diff-viewer/review-annotations" +import { DiffPanel } from "./DiffPanel" + +const CACHE_SIZE = 4 + +interface Entry { + key: string + cacheKey: string + ctx: string + used: number +} + +interface Props { + current: Accessor + context: Accessor + project: Accessor + active: Accessor + data: Accessor> + loading: (key: string) => boolean + loadingFiles: (key: string) => Set + notice: (key: string) => string | undefined + comments: (ctx: string) => ReviewComment[] + setComments: (ctx: string, comments: ReviewComment[]) => void + composer: (key: string) => ReviewComposer + lead: () => JSX.Element + canRevert: boolean + diffStyle: "unified" | "split" + onDiffStyleChange: (style: "unified" | "split") => void + markdownRender: boolean + onMarkdownRenderChange: (render: boolean) => void + onSendClick: () => void + onClose: () => void + onExpand?: () => void + onRequestDiff: (key: string, file: string) => void + onOpenFile: (ctx: string, file: string, line?: number) => void + onOpenDocument: (file: string) => void + onRevertFile: (key: string, ctx: string, file: string) => void + revertingFiles: (key: string) => Set + activeTerminalId?: string +} + +export const DiffPanelCache: Component = (props) => { + const [entries, setEntries] = createSignal([]) + let used = 0 + + createEffect(() => { + if (!props.active()) return + const key = props.current() + const ctx = props.context() + const project = props.project() ?? "single" + if (!key || !ctx) return + const cacheKey = `${project}\0${key}` + setEntries((prev) => { + const prefix = `${project}\0` + const scoped = prev.filter((item) => item.cacheKey.startsWith(prefix)) + const current = scoped.find((item) => item.cacheKey === cacheKey) + if (current) { + current.used = ++used + return scoped + } + const next = [...scoped, { key, cacheKey, ctx, used: ++used }] + if (next.length <= CACHE_SIZE) return next + const oldest = next.reduce((entry, item) => (item.used < entry.used ? item : entry)) + return next.filter((item) => item !== oldest) + }) + }) + + return ( + + {(entry) => { + const active = createMemo( + () => props.active() && `${props.project() ?? "single"}\0${props.current()}` === entry.cacheKey, + ) + return ( +
+ props.setComments(entry.ctx, comments)} + composer={props.composer(entry.cacheKey)} + onSendClick={props.onSendClick} + onClose={props.onClose} + onExpand={props.onExpand} + onRequestDiff={(file) => props.onRequestDiff(entry.key, file)} + onOpenFile={(file, line) => props.onOpenFile(entry.ctx, file, line)} + onOpenDocument={props.onOpenDocument} + onRevertFile={(file) => props.onRevertFile(entry.key, entry.ctx, file)} + revertingFiles={props.revertingFiles(entry.key)} + activeTerminalId={props.activeTerminalId} + /> +
+ ) + }} + + ) +} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/agent-manager.css b/packages/kilo-vscode/webview-ui/agent-manager/agent-manager.css index ab10cd505d..0c8f0570d3 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/agent-manager.css +++ b/packages/kilo-vscode/webview-ui/agent-manager/agent-manager.css @@ -1939,6 +1939,24 @@ body.am-wt-dragging-active * { overflow: hidden; } +.am-diff-panel-cache { + position: absolute; + inset: 0; + display: flex; + min-width: 0; + min-height: 0; + opacity: 0; + pointer-events: none; + z-index: 0; + background: var(--surface-base); +} + +.am-diff-panel-cache-active { + opacity: 1; + pointer-events: auto; + z-index: 2; +} + .am-diff-panel-wrapper > [data-component="resize-handle"]::after { background: var(--surface-interactive-base); } diff --git a/packages/kilo-vscode/webview-ui/agent-manager/revert-file.ts b/packages/kilo-vscode/webview-ui/agent-manager/revert-file.ts index 3f2a528518..fa2d340fda 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/revert-file.ts +++ b/packages/kilo-vscode/webview-ui/agent-manager/revert-file.ts @@ -28,9 +28,9 @@ export function createRevertFile( return files()[id] ?? new Set() }) - function revert(file: string) { - const id = diffScopeId() - const context = ctx() + const revertingFor = (id: string) => files()[id] ?? new Set() + + function revertFor(id: string | undefined, context: string | undefined, source: string, file: string) { if (!id || !context) return setFiles((prev) => { const set = new Set(prev[id] ?? []) @@ -42,10 +42,14 @@ export function createRevertFile( projectId: projectId?.(), sessionId: context, file, - scope: scope(), + scope: source, }) } + function revert(file: string) { + revertFor(diffScopeId(), ctx(), scope(), file) + } + function onResult(ev: AgentManagerRevertWorktreeFileResultMessage) { setFiles((prev) => { const set = new Set(prev[ev.sessionId] ?? []) @@ -62,5 +66,5 @@ export function createRevertFile( } } - return { reverting, revert, onResult } + return { reverting, revertingFor, revert, revertFor, onResult } } diff --git a/packages/kilo-vscode/webview-ui/agent-manager/review-composers.ts b/packages/kilo-vscode/webview-ui/agent-manager/review-composers.ts new file mode 100644 index 0000000000..2901c323ba --- /dev/null +++ b/packages/kilo-vscode/webview-ui/agent-manager/review-composers.ts @@ -0,0 +1,24 @@ +import type { Accessor } from "solid-js" +import { createReviewComposer, type ReviewComposer } from "../diff-viewer/review-annotations" + +export function createReviewComposers(project: Accessor) { + const values = new Map() + + const get = (key: string) => { + const current = values.get(key) + if (current) return current + const next = createReviewComposer() + values.set(key, next) + return next + } + + const clear = (ctx: string | null) => { + if (!ctx) return + const prefix = `${project() ?? "single"}\0${ctx}` + for (const key of values.keys()) { + if (key === prefix || key.startsWith(`${prefix}#`)) values.delete(key) + } + } + + return { get, clear } +} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/worktree-diffs.ts b/packages/kilo-vscode/webview-ui/agent-manager/worktree-diffs.ts index 60ba7cb599..1de9f56219 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/worktree-diffs.ts +++ b/packages/kilo-vscode/webview-ui/agent-manager/worktree-diffs.ts @@ -34,10 +34,18 @@ export function createWorktreeDiffs( project: () => string | undefined = () => undefined, ) { const [diffDatas, setDiffDatas] = createSignal>({}) - const [diffLoading, setDiffLoading] = createSignal(false) + const [diffLoadings, setDiffLoadings] = createSignal>({}) + const diffLoading = () => Object.keys(diffLoadings()).length > 0 const [diffNotices, setDiffNotices] = createSignal>({}) const [diffFileLoading, setDiffFileLoading] = createSignal>>({}) + const reset = () => { + setDiffDatas({}) + setDiffLoadings({}) + setDiffNotices({}) + setDiffFileLoading({}) + } + const setDiffFilePending = (sessionId: string, file: string, value: boolean) => { setDiffFileLoading((prev) => { const session = prev[sessionId] ?? {} @@ -93,6 +101,13 @@ export function createWorktreeDiffs( return new Set(Object.keys(diffFileLoading()[id] ?? {})) } + /** Initial summary loading for one composite diff id. Cached results stay visible while refreshing. */ + const diffLoadingFor = (sessionId: Accessor) => { + const id = sessionId() + if (!id) return false + return diffLoadings()[id] === true && !(id in diffDatas()) + } + // Backend messages. const onWorktreeDiff = (ev: AgentManagerWorktreeDiffMessage) => { @@ -122,7 +137,18 @@ export function createWorktreeDiffs( } const onWorktreeDiffLoading = (ev: AgentManagerWorktreeDiffLoadingMessage) => { - setDiffLoading(ev.loading) + // One source is active per project. Replacing the map on start also clears + // an interrupted source whose stale completion is intentionally discarded. + if (ev.loading) { + setDiffLoadings({ [ev.sessionId]: true }) + return + } + setDiffLoadings((prev) => { + if (!prev[ev.sessionId]) return prev + const next = { ...prev } + delete next[ev.sessionId] + return next + }) } const onWorktreeDiffNotice = (ev: AgentManagerWorktreeDiffNoticeMessage) => { @@ -132,11 +158,13 @@ export function createWorktreeDiffs( return { diffDatas, diffLoading, - setDiffLoading, + setDiffLoading: (loading: boolean) => setDiffLoadings(loading ? diffLoadings() : {}), diffNotices, requestDiffFile, refreshStaleDiffs, diffFileLoadingFor, + diffLoadingFor, + reset, onWorktreeDiff, onWorktreeDiffFile, onWorktreeDiffLoading, diff --git a/packages/kilo-vscode/webview-ui/diff-viewer/diff-requests.ts b/packages/kilo-vscode/webview-ui/diff-viewer/diff-requests.ts index 904ceca13f..d76728ebe5 100644 --- a/packages/kilo-vscode/webview-ui/diff-viewer/diff-requests.ts +++ b/packages/kilo-vscode/webview-ui/diff-viewer/diff-requests.ts @@ -36,7 +36,7 @@ export function createDiffRequests(opts: DiffRequestOptions) { createEffect( on( - () => [opts.open(), opts.diffs(), opts.loading()] as const, + () => [opts.open(), opts.diffs(), opts.loading(), opts.send()] as const, ([open, diffs]) => { const files = new Set(open) for (const file of requested.keys()) {