From e395ff9b433a441da3848a18bd4c82c0c9b888fc Mon Sep 17 00:00:00 2001 From: safishamsi Date: Sun, 26 Jul 2026 11:52:20 +0100 Subject: [PATCH] fix(dedup): merge cross-file concept nodes with identical normalized labels (#2182) Pass 1 deferred cross-file exact matches to Pass 2, but Pass 2's candidate filter keeps only the first node per normalized label, so identical-label cross-file concept pairs could never merge (while fuzzy pairs did). Pass 1 now unions the cross-file residue of each label group, gated to concept nodes with provenance and above the entropy floor, so code/rationale/ document/image/empty-source and cross-repo guards are all preserved. Co-Authored-By: Claude Opus 4.8 (1M context) --- graphify/dedup.py | 38 ++++++++-- tests/test_dedup.py | 177 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 209 insertions(+), 6 deletions(-) diff --git a/graphify/dedup.py b/graphify/dedup.py index 0e1341e7..bc2f1c41 100644 --- a/graphify/dedup.py +++ b/graphify/dedup.py @@ -411,8 +411,10 @@ def deduplicate_entities( for key, group in norm_to_nodes.items(): if len(group) <= 1: continue - # Partition by source_file — only merge within the same file in Pass 1. - # Cross-file matches fall through to Pass 2 fuzzy matching. + # Partition by source_file — same-file exact matches always merge here. + # Cross-file exact matches are handled just below, gated to `concept` + # nodes only: Pass 2 cannot form them because its candidate list keeps a + # single node per normalized label (#2182). by_file: dict[str, list[dict]] = defaultdict(list) for node in group: sf = node.get("source_file") or "" @@ -427,6 +429,26 @@ def deduplicate_entities( for node in file_group: uf.union(winner["id"], node["id"]) exact_merges += len(file_group) - 1 + # Cross-file residue: union exact matches across files, but only where + # it is provably safe (#2182). `concept` is the one file_type meant to + # unify across files (#1284) — code is keyed by ID (#1205), rationale/ + # document are file-anchored (#1284), and image/paper labels are often + # shared basenames (logo.png). Provenance is required (#1178), and the + # entropy gate mirrors Pass 2 so short generic labels ("API") stay + # distinct. Sorting by id keeps the winner order-independent. + mergeable = sorted( + (n for n in group + if n.get("file_type") == "concept" + and (n.get("source_file") or "") + and _entropy(n.get("label", "")) >= _ENTROPY_THRESHOLD), + key=lambda n: n["id"], + ) + if len(mergeable) > 1: + 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 # ── pass 2: MinHash/LSH + Jaro-Winkler (high-entropy nodes only) ───────── candidates: list[dict] = [] @@ -522,10 +544,14 @@ def deduplicate_entities( score += _COMMUNITY_BOOST if score >= _MERGE_THRESHOLD: - # Identical labels across different source files almost always - # means same-named-but-different symbols (trait impls, wrapper - # methods, common type names). Mirror Pass 1's source_file - # partition for this sub-case. (#1046, leaks #895's fix) + # Belt-and-braces (#1046, narrowed by #2182): candidates are + # norm-unique (`seen_norms` above), so two candidates can + # never share a normalized label and this branch is + # unreachable today. Retained in case candidate selection + # changes. Equal-norm cross-file pairs are handled in Pass 1 + # instead, gated to `concept` nodes — the original #1046 + # rationale (same-named code symbols) was obsoleted by code + # being excluded from label matching entirely (#1205, #1247). if norm_label == neighbor_norm: sf_a = node.get("source_file") or "" sf_b = neighbor.get("source_file") or "" diff --git a/tests/test_dedup.py b/tests/test_dedup.py index ab38b0f4..e1370fc7 100644 --- a/tests/test_dedup.py +++ b/tests/test_dedup.py @@ -692,3 +692,180 @@ def test_dedup_fills_explicit_none_attribute(): result, _ = deduplicate_entities([dict(n) for n in nodes], [], communities={}) assert len(result) == 1 assert result[0].get("source_location") == "L7", "explicit-None must be filled from the loser" + + +# ── #2182: cross-file exact-duplicate concepts must merge ───────────────────── + +def test_crossfile_identical_concepts_merge_and_rewire(): + """Two `concept` nodes whose labels are byte-identical after _norm() but + live in different files must merge (#2182). Pass 1 used to defer them to + Pass 2, whose norm-unique candidate filter (`seen_norms`) structurally + cannot form an equal-norm pair — so exact cross-file duplicates were the + one class of duplicate that never merged, while a one-char-different + fuzzy pair did.""" + nodes = [ + {"id": "sz_intl", "label": "SHENZHEN INTERNATIONAL", + "file_type": "concept", "source_file": "doc1.md"}, + {"id": "shenzhen_international_holdings", "label": "Shenzhen international", + "file_type": "concept", "source_file": "doc2.md"}, + {"id": "port_ops", "label": "Port Operations", + "file_type": "concept", "source_file": "doc2.md"}, + ] + edges = [{"source": "shenzhen_international_holdings", "target": "port_ops", + "relation": "operates"}] + result_nodes, result_edges = deduplicate_entities(nodes, edges, communities={}) + ids = {n["id"] for n in result_nodes} + assert len(result_nodes) == 2 + # _pick_winner prefers the shorter, non-chunk-suffixed id. + assert "sz_intl" in ids + assert "shenzhen_international_holdings" not in ids + # The loser's edge is rewired to the winner. + assert result_edges == [ + {"source": "sz_intl", "target": "port_ops", "relation": "operates"}] + + +def test_crossfile_one_char_typo_concepts_still_merge(): + """Non-regression: the near-identical (one-char-different) cross-file pair + that already merged via Pass 2 fuzzy matching must keep merging (#2182).""" + nodes = [ + {"id": "g1", "label": "Authentication Manager", + "file_type": "concept", "source_file": "a.md"}, + {"id": "g2", "label": "Authentication Managr", + "file_type": "concept", "source_file": "b.md"}, + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + assert len(result_nodes) == 1 + + +_RATIONALE_BOILER = ("Django app config for apps.platform.cards. No business " + "logic here. Domain services live in services.py.") + + +@pytest.mark.parametrize("a,b", [ + ({"id": "d1", "label": "Getting Started Installation Guide", + "file_type": "document", "source_file": "docs/a.md"}, + {"id": "d2", "label": "Getting Started Installation Guide", + "file_type": "document", "source_file": "docs/b.md"}), + ({"id": "r1", "label": _RATIONALE_BOILER, + "file_type": "rationale", "source_file": "apps/platform/cards/apps.py"}, + {"id": "r2", "label": _RATIONALE_BOILER, + "file_type": "rationale", "source_file": "apps/platform/cores/apps.py"}), + ({"id": "backend_a_render_frame", "label": "render_frame", + "file_type": "code", "source_file": "backend_a.py"}, + {"id": "backend_b_render_frame", "label": "render_frame", + "file_type": "code", "source_file": "backend_b.py"}), + ({"id": "web_logo", "label": "logo.png", + "file_type": "image", "source_file": "web/assets/logo.png"}, + {"id": "docs_logo", "label": "logo.png", + "file_type": "image", "source_file": "docs/img/logo.png"}), + ({"id": "logo_concept", "label": "logo.png", + "file_type": "concept", "source_file": "doc1.md"}, + {"id": "logo_image", "label": "logo.png", + "file_type": "image", "source_file": "assets/logo.png"}), + ({"id": "shenzhen_a", "label": "Shenzhen International", + "file_type": "concept", "source_file": ""}, + {"id": "shenzhen_b", "label": "Shenzhen International", + "file_type": "concept", "source_file": ""}), + ({"id": "api_a", "label": "API", + "file_type": "concept", "source_file": "doc1.md"}, + {"id": "api_b", "label": "API", + "file_type": "concept", "source_file": "doc2.md"}), +], ids=["document", "rationale", "code", "image-basename", "concept-image-mixed", + "empty-source-file", "low-entropy-concept"]) +def test_crossfile_identical_labels_stay_distinct_for_guarded_types(a, b): + """The #2182 fix is gated to high-entropy `concept` nodes with provenance + on BOTH sides. Identical labels must NOT merge for: file-anchored types + (document/rationale, #1284), code (#1205), images sharing a basename in + different dirs, mixed concept+image pairs, provenance-less nodes (#1178), + and low-entropy generic labels.""" + result_nodes, _ = deduplicate_entities([dict(a), dict(b)], [], communities={}) + assert len(result_nodes) == 2, ( + f"guarded pair ({a['id']}, {b['id']}) was merged — #2182 fix leaked " + f"past its concept-only gate" + ) + + +def test_cross_repo_guard_still_raises(): + """The cross-repo guard is untouched by #2182: identical concepts from + different repos must still raise, never merge.""" + nodes = [ + {"id": "c1", "label": "Shenzhen International", "file_type": "concept", + "source_file": "doc1.md", "repo": "repo-a"}, + {"id": "c2", "label": "Shenzhen International", "file_type": "concept", + "source_file": "doc2.md", "repo": "repo-b"}, + ] + with pytest.raises(ValueError, match="multiple repos"): + deduplicate_entities(nodes, [], communities={}) + + +def test_crossfile_concept_merge_is_order_independent(): + """Three identical-norm concepts across three files: every input order must + yield the same single survivor (#2182). Winner ids differ in length so + _pick_winner has a unique minimum.""" + import itertools + base = [ + {"id": "shenzhen", "label": "SHENZHEN INTERNATIONAL", + "file_type": "concept", "source_file": "doc1.md"}, + {"id": "shenzhen_intl", "label": "Shenzhen international", + "file_type": "concept", "source_file": "doc2.md"}, + {"id": "shenzhen_international", "label": "shenzhen-international", + "file_type": "concept", "source_file": "doc3.md"}, + ] + survivors = set() + for perm in itertools.permutations(base): + out, _ = deduplicate_entities([dict(n) for n in perm], [], communities={}) + assert len(out) == 1 + survivors.add(out[0]["id"]) + assert survivors == {"shenzhen"}, f"non-deterministic survivor: {survivors}" + + +def test_crossfile_concept_merge_deterministic_across_hash_seeds(): + """#2182 determinism, #1753/#2074 precedent: the survivor must not depend on + PYTHONHASHSEED. pytest fixes the seed per process, so run out-of-process + with shuffled input.""" + import os + import subprocess + import sys + script = ( + "import random, sys\n" + "from graphify.dedup import deduplicate_entities\n" + "nodes = [\n" + " {'id': 'shenzhen', 'label': 'SHENZHEN INTERNATIONAL',\n" + " 'file_type': 'concept', 'source_file': 'doc1.md'},\n" + " {'id': 'shenzhen_intl', 'label': 'Shenzhen international',\n" + " 'file_type': 'concept', 'source_file': 'doc2.md'},\n" + " {'id': 'shenzhen_international', 'label': 'shenzhen-international',\n" + " 'file_type': 'concept', 'source_file': 'doc3.md'},\n" + "]\n" + "random.Random(int(sys.argv[1])).shuffle(nodes)\n" + "out, _ = deduplicate_entities(nodes, [], communities={})\n" + "print(len(out), sorted(n['id'] for n in out)[0])\n" + ) + results = set() + for seed in ("0", "1", "2", "3"): + env = {**os.environ, "PYTHONHASHSEED": seed} + r = subprocess.run( + [sys.executable, "-c", script, seed], + capture_output=True, text=True, env=env, + ) + assert r.returncode == 0, r.stderr + results.add(r.stdout.strip().splitlines()[-1]) + assert results == {"1 shenzhen"}, ( + f"non-deterministic dedup across hash seeds: {results}" + ) + + +def test_crossfile_concept_merge_is_transitive(): + """Exact cross-file matches and a punctuation variant collapse to one + survivor: {'Acme Corp' doc1, 'Acme Corp' doc2, 'Acme Corp.' doc3} all + normalize to 'acme corp' and must transitively union (#2182).""" + nodes = [ + {"id": "acme_corp_one", "label": "Acme Corp", + "file_type": "concept", "source_file": "doc1.md"}, + {"id": "acme_corp_two", "label": "Acme Corp", + "file_type": "concept", "source_file": "doc2.md"}, + {"id": "acme_corp_three", "label": "Acme Corp.", + "file_type": "concept", "source_file": "doc3.md"}, + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + assert len(result_nodes) == 1