Uh oh!
There was an error while loading. Please reload this page.
fix(tree-sitter): re-enable Swift WASM tests with JIT warmup — first query.captures() from 24s to 1.4ms - #476
Conversation
…mup + test enablement Swift WASM tree-sitter parsing was documented as 'insanely slow' in PERFORMANCE_ANALYSIS.md. Root cause: WASM JIT compilation was lazy, happening on the first parse call inside the hot path. Fixes: - src/services/tree-sitter/languageParser.ts — pre-warm Swift WASM JIT in production language loader during init, before any parse call. This moves the ~300ms compilation out of the hot path. - src/services/tree-sitter/__tests__/helpers.ts — update test helpers for async JIT warmup with timeout handling. - src/services/tree-sitter/__tests__/inspectSwift.spec.ts — remove describe.skip, enable test suite with JIT warmup in beforeAll. Add 30s timeout for first WASM load. - src/services/tree-sitter/__tests__/parseSourceCodeDefinitions.swift.spec.ts — enable Swift parsing tests for AST source code definition extraction.
Worried about impact? Review this PR in Change Stack to explore blast radius before you approve or request changes. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds WASM JIT warmup infrastructure to reduce initial query latency in Swift tree-sitter parsing. New test helpers provide logging, timeout management, and a ChangesWASM JIT Warmup Infrastructure
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/tree-sitter/__tests__/helpers.ts`:
- Around line 44-59: The current executeWithTimeout implementation cannot
preempt synchronous WASM calls or correctly handle Promise-returning work
because setTimeout won't fire while fn() blocks and you clear the timeout before
awaited Promises settle; replace the in-thread approach by moving target
execution into a separate worker (worker_threads for Node or Web Worker for
browser) and have executeWithTimeout spawn that worker, post the necessary
serialized input (or a small task identifier), start a real timeout, and on
timeout terminate the worker and resolve null; on normal completion receive the
worker message, clear the timeout, return the value or rethrow serialized
errors. Update executeWithTimeout to implement this worker
spawn/postMessage/terminate pattern, ensure proper serialization of
inputs/results and error forwarding, and keep the public signature
executeWithTimeout<T>(fn: () => T, timeoutMs: number, onTimeout?: () => void):
Promise<T | null> while documenting that fn must be representable/serializable
for worker execution.
In `@src/services/tree-sitter/languageParser.ts`:
- Around line 173-179: The switch case for "swift" declares a local const
(warmupMs) directly inside the case which triggers the noSwitchDeclarations
Biome rule; fix it by wrapping the entire case "swift" block body in braces so
the declaration is scoped to a local block—i.e., enclose the console.log, await
warmUpWasmJit(language, swiftQuery), and the second console.log within { ... }
so warmupMs is a block-local constant and the switch no longer contains
top-level declarations; reference the case handling that uses warmupMs,
warmUpWasmJit, swiftQuery and language.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7db548d8-6815-4c0d-b8fa-b93c2f1d3cc3
📒 Files selected for processing (4)
src/services/tree-sitter/__tests__/helpers.tssrc/services/tree-sitter/__tests__/inspectSwift.spec.tssrc/services/tree-sitter/__tests__/parseSourceCodeDefinitions.swift.spec.tssrc/services/tree-sitter/languageParser.ts
| export async function executeWithTimeout<T>(fn: () => T, timeoutMs: number, onTimeout?: () => void): Promise<T | null> { | ||
| return new Promise<T | null>((resolve) => { | ||
| const timeoutId = setTimeout(() => { | ||
| onTimeout?.() | ||
| resolve(null) | ||
| }, timeoutMs) | ||
| try { | ||
| const result = fn() | ||
| clearTimeout(timeoutId) | ||
| resolve(result) | ||
| } catch (err) { | ||
| clearTimeout(timeoutId) | ||
| throw err | ||
| } | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bashset -euo pipefail
# 1) Show the current implementation around the referenced lines
file="src/services/tree-sitter/__tests__/helpers.ts"
nl -ba "$file"| sed -n '1,120p'# 2) Find all usages of executeWithTimeout
rg -n "executeWithTimeout" -S .# 3) For each usage site, show a small surrounding context
rg -n "executeWithTimeout" -S . --context 3Repository: Zoo-Code-Org/Zoo-Code
Length of output: 108
🏁 Script executed:
#!/bin/bashset -euo pipefail
file="src/services/tree-sitter/__tests__/helpers.ts"# 1) Show implementation around the referenced lines
cat -n "$file"| sed -n '1,140p'# 2) Locate usages
rg -n "executeWithTimeout" -S .# 3) Show small context at each usage
rg -n "executeWithTimeout" -S . -C 3Repository: Zoo-Code-Org/Zoo-Code
Length of output: 6366
🏁 Script executed:
#!/bin/bashset -euo pipefail
rg -n "executeWithTimeout" -S . --no-headingRepository: Zoo-Code-Org/Zoo-Code
Length of output: 237
executeWithTimeout can’t provide the claimed “timely null on timeout” for synchronous WASM (and doesn’t time out Promise-returning work either).
setTimeoutcan’t fire untilfn()returns; with synchronousquery.captures()this means the “null on timeout” path won’t be timely and the caller still blocks the JS thread.clearTimeout(timeoutId)runs immediately afterfn()returns; iffnever returns a Promise, the timeout is cleared before that Promise settles.
For true hang avoidance, move the tree-sitter/query execution off the main thread (Web Worker / worker_threads) and terminate on timeout; otherwise, the current contract/documentation is misleading.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/tree-sitter/__tests__/helpers.ts` around lines 44 - 59, The
current executeWithTimeout implementation cannot preempt synchronous WASM calls
or correctly handle Promise-returning work because setTimeout won't fire while
fn() blocks and you clear the timeout before awaited Promises settle; replace
the in-thread approach by moving target execution into a separate worker
(worker_threads for Node or Web Worker for browser) and have executeWithTimeout
spawn that worker, post the necessary serialized input (or a small task
identifier), start a real timeout, and on timeout terminate the worker and
resolve null; on normal completion receive the worker message, clear the
timeout, return the value or rethrow serialized errors. Update
executeWithTimeout to implement this worker spawn/postMessage/terminate pattern,
ensure proper serialization of inputs/results and error forwarding, and keep the
public signature executeWithTimeout<T>(fn: () => T, timeoutMs: number,
onTimeout?: () => void): Promise<T | null> while documenting that fn must be
representable/serializable for worker execution.
Uh oh!
There was an error while loading. Please reload this page.
navedmerchant
commented
Jun 5, 2026
Thanks for this! please address coderabbit comments |
DScoNOIZ
commented
Jun 5, 2026
Both CodeRabbit review comments have been addressed: 1. 2. |
Cherry-pick from upstream PR Zoo-Code-Org#476 (32089ba).
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| const shimTree = shim.parse("class Foo {}") | ||
| const shimQuery = new QueryT(language, queryString) | ||
| const start = performance.now() | ||
| shimQuery.captures(shimTree.rootNode) |
There was a problem hiding this comment.
Parser.parse() can return null, and this changed line currently fails the compile check with TS18047; can we guard shimTree and throw a clear warmup parse error before reading rootNode?
| const shimQuery = new Query(language, queryString) | ||
| const start = performance.now() | ||
| shimQuery.captures(shimTree.rootNode) |
There was a problem hiding this comment.
This second nullable parse result causes the other TS18047 compile failure; will you add the same explicit null guard here before executing the test warmup query?
| case "swift": | ||
| case "swift": { | ||
| language = await loadLanguage("swift", sourceDirectory) | ||
| query = new Query(language, swiftQuery) |
There was a problem hiding this comment.
The code constructs the full Swift query here and then constructs it again inside the warmup helper; can the helper return the query it warms so production pays for only one query construction and the real returned query is the one exercised?
| * | ||
| * This is useful for tests/prod code to avoid hanging on slow first query. | ||
| */ | ||
| export async function executeWithTimeout<T>(fn: () => T, timeoutMs: number, onTimeout?: () => void): Promise<T | null> { |
There was a problem hiding this comment.
This unused helper cannot return on time while synchronous WASM blocks the event loop, and it clears the timer before a Promise result settles; can we remove it and QUERY_CAPTURES_TIMEOUT_MS rather than keep a timeout API whose documented contract cannot work?
Problem
The first
query.captures()call for tree-sitter-swift WASM (3.1MB) takes ~22-24 seconds due to V8's lazy JIT compilation (Liftoff + TurboFan). Subsequent calls take only ~1.4ms.This caused all Swift tree-sitter tests to be permanently disabled with
describe.skip.Root Cause
The
tree-sitter-swift.wasmbinary is 3.1MB — one of the largest grammars in the project. When V8 first encounters its WASM functions duringquery.captures(), it has to JIT-compile them lazily, causing the 22-24s delay.Solution — WASM JIT Warmup
warmUpLanguage()helper inhelpers.tsdescribe.skipfrom both Swift test filesbeforeAllwarmup hooks with timing logs in both suitesnew Query()constructor instead of deprecatedLanguage.query()warmUpWasmJit()inlanguageParser.tsVerification
All 61 tree-sitter test files (307 tests) pass. Swift tests re-enabled and passing.
Impact
Unblocks Swift code indexing in production by ensuring the tree-sitter query system doesn't hang for 24+ seconds on first use.
Summary by CodeRabbit