fix(aver-extractor): parse-once, canonical identities, per-language correctness - #8
fix(aver-extractor): parse-once, canonical identities, per-language correctness#85queezer wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 review |
✅ Action performedReview finished.
|
3279087 to
57bbe11
Compare
|
Review and validation update: CodeRabbit completed a fresh review with no actionable inline findings. Rebased onto current Local validation passed: format check; — Hermes Agent |
…orrectness - Parse each file once per extract_*_facts call and thread the root node into node-based collectors (was: up to ~23 re-parses per Rust file, 4-10 for other languages). Public string functions stay as thin parse-once wrappers. - rust.rs: emit ONE canonical, qualified identity per entity and call edge via a single scope-aware walk - module-qualified for mod items (Function:m::f), type-qualified for impl methods (Function:Type::method); drop the unqualified duplicate layer that merged same-named entities in the claim graph. - rust.rs: read impl headers from tree-sitter trait/type fields with generic base-name stripping instead of text splitting, fixing generics, where-clauses and unsafe impls. - c.rs/cpp.rs: only emit defines for struct/enum/class specifiers that have a body (usages like 'struct Foo *p;' and forward declarations no longer produce definitions). - csharp/swift: exclude file-locally declared interfaces/protocols from the extends branch (interface-only base lists no longer emit false 'extends Class:*'); Kotlin extends applies the same rule. - kotlin: classify class_declaration by its own keyword tokens and direct type_identifier child instead of text scans (data/sealed/ annotated classes found; comments mentioning 'class' cannot misclassify interfaces). - plugin runner: close stdin after the request write, add a wait timeout (default 30s, with_timeout to override), surface JSON-RPC error responses as Error::PluginError, and validate the response id. - minors: propagate utf8 errors in go/php/ruby; detect #[tokio::test]/#[rstest]/#[test_case] via attribute path segments and keep the marker across attributes/comments; scope module test mappings to the module's own items; expand grouped 'pub use' in module imports; C++ extends objects use the declared base kind; anchor heritage/base clauses on the type's own clause in js/cpp/php; keep Swift actors as Actor:* in implements/extends; share validate_facts between lib.rs and prose.
57bbe11 to
e4774cf
Compare
1 participant
Summary
Fixes all 7 major findings and 9 minors from the aver-extractor code review. Every
extract_<lang>_factsnow parses its file exactly once, Rust entities get a single canonical qualified identity, and each per-language correctness bug has a regression test (including negative assertions where the bug was a false positive).Major findings
1. Same source re-parsed many times per file — Every
extract_*_factsnow callsparse_with_languageonce and threadstree.root_node()into node-based collectors (rust.rs went from ~23 parses to 1; java 10, csharp 10, php 9, swift 8, kotlin/cpp 7, go/ts 6, ruby 5, c 4 — all now 1). The public string functions (extract_rust_functions,extract_csharp_classes, ...) remain as thin parse-once wrappers, so the public API and existing tests are unchanged.2. Rust entity identity collisions —
extract_rust_factsis now a single scope-aware walk emitting ONE canonical identity per entity and call edge:path defines Function:foo(unchanged)Module:m defines Function:m::foo(no more parallel unqualifiedFunction:foo)Type:T defines Function:T::m(methods no longer also appear as free functions or asFunction:m::methodmodule functions)fn f()now yieldFunction:a::f/Function:b::finstead of merging into oneFunction:f3. Text-split impl headers —
impl_itemheaders are read from the grammar'strait/typefields with generic base-name stripping (Trait<T>→Trait).impl<T> Trait<T> for Type<T> where T: Cloneandunsafe impl Send for Foonow produce correctimplementsfacts instead of garbage/none.4. C struct/enum usages emitted as defines —
struct_specifier/enum_specifier/class_specifieronly count as definitions when they have abodyfield (C and C++).struct Foo *p;andclass Foo;no longer emitdefines. Forward declarations still inform the C++ base-kind lookup.5. C#/Swift interface-only base lists emitted false
extends Class:*— the extends branch now skips names in the file-locally declared interface/protocol set and picks the first remaining base (Kotlinextendsapplies the same rule). New negative assertions cover the false positives.6. Kotlin text-scan classification —
class_declarationis classified by its own keyword tokens (class/interface/enum) and its directtype_identifierchild, not by scanning node text.data/sealed/annotated classes are found; an interface whose body mentions class in a comment is no longer misclassified; delegation specifiers are anchored to direct children.7. Plugin runner — stdin is explicitly closed after the request write (plugins reading to EOF can't deadlock), requests now have a wait timeout (default 30s,
JsonRpcPluginRunner::with_timeoutto override; timed-out children are killed), JSON-RPCerrorresponses surface asError::PluginError { code, message }instead of an opaque serde error, and the responseidis validated against the request (Error::PluginIdMismatch).Minors
utf8_text(...).unwrap_or_default()now propagates with?.collect_tests: detects#[tokio::test]/#[rstest]/#[test_case]via the attribute path's last segment; extra attributes or comments between attribute and fn no longer drop the marker;#[cfg(test)]is not a test.mod m { pub use a::{b, c}; }→Module:m imports Module:a::b+Module:a::c.struct Foo : Bar→extends Class:BarwhenBaris a class; falls back to the subject kind for undeclared bases).Actor:Namein implements/extends subjects (was folded intoClass:).validate_factsis shared betweensrc/lib.rsandprose/mod.rs(was duplicated verbatim).Test evidence
cargo test -p aver-extractor: 180 passed, 0 failed (was 148; +32 regression tests: 12 rust, 15 multilang, 5 plugin — deterministic, offline, no#[ignore])cargo clippy -p aver-extractor --all-targets -- -D warnings: cleancargo fmt: appliedcargo test -p aver-core --test extractor_ingestion --test extractor: 8 passed (aver-core's parallelExtractorFact/ingestion path unaffected — no workspace crate depends on aver-extractor yet)Sample of new negative-assertion regressions: no unqualified
Function:ffor mod-nested fns, no method-as-free-function, nodefines Struct:Fooforstruct Foo *p;, noextends Class:IRecallablefor C# interface-only bases, no outer-class capture of nested JS/C++/PHP clauses, plugin timeout/id-mismatch/error-response handling.