Repository navigation
perf(indexer): hash definitions in Rust on py-tree-sitter's own trees - #3475
Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Rust tree-walk algorithms rust/cgr_graph_rs/Cargo.toml, rust/cgr_graph_rs/src/trees/*, rust/cgr_graph_rs/THIRD_PARTY_LICENSES.md, rust/cgr_graph_rs/README.md |
Adds configurable Rust anchor hashing and AST fingerprinting over tree-sitter nodes. Rust tests cover the algorithms. Cargo dependencies, license entries, and extension documentation are updated. |
Python tree bridge rust/cgr_graph_rs/src/lib.rs, rust/cgr_graph_rs/src/trees/python.rs |
Registers the Python-facing bridge. Construction checks the py-tree-sitter version and object layouts. Each call checks node and tree types, pointers, and tree identity. |
Python dispatch and compatibility probe codebase_rag/constants/embedded_graph.py, codebase_rag/exceptions.py, codebase_rag/logs.py, codebase_rag/parsers/native_trees.py, codebase_rag/parsers/anchor_hash.py, codebase_rag/parsers/ast_fingerprint.py, codebase_rag/types_defs.py |
Adds bridge rules, initialization, and probe comparisons for tree descriptions, anchor hashes, and fingerprints. Public hash and fingerprint functions use the bridge when available and fall back to Python implementations when unavailable or when NotBorrowed is raised. |
Parity and release integration codebase_rag/tests/test_native_trees.py, .github/workflows/ci.yml, docs/architecture/embedded-graph.md, docs/reports/INDEXER_RUST_PLAN.md, pyproject.toml, rust/cgr_graph_rs/pyproject.toml, codebase_rag/tests/test_rust_extension_packaging.py |
Adds parity, fallback, and compatibility tests and includes the new test file in CI. Updates documentation, the extension version, dependency minimum, and published artifact expectations. |
Repository guidance
| Layer / File(s) | Summary |
|---|---|
Contributor and agent instructions .dockerignore, .github/copilot-instructions.md, .gitignore, AGENTS.md, CLAUDE.md, GEMINI.md |
Adds shared agent guidance and tree-sitter pin update procedures. Changes ignore rules for agent instruction files. |
Priority:
Estimated code review effort: 4 (Complex) | ~60 minutes
Change: Refactor
Sequence Diagram(s)
sequenceDiagram
participant PythonCaller
participant NativeTrees
participant TreeBridge
participant RustTrees
participant PythonTwin
PythonCaller->>NativeTrees: request anchor hash or AST fingerprint
NativeTrees->>TreeBridge: construct bridge with Python rules
TreeBridge->>RustTrees: compare tree layout, anchor hash, and fingerprint
NativeTrees-->>PythonCaller: report bridge unavailable after probe failure or mismatch
PythonCaller->>TreeBridge: request hash or fingerprint
TreeBridge->>RustTrees: walk borrowed tree node
TreeBridge-->>PythonCaller: raise NotBorrowed when node cannot be read
PythonCaller->>PythonTwin: compute result when bridge is unavailable or node is unreadable
Possibly related issues
- [Performance]: Python scope analysis is about 22% of indexing: cache it per scope in Python, then port it to Rust #3473: This PR ports the tree walks identified as foundations for later Rust scope analysis. Scope-analysis work remains outside this change.
Possibly related PRs
- vitali87/code-graph-rag#3329: This PR changes the implementation that computes definition
anchor_hashvalues used to repair gloss mentions after renames or moves.
Suggested reviewers: claude
Merge Risk: 
5dde5
-
No actionable issue is established from the available evidence. The change appears mergeable after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: moving definition hashing to Rust while using py-tree-sitter trees. It also follows Conventional Commits format. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description follows the repository template and covers every required section. It gives a clear summary of the two fingerprints moved to Rust, the py-tree-sitter bridge, and the pin pairing. It in… Full details: Description check
-
Explanation
The description follows the repository template and covers every required section. It gives a clear summary of the two fingerprints moved to Rust, the py-tree-sitter bridge, and the pin pairing. It includes a demo GIF with hashes, marks the Type of Change, lists related issues, and gives a detailed test plan. The checklist is complete, and the maintainer notes cover the crate version bump, toolchain change, and .gitignore impact. One gap: the Integration tests item is unchecked because Docker was unavailable, and the description says so. The Test Plan does not run the integration suite, which is a stated limitation and not a missing section.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
 [Medium impact] The PR appears safe to merge; no blocking defects were found. SummaryMoves definition hashes and clone fingerprints to Rust while keeping the Python implementations as fallbacks.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Definition node] --> B{Bridge passes setup and probe?}
B -->|No| P[Python hash walk]
B -->|Yes| C{Bridge can read this node?}
C -->|No| P
C -->|Yes| R[Rust walk of existing tree]
P --> O[Hash or fingerprint]
R --> O
Reviews (1) · Last reviewed commit: "refactor: keep the tree bridge's names a..." · Reviewed by Greptile |
bc366e6 to
dcb5346
Compare
vitali87
changed the title
dcb5346 to
073d57c
Compare
… the bridge reads
…r crate needs 1.90
|
Summary
anchor_hash), and one to find copy-pasted code (compute_ast_fingerprint). Both now run in Rust insidecgr-graph-rs, with identical results, about 6x faster per call. The Python versions stay as the reference and the fallback.docs/reports/INDEXER_RUST_PLAN.mdis rewritten from fresh profiles and orders the next steps by measured cost: [Bug]: Python indexing is quadratic in classes per module: _find_class_node re-runs the classes query over the whole module once per class (4,000 classes: 80-97 s vs 15 s memoised; semgrep's generated semgrep_output_v1.py 14-17 s vs 6 s) #3146, then [Performance]: Python scope analysis is about 22% of indexing: cache it per scope in Python, then port it to Rust #3473, then [Performance]: Call resolution (about 8% of indexing) stays in Python; fix its own hot spots, starting with a redundant full-registry scan in Java type lookup #3474.tree-sittercrate must move as a checked pair;test_native_trees.pyfails when either pin moves alone, and a newAGENTS.mdhas the bump procedure. Codex, Cursor, Windsurf and Copilot's coding agent read it directly, and Claude Code and Copilot chat and review throughCLAUDE.mdand.github/copilot-instructions.md, which import it or point to it.Demo
The GIF, recorded with VHS, indexes rich (213 Python files, no embeddings) from scratch on a
mainworktree, then on this branch. After each index it prints a hash of everyanchor_hashandast_fingerprintthe graph holds, and a hash of thecgr duplicatesreport built on them:e15f33bc…; duplicates report sha2568f94bf66….Timed without recording, in the same two worktrees (identical packages), over 5 alternating runs each, the branch was faster in every pair: sync median 12.52 s → 12.01 s (−4%), wall 16.32 s → 15.48 s. On rich the two hashes are about 5.7% of a run, so about 5% is all that porting them can save there. On code-graph-rag's own tree, where they are 9.4%, the run went from 222.5 s to 206.0 s.
The GIF is stored on the
claude/cgr-rust-porting-mrwohh-evidencebranch and is not part of this PR's diff.How it works
A py-tree-sitter
Nodeobject holds the C tree node inside it. The Rusttree-sittercrate 0.26.8 contains the same C code for walking a tree as the py-tree-sitter 0.26.0 that cgr pins:node.c,tree.c,tree_cursor.c,subtree.handlanguage.care byte-identical. SoTreeBridge(rust/cgr_graph_rs/src/trees/) copies the node out of the Python object and walks the real tree in place.Guards, because Rust reads memory py-tree-sitter owns:
tree_sitter.Node, that its tree is exactly atree_sitter.Tree, and that both point at the same underlying tree.codebase_rag/parsers/native_trees.pyparses a sample source and compares every node, and both fingerprints of every node, as Rust and Python read them. Any difference turns the bridge off for the process.NotBorrowed, and that call runs the Python version.The classification rules (which node types count as punctuation, identifiers and so on) are passed from
constants/into Rust as aTreeRulesTypedDict when the bridge is built, so the two versions can't drift apart on them.Results
anchor_hash, per definitioncompute_ast_fingerprint, per definitionBoth savings match what the profiles predicted for these two walks: 7.4% measured against 7.8% on code-graph-rag, and 4% against about 5% on rich. They were only about 9% of the run, so the larger wins are in the follow-up issues.
Correctness:
resolutionlabel of a fewCALLSedges, and two runs with the bridge off already differ in that label.Type of Change
Related Issues
Related to #3146, #3473, #3474 (the next steps the revised plan orders) and #3472 (a torch-free embedder, measured separately).
Test Plan
uv run pytest -n auto -m "not integration", with the extras CI installs): 23,842 passed, 4 failed. The 4 need toolchains this environment lacks: the .NET SDK (two C# oracle guards) and the Gogo/typeshelper, which could not download its modules (two Go oracle tests). All four fail the same way with the bridge switched off. After the crate version bump, the packaging tests (39) and the licence check pass.codebase_rag/tests/test_native_trees.pycompares Rust and Python on every named node of a sample in each language and of cgr's own parser sources. It also pins every fallback, and fails if the extension carries the bridge but the bridge is off.TestVersionPairin the same file holdspyproject.toml'stree-sitter==pin, andCargo.toml's andCargo.lock'stree-sittercrate, to the pair the bridge was checked with (TreeBridge.PY_TREE_SITTER,TreeBridge.TREE_SITTER_CRATE). Bumping either pin alone, as a Dependabot pull request would, fails that pin's test with a message naming the checked version and pointing toAGENTS.md. I checked this by moving both pins and watching both tests fail, then pass again once restored.src/trees/tests.rspin the Python versions' exact values.make test-integration, requires Docker). Not run: no Docker here.cargo test --release(56 passed);cargo clippy --features python: nothing new; the two existing warnings inlib.rsremain.Checklist
!in the title and its release notes inchanges/breaking/<PR number>.md: not a breaking change.make pre-commit)# type: ignore,cast(),Any, orobjecttype hintsFor the maintainer
cgr-graph-rsgoes from 0.1.0 to 0.1.1, as the "Crate version bumped with its code" check requires for any change to the library. The bridge only adds API, so the requirement stays below 0.2 and its floor rises to 0.1.1.uv.lockwas regenerated with uv 0.13.5, the version CI resolves; only the crate's version line changed. An older 0.1.0 wheel has noTreeBridge, and the Python versions run there..clusterfuzzlite/Dockerfilegoes from rustc 1.85.0 to 1.97.0, the toolchain this branch was built and tested with. The fuzz build compiles the crate from this checkout, and its new dependencies need at least 1.90 (tree-sitter-language0.1.9, which thetree-sittercrate pulls in;sha20.11 needs 1.88). I checked that the crate builds on 1.90.0 and 1.97.0 and fails on 1.85.0 exactly as the fuzz job did.tree-sittercrate, and the two bumped together, asAGENTS.md("Bumping tree-sitter") describes step by step. A lone bump failsTestVersionPair, and the bridge switches itself off, so the slowdown can't go unnoticed..gitignore:AGENTS.mdandCLAUDE.mdare no longer ignored, so every agent gets the same instructions from the repository;PROJECT.mdstays ignored. In a checkout that already has its own untrackedCLAUDE.md, pulling this stops with "untracked working tree files would be overwritten"; move those notes to the user-level~/.claude/CLAUDE.mdfirst..dockerignorestill keeps all of these out of the image.Summary by CodeRabbit