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) <noreply@anthropic.com>

internal/session/session.go

@@ -7,6 +7,7 @@ package session
 import (
 	"context"
 	"errors"
+	"fmt"
 	"log"
 	"path/filepath"
 	"sort"
@@ -58,6 +59,21 @@ const (
 	knowledgeError   = "error"
 )
 
+const reflectionTimeout = 30 * time.Second
+
+const reflectionHistoryN = 5
+
+// reflectionTopBuckets caps how many per-window time buckets the finished-session
+// block lists for the reviewer, keeping the prompt compact.
+const reflectionTopBuckets = 3
+
+const (
+	reflectionIdle    = "idle"
+	reflectionPending = "pending"
+	reflectionReady   = "ready"
+	reflectionAbsent  = "absent"
+)
+
 const (
 	driftDebounce = 10 * time.Second
 	driftTimeout  = 30 * time.Second
@@ -125,6 +141,12 @@ type Controller struct {
 	knowledgeChars  int
 	knowledgeGen    int
 
+	reviewer         ai.Reviewer
+	reflectionStatus string
+	reflectionRecap  string
+	carryForward     string // latest-wins takeaway; grounds the next coach
+	reflectionGen    int
+
 	allowedClasses []string // durable: the active session's allowed window classes
 	judge          ai.DriftJudge
 	driftStatus    string
@@ -194,6 +216,15 @@ type KnowledgeView struct {
 	Chars  int    `json:"chars,omitempty"`
 }
 
+// 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"`
+}
+
 // WindowView / BucketView / EvidenceView are the evidence projection.
 type WindowView struct {
 	Class string `json:"class"`
@@ -222,6 +253,7 @@ type State struct {
 	Coach        *CoachView          `json:"coach,omitempty"`
 	Tasks        *TasksView          `json:"tasks,omitempty"`
 	Knowledge    *KnowledgeView      `json:"knowledge,omitempty"`
+	Reflection   *ReflectionView     `json:"reflection,omitempty"`
 	Drift        *DriftView          `json:"drift,omitempty"`
 }
 
@@ -241,6 +273,9 @@ func New(snapshotPath string) (*Controller, error) {
 		sessionsDir:      filepath.Join(dir, "sessions"),
 		clock:            time.Now,
 		outcomePending:   s.OutcomePending,
+		reflectionStatus: s.ReflectionStatus,
+		reflectionRecap:  s.ReflectionRecap,
+		carryForward:     s.CarryForward,
 	}
 	if s.DeadlineUnixSecs > 0 {
 		c.deadline = time.Unix(s.DeadlineUnixSecs, 0)
@@ -354,6 +389,16 @@ func (c *Controller) stateLocked() State {
 			}
 			st.Knowledge = &KnowledgeView{Status: kstatus, Path: c.knowledgePath, Chars: c.knowledgeChars}
 		}
+		if c.carryForward != "" {
+			st.Reflection = &ReflectionView{CarryForward: c.carryForward}
+		}
+	}
+	if c.runtimeState == domain.RuntimeReview {
+		rstatus := c.reflectionStatus
+		if rstatus == "" {
+			rstatus = reflectionIdle
+		}
+		st.Reflection = &ReflectionView{Status: rstatus, Recap: c.reflectionRecap}
 	}
 	if c.runtimeState == domain.RuntimeActive {
 		status := c.driftStatus
@@ -387,6 +432,9 @@ func (c *Controller) persistLocked() error {
 		snap.SessionID = c.stats.SessionID
 	}
 	snap.AllowedWindowClasses = c.allowedClasses
+	snap.ReflectionStatus = c.reflectionStatus
+	snap.ReflectionRecap = c.reflectionRecap
+	snap.CarryForward = c.carryForward
 	return store.Save(c.snapshotPath, snap)
 }
 
@@ -538,6 +586,114 @@ func (c *Controller) startKnowledgeFetchLocked() {
 	}()
 }
 
+// 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()
+	// Read the history synchronously, here under the lock, on purpose: it must
+	// happen-before End appends the just-finished session to the audit chain, so
+	// that session is excluded from "recent history" and not double-counted (it
+	// is already carried in `finished`). Moving this into the goroutine would
+	// race with End's append and reintroduce that double-count. The read is
+	// bounded to reflectionHistoryN summaries and runs once per Review entry, not
+	// on any hot path.
+	history := buildReflectionHistory(c.auditPath)
+	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 >= reflectionTopBuckets {
+				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")
+}
+
 // AllowedClassesForTest exposes the session allowed classes for tests.
 func (c *Controller) AllowedClassesForTest() []string {
 	c.mu.Lock()
@@ -631,6 +787,20 @@ func (c *Controller) Refocus() error {
 	return c.persistLocked()
 }
 
+// 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
+}
+
 // RequestCoach starts an async coach call for intent. Returns ErrNotPlanning if
 // not in planning; otherwise never a hard error (failures surface as coach
 // state). The proposal is ephemeral and never persisted.
@@ -654,7 +824,7 @@ func (c *Controller) RequestCoach(intent string) error {
 	c.coachErr = ""
 	c.coachProposal = nil
 	coach := c.coach
-	grounding := c.knowledgeText
+	grounding := c.composedGroundingLocked()
 	c.mu.Unlock()
 	c.notify()
 
@@ -760,6 +930,7 @@ func (c *Controller) enterReview(outcome string) error {
 	}
 	c.runtimeState = next
 	c.outcomePending = outcome
+	c.startReflectionFetchLocked()
 	return c.persistLocked()
 }
 

internal/session/session_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"path/filepath"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"testing"
@@ -1029,3 +1030,150 @@ func TestCoachReceivesCachedGrounding(t *testing.T) {
 		t.Fatalf("coach grounding = %q, want the cached profile", fc.grounding())
 	}
 }
+
+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)
+	}
+}

internal/store/store.go

@@ -21,6 +21,10 @@ type Snapshot struct {
 	OutcomePending   string              `json:"outcome_pending,omitempty"`
 
 	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"`
 }
 
 // DefaultPath returns ~/.antidrift/state.json.