-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat(cli): add /model --compaction for configurable chat compression model #6019
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
15dd25b
2d65e71
e295f31
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -38,6 +38,9 @@ const MAIN_MODEL_CONFIGURATION_HINT = | |||||||||||||||||||
| const FAST_MODEL_CONFIGURATION_HINT = | ||||||||||||||||||||
| 'Configure models in settings.modelProviders and ensure the required environment variables are set. In interactive mode, run /auth to configure or switch providers, or run /model --fast without a model to choose from configured models.'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const COMPACTION_MODEL_CONFIGURATION_HINT = | ||||||||||||||||||||
| 'Configure models in settings.modelProviders and ensure the required environment variables are set. In interactive mode, run /auth to configure or switch providers, or run /model --compaction without a model to choose from configured models.'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const VISION_MODEL_CONFIGURATION_HINT = | ||||||||||||||||||||
| 'Configure an image-capable model in settings.modelProviders and ensure the required environment variables are set. Run /model --vision <model-id> to set it, or leave it unset to auto-pick a same-provider vision model.'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -92,7 +95,7 @@ async function switchMainModel( | |||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function formatUnavailableModelMessage( | ||||||||||||||||||||
| kind: 'Model' | 'Fast model' | 'Vision model', | ||||||||||||||||||||
| kind: 'Model' | 'Fast model' | 'Vision model' | 'Compaction model', | ||||||||||||||||||||
| modelName: string, | ||||||||||||||||||||
| authType: AuthType, | ||||||||||||||||||||
| availableModels: AvailableModel[], | ||||||||||||||||||||
|
|
@@ -110,7 +113,9 @@ function formatUnavailableModelMessage( | |||||||||||||||||||
| ? FAST_MODEL_CONFIGURATION_HINT | ||||||||||||||||||||
| : kind === 'Vision model' | ||||||||||||||||||||
| ? VISION_MODEL_CONFIGURATION_HINT | ||||||||||||||||||||
| : MAIN_MODEL_CONFIGURATION_HINT; | ||||||||||||||||||||
| : kind === 'Compaction model' | ||||||||||||||||||||
| ? COMPACTION_MODEL_CONFIGURATION_HINT | ||||||||||||||||||||
| : MAIN_MODEL_CONFIGURATION_HINT; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return ( | ||||||||||||||||||||
| `${kind} '${modelName}' is not available for auth type '${authType}'.\n` + | ||||||||||||||||||||
|
|
@@ -119,10 +124,10 @@ function formatUnavailableModelMessage( | |||||||||||||||||||
| ); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Fast and vision share the same "not configured for any auth type" message | ||||||||||||||||||||
| // Fast, vision, and compaction share the same "not configured for any auth type" message | ||||||||||||||||||||
| // shape, differing only in the label and the configuration hint. | ||||||||||||||||||||
| function formatUnavailableAuxModelMessage( | ||||||||||||||||||||
| label: 'Fast model' | 'Vision model', | ||||||||||||||||||||
| label: 'Fast model' | 'Vision model' | 'Compaction model', | ||||||||||||||||||||
| modelName: string, | ||||||||||||||||||||
| availableModels: AvailableModel[], | ||||||||||||||||||||
| hint: string, | ||||||||||||||||||||
|
|
@@ -166,6 +171,18 @@ function formatUnavailableVisionModelMessage( | |||||||||||||||||||
| ); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function formatUnavailableCompactionModelMessage( | ||||||||||||||||||||
| modelName: string, | ||||||||||||||||||||
| availableModels: AvailableModel[], | ||||||||||||||||||||
| ): string { | ||||||||||||||||||||
| return formatUnavailableAuxModelMessage( | ||||||||||||||||||||
| 'Compaction model', | ||||||||||||||||||||
| modelName, | ||||||||||||||||||||
| availableModels, | ||||||||||||||||||||
| COMPACTION_MODEL_CONFIGURATION_HINT, | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function formatAmbiguousVisionModelMessage( | ||||||||||||||||||||
| modelName: string, | ||||||||||||||||||||
| matchingModels: AvailableModel[], | ||||||||||||||||||||
|
|
@@ -243,15 +260,15 @@ function formatUnavailableVoiceModelMessage( | |||||||||||||||||||
| // Get an array of the available model IDs as strings, filtered by mode | ||||||||||||||||||||
| function getAvailableModelIds( | ||||||||||||||||||||
| context: CommandContext, | ||||||||||||||||||||
| mode: 'main' | 'fast' | 'voice' | 'vision' = 'main', | ||||||||||||||||||||
| mode: 'main' | 'fast' | 'voice' | 'vision' | 'compaction' = 'main', | ||||||||||||||||||||
| ) { | ||||||||||||||||||||
| const { services } = context; | ||||||||||||||||||||
| const { config } = services; | ||||||||||||||||||||
| if (!config) { | ||||||||||||||||||||
| return []; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| const availableModels = config.getAvailableModels().filter((m) => { | ||||||||||||||||||||
| if (mode === 'fast') return !m.voiceOnly; | ||||||||||||||||||||
| if (mode === 'fast' || mode === 'compaction') return !m.voiceOnly; | ||||||||||||||||||||
| if (mode === 'voice') return !m.fastOnly; | ||||||||||||||||||||
| // 'vision' and 'main' both exclude fast/voice-only models. | ||||||||||||||||||||
| return !m.fastOnly && !m.voiceOnly; | ||||||||||||||||||||
|
|
@@ -264,10 +281,10 @@ export const modelCommand: SlashCommand = { | |||||||||||||||||||
| completionPriority: 100, | ||||||||||||||||||||
| 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, or [model-id] [prompt] to run a one-off prompt on another model; the inline prompt is sent verbatim without @file expansion).', | ||||||||||||||||||||
| '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, or [model-id] [prompt] to run a one-off prompt on another model; the inline prompt is sent verbatim without @file expansion).', | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| argumentHint: '[--fast|--voice|--vision] [<model-id>] | <model-id> <prompt>', | ||||||||||||||||||||
| argumentHint: '[--fast|--voice|--vision|--compaction] [<model-id>] | <model-id> <prompt>', | ||||||||||||||||||||
| kind: CommandKind.BUILT_IN, | ||||||||||||||||||||
| supportedModes: ['interactive', 'non_interactive', 'acp'] as const, | ||||||||||||||||||||
| completion: async (context, partialArg) => { | ||||||||||||||||||||
|
|
@@ -289,13 +306,19 @@ export const modelCommand: SlashCommand = { | |||||||||||||||||||
| 'Set the image-capable model used to transcribe images for a text-only main model', | ||||||||||||||||||||
| ), | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| { | ||||||||||||||||||||
| value: '--compaction', | ||||||||||||||||||||
| description: t( | ||||||||||||||||||||
| 'Set the model used for chat compression (auto-compaction)', | ||||||||||||||||||||
| ), | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| ].filter((item) => item.value.startsWith(partialArg)); | ||||||||||||||||||||
| if (flagCompletions.length > 0) { | ||||||||||||||||||||
| return flagCompletions; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| const trimmed = partialArg.trim(); | ||||||||||||||||||||
| if (trimmed) { | ||||||||||||||||||||
| let mode: 'main' | 'fast' | 'voice' | 'vision' = 'main'; | ||||||||||||||||||||
| let mode: 'main' | 'fast' | 'voice' | 'vision' | 'compaction' = 'main'; | ||||||||||||||||||||
| let modelPrefix = trimmed; | ||||||||||||||||||||
| if (trimmed.startsWith('--fast ')) { | ||||||||||||||||||||
| mode = 'fast'; | ||||||||||||||||||||
|
|
@@ -306,6 +329,9 @@ export const modelCommand: SlashCommand = { | |||||||||||||||||||
| } else if (trimmed.startsWith('--vision ')) { | ||||||||||||||||||||
| mode = 'vision'; | ||||||||||||||||||||
| modelPrefix = trimmed.slice('--vision '.length); | ||||||||||||||||||||
| } else if (trimmed.startsWith('--compaction ')) { | ||||||||||||||||||||
| mode = 'compaction'; | ||||||||||||||||||||
| modelPrefix = trimmed.slice('--compaction '.length); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| return getAvailableModelIds(context, mode).filter((id) => | ||||||||||||||||||||
| id.startsWith(modelPrefix), | ||||||||||||||||||||
|
|
@@ -608,6 +634,85 @@ export const modelCommand: SlashCommand = { | |||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const isCompactionModelCommand = | ||||||||||||||||||||
| args === '--compaction' || args.startsWith('--compaction '); | ||||||||||||||||||||
| if (isCompactionModelCommand) { | ||||||||||||||||||||
| const modelName = args.replace('--compaction', '').trim(); | ||||||||||||||||||||
| if (!modelName) { | ||||||||||||||||||||
| if (context.executionMode !== 'interactive') { | ||||||||||||||||||||
| const compactionModel = | ||||||||||||||||||||
| context.services.settings?.merged?.compactionModel?.trim() || | ||||||||||||||||||||
| t('not set (falls back to fast model, then main model)'); | ||||||||||||||||||||
| return { | ||||||||||||||||||||
| type: 'message', | ||||||||||||||||||||
| messageType: 'info', | ||||||||||||||||||||
| content: t( | ||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Misleading "clear" instruction — no clear mechanism exists This message tells users to use Either implement the clear path (mirroring how — qwen3.7-max via Qwen Code /review |
||||||||||||||||||||
| 'Current compaction model: {{compactionModel}}\nUse "/model --compaction <model-id>" to set compaction model, or "/model --compaction " to clear the override.', | ||||||||||||||||||||
| { compactionModel }, | ||||||||||||||||||||
| ), | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| return { | ||||||||||||||||||||
| type: 'dialog', | ||||||||||||||||||||
| dialog: 'compaction-model', | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if (!settings) { | ||||||||||||||||||||
| return { | ||||||||||||||||||||
| type: 'message', | ||||||||||||||||||||
| messageType: 'error', | ||||||||||||||||||||
| content: t('Settings service not available.'), | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const selector = (() => { | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| return resolveModelId(modelName); | ||||||||||||||||||||
| } catch { | ||||||||||||||||||||
| return undefined; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| })(); | ||||||||||||||||||||
| if (!selector) { | ||||||||||||||||||||
| return { | ||||||||||||||||||||
| type: 'message', | ||||||||||||||||||||
| messageType: 'error', | ||||||||||||||||||||
| content: formatUnavailableCompactionModelMessage(modelName, []), | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const availableModels = ( | ||||||||||||||||||||
| selector.authType | ||||||||||||||||||||
| ? config.getAvailableModelsForAuthType(selector.authType) | ||||||||||||||||||||
| : config.getAllConfiguredModels() | ||||||||||||||||||||
| ).filter((m) => !m.voiceOnly); | ||||||||||||||||||||
| if (!availableModels.some((model) => model.id === selector.modelId)) { | ||||||||||||||||||||
| return { | ||||||||||||||||||||
| type: 'message', | ||||||||||||||||||||
| messageType: 'error', | ||||||||||||||||||||
| content: selector.authType | ||||||||||||||||||||
| ? formatUnavailableModelMessage( | ||||||||||||||||||||
| 'Compaction model', | ||||||||||||||||||||
| selector.modelId, | ||||||||||||||||||||
| selector.authType, | ||||||||||||||||||||
| availableModels, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| : formatUnavailableCompactionModelMessage( | ||||||||||||||||||||
| modelName, | ||||||||||||||||||||
| availableModels, | ||||||||||||||||||||
| ), | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| persistSetting(settings, 'compactionModel', modelName); | ||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Zero test coverage for 85+ lines of new command handling code with no tests. The existing
— qwen3.7-max via Qwen Code /review
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] Cross-provider model ambiguity — raw The
Suggested change
— qwen3.7-max via Qwen Code /review |
||||||||||||||||||||
| // Sync runtime Config so the compression service picks it up immediately. | ||||||||||||||||||||
| config.setCompactionModel(modelName); | ||||||||||||||||||||
| return { | ||||||||||||||||||||
| type: 'message', | ||||||||||||||||||||
| messageType: 'info', | ||||||||||||||||||||
| content: t('Compaction Model') + ': ' + modelName, | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const contentGeneratorConfig = config.getContentGeneratorConfig(); | ||||||||||||||||||||
| if (!contentGeneratorConfig) { | ||||||||||||||||||||
| return { | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] The newly added filter condition
(isVisionModelMode || !m.visionOnly) && // ← existing line: shows all models in vision mode
(isCompactionModelMode || !m.visionOnly), // ← new line added by this PRBoth conditions are ANDed. When
The combined AND result is (isCompactionModelMode || isVisionModelMode || !m.visionOnly),— claude-sonnet-4-6 via Qwen Code /review Generated by Claude Code
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] The fast/voice/vision handlers each early-return with mode-specific persistence ( 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, — claude-sonnet-4-6 via Qwen Code /review Generated by Claude Code |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -115,6 +115,7 @@ interface ModelDialogProps { | |||||
| isFastModelMode?: boolean; | ||||||
| isVoiceModelMode?: boolean; | ||||||
| isVisionModelMode?: boolean; | ||||||
| isCompactionModelMode?: boolean; | ||||||
| } | ||||||
|
|
||||||
| function maskApiKey(apiKey: string | undefined): string { | ||||||
|
|
@@ -239,6 +240,7 @@ export function ModelDialog({ | |||||
| isFastModelMode, | ||||||
| isVoiceModelMode, | ||||||
| isVisionModelMode, | ||||||
| isCompactionModelMode, | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Three
Each should mirror the pattern used by — qwen3.7-max via Qwen Code /review
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Left-arrow dismiss and preferred-entry gaps for Two integration points missed beyond the existing gaps already noted on this component:
— qwen3.7-max via Qwen Code /review |
||||||
| }: ModelDialogProps): React.JSX.Element { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] The callback has handlers for Add an
Also add — qwen3.7-max via Qwen Code /review |
||||||
| const config = useContext(ConfigContext); | ||||||
| const uiState = useContext(UIStateContext); | ||||||
|
|
@@ -261,7 +263,9 @@ export function ModelDialog({ | |||||
| (m.authType !== AuthType.QWEN_OAUTH || | ||||||
| authType === AuthType.QWEN_OAUTH) && | ||||||
| (isFastModelMode || !m.fastOnly) && | ||||||
| (isVoiceModelMode || !m.voiceOnly), | ||||||
| (isVoiceModelMode || !m.voiceOnly) && | ||||||
| (isVisionModelMode || !m.visionOnly) && | ||||||
| (isCompactionModelMode || !m.visionOnly), | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] Copy-paste bug: The compaction filter reuses
Suggested change
— qwen3.7-max via Qwen Code /review |
||||||
| ); | ||||||
|
|
||||||
| // Group registry models by authType | ||||||
|
|
@@ -315,7 +319,14 @@ export function ModelDialog({ | |||||
| } | ||||||
|
|
||||||
| return result; | ||||||
| }, [authType, config, isFastModelMode, isVoiceModelMode]); | ||||||
| }, [ | ||||||
| authType, | ||||||
| config, | ||||||
| isFastModelMode, | ||||||
| isVoiceModelMode, | ||||||
| isVisionModelMode, | ||||||
| isCompactionModelMode, | ||||||
| ]); | ||||||
|
|
||||||
| const MODEL_OPTIONS = useMemo( | ||||||
| () => | ||||||
|
|
@@ -411,8 +422,11 @@ export function ModelDialog({ | |||||
| // Check if current model is a runtime model | ||||||
| // Runtime snapshot ID is already in $runtime|${authType}|${modelId} format | ||||||
| const activeRuntimeSnapshot = | ||||||
| isFastModelMode || isVoiceModelMode || isVisionModelMode | ||||||
| ? undefined // fast/voice/vision models are never runtime model selections | ||||||
| isFastModelMode || | ||||||
| isVoiceModelMode || | ||||||
| isVisionModelMode || | ||||||
| isCompactionModelMode | ||||||
| ? undefined // fast/voice/vision/compaction models are never runtime model selections | ||||||
| : config?.getActiveRuntimeModelSnapshot?.(); | ||||||
| const currentBaseUrl = config | ||||||
| ?.getModelsConfig() | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Critical] Missing auth-type validation gate
The
--fasthandler (line 463) explicitly checksconfig.getContentGeneratorConfig()?.authTypeand returns an error when unavailable. This--compactionhandler skips that gate entirely, going straight toresolveModelId(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--fastshows.— qwen3.7-max via Qwen Code /review