Repository navigation
perf(renderer): index unread completion tabs once per snapshot - #17246
Conversation
Greptile SummaryThis PR replaces repeated per-tab scans of unread completion markers with a snapshot-identity WeakMap index while preserving existing marker ownership and truthiness semantics.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or quality issues identified. The cache preserves the prior ownership and truthy-marker behavior, while marker updates use replacement snapshots that naturally invalidate the identity-keyed index.
|
| Filename | Overview |
|---|---|
| src/renderer/src/components/tab-bar/terminal-tab-activity-status.ts | Adds a garbage-collectable per-snapshot unread-tab index; all identified producers preserve its immutability requirement. |
| src/renderer/src/components/tab-bar/terminal-tab-activity-status.test.ts | Extends behavioral parity coverage and verifies that one snapshot is enumerated only once across repeated tab lookups. |
Reviews (1): Last reviewed commit: "perf(renderer): index unread completion ..." | Re-trigger Greptile
|
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 (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds a Merge Risk: ⚪ Minimal · up to The change reuses a per-snapshot tab lookup index to reduce repeated renderer work without changing unread-marker behavior or external interfaces. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description is detailed and relevant, but it omits required template information. No linked issue is provided, and the required Visual Proof section is missing despite describing visible behavior validation. Several template sections and checklist confirmations are also absent. Resolution Add the required Linked Issue with an issue reference. Add a Visual Proof section with before-and-after attachments, or write N/A with a reason if the change has no visual or interaction impact. Include the required Testing, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist sections, with applicable items completed or marked N/A.
✨ 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 |
ELI5
Every mounted terminal tab was rereading the entire unread-completion list whenever Zustand published any store update. With many tabs and markers, that made one harmless publication do the same long scan hundreds or thousands of times on the renderer thread.
This builds the list of owning tab IDs once for each immutable unread-map snapshot. Every tab then does one constant-time membership check. No notification, marker, tab, or feature is capped or disabled.
User-visible problem
SortableTabstays mounted for every terminal and agent-session tab. Its selector calledhasUnreadAgentCompletionForTerminalTab, which scanned allUpane markers for each ofTtabs on every store publication—even when React ultimately rendered nothing. Accumulated sessions could therefore turn background status/title writes into synchronous O(T × U) input and status latency.Change
ReadonlySet<tabId>byunreadAgentCompletionPanesobject identity in aWeakMap.Production add, clear, close, hibernate, move, and purge paths already replace this map rather than mutating it in place; the test exercises add/clear through new snapshot identities.
Deterministic before / after
Node 24.18.0, actual helper, mostly-absent lookups, three samples per case. Before uses
origin/main; after uses a fresh marker-map identity for every sample so index construction is included.The committed structural regression test is timing-independent: 2,500 tab lookups over one 1,000-marker snapshot caused 2,500 enumerations / 2,500,000 marker reads before and exactly 1 enumeration / 1,000 reads after.
Parity measurements at 1,000 lookups × 10,000 markers kept identical hit counts for absent, first match, last match, explicit
false, and legacy no-colon keys. After times were 1.13–1.46 ms versus 1,110.50–1,330.56 ms before.Low-cardinality guard: one tab with an early match did not regress cold construction—at 10,000 markers it measured 1.157 ms before and 0.999 ms after; an absent match measured 1.267 ms before and 1.012 ms after.
Regression and compatibility proof
WeakMapavoids strong retention and supports multiple concurrently retained snapshots.Electron CDP validation used an isolated profile and the intended branch identity. On two real rendered terminal tabs, adding an unread completion marker to the inactive tab visibly applied the amber unread treatment only there; the real clear action removed the store marker and the treatment from both tabs. The only console errors were the pre-existing exhausted GitHub API limit.
Verification
pnpm tcpnpm run check:code-quality:changed— 0 findingspnpm run check:max-lines-ratchetoxfmtgit diff --checkGitHub overlap and merge risk
Checked before editing. Merged PRs #13554 and #16173 cache/gate different unread projections; no active PR implements this per-tab index. Open PR #9318 is the only direct file overlap and still scans the marker map per tab, so this is not duplicate work.