diff --git a/graphify/__main__.py b/graphify/__main__.py index cf3aeea4..52f04310 100644 --- a/graphify/__main__.py +++ b/graphify/__main__.py @@ -28,6 +28,9 @@ def _check_skill_version(skill_dst: Path) -> None: version_file = skill_dst.parent / ".graphify_version" if not version_file.exists(): return + if not skill_dst.exists(): + print(" warning: skill dir exists but SKILL.md is missing. Run 'graphify install' to repair.") + return installed = version_file.read_text(encoding="utf-8").strip() if installed != __version__: print(f" warning: skill is from graphify {installed}, package is {__version__}. Run 'graphify install' to update.") @@ -40,9 +43,9 @@ def _refresh_all_version_stamps() -> None: but not explicitly re-installed during this upgrade. """ for cfg in _PLATFORM_CONFIG.values(): - vf = Path.home() / cfg["skill_dst"] - vf = vf.parent / ".graphify_version" - if vf.exists(): + skill_dst = Path.home() / cfg["skill_dst"] + vf = skill_dst.parent / ".graphify_version" + if skill_dst.exists(): vf.write_text(__version__, encoding="utf-8") _SETTINGS_HOOK = { @@ -182,7 +185,16 @@ def install(platform: str = "claude") -> None: else: skill_dst = Path.home() / cfg["skill_dst"] skill_dst.parent.mkdir(parents=True, exist_ok=True) - shutil.copy(skill_src, skill_dst) + tmp_dst = skill_dst.with_suffix(skill_dst.suffix + ".tmp") + try: + shutil.copy(skill_src, tmp_dst) + os.replace(tmp_dst, skill_dst) + except Exception: + try: + tmp_dst.unlink(missing_ok=True) + except OSError: + pass + raise (skill_dst.parent / ".graphify_version").write_text(__version__, encoding="utf-8") print(f" skill installed -> {skill_dst}") diff --git a/graphify/build.py b/graphify/build.py index 0c18ca4d..82384361 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -222,25 +222,25 @@ def build_merge( ) -> nx.Graph: """Load existing graph.json, merge new chunks into it, and save back. - Never replaces — only grows (or prunes deleted-file nodes via prune_sources). + Never replaces - only grows (or prunes deleted-file nodes via prune_sources). Safe to call repeatedly: existing nodes and edges are preserved. """ - from networkx.readwrite import json_graph as _jg - graph_path = Path(graph_path) if graph_path.exists(): + # Read JSON directly instead of going through node_link_graph(). + # The latter rebuilds an undirected nx.Graph and then enumerating + # edges() yields endpoints based on node insertion order, which + # silently flips directional edges (e.g. `calls`) when the callee + # was inserted before the caller. The _src/_tgt direction-preserving + # attrs are popped before saving in export.py, so going through the + # NetworkX round-trip loses direction permanently (#760). data = json.loads(graph_path.read_text(encoding="utf-8")) - try: - existing_G = _jg.node_link_graph(data, edges="links") - except TypeError: - existing_G = _jg.node_link_graph(data) - # Reconstruct as a plain extraction dict so build() can merge it - existing_nodes = [{"id": n, **existing_G.nodes[n]} for n in existing_G.nodes] - existing_edges = [ - {"source": u, "target": v, **d} for u, v, d in existing_G.edges(data=True) - ] + links_key = "links" if "links" in data else "edges" + existing_nodes = list(data.get("nodes", [])) + existing_edges = list(data.get(links_key, [])) base = [{"nodes": existing_nodes, "edges": existing_edges}] else: + existing_nodes = [] base = [] all_chunks = base + list(new_chunks) diff --git a/tests/test_build.py b/tests/test_build.py index 5c52f363..6cbe8681 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -1,6 +1,6 @@ import json from pathlib import Path -from graphify.build import build_from_json, build +from graphify.build import build_from_json, build, build_merge FIXTURES = Path(__file__).parent / "fixtures" @@ -128,3 +128,70 @@ def test_real_invalid_file_type_still_warns(capsys): err = capsys.readouterr().err assert "invalid file_type" in err assert "weird_type" in err + + +def test_build_merge_preserves_call_edge_direction(tmp_path): + """Regression for #760. + + When the callee is defined before the caller in source, NetworkX's + undirected Graph stores edges in node-insertion order. Going through + node_link_graph() + edges() during build_merge previously flipped the + `calls` edge so that on the next save source/target were swapped. + + build_merge must read the saved JSON's source/target verbatim instead + of round-tripping through NetworkX. + """ + from graphify.extract import extract_js + from graphify.export import to_json + + # Callee `b` is defined before caller `a` so node insertion order + # is b, a. An undirected Graph then yields the edge as (b, a) on + # iteration, which is the wrong direction for `calls` (a calls b). + src = "function b() {}\nfunction a() { b(); }\n" + src_file = tmp_path / "x.js" + src_file.write_text(src) + + extraction = extract_js(src_file) + assert "error" not in extraction + + # Locate the `calls` edge in the raw extraction so we know the truth. + call_edges = [e for e in extraction["edges"] if e["relation"] == "calls"] + assert len(call_edges) == 1, "expected exactly one calls edge from the snippet" + truth_src = call_edges[0]["source"] + truth_tgt = call_edges[0]["target"] + + nodes_by_id = {n["id"]: n for n in extraction["nodes"]} + assert nodes_by_id[truth_src]["label"].startswith("a") + assert nodes_by_id[truth_tgt]["label"].startswith("b") + + # First build + save. + G1 = build([extraction], dedup=False) + graph_path = tmp_path / "graph.json" + communities: dict = {} + assert to_json(G1, communities, str(graph_path), force=True) + + # Verify direction is correct in the freshly written JSON. + saved = json.loads(graph_path.read_text()) + saved_calls = [e for e in saved.get("links", saved.get("edges", [])) + if e.get("relation") == "calls"] + assert len(saved_calls) == 1 + assert saved_calls[0]["source"] == truth_src + assert saved_calls[0]["target"] == truth_tgt + + # Now simulate `--update` with no new chunks — load + re-save. + G2 = build_merge([], graph_path, dedup=False) + assert to_json(G2, communities, str(graph_path), force=True) + + # The calls edge must still go a -> b, not b -> a. + reloaded = json.loads(graph_path.read_text()) + reloaded_calls = [e for e in reloaded.get("links", reloaded.get("edges", [])) + if e.get("relation") == "calls"] + assert len(reloaded_calls) == 1 + assert reloaded_calls[0]["source"] == truth_src, ( + f"calls edge source flipped after build_merge round-trip: " + f"expected {truth_src} (a), got {reloaded_calls[0]['source']}" + ) + assert reloaded_calls[0]["target"] == truth_tgt, ( + f"calls edge target flipped after build_merge round-trip: " + f"expected {truth_tgt} (b), got {reloaded_calls[0]['target']}" + )