Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions graphify/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
53 changes: 53 additions & 0 deletions graphify/dedup.py
Original file line number Diff line number Diff line change
Expand Up @@ -460,13 +460,54 @@ 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(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regressiondeduplicate_entities()

fans out to 21 callees (efferent coupling); 63 callers depend on it (afferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

nodes: list[dict],
edges: list[dict],
*,
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.

Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down
127 changes: 127 additions & 0 deletions tests/test_dedup_remaps_hyperedges.py
Original file line number Diff line number Diff line change
@@ -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"]
Loading