From b382a3b838e317acc284442361b9bae2f39c992e Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Thu, 27 Aug 2026 12:23:34 +0200 Subject: [PATCH] fix(vscode): keep large worktree reviews responsive --- .../cache-agent-manager-diff-batches.md | 6 + packages/kilo-ui/src/components/diff.tsx | 7 +- packages/kilo-ui/src/pierre/index.ts | 1 + .../kilo-vscode/src/agent-manager/GitOps.ts | 41 +- .../src/agent-manager/local-diff-batch.ts | 203 +++++++ .../src/agent-manager/local-diff-cache.ts | 288 +++++++++ .../src/agent-manager/local-diff.ts | 308 ++++------ .../src/agent-manager/semaphore.ts | 18 +- .../agent-manager/worktree-diff-controller.ts | 7 +- .../kilo-vscode/src/diff/sources/worktree.ts | 11 + .../tests/diff-scroll-preservation.spec.ts | 142 +++++ .../unit/agent-manager-diff-state.test.ts | 7 + .../unit/agent-manager-worktree-diffs.test.ts | 62 ++ .../tests/unit/diff-preview-request.test.ts | 32 + .../tests/unit/diff-source-catalog.test.ts | 39 +- .../kilo-vscode/tests/unit/git-ops.test.ts | 29 + .../kilo-vscode/tests/unit/local-diff.test.ts | 560 +++++++++++++++++- .../kilo-vscode/tests/unit/semaphore.test.ts | 40 ++ .../unit/worktree-diff-controller.test.ts | 166 +++++- .../tests/visual-regression.spec.mts | 4 + .../tests/visual-regression.spec.ts | 4 + .../agent-manager/AgentManagerApp.tsx | 7 +- .../webview-ui/agent-manager/DiffPanel.tsx | 9 +- .../agent-manager/DiffPanelCache.tsx | 2 +- .../agent-manager/agent-manager-review.css | 5 - .../agent-manager/agent-manager.css | 7 +- .../agent-manager/worktree-diffs.ts | 41 ++ .../webview-ui/diff-viewer/FileTree.tsx | 178 +++--- .../diff-viewer/FullScreenDiffView.tsx | 13 +- .../diff-viewer/diff-open-policy.ts | 5 +- .../webview-ui/diff-viewer/diff-requests.ts | 50 +- .../src/stories/agent-manager.stories.tsx | 224 +++++++ 32 files changed, 2173 insertions(+), 343 deletions(-) create mode 100644 .changeset/cache-agent-manager-diff-batches.md create mode 100644 packages/kilo-vscode/src/agent-manager/local-diff-batch.ts create mode 100644 packages/kilo-vscode/src/agent-manager/local-diff-cache.ts diff --git a/.changeset/cache-agent-manager-diff-batches.md b/.changeset/cache-agent-manager-diff-batches.md new file mode 100644 index 0000000000..c324daaf4a --- /dev/null +++ b/.changeset/cache-agent-manager-diff-batches.md @@ -0,0 +1,6 @@ +--- +"kilo-code": patch +"@kilocode/kilo-ui": patch +--- + +Load Agent Manager changes faster and preserve cached diffs when switching sessions. diff --git a/packages/kilo-ui/src/components/diff.tsx b/packages/kilo-ui/src/components/diff.tsx index be1307cecf..941fa87202 100644 --- a/packages/kilo-ui/src/components/diff.tsx +++ b/packages/kilo-ui/src/components/diff.tsx @@ -196,12 +196,13 @@ export function Diff(props: DiffProps) { "selectedLines", "commentedLines", "onRendered", + "visible", "virtualized", "sizeKey", ]) const mobile = createMediaQuery("(max-width: 640px)") - const [visible, setVisible] = createSignal(false) + const [visible, setVisible] = createSignal(local.visible === true) const before = createMemo(() => { if (local.fileDiff) return local.fileDiff.deletionLines.join("") @@ -277,6 +278,10 @@ export function Diff(props: DiffProps) { createEffect(() => { if (visible()) return + if (local.visible) { + setVisible(true) + return + } const cleanup = observe(container, () => setVisible(true)) onCleanup(cleanup) }) diff --git a/packages/kilo-ui/src/pierre/index.ts b/packages/kilo-ui/src/pierre/index.ts index 988835e6a2..3103d1dff3 100644 --- a/packages/kilo-ui/src/pierre/index.ts +++ b/packages/kilo-ui/src/pierre/index.ts @@ -53,6 +53,7 @@ type DiffShared = FileDiffOptions & { commentedLines?: SelectedLineRange[] onLineNumberSelectionEnd?: (selection: SelectedLineRange | null) => void onRendered?: () => void + visible?: boolean // When false, render the supplied diff once instead of row-virtualizing it. // Callers should supply hunk-bounded `fileDiff`/`patch` data for large source // files so eager rendering does not expand full before/after content. diff --git a/packages/kilo-vscode/src/agent-manager/GitOps.ts b/packages/kilo-vscode/src/agent-manager/GitOps.ts index e31ebc00c7..4c77064024 100644 --- a/packages/kilo-vscode/src/agent-manager/GitOps.ts +++ b/packages/kilo-vscode/src/agent-manager/GitOps.ts @@ -45,6 +45,7 @@ interface ExecOptions { stdin?: string timeout?: number signal?: AbortSignal + priority?: boolean } export interface ExecResult { @@ -595,11 +596,19 @@ 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; signal?: AbortSignal }): Promise { + execGit( + args: string[], + cwd: string, + options?: { stdin?: string; signal?: AbortSignal; priority?: boolean }, + ): Promise { return this.exec(args, cwd, options) } - execGitBuffer(args: string[], cwd: string, options?: { signal?: AbortSignal }): Promise { + execGitBuffer( + args: string[], + cwd: string, + options?: { stdin?: string; signal?: AbortSignal; priority?: boolean }, + ): Promise { return this.execBuffer(args, cwd, options) } @@ -609,32 +618,40 @@ export class GitOps { } private async execBuffer(args: string[], cwd: string, options?: ExecOptions): Promise { - if (this.controller.signal.aborted) { + if (this.controller.signal.aborted || options?.signal?.aborted) { return { code: 1, stdout: Buffer.alloc(0), stderr: "GitOps disposed" } } - const cmd = await this.executable().catch(() => undefined) - if (!cmd || this.controller.signal.aborted) { + const cmd = await this.executable(options?.signal).catch(() => undefined) + if (!cmd || this.controller.signal.aborted || options?.signal?.aborted) { 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, options?.signal) : invoke() + return this.semaphore ? this.semaphore.run(invoke, options?.signal, options?.priority) : invoke() } - private executable(): Promise { + private executable(cancel?: AbortSignal): Promise { const signal = this.controller.signal - if (signal.aborted) return Promise.reject(new Error("GitOps disposed")) + if (signal.aborted || cancel?.aborted) return Promise.reject(new Error("GitOps disposed")) return new Promise((resolve, reject) => { - const onAbort = () => reject(new Error("GitOps disposed")) - signal.addEventListener("abort", onAbort, { once: true }) + const clear = () => { + signal.removeEventListener("abort", abort) + cancel?.removeEventListener("abort", abort) + } + const abort = () => { + clear() + reject(new Error("GitOps disposed")) + } + signal.addEventListener("abort", abort, { once: true }) + cancel?.addEventListener("abort", abort, { once: true }) const cache = (this.executableCache ??= Promise.resolve().then(() => this.binary())) cache.then( (value) => { - signal.removeEventListener("abort", onAbort) + clear() resolve(value) }, (err) => { - signal.removeEventListener("abort", onAbort) + clear() if (this.executableCache === cache) this.executableCache = undefined reject(err) }, diff --git a/packages/kilo-vscode/src/agent-manager/local-diff-batch.ts b/packages/kilo-vscode/src/agent-manager/local-diff-batch.ts new file mode 100644 index 0000000000..14399b5d66 --- /dev/null +++ b/packages/kilo-vscode/src/agent-manager/local-diff-batch.ts @@ -0,0 +1,203 @@ +import * as fs from "fs/promises" +import { imageMime } from "../diff/shared/image" +import { resolveInside } from "../diff/shared/path" +import type { GitOps } from "./GitOps" +import type { WorktreeDiffEntry } from "./types" + +export type Meta = { + file: string + additions: number + deletions: number + status: "added" | "deleted" | "modified" + tracked: boolean + generatedLike: boolean + binary: boolean + stamp: string +} + +export type Batch = { + entries: Map + deferred: Set +} + +type Base = { id: string; bytes: number } + +export const MAX_DETAIL_BYTES = 20_000_000 +const MAX_BATCH_BYTES = 32 * 1024 * 1024 + +export function check(signal?: AbortSignal) { + if (signal?.aborted) throw new Error("Diff detail aborted") +} + +export function summarize(meta: Meta): WorktreeDiffEntry { + const image = imageMime(meta.file) !== undefined + return { + file: meta.file, + patch: "", + before: "", + after: "", + additions: meta.additions, + deletions: meta.deletions, + status: meta.status, + tracked: meta.tracked, + generatedLike: meta.generatedLike, + summarized: image || !meta.binary, + stamp: meta.stamp, + kind: image ? "image" : undefined, + } +} + +export async function fileSize(dir: string, file: string): Promise { + const full = resolveInside(dir, file) + if (!full) return 0 + const stat = await fs.lstat(full).catch(() => undefined) + return stat?.size ?? 0 +} + +export async function readAfter(dir: string, file: string, status: Meta["status"]): Promise { + if (status === "deleted") return "" + const full = resolveInside(dir, file) + if (!full) throw new Error(`Could not resolve working file for ${file}`) + const stat = await fs.lstat(full).catch(() => undefined) + if (!stat) throw new Error(`Could not read working file for ${file}`) + if (stat.isSymbolicLink()) return fs.readlink(full).catch(() => "") + if (!stat.isFile()) throw new Error(`Working path is not a file: ${file}`) + return fs.readFile(full, "utf-8").catch(() => { + throw new Error(`Could not read working file for ${file}`) + }) +} + +async function inspect(git: GitOps, dir: string, anc: string, metas: Meta[]) { + const items = metas.filter((meta) => meta.status !== "added") + const result = new Map() + if (items.length === 0) return result + const stdin = items.map((meta) => `${anc}:${meta.file}\n`).join("") + const output = await git.execGit(["cat-file", "--batch-check"], dir, { stdin, priority: true }) + if (output.code !== 0) throw new Error("Could not inspect base files") + const lines = output.stdout.trimEnd().split("\n") + if (lines.length !== items.length) throw new Error("Incomplete base file metadata") + for (const [index, meta] of items.entries()) { + const [id, type, value] = lines[index]!.split(" ") + const bytes = Number(value) + if (!id || type !== "blob" || !Number.isSafeInteger(bytes) || bytes < 0) { + throw new Error(`Could not inspect base file for ${meta.file}`) + } + result.set(meta.file, { id, bytes }) + } + return result +} + +async function blobs(git: GitOps, dir: string, metas: Meta[], base: Map) { + const items = metas.filter((meta) => meta.status !== "added") + const result = new Map() + if (items.length === 0) return result + const stdin = items.map((meta) => `${base.get(meta.file)!.id}\n`).join("") + const output = await git.execGitBuffer(["cat-file", "--batch"], dir, { stdin, priority: true }) + if (output.code !== 0) throw new Error("Could not read base files") + let offset = 0 + for (const meta of items) { + const end = output.stdout.indexOf(10, offset) + if (end === -1) throw new Error(`Incomplete base file for ${meta.file}`) + const [id, type, value] = output.stdout.subarray(offset, end).toString("utf8").split(" ") + const size = Number(value) + const expected = base.get(meta.file)! + const next = end + 1 + size + if (id !== expected.id || type !== "blob" || size !== expected.bytes || output.stdout[next] !== 10) { + throw new Error(`Invalid base file for ${meta.file}`) + } + result.set(meta.file, output.stdout.subarray(end + 1, next)) + offset = next + 1 + } + return result +} + +async function patches(git: GitOps, dir: string, anc: string, metas: Meta[]) { + const result = new Map() + if (metas.length === 0) return result + const output = await git.execGit( + [ + "-c", + "core.quotepath=false", + "diff", + "--no-ext-diff", + "--no-renames", + anc, + "--", + ...metas.map((meta) => meta.file), + ], + dir, + { priority: true }, + ) + if (output.code !== 0) throw new Error("Could not create file diffs") + const names = new Map(metas.map((meta) => [`diff --git a/${meta.file} b/${meta.file}`, meta.file])) + for (const patch of output.stdout.split(/(?=^diff --git )/m)) { + if (!patch) continue + const file = names.get(patch.slice(0, patch.indexOf("\n"))) + if (!file) throw new Error("Could not match a file diff") + result.set(file, patch) + } + return result +} + +export async function collect( + git: GitOps, + dir: string, + anc: string, + metas: Meta[], + log?: (...args: unknown[]) => void, +): Promise { + const entries = new Map() + const deferred = new Set() + if (metas.length === 0) return { entries, deferred } + const [base, sizes] = await Promise.all([ + inspect(git, dir, anc, metas), + Promise.all( + metas.map(async (meta) => [meta.file, meta.status === "deleted" ? 0 : await fileSize(dir, meta.file)] as const), + ), + ]) + const working = new Map(sizes) + const active: Meta[] = [] + let total = 0 + for (const meta of metas) { + const before = base.get(meta.file)?.bytes ?? 0 + const after = working.get(meta.file) ?? 0 + if (before > MAX_DETAIL_BYTES || after > MAX_DETAIL_BYTES) { + log?.("diffFile: file too large for detail view, returning summarized entry", { + file: meta.file, + beforeBytes: before, + afterBytes: after, + cap: MAX_DETAIL_BYTES, + }) + entries.set(meta.file, summarize(meta)) + continue + } + if (total + before + after > MAX_BATCH_BYTES) { + deferred.add(meta.file) + continue + } + total += before + after + active.push(meta) + } + const [before, diffs, after] = await Promise.all([ + blobs(git, dir, active, base), + patches(git, dir, anc, active), + Promise.all(active.map(async (meta) => [meta.file, await readAfter(dir, meta.file, meta.status)] as const)), + ]) + const values = new Map(after) + for (const meta of active) { + const value = values.get(meta.file) + const patch = diffs.get(meta.file) + if (value === undefined || patch === undefined || (meta.status !== "added" && !before.has(meta.file))) { + entries.set(meta.file, null) + continue + } + entries.set(meta.file, { + ...summarize(meta), + before: before.get(meta.file)?.toString("utf8") ?? "", + after: value, + patch, + summarized: false, + }) + } + return { entries, deferred } +} diff --git a/packages/kilo-vscode/src/agent-manager/local-diff-cache.ts b/packages/kilo-vscode/src/agent-manager/local-diff-cache.ts new file mode 100644 index 0000000000..10d6231494 --- /dev/null +++ b/packages/kilo-vscode/src/agent-manager/local-diff-cache.ts @@ -0,0 +1,288 @@ +import { imageMime } from "../diff/shared/image" +import type { Batch, Meta } from "./local-diff-batch" +import type { WorktreeDiffEntry } from "./types" + +type Value = WorktreeDiffEntry | null + +type Loader = { + summary: ( + dir: string, + base: string, + ) => Promise<{ anc: string; metas: Meta[]; entries: WorktreeDiffEntry[] } | undefined> + file: (dir: string, base: string, path: string, signal?: AbortSignal) => Promise + detail: (dir: string, anc: string, meta: Meta, signal?: AbortSignal) => Promise + batch: (dir: string, anc: string, metas: Meta[]) => Promise + log?: (...args: unknown[]) => void +} + +type Call = { active: boolean; signal?: AbortSignal } +type Item = { + id: string + scope: string + queue: string + dir: string + base: string + anc: string + meta: Meta + calls: Set + started: boolean + work: Promise + resolve: (value: Value) => void + reject: (error: unknown) => void +} + +const MAX_BATCH_FILES = 16 + +export function createDiffCache(load: Loader) { + const states = new Map }>() + const generations = new Map() + const details = new Map() + const pending = new Map; item?: Item }>() + const queues = new Map>() + let bytes = 0 + + const remember = (id: string, value: WorktreeDiffEntry) => { + const size = [value.before, value.after, value.patch, value.image?.before?.data, value.image?.after?.data].reduce( + (sum, value) => sum + Buffer.byteLength(value ?? ""), + 0, + ) + bytes -= details.get(id)?.bytes ?? 0 + 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) + } + } + + const identity = (dir: string, base: string, anc: string, meta: Meta) => + `${dir}\0${base}\0${anc}\0${meta.file}\0${meta.tracked}\0${meta.status}\0${meta.additions}\0${meta.deletions}\0${meta.binary}\0${meta.stamp}` + + const cached = (id: string) => { + const value = details.get(id) + if (!value) return undefined + details.delete(id) + details.set(id, value) + return value.value + } + + const watch = (id: string, work: Promise, valid: () => boolean) => { + pending.set(id, { work }) + work.then( + (value) => { + if (pending.get(id)?.work !== work) return + pending.delete(id) + if (!value || !valid()) return + if (value.image?.before?.error === "unreadable" || value.image?.after?.error === "unreadable") return + remember(id, value) + }, + () => { + if (pending.get(id)?.work === work) pending.delete(id) + }, + ) + return work + } + + const subscribe = (work: Promise, signal?: AbortSignal, cancel?: () => void): Promise => { + if (signal?.aborted) return Promise.reject(new Error("Diff detail aborted")) + return new Promise((resolve, reject) => { + let done = false + const abort = () => { + if (done) return + done = true + signal?.removeEventListener("abort", abort) + cancel?.() + reject(new Error("Diff detail aborted")) + } + signal?.addEventListener("abort", abort, { once: true }) + work.then( + (value) => { + if (done) return + done = true + signal?.removeEventListener("abort", abort) + resolve(value) + }, + (error) => { + if (done) return + done = true + signal?.removeEventListener("abort", abort) + reject(error) + }, + ) + }) + } + + const join = (item: Item, signal?: AbortSignal) => { + if (signal?.aborted) return Promise.reject(new Error("Diff detail aborted")) + const call: Call = { active: true, signal } + item.calls.add(call) + const work = subscribe(item.work, signal, () => { + if (!call.active) return + call.active = false + item.calls.delete(call) + if (item.started || item.calls.size !== 0) return + const queue = queues.get(item.queue) + if (queue?.get(item.id) === item) queue.delete(item.id) + if (queue?.size === 0) queues.delete(item.queue) + if (pending.get(item.id)?.work === item.work) pending.delete(item.id) + item.reject(new Error("Diff detail aborted")) + }) + const finish = () => { + call.active = false + item.calls.delete(call) + } + work.then(finish, finish) + return work + } + + const eligible = (meta: Meta) => + meta.tracked && !meta.binary && imageMime(meta.file) === undefined && !/[\r\n\t"\\]/.test(meta.file) + + const current = (item: Item) => { + const state = states.get(item.scope) + const meta = state?.metas.get(item.meta.file) + if (!state || !meta) return undefined + return { state, meta, id: identity(item.dir, item.base, state.anc, meta) } + } + + const fallback = async (item: Item): Promise => { + const latest = current(item) + if (!latest) return null + const value = cached(latest.id) + if (value) return value + const existing = pending.get(latest.id) + if (existing && existing.item !== item) return existing.work + const result = await load.detail(item.dir, latest.state.anc, latest.meta) + if (result.image?.before?.error === "unreadable" || result.image?.after?.error === "unreadable") return result + remember(latest.id, result) + return result + } + + const run = async (items: Item[]) => { + for (let index = 0; index < items.length; index += MAX_BATCH_FILES) { + const chunk = items.slice(index, index + MAX_BATCH_FILES).filter((item) => item.calls.size > 0) + if (chunk.length === 0) { + for (const item of items.slice(index, index + MAX_BATCH_FILES)) item.resolve(null) + continue + } + for (const item of chunk) item.started = true + if (chunk.length === 1) { + const item = chunk[0]! + try { + item.resolve(await load.detail(item.dir, item.anc, item.meta)) + } catch (error) { + item.reject(error) + } + continue + } + const value = await load + .batch( + chunk[0]!.dir, + chunk[0]!.anc, + chunk.map((item) => item.meta), + ) + .catch((error) => { + load.log?.("Bulk diff detail failed, falling back to single-file requests", error) + return undefined + }) + await Promise.all( + chunk.map(async (item) => { + try { + const latest = current(item) + const entry = value?.entries.get(item.meta.file) + if (latest?.id === item.id && entry && !value?.deferred.has(item.meta.file)) { + item.resolve(entry) + return + } + item.resolve(item.calls.size > 0 ? await fallback(item) : null) + } catch (error) { + item.reject(error) + } + }), + ) + } + } + + const schedule = (key: string) => { + setImmediate(() => { + const queue = queues.get(key) + if (!queue) return + queues.delete(key) + const items = [...queue.values()].filter((item) => item.calls.size > 0) + if (items.length === 0) return + void run(items).catch((error) => { + load.log?.("Bulk diff detail scheduling failed", error) + for (const item of items) item.reject(error) + }) + }) + } + + const queued = (id: string, dir: string, base: string, anc: string, meta: Meta, signal?: AbortSignal) => { + const scope = `${dir}\0${base}` + const key = `${scope}\0${anc}` + let queue = queues.get(key) + if (!queue) { + queue = new Map() + queues.set(key, queue) + schedule(key) + } + let item = queue.get(id) + if (!item) { + let resolve!: (value: Value) => void + let reject!: (error: unknown) => void + const work = new Promise((yes, no) => { + resolve = yes + reject = no + }) + item = { id, scope, queue: key, dir, base, anc, meta, calls: new Set(), started: false, work, resolve, reject } + queue.set(id, item) + watch(id, work, () => current(item!)?.id === id && (item!.started || item!.calls.size > 0)) + pending.get(id)!.item = item + } + return join(item, signal) + } + + const file = (dir: string, base: string, path: string, signal?: AbortSignal): Promise => { + if (signal?.aborted) return Promise.reject(new Error("Diff detail aborted")) + const state = states.get(`${dir}\0${base}`) + if (!state) return load.file(dir, base, path, signal) + const meta = state.metas.get(path) + if (!meta) return Promise.resolve(null) + const id = identity(dir, base, state.anc, meta) + const value = cached(id) + if (value) return Promise.resolve(value) + const existing = pending.get(id) + if (existing?.item) return join(existing.item, signal) + if (existing) return subscribe(existing.work, signal) + if (!eligible(meta)) { + const work = load.detail(dir, state.anc, meta, signal) + return subscribe( + watch(id, work, () => !signal?.aborted), + signal, + ) + } + return queued(id, dir, base, state.anc, meta, signal) + } + + 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 result = await load.summary(dir, base) + if (!result) { + if (generations.get(id) === generation) states.delete(id) + return [] + } + if (generations.get(id) === generation) { + states.delete(id) + states.set(id, { anc: result.anc, metas: new Map(result.metas.map((meta) => [meta.file, meta])) }) + if (states.size > 16) states.delete(states.keys().next().value!) + } + return result.entries + }, + file, + } +} diff --git a/packages/kilo-vscode/src/agent-manager/local-diff.ts b/packages/kilo-vscode/src/agent-manager/local-diff.ts index c142bef0ee..ce07b0af12 100644 --- a/packages/kilo-vscode/src/agent-manager/local-diff.ts +++ b/packages/kilo-vscode/src/agent-manager/local-diff.ts @@ -3,20 +3,11 @@ import { binaryFile } from "../diff/shared/binary" import { imageMime, loadImage, readImageFile } from "../diff/shared/image" import { resolveInside } from "../diff/shared/path" import type { GitOps } from "./GitOps" +import { check, collect, fileSize, MAX_DETAIL_BYTES, readAfter, summarize, type Meta } from "./local-diff-batch" +import { createDiffCache } from "./local-diff-cache" import type { WorktreeDiffEntry } from "./types" -type Status = "added" | "deleted" | "modified" - -type Meta = { - file: string - additions: number - deletions: number - status: Status - tracked: boolean - generatedLike: boolean - binary: boolean - stamp: string -} +type Status = Meta["status"] type Log = (...args: unknown[]) => void @@ -30,7 +21,8 @@ const MAX_UNTRACKED_BYTES = 1_000_000 * threshold we return a summarized entry (empty `before`/`after`/`patch`, * metadata preserved) so the webview can render counts without * materializing the content. */ -export const MAX_DETAIL_BYTES = 20_000_000 +export { MAX_DETAIL_BYTES } from "./local-diff-batch" +const MAX_SUMMARY_FILES = 32 /** * Local, Node.js-side replacement for the server's `WorktreeDiff.summary()` and @@ -95,21 +87,33 @@ export function generatedLike(file: string): boolean { const BASE_CANDIDATES = ["main", "master", "dev", "develop"] -export async function resolveBase(git: GitOps, dir: string, base: string): Promise { +export async function resolveBase(git: GitOps, dir: string, base: string, signal?: AbortSignal): Promise { // If the caller gave an explicit base, honor it. Return it as-is so merge-base // fails loudly on a stale/misspelled ref instead of silently diffing against // an unrelated candidate branch. if (base && base !== "HEAD") return base for (const name of BASE_CANDIDATES) { - const ok = await git.execGit(["rev-parse", "--verify", "--quiet", `refs/heads/${name}`], dir) + const ok = await git.execGit(["rev-parse", "--verify", "--quiet", `refs/heads/${name}`], dir, { + signal, + priority: true, + }) + check(signal) if (ok.code === 0) return name } return "HEAD" } -async function ancestor(git: GitOps, dir: string, base: string, log?: Log): Promise { - const resolvedBase = await resolveBase(git, dir, base) - const result = await git.execGit(["merge-base", "HEAD", resolvedBase], dir) +async function ancestor( + git: GitOps, + dir: string, + base: string, + log?: Log, + signal?: AbortSignal, +): Promise { + const resolvedBase = await resolveBase(git, dir, base, signal) + check(signal) + const result = await git.execGit(["merge-base", "HEAD", resolvedBase], dir, { signal, priority: true }) + check(signal) if (result.code !== 0) { log?.("git merge-base failed", { code: result.code, stderr: result.stderr.trim(), dir, base, resolvedBase }) return undefined @@ -135,10 +139,11 @@ function counts(value: string) { return result } -async function numstat(git: GitOps, dir: string, base: string, file?: string) { +async function numstat(git: GitOps, dir: string, base: string, file?: string, signal?: AbortSignal) { const args = ["-c", "core.quotepath=false", "diff", "--numstat", "--no-renames", base] if (file) args.push("--", file) - const result = await git.execGit(args, dir) + const result = await git.execGit(args, dir, { signal, priority: true }) + check(signal) return counts(result.code === 0 ? result.stdout : "") } @@ -185,8 +190,10 @@ function statusFromCode(code: string): Status { async function list(git: GitOps, dir: string, anc: string, log?: Log): Promise { const [tracked, untracked] = await Promise.all([ - git.execGit(["-c", "core.quotepath=false", "diff", "--raw", "--numstat", "--no-renames", anc], dir), - git.execGit(["ls-files", "--others", "--exclude-standard"], dir), + git.execGit(["-c", "core.quotepath=false", "diff", "--raw", "--numstat", "--no-renames", anc], dir, { + priority: true, + }), + git.execGit(["ls-files", "--others", "--exclude-standard"], dir, { priority: true }), ]) if (tracked.code !== 0) { log?.("git diff --raw --numstat failed", { code: tracked.code, stderr: tracked.stderr.trim() }) @@ -196,27 +203,39 @@ async function list(git: GitOps, dir: string, anc: string, log?: Log): Promise() const stats = counts(tracked.stdout) + const lines = tracked.stdout + .trim() + .split("\n") + .filter((line) => line.startsWith(":")) - for (const line of tracked.stdout.trim().split("\n")) { - if (!line.startsWith(":")) continue - const parts = line.split("\t") - const code = parts[0]?.split(" ").at(-1) - const file = parts.slice(1).join("\t") - if (!file || !code) continue - seen.add(file) - const status = statusFromCode(code) - const stat = stats.get(file) ?? { additions: 0, deletions: 0, binary: false } - result.push({ - file, - additions: stat.additions, - deletions: stat.deletions, - status, - tracked: true, - generatedLike: generatedLike(file), - binary: stat.binary, - stamp: - status === "deleted" ? `deleted:${anc}` : `${imageMime(file) ? `${anc}:` : ""}${await statStamp(dir, file)}`, - }) + for (let index = 0; index < lines.length; index += MAX_SUMMARY_FILES) { + const entries = await Promise.all( + lines.slice(index, index + MAX_SUMMARY_FILES).map(async (line): Promise => { + const parts = line.split("\t") + const code = parts[0]?.split(" ").at(-1) + const file = parts.slice(1).join("\t") + if (!file || !code) return undefined + seen.add(file) + const status = statusFromCode(code) + const stat = stats.get(file) ?? { additions: 0, deletions: 0, binary: false } + return { + file, + additions: stat.additions, + deletions: stat.deletions, + status, + tracked: true, + generatedLike: generatedLike(file), + binary: stat.binary, + stamp: + status === "deleted" + ? `deleted:${anc}` + : `${imageMime(file) ? `${anc}:` : ""}${await statStamp(dir, file)}`, + } + }), + ) + for (const entry of entries) { + if (entry) result.push(entry) + } } if (untracked.code !== 0) { @@ -226,47 +245,36 @@ async function list(git: GitOps, dir: string, anc: string, log?: Log): Promise file && !seen.has(file)) - for (const file of files.split("\n")) { - if (!file || seen.has(file)) continue - const full = resolveInside(dir, file) - if (!full) continue - const exists = await fs.lstat(full).catch(() => undefined) - if (!exists) continue - const binary = await binaryFile(full) - result.push({ - file, - additions: binary ? 0 : await lineCount(full), - deletions: 0, - status: "added", - tracked: false, - generatedLike: generatedLike(file), - binary, - stamp: await statStamp(dir, file), - }) + for (let index = 0; index < paths.length; index += MAX_SUMMARY_FILES) { + const entries = await Promise.all( + paths.slice(index, index + MAX_SUMMARY_FILES).map(async (file): Promise => { + const full = resolveInside(dir, file) + if (!full) return undefined + const exists = await fs.lstat(full).catch(() => undefined) + if (!exists) return undefined + const binary = await binaryFile(full) + return { + file, + additions: binary ? 0 : await lineCount(full), + deletions: 0, + status: "added", + tracked: false, + generatedLike: generatedLike(file), + binary, + stamp: await statStamp(dir, file), + } + }), + ) + for (const entry of entries) { + if (entry) result.push(entry) + } } return result } -function summarize(meta: Meta): WorktreeDiffEntry { - const image = imageMime(meta.file) !== undefined - return { - file: meta.file, - patch: "", - before: "", - after: "", - additions: meta.additions, - deletions: meta.deletions, - status: meta.status, - tracked: meta.tracked, - generatedLike: meta.generatedLike, - summarized: image || !meta.binary, - stamp: meta.stamp, - kind: image ? "image" : undefined, - } -} - /** * Hot polling path. Returns one summarized entry per changed file (tracked or * untracked) relative to `merge-base HEAD base`. No file contents are read — @@ -281,94 +289,37 @@ export async function diffSummary(git: GitOps, dir: string, base: string, log?: } export function createLocalDiff(git: GitOps, log?: Log) { - const states = new Map }>() - const generations = new Map() - const details = new Map() - const pending = new Map }>() - let bytes = 0 - - const forget = (id: string) => { - const value = details.get(id) - if (!value) return - bytes -= value.bytes - details.delete(id) - } - - const remember = (id: string, value: WorktreeDiffEntry, stamp: string) => { - const size = [value.before, value.after, value.patch, value.image?.before?.data, value.image?.after?.data].reduce( - (sum, value) => sum + Buffer.byteLength(value ?? ""), - 0, - ) - const current = details.get(id) - if (current) bytes -= current.bytes - details.delete(id) - details.set(id, { value, bytes: size, stamp }) - 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) + return createDiffCache({ + summary: async (dir, base) => { const anc = await ancestor(git, dir, base, log) - if (!anc) { - 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) + if (!anc) return undefined + const metas = await list(git, dir, anc, log) + return { anc, metas, entries: metas.map(summarize) } }, - 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 - const id = `${dir}\0${base}\0${state.anc}\0${file}\0${meta.tracked}\0${meta.status}\0${meta.additions}\0${meta.deletions}\0${meta.binary}\0${meta.stamp}` - const cached = details.get(id) - if (cached) { - if (cached.stamp === meta.stamp) { - remember(id, cached.value, meta.stamp) - return cached.value - } - forget(id) - } - 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) - if (value.image?.before?.error === "unreadable" || value.image?.after?.error === "unreadable") return - remember(id, value, meta.stamp) - }, - () => { - if (pending.get(id)?.work === work) pending.delete(id) - }, - ) - return work - }, - } + file: (dir, base, path, signal) => diffFile(git, dir, base, path, log, signal), + detail: (dir, anc, meta, signal) => materialize(git, dir, anc, meta, log, signal), + batch: (dir, anc, metas) => collect(git, dir, anc, metas, log), + log, + }) } -async function detailMeta(git: GitOps, dir: string, anc: string, file: string): Promise { +async function detailMeta( + git: GitOps, + dir: string, + anc: string, + file: string, + signal?: AbortSignal, +): Promise { const full = resolveInside(dir, file) if (!full) return undefined - const tracked = await git.execGit(["ls-files", "--error-unmatch", "--", file], dir) + const tracked = await git.execGit(["ls-files", "--error-unmatch", "--", file], dir, { signal, priority: true }) + check(signal) if (tracked.code !== 0) { - const untracked = await git.execGit(["ls-files", "--others", "--exclude-standard", "--", file], dir) + const untracked = await git.execGit(["ls-files", "--others", "--exclude-standard", "--", file], dir, { + signal, + priority: true, + }) + check(signal) if (untracked.code !== 0 || !untracked.stdout.split("\n").includes(file)) return undefined const exists = await fs.lstat(full).catch(() => undefined) if (!exists) return undefined @@ -388,7 +339,9 @@ async function detailMeta(git: GitOps, dir: string, anc: string, file: string): const nameStatus = await git.execGit( ["-c", "core.quotepath=false", "diff", "--name-status", "--no-renames", anc, "--", file], dir, + { signal, priority: true }, ) + check(signal) if (nameStatus.code !== 0) return undefined const line = nameStatus.stdout.trim().split("\n")[0] if (!line) return undefined @@ -397,7 +350,7 @@ async function detailMeta(git: GitOps, dir: string, anc: string, file: string): const pathPart = parts.slice(1).join("\t") || file if (!code) return undefined - const counts = await numstat(git, dir, anc, file) + const counts = await numstat(git, dir, anc, file, signal) const stat = counts.get(file) ?? counts.get(pathPart) ?? { additions: 0, deletions: 0, binary: false } const status = statusFromCode(code) return { @@ -416,18 +369,11 @@ async function detailMeta(git: GitOps, dir: string, anc: string, file: string): } 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 }) + const result = await git.execGit(["cat-file", "-s", `${anc}:${file}`], dir, { signal, priority: true }) if (result.code !== 0) throw new Error(`Could not read base blob for ${file}`) return parseInt(result.stdout.trim(), 10) || 0 } -async function fileSize(dir: string, file: string): Promise { - const full = resolveInside(dir, file) - if (!full) return 0 - const stat = await fs.lstat(full).catch(() => undefined) - return stat?.size ?? 0 -} - async function readBlob( git: GitOps, dir: string, @@ -435,7 +381,7 @@ async function readBlob( file: string, signal?: AbortSignal, ): Promise { - const result = await git.execGitBuffer(["show", `${ref}:${file}`], dir, { signal }) + const result = await git.execGitBuffer(["show", `${ref}:${file}`], dir, { signal, priority: true }) return result.code === 0 ? result.stdout : undefined } @@ -456,24 +402,11 @@ async function readBefore( signal?: AbortSignal, ): Promise { if (status === "added") return "" - const result = await git.execGit(["show", `${anc}:${file}`], dir, { signal }) + const result = await git.execGit(["show", `${anc}:${file}`], dir, { signal, priority: true }) if (result.code !== 0) throw new Error(`Could not read base file for ${file}`) return result.stdout } -async function readAfter(dir: string, file: string, status: Status): Promise { - if (status === "deleted") return "" - const full = resolveInside(dir, file) - if (!full) throw new Error(`Could not resolve working file for ${file}`) - const stat = await fs.lstat(full).catch(() => undefined) - if (!stat) throw new Error(`Could not read working file for ${file}`) - if (stat.isSymbolicLink()) return fs.readlink(full).catch(() => "") - if (!stat.isFile()) throw new Error(`Working path is not a file: ${file}`) - return fs.readFile(full, "utf-8").catch(() => { - throw new Error(`Could not read working file for ${file}`) - }) -} - async function unifiedPatch( git: GitOps, dir: string, @@ -484,7 +417,7 @@ async function unifiedPatch( const result = await git.execGit( ["-c", "core.quotepath=false", "diff", "--no-ext-diff", "--no-renames", anc, "--", file], dir, - { signal }, + { signal, priority: true }, ) if (result.code !== 0) throw new Error(`Could not create diff for ${file}`) return result.stdout @@ -506,12 +439,15 @@ export async function diffFile( base: string, file: string, log?: Log, + signal?: AbortSignal, ): Promise { - const anc = await ancestor(git, dir, base, log) + check(signal) + const anc = await ancestor(git, dir, base, log, signal) if (!anc) return null - const meta = await detailMeta(git, dir, anc, file) + const meta = await detailMeta(git, dir, anc, file, signal) + check(signal) if (!meta) return null - return materialize(git, dir, anc, meta, log) + return materialize(git, dir, anc, meta, log, signal) } async function materialize( diff --git a/packages/kilo-vscode/src/agent-manager/semaphore.ts b/packages/kilo-vscode/src/agent-manager/semaphore.ts index 9006c7bf67..9e042a62c5 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: { resolve: () => void; abort?: () => void }[] = [] + private readonly pending: { resolve: () => void; abort?: () => void; priority: boolean }[] = [] constructor(private readonly limit: number) {} - async run(fn: () => Promise, signal?: AbortSignal): Promise { - await this.acquire(signal) + async run(fn: () => Promise, signal?: AbortSignal, priority = false): Promise { + await this.acquire(signal, priority) try { return await fn() } finally { @@ -20,14 +20,14 @@ export class Semaphore { } } - private acquire(signal?: AbortSignal): Promise { + private acquire(signal: AbortSignal | undefined, priority: boolean): Promise { if (signal?.aborted) return Promise.reject(signal.reason) if (this.running < this.limit) { this.running++ return Promise.resolve() } return new Promise((resolve, reject) => { - const item: { resolve: () => void; abort: () => void } = { + const item: { resolve: () => void; abort: () => void; priority: boolean } = { resolve: () => { signal?.removeEventListener("abort", item.abort) this.running++ @@ -38,9 +38,15 @@ export class Semaphore { if (index !== -1) this.pending.splice(index, 1) reject(signal?.reason) }, + priority, } signal?.addEventListener("abort", item.abort, { once: true }) - this.pending.push(item) + const index = priority ? this.pending.findIndex((entry) => !entry.priority) : -1 + if (index === -1) { + this.pending.push(item) + return + } + this.pending.splice(index, 0, item) }) } diff --git a/packages/kilo-vscode/src/agent-manager/worktree-diff-controller.ts b/packages/kilo-vscode/src/agent-manager/worktree-diff-controller.ts index c974638913..13f9669f7f 100644 --- a/packages/kilo-vscode/src/agent-manager/worktree-diff-controller.ts +++ b/packages/kilo-vscode/src/agent-manager/worktree-diff-controller.ts @@ -39,6 +39,7 @@ export class WorktreeDiffController { /** Intended watch mode for the active context; isPolling lags the initial fetch. */ private poll = false private owner: string | undefined + private generation = 0 /** Ephemeral per-context base override, keyed by context id. */ private baseOverrides = new Map() @@ -248,6 +249,7 @@ export class WorktreeDiffController { } public stop(): void { + this.generation++ this.controller.stop() this.target = undefined this.poll = false @@ -284,15 +286,16 @@ export class WorktreeDiffController { } private async activate(id: string, poll: boolean, fetch: boolean): Promise { + const generation = ++this.generation this.target = undefined this.poll = poll const owner = this.ctx.projectId?.() this.owner = owner await this.ready("stateReady rejected, continuing diff activate:") - if (this.owner !== owner || this.ctx.projectId?.() !== owner) return + if (this.generation !== generation || this.owner !== owner || this.ctx.projectId?.() !== owner) return const { ctx } = parseDiffId(id) const resolved = await this.resolve(ctx) - if (this.owner !== owner || this.ctx.projectId?.() !== owner) return + if (this.generation !== generation || this.owner !== owner || this.ctx.projectId?.() !== owner) return this.target = resolved ? { sessionId: id, ...resolved } : undefined // Clear any stale source notice up front; sources only push a notice when // one is active, so a swap away from a noticing source must reset it. diff --git a/packages/kilo-vscode/src/diff/sources/worktree.ts b/packages/kilo-vscode/src/diff/sources/worktree.ts index 4aa8b690f1..a909a7508e 100644 --- a/packages/kilo-vscode/src/diff/sources/worktree.ts +++ b/packages/kilo-vscode/src/diff/sources/worktree.ts @@ -60,6 +60,7 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}): const log = opts.log ?? ((...args: unknown[]) => appendOutput(output!, "WorktreeDiffSource", ...args)) const git = opts.git ?? new GitOps({ log }) const controller = new AbortController() + let warmed = false const root = (): string | undefined => { const dir = opts.dir?.() @@ -121,6 +122,16 @@ export function createWorktreeDiffSource(opts: WorktreeDiffSourceOptions = {}): ? await opts.summary(current.directory, current.baseBranch) : await diffSummary(git, current.directory, current.baseBranch, log) const diffs = entries.map(toDiffFile) + if (!warmed && opts.file && !controller.signal.aborted) { + warmed = true + const initial = entries.filter((entry) => entry.summarized && !entry.kind && !entry.generatedLike).slice(0, 2) + for (const entry of initial) { + if (!entry.file) continue + void opts.file(current.directory, current.baseBranch, entry.file, controller.signal).catch((err) => { + log("Failed to prefetch initial diff:", err) + }) + } + } log(`Diff: ${diffs.length} file(s)`) return { diffs } }, diff --git a/packages/kilo-vscode/tests/diff-scroll-preservation.spec.ts b/packages/kilo-vscode/tests/diff-scroll-preservation.spec.ts index 640449e0b0..260d0af32e 100644 --- a/packages/kilo-vscode/tests/diff-scroll-preservation.spec.ts +++ b/packages/kilo-vscode/tests/diff-scroll-preservation.spec.ts @@ -3,6 +3,9 @@ import { expect, test, type Page } from "@playwright/test" const GLOBALS = "colorScheme:dark;theme:kilo-vscode;vscodeTheme:dark-modern" const STORY_ID = "agentmanager--full-screen-diff-agent-edit-scroll" const INLINE_STORY_ID = "agentmanager--diff-panel-scroll-up" +const CACHE_STORY_ID = "agentmanager--diff-panel-cached-worktree-switch" +const VIEWPORT_STORY_ID = "agentmanager--diff-panel-viewport-loading" +const TREE_STORY_ID = "agentmanager--file-tree-virtualized-large" function storyUrl() { return `/iframe.html?id=${STORY_ID}&viewMode=story&globals=${GLOBALS}` @@ -221,3 +224,142 @@ test("keeps the inline diff position stable while scrolling upward", async ({ pa expect(result.correction).toBeLessThanOrEqual(1) expect(result.range).toBeLessThanOrEqual(1) }) + +test("keeps cached worktree reviews visible on every switch frame", async ({ page }) => { + await page.setViewportSize({ width: 900, height: 760 }) + await page.goto(`/iframe.html?id=${CACHE_STORY_ID}&viewMode=story&globals=${GLOBALS}`, { waitUntil: "load" }) + await disableAnimations(page) + + for (let index = 1; index <= 12; index++) { + await page.getByTestId(`select-worktree-${index}`).click() + await expect(page.locator(".am-diff-panel-cache-active [data-file-path]")).toHaveAttribute( + "data-file-path", + `src/worktree-${index}.ts`, + ) + await expect(page.locator(".am-diff-panel-cache-active diffs-container [data-line]").first()).toBeVisible() + } + + const result = await page.evaluate(async () => { + const frames: Array<{ id: string; immediate: boolean; painted: boolean; remounted: boolean }> = [] + const panels = new Map() + for (let cycle = 0; cycle < 3; cycle++) { + for (let index = 1; index <= 12; index++) { + const id = `worktree-${index}` + const button = document.querySelector(`[data-testid="select-${id}"]`) + if (!button) throw new Error(`Missing worktree ${id}`) + button.click() + const panel = document.querySelector(".am-diff-panel-cache-active") + const known = panels.get(id) + const remounted = known !== undefined && known !== panel + if (panel) panels.set(id, panel) + const visible = () => { + const row = document.querySelector(".am-diff-panel-cache-active [data-file-path]") + if (row?.getAttribute("data-file-path") !== `src/${id}.ts`) return false + const line = row.querySelector("diffs-container")?.shadowRoot?.querySelector("[data-line]") + return Boolean(line?.textContent?.trim() && line.getBoundingClientRect().height > 0) + } + const immediate = visible() + await new Promise((resolve) => requestAnimationFrame(resolve)) + frames.push({ id, immediate, painted: visible(), remounted }) + } + } + return { + frames, + blank: frames.filter((frame) => !frame.immediate || !frame.painted), + remounts: frames.filter((frame) => frame.remounted), + panels: document.querySelectorAll(".am-diff-panel-cache").length, + active: document.querySelectorAll(".am-diff-panel-cache-active").length, + hidden: [...document.querySelectorAll(".am-diff-panel-cache:not(.am-diff-panel-cache-active)")].every( + (panel) => getComputedStyle(panel).contentVisibility === "hidden", + ), + } + }) + + expect(result.blank).toEqual([]) + expect(result.remounts).toEqual([]) + expect(result.panels).toBe(12) + expect(result.active).toBe(1) + expect(result.hidden).toBe(true) +}) + +test("loads only visible diff details and fetches distant rows when scrolling", async ({ page }) => { + await page.setViewportSize({ width: 900, height: 760 }) + await page.goto(`/iframe.html?id=${VIEWPORT_STORY_ID}&viewMode=story&globals=${GLOBALS}`, { waitUntil: "load" }) + await disableAnimations(page) + + const root = page.getByTestId("viewport-diff-review") + const scroll = page.locator(".am-diff-content") + await expect(scroll).toBeVisible() + await expect.poll(async () => Number(await root.getAttribute("data-request-count"))).toBeGreaterThan(0) + await expect(root.locator("diffs-container [data-line]").first()).toBeVisible() + + const initial = Number(await root.getAttribute("data-request-count")) + expect(initial).toBeLessThan(30) + expect(await root.getAttribute("data-requested")).not.toContain("src/file-119.ts") + await expect(root).toHaveAttribute("data-offscreen", "") + + await expect + .poll( + async () => { + await scroll.evaluate((element) => { + element.scrollTop = element.scrollHeight + }) + return (await root.getAttribute("data-requested"))?.includes("src/file-119.ts") ?? false + }, + { timeout: 10_000 }, + ) + .toBe(true) + + expect(Number(await root.getAttribute("data-request-count"))).toBeLessThan(40) + await expect(root).toHaveAttribute("data-offscreen", "") +}) + +test("resumes interrupted visible diff content without requiring a new summary", async ({ page }) => { + await page.goto(`/iframe.html?id=agentmanager--diff-panel-interrupted-loading&viewMode=story&globals=${GLOBALS}`, { + waitUntil: "load", + }) + const root = page.getByTestId("interrupted-review") + await expect(root).toHaveAttribute("data-requests", "1") + await page.getByTestId("interrupt-review").click() + await page.getByTestId("resume-review").click() + await expect(root).toHaveAttribute("data-requests", "2") + await expect(root.locator("diffs-container [data-line]").filter({ hasText: "after" })).toBeVisible() +}) + +test("virtualizes large review file trees while preserving navigation", async ({ page }) => { + await page.setViewportSize({ width: 900, height: 760 }) + await page.goto(`/iframe.html?id=${TREE_STORY_ID}&viewMode=story&globals=${GLOBALS}`, { waitUntil: "load" }) + await disableAnimations(page) + + const root = page.getByTestId("large-file-tree") + const scroll = root.locator(".am-file-tree-list") + const files = root.locator(".am-file-tree-file") + await expect(scroll).toBeVisible() + await expect.poll(async () => files.count()).toBeGreaterThan(0) + expect(await files.count()).toBeLessThan(80) + + await expect + .poll( + async () => { + await scroll.evaluate((element) => { + element.scrollTop = element.scrollHeight + }) + return root.getByText("file-0599.ts").count() + }, + { timeout: 10_000 }, + ) + .toBe(1) + + await root.getByText("file-0599.ts").click() + await expect(root).toHaveAttribute("data-selected", "src/group-19/file-0599.ts") + expect(await files.count()).toBeLessThan(80) + + await scroll.evaluate((element) => { + element.scrollTop = 0 + }) + await expect(root.locator(".am-file-tree-dir").first()).toBeVisible() + await root.locator(".am-file-tree-dir").first().click() + await expect(files).toHaveCount(0) + await root.locator(".am-file-tree-dir").first().click() + await expect.poll(async () => files.count()).toBeGreaterThan(0) +}) diff --git a/packages/kilo-vscode/tests/unit/agent-manager-diff-state.test.ts b/packages/kilo-vscode/tests/unit/agent-manager-diff-state.test.ts index 0fc9e234ce..ce4a0ff01a 100644 --- a/packages/kilo-vscode/tests/unit/agent-manager-diff-state.test.ts +++ b/packages/kilo-vscode/tests/unit/agent-manager-diff-state.test.ts @@ -195,4 +195,11 @@ describe("diff line virtualization", () => { ), ).toBe(true) }) + + it("virtualizes small patches when either source file exceeds the eager byte limit", () => { + const patch = "@@ -1 +1 @@\n-old\n+new\n" + expect(shouldVirtualizeDiff(diff({ patch, before: "x".repeat(256 * 1024 + 1), after: "new\n" }))).toBe(true) + expect(shouldVirtualizeDiff(diff({ patch, before: "old\n", after: "x".repeat(256 * 1024 + 1) }))).toBe(true) + expect(shouldVirtualizeDiff(diff({ patch, before: "x".repeat(256 * 1024), after: "new\n" }))).toBe(false) + }) }) 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 4965ca1bf1..9a7cb6afb0 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 @@ -38,6 +38,68 @@ describe("createWorktreeDiffs", () => { }) }) + it("retains completed details beyond the mounted review-panel limit", () => { + withDiffs((diffs) => { + const entry = { ...diff("a.ts"), before: "before", after: "after", patch: "+after", summarized: false } + diffs.onWorktreeDiff({ type: "agentManager.worktreeDiff", sessionId: "s1", diffs: [diff("a.ts")] }) + diffs.onWorktreeDiffFile({ + type: "agentManager.worktreeDiffFile", + sessionId: "s1", + file: "a.ts", + diff: entry, + }) + for (let index = 2; index <= 5; index++) { + diffs.onWorktreeDiff({ + type: "agentManager.worktreeDiff", + sessionId: `s${index}`, + diffs: [diff(`${index}.ts`)], + }) + } + + diffs.retain("s1") + expect(diffs.diffDatas()["single\0s1"]?.[0]).toBe(entry) + expect(Object.keys(diffs.diffDatas())).toHaveLength(5) + }) + }) + + it("evicts the least recently used retained worktree data", () => { + withDiffs((diffs) => { + for (let index = 1; index <= 16; index++) { + diffs.onWorktreeDiff({ + type: "agentManager.worktreeDiff", + sessionId: `s${index}`, + diffs: [diff(`${index}.ts`)], + }) + } + diffs.retain("s1") + diffs.onWorktreeDiff({ type: "agentManager.worktreeDiff", sessionId: "s17", diffs: [diff("17.ts")] }) + + expect(diffs.diffDatas()["single\0s1"]).toHaveLength(1) + expect(diffs.diffDatas()["single\0s2"]).toBeUndefined() + expect(diffs.diffDatas()["single\0s17"]).toHaveLength(1) + expect(Object.keys(diffs.diffDatas())).toHaveLength(16) + }) + }) + + it("bounds retained worktree content without evicting the active context", () => { + withDiffs((diffs) => { + const content = "x".repeat(17 * 1024 * 1024) + diffs.onWorktreeDiff({ + type: "agentManager.worktreeDiff", + sessionId: "s1", + diffs: [{ ...diff("first.ts"), before: content }], + }) + diffs.onWorktreeDiff({ + type: "agentManager.worktreeDiff", + sessionId: "s2", + diffs: [{ ...diff("second.ts"), before: content }], + }) + + expect(diffs.diffDatas()["single\0s1"]).toBeUndefined() + expect(diffs.diffDatas()["single\0s2"]).toHaveLength(1) + }) + }) + it("does not replace state when an update produces an identical diff list", () => { withDiffs((diffs) => { diffs.onWorktreeDiff({ type: "agentManager.worktreeDiff", sessionId: "s1", diffs: [diff("a.ts")] }) diff --git a/packages/kilo-vscode/tests/unit/diff-preview-request.test.ts b/packages/kilo-vscode/tests/unit/diff-preview-request.test.ts index ba61eeee9b..f80f406679 100644 --- a/packages/kilo-vscode/tests/unit/diff-preview-request.test.ts +++ b/packages/kilo-vscode/tests/unit/diff-preview-request.test.ts @@ -101,6 +101,38 @@ const SCRIPT = ` fail("did not request after existing loading state cleared " + JSON.stringify(blocked)) } disposeBlocked() + + const lazy = [] + const entries = Array.from({ length: 85 }, (_, index) => ({ + ...summary, + file: "src/file-" + index + ".ts", + })) + let request + const disposeLazy = createRoot((dispose) => { + request = createDiffRequests({ + key: () => "review-lazy", + diffs: () => entries, + open: () => entries.map((item) => item.file), + loading: () => undefined, + send: () => (file) => lazy.push(file), + eager: false, + }) + return dispose + }) + + await new Promise((resolve) => setTimeout(resolve, 0)) + if (lazy.length !== 0) fail("offscreen diffs requested eagerly " + JSON.stringify(lazy)) + request(entries[0]) + request(entries[1]) + request(entries[0]) + if (lazy.length !== 2 || lazy[0] !== entries[0].file || lazy[1] !== entries[1].file) { + fail("visible diff requests were not lazy and deduplicated " + JSON.stringify(lazy)) + } + request(entries[80]) + if (lazy.length !== 3 || lazy[2] !== entries[80].file) { + fail("newly mounted distant diff was not requested " + JSON.stringify(lazy)) + } + disposeLazy() console.log("${PASS}") ` diff --git a/packages/kilo-vscode/tests/unit/diff-source-catalog.test.ts b/packages/kilo-vscode/tests/unit/diff-source-catalog.test.ts index 6466df3d1a..5ba1e530a5 100644 --- a/packages/kilo-vscode/tests/unit/diff-source-catalog.test.ts +++ b/packages/kilo-vscode/tests/unit/diff-source-catalog.test.ts @@ -2,7 +2,8 @@ import { describe, it, expect } from "bun:test" import type { KiloConnectionService } from "../../src/services/cli-backend" import { DiffSourceCatalog } from "../../src/diff/sources/catalog" import { sessionDescriptor } from "../../src/diff/sources/session" -import { WORKSPACE_DESCRIPTOR } from "../../src/diff/sources/worktree" +import { WORKSPACE_DESCRIPTOR, createWorktreeDiffSource } from "../../src/diff/sources/worktree" +import { GitOps } from "../../src/agent-manager/GitOps" // Minimal stand-in for the connection service — the catalog only holds a // reference and passes it to the source factories, so we never exercise any @@ -123,6 +124,42 @@ describe("descriptor types", () => { }) }) +describe("initial worktree preview", () => { + it("starts only two text previews without waiting for their content", async () => { + const git = new GitOps({ log: () => undefined }) + const calls: string[] = [] + const source = createWorktreeDiffSource({ + git, + log: () => undefined, + dir: () => "/repo", + baseBranch: "main", + summary: async () => + Array.from({ length: 20 }, (_, index) => ({ + file: `file-${index}.ts`, + before: "", + after: "", + patch: "", + additions: 1, + deletions: 0, + summarized: true, + tracked: true, + generatedLike: false, + status: "modified" as const, + })), + file: async (_dir, _base, file) => { + calls.push(file) + return null + }, + }) + expect((await source.fetch()).diffs).toHaveLength(20) + expect(calls).toEqual(["file-0.ts", "file-1.ts"]) + await source.fetch() + expect(calls).toHaveLength(2) + source.dispose?.() + git.dispose() + }) +}) + describe("DiffSourceCatalog.dispose", () => { it("disposes without throwing when no branch resources were created", () => { const cat = makeCatalog() diff --git a/packages/kilo-vscode/tests/unit/git-ops.test.ts b/packages/kilo-vscode/tests/unit/git-ops.test.ts index c90aa3a7c0..54fd1cdd8f 100644 --- a/packages/kilo-vscode/tests/unit/git-ops.test.ts +++ b/packages/kilo-vscode/tests/unit/git-ops.test.ts @@ -57,6 +57,20 @@ describe("GitOps", () => { }) }) + it("passes stdin to binary Git commands without decoding their output", async () => { + await withRepo(async (cwd) => { + const git = new GitOps({ log: () => undefined, binary: async () => "git" }) + const value = "before\u0000after" + const object = await git.execGit(["hash-object", "-w", "--stdin"], cwd, { stdin: value }) + const result = await git.execGitBuffer(["cat-file", "--batch"], cwd, { + stdin: `${object.stdout.trim()}\n`, + }) + expect(result.code).toBe(0) + expect(result.stdout.includes(Buffer.from(value))).toBe(true) + git.dispose() + }) + }) + it("does not hold a semaphore slot while resolving Git", async () => { const semaphore = new Semaphore(1) let resolve!: (value: string) => void @@ -687,6 +701,21 @@ describe("GitOps", () => { git.dispose() expect(git.disposed).toBe(true) }) + + it("stops waiting for executable discovery when its request signal aborts", async () => { + let release!: (value: string) => void + const gate = new Promise((resolve) => { + release = resolve + }) + const git = new GitOps({ log: () => undefined, binary: () => gate }) + const ctl = new AbortController() + const pending = git.execGit(["status"], "/repo", { signal: ctl.signal }) + ctl.abort() + const result = await pending + expect(result.code).not.toBe(0) + release("git") + git.dispose() + }) }) describe("semaphore integration", () => { diff --git a/packages/kilo-vscode/tests/unit/local-diff.test.ts b/packages/kilo-vscode/tests/unit/local-diff.test.ts index 595d0bf349..0eb53e4e15 100644 --- a/packages/kilo-vscode/tests/unit/local-diff.test.ts +++ b/packages/kilo-vscode/tests/unit/local-diff.test.ts @@ -10,7 +10,7 @@ import { resolveBase, MAX_DETAIL_BYTES, } from "../../src/agent-manager/local-diff" -import { GitOps } from "../../src/agent-manager/GitOps" +import { GitOps, type ExecBufferResult, type ExecResult } from "../../src/agent-manager/GitOps" import { WorktreeDiffReverter } from "../../src/diff/shared/reverter" import { resolveLocalDiffTarget } from "../../src/diff/shared/target" @@ -68,6 +68,97 @@ async function withRepo(run: (dir: string, base: string) => Promise): Prom } } +function deferred() { + let resolve!: (value: T | PromiseLike) => void + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +function turn(): Promise { + return new Promise((resolve) => setImmediate(resolve)) +} + +function inspect(args: string[]): boolean { + return args[0] === "cat-file" && args[1] === "--batch-check" +} + +function blobs(args: string[]): boolean { + return args[0] === "cat-file" && args[1] === "--batch" +} + +function diff(args: string[]): boolean { + return args.includes("diff") && args.includes("--no-ext-diff") +} + +function size(args: string[]): boolean { + return args[0] === "cat-file" && args[1] === "-s" +} + +function show(args: string[]): boolean { + return args[0] === "show" +} + +function count(ops: RecordingGitOps, test: (args: string[]) => boolean): number { + return ops.calls.filter((call) => test(call.args)).length +} + +class RecordingGitOps extends GitOps { + readonly calls: Array<{ kind: "text" | "buffer"; args: string[]; cwd: string }> = [] + fail = false + private wait: Promise | undefined + private begin: (() => void) | undefined + private test: ((args: string[]) => boolean) | undefined + private held = false + + constructor() { + super({ log: () => undefined }) + } + + block(test: (args: string[]) => boolean, wait: Promise, begin: () => void): void { + this.test = test + this.wait = wait + this.begin = begin + this.held = true + } + + private async pause(args: string[]): Promise { + const test = this.test + const wait = this.wait + if (!this.held || !test || !wait || !test(args)) return + this.held = false + this.begin?.() + await wait + } + + override async execGit(args: string[], cwd: string, options?: Parameters[2]): Promise { + this.calls.push({ kind: "text", args, cwd }) + await this.pause(args) + if (this.fail && inspect(args)) return { code: 1, stdout: "", stderr: "batch inspection failed" } + return super.execGit(args, cwd, options) + } + + override async execGitBuffer( + args: string[], + cwd: string, + options?: Parameters[2], + ): Promise { + this.calls.push({ kind: "buffer", args, cwd }) + await this.pause(args) + return super.execGitBuffer(args, cwd, options) + } +} + +async function setup(dir: string, base: string): Promise { + await fs.mkdir(path.join(dir, "folder")) + await fs.writeFile(path.join(dir, "folder", "space file.txt"), "space base\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\n") + runSync(dir, ["add", "."]) + runSync(dir, ["commit", "-m", "add tracked files"]) + runSync(dir, ["branch", "-f", base]) +} + describe("generatedLike", () => { it("matches files in ignored folders", () => { expect(generatedLike("node_modules/foo.js")).toBe(true) @@ -177,6 +268,25 @@ describe("diffSummary", () => { }) }) + it("preserves tracked and untracked order across parallel metadata batches", async () => { + await withRepo(async (dir, base) => { + const tracked = Array.from({ length: 40 }, (_, index) => `tracked-${String(index).padStart(2, "0")}.txt`) + await Promise.all(tracked.map((file) => fs.writeFile(path.join(dir, file), "before\n"))) + runSync(dir, ["add", "."]) + runSync(dir, ["commit", "-m", "add tracked files"]) + runSync(dir, ["branch", "-f", base]) + await Promise.all(tracked.map((file) => fs.writeFile(path.join(dir, file), "before\nafter\n"))) + const untracked = Array.from({ length: 40 }, (_, index) => `untracked-${String(index).padStart(2, "0")}.txt`) + await Promise.all(untracked.map((file) => fs.writeFile(path.join(dir, file), "new\n"))) + + const result = await diffSummary(git(), dir, base) + expect(result.map((item) => item.file)).toEqual([...tracked, ...untracked]) + expect(result.slice(0, tracked.length).every((item) => item.tracked && item.additions === 1)).toBe(true) + expect(result.slice(tracked.length).every((item) => !item.tracked && item.additions === 1)).toBe(true) + expect(result.every((item) => typeof item.stamp === "string" && item.stamp.length > 0)).toBe(true) + }) + }) + it("includes untracked files as added with tracked=false", async () => { await withRepo(async (dir, base) => { await fs.writeFile(path.join(dir, "untracked.txt"), "a\nb\nc\n") @@ -493,6 +603,454 @@ describe("diffFile", () => { }) }) +describe("createLocalDiff concurrent details", () => { + it("coalesces tracked, spaced, and untracked text details", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nupdated\n") + await fs.writeFile(path.join(dir, "folder", "space file.txt"), "space base\nupdated\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\ndiff --git a/fake.txt b/fake.txt\nupdated\n") + await fs.writeFile(path.join(dir, "new file.txt"), "new\ntext\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + + const files = ["seed.txt", "folder/space file.txt", "third file.txt", "new file.txt"] + const result = await Promise.all(files.map((file) => local.file(dir, base, file))) + const [seed, space, third, fresh] = result + + expect(seed?.before).toBe("seed\n") + expect(seed?.after).toBe("seed\nupdated\n") + expect(seed?.patch).toContain("+updated") + expect(space?.before).toBe("space base\n") + expect(space?.after).toBe("space base\nupdated\n") + expect(space?.patch).toContain(`diff --git a/folder/space file.txt b/folder/space file.txt`) + expect(third?.before).toBe("third base\n") + expect(third?.after).toBe("third base\ndiff --git a/fake.txt b/fake.txt\nupdated\n") + expect(third?.patch).toContain("+updated") + expect(third?.patch).toContain("+diff --git a/fake.txt b/fake.txt") + expect(fresh?.before).toBe("") + expect(fresh?.after).toBe("new\ntext\n") + expect(fresh?.patch).toContain("+new") + expect(await local.file(dir, base, "seed.txt")).toBe(seed) + expect(await local.file(dir, base, "folder/space file.txt")).toBe(space) + expect(await local.file(dir, base, "third file.txt")).toBe(third) + expect(await local.file(dir, base, "new file.txt")).toBe(fresh) + expect(count(ops, inspect)).toBe(1) + expect(count(ops, blobs)).toBe(1) + expect(count(ops, diff)).toBe(1) + const patch = ops.calls.find((call) => diff(call.args)) + expect(patch?.args).toContain("seed.txt") + expect(patch?.args).toContain("folder/space file.txt") + expect(patch?.args).toContain("third file.txt") + expect(seed?.patch).not.toContain("folder/space file.txt") + expect(space?.patch).not.toContain("third file.txt") + }) + }) + + it("coalesces requests delivered by separate source-controller timers", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\ntimer\n") + await fs.writeFile(path.join(dir, "folder", "space file.txt"), "space base\ntimer\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\ntimer\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + + const files = ["seed.txt", "folder/space file.txt", "third file.txt"] + const result = await Promise.all( + files.map( + (file) => + new Promise>>((resolve, reject) => { + setTimeout(() => void local.file(dir, base, file).then(resolve, reject), 0) + }), + ), + ) + + expect(result.every((item) => item?.after.includes("timer"))).toBe(true) + expect(count(ops, inspect)).toBe(1) + expect(count(ops, blobs)).toBe(1) + expect(count(ops, diff)).toBe(1) + }) + }) + + it("does not delay untracked previews behind a tracked Git batch", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\ntracked\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\ntracked\n") + await fs.writeFile(path.join(dir, "fresh.txt"), "available immediately\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + const gate = deferred() + const ready = deferred() + ops.block(inspect, gate.promise, ready.resolve) + + const first = local.file(dir, base, "seed.txt") + const second = local.file(dir, base, "third file.txt") + const fresh = local.file(dir, base, "fresh.txt") + await ready.promise + const result = await Promise.race([ + fresh.then((value) => ({ ready: true, value })), + new Promise<{ ready: false; value: undefined }>((resolve) => { + setTimeout(() => resolve({ ready: false, value: undefined }), 100) + }), + ]) + gate.resolve() + await Promise.all([first, second]) + + expect(result.ready).toBe(true) + expect(result.value?.after).toBe("available immediately\n") + }) + }) + + it("keeps a singleton on the existing single-file Git commands", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nsingle\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + + const result = await local.file(dir, base, "seed.txt") + + expect(result?.after).toBe("seed\nsingle\n") + expect(count(ops, inspect)).toBe(0) + expect(count(ops, blobs)).toBe(0) + expect(count(ops, size)).toBe(1) + expect(count(ops, show)).toBe(1) + expect(count(ops, diff)).toBe(1) + }) + }) + + it("shares one result for duplicate concurrent callers in a batch", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nduplicate\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\nduplicate\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + + const [first, second, sibling] = await Promise.all([ + local.file(dir, base, "seed.txt"), + local.file(dir, base, "seed.txt"), + local.file(dir, base, "third file.txt"), + ]) + + expect(first).toBe(second) + expect(first?.after).toBe("seed\nduplicate\n") + expect(sibling?.after).toBe("third base\nduplicate\n") + expect(count(ops, inspect)).toBe(1) + expect(count(ops, blobs)).toBe(1) + expect(count(ops, diff)).toBe(1) + }) + }) + + it("aborts one subscriber without canceling a sibling", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfirst\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\nsecond\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + const gate = deferred() + const ready = deferred() + ops.block(inspect, gate.promise, ready.resolve) + const ctl = new AbortController() + const first = local.file(dir, base, "seed.txt", ctl.signal) + const sibling = local.file(dir, base, "third file.txt") + await ready.promise + ctl.abort() + + await expect(first).rejects.toThrow() + gate.resolve() + expect((await sibling)?.after).toBe("third base\nsecond\n") + expect(count(ops, inspect)).toBe(1) + expect(count(ops, blobs)).toBe(1) + expect(count(ops, diff)).toBe(1) + }) + }) + + it("aborts one subscriber without canceling another subscriber of the same identity", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nshared\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\nother\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + const gate = deferred() + const ready = deferred() + ops.block(inspect, gate.promise, ready.resolve) + const one = new AbortController() + const two = new AbortController() + const first = local.file(dir, base, "seed.txt", one.signal) + const second = local.file(dir, base, "seed.txt", two.signal) + const sibling = local.file(dir, base, "third file.txt") + await ready.promise + one.abort() + + await expect(first).rejects.toThrow() + gate.resolve() + expect((await second)?.after).toBe("seed\nshared\n") + expect((await sibling)?.after).toBe("third base\nother\n") + expect(count(ops, inspect)).toBe(1) + expect(count(ops, blobs)).toBe(1) + expect(count(ops, diff)).toBe(1) + }) + }) + + it("does not spawn detail Git when every queued caller aborts before flush", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nqueued\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\nqueued\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + const firstCtl = new AbortController() + const secondCtl = new AbortController() + const first = local.file(dir, base, "seed.txt", firstCtl.signal) + const second = local.file(dir, base, "third file.txt", secondCtl.signal) + firstCtl.abort() + secondCtl.abort() + + await expect(first).rejects.toThrow() + await expect(second).rejects.toThrow() + await turn() + expect(ops.calls).toHaveLength(0) + }) + }) + + it("keeps a started singleton cache-owned across a worktree switch", async () => { + await withRepo(async (dir, base) => { + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nretained\n") + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + const gate = deferred() + const ready = deferred() + ops.block(size, gate.promise, ready.resolve) + const controller = new AbortController() + const first = local.file(dir, base, "seed.txt", controller.signal) + await ready.promise + controller.abort() + await expect(first).rejects.toThrow() + const second = local.file(dir, base, "seed.txt") + gate.resolve() + const value = await second + expect(value?.after).toBe("seed\nretained\n") + expect(count(ops, size)).toBe(1) + expect(count(ops, show)).toBe(1) + expect(count(ops, diff)).toBe(1) + const calls = ops.calls.length + expect(await local.file(dir, base, "seed.txt")).toBe(value) + expect(ops.calls).toHaveLength(calls) + }) + }) + + it("keeps a started batch cache-owned across aborts and resubscription", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nowned\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\nowned\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + const gate = deferred() + const ready = deferred() + ops.block(inspect, gate.promise, ready.resolve) + const one = new AbortController() + const two = new AbortController() + const first = local.file(dir, base, "seed.txt", one.signal) + const second = local.file(dir, base, "third file.txt", two.signal) + await ready.promise + one.abort() + two.abort() + await expect(first).rejects.toThrow() + await expect(second).rejects.toThrow() + + const resub = local.file(dir, base, "seed.txt", new AbortController().signal) + gate.resolve() + const result = await resub + expect(result?.after).toBe("seed\nowned\n") + const before = ops.calls.length + expect(await local.file(dir, base, "seed.txt")).toBe(result) + expect(ops.calls.length).toBe(before) + expect(count(ops, inspect)).toBe(1) + expect(count(ops, blobs)).toBe(1) + expect(count(ops, diff)).toBe(1) + }) + }) + + it("preserves a started batch through an unchanged summary refresh", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nrefresh\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\nrefresh\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + const gate = deferred() + const ready = deferred() + ops.block(inspect, gate.promise, ready.resolve) + const first = local.file(dir, base, "seed.txt") + const second = local.file(dir, base, "third file.txt") + await ready.promise + await local.summary(dir, base) + gate.resolve() + const [one, two] = await Promise.all([first, second]) + + expect(one?.after).toBe("seed\nrefresh\n") + expect(two?.after).toBe("third base\nrefresh\n") + const before = ops.calls.length + expect(await local.file(dir, base, "seed.txt")).toBe(one) + expect(await local.file(dir, base, "third file.txt")).toBe(two) + expect(ops.calls.length).toBe(before) + expect(count(ops, inspect)).toBe(1) + expect(count(ops, blobs)).toBe(1) + expect(count(ops, diff)).toBe(1) + }) + }) + + it("refreshes a changed sibling without corrupting an unchanged sibling", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfirst\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\nfirst\n") + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + const gate = deferred() + const ready = deferred() + ops.block(inspect, gate.promise, ready.resolve) + const first = local.file(dir, base, "seed.txt") + const second = local.file(dir, base, "third file.txt") + await ready.promise + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nchanged while loading\n") + await local.summary(dir, base) + gate.resolve() + const stable = await second + await first.catch(() => null) + + expect(stable?.after).toBe("third base\nfirst\n") + const current = await local.file(dir, base, "seed.txt") + expect(current?.after).toBe("seed\nchanged while loading\n") + expect(current).not.toBe(stable) + }) + }) + + it("falls back independently when shared batch inspection fails", async () => { + await withRepo(async (dir, base) => { + await setup(dir, base) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\nfallback\n") + await fs.writeFile(path.join(dir, "folder", "space file.txt"), "space base\nfallback\n") + await fs.writeFile(path.join(dir, "third file.txt"), "third base\nfallback\n") + + const ops = new RecordingGitOps() + ops.fail = true + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + const files = ["seed.txt", "folder/space file.txt", "third file.txt"] + const result = await Promise.all(files.map((file) => local.file(dir, base, file))) + + expect(result.map((item) => item?.summarized)).toEqual([false, false, false]) + expect(result[0]?.after).toBe("seed\nfallback\n") + expect(result[1]?.after).toBe("space base\nfallback\n") + expect(result[2]?.after).toBe("third base\nfallback\n") + expect(count(ops, inspect)).toBe(1) + expect(count(ops, blobs)).toBe(0) + expect(count(ops, size)).toBe(3) + expect(count(ops, show)).toBe(3) + expect(count(ops, diff)).toBe(3) + const before = ops.calls.length + expect(await local.file(dir, base, "seed.txt")).toBe(result[0]) + expect(ops.calls.length).toBe(before) + }) + }) + + it("keeps images and non-image binaries on their existing detail paths", async () => { + await withRepo(async (dir, base) => { + const before = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0x01]) + const after = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0xff]) + const bytes = Buffer.from([0x00, 0x01, 0x02, 0x03]) + await fs.writeFile(path.join(dir, "banner.png"), before) + await fs.writeFile(path.join(dir, "tone.bin"), bytes) + runSync(dir, ["add", "banner.png", "tone.bin"]) + runSync(dir, ["commit", "-m", "add binary files"]) + runSync(dir, ["branch", "-f", base]) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\ntext\n") + await fs.writeFile(path.join(dir, "banner.png"), after) + await fs.writeFile(path.join(dir, "tone.bin"), Buffer.concat([bytes, Buffer.from([0xff])])) + + const ops = new RecordingGitOps() + const local = createLocalDiff(ops) + await local.summary(dir, base) + ops.calls.splice(0) + const [text, image, binary] = await Promise.all([ + local.file(dir, base, "seed.txt"), + local.file(dir, base, "banner.png"), + local.file(dir, base, "tone.bin"), + ]) + + expect(text?.after).toBe("seed\ntext\n") + expect(image?.image?.before?.data).toBe(before.toString("base64")) + expect(image?.image?.after?.data).toBe(after.toString("base64")) + expect(binary?.summarized).toBe(false) + expect(binary?.before).toBe("") + expect(binary?.after).toBe("") + expect(binary?.patch).toBe("") + expect(count(ops, inspect)).toBe(0) + expect(count(ops, blobs)).toBe(0) + expect(count(ops, size)).toBe(2) + expect(count(ops, show)).toBe(2) + expect(count(ops, diff)).toBe(1) + }) + }) + + it("preserves summarized results for oversized and unsafe paths", async () => { + await withRepo(async (dir, base) => { + await fs.writeFile(path.join(dir, "seed.txt"), "a".repeat(MAX_DETAIL_BYTES + 1)) + const local = createLocalDiff(git()) + await local.summary(dir, base) + + const result = await local.file(dir, base, "seed.txt") + expect(result?.summarized).toBe(true) + expect(result?.before).toBe("") + expect(result?.after).toBe("") + expect(result?.patch).toBe("") + expect(await local.file(dir, base, "../outside.txt")).toBeNull() + }) + }) +}) + describe("resolveLocalDiffTarget + revertFile", () => { it("uses the remote's current trunk when local origin/HEAD is stale", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "local-diff-stale-head-")) diff --git a/packages/kilo-vscode/tests/unit/semaphore.test.ts b/packages/kilo-vscode/tests/unit/semaphore.test.ts index dc64b8b0e5..f7398283fe 100644 --- a/packages/kilo-vscode/tests/unit/semaphore.test.ts +++ b/packages/kilo-vscode/tests/unit/semaphore.test.ts @@ -69,6 +69,29 @@ describe("Semaphore", () => { expect(order).toEqual([1, 2, 3]) }) + it("runs interactive work before queued background tasks while preserving priority order", async () => { + const sem = new Semaphore(1) + const order: string[] = [] + let release: () => void = () => {} + const held = sem.run( + () => + new Promise((resolve) => { + order.push("running") + release = resolve + }), + ) + await Promise.resolve() + + const first = sem.run(async () => order.push("background-1")) + const second = sem.run(async () => order.push("background-2")) + const urgent = sem.run(async () => order.push("interactive-1"), undefined, true) + const next = sem.run(async () => order.push("interactive-2"), undefined, true) + + release() + await Promise.all([held, first, second, urgent, next]) + expect(order).toEqual(["running", "interactive-1", "interactive-2", "background-1", "background-2"]) + }) + it("removes an aborted task from the pending queue", async () => { const sem = new Semaphore(1) let release: () => void = () => {} @@ -85,6 +108,23 @@ describe("Semaphore", () => { await first }) + it("removes aborted interactive work without delaying background tasks", async () => { + const sem = new Semaphore(1) + let release: () => void = () => {} + const held = sem.run(() => new Promise((resolve) => (release = resolve))) + await Promise.resolve() + const background = sem.run(async () => "background") + const controller = new AbortController() + const urgent = sem.run(async () => "interactive", controller.signal, true) + + controller.abort(new Error("cancelled")) + await expect(urgent).rejects.toThrow("cancelled") + release() + + expect(await background).toBe("background") + await held + }) + 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/worktree-diff-controller.test.ts b/packages/kilo-vscode/tests/unit/worktree-diff-controller.test.ts index 59888079ed..fc56991549 100644 --- a/packages/kilo-vscode/tests/unit/worktree-diff-controller.test.ts +++ b/packages/kilo-vscode/tests/unit/worktree-diff-controller.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect } from "bun:test" import { WorktreeDiffController } from "../../src/agent-manager/worktree-diff-controller" import type { DiffSourceCatalog } from "../../src/diff/sources/catalog" import type { DiffSource } from "../../src/diff/sources/types" -import type { PanelContext } from "../../src/diff/types" +import type { DiffFile, PanelContext } from "../../src/diff/types" import type { GitOps } from "../../src/agent-manager/GitOps" import type { WorktreeStateManager } from "../../src/agent-manager/WorktreeStateManager" @@ -10,17 +10,30 @@ import type { WorktreeStateManager } from "../../src/agent-manager/WorktreeState // base branch the active source was (re)built with. The controller, scope // resolution, and SourceController lifecycle under test are all real. // Contexts are worktree ids (the sidebar selection), not session ids. -function make(onFetch?: (n: number) => Promise) { +type Opts = { + fetch?: (n: number) => Promise + ready?: () => Promise | undefined + git?: GitOps + trees?: Record + diffs?: (id: string, ctx: PanelContext) => DiffFile[] +} + +function make(opts: Opts = {}) { const builds: { id: string; ctx: PanelContext }[] = [] + const posted: unknown[] = [] let fetches = 0 + const trees = opts.trees ?? { + w1: { id: "w1", path: "/wt", parentBranch: "main", remote: "origin" }, + w2: { id: "w2", path: "/wt-2", parentBranch: "main", remote: "origin" }, + } const catalog = { build: (id: string, ctx: PanelContext): DiffSource => { builds.push({ id, ctx }) return { descriptor: { id, type: "workspace", group: "Git", capabilities: { revert: true, comments: true } }, async fetch() { - await onFetch?.(++fetches) - return { diffs: [] } + await opts.fetch?.(++fetches) + return { diffs: opts.diffs?.(id, ctx) ?? [] } }, } }, @@ -28,21 +41,20 @@ function make(onFetch?: (n: number) => Promise) { const state = { getSession: (id: string) => (id === "s1" ? { id: "s1", worktreeId: "w1", createdAt: "" } : undefined), - getWorktree: (id: string) => - id === "w1" ? { id: "w1", path: "/wt", parentBranch: "main", remote: "origin" } : undefined, + getWorktree: (id: string) => trees[id], } as unknown as WorktreeStateManager const controller = new WorktreeDiffController({ getState: () => state, getRoot: () => "/repo", - getStateReady: () => undefined, + getStateReady: opts.ready ?? (() => undefined), catalog, - git: {} as GitOps, + git: opts.git ?? ({} as GitOps), localDiffFile: async () => null, - post: () => {}, + post: (message) => posted.push(message), log: () => {}, }) - return { controller, builds } + return { controller, builds, posted } } const tick = () => new Promise((resolve) => setTimeout(resolve, 0)) @@ -55,6 +67,18 @@ async function waitFor(cond: () => boolean): Promise { throw new Error("waitFor timed out") } +function defer() { + let resolve: () => void = () => {} + const promise = new Promise((done) => (resolve = done)) + return { promise, resolve } +} + +function byType(items: unknown[], type: string) { + return items.filter((item): item is { type: string } => { + return typeof item === "object" && item !== null && (item as { type?: unknown }).type === type + }) +} + describe("WorktreeDiffController.setBase", () => { it("rebuilds the active source against the overridden base branch", async () => { const { controller, builds } = make() @@ -96,8 +120,10 @@ describe("WorktreeDiffController.setBase", () => { // survive the base change rather than downgrading the panel to one-shot. let release: () => void = () => {} const gate = new Promise((resolve) => (release = resolve)) - const { controller, builds } = make(async (n) => { - if (n === 1) await gate + const { controller, builds } = make({ + fetch: async (n) => { + if (n === 1) await gate + }, }) controller.start("w1#branch") @@ -117,4 +143,120 @@ describe("WorktreeDiffController.setBase", () => { controller.stop() }) + + it("uses the latest activation when resolve finishes out of order", async () => { + const a = defer() + const b = defer() + let calls = 0 + const git = { + currentBranch: async () => { + const wait = calls++ === 0 ? a : b + await wait.promise + return "feature" + }, + resolveTrackingBranch: async () => "origin/main", + } as unknown as GitOps + const { controller, builds } = make({ git }) + + controller.start("local#branch") + await waitFor(() => calls === 1) + controller.start("local#staged") + await waitFor(() => calls === 2) + + b.resolve() + await waitFor(() => builds.length === 1) + a.resolve() + await tick() + + expect(builds).toHaveLength(1) + expect(builds[0]!.id).toBe("staged") + + controller.stop() + }) + + it("uses the latest request in an A-to-B-to-A sequence", async () => { + const a1 = defer() + const b = defer() + const a2 = defer() + const waits = [a1, b, a2] + let calls = 0 + const git = { + currentBranch: async () => { + const wait = waits[calls++]! + await wait.promise + return "feature" + }, + resolveTrackingBranch: async () => "origin/main", + } as unknown as GitOps + const { controller, builds } = make({ git }) + + controller.start("local#branch") + await waitFor(() => calls === 1) + controller.start("local#staged") + await waitFor(() => calls === 2) + controller.start("local#branch") + await waitFor(() => calls === 3) + + a2.resolve() + await waitFor(() => builds.length === 1) + b.resolve() + a1.resolve() + await tick() + + expect(builds).toHaveLength(1) + expect(builds[0]!.id).toBe("workspace") + + controller.stop() + }) + + it("does not activate after a pending request is stopped", async () => { + const ready = defer() + const { controller, builds } = make({ ready: () => ready.promise }) + + controller.start("w1#branch") + controller.stop() + ready.resolve() + await tick() + + expect(builds).toHaveLength(0) + }) + + it("drops source data and polling from a stopped initial fetch", async () => { + const fetch = defer() + const { controller, builds, posted } = make({ + fetch: async () => { + await fetch.promise + }, + diffs: () => [{ file: "stale.ts", before: "", after: "", additions: 0, deletions: 0 }], + }) + + controller.start("w1#branch") + await waitFor(() => builds.length === 1) + controller.stop() + fetch.resolve() + await tick() + + expect(byType(posted, "agentManager.worktreeDiff")).toHaveLength(0) + + controller.start("w1#branch") + await waitFor(() => builds.length === 2) + controller.stop() + }) + + it("keeps synchronous repeated starts while the initial fetch is pending", async () => { + const fetch = defer() + const { controller, builds } = make({ + fetch: async (n) => { + if (n === 1) await fetch.promise + }, + }) + + controller.start("w1#branch") + await waitFor(() => builds.length === 1) + controller.start("w1#branch") + await waitFor(() => builds.length === 2) + fetch.resolve() + + controller.stop() + }) }) diff --git a/packages/kilo-vscode/tests/visual-regression.spec.mts b/packages/kilo-vscode/tests/visual-regression.spec.mts index 437ac4bd85..4a53b19309 100644 --- a/packages/kilo-vscode/tests/visual-regression.spec.mts +++ b/packages/kilo-vscode/tests/visual-regression.spec.mts @@ -78,6 +78,10 @@ async function settle(page: Page) { // Side terminal tabs mount live xterm instances whose websocket error text // lands at indeterminate times. const SKIP = new Set([ + "agentmanager--diff-panel-cached-worktree-switch", + "agentmanager--diff-panel-viewport-loading", + "agentmanager--diff-panel-interrupted-loading", + "agentmanager--file-tree-virtualized-large", "agentmanager--worktree-item-busy", "agentmanager--full-screen-diff-agent-edit-scroll", "agentmanager--side-terminal-panel-tabs", diff --git a/packages/kilo-vscode/tests/visual-regression.spec.ts b/packages/kilo-vscode/tests/visual-regression.spec.ts index 392b7b23df..3544ea7178 100644 --- a/packages/kilo-vscode/tests/visual-regression.spec.ts +++ b/packages/kilo-vscode/tests/visual-regression.spec.ts @@ -82,6 +82,10 @@ async function settle(page: Page) { // appearance baseline. const SKIP = new Set([ "chat--chat-view-session-dock-stability", + "agentmanager--diff-panel-cached-worktree-switch", + "agentmanager--diff-panel-viewport-loading", + "agentmanager--diff-panel-interrupted-loading", + "agentmanager--file-tree-virtualized-large", "agentmanager--worktree-item-busy", "agentmanager--full-screen-diff-agent-edit-scroll", "agentmanager--side-terminal-panel-tabs", diff --git a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx index 6c1f949435..5ca5493bd3 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx @@ -8,6 +8,7 @@ import { createMemo, createEffect, on, + untrack, onMount, onCleanup, type Component, @@ -1693,6 +1694,7 @@ const AgentManagerContent: Component = () => { const id = review.id() if ((panel || active) && id) { + untrack(() => diffs.retain(id)) vscode.postMessage({ type: "agentManager.startDiffWatch", projectId: activeProjectId(), ...wireDiffId(id) }) return } @@ -2668,10 +2670,7 @@ const AgentManagerContent: Component = () => { revertingFiles={revertCtl.revertingFor} activeTerminalId={terms.activeId()} contexts={() => new Set(worktrees().map((wt) => wt.id))} - onEvict={(key) => { - composers.drop(key) - diffs.drop(key) - }} + onEvict={(key) => composers.drop(key)} /> = (props) => { open, loading: () => props.loadingFiles, send: () => (props.active === false ? undefined : props.onRequestDiff), + eager: false, }) // --- CRUD --- @@ -606,13 +607,16 @@ export const DiffPanel: Component = (props) => { const isLargeCollapsed = () => isLargeDiffFile(diff) && !open().includes(diff.file) const isLoadingDetail = () => props.loadingFiles?.has(diff.file) ?? false const fileCommentCount = () => (commentsByFile().get(diff.file) ?? []).length + const viewport = createDiffViewport(scroller) createEffect(() => { - if (diff.kind === "image" && open().includes(diff.file)) request(diff) + if (props.active === false || !viewport.visible() || !open().includes(diff.file)) return + request(diff) }) return ( = (props) => { diffStyle={props.diffStyle ?? "unified"} sizeKey={diffSizeKey(props.sessionKey, diff, props.diffStyle ?? "unified")} virtualized={shouldVirtualizeDiff(diff)} + visible={viewport.visible() && props.active !== false} annotations={annotationsForFile(diff.file)} renderAnnotation={buildAnnotation} enableGutterUtility={true} diff --git a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanelCache.tsx b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanelCache.tsx index 0bf696f83c..59f6ed17fc 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/DiffPanelCache.tsx +++ b/packages/kilo-vscode/webview-ui/agent-manager/DiffPanelCache.tsx @@ -5,7 +5,7 @@ import type { ReviewComposer } from "../diff-viewer/review-annotations" import { DiffPanel } from "./DiffPanel" import { diffDataKey } from "./worktree-diffs" -const CACHE_SIZE = 4 +const CACHE_SIZE = 16 interface Entry { key: string diff --git a/packages/kilo-vscode/webview-ui/agent-manager/agent-manager-review.css b/packages/kilo-vscode/webview-ui/agent-manager/agent-manager-review.css index f9334bdfa6..1cd57e1ad6 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/agent-manager-review.css +++ b/packages/kilo-vscode/webview-ui/agent-manager/agent-manager-review.css @@ -192,11 +192,6 @@ padding: 4px 6px; } -.am-file-tree-group { - display: flex; - flex-direction: column; -} - /* Shared row base for files and directories */ .am-file-tree-dir, .am-file-tree-file { 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 d3c8961a14..11674a7db6 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/agent-manager.css +++ b/packages/kilo-vscode/webview-ui/agent-manager/agent-manager.css @@ -1858,6 +1858,7 @@ body.am-wt-dragging-active * { min-width: 0; min-height: 0; opacity: 0; + content-visibility: hidden; pointer-events: none; z-index: 0; background: var(--surface-base); @@ -1865,6 +1866,7 @@ body.am-wt-dragging-active * { .am-diff-panel-cache-active { opacity: 1; + content-visibility: visible; pointer-events: auto; z-index: 2; } @@ -4305,11 +4307,6 @@ body.am-wt-dragging-active * { padding: 4px 6px; } -.am-file-tree-group { - display: flex; - flex-direction: column; -} - /* Shared row base for files and directories */ .am-file-tree-dir, .am-file-tree-file { 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 350ba0ea5c..f1c2590cd2 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/worktree-diffs.ts +++ b/packages/kilo-vscode/webview-ui/agent-manager/worktree-diffs.ts @@ -19,6 +19,9 @@ import type { WorktreeFileDiff, } from "../src/types/messages" +const LIMIT = 16 +const BUDGET = 64 * 1024 * 1024 + /** * Decompose a composite diff id (`ctx#scope`, or `ctx#session:`) into the * wire fields the extension expects. Bare ids (no scope separator) parse to @@ -42,10 +45,14 @@ export function createWorktreeDiffs( const diffLoading = () => Object.keys(diffLoadings()).length > 0 const [diffNotices, setDiffNotices] = createSignal>({}) const [diffFileLoading, setDiffFileLoading] = createSignal>>({}) + const sizes = new Map() + let bytes = 0 const key = (id: string) => diffDataKey(project(), id) const reset = () => { + sizes.clear() + bytes = 0 setDiffDatas({}) setDiffLoadings({}) setDiffNotices({}) @@ -54,6 +61,11 @@ export function createWorktreeDiffs( const drop = (id: string) => { const data = id.includes("\0") ? id : key(id) + const size = sizes.get(data) + if (size !== undefined) { + bytes -= size + sizes.delete(data) + } const remove = >(prev: T): T => { if (!(data in prev)) return prev const next = { ...prev } @@ -66,6 +78,32 @@ export function createWorktreeDiffs( setDiffFileLoading(remove) } + const retain = (id: string) => { + const data = id.includes("\0") ? id : key(id) + const entries = diffDatas()[data] + if (!entries) return + const size = entries.reduce( + (total, item) => + total + + 2 * + (item.before.length + + item.after.length + + (item.patch?.length ?? 0) + + (item.image?.before?.data?.length ?? 0) + + (item.image?.after?.data?.length ?? 0)), + 0, + ) + bytes -= sizes.get(data) ?? 0 + sizes.delete(data) + sizes.set(data, size) + bytes += size + while ((sizes.size > LIMIT || bytes > BUDGET) && sizes.size > 1) { + const oldest = sizes.keys().next().value + if (!oldest) return + drop(oldest) + } + } + const setDiffFilePending = (sessionId: string, file: string, value: boolean) => { setDiffFileLoading((prev) => { const session = prev[sessionId] ?? {} @@ -143,6 +181,7 @@ export function createWorktreeDiffs( if (existing && existing.length === next.length && existing.every((old, i) => old === next[i])) return prev return { ...prev, [data]: next } }) + retain(data) if (staleFiles) refreshStaleDiffs(ev.sessionId, staleFiles, data, ev.projectId) } @@ -154,6 +193,7 @@ export function createWorktreeDiffs( const next = existing.map((item) => (item.file === ev.diff!.file ? ev.diff! : item)) return { ...prev, [data]: next } }) + retain(data) setDiffFilePending(data, ev.diff.file, false) return } @@ -190,6 +230,7 @@ export function createWorktreeDiffs( diffFileLoadingFor, diffLoadingFor, diffDataKey, + retain, drop, reset, onWorktreeDiff, diff --git a/packages/kilo-vscode/webview-ui/diff-viewer/FileTree.tsx b/packages/kilo-vscode/webview-ui/diff-viewer/FileTree.tsx index 449c922ea0..e630c81c1a 100644 --- a/packages/kilo-vscode/webview-ui/diff-viewer/FileTree.tsx +++ b/packages/kilo-vscode/webview-ui/diff-viewer/FileTree.tsx @@ -1,4 +1,5 @@ import { type Component, createSignal, createMemo, For, Show } from "solid-js" +import { Virtualizer } from "virtua/solid" import { FileIcon } from "@kilocode/kilo-ui/file-icon" import { Icon } from "@kilocode/kilo-ui/icon" import { IconButton } from "@kilocode/kilo-ui/icon-button" @@ -20,71 +21,7 @@ interface FileTreeProps { showSummary?: boolean } -const DirectoryNode: Component<{ - node: FileTreeNode - activeFile: string | null - onFileSelect: (path: string) => void - depth: number - commentsByFile?: Map - selectedFiles?: Set - onFileToggle?: (path: string, checked: boolean) => void - onRevertFile?: (path: string) => void - revertingFiles?: Set -}> = (props) => { - const [expanded, setExpanded] = createSignal(true) - const hasActiveDescendant = createMemo(() => { - if (!props.activeFile) return false - return props.activeFile.startsWith(props.node.path + "/") - }) - - return ( -
- - - - {(child) => ( - - } - > - - - )} - - -
- ) -} +type Row = { node: FileTreeNode; depth: number } const FileNode: Component<{ node: FileTreeNode @@ -183,7 +120,44 @@ const FileNode: Component<{ export const FileTree: Component = (props) => { const { t } = useLanguage() - const tree = createMemo(() => flatten(buildFileTree(props.diffs))) + const cache = new Map() + let signature = "" + const tree = createMemo((previous) => { + const key = props.diffs + .map((diff) => `${diff.file}\0${diff.status}\0${diff.additions}\0${diff.deletions}`) + .join("\n") + if (key === signature) return previous + signature = key + cache.clear() + return flatten(buildFileTree(props.diffs)) + }, []) + const [collapsed, setCollapsed] = createSignal(new Set()) + const [scroller, setScroller] = createSignal() + const rows = createMemo(() => { + const result: Row[] = [] + const hidden = collapsed() + const visit = (nodes: FileTreeNode[], depth: number) => { + for (const node of nodes) { + const item = cache.get(node.path) ?? { node, depth } + cache.set(node.path, item) + result.push(item) + if (node.children && !hidden.has(node.path)) visit(node.children, depth + 1) + } + } + visit(tree(), 0) + return result + }) + const toggle = (path: string) => { + setCollapsed((prev) => { + const next = new Set(prev) + if (next.has(path)) { + next.delete(path) + return next + } + next.add(path) + return next + }) + } const commentsByFile = createMemo(() => { const map = new Map() for (const comment of props.comments ?? []) { @@ -197,41 +171,47 @@ export const FileTree: Component = (props) => { return { files: props.diffs.length, additions: adds, deletions: dels } }) + const row = (item: Row) => ( + + } + > + + + ) + return (
-
- - {(node) => ( - - } - > - - - )} - +
+ 100} fallback={{row}}> + + {(root) => ( + + {row} + + )} + +
diff --git a/packages/kilo-vscode/webview-ui/diff-viewer/FullScreenDiffView.tsx b/packages/kilo-vscode/webview-ui/diff-viewer/FullScreenDiffView.tsx index f15f6e617d..d76e1af0d7 100644 --- a/packages/kilo-vscode/webview-ui/diff-viewer/FullScreenDiffView.tsx +++ b/packages/kilo-vscode/webview-ui/diff-viewer/FullScreenDiffView.tsx @@ -63,7 +63,7 @@ import { VirtualDiffList } from "./VirtualDiffList" import { isMarkdownFile, MarkdownDiffView } from "./MarkdownDiffView" import { ImageDiffView } from "./ImageDiffView" import { createDiffRows, diffSizeKey } from "./diff-state" -import { createDiffRequests } from "./diff-requests" +import { createDiffRequests, createDiffViewport } from "./diff-requests" type DiffStyle = "unified" | "split" @@ -289,6 +289,7 @@ export const FullScreenDiffView: Component = (props) => open, loading: () => props.loadingFiles, send: () => props.onRequestDiff, + eager: false, }) // --- CRUD --- @@ -489,8 +490,9 @@ export const FullScreenDiffView: Component = (props) => } const handleFileSelect = (path: string) => { - setActiveFile(path) const diff = props.diffs.find((item) => item.file === path) + if (diff) request(diff) + setActiveFile(path) if (diff && isDiffExpandable(diff) && !open().includes(path)) setOpen((prev) => [...prev, path]) requestAnimationFrame(() => { const index = rows().findIndex((diff) => diff.file === path) @@ -677,13 +679,15 @@ export const FullScreenDiffView: Component = (props) => const isLargeCollapsed = () => isLargeDiffFile(diff) && !open().includes(diff.file) const isLoadingDetail = () => props.loadingFiles?.has(diff.file) ?? false const fileCommentCount = () => (commentsByFile().get(diff.file) ?? []).length + const viewport = createDiffViewport(scroller) createEffect(() => { - if (diff.kind === "image" && open().includes(diff.file)) request(diff) + if (!viewport.visible() || !open().includes(diff.file)) return + request(diff) }) return ( - +
@@ -815,6 +819,7 @@ export const FullScreenDiffView: Component = (props) => diffStyle={props.diffStyle} sizeKey={diffSizeKey(props.sessionKey, diff, props.diffStyle)} virtualized={shouldVirtualizeDiff(diff)} + visible={viewport.visible()} annotations={annotationsForFile(diff.file)} renderAnnotation={buildAnnotation} enableGutterUtility={props.canComment !== false} diff --git a/packages/kilo-vscode/webview-ui/diff-viewer/diff-open-policy.ts b/packages/kilo-vscode/webview-ui/diff-viewer/diff-open-policy.ts index 827f51e8d8..d3b379ce4d 100644 --- a/packages/kilo-vscode/webview-ui/diff-viewer/diff-open-policy.ts +++ b/packages/kilo-vscode/webview-ui/diff-viewer/diff-open-policy.ts @@ -2,6 +2,7 @@ import type { WorktreeFileDiff } from "../src/types/messages" export const LONG_DIFF_MARKER_FILE_COUNT = 50 export const EXTREME_DIFF_CHANGED_LINES = 2_000 +const MAX_EAGER_BYTES = 256 * 1024 export function isLargeDiffFile(diff: WorktreeFileDiff): boolean { return diff.additions + diff.deletions > EXTREME_DIFF_CHANGED_LINES @@ -10,7 +11,9 @@ export function isLargeDiffFile(diff: WorktreeFileDiff): boolean { // The outer file-row virtualizer bounds the review DOM. Pierre only needs its // nested line virtualizer when a single file is extreme or lacks a hunk patch. export function shouldVirtualizeDiff(diff: WorktreeFileDiff): boolean { - return !diff.patch || isLargeDiffFile(diff) + return ( + !diff.patch || isLargeDiffFile(diff) || diff.before.length > MAX_EAGER_BYTES || diff.after.length > MAX_EAGER_BYTES + ) } export function isDiffExpandable(diff: WorktreeFileDiff): boolean { 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 8d51779566..52d24f8863 100644 --- a/packages/kilo-vscode/webview-ui/diff-viewer/diff-requests.ts +++ b/packages/kilo-vscode/webview-ui/diff-viewer/diff-requests.ts @@ -1,4 +1,4 @@ -import { createEffect, on, type Accessor } from "solid-js" +import { createEffect, createSignal, on, onCleanup, type Accessor } from "solid-js" import type { WorktreeFileDiff } from "../src/types/messages" import { isDiffExpandable } from "./diff-open-policy" import { diffToken } from "./diff-state" @@ -9,6 +9,53 @@ interface DiffRequestOptions { open: Accessor loading: Accessor | undefined> send: Accessor<((file: string) => void) | undefined> + eager?: boolean +} + +type Watch = { observer: IntersectionObserver; entries: Map void> } + +const watchers = new WeakMap() + +function observeDiffRequest(node: Element, root: Element, run: (visible: boolean) => void): () => void { + if (typeof IntersectionObserver === "undefined") { + run(true) + return () => {} + } + + let state = watchers.get(root) + if (!state) { + const entries = new Map void>() + const observer = new IntersectionObserver( + (items) => { + for (const item of items) entries.get(item.target)?.(item.isIntersecting) + }, + { root, rootMargin: "200px 0px" }, + ) + state = { observer, entries } + watchers.set(root, state) + } + + state.entries.set(node, run) + state.observer.observe(node) + return () => { + state.entries.delete(node) + state.observer.unobserve(node) + if (state.entries.size > 0) return + state.observer.disconnect() + if (watchers.get(root) === state) watchers.delete(root) + } +} + +export function createDiffViewport(root: Accessor) { + const [element, setElement] = createSignal() + const [visible, setVisible] = createSignal(false) + createEffect(() => { + const node = element() + const viewport = root() + if (!node || !viewport) return + onCleanup(observeDiffRequest(node, viewport, setVisible)) + }) + return { ref: (node: Element) => setElement(node), visible } } export function createDiffRequests(opts: DiffRequestOptions) { @@ -52,6 +99,7 @@ export function createDiffRequests(opts: DiffRequestOptions) { for (const file of requested.keys()) { if (!files.has(file)) requested.delete(file) } + if (opts.eager === false) return for (const file of open) { const diff = diffs.find((item) => item.file === file) if (!diff || diff.kind === "image") continue diff --git a/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx b/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx index 6c6b87899a..e28d7939c0 100644 --- a/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx +++ b/packages/kilo-vscode/webview-ui/src/stories/agent-manager.stories.tsx @@ -8,6 +8,8 @@ import type { Meta, StoryObj } from "storybook-solidjs-vite" import { StoryProviders, defaultMockData, mockSessionValue, t } from "./StoryProviders" import { FileTree } from "../../diff-viewer/FileTree" import { DiffPanel } from "../../agent-manager/DiffPanel" +import { DiffPanelCache } from "../../agent-manager/DiffPanelCache" +import { createReviewComposers } from "../../agent-manager/review-composers" import { FullScreenDiffView } from "../../diff-viewer/FullScreenDiffView" import { WorktreeItem } from "../../agent-manager/WorktreeItem" import { ChatView } from "../components/chat/ChatView" @@ -305,6 +307,37 @@ export const FileTreeEmpty: Story = { ), } +export const FileTreeVirtualizedLarge: Story = { + name: "FileTree - virtualized large review", + render: () => { + const diffs = Array.from({ length: 600 }, (_, index): WorktreeFileDiff => { + const group = String(Math.floor(index / 30)).padStart(2, "0") + const file = String(index).padStart(4, "0") + return { + file: `src/group-${group}/file-${file}.ts`, + before: "", + after: "", + patch: "", + additions: 1, + deletions: 0, + status: "modified", + tracked: true, + generatedLike: false, + summarized: true, + } + }) + const [selected, setSelected] = createSignal(diffs[0]!.file) + + return ( + +
+ +
+ + ) + }, +} + // --------------------------------------------------------------------------- // DiffPanel // --------------------------------------------------------------------------- @@ -352,6 +385,197 @@ export const DiffPanelScrollUp: Story = { }, } +export const DiffPanelCachedWorktreeSwitch: Story = { + name: "DiffPanel - switch cached worktrees without blank frames", + render: () => { + const ids = Array.from({ length: 12 }, (_, index) => `worktree-${index + 1}`) + const [current, setCurrent] = createSignal(ids[0]!) + const values = Object.fromEntries(ids.map((id) => [`single\0${id}#branch`, [edited(id, `src/${id}.ts`)]])) + const composers = createReviewComposers(() => undefined) + + return ( + +
+
+ {ids.map((id) => ( + + ))} +
+
+ `${current()}#branch`} + context={current} + project={() => undefined} + active={() => true} + contexts={() => new Set(ids)} + data={() => values} + loading={() => false} + loadingFiles={() => new Set()} + notice={() => undefined} + comments={() => []} + setComments={() => {}} + composer={composers.get} + lead={() => Branch} + canRevert={false} + diffStyle="unified" + onDiffStyleChange={() => {}} + markdownRender={false} + onMarkdownRenderChange={() => {}} + onSendClick={() => {}} + onClose={() => {}} + onRequestDiff={() => {}} + onOpenFile={() => {}} + onOpenDocument={() => {}} + onRevertFile={() => {}} + revertingFiles={() => new Set()} + /> +
+
+ + ) + }, +} + +export const DiffPanelViewportLoading: Story = { + name: "DiffPanel - load only visible file details", + render: () => { + const [entries, setEntries] = createSignal( + Array.from({ length: 120 }, (_, index) => ({ + file: `src/file-${String(index).padStart(3, "0")}.ts`, + before: "", + after: "", + patch: "", + additions: 1, + deletions: 1, + status: "modified", + tracked: true, + generatedLike: false, + summarized: true, + stamp: "1:1", + })), + ) + const [requested, setRequested] = createSignal([]) + const [offscreen, setOffscreen] = createSignal([]) + const load = (file: string) => { + const root = document.querySelector("[data-testid=viewport-diff-review] .am-diff-content") + const row = root?.querySelector(`[data-file-path="${CSS.escape(file)}"]`) + if (root && row) { + const box = root.getBoundingClientRect() + const rect = row.getBoundingClientRect() + if (rect.bottom < box.top - 201 || rect.top > box.bottom + 201) setOffscreen((prev) => [...prev, file]) + } + setRequested((prev) => (prev.includes(file) ? prev : [...prev, file])) + queueMicrotask(() => { + setEntries((prev) => + prev.map((item) => + item.file === file + ? { + ...item, + before: "before\n", + after: "after\n", + patch: `--- a/${file}\n+++ b/${file}\n@@ -1 +1 @@\n-before\n+after\n`, + summarized: false, + } + : item, + ), + ) + }) + } + + return ( + +
+ {}} + comments={[]} + onCommentsChange={() => {}} + onClose={() => {}} + onRequestDiff={load} + /> +
+ + ) + }, +} + +export const DiffPanelInterruptedLoading: Story = { + name: "DiffPanel - resume interrupted visible file", + render: () => { + const [active, setActive] = createSignal(true) + const [count, setCount] = createSignal(0) + const [loading, setLoading] = createSignal(new Set()) + const [entries, setEntries] = createSignal([ + { file: "src/resume.ts", before: "", after: "", patch: "", additions: 1, deletions: 1, summarized: true }, + ]) + const request = (file: string) => { + setCount((value) => value + 1) + setLoading(new Set([file])) + if (count() === 1) return + queueMicrotask(() => { + setEntries([ + { + ...entries()[0]!, + before: "before\n", + after: "after\n", + patch: "--- a/src/resume.ts\n+++ b/src/resume.ts\n@@ -1 +1 @@\n-before\n+after\n", + summarized: false, + }, + ]) + setLoading(new Set()) + }) + } + return ( + +
+ + +
+ {}} + comments={[]} + onCommentsChange={() => {}} + onClose={() => {}} + onRequestDiff={request} + /> +
+
+ + ) + }, +} + const buttonFixtureStyle: JSX.CSSProperties = { display: "inline-flex", "align-items": "center",