diff --git a/graphify/build.py b/graphify/build.py index bb03fe1f5..494110917 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -973,6 +973,11 @@ 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) + # A merged C/C++/ObjC decl/def node also carries the definition's + # file; it is a source path like any other and must be relativized + # too, or the graph ships the build machine's absolute path. + 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()) diff --git a/graphify/extract.py b/graphify/extract.py index e015c9d71..7a8c5cf22 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -7168,24 +7168,28 @@ def _sf_entry(sf: str, sf_path: Path) -> tuple[str, str, tuple[str, ...]]: for item in all_nodes + all_edges: sf = item.get("source_file") - if not sf: - continue - sf_path = Path(sf) - if not sf_path.is_absolute(): - continue - new_sf, canonical_id, keys = _sf_entry(str(sf), sf_path) - if "id" in item: - for key in keys: - if key == canonical_id or key in ext_id_remap: - continue - if key in owned_ids and item.get("id") != key: - # The key is a real node's id minted some other way — - # renaming it (or edges onto it) would corrupt the graph. - # The node that owns it registers it itself when its own - # id IS the absolute-derived form (#2195 stub). - continue - ext_id_remap[key] = canonical_id - item["source_file"] = new_sf + if sf: + sf_path = Path(sf) + if sf_path.is_absolute(): + new_sf, canonical_id, keys = _sf_entry(str(sf), sf_path) + if "id" in item: + for key in keys: + if key == canonical_id or key in ext_id_remap: + continue + if key in owned_ids and item.get("id") != key: + # The key is a real node's id minted some other way — + # renaming it (or edges onto it) would corrupt the graph. + # The node that owns it registers it itself when its own + # id IS the absolute-derived form (#2195 stub). + continue + ext_id_remap[key] = canonical_id + item["source_file"] = new_sf + df = item.get("definition_file") + if df: + df_path = Path(df) + if df_path.is_absolute(): + new_df, _, _ = _sf_entry(str(df), df_path) + item["definition_file"] = new_df if ext_id_remap: # Bash entrypoint ids are the file-level id + "__entry" @@ -7289,6 +7293,9 @@ def _canon(nid: str) -> str: _sf = _item.get("source_file") if _sf and "\\" in str(_sf): _item["source_file"] = PurePath(_sf).as_posix() + _df = _item.get("definition_file") + if _df and "\\" in str(_df): + _item["definition_file"] = PurePath(_df).as_posix() return { "nodes": all_nodes, diff --git a/graphify/watch.py b/graphify/watch.py index fbcbbc011..d38484e84 100644 --- a/graphify/watch.py +++ b/graphify/watch.py @@ -318,22 +318,30 @@ def _changed_path_candidates(raw: Path, *, change_root: Path, watch_root: Path) return candidates +# Every stored path that names a file in the scanned tree. ``definition_file`` +# is the implementation site recorded when a C/C++/ObjC declaration and its +# definition merge into one node; it must stay repo-relative like its sibling +# ``source_file`` so a graph built on one machine reads on another. +_PORTABLE_PATH_KEYS = ("source_file", "definition_file") + + def _relativize_source_files(payload: dict, root: Path, *, scope: Path | None = None) -> None: for bucket in ("nodes", "edges", "hyperedges"): for item in payload.get(bucket, []): - source = item.get("source_file") - if not source: - continue - source_path = Path(source) - if not source_path.is_absolute(): - continue - try: - resolved = source_path.resolve() - if scope is not None and not _is_relative_to(resolved, scope): + for key in _PORTABLE_PATH_KEYS: + source = item.get(key) + if not source: + continue + source_path = Path(source) + if not source_path.is_absolute(): + continue + try: + resolved = source_path.resolve() + if scope is not None and not _is_relative_to(resolved, scope): + continue + item[key] = resolved.relative_to(root).as_posix() + except ValueError: continue - item["source_file"] = resolved.relative_to(root).as_posix() - except ValueError: - continue def _rebase_relative_source_files(payload: dict, source_root: Path, target_root: Path) -> None: @@ -342,13 +350,14 @@ def _rebase_relative_source_files(payload: dict, source_root: Path, target_root: return for bucket in ("nodes", "edges", "hyperedges"): for item in payload.get(bucket, []): - source = item.get("source_file") - if not source or Path(source).is_absolute(): - continue - try: - item["source_file"] = (source_root / source).relative_to(target_root).as_posix() - except ValueError: - continue + for key in _PORTABLE_PATH_KEYS: + source = item.get(key) + if not source or Path(source).is_absolute(): + continue + try: + item[key] = (source_root / source).relative_to(target_root).as_posix() + except ValueError: + continue class _StoredSourcePaths: diff --git a/tests/test_build.py b/tests/test_build.py index b376b173b..fe9cf4772 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -1614,6 +1614,36 @@ def test_norm_source_file_relativizes_a_posix_absolute_path(): ) == "docs/api/README.md" +def test_build_from_json_relativizes_definition_file(): + """A merged C/C++/ObjC decl/def node records where the symbol is implemented + in `definition_file`. That is a path into the scanned tree just like + `source_file`, so the graph must store it repo-relative — otherwise the + build machine's absolute path ships in graph.json and a reader on another + checkout (or the MCP `get_node` answer) points at a file that is not there.""" + from graphify.build import build_from_json + + root = "/home/ci/build/repo" + extraction = { + "nodes": [{ + "id": "foo_bar", + "label": "bar", + "type": "function", + "file_type": "code", + "_origin": "ast", + "source_file": f"{root}/src/Foo.h", + "source_location": "L10", + "definition_file": f"{root}/src/Foo.cpp", + "definition_location": "L42", + }], + "edges": [], + } + G = build_from_json(extraction, root=root) + assert G.nodes["foo_bar"]["source_file"] == "src/Foo.h" + assert G.nodes["foo_bar"]["definition_file"] == "src/Foo.cpp" + # the line number is a plain string and must survive untouched + assert G.nodes["foo_bar"]["definition_location"] == "L42" + + def test_derive_prune_root_recovers_root_from_posix_absolute_prune_sources(): """The prune-root recovery skips any prune source it thinks is relative. diff --git a/tests/test_languages.py b/tests/test_languages.py index 46dae524c..aa7d49486 100644 --- a/tests/test_languages.py +++ b/tests/test_languages.py @@ -3592,6 +3592,9 @@ def test_cpp_paired_merged_node_records_definition_site(): bar = bars[0] assert str(bar["source_file"]).endswith("Foo.h"), bar assert str(bar.get("definition_file", "")).endswith("Foo.cpp"), bar + assert not Path(bar["source_file"]).is_absolute(), bar + assert not Path(bar["definition_file"]).is_absolute(), bar + assert Path(bar["source_file"]).parent == Path(bar["definition_file"]).parent, bar assert bar.get("definition_location"), bar diff --git a/tests/test_watch.py b/tests/test_watch.py index a189446b6..9f00b5b62 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -4203,3 +4203,62 @@ def test_markdown_reconcile_does_not_suffix_match_top_level_target(tmp_path): assert _rebuild_code(corpus, no_cluster=True, acquire_lock=False) is True links = json.loads(graph_path.read_text(encoding="utf-8"))["links"] assert not any(edge.get("relation") == "references" for edge in links) + + +# --- portable paths: definition_file travels with source_file -------------- + +def test_relativize_source_files_relativizes_definition_file(tmp_path): + """`definition_file` (the implementation site recorded when a C/C++/ObjC + decl/def pair merges) names a file in the scanned tree exactly like + `source_file`, so it must be relativized too. Left absolute, the graph + carries the build machine's paths and cannot be read on another checkout.""" + from graphify.watch import _relativize_source_files + + root = tmp_path.resolve() + payload = {"nodes": [{ + "id": "foo_bar", + "source_file": str(root / "src" / "Foo.h"), + "definition_file": str(root / "src" / "Foo.cpp"), + }]} + _relativize_source_files(payload, root) + node = payload["nodes"][0] + assert node["source_file"] == "src/Foo.h" + assert node["definition_file"] == "src/Foo.cpp" + + +def test_relativize_source_files_leaves_an_outside_definition_file_alone(tmp_path): + """The scope guard applies to the new key as well: a path outside the + watched tree is left as-is rather than being forced under the root.""" + from graphify.watch import _relativize_source_files + + root = (tmp_path / "repo").resolve() + (root).mkdir() + outside = (tmp_path / "elsewhere" / "Foo.cpp").resolve() + payload = {"nodes": [{ + "id": "foo_bar", + "source_file": str(root / "Foo.h"), + "definition_file": str(outside), + }]} + _relativize_source_files(payload, root, scope=root) + node = payload["nodes"][0] + assert node["source_file"] == "Foo.h" + assert node["definition_file"] == str(outside) + + +def test_rebase_relative_source_files_rebases_definition_file(tmp_path): + """Cache-root-relative rebasing moves both keys, so a decl/def node built + under a cache root keeps a definition site that resolves from the project + root instead of pointing one directory level off.""" + from graphify.watch import _rebase_relative_source_files + + source_root = tmp_path / "cache" / "pkg" + target_root = tmp_path / "cache" + payload = {"nodes": [{ + "id": "foo_bar", + "source_file": "src/Foo.h", + "definition_file": "src/Foo.cpp", + }]} + _rebase_relative_source_files(payload, source_root, target_root) + node = payload["nodes"][0] + assert node["source_file"] == "pkg/src/Foo.h" + assert node["definition_file"] == "pkg/src/Foo.cpp"