mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-01 15:32:11 +08:00
core: drop the JavaScript diff fallback that's no longer needed
The git-based diff path has been reliable across all tests and in production since the TUI-freeze fix shipped. Removing the JS Myers fallback eliminates ~220 lines of belt-and-suspenders code (a size-cap guard, a git cat-file blob loader, and a per-file git show fallback). If git ever fails now, callers emit an empty patch string; additions/deletions from git --numstat still come through unchanged.
This commit is contained in:
@@ -5,8 +5,6 @@ import { AppFileSystem } from "@/filesystem"
|
||||
import { Git } from "@/git"
|
||||
import { Effect, Layer, Context } from "effect"
|
||||
import * as Stream from "effect/Stream"
|
||||
import { formatPatch, structuredPatch } from "diff"
|
||||
import { DiffEngine } from "@/kilocode/snapshot/diff-engine" // kilocode_change
|
||||
import { DiffFull } from "@/kilocode/snapshot/diff-full" // kilocode_change
|
||||
import fuzzysort from "fuzzysort"
|
||||
import ignore from "ignore"
|
||||
@@ -561,20 +559,10 @@ export namespace File {
|
||||
diff = yield* gitText(["-c", "core.fsmonitor=false", "diff", "--staged", "--", file])
|
||||
}
|
||||
if (diff.trim()) {
|
||||
// kilocode_change start — generate the full-context patch via git,
|
||||
// never through the JS Myers implementation
|
||||
// kilocode_change start — patch via git, never the JS Myers implementation
|
||||
const got = yield* DiffFull.file(gitText, file)
|
||||
if (got) return { type: "text" as const, content, patch: got.patch, diff: got.text }
|
||||
// kilocode_change end
|
||||
const original = yield* git.show(Instance.directory, "HEAD", file)
|
||||
// kilocode_change start — cap the Myers fallback so a git regression can't freeze the loop
|
||||
if (DiffEngine.shouldSkip(original, content)) return { type: "text" as const, content }
|
||||
// kilocode_change end
|
||||
const patch = structuredPatch(file, file, original, content, "old", "new", {
|
||||
context: Infinity,
|
||||
ignoreWhitespace: true,
|
||||
})
|
||||
return { type: "text" as const, content, patch, diff: formatPatch(patch) }
|
||||
}
|
||||
return { type: "text" as const, content }
|
||||
}
|
||||
|
||||
@@ -1,33 +0,0 @@
|
||||
// kilocode_change - fallback-only cap around the JS Myers path.
|
||||
//
|
||||
// Primary patch generation goes through `DiffFull.batch` / `DiffFull.file`
|
||||
// (git-based). This module exists solely so that if git fails and we fall
|
||||
// back to `structuredPatch`, we don't reintroduce the event-loop freeze on
|
||||
// huge-file diffs. Never hit in normal operation.
|
||||
|
||||
export namespace DiffEngine {
|
||||
/** Hard byte cap on a single side (before or after) of a diff. 512 KB. */
|
||||
export const MAX_INPUT_BYTES = 512 * 1024
|
||||
/** Hard line cap on a single side of a diff. */
|
||||
export const MAX_INPUT_LINES = 2000
|
||||
|
||||
function lines(text: string) {
|
||||
if (!text) return 0
|
||||
const len = text.length
|
||||
if (len === 0) return 0
|
||||
let count = 1
|
||||
for (let i = 0; i < len; i++) {
|
||||
if (text.charCodeAt(i) === 10) count++
|
||||
}
|
||||
// trailing newline does not create an extra line
|
||||
if (text.charCodeAt(len - 1) === 10) count--
|
||||
return count
|
||||
}
|
||||
|
||||
/** Returns true if the inputs are too big to run through `structuredPatch` safely. */
|
||||
export function shouldSkip(before: string, after: string): boolean {
|
||||
if (before.length > MAX_INPUT_BYTES || after.length > MAX_INPUT_BYTES) return true
|
||||
if (lines(before) > MAX_INPUT_LINES || lines(after) > MAX_INPUT_LINES) return true
|
||||
return false
|
||||
}
|
||||
}
|
||||
@@ -1,13 +1,14 @@
|
||||
// kilocode_change - new file
|
||||
//
|
||||
// Primary patch generation. Runs `git diff --unified=INT_MAX` to produce
|
||||
// Patch generation. Runs `git diff --unified=INT_MAX` to produce
|
||||
// unified-diff text for a set of files, instead of the npm `diff` package's
|
||||
// JS Myers implementation. Myers is O(N*M) with full context, so on
|
||||
// huge-file diffs it can block the event loop for minutes (the TUI freeze
|
||||
// where ESC stopped working after a turn).
|
||||
//
|
||||
// Both helpers fail soft: on any git error they return an empty value so
|
||||
// callers can fall back to the JS Myers path (guarded by `DiffEngine`).
|
||||
// callers emit an empty patch string. Additions/deletions come from
|
||||
// `git --numstat` and stay accurate.
|
||||
|
||||
import { Effect } from "effect"
|
||||
import { parsePatch } from "diff"
|
||||
@@ -35,8 +36,9 @@ export namespace DiffFull {
|
||||
* `files` entries must use forward slashes (git's output uses `/` even on
|
||||
* Windows); paths with backslashes will silently miss the suffix match.
|
||||
*
|
||||
* Returns an empty map if `files` is empty or git fails. Callers fall back
|
||||
* to the JS Myers path for any file that is missing from the map.
|
||||
* Returns an empty map if `files` is empty or git fails. Callers emit an
|
||||
* empty patch string for any file missing from the map; numstat-derived
|
||||
* additions/deletions stay accurate.
|
||||
*/
|
||||
export const batch = Effect.fn("DiffFull.batch")(function* (
|
||||
git: (cmd: string[]) => Effect.Effect<GitResult>,
|
||||
@@ -72,7 +74,7 @@ export namespace DiffFull {
|
||||
parseBatch(result.text, chunk, map)
|
||||
}
|
||||
if (failed) {
|
||||
log.info("git diff failed, falling back to JS Myers", {
|
||||
log.info("git diff failed, emitting empty patches for affected files", {
|
||||
chunksFailed: failed,
|
||||
filesTotal: files.length,
|
||||
stderr,
|
||||
@@ -84,7 +86,8 @@ export namespace DiffFull {
|
||||
/**
|
||||
* Generate a structured + unified diff for a single file in the working
|
||||
* tree vs HEAD using `git diff --ignore-all-space --unified=INT_MAX`.
|
||||
* Returns `null` if git produces no output (caller falls back to Myers).
|
||||
* Returns `null` if git produces no output (caller emits a content-only
|
||||
* response with no patch).
|
||||
*/
|
||||
export const file = Effect.fn("DiffFull.file")(function* (
|
||||
gitText: (args: string[]) => Effect.Effect<string>,
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import { Cause, Duration, Effect, Layer, Schedule, Semaphore, Context, Stream } from "effect"
|
||||
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"
|
||||
import { formatPatch, structuredPatch } from "diff"
|
||||
import path from "path"
|
||||
import z from "zod"
|
||||
import { makeRuntime } from "@/effect/run-service" // kilocode_change
|
||||
@@ -12,7 +11,6 @@ import { Config } from "../config/config"
|
||||
import { Global } from "../global"
|
||||
import { Hash } from "../util/hash"
|
||||
import { Log } from "../util/log"
|
||||
import { DiffEngine } from "../kilocode/snapshot/diff-engine" // kilocode_change
|
||||
import { DiffFull } from "../kilocode/snapshot/diff-full" // kilocode_change
|
||||
|
||||
export namespace Snapshot {
|
||||
@@ -527,144 +525,6 @@ export namespace Snapshot {
|
||||
const diffFull = Effect.fnUntraced(function* (from: string, to: string) {
|
||||
return yield* locked(
|
||||
Effect.gen(function* () {
|
||||
type Row = {
|
||||
file: string
|
||||
status: "added" | "deleted" | "modified"
|
||||
binary: boolean
|
||||
additions: number
|
||||
deletions: number
|
||||
}
|
||||
|
||||
type Ref = {
|
||||
file: string
|
||||
side: "before" | "after"
|
||||
ref: string
|
||||
}
|
||||
|
||||
const show = Effect.fnUntraced(function* (row: Row) {
|
||||
if (row.binary) return ["", ""]
|
||||
if (row.status === "added") {
|
||||
return [
|
||||
"",
|
||||
yield* git([...cfg, ...args(["show", `${to}:${row.file}`])]).pipe(
|
||||
Effect.map((item) => item.text),
|
||||
),
|
||||
]
|
||||
}
|
||||
if (row.status === "deleted") {
|
||||
return [
|
||||
yield* git([...cfg, ...args(["show", `${from}:${row.file}`])]).pipe(
|
||||
Effect.map((item) => item.text),
|
||||
),
|
||||
"",
|
||||
]
|
||||
}
|
||||
return yield* Effect.all(
|
||||
[
|
||||
git([...cfg, ...args(["show", `${from}:${row.file}`])]).pipe(Effect.map((item) => item.text)),
|
||||
git([...cfg, ...args(["show", `${to}:${row.file}`])]).pipe(Effect.map((item) => item.text)),
|
||||
],
|
||||
{ concurrency: 2 },
|
||||
)
|
||||
})
|
||||
|
||||
const load = Effect.fnUntraced(
|
||||
function* (rows: Row[]) {
|
||||
const refs = rows.flatMap((row) => {
|
||||
if (row.binary) return []
|
||||
if (row.status === "added")
|
||||
return [{ file: row.file, side: "after", ref: `${to}:${row.file}` } satisfies Ref]
|
||||
if (row.status === "deleted") {
|
||||
return [{ file: row.file, side: "before", ref: `${from}:${row.file}` } satisfies Ref]
|
||||
}
|
||||
return [
|
||||
{ file: row.file, side: "before", ref: `${from}:${row.file}` } satisfies Ref,
|
||||
{ file: row.file, side: "after", ref: `${to}:${row.file}` } satisfies Ref,
|
||||
]
|
||||
})
|
||||
if (!refs.length) return new Map<string, { before: string; after: string }>()
|
||||
|
||||
const proc = ChildProcess.make("git", [...cfg, ...args(["cat-file", "--batch"])], {
|
||||
cwd: state.directory,
|
||||
extendEnv: true,
|
||||
stdin: Stream.make(new TextEncoder().encode(refs.map((item) => item.ref).join("\n") + "\n")),
|
||||
})
|
||||
const handle = yield* spawner.spawn(proc)
|
||||
const [out, err] = yield* Effect.all(
|
||||
[Stream.mkUint8Array(handle.stdout), Stream.mkString(Stream.decodeText(handle.stderr))],
|
||||
{ concurrency: 2 },
|
||||
)
|
||||
const code = yield* handle.exitCode
|
||||
if (code !== 0) {
|
||||
log.info("git cat-file --batch failed during snapshot diff, falling back to per-file git show", {
|
||||
stderr: err,
|
||||
refs: refs.length,
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
const fail = (msg: string, extra?: Record<string, string>) => {
|
||||
log.info(msg, { ...extra, refs: refs.length })
|
||||
return undefined
|
||||
}
|
||||
|
||||
const map = new Map<string, { before: string; after: string }>()
|
||||
const dec = new TextDecoder()
|
||||
let i = 0
|
||||
for (const ref of refs) {
|
||||
let end = i
|
||||
while (end < out.length && out[end] !== 10) end += 1
|
||||
if (end >= out.length) {
|
||||
return fail(
|
||||
"git cat-file --batch returned a truncated header during snapshot diff, falling back to per-file git show",
|
||||
)
|
||||
}
|
||||
|
||||
const head = dec.decode(out.slice(i, end))
|
||||
i = end + 1
|
||||
const hit = map.get(ref.file) ?? { before: "", after: "" }
|
||||
if (head.endsWith(" missing")) {
|
||||
map.set(ref.file, hit)
|
||||
continue
|
||||
}
|
||||
|
||||
const match = head.match(/^[0-9a-f]+ blob (\d+)$/)
|
||||
if (!match) {
|
||||
return fail(
|
||||
"git cat-file --batch returned an unexpected header during snapshot diff, falling back to per-file git show",
|
||||
{ head },
|
||||
)
|
||||
}
|
||||
|
||||
const size = Number(match[1])
|
||||
if (!Number.isInteger(size) || size < 0 || i + size >= out.length || out[i + size] !== 10) {
|
||||
return fail(
|
||||
"git cat-file --batch returned truncated content during snapshot diff, falling back to per-file git show",
|
||||
{ head },
|
||||
)
|
||||
}
|
||||
|
||||
const text = dec.decode(out.slice(i, i + size))
|
||||
if (ref.side === "before") hit.before = text
|
||||
if (ref.side === "after") hit.after = text
|
||||
map.set(ref.file, hit)
|
||||
i += size + 1
|
||||
}
|
||||
|
||||
if (i !== out.length) {
|
||||
return fail(
|
||||
"git cat-file --batch returned trailing data during snapshot diff, falling back to per-file git show",
|
||||
)
|
||||
}
|
||||
|
||||
return map
|
||||
},
|
||||
Effect.scoped,
|
||||
Effect.catch(() =>
|
||||
Effect.succeed<Map<string, { before: string; after: string }> | undefined>(undefined),
|
||||
),
|
||||
)
|
||||
|
||||
const result: Snapshot.FileDiff[] = []
|
||||
const status = new Map<string, "added" | "deleted" | "modified">()
|
||||
|
||||
@@ -704,7 +564,7 @@ export namespace Snapshot {
|
||||
binary,
|
||||
additions: Number.isFinite(additions) ? additions : 0,
|
||||
deletions: Number.isFinite(deletions) ? deletions : 0,
|
||||
} satisfies Row,
|
||||
},
|
||||
]
|
||||
})
|
||||
|
||||
@@ -717,37 +577,25 @@ export namespace Snapshot {
|
||||
}
|
||||
|
||||
const step = 100
|
||||
const patch = (file: string, before: string, after: string) =>
|
||||
formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER }))
|
||||
|
||||
for (let i = 0; i < rows.length; i += step) {
|
||||
const run = rows.slice(i, i + step)
|
||||
// kilocode_change start — bulk-load patches from `git diff` so Myers never runs on the main thread
|
||||
// kilocode_change start — bulk-load patches from `git diff`; no JS Myers fallback
|
||||
const patches = yield* DiffFull.batch(
|
||||
(cmd) => git([...quote, ...args(cmd)], { cwd: state.directory }),
|
||||
from,
|
||||
to,
|
||||
run.filter((r) => !r.binary).map((r) => r.file),
|
||||
)
|
||||
// kilocode_change end
|
||||
const text = yield* load(run)
|
||||
|
||||
for (const row of run) {
|
||||
const hit = text?.get(row.file) ?? { before: "", after: "" }
|
||||
const [before, after] = row.binary ? ["", ""] : text ? [hit.before, hit.after] : yield* show(row)
|
||||
// kilocode_change start — prefer the git-diff output; if we have to fall back to Myers,
|
||||
// cap it so a future git regression can't reintroduce the event-loop freeze
|
||||
const got = patches.get(row.file)
|
||||
const capped = !got && !row.binary && DiffEngine.shouldSkip(before, after)
|
||||
result.push({
|
||||
file: row.file,
|
||||
patch: row.binary ? "" : (got ?? (capped ? "" : patch(row.file, before, after))),
|
||||
patch: row.binary ? "" : (patches.get(row.file) ?? ""),
|
||||
additions: row.additions,
|
||||
deletions: row.deletions,
|
||||
status: row.status,
|
||||
})
|
||||
// kilocode_change end
|
||||
}
|
||||
// kilocode_change end
|
||||
}
|
||||
|
||||
return result
|
||||
|
||||
@@ -1,22 +0,0 @@
|
||||
import { test, expect } from "bun:test"
|
||||
import { DiffEngine } from "../../src/kilocode/snapshot/diff-engine"
|
||||
import { Log } from "../../src/util/log"
|
||||
|
||||
Log.init({ print: false })
|
||||
|
||||
test("shouldSkip returns false for small inputs", () => {
|
||||
expect(DiffEngine.shouldSkip("a", "b")).toBe(false)
|
||||
expect(DiffEngine.shouldSkip("hello\nworld", "hello\nworld!")).toBe(false)
|
||||
})
|
||||
|
||||
test("shouldSkip returns true when bytes exceed MAX_INPUT_BYTES", () => {
|
||||
const big = "x".repeat(DiffEngine.MAX_INPUT_BYTES + 1)
|
||||
expect(DiffEngine.shouldSkip(big, "small")).toBe(true)
|
||||
expect(DiffEngine.shouldSkip("small", big)).toBe(true)
|
||||
})
|
||||
|
||||
test("shouldSkip returns true at exactly MAX_INPUT_LINES + 1", () => {
|
||||
const many = "a\n".repeat(DiffEngine.MAX_INPUT_LINES + 1)
|
||||
expect(DiffEngine.shouldSkip(many, "small")).toBe(true)
|
||||
expect(DiffEngine.shouldSkip("small", many)).toBe(true)
|
||||
})
|
||||
Reference in New Issue
Block a user