diff --git a/CHANGELOG.md b/CHANGELOG.md index 2259f70..1ec1cf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Full release notes with details on each version: [GitHub Releases](https://githu ## Unreleased +- Fix: fuzzy dedup no longer over-merges distinct nodes in three cases. (1) Numbered/versioned siblings whose embedded digit runs differ as zero-padding-insensitive multisets (`ADR 0011` vs `ADR 0013`, `3.1 Product Goals` vs `1.1 Product Goals`, `40%+ …` vs `<20% …`) never merge. (2) `rationale`/`document` nodes are file-anchored like code (#1205's reasoning): near-identical docstring/heading boilerplate in parallel files no longer collapses across files, while same-file duplicates still merge. (3) Cross-file labels that share a long prefix but diverge in a distinguishing token (`testing-library jest-native` vs `react-native`) are scored on plain Jaro instead of Jaro-Winkler, so the leading-prefix bonus can no longer fabricate a merge; genuine cross-file duplicates still clear the bar on Jaro alone, and same-file near-duplicates keep Jaro-Winkler. Guards are mirrored into the `--dedup-llm` ambiguous-pair collection. (#1284 thanks @van4oza, #1243) - Fix: every platform's query skill now ships **both** the vocab/IDF query-expansion step and the inline NetworkX fallback. Previously the two capabilities were split across `cli.md` / `cli-inline.md` so no platform got both — Claude had the superior expansion but no CLI-down fallback, while all other platforms had the fallback but the weaker raw-question matcher. The two fragments are merged into one unified `query` reference (and stub) shipped to all hosts; the `query_variant` enum and its coverage-audit exemption are removed (#1325; thanks @LeanderBlume). - Fix: cross-file Java `implements`/`inherits`/`imports` edges no longer orphan onto bare "shadow" nodes when two packages define a same-named type. The referencing file's `import` statement now disambiguates by exact package (FQN) and re-points the edge to the real definition, dropping the orphan stub. Previously `_rewire_unique_stub_nodes` could only repair the globally-unique case, so same-named interfaces (common in large Java codebases — `Handler`, `Service`, interface+impl pairs) left the real definition isolated in its own community (#1318). - Fix: Swift imports of the same module from multiple files now collapse to a single shared `type=module` node instead of N path-qualified duplicates. The import target is tagged `type=module` and exempted from id-disambiguation, so reverse traversal ("what imports CoreKit?") works; the `--no-cluster` writer also now dedupes nodes by id (and edges) to match the clustered `build_from_json` path. Builds on the v0.8.40 Swift-import fix (#1327, #1330; thanks @duncan-daydream). diff --git a/graphify/dedup.py b/graphify/dedup.py index 730c252..d02aedc 100644 --- a/graphify/dedup.py +++ b/graphify/dedup.py @@ -10,7 +10,7 @@ import unicodedata from collections import defaultdict from graphify._minhash import MinHash, MinHashLSH -from rapidfuzz.distance import JaroWinkler +from rapidfuzz.distance import Jaro, JaroWinkler # ── helpers ─────────────────────────────────────────────────────────────────── @@ -89,6 +89,52 @@ def _short_label_blocked(a: str, b: str, jw_score: float) -> bool: return True +_DIGIT_RUN = re.compile(r"\d+") + + +def _numeric_tokens_differ(a: str, b: str) -> bool: + """True when two labels carry different embedded numbers (#1284). + + Long labels that differ only in their digit runs ("ADR 0011 §D5" vs + "ADR 0013 D4", "3.1 Product Goals" vs "1.1 Product Goals", "block3" vs + "block13", "40%+ retention" vs "<20% retention") are numbered/versioned + siblings, not duplicates -- but the long shared boilerplate keeps + Jaro-Winkler above _MERGE_THRESHOLD, and _is_variant_pair only covers + short trailing suffixes. Digit runs are compared as multisets with + leading zeros stripped, so zero-padding ("09" vs "9") does not count as + a difference. (String comparison, not int(): a pathological label with a + >4300-digit run would crash int() on Python's conversion limit.) Labels + with identical numbers, or none at all, are unaffected. + """ + if a == b: + return False + return sorted(t.lstrip("0") or "0" for t in _DIGIT_RUN.findall(a)) != \ + sorted(t.lstrip("0") or "0" for t in _DIGIT_RUN.findall(b)) + + +# file_type values whose identity is anchored to their source location, not +# their label text. Like code (#1205), these must not be label-merged across +# files: rationale = module/class docstrings, document = headings/positional +# content. `concept` is intentionally excluded -- it is the type meant to unify +# across files (protected from over-merge by the numeric/Jaro guards instead). +_FILE_ANCHORED_NONCODE = frozenset({"rationale", "document"}) + + +def _crossfile_fileanchored_blocked(node: dict, neighbor: dict) -> bool: + """Block label-based merging of file-anchored non-code nodes across files (#1284). + + rationale/document nodes are docstring- and heading-derived and as + file-anchored as the code they describe (#1205's reasoning, one layer up): + parallel modules carry near-identical boilerplate ("Django app config for + apps.. No business logic here...") that differs by one word and sails + past the JW threshold. Same-file duplicates of these types may still merge. + """ + if (node.get("file_type") not in _FILE_ANCHORED_NONCODE + and neighbor.get("file_type") not in _FILE_ANCHORED_NONCODE): + return False + return (node.get("source_file") or "") != (neighbor.get("source_file") or "") + + # ── union-find ──────────────────────────────────────────────────────────────── class _UF: @@ -270,7 +316,20 @@ def deduplicate_entities( continue neighbor_norm = norm_cache.get(neighbor_id) or _norm(neighbor.get("label", neighbor.get("id", ""))) - score = JaroWinkler.normalized_similarity(norm_label, neighbor_norm) * 100 + # Cross-file long labels score on plain Jaro (no prefix bonus). + # Jaro-Winkler's leading-prefix bonus lifts pairs that share a + # prefix but diverge in a distinguishing token ("testing-library + # jest-native" vs "react-native") past threshold, fabricating + # destructive cross-file merges; on Jaro alone they fall short + # while true cross-file duplicates still clear it (#1243). Same-file + # near-duplicates keep Jaro-Winkler (low-risk, and a mid-string + # stopword insertion needs the prefix bonus to merge); short labels + # keep Jaro-Winkler too (gated by _short_label_blocked). + _xfile = (node.get("source_file") or "") != (neighbor.get("source_file") or "") + if _xfile and max(len(norm_label), len(neighbor_norm)) >= 12: + score = Jaro.normalized_similarity(norm_label, neighbor_norm) * 100 + else: + score = JaroWinkler.normalized_similarity(norm_label, neighbor_norm) * 100 if _is_variant_pair(norm_label, neighbor_norm): continue @@ -283,6 +342,13 @@ def deduplicate_entities( _lo, _hi = sorted((norm_label, neighbor_norm), key=len) if _hi.startswith(_lo) and _hi != _lo: continue + # Numbered/versioned siblings and cross-file file-anchored + # boilerplate (rationale/document) are decisively distinct + # regardless of score (#1284). + if _numeric_tokens_differ(norm_label, neighbor_norm): + continue + if _crossfile_fileanchored_blocked(node, neighbor): + continue c1 = communities.get(node_id) c2 = communities.get(neighbor_id) @@ -407,7 +473,12 @@ def _llm_tiebreak( if uf.find(node["id"]) == uf.find(neighbor["id"]): continue norm_j = _norm(neighbor.get("label", neighbor.get("id", ""))) - score = JaroWinkler.normalized_similarity(norm_i, norm_j) * 100 + # Mirror pass 2: plain Jaro for cross-file long labels (#1243). + _xfile = (node.get("source_file") or "") != (neighbor.get("source_file") or "") + if _xfile and max(len(norm_i), len(norm_j)) >= 12: + score = Jaro.normalized_similarity(norm_i, norm_j) * 100 + else: + score = JaroWinkler.normalized_similarity(norm_i, norm_j) * 100 if _is_variant_pair(norm_i, norm_j): continue if _short_label_blocked(norm_i, norm_j, score): @@ -415,6 +486,11 @@ def _llm_tiebreak( _lo, _hi = sorted((norm_i, norm_j), key=len) if _hi.startswith(_lo) and _hi != _lo: continue + # Mirror pass 2: decisively-distinct pairs never reach the LLM (#1284). + if _numeric_tokens_differ(norm_i, norm_j): + continue + if _crossfile_fileanchored_blocked(node, neighbor): + continue c1 = communities.get(node["id"]) c2 = communities.get(neighbor["id"]) if (c1 is not None and c2 is not None and c1 == c2 diff --git a/tests/test_dedup.py b/tests/test_dedup.py index 0575900..0f5d911 100644 --- a/tests/test_dedup.py +++ b/tests/test_dedup.py @@ -268,3 +268,99 @@ def test_prefix_guard_fires_for_extension_pairs(): assert hi.startswith(lo) and hi != lo, ( f"Prefix guard should fire for ({a!r}, {b!r}) but did not" ) + + +# ── #1284: numbered siblings + cross-file file-anchored boilerplate ────────── + +def test_numeric_tokens_differ_helper(): + """_numeric_tokens_differ compares digit runs as zero-padding-insensitive + multisets (#1284).""" + from graphify.dedup import _numeric_tokens_differ + assert _numeric_tokens_differ("adr 0011 d5 pipeline placement", "adr 0013 d4 pipeline placement") + assert _numeric_tokens_differ("3 1 product goals", "1 1 product goals") + assert _numeric_tokens_differ("code block3", "code block13") + assert not _numeric_tokens_differ("phase 09 overview", "phase 9 overview") # zero-padding + assert not _numeric_tokens_differ("module layout wave 3", "module layouts wave 3") + assert not _numeric_tokens_differ("graph extractor", "graph extractar") # digitless + + +def test_dedup_does_not_merge_numbered_siblings(): + """Long labels differing only in embedded numbers (ADR/section/issue ids) + must not merge — numbered siblings, not duplicates (#1284).""" + nodes = [ + {"id": "n1", "label": "Pipeline placement — 4 call sites (ADR 0013 D4)", + "file_type": "document", "source_file": "docs/index-activity.md"}, + {"id": "n2", "label": "Pipeline placement — 4 call sites (ADR 0011 §D5)", + "file_type": "document", "source_file": "docs/schema-matcher.md"}, + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + assert len(result_nodes) == 2 + + +def test_dedup_does_not_merge_crossfile_rationale_boilerplate(): + """Rationale nodes are file-anchored like code (#1205): parallel modules' + boilerplate docstrings differing by one word must not merge (#1284).""" + boiler = ("Django app config for {}. No business logic here. " + "Domain services live in services.py and adapters in providers.") + nodes = [ + {"id": "r1", "label": boiler.format("apps.platform.cards"), + "file_type": "rationale", "source_file": "apps/platform/cards/apps.py"}, + {"id": "r2", "label": boiler.format("apps.platform.cores"), + "file_type": "rationale", "source_file": "apps/platform/cores/apps.py"}, + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + assert len(result_nodes) == 2 + + +def test_dedup_does_not_merge_crossfile_document_headings(): + """Document nodes are file-anchored too: near-identical headings in different + files are distinct sections, not duplicates (#1284, extends the rationale guard).""" + nodes = [ + {"id": "d1", "label": "Getting Started Installation Guide", + "file_type": "document", "source_file": "docs/a.md"}, + {"id": "d2", "label": "Getting Started Installation Setup", + "file_type": "document", "source_file": "docs/b.md"}, + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + assert len(result_nodes) == 2 + + +def test_dedup_still_merges_samefile_rationale_duplicates(): + """The file-anchored guard only blocks cross-file pairs — near-identical + rationale duplicates within one file still merge (#1284 non-regression).""" + nodes = [ + {"id": "r1", "label": "Counts-only metrics export, a read-only aggregation service.", + "file_type": "rationale", "source_file": "apps/schemas/metrics.py"}, + {"id": "r2", "label": "Counts-only metrics export, the read-only aggregation service.", + "file_type": "rationale", "source_file": "apps/schemas/metrics.py"}, + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + assert len(result_nodes) == 1 + + +# ── #1243: JaroWinkler prefix-bonus over-merge (cross-file) ────────────────── + +def test_dedup_does_not_merge_crossfile_shared_prefix_divergence(): + """Cross-file labels sharing a long prefix but diverging in a distinguishing + token ("…jest native" vs "…react native") get JaroWinkler's prefix bonus past + threshold but are distinct entities; scoring them on plain Jaro blocks the + merge (#1243).""" + nodes = [ + {"id": "p1", "label": "testing library jest native", + "file_type": "concept", "source_file": "pkg-a/package.json"}, + {"id": "p2", "label": "testing library react native", + "file_type": "concept", "source_file": "pkg-b/package.json"}, + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + assert len(result_nodes) == 2 + + +def test_dedup_still_merges_crossfile_true_duplicates(): + """The #1243 guard only drops the prefix bonus — a genuine cross-file + duplicate (high similarity on Jaro alone) must still merge.""" + nodes = [ + {"id": "g1", "label": "GraphExtractor", "file_type": "concept", "source_file": "a.md"}, + {"id": "g2", "label": "Graph Extractor", "file_type": "concept", "source_file": "b.md"}, + ] + result_nodes, _ = deduplicate_entities(nodes, [], communities={}) + assert len(result_nodes) == 1