fix(vscode): keep large worktree reviews responsive

This commit is contained in:
marius-kilocode
2026-08-27 12:23:34 +02:00
parent f273509a68
commit b382a3b838
32 changed files with 2173 additions and 343 deletions
@@ -0,0 +1,6 @@
---
"kilo-code": patch
"@kilocode/kilo-ui": patch
---
Load Agent Manager changes faster and preserve cached diffs when switching sessions.
+6 -1
View File
@@ -196,12 +196,13 @@ export function Diff<T>(props: DiffProps<T>) {
"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<T>(props: DiffProps<T>) {
createEffect(() => {
if (visible()) return
if (local.visible) {
setVisible(true)
return
}
const cleanup = observe(container, () => setVisible(true))
onCleanup(cleanup)
})
+1
View File
@@ -53,6 +53,7 @@ type DiffShared<T> = FileDiffOptions<T> & {
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.
@@ -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<ExecResult> {
execGit(
args: string[],
cwd: string,
options?: { stdin?: string; signal?: AbortSignal; priority?: boolean },
): Promise<ExecResult> {
return this.exec(args, cwd, options)
}
execGitBuffer(args: string[], cwd: string, options?: { signal?: AbortSignal }): Promise<ExecBufferResult> {
execGitBuffer(
args: string[],
cwd: string,
options?: { stdin?: string; signal?: AbortSignal; priority?: boolean },
): Promise<ExecBufferResult> {
return this.execBuffer(args, cwd, options)
}
@@ -609,32 +618,40 @@ export class GitOps {
}
private async execBuffer(args: string[], cwd: string, options?: ExecOptions): Promise<ExecBufferResult> {
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<string> {
private executable(cancel?: AbortSignal): Promise<string> {
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<string>((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)
},
@@ -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<string, WorktreeDiffEntry | null>
deferred: Set<string>
}
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<number> {
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<string> {
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<string, Base>()
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<string, Base>) {
const items = metas.filter((meta) => meta.status !== "added")
const result = new Map<string, Buffer>()
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<string, string>()
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<Batch> {
const entries = new Map<string, WorktreeDiffEntry | null>()
const deferred = new Set<string>()
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 }
}
@@ -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<Value>
detail: (dir: string, anc: string, meta: Meta, signal?: AbortSignal) => Promise<WorktreeDiffEntry>
batch: (dir: string, anc: string, metas: Meta[]) => Promise<Batch>
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<Call>
started: boolean
work: Promise<Value>
resolve: (value: Value) => void
reject: (error: unknown) => void
}
const MAX_BATCH_FILES = 16
export function createDiffCache(load: Loader) {
const states = new Map<string, { anc: string; metas: Map<string, Meta> }>()
const generations = new Map<string, number>()
const details = new Map<string, { value: WorktreeDiffEntry; bytes: number }>()
const pending = new Map<string, { work: Promise<Value>; item?: Item }>()
const queues = new Map<string, Map<string, Item>>()
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<Value>, 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<Value>, signal?: AbortSignal, cancel?: () => void): Promise<Value> => {
if (signal?.aborted) return Promise.reject(new Error("Diff detail aborted"))
return new Promise<Value>((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<Value> => {
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<Value>((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<Value> => {
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<WorktreeDiffEntry[]> => {
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,
}
}
@@ -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<string> {
export async function resolveBase(git: GitOps, dir: string, base: string, signal?: AbortSignal): Promise<string> {
// 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<string | undefined> {
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<string | undefined> {
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<Meta[]> {
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<M
const result: Meta[] = []
const seen = new Set<string>()
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<Meta | undefined> => {
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<M
const files = untracked.stdout.trim()
if (!files) return result
const paths = files.split("\n").filter((file) => 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<Meta | undefined> => {
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<string, { anc: string; metas: Map<string, Meta> }>()
const generations = new Map<string, number>()
const details = new Map<string, { value: WorktreeDiffEntry; bytes: number; stamp: string }>()
const pending = new Map<string, { signal?: AbortSignal; work: Promise<WorktreeDiffEntry> }>()
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<WorktreeDiffEntry[]> => {
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<WorktreeDiffEntry | null> => {
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<Meta | undefined> {
async function detailMeta(
git: GitOps,
dir: string,
anc: string,
file: string,
signal?: AbortSignal,
): Promise<Meta | undefined> {
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<number> {
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<number> {
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<Buffer | undefined> {
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<string> {
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<string> {
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<WorktreeDiffEntry | null> {
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(
@@ -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<T>(fn: () => Promise<T>, signal?: AbortSignal): Promise<T> {
await this.acquire(signal)
async run<T>(fn: () => Promise<T>, signal?: AbortSignal, priority = false): Promise<T> {
await this.acquire(signal, priority)
try {
return await fn()
} finally {
@@ -20,14 +20,14 @@ export class Semaphore {
}
}
private acquire(signal?: AbortSignal): Promise<void> {
private acquire(signal: AbortSignal | undefined, priority: boolean): Promise<void> {
if (signal?.aborted) return Promise.reject(signal.reason)
if (this.running < this.limit) {
this.running++
return Promise.resolve()
}
return new Promise<void>((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)
})
}
@@ -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<string, string>()
@@ -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<void> {
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.
@@ -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 }
},
@@ -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<string, Element>()
for (let cycle = 0; cycle < 3; cycle++) {
for (let index = 1; index <= 12; index++) {
const id = `worktree-${index}`
const button = document.querySelector<HTMLButtonElement>(`[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)
})
@@ -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)
})
})
@@ -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")] })
@@ -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}")
`
@@ -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()
@@ -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<string>((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", () => {
@@ -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<void>): Prom
}
}
function deferred<T>() {
let resolve!: (value: T | PromiseLike<T>) => void
const promise = new Promise<T>((done) => {
resolve = done
})
return { promise, resolve }
}
function turn(): Promise<void> {
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<void> | 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<void>, begin: () => void): void {
this.test = test
this.wait = wait
this.begin = begin
this.held = true
}
private async pause(args: string[]): Promise<void> {
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<GitOps["execGit"]>[2]): Promise<ExecResult> {
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<GitOps["execGitBuffer"]>[2],
): Promise<ExecBufferResult> {
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<void> {
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<Awaited<ReturnType<typeof local.file>>>((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<void>()
const ready = deferred<void>()
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<void>()
const ready = deferred<void>()
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<void>()
const ready = deferred<void>()
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<void>()
const ready = deferred<void>()
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<void>()
const ready = deferred<void>()
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<void>()
const ready = deferred<void>()
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<void>()
const ready = deferred<void>()
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-"))
@@ -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<void>((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<void>((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
@@ -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<void>) {
type Opts = {
fetch?: (n: number) => Promise<void>
ready?: () => Promise<void> | undefined
git?: GitOps
trees?: Record<string, { id: string; path: string; parentBranch: string; remote: string }>
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<void>) {
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<void> {
throw new Error("waitFor timed out")
}
function defer() {
let resolve: () => void = () => {}
const promise = new Promise<void>((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<void>((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()
})
})
@@ -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<string>([
"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",
@@ -82,6 +82,10 @@ async function settle(page: Page) {
// appearance baseline.
const SKIP = new Set<string>([
"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",
@@ -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)}
/>
<Show when={sidePanel() === SidePanel.PR && activePR()}>
<PRPanelHost
@@ -70,7 +70,7 @@ import { treeOrder } from "../diff-viewer/file-tree-utils"
import { isMarkdownFile, MarkdownDiffView } from "../diff-viewer/MarkdownDiffView"
import { ImageDiffView } from "../diff-viewer/ImageDiffView"
import { createDiffRows, diffSizeKey } from "../diff-viewer/diff-state"
import { createDiffRequests } from "../diff-viewer/diff-requests"
import { createDiffRequests, createDiffViewport } from "../diff-viewer/diff-requests"
// --- Data model ---
@@ -290,6 +290,7 @@ export const DiffPanel: Component<DiffPanelProps> = (props) => {
open,
loading: () => props.loadingFiles,
send: () => (props.active === false ? undefined : props.onRequestDiff),
eager: false,
})
// --- CRUD ---
@@ -606,13 +607,16 @@ export const DiffPanel: Component<DiffPanelProps> = (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 (
<Accordion.Item
ref={viewport.ref}
value={diff.file}
data-slot="session-review-accordion-item"
data-file-path={diff.file}
@@ -759,6 +763,7 @@ export const DiffPanel: Component<DiffPanelProps> = (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}
@@ -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
@@ -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 {
@@ -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 {
@@ -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:<sid>`) 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<Record<string, string | undefined>>({})
const [diffFileLoading, setDiffFileLoading] = createSignal<Record<string, Record<string, true>>>({})
const sizes = new Map<string, number>()
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 = <T extends Record<string, unknown>>(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,
@@ -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<string, number>
selectedFiles?: Set<string>
onFileToggle?: (path: string, checked: boolean) => void
onRevertFile?: (path: string) => void
revertingFiles?: Set<string>
}> = (props) => {
const [expanded, setExpanded] = createSignal(true)
const hasActiveDescendant = createMemo(() => {
if (!props.activeFile) return false
return props.activeFile.startsWith(props.node.path + "/")
})
return (
<div class="am-file-tree-group">
<button
class={`am-file-tree-dir ${hasActiveDescendant() ? "am-file-tree-dir-highlight" : ""}`}
style={{ "padding-left": `${8 + props.depth * 12}px` }}
onClick={() => setExpanded((p) => !p)}
>
<Icon name={expanded() ? "chevron-down" : "chevron-right"} size="small" />
<Icon name="folder" size="small" />
<span class="am-file-tree-name">{props.node.name}</span>
</button>
<Show when={expanded()}>
<For each={props.node.children ?? []}>
{(child) => (
<Show
when={child.children}
fallback={
<FileNode
node={child}
activeFile={props.activeFile}
onFileSelect={props.onFileSelect}
depth={props.depth + 1}
commentsByFile={props.commentsByFile}
selectedFiles={props.selectedFiles}
onFileToggle={props.onFileToggle}
onRevertFile={props.onRevertFile}
revertingFiles={props.revertingFiles}
/>
}
>
<DirectoryNode
node={child}
activeFile={props.activeFile}
onFileSelect={props.onFileSelect}
depth={props.depth + 1}
commentsByFile={props.commentsByFile}
selectedFiles={props.selectedFiles}
onFileToggle={props.onFileToggle}
onRevertFile={props.onRevertFile}
revertingFiles={props.revertingFiles}
/>
</Show>
)}
</For>
</Show>
</div>
)
}
type Row = { node: FileTreeNode; depth: number }
const FileNode: Component<{
node: FileTreeNode
@@ -183,7 +120,44 @@ const FileNode: Component<{
export const FileTree: Component<FileTreeProps> = (props) => {
const { t } = useLanguage()
const tree = createMemo(() => flatten(buildFileTree(props.diffs)))
const cache = new Map<string, Row>()
let signature = ""
const tree = createMemo<FileTreeNode[]>((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<string>())
const [scroller, setScroller] = createSignal<HTMLDivElement>()
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<string, number>()
for (const comment of props.comments ?? []) {
@@ -197,41 +171,47 @@ export const FileTree: Component<FileTreeProps> = (props) => {
return { files: props.diffs.length, additions: adds, deletions: dels }
})
const row = (item: Row) => (
<Show
when={item.node.children}
fallback={
<FileNode
node={item.node}
activeFile={props.activeFile}
onFileSelect={props.onFileSelect}
depth={item.depth}
commentsByFile={commentsByFile()}
selectedFiles={props.selectedFiles}
onFileToggle={props.onFileToggle}
onRevertFile={props.onRevertFile}
revertingFiles={props.revertingFiles}
/>
}
>
<button
class={`am-file-tree-dir ${props.activeFile?.startsWith(item.node.path + "/") ? "am-file-tree-dir-highlight" : ""}`}
style={{ "padding-left": `${8 + item.depth * 12}px` }}
onClick={() => toggle(item.node.path)}
>
<Icon name={collapsed().has(item.node.path) ? "chevron-right" : "chevron-down"} size="small" />
<Icon name="folder" size="small" />
<span class="am-file-tree-name">{item.node.name}</span>
</button>
</Show>
)
return (
<div class="am-file-tree">
<div class="am-file-tree-list">
<For each={tree()}>
{(node) => (
<Show
when={node.children}
fallback={
<FileNode
node={node}
activeFile={props.activeFile}
onFileSelect={props.onFileSelect}
depth={0}
commentsByFile={commentsByFile()}
selectedFiles={props.selectedFiles}
onFileToggle={props.onFileToggle}
onRevertFile={props.onRevertFile}
revertingFiles={props.revertingFiles}
/>
}
>
<DirectoryNode
node={node}
activeFile={props.activeFile}
onFileSelect={props.onFileSelect}
depth={0}
commentsByFile={commentsByFile()}
selectedFiles={props.selectedFiles}
onFileToggle={props.onFileToggle}
onRevertFile={props.onRevertFile}
revertingFiles={props.revertingFiles}
/>
</Show>
)}
</For>
<div ref={setScroller} class="am-file-tree-list">
<Show when={props.diffs.length > 100} fallback={<For each={rows()}>{row}</For>}>
<Show when={scroller()}>
{(root) => (
<Virtualizer data={rows()} scrollRef={root()} itemSize={28} bufferSize={280}>
{row}
</Virtualizer>
)}
</Show>
</Show>
</div>
<Show when={props.showSummary !== false}>
<div class="am-file-tree-summary">
@@ -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<FullScreenDiffViewProps> = (props) =>
open,
loading: () => props.loadingFiles,
send: () => props.onRequestDiff,
eager: false,
})
// --- CRUD ---
@@ -489,8 +490,9 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (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<FullScreenDiffViewProps> = (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 (
<Accordion.Item value={diff.file} data-file-path={diff.file}>
<Accordion.Item ref={viewport.ref} value={diff.file} data-file-path={diff.file}>
<StickyAccordionHeader>
<Accordion.Trigger>
<div data-slot="session-review-trigger-content">
@@ -815,6 +819,7 @@ export const FullScreenDiffView: Component<FullScreenDiffViewProps> = (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}
@@ -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 {
@@ -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<string[]>
loading: Accessor<Set<string> | undefined>
send: Accessor<((file: string) => void) | undefined>
eager?: boolean
}
type Watch = { observer: IntersectionObserver; entries: Map<Element, (visible: boolean) => void> }
const watchers = new WeakMap<Element, Watch>()
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<Element, (visible: boolean) => 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<Element | undefined>) {
const [element, setElement] = createSignal<Element>()
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
@@ -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 (
<StoryProviders>
<div data-testid="large-file-tree" data-selected={selected()} style={{ width: "420px", height: "520px" }}>
<FileTree diffs={diffs} activeFile={selected()} onFileSelect={setSelected} />
</div>
</StoryProviders>
)
},
}
// ---------------------------------------------------------------------------
// 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 (
<StoryProviders noPadding>
<div style={{ height: "700px", display: "flex", "flex-direction": "column" }}>
<div data-testid="cached-worktree-tabs">
{ids.map((id) => (
<button type="button" data-testid={`select-${id}`} onClick={() => setCurrent(id)}>
{id}
</button>
))}
</div>
<div class="am-diff-panel-wrapper" style={{ flex: 1 }}>
<DiffPanelCache
current={() => `${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={() => <span>Branch</span>}
canRevert={false}
diffStyle="unified"
onDiffStyleChange={() => {}}
markdownRender={false}
onMarkdownRenderChange={() => {}}
onSendClick={() => {}}
onClose={() => {}}
onRequestDiff={() => {}}
onOpenFile={() => {}}
onOpenDocument={() => {}}
onRevertFile={() => {}}
revertingFiles={() => new Set()}
/>
</div>
</div>
</StoryProviders>
)
},
}
export const DiffPanelViewportLoading: Story = {
name: "DiffPanel - load only visible file details",
render: () => {
const [entries, setEntries] = createSignal<WorktreeFileDiff[]>(
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<string[]>([])
const [offscreen, setOffscreen] = createSignal<string[]>([])
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 (
<StoryProviders noPadding>
<div
data-testid="viewport-diff-review"
data-request-count={requested().length}
data-requested={requested().join("|")}
data-offscreen={offscreen().join("|")}
style={{ height: "700px", display: "flex", "flex-direction": "column" }}
>
<DiffPanel
diffs={entries()}
loading={false}
sessionKey="viewport-diff-review"
diffStyle="unified"
onDiffStyleChange={() => {}}
comments={[]}
onCommentsChange={() => {}}
onClose={() => {}}
onRequestDiff={load}
/>
</div>
</StoryProviders>
)
},
}
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<string>())
const [entries, setEntries] = createSignal<WorktreeFileDiff[]>([
{ 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<string>())
})
}
return (
<StoryProviders noPadding>
<div
data-testid="interrupted-review"
data-requests={count()}
style={{ height: "700px", display: "flex", "flex-direction": "column" }}
>
<button
data-testid="interrupt-review"
onClick={() => {
setActive(false)
setLoading(new Set<string>())
}}
>
Interrupt
</button>
<button data-testid="resume-review" onClick={() => setActive(true)}>
Resume
</button>
<div style={{ flex: 1, "min-height": 0, display: "flex", "flex-direction": "column" }}>
<DiffPanel
diffs={entries()}
loading={false}
active={active()}
loadingFiles={loading()}
sessionKey="interrupted-review"
diffStyle="unified"
onDiffStyleChange={() => {}}
comments={[]}
onCommentsChange={() => {}}
onClose={() => {}}
onRequestDiff={request}
/>
</div>
</div>
</StoryProviders>
)
},
}
const buttonFixtureStyle: JSX.CSSProperties = {
display: "inline-flex",
"align-items": "center",