Skip to content

Commit

Permalink
Support User Defines and Macro References AB Experiment
Browse files Browse the repository at this point in the history
- Depends on #12979 to merge first.
- copilotcppMacroReferenceFilter: regex string to filter macro reference for telemetry.
- Telemetry related to user defines and macro references.
- Added new trait compilerUserDefines to note the relevant user defines to the editing file.
  • Loading branch information
kuchungmsft committed Nov 19, 2024
1 parent 92e251b commit 2672a83
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 10 deletions.
2 changes: 2 additions & 0 deletions Extension/src/LanguageServer/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,8 @@ export interface ChatContextResult {

export interface FileContextResult {
compilerArguments: string[];
compilerUserDefines: string[];
macroReferences: string[];
}

export interface ProjectContextResult {
Expand Down
4 changes: 4 additions & 0 deletions Extension/src/LanguageServer/copilotProviders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ export async function registerRelatedFilesProvider(): Promise<void> {
const compilerArgumentsValue = updatedArguments.join(", ");
traits.push({ name: "compilerArguments", value: compilerArgumentsValue, includeInPrompt: true, promptTextOverride: `The compiler arguments include: ${compilerArgumentsValue}.` });
}
if (cppContext.compilerUserDefinesRelevant.length > 0) {
const compilerUserDefinesValue = cppContext.compilerUserDefinesRelevant.join(", ");
traits.push({ name: "compilerUserDefines", value: compilerUserDefinesValue, includeInPrompt: true, promptTextOverride: `These compiler command line user defines may be relevent: ${compilerUserDefinesValue}.` });
}
if (directAsks) {
traits.push({ name: "directAsks", value: directAsks, includeInPrompt: true, promptTextOverride: directAsks });
}
Expand Down
33 changes: 31 additions & 2 deletions Extension/src/LanguageServer/lmTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export interface ProjectContext {
targetPlatform: string;
targetArchitecture: string;
compilerArguments: string[];
compilerUserDefinesRelevant: string[];
}

// Set these values for local testing purpose without involving control tower.
Expand Down Expand Up @@ -93,6 +94,15 @@ function filterComplierArguments(compiler: string, compilerArguments: string[],
return compilerArguments.filter(arg => defaultFilter?.test(arg) || additionalFilter?.test(arg));
}

// We can set up copilotcppMacroReferenceFilter feature flag to filter macro references to learn about
// macro usage distribution, i.e., compiler or platform specific macros, or even the presence of certain macros.
const defaultMacroReferenceFilter: RegExp | undefined = undefined;
function filterMacroReferences(macroReferences: string[], context: { flags: Record<string, unknown> }): string[] {
const filter: RegExp | undefined = context.flags.copilotcppMacroReferenceFilter ? new RegExp(context.flags.copilotcppMacroReferenceFilter as string) : undefined;

return macroReferences.filter(macro => defaultMacroReferenceFilter?.test(macro) || filter?.test(macro));
}

export async function getProjectContext(context: { flags: Record<string, unknown> }, token: vscode.CancellationToken): Promise<ProjectContext | undefined> {
const telemetryProperties: Record<string, string> = {};
try {
Expand All @@ -110,7 +120,8 @@ export async function getProjectContext(context: { flags: Record<string, unknown
compiler: projectContext.result.compiler,
targetPlatform: projectContext.result.targetPlatform,
targetArchitecture: projectContext.result.targetArchitecture,
compilerArguments: []
compilerArguments: [],
compilerUserDefinesRelevant: []
};

if (projectContext.result.language) {
Expand All @@ -129,14 +140,32 @@ export async function getProjectContext(context: { flags: Record<string, unknown
telemetryProperties["targetArchitecture"] = projectContext.result.targetArchitecture;
}
telemetryProperties["compilerArgumentCount"] = projectContext.result.fileContext.compilerArguments.length.toString();
// Telemtry to learn about the argument distribution. The filtered arguments are expected to be non-PII.
// Telemtry to learn about the argument and macro distribution. The filtered arguments and macro references
// are expected to be non-PII.
if (projectContext.result.fileContext.compilerArguments.length) {
const filteredCompilerArguments = filterComplierArguments(projectContext.result.compiler, projectContext.result.fileContext.compilerArguments, context);
if (filteredCompilerArguments.length > 0) {
telemetryProperties["filteredCompilerArguments"] = filteredCompilerArguments.join(', ');
result.compilerArguments = filteredCompilerArguments;
}
}
telemetryProperties["compilerUserDefinesCount"] = projectContext.result.fileContext.compilerUserDefines.length.toString();
if (projectContext.result.fileContext.compilerUserDefines.length > 0) {
const userDefinesWithoutValue = projectContext.result.fileContext.compilerUserDefines.map(value => value.split('=')[0]);
const userDefinesRelatedToThisFile = userDefinesWithoutValue.filter(value => projectContext.result?.fileContext.macroReferences.includes(value));
if (userDefinesRelatedToThisFile.length > 0) {
// Don't care the actual name of the user define, just the count that's relevant.
telemetryProperties["compilerUserDefinesRelevantCount"] = userDefinesRelatedToThisFile.length.toString();
result.compilerUserDefinesRelevant = userDefinesRelatedToThisFile;
}
}
telemetryProperties["macroReferenceCount"] = projectContext.result.fileContext.macroReferences.length.toString();
if (projectContext.result.fileContext.macroReferences.length > 0) {
const filteredMacroReferences = filterMacroReferences(projectContext.result.fileContext.macroReferences, context);
if (filteredMacroReferences.length > 0) {
telemetryProperties["filteredMacroReferences"] = filteredMacroReferences.join(', ');
}
}

return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ describe('copilotProviders Tests', () => {
compiler: 'MSVC',
targetPlatform: 'Windows',
targetArchitecture: 'x64',
compilerArguments: []
compilerArguments: [],
compilerUserDefinesRelevant: []
};

it('should not provide project context traits when copilotcppTraits flag is false.', async () => {
Expand Down Expand Up @@ -245,7 +246,8 @@ describe('copilotProviders Tests', () => {
compiler: 'MSVC',
targetPlatform: 'Windows',
targetArchitecture: 'x64',
compilerArguments: ['/std:c++17', '/GR-', '/EHs-c-', '/await']
compilerArguments: ['/std:c++17', '/GR-', '/EHs-c-', '/await'],
compilerUserDefinesRelevant: ['MY_FOO', 'MY_BAR']
};

it('should provide compiler command line traits if available.', async () => {
Expand All @@ -266,6 +268,10 @@ describe('copilotProviders Tests', () => {
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 === 'compilerUserDefines'), 'result.traits should have a compiler user defines trait');
ok(result.traits.find((trait) => trait.name === 'compilerUserDefines')?.value === 'MY_FOO, MY_BAR', 'result.traits should have a compiler user defines trait with value "MY_FOO, MY_BAR"');
ok(result.traits.find((trait) => trait.name === 'compilerUserDefines')?.includeInPrompt, 'result.traits should have a compiler user defines trait with includeInPrompt true');
ok(result.traits.find((trait) => trait.name === 'compilerUserDefines')?.promptTextOverride === 'These compiler command line user defines may be relevent: MY_FOO, MY_BAR.', 'result.traits should have a compiler args trait with promptTextOverride');
ok(!result.traits.find((trait) => trait.name === 'directAsks'), 'result.traits should not have a direct asks trait');
});

Expand Down
84 changes: 78 additions & 6 deletions Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,21 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
expectedCompiler,
context,
compilerArguments: compilerArguments,
expectedCompilerArguments
expectedCompilerArguments,
compilerUserDefines,
expectedCompilerUserDefines,
macroReferences,
expectedMacroReferences
}: {
compiler: string;
expectedCompiler: string;
context: { flags: Record<string, unknown> };
compilerArguments: string[];
expectedCompilerArguments: string[];
compilerUserDefines: string[];
expectedCompilerUserDefines: string[];
macroReferences: string[];
expectedMacroReferences: string[];
}) => {
arrangeProjectContextFromCppTools({
projectContextFromCppTools: {
Expand All @@ -195,7 +203,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
targetPlatform: 'windows',
targetArchitecture: 'x64',
fileContext: {
compilerArguments: compilerArguments
compilerArguments: compilerArguments,
compilerUserDefines: compilerUserDefines,
macroReferences: macroReferences
}
}
});
Expand Down Expand Up @@ -228,6 +238,22 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
"filteredCompilerArguments": expectedCompilerArguments.join(', ')
})));
}
ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({
"compilerUserDefinesCount": compilerUserDefines.length.toString()
})));
if (expectedCompilerUserDefines.length > 0) {
ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({
"compilerUserDefinesRelevantCount": expectedCompilerUserDefines.length.toString()
})));
}
ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({
'macroReferenceCount': macroReferences.length.toString()
})));
if (expectedMacroReferences.length > 0) {
ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({
'filteredMacroReferences': expectedMacroReferences.join(', ')
})));
}
ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({
'time': sinon.match.string
})));
Expand All @@ -238,6 +264,7 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
ok(result.targetPlatform === 'Windows');
ok(result.targetArchitecture === 'x64');
ok(JSON.stringify(result.compilerArguments) === JSON.stringify(expectedCompilerArguments));
ok(JSON.stringify(result.compilerUserDefinesRelevant) === JSON.stringify(expectedCompilerUserDefines));
};

it('should log telemetry and provide cpp context properly when experimental flags are not defined.', async () => {
Expand All @@ -246,7 +273,11 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
expectedCompiler: 'GCC',
context: { flags: {} },
compilerArguments: ['-Wall', '-Werror', '-std=c++20'],
expectedCompilerArguments: []
expectedCompilerArguments: [],
compilerUserDefines: [],
expectedCompilerUserDefines: [],
macroReferences: [],
expectedMacroReferences: []
});
});

Expand All @@ -256,7 +287,39 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
expectedCompiler: 'GCC',
context: { flags: { copilotcppGccCompilerArgumentFilter: '^-(fno\-exceptions|fno\-rtti)$' } },
compilerArguments: ['-Wall', '-Werror', '-std=c++20', '-fno-exceptions', '-fno-rtti', '-pthread', '-O3', '-funroll-loops'],
expectedCompilerArguments: ['-fno-exceptions', '-fno-rtti']
expectedCompilerArguments: ['-fno-exceptions', '-fno-rtti'],
compilerUserDefines: [],
expectedCompilerUserDefines: [],
macroReferences: [],
expectedMacroReferences: []
});
});

it('should honor content-exclusion and provide user defines based on macro references.', async () => {
await testGetProjectContext({
compiler: 'gcc',
expectedCompiler: 'GCC',
context: { flags: {} },
compilerArguments: [],
expectedCompilerArguments: [],
compilerUserDefines: ['FOO', 'BAR', 'XYZ'],
expectedCompilerUserDefines: ['FOO', 'XYZ'],
macroReferences: ['FOO', 'XYZ', 'BAZ'],
expectedMacroReferences: []
});
});

it('should log macro reference telemetry based on copilotcppMacroReferenceFilter.', async () => {
await testGetProjectContext({
compiler: 'gcc',
expectedCompiler: 'GCC',
context: { flags: { copilotcppMacroReferenceFilter: '^(FOO|XYZ)$' } },
compilerArguments: [],
expectedCompilerArguments: [],
compilerUserDefines: [],
expectedCompilerUserDefines: [],
macroReferences: ['FOO', 'XYZ', 'BAZ'],
expectedMacroReferences: ['FOO', 'XYZ']
});
});

Expand All @@ -272,7 +335,11 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
}
},
compilerArguments: ['-fno-exceptions', '-fno-rtti'],
expectedCompilerArguments: []
expectedCompilerArguments: [],
compilerUserDefines: [],
expectedCompilerUserDefines: [],
macroReferences: [],
expectedMacroReferences: []
});
});

Expand All @@ -285,7 +352,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
targetPlatform: 'arduino',
targetArchitecture: 'bar',
fileContext: {
compilerArguments: []
compilerArguments: [],
compilerUserDefines: [],
macroReferences: []
}
}
});
Expand All @@ -295,6 +364,8 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
ok(languageModelToolTelemetryStub.calledOnce, 'logLanguageModelToolEvent should be called once');
ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({
"compilerArgumentCount": '0',
"compilerUserDefinesCount": '0',
"macroReferenceCount": '0',
"targetArchitecture": 'bar'
})));
ok(result, 'result should not be undefined');
Expand All @@ -304,5 +375,6 @@ describe('CppConfigurationLanguageModelTool Tests', () => {
ok(result.targetPlatform === '');
ok(result.targetArchitecture === 'bar');
ok(result.compilerArguments.length === 0);
ok(result.compilerUserDefinesRelevant.length === 0);
});
});

0 comments on commit 2672a83

Please sign in to comment.