From b43ba5179df1e6d7d24c1eeb5854960e196e998a Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Sun, 31 May 2026 18:00:32 -0400 Subject: [PATCH] Orchestrate semantic nudge on the on-task path Co-Authored-By: Claude Opus 4.8 --- .../2026-05-31-m3.5-semantic-nudge-design.md | 13 +- internal/session/session.go | 115 +++++++++- internal/session/session_test.go | 206 ++++++++++++++++++ 3 files changed, 321 insertions(+), 13 deletions(-) diff --git a/docs/superpowers/specs/2026-05-31-m3.5-semantic-nudge-design.md b/docs/superpowers/specs/2026-05-31-m3.5-semantic-nudge-design.md index 677455c..9366b67 100644 --- a/docs/superpowers/specs/2026-05-31-m3.5-semantic-nudge-design.md +++ b/docs/superpowers/specs/2026-05-31-m3.5-semantic-nudge-design.md @@ -109,11 +109,14 @@ New fields on `Controller` (all reset per session in `resetDriftLocked`): - `nudgeMessage string` — the current soft advisory ("" = none). - `lastNudgedAt time.Time` — debounce timestamp. -The session-identity guard reuses the existing `driftGen` counter: it is -incremented only at session boundaries (`resetDriftLocked`), so a single -generation value correctly discards both stale drift *and* stale nudge results. -A nudge in flight when the user clicks "This is on task" / "Back to task" -remains valid — those actions do not change `driftGen` and the nudge is about +The nudge needs a session-identity guard, but `driftGen` cannot serve it: +`driftGen` is bumped on every drift-judgment launch (not only at session +boundaries), so an interleaved drift judgment within the same session would +wrongly discard a valid in-flight nudge. The nudge therefore uses a dedicated +`sessionGen` counter, incremented only in `resetDriftLocked`, so it survives an +interleaved drift judgment yet is still discarded when the session ends. A nudge +in flight when the user clicks "This is on task" / "Back to task" likewise +remains valid — those actions do not change `sessionGen` and the nudge is about the session trajectory, not a single class. New constants alongside the drift ones: diff --git a/internal/session/session.go b/internal/session/session.go index 870cb68..170071d 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -40,8 +40,12 @@ const ( const ( driftDebounce = 10 * time.Second driftTimeout = 30 * time.Second + nudgeDebounce = 5 * time.Minute + nudgeTimeout = 30 * time.Second ) +const recentTitlesMax = 10 + const ( driftIdle = "idle" driftPending = "pending" @@ -93,8 +97,14 @@ type Controller struct { driftStatus string driftReason string driftGen int + sessionGen int // bumped only at session boundaries; nudge staleness guard lastJudgedAt time.Time judgedClasses map[string]ai.Verdict + + nudge ai.Nudger + recentTitles []string // in-memory ring of recent distinct titles this session + nudgeMessage string // current soft advisory ("" = none) + lastNudgedAt time.Time } // CommitmentView is the UI projection of the active commitment. @@ -114,10 +124,13 @@ type ProposalView struct { AllowedWindowClasses []string `json:"allowed_window_classes,omitempty"` } -// DriftView is the active-only drift projection. +// DriftView is the active-only drift projection. Nudge is a separate axis from +// Status: it is populated precisely when Status is "ontask" (semantic drift +// inside an allowed app), so it cannot be folded into the status enum. type DriftView struct { Status string `json:"status"` Reason string `json:"reason,omitempty"` + Nudge string `json:"nudge,omitempty"` } type CoachView struct { @@ -272,7 +285,7 @@ func (c *Controller) stateLocked() State { if status == "" { status = driftIdle } - st.Drift = &DriftView{Status: status, Reason: c.driftReason} + st.Drift = &DriftView{Status: status, Reason: c.driftReason, Nudge: c.nudgeMessage} } return st } @@ -347,13 +360,41 @@ func (c *Controller) SetDriftJudge(j ai.DriftJudge) { c.mu.Unlock() } +// SetNudge injects the AI semantic nudge judge. A nil nudger disables nudging; +// local matching and the drift judge are unaffected. +func (c *Controller) SetNudge(n ai.Nudger) { + c.mu.Lock() + c.nudge = n + c.mu.Unlock() +} + +// commitmentLineLocked renders the active commitment as a single line for AI +// prompts, or "" if there is none. Caller holds mu. +func (c *Controller) commitmentLineLocked() string { + if c.commitment == nil { + return "" + } + return c.commitment.NextAction + " — " + c.commitment.SuccessCondition +} + +// recentTitlesForTest returns a copy of the recent-titles ring (test-only). +func (c *Controller) recentTitlesForTest() []string { + c.mu.Lock() + defer c.mu.Unlock() + return append([]string(nil), c.recentTitles...) +} + // resetDriftLocked clears all drift machinery for a fresh session. Caller holds mu. func (c *Controller) resetDriftLocked() { c.driftStatus = driftIdle c.driftReason = "" c.driftGen++ + c.sessionGen++ c.lastJudgedAt = time.Time{} c.judgedClasses = map[string]ai.Verdict{} + c.recentTitles = nil + c.nudgeMessage = "" + c.lastNudgedAt = time.Time{} } // OnTask appends the current window class to the session allowed-context, clears @@ -594,6 +635,7 @@ func (c *Controller) RecordWindow(snap evidence.WindowSnapshot) { now := c.clock() _ = store.AppendFocus(c.sessionsDir, c.stats.SessionID, focusEvent(now, snap)) c.applyEvent(now, snap) + c.recordTitleLocked(snap.Title) launch := c.evaluateDriftLocked(now, snap) c.mu.Unlock() if launch != nil { @@ -610,12 +652,14 @@ func (c *Controller) RecordWindow(snap evidence.WindowSnapshot) { func (c *Controller) evaluateDriftLocked(now time.Time, snap evidence.WindowSnapshot) func() { class, title := snap.Class, snap.Title - // 1. Local match: authoritative on-task, no LLM. + // 1. Local match: authoritative on-task, no LLM. This is also the ONLY path + // where the semantic nudge runs — the drift judge stays silent here, so the + // two are mutually exclusive per observation. ac := domain.AllowedContext{WindowClasses: c.allowedClasses} if evidence.MatchesAllowed(ac, class, title) { c.driftStatus = driftOnTask c.driftReason = "" - return nil + return c.maybeNudgeLocked(now) } // 2. Per-class cache. @@ -643,10 +687,7 @@ func (c *Controller) evaluateDriftLocked(now time.Time, snap evidence.WindowSnap c.driftStatus = driftPending c.driftReason = "" judge := c.judge - commitment := "" - if c.commitment != nil { - commitment = c.commitment.NextAction + " — " + c.commitment.SuccessCondition - } + commitment := c.commitmentLineLocked() return func() { ctx, cancel := context.WithTimeout(context.Background(), driftTimeout) @@ -678,6 +719,64 @@ func (c *Controller) evaluateDriftLocked(now time.Time, snap evidence.WindowSnap } } +// maybeNudgeLocked decides whether to launch a semantic nudge on the on-task +// local-match path. It returns the nudging closure when eligible (judge wired, +// at least two titles of history, debounce elapsed), else nil. The closure +// follows the drift-judge discipline: it runs in a goroutine after the caller +// releases the mutex, re-acquires the lock, guards on sessionGen (a session-only +// counter, so an interleaved drift judgment within the same session does not +// discard the nudge), never fabricates a concern on error, and notifies only +// after unlocking. Caller holds mu and has already verified the runtime is Active. +func (c *Controller) maybeNudgeLocked(now time.Time) func() { + if c.nudge == nil || len(c.recentTitles) < 2 { + return nil + } + if !c.lastNudgedAt.IsZero() && now.Sub(c.lastNudgedAt) < nudgeDebounce { + return nil + } + c.lastNudgedAt = now + gen := c.sessionGen + nudge := c.nudge + commitment := c.commitmentLineLocked() + titles := append([]string(nil), c.recentTitles...) + + return func() { + ctx, cancel := context.WithTimeout(context.Background(), nudgeTimeout) + defer cancel() + msg, err := nudge.Nudge(ctx, commitment, titles) + + c.mu.Lock() + if gen != c.sessionGen || c.runtimeState != domain.RuntimeActive { + c.mu.Unlock() + return // stale: a new session started or the session ended + } + if err != nil { + c.mu.Unlock() + log.Printf("session: nudge failed: %v", err) + return // never fabricate a concern; leave any prior message intact + } + c.nudgeMessage = msg // "" clears a prior advisory once back on trajectory + c.mu.Unlock() + c.notify() + } +} + +// recordTitleLocked appends a non-empty title to the recent ring, skipping a +// consecutive duplicate, and caps the ring at recentTitlesMax. Caller holds mu. +func (c *Controller) recordTitleLocked(title string) { + t := strings.TrimSpace(title) + if t == "" { + return + } + if n := len(c.recentTitles); n > 0 && c.recentTitles[n-1] == t { + return + } + c.recentTitles = append(c.recentTitles, t) + if len(c.recentTitles) > recentTitlesMax { + c.recentTitles = c.recentTitles[len(c.recentTitles)-recentTitlesMax:] + } +} + // applyVerdictLocked maps a verdict onto drift state. Caller holds mu. func (c *Controller) applyVerdictLocked(v ai.Verdict) { if v.OnTask { diff --git a/internal/session/session_test.go b/internal/session/session_test.go index fcd3933..eb3c632 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -3,6 +3,7 @@ package session import ( "context" "errors" + "fmt" "path/filepath" "sync/atomic" "testing" @@ -549,3 +550,208 @@ func TestLeavingPlanningClearsCoach(t *testing.T) { t.Fatal("coach view must be gone once Active") } } + +// fakeNudger is a controllable ai.Nudger for tests. An optional gate channel +// blocks the call until closed, mirroring fakeJudge. +type fakeNudger struct { + msg string + err error + gate chan struct{} + calls int32 +} + +func (f *fakeNudger) Nudge(ctx context.Context, commitment string, recentTitles []string) (string, error) { + atomic.AddInt32(&f.calls, 1) + if f.gate != nil { + <-f.gate + } + return f.msg, f.err +} + +// nudgerFunc adapts a function to ai.Nudger for tests needing per-call results. +type nudgerFunc func(context.Context, string, []string) (string, error) + +func (f nudgerFunc) Nudge(ctx context.Context, c string, t []string) (string, error) { + return f(ctx, c, t) +} + +func waitNudge(t *testing.T, c *Controller, want string) { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + st := c.State() + if st.Drift != nil && st.Drift.Nudge == want { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("nudge never reached %q (last: %+v)", want, c.State().Drift) +} + +func TestNudgeFiresOnOnTaskPath(t *testing.T) { + c, _ := newTestController(t) + fn := &fakeNudger{msg: "drifted to an unrelated file"} + c.SetNudge(fn) + startActive(t, c, []string{"code"}) + c.RecordWindow(obs("code", "auth.go")) // history 1 -> not yet eligible + c.RecordWindow(obs("code", "styles.css")) // history 2 -> eligible, launches + waitNudge(t, c, "drifted to an unrelated file") + if got := atomic.LoadInt32(&fn.calls); got != 1 { + t.Fatalf("expected exactly one nudge call, got %d", got) + } +} + +func TestNudgeDoesNotRunOnDriftPath(t *testing.T) { + c, _ := newTestController(t) + fn := &fakeNudger{msg: "should not run"} + c.SetNudge(fn) + c.SetDriftJudge(&fakeJudge{verdict: ai.Verdict{OnTask: false, Reason: "off-task"}}) + startActive(t, c, []string{"code"}) + c.RecordWindow(obs("firefox", "YouTube")) + c.RecordWindow(obs("firefox", "Reddit")) + waitDriftStatus(t, c, "drifting") + time.Sleep(50 * time.Millisecond) + if got := atomic.LoadInt32(&fn.calls); got != 0 { + t.Fatalf("nudge must not run on the drift path, calls=%d", got) + } +} + +func TestNudgeDebounced(t *testing.T) { + c, _ := newTestController(t) + now := time.Now() + c.SetClock(func() time.Time { return now }) + fn := &fakeNudger{msg: "drift"} + c.SetNudge(fn) + startActive(t, c, []string{"code"}) + c.RecordWindow(obs("code", "a.go")) + c.RecordWindow(obs("code", "b.go")) // fires nudge #1 + waitNudge(t, c, "drift") + c.RecordWindow(obs("code", "c.go")) // within debounce window -> suppressed + time.Sleep(50 * time.Millisecond) + if got := atomic.LoadInt32(&fn.calls); got != 1 { + t.Fatalf("debounce should allow one nudge, calls=%d", got) + } + now = now.Add(2 * nudgeDebounce) + c.RecordWindow(obs("code", "d.go")) // debounce elapsed -> nudge #2 + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) && atomic.LoadInt32(&fn.calls) < 2 { + time.Sleep(5 * time.Millisecond) + } + if got := atomic.LoadInt32(&fn.calls); got != 2 { + t.Fatalf("expected second nudge after debounce, calls=%d", got) + } +} + +func TestNudgeErrorDoesNotFabricate(t *testing.T) { + c, _ := newTestController(t) + fn := &fakeNudger{err: errors.New("backend down")} + c.SetNudge(fn) + startActive(t, c, []string{"code"}) + c.RecordWindow(obs("code", "a.go")) + c.RecordWindow(obs("code", "b.go")) + deadline := time.Now().Add(1 * time.Second) + for time.Now().Before(deadline) && atomic.LoadInt32(&fn.calls) == 0 { + time.Sleep(5 * time.Millisecond) + } + time.Sleep(50 * time.Millisecond) + if st := c.State(); st.Drift == nil || st.Drift.Nudge != "" { + t.Fatalf("error must not set a nudge message: %+v", st.Drift) + } +} + +func TestNudgeClearsWhenBackOnTrack(t *testing.T) { + c, _ := newTestController(t) + now := time.Now() + c.SetClock(func() time.Time { return now }) + msgs := make(chan string, 2) + msgs <- "drifted" + msgs <- "" // back on track + c.SetNudge(nudgerFunc(func(context.Context, string, []string) (string, error) { + return <-msgs, nil + })) + startActive(t, c, []string{"code"}) + c.RecordWindow(obs("code", "a.go")) + c.RecordWindow(obs("code", "b.go")) + waitNudge(t, c, "drifted") + now = now.Add(2 * nudgeDebounce) + c.RecordWindow(obs("code", "c.go")) + waitNudge(t, c, "") // an on-track result clears the prior advisory +} + +func TestNilNudgerStaysSilentOnTask(t *testing.T) { + c, _ := newTestController(t) + startActive(t, c, []string{"code"}) + c.RecordWindow(obs("code", "a.go")) + c.RecordWindow(obs("code", "b.go")) + time.Sleep(50 * time.Millisecond) + st := c.State() + if st.Drift == nil || st.Drift.Status != "ontask" { + t.Fatalf("expected ontask, got %+v", st.Drift) + } + if st.Drift.Nudge != "" { + t.Fatalf("nil nudger must stay silent, got %q", st.Drift.Nudge) + } +} + +func TestNudgeSurvivesInterleavedDriftJudgment(t *testing.T) { + c, _ := newTestController(t) + fn := &fakeNudger{msg: "kept", gate: make(chan struct{})} + c.SetNudge(fn) + c.SetDriftJudge(&fakeJudge{verdict: ai.Verdict{OnTask: false, Reason: "off-task"}}) + startActive(t, c, []string{"code"}) + c.RecordWindow(obs("code", "a.go")) + c.RecordWindow(obs("code", "b.go")) // nudge launches, blocks on gate + deadline := time.Now().Add(1 * time.Second) + for time.Now().Before(deadline) && atomic.LoadInt32(&fn.calls) == 0 { + time.Sleep(5 * time.Millisecond) + } + // An interleaved drift judgment bumps driftGen; the nudge must NOT be + // discarded by it — it is guarded by session identity, not driftGen. + c.RecordWindow(obs("firefox", "YouTube")) + close(fn.gate) // nudge returns within the same session + waitNudge(t, c, "kept") +} + +func TestStaleNudgeDiscardedAfterEnd(t *testing.T) { + c, _ := newTestController(t) + fn := &fakeNudger{msg: "late", gate: make(chan struct{})} + c.SetNudge(fn) + startActive(t, c, []string{"code"}) + c.RecordWindow(obs("code", "a.go")) + c.RecordWindow(obs("code", "b.go")) // launches nudge, blocks on gate + deadline := time.Now().Add(1 * time.Second) + for time.Now().Before(deadline) && atomic.LoadInt32(&fn.calls) == 0 { + time.Sleep(5 * time.Millisecond) + } + _ = c.Complete() + _ = c.End() + // Start a fresh session; the previous session's in-flight nudge must not + // leak into it. + c.SetNudge(nil) // the new session launches no nudges of its own + startActive(t, c, []string{"code"}) + c.RecordWindow(obs("code", "c.go")) + close(fn.gate) // release the stale goroutine from the first session + time.Sleep(50 * time.Millisecond) + st := c.State() + if st.RuntimeState != domain.RuntimeActive { + t.Fatalf("expected Active second session, got %s", st.RuntimeState) + } + if st.Drift == nil || st.Drift.Nudge != "" { + t.Fatalf("stale nudge must not leak into the new session: %+v", st.Drift) + } +} + +func TestRecentTitlesRingCapsAtTen(t *testing.T) { + c, _ := newTestController(t) + startActive(t, c, []string{"code"}) + for i := 0; i < 15; i++ { + c.RecordWindow(obs("code", fmt.Sprintf("file%d.go", i))) + } + titles := c.recentTitlesForTest() + if len(titles) != 10 { + t.Fatalf("ring should cap at 10, got %d", len(titles)) + } + if titles[0] != "file5.go" { + t.Fatalf("ring should drop oldest first, got first=%q", titles[0]) + } +}