Skip to content

Try to reduce how often LocalAgentsSessionsController fires updates#305525

Merged
mjbvz merged 2 commits intomicrosoft:mainfrom
mjbvz:dev/mjbvz/whispering-swift
Mar 27, 2026
Merged

Try to reduce how often LocalAgentsSessionsController fires updates#305525
mjbvz merged 2 commits intomicrosoft:mainfrom
mjbvz:dev/mjbvz/whispering-swift

Conversation

@mjbvz
Copy link
Copy Markdown
Collaborator

@mjbvz mjbvz commented Mar 27, 2026

LocalAgentsSessionsController is firing updates on every single response change. This PR tries to reduce this by doing a object equality check before firing the update. In a follow up I'll also see if we can debounce listening to so many request updates

`LocalAgentsSessionsController` is firing updates on every single response change. This PR tries to reduce this by doing a object equality check before firing the update. In a follow up I'll also see if we can debounce listening to so many request updates
Copilot AI review requested due to automatic review settings March 27, 2026 06:07
@vs-code-engineering
Copy link
Copy Markdown
Contributor

vs-code-engineering bot commented Mar 27, 2026

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@jrieken

Matched files:

  • src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingServiceImpl.ts

@bpasero
Copy link
Copy Markdown
Member

bpasero commented Mar 27, 2026

@mjbvz actual test failures

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to reduce churn in the Agent Sessions UI by making LocalAgentsSessionsController emit item deltas only when a session item’s computed shape actually changes, rather than on every response/model change.

Changes:

  • Reworked LocalAgentsSessionsController to track per-model changes and only fire addedOrUpdated when the computed session item differs.
  • Renamed IChatService.onDidDisposeSession event payload field from sessionResourcesessionResources across call sites.
  • Extracted a chatModelToChatDetail helper and reused it in live session item computation.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts Updates dispose-session event payload field name in tests.
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts Updates dispose-session cleanup loop to use sessionResources.
src/vs/workbench/contrib/terminalContrib/chat/browser/terminalChatService.ts Updates session-dispose cleanup loops to use sessionResources.
src/vs/workbench/contrib/chat/test/common/chatService/mockChatService.ts Updates mock dispose-session emitter payload to sessionResources.
src/vs/workbench/contrib/chat/test/common/chatService/chatService.test.ts Updates test iteration to use sessionResources.
src/vs/workbench/contrib/chat/common/widget/chatResponseResourceFileSystemProvider.ts Updates session-dispose cleanup to use sessionResources.
src/vs/workbench/contrib/chat/common/chatService/chatServiceImpl.ts Refactors live-session detail mapping and introduces exported chatModelToChatDetail.
src/vs/workbench/contrib/chat/common/chatService/chatService.ts Renames dispose-session event payload field to sessionResources.
src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingServiceImpl.ts Updates session-dispose handling to use sessionResources.
src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts Main change: per-model listeners + equality check to reduce update firing.
src/vs/workbench/api/browser/mainThreadChatAgents2.ts Updates session-dispose handling to use sessionResources.

Comment thread src/vs/workbench/contrib/chat/common/chatService/chatServiceImpl.ts Outdated
Copy link
Copy Markdown
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CCR

@mjbvz mjbvz enabled auto-merge March 27, 2026 16:22
@mjbvz mjbvz dismissed bpasero’s stale review March 27, 2026 16:51

fixed tests

@mjbvz mjbvz merged commit a712e00 into microsoft:main Mar 27, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants