-
Notifications
You must be signed in to change notification settings - Fork 3.1k
perf(vscode): parallelize build validation and cache SDK generation #12807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1191,23 +1191,29 @@ | |
| }, | ||
| "scripts": { | ||
| "prepare:cli-binary": "bun script/local-bin.ts", | ||
| "compile": "bun run prepare:cli-binary -- --force && bun run rebuild-sdk && bun run typecheck && bun run lint && node esbuild.js", | ||
| "prepare:sdk": "bun script/prepare-sdk.ts", | ||
| "build:launch": "bun run prepare:cli-binary && bun run prepare:sdk && bun run build:check:production", | ||
| "compile": "bun run prepare:cli-binary -- --force && bun run rebuild-sdk && bun run build:check", | ||
| "watch": "bun run rebuild-sdk && bun run --parallel watch:esbuild watch:tsc", | ||
| "watch:esbuild": "bun run prepare:cli-binary && node esbuild.js --watch", | ||
| "watch:tsc": "tsc --noEmit --watch --project tsconfig.json", | ||
| "watch:cli": "bun script/watch-cli.ts", | ||
| "package": "bun run prepare:cli-binary && bun run rebuild-sdk && bun run typecheck && bun run lint && node esbuild.js --production", | ||
| "package": "bun run prepare:cli-binary && bun run rebuild-sdk && bun run build:check:production", | ||
| "build:check": "bun run --parallel check-types check-types:webview lint bundle", | ||
| "build:check:production": "bun run --parallel check-types check-types:webview lint bundle:production", | ||
| "bundle": "bun esbuild.js", | ||
| "bundle:production": "bun esbuild.js --production", | ||
| "compile-tests": "tsc -p . --outDir out", | ||
| "watch-tests": "tsc -p . -w --outDir out", | ||
| "pretest": "bun run compile-tests && bun run compile && bun run lint", | ||
| "check-types": "tsc --noEmit", | ||
| "check-types:webview": "tsc --noEmit --project webview-ui/tsconfig.json", | ||
| "typecheck": "bun run check-types && bun run check-types:webview", | ||
| "check-types": "tsgo --noEmit", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: Every other package in the monorepo that runs Reply with |
||
| "check-types:webview": "tsgo --noEmit --project webview-ui/tsconfig.json", | ||
| "typecheck": "bun run --parallel check-types check-types:webview", | ||
| "format": "prettier --write .", | ||
| "format:check": "prettier --check .", | ||
| "knip": "knip", | ||
| "check-kilocode-change": "! grep -rIn 'kilocode_change' . ../kilo-ui/ --exclude='package.json' --exclude='*.md' --exclude-dir='node_modules' --exclude-dir='dist' | grep -v '`kilocode_change`'", | ||
| "lint": "eslint src webview-ui", | ||
| "lint": "eslint --cache --cache-strategy content --cache-location node_modules/.cache/eslint src webview-ui", | ||
| "test": "vscode-test", | ||
| "test:unit": "bun test tests/unit/ --dots", | ||
| "rebuild-sdk": "bun run --cwd ../sdk/js build", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| #!/usr/bin/env bun | ||
| import { $ } from "bun" | ||
| import { createHash } from "node:crypto" | ||
| import { existsSync, mkdirSync } from "node:fs" | ||
| import { dirname, join } from "node:path" | ||
|
|
||
| const root = join(import.meta.dir, "..") | ||
| const repo = join(root, "..", "..") | ||
| const sdk = join(repo, "packages", "sdk", "js") | ||
| const cache = join(root, "node_modules", ".cache", "sdk-build.json") | ||
| const inputs = [ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: The hand-maintained input allowlist omits packages that are in the OpenAPI generation module graph, so the cache can silently serve a stale SDK
Because a missed input produces a cache hit, the failure is silent: Suggest making this fail-safe rather than fail-open — e.g. fingerprint Reply with |
||
| "package.json", | ||
| "bun.lock", | ||
| "packages/opencode", | ||
| "packages/core", | ||
| "packages/effect-drizzle-sqlite", | ||
| "packages/effect-sqlite-node", | ||
| "packages/kilo-gateway", | ||
| "packages/kilo-indexing", | ||
| "packages/kilo-memory", | ||
| "packages/kilo-sandbox", | ||
| "packages/kilo-telemetry", | ||
| "packages/llm", | ||
| "packages/plugin", | ||
| "packages/plugin-atomic-chat", | ||
| "packages/server", | ||
| "packages/util", | ||
| "packages/sdk/js/package.json", | ||
| "packages/sdk/js/tsconfig.json", | ||
| "packages/sdk/js/script", | ||
| "packages/kilo-vscode/script/prepare-sdk.ts", | ||
| ] | ||
| const outputs = ["packages/sdk/js/src"] | ||
|
|
||
| function log(msg: string) { | ||
| console.log(`[prepare-sdk] ${msg}`) | ||
| } | ||
|
|
||
| async function fingerprint(paths: string[]) { | ||
| const [tree, diff, extra] = await Promise.all([ | ||
| $`git ls-tree -r HEAD -- ${paths}`.cwd(repo).quiet(), | ||
| $`git diff --binary HEAD -- ${paths}`.cwd(repo).quiet(), | ||
| $`git ls-files --others --exclude-standard -z -- ${paths}`.cwd(repo).quiet(), | ||
| ]) | ||
| const hash = createHash("sha256").update(tree.text()).update(diff.text()) | ||
| const files = extra.text().split("\0").filter(Boolean).sort() | ||
|
|
||
| for (const file of files) { | ||
| hash.update(file) | ||
| hash.update(new Uint8Array(await Bun.file(join(repo, file)).arrayBuffer())) | ||
| } | ||
|
|
||
| return hash.digest("hex") | ||
| } | ||
|
|
||
| async function load() { | ||
| const file = Bun.file(cache) | ||
| if (!(await file.exists())) return | ||
|
|
||
| try { | ||
| const value: unknown = await file.json() | ||
| if (!value || typeof value !== "object") return | ||
| const input = Reflect.get(value, "input") | ||
| const output = Reflect.get(value, "output") | ||
| if (typeof input === "string" && typeof output === "string") return { input, output } | ||
| } catch (err) { | ||
| log(`Ignoring invalid cache: ${err instanceof Error ? err.message : String(err)}`) | ||
| } | ||
| } | ||
|
|
||
| const input = await fingerprint(inputs) | ||
| const prior = await load() | ||
| const ready = existsSync(join(sdk, "dist", "index.js")) && existsSync(join(sdk, "dist", "v2", "index.js")) | ||
|
|
||
| if (prior?.input === input && prior.output === (await fingerprint(outputs)) && ready) { | ||
| log("SDK inputs and generated output are unchanged") | ||
| process.exit(0) | ||
| } | ||
|
|
||
| log("SDK inputs changed, rebuilding generated client") | ||
| await $`bun run build`.cwd(sdk) | ||
|
|
||
| mkdirSync(dirname(cache), { recursive: true }) | ||
| await Bun.write(cache, JSON.stringify({ input, output: await fingerprint(outputs) }) + "\n") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,9 @@ declare module "*.svg" { | |
| export default src | ||
| } | ||
|
|
||
| declare module "*.css" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SUGGESTION: These shorthand ambient declarations resolve to
Since these are side-effect-only imports ( Reply with |
||
| declare module "@kilocode/kilo-ui/styles" | ||
|
|
||
| declare module "*?worker&url" { | ||
| const src: string | ||
| export default src | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WARNING: Verify that
bun run --parallelpropagates a non-zero exit code when one of the parallel scripts failsbuild:check,build:check:production, andtypecheckare now the only gates for type and lint errors in this package (turbo typecheckin CI runstypecheck, and release validation runspackage). Before this PR the&&chain guaranteed that atsc/eslintfailure aborted the whole command.--parallelwas previously only used forwatch, where exit codes are irrelevant, so this is the first time correctness depends on it. If Bun aggregates only the last (or the first-finishing) script's status, type and lint errors will silently pass CI. Worth adding a one-off check (e.g. temporarily introduce a type error and confirmbun run typecheckexits non-zero).Secondary point that holds regardless: with parallel execution
bundle/bundle:productionnow writesdist/even whencheck-typesorlintfail, so a failedcompile/packageleaves behind a bundle that looks valid.script/launch.tsis safe because$throws, but anything that inspectsdist/after a failed build (e.g.--mode vsixreruns, cached CI steps) can now pick up artifacts that never passed validation.Reply with
@kilocode-bot fix itto have Kilo Code address this issue.