recall: a re-tapped fact drops its superseded vector (V-493)

The fact vector id carries a timestamp, so tapping the same key twice added a
row instead of replacing one and recall then scored the old value against the
current one. CorrectValue and VoidLatestFact already prune the key; an ordinary
re-tap is the third way a value is superseded and it did not.

actionFact now prunes fact:<key>: before inserting, so exactly one vector
survives per key. InMemoryStore gained the matching DeletePrefix, because a
test double that quietly kept both rows would pass a test the daemon fails.

The prune is best-effort and silent on a store that cannot do it: the fact row
is the truth, and a stale vector costs a wrong recall, not a lost fact.

cmd/mavend/actions_fact.go

@@ -6,6 +6,7 @@ import (
 	"strconv"
 
 	"github.com/kami/maven/internal/ipc"
+	"github.com/kami/maven/internal/memory"
 	"github.com/kami/maven/internal/phraser"
 	"github.com/kami/maven/internal/router"
 	"github.com/kami/maven/internal/store"
@@ -89,7 +90,15 @@ func (h *reactiveHandler) actionFact(ctx context.Context, dec router.Decision) s
 	// hears; storing the utterance meant recall answered with his own sentence
 	// rather than the value. The utterance stays alongside as provenance —
 	// readable on /trace, never the answer and never embedded.
+	//
+	// The vector id carries a timestamp, so a second tap of the same key adds a
+	// row rather than replacing one, and recall then scores the superseded
+	// value against the current one. CorrectValue and VoidLatestFact already
+	// drop the key's vectors; an ordinary re-tap is the third way a value is
+	// superseded and it did not (#493). Dropping first keeps exactly one vector
+	// per key, which is what "recall answers with the current value" means.
 	if h.recall.memStore != nil {
+		pruneFactVectors(ctx, h.recall.memStore, dec.Slots.Key)
 		text := store.FactRecallText(dec.Slots.Key, dec.Slots.Value)
 		if vec, err := router.EmbedPassage(ctx, h.recall.embedder, text); err != nil {
 			log.Printf("voice: embed fact for memory: %v", err)
@@ -114,3 +123,29 @@ func (h *reactiveHandler) actionFact(ctx context.Context, dec router.Decision) s
 	}
 	return "" // replier phrases the success reply
 }
+
+// vectorPruner — the part of the vector index this file needs and memory.Store
+// does not carry. store.MemoryStore implements it; the in-memory test double
+// may not, and a double that cannot prune is not a reason to fail a fact write.
+type vectorPruner interface {
+	DeletePrefix(ctx context.Context, prefix string) (int64, error)
+}
+
+// pruneFactVectors drops every vector for one fact key, so the insert that
+// follows is the only one left. Best-effort and silent on a store that cannot
+// prune: the fact row is the truth, and a stale vector costs a wrong recall,
+// not a lost fact.
+func pruneFactVectors(ctx context.Context, ms memory.Store, key string) {
+	p, ok := ms.(vectorPruner)
+	if !ok {
+		return
+	}
+	n, err := p.DeletePrefix(ctx, "fact:"+key+":")
+	if err != nil {
+		log.Printf("voice: prune memory vectors for %q: %v", key, err)
+		return
+	}
+	if n > 0 {
+		log.Printf("voice: %q superseded, dropped %d stale memory vector(s)", key, n)
+	}
+}

cmd/mavend/factgate_test.go

@@ -167,3 +167,69 @@ func TestActionFact_AskedToRememberAComplaintStillWrites(t *testing.T) {
 		t.Fatalf("an explicit capture was refused: %v", err)
 	}
 }
+
+// A second tap of the same key supersedes the first, so recall must hold one
+// vector and it must be the new value (Vikunja #493). Before this the id
+// carried a timestamp, both rows stayed, and the superseded value went on
+// competing for the turn.
+func TestActionFact_ARetapSupersedesTheOldVector(t *testing.T) {
+	ctx := context.Background()
+	h, _ := newFactGateHandler(t, time.Now())
+
+	h.actionFact(ctx, router.Decision{
+		Intent:    router.IntentFact,
+		Utterance: "запиши что я пил воду",
+		Slots:     router.Slots{Key: "water", HasKey: true, Value: `"250мл"`},
+	})
+	// A later tap of the same key. The clock moves, so the old id and the new
+	// one differ — which is exactly what used to leave two rows behind.
+	h.now = func() time.Time { return time.Now().Add(time.Hour) }
+	h.actionFact(ctx, router.Decision{
+		Intent:    router.IntentFact,
+		Utterance: "запиши что я пил воду",
+		Slots:     router.Slots{Key: "water", HasKey: true, Value: `"500мл"`},
+	})
+
+	hits, err := h.recall.memStore.Search(ctx, mustEmbedPassage(t, h, "вода"), 5)
+	if err != nil {
+		t.Fatalf("memory search: %v", err)
+	}
+	if len(hits) != 1 {
+		t.Fatalf("want one vector for the key, got %d: %+v", len(hits), hits)
+	}
+	if got := hits[0].Meta["text"]; got != "water — 500мл" {
+		t.Errorf("indexed text = %q, want the current value", got)
+	}
+}
+
+// Another key is not this key. A prefix delete that widened would take the
+// whole index with it.
+func TestActionFact_ARetapLeavesOtherKeysAlone(t *testing.T) {
+	ctx := context.Background()
+	h, _ := newFactGateHandler(t, time.Now())
+
+	h.actionFact(ctx, router.Decision{
+		Intent:    router.IntentFact,
+		Utterance: "запиши что я обедал",
+		Slots:     router.Slots{Key: "meal", HasKey: true, Value: `"суп"`},
+	})
+	h.actionFact(ctx, router.Decision{
+		Intent:    router.IntentFact,
+		Utterance: "запиши что я пил воду",
+		Slots:     router.Slots{Key: "water", HasKey: true, Value: `"250мл"`},
+	})
+
+	hits, err := h.recall.memStore.Search(ctx, mustEmbedPassage(t, h, "обед"), 5)
+	if err != nil {
+		t.Fatalf("memory search: %v", err)
+	}
+	var found bool
+	for _, hit := range hits {
+		if hit.Meta["text"] == "meal — суп" {
+			found = true
+		}
+	}
+	if !found {
+		t.Fatalf("writing water dropped the meal vector: %+v", hits)
+	}
+}

internal/memory/store.go

@@ -132,6 +132,26 @@ func (s *InMemoryStore) Delete(_ context.Context, id string) error {
 	return nil
 }
 
+// DeletePrefix removes every row whose id starts with prefix and returns how
+// many went. It matches store.MemoryStore's method so a test double and the
+// real index agree about superseding a fact (Vikunja #493) — a double that
+// silently kept the old vectors would pass a test the daemon fails.
+func (s *InMemoryStore) DeletePrefix(_ context.Context, prefix string) (int64, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	kept := s.items[:0]
+	var n int64
+	for _, it := range s.items {
+		if strings.HasPrefix(it.id, prefix) {
+			n++
+			continue
+		}
+		kept = append(kept, it)
+	}
+	s.items = kept
+	return n, nil
+}
+
 func (s *InMemoryStore) Search(_ context.Context, vec []float32, topK int) ([]Result, error) {
 	s.mu.RLock()
 	defer s.mu.RUnlock()