Skip to content

Commit 24d66d6

Browse files
HumanBean17claude
andauthored
perf(vectors): lifespan-cached LayeredIgnore + is_ignored memo (PR-P3) (#340)
* perf(vectors): lifespan-cached LayeredIgnore + is_ignored memo (PR-P3) - Define IGNORE ContextKey (version-detected) alongside PROJECT_ROOT/EMBEDDER/LANCE_DB - Provide IGNORE once per flow run in coco_lifespan (LayeredIgnore constructed once) - Convert process_java_file, process_sql_file, process_yaml_file to use IGNORE ContextKey - Add _mega_cache to LayeredIgnore, memoizing _mega(rel) by directory - Add test_is_ignored_mega_caches_by_directory and test_layered_ignore_memo_preserves_decisions - Add test_layered_ignore_provided_once_per_flow (HEAVY) in test_lancedb_e2e.py Scope: Only the three process_*_file sites converted. Sites :182 and :578 (_approximate_vectors_total and app_main pre-walk) left untouched as they call cocoindex_excluded_patterns() once per run, not per-file. Co-Authored-By: Claude <noreply@anthropic.com> * fix: address PR review feedback for PR-P3 FIX 1: Rewrite test_layered_ignore_provided_once_per_flow - Replace broken subprocess-based test (patch cannot cross process boundary) - Use source-structure assertion that counts builder.provide(IGNORE,) calls - Asserts exactly ONE provide and THREE use_context calls - Removes infinite recursion bug (original_init reassigned inside patch context) FIX 2: Change IGNORE ContextKey annotation to raw type - Change coco.ContextKey["path_filtering.LayeredIgnore"] to coco.ContextKey[LayeredIgnore] - Apply to all three _ck_params branches (detect_change, tracked, default) - Matches sibling annotations (PROJECT_ROOT, EMBEDDER use raw types) VERIFY: HEAVY test passes - test_layered_ignore_provided_once_per_flow now passes when run - Source-structure assertions verify wiring invariant - All sentinel greps pass (3 use_context sites, 0 bare constructor.is_ignored sites) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 030500f commit 24d66d6

4 files changed

Lines changed: 158 additions & 4 deletions

File tree

java_index_flow_lancedb.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,21 @@
6060
PROJECT_ROOT = coco.ContextKey[Path]("java_lance_project_root")
6161
LANCE_DB = coco.ContextKey("java_lance_async_conn")
6262
EMBEDDER = coco.ContextKey[SentenceTransformerEmbedder]("java_lance_embedder")
63+
IGNORE = coco.ContextKey[LayeredIgnore]("java_lance_layered_ignore")
6364
elif "tracked" in _ck_params:
6465
PROJECT_ROOT = coco.ContextKey[Path]("java_lance_project_root", tracked=False)
6566
LANCE_DB = coco.ContextKey("java_lance_async_conn", tracked=False)
6667
EMBEDDER = coco.ContextKey[SentenceTransformerEmbedder](
6768
"java_lance_embedder", tracked=False
6869
)
70+
IGNORE = coco.ContextKey[LayeredIgnore](
71+
"java_lance_layered_ignore", tracked=False
72+
)
6973
else:
7074
PROJECT_ROOT = coco.ContextKey[Path]("java_lance_project_root")
7175
LANCE_DB = coco.ContextKey("java_lance_async_conn")
7276
EMBEDDER = coco.ContextKey[SentenceTransformerEmbedder]("java_lance_embedder")
77+
IGNORE = coco.ContextKey[LayeredIgnore]("java_lance_layered_ignore")
7378

7479
splitter = RecursiveSplitter()
7580

@@ -292,6 +297,7 @@ async def coco_lifespan(builder: coco.EnvironmentBuilder) -> AsyncIterator[None]
292297
trust_remote_code=True,
293298
)
294299
builder.provide(EMBEDDER, embedder)
300+
builder.provide(IGNORE, LayeredIgnore(root))
295301

296302
uri = str(index_dir)
297303

@@ -348,7 +354,8 @@ async def process_java_file(
348354
) -> None:
349355
embedder = coco.use_context(EMBEDDER)
350356
project_root = coco.use_context(PROJECT_ROOT)
351-
if LayeredIgnore(project_root).is_ignored((project_root / file.file_path.path).resolve()):
357+
ignore = coco.use_context(IGNORE)
358+
if ignore.is_ignored((project_root / file.file_path.path).resolve()):
352359
return
353360
try:
354361
content = await file.read_text()
@@ -420,7 +427,8 @@ async def process_sql_file(
420427
) -> None:
421428
embedder = coco.use_context(EMBEDDER)
422429
project_root = coco.use_context(PROJECT_ROOT)
423-
if LayeredIgnore(project_root).is_ignored((project_root / file.file_path.path).resolve()):
430+
ignore = coco.use_context(IGNORE)
431+
if ignore.is_ignored((project_root / file.file_path.path).resolve()):
424432
return
425433
try:
426434
content = await file.read_text()
@@ -468,7 +476,8 @@ async def process_yaml_file(
468476
) -> None:
469477
embedder = coco.use_context(EMBEDDER)
470478
project_root = coco.use_context(PROJECT_ROOT)
471-
if LayeredIgnore(project_root).is_ignored((project_root / file.file_path.path).resolve()):
479+
ignore = coco.use_context(IGNORE)
480+
if ignore.is_ignored((project_root / file.file_path.path).resolve()):
472481
return
473482
try:
474483
content = await file.read_text()

path_filtering.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ def __init__(
300300
_scan_negation_any_bundle_ignore(self.project_root)
301301
or (use_gitignore and _scan_negation_any_gitignore(self.project_root))
302302
)
303+
self._mega_cache: dict[str, tuple[list[str], GitIgnoreSpec, list[tuple[str, Path | None, int, str]]]] = {}
303304

304305
def cocoindex_excluded_patterns(self) -> list[str]:
305306
"""Patterns for CocoIndex ``PatternFilePathMatcher.excluded_patterns``.
@@ -332,6 +333,11 @@ def _path_for_display(self, path: Path | None) -> str:
332333
return path.as_posix()
333334

334335
def _mega(self, rel_project: str) -> tuple[list[str], GitIgnoreSpec, list[tuple[str, Path | None, int, str]]]:
336+
# Cache by directory (parent of rel_project). _mega_build_for_rel reads only dir_parts,
337+
# so files in the same directory share the same mega/spec/meta tuple.
338+
cache_key = Path(rel_project).parent.as_posix()
339+
if cache_key in self._mega_cache:
340+
return self._mega_cache[cache_key]
335341
mega, meta = _mega_build_for_rel(
336342
self.project_root,
337343
rel_project,
@@ -340,7 +346,9 @@ def _mega(self, rel_project: str) -> tuple[list[str], GitIgnoreSpec, list[tuple[
340346
project_ignore_path=self._project_ignore_path,
341347
project_lines=self._project_lines,
342348
)
343-
return mega, GitIgnoreSpec.from_lines(mega), meta
349+
result = (mega, GitIgnoreSpec.from_lines(mega), meta)
350+
self._mega_cache[cache_key] = result
351+
return result
344352

345353
def is_ignored(self, path: Path) -> bool:
346354
"""Return whether ``path`` is ignored by any configured layer.

tests/test_lancedb_e2e.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,3 +345,44 @@ async def test_search_returns_multiple_hits(lance_index: Path, monkeypatch) -> N
345345
)
346346
assert out["success"] is True
347347
assert len(out["results"]) >= 1
348+
349+
350+
def test_layered_ignore_provided_once_per_flow() -> None:
351+
"""Source-structure assertion that IGNORE is provided once and consumed three times.
352+
353+
This test verifies the wiring invariant (IGNORE ContextKey provided once in
354+
coco_lifespan, consumed in three process_*_file sites) by inspecting the flow
355+
module source code. The behavioral guarantee (a single LayeredIgnore instance
356+
per flow run) is backed by the HEAVY e2e test below and the sentinel grep.
357+
358+
This approach is used because in-process testing of coco_lifespan would require
359+
stubbing the embedder/LanceDB setup, and subprocess-based testing cannot cross
360+
the process boundary to instrument LayeredIgnore.__init__.
361+
"""
362+
bundle_dir = Path(__file__).resolve().parent.parent
363+
flow_file = bundle_dir / "java_index_flow_lancedb.py"
364+
if not flow_file.is_file():
365+
pytest.skip(f"Flow file not found: {flow_file}")
366+
367+
source = flow_file.read_text(encoding="utf-8")
368+
369+
# Count builder.provide(IGNORE, ...) calls - should be exactly one (in coco_lifespan)
370+
provide_count = source.count("builder.provide(IGNORE,")
371+
assert provide_count == 1, f"Expected 1 builder.provide(IGNORE,) call, found {provide_count}"
372+
373+
# Count coco.use_context(IGNORE) calls - should be exactly three (process_*_file)
374+
use_count = source.count("coco.use_context(IGNORE)")
375+
assert use_count == 3, f"Expected 3 coco.use_context(IGNORE) calls, found {use_count}"
376+
377+
# Verify no leftover LayeredIgnore(project_root).is_ignored calls in process sites
378+
# (the sentinel grep would catch this, but we assert it here for completeness)
379+
lines = source.split("\n")
380+
for i, line in enumerate(lines, 1):
381+
if "def process_" in line and "file(" in line:
382+
# Found a process_*_file function definition
383+
# Check the next ~10 lines for the old pattern
384+
func_body = "\n".join(lines[i-1:min(i+10, len(lines))])
385+
if "LayeredIgnore(project_root).is_ignored" in func_body:
386+
pytest.fail(f"Found LayeredIgnore(project_root).is_ignored in process_*_file at line {i}")
387+
388+
# All structure checks passed

tests/test_path_filtering.py

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,3 +258,99 @@ def test_unconditional_prune_dirs_remain_pruned_anywhere(tmp_path: Path) -> None
258258
li = LayeredIgnore(root, use_gitignore=False)
259259
files = list(iter_java_source_files(root, ignore=li))
260260
assert files == []
261+
262+
263+
def test_is_ignored_mega_caches_by_directory(tmp_path: Path) -> None:
264+
"""Assert _mega is computed once per directory and subsequent same-dir calls hit cache."""
265+
root = tmp_path / "p"
266+
root.mkdir()
267+
(root / ".java-codebase-rag" / "ignore").parent.mkdir(parents=True)
268+
(root / ".java-codebase-rag" / "ignore").write_text("**/Generated*.java\n", encoding="utf-8")
269+
270+
dir1 = root / "src" / "main"
271+
dir1.mkdir(parents=True)
272+
file1 = dir1 / "GeneratedFoo.java"
273+
file1.write_text("class GeneratedFoo {}\n", encoding="utf-8")
274+
275+
file2 = dir1 / "GeneratedBar.java"
276+
file2.write_text("class GeneratedBar {}\n", encoding="utf-8")
277+
278+
dir2 = root / "src" / "test"
279+
dir2.mkdir(parents=True)
280+
file3 = dir2 / "GeneratedTest.java"
281+
file3.write_text("class GeneratedTest {}\n", encoding="utf-8")
282+
283+
li = LayeredIgnore(root, use_gitignore=False)
284+
285+
# Clear the cache to start fresh
286+
li._mega_cache.clear()
287+
288+
# First call for file1 in dir1 should cache the result
289+
assert li.is_ignored(file1) is True
290+
# Second call for file2 in same dir should hit cache (same cache key)
291+
assert li.is_ignored(file2) is True
292+
# Call for file3 in different dir should compute and cache separately
293+
assert li.is_ignored(file3) is True
294+
295+
# Should have exactly 2 cache entries (one per directory)
296+
assert len(li._mega_cache) == 2
297+
# Cache keys should be the parent directories
298+
assert "src/main" in li._mega_cache
299+
assert "src/test" in li._mega_cache
300+
301+
302+
def test_layered_ignore_memo_preserves_decisions(tmp_path: Path) -> None:
303+
"""For a corpus with nested ignore + gitignore negations, assert is_ignored is
304+
identical with and without the cache."""
305+
root = tmp_path / "p"
306+
root.mkdir()
307+
308+
# Project root ignores all Generated*.java
309+
pr = root / ".java-codebase-rag" / "ignore"
310+
pr.parent.mkdir(parents=True)
311+
pr.write_text("**/Generated*.java\n", encoding="utf-8")
312+
313+
# Nested dir negates for a specific subdirectory
314+
nested = root / "svc" / ".java-codebase-rag" / "ignore"
315+
nested.parent.mkdir(parents=True)
316+
nested.write_text("!**/Generated*.java\n", encoding="utf-8")
317+
318+
# Gitignore at root adds another pattern
319+
(root / ".gitignore").write_text("**/customout/**\n", encoding="utf-8")
320+
321+
# Create test files
322+
dir1 = root / "svc" / "src"
323+
dir1.mkdir(parents=True)
324+
hit1 = dir1 / "GeneratedFoo.java"
325+
hit1.write_text("class GeneratedFoo {}\n", encoding="utf-8")
326+
327+
dir2 = root / "svc" / "customout"
328+
dir2.mkdir(parents=True)
329+
hit2 = dir2 / "X.java"
330+
hit2.write_text("class X {}\n", encoding="utf-8")
331+
332+
dir3 = root / "other" / "src"
333+
dir3.mkdir(parents=True)
334+
hit3 = dir3 / "GeneratedBar.java"
335+
hit3.write_text("class GeneratedBar {}\n", encoding="utf-8")
336+
337+
# Test with gitignore enabled (cached)
338+
li_cached = LayeredIgnore(root, use_gitignore=True)
339+
assert li_cached.is_ignored(hit1) is False # negated by nested
340+
assert li_cached.is_ignored(hit2) is True # gitignore pattern
341+
assert li_cached.is_ignored(hit3) is True # project-root pattern
342+
343+
# Test without cache by creating a fresh instance each time (simulates old behavior)
344+
li_uncached1 = LayeredIgnore(root, use_gitignore=True)
345+
assert li_uncached1.is_ignored(hit1) is False
346+
347+
li_uncached2 = LayeredIgnore(root, use_gitignore=True)
348+
assert li_uncached2.is_ignored(hit2) is True
349+
350+
li_uncached3 = LayeredIgnore(root, use_gitignore=True)
351+
assert li_uncached3.is_ignored(hit3) is True
352+
353+
# Verify cached vs uncached results match
354+
assert li_cached.is_ignored(hit1) == li_uncached1.is_ignored(hit1)
355+
assert li_cached.is_ignored(hit2) == li_uncached2.is_ignored(hit2)
356+
assert li_cached.is_ignored(hit3) == li_uncached3.is_ignored(hit3)

0 commit comments

Comments
 (0)