From 6a7409c06d05da5a7020a2b87d07b86542bfbb51 Mon Sep 17 00:00:00 2001 From: SrijanSriv Date: Sun, 4 Oct 2026 22:26:26 +0530 Subject: [PATCH] fix(dedup): keep unstamped document nodes apart across files The exact pass joined equal labels without calling _crossfile_fileanchored_blocked, so repeated headings collapsed across source files. The fuzzy pass already had that block. Fixes #3094 --- graphify/dedup.py | 71 ++++++++++++++++++--------------- tests/test_dedup.py | 96 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 121 insertions(+), 46 deletions(-) diff --git a/graphify/dedup.py b/graphify/dedup.py index b1b247571..ff4bcd868 100644 --- a/graphify/dedup.py +++ b/graphify/dedup.py @@ -678,6 +678,30 @@ def deduplicate_entities( if prot is not None: prot_by_root[new_root] = prot + def _crossfile_union(left: dict, right: dict, *, with_prot: bool) -> None: + nonlocal exact_merges + if _crossfile_fileanchored_blocked(left, right): + return + left_id = left["id"] + right_id = right["id"] + if with_prot: + px = _get_prot(left_id) + py = _get_prot(right_id) + if px is not None and py is not None and px != py: + return + if uf.find(left_id) == uf.find(right_id): + return + if with_prot: + _union_with_prot(left_id, right_id) + else: + uf.union(left_id, right_id) + exact_merges += 1 + + def _crossfile_join(members: list[dict], *, with_prot: bool) -> None: + for index, left in enumerate(members): + for right in members[index + 1:]: + _crossfile_union(left, right, with_prot=with_prot) + for key, group in norm_to_nodes.items(): if len(group) <= 1: continue @@ -726,20 +750,15 @@ def deduplicate_entities( # it is provably safe (#2182). `concept` is the one file_type meant to # unify across files (#1284) — code is keyed by ID (#1205) and # image/paper labels are often shared basenames (logo.png), so both stay - # blocked. rationale/document join `concept` here ONLY when the node - # reads as an entity inside its file (#296): a file-anchored - # *file_type* does not make an individual node file-anchored. An entity - # extracted from a note — a person, a project — inherits `document` from - # the file's extension, not from anything about itself, so in note-heavy - # corpora almost no entity node is typed `concept` and this merge never - # got to run on them. A file's own node and its headings still never - # merge (#1284, #3094). + # blocked. A rationale or document node can still enter this list when + # it reads as an entity (#296). Each union calls + # `_crossfile_fileanchored_blocked`, so that node does not join a node + # from another file (#3094). Concept nodes still join across files. + # A file's own node and its headings never enter the list (#1284). # Provenance is required (#1178), and the entropy gate mirrors Pass 2 so # short generic labels ("API") stay distinct — both untouched here. - # Scoped to this exact-normalization pass: Pass 2's fuzzy - # `_crossfile_fileanchored_blocked` is unchanged, so #1284's - # near-identical boilerplate and heading siblings stay blocked. - # Sorting by id keeps the winner order-independent. + # Pass 2 still calls the same block, so near-identical boilerplate stays + # blocked (#1284). Sorting by id keeps the pair order stable. mergeable = sorted( (n for n in group if (n.get("file_type") == "concept" @@ -758,30 +777,18 @@ def deduplicate_entities( # NEVER collapse them during incremental merge. continue if prot_members: - # Mixed: pick AT MOST ONE protected survivor for incoming nodes to fold into. - # Multiple protected nodes must remain separate independent entities. + # Mixed: fold an incoming node into at most one protected + # survivor. Protected nodes stay separate from each other. + # Incoming nodes the block still allows are joined to each + # other, so a blocked winner does not leave them apart. canonical_winner = _pick_winner(prot_members) for inc in inc_members: - px = _get_prot(canonical_winner["id"]) - py = _get_prot(inc["id"]) - if px is not None and py is not None and px != py: - continue - if uf.find(canonical_winner["id"]) != uf.find(inc["id"]): - _union_with_prot(canonical_winner["id"], inc["id"]) - exact_merges += 1 + _crossfile_union(canonical_winner, inc, with_prot=True) + _crossfile_join(inc_members, with_prot=True) else: - # Incoming only: merge normally - winner = _pick_winner(inc_members) - for node in inc_members: - if uf.find(winner["id"]) != uf.find(node["id"]): - _union_with_prot(winner["id"], node["id"]) - exact_merges += 1 + _crossfile_join(inc_members, with_prot=True) else: - winner = _pick_winner(mergeable) - for node in mergeable: - if uf.find(winner["id"]) != uf.find(node["id"]): - uf.union(winner["id"], node["id"]) - exact_merges += 1 + _crossfile_join(mergeable, with_prot=False) # ── pass 2: MinHash/LSH + Jaro-Winkler (high-entropy nodes only) ───────── candidates: list[dict] = [] diff --git a/tests/test_dedup.py b/tests/test_dedup.py index b855fbce5..4264f29a7 100644 --- a/tests/test_dedup.py +++ b/tests/test_dedup.py @@ -1346,24 +1346,22 @@ def test_reads_as_file_entity_helper(): def test_dedup_merges_crossfile_document_entity_variants(): - """The reported bug (#296): case/prefix variants of one entity, extracted - from three different notes and typed `document` by extension, must collapse - to a single node.""" + """Unstamped document nodes that share a label stay one node per file. + + The exact pass calls the same cross-file block as the fuzzy pass (#3094). + Three notes no longer collapse to one node. + """ nodes = [ {"id": "journal_2024_03_0%d_cyrilxbt" % i, "label": variant, "file_type": "document", "source_file": "journal/2024-03-0%d.md" % i} for i, variant in enumerate(_CYRIL_VARIANTS, start=1) ] result_nodes, _ = deduplicate_entities(nodes, [], communities={}) - assert len(result_nodes) == 1, ( - "cross-file `document` entity variants did not merge -- the #296 gate " - "widening is not reaching Pass 1's cross-file residue" - ) + assert len(result_nodes) == 3 def test_dedup_merges_crossfile_rationale_entity_variants(): - """`rationale` rides the same gate as `document` (#296): entity nodes of - that type, provably not their files' own nodes, merge on an exact label.""" + """Unstamped rationale nodes that share a label stay one node per file (#3094).""" nodes = [ {"id": "svc_alpha_py_retention_window", "label": "Retention Window", "file_type": "rationale", "source_file": "svc/alpha.py"}, @@ -1371,7 +1369,7 @@ def test_dedup_merges_crossfile_rationale_entity_variants(): "file_type": "rationale", "source_file": "svc/beta.py"}, ] result_nodes, _ = deduplicate_entities(nodes, [], communities={}) - assert len(result_nodes) == 1 + assert len(result_nodes) == 2 def test_dedup_never_merges_a_files_own_node_away(): @@ -1434,10 +1432,11 @@ def test_dedup_crossfile_entity_merge_keeps_the_provenance_gate(): def test_dedup_crossfile_fuzzy_fileanchored_block_is_untouched(): - """#296 widens only the exact-normalization pass. Pass 2's fuzzy - `_crossfile_fileanchored_blocked` is unchanged, so near-identical (not - identical) document labels in different files still stay distinct -- the - #1284 guard keeps doing its job on entity nodes too.""" + """Pass 2 still refuses near-identical document labels in different files. + + The exact pass now calls the same block (#3094). This pair is not an exact + label match, so only the fuzzy pass can see it, and the block keeps both nodes. + """ nodes = [ {"id": "docs_a_guide", "label": "Getting Started Installation Guide", "file_type": "document", "source_file": "docs/a.md"}, @@ -1448,6 +1447,75 @@ def test_dedup_crossfile_fuzzy_fileanchored_block_is_untouched(): assert len(result_nodes) == 2 +def _doc(node_id, label, source_file, file_type="document"): + return { + "id": node_id, + "label": label, + "file_type": file_type, + "source_file": source_file, + "source_location": None, + } + + +def test_exact_pass_keeps_unstamped_documents_across_files(tmp_path, capsys): + """The 0.9.66 script: five unstamped document nodes, three source files.""" + from graphify.build import build_merge + + extraction = {"nodes": [ + _doc("a_decisions", "Decisions", "Sessions/session-A.md"), + _doc("b_decisions", "Decisions", "Sessions/session-B.md"), + _doc("c_decisions", "Decisions", "Sessions/session-C.md"), + _doc("a_open_items", "Open items", "Sessions/session-A.md"), + _doc("b_open_items", "Open items", "Sessions/session-B.md"), + ], "edges": [], "hyperedges": [], "input_tokens": 0, "output_tokens": 0} + graph = build_merge( + [extraction], + graph_path=tmp_path / "missing.json", + dedup=True, + ) + files = {data.get("source_file") for _, data in graph.nodes(data=True)} + assert graph.number_of_nodes() == 5 + assert files == { + "Sessions/session-A.md", + "Sessions/session-B.md", + "Sessions/session-C.md", + } + assert "Deduplicated" not in capsys.readouterr().out + + +def test_exact_pass_still_merges_same_file_document_duplicate(): + nodes = [ + _doc("a1", "Decisions", "Sessions/session-A.md"), + _doc("a2", "Decisions", "Sessions/session-A.md"), + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + assert len(result_nodes) == 1 + + +def test_exact_pass_still_merges_crossfile_concepts(): + nodes = [ + _doc("c1", "Decisions", "Sessions/session-A.md", file_type="concept"), + _doc("c2", "Decisions", "Sessions/session-B.md", file_type="concept"), + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + assert len(result_nodes) == 1 + + +def test_exact_pass_merges_concepts_beside_a_blocked_document(): + """A document winner must not leave the two concept nodes unjoined.""" + nodes = [ + _doc("a", "Decisions", "Sessions/session-A.md"), + _doc("c1", "Decisions", "Sessions/session-B.md", file_type="concept"), + _doc("c2", "Decisions", "Sessions/session-C.md", file_type="concept"), + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + kinds = sorted(node["file_type"] for node in result_nodes) + ids = {node["id"] for node in result_nodes} + assert len(result_nodes) == 2 + assert kinds == ["concept", "document"] + assert "a" in ids + + def test_dedup_never_merges_repeated_headings_across_files(): """#3094's verification case must keep holding under #296: sibling documents that each carry the same `## Decisions` / `## Next steps` sections keep one