refactor(sidebar): move the sidebar's time rules into shared pure reducers (#29943)

* refactor(sidebar): move sort settling, the Smart latch and sweep retention into shared pure reducers

* refactor(sidebar): let the settle reducer own the Manual and mode-switch rule
This commit is contained in:
Jinwoo Hong
2026-10-16 19:59:14 -07:00
committed by GitHub
parent f7a8e633af
commit 7bf3e150ed
14 changed files with 241 additions and 41 deletions
@@ -3,7 +3,7 @@ import { computeVisibleWorktreeIds } from './visible-worktrees'
import {
createSleepingSweepRetentionState,
updateSleepingSweepRetention
} from './sleeping-sweep-retention'
} from '../../../../shared/sidebar-list/sleeping-sweep-retention'
import { hasActiveWorkspaceActivity } from '@/lib/worktree-activity-state'
import { LOCAL_EXECUTION_HOST_ID } from '../../../../shared/execution-host'
import type { Repo } from '../../../../shared/repo-types'
@@ -8,7 +8,7 @@ import {
cloneSleepingSweepRetentionState,
createSleepingSweepRetentionState,
updateSleepingSweepRetention
} from '../../sleeping-sweep-retention'
} from '../../../../../../shared/sidebar-list/sleeping-sweep-retention'
type SweepActivityInputs = Pick<
VisibleWorktreeOptions,
@@ -360,4 +360,31 @@ describe('useSidebarWorktreeSortOrder', () => {
vi.unstubAllGlobals()
}
})
it('keeps Smart latched after its live signal ends, until the list unmounts', () => {
vi.stubGlobal('api', { worktrees: { persistSortOrder: vi.fn(async () => ({ updated: 0 })) } })
try {
seed(
[makeWorktree('a', 'A', { sortOrder: 2 }), makeWorktree('b', 'B', { sortOrder: 1 })],
'smart'
)
useAppStore.setState({
tabsByWorktree: { a: [makeTerminalTab({ id: 'tab-a', worktreeId: 'a' })] },
ptyIdsByTabId: { 'tab-a': ['pty-a'] }
})
const latched = renderSortOrder()
act(() => useAppStore.setState({ ptyIdsByTabId: {} }))
const before = latched.result.current
act(() => publishSortOrders({ a: 1, b: 2 }))
expect(latched.result.current).toBe(before)
latched.unmount()
const cold = renderSortOrder()
expect(cold.result.current).toEqual(['b', 'a'])
act(() => publishSortOrders({ a: 3 }))
expect(cold.result.current).toEqual(['a', 'b'])
} finally {
vi.unstubAllGlobals()
}
})
})
@@ -17,6 +17,10 @@ import {
type SmartClass,
type WorktreeAttention
} from '../../smart-attention'
import {
ranksFollowPersistedOrder,
smartSortPhase
} from '../../../../../../shared/sidebar-list/smart-sort-phase'
import { useReusedArrayIdentity } from './use-reused-array-identity'
function trackSmartClassDistribution(attention: ReadonlyMap<string, WorktreeAttention>): void {
@@ -63,10 +67,11 @@ export function useSidebarWorktreeSortOrder(args: {
// Why a latching ref: a live signal makes Smart authoritative for the session, even after that activity ends.
const sessionHasHadLiveSmartSignal = useRef(false)
// Why: cold Smart and Manual fallback rows rank by sortOrder and never persist it, so they can follow another client's save without looping.
const persistedRankTrigger =
sortBy === 'manual' || (sortBy === 'smart' && !sessionHasHadLiveSmartSignal.current)
? sortOrderEpoch
: 0
const followsPersistedRanks = ranksFollowPersistedOrder(
sortBy,
sessionHasHadLiveSmartSignal.current
)
const persistedRankTrigger = followsPersistedRanks ? sortOrderEpoch : 0
const recomputedSort = useMemo(() => {
const state = useAppStore.getState()
@@ -79,7 +84,7 @@ export function useSidebarWorktreeSortOrder(args: {
let detectedLiveSmartSignal = false
// Why cold-start detection: agent-status hydrates async, so the warm comparator would collapse all to Class 4; keep the persisted order until a live signal appears.
if (sortBy === 'smart' && !sessionHasHadLiveSmartSignal.current) {
if (smartSortPhase(sortBy, sessionHasHadLiveSmartSignal.current) === 'cold') {
// Why tabHasLivePty over tab.ptyId: slept terminals keep tab.ptyId as a wake hint, so it'd falsely keep cold-start ordering off.
const hasAnyLivePty = Object.values(state.tabsByWorktree).some((tabs) =>
tabs.some((tab) => tabHasLivePty(state.ptyIdsByTabId, tab.id))
@@ -189,7 +194,10 @@ export function useSidebarWorktreeSortOrder(args: {
// Why: only persist during live sessions so cold start reads the persisted order instead of overwriting it.
useEffect(() => {
if (sortBy !== 'smart' || sortedIds.length === 0 || !sessionHasHadLiveSmartSignal.current) {
if (
sortedIds.length === 0 ||
smartSortPhase(sortBy, sessionHasHadLiveSmartSignal.current) !== 'live'
) {
return
}
// Why: sortOrder lives in each host's worktreeMeta, so persist each host's ids on that host.
@@ -10,7 +10,7 @@ import {
makeTerminalTab,
makeWorktree
} from '../../../worktree-jump-palette-test-fixtures'
import { SLEEPING_SWEEP_SETTLE_MS } from '../../sleeping-sweep-retention'
import { SLEEPING_SWEEP_SETTLE_MS } from '../../../../../../shared/sidebar-list/sleeping-sweep-retention'
import { forgetWorktreeSleepIntent, markWorktreeSleepIntent } from '@/lib/worktree-sleep-intent'
import { useVisibleSidebarWorktrees } from './use-visible-worktrees'
@@ -1,6 +1,6 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { useAppStore } from './index'
import { SORT_SETTLE_MS } from './settled-sort-epoch'
import { SORT_SETTLE_MS } from '../../../shared/sidebar-list/sort-settle'
import { applyWebSessionTabsStorePatch } from '@/runtime/web-session-tabs-sync/store-patch'
import { makeWorktree } from '@/components/worktree-jump-palette-test-fixtures'
+33 -30
View File
@@ -8,12 +8,14 @@
* the settled value stays correct without any component mounted.
*/
import type { StoreApi } from 'zustand'
import {
INITIAL_SORT_SETTLE_STATE,
reduceSortSettle,
type SortSettleEvent
} from '../../../shared/sidebar-list/sort-settle'
import type { AppState } from './types'
import { getIndexedAllWorktrees } from './worktree-repo-index'
// Why: time-decaying scores would make rows jump on every bump; coalesce a burst into one re-sort.
export const SORT_SETTLE_MS = 3_000
function countLiveWorktrees(worktreesByRepo: AppState['worktreesByRepo']): number {
let count = 0
for (const worktree of getIndexedAllWorktrees(worktreesByRepo)) {
@@ -28,37 +30,38 @@ function countLiveWorktrees(worktreesByRepo: AppState['worktreesByRepo']): numbe
export function installSettledSortEpoch(
api: Pick<StoreApi<AppState>, 'setState' | 'subscribe'>
): void {
let settleState = INITIAL_SORT_SETTLE_STATE
let timer: ReturnType<typeof setTimeout> | undefined
// Why a baseline from the last bump (not the previous write): a row change that skipped
// its bump (stale-host purge) must not re-sort on its own.
let liveWorktreeCountAtLastBump = 0
const settle = (): void => api.setState((s) => ({ settledSortEpoch: s.sortEpoch }))
api.subscribe((state, previous) => {
const epochChanged = state.sortEpoch !== previous.sortEpoch
let structuralChange = false
// Why also on row writes: a bump-less add during a pending window must still settle now.
if (epochChanged || state.worktreesByRepo !== previous.worktreesByRepo) {
const count = countLiveWorktrees(state.worktreesByRepo)
structuralChange = count !== liveWorktreeCountAtLastBump
if (epochChanged) {
liveWorktreeCountAtLastBump = count
const dispatch = (event: SortSettleEvent): void => {
const now = Date.now()
const step = reduceSortSettle(settleState, event, now)
// Why: the timer mirrors the reducer's deadline, re-armed only when it moves.
if (step.state.deadline !== settleState.deadline) {
clearTimeout(timer)
if (step.state.deadline !== null) {
timer = setTimeout(() => dispatch({ kind: 'deadline' }), step.state.deadline - now)
}
}
if (state.settledSortEpoch === state.sortEpoch) {
// Why: any write that lands settled — settle() itself or a store reset — retires the pending timer.
clearTimeout(timer)
return
}
// Why: adds/removes, Manual (direct manipulation), and a mode switch never wait out the window.
if (structuralChange || state.sortBy === 'manual' || state.sortBy !== previous.sortBy) {
settle()
return
}
if (epochChanged) {
clearTimeout(timer)
timer = setTimeout(settle, SORT_SETTLE_MS)
settleState = step.state
if (step.settle) {
api.setState((s) => ({ settledSortEpoch: s.sortEpoch }))
}
}
api.subscribe((state, previous) => {
const epochBumped = state.sortEpoch !== previous.sortEpoch
// Why also on row writes: a bump-less add during a pending window must still settle now.
const rowsChanged = state.worktreesByRepo !== previous.worktreesByRepo
dispatch({
kind: 'write',
change:
epochBumped || rowsChanged
? { epochBumped, liveCount: countLiveWorktrees(state.worktreesByRepo) }
: null,
settled: state.settledSortEpoch === state.sortEpoch,
sortBy: state.sortBy,
previousSortBy: previous.sortBy
})
})
}
@@ -0,0 +1,17 @@
import { describe, expect, it } from 'vitest'
import { ranksFollowPersistedOrder, smartSortPhase } from './smart-sort-phase'
describe('smartSortPhase', () => {
it('is cold until latched, then live, and absent outside Smart', () => {
expect(smartSortPhase('smart', false)).toBe('cold')
expect(smartSortPhase('smart', true)).toBe('live')
expect(smartSortPhase('recent', true)).toBeNull()
})
it('follows persisted ranks under Manual and cold Smart only', () => {
expect(ranksFollowPersistedOrder('manual', true)).toBe(true)
expect(ranksFollowPersistedOrder('smart', false)).toBe(true)
expect(ranksFollowPersistedOrder('smart', true)).toBe(false)
expect(ranksFollowPersistedOrder('recent', false)).toBe(false)
})
})
@@ -0,0 +1,21 @@
import type { WorkspaceSortBy } from '../workspace-view/workspace-row-sort'
/**
* Smart's cold start. Agent status hydrates after the list mounts, so until the list has seen a
* live signal Smart keeps the persisted order and never writes it back. Once latched, Smart stays
* live for the list's lifetime, even after that activity ends.
*/
export type SmartSortPhase = 'cold' | 'live'
/** Null outside Smart. */
export function smartSortPhase(sortBy: WorkspaceSortBy, latched: boolean): SmartSortPhase | null {
if (sortBy !== 'smart') {
return null
}
return latched ? 'live' : 'cold'
}
/** Manual and cold Smart rank by persisted order, so they follow another client's save. */
export function ranksFollowPersistedOrder(sortBy: WorkspaceSortBy, latched: boolean): boolean {
return sortBy === 'manual' || smartSortPhase(sortBy, latched) === 'cold'
}
@@ -0,0 +1,66 @@
import { describe, expect, it } from 'vitest'
import {
reduceSortSettle,
SORT_SETTLE_MS,
type SortSettleEvent,
type SortSettleState
} from './sort-settle'
const settled = { liveCountAtLastBump: 2, deadline: null }
function write(overrides: Partial<Extract<SortSettleEvent, { kind: 'write' }>>): SortSettleEvent {
return {
kind: 'write',
change: { epochBumped: true, liveCount: 2 },
settled: false,
sortBy: 'recent',
previousSortBy: 'recent',
...overrides
}
}
describe('reduceSortSettle', () => {
it('restarts the window on every bump in a burst and settles once at its deadline', () => {
let state: SortSettleState = settled
for (const now of [0, 1_000, 2_500]) {
const step = reduceSortSettle(state, write({}), now)
expect(step).toEqual({ state: { ...settled, deadline: now + SORT_SETTLE_MS }, settle: false })
state = step.state
}
const quiet = reduceSortSettle(state, write({ change: null }), 4_000)
expect(quiet.state.deadline).toBe(2_500 + SORT_SETTLE_MS)
expect(reduceSortSettle(quiet.state, { kind: 'deadline' }, 5_500)).toEqual({
state: settled,
settle: true
})
})
it('settles adds and removes at once, against the count at the last bump', () => {
const pending = reduceSortSettle(settled, write({}), 0).state
expect(
reduceSortSettle(pending, write({ change: { epochBumped: false, liveCount: 3 } }), 10)
).toEqual({
state: settled,
settle: true
})
expect(
reduceSortSettle(pending, write({ change: { epochBumped: true, liveCount: 1 } }), 10)
).toEqual({
state: { liveCountAtLastBump: 1, deadline: null },
settle: true
})
})
it('settles Manual and mode switches without waiting', () => {
for (const modes of [{ sortBy: 'manual' }, { previousSortBy: 'smart' }] as const) {
expect(reduceSortSettle(settled, write(modes), 0)).toEqual({ state: settled, settle: true })
}
})
it('retires a pending deadline when a write lands settled', () => {
const pending = reduceSortSettle(settled, write({}), 0).state
const reset = reduceSortSettle(pending, write({ change: null, settled: true }), 10)
expect(reset).toEqual({ state: settled, settle: false })
expect(reduceSortSettle(reset.state, { kind: 'deadline' }, 3_000).settle).toBe(false)
})
})
+49
View File
@@ -0,0 +1,49 @@
import type { WorkspaceSortBy } from '../workspace-view/workspace-row-sort'
// Why: time-decaying scores would make rows jump on every bump; coalesce a burst into one re-sort.
export const SORT_SETTLE_MS = 3_000
export type SortSettleState = {
/** Why from the last bump (not the previous write): a row change that skipped its bump (stale-host purge) must not re-sort on its own. */
liveCountAtLastBump: number
/** When the pending burst settles; null when nothing is pending. */
deadline: number | null
}
export type SortSettleEvent =
| {
kind: 'write'
/** Null when the write changed neither sortEpoch nor the rows. */
change: { epochBumped: boolean; liveCount: number } | null
/** The write already lands settled: settling itself, or a store reset. */
settled: boolean
sortBy: WorkspaceSortBy
previousSortBy: WorkspaceSortBy
}
| { kind: 'deadline' }
export type SortSettleStep = { state: SortSettleState; settle: boolean }
export const INITIAL_SORT_SETTLE_STATE: SortSettleState = { liveCountAtLastBump: 0, deadline: null }
export function reduceSortSettle(
state: SortSettleState,
event: SortSettleEvent,
now: number
): SortSettleStep {
if (event.kind === 'deadline') {
return { state: { ...state, deadline: null }, settle: state.deadline !== null }
}
const { change } = event
const structuralChange = change !== null && change.liveCount !== state.liveCountAtLastBump
const liveCountAtLastBump = change?.epochBumped ? change.liveCount : state.liveCountAtLastBump
if (event.settled) {
return { state: { liveCountAtLastBump, deadline: null }, settle: false }
}
// Why: adds/removes, Manual (direct manipulation), and a mode switch never wait out the window.
if (structuralChange || event.sortBy === 'manual' || event.sortBy !== event.previousSortBy) {
return { state: { liveCountAtLastBump, deadline: null }, settle: true }
}
const deadline = change?.epochBumped ? now + SORT_SETTLE_MS : state.deadline
return { state: { liveCountAtLastBump, deadline }, settle: false }
}
@@ -7,7 +7,11 @@ import {
type WorkspaceRowFactReader,
type WorkspaceRowFilters
} from './workspace-row-filters'
import { buildWorkspaceRowComparator, recentActivityWithCreateGrace } from './workspace-row-sort'
import {
buildWorkspaceRowComparator,
CREATE_GRACE_MS,
recentActivityWithCreateGrace
} from './workspace-row-sort'
type Row = { id: string; folder?: boolean; facts?: Partial<Record<string, boolean>> }
@@ -70,6 +74,11 @@ describe('buildWorkspaceRowComparator', () => {
expect(recentActivityWithCreateGrace(10, Number.POSITIVE_INFINITY, 0)).toBe(10)
})
it('holds the create-grace floor until exactly createdAt + CREATE_GRACE_MS', () => {
expect(recentActivityWithCreateGrace(10, 0, CREATE_GRACE_MS - 1)).toBe(CREATE_GRACE_MS)
expect(recentActivityWithCreateGrace(10, 0, CREATE_GRACE_MS)).toBe(10)
})
it('puts unranked rows after ranked ones under manual order, then by name', () => {
const ranks: Record<string, number | null> = { a: null, b: 1, c: 5, d: null }
const compare = buildWorkspaceRowComparator<string>('manual', {