mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-26 16:26:42 +00:00
fix(build): offset community ids per input in merge-graphs (#3014)
merge-graphs kept each input's community ids as-is, so community 0 from repo A and community 0 from repo B fused into one meta-community spanning unrelated repos. Offset each input's community ids past the running maximum so every input's communities stay globally distinct after merge, preserving within-input structure and recording the original id in local_community.
This commit is contained in:
committed by
safishamsi
parent
ad7c7d58d1
commit
4e2e23278a
+13
-1
@@ -1980,19 +1980,31 @@ def build_merge(
|
||||
return G
|
||||
|
||||
|
||||
def prefix_graph_for_global(G: nx.Graph, repo_tag: str) -> nx.Graph:
|
||||
def prefix_graph_for_global(
|
||||
G: nx.Graph, repo_tag: str, community_offset: int = 0
|
||||
) -> nx.Graph:
|
||||
"""Return a copy of G with all node IDs prefixed with repo_tag::.
|
||||
|
||||
Labels are preserved unchanged (for display). A 'local_id' attribute
|
||||
is added to each node so the original ID can be recovered. Edges and
|
||||
their directional attributes (_src/_tgt) are rewritten to match the new
|
||||
prefixed IDs. The 'repo' attribute is set on every node.
|
||||
|
||||
community_offset shifts each node's integer 'community' id into a shared
|
||||
id space and records the original in 'local_community': every input graph
|
||||
numbers its communities from 0, so ids carried across a merge unchanged
|
||||
collide and the aggregated community view fuses unrelated communities
|
||||
into one meta-node (#3014). 0 (the default) leaves communities untouched.
|
||||
"""
|
||||
relabel = {n: f"{repo_tag}::{n}" for n in G.nodes}
|
||||
H = nx.relabel_nodes(G, relabel, copy=True)
|
||||
for node, data in H.nodes(data=True):
|
||||
data["repo"] = repo_tag
|
||||
data.setdefault("local_id", node.split("::", 1)[1])
|
||||
cid = data.get("community")
|
||||
if community_offset and isinstance(cid, int):
|
||||
data["local_community"] = cid
|
||||
data["community"] = cid + community_offset
|
||||
for u, v, data in H.edges(data=True):
|
||||
if "_src" in data and data["_src"] in relabel:
|
||||
data["_src"] = relabel[data["_src"]]
|
||||
|
||||
+15
-1
@@ -2541,11 +2541,25 @@ def dispatch_command(cmd: str) -> None:
|
||||
# diagnosis in PR #1691). Collect every input's prefixed hyperedges
|
||||
# and re-attach the union after composing.
|
||||
collected_hyperedges: list = []
|
||||
# Offset each input's community ids into a shared id space as it is
|
||||
# prefixed: every input numbers its communities from 0, so ids carried
|
||||
# across unchanged collide in the merged graph and the aggregated
|
||||
# community view fuses unrelated communities into one meta-node (#3014).
|
||||
# The first input keeps its original ids (offset 0); local_community on
|
||||
# the prefixed nodes preserves each repo's own partition.
|
||||
community_offset = 0
|
||||
for G, repo_tag in zip(graphs, repo_tags):
|
||||
prefixed = _to_simple(_prefix(G, repo_tag))
|
||||
prefixed = _to_simple(_prefix(G, repo_tag, community_offset=community_offset))
|
||||
hes = prefixed.graph.get("hyperedges")
|
||||
if isinstance(hes, list):
|
||||
collected_hyperedges.extend(h for h in hes if isinstance(h, dict))
|
||||
cids = [
|
||||
d["community"]
|
||||
for _, d in prefixed.nodes(data=True)
|
||||
if isinstance(d.get("community"), int)
|
||||
]
|
||||
if cids:
|
||||
community_offset = max(community_offset, max(cids) + 1)
|
||||
merged = _nx.compose(merged, prefixed)
|
||||
# A contract type both repos declare arrives as two unconnected nodes,
|
||||
# since every id is repo-prefixed. Link them so a traversal can cross
|
||||
|
||||
@@ -81,6 +81,38 @@ def test_prefix_graph_rewrites_edge_directional_attributes():
|
||||
assert data["_tgt"] == "repoA::collections"
|
||||
|
||||
|
||||
def test_prefix_graph_offsets_community_ids():
|
||||
"""#3014: every input graph numbers its communities from 0, so a merge
|
||||
carrying ids across unchanged fuses community 0 of one repo with community
|
||||
0 of another into a single meta-node in the aggregated view. An offset must
|
||||
shift integer ids into a shared id space and keep the per-repo id in
|
||||
local_community."""
|
||||
from graphify.build import prefix_graph_for_global
|
||||
G = _make_graph(
|
||||
[{"id": "a", "community": 0}, {"id": "b", "community": 1}], [],
|
||||
)
|
||||
H = prefix_graph_for_global(G, "repoA", community_offset=5)
|
||||
assert H.nodes["repoA::a"]["community"] == 5
|
||||
assert H.nodes["repoA::a"]["local_community"] == 0
|
||||
assert H.nodes["repoA::b"]["community"] == 6
|
||||
assert H.nodes["repoA::b"]["local_community"] == 1
|
||||
|
||||
|
||||
def test_prefix_graph_zero_offset_leaves_communities_untouched():
|
||||
"""The default offset must be a no-op — no community rewrite, no
|
||||
local_community noise — so single-repo callers (global store, tests)
|
||||
keep their ids exactly as stored."""
|
||||
from graphify.build import prefix_graph_for_global
|
||||
G = _make_graph(
|
||||
[{"id": "a", "community": 0}, {"id": "b", "community": 1}], [],
|
||||
)
|
||||
H = prefix_graph_for_global(G, "repoA")
|
||||
assert H.nodes["repoA::a"]["community"] == 0
|
||||
assert H.nodes["repoA::b"]["community"] == 1
|
||||
assert "local_community" not in H.nodes["repoA::a"]
|
||||
assert "local_community" not in H.nodes["repoA::b"]
|
||||
|
||||
|
||||
|
||||
def test_prune_repo_removes_correct_nodes():
|
||||
from graphify.build import prune_repo_from_graph
|
||||
|
||||
@@ -247,3 +247,43 @@ def test_merge_graphs_reads_top_level_only_hyperedges(tmp_path):
|
||||
assert [h["id"] for h in data["hyperedges"]] == ["alpha::h_top"]
|
||||
assert data["hyperedges"][0]["nodes"] == ["alpha::x"]
|
||||
|
||||
|
||||
|
||||
def _write_with_communities(p: Path, nodes):
|
||||
p.parent.mkdir(parents=True, exist_ok=True)
|
||||
p.write_text(json.dumps({
|
||||
"directed": False, "multigraph": False, "graph": {},
|
||||
"nodes": [{"id": nid, "community": cid} for nid, cid in nodes],
|
||||
"links": [],
|
||||
}))
|
||||
|
||||
|
||||
def test_merge_graphs_offsets_communities_so_repos_do_not_fuse(tmp_path):
|
||||
# #3014: every input numbers its communities from 0, so merge-graphs
|
||||
# carrying ids across unchanged made community 0 of repo alpha and
|
||||
# community 0 of repo beta the SAME community — the aggregated community
|
||||
# view then fused unrelated communities into one meta-node. Each input's
|
||||
# ids must be offset into a shared id space, with the per-repo partition
|
||||
# kept in local_community.
|
||||
a = tmp_path / "alpha" / "graphify-out" / "graph.json"
|
||||
b = tmp_path / "beta" / "graphify-out" / "graph.json"
|
||||
_write_with_communities(a, [("a0", 0), ("a1", 0), ("a2", 1)])
|
||||
_write_with_communities(b, [("b0", 0), ("b1", 1), ("b2", 1), ("b3", 2)])
|
||||
out = tmp_path / "merged.json"
|
||||
|
||||
r = _run(["merge-graphs", str(a), str(b), "--out", str(out)], tmp_path)
|
||||
assert r.returncode == 0, r.stderr
|
||||
data = json.loads(out.read_text())
|
||||
a_nodes = [n for n in data["nodes"] if n["id"].startswith("alpha::")]
|
||||
b_nodes = [n for n in data["nodes"] if n["id"].startswith("beta::")]
|
||||
# the first input keeps its original ids (offset 0)
|
||||
assert {n["community"] for n in a_nodes} == {0, 1}
|
||||
# the second input is shifted past the first: {2, 3, 4}, disjoint
|
||||
assert {n["community"] for n in b_nodes} == {2, 3, 4}
|
||||
assert not (
|
||||
{n["community"] for n in a_nodes} & {n["community"] for n in b_nodes}
|
||||
), "community ids still collide across repos (#3014)"
|
||||
# all five distinct communities survive the merge
|
||||
assert len({n["community"] for n in data["nodes"]}) == 5
|
||||
# the per-repo partition is preserved
|
||||
assert {n["local_community"] for n in b_nodes} == {0, 1, 2}
|
||||
|
||||
Reference in New Issue
Block a user