diff --git a/graphify/_minhash.py b/graphify/_minhash.py new file mode 100644 index 000000000..aabeb654f --- /dev/null +++ b/graphify/_minhash.py @@ -0,0 +1,107 @@ +"""MinHash + band-LSH — datasketch-compatible drop-in (no scipy). + +datasketch.lsh has `from scipy.integrate import quad` at module level. +scipy's array_api_compat layer then lazily loads numpy.testing, which calls +platform.machine() at import time to set test-skip decorator constants — and +that in turn spawns cmd.exe via subprocess, hanging for minutes under EDR +software in corporate Windows environments. + +Covers the exact MinHash/MinHashLSH API surface used by dedup.py. +Hash family (Mersenne-prime permutations) and LSH band structure are +equivalent to datasketch so dedup quality is unchanged. +""" +from __future__ import annotations +import hashlib +import struct + +import numpy as np + + +_MP = np.uint64((1 << 61) - 1) # Mersenne prime for the hash family +_MH = np.uint64(0xFFFF_FFFF) # mask to 32-bit values + +# One (a, b) coefficient array per num_perm, shared across all instances. +_MH_COEFFS: dict[int, tuple[np.ndarray, np.ndarray]] = {} + + +def _mh_coeffs(num_perm: int) -> tuple[np.ndarray, np.ndarray]: + if num_perm not in _MH_COEFFS: + rng = np.random.RandomState(1) + a = rng.randint(1, int(_MP), num_perm, dtype=np.uint64) + b = rng.randint(0, int(_MP), num_perm, dtype=np.uint64) + _MH_COEFFS[num_perm] = (a, b) + return _MH_COEFFS[num_perm] + + +class MinHash: + """MinHash sketch — same API as datasketch.MinHash for the subset used here.""" + + __slots__ = ("num_perm", "hashvalues", "_a", "_b") + + def __init__(self, num_perm: int = 128) -> None: + self.num_perm = num_perm + self.hashvalues = np.full(num_perm, int(_MH), dtype=np.uint64) + self._a, self._b = _mh_coeffs(num_perm) + + def update(self, v: bytes) -> None: + hv = np.uint64(struct.unpack(" float: + """Numerical integration — replaces scipy.integrate.quad for LSH param search.""" + h = (hi - lo) / n + return h * sum(f(lo + i * h) for i in range(n)) + + +_LSH_PARAMS_CACHE: dict[tuple[float, int], tuple[int, int]] = {} + + +def _optimal_lsh_params(threshold: float, num_perm: int) -> tuple[int, int]: + """Find (bands, rows) that minimise weighted FP+FN error, without scipy.""" + key = (threshold, num_perm) + if key in _LSH_PARAMS_CACHE: + return _LSH_PARAMS_CACHE[key] + best_err, best = float("inf"), (1, 1) + for b in range(1, num_perm + 1): + for r in range(1, num_perm // b + 1): + fp = _lsh_integrate( + lambda s, _b=float(b), _r=float(r): 1 - (1 - s ** _r) ** _b, + 0.0, threshold, + ) + fn = _lsh_integrate( + lambda s, _b=float(b), _r=float(r): 1 - (1 - (1 - s ** _r) ** _b), + threshold, 1.0, + ) + err = 0.5 * fp + 0.5 * fn + if err < best_err: + best_err, best = err, (b, r) + _LSH_PARAMS_CACHE[key] = best + return best + + +class MinHashLSH: + """Band-hashing LSH — same API as datasketch.MinHashLSH for the subset used here.""" + + def __init__(self, threshold: float = 0.5, num_perm: int = 128) -> None: + self.b, self.r = _optimal_lsh_params(threshold, num_perm) + self._tables: list[dict[bytes, list[str]]] = [{} for _ in range(self.b)] + self._keys: set[str] = set() + + def insert(self, key: str, minhash: MinHash) -> None: + if key in self._keys: + raise ValueError(f"Key {key!r} already exists in MinHashLSH") + self._keys.add(key) + hv = minhash.hashvalues + for i, table in enumerate(self._tables): + band = hv[i * self.r : (i + 1) * self.r].tobytes() + table.setdefault(band, []).append(key) + + def query(self, minhash: MinHash) -> list[str]: + hv = minhash.hashvalues + candidates: set[str] = set() + for i, table in enumerate(self._tables): + band = hv[i * self.r : (i + 1) * self.r].tobytes() + candidates.update(table.get(band, [])) + return list(candidates) diff --git a/graphify/dedup.py b/graphify/dedup.py index 320d2a67e..5b45bc970 100644 --- a/graphify/dedup.py +++ b/graphify/dedup.py @@ -9,7 +9,7 @@ import re import unicodedata from collections import defaultdict -from datasketch import MinHash, MinHashLSH +from graphify._minhash import MinHash, MinHashLSH from rapidfuzz.distance import JaroWinkler diff --git a/graphify/detect.py b/graphify/detect.py index afb8d6d76..411376475 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -757,7 +757,13 @@ def _load_graphifyignore(root: Path) -> list[tuple[Path, str]]: return patterns -def _is_ignored(path: Path, root: Path, patterns: list[tuple[Path, str]]) -> bool: +def _is_ignored( + path: Path, + root: Path, + patterns: list[tuple[Path, str]], + *, + _cache: dict[Path, bool] | None = None, +) -> bool: """Return True if the path should be ignored per .graphifyignore patterns. Uses gitignore last-match-wins semantics: all patterns are evaluated in @@ -766,12 +772,18 @@ def _is_ignored(path: Path, root: Path, patterns: list[tuple[Path, str]]) -> boo Enforces gitignore's parent-exclusion rule: a ! pattern cannot re-include a file whose ancestor directory is already excluded. + + _cache: optional dict shared across calls within the same scan. Ancestor + directory results are memoised so files under the same subtree don't + re-evaluate the same patterns repeatedly. """ if not patterns: return False def _eval(target: Path) -> bool: """Apply last-match-wins to a single target path.""" + if _cache is not None and target in _cache: + return _cache[target] def _matches(rel: str, p: str, anchored: bool) -> bool: if anchored: return fnmatch.fnmatch(rel, p) @@ -818,6 +830,8 @@ def _is_ignored(path: Path, root: Path, patterns: list[tuple[Path, str]]) -> boo if matched: result = not negated # last match wins; ! flips to un-ignore + if _cache is not None: + _cache[target] = result return result # Gitignore parent-exclusion rule: a ! re-include cannot rescue a file @@ -983,6 +997,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: skipped_sensitive: list[str] = [] ignore_patterns = _load_graphifyignore(root) + ignore_cache: dict[Path, bool] = {} # shared across all _is_ignored calls in this scan # CLI --exclude patterns are anchored at the scan root and appended last # so they win over any .graphifyignore/.gitignore rules (#947). if extra_excludes: @@ -1021,7 +1036,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: dirnames[:] = [ d for d in dirnames if not _is_noise_dir(d, dp) - and (has_negation or not _is_ignored(dp / d, root, ignore_patterns)) + and (has_negation or not _is_ignored(dp / d, root, ignore_patterns, _cache=ignore_cache)) ] for fname in filenames: if fname in _SKIP_FILES: @@ -1042,7 +1057,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: # Skip files inside our own converted/ dir (avoid re-processing sidecars) if str(p).startswith(str(converted_dir)): continue - if not in_memory and _is_ignored(p, root, ignore_patterns): + if not in_memory and _is_ignored(p, root, ignore_patterns, _cache=ignore_cache): continue if _is_sensitive(p): skipped_sensitive.append(str(p)) @@ -1063,7 +1078,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: skipped_sensitive.append(str(p) + f" [Google Workspace export failed: {exc}]") continue if md_path: - if _is_ignored(md_path, root, ignore_patterns): + if _is_ignored(md_path, root, ignore_patterns, _cache=ignore_cache): continue files[ftype].append(str(md_path)) total_words += count_words(md_path) @@ -1074,7 +1089,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: if p.suffix.lower() in OFFICE_EXTENSIONS: md_path = convert_office_file(p, converted_dir) if md_path: - if _is_ignored(md_path, root, ignore_patterns): + if _is_ignored(md_path, root, ignore_patterns, _cache=ignore_cache): continue files[ftype].append(str(md_path)) total_words += count_words(md_path) diff --git a/pyproject.toml b/pyproject.toml index 3653bae52..efbadd7d1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,7 @@ keywords = ["claude", "claude-code", "codex", "opencode", "kilo", "cursor", "gem requires-python = ">=3.10" dependencies = [ "networkx>=3.4", - "datasketch>=1.6", + "numpy>=1.21", "rapidfuzz>=3.0", "tree-sitter>=0.23.0", "tree-sitter-python", diff --git a/tests/test_detect.py b/tests/test_detect.py index 00efc8fd2..20785181f 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -621,6 +621,101 @@ def test_anchored_multi_segment_pattern(tmp_path): ) +# Tests for #1235 - memoise _is_ignored/_eval results via a per-detect() cache + +def test_is_ignored_cache_matches_uncached_results(tmp_path): + """A shared _cache must not change _is_ignored results, including negation. + + Builds a tree with a normal ignore pattern and a negation pattern, then + asserts that evaluating every path with a cache yields identical results + to evaluating without one (#1235). + """ + from graphify.detect import _is_ignored, _load_graphifyignore + + # Normal pattern: ignore everything under build/. + # Negation pattern: re-include logs/keep.log even though *.log is ignored. + (tmp_path / "build" / "sub").mkdir(parents=True) + (tmp_path / "logs").mkdir() + (tmp_path / "src").mkdir() + paths = [ + tmp_path / "build", + tmp_path / "build" / "out.o", + tmp_path / "build" / "sub", + tmp_path / "build" / "sub" / "deep.o", + tmp_path / "logs", + tmp_path / "logs" / "drop.log", + tmp_path / "logs" / "keep.log", + tmp_path / "src" / "main.py", + ] + for p in paths: + if p.suffix: + p.write_text("x") + (tmp_path / ".graphifyignore").write_text( + "build/\n*.log\n!logs/keep.log\n" + ) + patterns = _load_graphifyignore(tmp_path) + + cache: dict = {} + for p in paths: + uncached = _is_ignored(p, tmp_path, patterns) + cached = _is_ignored(p, tmp_path, patterns, _cache=cache) + assert cached == uncached, ( + f"cached result for {p} ({cached}) differs from uncached ({uncached})" + ) + + # Sanity: the negation actually fired so the test exercises a non-trivial case. + assert not _is_ignored(tmp_path / "logs" / "keep.log", tmp_path, patterns) + assert _is_ignored(tmp_path / "logs" / "drop.log", tmp_path, patterns) + + +def test_is_ignored_cache_evaluates_each_dir_once(): + """Siblings under the same subtree must share the cached parent result (#1235). + + Counts how many times each unique target path is evaluated through the + cache: every directory (ancestor) should be evaluated exactly once across + a multi-file subtree rather than once per descendant file. + """ + from graphify.detect import _is_ignored + + root = Path("/repo") + patterns = [(root, "*.tmp")] # non-empty so _eval runs + + # A subtree where many files share the same ancestor directories. + files = [ + root / "a" / "b" / "f1.py", + root / "a" / "b" / "f2.py", + root / "a" / "b" / "f3.py", + root / "a" / "c" / "f4.py", + root / "a" / "c" / "f5.py", + ] + + eval_counts: dict[Path, int] = {} + + # A dict subclass records every cache write. Since _eval writes to the + # cache exactly once per computed target (and reads short-circuit before + # any write), one write == one evaluation of that path. + class CountingCache(dict): + def __setitem__(self, key, value): + eval_counts[key] = eval_counts.get(key, 0) + 1 + super().__setitem__(key, value) + + cache = CountingCache() + for f in files: + _is_ignored(f, root, patterns, _cache=cache) + + # Each unique path (files + ancestor dirs) must be computed exactly once. + for target, count in eval_counts.items(): + assert count == 1, f"{target} evaluated {count} times, expected 1 (cache miss)" + + # Shared ancestors must be present and counted only once each. + assert eval_counts[root / "a"] == 1 + assert eval_counts[root / "a" / "b"] == 1 + assert eval_counts[root / "a" / "c"] == 1 + # All five distinct files are computed once each. + for f in files: + assert eval_counts[f] == 1 + + # Regression tests for #920 - sensitive pattern misses underscore-prefixed names def test_sensitive_flags_api_token_txt(): assert _is_sensitive(Path("api_token.txt")) diff --git a/tests/test_minhash.py b/tests/test_minhash.py new file mode 100644 index 000000000..4c1f6cd5d --- /dev/null +++ b/tests/test_minhash.py @@ -0,0 +1,101 @@ +"""Tests for graphify/_minhash.py — MinHash sketch and band-LSH.""" +from __future__ import annotations +import numpy as np +import pytest +from graphify._minhash import MinHash, MinHashLSH, _optimal_lsh_params + + +def _minhash_for(text: str, num_perm: int = 128) -> MinHash: + m = MinHash(num_perm=num_perm) + for i in range(0, len(text) - 2): + m.update(text[i:i + 3].encode()) + return m + + +# ── MinHash ─────────────────────────────────────────────────────────────────── + +def test_identical_texts_produce_identical_hashvalues(): + a = _minhash_for("graphextractor") + b = _minhash_for("graphextractor") + assert np.array_equal(a.hashvalues, b.hashvalues) + + +def test_similar_texts_share_most_hashvalues(): + a = _minhash_for("authentication manager") + b = _minhash_for("authentication managers") + overlap = np.sum(a.hashvalues == b.hashvalues) / len(a.hashvalues) + assert overlap > 0.5 + + +def test_unrelated_texts_share_few_hashvalues(): + a = _minhash_for("authentication manager") + b = _minhash_for("file system watcher") + overlap = np.sum(a.hashvalues == b.hashvalues) / len(a.hashvalues) + assert overlap < 0.3 + + +def test_update_mutates_hashvalues(): + m = MinHash(num_perm=64) + before = m.hashvalues.copy() + m.update(b"hello") + assert not np.array_equal(m.hashvalues, before) + + +# ── MinHashLSH ──────────────────────────────────────────────────────────────── + +def test_near_duplicates_are_candidates(): + lsh = MinHashLSH(threshold=0.5, num_perm=128) + a = _minhash_for("authentication manager") + b = _minhash_for("authentication managers") + lsh.insert("a", a) + lsh.insert("b", b) + assert "b" in lsh.query(a) + + +def test_unrelated_strings_not_candidates(): + lsh = MinHashLSH(threshold=0.5, num_perm=128) + a = _minhash_for("authentication manager") + b = _minhash_for("file system watcher") + lsh.insert("a", a) + lsh.insert("b", b) + assert "b" not in lsh.query(a) + + +def test_query_always_returns_self(): + lsh = MinHashLSH(threshold=0.5, num_perm=128) + m = _minhash_for("graphextractor") + lsh.insert("x", m) + assert "x" in lsh.query(m) + + +def test_duplicate_insert_raises(): + lsh = MinHashLSH(threshold=0.5, num_perm=128) + m = _minhash_for("foo") + lsh.insert("key", m) + with pytest.raises(ValueError, match="already exists"): + lsh.insert("key", m) + + +# ── _optimal_lsh_params ─────────────────────────────────────────────────────── + +def test_optimal_params_within_budget(): + b, r = _optimal_lsh_params(0.5, 128) + assert b >= 1 and r >= 1 + assert b * r <= 128 + + +def test_optimal_params_cached(): + result1 = _optimal_lsh_params(0.7, 128) + result2 = _optimal_lsh_params(0.7, 128) + assert result1 is result2 + + +# ── EDR regression: scipy / numpy.testing must not be imported ────────────────── + +def test_dedup_import_does_not_pull_scipy_or_numpy_testing(): + import sys + for mod in ("scipy", "numpy.testing"): + sys.modules.pop(mod, None) + import graphify.dedup # noqa: F401 + assert "scipy" not in sys.modules + assert "numpy.testing" not in sys.modules