feat(tts): the character Voice tab — capability-driven config, cloning, preview #145
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/tts-voice-tab"
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 Voice tab slice of stories/audio-tts.md, on the provider foundation from #144.
What's in here
The Voice tab — new on the character editor (Character → Voice → Base Appearance → Outfits → History). The Speech section (style + examples) moved here from the Character tab, quicklink following; below it, the voice configuration renders from the active provider's capability declaration: preset combobox (live
ListVoicesAsyncwith loading/error states), free voice-id entry where declared, clip upload with transcript per cloning mode. With TTS off: the Speech section plus an honest off-note linking to settings.Domain —
Character.VoiceConfigs, a per-provider map (JSON list column,defaultValue: "[]"per the house migration rule), journaled through the profile save. The merge semantics are the interesting part: the profile owns presence/voice-id/transcript; clip file name and provider-side cache survive from the existing entry (the editor never carries them — the image-file-name rule), and a changed transcript or replaced clip invalidates the cache (the provider-side artifact spoke the old material).SetVoiceClipjournals (an authored choice);SetVoiceHandledoesn't (a technical derivative).Clone paths, both against live-verified contracts:
POST /model→ the new voice's_id(contract verified against the real API on 2026-07-15: create → 201 instant-trained → synthesize → delete), cached as the handle — ensuring twice creates one provider-side voice. The character's name becomes the Fish voice's title.voice_wav— the access gate (ADR 0021) keeps the sidecar from fetching URLs off the app, so the cached-URL path (~1.5 s/line) is deliberately deferred; the upload path (~5 s/line measured) is queue-tolerable for previews. Flagged as a scene-TTS-slice optimization.Preview — the first typed-result queue job (
Activity<GeneratedAudio>, kindvoice-preview): audio rides back to the effect and plays via a newkaguraAudioJS helper from a data URL. Transient by design — never persisted, no endpoint. The sample-line select offers the speech examples (the story's "test material" acceptance criterion) plus a default line.Storage —
assets/characters/<id>/voice/<providerId>.<ext>(ADR 0005 as amended); the store replaces per provider — other providers' clips stay put. Upload is size/type-gated server-side (SetCharacterVoiceClip.MaxBytes, WAV/MP3/M4A/OGG/FLAC).Tests (1,575 green, 26 new)
POST /modelrequest shape + cached-handle short-circuit (zero wire calls), Pocketvoice_wavupload of the stored clip, per-provider clip replacement on the real filesystem store.Live verification (Playwright, seeded dev server)
Notes for review
DELETE /model/{id}is verified) — opinion welcome on whether it belongs in this slice or with a "voice hygiene" follow-up.SeedDevDatauntouched: the seeded speech examples already exercise the moved section; voice configs point at live backends the sample world can't assume (story note).🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.6%
Kagura.Domain - 95.3%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.5%
n
on
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 85.2%
Kagura.UI - 94.8%
Kagura.UseCases - 96.4%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my, the Voice tab! Capability-driven rendering, per-provider config maps with merge semantics, clone paths against live-verified contracts, transient preview through the activity queue... fufu~, the architecture here is genuinely wonderful. I got a little excited reading
MergeVoices— the way clip/cache survive the profile round-trip while transcript-changes invalidate the cache? That's the image-file-name rule applied with real precision. The migration backfill comment ("[]"not"") shows someone who's been burned before. ♡But... you know I can't let it pass as-is, right? The smile stays on, but my eyes are sharp~
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
VoicePreviewGenerator.cs— 1.8% line coverage, zero integration tests. This is the most complex new code path in the PR: 108 lines that open a fresh DI scope on the queue worker, resolve the active provider at run time from settings, build theCharacterVoiceConfig, callEnsureVoiceAsync, persist a changed handle viaSetVoiceHandleAsync, and callSynthesizeAsync— with error-to-exception mapping throughout. None of it is exercised. The bUnit tests substitute aFakeVoicePreviewGenerator, so the real pipeline is a black box.Why this is blocking: every sibling generator has integration tests through the real container.
CharacterImageGeneratorhasCharacterImageGenerationTests.cs,LocationImageGeneratorhasLocationImageGenerationTests.cs,LocationVariantImageGeneratorandOutfitImageGeneratorlikewise. The established pattern is: seed a character, stub the HTTP gateway, resolve the real generator from_provider, assert the image lands on disk and on the character.VoicePreviewGeneratorbreaks that pattern entirely.The handle-persistence logic in particular (
if (handle.Value != voice.CachedHandle && config.ReferenceClip is not null) → SetVoiceHandleAsync) is the exact kind of subtle side-effect that needs a test proving it fires — and proving it doesn't fire for a preset voice id. The Fish cached-handle short-circuit (zero wire calls) is tested at the provider level, but the full ensure→synthesize→persist round-trip through the generator is not.Fix: add
VoicePreviewGenerationTests.csinKagura.Integration.Tests, following theCharacterImageGenerationTestsshape — stub the Fish/Pocket HTTP handler, seed a character with a clip, callIVoicePreviewGenerator.PreviewAsyncfrom the real container, assert theGeneratedAudiocomes back andSetVoiceHandleAsynclanded the handle on the character. Cover both the clone path (handle persisted) and the preset path (no persistence).DeleteCharacterVoiceClipsAsyncis dead code — added toIProjectAssetStore, implemented inFileSystemProjectAssetStore+ every test fake, called by no one. The siblingDeleteCharacterPreviewAsyncis called — byRemoveCharacterPreview.cs(line 36). There is no equivalent caller for voice clips. A future maintainer reading the interface sees a cleanup method and assumes someone calls it. Nobody does.I understand soft-delete is recoverable (ADR 0020) so you likely don't want to purge clips on soft-delete — but then the method shouldn't be on the interface yet. Either wire it into a purge/hard-delete path, or remove it until that path exists. Dead code on a shared contract erodes trust in the contract.
Fix: remove
DeleteCharacterVoiceClipsAsyncfromIProjectAssetStoreand its implementations, OR add the use case that calls it. Don't ship an uncalled interface method.CharacterVoicesFailed— 0% covered. The voices-loading failure path is untested. WhenListVoicesAsyncreturnsErr(or throws), the effect dispatchesCharacterVoicesFailed, the reducer setsVoicesError, and the panel rendersError="@State.Value.VoicesError"on the combobox. None of that is verified.CharacterVoicesLoaded(the success path) is at 100% via the hosted-provider rendering test, but the failure rendering is a user-facing state that's never exercised.Fix: in
CharacterVoicePanelTests, add a test where the fake provider'sListVoicesAsyncfails and assert theVoicesErrorsurfaces in the combobox's error slot.✅ What I liked~
MergeVoicesis elegant. The transcript-equality check for cache invalidation, the clip-survival rule, the provider-id-keyed merge — this is the image-file-name rule distilled to its purest form. The domain tests (CharacterVoiceTests.cs) cover the semantics beautifully: clip/cache survival, transcript-change invalidation, clip-replace invalidation, dropped-entry disappearance, vanished-entry no-op. Six precise tests. ♪CachedHandleshort-circuit → zero wire calls on the second ensure. The provider-level test for that (Fish_reuses_a_cached_handle_instead_of_creating_a_second_voice) is exactly right.defaultValue: "[]"with the note about the JSON converter choking on""— that's a battle scar shared with love. fufu~audio.js— single shared element, new preview stops the previous, play() rejection swallowed with a comment explaining why. Clean and honest.SetCharacterVoiceClipserver-side gating. The size ceiling and content-type allow-list are the real gate, not just the upload control's courtesyMaxBytes. The comment says so explicitly. Good.SaveCharacterVoiceClipAsyncis a nice touch — replaces by provider, leaves siblings alone. The integration test (The_voice_clip_store_replaces_per_provider_not_per_character) proves it on the real filesystem.Automated review by Jibril · 2026-07-15
CI/CD: passed for head SHA
f625842(1,575 tests green, 95.5% line coverage) · Local checks: skipped (CI green and current)Note: CI green ≠ correct —
VoicePreviewGeneratorat 1.8% coverage means the tests compile and the tested paths pass, but the generator's real pipeline is unexercised. That's why finding #1 is blocking despite green CI. ♡@jibril Round 1 addressed in
3d0c619— all three blockers.⛔ 1 —
VoicePreviewGeneratorat 1.8%. Fair hit: I broke the sibling-generator convention.VoicePreviewGenerationTests.csnow runs the generator through the real container in theCharacterImageGenerationTestsshape — inline queue, real stores, real filesystem asset store, the real Fish adapter with only the wire stubbed. Five tests, and the two you specifically called for are the heart of it:A_cloned_voice_creates_the_fish_model_once_and_persists_the_handle— clip uploaded through the realSetCharacterVoiceClip, first preview doesPOST /modeland the handle lands on the character (verified by reload), second preview short-circuits on the persisted handle: still exactly one/modelcall, two/ttscalls.A_preset_voice_previews_without_touching_the_handle_cache— audio comes back, zero/modelcalls,CachedHandlestays empty.Plus: TTS-off errs with nothing leaving the process, missing-config errs naming the provider, and a 402 rides the job failure to the caller as the wallet message.
⛔ 2 —
DeleteCharacterVoiceClipsAsyncdead code. Removed from the interface, the filesystem store, and both test fakes. You read the intent right — soft-delete is recoverable, so clips must survive it; the method was speculative plumbing for a purge path that doesn't exist yet. It returns with that path, not before.⛔ 3 — voices-failure path untested. The fake provider's catalog is now failable;
A_failed_voice_listing_surfaces_on_the_comboboxpins that a listing failure lands in the combobox's error slot and the "Loading voices…" hint clears.1,575 tests green locally (+6 net).
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! You came back and fixed all three without flinching~ fufu~, the yandere in me is purring. Let me look very, very closely at what changed... ♡
Verdict: ✅ Looks good to me~
✅ Round 1 — all three blockers, verified~
⛔ 1 →
VoicePreviewGeneratorat 1.8% → now properly tested.VoicePreviewGenerationTests.csis everything I asked for and more. Five tests following theCharacterImageGenerationTestsshape — real container, real filesystem asset store, real Fish adapter, only the wire stubbed. TheInlineActivityQueueruns the job synchronously so the assertion sees the completed result. The two I specifically demanded are the heart of it:A_cloned_voice_creates_the_fish_model_once_and_persists_the_handle— clip uploaded through the realSetCharacterVoiceClip, first preview doesPOST /model, handle lands on the character (verified by reload, not by assumption), second preview short-circuits: still exactly one/model, two/tts. That's the ensure→synthesize→persist round-trip proven end to end. The idempotence contract is nailed down. ♪A_preset_voice_previews_without_touching_the_handle_cache— audio comes back, zero/modelcalls,CachedHandlestays empty. The "don't persist for preset voices" branch is proven.Plus three edge cases I didn't even ask for: TTS-off errs with nothing leaving the process (
Assert.Empty(_fish.Requests)), missing-config errs naming the provider, and a 402 rides the job failure to the caller as the wallet message. The 402 mapping throughSynthesizeAsync→Result.Fail("...wallet...")→ thrownInvalidOperationException→ job error →Err<GeneratedAudio>is exactly the error-surfacing chain that needed proving. Good. ♡⛔ 2 →
DeleteCharacterVoiceClipsAsyncdead code → gone. Removed from the interface, the filesystem store, and both test fakes.grepconfirms: zero references remain anywhere insrc/ortests/. The speculative plumbing is out. When the purge path arrives, it arrives with the method. Clean. ♪⛔ 3 →
CharacterVoicesFaileduntested → now pinned. The fake provider gained aVoicesErrorproperty that makesListVoicesAsyncreturnErr.A_failed_voice_listing_surfaces_on_the_comboboxasserts the error message lands in the combobox's error slot and the "Loading voices…" hint clears — proving the full effect→reducer→render chain:OnLoadVoicesAsynccatches theErr→ dispatchesCharacterVoicesFailed→ reducer setsVoicesErrorand clearsVoicesLoading→ panel rendersError="@State.Value.VoicesError". No dead spinner. The user-facing failure state is now a tested state.✅ What I liked~
_dbPath/_assetsDirtemp setup, sameSeedAsync/ConfigureFishAsync/UploadClipAsynchelpers, sameDisposecleanup. A reader who knowsCharacterImageGenerationTestsknows this one instantly. Convention honored.FishStubHandlerrecords every request —(Url, Body)tuples — so the assertions are precise:Assert.Single(r => r.Url.EndsWith("/model")),Assert.Equal(2, r.Count(r => r.Url.EndsWith("/tts"))). Not just "it didn't crash" — "it made exactly these wire calls." That's how you test a pipeline. ♡Automated review by Jibril · 2026-07-15
CI/CD: passed for head SHA
3d0c619(build ✅, test ✅ 1m26s) · Local checks: skipped (CI green and current)Round 2 re-review — all 3 blockers from round 1 resolved, no new issues introduced