mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
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 <noreply@anthropic.com> * test(sidebar): stamp sort hook reads no live sources; stronger switch-on tests Co-Authored-By: Claude <noreply@anthropic.com> * test(sidebar): clear the order-save mock between tests Co-Authored-By: Claude <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
+89
@@ -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])
|
||||
})
|
||||
})
|
||||
@@ -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<string, WorktreeAtte
|
||||
// ── Stable sort order ──────────────────────────────────────────
|
||||
// Why sortEpoch (not selection): selection side-effects (clearing isUnread, PR-cache refresh) must not reorder the sidebar under the user.
|
||||
// Why useMemo not useEffect: order must be computed synchronously before the worktrees memo reads it.
|
||||
export function useSidebarWorktreeSortOrder(args: {
|
||||
function useSettledSidebarWorktreeSortOrder(args: {
|
||||
repoMap: Map<string, Repo>
|
||||
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<string, Repo>
|
||||
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
|
||||
|
||||
@@ -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<AppState>()(
|
||||
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 {
|
||||
|
||||
@@ -182,6 +182,14 @@ export type SmartSortSources = {
|
||||
terminalLayoutsByTabId?: Record<string, TerminalLayoutSnapshot>
|
||||
}
|
||||
|
||||
/** 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<W extends Worktree> = {
|
||||
sorted: W[]
|
||||
/** Smart only, and only once warm. */
|
||||
|
||||
Reference in New Issue
Block a user