Skip to content

fix: remove default ssml config value for cartesia tts - #2284

Open
YiminW wants to merge 1 commit into
mainfrom
dev/cartesia_tts_remove_default_smml
Open

fix: remove default ssml config value for cartesia tts#2284
YiminW wants to merge 1 commit into
mainfrom
dev/cartesia_tts_remove_default_smml

Conversation

@YiminW

@YiminW YiminW commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown

Reviewed. The fix is correct and well-targeted.

I verified the failure it addresses: property.json shipped ssml.enabled: true together with emotion: "angry", and _compose_ssml_text in extension.py only early-returns when enabled is falsy — so every utterance synthesized with the stock config was prefixed with <emotion value="angry"/>. Removing the block makes CartesiaSSMLConfig supply the defaults instead (enabled: bool = False), so no tags are emitted unless a user opts in. Good call keeping the ssml schema in manifest.json: the property stays supported and documented, it just is no longer forced on anyone. The version bumps are consistent — manifest.json and pyproject.toml both go 0.8.2 to 0.8.3.

The fix looks incomplete: three test configs still carry the same block. tests/configs/ still has the identical ssml block, emotion: "angry" included, in property_basic_audio_setting1.json, property_basic_audio_setting2.json, and property_dump.json. These are not dead files: the tts_guarder integration suite loads two of them by name (ai_agents/agents/integration_tests/tts_guarder/tests/test_basic_audio_setting.py defines TTS_BASIC_AUDIO_SETTING_CONFIG_FILE1/2 as exactly those filenames). So after this PR the audio-setting and dump tests still exercise SSML-enabled, angry-emotion synthesis, a configuration that no longer matches anything the extension ships. That is the wrong way round: the tests validating audio output should run the default path, otherwise the regression you just fixed would not be caught if it returned. I would drop the ssml block from all three here, since it is the same edit.

No regression test pins the fix. test_ssml_presets and test_ssml_metadata_overrides both pass ssml explicitly in their inline config, so neither broke nor covers this change. A short test asserting that a config of just {"params": {"api_key": "..."}} reaches the client with text byte-identical to the input would lock the behavior in cheaply — the ExtensionTesterSSML harness already captures t.text.

Latent issue in CartesiaSSMLConfig.merge (pre-existing, not introduced here). merge skips incoming None values, but enabled is a non-optional bool defaulting to False, so a partial patch such as {"ssml": {"emotion": "sad"}} constructs enabled=False and silently overwrites an enabled base. Harmless now that the base default is also False, and this PR incidentally defuses it — but if a preset is ever reintroduced as a default it will bite. Declaring enabled: bool | None = None and resolving to False at read time would close it properly.

Convention nits. Per AGENTS.md, branches are type/short-description, hyphen-separated — dev/cartesia_tts_remove_default_smml uses a dev/ prefix and underscores, and has a typo (smml should be ssml). The commit title itself is formatted correctly. The PR body is empty; one line naming the symptom (all speech rendered angry) would help whoever bisects this later.

No blockers apart from the leftover test configs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant