From e798864bf7f063bf0feaaa12161a8ce4e1a13d81 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 03:13:10 -0700 Subject: [PATCH] perf(sidebar): stop rebuilding per-card selector records on every store write Zustand re-runs every mounted subscriber's selector on every store write. The per-worktree sidebar selectors built a fresh Record per call, so 15 visible cards x 6 reads x every write allocated a record each time even when nothing they read had changed. - Add createWorktreeRecordSelector: gates the build on the source slice identities, memoizes per worktree id, and carries the previous generation forward so a rebuild with equal contents keeps its reference. - Route the pane-title, live-PTY, layout-root and terminal-layout selectors through it, and return a shared frozen empty when a worktree has no tabs. - Swap useWorktreeAgentRows' inactive-branch `[]`/`{}` literals for the shared frozen constants so the `active` gate actually short-circuits on identity. - Identity-cache the sidebar pending-worktree-creation key list, which ran Object.values(...).map(...) from an always-mounted subscriber. - Drop `key={text}` from TruncatedSidebarLabel so a label change remeasures in place instead of remounting the span and rebuilding its ResizeObserver. - Remove the non-compositable `width` from the board drop indicator's will-change hint. --- src/renderer/src/assets/main.css | 4 +- .../sidebar/truncated-sidebar-label.test.tsx | 36 ++++++ .../sidebar/truncated-sidebar-label.tsx | 19 +-- .../sidebar/useWorktreeAgentRows.ts | 47 ++++++-- .../worktree-agent-row-selectors.test.ts | 113 ++++++++++++++++-- .../sidebar/worktree-agent-row-selectors.ts | 38 ++++-- .../worktree-card-status-inputs.test.ts | 74 ++++++++++++ .../sidebar/worktree-card-status-inputs.ts | 84 ++++++++----- .../pending-worktree-creation-keys.test.ts | 53 ++++++++ .../listing/pending-worktree-creation-keys.ts | 39 ++++++ .../worktree-list/listing/use-section-rows.ts | 13 +- .../sidebar/worktree-record-selector-cache.ts | 63 ++++++++++ 12 files changed, 507 insertions(+), 76 deletions(-) create mode 100644 src/renderer/src/components/sidebar/worktree-list/listing/pending-worktree-creation-keys.test.ts create mode 100644 src/renderer/src/components/sidebar/worktree-list/listing/pending-worktree-creation-keys.ts create mode 100644 src/renderer/src/components/sidebar/worktree-record-selector-cache.ts diff --git a/src/renderer/src/assets/main.css b/src/renderer/src/assets/main.css index 1b5f40ccdb4..8187fe496c7 100644 --- a/src/renderer/src/assets/main.css +++ b/src/renderer/src/assets/main.css @@ -1956,7 +1956,9 @@ html.native-shell .app-layout { transform 120ms cubic-bezier(0.2, 0.8, 0.2, 1), width 120ms cubic-bezier(0.2, 0.8, 0.2, 1), opacity 80ms ease-out; - will-change: transform, width, opacity; + /* Why no `width`: it is not compositable, so hinting it only pins a layer that + has to be re-rastered every frame of the transition anyway. */ + will-change: transform, opacity; } [data-workspace-board-card-drop-indicator='true']::before, diff --git a/src/renderer/src/components/sidebar/truncated-sidebar-label.test.tsx b/src/renderer/src/components/sidebar/truncated-sidebar-label.test.tsx index 27ed816c53a..1de1fac0140 100644 --- a/src/renderer/src/components/sidebar/truncated-sidebar-label.test.tsx +++ b/src/renderer/src/components/sidebar/truncated-sidebar-label.test.tsx @@ -105,4 +105,40 @@ describe('TruncatedSidebarLabel', () => { expect(container.textContent).toContain('feature/really-long-branch-name') expect(container.querySelector('[data-tooltip-content]')).toBeNull() }) + + // Why: worktree titles change on the hot store-write path. Remounting the + // span per text change tore down and rebuilt its ResizeObserver every time. + it('keeps one ResizeObserver across a label text change', async () => { + const originalResizeObserver = globalThis.ResizeObserver + let constructed = 0 + let disconnected = 0 + class CountingResizeObserver { + constructor(_callback: ResizeObserverCallback) { + constructed += 1 + } + observe(): void {} + unobserve(): void {} + disconnect(): void { + disconnected += 1 + } + } + globalThis.ResizeObserver = CountingResizeObserver as unknown as typeof ResizeObserver + + try { + await act(async () => { + root.render() + }) + expect(constructed).toBe(1) + + await act(async () => { + root.render() + }) + + expect(container.textContent).toBe('fix/short') + expect(constructed).toBe(1) + expect(disconnected).toBe(0) + } finally { + globalThis.ResizeObserver = originalResizeObserver + } + }) }) diff --git a/src/renderer/src/components/sidebar/truncated-sidebar-label.tsx b/src/renderer/src/components/sidebar/truncated-sidebar-label.tsx index c7afaa2e9b0..80955b5bfe8 100644 --- a/src/renderer/src/components/sidebar/truncated-sidebar-label.tsx +++ b/src/renderer/src/components/sidebar/truncated-sidebar-label.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useState } from 'react' +import React, { useCallback, useLayoutEffect, useState } from 'react' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { cn } from '@/lib/utils' @@ -23,6 +23,7 @@ export function TruncatedSidebarLabel({ tooltipSide = 'right', tooltipSideOffset = 8 }: TruncatedSidebarLabelProps): React.JSX.Element { + const nodeRef = React.useRef(null) const resizeObserverRef = React.useRef(null) const removeResizeListenerRef = React.useRef<(() => void) | null>(null) const [truncated, setTruncated] = useState(false) @@ -39,6 +40,7 @@ export function TruncatedSidebarLabel({ removeResizeListenerRef.current?.() removeResizeListenerRef.current = null + nodeRef.current = node if (!node) { measureTruncated(null) return @@ -60,14 +62,15 @@ export function TruncatedSidebarLabel({ [measureTruncated] ) + // Why: ResizeObserver does not fire when only the rendered text changes, but + // scrollWidth can. Remeasure in place rather than remounting the span, which + // would tear down and rebuild the observer on every title update. + useLayoutEffect(() => { + measureTruncated(nodeRef.current) + }, [measureTruncated, text]) + const label = ( - + {text} ) diff --git a/src/renderer/src/components/sidebar/useWorktreeAgentRows.ts b/src/renderer/src/components/sidebar/useWorktreeAgentRows.ts index b2f45a8a9a0..366bce5e5ed 100644 --- a/src/renderer/src/components/sidebar/useWorktreeAgentRows.ts +++ b/src/renderer/src/components/sidebar/useWorktreeAgentRows.ts @@ -5,22 +5,33 @@ import { applyAgentRowLineage } from '@/components/dashboard/agent-row-lineage' import { migrationUnsupportedToAgentStatusEntry } from '@/lib/migration-unsupported-agent-entry' import { useAppStore } from '@/store' import { + EMPTY_LIVE_PTY_IDS, + EMPTY_RUNTIME_PANE_TITLES, selectLivePtyIdsForWorktree, selectRuntimePaneTitlesForWorktree } from './worktree-card-status-inputs' import { buildWorktreeAgentRows } from './worktree-agent-rows' import { + EMPTY_LIVE_ENTRIES, + EMPTY_MIGRATION_UNSUPPORTED_ENTRIES, + EMPTY_RETAINED, + EMPTY_TERMINAL_LAYOUTS, selectLiveAgentStatusEntriesForWorktree, selectMigrationUnsupportedEntriesForWorktree, selectRuntimeAgentOrchestrationForWorktree, selectRetainedAgentEntriesForWorktree, selectTerminalLayoutsForWorktree } from './worktree-agent-row-selectors' +import { EMPTY_WORKTREE_AGENT_ORCHESTRATION } from './worktree-agent-orchestration-index' +import { EMPTY_TABS } from './WorktreeCardHelpers' import { createWorktreeAgentFreshnessSelector, EMPTY_WORKTREE_AGENT_FRESHNESS_SIGNATURE } from './worktree-agent-freshness-selector' +// Why frozen: shared by every inactive card, and callers only read these rows. +const EMPTY_AGENT_ROWS = Object.freeze([]) as unknown as DashboardAgentRow[] + export { buildWorktreeAgentRows } from './worktree-agent-rows' export { selectLiveAgentStatusEntriesForWorktree, @@ -44,35 +55,51 @@ export function useWorktreeAgentRows(worktreeId: string, active = true): Dashboa () => createWorktreeAgentFreshnessSelector(worktreeId), [worktreeId] ) - const tabs = useAppStore((s) => (active ? s.tabsByWorktree[worktreeId] : undefined)) + const tabs = useAppStore((s) => (active ? s.tabsByWorktree[worktreeId] : EMPTY_TABS)) // Why: narrow the subscriptions to only THIS worktree's entries via // useShallow. Subscribing to the whole agentStatusByPaneKey map would make // every on-screen card re-render on any agent-status update anywhere — // O(worktrees²) render amplification. Pre-filtering here means the card // only re-renders when something relevant to THIS worktree changes. const liveEntries = useAppStore( - useShallow((s) => (active ? selectLiveAgentStatusEntriesForWorktree(s, worktreeId) : [])) + useShallow((s) => + active ? selectLiveAgentStatusEntriesForWorktree(s, worktreeId) : EMPTY_LIVE_ENTRIES + ) ) // Why: keep the store selector limited to stable raw records. Converting // migration entries creates fresh objects with Date.now(), which breaks // useSyncExternalStore's cached-snapshot contract and can blank Electron. const migrationUnsupported = useAppStore( - useShallow((s) => (active ? selectMigrationUnsupportedEntriesForWorktree(s, worktreeId) : [])) + useShallow((s) => + active + ? selectMigrationUnsupportedEntriesForWorktree(s, worktreeId) + : EMPTY_MIGRATION_UNSUPPORTED_ENTRIES + ) ) const retained = useAppStore( - useShallow((s) => (active ? selectRetainedAgentEntriesForWorktree(s, worktreeId) : [])) + useShallow((s) => + active ? selectRetainedAgentEntriesForWorktree(s, worktreeId) : EMPTY_RETAINED + ) ) const runtimePaneTitlesByTabId = useAppStore( - useShallow((s) => (active ? selectRuntimePaneTitlesForWorktree(s, worktreeId) : {})) + useShallow((s) => + active ? selectRuntimePaneTitlesForWorktree(s, worktreeId) : EMPTY_RUNTIME_PANE_TITLES + ) ) const ptyIdsByTabId = useAppStore( - useShallow((s) => (active ? selectLivePtyIdsForWorktree(s, worktreeId) : {})) + useShallow((s) => (active ? selectLivePtyIdsForWorktree(s, worktreeId) : EMPTY_LIVE_PTY_IDS)) ) const terminalLayoutsByTabId = useAppStore( - useShallow((s) => (active ? selectTerminalLayoutsForWorktree(s, worktreeId) : {})) + useShallow((s) => + active ? selectTerminalLayoutsForWorktree(s, worktreeId) : EMPTY_TERMINAL_LAYOUTS + ) ) const runtimeAgentOrchestrationByPaneKey = useAppStore( - useShallow((s) => (active ? selectRuntimeAgentOrchestrationForWorktree(s, worktreeId) : {})) + useShallow((s) => + active + ? selectRuntimeAgentOrchestrationForWorktree(s, worktreeId) + : EMPTY_WORKTREE_AGENT_ORCHESTRATION + ) ) const agentFreshnessSignature = useAppStore((s) => active ? selectAgentFreshness(s) : EMPTY_WORKTREE_AGENT_FRESHNESS_SIGNATURE @@ -80,7 +107,7 @@ export function useWorktreeAgentRows(worktreeId: string, active = true): Dashboa return useMemo(() => { if (!active) { - return [] + return EMPTY_AGENT_ROWS } // Why: Date.now() is read inside the memo so stale-decay recalculates when // this worktree's freshness signature changes, even without new PTY data. @@ -97,7 +124,7 @@ export function useWorktreeAgentRows(worktreeId: string, active = true): Dashboa : liveEntries return applyAgentRowLineage( buildWorktreeAgentRows({ - tabs: tabs ?? [], + tabs: tabs ?? EMPTY_TABS, entries, retained, runtimePaneTitlesByTabId, diff --git a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.test.ts b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.test.ts index cbd3da64212..3e4e6defc6d 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.test.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.test.ts @@ -9,10 +9,15 @@ import type { RetainedAgentEntry } from '@/store/slices/agent-status' import { makePaneKey } from '../../../../shared/stable-pane-id' import { getLiveEntriesFullRebuildCountForTests } from './worktree-agent-live-index-patch' import { + EMPTY_LIVE_ENTRIES, + EMPTY_MIGRATION_UNSUPPORTED_ENTRIES, + EMPTY_RETAINED, + EMPTY_TERMINAL_LAYOUTS, selectLiveAgentStatusEntriesForWorktree, selectMigrationUnsupportedEntriesForWorktree, selectRuntimeAgentOrchestrationForWorktree, - selectRetainedAgentEntriesForWorktree + selectRetainedAgentEntriesForWorktree, + selectTerminalLayoutsForWorktree } from './worktree-agent-row-selectors' const PANE_KEY_1 = makePaneKey('tab-1', '22222222-2222-4222-8222-222222222222') @@ -94,8 +99,14 @@ describe('selectMigrationUnsupportedEntriesForWorktree', () => { describe('selectLiveAgentStatusEntriesForWorktree', () => { it('reuses unaffected worktree arrays when another worktree receives a same-state ping', () => { - const wt1Entry = makeEntry(PANE_KEY_1, 1000, { state: 'working', prompt: 'first' }) - const wt2Entry = makeEntry(PANE_KEY_2, 1000, { state: 'working', prompt: 'first' }) + const wt1Entry = makeEntry(PANE_KEY_1, 1000, { + state: 'working', + prompt: 'first' + }) + const wt2Entry = makeEntry(PANE_KEY_2, 1000, { + state: 'working', + prompt: 'first' + }) const state = { tabsByWorktree: { 'wt-1': [makeTab('tab-1')], @@ -192,8 +203,14 @@ describe('selectLiveAgentStatusEntriesForWorktree', () => { }) it('patches instead of full-rebuilding across within-state pings, and stays correct on transitions', () => { - const wt1Entry = makeEntry(PANE_KEY_1, 1000, { state: 'working', prompt: 'wt1 prompt' }) - const wt2Entry = makeEntry(PANE_KEY_2, 1000, { state: 'working', prompt: 'wt2 prompt' }) + const wt1Entry = makeEntry(PANE_KEY_1, 1000, { + state: 'working', + prompt: 'wt1 prompt' + }) + const wt2Entry = makeEntry(PANE_KEY_2, 1000, { + state: 'working', + prompt: 'wt2 prompt' + }) const baseState = { tabsByWorktree: { 'wt-1': [makeTab('tab-1')], @@ -256,7 +273,10 @@ describe('selectLiveAgentStatusEntriesForWorktree', () => { }) it('falls back to a full rebuild when a within-map update changes worktree attribution', () => { - const entry = makeEntry(PANE_KEY_1, 1000, { state: 'working', worktreeId: 'wt-1' }) + const entry = makeEntry(PANE_KEY_1, 1000, { + state: 'working', + worktreeId: 'wt-1' + }) const state = { // No tab membership: bucketing comes from entry.worktreeId attribution. tabsByWorktree: { 'wt-1': [], 'wt-2': [] }, @@ -276,7 +296,10 @@ describe('selectLiveAgentStatusEntriesForWorktree', () => { }) it('falls back to a full rebuild when a live entry completes with its tab gone', () => { - const entry = makeEntry(PANE_KEY_1, 1000, { state: 'working', worktreeId: 'wt-1' }) + const entry = makeEntry(PANE_KEY_1, 1000, { + state: 'working', + worktreeId: 'wt-1' + }) const state = { tabsByWorktree: { 'wt-1': [] }, agentStatusByPaneKey: { [PANE_KEY_1]: entry }, @@ -427,3 +450,79 @@ describe('selectRetainedAgentEntriesForWorktree', () => { expect(secondWt2[0]?.startedAt).toBe(1100) }) }) + +describe('selectTerminalLayoutsForWorktree', () => { + const layout = { + root: { type: 'leaf', leafId: '44444444-4444-4444-8444-444444444444' }, + activeLeafId: '44444444-4444-4444-8444-444444444444', + expandedLeafId: null, + ptyIdsByLeafId: {} + } as const + + // Why: every visible card re-runs this selector on every store write; a fresh + // record per call is an allocation per card per write. + it('returns one identity per store generation', () => { + const state = { + tabsByWorktree: { 'wt-1': [makeTab('tab-1')] }, + terminalLayoutsByTabId: { 'tab-1': layout } + } + + expect(selectTerminalLayoutsForWorktree(state, 'wt-1')).toBe( + selectTerminalLayoutsForWorktree(state, 'wt-1') + ) + }) + + it('returns the shared frozen empty for a worktree with no tabs', () => { + const state = { tabsByWorktree: {}, terminalLayoutsByTabId: {} } + + expect(selectTerminalLayoutsForWorktree(state, 'missing')).toBe(EMPTY_TERMINAL_LAYOUTS) + }) +}) + +// Why: the inline-agents hook short-circuits on `active`; the off branch has to +// hand back a shared identity or the gate allocates on every store write. +describe('inactive-card empty constants', () => { + it('are frozen and shared', () => { + for (const empty of [ + EMPTY_LIVE_ENTRIES, + EMPTY_MIGRATION_UNSUPPORTED_ENTRIES, + EMPTY_RETAINED, + EMPTY_TERMINAL_LAYOUTS + ]) { + expect(Object.isFrozen(empty)).toBe(true) + } + expect( + selectLiveAgentStatusEntriesForWorktree( + { + agentStatusByPaneKey: {}, + migrationUnsupportedByPtyId: {}, + retainedAgentsByPaneKey: {}, + tabsByWorktree: {} + }, + 'missing' + ) + ).toBe(EMPTY_LIVE_ENTRIES) + expect( + selectMigrationUnsupportedEntriesForWorktree( + { + agentStatusByPaneKey: {}, + migrationUnsupportedByPtyId: {}, + retainedAgentsByPaneKey: {}, + tabsByWorktree: {} + }, + 'missing' + ) + ).toBe(EMPTY_MIGRATION_UNSUPPORTED_ENTRIES) + expect( + selectRetainedAgentEntriesForWorktree( + { + agentStatusByPaneKey: {}, + migrationUnsupportedByPtyId: {}, + retainedAgentsByPaneKey: {}, + tabsByWorktree: {} + }, + 'missing' + ) + ).toBe(EMPTY_RETAINED) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts index 4947fce433f..99f88f8e32c 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts @@ -13,11 +13,18 @@ import { recordLiveEntriesFullRebuild } from './worktree-agent-live-index-patch' import { selectWorktreeAgentOrchestration } from './worktree-agent-orchestration-index' +import { createWorktreeRecordSelector } from './worktree-record-selector-cache' import type { TerminalLayoutSnapshot } from '../../../../shared/terminal-tab-types' -const EMPTY_LIVE_ENTRIES: AgentStatusEntry[] = [] -const EMPTY_MIGRATION_UNSUPPORTED_ENTRIES: MigrationUnsupportedPtyEntry[] = [] -const EMPTY_RETAINED: RetainedAgentEntry[] = [] +// Why frozen and exported: card hooks return these from their inactive branch, +// so the identity has to be shared app-wide and safe from stray writes. +export const EMPTY_LIVE_ENTRIES = Object.freeze([]) as unknown as AgentStatusEntry[] +export const EMPTY_MIGRATION_UNSUPPORTED_ENTRIES = Object.freeze( + [] +) as unknown as MigrationUnsupportedPtyEntry[] +export const EMPTY_RETAINED = Object.freeze([]) as unknown as RetainedAgentEntry[] +export const EMPTY_TERMINAL_LAYOUTS: Record = + Object.freeze({}) // Why: selector unit tests often pass partial store mocks; production state // owns these maps, but missing mock maps should behave like empty slices. const EMPTY_RECORD = {} @@ -268,13 +275,20 @@ export function selectRuntimeAgentOrchestrationForWorktree( return selectWorktreeAgentOrchestration(state, worktreeId) } -export function selectTerminalLayoutsForWorktree( - state: Pick, - worktreeId: string -): Record { - const out: Record = {} - for (const tab of (state.tabsByWorktree ?? EMPTY_RECORD)[worktreeId] ?? []) { - out[tab.id] = (state.terminalLayoutsByTabId ?? EMPTY_RECORD)[tab.id] +export const selectTerminalLayoutsForWorktree = createWorktreeRecordSelector< + Pick, + Record +>({ + readSources: (state) => [ + state.tabsByWorktree ?? EMPTY_RECORD, + state.terminalLayoutsByTabId ?? EMPTY_RECORD + ], + empty: EMPTY_TERMINAL_LAYOUTS, + build: (state, worktreeId) => { + const out: Record = {} + for (const tab of (state.tabsByWorktree ?? EMPTY_RECORD)[worktreeId] ?? []) { + out[tab.id] = (state.terminalLayoutsByTabId ?? EMPTY_RECORD)[tab.id] + } + return out } - return out -} +}) diff --git a/src/renderer/src/components/sidebar/worktree-card-status-inputs.test.ts b/src/renderer/src/components/sidebar/worktree-card-status-inputs.test.ts index 7d366b845e8..59675017b2a 100644 --- a/src/renderer/src/components/sidebar/worktree-card-status-inputs.test.ts +++ b/src/renderer/src/components/sidebar/worktree-card-status-inputs.test.ts @@ -6,6 +6,9 @@ import type { TerminalTab } from '../../../../shared/terminal-tab-types' import { + EMPTY_LIVE_PTY_IDS, + EMPTY_RUNTIME_PANE_TITLES, + EMPTY_TERMINAL_LAYOUT_ROOTS, selectLivePtyIdsForWorktree, selectTerminalLayoutRootsForWorktree, selectTerminalLayoutRootsForWorktrees, @@ -145,4 +148,75 @@ describe('worktree card status input selectors', () => { ) ).toBe(true) }) + + // Why: zustand re-runs every mounted card's selector on every store write, so + // a fresh record per call multiplies by (visible cards x writes/sec). + it('returns one identity per store generation instead of rebuilding per call', () => { + const worktreeId = 'repo1::/path/wt1' + const state: SelectorState & LayoutRootSelectorState = { + tabsByWorktree: { + [worktreeId]: [makeTab('tab-1', worktreeId)] + }, + runtimePaneTitlesByTabId: { 'tab-1': { 0: 'codex [working]' } }, + ptyIdsByTabId: { 'tab-1': ['pty-1'] }, + terminalLayoutsByTabId: { + 'tab-1': makeLayout( + { type: 'leaf', leafId: '11111111-1111-4111-8111-111111111111' }, + 'pty-1' + ) + } + } + + expect(selectRuntimePaneTitlesForWorktree(state, worktreeId)).toBe( + selectRuntimePaneTitlesForWorktree(state, worktreeId) + ) + expect(selectLivePtyIdsForWorktree(state, worktreeId)).toBe( + selectLivePtyIdsForWorktree(state, worktreeId) + ) + expect(selectTerminalLayoutRootsForWorktree(state, worktreeId)).toBe( + selectTerminalLayoutRootsForWorktree(state, worktreeId) + ) + }) + + it('carries the same identity across unrelated pane-title and PTY churn', () => { + const worktreeId = 'repo1::/path/wt1' + const state: SelectorState = { + tabsByWorktree: { + [worktreeId]: [makeTab('tab-1', worktreeId)] + }, + runtimePaneTitlesByTabId: { 'tab-1': { 0: 'codex [working]' } }, + ptyIdsByTabId: { 'tab-1': ['pty-1'] } + } + const unrelatedUpdate: SelectorState = { + ...state, + runtimePaneTitlesByTabId: { + ...state.runtimePaneTitlesByTabId, + 'other-tab': { 0: 'claude [permission]' } + }, + ptyIdsByTabId: { ...state.ptyIdsByTabId, 'other-tab': ['pty-other'] } + } + + expect(selectRuntimePaneTitlesForWorktree(state, worktreeId)).toBe( + selectRuntimePaneTitlesForWorktree(unrelatedUpdate, worktreeId) + ) + expect(selectLivePtyIdsForWorktree(state, worktreeId)).toBe( + selectLivePtyIdsForWorktree(unrelatedUpdate, worktreeId) + ) + }) + + it('returns the shared frozen empty for a worktree with no tabs', () => { + const state: SelectorState & LayoutRootSelectorState = { + tabsByWorktree: {}, + runtimePaneTitlesByTabId: {}, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: {} + } + + expect(selectRuntimePaneTitlesForWorktree(state, 'missing')).toBe(EMPTY_RUNTIME_PANE_TITLES) + expect(selectLivePtyIdsForWorktree(state, 'missing')).toBe(EMPTY_LIVE_PTY_IDS) + expect(selectTerminalLayoutRootsForWorktree(state, 'missing')).toBe(EMPTY_TERMINAL_LAYOUT_ROOTS) + expect(Object.isFrozen(EMPTY_RUNTIME_PANE_TITLES)).toBe(true) + expect(Object.isFrozen(EMPTY_LIVE_PTY_IDS)).toBe(true) + expect(Object.isFrozen(EMPTY_TERMINAL_LAYOUT_ROOTS)).toBe(true) + }) }) diff --git a/src/renderer/src/components/sidebar/worktree-card-status-inputs.ts b/src/renderer/src/components/sidebar/worktree-card-status-inputs.ts index 128afafe03d..cd9f5c4f7c5 100644 --- a/src/renderer/src/components/sidebar/worktree-card-status-inputs.ts +++ b/src/renderer/src/components/sidebar/worktree-card-status-inputs.ts @@ -1,9 +1,19 @@ import type { AppState } from '@/store/types' import type { TerminalPaneLayoutNode } from '../../../../shared/terminal-tab-types' +import { createWorktreeRecordSelector } from './worktree-record-selector-cache' // Why: these selectors return fresh maps whose top-level values preserve // underlying per-tab references, so callers must compare them shallowly. +// Why frozen: one instance is shared by every card, so a stray write would leak +// across worktrees instead of failing locally. +export const EMPTY_RUNTIME_PANE_TITLES: Record> = Object.freeze({}) +export const EMPTY_LIVE_PTY_IDS: Record = Object.freeze({}) +export const EMPTY_TERMINAL_LAYOUT_ROOTS: Record< + string, + TerminalPaneLayoutNode | null | undefined +> = Object.freeze({}) + type WorktreeCardStatusInputState = Pick & { tabsByWorktree: Record } @@ -12,44 +22,56 @@ type WorktreeCardLayoutRootInputState = Pick tabsByWorktree: Record } -export function selectRuntimePaneTitlesForWorktree( - state: WorktreeCardStatusInputState, - worktreeId: string -): Record> { - const out: Record> = {} - for (const tab of state.tabsByWorktree[worktreeId] ?? []) { - const paneTitles = state.runtimePaneTitlesByTabId[tab.id] - if (paneTitles) { - out[tab.id] = paneTitles +export const selectRuntimePaneTitlesForWorktree = createWorktreeRecordSelector< + WorktreeCardStatusInputState, + Record> +>({ + readSources: (state) => [state.tabsByWorktree, state.runtimePaneTitlesByTabId], + empty: EMPTY_RUNTIME_PANE_TITLES, + build: (state, worktreeId) => { + const out: Record> = {} + for (const tab of state.tabsByWorktree[worktreeId] ?? []) { + const paneTitles = state.runtimePaneTitlesByTabId[tab.id] + if (paneTitles) { + out[tab.id] = paneTitles + } } + return out } - return out -} +}) -export function selectLivePtyIdsForWorktree( - state: WorktreeCardStatusInputState, - worktreeId: string -): Record { - const out: Record = {} - for (const tab of state.tabsByWorktree[worktreeId] ?? []) { - const ids = state.ptyIdsByTabId[tab.id] - if (ids && ids.length > 0) { - out[tab.id] = ids +export const selectLivePtyIdsForWorktree = createWorktreeRecordSelector< + WorktreeCardStatusInputState, + Record +>({ + readSources: (state) => [state.tabsByWorktree, state.ptyIdsByTabId], + empty: EMPTY_LIVE_PTY_IDS, + build: (state, worktreeId) => { + const out: Record = {} + for (const tab of state.tabsByWorktree[worktreeId] ?? []) { + const ids = state.ptyIdsByTabId[tab.id] + if (ids && ids.length > 0) { + out[tab.id] = ids + } } + return out } - return out -} +}) -export function selectTerminalLayoutRootsForWorktree( - state: WorktreeCardLayoutRootInputState, - worktreeId: string -): Record { - const out: Record = {} - for (const tab of state.tabsByWorktree[worktreeId] ?? []) { - out[tab.id] = state.terminalLayoutsByTabId[tab.id]?.root +export const selectTerminalLayoutRootsForWorktree = createWorktreeRecordSelector< + WorktreeCardLayoutRootInputState, + Record +>({ + readSources: (state) => [state.tabsByWorktree, state.terminalLayoutsByTabId], + empty: EMPTY_TERMINAL_LAYOUT_ROOTS, + build: (state, worktreeId) => { + const out: Record = {} + for (const tab of state.tabsByWorktree[worktreeId] ?? []) { + out[tab.id] = state.terminalLayoutsByTabId[tab.id]?.root + } + return out } - return out -} +}) export function selectTerminalLayoutRootsForWorktrees( state: WorktreeCardLayoutRootInputState, diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/pending-worktree-creation-keys.test.ts b/src/renderer/src/components/sidebar/worktree-list/listing/pending-worktree-creation-keys.test.ts new file mode 100644 index 00000000000..d45a7bf392f --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list/listing/pending-worktree-creation-keys.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest' +import type { AppState } from '@/store/types' +import { + EMPTY_PENDING_WORKTREE_CREATION_KEYS, + selectPendingWorktreeCreationKeys +} from './pending-worktree-creation-keys' + +type PendingCreations = AppState['pendingWorktreeCreations'] + +function makePending(creationId: string, repoId: string): PendingCreations[string] { + return { + creationId, + request: { repoId } + } as unknown as PendingCreations[string] +} + +describe('selectPendingWorktreeCreationKeys', () => { + // Why: this runs inside an always-mounted sidebar subscriber, so zustand + // re-evaluates it on every store write in the app. + it('returns the shared frozen empty when nothing is pending', () => { + const empty: PendingCreations = {} + + expect(selectPendingWorktreeCreationKeys(empty)).toBe(EMPTY_PENDING_WORKTREE_CREATION_KEYS) + expect(selectPendingWorktreeCreationKeys({})).toBe(EMPTY_PENDING_WORKTREE_CREATION_KEYS) + expect(selectPendingWorktreeCreationKeys(undefined)).toBe(EMPTY_PENDING_WORKTREE_CREATION_KEYS) + expect(Object.isFrozen(EMPTY_PENDING_WORKTREE_CREATION_KEYS)).toBe(true) + }) + + it('builds the key list once per slice identity', () => { + const pending: PendingCreations = { + 'creation-1': makePending('creation-1', 'repo with space') + } + + const first = selectPendingWorktreeCreationKeys(pending) + expect(first).toEqual(['creation-1 repo with space']) + expect(selectPendingWorktreeCreationKeys(pending)).toBe(first) + }) + + it('rebuilds when the slice is replaced', () => { + const before: PendingCreations = { + 'creation-1': makePending('creation-1', 'repo-1') + } + const after: PendingCreations = { + ...before, + 'creation-2': makePending('creation-2', 'repo-2') + } + + expect(selectPendingWorktreeCreationKeys(after)).toEqual([ + 'creation-1 repo-1', + 'creation-2 repo-2' + ]) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/pending-worktree-creation-keys.ts b/src/renderer/src/components/sidebar/worktree-list/listing/pending-worktree-creation-keys.ts new file mode 100644 index 00000000000..857edd9c3c9 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list/listing/pending-worktree-creation-keys.ts @@ -0,0 +1,39 @@ +import type { AppState } from '@/store/types' + +// Why frozen: the sidebar row model is always mounted and this list is empty +// almost always, so one shared identity serves every read. +export const EMPTY_PENDING_WORKTREE_CREATION_KEYS: string[] = Object.freeze( + [] +) as unknown as string[] + +const keysBySource = new WeakMap() + +/** + * Flat `" "` keys for the pending-creation sidebar rows. + * + * Why identity-cached: this runs inside an always-mounted subscriber, so an + * unmemoized `Object.values(...).map(...)` allocated an array plus one template + * string per pending creation on every store write in the app. Keyed on the + * slice reference, so it only rebuilds when the slice itself is replaced. + * + * Split on the first space — creationId is a UUID (no space) so a + * space-containing repoId stays intact. + */ +export function selectPendingWorktreeCreationKeys( + pendingWorktreeCreations: AppState['pendingWorktreeCreations'] | undefined +): string[] { + if (!pendingWorktreeCreations) { + return EMPTY_PENDING_WORKTREE_CREATION_KEYS + } + const cached = keysBySource.get(pendingWorktreeCreations) + if (cached) { + return cached + } + const creations = Object.values(pendingWorktreeCreations) + const keys = + creations.length === 0 + ? EMPTY_PENDING_WORKTREE_CREATION_KEYS + : creations.map((creation) => `${creation.creationId} ${creation.request.repoId}`) + keysBySource.set(pendingWorktreeCreations, keys) + return keys +} diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/use-section-rows.ts b/src/renderer/src/components/sidebar/worktree-list/listing/use-section-rows.ts index 16899dbc146..ddd91a23653 100644 --- a/src/renderer/src/components/sidebar/worktree-list/listing/use-section-rows.ts +++ b/src/renderer/src/components/sidebar/worktree-list/listing/use-section-rows.ts @@ -19,6 +19,7 @@ import { getEmptyProjectPlaceholderRepoIds } from '../../empty-project-placehold import { addHostSectionRows } from '../../host-section-rows' import { orderHostSectionOptions } from '../../host-section-order' import { buildSidebarHostOptions } from '../../sidebar-host-options' +import { selectPendingWorktreeCreationKeys } from './pending-worktree-creation-keys' type SectionRowsArgs = { groupBy: WorktreeGroupBy @@ -96,19 +97,17 @@ export function useSidebarSectionRows(args: SectionRowsArgs) { ) // Why: subscribe on a flat key array (useShallow) so progress ticks don't rebuild the whole row model. - // Split on first space — creationId is a UUID (no space) so a space-containing repoId stays intact. const pendingCreationKeys = useAppStore( - useShallow((s) => - Object.values(s.pendingWorktreeCreations ?? {}).map( - (creation) => `${creation.creationId} ${creation.request.repoId}` - ) - ) + useShallow((s) => selectPendingWorktreeCreationKeys(s.pendingWorktreeCreations)) ) const pendingCreations = useMemo( () => pendingCreationKeys.map((key) => { const separator = key.indexOf(' ') - return { creationId: key.slice(0, separator), repoId: key.slice(separator + 1) } + return { + creationId: key.slice(0, separator), + repoId: key.slice(separator + 1) + } }), [pendingCreationKeys] ) diff --git a/src/renderer/src/components/sidebar/worktree-record-selector-cache.ts b/src/renderer/src/components/sidebar/worktree-record-selector-cache.ts new file mode 100644 index 00000000000..220d2d074bf --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-record-selector-cache.ts @@ -0,0 +1,63 @@ +import { shallow } from 'zustand/shallow' + +type WorktreeRecordGeneration = { + sources: readonly unknown[] + carried: ReadonlyMap | null + byWorktreeId: Map +} + +function sameSources(previous: readonly unknown[], next: readonly unknown[]): boolean { + if (previous.length !== next.length) { + return false + } + for (let index = 0; index < next.length; index += 1) { + if (previous[index] !== next[index]) { + return false + } + } + return true +} + +/** + * Wraps a per-worktree record selector in a store-identity-keyed cache. + * + * Zustand re-runs every mounted subscriber's selector on every store write, so + * an unmemoized build allocates one record per visible card per write even when + * nothing it reads changed. Gating on the source slice identities collapses that + * to one build per worktree per generation; carrying the previous generation + * forward keeps the reference stable when a rebuild produces equal contents, so + * downstream `useShallow`/`useMemo` gates short-circuit on identity. + * + * The returned records are shared by every caller and must never be mutated. + */ +export function createWorktreeRecordSelector(options: { + readSources: (state: TState) => readonly unknown[] + build: (state: TState, worktreeId: string) => TValue + empty: TValue +}): (state: TState, worktreeId: string) => TValue { + let generation: WorktreeRecordGeneration | null = null + return (state, worktreeId) => { + const sources = options.readSources(state) + if (!generation || !sameSources(generation.sources, sources)) { + generation = { + sources, + carried: generation?.byWorktreeId ?? null, + byWorktreeId: new Map() + } + } + const cached = generation.byWorktreeId.get(worktreeId) + if (cached !== undefined) { + return cached + } + const built = options.build(state, worktreeId) + const carried = generation.carried?.get(worktreeId) + let value = built + if (Object.keys(built).length === 0) { + value = options.empty + } else if (carried && shallow(carried, built)) { + value = carried + } + generation.byWorktreeId.set(worktreeId, value) + return value + } +}