Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 73 additions & 10 deletions code_review_graph/incremental.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import re
import subprocess
import time
from pathlib import Path
from pathlib import Path, PurePosixPath
from typing import Optional

from .graph import GraphStore
Expand All @@ -22,24 +22,58 @@
logger = logging.getLogger(__name__)

# Default ignore patterns (in addition to .gitignore)
#
# Two pattern styles:
# 1. `**/name/**` — matches `name` as any path segment (safe-anywhere).
# Use only for directories that are NEVER legitimate source code names
# (node_modules, __pycache__, .venv, vendor, .gradle, .dart_tool, etc.).
# 2. `name/**` — matches only at repo root.
# Use for directories that MAY be valid source names in some projects
# (packages/, bin/, build/, dist/, storage/, obj/).
DEFAULT_IGNORE_PATTERNS = [
# Tool-owned (always safe anywhere)
".code-review-graph/**",
"node_modules/**",
".git/**",
"__pycache__/**",
"*.pyc",
".venv/**",
"venv/**",
# Dependency directories — never source code, safe to match anywhere
"**/node_modules/**",
"**/__pycache__/**",
"**/.venv/**",
"**/venv/**",
"**/vendor/**", # Composer (PHP), Go modules, Ruby/Rails
"**/.bundle/**", # Ruby Bundler
"**/.gradle/**", # Gradle cache
"**/.dart_tool/**", # Dart/Flutter
"**/.pub-cache/**", # Dart/Flutter
"**/.cache/**",
# Framework build/output dirs — at repo root only to avoid matching
# legit source dirs (e.g. src/build/, packages/, etc.)
".next/**",
".nuxt/**",
"dist/**",
"build/**",
".next/**",
"target/**",
"target/**", # Rust (also used by Maven/SBT)
"bin/**", # .NET (root only; src/bin/ not ignored)
"obj/**", # .NET
# NOTE: `packages/**` (.NET NuGet) is NOT included because it conflicts
# with npm/Lerna/Turborepo monorepos where packages/ holds source code.
# .NET users should add it via .code-review-graphignore.
"storage/**", # Laravel
"bootstrap/cache/**", # Laravel
"public/build/**", # Laravel Mix/Vite output
"coverage/**",
".tmp/**",
"tmp/**",
# Python cache file
"*.pyc",
# Minified / generated single files
"*.min.js",
"*.min.css",
"*.map",
"*.lock",
"package-lock.json",
"yarn.lock",
"*.jar", # Java compiled
# Database files
"*.db",
"*.sqlite",
"*.db-journal",
Expand Down Expand Up @@ -116,8 +150,37 @@ def _load_ignore_patterns(repo_root: Path) -> list[str]:


def _should_ignore(path: str, patterns: list[str]) -> bool:
"""Check if a path matches any ignore pattern."""
return any(fnmatch.fnmatch(path, p) for p in patterns)
"""Check if a path matches any ignore pattern.

Pattern semantics:
- ``**/name/**`` — matches ``name`` as any path segment (safe-anywhere).
Use for dirs that are never valid source code (node_modules, vendor).
- ``name/**`` — matches only at the repo root (first segment is ``name``).
Use for dirs that may be valid source names in some projects (packages,
bin, build).
- ``*.ext`` and other non-``**`` patterns fall back to ``fnmatch``.
"""
parts = PurePosixPath(path).parts
for p in patterns:
# Safe-anywhere: **/name/**
if p.startswith("**/") and p.endswith("/**"):
segment = p[3:-3]
if segment in parts:
return True
# Root-relative: name/** — matches only if first segment is `name`
elif p.endswith("/**"):
prefix = p[:-3]
# Support multi-segment prefixes like "bootstrap/cache"
prefix_parts = prefix.split("/")
if (
len(parts) >= len(prefix_parts)
and parts[: len(prefix_parts)] == tuple(prefix_parts)
):
return True
# Plain glob (e.g. *.pyc, *.min.js)
elif fnmatch.fnmatch(path, p):
return True
return False


def _is_binary(path: Path) -> bool:
Expand Down
63 changes: 60 additions & 3 deletions tests/test_incremental.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ def test_cleans_legacy_side_files(self, tmp_path):
class TestIgnorePatterns:
def test_default_patterns_loaded(self, tmp_path):
patterns = _load_ignore_patterns(tmp_path)
assert "node_modules/**" in patterns
assert "**/node_modules/**" in patterns
assert ".git/**" in patterns
assert "__pycache__/**" in patterns
assert "**/__pycache__/**" in patterns

def test_custom_ignore_file(self, tmp_path):
ignore = tmp_path / ".code-review-graphignore"
Expand All @@ -95,12 +95,69 @@ def test_custom_ignore_file(self, tmp_path):
assert "" not in patterns

def test_should_ignore_matches(self):
patterns = ["node_modules/**", "*.pyc", ".git/**"]
patterns = ["**/node_modules/**", "*.pyc", ".git/**"]
assert _should_ignore("node_modules/foo/bar.js", patterns)
assert _should_ignore("test.pyc", patterns)
assert _should_ignore(".git/HEAD", patterns)
assert not _should_ignore("src/main.py", patterns)

def test_safe_anywhere_matches_nested_paths(self):
"""**/name/** patterns match nested dependency dirs (monorepos)."""
patterns = ["**/node_modules/**", "**/vendor/**", "**/__pycache__/**"]
# Nested node_modules (npm workspaces, Lerna, Turborepo)
assert _should_ignore("packages/app/node_modules/react/index.js", patterns)
# Nested vendor (PHP monorepo)
assert _should_ignore("services/api/vendor/guzzlehttp/Client.php", patterns)
# Nested __pycache__
assert _should_ignore("src/utils/__pycache__/helpers.cpython-311.pyc", patterns)
# Actual source code must not be affected
assert not _should_ignore("packages/app/src/main.ts", patterns)
assert not _should_ignore("src/vendors/custom.php", patterns)

def test_root_relative_patterns_dont_match_nested(self):
"""name/** patterns should only match at root, not nested `name/` dirs."""
patterns = ["packages/**", "bin/**", "build/**"]
# Root-level matches
assert _should_ignore("packages/nuget-cache/lib.dll", patterns)
assert _should_ignore("bin/Debug/net8.0/app.dll", patterns)
assert _should_ignore("build/output.txt", patterns)
# Nested `packages/` in monorepo must NOT match
assert not _should_ignore("apps/web/packages/src/main.ts", patterns)
assert not _should_ignore("services/api/bin/helper.sh", patterns)
assert not _should_ignore("docs/build/page.md", patterns)

def test_multi_segment_root_prefix(self):
"""Multi-segment patterns like `bootstrap/cache/**` match only at root."""
patterns = ["bootstrap/cache/**"]
assert _should_ignore("bootstrap/cache/packages.php", patterns)
assert not _should_ignore("src/bootstrap/cache/file.php", patterns)

def test_should_ignore_framework_patterns(self):
"""Framework-specific dirs from DEFAULT_IGNORE_PATTERNS."""
from code_review_graph.incremental import DEFAULT_IGNORE_PATTERNS

patterns = DEFAULT_IGNORE_PATTERNS
# Safe-anywhere: dependency dirs never used as source
assert _should_ignore("vendor/laravel/framework/src/Collection.php", patterns)
assert _should_ignore("services/api/vendor/pkg/file.go", patterns) # nested
assert _should_ignore(".dart_tool/package_config.json", patterns)
assert _should_ignore(".gradle/caches/transforms-3/file.jar", patterns)
assert _should_ignore("node_modules/react/index.js", patterns)
assert _should_ignore("apps/api/node_modules/foo.js", patterns) # nested
# Root-only: Laravel framework dirs
assert _should_ignore("storage/logs/laravel.log", patterns)
assert _should_ignore("bootstrap/cache/packages.php", patterns)
assert _should_ignore("public/build/assets/app.js", patterns)
# Root-only: .NET
assert _should_ignore("bin/Debug/net8.0/app.dll", patterns)
assert _should_ignore("obj/Release/app.assets.cache", patterns)
# Monorepo-safe: `packages/` is NOT in defaults (npm/Lerna/Turborepo
# use it for source code; .NET users must add it to .code-review-graphignore)
assert not _should_ignore("packages/app/src/main.ts", patterns)
assert not _should_ignore("packages/Newtonsoft.Json.13.0.1/lib.dll", patterns)
assert not _should_ignore("apps/web/src/main.tsx", patterns)
assert not _should_ignore("app/Http/Controllers/UserController.php", patterns)


class TestIsBinary:
def test_text_file_is_not_binary(self, tmp_path):
Expand Down