diff --git a/docs/superpowers/specs/2026-06-01-faithful-reflection-design.md b/docs/superpowers/specs/2026-06-01-faithful-reflection-design.md new file mode 100644 index 0000000..70b360b --- /dev/null +++ b/docs/superpowers/specs/2026-06-01-faithful-reflection-design.md @@ -0,0 +1,209 @@ +# Faithful reflection — on/off-task split — design + +**Date:** 2026-06-01 +**Status:** Approved (brainstorming), pending implementation plan + +## Problem + +When a session ends, `buildReflectionFinishedLocked` (`internal/session/roles.go`) +hands the AI reviewer the commitment, outcome, context-switch count, and the top +few time buckets as bare `class · title: Nm` lines. Nothing in that block tells +the reviewer which of those buckets were *off-task*. If 20 minutes of "firefox" +was doom-scrolling, the reviewer cannot see it and writes a charitable recap. + +The daemon already knows, minute to minute, whether the active window is on- or +off-task: `evaluateDriftLocked` maintains a live `driftStatus` of `ontask` / +`drifting` / `idle` / `pending`. But that signal is never attached to the time +buckets — buckets are pure `(class, title) → duration`. This design carries the +live drift signal into the time accounting so reflection can report on-task vs +off-task honestly. + +## Decisions (locked during brainstorming) + +1. **Faithful, per-segment classification** — tag each slice of time with the + live `driftStatus` at the moment it is credited, accumulating on-task / + off-task / unclassified durations. Not reconstructed from end-state (which + would retro-taint earlier on-task time and is only class-level, not + tab-level). +2. **Split top lists in the reflection block** — lead with on-task / off-task / + unclassified minute totals, then a top-N on-task list and a top-N off-task + list, each capped at `reflectionTopBuckets`. +3. **In-memory only; no log schema change** — the split lives in live + `EvidenceStats`. After a mid-session daemon restart, pre-restart time replays + as `unclassified` (its drift status is no longer known). Honest degrade — + never falsely on-task. The on-disk focus log format is unchanged. + +## Architecture + +### The key insight: credit-time status is already correct + +In `RecordWindow` (`internal/session/drift.go`), the call order per observation is: + +``` +applyEvent(now, snap) // credits the JUST-ENDED segment to its bucket +recordTitleLocked(...) +evaluateDriftLocked(now, snap) // reclassifies driftStatus for the NEW window +``` + +`applyEvent` credits the segment that just ended *before* `evaluateDriftLocked` +reclassifies. So at the instant a segment is credited, `c.driftStatus` still +holds *that segment's* classification. We read it directly — no extra +bookkeeping, no separate timeline. + +This holds at the only two credit sites: + +- `applyEvent` (`internal/session/stats.go`) — credits the prior segment when a + new observation arrives. +- The end-of-session flush (`internal/session/session.go:280-281`) — credits the + final open segment on the way into Review; `driftStatus` there is the current + (last) window's classification. Correct. + +## Data structures (`internal/session/stats.go`) + +`EvidenceStats` gains two per-bucket maps and one scalar. The existing `Buckets` +map (total time per bucket) is unchanged — it still feeds the live evidence +panel (`views.go`) and the persisted history summary (`store`), both untouched. +The split is purely additive. + +```go +type EvidenceStats struct { + SessionID string + StartedUnix int64 + Buckets map[bucketKey]time.Duration // total per bucket (unchanged) + OnTask map[bucketKey]time.Duration // on-task portion per bucket + OffTask map[bucketKey]time.Duration // off-task portion per bucket + unclassified time.Duration // idle/pending time, total only + + SwitchCount int + Current evidence.WindowSnapshot + lastFocusAt time.Time + lastKey bucketKey + hasLast bool +} +``` + +`OnTask` and `OffTask` are keyed per bucket because the reflection block lists +them by name. `unclassified` is a scalar because it is only ever shown as a total +(no list). The three split maps/scalar are allocated wherever `Buckets` is +allocated today: `StartManualCommitment` (`session.go:248-251`) and both +`replayStats` branches (`stats.go:45-56`). + +### Crediting helper + +A single helper centralizes crediting so both credit sites stay in sync and the +status→bucket mapping lives in one place: + +```go +// creditLocked credits duration d to bucket k: always to the total, and to the +// on/off/unclassified split per the live drift status (the classification of the +// segment being credited). Caller holds mu. +func (c *Controller) creditLocked(k bucketKey, d time.Duration) { + c.stats.Buckets[k] += d + switch c.driftStatus { + case driftOnTask: + c.stats.OnTask[k] += d + case driftDrifting: + c.stats.OffTask[k] += d + default: // driftIdle, driftPending + c.stats.unclassified += d + } +} +``` + +Status→bucket mapping: + +| `driftStatus` | bucket | +|-----------------|---------------| +| `driftOnTask` | OnTask[k] | +| `driftDrifting` | OffTask[k] | +| `driftIdle` | unclassified | +| `driftPending` | unclassified | + +`applyEvent`'s credit line and the end-of-session flush both call +`creditLocked` instead of writing `Buckets[...]` directly. + +**Restart degrade:** `replayStats` calls `applyEvent` while `driftStatus` is at +its reset value (`driftIdle`, set by `resetDriftLocked`), so all replayed +pre-restart time routes to `unclassified`. Honest — never falsely on-task. + +Invariant: for every bucket `k`, `Buckets[k] == OnTask[k] + OffTask[k] + (its +unclassified portion)`. The unclassified portion is not tracked per bucket, only +in aggregate, because it is never displayed per bucket. + +## Rendering (`internal/session/roles.go`, `buildReflectionFinishedLocked`) + +Compute the totals by summing the maps; `unclassified` is the scalar. Reuse the +existing `bucketViews` (`views.go:182`), which already sorts a +`map[bucketKey]time.Duration` descending by seconds, for each top-N list, capped +at `reflectionTopBuckets` (currently 3). A bucket worked both on- and off-task +(the user refocused) appears in both lists — its on-portion in one, off-portion +in the other. Faithful. + +Rendered block (totals always shown; a list is omitted when empty): + +``` +Next action: +Success condition: +Outcome: completed +On-task 35m / Off-task 22m / Unclassified 3m +Context switches: 14 +On-task: +- code · roles.go: 30m +- term · go test: 5m +Off-task: +- firefox · r/news: 12m +- discord · #random: 7m +``` + +A fully on-task session shows the totals line and the On-task list, and omits the +`Off-task:` block entirely (and vice versa). The minute values use the same +`Seconds/60` integer-minute rendering already used for buckets. + +## Error handling / degrade + +- **No drift judge wired:** every off-task-candidate segment stays `driftIdle` + (see `evaluateDriftLocked` step 3), so all such time is `unclassified` rather + than off-task. The reflection block then shows on-task + unclassified totals, + which is honest: without a judge we genuinely do not know. +- **Sensor unavailable:** the segment is bucketed under the existing + `unavailableTitle` key (`keyFor`, `stats.go:62-67`) and split per the + then-current `driftStatus` like any other segment. No special case. +- **Mid-session restart:** pre-restart time → `unclassified` (above). +- **`stats == nil`:** `buildReflectionFinishedLocked` already guards `c.stats != + nil` before rendering buckets; the split rendering sits inside that same guard. + +## Testing + +- **`creditLocked` unit test** (`stats_test.go` or `session_test.go`): with a + fixed `bucketKey` and duration, assert each `driftStatus` value routes to the + correct map/scalar (`driftOnTask`→`OnTask`, `driftDrifting`→`OffTask`, + `driftIdle`/`driftPending`→`unclassified`). +- **End-to-end reflection block test** (`session_test.go`): with a fake clock and + a fake `ai.DriftJudge`, drive `RecordWindow` through a sequence where some + segments match the allowlist (on-task) and some are judged not-on-task + (off-task), then transition to Review and assert `buildReflectionFinishedLocked` + output contains the correct `On-task`/`Off-task`/`Unclassified` minute totals, + the `On-task:` and `Off-task:` headers, and the expected named off-task bucket + line. +- **Empty-list omission test:** a fully on-task session renders no `Off-task:` + block. + +## Files touched + +- `internal/session/stats.go` — `EvidenceStats` split fields; allocate them; + `creditLocked` helper; route `applyEvent` through it. +- `internal/session/session.go` — allocate split maps in + `StartManualCommitment`; route the end-of-session flush through `creditLocked`. +- `internal/session/roles.go` — `buildReflectionFinishedLocked` renders totals + + split top lists. +- `internal/session/session_test.go` / `stats_test.go` — tests above. + +## Out of scope (YAGNI) + +- Persisting the split to the focus log or the audit/history summary (no + cross-restart reconstruction; history charitable-ness is a separate concern). +- Per-bucket unclassified breakdown (only the aggregate is shown). +- Any change to the live evidence panel, the drift/nudge pipeline, or the + reviewer prompt contract beyond the finished-session block text. +- Surfacing the split in the web UI (this design targets the reviewer input + only).