From 1e47eaca5ab05e65eb9ac3e8f2124225529b3129 Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 31 Jul 2026 13:42:07 +0400 Subject: [PATCH] Record which embedder wrote the stored vectors and warn on a swap (#378) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ --- cmd/mavend/voice.go | 25 +++++++++++ internal/router/embedder.go | 23 ++++++++++ internal/router/embedderid_test.go | 24 +++++++++++ internal/router/onnxembedder.go | 21 ++++++++++ internal/store/meta.go | 65 +++++++++++++++++++++++++++++ internal/store/meta_test.go | 67 ++++++++++++++++++++++++++++++ internal/store/migrations.go | 5 +++ 7 files changed, 230 insertions(+) create mode 100644 internal/router/embedderid_test.go create mode 100644 internal/store/meta.go create mode 100644 internal/store/meta_test.go diff --git a/cmd/mavend/voice.go b/cmd/mavend/voice.go index 0df260a..06a1b35 100644 --- a/cmd/mavend/voice.go +++ b/cmd/mavend/voice.go @@ -171,6 +171,7 @@ func wireVoice(cfg *config.Config, coreAPI ipc.CoreAPI, phr phraser.Phraser, mem emb = router.NewHashEmbedder(1024) } w.embedder = emb + checkStoredEmbedder(dataStore, emb) // ----- tool executor (the enabled act allowlist, store-backed) ----- // Config tools are the declarative bootstrap: seed them into the store as @@ -1767,3 +1768,27 @@ func jsonStringImpl(s string) string { b = append(b, '"') return string(b) } + +// checkStoredEmbedder compares the embedder we just loaded with the one that +// wrote the vectors already in the DB (Vikunja #378). +// +// The two models we have both make 384-dim vectors, so a size check catches +// nothing: after a swap, recall silently compares vectors from different +// spaces and the scores are noise. So we say it out loud. Nothing is changed +// here — recall keeps running exactly as before until the backfill lands. +func checkStoredEmbedder(dataStore *store.Store, emb router.Embedder) { + if dataStore == nil { + return + } + current := router.EmbedderID(emb) + stored, mismatch, err := dataStore.CheckEmbedder(context.Background(), current) + if err != nil { + log.Printf("voice: embedder marker check failed: %v", err) + return + } + if mismatch { + log.Printf("voice: WARNING embedder MISMATCH — stored vectors were written by %q but the configured embedder is %q; recall scores are noise until the notes and facts are re-embedded (Vikunja #378)", stored, current) + return + } + log.Printf("voice: embedder marker ok (%s)", current) +} diff --git a/internal/router/embedder.go b/internal/router/embedder.go index a69f535..24e3eca 100644 --- a/internal/router/embedder.go +++ b/internal/router/embedder.go @@ -2,6 +2,7 @@ package router import ( "context" + "fmt" "math" "unicode" ) @@ -19,6 +20,24 @@ type Embedder interface { Close() error } +// IdentifiedEmbedder — an embedder that can name itself. The name goes into +// the DB next to the vectors it wrote, so a later model swap is caught instead +// of silently returning nonsense scores (Vikunja #378). +type IdentifiedEmbedder interface { + Embedder + ID() string +} + +// EmbedderID is the stable string stored alongside the vectors. It comes from +// the embedder itself — nobody hand-types a model name twice — and changes +// whenever the model or its dimension changes. +func EmbedderID(e Embedder) string { + if i, ok := e.(IdentifiedEmbedder); ok { + return i.ID() + } + return fmt.Sprintf("unknown@%d", e.Dim()) +} + // AsymmetricEmbedder — an embedder that wants to know whether a text is a // search query or a stored passage. Recall is asymmetric: a short question // goes in, a longer note comes out. The e5 family is trained for exactly that @@ -70,6 +89,10 @@ func NewHashEmbedder(dim int) *HashEmbedder { func (h *HashEmbedder) Dim() int { return h.dim } +// ID names this embedder for the DB marker. The dimension is part of it +// because a HashEmbedder of another width is a different vector space. +func (h *HashEmbedder) ID() string { return fmt.Sprintf("hash@%d", h.dim) } + func (h *HashEmbedder) Close() error { return nil } func (h *HashEmbedder) Embed(_ context.Context, text string) ([]float32, error) { diff --git a/internal/router/embedderid_test.go b/internal/router/embedderid_test.go new file mode 100644 index 0000000..03f1e16 --- /dev/null +++ b/internal/router/embedderid_test.go @@ -0,0 +1,24 @@ +package router + +import "testing" + +func TestEmbedderIDFromModelPath(t *testing.T) { + got := modelIDFromPath("/opt/maven/models/embedder/multilingual-e5-small.onnx") + if got != "multilingual-e5-small@384" { + t.Fatalf("modelIDFromPath = %q", got) + } + // A different model file must produce a different id, even at 384 dim. + old := modelIDFromPath("/opt/maven/models/embedder/paraphrase-multilingual-MiniLM-L12-v2.onnx") + if old == got { + t.Fatal("two different models share one id") + } +} + +func TestEmbedderIDIncludesDim(t *testing.T) { + if id := EmbedderID(NewHashEmbedder(1024)); id != "hash@1024" { + t.Fatalf("EmbedderID = %q", id) + } + if EmbedderID(NewHashEmbedder(1024)) == EmbedderID(NewHashEmbedder(384)) { + t.Fatal("dimension not part of the id") + } +} diff --git a/internal/router/onnxembedder.go b/internal/router/onnxembedder.go index 770790b..cbbf63c 100644 --- a/internal/router/onnxembedder.go +++ b/internal/router/onnxembedder.go @@ -33,6 +33,7 @@ const ( type onnxEmbedder struct { tokenizer *unigramTokenizer session *ort.DynamicSession[int64, float32] + id string } func NewONNXEmbedder(modelPath, tokenizerPath, libPath string) (*onnxEmbedder, error) { @@ -58,11 +59,31 @@ func NewONNXEmbedder(modelPath, tokenizerPath, libPath string) (*onnxEmbedder, e return &onnxEmbedder{ tokenizer: tok, session: session, + id: modelIDFromPath(modelPath), }, nil } func (e *onnxEmbedder) Dim() int { return embedDim } +// ID names the loaded model for the DB marker (Vikunja #378): the model file's +// own name plus the dimension, so pointing the config at another model changes +// the string on its own. +func (e *onnxEmbedder) ID() string { return e.id } + +// modelIDFromPath turns /opt/.../multilingual-e5-small.onnx into +// "multilingual-e5-small@384". +func modelIDFromPath(modelPath string) string { + name := modelPath + if i := strings.LastIndexAny(name, "/\\"); i >= 0 { + name = name[i+1:] + } + name = strings.TrimSuffix(name, ".onnx") + if name == "" { + name = "onnx" + } + return fmt.Sprintf("%s@%d", name, embedDim) +} + // Embed treats the text as a query. The classifier compares one short // utterance to another short seed phrase, so both sides get the same prefix // and the comparison stays fair. The recall path must call EmbedQuery and diff --git a/internal/store/meta.go b/internal/store/meta.go new file mode 100644 index 0000000..fc88905 --- /dev/null +++ b/internal/store/meta.go @@ -0,0 +1,65 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" +) + +// metaKeyEmbedderID names the embedder that wrote the stored vectors. +// +// Why one value for the whole DB and not a column on every vector row: the +// vectors are only ever rewritten all at once (one backfill re-embeds every +// note and fact together), so a per-row marker would hold the same string in +// every row and cost a column on two tables for nothing. +const metaKeyEmbedderID = "embedder_id" + +// Meta reads a single value from the meta table. Missing key ⇒ empty string. +func (s *Store) Meta(ctx context.Context, key string) (string, error) { + var v string + err := s.db.QueryRowContext(ctx, `SELECT value FROM meta WHERE key = ?`, key).Scan(&v) + if errors.Is(err, sql.ErrNoRows) { + return "", nil + } + if err != nil { + return "", fmt.Errorf("read meta %s: %w", key, err) + } + return v, nil +} + +// SetMeta writes (or overwrites) a single meta value. +func (s *Store) SetMeta(ctx context.Context, key, value string) error { + _, err := s.db.ExecContext(ctx, + `INSERT INTO meta (key, value) VALUES (?,?) + ON CONFLICT(key) DO UPDATE SET value = excluded.value`, key, value) + if err != nil { + return fmt.Errorf("write meta %s: %w", key, err) + } + return nil +} + +// CheckEmbedder compares the embedder now configured against the one that +// wrote the stored vectors. +// +// A DB that has never recorded one is claimed for the current embedder: either +// it is fresh (nothing stored yet, nothing to fix) or it predates this marker. +// Returns the stored id and whether it differs from the current one. +// +// Vectors from two different models live in different spaces, so cosine +// between them is noise rather than a low score — and both of our models are +// 384-dimensional, so nothing else catches it. +// +// TODO(#378): on a mismatch, run the one-shot backfill here — re-embed every +// stored note and fact text with the current embedder (EmbedPassage side), +// write the vectors back, then SetMeta the current id. +func (s *Store) CheckEmbedder(ctx context.Context, currentID string) (stored string, mismatch bool, err error) { + stored, err = s.Meta(ctx, metaKeyEmbedderID) + if err != nil { + return "", false, err + } + if stored == "" { + return currentID, false, s.SetMeta(ctx, metaKeyEmbedderID, currentID) + } + return stored, stored != currentID, nil +} diff --git a/internal/store/meta_test.go b/internal/store/meta_test.go new file mode 100644 index 0000000..173db56 --- /dev/null +++ b/internal/store/meta_test.go @@ -0,0 +1,67 @@ +package store + +import ( + "context" + "testing" +) + +// A fresh DB has no marker yet, so the current embedder is recorded and +// nothing is flagged. +func TestCheckEmbedderFreshDBRecords(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + + stored, mismatch, err := s.CheckEmbedder(ctx, "multilingual-e5-small@384") + if err != nil { + t.Fatalf("CheckEmbedder: %v", err) + } + if mismatch { + t.Fatal("fresh DB reported a mismatch") + } + if stored != "multilingual-e5-small@384" { + t.Fatalf("stored = %q", stored) + } + got, err := s.Meta(ctx, metaKeyEmbedderID) + if err != nil { + t.Fatalf("Meta: %v", err) + } + if got != "multilingual-e5-small@384" { + t.Fatalf("marker not persisted, got %q", got) + } +} + +// Both models are 384-dim, so this is the only thing that catches the swap. +func TestCheckEmbedderDifferentModelMismatch(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + + if err := s.SetMeta(ctx, metaKeyEmbedderID, "paraphrase-multilingual-MiniLM-L12-v2@384"); err != nil { + t.Fatalf("SetMeta: %v", err) + } + stored, mismatch, err := s.CheckEmbedder(ctx, "multilingual-e5-small@384") + if err != nil { + t.Fatalf("CheckEmbedder: %v", err) + } + if !mismatch { + t.Fatal("different embedder not detected") + } + if stored != "paraphrase-multilingual-MiniLM-L12-v2@384" { + t.Fatalf("stored = %q", stored) + } +} + +// The same embedder must never raise a false alarm, including on re-check. +func TestCheckEmbedderSameModelNoAlarm(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + + for i := 0; i < 2; i++ { + _, mismatch, err := s.CheckEmbedder(ctx, "multilingual-e5-small@384") + if err != nil { + t.Fatalf("CheckEmbedder: %v", err) + } + if mismatch { + t.Fatalf("false alarm on pass %d", i) + } + } +} diff --git a/internal/store/migrations.go b/internal/store/migrations.go index 2bb3b52..b9b7ff0 100644 --- a/internal/store/migrations.go +++ b/internal/store/migrations.go @@ -83,6 +83,11 @@ ALTER TABLE reminders ADD COLUMN next_fire_ts INTEGER;`, // #2 expires_ts INTEGER NOT NULL ); CREATE INDEX IF NOT EXISTS idx_dialogue_sessions_expires ON dialogue_sessions (expires_ts);`, // #10 — the follow-up session survives a restart (Vikunja #363); small, TTL-pruned table, not a history log + + `CREATE TABLE IF NOT EXISTS meta ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL + );`, // #11 — small key/value table for facts about the DB itself; first key is embedder_id (Vikunja #378) } // migrate applies every migration with a number greater than the DB's current