Support for copilot-generated summaries in quick info. (On-the-fly docs)

[spebl]

This introduces a new setting C_Cpp.onTheFlyDocsEnabled to control the display of the option to show copilot-generated summaries in the hover tooltip.
When enabled, and also authenticated with the vscode-copilot extension, the hover tooltip will display an option to generate a summary of the symbol with copilot.
The setting is defaulted to default which will check the feature flag control to determine if the feature should be enabled, which allows for slow rollout and the ability to rollback should any issues arise.

Updating the vscode requirement to 1.90.0 to support using copilot features with "vscode.lm".

The IntelliSense client changes supporting this feature are included in a separate PR against that repository.

Some workarounds were needed to support icon rendering in hover markdown and updating hover content dynamically which currently requires the hover to be closed and reopened. Potential fixes from proposals and unreleased patches are linked where applicable.

All feedback is very welcome!

[benmcmorran]

Left a few wording comments but mostly looks good to me ✨. Leaving official sign-off for cpptools owners.

[benmcmorran]

Not sure what standard practice is in cpptools, but should we be sending some error telemetry here too?

[sean-mcmanus]

@spebl Did you want this in our next 1.22.x or what release?

[spebl]

@spebl Did you want this in our next 1.22.x or what release?

Not currently targeting a specific release, no need to block anything on this going in. I first wanted to get feedback on this approach, along with the upgrading of the vscode version. I'm taking a look now at how we can best keep support for the older versions while also using the new language model apis when available.

[Colengms]

It's common for multiple hover requests to be issued and get backlogged as the user moves the mouse around. As each one is generated, the prior one is cancelled. I think the cancellation token needs to be plumbed up here, all the way to these calls to sendRequest, in order for the lsp_manager on the native side to handle cancellation.

Cancellation will now cause an exception to be thrown from sendRequest, which you could catch, check for the specific cancellation error code, and handle appropriately (i.e., to throw a CancellationError exception, if that is what calling code expects).

[spebl]

I'm hoping that since we're no longer going back and forth from the lsp server, this shouldn't be an issue. I've added exception catching around the one call this still does make to the native side. Let me know if there is any additional cancellation I should be tracking still.

[Colengms]

It looks like this sendRequest was moved to getRequestInfo and doesn't have a cancellation token passed to it. I think you'll want to capture the cancellation token from the original(?) hover request (or, maybe the subsequent hover request caused by showCopilotContent?), and pass that to sendRequest to communicate that cancellation to the native process. Even if you're not explicitly checking for cancellation (!request.is_active()) in your LSP message handler, this would allow the LSP manager itself to discard requests in queue that have already been cancelled and to discard the response for an operation already cancelled.

[benmcmorran]

Capturing notes from a quick discussion I had with @Colengms. He's going to go ahead and move from LSP-based hover to a HoverProvider which should make it easier for us to opt-in to icons and avoid async work during settings initialization. We also discussed a potential alternative using multiple HoverProviders where a provider that actually displays Copilot results simply blocks until the user clicks the Copilot link from the first provider. FYI @spebl

[sean-mcmanus]

I think the putting Copilot text on a separate line is excessive. The user already saw "Copilot" text when they clicked the link.

I think the symbol is sufficient -- and on the same line:

versus:

.

[Colengms]

Similar feedback to the other call to sendRequest. Assuming this happens in the context of a hover operation, could you pass the associated cancellation token? For some of our calls to sendRequest that are triggered by commands, we don't get a token from VS Code, so don't pass it along. But we should pass along cancellation tokens for any user operation that has one associated with it.

[spebl]

We don't directly have the token at call time, but we're relying on other stashed hover values so I've added similar logic for the cancellation token. Also, thanks for the heads up on the multiple send request calls added, this one isn't used anymore so I was able to remove it.

[Colengms]

Could you clarify? I would expect a hover-related operation to be conceptually cancelable/dismissible by the user. (It looks like you currently have a bug where, if a user has moved on to another task, a slow CopilotHover request can result in the cursor being repositioned, potentially even when the user is mid-typing?).

I think it's important that this feature handle cancellation. The cpptools_getCopilotHoverInfo handler on the native side will acquire a (potentially new) TU, which may impose a hefty perf impact. Even if the TU were already existing, if racing with edit, it may trigger a preparse. There could be other messages in queue, delaying delivery of yours, or piling up after yours.

For a lot of users, IntelliSense is very slow. I have a project at home in which a TU can take more than a minute to perform a preparse. If racing with edits, we don't want to waste time fully completing an operation whose result won't be used. As soon as there are subsequent edits in queue, the preparse it performs potentially be entirely wasted.

[spebl]

Sorry, I think my response was extra confusing since this code location doesn't make sense anymore (was removed).

For the other case where we're still calling to the native layer, we're not in the Copilot Hover's call stack directly, so we have to store the last hover cancellation token to use it later when called through the "onCopilotHover" command.

It looks like there's a bug with which token I was holding, so I'm fixing that now to make sure we have the most up-to-date hover cancellation token when checking it and passing it through to the native layer.

I agree that cancellation is important, trying to handle it from the command callback function has been a little more complicated since we're currently invoking multiple follow up hovers. Hopefully we can remove that need in a future update with better platform support, which would really help streamline the cancellation.

[Colengms]

In my last response, I was specifically looking at calls to our languageClient.sendRequest, trying to ensure they all had tokens passed to them. It looks like one of your last pushes addressed that. I may have been looking at stale code.

One possible issue: In onCopilotHover, it looks like you're passing in a token from a new (not otherwise referenced) token source:

        chatResponse = await model.sendRequest(
            messages,
            {},
            new vscode.CancellationTokenSource().token
        );

I'm guessing maybe you had to pass a token (undefined wasn't accepted?), and didn't have a valid one?

trying to handle it from the command callback function has been a little more complicated since we're currently invoking multiple follow up hovers

Are those follow-ups all in the context of the same hover operation? If so, could the original hover operation's token be used for all of them?

Normally, I'd suggest passing an argument when programmatically invoking the command, but it looks like you're embedding the command as text into the hover text. Note that you can still pass an encoded argument to the command using syntax like so:

[Find my new Compiler](command:C_Cpp.RescanCompilers?%22walkthrough%22)",

Follow the call into onRescanCompilers for an example of how to extract it. I don't think you'd be able to pass through an object reference, but you could pass through a key that you could associate a value with in a map.

[spebl]

Thanks! I'll push out my updated changes soon that seems to handle the cancellation much better by holding on to the hover's cancellation token and tracking which operations are changing the editor's focus more intentionally.