fix(smart-sort): correct liveness check + freshness gate from review

- smart-sort.ts cold-start: switch from `tab.ptyId` (wake-hint) to
  `tabHasLivePty(ptyIdsByTabId, tab.id)` so slept tabs after restart
  don't skip the persisted-sortOrder fallback.
- smart-attention.ts: only suppress the title-heuristic fallback for
  panes whose hook entry is fresh, so stale hooks no longer hide a
  live `permission` title behind a dead entry.
- WorktreeList.tsx: drop dead `prCache` subscription for smart mode
  (the new comparator doesn't read it) and refresh the now-stale
  cold-start comment.
- smart-sort.test.ts: pin the slept-tab cold-start path; correct a
  misleading comment about a not-actually-dropped heuristic.
- docs: align telemetry property names with the shipped schema and
  drop stale `WorktreeJumpPalette.tsx` line-number references.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
brennanb2025
2026-05-11 13:54:05 -07:00
co-authored by Orca
parent 3d21cd903e
commit 5bdaec12ae
5 changed files with 44 additions and 18 deletions
+5 -5
View File
@@ -154,7 +154,7 @@ Primary:
- `src/renderer/src/components/sidebar/smart-attention.test.ts` — **new file**: helper-level tests
- `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.
- `src/renderer/src/components/WorktreeJumpPalette.tsx` — thread `agentStatusByPaneKey`, `runtimePaneTitlesByTabId`, and `ptyIdsByTabId` into the two `sortWorktreesSmart(...)` calls (the typed-query and browser-tabs paths).
No changes:
- `src/shared/agent-status-types.ts` — already provides `AgentStatusEntry`, `AgentStateHistoryEntry.startedAt`, `AgentStateHistoryEntry.interrupted`, `AGENT_STATUS_STALE_AFTER_MS`, `AGENT_STATE_HISTORY_MAX`.
@@ -324,7 +324,7 @@ Callers today (verify before each edit):
- `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.
- The two `sortWorktreesSmart(...)` calls in `src/renderer/src/components/WorktreeJumpPalette.tsx` (the typed-query and browser-tabs paths) — updated to pass `agentStatusByPaneKey`, `runtimePaneTitlesByTabId`, and `ptyIdsByTabId` from the store.
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.
@@ -355,7 +355,7 @@ This section is the implementation handoff. Follow in order; each step lands a c
- 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`, `state.runtimePaneTitlesByTabId`, and `state.ptyIdsByTabId`.
- Update both `sortWorktreesSmart(...)` calls in `WorktreeJumpPalette.tsx` (the typed-query and browser-tabs paths) 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.
@@ -397,9 +397,9 @@ This section is the implementation handoff. Follow in order; each step lands a c
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_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: `class1`, `class2`, `class3`, `class4`, `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.
- **`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"). A future inverse event (`recent_to_smart_switch`) would pair with this for net-flow analysis; not shipped in v1 because the abandonment direction is the cleaner regression signal on its own.
Keep payloads small; no per-worktree IDs or paths in event properties.
@@ -563,10 +563,10 @@ const WorktreeList = React.memo(function WorktreeList() {
const cardProps = useAppStore((s) => s.worktreeCardProperties)
// PR cache is needed for PR-status grouping, smart sorting, and when the
// PR card property is visible.
// PR cache is needed for PR-status grouping and when the PR card property
// is visible.
const prCache = useAppStore((s) =>
groupBy === 'pr-status' || sortBy === 'smart' || cardProps.includes('pr') ? s.prCache : null
groupBy === 'pr-status' || cardProps.includes('pr') ? s.prCache : null
)
const sortEpoch = useAppStore((s) => s.sortEpoch)
@@ -645,13 +645,14 @@ const WorktreeList = React.memo(function WorktreeList() {
(worktree) => !worktree.isArchived
)
// Why cold-start detection: the smart score is dominated by ephemeral
// signals (running jobs +60, live terminals +12, needs attention +35)
// that vanish after restart. Recomputing the smart score on cold start
// produces a shuffled ordering because those signals are gone while
// persistent ones (unread, linked PR) survive — changing relative ranks.
// Instead, restore the pre-shutdown order from the persisted sortOrder
// snapshot, and switch to the live smart score once PTYs start spawning.
// Why cold-start detection: smart-class resolution depends on the
// agent-status snapshot (agentStatusByPaneKey) hydrating from the hook
// server, which lands asynchronously after launch. Running the warm
// comparator before that arrives would collapse every worktree to Class 4
// and shuffle the sidebar against the comparator's tiebreakers. Restore
// the pre-shutdown order from the persisted sortOrder snapshot until any
// live PTY appears, then switch to the live class layer. See Edge case 8
// in docs/smart-worktree-order-redesign.md.
if (sortBy === 'smart' && !sessionHasHadPty.current) {
// Why: `tabHasLivePty` (over `ptyIdsByTabId`) is the source of truth for
// liveness — slept terminals retain `tab.ptyId` as a wake hint, so reading
@@ -268,6 +268,13 @@ export function buildAttentionByWorktree(
if (hookEntries) {
for (const entry of hookEntries) {
panes.push({ kind: 'hook', entry })
// Why: only fresh hook entries should suppress the title-heuristic
// fallback for their pane. A stale hook is filtered out by
// resolveAttention; if we marked its pane as "hook-covered" we'd hide
// the live title behind a dead entry and drop the worktree to Class 4.
if (!isExplicitAgentStatusFresh(entry, now, AGENT_STATUS_STALE_AFTER_MS)) {
continue
}
const paneId = paneIdFromPaneKey(entry.paneKey)
if (paneId !== null) {
hookPaneIds.add(paneId)
@@ -451,6 +451,20 @@ describe('sortWorktreesSmart — cold start fallback', () => {
expect(sorted.map((w) => w.id)).toEqual(['b', 'a'])
})
it('treats slept tabs (tab.ptyId without live entry) as cold start', () => {
// Why: tab.ptyId is the wake-hint sessionId preserved under sleep — not a
// liveness signal. With slept tabs but no live PTYs, sortWorktreesSmart
// must fall back to persisted sortOrder.
const a = makeWorktree({ id: 'a', sortOrder: 1, displayName: 'a' })
const b = makeWorktree({ id: 'b', sortOrder: 2, displayName: 'b' })
const tabsByWorktree = {
[a.id]: [makeTab({ id: 'ta', worktreeId: a.id, ptyId: 'wake-hint' })]
}
// ptyIdsByTabId is empty — slept tab has wake-hint ptyId but no live entry.
const sorted = sortWorktreesSmart([a, b], tabsByWorktree, repoMap, {}, {}, {})
expect(sorted.map((w) => w.id)).toEqual(['b', 'a'])
})
it('uses the smart comparator once a PTY is alive', () => {
const blocked = makeWorktree({ id: 'blocked', displayName: 'Blocked', sortOrder: 0 })
const done = makeWorktree({ id: 'done', displayName: 'Done', sortOrder: 100 })
@@ -487,8 +501,8 @@ describe('sortWorktreesSmart — cold start fallback', () => {
describe('sortWorktreesSmart — palette caller regression', () => {
// Why: WorktreeJumpPalette routes typed queries through sortWorktreesSmart.
// This test pins that the palette path uses the class layer (not the
// dropped terminal-title heuristic) when threading agentStatusByPaneKey.
// This test pins that the palette path uses the class layer (not just the
// recent-activity fallback) when threading agentStatusByPaneKey.
it('palette ranks blocked above working when both flow through sortWorktreesSmart', () => {
const blocked = makeWorktree({ id: 'blocked', displayName: 'A-Blocked' })
const working = makeWorktree({ id: 'working', displayName: 'B-Working' })
@@ -1,5 +1,6 @@
import type { Worktree, Repo, TerminalTab } from '../../../../shared/types'
import type { AgentStatusEntry } from '../../../../shared/agent-status-types'
import { tabHasLivePty } from '@/lib/tab-has-live-pty'
import { IDLE, buildAttentionByWorktree, type WorktreeAttention } from './smart-attention'
type SortBy = 'name' | 'smart' | 'recent' | 'repo'
@@ -122,9 +123,12 @@ export function sortWorktreesSmart(
runtimePaneTitlesByTabId: Record<string, Record<number, string>>,
ptyIdsByTabId: Record<string, string[]>
): Worktree[] {
// Why: `tabHasLivePty` (over `ptyIdsByTabId`) is the source of truth for
// liveness — slept terminals retain `tab.ptyId` as a wake hint, so reading
// it directly would falsely keep cold-start ordering off after restart.
const hasAnyLivePty = Object.values(tabsByWorktree)
.flat()
.some((t) => t.ptyId)
.some((tab) => tabHasLivePty(ptyIdsByTabId, tab.id))
if (!hasAnyLivePty) {
// Cold start: use persisted sortOrder snapshot until the agent-status