Concurrent callers raced past the tree-sitter load and got regex - #51
Merged
Conversation
…got regex
getAnalyzer gated its load on a boolean that flipped the instant the first
caller arrived:
if (!treeSitterLoadAttempted && extensionGrammarsPath) {
treeSitterLoadAttempted = true; // immediately
...
treeSitterAnalyzer = await ... // much later
}
if (treeSitterAnalyzer?.supportedExtensions.has(ext)) ...
The symbol indexer calls getAnalyzer CONCURRENTLY for every file in the
workspace via Promise.allSettled. All but the first saw the flag already set,
skipped the load block, found treeSitterAnalyzer still null, and took the regex
fallback. The grammars loaded correctly the whole time — into a variable nobody
was waiting on.
Pre-existing, and invisible until #47: before that fix the module could not be
imported at all, so nothing reached this. Fixing the import exposed it, and the
symptom was identical — a graph that still looked regex-built after installing
the fixed build.
Diagnosed from a real install rather than by reading. The SideCar log showed
grammars loading at 13:51:15, and the graph written that same second carried 6
`method` symbols against tree-sitter's 1487 while matching regex on every other
kind. `method` is the categorical tell: the regex analyzer emits none at all,
so counting them beats comparing two similar function totals.
The load is now memoized as a promise every caller awaits. Measured at 43ms for
28 extensions, so awaiting it is free — note that contradicts the 3m20s figure
in getGrammarsPath's comment, which appears stale.
The regression test fires 50 concurrent calls, because one or two do not
reproduce it. Mutation-checked: restoring the fire-and-forget shape reports
"50 of 50 concurrent callers fell back to regex".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅ All checks passed
Posted by SideCarAI-Bot |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The reason tree-sitter still was not running after #48 shipped it.
The race
The symbol indexer calls
getAnalyzerconcurrently for every file in theworkspace (
Promise.allSettledover ~1000 files). All but the first saw theflag already set, skipped the load block, found the analyzer still null, and
took the regex fallback.
The grammars loaded correctly the whole time — into a variable nobody awaited.
Why every previous test passed
A sequential call always worked. Every direct test of
createTreeSitterAnalyzer, including the ones I added in #45 and #48, called itonce and got a working analyzer. Only concurrency exposes this, and nothing
tested that.
Diagnosed from a real install
The SideCar output channel showed grammars loading at
13:51:15, and the graphwritten that same second was regex-shaped:
methodis the categorical discriminator — the regex analyzer emits none atall, so their near-absence is proof rather than a judgement call about two
similar function counts. I initially mis-read this myself with a
method > 0threshold that called 6 a success.
Cost
Memoizing the load means every caller awaits it. Measured: 43ms for 28
extensions, so it is free.
Worth flagging that this contradicts the comment on
getGrammarsPath, whichclaims loading all grammars took "3m20s cold in the extension host". Either
that figure is stale or the host is far slower than Node; if the latter, this
delays first index rather than blocking the UI, and correctness beats a fast
wrong answer. Someone should re-measure in the host and fix whichever comment is
lying.
Testing
50 concurrent calls — one or two do not reproduce it. Mutation-checked:
restoring the fire-and-forget shape reports "50 of 50 concurrent callers fell
back to regex".
Relationship to #50
#50 bumps
GRAPH_VERSIONso existing caches repopulate. Still needed and stillcorrect — but it was not sufficient on its own, because a fresh rebuild also
used regex. Both are required for a user to actually get a tree-sitter graph.
npm run check: green.🤖 Generated with Claude Code