Add Go CI coverage and a cross-language proto drift check - #12
Merged
Conversation
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>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
PR #11 merged with no CI protection at all —
ci.ymlhad Python 3.11 and Node 20 only, so nothing compiled the Go service, rango 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-gogofmt (excluding
gen/, which the drift job owns),go vet,go test -race, and a cgo build.CGO_ENABLED=1throughout, plus an explicitcc --versionstep — every tree-sitter grammar is a cgo package with its own generatedparser.c, so if the runner ever loses its C toolchain the failure should name that rather than surface as a link error.proto-codegen-driftRegenerates both the Go and Python stubs from
proto/indexer.protoand fails if either differs from what's 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, not
@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 this 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 I'd written that claimed the comparison was looser than it actually was.
Go tests, which did not exist before
These cover the properties I'd previously only verified by hand:
.tsxparses under thetsxgrammar with no parse error — the plain TypeScript grammar cannot parse JSX, the bug that shipped in the Python parser'class Café:'and'def método(self):'rather than misaligned fragments, because tree-sitter offsets are byte offsetsHadParseErroris set on a syntax error rather than swallowed, since tree-sitter returns a partial tree instead of failingMaxFilesactually stops the walk and setsTruncated— the Python original'sbreakleft only the inner filename loop, soos.walkcontinued into the next directoryTwo things caught locally
Validating the workflow YAML before pushing caught an unquoted colon in a step name (
Verify C toolchain (required: grammars are cgo)) that YAML reads as a mapping — it would have failed the run. Andgofmtflagged two of my own source files.Verified
gofmt clean ·
go vetclean ·go test ./...passes both packages · cgo build succeeds · proto round-trip byte-identical · workflow parses with 4 jobs🤖 Generated with Claude Code