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 Content Exclusion for Copilot Hover #13143

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 15 additions & 3 deletions Extension/.scripts/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const Git = async (...args: Parameters<Awaited<CommandFunction>>) => (awa
export const GitClean = async (...args: Parameters<Awaited<CommandFunction>>) => (await new Command(await git, 'clean'))(...args);

export async function getModifiedIgnoredFiles() {
const {code, error, stdio } = await GitClean('-Xd', '-n');
const { code, error, stdio } = await GitClean('-Xd', '-n');
if (code) {
throw new Error(`\n${error.all().join('\n')}`);
}
Expand All @@ -65,11 +65,11 @@ export async function rimraf(...paths: string[]) {
}
if (await filepath.isFolder(each)) {
verbose(`Removing folder ${red(each)}`);
all.push(rm(each, {recursive: true, force: true}));
all.push(rm(each, { recursive: true, force: true }));
continue;
}
verbose(`Removing file ${red(each)}`);
all.push(rm(each, {force: true}));
all.push(rm(each, { force: true }));
}
await Promise.all(all);
}
Expand Down Expand Up @@ -345,3 +345,15 @@ export async function checkBinaries() {
}
return failing;
}

export async function checkProposals() {
let failing = false;

await rm(`${$root}/vscode.proposed.chatParticipantAdditions.d.ts`);
failing = await assertAnyFile('vscode.proposed.chatParticipantAdditions.d.ts') && (quiet || warn(`The VSCode import file '${$root}/vscode.proposed.chatParticipantAdditions.d.ts' should not be present.`)) || failing;

if (!failing) {
verbose('VSCode proposals appear to be in place.');
}
return failing;
}
11 changes: 10 additions & 1 deletion Extension/.scripts/verify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* See 'LICENSE' in the project root for license information.
* ------------------------------------------------------------------------------------------ */

import { checkBinaries, checkCompiled, checkDTS, checkPrep, error, green } from './common';
import { checkBinaries, checkCompiled, checkDTS, checkPrep, checkProposals, error, green } from './common';
const quiet = process.argv.includes('--quiet');

export async function main() {
Expand Down Expand Up @@ -50,3 +50,12 @@ export async function dts() {
process.exit(1);
}
}

export async function proposals() {
let failing = false;
failing = (await checkProposals() && (quiet || error(`Issue with VSCode proposals. Run ${green('yarn prep')} to fix it.`))) || failing;

if (failing) {
process.exit(1);
}
}
5 changes: 3 additions & 2 deletions Extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
"Snippets"
],
"enabledApiProposals": [
"terminalDataWriteEvent"
"terminalDataWriteEvent",
"chatParticipantAdditions"
],
"capabilities": {
"untrustedWorkspaces": {
Expand Down Expand Up @@ -6537,7 +6538,7 @@
"translations-generate": "set NODE_OPTIONS=--no-experimental-fetch && gulp translations-generate",
"translations-import": "gulp translations-import",
"import-edge-strings": "ts-node -T ./.scripts/import_edge_strings.ts",
"prep:dts": "yarn verify dts --quiet || (npx @vscode/dts dev && npx @vscode/dts main)",
"prep:dts": "yarn verify dts --quiet || (npx @vscode/dts main && npx @vscode/dts dev && yarn verify proposals)",
"build": "yarn prep:dts && echo [Building TypeScript code] && tsc --build tsconfig.json"
},
"devDependencies": {
Expand Down
11 changes: 5 additions & 6 deletions Extension/src/LanguageServer/Providers/CopilotHoverProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import * as vscode from 'vscode';
import { Position, ResponseError } from 'vscode-languageclient';
import * as nls from 'vscode-nls';
import { DefaultClient, GetCopilotHoverInfoParams, GetCopilotHoverInfoRequest } from '../client';
import { DefaultClient, GetCopilotHoverInfoParams, GetCopilotHoverInfoRequest, GetCopilotHoverInfoResult } from '../client';
import { RequestCancelled, ServerCancelled } from '../protocolFilter';
import { CppSettings } from '../settings';

Expand Down Expand Up @@ -92,8 +92,8 @@ export class CopilotHoverProvider implements vscode.HoverProvider {
return this.currentCancellationToken;
}

public async getRequestInfo(document: vscode.TextDocument, position: vscode.Position): Promise<string> {
let requestInfo = "";
public async getRequestInfo(document: vscode.TextDocument, position: vscode.Position): Promise<GetCopilotHoverInfoResult> {
let response: GetCopilotHoverInfoResult;
const params: GetCopilotHoverInfoParams = {
textDocument: { uri: document.uri.toString() },
position: Position.create(position.line, position.character)
Expand All @@ -105,16 +105,15 @@ export class CopilotHoverProvider implements vscode.HoverProvider {
}

try {
const response = await this.client.languageClient.sendRequest(GetCopilotHoverInfoRequest, params, this.currentCancellationToken);
requestInfo = response.content;
response = await this.client.languageClient.sendRequest(GetCopilotHoverInfoRequest, params, this.currentCancellationToken);
} catch (e: any) {
if (e instanceof ResponseError && (e.code === RequestCancelled || e.code === ServerCancelled)) {
throw new vscode.CancellationError();
}
throw e;
}

return requestInfo;
return response;
}

public isCancelled(document: vscode.TextDocument, position: vscode.Position): boolean {
Expand Down
3 changes: 2 additions & 1 deletion Extension/src/LanguageServer/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,9 @@ export interface GetCopilotHoverInfoParams {
position: Position;
}

interface GetCopilotHoverInfoResult {
export interface GetCopilotHoverInfoResult {
content: string;
files: string[];
}

export interface ChatContextResult {
Expand Down
24 changes: 20 additions & 4 deletions Extension/src/LanguageServer/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as StreamZip from 'node-stream-zip';
import * as os from 'os';
import * as path from 'path';
import * as vscode from 'vscode';
import { Range } from 'vscode-languageclient';
import { CancellationToken, Range } from 'vscode-languageclient';
import * as nls from 'vscode-nls';
import { TargetPopulation } from 'vscode-tas-client';
import * as which from 'which';
Expand Down Expand Up @@ -1430,18 +1430,34 @@ async function onCopilotHover(): Promise<void> {

// Gather the content for the query from the client.
const requestInfo = await copilotHoverProvider.getRequestInfo(hoverDocument, hoverPosition);
if (requestInfo.length === 0) {
try {
for (const file of requestInfo.files) {
const fileUri = vscode.Uri.file(file);
if (await vscodelm.fileIsIgnored(fileUri, copilotHoverProvider.getCurrentHoverCancellationToken() ?? CancellationToken.None)) {
telemetry.logLanguageServerEvent("CopilotHover", { "Message": "Copilot summary is not available due to content exclusion." });
await showCopilotContent(copilotHoverProvider, hoverDocument, hoverPosition, localize("copilot.hover.unavailable", "Copilot summary is not available.") + "\n\n" +
localize("copilot.hover.excluded", "The file containing this symbol's definition or declaration has been excluded from use with Copilot."));
return;
}
}
} catch (err) {
if (err instanceof Error) {
await reportCopilotFailure(copilotHoverProvider, hoverDocument, hoverPosition, err.message);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this err.message contain PII, such as the user name and paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call that logging any generic error could have that issue. Perhaps something like "Copilot Hover Error: " + err.name? Should be safe from PII but it would be good to be able to track what type of errors are hit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I used err.name in my PR at #13158 .

Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't sure how to determine if vscode.LanguageModelError should be handled (which has code) versus Error (which only has name).

Copy link
Contributor

@sean-mcmanus sean-mcmanus Jan 16, 2025

Choose a reason for hiding this comment

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

The telemetry already is sent via telemetry.logLanguageServerEvent("CopilotHoverError", { "ErrorMessage": so adding "Copilot Hover Error: " doesn't seem to add any info.

}
return;
}
if (requestInfo.content.length === 0) {
// Context is not available for this symbol.
telemetry.logLanguageServerEvent("CopilotHover", { "Message": "Copilot summary is not available for this symbol." });
await showCopilotContent(copilotHoverProvider, hoverDocument, hoverPosition, localize("copilot.hover.unavailable", "Copilot summary is not available for this symbol."));
await showCopilotContent(copilotHoverProvider, hoverDocument, hoverPosition, localize("copilot.hover.unavailable.symbol", "Copilot summary is not available for this symbol."));
return;
}

const locale = getLocaleId();

const messages = [
vscode.LanguageModelChatMessage
.User(requestInfo + locale)];
.User(requestInfo.content + locale)];

const [model] = await vscodelm.selectChatModels(modelSelector);

Expand Down
Loading