Record which embedder wrote the stored vectors (#378, part 1) #28

Closed
claude wants to merge 3 commits from overnight/embedder-marker-pr into overnight/note-recall-pr
Contributor

Two commits: the marker, then a fix to it. First half of Vikunja #378 — the backfill itself is separate.

Why this needs to exist at all. The embedder changed from MiniLM to e5-small. The two models put text in completely different vector spaces, so comparing an old stored vector against a new query vector gives you noise, not a low score — she confidently returns an unrelated note. And both models are 384-dimensional, so a size check catches nothing. There is no way to notice this except by writing down which model wrote the vectors.

One marker for the whole DB, not a column per row. The backfill re-embeds every note and fact in one pass, so a per-row marker would hold the identical string in every row and cost a column on two tables for nothing. If backfill ever becomes resumable, per-row becomes right. New meta table, migration #11.

The identity is derived, not typed twice. It's the model filename plus the dimensions — multilingual-e5-small@384, hash@1024. Pointing voice.embedder.model_path at a different file changes the string by itself, so there's no constant to keep in sync and forget.

What it says on a mismatch:

voice: WARNING embedder MISMATCH — stored vectors were written by "…MiniLM…@384" but the configured embedder is "multilingual-e5-small@384"; recall scores are noise until the notes and facts are re-embedded

The second commit is me fixing the first. It treated "no marker" as "fresh DB, claim it for the current embedder" — which is exactly wrong for your box. Your DB was written by the old embedder and predates the marker, so on first start it would have been silently stamped as e5 and the warning would never have fired. The one case this whole task exists for was the one case it missed.

Now: no marker and no stored vectors is a fresh DB, claim it. No marker but vectors already there means the provenance is unknown, so it reports a mismatch and does not stamp the DB — stamping it would silence the warning on the next restart. There's a test for that specific case.

Nothing about recall changed. Detection and a log line only.

One decision left for you, deliberately. Should recall refuse to answer while the marker mismatches? A mismatch means every stored vector is noise, so recall will confidently return an unrelated note — which is worse than silence, and "not a guesser-of-truth" is a stated non-goal. The suggestion is to suppress vector-ranked results while mismatched and say she can't search her notes right now, rather than fail the turn. Nothing here does that; it's a behaviour change and it's your call.

One thing left alone: the check only runs when voice is wired, so a mavend started with voice off never looks at the marker. Harmless today because nothing else writes vectors — worth moving to store-open if that ever changes.

Two commits: the marker, then a fix to it. First half of Vikunja #378 — the backfill itself is separate. **Why this needs to exist at all.** The embedder changed from MiniLM to e5-small. The two models put text in completely different vector spaces, so comparing an old stored vector against a new query vector gives you **noise, not a low score** — she confidently returns an unrelated note. And **both models are 384-dimensional**, so a size check catches nothing. There is no way to notice this except by writing down which model wrote the vectors. **One marker for the whole DB**, not a column per row. The backfill re-embeds every note and fact in one pass, so a per-row marker would hold the identical string in every row and cost a column on two tables for nothing. If backfill ever becomes resumable, per-row becomes right. New `meta` table, migration **#11**. **The identity is derived, not typed twice.** It's the model filename plus the dimensions — `multilingual-e5-small@384`, `hash@1024`. Pointing `voice.embedder.model_path` at a different file changes the string by itself, so there's no constant to keep in sync and forget. **What it says on a mismatch:** > voice: WARNING embedder MISMATCH — stored vectors were written by "…MiniLM…@384" but the configured embedder is "multilingual-e5-small@384"; recall scores are noise until the notes and facts are re-embedded **The second commit is me fixing the first.** It treated "no marker" as "fresh DB, claim it for the current embedder" — which is **exactly wrong for your box.** Your DB was written by the old embedder and predates the marker, so on first start it would have been silently stamped as e5 and the warning would never have fired. The one case this whole task exists for was the one case it missed. Now: no marker *and* no stored vectors is a fresh DB, claim it. No marker *but vectors already there* means the provenance is unknown, so it reports a mismatch and **does not** stamp the DB — stamping it would silence the warning on the next restart. There's a test for that specific case. **Nothing about recall changed.** Detection and a log line only. **One decision left for you, deliberately.** Should recall refuse to answer while the marker mismatches? A mismatch means every stored vector is noise, so recall will confidently return an unrelated note — which is worse than silence, and "not a guesser-of-truth" is a stated non-goal. The suggestion is to suppress vector-ranked results while mismatched and say she can't search her notes right now, rather than fail the turn. Nothing here does that; it's a behaviour change and it's your call. **One thing left alone:** the check only runs when voice is wired, so a mavend started with voice off never looks at the marker. Harmless today because nothing else writes vectors — worth moving to store-open if that ever changes.
claude added 3 commits 2026-07-31 11:44:59 +02:00
The embedder moved from paraphrase-multilingual-MiniLM-L12-v2 to
multilingual-e5-small. Both are 384-dimensional, so nothing in the code
noticed: cosine between an old stored vector and a new query vector is
noise, and recall degrades silently.

So the DB now records the embedder that wrote its vectors. One value for
the whole DB (migration #11, a small `meta` key/value table) rather than a
column on every vector row: the backfill re-embeds every note and fact in
one pass, so a per-row marker would hold the same string in every row and
cost a column on two tables for nothing.

The identity comes from the embedder itself via a new optional ID() method
("multilingual-e5-small@384", model file name plus dimension), so pointing
the config at another model changes the string without anyone editing a
constant. mavend logs a loud WARNING at startup naming both the stored and
the configured embedder when they differ.

Detection only — recall behaviour is unchanged. TODO(#378) in
store.CheckEmbedder marks where the backfill will hook in.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
Owner

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47.

Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47. Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.
kami closed this pull request 2026-07-31 20:22:19 +02:00

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#28