feat(cli): add /model --compaction for configurable chat compression model#6019
feat(cli): add /model --compaction for configurable chat compression model#6019Rajeshwaran-R wants to merge 3 commits into
Conversation
Chat compression now uses config.getFastModel?.() (e.g., Qwen/Qwen3-Alpha) instead of the main model for compression side queries. This reduces cost while maintaining functionality. - Removed `model` parameter from compress() method signature - Pass config.getFastModel?.() to runSideQuery for compression - Update test to verify fast model is passed correctly - Test renamed: 'forwards model...' → 'passes getFastModel to runSideQuery for compression'
…model Introduce dedicated compaction model setting (configurable via /model --compaction) with fallback chain: compactionModel → fastModel → main model. Adds UI dialog mode, slash command flag, and runtime snapshot guard so compaction models are never presented as runtime selections.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Additional findings (not mappable to diff lines):
[Suggestion] ModelDialog.tsx — title missing isCompactionModelMode branch. The dialog title conditional has branches for voice, vision, and fast modes but no isCompactionModelMode branch. When the compaction model picker opens, it shows the generic "Select Model" title instead of "Select Compaction Model", making it look like the user is changing the primary model.
[Suggestion] ModelDialog.tsx — no preferredCompactionModelEntry in the preferredKey chain. The currently configured compaction model is not highlighted when the dialog opens (unlike fast, voice, and vision modes which each have a preferred*ModelEntry computation).
| purpose: 'chat-compression', | ||
| skipOutputLanguagePreference: true, | ||
| model, | ||
| model: config.getFastModel?.(), |
There was a problem hiding this comment.
[Critical] config.getFastModel?.() should be config.getCompactionModel()
The PR adds getCompactionModel() on Config (with the correct fallback chain: compactionModel → fastModel → main model), but the compression service bypasses it entirely and hard-codes the fast model. Setting /model --compaction <id> persists the value and syncs it to runtime Config, but compression always uses the fast model — the feature is dead code end-to-end.
| model: config.getFastModel?.(), | |
| model: config.getCompactionModel(), | |
| // Compression uses the compaction model (falls back to fast model, then main model). | |
| // See https://github.com/QwenLM/qwen-code/issues/5956 |
Also update the test in chatCompressionService.test.ts to mock getCompactionModel instead of getFastModel.
— qwen3.7-max via Qwen Code /review
| (isVoiceModelMode || !m.voiceOnly), | ||
| (isVoiceModelMode || !m.voiceOnly) && | ||
| (isVisionModelMode || !m.visionOnly) && | ||
| (isCompactionModelMode || !m.visionOnly), |
There was a problem hiding this comment.
[Critical] Copy-paste bug: (isCompactionModelMode || !m.visionOnly) should be (isCompactionModelMode || !m.voiceOnly)
The compaction filter reuses !m.visionOnly from the vision line above. But getAvailableModelIds (line 219) and the CLI validation (line 613) both use !m.voiceOnly for compaction mode (same as fast mode). The dialog and CLI will show different model sets: the dialog excludes vision-only models (irrelevant to text compression) but includes voice-only models (unsuitable for compression).
| (isCompactionModelMode || !m.visionOnly), | |
| (isCompactionModelMode || !m.voiceOnly), |
— qwen3.7-max via Qwen Code /review
| isVoiceModelMode, | ||
| isVisionModelMode, | ||
| isCompactionModelMode, | ||
| }: ModelDialogProps): React.JSX.Element { |
There was a problem hiding this comment.
[Critical] handleSelect is missing an isCompactionModelMode handler block
The callback has handlers for isVoiceModelMode, isFastModelMode, and isVisionModelMode but no if (isCompactionModelMode) branch. When a user opens the compaction model picker via /model --compaction (no model-id) and selects a model from the dialog, the code falls through to the main model selection path (config.switchModel(...)), silently changing the user's primary model instead of setting the compaction model.
Add an if (isCompactionModelMode) block mirroring the isFastModelMode pattern:
- Encode the selector via
encodeAuxModelSelector(selected) - Persist via
settings.setValue(scope, 'compactionModel', fastModel) - Call
config?.setCompactionModel(fastModel) - Add a history item and
return
Also add isCompactionModelMode to the handleSelect dependency array (currently missing — the callback won't re-create when compaction mode activates, causing a stale closure).
— qwen3.7-max via Qwen Code /review
| expect(mockGenerateText).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| model: mockModel, | ||
| model: 'fast-model-v1', |
There was a problem hiding this comment.
[Critical] Test verifies the wrong behavior
This test was renamed from "forwards model, maxAttempts, and thinkingConfig to runSideQuery" to "passes getFastModel to runSideQuery for compression" and asserts model: 'fast-model-v1'. But the compression service should use config.getCompactionModel() (which falls back to the fast model), not config.getFastModel() directly.
After fixing the service to use getCompactionModel():
- Mock
getCompactionModelreturning'compaction-model-v1'and assert it's used - Add a fallback test: when
getCompactionModelreturns undefined, assert the fast model is used
— qwen3.7-max via Qwen Code /review
| promptId, | ||
| force, | ||
| model, | ||
| config, |
There was a problem hiding this comment.
[Suggestion] CompressOptions.model is now a dead interface field
The model field remains required (model: string) in the CompressOptions interface and all callers in geminiChat.ts still compute and pass it, but compress() no longer destructures or uses it (the diff removed model from the destructure at line 319). This is a misleading API contract — callers believe they're controlling the compression model, but the value is silently discarded.
Either remove model from CompressOptions and update all callers, or (if keeping the interface for future use) mark it as @deprecated with a comment.
— qwen3.7-max via Qwen Code /review
| fastOnly?: boolean; | ||
| /** When true, this model only appears in the voice model selector */ | ||
| voiceOnly?: boolean; | ||
| /** When true, this model only appears in the vision model selector */ |
There was a problem hiding this comment.
[Critical] visionOnly is never propagated — the feature is dead on arrival
This PR adds visionOnly?: boolean to AvailableModel and uses it in ModelDialog filters (lines 257-258), but modelRegistry.ts was never updated to propagate the field from model configs. The registry only copies fastOnly and voiceOnly (modelRegistry.ts:233-234). Since no code ever sets visionOnly on a model, m.visionOnly is always undefined and !undefined is true — both vision mode and compaction mode filters that reference visionOnly have zero effect.
Add visionOnly: model.visionOnly, to modelRegistry.ts alongside the existing fastOnly/voiceOnly propagation, and add validation to reject models with both visionOnly and other flags set (mirroring the existing fastOnly + voiceOnly check at modelRegistry.ts:332).
— qwen3.7-max via Qwen Code /review
| }; | ||
| } | ||
|
|
||
| persistSetting(settings, 'compactionModel', modelName); |
There was a problem hiding this comment.
[Suggestion] Zero test coverage for /model --compaction command handler
85+ lines of new command handling code with no tests. The existing --fast and --vision flags have extensive test coverage that should be mirrored here. Missing tests for:
- Setting a valid compaction model:
/model --compaction <model-id> - Rejecting unavailable compaction models (both bare and auth-qualified)
- Non-interactive mode showing current compaction model status
- Dialog action when
/model --compactionis run with no args in interactive mode Config.getCompactionModel()/setCompactionModel()/resolveCompactionModelSelector()methodsisCompactionModelModestate management inuseModelCommand
— qwen3.7-max via Qwen Code /review
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
2 Critical findings in ModelDialog.tsx: (1) the new (isCompactionModelMode || !m.visionOnly) filter condition ANDs with the existing vision-mode filter, blocking vision-only models from the /model --vision list; (2) handleSelect lacks an isCompactionModelMode early-return path, causing model selection in compaction mode to call config.switchModel() and overwrite the primary model instead of persisting a compaction model.
— claude-sonnet-4-6 via Qwen Code /review
Generated by Claude Code
There was a problem hiding this comment.
[Critical] The newly added filter condition (isCompactionModelMode || !m.visionOnly) silently prevents vision-only models from appearing in the /model --vision dialog.
availableModelEntries filters registry models with two back-to-back conditions:
(isVisionModelMode || !m.visionOnly) && // ← existing line: shows all models in vision mode
(isCompactionModelMode || !m.visionOnly), // ← new line added by this PRBoth conditions are ANDed. When isVisionModelMode = true and isCompactionModelMode = false (i.e., the user runs /model --vision):
- Condition 1 evaluates to
true || !m.visionOnly=true(correct — include vision-only models) - Condition 2 evaluates to
false || !m.visionOnly=!m.visionOnly(wrong — excludes vision-only models)
The combined AND result is !m.visionOnly, so vision-only models can never appear in the selection list, defeating the purpose of condition 1. The fix is either to remove the new line (since condition 1 already handles the vision filter) or to add isVisionModelMode to the new guard:
(isCompactionModelMode || isVisionModelMode || !m.visionOnly),— claude-sonnet-4-6 via Qwen Code /review
Generated by Claude Code
There was a problem hiding this comment.
[Critical] handleSelect has no isCompactionModelMode branch. Selecting a model while in compaction mode falls through to config.switchModel(), which switches the primary chat model instead of persisting a compaction model.
The fast/voice/vision handlers each early-return with mode-specific persistence (settings.setValue(scope, 'fastModel', ...), 'visionModel', etc.). The compaction mode is missing an equivalent:
if (isCompactionModelMode) {
const compactionModel = encodeAuxModelSelector(selected);
const scope = getPersistScopeForModelSelection(settings);
settings.setValue(scope, 'compactionModel', compactionModel);
config?.setCompactionModel?.(compactionModel);
uiState?.historyManager.addItem({ type: 'success', text: `Compaction Model: ${compactionModel}` }, Date.now());
onClose();
return;
}Without this, /model --compaction + Enter overwrites the user's primary model setting, which is a destructive side effect of what should be a narrowly-scoped compaction model selection.
— claude-sonnet-4-6 via Qwen Code /review
Generated by Claude Code
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Suggestion] Missing compactionModel in packages/vscode-ide-companion/schemas/settings.schema.json
The PR adds compactionModel to settingsSchema.ts but doesn't regenerate the committed schema JSON. fastModel and visionModel entries exist; compactionModel is missing. Run npm run generate:settings-schema and include the updated file.
— qwen3.7-max via Qwen Code /review
| get description() { | ||
| return t( | ||
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, [model-id] to switch immediately).', | ||
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, --compaction for chat compression model, [model-id] to switch immediately).', |
There was a problem hiding this comment.
[Critical] i18n test failure — missing translations for new description
The new description string with --compaction for chat compression model has no entries in zh.js or zh-TW.js. Both are strict-parity locales, so mustTranslateKeys.test.ts fails, blocking CI.
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, --compaction for chat compression model, [model-id] to switch immediately).', | |
| 'Switch the model for this session (--fast for suggestion model, --voice for voice transcription model, --vision for the vision bridge model, --compaction for chat compression model, [model-id] to switch immediately).', |
Add the translation key to both Chinese locale files (packages/cli/src/i18n/locales/zh.js and zh-TW.js).
— qwen3.7-max via Qwen Code /review
| ? this.getAllConfiguredModels([selector.authType]) | ||
| : this.getAllConfiguredModels(); | ||
| if (!available.some((m) => m.id === selector.modelId)) { | ||
| return undefined; |
There was a problem hiding this comment.
[Suggestion] getCompactionModel() returns undefined instead of falling back when configured model is unavailable
When compactionModel is set but the model is no longer available (e.g., provider changed, credentials expired), this returns undefined instead of falling through to getFastModel() ?? getModel(). The documented priority chain (compactionModel → fastModel → main model) breaks at the first step.
| return undefined; | |
| return this.getFastModel() ?? this.getModel(); |
— qwen3.7-max via Qwen Code /review
| isFastModelMode, | ||
| isVoiceModelMode, | ||
| isVisionModelMode, | ||
| isCompactionModelMode, |
There was a problem hiding this comment.
[Suggestion] Three isCompactionModelMode integration gaps in this component
isCompactionModelMode is destructured here but not used in several places where the other mode flags are checked:
-
Dialog title (~line 790): No
isCompactionModelModebranch in the title ternary — falls through to generic "Select Model" instead of "Select Compaction Model". -
Left-arrow escape (~line 491):
useKeypresshandler checksisFastModelMode || isVoiceModelMode || isVisionModelModebut omitsisCompactionModelMode— left arrow won't dismiss the compaction dialog. -
Preferred entry (~line 430-483): No
preferredCompactionModelEntrycomputed — dialog opens with cursor at the wrong position instead of highlighting the currently configured compaction model.
Each should mirror the pattern used by isFastModelMode/isVisionModelMode.
— qwen3.7-max via Qwen Code /review
| return { | ||
| type: 'message', | ||
| messageType: 'info', | ||
| content: t( |
There was a problem hiding this comment.
[Suggestion] Misleading "clear" instruction — no clear mechanism exists
This message tells users to use "/model --compaction " to clear the override, but when an empty/whitespace model name is passed, the code only shows the info message or opens the dialog — it never calls setCompactionModel(undefined) or persistSetting(settings, 'compactionModel', '').
Either implement the clear path (mirroring how --fast could work), or remove the false instruction from this message.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /resolve |
Resolved merge conflicts between the --compaction feature and main: - Keep both formatUnavailableCompactionModelMessage (PR) and formatAmbiguousVisionModelMessage (main) as independent new functions - Combine --compaction flag and inline prompt support in the /model command description and argumentHint - Keep both compaction model methods (PR) and reasoning effort methods (main) as independent new methods on Config - Update test assertion to match the merged description string Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution Summary — PR #6019PR: feat(cli): add /model --compaction for configurable chat compression model Conflicts Resolved1.
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Suggestion] ModelDialog.tsx:506 — Left-arrow dismiss missing isCompactionModelMode. The escape/left-arrow handler (isFastModelMode || isVoiceModelMode || isVisionModelMode) omits isCompactionModelMode, breaking the established UX shortcut for dismissing specialized model pickers.
[Suggestion] ModelDialog.tsx:440 — No preferredCompactionModelEntry. The preferredKey resolution chain walks activeRuntimeSnapshot → preferredVoiceModelEntry → preferredVisionModelEntry → preferredFastModelEntry → default with no branch for compaction. The current compaction model is never highlighted in the picker, giving users no visual feedback about what is already configured.
— qwen3.7-max via Qwen Code /review
| }; | ||
| } | ||
|
|
||
| const isCompactionModelCommand = |
There was a problem hiding this comment.
[Critical] Missing auth-type validation gate
The --fast handler (line 463) explicitly checks config.getContentGeneratorConfig()?.authType and returns an error when unavailable. This --compaction handler skips that gate entirely, going straight to resolveModelId(modelName).
When no auth type is configured, /model --compaction <id> produces a confusing error path instead of the clear "Authentication type not available." message that --fast shows.
| const isCompactionModelCommand = | |
| const isCompactionModelCommand = | |
| args === '--compaction' || args.startsWith('--compaction '); | |
| if (isCompactionModelCommand) { | |
| const contentGeneratorConfig = config.getContentGeneratorConfig(); | |
| const authType = contentGeneratorConfig?.authType; | |
| if (!authType) { | |
| return { | |
| type: 'message', | |
| messageType: 'error', | |
| content: t('Authentication type not available.'), | |
| }; | |
| } | |
| const modelName = args.replace('--compaction', '').trim(); |
— qwen3.7-max via Qwen Code /review
| }; | ||
| } | ||
|
|
||
| persistSetting(settings, 'compactionModel', modelName); |
There was a problem hiding this comment.
[Critical] Cross-provider model ambiguity — raw modelName persisted without authType qualifier
The --vision handler (line 614-617) builds qualifiedModelName = \${authType}:${modelId}`before persisting. This handler persists the baremodelName. When the same model ID (e.g., gpt-4o`) is configured under multiple providers, compression traffic could silently route to a different provider than the user intended.
| persistSetting(settings, 'compactionModel', modelName); | |
| const matched = availableModels.find((m) => m.id === selector.modelId); | |
| const qualifiedModelName = selector.authType | |
| ? `${selector.authType}:${selector.modelId}` | |
| : matched?.authType | |
| ? `${matched.authType}:${selector.modelId}` | |
| : modelName; | |
| persistSetting(settings, 'compactionModel', qualifiedModelName); | |
| config.setCompactionModel(qualifiedModelName); |
— qwen3.7-max via Qwen Code /review
| const [isFastModelMode, setIsFastModelMode] = useState(false); | ||
| const [isVoiceModelMode, setIsVoiceModelMode] = useState(false); | ||
| const [isVisionModelMode, setIsVisionModelMode] = useState(false); | ||
| const [isCompactionModelMode, setIsCompactionModelMode] = useState(false); |
There was a problem hiding this comment.
[Suggestion] Missing test coverage for compaction mode in useModelCommand.test.ts
The hook adds isCompactionModelMode with mutual-exclusion logic (compaction suppresses fast/voice/vision) and closeModelDialog resets it, but useModelCommand.test.ts has no test verifying:
openModelDialog({ compactionModelMode: true })setsisCompactionModelModeand suppresses other modescloseModelDialog()resetsisCompactionModelModeto false
Existing tests cover voice/vision modes; compaction should mirror that coverage.
— qwen3.7-max via Qwen Code /review
| this.autoSkillConfirm = params.autoSkillConfirm ?? true; | ||
| this.fastModel = params.fastModel || undefined; | ||
| this.visionModel = params.visionModel || undefined; | ||
| this.compactionModel = params.compactionModel || undefined; |
There was a problem hiding this comment.
[Suggestion] Duplicated resolution logic — resolveCompactionModelSelector() (line 3085) is a verbatim copy of resolveFastModelSelector() (line 3021)
Both methods share identical control flow: check field → resolveModelId → bare-name fallback via currentAuthType → getAllConfiguredModels. The only differences are this.fastModel vs this.compactionModel and the debug log string.
With three auxiliary model types (fast/vision/compaction), extracting a shared helper would prevent drift:
private resolveAuxModelSelector(fieldName: 'fastModel' | 'visionModel' | 'compactionModel', label: string) {
const value = this[fieldName];
if (!value) return undefined;
// ... shared logic ...
}— qwen3.7-max via Qwen Code /review
| isFastModelMode, | ||
| isVoiceModelMode, | ||
| isVisionModelMode, | ||
| isCompactionModelMode, |
There was a problem hiding this comment.
[Suggestion] Left-arrow dismiss and preferred-entry gaps for isCompactionModelMode
Two integration points missed beyond the existing gaps already noted on this component:
-
Left-arrow handler (~line 506):
(isFastModelMode || isVoiceModelMode || isVisionModelMode)omitsisCompactionModelMode. In all other specialized pickers, left-arrow dismisses the dialog. -
Preferred entry (~line 440): The preferredKey resolution chain has no
preferredCompactionModelEntry, so the current compaction model is never highlighted in the picker.
— qwen3.7-max via Qwen Code /review
7 participants
Adds the
--compactionflag to/model, letting users configure a dedicated model for chat compression (auto-compact).Changes:
--compactionflag on/modelcommand, opening a model dialog in compaction modecompactionModelModeoption in UIActionsContextopenModelDialog'compaction-model'dialog type in the Dialog union'Compaction model'label in unavailable model error messagesvisionOnlyproperty onAvailableModelinterface (used by ModelDialog)modelCommand.test.tsassertion for the new descriptionWhy: Chat compression can be expensive when using the primary model. Allowing a separate, cheaper model for compaction reduces token costs without affecting main conversation quality.
🤖 Generated with Qwen Code (2-shotted by Qwen-Coder)