Add faithful-reflection on/off-task split design spec
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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: <na>
|
||||||
|
Success condition: <sc>
|
||||||
|
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).
|
||||||
Reference in New Issue
Block a user