From 85456d383333a40c0ea60a28e370884cfdd1493f Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 03:12:28 +0400 Subject: [PATCH 1/2] tasks: count overdue by calendar day like the ranker does (V-581) Stalls compared the due instant to now while Rank compares whole calendar days, so a task due at 09:00 was counted overdue from 09:01 while its own row on the same page still read the reason as today. Stalls now reads dayDelta. A row with no capture time also counted as sitting, because the zero time is January of year 1 and every span from it clears ten days. Rank already guarded that and Stalls did not. Folded the open-versus-candidate partition FormatRU and Spoken each carried into one split helper. The two have to agree on where that line falls. --- internal/tasks/rank.go | 32 ++++++++++++++++---------------- internal/tasks/stall.go | 10 ++++++++-- internal/tasks/stall_test.go | 21 +++++++++++++++++++++ 3 files changed, 45 insertions(+), 18 deletions(-) diff --git a/internal/tasks/rank.go b/internal/tasks/rank.go index cafdd9f..7bc525e 100644 --- a/internal/tasks/rank.go +++ b/internal/tasks/rank.go @@ -208,14 +208,7 @@ const SpokenLimit = 5 // DayPlan.Spoken is built core-side: two formatters drift, and then she says // one order and shows another. func FormatRU(ranked []Ranked) string { - var open, cands []Ranked - for _, r := range ranked { - if r.Status == StatusCandidate { - cands = append(cands, r) - } else { - open = append(open, r) - } - } + open, cands := split(ranked) if len(open) == 0 && len(cands) == 0 { return say.S(say.TasksNone, nil) } @@ -243,6 +236,20 @@ func FormatRU(ranked []Ranked) string { return b.String() } +// split separates confirmed work from candidates, preserving Rank's order +// within each half. FormatRU and Spoken have to agree on where the line falls, +// so they read it from one place. +func split(ranked []Ranked) (open, cands []Ranked) { + for _, r := range ranked { + if r.Status == StatusCandidate { + cands = append(cands, r) + } else { + open = append(open, r) + } + } + return open, cands +} + // joinRU lists up to limit tasks, then says how many are left. withReasons // attaches the parenthesised reason — candidates are listed bare, since their // due dates are Maven's reading of a mail and not something he stated. @@ -273,14 +280,7 @@ func joinRU(rs []Ranked, limit int, withReasons bool) string { // an ordinal resolves against is built here and not by a caller guessing how // the renderer split and truncated it. func Spoken(ranked []Ranked) []Ranked { - var open, cands []Ranked - for _, r := range ranked { - if r.Status == StatusCandidate { - cands = append(cands, r) - } else { - open = append(open, r) - } - } + open, cands := split(ranked) out := make([]Ranked, 0, 2*SpokenLimit) for _, group := range [][]Ranked{open, cands} { if len(group) > SpokenLimit { diff --git a/internal/tasks/stall.go b/internal/tasks/stall.go index 63ff9ad..08f7015 100644 --- a/internal/tasks/stall.go +++ b/internal/tasks/stall.go @@ -56,10 +56,16 @@ func Stalls(items []Item, now time.Time) []Stall { // meant to act on. continue } - if it.Due != nil && it.Due.Before(now) { + // Whole calendar days, the reading Rank already uses. An instant + // comparison calls a task due at 18:00 overdue from 18:01, so the count + // above the table would say "просрочено" beside a row whose own reason + // still said "сегодня". + if it.Due != nil && dayDelta(*it.Due, now) < 0 { overdue++ } - if now.Sub(it.Created) >= StallDays*24*time.Hour { + // A row with no capture time has not sat for anything. Without the + // guard its zero time is January of year 1 and it always counts. + if !it.Created.IsZero() && now.Sub(it.Created) >= StallDays*24*time.Hour { sitting++ } } diff --git a/internal/tasks/stall_test.go b/internal/tasks/stall_test.go index 7a3ddec..d082c55 100644 --- a/internal/tasks/stall_test.go +++ b/internal/tasks/stall_test.go @@ -54,6 +54,27 @@ func TestStallsSaysNothingWhenThereIsNothing(t *testing.T) { } } +func TestStallsCountsOverdueByCalendarDay(t *testing.T) { + // Same reading as Rank, or the count above the table contradicts the reason + // in the row: due at 09:00, asked at 12:00, and it is still due today. + now := stallNow() + earlier := now.Add(-3 * time.Hour) + items := []Item{{ID: 1, Text: "оплатить интернет", Status: StatusOpen, Created: now.Add(-time.Hour), Due: &earlier}} + if got := Stalls(items, now); len(got) != 0 { + t.Fatalf("shapes = %+v, want none — a task due today is not overdue", got) + } +} + +func TestStallsIgnoresATaskWithNoCaptureTime(t *testing.T) { + // The zero time is January of year 1, so an unstamped row would count as + // sitting forever. + now := stallNow() + items := []Item{{ID: 1, Text: "купить молоко", Status: StatusOpen}} + if got := Stalls(items, now); len(got) != 0 { + t.Fatalf("shapes = %+v, want none", got) + } +} + func TestStallsCountsNoJudgement(t *testing.T) { // The line this shape may not cross. Every sentence states a count; none of // them says whether the work matters or should be dropped. From 5447f08c0698c49ea278f7697a7dd3b632e6a3f5 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 6 Aug 2026 03:12:36 +0400 Subject: [PATCH 2/2] morning, routine: reject the two configs that silently do nothing (V-581) Both Due functions key their last-fired map by routine name, so two routines sharing a name took turns suppressing each other and one of them never fired. Validate now rejects a duplicate name in either package. parseHHMM checked the digits arithmetically, which let a stray character cancel out: window_start of 2 :00 loaded as 04:00 and passed the validation that exists to catch that typo. Each of the four positions is now checked as a digit, which makes the negative bounds unreachable and they are gone. Folded the three copies of the unevidenced-item loop in Evaluate, Outstanding and Due into one helper. --- internal/morning/morning.go | 54 +++++++++++++++++++++----------- internal/morning/morning_test.go | 13 ++++++++ internal/morning/plan.go | 8 ++--- internal/routine/routine.go | 13 ++++++-- internal/routine/routine_test.go | 6 ++++ 5 files changed, 69 insertions(+), 25 deletions(-) diff --git a/internal/morning/morning.go b/internal/morning/morning.go index 418ee44..3a55488 100644 --- a/internal/morning/morning.go +++ b/internal/morning/morning.go @@ -104,14 +104,22 @@ func OptionalOnly(missing []Item) []Item { } // 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 -// typo surfaces at startup, not as a silently-broken checklist at runtime. +// name/items, an unparseable HH:MM, an inverted window, a duplicate routine or +// item key, or an out-of-range weekday. Called at config load so a typo +// surfaces at startup, not as a silently-broken checklist at runtime. +// +// Routine names must be unique because Due keys its once-a-day map by name. Two +// routines sharing one would take turns suppressing each other's nudge. func Validate(routines []Routine) error { + names := make(map[string]bool, len(routines)) for _, r := range routines { if r.Name == "" { return fmt.Errorf("morning: name is required") } + if names[r.Name] { + return fmt.Errorf("morning routine %q: duplicate name", r.Name) + } + names[r.Name] = true if len(r.Items) == 0 { return fmt.Errorf("morning routine %q: at least one item is required", r.Name) } @@ -177,6 +185,17 @@ func Evaluate(r Routine, facts map[string]store.Fact, now time.Time) Status { return st } +// missing lists the items of r with no evidence in [start, now]. +func missing(r Routine, facts map[string]store.Fact, start, now time.Time) []Item { + var out []Item + for _, it := range r.Items { + if !evidenced(it, facts, start, now) { + out = append(out, it) + } + } + return out +} + // 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 @@ -191,13 +210,7 @@ func Outstanding(r Routine, facts map[string]store.Fact, now time.Time) []Item { 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 + return missing(r, facts, start, now) } // Due returns the routines that have reached their nudge time today with at @@ -222,24 +235,19 @@ func Due(routines []Routine, facts map[string]store.Fact, last map[string]time.T if !ok || now.Before(nudgeAt) { continue } - var missing []Item - for _, it := range r.Items { - if !evidenced(it, facts, start, now) { - missing = append(missing, it) - } - } + skipped := missing(r, facts, start, now) // 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 { + if len(Required(skipped)) == 0 { continue } if prev, seen := last[r.Name]; seen && sameDay(prev, now) { continue } last[r.Name] = now - out = append(out, Candidate{Routine: r, Missing: missing}) + out = append(out, Candidate{Routine: r, Missing: skipped}) } return out } @@ -284,13 +292,21 @@ func sameDay(a, b time.Time) bool { return ay == by && am == bm && ad == bd } +// parseHHMM reads a five-character "HH:MM". Every digit is checked as a digit: +// arithmetic alone lets a stray character cancel out, so "2 :00" used to load +// as 04:00 and Validate passed the typo it exists to catch. func parseHHMM(s string) (hour, min int, ok bool) { if len(s) != 5 || s[2] != ':' { return 0, 0, false } + for _, i := range [4]int{0, 1, 3, 4} { + if s[i] < '0' || s[i] > '9' { + return 0, 0, false + } + } h := int(s[0]-'0')*10 + int(s[1]-'0') m := int(s[3]-'0')*10 + int(s[4]-'0') - if h < 0 || h > 23 || m < 0 || m > 59 { + if h > 23 || m > 59 { return 0, 0, false } return h, m, true diff --git a/internal/morning/morning_test.go b/internal/morning/morning_test.go index 1a9385e..9b3338c 100644 --- a/internal/morning/morning_test.go +++ b/internal/morning/morning_test.go @@ -56,6 +56,19 @@ func TestValidate(t *testing.T) { if err := Validate([]Routine{bad}); err == nil { t.Fatal("expected error for duplicate item key") } + + // Due keys its once-a-day map by name, so two routines sharing one would + // suppress each other's nudge instead of both firing. + if err := Validate([]Routine{r, r}); err == nil { + t.Fatal("expected error for duplicate routine name") + } + + // Arithmetic alone let a stray character cancel out: "2 :00" read as 04:00. + bad = r + bad.WindowStart = "2 :00" + if err := Validate([]Routine{bad}); err == nil { + t.Fatal("expected error for a non-digit in window_start") + } } func TestEvaluateInactiveOutsideWindow(t *testing.T) { diff --git a/internal/morning/plan.go b/internal/morning/plan.go index 5129187..ee863a2 100644 --- a/internal/morning/plan.go +++ b/internal/morning/plan.go @@ -113,12 +113,12 @@ func BuildPlan(routines []Routine, facts map[string]store.Fact, events, reminder func checklistEntries(routines []Routine, facts map[string]store.Fact, now time.Time) []PlanEntry { var out []PlanEntry for _, r := range routines { - missing := Outstanding(r, facts, now) - if len(missing) == 0 { + left := Outstanding(r, facts, now) + if len(left) == 0 { continue } - labels := make([]string, 0, len(missing)) - for _, it := range missing { + labels := make([]string, 0, len(left)) + for _, it := range left { label := it.Label if label == "" { label = it.Key diff --git a/internal/routine/routine.go b/internal/routine/routine.go index 3ee082e..6955dbe 100644 --- a/internal/routine/routine.go +++ b/internal/routine/routine.go @@ -38,13 +38,22 @@ type Routine struct { } // Validate reports the first structural problem with a routine set: a missing -// name/cron/body or an unparseable cron expression. Called at config load so a -// typo surfaces at startup, not as a silently-never-firing routine at runtime. +// name/cron/body, a duplicate name or an unparseable cron expression. Called at +// config load so a typo surfaces at startup, not as a silently-never-firing +// routine at runtime. +// +// Names must be unique because Due keys its last-fired map by name. Two +// routines sharing one would take turns being suppressed by each other's fire. func Validate(routines []Routine) error { + seen := make(map[string]bool, len(routines)) for _, r := range routines { if r.Name == "" { return fmt.Errorf("routine: name is required") } + if seen[r.Name] { + return fmt.Errorf("routine %q: duplicate name", r.Name) + } + seen[r.Name] = true if r.Body == "" { return fmt.Errorf("routine %q: body is required", r.Name) } diff --git a/internal/routine/routine_test.go b/internal/routine/routine_test.go index 7ca89c5..8817cfb 100644 --- a/internal/routine/routine_test.go +++ b/internal/routine/routine_test.go @@ -60,6 +60,12 @@ func TestValidate(t *testing.T) { } }) } + + // Due keys its last-fired map by name, so two routines sharing one would + // take turns being suppressed by the other's fire. + if err := Validate(append(ok, ok[0])); err == nil { + t.Error("expected an error for a duplicate name, got nil") + } } func TestDue(t *testing.T) {