perf(renderer): narrow the App-root badge and terminal pty-set subscriptions

The unread dock badge held the App root subscribed to `tabsByWorktree`, so every
agent title frame re-rendered the whole shell for an integer that had not moved.
The terminal snapshot-capability memo was keyed on the same raw maps plus
`terminalLayoutsByTabId`, so title frames and active-leaf moves rebuilt the whole
pty-id set — work its own value key then discarded.

Both now gate on the exact fields their consumer reads, compared in place.
This commit is contained in:
Neil
2026-09-03 20:43:23 -07:00
parent 7ed86a98ae
commit 3919c37f5f
10 changed files with 465 additions and 44 deletions
@@ -0,0 +1,204 @@
// @vitest-environment happy-dom
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type * as SnapshotCapabilityModule from './terminal-provider-snapshot-capability'
import type { TerminalLayoutSnapshot } from '../../../../shared/terminal-tab-types'
import { makeTab, makeWorktree } from '@/store/slices/store-test-helpers'
const { collectPtyIds } = vi.hoisted(() => ({ collectPtyIds: vi.fn() }))
vi.mock('./terminal-provider-snapshot-capability', async (importOriginal) => {
const actual = await importOriginal<typeof SnapshotCapabilityModule>()
collectPtyIds.mockImplementation(actual.collectTerminalProviderSnapshotPtyIds)
return { ...actual, collectTerminalProviderSnapshotPtyIds: collectPtyIds }
})
import { useAppStore } from '@/store'
import { createTerminalProviderSnapshotBoundPtyIdsSelector } from './terminal-provider-snapshot-bound-pty-ids'
const initialState = useAppStore.getInitialState()
const TAB_COUNT = 40
const WORKTREE_ID = 'repo::worktree-0'
function tabId(index: number): string {
return `tab-${index}`
}
function leafLayout(index: number, activeLeafId: string): TerminalLayoutSnapshot {
return {
root: {
type: 'split' as const,
direction: 'vertical' as const,
first: { type: 'leaf' as const, leafId: `leaf-a-${index}` },
second: { type: 'leaf' as const, leafId: `leaf-b-${index}` }
},
activeLeafId,
expandedLeafId: null,
ptyIdsByLeafId: {
[`leaf-a-${index}`]: `pty-${index}-a`,
[`leaf-b-${index}`]: `pty-${index}-b`
}
}
}
function seedWorkspace(): void {
const worktrees = Array.from({ length: 8 }, (_, index) =>
makeWorktree({ id: `repo::worktree-${index}`, repoId: 'repo' })
)
const tabs = Array.from({ length: TAB_COUNT }, (_, index) =>
makeTab({ id: tabId(index), worktreeId: WORKTREE_ID, ptyId: `pty-${index}` })
)
useAppStore.setState({
worktreesByRepo: { repo: worktrees },
tabsByWorktree: { [WORKTREE_ID]: tabs },
ptyIdsByTabId: Object.fromEntries(tabs.map((tab, index) => [tab.id, [`pty-${index}`]])),
terminalLayoutsByTabId: Object.fromEntries(
tabs.map((tab, index) => [tab.id, leafLayout(index, `leaf-a-${index}`)])
)
})
}
describe('createTerminalProviderSnapshotBoundPtyIdsSelector', () => {
let select: ReturnType<typeof createTerminalProviderSnapshotBoundPtyIdsSelector>
let boundPtyIds: string[]
let unsubscribe: () => void
beforeEach(() => {
useAppStore.setState(initialState, true)
seedWorkspace()
collectPtyIds.mockClear()
select = createTerminalProviderSnapshotBoundPtyIdsSelector()
boundPtyIds = select(useAppStore.getState())
// Why subscribe: this mirrors how zustand drives the selector — once per store write.
unsubscribe = useAppStore.subscribe((state) => {
boundPtyIds = select(state)
})
})
afterEach(() => {
unsubscribe()
useAppStore.setState(initialState, true)
})
// Why: the collector walks every tab and every layout leaf, and both of these rewrite the maps it
// reads without being able to change the pty set.
it('never re-collects for agent title frames or active-leaf moves', () => {
expect(collectPtyIds).toHaveBeenCalledTimes(1)
const initialBoundPtyIds = boundPtyIds
for (let index = 0; index < TAB_COUNT; index += 1) {
useAppStore.getState().updateTabTitle(tabId(index), `agent frame ${index}`)
}
for (let index = 0; index < TAB_COUNT; index += 1) {
useAppStore.getState().setTabLayout(tabId(index), leafLayout(index, `leaf-b-${index}`))
}
expect(useAppStore.getState().tabsByWorktree).not.toBe(initialState.tabsByWorktree)
expect(useAppStore.getState().terminalLayoutsByTabId[tabId(0)]?.activeLeafId).toBe('leaf-b-0')
expect(collectPtyIds).toHaveBeenCalledTimes(1)
expect(boundPtyIds).toBe(initialBoundPtyIds)
})
it('re-collects and republishes when the pty set actually changes', () => {
const expectRecollect = (label: string, mutate: () => void): string[] => {
const before = collectPtyIds.mock.calls.length
const previousBoundPtyIds = boundPtyIds
mutate()
expect(collectPtyIds.mock.calls.length, label).toBeGreaterThan(before)
expect(boundPtyIds, label).not.toBe(previousBoundPtyIds)
return boundPtyIds
}
expectRecollect('pty bound to a leaf', () => {
useAppStore.getState().replaceTerminalLayoutPanePtyId(tabId(0), 'leaf-b-0', 'pty-0-b-next')
})
expect(boundPtyIds).toContain('pty-0-b-next')
expectRecollect('split pty bound to a tab', () => {
useAppStore.setState((state) => ({
ptyIdsByTabId: { ...state.ptyIdsByTabId, [tabId(1)]: ['pty-1', 'pty-1-split'] }
}))
})
expect(boundPtyIds).toContain('pty-1-split')
expectRecollect('split pty unbound from a tab', () => {
useAppStore.setState((state) => ({
ptyIdsByTabId: { ...state.ptyIdsByTabId, [tabId(1)]: ['pty-1'] }
}))
})
expect(boundPtyIds).not.toContain('pty-1-split')
expectRecollect('pending reconnect pty appears', () => {
useAppStore.setState({ pendingReconnectPtyIdByTabId: { [tabId(2)]: 'pty-2-reconnect' } })
})
expect(boundPtyIds).toContain('pty-2-reconnect')
expectRecollect('leaf added', () => {
useAppStore.getState().setTabLayout(tabId(3), {
...leafLayout(3, 'leaf-a-3'),
root: {
type: 'split',
direction: 'vertical',
first: { type: 'leaf', leafId: 'leaf-a-3' },
second: {
type: 'split',
direction: 'horizontal',
first: { type: 'leaf', leafId: 'leaf-b-3' },
second: { type: 'leaf', leafId: 'leaf-c-3' }
}
},
ptyIdsByLeafId: {
'leaf-a-3': 'pty-3-a',
'leaf-b-3': 'pty-3-b',
'leaf-c-3': 'pty-3-c'
}
})
})
expect(boundPtyIds).toContain('pty-3-c')
expectRecollect('leaf removed', () => {
useAppStore.getState().setTabLayout(tabId(3), leafLayout(3, 'leaf-a-3'))
})
expect(boundPtyIds).not.toContain('pty-3-c')
expectRecollect('tab added', () => {
useAppStore.setState((state) => ({
tabsByWorktree: {
...state.tabsByWorktree,
[WORKTREE_ID]: [
...(state.tabsByWorktree[WORKTREE_ID] ?? []),
makeTab({ id: 'tab-new', worktreeId: WORKTREE_ID, ptyId: 'pty-new' })
]
}
}))
})
expect(boundPtyIds).toContain('pty-new')
expectRecollect('tab removed', () => {
useAppStore.setState((state) => ({
tabsByWorktree: {
...state.tabsByWorktree,
[WORKTREE_ID]: (state.tabsByWorktree[WORKTREE_ID] ?? []).filter(
(tab) => tab.id !== 'tab-new'
)
}
}))
})
expect(boundPtyIds).not.toContain('pty-new')
})
// Why: the id array identity is the synchronization loop's restart trigger, so a rebuild that
// lands on the same set must reuse the previous array rather than restart the loop.
it('keeps the array identity when a rebuild lands on the same id set', () => {
const initialBoundPtyIds = boundPtyIds
useAppStore.setState((state) => ({
tabsByWorktree: {
...state.tabsByWorktree,
[WORKTREE_ID]: (state.tabsByWorktree[WORKTREE_ID] ?? []).toReversed()
}
}))
expect(collectPtyIds).toHaveBeenCalledTimes(2)
expect(boundPtyIds).toBe(initialBoundPtyIds)
})
})
@@ -0,0 +1,88 @@
import { reuseArrayIfEqual } from '@/components/sidebar/worktree-agent-row-selectors'
import { sameBucketRecords } from '@/lib/bucket-record-equality'
import { sameStringRecord } from '@/lib/terminal-layout-equality'
import {
type SnapshotCapabilityBindingState,
type SnapshotCapabilityTab,
collectTerminalProviderSnapshotPtyIds
} from './terminal-provider-snapshot-capability'
type TabsByWorktree = SnapshotCapabilityBindingState['tabsByWorktree']
type LayoutsByTabId = NonNullable<SnapshotCapabilityBindingState['terminalLayoutsByTabId']>
const EMPTY_TABS: TabsByWorktree = Object.freeze({})
const EMPTY_LAYOUTS: LayoutsByTabId = Object.freeze({})
function sameBoundTab(previous: SnapshotCapabilityTab, next: SnapshotCapabilityTab): boolean {
return previous.id === next.id && previous.ptyId === next.ptyId
}
/** Leaf pty bindings are the only layout field the collector reads; active-leaf moves, pane titles
* and buffer captures all replace the layout object without touching them. */
function sameLayoutLeafPtyIds(previous: LayoutsByTabId, next: LayoutsByTabId): boolean {
if (previous === next) {
return true
}
const tabIds = Object.keys(next)
if (tabIds.length !== Object.keys(previous).length) {
return false
}
for (const tabId of tabIds) {
const nextLayout = next[tabId]
const previousLayout = previous[tabId]
if (previousLayout === nextLayout) {
continue
}
if (
!previousLayout ||
!sameStringRecord(previousLayout.ptyIdsByLeafId, nextLayout?.ptyIdsByLeafId)
) {
return false
}
}
return true
}
/**
* Why: the collector walks every tab and every layout leaf, and the maps it reads are rewritten by
* agent title frames and active-leaf moves that cannot alter the pty set. Gating it on the fields it
* actually reads — `tab.id`, `tab.ptyId`, `ptyIdsByTabId`, `pendingReconnectPtyIdByTabId`,
* `layout.ptyIdsByLeafId` — keeps those frames free, and reusing the prior array when a real rebuild
* lands on the same set keeps the synchronization effect asleep.
*
* Why chaining against the immediately preceding state is enough: equality over those fields is
* transitive, so a run of unchanged states is equivalent to comparing against the state that
* produced the cached array.
*/
export function createTerminalProviderSnapshotBoundPtyIdsSelector(): (
state: SnapshotCapabilityBindingState
) => string[] {
let previousTabsByWorktree: TabsByWorktree = EMPTY_TABS
let previousLayoutsByTabId: LayoutsByTabId = EMPTY_LAYOUTS
let previousPtyIdsByTabId: SnapshotCapabilityBindingState['ptyIdsByTabId'] | undefined
let previousPendingReconnectPtyIdByTabId: SnapshotCapabilityBindingState['pendingReconnectPtyIdByTabId']
let boundPtyIds: string[] = []
let collected = false
return (state) => {
const layoutsByTabId = state.terminalLayoutsByTabId ?? EMPTY_LAYOUTS
const unchanged =
collected &&
previousPtyIdsByTabId === state.ptyIdsByTabId &&
previousPendingReconnectPtyIdByTabId === state.pendingReconnectPtyIdByTabId &&
sameBucketRecords(previousTabsByWorktree, state.tabsByWorktree, sameBoundTab) &&
sameLayoutLeafPtyIds(previousLayoutsByTabId, layoutsByTabId)
if (!unchanged) {
boundPtyIds = reuseArrayIfEqual(
boundPtyIds,
collectTerminalProviderSnapshotPtyIds(state).sort()
)
previousPtyIdsByTabId = state.ptyIdsByTabId
previousPendingReconnectPtyIdByTabId = state.pendingReconnectPtyIdByTabId
collected = true
}
previousTabsByWorktree = state.tabsByWorktree
previousLayoutsByTabId = layoutsByTabId
return boundPtyIds
}
}
@@ -1,7 +1,7 @@
type SnapshotCapability = { id: string; authoritative: boolean | null }
type SnapshotCapabilityResolver = (ids: string[]) => Promise<SnapshotCapability[]>
type SnapshotCapabilityTab = { id: string; ptyId?: string | null }
type SnapshotCapabilityBindingState = {
export type SnapshotCapabilityTab = { id: string; ptyId?: string | null }
export type SnapshotCapabilityBindingState = {
tabsByWorktree: Readonly<Record<string, readonly SnapshotCapabilityTab[]>>
ptyIdsByTabId: Readonly<Record<string, readonly string[]>>
pendingReconnectPtyIdByTabId?: Readonly<Record<string, string>>
@@ -1,38 +1,22 @@
import { useEffect, useMemo, useSyncExternalStore } from 'react'
import { useAppStore } from '@/store'
import { createTerminalProviderSnapshotBoundPtyIdsSelector } from './terminal-provider-snapshot-bound-pty-ids'
import {
collectTerminalProviderSnapshotPtyIds,
getTerminalProviderSnapshotCapabilityRevision,
subscribeTerminalProviderSnapshotCapability,
startTerminalProviderSnapshotCapabilitySynchronization
} from './terminal-provider-snapshot-capability'
export function useTerminalProviderSnapshotCapability(enabled: boolean): number {
const tabsByWorktree = useAppStore((state) => state.tabsByWorktree)
const ptyIdsByTabId = useAppStore((state) => state.ptyIdsByTabId)
const pendingReconnectPtyIdByTabId = useAppStore((state) => state.pendingReconnectPtyIdByTabId)
const terminalLayoutsByTabId = useAppStore((state) => state.terminalLayoutsByTabId)
// Why the full field set: synchronization PRUNES cached verdicts outside the
// collected ids, so a collector narrower than startup's (App.tsx refresh
// passes full state) would evict valid answers for split-leaf and
// pending-reconnect ptys back into the exempt-by-default unknown state.
// Why keyed: layouts change without changing the pty set (active-leaf churn);
// the synchronization loop must restart only on genuine id-set changes.
// Why memoized on the map identities: this runs at Terminal's render cadence,
// and only a change to one of the four collected maps can alter the id set.
const boundPtyIdsKey = useMemo(
() =>
JSON.stringify(
collectTerminalProviderSnapshotPtyIds({
tabsByWorktree,
ptyIdsByTabId,
pendingReconnectPtyIdByTabId,
terminalLayoutsByTabId
}).sort()
),
[tabsByWorktree, ptyIdsByTabId, pendingReconnectPtyIdByTabId, terminalLayoutsByTabId]
)
const boundPtyIds = useMemo(() => JSON.parse(boundPtyIdsKey) as string[], [boundPtyIdsKey])
// Why a selector: it returns an identity-stable id set, so the
// synchronization loop restarts only on genuine id-set changes and title
// frames and active-leaf moves never reach the collector at all.
const selectBoundPtyIds = useMemo(() => createTerminalProviderSnapshotBoundPtyIdsSelector(), [])
const boundPtyIds = useAppStore(selectBoundPtyIds)
const capabilityRevision = useSyncExternalStore(
subscribeTerminalProviderSnapshotCapability,
getTerminalProviderSnapshotCapabilityRevision,
@@ -123,4 +123,58 @@ describe('useUnreadDockBadge', () => {
expect(getUnreadBadgeCount).toHaveBeenCalledTimes(5)
expect(setUnreadDockBadgeCount).toHaveBeenLastCalledWith(0)
})
// Why render-counted: this hook is mounted on the App root, so anything that wakes its
// subscription re-renders the whole shell — the chrome layout, both providers and every
// non-memoised overlay — for a badge integer that did not move.
it('leaves the App root asleep through title frames and wakes it only on a badge change', () => {
const worktrees = Array.from({ length: 20 }, (_, index) =>
makeWorktree({ id: `repo::worktree-${index}`, repoId: 'repo' })
)
const tabsByWorktree = Object.fromEntries(
worktrees.map((worktree, index) => [
worktree.id,
[makeTab({ id: `tab-${index}`, worktreeId: worktree.id })]
])
)
useAppStore.setState({
worktreesByRepo: { repo: worktrees },
tabsByWorktree,
unreadTerminalTabs: { 'tab-19': true }
})
let renders = 0
renderHook(() => {
renders += 1
return useUnreadDockBadge()
})
const rendersAfterMount = renders
// Separate acts: title frames arrive as individual store writes, not one batch.
for (let index = 0; index < 20; index += 1) {
act(() => useAppStore.getState().updateTabTitle(`tab-${index}`, `agent frame ${index}`))
}
expect(useAppStore.getState().tabsByWorktree).not.toBe(tabsByWorktree)
expect(renders).toBe(rendersAfterMount)
// A tab becomes unread.
act(() => useAppStore.setState({ unreadTerminalTabs: { 'tab-19': true, 'tab-0': true } }))
expect(renders).toBe(rendersAfterMount + 1)
expect(setUnreadDockBadgeCount).toHaveBeenLastCalledWith(2)
// A tab is read.
act(() => useAppStore.setState({ unreadTerminalTabs: { 'tab-19': true } }))
expect(renders).toBe(rendersAfterMount + 2)
expect(setUnreadDockBadgeCount).toHaveBeenLastCalledWith(1)
// A tab holding unread state closes.
act(() =>
useAppStore.setState({
tabsByWorktree: { ...useAppStore.getState().tabsByWorktree, 'repo::worktree-19': [] },
unreadTerminalTabs: {}
})
)
expect(renders).toBe(rendersAfterMount + 3)
expect(setUnreadDockBadgeCount).toHaveBeenLastCalledWith(0)
})
})
+6 -14
View File
@@ -1,6 +1,5 @@
import { useEffect, useMemo } from 'react'
import { useShallow } from 'zustand/react/shallow'
import { getUnreadBadgeCount } from '@/lib/unread-badge-count'
import { createUnreadBadgeCountSelector } from '@/lib/unread-badge-count-selector'
import { useAppStore } from '@/store'
function setUnreadDockBadgeCountBestEffort(count: number): void {
@@ -18,18 +17,11 @@ export function clearUnreadDockBadgeCount(): void {
}
export function useUnreadDockBadge(): typeof clearUnreadDockBadgeCount {
const { worktreesByRepo, tabsByWorktree, unreadTerminalTabs } = useAppStore(
useShallow((state) => ({
worktreesByRepo: state.worktreesByRepo,
tabsByWorktree: state.tabsByWorktree,
unreadTerminalTabs: state.unreadTerminalTabs
}))
)
// Why: this hook is always mounted; unrelated remote writes must not rescan every workspace.
const unreadCount = useMemo(
() => getUnreadBadgeCount({ worktreesByRepo, tabsByWorktree, unreadTerminalTabs }),
[tabsByWorktree, unreadTerminalTabs, worktreesByRepo]
)
// Why a selector and not the raw maps: this hook is mounted on the App root, so subscribing to
// `tabsByWorktree` re-rendered the entire shell on every title frame. The selector both skips the
// rescan and keeps the subscription quiet unless the badge integer itself changes.
const selectUnreadBadgeCount = useMemo(() => createUnreadBadgeCountSelector(), [])
const unreadCount = useAppStore(selectUnreadBadgeCount)
// oxlint-disable-next-line react-doctor/no-derived-state-effect -- Why: this syncs an external OS dock badge, not React render state.
useEffect(() => {
@@ -0,0 +1,42 @@
/**
* Identity-first equality over a `Record<string, readonly T[]>` under a projection of each item.
*
* Why not `createWorktreeTabBucketProjection`: that builds a projected record so callers can hold
* it; a subscriber that only needs "did my fields change?" pays for an allocation per store write.
* This answers the same question by comparing in place.
*/
export function sameBucketRecords<T>(
previous: Readonly<Record<string, readonly T[]>>,
next: Readonly<Record<string, readonly T[]>>,
isSameItem: (previous: T, next: T) => boolean
): boolean {
if (previous === next) {
return true
}
const keys = Object.keys(next)
if (keys.length !== Object.keys(previous).length) {
return false
}
for (const key of keys) {
const nextItems = next[key]
const previousItems = previous[key]
if (previousItems === nextItems) {
continue
}
if (!previousItems || !nextItems || previousItems.length !== nextItems.length) {
return false
}
for (let index = 0; index < nextItems.length; index += 1) {
const previousItem = previousItems[index]
const nextItem = nextItems[index]
if (
previousItem === undefined ||
nextItem === undefined ||
!isSameItem(previousItem, nextItem)
) {
return false
}
}
}
return true
}
@@ -3,7 +3,8 @@ import type {
TerminalPaneLayoutNode
} from '../../../shared/terminal-tab-types'
function sameStringRecord(
/** Exported so pty-topology gates can reuse the leaf-map comparison this equality already defines. */
export function sameStringRecord(
a: Readonly<Record<string, string>> | undefined,
b: Readonly<Record<string, string>> | undefined
): boolean {
@@ -0,0 +1,50 @@
import { sameBucketRecords } from './bucket-record-equality'
import {
type UnreadBadgeCountSources,
type UnreadBadgeTab,
type UnreadBadgeWorktree,
getUnreadBadgeCount
} from './unread-badge-count'
const EMPTY_BUCKETS = Object.freeze({})
function sameBadgeWorktree(previous: UnreadBadgeWorktree, next: UnreadBadgeWorktree): boolean {
return previous.id === next.id && previous.isUnread === next.isUnread
}
function sameBadgeTab(previous: UnreadBadgeTab, next: UnreadBadgeTab): boolean {
return previous.id === next.id
}
/**
* Why: the App root holds this subscription for a single integer. Returning the raw maps re-rendered
* the whole shell on every agent title frame; selecting the count instead means the subscription
* only notifies when the badge value can actually have moved.
*
* Why chaining against the immediately preceding state is enough: equality over the count's read set
* — worktree `id`/`isUnread`, tab `id`, and the unread map identity — is transitive, so a run of
* unchanged states is equivalent to comparing against the state that produced the cached count.
*/
export function createUnreadBadgeCountSelector(): (state: UnreadBadgeCountSources) => number {
let previousWorktreesByRepo: UnreadBadgeCountSources['worktreesByRepo'] = EMPTY_BUCKETS
let previousTabsByWorktree: UnreadBadgeCountSources['tabsByWorktree'] = EMPTY_BUCKETS
let previousUnreadTerminalTabs: UnreadBadgeCountSources['unreadTerminalTabs'] | undefined
let unreadCount = 0
let counted = false
return (state) => {
const unchanged =
counted &&
previousUnreadTerminalTabs === state.unreadTerminalTabs &&
sameBucketRecords(previousWorktreesByRepo, state.worktreesByRepo, sameBadgeWorktree) &&
sameBucketRecords(previousTabsByWorktree, state.tabsByWorktree, sameBadgeTab)
if (!unchanged) {
unreadCount = getUnreadBadgeCount(state)
previousUnreadTerminalTabs = state.unreadTerminalTabs
counted = true
}
previousWorktreesByRepo = state.worktreesByRepo
previousTabsByWorktree = state.tabsByWorktree
return unreadCount
}
}
+11 -5
View File
@@ -1,15 +1,21 @@
import type { TerminalTab } from '../../../shared/terminal-tab-types'
import type { Worktree } from '../../../shared/worktree/types'
/** The only fields the count reads, so a projection over them is a sound cache key. */
export type UnreadBadgeWorktree = Pick<Worktree, 'id' | 'isUnread'>
export type UnreadBadgeTab = Pick<TerminalTab, 'id'>
export type UnreadBadgeCountSources = {
worktreesByRepo: Readonly<Record<string, readonly UnreadBadgeWorktree[]>>
tabsByWorktree: Readonly<Record<string, readonly UnreadBadgeTab[]>>
unreadTerminalTabs: Readonly<Record<string, true>>
}
export function getUnreadBadgeCount({
worktreesByRepo,
tabsByWorktree,
unreadTerminalTabs
}: {
worktreesByRepo: Record<string, Worktree[]>
tabsByWorktree: Record<string, TerminalTab[]>
unreadTerminalTabs: Record<string, true>
}): number {
}: UnreadBadgeCountSources): number {
const unreadWorktreeIds = new Set<string>()
for (const worktrees of Object.values(worktreesByRepo)) {