Merge pull request 'Bug: morning.Item has no required/optional flag, so behaviour 1 of task 280 cannot hold' (#125) from task/473-bug-morning-item-has-no-required-flag into master
This commit was merged in pull request #125.
This commit is contained in:
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
+21
-5
@@ -761,11 +761,7 @@ func (t *tickLoop) fireMorningRoutines(ctx context.Context, now time.Time, state
|
|||||||
facts := t.gatherMorningFacts(ctx)
|
facts := t.gatherMorningFacts(ctx)
|
||||||
|
|
||||||
for _, cand := range morning.Due(t.morningRoutines, facts, t.morningLast, now) {
|
for _, cand := range morning.Due(t.morningRoutines, facts, t.morningLast, now) {
|
||||||
labels := make([]string, len(cand.Missing))
|
body := morningNudgeBody(cand)
|
||||||
for i, it := range cand.Missing {
|
|
||||||
labels[i] = it.Label
|
|
||||||
}
|
|
||||||
body := fmt.Sprintf("%s: не сделано — %s", cand.Routine.Name, strings.Join(labels, ", "))
|
|
||||||
pn := delivery.PhrasedNudge{
|
pn := delivery.PhrasedNudge{
|
||||||
Candidate: loop.Candidate{
|
Candidate: loop.Candidate{
|
||||||
Rule: loop.Rule{Name: "morning:" + cand.Routine.Name, Severity: loop.Severity(cand.Routine.Severity)},
|
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
|
// gatherMorningFacts reads the latest fact for every item's fact_key across
|
||||||
// all configured morning routines. Shared by fireMorningRoutines (nudge
|
// all configured morning routines. Shared by fireMorningRoutines (nudge
|
||||||
// decision) and morningStatus (read-only query) so the two paths can never
|
// decision) and morningStatus (read-only query) so the two paths can never
|
||||||
|
|||||||
@@ -601,6 +601,9 @@ type MorningRoutineItemConfig struct {
|
|||||||
Key string `json:"key"`
|
Key string `json:"key"`
|
||||||
FactKey string `json:"fact_key"`
|
FactKey string `json:"fact_key"`
|
||||||
Label string `json:"label"`
|
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
|
// 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 {
|
for i, r := range mc {
|
||||||
items := make([]morning.Item, len(r.Items))
|
items := make([]morning.Item, len(r.Items))
|
||||||
for j, it := range 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))
|
weekdays := make([]time.Weekday, len(r.Weekdays))
|
||||||
for j, w := range r.Weekdays {
|
for j, w := range r.Weekdays {
|
||||||
|
|||||||
@@ -30,6 +30,18 @@ type Item struct {
|
|||||||
Key string
|
Key string
|
||||||
FactKey string
|
FactKey string
|
||||||
Label string // RU text surfaced when this item is still missing.
|
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
|
// 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
|
// 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 {
|
type Candidate struct {
|
||||||
Routine Routine
|
Routine Routine
|
||||||
Missing []Item
|
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
|
// Validate reports the first structural problem with a routine set: missing
|
||||||
// name/items, an unparseable HH:MM, an inverted window, a duplicate item key
|
// 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
|
// 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)
|
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
|
continue
|
||||||
}
|
}
|
||||||
if prev, seen := last[r.Name]; seen && sameDay(prev, now) {
|
if prev, seen := last[r.Name]; seen && sameDay(prev, now) {
|
||||||
|
|||||||
@@ -182,3 +182,40 @@ func TestDueRespectsExplicitNudgeAt(t *testing.T) {
|
|||||||
t.Fatalf("expected candidate at explicit nudge_at, got %d", len(out))
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user