From ee91477d9f65fe06f53cfb8e9276102ed8ec3e11 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 18 Jul 2026 14:05:29 -0700 Subject: [PATCH] perf(renderer): patch sidebar live-agent index on within-state status pings instead of full rebuild Co-authored-by: Orca --- .../worktree-agent-row-selectors.test.ts | 104 ++++++++++++++++ .../sidebar/worktree-agent-row-selectors.ts | 115 ++++++++++++++++-- 2 files changed, 212 insertions(+), 7 deletions(-) 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 a40b6840038..2bed7bdbe00 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 @@ -7,6 +7,7 @@ import type { TerminalTab } from '../../../../shared/types' import type { RetainedAgentEntry } from '@/store/slices/agent-status' import { makePaneKey } from '../../../../shared/stable-pane-id' import { + getLiveEntriesFullRebuildCountForTests, selectLiveAgentStatusEntriesForWorktree, selectMigrationUnsupportedEntriesForWorktree, selectRuntimeAgentOrchestrationForWorktree, @@ -151,6 +152,109 @@ describe('selectLiveAgentStatusEntriesForWorktree', () => { expect(selectLiveAgentStatusEntriesForWorktree(state, 'wt-1')).toEqual([childEntry]) }) + 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 baseState = { + tabsByWorktree: { + 'wt-1': [makeTab('tab-1')], + 'wt-2': [makeTab('tab-2')] + }, + agentStatusByPaneKey: { + [PANE_KEY_1]: wt1Entry, + [PANE_KEY_2]: wt2Entry + }, + migrationUnsupportedByPtyId: {}, + retainedAgentsByPaneKey: {} + } + + // Prime the cache (one full rebuild allowed here). + const primedWt1 = selectLiveAgentStatusEntriesForWorktree(baseState, 'wt-1') + const primedWt2 = selectLiveAgentStatusEntriesForWorktree(baseState, 'wt-2') + const rebuildsAfterPrime = getLiveEntriesFullRebuildCountForTests() + + // Simulate a burst of same-state pings: setAgentStatus mints a new map and + // a new entry object per ping, with only prompt/tool metadata changing. + let state = baseState + let latestWt2 = primedWt2 + for (let ping = 0; ping < 50; ping += 1) { + state = { + ...state, + agentStatusByPaneKey: { + ...state.agentStatusByPaneKey, + [PANE_KEY_2]: { + ...wt2Entry, + prompt: `wt2 prompt ${ping}`, + toolName: 'Bash', + toolInput: `cmd ${ping}`, + updatedAt: 1000 + ping + } + } + } + const wt1 = selectLiveAgentStatusEntriesForWorktree(state, 'wt-1') + latestWt2 = selectLiveAgentStatusEntriesForWorktree(state, 'wt-2') + // Unaffected worktree keeps identity; owning worktree serves the fresh entry. + expect(wt1).toBe(primedWt1) + expect(latestWt2).toHaveLength(1) + expect(latestWt2[0]?.prompt).toBe(`wt2 prompt ${ping}`) + expect(latestWt2[0]?.toolInput).toBe(`cmd ${ping}`) + } + // The O(all live agents) rebuild body never ran for within-state pings. + expect(getLiveEntriesFullRebuildCountForTests()).toBe(rebuildsAfterPrime) + expect(latestWt2).not.toBe(primedWt2) + + // A real transition that changes bucketing (working -> done with the tab + // still present keeps the bucket; removing the entry must drop it). + const doneState = { + ...state, + agentStatusByPaneKey: { + [PANE_KEY_1]: state.agentStatusByPaneKey[PANE_KEY_1] + } + } + expect(selectLiveAgentStatusEntriesForWorktree(doneState, 'wt-2')).toEqual([]) + expect(selectLiveAgentStatusEntriesForWorktree(doneState, 'wt-1')).toEqual([wt1Entry]) + expect(getLiveEntriesFullRebuildCountForTests()).toBe(rebuildsAfterPrime + 1) + }) + + 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 state = { + // No tab membership: bucketing comes from entry.worktreeId attribution. + tabsByWorktree: { 'wt-1': [], 'wt-2': [] }, + agentStatusByPaneKey: { [PANE_KEY_1]: entry }, + migrationUnsupportedByPtyId: {}, + retainedAgentsByPaneKey: {} + } + expect(selectLiveAgentStatusEntriesForWorktree(state, 'wt-1')).toEqual([entry]) + + const moved = { ...entry, worktreeId: 'wt-2' } + const nextState = { + ...state, + agentStatusByPaneKey: { [PANE_KEY_1]: moved } + } + expect(selectLiveAgentStatusEntriesForWorktree(nextState, 'wt-1')).toEqual([]) + expect(selectLiveAgentStatusEntriesForWorktree(nextState, 'wt-2')).toEqual([moved]) + }) + + 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 state = { + tabsByWorktree: { 'wt-1': [] }, + agentStatusByPaneKey: { [PANE_KEY_1]: entry }, + migrationUnsupportedByPtyId: {}, + retainedAgentsByPaneKey: {} + } + expect(selectLiveAgentStatusEntriesForWorktree(state, 'wt-1')).toEqual([entry]) + + // done + tab absent must drop the row (bucket rule), not be patched in place. + const done = { ...entry, state: 'done' as const } + const nextState = { + ...state, + agentStatusByPaneKey: { [PANE_KEY_1]: done } + } + expect(selectLiveAgentStatusEntriesForWorktree(nextState, 'wt-1')).toEqual([]) + }) + it('does not use worktree attribution for a completed row whose tab is gone', () => { const closedEntry = makeEntry(PANE_KEY_1, 1000, { state: 'done', 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 567454f45cc..b70c5e72239 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-row-selectors.ts @@ -51,6 +51,13 @@ let liveEntriesByWorktreeCache: LiveEntriesByWorktreeCache | null = null let migrationUnsupportedByWorktreeCache: MigrationUnsupportedByWorktreeCache | null = null let retainedEntriesByWorktreeCache: RetainedEntriesByWorktreeCache | null = null +// Why: test-only observability — proves same-key entry updates (per-ping +// setAgentStatus map churn) take the O(changed) patch path, not a full rebuild. +let liveEntriesFullRebuildCount = 0 +export function getLiveEntriesFullRebuildCountForTests(): number { + return liveEntriesFullRebuildCount +} + function reuseArrayIfEqual(previous: T[] | undefined, next: T[]): T[] { if (!previous || previous.length !== next.length) { return next @@ -79,6 +86,90 @@ function getTabIdToWorktreeId( return tabIdToWorktreeId } +// Why: keep early attributed child rows, but hide completed rows once their tab is gone. +function liveEntryWorktreeId( + paneKey: string, + entry: AgentStatusEntry, + tabIdToWorktreeId: Map +): string | undefined { + const parsed = parsePaneKey(paneKey) + if (!parsed) { + return undefined + } + const tabWorktreeId = tabIdToWorktreeId.get(parsed.tabId) + return tabWorktreeId ?? (entry.state === 'done' ? undefined : entry.worktreeId) +} + +/** + * Patches the cached by-worktree index in place of a full rebuild when the + * live map changed only by replacing entries under existing pane keys with + * the same bucketing (worktree attribution and done-ness). + * + * Why: setAgentStatus mints a new agentStatusByPaneKey on EVERY status ping, + * including same-state working prompt/tool updates. Rebuilding the whole + * index (parsePaneKey + bucketing across all live agents) per ping is the + * dominant selector cost under parallel agents; a within-state ping only + * needs the owning worktree's bucket refreshed. + */ +function patchLiveEntriesByWorktree( + cache: LiveEntriesByWorktreeCache, + agentStatusByPaneKey: WorktreeAgentRowsState['agentStatusByPaneKey'], + tabIdToWorktreeId: Map +): Map | null { + const previousMap = cache.agentStatusByPaneKey + const changed: Array<{ paneKey: string; entry: AgentStatusEntry }> = [] + let keyCount = 0 + for (const paneKey in agentStatusByPaneKey) { + keyCount += 1 + const entry = agentStatusByPaneKey[paneKey] + const previous = previousMap[paneKey] + if (previous === entry) { + continue + } + // Why: bail on added keys or bucket-determinant changes — the bucket rule + // depends only on paneKey, the (reference-equal) tab index, worktree + // attribution, and done-ness, so equal determinants mean the same bucket. + if ( + previous === undefined || + previous.worktreeId !== entry.worktreeId || + (previous.state === 'done') !== (entry.state === 'done') + ) { + return null + } + changed.push({ paneKey, entry }) + } + if (keyCount !== Object.keys(previousMap).length) { + // Why: removed keys need buckets dropped; leave that to the full rebuild. + return null + } + if (changed.length === 0) { + return cache.entriesByWorktree + } + + const entriesByWorktree = new Map(cache.entriesByWorktree) + const clonedBuckets = new Set() + for (const { paneKey, entry } of changed) { + const worktreeId = liveEntryWorktreeId(paneKey, entry, tabIdToWorktreeId) + if (!worktreeId) { + continue + } + const bucket = entriesByWorktree.get(worktreeId) + const index = bucket?.indexOf(previousMap[paneKey]) ?? -1 + if (!bucket || index < 0) { + return null + } + const nextBucket = clonedBuckets.has(worktreeId) ? bucket : bucket.slice() + // Why: in-position replacement preserves iteration order, matching what a + // full rebuild would produce (spread updates keep object insertion order). + nextBucket[index] = entry + if (!clonedBuckets.has(worktreeId)) { + clonedBuckets.add(worktreeId) + entriesByWorktree.set(worktreeId, nextBucket) + } + } + return entriesByWorktree +} + function getLiveEntriesByWorktree(state: WorktreeAgentRowsState): Map { const agentStatusByPaneKey = state.agentStatusByPaneKey ?? EMPTY_RECORD const tabsByWorktree = state.tabsByWorktree ?? EMPTY_RECORD @@ -90,16 +181,26 @@ function getLiveEntriesByWorktree(state: WorktreeAgentRowsState): Map() for (const [paneKey, entry] of Object.entries(agentStatusByPaneKey)) { - const parsed = parsePaneKey(paneKey) - if (!parsed) { - continue - } - const tabWorktreeId = tabIdToWorktreeId.get(parsed.tabId) - // Why: keep early attributed child rows, but hide completed rows once their tab is gone. - const worktreeId = tabWorktreeId ?? (entry.state === 'done' ? undefined : entry.worktreeId) + const worktreeId = liveEntryWorktreeId(paneKey, entry, tabIdToWorktreeId) if (!worktreeId) { continue }