fix(build): make definition_file portable, like its sibling source_file (#3223)

graphify/build.py

@@ -973,6 +973,13 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
                 continue
             if "source_file" in node:
                 node["source_file"] = _norm_source_file(node["source_file"], _root)
+            # definition_file names a file inside the scanned tree exactly like
+            # source_file (the #2990 decl/def merge stamps it from the impl's
+            # source_file BEFORE this normalization runs), so it must be made
+            # portable the same way - it used to ship absolute, leaking the
+            # build host's layout into graph.json and MCP get_node (#3223).
+            if "definition_file" in node:
+                node["definition_file"] = _norm_source_file(node["definition_file"], _root)
         G.add_node(node["id"], **{k: v for k, v in node.items() if k != "id"})
     node_set = set(G.nodes())
 
@@ -1202,6 +1209,10 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
             )
         if "source_file" in attrs:
             attrs["source_file"] = _norm_source_file(attrs["source_file"], _root)
+        if attrs.get("definition_file"):
+            # Same portability rule as source_file (#3223); heals a graph
+            # written before the fix on its next rebuild.
+            attrs["definition_file"] = _norm_source_file(attrs["definition_file"], _root)
         # Drop cross-language phantom edges — the same short names (render, parse,
         # time, ...) recur across language boundaries, so an unresolved target can
         # bind to a same-named node in another language. The extraction spec forbids

graphify/cache.py

@@ -606,31 +606,35 @@ def _relativize_source_files_in(payload: dict, root: Path) -> None:
     # source_file the same way nodes/edges/hyperedges do, so it needs the same
     # portable-path treatment for cache entries to round-trip correctly across
     # machines/checkout directories.
+    # definition_file (#2990) is a path into the scanned tree exactly like
+    # source_file; a cache entry keeping it absolute replayed the build host's
+    # layout on every warm hit (#3223).
     for bucket in ("nodes", "edges", "hyperedges", "raw_calls"):
         for item in payload.get(bucket, []):
             if not isinstance(item, dict):
                 continue
-            source = item.get("source_file")
-            if not source:
-                continue
-            sp = Path(source)
-            if not sp.is_absolute():
-                # os.path.abspath is lexical (no symlink resolution), matching
-                # the symbolic relativization below.
-                cwd_form = Path(os.path.abspath(sp))
-                try:
-                    if cwd_form == root_resolved / sp or not cwd_form.exists():
-                        continue  # already root-relative, or a ghost path
-                except OSError:
+            for key in ("source_file", "definition_file"):
+                source = item.get(key)
+                if not source:
                     continue
-                sp = cwd_form
-            try:
-                rel = os.path.relpath(sp, root_resolved)
-            except (ValueError, OSError):
-                continue  # out-of-root (e.g. Windows cross-drive)
-            if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"):
-                continue  # escaped root — keep absolute
-            item["source_file"] = rel.replace(os.sep, "/")
+                sp = Path(source)
+                if not sp.is_absolute():
+                    # os.path.abspath is lexical (no symlink resolution),
+                    # matching the symbolic relativization below.
+                    cwd_form = Path(os.path.abspath(sp))
+                    try:
+                        if cwd_form == root_resolved / sp or not cwd_form.exists():
+                            continue  # already root-relative, or a ghost path
+                    except OSError:
+                        continue
+                    sp = cwd_form
+                try:
+                    rel = os.path.relpath(sp, root_resolved)
+                except (ValueError, OSError):
+                    continue  # out-of-root (e.g. Windows cross-drive)
+                if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"):
+                    continue  # escaped root — keep absolute
+                item[key] = rel.replace(os.sep, "/")
 
 
 def _normalize_source_file_value(src: "str | Path", root_resolved: Path) -> str:
@@ -911,16 +915,18 @@ def _absolutize_source_files_in(payload: dict, root: Path) -> None:
         for item in payload.get(bucket, []):
             if not isinstance(item, dict):
                 continue
-            source = item.get("source_file")
-            if not source:
-                continue
-            sp = Path(source)
-            if sp.is_absolute():
-                continue
-            try:
-                item["source_file"] = str(root_resolved / sp)
-            except (TypeError, OSError):
-                continue
+            # Mirror of the relativize side: definition_file re-anchors too (#3223).
+            for key in ("source_file", "definition_file"):
+                source = item.get(key)
+                if not source:
+                    continue
+                sp = Path(source)
+                if sp.is_absolute():
+                    continue
+                try:
+                    item[key] = str(root_resolved / sp)
+                except (TypeError, OSError):
+                    continue
 
 
 def cache_dir(root: Path = Path("."), kind: str = "ast",

tests/test_definition_file_portability.py

@@ -0,0 +1,89 @@
+"""`definition_file` is portable, like its sibling `source_file` (#3223).
+
+The #2990 decl/def merge stamps `definition_file` from the implementation's
+then-absolute `source_file` — and nothing ever relativized it. graph.json
+shipped `"source_file": "src/Foo.h"` beside
+`"definition_file": "/home/ci/build/.../src/Foo.cpp"`: a path no other
+machine can open, leaking the build host's layout into every consumer,
+including MCP `get_node`'s "Defined in:" line.
+"""
+from __future__ import annotations
+
+import io
+import tempfile
+from contextlib import redirect_stdout
+from pathlib import Path
+
+from graphify.build import build_from_json
+from graphify.cache import _absolutize_source_files_in, _relativize_source_files_in
+from graphify.extract import extract
+
+FOO_H = "#pragma once\n\nclass Foo {\npublic:\n    int Bar(int x);\n};\n"
+FOO_CPP = '#include "Foo.h"\n\nint Foo::Bar(int x) {\n    return x + 1;\n}\n'
+
+
+def _decl_def_graph(tmp_path):
+    src = tmp_path / "src"
+    src.mkdir()
+    (src / "Foo.h").write_text(FOO_H, encoding="utf-8")
+    (src / "Foo.cpp").write_text(FOO_CPP, encoding="utf-8")
+    with redirect_stdout(io.StringIO()):
+        r = extract([src / "Foo.h", src / "Foo.cpp"], cache_root=Path(tempfile.mkdtemp()),
+                    root=tmp_path, parallel=False)
+    G = build_from_json({"nodes": r["nodes"], "edges": r["edges"], "hyperedges": []},
+                        root=str(tmp_path))
+    return G
+
+
+def test_the_issues_repro_yields_a_relative_definition_file(tmp_path):
+    G = _decl_def_graph(tmp_path)
+    carriers = [(n, d) for n, d in G.nodes(data=True) if d.get("definition_file")]
+    assert carriers, "the decl/def pair must produce a definition_file carrier"
+    for _n, d in carriers:
+        df = str(d["definition_file"]).replace("\\", "/")
+        assert df == "src/Foo.cpp", df
+        assert str(d.get("source_file", "")).replace("\\", "/") == "src/Foo.h"
+
+
+def test_build_normalizes_a_prebuilt_absolute_definition_file(tmp_path):
+    impl = tmp_path / "src" / "Foo.cpp"
+    impl.parent.mkdir()
+    impl.write_text(FOO_CPP, encoding="utf-8")
+    G = build_from_json({
+        "nodes": [{"id": "n", "label": "Bar", "file_type": "code",
+                   "source_file": str(tmp_path / "src" / "Foo.h"),
+                   "definition_file": str(impl)}],
+        "edges": [], "hyperedges": [],
+    }, root=str(tmp_path))
+    d = G.nodes["n"]
+    assert str(d["definition_file"]).replace("\\", "/") == "src/Foo.cpp"
+
+
+def test_an_out_of_root_definition_file_is_left_alone(tmp_path):
+    outside = tmp_path.parent / "elsewhere.cpp"
+    G = build_from_json({
+        "nodes": [{"id": "n", "label": "Bar", "file_type": "code",
+                   "source_file": "src/Foo.h",
+                   "definition_file": str(outside)}],
+        "edges": [], "hyperedges": [],
+    }, root=str(tmp_path))
+    # out-of-root stays absolute; separators are normalized like source_file's
+    assert Path(G.nodes["n"]["definition_file"]) == outside
+
+
+def test_cache_round_trip_keeps_definition_file_portable(tmp_path):
+    root = tmp_path / "proj"
+    (root / "src").mkdir(parents=True)
+    f = root / "src" / "Foo.cpp"
+    f.write_text(FOO_CPP, encoding="utf-8")
+    payload = {"nodes": [{"id": "n", "source_file": str(root / "src" / "Foo.h"),
+                          "definition_file": str(f)}],
+               "edges": []}
+    _relativize_source_files_in(payload, root)
+    stored = payload["nodes"][0]
+    assert stored["definition_file"] == "src/Foo.cpp"
+    assert stored["source_file"] == "src/Foo.h"
+    _absolutize_source_files_in(payload, root)
+    restored = payload["nodes"][0]
+    assert Path(restored["definition_file"]) == root / "src" / "Foo.cpp"
+    assert Path(restored["source_file"]) == root / "src" / "Foo.h"