memory: keep voiceprints out of note and fact recall

Speaker profiles share the vector table with notes and facts. The doc
comment said reading them through Catalog is what keeps recall from
ranking a voiceprint. It is not. Catalog controls how speaker code reads
its own rows and says nothing about Search, which scanned every row.
What actually hid them was cosine returning 0 on a width mismatch, so a
192-dim ECAPA row scored 0 against a 384-dim query. Some x-vector
exports are 384-dim, and one of those would have surfaced speaker:kami
as a recall hit carrying the name of a person.

Both backends now skip the prefix in Search, and the prefix is one
constant in internal/memory so the store layer can filter on it without
importing internal/speaker.

Two more differences between the backends closed here. ByPrefix on the
in-memory store returned the stored metadata map by reference, so a
caller editing a returned Record edited the row, while the persistent
one unmarshals fresh. And the append to upsert change in Insert is a fix
in its own right, not only a speaker concern: any re-indexed id used to
leave a second stale copy searchable.

Found in review of #74.

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})
 	}

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"])
+	}
+}

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)
 	}

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)
+	}
+}