From 67563ed1f6d55667c2ba4f87f02f5ba267119e99 Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 31 Jul 2026 23:07:33 +0400 Subject: [PATCH] Run pattern detection from the digestion tick, not just voice (#43) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit detectPattern only ever fired as a side effect of a voice fact-write, so a recurring pattern already sitting in history went unnoticed until he happened to mention it again by voice — the opposite of proactive. Split the pipeline: extraction (fact -> normalized event) stays where a fact is written, in voice.go, since it's tied to that write regardless of who's talking. Detection (events -> stable pattern -> proposed_routines row) moves into shared code (patterns.go's detectAndPropose) that both the voice path and the new tick.go:detectPatterns call. The tick runs it every cycle over every action+object pair on record (store.DistinctEventPairs, added), so a pattern gets noticed on the daemon's own schedule. Idempotence and the dismiss-must-stick requirement turned out to already be handled by the store, not something the tick needs to reinvent: proposed_routines has UNIQUE(action, object) and CreateProposedRoutine does ON CONFLICT DO NOTHING, and DismissProposedRoutine flips status in place without deleting the row. So a pair already proposed, accepted, OR dismissed is a silent no-op on every later tick — a dismissed pattern can never resurface, and re-running the scan never spams the /routines page. Kept the voice-path call (immediate spoken confirmation is a nice feature UX-wise and is now redundant-but-harmless with the tick, since both paths share the same guarded detectAndPropose). Tick-side detection only ever writes a row; it does not notify, ring, or speak, keeping Maven "not a nag, not autonomous" — the /routines page is still the only place a proposal becomes visible, and only accepting it starts producing nudges (fireAcceptedRoutines). Also fixed the stale vikunja#46 reference in proposed_routines.go — the TODO it named is what this commit does. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ --- cmd/mavend/patterns.go | 74 +++++++++++++++++ cmd/mavend/patterns_test.go | 122 ++++++++++++++++++++++++++++ cmd/mavend/tick.go | 51 ++++++++++++ cmd/mavend/voice.go | 41 ++-------- internal/store/events.go | 30 +++++++ internal/store/proposed_routines.go | 7 +- 6 files changed, 290 insertions(+), 35 deletions(-) create mode 100644 cmd/mavend/patterns.go create mode 100644 cmd/mavend/patterns_test.go diff --git a/cmd/mavend/patterns.go b/cmd/mavend/patterns.go new file mode 100644 index 0000000..ab90fed --- /dev/null +++ b/cmd/mavend/patterns.go @@ -0,0 +1,74 @@ +// mavend/patterns.go — the shared detect+propose step of pattern inference +// (Vikunja #43). Event *extraction* (fact -> action/object) happens at fact- +// write time in voice.go's detectPattern, tied to whichever channel wrote the +// fact. Detection — turning a run of events into a proposed routine — is +// channel-agnostic: it only needs what's already in the events table, so it +// runs both right after a voice fact-write (for the immediate "напоминать?" +// confirmation) and, proactively, from the digestion tick (tick.go's +// detectPatterns) over every action+object pair on record, not just the one +// that was just talked about. +package main + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/kami/maven/internal/pattern" + "github.com/kami/maven/internal/store" +) + +// detectAndPropose runs the pattern detector over every recorded event for +// action+object and, if a stable pattern is found and nothing has been +// proposed/accepted/dismissed for this pair yet, creates a proposed_routines +// row. Returns (nil, 0, nil) — not an error — whenever there is nothing new +// to report: too few events, irregular intervals, or a pair that already has +// a row in any status. That last case is the one that matters most: it is +// how a routine the owner already DISMISSED stays dismissed forever, because +// the row survives dismissal (status flips in place, see +// store.DismissProposedRoutine) and both the Lookup check here and the +// table's UNIQUE(action, object) constraint refuse to create a second one. +func detectAndPropose(ctx context.Context, ds *store.Store, action, object string, ts time.Time) (*pattern.ProposedRoutine, int64, error) { + events, err := ds.EventsFor(ctx, action, object) + if err != nil { + return nil, 0, fmt.Errorf("events for %s/%s: %w", action, object, err) + } + patEvents := make([]pattern.Event, len(events)) + for i, e := range events { + patEvents[i] = pattern.Event{ + FactID: e.FactID, + Action: e.Action, + Object: e.Object, + Ts: e.Ts, + } + } + r, err := pattern.Detect(patEvents) + if err != nil { + return nil, 0, fmt.Errorf("detect %s/%s: %w", action, object, err) + } + if r == nil { + return nil, 0, nil // not enough data or intervals too irregular + } + + // Belt: check first so the common "nothing new" case never even attempts + // an insert. Suspenders: CreateProposedRoutine's ON CONFLICT DO NOTHING + // (backed by the UNIQUE(action,object) constraint) is the actual + // guarantee — this Lookup is an optimization, not the source of truth. + existing, err := ds.LookupProposedRoutine(ctx, r.Action, r.Object) + if err != nil { + return nil, 0, fmt.Errorf("lookup proposed routine %s/%s: %w", action, object, err) + } + if existing != nil { + return nil, 0, nil // already proposed, accepted, or dismissed — say nothing + } + + id, err := ds.CreateProposedRoutine(ctx, r.Action, r.Object, r.IntervalDays, ts) + if err != nil { + if errors.Is(err, store.ErrProposedRoutineExists) { + return nil, 0, nil // lost a race with another caller — not an error + } + return nil, 0, fmt.Errorf("create proposed routine %s/%s: %w", action, object, err) + } + return r, id, nil +} diff --git a/cmd/mavend/patterns_test.go b/cmd/mavend/patterns_test.go new file mode 100644 index 0000000..db2faf5 --- /dev/null +++ b/cmd/mavend/patterns_test.go @@ -0,0 +1,122 @@ +package main + +import ( + "context" + "database/sql" + "testing" + "time" + + "github.com/kami/maven/internal/store" +) + +// seedRefillEvents writes N weekly "refill/cat_water" events straight to the +// events table — this is what the tick reads, independent of any utterance. +func seedRefillEvents(t *testing.T, st *store.Store, ctx context.Context, base time.Time, n int) { + t.Helper() + for i := 0; i < n; i++ { + factID, err := st.WriteFact(ctx, base.Add(time.Duration(i)*7*24*time.Hour), store.KindSelf, + "cat_water", "refill", "test", 1.0, sql.NullInt64{}) + if err != nil { + t.Fatalf("write fact %d: %v", i, err) + } + if _, err := st.CreateEvent(ctx, factID, "refill", "cat_water", base.Add(time.Duration(i)*7*24*time.Hour)); err != nil { + t.Fatalf("create event %d: %v", i, err) + } + } +} + +// TestTickDetectsPatternFromStoredEvents proves the tick notices a pattern on +// its own, reading straight from the store — not as a side effect of a live +// utterance (Vikunja #43). Three weekly events with no voice turn in sight +// must produce exactly one proposed routine. +func TestTickDetectsPatternFromStoredEvents(t *testing.T) { + st := newTestStore(t) + ctx := context.Background() + now := refNow() + seedRefillEvents(t, st, ctx, now, 3) + + tl := newTestTickLoop(t, st, &fakeSink{}, nil) + tl.detectPatterns(ctx, now) + + rows, err := st.ListProposedRoutines(ctx) + if err != nil { + t.Fatalf("list proposed routines: %v", err) + } + if len(rows) != 1 { + t.Fatalf("proposed routines = %d, want 1: %+v", len(rows), rows) + } + if rows[0].Action != "refill" || rows[0].Object != "cat_water" { + t.Errorf("proposed routine = %s/%s, want refill/cat_water", rows[0].Action, rows[0].Object) + } +} + +// TestTickPatternDetectionIsIdempotent proves running the tick's pattern scan +// twice does not spam a second proposal for the same pair, and that the store +// itself is what stops the duplicate (not tick-local state) — the whole point +// of the guard, since the tick has no memory of what it proposed last time. +func TestTickPatternDetectionIsIdempotent(t *testing.T) { + st := newTestStore(t) + ctx := context.Background() + now := refNow() + seedRefillEvents(t, st, ctx, now, 3) + + tl := newTestTickLoop(t, st, &fakeSink{}, nil) + tl.detectPatterns(ctx, now) + tl.detectPatterns(ctx, now.Add(time.Hour)) + + rows, err := st.ListProposedRoutines(ctx) + if err != nil { + t.Fatalf("list proposed routines: %v", err) + } + if len(rows) != 1 { + t.Fatalf("proposed routines after two ticks = %d, want 1 (no duplicate): %+v", len(rows), rows) + } +} + +// TestTickPatternDetectionRespectsDismissal proves the single worst failure +// mode here — a proposal the owner already said no to coming back on the next +// tick — cannot happen. Dismissal flips the row's status in place; it must +// still be there to block re-proposal. +func TestTickPatternDetectionRespectsDismissal(t *testing.T) { + st := newTestStore(t) + ctx := context.Background() + now := refNow() + seedRefillEvents(t, st, ctx, now, 3) + + tl := newTestTickLoop(t, st, &fakeSink{}, nil) + tl.detectPatterns(ctx, now) + + rows, err := st.ListProposedRoutines(ctx) + if err != nil { + t.Fatalf("list proposed routines: %v", err) + } + if len(rows) != 1 { + t.Fatalf("setup: proposed routines = %d, want 1", len(rows)) + } + if err := st.DismissProposedRoutine(ctx, rows[0].ID); err != nil { + t.Fatalf("dismiss: %v", err) + } + + // More events for the same pair arrive, and the tick runs again — a + // dismissed pattern must not resurface. + seedRefillEvents(t, st, ctx, now.Add(30*24*time.Hour), 3) + tl.detectPatterns(ctx, now.Add(60*24*time.Hour)) + + proposed, err := st.ListProposedRoutinesByStatus(ctx, store.RoutineProposed) + if err != nil { + t.Fatalf("list proposed: %v", err) + } + if len(proposed) != 0 { + t.Fatalf("a dismissed pattern came back: %+v", proposed) + } + all, err := st.ListProposedRoutinesByStatus(ctx, "") + if err != nil { + t.Fatalf("list all: %v", err) + } + if len(all) != 1 { + t.Fatalf("total rows for the pair = %d, want 1 (still dismissed, not duplicated): %+v", len(all), all) + } + if all[0].Status != store.RoutineDismissed { + t.Errorf("status = %s, want dismissed", all[0].Status) + } +} diff --git a/cmd/mavend/tick.go b/cmd/mavend/tick.go index 39d1c83..e9b5c1e 100644 --- a/cmd/mavend/tick.go +++ b/cmd/mavend/tick.go @@ -195,6 +195,14 @@ func (t *tickLoop) tick(ctx context.Context, now time.Time) { // nudge time. See internal/morning for the "why not four timers" rationale. t.fireMorningRoutines(ctx, now, state) + // pattern detection: scan every action+object pair with recorded events + // and propose a routine for any stable one not already decided (Vikunja + // #43). This used to only run as a side effect of the voice fact-write + // path, so a pattern already sitting in history went unnoticed until he + // happened to mention it again by voice. See patterns.go and + // detectPatterns below for how idempotence and dismissal are respected. + t.detectPatterns(ctx, now) + // reminders: gate-bypassing class. fired once, marked after a successful // delivery. a failed send leaves the reminder pending — the next tick // re-gathers and re-attempts. @@ -342,6 +350,49 @@ func (t *tickLoop) flushDigest(ctx context.Context, now time.Time, state loop.St t.digestQ = nil } +// detectPatterns runs the pattern detector proactively over every +// action+object pair that has ever produced an event, independent of +// whichever fact write (or channel) last touched it (Vikunja #43). This is +// what makes pattern inference actually proactive: it fires on the daemon's +// own schedule reading accumulated history, not only as a side effect of a +// live voice turn. +// +// Idempotence and noise are handled by the store, not here — this function +// is safe to call every tick: +// - Same pattern, tick after tick: detectAndPropose's LookupProposedRoutine +// check plus proposed_routines' UNIQUE(action, object) constraint (with +// CreateProposedRoutine's ON CONFLICT DO NOTHING) mean a pair that +// already has a row — in ANY status — produces no second row and no log +// spam beyond the one line at genuine creation. +// - A DISMISSED proposal must never come back. DismissProposedRoutine flips +// status in place; the row is never deleted. So the same Lookup check +// that stops a duplicate "proposed" also stops a "dismissed" one from +// resurrecting — there is nothing tick-specific to get right here beyond +// calling the same shared path the voice route already used. +// +// This only ever creates a row for the /routines page to show. It does not +// notify, ring, or speak — Maven is "not a nag, not autonomous" (CLAUDE.md), +// and detection is not the same act as disturbing him about it. A proposal +// only starts producing nudges once he accepts it (fireAcceptedRoutines). +func (t *tickLoop) detectPatterns(ctx context.Context, now time.Time) { + pairs, err := t.store.DistinctEventPairs(ctx) + if err != nil { + log.Printf("tick: distinct event pairs: %v", err) + return + } + for _, p := range pairs { + r, _, err := detectAndPropose(ctx, t.store, p.Action, p.Object, now) + if err != nil { + log.Printf("tick: detect pattern %s/%s: %v", p.Action, p.Object, err) + continue + } + if r == nil { + continue // no stable pattern, or already proposed/accepted/dismissed + } + log.Printf("tick: proposed routine: %s/%s every %.1f days", r.Action, r.Object, r.IntervalDays) + } +} + // routinesFromConfig maps the config's routine blocks to the engine type. // Validation (cron parses, name/body present, severity defaulted) already ran // in config.Load, so this is a pure field copy. diff --git a/cmd/mavend/voice.go b/cmd/mavend/voice.go index af5feb0..8e59acc 100644 --- a/cmd/mavend/voice.go +++ b/cmd/mavend/voice.go @@ -874,42 +874,19 @@ func (h *reactiveHandler) detectPattern(ctx context.Context, factID int64, key, log.Printf("voice: create event: %v", err) return "" } - events, err := h.dataStore.EventsFor(ctx, ev.Action, ev.Object) + // Detect+propose (Vikunja #43) is shared with the digestion tick's + // proactive scan — see patterns.go. Event *extraction* above stays here, + // tied to this fact write; detection over the accumulated history does + // not need to happen right now for the voice path to have already done + // its job — it's dedupe-safe to also let the next tick find the same + // pattern independently. + r, id, err := detectAndPropose(ctx, h.dataStore, ev.Action, ev.Object, ts) if err != nil { - log.Printf("voice: events for %s/%s: %v", ev.Action, ev.Object, err) - return "" - } - // Convert store.Events to pattern.Events for the detector. - patEvents := make([]pattern.Event, len(events)) - for i, e := range events { - patEvents[i] = pattern.Event{ - FactID: e.FactID, - Action: e.Action, - Object: e.Object, - Ts: e.Ts, - } - } - r, err := pattern.Detect(patEvents) - if err != nil { - log.Printf("voice: pattern detect: %v", err) + log.Printf("voice: detect pattern %s/%s: %v", ev.Action, ev.Object, err) return "" } if r == nil { - return "" // not enough data or intervals too irregular - } - // Check if already proposed/accepted/dismissed for this pair. - existing, err := h.dataStore.LookupProposedRoutine(ctx, r.Action, r.Object) - if err != nil { - log.Printf("voice: lookup proposed routine: %v", err) - return "" - } - if existing != nil { - return "" // already proposed, accepted, or dismissed - } - id, err := h.dataStore.CreateProposedRoutine(ctx, r.Action, r.Object, r.IntervalDays, ts) - if err != nil { - log.Printf("voice: create proposed routine: %v", err) - return "" + return "" // not enough data, too irregular, or already proposed/decided } log.Printf("voice: proposed routine: %s/%s every %.1f days", r.Action, r.Object, r.IntervalDays) diff --git a/internal/store/events.go b/internal/store/events.go index e3dd66a..f40d4e1 100644 --- a/internal/store/events.go +++ b/internal/store/events.go @@ -35,6 +35,36 @@ func (s *Store) CreateEvent(ctx context.Context, factID int64, action, object st return id, nil } +// EventPair identifies one action+object grouping in the events table — the +// unit the pattern detector reasons about. +type EventPair struct { + Action string + Object string +} + +// DistinctEventPairs returns every distinct action+object pair that has at +// least one event, in no particular order. This is what lets the proactive +// digestion tick run the pattern detector over everything accumulated so far +// instead of only the pair touched by the utterance that just landed +// (Vikunja #43) — the tick has no "current utterance," so it has to ask the +// store what to look at. +func (s *Store) DistinctEventPairs(ctx context.Context) ([]EventPair, error) { + rows, err := s.db.QueryContext(ctx, `SELECT DISTINCT action, object FROM events`) + if err != nil { + return nil, fmt.Errorf("distinct event pairs: %w", err) + } + defer rows.Close() + var out []EventPair + for rows.Next() { + var p EventPair + if err := rows.Scan(&p.Action, &p.Object); err != nil { + return nil, err + } + out = append(out, p) + } + return out, rows.Err() +} + // EventsFor returns all events matching action+object, ordered by ts ascending // (oldest first — the order the pattern detector needs for interval computation). func (s *Store) EventsFor(ctx context.Context, action, object string) ([]Event, error) { diff --git a/internal/store/proposed_routines.go b/internal/store/proposed_routines.go index 741fd0a..67e43f5 100644 --- a/internal/store/proposed_routines.go +++ b/internal/store/proposed_routines.go @@ -51,9 +51,10 @@ var ( // keep finding the pattern, and every re-propose is refused here. Maven is not // a nag. // -// TODO(vikunja#46): the detector currently only writes here from the voice -// path. Once digestion runs the detector on its own tick, that tick should -// call this too, so a pattern gets noticed even with nobody at the mic. +// Vikunja #43: this is called both from the voice fact-write path (for the +// immediate spoken confirmation) and from the digestion tick's proactive +// scan (cmd/mavend/tick.go's detectPatterns, via patterns.go's +// detectAndPropose), so a pattern gets noticed even with nobody at the mic. func (s *Store) CreateProposedRoutine(ctx context.Context, action, object string, intervalDays float64, ts time.Time) (int64, error) { res, err := s.db.ExecContext(ctx, `INSERT INTO proposed_routines (action, object, interval_days, status, created_ts)