mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-08 06:35:56 +00:00
fix(merge-chunks): fail when no chunk validates
This commit is contained in:
committed by
safishamsi
parent
dea6ec0c24
commit
5a480a83a7
+18
-2
@@ -3158,12 +3158,15 @@ def dispatch_command(cmd: str) -> None:
|
||||
chunk_files.extend(sorted(expanded) if expanded else [arg])
|
||||
merged: dict = {"nodes": [], "edges": [], "hyperedges": [], "input_tokens": 0, "output_tokens": 0}
|
||||
seen_ids: set[str] = set()
|
||||
valid_chunks = 0
|
||||
# These chunk files are untrusted subagent output. load_validated_...
|
||||
# stats the file size BEFORE reading it (so a multi-GB chunk can't blow up
|
||||
# memory), parses the JSON, and validates the security caps + the node/
|
||||
# edge id charset that blocks path traversal (#825) — the same enforcement
|
||||
# the skill merge path applies. A bad chunk is skipped with a warning
|
||||
# (filter semantics; never abort). Deliberately NOT wired into
|
||||
# while valid siblings still merge; if every chunk is invalid, fail
|
||||
# closed instead of reporting success and replacing --out with an empty
|
||||
# semantic layer. Deliberately NOT wired into
|
||||
# build_from_json/load_graph_json, which must keep loading valid
|
||||
# pre-existing graphs. file_type is left to build's coercion (#840).
|
||||
from graphify.semantic_cleanup import load_validated_semantic_fragment
|
||||
@@ -3176,6 +3179,7 @@ def dispatch_command(cmd: str) -> None:
|
||||
file=sys.stderr,
|
||||
)
|
||||
continue
|
||||
valid_chunks += 1
|
||||
for n in chunk.get("nodes", []):
|
||||
if n.get("id") not in seen_ids:
|
||||
seen_ids.add(n["id"])
|
||||
@@ -3188,11 +3192,23 @@ def dispatch_command(cmd: str) -> None:
|
||||
for _tok in ("input_tokens", "output_tokens"):
|
||||
_v = chunk.get(_tok, 0)
|
||||
merged[_tok] += _v if isinstance(_v, (int, float)) else 0
|
||||
if not valid_chunks:
|
||||
print(
|
||||
f"[graphify merge-chunks] error: no valid chunks to merge; "
|
||||
f"refusing to write {out_path}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
out_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
from graphify.paths import write_json_atomic as _wja
|
||||
_wja(out_path, merged, ensure_ascii=False)
|
||||
chunk_summary = (
|
||||
f"{valid_chunks} chunks"
|
||||
if valid_chunks == len(chunk_files)
|
||||
else f"{valid_chunks} of {len(chunk_files)} chunks"
|
||||
)
|
||||
print(
|
||||
f"Merged {len(chunk_files)} chunks: {len(merged['nodes'])} nodes, {len(merged['edges'])} edges, "
|
||||
f"Merged {chunk_summary}: {len(merged['nodes'])} nodes, {len(merged['edges'])} edges, "
|
||||
f"{merged['input_tokens']:,} in / {merged['output_tokens']:,} out tokens"
|
||||
)
|
||||
|
||||
|
||||
@@ -3,11 +3,12 @@
|
||||
merge-chunks concatenates agent-written `.graphify_chunk_*.json` files. Those are
|
||||
untrusted output, so each is run through `validate_semantic_fragment` (caps + the
|
||||
node/edge ID charset that blocks path-escape). An invalid chunk is skipped with a
|
||||
warning; valid chunks still merge.
|
||||
warning; valid chunks still merge, but an all-invalid input set fails closed.
|
||||
"""
|
||||
import json
|
||||
|
||||
import graphify.__main__ as mainmod
|
||||
import pytest
|
||||
|
||||
|
||||
def _write(path, obj):
|
||||
@@ -32,19 +33,64 @@ def test_merge_chunks_skips_chunk_with_path_escape_id(tmp_path, monkeypatch, cap
|
||||
|
||||
merged = json.loads(out.read_text())
|
||||
assert {n["id"] for n in merged["nodes"]} == {"pkg.mod.good"}
|
||||
assert "skipping invalid chunk" in capsys.readouterr().err
|
||||
captured = capsys.readouterr()
|
||||
assert "skipping invalid chunk" in captured.err
|
||||
assert "Merged 1 of 2 chunks" in captured.out
|
||||
|
||||
|
||||
def test_merge_chunks_skips_malformed_shape(tmp_path, monkeypatch, capsys):
|
||||
def test_merge_chunks_fails_closed_when_every_chunk_is_invalid(tmp_path, monkeypatch, capsys):
|
||||
bad = tmp_path / ".graphify_chunk_0.json"
|
||||
_write(bad, {"nodes": "not-a-list", "edges": []})
|
||||
out = tmp_path / "merged.json"
|
||||
out.write_text('{"previous": "semantic result"}', encoding="utf-8")
|
||||
|
||||
_run_merge(monkeypatch, ["graphify", "merge-chunks", str(bad), "--out", str(out)])
|
||||
with pytest.raises(SystemExit) as exc:
|
||||
_run_merge(monkeypatch, ["graphify", "merge-chunks", str(bad), "--out", str(out)])
|
||||
|
||||
assert exc.value.code == 1
|
||||
assert json.loads(out.read_text()) == {"previous": "semantic result"}
|
||||
err = capsys.readouterr().err
|
||||
assert "skipping invalid chunk" in err
|
||||
assert "no valid chunks to merge" in err
|
||||
|
||||
|
||||
def test_merge_chunks_accepts_valid_empty_chunk(tmp_path, monkeypatch):
|
||||
"""A valid fragment may legitimately contain no entities; it still counts."""
|
||||
empty = tmp_path / ".graphify_chunk_0.json"
|
||||
_write(empty, {"nodes": [], "edges": [], "hyperedges": []})
|
||||
out = tmp_path / "merged.json"
|
||||
|
||||
_run_merge(monkeypatch, ["graphify", "merge-chunks", str(empty), "--out", str(out)])
|
||||
|
||||
merged = json.loads(out.read_text())
|
||||
assert merged["nodes"] == []
|
||||
assert "skipping invalid chunk" in capsys.readouterr().err
|
||||
assert merged["edges"] == []
|
||||
|
||||
|
||||
def test_merge_chunks_fails_closed_without_chunk_arguments(tmp_path, monkeypatch, capsys):
|
||||
out = tmp_path / "merged.json"
|
||||
|
||||
with pytest.raises(SystemExit) as exc:
|
||||
_run_merge(monkeypatch, ["graphify", "merge-chunks", "--out", str(out)])
|
||||
|
||||
assert exc.value.code == 1
|
||||
assert not out.exists()
|
||||
assert "no valid chunks to merge" in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_merge_chunks_fails_closed_on_unmatched_glob(tmp_path, monkeypatch, capsys):
|
||||
out = tmp_path / "merged.json"
|
||||
out.write_text('{"previous": true}', encoding="utf-8")
|
||||
unmatched = str(tmp_path / ".graphify_chunk_*.json")
|
||||
|
||||
with pytest.raises(SystemExit) as exc:
|
||||
_run_merge(monkeypatch, ["graphify", "merge-chunks", unmatched, "--out", str(out)])
|
||||
|
||||
assert exc.value.code == 1
|
||||
assert json.loads(out.read_text()) == {"previous": True}
|
||||
err = capsys.readouterr().err
|
||||
assert "skipping invalid chunk" in err
|
||||
assert "no valid chunks to merge" in err
|
||||
|
||||
|
||||
def test_merge_chunks_accepts_synonym_file_type(tmp_path, monkeypatch):
|
||||
|
||||
Reference in New Issue
Block a user