fix(cli): surface four silent success-exit failures (#2534)
cluster-only warns when --backend/--model/--batch-size are ignored on the label-reuse path; the community-label prompt key no longer collides with the discard sentinel (an echoed key was silently dropped); tree --root exits non-zero when it matches no source file instead of flattening the tree; and cluster-only stamps built_at_commit from the analysed graph, not the shell cwd. Also folds in the cluster-only refused-write guard from PR #2522 (thanks @aniJani). Thanks @elecnix for the report. 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
9f25a3aaa1
commit
6ba0868228
+66
-16
@@ -1638,6 +1638,12 @@ def dispatch_command(cmd: str) -> None:
|
||||
co_exclude_hubs: float | None = None
|
||||
label_max_concurrency: int = 4
|
||||
label_batch_size: int = 100
|
||||
# #2534: defaults make presence undetectable for these, so track whether
|
||||
# the user actually passed them — the label-reuse branch below warns when
|
||||
# labeling flags are silently ignored. (--backend/--model default to None,
|
||||
# so `is not None` already means "explicitly passed" for those.)
|
||||
label_max_concurrency_explicit = False
|
||||
label_batch_size_explicit = False
|
||||
i_arg = 0
|
||||
while i_arg < len(args):
|
||||
a = args[i_arg]
|
||||
@@ -1660,13 +1666,13 @@ def dispatch_command(cmd: str) -> None:
|
||||
elif a.startswith("--exclude-hubs="):
|
||||
co_exclude_hubs = float(a.split("=", 1)[1]); i_arg += 1
|
||||
elif a == "--max-concurrency" and i_arg + 1 < len(args):
|
||||
label_max_concurrency = int(args[i_arg + 1]); i_arg += 2
|
||||
label_max_concurrency = int(args[i_arg + 1]); label_max_concurrency_explicit = True; i_arg += 2
|
||||
elif a.startswith("--max-concurrency="):
|
||||
label_max_concurrency = int(a.split("=", 1)[1]); i_arg += 1
|
||||
label_max_concurrency = int(a.split("=", 1)[1]); label_max_concurrency_explicit = True; i_arg += 1
|
||||
elif a == "--batch-size" and i_arg + 1 < len(args):
|
||||
label_batch_size = int(args[i_arg + 1]); i_arg += 2
|
||||
label_batch_size = int(args[i_arg + 1]); label_batch_size_explicit = True; i_arg += 2
|
||||
elif a.startswith("--batch-size="):
|
||||
label_batch_size = int(a.split("=", 1)[1]); i_arg += 1
|
||||
label_batch_size = int(a.split("=", 1)[1]); label_batch_size_explicit = True; i_arg += 1
|
||||
elif a in ("--no-viz", "--missing-only") or a.startswith("--min-community-size="):
|
||||
i_arg += 1
|
||||
elif a.startswith("--"):
|
||||
@@ -1774,6 +1780,22 @@ def dispatch_command(cmd: str) -> None:
|
||||
# as fresh forever, permanently blocking real labeling on later runs.
|
||||
placeholder_only = False
|
||||
if labels_path.exists() and not force_relabel:
|
||||
# #2534: this branch never calls the LLM, so labeling flags would be
|
||||
# silently ignored. Reuse is still the correct (exit 0) outcome — but
|
||||
# say so, instead of letting `--backend openai` look like it relabeled.
|
||||
_ignored_label_flags = [flag for flag, given in (
|
||||
("--backend", label_backend is not None),
|
||||
("--model", label_model is not None),
|
||||
("--batch-size", label_batch_size_explicit),
|
||||
("--max-concurrency", label_max_concurrency_explicit),
|
||||
) if given]
|
||||
if _ignored_label_flags:
|
||||
print(
|
||||
f"[graphify] warning: {'/'.join(_ignored_label_flags)} ignored: "
|
||||
f"reusing saved labels at {labels_path}. "
|
||||
f"Run `graphify label` to relabel, or delete the labels file.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
# Reuse saved labels, but don't blindly trust them: the graph may have
|
||||
# been re-scoped/re-clustered since labeling, in which case a cid now
|
||||
# covers a DIFFERENT community and its old (LLM) name is wrong (#label-stale).
|
||||
@@ -1868,16 +1890,41 @@ def dispatch_command(cmd: str) -> None:
|
||||
)
|
||||
# Only let the LLM OVERRIDE where it produced a real name — its no-backend
|
||||
# fallback returns "Community {cid}" placeholders, which must not clobber
|
||||
# the deterministic hub labels.
|
||||
# the deterministic hub labels. Also reject a model echoing the prompt
|
||||
# key back ("5" for community 5) — that is an id, not a name (#2534).
|
||||
labels.update({
|
||||
cid: v for cid, v in generated_labels.items()
|
||||
if v and v != f"Community {cid}"
|
||||
if v and v != f"Community {cid}" and v != str(cid)
|
||||
})
|
||||
stages.mark("label")
|
||||
questions = suggest_questions(G, communities, labels)
|
||||
# cluster-only re-clusters an EXISTING graph: the code content is exactly
|
||||
# what extract saw, so keep the extract-time commit stamp instead of
|
||||
# re-deriving it from the shell's cwd — running from another repo used to
|
||||
# re-stamp graph.json with THAT repo's HEAD (#2534). When the loaded
|
||||
# graph predates the stamp, fall back to the analysed repo's HEAD via
|
||||
# the cwd-aware helper (#2316).
|
||||
_commit = _raw.get("built_at_commit")
|
||||
if not _commit:
|
||||
from graphify.watch import _git_head as _gh
|
||||
_commit = _gh(cwd=watch_path)
|
||||
# Snapshot BEFORE any artifact is replaced: GRAPH_REPORT.md was written
|
||||
# first, so the dated folder held the NEW report, not the previous (#2402).
|
||||
from graphify.export import backup_if_protected as _backup
|
||||
_backup(out)
|
||||
# The #479 guard can refuse this write, so it goes before the sidecars —
|
||||
# a report and labels describing a clustering graph.json does not contain
|
||||
# are worse than no run at all (#2436).
|
||||
if not to_json(G, communities, str(out / "graph.json"),
|
||||
community_labels=labels, built_at_commit=_commit):
|
||||
print(
|
||||
"graph.json NOT written: refusing to overwrite (see warning above). "
|
||||
"GRAPH_REPORT.md, .graphify_labels.json and .graphify_analysis.json "
|
||||
"left untouched.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
tokens = label_token_usage
|
||||
from graphify.export import _git_head as _gh
|
||||
_commit = _gh()
|
||||
from graphify.report import load_learning_for_report as _llfr
|
||||
report = generate(G, communities, cohesion, labels, gods, surprises,
|
||||
{"warning": "cluster-only mode — file stats not available"},
|
||||
@@ -1886,8 +1933,6 @@ def dispatch_command(cmd: str) -> None:
|
||||
learning=_llfr(out / "graph.json"))
|
||||
(out / "GRAPH_REPORT.md").write_text(report, encoding="utf-8")
|
||||
stages.mark("report")
|
||||
from graphify.export import backup_if_protected as _backup
|
||||
_backup(out)
|
||||
analysis = {
|
||||
"communities": {str(k): v for k, v in communities.items()},
|
||||
"cohesion": {str(k): v for k, v in cohesion.items()},
|
||||
@@ -1899,7 +1944,6 @@ def dispatch_command(cmd: str) -> None:
|
||||
json.dumps(analysis, indent=2, ensure_ascii=False),
|
||||
encoding="utf-8",
|
||||
)
|
||||
to_json(G, communities, str(out / "graph.json"), community_labels=labels)
|
||||
# Don't persist placeholder-only labels (or their .sig): leaving the
|
||||
# sidecar absent lets a later run generate real labels instead of reading
|
||||
# back "Community N" as authoritative (#2073).
|
||||
@@ -2067,11 +2111,17 @@ def dispatch_command(cmd: str) -> None:
|
||||
_enforce_graph_size_cap_or_exit(graph_path)
|
||||
if output_path is None:
|
||||
output_path = graph_path.parent / "GRAPH_TREE.html"
|
||||
out = write_tree_html(
|
||||
graph_path=graph_path, output_path=output_path,
|
||||
root=root, max_children=max_children,
|
||||
top_k_edges=top_k_edges, project_label=project_label,
|
||||
)
|
||||
try:
|
||||
out = write_tree_html(
|
||||
graph_path=graph_path, output_path=output_path,
|
||||
root=root, max_children=max_children,
|
||||
top_k_edges=top_k_edges, project_label=project_label,
|
||||
)
|
||||
except ValueError as exc:
|
||||
# e.g. an explicit --root that matches no source_file — the tree
|
||||
# would silently flatten, so fail loudly instead (#2534).
|
||||
print(f"error: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
size_kb = out.stat().st_size / 1024
|
||||
print(f"wrote {out} ({size_kb:.1f} KB)")
|
||||
print(f"open with: xdg-open {out} (or file://{out.resolve()})")
|
||||
|
||||
+6
-1
@@ -2856,7 +2856,11 @@ def _community_label_lines(G, communities, gods, max_communities, top_k):
|
||||
if len(names) >= top_k:
|
||||
break
|
||||
if names:
|
||||
lines.append(f"Community {cid}: {', '.join(names)}")
|
||||
# Bare id key, NOT "Community {cid}: ..." — that string doubles as the
|
||||
# placeholder sentinel (_placeholder_community_labels), so a model that
|
||||
# echoed the key back produced a "name" indistinguishable from the
|
||||
# no-backend fallback and the caller's sentinel filter dropped it (#2534).
|
||||
lines.append(f"{cid}: {', '.join(names)}")
|
||||
labeled_cids.append(int(cid))
|
||||
return lines, labeled_cids
|
||||
|
||||
@@ -2927,6 +2931,7 @@ def _label_batch_with_retry(
|
||||
"You are naming clusters in a knowledge graph. For each community below, "
|
||||
"return a concise 2-5 word plain-language name describing what it is about "
|
||||
"(e.g. \"Order Management\", \"Payment Flow\", \"Auth Middleware\"). "
|
||||
"Each input line is '<community id>: <representative member names>'. "
|
||||
"Respond ONLY with a JSON object mapping the community id (as a string) to "
|
||||
"its name - no prose, no markdown fences.\n\n" + "\n".join(batch_lines)
|
||||
)
|
||||
|
||||
@@ -77,12 +77,17 @@ def build_tree(
|
||||
Each leaf is either a code symbol (class / top-level function) or
|
||||
a synthetic "(+N more)" placeholder for truncated wide directories.
|
||||
Each interior node carries ``total_count = sum of leaf counts``.
|
||||
|
||||
Raises ``ValueError`` when an explicit ``root`` matches none of the
|
||||
graph's ``source_file`` paths — every file would silently attach flat
|
||||
to the root and the hierarchy would be lost (#2534).
|
||||
"""
|
||||
nodes: List[Dict[str, Any]] = list(graph.get("nodes", []))
|
||||
file_nodes = [n for n in nodes if n.get("source_file")]
|
||||
if not file_nodes:
|
||||
return {"name": "(empty graph)", "total_count": 0, "children": []}
|
||||
|
||||
explicit_root = root is not None
|
||||
if root is None:
|
||||
root = _common_root([n["source_file"] for n in file_nodes])
|
||||
root_path = Path(root)
|
||||
@@ -112,11 +117,13 @@ def build_tree(
|
||||
parent["children"].append(node)
|
||||
return node
|
||||
|
||||
matched_files = 0
|
||||
for src_file, syms in sorted(by_file.items()):
|
||||
src_path = Path(src_file)
|
||||
try:
|
||||
rel = src_path.relative_to(root_path)
|
||||
parent_path = (root_path / rel).parent
|
||||
matched_files += 1
|
||||
except ValueError:
|
||||
parent_path = root_path
|
||||
parent_dir = _ensure_dir(parent_path)
|
||||
@@ -153,6 +160,17 @@ def build_tree(
|
||||
}
|
||||
parent_dir["children"].append(file_node)
|
||||
|
||||
# An explicit --root that matches NOTHING would silently flatten the whole
|
||||
# hierarchy (every file attaches to the root); the common case is passing an
|
||||
# absolute checkout path while source_file is stored repo-relative (#2534).
|
||||
# A PARTIAL match stays fine — files outside the root legitimately attach flat.
|
||||
if explicit_root and matched_files == 0:
|
||||
sample = min(by_file)
|
||||
raise ValueError(
|
||||
f"--root {root} matched 0 of {len(by_file)} source files "
|
||||
f"(source_file paths are stored repo-relative, e.g. {sample!r})"
|
||||
)
|
||||
|
||||
# Sort each dir's children + propagate total_count up.
|
||||
def _finalise(d: Dict[str, Any]) -> int:
|
||||
kids = d.get("children") or []
|
||||
|
||||
@@ -650,3 +650,128 @@ def test_graph_json_node_ids_are_portable_across_checkout_paths(tmp_path):
|
||||
leak = {"alice_home", "bob_elsewhere", "checkout", "tmp", "private", "users", "home", "var"}
|
||||
assert not any(part in leak for ident in a for part in ident.split("_")), \
|
||||
f"node id embeds an absolute-path component: {a}"
|
||||
|
||||
|
||||
# ── cluster-only silent failures (#2534) + refused-write exit code (#2522) ───
|
||||
|
||||
|
||||
def test_cluster_only_reports_failure_when_write_is_refused(tmp_path):
|
||||
"""The #479 shrink guard can refuse to overwrite graph.json. cluster-only
|
||||
still printed "graph.json updated" and exited 0, and it had already written
|
||||
GRAPH_REPORT.md and the labels for the clustering it then discarded (#2436).
|
||||
Contributed by @aniJani (#2522)."""
|
||||
out = _make_graph(tmp_path)
|
||||
graph_json = out / "graph.json"
|
||||
|
||||
# Duplicate node ids make the file look larger than the graph it loads into,
|
||||
# which is what trips the guard on a real re-cluster.
|
||||
data = json.loads(graph_json.read_text(encoding="utf-8"))
|
||||
data["nodes"] = data["nodes"] + data["nodes"][:3]
|
||||
graph_json.write_text(json.dumps(data), encoding="utf-8")
|
||||
|
||||
report = out / "GRAPH_REPORT.md"
|
||||
report.write_text("PREVIOUS REPORT\n", encoding="utf-8")
|
||||
before = graph_json.read_text(encoding="utf-8")
|
||||
|
||||
r = _run(["cluster-only", ".", "--no-viz", "--no-label"], tmp_path)
|
||||
|
||||
assert r.returncode != 0, r.stdout
|
||||
assert "graph.json NOT written" in r.stderr, r.stderr
|
||||
assert "graph.json updated" not in r.stdout, r.stdout
|
||||
assert graph_json.read_text(encoding="utf-8") == before
|
||||
assert report.read_text(encoding="utf-8") == "PREVIOUS REPORT\n"
|
||||
|
||||
|
||||
def test_cluster_only_happy_path_exits_zero(tmp_path):
|
||||
"""Guard for the #2522 reordering: the normal re-cluster still succeeds."""
|
||||
_make_graph(tmp_path)
|
||||
r = _run(["cluster-only", ".", "--no-viz"], tmp_path)
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert "Done -" in r.stdout, r.stdout
|
||||
assert "communities" in r.stdout
|
||||
|
||||
|
||||
def test_cluster_only_warns_when_labeling_flags_are_ignored(tmp_path):
|
||||
"""#2534 case 1: with a saved .graphify_labels.json the reuse branch never
|
||||
calls the LLM, so --backend/--model/--batch-size used to be silently
|
||||
ignored while exiting 0. Reuse stays a success (exit 0), but the ignored
|
||||
flags must be named on stderr."""
|
||||
_make_graph(tmp_path) # persists .graphify_labels.json -> reuse branch
|
||||
|
||||
r = _run(["cluster-only", ".", "--backend", "openai", "--no-viz"], tmp_path)
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert "--backend" in r.stderr and "ignored" in r.stderr, r.stderr
|
||||
assert "reusing saved labels" in r.stderr, r.stderr
|
||||
# the reuse branch must NOT have gone through the LLM labeling path
|
||||
assert "Labeling communities" not in r.stdout, r.stdout
|
||||
|
||||
|
||||
def test_cluster_only_reuse_without_labeling_flags_stays_quiet(tmp_path):
|
||||
"""Control for the #2534 case-1 warning: plain reuse prints no flag warning."""
|
||||
_make_graph(tmp_path)
|
||||
r = _run(["cluster-only", ".", "--no-viz"], tmp_path)
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert "ignored" not in r.stderr, r.stderr
|
||||
assert "reusing saved labels" not in r.stderr, r.stderr
|
||||
|
||||
|
||||
# ── cluster-only must not re-stamp built_at_commit from the shell's cwd ──────
|
||||
|
||||
|
||||
def _init_git_repo(path: Path, message: str) -> str:
|
||||
"""git init + one empty commit; returns the HEAD sha."""
|
||||
path.mkdir(parents=True, exist_ok=True)
|
||||
subprocess.run(["git", "init", "-q", str(path)], check=True, capture_output=True)
|
||||
subprocess.run(
|
||||
["git", "-C", str(path), "-c", "user.email=t@test", "-c", "user.name=t",
|
||||
"-c", "commit.gpgsign=false", "commit", "--allow-empty", "-q", "-m", message],
|
||||
check=True, capture_output=True,
|
||||
)
|
||||
return subprocess.run(
|
||||
["git", "-C", str(path), "rev-parse", "HEAD"],
|
||||
check=True, capture_output=True, text=True,
|
||||
).stdout.strip()
|
||||
|
||||
|
||||
def test_cluster_only_preserves_built_at_commit_from_other_repo_cwd(tmp_path):
|
||||
"""#2534 case 4: cluster-only re-clusters an EXISTING graph, so the
|
||||
extract-time built_at_commit must survive — running it from a DIFFERENT
|
||||
repo used to re-stamp graph.json with that repo's HEAD."""
|
||||
target = tmp_path / "target"
|
||||
commit_x = _init_git_repo(target, "init target")
|
||||
out = _make_graph(target)
|
||||
graph_json = out / "graph.json"
|
||||
data = json.loads(graph_json.read_text(encoding="utf-8"))
|
||||
data["built_at_commit"] = commit_x
|
||||
graph_json.write_text(json.dumps(data), encoding="utf-8")
|
||||
|
||||
other = tmp_path / "other"
|
||||
commit_y = _init_git_repo(other, "init other")
|
||||
assert commit_x != commit_y
|
||||
|
||||
r = _run(["cluster-only", str(target), "--no-viz"], other)
|
||||
assert r.returncode == 0, r.stderr
|
||||
final = json.loads(graph_json.read_text(encoding="utf-8"))
|
||||
assert final.get("built_at_commit") == commit_x, (
|
||||
f"built_at_commit re-stamped from the shell's cwd: expected {commit_x}, "
|
||||
f"got {final.get('built_at_commit')} (other repo HEAD is {commit_y})"
|
||||
)
|
||||
|
||||
|
||||
def test_cluster_only_preserves_built_at_commit_from_non_repo_cwd(tmp_path):
|
||||
"""#2534 case 4: from a cwd that is not a git repo at all, the stamp must
|
||||
still be preserved — not dropped."""
|
||||
target = tmp_path / "target"
|
||||
commit_x = _init_git_repo(target, "init target")
|
||||
out = _make_graph(target)
|
||||
graph_json = out / "graph.json"
|
||||
data = json.loads(graph_json.read_text(encoding="utf-8"))
|
||||
data["built_at_commit"] = commit_x
|
||||
graph_json.write_text(json.dumps(data), encoding="utf-8")
|
||||
|
||||
plain = tmp_path / "plain"
|
||||
plain.mkdir()
|
||||
r = _run(["cluster-only", str(target), "--no-viz"], plain)
|
||||
assert r.returncode == 0, r.stderr
|
||||
final = json.loads(graph_json.read_text(encoding="utf-8"))
|
||||
assert final.get("built_at_commit") == commit_x
|
||||
|
||||
+70
-6
@@ -4,6 +4,7 @@ Backend calls are mocked - no network. Covers the happy path, partial replies,
|
||||
malformed replies, and the no-backend fallback.
|
||||
"""
|
||||
import json
|
||||
import re
|
||||
import sys
|
||||
|
||||
import networkx as nx
|
||||
@@ -265,8 +266,10 @@ def test_label_communities_batches_when_over_batch_size(monkeypatch):
|
||||
|
||||
def fake_call(prompt, *, backend, max_tokens=200):
|
||||
# The fake reads which cids the prompt asks about and answers all of them.
|
||||
cids = [int(line.split(":", 1)[0].removeprefix("Community ").strip())
|
||||
for line in prompt.splitlines() if line.startswith("Community ")]
|
||||
# #2534: prompt lines are now "<cid>: <names>" — the old "Community {cid}:"
|
||||
# key collided with the placeholder sentinel and echoed keys were dropped.
|
||||
cids = [int(m.group(1)) for m in
|
||||
(re.match(r"^(\d+): ", line) for line in prompt.splitlines()) if m]
|
||||
calls.append(len(cids))
|
||||
return "{" + ", ".join(f'"{c}": "Cluster {c}"' for c in cids) + "}"
|
||||
|
||||
@@ -287,8 +290,10 @@ def test_label_communities_partial_batch_failure_keeps_successful_batches(monkey
|
||||
|
||||
def fake_call(prompt, *, backend, max_tokens=200):
|
||||
n_calls[0] += 1
|
||||
cids = [int(line.split(":", 1)[0].removeprefix("Community ").strip())
|
||||
for line in prompt.splitlines() if line.startswith("Community ")]
|
||||
# #2534: prompt lines are now "<cid>: <names>" — the old "Community {cid}:"
|
||||
# key collided with the placeholder sentinel and echoed keys were dropped.
|
||||
cids = [int(m.group(1)) for m in
|
||||
(re.match(r"^(\d+): ", line) for line in prompt.splitlines()) if m]
|
||||
if n_calls[0] == 2:
|
||||
raise RuntimeError("simulated transient backend failure")
|
||||
return "{" + ", ".join(f'"{c}": "Named {c}"' for c in cids) + "}"
|
||||
@@ -323,8 +328,10 @@ def test_label_communities_max_communities_caps_total(monkeypatch):
|
||||
captured_cids = []
|
||||
|
||||
def fake_call(prompt, *, backend, max_tokens=200):
|
||||
cids = [int(line.split(":", 1)[0].removeprefix("Community ").strip())
|
||||
for line in prompt.splitlines() if line.startswith("Community ")]
|
||||
# #2534: prompt lines are now "<cid>: <names>" — the old "Community {cid}:"
|
||||
# key collided with the placeholder sentinel and echoed keys were dropped.
|
||||
cids = [int(m.group(1)) for m in
|
||||
(re.match(r"^(\d+): ", line) for line in prompt.splitlines()) if m]
|
||||
captured_cids.extend(cids)
|
||||
return "{" + ", ".join(f'"{c}": "X{c}"' for c in cids) + "}"
|
||||
|
||||
@@ -550,3 +557,60 @@ def test_cluster_only_heals_persisted_placeholder_but_reuses_genuine(tmp_path, m
|
||||
saved = json.loads(labels_path.read_text(encoding="utf-8"))
|
||||
assert saved["0"] != "Community 0", "stuck placeholder was not healed (#2073)"
|
||||
assert saved["1"] == "Payment Flow", "genuine label was not reused"
|
||||
|
||||
|
||||
# ── #2534 case 2: prompt key must not collide with the placeholder sentinel ──
|
||||
|
||||
|
||||
def test_label_prompt_lines_use_bare_cid_keys():
|
||||
"""The prompt line used to read "Community {cid}: ..." — the exact string of
|
||||
the no-backend placeholder sentinel. A model that echoed the key back as the
|
||||
name produced a reply indistinguishable from the fallback, so the caller's
|
||||
sentinel filter silently dropped it (#2534). The key must be the bare id."""
|
||||
from graphify.llm import _community_label_lines
|
||||
|
||||
G, communities = _graph()
|
||||
lines, labeled_cids = _community_label_lines(G, communities, None, 10, 12)
|
||||
assert sorted(labeled_cids) == [0, 1]
|
||||
for line in lines:
|
||||
assert re.match(r"^\d+: ", line), f"prompt line key is not a bare id: {line!r}"
|
||||
assert not line.startswith("Community "), f"sentinel-colliding key: {line!r}"
|
||||
|
||||
|
||||
def test_label_cli_drops_sentinel_and_bare_key_echoes(tmp_path, monkeypatch):
|
||||
"""#2534 case 2, cli side: an LLM reply that echoes the sentinel
|
||||
("Community 5") or the bare prompt key ("7") must not survive the filter —
|
||||
the deterministic hub labels win — while a real name still overrides."""
|
||||
import graphify.__main__ as cli
|
||||
|
||||
out = tmp_path / "graphify-out"
|
||||
out.mkdir()
|
||||
graph = {
|
||||
"directed": False,
|
||||
"multigraph": False,
|
||||
"nodes": [
|
||||
{"id": "orders", "label": "OrderService", "community": 0},
|
||||
{"id": "pay", "label": "PaymentService", "community": 5},
|
||||
{"id": "ship", "label": "ShippingService", "community": 7},
|
||||
],
|
||||
"links": [],
|
||||
}
|
||||
(out / "graph.json").write_text(json.dumps(graph), encoding="utf-8")
|
||||
|
||||
def fake_generate(G, communities, *, backend=None, model=None, gods=None,
|
||||
quiet=False, max_concurrency=4, batch_size=100, usage_out=None):
|
||||
return {0: "Order Management", 5: "Community 5", 7: "7"}, "llm"
|
||||
|
||||
monkeypatch.setattr("graphify.llm.generate_community_labels", fake_generate)
|
||||
monkeypatch.setattr("graphify.export.to_html", lambda *args, **kwargs: None)
|
||||
monkeypatch.setattr(
|
||||
sys, "argv",
|
||||
["graphify", "label", str(tmp_path), "--backend", "gemini", "--no-viz"],
|
||||
)
|
||||
|
||||
cli.main()
|
||||
|
||||
labels = json.loads((out / ".graphify_labels.json").read_text(encoding="utf-8"))
|
||||
assert labels["0"] == "Order Management" # real name survives
|
||||
assert labels["5"] == "PaymentService" # sentinel echo dropped -> hub label
|
||||
assert labels["7"] == "ShippingService" # bare-key echo dropped -> hub label
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
"""Tests for graphify tree / tree_html.build_tree.
|
||||
|
||||
#2534 case 3: `tree --root <path>` that matches NO source_file used to silently
|
||||
flatten the whole hierarchy (every file attached flat to the root) while the CLI
|
||||
still printed "wrote ..." and exited 0 — source_file paths are stored
|
||||
repo-relative, so an absolute --root never matched. A zero-match explicit root
|
||||
now raises/exits 1; the default computed root and partial matches stay exit 0.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from graphify.tree_html import build_tree
|
||||
|
||||
PYTHON = sys.executable
|
||||
|
||||
|
||||
def _run(args: list[str], cwd: Path) -> subprocess.CompletedProcess:
|
||||
return subprocess.run(
|
||||
[PYTHON, "-m", "graphify"] + args,
|
||||
cwd=cwd,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
|
||||
|
||||
def _graph(files: dict[str, str]) -> dict:
|
||||
"""Minimal node-link graph: one symbol node per {source_file: label}."""
|
||||
return {
|
||||
"directed": False,
|
||||
"nodes": [
|
||||
{"id": f"n{i}", "label": label, "source_file": src}
|
||||
for i, (src, label) in enumerate(sorted(files.items()))
|
||||
],
|
||||
"links": [],
|
||||
}
|
||||
|
||||
|
||||
def _write_graph(tmp_path: Path, graph: dict) -> Path:
|
||||
out = tmp_path / "graphify-out"
|
||||
out.mkdir()
|
||||
(out / "graph.json").write_text(json.dumps(graph), encoding="utf-8")
|
||||
return out
|
||||
|
||||
|
||||
# ── build_tree unit tests ─────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_build_tree_explicit_root_matching_nothing_raises():
|
||||
graph = _graph({"pkg/a.py": "func_a", "pkg/sub/b.py": "func_b"})
|
||||
with pytest.raises(ValueError, match=r"matched 0 of 2 source files"):
|
||||
build_tree(graph, root="/abs/checkout/elsewhere")
|
||||
|
||||
|
||||
def test_build_tree_error_names_a_repo_relative_sample():
|
||||
graph = _graph({"pkg/a.py": "func_a"})
|
||||
with pytest.raises(ValueError, match=r"repo-relative.*'pkg/a\.py'"):
|
||||
build_tree(graph, root="/abs/nonmatch")
|
||||
|
||||
|
||||
def test_build_tree_partial_match_does_not_raise():
|
||||
"""Files outside an explicit root legitimately attach flat — only a
|
||||
ZERO-match explicit root is an error."""
|
||||
graph = _graph({"pkg/a.py": "func_a", "other/c.py": "func_c"})
|
||||
tree = build_tree(graph, root="pkg")
|
||||
assert tree["name"] == "pkg"
|
||||
assert tree["total_count"] > 0
|
||||
|
||||
|
||||
def test_build_tree_default_root_keeps_hierarchy():
|
||||
graph = _graph({"pkg/a.py": "func_a", "pkg/sub/b.py": "func_b"})
|
||||
tree = build_tree(graph) # computed common root — always matches
|
||||
assert tree["name"] == "pkg"
|
||||
names = {c["name"] for c in tree["children"]}
|
||||
assert "a.py" in names and "sub" in names
|
||||
|
||||
|
||||
# ── CLI integration ───────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_tree_cli_absolute_root_mismatch_exits_1(tmp_path):
|
||||
_write_graph(tmp_path, _graph({"pkg/a.py": "func_a", "pkg/sub/b.py": "func_b"}))
|
||||
r = _run(["tree", "--root", "/abs/nonmatch"], tmp_path)
|
||||
assert r.returncode == 1, r.stdout
|
||||
assert "matched 0 of" in r.stderr, r.stderr
|
||||
# the handler's error pattern, not an unhandled traceback
|
||||
assert "error:" in r.stderr, r.stderr
|
||||
assert "Traceback" not in r.stderr, r.stderr
|
||||
assert "wrote" not in r.stdout, r.stdout
|
||||
assert not (tmp_path / "graphify-out" / "GRAPH_TREE.html").exists()
|
||||
|
||||
|
||||
def test_tree_cli_default_root_succeeds(tmp_path):
|
||||
_write_graph(tmp_path, _graph({"pkg/a.py": "func_a", "pkg/sub/b.py": "func_b"}))
|
||||
r = _run(["tree"], tmp_path)
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert "wrote" in r.stdout, r.stdout
|
||||
html = (tmp_path / "graphify-out" / "GRAPH_TREE.html").read_text(encoding="utf-8")
|
||||
# the hierarchy survives: pkg is the root, sub is an interior node
|
||||
assert '"name":"pkg"' in html
|
||||
assert '"name":"sub"' in html
|
||||
|
||||
|
||||
def test_tree_cli_partial_match_root_succeeds(tmp_path):
|
||||
_write_graph(tmp_path, _graph({"pkg/a.py": "func_a", "other/c.py": "func_c"}))
|
||||
r = _run(["tree", "--root", "pkg"], tmp_path)
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert (tmp_path / "graphify-out" / "GRAPH_TREE.html").exists()
|
||||
Reference in New Issue
Block a user