mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-17 09:29:27 +00:00
fix(cluster): keep a community label when membership overlap stays a majority (#2298)
A one-node membership change failed the exact signature check and replaced the saved name with the hub label. Keep the name when the shared nodes are a strict majority of both the saved set and the current set.
Related to #2298
(cherry picked from commit 62e83eabff)
This commit is contained in:
1 parent
f48425e6e0
commit
a0587559ca
5 files changed
+293
-22
No files matched your search
+33
-11
@@ -2523,12 +2523,19 @@ def dispatch_command(cmd: str) -> None:
|
||||
# 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).
|
||||
# Validate each community against the membership signature saved beside the
|
||||
# labels; any community that changed (or has no saved label) is renamed by
|
||||
# its current hub — deterministic and correct-by-construction — and the user
|
||||
# labels; a community whose overlap is not a strict majority of both
|
||||
# the saved members and the current members (or that has no saved
|
||||
# label) is renamed by its current hub — deterministic and
|
||||
# correct-by-construction — and the user
|
||||
# is told to `graphify label` for fresh LLM names. Unchanged communities keep
|
||||
# their saved label. When no signature sidecar exists (labels predate this),
|
||||
# fall back to hub-filling only the communities missing a label.
|
||||
from graphify.cluster import community_member_sigs, label_communities_by_hub
|
||||
# an equal community count still hub-fills only communities missing a
|
||||
# label; a changed count keeps a label only on majority overlap.
|
||||
from graphify.cluster import (
|
||||
community_member_sigs,
|
||||
label_communities_by_hub,
|
||||
label_still_applies,
|
||||
)
|
||||
sig_path = labels_path.parent / (labels_path.name + ".sig")
|
||||
saved_sigs: dict[int, str] = {}
|
||||
if sig_path.exists():
|
||||
@@ -2555,14 +2562,29 @@ def dispatch_command(cmd: str) -> None:
|
||||
and existing_labels[cid] != f"Community {cid}"
|
||||
)
|
||||
if saved_sigs:
|
||||
# Precise: the membership signature tells us if this exact
|
||||
# community changed since it was labeled.
|
||||
fresh = have_label and saved_sigs.get(cid) == cur_sigs.get(cid)
|
||||
# Equal signature, or a strict majority of both the saved
|
||||
# members and the current members (#2298).
|
||||
fresh = have_label and label_still_applies(
|
||||
cid,
|
||||
communities[cid],
|
||||
previous_node_community,
|
||||
saved_sigs.get(cid),
|
||||
cur_sigs.get(cid),
|
||||
)
|
||||
elif count_mismatch:
|
||||
# No signature sidecar. A changed count used to drop every
|
||||
# label; keep one only on that same majority overlap.
|
||||
fresh = have_label and label_still_applies(
|
||||
cid,
|
||||
communities[cid],
|
||||
previous_node_community,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
else:
|
||||
# No signature sidecar (labels predate it). A differing community
|
||||
# COUNT means the labels describe a different clustering, so a cid's
|
||||
# old label can't be trusted; equal count is the best "same" signal.
|
||||
fresh = have_label and not count_mismatch
|
||||
# No signature sidecar and an equal community count: that
|
||||
# count is still the best available "unchanged" signal.
|
||||
fresh = have_label
|
||||
if fresh:
|
||||
labels[cid] = existing_labels[cid]
|
||||
else:
|
||||
|
||||
@@ -480,3 +480,28 @@ def remap_communities_to_previous(
|
||||
for new_cid, nodes in communities.items():
|
||||
remapped[new_to_final[new_cid]] = sorted(nodes)
|
||||
return dict(sorted(remapped.items(), key=lambda kv: kv[0]))
|
||||
|
||||
|
||||
def label_still_applies(
|
||||
cid: int,
|
||||
current_members: list[str],
|
||||
previous_node_community: dict[str, int],
|
||||
saved_sig: str | None,
|
||||
current_sig: str | None,
|
||||
) -> bool:
|
||||
"""Whether a saved label still names this community (#2298).
|
||||
|
||||
An equal membership signature keeps the label. Otherwise the label stays
|
||||
only when the nodes shared with the previous assignment are a strict
|
||||
majority of both the previous set and the current set. A one-node fragment
|
||||
of a large community fails that test, so remap reusing the old id is not
|
||||
enough to keep the name.
|
||||
"""
|
||||
if saved_sig is not None and current_sig is not None and saved_sig == current_sig:
|
||||
return True
|
||||
saved = {str(node) for node, old in previous_node_community.items() if old == cid}
|
||||
current = {str(node) for node in current_members}
|
||||
if not saved or not current:
|
||||
return False
|
||||
overlap = len(saved & current)
|
||||
return overlap * 2 > len(saved) and overlap * 2 > len(current)
|
||||
+32
-11
@@ -2439,11 +2439,12 @@ def _rebuild_code(
|
||||
# rebuild that adds nodes, cid 30 can cover a completely different community
|
||||
# and its old name is then simply wrong. Validate every reused label against
|
||||
# the membership signature saved beside the labels — the same guard the
|
||||
# cluster-only path applies — and drop any whose community changed so the
|
||||
# cluster-only path applies — and drop any whose overlap is not a strict
|
||||
# majority of both the saved members and the current members so the
|
||||
# hub-fill below renames it, deterministically and correct-by-construction.
|
||||
# Without this, an incremental `graphify update` launders stale names into
|
||||
# labels.json as though they were current (#label-stale).
|
||||
from graphify.cluster import community_member_sigs
|
||||
from graphify.cluster import community_member_sigs, label_still_applies
|
||||
cur_sigs = community_member_sigs(communities)
|
||||
saved_sigs: dict[int, str] = {}
|
||||
if sig_file.exists():
|
||||
@@ -2456,16 +2457,36 @@ def _rebuild_code(
|
||||
except Exception:
|
||||
saved_sigs = {}
|
||||
if saved_sigs:
|
||||
# Precise: the signature tells us exactly which communities changed.
|
||||
stale = {cid for cid in labels if saved_sigs.get(cid) != cur_sigs.get(cid)}
|
||||
# Equal signature, or a strict majority of both the saved members
|
||||
# and the current members (#2298).
|
||||
stale = {
|
||||
cid for cid in labels
|
||||
if not label_still_applies(
|
||||
cid,
|
||||
communities.get(cid, []),
|
||||
previous_node_community,
|
||||
saved_sigs.get(cid),
|
||||
cur_sigs.get(cid),
|
||||
)
|
||||
}
|
||||
elif len(raw) != len(communities):
|
||||
# No sidecar (labels predate it). A differing community count used
|
||||
# to drop every label; keep one only on that same majority overlap.
|
||||
# Compare against the tracked file alone (`raw`), not the
|
||||
# pending-merged `labels` -- a from-empty rebuild's fresh pending
|
||||
# entries must not count toward "how many labels were saved".
|
||||
stale = {
|
||||
cid for cid in labels
|
||||
if not label_still_applies(
|
||||
cid,
|
||||
communities.get(cid, []),
|
||||
previous_node_community,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
}
|
||||
else:
|
||||
# No sidecar (labels predate it). A differing community COUNT means the
|
||||
# labels describe a different clustering, so no cid's label is trustworthy;
|
||||
# an equal count is the best available "unchanged" signal. Compare
|
||||
# against the tracked file alone (`raw`), not the pending-merged
|
||||
# `labels` -- a from-empty rebuild's fresh pending entries must not
|
||||
# count toward "how many labels were saved".
|
||||
stale = set(labels) if len(raw) != len(communities) else set()
|
||||
stale = set()
|
||||
for cid in stale:
|
||||
del labels[cid]
|
||||
pending_cids.discard(cid)
|
||||
|
||||
@@ -841,6 +841,150 @@ def test_cluster_only_heals_persisted_placeholder_but_reuses_genuine(tmp_path, m
|
||||
assert saved["1"] == "Payment Flow", "genuine label was not reused"
|
||||
|
||||
|
||||
def _auth_billing_graph(out: Path) -> None:
|
||||
"""Two saved communities plus one unclustered node (#2298)."""
|
||||
graph = {
|
||||
"directed": False,
|
||||
"multigraph": False,
|
||||
"nodes": [
|
||||
{"id": "a1", "label": "auth_hub", "community": 0},
|
||||
{"id": "a2", "label": "auth_a", "community": 0},
|
||||
{"id": "a3", "label": "auth_b", "community": 0},
|
||||
{"id": "a4", "label": "auth_c", "community": 0},
|
||||
{"id": "b1", "label": "bill_hub", "community": 1},
|
||||
{"id": "b2", "label": "bill_a", "community": 1},
|
||||
{"id": "b3", "label": "bill_b", "community": 1},
|
||||
{"id": "b4", "label": "bill_c", "community": 1},
|
||||
{"id": "a_new", "label": "auth_new"},
|
||||
],
|
||||
"links": [
|
||||
{"source": "a1", "target": "a2", "relation": "calls"},
|
||||
{"source": "a1", "target": "a3", "relation": "calls"},
|
||||
{"source": "a1", "target": "a4", "relation": "calls"},
|
||||
{"source": "b1", "target": "b2", "relation": "calls"},
|
||||
{"source": "b1", "target": "b3", "relation": "calls"},
|
||||
{"source": "b1", "target": "b4", "relation": "calls"},
|
||||
],
|
||||
}
|
||||
(out / "graph.json").write_text(json.dumps(graph), encoding="utf-8")
|
||||
|
||||
|
||||
def _patch_cluster_only_reuse(monkeypatch):
|
||||
# `graphify.cluster` is a lazy function on the package until the submodule
|
||||
# is imported. cluster-only does `from graphify.cluster import cluster`.
|
||||
import graphify.cluster # noqa: F401
|
||||
import graphify.__main__ as cli
|
||||
|
||||
monkeypatch.setattr(cli, "_check_skill_version", lambda _: None)
|
||||
monkeypatch.setattr("graphify.export.to_html", lambda *a, **k: None)
|
||||
|
||||
def _fail_generate(*a, **k):
|
||||
raise AssertionError("generate_community_labels must not be called on the reuse path")
|
||||
|
||||
monkeypatch.setattr("graphify.llm.generate_community_labels", _fail_generate)
|
||||
return cli
|
||||
|
||||
|
||||
def test_cluster_only_keeps_label_on_majority_membership_overlap(tmp_path, monkeypatch):
|
||||
"""#2298: one added node must not replace a saved name when the shared
|
||||
members are a strict majority of both the saved set and the current set.
|
||||
A one-node fragment of a large community must not keep that name."""
|
||||
from graphify.cluster import community_member_sigs
|
||||
|
||||
cli = _patch_cluster_only_reuse(monkeypatch)
|
||||
|
||||
root = tmp_path / "add-one"
|
||||
out = root / "graphify-out"
|
||||
out.mkdir(parents=True)
|
||||
_auth_billing_graph(out)
|
||||
labels_path = out / ".graphify_labels.json"
|
||||
labels_path.write_text(
|
||||
json.dumps({"0": "Auth Engine", "1": "Billing Desk"}), encoding="utf-8"
|
||||
)
|
||||
old_members = {
|
||||
0: ["a1", "a2", "a3", "a4"],
|
||||
1: ["b1", "b2", "b3", "b4"],
|
||||
}
|
||||
(out / ".graphify_labels.json.sig").write_text(
|
||||
json.dumps({str(k): v for k, v in community_member_sigs(old_members).items()}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
def _cluster_with_new(G, *args, **kwargs):
|
||||
return {
|
||||
0: ["a1", "a2", "a3", "a4", "a_new"],
|
||||
1: ["b1", "b2", "b3", "b4"],
|
||||
}
|
||||
|
||||
monkeypatch.setattr("graphify.cluster.cluster", _cluster_with_new)
|
||||
monkeypatch.setattr(sys, "argv", ["graphify", "cluster-only", str(root), "--no-viz"])
|
||||
cli.main()
|
||||
labels = json.loads(labels_path.read_text(encoding="utf-8"))
|
||||
assert labels["0"] == "Auth Engine"
|
||||
assert labels["1"] == "Billing Desk"
|
||||
|
||||
fragment = tmp_path / "fragment"
|
||||
fragment_out = fragment / "graphify-out"
|
||||
fragment_out.mkdir(parents=True)
|
||||
member_ids = [f"n{i:03d}" for i in range(200)]
|
||||
fragment_graph = {
|
||||
"directed": False,
|
||||
"multigraph": False,
|
||||
"nodes": [
|
||||
{"id": nid, "label": nid, "community": 0}
|
||||
for nid in member_ids
|
||||
],
|
||||
"links": [],
|
||||
}
|
||||
(fragment_out / "graph.json").write_text(json.dumps(fragment_graph), encoding="utf-8")
|
||||
(fragment_out / ".graphify_labels.json").write_text(
|
||||
json.dumps({"0": "Big Service"}), encoding="utf-8"
|
||||
)
|
||||
(fragment_out / ".graphify_labels.json.sig").write_text(
|
||||
json.dumps({str(k): v for k, v in community_member_sigs({0: member_ids}).items()}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
def _cluster_singletons(G, *args, **kwargs):
|
||||
return {i: [f"n{i:03d}"] for i in range(200)}
|
||||
|
||||
monkeypatch.setattr("graphify.cluster.cluster", _cluster_singletons)
|
||||
monkeypatch.setattr(sys, "argv", ["graphify", "cluster-only", str(fragment), "--no-viz"])
|
||||
cli.main()
|
||||
fragment_labels = json.loads(
|
||||
(fragment_out / ".graphify_labels.json").read_text(encoding="utf-8")
|
||||
)
|
||||
assert "Big Service" not in fragment_labels.values()
|
||||
|
||||
|
||||
def test_cluster_only_keeps_unchanged_label_when_count_changes(tmp_path, monkeypatch):
|
||||
"""#2298: with no signature file, a new singleton community must not hub-rename
|
||||
communities whose membership did not shrink."""
|
||||
cli = _patch_cluster_only_reuse(monkeypatch)
|
||||
|
||||
out = tmp_path / "graphify-out"
|
||||
out.mkdir()
|
||||
_auth_billing_graph(out)
|
||||
labels_path = out / ".graphify_labels.json"
|
||||
labels_path.write_text(
|
||||
json.dumps({"0": "Auth Engine", "1": "Billing Desk"}), encoding="utf-8"
|
||||
)
|
||||
|
||||
def _cluster_plus_singleton(G, *args, **kwargs):
|
||||
return {
|
||||
0: ["a1", "a2", "a3", "a4"],
|
||||
1: ["b1", "b2", "b3", "b4"],
|
||||
2: ["a_new"],
|
||||
}
|
||||
|
||||
monkeypatch.setattr("graphify.cluster.cluster", _cluster_plus_singleton)
|
||||
monkeypatch.setattr(sys, "argv", ["graphify", "cluster-only", str(tmp_path), "--no-viz"])
|
||||
cli.main()
|
||||
labels = json.loads(labels_path.read_text(encoding="utf-8"))
|
||||
assert labels["0"] == "Auth Engine"
|
||||
assert labels["1"] == "Billing Desk"
|
||||
|
||||
|
||||
# ── #2534 case 2: prompt key must not collide with the placeholder sentinel ──
|
||||
|
||||
|
||||
|
||||
@@ -493,6 +493,65 @@ def test_rebuild_code_drops_labels_whose_community_changed(tmp_path):
|
||||
)
|
||||
|
||||
|
||||
def test_rebuild_keeps_label_on_majority_overlap(tmp_path, monkeypatch):
|
||||
"""#2298: an update that adds one node to a community keeps the saved name
|
||||
when the previous members are still a strict majority of the new set."""
|
||||
import json
|
||||
from graphify.watch import _rebuild_code
|
||||
|
||||
corpus = tmp_path / "corpus"
|
||||
corpus.mkdir()
|
||||
source = (
|
||||
"def alpha():\n return beta()\n\n"
|
||||
"def beta():\n return gamma()\n\n"
|
||||
"def gamma():\n return 1\n"
|
||||
)
|
||||
(corpus / "a.py").write_text(source, encoding="utf-8")
|
||||
assert _rebuild_code(corpus, acquire_lock=False) is True
|
||||
|
||||
out = corpus / "graphify-out"
|
||||
labels_file = out / ".graphify_labels.json"
|
||||
graph = json.loads((out / "graph.json").read_text(encoding="utf-8"))
|
||||
communities: dict[int, list[str]] = {}
|
||||
previous: dict[str, int] = {}
|
||||
for node in graph["nodes"]:
|
||||
cid = node.get("community")
|
||||
node_id = node.get("id")
|
||||
if cid is None or node_id is None:
|
||||
continue
|
||||
communities.setdefault(int(cid), []).append(str(node_id))
|
||||
previous[str(node_id)] = int(cid)
|
||||
assert communities, "expected the first rebuild to cluster at least one community"
|
||||
largest = max(communities, key=lambda cid: (len(communities[cid]), -cid))
|
||||
assert len(communities[largest]) >= 2, (
|
||||
f"largest community {largest} has {communities[largest]}; need at least 2 nodes"
|
||||
)
|
||||
|
||||
distinctive = "Kept Majority Name"
|
||||
labels_file.write_text(json.dumps({str(largest): distinctive}), encoding="utf-8")
|
||||
|
||||
(corpus / "a.py").write_text(
|
||||
source + "\ndef delta():\n return alpha()\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
def _cluster_extend_largest(G, *args, **kwargs):
|
||||
grouped: dict[int, list[str]] = {}
|
||||
for node in G.nodes:
|
||||
nid = str(node)
|
||||
cid = previous.get(nid, largest)
|
||||
grouped.setdefault(cid, []).append(nid)
|
||||
return grouped
|
||||
|
||||
import graphify.cluster # bind the submodule; the package attribute is lazy
|
||||
|
||||
monkeypatch.setattr(graphify.cluster, "cluster", _cluster_extend_largest)
|
||||
assert _rebuild_code(corpus, acquire_lock=False) is True
|
||||
|
||||
saved = json.loads(labels_file.read_text(encoding="utf-8"))
|
||||
assert distinctive in saved.values()
|
||||
|
||||
|
||||
def test_rebuild_code_keeps_hub_fallback_labels_out_of_the_tracked_file(tmp_path):
|
||||
"""#3334: a from-empty rebuild (a fresh clone, a new worktree, any CI job
|
||||
with no prior graphify-out/) names every community by its deterministic
|
||||
|
||||
Reference in new issue
Block a user