diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d659f4..6c5474a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Full release notes with details on each version: [GitHub Releases](https://githu ## Unreleased +- Fix: `graphify --update` no longer drops hyperedges from unchanged files (#1574, thanks @socar-tender). `build_merge` read only nodes and edges from the existing `graph.json`, never hyperedges — so every incremental update collapsed the graph's hyperedge set (the semantic domain-flow groupings) down to just the re-extracted files'. Existing hyperedges are now carried forward: re-extracted files' prior hyperedges are replaced by their new version (by `source_file`), deleted files' are pruned, and the rest are preserved with id-dedup — mirroring how `watch` already handled it. +- Fix: `graphify --update` no longer leaves ghost nodes for deleted files when `build_merge` is called without `root` (#1571, thanks @goodjira). Absolute `prune_sources` paths (from `detect_incremental`) never relativized to match the stored relative `source_file` keys, so deleted files' nodes survived the prune. `build_merge` now infers a fallback root when none is passed — the committed `graphify-out/.graphify_root` marker, else the output dir's parent — so pruning (and re-extract replacement) work regardless of the caller. The shipped `--update` runbooks already pass `root`; this hardens the library for any caller that doesn't. - Feat: extend `indirect_call` to assignment and return references (#1569, #1566 slice 2, thanks @sheik-hiiobd). A function bound to a name (`cb = handler`), returned from a factory (`def make(): return handler`), or aliased at module level (`CALLBACK = handler`) now emits an `indirect_call` edge, so `affected` reaches it. Captures the value side only (a bare name or a bare unpack `a, b = f, g`); a collection literal on the RHS stays with the dispatch-table scan. Reuses the shared guard, so the inverted-shadow trap is handled by construction — a param/local named on the RHS still hits the shadow guard and emits nothing (no return of #1565's false edges). Function and module scope; Python only for now. - Fix: the skill-version mismatch warning is now direction-aware (#1568, thanks @TPAteeq). It used to advise `Run 'graphify install' to update` on ANY version difference, but `install` writes the package's own bundled skill and re-stamps the version — so when the skill on disk was NEWER than the package (a stale `uv tool` CLI, or a contributor's dev checkout), following that advice silently DOWNGRADED the skill to make the warning go away. Now when the skill is newer, the warning recommends upgrading the package (`uv tool upgrade graphifyy` / `pip install -U graphifyy`) instead; the older-skill case still recommends `install`. Versions compare numerically (so `0.10` > `0.9`). - Feat: extend `indirect_call` capture to JS/TS (#1566). The same model now applies to JavaScript and TypeScript: a callback passed by name (`arr.map(fn)`, `setTimeout(fn)`, Express-style `app.get("/", handler)`, event wiring `emitter.on("e", handler)`) and functions listed in object/array dispatch tables (`const ROUTES = { create: handler }`, `const HOOKS = [onStart, onStop]`). Arrow-const functions (`const cb = () => {}`) count as callable targets; object shorthand (`{ handler }`) is a reference; inline arrows/function expressions are direct definitions and are not captured; object KEYS and non-callable values are excluded. Same guards as Python: callable-target-only, not shadowed by a param/local/module reassignment, single-definition god-node guard cross-file. Cross-file resolution is import-aware — a `import { onEvent }` edge to the symbol no longer suppresses the `indirect_call` to it. Module-level call-argument registration (idiomatic in JS) is captured in addition to the function-scoped capture Python has. diff --git a/graphify/build.py b/graphify/build.py index eb7de41..258036f 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -121,6 +121,30 @@ def _norm_source_file(p: str | None, root: str | None = None) -> str | None: return p +def _infer_merge_root(graph_path: Path) -> str | None: + """Best-effort scan root for relativizing paths in build_merge when the caller + passes no ``root`` (#1571). + + Prefers the committed ``graphify-out/.graphify_root`` marker — the authoritative + scan root graphify records at build/watch time (#686/#1423) — then falls back to + the directory that contains the output dir (``graph.json``'s grandparent, i.e. + ``/graphify-out/graph.json`` -> ````). Returns None if neither + resolves, in which case normalization is a no-op (prior behavior). + """ + try: + marker = graph_path.parent / ".graphify_root" + if marker.exists(): + recorded = marker.read_text(encoding="utf-8").strip() + if recorded: + return str(Path(recorded).resolve()) + except OSError: + pass + try: + return str(graph_path.parent.parent.resolve()) + except Exception: + return None + + def edge_data(G: nx.Graph, u: str, v: str) -> dict: """Return one edge attribute dict for (u, v), tolerating MultiGraph. @@ -718,12 +742,26 @@ def build_merge( links_key = "links" if "links" in data else "edges" existing_nodes = list(data.get("nodes", [])) existing_edges = list(data.get(links_key, [])) + existing_hyperedges = list(data.get("hyperedges", [])) had_graph = True else: existing_nodes = [] existing_edges = [] + existing_hyperedges = [] had_graph = False + # Effective root for relativizing absolute source_file / prune paths back to the + # stored relative source_file keys. When the caller passes root we use it; + # otherwise fall back to the graph's recorded scan root, so absolute + # prune_sources and new-chunk paths still match even when a caller omits root + # (#1571 — the skill's --update runbook calls build_merge without root, so + # absolute deleted-file paths never matched the relative node keys and their + # nodes survived as ghosts). + _eff_root = ( + str(Path(root).resolve()) if root is not None + else _infer_merge_root(graph_path) + ) + # Re-extracted files REPLACE their prior contribution. Every source_file # present in new_chunks is dropped from the loaded base before merging, so a # CHANGED file's stale nodes/edges don't accumulate across incremental @@ -733,7 +771,7 @@ def build_merge( # for them; genuinely deleted files are still handled via prune_sources. # Matched in both raw and _norm_source_file form because new_chunks may carry # absolute win32 paths while the stored graph keeps relative posix (#1007). - _replace_root = str(Path(root).resolve()) if root is not None else None + _replace_root = _eff_root new_sources: set[str] = set() for ch in new_chunks: for n in ch.get("nodes", []): @@ -756,23 +794,46 @@ def build_merge( all_chunks = base + list(new_chunks) G = build(all_chunks, directed=directed, dedup=dedup, dedup_llm_backend=dedup_llm_backend, root=root) + # Prune set for deleted source files — both the raw form (matches nodes that + # kept absolute source_file) and the normalised relative form (matches nodes + # relativised by _norm_source_file at build time). .resolve() (via _eff_root) + # handles symlinked roots and ".." / "./" segments so Path.relative_to() + # succeeds even when the scan root is a symlink. (#1007, #1571) + prune_set: set[str] = set() + for p in (prune_sources or []): + if not p: + continue + prune_set.add(p) + norm = _norm_source_file(p, _eff_root) + if norm: + prune_set.add(norm) + + # Carry forward hyperedges from files that were neither re-extracted nor + # deleted (#1574). build() only sees the new chunks' hyperedges, so without + # this every --update collapses the graph's hyperedge set down to just the + # changed files'. Re-extracted files' prior hyperedges are dropped (their new + # version is already in G — replace-per-source, like nodes/edges); deleted + # files' are dropped via prune_set. id-dedup (attach_hyperedges) so a carried + # hyperedge never duplicates one the new chunks re-emitted. Mirrors watch.py, + # which already preserves existing hyperedges across a rebuild. + if existing_hyperedges: + carried = [] + for he in existing_hyperedges: + if not isinstance(he, dict): + continue + sf = he.get("source_file") + norm = _norm_source_file(sf, _eff_root) + if sf in new_sources or norm in new_sources: + continue # re-extracted — replaced by the new chunk's version + if sf in prune_set or norm in prune_set: + continue # deleted — pruned + carried.append(he) + if carried: + from graphify.export import attach_hyperedges + attach_hyperedges(G, carried) + # Prune nodes and edges from deleted source files if prune_sources: - # Build a set containing both the raw form (matches nodes that kept - # absolute source_file) and the normalised relative form (matches nodes - # that were relativised by _norm_source_file at build time). - # .resolve() handles symlinked roots and redundant ".." / "./" segments - # so Path.relative_to() succeeds even when the scan root is a symlink. - # (#1007: manifest absolute paths vs graph relative source_file mismatch) - _root_str = str(Path(root).resolve()) if root is not None else None - prune_set: set[str] = set() - for p in prune_sources: - if not p: - continue - prune_set.add(p) - norm = _norm_source_file(p, _root_str) - if norm: - prune_set.add(norm) to_remove = [ n for n, d in G.nodes(data=True) if d.get("source_file") in prune_set diff --git a/tests/test_build_merge_hyperedges_and_prune.py b/tests/test_build_merge_hyperedges_and_prune.py new file mode 100644 index 0000000..dba0d68 --- /dev/null +++ b/tests/test_build_merge_hyperedges_and_prune.py @@ -0,0 +1,126 @@ +"""Incremental --update: hyperedge preservation (#1574) and root-less prune (#1571). + +build_merge backs `graphify --update`. Two regressions covered here: + +- #1574: it read only nodes+edges from the existing graph.json, never hyperedges, + so every incremental update collapsed the graph's hyperedge set down to just the + re-extracted files'. Now existing hyperedges are carried forward, with + re-extracted files' replaced (by source_file) and deleted files' pruned. +- #1571: when a caller omits `root` (the skill's --update runbook does), absolute + prune_sources never relativized to match the stored relative source_file keys, so + deleted files' nodes survived as ghosts. build_merge now infers a fallback root. +""" +from __future__ import annotations + +import json +from pathlib import Path + +from graphify.build import build_merge, _infer_merge_root + + +def _write_graph(graph_path: Path, nodes, edges, hyperedges) -> None: + """Write a graph.json in the shape to_json emits (top-level hyperedges).""" + graph_path.write_text( + json.dumps({"nodes": nodes, "edges": edges, "hyperedges": hyperedges}), + encoding="utf-8", + ) + + +def _he_ids(G) -> set[str]: + return {h["id"] for h in G.graph.get("hyperedges", [])} + + +# ── #1574: hyperedge preservation ───────────────────────────────────────────── + +def _seed_two_file_graph(tmp_path): + root = tmp_path / "corpus" + root.mkdir() + graph_path = tmp_path / "graph.json" + nodes = [ + {"id": "a1", "label": "a1", "file_type": "document", "source_file": "a.md"}, + {"id": "b1", "label": "b1", "file_type": "document", "source_file": "b.md"}, + ] + hyperedges = [ + {"id": "he_a", "label": "flow A", "source_file": "a.md", "nodes": ["a1"]}, + {"id": "he_b", "label": "flow B", "source_file": "b.md", "nodes": ["b1"]}, + {"id": "he_global", "label": "cross-file flow", "nodes": ["a1", "b1"]}, # no source_file + ] + _write_graph(graph_path, nodes, [], hyperedges) + return root, graph_path + + +def test_update_preserves_hyperedges_of_unchanged_files(tmp_path): + root, graph_path = _seed_two_file_graph(tmp_path) + # Re-extract only b.md, with a fresh hyperedge for it. + new_chunk = { + "nodes": [{"id": "b1", "label": "b1", "file_type": "document", "source_file": "b.md"}], + "edges": [], + "hyperedges": [{"id": "he_b_v2", "label": "flow B v2", "source_file": "b.md", "nodes": ["b1"]}], + } + G = build_merge([new_chunk], graph_path, dedup=False, root=root) + ids = _he_ids(G) + assert "he_a" in ids # unchanged file's hyperedge preserved (the bug) + assert "he_global" in ids # source_file-less hyperedge preserved + assert "he_b_v2" in ids # re-extracted file's new hyperedge present + assert "he_b" not in ids # re-extracted file's OLD hyperedge replaced + + +def test_update_without_root_still_preserves_hyperedges(tmp_path): + """The runbook omits root; the fallback root must not break preservation.""" + root, graph_path = _seed_two_file_graph(tmp_path) + new_chunk = { + "nodes": [{"id": "b1", "label": "b1", "file_type": "document", "source_file": "b.md"}], + "edges": [], + "hyperedges": [{"id": "he_b_v2", "source_file": "b.md", "nodes": ["b1"]}], + } + G = build_merge([new_chunk], graph_path, dedup=False) # no root + ids = _he_ids(G) + assert {"he_a", "he_global", "he_b_v2"} <= ids + assert "he_b" not in ids + + +def test_deleted_file_hyperedges_are_pruned(tmp_path): + root, graph_path = _seed_two_file_graph(tmp_path) + deleted_abs = [str(root / "a.md")] + G = build_merge([], graph_path, prune_sources=deleted_abs, dedup=False, root=root) + ids = _he_ids(G) + assert "he_a" not in ids # deleted file's hyperedge pruned + assert "he_b" in ids # untouched file's hyperedge kept + assert "he_global" in ids # global hyperedge kept + # and its node is gone too + assert "a1" not in set(G.nodes) + + +# ── #1571: root-less prune (absolute deleted paths vs relative node keys) ────── + +def test_prune_without_root_removes_ghost_nodes_via_grandparent_fallback(tmp_path): + root = tmp_path / "corpus" + (root / "graphify-out").mkdir(parents=True) + graph_path = root / "graphify-out" / "graph.json" + nodes = [ + {"id": "h1", "label": "handoff", "file_type": "document", "source_file": "HANDOFF.md"}, + {"id": "k1", "label": "keep", "file_type": "document", "source_file": "KEEP.md"}, + ] + _write_graph(graph_path, nodes, [], []) + # Runbook-style call: absolute prune path, NO root passed. + deleted_abs = [str(root / "HANDOFF.md")] + G = build_merge([], graph_path, prune_sources=deleted_abs, dedup=False) + labels = {d["label"] for _, d in G.nodes(data=True)} + assert "handoff" not in labels, "deleted file's ghost node must be pruned without root" + assert "keep" in labels + + +def test_prune_without_root_uses_graphify_root_marker(tmp_path): + # graph.json not under a /graphify-out layout, so grandparent wouldn't + # help — the committed .graphify_root marker must be honored instead. + out = tmp_path / "out" + out.mkdir() + graph_path = out / "graph.json" + real_root = tmp_path / "elsewhere" / "repo" + real_root.mkdir(parents=True) + (out / ".graphify_root").write_text(str(real_root), encoding="utf-8") + nodes = [{"id": "h1", "label": "handoff", "file_type": "document", "source_file": "HANDOFF.md"}] + _write_graph(graph_path, nodes, [], []) + assert _infer_merge_root(graph_path) == str(real_root.resolve()) + G = build_merge([], graph_path, prune_sources=[str(real_root / "HANDOFF.md")], dedup=False) + assert "handoff" not in {d["label"] for _, d in G.nodes(data=True)}