Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
0eac8bf
initial on-the-fly docs implementation for vscode
spebl Aug 5, 2024
93d7bb0
merge from main
spebl Aug 5, 2024
2667ec5
add localization support
spebl Aug 6, 2024
fb310c8
add support for icons in hover
spebl Aug 7, 2024
433f22d
add support for feature flag control
spebl Aug 7, 2024
7ec62d9
add setting and strings
spebl Aug 7, 2024
7a41841
fix settings when copilot models load later and fix showing copilot w…
spebl Aug 8, 2024
db42258
'otf docs' -> 'copilot hover' in naming conventions and strings. fixe…
spebl Aug 14, 2024
c73a589
fix spacing
spebl Aug 14, 2024
aa6779c
merge from main
spebl Aug 14, 2024
28390f1
reset vscode version
spebl Aug 14, 2024
3d5b7db
merge from main
spebl Aug 26, 2024
bd1eb43
merge from main
spebl Sep 3, 2024
d9c74c3
merge from main
spebl Sep 10, 2024
c31c4ab
merge from main
spebl Sep 12, 2024
9742465
fix formatting
spebl Sep 24, 2024
3bb4f1f
merge from main
spebl Sep 24, 2024
1cf0785
merge from main
spebl Sep 25, 2024
6076ca9
swap to using new copilot hover provider as to not interfere with exi…
spebl Sep 28, 2024
fd7095d
Add RAI disclaimers
benmcmorran Oct 9, 2024
9e1a547
merge from vscode
spebl Oct 10, 2024
d6a340f
merge from upstream
spebl Oct 10, 2024
f8cdde6
Copilot summary should now only show up when there is already hover c…
spebl Oct 11, 2024
713f430
Merge branch 'main' into dev/spebl/otfdocs
spebl Oct 11, 2024
10fb3cf
move AI inaccuracy warning to hover tooltip, add sparkle to load, che…
spebl Oct 11, 2024
3a0b8d8
prompt user to reload workspace when changing copilot hover option
spebl Oct 11, 2024
1be1fcc
merge from upstream
spebl Oct 11, 2024
ceb9f8e
merge from main
spebl Oct 18, 2024
9376135
merge from main
spebl Oct 21, 2024
ba5f149
merge from main
spebl Oct 22, 2024
8707994
Merge branch 'main' into dev/spebl/otfdocs
spebl Oct 22, 2024
1b37fd1
track and pass cancellation token + fix copilot hover showing up befo…
spebl Oct 24, 2024
f8fc500
Merge branch 'dev/spebl/otfdocs' of https://github.com/Microsoft/vsco…
spebl Oct 24, 2024
223d52b
better handling of intentional cancellation by user
spebl Oct 28, 2024
c055ce2
ensure tokens and cancellation are all in sync between copilot hover …
spebl Oct 28, 2024
3af533d
Merge branch 'main' into dev/spebl/otfdocs
spebl Oct 29, 2024
ad991c6
Merge branch 'main' into dev/spebl/otfdocs
spebl Oct 30, 2024
76000c7
swap to use same params for copilot hover info as text document posit…
spebl Nov 13, 2024
0fc52ed
Merge branch 'main' of https://github.com/Microsoft/vscode-cpptools i…
spebl Nov 13, 2024
1788813
Merge branch 'dev/spebl/otfdocs' of https://github.com/Microsoft/vsco…
spebl Nov 13, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3313,6 +3313,17 @@
"default": false,
"markdownDescription": "%c_cpp.configuration.addNodeAddonIncludePaths.markdownDescription%",
"scope": "application"
},
"C_Cpp.copilotHover": {
sean-mcmanus marked this conversation as resolved.
Show resolved Hide resolved
"type": "string",
"enum": [
"default",
"enabled",
"disabled"
],
"default": "default",
"markdownDescription": "%c_cpp.configuration.copilotHover.markdownDescription%",
"scope": "window"
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions Extension/package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,12 @@
"Markdown text between `` should not be translated or localized (they represent literal text) and the capitalization, spacing, and punctuation (including the ``) should not be altered."
]
},
"c_cpp.configuration.copilotHover.markdownDescription": {
"message": "If `enabled`, the hover tooltip will display an option to generate a summary of the symbol with Copilot. If `disabled`, the option will not be displayed.",
"comment": [
"Markdown text between `` should not be translated or localized (they represent literal text) and the capitalization, spacing, and punctuation (including the ``) should not be altered."
]
},
"c_cpp.configuration.renameRequiresIdentifier.markdownDescription": {
"message": "If `true`, 'Rename Symbol' will require a valid C/C++ identifier.",
"comment": [
Expand Down
129 changes: 129 additions & 0 deletions Extension/src/LanguageServer/Providers/CopilotHoverProvider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/* --------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All Rights Reserved.
* See 'LICENSE' in the project root for license information.
* ------------------------------------------------------------------------------------------ */
import * as vscode from 'vscode';
import { Position, ResponseError } from 'vscode-languageclient';
import * as nls from 'vscode-nls';
import { DefaultClient, GetCopilotHoverInfoParams, GetCopilotHoverInfoRequest } from '../client';
import { RequestCancelled, ServerCancelled } from '../protocolFilter';
import { CppSettings } from '../settings';

nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })();
const localize: nls.LocalizeFunc = nls.loadMessageBundle();

export class CopilotHoverProvider implements vscode.HoverProvider {
private client: DefaultClient;
private currentDocument: vscode.TextDocument | undefined;
private currentPosition: vscode.Position | undefined;
private waiting: boolean = false;
private ready: boolean = false;
private cancelled: boolean = false;
private cancelledDocument: vscode.TextDocument | undefined;
private cancelledPosition: vscode.Position | undefined;
private content: string | undefined;
constructor(client: DefaultClient) {
this.client = client;
}

public async provideHover(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken): Promise<vscode.Hover | undefined> {
await this.client.ready;
const settings: CppSettings = new CppSettings(vscode.workspace.getWorkspaceFolder(document.uri)?.uri);
if (settings.hover === "disabled") {
return undefined;
}

if (!this.isNewHover(document, position)) {
if (this.ready) {
const contentMarkdown = new vscode.MarkdownString(`$(sparkle) Copilot\n\n${this.content}`, true);
return new vscode.Hover(contentMarkdown);
}
if (this.waiting) {
const loadingMarkdown = new vscode.MarkdownString("$(sparkle) $(loading~spin)", true);
return new vscode.Hover(loadingMarkdown);
}
}

// Fresh hover, reset state.
this.reset();
// Wait for the main hover provider to finish and confirm it has content.
const hoverProvider = this.client.getHoverProvider();
if (!await hoverProvider?.contentReady) {
return undefined;
}
if (token.isCancellationRequested) {
throw new vscode.CancellationError();
}
this.currentDocument = document;
this.currentPosition = position;
const commandString = "$(sparkle) [" + localize("generate.copilot.description", "Generate Copilot summary") + "](command:C_Cpp.ShowCopilotHover \"" + localize("copilot.disclaimer", "AI-generated content may be incorrect.") + "\")";
const commandMarkdown = new vscode.MarkdownString(commandString);
commandMarkdown.supportThemeIcons = true;
commandMarkdown.isTrusted = true;
return new vscode.Hover(commandMarkdown);
}

public showWaiting(): void {
this.waiting = true;
}

public showContent(content: string): void {
this.ready = true;
this.content = content;
}

public getCurrentHoverDocument(): vscode.TextDocument | undefined {
return this.currentDocument;
}

public getCurrentHoverPosition(): vscode.Position | undefined {
return this.currentPosition;
}

public async getRequestInfo(document: vscode.TextDocument, position: vscode.Position): Promise<string> {
let requestInfo = "";
const params: GetCopilotHoverInfoParams = {
uri: document.uri.toString(),
position: Position.create(position.line, position.character)
};
await this.client.ready;
try {
const response = await this.client.languageClient.sendRequest(GetCopilotHoverInfoRequest, params);
requestInfo = response.content;
} catch (e: any) {
if (e instanceof ResponseError && (e.code === RequestCancelled || e.code === ServerCancelled)) {
throw new vscode.CancellationError();
}
throw e;
}

return requestInfo;
}

public isCancelled(document: vscode.TextDocument, position: vscode.Position): boolean {
if (this.cancelled && this.cancelledDocument === document && this.cancelledPosition === position) {
// Cancellation is being acknowledged.
this.cancelled = false;
this.cancelledDocument = undefined;
this.cancelledPosition = undefined;
return true;
}
return false;
}

private reset(): void {
// If there was a previous call, cancel it.
if (this.waiting) {
this.cancelled = true;
this.cancelledDocument = this.currentDocument;
this.cancelledPosition = this.currentPosition;
}
this.waiting = false;
this.ready = false;
this.content = undefined;
}

private isNewHover(document: vscode.TextDocument, position: vscode.Position): boolean {
return !(this.currentDocument === document && this.currentPosition?.line === position.line && (this.currentPosition?.character === position.character || this.currentPosition?.character === position.character - 1));
}
}
8 changes: 8 additions & 0 deletions Extension/src/LanguageServer/Providers/HoverProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@
* ------------------------------------------------------------------------------------------ */
import * as vscode from 'vscode';
import { Position, ResponseError, TextDocumentPositionParams } from 'vscode-languageclient';
import { ManualSignal } from '../../Utility/Async/manualSignal';
import { DefaultClient, HoverRequest } from '../client';
import { RequestCancelled, ServerCancelled } from '../protocolFilter';
import { CppSettings } from '../settings';

export class HoverProvider implements vscode.HoverProvider {
private client: DefaultClient;
private readonly hasContent = new ManualSignal<boolean>(true);
constructor(client: DefaultClient) {
this.client = client;
}

public async provideHover(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken): Promise<vscode.Hover | undefined> {
this.hasContent.reset();
const settings: CppSettings = new CppSettings(vscode.workspace.getWorkspaceFolder(document.uri)?.uri);
if (settings.hover === "disabled") {
return undefined;
Expand Down Expand Up @@ -52,6 +55,11 @@ export class HoverProvider implements vscode.HoverProvider {
hoverResult.range.end.line, hoverResult.range.end.character);
}

this.hasContent.resolve(strings.length > 0);
return new vscode.Hover(strings, range);
}

get contentReady(): Promise<boolean> {
return this.hasContent;
}
}
51 changes: 49 additions & 2 deletions Extension/src/LanguageServer/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { localizedStringCount, lookupString } from '../nativeStrings';
import { SessionState } from '../sessionState';
import * as telemetry from '../telemetry';
import { TestHook, getTestHook } from '../testHook';
import { CopilotHoverProvider } from './Providers/CopilotHoverProvider';
import { HoverProvider } from './Providers/HoverProvider';
import {
CodeAnalysisDiagnosticIdentifiersAndUri,
Expand Down Expand Up @@ -533,6 +534,15 @@ export interface GetIncludesResult {
includedFiles: string[];
}

export interface GetCopilotHoverInfoParams {
uri: string;
position: Position;
}

interface GetCopilotHoverInfoResult {
content: string;
}

export interface ChatContextResult {
language: string;
standardVersion: string;
Expand All @@ -554,6 +564,7 @@ export const FormatDocumentRequest: RequestType<FormatParams, FormatResult, void
export const FormatRangeRequest: RequestType<FormatParams, FormatResult, void> = new RequestType<FormatParams, FormatResult, void>('cpptools/formatRange');
export const FormatOnTypeRequest: RequestType<FormatParams, FormatResult, void> = new RequestType<FormatParams, FormatResult, void>('cpptools/formatOnType');
export const HoverRequest: RequestType<TextDocumentPositionParams, vscode.Hover, void> = new RequestType<TextDocumentPositionParams, vscode.Hover, void>('cpptools/hover');
export const GetCopilotHoverInfoRequest: RequestType<GetCopilotHoverInfoParams, GetCopilotHoverInfoResult, void> = new RequestType<GetCopilotHoverInfoParams, GetCopilotHoverInfoResult, void>('cpptools/getCopilotHoverInfo');
const CreateDeclarationOrDefinitionRequest: RequestType<CreateDeclarationOrDefinitionParams, CreateDeclarationOrDefinitionResult, void> = new RequestType<CreateDeclarationOrDefinitionParams, CreateDeclarationOrDefinitionResult, void>('cpptools/createDeclDef');
const ExtractToFunctionRequest: RequestType<ExtractToFunctionParams, WorkspaceEditResult, void> = new RequestType<ExtractToFunctionParams, WorkspaceEditResult, void>('cpptools/extractToFunction');
const GoToDirectiveInGroupRequest: RequestType<GoToDirectiveInGroupParams, Position | undefined, void> = new RequestType<GoToDirectiveInGroupParams, Position | undefined, void>('cpptools/goToDirectiveInGroup');
Expand Down Expand Up @@ -790,6 +801,7 @@ export interface Client {
getShowConfigureIntelliSenseButton(): boolean;
setShowConfigureIntelliSenseButton(show: boolean): void;
addTrustedCompiler(path: string): Promise<void>;
getCopilotHoverProvider(): CopilotHoverProvider | undefined;
getIncludes(maxDepth: number, token: vscode.CancellationToken): Promise<GetIncludesResult>;
getChatContext(token: vscode.CancellationToken): Promise<ChatContextResult>;
}
Expand Down Expand Up @@ -824,11 +836,14 @@ export class DefaultClient implements Client {
private settingsTracker: SettingsTracker;
private loggingLevel: number = 1;
private configurationProvider?: string;
private hoverProvider: HoverProvider | undefined;
private copilotHoverProvider: CopilotHoverProvider | undefined;

public lastCustomBrowseConfiguration: PersistentFolderState<WorkspaceBrowseConfiguration | undefined> | undefined;
public lastCustomBrowseConfigurationProviderId: PersistentFolderState<string | undefined> | undefined;
public lastCustomBrowseConfigurationProviderVersion: PersistentFolderState<Version> | undefined;
public currentCaseSensitiveFileSupport: PersistentWorkspaceState<boolean> | undefined;
public currentCopilotHoverEnabled: PersistentWorkspaceState<string> | undefined;
private registeredProviders: PersistentFolderState<string[]> | undefined;

private configStateReceived: ConfigStateReceived = { compilers: false, compileCommands: false, configProviders: undefined, timeout: false };
Expand Down Expand Up @@ -1257,8 +1272,16 @@ export class DefaultClient implements Client {
this.registerFileWatcher();
initializedClientCount = 0;
this.inlayHintsProvider = new InlayHintsProvider();
this.hoverProvider = new HoverProvider(this);

this.disposables.push(vscode.languages.registerHoverProvider(util.documentSelector, new HoverProvider(this)));
const settings: CppSettings = new CppSettings();
this.currentCopilotHoverEnabled = new PersistentWorkspaceState<string>("cpp.copilotHover", settings.copilotHover);
if (settings.copilotHover === "enabled" ||
(settings.copilotHover === "default" && await telemetry.isFlightEnabled("cpp.copilotHover"))) {
this.copilotHoverProvider = new CopilotHoverProvider(this);
this.disposables.push(vscode.languages.registerHoverProvider(util.documentSelector, this.copilotHoverProvider));
}
this.disposables.push(vscode.languages.registerHoverProvider(util.documentSelector, this.hoverProvider));
this.disposables.push(vscode.languages.registerInlayHintsProvider(util.documentSelector, this.inlayHintsProvider));
this.disposables.push(vscode.languages.registerRenameProvider(util.documentSelector, new RenameProvider(this)));
this.disposables.push(vscode.languages.registerReferenceProvider(util.documentSelector, new FindAllReferencesProvider(this)));
Expand All @@ -1276,7 +1299,6 @@ export class DefaultClient implements Client {
this.codeFoldingProvider = new FoldingRangeProvider(this);
this.codeFoldingProviderDisposable = vscode.languages.registerFoldingRangeProvider(util.documentSelector, this.codeFoldingProvider);

const settings: CppSettings = new CppSettings();
if (settings.isEnhancedColorizationEnabled && semanticTokensLegend) {
this.semanticTokensProvider = new SemanticTokensProvider();
this.semanticTokensProviderDisposable = vscode.languages.registerDocumentSemanticTokensProvider(util.documentSelector, this.semanticTokensProvider, semanticTokensLegend);
Expand Down Expand Up @@ -1451,6 +1473,9 @@ export class DefaultClient implements Client {
if (this.currentCaseSensitiveFileSupport && workspaceSettings.isCaseSensitiveFileSupportEnabled !== this.currentCaseSensitiveFileSupport.Value) {
void util.promptForReloadWindowDueToSettingsChange();
}
if (this.currentCopilotHoverEnabled && workspaceSettings.copilotHover !== this.currentCopilotHoverEnabled.Value) {
void util.promptForReloadWindowDueToSettingsChange();
}
return {
filesAssociations: workspaceOtherSettings.filesAssociations,
workspaceFallbackEncoding: workspaceOtherSettings.filesEncoding,
Expand All @@ -1473,6 +1498,7 @@ export class DefaultClient implements Client {
codeAnalysisMaxConcurrentThreads: workspaceSettings.codeAnalysisMaxConcurrentThreads,
codeAnalysisMaxMemory: workspaceSettings.codeAnalysisMaxMemory,
codeAnalysisUpdateDelay: workspaceSettings.codeAnalysisUpdateDelay,
copilotHover: workspaceSettings.copilotHover,
workspaceFolderSettings: workspaceFolderSettingsParams
};
}
Expand Down Expand Up @@ -1583,6 +1609,12 @@ export class DefaultClient implements Client {
// We manually restart the language server so tell the LanguageClient not to do it automatically for us.
return { action: CloseAction.DoNotRestart, message };
}
},
markdown: {
isTrusted: true
// TODO: support for icons in markdown is not yet in the released version of vscode-languageclient.
// Based on PR (https://github.com/microsoft/vscode-languageserver-node/pull/1504)
//supportThemeIcons: true
}

// TODO: should I set the output channel? Does this sort output between servers?
Expand Down Expand Up @@ -4009,6 +4041,19 @@ export class DefaultClient implements Client {
compilerDefaults = await this.requestCompiler(path);
DebugConfigurationProvider.ClearDetectedBuildTasks();
}

public getHoverProvider(): HoverProvider | undefined {
return this.hoverProvider;
}

public getCopilotHoverProvider(): CopilotHoverProvider | undefined {
return this.copilotHoverProvider;
}

public async getCopilotHoverInfo(): Promise<GetCopilotHoverInfoResult> {
await this.ready;
return this.languageClient.sendRequest(GetCopilotHoverInfoRequest, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

}
}

function getLanguageServerFileName(): string {
Expand Down Expand Up @@ -4120,6 +4165,8 @@ class NullClient implements Client {
getShowConfigureIntelliSenseButton(): boolean { return false; }
setShowConfigureIntelliSenseButton(show: boolean): void { }
addTrustedCompiler(path: string): Promise<void> { return Promise.resolve(); }
getCopilotHoverProvider(): CopilotHoverProvider | undefined { return undefined; }
getCopilotHoverInfo(): Promise<GetCopilotHoverInfoResult> { return Promise.resolve({} as GetCopilotHoverInfoResult); }
getIncludes(maxDepth: number, token: vscode.CancellationToken): Promise<GetIncludesResult> { return Promise.resolve({} as GetIncludesResult); }
getChatContext(token: vscode.CancellationToken): Promise<ChatContextResult> { return Promise.resolve({} as ChatContextResult); }
}
Loading
Loading