microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
160.11k stars 28.04k forks source link

Inline chat: response associated with the wrong widget? #212785

Open bpasero opened 1 month ago

bpasero commented 1 month ago

This maybe related to https://github.com/microsoft/vscode/issues/212780.

For the support for text-to-speech from a response, I rely on IChatWidgetService.getWidgetBySessionId() to be able to control the lifecycle of the session in the context of the chat. This works fine in most chat locations, but not in inline and I wonder if its related to inline having 2 modes ("lightweight" and "heavy"). I get back a widget that is not the focused chat widget in the following scenario:

Steps:

Notice how the widget returned from getWidgetBySessionId is not the same as lastFocusedWidget. I think the widget associated with the session ID is still the "lightweight" variant?

image

bpasero commented 1 month ago

I have a workaround btw:

https://github.com/microsoft/vscode/blob/8693728cc3a010b65ed07a0f74121e88276b6bf2/src/vs/workbench/contrib/chat/electron-sandbox/actions/voiceChatActions.ts#L686-L691

jrieken commented 1 month ago

I rely on IChatWidgetService.getWidgetBySessionId() to be able to

Don't do that. It is realistic that one model is attached to multiple widgets - that's for inline chat but also for chat editors

bpasero commented 1 month ago

I see. Then I will need another way of finding the chat widget for a response. I only have a ChatResponseViewModel in hand:

https://github.com/microsoft/vscode/blob/ffb2581f0866e2ec33d2853e593955e3e8535796/src/vs/workbench/contrib/chat/electron-sandbox/actions/voiceChatActions.ts#L841-L847

jrieken commented 1 month ago

I believe chatWidgetService.lastFocusedWidget is the best way to achieve that. Using the response argument is already fishy because it won't be set when users invoke your command from a keybinding.

bpasero commented 1 month ago

Yeah I will go with that 👍