From c46c033dc1cfd6ac627d24cb3cb1d243fe7e5abb Mon Sep 17 00:00:00 2001 From: mallyskies Date: Mon, 6 Jul 2026 23:14:45 -0600 Subject: [PATCH 1/2] fix(build): don't let an ambiguous legacy-stem alias silently merge files build_from_json's "pre-migration alias index" (#1504) registers each real file's OLD-style bare-stem id (extension dropped) as an alias so a stale cached fragment referencing that old form still resolves after an id-scheme migration. It never checked for collisions: two unrelated real files easily compute the same bare alias (e.g. "ping.h" and "ping.php" in different directories both alias to "ping"), and dict.setdefault let whichever file happened to iterate first in a Python set win, arbitrarily. A dangling edge with a bare, deliberately-unscoped fallback target (e.g. the C/C++ extractor's last-resort id for an #include it couldn't resolve to a real path) would ride that alias onto whatever unrelated file won the collision -- silently wiring, say, a C++ server file to an unrelated PHP script, purely because both files happen to share a common basename somewhere in the corpus. Now every candidate for an alias is collected before any of them are committed to norm_to_id, and the alias is only trusted when exactly one real file claims it. Ambiguous aliases are dropped entirely, so the dangling edge correctly stays dangling (and gets discarded) instead of merging two unrelated files -- same "don't guess through ambiguity" principle already applied to stub-node disambiguation and cross-file call resolution elsewhere in this codebase. Found via graphify-practice round 2: a `path` query between two unrelated symbols routed through exactly this kind of bogus edge, traced back to Dev/Poker/TDServer/server.cpp's unresolved `#include "ping.h"` / `#include "utility.h"` landing on unrelated www.masque.com PHP scripts of the same bare name. --- graphify/build.py | 20 +++++++++++++-- tests/test_build.py | 59 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/graphify/build.py b/graphify/build.py index 279eccbf4..077db58ca 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -507,7 +507,20 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat # fragment (e.g. an incremental update whose fragment references a symbol in a # file that was NOT re-extracted) still resolves to the migrated node instead # of dangling. Only fills gaps — never overrides a real node id. + # + # The old-stem form drops the extension and (for the file node itself) every + # directory but the immediate parent, so it collapses easily: "ping.h" and + # "ping.php" in different directories both alias to bare "ping". Collecting + # every candidate for an alias BEFORE committing any of them — and only + # committing when exactly one candidate claims it — keeps this a precise + # re-keying aid instead of a silent cross-file (and cross-language) merge. + # Without this, a dangling edge to a bare, deliberately-unscoped fallback id + # (e.g. the C/C++ extractor's last-resort target for an #include it couldn't + # resolve to a real path) could ride this alias onto whichever unrelated + # same-stem file happened to be inserted first into ``node_set`` — a Python + # set, so "first" is hash-order, not anything meaningful. from graphify.extractors.base import _file_stem as _fs + _alias_candidates: dict[str, set[str]] = {} for nid in node_set: attrs = G.nodes[nid] sf = attrs.get("source_file") @@ -524,8 +537,11 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat if old_stem == new_stem: continue alias = old_stem + suffix - norm_to_id.setdefault(_normalize_id(alias), nid) - norm_to_id.setdefault(alias, nid) + _alias_candidates.setdefault(_normalize_id(alias), set()).add(nid) + _alias_candidates.setdefault(alias, set()).add(nid) + for alias_key, candidates in _alias_candidates.items(): + if len(candidates) == 1: + norm_to_id.setdefault(alias_key, next(iter(candidates))) # Iterate edges in a deterministic order. The graph is undirected and stores # direction in _src/_tgt; when two edges collapse onto the same node pair the # last write wins, so an unstable iteration order flips _src/_tgt run-to-run diff --git a/tests/test_build.py b/tests/test_build.py index 2d8bfdd60..1e1d5b56e 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -505,6 +505,65 @@ def test_build_relativizes_absolute_source_file(tmp_path): assert sf == "src/main.py" +def test_build_from_json_ambiguous_old_stem_alias_stays_dangling(tmp_path): + """The #1504 old-stem alias (e.g. "ping.h" -> bare "ping") is meant to let a + stale-id edge from an un-re-extracted fragment still find its own file after + a rekey. But the old-stem form drops the extension and most of the path, so + two unrelated real files easily collapse onto the same bare alias (a C header + and a PHP script both named "ping", in different directories). A dangling + edge produced by an unrelated third file's own unscoped fallback id (e.g. the + C/C++ extractor's last-resort target for an #include it couldn't resolve to + a real path) must not silently ride that alias onto an arbitrary one of them + — it should stay dangling and get dropped, same as any other unresolvable + edge, rather than wire two unrelated files/languages together by accident.""" + root = tmp_path / "repo" + root.mkdir() + extraction = { + "nodes": [ + # Ids given in their canonical (post-extract.py, extension-stripped) + # form, matching what a real graphify update run would already have + # produced before build_from_json assembles the final graph. + {"id": "dev_monitoring_ping", "label": "ping.h", "file_type": "code", + "source_file": "Dev/monitoring/ping.h"}, + {"id": "www_pages_api_ping", "label": "ping.php", "file_type": "code", + "source_file": "www/pages/api/ping.php"}, + {"id": "dev_poker_server", "label": "server.cpp", "file_type": "code", + "source_file": "Dev/poker/server.cpp"}, + ], + "edges": [ + # The unscoped, deliberately-unresolved fallback edge a C/C++ #include + # resolver leaves behind when it can't find the header on disk. + {"source": "dev_poker_server", "target": "ping", "relation": "imports", + "confidence": "EXTRACTED", "source_file": "Dev/poker/server.cpp"}, + ], + } + G = build_from_json(extraction, root=root) + assert not G.has_edge("dev_poker_server", "dev_monitoring_ping") + assert not G.has_edge("dev_poker_server", "www_pages_api_ping") + + +def test_build_from_json_unambiguous_old_stem_alias_still_resolves(tmp_path): + """Companion to the ambiguous case above: when exactly one real file claims + an old-stem alias, a dangling edge to that bare alias should still resolve + to it — the #1504 migration-compat behavior this index exists for.""" + root = tmp_path / "repo" + root.mkdir() + extraction = { + "nodes": [ + {"id": "dev_monitoring_utility", "label": "utility.h", "file_type": "code", + "source_file": "Dev/monitoring/utility.h"}, + {"id": "dev_poker_server", "label": "server.cpp", "file_type": "code", + "source_file": "Dev/poker/server.cpp"}, + ], + "edges": [ + {"source": "dev_poker_server", "target": "utility", "relation": "imports", + "confidence": "EXTRACTED", "source_file": "Dev/poker/server.cpp"}, + ], + } + G = build_from_json(extraction, root=root) + assert G.has_edge("dev_poker_server", "dev_monitoring_utility") + + def test_build_from_json_relative_source_file_unchanged(tmp_path): """Already-relative source_file paths must not be modified.""" extraction = { From 19ad9b8ea68cbca9ca57bacfe5be3c43a0d6dba0 Mon Sep 17 00:00:00 2001 From: mallyskies Date: Mon, 6 Jul 2026 23:55:30 -0600 Subject: [PATCH 2/2] fix(build): recognize a salted file node as its own alias claimant Follow-up to the previous commit on this branch. That fix's ambiguity check missed a case: it detects "is this node the file itself" by checking whether the node's id starts with the file's plain new_stem, but a same-directory .h/.cpp pair that collides on their shared pre-extension id gets salted apart by _disambiguate_colliding_node_ids into ids like "tools_aolserver_utility_h_tools_aolserver_utility" -- no longer a clean new_stem prefix. That salted header silently failed to compute an empty suffix, so it never entered the bare "utility" alias race at all, leaving an unrelated wwwapi.masque.com/pages/utility.php as the lone (wrong) "unambiguous" winner -- reproduced exactly against the real depot's Tools/aolserver/utility.h and .cpp. Detect "this node IS the file" by label instead: every file node's label is its own basename regardless of what its id looks like after salting. That keeps a salted file node in the alias competition, so the real collision between the C header and the PHP file is correctly caught as ambiguous. --- graphify/build.py | 21 ++++++++++++++++++--- tests/test_build.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/graphify/build.py b/graphify/build.py index 077db58ca..b26ddcb61 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -519,6 +519,18 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat # resolve to a real path) could ride this alias onto whichever unrelated # same-stem file happened to be inserted first into ``node_set`` — a Python # set, so "first" is hash-order, not anything meaningful. + # + # A file node's OWN id is not always a clean ``new_stem`` prefix: when a + # same-directory ``.h``/``.cpp`` pair collides on their shared pre-extension + # id, _disambiguate_colliding_node_ids salts both apart into ids like + # ``tools_aolserver_utility_h_tools_aolserver_utility`` — which no longer + # string-prefixes cleanly for the suffix math below. Detecting "this IS the + # file node" by label (every file node's label is its own basename, + # regardless of id mangling) instead of by id shape keeps a salted file node + # in the alias competition, so a genuine collision (a C header AND an + # unrelated same-named PHP script) is still caught as ambiguous instead of + # the header silently dropping out of the race and leaving the PHP file as + # the lone (wrong) "unambiguous" winner. from graphify.extractors.base import _file_stem as _fs _alias_candidates: dict[str, set[str]] = {} for nid in node_set: @@ -530,9 +542,12 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat if rel.is_absolute(): continue new_stem = make_id(_fs(rel)) - suffix = "" - if _normalize_id(nid).startswith(new_stem): - suffix = _normalize_id(nid)[len(new_stem):] # leading "_entity" or "" + if str(attrs.get("label", "")) == rel.name: + suffix = "" # this node IS the file, whatever its (possibly salted) id + else: + suffix = "" + if _normalize_id(nid).startswith(new_stem): + suffix = _normalize_id(nid)[len(new_stem):] # leading "_entity" or "" for old_stem in _old_file_stems(rel): if old_stem == new_stem: continue diff --git a/tests/test_build.py b/tests/test_build.py index 1e1d5b56e..e8c0309a9 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -542,6 +542,40 @@ def test_build_from_json_ambiguous_old_stem_alias_stays_dangling(tmp_path): assert not G.has_edge("dev_poker_server", "www_pages_api_ping") +def test_build_from_json_ambiguous_alias_detected_despite_header_impl_salting(tmp_path): + """A same-directory .h/.cpp pair collides on their shared pre-extension id + and gets salted apart into ids like "tools_aolserver_utility_h_..." — no + longer a clean new_stem prefix. The ambiguity check must still recognize + the salted header as a legitimate claimant for the bare old-stem alias (by + label, not id shape), so a real collision with an unrelated same-named PHP + file is still caught instead of the header silently dropping out of the + race and leaving the PHP file as the lone "unambiguous" winner (this + reproduced against the real depot: Tools/aolserver/utility.h and .cpp, + salted apart, let wwwapi.masque.com/pages/utility.php win the bare + "utility" alias uncontested).""" + root = tmp_path / "repo" + root.mkdir() + extraction = { + "nodes": [ + {"id": "tools_aolserver_utility_h_tools_aolserver_utility", "label": "utility.h", + "file_type": "code", "source_file": "Tools/aolserver/utility.h"}, + {"id": "tools_aolserver_utility_cpp_tools_aolserver_utility", "label": "utility.cpp", + "file_type": "code", "source_file": "Tools/aolserver/utility.cpp"}, + {"id": "wwwapi_masque_com_pages_utility", "label": "utility.php", + "file_type": "code", "source_file": "wwwapi.masque.com/pages/utility.php"}, + {"id": "dev_poker_server", "label": "server.cpp", "file_type": "code", + "source_file": "Dev/poker/server.cpp"}, + ], + "edges": [ + {"source": "dev_poker_server", "target": "utility", "relation": "imports", + "confidence": "EXTRACTED", "source_file": "Dev/poker/server.cpp"}, + ], + } + G = build_from_json(extraction, root=root) + assert not G.has_edge("dev_poker_server", "wwwapi_masque_com_pages_utility") + assert not G.has_edge("dev_poker_server", "tools_aolserver_utility_h_tools_aolserver_utility") + + def test_build_from_json_unambiguous_old_stem_alias_still_resolves(tmp_path): """Companion to the ambiguous case above: when exactly one real file claims an old-stem alias, a dangling edge to that bare alias should still resolve