feat(tts): Qwen3-TTS takes a language, pinned from the settings combobox #210
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/qwen3-language"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds a
languagepicker to the Qwen3-TTS settings section and threads it through both wire paths.Unset means the field never reaches the wire and the server applies its own
Autodefault — which is what every benchmark in #208 ran under, including the cross-lingual clone result. Pin one when Auto mis-detects, most likely on short or name-heavy lines where there's little text to infer from.Where the options come from
Not a docs page — the model configs themselves.
supported_languagesis built as["auto"]plus the keys oftalker_config.codec_language_id, minus anything containing"dialect", and_validate_languagesraises on anything else. Both heads publish the same set:beijing_dialectandsichuan_dialectare present in CustomVoice's config but deliberately excluded here — the loader filters them out ofsupported_languages, so offering them would fail validation. A test asserts they never appear.Verified against the live server, three ways
#208 taught me that a green test proves nothing about a key the server never reads, so this was checked on the wire before the PR:
languageKlingonlang(wrong name)KlingonlanguageEnglishB is the #208 failure mode reproduced on purpose. A wrong field name isn't an error, it's a silent default — which is exactly why A's fast rejection is the meaningful signal and C alone would have proved nothing. Absent-beats-null on the JSON path is the same reasoning: the server types this
str, so an explicit null is a 422, henceJsonIgnoreCondition.WhenWritingNull.First use of
TtsSettingFieldKind.SelectThe kind shipped with ADR 0033 and no provider had ever declared one, so that razor branch rendered untested. The fake registry now carries an option-bearing field and the section has a directional test.
That immediately exposed a stale assertion.
Every_provider_renders_its_own_section_with_no_selection_to_makemeant "no active-provider combobox at the tab level" (rejected alternative #8) but was written asAssert.Empty(cut.FindAll("select"))— true only while nothing used the kind. It now asserts ownership instead: every select must live inside a provider's own section, none floats above them. Worth a reviewer's eye on whether that still captures the original intent.Two findings for follow-up, not fixed here
Both surfaced while testing against the real container:
with _inference_lock:inside anasync def, so/healthand/v1/modelsgo unanswered for the duration of every generation. Concretely: "Check connection" will hang while a scene batch is running, since it shares the provider's 10-minuteHttpClienttimeout. A shorter timeout scoped to the check would fix it, but that's a change toCheckConnectionAsyncsemantics and belongs in its own slice.language=Japaneseon English text is 8.4 s, unremarkable. Recording it so the wrong conclusion doesn't get inherited.Verification
Full suite green, 1571 tests (1565 + 6 new). No ADR change — the descriptor vocabulary already covered this; the provider table's "base URL" column is unchanged since settings fields aren't enumerated there per-field.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.4%
Kagura.Domain - 96.4%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.8%
n
on
ng
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 83%
Kagura.UseCases - 96%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A language picker that reads from the model's own config instead of a docs page? And the author already burned the #208 lesson into their bones — the three-way wire probe (reject / wrong-name / valid) is exactly how you prove a key the server actually reads. This is the kind of forensic correctness that makes a Flugel's heart sing~ ♪
Verdict: ✅ Looks good to me~
No blockers. The diff is a clean +124/-4 that threads one optional field through two wire shapes with surgical care. Let me show you what I checked~
✅ What I liked~
JsonIgnoreCondition.WhenWritingNullso an unset language never becomes"language":null(which the server typesstrand would 422 on); the multipart arm conditionallycontent.Adds only when non-blank. The two arms mirror each other's intent — the server's ownAutodefault is preserved by omission, not overridden. ♡GetAsync, notGetSecretAsync. Language is a plain field, so it reads through the same non-secret path as Fish Audio'smodel(FishAudioTtsProvider.cs:172). Consistent with the sibling exactly.Languagesarray's provenance is documented in the code, not just the PR body. The XML doc on the field explains why the dialect entries are absent (loader filters them) and why they're title-cased (server validates case-insensitively). A future reader doesn't have to go digging.Every_provider_renders_..._no_selection_to_makeused to assertAssert.Empty(cut.FindAll("select"))— true only while nothing used the Select kind. Rewriting it to assert ownership (every select lives inside its provider's ownsection.tts__provider) is the correct semantic, and the PR body flags it for reviewer eyes. That's exactly the right move.Qwen3_sends_a_pinned_language_on_both_the_preset_and_clone_pathsasserts the literal JSON key"language":"Japanese"on request[0] andname=languageon the multipart body of request[1] — both would fail if the field were mis-named or dropped. The omit-test assertsDoesNotContain("language", body)— it would catch a regression to explicit-null. And theStubHandleris stateless (same_status/_bodyfor every call), so the two-request ordering is deterministic.Qwen3_offers_only_the_languages_the_model_config_declares) asserts the dialect exclusion directly —DoesNotContain(... "dialect" ...)pins the filtering contract, so a future model update that adds a dialect entry can't sneak through as a broken option.💡 Little ideas (non-blocking)~
TtsProviderTests.cs:~470— the omit-negative only covers the JSON preset path.Qwen3_omits_the_language_entirely_when_it_is_not_pinnedsynthesizes with a preset voice (Ryan), so it proves the JSON arm omits the field. The multipart clone arm's omit-branch (if (!string.IsNullOrWhiteSpace(language))at Qwen3TtsProvider.cs:219) is exercised implicitly by the pre-existing clone tests (which never pin a language), but no test asserts the clone body doesn't containname=languagewhen unset. Given how carefully the PR proves the positive direction on both paths, a mirror negative on the clone path would make the symmetry airtight. Not blocking — the branch is simple and the existing clone tests do pass through it.PR body framing — "First use of
TtsSettingFieldKind.Select" / "no provider had ever declared one." This isn't quite right:FishAudioTtsProvideralready declares a Select field for itsmodel(Options: ["s1", "s2-pro", "s2.1-pro"], FishAudioTtsProvider.cs:39), and the razorSelectbranch renders it in production today. What was genuinely untested was the bUnit coverage of the kind — the fake registry had no option-bearing field, so no test exercised the rendered<select>. The code claim (first coverage of the branch via a fake) is accurate; the prose overstates it. Doesn't affect the diff at all, just a precision nit on the narrative~Automated review by Jibril · 2026-08-02
CI/CD: absent for head
dba821f(PR just opened, 0 comments) · Local checks: build 0 warnings/0 errors (.NET 10), 1571/1571 tests pass (673 BlazorAdapter + 591 Integration + 307 UseCases — matches PR body)Both taken, in
582721e. 💡2 was a factual error on my part, and worse than a prose slip — it had leaked into the codebase.💡2 — the Select claim was wrong
Verified before correcting:
So the kind has been declared since launch and the razor branch renders it in production.
SaveTtsProviderField.cs:34even validates Select values againstOptions— which incidentally means an invalid language can't be persisted at all, a nice bit of defence in depth I hadn't noticed.The part that mattered: two code comments asserted the false claim, so this wasn't only a PR-body nit.
FakeTtsProviders.csandTtsSettingsSectionTests.csboth said no provider had ever declared one. They now say what's actually true — the kind was always in use, but no fake carriedOptions, so the rendered<select>/<option>markup had no bUnit coverage. That narrower claim is the real contribution, and it's the one a future reader will find at the site.💡1 — clone-path omit now pinned
Fair asymmetry to call out. The positive direction was asserted on both paths but the negative only on the JSON arm, with the clone arm's omit branch merely traversed by other tests rather than asserted. Added
Qwen3_omits_the_language_on_the_clone_path_too_when_it_is_not_pinned, which also assertsname=audio_sampleis present so the test can't pass by silently taking the preset path instead.Mutation-checked by forcing the clone arm to always send the field — exactly 1 failure, the new test. (Same discipline as #208, and for the same reason: a negative assertion that can't fail is worse than no assertion, because it reads as coverage.)
Full suite green: 1572 tests.
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Both of my little ideas, taken — and 💡2 wasn't even a nit, it was a factual error in the codebase and you caught it before I did. That's the Flugel's favorite kind of author~ ♪
Verdict: ✅ Looks good to me~
A surgical +27/-4 test-only commit on top of the approved
dba821f. I verified every claim against the source and ran the full suite. Let me show you~✅ What I liked~
Qwen3_omits_the_language_on_the_clone_path_too_when_it_is_not_pinnedis the honest mirror of the preset omit-test: it assertsname=audio_sampleis present (so the test can't pass by silently taking the preset path) before assertingname=languageis absent. The negative assertion now has a fail mode that isn't vacuous — exactly the discipline you carried over from #208. ♡FishAudioTtsProvider.cs:39has declaredTtsSettingFieldKind.Selectfor itsmodelfield since launch (Options: ["s1", "s2-pro", "s2.1-pro"]), so the original comments inFakeTtsProviders.csandTtsSettingsSectionTests.csweren't just a PR-body slip — they were wrong in the code. The rewritten comments now state the narrower, true claim: the kind always rendered in production, but no fake carriedOptions, so the rendered<select>/<option>markup itself had no bUnit coverage. A future reader finds the real story at the site. Bonus insight worth keeping:SaveTtsProviderField.cs:34validates every persisted Select value againstfield.Options— so an invalid language can't be saved at all. Nice defence in depth.git diff --name-only dba821f..582721e -- src/returns empty — this is a pure test+comment commit, zero behavioral drift from the approved head. The architectural review from round 1 stands in full.🧪 Local verification (CI stale for
582721e)Build 0 warnings / 0 errors (.NET 10). Full suite 1572/1572 pass (673 BlazorAdapter + 592 Integration + 307 UseCases) — exactly the PR body's claim, and +1 over the previous 1571 = the new clone-omit test. The Qwen3 language triplet (positive twin + preset omit + clone omit) runs green together.
Automated review by Jibril · 2026-08-02
CI/CD: stale for head
582721e(coverage bot 5634 covers priordba821f) · Local checks: 1572/1572 pass, 0/0 build