diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d8112c1..75a16b8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -86,3 +86,110 @@ jobs: - name: Run Tests run: pnpm --filter web test + + indexer-go: + runs-on: ubuntu-latest + defaults: + run: + working-directory: ./services/indexer + + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: '1.23' + cache-dependency-path: services/indexer/go.sum + + # Every tree-sitter grammar is a cgo package with its own generated parser.c, so this + # job cannot use CGO_ENABLED=0 and needs a C toolchain. ubuntu-latest ships gcc, but + # pinning the expectation here makes the failure obvious if that ever changes. + - name: Verify C toolchain is present (grammars are cgo) + run: cc --version + + - name: Formatting + run: | + # gen/ is protoc output and is checked by the codegen-drift step instead. + unformatted=$(gofmt -l . | grep -v '^gen/' || true) + if [ -n "$unformatted" ]; then + echo "::error::gofmt needed for: $unformatted" + gofmt -d $unformatted + exit 1 + fi + + - name: Vet + run: CGO_ENABLED=1 go vet ./... + + - name: Test + run: CGO_ENABLED=1 go test -race ./... + + - name: Build + run: CGO_ENABLED=1 go build -o /tmp/indexer . + + # The contract is shared between two languages that generate stubs independently. Without + # this, editing indexer.proto and regenerating only one side produces a mismatch that + # nothing catches until runtime -- the Go server and Python client would simply disagree. + proto-codegen-drift: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: '1.23' + cache-dependency-path: services/indexer/go.sum + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Install protoc and plugins + run: | + PROTOC_VERSION=29.3 + curl -fsSL -o /tmp/protoc.zip \ + "https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-linux-x86_64.zip" + unzip -q /tmp/protoc.zip -d /tmp/protoc + echo "/tmp/protoc/bin" >> "$GITHUB_PATH" + # 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 that + # has nothing to do with the proto. These are the exact versions that produced the + # committed stubs; bump them and the stubs together. + go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.36.12 + go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.6.2 + echo "$(go env GOPATH)/bin" >> "$GITHUB_PATH" + python -m pip install --quiet "grpcio-tools==1.83.0" + + - name: Regenerate Go stubs + working-directory: ./services/indexer + run: | + protoc --proto_path=proto --go_out=gen --go_opt=paths=source_relative \ + --go-grpc_out=gen --go-grpc_opt=paths=source_relative proto/indexer.proto + + - name: Regenerate Python stubs + working-directory: ./apps/api + run: | + python -m grpc_tools.protoc \ + --proto_path=../../services/indexer/proto \ + --python_out=src/core/indexer --grpc_python_out=src/core/indexer \ + ../../services/indexer/proto/indexer.proto + # The generated grpc stub uses a flat import; the committed copy is package-relative. + sed -i 's/^import indexer_pb2 as indexer__pb2$/from src.core.indexer import indexer_pb2 as indexer__pb2/' \ + src/core/indexer/indexer_pb2_grpc.py + + - name: Fail if committed stubs differ from the proto + run: | + # A strict whole-file diff is safe because protoc, both Go plugins and grpcio-tools + # are all pinned above; the round-trip was verified byte-identical. + if ! git diff --quiet -- services/indexer/gen apps/api/src/core/indexer; then + echo "::error::Generated stubs are out of date with proto/indexer.proto." + echo "Regenerate both sides and commit; see services/indexer/README.md." + git diff --stat -- services/indexer/gen apps/api/src/core/indexer + git diff -- services/indexer/gen apps/api/src/core/indexer | head -60 + exit 1 + fi + echo "Generated stubs match the proto on both sides." diff --git a/services/indexer/README.md b/services/indexer/README.md index 7b21a25..0639128 100644 --- a/services/indexer/README.md +++ b/services/indexer/README.md @@ -61,6 +61,32 @@ CGO_ENABLED=1 go build -o bin/indexer . `CGO_ENABLED=1` is mandatory: every grammar is cgo. That means a C toolchain in any builder image, no `scratch` base, and a slower uncached build than the all-wheels Python image. +## Regenerating the stubs + +Both sides are generated from `proto/indexer.proto` and both are committed. CI regenerates +them and fails if they differ, so an edit to the proto that updates only one language +cannot merge. + +```bash +# Go +cd services/indexer +protoc --proto_path=proto --go_out=gen --go_opt=paths=source_relative \ + --go-grpc_out=gen --go-grpc_opt=paths=source_relative proto/indexer.proto + +# Python +cd apps/api +python -m grpc_tools.protoc --proto_path=../../services/indexer/proto \ + --python_out=src/core/indexer --grpc_python_out=src/core/indexer \ + ../../services/indexer/proto/indexer.proto +sed -i 's/^import indexer_pb2 as indexer__pb2$/from src.core.indexer import indexer_pb2 as indexer__pb2/' \ + src/core/indexer/indexer_pb2_grpc.py +``` + +Generator versions are **pinned in CI** (protoc 29.3, protoc-gen-go v1.36.12, +protoc-gen-go-grpc v1.6.2, grpcio-tools 1.83.0) because generated files embed the +generator version — an unpinned plugin would change the output and fail the drift check +for a reason unrelated to the contract. Bump the pins and the committed stubs together. + ## Grammar scope, stated rather than implied This build links **5** grammars (python, javascript, typescript, tsx, go) against the @@ -109,5 +135,6 @@ The service is **not wired into the indexing pipeline** — `IndexingService` st Python parser, and `IndexerClient` is opt-in. Doing that swap needs a decision this PR does not make: the service must be co-located with the API on the shared volume, because clones live under `./data/repos//` and a Fly/EBS-style volume attaches to exactly one -machine. There is also no CI job for Go yet (`ci.yml` has Python 3.11 and Node 20 only, no -cgo toolchain, no grammar-compile caching, no cross-language contract test). +machine. CI now covers this service: `indexer-go` runs gofmt, `go vet`, `go test -race` and a cgo +build, and `proto-codegen-drift` regenerates both languages' stubs and fails if they differ +from what is committed. diff --git a/services/indexer/internal/parse/parse.go b/services/indexer/internal/parse/parse.go index 60ae5a4..887cf99 100644 --- a/services/indexer/internal/parse/parse.go +++ b/services/indexer/internal/parse/parse.go @@ -97,9 +97,9 @@ var configs = map[string]langConfig{ var extToLang = map[string]string{ ".py": "python", ".js": "javascript", ".jsx": "javascript", - ".ts": "typescript", + ".ts": "typescript", ".tsx": "tsx", // never "typescript" - ".go": "go", + ".go": "go", } // LanguageFor reports the grammar for an extension, and whether one exists. diff --git a/services/indexer/internal/parse/parse_test.go b/services/indexer/internal/parse/parse_test.go new file mode 100644 index 0000000..3918ff6 --- /dev/null +++ b/services/indexer/internal/parse/parse_test.go @@ -0,0 +1,174 @@ +package parse + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func write(t *testing.T, dir, name, content string) string { + t.Helper() + p := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return p +} + +// The plain TypeScript grammar cannot parse JSX. Mapping .tsx to it produces a tree full +// of ERROR nodes for every .tsx file -- the exact bug that shipped in the Python parser. +func TestTSXUsesTheTSXGrammar(t *testing.T) { + dir := t.TempDir() + src := "export function Widget({ label }: { label: string }) {\n" + + " return
{label}
;\n}\n" + abs := write(t, dir, "Widget.tsx", src) + + chunks, err := File(abs, "Widget.tsx") + if err != nil { + t.Fatalf("parse: %v", err) + } + if len(chunks) == 0 { + t.Fatal("expected at least one chunk") + } + for _, c := range chunks { + if c.Language != "tsx" { + t.Errorf("language = %q, want tsx (the plain TS grammar cannot parse JSX)", c.Language) + } + if c.HadParseError { + t.Errorf("chunk %q has a parse error; JSX should parse cleanly under tsx", c.Name) + } + } +} + +// tree-sitter offsets are BYTE offsets. Slicing a decoded string with them corrupts every +// chunk after the first multi-byte character. +func TestChunksAfterNonASCIIAreNotMisaligned(t *testing.T) { + dir := t.TempDir() + src := "# héllo — wörld 日本語 🎉\n" + + "def after_unicode(x):\n return x * 2\n" + + "class Café:\n def método(self):\n return \"ok\"\n" + abs := write(t, dir, "unicode.py", src) + + chunks, err := File(abs, "unicode.py") + if err != nil { + t.Fatalf("parse: %v", err) + } + + byName := map[string]Chunk{} + for _, c := range chunks { + if c.Name != "" { + byName[c.Name] = c + } + } + + for _, want := range []struct{ name, prefix string }{ + {"after_unicode", "def after_unicode"}, + {"Café", "class Café:"}, + {"método", "def método(self):"}, + } { + got, ok := byName[want.name] + if !ok { + t.Errorf("missing chunk %q (got %v)", want.name, keys(byName)) + continue + } + if !strings.HasPrefix(got.Content, want.prefix) { + t.Errorf("chunk %q content = %q, want prefix %q -- byte/char offset mismatch", + want.name, truncateForMsg(got.Content), want.prefix) + } + } +} + +// tree-sitter returns a partial tree rather than failing on a syntax error, so the flag is +// the only way a caller can decide to fall back to raw indexing. +func TestSyntaxErrorIsReportedNotSwallowed(t *testing.T) { + dir := t.TempDir() + abs := write(t, dir, "broken.py", "def ok():\n return 1\n\nclass ((( :\n") + + chunks, err := File(abs, "broken.py") + if err != nil { + t.Fatalf("parse should not fail on a syntax error: %v", err) + } + if len(chunks) == 0 { + t.Fatal("expected chunks even from a file with errors") + } + found := false + for _, c := range chunks { + if c.HadParseError { + found = true + } + } + if !found { + t.Error("HadParseError was never set on a file with a syntax error") + } +} + +func TestExtensionMapping(t *testing.T) { + cases := map[string]string{ + ".py": "python", ".ts": "typescript", ".tsx": "tsx", + ".js": "javascript", ".jsx": "javascript", ".go": "go", + } + for ext, want := range cases { + got, ok := LanguageFor(ext) + if !ok || got != want { + t.Errorf("LanguageFor(%q) = %q,%v; want %q", ext, got, ok, want) + } + } + if _, ok := LanguageFor(".rs"); ok { + t.Error("LanguageFor(.rs) should report unlinked: this build has 5 grammars, not 9") + } +} + +func TestUnknownExtensionYieldsNoChunks(t *testing.T) { + dir := t.TempDir() + abs := write(t, dir, "notes.txt", "plain text") + chunks, err := File(abs, "notes.txt") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(chunks) != 0 { + t.Errorf("expected no chunks for an unlinked extension, got %d", len(chunks)) + } +} + +// A file with no extractable declarations still carries meaning; dropping it would lose +// content the Python side keeps. +func TestFileWithNoDeclarationsFallsBackToModuleChunk(t *testing.T) { + dir := t.TempDir() + abs := write(t, dir, "consts.py", "A = 1\nB = 2\n") + chunks, err := File(abs, "consts.py") + if err != nil { + t.Fatalf("parse: %v", err) + } + if len(chunks) != 1 || chunks[0].ChunkType != "module" { + t.Fatalf("want one module chunk, got %+v", chunks) + } +} + +func TestLinkedReportsEveryMappedExtension(t *testing.T) { + linked := Linked() + if len(linked) == 0 { + t.Fatal("Linked() must report grammars so callers never assume parity with Python") + } + if exts, ok := linked["tsx"]; !ok || len(exts) == 0 { + t.Error("tsx must be reported separately from typescript") + } +} + +func keys(m map[string]Chunk) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +} + +func truncateForMsg(s string) string { + if len(s) > 60 { + return s[:60] + "..." + } + return s +} diff --git a/services/indexer/internal/walk/walk.go b/services/indexer/internal/walk/walk.go index 6a179b0..372a664 100644 --- a/services/indexer/internal/walk/walk.go +++ b/services/indexer/internal/walk/walk.go @@ -30,7 +30,7 @@ var indexedExts = map[string]struct{}{ ".cs": {}, ".csx": {}, ".rb": {}, ".rake": {}, ".gemspec": {}, ".php": {}, ".swift": {}, ".kt": {}, ".erb": {}, - ".md": {}, ".json": {}, + ".md": {}, ".json": {}, } // Mirrors INDEXED_FILENAMES: extensionless files worth indexing. diff --git a/services/indexer/internal/walk/walk_test.go b/services/indexer/internal/walk/walk_test.go new file mode 100644 index 0000000..d0ec1d5 --- /dev/null +++ b/services/indexer/internal/walk/walk_test.go @@ -0,0 +1,134 @@ +package walk + +import ( + "fmt" + "os" + "path/filepath" + "testing" +) + +func seed(t *testing.T, dir string, files map[string]string) { + t.Helper() + for name, body := range files { + p := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } +} + +func TestSkipsExcludedDirectories(t *testing.T) { + dir := t.TempDir() + seed(t, dir, map[string]string{ + "src/app.ts": "x", + "node_modules/pkg/index.js": "x", + ".git/config": "x", + "dist/bundle.js": "x", + ".venv/lib/mod.py": "x", + }) + + got, err := Find(dir, Options{}) + if err != nil { + t.Fatal(err) + } + if len(got.Paths) != 1 || got.Paths[0] != "src/app.ts" { + t.Errorf("paths = %v, want only src/app.ts", got.Paths) + } +} + +// The Python original's cap did not work: its `break` left only the inner filename loop, +// so os.walk continued into the next directory and kept appending. This is the regression +// test for that class of bug. +func TestMaxFilesActuallyStopsTheWalk(t *testing.T) { + dir := t.TempDir() + files := map[string]string{} + for d := 0; d < 5; d++ { + for f := 0; f < 10; f++ { + files[fmt.Sprintf("d%d/f%d.py", d, f)] = "x" + } + } + seed(t, dir, files) + + got, err := Find(dir, Options{MaxFiles: 12}) + if err != nil { + t.Fatal(err) + } + if len(got.Paths) > 12 { + t.Errorf("found %d files with MaxFiles=12: the cap leaked", len(got.Paths)) + } + if !got.Truncated { + t.Error("Truncated must be set, or a partial index looks like full coverage") + } +} + +func TestTruncatedIsFalseWhenEverythingFits(t *testing.T) { + dir := t.TempDir() + seed(t, dir, map[string]string{"a.py": "x", "b.py": "x"}) + got, err := Find(dir, Options{MaxFiles: 100}) + if err != nil { + t.Fatal(err) + } + if got.Truncated { + t.Error("Truncated set despite the walk completing") + } +} + +func TestSkipsOversizeFiles(t *testing.T) { + dir := t.TempDir() + big := make([]byte, 3*1024) + for i := range big { + big[i] = 'x' + } + seed(t, dir, map[string]string{"small.py": "x"}) + if err := os.WriteFile(filepath.Join(dir, "big.py"), big, 0o644); err != nil { + t.Fatal(err) + } + + got, err := Find(dir, Options{MaxFileSizeKB: 2}) + if err != nil { + t.Fatal(err) + } + if len(got.Paths) != 1 || got.Paths[0] != "small.py" { + t.Errorf("paths = %v, want only small.py", got.Paths) + } + if got.Skipped != 1 { + t.Errorf("Skipped = %d, want 1", got.Skipped) + } +} + +func TestIndexesExtensionlessKnownFilenames(t *testing.T) { + dir := t.TempDir() + seed(t, dir, map[string]string{ + "Gemfile": "x", "Rakefile": "x", "config.ru": "x", "LICENSE": "x", + }) + got, err := Find(dir, Options{}) + if err != nil { + t.Fatal(err) + } + if len(got.Paths) != 3 { + t.Errorf("paths = %v, want the three known Ruby filenames and not LICENSE", got.Paths) + } +} + +func TestPathsAreRelativeAndSlashSeparated(t *testing.T) { + dir := t.TempDir() + seed(t, dir, map[string]string{"a/b/c.py": "x"}) + got, err := Find(dir, Options{}) + if err != nil { + t.Fatal(err) + } + // Must match CodeFile.path on the Python side, which is repo-relative with forward + // slashes regardless of platform. + if len(got.Paths) != 1 || got.Paths[0] != "a/b/c.py" { + t.Errorf("paths = %v, want [a/b/c.py]", got.Paths) + } +} + +func TestMissingRootIsNotAPanic(t *testing.T) { + if _, err := Find(filepath.Join(t.TempDir(), "nope"), Options{}); err != nil { + t.Errorf("a missing root should return empty, not error: %v", err) + } +}