fix(graph): correct symbol resolution, centrality, and edge recall (v2.6.0) - #163
Merged
Conversation
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.
`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.
`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.
🔴 Arbor PR Walk
Changed Files
|
| Category | File | Symbols |
|---|---|---|
| File I/O | crates/arbor-graph/src/graph.rs |
remove_file, test_graph_remove_file_cleanup |
| Input Validation | crates/arbor-cli/src/commands.rs |
Command Injection (CWE-78), line 878 |
Sensitive call paths
- change reaches sink:
agent_onboard→resolve_project_path→find_workspace_root→strip_verbatim_prefix(3 hops) — full graph path to Command Injection in crates/arbor-cli/src/commands.rs line 878 - change reaches sink:
agent_onboard→resolve_project_path→find_workspace_root→strip_verbatim_prefix(3 hops) — full graph path to Command Injection in crates/arbor-cli/src/commands.rs line 904 - change reaches sink:
agent_onboard→resolve_project_path→find_workspace_root→strip_verbatim_prefix(3 hops) — full graph path to Command Injection in crates/arbor-cli/src/commands.rs line 909 - change reaches sink:
agent_onboard→resolve_project_path→find_workspace_root→strip_verbatim_prefix(3 hops) — full graph path to Command Injection in crates/arbor-cli/src/commands.rs line 1503 - change reaches sink:
agent_onboard→resolve_project_path→find_workspace_root→strip_verbatim_prefix(3 hops) — full graph path to Command Injection in crates/arbor-cli/src/commands.rs line 1524 - change reaches sink:
agent_onboard→resolve_project_path→find_workspace_root→strip_verbatim_prefix(3 hops) — full graph path to Command Injection in crates/arbor-cli/src/commands.rs line 1810 - change reaches sink:
agent_onboard→resolve_project_path→find_workspace_root→strip_verbatim_prefix(3 hops) — full graph path to Command Injection in crates/arbor-cli/src/commands.rs line 2615
- 🟡 Fix Command Injection in
commands.rs(line 878) [31 internal callers — inspect full call path before merge; path: agent_onboard -> resolve_project_path -> find_workspace_root -> strip_verbatim_prefix] — Command::new() with non-literal command name — validate all inputs. - 🟡 Verify input validation still rejects malformed and malicious input
📊 Analysis confidence: High · 1727 nodes · 10035ms
- Graph has 1727 nodes and 856 edges — well-connected codebase
- 44 symbols changed, 71 upstream nodes analyzed
Arbor · View full report → · arbor v2.5.0 · 10035ms · 1727 nodes · verify locally with arbor diff · Deterministic PR blast-radius · Was this useful? 👍 👎
🌳 Arbor Impact ReportRisk Level: 🔴 Critical | Blast Radius: 180 nodes | Changed Symbols: 273 Changed Files
📊 Visual Impact Graphgraph TD
classDef changed fill:#ef4444,stroke:#333,stroke-width:2px,color:#fff;
classDef caller fill:#f59e0b,stroke:#333,stroke-width:1px,color:#fff;
find_workspace_root[find_workspace_root] --> strip_verbatim_prefix[strip_verbatim_prefix]
resolve_project_path[resolve_project_path] --> find_workspace_root[find_workspace_root]
class Result changed;
class DiffSummary changed;
class find_workspace_root changed;
class ROOT_MARKERS changed;
class strip_verbatim_prefix changed;
class resolve_project_path caller;
Impact Summary
Powered by Arbor v2.6.0 — graph-native code intelligence |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nine defects in the analysis core, each found by running Arbor against real
codebases (honojs/hono, psf/requests) rather than fixtures, and each with a
regression test.
Correctness
Determinism.
resolve_with_contextdecided same-directory locality byiterating a
HashMapand taking the first hit. Rust seedsRandomStateperprocess, so the same binary on the same input could build different edges
between runs — the opposite of the deterministic walk Arbor advertises.
Resolution now works from an explicit index and sorts candidates by
(file, node index). A cross-process test indexes a fixture eight times infresh processes and asserts one identical graph.
Symbol collisions. The FQN map used
HashMap::insert, so a seconddefinition of
handler,new, orprocesssilently replaced the first. Theloser became unreachable: zero callers, zero centrality, invisible to blast
radius. The table now keeps every definition and reports ambiguity.
Ambiguous lookups answered confidently and wrongly.
arbor inspect getPathon hono reported "unreachable, 0 callers, may be dead code" about a function
called from ten places. Eleven call sites across the CLI and MCP bridge each
resolved names with
find_by_name(..).first()— whichever file parsed first.ArborGraph::resolve_symbol_rankedis now the single policy, and the CLI sayswhich definition it chose and what else matched.
TypeScript symbols were extracted twice. The
export_statementarm recursedinto its declaration children, then the generic loop recursed into the same
children again. Every exported symbol became two vertices sharing one
CodeNode::id. On a 149-file Next.js app that was 133 phantom nodes — 25% ofthe graph — splitting centrality and double-counting blast radius.
Prose was indexed as code. Markdown headings became graph nodes, so a
# fetch benchmarkheading in a README competed with realfetchdefinitionsduring resolution. Shell comments became functions:
# Compare app.fetch() between refsyielded a function named"# Compare app.fetch". 59 phantomnodes on hono, every one a false resolution candidate.
Centrality was not comparable between repositories. Scores were divided by
the graph maximum, so the top node was
1.0by construction and a thresholdlike
0.6meant "60% as central as whatever the biggest thing here happens tobe". Adding one hub rescaled every other node. Centrality is now a percentile
rank; raw PageRank is retained for warm starts.
Recall
obj.method()was discarded outright in TS/JS on the grounds that typing thereceiver needs inference we lack. That is the dominant call shape in real
TypeScript, so the graph came out nearly edgeless on the largest supported
ecosystem — and an empty graph reports a blast radius of zero, which reads as
"safe" rather than "unknown". Such calls now resolve by method name under a
fan-out cap and reduced confidence.
Measured on identical node sets, after the duplicate fix:
arbor-cloud/web/src(TS)arbor-graph/src(Rust)New capability
Edge confidence. Every edge carries
[0,1]derived from how it resolved,so BFS no longer treats a proven same-file call and a heuristic suffix guess as
identical evidence.
confident_edge_countexposes how much of a graph rests oninference.
Hunk-level impact.
changed_node_ids_for_rangesmaps diff line ranges tothe symbols that actually overlap them;
parse_unified_diff_rangesreads themfrom a patch. File granularity treats a one-line edit in a 60-symbol file as
changing all 60.
Concept search. Identifiers are tokenized (snake, camel, Pascal, acronym
runs) and expanded through curated concept clusters, so
loginfindsget_authenticated— which shares no substring and was previously unfindable.Docstrings, signatures, and file paths are now indexed too; they were parsed
into every node and then ignored. Results carry the match kind, so an exact hit
and a concept guess stay distinguishable.
Verification
320 tests pass,
clippy --all-targets -D warningsclean,fmtclean.Exercised end to end via the CLI and the MCP bridge against hono (307 TS files)
and psf/requests, plus a real merged PR through the full analysis pipeline.
Version bumped to 2.6.0. The bump also invalidates graph caches via
CACHE_VERSION, which is required: the serialized format gained edgeconfidence and raw centrality.