From 3999dbc67e5cfbf88f73f03cfd43113616a781a3 Mon Sep 17 00:00:00 2001 From: safishamsi Date: Sun, 28 Jun 2026 17:29:57 +0100 Subject: [PATCH] feat(ids): warn on legacy-id graphs + harden re-key source_file contract (#1504) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pre-0.9.0 safety to-dos from the migration spike: 1. Auto-detect-and-warn: graph_has_legacy_ids() samples a loaded graph's node ids and detects the pre-#1504 (parent-dir/filename) scheme. Read-only consumers that don't re-extract — `graphify query` and the MCP serve loader — now print a one-line nudge to rebuild with `extract --force` when they load an old graph. Fires only on legacy graphs (verified: canonical graphs stay silent). 2. Re-key source_file contract: pinned with tests that a relative source_file is migrated to the full-path id while an absolute/unrelativizable one is skipped (its on-disk path must never leak into a persisted id). The is_absolute() guard already enforced this; the tests make the contract explicit. Full suite 2507 passed. Lands on v9. Co-Authored-By: Claude Opus 4.8 (1M context) --- graphify/__main__.py | 11 +++++++++++ graphify/build.py | 37 +++++++++++++++++++++++++++++++++++++ graphify/serve.py | 10 ++++++++++ tests/test_build.py | 26 ++++++++++++++++++++++++++ 4 files changed, 84 insertions(+) diff --git a/graphify/__main__.py b/graphify/__main__.py index b9a8b23..2bad90a 100644 --- a/graphify/__main__.py +++ b/graphify/__main__.py @@ -2867,6 +2867,17 @@ def main() -> None: G = json_graph.node_link_graph(_raw, edges="links") except TypeError: G = json_graph.node_link_graph(_raw) + try: + from graphify.build import graph_has_legacy_ids as _legacy + if _legacy(_raw.get("nodes", [])): + print( + "[graphify] note: this graph uses the pre-#1504 node-ID scheme; " + "rebuild with `graphify extract --force` to get path-qualified IDs " + "(fixes same-name-file collisions).", + file=sys.stderr, + ) + except Exception: + pass except Exception as exc: print(f"error: could not load graph: {exc}", file=sys.stderr) sys.exit(1) diff --git a/graphify/build.py b/graphify/build.py index dd0ea29..bdd56a3 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -196,6 +196,43 @@ def _semantic_id_remap(nodes: list, root: str | None) -> dict: return remap +def graph_has_legacy_ids(nodes: list, root: str | Path | None = None, sample: int = 300) -> bool: + """Whether a loaded graph still uses pre-#1504 node IDs (parent-dir / filename + stem) rather than the full repo-relative path. Read-only consumers (query, + serve) use this to nudge the user to rebuild, since they don't re-extract. + + Heuristic and cheap: samples nodes with a relative ``source_file`` and returns + True as soon as one ID matches an OLD stem form but NOT the canonical full-path + form. Absolute/sourceless nodes are skipped (can't be classified).""" + from graphify.extractors.base import _file_stem + _r = str(root) if root is not None else None + checked = 0 + for node in nodes: + if not isinstance(node, dict): + continue + nid = node.get("id") + sf = node.get("source_file") + if not nid or not isinstance(nid, str) or not sf: + continue + rel = Path(_norm_source_file(str(sf), _r) or str(sf)) + if rel.is_absolute(): + continue + new_stem = make_id(_file_stem(rel)) + if not new_stem: + continue + norm = _normalize_id(nid) + if norm == new_stem or norm.startswith(new_stem + "_"): + checked += 1 + else: + for old in _old_file_stems(rel): + if old != new_stem and (norm == old or norm.startswith(old + "_")): + return True + checked += 1 + if checked >= sample: + break + return False + + def build_from_json(extraction: dict, *, directed: bool = False, root: str | Path | None = None) -> nx.Graph: """Build a NetworkX graph from an extraction dict. diff --git a/graphify/serve.py b/graphify/serve.py index 53daf0c..f096f10 100644 --- a/graphify/serve.py +++ b/graphify/serve.py @@ -31,6 +31,16 @@ def _load_graph(graph_path: str) -> nx.Graph: if "links" not in data and "edges" in data: data = dict(data, links=data["edges"]) data = {**data, "directed": True} + try: + from graphify.build import graph_has_legacy_ids as _legacy + if _legacy(data.get("nodes", [])): + print( + "[graphify] note: this graph uses the pre-#1504 node-ID scheme; " + "rebuild with `graphify extract --force` for path-qualified IDs.", + file=sys.stderr, + ) + except Exception: + pass try: return json_graph.node_link_graph(data, edges="links") except TypeError: diff --git a/tests/test_build.py b/tests/test_build.py index 337c1d3..8a5d9dc 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -718,3 +718,29 @@ def test_build_from_json_skips_edge_with_non_hashable_endpoint(): assert G.number_of_nodes() == 2 assert G.number_of_edges() == 1 assert G.has_edge("a", "b") + + +# ── #1504 migration: legacy-id detection + re-key source_file contract ────────── + +def test_graph_has_legacy_ids_detects_old_scheme(): + """The read-only-consumer nudge (query/serve) flags a pre-#1504 graph and + leaves a canonical one alone.""" + from graphify.build import graph_has_legacy_ids + old = [{"id": "api_readme", "source_file": "docs/v1/api/README.md", "type": "document"}] + new = [{"id": "docs_v1_api_readme", "source_file": "docs/v1/api/README.md", "type": "document"}] + assert graph_has_legacy_ids(old, root=".") is True + assert graph_has_legacy_ids(new, root=".") is False + # sourceless / top-level nodes don't false-positive + assert graph_has_legacy_ids([{"id": "setup", "source_file": "setup.py"}], root=".") is False + assert graph_has_legacy_ids([{"id": "x", "label": "y"}], root=".") is False + + +def test_semantic_rekey_relative_vs_absolute_source_file(): + """Re-key contract: a relative source_file is migrated; an absolute one is left + untouched (it can't be relativized, so its on-disk path must not leak into IDs).""" + from graphify.build import _semantic_id_remap + rel = [{"id": "api_readme", "source_file": "docs/v1/api/README.md", "type": "document"}] + assert _semantic_id_remap(rel, ".") == {"api_readme": "docs_v1_api_readme"} + # absolute path with no resolvable root → skipped, not remapped to an abs-path id + ab = [{"id": "api_readme", "source_file": "/abs/docs/v1/api/README.md", "type": "document"}] + assert _semantic_id_remap(ab, None) == {}