feat: multi-invocation incremental builds — --sync-key, changes/verify subcommands, snapshot hardening - #398
Open
ww2283 wants to merge 7 commits into
Conversation
…mmands, zero-chunk snapshot commit, LEANN_NO_REGISTER (#2) * feat(cli): --sync-key stable snapshot identity Keyed builds use one global FileSynchronizer over the expanded file manifest (snapshot sync_key_{sha256(key)[:12]}.pickle) instead of per-docs-list hashed snapshots, so varying --docs lists across invocations keep merkle incrementality. Key lifecycle persisted in sync_roots.json (reuse stored key; rekey requires --force). load_snapshot() now raises SnapshotCorruptError on unreadable pickles (missing file still means first build); keyed builds fail loudly on corruption instead of warn-and-skip. Also: remove-only deltas on non-IVF backends now trigger the full rebuild instead of returning early (previously gated on IVF). * feat(cli): leann changes subcommand Non-mutating merkle diff against the stored snapshot: prints one JSON document {added, modified, removed} (sorted) on stdout, diagnostics on stderr. Scope comes from --docs or, when omitted, from the index's stored sync_roots.json. Snapshot errors are strict (nonzero exit, no false-clean report) instead of build's warn-and-skip. LeannCLI no longer eagerly creates .leann/indexes at construction, so read-only commands leave the working directory untouched. * feat(cli): leann verify subcommand Cross-artifact integrity check: passages.jsonl/.idx offset and id-set consistency for any backend, plus IVF invariants (id-map exact inverses, id-map values vs offset-map keys, FAISS ntotal vs id-map cardinality, next_id monotonicity, missing index file). Exit 0 clean, 1 with a findings report. Covers vector/id-map/passage artifacts only (not BM25 sqlite or ids.txt). * fix(cli): commit snapshot on safely-empty incremental deltas An add/modify-only delta that loads successfully but produces zero chunks now commits the snapshot and sync config, so the same no-op delta is not rediscovered on every subsequent build. Loader failures and removal-bearing deltas still leave the snapshot untouched. Snapshot pickles and sync_roots.json now publish via tmp + os.replace so an interrupted write cannot corrupt existing state. * feat(registry): LEANN_NO_REGISTER env switch LEANN_NO_REGISTER=1 makes register_project_directory a no-op before any path traversal or ~/.leann creation, for automation running builds in disposable directories. * fix(cli): verify reads real on-disk IVF artifact names E2E smoke against a real IVF build showed verify looked for documents.leann.{meta.json → wrong via with_suffix, ivf_id_map.json, index}; the backend actually writes documents.leann.meta.json, documents.ivf_id_map.json and documents.index (id map and FAISS file use the stem without .leann). Test fixtures now mirror the real layout. * fix(cli): harden incremental build/changes/verify error paths from PR #2 review - refuse empty-delta snapshot commit when document loads failed (masked data loss) - changes: exit 1 on missing index, empty scope, or wrong --sync-key - corrupt sync snapshot / sync_roots.json fail loud; --force resets - sync: unreadable existing file keeps previous hash instead of 'removed' - verify: no crash on non-numeric id-map keys; no misleading findings when idx unreadable - drop dead 'changes --json' flag; register project dir on empty-delta commit - changelog entry for the PR's features * fix: address Gemini review findings for PR #2 - abort incremental build before mutation on any document load failure - verify: snapshot presence/unpickle checks, IVF type + mapped-id invlists check, type validation for decodable-but-malformed artifacts - verify test fixture builds a real IndexIVFFlat (was IndexFlatL2) - sync: os.walk onerror fails loud instead of mass-removal on unreadable subtree - changes: index-existence check applies with --docs too - sync_roots.json wrong-JSON-type handled by the --force recovery path 7 issues fixed, 0 false positives, 0 skipped See .doc/pr-review-comments/PR-2-multi-invocation-incremental.md * fix: watch survives fail-loud errors; changes rejects key on unkeyed index * fix: CI lint/type errors in new tests
With two faiss SWIG builds loaded in one process (faiss-cpu + the hnsw bundle, as on CI), downcast_index silently loses invlists access and the mapped-id check was skipped, returning a false healthy verdict. A subprocess loads exactly one build.
ubuntu-22.04's apt patchelf (0.14.3) fell below current auditwheel's >=0.14.5 floor, failing every Linux wheel-repair job.
Contributor
Author
|
Note on the Linux build failures: they are unrelated to this PR's code — current I've folded a one-line fix into this branch (ec83163): pip-install (The earlier macos failure was a real bug in this PR's |
* fix(verify): allow duplicate content-hash passage ids (#5) Content-hash ids are legitimately many-to-one for byte-identical chunks; verify's bijection invariants flagged healthy indexes. - jsonl dup ids allowed only when text is identical per id - idx cardinality compared against unique jsonl ids - IVF passage_to_id checked as last-wins partial inverse over deduped id_to_passage values * docs: changelog entry for verify duplicate-id fix (#5) * fix(verify): harden duplicate-id conflict check against mixed types Codex review of PR #6: type-prefixed text hash so 1 vs "1" counts as different text, and type-independent sort of conflicting ids so mixed int/str ids report findings instead of raising TypeError.
Contributor
Author
|
Appended |
Collaborator
|
@ww2283 Can we fix CI? |
Contributor
Author
|
@ASuresh0524 of course, the ci is too comprehensive:) |
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
2 participants
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.
Motivation
Callers that split large index builds into checkpointed multi-leg invocations (e.g. automation driving
leann buildwith varying--docslists) hit two limitations:--docslist — any variation between invocations creates a fresh snapshot namespace, losing all Merkle incrementality: unchanged files re-embed and duplicate.Changes (all opt-in / additive; no default behavior changes)
leann build --sync-key <key>— caller-supplied stable snapshot identity. Two builds with different--docslists but the same key share one snapshot; files common to both are embedded once. Mismatched keys are rejected unless--forcerekeys.leann changes <index> [--docs ...] [--sync-key K]— non-mutating Merkle diff; prints one JSON doc{added, modified, removed}(sorted) on stdout, exit 0. Errors (exit 1) on missing index, empty sync scope, wrong sync key, or corrupt snapshot rather than reporting a false clean delta.leann verify <index>— cross-artifact integrity check: passages.jsonl vs offsets pickle, IVF id-map exact inversion, FAISSntotalvs id-map cardinality, mapped ids present in the inverted lists,next_idmonotonicity, sync snapshots unpickle. Exit 0 healthy / 1 with findings.LEANN_NO_REGISTER=1— skip global~/.leann/projects.jsonregistration, for automation running builds in disposable directories.sync_roots.jsonfail loud on build (--forceresets); a transiently unreadable file keeps its previous hash instead of being classified as removed (which deleted its chunks); unreadable subtrees abort change detection instead of reading as mass removal;leann watchskips a tick on these errors instead of dying.Testing
40+ new tests across
tests/test_sync_key.py,test_cli_changes.py,test_cli_verify.py,test_zero_chunk_commit.py,test_no_register.py, plus additions totest_sync.py/test_watch_sync_scope.py. The verify fixtures build a realIndexIVFFlat+DirectMap.Hashtable. The changes went through three review rounds (two independent reviewer passes + a re-review) on our fork before this PR; 17 findings fixed.Rebased onto current main (dc85934);
import leann+ the touched suites pass and ruff is clean on this branch.