perf(tabs): index tab agent status by tab instead of scanning the global map - #12413
perf(tabs): index tab agent status by tab instead of scanning the global map#12413brennanb2025 wants to merge 2 commits into
Conversation
…bal map resolveAnyCompletedTabAgent and its live/retained twins scanned the whole agentStatusByPaneKey map and parsed every pane key, once per tab per render — ~10^5 parsePaneKey calls per render pass with 200 tabs. Cache a per-tab pane index on the map's identity (the store replaces it on every write) so a render pass scans once instead of once per tab. Insertion order is preserved because the resolvers return the first match.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds cached indexes for live, completed, and retained tab-agent status entries. The indexes preserve insertion order, filter invalid pane keys and unsupported agents, and refresh when source-map identity changes. Tab-agent resolution now uses shared selectors and exclusion logic. Tests compare indexed results with scan-based oracles and cover randomized fixtures, ordering, filtering, malformed keys, empty maps, active-leaf exclusion, and map replacement. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Electron QA evidence from an isolated dev profile on the PR worktree. Working agent status — Codex identity remains visible in the tab bar and worktree card:
Completed agent status — the completed marker and Codex identity remain visible with the surrounding terminal UI intact:
Validated in an isolated Electron instance over CDP; this PR intentionally has no visual design change. |
1 participant
Summary
Tab-bar agent-icon resolution no longer scans the entire global agent-status map once per tab per render.
resolveAnyCompletedTabAgent(and its live/retained twins insrc/renderer/src/lib/tab-agent.ts) didObject.entries(agentStatusByPaneKey)plus aparsePaneKeyon every key, anduseTabAgentcalls them for every mounted tab on every render — with 200 worktrees/tabs and hundreds of status entries that is ~10^5parsePaneKeycalls per render pass.CPU profiles taken on the prod app with 200 worktrees showed
resolveSiblingCompletedTabAgent/resolveAnyCompletedTabAgentin every profile: 4.6% self-time during a cold worktree switch (~12% of the switch window including the surrounding frames in the same chunk) and 1.5% during steady-state churn.This adds
src/renderer/src/lib/tab-agent-status-index.ts: a per-tab index of icon-capable panes, cached on the identity of the source map. The store replacesagentStatusByPaneKey/retainedAgentsByPaneKeyon every write (and keeps the identity when nothing changed), so identity is an exact invalidation signal — one scan per store write instead of one per tab per render. This is the same idiom already used byterminal-tab-agent-type-index.ts.Behavior-preserving: the index keeps each tab's panes in the source map's insertion order, because the resolvers return the FIRST match and a split tab with two done panes running different agents would otherwise flip its icon. Focused-pane lookups were already O(1) and are unchanged.
Measured
Micro-benchmark (scratch script, not committed): 800 entries across 200 tabs × 4 leaves, mixed
done/working; per render pass a fresh map identity (the real pattern: a store write replaces the map, then all tabs re-render) then 200 tabs × (resolveFocusedCompletedTabAgent+resolveSiblingCompletedTabAgent); 100 render passes; best of two timed runs after warm-up.47.9× faster, 0.252 ms per simulated render pass (target was ≥10× and <1 ms). Both implementations returned identical results over the whole run (parity sink 23400 = 23400).
Screenshots
No visual change.
Testing
pnpm lint(full gate, passed)pnpm typecheck(pnpm run tc, afterrm -f config/*.tsbuildinfo)pnpm test— targeted: all ofsrc/renderer/src/lib+src/renderer/src/components/tab-bar(407 files, 3794 passed / 1 skipped). Full-repopnpm testnot run locally; CI covers it.pnpm build(not run; no build-surface change — renderer-only TS)New
src/renderer/src/lib/tab-agent-status-index.test.ts:activeLeafId) assert all six exported resolvers return identical results.parsePaneKeyrejects, empty maps, and re-indexing when the store replaces the map identity.Also ran, since the pre-commit hook was not executable in this worktree:
config/scripts/check-changed-code-quality.mjs(0 new findings across 3 changed files, incl. type-aware + the react-doctor oxlint plugin) andconfig/scripts/check-react-doctor-changed.mjs(0 issues).AI Review Report
Self-review of the diff with the following risks checked:
tabId, excludedleafId, then took the first entry whose agent resolved non-null; the index applies exactly those filters at build time and preserves order. Entries that failparsePaneKeyor whoseagentTypeis not iconable (undefined,'unknown', custom names) are dropped at build time, which is equivalent to the oldcontinue. Verified empirically by the 250-case oracle suite rather than by inspection alone.agentStatusByPaneKey/retainedAgentsByPaneKeyinstore/slices/agent-status.ts: all replace the record (nextLive, spread copies,removePaneKeys,movePaneKeyedRecord); no in-place mutation, no immer drafts. Identity-keying is therefore exact. A stale index would require mutating a map in place, which no code path does.useTabAgent's four calls per tab hit the cache. Only alternating between two different map objects would thrash, which the store never does.Map,Object.entries, string compare) with identical behavior on all three platforms.Array#toReversedin the test is ES2023, available in the Electron/Node runtimes the repo already targets (already used elsewhere insrc/renderer). Nothing here is specific to local vs SSH/remote worktrees or to git-worktree vs folder workspaces — both feed the same pane-key-shaped map.tab-agent.tsexports are unchanged; its only non-test importer (use-tab-agent.ts) needed no edit.Flagged and addressed: an initial version used
Array#reverse()in a test, whichoxlintrejects (unicorn/no-array-reverse) → switched totoReversed().Security Audit
Low risk; no new attack surface.
parsePaneKeyvalidator (single-colon, UUID leaf); malformed keys are dropped exactly as before, and a test covers that. No new parsing, no regex added, no user-controlled string is interpolated anywhere.child_process, no filesystem access, no credentials.tabId → [{leafId, agent}]) over entries that were already retained in the store, bounded by the map that already exists.No follow-up needed.
Notes
console, so it wrote results to/tmp/tab-agent-bench.txt); it is intentionally not committed, since it duplicates the pre-index implementation which now lives in the parity test as the oracle.src/renderer/src/components/terminal-pane/terminal-tab-agent-type-index.ts; that selector is left alone — this PR is scoped to thetab-agent.tsresolvers.done) resolversresolveSiblingTabAgent/resolveFocusedTabAgenthad the identical full-map-scan shape and are indexed here too, since one pass builds both the live and completed indexes.