mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-28 01:06:36 +00:00
fix(dedup): rewire hyperedge members onto survivors instead of dropping them (#2805)
Node dedup rewired edge endpoints to survivors but never remapped hyperedge members, so a member naming a merged-away node silently vanished from the rebuilt graph (the group shrank with no dangling reference). deduplicate_entities now rewires hyperedge member ids through the same union-find survivor map the edges use, de-duplicating members within each hyperedge; id-less/malformed hyperedges pass through untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
795102cc28
commit
0d145fe110
@@ -1369,6 +1369,9 @@ def build(
|
||||
combined["nodes"], combined["edges"] = deduplicate_entities(
|
||||
combined["nodes"], combined["edges"], communities={},
|
||||
dedup_llm_backend=dedup_llm_backend, root=root,
|
||||
# Hyperedge members reference node ids too, so they need the same
|
||||
# survivor rewiring the edges get (#2805).
|
||||
hyperedges=combined.get("hyperedges"),
|
||||
)
|
||||
return build_from_json(combined, directed=directed, root=root)
|
||||
|
||||
|
||||
@@ -460,6 +460,46 @@ def _report_id_collision(nid: str, survivor: dict, losers: list[dict]) -> None:
|
||||
|
||||
# ── main entry point ──────────────────────────────────────────────────────────
|
||||
|
||||
def _remap_hyperedge_members(hyperedges: list[dict], remap: dict[str, str]) -> None:
|
||||
"""Rewire hyperedge member ids onto dedup survivors, in place.
|
||||
|
||||
Members come in both shapes the rest of the codebase tolerates — a bare id
|
||||
string, or an object carrying one — so both are handled;
|
||||
``_normalize_hyperedge_members`` fixes the SHAPE but never resolves a member
|
||||
against surviving node ids, which is why this is needed as well.
|
||||
|
||||
Two members that remap onto the same survivor collapse to one entry. That
|
||||
shrinks the group, but honestly: they were the same entity, and the previous
|
||||
behaviour dropped the loser without promoting it, which shrank the group
|
||||
*and* lost the participant. Order is preserved so a rebuilt graph does not
|
||||
churn.
|
||||
"""
|
||||
for he in hyperedges:
|
||||
if not isinstance(he, dict):
|
||||
continue
|
||||
members = he.get("nodes")
|
||||
if not isinstance(members, list):
|
||||
continue
|
||||
seen: set = set()
|
||||
rewired: list = []
|
||||
for m in members:
|
||||
if isinstance(m, str):
|
||||
new_id = remap.get(m, m)
|
||||
entry = new_id
|
||||
elif isinstance(m, dict):
|
||||
raw = m.get("id")
|
||||
new_id = remap.get(raw, raw) if isinstance(raw, str) else raw
|
||||
entry = dict(m, id=new_id) if new_id != raw else m
|
||||
else:
|
||||
new_id, entry = None, m
|
||||
if isinstance(new_id, str):
|
||||
if new_id in seen:
|
||||
continue
|
||||
seen.add(new_id)
|
||||
rewired.append(entry)
|
||||
he["nodes"] = rewired
|
||||
|
||||
|
||||
def deduplicate_entities(
|
||||
nodes: list[dict],
|
||||
edges: list[dict],
|
||||
@@ -467,6 +507,7 @@ def deduplicate_entities(
|
||||
communities: dict[str, int],
|
||||
dedup_llm_backend: str | None = None,
|
||||
root: str | Path | None = None,
|
||||
hyperedges: "list[dict] | None" = None,
|
||||
) -> tuple[list[dict], list[dict]]:
|
||||
"""Deduplicate near-identical entities in a knowledge graph.
|
||||
|
||||
@@ -477,6 +518,9 @@ def deduplicate_entities(
|
||||
dedup_llm_backend: if set, use LLM to resolve ambiguous pairs
|
||||
root: scan root; ID-collision ranking judges source paths relative to
|
||||
it so path form and checkout location cannot flip the survivor (#2532)
|
||||
hyperedges: when given, member ids are rewired to survivors IN PLACE,
|
||||
the same way edge endpoints are. Optional and mutating rather than
|
||||
returned so existing two-tuple callers are unaffected (#2805).
|
||||
|
||||
Returns:
|
||||
(deduped_nodes, deduped_edges) with edges rewired to survivors
|
||||
@@ -779,6 +823,15 @@ def deduplicate_entities(
|
||||
msg += f" ({', '.join(parts)})"
|
||||
print(msg + ".", flush=True)
|
||||
|
||||
# Hyperedge members are node references exactly like edge endpoints, and
|
||||
# must follow the survivor for the same reason. Without this the member
|
||||
# naming a merged-away id was simply absent from the rebuilt graph: the
|
||||
# group lost a participant silently, could fall under the 3-member threshold
|
||||
# that makes it a hyperedge at all, and left NO dangling reference, so a
|
||||
# referential-integrity check saw nothing wrong (#2805).
|
||||
if hyperedges:
|
||||
_remap_hyperedge_members(hyperedges, remap)
|
||||
|
||||
deduped_nodes = [n for n in unique_nodes if n["id"] not in remap]
|
||||
deduped_edges = []
|
||||
for edge in edges:
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
"""Dedup must rewire hyperedge members onto survivors, not drop them.
|
||||
|
||||
`build()` rewires EDGE endpoints to dedup survivors, but `combined["hyperedges"]`
|
||||
never went through the same remap. The member naming a merged-away id was simply
|
||||
absent from the rebuilt graph, so the group lost a participant — and could fall
|
||||
under the 3-member threshold that makes it a hyperedge at all — with nothing on
|
||||
stderr and, crucially, **no dangling reference**, so a referential-integrity
|
||||
check saw a perfectly consistent graph (#2805).
|
||||
|
||||
`_normalize_hyperedge_members` / `_coerce_hyperedge_member_refs` normalise member
|
||||
SHAPE (bare id vs object) but never resolve a member against surviving node ids,
|
||||
which is why they do not cover this.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from graphify.build import build
|
||||
from graphify.dedup import _remap_hyperedge_members
|
||||
|
||||
|
||||
def _node(nid, label):
|
||||
return {"id": nid, "label": label, "file_type": "concept",
|
||||
"source_file": "notes/a.md"}
|
||||
|
||||
|
||||
def _extraction(members):
|
||||
"""Two nodes that normalise to the same label, so dedup merges them; the
|
||||
hyperedge names the id that loses."""
|
||||
return {
|
||||
"nodes": [
|
||||
_node("alpha_a", "Alpha Concept"),
|
||||
_node("alpha_concept_long_variant_id", "alpha concept"),
|
||||
_node("beta_node", "Beta"),
|
||||
_node("gamma_node", "Gamma"),
|
||||
],
|
||||
"edges": [],
|
||||
"hyperedges": [{"id": "the_group", "label": "The Group",
|
||||
"nodes": members, "relation": "participate_in",
|
||||
"confidence": "INFERRED", "confidence_score": 0.75,
|
||||
"source_file": "notes/a.md"}],
|
||||
}
|
||||
|
||||
|
||||
def _members(G):
|
||||
hes = G.graph.get("hyperedges", [])
|
||||
assert len(hes) == 1, hes
|
||||
return [m if isinstance(m, str) else m.get("id") for m in hes[0]["nodes"]]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The bug
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_member_follows_the_survivor_instead_of_vanishing():
|
||||
G = build([_extraction(
|
||||
["alpha_concept_long_variant_id", "beta_node", "gamma_node"])])
|
||||
assert _members(G) == ["alpha_a", "beta_node", "gamma_node"]
|
||||
|
||||
|
||||
def test_the_group_keeps_its_size():
|
||||
"""The quiet part: a group of 3 became a group of 2, which can drop it below
|
||||
the threshold that makes it a hyperedge."""
|
||||
G = build([_extraction(
|
||||
["alpha_concept_long_variant_id", "beta_node", "gamma_node"])])
|
||||
assert len(_members(G)) == 3
|
||||
|
||||
|
||||
def test_no_member_is_left_pointing_at_a_merged_away_id():
|
||||
G = build([_extraction(
|
||||
["alpha_concept_long_variant_id", "beta_node", "gamma_node"])])
|
||||
assert all(m in G.nodes for m in _members(G))
|
||||
assert "alpha_concept_long_variant_id" not in G.nodes
|
||||
|
||||
|
||||
def test_object_shaped_members_are_remapped_too():
|
||||
"""Members are tolerated as bare ids or as objects carrying one."""
|
||||
G = build([_extraction([
|
||||
{"id": "alpha_concept_long_variant_id", "role": "subject"},
|
||||
{"id": "beta_node"}, {"id": "gamma_node"},
|
||||
])])
|
||||
assert _members(G) == ["alpha_a", "beta_node", "gamma_node"]
|
||||
|
||||
|
||||
def test_an_untouched_hyperedge_is_unchanged():
|
||||
G = build([_extraction(["alpha_a", "beta_node", "gamma_node"])])
|
||||
assert _members(G) == ["alpha_a", "beta_node", "gamma_node"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _remap_hyperedge_members directly
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_two_members_collapsing_onto_one_survivor_dedupe():
|
||||
"""They were the same entity, so one entry is right. The old code shrank the
|
||||
group AND lost the participant; this shrinks it because the members really
|
||||
were duplicates."""
|
||||
hes = [{"id": "h", "nodes": ["a_old", "a_new", "b"]}]
|
||||
_remap_hyperedge_members(hes, {"a_old": "a", "a_new": "a"})
|
||||
assert hes[0]["nodes"] == ["a", "b"]
|
||||
|
||||
|
||||
def test_member_order_is_preserved():
|
||||
hes = [{"id": "h", "nodes": ["c", "b_old", "a"]}]
|
||||
_remap_hyperedge_members(hes, {"b_old": "b"})
|
||||
assert hes[0]["nodes"] == ["c", "b", "a"]
|
||||
|
||||
|
||||
def test_object_members_keep_their_other_fields():
|
||||
hes = [{"id": "h", "nodes": [{"id": "x_old", "role": "subject"}]}]
|
||||
_remap_hyperedge_members(hes, {"x_old": "x"})
|
||||
assert hes[0]["nodes"] == [{"id": "x", "role": "subject"}]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("he", [
|
||||
{"id": "h"}, # no members key
|
||||
{"id": "h", "nodes": None}, # members not a list
|
||||
{"id": "h", "nodes": []}, # empty
|
||||
{"id": "h", "nodes": [None, 7]}, # junk members
|
||||
"not-a-dict",
|
||||
])
|
||||
def test_malformed_hyperedges_do_not_raise(he):
|
||||
_remap_hyperedge_members([he], {"a": "b"})
|
||||
|
||||
|
||||
def test_an_empty_remap_changes_nothing():
|
||||
hes = [{"id": "h", "nodes": ["a", "b", "c"]}]
|
||||
_remap_hyperedge_members(hes, {})
|
||||
assert hes[0]["nodes"] == ["a", "b", "c"]
|
||||
Reference in New Issue
Block a user