fix(detect): preserve Git-tracked files matched by a .gitignore pattern (#2759)
A file that is git-tracked but also matches a .gitignore pattern (a committed file later added to .gitignore, or a force-added one) was silently dropped from the corpus, even though git never un-tracks such a file — so the graph lost real, shipping source. detect now runs `git ls-files` once per scan, keeps a set of tracked-file identity keys plus their ancestor directories (so a git-only-ignored parent dir is not pruned before the walk reaches the tracked file below it), and exempts tracked paths from .gitignore. .graphifyignore and --exclude stay authoritative, and a non-git corpus is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
e6b0efb66b
commit
95ad0adcaa
+130
-5
@@ -6,6 +6,7 @@ import os
|
||||
import re
|
||||
import shlex
|
||||
import stat
|
||||
import subprocess
|
||||
import time
|
||||
import unicodedata
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
@@ -1020,6 +1021,65 @@ def _find_vcs_root(start: Path) -> Path | None:
|
||||
current = parent
|
||||
|
||||
|
||||
def _path_identity(path: Path) -> str:
|
||||
"""Portable comparison key for an existing filesystem path."""
|
||||
return _nfc(os.path.normcase(os.path.abspath(os.fspath(path))))
|
||||
|
||||
|
||||
def _git_tracked_path_keys(root: Path) -> tuple[set[str], set[str]]:
|
||||
"""Return tracked-file keys and their ancestor-directory keys under *root*.
|
||||
|
||||
Gitignore rules do not apply to paths already present in Git's index. Ask
|
||||
Git once per scan/predicate construction with NUL-delimited output so every
|
||||
valid filename is preserved. Missing Git, a non-Git VCS marker, command
|
||||
failure, and malformed output all fail closed to the historical ignore
|
||||
behavior rather than making discovery fail (#2759).
|
||||
"""
|
||||
root = root.resolve()
|
||||
vcs_root = _find_vcs_root(root)
|
||||
if vcs_root is None or not (vcs_root / ".git").exists():
|
||||
return set(), set()
|
||||
try:
|
||||
proc = subprocess.run(
|
||||
["git", "-C", str(vcs_root), "ls-files", "-z", "--cached"],
|
||||
stdin=subprocess.DEVNULL,
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.DEVNULL,
|
||||
check=False,
|
||||
timeout=30,
|
||||
env={**os.environ, "GIT_OPTIONAL_LOCKS": "0"},
|
||||
)
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
return set(), set()
|
||||
if proc.returncode != 0:
|
||||
return set(), set()
|
||||
|
||||
tracked_files: set[str] = set()
|
||||
tracked_dirs: set[str] = set()
|
||||
for raw in proc.stdout.split(b"\0"):
|
||||
if not raw:
|
||||
continue
|
||||
path = Path(os.path.abspath(vcs_root / os.fsdecode(raw)))
|
||||
try:
|
||||
path.relative_to(root)
|
||||
except ValueError:
|
||||
continue
|
||||
# Deleted index entries and submodule gitlinks are not discoverable
|
||||
# files. Symlinks to regular files remain eligible; the existing
|
||||
# in-root target guard still decides whether they may enter the corpus.
|
||||
if not _is_regular_file(path):
|
||||
continue
|
||||
tracked_files.add(_path_identity(path))
|
||||
parent = path.parent
|
||||
while parent != root:
|
||||
parent_key = _path_identity(parent)
|
||||
if parent_key in tracked_dirs:
|
||||
break # its ancestors were added with the first file below it
|
||||
tracked_dirs.add(parent_key)
|
||||
parent = parent.parent
|
||||
return tracked_files, tracked_dirs
|
||||
|
||||
|
||||
def _git_info_exclude(vcs_root: Path) -> Path | None:
|
||||
"""Resolve ``$GIT_DIR/info/exclude`` for the repo rooted at ``vcs_root``.
|
||||
|
||||
@@ -1259,6 +1319,32 @@ def _is_ignored(
|
||||
return _eval(path)
|
||||
|
||||
|
||||
def _is_scan_ignored(
|
||||
path: Path,
|
||||
root: Path,
|
||||
patterns: list[tuple[Path, str]],
|
||||
explicit_patterns: list[tuple[Path, str]],
|
||||
tracked_files: set[str],
|
||||
tracked_dirs: set[str],
|
||||
*,
|
||||
cache: dict[Path, bool],
|
||||
explicit_cache: dict[Path, bool],
|
||||
) -> bool:
|
||||
"""Apply ignore rules while preserving Git-tracked paths.
|
||||
|
||||
``patterns`` combines Git and graph-specific rules. ``explicit_patterns``
|
||||
contains only .graphifyignore/--exclude rules, which remain authoritative
|
||||
even for tracked files. A tracked file's ancestor directories are preserved
|
||||
from Git-only pruning so the walk can reach the file (#2759).
|
||||
"""
|
||||
if not _is_ignored(path, root, patterns, _cache=cache):
|
||||
return False
|
||||
if _is_ignored(path, root, explicit_patterns, _cache=explicit_cache):
|
||||
return True
|
||||
identity = _path_identity(path)
|
||||
return identity not in tracked_files and identity not in tracked_dirs
|
||||
|
||||
|
||||
def ignored_predicate(
|
||||
root: Path,
|
||||
*,
|
||||
@@ -1285,12 +1371,18 @@ def ignored_predicate(
|
||||
"""
|
||||
root = root.resolve()
|
||||
patterns = _load_graphifyignore(root, gitignore=gitignore)
|
||||
explicit_patterns = _load_graphifyignore(root, gitignore=False)
|
||||
tracked_files, tracked_dirs = (
|
||||
_git_tracked_path_keys(root) if gitignore else (set(), set())
|
||||
)
|
||||
if extra_excludes:
|
||||
for pat in extra_excludes:
|
||||
line = _parse_gitignore_line(pat)
|
||||
if line:
|
||||
patterns.append((root, line))
|
||||
explicit_patterns.append((root, line))
|
||||
cache: dict[Path, bool] = {}
|
||||
explicit_cache: dict[Path, bool] = {}
|
||||
# root's own ignore file is the last entry of _load_graphifyignore's chain.
|
||||
loaded_dirs: set[Path] = {root}
|
||||
|
||||
@@ -1317,7 +1409,19 @@ def ignored_predicate(
|
||||
if ancestor not in loaded_dirs:
|
||||
loaded_dirs.add(ancestor)
|
||||
patterns.extend(_load_dir_own_ignore(ancestor, gitignore=gitignore))
|
||||
return _is_ignored(path, root, patterns, _cache=cache)
|
||||
explicit_patterns.extend(
|
||||
_load_dir_own_ignore(ancestor, gitignore=False)
|
||||
)
|
||||
return _is_scan_ignored(
|
||||
path,
|
||||
root,
|
||||
patterns,
|
||||
explicit_patterns,
|
||||
tracked_files,
|
||||
tracked_dirs,
|
||||
cache=cache,
|
||||
explicit_cache=explicit_cache,
|
||||
)
|
||||
|
||||
return _ignored
|
||||
|
||||
@@ -1398,7 +1502,12 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
|
||||
ignored: list[str] = []
|
||||
pruned_noise: list[str] = []
|
||||
ignore_patterns = _load_graphifyignore(root, gitignore=gitignore)
|
||||
explicit_ignore_patterns = _load_graphifyignore(root, gitignore=False)
|
||||
tracked_files, tracked_dirs = (
|
||||
_git_tracked_path_keys(root) if gitignore else (set(), set())
|
||||
)
|
||||
ignore_cache: dict[Path, bool] = {} # shared across all _is_ignored calls in this scan
|
||||
explicit_ignore_cache: dict[Path, bool] = {}
|
||||
# CLI --exclude patterns are anchored at the scan root and appended last
|
||||
# so they win over any .graphifyignore/.gitignore rules (#947).
|
||||
if extra_excludes:
|
||||
@@ -1406,6 +1515,19 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
|
||||
line = _parse_gitignore_line(pat)
|
||||
if line:
|
||||
ignore_patterns.append((root, line))
|
||||
explicit_ignore_patterns.append((root, line))
|
||||
|
||||
def _ignored_for_scan(path: Path) -> bool:
|
||||
return _is_scan_ignored(
|
||||
path,
|
||||
root,
|
||||
ignore_patterns,
|
||||
explicit_ignore_patterns,
|
||||
tracked_files,
|
||||
tracked_dirs,
|
||||
cache=ignore_cache,
|
||||
explicit_cache=explicit_ignore_cache,
|
||||
)
|
||||
|
||||
# Always include graphify-out/memory/ - query results filed back into the graph
|
||||
memory_dir = root / GRAPHIFY_OUT / "memory"
|
||||
@@ -1455,6 +1577,9 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
|
||||
# file governs its own subtree the same way git honors it (#1206).
|
||||
if dp != root:
|
||||
ignore_patterns.extend(_load_dir_own_ignore(dp, gitignore=gitignore))
|
||||
explicit_ignore_patterns.extend(
|
||||
_load_dir_own_ignore(dp, gitignore=False)
|
||||
)
|
||||
# Prune noise dirs in-place so os.walk never descends into them.
|
||||
# Dot dirs are allowed — users often want .github/, .claude/, etc.
|
||||
# Framework caches (.next, .nuxt, …) are caught by _is_noise_dir.
|
||||
@@ -1485,7 +1610,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
|
||||
# than vanishing silently (#2058).
|
||||
pruned_noise.append(str(dp / d) + os.sep)
|
||||
continue
|
||||
if _is_ignored(dp / d, root, ignore_patterns, _cache=ignore_cache):
|
||||
if _ignored_for_scan(dp / d):
|
||||
ignored.append(str(dp / d) + os.sep)
|
||||
continue
|
||||
kept_dirs.append(d)
|
||||
@@ -1518,7 +1643,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, _cache=ignore_cache):
|
||||
if not in_memory and _ignored_for_scan(p):
|
||||
ignored.append(str(p))
|
||||
continue
|
||||
if not _resolves_under_root(p, root):
|
||||
@@ -1562,7 +1687,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, _cache=ignore_cache):
|
||||
if _ignored_for_scan(md_path):
|
||||
continue
|
||||
files[ftype].append(str(md_path))
|
||||
total_words += _wc(md_path)
|
||||
@@ -1573,7 +1698,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, root=root)
|
||||
if md_path:
|
||||
if _is_ignored(md_path, root, ignore_patterns, _cache=ignore_cache):
|
||||
if _ignored_for_scan(md_path):
|
||||
continue
|
||||
files[ftype].append(str(md_path))
|
||||
total_words += _wc(md_path)
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import os
|
||||
import subprocess
|
||||
import unicodedata
|
||||
import pytest
|
||||
from pathlib import Path
|
||||
@@ -8,6 +9,14 @@ from graphify import detect as detect_mod
|
||||
FIXTURES = Path(__file__).parent / "fixtures"
|
||||
|
||||
|
||||
def _git(repo: Path, *args: str) -> None:
|
||||
subprocess.run(
|
||||
["git", "-C", str(repo), *args],
|
||||
check=True,
|
||||
capture_output=True,
|
||||
)
|
||||
|
||||
|
||||
def as_posix_list(paths) -> list[str]:
|
||||
"""Normalize detect() output to forward slashes before matching on it.
|
||||
|
||||
@@ -428,6 +437,108 @@ def test_gitignore_nested_below_root_excludes_file(tmp_path):
|
||||
assert result["graphifyignore_patterns"] == 2
|
||||
|
||||
|
||||
def test_gitignore_keeps_tracked_file_but_drops_untracked_sibling(tmp_path):
|
||||
"""Gitignore rules do not apply to tracked files, matching Git itself (#2759)."""
|
||||
_git(tmp_path, "init", "-q")
|
||||
storage = tmp_path / "storage"
|
||||
storage.mkdir()
|
||||
tracked = storage / "fileWatcher.js"
|
||||
tracked.write_text("export function watch(){ return 1; }", encoding="utf-8")
|
||||
app = tmp_path / "app.js"
|
||||
app.write_text("export function ok(){ return 2; }", encoding="utf-8")
|
||||
_git(tmp_path, "add", "storage/fileWatcher.js", "app.js")
|
||||
|
||||
(tmp_path / ".gitignore").write_text("storage/\n", encoding="utf-8")
|
||||
untracked = storage / "scratch.js"
|
||||
untracked.write_text("export function scratch(){ return 3; }", encoding="utf-8")
|
||||
|
||||
result = detect(tmp_path)
|
||||
code = {Path(path).name for path in result["files"]["code"]}
|
||||
|
||||
assert code == {"app.js", "fileWatcher.js"}
|
||||
assert str(untracked) in result["ignored"]
|
||||
assert str(tracked) not in result["ignored"]
|
||||
|
||||
|
||||
def test_graphifyignore_still_excludes_git_tracked_file(tmp_path):
|
||||
"""A graph-specific exclusion remains authoritative for tracked paths."""
|
||||
_git(tmp_path, "init", "-q")
|
||||
storage = tmp_path / "storage"
|
||||
storage.mkdir()
|
||||
tracked = storage / "fileWatcher.js"
|
||||
tracked.write_text("export function watch(){ return 1; }", encoding="utf-8")
|
||||
_git(tmp_path, "add", "storage/fileWatcher.js")
|
||||
(tmp_path / ".graphifyignore").write_text("storage/\n", encoding="utf-8")
|
||||
|
||||
result = detect(tmp_path)
|
||||
|
||||
assert str(tracked) not in result["files"]["code"]
|
||||
assert any(entry.rstrip(os.sep) == str(storage) for entry in result["ignored"])
|
||||
|
||||
|
||||
def test_tracked_gitignore_exemption_works_for_subdirectory_scan(tmp_path):
|
||||
"""Tracked paths are repo-relative even when the requested scan root is nested."""
|
||||
_git(tmp_path, "init", "-q")
|
||||
project = tmp_path / "packages" / "app"
|
||||
storage = project / "storage"
|
||||
storage.mkdir(parents=True)
|
||||
tracked = storage / "fileWatcher.js"
|
||||
tracked.write_text("export function watch(){ return 1; }", encoding="utf-8")
|
||||
_git(tmp_path, "add", "packages/app/storage/fileWatcher.js")
|
||||
(tmp_path / ".gitignore").write_text("storage/\n", encoding="utf-8")
|
||||
|
||||
result = detect(project)
|
||||
|
||||
assert str(tracked) in result["files"]["code"]
|
||||
|
||||
|
||||
def test_ignored_predicate_keeps_git_tracked_ignored_file(tmp_path):
|
||||
"""Watch reconciliation must agree with detect() for tracked paths (#2759)."""
|
||||
_git(tmp_path, "init", "-q")
|
||||
tracked = tmp_path / "tracked.py"
|
||||
tracked.write_text("value = 1\n", encoding="utf-8")
|
||||
_git(tmp_path, "add", "tracked.py")
|
||||
(tmp_path / ".gitignore").write_text("tracked.py\n", encoding="utf-8")
|
||||
|
||||
ignored = detect_mod.ignored_predicate(tmp_path, gitignore=True)
|
||||
|
||||
assert ignored(tracked) is False
|
||||
|
||||
|
||||
def test_extra_exclude_still_excludes_git_tracked_file(tmp_path):
|
||||
"""CLI/persisted excludes are graph-level intent, not Git ignore rules."""
|
||||
_git(tmp_path, "init", "-q")
|
||||
tracked = tmp_path / "tracked.py"
|
||||
tracked.write_text("value = 1\n", encoding="utf-8")
|
||||
_git(tmp_path, "add", "tracked.py")
|
||||
|
||||
result = detect(tmp_path, extra_excludes=["tracked.py"])
|
||||
|
||||
assert str(tracked) not in result["files"]["code"]
|
||||
assert str(tracked) in result["ignored"]
|
||||
|
||||
|
||||
def test_git_tracking_probe_failure_preserves_ignore_behavior(
|
||||
tmp_path, monkeypatch
|
||||
):
|
||||
"""A missing/broken Git command must not fail open or abort discovery."""
|
||||
_git(tmp_path, "init", "-q")
|
||||
ignored_file = tmp_path / "ignored.py"
|
||||
ignored_file.write_text("value = 1\n", encoding="utf-8")
|
||||
_git(tmp_path, "add", "ignored.py")
|
||||
(tmp_path / ".gitignore").write_text("ignored.py\n", encoding="utf-8")
|
||||
|
||||
def _git_unavailable(*args, **kwargs):
|
||||
raise OSError("git unavailable")
|
||||
|
||||
monkeypatch.setattr(detect_mod.subprocess, "run", _git_unavailable)
|
||||
|
||||
result = detect(tmp_path)
|
||||
|
||||
assert str(ignored_file) not in result["files"]["code"]
|
||||
assert str(ignored_file) in result["ignored"]
|
||||
|
||||
|
||||
def test_gitignore_nested_below_root_prunes_whole_directory(tmp_path):
|
||||
"""A nested .gitignore excluding a directory prevents descending into it."""
|
||||
sub = tmp_path / "vendor" / "sub"
|
||||
|
||||
Reference in New Issue
Block a user