fix(llm): unwrap FileSlice via unit_path in the checkpoint allowlist (#1870)#1882
Open
HerenderKumar wants to merge 1 commit into
Open
fix(llm): unwrap FileSlice via unit_path in the checkpoint allowlist (#1870)#1882HerenderKumar wants to merge 1 commit into
HerenderKumar wants to merge 1 commit into
Conversation
…raphify-Labs#1870) _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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1870
_checkpoint_chunkresolved each unit's source file withgetattr(item, "rel", None) or item, butFileSlicecarries its file in.path— it has no.rel— so for every sliced document theFileSliceobject itself leaked intoallowed_source_files.save_semantic_cachethen raised onPath(FileSlice), the best-effort guard downgraded that to theincremental cache checkpoint failedwarning, and every chunk containing a slice silently lost its checkpoint — so a resumed run re-billed those chunks instead of reusing the cache.llm.py: build the allowlist withunit_path(...)—file_slice.py's canonical unwrap (parentPathfor a slice, barePathunchanged), already imported and used elsewhere in the module — and fix the stale.relcommenttests/test_chunking.py: regression test that drivesextract_corpus_parallelover a corpus whose document slices into 7 units and asserts the checkpoint fires with real paths; on currentv8it fails with the exactFileSliceleak from the reportSince a slice's
.pathis the same value the file would have had as a barePath, the allowlist entries for sliced files now match unsliced behavior exactly.