From 82fd7d35a5978e5e5b401dd89128d7e5ca682947 Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Mon, 1 Jun 2026 09:20:55 -0400 Subject: [PATCH] Add M7 reflection implementation plan Co-Authored-By: Claude Opus 4.8 (1M context) --- .../plans/2026-06-01-m7-reflection.md | 1069 +++++++++++++++++ 1 file changed, 1069 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-01-m7-reflection.md diff --git a/docs/superpowers/plans/2026-06-01-m7-reflection.md b/docs/superpowers/plans/2026-06-01-m7-reflection.md new file mode 100644 index 0000000..27f22c9 --- /dev/null +++ b/docs/superpowers/plans/2026-06-01-m7-reflection.md @@ -0,0 +1,1069 @@ +# M7 — Reflection Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add the fourth AI role — the **Reviewer** — so that when a session ends the daemon reflects on it (read against recent sessions), shows a one-line recap on the Review screen, and carries a one-line takeaway forward to ground the coach on the next planning cycle. + +**Architecture:** A new leaf `ai.Reviewer` role (primitives only) mirrors Coach/DriftJudge/Nudge. The `session.Controller` fetches a reflection asynchronously on entering Review (generation-guarded, non-blocking, graceful), caches the recap, and stores a latest-wins `carryForward` that rides the existing snapshot and composes into the coach's existing free-form `grounding` string — so `ai.Coach`'s signature is unchanged. The two short lines are display data, so they cross the wire (unlike the M6 profile). Persistence is snapshot-only; no audit-chain changes. + +**Tech Stack:** Go 1.26, stdlib only (`context`, `encoding/json`, `fmt`, `strings`); `go test` for tests; vanilla JS/CSS for the UI. + +**Design spec:** `docs/superpowers/specs/2026-06-01-m7-reflection-design.md` + +--- + +## File Structure + +- **Create** `internal/ai/reflection.go` — the `Reflection` struct, `Reviewer` interface, `Service.Review`, prompt builder, parser. +- **Create** `internal/ai/reflection_test.go` — prompt + parser + service tests. +- **Modify** `internal/store/audit.go` — add exported `RecentSessions(path, n)`. +- **Modify** `internal/store/audit_test.go` — test `RecentSessions`. +- **Modify** `internal/store/store.go` — add reflection fields to `Snapshot`. +- **Modify** `internal/session/session.go` — reviewer field/constants, `SetReviewer`, `startReflectionFetchLocked`, `enterReview` hook, projection (Review + Planning), snapshot persist/restore, grounding composition in `RequestCoach`. +- **Modify** `internal/session/session_test.go` — reviewer fakes + reflection tests. +- **Modify** `cmd/antidriftd/main.go` — wire the reviewer (`ctrl.SetReviewer`). +- **Modify** `internal/web/web_test.go` — assert the reflection crosses the wire. +- **Modify** `internal/web/static/app.js` — render recap on Review, carry-forward on Planning. +- **Modify** `internal/web/static/app.css` — one styling line. +- **Modify** `README.md` — M7 Status paragraph. + +> **Note on intermediate build state:** No task changes an existing exported signature, so the tree builds green after every task. (M7 adds to the `ai`/`store`/`session` APIs; it changes none.) + +--- + +## Task 1: The `ai.Reviewer` role + +**Files:** +- Create: `internal/ai/reflection.go` +- Test: `internal/ai/reflection_test.go` + +The `ai` package is a leaf: it takes only primitive strings. The controller (Task 3) builds the `finished` and `history` strings. The existing `fakeBackend` (in `coach_test.go`, same package) is reused by the tests here — do **not** redefine it. + +- [ ] **Step 1: Write the failing tests** + +Create `internal/ai/reflection_test.go`: + +```go +package ai + +import ( + "context" + "errors" + "strings" + "testing" +) + +func TestReviewPromptIncludesSessionAndHistory(t *testing.T) { + fb := &fakeBackend{out: `{"recap":"r","carry_forward":"c"}`} + if _, err := NewService(fb).Review(context.Background(), "FINISHED-BLOCK", "HISTORY-BLOCK"); err != nil { + t.Fatalf("review: %v", err) + } + if !strings.Contains(fb.gotPrompt, "FINISHED-BLOCK") { + t.Fatalf("prompt missing finished session: %s", fb.gotPrompt) + } + if !strings.Contains(fb.gotPrompt, "HISTORY-BLOCK") || !strings.Contains(fb.gotPrompt, "Recent sessions") { + t.Fatalf("prompt missing history block: %s", fb.gotPrompt) + } +} + +func TestReviewPromptOmitsEmptyHistory(t *testing.T) { + fb := &fakeBackend{out: `{"recap":"r","carry_forward":"c"}`} + if _, err := NewService(fb).Review(context.Background(), "FINISHED-BLOCK", ""); err != nil { + t.Fatalf("review: %v", err) + } + if strings.Contains(fb.gotPrompt, "Recent sessions") { + t.Fatalf("empty history must not add a history header: %s", fb.gotPrompt) + } +} + +func TestReviewServiceParsesReflection(t *testing.T) { + fb := &fakeBackend{out: `sure: {"recap":"held focus on the port","carry_forward":"start in the editor next time"}`} + refl, err := NewService(fb).Review(context.Background(), "f", "h") + if err != nil { + t.Fatalf("review: %v", err) + } + if refl.Recap != "held focus on the port" || refl.CarryForward != "start in the editor next time" { + t.Fatalf("bad reflection: %+v", refl) + } +} + +func TestReviewServiceBackendError(t *testing.T) { + fb := &fakeBackend{err: errors.New("boom")} + if _, err := NewService(fb).Review(context.Background(), "f", "h"); err == nil { + t.Fatal("want backend error") + } +} + +func TestParseReflectionEmpty(t *testing.T) { + if _, err := parseReflection(""); !errors.Is(err, ErrEmptyResponse) { + t.Fatalf("want ErrEmptyResponse, got %v", err) + } +} + +func TestParseReflectionNoJSON(t *testing.T) { + if _, err := parseReflection("I cannot help."); !errors.Is(err, ErrNoJSON) { + t.Fatalf("want ErrNoJSON, got %v", err) + } +} + +func TestParseReflectionRequiresRecap(t *testing.T) { + if _, err := parseReflection(`{"recap":" ","carry_forward":"x"}`); !errors.Is(err, ErrInvalidReflection) { + t.Fatalf("blank recap should be invalid, got %v", err) + } +} + +func TestParseReflectionAllowsEmptyCarryForward(t *testing.T) { + refl, err := parseReflection(`{"recap":"did the thing","carry_forward":""}`) + if err != nil { + t.Fatalf("carry_forward may be empty: %v", err) + } + if refl.Recap != "did the thing" || refl.CarryForward != "" { + t.Fatalf("bad reflection: %+v", refl) + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `go test ./internal/ai/ -run Refl -v` +Expected: build failure — `undefined: parseReflection`, `undefined: ErrInvalidReflection`, `Review` not a method of `*Service`. + +- [ ] **Step 3: Write the implementation** + +Create `internal/ai/reflection.go`: + +```go +package ai + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" +) + +// Reflection is the reviewer's output: two short, single-line fields. +type Reflection struct { + Recap string // backward-looking: how the session went (shown on Review) + CarryForward string // forward-looking: one takeaway for the next session +} + +// Reviewer reflects on a just-finished session, read against recent history. It +// takes primitives, not domain/store types, so ai stays a leaf package. +// +// finished: a compact description of the session that just ended. +// history: a compact description of the last few prior sessions ("" if none). +type Reviewer interface { + Review(ctx context.Context, finished, history string) (Reflection, error) +} + +// ErrInvalidReflection marks output that parsed as JSON but lacked a usable +// recap, so callers can distinguish "no attempt" from "bad attempt". +var ErrInvalidReflection = errors.New("ai: invalid reflection") + +// Review makes Service satisfy Reviewer over the same backend as Coach. +func (s *Service) Review(ctx context.Context, finished, history string) (Reflection, error) { + out, err := s.backend.Run(ctx, buildReviewPrompt(finished, history)) + if err != nil { + return Reflection{}, err + } + return parseReflection(out) +} + +func buildReviewPrompt(finished, history string) string { + preamble := `You are a focus reviewer. A work session just ended. Reflect on it in two short lines. + +Respond with ONLY a JSON object, no prose and no code fences, exactly this shape: +{"recap": "", "carry_forward": ""} + +Rules: +- recap: one short sentence, backward-looking, grounded in the session below. +- carry_forward: one short, actionable sentence for the next session. It may reference patterns across the recent sessions if any are given. +- Keep each field to a single short sentence.` + + hist := "" + if strings.TrimSpace(history) != "" { + hist = "\n\n## Recent sessions (oldest first)\n" + history + } + return preamble + "\n\n## Session that just ended\n" + finished + hist +} + +type rawReflection struct { + Recap string `json:"recap"` + CarryForward string `json:"carry_forward"` +} + +// parseReflection extracts a Reflection from raw CLI output. A blank recap is +// rejected (ErrInvalidReflection); an empty carry_forward is allowed. +func parseReflection(s string) (Reflection, error) { + if strings.TrimSpace(s) == "" { + return Reflection{}, ErrEmptyResponse + } + if strings.IndexByte(s, '{') < 0 { + return Reflection{}, ErrNoJSON + } + jsonStr, err := extractJSON(s) + if err != nil { + return Reflection{}, fmt.Errorf("%w: %v", ErrInvalidReflection, err) + } + var raw rawReflection + if err := json.Unmarshal([]byte(jsonStr), &raw); err != nil { + return Reflection{}, fmt.Errorf("%w: %v", ErrInvalidReflection, err) + } + recap := strings.TrimSpace(raw.Recap) + if recap == "" { + return Reflection{}, ErrInvalidReflection + } + return Reflection{Recap: recap, CarryForward: strings.TrimSpace(raw.CarryForward)}, nil +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `go test ./internal/ai/ -v` +Expected: PASS (all existing `ai` tests plus the new reflection tests). + +- [ ] **Step 5: Commit** + +```bash +git add internal/ai/reflection.go internal/ai/reflection_test.go +git commit -m "Add reviewer role to the ai port + +A fourth leaf role: Review(finished, history) returns a two-line +Reflection (recap + carry_forward) over the same CLI backend as the +coach. Primitive-only, with a JSON prompt and a tolerant parser that +rejects a blank recap but allows an empty carry_forward. + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +## Task 2: `store.RecentSessions` + +**Files:** +- Modify: `internal/store/audit.go` +- Test: `internal/store/audit_test.go` + +`readSummaries` is unexported. Add a bounded, exported reader returning the last *n* summaries in **oldest-first** order. The controller formats these into the `history` string. + +- [ ] **Step 1: Write the failing test** + +Append to `internal/store/audit_test.go`: + +```go +func TestRecentSessionsReturnsLastNOldestFirst(t *testing.T) { + path := auditFixture(t) + for _, id := range []string{"a", "b", "c"} { + if err := AppendSession(path, sampleSummary("session-"+id)); err != nil { + t.Fatalf("append %s: %v", id, err) + } + } + got, err := RecentSessions(path, 2) + if err != nil { + t.Fatalf("recent: %v", err) + } + if len(got) != 2 { + t.Fatalf("want 2 summaries, got %d", len(got)) + } + if got[0].SessionID != "session-b" || got[1].SessionID != "session-c" { + t.Fatalf("want [b c] oldest-first, got [%s %s]", got[0].SessionID, got[1].SessionID) + } +} + +func TestRecentSessionsFewerThanN(t *testing.T) { + path := auditFixture(t) + _ = AppendSession(path, sampleSummary("session-a")) + got, err := RecentSessions(path, 5) + if err != nil { + t.Fatalf("recent: %v", err) + } + if len(got) != 1 || got[0].SessionID != "session-a" { + t.Fatalf("want [a], got %+v", got) + } +} + +func TestRecentSessionsMissingOrZero(t *testing.T) { + if got, err := RecentSessions(auditFixture(t), 5); err != nil || got != nil { + t.Fatalf("missing chain: want (nil,nil), got (%+v,%v)", got, err) + } + path := auditFixture(t) + _ = AppendSession(path, sampleSummary("session-a")) + if got, err := RecentSessions(path, 0); err != nil || got != nil { + t.Fatalf("n=0: want (nil,nil), got (%+v,%v)", got, err) + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `go test ./internal/store/ -run RecentSessions -v` +Expected: build failure — `undefined: RecentSessions`. + +- [ ] **Step 3: Write the implementation** + +Add to `internal/store/audit.go` (after the `readSummaries` function): + +```go +// RecentSessions returns up to n most-recent summaries from the audit chain in +// oldest-first order. A missing/empty chain or n <= 0 yields (nil, nil). +func RecentSessions(path string, n int) ([]SessionSummary, error) { + if n <= 0 { + return nil, nil + } + all, err := readSummaries(path) + if err != nil { + return nil, err + } + if len(all) == 0 { + return nil, nil + } + if len(all) > n { + all = all[len(all)-n:] + } + return all, nil +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `go test ./internal/store/ -v` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add internal/store/audit.go internal/store/audit_test.go +git commit -m "Add RecentSessions reader over the audit chain + +Exposes the last n session summaries (oldest-first) for the reviewer to +read as recent-history context. Missing/empty chain or n<=0 yields nil. + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +## Task 3: Controller reflection wiring + +**Files:** +- Modify: `internal/store/store.go` (snapshot fields) +- Modify: `internal/session/session.go` +- Test: `internal/session/session_test.go` + +This is the integration task: the reviewer is fetched on entering Review, the result is cached and rides the snapshot, the `carryForward` composes into the coach's grounding, and the projection exposes a recap (Review) and carry-forward (Planning). The fetch's generation guard is **generation-only** (no runtime-state gate) because the `carryForward` must still apply if the user clicks **End** before the reviewer returns — only a superseded *review* should be discarded. + +- [ ] **Step 1: Add the snapshot fields** + +In `internal/store/store.go`, extend the `Snapshot` struct (add the three fields after `AllowedWindowClasses`): + +```go + AllowedWindowClasses []string `json:"allowed_window_classes,omitempty"` + + ReflectionStatus string `json:"reflection_status,omitempty"` + ReflectionRecap string `json:"reflection_recap,omitempty"` + CarryForward string `json:"carry_forward,omitempty"` +} +``` + +Run: `go build ./internal/store/` +Expected: builds clean. + +- [ ] **Step 2: Write the failing controller tests** + +Append to `internal/session/session_test.go`. First add `"strings"` to the import block (it is not yet imported), placing it after `"sync/atomic"`: + +```go + "strings" + "sync" + "sync/atomic" +``` + +Then append the fakes, helpers, and tests: + +```go +type fakeReviewer struct { + refl ai.Reflection + err error + gate chan struct{} // if non-nil, Review blocks until it receives + calls int32 +} + +func (f *fakeReviewer) Review(ctx context.Context, finished, history string) (ai.Reflection, error) { + atomic.AddInt32(&f.calls, 1) + if f.gate != nil { + <-f.gate + } + return f.refl, f.err +} + +// waitReflectionStatus polls until the reflection view reaches want, or fails after 2s. +func waitReflectionStatus(t *testing.T, c *Controller, want string) State { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + st := c.State() + if st.Reflection != nil && st.Reflection.Status == want { + return st + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("reflection status never reached %q (last: %+v)", want, c.State().Reflection) + return State{} +} + +func driveToReview(t *testing.T, c *Controller) { + t.Helper() + if err := c.EnterPlanning(); err != nil { + t.Fatalf("planning: %v", err) + } + if err := c.StartManualCommitment("write plan", "plan done", 25*time.Minute, nil); err != nil { + t.Fatalf("start: %v", err) + } + if err := c.Complete(); err != nil { + t.Fatalf("complete: %v", err) + } +} + +func TestReflectionFetchedOnReview(t *testing.T) { + c, _ := newTestController(t) + c.SetReviewer(&fakeReviewer{refl: ai.Reflection{Recap: "held focus", CarryForward: "start in the editor"}}) + driveToReview(t, c) + st := waitReflectionStatus(t, c, "ready") + if st.Reflection.Recap != "held focus" { + t.Fatalf("recap not projected on Review: %+v", st.Reflection) + } +} + +func TestNoReviewerYieldsIdleReflection(t *testing.T) { + c, _ := newTestController(t) + driveToReview(t, c) + st := c.State() + if st.Reflection == nil || st.Reflection.Status != "idle" { + t.Fatalf("nil reviewer should yield idle reflection, got %+v", st.Reflection) + } + if err := c.End(); err != nil { + t.Fatalf("End must still work with no reviewer: %v", err) + } +} + +func TestCarryForwardGroundsNextCoach(t *testing.T) { + c, _ := newTestController(t) + c.SetReviewer(&fakeReviewer{refl: ai.Reflection{Recap: "ok", CarryForward: "begin with the hardest test"}}) + fc := &fakeCoach{prop: ai.Proposal{NextAction: "a", SuccessCondition: "b", TimeboxSecs: 1200}} + c.SetCoach(fc) + driveToReview(t, c) + waitReflectionStatus(t, c, "ready") + if err := c.End(); err != nil { + t.Fatalf("end: %v", err) + } + if err := c.EnterPlanning(); err != nil { + t.Fatalf("planning: %v", err) + } + if err := c.RequestCoach("ship it"); err != nil { + t.Fatalf("request coach: %v", err) + } + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) && fc.grounding() == "" { + time.Sleep(5 * time.Millisecond) + } + if g := fc.grounding(); !strings.Contains(g, "begin with the hardest test") { + t.Fatalf("carry-forward not threaded into coach grounding: %q", g) + } +} + +func TestReflectionStaleResultDiscarded(t *testing.T) { + c, _ := newTestController(t) + slow := &fakeReviewer{refl: ai.Reflection{Recap: "OLD", CarryForward: "old"}, gate: make(chan struct{})} + c.SetReviewer(slow) + driveToReview(t, c) // gen1 pending, goroutine blocks on gate + waitReflectionStatus(t, c, "pending") + + _ = c.End() // Review -> Locked; gen1 still blocked + fast := &fakeReviewer{refl: ai.Reflection{Recap: "NEW", CarryForward: "new"}} + c.SetReviewer(fast) + driveToReview(t, c) // gen2 -> ready NEW + st := waitReflectionStatus(t, c, "ready") + if st.Reflection.Recap != "NEW" { + t.Fatalf("expected NEW, got %+v", st.Reflection) + } + + close(slow.gate) // release gen1; it must be discarded + time.Sleep(50 * time.Millisecond) + if got := c.State().Reflection.Recap; got != "NEW" { + t.Fatalf("stale gen1 overwrote reflection: %q", got) + } +} + +func TestCarryForwardSurvivesRestart(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + first, err := New(path) + if err != nil { + t.Fatalf("new: %v", err) + } + first.SetReviewer(&fakeReviewer{refl: ai.Reflection{Recap: "ok", CarryForward: "lead with tests"}}) + if err := first.EnterPlanning(); err != nil { + t.Fatalf("planning: %v", err) + } + if err := first.StartManualCommitment("a", "b", 25*time.Minute, nil); err != nil { + t.Fatalf("start: %v", err) + } + if err := first.Complete(); err != nil { + t.Fatalf("complete: %v", err) + } + waitReflectionStatus(t, first, "ready") + if err := first.End(); err != nil { + t.Fatalf("end: %v", err) + } + + second, err := New(path) + if err != nil { + t.Fatalf("reopen: %v", err) + } + if err := second.EnterPlanning(); err != nil { + t.Fatalf("planning: %v", err) + } + st := second.State() + if st.Reflection == nil || st.Reflection.CarryForward != "lead with tests" { + t.Fatalf("carry-forward not restored onto planning: %+v", st.Reflection) + } +} +``` + +- [ ] **Step 3: Run the tests to verify they fail** + +Run: `go test ./internal/session/ -run 'Reflection|CarryForward|Reviewer' -v` +Expected: build failure — `undefined: (*Controller).SetReviewer`, `st.Reflection` undefined. + +- [ ] **Step 4: Add reflection constants** + +In `internal/session/session.go`, add `"fmt"` to the import block (after `"errors"`): + +```go + "context" + "errors" + "fmt" + "log" +``` + +Then add the constants after the `knowledge` const block (after line ~59): + +```go +const reflectionTimeout = 30 * time.Second + +const reflectionHistoryN = 5 + +const ( + reflectionIdle = "idle" + reflectionPending = "pending" + reflectionReady = "ready" + reflectionAbsent = "absent" +) +``` + +- [ ] **Step 5: Add controller fields** + +In the `Controller` struct, add a reflection block after the `knowledge` block (after `knowledgeGen int`): + +```go + reviewer ai.Reviewer + reflectionStatus string + reflectionRecap string + carryForward string // latest-wins takeaway; grounds the next coach + reflectionGen int +``` + +- [ ] **Step 6: Add the `ReflectionView` and the `State` field** + +After the `KnowledgeView` type, add: + +```go +// ReflectionView projects the reviewer's output. Recap is shown on Review; +// CarryForward is shown on the next Planning screen. Unlike the knowledge +// profile, these short lines exist to be displayed, so they cross the wire. +type ReflectionView struct { + Status string `json:"status,omitempty"` + Recap string `json:"recap,omitempty"` + CarryForward string `json:"carry_forward,omitempty"` +} +``` + +In the `State` struct, add the field after `Knowledge`: + +```go + Knowledge *KnowledgeView `json:"knowledge,omitempty"` + Reflection *ReflectionView `json:"reflection,omitempty"` + Drift *DriftView `json:"drift,omitempty"` +``` + +- [ ] **Step 7: Restore reflection fields in `New`** + +In `New`, extend the `Controller` literal (after `outcomePending: s.OutcomePending,`): + +```go + clock: time.Now, + outcomePending: s.OutcomePending, + reflectionStatus: s.ReflectionStatus, + reflectionRecap: s.ReflectionRecap, + carryForward: s.CarryForward, + } +``` + +- [ ] **Step 8: Persist reflection fields in `persistLocked`** + +In `persistLocked`, after `snap.AllowedWindowClasses = c.allowedClasses`: + +```go + snap.AllowedWindowClasses = c.allowedClasses + snap.ReflectionStatus = c.reflectionStatus + snap.ReflectionRecap = c.reflectionRecap + snap.CarryForward = c.carryForward + return store.Save(c.snapshotPath, snap) +``` + +- [ ] **Step 9: Project reflection in `stateLocked`** + +Inside `stateLocked`, in the `if c.runtimeState == domain.RuntimePlanning {` block, after the knowledge projection (`st.Knowledge = ...` closing brace), add: + +```go + if c.carryForward != "" { + st.Reflection = &ReflectionView{CarryForward: c.carryForward} + } + } +``` + +Then, after that planning block closes (before `if c.runtimeState == domain.RuntimeActive {`), add a Review block: + +```go + if c.runtimeState == domain.RuntimeReview { + rstatus := c.reflectionStatus + if rstatus == "" { + rstatus = reflectionIdle + } + st.Reflection = &ReflectionView{Status: rstatus, Recap: c.reflectionRecap} + } +``` + +- [ ] **Step 10: Add `SetReviewer`, `startReflectionFetchLocked`, and the input builders** + +After the `SetKnowledge`/`SetKnowledgePath`/`startKnowledgeFetchLocked` group (after `startKnowledgeFetchLocked` closes, ~line 539), add: + +```go +// SetReviewer injects the AI reviewer. A nil reviewer keeps reflection idle and +// leaves the coach ungrounded by any carry-forward. +func (c *Controller) SetReviewer(r ai.Reviewer) { + c.mu.Lock() + c.reviewer = r + c.mu.Unlock() +} + +// startReflectionFetchLocked kicks off an asynchronous reflection when a +// reviewer is set, on entering Review. Unlike the tasks/knowledge fetches, the +// completion guard is generation-only (not state-gated): the carry-forward must +// still apply if the user clicks End before the reviewer returns. A superseded +// review (a later session's fetch) bumps the generation and discards this one. +// The recap and carry-forward are cleared up front so a failed/slow reviewer +// never leaves stale data from the previous session. Caller holds mu. +func (c *Controller) startReflectionFetchLocked() { + c.reflectionRecap = "" + c.carryForward = "" + if c.reviewer == nil { + c.reflectionStatus = reflectionIdle + return + } + c.reflectionGen++ + gen := c.reflectionGen + c.reflectionStatus = reflectionPending + reviewer := c.reviewer + finished := c.buildReflectionFinishedLocked() + history := buildReflectionHistory(c.auditPath) // small, bounded file read + go func() { + ctx, cancel := context.WithTimeout(context.Background(), reflectionTimeout) + defer cancel() + refl, err := reviewer.Review(ctx, finished, history) + + c.mu.Lock() + if gen != c.reflectionGen { + c.mu.Unlock() + return // superseded review: discard + } + if err != nil || strings.TrimSpace(refl.Recap) == "" { + c.reflectionStatus = reflectionAbsent + c.reflectionRecap = "" + c.carryForward = "" + } else { + c.reflectionStatus = reflectionReady + c.reflectionRecap = refl.Recap + c.carryForward = refl.CarryForward + } + _ = c.persistLocked() + c.mu.Unlock() + c.notify() + }() +} + +// buildReflectionFinishedLocked renders the just-finished session as a compact +// block for the reviewer. Caller holds mu; c.stats/c.commitment are still set +// (End clears them, but enterReview runs before End). Reuses bucketViews for the +// per-window totals, already sorted desc by seconds. +func (c *Controller) buildReflectionFinishedLocked() string { + var na, sc string + if c.commitment != nil { + na, sc = c.commitment.NextAction, c.commitment.SuccessCondition + } + outcome := c.outcomePending + if outcome == "" { + outcome = "completed" + } + var b strings.Builder + fmt.Fprintf(&b, "Next action: %s\n", na) + fmt.Fprintf(&b, "Success condition: %s\n", sc) + fmt.Fprintf(&b, "Outcome: %s\n", outcome) + if c.stats != nil { + fmt.Fprintf(&b, "Context switches: %d\n", c.stats.SwitchCount) + for i, bv := range bucketViews(c.stats.Buckets) { + if i >= 3 { + break + } + fmt.Fprintf(&b, "- %s · %s: %dm\n", bv.Class, bv.Title, bv.Seconds/60) + } + } + return strings.TrimRight(b.String(), "\n") +} + +// buildReflectionHistory renders the last few prior sessions as compact lines. +// The just-finished session is not yet in the chain (End appends it), so it is +// not double-counted. Returns "" when there is no usable history. +func buildReflectionHistory(auditPath string) string { + sums, err := store.RecentSessions(auditPath, reflectionHistoryN) + if err != nil || len(sums) == 0 { + return "" + } + var b strings.Builder + for _, s := range sums { + top := "" + if len(s.Buckets) > 0 { + top = fmt.Sprintf(", top %s %dm", s.Buckets[0].Class, s.Buckets[0].Seconds/60) + } + fmt.Fprintf(&b, "- %s: %s (%d switches%s)\n", s.Outcome, s.NextAction, s.SwitchCount, top) + } + return strings.TrimRight(b.String(), "\n") +} +``` + +- [ ] **Step 11: Fire the fetch on entering Review** + +In `enterReview`, insert the fetch after `c.outcomePending = outcome` and before the final `return c.persistLocked()`: + +```go + c.runtimeState = next + c.outcomePending = outcome + c.startReflectionFetchLocked() + return c.persistLocked() +``` + +- [ ] **Step 12: Compose the carry-forward into the coach grounding** + +Add a helper near `RequestCoach`: + +```go +// composedGroundingLocked combines the standing profile (knowledge port) with +// the latest carry-forward takeaway into the single free-form grounding string +// the coach already accepts. Caller holds mu. +func (c *Controller) composedGroundingLocked() string { + g := c.knowledgeText + if c.carryForward != "" { + if g != "" { + g += "\n\n" + } + g += "Last session's takeaway: " + c.carryForward + } + return g +} +``` + +Then in `RequestCoach`, replace the line `grounding := c.knowledgeText` with: + +```go + grounding := c.composedGroundingLocked() +``` + +- [ ] **Step 13: Run the tests to verify they pass** + +Run: `go test ./internal/session/ -run 'Reflection|CarryForward|Reviewer' -v` +Expected: PASS (5 new tests). + +- [ ] **Step 14: Run the full session + store + ai suites with the race detector** + +Run: `go test -race ./internal/session/ ./internal/store/ ./internal/ai/` +Expected: PASS. (`TestCoachReceivesCachedGrounding` still passes: with no reviewer set, `carryForward` is "" and the composed grounding equals the profile text exactly.) + +- [ ] **Step 15: Commit** + +```bash +git add internal/store/store.go internal/session/session.go internal/session/session_test.go +git commit -m "Reflect on entering Review and ground the next coach + +The controller fetches a reflection asynchronously on enterReview +(generation-guarded, non-blocking, graceful) and caches a one-line +recap plus a latest-wins carry-forward. The recap projects onto Review, +the carry-forward onto the next Planning, and both ride the snapshot. +RequestCoach composes the carry-forward into the coach's existing +free-form grounding string, so ai.Coach is unchanged. + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +## Task 4: Daemon wiring + web payload test + +**Files:** +- Modify: `cmd/antidriftd/main.go` +- Test: `internal/web/web_test.go` + +- [ ] **Step 1: Write the failing web test** + +Append to `internal/web/web_test.go`: + +```go +type stubReviewer struct { + refl ai.Reflection +} + +func (s stubReviewer) Review(ctx context.Context, finished, history string) (ai.Reflection, error) { + return s.refl, nil +} + +func TestReflectionFlowsToReviewThenPlanning(t *testing.T) { + s := newTestServer(t) + s.ctrl.SetReviewer(stubReviewer{refl: ai.Reflection{Recap: "held focus well", CarryForward: "start in the editor"}}) + r := s.Router() + _ = post(t, r, "/planning", "") + body := `{"next_action":"a","success_condition":"b","timebox_secs":1500}` + if w := post(t, r, "/commitment", body); w.Code != http.StatusOK { + t.Fatalf("/commitment code %d body %s", w.Code, w.Body.String()) + } + if w := post(t, r, "/complete", ""); w.Code != http.StatusOK { + t.Fatalf("/complete code %d", w.Code) + } + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if rv := s.ctrl.State().Reflection; rv != nil && rv.Status == "ready" { + break + } + time.Sleep(5 * time.Millisecond) + } + js := s.stateJSON() + if !strings.Contains(js, `"recap":"held focus well"`) { + t.Fatalf("review payload missing recap: %s", js) + } + + // End -> Locked -> Planning: the carry-forward should surface on planning. + if w := post(t, r, "/end", ""); w.Code != http.StatusOK { + t.Fatalf("/end code %d", w.Code) + } + if w := post(t, r, "/planning", ""); w.Code != http.StatusOK { + t.Fatalf("/planning code %d", w.Code) + } + js2 := s.stateJSON() + if !strings.Contains(js2, `"carry_forward":"start in the editor"`) { + t.Fatalf("planning payload missing carry-forward: %s", js2) + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `go test ./internal/web/ -run Reflection -v` +Expected: build failure — `s.ctrl.SetReviewer` undefined is already resolved by Task 3, so the failure here is the missing daemon wiring is **not** what this tests; the test should actually compile and PASS already (the controller wiring from Task 3 is enough). If it PASSES, that is expected — proceed to Step 3 to add the daemon wiring (which the test does not exercise). If it FAILS to compile, re-check Task 3 was applied. + +> Rationale: the web payload is produced by the controller, which Task 3 already wired. This test guards the wire format. The daemon change in Step 3 is the production wiring (`main.go`) that no test exercises, mirroring how the M6 tasks/knowledge adapters are wired. + +- [ ] **Step 3: Wire the reviewer in `main.go`** + +In `cmd/antidriftd/main.go`, in the AI block, add `ctrl.SetReviewer(svc)` alongside the other roles and update the log line: + +```go + svc := ai.NewService(backend) + ctrl.SetCoach(svc) + ctrl.SetDriftJudge(svc) + ctrl.SetNudge(svc) + ctrl.SetReviewer(svc) + log.Printf("ai: %s backend (coach + drift judge + nudge + reviewer)", backend.Name()) +``` + +- [ ] **Step 4: Run the test + build to verify** + +Run: `go test ./internal/web/ -run Reflection -v && go build ./...` +Expected: test PASS; build clean. + +- [ ] **Step 5: Commit** + +```bash +git add cmd/antidriftd/main.go internal/web/web_test.go +git commit -m "Wire the reviewer and assert reflection on the wire + +main injects the AI service as the reviewer alongside the other roles. +A web test drives a session to Review and asserts the recap rides the +state payload, then to the next Planning and asserts the carry-forward +does too. + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +## Task 5: Planning + Review UI + +**Files:** +- Modify: `internal/web/static/app.js` +- Modify: `internal/web/static/app.css` +- Modify: `README.md` + +No JS test harness exists in this repo (consistent with M2–M6); this task is verified by `go build`/`go vet` and a human visual check. Keep the changes small and presentational. + +- [ ] **Step 1: Add the two render helpers** + +In `internal/web/static/app.js`, after `updatePlanningKnowledge` (ends ~line 180), add: + +```js +// reflectionBlock renders the reviewer's recap on the Review screen. idle/nil or +// an empty recap renders nothing; pending shows a quiet line; ready shows the +// one-line recap. +function reflectionBlock(refl) { + if (!refl) return ''; + if (refl.status === 'pending') return `
reflecting…
`; + if (refl.status === 'ready' && refl.recap) { + return `
${refl.recap}
`; + } + return ''; +} + +// updatePlanningReflection renders last session's carry-forward takeaway as a +// quiet one-liner on the planning screen. Nothing renders without one. +function updatePlanningReflection(refl) { + const el = document.getElementById('reflectBand'); + if (!el) return; + if (!refl || !refl.carry_forward) { el.innerHTML = ''; return; } + el.innerHTML = `Last time: ${refl.carry_forward}`; +} +``` + +- [ ] **Step 2: Call the planning helper in both planning render paths** + +In `render`, the incremental planning branch currently reads: + +```js + if (rs === 'planning' && renderedState === 'planning') { + updatePlanningCoach(state.coach); + updatePlanningTasks(state.tasks); + updatePlanningKnowledge(state.knowledge); + return; + } +``` + +Add the reflection call: + +```js + if (rs === 'planning' && renderedState === 'planning') { + updatePlanningCoach(state.coach); + updatePlanningTasks(state.tasks); + updatePlanningKnowledge(state.knowledge); + updatePlanningReflection(state.reflection); + return; + } +``` + +In the full planning render (the `} else if (rs === 'planning') {` block), add the `reflectBand` div after `knowBand`: + +```js +
+
+
+``` + +and add the call alongside the other planning updaters at the end of that block: + +```js + updatePlanningCoach(state.coach); + updatePlanningTasks(state.tasks); + updatePlanningKnowledge(state.knowledge); + updatePlanningReflection(state.reflection); +``` + +- [ ] **Step 3: Render the recap on the Review screen** + +In the `} else if (rs === 'review') {` block, insert `reflectionBlock` before `reviewSummary`: + +```js + ${reflectionBlock(state.reflection)} + ${reviewSummary(state.evidence)} +``` + +- [ ] **Step 4: Add the styling line** + +In `internal/web/static/app.css`, after the `.knowline { opacity: 0.85; }` line, add: + +```css +.reflectline { opacity: 0.85; } +``` + +- [ ] **Step 5: Update the README Status section** + +In `README.md`, add a new paragraph at the top of the `## Status` section (above the M6 paragraph): + +```markdown +M7 (reflection): when a session ends, a fourth AI role — the reviewer — +reflects on it, read against your recent sessions, and produces two short +lines: a recap shown on the Review screen, and a carry-forward takeaway that +grounds the coach the next time you plan. It runs once asynchronously on +entering Review, never blocks the End button, and degrades gracefully — with +no backend (or a slow/failed call) Review and Planning behave exactly as +before. The carry-forward is snapshot-persisted (latest-wins) and composes +into the coach's grounding; the reflection lines are short and cross the wire +by design, while the knowledge profile still does not. +``` + +- [ ] **Step 6: Verify build and vet** + +Run: `go build ./... && go vet ./... && go test ./...` +Expected: all PASS. + +- [ ] **Step 7: Commit** + +```bash +git add internal/web/static/app.js internal/web/static/app.css README.md +git commit -m "Show the recap on Review and the carry-forward on Planning + +The Review screen renders the reviewer's one-line recap (quiet pending +line, then the recap); the Planning screen renders last session's +carry-forward as a 'Last time: …' one-liner, mirroring the knowledge +indicator. Updates the README Status section for M7. + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +## Final Verification + +After all tasks, confirm the milestone holds end-to-end: + +- [ ] `go build ./...` — clean. +- [ ] `go vet ./...` — clean. +- [ ] `go test -race ./...` — all packages PASS. +- [ ] **Spec invariants:** + - `ai.Coach`'s signature is unchanged (grep: `func.*Coach(ctx context.Context, intent, grounding string)` still the only Coach signature). + - No new file or format under `~/.antidrift` beyond snapshot fields (grep `audit.go` for no new write paths; `RecentSessions` is read-only). + - The reflection recap/carry-forward appear in `State` JSON; the knowledge profile text still does not (`TestPlanningStatePayloadCarriesKnowledge` still passes). + - End is never blocked on the reviewer (`TestNoReviewerYieldsIdleReflection` and the stale-discard test cover the fast-End path). + +--- + +## Self-Review Notes + +- **Spec coverage:** new role (Task 1) ✓; recent-history input via `RecentSessions` (Task 2) ✓; fetch-on-Review + generation guard + graceful + non-blocking + snapshot persistence + grounding composition + projection (Task 3) ✓; daemon wiring + wire-format (Task 4) ✓; Review recap + Planning carry-forward UI + README (Task 5) ✓; "out of scope" items (no reflections.jsonl, no Coach signature change, no session.go refactor) are respected. +- **Type consistency:** `Reflection{Recap, CarryForward}`, `ReflectionView{Status, Recap, CarryForward}`, `Snapshot.{ReflectionStatus, ReflectionRecap, CarryForward}`, and controller fields `reflectionStatus/reflectionRecap/carryForward/reflectionGen/reviewer` are used identically across tasks. JSON keys `recap`/`carry_forward`/`status` match between the view tags and the JS/web assertions. +- **Generation guard difference:** documented in Task 3 Step 10 — generation-only (not state-gated) so the carry-forward applies even after a fast End, unlike tasks/knowledge which gate on `RuntimePlanning`.