diff --git a/cmd/mavend/actions_fact.go b/cmd/mavend/actions_fact.go index 3aeaa5e..9cf5cf0 100644 --- a/cmd/mavend/actions_fact.go +++ b/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) + } +} diff --git a/cmd/mavend/factgate_test.go b/cmd/mavend/factgate_test.go index c5c6958..842bb2c 100644 --- a/cmd/mavend/factgate_test.go +++ b/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) + } +} diff --git a/internal/memory/store.go b/internal/memory/store.go index 5077e47..416227d 100644 --- a/internal/memory/store.go +++ b/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()