perf(lanes): reduce hidden-work and session refresh load#514
Merged
Conversation
Measured with perf-pass run lanes-20260531-1421-after1 against baseline lanes-20260531-1421-baseline. Lanes navigation localRuntime.callAction count dropped 87 -> 28 and runtime IPC duration dropped 25.4s -> 13.3s. Stack Graph open now hydrates agent rosters on demand, adding 63 callAction calls totaling 450ms.
Minimized Work lane switch local-runtime callAction dropped 22 -> 16 calls and 2230ms -> 1545ms summed duration in lanes-20260531-compare-candidates-2.
CLI-heavy benchmark with 50k shell sessions and 20 chat sessions improved chat session query time from 11.6ms to 0.079ms per call by using lane/tool indexes and separating legacy fallback rows.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR unifies multiple performance optimizations across the Lanes tab: session filtering by tool type, real-time app resource usage monitoring with pressure indicators, visibility-aware refresh deferral for background operations, deferred pane mounting with staggered timing, activity-based gating of Git Actions side effects, PR refresh scheduling for visible lanes, terminal visibility management, and adaptive grid terminal refresh based on system pressure. ChangesLanes Tab Performance Optimization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
arul28
changed the title
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
June 2, 2026 18:43
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/lib/sessionListCache.ts (1)
66-92:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
projectRootonly changes the cache key, not the data being fetched.When
options.projectRootis set, this path namespaces the cache entry under that root, but the actual fetch still callswindow.ade.sessions.list(normalizeArgs(args)). If the active project inuseAppStoreis different, you can cache/project/arows under a/project/bkey and then serve the wrong sessions on later reads. Either plumbprojectRootthrough the IPC/shared/preload contract or remove the override until the fetch can honor it. As per coding guidelines, "Keep IPC contracts, preload types, shared types, and renderer usage in sync whenever an interface changes."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/lib/sessionListCache.ts` around lines 66 - 92, The cache key uses options.projectRoot but the fetch still calls window.ade.sessions.list(normalizeArgs(args)), which can return sessions for a different active project; update the fetch to honor the projectRoot (or stop namespacing the key) by passing options.projectRoot into the normalized args used by window.ade.sessions.list (e.g., extend normalizeArgs(args) to accept projectRoot and call window.ade.sessions.list(normalizeArgs(args, options?.projectRoot)) so the data fetched matches the cache key), or remove the projectRoot override entirely from the key until the IPC/preload contract is updated.
🧹 Nitpick comments (2)
apps/desktop/src/main/services/chat/agentChatService.ts (1)
2551-2557: ⚡ Quick winConsider deriving the type guard from the constant to eliminate duplication.
The
CHAT_SESSION_TOOL_TYPESconstant and theisChatToolTypetype guard (lines 2559-2561) both hardcode the same list of chat tool types. If a new chat tool type is added, both locations must be updated, creating a maintenance hazard.♻️ Suggested refactor to derive the type from the constant
+type ChatToolType = (typeof CHAT_SESSION_TOOL_TYPES)[number]; + function isChatToolType( toolType: TerminalToolType | null | undefined, -): toolType is "codex-chat" | "claude-chat" | "opencode-chat" | "cursor" | "droid-chat" { +): toolType is ChatToolType { + return toolType != null && CHAT_SESSION_TOOL_TYPES.includes(toolType as ChatToolType); - return ( - toolType === "codex-chat" || - toolType === "claude-chat" || - toolType === "opencode-chat" || - toolType === "cursor" || - toolType === "droid-chat" - ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/chat/agentChatService.ts` around lines 2551 - 2557, The CHAT_SESSION_TOOL_TYPES array is duplicated in the isChatToolType guard causing maintenance drift; update isChatToolType to derive membership from the CHAT_SESSION_TOOL_TYPES constant instead of re-listing values (e.g., ensure CHAT_SESSION_TOOL_TYPES is a literal/readonly array and have isChatToolType return CHAT_SESSION_TOOL_TYPES.includes(value as TerminalToolType)), so adding new chat tool types requires changing only CHAT_SESSION_TOOL_TYPES.apps/desktop/src/renderer/components/terminals/TerminalView.tsx (1)
649-671: ⚡ Quick winMake
inertattribute handling consistent witharia-hidden.The
aria-hiddenattribute now uses explicitremoveAttribute/setAttribute(lines 658-662), which is clearer than the previous toggle-based approach. However, theinertattribute still usestoggleAttribute(line 667), creating an inconsistency in the same function.♻️ Suggested refactor for consistency
try { if (active) { runtime.host.removeAttribute("aria-hidden"); } else { runtime.host.setAttribute("aria-hidden", "true"); } } catch { // ignore } try { - runtime.host.toggleAttribute("inert", !active); + if (active) { + runtime.host.removeAttribute("inert"); + } else { + runtime.host.setAttribute("inert", ""); + } } catch { // ignore }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/terminals/TerminalView.tsx` around lines 649 - 671, Summary: Make inert attribute handling consistent with aria-hidden by using explicit setAttribute/removeAttribute in setRuntimeInteractionState. In function setRuntimeInteractionState, replace the runtime.host.toggleAttribute("inert", !active) usage with explicit calls: when active is true call runtime.host.removeAttribute("inert"), otherwise call runtime.host.setAttribute("inert", "true") (or set empty string) inside the existing try/catch so behavior matches the aria-hidden branch; keep the same runtime and host references.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/main/services/ipc/registerIpc.ts`:
- Around line 810-822: getAppResourceUsageSnapshot mixes global Electron metrics
(app.getAppMetrics()) with PTY/runtime usage from a single context (ptyService,
sessionService, localRuntimeConnectionPool, processRegistry), producing
inconsistent totals; fix by either aggregating PTY/runtime usage across all
active contexts or returning a per-window scoped snapshot: update
getAppResourceUsageSnapshot (and any callers) to either iterate all contexts to
sum combinePtyResourceUsage and getRuntimeOwnedPtyUsage, or change the API to
accept a window/sender identifier and only use app.getAppMetrics() together with
the matching context's PTY/runtime snapshot so metrics are consistent (apply
same change pattern used around the other occurrence noted at lines ~3530-3538).
In `@apps/desktop/src/renderer/components/lanes/LanesPage.tsx`:
- Around line 1149-1185: The effect in LanesPage.tsx that runs the opportunistic
PR refresh bails when document.visibilityState !== "visible" but won't re-run on
hidden→visible transitions because visibility isn't in the dependency list; add
a visibility-aware trigger (e.g., a small piece of state like isDocumentVisible
or a "visibilitychange" listener) and include that state in the useEffect
dependency array so the effect re-evaluates when the tab becomes visible again;
update the useEffect (the block that reads project?.rootPath, active,
selectVisibleLanePrRefreshIds, and sets
laneVisiblePrRefreshRequestedAtRef/current timers) to depend on this new
visibility signal so the refresh selection runs on visibilitychange.
- Around line 201-209: The effect currently resets ready whenever delayMs or
cacheKey changes, causing remounts; change it to only apply the initial delay
the first time a given cacheKey appears. Implement a persistent ref/Set (e.g.,
initializedCacheKeys via useRef) and in the useEffect for cacheKey/delayMs
check: if initializedCacheKeys has cacheKey, immediately setReady(true) and do
not toggle to false or start a timer; otherwise add cacheKey to the Set, apply
the existing delay behavior (setReady(false), start timer -> setReady(true)),
and call clearTimeout in the cleanup. Keep references to cacheKey, delayMs,
setReady, and the useEffect as the location to modify.
In `@apps/desktop/src/renderer/lib/resourcePressure.ts`:
- Around line 75-79: The function formatSystemMemory incorrectly treats
freeMemoryMB === 0 as missing; change the falsy checks to explicit
null/undefined checks so zero is accepted: in formatSystemMemory, replace the
condition "if (!usage?.freeMemoryMB || !usage.totalMemoryMB ||
usage.totalMemoryMB <= 0) return null;" with explicit checks like "if (usage ==
null || usage.freeMemoryMB == null || usage.totalMemoryMB == null ||
usage.totalMemoryMB <= 0) return null;" so that usage.freeMemoryMB === 0 yields
"100% system memory used" as expected.
In `@apps/desktop/src/renderer/lib/sessionListCache.ts`:
- Around line 25-29: The toolTypes branch keeps original strings so values with
surrounding whitespace are treated as different keys; change the pipeline in the
block handling args.toolTypes (where normalized.toolTypes is set) to trim each
toolType first, filter out empty trimmed strings, then deduplicate (e.g., via
new Set) and sort, and assign the resulting array to normalized.toolTypes so all
entries are stored and cached using their trimmed form.
---
Outside diff comments:
In `@apps/desktop/src/renderer/lib/sessionListCache.ts`:
- Around line 66-92: The cache key uses options.projectRoot but the fetch still
calls window.ade.sessions.list(normalizeArgs(args)), which can return sessions
for a different active project; update the fetch to honor the projectRoot (or
stop namespacing the key) by passing options.projectRoot into the normalized
args used by window.ade.sessions.list (e.g., extend normalizeArgs(args) to
accept projectRoot and call window.ade.sessions.list(normalizeArgs(args,
options?.projectRoot)) so the data fetched matches the cache key), or remove the
projectRoot override entirely from the key until the IPC/preload contract is
updated.
---
Nitpick comments:
In `@apps/desktop/src/main/services/chat/agentChatService.ts`:
- Around line 2551-2557: The CHAT_SESSION_TOOL_TYPES array is duplicated in the
isChatToolType guard causing maintenance drift; update isChatToolType to derive
membership from the CHAT_SESSION_TOOL_TYPES constant instead of re-listing
values (e.g., ensure CHAT_SESSION_TOOL_TYPES is a literal/readonly array and
have isChatToolType return CHAT_SESSION_TOOL_TYPES.includes(value as
TerminalToolType)), so adding new chat tool types requires changing only
CHAT_SESSION_TOOL_TYPES.
In `@apps/desktop/src/renderer/components/terminals/TerminalView.tsx`:
- Around line 649-671: Summary: Make inert attribute handling consistent with
aria-hidden by using explicit setAttribute/removeAttribute in
setRuntimeInteractionState. In function setRuntimeInteractionState, replace the
runtime.host.toggleAttribute("inert", !active) usage with explicit calls: when
active is true call runtime.host.removeAttribute("inert"), otherwise call
runtime.host.setAttribute("inert", "true") (or set empty string) inside the
existing try/catch so behavior matches the aria-hidden branch; keep the same
runtime and host references.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 814fd50b-4a7f-49b9-b658-24af318160cd
⛔ Files ignored due to path filters (1)
README.mdis excluded by!*.md
📒 Files selected for processing (36)
.agents/skills/ade-perf-lanes/SKILL.mdapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.tsapps/desktop/src/main/services/pty/ptyService.tsapps/desktop/src/main/services/sessions/sessionService.test.tsapps/desktop/src/main/services/sessions/sessionService.tsapps/desktop/src/main/services/state/kvDb.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/app/TopBar.test.tsxapps/desktop/src/renderer/components/app/TopBar.tsxapps/desktop/src/renderer/components/lanes/CommitTimeline.test.tsxapps/desktop/src/renderer/components/lanes/CommitTimeline.tsxapps/desktop/src/renderer/components/lanes/LaneGitActionsPane.test.tsxapps/desktop/src/renderer/components/lanes/LaneGitActionsPane.tsxapps/desktop/src/renderer/components/lanes/LanesPage.test.tsapps/desktop/src/renderer/components/lanes/LanesPage.tsxapps/desktop/src/renderer/components/lanes/lanePageModel.tsapps/desktop/src/renderer/components/lanes/useLaneWorkSessions.test.tsapps/desktop/src/renderer/components/lanes/useLaneWorkSessions.tsapps/desktop/src/renderer/components/terminals/TerminalView.test.tsxapps/desktop/src/renderer/components/terminals/TerminalView.tsxapps/desktop/src/renderer/components/terminals/WorkViewArea.test.tsxapps/desktop/src/renderer/components/terminals/WorkViewArea.tsxapps/desktop/src/renderer/components/terminals/useWorkSessions.test.tsapps/desktop/src/renderer/components/terminals/useWorkSessions.tsapps/desktop/src/renderer/components/ui/PaneTilingLayout.tsxapps/desktop/src/renderer/lib/resourcePressure.tsapps/desktop/src/renderer/lib/sessionListCache.test.tsapps/desktop/src/renderer/lib/sessionListCache.tsapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/types/core.tsapps/desktop/src/shared/types/sessions.ts
👮 Files not reviewed due to content moderation or server errors (1)
- apps/desktop/src/main/services/chat/agentChatService.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What changed
Validation
Risks
Summary by CodeRabbit
New Features
Improvements
Greptile Summary
This PR reduces background CPU and IPC load across the Lanes and Work surfaces by gating expensive operations (stack-graph roster hydration, hidden-pane mounting, minimized git-action panes, PR tag refreshes, session-list invalidation) on visibility and activity. It also adds app/PTY resource-usage snapshots piped from the main process to the renderer for adaptive terminal-grid refresh rate throttling.
appGetResourceUsageIPC handler samples Electron app metrics + PTY process trees via a sharedpscall (900 ms cache);sessionService.list()gains atoolTypesmulti-query filter used byagentChatServiceto avoid loading non-chat sessions.resourcePressure.tscoalesces IPC fetches and converts signals (CPU %, heap ratio, event-loop drift) into a 0–4 pressure level;WorkViewAreauses FNV-1a bucket rotation to stagger inactive grid-terminal refreshes under load;TopBarshows aResourcePressureIndicatorat level > 0.useLaneWorkSessionsanduseWorkSessionsdefer session-list refreshes while the document is hidden and flush on visibility restore;LanesPagesplits lane-runtime events into fast lifecycle vs. slow data buckets and schedules per-PR opportunistic refreshes for visible lanes.Confidence Score: 5/5
The change is safe to merge; the one code-quality note is a defensive fix for a silently-failing native method call.
All new gating logic uses conservative visibility and activity guards, the adaptive throttling degrades gracefully back to full-speed for active sessions, and the scoped cache-invalidation over-invalidates rather than under-invalidates. The only concrete defect is in getSystemMemoryMB: the Electron native method is detached from its receiver before calling, which will throw on Electron builds that enforce this binding — but the try/catch silently returns null, so memory pressure readings just go missing rather than crashing. The rest of the diff is well-structured and thoroughly tested.
apps/desktop/src/main/services/ipc/registerIpc.ts — the getSystemMemoryMB unbound call.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Renderer participant Preload participant MainIPC as Main IPC (registerIpc) participant PtyService participant AppMetrics as Electron app.getAppMetrics() participant OS as process.getSystemMemoryInfo() Renderer->>Preload: window.ade.app.getResourceUsage() Preload->>MainIPC: IPC appGetResourceUsage Note over MainIPC: Cache hit? (< 900 ms) MainIPC->>AppMetrics: app.getAppMetrics() MainIPC->>PtyService: getResourceUsageSnapshot(sharedReadRows) PtyService->>OS: spawnSync("ps") via readProcessMetricRows (once, shared) PtyService-->>MainIPC: PtyProcessResourceUsageSnapshot MainIPC->>OS: getSystemMemoryInfo() (unbound — see comment) MainIPC-->>Preload: AppResourceUsageSnapshot (cached) Preload-->>Renderer: snapshot Renderer->>Renderer: pressureLevelForSignals(driftMs, heapRatio, usage) Renderer->>Renderer: gridTerminalRefreshPolicyForPressure(level) Renderer->>Renderer: shouldStreamGridTerminal(sessionId, bucket, pulse)Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "test(sessions): align refresh cache expe..." | Re-trigger Greptile