[travsr-embed] Add --reembed full-rebuild mode (v1.6.0) - #24
Conversation
…notice --reindex --reembed clears every stored vector for the active model and the recorded engine, then re-embeds every node with the current engine and rebuilds the HNSW index, so the index ends single-engine. This is the real command the mixed-engine notice points at (audit F3). Clearing before re-embedding keeps it crash-safe: a killed run leaves fewer rows, all from the current engine, and a later ordinary reindex fills the rest. Move the mixed-engine notice to a single end-of-run line instead of mid-progress (audit F8), phrased in plain language, and pointing at the command that now exists. Bump to v1.6.0.
raj-rkv
left a comment
There was a problem hiding this comment.
Reviewed against a fresh worktree off origin/main (ba99b76 vs d54adad, 0 commits behind). All CI green. I could not run the suite locally (this machine's c++ cannot build the vendored simsimd AVX-512 fp16 path in usearch 2.24.0), so I relied on CI plus reading the code.
The core design choice is right: clearing the model's rows up front so the existing NOT EXISTS drain loop is reused unchanged is much cleaner than trying to bypass the filter, and the reasoning is well documented. Moving the mixed-engine notice to end-of-run and pointing it at a command that now exists is a real improvement. Two issues.
1. The crash-safety claim does not cover the HNSW file, which is what search actually reads. Details inline on prepare_full_reembed.
2. --reembed is not guarded against the partial-work flags, and one of those combinations is reachable today from Travsr-com/travsr#772. Details inline on the argument validation block.
Neither is hard to fix and the rest of the change looks solid. The two new tests cover the right things: that provenance only reports a genuine engine change, and that the prep clears the active model while leaving other models alone.
…uard Remove the model's stale HNSW index files inside prepare_full_reembed, in the same step as the vector-row delete. rebuild_index only rewrites the .usearch files after the full re-embed, so before this a run killed mid reembed left a stale index that kept serving old-engine vectors for rows that no longer existed in embed.db. Dropping the index up front makes a killed run degrade to no index, which is visible and recoverable by a plain --reindex. Guard --reembed against the partial-work selectors (--phase1/--phase2/--shard/--row-start/--row-end). prepare_full_reembed clears the whole model before dispatch, so a partial selector would wipe everything and refill only a slice, and --shard would have each worker wipe the table its siblings are filling. The host cannot reach the shard case today, but the phase case is reachable from travsr#772. Update the CHANGELOG crash-safety wording to cover the index, and refresh the now-stale macos_engine doc comment in model.rs (F6 makes the key durable across embed init).
raj-rkv
left a comment
There was a problem hiding this comment.
Approving. Re-reviewed f3d956a in a refreshed worktree off origin/main (d54adad, still 0 behind). Both findings from the last round are properly fixed, not papered over, and you picked up the model.rs doc staleness I had only mentioned in passing.
Index crash-safety. The removal is in the same step as the row delete, covers both the code and doc index paths, and propagates anything that is not NotFound. I confirmed the paths match what rebuild_index writes (both helpers resolve against db_path.parent(), and main now passes the graph.db through). I also checked the platform question the change invites, and it holds up on both: unlink under an mmap is safe on Unix, and Windows serves from a load() copy so there is no open-handle conflict. reload_from_disk being mtime-gated means the daemon keeps serving its existing mapping rather than erroring while the index is briefly absent.
--reembed selector guard. All five selectors covered, placed before any deletion happens. phase is only ever moved off Phase::All by the two phase flags, so the matches! check is exact. The --shard case was the sharp one and it is now impossible.
CI is green across all eight jobs on this head, including the Windows build. I still could not run the suite locally (this machine's c++ cannot build the vendored simsimd AVX-512 fp16 path in usearch 2.24.0), so the test evidence here is CI's, not mine.
One sequencing note, not a blocker on this PR: ADR-019 in travsr#772 has this shipping first, and travsr#772's floor bump to v1.6.0 is currently red precisely because v1.6.0 does not exist yet. So this wants a merge plus a published v1.6.0 release before travsr#772 can go green.
Also worth knowing: the new guard makes travsr embed reindex --rebuild --phase1 N fail loudly at the sidecar instead of silently discarding the phase 2 tier, which is a real improvement. travsr#772 still deserves its own clap-level conflicts_with so the user gets a clean argument error rather than a sidecar exit, and I have left that finding open over there.
What
Adds a real full-rebuild path to the embed sidecar so a mixed-engine index can be made consistent, and stops the mixed-engine notice from firing mid-progress. Addresses EMBED_UX_AUDIT findings F3 and F8.
Changes
--reindex <db> --reembed(new): clears every stored vector for the active model and the recorded engine, then re-embeds every node with the current engine and rebuilds the HNSW index, so the index ends single-engine. Clearing before re-embedding keeps it crash-safe: a killed run leaves fewer rows, all from the current engine, and a later ordinary--reindexfills the rest.travsr embed reindex --rebuild, which the paired main-repo change adds.Testing
cargo clippy --all-targets -- -D warnings: clean.cargo test: 119 passed, 0 failed. Addsreembed_prep_clears_vectors_and_engineandprovenance_reports_only_a_real_engine_change.--reindex --reembed, confirmed all vectors cleared and re-embedded, both HNSW indexes rebuilt, and the index landed single-engine with no mid-run notice.Release ordering
This release must be cut before the paired main-repo PR (Travsr-com/travsr) is merged: that PR raises the embed sidecar version floor to v1.6.0, and its RFC-025 honesty test requires this release to be published first.