Add contextWindowSize Configuration Support#1539
Conversation
- Add contextWindowSize field to ContentGeneratorConfig interface - Update tokenLimit function to accept contentGeneratorConfig parameter - Implement priority logic: user config > auto-detection - Update chatCompressionService to use new API via getContentGeneratorConfig() - Add contextWindowSize to MODEL_GENERATION_CONFIG_FIELDS for config resolution - Add contextWindowSize to CLI settings schema (model.generationConfig) - Update UI components (Footer, ContextUsageDisplay) to use new config API - Fix test mocks to include getContentGeneratorConfig method This refactor avoids modifying 71+ test files by moving the config to the generator level instead of the Config class level. Modified files: - packages/core/src/core/contentGenerator.ts - packages/core/src/core/tokenLimits.ts - packages/core/src/services/chatCompressionService.ts - packages/core/src/services/chatCompressionService.test.ts - packages/core/src/models/constants.ts - packages/cli/src/config/settingsSchema.ts - packages/cli/src/ui/components/ContextUsageDisplay.tsx - packages/cli/src/ui/components/Footer.tsx
📋 Review SummaryThis PR introduces a new configuration option for context window size in the ContentGeneratorConfig, allowing users to override automatic detection. The changes are well-structured and maintain backward compatibility while providing more control over token limits for different use cases. 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
|
模型 |
| export function tokenLimit( | ||
| model: Model, | ||
| type: TokenLimitType = 'input', | ||
| contentGeneratorConfig?: { contextWindowSize?: number }, |
There was a problem hiding this comment.
There are still multiple call sites that compute context limits via tokenLimit(model) without the config. It is not correct.
There was a problem hiding this comment.
I would suggest setup contextWindowSize in contentGeneratorConfig in the right timing (by calling old tokenLimit if the value is not set by user), and replace all occurrences of tokenLimit to contentGeneratorConfig?.contextWindowSize. Because it is a property tightly connected to model.
Resolved conflicts in: - packages/cli/src/ui/components/ContextUsageDisplay.tsx - packages/cli/src/ui/components/Footer.tsx - packages/cli/src/ui/components/Footer.test.tsx - docs/users/configuration/settings.md Changes: - Merged main branch UI improvements with rightItems architecture - Updated contextWindowSize documentation to be more concise - Preserved all main branch features and functionality
- Initialize contextWindowSize and maxOutputTokens in contentGeneratorConfig during config resolution - Remove third parameter from tokenLimit() function for cleaner API - Replace all tokenLimit() calls with direct config property access for better performance - Add maxOutputTokens field to ContentGeneratorConfig type - Update dashscope provider to use config.maxOutputTokens - Auto-detect token limits from model during initialization if not user-configured - Update settingsSchema: set contextWindowSize default to undefined and showInDialog to false Benefits: - Token limits calculated once during initialization instead of repeatedly - Cleaner API with fewer parameters - Better performance by caching computed values - User configuration takes precedence over auto-detection - All 72 unit tests passing
- Update client.test.ts to mock config.getContentGeneratorConfig() instead of tokenLimit() - Remove unused tokenLimit import - Fix compression tests by adding contextWindowSize mock to ensure compression is triggered - Update config.test.ts to match new getTruncateToolOutputThreshold() calculation logic - Update dashscope.test.ts to adapt to maxOutputTokens configuration - Remove obsolete buildRuntimeFetchOptions mock - All 3438 tests now pass (100% pass rate)
| description: | ||
| "Overrides the default context window size for the selected model. Use this setting when a provider's effective context limit differs from Qwen Code's default. This value defines the model's assumed maximum context capacity, not a per-request token limit.", | ||
| parentKey: 'generationConfig', | ||
| childKey: 'contextWindowSize', |
There was a problem hiding this comment.
The childKey is a history mistake I suppose, pls remove all of them.
| export const ContextUsageDisplay = ({ | ||
| promptTokenCount, | ||
| model, | ||
| model: _model, |
There was a problem hiding this comment.
Please remove model from props, and pass contextWindowSize instead of Config object
| _meta: { | ||
| contextLimit: tokenLimit(model.id), | ||
| // Use the contextWindowSize from config, which is always set during initialization | ||
| contextLimit: contentGeneratorConfig?.contextWindowSize, |
There was a problem hiding this comment.
This place should not use contentGeneratorConfig?.contextWindowSize. The context window size of each model might differs, need a better solution.
| private applyOutputTokenLimit<T extends { max_tokens?: number | null }>( | ||
| request: T, | ||
| model: string, | ||
| _model: string, |
There was a problem hiding this comment.
Do not write this code like this easily.
| contextWindowSize?: number; | ||
| // Maximum output tokens override. If set to a positive number, it will override | ||
| // the automatic detection. Leave undefined to use automatic detection. | ||
| maxOutputTokens?: number; |
There was a problem hiding this comment.
I don't like this maxOutputTokens. It is not in the initial PR, it is not in the review comments, anyhow it comes up without proper reason.
There was a problem hiding this comment.
I am ok with it if the logic is solid with pr description and docs updated.
- Remove legacy childKey field from settingsSchema - Add contextWindowSize and maxOutputTokens to documentation - Refactor ContextUsageDisplay to accept contextWindowSize directly instead of Config object - Add useMemo optimization for contextWindowSize in Footer component - Fix logic gaps in contentGenerator for contextWindowSize and maxOutputTokens initialization - Increase DEFAULT_OUTPUT_TOKEN_LIMIT from 4K to 8K for better usability - Add fallback to default values when model is not available
…ontext-window-size-config
- Fix handleModelChange to update contextWindowSize and maxOutputTokens during hot-update - Fix dashscope.ts to use contentGeneratorConfig.maxOutputTokens instead of tokenLimit() - Fix acpAgent.ts to use model-specific contextLimit for each model - Add comprehensive tests for model switching scenarios - Fix all TypeScript type errors (index signature and ConfigSource types) - Fix all ESLint errors (remove 'any' types)
The tokenLimit function has a default parameter value of 'input' for the type parameter, so explicitly passing 'input' is redundant.
Clarify that when modelLimit is undefined, it could be either: - No limit configured - Config not initialized yet In both cases, we don't modify max_tokens and let the API handle it.
- Remove maxOutputTokens field from ContentGeneratorConfig - Remove maxOutputTokens auto-calculation logic in contentGenerator.ts - Remove maxOutputTokens sync logic in config.ts hot-update - Update dashscope.ts to use tokenLimit() function for dynamic calculation - Remove maxOutputTokens from documentation (settings.md) - Update all test cases to use correct model output limits - All 143 tests passing (config.test.ts: 105, dashscope.test.ts: 38) Changes: - Users can no longer configure maxOutputTokens via settings - Output token limits are now always dynamically calculated based on model - contextWindowSize (input) remains user-configurable - Prevents long text output truncation caused by default value settings
| // User explicitly set contextWindowSize | ||
| setSource(sources, 'contextWindowSize', seedOrUnknown('contextWindowSize')); | ||
| } | ||
|
|
There was a problem hiding this comment.
It is not appropriate to place these fallback values calculation here because this is method is like a validation not calculation.
ModelsConfig.applyResolvedModelDefaults is more appropriate.
…Defaults - Move contextWindowSize auto-detection from resolveContentGeneratorConfigWithSources to ModelsConfig.applyResolvedModelDefaults - Improves separation of concerns: validation vs. default value application - resolveContentGeneratorConfigWithSources now focuses on validation and parsing - applyResolvedModelDefaults handles all model-specific default configurations - Add tokenLimit import to modelsConfig.ts - All 105 tests passing Addresses PR review comment about appropriate placement of fallback calculation logic.
| const modelLimit = tokenLimit(model, 'output'); | ||
| // Dynamically calculate output token limit using tokenLimit function | ||
| // This ensures we always use the latest model-specific limits without relying on user configuration | ||
| const modelLimit = tokenLimit(request.model, 'output'); |
There was a problem hiding this comment.
Why ensures we always use the latest model-specific limits without relying on user configuration?
This tokenLimit does not cover all dashscope models' context window size.
Addition: if we decide to respect user config context window size instead of using tokenLimit, which is very reasonable, then we should take either actions below:
- add these config in
packages/cli/src/ui/models/availableModels.tsto ensure our hard-coded qwen-oauth models have correct limits set. - route
coder-modelandvision-modelto use our hard-coded params here.
There was a problem hiding this comment.
I've refactored the code to move the contextWindowSize fallback calculation from resolveContentGeneratorConfigWithSources to ModelsConfig.applyResolvedModelDefaults. This improves the separation of concerns:
resolveContentGeneratorConfigWithSourcesnow focuses purely on validation and parsingapplyResolvedModelDefaultshandles all model-specific default value applications, includingcontextWindowSize
This refactoring ensures that:
- Default value calculation is placed in the appropriate location alongside other generation config defaults (timeout, maxRetries, samplingParams, etc.)
- The validation method remains focused on its core responsibility
- All call paths (refreshAuth and switchModel) correctly apply the fallback values before validation
There was a problem hiding this comment.
Issue 1: Context Window Size Doesn't Update on Model Switch
When ModelsConfig.setModel() is called programmatically, the contextWindowSize in contentGeneratorConfig is not being refreshed. This causes the context usage display in the footer to show stale/incorrect values from the previous model rather than the newly selected model's configuration.
Issue 2: ACP Agent Ignores Provider-Configured contextWindowSize
The ACP agent calculates contextLimit using tokenLimit() directly, which only performs auto-detection based on model name patterns. This means any contextWindowSize explicitly configured in modelProviders[].generationConfig is not being respected.
…h and ACP agent config priority - Fix contextWindowSize not updating when switching models via setModel() - Fix ACP agent to respect provider-configured contextWindowSize before auto-detection - Simplify getTruncateToolOutputThreshold to use static threshold - Add support for GLM-4.7, Kimi-2.5, and MiniMax-M2.1 models - Update context usage display to require explicit contextWindowSize Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
- Footer.test.tsx: provide contextWindowSize in mock config to match Footer component's new requirement for displaying context usage - tokenLimits.test.ts: consolidate Kimi K2 tests and update expectations to 256K for all variants to match the implementation
Add contextWindowSize Configuration Support
Overview
This PR adds support for configuring context window size (
contextWindowSize) through theContentGeneratorConfiginterface. Users can now override automatic detection values to provide more granular control over the model's input token limits for different use cases.Motivation
Key Changes
1. Core Interface Enhancement (
packages/core/src/core/contentGenerator.ts)New Field:
contextWindowSize?: number- Context window size configuration (user-configurable)Logic Improvements:
contextWindowSizefield toContentGeneratorConfiginterfaceresolveContentGeneratorConfigWithSourcesfocuses on validation and parsing2. Model Configuration Enhancement (
packages/core/src/models/modelsConfig.ts)Key Changes:
contextWindowSizefallback calculation inapplyResolvedModelDefaults()methodtokenLimit(model, 'input')for automatic detection when not explicitly configuredcontextWindowSizeis always calculated before validation3. Token Limits Enhancement (
packages/core/src/core/tokenLimits.ts)Key Changes:
DEFAULT_TOKEN_LIMITconstant for use in other modulestokenLimit()function4. Configuration Resolution (
packages/core/src/models/constants.ts)'contextWindowSize'toMODEL_GENERATION_CONFIG_FIELDSconstant5. Chat Compression Service (
packages/core/src/services/chatCompressionService.ts)config.getContentGeneratorConfig()to get configuration6. CLI Settings Schema (
packages/cli/src/config/settingsSchema.ts)New Configuration Option:
model.generationConfig.contextWindowSize- Context window size overrideCleanup:
childKeyfield (from type definition and all usages)7. UI Component Refactoring
ContextUsageDisplay (
packages/cli/src/ui/components/ContextUsageDisplay.tsx):contextWindowSizeparameter instead of entireConfigobjectmodelparameterConfigtype, reducing couplingFooter (
packages/cli/src/ui/components/Footer.tsx):useMemoto optimizecontextWindowSizecalculation, avoiding unnecessary recomputation8. Configuration Hot-Update (
packages/core/src/config/config.ts)contextWindowSizewhen switching models9. Documentation Enhancement (
docs/users/configuration/settings.md)New Documentation:
contextWindowSizetomodel.generationConfigdescriptioncontextWindowSizefield's purpose and behavior10. Test Updates
Updated Test Files:
packages/core/src/config/config.test.ts- Added comprehensive tests forcontextWindowSizeconfigurationpackages/core/src/services/chatCompressionService.test.ts- Updated to use new config APIpackages/cli/src/ui/components/Footer.test.tsx- Updated component testspackages/core/src/core/client.test.ts- Updated client testspackages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts- Updated provider testsAll tests have been updated to adapt to the new API and configuration structure.
Configuration Behavior
contextWindowSize Priority System
tokenLimit(model, 'input')Use Cases:
Configuration Example
{ "model": { "generationConfig": { "contextWindowSize": 128000, "customHeaders": { "X-Request-ID": "req-123" }, "samplingParams": { "temperature": 0.2, "top_p": 0.8, "max_tokens": 1024 } } } }Testing
✅ All existing tests pass (3452 tests)
✅ Pre-commit hooks (prettier, eslint) pass
✅ Configuration correctly applied and resolved
✅ 5 test files updated to adapt to new API
✅ No new lint errors
Code Quality
✅ Separation of Concerns:
resolveContentGeneratorConfigWithSourcesfocuses on validation and parsingModelsConfig.applyResolvedModelDefaultshandles default value application✅ Code Statistics:
Impact Statistics
Migration Notes
No Breaking Changes
contextWindowSizeis optional: Users can explicitly set values when neededmaxOutputTokensremoval is transparent: Always calculated automaticallyAPI Changes
Removed Fields:
childKey- Legacy field, completely removed from settings schemaNew Fields:
ContentGeneratorConfig.contextWindowSize- Context window size (user-configurable)Component Interface Changes:
ContextUsageDisplaynow receivescontextWindowSizeinstead ofconfigobjectRelated Issues
#1400
Checklist
contextWindowSizewith automatic fallback