fix(extract): extend the incomplete-build shrink guard to the --no-cluster path
The clustered write is guarded by to_json's #479 shrink check, but the `--no-cluster` raw-dump path writes graph.json directly and had no guard, so an incomplete `--no-cluster` build could still overwrite a larger complete graph with a partial one — the residual gap noted in the original change. Add `existing_graph_node_count` in graphify.export (mirrors to_json's guard and respects the graph-size cap) and, on the raw path, refuse the write with exit 1 before the manifest when the build was incomplete and the new graph has fewer nodes than the existing one — unless --allow-partial is passed. Both write paths now enforce the same guarantee.
This commit is contained in:
+25
-5
@@ -2762,7 +2762,10 @@ def dispatch_command(cmd: str) -> None:
|
||||
# across modes (#1317; node dedup also collapses shared Swift module
|
||||
# anchors emitted per importing file, #1327).
|
||||
from graphify.build import dedupe_edges as _dedupe_edges, dedupe_nodes as _dedupe_nodes
|
||||
from graphify.export import backup_if_protected as _backup
|
||||
from graphify.export import (
|
||||
backup_if_protected as _backup,
|
||||
existing_graph_node_count as _existing_graph_node_count,
|
||||
)
|
||||
if (
|
||||
incremental_mode
|
||||
and not code_files
|
||||
@@ -2808,6 +2811,24 @@ def dispatch_command(cmd: str) -> None:
|
||||
_e["source_file"] = (
|
||||
_node_sf.get(_e.get("source")) or _node_sf.get(_e.get("target")) or ""
|
||||
)
|
||||
# RT-parity for the raw path: an incomplete build must not force a
|
||||
# partial graph over a larger complete one here either. The clustered
|
||||
# path gets this from to_json's #479 guard; this path never calls
|
||||
# to_json, so replicate the shrink check against the existing file and
|
||||
# exit before the write/manifest unless --allow-partial is set.
|
||||
if _extraction_incomplete and not cli_allow_partial:
|
||||
_existing_n = _existing_graph_node_count(graph_json_path)
|
||||
if _existing_n is not None and len(merged["nodes"]) < _existing_n:
|
||||
print(
|
||||
"[graphify extract] error: extraction was incomplete (an AST/"
|
||||
"semantic pass failed) and the resulting --no-cluster graph is "
|
||||
f"smaller than the existing {graph_json_path} "
|
||||
f"({len(merged['nodes'])} < {_existing_n} nodes). Refusing to "
|
||||
"overwrite a complete graph with a partial one. Re-run after "
|
||||
"fixing the failures, or pass --allow-partial to overwrite anyway.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
_backup(graphify_out)
|
||||
graph_json_path.write_text(
|
||||
json.dumps(merged, indent=2), encoding="utf-8"
|
||||
@@ -2908,10 +2929,9 @@ def dispatch_command(cmd: str) -> None:
|
||||
# graph could silently overwrite a good complete one, so fall back to the
|
||||
# shrink guard (force=False) unless the user opts in with --allow-partial.
|
||||
#
|
||||
# Scope: this guards the clustered write path only. The `--no-cluster`
|
||||
# raw-dump path (above) writes graph.json directly and has never had a
|
||||
# shrink guard, so an incomplete --no-cluster run can still overwrite a
|
||||
# complete graph — a pre-existing gap this change does not close.
|
||||
# Both write paths are guarded: the clustered path here via to_json's
|
||||
# #479 check, and the `--no-cluster` raw-dump path above via the same
|
||||
# shrink check against the existing file (existing_graph_node_count).
|
||||
#
|
||||
# Trade-off: this reuses to_json's coarse node-count guard, not the
|
||||
# source-aware _check_shrink that watch/update use. On an incremental run
|
||||
|
||||
@@ -180,6 +180,38 @@ def _git_head() -> str | None:
|
||||
return None
|
||||
|
||||
|
||||
def existing_graph_node_count(path: "str | Path") -> int | None:
|
||||
"""Node count of an existing graph.json, or None when it can't be compared
|
||||
(absent, empty, unreadable, malformed, or over the size cap).
|
||||
|
||||
The raw ``--no-cluster`` write path uses this to apply the same #479 shrink
|
||||
guard that :func:`to_json` applies inline for the clustered path. None means
|
||||
"can't verify — let the write proceed", matching how ``to_json`` treats an
|
||||
empty/oversized/unreadable existing file.
|
||||
"""
|
||||
p = Path(path)
|
||||
if not p.exists():
|
||||
return None
|
||||
from graphify.security import check_graph_file_size_cap
|
||||
try:
|
||||
check_graph_file_size_cap(p)
|
||||
except Exception:
|
||||
# Oversized: reading it to compare would be the DoS the cap guards against.
|
||||
return None
|
||||
try:
|
||||
raw = p.read_text(encoding="utf-8")
|
||||
except Exception:
|
||||
return None
|
||||
if not raw.strip():
|
||||
return None
|
||||
try:
|
||||
data = json.loads(raw)
|
||||
except Exception:
|
||||
return None
|
||||
nodes = data.get("nodes") if isinstance(data, dict) else None
|
||||
return len(nodes) if isinstance(nodes, list) else None
|
||||
|
||||
|
||||
def to_json(G: nx.Graph, communities: dict[int, list[str]], output_path: str, *, force: bool = False, built_at_commit: str | None = None, community_labels: dict[int, str] | None = None) -> bool:
|
||||
# Safety check: refuse to silently shrink an existing graph (#479)
|
||||
existing_path = Path(output_path)
|
||||
|
||||
@@ -775,3 +775,15 @@ def test_to_html_handles_null_source_file_and_label(tmp_path):
|
||||
out = tmp_path / "graph.html"
|
||||
to_html(G, {0: ["n1", "n2", "n3"]}, str(out))
|
||||
assert out.exists() and out.stat().st_size > 0
|
||||
|
||||
|
||||
def test_existing_graph_node_count(tmp_path):
|
||||
from graphify.export import existing_graph_node_count
|
||||
p = tmp_path / "graph.json"
|
||||
assert existing_graph_node_count(p) is None # absent
|
||||
p.write_text("", encoding="utf-8")
|
||||
assert existing_graph_node_count(p) is None # empty
|
||||
p.write_text("{not json", encoding="utf-8")
|
||||
assert existing_graph_node_count(p) is None # malformed
|
||||
p.write_text('{"nodes": [{"id": "a"}, {"id": "b"}], "links": []}', encoding="utf-8")
|
||||
assert existing_graph_node_count(p) == 2 # valid
|
||||
|
||||
@@ -108,3 +108,62 @@ def test_complete_extraction_keeps_force_write(monkeypatch, tmp_path):
|
||||
mainmod.main()
|
||||
|
||||
assert rec["called"] and rec["force"] is True
|
||||
|
||||
|
||||
def _seed_existing_graph(gout, n):
|
||||
import json
|
||||
gout.mkdir(parents=True, exist_ok=True)
|
||||
(gout / "graph.json").write_text(
|
||||
json.dumps({"nodes": [{"id": f"keep{i}", "label": f"k{i}"} for i in range(n)],
|
||||
"links": []}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
def _arm_no_cluster(monkeypatch, tmp_path, *, extra_argv=()):
|
||||
corpus = _make_docs_corpus(tmp_path)
|
||||
out_dir = tmp_path / "out"
|
||||
gout = out_dir / "graphify-out"
|
||||
_seed_existing_graph(gout, 5) # existing complete graph
|
||||
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-test-fake-key")
|
||||
|
||||
def _stub_corpus(paths, **kwargs):
|
||||
on_chunk = kwargs.get("on_chunk_done")
|
||||
if on_chunk:
|
||||
on_chunk(0, 3, {"nodes": [], "edges": [], "hyperedges": []}) # 1 of 3 -> partial
|
||||
return {"nodes": [{"id": "s1", "source_file": str(corpus / "README.md"),
|
||||
"file_type": "document", "label": "Notes"}],
|
||||
"edges": [], "hyperedges": [], "input_tokens": 1, "output_tokens": 1}
|
||||
|
||||
monkeypatch.setattr("graphify.llm.extract_corpus_parallel", _stub_corpus)
|
||||
monkeypatch.setattr(mainmod, "_check_skill_version", lambda _: None)
|
||||
monkeypatch.setattr(
|
||||
mainmod.sys, "argv",
|
||||
["graphify", "extract", str(corpus), "--backend", "claude", "--no-cluster",
|
||||
"--out", str(out_dir), *extra_argv],
|
||||
)
|
||||
return gout / "graph.json"
|
||||
|
||||
|
||||
def test_no_cluster_incomplete_build_refuses_to_shrink(tmp_path, monkeypatch, capsys):
|
||||
import json
|
||||
graph = _arm_no_cluster(monkeypatch, tmp_path)
|
||||
|
||||
with pytest.raises(SystemExit) as exc:
|
||||
mainmod.main()
|
||||
|
||||
assert exc.value.code == 1
|
||||
assert "Refusing to overwrite" in capsys.readouterr().err
|
||||
# The existing 5-node graph is untouched — the partial 1-node graph was refused.
|
||||
assert len(json.loads(graph.read_text())["nodes"]) == 5
|
||||
|
||||
|
||||
def test_no_cluster_allow_partial_overwrites(tmp_path, monkeypatch):
|
||||
import json
|
||||
graph = _arm_no_cluster(monkeypatch, tmp_path, extra_argv=["--allow-partial"])
|
||||
|
||||
with pytest.raises(SystemExit) as exc:
|
||||
mainmod.main()
|
||||
|
||||
assert exc.value.code == 0 # the raw --no-cluster path exits 0 on success
|
||||
assert len(json.loads(graph.read_text())["nodes"]) == 1
|
||||
|
||||
Reference in New Issue
Block a user