inlineChat: bypass zone widget in hover mode, use direct sendRequest#308726
Merged
inlineChat: bypass zone widget in hover mode, use direct sendRequest#308726
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes inline chat hover mode by avoiding lazy instantiation/configuration of InlineChatZoneWidget and instead submitting requests directly via IChatService.sendRequest() against the session’s ChatModel, keeping the existing zone mode flow intact.
Changes:
- Split
run()into_runHover()(directsendRequest) and_runZone()(existing widget-based flow). - Added hover-mode helpers
_resolveModelId()and_buildLocationData()to replicate widget-derived request options. - Added a new browser test suite to validate hover-mode
sendRequestoption parity (location, locationData, modelId, modeInfo).
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/inlineChat/browser/inlineChatController.ts | Adds hover-mode direct request submission path and supporting model/location resolution helpers; guards autoruns to avoid forcing zone widget instantiation. |
| src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts | New tests asserting hover-mode sendRequest parameters match expected parity behaviors. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 3
src/vs/workbench/contrib/inlineChat/test/browser/inlineChatController.test.ts
Outdated
Show resolved
Hide resolved
lramos15
approved these changes
Apr 9, 2026
joshspicer
pushed a commit
that referenced
this pull request
Apr 9, 2026
…308726) * inline chat: bypass zone widget in hover mode, use direct sendRequest * address review: notebook config gate, faked timers in tests
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
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
In hover mode, the
InlineChatZoneWidgetwas being lazily instantiated, fully configured (model, attachments, input text), used to submit requests viachatWidget.acceptInput(), then immediately hidden. This PR eliminates that intermediary by callingIChatService.sendRequest()directly on the session'sChatModel, so the zone widget is never instantiated in hover mode.Session Context
Key decisions from the development session:
sendRequestoveraddRequest: Uses the high-levelIChatService.sendRequest()which handles parsing, instruction collection, and agent invocation - not the low-levelChatModel.addRequest()._runZone()codepath is the originalrun()logic, completely unchanged (exceptattachDiagnosticsdefaulting totruesince it's always zone mode)._resolveModelId()useslookupLanguageModelByQualifiedName: Same 3-tier priority as_applyModelDefaults(user session choice >inlineChat.defaultModelsetting > vendor default for location), but queriesILanguageModelsServicedirectly instead of going throughchatWidget.input.switchModelByQualifiedName()._buildLocationData()replicates zone widget'sresolveData: Produces identicalIChatLocationDatashape, handling both regular editors and notebook cells.this._zone.valuein hover mode (which would force instantiation). The diff-adjustment autorun usesrawValue?.so it only acts when the zone already exists.InlineChatRunOptionsAPI unchanged: External callers (extensions, actions) don't need changes.as unknown as Tused in test mocks:Partial<T> as Twas insufficient for deeply-nested mock objects;unknownintermediary was chosen overanyto satisfy lint rules.Changes
run()split into_runHover()and_runZone()- dispatches based on_renderMode_resolveModelId(location)- resolves language model ID without zone widget_buildLocationData()- buildsIChatLocationDatafrom editor state without zone widget_zone.value->_zone.rawValue?.where appropriate for hover modeinlineChatController.test.ts- verifies sendRequest options (location, locationData, modelId, modeInfo) match expected values in hover mode