perf(sidebar): stabilize worktree card props so cards memo-bail on order-preserving epoch bumps (#9392)

* perf(sidebar): stable per-group onLineageToggle identity so lineage-parent WorktreeCards memo-bail

Co-authored-by: Orca <help@stably.ai>

* perf(sidebar): reuse array identity for visible/rendered/selected worktrees so cards memo-bail on epoch bumps

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil 2026-07-20 19:14:27 -07:00 committed by GitHub
parent d834cc6303
commit 8182141fcd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 447 additions and 21 deletions

View File

@ -0,0 +1,323 @@
// @vitest-environment happy-dom
import { act, type ReactNode } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
import type { Repo, Worktree, WorktreeCardProperty } from '../../../../shared/types'
globalThis.IS_REACT_ACT_ENVIRONMENT = true
const mockStore = vi.hoisted(() => ({
state: {} as Record<string, unknown>,
activateWorktreeFromSidebar: vi.fn(),
openModal: vi.fn()
}))
// Counts invocations of the memo'd card's inner render function. A bail-out
// (React.memo shallow-equal props) does NOT invoke it — which is the claim
// under test: order-preserving epoch bumps must not re-render cards.
const cardRenderSpy = vi.hoisted(() => vi.fn())
type WorktreeListComponent = React.ComponentType<{
scrollOffsetRef: React.RefObject<number>
scrollAnchorRef: React.RefObject<unknown>
}>
let WorktreeList: WorktreeListComponent
vi.mock('@/store', () => {
const useAppStore = ((selector: (state: Record<string, unknown>) => unknown) =>
selector(mockStore.state)) as ((
selector: (state: Record<string, unknown>) => unknown
) => unknown) & {
getState: () => Record<string, unknown>
}
useAppStore.getState = () => mockStore.state
return { useAppStore }
})
vi.mock('@tanstack/react-virtual', () => ({
defaultRangeExtractor: ({ startIndex, endIndex }: { startIndex: number; endIndex: number }) =>
Array.from({ length: endIndex - startIndex + 1 }, (_, index) => startIndex + index),
measureElement: () => 32,
useVirtualizer: ({ count }: { count: number }) => ({
elementsCache: new Map(),
getTotalSize: () => count * 96,
getVirtualItems: () =>
Array.from({ length: count }, (_, index) => ({
index,
key: `row-${index}`,
start: index * 96
})),
measureElement: vi.fn(),
scrollToIndex: vi.fn()
})
}))
vi.mock('@/hooks/useVirtualizedScrollAnchor', () => ({
VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT: 'orca:test-record-scroll-anchor',
useVirtualizedScrollAnchor: vi.fn()
}))
vi.mock('./project-header-drag', () => ({
useRepoHeaderDrag: () => ({
state: { draggingRepoId: null, dropIndicatorY: null },
onHandlePointerDown: vi.fn()
}),
isRepoHeaderActionTarget: () => false
}))
vi.mock('@/components/ui/hover-card', () => ({
HoverCard: ({ children }: { children: ReactNode }) => <>{children}</>,
HoverCardContent: ({ children }: { children: ReactNode }) => <div>{children}</div>,
HoverCardTrigger: ({ children }: { children: ReactNode }) => <>{children}</>
}))
vi.mock('@/components/ui/tooltip', () => ({
Tooltip: ({ children }: { children: ReactNode }) => <>{children}</>,
TooltipContent: ({ children }: { children: ReactNode }) => <>{children}</>,
TooltipTrigger: ({ children }: { children: ReactNode }) => <>{children}</>
}))
vi.mock('@/components/ui/dropdown-menu', () => ({
DropdownMenu: ({ children }: { children: ReactNode }) => <>{children}</>,
DropdownMenuContent: ({ children }: { children: ReactNode }) => <div>{children}</div>,
DropdownMenuItem: ({ children, onSelect }: { children: ReactNode; onSelect?: () => void }) => (
<button onClick={onSelect}>{children}</button>
),
DropdownMenuSeparator: () => <hr />,
DropdownMenuSub: ({ children }: { children: ReactNode }) => <>{children}</>,
DropdownMenuSubContent: ({ children }: { children: ReactNode }) => <div>{children}</div>,
DropdownMenuSubTrigger: ({ children }: { children: ReactNode }) => <div>{children}</div>,
DropdownMenuTrigger: ({ children }: { children: ReactNode }) => <>{children}</>
}))
vi.mock('@/lib/sidebar-worktree-activation', () => ({
activateWorktreeFromSidebar: mockStore.activateWorktreeFromSidebar
}))
vi.mock('@/lib/worktree-activation', () => ({
activateAndRevealWorktree: vi.fn()
}))
vi.mock('@/runtime/runtime-rpc-client', () => ({
getActiveRuntimeTarget: () => ({ kind: 'local' }),
callRuntimeRpc: vi.fn()
}))
vi.mock('./WorktreeCardAgents', () => ({
default: () => <div>Agent row</div>,
SUPPRESS_WORKTREE_LIST_SCROLL_ADJUSTMENT_EVENT: 'orca:test-suppress-scroll-adjustment'
}))
vi.mock('./WorktreeCard', async () => {
const ReactModule = await import('react')
const MockWorktreeCard = ReactModule.memo(function WorktreeCard({
worktree
}: {
worktree: Worktree
}) {
cardRenderSpy(worktree.id)
return <div data-mock-worktree-card={worktree.id} />
})
return { default: MockWorktreeCard }
})
function makeRepo(): Repo {
return {
id: 'repo-1',
path: '/tmp/card-memo-stability',
displayName: 'card-memo-stability',
badgeColor: '#999999',
addedAt: 1
}
}
function makeWorktree(args: { id: string; displayName: string; sortOrder: number }): Worktree {
return {
id: args.id,
instanceId: `${args.id}-instance`,
repoId: 'repo-1',
path: `/tmp/card-memo-stability/${args.id}`,
displayName: args.displayName,
branch: `${args.id}-branch`,
head: 'abc123',
isBare: false,
isMainWorktree: false,
comment: '',
linkedIssue: null,
linkedPR: null,
linkedLinearIssue: null,
isArchived: false,
isUnread: false,
isPinned: false,
sortOrder: args.sortOrder,
lastActivityAt: args.sortOrder
}
}
function makeFolderWorkspacePathStatusState(): Record<string, unknown> {
return {
fetchFolderWorkspacePathStatus: vi.fn(),
folderWorkspacePathStatuses: {},
folderWorkspaces: [],
getFolderWorkspacePathStatusCacheKey: (request: unknown) => JSON.stringify(request),
getFreshFolderWorkspacePathStatus: vi.fn(() => null)
}
}
function setFlatWorktreeState(): void {
const repo = makeRepo()
const worktrees = [
makeWorktree({ id: 'wt-a', displayName: 'alpha', sortOrder: 20 }),
makeWorktree({ id: 'wt-b', displayName: 'beta', sortOrder: 10 })
]
mockStore.state = {
...makeFolderWorkspacePathStatusState(),
activeModal: '',
activeView: 'terminal',
activeWorktreeId: null,
agentStatusByPaneKey: {},
agentStatusEpoch: 0,
browserTabsByWorktree: {},
clearPendingRevealWorktreeId: vi.fn(),
collapsedGroups: new Set<string>(),
deleteStateByWorktreeId: {},
detectedWorktreesByRepo: {},
fetchHostedReviewForBranch: vi.fn(),
fetchIssue: vi.fn(),
fetchLinearIssue: vi.fn(),
filterRepoIds: [],
gitConflictOperationByWorktree: {},
groupBy: 'none',
hideDefaultBranchWorkspace: false,
hostedReviewCache: {},
issueCache: {},
linearIssueCache: {},
linearStatus: null,
migrationUnsupportedByPtyId: {},
openModal: mockStore.openModal,
openSettingsPage: vi.fn(),
openSettingsTarget: null,
openTaskPage: vi.fn(),
pendingRevealWorktree: null,
prCache: {},
projectGroups: [],
ptyIdsByTabId: {},
recordFeatureInteraction: vi.fn(),
remoteBranchConflictByWorktreeId: {},
reorderRepos: vi.fn(),
reportVisibleGitHubPRRefreshCandidates: vi.fn(),
repos: [repo],
retainedAgentsByPaneKey: {},
revealWorktreeInSidebar: vi.fn(),
runtimePaneTitlesByTabId: {},
setFilterRepoIds: vi.fn(),
setHideDefaultBranchWorkspace: vi.fn(),
setRenamingWorktreeId: vi.fn(),
setShowSleepingWorkspaces: vi.fn(),
setSortBy: vi.fn(),
setWorktreesPinnedAndReveal: vi.fn(),
settings: null,
showSleepingWorkspaces: true,
sortBy: 'manual',
sortEpoch: 0,
sshConnectedGeneration: 0,
sshConnectionStates: new Map(),
sshTargetLabels: new Map(),
tabsByWorktree: {},
terminalLayoutsByTabId: {},
toggleCollapsedGroup: vi.fn(),
updateRepo: vi.fn(),
updateWorktreeMeta: vi.fn(),
updateWorktreesMeta: vi.fn(),
workspaceHostScope: 'all',
workspacePortScan: null,
workspaceStatuses: [],
worktreeCardProperties: ['status', 'pr', 'comment'] satisfies WorktreeCardProperty[],
worktreeLineageById: {},
worktreesByRepo: {
[repo.id]: worktrees
}
}
}
const mountedRoots: Root[] = []
async function renderList(root: Root): Promise<void> {
await act(async () => {
// Why fresh ref objects each render: they defeat WorktreeList's own memo
// like a store-subscription re-render would, so the test exercises a full
// parent re-render and isolates whether the CARDS bail.
root.render(
<WorktreeList scrollOffsetRef={{ current: 0 }} scrollAnchorRef={{ current: null }} />
)
})
}
describe('WorktreeCard memo bail-out across epoch bumps', () => {
beforeAll(async () => {
WorktreeList = (await import('./WorktreeList')).default as WorktreeListComponent
}, 60_000)
beforeEach(() => {
vi.clearAllMocks()
setFlatWorktreeState()
})
afterEach(async () => {
await act(async () => {
for (const root of mountedRoots.splice(0)) {
root.unmount()
}
})
document.body.innerHTML = ''
})
it('does not re-render cards on an order-preserving sortEpoch bump', async () => {
const container = document.createElement('div')
document.body.appendChild(container)
const root = createRoot(container)
mountedRoots.push(root)
await renderList(root)
expect(container.querySelectorAll('[data-mock-worktree-card]')).toHaveLength(2)
// Baseline: a parent re-render with unchanged store state must not
// re-invoke card render functions.
const countAfterMount = cardRenderSpy.mock.calls.length
await renderList(root)
expect(cardRenderSpy.mock.calls.length).toBe(countAfterMount)
// Order-preserving epoch bump: same worktrees, same order, new epoch.
// Manual sort applies the bump without the smart-sort settle debounce.
mockStore.state = { ...mockStore.state, sortEpoch: 1 }
await renderList(root)
expect(cardRenderSpy.mock.calls.length).toBe(countAfterMount)
})
it('re-renders a card when its own worktree data changes', async () => {
const container = document.createElement('div')
document.body.appendChild(container)
const root = createRoot(container)
mountedRoots.push(root)
await renderList(root)
const countAfterMount = cardRenderSpy.mock.calls.length
const worktrees = (mockStore.state.worktreesByRepo as Record<string, Worktree[]>)['repo-1']!
mockStore.state = {
...mockStore.state,
sortEpoch: 2,
worktreesByRepo: {
'repo-1': [{ ...worktrees[0]!, displayName: 'alpha renamed' }, worktrees[1]!]
}
}
await renderList(root)
// The changed card re-renders; identity reuse must not freeze real updates.
expect(cardRenderSpy.mock.calls.length).toBeGreaterThan(countAfterMount)
expect(cardRenderSpy).toHaveBeenCalledWith('wt-a')
})
})

View File

@ -24,6 +24,8 @@ import {
Trash2
} from 'lucide-react'
import { useAppStore } from '@/store'
import { createLineageToggleHandlerCache } from './worktree-lineage-toggle-handler-cache'
import { reuseArrayIfEqual } from './worktree-agent-row-selectors'
import { useShallow } from 'zustand/react/shallow'
import type { AppState } from '@/store/types'
import {
@ -312,6 +314,14 @@ type ProjectGroupDeleteDialogState = {
removeContainedProjects: boolean
}
// Why: epoch-driven recomputes often produce arrays whose contents and order are unchanged; reusing the previous identity when element-wise equal keeps downstream memos and React.memo'd cards bailing out. Safe only because elements (Worktree objects / id strings) are immutably REPLACED on change — never wrap arrays of mutated-in-place objects.
function useReusedArrayIdentity<T>(next: T[]): T[] {
const previousRef = useRef<T[]>(next)
const result = reuseArrayIfEqual(previousRef.current, next)
previousRef.current = result
return result
}
// Debounce re-sort after a sortEpoch bump so background score changes don't jar row positions.
const SORT_SETTLE_MS = 3_000
const USER_SCROLL_MEASUREMENT_ADJUSTMENT_SUPPRESS_MS = 500
@ -2449,6 +2459,12 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
},
[recordCurrentScrollAnchor, toggleGroup]
)
// Why: memo'd WorktreeCard needs a per-group-key stable onLineageToggle
// identity to bail out of re-renders; see worktree-lineage-toggle-handler-cache.
const getLineageToggleHandler = useMemo(
() => createLineageToggleHandlerCache(toggleGroupWithScrollAnchor),
[toggleGroupWithScrollAnchor]
)
const navigateWorktree = useCallback(
(direction: 'up' | 'down') => {
@ -4856,11 +4872,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
lineageChildrenStyle={lineageChildrenStyle}
onLineageToggle={
lineageToggleGroupKey
? (event) => {
event.preventDefault()
event.stopPropagation()
toggleGroupWithScrollAnchor(lineageToggleGroupKey)
}
? getLineageToggleHandler(lineageToggleGroupKey)
: undefined
}
/>
@ -5462,7 +5474,7 @@ const WorktreeList = React.memo(function WorktreeList({
}, [sortedIds, sortBy])
// Flatten/filter/sort via the shared utility so card order matches Cmd+19 numbering.
const visibleWorktrees = useMemo(() => {
const recomputedVisibleWorktrees = useMemo(() => {
void agentStatusEpoch
const ids = computeVisibleWorktreeIds(worktreesByRepo, sortedIds, {
filterRepoIds,
@ -5514,6 +5526,10 @@ const WorktreeList = React.memo(function WorktreeList({
worktreeLineageById,
worktreesByRepo
])
// Why: agentStatusEpoch bumps recompute this memo even when membership and
// order are unchanged; keeping the previous identity stops the whole
// rows/sectionRows/renderedWorktrees chain from churning per epoch.
const visibleWorktrees = useReusedArrayIdentity(recomputedVisibleWorktrees)
const worktrees = visibleWorktrees
const collapsedGroups = useAppStore((s) => s.collapsedGroups)
@ -5842,9 +5858,14 @@ const WorktreeList = React.memo(function WorktreeList({
() => getRenderedWorktreesInSidebarOrder(sectionRows, pinnedDisplayPolicy),
[pinnedDisplayPolicy, sectionRows]
)
const renderedWorktreeIds = useMemo(
() => uniqueWorktreeIds(renderedWorktrees.map((worktree) => worktree.id)),
[renderedWorktrees]
// Why: order-preserving sectionRows rebuilds must not give this array a new
// identity — updateSelectionForGesture depends on it, and a fresh identity
// there defeats React.memo bail-out for every WorktreeCard on epoch bumps.
const renderedWorktreeIds = useReusedArrayIdentity(
useMemo(
() => uniqueWorktreeIds(renderedWorktrees.map((worktree) => worktree.id)),
[renderedWorktrees]
)
)
const [selectedWorktreeIds, setSelectedWorktreeIds] = useState<Set<string>>(new Set())
const [selectionAnchorId, setSelectionAnchorId] = useState<string | null>(null)
@ -5862,18 +5883,23 @@ const WorktreeList = React.memo(function WorktreeList({
setSelectionAnchorId(prunedSelection.anchorId)
}
const selectedWorktrees = useMemo(() => {
if (selectedWorktreeIds.size === 0) {
return []
}
const selected = new Map<string, Worktree>()
for (const worktree of renderedWorktrees) {
if (selectedWorktreeIds.has(worktree.id) && !selected.has(worktree.id)) {
selected.set(worktree.id, worktree)
// Why identity reuse: the empty/unchanged-selection case must keep one array
// identity — selectForContextMenu and both drag-start handlers depend on
// this array, and card memo bail-out depends on those staying stable.
const selectedWorktrees = useReusedArrayIdentity(
useMemo(() => {
if (selectedWorktreeIds.size === 0) {
return []
}
}
return Array.from(selected.values())
}, [renderedWorktrees, selectedWorktreeIds])
const selected = new Map<string, Worktree>()
for (const worktree of renderedWorktrees) {
if (selectedWorktreeIds.has(worktree.id) && !selected.has(worktree.id)) {
selected.set(worktree.id, worktree)
}
}
return Array.from(selected.values())
}, [renderedWorktrees, selectedWorktreeIds])
)
useEffect(() => {
if (selectedWorktreeIds.size === 0) {

View File

@ -51,7 +51,10 @@ let liveEntriesByWorktreeCache: LiveEntriesByWorktreeCache | null = null
let migrationUnsupportedByWorktreeCache: MigrationUnsupportedByWorktreeCache | null = null
let retainedEntriesByWorktreeCache: RetainedEntriesByWorktreeCache | null = null
function reuseArrayIfEqual<T>(previous: T[] | undefined, next: T[]): T[] {
// Why exported: WorktreeList reuses this exact-equality identity check to keep
// derived arrays referentially stable across order-preserving epoch bumps so
// memo'd cards can bail out of re-render.
export function reuseArrayIfEqual<T>(previous: T[] | undefined, next: T[]): T[] {
if (!previous || previous.length !== next.length) {
return next
}

View File

@ -0,0 +1,48 @@
import { describe, expect, it, vi } from 'vitest'
import type React from 'react'
import { createLineageToggleHandlerCache } from './worktree-lineage-toggle-handler-cache'
const makeEvent = (): React.MouseEvent<HTMLButtonElement> =>
({
preventDefault: vi.fn(),
stopPropagation: vi.fn()
}) as unknown as React.MouseEvent<HTMLButtonElement>
describe('createLineageToggleHandlerCache', () => {
it('returns a referentially stable handler for the same group key across calls', () => {
const getHandler = createLineageToggleHandlerCache(vi.fn())
const first = getHandler('lineage:alpha')
const second = getHandler('lineage:alpha')
// Why: stable identity is what lets React.memo'd WorktreeCard bail out.
expect(second).toBe(first)
})
it('returns distinct handlers for distinct group keys', () => {
const getHandler = createLineageToggleHandlerCache(vi.fn())
expect(getHandler('lineage:alpha')).not.toBe(getHandler('lineage:beta'))
})
it('prevents default, stops propagation, and toggles the bound group key', () => {
const toggleGroup = vi.fn()
const getHandler = createLineageToggleHandlerCache(toggleGroup)
const event = makeEvent()
getHandler('lineage:alpha')(event)
expect(event.preventDefault).toHaveBeenCalledTimes(1)
expect(event.stopPropagation).toHaveBeenCalledTimes(1)
expect(toggleGroup).toHaveBeenCalledExactlyOnceWith('lineage:alpha')
})
it('keeps each cached handler bound to its own group key', () => {
const toggleGroup = vi.fn()
const getHandler = createLineageToggleHandlerCache(toggleGroup)
getHandler('lineage:beta')(makeEvent())
expect(toggleGroup).toHaveBeenCalledExactlyOnceWith('lineage:beta')
})
})

View File

@ -0,0 +1,26 @@
import type React from 'react'
export type LineageToggleHandler = (event: React.MouseEvent<HTMLButtonElement>) => void
// Why: WorktreeCard is React.memo'd; an inline arrow per row would give
// onLineageToggle a fresh identity every render and defeat the memo bail-out
// for every lineage-parent card on each sort/status epoch bump. Cache one
// handler per group key (bounded by lineage-group count) so identity is stable.
export const createLineageToggleHandlerCache = (
toggleGroup: (groupKey: string) => void
): ((groupKey: string) => LineageToggleHandler) => {
const handlersByGroupKey = new Map<string, LineageToggleHandler>()
return (groupKey: string) => {
const cached = handlersByGroupKey.get(groupKey)
if (cached) {
return cached
}
const handler: LineageToggleHandler = (event) => {
event.preventDefault()
event.stopPropagation()
toggleGroup(groupKey)
}
handlersByGroupKey.set(groupKey, handler)
return handler
}
}