From 3d21cd903e049e1e0c65c2e98ee3097df42f840d Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Mon, 11 May 2026 13:05:41 -0700 Subject: [PATCH] feat(smart-sort): title-heuristic fallback for hookless agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Per-pane resolution: hook entries authoritative when fresh, terminal-title detection fills in for hookless agents (Edge case 9) - buildAttentionByWorktree threads runtimePaneTitlesByTabId + ptyIdsByTabId with tabHasLivePty gate so slept-tab titles don't leak - buildWorktreeComparator's attentionByWorktree is now non-optional, plus NaN/Infinity guards in resolveAttention and history walk - setRuntimePaneTitle / clearRuntimePaneTitle bump sortEpoch when title classification changes so hookless permission prompts re-sort promptly - Tooltip on Smart sort menu item; PostHog events for class distribution, Class 1 promotion (with cause), and smart→recent regression switch Co-authored-by: Orca --- docs/smart-worktree-order-redesign.md | 183 +++++++++---- .../src/components/WorktreeJumpPalette.tsx | 31 ++- .../src/components/sidebar/SidebarHeader.tsx | 47 +++- .../src/components/sidebar/WorktreeList.tsx | 98 ++++++- .../sidebar/smart-attention.test.ts | 257 +++++++++++++++++- .../src/components/sidebar/smart-attention.ts | 198 +++++++++++--- .../src/components/sidebar/smart-sort.test.ts | 48 +++- .../src/components/sidebar/smart-sort.ts | 28 +- .../components/sidebar/visible-worktrees.ts | 8 +- .../runtime-pane-title-sort-epoch.test.ts | 53 ++++ .../src/store/slices/store-cascades.test.ts | 2 +- src/renderer/src/store/slices/terminals.ts | 24 +- src/shared/telemetry-events.ts | 35 ++- 13 files changed, 864 insertions(+), 148 deletions(-) create mode 100644 src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts diff --git a/docs/smart-worktree-order-redesign.md b/docs/smart-worktree-order-redesign.md index dbb5ff8c1e7..78171ca6c86 100644 --- a/docs/smart-worktree-order-redesign.md +++ b/docs/smart-worktree-order-redesign.md @@ -32,12 +32,10 @@ Two upstream changes make hook-reported agent state a reliable primary signal: - `interrupted` (`done` was a Ctrl+C) - `stateHistory[]` (rolling log of prior states; renderer-only) -2. **Persisted across restart.** The hook server's per-pane status cache (`lastStatusByPaneKey`) is persisted to `userData/agent-hooks/last-status.json` and rehydrated on launch. After relaunch, every pane's last-known `state` and `stateStartedAt` are restored before any new hook event arrives. Smart sort sees the same agent state it saw at quit. +2. **Persisted across restart.** The hook server's per-pane status cache (`lastStatusByPaneKey`) is persisted to `userData/agent-hooks/last-status.json` and rehydrated on launch. After relaunch, every pane's last-known `state` and `stateStartedAt` are restored before any new hook event arrives. Smart sort sees the same agent state it saw at quit. **Now merged on `main`** — no longer a sibling-branch dependency. Caveat: only the **current** entry is persisted (on-disk shape carries `receivedAt`/`stateStartedAt` alongside the `ParsedAgentStatusPayload`; restored via `AgentStatusIpcPayload` on the IPC, consumed via `setAgentStatus`'s 4th `timing` arg). The renderer's per-pane `stateHistory[]` is rebuilt from live IPC and starts empty after a restart. This affects exactly one case the redesign cares about — see "Cold start after restart" under Edge cases. - **Sequencing:** the persistence work lands in a separate change (sibling branch `agent-status-preserve-restart`). The smart-sort code can ship before persistence is merged, but the "Smart sees pre-quit state on restart" guarantee only holds once persistence is in. Without persistence, every cold start drops every worktree to Class 4 until each agent's next live hook event repopulates `agentStatusByPaneKey`. Acceptable as an interim state; document it in release notes. - This is a much better signal than terminal-title heuristics. The redesign leans on it as the *primary* ordering signal, not one of seven weighted ones. --- @@ -112,19 +110,38 @@ The card decoration stays — we're not removing visual signals, only their effe - **Tests can assert orderings, not numbers.** `expect(after('blocked')).toRankAbove(after('done'))` and `expect(within('done').newer).toRankAbove(within('done').older)` are meaningful invariants. `expect(score).toBe(12)` is not. - **Lean on the new default-on, persisted-across-restart signal.** Agent state is reliable for every user *and* survives quit/relaunch (per "Context" above). The class layer reads `agentStatusByPaneKey` directly, which is hydrated before sort runs. +### Menu label and tooltip + +The sort-menu label stays as **"Smart"** — no rename. We attach a one-line tooltip on the Smart menu item that summarizes the rule for new users: *"Agents that need attention, then most recent activity."* Lives on the Smart row in the sort dropdown; no other UI changes. + ## Edge cases 1. **Worktree with no agent ever.** No live entry → Class 4 (idle). Sorts below all classes, then by `effectiveRecentActivity`. Same effective behavior as today's "Recent" within the idle group. 2. **Agent transitions `done` → `working`.** Class flips from 2 to 3. The worktree drops below all `done`/`blocked` worktrees. *Within Class 3*, the prior `done` timestamp from `stateHistory[]` drives rank, so it sits at the top of the working group — i.e., "it's the most recently-relevant working agent." This is intentional: once the user has new work running, the *category* of attention has changed, and the visible dot agrees with the position. -3. **User views the worktree.** Class and timestamp do not change on view. The worktree stays where it was. We do not auto-demote on view in v1 — see Open Question #4. -4. **Agent goes `blocked` → `working` → `blocked` quickly.** Each `blocked` transition makes the worktree Class 1 with a fresh `stateStartedAt`. It stays at the top through the back-and-forth, which is correct. +3. **User views the worktree.** Class and timestamp do not change on view. The worktree stays where it was. This is the urgency invariant's UX corollary — see "Rankings change only on objective agent state transitions" under Design principles below: clicking a worktree never re-ranks anything, so users can track positions visually across interactions. +4. **Agent goes `blocked` → `working` → `blocked` quickly.** Each `blocked` transition makes the worktree Class 1 with a fresh `stateStartedAt`. It stays at the top through the back-and-forth, which is correct. The existing `SORT_SETTLE_MS` debounce in `WorktreeList` already coalesces rapid transitions into a single re-sort, so visual flutter from a working↔blocked oscillation is mitigated. Worth measuring during dogfood — if the debounce window proves too short for sub-second hook bursts, lengthen it before adding any class-specific suppression. 5. **Multiple panes per worktree.** Class is the **min** across all panes (most-attention-demanding wins — Class 1 < 2 < 3 < 4). `attentionTimestamp` is the **max** across panes within the resolved class. Any pane that's blocked pulls the whole worktree to Class 1; among Class-1 worktrees, the most-recent `blocked` event ranks first. 6. **Brand-new worktree with no agent yet.** Class 4 (idle). The existing `CREATE_GRACE_MS` floor on `effectiveRecentActivity` keeps it at the top of the idle group during the post-create window. Already covered today. 7. **Stale agent entries.** An entry whose `updatedAt` is older than `AGENT_STATUS_STALE_AFTER_MS` (30 min) is treated as if no live entry exists — the worktree falls to Class 4. Matches existing freshness logic in `isExplicitAgentStatusFresh`. 8. **Cold start after restart.** Hook server hydrates `lastStatusByPaneKey` from disk before binding the listener; the renderer pulls a snapshot when settings + workspace tabs are ready. Before that snapshot arrives, every worktree is Class 4 — Smart momentarily falls back to `effectiveRecentActivity` ordering (the same key the `recent` sort uses), then re-ranks classes 1/2/3 as soon as the snapshot lands. The snapshot restores `state` and `stateStartedAt` per pane, so Classes 1/2 and current-state-driven Class 3 timestamps work normally. The bootstrap delivers N pane entries through N `setAgentStatus` calls (one sortEpoch bump each); the existing `SORT_SETTLE_MS` debounce in `WorktreeList` coalesces them into a single re-sort. + The very-first-frame case (before any PTY is alive at all) is already handled separately: `sortWorktreesSmart`'s existing `!hasAnyLivePty` cold-start branch falls back to the persisted `sortOrder` snapshot until the first warm sort runs. That branch survives this redesign — the new comparator only takes over once the snapshot has landed. No change needed; flagged here so reviewers don't think U5 ("one-frame collapse to Class 4") is unhandled. + The one *narrow* gap: `stateHistory[]` is renderer-only, not persisted. After restart, a pane that's *currently* `working` cannot recover its prior `done`/`blocked` timestamp from before quit — Class 3's within-class timestamp falls back to the current `working` `stateStartedAt`. In practice this only matters when (a) the agent was working at quit time *and* (b) had a prior attention event the user wants reflected in rank. The far-more-common cases — `done`/`blocked` at quit, or `working` agent that did its first attention event in this session — are unaffected. -9. **Agents without hooks (aider, custom scripts).** Class is derived from explicit hook state only. Agents that don't ship hooks land in Class 4 — same as no-agent worktrees. The terminal-title heuristic permission detector is *not* extended into the new sort: Open Question #3 below. + + The sidebar's cold-start branch in `WorktreeList.tsx` uses a latching `sessionHasHadPty` ref (never reverts mid-session) while `sortWorktreesSmart`'s built-in cold-start branch is point-in-time on `!hasAnyLivePty` — if all PTYs close mid-session, the palette can momentarily flip to cold-start ordering while the sidebar does not. Accepted asymmetry; the palette is a transient overlay. +9. **Agents without hooks (aider, custom scripts).** Hook state wins when present; the terminal-title heuristic (`detectAgentStatusFromTitle` in `src/renderer/src/lib/agent-status.ts`) is the **fallback** for panes with no fresh hook entry. Resolution per pane: + + - Fresh hook entry → use it (today's path). + - No fresh hook entry (`!isExplicitAgentStatusFresh`, including stale entries past `AGENT_STATUS_STALE_AFTER_MS`) **and** a runtime pane title that `detectAgentStatusFromTitle` recognizes → derive class from the title: + - `'permission'` → Class 1. Timestamp = `now`. Why `now`: the title detector has no `stateStartedAt`; using `now` keeps the worktree at the top of Class 1 until the next re-sort, which matches the user's mental model ("just noticed it needs me"). + - `'working'` → Class 3. Timestamp = the worktree's `lastActivityAt` (best proxy for "when did this agent last do something" — `stateHistory[]` doesn't exist for title-derived state). Why worktree-level: `TerminalTab` has no per-tab `lastActivityAt`; the worktree-level value is sufficient because Class 3 within-class ordering compares across worktrees, not across panes inside one. + - `'idle'` or `null` → Class 4. + - Hook state is authoritative whenever fresh: a pane whose hook says `done` but whose title detector says `'working'` stays Class 2. Title-heuristic only fires when hook is missing or stale. + + Tradeoff: the title heuristic is fragile (terminal-title-string matching, gated on PTY liveness via `tabHasLivePty`), but for hookless users it's strictly better than collapsing every aider/custom-script pane to Class 4 and burying live attention prompts under stale `done`s. We accept the fragility because (a) it only feeds the fallback path, (b) hook-equipped agents are unaffected, and (c) the heuristic is the same code that already drives the per-card status dot — there's no new fragile surface, we're just routing existing detection into the comparator. + + **Mixed-population tradeoff:** in workspaces where some panes are hookful and some aren't, a freshly-noticed hookless `'permission'` pane (`ts = now`) will outrank a hookful `blocked` pane whose `stateStartedAt` is older by even a second. We accept this — "just noticed" is the closest stable signal we have for hookless panes, and most users run a single agent type. ## Implementation sketch @@ -135,12 +152,14 @@ Primary: - `src/renderer/src/components/sidebar/smart-attention.ts` — **new file**: `SmartClass`, `WorktreeAttention`, `resolveAttention`, `buildAttentionByWorktree`, `mostRecentAttentionInHistory` - `src/renderer/src/components/sidebar/smart-sort.test.ts` — replace numerical assertions with ordering invariants - `src/renderer/src/components/sidebar/smart-attention.test.ts` — **new file**: helper-level tests -- `src/renderer/src/components/WorktreeJumpPalette.tsx` — thread `agentStatusByPaneKey` at lines 227 and 239 +- `src/renderer/src/components/sidebar/WorktreeList.tsx` — thread `runtimePaneTitlesByTabId` and `ptyIdsByTabId` into the sort path so the title-heuristic fallback (Edge case 9) can fire for hookless panes. +- `src/renderer/src/components/sidebar/visible-worktrees.ts` — same: callers of `sortWorktreesSmart`/`buildWorktreeComparator` must thread the new state. +- `src/renderer/src/components/WorktreeJumpPalette.tsx` — thread `agentStatusByPaneKey`, `runtimePaneTitlesByTabId`, and `ptyIdsByTabId` at lines 227 and 239. No changes: - `src/shared/agent-status-types.ts` — already provides `AgentStatusEntry`, `AgentStateHistoryEntry.startedAt`, `AgentStateHistoryEntry.interrupted`, `AGENT_STATUS_STALE_AFTER_MS`, `AGENT_STATE_HISTORY_MAX`. - `src/renderer/src/store/slices/agent-status.ts` — already preserves `interrupted` on `stateHistory` rows (the helper relies on this). -- `src/renderer/src/lib/agent-status.ts` — `isExplicitAgentStatusFresh` is reused as-is. +- `src/renderer/src/lib/agent-status.ts` — `isExplicitAgentStatusFresh` and `detectAgentStatusFromTitle` are reused as-is for the heuristic fallback. - `src/main/agent-hooks/server.ts` — persistence is a separate change on the sibling branch. ### Data model @@ -158,39 +177,70 @@ type WorktreeAttention = { ### Per-worktree resolution -For each worktree, walk its panes' entries in `agentStatusByPaneKey`: +For each worktree, walk its panes — preferring fresh hook entries from `agentStatusByPaneKey`, falling back to title-heuristic detection over `runtimePaneTitlesByTabId` for hookless panes (Edge case 9). NaN-safety: `resolveAttention` should treat `Number.isFinite(stateStartedAt) === false` the same as a missing entry — a corrupted timestamp would silently sink the worktree to the bottom of its class because NaN compares false against everything. ```ts +type PaneInput = + | { kind: 'hook'; entry: AgentStatusEntry } + // Why: TerminalTab has no per-tab lastActivityAt; worktree-level value is + // enough since within-class ordering compares across worktrees. + | { kind: 'title'; status: AgentStatus | null; worktreeLastActivityAt: number } + function resolveAttention( - panes: AgentStatusEntry[], // entries belonging to this worktree's panes + panes: PaneInput[], now: number ): WorktreeAttention { let bestCls: SmartClass = 4 let bestTs = 0 - for (const entry of panes) { - if (!isExplicitAgentStatusFresh(entry, now, AGENT_STATUS_STALE_AFTER_MS)) continue - + for (const pane of panes) { let cls: SmartClass let ts: number - if (entry.state === 'blocked' || entry.state === 'waiting') { - cls = 1 - ts = entry.stateStartedAt - } else if (entry.state === 'done') { - // Why: an interrupted `done` (user pressed Ctrl+C) is the user signalling - // "I'm done with this turn". Treat as idle, not as Class 2 attention. - if (entry.interrupted) continue - cls = 2 - ts = entry.stateStartedAt + + if (pane.kind === 'hook') { + const entry = pane.entry + if (!isExplicitAgentStatusFresh(entry, now, AGENT_STATUS_STALE_AFTER_MS)) continue + // Why: defensive guard. NaN/Infinity from a corrupted stateStartedAt would + // poison comparisons (NaN > anything === false), silently dropping the + // worktree to the bottom of its class. Treat as missing. + if (!Number.isFinite(entry.stateStartedAt)) continue + + if (entry.state === 'blocked' || entry.state === 'waiting') { + cls = 1 + ts = entry.stateStartedAt + } else if (entry.state === 'done') { + // Why: an interrupted `done` (user pressed Ctrl+C) is the user signalling + // "I'm done with this turn". Treat as idle, not as Class 2 attention. + if (entry.interrupted) continue + cls = 2 + ts = entry.stateStartedAt + } else { + // working + cls = 3 + // Why: within the working class, sort by the most-recent prior attention + // event so a worktree that just transitioned done→working stays above one + // that's been working for an hour. Falls back to current stateStartedAt + // when stateHistory is empty (e.g. fresh after restart — see Edge case 8). + const prior = mostRecentAttentionInHistory(entry.stateHistory) + ts = prior ?? entry.stateStartedAt + } } else { - // working - cls = 3 - // Why: within the working class, sort by the most-recent prior attention - // event so a worktree that just transitioned done→working stays above one - // that's been working for an hour. Falls back to current stateStartedAt - // when stateHistory is empty (e.g. fresh after restart — see Edge case 8). - const prior = mostRecentAttentionInHistory(entry.stateHistory) - ts = prior ?? entry.stateStartedAt + // Title-heuristic fallback (no fresh hook entry for this pane). See + // Edge case 9. Fragility tradeoff lives there; the rule is: hook wins + // when fresh, title-heuristic fills the gap, no override of fresh hook. + if (pane.status === 'permission') { + cls = 1 + // Why now: the title detector exposes no stateStartedAt. Using `now` + // pins the worktree to the top of Class 1 until a hook event or the + // next sort, matching the user's "just noticed" mental model. + ts = now + } else if (pane.status === 'working') { + cls = 3 + ts = pane.worktreeLastActivityAt + } else { + // 'idle' or null: nothing to assert; pane stays in Class 4. + continue + } } // Why min: smaller class number = higher priority. Any pane in a more @@ -212,6 +262,9 @@ function mostRecentAttentionInHistory(history: AgentStateHistoryEntry[]): number // out, so we see it on historical entries the same way as on the current. if (h.state === 'done' && h.interrupted) continue if (h.state === 'done' || h.state === 'blocked' || h.state === 'waiting') { + // Why: mirror the current-entry guard. NaN is silently skipped by `>`, + // but Infinity would pin the worktree at the top of Class 3 forever. + if (!Number.isFinite(h.startedAt)) continue if (h.startedAt > max) max = h.startedAt } } @@ -225,6 +278,8 @@ function mostRecentAttentionInHistory(history: AgentStateHistoryEntry[]): number `agentStatusByPaneKey` is keyed by `${tabId}:${paneId}`. Build a `tabId → entries[]` index once per sort (the existing `buildExplicitEntriesByTabId` helper does this), then walk each worktree's tabs and accumulate. The aggregate cost is `O(E + N × T × H)` per sort, where E = total entries, N = worktrees, T = tabs/worktree, H = history length. With H bounded at 20 and typical T ≤ 4, this is comfortably below the existing decorate-sort-undecorate path. +`buildAttentionByWorktree` accepts `runtimePaneTitlesByTabId` and `ptyIdsByTabId` alongside `agentStatusByPaneKey` (matching the pattern in `getWorkingAgentsPerWorktree`/`countWorkingAgents` in `src/renderer/src/lib/agent-status.ts`). For each pane with no fresh hook entry, look up its title and gate on `tabHasLivePty` before running `detectAgentStatusFromTitle` — slept tabs preserve their pane titles under `keepIdentifiers`, so the liveness gate is what stops a stale "✋" title from re-promoting a dead pane to Class 1. + ### The comparator ```ts @@ -262,16 +317,16 @@ For the *currently-focused* worktree specifically, we do not freeze its position ### Caller updates -Every caller of `sortWorktreesSmart` (and the smart branch of `buildWorktreeComparator`) MUST thread `agentStatusByPaneKey`. With the terminal-title heuristic dropped from sort (Open Question #3), a caller that omits agent status collapses every worktree to Class 4 — no fallback signal carries the ranking. +Every caller of `sortWorktreesSmart` (and the smart branch of `buildWorktreeComparator`) MUST thread three store reads: `agentStatusByPaneKey`, `runtimePaneTitlesByTabId`, and `ptyIdsByTabId`. The first carries the primary signal; the other two enable the title-heuristic fallback for hookless agents (Edge case 9). A caller that omits all three collapses every worktree to Class 4; a caller that omits the title-related two silently disables fallback and buries hookless permission prompts. Callers today (verify before each edit): -- `src/renderer/src/components/sidebar/visible-worktrees.ts:179` — already threads `state.agentStatusByPaneKey` (no change needed). -- `src/renderer/src/components/sidebar/visible-worktrees.ts:188` — already threads `state.agentStatusByPaneKey` (no change needed). -- `src/renderer/src/components/sidebar/WorktreeList.tsx:652` — uses `buildWorktreeComparator`; verify it threads agent status (it does today via the parent state read). -- `src/renderer/src/components/WorktreeJumpPalette.tsx:227` and `:239` — currently `sortWorktreesSmart(visibleWorktrees, tabsByWorktree, repoMap, prCache)` (4 args). **Must change** to pass `agentStatusByPaneKey` from the store. +- `src/renderer/src/components/sidebar/visible-worktrees.ts:179` — already threads `state.agentStatusByPaneKey`. **Must add** `state.runtimePaneTitlesByTabId` and `state.ptyIdsByTabId`. +- `src/renderer/src/components/sidebar/visible-worktrees.ts:188` — same: already threads agent status, **must add** the two title-fallback inputs. +- `src/renderer/src/components/sidebar/WorktreeList.tsx:652` — uses `buildWorktreeComparator`; verify the parent state read picks up the two new fields and threads them through. +- `src/renderer/src/components/WorktreeJumpPalette.tsx:227` and `:239` — currently `sortWorktreesSmart(visibleWorktrees, tabsByWorktree, repoMap, prCache)` (4 args). **Must change** to pass `agentStatusByPaneKey`, `runtimePaneTitlesByTabId`, and `ptyIdsByTabId` from the store. -Make `agentStatusByPaneKey` **non-optional** in the new `sortWorktreesSmart` signature. Why: a forgotten caller fails type-check rather than silently regressing palette ordering to "all Class 4". Same for the `agentStatusByPaneKey` parameter on `buildWorktreeComparator`'s smart branch. +Make all three new parameters **non-optional** in the new `sortWorktreesSmart` signature (and on `buildWorktreeComparator`'s smart branch). Why: a forgotten caller fails type-check rather than silently regressing the palette to "all Class 4" or quietly losing the hookless-fallback path. ### Implementation plan (step-by-step) @@ -280,29 +335,32 @@ This section is the implementation handoff. Follow in order; each step lands a c **Step 1 — Add the class + attention helpers.** - New file: `src/renderer/src/components/sidebar/smart-attention.ts`. - Export `SmartClass = 1 | 2 | 3 | 4`, `WorktreeAttention = { cls: SmartClass; attentionTimestamp: number }`, the constant `IDLE: WorktreeAttention = { cls: 4, attentionTimestamp: 0 }`. -- Implement `resolveAttention(panes, now)` and `mostRecentAttentionInHistory(history)` exactly as in "Per-worktree resolution" above. -- Unit tests for the helper directly (faster to iterate than going through the full sort): one entry per class, multi-pane min-class/max-timestamp, interrupted-done skip, history-only-interrupted fallback, stale-entry skip. +- Implement `resolveAttention(panes, now)` and `mostRecentAttentionInHistory(history)` exactly as in "Per-worktree resolution" above. `resolveAttention` accepts `PaneInput` (hook entry OR title-heuristic input) per pane. +- Unit tests for the helper directly (faster to iterate than going through the full sort): one entry per class, multi-pane min-class/max-timestamp, interrupted-done skip, history-only-interrupted fallback, stale-entry skip, NaN `stateStartedAt` skip, title-heuristic `'permission'`/`'working'`/`'idle'` mapping, hook-overrides-title (fresh hook + matching title doesn't double-promote). **Step 2 — Build the per-worktree attention map.** -- Add `buildAttentionByWorktree(worktrees, tabsByWorktree, agentStatusByPaneKey, now)` to `smart-attention.ts`. +- Add `buildAttentionByWorktree(worktrees, tabsByWorktree, agentStatusByPaneKey, runtimePaneTitlesByTabId, ptyIdsByTabId, now)` to `smart-attention.ts`. - Reuse the existing `buildExplicitEntriesByTabId` index from `smart-sort.ts` (move it into `smart-attention.ts` if cleaner — it's only used by the smart path). -- Walk each worktree's tabs, collect their `AgentStatusEntry`s, call `resolveAttention`, store result in a `Map`. +- Walk each worktree's tabs. For each tab, collect hook entries (keyed `${tabId}:${paneId}`); for tabs/panes with no fresh hook entry, gate on `tabHasLivePty(ptyIdsByTabId, tab.id)` and call `detectAgentStatusFromTitle` over the pane's `runtimePaneTitlesByTabId[tab.id]` map (or `tab.title` if no per-pane titles yet — same fallback shape as `getWorkingAgentsPerWorktree`). Pass the resulting `PaneInput[]` to `resolveAttention`. **Step 3 — Rewrite the comparator.** - In `smart-sort.ts`, replace the smart branch of `buildWorktreeComparator` with the class-then-recency comparator from "The comparator" section. - Replace `precomputedScores` parameter with `attentionByWorktree: Map`. -- Make `agentStatusByPaneKey` non-optional on this parameter list (TypeScript will surface every caller). +- Make `agentStatusByPaneKey`, `runtimePaneTitlesByTabId`, and `ptyIdsByTabId` non-optional on this parameter list (TypeScript will surface every caller). - Delete `SmartSortOverride`, `smartSortOverrides`, `getSmartSortCandidate`, `precomputedScores`, `computeSmartScore`, `computeSmartScoreFromSignals`, `hasRecentPRSignal`. Keep `effectiveRecentActivity` and `CREATE_GRACE_MS` (used by `recent` and Class 4 tiebreaker). **Step 4 — Rewrite `sortWorktreesSmart`.** - Build `attentionByWorktree` once via `buildAttentionByWorktree`. - Pass it into the comparator. -- Keep the existing cold-start branch (`!hasAnyLivePty`) using persisted `sortOrder` as today — see Open Question #5. +- Keep the existing cold-start branch (`!hasAnyLivePty`) using persisted `sortOrder` as today — see Open Question #3 (Migration) and Edge case 8. **Step 5 — Update callers.** -- Update `WorktreeJumpPalette.tsx:227` and `:239` to pass `state.agentStatusByPaneKey`. +- Update `WorktreeJumpPalette.tsx:227` and `:239` to pass `state.agentStatusByPaneKey`, `state.runtimePaneTitlesByTabId`, and `state.ptyIdsByTabId`. +- Update `WorktreeList.tsx` and `visible-worktrees.ts` to thread the same two title-fallback fields. - Run `pnpm typecheck` to confirm no other callers slipped through. +**Step 5a — Bump `sortEpoch` on title-fallback-relevant changes.** The title-fallback path now influences ordering (see "Title heuristic stays in sort as the hookless fallback" under Design principles, and Edge case 9), but `setRuntimePaneTitle` in `src/renderer/src/store/slices/terminals.ts` does not currently bump `sortEpoch`. Without this, a hookless agent that transitions to `'permission'` via a title change won't trigger a re-sort until some unrelated event fires. Required: any mutation that changes `runtimePaneTitlesByTabId` (today: `setRuntimePaneTitle`, `clearRuntimePaneTitle`) MUST bump `sortEpoch` whenever the new title's `detectAgentStatusFromTitle` output differs from the previous title's output. This is a behavioral change from today (today's epoch only bumps on hook events + activity events). Suggested coarser approach if the per-title classification check proves awkward: bump unconditionally on title change — cheap, and already debounced by `SORT_SETTLE_MS` in `WorktreeList`. + **Step 6 — Rewrite tests.** - Replace numerical-score assertions in `smart-sort.test.ts` with the ordering invariants listed in "Tests to rewrite" below. - Add a new test file `smart-attention.test.ts` for the `resolveAttention` helper. @@ -313,9 +371,9 @@ This section is the implementation handoff. Follow in order; each step lands a c - Manual smoke: open Orca with several worktrees + agents in different states, switch to Smart sort, verify ordering matches the class table. - Confirm no new console errors when `agentStatusByPaneKey` is empty (e.g., very early in app startup). -**What this design depends on, in order:** +**What this design depends on:** 1. Default-on agent status — **already merged on `main`** (PR #1538). -2. Persistence of `lastStatusByPaneKey` across restart — **separate change on the sibling branch** `agent-status-preserve-restart`. Smart-sort code can ship before this; the "Smart sees pre-quit state after restart" guarantee comes online when persistence merges. Verify the sibling branch's `AgentStatusIpcPayload` and `setAgentStatus`'s 4th `timing` arg are already present before relying on Edge case 8's claim about restored timestamps. +2. Persistence of `lastStatusByPaneKey` across restart — **also merged on `main`**. `AgentStatusIpcPayload` and `setAgentStatus`'s 4th `timing` arg are present, so Edge case 8's claim about restored timestamps holds at ship time. ### Tests to rewrite @@ -326,16 +384,41 @@ This section is the implementation handoff. Follow in order; each step lands a c - Two `blocked` worktrees: the one whose `stateStartedAt` is more recent ranks first. - A worktree with `state='working'` and a prior `done` in `stateHistory` ranks above another `working` worktree with no history. - A `working` worktree whose history contains only interrupted-`done` entries falls back to the current `stateStartedAt` (the helper returns null and `??` activates). +- A `working` worktree whose history contains a non-finite (`NaN`/`Infinity`) `startedAt` on an attention entry skips that entry — same guard as the current-entry path; an `Infinity` would otherwise pin the worktree at the top of Class 3. - `interrupted` `done` worktrees rank in Class 4, not Class 2. - Stale entries (`updatedAt > AGENT_STATUS_STALE_AFTER_MS` ago) are ignored — worktree falls to Class 4. - Class 4 ties break on `effectiveRecentActivity`, then `displayName`. - `working` transitions never promote a worktree above a `blocked` one. -- **Palette caller regression**: `WorktreeJumpPalette` ranks a `blocked` worktree above a `working` one when both flow through `sortWorktreesSmart` with `agentStatusByPaneKey` threaded — pins that the palette path uses class, not the dropped heuristic. +- **Palette caller regression**: `WorktreeJumpPalette` ranks a `blocked` worktree above a `working` one when both flow through `sortWorktreesSmart` with `agentStatusByPaneKey` threaded — pins that the palette path uses class. +- **Title-heuristic fallback**: a pane with no fresh hook entry but a runtime title of `"✋ Gemini CLI"` (or any title `detectAgentStatusFromTitle` maps to `'permission'`) sorts into Class 1 above a hook-driven `done` worktree. Companion test: a `'working'`-titled hookless pane sorts into Class 3 with `attentionTimestamp = worktree.lastActivityAt`. Companion test: a fresh hook entry of `done` plus a title that the detector reads as `'working'` stays Class 2 — hook wins when fresh. +- **Title change bumps `sortEpoch`**: a title change from a `'working'`-classified string to a `'permission'`-classified string bumps `sortEpoch` and triggers a re-sort, so a hookless permission prompt promotes into Class 1 without waiting for an unrelated event. + +## Telemetry + +Three PostHog events ship with this change to validate the redesign post-launch (Orca project ID 406068): + +- **`smart_sort_class_distribution`** — fired from `WorktreeList` after each completed `sortedIds` recomputation, throttled to once per ~30s. Stops firing when `sortBy !== 'smart'` (timer ownership lives with the component, so a sort-selector switch cancels it cleanly). Properties: `class1Count`, `class2Count`, `class3Count`, `class4Count`, `totalWorktrees`. Lets us see whether real users actually have Class 1/2/3 populated or whether everyone sits in Class 4 (signal that hook coverage is too low and the redesign isn't doing work). +- **`smart_sort_class_1_promotion`** — fired from `WorktreeList` after each `sortedIds` recomputation, suppressing repeat-fires by comparing against a `prevClassByWorktreeId` map keyed by worktree id. Only fires on transitions INTO Class 1 (not on Class 1 → Class 1 noise from transient stale-window flickers). Properties: `cause` ∈ `{ 'blocked', 'waiting', 'title-heuristic' }`. Distinguishes hook-driven promotions from the title-heuristic fallback path so we can tell whether Edge case 9 is carrying weight or whether the heuristic is rarely needed. +- **`smart_to_recent_switch`** — fired when a user switches the sort selector from `smart` to `recent` *after* the redesign ships. A spike post-launch is the cleanest regression signal we have ("users are voting with their feet"). Pair with the inverse `recent_to_smart_switch` to read net flow. + +Keep payloads small; no per-worktree IDs or paths in event properties. + +## Design principles / settled decisions + +The Phase A review settled two questions that earlier drafts left open. Recording them here so future iterations don't relitigate them. + +### Rankings change only on objective agent state transitions + +Class and `attentionTimestamp` move when the *agent* changes state — hook event, transition, or staleness threshold. They do **not** move when the *user* changes view, focuses a worktree, marks unread badges as read, or otherwise interacts. Why: when 8+ worktrees are visible, users build muscle memory for *positions*. If clicking a worktree silently re-ranked the list, the next worktree the user wanted would have shifted underneath them. Whether a worktree is "visited done" or "unvisited done" is a per-user, per-glance concept that belongs on the unread badge, not in the sort comparator. The dot, the badge, and the rank position tell three orthogonal stories — keeping rank tied to objective state is what lets all three coexist. + +This means: no view-demote, no activation-demote, no "decay the class once seen." Edge case 3 is the visible consequence. If users later report that the top of the list is stuck on something they already handled, the right fix is on the *badge* (which already records "seen"), not on the sort. + +### Title heuristic stays in sort as the hookless fallback + +Hooks are the primary signal; the title heuristic (`detectAgentStatusFromTitle`) is the fallback for panes without fresh hook state (Edge case 9 has the full rule). Why kept: dropping it entirely would dump every aider/custom-script user's permission prompts into Class 4, defeating the redesign's main promise for that population. Why limited to fallback: the heuristic is string-matching on terminal titles — fragile under agent updates and locale changes. By gating it on `!isExplicitAgentStatusFresh`, hook-equipped agents are unaffected and the fragility surfaces only where it strictly improves over Class 4. ## Open questions 1. **Should we persist `attentionTimestamp` per worktree?** Recommendation: **no, for v1.** The hook-server cache persistence (sibling branch) covers the practical cold-start case — restart restores per-pane current state. A second persistence layer for worktree-level attention is redundant and adds a migration. Revisit only if users report cross-restart staleness in the `stateHistory`-only edge case (Edge case 8). -2. **Should viewing the worktree clear/demote the class?** Recommendation: **no for v1.** Keeping Class membership tied to objective agent state (not user view) means the dot, badge, and rank position tell a consistent story. The unread badge already records "you haven't seen it yet" and is the right place for view-acknowledged semantics. If users complain that the top of the list is stuck on something they already handled, the simplest follow-up is "demote on activation if and only if the unread badge clears" — small, scoped, additive. -3. **What about the heuristic permission-prompt detector for no-hook agents?** Recommendation: **drop it from sort.** The heuristic is fragile (terminal-title-string matching) and the new design is built on hook state. Agents without hooks (aider, custom shell scripts) land in Class 4 — same rank position as today's `recent` would give them. The detector continues to drive the per-card status dot for visibility, but no longer feeds into ranking. Trading a small visible-rank regression for non-hook agents in exchange for a much simpler, harder-to-fool comparator. -4. **What about a user actively typing in a no-agent worktree?** With Class 4 idle worktrees ordered by `effectiveRecentActivity`, an active no-agent worktree ranks at the top of Class 4 (above truly-stale ones) but still below every Class 1/2/3 worktree. For users whose primary workflow is "type in a worktree without an agent," Smart will feel agent-centric. **This is by design** — that's what `recent` is for. We should consider renaming the `lastActivityAt` tiebreaker logic if user feedback suggests confusion, but no change in v1. -5. **Migration.** Existing users with `sortBy: 'smart'` keep that selection — they get the new behavior automatically. The persisted `sortOrder` snapshot (used in cold-start before any PTY is alive) becomes meaningless under the new comparator; we discard it on first warm sort and re-snapshot. First cold start after this lands uses the pre-existing `sortOrder` snapshot until the persisted hook-server cache hydrates and the first warm sort runs (~1 frame after the bootstrap snapshot arrives), then the snapshot is overwritten. Briefly stale-looking ordering on first launch after upgrade is acceptable; no explicit invalidation needed. No user-visible migration needed. +2. **What about a user actively typing in a no-agent worktree?** With Class 4 idle worktrees ordered by `effectiveRecentActivity`, an active no-agent worktree ranks at the top of Class 4 (above truly-stale ones) but still below every Class 1/2/3 worktree. For users whose primary workflow is "type in a worktree without an agent," Smart will feel agent-centric. **This is by design** — that's what `recent` is for. We should consider renaming the `lastActivityAt` tiebreaker logic if user feedback suggests confusion, but no change in v1. +3. **Migration.** Existing users with `sortBy: 'smart'` keep that selection — they get the new behavior automatically. The persisted `sortOrder` snapshot (used in cold-start before any PTY is alive) becomes meaningless under the new comparator; we discard it on first warm sort and re-snapshot. First cold start after this lands uses the pre-existing `sortOrder` snapshot until the persisted hook-server cache hydrates and the first warm sort runs (~1 frame after the bootstrap snapshot arrives), then the snapshot is overwritten. Briefly stale-looking ordering on first launch after upgrade is acceptable; no explicit invalidation needed. No user-visible migration needed. diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index 772a52eac0d..cfa7f2a6d97 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -230,7 +230,14 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { const sortedWorktrees = useMemo( () => hasQuery - ? sortWorktreesSmart(visibleWorktrees, tabsByWorktree, repoMap, agentStatusByPaneKey) + ? sortWorktreesSmart( + visibleWorktrees, + tabsByWorktree, + repoMap, + agentStatusByPaneKey, + runtimePaneTitlesByTabId, + ptyIdsByTabId + ) : switchableWorktreesForRows, [ hasQuery, @@ -238,7 +245,9 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { switchableWorktreesForRows, tabsByWorktree, repoMap, - agentStatusByPaneKey + agentStatusByPaneKey, + runtimePaneTitlesByTabId, + ptyIdsByTabId ] ) @@ -249,8 +258,22 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null { // tab on the default-branch worktree before toggling hide-on should still // be able to Cmd+J back to it — the setting hides the *workspace row*, // not the browser tabs that live inside it. - return sortWorktreesSmart(allWorktrees, tabsByWorktree, repoMap, agentStatusByPaneKey) - }, [allWorktrees, tabsByWorktree, repoMap, agentStatusByPaneKey]) + return sortWorktreesSmart( + allWorktrees, + tabsByWorktree, + repoMap, + agentStatusByPaneKey, + runtimePaneTitlesByTabId, + ptyIdsByTabId + ) + }, [ + allWorktrees, + tabsByWorktree, + repoMap, + agentStatusByPaneKey, + runtimePaneTitlesByTabId, + ptyIdsByTabId + ]) // Why: browser rows need worktree lookups for repo badge colors, and browser // search intentionally includes archived worktrees. This map must cover all diff --git a/src/renderer/src/components/sidebar/SidebarHeader.tsx b/src/renderer/src/components/sidebar/SidebarHeader.tsx index 5757f44a606..e84bde04b85 100644 --- a/src/renderer/src/components/sidebar/SidebarHeader.tsx +++ b/src/renderer/src/components/sidebar/SidebarHeader.tsx @@ -38,10 +38,14 @@ const PROPERTY_OPTIONS: { id: WorktreeCardProperty; label: string }[] = [ ] const SORT_OPTIONS = [ - { id: 'name', label: 'Name' }, - { id: 'smart', label: 'Smart' }, - { id: 'recent', label: 'Recent' }, - { id: 'repo', label: 'Repo' } + { id: 'name', label: 'Name', description: null }, + { + id: 'smart', + label: 'Smart', + description: 'Agents that need attention, then most recent activity.' + }, + { id: 'recent', label: 'Recent', description: null }, + { id: 'repo', label: 'Repo', description: null } ] as const const isMac = navigator.userAgent.includes('Mac') @@ -116,17 +120,30 @@ const SidebarHeader = React.memo(function SidebarHeader() { value={sortBy} onValueChange={(v) => setSortBy(v as typeof sortBy)} > - {SORT_OPTIONS.map((opt) => ( - e.preventDefault()} - > - {opt.label} - - ))} + {SORT_OPTIONS.map((opt) => { + const radioItem = ( + e.preventDefault()} + > + {opt.label} + + ) + if (!opt.description) { + return radioItem + } + return ( + + {radioItem} + + {opt.description} + + + ) + })} diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index a09834d43c5..7682d8fdb62 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -16,7 +16,12 @@ import { cn } from '@/lib/utils' import type { Worktree, Repo } from '../../../../shared/types' import { isGitRepoKind } from '../../../../shared/repo-kind' import { buildWorktreeComparator } from './smart-sort' -import { buildAttentionByWorktree } from './smart-attention' +import { + buildAttentionByWorktree, + type SmartClass, + type WorktreeAttention +} from './smart-attention' +import { track } from '@/lib/telemetry' import { tabHasLivePty } from '@/lib/tab-has-live-pty' import { type GroupHeaderRow, @@ -628,6 +633,12 @@ const WorktreeList = React.memo(function WorktreeList() { // Why useMemo instead of useEffect: the sort order must be computed // synchronously *before* the worktrees memo reads it, otherwise the // first render (and epoch bumps) would use stale/empty data from the ref. + // Why a ref alongside the memo: telemetry effects need access to the most + // recently computed attention map without forcing every render to read it + // from store state again. The ref captures whatever the memo last produced + // for the smart branch. + const lastAttentionByWorktreeRef = useRef | null>(null) + const sortedIds = useMemo(() => { const state = useAppStore.getState() const nonArchivedWorktrees = getAllWorktreesFromState(state).filter( @@ -654,6 +665,7 @@ const WorktreeList = React.memo(function WorktreeList() { nonArchivedWorktrees.sort( (a, b) => b.sortOrder - a.sortOrder || a.displayName.localeCompare(b.displayName) ) + lastAttentionByWorktreeRef.current = null return nonArchivedWorktrees.map((w) => w.id) } } @@ -670,9 +682,12 @@ const WorktreeList = React.memo(function WorktreeList() { nonArchivedWorktrees, currentTabs, state.agentStatusByPaneKey, + state.runtimePaneTitlesByTabId, + state.ptyIdsByTabId, now ) - : null + : new Map() + lastAttentionByWorktreeRef.current = sortBy === 'smart' ? attentionByWorktree : null nonArchivedWorktrees.sort(buildWorktreeComparator(sortBy, repoMap, now, attentionByWorktree)) return nonArchivedWorktrees.map((w) => w.id) // debouncedSortEpoch is an intentional trigger: it's not read inside the @@ -681,6 +696,85 @@ const WorktreeList = React.memo(function WorktreeList() { // oxlint-disable-next-line react-hooks/exhaustive-deps }, [debouncedSortEpoch, repoMap, sortBy]) + // Why a ref of prior class per worktree: smart_sort_class_1_promotion must + // fire only on transitions INTO Class 1, not on every recompute that keeps + // a worktree there. Suppressing repeats with a ref keeps the event signal + // clean without growing component state. + const prevClassByWorktreeIdRef = useRef>(new Map()) + + useEffect(() => { + const attention = lastAttentionByWorktreeRef.current + if (sortBy !== 'smart' || !attention) { + // Why reset: when the user switches off Smart, drop the prior-class map + // so re-entering Smart doesn't fire stale promotion events for worktrees + // whose state has since changed. + prevClassByWorktreeIdRef.current = new Map() + return + } + const next = new Map() + for (const [worktreeId, info] of attention) { + const prev = prevClassByWorktreeIdRef.current.get(worktreeId) + if (info.cls === 1 && prev !== 1 && info.cause) { + track('smart_sort_class_1_promotion', { cause: info.cause }) + } + next.set(worktreeId, info.cls) + } + prevClassByWorktreeIdRef.current = next + }, [sortBy, sortedIds]) + + // Why a 30s timer owned by the component: the class-distribution event is + // a coarse health signal — we only need it often enough to see the steady + // state, not every render. Cancelling on unmount or sort switch keeps the + // timer from firing while the user is in Recent/Name/Repo modes. + useEffect(() => { + if (sortBy !== 'smart') { + return + } + const fire = () => { + const attention = lastAttentionByWorktreeRef.current + if (!attention) { + return + } + let class1 = 0 + let class2 = 0 + let class3 = 0 + let class4 = 0 + for (const info of attention.values()) { + if (info.cls === 1) { + class1++ + } else if (info.cls === 2) { + class2++ + } else if (info.cls === 3) { + class3++ + } else { + class4++ + } + } + track('smart_sort_class_distribution', { + class1, + class2, + class3, + class4, + totalWorktrees: attention.size + }) + } + fire() + const timer = setInterval(fire, 30_000) + return () => clearInterval(timer) + }, [sortBy]) + + // Why fire on the transition: switching away from Smart is the user signal + // we care about (regression). Use a ref to compare against the previous + // value so we don't double-fire when sortBy momentarily round-trips. + const prevSortByRef = useRef(sortBy) + useEffect(() => { + const prev = prevSortByRef.current + prevSortByRef.current = sortBy + if (prev === 'smart' && sortBy === 'recent') { + track('smart_to_recent_switch', {}) + } + }, [sortBy]) + // Persist the computed sort order so the sidebar can be restored after // restart. Only persist during live sessions (sessionHasHadPty latched) — // on cold start we are *reading* the persisted order, not overwriting it. diff --git a/src/renderer/src/components/sidebar/smart-attention.test.ts b/src/renderer/src/components/sidebar/smart-attention.test.ts index 4ec17d910f6..05120eae905 100644 --- a/src/renderer/src/components/sidebar/smart-attention.test.ts +++ b/src/renderer/src/components/sidebar/smart-attention.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines */ import { describe, expect, it } from 'vitest' import { AGENT_STATUS_STALE_AFTER_MS, @@ -8,10 +9,19 @@ import { IDLE, buildAttentionByWorktree, mostRecentAttentionInHistory, - resolveAttention + resolveAttention, + type PaneInput } from './smart-attention' import type { TerminalTab, Worktree } from '../../../../shared/types' +function hookPane(entry: AgentStatusEntry): PaneInput { + return { kind: 'hook', entry } +} + +function hookPanes(entries: AgentStatusEntry[]): PaneInput[] { + return entries.map((entry) => ({ kind: 'hook', entry })) +} + const NOW = new Date('2026-03-27T12:00:00.000Z').getTime() function makeEntry(overrides: Partial & { paneKey: string }): AgentStatusEntry { @@ -73,6 +83,18 @@ describe('mostRecentAttentionInHistory', () => { ]) ).toBeNull() }) + + it('skips history rows with non-finite startedAt', () => { + // Why: NaN passes through > silently; Infinity would pin the worktree + // at the top of Class 3 forever. Treat non-finite as missing. + expect( + mostRecentAttentionInHistory([ + makeHistory('done', Number.NaN), + makeHistory('blocked', Number.POSITIVE_INFINITY), + makeHistory('done', NOW - 5_000) + ]) + ).toBe(NOW - 5_000) + }) }) describe('resolveAttention', () => { @@ -87,7 +109,11 @@ describe('resolveAttention', () => { stateStartedAt: NOW - 60_000, updatedAt: NOW - 30_000 }) - expect(resolveAttention([entry], NOW)).toEqual({ cls: 1, attentionTimestamp: NOW - 60_000 }) + expect(resolveAttention([hookPane(entry)], NOW)).toEqual({ + cls: 1, + attentionTimestamp: NOW - 60_000, + cause: 'blocked' + }) }) it('classifies a waiting pane as Class 1', () => { @@ -97,7 +123,7 @@ describe('resolveAttention', () => { stateStartedAt: NOW - 60_000, updatedAt: NOW - 30_000 }) - expect(resolveAttention([entry], NOW).cls).toBe(1) + expect(resolveAttention([hookPane(entry)], NOW).cls).toBe(1) }) it('classifies a done pane as Class 2', () => { @@ -107,7 +133,10 @@ describe('resolveAttention', () => { stateStartedAt: NOW - 90_000, updatedAt: NOW - 30_000 }) - expect(resolveAttention([entry], NOW)).toEqual({ cls: 2, attentionTimestamp: NOW - 90_000 }) + expect(resolveAttention([hookPane(entry)], NOW)).toEqual({ + cls: 2, + attentionTimestamp: NOW - 90_000 + }) }) it('treats interrupted done as idle', () => { @@ -118,7 +147,7 @@ describe('resolveAttention', () => { stateStartedAt: NOW - 90_000, updatedAt: NOW - 30_000 }) - expect(resolveAttention([entry], NOW)).toEqual(IDLE) + expect(resolveAttention([hookPane(entry)], NOW)).toEqual(IDLE) }) it('classifies a working pane with prior done as Class 3 with the prior timestamp', () => { @@ -129,7 +158,7 @@ describe('resolveAttention', () => { updatedAt: NOW - 1_000, stateHistory: [makeHistory('done', NOW - 5 * 60_000)] }) - expect(resolveAttention([entry], NOW)).toEqual({ + expect(resolveAttention([hookPane(entry)], NOW)).toEqual({ cls: 3, attentionTimestamp: NOW - 5 * 60_000 }) @@ -143,7 +172,7 @@ describe('resolveAttention', () => { updatedAt: NOW - 1_000, stateHistory: [] }) - expect(resolveAttention([entry], NOW)).toEqual({ + expect(resolveAttention([hookPane(entry)], NOW)).toEqual({ cls: 3, attentionTimestamp: NOW - 10_000 }) @@ -157,7 +186,7 @@ describe('resolveAttention', () => { updatedAt: NOW - 1_000, stateHistory: [makeHistory('done', NOW - 60_000, true)] }) - expect(resolveAttention([entry], NOW)).toEqual({ + expect(resolveAttention([hookPane(entry)], NOW)).toEqual({ cls: 3, attentionTimestamp: NOW - 10_000 }) @@ -170,7 +199,7 @@ describe('resolveAttention', () => { stateStartedAt: NOW - AGENT_STATUS_STALE_AFTER_MS - 60_000, updatedAt: NOW - AGENT_STATUS_STALE_AFTER_MS - 60_000 }) - expect(resolveAttention([entry], NOW)).toEqual(IDLE) + expect(resolveAttention([hookPane(entry)], NOW)).toEqual(IDLE) }) it('takes the most attention-demanding class across multiple panes', () => { @@ -192,7 +221,7 @@ describe('resolveAttention', () => { stateStartedAt: NOW - 1_000, updatedAt: NOW - 100 }) - expect(resolveAttention([done, working, blocked], NOW).cls).toBe(1) + expect(resolveAttention(hookPanes([done, working, blocked]), NOW).cls).toBe(1) }) it('within the resolved class, takes the freshest attention timestamp across panes', () => { @@ -208,11 +237,95 @@ describe('resolveAttention', () => { stateStartedAt: NOW - 5_000, updatedAt: NOW - 1_000 }) - expect(resolveAttention([olderBlocked, newerBlocked], NOW)).toEqual({ + expect(resolveAttention(hookPanes([olderBlocked, newerBlocked]), NOW)).toEqual({ cls: 1, - attentionTimestamp: NOW - 5_000 + attentionTimestamp: NOW - 5_000, + cause: 'blocked' }) }) + + it('skips entries with non-finite stateStartedAt', () => { + // Why: NaN > anything === false, so without the guard a corrupted entry + // would silently sink the worktree to the bottom of its class. + const corrupted = makeEntry({ + paneKey: 't:1', + state: 'blocked', + stateStartedAt: Number.NaN, + updatedAt: NOW - 1_000 + }) + expect(resolveAttention([hookPane(corrupted)], NOW)).toEqual(IDLE) + }) + + it('title-heuristic permission maps to Class 1 with ts = now', () => { + expect( + resolveAttention( + [{ kind: 'title', status: 'permission', worktreeLastActivityAt: NOW - 60_000 }], + NOW + ) + ).toEqual({ cls: 1, attentionTimestamp: NOW, cause: 'title-heuristic' }) + }) + + it('title-heuristic working maps to Class 3 with ts = worktree.lastActivityAt', () => { + expect( + resolveAttention( + [{ kind: 'title', status: 'working', worktreeLastActivityAt: NOW - 30_000 }], + NOW + ) + ).toEqual({ cls: 3, attentionTimestamp: NOW - 30_000 }) + }) + + it('title-heuristic idle / null contributes nothing (Class 4)', () => { + expect( + resolveAttention( + [ + { kind: 'title', status: 'idle', worktreeLastActivityAt: NOW - 1_000 }, + { kind: 'title', status: null, worktreeLastActivityAt: NOW - 1_000 } + ], + NOW + ) + ).toEqual(IDLE) + }) + + it('hook entry overrides title heuristic on the same pane (hook wins when fresh)', () => { + // Why: per-pane authority means a fresh hook entry beats whatever the + // title says — a 'done' hook plus a 'working'-classified title stays + // Class 2. + const done = makeEntry({ + paneKey: 't:1', + state: 'done', + stateStartedAt: NOW - 30_000, + updatedAt: NOW - 1_000 + }) + expect( + resolveAttention( + [ + hookPane(done), + { kind: 'title', status: 'working', worktreeLastActivityAt: NOW - 60_000 } + ], + NOW + ).cls + ).toBe(2) + }) + + it('per-pane authority across panes: pane A hook=done, pane B title=permission → Class 1', () => { + // Why: hook authority is per-pane, not per-worktree. A hookless pane + // showing 'permission' must still promote the whole worktree to Class 1. + const done = makeEntry({ + paneKey: 'tab:1', + state: 'done', + stateStartedAt: NOW - 30_000, + updatedAt: NOW - 1_000 + }) + expect( + resolveAttention( + [ + hookPane(done), + { kind: 'title', status: 'permission', worktreeLastActivityAt: NOW - 1_000 } + ], + NOW + ) + ).toEqual({ cls: 1, attentionTimestamp: NOW, cause: 'title-heuristic' }) + }) }) describe('buildAttentionByWorktree', () => { @@ -251,9 +364,17 @@ describe('buildAttentionByWorktree', () => { } } + function ptyMap(tabIds: string[]): Record { + const out: Record = {} + for (const id of tabIds) { + out[id] = ['pty-1'] + } + return out + } + it('returns IDLE for worktrees with no tabs', () => { const w = makeWorktree('wt-1') - const map = buildAttentionByWorktree([w], {}, {}, NOW) + const map = buildAttentionByWorktree([w], {}, {}, {}, {}, NOW) expect(map.get(w.id)).toEqual(IDLE) }) @@ -274,8 +395,12 @@ describe('buildAttentionByWorktree', () => { updatedAt: NOW - 1_000 }) } - const map = buildAttentionByWorktree([w], { [w.id]: [tab] }, entries, NOW) - expect(map.get(w.id)).toEqual({ cls: 1, attentionTimestamp: NOW - 5_000 }) + const map = buildAttentionByWorktree([w], { [w.id]: [tab] }, entries, {}, ptyMap([tab.id]), NOW) + expect(map.get(w.id)).toEqual({ + cls: 1, + attentionTimestamp: NOW - 5_000, + cause: 'blocked' + }) }) it('skips malformed paneKeys (no colon)', () => { @@ -292,6 +417,108 @@ describe('buildAttentionByWorktree', () => { updatedAt: NOW - 100 }) }, + {}, + ptyMap([tab.id]), + NOW + ) + expect(map.get(w.id)).toEqual(IDLE) + }) + + it('title-heuristic Class 1: hookless pane with permission title → Class 1 with ts = now', () => { + const w = makeWorktree('wt-1') + const tab = makeTab('tab-1', w.id) + const map = buildAttentionByWorktree( + [w], + { [w.id]: [tab] }, + {}, + { [tab.id]: { 1: '✋ Gemini CLI' } }, + ptyMap([tab.id]), + NOW + ) + expect(map.get(w.id)).toEqual({ + cls: 1, + attentionTimestamp: NOW, + cause: 'title-heuristic' + }) + }) + + it('title-heuristic Class 3: hookless pane with working title → ts = worktree.lastActivityAt', () => { + const w = { ...makeWorktree('wt-1'), lastActivityAt: NOW - 30_000 } + const tab = makeTab('tab-1', w.id) + const map = buildAttentionByWorktree( + [w], + { [w.id]: [tab] }, + {}, + { [tab.id]: { 1: '⠋ Claude' } }, + ptyMap([tab.id]), + NOW + ) + expect(map.get(w.id)).toEqual({ cls: 3, attentionTimestamp: NOW - 30_000 }) + }) + + it('hook overrides title on the same pane (hook=done + working-style title stays Class 2)', () => { + const w = makeWorktree('wt-1') + const tab = makeTab('tab-1', w.id) + const entries: Record = { + 'tab-1:1': makeEntry({ + paneKey: 'tab-1:1', + state: 'done', + stateStartedAt: NOW - 30_000, + updatedAt: NOW - 1_000 + }) + } + const map = buildAttentionByWorktree( + [w], + { [w.id]: [tab] }, + entries, + // Same paneId 1 — must NOT double-promote into Class 3. + { [tab.id]: { 1: '⠋ Claude' } }, + ptyMap([tab.id]), + NOW + ) + expect(map.get(w.id)).toEqual({ cls: 2, attentionTimestamp: NOW - 30_000 }) + }) + + it('per-pane authority across panes: pane A fresh hook=done, pane B no hook + permission title → Class 1', () => { + const w = makeWorktree('wt-1') + const tab = makeTab('tab-1', w.id) + const entries: Record = { + 'tab-1:1': makeEntry({ + paneKey: 'tab-1:1', + state: 'done', + stateStartedAt: NOW - 30_000, + updatedAt: NOW - 1_000 + }) + } + const map = buildAttentionByWorktree( + [w], + { [w.id]: [tab] }, + entries, + // Pane 2 has no hook — title fallback fires for it. + { [tab.id]: { 1: 'something', 2: '✋ Gemini CLI' } }, + ptyMap([tab.id]), + NOW + ) + expect(map.get(w.id)).toEqual({ + cls: 1, + attentionTimestamp: NOW, + cause: 'title-heuristic' + }) + }) + + it('does not fire title fallback for tabs without a live PTY', () => { + // Why: runtimePaneTitlesByTabId is preserved under sleep; without the + // tabHasLivePty gate, a slept tab whose preserved title still matches a + // working pattern would leak into the comparator. + const w = makeWorktree('wt-1') + const tab = makeTab('tab-1', w.id) + const map = buildAttentionByWorktree( + [w], + { [w.id]: [tab] }, + {}, + { [tab.id]: { 1: '✋ Gemini CLI' } }, + // No live pty for this tab. + {}, NOW ) expect(map.get(w.id)).toEqual(IDLE) diff --git a/src/renderer/src/components/sidebar/smart-attention.ts b/src/renderer/src/components/sidebar/smart-attention.ts index f42f84bb36d..98e3d83847f 100644 --- a/src/renderer/src/components/sidebar/smart-attention.ts +++ b/src/renderer/src/components/sidebar/smart-attention.ts @@ -1,4 +1,6 @@ -import { isExplicitAgentStatusFresh } from '@/lib/agent-status' +import { detectAgentStatusFromTitle, isExplicitAgentStatusFresh } from '@/lib/agent-status' +import { tabHasLivePty } from '@/lib/tab-has-live-pty' +import type { AgentStatus } from '../../../../shared/agent-detection' import type { TerminalTab, Worktree } from '../../../../shared/types' import { AGENT_STATUS_STALE_AFTER_MS, @@ -18,6 +20,15 @@ import { */ export type SmartClass = 1 | 2 | 3 | 4 +/** + * What surfaced a worktree into Class 1. Carried only for Class 1 results + * because that's the only class the telemetry promotion event reports on. + * - `blocked` / `waiting`: hook entry in that state. + * - `title-heuristic`: no fresh hook entry; runtime pane title classified + * as `'permission'` by `detectAgentStatusFromTitle`. + */ +export type AttentionCause = 'blocked' | 'waiting' | 'title-heuristic' + /** * Per-worktree resolution computed once before sorting. * @@ -29,10 +40,15 @@ export type SmartClass = 1 | 2 | 3 | 4 * `working` `stateStartedAt` when no prior attention event exists. * - Class 4: `0` — the comparator drops to `effectiveRecentActivity` for * within-class ordering on idle worktrees. + * + * `cause` is set only when `cls === 1`, and reflects the input that won the + * within-class max-timestamp comparison. Used for the + * `smart_sort_class_1_promotion` telemetry event. */ export type WorktreeAttention = { cls: SmartClass attentionTimestamp: number + cause?: AttentionCause } export const IDLE: WorktreeAttention = { cls: 4, attentionTimestamp: 0 } @@ -53,6 +69,12 @@ export function mostRecentAttentionInHistory(history: AgentStateHistoryEntry[]): continue } if (h.state === 'done' || h.state === 'blocked' || h.state === 'waiting') { + // Why: NaN is silently skipped by `>`, but Infinity from a corrupted + // row would pin the worktree at the top of Class 3 forever. Treat + // non-finite values as missing. + if (!Number.isFinite(h.startedAt)) { + continue + } if (h.startedAt > max) { max = h.startedAt } @@ -62,45 +84,89 @@ export function mostRecentAttentionInHistory(history: AgentStateHistoryEntry[]): } /** - * Resolve a worktree's class + attention timestamp from its panes' agent - * status entries. Stale entries (older than `AGENT_STATUS_STALE_AFTER_MS`) - * are skipped — the worktree falls to Class 4 if no fresh entry exists. + * One pane's contribution to a worktree's attention class. Hook entries from + * `agentStatusByPaneKey` are authoritative when fresh; otherwise we fall back + * to the terminal-title heuristic for hookless agents (Edge case 9 in the + * design doc). Hook authority is per-pane, not per-worktree — a worktree with + * a fresh hook on pane A and only a title on pane B mixes both branches. + */ +export type PaneInput = + | { kind: 'hook'; entry: AgentStatusEntry } + // Why: TerminalTab has no per-tab lastActivityAt; the worktree-level value + // is enough since within-class ordering compares across worktrees. + | { kind: 'title'; status: AgentStatus | null; worktreeLastActivityAt: number } + +/** + * Resolve a worktree's class + attention timestamp from its panes' inputs. + * Stale hook entries (older than `AGENT_STATUS_STALE_AFTER_MS`) are skipped + * — the worktree falls to Class 4 if no fresh hook entry and no recognized + * title heuristic exists. * * Across multiple panes: * - `cls` is the **min** (most attention-demanding pane wins). * - `attentionTimestamp` is the **max** within the resolved class. */ -export function resolveAttention(panes: AgentStatusEntry[], now: number): WorktreeAttention { +export function resolveAttention(panes: PaneInput[], now: number): WorktreeAttention { let bestCls: SmartClass = 4 let bestTs = 0 + let bestCause: AttentionCause | undefined - for (const entry of panes) { - if (!isExplicitAgentStatusFresh(entry, now, AGENT_STATUS_STALE_AFTER_MS)) { - continue - } - + for (const pane of panes) { let cls: SmartClass let ts: number - if (entry.state === 'blocked' || entry.state === 'waiting') { - cls = 1 - ts = entry.stateStartedAt - } else if (entry.state === 'done') { - // Why: an interrupted `done` (user pressed Ctrl+C) is the user signalling - // "I'm done with this turn". Treat as idle, not as Class 2 attention. - if (entry.interrupted) { + let cause: AttentionCause | undefined + + if (pane.kind === 'hook') { + const entry = pane.entry + if (!isExplicitAgentStatusFresh(entry, now, AGENT_STATUS_STALE_AFTER_MS)) { continue } - cls = 2 - ts = entry.stateStartedAt + // Why: defensive guard. NaN/Infinity from a corrupted stateStartedAt would + // poison comparisons (NaN > anything === false), silently dropping the + // worktree to the bottom of its class. Treat as a missing entry. + if (!Number.isFinite(entry.stateStartedAt)) { + continue + } + + if (entry.state === 'blocked' || entry.state === 'waiting') { + cls = 1 + ts = entry.stateStartedAt + cause = entry.state + } else if (entry.state === 'done') { + // Why: an interrupted `done` (user pressed Ctrl+C) is the user signalling + // "I'm done with this turn". Treat as idle, not as Class 2 attention. + if (entry.interrupted) { + continue + } + cls = 2 + ts = entry.stateStartedAt + } else { + // working + cls = 3 + // Why: within Class 3, sort by the most recent prior attention event so + // a worktree that just transitioned done→working stays above one that's + // been working for an hour. Falls back to the current stateStartedAt + // when stateHistory is empty (e.g. fresh after restart). + const prior = mostRecentAttentionInHistory(entry.stateHistory) + ts = prior ?? entry.stateStartedAt + } } else { - // working - cls = 3 - // Why: within Class 3, sort by the most recent prior attention event so - // a worktree that just transitioned done→working stays above one that's - // been working for an hour. Falls back to the current stateStartedAt - // when stateHistory is empty (e.g. fresh after restart). - const prior = mostRecentAttentionInHistory(entry.stateHistory) - ts = prior ?? entry.stateStartedAt + // Title-heuristic fallback (no fresh hook entry for this pane). Hook + // wins when fresh; this branch only fires for hookless panes. + if (pane.status === 'permission') { + cls = 1 + // Why now: the title detector exposes no stateStartedAt. Using `now` + // pins the worktree to the top of Class 1 until a hook event or the + // next sort, matching the user's "just noticed" mental model. + ts = now + cause = 'title-heuristic' + } else if (pane.status === 'working') { + cls = 3 + ts = pane.worktreeLastActivityAt + } else { + // 'idle' or null: nothing to assert; pane stays in Class 4. + continue + } } // Why min on class: smaller class number = higher priority. Any pane in a @@ -109,10 +175,13 @@ export function resolveAttention(panes: AgentStatusEntry[], now: number): Worktr if (cls < bestCls || (cls === bestCls && ts > bestTs)) { bestCls = cls bestTs = ts + bestCause = cause } } - return { cls: bestCls, attentionTimestamp: bestTs } + return bestCls === 1 && bestCause + ? { cls: bestCls, attentionTimestamp: bestTs, cause: bestCause } + : { cls: bestCls, attentionTimestamp: bestTs } } /** @@ -146,9 +215,30 @@ export function buildExplicitEntriesByTabId( return byTab } +/** + * Extract the paneId from a `${tabId}:${paneId}` paneKey, returning null for + * malformed keys (no colon or non-numeric tail). Used for per-pane authority: + * we need to know which paneIds already have a fresh hook entry so we don't + * double-count them via the title fallback. + */ +function paneIdFromPaneKey(paneKey: string): number | null { + const colon = paneKey.indexOf(':') + if (colon <= 0) { + return null + } + const id = Number(paneKey.slice(colon + 1)) + return Number.isFinite(id) ? id : null +} + /** * Build the per-worktree attention map consumed by the smart comparator. * + * Hook authority is per-pane: each pane that has a fresh hook entry uses it; + * each pane without one falls back to the title heuristic when its runtime + * pane title (or tab title for unmounted tabs) maps to a known status. The + * title branch is gated on `tabHasLivePty` so slept tabs whose preserved + * titles still match a working pattern don't leak through. + * * Cost: O(E + N × T × H) where E = total entries, N = worktrees, T = tabs per * worktree, H = history length (bounded at AGENT_STATE_HISTORY_MAX = 20). */ @@ -156,6 +246,8 @@ export function buildAttentionByWorktree( worktrees: Worktree[], tabsByWorktree: Record | null, agentStatusByPaneKey: Record | undefined, + runtimePaneTitlesByTabId: Record>, + ptyIdsByTabId: Record, now: number ): Map { const byTab = buildExplicitEntriesByTabId(agentStatusByPaneKey) @@ -167,14 +259,54 @@ export function buildAttentionByWorktree( result.set(worktree.id, IDLE) continue } - const panes: AgentStatusEntry[] = [] + const panes: PaneInput[] = [] for (const tab of tabs) { - const bucket = byTab.get(tab.id) - if (bucket) { - for (const entry of bucket) { - panes.push(entry) + const hookEntries = byTab.get(tab.id) + // Why: paneIds covered by a hook entry skip the title fallback so we + // don't double-count them. Hook authority is per-pane. + const hookPaneIds = new Set() + if (hookEntries) { + for (const entry of hookEntries) { + panes.push({ kind: 'hook', entry }) + const paneId = paneIdFromPaneKey(entry.paneKey) + if (paneId !== null) { + hookPaneIds.add(paneId) + } } } + + // Why gate on tabHasLivePty: runtimePaneTitlesByTabId is preserved under + // sleep (keepIdentifiers), so a slept tab whose pane titles still match + // a working pattern would otherwise leak into the comparator. + if (!tabHasLivePty(ptyIdsByTabId, tab.id)) { + continue + } + + const paneTitles = runtimePaneTitlesByTabId[tab.id] + if (paneTitles && Object.keys(paneTitles).length > 0) { + // Why: split-pane tabs can host multiple agents; each pane reports + // its own title. Mirrors the precedence used by getWorkingAgentsPerWorktree. + for (const [paneIdStr, title] of Object.entries(paneTitles)) { + const paneId = Number(paneIdStr) + if (hookPaneIds.has(paneId)) { + continue + } + panes.push({ + kind: 'title', + status: detectAgentStatusFromTitle(title), + worktreeLastActivityAt: worktree.lastActivityAt + }) + } + } else if (hookPaneIds.size === 0) { + // Why: tabs we have not mounted yet (restored-but-unvisited) only + // expose the legacy tab title. Fall back to it only when no pane-level + // titles or hook entries exist for this tab. + panes.push({ + kind: 'title', + status: detectAgentStatusFromTitle(tab.title), + worktreeLastActivityAt: worktree.lastActivityAt + }) + } } result.set(worktree.id, resolveAttention(panes, now)) } diff --git a/src/renderer/src/components/sidebar/smart-sort.test.ts b/src/renderer/src/components/sidebar/smart-sort.test.ts index 30d8ad60f2d..e5b0781aad1 100644 --- a/src/renderer/src/components/sidebar/smart-sort.test.ts +++ b/src/renderer/src/components/sidebar/smart-sort.test.ts @@ -87,6 +87,16 @@ function makeHistory( return { state, prompt: '', startedAt, interrupted: interrupted || undefined } } +function ptyMapForTabs(tabsByWorktree: Record): Record { + const out: Record = {} + for (const tabs of Object.values(tabsByWorktree)) { + for (const tab of tabs) { + out[tab.id] = ['pty-1'] + } + } + return out +} + /** * Sort helper: builds the attention map and runs the smart comparator. Mirrors * what callers do in production (visible-worktrees, WorktreeList). @@ -96,7 +106,14 @@ function sortSmart( tabsByWorktree: Record, agentStatusByPaneKey: Record ): Worktree[] { - const attention = buildAttentionByWorktree(worktrees, tabsByWorktree, agentStatusByPaneKey, NOW) + const attention = buildAttentionByWorktree( + worktrees, + tabsByWorktree, + agentStatusByPaneKey, + {}, + ptyMapForTabs(tabsByWorktree), + NOW + ) return [...worktrees].sort(buildWorktreeComparator('smart', repoMap, NOW, attention)) } @@ -429,7 +446,7 @@ describe('sortWorktreesSmart — cold start fallback', () => { const a = makeWorktree({ id: 'a', displayName: 'A', sortOrder: 1 }) const b = makeWorktree({ id: 'b', displayName: 'B', sortOrder: 2 }) // No tabs, no PTYs — cold start path. - const sorted = sortWorktreesSmart([a, b], {}, repoMap, {}) + const sorted = sortWorktreesSmart([a, b], {}, repoMap, {}, {}, {}) // Higher sortOrder wins on cold start. expect(sorted.map((w) => w.id)).toEqual(['b', 'a']) }) @@ -455,7 +472,14 @@ describe('sortWorktreesSmart — cold start fallback', () => { updatedAt: NOW - 1_000 }) } - const sorted = sortWorktreesSmart([done, blocked], tabsByWorktree, repoMap, entries) + const sorted = sortWorktreesSmart( + [done, blocked], + tabsByWorktree, + repoMap, + entries, + {}, + ptyMapForTabs(tabsByWorktree) + ) // Smart comparator wins over sortOrder because at least one PTY is live. expect(sorted.map((w) => w.id)).toEqual(['blocked', 'done']) }) @@ -491,7 +515,9 @@ describe('sortWorktreesSmart — palette caller regression', () => { [working, blocked], tabsByWorktree, repoMap, - agentStatusByPaneKey + agentStatusByPaneKey, + {}, + ptyMapForTabs(tabsByWorktree) ) expect(sorted.map((w) => w.id)).toEqual(['blocked', 'working']) }) @@ -511,7 +537,7 @@ describe('buildWorktreeComparator — recent (lastActivityAt)', () => { }) const worktrees = [older, newer] - worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW)) + worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW, new Map())) expect(worktrees.map((w) => w.id)).toEqual(['newer', 'older']) }) @@ -529,7 +555,7 @@ describe('buildWorktreeComparator — recent (lastActivityAt)', () => { }) const worktrees = [legacy, touched] - worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW)) + worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW, new Map())) expect(worktrees.map((w) => w.id)).toEqual(['touched', 'legacy']) }) @@ -547,7 +573,7 @@ describe('buildWorktreeComparator — recent (lastActivityAt)', () => { }) const worktrees = [bravo, alpha] - worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW)) + worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW, new Map())) expect(worktrees.map((w) => w.id)).toEqual(['alpha', 'bravo']) }) @@ -567,7 +593,7 @@ describe('buildWorktreeComparator — recent (lastActivityAt)', () => { }) const worktrees = [staleHighOrder, freshActive] - worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW)) + worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW, new Map())) expect(worktrees.map((w) => w.id)).toEqual(['fresh-active', 'stale-high-order']) }) @@ -621,7 +647,7 @@ describe('buildWorktreeComparator — recent with createdAt grace window', () => }) const worktrees = [bumpedByAmbient, newWorktree] - worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW)) + worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW, new Map())) expect(worktrees.map((w) => w.id)).toEqual(['new', 'bumped']) }) @@ -640,7 +666,7 @@ describe('buildWorktreeComparator — recent with createdAt grace window', () => }) const worktrees = [oldCreated, freshActivity] - worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW)) + worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW, new Map())) expect(worktrees.map((w) => w.id)).toEqual(['fresh-activity', 'old-created']) }) @@ -650,7 +676,7 @@ describe('buildWorktreeComparator — recent with createdAt grace window', () => const bravo = makeWorktree({ id: 'bravo', displayName: 'Bravo', lastActivityAt: 10_000 }) const worktrees = [alpha, bravo] - worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW)) + worktrees.sort(buildWorktreeComparator('recent', repoMap, NOW, new Map())) expect(worktrees.map((w) => w.id)).toEqual(['bravo', 'alpha']) }) diff --git a/src/renderer/src/components/sidebar/smart-sort.ts b/src/renderer/src/components/sidebar/smart-sort.ts index 76d3bc74ea9..64bbc1717b2 100644 --- a/src/renderer/src/components/sidebar/smart-sort.ts +++ b/src/renderer/src/components/sidebar/smart-sort.ts @@ -41,23 +41,23 @@ export function effectiveRecentActivity(worktree: Worktree, now: number): number * * Smart mode requires `attentionByWorktree` — a per-worktree class + * timestamp map built once before sorting (see `buildAttentionByWorktree`). - * Without it, every worktree collapses to Class 4 (idle) and the comparator - * silently degrades to recent-activity ordering, so callers MUST thread the - * map for smart mode. + * Why non-optional: a forgotten caller would silently regress every worktree + * to Class 4 (idle) and degrade the comparator to recent-activity ordering; + * making the param required surfaces the omission as a typecheck error. */ export function buildWorktreeComparator( sortBy: SortBy, repoMap: Map, - now: number = Date.now(), - attentionByWorktree: Map | null = null + now: number, + attentionByWorktree: Map ): (a: Worktree, b: Worktree) => number { return (a, b) => { switch (sortBy) { case 'name': return a.displayName.localeCompare(b.displayName) case 'smart': { - const aw = attentionByWorktree?.get(a.id) ?? IDLE - const bw = attentionByWorktree?.get(b.id) ?? IDLE + const aw = attentionByWorktree.get(a.id) ?? IDLE + const bw = attentionByWorktree.get(b.id) ?? IDLE return ( // Why: 1 < 2 < 3 < 4 — lower class outranks higher. aw.cls - bw.cls || @@ -108,15 +108,19 @@ export function buildWorktreeComparator( * Both the palette and `getVisibleWorktreeIds()` import this to avoid * duplicating the cold/warm branching logic. * - * `agentStatusByPaneKey` is required: the comparator depends on it for class - * resolution, and a forgotten caller would silently regress every worktree to - * Class 4. + * `agentStatusByPaneKey` carries the primary signal; `runtimePaneTitlesByTabId` + * and `ptyIdsByTabId` enable the title-heuristic fallback for hookless agents + * (Edge case 9 in the design doc). Why all three are non-optional: a forgotten + * caller would silently regress every worktree to Class 4 or quietly disable + * the hookless-fallback path. */ export function sortWorktreesSmart( worktrees: Worktree[], tabsByWorktree: Record, repoMap: Map, - agentStatusByPaneKey: Record + agentStatusByPaneKey: Record, + runtimePaneTitlesByTabId: Record>, + ptyIdsByTabId: Record ): Worktree[] { const hasAnyLivePty = Object.values(tabsByWorktree) .flat() @@ -135,6 +139,8 @@ export function sortWorktreesSmart( worktrees, tabsByWorktree, agentStatusByPaneKey, + runtimePaneTitlesByTabId, + ptyIdsByTabId, now ) diff --git a/src/renderer/src/components/sidebar/visible-worktrees.ts b/src/renderer/src/components/sidebar/visible-worktrees.ts index e4b9ba0e67f..2f6a632b834 100644 --- a/src/renderer/src/components/sidebar/visible-worktrees.ts +++ b/src/renderer/src/components/sidebar/visible-worktrees.ts @@ -184,11 +184,15 @@ export function getVisibleWorktreeIds(): string[] { allWorktrees, state.tabsByWorktree, repoMap, - state.agentStatusByPaneKey + state.agentStatusByPaneKey, + state.runtimePaneTitlesByTabId, + state.ptyIdsByTabId ).map((w) => w.id) } else { + // Why empty map: non-smart branches don't read attentionByWorktree, but + // the param is required to keep smart-mode callers honest at the type level. const sorted = [...allWorktrees].sort( - buildWorktreeComparator(state.sortBy, repoMap, Date.now()) + buildWorktreeComparator(state.sortBy, repoMap, Date.now(), new Map()) ) sortedIds = sorted.map((w) => w.id) } diff --git a/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts b/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts new file mode 100644 index 00000000000..e7368d76dcc --- /dev/null +++ b/src/renderer/src/store/slices/runtime-pane-title-sort-epoch.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) +vi.mock('@/runtime/sync-runtime-graph', () => ({ + scheduleRuntimeGraphSync: vi.fn() +})) +vi.mock('@/components/terminal-pane/pty-transport', () => ({ + registerEagerPtyBuffer: vi.fn(), + ensurePtyDispatcher: vi.fn(), + unregisterPtyDataHandlers: vi.fn() +})) +vi.mock('@/components/terminal-pane/shutdown-buffer-captures', () => ({ + shutdownBufferCaptures: vi.fn() +})) + +// @ts-expect-error -- minimal preload API stub for the slice's IPC writes +globalThis.window = { api: {} } + +import { createTestStore } from './store-test-helpers' + +describe('runtimePaneTitle → sortEpoch', () => { + it('bumps sortEpoch when the new title classifies differently than the previous title', () => { + // Why: smart sort's title-heuristic fallback (Edge case 9) reads + // runtimePaneTitlesByTabId. A hookless agent transitioning from + // 'working' to 'permission' must trigger a re-sort. + const store = createTestStore() + const before = store.getState().sortEpoch + store.getState().setRuntimePaneTitle('tab-1', 1, '⠋ Claude') + const afterWorking = store.getState().sortEpoch + expect(afterWorking).toBeGreaterThan(before) + store.getState().setRuntimePaneTitle('tab-1', 1, '✋ Gemini CLI') + expect(store.getState().sortEpoch).toBeGreaterThan(afterWorking) + }) + + it('does not bump sortEpoch when the classification is unchanged', () => { + // Why: incidental title noise (spinner frame, prompt suffix) shouldn't + // churn the sidebar order. + const store = createTestStore() + store.getState().setRuntimePaneTitle('tab-1', 1, '⠋ Claude') + const baseline = store.getState().sortEpoch + // Spinner frame change — still classifies as 'working'. + store.getState().setRuntimePaneTitle('tab-1', 1, '⠙ Claude') + expect(store.getState().sortEpoch).toBe(baseline) + }) + + it('bumps sortEpoch when clearing a classified title back to none', () => { + const store = createTestStore() + store.getState().setRuntimePaneTitle('tab-1', 1, '✋ Gemini CLI') + const baseline = store.getState().sortEpoch + store.getState().clearRuntimePaneTitle('tab-1', 1) + expect(store.getState().sortEpoch).toBeGreaterThan(baseline) + }) +}) diff --git a/src/renderer/src/store/slices/store-cascades.test.ts b/src/renderer/src/store/slices/store-cascades.test.ts index ca557502ae3..dead4c9d18b 100644 --- a/src/renderer/src/store/slices/store-cascades.test.ts +++ b/src/renderer/src/store/slices/store-cascades.test.ts @@ -403,7 +403,7 @@ describe('setActiveWorktree', () => { const worktrees = [...store.getState().worktreesByRepo.repo1] const repoMap = new Map(store.getState().repos.map((repo) => [repo.id, repo])) - worktrees.sort(buildWorktreeComparator('smart', repoMap, now)) + worktrees.sort(buildWorktreeComparator('smart', repoMap, now, new Map())) expect(worktrees.map((worktree) => worktree.id)).toEqual([backgroundId, focusedId]) }) diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index 562abda053b..a1a01bd1781 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -728,14 +728,24 @@ export const createTerminalSlice: StateCreator setRuntimePaneTitle: (tabId, paneId, title) => { set((s) => { const currentByPane = s.runtimePaneTitlesByTabId[tabId] ?? {} - if (currentByPane[paneId] === title) { + const prevTitle = currentByPane[paneId] + if (prevTitle === title) { return s } + // Why: smart sort's title-heuristic fallback (Edge case 9) reads + // runtimePaneTitlesByTabId. A hookless agent transitioning from + // 'working' → 'permission' via a title change must trigger a re-sort, + // otherwise the worktree stays in its old class until some unrelated + // event fires. Bumping only on classification change keeps incidental + // title noise (spinner frame, prompt suffix) from churning the sidebar. + const classificationChanged = + detectAgentStatusFromTitle(prevTitle ?? '') !== detectAgentStatusFromTitle(title) return { runtimePaneTitlesByTabId: { ...s.runtimePaneTitlesByTabId, [tabId]: { ...currentByPane, [paneId]: title } - } + }, + ...(classificationChanged ? { sortEpoch: s.sortEpoch + 1 } : {}) } }) }, @@ -746,6 +756,7 @@ export const createTerminalSlice: StateCreator if (!currentByPane || !(paneId in currentByPane)) { return s } + const prevTitle = currentByPane[paneId] const nextByPane = { ...currentByPane } delete nextByPane[paneId] @@ -756,7 +767,14 @@ export const createTerminalSlice: StateCreator delete next[tabId] } - return { runtimePaneTitlesByTabId: next } + // Why: clearing a 'working'/'permission'-classified title back to none + // changes the title-heuristic verdict for that pane, so the smart sort + // needs a re-sort. See setRuntimePaneTitle for the rationale. + const hadClassification = detectAgentStatusFromTitle(prevTitle ?? '') !== null + return { + runtimePaneTitlesByTabId: next, + ...(hadClassification ? { sortEpoch: s.sortEpoch + 1 } : {}) + } }) }, diff --git a/src/shared/telemetry-events.ts b/src/shared/telemetry-events.ts index 65661ddc7a6..410ce3bad29 100644 --- a/src/shared/telemetry-events.ts +++ b/src/shared/telemetry-events.ts @@ -494,6 +494,35 @@ const onboardingGhosttyDiscoveredSchema = z }) .strict() const onboardingGhosttyImportClickedSchema = z.object({ cohort: cohortSchema }).strict() + +// Why: smart-sort telemetry. The class distribution event tells us whether +// real users have meaningful Class 1/2/3 populations (signal that the +// redesign is doing work) or whether everyone collapses to Class 4 (signal +// that hook coverage is too low). The Class 1 promotion event distinguishes +// hook-driven attention from the title-heuristic fallback so we can tell +// whether Edge case 9 is carrying weight. The smart→recent switch event is +// our regression signal: users abandoning Smart for Recent. +const smartSortClassDistributionSchema = z + .object({ + class1: z.number().int().nonnegative(), + class2: z.number().int().nonnegative(), + class3: z.number().int().nonnegative(), + class4: z.number().int().nonnegative(), + totalWorktrees: z.number().int().nonnegative() + }) + .strict() +const smartSortClass1PromotionSchema = z + .object({ + cause: z.enum(['blocked', 'waiting', 'title-heuristic']) + }) + .strict() +// Why a placeholder field instead of `z.object({})`: an empty zod object +// infers as TS `{}` (which in TS means "anything non-null/undefined"). That +// upsets the `keyof EventMap[N]` probes used by COHORT_EXTENDED_SET and +// ONBOARDING_COHORT_SET, breaking their compile-time roster sync checks. +// Carrying a single optional `_v` discriminator dodges the issue and +// preserves room to add future fields without renaming the event. +const smartToRecentSwitchSchema = z.object({ _v: z.literal(1).optional() }).strict() const onboardingGhosttyImportFailedSchema = z .object({ // `'no_config'` is reserved for a future explicit "preview returned @@ -546,7 +575,11 @@ export const eventSchemas = { onboarding_ghostty_discovered: onboardingGhosttyDiscoveredSchema, onboarding_ghostty_import_clicked: onboardingGhosttyImportClickedSchema, onboarding_ghostty_import_failed: onboardingGhosttyImportFailedSchema, - activation_checklist_item_completed: activationChecklistItemCompletedSchema + activation_checklist_item_completed: activationChecklistItemCompletedSchema, + + smart_sort_class_distribution: smartSortClassDistributionSchema, + smart_sort_class_1_promotion: smartSortClass1PromotionSchema, + smart_to_recent_switch: smartToRecentSwitchSchema } as const export type EventMap = { [N in keyof typeof eventSchemas]: z.infer<(typeof eventSchemas)[N]> }