fix gitignore parent-exclusion rule (#882) and dedup false merges on short labels (#878)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

graphify/dedup.py

@@ -46,6 +46,45 @@ def _make_minhash(text: str, num_perm: int = 128) -> MinHash:
     return m
 
 
+# Matches labels whose trailing token is a version/variant suffix:
+# digits optionally followed by letters (chip SKUs: ASR1603, M1, Cortex-A55)
+# or 2+ letters (codename revisions: cranelr vs cranel).
+# Requires the stem to end in a letter so plain words don't accidentally match.
+_VARIANT_SUFFIX = re.compile(r"^(.*[a-z])([0-9]+[a-z]*|[a-z]{2,})$")
+
+
+def _is_variant_pair(a: str, b: str) -> bool:
+    """True if a and b are sibling model/SKU variants (same stem, different suffix).
+
+    Only applied to short labels (< 12 chars); long labels go through JW normally.
+    """
+    if a == b:
+        return False
+    if max(len(a), len(b)) >= 12:
+        return False
+    ma, mb = _VARIANT_SUFFIX.match(a), _VARIANT_SUFFIX.match(b)
+    if not (ma and mb):
+        return False
+    return ma.group(1) == mb.group(1) and ma.group(2) != mb.group(2)
+
+
+def _short_label_blocked(a: str, b: str, jw_score: float) -> bool:
+    """Block fuzzy merge for short labels unless it's a same-length single-char substitution.
+
+    Insertions/deletions on short strings (cranel/cranelr, M1/M1 Pro) produce
+    high Jaro-Winkler scores due to the prefix bonus but are almost never true
+    duplicates — they're abbreviations or variants.
+    """
+    if max(len(a), len(b)) >= 12:
+        return False
+    from rapidfuzz.distance import DamerauLevenshtein
+    # Allow only same-length single-char substitutions (true typos like "Extractor"/"Extractar").
+    # Block length-differing pairs regardless of score.
+    if jw_score >= 97.0 and len(a) == len(b) and DamerauLevenshtein.distance(a, b) <= 1:
+        return False
+    return True
+
+
 # ── union-find ────────────────────────────────────────────────────────────────
 
 class _UF:
@@ -185,9 +224,15 @@ def deduplicate_entities(
                 neighbor_norm = _norm(neighbor.get("label", neighbor.get("id", "")))
                 score = JaroWinkler.normalized_similarity(norm_label, neighbor_norm) * 100
 
+                if _is_variant_pair(norm_label, neighbor_norm):
+                    continue
+                if _short_label_blocked(norm_label, neighbor_norm, score):
+                    continue
+
                 c1 = communities.get(node_id)
                 c2 = communities.get(neighbor_id)
-                if c1 is not None and c2 is not None and c1 == c2:
+                if (c1 is not None and c2 is not None and c1 == c2
+                        and min(len(norm_label), len(neighbor_norm)) >= 12):
                     score += _COMMUNITY_BOOST
 
                 if score >= _MERGE_THRESHOLD:
@@ -297,9 +342,14 @@ def _llm_tiebreak(
                 continue
             norm_j = _norm(neighbor.get("label", neighbor.get("id", "")))
             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):
+                continue
             c1 = communities.get(node["id"])
             c2 = communities.get(neighbor["id"])
-            if c1 is not None and c2 is not None and c1 == c2:
+            if (c1 is not None and c2 is not None and c1 == c2
+                    and min(len(norm_i), len(norm_j)) >= 12):
                 score += _COMMUNITY_BOOST
             if low <= score < high:
                 ambiguous.append((node, neighbor, score))

graphify/detect.py

@@ -481,55 +481,76 @@ def _is_ignored(path: Path, root: Path, patterns: list[tuple[Path, str]]) -> boo
     Uses gitignore last-match-wins semantics: all patterns are evaluated in
     order; the final matching pattern determines the result. Negation patterns
     (starting with !) un-ignore a previously ignored path.
+
+    Enforces gitignore's parent-exclusion rule: a ! pattern cannot re-include
+    a file whose ancestor directory is already excluded.
     """
     if not patterns:
         return False
 
-    def _matches(rel: str, p: str) -> bool:
-        parts = rel.split("/")
-        if fnmatch.fnmatch(rel, p):
-            return True
-        if fnmatch.fnmatch(path.name, p):
-            return True
-        for i, part in enumerate(parts):
-            if fnmatch.fnmatch(part, p):
+    def _eval(target: Path) -> bool:
+        """Apply last-match-wins to a single target path."""
+        def _matches(rel: str, p: str) -> bool:
+            parts = rel.split("/")
+            if fnmatch.fnmatch(rel, p):
                 return True
-            if fnmatch.fnmatch("/".join(parts[:i + 1]), p):
+            if fnmatch.fnmatch(target.name, p):
                 return True
-        return False
+            for i, part in enumerate(parts):
+                if fnmatch.fnmatch(part, p):
+                    return True
+                if fnmatch.fnmatch("/".join(parts[:i + 1]), p):
+                    return True
+            return False
 
-    result = False
-    for anchor, pattern in patterns:
-        negated = pattern.startswith("!")
-        raw = pattern[1:] if negated else pattern
-        anchored = raw.startswith("/")
-        p = raw.strip("/")
-        if not p:
-            continue
+        result = False
+        for anchor, pattern in patterns:
+            negated = pattern.startswith("!")
+            raw = pattern[1:] if negated else pattern
+            anchored = raw.startswith("/")
+            p = raw.strip("/")
+            if not p:
+                continue
 
-        matched = False
-        if anchored:
-            try:
-                rel_anchor = str(path.relative_to(anchor)).replace(os.sep, "/")
-                matched = _matches(rel_anchor, p)
-            except ValueError:
-                pass
-        else:
-            try:
-                rel = str(path.relative_to(root)).replace(os.sep, "/")
-                matched = _matches(rel, p)
-            except ValueError:
-                pass
-            if not matched and anchor != root:
+            matched = False
+            if anchored:
                 try:
-                    rel_anchor = str(path.relative_to(anchor)).replace(os.sep, "/")
+                    rel_anchor = str(target.relative_to(anchor)).replace(os.sep, "/")
                     matched = _matches(rel_anchor, p)
                 except ValueError:
                     pass
+            else:
+                try:
+                    rel = str(target.relative_to(root)).replace(os.sep, "/")
+                    matched = _matches(rel, p)
+                except ValueError:
+                    pass
+                if not matched and anchor != root:
+                    try:
+                        rel_anchor = str(target.relative_to(anchor)).replace(os.sep, "/")
+                        matched = _matches(rel_anchor, p)
+                    except ValueError:
+                        pass
 
-        if matched:
-            result = not negated  # last match wins; ! flips to un-ignore
-    return result
+            if matched:
+                result = not negated  # last match wins; ! flips to un-ignore
+        return result
+
+    # Gitignore parent-exclusion rule: a ! re-include cannot rescue a file
+    # whose ancestor directory is already excluded. Walk ancestors top-down;
+    # if any ancestor is excluded, the file is excluded regardless of later
+    # ! patterns targeting the file or a sub-path.
+    try:
+        rel_parts = path.relative_to(root).parts
+    except ValueError:
+        return _eval(path)
+
+    ancestor = root
+    for part in rel_parts[:-1]:
+        ancestor = ancestor / part
+        if _eval(ancestor):
+            return True
+    return _eval(path)
 
 
 def _load_graphifyinclude(root: Path) -> list[tuple[Path, str]]:

tests/test_dedup.py

@@ -135,3 +135,45 @@ def test_build_calls_dedup():
     }
     G = build([chunk1, chunk2])
     assert G.number_of_nodes() == 1
+
+
+# --- #878: fuzzy dedup false merges on short/variant labels ---
+
+def test_dedup_does_not_merge_numeric_variants(tmp_path):
+    """Chip SKU variants (ASR1603 vs ASR1605) must not be merged (#878)."""
+    nodes = _make_nodes("ASR1603", "ASR1605")
+    result_nodes, _ = deduplicate_entities(nodes, [], communities={})
+    assert len(result_nodes) == 2, "ASR1603 and ASR1605 are distinct chip models, not duplicates"
+
+
+def test_dedup_does_not_merge_short_insertion_variants(tmp_path):
+    """Short labels differing by an insertion (cranel vs cranelr) must not merge (#878)."""
+    nodes = _make_nodes("cranel", "cranelr")
+    result_nodes, _ = deduplicate_entities(nodes, [], communities={})
+    assert len(result_nodes) == 2, "cranel and cranelr are distinct, not a typo"
+
+
+def test_dedup_does_not_merge_model_with_suffix(tmp_path):
+    """M1 vs M1 Pro must not merge (#878)."""
+    nodes = _make_nodes("M1", "M1 Pro")
+    result_nodes, _ = deduplicate_entities(nodes, [], communities={})
+    assert len(result_nodes) == 2, "M1 and M1 Pro are distinct Apple chip variants"
+
+
+def test_dedup_still_merges_real_typos():
+    """Genuine same-length single-char typos should still merge (#878 non-regression)."""
+    from graphify.dedup import _is_variant_pair, _short_label_blocked
+    from rapidfuzz.distance import JaroWinkler
+    a, b = "graphextractor", "graphextractar"
+    score = JaroWinkler.normalized_similarity(a, b) * 100
+    assert not _is_variant_pair(a, b), "not a variant pair"
+    assert not _short_label_blocked(a, b, score), "long-enough label, should not be blocked"
+
+
+def test_variant_pair_helper():
+    """_is_variant_pair correctly identifies chip-model variant pairs (#878)."""
+    from graphify.dedup import _is_variant_pair
+    assert _is_variant_pair("asr1603", "asr1605")
+    assert _is_variant_pair("cortex a55", "cortex a55x")
+    assert not _is_variant_pair("graphextractor", "graphextracter")
+    assert not _is_variant_pair("foo", "foo")

tests/test_detect.py

@@ -416,3 +416,47 @@ def test_detect_skips_graphify_own_cache(tmp_path):
     all_files = [f for files in result["files"].values() for f in files]
     assert not any(".graphify" in f for f in all_files)
     assert any("app.py" in f for f in all_files)
+
+
+# --- #882: gitignore parent-exclusion rule for ! re-includes ---
+
+def test_negation_cannot_rescue_file_under_excluded_dir(tmp_path):
+    """A ! re-include cannot un-ignore a file whose parent dir is excluded (#882)."""
+    from graphify.detect import _is_ignored, _load_graphifyignore
+    android = tmp_path / "android" / "app" / "src"
+    android.mkdir(parents=True)
+    victim = android / "Main.kt"
+    victim.write_text("fun main() {}")
+    (tmp_path / ".graphifyignore").write_text("android/\n!src/\n")
+    patterns = _load_graphifyignore(tmp_path)
+    assert _is_ignored(victim, tmp_path, patterns), (
+        "android/app/src/Main.kt must remain ignored even with !src/ because "
+        "the parent android/ is excluded"
+    )
+
+
+def test_negation_works_when_no_ancestor_excluded(tmp_path):
+    """A ! re-include must still un-ignore a file when no ancestor is excluded (#882)."""
+    from graphify.detect import _is_ignored, _load_graphifyignore
+    src = tmp_path / "src"
+    src.mkdir()
+    keep = src / "keep.py"
+    keep.write_text("x = 1")
+    (tmp_path / ".graphifyignore").write_text("*.py\n!src/keep.py\n")
+    patterns = _load_graphifyignore(tmp_path)
+    assert not _is_ignored(keep, tmp_path, patterns), (
+        "src/keep.py should be un-ignored by !src/keep.py since src/ itself is not excluded"
+    )
+
+
+def test_negation_ancestor_itself_reincluded(tmp_path):
+    """If the ancestor dir itself is re-included, its children should not be blocked (#882)."""
+    from graphify.detect import _is_ignored, _load_graphifyignore
+    vendor = tmp_path / "vendor" / "lib"
+    vendor.mkdir(parents=True)
+    f = vendor / "utils.py"
+    f.write_text("x = 1")
+    (tmp_path / ".graphifyignore").write_text("vendor/\n!vendor/\n")
+    patterns = _load_graphifyignore(tmp_path)
+    # vendor/ is excluded then re-included; ancestor eval returns False so file is evaluated on its own
+    assert not _is_ignored(f, tmp_path, patterns)