From f4cf903ba5caa276ebafad8476fdcfeb22596dc5 Mon Sep 17 00:00:00 2001 From: "Qwen3.6 Plus agent" Date: Fri, 3 Jul 2026 09:28:12 +0300 Subject: [PATCH 1/5] fix(web-shell): encode vision model selection & polish picker Address Wenshao's review comments on #6209: [Critical] Encode vision model selection before persisting - handleVisionModelSelect now strips ACP (authType) suffix and stores as authType:modelId format expected by core's resolveVisionModelSelection() - Without this, picker selections silently fail to resolve when the same model ID appears on multiple providers [Suggestion] Add currentVisionModel derivation - Mirror currentVoiceModel pattern so the picker highlights the active vision model instead of falling back to the main model [Suggestion] Extract MODE_TITLE_KEY record for exhaustive dispatch - Replace duplicated 4-way ternary in App.tsx dialog title with a single Record lookup - Replace if/else if/else onSelect chain with a handlers record that would fail at compile time if a new mode is added without a handler [Suggestion] Add settings.label/description.visionModel i18n keys - Add to both EN and ZH locales so Chinese users see proper labels in the Settings dialog Files changed: - App.tsx: encoding fix, currentVisionModel, MODE_TITLE_KEY, handlers record - i18n.tsx: visionModel label + description (EN + ZH) --- packages/web-shell/client/App.tsx | 55 +++++++++++++++++++----------- packages/web-shell/client/i18n.tsx | 6 ++++ 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index e3baacbaf0b..d61ab326f5e 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -218,6 +218,14 @@ const COMPACT_MODE_SETTING_KEY = 'ui.compactMode'; const HIDE_TIPS_SETTING_KEY = 'ui.hideTips'; const HIDDEN_COMPOSER_MODEL_IDS = new Set(['coder-model(qwen-oauth)']); +/** Maps each ModelDialogMode to its i18n title key — single source of truth. */ +const MODE_TITLE_KEY: Record = { + main: 'model.select', + fast: 'model.setFast', + voice: 'model.setVoice', + vision: 'model.setVision', +}; + function isVisibleComposerModel(model: { id: string }): boolean { return !HIDDEN_COMPOSER_MODEL_IDS.has(model.id); } @@ -1650,6 +1658,12 @@ export function App({ )?.values.effective; return typeof value === 'string' && value.trim() ? value.trim() : undefined; })(); + const currentVisionModel = (() => { + const value = workspaceSettings.find( + (setting) => setting.key === 'visionModel', + )?.values.effective; + return typeof value === 'string' && value.trim() ? value.trim() : undefined; + })(); const shellOutputMaxLines = resolveShellOutputMaxLines(workspaceSettings); const [compactMode, setCompactMode] = useState(false); const compactModeRef = useRef(compactMode); @@ -3315,7 +3329,11 @@ export function App({ const handleVisionModelSelect = useCallback( (modelId: string) => { - setWorkspaceSetting('workspace', 'visionModel', modelId).catch( + // Model IDs from the picker arrive in ACP format: `modelId(authType)`. + // Core's resolveVisionModelSelection() expects `authType:modelId`. + const match = modelId.match(/^(.+)\(([^()]+)\)$/); + const encoded = match ? `${match[2]}:${match[1]}` : modelId; + setWorkspaceSetting('workspace', 'visionModel', encoded).catch( (error: unknown) => reportError(error, t('model.setVision')), ); }, @@ -3456,15 +3474,7 @@ export function App({ )} {modelDialogMode && ( setModelDialogMode(null)} > @@ -3472,18 +3482,23 @@ export function App({ mode={modelDialogMode} models={modelDialogMode === 'voice' ? voiceModels : undefined} currentModelId={ - modelDialogMode === 'voice' ? currentVoiceModel : undefined + modelDialogMode === 'voice' + ? currentVoiceModel + : modelDialogMode === 'vision' + ? currentVisionModel + : undefined } onSelect={(modelId) => { - if (modelDialogMode === 'fast') { - handleFastModelSelect(modelId); - } else if (modelDialogMode === 'voice') { - handleVoiceModelSelect(modelId); - } else if (modelDialogMode === 'vision') { - handleVisionModelSelect(modelId); - } else { - handleModelSelect(modelId); - } + const handlers: Record< + ModelDialogMode, + (id: string) => void + > = { + main: handleModelSelect, + fast: handleFastModelSelect, + voice: handleVoiceModelSelect, + vision: handleVisionModelSelect, + }; + handlers[modelDialogMode ?? 'main'](modelId); setModelDialogMode(null); }} /> diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index b09fb1abfea..77d361473f9 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -1209,6 +1209,9 @@ const EN: Messages = { 'Frontend-only chat content width. Stored in this browser.', 'settings.option.ui.chatWidth.1000': 'Regular', 'settings.option.ui.chatWidth.wide': 'Ultra wide', + 'settings.label.visionModel': 'Vision Model', + 'settings.description.visionModel': + 'Image-capable model used as the vision bridge. Leave empty to auto-select.', 'welcome.changeModel': '(/model to change)', 'welcome.defaultModel': 'unknown model', 'welcome.modeHint': 'Shift+Tab or /approval-mode', @@ -2442,6 +2445,9 @@ const ZH: Messages = { 'settings.label.fastModel': '快速模型', 'settings.description.fastModel': '用于生成提示建议和推测执行的模型。留空则使用主模型。较小/更快的模型(例如 qwen3-coder-flash)可以降低延迟和成本。', + 'settings.label.visionModel': '视觉模型', + 'settings.description.visionModel': + '用于视觉桥接的图像能力模型。留空则自动选择。', 'settings.label.context.fileFiltering.respectGitIgnore': '遵守 .gitignore', 'settings.description.context.fileFiltering.respectGitIgnore': '搜索时遵守 .gitignore 文件。', From b22641c42cb0c7df1c141ee8daa3ced0d669cff1 Mon Sep 17 00:00:00 2001 From: "Qwen3.6 Plus agent" Date: Fri, 3 Jul 2026 09:28:12 +0300 Subject: [PATCH 2/5] fix(web-shell): encode vision model selection & polish picker Address Wenshao's review comments on #6209: [Critical] Encode vision model selection before persisting - handleVisionModelSelect now strips ACP (authType) suffix and stores as authType:modelId format expected by core's resolveVisionModelSelection() - Without this, picker selections silently fail to resolve when the same model ID appears on multiple providers [Suggestion] Add currentVisionModel derivation - Mirror currentVoiceModel pattern so the picker highlights the active vision model instead of falling back to the main model [Suggestion] Extract MODE_TITLE_KEY record for exhaustive dispatch - Replace duplicated 4-way ternary in App.tsx dialog title with a single Record lookup - Replace if/else if/else onSelect chain with a handlers record that would fail at compile time if a new mode is added without a handler [Suggestion] Add settings.label/description.visionModel i18n keys - Add to both EN and ZH locales so Chinese users see proper labels in the Settings dialog Files changed: - App.tsx: encoding fix, currentVisionModel, MODE_TITLE_KEY, handlers record - i18n.tsx: visionModel label + description (EN + ZH) --- packages/web-shell/client/App.tsx | 55 +++++++++++++++++++----------- packages/web-shell/client/i18n.tsx | 6 ++++ 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index e3baacbaf0b..d61ab326f5e 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -218,6 +218,14 @@ const COMPACT_MODE_SETTING_KEY = 'ui.compactMode'; const HIDE_TIPS_SETTING_KEY = 'ui.hideTips'; const HIDDEN_COMPOSER_MODEL_IDS = new Set(['coder-model(qwen-oauth)']); +/** Maps each ModelDialogMode to its i18n title key — single source of truth. */ +const MODE_TITLE_KEY: Record = { + main: 'model.select', + fast: 'model.setFast', + voice: 'model.setVoice', + vision: 'model.setVision', +}; + function isVisibleComposerModel(model: { id: string }): boolean { return !HIDDEN_COMPOSER_MODEL_IDS.has(model.id); } @@ -1650,6 +1658,12 @@ export function App({ )?.values.effective; return typeof value === 'string' && value.trim() ? value.trim() : undefined; })(); + const currentVisionModel = (() => { + const value = workspaceSettings.find( + (setting) => setting.key === 'visionModel', + )?.values.effective; + return typeof value === 'string' && value.trim() ? value.trim() : undefined; + })(); const shellOutputMaxLines = resolveShellOutputMaxLines(workspaceSettings); const [compactMode, setCompactMode] = useState(false); const compactModeRef = useRef(compactMode); @@ -3315,7 +3329,11 @@ export function App({ const handleVisionModelSelect = useCallback( (modelId: string) => { - setWorkspaceSetting('workspace', 'visionModel', modelId).catch( + // Model IDs from the picker arrive in ACP format: `modelId(authType)`. + // Core's resolveVisionModelSelection() expects `authType:modelId`. + const match = modelId.match(/^(.+)\(([^()]+)\)$/); + const encoded = match ? `${match[2]}:${match[1]}` : modelId; + setWorkspaceSetting('workspace', 'visionModel', encoded).catch( (error: unknown) => reportError(error, t('model.setVision')), ); }, @@ -3456,15 +3474,7 @@ export function App({ )} {modelDialogMode && ( setModelDialogMode(null)} > @@ -3472,18 +3482,23 @@ export function App({ mode={modelDialogMode} models={modelDialogMode === 'voice' ? voiceModels : undefined} currentModelId={ - modelDialogMode === 'voice' ? currentVoiceModel : undefined + modelDialogMode === 'voice' + ? currentVoiceModel + : modelDialogMode === 'vision' + ? currentVisionModel + : undefined } onSelect={(modelId) => { - if (modelDialogMode === 'fast') { - handleFastModelSelect(modelId); - } else if (modelDialogMode === 'voice') { - handleVoiceModelSelect(modelId); - } else if (modelDialogMode === 'vision') { - handleVisionModelSelect(modelId); - } else { - handleModelSelect(modelId); - } + const handlers: Record< + ModelDialogMode, + (id: string) => void + > = { + main: handleModelSelect, + fast: handleFastModelSelect, + voice: handleVoiceModelSelect, + vision: handleVisionModelSelect, + }; + handlers[modelDialogMode ?? 'main'](modelId); setModelDialogMode(null); }} /> diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index b09fb1abfea..77d361473f9 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -1209,6 +1209,9 @@ const EN: Messages = { 'Frontend-only chat content width. Stored in this browser.', 'settings.option.ui.chatWidth.1000': 'Regular', 'settings.option.ui.chatWidth.wide': 'Ultra wide', + 'settings.label.visionModel': 'Vision Model', + 'settings.description.visionModel': + 'Image-capable model used as the vision bridge. Leave empty to auto-select.', 'welcome.changeModel': '(/model to change)', 'welcome.defaultModel': 'unknown model', 'welcome.modeHint': 'Shift+Tab or /approval-mode', @@ -2442,6 +2445,9 @@ const ZH: Messages = { 'settings.label.fastModel': '快速模型', 'settings.description.fastModel': '用于生成提示建议和推测执行的模型。留空则使用主模型。较小/更快的模型(例如 qwen3-coder-flash)可以降低延迟和成本。', + 'settings.label.visionModel': '视觉模型', + 'settings.description.visionModel': + '用于视觉桥接的图像能力模型。留空则自动选择。', 'settings.label.context.fileFiltering.respectGitIgnore': '遵守 .gitignore', 'settings.description.context.fileFiltering.respectGitIgnore': '搜索时遵守 .gitignore 文件。', From 4ad0e11afb2e3ee384adfe77991331b0ba1be998 Mon Sep 17 00:00:00 2001 From: DeepSeek V4 Pro agent Date: Fri, 3 Jul 2026 12:18:51 +0300 Subject: [PATCH 3/5] fix(web-shell): address review comments for vision model picker encoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract encodeVisionModelForSetting / decodeVisionModelForPicker into shared utils/modelEncoding.ts so they can be tested in isolation - Add 17 unit tests covering ACP encoding, colon-bearing IDs, empty parens passthrough, and round-trip identity - Memoize modelHandlers record with useMemo to avoid re-allocation on every model picker click - Replace dead fallback (?? 'main') — the outer modelDialogMode guard already ensures non-null, so use an explicit if-guard instead --- packages/web-shell/client/App.tsx | 50 ++++++--- .../client/utils/modelEncoding.test.ts | 106 ++++++++++++++++++ .../web-shell/client/utils/modelEncoding.ts | 44 ++++++++ 3 files changed, 185 insertions(+), 15 deletions(-) create mode 100644 packages/web-shell/client/utils/modelEncoding.test.ts create mode 100644 packages/web-shell/client/utils/modelEncoding.ts diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index c01ce073f0b..f6523cf7589 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -97,6 +97,12 @@ import { filterModelSwitchMessages } from './utils/modelSwitchMessages'; import { decideEscapeIntent } from './utils/escapeIntent'; import type { SkillInfo } from './completions/slashCompletion'; import { collectSystemInfo } from './utils/systemInfo'; +import { + decodeVisionModelForPicker, + encodeFastModelForSetting, + encodeVisionModelForSetting, + extractBareModelId, +} from './utils/modelEncoding'; import { appendOrDeferLocalUserMessage } from './utils/localCommandQueue'; import { QueuedPromptDisplay } from './components/QueuedPromptDisplay'; import { useQueuedPrompts } from './hooks/useQueuedPrompts'; @@ -1662,7 +1668,8 @@ export function App({ const value = workspaceSettings.find( (setting) => setting.key === 'visionModel', )?.values.effective; - return typeof value === 'string' && value.trim() ? value.trim() : undefined; + if (typeof value !== 'string' || !value.trim()) return undefined; + return decodeVisionModelForPicker(value.trim()); })(); const shellOutputMaxLines = resolveShellOutputMaxLines(workspaceSettings); const [compactMode, setCompactMode] = useState(false); @@ -3318,7 +3325,10 @@ export function App({ blockLocalCommandDuringTurn(); return; } - sendPrompt(`/model --fast ${modelId}`).catch((error: unknown) => { + // Model IDs from the picker arrive in ACP format: `modelId(authType)`. + // Core's resolveFastModelSelector() expects `authType:modelId`. + const encoded = encodeFastModelForSetting(modelId); + sendPrompt(`/model --fast ${encoded}`).catch((error: unknown) => { reportError(error, 'Failed to switch fast model'); }); }, @@ -3327,7 +3337,10 @@ export function App({ const handleVoiceModelSelect = useCallback( (modelId: string) => { - setWorkspaceSetting('workspace', 'voiceModel', modelId).catch( + // Model IDs from the picker arrive in ACP format: `modelId(authType)`. + // Voice model resolution expects bare model IDs (not authType:modelId). + const bareModelId = extractBareModelId(modelId); + setWorkspaceSetting('workspace', 'voiceModel', bareModelId).catch( (error: unknown) => reportError(error, t('model.setVoice')), ); }, @@ -3338,8 +3351,7 @@ export function App({ (modelId: string) => { // Model IDs from the picker arrive in ACP format: `modelId(authType)`. // Core's resolveVisionModelSelection() expects `authType:modelId`. - const match = modelId.match(/^(.+)\(([^()]+)\)$/); - const encoded = match ? `${match[2]}:${match[1]}` : modelId; + const encoded = encodeVisionModelForSetting(modelId); setWorkspaceSetting('workspace', 'visionModel', encoded).catch( (error: unknown) => reportError(error, t('model.setVision')), ); @@ -3347,6 +3359,21 @@ export function App({ [reportError, setWorkspaceSetting, t], ); + const modelHandlers: Record void> = useMemo( + () => ({ + main: handleModelSelect, + fast: handleFastModelSelect, + voice: handleVoiceModelSelect, + vision: handleVisionModelSelect, + }), + [ + handleModelSelect, + handleFastModelSelect, + handleVoiceModelSelect, + handleVisionModelSelect, + ], + ); + const commands = useMemo(() => { const skillNames = new Set(connection.skills ?? []); return mergeCommands(connection.commands ?? [], getLocalCommands(t)) @@ -3496,16 +3523,9 @@ export function App({ : undefined } onSelect={(modelId) => { - const handlers: Record< - ModelDialogMode, - (id: string) => void - > = { - main: handleModelSelect, - fast: handleFastModelSelect, - voice: handleVoiceModelSelect, - vision: handleVisionModelSelect, - }; - handlers[modelDialogMode ?? 'main'](modelId); + if (modelDialogMode) { + modelHandlers[modelDialogMode](modelId); + } setModelDialogMode(null); }} /> diff --git a/packages/web-shell/client/utils/modelEncoding.test.ts b/packages/web-shell/client/utils/modelEncoding.test.ts new file mode 100644 index 00000000000..921dbaf2d90 --- /dev/null +++ b/packages/web-shell/client/utils/modelEncoding.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, it } from 'vitest'; +import { + encodeVisionModelForSetting, + encodeFastModelForSetting, + extractBareModelId, + decodeVisionModelForPicker, +} from './modelEncoding'; + +describe('encodeVisionModelForSetting', () => { + it('encodes standard ACP format', () => { + expect(encodeVisionModelForSetting('qwen-max(qwen-oauth)')).toBe( + 'qwen-oauth:qwen-max', + ); + }); + + it('passes through non-ACP format unchanged', () => { + expect(encodeVisionModelForSetting('plain-model')).toBe('plain-model'); + }); + + it('passes through empty parens unchanged', () => { + expect(encodeVisionModelForSetting('model()')).toBe('model()'); + }); + + it('passes through colon-bearing IDs unchanged', () => { + expect(encodeVisionModelForSetting('openai:gpt-4o')).toBe('openai:gpt-4o'); + }); + + it('handles nested parentheses by matching outermost ACP pattern', () => { + // The regex matches the outermost group: modelId(authType) + expect(encodeVisionModelForSetting('model(name)(extra)')).toBe( + 'extra:model(name)', + ); + }); +}); + +describe('encodeFastModelForSetting', () => { + it('encodes standard ACP format', () => { + expect(encodeFastModelForSetting('qwen-max(qwen-oauth)')).toBe( + 'qwen-oauth:qwen-max', + ); + }); + + it('passes through non-ACP format unchanged', () => { + expect(encodeFastModelForSetting('plain-model')).toBe('plain-model'); + }); +}); + +describe('extractBareModelId', () => { + it('extracts bare model ID from ACP format', () => { + expect(extractBareModelId('qwen-max(qwen-oauth)')).toBe('qwen-max'); + }); + + it('passes through non-ACP format unchanged', () => { + expect(extractBareModelId('plain-model')).toBe('plain-model'); + }); + + it('passes through empty parens unchanged', () => { + // The regex requires at least one char in each capture group, + // so '()' does not match and passes through. + expect(extractBareModelId('()')).toBe('()'); + }); +}); + +describe('decodeVisionModelForPicker', () => { + it('decodes authType:modelId back to ACP format', () => { + expect(decodeVisionModelForPicker('qwen-oauth:qwen-max')).toBe( + 'qwen-max(qwen-oauth)', + ); + }); + + it('handles colon-bearing model IDs — splits on first colon only', () => { + expect(decodeVisionModelForPicker('openai:gpt-4o:online')).toBe( + 'gpt-4o:online(openai)', + ); + }); + + it('passes through values without a colon', () => { + expect(decodeVisionModelForPicker('plain-model')).toBe('plain-model'); + }); + + it('passes through ACP-formatted values unchanged', () => { + expect(decodeVisionModelForPicker('qwen-max(qwen-oauth)')).toBe( + 'qwen-max(qwen-oauth)', + ); + }); + + it('passes through empty string unchanged', () => { + expect(decodeVisionModelForPicker('')).toBe(''); + }); +}); + +describe('round-trip: encode then decode', () => { + it('preserves the original ACP format', () => { + const original = 'qwen-vl-max(qwen-oauth)'; + const encoded = encodeVisionModelForSetting(original); + const decoded = decodeVisionModelForPicker(encoded); + expect(decoded).toBe(original); + }); + + it('preserves colon-bearing IDs after round-trip', () => { + const original = 'gpt-4o:new-model(openai)'; + const encoded = encodeVisionModelForSetting(original); + const decoded = decodeVisionModelForPicker(encoded); + expect(decoded).toBe(original); + }); +}); diff --git a/packages/web-shell/client/utils/modelEncoding.ts b/packages/web-shell/client/utils/modelEncoding.ts new file mode 100644 index 00000000000..433cde91f95 --- /dev/null +++ b/packages/web-shell/client/utils/modelEncoding.ts @@ -0,0 +1,44 @@ +/** + * Encodes a model ID from ACP format (modelId(authType)) to storage format + * (authType:modelId). Core's resolveVisionModelSelection() expects this format. + * If the input is not in ACP format, returns it unchanged. + */ +export function encodeVisionModelForSetting(modelId: string): string { + const match = modelId.match(/^(.+)\(([^()]+)\)$/); + return match ? `${match[2]}:${match[1]}` : modelId; +} + +/** + * Encodes a fast model ID from ACP format (modelId(authType)) to storage format + * (authType:modelId). Core's resolveFastModelSelector() expects this format. + * If the input is not in ACP format, returns it unchanged. + */ +export function encodeFastModelForSetting(modelId: string): string { + const match = modelId.match(/^(.+)\(([^()]+)\)$/); + return match ? `${match[2]}:${match[1]}` : modelId; +} + +/** + * Extracts the bare model ID from ACP format (modelId(authType)). + * Voice model resolution expects bare model IDs (not authType:modelId). + * If the input is not in ACP format, returns it unchanged. + */ +export function extractBareModelId(modelId: string): string { + const match = modelId.match(/^(.+)\(([^()]+)\)$/); + return match ? match[1] : modelId; +} + +/** + * Decodes a stored model ID from authType:modelId format back to ACP format + * (modelId(authType)). Used for picker comparison where model IDs are in ACP + * format. Splits on the first colon — safe for colon-bearing model IDs + * (e.g., 'openai:gpt-4o:online' → 'gpt-4o:online(openai)'). + * If the input has no colon, returns it unchanged. + */ +export function decodeVisionModelForPicker(storedValue: string): string { + const colonIdx = storedValue.indexOf(':'); + if (colonIdx > 0) { + return `${storedValue.slice(colonIdx + 1)}(${storedValue.slice(0, colonIdx)})`; + } + return storedValue; +} From fd8e83a8985d13121cb73124a25aa4b72d297200 Mon Sep 17 00:00:00 2001 From: DeepSeek V4 Pro agent Date: Fri, 3 Jul 2026 12:36:51 +0300 Subject: [PATCH 4/5] test(web-shell): add edge-case tests for model encoding functions - Add passthrough tests for already-encoded colon format - Add malformed input tests (bare authType, unclosed paren, double-parens) - Add leadin-colon malformed input test for decode - Add empty string passthrough test - 23 encoding tests passing (up from 17), full suite: 735 passing --- .../client/utils/modelEncoding.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/web-shell/client/utils/modelEncoding.test.ts b/packages/web-shell/client/utils/modelEncoding.test.ts index 921dbaf2d90..67449282951 100644 --- a/packages/web-shell/client/utils/modelEncoding.test.ts +++ b/packages/web-shell/client/utils/modelEncoding.test.ts @@ -31,6 +31,33 @@ describe('encodeVisionModelForSetting', () => { 'extra:model(name)', ); }); + + it('passes through already-encoded colon format unchanged', () => { + // If someone somehow passes an already-encoded authType:modelId, + // it should be left alone — not double-encoded. + expect(encodeVisionModelForSetting('qwen-oauth:qwen-vl-max')).toBe( + 'qwen-oauth:qwen-vl-max', + ); + }); + + it('passes through malformed — bare authType only', () => { + // '(authType)' has no modelId before the parens and no text + // before the opening paren that can serve as group 1, so the + // regex won't match and we get the input back unchanged. + expect(encodeVisionModelForSetting('(authType)')).toBe('(authType)'); + }); + + it('passes through malformed — unclosed paren', () => { + expect(encodeVisionModelForSetting('modelId(')).toBe('modelId('); + }); + + it('passes through malformed — double-parens inside', () => { + expect(encodeVisionModelForSetting('a((b))')).toBe('a((b))'); + }); + + it('passes through empty string unchanged', () => { + expect(encodeVisionModelForSetting('')).toBe(''); + }); }); describe('encodeFastModelForSetting', () => { @@ -87,6 +114,11 @@ describe('decodeVisionModelForPicker', () => { it('passes through empty string unchanged', () => { expect(decodeVisionModelForPicker('')).toBe(''); }); + + it('passes through leading-colon malformed input', () => { + // colonIdx === 0, which is not > 0, so passthrough + expect(decodeVisionModelForPicker(':modelId')).toBe(':modelId'); + }); }); describe('round-trip: encode then decode', () => { From 86d5a15f37d476ed5d25cbeda0c8a43b79839418 Mon Sep 17 00:00:00 2001 From: DeepSeek V4 Pro agent Date: Fri, 3 Jul 2026 15:12:24 +0300 Subject: [PATCH 5/5] =?UTF-8?q?fix:=20PR=20#6236=20follow-up=20=E2=80=94?= =?UTF-8?q?=20vision=20model=20encoding=20+=20fast=20model=20highlight?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - decodeVisionModelForPicker: strip \0baseUrl suffix before decoding to ACP - Remove dead encodeFastModelForSetting (fast picker strips ACP suffix before handler) - Add currentFastModel derivation + 'fast' branch to currentModelId ternary - Fix misleading voice handler comment (bare IDs, not ACP) - Replace unnecessary useMemo on modelHandlers with plain object Co-authored-by: atlarix-agent --- packages/web-shell/client/App.tsx | 43 +++++++++---------- .../client/utils/modelEncoding.test.ts | 35 +++++++++------ .../web-shell/client/utils/modelEncoding.ts | 25 +++-------- 3 files changed, 50 insertions(+), 53 deletions(-) diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index f6523cf7589..217886a127d 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -99,7 +99,6 @@ import type { SkillInfo } from './completions/slashCompletion'; import { collectSystemInfo } from './utils/systemInfo'; import { decodeVisionModelForPicker, - encodeFastModelForSetting, encodeVisionModelForSetting, extractBareModelId, } from './utils/modelEncoding'; @@ -1671,6 +1670,12 @@ export function App({ if (typeof value !== 'string' || !value.trim()) return undefined; return decodeVisionModelForPicker(value.trim()); })(); + const currentFastModel = (() => { + const value = workspaceSettings.find( + (setting) => setting.key === 'fastModel', + )?.values.effective; + return typeof value === 'string' && value.trim() ? value.trim() : undefined; + })(); const shellOutputMaxLines = resolveShellOutputMaxLines(workspaceSettings); const [compactMode, setCompactMode] = useState(false); const compactModeRef = useRef(compactMode); @@ -3325,10 +3330,10 @@ export function App({ blockLocalCommandDuringTurn(); return; } - // Model IDs from the picker arrive in ACP format: `modelId(authType)`. - // Core's resolveFastModelSelector() expects `authType:modelId`. - const encoded = encodeFastModelForSetting(modelId); - sendPrompt(`/model --fast ${encoded}`).catch((error: unknown) => { + // Model IDs from the picker arrive as bare model IDs (baseModelId), not + // ACP format. The model picker strips the (authType) suffix before + // calling this handler. + sendPrompt(`/model --fast ${modelId}`).catch((error: unknown) => { reportError(error, 'Failed to switch fast model'); }); }, @@ -3337,8 +3342,8 @@ export function App({ const handleVoiceModelSelect = useCallback( (modelId: string) => { - // Model IDs from the picker arrive in ACP format: `modelId(authType)`. - // Voice model resolution expects bare model IDs (not authType:modelId). + // Model IDs from the voice picker arrive as bare model IDs (baseModelId), + // not ACP format. extractVoiceModels() sets id to the baseModelId. const bareModelId = extractBareModelId(modelId); setWorkspaceSetting('workspace', 'voiceModel', bareModelId).catch( (error: unknown) => reportError(error, t('model.setVoice')), @@ -3359,20 +3364,12 @@ export function App({ [reportError, setWorkspaceSetting, t], ); - const modelHandlers: Record void> = useMemo( - () => ({ - main: handleModelSelect, - fast: handleFastModelSelect, - voice: handleVoiceModelSelect, - vision: handleVisionModelSelect, - }), - [ - handleModelSelect, - handleFastModelSelect, - handleVoiceModelSelect, - handleVisionModelSelect, - ], - ); + const modelHandlers: Record void> = { + main: handleModelSelect, + fast: handleFastModelSelect, + voice: handleVoiceModelSelect, + vision: handleVisionModelSelect, + }; const commands = useMemo(() => { const skillNames = new Set(connection.skills ?? []); @@ -3520,7 +3517,9 @@ export function App({ ? currentVoiceModel : modelDialogMode === 'vision' ? currentVisionModel - : undefined + : modelDialogMode === 'fast' + ? currentFastModel + : undefined } onSelect={(modelId) => { if (modelDialogMode) { diff --git a/packages/web-shell/client/utils/modelEncoding.test.ts b/packages/web-shell/client/utils/modelEncoding.test.ts index 67449282951..a493252d370 100644 --- a/packages/web-shell/client/utils/modelEncoding.test.ts +++ b/packages/web-shell/client/utils/modelEncoding.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from 'vitest'; import { encodeVisionModelForSetting, - encodeFastModelForSetting, extractBareModelId, decodeVisionModelForPicker, } from './modelEncoding'; @@ -60,18 +59,6 @@ describe('encodeVisionModelForSetting', () => { }); }); -describe('encodeFastModelForSetting', () => { - it('encodes standard ACP format', () => { - expect(encodeFastModelForSetting('qwen-max(qwen-oauth)')).toBe( - 'qwen-oauth:qwen-max', - ); - }); - - it('passes through non-ACP format unchanged', () => { - expect(encodeFastModelForSetting('plain-model')).toBe('plain-model'); - }); -}); - describe('extractBareModelId', () => { it('extracts bare model ID from ACP format', () => { expect(extractBareModelId('qwen-max(qwen-oauth)')).toBe('qwen-max'); @@ -119,6 +106,28 @@ describe('decodeVisionModelForPicker', () => { // colonIdx === 0, which is not > 0, so passthrough expect(decodeVisionModelForPicker(':modelId')).toBe(':modelId'); }); + + it('strips \\0baseUrl suffix before decoding', () => { + expect( + decodeVisionModelForPicker( + 'qwen-oauth:qwen-vl-max\0https://api.example.com', + ), + ).toBe('qwen-vl-max(qwen-oauth)'); + }); + + it('handles colon-bearing ID with \\0baseUrl suffix', () => { + expect( + decodeVisionModelForPicker( + 'openai:gpt-4o:online\0https://api.openai.com', + ), + ).toBe('gpt-4o:online(openai)'); + }); + + it('passes through bare ID with \\0baseUrl suffix but no colon', () => { + expect( + decodeVisionModelForPicker('plain-model\0https://api.example.com'), + ).toBe('plain-model'); + }); }); describe('round-trip: encode then decode', () => { diff --git a/packages/web-shell/client/utils/modelEncoding.ts b/packages/web-shell/client/utils/modelEncoding.ts index 433cde91f95..e5748c73b81 100644 --- a/packages/web-shell/client/utils/modelEncoding.ts +++ b/packages/web-shell/client/utils/modelEncoding.ts @@ -8,21 +8,6 @@ export function encodeVisionModelForSetting(modelId: string): string { return match ? `${match[2]}:${match[1]}` : modelId; } -/** - * Encodes a fast model ID from ACP format (modelId(authType)) to storage format - * (authType:modelId). Core's resolveFastModelSelector() expects this format. - * If the input is not in ACP format, returns it unchanged. - */ -export function encodeFastModelForSetting(modelId: string): string { - const match = modelId.match(/^(.+)\(([^()]+)\)$/); - return match ? `${match[2]}:${match[1]}` : modelId; -} - -/** - * Extracts the bare model ID from ACP format (modelId(authType)). - * Voice model resolution expects bare model IDs (not authType:modelId). - * If the input is not in ACP format, returns it unchanged. - */ export function extractBareModelId(modelId: string): string { const match = modelId.match(/^(.+)\(([^()]+)\)$/); return match ? match[1] : modelId; @@ -36,9 +21,13 @@ export function extractBareModelId(modelId: string): string { * If the input has no colon, returns it unchanged. */ export function decodeVisionModelForPicker(storedValue: string): string { - const colonIdx = storedValue.indexOf(':'); + // CLI stores custom-endpoint vision models as authType:modelId\0baseUrl. + // Strip the \0 suffix before decoding to ACP format. + const nullIdx = storedValue.indexOf('\0'); + const selector = nullIdx >= 0 ? storedValue.slice(0, nullIdx) : storedValue; + const colonIdx = selector.indexOf(':'); if (colonIdx > 0) { - return `${storedValue.slice(colonIdx + 1)}(${storedValue.slice(0, colonIdx)})`; + return `${selector.slice(colonIdx + 1)}(${selector.slice(0, colonIdx)})`; } - return storedValue; + return selector; }