Draft
Conversation
…metered-connections-fixes
Contributor
There was a problem hiding this comment.
Pull request overview
Adds metered-network awareness to inline completions providers (opt-out via metadata) and extends telemetry to respect metered connections, ensuring network-heavy features can be suppressed when the connection is metered.
Changes:
- Introduces
usesNetworkRequests?: booleanfor inline completions provider metadata / provider definitions and wires it through the ext host ↔ main thread registration pipeline. - Filters inline completions providers when
IMeteredConnectionService.isConnectionMeteredis true. - Plumbs
IMeteredConnectionServiceinto workbench telemetry and updates test/fixture service setups to include the new service.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/vscode-dts/vscode.proposed.inlineCompletionsAdditions.d.ts | Adds proposed API metadata flag usesNetworkRequests (defaulting to true). |
| src/vs/monaco.d.ts | Adds usesNetworkRequests to Monaco inline completions provider definition. |
| src/vs/editor/common/languages.ts | Adds usesNetworkRequests to the internal InlineCompletionsProvider interface. |
| src/vs/workbench/api/common/extHostLanguageFeatures.ts | Sends usesNetworkRequests (default true) over the ext host protocol during inline completions provider registration. |
| src/vs/workbench/api/common/extHost.protocol.ts | Extends $registerInlineCompletionsSupport signature with usesNetworkRequests. |
| src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts | Accepts and stores usesNetworkRequests on the extension-backed inline completions provider. |
| src/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionsModel.ts | Skips providers marked as using network requests when the connection is metered. |
| src/vs/workbench/services/telemetry/electron-browser/telemetryService.ts | Injects IMeteredConnectionService and passes it into the base telemetry service config. |
| src/vs/platform/telemetry/common/telemetryService.ts | Ensures buffered events flush through _log (so metered filtering applies at flush time). |
| src/vs/workbench/test/browser/componentFixtures/fixtureUtils.ts | Adds an IMeteredConnectionService test stub to fixture service wiring. |
| src/vs/editor/contrib/inlineCompletions/test/browser/utils.ts | Adds an IMeteredConnectionService test stub to inline completions test harness wiring. |
Comment on lines
+473
to
+482
| const isMetered = this._meteredConnectionService.isConnectionMetered; | ||
|
|
||
| const availableProviders: InlineCompletionsProvider[] = []; | ||
| for (const provider of unsuppressedProviders) { | ||
| if (provider.groupId && excludedGroupIds.has(provider.groupId)) { | ||
| continue; | ||
| } | ||
| if (isMetered && provider.usesNetworkRequests) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
This introduces new behavior (skipping providers on metered connections when usesNetworkRequests is true), but there doesn't appear to be a test asserting that providers are filtered correctly when IMeteredConnectionService.isConnectionMetered is true. Consider adding a unit test that stubs IMeteredConnectionService to true and verifies a provider with usesNetworkRequests: true is not queried (and one with usesNetworkRequests: false still is).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.