diff --git a/internal/memory/store.go b/internal/memory/store.go index d47f80c..5077e47 100644 --- a/internal/memory/store.go +++ b/internal/memory/store.go @@ -7,6 +7,20 @@ import ( "sync" ) +// NonRecallPrefix — rows whose id starts with this are excluded from Search by +// every backend. Speaker voiceprints live in the same vector table as notes and +// facts (internal/speaker writes them under this prefix), and they are not +// recall material: a voiceprint has no text to read back and surfacing one as a +// note hit leaks a name attached to a biometric. +// +// Reading them through Catalog.ByPrefix was documented as what keeps them out +// of recall. It is not. It controls how speaker code reads its own rows and +// says nothing about what Search scores. What actually kept them out was that +// cosine returns 0 on a width mismatch, so a 192-dim voiceprint scored 0 +// against a 384-dim query. That is a coincidence of two model choices — some +// x-vector exports are 384-dim — and not an invariant. This is the invariant. +const NonRecallPrefix = "speaker:" + // Result is a single search hit. type Result struct { ID string @@ -93,7 +107,14 @@ func (s *InMemoryStore) ByPrefix(_ context.Context, prefix string) ([]Record, er if !strings.HasPrefix(it.id, prefix) { continue } - out = append(out, Record{ID: it.id, Vec: append([]float32(nil), it.vec...), Meta: it.meta}) + // Copy the metadata too. Returning it.meta by reference let a caller + // mutating the returned map edit the stored row, and the persistent + // backend unmarshals fresh, so the two disagreed. + meta := make(map[string]string, len(it.meta)) + for k, v := range it.meta { + meta[k] = v + } + out = append(out, Record{ID: it.id, Vec: append([]float32(nil), it.vec...), Meta: meta}) } return out, nil } @@ -127,6 +148,9 @@ func (s *InMemoryStore) Search(_ context.Context, vec []float32, topK int) ([]Re scores := make([]scored, 0, len(s.items)) for _, it := range s.items { + if strings.HasPrefix(it.id, NonRecallPrefix) { + continue + } score := cosine(vec, it.vec) scores = append(scores, scored{id: it.id, score: score, meta: it.meta}) } diff --git a/internal/memory/store_test.go b/internal/memory/store_test.go index 06856fb..62c69a7 100644 --- a/internal/memory/store_test.go +++ b/internal/memory/store_test.go @@ -80,3 +80,75 @@ func TestCosineEdgeCases(t *testing.T) { t.Errorf("dot(1,2;1,2) = %f, want 5", c) } } + +// The in-memory backend has to hide voiceprints from recall exactly like the +// persistent one, or a test passing here says nothing about the daemon. +func TestInMemorySearchSkipsVoiceprints(t *testing.T) { + ctx := context.Background() + s := NewInMemoryStore() + if err := s.Insert(ctx, "note:1", []float32{0, 1}, map[string]string{"text": "заметка"}); err != nil { + t.Fatal(err) + } + if err := s.Insert(ctx, NonRecallPrefix+"kami", []float32{1, 0}, map[string]string{"name": "Ками"}); err != nil { + t.Fatal(err) + } + got, err := s.Search(ctx, []float32{1, 0}, 10) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 || got[0].ID != "note:1" { + t.Fatalf("Search = %+v, want just the note", got) + } + recs, err := s.ByPrefix(ctx, NonRecallPrefix) + if err != nil || len(recs) != 1 { + t.Fatalf("ByPrefix = %+v, %v; want the voiceprint", recs, err) + } +} + +// ByPrefix hands back a copy of the metadata. It used to return the stored map +// by reference, so a caller editing a returned Record silently edited the row, +// and the persistent backend did not behave that way. +func TestInMemoryByPrefixCopiesMeta(t *testing.T) { + ctx := context.Background() + s := NewInMemoryStore() + if err := s.Insert(ctx, "speaker:kami", []float32{1, 0}, map[string]string{"name": "Ками"}); err != nil { + t.Fatal(err) + } + recs, err := s.ByPrefix(ctx, "speaker:") + if err != nil || len(recs) != 1 { + t.Fatalf("ByPrefix = %+v, %v", recs, err) + } + recs[0].Meta["name"] = "не Ками" + + again, err := s.ByPrefix(ctx, "speaker:") + if err != nil { + t.Fatal(err) + } + if again[0].Meta["name"] != "Ками" { + t.Errorf("the stored row was edited through the returned map: %q", again[0].Meta["name"]) + } +} + +// Insert upserts. This is not only a speaker-profile concern: every user of the +// in-memory store used to accumulate a second row for a re-indexed id, and the +// stale copy stayed searchable. +func TestInMemoryInsertUpserts(t *testing.T) { + ctx := context.Background() + s := NewInMemoryStore() + if err := s.Insert(ctx, "note:1", []float32{1, 0}, map[string]string{"text": "старое"}); err != nil { + t.Fatal(err) + } + if err := s.Insert(ctx, "note:1", []float32{0, 1}, map[string]string{"text": "новое"}); err != nil { + t.Fatal(err) + } + got, err := s.Search(ctx, []float32{1, 0}, 10) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 { + t.Fatalf("Search returned %d rows, want 1 (the old copy is still searchable)", len(got)) + } + if got[0].Meta["text"] != "новое" { + t.Errorf("row = %q, want the replacement", got[0].Meta["text"]) + } +} diff --git a/internal/store/memory.go b/internal/store/memory.go index 3d0527a..bc83648 100644 --- a/internal/store/memory.go +++ b/internal/store/memory.go @@ -64,11 +64,17 @@ func (m *MemoryStore) Insert(ctx context.Context, id string, vec []float32, meta // Search returns the topK nearest rows by cosine similarity. A full scan; see // the type doc for why that's fine at this scale. +// +// Rows under memory.NonRecallPrefix are excluded in SQL. They are speaker +// voiceprints sharing this table, and note recall must not rank them; see that +// constant for why the previous arrangement only appeared to do this. func (m *MemoryStore) Search(ctx context.Context, vec []float32, topK int) ([]memory.Result, error) { if topK <= 0 { topK = 10 } - rows, err := m.db.QueryContext(ctx, `SELECT id, vec, meta FROM memory_vectors`) + rows, err := m.db.QueryContext(ctx, + `SELECT id, vec, meta FROM memory_vectors WHERE id NOT LIKE ? ESCAPE '\'`, + escapeLike(memory.NonRecallPrefix)+"%") if err != nil { return nil, fmt.Errorf("memory: scan: %w", err) } diff --git a/internal/store/memory_test.go b/internal/store/memory_test.go index 3e57243..8ba2b09 100644 --- a/internal/store/memory_test.go +++ b/internal/store/memory_test.go @@ -3,7 +3,10 @@ package store import ( "context" "path/filepath" + "strings" "testing" + + "github.com/kami/maven/internal/memory" ) func newMemTestStore(t *testing.T) *Store { @@ -101,3 +104,38 @@ func TestMemoryStorePersistsAcrossReopen(t *testing.T) { t.Fatalf("memory did not survive reopen: %v", got) } } + +// A voiceprint sharing the vector table must never rank as a note hit. The +// dimensions match here on purpose: what used to hide these rows was cosine +// returning 0 on a width mismatch, which is a property of two model choices and +// not of the store. +func TestMemoryStoreSearchSkipsVoiceprints(t *testing.T) { + ctx := context.Background() + m := newMemTestStore(t).VectorMemory() + + if err := m.Insert(ctx, "note:1", []float32{0, 1, 0}, map[string]string{"text": "заметка"}); err != nil { + t.Fatal(err) + } + if err := m.Insert(ctx, memory.NonRecallPrefix+"kami", []float32{1, 0, 0}, map[string]string{"name": "Ками"}); err != nil { + t.Fatal(err) + } + + got, err := m.Search(ctx, []float32{1, 0, 0}, 10) + if err != nil { + t.Fatalf("Search: %v", err) + } + for _, r := range got { + if strings.HasPrefix(r.ID, memory.NonRecallPrefix) { + t.Fatalf("recall returned a voiceprint: %+v", r) + } + } + if len(got) != 1 || got[0].ID != "note:1" { + t.Fatalf("Search = %+v, want just the note", got) + } + + // The row is still there for the speaker code that owns it. + recs, err := m.ByPrefix(ctx, memory.NonRecallPrefix) + if err != nil || len(recs) != 1 { + t.Fatalf("ByPrefix = %+v, %v; want the voiceprint", recs, err) + } +}