Skip to content

Commit 533909c

Browse files
HumanBean17claude
andauthored
fix(cli): erase clears all builder-owned files (#349, #350) (#364)
* fix(cli): erase clears all builder-owned files (crash marker + hash tmp) (#349, #350) erase left .graph_increment_in_progress and .graph_hashes.json.tmp on disk because _cmd_erase hardcoded only .graph_hashes.json. The surviving crash marker then forced the next increment into a silent full rebuild (explained only under --verbose); the .tmp was orphan cruft. Export the builder-owned filenames from build_ast_graph.BUILDER_OWNED_INDEX_FILES and have erase clear all of them from one list, so erase and the builder cannot drift. Co-Authored-By: Claude <noreply@anthropic.com> * perf(cli): lazy-import build_ast_graph in _cmd_erase The top-level `from build_ast_graph import BUILDER_OWNED_INDEX_FILES` pulled numpy/ladybug/pyarrow/tree_sitter on every CLI invocation (~54ms; measured via -X importtime: cli cumulative import went 44ms -> 98ms), including `--help` -- violating this file's own lazy-import invariant. The three filenames are only needed on the erase path, so move the import into _cmd_erase. After the move, build_ast_graph is no longer imported at cli import time and cumulative import is back to ~39ms. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9ac7da1 commit 533909c

3 files changed

Lines changed: 74 additions & 14 deletions

File tree

build_ast_graph.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -472,10 +472,26 @@ class IncrementalResult:
472472
elapsed_sec: float
473473

474474

475+
# --- Builder-owned files in the index dir (single source of truth) ---------------
476+
# Every artifact the graph builder writes next to code_graph.lbug. The lifecycle
477+
# CLI's `erase` clears all of these from one list so the builder and erase cannot
478+
# drift (issues #349 / #350): previously erase hardcoded ".graph_hashes.json" only
479+
# and left the crash marker (.graph_increment_in_progress) and the atomic-write
480+
# temp (.graph_hashes.json.tmp) behind on disk.
481+
GRAPH_HASHES_FILENAME = ".graph_hashes.json"
482+
GRAPH_HASHES_TMP_FILENAME = ".graph_hashes.json.tmp"
483+
GRAPH_INCREMENT_MARKER_FILENAME = ".graph_increment_in_progress"
484+
BUILDER_OWNED_INDEX_FILES: tuple[str, ...] = (
485+
GRAPH_HASHES_FILENAME,
486+
GRAPH_HASHES_TMP_FILENAME,
487+
GRAPH_INCREMENT_MARKER_FILENAME,
488+
)
489+
490+
475491
class FileHashTracker:
476492
"""Track content hashes for incremental graph rebuild."""
477493
def __init__(self, index_dir: Path):
478-
self._path = index_dir / ".graph_hashes.json"
494+
self._path = index_dir / GRAPH_HASHES_FILENAME
479495
self._hashes: dict[str, str] = {} # rel_path -> sha256_hex
480496

481497
def load(self) -> None:
@@ -491,7 +507,7 @@ def load(self) -> None:
491507

492508
def save(self) -> None:
493509
"""Persist hashes to disk atomically (write .tmp, rename)."""
494-
tmp_path = self._path.with_suffix(".json.tmp")
510+
tmp_path = self._path.parent / GRAPH_HASHES_TMP_FILENAME
495511
try:
496512
with open(tmp_path, "w", encoding="utf-8") as f:
497513
json.dump(self._hashes, f, sort_keys=True)
@@ -3811,7 +3827,7 @@ def incremental_rebuild(
38113827
_verbose_stderr_line(f"[increment] detected {len(added)} added, {len(changed)} changed, {len(removed)} removed files")
38123828

38133829
# Step 2: Crash marker check
3814-
crash_marker_path = index_dir / ".graph_increment_in_progress"
3830+
crash_marker_path = index_dir / GRAPH_INCREMENT_MARKER_FILENAME
38153831
if crash_marker_path.exists():
38163832
if verbose:
38173833
_verbose_stderr_line("[increment] crash marker exists; falling back to full rebuild")

java_codebase_rag/cli.py

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from __future__ import annotations
22

3-
# Heavy imports (`server`, `pr_analysis`, `path_filtering.LayeredIgnore`) stay lazy
4-
# inside handlers so `java-codebase-rag --help` stays fast.
3+
# Heavy imports (`server`, `pr_analysis`, `path_filtering.LayeredIgnore`,
4+
# `build_ast_graph`) stay lazy inside handlers so `java-codebase-rag --help` stays fast.
55

66
import argparse
77
import asyncio
@@ -605,8 +605,13 @@ def _cmd_erase(args: argparse.Namespace) -> int:
605605
cfg = _resolved_from_ns(args)
606606
_startup_hints(cfg)
607607
cfg.apply_to_os_environ()
608-
graph_hashes_path = cfg.ladybug_path.parent / ".graph_hashes.json"
609-
to_describe: list[Path] = [cfg.ladybug_path, cfg.cocoindex_db, graph_hashes_path]
608+
# Lazy import: build_ast_graph transitively pulls numpy/ladybug/pyarrow/
609+
# tree_sitter (~54ms), and these filenames are only needed on the erase path.
610+
# Keeping it out of the top-level import lets `java-codebase-rag --help` (and
611+
# every other command) stay fast -- see the lazy-import invariant atop this file.
612+
from build_ast_graph import BUILDER_OWNED_INDEX_FILES
613+
builder_paths = [cfg.ladybug_path.parent / name for name in BUILDER_OWNED_INDEX_FILES]
614+
to_describe: list[Path] = [cfg.ladybug_path, cfg.cocoindex_db, *builder_paths]
610615
if cfg.index_dir.is_dir():
611616
try:
612617
import lancedb
@@ -643,15 +648,18 @@ def work(progress: "PipelineProgress | None") -> int:
643648
)
644649
elif drop.returncode != 0:
645650
print(clip(drop.stderr, 4000), file=sys.stderr)
646-
# Remove the LadybugDB graph, the cocoindex state store, and the graph
647-
# builder's content-hash store. Each is removed by type (see _rm_any):
648-
# code_graph.lbug is a file here but may be a dir under kuzu, while
649-
# cocoindex.db is a directory — a type-blind delete silently no-oped on
650-
# one or the other, and .graph_hashes.json was never targeted at all
651-
# (issue #346).
651+
# Remove the LadybugDB graph, the cocoindex state store, and every
652+
# builder-owned bookkeeping file next to code_graph.lbug (the content-hash
653+
# store, its atomic-write temp, and the incremental crash marker). Each is
654+
# removed by type (see _rm_any): code_graph.lbug is a file here but may be
655+
# a dir under kuzu, while cocoindex.db is a directory — a type-blind delete
656+
# silently no-oped on one or the other, and the builder files were never
657+
# targeted at all (issues #346 / #349 / #350). The list comes from
658+
# build_ast_graph.BUILDER_OWNED_INDEX_FILES so erase and the builder cannot drift.
652659
_rm_any(cfg.ladybug_path)
653660
_rm_any(cfg.cocoindex_db)
654-
_rm_any(graph_hashes_path)
661+
for builder_path in builder_paths:
662+
_rm_any(builder_path)
655663
if cfg.index_dir.is_dir():
656664
try:
657665
import lancedb

tests/test_java_codebase_rag_cli.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,42 @@ def test_erase_removes_graph_file_cocoindex_dir_and_hash_store(tmp_path: Path) -
138138
assert not (idx / ".graph_hashes.json").exists(), "erase left .graph_hashes.json on disk"
139139

140140

141+
def test_erase_removes_increment_marker_and_hash_store_tmp(tmp_path: Path) -> None:
142+
"""erase must also clear the rest of the builder's bookkeeping files.
143+
144+
Regression for issues #349 / #350: erase removed code_graph.lbug,
145+
cocoindex.db, and .graph_hashes.json but left the incremental crash marker
146+
(``.graph_increment_in_progress``) and the atomic-write temp
147+
(``.graph_hashes.json.tmp``) on disk. The marker surviving erase -> init then
148+
forced the next ``increment`` into a silent full rebuild (explained only under
149+
``--verbose``); the ``.tmp`` was pure cruft that defeated erase's "clean slate".
150+
Both are builder-owned files (``build_ast_graph.BUILDER_OWNED_INDEX_FILES``),
151+
so erase clears them from the same source of truth instead of hardcoding names.
152+
"""
153+
idx = tmp_path / "erase_builder_state"
154+
idx.mkdir()
155+
(idx / "code_graph.lbug").write_bytes(b"fake-kuzu-db")
156+
(idx / ".graph_hashes.json").write_text("{}", encoding="utf-8")
157+
# Simulate a crashed increment (marker left behind) + a crashed hash-store
158+
# save (atomic-write temp orphaned before the os.replace).
159+
(idx / ".graph_increment_in_progress").write_text("", encoding="utf-8")
160+
(idx / ".graph_hashes.json.tmp").write_text("partial", encoding="utf-8")
161+
env = os.environ.copy()
162+
env["JAVA_CODEBASE_RAG_INDEX_DIR"] = str(idx)
163+
env["JAVA_CODEBASE_RAG_SOURCE_ROOT"] = str(tmp_path)
164+
proc = _run_cli(
165+
["erase", "--source-root", str(tmp_path), "--index-dir", str(idx), "--yes"],
166+
env=env,
167+
)
168+
assert proc.returncode == 0, proc.stderr + proc.stdout
169+
assert not (idx / ".graph_increment_in_progress").exists(), (
170+
"erase left .graph_increment_in_progress; next increment would silently full-rebuild (#349)"
171+
)
172+
assert not (idx / ".graph_hashes.json.tmp").exists(), (
173+
"erase left .graph_hashes.json.tmp orphan (#350)"
174+
)
175+
176+
141177
def test_embedding_model_precedence_cli_over_env_over_yaml_over_default(
142178
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
143179
) -> None:

0 commit comments

Comments
 (0)