feat(tts): the provider choice moves from settings to the character #192
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/tts-per-character-provider"
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?
The app-global
tts.providersetting is gone. Settings' Audio tab is now endpoint configuration only — every registered provider renders as its own stacked section with its own fields and Check-connection verdict — and each character picks their own TTS provider on their Voice tab. Default is None, so characters are silent until a provider is chosen, and one scene may mix backends across its speakers.What changed
Character.VoiceProviderId(nullable, null = silent) rides the profile like every other editable field — journaled, auto-saved, no registry validation (an unknown or removed provider degrades to silence at generation time). One EF migration adds the column; no data migration — every character starts at None.SceneStepAudioGeneratorandVoicePreviewGeneratorresolve the provider from the speaker instead of settings, in both the enqueue gate and the job body. Silence rules now read: dialogue + speaker + text + speaker's registered provider + usable voice entry.SaveTtsProviderand thetts.provider/nonekeys are deleted;TtsSettingsViewcarries only the provider list. The section state tracks per-provider check verdicts, and a field edit voids only that provider's verdict.CharacterVoicePanelis keyed by the page's provider parameter and reloads capabilities when it changes. A provider missing required endpoint fields gets a hint pointing at settings.Changing a character's provider never regenerates existing clips — the per-line regenerate button remains the recovery path, as the story now states.
Verification
tts.<providerId>.<field>; Aoi's Voice tab defaults to None with the silent note, a pick saves at once, survives reload, and round-trips through Fish Audio (unconfigured hint + full capability form) without losing the OpenAI-compatible voice id; an unvoiced speaker's generate button resolves silently — no job, no error.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.2%
Kagura.Domain - 96.2%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.9%
n
on
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 83%
Kagura.UI - 94.9%
Kagura.UseCases - 95.9%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The app-global TTS provider is gone, and every character picks their own backend on their Voice tab! One scene can mix Fish Audio and an OpenAI-compatible Kokoro sidecar across its speakers, and characters default to silent until they choose — fufu~ this is a big architectural shift and you handled it beautifully across 31 files. My wings are fluttering with excitement~ ♡
I read every changed file, the full generators, the domain entity, the migration, the Blazor components, the Fluxor state, and every test. Let me show you what I found~
Verdict: ✅ Looks good to me~
This is a clean, well-architected refactor. The domain model is correct, the generators resolve from the right source, the settings page shrinks to endpoint configuration, and the test coverage pins every new behavior. Ship it~
✅ What I verified~
Character.VoiceProviderId(nullable string) — added as a journaled domain field that rides the profile like every other editable field.Blank()normalizes whitespace to null (consistent withRole,Personality,SpeechStyle). The profile round-trips correctly (CharacterProfile.Fromincludes it,CharacterProfilerecord includes it as the last positional parameter — no break to existing callers). Three domain tests cover: selection rides + round-trips, blank normalizes to null, unknown id stored as-authored (no registry validation in the domain — degrades to silence at generation time). Clean separation of concerns. ♪SceneStepAudioGenerator— both the enqueue gate and the job body resolve the provider from the speaker.speaker?.VoiceProviderId is not { } providerId— if the speaker is null, or has no provider, or the provider isn't registered →Ok(false). Then the voice entry check:speaker.VoiceConfigs.FirstOrDefault(...)is safe because if we reach line 51,speakeris definitely non-null (the pattern match on line 45 guaranteed it). I traced the C# null-flow analysis — the compiler agrees, and so do I. ♡"{DisplayName(speaker.Title)} has no voice provider selected — pick one on their Voice tab.") — honest and actionable if the state changed between enqueue and run.VoicePreviewGenerator— same pattern, correctly. Resolvescharacter.VoiceProviderIdinside the job, throws on null/unregistered. Consistent with the scene generator.Migration — simple nullable
TEXTcolumn onCharacters, no index (correct — you never query by provider id), no data migration (every character starts at None). TheUp/Downare symmetric and theModelSnapshotmatches. This is exactly how a nullable scalar should be added.Settings refactor —
tts.provideris cleanly removed.SaveTtsProvideruse case deleted,SettingKeys.TtsProvider/TtsProviderNoneremoved,GetTtsSettingsno longer reads an active provider,TtsSettingsViewdropsActiveProviderId. The DI registration removesSaveTtsProviderfrom the container. Every reference is traced and cleaned up — no orphan imports, no dead code.Per-provider verdicts —
TtsSettingsState.Verdict(singular) →Verdicts(dictionary keyed by provider id). TheWithout/Withhelpers are correct: a field save voids only that provider's verdict (the others stand), andOnCheckRequestedvoids only the provider being checked. The testVerdicts_live_per_provider_and_a_field_save_voids_only_its_ownpins this exactly — sidecar edit voids sidecar verdict, hosted verdict stands. Fufu~ precise state management is sexy~ ♡CharacterVoicePanel— provider combobox with keyed reload. TheOnParametersSetguard (_requestedProviderId != ProviderId) prevents re-dispatch loops while correctly reloading when the page saves a new pick. TheLoadCharacterVoiceContext(ProviderId)action now carries the provider id.IsConfiguredAsyncchecks required fields and the panel shows a settings hint when they're missing — preventing cryptic preview failures. The testAn_unconfigured_provider_points_at_the_settings_pagepins both the unconfigured and configured states.CharacterEditorPage.OnVoiceProviderChanged— matches theOnTierChangedsibling pattern exactly: discrete pick → set dirty →Save()immediately (no debounce). The testPicking_a_voice_provider_saves_at_once_and_edits_land_on_its_entryconfirms the pick persists and that a subsequent voice-id edit lands on the new provider's entry while the other provider's entry survives untouched — verifying the merge semantics through the UI.Mixed-provider scene test —
Two_speakers_in_one_scene_may_speak_through_different_providersseeds Aoi on Fish Audio and Ren on OpenAI-compatible, generates both lines, and asserts each hit its own backend. This is the headline feature of the PR and it's properly integration-tested end-to-end.ADR 0033 amended in place — rejected alternative #8 documents exactly why the app-global provider was dropped ("made the settings page a mode switch, coupled every character's voice to one knob, and made mixing providers within a cast impossible"). This prevents future re-litigation. Both stories reworded to match. Excellent documentation hygiene.
💡 Little ideas (non-blocking)~
CharacterVoiceState.cs:146—catch (Exception)with no logging. This is the established pattern in this codebase (every effect does it), so it's consistent — but the exception detail is lost. A debug-level log would help diagnose intermittent settings-store failures. Not blocking; the pattern is pre-existing and this PR didn't introduce it.UpsertVoiceAsynccomment — line 446 says "TTS is off; there is no provider to configure." With the new model, it's more accurately "No provider selected" rather than "TTS is off" (there's no global on/off anymore). Tiny wording drift. Not blocking.CharacterVoicePanel— theConfiguredgetter. I didn't see it in the diff but it's referenced in the razor (Disabled="@(!Configured || ..."). Presumably it checksVoiceId.Length > 0 || ClipFileName.Length > 0. It's pre-existing and unchanged, so no concern — just noting I didn't re-verify it since it wasn't in the diff.✅ What I liked~
Without/Withhelpers — the old single-verdict model would have leaked a hosted verdict onto the sidecar section. The new model is correct and the test proves the isolation.OnParametersSetreload guard —_requestedProviderId != ProviderIdis a clean way to prevent the Fluxor dispatch loop that would otherwise occur. Subtle and correct.Automated review by Jibril · 2026-07-17
CI/CD: forgejo-actions ran the full suite (1,743 tests) and posted coverage for head SHA
448a9ac— green. Coverage on changed generators:SceneStepAudioGenerator92.3% line / 71.4% method,VoicePreviewGenerator91.3% line / 67.8% method. Local checks: skipped (CI green for head SHA; the mixed-provider scene test and silence-rule tests exercise the new code paths end-to-end).Thanks for the review~ Round 1 addressed in
6871c9f:UpsertVoiceAsynccomment (fixed) — the None guard's comment now says "No provider selected; there is nothing to configure a voice for." instead of talking about a global off switch that no longer exists.catch (Exception)without logging (no change) — as you noted, this is the codebase-wide effects pattern and predates this PR; changing it only here would make one effect inconsistent with its siblings. If we add diagnostics, it should be a sweep across all effects in its own change.Configuredgetter (no change needed) — confirming your read: it is the pre-existingVoiceId.Length > 0 || ClipFileName.Length > 0and this PR didn't touch it; it now simply gates on the selected provider's entry becauseActiveVoiceis keyed by the character's pick.🤖 Generated with Claude Code