From a0587559ca5b0bb087b305bff8c4edd41005992b Mon Sep 17 00:00:00 2001 From: SrijanSriv Date: Wed, 14 Oct 2026 15:47:15 +0530 Subject: [PATCH] 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 62e83eabff27b14a5b4f17d2b36e0b0cb0913326) --- graphify/cli.py | 44 +++++++++---- graphify/cluster.py | 25 +++++++ graphify/watch.py | 43 ++++++++---- tests/test_labeling.py | 144 +++++++++++++++++++++++++++++++++++++++++ tests/test_watch.py | 59 +++++++++++++++++ 5 files changed, 293 insertions(+), 22 deletions(-) diff --git a/graphify/cli.py b/graphify/cli.py index 73d94abc8..6a20e3dad 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -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: diff --git a/graphify/cluster.py b/graphify/cluster.py index 6bcecc95d..cf364b4c4 100644 --- a/graphify/cluster.py +++ b/graphify/cluster.py @@ -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) diff --git a/graphify/watch.py b/graphify/watch.py index a71765721..afb5f7efc 100644 --- a/graphify/watch.py +++ b/graphify/watch.py @@ -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) diff --git a/tests/test_labeling.py b/tests/test_labeling.py index ceab5d120..fed61e6e8 100644 --- a/tests/test_labeling.py +++ b/tests/test_labeling.py @@ -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 ── diff --git a/tests/test_watch.py b/tests/test_watch.py index 506a4f93b..bb4d12599 100644 --- a/tests/test_watch.py +++ b/tests/test_watch.py @@ -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