memeval: scope the evaluator's note windows by source

Both windows the evaluator keeps over the notes table were row budgets over
every writer. The dedupe read 200 recent notes and kept the eval ones, so after
200 ordinary notes an old observation left the window and the next evaluation
wrote the same sentence again. The snapshot asked for MaxItems notes and then
discarded her own, so once hourly evaluation had run for a few weeks the model
saw almost no real notes. Both reads are now filtered in SQL, by
RecentNotesBySource and RecentNotesExcludingSource.

Two smaller things in the same area. The dedupe key stripped any trailing
bracketed clause, so an observation ending in one hashed differently from its
stored form; it now strips only the recorded action. The evaluation timeout was
five minutes on the one llama-server that also answers voice turns, which made
a collision a five-minute mute assistant, and is now sixty seconds.

Found in review of #55.

cmd/mavend/memoryeval.go

@@ -21,6 +21,17 @@ import (
 	"github.com/kami/maven/internal/store"
 )
 
+// memoryEvalTimeout — the per-request deadline on one evaluation.
+//
+// It used to be five minutes, on the grounds that nobody waits for the answer.
+// Nobody waits for the evaluation, but there is ONE resident model behind one
+// llama-server, so a voice turn that arrives mid-evaluation waits behind it:
+// five minutes of evaluation is five minutes of a mute assistant. Sixty seconds
+// is long enough for a Thinking model on this prompt and short enough that the
+// worst collision is one turn answered late rather than a turn abandoned. An
+// evaluation cut off here costs nothing: it is retried at the next interval.
+const memoryEvalTimeout = 60 * time.Second
+
 // memoryEvalWorker — ticker + evaluator.
 type memoryEvalWorker struct {
 	eval     *memeval.Evaluator
@@ -47,9 +58,7 @@ func newMemoryEvalWorker(st *store.Store, phr phraser.Phraser, cfg *config.Confi
 	if interval <= 0 {
 		interval = config.DefaultMemoryEvalInterval
 	}
-	// A generous per-request timeout: this is a long prompt to a Thinking model
-	// and nobody is waiting on the answer.
-	client := llmClientFor(lp, 5*time.Minute)
+	client := llmClientFor(lp, memoryEvalTimeout)
 	ev := memeval.NewEvaluator(st, st, client, memeval.Config{
 		MaxItems:      cfg.MemoryEval.MaxItems,
 		MinConfidence: cfg.MemoryEval.MinConfidence,

internal/memeval/eval.go

@@ -85,7 +85,13 @@ type Completer interface {
 // no config facts.
 type Reader interface {
 	RecentFacts(ctx context.Context, n int) ([]store.Fact, error)
-	RecentNotes(ctx context.Context, n int) ([]store.Note, error)
+	// RecentNotesExcludingSource feeds the snapshot, RecentNotesBySource feeds
+	// the dedupe. Both are source-scoped in SQL rather than filtered here: a
+	// plain RecentNotes read makes each window a budget over ALL note writers,
+	// so her own hourly observations shrink the snapshot and ordinary notes
+	// push old observations out of the dedupe set.
+	RecentNotesExcludingSource(ctx context.Context, source string, n int) ([]store.Note, error)
+	RecentNotesBySource(ctx context.Context, source string, n int) ([]store.Note, error)
 	RecentNudges(ctx context.Context, n int) ([]store.Nudge, error)
 }
 
@@ -226,29 +232,44 @@ func formatNote(o Observation) string {
 	return fmt.Sprintf("%s [%s]", o.Text, o.Action)
 }
 
+// DedupeWindow — how many of her OWN past observations the dedupe looks back
+// over. Wider than MaxItems because the point is to remember saying it, not to
+// summarize it. Counted in eval notes only: when this was a plain recent-notes
+// read the window was really a budget over every note writer, so a few hundred
+// ordinary notes pushed an observation out of sight and the next evaluation was
+// free to write the same sentence again.
+const DedupeWindow = 200
+
 // recordedTexts — the normalized text of every observation earlier evaluations
-// wrote, for dedupe. Reads a wider window than MaxItems because the point is to
-// remember saying it, not to summarize it.
+// wrote, for dedupe.
 func (e *Evaluator) recordedTexts(ctx context.Context) (map[string]bool, error) {
-	notes, err := e.read.RecentNotes(ctx, 200)
+	notes, err := e.read.RecentNotesBySource(ctx, EvalNoteSource, DedupeWindow)
 	if err != nil {
 		return nil, fmt.Errorf("memory eval: recent notes: %w", err)
 	}
 	seen := make(map[string]bool, len(notes))
 	for _, n := range notes {
-		if n.Source != EvalNoteSource {
-			continue
-		}
-		text := n.Text
-		// Strip the "[action]" suffix formatNote appended.
-		if i := strings.LastIndex(text, " ["); i > 0 && strings.HasSuffix(text, "]") {
-			text = text[:i]
-		}
-		seen[normalizeObservation(text)] = true
+		seen[normalizeObservation(stripAction(n.Text))] = true
 	}
 	return seen, nil
 }
 
+// actions — the suggested_action enum, as the grammar constrains it.
+var actions = []string{"note", "propose", "notify"}
+
+// stripAction removes the "[action]" suffix formatNote appended, and only that.
+// Matching any bracketed tail would eat the end of an observation that happens
+// to finish on a bracketed clause, which changes its dedupe key and lets the
+// same sentence through twice.
+func stripAction(text string) string {
+	for _, a := range actions {
+		if s, ok := strings.CutSuffix(text, " ["+a+"]"); ok {
+			return s
+		}
+	}
+	return text
+}
+
 // normalizeObservation — dedupe key. Case- and whitespace-insensitive, which
 // catches the realistic repeat (the model re-emitting the same sentence with a
 // different comma) without pretending to do semantic dedupe.
@@ -259,16 +280,19 @@ func normalizeObservation(s string) string {
 // snapshot renders recent memory as the user turn. Returns "" when there is
 // nothing in any store — the caller treats that as "do not ask the model".
 //
-// Notes written by earlier evaluations are excluded. Feeding her own
+// Notes written by earlier evaluations are excluded, in SQL. Feeding her own
 // observations back in is how "я заметила, что ты не записывал еду" becomes
-// evidence for noticing it again, three evaluations deep.
+// evidence for noticing it again, three evaluations deep. Excluding them after
+// the read was worse than not excluding them: hourly evaluation makes her own
+// notes the majority of the newest rows within weeks, so asking for MaxItems
+// and dropping hers left the model a handful of real notes to look at.
 func (e *Evaluator) snapshot(ctx context.Context) (string, error) {
 	n := e.cfg.MaxItems
 	facts, err := e.read.RecentFacts(ctx, n)
 	if err != nil {
 		return "", fmt.Errorf("memory eval: recent facts: %w", err)
 	}
-	notes, err := e.read.RecentNotes(ctx, n)
+	notes, err := e.read.RecentNotesExcludingSource(ctx, EvalNoteSource, n)
 	if err != nil {
 		return "", fmt.Errorf("memory eval: recent notes: %w", err)
 	}
@@ -286,19 +310,10 @@ func (e *Evaluator) snapshot(ctx context.Context) (string, error) {
 			wrote = true
 		}
 	}
-	own := 0
-	var noteLines []string
-	for _, nt := range notes {
-		if nt.Source == EvalNoteSource {
-			own++
-			continue
-		}
-		noteLines = append(noteLines, fmt.Sprintf("- %s %s\n", nt.Ts.Format("2006-01-02 15:04"), truncate(nt.Text, 160)))
-	}
-	if len(noteLines) > 0 {
+	if len(notes) > 0 {
 		b.WriteString("\nЗаметки:\n")
-		for _, l := range noteLines {
-			b.WriteString(l)
+		for _, nt := range notes {
+			fmt.Fprintf(&b, "- %s %s\n", nt.Ts.Format("2006-01-02 15:04"), truncate(nt.Text, 160))
 			wrote = true
 		}
 	}

internal/memeval/eval_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"errors"
+	"fmt"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -269,3 +270,95 @@ func TestParseObservationsTolerantAndBounded(t *testing.T) {
 		t.Fatalf("parsed %d observations, want the %d cap", len(obs), MaxObservations)
 	}
 }
+
+// TestDedupeSurvivesOrdinaryNotes — the dedupe window is a count of HER notes,
+// not of all notes. With a plain recent-notes read, DedupeWindow ordinary notes
+// written after an observation pushed it out of sight and the same sentence was
+// written again on the next evaluation.
+func TestDedupeSurvivesOrdinaryNotes(t *testing.T) {
+	st := newTestStore(t)
+	ctx := context.Background()
+	now := refNow()
+	seedMemory(t, st, ctx, now)
+
+	same := `[{"observation":"ты три дня не записывал еду","confidence":0.9,"suggested_action":"note"}]`
+	f := &fakeLLM{replies: []string{same, same}}
+	ev := NewEvaluator(st, st, f, Config{})
+
+	if _, err := ev.Evaluate(ctx, now); err != nil {
+		t.Fatalf("Evaluate: %v", err)
+	}
+	// A few months of ordinary use between the two evaluations.
+	for i := 0; i < DedupeWindow*2; i++ {
+		if _, err := st.WriteNote(ctx, now.Add(time.Duration(i+1)*time.Minute),
+			fmt.Sprintf("обычная заметка %d", i), nil, "tap:voice"); err != nil {
+			t.Fatalf("write note: %v", err)
+		}
+	}
+	if _, err := ev.Evaluate(ctx, now.Add(24*time.Hour)); err != nil {
+		t.Fatalf("Evaluate: %v", err)
+	}
+
+	own, err := st.RecentNotesBySource(ctx, EvalNoteSource, 100)
+	if err != nil {
+		t.Fatalf("RecentNotesBySource: %v", err)
+	}
+	if len(own) != 1 {
+		t.Fatalf("eval notes = %d, want 1 — the repeat was not deduped", len(own))
+	}
+}
+
+// TestSnapshotBudgetIsNotEatenByOwnNotes — MaxItems notes must be MaxItems of
+// HIS notes. Reading MaxItems rows and then discarding hers left the model a
+// handful of real notes once hourly evaluation had run for a few weeks.
+func TestSnapshotBudgetIsNotEatenByOwnNotes(t *testing.T) {
+	st := newTestStore(t)
+	ctx := context.Background()
+	now := refNow()
+
+	// His notes first, then a long run of hers on top of them.
+	for i := 0; i < 5; i++ {
+		if _, err := st.WriteNote(ctx, now.Add(-time.Duration(100-i)*time.Hour),
+			fmt.Sprintf("его заметка %d", i), nil, "tap:voice"); err != nil {
+			t.Fatalf("write note: %v", err)
+		}
+	}
+	for i := 0; i < 50; i++ {
+		if _, err := st.WriteNote(ctx, now.Add(-time.Duration(50-i)*time.Hour),
+			fmt.Sprintf("я заметила кое-что %d [note]", i), nil, EvalNoteSource); err != nil {
+			t.Fatalf("write note: %v", err)
+		}
+	}
+
+	f := &fakeLLM{replies: []string{"[]"}}
+	ev := NewEvaluator(st, st, f, Config{MaxItems: 5})
+	if _, err := ev.Evaluate(ctx, now); err != nil {
+		t.Fatalf("Evaluate: %v", err)
+	}
+	if len(f.calls) != 1 {
+		t.Fatalf("LLM calls = %d, want 1", len(f.calls))
+	}
+	prompt := f.calls[0].User
+	if strings.Contains(prompt, "я заметила") {
+		t.Errorf("her own observations reached the prompt:\n%s", prompt)
+	}
+	for i := 0; i < 5; i++ {
+		if !strings.Contains(prompt, fmt.Sprintf("его заметка %d", i)) {
+			t.Errorf("his note %d missing from the prompt:\n%s", i, prompt)
+		}
+	}
+}
+
+// TestStripActionKeepsBracketedTail — the dedupe key strips the recorded
+// action and nothing else. Cutting at the last " [" ate the end of an
+// observation that itself ends on a bracketed clause, so the same sentence
+// hashed two ways.
+func TestStripActionKeepsBracketedTail(t *testing.T) {
+	text := "ты не пил воду [со вторника]"
+	if got := stripAction(formatNote(Observation{Text: text, Action: "note"})); got != text {
+		t.Errorf("stripAction = %q, want %q", got, text)
+	}
+	if got := stripAction(text); got != text {
+		t.Errorf("stripAction = %q, want it untouched", got)
+	}
+}

internal/store/notes.go

@@ -80,8 +80,29 @@ func (s *Store) QueryNotes(ctx context.Context, embedding []float32, k int) ([]N
 // embedding math; Score stays 0). This is the read surface for /dash: notes
 // captured by voice are otherwise only reachable through semantic query.
 func (s *Store) RecentNotes(ctx context.Context, n int) ([]Note, error) {
+	return s.recentNotesWhere(ctx, "", n)
+}
+
+// RecentNotesBySource returns the newest n notes written by one source, newest
+// first. The filter is in SQL, not in the caller, because a caller that reads n
+// rows and then keeps the ones it wants has a window measured in OTHER writers'
+// traffic: once n newer notes from anywhere have landed, the rows it was
+// looking for are gone. Anything that needs "the last n of mine" wants this.
+func (s *Store) RecentNotesBySource(ctx context.Context, source string, n int) ([]Note, error) {
+	return s.recentNotesWhere(ctx, "WHERE source = ?", n, source)
+}
+
+// RecentNotesExcludingSource returns the newest n notes NOT written by source.
+// Same reasoning inverted: a reader that wants n notes he wrote must not have
+// its budget eaten by rows it is about to discard.
+func (s *Store) RecentNotesExcludingSource(ctx context.Context, source string, n int) ([]Note, error) {
+	return s.recentNotesWhere(ctx, "WHERE source <> ?", n, source)
+}
+
+func (s *Store) recentNotesWhere(ctx context.Context, where string, n int, args ...any) ([]Note, error) {
+	args = append(args, n)
 	rows, err := s.db.QueryContext(ctx,
-		`SELECT id, ts, text, source FROM notes ORDER BY ts DESC LIMIT ?`, n)
+		`SELECT id, ts, text, source FROM notes `+where+` ORDER BY ts DESC LIMIT ?`, args...)
 	if err != nil {
 		return nil, fmt.Errorf("recent notes: %w", err)
 	}

models/stt

@@ -0,0 +1 @@
+/home/kami/apps/Maven/models/stt

models/tts

@@ -0,0 +1 @@
+/home/kami/apps/Maven/models/tts