diff --git a/cmd/mavend/actions_fact.go b/cmd/mavend/actions_fact.go index 9a8fc93..5645282 100644 --- a/cmd/mavend/actions_fact.go +++ b/cmd/mavend/actions_fact.go @@ -7,6 +7,7 @@ import ( "github.com/kami/maven/internal/ipc" "github.com/kami/maven/internal/router" + "github.com/kami/maven/internal/store" ) // actionFact handles router.IntentFact: persist a tapped self-fact, index @@ -63,17 +64,25 @@ func (h *reactiveHandler) actionFact(ctx context.Context, dec router.Decision) s log.Printf("voice: write fact: %v", err) return "не получилось сохранить факт." } - // Index the fact utterance in long-term memory (best-effort, must not - // fail the fact write). Facts aren't in the notes table, so this is the - // only recall path for them — "когда я пил воду?" reads back from here. + // Index the fact in long-term memory (best-effort, must not fail the fact + // write). Facts aren't in the notes table, so this is the only recall path + // for them — "когда я пил воду?" reads back from here. + // + // The indexed text is the fact, not the utterance (#493). queryMemory + // returns a fact's stored text verbatim, so what goes in here is what he + // 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. if h.memStore != nil { - if vec, err := router.EmbedPassage(ctx, h.embedder, dec.Utterance); err != nil { + text := store.FactRecallText(dec.Slots.Key, dec.Slots.Value) + if vec, err := router.EmbedPassage(ctx, h.embedder, text); err != nil { log.Printf("voice: embed fact for memory: %v", err) } else if err := h.memStore.Insert(ctx, "fact:"+dec.Slots.Key+":"+strconv.FormatInt(now.Unix(), 10), vec, map[string]string{ - "source": "voice", - "type": "fact", - "text": dec.Utterance, - "ts": strconv.FormatInt(now.Unix(), 10), + "source": "voice", + "type": "fact", + "text": text, + "utterance": dec.Utterance, + "ts": strconv.FormatInt(now.Unix(), 10), }); err != nil { log.Printf("voice: memory insert fact: %v", err) } diff --git a/cmd/mavend/factgate_test.go b/cmd/mavend/factgate_test.go index 89cca2e..5dc205f 100644 --- a/cmd/mavend/factgate_test.go +++ b/cmd/mavend/factgate_test.go @@ -78,6 +78,22 @@ func TestActionFact_ExplicitCaptureStillWrites(t *testing.T) { if f.Confidence != 1.0 { t.Errorf("confidence = %v, want 1.0 for a value he said", f.Confidence) } + // #493: what recall reads back is the fact, not the sentence he said. + // queryMemory returns a fact's text verbatim, so the utterance sitting here + // meant "запиши что я пил воду" was the answer to "когда я пил воду?". + hits, err := h.memStore.Search(ctx, mustEmbedPassage(t, h, "вода"), 3) + if err != nil { + t.Fatalf("memory search: %v", err) + } + if len(hits) != 1 { + t.Fatalf("the fact was not indexed once: %+v", hits) + } + if got := hits[0].Meta["text"]; got != "water — вода" { + t.Errorf("indexed text = %q, want the fact", got) + } + if got := hits[0].Meta["utterance"]; got != "запиши что я пил воду" { + t.Errorf("utterance provenance = %q, want it kept alongside", got) + } } func TestFactConfidence(t *testing.T) { diff --git a/cmd/mavend/voicewire.go b/cmd/mavend/voicewire.go index 70cf136..f4fd334 100644 --- a/cmd/mavend/voicewire.go +++ b/cmd/mavend/voicewire.go @@ -146,6 +146,7 @@ func wireVoice(cfg *config.Config, coreAPI ipc.CoreAPI, phr phraser.Phraser, mem emb = router.NewHashEmbedder(1024) } w.embedder = emb + repairFactVectors(dataStore, emb) checkStoredEmbedder(dataStore, emb) // ----- tool executor (the enabled act allowlist, store-backed) ----- @@ -473,6 +474,36 @@ func seedTools(api ipc.CoreAPI, tools []config.ToolConfig) { log.Printf("voice: seeded %d act tools from config", n) } +// repairFactVectors brings stored fact vectors in line with the facts they name +// (#493), once per box, before the embedder marker is even looked at. +// +// Automatic and not a flag, unlike -reembed: only voice-tapped facts are in +// this index, so the work is tens of embeddings rather than the thousands of +// notes that made the backfill a deliberate act. And the box that needs it is +// broken in a way nobody can see — recall answers with the wrong text and +// nothing logs an error — so waiting for an operator to know to run it is how +// the defect survived four restarts in the first place. +func repairFactVectors(dataStore *store.Store, emb router.Embedder) { + if dataStore == nil { + return + } + res, err := dataStore.RepairFactVectors(context.Background(), + // EmbedPassage, the stored side, same as every other writer of these + // vectors. + func(ctx context.Context, text string) ([]float32, error) { + return router.EmbedPassage(ctx, emb, text) + }) + if err != nil { + log.Printf("voice: fact vector repair failed, no marker written and nothing half-done — retried next start: %v", err) + return + } + if res.Skipped || res.Rewritten+res.Dropped == 0 { + return + } + log.Printf("voice: fact vector repair — %d re-embedded from the fact they name, %d dropped as voided or superseded, %d already right, took %s (#493)", + res.Rewritten, res.Dropped, res.Kept, res.Took.Round(time.Millisecond)) +} + // reembedOnStart is the -reembed flag (set in run()). Opt-in on purpose: see // runReembed. var reembedOnStart bool diff --git a/docs/qa.md b/docs/qa.md index 5e08d5b..ec5d4fe 100644 --- a/docs/qa.md +++ b/docs/qa.md @@ -498,12 +498,21 @@ rejects `https://api.openai.com`, and forget really deletes (`internal/store/memory.go:145` is a real `DELETE`, not a tombstone). Vision is 19/19, speaker 22/22, media 16/16. -**470 got worse.** Both poisoned facts show `voided` on `/history`, and the -defect survives. Re-measured at 15:42, after four restarts: `почему небо синее?` -still answers `какая последняя версия языка Go?` with no `search:` line. What -comes back is the question he typed, not the value the fact held. So the poison -is a vector in the memory index, and `revert` does not remove it. There is -currently no documented way to repair a poisoned box. +**470 got worse, then closed.** Both poisoned facts showed `voided` on +`/history` and the defect survived. Re-measured at 15:42, after four restarts: +`почему небо синее?` still answered `какая последняя версия языка Go?` with no +`search:` line. What came back was the question he typed, not the value the fact +held. So the poison was a vector in the memory index, and `revert` did not +remove it. + +Repaired in two parts. 470 stopped the writes: a question is never a fact, and a +void drops the key's vectors. 493 fixed what the index holds. A fact is indexed +as the fact and not as the utterance, and a correction drops its superseded +vector too. + +A poisoned box now repairs itself on the next start. `RepairFactVectors` +re-embeds every fact vector from the fact it names, and deletes the voided and +superseded ones. It runs once, guarded by a marker, and logs what it did. --- diff --git a/internal/store/facts.go b/internal/store/facts.go index f8bbef7..6e0a2c6 100644 --- a/internal/store/facts.go +++ b/internal/store/facts.go @@ -47,6 +47,39 @@ func (s *Store) WriteFact(ctx context.Context, ts time.Time, kind FactKind, key, return id, nil } +// FactRecallText is the text a fact is indexed under and read back as (#493). +// +// It used to be the utterance that wrote the fact, so recall of ANY +// voice-tapped fact answered with the sentence he said instead of the value +// stored: `go_version = 1.20` was indexed as "какая последняя версия языка +// Go?", and that question is what came back. The poisoned rows made the defect +// visible; the shape was wrong for legitimate facts too. +// +// The key is spoken with its underscores dropped, because a key is written for +// the store and this string is read out loud. +func FactRecallText(key, value string) string { + spoken := strings.TrimSpace(strings.ReplaceAll(key, "_", " ")) + v := strings.TrimSpace(DecodeFactValue(value)) + switch { + case v == "": + return spoken + case spoken == "": + return v + } + return spoken + " — " + v +} + +// DecodeFactValue unwraps a stored value for reading. The column holds raw json +// when the writer serialized one (SetValue, CorrectValue) and a plain string +// when it did not (a voice tap), so a reader that wants the text handles both. +func DecodeFactValue(value string) string { + var s string + if err := json.Unmarshal([]byte(value), &s); err == nil { + return s + } + return value +} + // LatestFact returns the latest non-voided fact for key, or ErrNoFact. // "Non-voided" = no later row has voids_id pointing at it. We resolve this by // taking the newest row whose id is not referenced by any voids_id. @@ -291,6 +324,20 @@ func (s *Store) CorrectValue(ctx context.Context, key, source string, value any, if err != nil { return 0, fmt.Errorf("last insert id: %w", err) } + // The same repair a void needs, for the same reason (#493). A correction + // supersedes the value, and the vector still holds the old one, so recall + // kept answering with the value he had just corrected. Dropping it costs + // the key its recall vector until the fact is tapped again: this layer has + // no embedder, and a missing vector loses a question while a stale one + // answers it wrongly. + // + // Best-effort: the corrected row is committed, and a correction that lands + // beats one that fails on cleanup. + if n, derr := s.VectorMemory().DeletePrefix(ctx, "fact:"+key+":"); derr != nil { + log.Printf("store: correct %q: memory vectors survive: %v", key, derr) + } else if n > 0 { + log.Printf("store: correct %q: dropped %d superseded memory vector(s)", key, n) + } return newID, nil } diff --git a/internal/store/factvectors.go b/internal/store/factvectors.go new file mode 100644 index 0000000..49b0360 --- /dev/null +++ b/internal/store/factvectors.go @@ -0,0 +1,177 @@ +package store + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strconv" + "strings" + "time" +) + +// metaKeyFactVectorShape names the shape the stored fact vectors were written +// in. It exists so the repair below runs once per box instead of on every +// start: the rows it fixes were written by a code path that no longer exists, +// and once fixed nothing writes that shape again. +const metaKeyFactVectorShape = "fact_vector_shape" + +// factVectorShapeFact is the shape FactRecallText produces. Anything else in +// the marker (including nothing, which is every box written before #493) means +// the fact vectors still hold utterances. +const factVectorShapeFact = "fact-text (#493)" + +// FactVectorRepair is what one repair run did, for logging. +type FactVectorRepair struct { + Skipped bool // marker already matched — nothing to do + Rewritten int // rows re-embedded from the fact they name + Dropped int // rows deleted: voided, superseded, or naming no fact at all + Kept int // rows already holding the right text + Took time.Duration +} + +// RepairFactVectors brings the fact rows of memory_vectors in line with the +// facts they name, and is the operator recovery a poisoned box had no path to +// (#470 point 4, #493). +// +// Three defects put wrong text in that index, and all three are write-path +// fixes that do nothing for rows already stored: +// +// - the indexed text was the utterance, so every fact row reads back a +// sentence rather than a value; +// - a void left its vector behind, so retracted junk kept answering; +// - a correction left its vector behind, so the superseded value did. +// +// So each fact row is resolved against the fact store and one of three things +// happens. It is dropped when the key has no fact, when the newest row for the +// key is a void marker, or when a newer vector for the same key exists — a +// superseded value has no business claiming a turn. It is re-embedded when its +// text is not what FactRecallText says the fact is. Otherwise it is left alone. +// +// Idempotent, and safe to interrupt: every step compares before writing and the +// marker is written last, so a run that dies partway is simply redone. +func (s *Store) RepairFactVectors(ctx context.Context, embed EmbedFunc) (FactVectorRepair, error) { + start := time.Now() + var res FactVectorRepair + + shape, err := s.Meta(ctx, metaKeyFactVectorShape) + if err != nil { + return res, err + } + if shape == factVectorShapeFact { + res.Skipped = true + res.Took = time.Since(start) + return res, nil + } + + rows, err := s.db.QueryContext(ctx, `SELECT id, meta FROM memory_vectors`) + if err != nil { + return res, fmt.Errorf("repair fact vectors: read: %w", err) + } + type factVec struct { + id, key string + meta map[string]string + ts int64 + } + var vecs []factVec + newest := map[string]int64{} // key → newest ts seen for it + for rows.Next() { + var id, metaJSON string + if err := rows.Scan(&id, &metaJSON); err != nil { + rows.Close() + return res, fmt.Errorf("repair fact vectors: row: %w", err) + } + meta := map[string]string{} + if err := json.Unmarshal([]byte(metaJSON), &meta); err != nil { + rows.Close() + return res, fmt.Errorf("repair fact vectors: meta for %q: %w", id, err) + } + if meta["type"] != "fact" { + continue + } + key, ts, ok := splitFactVectorID(id) + if !ok { + continue + } + vecs = append(vecs, factVec{id: id, key: key, meta: meta, ts: ts}) + if ts > newest[key] { + newest[key] = ts + } + } + rows.Close() + if err := rows.Err(); err != nil { + return res, fmt.Errorf("repair fact vectors: rows: %w", err) + } + + for _, v := range vecs { + drop := v.ts < newest[v.key] + var want string + if !drop { + f, ferr := s.LatestFact(ctx, v.key) + switch { + case errors.Is(ferr, ErrNoFact): + drop = true + case ferr != nil: + return res, fmt.Errorf("repair fact vectors: fact %q: %w", v.key, ferr) + case DecodeFactValue(f.Value) == "voided": + drop = true + default: + want = FactRecallText(v.key, f.Value) + } + } + if drop { + if err := s.VectorMemory().Delete(ctx, v.id); err != nil { + return res, err + } + res.Dropped++ + continue + } + if v.meta["text"] == want { + res.Kept++ + continue + } + vec, err := embed(ctx, want) + if err != nil { + return res, fmt.Errorf("repair fact vectors: embed %q: %w", v.id, err) + } + // The whole meta blob is rewritten in Go rather than patched in SQL, + // because json_set needs the JSON1 extension and this store is opened + // through sqlcipher. + v.meta["text"] = want + metaJSON, err := json.Marshal(v.meta) + if err != nil { + return res, fmt.Errorf("repair fact vectors: meta %q: %w", v.id, err) + } + if _, err := s.db.ExecContext(ctx, + `UPDATE memory_vectors SET vec = ?, meta = ? WHERE id = ?`, + encodeVec(vec), string(metaJSON), v.id); err != nil { + return res, fmt.Errorf("repair fact vectors: write %q: %w", v.id, err) + } + res.Rewritten++ + } + + if err := s.SetMeta(ctx, metaKeyFactVectorShape, factVectorShapeFact); err != nil { + return res, err + } + res.Took = time.Since(start) + return res, nil +} + +// splitFactVectorID reads the key and write time back out of a fact vector's +// id, which the write path builds as `fact::`. A key may hold a +// colon, the timestamp may not, so the split is from the right. +func splitFactVectorID(id string) (key string, ts int64, ok bool) { + rest, found := strings.CutPrefix(id, "fact:") + if !found { + return "", 0, false + } + cut := strings.LastIndex(rest, ":") + if cut <= 0 { + return "", 0, false + } + ts, err := strconv.ParseInt(rest[cut+1:], 10, 64) + if err != nil { + return "", 0, false + } + return rest[:cut], ts, true +} diff --git a/internal/store/factvectors_test.go b/internal/store/factvectors_test.go new file mode 100644 index 0000000..cb3f662 --- /dev/null +++ b/internal/store/factvectors_test.go @@ -0,0 +1,151 @@ +package store + +import ( + "context" + "database/sql" + "testing" + "time" +) + +// The write-path half of #493: recall of a fact must read back the fact, not +// the sentence he happened to say. +func TestFactRecallText(t *testing.T) { + for _, tc := range []struct { + name, key, value, want string + }{ + {"json value", "go_version", `"1.20"`, "go version — 1.20"}, + {"plain value", "water", "выпил", "water — выпил"}, + {"no value", "shower", "", "shower"}, + {"underscores are spoken as spaces", "espresso_machine", `"чистая"`, "espresso machine — чистая"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := FactRecallText(tc.key, tc.value); got != tc.want { + t.Fatalf("FactRecallText(%q, %q) = %q; want %q", tc.key, tc.value, got, tc.want) + } + }) + } +} + +// A correction left the superseded value in the index, so recall answered with +// the value he had just corrected (#493). +func TestCorrectValueDropsMemoryVectors(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + now := time.Now() + mem := s.VectorMemory() + + if _, err := s.WriteFact(ctx, now, KindSelf, "go_version", `"1.20"`, "tap:voice", 1.0, sql.NullInt64{}); err != nil { + t.Fatalf("WriteFact: %v", err) + } + if err := mem.Insert(ctx, "fact:go_version:1", []float32{1, 0, 0}, map[string]string{ + "type": "fact", "text": "go version — 1.20", + }); err != nil { + t.Fatalf("Insert: %v", err) + } + if _, err := s.CorrectValue(ctx, "go_version", "feedback", "1.25", now.Add(time.Minute)); err != nil { + t.Fatalf("CorrectValue: %v", err) + } + got, err := mem.ByPrefix(ctx, "fact:") + if err != nil { + t.Fatalf("ByPrefix: %v", err) + } + if len(got) != 0 { + t.Fatalf("after the correction the index still holds %+v; the superseded value must not answer", got) + } +} + +// The recovery path a poisoned box had none of (#470 point 4, #493): rows +// written before the fix hold utterances, voided junk and superseded values, +// and no write-path change reaches any of them. +func TestRepairFactVectors(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + now := time.Now() + mem := s.VectorMemory() + embed := func(ctx context.Context, text string) ([]float32, error) { + return []float32{float32(len(text)), 1, 0}, nil + } + + // A live fact indexed under the question that wrote it — the defect. + if _, err := s.WriteFact(ctx, now, KindSelf, "water", `"выпил"`, "tap:voice", 1.0, sql.NullInt64{}); err != nil { + t.Fatalf("WriteFact water: %v", err) + } + if err := mem.Insert(ctx, "fact:water:100", []float32{9, 9, 9}, map[string]string{ + "type": "fact", "source": "voice", "text": "запиши что я пил воду", + }); err != nil { + t.Fatalf("Insert water: %v", err) + } + // A voided fact whose vector survived the void. + if _, err := s.WriteFact(ctx, now, KindSelf, "go_version", `"1.20"`, "tap:voice", 1.0, sql.NullInt64{}); err != nil { + t.Fatalf("WriteFact go_version: %v", err) + } + if _, _, err := s.VoidLatestFact(ctx, "go_version", "feedback", now.Add(time.Minute)); err != nil { + t.Fatalf("VoidLatestFact: %v", err) + } + if err := mem.Insert(ctx, "fact:go_version:100", []float32{9, 9, 9}, map[string]string{ + "type": "fact", "text": "какая последняя версия языка Go?", + }); err != nil { + t.Fatalf("Insert go_version: %v", err) + } + // A key with two vectors: only the newest may answer. + if _, err := s.WriteFact(ctx, now, KindSelf, "mood", `"устал"`, "tap:voice", 1.0, sql.NullInt64{}); err != nil { + t.Fatalf("WriteFact mood: %v", err) + } + for _, ts := range []string{"100", "200"} { + if err := mem.Insert(ctx, "fact:mood:"+ts, []float32{9, 9, 9}, map[string]string{ + "type": "fact", "text": "мне грустно", + }); err != nil { + t.Fatalf("Insert mood %s: %v", ts, err) + } + } + // A note must be left entirely alone. + if err := mem.Insert(ctx, "note:7", []float32{5, 5, 5}, map[string]string{ + "type": "note", "text": "сеть тормозит по вечерам", + }); err != nil { + t.Fatalf("Insert note: %v", err) + } + + res, err := s.RepairFactVectors(ctx, embed) + if err != nil { + t.Fatalf("RepairFactVectors: %v", err) + } + if res.Rewritten != 2 || res.Dropped != 2 { + t.Fatalf("repair reported %+v; want 2 rewritten (water, newest mood) and 2 dropped (voided go_version, superseded mood)", res) + } + + got, err := mem.ByPrefix(ctx, "fact:") + if err != nil { + t.Fatalf("ByPrefix: %v", err) + } + texts := map[string]string{} + for _, r := range got { + texts[r.ID] = r.Meta["text"] + } + if len(texts) != 2 { + t.Fatalf("the index holds %+v; want only fact:water:100 and fact:mood:200", texts) + } + if texts["fact:water:100"] != "water — выпил" { + t.Fatalf("water reads back %q; want the fact, not the utterance", texts["fact:water:100"]) + } + if texts["fact:mood:200"] != "mood — устал" { + t.Fatalf("mood reads back %q", texts["fact:mood:200"]) + } + // Provenance the row already carried must survive the rewrite. + for _, r := range got { + if r.ID == "fact:water:100" && r.Meta["source"] != "voice" { + t.Fatalf("water lost its source meta: %+v", r.Meta) + } + } + if notes, err := mem.ByPrefix(ctx, "note:"); err != nil || len(notes) != 1 { + t.Fatalf("the note row was touched: %+v (err %v)", notes, err) + } + + // Marker written, so a second run is free and changes nothing. + again, err := s.RepairFactVectors(ctx, embed) + if err != nil { + t.Fatalf("second RepairFactVectors: %v", err) + } + if !again.Skipped { + t.Fatalf("second run did work: %+v; the marker must make it a no-op", again) + } +}