feat(parser): add Rust support#18
Conversation
Architecture Drift ReportAlgorithm: PKG | Entities: 719 | Components: 7 Drift from Baseline
Changes
Smells (1)
Generated by arcade-agent |
🤖 Architecture Analysis SummaryPowered by arcade-agent — automatic architectural self-analysis 📈 Metric EvolutionBaseline commit: Legend: 🟢 better · 🔴 worse · 🟡 low impact · ⚪ no change
🏛️ Current Architecture
🧭 Principle Signals
🎯 Score DriversBiggest risks
Strongest areas
🕸️ High-Level Designgraph TD
Algorithms["Algorithms\n43 entities\n5 classes / 3 methods"]
Budget["Budget\n3 entities\n0 classes / 0 methods"]
Cache["Cache\n4 entities\n0 classes / 0 methods"]
Ci["Ci\n5 entities\n0 classes / 0 methods"]
Exporters["Exporters\n15 entities\n1 classes / 0 methods"]
Incremental["Incremental\n2 entities\n1 classes / 2 methods"]
Parsers["Parsers\n22 entities\n16 classes / 32 methods"]
Serialization["Serialization\n8 entities\n0 classes / 0 methods"]
Tools["Tools\n18 entities\n4 classes / 2 methods"]
Cache --> Serialization
Ci --> Algorithms
Ci --> Exporters
Ci --> Serialization
Ci --> Tools
Serialization --> Algorithms
Serialization --> Parsers
Tools --> Cache
Tools --> Parsers
🏗️ Components breakdown
🚨 Architectural Smells✅ No architectural smells detected. 📈 Evolution vs BaselineBaseline commit: Architecture-to-Architecture (A2A) Comparison
Component matching detailsMatched:
High-level component statistics
Before/After Mermaid diagramsBaseline graph TD
Algorithms["Algorithms\n39 entities\n5 classes / 3 methods"]
Budget["Budget\n3 entities\n0 classes / 0 methods"]
Cache["Cache\n4 entities\n0 classes / 0 methods"]
Ci["Ci\n5 entities\n0 classes / 0 methods"]
Exporters["Exporters\n14 entities\n1 classes / 0 methods"]
Incremental["Incremental\n2 entities\n1 classes / 2 methods"]
Parsers["Parsers\n17 entities\n11 classes / 27 methods"]
Serialization["Serialization\n8 entities\n0 classes / 0 methods"]
Tools["Tools\n23 entities\n4 classes / 2 methods"]
Cache --> Serialization
Ci --> Algorithms
Ci --> Exporters
Ci --> Serialization
Ci --> Tools
Serialization --> Algorithms
Serialization --> Parsers
Tools --> Algorithms
Tools --> Cache
Tools --> Exporters
Tools --> Parsers
Current graph TD
Algorithms["Algorithms\n43 entities\n5 classes / 3 methods"]
Budget["Budget\n3 entities\n0 classes / 0 methods"]
Cache["Cache\n4 entities\n0 classes / 0 methods"]
Ci["Ci\n5 entities\n0 classes / 0 methods"]
Exporters["Exporters\n15 entities\n1 classes / 0 methods"]
Incremental["Incremental\n2 entities\n1 classes / 2 methods"]
Parsers["Parsers\n22 entities\n16 classes / 32 methods"]
Serialization["Serialization\n8 entities\n0 classes / 0 methods"]
Tools["Tools\n18 entities\n4 classes / 2 methods"]
Cache --> Serialization
Ci --> Algorithms
Ci --> Exporters
Ci --> Serialization
Ci --> Tools
Serialization --> Algorithms
Serialization --> Parsers
Tools --> Cache
Tools --> Parsers
Component dependency delta
Smell changes:
💡 CI/CD Insights
📄 View HTML reports and artifacts This comment is auto-generated by the self-dogfooding CI job. It updates on every push to this PR. |
75ae540 to
9281b4e
Compare
There was a problem hiding this comment.
Pull request overview
Adds first-class Rust parsing support to arcade-agent, integrating a new tree-sitter-based Rust parser into ingest/parse flows, caching, and project documentation so Rust projects (including Cargo workspaces) can be analyzed like existing languages.
Changes:
- Introduces
RustParserwith module-convention handling, entity extraction (types/traits/functions/methods), and a second-pass linker for imports/references/trait relationships. - Extends language detection/ingest, parse caching, and CI/MCP/action help text to include
rust. - Updates docs/roadmap and adds a Rust-focused regression test suite (including workspace + cache invalidation checks).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_parsers/test_rust.py | Adds Rust parser regression coverage across module conventions, impl ownership, imports, and Cargo workspace behaviors. |
| tests/test_cache.py | Ensures cache key changes when Rust source files change. |
| src/arcade_agent/tools/ingest.py | Adds Rust file extensions and Cargo workspace-aware source-root detection. |
| src/arcade_agent/tools/adapters/mcp.py | Updates MCP adapter docstrings/help to list Rust as a supported language override. |
| src/arcade_agent/parsers/rust.py | New Rust tree-sitter parser + linker implementation with Cargo workspace/module modeling. |
| src/arcade_agent/parsers/init.py | Registers Rust parser behind an optional dependency import guard. |
| src/arcade_agent/ci/run_self_analysis.py | Updates CLI help to include Rust language override. |
| src/arcade_agent/cache.py | Includes .rs in cache-key file hashing. |
| ROADMAP.md | Marks the Rust parser roadmap item as shipped and describes scope. |
| README.md | Documents Rust support and updates installation extras to include [languages]. |
| pyproject.toml | Adds tree-sitter-rust to the optional languages extra. |
| actions/analyze/action.yml | Extends action input docs to include Rust as a language override. |
| .github/workflows/architecture-analysis-reusable.yml | Extends reusable workflow input docs to include Rust as a language override. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lemduc
left a comment
lemduc
left a comment
There was a problem hiding this comment.
Strong parser overall — and I want to lead with what you got right: defect classes 2–4 from the Kotlin review (function-local hoisting, cross-package leaf-fallback edges, lost inheritance/shared props) are all clean here, with regression tests. resolve() returning None for unresolved multi-segment paths, the blanket-generic-impl skip, and the cfg-alternative dedupe are exactly the right instincts. Registration surfaces are complete, and the ripgrep validation numbers in the PR body held up under independent re-run.
One blocking finding, and it's an ironic one:
1. [Blocking] The recursion defect class from the Kotlin review is back — in four places. _references is iterative (so the original deep-parens killer passes — proof you know the pattern), but these four still recurse unboundedly, and extraction runs outside the per-file try (rust.py:475-481 wraps only stat/read/parse, catching only (OSError, ValueError)):
| Crash input (one ~5KB poisoned file) | Site | Depth |
|---|---|---|
a::a::…::T path, 991 segments |
_path_segments (~L131) |
991 |
use a::{a::{…}}, 991 levels |
_flatten_use (~L160) |
991 |
mod m { mod m { … }}, 993 levels |
visit_container (~L456, mod_item) |
993 |
impl T for &&&…&R / ((((R)))), 993 wrappers |
_base_type_path (~L224) |
993 |
Each reproduces RecursionError out of RustParser().parse(), killing the entire analysis and losing the good sibling files' entities. Fix pattern is already on main from the Kotlin follow-up (b7effc5): explicit stacks. Please also widen the per-file except to Exception like kotlin.py:374 as a backstop.
2. [Decide before merge] #[cfg(test)] mod tests contents become production entities. Rust unit tests live inline in production files, and exclude_tests=True only filters test paths — on a fresh ripgrep clone, 548 of 3,184 entities (17%) and 37 phantom packages (e.g. grep_cli.escape.tests.backslash) are test items, which will surface as bogus components in recovery and skew metrics. Suggested: when exclude_tests, skip mod_items whose preceding sibling attribute is #[cfg(test)] — or at minimum document the behavior. Maintainer call since it changes graph shape.
Non-blocking:
_MAX_FILE_BYTES = 1_000_000silently drops >1MB files from the graph — new behavior no other parser has, undocumented (and it doesn't mitigate the recursion crashes; the 991-segment file is ~5KB).- README labels Rust "(full)" while Kotlin is "(structural)" — same tier, inconsistent label.
rel_pathat ~L483 duplicates therelative_toalready computed inside the try.- Per-file except tuple differs from kotlin.py's — align while fixing finding 1.
Also a heads-up on land order: #19 (polyglot) structurally rewrites ingest.py, and your touches there are small — expect to rebase this onto #19 rather than the reverse; while doing so, eyeball the interaction between your Cargo-workspace root detection in _detect_source_root and #19's multilang path (which deliberately bypasses it).
Test results on the branch: 290 passed, ruff clean; deep-parens/local-types/qualified-external-leaf/macros/invalid-file adversarial runs all clean except the four recursion inputs above. Fix finding 1, decide finding 2, and this is merge-ready.
9281b4e to
55d9760
Compare
|
Addressed all requested changes in Blocking fixes
Inline review fixes
Recurrence prevention and dogfooding
Validation
The PR is currently |
|
Resolved the remaining Root cause: package recovery correctly recognized thin facades by outgoing dependency affinity, but any caller from another component prevented reassignment. This left The refinement now treats incoming callers as usage, not responsibility, only for oversized package buckets. Compact boundaries remain package-anchored, so focused components such as Self-dogfood delta:
Validation: |
Addresses the remaining PR #18 review findings after 55d9760. Finding 2 — `#[cfg(test)] mod tests` contents became production entities. 55d9760 started skipping those modules but did so unconditionally. The skip is now gated on `exclude_tests`, which is what the reviewer asked for and what keeps the flag honest: `ingest(exclude_tests=False)` must still yield test entities. `exclude_tests` is a `LanguageParser` attribute (default True, ignored by parsers without inline tests), threaded from the `parse` tool and `analyze`, and folded into the parse cache key so the two graph shapes cannot collide in the cache. Non-blocking — removed `_MAX_FILE_BYTES`. It silently dropped >1MB files, no other parser has it, and it never mitigated the recursion crashes it appeared to guard against; the per-file `except Exception` boundary is the real backstop. Recorded the rule in the bug catalog and corrected the README, which had documented the cap. Tests: cfg(test) modules kept with exclude_tests=False and dropped (with their nested items) by default, the parse tool threading the flag, cache key sensitivity, a >1MB file surviving, and a parenthesized-type case added to the deep-AST matrix. Signed-off-by: Duc Minh Le <duclm.bk@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up — per-finding statusVerified each finding against the branch as it stands (three commits landed after the review:
Finding 1 — verified, not just assumedAll four sites are explicit LIFO worklists now, and extraction moved inside the per-file
No
|
The merge+relink introduced for roadmap #18 was language-blind: all three resolution paths (unique-leaf fallback in resolve_name, leaf import fallback and exact-FQN import match in relink_edges) could bind an entity to one written in an unrelated language. On a Python+Java fixture this fabricated `app.service.PaymentHandler -[extends]-> com.example.core.Base` and an import edge from a Python module to the Java class with the same FQN, which then propagated into pkg recovery and WCA clustering. Introduce language families (jvm = java+kotlin; every other language its own family) and gate every resolution path plus the merge step on family compatibility. The unique-leaf index is now built per family, so in-family resolution is unaffected. Cross-family FQN collisions no longer drop an entity silently: the later one is re-keyed as `<fqn>#<language>` with its edges and package listings remapped, and collision counts are reported in the new DependencyGraph.metadata (also surfaced in MCP summaries, since agents never see log lines). Single-language output is unchanged byte-for-byte (verified against origin/main across six fixture/language combinations; metadata is omitted from serialization when empty). Also from the review: - document the supported polyglot pairs (module docstring, parse/MCP docstrings, README table, ROADMAP) - drop the dead leaf-split in resolve_name - remove the scala entry from _LANG_PREFERRED_ROOTS (no Scala parser) - filter provided files by language in the single-language branch too, with a warning for skipped files - count only files (not directories) when resolving language="multi" - sort/deduplicate an explicit languages list so ordering cannot change collision winners Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add languages=[] / language="multi" for Maven dual-root discovery and merge+relink of import/extends/implements across Java↔Kotlin FQN space (roadmap #18 MVP). Wire MCP and CI analyze inputs; keep first entity on cross-language FQN collisions. Co-authored-by: Cursor <cursoragent@cursor.com>
The merge+relink introduced for roadmap #18 was language-blind: all three resolution paths (unique-leaf fallback in resolve_name, leaf import fallback and exact-FQN import match in relink_edges) could bind an entity to one written in an unrelated language. On a Python+Java fixture this fabricated `app.service.PaymentHandler -[extends]-> com.example.core.Base` and an import edge from a Python module to the Java class with the same FQN, which then propagated into pkg recovery and WCA clustering. Introduce language families (jvm = java+kotlin; every other language its own family) and gate every resolution path plus the merge step on family compatibility. The unique-leaf index is now built per family, so in-family resolution is unaffected. Cross-family FQN collisions no longer drop an entity silently: the later one is re-keyed as `<fqn>#<language>` with its edges and package listings remapped, and collision counts are reported in the new DependencyGraph.metadata (also surfaced in MCP summaries, since agents never see log lines). Single-language output is unchanged byte-for-byte (verified against origin/main across six fixture/language combinations; metadata is omitted from serialization when empty). Also from the review: - document the supported polyglot pairs (module docstring, parse/MCP docstrings, README table, ROADMAP) - drop the dead leaf-split in resolve_name - remove the scala entry from _LANG_PREFERRED_ROOTS (no Scala parser) - filter provided files by language in the single-language branch too, with a warning for skipped files - count only files (not directories) when resolving language="multi" - sort/deduplicate an explicit languages list so ordering cannot change collision winners Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Posting a self-review before merge — I verified the branch in an isolated worktree: 314 tests pass, ruff clean, mypy clean on 1. Confirmed bug: comment between
|
* feat: polyglot multilang ingest/parse with cross-language relink Add languages=[] / language="multi" for Maven dual-root discovery and merge+relink of import/extends/implements across Java↔Kotlin FQN space (roadmap #18 MVP). Wire MCP and CI analyze inputs; keep first entity on cross-language FQN collisions. Co-authored-by: Cursor <cursoragent@cursor.com> * test: add Java+Kotlin polyglot E2E coverage Cover ingest→parse→recover, MCP session pipeline, and CLI --languages self-analysis against existing mixed fixtures. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address Copilot multilang parse/ingest review Detect multi-language parse from path suffixes without requiring files on disk, and fail fast on unknown ingest language overrides. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: complete polyglot MCP pipeline * fix(multilang): scope cross-language merge and relink by language family The merge+relink introduced for roadmap #18 was language-blind: all three resolution paths (unique-leaf fallback in resolve_name, leaf import fallback and exact-FQN import match in relink_edges) could bind an entity to one written in an unrelated language. On a Python+Java fixture this fabricated `app.service.PaymentHandler -[extends]-> com.example.core.Base` and an import edge from a Python module to the Java class with the same FQN, which then propagated into pkg recovery and WCA clustering. Introduce language families (jvm = java+kotlin; every other language its own family) and gate every resolution path plus the merge step on family compatibility. The unique-leaf index is now built per family, so in-family resolution is unaffected. Cross-family FQN collisions no longer drop an entity silently: the later one is re-keyed as `<fqn>#<language>` with its edges and package listings remapped, and collision counts are reported in the new DependencyGraph.metadata (also surfaced in MCP summaries, since agents never see log lines). Single-language output is unchanged byte-for-byte (verified against origin/main across six fixture/language combinations; metadata is omitted from serialization when empty). Also from the review: - document the supported polyglot pairs (module docstring, parse/MCP docstrings, README table, ROADMAP) - drop the dead leaf-split in resolve_name - remove the scala entry from _LANG_PREFERRED_ROOTS (no Scala parser) - filter provided files by language in the single-language branch too, with a warning for skipped files - count only files (not directories) when resolving language="multi" - sort/deduplicate an explicit languages list so ordering cannot change collision winners Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Duc Le <duclm.bk@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
srcrootsAddresses the Rust parser roadmap item tracked in #13. The status-board issue should remain open.
Accuracy review
The regression suite covers raw identifiers, unions, async/unsafe/extern functions, inline modules,
superglob imports, aliases, generic owners, cfg-alternative impls, external qualified owners, blanket generic impls, Cargo workspaces, source-root detection, and cache invalidation.Two false-positive cases found during review are explicitly guarded:
std::io::Errorcannot resolve to an unrelated localErrorimpl<T> Trait for &mut Tcannot attach methods to an unrelated local structTKnown static-analysis boundaries: macro-generated items are not expanded, cfg alternatives are structurally merged, and external dependency entities are intentionally not added.
Validation
.venv/bin/python -m pytest -q— 261 passed.venv/bin/python -m ruff check src/ tests/— passedparsers/rust.py(remaining findings are pre-existing in five imported modules)227381db0ee83dfa4341f1e27ff9617c0f5ad992(66k+ stars):arcade_agent/parsers/rust.pyis packaged