get_dependencies (direction=in): crate-scope reverse-dep matching to cut generic-name false positives #9

New issue

Closed

opened 2026-07-07 12:43:53 +02:00 by buildagent · 1 comment

buildagent commented

2026-07-07 12:43:53 +02:00

Member

Summary

get_dependencies with direction=in (reverse dependencies / "who imports this file") over-reports when the target file's module-name keys are generic and collide with same-named modules in other crates of the workspace. This is the known recall-over-precision tradeoff in segment matching; this issue tracks tightening it toward precision without losing recall.

Where

crates/daemon/src/local_index.rs — module_matches_keys() and importers_of_file()
Matching splits an importer's module path on [: . / \] and returns the file if any segment equals any of the target file's keys (file stem + declared module-symbol names).

Observed (dogfooded live on v0.5.8)

Target	`in` result	Assessment
`crates/plugins/src/lib.rs`	27 importers	Inflated. `lib.rs` declares `pub mod common; pub mod pos; …`. Matching then pulls in the daemon's `crates/daemon/tests/common/` importers (a different `common`) and the ruby fixture's `lib/greeter` (matches stem `lib`). Most are false positives.
`crates/mcp-server/src/gitdiff.rs`	2 importers	Correct — both real `use crate::gitdiff::…` sites in `server.rs`.
`crates/indexer/src/test_paths.rs`	0	Correct (used via fully-qualified inline `crate::test_paths::…`, no `use`).

Conclusion: noise scales with name genericness, not a regression. Distinctive names resolve precisely; generic ones (common, pos, lib, mod, utils) attract cross-crate collisions.

Why it's hard

The import string alone (common::{build_daemon}) doesn't encode which crate its common resolves to. Precise matching needs crate/scope context: crate::common in the plugins crate must NOT match an importer whose common lives in the daemon crate.

Proposed directions (pick one, smallest that works)

Crate-boundary scoping — resolve each import's crate context from the importer's path (nearest Cargo.toml / crate root) and require the target and importer to share a crate for crate::/super::/relative imports. Highest precision; most work.
Prefer-longer-key / full-path match — when the target file exposes a qualifiable path (e.g. crate::gitdiff), prefer matches on the longer qualified segment chain over single generic segments. Cheaper; helps the lib.rs case partially.
Confidence signal — keep current recall but tag each importer result with a confidence (e.g. lower for single generic-segment matches) so callers/UI can rank or filter. Least invasive; preserves recall.

Leaning toward (3) as an immediate mitigation + (1) as the correct long-term fix.

Acceptance

get_dependencies in on crates/plugins/src/lib.rs returns only true importers of that crate's modules (plugin-internal crate::common / crate::pos users), excluding the daemon-test common and the ruby fixture.
Distinctive-name behavior (gitdiff.rs → 2, test_paths.rs → 0) unchanged.
E2E coverage: a fixture with a generic module name duplicated across two crates, asserting no cross-crate leakage. Parity across all 6 languages / 3 OSes / 2 arches per project convention.

Context

Surfaced during the v0.5.8 deep dogfood pass. Not a defect — the current behavior is documented in the importers_of_file doc comment as an intentional recall bias. This issue is the tracked enhancement to move it toward precision.

## Summary `get_dependencies` with `direction=in` (reverse dependencies / "who imports this file") over-reports when the target file's module-name keys are **generic** and collide with same-named modules in *other* crates of the workspace. This is the known recall-over-precision tradeoff in segment matching; this issue tracks tightening it toward precision without losing recall. ## Where - `crates/daemon/src/local_index.rs` — `module_matches_keys()` and `importers_of_file()` - Matching splits an importer's module path on `[: . / \]` and returns the file if **any** segment equals **any** of the target file's keys (file stem + declared module-symbol names). ## Observed (dogfooded live on v0.5.8) | Target | `in` result | Assessment | |---|---|---| | `crates/plugins/src/lib.rs` | **27 importers** | Inflated. `lib.rs` declares `pub mod common; pub mod pos; …`. Matching then pulls in the **daemon's** `crates/daemon/tests/common/` importers (a *different* `common`) and the ruby fixture's `lib/greeter` (matches stem `lib`). Most are false positives. | | `crates/mcp-server/src/gitdiff.rs` | 2 importers | Correct — both real `use crate::gitdiff::…` sites in `server.rs`. | | `crates/indexer/src/test_paths.rs` | 0 | Correct (used via fully-qualified inline `crate::test_paths::…`, no `use`). | Conclusion: noise scales with **name genericness**, not a regression. Distinctive names resolve precisely; generic ones (`common`, `pos`, `lib`, `mod`, `utils`) attract cross-crate collisions. ## Why it's hard The import string alone (`common::{build_daemon}`) doesn't encode which crate its `common` resolves to. Precise matching needs crate/scope context: `crate::common` in the plugins crate must NOT match an importer whose `common` lives in the daemon crate. ## Proposed directions (pick one, smallest that works) 1. **Crate-boundary scoping** — resolve each import's crate context from the importer's path (nearest `Cargo.toml` / crate root) and require the target and importer to share a crate for `crate::`/`super::`/relative imports. Highest precision; most work. 2. **Prefer-longer-key / full-path match** — when the target file exposes a qualifiable path (e.g. `crate::gitdiff`), prefer matches on the longer qualified segment chain over single generic segments. Cheaper; helps the `lib.rs` case partially. 3. **Confidence signal** — keep current recall but tag each importer result with a `confidence` (e.g. lower for single generic-segment matches) so callers/UI can rank or filter. Least invasive; preserves recall. Leaning toward **(3) as an immediate mitigation** + **(1) as the correct long-term fix**. ## Acceptance - `get_dependencies in` on `crates/plugins/src/lib.rs` returns only true importers of *that crate's* modules (plugin-internal `crate::common` / `crate::pos` users), excluding the daemon-test `common` and the ruby fixture. - Distinctive-name behavior (`gitdiff.rs` → 2, `test_paths.rs` → 0) unchanged. - E2E coverage: a fixture with a generic module name duplicated across two crates, asserting no cross-crate leakage. Parity across all 6 languages / 3 OSes / 2 arches per project convention. ## Context Surfaced during the v0.5.8 deep dogfood pass. Not a defect — the current behavior is documented in the `importers_of_file` doc comment as an intentional recall bias. This issue is the tracked enhancement to move it toward precision.

buildagent added this to the v0.5.9 milestone

2026-07-07 12:45:23 +02:00

buildagent commented

2026-07-07 16:14:00 +02:00

Author

Member

Implemented on fix/ultradeep-review-findings (commit 12f942a) — a combination of directions (1) crate-scoping and (2) prefer-distinctive, without needing manifest parsing.

importers_of_file now scopes ambiguous matches to the target's own package. A key is ambiguous if it names a module defined in a different package (queried from the symbol table) or is a generic module-root stem (lib/mod/main/index/__init__/app). A match resting solely on ambiguous keys is kept only when the importer shares the target's package root (package_root, a lexical crate/pkg-root heuristic for crate/<name>/src, <pkg>/lib, and single-root src layouts). A match on a distinctive key (unique to the target's package, e.g. gitdiff) still resolves cross-package, preserving real cross-crate reverse-deps.

Acceptance:

The common cross-crate leak is closed: a two-crate fixture where both crates declare common asserts the daemon-package importer is dropped while the plugins-package sibling is kept.
Distinctive-name behavior preserved: gitdiff resolves cross-package (test), and the existing gitdiff.rs → 2 / test_paths.rs → 0 lang_e2e coverage stays green.
The lib-stem leak (ruby fixture) is closed by the generic-stem rule.

Honest scope note: this is precision-over-recall by design — an ambiguous-named module imported cross-crate via its (unqualifiable-from-the-string) crate name is conservatively dropped. That's the genuinely-ambiguous case the issue's "Why it's hard" section describes; full manifest-based crate resolution would be the only way to recover it, and is a larger follow-up. This removes the documented false positives. Closing.

Implemented on `fix/ultradeep-review-findings` (commit `12f942a`) — a combination of directions **(1) crate-scoping** and **(2) prefer-distinctive**, without needing manifest parsing. `importers_of_file` now scopes **ambiguous** matches to the target's own package. A key is ambiguous if it names a module defined in a *different* package (queried from the symbol table) or is a generic module-root stem (`lib`/`mod`/`main`/`index`/`__init__`/`app`). A match resting solely on ambiguous keys is kept only when the importer shares the target's package root (`package_root`, a lexical crate/pkg-root heuristic for `crate/<name>/src`, `<pkg>/lib`, and single-root `src` layouts). A match on a **distinctive** key (unique to the target's package, e.g. `gitdiff`) still resolves cross-package, preserving real cross-crate reverse-deps. **Acceptance:** - The `common` cross-crate leak is closed: a two-crate fixture where both crates declare `common` asserts the daemon-package importer is dropped while the plugins-package sibling is kept. - Distinctive-name behavior preserved: `gitdiff` resolves cross-package (test), and the existing `gitdiff.rs → 2` / `test_paths.rs → 0` lang_e2e coverage stays green. - The `lib`-stem leak (ruby fixture) is closed by the generic-stem rule. **Honest scope note:** this is precision-over-recall by design — an ambiguous-named module imported cross-crate via its (unqualifiable-from-the-string) crate name is conservatively dropped. That's the genuinely-ambiguous case the issue's "Why it's hard" section describes; full manifest-based crate resolution would be the only way to recover it, and is a larger follow-up. This removes the documented false positives. Closing.

buildagent closed this issue

2026-07-07 16:14:12 +02:00