Skip to content

fix(db): resolve ::-qualified call targets without breaking namespaced-name storage#148

Merged
jrollin merged 1 commit into
mainfrom
fix/resolver-colon-split-full-name-first
Jul 6, 2026
Merged

fix(db): resolve ::-qualified call targets without breaking namespaced-name storage#148
jrollin merged 1 commit into
mainfrom
fix/resolver-colon-split-full-name-first

Conversation

@jrollin

@jrollin jrollin commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Problem

The heuristic edge resolver reduced target_name to a simple_name by splitting on . (method chains) and \ (PHP namespaces) but not :: (Rust/C++ path separator). So Rust path calls like Pool::new, Config::load, Vec::new never matched any bare-stored callee symbol and failed every one of the 6 resolution tiers — the largest driver of Rust being the lowest-scored language on make bench-resolution (~45%).

Fix: full-name-first, ::-reduced fallback through locality tiers only

A naive "just split on :: too" is not all-language safe — Ruby stores namespaced symbols under their full name (class Foo::Bar → symbol Foo::Bar, ruby.rs extract_constant_name keeps the whole scope_resolution), so reducing an Inherits target Baz::QuuxQuux would miss it and regress every Ruby namespaced inheritance/reference. It also lets a fully-qualified external call std::mem::swap reduce to swap and false-resolve onto a lone project symbol.

So the resolver now:

  • full_name (split only on ./\, the prior behavior) is tried first — preserves qualified-name storage (Ruby Baz::Quux, PHP App\Auth\Foo).
  • reduced_name (further past ::, Some only when it differs) is tried second, through locality tiers 1–4 only. Those are file/dir/scope-guarded, so a fallback can only match a symbol already reachable from the caller — safe.
  • Global tier (5+6) uses full_name only: std::mem::swap stays unresolved rather than reducing to swap and fabricating a call edge.

Measurements (before → after, heuristic --no-lsp)

Lang before after Δ
Rust 777 (45.47%) 808 (47.28%) +31 (+1.81pp)
Ruby 388 388 0
Python / PHP / TS / Go / Java 0 (byte-identical)

Concrete: on the webapp_rs fixture, Config::load (10 call sites) resolved to nothing before (refs load → "No references found"); after, all 10 resolve. The gain is a correct +31, not a naive ::-split's inflated +68 — the 37-edge difference was exactly the false positives this design avoids. Index wall-clock unchanged (best-of-5 identical).

Regression-first tests

Each new test was verified against three resolver versions:

Test plain main naive ::-split (rejected) this fix
Ruby namespaced inherits ❌ regression
std::mem::swap unresolved ❌ false positive
Rust gains (Pool::new, nested ::, dot-then-colon, View::render) ❌ no gain

Review

Ran an xhigh workflow-backed code review on the first (naive) attempt; it caught the Ruby regression and the external-call false-positive, both independently reproduced and now fixed with guarding tests. Also addressed the review's test-quality findings (mislabeled tier, redundant internal-column read, a false-green ambiguity test).

Checks

  • cargo test --workspace (33 suites) ✅ — cartog-db 208 pass
  • cargo test -p cartog-db --no-default-features
  • cargo fmt --check, cargo clippy --all-targets [--no-default-features] -- -D warnings
  • cargo test --doc -p cartog-db, cargo doc --no-deps -p cartog-db (warning-clean) ✅

No extractor / ground-truth / docs / site change: stored target_name is untouched, and this is an internal resolver-quality fix (no new command/flag/config/MCP-tool/language/count).

🤖 Generated with Claude Code

https://claude.ai/code/session_01AhoiweoMbZQ6FQgAcrfiWu

Summary by CodeRabbit

  • Bug Fixes

    • Improved edge resolution for namespaced calls, making local matches more accurate when names include ::, . or path separators.
    • Reduced false positives for fully qualified external calls and ambiguous names, so unresolved references are handled more reliably.
    • Preserved correct precedence for nearby matches, such as same-file resolution over broader project-wide matches.
  • Tests

    • Added regression coverage for namespaced resolution, ambiguous matches, and locality-based tie-breaking.

…d-name storage

The heuristic resolver reduced target_name on `.`/`\` but not `::`, so Rust path
calls (`Pool::new`, `Config::load`) never matched a bare-stored callee and stayed
unresolved — the largest driver of Rust's low resolution rate.

Reduce past `::` too, but full-name-first with a guarded fallback:

- full_name (split only on `.`/`\`) is tried first. Ruby stores namespaced symbols
  under their full name (`class Foo::Bar` -> name `Foo::Bar`), so a naive `::`-split
  would reduce an Inherits target `Baz::Quux` to `Quux` and miss it — a regression.
- reduced_name (past `::`) is tried second, through the locality tiers (1-4) only.
  Those are file/dir/scope-guarded, so a fallback can only match a symbol already
  reachable from the caller.
- The global tier (5+6) uses full_name only: a fully-qualified external call like
  `std::mem::swap` stays unresolved instead of reducing to `swap` and false-resolving
  onto a lone project symbol.

Rust Calls resolution 45.47% -> 47.28% (+31 edges) on the webapp_rs fixture; Ruby,
Python, PHP, TS, Go, Java byte-identical. Index wall-clock unchanged.

Adds regression tests for the Rust gain, the Ruby full-name path, and the external-
call guard; each fails on the naive `::`-split and passes here.
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 25686ab7-19fc-4015-9db3-fb5d0f55f636

📥 Commits

Reviewing files that changed from the base of the PR and between caf64ab and 6828077.

📒 Files selected for processing (2)
  • crates/cartog-db/src/store/resolution.rs
  • crates/cartog-db/src/tests/resolution.rs

📝 Walkthrough

Walkthrough

The edge resolver in resolve_edge_batch was refactored to compute both a full_name and a reduced_name (split on ::) for target matching. Locality tiers 1-4 try full_name then fall back to reduced_name; global tiers 5-6 use full_name only. New regression tests validate these matching behaviors and provenance outcomes.

Changes

Edge Resolver Name Reduction

Layer / File(s) Summary
Locality tier candidate resolution
crates/cartog-db/src/store/resolution.rs
Replaces single simple_name with full_name/reduced_name computation and an ordered candidate loop for tiers 1-4, building an Option<(id, provenance)> instead of per-tier continue chains.
Global tier resolution restricted to full_name
crates/cartog-db/src/store/resolution.rs
Updates tiers 5-6 to search project-wide using full_name only, avoiding reduced_name to prevent false positives from fully-qualified external calls.
Regression tests for name reduction and provenance
crates/cartog-db/src/tests/resolution.rs
Adds a call_edge_target helper and tests covering same-file reduced-name resolution, global full-name resolution, ambiguous/false-positive cases, and mixed ./:: name forms.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • jrollin/cartog#86: Also modifies resolve_edge_batch's tier-selection/update logic in the same file, extending resolution updates for provenance/resolution_source.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: resolving ::-qualified call targets while preserving namespaced-name storage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/resolver-colon-split-full-name-first

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.38%. Comparing base (caf64ab) to head (6828077).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #148   +/-   ##
=======================================
  Coverage   88.38%   88.38%           
=======================================
  Files          96       96           
  Lines       32574    32573    -1     
=======================================
  Hits        28791    28791           
+ Misses       3783     3782    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jrollin jrollin merged commit 69e1280 into main Jul 6, 2026
14 checks passed
@jrollin jrollin deleted the fix/resolver-colon-split-full-name-first branch July 6, 2026 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant