From 068d09297acf2d4d799cf086370471a33e04aa33 Mon Sep 17 00:00:00 2001 From: HerenderKumar Date: Tue, 14 Jul 2026 11:28:54 +0530 Subject: [PATCH] fix(llm): unwrap FileSlice via unit_path in the checkpoint allowlist (#1870) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _checkpoint_chunk built allowed_source_files with getattr(item, "rel", None) or item — but FileSlice carries its file in .path, not .rel, so every sliced document leaked the FileSlice object itself into the list. save_semantic_cache then raised on Path(FileSlice), the best-effort guard swallowed it, and every chunk containing a slice silently skipped its incremental checkpoint — a resumed run re-billed those chunks instead of reusing the cache. Build the allowlist with unit_path, which exists for exactly this unwrap, and pin the contract with a test that drives extract_corpus_parallel over a corpus with a sliced document. --- graphify/llm.py | 6 +++--- tests/test_chunking.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/graphify/llm.py b/graphify/llm.py index 23b30204f..9c303a1b9 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -1918,9 +1918,9 @@ def _checkpoint_chunk(result: dict, chunk: "list[Path | FileSlice]") -> None: # (#1757). The model can attribute a node's source_file to another # corpus file; without this bound, that stray node would clobber the # other file's complete cache entry (or, with merge_existing, pollute - # it). A FileSlice reports its file via `.rel`; a bare Path is the - # relative source_file itself. - allowed = [getattr(item, "rel", None) or item for item in chunk] + # it). unit_path unwraps a FileSlice to its parent file (#1870); a + # bare Path is the relative source_file itself. + allowed = [unit_path(item) for item in chunk] _scs( result.get("nodes", []), result.get("edges", []), diff --git a/tests/test_chunking.py b/tests/test_chunking.py index b39262adf..3c7ca95c1 100644 --- a/tests/test_chunking.py +++ b/tests/test_chunking.py @@ -203,6 +203,43 @@ def record(chunk, **kwargs): assert call_order == [("f0.py",), ("f1.py",), ("f2.py",)] +# ---- Incremental cache checkpoint -------------------------------------------- + +def test_checkpoint_allowlist_unwraps_file_slices(tmp_path, monkeypatch): + """#1870: a chunk containing FileSlice units must checkpoint with the + slice's parent path in allowed_source_files — leaking the FileSlice + object itself made save_semantic_cache raise, silently skipping the + checkpoint for every chunk with a sliced document.""" + from graphify import llm + from graphify.llm import extract_corpus_parallel + + big = tmp_path / "big.md" + big.write_text("# heading\n" + "paragraph line\n" * 40) + + captured = {} + + def _capture_cache(*args, **kwargs): + captured.update(kwargs) + return 0 + + monkeypatch.setattr("graphify.cache.save_semantic_cache", _capture_cache) + monkeypatch.delenv("GRAPHIFY_NO_INCREMENTAL_CACHE", raising=False) + + with patch.object(llm, "_FILE_CHAR_CAP", 100), \ + patch("graphify.llm.extract_files_direct", + side_effect=lambda chunk, **kw: _stub_chunk_result(len(chunk), 0)): + extract_corpus_parallel( + [big], backend="kimi", token_budget=None, chunk_size=16, max_concurrency=1 + ) + + allowed = captured.get("allowed_source_files") + assert allowed, "checkpoint should have written the chunk to the cache" + assert all(isinstance(p, Path) for p in allowed), ( + f"allowed_source_files must hold real paths, got: {allowed!r}" + ) + assert set(map(str, allowed)) == {str(big)} + + def test_corpus_parallel_merge_order_is_submission_order_not_completion(tmp_path): """#1632: merged node/edge order must be deterministic (submission order), not the order chunks' network calls happen to finish. We skew latencies so