Skip to content

[travsr-embed] Add --reembed full-rebuild mode (v1.6.0) - #24

Merged
anketpratapsingh merged 2 commits into
mainfrom
feat/reembed-mode
Aug 23, 2026
Merged

[travsr-embed] Add --reembed full-rebuild mode (v1.6.0)#24
anketpratapsingh merged 2 commits into
mainfrom
feat/reembed-mode

Conversation

@Abhishek5517

Copy link
Copy Markdown
Contributor

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 --reindex fills the rest.
  • Mixed-engine notice moved to end-of-run (F8): emitted once at the end of a run instead of mid-progress, phrased plainly, and pointing at travsr embed reindex --rebuild, which the paired main-repo change adds.
  • Version bump to v1.6.0.

Testing

  • cargo clippy --all-targets -- -D warnings: clean.
  • cargo test: 119 passed, 0 failed. Adds reembed_prep_clears_vectors_and_engine and provenance_reports_only_a_real_engine_change.
  • Real E2E on a copy of a live repo (arctic-embed-m, 8,868 nodes): simulated a prior engine, ran --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.

…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 raj-rkv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/main.rs Outdated
Comment thread src/main.rs
…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 raj-rkv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@anketpratapsingh
anketpratapsingh merged commit dfaa4da into main Aug 23, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants