diff --git a/RECALL-EVAL-31-07-2026.md b/RECALL-EVAL-31-07-2026.md index 39fb389..ead570c 100644 --- a/RECALL-EVAL-31-07-2026.md +++ b/RECALL-EVAL-31-07-2026.md @@ -75,6 +75,14 @@ same vector. A note is indexed in both places with the same embedding, so if it `QueryNotes` it fails again here — the branch can only ever return a **fact**. Its comment calls it "additive"; for notes it is not. +**Fixed (Vikunja #373).** The memory pass now runs *first*, as one search over notes and facts with +one gate, so whichever memory is clearly the best match answers — note or fact. The notes-only pass +stays behind it for notes the vector index does not hold. No threshold changed, so the set of +questions Maven answers is the same; only which memory answers them. The fixture gained two mixed +note+fact cases (`ru-mixed-031`, `ru-mixed-032`), which is why the counts below are out of 27 +answerable cases and not 25: hash recall@1 36.0% (9/25) → 37.0% (10/27), e5 recall@1 72.0% (18/25) → +70.4% (19/27) with answered-after-gate 68.0% → 66.7% and false recall unchanged at 1/5. + ### 5. Ranking has no recency or type signal, and the store is not the bottleneck `internal/store/notes.go:67` sorts by cosine and uses `ts` only to break an exact float tie, which diff --git a/cmd/mavend/query_recall_test.go b/cmd/mavend/query_recall_test.go new file mode 100644 index 0000000..b97c436 --- /dev/null +++ b/cmd/mavend/query_recall_test.go @@ -0,0 +1,158 @@ +package main + +import ( + "context" + "fmt" + "math" + "testing" + "time" + + "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/voice" +) + +// fixedEmbedder hands back a vector chosen per text, so a test can say exactly +// how close each stored memory is to the question. The real embedders make +// scores that are realistic but not controllable, and this test is about the +// gate, not about the embedder. +type fixedEmbedder struct{ vecs map[string][]float32 } + +func (f *fixedEmbedder) Dim() int { return 4 } +func (f *fixedEmbedder) Close() error { return nil } + +func (f *fixedEmbedder) Embed(_ context.Context, text string) ([]float32, error) { + v, ok := f.vecs[text] + if !ok { + return nil, fmt.Errorf("fixedEmbedder: no vector for %q", text) + } + return v, nil +} + +// scoreVec builds a unit vector whose cosine against the query vector +// (1,0,0,0) is exactly score. +func scoreVec(score float64) []float32 { + rest := math.Sqrt(1 - score*score) + return []float32{float32(score), float32(rest), 0, 0} +} + +// recordingPhraser remembers what the query path handed it to phrase, which is +// how the test can tell which pass produced the answer. +type recordingPhraser struct { + *phraser.Stub + notes []string +} + +func (r *recordingPhraser) PhraseQuery(ctx context.Context, utterance string, notes []string) (string, error) { + r.notes = notes + return r.Stub.PhraseQuery(ctx, utterance, notes) +} + +// recallCase — one stored memory: its text, how close it is to the question, +// whether it is a note or a fact, and whether the notes table holds it too. +type recallCase struct { + text string + score float64 + kind string +} + +// buildRecallHandler stores the given memories and returns a handler whose +// query path can be run directly. Notes go into BOTH the notes table and the +// vector index, which is what the daemon does (voice.go's IntentNote). +func buildRecallHandler(t *testing.T, question string, mems []recallCase) (*reactiveHandler, *recordingPhraser) { + t.Helper() + ctx := context.Background() + st := newTestStore(t) + emb := &fixedEmbedder{vecs: map[string][]float32{question: {1, 0, 0, 0}}} + mem := memory.NewInMemoryStore() + now := time.Now() + + for i, m := range mems { + vec := scoreVec(m.score) + emb.vecs[m.text] = vec + id := fmt.Sprintf("%s:%d", m.kind, i) + if m.kind == "note" { + if _, err := st.WriteNote(ctx, now, m.text, vec, "tap:voice"); err != nil { + t.Fatalf("WriteNote: %v", err) + } + } + if err := mem.Insert(ctx, id, vec, map[string]string{"text": m.text, "type": m.kind}); err != nil { + t.Fatalf("memory insert: %v", err) + } + } + + phr := &recordingPhraser{Stub: phraser.NewStub()} + h := &reactiveHandler{ + api: ipc.NewStoreAPI(st), + embedder: emb, + replier: voice.NewStubReplier(), + phraser: phr, + now: func() time.Time { return now }, + memStore: mem, + dataStore: st, + queryMinScore: 0.55, + queryMinMargin: 0.008, + weatherProvider: nil, + } + return h, phr +} + +func askQuery(t *testing.T, h *reactiveHandler, question string) string { + t.Helper() + return h.applyAction(context.Background(), router.Decision{ + Intent: router.IntentQuery, + Utterance: question, + }) +} + +// TestQueryRecallNoteCanWin — the note-recall regression (Vikunja #373). Notes +// and facts share one vector index, and a note that clearly beats everything +// else must be the answer. Before the fix the memory pass only ran after the +// notes-only gate had already rejected the same note at the same score, so only +// a fact could ever come back from it. +func TestQueryRecallNoteCanWin(t *testing.T) { + const q = "где молоко" + + t.Run("a clearly best note answers", func(t *testing.T) { + h, phr := buildRecallHandler(t, q, []recallCase{ + {text: "молоко стоит в холодильнике", score: 0.90, kind: "note"}, + {text: "выучил пару аккордов", score: 0.50, kind: "note"}, + }) + reply := askQuery(t, h, q) + if want := "вот что я нашла: молоко стоит в холодильнике"; reply != want { + t.Errorf("reply %q, want %q", reply, want) + } + // One text, the winning memory's — the answer came from the memory + // pass, not from handing the phraser every note in the table. + if len(phr.notes) != 1 || phr.notes[0] != "молоко стоит в холодильнике" { + t.Errorf("phraser got %q, want just the recalled note", phr.notes) + } + }) + + // The other half of "one gate over everything": a fact that matches better + // than the best note now answers, instead of losing to a note that only had + // to beat other notes. + t.Run("the better-matching fact answers", func(t *testing.T) { + h, _ := buildRecallHandler(t, q, []recallCase{ + {text: "молоко стоит в холодильнике", score: 0.80, kind: "note"}, + {text: "купил молоко в среду", score: 0.95, kind: "fact"}, + }) + if reply := askQuery(t, h, q); reply != "купил молоко в среду" { + t.Errorf("reply %q, want the fact read back", reply) + } + }) + + // The gate is untouched: two memories this close mean the embedder cannot + // tell them apart, and silence still beats a coin flip. + t.Run("no clear best stays silent", func(t *testing.T) { + h, _ := buildRecallHandler(t, q, []recallCase{ + {text: "молоко стоит в холодильнике", score: 0.860, kind: "note"}, + {text: "молоко закончилось", score: 0.858, kind: "note"}, + }) + if reply := askQuery(t, h, q); reply != "не знаю." { + t.Errorf("reply %q, want silence", reply) + } + }) +} diff --git a/cmd/mavend/recall.go b/cmd/mavend/recall.go index 1073989..d06aeb3 100644 --- a/cmd/mavend/recall.go +++ b/cmd/mavend/recall.go @@ -2,21 +2,24 @@ package main import "github.com/kami/maven/internal/memory" -// bestRecall is the read side of the long-term memory store: the top hit's -// stored text when it clears the confidence gate. This recalls across BOTH -// notes and facts (facts aren't in the notes table, so this is the only path -// that can answer "when did I last …?" from a captured fact). A note hit here -// is redundant with the notes-RAG path — by design; the two indexes can diverge -// once the backend is swapped for a persistent/external store. ok=false when -// the hit fails the confidence gate (see memory.Confident: an absolute floor -// plus a margin over the runner-up) or carries no text. -func bestRecall(results []memory.Result, minScore, minMargin float64) (string, bool) { +// bestRecall is the read side of the long-term memory store: the top hit when +// it clears the confidence gate. The index holds BOTH notes and facts, and +// either can win — the caller looks at the returned hit's meta["type"] to see +// which. Facts aren't in the notes table, so this is the only path that can +// answer "when did I last …?" from a captured fact. +// +// The whole hit is returned, not just its text, because "which memory answered" +// decides how the answer is said: a note gets phrased in Maven's voice, a fact +// is read back as stored. +// +// ok=false when the hit fails the confidence gate (see memory.Confident: an +// absolute floor plus a margin over the runner-up) or carries no text. +func bestRecall(results []memory.Result, minScore, minMargin float64) (memory.Result, bool) { if !memory.Confident(results, minScore, minMargin) { - return "", false + return memory.Result{}, false } - text := results[0].Meta["text"] - if text == "" { - return "", false + if results[0].Meta["text"] == "" { + return memory.Result{}, false } - return text, true + return results[0], true } diff --git a/cmd/mavend/recall_test.go b/cmd/mavend/recall_test.go index 049c79b..1a70b2f 100644 --- a/cmd/mavend/recall_test.go +++ b/cmd/mavend/recall_test.go @@ -39,8 +39,27 @@ func TestBestRecall(t *testing.T) { if !ok { t.Fatal("clearing hit not returned") } - if got != "выпил воды в три часа" { - t.Errorf("wrong text: %q", got) + if got.Meta["text"] != "выпил воды в три часа" { + t.Errorf("wrong text: %q", got.Meta["text"]) + } + if got.Meta["type"] != "fact" { + t.Errorf("kind lost: %q", got.Meta["type"]) + } + }) + + // The index holds notes and facts together, so a note has to be able to win + // it — for a long time it could not (Vikunja #373). + t.Run("a note can win", func(t *testing.T) { + res := []memory.Result{ + {Score: 0.86, Meta: map[string]string{"text": "молоко в холодильнике", "type": "note"}}, + {Score: 0.61, Meta: map[string]string{"text": "выпил воды", "type": "fact"}}, + } + got, ok := bestRecall(res, min, margin) + if !ok { + t.Fatal("clearly-best note not returned") + } + if got.Meta["type"] != "note" || got.Meta["text"] != "молоко в холодильнике" { + t.Errorf("got %v, want the note", got.Meta) } }) diff --git a/cmd/mavend/voice.go b/cmd/mavend/voice.go index e5af719..0df260a 100644 --- a/cmd/mavend/voice.go +++ b/cmd/mavend/voice.go @@ -786,11 +786,43 @@ func (h *reactiveHandler) applyAction(ctx context.Context, dec router.Decision) log.Printf("voice: embed query: %v", err) return "не получилось найти ответ." } + // Long-term memory first: ONE search over everything Maven remembers + // (notes and facts share this index) and ONE confidence gate, so the + // memory that is clearly the best match answers — a note just as much + // as a fact. + // + // This used to run only after the notes-only gate below had already + // rejected the same note at the same score, which no note could ever + // survive a second time: the branch could only return a fact (#373). + // Order, not the gate, was the bug — the set of questions Maven answers + // is unchanged, only which memory gets to answer them. + if h.memStore != nil { + if hits, herr := h.memStore.Search(ctx, vec, 3); herr == nil { + if hit, ok := bestRecall(hits, h.queryMinScore, h.queryMinMargin); ok { + text := hit.Meta["text"] + // A note is phrased in Maven's voice; a fact is read back + // as it was stored. + if hit.Meta["type"] == "note" { + if reply, perr := h.phraser.PhraseQuery(ctx, dec.Utterance, []string{text}); perr == nil && reply != "" { + return reply + } + } + return text + } + } else { + log.Printf("voice: memory search: %v", herr) + } + } + notes, err := h.api.QueryNotes(ctx, vec, 5) if err != nil { log.Printf("voice: query notes: %v", err) return "не получилось найти ответ." } + // Notes-only pass, for notes the vector index above does not hold (an + // older note written before it existed). Same gate, notes-only + // candidates. + // // Confidence gate: below it, say "I don't know" rather than read back // the least-unrelated note — a confident wrong recall is worse than a // gap (spec's "not a guesser-of-truth"). Same instinct as the loop's @@ -802,16 +834,6 @@ func (h *reactiveHandler) applyAction(ctx context.Context, dec router.Decision) noteScores[i] = n.Score } if !memory.ConfidentScores(noteScores, h.queryMinScore, h.queryMinMargin) { - // Long-term memory recall (notes + facts) before general knowledge: - // the notes table can't answer fact questions, but the memory store - // indexes both. Only runs when notes-RAG already gave up → additive. - if h.memStore != nil { - if hits, herr := h.memStore.Search(ctx, vec, 3); herr == nil { - if text, ok := bestRecall(hits, h.queryMinScore, h.queryMinMargin); ok { - return text - } - } - } // Try general knowledge from the phraser before giving up reply, err := h.phraser.PhraseQuery(ctx, dec.Utterance, nil) if err != nil || reply == "" { diff --git a/internal/memory/recalleval/recalleval.go b/internal/memory/recalleval/recalleval.go index a9336b6..fe310e1 100644 --- a/internal/memory/recalleval/recalleval.go +++ b/internal/memory/recalleval/recalleval.go @@ -422,6 +422,8 @@ func rankNote(inTop3 bool) string { // bestRecall mirrors cmd/mavend/recall.go — the gate the daemon actually // applies to a memory hit. Duplicated rather than imported because package main // is not importable; recalleval_test.go asserts the two agree in behaviour. +// The daemon returns the whole hit (a note and a fact are said differently); +// the harness only scores what came back, so it keeps returning the text. func bestRecall(results []memory.Result, minScore, minMargin float64) string { if !memory.Confident(results, minScore, minMargin) { return "" diff --git a/internal/memory/recalleval/recalleval_test.go b/internal/memory/recalleval/recalleval_test.go index 5ff3f9d..95e7449 100644 --- a/internal/memory/recalleval/recalleval_test.go +++ b/internal/memory/recalleval/recalleval_test.go @@ -197,7 +197,8 @@ func TestHashRecallBaseline(t *testing.T) { t.Log("\n" + rep.String() + rep.Failures()) t.Log("\ngate sweep:\n" + sweep(t, router.NewHashEmbedder(hashDim), f)) - // 0.32 sits under the observed 0.360 recall@1. + // 0.32 sits under the observed 0.370 recall@1 (was 0.360 over 25 answerable + // cases; the two mixed note+fact cases added with #373 make it 27). const floorRecall1 = 0.32 if rep.Recall1() < floorRecall1 { t.Errorf("recall@1 %.3f below ratchet %.2f — note recall regressed", rep.Recall1(), floorRecall1) diff --git a/internal/memory/recalleval/ru_recall_v1.json b/internal/memory/recalleval/ru_recall_v1.json index fd7c7f4..7659680 100644 --- a/internal/memory/recalleval/ru_recall_v1.json +++ b/internal/memory/recalleval/ru_recall_v1.json @@ -387,6 +387,32 @@ {"id": "n2", "text": "wifi channel is 6", "kind": "note"}, {"id": "n3", "text": "the guest network is off", "kind": "note"} ] + }, + { + "id": "ru-mixed-031", + "lang": "ru", + "tags": ["mixed", "paraphrase", "hard"], + "note": "notes and facts in one store and the note is the answer — the daemon indexes both (Vikunja #373)", + "query": "куда я спрятал второй ключ от квартиры", + "want": "n1", + "notes": [ + {"id": "n1", "text": "запасной ключ от квартиры лежит в синей коробке на полке", "kind": "note"}, + {"id": "x1", "text": "поменял замок в двери двадцатого июня", "kind": "fact"}, + {"id": "x2", "text": "отдал ключ соседке в мае", "kind": "fact"} + ] + }, + { + "id": "ru-mixed-032", + "lang": "ru", + "tags": ["mixed", "distractor"], + "note": "the mirror of ru-mixed-031: the fact answers and the notes are the distractors", + "query": "когда я в последний раз заливал бензин", + "want": "x1", + "notes": [ + {"id": "x1", "text": "залил полный бак в четверг вечером", "kind": "fact"}, + {"id": "n1", "text": "на заправке у моста дешевле бензин", "kind": "note"}, + {"id": "n2", "text": "надо поменять зимние шины", "kind": "note"} + ] } ] }