Warn about vectors written before the marker existed
This commit is contained in:
+40
-8
@@ -39,17 +39,27 @@ func (s *Store) SetMeta(ctx context.Context, key, value string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// EmbedderUnknown is the stored id reported for a DB that already holds
|
||||
// vectors but never recorded who wrote them.
|
||||
const EmbedderUnknown = "unknown (written before this marker existed)"
|
||||
|
||||
// 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.
|
||||
// wrote the stored vectors. Returns the stored id and whether it differs.
|
||||
//
|
||||
// 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.
|
||||
//
|
||||
// Three cases, and the middle one is the one that actually matters:
|
||||
//
|
||||
// - marker present ⇒ compare the two ids.
|
||||
// - marker absent but vectors already stored ⇒ this is a DB from before the
|
||||
// marker, so we cannot know who wrote them. Report a mismatch. This is the
|
||||
// real case on the deployed box: those vectors came from the old embedder,
|
||||
// and claiming them for the current one would hide the exact problem the
|
||||
// marker was added to catch.
|
||||
// - marker absent and no vectors ⇒ fresh DB, claim it, nothing to fix.
|
||||
//
|
||||
// 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.
|
||||
@@ -58,8 +68,30 @@ func (s *Store) CheckEmbedder(ctx context.Context, currentID string) (stored str
|
||||
if err != nil {
|
||||
return "", false, err
|
||||
}
|
||||
if stored == "" {
|
||||
return currentID, false, s.SetMeta(ctx, metaKeyEmbedderID, currentID)
|
||||
if stored != "" {
|
||||
return stored, stored != currentID, nil
|
||||
}
|
||||
return stored, stored != currentID, nil
|
||||
n, err := s.countVectors(ctx)
|
||||
if err != nil {
|
||||
return "", false, err
|
||||
}
|
||||
if n > 0 {
|
||||
return EmbedderUnknown, true, nil
|
||||
}
|
||||
return currentID, false, s.SetMeta(ctx, metaKeyEmbedderID, currentID)
|
||||
}
|
||||
|
||||
// countVectors — how many stored rows carry an embedding. Used only to tell a
|
||||
// fresh DB apart from one that predates the marker.
|
||||
func (s *Store) countVectors(ctx context.Context) (int, error) {
|
||||
var notes, vecs int
|
||||
if err := s.db.QueryRowContext(ctx,
|
||||
`SELECT count(*) FROM notes WHERE embedding IS NOT NULL`).Scan(¬es); err != nil {
|
||||
return 0, fmt.Errorf("count note vectors: %w", err)
|
||||
}
|
||||
if err := s.db.QueryRowContext(ctx,
|
||||
`SELECT count(*) FROM memory_vectors`).Scan(&vecs); err != nil {
|
||||
return 0, fmt.Errorf("count memory vectors: %w", err)
|
||||
}
|
||||
return notes + vecs, nil
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ package store
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// A fresh DB has no marker yet, so the current embedder is recorded and
|
||||
@@ -30,6 +31,38 @@ func TestCheckEmbedderFreshDBRecords(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The deployed box: notes were written by the old embedder, before the marker
|
||||
// existed. Claiming them for the current one would hide exactly the problem
|
||||
// the marker is for, so an unmarked DB that already holds vectors is a
|
||||
// mismatch.
|
||||
func TestCheckEmbedderUnmarkedDBWithVectorsIsMismatch(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
if _, err := s.WriteNote(ctx, time.Now(), "молоко в холодильнике", []float32{0.1, 0.2}, "voice"); err != nil {
|
||||
t.Fatalf("WriteNote: %v", err)
|
||||
}
|
||||
|
||||
stored, mismatch, err := s.CheckEmbedder(ctx, "multilingual-e5-small@384")
|
||||
if err != nil {
|
||||
t.Fatalf("CheckEmbedder: %v", err)
|
||||
}
|
||||
if !mismatch {
|
||||
t.Fatal("an unmarked DB with stored vectors should report a mismatch")
|
||||
}
|
||||
if stored != EmbedderUnknown {
|
||||
t.Fatalf("stored = %q, want %q", stored, EmbedderUnknown)
|
||||
}
|
||||
// It must NOT claim the DB — that would silence the warning on restart.
|
||||
got, err := s.Meta(ctx, metaKeyEmbedderID)
|
||||
if err != nil {
|
||||
t.Fatalf("Meta: %v", err)
|
||||
}
|
||||
if got != "" {
|
||||
t.Fatalf("marker written despite unknown provenance: %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)
|
||||
|
||||
Reference in New Issue
Block a user