Autocomplete: add items to CompletionEvent with tree-sitter fields for analytics
#1144
Conversation
| // Information about each completion item received per one completion event | ||
| items: CompletionItemInfo[] | ||
| } | ||
|
|
||
| export interface ItemPostProcesssingInfo { | ||
| // Number of ERROR nodes found in the completion insert text after pasting | ||
| // it into the document and parsing this range with tree-sitter. | ||
| parseErrorCount?: number | ||
| // Number of lines truncated for multiline completions. | ||
| lineTruncatedCount?: number | ||
| // The truncation approach used. | ||
| truncatedWith?: 'tree-sitter' | 'indentation' | ||
| } | ||
|
|
||
| interface CompletionItemInfo extends ItemPostProcesssingInfo { | ||
| lineCount: number | ||
| charCount: number | ||
| } |
There was a problem hiding this comment.
These fields are added to CompletionEvent
There was a problem hiding this comment.
Cc @chenkc805 and @kelsey-brown
| @@ -103,7 +124,7 @@ export function networkRequestStarted(id: string, contextSummary: ContextSummary | |||
| } | |||
| } | |||
|
|
|||
| export function loaded(id: string): void { | |||
| export function loaded(id: string, items: InlineCompletionItemWithAnalytics[]): void { | |||
There was a problem hiding this comment.
New analytics info is added to the existing events here.
| export function processInlineCompletions( | ||
| items: InlineCompletionItem[], | ||
| params: ProcessInlineCompletionsParams | ||
| ): InlineCompletionItem[] { | ||
| ): InlineCompletionItemWithAnalytics[] { |
There was a problem hiding this comment.
processInlineCompletions now returns all the information required for logging. Another way to implement it is to pass the logId here and sprinkle log calls throughout the internal logic, but I found that it makes it harder to test the whole pipeline, so the approach where processInlineCompletions is a pure function.
| @@ -120,7 +155,7 @@ export function loaded(id: string): void { | |||
| // | |||
| // For statistics logging we start a timeout matching the READ_TIMEOUT_MS so we can increment the | |||
| // suggested completion count as soon as we count it as such. | |||
| export function suggested(id: string, source: string, completion: InlineCompletionItem): void { | |||
| export function suggested(id: string, source: InlineCompletionsResultSource, completion: InlineCompletionItem): void { | |||
There was a problem hiding this comment.
Should the source move into the InlineCompletionItem too?
There was a problem hiding this comment.
The InlineCompletionItem type is compliant with vscode.InlineCompletionItem and the vscode one doesn't have the source field. I'm not sure why we use an alias instead of vscode.InlineCompletionItem directly (I support it because of compatibility issues with non-VS-Code environments).
| import { ProviderConfig } from './providers/provider' | ||
| import { RequestManager, RequestParams } from './request-manager' | ||
| import { ProvideInlineCompletionItemsTracer, ProvideInlineCompletionsItemTraceData } from './tracer' | ||
| import { InlineCompletionItem } from './types' | ||
|
|
||
| interface AutocompleteResult extends vscode.InlineCompletionList { | ||
| completionEvent?: CompletionEvent |
There was a problem hiding this comment.
Should this also have items?
There was a problem hiding this comment.
Yep, extends vscode.InlineCompletionList handles this part
| @@ -15,7 +15,7 @@ import type * as vscode from 'vscode' | |||
| // at Object.<anonymous> (/snapshot/dist/agent.js) | |||
| // at Module._compile (pkg/prelude/bootstrap.js:1926:22) | |||
| // </VERY IMPORTANT> | |||
| import type { InlineCompletionItemProvider } from '../../vscode/src/completions/vscodeInlineCompletionItemProvider' | |||
| import type { InlineCompletionItemProvider } from '../../vscode/src/completions/inline-completion-item-provider' | |||
There was a problem hiding this comment.
Oh no this will take some time to get used too 🙈
There was a problem hiding this comment.
Trying to be consistent with filenames, most of the things are kebab-case and we do not prefix modules in the vscode package with vscode 😛
There was a problem hiding this comment.
Yeah not saying it doesn't make sense 🙈
Adds a changelog item for #1144
…s for analytics (#1144) Adds information about completion items to the `CompletionEvent` we send to our analytics platforms.
Adds a changelog item for #1144
Context
CompletionEventwe send to our analytics platforms.Test plan
Updated unit tests + CI.