ContextProvider resolve handler improvements #13296
Conversation
-telemetry: fix cancellation events. -telemetry: more diag event for registration failure.
lukka
changed the title
| const telemetryMetrics: Record<string, number> = {}; | ||
| const projectContextPromise = (async (): Promise<ProjectContext | undefined> => { | ||
| const getProjectContextStartTime = performance.now(); | ||
| const projectContext = await getProjectContext(docUri, { flags: {} }, telemetryProperties, telemetryMetrics); |
There was a problem hiding this comment.
I think it's fine to pass a hardcoded flags: {} here, but just wanted to double check that 1) that will pass only the language standard trait and 2) we don't expect to need to experiment with any other configuration for traits before we ship a new pre-release version. I believe both of those conditions are true.
| // A new incoming snippet request from Copilot arrived and this request | ||
| // is now obsolete, give up immediately with no exception. | ||
| if (e instanceof InternalCancellationError) { | ||
| logMessage += `, (internal cancellation) `; |
There was a problem hiding this comment.
FYI, there's a pre-existing issue with the various logMessage's ending with : or : and then having a , (internal cancellation) or other , message added. Ideally, it would not log :, or : ,
| @@ -581,7 +582,7 @@ interface FilesEncodingChanged { | |||
| export interface CopilotCompletionContextResult { | |||
| requestId: number; | |||
| isResultMissing: boolean; | |||
| snippets: SnippetEntry[]; | |||
| snippets: SupportedContextItem[]; | |||
There was a problem hiding this comment.
I'm seeing the actual data still be a SnippetEntry. Does that matter? i.e. the SupportedContextItem doesn't seem to have entries like startLine.
export interface SnippetEntry {
uri: string;
value: string;
startLine: number;
endLine: number;
importance: number;
}
|
FYI, I have a PR opened to clean up the compiler argument traits, which may impact this PR, see #13278. |
Should your PR get checked in first, right? |
| const telemetryMetrics: Record<string, number> = {}; | ||
| const projectContextPromise = (async (): Promise<ProjectContext | undefined> => { | ||
| const getProjectContextStartTime = performance.now(); | ||
| const projectContext = await getProjectContext(docUri, { flags: {} }, telemetryProperties, telemetryMetrics); |
There was a problem hiding this comment.
Should this be cached? It seems strange to re-request this on every completion. Wasn't the previous related files version cached for like 2 minutes or something like that?
Changes:
-new feature: add traits for C++ lang version
-telemetry: fix cancellation events.
-telemetry: more diag event for registration failure.