From 7aa3fee2b9b7c5774bfe285ceeb7b6d08c615b43 Mon Sep 17 00:00:00 2001 From: Yyunozor Date: Tue, 28 Jul 2026 00:45:58 +0200 Subject: [PATCH] fix(extract): canonicalize out-of-root import/include edge targets (#2243) Follow-up to #1899. That fix taught the relativization pass to catch a NODE whose id was minted from an absolute out-of-root path and give it a portable "ext_"-namespaced id, by matching the node's own id against _make_id(str(its source_file)). But several cross-file resolvers (Python relative imports, C/C++/ObjC quoted #include) only ever emit an EDGE for an import target, no node -- so when that target lives outside the scan root, the belt-and-braces pass has nothing to learn the old->new id from, and the edge keeps the raw _make_id(str(absolute_path)) slug forever. The scan path, including the OS username, ends up in links[].source/target, and differs between machines/checkouts even though the node id sets are identical. _import_c also never stamped the transient `target_file` hint (#1814/#2169) its Python/JS siblings already use for exactly this kind of cross-file target canonicalization, so it could not benefit from that machinery either. Fix, in the two places this root cause actually lives: - _import_c now stamps target_file on a resolved #include, mirroring _import_python/_import_js. - The id_remap pass that already walks target_file-stamped edges to canonicalize in-root-but-unscanned targets now also handles the out-of-root branch it previously skipped ("leave its ids alone"): an existing out-of-root target gets the same portable ext_-namespaced id an out-of-root NODE already gets, so an edge with no node of its own is covered too. A target that does not exist on disk still stays dangling, unchanged from before. _portable_out_of_root_sf moved next to id_remap so both the new edge-target branch and the existing node-level pass share one implementation. Four tests in tests/test_extract.py: the out-of-root #include gets a portable id instead of the raw slug, and its transient target_file hint never leaks into the returned edge; the same corpus built from two differently-nested checkout paths produces a byte-identical target (the reported non-determinism, made explicit); an in-root, same-batch include still resolves to the real node's id (negative/regression guard); and the equivalent out-of-root Python relative import is fixed too, since the gap was in the shared remap path, not language-specific. Known limitation: this covers every current target_file-stamping resolver (Python relative imports, C/C++/ObjC #include, JS/TS/Svelte/Astro/Vue rescued imports). A resolver that mints a path-derived edge target WITHOUT stamping target_file at all -- none do today -- would still leak; the fix closes the gap in the shared mechanism, not a per-language allowlist. --- graphify/extract.py | 73 ++++++++++++++++++++--------- tests/test_extract.py | 105 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 23 deletions(-) diff --git a/graphify/extract.py b/graphify/extract.py index 9de0c7622..8616f4ac7 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -550,6 +550,11 @@ def _import_c(node, source: bytes, file_nid: str, stem: str, edges: list, str_pa "source_file": str_path, "source_location": f"L{node.start_point[0] + 1}", "weight": 1.0, + # Stamp the resolved target, mirroring _import_python (#1814): + # without it, an include whose header lives outside this + # batch's paths keeps the raw absolute-path id no later pass + # ever learns to relativize (#2243). + "target_file": str(resolved), }) break module_name = raw.split("/")[-1].split(".")[0] @@ -4676,6 +4681,30 @@ def extract( # subagents generate (#1033). Resolve before relativizing so paths passed in # relative form still anchor to the (resolved) root. id_remap: dict[str, str] = {} + # A target OUTSIDE the scan root (an out-of-root ProjectReference/.sln/bash + # `source`/#include/relative import) can't be made relative to root; leaving + # it absolute leaked the scan path including the OS username into a + # committed graph.json (#1899). Fall back to a walk-up relative form, or the + # bare basename when that would still embed foreign path segments (a + # far-away or cross-drive target). Shared below by both the target_file + # remap loop (edges with no node of their own, #2243) and the node-level + # relativization pass further down (#1899/#2195). + def _portable_out_of_root_sf(p: Path) -> str: + try: + rel = os.path.relpath(str(p), str(root)).replace("\\", "/") + except ValueError: + return p.name # different Windows drive: no relative path exists + updepth = 0 + for seg in rel.split("/"): + if seg == "..": + updepth += 1 + else: + break + # More than a couple of walk-ups means the target lives well outside the + # corpus; its ancestor dirs would embed foreign (possibly user-named) + # segments, so collapse to the basename. + return p.name if updepth > 3 else rel + # Symbol node IDs embed the file stem as a prefix (_file_node_id of the path # the extractor saw). For a root-level file that stem picks up the absolute # parent directory name, so a symbol becomes _main_run while the @@ -4732,7 +4761,25 @@ def extract( try: _tp.relative_to(root) except ValueError: - continue # out-of-root target: leave its ids alone + # Out-of-root target: `_file_node_id` (used below for in-root + # targets) needs a root-relative path, so it cannot help here. + # No node stands for this target either (target_file-stamped + # edges intentionally mint no stub node, #2195), so unlike an + # out-of-root node the belt-and-braces pass below never learns + # this id from anywhere — without registering it here the raw + # scan-path slug survives in the edge forever (#2243). Give it + # the same portable "ext_" id an out-of-root node would get; a + # target that does not actually exist on disk stays dangling, + # exactly as before. + try: + if _tp.is_file(): + ext_new_id = _make_id("ext", _portable_out_of_root_sf(_tp)) + id_remap[_make_id(str(_tp))] = ext_new_id + if _raw_tp != _tp: + id_remap[_make_id(str(_raw_tp))] = ext_new_id + except OSError: + pass + continue try: if not _tp.is_file(): # Speculatively-resolved target that doesn't exist (e.g. an @@ -5367,29 +5414,9 @@ def _has_import_evidence(candidate_id: str) -> bool: run_language_resolvers(paths, per_file, all_nodes, all_edges) # Relativize source_file fields so paths are portable across machines (#555). - # A target OUTSIDE the scan root (an out-of-root ProjectReference/.sln/bash - # `source`) can't be made relative to root; leaving it absolute leaked the - # scan path including the OS username into a committed graph.json (#1899). - # Fall back to a walk-up relative form, or the bare basename when that would - # still embed foreign path segments (a far-away or cross-drive target). When - # the node's id was itself minted from the absolute path, remap it to a + # When the node's id was itself minted from the absolute path, remap it to a # portable id and rewrite the edge endpoints that reference it. - def _portable_out_of_root_sf(p: Path) -> str: - try: - rel = os.path.relpath(str(p), str(root)).replace("\\", "/") - except ValueError: - return p.name # different Windows drive: no relative path exists - updepth = 0 - for seg in rel.split("/"): - if seg == "..": - updepth += 1 - else: - break - # More than a couple of walk-ups means the target lives well outside the - # corpus; its ancestor dirs would embed foreign (possibly user-named) - # segments, so collapse to the basename. - return p.name if updepth > 3 else rel - + # ``_portable_out_of_root_sf`` is defined above, by ``id_remap`` (#2243). ext_id_remap: dict[str, str] = {} for item in all_nodes + all_edges: sf = item.get("source_file") diff --git a/tests/test_extract.py b/tests/test_extract.py index e0a880b89..488bdcf49 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -1029,6 +1029,111 @@ def test_out_of_tree_cache_root_keeps_source_file_relative_to_scan_root(tmp_path assert str(tmp_path) not in n["id"] +def test_c_include_out_of_root_target_id_is_portable(tmp_path): + """#2243 (residual of #1899, in edges not nodes): a `#include "../lib/foo.h"` + reaching OUTSIDE the scan root must not leak the absolute scan path + (including the OS username) into the edge's target id. #1899's out-of-root + fix taught the belt-and-braces pass to catch a NODE whose id was minted from + the same absolute path it carries as source_file -- but `_import_c` mints no + node of its own for an include target, only an edge, so that pass had + nothing to learn from and the raw `_make_id(str(absolute_path))` slug + survived untouched.""" + app = tmp_path / "app" + app.mkdir() + lib = tmp_path / "lib" + lib.mkdir() + (lib / "foo.h").write_text("int foo_compute(int x);\n") + (app / "main.c").write_text( + '#include "../lib/foo.h"\nint main(void) { return foo_compute(1); }\n' + ) + result = extract([app / "main.c"], cache_root=app) + marker = str(tmp_path) + for e in result["edges"]: + for f in ("source", "target", "source_file"): + assert marker not in str(e.get(f, "")), f"leaked into edge {f}: {e}" + assert "target_file" not in e, f"transient target_file hint leaked: {e}" + include_edges = [e for e in result["edges"] if e["relation"] == "imports"] + assert include_edges, "expected an imports edge for the #include" + assert include_edges[0]["target"] == "ext_lib_foo_h" + + +def test_c_include_out_of_root_target_id_is_deterministic_across_checkout_paths(tmp_path): + """#2243: the SAME corpus, scanned from two differently-named, differently + nested checkout locations, must produce a byte-identical edge target id for + an out-of-root `#include`. Before the fix each checkout baked its own + absolute scan path into the target, so a graph.json committed to git showed + a spurious `links` diff on every rebuild even though nothing else changed.""" + + def _build(root_dir_name): + base = tmp_path / root_dir_name / "deeper" / "nesting" + app = base / "app" + app.mkdir(parents=True) + lib = base / "lib" + lib.mkdir() + (lib / "foo.h").write_text("int foo_compute(int x);\n") + (app / "main.c").write_text( + '#include "../lib/foo.h"\nint main(void) { return foo_compute(1); }\n' + ) + result = extract([app / "main.c"], cache_root=app) + return [e["target"] for e in result["edges"] if e["relation"] == "imports"][0] + + target_a = _build("checkout_alice") + target_b = _build("checkout_bob_at_a_totally_different_nesting_depth") + assert target_a == target_b == "ext_lib_foo_h" + + +def test_c_include_in_root_same_batch_still_resolves_to_real_node(tmp_path): + """Negative companion to the two tests above: when the included header IS + inside the scan root and IS part of the same extraction batch, the edge must + keep pointing at the real file node's id -- the out-of-root fix must never + fire, or dangle, for a target the scan already covers.""" + app = tmp_path / "app" + app.mkdir() + lib = tmp_path / "lib" + lib.mkdir() + (lib / "foo.h").write_text("int foo_compute(int x);\n") + (app / "main.c").write_text( + '#include "../lib/foo.h"\nint main(void) { return foo_compute(1); }\n' + ) + result = extract([app / "main.c", lib / "foo.h"], cache_root=tmp_path, root=tmp_path) + header_nodes = [n for n in result["nodes"] if n.get("source_file") == "lib/foo.h"] + assert header_nodes, "expected a real node for the in-batch header" + include_edges = [ + e for e in result["edges"] + if e["relation"] == "imports" and e.get("source_file") == "app/main.c" + ] + assert include_edges + assert include_edges[0]["target"] == header_nodes[0]["id"] + assert not include_edges[0]["target"].startswith("ext_") + + +def test_python_relative_import_out_of_root_target_id_is_portable(tmp_path): + """#2243 is not C-specific: it is a gap in the shared target_file remap path + every language resolver funnels through. Python's cross-directory relative + import already stamped `target_file` (#1814/#2169) -- but for a genuinely + out-of-root target that stamp was still discarded ("out-of-root target: + leave its ids alone") with no fallback, so the raw absolute-path id leaked + exactly as it did for C. Covering this second, independent consumer of the + same remap path guards against a fix that only special-cased `_import_c`.""" + app = tmp_path / "app" + app.mkdir() + lib = tmp_path / "lib" + lib.mkdir() + (app / "__init__.py").write_text("") + (lib / "__init__.py").write_text("") + (lib / "mod.py").write_text("def compute(x):\n return x + 1\n") + (app / "main.py").write_text( + "from ..lib.mod import compute\n\ndef run():\n return compute(1)\n" + ) + result = extract([app / "main.py", app / "__init__.py"], cache_root=app) + marker = str(tmp_path) + for e in result["edges"]: + assert marker not in str(e.get("target", "")), f"leaked into edge target: {e}" + import_edges = [e for e in result["edges"] if e["relation"] == "imports_from"] + assert import_edges + assert import_edges[0]["target"] == "ext_lib_mod_py" + + def test_python_module_qualified_call_resolves_extracted(tmp_path): """`module.func()` where `module` is imported resolves to the callable that module contains, with an EXTRACTED `calls` edge (#1883). A lowercase module