feat(voice): wire voiceStyle through _synthesize to the TTS providers - #863
feat(voice): wire voiceStyle through _synthesize to the TTS providers#863drewdrewthis wants to merge 6 commits into
Conversation
`setOneShotOverride({ voiceStyle })` installed a style, the simulator logged
"no TTS provider currently honours it", and then called `_synthesize(text,
voice)` — the style was dropped on the floor (#533). Wire it end to end:
- `TtsSynthesisOptions { voiceStyle? }` — per-call synthesis options that are
NOT part of the voice identity. `TTSCallable` widens to a third optional
argument, so existing two-argument providers stay assignable, and
`synthesize()` forwards it to the provider.
- The LRU cache key becomes `(sha256(text), voice, voiceStyle)`. Without the
style in the key, an angry turn and a neutral turn with the same text+voice
share one entry and whichever ran first silently serves its bytes to the
other. Effects still stay OUT of the key.
- `UserSimulatorVoiceConfig.voiceStyle` and `TtsConfig.voiceStyle` give a
simulator-level and a per-run default; `effectiveVoiceStyle()` resolves them
with the same precedence shape as `effectiveVoice`/`effectiveAudioEffects` —
one-shot override, then simulator config, then per-run config. A null
override still means "none installed", not "force unstyled".
- Delete `warnVoiceStyleOnce`: the warning it emitted is now false. The
"provider ignores the style" case moves to the TTS router in the next commit.
TS leads Python here — `python/scenario/voice/tts.py` still has no style
channel, so this is explicitly NOT parity.
The router forwards `options` only when one is actually set, keeping the
unstyled path's historical two-argument call shape for providers (and spies)
that observe arity.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MWeYLrzADoJe5EJS87Lg4M
… instructions)
The style now reaches `_synthesize`, but no backend did anything with it.
Give each provider a real style channel and make the gap visible where one
does not exist:
- `TtsProvider.supportsVoiceStyle` — providers opt in. The registry keeps the
flag alongside the callable, and `synthesizeRaw` warns ONCE per provider
(naming the provider and the style) before stripping the style for one that
has not opted in. Silence would be worse than the old wrong warning: a
scripted `user("…", { voiceStyle: "angry" })` would return cheerful audio
with nothing saying why.
- ElevenLabs: prepend an inline `[angry]` marker to the text. EL's
`voiceSettings.style` is a NUMERIC 0-1 exaggeration knob, not a named style,
so `"angry"` has nowhere to go there; `eleven_v3` — already the pinned model
for exactly this reason — DOES honour inline paralinguistic markers, so that
is the mechanism that makes a named style audible. Idempotent, so a caller
who hand-wrote `[angry] …` is not double-prepended.
- OpenAI: `gpt-4o-mini-tts` takes an `instructions` parameter for delivery
control. The key is omitted entirely when no style is set, so the unstyled
request stays byte-for-byte the one this callable always sent. This is the
one documented exception to the file's "swap the whole callable rather than
parameterise it" rule — voiceStyle is per-utterance, not per-backend.
`__resetVoiceStyleWarnings()` is an @internal test seam, kept separate from
`clearTtsCache()` because a suite that clears the cache between cases must not
silently re-arm the warning it is asserting fires exactly once.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MWeYLrzADoJe5EJS87Lg4M
The scenario was @todo because "no TTS backend changes timbre by style yet". With the ElevenLabs and OpenAI style channels wired, that is no longer true — re-tag it @ts-simulator and point the comment at the binding. Given/When/Then text is byte-identical; the test binds on those literal strings. Also corrects the two other places that repeated the stale claim: the user-simulator-voice test docstring, and the effects recipe's callout, which told every reader the style hint is never applied. That callout now splits the two SDKs explicitly rather than claiming a parity that does not exist — TypeScript routes the style to the provider, Python still warns and drops it. The binding itself lands in the next commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWeYLrzADoJe5EJS87Lg4M
Binds `Per-step voice override applies to only that step`, closing the @ts-simulator set at 5. The `_synthesize` stub records `(text, voice, voiceStyle)` per turn, so the assertion is on what the TTS layer was actually asked for — a scenario that only checked the audio came back would have passed against the old code that dropped the style. Widens the shared `stubSynth` helper to the three-argument `_synthesize` signature. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWeYLrzADoJe5EJS87Lg4M
…dence A passing unit test proves the `[angry]` marker is put on the request; it cannot prove ElevenLabs honours it. `scripts/voice-style-live-proof.ts` runs the real API and discriminates on the transcript, not on bytes — `eleven_v3` is non-deterministic, so two identical unstyled requests already differ. It is the only EL model that CONSUMES an inline marker as a delivery instruction; every other model reads it aloud. The styled clip never says "angry", so the marker was consumed, not voiced. Checked-in evidence under `javascript/outputs/recordings/issue533_voicestyle/` (bare.wav, angry.wav, manifest.json, screenshots) so a reviewer can hear the difference — the one thing no assertion can make. `javascript/outputs/` gets the same gitignore whitelist shape as `python/outputs/`. Also fixes the import/order lint errors in the two touched test files (two of them pre-existing), keeping a zero lint delta on everything this branch edits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWeYLrzADoJe5EJS87Lg4M
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (20)
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 |
…olicy `python/outputs/` carries two READMEs — a parent index and an audio policy at `recordings/`. The javascript side had only the parent. The audio-policy one is where the load-bearing warning belongs: byte-difference is NOT proof, because eleven_v3 is non-deterministic, so any recording claiming "styled differs from unstyled" has to discriminate on something stronger. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MWeYLrzADoJe5EJS87Lg4M
rogeriochaves
left a comment
There was a problem hiding this comment.
there are a bunch of screenshots for the pr that shouldnt be commited here, we should push pr screenshots to langwatch/pr-screenshots instead
|
Automated low-risk assessment This PR was evaluated against the repository's Low-Risk Pull Requests procedure and does not qualify as low risk.
This PR requires a manual review before merging. |
Why
UserSimulatorAgent.setOneShotOverride({ voiceStyle })set_voiceStyleOverride, emitted a one-shotconsole.warn, and then dropped the value — no TTS provider ever saw it. A scenario author writinguser("I'm really upset!", { voiceStyle: "angry" })got cheerful audio and a warning. This threads the style all the way to the wire and re-enables the spec scenario that was retagged@todoin #528.Closes #533
What changed
voiceStyleis a per-callTtsSynthesisOptionsfield, not part of the voice string → alternative was folding it into the litellmprovider/nameidentity (elevenlabs/rachel:angry). Rejected: the style varies per utterance while the voice does not, so baking it into the identity would fragment the provider registry and make a one-turn override a different backend. Consequence:TTSCallablegrew an optional third parameter — source-compatible, andsynthesizeRawonly widens the call when an option is actually set, so a pre-existingtoHaveBeenCalledWith(text, voice)assertion and any third-party 2-arg provider keep their exact call shape.[angry]marker, not tovoiceSettings.style→ the issue said "ElevenLabsstylefield". That field is a numeric 0–1 exaggeration knob, so the string"angry"has nowhere to go, and mapping a name onto a number would be inventing a meaning the API never promised.eleven_v3— already the pinned model, for exactly this reason — consumes inline paralinguistic markers. Consequence: this is the deviation most worth a second opinion; if you want the numeric knob exposed too, that is an additive follow-up, not a rework.instructions, rather than "ignoring it with a warning" → the issue said OpenAI TTS has no style support and "needs design". That is out of date:gpt-4o-mini-tts, the pinned model, exposesinstructionsfor precisely this. Consequence: the feature works on the default provider instead of only on ElevenLabs. The warn-and-ignore path the issue asked for still exists — it now fires for any provider that has not declaredsupportsVoiceStyle, which is the honest home for it.console.warnper provider per process, then the style is stripped (only the style — any other option still reaches the backend).How it works
_voiceStyleOverride === nullkeeps its existing meaning of "no override installed" — not "force unstyled" — so between steps the configured defaults still apply.Test plan
Test Files 97 passed | 1 skipped (98)·Tests 1101 passed | 4 skipped (1105)·tscexit 0.New tests (35 across 6 files, all
#533-tagged):src/agents/__tests__/user-simulator-voice-style.test.ts_synthesizearg 3 and revert; config default; override-beats-config; per-runvoice.tts.voiceStyle; sim-beats-per-run;voiceifyTextscripted pathsrc/voice/tts/__tests__/synthesize-voice-style.test.tssrc/voice/tts/__tests__/elevenlabs-voice-style.test.ts[angry]prepend, bare text without a style, no double-prependsrc/voice/tts/__tests__/openai-voice-style.test.tsinstructionspresent with a style, key absent without onesrc/voice/tts/__tests__/voice-style-unsupported-warning.test.tssrc/agents/__tests__/user-simulator-voice.test.tsLint: every file this branch touches is eslint-clean (
exit 0).maincarries ~122 pre-existing errors in files this PR does not touch, so the claim is a zero delta, not absolute green — two pre-existingimport/ordererrors in a touched test file were fixed on the way past.Human verification
git fetch && git checkout issue533/voicestyle-override && cd javascript && pnpm installnpx vitest run src/voice/tts/__tests__ src/agents/__tests__/user-simulator-voice-style.test.ts→ 6 files, 35 tests green.sed -i 's/this._synthesize(text, voice, voiceStyle)/this._synthesize(text, voice)/' src/agents/user-simulator-agent.ts→ re-run → 7 failures.git checkout -- src/agents/user-simulator-agent.tsto restore.bare.wavis 3.52 s,angry.wavis 4.08 s — same sentence, same voice, same model.ELEVENLABS_API_KEY=... npx tsx scripts/voice-style-live-proof.ts→ regenerates the two clips and re-runs all four checks.How I can prove I was successful
1. The style reaches the real ElevenLabs API and is honoured — proven
Run this session against the live API via
javascript/scripts/voice-style-live-proof.ts.Byte-difference is deliberately NOT the proof. The control line shows
eleven_v3is non-deterministic — two identical unstyled requests returned 168960 vs 161280 bytes. So "styled bytes ≠ unstyled bytes" would have been a fake proof, and this is the trap the harness is built to avoid.The real discriminator is what the audio says.
eleven_v3is the only ElevenLabs model that consumes an inline marker as a delivery instruction; every other model reads it aloud as text. Transcribing both clips with ElevenLabs Scribe (an independent path from the TTS request being asserted about):bare.wavI have been on hold for forty minutes…"I have been on hold for 40 minutes and nobody has helped me."angry.wav[angry] I have been on hold for forty minutes…"I have been on hold for 40 minutes and nobody has helped me"The styled clip never says "angry" and still speaks the full line → the marker was consumed as a directive, not voiced. Plus, through the router: the cache did not collide, and re-reading the unstyled key returned the cached bare bytes rather than the styled ones.
Play them — this is the part no assertion can make:
bare.wavangry.wavmanifest.json— requests, hashes, transcripts, and an explicitnot_provenfield.2. The tests actually fail without the code — proven
Green tests are not evidence, so I mutated production five ways and re-ran. Every mutant was killed, and the tree was restored and re-verified green afterwards:
voiceStylebefore_synthesize(the original #533 bug)applyVoiceStylebecomes identityinstructionsmappingvoiceStyleM1 restores the exact pre-PR behaviour and turns the re-bound spec scenario red — which is the direct evidence that AC-3 and AC-4 are wired to real behaviour and not to a tautology.
3. Acceptance criteria — all four proven
_voiceStyleOverride+ basevoiceStyleinto_synthesizeargsf3a8148effectiveVoiceStyleprecedence covered 6 ways70d282einstructionspresent/absent@unit @ts-simulatorb308c9aspecs/voice-agents.feature— Given/When/Then left byte-identical so the binding matches_synthesizeand the revert3665db64. No user-observable surface
This is a backend-only SDK change with no UI surface — the observable surface is the synthesized audio, which is what §1 exercises against the live provider.
Anything surprising?
b308c9are-tags the spec scenario one commit before3665db6adds its binding, so that single commit has a matched-but-unbound scenario and a red suite. This is inherent to the requested commit-per-criterion order —HEADand every other commit are green.docs/docs/pages/voice/recipes/effects.mdxwas updated to state the split explicitly rather than imply parity, and the Python half is tracked in feat(python): wire voice_style through _synthesize (parity with TS #533) #862.angry.wavsounds angry to a human. No automated check can assert timbre. Everything above proves the marker reached ElevenLabs and was consumed as a directive; whether the delivery is convincing is a judgement for your ears, which is why both clips are checked in.javascript/outputs/is new, mirroringpython/outputs/'s gitignore-whitelist shape. Onlyissue533_voicestyle/is whitelisted; everything else under it stays ignored.