feat(ids): warn on legacy-id graphs + harden re-key source_file contract (#1504)
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
388d1b64db
commit
3999dbc67e
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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) == {}
|
||||
|
||||
Reference in New Issue
Block a user