From 0d145fe110e2d627297f9b03e558de33dfd3f8a6 Mon Sep 17 00:00:00 2001 From: abhay-codes07 <182421137+abhay-codes07@users.noreply.github.com> Date: Mon, 17 Aug 2026 15:34:16 +0100 Subject: [PATCH] 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) --- graphify/build.py | 3 + graphify/dedup.py | 53 +++++++++++ tests/test_dedup_remaps_hyperedges.py | 127 ++++++++++++++++++++++++++ 3 files changed, 183 insertions(+) create mode 100644 tests/test_dedup_remaps_hyperedges.py diff --git a/graphify/build.py b/graphify/build.py index 6cae7539..470f2c6b 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -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) diff --git a/graphify/dedup.py b/graphify/dedup.py index a777bdca..4816f103 100644 --- a/graphify/dedup.py +++ b/graphify/dedup.py @@ -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: diff --git a/tests/test_dedup_remaps_hyperedges.py b/tests/test_dedup_remaps_hyperedges.py new file mode 100644 index 00000000..45a59686 --- /dev/null +++ b/tests/test_dedup_remaps_hyperedges.py @@ -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"]