From 2815adee030e0083d591568938a5fd738917a778 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 4 Aug 2026 03:17:29 +0400 Subject: [PATCH] morning: an item can be optional, so a skipped stretch is not a skipped pill (V-473) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Item carried only Key, FactKey and Label, so every checklist entry was implicitly required and behaviour 1 of #280 could not hold at all. It was not thin config — there was no field to set. Item.Optional, `"optional": true` in the routine config, default false, so a routine written before today behaves exactly as it did. Due now fires on a missing required item and not on an optional one, and the optional stragglers still travel in Missing so the one message per day per routine can name them after the required ones, in softer words. Evidence, the window and the day plan treat both kinds alike. A missing optional item is still missing — it just does not earn a nudge, because a checklist where everything is mandatory is one he learns to ignore. --- cmd/mavend/morning_nudge_test.go | 39 +++++++++++++++++++++++++++ cmd/mavend/tick.go | 26 ++++++++++++++---- internal/config/config.go | 5 +++- internal/morning/morning.go | 45 ++++++++++++++++++++++++++++++-- internal/morning/morning_test.go | 37 ++++++++++++++++++++++++++ 5 files changed, 144 insertions(+), 8 deletions(-) create mode 100644 cmd/mavend/morning_nudge_test.go diff --git a/cmd/mavend/morning_nudge_test.go b/cmd/mavend/morning_nudge_test.go new file mode 100644 index 0000000..8f4fc07 --- /dev/null +++ b/cmd/mavend/morning_nudge_test.go @@ -0,0 +1,39 @@ +package main + +import ( + "strings" + "testing" + + "github.com/kami/maven/internal/morning" +) + +// TestMorningNudgeBodySeparatesOptional — the one message a routine is allowed +// per day says what was not done, then what he could still do (Vikunja #473). +func TestMorningNudgeBodySeparatesOptional(t *testing.T) { + cand := morning.Candidate{ + Routine: morning.Routine{Name: "утро"}, + Missing: []morning.Item{ + {Key: "meds", Label: "таблетки"}, + {Key: "stretch", Label: "растяжка", Optional: true}, + }, + } + body := morningNudgeBody(cand) + if !strings.Contains(body, "не сделано — таблетки") { + t.Fatalf("the required item must be named as not done: %q", body) + } + if !strings.Contains(body, "если будет время — растяжка") { + t.Fatalf("the optional item must read softer: %q", body) + } + if strings.Contains(body, "не сделано — таблетки, растяжка") { + t.Fatalf("optional must not be folded into the required list: %q", body) + } + + // Nothing optional missing: the sentence is what it always was. + only := morning.Candidate{ + Routine: morning.Routine{Name: "утро"}, + Missing: []morning.Item{{Key: "meds", Label: "таблетки"}}, + } + if got, want := morningNudgeBody(only), "утро: не сделано — таблетки"; got != want { + t.Fatalf("morningNudgeBody = %q, want %q", got, want) + } +} diff --git a/cmd/mavend/tick.go b/cmd/mavend/tick.go index 75d5600..52c4732 100644 --- a/cmd/mavend/tick.go +++ b/cmd/mavend/tick.go @@ -761,11 +761,7 @@ func (t *tickLoop) fireMorningRoutines(ctx context.Context, now time.Time, state facts := t.gatherMorningFacts(ctx) for _, cand := range morning.Due(t.morningRoutines, facts, t.morningLast, now) { - labels := make([]string, len(cand.Missing)) - for i, it := range cand.Missing { - labels[i] = it.Label - } - body := fmt.Sprintf("%s: не сделано — %s", cand.Routine.Name, strings.Join(labels, ", ")) + body := morningNudgeBody(cand) pn := delivery.PhrasedNudge{ Candidate: loop.Candidate{ Rule: loop.Rule{Name: "morning:" + cand.Routine.Name, Severity: loop.Severity(cand.Routine.Severity)}, @@ -781,6 +777,26 @@ func (t *tickLoop) fireMorningRoutines(ctx context.Context, now time.Time, state } } +// morningNudgeBody words the one message a routine gets per day. Required +// items are what she says was not done; optional ones follow, worded as +// something he could still do rather than something he owes (Vikunja #473). +// Operator text, not phrased by the model, for the same reason it always was: +// a checklist item must not be invented. +func morningNudgeBody(cand morning.Candidate) string { + labels := func(items []morning.Item) string { + out := make([]string, len(items)) + for i, it := range items { + out[i] = it.Label + } + return strings.Join(out, ", ") + } + body := fmt.Sprintf("%s: не сделано — %s", cand.Routine.Name, labels(morning.Required(cand.Missing))) + if opt := morning.OptionalOnly(cand.Missing); len(opt) > 0 { + body += fmt.Sprintf(". если будет время — %s", labels(opt)) + } + return body +} + // gatherMorningFacts reads the latest fact for every item's fact_key across // all configured morning routines. Shared by fireMorningRoutines (nudge // decision) and morningStatus (read-only query) so the two paths can never diff --git a/internal/config/config.go b/internal/config/config.go index ff72b31..990ae1e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -601,6 +601,9 @@ type MorningRoutineItemConfig struct { Key string `json:"key"` FactKey string `json:"fact_key"` Label string `json:"label"` + // Optional — this one being skipped does not earn a nudge. Default false, + // so a routine written before 04-08-2026 keeps behaving as it did. + Optional bool `json:"optional,omitempty"` } // QuietHoursConfig — a recurring daily quiet-window. Times are local to the @@ -1734,7 +1737,7 @@ func morningRoutinesFromConfig(mc []MorningRoutineConfig) []morning.Routine { for i, r := range mc { items := make([]morning.Item, len(r.Items)) for j, it := range r.Items { - items[j] = morning.Item{Key: it.Key, FactKey: it.FactKey, Label: it.Label} + items[j] = morning.Item{Key: it.Key, FactKey: it.FactKey, Label: it.Label, Optional: it.Optional} } weekdays := make([]time.Weekday, len(r.Weekdays)) for j, w := range r.Weekdays { diff --git a/internal/morning/morning.go b/internal/morning/morning.go index 4034bef..418ee44 100644 --- a/internal/morning/morning.go +++ b/internal/morning/morning.go @@ -30,6 +30,18 @@ type Item struct { Key string FactKey string Label string // RU text surfaced when this item is still missing. + // Optional — a missing one is not worth a nudge on its own. + // + // Every item was implicitly required until 04-08-2026, because there was + // no field, so a skipped stretch read exactly like skipped medication and + // #280's first behaviour could not hold (Vikunja #473). A checklist where + // everything is mandatory is a checklist he learns to ignore. + // + // It changes two things and nothing else: an all-optional routine never + // nudges, and a nudge that does fire names the optional stragglers after + // the required ones, in softer words. Evidence, the window and the day + // plan treat both kinds alike — a missing optional item is still missing. + Optional bool } // Routine — one daily checklist. WindowStart/WindowEnd are "HH:MM" local @@ -60,12 +72,37 @@ type Status struct { } // Candidate — a routine that's due for its one-per-day nag: the window has -// reached NudgeAt and at least one item is still unevidenced. +// reached NudgeAt and at least one REQUIRED item is still unevidenced. Missing +// carries the optional stragglers too, so the one message she is allowed per +// day per routine can mention them; they never cause it. type Candidate struct { Routine Routine Missing []Item } +// Required reports the missing items that are not optional. The nudge fires on +// these; the rest ride along. +func Required(missing []Item) []Item { + var out []Item + for _, it := range missing { + if !it.Optional { + out = append(out, it) + } + } + return out +} + +// OptionalOnly is the other half of Required. +func OptionalOnly(missing []Item) []Item { + var out []Item + for _, it := range missing { + if it.Optional { + out = append(out, it) + } + } + return out +} + // Validate reports the first structural problem with a routine set: missing // name/items, an unparseable HH:MM, an inverted window, a duplicate item key // within a routine, or an out-of-range weekday. Called at config load so a @@ -191,7 +228,11 @@ func Due(routines []Routine, facts map[string]store.Fact, last map[string]time.T missing = append(missing, it) } } - if len(missing) == 0 { + // A day where only the optional items were skipped is a fine day, and + // nagging about it is what teaches him to stop listening (Vikunja + // #473). The optional ones still travel in Missing so the message can + // mention them when it is being sent anyway. + if len(Required(missing)) == 0 { continue } if prev, seen := last[r.Name]; seen && sameDay(prev, now) { diff --git a/internal/morning/morning_test.go b/internal/morning/morning_test.go index 5609bff..1a9385e 100644 --- a/internal/morning/morning_test.go +++ b/internal/morning/morning_test.go @@ -182,3 +182,40 @@ func TestDueRespectsExplicitNudgeAt(t *testing.T) { t.Fatalf("expected candidate at explicit nudge_at, got %d", len(out)) } } + +// TestOptionalItemsDoNotEarnANudge — behaviour 1 of #280, which could not hold +// while every item was implicitly required (Vikunja #473). +func TestOptionalItemsDoNotEarnANudge(t *testing.T) { + r := Routine{ + Name: "утро", + WindowStart: "07:00", + WindowEnd: "10:00", + Items: []Item{ + {Key: "meds", FactKey: "meds", Label: "таблетки"}, + {Key: "stretch", FactKey: "stretch", Label: "растяжка", Optional: true}, + }, + } + now := time.Date(2026, 8, 4, 10, 0, 0, 0, time.UTC) + took := map[string]store.Fact{"meds": {Key: "meds", Ts: now.Add(-2 * time.Hour)}} + + // Only the stretch was skipped: nothing to say. + if due := Due([]Routine{r}, took, map[string]time.Time{}, now); len(due) != 0 { + t.Fatalf("an optional item alone must not nudge, got %+v", due) + } + // The medication was skipped: she says so, and mentions the stretch too. + due := Due([]Routine{r}, map[string]store.Fact{}, map[string]time.Time{}, now) + if len(due) != 1 { + t.Fatalf("a missing required item must nudge, got %+v", due) + } + if got := Required(due[0].Missing); len(got) != 1 || got[0].Key != "meds" { + t.Fatalf("Required = %+v, want the meds item alone", got) + } + if got := OptionalOnly(due[0].Missing); len(got) != 1 || got[0].Key != "stretch" { + t.Fatalf("OptionalOnly = %+v, want the stretch item alone", got) + } + // The window still reports it as missing — optional is not invisible. + st := Evaluate(r, map[string]store.Fact{}, now.Add(-time.Hour)) + if len(st.Missing) != 2 { + t.Fatalf("Evaluate must still list both, got %+v", st.Missing) + } +}