Add a Go indexer service with a gRPC streaming contract - #11
Merged
Conversation
Extracts the walk-and-parse stage behind rpc IndexRepo(IndexRequest) returns (stream
IndexEvent). Opt-in: IndexingService still uses the Python parser, and nothing changes
until indexer_grpc_enabled is set.
WHY, STATED HONESTLY: NOT SPEED
The obvious claim is false and was measured before writing any Go. On 900 files:
sequential 1115ms, ThreadPool(4) 1092ms (1.02x), ProcessPool(4) 393ms (2.84x).
py-tree-sitter never releases the GIL, so threads gain nothing -- which also means the
obvious anyio.to_thread fix would not have worked. Only ~41% of the parse phase is C, so a
perfect native rewrite ceilings near 2.4x, BELOW what ProcessPoolExecutor on the existing
Python already delivers for half a day of work.
The actual reason is a process and failure boundary: repos.py runs indexing by spinning a
new event loop inside an anyio threadpool thread and blocking it for minutes, in the same
process that serves chat. Server streaming is why gRPC rather than one big response --
progress becomes part of the contract instead of shared mutable state, the same bug class
that made the SSE progress bar show only 0% or 100%.
BINDING CHOICE
Official tree-sitter/go-tree-sitter, not smacker/go-tree-sitter -- which has roughly twice
the stars (562 vs 288) and outranks it in search results but was last pushed 2024-08-27
with 42 open issues and no deprecation notice. Verified against the GitHub API rather than
assumed, the same way the Terraform provider choice was checked.
TWO BUGS NOT REPEATED
Both were fixed in the Python parser earlier; this gets them right from the start.
- .tsx uses LanguageTSX(), not LanguageTypescript(). The plain TS grammar cannot parse JSX.
- Chunk text comes from node.Utf8Text(source) over the source BYTES, because tree-sitter
offsets are byte offsets and slicing a decoded string with them corrupts every chunk
after the first multi-byte character.
had_parse_error is on the wire because tree-sitter returns a partial tree rather than
failing, so without it the Python caller cannot decide to fall back to raw indexing.
ONE BUG FIXED IN THE PORT
Python's _find_files max-files cap never worked: its `break` left only the inner filename
loop, so os.walk continued into the next directory and kept appending. The Go walker uses
fs.SkipAll and reports Truncated, so a partial index is not mistaken for full coverage.
VERIFIED END TO END, Go server streaming to the Python client over real code
(apps/api/src): 5 progress events, 3 chunk batches, 509 chunks from 59 files in 99ms, 0
parse errors; kinds {module 17, class 121, function 109, method 262}; spot-check
class 'Level' in api/graphql/schema.py L47-53 matches the file. Correctness cases: a .tsx
component parses with had_parse_error=False under the tsx grammar, and after a line
containing "héllo — wörld 日本語 🎉" the chunks still slice as 'class Café:' and
'def método(self):' rather than misaligned fragments.
Suite: 168 passed, ruff clean. Generated *_pb2*.py are excluded from ruff, since any fix
there is overwritten by the next protoc run.
NOT DONE, deliberately: the service is not wired into the pipeline. That swap needs a
placement decision this commit does not make -- the service must be co-located with the API
on the shared volume, because clones live under ./data/repos/<owner>/<name> and that volume
attaches to exactly one machine. There is also no Go CI job yet (ci.yml has Python 3.11 and
Node 20 only, no cgo toolchain, no grammar-compile caching, no cross-language contract
test), and this build links 5 grammars against the Python side's 9 -- Health() reports which,
so the caller never assumes parity.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ShreeBohara
added a commit
that referenced
this pull request
Aug 13, 2026
PR #11 merged with no CI protection at all: ci.yml had Python 3.11 and Node 20 only, so nothing compiled the Go service, ran go vet, or checked that the two languages' generated stubs still agreed. Its automated review also never ran -- the Codex bot hit its usage limit -- so that code landed unguarded twice over. indexer-go: gofmt (excluding gen/, which the drift job owns), go vet, go test -race, and a cgo build. CGO_ENABLED=1 throughout and an explicit `cc --version` step, because every tree-sitter grammar is a cgo package with its own generated parser.c -- if the runner ever loses its C toolchain the failure should name that rather than surfacing as a link error. proto-codegen-drift: regenerates both the Go and Python stubs from proto/indexer.proto and fails if either differs from what is committed. Without it, editing the proto and regenerating only one side produces a server and client that disagree, and nothing catches it until runtime. Generators are PINNED (protoc 29.3, protoc-gen-go v1.36.12, protoc-gen-go-grpc v1.6.2, grpcio-tools 1.83.0) rather than @latest. Generated files embed the generator version ("protoc-gen-go v1.36.12", "Protobuf Python Version: 7.35.1"), so an unpinned plugin would eventually change the output and fail the check for a reason unrelated to the contract. I verified the round-trip is byte-identical on both sides with these versions before relying on a strict whole-file diff, and corrected a comment that had claimed the comparison was looser than it actually is. Go tests, which did not exist before, cover the properties that were only verified by hand: - .tsx parses under the tsx grammar with no parse error (the plain TypeScript grammar cannot parse JSX -- the bug that shipped in the Python parser) - chunks following a line containing non-ASCII text still slice as 'class Café:' and 'def método(self):' rather than misaligned fragments, because tree-sitter offsets are byte offsets - HadParseError is set on a syntax error rather than swallowed, since tree-sitter returns a partial tree instead of failing - MaxFiles actually stops the walk and sets Truncated -- the Python original's `break` left only the inner filename loop, so os.walk continued into the next directory - extension mapping, unlinked extensions, and the module-chunk fallback Also ran gofmt over the two source files it flagged, and validated the workflow YAML locally, which caught an unquoted colon in a step name that would have failed the run. Verified: gofmt clean, go vet clean, go test ./... passes both packages, cgo build succeeds, proto round-trip byte-identical, workflow parses with 4 jobs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Extracts the walk-and-parse stage behind
rpc IndexRepo(IndexRequest) returns (stream IndexEvent). Opt-in —IndexingServicestill uses the Python parser and nothing changes until enabled.Why — and it is not speed
The obvious claim ("rewrote the parser in Go for performance") is false, and I measured it before writing any Go:
py-tree-sitternever releases the GIL, so threads are ~1.0x — which also means the obviousanyio.to_threadfix would not have worked. Only ~41% of the parse phase is C, so a perfect native rewrite ceilings near 2.4x, below the 2.84x thatProcessPoolExecutoron the existing Python already gives for half a day's work.The real reason is a process and failure boundary:
repos.pyruns indexing by spinning a new event loop inside an anyio threadpool thread and blocking it for minutes, in the same process that serves chat.Server streaming is why gRPC rather than one big response — progress becomes part of the contract instead of shared mutable state, the same bug class that made the SSE progress bar show only 0% or 100%.
Binding choice, verified not assumed
Uses the official
tree-sitter/go-tree-sitter. Notsmacker/go-tree-sitter, which is the trap — checked against the GitHub API:tree-sitter/go-tree-sittersmacker/go-tree-sitterThe more popular one is the stale one, and it outranks the official binding in search results.
Two bugs not repeated
Both were fixed in the Python parser earlier; this gets them right from the start, with tests.
.tsxusesLanguageTSX(), notLanguageTypescript()— the plain TS grammar cannot parse JSX.node.Utf8Text(source)over source bytes, because tree-sitter offsets are byte offsets.had_parse_erroris on the wire because tree-sitter returns a partial tree rather than failing — without it the Python caller cannot decide to fall back to raw indexing.One bug fixed in the port
Python's
_find_filesmax-files cap never worked: itsbreakleft only the inner filename loop, soos.walkcontinued into the next directory and kept appending. The Go walker usesfs.SkipAlland reportsTruncated, so a partial index isn't mistaken for full coverage.Verified end to end
Go server streaming to the Python client over real code (
apps/api/src):Correctness cases:
That second block is the byte-offset test — the chunks come after a line containing
héllo — wörld 日本語 🎉and still slice correctly.168 tests pass, ruff clean. Generated
*_pb2*.pyare excluded from ruff, since any fix there is overwritten by the nextprotocrun.Not done, deliberately
The service is not wired into the pipeline. That swap needs a placement decision this PR doesn't make: the service must be co-located with the API on the shared volume, because clones live under
./data/repos/<owner>/<name>and that volume attaches to exactly one machine.Also: no Go CI job (
ci.ymlhas Python 3.11 and Node 20 only — no cgo toolchain, no grammar-compile caching, no cross-language contract test), and this build links 5 grammars against the Python side's 9.Health()reports which, so the caller never assumes parity.🤖 Generated with Claude Code