fix(tts): route MiMo Pro as chat#1682
Conversation
📝 WalkthroughWalkthroughThis PR refines chat-audio TTS routing for MiMo models by centralizing model detection logic, expanding prefix support for Xiaomi variants, and implementing defensive audio extraction that safely handles different response content shapes while preserving error behavior for missing audio. ChangesChat Audio TTS Routing Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/archives/chat-audio-tts-routing/plan.md (1)
1-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove SDD docs from
docs/archives/to the required SDD location.For this bug-fix PR, these new SDD artifacts should live under
docs/issues/<goal>/rather thandocs/archives/.... Please relocate this doc set (plan/spec/tasks) to the required folder convention.As per coding guidelines
docs/**/*.md: Create specification-driven development documentation in kebab-case folders:docs/features/<goal>/for new features,docs/issues/<goal>/for bug fixes,docs/architecture/<goal>/for refactors🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/archives/chat-audio-tts-routing/plan.md` around lines 1 - 22, Move the SDD markdown from docs/archives/chat-audio-tts-routing/plan.md into the proper bug-fix docs folder using kebab-case (e.g., docs/issues/chat-audio-tts-routing/plan.md), updating any internal links if present; keep the file name but relocate the folder to docs/issues/<goal>/ per guidelines and ensure the spec references the implementation symbols isChatAudioTtsModel and executeTtsPatternB so readers can find the related code.
🧹 Nitpick comments (3)
test/main/shared/ttsSettings.test.ts (1)
10-13: ⚡ Quick winAdd a direct
isTtsModelIdnegative for non-TTSxiaomi-mimo-vIDs.You already cover chat-audio classification for
xiaomivariants; addingisTtsModelId('xiaomi-mimo-v2.5-pro') === falsewill lock the newly added prefix path end-to-end.Suggested test addition
expect(isChatAudioTtsModel('mimo-v2.5-pro')).toBe(false) expect(isChatAudioTtsModel('xiaomimimo/mimo-v2.5-pro')).toBe(false) + expect(isTtsModelId('xiaomi-mimo-v2.5-pro')).toBe(false) expect(isTtsModelId('mimo-v2.5-pro')).toBe(false)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/main/shared/ttsSettings.test.ts` around lines 10 - 13, Add a negative assertion for the non-TTS `xiaomi-mimo-v` prefix to ensure the `isTtsModelId` path is covered: update the test block to call isTtsModelId('xiaomi-mimo-v2.5-pro') and expect false so both `isChatAudioTtsModel` and `isTtsModelId` reject `xiaomi-mimo` variants (referencing the existing expectations around isChatAudioTtsModel and isTtsModelId in this test).test/main/presenter/llmProviderPresenter/aiSdkRuntime.test.ts (1)
490-564: ⚡ Quick winAdd a regression for multiple audio parts with a later valid payload.
Please add a case where the first
type: 'audio'part has empty/missingaudio.dataand a later one is valid, then assert successful extraction. This locks fallback behavior for mixed content arrays.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/main/presenter/llmProviderPresenter/aiSdkRuntime.test.ts` around lines 490 - 564, Add a new test variant for runAiSdkCoreStream that returns a content array where the first content entry of type 'audio' has empty or missing audio.data and a later content entry of type 'audio' contains a valid base64 payload (e.g., 'ZmFrZS1hdWRpby1wYXJ0'); stub fetch the same way (vi.stubGlobal('fetch', fetchMock)) and call runAiSdkCoreStream with the same context and params, then assert the emitted events include the image_data using the later valid audio payload (mimeType 'audio/wav') and a final stop event, verifying the function falls back to later audio parts when earlier ones lack data.src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts (1)
411-419: ⚡ Quick winSearch for the first valid audio payload, not just the first
audiopart.Current logic stops at the first
type === 'audio'entry even if itsaudio.datais empty. Iterating until the first valid non-empty string avoids false “missing audio” errors on mixed content arrays.Suggested robustness patch
- const audioPart = content.find( - (item) => item && typeof item === 'object' && 'type' in item && item.type === 'audio' - ) - const audioData = - audioPart && typeof audioPart === 'object' && 'audio' in audioPart - ? (audioPart.audio as { data?: unknown } | undefined)?.data - : undefined - - return typeof audioData === 'string' && audioData ? audioData : undefined + for (const item of content) { + if (!item || typeof item !== 'object') continue + if (!('type' in item) || item.type !== 'audio') continue + + const audioData = + 'audio' in item ? (item.audio as { data?: unknown } | undefined)?.data : undefined + if (typeof audioData === 'string' && audioData) { + return audioData + } + } + + return undefined🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts` around lines 411 - 419, The current use of content.find to get audioPart stops at the first item with type==='audio' even if its audio.data is empty; modify the logic that derives audioPart/audioData (the content.find call and the audioData extraction) to search for the first item where item.type === 'audio' and item.audio?.data is a non-empty string (or loop through content until you find such an item), then return that data string or undefined; update the code paths referencing audioPart and audioData to use this validated audio payload check so empty audio entries are skipped.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/archives/chat-audio-tts-routing/plan.md`:
- Around line 1-22: Move the SDD markdown from
docs/archives/chat-audio-tts-routing/plan.md into the proper bug-fix docs folder
using kebab-case (e.g., docs/issues/chat-audio-tts-routing/plan.md), updating
any internal links if present; keep the file name but relocate the folder to
docs/issues/<goal>/ per guidelines and ensure the spec references the
implementation symbols isChatAudioTtsModel and executeTtsPatternB so readers can
find the related code.
---
Nitpick comments:
In `@src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts`:
- Around line 411-419: The current use of content.find to get audioPart stops at
the first item with type==='audio' even if its audio.data is empty; modify the
logic that derives audioPart/audioData (the content.find call and the audioData
extraction) to search for the first item where item.type === 'audio' and
item.audio?.data is a non-empty string (or loop through content until you find
such an item), then return that data string or undefined; update the code paths
referencing audioPart and audioData to use this validated audio payload check so
empty audio entries are skipped.
In `@test/main/presenter/llmProviderPresenter/aiSdkRuntime.test.ts`:
- Around line 490-564: Add a new test variant for runAiSdkCoreStream that
returns a content array where the first content entry of type 'audio' has empty
or missing audio.data and a later content entry of type 'audio' contains a valid
base64 payload (e.g., 'ZmFrZS1hdWRpby1wYXJ0'); stub fetch the same way
(vi.stubGlobal('fetch', fetchMock)) and call runAiSdkCoreStream with the same
context and params, then assert the emitted events include the image_data using
the later valid audio payload (mimeType 'audio/wav') and a final stop event,
verifying the function falls back to later audio parts when earlier ones lack
data.
In `@test/main/shared/ttsSettings.test.ts`:
- Around line 10-13: Add a negative assertion for the non-TTS `xiaomi-mimo-v`
prefix to ensure the `isTtsModelId` path is covered: update the test block to
call isTtsModelId('xiaomi-mimo-v2.5-pro') and expect false so both
`isChatAudioTtsModel` and `isTtsModelId` reject `xiaomi-mimo` variants
(referencing the existing expectations around isChatAudioTtsModel and
isTtsModelId in this test).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dc587677-fef5-49d2-83ee-253aa2bf22c5
📒 Files selected for processing (7)
docs/archives/chat-audio-tts-routing/plan.mddocs/archives/chat-audio-tts-routing/spec.mddocs/archives/chat-audio-tts-routing/tasks.mdsrc/main/presenter/llmProviderPresenter/aiSdk/runtime.tssrc/shared/ttsSettings.tstest/main/presenter/llmProviderPresenter/aiSdkRuntime.test.tstest/main/shared/ttsSettings.test.ts
Summary
Tests
Summary by CodeRabbit
Bug Fixes
Tests