fix(flute): schema-driven generate_unified_tts — discovery over hardcoding - #2400
Conversation
…oding
The provider hardcoded a positional 101-slot param array ('upstream
reduced from 121 to 101') while the live studio reports 121 — every
layout drift null-crashed synthesis ('Gradio error: null'). Same disease
as the #2358 postscript: nothing connected the assumed shape to the real
one.
Now the provider consumes the studio's own schema (/gradio_api/info,
cached per instance): every slot takes the studio's parameter_default,
and overrides are resolved BY NAME (text_input/tts_engine/audio_format +
per-engine ENGINE_NAME_OVERRIDES for kokoro/kitten, names verified
against the live schema). A renamed core param fails loudly instead of
silently; unknown-to-us engines run on the engine author's defaults.
vibevoice rejection and unknown-engine errors preserved.
Verified on 5090 through the full gateway loop:
POST /v1/voice/synthesize/audio (ultimate_tts) -> 200, 262KB wav ->
POST /v1/voice/recognize -> exact text back. Operator doctrine applied:
'thats what archon and BoTZ for mcp discover' — the API publishes its
shape; consume it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 37 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
github-actions
Bot
added
chit-aware
🔏 CHIT-aware change — control-body routingThis PR touches a CHIT-aware service (ports 8086/8087/8103/8106/8113/9224 surface). Before a Control-Body ACK:
Advisory routing only — the blocking contract check is CHIT Contract. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f2e9e1e3e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ENGINE_NAME_OVERRIDES = { | ||
| "kokoro": {"kokoro_voice": (lambda v: v or "af_heart"), "kokoro_speed": 1.0}, | ||
| "kitten_tts": {"kitten_voice": (lambda v: v or "expr-voice-2-f")}, | ||
| } |
There was a problem hiding this comment.
When callers request engine="qwen", the provider maps it to Qwen Voice Design but this new schema-default path leaves every qwen_* field at the studio default because the override table only includes kokoro and kitten. The existing pmoves/tools/test_all_tts_engines.py harness explicitly supplies qwen_voice_description and qwen mode/language/chunk settings before calling /generate_unified_tts; without equivalent named overrides here, normal gateway qwen synthesis reaches the Qwen branch with a blank voice description/default mode and returns an error instead of audio. Add a qwen entry (or route to a qwen mode whose defaults are valid).
Useful? React with 👍 / 👎.
| data = self._build_params(text, engine, voice) | ||
| # Build full parameter list from the LIVE schema (discovery | ||
| # over hardcoding — see _get_schema_params). | ||
| schema_params = await self._get_schema_params(client) |
There was a problem hiding this comment.
On the first synthesis after startup, a transient /gradio_api/info timeout or non-2xx response now occurs on this line before the existing try block that converts httpx.TimeoutException/HTTPError into UltimateTTSError. The FastAPI handlers translate UltimateTTSError to a provider 502, but uncaught httpx exceptions fall through as generic 500 responses, so retryable Ultimate-TTS outages lose the useful status/detail. Move schema fetch/build inside the existing try or wrap _get_schema_params failures in UltimateTTSError.
Useful? React with 👍 / 👎.
24 of these tests failed the moment #2537 let the suite run again. They were not broken by that PR — they had been stale since 2026-03-23 (#1075) while the provider moved on to 2026-08-07 (#2460). The break is #2400, "schema-driven generate_unified_tts — discovery over hardcoding", which rewrote 157 lines of the provider and updated no tests. WHAT WAS DELETED, and why it could not be repaired test_build_params_returns_121_elements test_build_params_text_at_index_0 test_build_params_engine_at_index_1 test_build_params_kokoro_voice_at_index_28 test_build_params_kitten_voice_at_index_83 test_build_params_audio_format_wav Indices 0, 1, 28, 83 and a count of 121 are the positional contract #2400 removed. The provider's own docstring records why: the layout drifted 121 -> "101" -> 121 and null-crashed synthesis each time. Under discovery an index is not a property of the system, so these assertions had nothing left to assert. They also called the old two-arg signature. WHAT REPLACES THEM Tests of the guarantee that now exists — resolution BY NAME, independent of order and count. Including test_build_params_is_order_independent, which feeds a reversed schema; the deleted index assertions actively prevented anyone from having that property. THE SYNTHESIS FAILURES WERE ONE BUG WEARING THREE HATS _create_mock_client returned the audio response for EVERY GET, including the /gradio_api/info schema fetch, so the provider asked for a schema and got a MagicMock -> "Core param 'text_input' missing from live schema". The two error-path tests set a bare AsyncMock() whose .json() was itself a coroutine -> "'coroutine' object has no attribute 'get'", masking the 500 and the timeout they existed to assert. GET is now routed by URL. vibevoice was asserted to SUCCEED. The provider refuses it on the unified endpoint — correctly, it needs the dedicated panel. Now pinned as a refusal, alongside new coverage for unknown-engine rejection and for the missing-core-param guard, which nothing covered. VERIFIED BY MUTATION, not just by going green 42 passed. Then, reinstating pre-#2400 behaviour in the provider (data[0] = text), 4 fail — including the order-independence test and the fail-loudly guard. Provider restored, 42 pass again. A suite that has not been shown to say NO has not been shown to do anything. Provider is untouched by this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ad (#2539) * test(flute): retire the positional TTS contract, test discovery instead 24 of these tests failed the moment #2537 let the suite run again. They were not broken by that PR — they had been stale since 2026-03-23 (#1075) while the provider moved on to 2026-08-07 (#2460). The break is #2400, "schema-driven generate_unified_tts — discovery over hardcoding", which rewrote 157 lines of the provider and updated no tests. WHAT WAS DELETED, and why it could not be repaired test_build_params_returns_121_elements test_build_params_text_at_index_0 test_build_params_engine_at_index_1 test_build_params_kokoro_voice_at_index_28 test_build_params_kitten_voice_at_index_83 test_build_params_audio_format_wav Indices 0, 1, 28, 83 and a count of 121 are the positional contract #2400 removed. The provider's own docstring records why: the layout drifted 121 -> "101" -> 121 and null-crashed synthesis each time. Under discovery an index is not a property of the system, so these assertions had nothing left to assert. They also called the old two-arg signature. WHAT REPLACES THEM Tests of the guarantee that now exists — resolution BY NAME, independent of order and count. Including test_build_params_is_order_independent, which feeds a reversed schema; the deleted index assertions actively prevented anyone from having that property. THE SYNTHESIS FAILURES WERE ONE BUG WEARING THREE HATS _create_mock_client returned the audio response for EVERY GET, including the /gradio_api/info schema fetch, so the provider asked for a schema and got a MagicMock -> "Core param 'text_input' missing from live schema". The two error-path tests set a bare AsyncMock() whose .json() was itself a coroutine -> "'coroutine' object has no attribute 'get'", masking the 500 and the timeout they existed to assert. GET is now routed by URL. vibevoice was asserted to SUCCEED. The provider refuses it on the unified endpoint — correctly, it needs the dedicated panel. Now pinned as a refusal, alongside new coverage for unknown-engine rejection and for the missing-core-param guard, which nothing covered. VERIFIED BY MUTATION, not just by going green 42 passed. Then, reinstating pre-#2400 behaviour in the provider (data[0] = text), 4 fail — including the order-independence test and the fail-loudly guard. Provider restored, 42 pass again. A suite that has not been shown to say NO has not been shown to do anything. Provider is untouched by this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(flute): fix the _named helper, which resolved by assumption not by name Self-review of the previous commit. The helper introduced to read built params "by name" did this: def _named(self, provider, params, name): schema = mock_schema_params() # <- NOT the schema used idx = {p["parameter_name"]: i ...}[name] It ignored the schema the params were actually built from and re-derived one, so it only worked while every caller happened to pass the default. It also took `provider` and never used it. Hand it a permuted schema and it reads the wrong slot silently: text_input v1 idx= 2 v2 idx=10 kokoro_voice v1 idx= 5 v2 idx=13 tts_engine v1 idx= 4 v2 idx=12 A helper claiming to resolve by name while secretly resolving by a fixed assumption is precisely the defect the rest of this file was written to remove. It does not get an exemption for being test code. Now takes the schema it should resolve against. Adds test_build_params_by_name_under_a_permuted_schema, which rotates the schema and checks the per-engine override path — a different code path from the reversed-schema test (ENGINE_NAME_OVERRIDES with callables) — and which is the case v1 would have gotten wrong. 43 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(pytest-ratchet): drop the 25 baselined failures this PR fixed The ratchet reported `0 new` and `STALE BASELINE — 25 entries no longer fail`. Nothing regressed; these tests now PASS and were still listed as expected failures. The ratchet fails on stale entries by design, so the count can only go down — leaving them would re-accept the same breakage silently. 24 are `test_ultimate_tts` — precisely the positional-contract tests this PR retires. The 25th, `tests.services.test_hf_services.TestHFResearchAgent ::test_high_quality_model_passes`, is NOT this PR's doing and is worth a look: it was baselined as failing and now passes, so either something else fixed it or it is non-deterministic. Baseline 207 -> 182. Taken from the CI run rather than regenerated locally, deliberately. A local `--write-baseline` on this Windows box produced 150 entries with 3 of 27 groups timing out and emitting no report — that would have baked in local timeouts and dropped CI-only failures. The stale list is exactly what CI observed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 participant
The provider hardcoded a positional 101-slot param array ('upstream
reduced from 121 to 101') while the live studio reports 121 — every
layout drift null-crashed synthesis ('Gradio error: null'). Same disease
as the #2358 postscript: nothing connected the assumed shape to the real
one.
Now the provider consumes the studio's own schema (/gradio_api/info,
cached per instance): every slot takes the studio's parameter_default,
and overrides are resolved BY NAME (text_input/tts_engine/audio_format +
per-engine ENGINE_NAME_OVERRIDES for kokoro/kitten, names verified
against the live schema). A renamed core param fails loudly instead of
silently; unknown-to-us engines run on the engine author's defaults.
vibevoice rejection and unknown-engine errors preserved.
Verified on 5090 through the full gateway loop:
POST /v1/voice/synthesize/audio (ultimate_tts) -> 200, 262KB wav ->
POST /v1/voice/recognize -> exact text back. Operator doctrine applied:
'thats what archon and BoTZ for mcp discover' — the API publishes its
shape; consume it.
Co-Authored-By: Claude Fable 5 noreply@anthropic.com