Add statistics for default diagnostic provider#3232
Conversation
There was a problem hiding this comment.
Pull request overview
Adds statistics tracking for the “default diagnostics” provider so its diagnostic bag is represented in context-provider telemetry, and updates tests to exercise the new API entry point.
Changes:
- Introduces
addDefaultDiagnosticBag(...)to append default diagnostics and record provider statistics/resolution metadata. - Updates diagnostics context processing naming and expectation setup helper usage.
- Updates prompt factory tests to call the new method and use stronger typing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/extension/completions-core/vscode-node/lib/src/prompt/contextProviders/diagnostics.ts | Refactors expectation setup helper for diagnostic bags (but still impacts how expectations are recorded). |
| src/extension/completions-core/vscode-node/lib/src/prompt/completionsPromptFactory/componentsCompletionsPromptFactory.tsx | Adds default-diagnostics bag injection plus statistics + resolvedContextItems telemetry wiring. |
| src/extension/completions-core/vscode-node/lib/src/prompt/completionsPromptFactory/test/completionsPromptFactory.test.tsx | Updates tests to call addDefaultDiagnosticBag and adjusts types/imports accordingly. |
| // Set expectations for the diagnostics provided. | ||
| for (const item of diagnosticBags) { | ||
| setupExpectationsForDiagnostics(accessor, completionId, item.data, item.providerId); | ||
| setupExpectationsForDiagnosticBags(accessor, completionId, item.data, item.providerId); | ||
| } |
There was a problem hiding this comment.
getDiagnosticsFromContextItems currently adds expectations twice for the same diagnostic bag: once via setupExpectationsForDiagnosticBags(...) and again inside the validity filter (including potentially adding both included and content_excluded for invalid items). This can lead to duplicated/conflicting expectations and incorrect usage telemetry (e.g. an excluded diagnostic also having an included expectation, which will later be marked as error). Consider removing the upfront setupExpectationsForDiagnosticBags call and only setting expectations in the validity filter (or otherwise ensuring each bag gets exactly one expectation).
| const settings = this.instantiationService.invokeFunction(getDefaultDiagnosticSettings); | ||
| diagnosticBags = this.addDefaultDiagnosticBag(resolvedContextItems, diagnosticBags, completionId, completionState, settings); | ||
| return { traits, codeSnippets, diagnostics: diagnosticBags, turnOffSimilarFiles, resolvedContextItems }; |
There was a problem hiding this comment.
resolveContext calls addDefaultDiagnosticBag(...) unconditionally, but addDefaultDiagnosticBag records expectations/resolution in contextProviderStatistics. If useContextProviderAPI(...) is false, computeMatch(...) is never called, so these expectations are never cleared (they’re only cleared in computeMatch’s finally), which can lead to stale/accumulating expectations and incorrect telemetry on later completions. Consider only recording statistics / pushing into resolvedContextItems when the context provider API is enabled (or ensure expectations are cleared when telemetry won’t be computed).
| type PromptFactoryWithDiagnostic = IPromptFactory & { | ||
| addDefaultDiagnosticBag( | ||
| resolvedContextItems: ResolvedContextItem[], | ||
| bags: DiagnosticBagWithId[] | undefined, | ||
| completionId: string, | ||
| completionState: CompletionState, | ||
| settings: DefaultDiagnosticSettings | ||
| ): DiagnosticBagWithId[] | undefined; | ||
| }; |
There was a problem hiding this comment.
addDefaultDiagnosticBag now mutates resolvedContextItems and adds expectations/lastResolution for provider statistics, but the updated tests only validate the returned diagnostic bags. Consider adding coverage that asserts (1) resolvedContextItems gets an entry for copilot.chat.defaultDiagnostics with expected resolution/matchScore, and (2) provider statistics were updated (e.g. by overriding ICompletionsContextProviderService in the test context to use TestContextProviderStatistics so expectations/lastResolution can be inspected).
2 participants
Enhance the default diagnostic provider by adding statistics tracking and updating related tests to ensure functionality.