From a719dcd6f7cacdc7da1fa0f95c0bc2aa071409f8 Mon Sep 17 00:00:00 2001 From: abhay-codes07 Date: Mon, 17 Aug 2026 14:43:22 +0530 Subject: [PATCH] fix(dedup): rewire hyperedge members onto survivors instead of dropping them (#2805) build() rewires EDGE endpoints to dedup survivors, but combined["hyperedges"] never went through the same remap -- dedup.py had no reference to hyperedges at all. The member naming a merged-away id was simply absent from the rebuilt graph. The failure is quiet in the worst way. The group loses a participant, can fall under the 3-member threshold that makes it a hyperedge at all, and leaves NO dangling reference behind -- so a referential-integrity check sees a perfectly consistent graph. On the reporter's fixture a 3-member group came back with 2. _normalize_hyperedge_members / _coerce_hyperedge_member_refs normalise member SHAPE (bare id vs object carrying one) but never resolve a member against surviving node ids, so they did not cover this. deduplicate_entities now takes an optional `hyperedges` list and rewires member ids in place, alongside the edge rewiring it already did. Optional and mutating rather than added to the return tuple so every existing two-tuple caller is unaffected; deduplicate_by_label has the same shape and is left alone since nothing routes hyperedges through it. Both member shapes are handled, other fields on object members are preserved, and member order is kept so a rebuilt graph does not churn. Two members that remap onto the SAME survivor collapse to one entry: that shrinks the group, but they were the same entity -- where the old behaviour shrank it and lost the participant as well. --- 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 7f195fe2f4..05c7ebf221 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -1340,6 +1340,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 a777bdca13..4816f103b6 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 0000000000..45a5968683 --- /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"]