Compare commits

...

2 Commits

Author SHA1 Message Date
claude 9aabb01e2a recalleval: a filler id a case reuses is refused at load (V-386)
Every case is scored over its own notes plus the whole filler set, and the
two stores disagree about a repeated id: sqlite upserts on it, the in-memory
store appends. So one collision makes a case score differently on the two
backends, and it reads as an embedder or gate difference — the one thing this
harness exists to measure. It was dodged by hand during #373 by renaming two
ids.

The check sits in Load rather than in TestLoadFixture, so it covers every
caller of the fixture and not only the one that remembers to look.
2026-08-04 03:18:42 +04:00
claude 2815adee03 morning: an item can be optional, so a skipped stretch is not a skipped pill (V-473)
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.
2026-08-04 03:17:29 +04:00
7 changed files with 204 additions and 8 deletions
+39
View File
@@ -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
View File
@@ -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
+4 -1
View File
@@ -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 {
+35
View File
@@ -90,9 +90,44 @@ func Load() (Fixture, error) {
if len(f.Cases) == 0 {
return Fixture{}, fmt.Errorf("fixture has no cases")
}
if err := checkIDs(f); err != nil {
return Fixture{}, err
}
return f, nil
}
// checkIDs refuses a fixture where a case note and a filler note share an id.
//
// Every case is scored over its own notes plus the whole filler set, and the
// two stores disagree about what a repeated id means: the sqlite store upserts
// on it, the in-memory store appends. So one collision makes a case score
// differently on the two backends, and it reads as an embedder or gate
// difference, which is the one thing this harness exists to measure (Vikunja
// #386). It was dodged once by hand during #373 by renaming two ids.
//
// Checked in Load rather than in the test, so every caller of the fixture is
// covered and not only the one that remembers to look.
func checkIDs(f Fixture) error {
filler := make(map[string]bool, len(f.Filler))
for _, n := range f.Filler {
if n.ID == "" {
return fmt.Errorf("filler note with an empty id")
}
if filler[n.ID] {
return fmt.Errorf("duplicate filler note id %q", n.ID)
}
filler[n.ID] = true
}
for _, c := range f.Cases {
for _, n := range c.Notes {
if filler[n.ID] {
return fmt.Errorf("case %s: note id %q collides with a filler note", c.ID, n.ID)
}
}
}
return nil
}
// NewStore builds an empty store for one case, plus a function to release it.
// A factory rather than a store because every case needs a clean index — notes
// from case A must not be visible to case B's query.
@@ -337,3 +337,28 @@ func marginSweep(t *testing.T, emb router.Embedder, f Fixture) string {
}
return b.String()
}
// TestFillerIDCollisionIsRefused — the guard that keeps a fixture edit from
// looking like a backend difference (Vikunja #386).
func TestFillerIDCollisionIsRefused(t *testing.T) {
f := Fixture{
SchemaVersion: SchemaVersion,
Cases: []Case{{ID: "ru-001", Notes: []StoredNote{{ID: "f1", Text: "..."}}}},
Filler: []StoredNote{{ID: "f1", Text: "..."}},
}
if err := checkIDs(f); err == nil {
t.Fatal("a case note reusing a filler id must be refused")
}
f.Filler = append(f.Filler, StoredNote{ID: "f1", Text: "..."})
if err := checkIDs(Fixture{SchemaVersion: SchemaVersion, Filler: f.Filler}); err == nil {
t.Fatal("a duplicate filler id must be refused")
}
ok := Fixture{
SchemaVersion: SchemaVersion,
Cases: []Case{{ID: "ru-001", Notes: []StoredNote{{ID: "n1", Text: "..."}}}},
Filler: []StoredNote{{ID: "f1", Text: "..."}},
}
if err := checkIDs(ok); err != nil {
t.Fatalf("a clean fixture must pass: %v", err)
}
}
+43 -2
View File
@@ -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) {
+37
View File
@@ -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)
}
}