Merge task/532-presence-state-is-never-persisted-hyster

cmd/mavend/presence_persist_test.go

@@ -0,0 +1,95 @@
+package main
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/kami/maven/internal/loop"
+	"github.com/kami/maven/internal/store"
+)
+
+// The row is the whole mechanism, and nothing wrote it (Vikunja #532).
+//
+// The existing hysteresis test in internal/store scores the pure function and
+// passed throughout, which is exactly why this went unnoticed: Resolve was
+// always correct and was always handed the cold-start Away. So this test asserts
+// the round trip — the tick writes what gather resolved, and the next load
+// reads it back — rather than re-testing the function.
+func TestTickPersistsTheResolvedBucket(t *testing.T) {
+	ctx := context.Background()
+	st := newTestStore(t)
+	now := time.Now()
+
+	// Cold start: no row, so a load must say Away and the zero time.
+	b, score, updated, err := st.LoadPresenceState(ctx)
+	if err != nil {
+		t.Fatalf("load before: %v", err)
+	}
+	if b != store.Away || score != 0 || !updated.IsZero() {
+		t.Fatalf("cold start = %s/%v/%v, want away/0/zero", b, score, updated)
+	}
+
+	tl := &tickLoop{store: st}
+	tl.savePresence(ctx, loop.State{Presence: store.Present, PresenceScore: 0.9}, now)
+
+	b, score, updated, err = st.LoadPresenceState(ctx)
+	if err != nil {
+		t.Fatalf("load after: %v", err)
+	}
+	if b != store.Present {
+		t.Errorf("bucket = %s, want present", b)
+	}
+	if score != 0.9 {
+		t.Errorf("score = %v, want 0.9", score)
+	}
+	if updated.IsZero() {
+		t.Error("updated_ts was not written, so /dash still reads (never)")
+	}
+}
+
+// The singleton stays a singleton, and a later tick overwrites rather than
+// accumulating. A row per tick would make LoadPresenceState's single-row query
+// return whichever one SQLite felt like.
+func TestPresenceStateIsOverwrittenNotAppended(t *testing.T) {
+	ctx := context.Background()
+	st := newTestStore(t)
+	tl := &tickLoop{store: st}
+	now := time.Now()
+
+	tl.savePresence(ctx, loop.State{Presence: store.Present, PresenceScore: 0.9}, now)
+	tl.savePresence(ctx, loop.State{Presence: store.Away, PresenceScore: 0.1}, now.Add(time.Minute))
+
+	b, score, _, err := st.LoadPresenceState(ctx)
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	if b != store.Away || score != 0.1 {
+		t.Fatalf("got %s/%v, want the second write (away/0.1)", b, score)
+	}
+}
+
+// What the persisted row buys: the hold band. A score sitting between Exit and
+// Enter holds Present when the last bucket was Present, and stays Away when it
+// was Away. Before the write existed the second arm was the only one that could
+// ever run, so presence dropped at roughly four minutes of idle instead of
+// holding to the exit threshold at about nine.
+func TestPersistedBucketIsWhatFeedsHysteresis(t *testing.T) {
+	ctx := context.Background()
+	st := newTestStore(t)
+	tl := &tickLoop{store: st}
+
+	mid := (store.PresenceExit + store.PresenceEnter) / 2
+	if mid <= store.PresenceExit || mid >= store.PresenceEnter {
+		t.Fatalf("%v is not inside the hold band", mid)
+	}
+
+	tl.savePresence(ctx, loop.State{Presence: store.Present, PresenceScore: 0.9}, time.Now())
+	last, _, _, err := st.LoadPresenceState(ctx)
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+	if got := store.Resolve(mid, last); got != store.Present {
+		t.Errorf("Resolve(%v, %s) = %s, want present — the hold band did not apply", mid, last, got)
+	}
+}

cmd/mavend/tick.go

@@ -160,6 +160,7 @@ func (t *tickLoop) tick(ctx context.Context, now time.Time) {
 		log.Printf("tick: gather: %v", err)
 		return
 	}
+	t.savePresence(ctx, state, now)
 
 	// proactive: at most one candidate, max severity.
 	cand, trace := loop.ExplainTick(state, t.rules)
@@ -260,6 +261,31 @@ func (t *tickLoop) tick(ctx context.Context, now time.Time) {
 	}
 }
 
+// savePresence writes back the bucket GatherState just resolved.
+//
+// It lives here and not in GatherState because that method holds a read-only
+// transaction on purpose — one consistent snapshot per tick — and a write
+// inside it would either break that guarantee or quietly upgrade the
+// transaction. The tick is the layer that already owns writes.
+//
+// Nothing wrote this row before (Vikunja #532), and the row is the whole
+// mechanism, so two things were broken at once. Hysteresis was dead: lastBucket
+// read the cold-start Away on every tick, so store.Resolve only ever took the
+// `last == Away` arm and demanded a full PresenceEnter score to say he is
+// there. The 0.30-0.55 hold band the function exists to provide never applied
+// once. And every readout lied: /dash and ipc.Presence read this row, so they
+// showed "away — score 0.00 (never)" while desk_active facts were arriving
+// every sixty seconds.
+//
+// A failure logs and the tick continues. The gate reads the in-memory bucket,
+// which is why nudge routing kept working through all of this — losing the
+// write costs the next tick's hysteresis, not this tick's decisions.
+func (t *tickLoop) savePresence(ctx context.Context, state loop.State, now time.Time) {
+	if err := t.store.SavePresenceState(ctx, state.Presence, state.PresenceScore, now); err != nil {
+		log.Printf("tick: save presence state: %v", err)
+	}
+}
+
 // repeatableRules drops keys whose rule is not wired any more.
 //
 // The repeat path reads the nudges table, not the rule set: any sev4 telegram