diff --git a/graphify/dedup.py b/graphify/dedup.py index 9bdf2603..37a1351d 100644 --- a/graphify/dedup.py +++ b/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)) diff --git a/graphify/detect.py b/graphify/detect.py index 78174c27..5351a57f 100644 --- a/graphify/detect.py +++ b/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]]: diff --git a/tests/test_dedup.py b/tests/test_dedup.py index 18bed3b4..293d2a8f 100644 --- a/tests/test_dedup.py +++ b/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") diff --git a/tests/test_detect.py b/tests/test_detect.py index 96775cf2..40fa21a0 100644 --- a/tests/test_detect.py +++ b/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)