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.
This commit is contained in:
+18
-3
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user