From 63f0cb9570f581b7589534d9c630e54bd41debf7 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Sun, 18 Oct 2026 15:31:39 -0400 Subject: [PATCH] refactor(sidebar): the stamp-sorted window list runs no settle, latch or saved order (dark) (#30401) * refactor(sidebar): the stamp-sorted window list runs no settle, latch or saved order (dark) With ORCA_SIDEBAR_INPUTS=main the window's sort hook is a pure read of the rows' stamps: no settled-epoch store listener, no Smart latch, no saved Smart order writes, no class telemetry over the old attention map. Switch off: the existing hook, unchanged. Co-Authored-By: Claude * test(sidebar): stamp sort hook reads no live sources; stronger switch-on tests Co-Authored-By: Claude * test(sidebar): clear the order-save mock between tests Co-Authored-By: Claude * fix(sidebar): stamp sort reads a shared empty-sources constant, not a renderer layout literal The layout-writer ratchet counts layout field keys written in the renderer; the empty sources object now lives beside SmartSortSources in shared code. Co-Authored-By: Claude --------- Co-authored-by: Claude --- .../listing/use-sort-order-stamps.test.tsx | 89 +++++++++++++++++++ .../worktree-list/listing/use-sort-order.ts | 72 +++++++++++---- src/renderer/src/store/index.ts | 6 +- src/shared/sidebar-list/smart-sort.ts | 8 ++ 4 files changed, 159 insertions(+), 16 deletions(-) create mode 100644 src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order-stamps.test.tsx diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order-stamps.test.tsx b/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order-stamps.test.tsx new file mode 100644 index 00000000000..f5cebb189a3 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order-stamps.test.tsx @@ -0,0 +1,89 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it, vi } from 'vitest' +import { act, cleanup, renderHook } from '@testing-library/react' +import { useAppStore } from '@/store' +import type { Worktree } from '../../../../../../shared/worktree/types' +import { makeRepo, makeWorktree } from '../../../worktree-jump-palette-test-fixtures' +import { useSidebarWorktreeSortOrder } from './use-sort-order' + +vi.mock('../../../../../../shared/sidebar-inputs-switch', () => ({ + ORCA_SIDEBAR_INPUTS_IS_MAIN: true +})) +vi.mock('@/lib/sidebar-list-session', () => ({ useSidebarListSession: () => {} })) +const persistOrder = vi.hoisted(() => vi.fn()) +vi.mock('@/lib/worktree-sort-order-persistence', () => ({ + persistWorktreeSortOrderByHost: persistOrder +})) + +const initialState = useAppStore.getInitialState() +const repo = makeRepo() +const repoMap = new Map([[repo.id, repo]]) + +function setRows(worktrees: Worktree[]): void { + act(() => { + useAppStore.setState({ worktreesByRepo: { [repo.id]: worktrees } }) + }) +} + +describe('useSidebarWorktreeSortOrder with the switch on', () => { + afterEach(() => { + persistOrder.mockClear() + cleanup() + useAppStore.setState(initialState, true) + }) + + it('re-sorts as soon as a stamp lands, with no settle window and no saved order', () => { + const a = makeWorktree(`${repo.id}::/a`, 'a', { repoId: repo.id, lastActivityAt: 2 }) + const b = makeWorktree(`${repo.id}::/b`, 'b', { repoId: repo.id, lastActivityAt: 1 }) + useAppStore.setState({ sortBy: 'smart', worktreesByRepo: { [repo.id]: [a, b] } }) + const { result } = renderHook(() => useSidebarWorktreeSortOrder({ repoMap, sortBy: 'smart' })) + expect(result.current).toEqual([a.id, b.id]) + + const askingB: Worktree = { ...b, smartTier: 'needs-you', smartAt: 3 } + setRows([a, askingB]) + expect(result.current).toEqual([b.id, a.id]) + // The store keeps no settled epoch: a bump with a new row would settle it at once otherwise. + const c = makeWorktree(`${repo.id}::/c`, 'c', { repoId: repo.id, lastActivityAt: 9 }) + act(() => { + useAppStore.setState((s) => ({ + sortEpoch: s.sortEpoch + 1, + worktreesByRepo: { [repo.id]: [a, askingB, c] } + })) + }) + expect(useAppStore.getState().settledSortEpoch).toBe(initialState.settledSortEpoch) + expect(result.current).toEqual([b.id, c.id, a.id]) + expect(persistOrder).not.toHaveBeenCalled() + }) + + it('keeps the same order array on an unrelated row write, and re-sorts on a repo rename', () => { + const other = { ...makeRepo(), id: 'repo-2', displayName: 'Zed' } + const a = makeWorktree(`${repo.id}::/a`, 'a', { repoId: repo.id }) + const z = makeWorktree(`${other.id}::/z`, 'z', { repoId: other.id }) + useAppStore.setState({ worktreesByRepo: { [repo.id]: [a], [other.id]: [z] } }) + const maps = { + before: new Map([ + [repo.id, repo], + [other.id, other] + ]), + after: new Map([ + [repo.id, repo], + [other.id, { ...other, displayName: 'Aardvark' }] + ]) + } + const { result, rerender } = renderHook( + ({ map }) => useSidebarWorktreeSortOrder({ repoMap: map, sortBy: 'repo' }), + { initialProps: { map: maps.before } } + ) + const first = result.current + expect(first).toEqual([a.id, z.id]) + act(() => { + useAppStore.setState({ + worktreesByRepo: { [repo.id]: [{ ...a, isUnread: true }], [other.id]: [z] } + }) + }) + expect(result.current).toBe(first) + rerender({ map: maps.after }) + expect(result.current).toEqual([z.id, a.id]) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.ts b/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.ts index afc8a262a37..00dc155b8e5 100644 --- a/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.ts +++ b/src/renderer/src/components/sidebar/worktree-list/listing/use-sort-order.ts @@ -4,7 +4,11 @@ import { getAllWorktreesFromState } from '@/store/selectors' import { track } from '@/lib/telemetry' import { persistWorktreeSortOrderByHost } from '@/lib/worktree-sort-order-persistence' import type { Repo } from '../../../../../../shared/repo-types' -import { rankWorktreesForSort, type SortBy } from '../../../../../../shared/sidebar-list/smart-sort' +import { + NO_LIVE_SMART_SORT_SOURCES, + rankWorktreesForSort, + type SortBy +} from '../../../../../../shared/sidebar-list/smart-sort' import type { SmartClass, WorktreeAttention @@ -49,7 +53,7 @@ function trackSmartClassDistribution(attention: ReadonlyMap sortBy: SortBy }): string[] { @@ -57,8 +61,6 @@ export function useSidebarWorktreeSortOrder(args: { // Why settled (not live): the store coalesces bump bursts so rows don't jump (store/settled-sort-epoch.ts). const settledSortEpoch = useAppStore((s) => s.settledSortEpoch) const sortOrderEpoch = useAppStore((s) => s.sortOrderEpoch) - // Switch point: the host's stamps arrive on the rows, so a re-list is what re-sorts. - const stampedRows = useAppStore((s) => (ORCA_SIDEBAR_INPUTS_IS_MAIN ? s.worktreesByRepo : null)) // Why a latching ref: a live signal makes Smart authoritative for the session, even after that activity ends. const sessionHasHadLiveSmartSignal = useRef(false) @@ -84,9 +86,9 @@ export function useSidebarWorktreeSortOrder(args: { attentionByWorktree: ranking.attentionByWorktree, detectedLiveSmartSignal: ranking.detectedLiveSmartSignal } - // settledSortEpoch and stampedRows are triggers not read in the memo; a change signals a recompute. + // settledSortEpoch is an intentional trigger not read in the memo; its change signals a recompute. // oxlint-disable-next-line react-hooks/exhaustive-deps - }, [settledSortEpoch, persistedRankTrigger, stampedRows, repoMap, sortBy]) + }, [settledSortEpoch, persistedRankTrigger, repoMap, sortBy]) // Why: stable ID order prevents rank-only refreshes from echoing an unchanged snapshot. const sortedIds = useReusedArrayIdentity(recomputedSort.sortedIds) @@ -141,15 +143,7 @@ export function useSidebarWorktreeSortOrder(args: { hasTrackedSmartDistributionRef.current = true }, [sortBy, recomputedSort.attentionByWorktree, recomputedSort.sortedIds]) - // Why fire on the transition: switching away from Smart is the signal; compare via ref so a round-trip doesn't double-fire. - const prevSortByRef = useRef(sortBy) - useEffect(() => { - const prev = prevSortByRef.current - prevSortByRef.current = sortBy - if (prev === 'smart' && sortBy === 'recent') { - track('smart_to_recent_switch', {}) - } - }, [sortBy]) + useTrackSmartToRecentSwitch(sortBy) // Why: only persist during live sessions so cold start reads the persisted order instead of overwriting it. useEffect(() => { @@ -165,3 +159,51 @@ export function useSidebarWorktreeSortOrder(args: { return sortedIds } + +// Why fire on the transition: switching away from Smart is the signal; compare via ref so a round-trip doesn't double-fire. +function useTrackSmartToRecentSwitch(sortBy: SortBy): void { + const prevSortByRef = useRef(sortBy) + useEffect(() => { + const prev = prevSortByRef.current + prevSortByRef.current = sortBy + if (prev === 'smart' && sortBy === 'recent') { + track('smart_to_recent_switch', {}) + } + }, [sortBy]) +} + +/** + * Switch on: the order is a pure read of the owning hosts' stamps on the rows, so a re-list is + * what re-sorts. No settle window, latch, saved Smart order or create grace. + */ +function useStampSidebarWorktreeSortOrder(args: { + repoMap: Map + sortBy: SortBy +}): string[] { + const { repoMap, sortBy } = args + const worktreesByRepo = useAppStore((s) => s.worktreesByRepo) + const ranked = useMemo( + () => + rankWorktreesForSort({ + worktrees: getAllWorktreesFromState({ worktreesByRepo }).filter( + (worktree) => !worktree.isArchived + ), + sortBy, + latched: false, + repoMap, + // Stamps read no clock and no live sources. + now: 0, + sources: NO_LIVE_SMART_SORT_SOURCES + }).sorted.map((worktree) => worktree.id), + [worktreesByRepo, repoMap, sortBy] + ) + const sortedIds = useReusedArrayIdentity(ranked) + // Why: main's sleeping retention lives as long as this list. + useSidebarListSession(sortBy, sortedIds) + useTrackSmartToRecentSwitch(sortBy) + return sortedIds +} + +export const useSidebarWorktreeSortOrder = ORCA_SIDEBAR_INPUTS_IS_MAIN + ? useStampSidebarWorktreeSortOrder + : useSettledSidebarWorktreeSortOrder diff --git a/src/renderer/src/store/index.ts b/src/renderer/src/store/index.ts index 78a0437b2d5..a41e03c1c81 100644 --- a/src/renderer/src/store/index.ts +++ b/src/renderer/src/store/index.ts @@ -58,6 +58,7 @@ import { } from '@/lib/http-link-routing' import { installStoreListenerCensus } from './store-listener-census' import { installSettledSortEpoch } from './settled-sort-epoch' +import { ORCA_SIDEBAR_INPUTS_IS_MAIN } from '../../../shared/sidebar-inputs-switch' import { withReactCommitCascadeWriteProbe } from './react-commit-cascade-write-probe' import { withStoreIdentityChurnProbe } from './store-identity-churn-probe' import { @@ -79,7 +80,10 @@ export const useAppStore = create()( withDevelopmentStoreProbes( withReactCommitCascadeWriteProbe((...a) => { // Why first: settling inside the bump's own notify means hook subscribers never see it unsettled. - installSettledSortEpoch(a[2]) + // Switch point: the stamp-sorted list reads no settled epoch. + if (!ORCA_SIDEBAR_INPUTS_IS_MAIN) { + installSettledSortEpoch(a[2]) + } // Why: the inner api is only reachable here, before create() copies subscribe onto the hook. installStoreListenerCensus(a[2]) return { diff --git a/src/shared/sidebar-list/smart-sort.ts b/src/shared/sidebar-list/smart-sort.ts index 69f7baecb30..560d65a18a3 100644 --- a/src/shared/sidebar-list/smart-sort.ts +++ b/src/shared/sidebar-list/smart-sort.ts @@ -182,6 +182,14 @@ export type SmartSortSources = { terminalLayoutsByTabId?: Record } +/** For stamp-sorted callers: the stamps need no live terminal or agent sources. */ +export const NO_LIVE_SMART_SORT_SOURCES: SmartSortSources = Object.freeze({ + tabsByWorktree: {}, + agentStatusByPaneKey: {}, + runtimePaneTitlesByTabId: {}, + ptyIdsByTabId: {} +}) + export type WorktreeSortRanking = { sorted: W[] /** Smart only, and only once warm. */