From ed9c87887f10c4c6f691b0e275ab7f201e503024 Mon Sep 17 00:00:00 2001 From: anonymous Date: Sun, 19 Jul 2026 21:31:39 -0500 Subject: [PATCH 1/4] feat(parser): add Rust support --- .../architecture-analysis-reusable.yml | 2 +- README.md | 9 +- ROADMAP.md | 4 +- actions/analyze/action.yml | 2 +- pyproject.toml | 1 + src/arcade_agent/cache.py | 2 +- src/arcade_agent/ci/run_self_analysis.py | 2 +- src/arcade_agent/parsers/__init__.py | 5 + src/arcade_agent/parsers/rust.py | 630 ++++++++++++++++++ src/arcade_agent/tools/adapters/mcp.py | 4 +- src/arcade_agent/tools/ingest.py | 14 +- tests/test_cache.py | 9 + tests/test_parsers/test_rust.py | 238 +++++++ 13 files changed, 911 insertions(+), 11 deletions(-) create mode 100644 src/arcade_agent/parsers/rust.py create mode 100644 tests/test_parsers/test_rust.py diff --git a/.github/workflows/architecture-analysis-reusable.yml b/.github/workflows/architecture-analysis-reusable.yml index 9b6e39e..c68b652 100644 --- a/.github/workflows/architecture-analysis-reusable.yml +++ b/.github/workflows/architecture-analysis-reusable.yml @@ -24,7 +24,7 @@ on: type: string default: "." language: - description: Optional language override (java, python, typescript, c, go, kotlin). + description: Optional language override (java, python, typescript, c, go, kotlin, rust). required: false type: string default: "" diff --git a/README.md b/README.md index 82c2873..09d09cd 100644 --- a/README.md +++ b/README.md @@ -18,10 +18,10 @@ Provides composable tools for parsing source code, recovering architecture, dete ## Install ```bash -pip install -e ".[dev]" +pip install -e ".[dev,languages]" # With MCP server support (for AI agent integration) -pip install -e ".[mcp,dev]" +pip install -e ".[mcp,dev,languages]" ``` ## Quick Start @@ -130,6 +130,9 @@ smell burden, or another architectural pressure. - TypeScript/JavaScript (full support) - Go (full support) - Kotlin (structural support via optional `[languages]` extra; import + inheritance graph) +- Rust (structs, enums, unions, traits, type aliases, functions, methods, + imports, qualified references, trait inheritance/implementations, and Cargo + workspaces) ## Example: ARCADE Core @@ -318,7 +321,7 @@ arcade-agent ports and extends the capabilities of the original [ARCADE](https:/ | 6 quality metrics | Done | RCI, TurboMQ, BasicMQ, IntraConnectivity, InterConnectivity, TwoWayPairRatio | | Balanced architecture score | Done | Derived reporting score combining core metrics, principle signals, and smell burden | | A2A architecture comparison | Done | Hungarian algorithm on Jaccard similarity | -| Multi-language parsing | Done | Java, Python, C/C++, TypeScript/JavaScript, Go (full); Kotlin (structural) | +| Multi-language parsing | Done | Java, Python, C/C++, TypeScript/JavaScript, Go, Rust (full); Kotlin (structural) | | 5 export formats | Done | HTML, DOT, JSON, RSF, Mermaid | | LLM concern extraction | Done | Claude CLI for semantic BCO/SPF detection | | MCP server | Done | Expose tools to AI agents via Model Context Protocol with session store | diff --git a/ROADMAP.md b/ROADMAP.md index 469311f..c30bf0c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -43,7 +43,9 @@ Handle real-world polyglot monorepos. - [x] **15. TypeScript/JS parser** — Shipped in #8 (`parsers/typescript.py`). - [x] **16a. Go parser** — Shipped alongside TS/JS in #8 (`parsers/go.py`). - [x] **16a2. Kotlin parser** — Shipped (`parsers/kotlin.py`) for JVM/Kotlin-first repos (e.g. embabel-agent). -- [ ] **16b. Rust parser** — Still open. High-demand language for agent-assisted development. +- [x] **16b. Rust parser** — Tree-sitter parser for Rust modules, types, traits, functions, + methods, imports, qualified references, trait relationships, and Cargo workspaces + (`parsers/rust.py`). - [x] **17. Incremental parsing** — Content-hash extract cache shipped in #9 (`incremental.py`), wired for the Python parser only; extending to the other two-pass parsers is follow-up. - [ ] **18. Cross-language dependency tracking** — Java↔Python via gRPC, TS frontend↔Java backend, etc. diff --git a/actions/analyze/action.yml b/actions/analyze/action.yml index 649dfc1..d550981 100644 --- a/actions/analyze/action.yml +++ b/actions/analyze/action.yml @@ -23,7 +23,7 @@ inputs: required: false default: "." language: - description: Optional language override (java, python, typescript, c, go, kotlin). + description: Optional language override (java, python, typescript, c, go, kotlin, rust). required: false default: "" repo-name: diff --git a/pyproject.toml b/pyproject.toml index 8199738..c59881d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,6 +45,7 @@ languages = [ "tree-sitter-c>=0.23.0", "tree-sitter-cpp>=0.23.0", "tree-sitter-kotlin>=1.1.0", + "tree-sitter-rust>=0.23.0", ] mcp = [ "mcp[cli]>=1.0", diff --git a/src/arcade_agent/cache.py b/src/arcade_agent/cache.py index 91d222f..cdcbcef 100644 --- a/src/arcade_agent/cache.py +++ b/src/arcade_agent/cache.py @@ -46,7 +46,7 @@ def cache_key(source_path: str, language: str | None, files: list[str] | None) - # Hash all source-like files under root file_paths = sorted(str(f) for f in root.rglob("*") if f.is_file() and f.suffix in { ".java", ".py", ".c", ".cpp", ".h", ".hpp", ".ts", ".tsx", ".js", ".jsx", - ".go", ".kt", ".kts", + ".go", ".kt", ".kts", ".rs", }) for fp in file_paths: diff --git a/src/arcade_agent/ci/run_self_analysis.py b/src/arcade_agent/ci/run_self_analysis.py index 3e137f3..a7cff78 100644 --- a/src/arcade_agent/ci/run_self_analysis.py +++ b/src/arcade_agent/ci/run_self_analysis.py @@ -100,7 +100,7 @@ def main() -> None: parser.add_argument( "--language", default="", - help="Optional language override (java, python, typescript, c, go, kotlin)", + help="Optional language override (java, python, typescript, c, go, kotlin, rust)", ) parser.add_argument( "--repo-name", diff --git a/src/arcade_agent/parsers/__init__.py b/src/arcade_agent/parsers/__init__.py index ce1c445..1e971f1 100644 --- a/src/arcade_agent/parsers/__init__.py +++ b/src/arcade_agent/parsers/__init__.py @@ -26,6 +26,11 @@ except ImportError: pass +try: + import arcade_agent.parsers.rust # noqa: F401 +except ImportError: + pass + from arcade_agent.parsers.base import LanguageParser, get_parser __all__ = ["LanguageParser", "get_parser"] diff --git a/src/arcade_agent/parsers/rust.py b/src/arcade_agent/parsers/rust.py new file mode 100644 index 0000000..240fe59 --- /dev/null +++ b/src/arcade_agent/parsers/rust.py @@ -0,0 +1,630 @@ +"""Rust parser using tree-sitter. + +The parser follows Rust's file-module convention (``lib.rs``/``main.rs`` at +the crate root, ``mod.rs`` for a directory module) and extracts structs, +enums, unions, traits, type aliases, functions, and methods. A second pass +resolves ``use`` declarations, same-module references, qualified paths, and +trait inheritance/implementations into dependency edges. Cargo workspaces are +kept intact and each member crate gets a stable graph prefix. +""" + +from __future__ import annotations + +import tomllib +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +import tree_sitter_rust as tsrust +from tree_sitter import Language, Node, Parser + +from arcade_agent.parsers.base import LanguageParser, register_parser +from arcade_agent.parsers.graph import DependencyGraph, Edge, Entity + +RUST_LANGUAGE = Language(tsrust.language()) + +_MAX_FILE_BYTES = 1_000_000 +_TYPE_ITEMS = { + "struct_item": "struct", + "enum_item": "enum", + "union_item": "union", + "trait_item": "trait", + "type_item": "type", +} + + +@dataclass(frozen=True) +class _Import: + path: tuple[str, ...] + alias: str + wildcard: bool = False + + @property + def display(self) -> str: + suffix = "::*" if self.wildcard else "" + return "::".join(self.path) + suffix + + +@dataclass +class _References: + simple: set[str] + qualified: set[tuple[str, ...]] + + +@dataclass +class _PendingMethod: + name: str + references: _References + + +@dataclass +class _PendingImpl: + owner_path: tuple[str, ...] + trait_path: tuple[str, ...] + generic_parameters: frozenset[str] + module: tuple[str, ...] + crate: tuple[str, ...] + imports: list[_Import] + rel_path: str + methods: list[_PendingMethod] + + +def _get_text(node: Node | None) -> str: + raw_text = None if node is None else node.text + return "" if raw_text is None else raw_text.decode(errors="replace") + + +def _identifier(text: str) -> str: + """Normalize raw Rust identifiers (``r#type`` -> ``type``).""" + return text[2:] if text.startswith("r#") else text + + +def _cargo_data(directory: Path) -> dict[str, Any]: + manifest = directory / "Cargo.toml" + try: + return tomllib.loads(manifest.read_text()) if manifest.is_file() else {} + except (OSError, tomllib.TOMLDecodeError): + return {} + + +def _crate_context(file_path: Path, root: Path, is_workspace: bool) -> tuple[Path, tuple[str, ...]]: + """Return the source root and graph prefix for the file's Cargo crate.""" + if not is_workspace: + conventional_source = root / "src" + if conventional_source in file_path.parents: + return conventional_source, () + return root, () + + current = file_path.parent + crate_dir: Path | None = None + while current == root or root in current.parents: + if (current / "Cargo.toml").is_file(): + crate_dir = current + break + if current == root: + break + current = current.parent + if crate_dir is None: + return root, () + + package = _cargo_data(crate_dir).get("package", {}) + crate_name = str(package.get("name", crate_dir.name)).replace("-", "_") + source_root = crate_dir / "src" + if source_root not in file_path.parents: + source_root = crate_dir + return source_root, (_identifier(crate_name),) + + +def _module_name(file_path: Path, source_root: Path, crate: tuple[str, ...]) -> tuple[str, ...]: + """Return the Rust module path represented by a source file.""" + rel = file_path.relative_to(source_root) + parts = list(rel.parts) + stem = Path(parts[-1]).stem + directory = parts[:-1] + if stem in {"lib", "main"}: + return (*crate, *directory) + if stem == "mod": + return (*crate, *directory) + return tuple([*crate, *directory, stem]) + + +def _path_segments(node: Node | None) -> tuple[str, ...]: + """Extract ``a::b::Name`` segments from a path-like AST node.""" + if node is None: + return () + if node.type in { + "identifier", + "type_identifier", + "crate", + "self", + "super", + "metavariable", + }: + return (_identifier(_get_text(node)),) + + path = node.child_by_field_name("path") + name = node.child_by_field_name("name") + if path is not None or name is not None: + return (*_path_segments(path), *_path_segments(name)) + + # use_wildcard has no named fields in tree-sitter-rust 0.24. + if node.type == "use_wildcard" and node.named_children: + return _path_segments(node.named_children[0]) + + # References through generic paths only need the base type name. + if node.type == "generic_type": + return _path_segments(node.child_by_field_name("type")) + return () + + +def _flatten_use(node: Node, prefix: tuple[str, ...] = ()) -> list[_Import]: + """Flatten a Rust use tree into concrete imports.""" + if node.type == "use_declaration": + argument = node.child_by_field_name("argument") + return _flatten_use(argument, prefix) if argument is not None else [] + + if node.type == "scoped_use_list": + path = (*prefix, *_path_segments(node.child_by_field_name("path"))) + use_list = node.child_by_field_name("list") + if use_list is None: + return [] + imports: list[_Import] = [] + for child in use_list.named_children: + if child.type == "self": + alias = path[-1] if path else "self" + imports.append(_Import(path=path, alias=alias)) + else: + imports.extend(_flatten_use(child, path)) + return imports + + if node.type == "use_list": + imports = [] + for child in node.named_children: + imports.extend(_flatten_use(child, prefix)) + return imports + + if node.type == "use_as_clause": + path = (*prefix, *_path_segments(node.child_by_field_name("path"))) + alias_node = node.child_by_field_name("alias") + alias = _identifier(_get_text(alias_node)) if alias_node is not None else path[-1] + return [_Import(path=path, alias=alias)] if path else [] + + if node.type == "use_wildcard": + path = (*prefix, *_path_segments(node)) + return [_Import(path=path, alias="*", wildcard=True)] if path else [] + + path = (*prefix, *_path_segments(node)) + return [_Import(path=path, alias=path[-1])] if path else [] + + +def _extract_imports(container: Node) -> list[_Import]: + imports: list[_Import] = [] + for child in container.named_children: + if child.type == "use_declaration": + imports.extend(_flatten_use(child)) + return imports + + +def _references(node: Node) -> _References: + simple: set[str] = set() + qualified: set[tuple[str, ...]] = set() + stack = [node] + while stack: + current = stack.pop() + if current.type in {"scoped_identifier", "scoped_type_identifier"}: + path = _path_segments(current) + if len(path) > 1: + qualified.add(path) + elif current.type in {"identifier", "type_identifier"}: + simple.add(_identifier(_get_text(current))) + stack.extend(current.named_children) + return _References(simple=simple, qualified=qualified) + + +def _base_type_path(node: Node | None) -> tuple[str, ...]: + """Get the implemented type path without generic arguments.""" + if node is None: + return () + # Wrapper nodes can contain lifetimes before the actual type. Follow the + # grammar's explicit type field so ``&'a mut T`` resolves to T, not ``a``. + wrapped_type = node.child_by_field_name("type") + if node.type in {"reference_type", "pointer_type", "array_type", "slice_type"}: + return _base_type_path(wrapped_type) + direct = _path_segments(node) + if direct: + return direct + for child in node.named_children: + path = _base_type_path(child) + if path: + return path + return () + + +def _generic_type_parameters(node: Node) -> frozenset[str]: + """Return type parameter names declared by an impl (excluding lifetimes).""" + parameters = node.child_by_field_name("type_parameters") + if parameters is None: + return frozenset() + return frozenset( + _identifier(_get_text(name)) + for parameter in parameters.named_children + if parameter.type == "type_parameter" + for name in [parameter.child_by_field_name("name")] + if name is not None + ) + + +def _normalize_path( + path: tuple[str, ...], + current: tuple[str, ...], + crate: tuple[str, ...] = (), +) -> tuple[str, ...]: + if not path: + return () + parts = list(path) + if parts[0] == "crate": + return (*crate, *parts[1:]) + if parts[0] == "self": + return (*current, *parts[1:]) + if parts[0] == "super": + base = list(current) + while parts and parts[0] == "super": + if base: + base.pop() + parts.pop(0) + return (*base, *parts) + return (*current, *parts) + + +def _deduplicate(edges: list[Edge]) -> list[Edge]: + seen: set[tuple[str, str, str]] = set() + unique: list[Edge] = [] + for edge in edges: + key = (edge.source, edge.target, edge.relation) + if edge.source != edge.target and key not in seen: + seen.add(key) + unique.append(edge) + return unique + + +@register_parser +class RustParser(LanguageParser): + """Rust source code parser using tree-sitter.""" + + @property + def language(self) -> str: + return "rust" + + @property + def file_extensions(self) -> list[str]: + return [".rs"] + + def parse(self, files: list[Path], root: Path) -> DependencyGraph: + parser = Parser(RUST_LANGUAGE) + root = root.resolve() + + entities: dict[str, Entity] = {} + packages: dict[str, list[str]] = {} + entity_refs: dict[str, _References] = {} + entity_imports: dict[str, list[_Import]] = {} + entity_crates: dict[str, tuple[str, ...]] = {} + pending_impls: list[_PendingImpl] = [] + pending_trait_bounds: list[ + tuple[str, tuple[str, ...], tuple[str, ...], tuple[str, ...], list[_Import]] + ] = [] + is_workspace = "workspace" in _cargo_data(root) + + def add_entity( + *, + name: str, + kind: str, + module: tuple[str, ...], + rel_path: str, + imports: list[_Import], + node: Node | None, + crate: tuple[str, ...], + owner: str | None = None, + fqn_override: str | None = None, + references: _References | None = None, + ) -> str: + package = ".".join(module) + fqn = fqn_override or (f"{package}.{name}" if package else name) + entities[fqn] = Entity( + fqn=fqn, + name=name, + package=package, + file_path=rel_path, + kind=kind, + language="rust", + imports=[item.display for item in imports], + properties={"owner": owner} if owner else {}, + ) + package_entities = packages.setdefault(package, []) + if fqn not in package_entities: + package_entities.append(fqn) + if references is not None: + entity_refs[fqn] = references + elif node is not None: + entity_refs[fqn] = _references(node) + else: + entity_refs[fqn] = _References(set(), set()) + entity_imports[fqn] = imports + entity_crates[fqn] = crate + return fqn + + def visit_container( + container: Node, + module: tuple[str, ...], + rel_path: str, + file_stem: str, + crate: tuple[str, ...], + is_file_root: bool = False, + ) -> None: + imports = _extract_imports(container) + direct_entities = 0 + + for node in container.named_children: + if node.type in _TYPE_ITEMS: + name_node = node.child_by_field_name("name") + if name_node is None: + continue + name = _identifier(_get_text(name_node)) + owner = add_entity( + name=name, + kind=_TYPE_ITEMS[node.type], + module=module, + rel_path=rel_path, + imports=imports, + node=node, + crate=crate, + ) + direct_entities += 1 + if node.type == "trait_item": + bounds = node.child_by_field_name("bounds") + if bounds is not None: + for bound in bounds.named_children: + bound_path = _base_type_path(bound) + if bound_path: + pending_trait_bounds.append( + (owner, bound_path, module, crate, imports) + ) + body = node.child_by_field_name("body") + if body is not None: + for member in body.named_children: + if member.type not in {"function_item", "function_signature_item"}: + continue + method_name = member.child_by_field_name("name") + if method_name is None: + continue + add_entity( + name=_identifier(_get_text(method_name)), + kind="method", + module=module, + rel_path=rel_path, + imports=imports, + node=member, + crate=crate, + owner=owner, + fqn_override=f"{owner}.{_identifier(_get_text(method_name))}", + ) + elif node.type == "function_item": + name_node = node.child_by_field_name("name") + if name_node is not None: + add_entity( + name=_identifier(_get_text(name_node)), + kind="function", + module=module, + rel_path=rel_path, + imports=imports, + node=node, + crate=crate, + ) + direct_entities += 1 + elif node.type == "impl_item": + type_node = node.child_by_field_name("type") + trait_node = node.child_by_field_name("trait") + body = node.child_by_field_name("body") + owner_path = _base_type_path(type_node) + if not owner_path or body is None: + continue + methods: list[_PendingMethod] = [] + for member in body.named_children: + if member.type != "function_item": + continue + method_name = member.child_by_field_name("name") + if method_name is None: + continue + name = _identifier(_get_text(method_name)) + methods.append(_PendingMethod(name=name, references=_references(member))) + pending_impls.append( + _PendingImpl( + owner_path=owner_path, + trait_path=_base_type_path(trait_node), + generic_parameters=_generic_type_parameters(node), + module=module, + crate=crate, + imports=imports, + rel_path=rel_path, + methods=methods, + ) + ) + elif node.type == "mod_item": + name_node = node.child_by_field_name("name") + body = node.child_by_field_name("body") + if name_node is not None and body is not None: + child_module = (*module, _identifier(_get_text(name_node))) + visit_container(body, child_module, rel_path, file_stem, crate) + + if is_file_root and direct_entities == 0: + module_name = ".".join(module) + name = module[-1] if module else file_stem + fqn = module_name or file_stem + add_entity( + name=name, + kind="module", + module=module[:-1] if module else (), + rel_path=rel_path, + imports=imports, + node=container, + crate=crate, + fqn_override=fqn, + ) + + for source_file in files: + source_file = source_file.resolve() + try: + source_file.relative_to(root) + if source_file.stat().st_size > _MAX_FILE_BYTES: + continue + tree = parser.parse(source_file.read_bytes()) + except (OSError, ValueError): + continue + + rel_path = str(source_file.relative_to(root)) + source_root, crate = _crate_context(source_file, root, is_workspace) + visit_container( + tree.root_node, + _module_name(source_file, source_root, crate), + rel_path, + source_file.stem, + crate, + is_file_root=True, + ) + + non_member_entities = [e for e in entities.values() if e.kind != "method"] + by_module_name = {(e.package, e.name): e.fqn for e in non_member_entities} + by_simple_name: dict[str, list[str]] = {} + for entity in non_member_entities: + by_simple_name.setdefault(entity.name, []).append(entity.fqn) + + def resolve( + path: tuple[str, ...], + current_package: str, + crate: tuple[str, ...] = (), + ) -> str | None: + current = tuple(part for part in current_package.split(".") if part) + candidates: list[tuple[str, ...]] = [] + if path and path[0] in {"crate", "self", "super"}: + candidates.append(_normalize_path(path, current, crate)) + else: + candidates.extend([(*current, *path), path]) + for candidate in candidates: + fqn = ".".join(candidate) + if fqn in entities: + return fqn + # A qualified path that did not match is external or unresolved; + # falling back by its final name could link ``std::io::Error`` to + # an unrelated local ``Error``. Keep the unique-name fallback only + # for simple paths (notably owners imported through a glob). + if len(path) == 1: + matches = by_simple_name.get(path[-1], []) + if len(matches) == 1: + return matches[0] + return None + + def expand_alias(path: tuple[str, ...], imports: list[_Import]) -> tuple[str, ...]: + if not path: + return path + for item in imports: + if not item.wildcard and path[0] == item.alias: + return (*item.path, *path[1:]) + return path + + # Resolve impl owners only after all type declarations are known. This + # correctly attaches impls written in sibling modules and avoids graph + # entities for external/blanket owners such as std::io::Error, Box, + # or &'a mut T. + edges: list[Edge] = [] + owner_kinds = {"struct", "enum", "union", "type"} + + for ( + bound_owner, + bound_path, + bound_module, + bound_crate, + bound_imports, + ) in pending_trait_bounds: + package = ".".join(bound_module) + bound_target = resolve(expand_alias(bound_path, bound_imports), package, bound_crate) + if bound_target and entities[bound_target].kind == "trait": + bound_name = "::".join(bound_path) + if bound_name not in entities[bound_owner].interfaces: + entities[bound_owner].interfaces.append(bound_name) + edges.append(Edge(bound_owner, bound_target, "extends")) + + for pending in pending_impls: + package = ".".join(pending.module) + owner_path = expand_alias(pending.owner_path, pending.imports) + if len(owner_path) == 1 and owner_path[0] in pending.generic_parameters: + continue + resolved_owner = resolve(owner_path, package, pending.crate) + if resolved_owner is None or entities[resolved_owner].kind not in owner_kinds: + continue + + owner_module = tuple( + part for part in entities[resolved_owner].package.split(".") if part + ) + for method in pending.methods: + add_entity( + name=method.name, + kind="method", + module=owner_module, + rel_path=pending.rel_path, + imports=pending.imports, + node=None, + crate=pending.crate, + owner=resolved_owner, + fqn_override=f"{resolved_owner}.{method.name}", + references=method.references, + ) + + if pending.trait_path: + trait_path = expand_alias(pending.trait_path, pending.imports) + trait_target = resolve(trait_path, package, pending.crate) + if trait_target and entities[trait_target].kind == "trait": + trait_name = "::".join(pending.trait_path) + if trait_name not in entities[resolved_owner].interfaces: + entities[resolved_owner].interfaces.append(trait_name) + edges.append(Edge(resolved_owner, trait_target, "implements")) + + for fqn, entity in entities.items(): + refs = entity_refs.get(fqn, _References(set(), set())) + imports = entity_imports.get(fqn, []) + crate = entity_crates.get(fqn, ()) + + for item in imports: + if item.wildcard: + module_path = _normalize_path( + item.path, + tuple(part for part in entity.package.split(".") if part), + crate, + ) + wildcard_module = ".".join(module_path) + for ref in refs.simple: + wildcard_target = by_module_name.get((wildcard_module, ref)) + if wildcard_target: + edges.append(Edge(fqn, wildcard_target, "import")) + continue + if item.alias not in refs.simple and entity.kind != "module": + continue + import_target = resolve(item.path, entity.package, crate) + if import_target: + edges.append(Edge(fqn, import_target, "import")) + + for ref in refs.simple: + same_module_target = by_module_name.get((entity.package, ref)) + if same_module_target: + edges.append(Edge(fqn, same_module_target, "uses")) + + for qualified_path in refs.qualified: + qualified_target = resolve( + expand_alias(qualified_path, imports), entity.package, crate + ) + if qualified_target: + edges.append(Edge(fqn, qualified_target, "uses")) + + return DependencyGraph( + entities=entities, + edges=_deduplicate(edges), + packages=packages, + ) diff --git a/src/arcade_agent/tools/adapters/mcp.py b/src/arcade_agent/tools/adapters/mcp.py index 52b8d90..1627d89 100644 --- a/src/arcade_agent/tools/adapters/mcp.py +++ b/src/arcade_agent/tools/adapters/mcp.py @@ -146,7 +146,7 @@ def ingest( Args: source: Git repo URL or local directory path. - language: Override language detection (java, python, c, typescript, go, kotlin). + language: Override language detection (java, python, c, typescript, go, kotlin, rust). work_dir: Directory to clone into. Uses temp dir if None. exclude_tests: Exclude test/vendor/build directories (default True). source_root: Override source root (e.g. 'src/main/java'). @@ -181,7 +181,7 @@ def parse( Args: source_path: Root directory of the project. - language: Language to parse (java, python, c, typescript, go, kotlin). + language: Language to parse (java, python, c, typescript, go, kotlin, rust). Auto-detected if None. files: Specific files to parse. Discovers all if None. use_cache: Return cached results when source files haven't changed. diff --git a/src/arcade_agent/tools/ingest.py b/src/arcade_agent/tools/ingest.py index 6280cec..e312d40 100644 --- a/src/arcade_agent/tools/ingest.py +++ b/src/arcade_agent/tools/ingest.py @@ -4,6 +4,7 @@ import shutil import tempfile +import tomllib from dataclasses import dataclass, field from pathlib import Path from typing import TYPE_CHECKING @@ -40,6 +41,7 @@ def cleanup(self) -> None: "c": [".c", ".h", ".cpp", ".hpp", ".cc", ".cxx"], "go": [".go"], "kotlin": [".kt", ".kts"], + "rust": [".rs"], } # Reverse mapping @@ -105,6 +107,16 @@ def _detect_source_root(path: Path, language: str | None = None) -> Path: Checks for well-known source root patterns (e.g., src/main/java for Maven). Falls back to the project root. """ + # A Cargo workspace may contain a root crate plus multiple member crates. + # Narrowing it to the first ``src`` directory would silently drop members. + if language == "rust": + manifest = path / "Cargo.toml" + try: + if manifest.is_file() and "workspace" in tomllib.loads(manifest.read_text()): + return path + except (OSError, tomllib.TOMLDecodeError): + pass + for candidate in _SOURCE_ROOTS: root = path / candidate if root.is_dir(): @@ -204,7 +216,7 @@ def ingest( Args: source: Git repo URL or local directory path. - language: Override language detection (java, python, typescript, c, go, kotlin). + language: Override language detection (java, python, typescript, c, go, kotlin, rust). work_dir: Directory to clone into. Uses temp dir if None. exclude_tests: Exclude test/vendor/build directories (default: True). source_root: Override source root (e.g., 'src/main/java'). Auto-detected if None. diff --git a/tests/test_cache.py b/tests/test_cache.py index 4f6906f..1d168a7 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -60,6 +60,15 @@ def test_cache_key_changes_with_file_modification(tmp_project): assert k1 != k2 +def test_cache_key_tracks_rust_source_files(tmp_project): + rust_file = tmp_project / "src" / "lib.rs" + rust_file.write_text("pub struct Before;") + k1 = cache_key(str(tmp_project), "rust", None) + rust_file.write_text("pub struct After;") + k2 = cache_key(str(tmp_project), "rust", None) + assert k1 != k2 + + def test_cache_miss_returns_none(tmp_project): result = get_cached_graph(str(tmp_project), "nonexistent_key") assert result is None diff --git a/tests/test_parsers/test_rust.py b/tests/test_parsers/test_rust.py new file mode 100644 index 0000000..ef64518 --- /dev/null +++ b/tests/test_parsers/test_rust.py @@ -0,0 +1,238 @@ +"""Tests for the Rust parser.""" + +import pytest + +pytest.importorskip("tree_sitter_rust") +from arcade_agent.parsers.rust import RustParser # noqa: E402 +from arcade_agent.tools.ingest import ingest # noqa: E402 +from arcade_agent.tools.parse import parse # noqa: E402 + + +def _project(tmp_path): + (tmp_path / "models.rs").write_text( + "pub struct User { pub id: u64 }\n" + "pub enum Role { Admin, Member }\n" + "pub struct Error;\n" + "pub struct T;\n" + "pub type UserId = u64;\n" + ) + (tmp_path / "repository.rs").write_text( + "use crate::models::User;\n" + "pub trait Resource {}\n" + "pub trait Repository: Resource {\n" + " fn find(&self, id: u64) -> Option;\n" + "}\n" + "pub struct Store;\n" + "impl Repository for Store {\n" + " fn find(&self, id: u64) -> Option { None }\n" + "}\n" + ) + (tmp_path / "service").mkdir() + (tmp_path / "service" / "mod.rs").write_text( + "use crate::models::{Role, User};\n" + "use crate::repository::Repository as Repo;\n" + "pub struct UserService { repo: R }\n" + "pub struct MemoryRepository;\n" + "impl Repo for MemoryRepository {\n" + " fn find(&self, id: u64) -> Option { None }\n" + "}\n" + "impl UserService {\n" + " pub fn get(&self, id: u64) -> Option { self.repo.find(id) }\n" + " pub fn role(&self) -> Role { Role::Member }\n" + "}\n" + ) + (tmp_path / "lib.rs").write_text( + "pub mod models;\n" + "pub mod repository;\n" + "pub mod service;\n" + "pub mod impls;\n" + "pub use service::UserService;\n" + ) + (tmp_path / "impls.rs").write_text( + "use crate::models::User as Account;\n" + "pub trait DisplayAccount { fn display(&self); }\n" + "impl DisplayAccount for Account { fn display(&self) {} }\n" + "pub trait ExternalOwnerHook { fn hook(&self); }\n" + "impl ExternalOwnerHook for std::io::Error { fn hook(&self) {} }\n" + "pub trait Forward { fn forward(&self); }\n" + "impl<'a, T> Forward for &'a mut T { fn forward(&self) {} }\n" + "#[cfg(unix)] impl Account { fn platform(&self) {} }\n" + "#[cfg(windows)] impl Account { fn platform(&self) {} }\n" + ) + return sorted(tmp_path.rglob("*.rs")) + + +def test_rust_parser_properties(): + parser = RustParser() + assert parser.language == "rust" + assert parser.file_extensions == [".rs"] + + +def test_rust_parser_extracts_types_functions_and_methods(tmp_path): + graph = RustParser().parse(_project(tmp_path), tmp_path) + + assert graph.entities["models.User"].kind == "struct" + assert graph.entities["models.Role"].kind == "enum" + assert graph.entities["models.UserId"].kind == "type" + assert graph.entities["repository.Repository"].kind == "trait" + assert graph.entities["repository.Repository.find"].kind == "method" + assert graph.entities["repository.Store.find"].properties["owner"] == "repository.Store" + assert graph.entities["service.UserService.get"].kind == "method" + assert graph.entities["service.UserService.get"].language == "rust" + + +def test_rust_parser_uses_rust_file_module_conventions(tmp_path): + graph = RustParser().parse(_project(tmp_path), tmp_path) + + assert "models" in graph.packages + assert "repository" in graph.packages + assert "service" in graph.packages + assert graph.entities["service.UserService"].file_path == "service/mod.rs" + assert "lib" in graph.entities # module-only crate root remains visible + + +def test_rust_parser_resolves_imports_references_and_trait_impls(tmp_path): + graph = RustParser().parse(_project(tmp_path), tmp_path) + edges = {(edge.source, edge.target, edge.relation) for edge in graph.edges} + + assert ("service.UserService.get", "models.User", "import") in edges + assert ("service.UserService.role", "models.Role", "import") in edges + assert ("repository.Store", "repository.Repository", "implements") in edges + assert ("service.MemoryRepository", "repository.Repository", "implements") in edges + assert ("repository.Repository", "repository.Resource", "extends") in edges + assert ("repository.Repository.find", "models.User", "import") in edges + + +def test_rust_parser_handles_inline_modules_and_empty_input(tmp_path): + source = tmp_path / "lib.rs" + source.write_text( + "mod internal {\n" + " pub struct Config;\n" + " impl Config { pub fn load() -> Self { Self } }\n" + "}\n" + ) + + graph = RustParser().parse([source], tmp_path) + assert "internal.Config" in graph.entities + assert "internal.Config.load" in graph.entities + + empty = RustParser().parse([], tmp_path) + assert empty.num_entities == 0 + assert empty.num_edges == 0 + + +def test_rust_parser_handles_ripgrep_impl_owner_regressions(tmp_path): + # Reduced from ripgrep 227381d: sibling impls from globset/serde_impl.rs, + # external owners from matcher/src/lib.rs, and reference blanket impls from + # ignore/src/walk.rs and searcher/src/sink.rs. + graph = RustParser().parse(_project(tmp_path), tmp_path) + edges = {(edge.source, edge.target, edge.relation) for edge in graph.edges} + + display = graph.entities["models.User.display"] + assert display.properties["owner"] == "models.User" + assert display.file_path == "impls.rs" + assert ("models.User", "impls.DisplayAccount", "implements") in edges + + # External and generic blanket impls must not manufacture local owners. + assert "std.io.Error.hook" not in graph.entities + assert "models.Error.hook" not in graph.entities + assert "impls.a.forward" not in graph.entities + assert "impls.T.forward" not in graph.entities + assert "models.T.forward" not in graph.entities + assert all( + entity.properties["owner"] in graph.entities + for entity in graph.entities.values() + if entity.kind == "method" + ) + + # Mutually exclusive cfg impls collapse to one graph method without + # duplicating package membership. + assert "models.User.platform" in graph.entities + assert all(len(fqns) == len(set(fqns)) for fqns in graph.packages.values()) + + +def test_rust_parser_handles_union_raw_identifiers_and_function_modifiers(tmp_path): + source = tmp_path / "advanced.rs" + source.write_text( + "pub union Payload { integer: u64, float: f64 }\n" + "pub struct r#type;\n" + "pub async fn fetch() {}\n" + "pub unsafe fn unchecked() {}\n" + 'pub extern "C" fn exported() {}\n' + ) + + graph = RustParser().parse([source], tmp_path) + assert graph.entities["advanced.Payload"].kind == "union" + assert graph.entities["advanced.type"].kind == "struct" + assert graph.entities["advanced.fetch"].kind == "function" + assert graph.entities["advanced.unchecked"].kind == "function" + assert graph.entities["advanced.exported"].kind == "function" + + +def test_rust_parser_resolves_super_glob_imports_in_inline_modules(tmp_path): + source = tmp_path / "lib.rs" + source.write_text( + "mod models { pub struct Config; }\n" + "mod service {\n" + " use super::models::*;\n" + " pub fn load(_: Config) {}\n" + "}\n" + ) + + graph = RustParser().parse([source], tmp_path) + edges = {(edge.source, edge.target, edge.relation) for edge in graph.edges} + assert ("service.load", "models.Config", "import") in edges + + +def test_rust_is_auto_detected_by_ingest_and_parse(tmp_path): + files = _project(tmp_path) + + repo = ingest(str(tmp_path)) + assert repo.language == "rust" + assert repo.source_files == files + + graph = parse(str(tmp_path), use_cache=False) + assert "models.User" in graph.entities + + +def test_rust_cargo_workspace_keeps_member_crates_and_crate_paths(tmp_path): + (tmp_path / "Cargo.toml").write_text('[workspace]\nmembers = ["app", "worker"]\n') + for crate in ("app", "worker"): + source = tmp_path / crate / "src" + source.mkdir(parents=True) + (tmp_path / crate / "Cargo.toml").write_text( + f'[package]\nname = "{crate}"\nversion = "0.1.0"\n' + ) + (tmp_path / "worker" / "src" / "lib.rs").write_text("pub struct Worker;\n") + (tmp_path / "app" / "src" / "lib.rs").write_text( + "use worker::Worker;\npub struct App { worker: Worker }\n" + ) + + repo = ingest(str(tmp_path), language="rust") + assert repo.path == tmp_path + assert len(repo.source_files) == 2 + + graph = parse( + str(repo.path), + language="rust", + files=[str(path) for path in repo.source_files], + use_cache=False, + ) + assert "app.App" in graph.entities + assert "worker.Worker" in graph.entities + assert ("app.App", "worker.Worker", "import") in { + (edge.source, edge.target, edge.relation) for edge in graph.edges + } + + +def test_rust_direct_parse_uses_single_crate_src_as_module_root(tmp_path): + (tmp_path / "Cargo.toml").write_text('[package]\nname = "single-crate"\nversion = "0.1.0"\n') + source = tmp_path / "src" + source.mkdir() + (source / "lib.rs").write_text("pub struct RootType;\n") + (source / "service.rs").write_text("pub struct Service;\n") + + graph = parse(str(tmp_path), language="rust", use_cache=False) + assert "RootType" in graph.entities + assert "service.Service" in graph.entities + assert "src.RootType" not in graph.entities From 55d97609c5394260d336d707801e2eaf4efefea8 Mon Sep 17 00:00:00 2001 From: Tony Nguyen Date: Tue, 21 Jul 2026 07:13:21 -0500 Subject: [PATCH 2/4] fix(parser): harden Rust analysis --- .../harden-tree-sitter-parsers/SKILL.md | 43 ++ .../architecture-analysis-reusable.yml | 2 +- README.md | 22 +- ROADMAP.md | 4 +- actions/analyze/action.yml | 2 +- docs/BUG_CATALOG.md | 47 ++ examples/workflows/arcade-agent-analysis.yml | 2 +- src/arcade_agent/cache.py | 40 +- src/arcade_agent/parsers/rust.py | 494 +++++++++++------- src/arcade_agent/tools/ingest.py | 8 +- tests/test_cache.py | 16 + tests/test_parsers/test_rust.py | 72 +++ 12 files changed, 552 insertions(+), 200 deletions(-) create mode 100644 .github/skills/harden-tree-sitter-parsers/SKILL.md create mode 100644 docs/BUG_CATALOG.md diff --git a/.github/skills/harden-tree-sitter-parsers/SKILL.md b/.github/skills/harden-tree-sitter-parsers/SKILL.md new file mode 100644 index 0000000..a962910 --- /dev/null +++ b/.github/skills/harden-tree-sitter-parsers/SKILL.md @@ -0,0 +1,43 @@ +--- +name: harden-tree-sitter-parsers +description: Harden new or changed tree-sitter parsers against adversarial nesting, malformed files, partial-state leaks, and stale non-source cache inputs. +reliability: validated-2x +--- + +# Harden Tree-sitter Parsers + +Use this skill for new parser implementations, parser reviews, recursion failures, +or changes to AST traversal and linking. + +## Required workflow + +1. Inventory every AST traversal helper and classify it as iterative or recursive. +2. Replace source-depth recursion with an explicit stack or queue. Preserve traversal + order deliberately and avoid repeated tuple/list copying where practical. +3. Extract each file into isolated temporary state. Merge entities, edges, imports, + packages, and pending links only after the file succeeds. +4. Log skipped files with the failure class; do not silently discard valid siblings. +5. Add adversarial fixtures deeper than `sys.getrecursionlimit()` for every distinct + traversal shape. Each fixture must be parsed beside a valid sibling file. +6. Test cache invalidation for manifests or configuration that changes graph identity. +7. Run, in order: + - focused parser and cache tests; + - Ruff and the full test suite; + - a large real repository for the target language; + - arcade-agent self-analysis before/after, reporting metric and smell deltas. +8. Record any newly discovered reusable failure class in `docs/BUG_CATALOG.md`. + +## Acceptance invariants + +- No `RecursionError` for valid tree-sitter AST depth within the configured file limit. +- One malformed or adversarial file cannot erase healthy sibling entities. +- No partial entities from a failed file enter the final graph. +- No dangling edges, missing method owners, or duplicate package membership. +- Relevant non-source inputs invalidate cached graphs. +- Correctness and explicit failure behavior take precedence over cosmetic metric gains. + +## Evidence + +- Kotlin follow-up `b7effc5`: iterative deep-expression traversal and sibling survival. +- Rust PR #18: iterative path/use/module/type traversal, transactional file extraction, + Cargo-aware cache invalidation, and adversarial regression matrix. diff --git a/.github/workflows/architecture-analysis-reusable.yml b/.github/workflows/architecture-analysis-reusable.yml index c68b652..bb87589 100644 --- a/.github/workflows/architecture-analysis-reusable.yml +++ b/.github/workflows/architecture-analysis-reusable.yml @@ -12,7 +12,7 @@ on: description: Released arcade-agent package version to install, or "source" to install the checked-out repository (self-dogfooding). required: false type: string - default: "0.1.1" + default: "0.2.0" install-extras: description: Extras string appended to the arcade-agent package install. required: false diff --git a/README.md b/README.md index 09d09cd..df95196 100644 --- a/README.md +++ b/README.md @@ -130,9 +130,15 @@ smell burden, or another architectural pressure. - TypeScript/JavaScript (full support) - Go (full support) - Kotlin (structural support via optional `[languages]` extra; import + inheritance graph) -- Rust (structs, enums, unions, traits, type aliases, functions, methods, +- Rust (structural support for structs, enums, unions, traits, type aliases, functions, methods, imports, qualified references, trait inheritance/implementations, and Cargo - workspaces) + workspaces). Inline `#[cfg(test)]` modules are excluded from production graphs; + files larger than 1 MB are skipped with a warning. + +Tree-sitter parser changes follow the +[adversarial hardening workflow](.github/skills/harden-tree-sitter-parsers/SKILL.md). +Recurring parser failure classes are recorded in the +[living bug catalog](docs/BUG_CATALOG.md). ## Example: ARCADE Core @@ -254,17 +260,17 @@ jobs: issues: write pull-requests: write steps: - - uses: lemduc/arcade-agent/actions/analyze@v0.1.1 + - uses: lemduc/arcade-agent/actions/analyze@v0.2.0 with: - arcade-agent-version: "0.1.1" + arcade-agent-version: "0.2.0" ``` Common optional inputs: ```yaml - - uses: lemduc/arcade-agent/actions/analyze@v0.1.1 + - uses: lemduc/arcade-agent/actions/analyze@v0.2.0 with: - arcade-agent-version: "0.1.1" + arcade-agent-version: "0.2.0" source-path: "." language: "" primary-algorithm: pkg @@ -273,7 +279,7 @@ Common optional inputs: ``` For reproducible CI, keep `arcade-agent-version` pinned to a released package -version such as `"0.1.1"`. Avoid `latest` in shared CI because a new package +version such as `"0.2.0"`. Avoid `latest` in shared CI because a new package release can change analyzer behavior without a workflow review. The action stores the baseline as a GitHub Actions artifact on @@ -321,7 +327,7 @@ arcade-agent ports and extends the capabilities of the original [ARCADE](https:/ | 6 quality metrics | Done | RCI, TurboMQ, BasicMQ, IntraConnectivity, InterConnectivity, TwoWayPairRatio | | Balanced architecture score | Done | Derived reporting score combining core metrics, principle signals, and smell burden | | A2A architecture comparison | Done | Hungarian algorithm on Jaccard similarity | -| Multi-language parsing | Done | Java, Python, C/C++, TypeScript/JavaScript, Go, Rust (full); Kotlin (structural) | +| Multi-language parsing | Done | Java, Python, C/C++, TypeScript/JavaScript, Go (full); Kotlin, Rust (structural) | | 5 export formats | Done | HTML, DOT, JSON, RSF, Mermaid | | LLM concern extraction | Done | Claude CLI for semantic BCO/SPF detection | | MCP server | Done | Expose tools to AI agents via Model Context Protocol with session store | diff --git a/ROADMAP.md b/ROADMAP.md index c30bf0c..79eb966 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -62,7 +62,7 @@ Work everywhere agents work. | Priority | Items | Rationale | |----------|-------|-----------| -| **Done** | 1–9, 12, 13, 14, 15, 16a, 17 | Phases 1–2 + TS/JS & Go parsers, incremental parsing (Python), `diff_impact`, `context_for_task`, `api_surface` | +| **Done** | 1–9, 12, 13, 14, 15, 16a, 16a2, 16b, 17 | Phases 1–2 + TS/JS, Go, Kotlin & Rust parsers, incremental parsing (Python), `diff_impact`, `context_for_task`, `api_surface` | | **Now** | 10 | Architectural changelog | -| **Next** | 11, 16b | Component ownership, Rust parser | +| **Next** | 11 | Component ownership | | **Then** | 18–22 | Cross-language tracking, ecosystem breadth | diff --git a/actions/analyze/action.yml b/actions/analyze/action.yml index d550981..776859b 100644 --- a/actions/analyze/action.yml +++ b/actions/analyze/action.yml @@ -5,7 +5,7 @@ inputs: arcade-agent-version: description: Released arcade-agent package version to install. required: false - default: "0.1.1" + default: "0.2.0" python-version: description: Python version used for analysis. required: false diff --git a/docs/BUG_CATALOG.md b/docs/BUG_CATALOG.md new file mode 100644 index 0000000..550fc70 --- /dev/null +++ b/docs/BUG_CATALOG.md @@ -0,0 +1,47 @@ +# Parser Robustness Bug Catalog + +Reliability: `validated-2x` + +This is the living catalog for parser failure classes that can abort or distort +whole-repository analysis. Entries use reproducible fixtures and design-time +prevention rules so the same defect is not rediscovered language by language. + +## Design-time checklist + +- Traverse untrusted AST depth with explicit stacks or queues, never Python recursion. +- Put every file extraction behind a transactional boundary: publish its entities only + after extraction succeeds. +- Test nesting deeper than `sys.getrecursionlimit()` for every traversal shape. +- Pair each poisoned input with a healthy sibling file and assert the sibling survives. +- Track non-source inputs such as manifests when they affect graph identity or cache keys. +- Run focused tests, the full suite, a large real repository, and arcade-agent's own + self-analysis before publishing parser changes. + +## 1. Kotlin deep-expression traversal aborted repository analysis + +- **Symptom:** A machine-generated expression with thousands of nested parentheses + raised `RecursionError`; valid sibling files disappeared because parsing aborted. +- **Root cause:** Recursive AST descent treated source nesting as trusted call-stack depth. +- **Detection:** Parse a deeply nested Kotlin file beside a valid file and assert the + valid entity remains in the graph. +- **Fix:** Replace recursive descent with explicit stacks and isolate failures per file. +- **Prevention:** Apply the parser hardening skill to every new or materially changed + tree-sitter traversal. +- First encountered: Kotlin parser follow-up `b7effc5`. +- **Pattern note:** First confirmed instance of cross-language AST depth fragility. + +## 2. Rust path/use/module/type traversals repeated the recursion defect + +- **Symptom:** Roughly 1,000 nested path segments, use groups, inline modules, or type + wrappers raised `RecursionError` and killed analysis for healthy sibling files. +- **Root cause:** Four helpers used recursive descent even though `_references` already + demonstrated the safe iterative pattern; extraction also ran outside the file-level + exception boundary. +- **Detection:** Parameterize all four AST shapes above the Python recursion limit and + parse each beside a valid Rust file. +- **Fix:** Use explicit LIFO worklists, publish per-file extraction state transactionally, + and log-and-skip unexpected file-level failures. +- **Prevention:** Require the shared adversarial matrix and self-dogfood before parser PRs. +- First encountered: Rust parser PR #18 review, 2026-07-21. +- **Pattern note:** Second confirmed cross-language instance. Keep the class on the + design checklist; wait for a third instance before naming a broader meta-pattern. diff --git a/examples/workflows/arcade-agent-analysis.yml b/examples/workflows/arcade-agent-analysis.yml index 513d67e..73e03d4 100644 --- a/examples/workflows/arcade-agent-analysis.yml +++ b/examples/workflows/arcade-agent-analysis.yml @@ -8,7 +8,7 @@ on: # Copy this file to .github/workflows/arcade-agent-analysis.yml in any repository # that should run arcade-agent architecture analysis. env: - ARCADE_AGENT_VERSION: "0.1.1" + ARCADE_AGENT_VERSION: "0.2.0" PYTHON_VERSION: "3.12" INSTALL_EXTRAS: "[languages]" SOURCE_PATH: "." diff --git a/src/arcade_agent/cache.py b/src/arcade_agent/cache.py index cdcbcef..1fd973d 100644 --- a/src/arcade_agent/cache.py +++ b/src/arcade_agent/cache.py @@ -41,15 +41,41 @@ def cache_key(source_path: str, language: str | None, files: list[str] | None) - hasher.update((language or "auto").encode()) if files: - file_paths = sorted(files) + file_paths = set(files) else: # Hash all source-like files under root - file_paths = sorted(str(f) for f in root.rglob("*") if f.is_file() and f.suffix in { - ".java", ".py", ".c", ".cpp", ".h", ".hpp", ".ts", ".tsx", ".js", ".jsx", - ".go", ".kt", ".kts", ".rs", - }) - - for fp in file_paths: + file_paths = { + str(f) + for f in root.rglob("*") + if f.is_file() + and f.suffix + in { + ".java", + ".py", + ".c", + ".cpp", + ".h", + ".hpp", + ".ts", + ".tsx", + ".js", + ".jsx", + ".go", + ".kt", + ".kts", + ".rs", + } + } + + tracks_rust = language in {None, "rust"} or any( + Path(file_path).suffix == ".rs" for file_path in file_paths + ) + if tracks_rust: + file_paths.update( + str(manifest) for manifest in root.rglob("Cargo.toml") if manifest.is_file() + ) + + for fp in sorted(file_paths): p = Path(fp) hasher.update(fp.encode()) if p.exists(): diff --git a/src/arcade_agent/parsers/rust.py b/src/arcade_agent/parsers/rust.py index 240fe59..6743316 100644 --- a/src/arcade_agent/parsers/rust.py +++ b/src/arcade_agent/parsers/rust.py @@ -10,6 +10,7 @@ from __future__ import annotations +import logging import tomllib from dataclasses import dataclass from pathlib import Path @@ -22,6 +23,7 @@ from arcade_agent.parsers.graph import DependencyGraph, Edge, Entity RUST_LANGUAGE = Language(tsrust.language()) +logger = logging.getLogger(__name__) _MAX_FILE_BYTES = 1_000_000 _TYPE_ITEMS = { @@ -82,8 +84,11 @@ def _identifier(text: str) -> str: def _cargo_data(directory: Path) -> dict[str, Any]: manifest = directory / "Cargo.toml" try: - return tomllib.loads(manifest.read_text()) if manifest.is_file() else {} - except (OSError, tomllib.TOMLDecodeError): + if not manifest.is_file(): + return {} + with manifest.open("rb") as manifest_file: + return tomllib.load(manifest_file) + except (OSError, UnicodeDecodeError, tomllib.TOMLDecodeError): return {} @@ -132,69 +137,101 @@ def _path_segments(node: Node | None) -> tuple[str, ...]: """Extract ``a::b::Name`` segments from a path-like AST node.""" if node is None: return () - if node.type in { + + segments: list[str] = [] + stack = [node] + leaf_types = { "identifier", "type_identifier", "crate", "self", "super", "metavariable", - }: - return (_identifier(_get_text(node)),) - - path = node.child_by_field_name("path") - name = node.child_by_field_name("name") - if path is not None or name is not None: - return (*_path_segments(path), *_path_segments(name)) - - # use_wildcard has no named fields in tree-sitter-rust 0.24. - if node.type == "use_wildcard" and node.named_children: - return _path_segments(node.named_children[0]) - - # References through generic paths only need the base type name. - if node.type == "generic_type": - return _path_segments(node.child_by_field_name("type")) - return () + } + while stack: + current = stack.pop() + if current.type in leaf_types: + segments.append(_identifier(_get_text(current))) + continue + + path = current.child_by_field_name("path") + name = current.child_by_field_name("name") + if path is not None or name is not None: + # LIFO order: visit the path before its final name. + if name is not None: + stack.append(name) + if path is not None: + stack.append(path) + continue + + # use_wildcard has no named fields in tree-sitter-rust 0.24. + if current.type == "use_wildcard" and current.named_children: + stack.append(current.named_children[0]) + continue + + # References through generic paths only need the base type name. + if current.type == "generic_type": + generic_type = current.child_by_field_name("type") + if generic_type is not None: + stack.append(generic_type) + + return tuple(segments) def _flatten_use(node: Node, prefix: tuple[str, ...] = ()) -> list[_Import]: """Flatten a Rust use tree into concrete imports.""" - if node.type == "use_declaration": - argument = node.child_by_field_name("argument") - return _flatten_use(argument, prefix) if argument is not None else [] - - if node.type == "scoped_use_list": - path = (*prefix, *_path_segments(node.child_by_field_name("path"))) - use_list = node.child_by_field_name("list") - if use_list is None: - return [] - imports: list[_Import] = [] - for child in use_list.named_children: - if child.type == "self": - alias = path[-1] if path else "self" - imports.append(_Import(path=path, alias=alias)) - else: - imports.extend(_flatten_use(child, path)) - return imports - - if node.type == "use_list": - imports = [] - for child in node.named_children: - imports.extend(_flatten_use(child, prefix)) - return imports - - if node.type == "use_as_clause": - path = (*prefix, *_path_segments(node.child_by_field_name("path"))) - alias_node = node.child_by_field_name("alias") - alias = _identifier(_get_text(alias_node)) if alias_node is not None else path[-1] - return [_Import(path=path, alias=alias)] if path else [] - - if node.type == "use_wildcard": - path = (*prefix, *_path_segments(node)) - return [_Import(path=path, alias="*", wildcard=True)] if path else [] + imports: list[_Import] = [] + pending: list[tuple[Node, tuple[str, ...]]] = [(node, prefix)] + while pending: + current, current_prefix = pending.pop() + + if current.type == "use_declaration": + argument = current.child_by_field_name("argument") + if argument is not None: + pending.append((argument, current_prefix)) + continue + + if current.type == "scoped_use_list": + path = ( + *current_prefix, + *_path_segments(current.child_by_field_name("path")), + ) + use_list = current.child_by_field_name("list") + if use_list is not None: + pending.extend((child, path) for child in reversed(use_list.named_children)) + continue + + if current.type == "use_list": + pending.extend((child, current_prefix) for child in reversed(current.named_children)) + continue + + if current.type == "self" and current_prefix: + imports.append(_Import(path=current_prefix, alias=current_prefix[-1])) + continue + + if current.type == "use_as_clause": + path = ( + *current_prefix, + *_path_segments(current.child_by_field_name("path")), + ) + if not path: + continue + alias_node = current.child_by_field_name("alias") + alias = _identifier(_get_text(alias_node)) if alias_node is not None else path[-1] + imports.append(_Import(path=path, alias=alias)) + continue + + if current.type == "use_wildcard": + path = (*current_prefix, *_path_segments(current)) + if path: + imports.append(_Import(path=path, alias="*", wildcard=True)) + continue + + path = (*current_prefix, *_path_segments(current)) + if path: + imports.append(_Import(path=path, alias=path[-1])) - path = (*prefix, *_path_segments(node)) - return [_Import(path=path, alias=path[-1])] if path else [] + return imports def _extract_imports(container: Node) -> list[_Import]: @@ -205,6 +242,17 @@ def _extract_imports(container: Node) -> list[_Import]: return imports +def _is_cfg_test_attribute(node: Node) -> bool: + """Return whether an attribute is exactly ``#[cfg(test)]``.""" + if node.type != "attribute_item": + return False + return any( + "".join(_get_text(child).split()) == "cfg(test)" + for child in node.named_children + if child.type == "attribute" + ) + + def _references(node: Node) -> _References: simple: set[str] = set() qualified: set[tuple[str, ...]] = set() @@ -215,6 +263,14 @@ def _references(node: Node) -> _References: path = _path_segments(current) if len(path) > 1: qualified.add(path) + # The leading segment drives import-alias resolution. The + # immediate owner prefix preserves references such as + # ``Type::associated_item`` without recomputing every nested + # scoped path (which is quadratic for generated long paths). + simple.add(path[0]) + if len(path) > 2: + qualified.add(path[:-1]) + continue elif current.type in {"identifier", "type_identifier"}: simple.add(_identifier(_get_text(current))) stack.extend(current.named_children) @@ -225,18 +281,23 @@ def _base_type_path(node: Node | None) -> tuple[str, ...]: """Get the implemented type path without generic arguments.""" if node is None: return () - # Wrapper nodes can contain lifetimes before the actual type. Follow the - # grammar's explicit type field so ``&'a mut T`` resolves to T, not ``a``. - wrapped_type = node.child_by_field_name("type") - if node.type in {"reference_type", "pointer_type", "array_type", "slice_type"}: - return _base_type_path(wrapped_type) - direct = _path_segments(node) - if direct: - return direct - for child in node.named_children: - path = _base_type_path(child) - if path: - return path + + stack = [node] + wrapper_types = {"reference_type", "pointer_type", "array_type", "slice_type"} + while stack: + current = stack.pop() + # Wrapper nodes can contain lifetimes before the actual type. Follow + # the explicit type field so ``&'a mut T`` resolves to T, not ``a``. + if current.type in wrapper_types: + wrapped_type = current.child_by_field_name("type") + if wrapped_type is not None: + stack.append(wrapped_type) + continue + + direct = _path_segments(current) + if direct: + return direct + stack.extend(reversed(current.named_children)) return () @@ -360,136 +421,215 @@ def visit_container( crate: tuple[str, ...], is_file_root: bool = False, ) -> None: - imports = _extract_imports(container) - direct_entities = 0 - - for node in container.named_children: - if node.type in _TYPE_ITEMS: - name_node = node.child_by_field_name("name") - if name_node is None: + container_stack = [(container, module, is_file_root)] + while container_stack: + current_container, current_module, current_is_file_root = container_stack.pop() + imports = _extract_imports(current_container) + direct_entities = 0 + nested_containers: list[tuple[Node, tuple[str, ...], bool]] = [] + pending_attributes: list[Node] = [] + + for node in current_container.named_children: + if node.type == "attribute_item": + pending_attributes.append(node) continue - name = _identifier(_get_text(name_node)) - owner = add_entity( - name=name, - kind=_TYPE_ITEMS[node.type], - module=module, - rel_path=rel_path, - imports=imports, - node=node, - crate=crate, + + is_cfg_test = any( + _is_cfg_test_attribute(attribute) for attribute in pending_attributes ) - direct_entities += 1 - if node.type == "trait_item": - bounds = node.child_by_field_name("bounds") - if bounds is not None: - for bound in bounds.named_children: - bound_path = _base_type_path(bound) - if bound_path: - pending_trait_bounds.append( - (owner, bound_path, module, crate, imports) - ) - body = node.child_by_field_name("body") - if body is not None: - for member in body.named_children: - if member.type not in {"function_item", "function_signature_item"}: - continue - method_name = member.child_by_field_name("name") - if method_name is None: - continue - add_entity( - name=_identifier(_get_text(method_name)), - kind="method", - module=module, - rel_path=rel_path, - imports=imports, - node=member, - crate=crate, - owner=owner, - fqn_override=f"{owner}.{_identifier(_get_text(method_name))}", - ) - elif node.type == "function_item": - name_node = node.child_by_field_name("name") - if name_node is not None: - add_entity( - name=_identifier(_get_text(name_node)), - kind="function", - module=module, + pending_attributes.clear() + + if node.type in _TYPE_ITEMS: + name_node = node.child_by_field_name("name") + if name_node is None: + continue + name = _identifier(_get_text(name_node)) + owner = add_entity( + name=name, + kind=_TYPE_ITEMS[node.type], + module=current_module, rel_path=rel_path, imports=imports, node=node, crate=crate, ) direct_entities += 1 - elif node.type == "impl_item": - type_node = node.child_by_field_name("type") - trait_node = node.child_by_field_name("trait") - body = node.child_by_field_name("body") - owner_path = _base_type_path(type_node) - if not owner_path or body is None: - continue - methods: list[_PendingMethod] = [] - for member in body.named_children: - if member.type != "function_item": - continue - method_name = member.child_by_field_name("name") - if method_name is None: + if node.type == "trait_item": + bounds = node.child_by_field_name("bounds") + if bounds is not None: + for bound in bounds.named_children: + bound_path = _base_type_path(bound) + if bound_path: + pending_trait_bounds.append( + ( + owner, + bound_path, + current_module, + crate, + imports, + ) + ) + body = node.child_by_field_name("body") + if body is not None: + for member in body.named_children: + if member.type not in { + "function_item", + "function_signature_item", + }: + continue + method_name = member.child_by_field_name("name") + if method_name is None: + continue + normalized_name = _identifier(_get_text(method_name)) + add_entity( + name=normalized_name, + kind="method", + module=current_module, + rel_path=rel_path, + imports=imports, + node=member, + crate=crate, + owner=owner, + fqn_override=f"{owner}.{normalized_name}", + ) + elif node.type == "function_item": + name_node = node.child_by_field_name("name") + if name_node is not None: + add_entity( + name=_identifier(_get_text(name_node)), + kind="function", + module=current_module, + rel_path=rel_path, + imports=imports, + node=node, + crate=crate, + ) + direct_entities += 1 + elif node.type == "impl_item": + type_node = node.child_by_field_name("type") + trait_node = node.child_by_field_name("trait") + body = node.child_by_field_name("body") + owner_path = _base_type_path(type_node) + if not owner_path or body is None: continue - name = _identifier(_get_text(method_name)) - methods.append(_PendingMethod(name=name, references=_references(member))) - pending_impls.append( - _PendingImpl( - owner_path=owner_path, - trait_path=_base_type_path(trait_node), - generic_parameters=_generic_type_parameters(node), - module=module, - crate=crate, - imports=imports, - rel_path=rel_path, - methods=methods, + methods: list[_PendingMethod] = [] + for member in body.named_children: + if member.type != "function_item": + continue + method_name = member.child_by_field_name("name") + if method_name is None: + continue + name = _identifier(_get_text(method_name)) + methods.append( + _PendingMethod( + name=name, + references=_references(member), + ) + ) + pending_impls.append( + _PendingImpl( + owner_path=owner_path, + trait_path=_base_type_path(trait_node), + generic_parameters=_generic_type_parameters(node), + module=current_module, + crate=crate, + imports=imports, + rel_path=rel_path, + methods=methods, + ) ) + elif node.type == "mod_item" and not is_cfg_test: + name_node = node.child_by_field_name("name") + body = node.child_by_field_name("body") + if name_node is not None and body is not None: + child_module = ( + *current_module, + _identifier(_get_text(name_node)), + ) + nested_containers.append((body, child_module, False)) + + container_stack.extend(reversed(nested_containers)) + if current_is_file_root and direct_entities == 0: + module_name = ".".join(current_module) + name = current_module[-1] if current_module else file_stem + fqn = module_name or file_stem + add_entity( + name=name, + kind="module", + module=current_module[:-1] if current_module else (), + rel_path=rel_path, + imports=imports, + node=current_container, + crate=crate, + fqn_override=fqn, ) - elif node.type == "mod_item": - name_node = node.child_by_field_name("name") - body = node.child_by_field_name("body") - if name_node is not None and body is not None: - child_module = (*module, _identifier(_get_text(name_node))) - visit_container(body, child_module, rel_path, file_stem, crate) - - if is_file_root and direct_entities == 0: - module_name = ".".join(module) - name = module[-1] if module else file_stem - fqn = module_name or file_stem - add_entity( - name=name, - kind="module", - module=module[:-1] if module else (), - rel_path=rel_path, - imports=imports, - node=container, - crate=crate, - fqn_override=fqn, - ) + + all_entities: dict[str, Entity] = {} + all_packages: dict[str, list[str]] = {} + all_entity_refs: dict[str, _References] = {} + all_entity_imports: dict[str, list[_Import]] = {} + all_entity_crates: dict[str, tuple[str, ...]] = {} + all_pending_impls: list[_PendingImpl] = [] + all_pending_trait_bounds: list[ + tuple[str, tuple[str, ...], tuple[str, ...], tuple[str, ...], list[_Import]] + ] = [] for source_file in files: - source_file = source_file.resolve() + # Extract each file transactionally. If a malformed or adversarial + # file trips an unexpected parser edge case, discard its partial + # state and preserve entities from healthy sibling files. + entities = {} + packages = {} + entity_refs = {} + entity_imports = {} + entity_crates = {} + pending_impls = [] + pending_trait_bounds = [] try: - source_file.relative_to(root) + source_file = source_file.resolve() + rel_path = str(source_file.relative_to(root)) if source_file.stat().st_size > _MAX_FILE_BYTES: + logger.warning( + "Skipping Rust source larger than %d bytes: %s", + _MAX_FILE_BYTES, + source_file, + ) continue tree = parser.parse(source_file.read_bytes()) - except (OSError, ValueError): + source_root, crate = _crate_context(source_file, root, is_workspace) + visit_container( + tree.root_node, + _module_name(source_file, source_root, crate), + rel_path, + source_file.stem, + crate, + is_file_root=True, + ) + except Exception as error: + logger.warning( + "Skipping Rust source after extraction failure (%s): %s", + type(error).__name__, + source_file, + ) continue - rel_path = str(source_file.relative_to(root)) - source_root, crate = _crate_context(source_file, root, is_workspace) - visit_container( - tree.root_node, - _module_name(source_file, source_root, crate), - rel_path, - source_file.stem, - crate, - is_file_root=True, - ) + all_entities.update(entities) + all_entity_refs.update(entity_refs) + all_entity_imports.update(entity_imports) + all_entity_crates.update(entity_crates) + all_pending_impls.extend(pending_impls) + all_pending_trait_bounds.extend(pending_trait_bounds) + for package, fqns in packages.items(): + package_entities = all_packages.setdefault(package, []) + package_entities.extend(fqn for fqn in fqns if fqn not in package_entities) + + entities = all_entities + packages = all_packages + entity_refs = all_entity_refs + entity_imports = all_entity_imports + entity_crates = all_entity_crates + pending_impls = all_pending_impls + pending_trait_bounds = all_pending_trait_bounds non_member_entities = [e for e in entities.values() if e.kind != "method"] by_module_name = {(e.package, e.name): e.fqn for e in non_member_entities} diff --git a/src/arcade_agent/tools/ingest.py b/src/arcade_agent/tools/ingest.py index e312d40..a6c6bd8 100644 --- a/src/arcade_agent/tools/ingest.py +++ b/src/arcade_agent/tools/ingest.py @@ -112,9 +112,11 @@ def _detect_source_root(path: Path, language: str | None = None) -> Path: if language == "rust": manifest = path / "Cargo.toml" try: - if manifest.is_file() and "workspace" in tomllib.loads(manifest.read_text()): - return path - except (OSError, tomllib.TOMLDecodeError): + if manifest.is_file(): + with manifest.open("rb") as manifest_file: + if "workspace" in tomllib.load(manifest_file): + return path + except (OSError, UnicodeDecodeError, tomllib.TOMLDecodeError): pass for candidate in _SOURCE_ROOTS: diff --git a/tests/test_cache.py b/tests/test_cache.py index 1d168a7..d4acc61 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -69,6 +69,22 @@ def test_cache_key_tracks_rust_source_files(tmp_project): assert k1 != k2 +def test_cache_key_tracks_cargo_manifests_with_explicit_rust_files(tmp_project): + rust_file = tmp_project / "src" / "lib.rs" + rust_file.write_text("pub struct App;") + manifest = tmp_project / "Cargo.toml" + manifest.write_text('[package]\nname = "before"\n') + files = [str(rust_file)] + k1 = cache_key(str(tmp_project), "rust", files) + + manifest.write_text('[package]\nname = "after"\n') + newer = manifest.stat().st_mtime + 2 + os.utime(manifest, (newer, newer)) + k2 = cache_key(str(tmp_project), "rust", files) + + assert k1 != k2 + + def test_cache_miss_returns_none(tmp_project): result = get_cached_graph(str(tmp_project), "nonexistent_key") assert result is None diff --git a/tests/test_parsers/test_rust.py b/tests/test_parsers/test_rust.py index ef64518..6a9566c 100644 --- a/tests/test_parsers/test_rust.py +++ b/tests/test_parsers/test_rust.py @@ -3,6 +3,7 @@ import pytest pytest.importorskip("tree_sitter_rust") +import arcade_agent.parsers.rust as rust_parser # noqa: E402 from arcade_agent.parsers.rust import RustParser # noqa: E402 from arcade_agent.tools.ingest import ingest # noqa: E402 from arcade_agent.tools.parse import parse # noqa: E402 @@ -121,6 +122,77 @@ def test_rust_parser_handles_inline_modules_and_empty_input(tmp_path): assert empty.num_edges == 0 +@pytest.mark.parametrize( + "poisoned_source", + [ + "pub type Poison = " + "::".join(["a"] * 1_200 + ["T"]) + ";", + "use " + "a::{" * 1_200 + "T" + "}" * 1_200 + ";", + "mod nested {" * 1_200 + "pub struct Deep;" + "}" * 1_200, + "struct R; trait Marker {} impl Marker for " + "&" * 1_200 + "R {}", + ], + ids=["qualified-path", "nested-use", "inline-modules", "wrapped-type"], +) +def test_rust_parser_handles_deep_ast_without_losing_sibling_files(tmp_path, poisoned_source): + """Machine-generated nesting in one file must not abort full analysis.""" + poisoned = tmp_path / "poisoned.rs" + poisoned.write_text(poisoned_source) + valid = tmp_path / "valid.rs" + valid.write_text("pub struct Survives;\n") + + graph = RustParser().parse([poisoned, valid], tmp_path) + assert "valid.Survives" in graph.entities + + +def test_rust_parser_discards_partial_state_when_one_file_fails(tmp_path, monkeypatch): + poisoned = tmp_path / "poisoned.rs" + poisoned.write_text("pub struct Poisoned;\n") + valid = tmp_path / "valid.rs" + valid.write_text("pub struct Survives;\n") + original_extract_imports = rust_parser._extract_imports + + def fail_for_poisoned_file(container): + if b"Poisoned" in container.text: + raise RuntimeError("synthetic extraction failure") + return original_extract_imports(container) + + monkeypatch.setattr(rust_parser, "_extract_imports", fail_for_poisoned_file) + graph = RustParser().parse([poisoned, valid], tmp_path) + + assert "poisoned.Poisoned" not in graph.entities + assert "valid.Survives" in graph.entities + + +def test_rust_parser_skips_cfg_test_inline_modules(tmp_path): + source = tmp_path / "lib.rs" + source.write_text( + "pub struct Production;\n" + "#[cfg(test)]\n" + "mod tests {\n" + " struct Fixture;\n" + " fn helper() {}\n" + "}\n" + "#[cfg(not(test))]\n" + "mod runtime { pub struct Included; }\n" + ) + + graph = RustParser().parse([source], tmp_path) + assert "Production" in graph.entities + assert "runtime.Included" in graph.entities + assert all(not fqn.startswith("tests") for fqn in graph.entities) + + +def test_rust_parser_tolerates_invalid_cargo_manifest_encoding(tmp_path): + (tmp_path / "Cargo.toml").write_bytes(b"\xff\xfe") + source = tmp_path / "lib.rs" + source.write_text("pub struct StillParsed;\n") + + repo = ingest(str(tmp_path), language="rust") + graph = RustParser().parse([source], tmp_path) + + assert repo.source_files == [source] + assert "StillParsed" in graph.entities + + def test_rust_parser_handles_ripgrep_impl_owner_regressions(tmp_path): # Reduced from ripgrep 227381d: sibling impls from globset/serde_impl.rs, # external owners from matcher/src/lib.rs, and reference blanket impls from From 86fb591a97c8606fa34d2ec0dda3f2c64f2dfae4 Mon Sep 17 00:00:00 2001 From: Tony Nguyen Date: Tue, 21 Jul 2026 08:37:48 -0500 Subject: [PATCH 3/4] fix(recovery): resolve Tools concern overload --- src/arcade_agent/algorithms/concern.py | 10 ++- src/arcade_agent/tools/recover.py | 22 +++-- tests/test_tools/test_recover.py | 117 ++++++++++++++++++++++++- 3 files changed, 138 insertions(+), 11 deletions(-) diff --git a/src/arcade_agent/algorithms/concern.py b/src/arcade_agent/algorithms/concern.py index 90e00fe..326455c 100644 --- a/src/arcade_agent/algorithms/concern.py +++ b/src/arcade_agent/algorithms/concern.py @@ -15,13 +15,17 @@ log = logging.getLogger(__name__) +CONCERN_OVERLOAD_ENTITY_THRESHOLD = 20 +CONCERN_OVERLOAD_HIGH_ENTITY_THRESHOLD = 40 +CONCERN_OVERLOAD_MIN_INTERNAL_EDGES_PER_ENTITY = 0.4 + def detect_concern_overload( architecture: Architecture, dep_graph: DependencyGraph, - threshold: int = 20, - high_threshold: int = 40, - min_internal_edges_per_entity: float = 0.4, + threshold: int = CONCERN_OVERLOAD_ENTITY_THRESHOLD, + high_threshold: int = CONCERN_OVERLOAD_HIGH_ENTITY_THRESHOLD, + min_internal_edges_per_entity: float = CONCERN_OVERLOAD_MIN_INTERNAL_EDGES_PER_ENTITY, ) -> list[dict]: """Detect components with too many responsibilities. diff --git a/src/arcade_agent/tools/recover.py b/src/arcade_agent/tools/recover.py index f2c3c26..8efe73e 100644 --- a/src/arcade_agent/tools/recover.py +++ b/src/arcade_agent/tools/recover.py @@ -4,6 +4,7 @@ from arcade_agent.algorithms.arc import arc from arcade_agent.algorithms.architecture import Architecture, Component from arcade_agent.algorithms.clustering import wca +from arcade_agent.algorithms.concern import CONCERN_OVERLOAD_ENTITY_THRESHOLD from arcade_agent.algorithms.limbo import limbo from arcade_agent.parsers.graph import DependencyGraph from arcade_agent.tools.registry import tool @@ -86,8 +87,11 @@ def _refine_facade_groups( Package-only grouping can overstate architectural coupling when a package mainly exposes adapter functions that delegate straight into one subsystem. This refinement moves an entity only when it has no ties to peers in its - current group and all of its known dependencies point outward to one other - group. Entities without dependencies or with mixed responsibilities stay put. + current group and all of its known outgoing dependencies point outward to + one other group. For an oversized package bucket, incoming callers from + other groups do not define a facade's responsibility and therefore do not + pin it to the bucket. Compact public boundaries remain package-anchored. + Entities without dependencies or with mixed responsibilities stay put. """ membership = _entity_group_membership(groups) utility_hubs = _local_utility_hubs(dep_graph, membership) @@ -112,7 +116,7 @@ def _refine_facade_groups( incoming_sources = incoming_by_entity.get(entity_fqn, []) own_group_links = 0 target_groups: set[str] = set() - disqualify = False + has_external_callers = False for neighbor in outgoing_targets: neighbor_group = membership.get(neighbor) @@ -134,10 +138,14 @@ def _refine_facade_groups( continue own_group_links += 1 else: - disqualify = True - break - - if disqualify or own_group_links > 0 or len(target_groups) != 1: + has_external_callers = True + + is_oversized_bucket = len(groups[own_group]) > CONCERN_OVERLOAD_ENTITY_THRESHOLD + if ( + own_group_links > 0 + or len(target_groups) != 1 + or (has_external_callers and not is_oversized_bucket) + ): continue moves[entity_fqn] = next(iter(target_groups)) diff --git a/tests/test_tools/test_recover.py b/tests/test_tools/test_recover.py index 041cc51..ef4cf82 100644 --- a/tests/test_tools/test_recover.py +++ b/tests/test_tools/test_recover.py @@ -91,7 +91,7 @@ def test_package_based_recovery_reassigns_thin_facades(): source="com.example.api.facade", target="com.example.impl.worker", relation="import", - ) + ), ], packages={ "com.example.api": [ @@ -115,6 +115,121 @@ def test_package_based_recovery_reassigns_thin_facades(): assert "facade refinement" in arch.rationale +def test_package_recovery_reassigns_called_facade_from_oversized_bucket(): + api_entities = { + f"com.example.api.peer{i}": Entity( + fqn=f"com.example.api.peer{i}", + name=f"peer{i}", + package="com.example.api", + file_path=f"peer{i}.py", + kind="function", + language="python", + ) + for i in range(20) + } + facade = Entity( + fqn="com.example.api.facade", + name="facade", + package="com.example.api", + file_path="facade.py", + kind="function", + language="python", + ) + worker = Entity( + fqn="com.example.impl.worker", + name="worker", + package="com.example.impl", + file_path="worker.py", + kind="function", + language="python", + ) + caller = Entity( + fqn="com.example.cli.command", + name="command", + package="com.example.cli", + file_path="command.py", + kind="function", + language="python", + ) + graph = DependencyGraph( + entities={ + **api_entities, + facade.fqn: facade, + worker.fqn: worker, + caller.fqn: caller, + }, + edges=[ + Edge(source=facade.fqn, target=worker.fqn, relation="import"), + Edge(source=caller.fqn, target=facade.fqn, relation="import"), + ], + packages={ + "com.example.api": [*api_entities, facade.fqn], + "com.example.impl": [worker.fqn], + "com.example.cli": [caller.fqn], + }, + ) + + arch = recover(graph, algorithm="pkg") + membership = { + entity_fqn: component.name + for component in arch.components + for entity_fqn in component.entities + } + + assert membership[facade.fqn] == membership[worker.fqn] + assert membership[caller.fqn] != membership[worker.fqn] + assert {membership[fqn] for fqn in api_entities} == {"Api"} + + +def test_package_recovery_keeps_called_facade_in_compact_boundary(): + entities = { + fqn: Entity( + fqn=fqn, + name=fqn.rsplit(".", 1)[-1], + package=package, + file_path=f"{fqn.rsplit('.', 1)[-1]}.py", + kind="function", + language="python", + ) + for fqn, package in { + "com.example.api.facade": "com.example.api", + "com.example.api.peer": "com.example.api", + "com.example.impl.worker": "com.example.impl", + "com.example.cli.command": "com.example.cli", + }.items() + } + graph = DependencyGraph( + entities=entities, + edges=[ + Edge( + source="com.example.api.facade", + target="com.example.impl.worker", + relation="import", + ), + Edge( + source="com.example.cli.command", + target="com.example.api.facade", + relation="import", + ), + ], + packages={ + "com.example.api": ["com.example.api.facade", "com.example.api.peer"], + "com.example.impl": ["com.example.impl.worker"], + "com.example.cli": ["com.example.cli.command"], + }, + ) + + arch = recover(graph, algorithm="pkg") + membership = { + entity_fqn: component.name + for component in arch.components + for entity_fqn in component.entities + } + + assert membership["com.example.api.facade"] == "Api" + assert membership["com.example.api.facade"] != membership["com.example.impl.worker"] + + def test_unknown_algorithm(sample_graph): with pytest.raises(ValueError, match="Unknown algorithm"): recover(sample_graph, algorithm="unknown") From cbea67fd290ae52385a4e3745f9ccfc7e04ddbc4 Mon Sep 17 00:00:00 2001 From: Duc Le Date: Wed, 22 Jul 2026 13:02:38 +0700 Subject: [PATCH 4/4] fix(parser): gate Rust cfg(test) skip on exclude_tests, drop file cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-Authored-By: Claude Fable 5 --- README.md | 5 +- docs/BUG_CATALOG.md | 3 ++ src/arcade_agent/cache.py | 11 +++- src/arcade_agent/parsers/base.py | 12 +++++ src/arcade_agent/parsers/rust.py | 28 ++++++---- src/arcade_agent/tools/analyze.py | 1 + src/arcade_agent/tools/parse.py | 9 +++- tests/test_cache.py | 7 +++ tests/test_parsers/test_rust.py | 86 ++++++++++++++++++++++++++++--- 9 files changed, 140 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index df95196..74dfba3 100644 --- a/README.md +++ b/README.md @@ -132,8 +132,9 @@ smell burden, or another architectural pressure. - Kotlin (structural support via optional `[languages]` extra; import + inheritance graph) - Rust (structural support for structs, enums, unions, traits, type aliases, functions, methods, imports, qualified references, trait inheritance/implementations, and Cargo - workspaces). Inline `#[cfg(test)]` modules are excluded from production graphs; - files larger than 1 MB are skipped with a warning. + workspaces). Rust unit tests live inline, so with `exclude_tests=True` (the + default) `#[cfg(test)]` modules are left out of the graph as well; pass + `exclude_tests=False` to `ingest`/`parse`/`analyze` to keep them. Tree-sitter parser changes follow the [adversarial hardening workflow](.github/skills/harden-tree-sitter-parsers/SKILL.md). diff --git a/docs/BUG_CATALOG.md b/docs/BUG_CATALOG.md index 550fc70..36f4f5e 100644 --- a/docs/BUG_CATALOG.md +++ b/docs/BUG_CATALOG.md @@ -14,6 +14,9 @@ prevention rules so the same defect is not rediscovered language by language. - Test nesting deeper than `sys.getrecursionlimit()` for every traversal shape. - Pair each poisoned input with a healthy sibling file and assert the sibling survives. - Track non-source inputs such as manifests when they affect graph identity or cache keys. +- Do not add per-parser input caps (file size, node counts) as a stand-in for robustness: + they drop real code silently, diverge from the other parsers, and never fix the + traversal defect they appear to mitigate. The per-file exception boundary is the backstop. - Run focused tests, the full suite, a large real repository, and arcade-agent's own self-analysis before publishing parser changes. diff --git a/src/arcade_agent/cache.py b/src/arcade_agent/cache.py index 1fd973d..44fd5d4 100644 --- a/src/arcade_agent/cache.py +++ b/src/arcade_agent/cache.py @@ -20,7 +20,12 @@ def _cache_dir(project_root: Path) -> Path: return project_root / _CACHE_DIR -def cache_key(source_path: str, language: str | None, files: list[str] | None) -> str: +def cache_key( + source_path: str, + language: str | None, + files: list[str] | None, + exclude_tests: bool = True, +) -> str: """Compute a cache key from source path, language, and file mtimes. The key is a SHA-256 hash of the sorted file paths and their modification @@ -31,6 +36,9 @@ def cache_key(source_path: str, language: str | None, files: list[str] | None) - source_path: Root directory of the project. language: Language being parsed (or None for auto-detect). files: Specific files to parse, or None to discover all. + exclude_tests: Whether inline test code is excluded. Parsers that honor + it (Rust) produce a different graph for the same files, so it must + take part in the key. Returns: A hex digest string usable as a cache filename. @@ -39,6 +47,7 @@ def cache_key(source_path: str, language: str | None, files: list[str] | None) - hasher = hashlib.sha256() hasher.update(str(root).encode()) hasher.update((language or "auto").encode()) + hasher.update(b"tests:excluded" if exclude_tests else b"tests:included") if files: file_paths = set(files) diff --git a/src/arcade_agent/parsers/base.py b/src/arcade_agent/parsers/base.py index 056e55e..6b7016b 100644 --- a/src/arcade_agent/parsers/base.py +++ b/src/arcade_agent/parsers/base.py @@ -9,6 +9,18 @@ class LanguageParser(ABC): """Abstract base class for language-specific parsers.""" + def __init__(self, exclude_tests: bool = True) -> None: + """Create a parser. + + Args: + exclude_tests: Whether test code should be kept out of the graph. + File-level exclusion happens in ``ingest``; this flag lets a + parser additionally drop *inline* test constructs that live in + production files (e.g. Rust's ``#[cfg(test)] mod tests``). + Parsers for languages without inline tests ignore it. + """ + self.exclude_tests = exclude_tests + @property @abstractmethod def language(self) -> str: diff --git a/src/arcade_agent/parsers/rust.py b/src/arcade_agent/parsers/rust.py index 6743316..3f27a2a 100644 --- a/src/arcade_agent/parsers/rust.py +++ b/src/arcade_agent/parsers/rust.py @@ -6,6 +6,12 @@ resolves ``use`` declarations, same-module references, qualified paths, and trait inheritance/implementations into dependency edges. Cargo workspaces are kept intact and each member crate gets a stable graph prefix. + +Rust unit tests are conventionally written inline as ``#[cfg(test)] mod tests`` +inside the production file, so path-based test exclusion never sees them. When +``exclude_tests`` is set (the default), those modules and everything below them +are left out of the graph; with ``exclude_tests=False`` they are extracted like +any other module. """ from __future__ import annotations @@ -25,7 +31,6 @@ RUST_LANGUAGE = Language(tsrust.language()) logger = logging.getLogger(__name__) -_MAX_FILE_BYTES = 1_000_000 _TYPE_ITEMS = { "struct_item": "struct", "enum_item": "enum", @@ -350,7 +355,11 @@ def _deduplicate(edges: list[Edge]) -> list[Edge]: @register_parser class RustParser(LanguageParser): - """Rust source code parser using tree-sitter.""" + """Rust source code parser using tree-sitter. + + Honors ``exclude_tests`` (default ``True``) by skipping inline + ``#[cfg(test)]`` modules, which path-based test exclusion cannot reach. + """ @property def language(self) -> str: @@ -538,7 +547,13 @@ def visit_container( methods=methods, ) ) - elif node.type == "mod_item" and not is_cfg_test: + elif node.type == "mod_item": + # Rust unit tests live inline, so path-based test + # exclusion in ``ingest`` cannot see them. Drop + # ``#[cfg(test)]`` modules only when the caller asked + # for test code to be excluded. + if is_cfg_test and self.exclude_tests: + continue name_node = node.child_by_field_name("name") body = node.child_by_field_name("body") if name_node is not None and body is not None: @@ -588,13 +603,6 @@ def visit_container( try: source_file = source_file.resolve() rel_path = str(source_file.relative_to(root)) - if source_file.stat().st_size > _MAX_FILE_BYTES: - logger.warning( - "Skipping Rust source larger than %d bytes: %s", - _MAX_FILE_BYTES, - source_file, - ) - continue tree = parser.parse(source_file.read_bytes()) source_root, crate = _crate_context(source_file, root, is_workspace) visit_container( diff --git a/src/arcade_agent/tools/analyze.py b/src/arcade_agent/tools/analyze.py index ffbe400..c23e4df 100644 --- a/src/arcade_agent/tools/analyze.py +++ b/src/arcade_agent/tools/analyze.py @@ -88,6 +88,7 @@ def _run_sync_pipeline( language=repository.language or language, files=[str(path) for path in repository.source_files], use_cache=use_cache, + exclude_tests=exclude_tests, ) if on_stage is not None: on_stage("graph", graph) diff --git a/src/arcade_agent/tools/parse.py b/src/arcade_agent/tools/parse.py index f9b3d23..342b687 100644 --- a/src/arcade_agent/tools/parse.py +++ b/src/arcade_agent/tools/parse.py @@ -23,6 +23,7 @@ def parse( language: str | None = None, files: list[str] | None = None, use_cache: bool = True, + exclude_tests: bool = True, ) -> DependencyGraph: """Parse source code and extract a dependency graph. @@ -31,6 +32,9 @@ def parse( language: Language to parse (java, python, etc.). Auto-detected if None. files: Specific files to parse. If None, discovers all files. use_cache: If True, return cached results when source files haven't changed. + exclude_tests: If True, parsers that recognize inline test constructs + (e.g. Rust's ``#[cfg(test)] mod tests``) leave them out of the graph. + Mirrors the `ingest` flag, which only excludes whole test *paths*. Returns: DependencyGraph with entities, edges, and package info. @@ -39,7 +43,7 @@ def parse( # Check cache before doing expensive parsing if use_cache: - key = cache_key(source_path, language, files) + key = cache_key(source_path, language, files, exclude_tests) cached = get_cached_graph(source_path, key) if cached is not None: return cached @@ -70,6 +74,7 @@ def parse( raise ValueError("No language specified and auto-detection failed") parser = get_parser(language) + parser.exclude_tests = exclude_tests # Two cache layers: the whole-graph cache above returns instantly when NOTHING # changed; when some files changed we fall here and parse incrementally — @@ -84,7 +89,7 @@ def parse( # Store in cache for next time if use_cache: - key = cache_key(source_path, language, files) + key = cache_key(source_path, language, files, exclude_tests) put_cached_graph(source_path, key, graph) return graph diff --git a/tests/test_cache.py b/tests/test_cache.py index d4acc61..166d10b 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -85,6 +85,13 @@ def test_cache_key_tracks_cargo_manifests_with_explicit_rust_files(tmp_project): assert k1 != k2 +def test_cache_key_changes_with_exclude_tests(tmp_project): + """Rust graphs differ by exclude_tests, so cached graphs must not collide.""" + k1 = cache_key(str(tmp_project), "rust", None) + k2 = cache_key(str(tmp_project), "rust", None, exclude_tests=False) + assert k1 != k2 + + def test_cache_miss_returns_none(tmp_project): result = get_cached_graph(str(tmp_project), "nonexistent_key") assert result is None diff --git a/tests/test_parsers/test_rust.py b/tests/test_parsers/test_rust.py index 6a9566c..a78a227 100644 --- a/tests/test_parsers/test_rust.py +++ b/tests/test_parsers/test_rust.py @@ -129,8 +129,19 @@ def test_rust_parser_handles_inline_modules_and_empty_input(tmp_path): "use " + "a::{" * 1_200 + "T" + "}" * 1_200 + ";", "mod nested {" * 1_200 + "pub struct Deep;" + "}" * 1_200, "struct R; trait Marker {} impl Marker for " + "&" * 1_200 + "R {}", + "struct R; trait Marker {} impl Marker for " + + "(" * 1_200 + + "R" + + ")" * 1_200 + + " {}", + ], + ids=[ + "qualified-path", + "nested-use", + "inline-modules", + "wrapped-type", + "parenthesized-type", ], - ids=["qualified-path", "nested-use", "inline-modules", "wrapped-type"], ) def test_rust_parser_handles_deep_ast_without_losing_sibling_files(tmp_path, poisoned_source): """Machine-generated nesting in one file must not abort full analysis.""" @@ -162,23 +173,84 @@ def fail_for_poisoned_file(container): assert "valid.Survives" in graph.entities +_CFG_TEST_SOURCE = ( + "pub struct Production;\n" + "#[cfg(test)]\n" + "mod tests {\n" + " struct Fixture;\n" + " fn helper() {}\n" + "}\n" + "#[cfg(not(test))]\n" + "mod runtime { pub struct Included; }\n" +) + + def test_rust_parser_skips_cfg_test_inline_modules(tmp_path): + source = tmp_path / "lib.rs" + source.write_text(_CFG_TEST_SOURCE) + + graph = RustParser().parse([source], tmp_path) + assert "Production" in graph.entities + assert "runtime.Included" in graph.entities + assert all(not fqn.startswith("tests") for fqn in graph.entities) + assert "tests" not in graph.packages + + +def test_rust_parser_keeps_cfg_test_modules_when_tests_are_not_excluded(tmp_path): + source = tmp_path / "lib.rs" + source.write_text(_CFG_TEST_SOURCE) + + graph = RustParser(exclude_tests=False).parse([source], tmp_path) + assert "Production" in graph.entities + assert "runtime.Included" in graph.entities + assert "tests.Fixture" in graph.entities + assert "tests.helper" in graph.entities + + +def test_rust_parser_skips_nested_items_under_cfg_test_module(tmp_path): + """Everything below a #[cfg(test)] module is dropped, not just its head.""" source = tmp_path / "lib.rs" source.write_text( "pub struct Production;\n" "#[cfg(test)]\n" "mod tests {\n" - " struct Fixture;\n" - " fn helper() {}\n" + " mod inner {\n" + " pub struct DeepFixture;\n" + " }\n" + " impl super::Production {\n" + " fn only_for_tests(&self) {}\n" + " }\n" "}\n" - "#[cfg(not(test))]\n" - "mod runtime { pub struct Included; }\n" ) graph = RustParser().parse([source], tmp_path) assert "Production" in graph.entities - assert "runtime.Included" in graph.entities - assert all(not fqn.startswith("tests") for fqn in graph.entities) + assert all("Fixture" not in fqn for fqn in graph.entities) + assert "Production.only_for_tests" not in graph.entities + + +def test_parse_tool_threads_exclude_tests_to_the_rust_parser(tmp_path): + source = tmp_path / "lib.rs" + source.write_text(_CFG_TEST_SOURCE) + + excluded = parse(str(tmp_path), language="rust", use_cache=False) + included = parse(str(tmp_path), language="rust", use_cache=False, exclude_tests=False) + + assert "tests.Fixture" not in excluded.entities + assert "tests.Fixture" in included.entities + + +def test_rust_parser_does_not_silently_drop_large_files(tmp_path): + """No parser caps input size; a >1 MB crate file must still be extracted.""" + # Bulk is comments so the file crosses 1 MB without a huge entity count. + filler = "// {}\n".format("padding " * 12) * 12_000 + source = tmp_path / "lib.rs" + source.write_text(f"pub struct Head;\n{filler}pub struct Tail;\n") + assert source.stat().st_size > 1_000_000 + + graph = RustParser().parse([source], tmp_path) + assert "Head" in graph.entities + assert "Tail" in graph.entities def test_rust_parser_tolerates_invalid_cargo_manifest_encoding(tmp_path):