Skip to content

Memoize getFirstNonCopilotModel in scenario automation endpoint provider#319638

Merged
amunger merged 4 commits into
mainfrom
aamunger/automationBYOK
Jun 2, 2026
Merged

Memoize getFirstNonCopilotModel in scenario automation endpoint provider#319638
amunger merged 4 commits into
mainfrom
aamunger/automationBYOK

Conversation

@amunger
Copy link
Copy Markdown
Collaborator

@amunger amunger commented Jun 2, 2026

fix https://github.com/microsoft/vscode-copilot-evaluation/issues/4075

In scenario-automation runs against a BYOK no-auth user, ScenarioAutomationEndpointProviderImpl.getChatEndpoint is invoked many times per agent step (main model + utility + utility-small + tool detection + per-MCP-call). Each call hit getFirstNonCopilotModel, which always called lm.selectChatModels() (empty selector). The empty selector fans out across every registered language-model vendor and re-resolves each one — sustained ~4 Hz for an entire 10-minute agent turn in BYOK eval runs, ~2.4k empty-selector calls per turn, with 99.99 percent reporting no changes.

Cache the first non-copilot model for the lifetime of the provider and invalidate on lm.onDidChangeChatModels, debounced through a MicrotaskDelay Delayer so synchronous bursts collapse into a single invalidation. The change listener is installed lazily on first cache use, so production paths that never hit the no-auth branch do not subscribe.

In scenario-automation runs against a BYOK no-auth user, `ScenarioAutomationEndpointProviderImpl.getChatEndpoint` is invoked many times per agent step (main model + utility + utility-small + tool detection + per-MCP-call). Each call hit `getFirstNonCopilotModel`, which always called `lm.selectChatModels()` (empty selector). The empty selector fans out across every registered language-model vendor and re-resolves each one — sustained ~4 Hz for an entire 10-minute agent turn in BYOK eval runs, ~2.4k empty-selector calls per turn, with 99.99 percent reporting `no changes`.

Cache the first non-copilot model for the lifetime of the provider and invalidate on `lm.onDidChangeChatModels`, debounced through a `MicrotaskDelay` `Delayer` so synchronous bursts collapse into a single invalidation. The change listener is installed lazily on first cache use, so production paths that never hit the no-auth branch do not subscribe.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 2, 2026 20:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces scenario-automation overhead in the Copilot extension by memoizing the “first non-copilot” language model selection used by the no-auth/BYOK automation path, avoiding repeated lm.selectChatModels() fan-out calls.

Changes:

  • Cache the first non-copilot LanguageModelChat lookup for the lifetime of ScenarioAutomationEndpointProviderImpl.
  • Invalidate the cache on lm.onDidChangeChatModels, debounced via a microtask Delayer to coalesce bursty model-set updates.
  • Install the change listener lazily on first cache use to avoid unnecessary subscriptions on production paths.
Show a summary per file
File Description
extensions/copilot/src/extension/prompt/vscode-node/scenarioAutomationEndpointProviderImpl.ts Adds memoization + debounced invalidation for resolving the first non-copilot chat model in scenario automation runs.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 2

amunger and others added 3 commits June 2, 2026 13:17
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Repair syntax error introduced by autofix (extra closing brace after the IIFE block in _resolveFirstNonCopilotModel that prematurely closed the class).
- Add diagnostic logs so an eval bundle at default level shows the BYOK redirect engaging (info on resolve and on cache invalidation) and trace logs identifying which redirect branch ran for each request.
- Promote the silent `no models found` throw to error and the capi-proxy family-resolve fallback from trace to warn so both are visible at default log level.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@amunger amunger marked this pull request as ready for review June 2, 2026 20:42
@amunger amunger merged commit 79e25fe into main Jun 2, 2026
25 checks passed
@amunger amunger deleted the aamunger/automationBYOK branch June 2, 2026 23:07
@vs-code-engineering vs-code-engineering Bot added this to the 1.124.0 milestone Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants