fix(graph): correct symbol resolution, centrality, and edge recall (v2.6.0) (#163)

* fix(core): stop indexing prose as code

Found by running the CLI over honojs/hono.

Markdown was in the code-graph extension list "for knowledge graphs
(Lattice)", so a `# fetch benchmark` heading in a README became a graph node
named `fetch` — and then competed with the real `fetch` definitions when
resolving TypeScript call sites. Markdown is still parseable, just no longer
part of the default code index.

Shell comments became functions. `parse_shell_line` looks for `()` anywhere
on the line, so `# Compare app.fetch() between the working tree and a git
ref` produced a function named "# Compare app.fetch". The comment guard ran
*after* parsing and only skipped a line when nothing had matched, so any
comment that happened to parse was kept. Comments are now skipped before
parsing, and shell function names must be bare identifiers.

Together these were 59 phantom nodes on hono — every one of them a false
candidate during symbol resolution.

* fix(graph): stop warning about routine resolution outcomes

`arbor setup` on hono printed dozens of WARN lines before its success
message — every ambiguous method name and every unimported cross-module
reference. Names like `.get`, `.encode`, and `.match` are shared by many
types in any real codebase, and most references are stdlib or third-party
with no definition to find. None of it is a user problem.

These are diagnostics for someone debugging resolution, so they move to
`debug!`. A first run should end with a clean success line, not a wall of
warnings about normal behaviour.

* fix: pick the right definition when a symbol name is ambiguous

`arbor inspect getPath` on hono reported "Role: unreachable, Callers: 0" and
suggested the symbol might be dead code. `getPath` is one of hono's hottest
utilities, called from ten places including the line directly below it.

The graph was right; the lookup was not. `getPath` names five nodes — a
function in `utils/url.ts` and methods on four AWS Lambda event processors —
and every caller resolved it with `find_by_name(..).first()`, i.e. whichever
file happened to be parsed first. The CLI and the MCP bridge each had their
own copy of that mistake, eleven call sites in total.

`ArborGraph::resolve_symbol_ranked` becomes the single resolution policy:
rank by graph degree, then centrality, then file path, so the pick is both
meaningful and independent of parse order. The CLI additionally prints which
definition it chose and what else matched, because answering an ambiguous
question silently is how this turned into a confidently wrong answer.

Also strips Windows' `\?\` verbatim prefix from resolved paths, which was
leaking into every line of user-facing output, and makes file lookup
separator-insensitive — callers join a project root with a forward-slash
relative path, producing `C:\root\src/lib.rs` where the parser recorded
`C:\root\src\lib.rs`, so `file-graph` reported "No symbols found" for files
full of symbols.

Author: Anandb71

crates/arbor-cli/src/commands.rs

@@ -38,8 +38,29 @@ const ROOT_MARKERS: &[&str] = &[
     "pubspec.yaml",
 ];
 
+/// Removes Windows' extended-length path prefix.
+///
+/// `fs::canonicalize` returns verbatim paths (`\\?\C:\...`) on Windows. That
+/// prefix flowed into every stored node path and therefore into every line of
+/// user-facing output, where it is noise at best and confusing at worst.
+fn strip_verbatim_prefix(path: PathBuf) -> PathBuf {
+    match path.to_str() {
+        Some(s) => {
+            if let Some(rest) = s.strip_prefix(r"\\?\UNC\") {
+                PathBuf::from(format!(r"\\{rest}"))
+            } else if let Some(rest) = s.strip_prefix(r"\\?\") {
+                PathBuf::from(rest)
+            } else {
+                path
+            }
+        }
+        None => path,
+    }
+}
+
 fn find_workspace_root(start: &Path) -> PathBuf {
-    let mut current = fs::canonicalize(start).unwrap_or_else(|_| start.to_path_buf());
+    let mut current =
+        strip_verbatim_prefix(fs::canonicalize(start).unwrap_or_else(|_| start.to_path_buf()));
     if current.is_file() {
         if let Some(parent) = current.parent() {
             current = parent.to_path_buf();
@@ -1997,13 +2018,18 @@ pub fn refactor(
     let _ = ensure_arbor_initialized(&resolved_path)?;
     let graph = load_or_index_graph(&resolved_path)?;
 
-    // Find the target node
-    let node_idx = graph.get_index(target).or_else(|| {
-        graph
-            .find_by_name(target)
-            .first()
-            .and_then(|n| graph.get_index(&n.id))
-    });
+    // Find the target node, preferring the most connected definition when the
+    // name is ambiguous — picking the first parsed one reported unrelated
+    // methods as dead code.
+    let node_idx = match resolve_symbol_ranked(&graph, target) {
+        Ok((idx, others)) => {
+            if !json_output {
+                report_symbol_ambiguity(&graph, target, idx, &others);
+            }
+            Some(idx)
+        }
+        Err(_) => None,
+    };
 
     let node_idx = match node_idx {
         Some(idx) => idx,
@@ -3236,23 +3262,82 @@ pub fn audit(sink: &str, depth: usize, format: &str, path: &Path) -> Result<()>
     Ok(())
 }
 
+/// Resolves a symbol name to the node a user most likely meant.
+///
+/// Names collide constantly — `getPath` is a utility function in `utils/url.ts`
+/// *and* a method on four AWS Lambda event processors. This used to take
+/// `find_by_name(..).first()`, i.e. whichever file happened to be parsed first,
+/// and then answered as if that were the only candidate. On hono that meant
+/// `arbor inspect getPath` reported "unreachable, 0 callers, may be dead code"
+/// about a function 23 files depend on.
+///
+/// Candidates are ranked by graph degree, then centrality, then file path, so
+/// the pick is both meaningful and deterministic. Alternatives are returned so
+/// the caller can tell the user what else matched.
+fn resolve_symbol_ranked(
+    graph: &arbor_graph::ArborGraph,
+    symbol: &str,
+) -> Result<(arbor_graph::NodeId, Vec<arbor_graph::NodeId>)> {
+    let mut candidates = graph.resolve_symbol_ranked(symbol);
+    if candidates.is_empty() {
+        return Err(format!("Symbol '{}' not found", symbol).into());
+    }
+    let best = candidates.remove(0);
+    Ok((best, candidates))
+}
+
 fn resolve_symbol(graph: &arbor_graph::ArborGraph, symbol: &str) -> Result<arbor_graph::NodeId> {
-    graph
-        .get_index(symbol)
-        .or_else(|| {
-            graph
-                .find_by_name(symbol)
-                .first()
-                .and_then(|n| graph.get_index(&n.id))
-        })
-        .ok_or_else(|| format!("Symbol '{}' not found", symbol).into())
+    resolve_symbol_ranked(graph, symbol).map(|(best, _)| best)
+}
+
+/// Tells the user which definition was chosen when a name matched several.
+///
+/// Silence here is what turned an ambiguous lookup into a confidently wrong
+/// answer, so the note is printed even though it adds noise.
+fn report_symbol_ambiguity(
+    graph: &arbor_graph::ArborGraph,
+    symbol: &str,
+    chosen: arbor_graph::NodeId,
+    others: &[arbor_graph::NodeId],
+) {
+    if others.is_empty() {
+        return;
+    }
+    let describe = |i: arbor_graph::NodeId| {
+        graph
+            .get(i)
+            .map(|n| {
+                format!(
+                    "{} ({}) {}:{}",
+                    n.qualified_name, n.kind, n.file, n.line_start
+                )
+            })
+            .unwrap_or_default()
+    };
+
+    eprintln!(
+        "note: '{}' matches {} definitions; showing the most connected one:",
+        symbol,
+        others.len() + 1
+    );
+    eprintln!("      → {}", describe(chosen));
+    for &o in others.iter().take(4) {
+        eprintln!("        {}", describe(o));
+    }
+    if others.len() > 4 {
+        eprintln!("        … and {} more", others.len() - 4);
+    }
+    eprintln!("      Pass a qualified name (e.g. Class.method) to pick a specific one.");
 }
 
 pub fn callers(symbol: &str, path: &Path, json_output: bool) -> Result<()> {
     let resolved_path = resolve_project_path(path)?;
     let graph = load_or_index_graph(&resolved_path)?;
 
-    let idx = resolve_symbol(&graph, symbol)?;
+    let (idx, ambiguous_with) = resolve_symbol_ranked(&graph, symbol)?;
+    if !json_output {
+        report_symbol_ambiguity(&graph, symbol, idx, &ambiguous_with);
+    }
     let callers = graph.get_callers(idx);
 
     if json_output {
@@ -3296,7 +3381,10 @@ pub fn callees(symbol: &str, path: &Path, json_output: bool) -> Result<()> {
     let resolved_path = resolve_project_path(path)?;
     let graph = load_or_index_graph(&resolved_path)?;
 
-    let idx = resolve_symbol(&graph, symbol)?;
+    let (idx, ambiguous_with) = resolve_symbol_ranked(&graph, symbol)?;
+    if !json_output {
+        report_symbol_ambiguity(&graph, symbol, idx, &ambiguous_with);
+    }
     let callees = graph.get_callees(idx);
 
     if json_output {
@@ -3468,7 +3556,10 @@ pub fn inspect(symbol: &str, path: &Path, json_output: bool) -> Result<()> {
     let resolved_path = resolve_project_path(path)?;
     let graph = load_or_index_graph(&resolved_path)?;
 
-    let idx = resolve_symbol(&graph, symbol)?;
+    let (idx, ambiguous_with) = resolve_symbol_ranked(&graph, symbol)?;
+    if !json_output {
+        report_symbol_ambiguity(&graph, symbol, idx, &ambiguous_with);
+    }
     let node = graph
         .get(idx)
         .ok_or_else(|| format!("Node index invalid for '{}'", symbol))?;

crates/arbor-core/src/fallback_parser.rs

@@ -6,29 +6,58 @@
 
 use crate::node::{CodeNode, NodeKind};
 
-/// Extra language extensions supported via fallback parsing.
+/// Extra language extensions indexed into the **code** graph via fallback parsing.
+///
+/// Markdown is deliberately absent. It is parseable (see [`MARKDOWN_EXTENSIONS`])
+/// but headings are prose, not symbols: indexing them put a node named `fetch`
+/// into the graph for a `# fetch benchmark` heading in a README, which then
+/// competed with real `fetch` definitions when resolving TypeScript call sites.
+/// Callers that want a document graph ask for it explicitly.
 pub const FALLBACK_EXTENSIONS: &[&str] = &[
     "kt", "kts",   // Kotlin
     "swift", // Swift
     "rb",    // Ruby
     "php", "phtml", // PHP
     "sh", "bash", "zsh", // Shell
-    "md", "markdown", // Markdown for knowledge graphs (Lattice)
 ];
 
+/// Markdown extensions, parsed into `Section` nodes for document graphs.
+///
+/// Not part of [`FALLBACK_EXTENSIONS`] — opt in by calling
+/// [`parse_fallback_source`] with one of these directly.
+pub const MARKDOWN_EXTENSIONS: &[&str] = &["md", "markdown"];
+
 pub fn is_fallback_supported_extension(ext: &str) -> bool {
     let ext = ext.to_ascii_lowercase();
     FALLBACK_EXTENSIONS.iter().any(|e| *e == ext)
 }
 
+/// Whether this extension is parsed as prose rather than code.
+pub fn is_markdown_extension(ext: &str) -> bool {
+    let ext = ext.to_ascii_lowercase();
+    MARKDOWN_EXTENSIONS.iter().any(|e| *e == ext)
+}
+
 pub fn parse_fallback_source(source: &str, file_path: &str, ext: &str) -> Vec<CodeNode> {
     let ext = ext.to_ascii_lowercase();
+    let is_markdown = is_markdown_extension(&ext);
     let mut nodes = Vec::new();
 
     for (idx, line) in source.lines().enumerate() {
         let line_no = idx as u32 + 1;
         let trimmed = line.trim_start();
 
+        // Comments are never declarations. Skip them *before* parsing: the
+        // shell rule looks for `()` anywhere on the line, so the comment
+        // `# Compare app.fetch() between refs` used to yield a function named
+        // "# Compare app.fetch". Markdown is exempt — there `#` starts a
+        // heading, which is the thing we want.
+        if trimmed.is_empty()
+            || (!is_markdown && (trimmed.starts_with('#') || trimmed.starts_with("//")))
+        {
+            continue;
+        }
+
         let candidate = match ext.as_str() {
             "md" | "markdown" => parse_markdown_line(trimmed),
             "kt" | "kts" => parse_kotlin_line(trimmed),
@@ -39,12 +68,6 @@ pub fn parse_fallback_source(source: &str, file_path: &str, ext: &str) -> Vec<Co
             _ => None,
         };
 
-        if trimmed.is_empty()
-            || (trimmed.starts_with('#') || trimmed.starts_with("//")) && candidate.is_none()
-        {
-            continue;
-        }
-
         if let Some((name, kind)) = candidate {
             let col = (line.len().saturating_sub(trimmed.len())) as u32;
             let node = CodeNode::new(&name, &name, kind, file_path)
@@ -158,14 +181,27 @@ fn parse_shell_line(line: &str) -> Option<(String, NodeKind)> {
     // foo() {
     if let Some(paren_idx) = line.find("()") {
         let name = line[..paren_idx].trim();
-        if !name.is_empty() {
+        // The name must be a bare shell identifier. Without this check any
+        // line containing `()` anywhere — prose, a call, a pipeline — yielded
+        // a "function" whose name was the whole preceding text.
+        if is_shell_identifier(name) {
             return Some((name.to_string(), NodeKind::Function));
         }
     }
 
     None
 }
 
+/// Whether a string is a plain shell function name.
+fn is_shell_identifier(s: &str) -> bool {
+    !s.is_empty()
+        && s.chars()
+            .next()
+            .is_some_and(|c| c.is_ascii_alphabetic() || c == '_')
+        && s.chars()
+            .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
+}
+
 fn parse_markdown_line(line: &str) -> Option<(String, NodeKind)> {
     let trimmed = line.trim_start();
     if let Some(rest) = trimmed.strip_prefix("# ") {
@@ -215,9 +251,13 @@ mod tests {
 
     #[test]
     fn fallback_supports_requested_extensions() {
-        for ext in ["kt", "swift", "rb", "php", "sh", "md"] {
+        for ext in ["kt", "swift", "rb", "php", "sh"] {
             assert!(is_fallback_supported_extension(ext));
         }
+        // Markdown moved out of the code-graph set — headings are prose, and
+        // indexing them polluted symbol resolution. Still parseable on request.
+        assert!(!is_fallback_supported_extension("md"));
+        assert!(is_markdown_extension("md"));
     }
 
     #[test]
@@ -383,3 +423,63 @@ object Singleton
         assert!(nodes.iter().any(|n| n.name == "deploy_staging"));
     }
 }
+
+#[cfg(test)]
+mod regression_tests {
+    use super::*;
+
+    #[test]
+    fn markdown_is_not_indexed_as_code() {
+        // A `# fetch benchmark` heading in a README produced a graph node named
+        // `fetch`, which then competed with real `fetch` definitions when
+        // resolving TypeScript call sites.
+        assert!(!is_fallback_supported_extension("md"));
+        assert!(!is_fallback_supported_extension("markdown"));
+        assert!(is_markdown_extension("md"));
+    }
+
+    #[test]
+    fn markdown_still_parses_when_asked_directly() {
+        let nodes = parse_fallback_source("# fetch benchmark\n## setup\n", "README.md", "md");
+        assert_eq!(nodes.len(), 2);
+        assert!(nodes.iter().all(|n| n.kind == NodeKind::Section));
+    }
+
+    #[test]
+    fn shell_comments_are_not_functions() {
+        // `# Compare app.fetch() between the working tree and a git ref`
+        // matched the `()` rule and became a function named
+        // "# Compare app.fetch".
+        let src = "#!/bin/bash\n# Compare app.fetch() between refs\nreal_fn() {\n  echo hi\n}\n";
+        let nodes = parse_fallback_source(src, "compare.sh", "sh");
+
+        assert_eq!(nodes.len(), 1, "only the real function should be indexed");
+        assert_eq!(nodes[0].name, "real_fn");
+    }
+
+    #[test]
+    fn shell_names_must_be_identifiers() {
+        assert!(is_shell_identifier("deploy_app"));
+        assert!(is_shell_identifier("build-web"));
+        assert!(!is_shell_identifier("# Compare app.fetch"));
+        assert!(!is_shell_identifier("app.fetch"));
+        assert!(!is_shell_identifier(""));
+        assert!(!is_shell_identifier("2fast"));
+    }
+
+    #[test]
+    fn prose_containing_parens_is_not_a_function() {
+        let nodes = parse_fallback_source("echo \"call foo() now\"\n", "x.sh", "sh");
+        assert!(
+            nodes.is_empty(),
+            "a string mentioning foo() is not a definition"
+        );
+    }
+
+    #[test]
+    fn double_slash_comments_are_skipped() {
+        let nodes = parse_fallback_source("// fun notReal()\nfun real() {}\n", "a.kt", "kt");
+        assert_eq!(nodes.len(), 1);
+        assert_eq!(nodes[0].name, "real");
+    }
+}

crates/arbor-graph/src/builder.rs

@@ -10,7 +10,7 @@ use crate::symbol_table::SymbolTable;
 use arbor_core::{CodeNode, NodeKind};
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
-use tracing::warn;
+use tracing::debug;
 
 /// Beyond this many candidate definitions, a bare name carries no information
 /// and linking to all of them would swamp the graph with noise.
@@ -168,8 +168,10 @@ impl GraphBuilder {
                 let resolution = self.symbol_table.resolve_ref(lookup, &from_file);
 
                 if !resolution.is_resolved() {
-                    #[cfg(debug_assertions)]
-                    warn!(
+                    // The overwhelming majority of references are stdlib or
+                    // third-party and have no definition here. Expected, not
+                    // notable.
+                    debug!(
                         "Unresolved reference '{}' in {} (likely external/stdlib)",
                         reference,
                         from_file.display()
@@ -196,7 +198,9 @@ impl GraphBuilder {
                     // A name like `new`, `get`, or `run` with dozens of
                     // definitions carries no information. Linking to all of
                     // them would swamp the graph.
-                    warn!(
+                    // Routine on any real codebase — `.get`, `.encode`, `.match`
+                    // are shared by many types. Diagnostic, not a user problem.
+                    debug!(
                         "Reference '{}' in {} has {} candidate definitions — too ambiguous to link",
                         reference,
                         from_file.display(),
@@ -289,7 +293,7 @@ impl GraphBuilder {
         }
 
         // Different module and not imported: almost certainly a name collision.
-        warn!(
+        debug!(
             "Downgrading unimported cross-module reference '{}' in {} → {}",
             reference,
             from_file.display(),