Repository navigation
Harden embedding: atomic model switch, consistent vectors, downloads - #204
Merged
Merged
Conversation
A stability review of the local-embedding feature surfaced several ways it could silently degrade. Fix them: - Atomic model switch: reindex_all clears + rebuilds chunks in one transaction (the DELETE used to commit before embedding, so a mid-embed failure wiped the index for good), and set_embedder_and_reindex reverts to the previous model on any failure. apply_embedding_model surfaces the error instead of swallowing it, so a failed switch never leaves a new embedder pointed at stale-dimension vectors or a silently-empty index. - Consistent vectors: embed passages one at a time. A quantized model's per-batch scales made a batched passage drift from the same text embedded alone (verified cos 0.965), so stored vectors did not match single-text queries; one-at-a-time keeps every vector identical. - Download integrity: validate the body length against Content-Length, so a truncated transfer errors and cleans up instead of renaming a short file into place (which then fails to load on every retry). - Fail loudly on a zero-length tokenizer encoding instead of emitting an all-zero embedding.
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
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.
Summary
A stability review of the local notes-embedding feature surfaced several ways it could silently degrade. This hardens them:
reindex_allnow clears + rebuilds chunks in a single transaction — theDELETE FROM chunkpreviously committed before embedding, so a mid-embed failure wiped the index for good. Newset_embedder_and_reindexreverts to the previous embedder on any failure, andapply_embedding_modelsurfaces the error instead of swallowing it. A failed switch now leaves the previous model + index exactly as they were, rather than a new embedder pointed at stale-dimension vectors or a silently-emptied index.Content-Length, so a truncated transfer errors + removes its.partinstead of renaming a short file into place (which then fails to load on every retry, since the present short file is skipped).Test plan
a_failed_model_switch_keeps_the_old_index_and_embedderproves a failing embedder neither wipes the corpus nor activates — the previous embedder + chunks survive. Full suite 38/38.--ignored, real models): newqwen3_passage_embedding_is_independent_of_neighboursproves a passage embeds identically alone vs. alongside others on real Qwen3 (was cos 0.965 batched, now > 0.999); encoder (BGE-M3, GTE) + decoder ranking smoke tests still pass. 20 unit tests pass.cargo clippy -D warningsclean on wisp-embed, wisp-library, and the app;tauri devrebuilds + runs.