From 598638c2e3c22bf92cbb92ef2d8995846a9e50ed Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 3 Sep 2026 15:37:14 -0700 Subject: [PATCH] refactor(renderer): move the orchestration projection key onto the shared index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The batch builder and `worktree-agent-orchestration-index.ts` were near-duplicate implementations of the same attribution walk, and both had the self-invalidating `liveSource === agentStatusByPaneKey` gate. Fixing only the batch left the index — which every mounted WorktreeCard hits on every `agentStatus:set` — still rebuilding per publication. Put `paneWorktreeIds` on the index instead and reduce the batch to a `.get`-compatible view of it. That deletes the whole `requestedWorktreeIds` apparatus the batch fix needed (the `worktreeIds` memo threading, the optional `selectDashboardOrchestration` param, the `uniqueWorktreeIdsByInput` WeakMap and its no-mutation contract, `getRequestedTabMembership`), leaves one builder guarded by the index's randomized oracle test, and extends the fix to the sidebar. The projection is memoised on the live/retained map identities so it is computed once per publication rather than once per card, and a successful ordered compare adopts the new array so the remaining cards compare by identity. --- .../build-dashboard-bucket-counts.ts | 24 +- .../build-dashboard-snapshot.test.ts | 18 +- .../dashboard-orchestration-selection.ts | 6 +- ...worktree-agent-orchestration-batch.test.ts | 82 ++--- .../worktree-agent-orchestration-batch.ts | 310 ++---------------- ...worktree-agent-orchestration-index.test.ts | 88 +++++ .../worktree-agent-orchestration-index.ts | 136 ++++++-- 7 files changed, 270 insertions(+), 394 deletions(-) diff --git a/src/renderer/src/components/dashboard/build-dashboard-bucket-counts.ts b/src/renderer/src/components/dashboard/build-dashboard-bucket-counts.ts index 827ccec3856..e673ad73298 100644 --- a/src/renderer/src/components/dashboard/build-dashboard-bucket-counts.ts +++ b/src/renderer/src/components/dashboard/build-dashboard-bucket-counts.ts @@ -30,8 +30,6 @@ type ActiveWorkspacesMemo = { folderWorkspaces: unknown projectGroups: unknown workspaces: ActiveDashboardWorkspace[] - /** Held alongside the descriptors so the orchestration batch keeps one array identity. */ - worktreeIds: string[] } type WorktreeTallyMemo = { @@ -59,20 +57,17 @@ export function createDashboardBucketCountsCache(): DashboardBucketCountsCache { } /** - * The workspace descriptor list and its worktree ids, reused by identity while the - * inputs hold. + * The workspace descriptor list, reused by identity while its inputs hold. * * `collectActiveDashboardWorkspaces(state, false)` allocates one descriptor per * workspace (hundreds, in a large install) and, with metadata off, reads only the * four slices keyed here — see the read-set note on `DashboardWorkspaceState`. - * Every other slice it can touch sits behind an `includeMapMetadata` gate. The id - * array rides the same memo because the orchestration batch re-derives it on every - * agent-status write and only needs it to be stable, not fresh. + * Every other slice it can touch sits behind an `includeMapMetadata` gate. */ function selectActiveDashboardWorkspaces( state: DashboardWorkspaceState, cache: DashboardBucketCountsCache | undefined -): { workspaces: ActiveDashboardWorkspace[]; worktreeIds: string[] } { +): ActiveDashboardWorkspace[] { const memo = cache?.activeWorkspaces if ( memo && @@ -81,21 +76,19 @@ function selectActiveDashboardWorkspaces( memo.folderWorkspaces === state.folderWorkspaces && memo.projectGroups === state.projectGroups ) { - return memo + return memo.workspaces } const workspaces = collectActiveDashboardWorkspaces(state, false) - const worktreeIds = workspaces.map(({ worktree }) => worktree.id) if (cache) { cache.activeWorkspaces = { repos: state.repos, worktreesByRepo: state.worktreesByRepo, folderWorkspaces: state.folderWorkspaces, projectGroups: state.projectGroups, - workspaces, - worktreeIds + workspaces } } - return { workspaces, worktreeIds } + return workspaces } function countsEqual( @@ -171,11 +164,10 @@ export function buildDashboardBucketCounts( done: 0, idle: 0 } satisfies Record - const { workspaces: activeWorktrees, worktreeIds } = selectActiveDashboardWorkspaces(state, cache) + const activeWorktrees = selectActiveDashboardWorkspaces(state, cache) const { singletonOrchestration, orchestrationByWorktree } = selectDashboardOrchestration( state, - activeWorktrees, - worktreeIds + activeWorktrees ) if (cache) { startWorktreeAgentRowsCachePass(cache) diff --git a/src/renderer/src/components/dashboard/build-dashboard-snapshot.test.ts b/src/renderer/src/components/dashboard/build-dashboard-snapshot.test.ts index bae9892736d..698f759d7ab 100644 --- a/src/renderer/src/components/dashboard/build-dashboard-snapshot.test.ts +++ b/src/renderer/src/components/dashboard/build-dashboard-snapshot.test.ts @@ -10,6 +10,7 @@ import { makePaneKey } from '../../../../shared/stable-pane-id' import type { TerminalTab } from '../../../../shared/terminal-tab-types' import type { Worktree } from '../../../../shared/worktree/types' import { selectRuntimeAgentOrchestrationBatch } from '../sidebar/worktree-agent-orchestration-batch' +import { selectRuntimeAgentOrchestrationForWorktree } from '../sidebar/worktree-agent-row-selectors' import type * as DashboardSnapshotWorkspacesModule from './dashboard-snapshot-workspaces' import type * as AgentRowLineageModule from './agent-row-lineage' @@ -753,7 +754,10 @@ describe('buildDashboardSnapshot', () => { expect(snapshot.cards[0].task).toBe('Batched orchestration task') }) - it('releases stale batch references when production moves from multi to singleton to zero', () => { + // Why identity, not release: the batch is a view of the shared orchestration index, which + // mounted sidebar cards read through. A dashboard that drops below two worktrees must not + // invalidate it, and nothing the index reads changed across these transitions. + it('keeps batch records live and correct when production moves from multi to singleton to zero', () => { const secondLeafId = '77777777-7777-4777-8777-777777777777' const firstPaneKey = makePaneKey('tab-w1', LEAF_ID) const secondPaneKey = makePaneKey('tab-w2', secondLeafId) @@ -788,13 +792,17 @@ describe('buildDashboardSnapshot', () => { NOW ) const afterSingleton = selectRuntimeAgentOrchestrationBatch(multiState, requested) - expect(afterSingleton).not.toBe(firstBatch) - expect(afterSingleton.get('w1')).not.toBe(firstW1) + expect(afterSingleton).toBe(firstBatch) + expect(afterSingleton.get('w1')).toBe(firstW1) buildDashboardSnapshot(baseState({ repos: [], worktreesByRepo: {} }), NOW) const afterZero = selectRuntimeAgentOrchestrationBatch(multiState, requested) - expect(afterZero).not.toBe(afterSingleton) - expect(afterZero.get('w1')).not.toBe(afterSingleton.get('w1')) + expect(afterZero).toBe(firstBatch) + for (const worktreeId of requested) { + expect(afterZero.get(worktreeId)).toBe( + selectRuntimeAgentOrchestrationForWorktree(multiState, worktreeId) + ) + } }) it('scans orchestration runtime once for a dashboard snapshot', () => { diff --git a/src/renderer/src/components/dashboard/dashboard-orchestration-selection.ts b/src/renderer/src/components/dashboard/dashboard-orchestration-selection.ts index 40725e17191..d3e823fa6f0 100644 --- a/src/renderer/src/components/dashboard/dashboard-orchestration-selection.ts +++ b/src/renderer/src/components/dashboard/dashboard-orchestration-selection.ts @@ -9,9 +9,7 @@ import { selectRuntimeAgentOrchestrationForWorktree } from '../sidebar/worktree- /** Select the singleton or batched orchestration view for active workspaces. */ export function selectDashboardOrchestration( state: DashboardSnapshotState, - activeWorkspaces: readonly Pick[], - /** Pre-derived ids from the caller's memo; the batch caches on this array's identity. */ - activeWorktreeIds?: readonly string[] + activeWorkspaces: readonly Pick[] ): { singletonOrchestration: ReturnType | null orchestrationByWorktree: ReturnType | null @@ -23,7 +21,7 @@ export function selectDashboardOrchestration( if (activeWorkspaces.length >= 2) { orchestrationByWorktree = selectRuntimeAgentOrchestrationBatch( state, - activeWorktreeIds ?? activeWorkspaces.map(({ worktree }) => worktree.id) + activeWorkspaces.map(({ worktree }) => worktree.id) ) } else { releaseRuntimeAgentOrchestrationBatchCache() diff --git a/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.test.ts b/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.test.ts index d3a3bc9be2e..705a1e0c43e 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.test.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.test.ts @@ -7,11 +7,13 @@ import type { import { makePaneKey } from '../../../../shared/stable-pane-id' import type { TerminalTab } from '../../../../shared/terminal-tab-types' import { - _getRuntimeAgentOrchestrationBatchCountersForTest, EMPTY_WORKTREE_AGENT_ORCHESTRATION, - releaseRuntimeAgentOrchestrationBatchCache, selectRuntimeAgentOrchestrationBatch } from './worktree-agent-orchestration-batch' +import { + _getWorktreeAgentOrchestrationIndexBuildCountForTest, + releaseWorktreeAgentOrchestrationIndexCache +} from './worktree-agent-orchestration-index' import { selectRuntimeAgentOrchestrationForWorktree } from './worktree-agent-row-selectors' type BatchState = Parameters[0] @@ -287,7 +289,9 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { expect(getBatchRecord(replacedBatch, 'wt-2')).toBe(firstWt2) }) - it('releases raw and derived caches for empty requests and empty runtime', () => { + // Why this matters now that the batch is a view of the shared index: an empty dashboard must + // not drop a cache that every mounted sidebar card is still reading through. + it('leaves the shared index intact for an empty request and rebuilds after an empty runtime', () => { let tabIdReads = 0 const state = { tabsByWorktree: { @@ -307,14 +311,15 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { const first = getBatchRecord(selectRuntimeAgentOrchestrationBatch(state, ['target']), 'target') expect(tabIdReads).toBe(1) - selectRuntimeAgentOrchestrationBatch(state, []) + expect(selectRuntimeAgentOrchestrationBatch(state, []).size).toBe(0) const afterEmptyRequest = getBatchRecord( selectRuntimeAgentOrchestrationBatch(state, ['target']), 'target' ) - expect(tabIdReads).toBe(2) - expect(afterEmptyRequest).not.toBe(first) + expect(tabIdReads).toBe(1) + expect(afterEmptyRequest).toBe(first) + // An emptied orchestration map is a real change of the index's own domain, so it does drop. selectRuntimeAgentOrchestrationBatch({ ...state, runtimeAgentOrchestrationByPaneKey: {} }, [ 'target' ]) @@ -322,11 +327,11 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { selectRuntimeAgentOrchestrationBatch(state, ['target']), 'target' ) - expect(tabIdReads).toBe(3) - expect(afterEmptyRuntime).not.toBe(afterEmptyRequest) + expect(tabIdReads).toBe(2) + expect(afterEmptyRuntime).not.toBe(first) }) - it('keeps singleton tab work target-local', () => { + it('matches the per-worktree selector for a single requested worktree', () => { const tabCount = 10 const contextCount = 8 const makeCountedState = () => { @@ -405,22 +410,17 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { ) expect(Object.keys(actual)).toEqual(Object.keys(expected)) - // Why the batch stays tighter: it knows which worktrees are on screen. The - // shared index covers all of them, so it saves per *card*, not per worktree. - expect(batched.counts()).toEqual({ - runtimeEnumerations: 1, - runtimeValueReads: contextCount, - contextVisits: contextCount, - targetTabIdReads: 1, - unrelatedTabIdReads: 0 - }) - expect(reference.counts()).toEqual({ + // Why identical: the batch is the shared index, which walks every worktree's tabs once per + // tabs-slice identity — not once per request — so a one-worktree request costs the same. + const singleWorktreeCounts = { runtimeEnumerations: 1, runtimeValueReads: contextCount, contextVisits: contextCount, targetTabIdReads: 1, unrelatedTabIdReads: tabCount - 1 - }) + } + expect(batched.counts()).toEqual(singleWorktreeCounts) + expect(reference.counts()).toEqual(singleWorktreeCounts) }) it('collapses multi-worktree runtime scans and caches unchanged publications', () => { @@ -533,7 +533,9 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { }) // Publications that change nothing the index reads cost nothing, however - // many cards call in. + // many cards call in. The warm-up pass is the cost of the batch loop above having left the + // one cache slot on a different fixture store; production has a single store. + selectRuntimeAgentOrchestrationForWorktree(reference.state, requested[0]) const referenceBefore = reference.counts() for (let publication = 0; publication < publicationCount; publication += 1) { for (const worktreeId of requested) { @@ -542,11 +544,9 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { } expect(reference.counts()).toEqual(referenceBefore) - // Why this is the honest claim: a real live-status ping replaces - // agentStatusByPaneKey, so the index does rebuild once per publication. What - // the shared index removes is the mounted-card multiplier, not the - // per-publication rebuild. Tab reads stay flat because tab membership is - // keyed on the tabs slice, which a live-status ping does not replace. + // A real live-status ping replaces agentStatusByPaneKey wholesale. The index is keyed on + // what it reads out of that map, not on its identity, so an unrelated pane's ping costs + // nothing: no rebuild, no context revisit, however many cards call in. const churn = makeCountedState() for (const worktreeId of requested) { selectRuntimeAgentOrchestrationForWorktree(churn.state, worktreeId) @@ -565,7 +565,7 @@ describe('selectRuntimeAgentOrchestrationBatch', () => { expect(churn.counts()).toEqual({ runtimeEnumerations: 1, runtimeValueReads: contextCount, - contextVisits: contextCount * (publicationCount + 1), + contextVisits: contextCount, tabIdReads: worktreeCount }) }) @@ -597,11 +597,11 @@ describe('selectRuntimeAgentOrchestrationBatch live-map churn', () => { } function builds(): number { - return _getRuntimeAgentOrchestrationBatchCountersForTest().builds + return _getWorktreeAgentOrchestrationIndexBuildCountForTest() } it('rebuilds once across repeated agentStatus:set identity churn on unrelated panes', () => { - releaseRuntimeAgentOrchestrationBatchCache() + releaseWorktreeAgentOrchestrationIndexCache() const first = selectRuntimeAgentOrchestrationBatch( makeChurnState({ [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-1') }), requested @@ -627,7 +627,7 @@ describe('selectRuntimeAgentOrchestrationBatch live-map churn', () => { // reads straight out of the live/retained maps is unkeyed and can go stale. The build no // longer receives those maps at all, which shows up here as exactly one read per pane. it('reads each orchestrated pane out of the live and retained maps once per build', () => { - releaseRuntimeAgentOrchestrationBatchCache() + releaseWorktreeAgentOrchestrationIndexCache() const liveReads: string[] = [] const retainedReads: string[] = [] const countReads = (target: Value, reads: string[]): Value => @@ -666,7 +666,7 @@ describe('selectRuntimeAgentOrchestrationBatch live-map churn', () => { }) it('rebuilds when an orchestrated pane changes worktree or the entry set changes', () => { - releaseRuntimeAgentOrchestrationBatchCache() + releaseWorktreeAgentOrchestrationIndexCache() const first = selectRuntimeAgentOrchestrationBatch( makeChurnState({ [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-1') }), requested @@ -709,24 +709,4 @@ describe('selectRuntimeAgentOrchestrationBatch live-map churn', () => { expect(builds()).toBe(removedBuilds + 1) expect(removed.has('wt-1')).toBe(false) }) - - it('does not re-derive the requested id list for an unchanged input identity', () => { - releaseRuntimeAgentOrchestrationBatchCache() - const stableIds = ['wt-1', 'wt-2'] - selectRuntimeAgentOrchestrationBatch( - makeChurnState({ [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-1') }), - stableIds - ) - const after = _getRuntimeAgentOrchestrationBatchCountersForTest().uniqueIdComputations - for (let index = 0; index < 25; index += 1) { - selectRuntimeAgentOrchestrationBatch( - makeChurnState({ - [CHILD_KEY]: makeEntry(CHILD_KEY, 'wt-1'), - [`unrelated-${index}`]: makeEntry(`unrelated-${index}`, 'wt-9') - }), - stableIds - ) - } - expect(_getRuntimeAgentOrchestrationBatchCountersForTest().uniqueIdComputations).toBe(after) - }) }) diff --git a/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.ts b/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.ts index 6b759592cd0..4891632eeb3 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-orchestration-batch.ts @@ -1,6 +1,11 @@ import type { AppState } from '@/store/types' import type { AgentStatusOrchestrationContext } from '../../../../shared/agent-status-types' -import { parsePaneKey } from '../../../../shared/stable-pane-id' +import { + EMPTY_WORKTREE_AGENT_ORCHESTRATION_INDEX, + selectWorktreeAgentOrchestrationIndex +} from './worktree-agent-orchestration-index' + +export { EMPTY_WORKTREE_AGENT_ORCHESTRATION } from './worktree-agent-orchestration-index' type RuntimeOrchestrationState = Pick< AppState, @@ -10,297 +15,28 @@ type RuntimeOrchestrationState = Pick< | 'tabsByWorktree' > -type RuntimeOrchestrationMap = RuntimeOrchestrationState['runtimeAgentOrchestrationByPaneKey'] -type RuntimeOrchestrationRecord = Record - -type RuntimeDomainCache = { - source: RuntimeOrchestrationMap - orderedEntries: [string, AgentStatusOrchestrationContext][] -} - -type RequestedTabMembershipCache = { - tabsSource: RuntimeOrchestrationState['tabsByWorktree'] - requestedWorktreeIds: readonly string[] - requestedIds: Set - worktreeIdsByTabId: Map> -} +/** + * No-op: the batch has no cache of its own. Kept because the dashboard's singleton and + * zero-worktree branches still announce that they are done with the batch view, and the shared + * index behind it must survive that — mounted sidebar cards are reading the same records. + */ +export function releaseRuntimeAgentOrchestrationBatchCache(): void {} /** - * Everything `buildRuntimeBatch` is allowed to read. The live and retained maps are not in - * here and not in its scope; they reach it only as `paneWorktreeIds`. Anything a future build - * needs has to be added here, and this record is also the cache key, so the key cannot drift - * from the read set. + * The dashboard's multi-worktree orchestration view. + * + * Why this is the shared index verbatim: the batch used to build its own worktree-keyed records + * from the same four slices, restricted to the requested ids. Callers only ever `.get(id)`, so + * the extra keys are unobservable, and one builder means one cache to keep honest and one + * correctness oracle to satisfy. `worktreeIds` survives only as the empty-dashboard + * short-circuit, which keeps the runtime map unread when nothing is on screen. */ -type RuntimeBatchInputs = { - runtimeSource: RuntimeOrchestrationMap - tabsSource: RuntimeOrchestrationState['tabsByWorktree'] - paneWorktreeIds: readonly (string | undefined)[] - requestedWorktreeIds: readonly string[] -} - -type RuntimeBatchCache = { - inputs: RuntimeBatchInputs - recordsByWorktree: ReadonlyMap -} - -const EMPTY_RUNTIME_ORCHESTRATION: RuntimeOrchestrationMap = {} -const EMPTY_TABS_BY_WORKTREE: RuntimeOrchestrationState['tabsByWorktree'] = {} -const EMPTY_AGENT_STATUS: RuntimeOrchestrationState['agentStatusByPaneKey'] = {} -const EMPTY_RETAINED_AGENTS: RuntimeOrchestrationState['retainedAgentsByPaneKey'] = {} -const EMPTY_BATCH: ReadonlyMap = new Map() - -export const EMPTY_WORKTREE_AGENT_ORCHESTRATION: RuntimeOrchestrationRecord = Object.freeze({}) - -// Why null-prototype: a pane key of `__proto__` is a plain data key here; on a -// normal object the write vanishes into the prototype setter and repoints it. -function createRecord(): RuntimeOrchestrationRecord { - return Object.create(null) as RuntimeOrchestrationRecord -} - -let runtimeDomainCache: RuntimeDomainCache | null = null -let requestedTabMembershipCache: RequestedTabMembershipCache | null = null -let runtimeBatchCache: RuntimeBatchCache | null = null -let runtimeBatchBuildCount = 0 -let uniqueWorktreeIdComputeCount = 0 - -export function releaseRuntimeAgentOrchestrationBatchCache(): void { - runtimeDomainCache = null - requestedTabMembershipCache = null - runtimeBatchCache = null -} - -export function _getRuntimeAgentOrchestrationBatchCountersForTest(): { - builds: number - uniqueIdComputations: number -} { - return { builds: runtimeBatchBuildCount, uniqueIdComputations: uniqueWorktreeIdComputeCount } -} - -function getOrderedRuntimeEntries( - runtimeAgentOrchestrationByPaneKey: RuntimeOrchestrationMap -): [string, AgentStatusOrchestrationContext][] { - if (runtimeDomainCache?.source === runtimeAgentOrchestrationByPaneKey) { - return runtimeDomainCache.orderedEntries - } - const orderedEntries = Object.entries(runtimeAgentOrchestrationByPaneKey) - runtimeDomainCache = { source: runtimeAgentOrchestrationByPaneKey, orderedEntries } - return orderedEntries -} - -// Why: the dashboard re-derives this list on every agent-status write. Keying on the caller's -// array identity lets the snapshot path (fresh array per call) miss without evicting the -// memoised sidebar path. Callers must not mutate an array they have already passed in. -const uniqueWorktreeIdsByInput = new WeakMap() - -function uniqueWorktreeIds(worktreeIds: readonly string[]): string[] { - const memoized = uniqueWorktreeIdsByInput.get(worktreeIds) - if (memoized) { - return memoized - } - uniqueWorktreeIdComputeCount += 1 - const uniqueIds: string[] = [] - const seen = new Set() - for (const worktreeId of worktreeIds) { - if (!seen.has(worktreeId)) { - seen.add(worktreeId) - uniqueIds.push(worktreeId) - } - } - uniqueWorktreeIdsByInput.set(worktreeIds, uniqueIds) - return uniqueIds -} - -function hasSameOrderedValues(previous: readonly unknown[], next: readonly unknown[]): boolean { - if (previous === next) { - return true - } - if (previous.length !== next.length) { - return false - } - return previous.every((value, index) => value === next[index]) -} - -function runtimeBatchInputsEqual(previous: RuntimeBatchInputs, next: RuntimeBatchInputs): boolean { - return ( - previous.runtimeSource === next.runtimeSource && - previous.tabsSource === next.tabsSource && - hasSameOrderedValues(previous.paneWorktreeIds, next.paneWorktreeIds) && - hasSameOrderedValues(previous.requestedWorktreeIds, next.requestedWorktreeIds) - ) -} - -/** - * The batch's whole view of the live and retained maps: the `worktreeId` each orchestrated - * pane key resolves to, as live,retained pairs in ordered-entry order. A status write for any - * other pane cannot change the batch, so this projection — not the map identities — is the - * correct cache key. Exact runtime keys preserve early SSH attribution and ignore stale - * `entry.paneKey` fields carried by a live or retained row. - */ -function projectPaneWorktreeIds( - orderedRuntimeEntries: readonly [string, AgentStatusOrchestrationContext][], - agentStatusByPaneKey: RuntimeOrchestrationState['agentStatusByPaneKey'], - retainedAgentsByPaneKey: RuntimeOrchestrationState['retainedAgentsByPaneKey'] -): (string | undefined)[] { - const paneWorktreeIds: (string | undefined)[] = [] - for (const [paneKey] of orderedRuntimeEntries) { - paneWorktreeIds.push( - agentStatusByPaneKey[paneKey]?.worktreeId, - retainedAgentsByPaneKey[paneKey]?.worktreeId - ) - } - return paneWorktreeIds -} - -function getRequestedTabMembership( - tabsByWorktree: RuntimeOrchestrationState['tabsByWorktree'], - requestedWorktreeIds: readonly string[] -): RequestedTabMembershipCache { - if ( - requestedTabMembershipCache?.tabsSource === tabsByWorktree && - hasSameOrderedValues(requestedTabMembershipCache.requestedWorktreeIds, requestedWorktreeIds) - ) { - return requestedTabMembershipCache - } - - const requestedIds = new Set(requestedWorktreeIds) - const worktreeIdsByTabId = new Map>() - for (const worktreeId of requestedWorktreeIds) { - // Why: the batch must not make a singleton dashboard scan unrelated tabs. - for (const tab of tabsByWorktree[worktreeId] ?? []) { - const tabId = tab.id - const existing = worktreeIdsByTabId.get(tabId) - if (existing) { - existing.add(worktreeId) - } else { - worktreeIdsByTabId.set(tabId, new Set([worktreeId])) - } - } - } - requestedTabMembershipCache = { - tabsSource: tabsByWorktree, - requestedWorktreeIds, - requestedIds, - worktreeIdsByTabId - } - return requestedTabMembershipCache -} - -function reuseRecordIfOrderedEqual( - previous: RuntimeOrchestrationRecord | undefined, - next: RuntimeOrchestrationRecord -): RuntimeOrchestrationRecord { - if (!previous) { - return next - } - const previousEntries = Object.entries(previous) - const nextEntries = Object.entries(next) - if (previousEntries.length !== nextEntries.length) { - return next - } - for (let index = 0; index < nextEntries.length; index += 1) { - if ( - previousEntries[index]?.[0] !== nextEntries[index]?.[0] || - previousEntries[index]?.[1] !== nextEntries[index]?.[1] - ) { - return next - } - } - return previous -} - -function buildRuntimeBatch( - inputs: RuntimeBatchInputs, - orderedRuntimeEntries: [string, AgentStatusOrchestrationContext][] -): ReadonlyMap { - runtimeBatchBuildCount += 1 - const { requestedIds, worktreeIdsByTabId } = getRequestedTabMembership( - inputs.tabsSource, - inputs.requestedWorktreeIds - ) - - const recordsByWorktree = new Map() - let projectionCursor = 0 - for (const [paneKey, orchestration] of orderedRuntimeEntries) { - const targets = new Set() - const parsed = parsePaneKey(paneKey) - const parsedParent = orchestration.parentPaneKey - ? parsePaneKey(orchestration.parentPaneKey) - : null - if (parsed) { - for (const worktreeId of worktreeIdsByTabId.get(parsed.tabId) ?? []) { - targets.add(worktreeId) - } - } - if (parsedParent) { - for (const worktreeId of worktreeIdsByTabId.get(parsedParent.tabId) ?? []) { - targets.add(worktreeId) - } - } - - const liveWorktreeId = inputs.paneWorktreeIds[projectionCursor] - const retainedWorktreeId = inputs.paneWorktreeIds[projectionCursor + 1] - projectionCursor += 2 - if (typeof liveWorktreeId === 'string' && requestedIds.has(liveWorktreeId)) { - targets.add(liveWorktreeId) - } - if (typeof retainedWorktreeId === 'string' && requestedIds.has(retainedWorktreeId)) { - targets.add(retainedWorktreeId) - } - - for (const worktreeId of targets) { - let record = recordsByWorktree.get(worktreeId) - if (!record) { - record = createRecord() - recordsByWorktree.set(worktreeId, record) - } - record[paneKey] = orchestration - } - } - - const previousRecords = runtimeBatchCache?.recordsByWorktree - for (const [worktreeId, record] of recordsByWorktree) { - recordsByWorktree.set( - worktreeId, - reuseRecordIfOrderedEqual(previousRecords?.get(worktreeId), record) - ) - } - return recordsByWorktree -} - export function selectRuntimeAgentOrchestrationBatch( state: RuntimeOrchestrationState, worktreeIds: readonly string[] -): ReadonlyMap { - const requestedWorktreeIds = uniqueWorktreeIds(worktreeIds) - if (requestedWorktreeIds.length === 0) { - releaseRuntimeAgentOrchestrationBatchCache() - return EMPTY_BATCH +): ReadonlyMap> { + if (worktreeIds.length === 0) { + return EMPTY_WORKTREE_AGENT_ORCHESTRATION_INDEX } - - const runtimeAgentOrchestrationByPaneKey = - state.runtimeAgentOrchestrationByPaneKey ?? EMPTY_RUNTIME_ORCHESTRATION - const orderedRuntimeEntries = getOrderedRuntimeEntries(runtimeAgentOrchestrationByPaneKey) - if (orderedRuntimeEntries.length === 0) { - releaseRuntimeAgentOrchestrationBatchCache() - return EMPTY_BATCH - } - - const inputs: RuntimeBatchInputs = { - runtimeSource: runtimeAgentOrchestrationByPaneKey, - tabsSource: state.tabsByWorktree ?? EMPTY_TABS_BY_WORKTREE, - paneWorktreeIds: projectPaneWorktreeIds( - orderedRuntimeEntries, - state.agentStatusByPaneKey ?? EMPTY_AGENT_STATUS, - state.retainedAgentsByPaneKey ?? EMPTY_RETAINED_AGENTS - ), - requestedWorktreeIds - } - if (runtimeBatchCache && runtimeBatchInputsEqual(runtimeBatchCache.inputs, inputs)) { - return runtimeBatchCache.recordsByWorktree - } - - // buildRuntimeBatch reuses the previous records, so publish the new cache only after it runs. - const recordsByWorktree = buildRuntimeBatch(inputs, orderedRuntimeEntries) - runtimeBatchCache = { inputs, recordsByWorktree } - return recordsByWorktree + return selectWorktreeAgentOrchestrationIndex(state) } diff --git a/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.test.ts b/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.test.ts index e7881d95aff..2d9ba917520 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.test.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.test.ts @@ -7,6 +7,7 @@ import type { RetainedAgentEntry } from '@/store/slices/agent-status' import type { TerminalTab } from '../../../../shared/terminal-tab-types' import { makePaneKey, parsePaneKey } from '../../../../shared/stable-pane-id' import { + _getWorktreeAgentOrchestrationIndexBuildCountForTest, EMPTY_WORKTREE_AGENT_ORCHESTRATION, releaseWorktreeAgentOrchestrationIndexCache, selectWorktreeAgentOrchestration @@ -247,6 +248,93 @@ describe('selectWorktreeAgentOrchestration', () => { expect(selectWorktreeAgentOrchestration(retainedChurn, 'wt-1')).toBe(first) }) + // Why a build counter and not record identity: `reuseRecordIfOrderedEqual` hides a rebuild + // from every identity assertion, so the wasted O(tabs + contexts) pass under `agentStatus:set` + // — several a second on a busy install, with a fresh live map each time — was invisible. + it('does not rebuild when agentStatus:set replaces the live map without moving a pane', () => { + const paneKey = paneKeyFor('tab-1', 0) + const context = { taskId: 't', dispatchId: 'd' } + const tabsByWorktree = { 'wt-1': [makeTab('tab-1')] } + const runtimeAgentOrchestrationByPaneKey = { [paneKey]: context } + const publish = (agentStatusByPaneKey: Record): IndexState => + ({ + tabsByWorktree, + runtimeAgentOrchestrationByPaneKey, + agentStatusByPaneKey, + retainedAgentsByPaneKey: {} + }) as unknown as IndexState + + const first = selectWorktreeAgentOrchestration( + publish({ [paneKey]: makeEntry(paneKey, 'wt-1') }), + 'wt-1' + ) + const buildsAfterFirst = _getWorktreeAgentOrchestrationIndexBuildCountForTest() + + for (let tick = 0; tick < 25; tick += 1) { + // A fresh live map every tick, exactly as `agentStatus:set` replaces the slice, plus a + // stable entry for the orchestrated pane so the projection is non-trivially equal. + const published = publish({ + [paneKey]: makeEntry(paneKey, 'wt-1'), + [`unrelated-${tick}`]: makeEntry(`unrelated-${tick}`, 'wt-9') + }) + expect(selectWorktreeAgentOrchestration(published, 'wt-1')).toBe(first) + } + expect(_getWorktreeAgentOrchestrationIndexBuildCountForTest()).toBe(buildsAfterFirst) + + // ...and the projection is still load-bearing: moving that pane must re-attribute it. + const moved = publish({ [paneKey]: makeEntry(paneKey, 'wt-2') }) + expect(selectWorktreeAgentOrchestration(moved, 'wt-2')[paneKey]).toBe(context) + expect(_getWorktreeAgentOrchestrationIndexBuildCountForTest()).toBe(buildsAfterFirst + 1) + }) + + // Why counted rather than timed: the projection is the index's per-publication work, and + // recomputing it per card would put the O(contexts) scan back on the per-card path that the + // index exists to remove — which no identity or correctness assertion would notice. + it('projects the live and retained maps once per publication, not once per card', () => { + const cardCount = 8 + const contextCount = 6 + const tabsByWorktree: Record = {} + const runtimeAgentOrchestrationByPaneKey: Record = {} + for (let index = 0; index < cardCount; index += 1) { + tabsByWorktree[`wt-${index}`] = [makeTab(`tab-${index}`)] + } + for (let index = 0; index < contextCount; index += 1) { + runtimeAgentOrchestrationByPaneKey[paneKeyFor(`tab-${index}`, index)] = { + taskId: `t-${index}`, + dispatchId: `d-${index}` + } + } + let liveReads = 0 + let retainedReads = 0 + const countReads = (target: object, onRead: () => void): object => + new Proxy(target, { + get(source, key, receiver) { + if (typeof key === 'string') { + onRead() + } + return Reflect.get(source, key, receiver) + } + }) + const state = { + tabsByWorktree, + runtimeAgentOrchestrationByPaneKey, + agentStatusByPaneKey: countReads({}, () => { + liveReads += 1 + }), + retainedAgentsByPaneKey: countReads({}, () => { + retainedReads += 1 + }) + } as unknown as IndexState + + for (let card = 0; card < cardCount; card += 1) { + selectWorktreeAgentOrchestration(state, `wt-${card}`) + } + expect({ liveReads, retainedReads }).toEqual({ + liveReads: contextCount, + retainedReads: contextCount + }) + }) + it('rebuilds when a source it reads actually changes', () => { const context = { taskId: 't', dispatchId: 'd' } const paneKey = paneKeyFor('tab-1', 0) diff --git a/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.ts b/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.ts index 5cad5ec1f48..00e0c8af9e3 100644 --- a/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.ts +++ b/src/renderer/src/components/sidebar/worktree-agent-orchestration-index.ts @@ -22,11 +22,18 @@ type TabMembershipCache = { worktreeIdsByTabId: Map> } +type PaneWorktreeProjectionCache = { + runtimeSource: OrchestrationIndexState['runtimeAgentOrchestrationByPaneKey'] + liveSource: OrchestrationIndexState['agentStatusByPaneKey'] + retainedSource: OrchestrationIndexState['retainedAgentsByPaneKey'] + paneWorktreeIds: readonly (string | undefined)[] +} + type OrchestrationIndexCache = { runtimeSource: OrchestrationIndexState['runtimeAgentOrchestrationByPaneKey'] tabsSource: OrchestrationIndexState['tabsByWorktree'] - liveSource: OrchestrationIndexState['agentStatusByPaneKey'] - retainedSource: OrchestrationIndexState['retainedAgentsByPaneKey'] + /** @see projectPaneWorktreeIds — the build's whole view of the live and retained maps. */ + paneWorktreeIds: readonly (string | undefined)[] recordsByWorktree: ReadonlyMap } @@ -51,14 +58,79 @@ function createRecord(): RuntimeOrchestrationRecord { let runtimeEntriesCache: RuntimeEntriesCache | null = null let tabMembershipCache: TabMembershipCache | null = null +let paneWorktreeProjectionCache: PaneWorktreeProjectionCache | null = null let orchestrationIndexCache: OrchestrationIndexCache | null = null +let indexBuildCount = 0 export function releaseWorktreeAgentOrchestrationIndexCache(): void { runtimeEntriesCache = null tabMembershipCache = null + paneWorktreeProjectionCache = null orchestrationIndexCache = null } +export function _getWorktreeAgentOrchestrationIndexBuildCountForTest(): number { + return indexBuildCount +} + +/** + * The build's whole view of the live and retained maps: the `worktreeId` each orchestrated pane + * key resolves to, as live,retained pairs in entry order. A status write for any other pane + * cannot change the index, so this projection — not the map identities — is the correct cache + * key, and `agentStatus:set` replaces those maps several times a second. + * + * Why exact runtime keys: this preserves early SSH attribution and ignores stale `entry.paneKey` + * fields carried by a live or retained row. + */ +function projectPaneWorktreeIds( + runtimeSource: OrchestrationIndexState['runtimeAgentOrchestrationByPaneKey'], + runtimeEntries: readonly [string, AgentStatusOrchestrationContext][], + agentStatusByPaneKey: OrchestrationIndexState['agentStatusByPaneKey'], + retainedAgentsByPaneKey: OrchestrationIndexState['retainedAgentsByPaneKey'] +): readonly (string | undefined)[] { + // Why memoised on the map identities: every mounted card calls this selector on the same + // publication, and re-walking the contexts per card is the per-card cost the index removes. + if ( + paneWorktreeProjectionCache?.runtimeSource === runtimeSource && + paneWorktreeProjectionCache.liveSource === agentStatusByPaneKey && + paneWorktreeProjectionCache.retainedSource === retainedAgentsByPaneKey + ) { + return paneWorktreeProjectionCache.paneWorktreeIds + } + const paneWorktreeIds: (string | undefined)[] = [] + for (const [paneKey] of runtimeEntries) { + paneWorktreeIds.push( + agentStatusByPaneKey[paneKey]?.worktreeId, + retainedAgentsByPaneKey[paneKey]?.worktreeId + ) + } + paneWorktreeProjectionCache = { + runtimeSource, + liveSource: agentStatusByPaneKey, + retainedSource: retainedAgentsByPaneKey, + paneWorktreeIds + } + return paneWorktreeIds +} + +function hasSameOrderedValues( + previous: readonly (string | undefined)[], + next: readonly (string | undefined)[] +): boolean { + if (previous === next) { + return true + } + 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 +} + function reuseRecordIfOrderedEqual( previous: RuntimeOrchestrationRecord | undefined, next: RuntimeOrchestrationRecord @@ -109,12 +181,13 @@ function getWorktreeIdsByTabId( function buildIndex( runtimeEntries: [string, AgentStatusOrchestrationContext][], tabsByWorktree: OrchestrationIndexState['tabsByWorktree'], - agentStatusByPaneKey: OrchestrationIndexState['agentStatusByPaneKey'], - retainedAgentsByPaneKey: OrchestrationIndexState['retainedAgentsByPaneKey'] + paneWorktreeIds: readonly (string | undefined)[] ): ReadonlyMap { + indexBuildCount += 1 const worktreeIdsByTabId = getWorktreeIdsByTabId(tabsByWorktree) const recordsByWorktree = new Map() + let projectionCursor = 0 for (const [paneKey, orchestration] of runtimeEntries) { const parsed = parsePaneKey(paneKey) const parsedParent = orchestration.parentPaneKey @@ -134,13 +207,12 @@ function buildIndex( targets.add(worktreeId) } } - // Why exact runtime keys: this preserves early SSH attribution and ignores - // stale entry.paneKey fields carried by a live or retained row. - const liveWorktreeId = agentStatusByPaneKey[paneKey]?.worktreeId + const liveWorktreeId = paneWorktreeIds[projectionCursor] + const retainedWorktreeId = paneWorktreeIds[projectionCursor + 1] + projectionCursor += 2 if (typeof liveWorktreeId === 'string') { targets.add(liveWorktreeId) } - const retainedWorktreeId = retainedAgentsByPaneKey[paneKey]?.worktreeId if (typeof retainedWorktreeId === 'string') { targets.add(retainedWorktreeId) } @@ -166,16 +238,15 @@ function buildIndex( } /** - * Worktree-keyed index of runtime agent orchestration contexts, rebuilt only - * when one of its four source maps changes identity. + * Worktree-keyed index of runtime agent orchestration contexts, rebuilt only when the context + * map, the tabs slice, or the per-pane worktree projection of the live/retained maps changes. * - * Why: every mounted worktree card subscribes to its own orchestration slice, - * and Zustand re-runs every subscriber's selector on every store publication. - * Scanning the whole context map per card made that O(cards x contexts). What - * this removes is the per-card multiplier, not the rebuild itself: an agent - * ping replaces the live map, so the index still rebuilds once per publication. - * The first caller through a given store version pays O(tabs + contexts); the - * rest are a Map lookup. + * Why: every mounted worktree card subscribes to its own orchestration slice, and Zustand + * re-runs every subscriber's selector on every store publication. Scanning the whole context + * map per card made that O(cards x contexts). Keying on the live and retained map identities + * then made the index rebuild once per `agentStatus:set` even though a status write for an + * unorchestrated pane cannot change a single record; keying on the projection instead is what + * makes those publications free. */ export function selectWorktreeAgentOrchestrationIndex( state: OrchestrationIndexState @@ -184,7 +255,7 @@ export function selectWorktreeAgentOrchestrationIndex( state.runtimeAgentOrchestrationByPaneKey ?? EMPTY_SOURCE // Why cached separately from the index: enumerating the context map is the // per-publication cost this index exists to remove, and the entry list stays - // valid even when a churning live/retained slice forces an index rebuild. + // valid across the live/retained churn the projection absorbs. if (runtimeEntriesCache?.source !== runtimeAgentOrchestrationByPaneKey) { runtimeEntriesCache = { source: runtimeAgentOrchestrationByPaneKey, @@ -198,35 +269,38 @@ export function selectWorktreeAgentOrchestrationIndex( // Why the entries cache survives: dropping it would re-enumerate the empty // map once per card, which is the per-publication cost this index removes. tabMembershipCache = null + paneWorktreeProjectionCache = null orchestrationIndexCache = null return EMPTY_WORKTREE_AGENT_ORCHESTRATION_INDEX } const tabsByWorktree = state.tabsByWorktree ?? EMPTY_SOURCE - const agentStatusByPaneKey = state.agentStatusByPaneKey ?? EMPTY_SOURCE - const retainedAgentsByPaneKey = state.retainedAgentsByPaneKey ?? EMPTY_SOURCE + const paneWorktreeIds = projectPaneWorktreeIds( + runtimeAgentOrchestrationByPaneKey, + runtimeEntries, + state.agentStatusByPaneKey ?? EMPTY_SOURCE, + state.retainedAgentsByPaneKey ?? EMPTY_SOURCE + ) if ( orchestrationIndexCache?.runtimeSource === runtimeAgentOrchestrationByPaneKey && orchestrationIndexCache.tabsSource === tabsByWorktree && - orchestrationIndexCache.liveSource === agentStatusByPaneKey && - orchestrationIndexCache.retainedSource === retainedAgentsByPaneKey + hasSameOrderedValues(orchestrationIndexCache.paneWorktreeIds, paneWorktreeIds) ) { + // Why adopt the equal array: the remaining cards on this publication then compare by + // identity instead of walking it again. + orchestrationIndexCache.paneWorktreeIds = paneWorktreeIds return orchestrationIndexCache.recordsByWorktree } + // buildIndex reuses the previous build's records, so publish the new cache only after it runs. + const recordsByWorktree = buildIndex(runtimeEntries, tabsByWorktree, paneWorktreeIds) orchestrationIndexCache = { runtimeSource: runtimeAgentOrchestrationByPaneKey, tabsSource: tabsByWorktree, - liveSource: agentStatusByPaneKey, - retainedSource: retainedAgentsByPaneKey, - recordsByWorktree: buildIndex( - runtimeEntries, - tabsByWorktree, - agentStatusByPaneKey, - retainedAgentsByPaneKey - ) + paneWorktreeIds, + recordsByWorktree } - return orchestrationIndexCache.recordsByWorktree + return recordsByWorktree } export function selectWorktreeAgentOrchestration(