Fix context size handling for Agent Host#322470
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes context size handling for Agent Host-backed chat models by representing the selected context window as a first-class ModelSelection.maxContextWindow value (token count) rather than smuggling it through the string-only config bag. It also introduces recommendedContextWindows on models so the chat UI can present a context-size picker consistently across agents, while letting each agent map the numeric selection to its backend representation (e.g. Copilot SDK contextTier).
Changes:
- Add
recommendedContextWindowsto model info andmaxContextWindowto model selection in the agent-host protocol types, and plumb the new fields through side effects. - Synthesize a tokens-group “Context Size” picker from
recommendedContextWindows, and lift the selected numeric value intoModelSelection.maxContextWindowwhen creating sessions. - Update Copilot agent/session launching to resolve SDK
contextTierfrom the selected context window, and adjust tests accordingly.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts | Adds coverage for lifting context size into maxContextWindow and for synthesizing the picker from recommendedContextWindows. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts | Lifts tokens-group numeric config values into ModelSelection.maxContextWindow and includes it in selection equality checks. |
| src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostLanguageModelProvider.ts | Synthesizes a “Context Size” configuration property from recommendedContextWindows. |
| src/vs/platform/agentHost/test/node/copilotAgent.test.ts | Updates model schema expectations and adds a materialization test for mapping selected context window → SDK tier. |
| src/vs/platform/agentHost/test/node/agentSideEffects.test.ts | Updates model fixtures to include recommendedContextWindows. |
| src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts | Moves contextTier derivation into the agent and threads it through launch plans. |
| src/vs/platform/agentHost/node/copilot/copilotAgent.ts | Exposes recommendedContextWindows, accepts maxContextWindow on selections, and resolves SDK contextTier robustly. |
| src/vs/platform/agentHost/node/agentSideEffects.ts | Plumbs recommendedContextWindows into side-effect output. |
| src/vs/platform/agentHost/common/state/protocol/channels-root/state.ts | Extends protocol state types with recommendedContextWindows and ModelSelection.maxContextWindow. |
| src/vs/platform/agentHost/common/agentService.ts | Extends IAgentModelInfo with recommendedContextWindows. |
Copilot's findings
- Files reviewed: 10/10 changed files
- Comments generated: 3
| * directly. Replaced by the numeric {@link ContextSizeConfigKey}; still read from persisted sessions | ||
| * for backward compatibility. | ||
| */ | ||
| export const ContextTierConfigKey = 'contextTier'; |
There was a problem hiding this comment.
@roblourens thoughts on whether we need to keep this for persisted sessions?
There was a problem hiding this comment.
It's not clear to me, what data is being persisted that uses this key?
There was a problem hiding this comment.
I believe the model config is persisted indirectly as part of src/vs/platform/agentHost/node/sessionDatabase.ts
So old chat sessions would try to restore with this key
| * denominator, so switching models updates the widget before the next | ||
| * request is sent. The usage numerator still comes from the last response. | ||
| */ | ||
| private _selectedModelId: string | undefined; |
There was a problem hiding this comment.
What's the difference between selectedModelId and currentModelId?
There was a problem hiding this comment.
currentModelId is the actual model that was last executed, I think we still want to track it at some level instead of just writing over it. Wanted to sync with @lramos15 before actually fully removing.
It should be possible to preview changes to the context when switching models though
There was a problem hiding this comment.
FWIW I chatted with Logan and we agreed on the new mental model (the widget should show the newly selected model as a preview, instead of the historic content)
| * directly. Replaced by the numeric {@link ContextSizeConfigKey}; still read from persisted sessions | ||
| * for backward compatibility. | ||
| */ | ||
| export const ContextTierConfigKey = 'contextTier'; |
There was a problem hiding this comment.
It's not clear to me, what data is being persisted that uses this key?
Fixes #321707
Notes:
Dependent on AHP changes: