fix(dedup): pick the richer duplicate as survivor and keep the losers' fields (#3372)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q

graphify/dedup.py

@@ -850,6 +850,8 @@ def deduplicate_entities(
         n["id"]: (i, n) for i, n in enumerate(unique_nodes)
     }
 
+    # Survivors enriched with their losers' fields, substituted at the end.
+    enriched_by_id: dict[str, dict] = {}
     for root, members in components.items():
         if len(members) == 1:
             continue
@@ -861,6 +863,17 @@ def deduplicate_entities(
         ]
         winner = _pick_winner(group_nodes) if group_nodes else {"id": root}
         winner_id = winner["id"]
+        # Even with the right survivor, dropping the losers wholesale loses
+        # whatever fields only they carried. Fill the survivor's absent
+        # fields from each loser — the same never-override merge the
+        # same-source collision path already applies (#2091/#3372); loser
+        # order follows unique_nodes order, so the fill is deterministic.
+        merged = winner
+        for node in group_nodes:
+            if node["id"] != winner_id:
+                merged = _merge_missing_attributes(merged, node)
+        if merged != winner:
+            enriched_by_id[winner_id] = merged
         for member in members:
             if member != winner_id:
                 remap[member] = winner_id
@@ -892,7 +905,9 @@ def deduplicate_entities(
     if hyperedges:
         _remap_hyperedge_members(hyperedges, remap)
 
-    deduped_nodes = [n for n in unique_nodes if n["id"] not in remap]
+    deduped_nodes = [
+        enriched_by_id.get(n["id"], n) for n in unique_nodes if n["id"] not in remap
+    ]
     deduped_edges = []
     for edge in edges:
         e = dict(edge)
@@ -916,14 +931,53 @@ def deduplicate_entities(
     return deduped_nodes, deduped_edges
 
 
+# Keys every node carries (or that describe placement rather than content).
+# They say nothing about which duplicate is the better-established record, so
+# the richness score below ignores them.
+_RICHNESS_IGNORED_KEYS = frozenset({
+    "id", "label", "norm_label", "file_type", "source_file", "source_location",
+})
+
+
+def _content_richness(n: dict) -> int:
+    """How much actual content a node carries, for survivor selection (#3372).
+
+    Counts populated fields beyond the identity/placement baseline, weighting
+    ``attributes`` by entry count and ``_merged_from`` by its history length —
+    a node that already absorbed prior merges is the established record.
+    """
+    score = 0
+    for key, value in n.items():
+        if key in _RICHNESS_IGNORED_KEYS:
+            continue
+        if value is None or value == "" or value == [] or value == {}:
+            continue
+        score += 1
+        if key == "attributes" and isinstance(value, dict):
+            score += len(value)
+        elif key == "_merged_from" and isinstance(value, list):
+            score += len(value)
+    return score
+
+
 def _pick_winner(nodes: list[dict]) -> dict:
-    """Pick the canonical survivor: prefer no chunk suffix, then shorter ID."""
+    """Pick the canonical survivor: no chunk suffix, then richer content,
+    then shorter ID.
+
+    ID length used to be the primary signal after the chunk-suffix check,
+    which made a passing one-line mention on a shallow page (short id, one
+    fewer path segment) beat the dedicated, enriched page for the same entity
+    — every time the pattern occurred, the established node's content was
+    discarded (#3372). Content richness now decides first; ID shape only
+    breaks ties between equally-rich candidates, preserving the old
+    deterministic ordering there.
+    """
     if not nodes:
         raise ValueError("Cannot pick winner from empty list")
 
-    def _score(n: dict) -> tuple[int, int]:
+    def _score(n: dict) -> tuple[int, int, int]:
         has_suffix = bool(_CHUNK_SUFFIX.search(n["id"]))
-        return (1 if has_suffix else 0, len(n["id"]))
+        return (1 if has_suffix else 0, -_content_richness(n), len(n["id"]))
 
     return min(nodes, key=_score)
 

tests/test_dedup_survivor_richness.py

@@ -0,0 +1,98 @@
+"""Dedup survivor selection prefers content over ID shape (#3372).
+
+`_pick_winner` scored purely on chunk-suffix + ID length, so a passing
+one-line mention on a shallow page (short id) beat the dedicated, enriched
+page for the same entity, and the losers were dropped wholesale — the
+established node's attributes, description, confidence and merge history
+were discarded every time the pattern occurred. Richness now decides first
+(ID shape breaks ties), and the survivor is back-filled with any fields only
+a loser carried.
+"""
+
+from graphify.dedup import _pick_winner, deduplicate_entities
+
+
+def _rich(**over):
+    node = {
+        "id": "topics_networking_widget_x_widget_x",
+        "label": "Widget X",
+        "file_type": "concept",
+        "source_file": "topics/networking/widget-x.md",
+        "source_location": "L1",
+        "description": "The flagship telemetry relay used by every edge deployment.",
+        "attributes": {"protocol": "mqtt", "version": "3.2", "owner": "platform"},
+        "confidence": "EXTRACTED",
+        "_merged_from": ["widget_x_old"],
+    }
+    node.update(over)
+    return node
+
+
+def _shallow(**over):
+    node = {
+        "id": "sources_notes_widget_x",
+        "label": "Widget X",
+        "file_type": "concept",
+        "source_file": "sources/notes.md",
+        "source_location": "L14",
+    }
+    node.update(over)
+    return node
+
+
+def test_richer_node_survives_despite_longer_id():
+    """The issue's exact shape: dedicated nested page vs shallow mention."""
+    dn, _ = deduplicate_entities([_rich(), _shallow()], [], communities={})
+    labels = [n for n in dn if n["label"] == "Widget X"]
+    assert len(labels) == 1
+    surv = labels[0]
+    assert surv["id"] == "topics_networking_widget_x_widget_x"
+    assert surv["attributes"] == {"protocol": "mqtt", "version": "3.2",
+                                  "owner": "platform"}
+    assert surv["_merged_from"] == ["widget_x_old"]
+
+
+def test_loser_only_fields_are_folded_into_the_survivor():
+    """Even the right winner must not lose what only a loser carried."""
+    rich = _rich()
+    shallow = _shallow(summary="Mentioned during the Q3 review.")
+    dn, _ = deduplicate_entities([rich, shallow], [], communities={})
+    surv = next(n for n in dn if n["label"] == "Widget X")
+    assert surv["id"] == rich["id"]
+    assert surv.get("summary") == "Mentioned during the Q3 review."
+    # Never-override: the survivor's own fields stay its own.
+    assert surv["description"].startswith("The flagship")
+
+
+def test_equally_bare_nodes_keep_the_shorter_id_tiebreak():
+    """Old deterministic ordering survives for content-equal candidates."""
+    a = _shallow(id="a_widget_x", source_file="a/notes.md")
+    b = _shallow(id="longer_b_widget_x", source_file="b/notes.md")
+    assert _pick_winner([a, b])["id"] == "a_widget_x"
+    assert _pick_winner([b, a])["id"] == "a_widget_x"
+
+
+def test_chunk_suffix_still_dominates_richness():
+    """A chunk-suffixed id never beats a clean one, however rich."""
+    chunked = _rich(id="topics_widget_x_c3")
+    clean = _shallow()
+    assert _pick_winner([chunked, clean])["id"] == "sources_notes_widget_x"
+
+
+def test_richness_counts_content_not_placement():
+    from graphify.dedup import _content_richness
+
+    assert _content_richness(_shallow()) == 0
+    assert _content_richness(_rich()) > _content_richness(
+        _rich(attributes={"protocol": "mqtt"}))
+
+
+def test_edges_rewire_to_the_rich_survivor():
+    edges = [{"source": "sources_notes_widget_x", "target": "other",
+              "relation": "references", "source_file": "sources/notes.md"}]
+    dn, de = deduplicate_entities(
+        [_rich(), _shallow(), {"id": "other", "label": "Other page",
+                               "file_type": "concept",
+                               "source_file": "docs/other.md"}],
+        edges, communities={})
+    assert de[0]["source"] == "topics_networking_widget_x_widget_x"