Add mistv3 model support to Rime TTS services#4223
Conversation
mistv3 shares most params with mistv2 but does not support reduceLatency or phonemizeBetweenBrackets. Adds an explicit elif branch for mistv3 in _build_ws_params (RimeTTSService) and guards reduceLatency/phonemizeBetweenBrackets behind a mistv2 check in RimeHttpTTSService.
cdd98de to
248f5c8
Compare
|
Please add a changelog entry. See CONTRIBUTING.md for details: |
Codecov Report❌ Patch coverage is
... and 41 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
@markbackman changelog entry added! |
|
Is this ready for review? |
|
Based on the code, the PR description is out of date. Is that right? |
|
@markbackman apologies for the discrepancy/changes there -- we ended up landing more features for Mist v3 than expected yesterday. With the latest commit and the update to the description, this is now 100% ready for review and the description is accurate to the code. |
markbackman
left a comment
There was a problem hiding this comment.
Nice addition — just two items to address.
| if self._settings.pauseBetweenBrackets is not None: | ||
| params["pauseBetweenBrackets"] = json.dumps(self._settings.pauseBetweenBrackets) | ||
| if self._settings.phonemizeBetweenBrackets is not None: | ||
| params["phonemizeBetweenBrackets"] = json.dumps(self._settings.phonemizeBetweenBrackets) |
There was a problem hiding this comment.
This line is 101 characters — will fail the ruff 100-char limit.
Can you run uv run scripts/fix-ruff.sh or install the pre-commit hook (uv run pre-commit install)?
| params["temperature"] = self._settings.temperature | ||
| if self._settings.top_p is not None: | ||
| params["top_p"] = self._settings.top_p | ||
| elif self._settings.model == "mistv3": |
There was a problem hiding this comment.
Suggestion: consider restructuring to avoid duplicating pauseBetweenBrackets and phonemizeBetweenBrackets handling (same logic exists in the else branch). Something like:
if self._settings.model == "arcana":
# arcana-only params
...
else:
# shared mist/mistv2/mistv3 params
if self._settings.pauseBetweenBrackets is not None:
params["pauseBetweenBrackets"] = json.dumps(self._settings.pauseBetweenBrackets)
if self._settings.phonemizeBetweenBrackets is not None:
params["phonemizeBetweenBrackets"] = json.dumps(
self._settings.phonemizeBetweenBrackets
)
if self._settings.model == "mistv3":
if self._settings.inlineSpeedAlpha is not None:
params["inlineSpeedAlpha"] = self._settings.inlineSpeedAlpha
else: # mist/mistv2
if self._settings.reduceLatency is not None:
params["reduceLatency"] = self._settings.reduceLatency
...This way the shared params are written once, reducing the chance of them drifting apart.
JiwaniZakir
left a comment
There was a problem hiding this comment.
The docstring update on line ~77 correctly notes that speedAlpha now supports mist, mistv2, mistv3, but the new elif self._settings.model == "mistv3": block in _build_ws_params() omits speedAlpha entirely—only pauseBetweenBrackets, phonemizeBetweenBrackets, and inlineSpeedAlpha are added. If speedAlpha is genuinely supported by mistv3, it needs to be wired up in that block; if not, the docstring update is incorrect and should remain mistv2 only. Conversely, inlineSpeedAlpha is included in the mistv3 block but its docstring doesn't mention mistv3 support, so the documentation is inconsistent in both directions. It's also worth double-checking whether reduceLatency, noTextNormalization, and saveOovs truly remain mistv2-only or if mistv3 picks up any of those as well, since leaving them in the else branch means they'd silently be sent for mistv2 but not mistv3.
Adds
mistv3as a supported model inRimeTTSServiceandRimeHttpTTSService.mistv3supportsspeedAlpha,inlineSpeedAlpha,pauseBetweenBrackets, andphonemizeBetweenBrackets.reduceLatency,noTextNormalization, andsaveOovsare mistv2-only and are excluded.