feat(copilotcli): Perf impromvent by lazy loading chat session items#311817
feat(copilotcli): Perf impromvent by lazy loading chat session items#311817DonJayamanne merged 4 commits intomainfrom
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR introduces lazy resolution of chat session item details so expensive fields (notably git diff–derived changes) are only computed when a session item becomes visible, reducing startup/UI stalls when many sessions exist.
Changes:
- Adds a proposed API hook to lazily resolve
ChatSessionItemdetails (resolveChatSessionItem) and wires it through ext host ↔ main thread plumbing. - Updates the Agents/Sessions UI to trigger lazy resolution when rendering visible items.
- Implements lazy-loading in Copilot CLI session providers, guarded by a new advanced setting, and adds a lightweight “has cached changes” check in the worktree service.
Show a summary per file
| File | Description |
|---|---|
| src/vscode-dts/vscode.proposed.chatSessionsProvider.d.ts | Proposed API: adds resolveChatSessionItem hook (provider + controller) and bumps proposal version. |
| src/vs/workbench/contrib/chat/test/common/mockChatSessionsService.ts | Updates mock service to implement the new resolveSessionItem API. |
| src/vs/workbench/contrib/chat/common/chatSessionsService.ts | Extends IChatSessionsService / controller contracts to support item resolution. |
| src/vs/workbench/contrib/chat/browser/chatSessions/chatSessions.contribution.ts | Implements resolveSessionItem in the workbench chat sessions service. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsViewer.ts | Triggers lazy resolve when items render in the session list UI. |
| src/vs/workbench/api/test/browser/mainThreadChatSessions.test.ts | Adjusts controller registration to the new signature. |
| src/vs/workbench/api/common/extHostChatSessions.ts | Adds $resolveChatSessionItem and bridges deprecated provider resolve hook via controller surface. |
| src/vs/workbench/api/common/extHost.protocol.ts | Ext host protocol changes: supportsResolve param + resolve RPC. |
| src/vs/workbench/api/browser/mainThreadChatSessions.ts | Main thread controller: adds resolve RPC invocation + per-resource resolve cache. |
| extensions/copilot/src/platform/configuration/common/configurationService.ts | Adds advanced setting key for lazy-loading session items. |
| extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts | Defers building changes eagerly; adds resolve hook to fill in changes lazily. |
| extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts | Same lazy-change strategy for controller-based CLI sessions. |
| extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionWorktreeServiceImpl.ts | Adds hasWorktreeChanges to detect cached changes quickly. |
| extensions/copilot/src/extension/chatSessions/common/chatSessionWorktreeService.ts | Extends interface with hasWorktreeChanges. |
| extensions/copilot/package.nls.json | Adds localized description for the new lazy-load setting. |
| extensions/copilot/package.json | Contributes the new advanced setting to VS Code configuration. |
Copilot's findings
- Files reviewed: 15/16 changed files
- Comments generated: 3
DonJayamanne
left a comment
There was a problem hiding this comment.
address the code review comments
DonJayamanne
left a comment
There was a problem hiding this comment.
@copilot address the review comentsm
Renamed the NLS key from |
52e59d7 to
8885c14
Compare
Co-authored-by: Copilot <copilot@github.com>
8885c14 to
1ec4f3d
Compare
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts:446
originalReffor workspace changes no longer falls back torepositoryProperties.baseCommitwhenmergeBaseCommitis unavailable. That can make diff URIs point atHEADand show incorrect/unstable comparisons. Consider restoring the previous fallback (mergeBaseCommit ?? baseCommit ?? 'HEAD') so workspace diffs remain anchored even when merge-base computation fails or hasn't been recorded yet.
const originalRef = repositoryProperties?.mergeBaseCommit ?? 'HEAD';
- Files reviewed: 8/8 changed files
- Comments generated: 4
…lotCLIChatSessions.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot copilot@github.com
Fixes #309466