From b3c2fad4ec175ffd71a70aa113e6a5badb3cd5c7 Mon Sep 17 00:00:00 2001 From: kami Date: Sat, 1 Aug 2026 14:07:43 +0400 Subject: [PATCH] morning: recite the day the store actually holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four defects in the plan, all of them in what it reads or how it prints it. The checklist line was keyed on Status.Active, which Evaluate reports only inside the window, so a morning routine skipped and asked about at 14:00 said nothing. Outstanding answers the question the plan asks, "what did today still not get done", and the line stays placed at the nudge time so it sorts to the top of the day. Nothing before the window opens counts, so 06:00 is not a complaint. The event text kept the "@ 14:00-14:30" tail FactValue writes, next to a line that prints the hour itself, so every event said its time twice. Reminders came off ListReminders, which orders by creation, so the 500 row cap dropped a reminder stated long ago for today and kept one stated this morning for next year. PendingReminders bounds by fire time instead. The pending filter used a string literal, one typo from matching nothing. After now marks the plan it trimmed. "что дальше?" past the last item answered "на 03.08.2026 ничего не запланировано", which denies a day he just lived through. The surface the plan belongs on is still open, tracked as Vikunja #431; the comment in actions_query.go points at it. Found in review of #58. --- cmd/mavend/actions_query.go | 16 +++---- cmd/mavend/dayplan_test.go | 85 +++++++++++++++++++++++++++++++++++ cmd/mavend/tick.go | 17 ++++--- internal/morning/morning.go | 23 ++++++++++ internal/morning/plan.go | 29 +++++++++--- internal/morning/plan_test.go | 52 ++++++++++++++++++++- internal/store/reminders.go | 39 ++++++++++++++++ 7 files changed, 234 insertions(+), 27 deletions(-) diff --git a/cmd/mavend/actions_query.go b/cmd/mavend/actions_query.go index c36682d..6eb4834 100644 --- a/cmd/mavend/actions_query.go +++ b/cmd/mavend/actions_query.go @@ -139,11 +139,16 @@ func (h *reactiveHandler) queryFactByKey(ctx context.Context, t *queryTurn) (str // queryDayPlan — "какие планы на сегодня?", "что у меня по плану?", "что // дальше?" (Vikunja #128). Recites the day: calendar events, pending -// reminders, and any morning checklist still outstanding. +// reminders, and every morning checklist item today still has no evidence for, +// including the ones whose window has closed. // // Read-only by construction — the plan is assembled and rendered core-side and // nothing here schedules or announces. "что дальше?" asks for the rest of the // day, so that phrasing trims what has already passed. +// +// What surface this belongs on is still open, tracked as Vikunja #431 ("Board +// surface: Maven holds the work board, runs the intake form, never argues"). +// The spoken recital here is the current answer, not the decided one. func (h *reactiveHandler) queryDayPlan(ctx context.Context, t *queryTurn) (string, bool) { if !router.IsDayPlanQuery(t.dec.Utterance) { return "", false @@ -153,7 +158,7 @@ func (h *reactiveHandler) queryDayPlan(ctx context.Context, t *queryTurn) (strin log.Printf("voice: day plan: %v", err) return "не получилось собрать план.", true } - if !isRestOfDayQuery(t.dec.Utterance) { + if !router.IsRestOfDayQuery(t.dec.Utterance) { return plan.Spoken, true } // Rebuild the pure plan so the rest-of-day rendering is the same code that @@ -170,13 +175,6 @@ func (h *reactiveHandler) queryDayPlan(ctx context.Context, t *queryTurn) (strin return p.After(h.now()).FormatRU(), true } -// isRestOfDayQuery — "что дальше?" and its English form, the only plan phrasing -// that means "from now on" rather than "the whole day". -func isRestOfDayQuery(text string) bool { - s := strings.ToLower(text) - return strings.Contains(s, "дальше") || strings.Contains(s, "next") -} - // habitFactWindow — how many recent facts the behaviour profile is counted // over. Enough for a season of habits without scanning the whole store on every // question; the profile is recomputed on read, so the bound is the cost control. diff --git a/cmd/mavend/dayplan_test.go b/cmd/mavend/dayplan_test.go index 0bc40b8..539ee0a 100644 --- a/cmd/mavend/dayplan_test.go +++ b/cmd/mavend/dayplan_test.go @@ -2,13 +2,16 @@ package main import ( "context" + "database/sql" "errors" "strings" "testing" "time" + "github.com/kami/maven/internal/calendar" "github.com/kami/maven/internal/ipc" "github.com/kami/maven/internal/router" + "github.com/kami/maven/internal/store" ) // planAPI answers only DayPlan; every other call is unimplemented, which is @@ -85,12 +88,40 @@ func TestQueryDayPlanTrimsToRestOfDay(t *testing.T) { } } +// "что дальше?" after the last item of the day. The day was not empty, it is +// over, and the whole-day empty line says something false about a day he just +// lived through. +func TestQueryDayPlanRestOfDayWhenNothingIsLeft(t *testing.T) { + plan := samplePlan() + h := &reactiveHandler{api: &planAPI{plan: plan}, now: func() time.Time { + return time.Date(2026, 8, 3, 23, 0, 0, 0, time.UTC) + }} + reply, ok := h.queryDayPlan(context.Background(), &queryTurn{ + dec: router.Decision{Intent: router.IntentQuery, Utterance: "что дальше?"}, + }) + if !ok { + t.Fatal("expected the plan source to claim it") + } + if strings.Contains(reply, plan.Date.Format("02.01.2006")) { + t.Errorf("the day had things on it and they are done, not empty: %q", reply) + } + if reply != "на сегодня больше ничего не запланировано." { + t.Errorf("reply = %q", reply) + } +} + // A question that is not about the plan must fall through, or the plan buries // the calendar listing and the weather behind it. func TestQueryDayPlanPassesOnEverythingElse(t *testing.T) { for _, q := range []string{ "что у меня сегодня?", "какие планы на завтра?", + // The plan can only be built for the clock's own day. Naming another + // one has to fall through, not get answered with today. + "какие планы на понедельник?", + "какие планы на неделю?", + "какие планы на выходные?", + "what are my plans for friday?", "когда планёрка?", "какая погода?", "", @@ -225,3 +256,57 @@ func TestHabitSourcePrecedesCalendar(t *testing.T) { t.Errorf("habits at %d must come before calendar at %d", habits, cal) } } + +// The plan reads the store on the owner's clock: one line per event, the hour +// printed once, and reminders selected by fire time rather than by how +// recently they were stated. +func TestTickDayPlanReadsTheStore(t *testing.T) { + st := newTestStore(t) + ctx := context.Background() + tl := newTestTickLoop(t, st, &fakeSink{}, nil) + + now := time.Date(2026, 8, 3, 12, 0, 0, 0, time.Local) + day := time.Date(2026, 8, 3, 0, 0, 0, 0, time.Local) + ev := calendar.Event{ + Summary: "Standup", + Start: day.Add(14 * time.Hour), + End: day.Add(14*time.Hour + 30*time.Minute), + } + // Rescheduled: same key, a second row. + if _, err := st.WriteFact(ctx, ev.Start, store.KindEnv, calendar.FactKey(ev), + calendar.FactValue(ev), calendar.SourcePersonal, 1.0, sql.NullInt64{}); err != nil { + t.Fatalf("WriteFact: %v", err) + } + moved := ev + moved.Start, moved.End = day.Add(16*time.Hour), day.Add(16*time.Hour+30*time.Minute) + if _, err := st.WriteFact(ctx, moved.Start, store.KindEnv, calendar.FactKey(moved), + calendar.FactValue(moved), calendar.SourcePersonal, 1.0, sql.NullInt64{}); err != nil { + t.Fatalf("WriteFact: %v", err) + } + // One reminder today, one next year. Both are pending; only today's is a + // plan for today. + if _, err := st.CreateReminder(ctx, day.Add(18*time.Hour), "позвонить маме", ""); err != nil { + t.Fatalf("CreateReminder: %v", err) + } + if _, err := st.CreateReminder(ctx, day.AddDate(1, 0, 0), "продлить страховку", ""); err != nil { + t.Fatalf("CreateReminder: %v", err) + } + + plan := tl.dayPlan(ctx, now) + if len(plan.Items) != 2 { + t.Fatalf("got %d items, want the moved standup and today's reminder: %+v", len(plan.Items), plan.Items) + } + ev0 := plan.Items[0] + if ev0.Kind != "event" || ev0.At.In(time.Local).Format("15:04") != "16:00" { + t.Errorf("event = %+v, want the 16:00 one", ev0) + } + if ev0.Text != "Standup" { + t.Errorf("text = %q — the plan prints the hour itself", ev0.Text) + } + if plan.Items[1].Text != "позвонить маме" { + t.Errorf("second item = %+v", plan.Items[1]) + } + if strings.Contains(plan.Spoken, "страховку") { + t.Errorf("a reminder for next year is not today's plan: %q", plan.Spoken) + } +} diff --git a/cmd/mavend/tick.go b/cmd/mavend/tick.go index b3b7c7d..b1c37c4 100644 --- a/cmd/mavend/tick.go +++ b/cmd/mavend/tick.go @@ -19,6 +19,7 @@ import ( "sync" "time" + "github.com/kami/maven/internal/calendar" "github.com/kami/maven/internal/config" "github.com/kami/maven/internal/delivery" "github.com/kami/maven/internal/ipc" @@ -808,8 +809,10 @@ func (t *tickLoop) dayPlan(ctx context.Context, now time.Time) ipc.DayPlan { } for _, f := range facts { events = append(events, morning.PlanEntry{ - At: f.Ts, - Text: f.Value, + At: f.Ts, + // The plan prints the hour itself, so the "@ 14:00-14:30" tail the + // fact value carries would say it twice. + Text: calendar.FactSummary(f.Value), Kind: morning.PlanEvent, // Provenance below a calendar read (an ambient relay, #126) is // hedged rather than recited as fact. @@ -818,12 +821,12 @@ func (t *tickLoop) dayPlan(ctx context.Context, now time.Time) ipc.DayPlan { } var reminders []morning.PlanEntry - rems, err := t.store.ListReminders(ctx, dayPlanMaxReminders) + rems, err := t.store.PendingReminders(ctx, dayStart, dayEnd) if err != nil { - log.Printf("tick: day plan: list reminders: %v", err) + log.Printf("tick: day plan: pending reminders: %v", err) } for _, r := range rems { - if r.Status != "pending" { + if r.Status != store.ReminderPending { continue } fire := r.NextFireTs @@ -856,10 +859,6 @@ func (t *tickLoop) dayPlan(ctx context.Context, now time.Time) ipc.DayPlan { return out } -// dayPlanMaxReminders bounds the reminder scan. The plan covers one day; a -// pending queue longer than this is a bug elsewhere, not a plan to recite. -const dayPlanMaxReminders = 500 - // tune — the feedback auto-tuner's impure step. runs on a slow cadence // (autotuneInterval, see run) so it doesn't write a fact every tick. for each // rule: diff --git a/internal/morning/morning.go b/internal/morning/morning.go index 8a94db7..beabb0b 100644 --- a/internal/morning/morning.go +++ b/internal/morning/morning.go @@ -140,6 +140,29 @@ func Evaluate(r Routine, facts map[string]store.Fact, now time.Time) Status { return st } +// Outstanding reports the items of a routine that today has no evidence for, +// whether or not the window is still open. Evaluate answers "what is missing +// right now" and goes silent the moment the window closes; the day plan asks a +// different question, "what did today still not get done", and a skipped +// routine is exactly what it is worth telling him. Nothing before the window +// opens is outstanding yet, so the morning routine is not a complaint at 06:00. +func Outstanding(r Routine, facts map[string]store.Fact, now time.Time) []Item { + if !appliesToday(r, now) { + return nil + } + start, ok := todayAt(r.WindowStart, now) + if !ok || now.Before(start) { + return nil + } + var missing []Item + for _, it := range r.Items { + if !evidenced(it, facts, start, now) { + missing = append(missing, it) + } + } + return missing +} + // Due returns the routines that have reached their nudge time today with at // least one item still missing, and records `now` in `last` for each one // returned so it fires at most once per calendar day. The caller owns diff --git a/internal/morning/plan.go b/internal/morning/plan.go index 530a33a..ba2c699 100644 --- a/internal/morning/plan.go +++ b/internal/morning/plan.go @@ -48,10 +48,13 @@ type PlanEntry struct { Uncertain bool } -// Plan — the ordered day. Date is the calendar day it describes. +// Plan — the ordered day. Date is the calendar day it describes. Rest marks a +// plan trimmed by After, which changes what an empty one means: a day with +// nothing on it and a day whose last item has passed are different answers. type Plan struct { Date time.Time Items []PlanEntry + Rest bool } // BuildPlan orders everything known about the day Now falls on: calendar @@ -98,17 +101,23 @@ func BuildPlan(routines []Routine, facts map[string]store.Fact, events, reminder // checklistEntries renders one line per routine with work left in it, placed at // the routine's nudge time — where the checklist actually matters in the day. -// A routine that does not apply today, is not in its window, or is already +// A routine that does not apply today, has not opened yet, or is already // complete contributes nothing: the plan says what is left, not what was done. +// +// A closed window still counts. Asked at 14:00 with the morning routine +// unfinished, the plan used to say nothing about it, because Evaluate reports +// Active only inside the window. What he skipped is the one thing the plan can +// tell him that the calendar cannot, and the entry sorts to its nudge time, not +// to the moment of asking. func checklistEntries(routines []Routine, facts map[string]store.Fact, now time.Time) []PlanEntry { var out []PlanEntry for _, r := range routines { - st := Evaluate(r, facts, now) - if !st.Active || len(st.Missing) == 0 { + missing := Outstanding(r, facts, now) + if len(missing) == 0 { continue } - labels := make([]string, 0, len(st.Missing)) - for _, it := range st.Missing { + labels := make([]string, 0, len(missing)) + for _, it := range missing { label := it.Label if label == "" { label = it.Key @@ -136,7 +145,7 @@ func checklistEntries(routines []Routine, facts map[string]store.Fact, now time. // "что дальше?" as opposed to "какие планы на сегодня?". The Date is kept, so an // empty result still knows which day it is empty for. func (p Plan) After(now time.Time) Plan { - out := Plan{Date: p.Date} + out := Plan{Date: p.Date, Rest: true} for _, it := range p.Items { if it.At.Before(now) { continue @@ -151,6 +160,12 @@ func (p Plan) After(now time.Time) Plan { // she does not tell him to get on with it. func (p Plan) FormatRU() string { if len(p.Items) == 0 { + // "что дальше?" after the last item of the day. The day was not empty, + // it is over, and saying it was empty is a false statement about a day + // he just lived. + if p.Rest { + return "на сегодня больше ничего не запланировано." + } return fmt.Sprintf("на %s ничего не запланировано.", p.Date.Format("02.01.2006")) } parts := make([]string, len(p.Items)) diff --git a/internal/morning/plan_test.go b/internal/morning/plan_test.go index 35ff4d1..c7b282e 100644 --- a/internal/morning/plan_test.go +++ b/internal/morning/plan_test.go @@ -163,7 +163,55 @@ func TestPlanAfter(t *testing.T) { if len(empty.Items) != 0 { t.Errorf("got %+v", empty.Items) } - if !strings.Contains(empty.FormatRU(), "ничего не запланировано") { - t.Errorf("empty plan reads %q", empty.FormatRU()) + // An empty rest-of-day is not an empty day. Saying "на 03.08.2026 ничего + // не запланировано" at 23:00 denies the day he just lived. + if got, want := empty.FormatRU(), "на сегодня больше ничего не запланировано."; got != want { + t.Errorf("empty rest-of-day reads %q, want %q", got, want) + } +} + +// The plan says what today still has not got done, and a closed window does not +// make a skipped routine untrue. Evaluate reports Active only inside the +// window, so keying the checklist line off it meant the one thing the plan can +// tell him that the calendar cannot went silent at 11:00. +func TestBuildPlanKeepsAClosedWindowOutstanding(t *testing.T) { + now := time.Date(2026, 8, 3, 14, 0, 0, 0, time.UTC) + routines := []Routine{{ + Name: "утро", WindowStart: "07:00", WindowEnd: "11:00", NudgeAt: "10:30", + Items: []Item{ + {Key: "water", FactKey: "drank_water", Label: "выпить воды"}, + {Key: "pills", FactKey: "took_pills", Label: "витамины"}, + }, + }} + facts := map[string]store.Fact{"drank_water": {Ts: planAt(now, 8, 0)}} + + p := BuildPlan(routines, facts, nil, nil, now) + if len(p.Items) != 1 { + t.Fatalf("got %+v, want the unfinished morning routine", p.Items) + } + it := p.Items[0] + if it.Kind != PlanChecklist { + t.Errorf("kind = %q", it.Kind) + } + // Placed at the nudge time, so it sorts to the top of the day rather than + // to the moment of asking. + if got := it.At.Format("15:04"); got != "10:30" { + t.Errorf("placed at %s, want 10:30", got) + } + if !strings.Contains(it.Text, "витамины") || strings.Contains(it.Text, "выпить воды") { + t.Errorf("line = %q", it.Text) + } +} + +// A routine whose window has not opened yet is not outstanding. Nothing has +// been skipped at 06:00. +func TestBuildPlanIgnoresAnUnopenedWindow(t *testing.T) { + now := time.Date(2026, 8, 3, 6, 0, 0, 0, time.UTC) + routines := []Routine{{ + Name: "утро", WindowStart: "07:00", WindowEnd: "11:00", + Items: []Item{{Key: "water", FactKey: "drank_water", Label: "выпить воды"}}, + }} + if p := BuildPlan(routines, nil, nil, nil, now); len(p.Items) != 0 { + t.Fatalf("got %+v", p.Items) } } diff --git a/internal/store/reminders.go b/internal/store/reminders.go index 2fb4447..e3788c5 100644 --- a/internal/store/reminders.go +++ b/internal/store/reminders.go @@ -26,6 +26,15 @@ type Reminder struct { Collapsed []Reminder } +// Reminder lifecycle states. Named for the same reason DigestStatus is: a +// caller filtering on the string literal "pending" is one typo away from a +// filter that silently matches nothing. +const ( + ReminderPending = "pending" + ReminderFired = "fired" + ReminderCancelled = "cancelled" +) + var ( ErrReminderNotFound = errors.New("store: reminder not found") ErrReminderState = errors.New("store: reminder not in a mutable state") @@ -93,6 +102,36 @@ func (s *Store) DueReminders(ctx context.Context, now time.Time) ([]Reminder, er return out, rows.Err() } +// PendingReminders returns the pending reminders whose next fire time falls in +// [from, to), earliest first. +// +// The day plan used to take the newest 500 rows out of ListReminders, which +// orders by creation, and then filter them by day. A reminder stated long ago +// for today fell off the end of that scan while a reminder stated this morning +// for next year stayed on it. Bounding by fire time drops what is out of range +// instead of what is old. +func (s *Store) PendingReminders(ctx context.Context, from, to time.Time) ([]Reminder, error) { + rows, err := s.db.QueryContext(ctx, ` + SELECT id, created_ts, fire_ts, next_fire_ts, payload, status, cron + FROM reminders + WHERE status = ? AND next_fire_ts >= ? AND next_fire_ts < ? + ORDER BY next_fire_ts ASC, id ASC`, + ReminderPending, from.UnixMilli(), to.UnixMilli()) + if err != nil { + return nil, fmt.Errorf("pending reminders: %w", err) + } + defer rows.Close() + var out []Reminder + for rows.Next() { + r, err := scanReminder(rows) + if err != nil { + return nil, err + } + out = append(out, r) + } + return out, rows.Err() +} + // MarkReminder sets a reminder's status. Only valid transitions: pending→fired, // pending→cancelled. Anything else is a programming error. func (s *Store) MarkReminder(ctx context.Context, id int64, status string) error {