Skip to content

Commit 65f0bef

Browse files
committed
Claude Code review R1: fix stale TodoState comparisons, wire promoteTodo, update spec
1 parent b710240 commit 65f0bef

9 files changed

Lines changed: 45 additions & 19 deletions

File tree

docs/specs/alarm.md

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,14 @@ Each Session owns:
4646
- Transitional states: `MIGHT_BE_BUSY`, `MIGHT_NEED_ATTENTION`.
4747
- When the user enables the alarm, status transitions from `ALARM_DISABLED` to `NOTHING_TO_SHOW` and activity tracking begins fresh from that moment.
4848
- When the user disables the alarm, activity tracking stops and status returns to `ALARM_DISABLED`.
49-
- `todo: false | 'soft' | 'hard'`
50-
- Reminder state for the Session. Default `false`.
51-
- `'soft'`: auto-created when a ringing alarm is phantom-dismissed (any attention path). Dashed-outline pill. Auto-clears when the user types printable text into the terminal (synthetic terminal reports like focus events and cursor-position responses are excluded).
52-
- `'hard'`: explicitly set by the user via `t` key or context menu. Solid-outline pill. Only clears via explicit toggle.
53-
- Dismissing a ringing alarm when `todo` is already `'soft'` or `'hard'` does not downgrade it.
49+
- `todo: TodoState` (numeric)
50+
- Reminder state for the Session. Default `TODO_OFF` (`-1`).
51+
- `TODO_OFF` (`-1`): no TODO.
52+
- `[0, 1]` (soft TODO): auto-created when a ringing alarm is phantom-dismissed (any attention path). Value is the leaky-bucket fill level (`1` = full, `0` = about to clear). Dashed-outline pill. Uses a leaky-bucket mechanism: each printable keypress drains the bucket by `1/keypressesToEmpty` (default 5 keypresses to fully drain). When typing stops, the bucket refills to full over `timeToFullSeconds` (default 3 seconds). If the bucket empties completely, the soft TODO clears. Synthetic terminal reports (focus events, cursor-position responses) do not drain the bucket.
53+
- `TODO_HARD` (`2`): explicitly set by the user via `t` key or context menu. Solid-outline pill. Only clears via explicit toggle.
54+
- Dismissing a ringing alarm when `todo` is already soft or hard does not downgrade it.
55+
- Helper functions: `isSoftTodo(todo)`, `isHardTodo(todo)`, `hasTodo(todo)`.
56+
- Leaky-bucket tuning parameters are in `cfg.todoBucket`.
5457

5558
Each Session also owns:
5659

@@ -203,7 +206,7 @@ The Session leaves `ALARM_RINGING` and returns to `NOTHING_TO_SHOW` when any of
203206
- the user marks the Session as hard TODO (`t` key or context menu)
204207
- new output arrives while the Session has attention (starts a new `MIGHT_BE_BUSY` cycle; without attention the alarm stays ringing — see latch in transition rules)
205208

206-
All attention-based dismissals (the first three above) create a soft TODO if `todo` is currently `false`. This prevents phantom dismissals where the alarm vanishes without a trace. Typing printable text into the terminal auto-clears soft TODOs, so users who engage with the output don't accumulate breadcrumbs. Synthetic terminal reports (focus events, cursor-position responses) do not count as typing.
209+
All attention-based dismissals (the first three above) create a soft TODO if `todo` is currently `TODO_OFF`. This prevents phantom dismissals where the alarm vanishes without a trace. Printable keypresses drain the soft TODO's leaky bucket, and if the bucket empties completely the soft TODO clears — so users who engage with the output don't accumulate breadcrumbs. If the user stops typing, the bucket refills over `cfg.todoBucket.timeToFullSeconds` (default 3 s). Synthetic terminal reports (focus events, cursor-position responses) do not drain the bucket.
207210

208211
The Session leaves `ALARM_RINGING` and returns to `ALARM_DISABLED` when:
209212

@@ -215,7 +218,7 @@ The Session's alarm state is cleared entirely when:
215218

216219
If more output arrives later and the Session makes a fresh transition back into `ALARM_RINGING`, the alarm rings again.
217220

218-
Marking a Session as hard TODO resets the alarm to `NOTHING_TO_SHOW` and sets `todo = 'hard'`, but it does **not** disable future alarms. `todo` and the alarm toggle are separate concerns.
221+
Marking a Session as hard TODO resets the alarm to `NOTHING_TO_SHOW` and sets `todo = TODO_HARD`, but it does **not** disable future alarms. `todo` and the alarm toggle are separate concerns.
219222

220223
Disabling alarms disposes the activity monitor and returns `status` to `ALARM_DISABLED`.
221224

@@ -230,10 +233,10 @@ The Pane header exposes two independent concepts:
230233

231234
TODO pill:
232235

233-
- toggled in command mode with `t` (cycles: `false``'hard'`, `'soft'``'hard'`, `'hard'``false`)
234-
- shown when `todo` is `'soft'` or `'hard'`
235-
- `'soft'`: dashed-outline pill — auto-created on alarm dismiss, auto-clears on user input
236-
- `'hard'`: solid-outline pill — explicitly set, only clears manually
236+
- toggled in command mode with `t` (cycles: `TODO_OFF``TODO_HARD`, soft → `TODO_HARD`, `TODO_HARD``TODO_OFF`)
237+
- shown when `hasTodo(todo)` is true (i.e. `todo !== TODO_OFF`)
238+
- soft (`isSoftTodo(todo)`): dashed-outline pill — auto-created on alarm dismiss, drains via leaky bucket on typing
239+
- `TODO_HARD` (`isHardTodo(todo)`): solid-outline pill — explicitly set, only clears manually
237240
- clicking a soft pill shows a prompt: "Clear" / "Keep" (keep promotes to hard)
238241
- clicking a hard pill clears it
239242
- no empty placeholder when off
@@ -276,7 +279,7 @@ A Door is display-only for alarm state in v1. It must not replace the existing D
276279
Door indicators:
277280

278281
- show bell indicator only when `status !== 'ALARM_DISABLED'`
279-
- show TODO pill when `todo !== false` (`'soft'` or `'hard'`)
282+
- show TODO pill when `hasTodo(todo)` (soft or hard)
280283
- if `status === 'ALARM_RINGING'`, the Door itself gets the ringing treatment, not just a tiny icon
281284
- the Door bell icon shows the same dot badge as the Pane header for `MIGHT_BE_BUSY`, `BUSY`, and `MIGHT_NEED_ATTENTION` states, but smaller (4px vs 6px) to match the smaller bell icon
282285

@@ -366,7 +369,7 @@ Consequences:
366369
- A Session rings.
367370
- User clicks into the pane to read the output.
368371
- The alarm clears, a soft TODO appears (dashed pill).
369-
- User types a command → soft TODO auto-clears (they engaged).
372+
- User types a command → printable keypresses drain the soft TODO's leaky bucket; if enough keypresses occur without long pauses, the soft TODO clears (they engaged).
370373
- The Session later emits new output, progresses through `BUSY`, and eventually reaches `ALARM_RINGING` again.
371374

372375
### User dismisses but doesn't engage

lib/src/components/Pond.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
isSoftTodo,
3737
isHardTodo,
3838
hasTodo,
39+
TODO_OFF,
3940
} from '../lib/terminal-registry';
4041
import { resolvePanelElement, findPanelInDirection, findRestoreNeighbor, type DetachDirection } from '../lib/spatial-nav';
4142
import { cloneLayout, getLayoutStructureSignature } from '../lib/layout-snapshot';
@@ -338,12 +339,12 @@ function TodoAlarmDialog({
338339
<span className="text-[10px] font-mono text-muted">[t]</span>
339340
<span className="text-[11px] text-foreground font-medium w-10">TODO</span>
340341
<div className="flex gap-1 ml-auto">
341-
<button type="button" className={toggleBtn(sessionState.todo === 'hard')}
342-
onClick={() => { if (sessionState.todo !== 'hard') markSessionTodo(sessionId); }}>
342+
<button type="button" className={toggleBtn(isHardTodo(sessionState.todo))}
343+
onClick={() => { if (!isHardTodo(sessionState.todo)) markSessionTodo(sessionId); }}>
343344
hard
344345
</button>
345-
<button type="button" className={toggleBtn(sessionState.todo === false)}
346-
onClick={() => { if (sessionState.todo !== false) clearSessionTodo(sessionId); }}>
346+
<button type="button" className={toggleBtn(sessionState.todo === TODO_OFF)}
347+
onClick={() => { if (sessionState.todo !== TODO_OFF) clearSessionTodo(sessionId); }}>
347348
off
348349
</button>
349350
</div>
@@ -369,7 +370,7 @@ function TodoAlarmDialog({
369370
<div className="border-t border-border pt-2 text-[9px] leading-relaxed text-muted">
370371
When an alarming tab is selected,<br />
371372
the alarm is cleared and the tab gets a soft TODO.<br />
372-
Typing characters into the tab will automatically clear a soft TODO.
373+
Typing drains the soft TODO; stop typing and it refills.
373374
</div>
374375
</div>,
375376
document.body,

lib/src/lib/alarm-manager.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,14 @@ export function isSoftTodo(todo: TodoState): boolean { return todo >= 0 && todo
2121
export function isHardTodo(todo: TodoState): boolean { return todo === TODO_HARD; }
2222
export function hasTodo(todo: TodoState): boolean { return todo !== TODO_OFF; }
2323

24+
/** Migrate legacy persisted TodoState values (false/'soft'/'hard') to numeric. */
25+
export function migrateTodoState(todo: unknown): TodoState {
26+
if (typeof todo === 'number') return todo;
27+
if (todo === 'hard') return TODO_HARD;
28+
if (todo === 'soft') return TODO_SOFT_FULL;
29+
return TODO_OFF; // false, null, undefined, or any other unexpected value
30+
}
31+
2432
export type AlarmButtonActionResult = 'enabled' | 'disabled' | 'dismissed' | 'noop';
2533

2634
export interface AlarmState {
@@ -355,7 +363,7 @@ export class AlarmManager {
355363
*/
356364
restore(id: string, state: { status: string; todo: TodoState }): void {
357365
const entry = this.getOrCreateEntry(id);
358-
entry.todo = state.todo;
366+
entry.todo = migrateTodoState(state.todo);
359367
// If the alarm was enabled (anything other than ALARM_DISABLED), create a monitor
360368
if (state.status !== 'ALARM_DISABLED') {
361369
if (!entry.monitor) {

lib/src/lib/platform/fake-adapter.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ export class FakePtyAdapter implements PlatformAdapter {
143143
alarmClearAttention(id?: string): void { this.alarmManager.clearAttention(id); }
144144
alarmToggleTodo(id: string): void { this.alarmManager.toggleTodo(id); }
145145
alarmMarkTodo(id: string): void { this.alarmManager.markTodo(id); }
146+
alarmPromoteTodo(id: string): void { this.alarmManager.promoteTodo(id); }
146147
alarmClearTodo(id: string): void { this.alarmManager.clearTodo(id); }
147148
alarmDrainTodoBucket(id: string): void { this.alarmManager.drainTodoBucket(id); }
148149
onAlarmState(handler: (detail: AlarmStateDetail) => void): void { this.alarmStateHandlers.add(handler); }

lib/src/lib/platform/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ export interface PlatformAdapter {
5252
alarmClearAttention(id?: string): void;
5353
alarmToggleTodo(id: string): void;
5454
alarmMarkTodo(id: string): void;
55+
alarmPromoteTodo(id: string): void;
5556
alarmClearTodo(id: string): void;
5657
alarmDrainTodoBucket(id: string): void;
5758
onAlarmState(handler: (detail: AlarmStateDetail) => void): void;

lib/src/lib/platform/vscode-adapter.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,10 @@ export class VSCodeAdapter implements PlatformAdapter {
194194
this.vscode.postMessage({ type: 'alarm:markTodo', id });
195195
}
196196

197+
alarmPromoteTodo(id: string): void {
198+
this.vscode.postMessage({ type: 'alarm:promoteTodo', id });
199+
}
200+
197201
alarmClearTodo(id: string): void {
198202
this.vscode.postMessage({ type: 'alarm:clearTodo', id });
199203
}

lib/src/lib/session-save.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ function createPlatform(savedState: PersistedSession | null): PlatformAdapter {
4949
alarmClearAttention: () => {},
5050
alarmToggleTodo: () => {},
5151
alarmMarkTodo: () => {},
52+
alarmPromoteTodo: () => {},
5253
alarmClearTodo: () => {},
5354
alarmDrainTodoBucket: () => {},
5455
onAlarmState: () => {},

standalone/src/tauri-adapter.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,10 @@ export class TauriAdapter implements PlatformAdapter {
199199
this.alarmManager.markTodo(id);
200200
}
201201

202+
alarmPromoteTodo(id: string): void {
203+
this.alarmManager.promoteTodo(id);
204+
}
205+
202206
alarmClearTodo(id: string): void {
203207
this.alarmManager.clearTodo(id);
204208
}

vscode-ext/src/message-router.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,9 @@ export function attachRouter(
291291
case 'alarm:markTodo':
292292
alarmManager.markTodo(msg.id);
293293
break;
294+
case 'alarm:promoteTodo':
295+
alarmManager.promoteTodo(msg.id);
296+
break;
294297
case 'alarm:clearTodo':
295298
alarmManager.clearTodo(msg.id);
296299
break;

0 commit comments

Comments
 (0)