fix: extract bodiless trait methods and stop binary content reaching chunks

[stephane-segning]

1. Summary

This PR changes:

• Trait methods without a default body are never extracted (no chunk, no graph node) #1 — a Rust trait method declared without a default body (fn greet(&self);) is now extracted: it gets a graph node, a method edge from its trait, and a chunk. It parses as function_signature_item, not function_item, and was classified as nothing at all.
• Trait methods without a default body are never extracted (no chunk, no graph node) #1 (design) — such declarations are deliberately not call targets. See §2.
• Binary/NUL guard is bypassed on the graph-enabled walk path #2 — the binary/NUL guard moved to where a chunk is produced (chunk_text) and into the walk before either consumer, so it can no longer be bypassed by entering through a different door.
• Regenerated tests/golden/sample-repo.graph.json (additions only, hand-verified — see §4).

It solves:

• Trait methods without a default body are never extracted (no chunk, no graph node) #1 — trait methods without a default body are never extracted
• Binary/NUL guard is bypassed on the graph-enabled walk path #2 — binary/NUL guard is bypassed on the graph path

---

2. Intent

The intent of this PR is:

To close the two extraction gaps found when this crate was pulled out of its monorepo, without trading either of them for a quieter regression.

On #1, the obvious fix is wrong. Adding function_signature_item to the classifier makes a trait declaration a definition — correct — but it also makes it compete for its own name in the resolver. The resolver is precision-favouring: several same-named candidates with no disambiguating qualifier are dropped, not fanned out. So a trait with exactly one impl would go from one candidate to two the moment declarations were indexed, and every call to that method would stop resolving. The missing-symbol fix would have silently deleted calls edges — a strictly worse bug, because nothing would fail and no test asserted those edges.

The rule this PR adopts: a declaration is a definition (node, method edge, chunk) but not a call target. A call dispatches to an implementation, never to a declaration. Classifier::is_call_target is the seam, and a_call_to_a_single_impl_trait_method_still_resolves_to_the_impl is the test that proves the regression does not happen.

On #2, the guard existed and was correct — it was just at one of two call sites. chunk_file held it; the graph-enabled walk never calls chunk_file. Putting it where the chunk is produced makes the bypass structurally impossible rather than fixing this one instance of it. It also runs in the walk before extraction, so the graph stops ingesting binary content too — guarding only the chunker would have left tree-sitter parsing garbage into :Symbol nodes.

---

3. Scope

In Scope

• function_signature_item classification, and the call-target seam that keeps it out of resolution
• A shared is_binary guard applied at chunk-production and in the walk
• Regression tests for both, including the end-to-end resolver proof
• The one intended golden change

Out of Scope

• Other languages' equivalent shapes. Java interface methods and TypeScript method_signature/abstract members go through the tags.scm path, not interesting_node, and are left exactly as they are. All 6 other golden files are unchanged by this PR, which is the evidence they were not touched. Whether a tags-driven language has the same declaration-vs-implementation ambiguity was left open here — Classifier::is_call_target leaves Tagged as-is rather than guessing per-grammar. Adversarial review has since answered it: Java does have the identical bug, today. tree-sitter-java's tags.scm captures a bodiless method_declaration exactly like a concrete one, so a single-impl interface call is dropped as ambiguous. Confirmed pre-existing (byte-identical against main), so it is not a regression from this PR — tracked as Java interface methods: single-impl calls are dropped as ambiguous (the tags-path twin of #1) #5.
• TypeScript interface_declaration chunking, which interesting_node also omits. Noticed while here; genuinely a different bug, not filed as part of these two.
• Widening the NUL sniff beyond its 512-byte prefix window. That bound is pre-existing and now has a test documenting it as a decision rather than an accident.

---

4. Verification

I verified this change by:

• Running automated tests
• Running manual tests
• Checking logs
• Checking metrics
• Testing error cases
• Testing permissions/security behavior
• Testing rollback or failure behavior, if relevant

Commands run:

cargo test --no-fail-fast
cargo fmt --all -- --check
cargo clippy --all-targets --all-features --locked -- -D warnings
cargo llvm-cov --locked --all-targets --summary-only
cargo test --features container-tests --no-fail-fast

Results:

$ cargo test --no-fail-fast
    11 test binaries, all ok, 0 failed
    lib: 136 passed (was 130)

$ cargo fmt --all -- --check
    (clean)

$ cargo clippy --all-targets --all-features --locked -- -D warnings
    (no warnings)

$ cargo llvm-cov --locked --all-targets --summary-only
    TOTAL  95.40% lines   96.43% functions   96.00% regions
    (was 93.76% lines; CI floor is 85)

The golden diff, hand-verified. sample-repo is the only fixture containing a Rust trait, and it always contained trait Shape { fn describe(&self) -> f64; } at line 31 with an impl at line 35. The regenerated golden is additions only:

+ node  src/shapes.rs#31:describe   label "describe()"   line 31
+ edge  src/shapes.rs#30:Shape --method--> src/shapes.rs#31:describe

Nothing was removed. The pre-existing impl --method--> src/shapes.rs#35:describe is untouched, and no calls edge changed — which is exactly the property the design in §2 exists to protect. The other 6 golden files (java-repo, javascript-repo, polyglot-repo, python-repo, tsx-repo, typescript-repo) are byte-identical — they are asserted by the 13 test functions in tests/language_goldens.rs, which is a different number and was originally miscited here as “13 goldens”.

Tests added:

signature_only_trait_method_is_a_node_with_a_method_edge
signature_only_trait_method_is_not_a_call_target
a_bodied_method_next_to_its_declaration_is_still_the_only_call_target
a_call_to_a_single_impl_trait_method_still_resolves_to_the_impl
signature_only_trait_method_is_chunked_like_a_bodied_one
is_binary_detects_a_nul_only_within_the_sniff_window
build_graph_path_rejects_a_nul_laden_blob_exactly_as_chunk_file_does   (inverted from the bug-documenting test)
binary_blob_is_kept_out_of_the_graph_too_not_just_the_chunks

---

5. Screenshots / Evidence

Add evidence here:

• Issue Trait methods without a default body are never extracted (no chunk, no graph node) #1: Trait methods without a default body are never extracted (no chunk, no graph node) #1
• Issue Binary/NUL guard is bypassed on the graph-enabled walk path #2: Binary/NUL guard is bypassed on the graph-enabled walk path #2
• The two tests that previously asserted the buggy behaviour are inverted in this PR, not deleted — tests/robustness.rs's NUL test and src/graph/emit.rs's trait-method test both used to document the gap on purpose.

Adversarial review outcome. An independent reviewer drove the public API against hand-built fixtures on this branch and on a clean clone of main, to separate "introduced here" from "pre-existing". It could not break the is_call_target design for Rust (generic bounds, cross-file calls, two unrelated traits with a colliding method name, default-body-plus-override all resolve identically pre/post), and independently re-derived the golden diff as complete and correct. It produced two acted-on findings:

1. Java has the same ambiguity today — reproduced, confirmed pre-existing, filed as Java interface methods: single-impl calls are dropped as ambiguous (the tags-path twin of #1) #5.
2. WalkStats had no counter for the new binary-skip path, so a rejected file vanished from the summary line entirely — a fair hit, since before this PR that file was counted into files_chunked (with contaminated content). Fixed in this PR: files_skipped_binary, logged and asserted.

It also caught a factual imprecision in this description ("13 language goldens" — 13 is the test-function count; there are 6 other golden files), corrected above.

---

6. Risk Assessment

Risk level:

• Low
• Medium
• High

Potential risks:

• More chunks than before. Every bodiless trait method now produces a chunk, so a trait-heavy repo gains chunks and therefore embedding cost at index time. This is the point of the fix, but it is a real cost change, not free.
• More graph nodes than before, so graph_find_symbol will return declarations alongside implementations for the same name. Consumers that assumed one node per method name will now see two.
• Binary files are now skipped entirely on the graph path. If any repo was relying on garbage chunks existing, they disappear. This is intended.
• The declaration-is-not-a-call-target rule is a judgement call. It is the right default, but it means a call to a trait method whose impl lives outside the indexed repo now resolves to nothing rather than to the declaration.

Mitigation:

• The golden proves the change is additive on a real fixture, and the 6 other golden files prove no other language moved.
• The resolver behaviour is pinned by an explicit end-to-end test, so a future change that makes declarations call targets will fail loudly instead of quietly dropping edges.
• Both previously-buggy behaviours were covered by tests asserting the bug; those tests are inverted, so a regression in either direction is caught.
• The last risk is stated rather than mitigated — it is a deliberate trade, and the reviewer question in §8 asks about it directly.

---

7. AI Usage Declaration

AI was used for:

• Understanding existing code
• Generating code
• Refactoring
• Generating tests
• Drafting documentation
• Reviewing the diff
• Not used

Human verification:

• I understand every meaningful change in this PR
• I checked generated code manually
• I checked generated tests manually
• I removed unsupported AI assumptions
• I accept responsibility for this PR

Both bugs were independently reproduced with a minimal driver before any code was changed, and the golden diff was read line by line against the fixture source rather than regenerated and trusted.

---

8. Reviewer Focus

Please focus your review on:

• Correctness
• Architecture
• Security
• Performance
• Tests
• Maintainability
• Product intent
• Edge cases

Specifically:

• Is "a declaration is a definition but not a call target" the right rule? The alternative — declarations as call targets — is more intuitive and strictly worse in practice for single-impl traits. A third option would be to prefer bodied definitions inside pick(), which is more precise but pushes language-specific knowledge into the language-agnostic resolver.
• Should a call to a trait method with no in-repo impl fall back to the declaration? Today it resolves to nothing. That is a real loss for repos that implement external traits, and it would be a small change to pick().
• The is_binary sniff is a 512-byte prefix. A file with clean text followed by a NUL at byte 600 still gets chunked. Pre-existing, now tested and documented — but worth deciding whether the bound is right rather than inherited.

[changeset-bot]

⚠️ No Changeset found

Latest commit: daeece8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR