From 90103dd85a2cb21cb2fdc531d23540d3142b6f7a Mon Sep 17 00:00:00 2001 From: Glen Chung Date: Thu, 13 Feb 2025 10:42:29 -0800 Subject: [PATCH 1/2] Remove Compiler Argument Traits - Couldn't find examples of where this actually improved the prompt per ad-hoc analysis. --- Extension/src/LanguageServer/client.ts | 22 -- .../src/LanguageServer/copilotProviders.ts | 54 +---- Extension/src/LanguageServer/lmTool.ts | 77 +------ .../tests/copilotProviders.test.ts | 208 ++--------------- .../SingleRootProject/tests/lmTool.test.ts | 209 +----------------- 5 files changed, 38 insertions(+), 532 deletions(-) diff --git a/Extension/src/LanguageServer/client.ts b/Extension/src/LanguageServer/client.ts index 5f43018b3..772d64a72 100644 --- a/Extension/src/LanguageServer/client.ts +++ b/Extension/src/LanguageServer/client.ts @@ -555,19 +555,6 @@ export interface ChatContextResult { targetArchitecture: string; } -export interface FileContextResult { - compilerArguments: string[]; -} - -export interface ProjectContextResult { - language: string; - standardVersion: string; - compiler: string; - targetPlatform: string; - targetArchitecture: string; - fileContext: FileContextResult; -} - interface FolderFilesEncodingChanged { uri: string; filesEncoding: string; @@ -614,7 +601,6 @@ const GenerateDoxygenCommentRequest: RequestType = new RequestType('cpptools/didChangeCppProperties'); const IncludesRequest: RequestType = new RequestType('cpptools/getIncludes'); const CppContextRequest: RequestType = new RequestType('cpptools/getChatContext'); -const ProjectContextRequest: RequestType = new RequestType('cpptools/getProjectContext'); const CopilotCompletionContextRequest: RequestType = new RequestType('cpptools/getCompletionContext'); // Notifications to the server @@ -849,7 +835,6 @@ export interface Client { getCopilotHoverProvider(): CopilotHoverProvider | undefined; getIncludes(uri: vscode.Uri, maxDepth: number): Promise; getChatContext(uri: vscode.Uri, token: vscode.CancellationToken): Promise; - getProjectContext(uri: vscode.Uri): Promise; filesEncodingChanged(filesEncodingChanged: FilesEncodingChanged): void; getCompletionContext(fileName: vscode.Uri, caretOffset: number, featureFlag: CopilotCompletionContextFeatures, token: vscode.CancellationToken): Promise; } @@ -2338,12 +2323,6 @@ export class DefaultClient implements Client { return this.languageClient.sendRequest(IncludesRequest, params); } - public async getProjectContext(uri: vscode.Uri): Promise { - const params: TextDocumentIdentifier = { uri: uri.toString() }; - await this.ready; - return this.languageClient.sendRequest(ProjectContextRequest, params); - } - public async getChatContext(uri: vscode.Uri, token: vscode.CancellationToken): Promise { const params: TextDocumentIdentifier = { uri: uri.toString() }; await withCancellation(this.ready, token); @@ -4292,7 +4271,6 @@ class NullClient implements Client { getCopilotHoverProvider(): CopilotHoverProvider | undefined { return undefined; } getIncludes(uri: vscode.Uri, maxDepth: number): Promise { return Promise.resolve({} as GetIncludesResult); } getChatContext(uri: vscode.Uri, token: vscode.CancellationToken): Promise { return Promise.resolve({} as ChatContextResult); } - getProjectContext(uri: vscode.Uri): Promise { return Promise.resolve({} as ProjectContextResult); } filesEncodingChanged(filesEncodingChanged: FilesEncodingChanged): void { } getCompletionContext(file: vscode.Uri, caretOffset: number, featureFlag: CopilotCompletionContextFeatures, token: vscode.CancellationToken): Promise { return Promise.resolve({} as CopilotCompletionContextResult); } } diff --git a/Extension/src/LanguageServer/copilotProviders.ts b/Extension/src/LanguageServer/copilotProviders.ts index 573fa198c..32a63013e 100644 --- a/Extension/src/LanguageServer/copilotProviders.ts +++ b/Extension/src/LanguageServer/copilotProviders.ts @@ -12,7 +12,7 @@ import * as logger from '../logger'; import * as telemetry from '../telemetry'; import { GetIncludesResult } from './client'; import { getClients } from './extension'; -import { getCompilerArgumentFilterMap, getProjectContext } from './lmTool'; +import { getProjectContext } from './lmTool'; nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })(); const localize: nls.LocalizeFunc = nls.loadMessageBundle(); @@ -43,23 +43,20 @@ export async function registerRelatedFilesProvider(): Promise { for (const languageId of ['c', 'cpp', 'cuda-cpp']) { api.registerRelatedFilesProvider( { extensionId: util.extensionContext.extension.id, languageId }, - async (uri: vscode.Uri, context: { flags: Record }) => { + async (uri: vscode.Uri, context: { flags: Record }, cancellationToken: vscode.CancellationToken) => { const start = performance.now(); const telemetryProperties: Record = {}; const telemetryMetrics: Record = {}; try { const getIncludesHandler = async () => (await getIncludes(uri, 1))?.includedFiles.map(file => vscode.Uri.file(file)) ?? []; const getTraitsHandler = async () => { - const projectContext = await getProjectContext(uri, context, telemetryProperties, telemetryMetrics); + const projectContext = await getProjectContext(uri, context, cancellationToken, telemetryProperties, telemetryMetrics); if (!projectContext) { return undefined; } - let traits: CopilotTrait[] = [ - { name: "intelliSenseDisclaimer", value: '', includeInPrompt: true, promptTextOverride: `IntelliSense is currently configured with the following compiler information. It reflects the active configuration, and the project may have more configurations targeting different platforms.` }, - { name: "intelliSenseDisclaimerBeginning", value: '', includeInPrompt: true, promptTextOverride: `Beginning of IntelliSense information.` } - ]; + let traits: CopilotTrait[] = []; if (projectContext.language) { traits.push({ name: "language", value: projectContext.language, includeInPrompt: true, promptTextOverride: `The language is ${projectContext.language}.` }); } @@ -76,49 +73,6 @@ export async function registerRelatedFilesProvider(): Promise { traits.push({ name: "targetArchitecture", value: projectContext.targetArchitecture, includeInPrompt: true, promptTextOverride: `This build targets ${projectContext.targetArchitecture}.` }); } - if (projectContext.compiler) { - // We will process compiler arguments based on copilotcppXXXCompilerArgumentFilters and copilotcppCompilerArgumentDirectAskMap feature flags. - // The copilotcppXXXCompilerArgumentFilters are maps. The keys are regex strings for filtering and the values, if not empty, - // are the prompt text to use when no arguments are found. - // copilotcppCompilerArgumentDirectAskMap map individual matched argument to a prompt text. - // For duplicate matches, the last one will be used. - const filterMap = getCompilerArgumentFilterMap(projectContext.compiler, context); - if (filterMap !== undefined) { - const directAskMap: Record = context.flags.copilotcppCompilerArgumentDirectAskMap ? JSON.parse(context.flags.copilotcppCompilerArgumentDirectAskMap as string) : {}; - let directAsks: string = ''; - const remainingArguments: string[] = []; - - for (const key in filterMap) { - if (!key) { - continue; - } - - const matchedArgument = projectContext.compilerArguments[key] as string; - if (matchedArgument?.length > 0) { - if (directAskMap[matchedArgument]) { - directAsks += `${directAskMap[matchedArgument]} `; - } else { - remainingArguments.push(matchedArgument); - } - } else if (filterMap[key]) { - // Use the prompt text in the absence of argument. - directAsks += `${filterMap[key]} `; - } - } - - if (remainingArguments.length > 0) { - const compilerArgumentsValue = remainingArguments.join(", "); - traits.push({ name: "compilerArguments", value: compilerArgumentsValue, includeInPrompt: true, promptTextOverride: `The compiler arguments include: ${compilerArgumentsValue}.` }); - } - - if (directAsks) { - traits.push({ name: "directAsks", value: directAsks, includeInPrompt: true, promptTextOverride: directAsks }); - } - } - } - - traits.push({ name: "intelliSenseDisclaimerEnd", value: '', includeInPrompt: true, promptTextOverride: `End of IntelliSense information.` }); - const includeTraitsArray = context.flags.copilotcppIncludeTraits ? context.flags.copilotcppIncludeTraits as string[] : []; const includeTraits = new Set(includeTraitsArray); telemetryProperties["includeTraits"] = includeTraitsArray.join(','); diff --git a/Extension/src/LanguageServer/lmTool.ts b/Extension/src/LanguageServer/lmTool.ts index 1ede1d026..41677e584 100644 --- a/Extension/src/LanguageServer/lmTool.ts +++ b/Extension/src/LanguageServer/lmTool.ts @@ -9,7 +9,7 @@ import * as nls from 'vscode-nls'; import * as util from '../common'; import * as logger from '../logger'; import * as telemetry from '../telemetry'; -import { ChatContextResult, ProjectContextResult } from './client'; +import { ChatContextResult } from './client'; import { getClients } from './extension'; import { checkDuration } from './utils'; @@ -51,7 +51,7 @@ const knownValues: { [Property in keyof ChatContextResult]?: { [id: string]: str } }; -function formatChatContext(context: ChatContextResult | ProjectContextResult): void { +function formatChatContext(context: ChatContextResult): void { type KnownKeys = 'language' | 'standardVersion' | 'compiler' | 'targetPlatform'; for (const key in knownValues) { const knownKey = key as KnownKeys; @@ -68,75 +68,11 @@ export interface ProjectContext { compiler: string; targetPlatform: string; targetArchitecture: string; - compilerArguments: Record; } -export function getCompilerArgumentFilterMap(compiler: string, context: { flags: Record }): Record | undefined { - // The copilotcppXXXCompilerArgumentFilters are maps. - // The keys are regex strings and the values, if not empty, are the prompt text to use when no arguments are found. - let filterMap: Record | undefined; +export async function getProjectContext(uri: vscode.Uri, context: { flags: Record }, cancellationToken: vscode.CancellationToken, telemetryProperties: Record, telemetryMetrics: Record): Promise { try { - switch (compiler) { - case MSVC: - if (context.flags.copilotcppMsvcCompilerArgumentFilter !== undefined) { - filterMap = JSON.parse(context.flags.copilotcppMsvcCompilerArgumentFilter as string); - } - break; - case Clang: - if (context.flags.copilotcppClangCompilerArgumentFilter !== undefined) { - filterMap = JSON.parse(context.flags.copilotcppClangCompilerArgumentFilter as string); - } - break; - case GCC: - if (context.flags.copilotcppGccCompilerArgumentFilter !== undefined) { - filterMap = JSON.parse(context.flags.copilotcppGccCompilerArgumentFilter as string); - } - break; - } - } - catch { - // Intentionally swallow any exception. - } - return filterMap; -} - -function filterCompilerArguments(compiler: string, compilerArguments: string[], context: { flags: Record }, telemetryProperties: Record): Record { - const filterMap = getCompilerArgumentFilterMap(compiler, context); - if (filterMap === undefined) { - return {}; - } - - const combinedArguments = compilerArguments.join(' '); - const result: Record = {}; - const filteredCompilerArguments: string[] = []; - for (const key in filterMap) { - if (!key) { - continue; - } - const filter = new RegExp(key, 'g'); - const filtered = combinedArguments.match(filter); - if (filtered) { - filteredCompilerArguments.push(...filtered); - result[key] = filtered[filtered.length - 1]; - } - } - - if (filteredCompilerArguments.length > 0) { - // Telemetry to learn about the argument distribution. The filtered arguments are expected to be non-PII. - telemetryProperties["filteredCompilerArguments"] = filteredCompilerArguments.join(','); - } - - const filters = Object.keys(filterMap).filter(filter => !!filter).join(','); - if (filters) { - telemetryProperties["filters"] = filters; - } - - return result; -} - -export async function getProjectContext(uri: vscode.Uri, context: { flags: Record }, telemetryProperties: Record, telemetryMetrics: Record): Promise { - try { - const projectContext = await checkDuration(async () => await getClients()?.ActiveClient?.getProjectContext(uri) ?? undefined); + const projectContext = await checkDuration(async () => await getClients()?.ActiveClient?.getChatContext(uri, cancellationToken) ?? undefined); telemetryMetrics["projectContextDuration"] = projectContext.duration; if (!projectContext.result) { return undefined; @@ -151,8 +87,7 @@ export async function getProjectContext(uri: vscode.Uri, context: { flags: Recor standardVersion: projectContext.result.standardVersion, compiler: projectContext.result.compiler, targetPlatform: projectContext.result.targetPlatform, - targetArchitecture: projectContext.result.targetArchitecture, - compilerArguments: {} + targetArchitecture: projectContext.result.targetArchitecture }; if (projectContext.result.language) { @@ -175,8 +110,6 @@ export async function getProjectContext(uri: vscode.Uri, context: { flags: Recor if (projectContext.result.targetArchitecture) { telemetryProperties["targetArchitecture"] = projectContext.result.targetArchitecture; } - telemetryMetrics["compilerArgumentCount"] = projectContext.result.fileContext.compilerArguments.length; - result.compilerArguments = filterCompilerArguments(projectContext.result.compiler, projectContext.result.fileContext.compilerArguments, context, telemetryProperties); return result; } diff --git a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts index f4ace163b..ba466323d 100644 --- a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts +++ b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts @@ -145,20 +145,19 @@ describe('copilotProviders Tests', () => { ok(result.traits === undefined, 'result.traits should be undefined'); }); - const projectContextNoArgs: ProjectContext = { + const projectContext: ProjectContext = { language: 'C++', standardVersion: 'C++20', compiler: 'MSVC', targetPlatform: 'Windows', - targetArchitecture: 'x64', - compilerArguments: {} + targetArchitecture: 'x64' }; it('provides standardVersion trait by default.', async () => { arrange({ vscodeExtension: vscodeExtension, getIncludeFiles: { includedFiles }, - projectContext: projectContextNoArgs, + projectContext: projectContext, rootUri, flags: {} }); @@ -173,15 +172,21 @@ describe('copilotProviders Tests', () => { ok(result.traits.find((trait) => trait.name === 'standardVersion')?.value === 'C++20', 'result.traits should have a standardVersion trait with value "C++20"'); ok(result.traits.find((trait) => trait.name === 'standardVersion')?.includeInPrompt, 'result.traits should have a standardVersion trait with includeInPrompt true'); ok(result.traits.find((trait) => trait.name === 'standardVersion')?.promptTextOverride === 'This project uses the C++20 language standard.', 'result.traits should have a standardVersion trait with promptTextOverride'); + ok(telemetryStub.calledOnce, 'Telemetry should be called once'); + ok(telemetryStub.calledWithMatch('RelatedFilesProvider', sinon.match({ + 'traits': 'standardVersion' + }), sinon.match({ + 'duration': sinon.match.number + }))); }); it('provides traits per copilotcppIncludeTraits.', async () => { arrange({ vscodeExtension: vscodeExtension, getIncludeFiles: { includedFiles }, - projectContext: projectContextNoArgs, + projectContext: projectContext, rootUri, - flags: { copilotcppIncludeTraits: ['intelliSenseDisclaimer', 'intelliSenseDisclaimerBeginning', 'language', 'compiler', 'targetPlatform', 'targetArchitecture', 'intelliSenseDisclaimerEnd'] } + flags: { copilotcppIncludeTraits: ['language', 'compiler', 'targetPlatform', 'targetArchitecture'] } }); await moduleUnderTest.registerRelatedFilesProvider(); @@ -189,13 +194,7 @@ describe('copilotProviders Tests', () => { ok(result, 'result should be defined'); ok(result.traits, 'result.traits should be defined'); - ok(result.traits.length === 8, 'result.traits should have 8 traits if none are excluded'); - ok(result.traits.find((trait) => trait.name === 'intelliSenseDisclaimer'), 'result.traits should have a intellisense trait'); - ok(result.traits.find((trait) => trait.name === 'intelliSenseDisclaimer')?.includeInPrompt, 'result.traits should have a intellisense trait with includeInPrompt true'); - ok(result.traits.find((trait) => trait.name === 'intelliSenseDisclaimer')?.promptTextOverride === 'IntelliSense is currently configured with the following compiler information. It reflects the active configuration, and the project may have more configurations targeting different platforms.', 'result.traits should have a intellisense trait with promptTextOverride'); - ok(result.traits.find((trait) => trait.name === 'intelliSenseDisclaimerBeginning'), 'result.traits should have a intellisenseBegin trait'); - ok(result.traits.find((trait) => trait.name === 'intelliSenseDisclaimerBeginning')?.includeInPrompt, 'result.traits should have a intellisenseBegin trait with includeInPrompt true'); - ok(result.traits.find((trait) => trait.name === 'intelliSenseDisclaimerBeginning')?.promptTextOverride === 'Beginning of IntelliSense information.', 'result.traits should have a intellisenseBegin trait with promptTextOverride'); + ok(result.traits.length === 5, 'result.traits should have 5 traits if none are excluded'); ok(result.traits.find((trait) => trait.name === 'language'), 'result.traits should have a language trait'); ok(result.traits.find((trait) => trait.name === 'language')?.value === 'C++', 'result.traits should have a language trait with value "C++"'); ok(result.traits.find((trait) => trait.name === 'language')?.includeInPrompt, 'result.traits should have a language trait with includeInPrompt true'); @@ -204,10 +203,6 @@ describe('copilotProviders Tests', () => { ok(result.traits.find((trait) => trait.name === 'compiler')?.value === 'MSVC', 'result.traits should have a compiler trait with value "MSVC"'); ok(result.traits.find((trait) => trait.name === 'compiler')?.includeInPrompt, 'result.traits should have a compiler trait with includeInPrompt true'); ok(result.traits.find((trait) => trait.name === 'compiler')?.promptTextOverride === 'This project compiles using MSVC.', 'result.traits should have a compiler trait with promptTextOverride'); - ok(result.traits.find((trait) => trait.name === 'standardVersion'), 'result.traits should have a standardVersion trait'); - ok(result.traits.find((trait) => trait.name === 'standardVersion')?.value === 'C++20', 'result.traits should have a standardVersion trait with value "C++20"'); - ok(result.traits.find((trait) => trait.name === 'standardVersion')?.includeInPrompt, 'result.traits should have a standardVersion trait with includeInPrompt true'); - ok(result.traits.find((trait) => trait.name === 'standardVersion')?.promptTextOverride === 'This project uses the C++20 language standard.', 'result.traits should have a standardVersion trait with promptTextOverride'); ok(result.traits.find((trait) => trait.name === 'targetPlatform'), 'result.traits should have a targetPlatform trait'); ok(result.traits.find((trait) => trait.name === 'targetPlatform')?.value === 'Windows', 'result.traits should have a targetPlatform trait with value "Windows"'); ok(result.traits.find((trait) => trait.name === 'targetPlatform')?.includeInPrompt, 'result.traits should have a targetPlatform trait with includeInPrompt true'); @@ -216,187 +211,20 @@ describe('copilotProviders Tests', () => { ok(result.traits.find((trait) => trait.name === 'targetArchitecture')?.value === 'x64', 'result.traits should have a targetArchitecture trait with value "x64"'); ok(result.traits.find((trait) => trait.name === 'targetArchitecture')?.includeInPrompt, 'result.traits should have a targetArchitecture trait with includeInPrompt true'); ok(result.traits.find((trait) => trait.name === 'targetArchitecture')?.promptTextOverride === 'This build targets x64.', 'result.traits should have a targetArchitecture trait with promptTextOverride'); - ok(result.traits.find((trait) => trait.name === 'intelliSenseDisclaimerEnd'), 'result.traits should have a intellisenseEnd trait'); - ok(result.traits.find((trait) => trait.name === 'intelliSenseDisclaimerEnd')?.includeInPrompt, 'result.traits should have a intellisenseEnd trait with includeInPrompt true'); - ok(result.traits.find((trait) => trait.name === 'intelliSenseDisclaimerEnd')?.promptTextOverride === 'End of IntelliSense information.', 'result.traits should have a intellisenseEnd trait with promptTextOverride'); - }); - - it('handles errors during provider registration.', async () => { - arrange({}); - - await moduleUnderTest.registerRelatedFilesProvider(); - - ok(vscodeGetExtensionsStub.calledOnce, 'vscode.extensions.getExtension should be called once'); - ok(mockCopilotApi.registerRelatedFilesProvider.notCalled, 'registerRelatedFilesProvider should not be called'); - }); - - const projectContext: ProjectContext = { - language: 'C++', - standardVersion: 'C++17', - compiler: 'MSVC', - targetPlatform: 'Windows', - targetArchitecture: 'x64', - compilerArguments: { "/std:c++\d+": '/std:c++17', "/GR-?": '/GR-', "/EH[ascr-]+": '/EHs-c-', "/await": '/await' } - }; - - it('provides compiler argument traits.', async () => { - arrange({ - vscodeExtension: vscodeExtension, - getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] }, - projectContext: projectContext, - rootUri: vscode.Uri.file('C:\\src\\my_project'), - flags: { - copilotcppIncludeTraits: ['compilerArguments'], - copilotcppMsvcCompilerArgumentFilter: '{"/std:c++\d+": "", "/GR-?": "", "/EH[ascr-]+": "", "/await": ""}' - } - }); - await moduleUnderTest.registerRelatedFilesProvider(); - - const result = await callbackPromise; - - ok(result, 'result should be defined'); - ok(result.traits, 'result.traits should be defined'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments'), 'result.traits should have a compiler arguments trait'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.value === '/std:c++17, /GR-, /EHs-c-, /await', 'result.traits should have a compiler arguments trait with value "/std:c++17, /GR-, /EHs-c-, /await"'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.includeInPrompt, 'result.traits should have a compiler arguments trait with includeInPrompt true'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.promptTextOverride === 'The compiler arguments include: /std:c++17, /GR-, /EHs-c-, /await.', 'result.traits should have a compiler arguments trait with promptTextOverride'); - ok(!result.traits.find((trait) => trait.name === 'directAsks'), 'result.traits should not have a direct asks trait'); - }); - - it('provide direct ask traits of compiler arguments.', async () => { - arrange({ - vscodeExtension: vscodeExtension, - getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] }, - projectContext: projectContext, - rootUri: vscode.Uri.file('C:\\src\\my_project'), - flags: { - copilotcppIncludeTraits: ['directAsks', 'compilerArguments'], - copilotcppMsvcCompilerArgumentFilter: '{"/std:c++\d+": "", "/await": "", "/GR-?": "", "/EH[ascr-]+": ""}', - copilotcppCompilerArgumentDirectAskMap: '{"/GR-": "Do not generate code using RTTI keywords.", "/EHs-c-": "Do not generate code using exception handling keywords."}' - } - }); - await moduleUnderTest.registerRelatedFilesProvider(); - - const result = await callbackPromise; - - ok(result, 'result should be defined'); - ok(result.traits, 'result.traits should be defined'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments'), 'result.traits should have a compiler arguments trait'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.value === '/std:c++17, /await', 'result.traits should have a compiler arguments trait with value "/std:c++17, /await"'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.includeInPrompt, 'result.traits should have a compiler arguments trait with includeInPrompt true'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.promptTextOverride === 'The compiler arguments include: /std:c++17, /await.', 'result.traits should have a compiler arguments trait with promptTextOverride'); - ok(result.traits.find((trait) => trait.name === 'directAsks'), 'result.traits should have a direct asks trait'); - ok(result.traits.find((trait) => trait.name === 'directAsks')?.value === 'Do not generate code using RTTI keywords. Do not generate code using exception handling keywords. ', 'result.traits should have a direct asks value'); - ok(result.traits.find((trait) => trait.name === 'directAsks')?.includeInPrompt, 'result.traits should have a direct asks trait with includeInPrompt true'); - ok(result.traits.find((trait) => trait.name === 'directAsks')?.promptTextOverride === 'Do not generate code using RTTI keywords. Do not generate code using exception handling keywords. ', 'result.traits should have a direct ask trait with promptTextOverride'); - ok(telemetryStub.calledOnce, 'Telemetry should be called once'); ok(telemetryStub.calledWithMatch('RelatedFilesProvider', sinon.match({ - "includeTraits": 'directAsks,compilerArguments', - 'traits': 'standardVersion,compilerArguments,directAsks' + 'includeTraits': 'language,compiler,targetPlatform,targetArchitecture', + 'traits': 'language,compiler,standardVersion,targetPlatform,targetArchitecture' }), sinon.match({ 'duration': sinon.match.number }))); }); - it('ignore compilerArguments trait if empty.', async () => { - arrange({ - vscodeExtension: vscodeExtension, - getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] }, - projectContext: projectContext, - rootUri: vscode.Uri.file('C:\\src\\my_project'), - flags: { - copilotcppIncludeTraits: ['directAsks', 'compilerArguments'], - copilotcppMsvcCompilerArgumentFilter: '{"/std:c++\d+": "", "/await": "", "/GR-?": "", "/EH[ascr-]+": ""}', - copilotcppCompilerArgumentDirectAskMap: '{"/GR-": "abc.", "/EHs-c-": "def.", "/std:c++17": "ghi.", "/await": "jkl."}' - } - }); - await moduleUnderTest.registerRelatedFilesProvider(); - - const result = await callbackPromise; - - ok(result, 'result should be defined'); - ok(result.traits, 'result.traits should be defined'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments') === undefined, 'result.traits should not have a compiler arguments trait'); - ok(result.traits.find((trait) => trait.name === 'directAsks'), 'result.traits should have a direct asks trait'); - ok(telemetryStub.calledOnce, 'Telemetry should be called once'); - ok(telemetryStub.calledWithMatch('RelatedFilesProvider', sinon.match({ - "includeTraits": 'directAsks,compilerArguments', - 'traits': 'standardVersion,directAsks' - }))); - }); - - it('uses only last argument from the duplicates.', async () => { - arrange({ - vscodeExtension: vscodeExtension, - getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] }, - projectContext: { - language: 'C++', - standardVersion: 'C++20', - compiler: 'MSVC', - targetPlatform: 'Windows', - targetArchitecture: 'x64', - compilerArguments: { "/std:c++\d+": '/std:c++20', "/await": '/await' } - }, - rootUri: vscode.Uri.file('C:\\src\\my_project'), - flags: { - copilotcppIncludeTraits: ['compilerArguments'], - copilotcppMsvcCompilerArgumentFilter: '{"/std:c++\d+": "", "/await": ""}' - } - }); - await moduleUnderTest.registerRelatedFilesProvider(); - - const result = await callbackPromise; - - ok(result, 'result should be defined'); - ok(result.traits, 'result.traits should be defined'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments'), 'result.traits should have a compiler arguments trait'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.value === '/std:c++20, /await', 'result.traits should have a compiler arguments trait with value "/std:c++20, /await"'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.includeInPrompt, 'result.traits should have a compiler arguments trait with includeInPrompt true'); - ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.promptTextOverride === 'The compiler arguments include: /std:c++20, /await.', 'result.traits should have a compiler arguments trait with promptTextOverride'); - }); - - it('provides direct asks trait for absence of arguments.', async () => { - arrange({ - vscodeExtension: vscodeExtension, - getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] }, - projectContext: projectContextNoArgs, - rootUri: vscode.Uri.file('C:\\src\\my_project'), - flags: { - copilotcppIncludeTraits: ['directAsks'], - copilotcppMsvcCompilerArgumentFilter: - '{"/FOO": "/FOO is not set.", "/BAR": "/BAR is not set."}' - } - }); - await moduleUnderTest.registerRelatedFilesProvider(); - - const result = await callbackPromise; - - ok(result, 'result should be defined'); - ok(result.traits, 'result.traits should be defined'); - ok(result.traits.find((trait) => trait.name === 'directAsks'), 'result.traits should have a direct asks trait'); - ok(result.traits.find((trait) => trait.name === 'directAsks')?.value === '/FOO is not set. /BAR is not set. ', 'result.traits should have a direct asks value'); - ok(result.traits.find((trait) => trait.name === 'directAsks')?.includeInPrompt, 'result.traits should have a direct asks trait with includeInPrompt true'); - ok(result.traits.find((trait) => trait.name === 'directAsks')?.promptTextOverride === "/FOO is not set. /BAR is not set. ", 'result.traits should have a direct ask trait with promptTextOverride'); - }); + it('handles errors during provider registration.', async () => { + arrange({}); - it('does not accept empty regex.', async () => { - arrange({ - vscodeExtension: vscodeExtension, - getIncludeFiles: { includedFiles: ['c:\\system\\include\\vector', 'c:\\system\\include\\string', 'C:\\src\\my_project\\foo.h'] }, - projectContext: projectContextNoArgs, - rootUri: vscode.Uri.file('C:\\src\\my_project'), - flags: { - copilotcppIncludeTraits: ['directAsks'], - copilotcppMsvcCompilerArgumentFilter: - '{"": "Empty regex not allowed."}' - } - }); await moduleUnderTest.registerRelatedFilesProvider(); - const result = await callbackPromise; - - ok(result, 'result should be defined'); - ok(result.traits, 'result.traits should be defined'); - ok(result.traits.find((trait) => trait.name === 'directAsks') === undefined, 'result.traits should not have a direct asks trait'); + ok(vscodeGetExtensionsStub.calledOnce, 'vscode.extensions.getExtension should be called once'); + ok(mockCopilotApi.registerRelatedFilesProvider.notCalled, 'registerRelatedFilesProvider should not be called'); }); }); diff --git a/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts b/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts index 01f55c98b..f91e394e5 100644 --- a/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts +++ b/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts @@ -8,7 +8,7 @@ import { afterEach, beforeEach, describe, it } from 'mocha'; import * as sinon from 'sinon'; import * as vscode from 'vscode'; import * as util from '../../../../src/common'; -import { ChatContextResult, DefaultClient, ProjectContextResult } from '../../../../src/LanguageServer/client'; +import { ChatContextResult, DefaultClient } from '../../../../src/LanguageServer/client'; import { ClientCollection } from '../../../../src/LanguageServer/clientCollection'; import * as extension from '../../../../src/LanguageServer/extension'; import { CppConfigurationLanguageModelTool, getProjectContext } from '../../../../src/LanguageServer/lmTool'; @@ -138,15 +138,6 @@ describe('CppConfigurationLanguageModelTool Tests', () => { sinon.stub(util, 'isHeaderFile').returns(isHeaderFile ?? false); }; - const arrangeProjectContextFromCppTools = ({ projectContextFromCppTools, isCpp, isHeaderFile }: - { projectContextFromCppTools?: ProjectContextResult; isCpp?: boolean; isHeaderFile?: boolean } = - { projectContextFromCppTools: undefined, isCpp: undefined, isHeaderFile: false } - ) => { - activeClientStub.getProjectContext.resolves(projectContextFromCppTools); - sinon.stub(util, 'isCpp').returns(isCpp ?? true); - sinon.stub(util, 'isHeaderFile').returns(isHeaderFile ?? false); - }; - it('should log telemetry and provide #cpp chat context.', async () => { arrangeChatContextFromCppTools({ chatContextFromCppTools: { @@ -178,33 +169,26 @@ describe('CppConfigurationLanguageModelTool Tests', () => { compiler, expectedCompiler, context, - compilerArguments: compilerArguments, - expectedCompilerArguments, telemetryProperties, telemetryMetrics }: { compiler: string; expectedCompiler: string; context: { flags: Record }; - compilerArguments: string[]; - expectedCompilerArguments: Record; telemetryProperties: Record; telemetryMetrics: Record; }) => { - arrangeProjectContextFromCppTools({ - projectContextFromCppTools: { + arrangeChatContextFromCppTools({ + chatContextFromCppTools: { language: 'cpp', standardVersion: 'c++20', compiler: compiler, targetPlatform: 'windows', - targetArchitecture: 'x64', - fileContext: { - compilerArguments: compilerArguments - } + targetArchitecture: 'x64' } }); - const result = await getProjectContext(mockTextDocumentStub.uri, context, telemetryProperties, telemetryMetrics); + const result = await getProjectContext(mockTextDocumentStub.uri, context, new vscode.CancellationTokenSource().token, telemetryProperties, telemetryMetrics); ok(result, 'result should not be undefined'); ok(result.language === 'C++'); @@ -212,177 +196,15 @@ describe('CppConfigurationLanguageModelTool Tests', () => { ok(result.standardVersion === 'C++20'); ok(result.targetPlatform === 'Windows'); ok(result.targetArchitecture === 'x64'); - ok(JSON.stringify(result.compilerArguments) === JSON.stringify(expectedCompilerArguments)); }; - it('should provide compilerArguments based on copilotcppMsvcCompilerArgumentFilter.', async () => { - await testGetProjectContext({ - compiler: 'msvc', - expectedCompiler: 'MSVC', - context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo-?": ""}' } }, - compilerArguments: ['foo', 'bar', 'abc', 'foo-'], - expectedCompilerArguments: { 'foo-?': 'foo-' }, - telemetryProperties: {}, - telemetryMetrics: {} - }); - }); - - it('should provide compilerArguments based on copilotcppClangCompilerArgumentFilter.', async () => { - await testGetProjectContext({ - compiler: 'clang', - expectedCompiler: 'Clang', - context: { flags: { copilotcppClangCompilerArgumentFilter: '{"foo": "", "bar": ""}' } }, - compilerArguments: ['foo', 'bar', 'abc'], - expectedCompilerArguments: { 'foo': 'foo', 'bar': 'bar' }, - telemetryProperties: {}, - telemetryMetrics: {} - }); - }); - - it('should support spaces between argument and value.', async () => { - await testGetProjectContext({ - compiler: 'clang', - expectedCompiler: 'Clang', - context: { flags: { copilotcppClangCompilerArgumentFilter: '{"-std\\\\sc\\\\+\\\\+\\\\d+": ""}' } }, - compilerArguments: ['-std', 'c++17', '-std', 'foo', '-std', 'c++11', '-std', 'bar'], - expectedCompilerArguments: { '-std\\sc\\+\\+\\d+': '-std c++11' }, - telemetryProperties: {}, - telemetryMetrics: {} - }); - }); - - it('should provide compilerArguments based on copilotcppGccCompilerArgumentFilter.', async () => { - await testGetProjectContext({ - compiler: 'gcc', - expectedCompiler: 'GCC', - context: { flags: { copilotcppGccCompilerArgumentFilter: '{"foo": "", "bar": ""}' } }, - compilerArguments: ['foo', 'bar', 'abc', 'bar', 'foo', 'bar'], - expectedCompilerArguments: { 'foo': 'foo', 'bar': 'bar' }, - telemetryProperties: {}, - telemetryMetrics: {} - }); - }); - - it('should provide empty array for each regex if nothing matches.', async () => { - await testGetProjectContext({ - compiler: 'msvc', - expectedCompiler: 'MSVC', - context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo": "", "bar": ""}' } }, - compilerArguments: [], - expectedCompilerArguments: {}, - telemetryProperties: {}, - telemetryMetrics: {} - }); - }); - - it('should filter out all compilerArguments by default.', async () => { - await testGetProjectContext({ - compiler: 'gcc', - expectedCompiler: 'GCC', - context: { flags: {} }, - compilerArguments: ['foo', 'bar'], - expectedCompilerArguments: {}, - telemetryProperties: {}, - telemetryMetrics: {} - }); - }); - - it('should filter out all compilerArguments for empty regex.', async () => { - await testGetProjectContext({ - compiler: 'msvc', - expectedCompiler: 'MSVC', - context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"": ""}' } }, - compilerArguments: ['foo', 'bar'], - expectedCompilerArguments: {}, - telemetryProperties: {}, - telemetryMetrics: {} - }); - }); - - it('should filter out all compilerArguments for empty copilotcppMsvcCompilerArgumentFilter.', async () => { - await testGetProjectContext({ - compiler: 'msvc', - expectedCompiler: 'MSVC', - context: { - flags: { - copilotcppMsvcCompilerArgumentFilter: '' - } - }, - compilerArguments: ['foo', 'bar'], - expectedCompilerArguments: {}, - telemetryProperties: {}, - telemetryMetrics: {} - }); - }); - - it('should filter out all compilerArguments for invalid copilotcppMsvcCompilerArgumentFilter.', async () => { - await testGetProjectContext({ - compiler: 'msvc', - expectedCompiler: 'MSVC', - context: { - flags: { - copilotcppMsvcCompilerArgumentFilter: 'Not a map' - } - }, - compilerArguments: ['foo', 'bar'], - expectedCompilerArguments: {}, - telemetryProperties: {}, - telemetryMetrics: {} - }); - }); - - it('should filter out all compilerArguments for unknown compilers.', async () => { - await testGetProjectContext({ - compiler: 'unknown', - expectedCompiler: '', - context: { - flags: { - copilotcppMsvcCompilerArgumentFilter: '{"(foo|bar)": ""}', - copilotcppClangCompilerArgumentFilter: '{"(foo|bar)": ""}', - copilotcppGccCompilerArgumentFilter: '{"(foo|bar)": ""}' - } - }, - compilerArguments: ['foo', 'bar'], - expectedCompilerArguments: {}, - telemetryProperties: {}, - telemetryMetrics: {} - }); - }); - it('should send telemetry.', async () => { const telemetryProperties: Record = {}; const telemetryMetrics: Record = {}; const input = { compiler: 'msvc', expectedCompiler: 'MSVC', - context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo-?": "", "": "", "bar": "", "xyz": ""}' } }, - compilerArguments: ['foo', 'bar', 'foo-', 'abc'], - expectedCompilerArguments: { 'foo-?': 'foo-', 'bar': 'bar' }, - telemetryProperties, - telemetryMetrics - }; - await testGetProjectContext(input); - - ok(telemetryProperties['language'] === 'C++'); - ok(telemetryProperties['compiler'] === input.expectedCompiler); - ok(telemetryProperties['standardVersion'] === 'C++20'); - ok(telemetryProperties['targetPlatform'] === 'Windows'); - ok(telemetryProperties['targetArchitecture'] === 'x64'); - ok(telemetryProperties['filteredCompilerArguments'] === 'foo,foo-,bar'); - ok(telemetryProperties['filters'] === 'foo-?,bar,xyz'); - ok(telemetryMetrics['compilerArgumentCount'] === input.compilerArguments.length); - ok(telemetryMetrics['projectContextDuration'] !== undefined); - }); - - it('should send filter telemetry if available.', async () => { - const telemetryProperties: Record = {}; - const telemetryMetrics: Record = {}; - const input = { - compiler: 'msvc', - expectedCompiler: 'MSVC', - context: { flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo-?": "", "": "", "bar": "", "xyz": ""}' } }, - compilerArguments: ['abc'], - expectedCompilerArguments: {}, + context: { flags: {} }, telemetryProperties, telemetryMetrics }; @@ -393,46 +215,37 @@ describe('CppConfigurationLanguageModelTool Tests', () => { ok(telemetryProperties['standardVersion'] === 'C++20'); ok(telemetryProperties['targetPlatform'] === 'Windows'); ok(telemetryProperties['targetArchitecture'] === 'x64'); - ok(telemetryProperties['filteredCompilerArguments'] === undefined); - ok(telemetryProperties['filters'] === 'foo-?,bar,xyz'); - ok(telemetryMetrics['compilerArgumentCount'] === input.compilerArguments.length); ok(telemetryMetrics['projectContextDuration'] !== undefined); }); it('should not send telemetry for unknown values', async () => { - arrangeProjectContextFromCppTools({ - projectContextFromCppTools: { + arrangeChatContextFromCppTools({ + chatContextFromCppTools: { language: 'java', standardVersion: 'gnu++17', compiler: 'javac', targetPlatform: 'arduino', - targetArchitecture: 'bar', - fileContext: { - compilerArguments: [] - } + targetArchitecture: 'bar' } }); const telemetryProperties: Record = {}; const telemetryMetrics: Record = {}; const result = await getProjectContext(mockTextDocumentStub.uri, { - flags: { copilotcppMsvcCompilerArgumentFilter: '{"foo-?": "", "": "", "bar": "", "xyz": ""}' } - }, telemetryProperties, telemetryMetrics); + flags: {} + }, new vscode.CancellationTokenSource().token, telemetryProperties, telemetryMetrics); ok(telemetryProperties["targetArchitecture"] === 'bar'); - ok(telemetryProperties["filters"] === undefined); ok(telemetryProperties["language"] === undefined); ok(telemetryProperties["compiler"] === undefined); ok(telemetryProperties["standardVersion"] === undefined); ok(telemetryProperties["originalStandardVersion"] === 'gnu++17'); ok(telemetryProperties["targetPlatform"] === undefined); - ok(telemetryMetrics["compilerArgumentCount"] === 0); ok(result, 'result should not be undefined'); ok(result.language === ''); ok(result.compiler === ''); ok(result.standardVersion === ''); ok(result.targetPlatform === ''); ok(result.targetArchitecture === 'bar'); - ok(Object.keys(result.compilerArguments).length === 0); }); }); From ae6b83731c4182e9f30d20b2dfe14ac9a06b8c1c Mon Sep 17 00:00:00 2001 From: Glen Chung Date: Thu, 13 Feb 2025 11:18:39 -0800 Subject: [PATCH 2/2] Address review comment --- Extension/src/LanguageServer/lmTool.ts | 40 +++++++++++++------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/Extension/src/LanguageServer/lmTool.ts b/Extension/src/LanguageServer/lmTool.ts index 41677e584..40f48e7dd 100644 --- a/Extension/src/LanguageServer/lmTool.ts +++ b/Extension/src/LanguageServer/lmTool.ts @@ -72,43 +72,43 @@ export interface ProjectContext { export async function getProjectContext(uri: vscode.Uri, context: { flags: Record }, cancellationToken: vscode.CancellationToken, telemetryProperties: Record, telemetryMetrics: Record): Promise { try { - const projectContext = await checkDuration(async () => await getClients()?.ActiveClient?.getChatContext(uri, cancellationToken) ?? undefined); - telemetryMetrics["projectContextDuration"] = projectContext.duration; - if (!projectContext.result) { + const chatContext = await checkDuration(async () => await getClients()?.ActiveClient?.getChatContext(uri, cancellationToken) ?? undefined); + telemetryMetrics["projectContextDuration"] = chatContext.duration; + if (!chatContext.result) { return undefined; } - const originalStandardVersion = projectContext.result.standardVersion; + const originalStandardVersion = chatContext.result.standardVersion; - formatChatContext(projectContext.result); + formatChatContext(chatContext.result); const result: ProjectContext = { - language: projectContext.result.language, - standardVersion: projectContext.result.standardVersion, - compiler: projectContext.result.compiler, - targetPlatform: projectContext.result.targetPlatform, - targetArchitecture: projectContext.result.targetArchitecture + language: chatContext.result.language, + standardVersion: chatContext.result.standardVersion, + compiler: chatContext.result.compiler, + targetPlatform: chatContext.result.targetPlatform, + targetArchitecture: chatContext.result.targetArchitecture }; - if (projectContext.result.language) { - telemetryProperties["language"] = projectContext.result.language; + if (result.language) { + telemetryProperties["language"] = result.language; } - if (projectContext.result.compiler) { - telemetryProperties["compiler"] = projectContext.result.compiler; + if (result.compiler) { + telemetryProperties["compiler"] = result.compiler; } - if (projectContext.result.standardVersion) { - telemetryProperties["standardVersion"] = projectContext.result.standardVersion; + if (result.standardVersion) { + telemetryProperties["standardVersion"] = result.standardVersion; } else { if (originalStandardVersion) { telemetryProperties["originalStandardVersion"] = originalStandardVersion; } } - if (projectContext.result.targetPlatform) { - telemetryProperties["targetPlatform"] = projectContext.result.targetPlatform; + if (result.targetPlatform) { + telemetryProperties["targetPlatform"] = result.targetPlatform; } - if (projectContext.result.targetArchitecture) { - telemetryProperties["targetArchitecture"] = projectContext.result.targetArchitecture; + if (result.targetArchitecture) { + telemetryProperties["targetArchitecture"] = result.targetArchitecture; } return result;