Fix/graph truth layer - #162
Merged
Merged
Conversation
The `export_statement` arm recursed into its declaration children, and then the generic child loop below recursed into those same children again. Every exported symbol became two graph vertices sharing one `CodeNode::id`, since the id hashes (file, qualified_name, kind). On a 149-file Next.js app that was 133 phantom nodes — 25% of the graph. Each duplicate halved its symbol's centrality and double-counted it in blast radius. The arm was redundant anyway: the generic recursion already reaches every declaration, and `is_node_exported` reads the parent node, so export status survives without it.
`SymbolTable::insert` used `HashMap::insert`, so a second definition of an FQN silently replaced the first. Colliding names are ordinary — `handler` in twelve route files, `new` on forty structs, `process` in both `utils.py` and `helpers.py` — and the loser became unreachable: zero callers, zero centrality, invisible to blast radius. A silent false negative in the exact analysis the product sells. The table now keeps every definition and reports how a reference matched: exact FQN, same file, unique suffix, same directory, or genuinely ambiguous. Resolution is also deterministic now. It previously decided same-directory locality by iterating a `HashMap` and taking the first hit; Rust seeds `RandomState` per process, so the same binary on the same input could pick differently between runs. Candidates are sorted by (file, node index) before any choice is made, and a reverse file index replaces the scan. That scan was also the hot path: resolution walked every FQN in the repository per reference, with a nested scan over every file's export list. Unresolvable references — stdlib and third-party calls, the majority of call sites in real code — paid the full cost. Segment-aligned suffixes are now indexed, making it an O(1) lookup.
Edges were all created as bare `Edge::new`, so a proven same-file call and a heuristic suffix guess were indistinguishable downstream. BFS weighted them identically and every consumer treated the whole graph as fact. `Edge` now carries a confidence in [0,1]. The field defaults to 1.0 on deserialize, so graphs cached by an older build keep their previous behaviour exactly.
Edge construction now reads the resolution kind and stamps the matching confidence, so an exact fully-qualified match and a same-directory guess are no longer equal evidence. Ambiguous references produce a low-confidence edge to each candidate rather than being dropped, capped at four. Dropping them hid real call paths; emitting them unlabelled overstated certainty. The import filter also stops exempting same-directory targets. It only ever fired when a reference was unimported *and* the target lived elsewhere in the tree, which meant that in a flat `src/` layout — the common case for JS and Python projects — it never fired at all. Suspicion is now expressed as a confidence multiplier applied uniformly instead of a hard drop with a loophole.
`obj.method()` was discarded outright, on the grounds that typing `obj` needs inference we do not have. But that is the dominant call shape in real TypeScript and JavaScript, so the graph came out nearly edgeless on the largest ecosystem Arbor supports — and an empty graph reports a blast radius of zero, which reads as "safe" rather than "unknown". Such calls now emit a marker that the graph builder resolves by method name, under a tighter fan-out cap and a reduced confidence. Capitalised receivers (`Logger.info`, `MathUtils.add`) keep their qualifier and match exactly, since every mainstream convention reserves those for classes, enums, and namespace imports. Precision is preserved by the ambiguity machinery rather than by silence: a name shared by three or more definitions still links to nothing. Measured on identical node sets, after the duplicate-extraction fix: arbor-cloud/web/src (TS) 172 -> 196 edges (+14%) arbor-graph/src (Rust) 116 -> 167 edges (+44%)
Scores were divided by the largest score in the graph. That bounded the range but destroyed comparability: the top node is 1.0 by construction whether it has four callers or four hundred, so a threshold like 0.6 means "60% as central as whatever the biggest thing in this repository happens to be". In a codebase with one god object nothing ever cleared it; in a flat service almost everything did. It was also unstable. Adding a single new hub rescaled every other node, so a PR could change the reported centrality — and any risk level derived from it — of code it never touched. `centrality()` now returns a percentile rank: 0.9 means "more central than 90% of this repository", and carries that meaning everywhere. Raw PageRank is kept alongside it because warm-start recomputation needs the true fixed point, and `centrality_map()` returns raw so the watcher's incremental path converges exactly.
`changed_node_ids` works at file granularity: every symbol in a touched file counts as changed. For a one-line edit in a file with sixty functions that overstates the blast radius roughly sixtyfold, which is the common case for any PR-driven integration. `changed_node_ids_for_ranges` keeps only symbols whose line spans overlap the changed lines, and `parse_unified_diff_ranges` reads those ranges straight from a patch. The whole hunk span is used, context included: a delete-only hunk has no added lines to point at, but the deletion still changes the enclosing symbol and the context is what locates it. Files whose changes fall outside every tracked symbol — an import block, a comment, a top-level constant — are reported rather than silently dropped, so a caller can say "changed outside any tracked symbol" instead of implying zero impact.
🔴 Arbor PR Walk
Changed Files
🎯 Production Entry Points ReachedThis change propagates to these entry points (HTTP handlers, jobs, CLI commands):
✅ Before You Merge
🛑 Sensitive Path Check — GATE
Sensitive call paths
📊 Analysis confidence: High · 1710 nodes · 9924ms
Arbor · View full report → · 9924ms · 1710 nodes · Deterministic PR blast-radius · Was this useful? 👍 👎 |
🌳 Arbor Impact ReportRisk Level: 🔴 Critical | Blast Radius: 241 nodes | Changed Symbols: 487 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;
class Golden changed;
class Features changed;
class Arbor changed;
class Development changed;
class Configuration changed;
Impact Summary
Powered by Arbor v2.6.0 — graph-native code intelligence |
Search matched character substrings against names, so asking for `login` found nothing at all in a codebase whose function is called `get_authenticated`. The two share no substring. Anyone asking Arbor about an auth surface hit this immediately. Identifiers are now tokenized — snake, camel, Pascal, kebab, acronym runs — and expanded through curated concept clusters, so `login` reaches `authenticate`, `session`, `credential`, and the rest of its cluster. This is vocabulary resolution, not a semantic model: deterministic, offline, and auditable, with the clusters right there in the source. Docstrings, signatures, qualified names, and file paths are indexed too. All four were already parsed into every `CodeNode` and then ignored, so a function documented as "validates the user's login credentials" was unreachable by a search for "login". Results carry the match kind, so an exact hit and a concept guess are distinguishable instead of blended into one opaque ranking. `search()` keeps its literal-substring semantics; `search_ranked()` is the new lens. The substring verifier also stops rescanning the entire name index once per candidate, which made the old `search` slower than the linear scan its documentation claimed to replace.
Arbor's headline claim is a deterministic walk, and nothing tested it. Running the builder twice inside one process would not have caught the bug it is written for: a single process shares one hash seed, so `HashMap` iteration order is stable within a run and varies only between runs. These tests shell out to fresh processes and assert one identical graph across eight of them. The fixture is built to exercise every ambiguous path — colliding bare names across directories, same-name methods on different classes, and calls on untyped receivers. Also adds a `graph_stats` example reporting node count, edge count, edge density, confident-vs-weak edge split, and orphan count. It is what the edge recall figures in this branch were measured with, so the numbers are reproducible rather than asserted.
Documents the correctness work and what each defect actually cost, with the measured edge-recall figures and the command to reproduce them. The version bump is load-bearing, not ceremony: `CACHE_VERSION` derives from the package version, and centrality now means percentile rank rather than a max-normalized score. A cache written by 2.5.0 would deserialize cleanly and be interpreted wrongly, so it has to be invalidated. Concept search is documented as a library capability. `arbor query` still does literal substring matching and is described that way — the CLI wiring is not done, and claiming otherwise is the kind of overclaim this release exists to remove.
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.
Description
Brief description of what this PR does. Link any related issues.
Fixes #(issue number)
Type of Change
Changes Made
Testing
Describe how you tested your changes:
cargo test --allcargo clippy --allflutter test(if applicable)Screenshots (if applicable)
For visualizer changes, include before/after screenshots.
Checklist