mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-19 10:02:04 +08:00
fix(upstream): stop finder hanging on big assets
Replace the subprocess-based line diff in classifyDrift with an in-process multiset diff (approxDiff) so concurrent classifications don't deadlock on big text files like sprite.svg via Bun $.quiet() pipe-buffer stalls. Also filter non-code assets (SVG, PNG, fonts, archives, lock files, etc.) before classification so they don't bloat the report or stress git subprocesses. Switch kilo-only path excludes to glob pathspecs so every packages/kilo-*/ and **/kilocode/** dir is excluded without maintaining a hand list. Rename the 'whitespace-only' bucket to 'cosmetic-only' — with the multiset diff it also catches line reordering, so the old label was misleading.
This commit is contained in:
@@ -274,23 +274,25 @@ Options:
|
||||
--concurrency <n> Parallel classifications (default: 8).
|
||||
```
|
||||
|
||||
The command pre-filters with `git diff --name-only <last-merged-upstream>..HEAD`, excluding kilo-only paths (`packages/kilo-*`, `packages/opencode/src/kilocode/`, `packages/opencode/test/kilocode/`, `script/upstream/`), then classifies each file against the transformed upstream baseline:
|
||||
The command pre-filters with `git diff --name-only <last-merged-upstream>..HEAD`, excluding kilo-only paths (anything under `packages/kilo-*/`, any `**/kilocode/**` subdir, `script/upstream/`) and non-code assets (SVG, PNG, fonts, archives, lock files, etc. — see `SKIP_EXTENSIONS` / `SKIP_FILENAMES` in the script). The remaining text files are classified against the transformed upstream baseline:
|
||||
|
||||
| Bucket | Meaning | Action |
|
||||
|---|---|---|
|
||||
| `identical` | Local bytes already match transformed upstream (branding-only drift in raw git diff) | none |
|
||||
| `markers-only` | Stripping `kilocode_change` markers makes local match upstream | reset |
|
||||
| `whitespace-only` | Only non-marker diff is whitespace | reset |
|
||||
| `small-diff` | ≤ `--review-limit` non-marker, non-whitespace diff lines | reset |
|
||||
| `large-diff` | > `--review-limit` non-marker, non-whitespace diff lines | skipped |
|
||||
| `cosmetic-only` | Non-marker diff is only whitespace or reordered lines (the line multiset is identical) | reset |
|
||||
| `small-diff` | ≤ `--review-limit` non-marker, non-cosmetic diff lines | reset |
|
||||
| `large-diff` | > `--review-limit` non-marker, non-cosmetic diff lines | skipped |
|
||||
| `upstream-missing` | File does not exist upstream (kilo-only, intentional) | skipped |
|
||||
| `local-missing` | File tracked but missing locally (deleted in Kilo) | skipped |
|
||||
| `binary-diff` | Binary file differs | skipped (use `reset-to-upstream.ts` per file) |
|
||||
| `binary-identical` | Binary file already matches | none |
|
||||
|
||||
`markers-only`, `whitespace-only`, and `small-diff` buckets are auto-reset unless `--dry-run` is passed. A markdown summary is printed to stdout so you can review what happened and spot-check the resulting `git diff`. All resets land as uncommitted working-tree changes; `git diff` / `git checkout` is your safety net.
|
||||
Line counting uses an in-process multiset diff (pure JS, no subprocess) for speed and robustness against concurrent git output stalls on big files. Moved/reordered lines therefore count as zero drift, which is usually what you want for "is this file meaningfully different from upstream".
|
||||
|
||||
Tighten the blast radius with `--review-limit 0` (only `markers-only` and `whitespace-only`) or by scoping with a `path` argument (e.g. `packages/opencode/src/mcp`).
|
||||
`markers-only`, `cosmetic-only`, and `small-diff` buckets are auto-reset unless `--dry-run` is passed. A markdown summary is printed to stdout so you can review what happened and spot-check the resulting `git diff`. All resets land as uncommitted working-tree changes; `git diff` / `git checkout` is your safety net.
|
||||
|
||||
Tighten the blast radius with `--review-limit 0` (only `markers-only` and `cosmetic-only`) or by scoping with a `path` argument (e.g. `packages/opencode/src/mcp`).
|
||||
|
||||
## Using Custom Base Branches
|
||||
|
||||
|
||||
@@ -46,25 +46,52 @@ interface Entry extends ClassifyResult {
|
||||
reset?: boolean
|
||||
}
|
||||
|
||||
const KILO_ONLY_PATHS = [
|
||||
"packages/kilo-docs",
|
||||
"packages/kilo-gateway",
|
||||
"packages/kilo-i18n",
|
||||
"packages/kilo-indexing",
|
||||
"packages/kilo-jetbrains",
|
||||
"packages/kilo-telemetry",
|
||||
"packages/kilo-ui",
|
||||
"packages/kilo-vscode",
|
||||
"packages/opencode/src/kilocode",
|
||||
"packages/opencode/test/kilocode",
|
||||
"script/upstream",
|
||||
const KILO_ONLY_PATHSPECS = [
|
||||
":(exclude,glob)packages/kilo-*/**",
|
||||
":(exclude,glob)**/kilocode/**",
|
||||
":(exclude)script/upstream",
|
||||
]
|
||||
|
||||
const RESET_BUCKETS = new Set<Bucket>(["markers-only", "whitespace-only", "small-diff"])
|
||||
// Non-code assets never make sense to bulk-reset. Big binary-ish files (large
|
||||
// SVG sprites, icons, fonts, archives) also stress concurrent git subprocesses
|
||||
// and hide real drift in the report. Use reset-to-upstream.ts per file if you
|
||||
// really want to restore one of these.
|
||||
const SKIP_EXTENSIONS = new Set([
|
||||
".svg",
|
||||
".png",
|
||||
".jpg",
|
||||
".jpeg",
|
||||
".gif",
|
||||
".webp",
|
||||
".avif",
|
||||
".ico",
|
||||
".bmp",
|
||||
".woff",
|
||||
".woff2",
|
||||
".ttf",
|
||||
".otf",
|
||||
".eot",
|
||||
".zip",
|
||||
".tar",
|
||||
".gz",
|
||||
".br",
|
||||
".wasm",
|
||||
".bin",
|
||||
".db",
|
||||
".sqlite",
|
||||
".mp3",
|
||||
".mp4",
|
||||
".mov",
|
||||
".pdf",
|
||||
])
|
||||
|
||||
const SKIP_FILENAMES = new Set(["bun.lock", "package-lock.json", "yarn.lock", "pnpm-lock.yaml", "Cargo.lock"])
|
||||
|
||||
const RESET_BUCKETS = new Set<Bucket>(["markers-only", "cosmetic-only", "small-diff"])
|
||||
|
||||
const BUCKET_ORDER: Bucket[] = [
|
||||
"markers-only",
|
||||
"whitespace-only",
|
||||
"cosmetic-only",
|
||||
"small-diff",
|
||||
"large-diff",
|
||||
"identical",
|
||||
@@ -127,17 +154,37 @@ function args(): Args {
|
||||
}
|
||||
}
|
||||
|
||||
async function candidates(commit: string, scope: string | undefined, top: string): Promise<string[]> {
|
||||
const pathspecs = [scope ?? ".", ...KILO_ONLY_PATHS.map((p) => `:(exclude)${p}`)]
|
||||
async function candidates(
|
||||
commit: string,
|
||||
scope: string | undefined,
|
||||
top: string,
|
||||
): Promise<{ files: string[]; skippedAssets: string[] }> {
|
||||
const pathspecs = [scope ?? ".", ...KILO_ONLY_PATHSPECS]
|
||||
const result = await $`git diff --name-only ${commit}..HEAD -- ${pathspecs}`.cwd(top).quiet().nothrow()
|
||||
if (result.exitCode !== 0) {
|
||||
throw new Error(`Failed to list candidate files: ${result.stderr.toString()}`)
|
||||
}
|
||||
return result.stdout
|
||||
const all = result.stdout
|
||||
.toString()
|
||||
.split("\n")
|
||||
.map((line) => line.trim())
|
||||
.filter((line) => line.length > 0)
|
||||
|
||||
const files: string[] = []
|
||||
const skippedAssets: string[] = []
|
||||
for (const file of all) {
|
||||
if (asset(file)) skippedAssets.push(file)
|
||||
else files.push(file)
|
||||
}
|
||||
return { files, skippedAssets }
|
||||
}
|
||||
|
||||
function asset(file: string): boolean {
|
||||
const base = file.slice(file.lastIndexOf("/") + 1)
|
||||
if (SKIP_FILENAMES.has(base)) return true
|
||||
const dot = base.lastIndexOf(".")
|
||||
if (dot === -1) return false
|
||||
return SKIP_EXTENSIONS.has(base.slice(dot).toLowerCase())
|
||||
}
|
||||
|
||||
async function concurrent<T, R>(items: T[], limit: number, fn: (item: T, index: number) => Promise<R>): Promise<R[]> {
|
||||
@@ -167,8 +214,8 @@ function group(entries: Entry[]): Map<Bucket, Entry[]> {
|
||||
|
||||
function describe(bucket: Bucket, count: number, dryRun: boolean): { label: string; action: string } {
|
||||
if (bucket === "markers-only") return { label: `markers-only (${count})`, action: dryRun ? "would reset" : "reset" }
|
||||
if (bucket === "whitespace-only")
|
||||
return { label: `whitespace-only (${count})`, action: dryRun ? "would reset" : "reset" }
|
||||
if (bucket === "cosmetic-only")
|
||||
return { label: `cosmetic-only (${count})`, action: dryRun ? "would reset" : "reset" }
|
||||
if (bucket === "small-diff") return { label: `small-diff (${count})`, action: dryRun ? "would reset" : "reset" }
|
||||
if (bucket === "large-diff") return { label: `large-diff (${count})`, action: "skipped" }
|
||||
if (bucket === "identical") return { label: `identical (${count})`, action: "nothing to do" }
|
||||
@@ -178,7 +225,15 @@ function describe(bucket: Bucket, count: number, dryRun: boolean): { label: stri
|
||||
return { label: `local-missing (${count})`, action: "skipped" }
|
||||
}
|
||||
|
||||
function report(entries: Entry[], dryRun: boolean, tag: string, commit: string, scope: string, limit: number) {
|
||||
function report(
|
||||
entries: Entry[],
|
||||
skippedAssets: string[],
|
||||
dryRun: boolean,
|
||||
tag: string,
|
||||
commit: string,
|
||||
scope: string,
|
||||
limit: number,
|
||||
) {
|
||||
const grouped = group(entries)
|
||||
const lines: string[] = []
|
||||
|
||||
@@ -189,6 +244,7 @@ function report(entries: Entry[], dryRun: boolean, tag: string, commit: string,
|
||||
lines.push(`- Review limit: ${limit} non-marker diff line(s)`)
|
||||
lines.push(`- Mode: ${dryRun ? "dry-run (no writes)" : "auto-apply"}`)
|
||||
lines.push(`- Total candidates: ${entries.length}`)
|
||||
if (skippedAssets.length > 0) lines.push(`- Non-code assets skipped: ${skippedAssets.length}`)
|
||||
lines.push("")
|
||||
|
||||
lines.push(`## Summary`)
|
||||
@@ -201,6 +257,7 @@ function report(entries: Entry[], dryRun: boolean, tag: string, commit: string,
|
||||
const info = describe(bucket, items.length, dryRun)
|
||||
lines.push(`| ${bucket} | ${items.length} | ${info.action} |`)
|
||||
}
|
||||
if (skippedAssets.length > 0) lines.push(`| non-code-asset | ${skippedAssets.length} | skipped |`)
|
||||
lines.push("")
|
||||
|
||||
for (const bucket of BUCKET_ORDER) {
|
||||
@@ -241,9 +298,10 @@ async function main() {
|
||||
info(`Review limit: ${opts.reviewLimit} non-marker diff line(s)`)
|
||||
info(`Mode: ${opts.dryRun ? "dry-run" : "auto-apply"}`)
|
||||
|
||||
const files = await candidates(version.commit, scope, top)
|
||||
const { files, skippedAssets } = await candidates(version.commit, scope, top)
|
||||
if (skippedAssets.length > 0) info(`Skipping ${skippedAssets.length} non-code asset(s)`)
|
||||
if (files.length === 0) {
|
||||
success("No files differ from upstream in scope. Nothing to do.")
|
||||
success("No code files differ from upstream in scope. Nothing to do.")
|
||||
return
|
||||
}
|
||||
info(`Candidate files: ${files.length}`)
|
||||
@@ -272,7 +330,17 @@ async function main() {
|
||||
}
|
||||
|
||||
console.log("")
|
||||
console.log(report(entries, opts.dryRun, version.tag, version.commit, scope ?? "(all shared paths)", opts.reviewLimit))
|
||||
console.log(
|
||||
report(
|
||||
entries,
|
||||
skippedAssets,
|
||||
opts.dryRun,
|
||||
version.tag,
|
||||
version.commit,
|
||||
scope ?? "(all shared paths)",
|
||||
opts.reviewLimit,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
main().catch((err) => {
|
||||
|
||||
@@ -421,3 +421,28 @@ export async function changed(base: Text, head: Text, opts?: { ignoreWhitespace?
|
||||
await rm(dir, { recursive: true, force: true })
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure in-process line-diff used by bulk classifiers. Returns the number of
|
||||
* non-matching lines between two texts using a multiset approach (moving a line
|
||||
* around doesn't count as drift). Whitespace can optionally be ignored.
|
||||
*
|
||||
* Unlike `changed()`, this spawns no subprocesses so it is safe to run
|
||||
* concurrently without risking pipe-buffer deadlocks on large inputs.
|
||||
*/
|
||||
export function approxDiff(base: string, head: string, opts?: { ignoreWhitespace?: boolean }): number {
|
||||
if (base === head) return 0
|
||||
const norm = opts?.ignoreWhitespace ? (line: string) => line.replace(/\s+/g, " ").trim() : (line: string) => line
|
||||
const counts = new Map<string, number>()
|
||||
for (const line of base.split(/\r?\n/)) {
|
||||
const key = norm(line)
|
||||
counts.set(key, (counts.get(key) ?? 0) + 1)
|
||||
}
|
||||
for (const line of head.split(/\r?\n/)) {
|
||||
const key = norm(line)
|
||||
counts.set(key, (counts.get(key) ?? 0) - 1)
|
||||
}
|
||||
let total = 0
|
||||
for (const v of counts.values()) total += Math.abs(v)
|
||||
return total
|
||||
}
|
||||
|
||||
@@ -9,7 +9,7 @@
|
||||
|
||||
import { rm } from "node:fs/promises"
|
||||
import path from "node:path"
|
||||
import { binary, changed, clean, join } from "./markers"
|
||||
import { approxDiff, binary, clean, join } from "./markers"
|
||||
import { translate, upstreamData } from "./upstream"
|
||||
|
||||
export type ResetAction = "identical" | "deleted" | "written" | "skipped"
|
||||
@@ -72,7 +72,7 @@ export async function resetFile(opts: ResetOptions): Promise<ResetResult> {
|
||||
export type Bucket =
|
||||
| "identical"
|
||||
| "markers-only"
|
||||
| "whitespace-only"
|
||||
| "cosmetic-only"
|
||||
| "small-diff"
|
||||
| "large-diff"
|
||||
| "upstream-missing"
|
||||
@@ -120,14 +120,12 @@ export async function classifyDrift(opts: ClassifyOptions): Promise<ClassifyResu
|
||||
if (local === null) return { bucket: "local-missing" }
|
||||
if (local === translated) return { bucket: "identical" }
|
||||
|
||||
const cleanedLocal = clean(opts.file, local)
|
||||
const cleanedUpstream = clean(opts.file, translated)
|
||||
if (join(cleanedLocal.text) === join(cleanedUpstream.text)) return { bucket: "markers-only" }
|
||||
const cleanedLocal = join(clean(opts.file, local).text)
|
||||
const cleanedUpstream = join(clean(opts.file, translated).text)
|
||||
if (cleanedLocal === cleanedUpstream) return { bucket: "markers-only" }
|
||||
|
||||
const wsDiff = await changed(cleanedUpstream.text, cleanedLocal.text, { ignoreWhitespace: true })
|
||||
if (wsDiff.lines.size === 0 && wsDiff.deleted === 0) return { bucket: "whitespace-only" }
|
||||
|
||||
const count = wsDiff.lines.size + wsDiff.deleted
|
||||
const count = approxDiff(cleanedUpstream, cleanedLocal, { ignoreWhitespace: true })
|
||||
if (count === 0) return { bucket: "cosmetic-only" }
|
||||
if (count <= opts.reviewLimit) return { bucket: "small-diff", lines: count }
|
||||
return { bucket: "large-diff", lines: count }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user