From 4282f6b9a98fcc0e2fb75a91666f87b1abfa4e40 Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 31 Jul 2026 13:44:29 +0400 Subject: [PATCH] Warn about vectors written before the marker existed --- internal/store/meta.go | 48 ++++++++++++++++++++++++++++++------- internal/store/meta_test.go | 33 +++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/internal/store/meta.go b/internal/store/meta.go index fc88905..8edd0e3 100644 --- a/internal/store/meta.go +++ b/internal/store/meta.go @@ -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 } diff --git a/internal/store/meta_test.go b/internal/store/meta_test.go index 173db56..e63f07d 100644 --- a/internal/store/meta_test.go +++ b/internal/store/meta_test.go @@ -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)