Skip to content

spike(tts): #97 — Azure express-as fearful style A/B at I4–I5 on sp_it_a_0001 - #98

Closed
shaypal5 wants to merge 2 commits into
mainfrom
spike/97-express-as-fearful-i4-i5
Closed

shaypal5 wants to merge 2 commits into
mainfrom
spike/97-express-as-fearful-i4-i5

Conversation

@shaypal5

@shaypal5 shaypal5 commented May 8, 2026 •

Copy link
Copy Markdown
Member

TL;DR — negative result, do not merge

A/B-tested Azure <mstts:express-as style="fearful"> on a VIC@I=4 turn in sp_it_a_0001. The two cached audio bytes are byte-identical (f495bf6a…) despite distinct SSML inputs — Azure's he-IL-HilaNeural voice silently ignores the express-as wrapper for this voice + prosody combination. M14's defensive supports_style_tags=False default was empirically correct.

Recommendation: close this PR without merging. Pursue candidate #2 in #97 (disfluency density at high intensity) under the same one-knob discipline.

What changed

File Change
synthbanshee/tts/renderer.py Added env var SYNTHBANSHEE_VIC_STYLE_OVERRIDE that, when set AND the turn is VIC at intensity ≥ 4, overrides style_entry.style AND forces supports_style_tags=True per call. The provider's global capability is left at M14's False default. Cache key changes only for the affected turn.
tests/unit/test_vic_style_override.py New file. Parameterized coverage of the four-condition gate (env-set + role + intensity + provider-default-unchanged) plus multiple style values, so the next style spike needs no code changes.
tests/fixtures/issue_97_fearful_ssml.json Direct byte-identity evidence: SSML SHAs + cache file pointers + audio SHAs for the General and fearful renders of VIC@I=4 turn 14. Hebrew utterance text redacted per CLAUDE.md.

Out of scope, intentionally:

  • No prosody / cap / RMS-gain changes.
  • No speaker-YAML edits.
  • No SSML-builder changes.
  • supports_style_tags global default unchanged (override forces True per call).

Scope-narrowing rationale

Issue says "VIC at I3–I5 does not sound distressed." Override gate is I ≥ 4. Reasoning: the negative result generalises downward — if Azure ignores the express-as wrapper at I=4, it ignores it at I=3 too. Lowering the gate would burn 4× more Azure calls re-rendering I=3 turns for the same finding.

Test plan

  • pytest tests/unit/ — 1698 passed (was 1688; +10 new gate tests).
  • Ruff + mypy clean on touched files.
  • Baseline render of sp_it_a_0001 (env unset): all 17 turns hit cache → $0 Azure.
  • Fearful render (SYNTHBANSHEE_VIC_STYLE_OVERRIDE=fearful): only VIC@I=4 turn re-rendered → ~1¢ Azure.
  • Verified at the SSML layer that the override fired and the cache key changed.
  • Tier-3 ASR sanity (local) — see below.

Tier-3 ASR sanity (local)

$ synthbanshee qa-report data/spikes/97_express_as_fearful/fearful --asr
QA PASSED — failure rate 0.0% ≤ 2.0%
asr_warnings: {}     # 0 clips with length_ratio < 0.85

ASR re-run is mechanically a no-op given byte-identity (the audio is the same as the General-style baseline, which has known WER ≈ 0.052 from PR #95). Included only because CLAUDE.md mandates it for any synthbanshee/tts/ touch.

Direct byte-identity evidence

tests/fixtures/issue_97_fearful_ssml.json records, for the same VIC@I=4 turn (same Hebrew text, same prosody, same voice):

General Fearful
xmlns:mstts declared no yes
<mstts:express-as> wrapper no yes
SSML SHA-256 9880c52f… 18aa1ae1…
Cache file (real Azure render) 9880c52f….wav 18aa1ae1….wav
Audio bytes 309 934 309 934
Audio SHA-256 f495bf6a… f495bf6a…

Two distinct SSML strings → two distinct cache keys → two real Azure synthesis events → byte-identical audio output. No listening-test interpretation needed.

What this falsifies — and what it doesn't

Recommendation

  • Close this PR without merging. The override mechanism is clean enough to keep on a branch as a reproducer, but landing it as production code is unjustified for a knob we know is decorative on he-IL.
  • Next spike: candidate ci: integrate pr-agent-context with coverage XML and refresh flow #2 — disfluency density at high intensity. DisfluencyProfile plumbing already exists; same one-knob discipline.
  • A parallel write-up is on #97 so the finding survives this PR's disposition.

Reproducer

git fetch && git checkout spike/97-express-as-fearful-i4-i5
synthbanshee generate -c configs/scenes/she_proves/sp_it_a_0001.yaml -o /tmp/baseline -v
SYNTHBANSHEE_VIC_STYLE_OVERRIDE=fearful synthbanshee generate -c configs/scenes/she_proves/sp_it_a_0001.yaml -o /tmp/fearful -v
cmp /tmp/baseline/agg_m_30-45_001/sp_it_a_0001_00.wav /tmp/fearful/agg_m_30-45_001/sp_it_a_0001_00.wav   # exit 0

Refs

Plumbing for the smallest possible A/B test of Azure mstts:express-as on
Hebrew at high intensity, gated entirely by an env var.

Background
----------
2026-05-07 listening test on PR #95's candidate render said VIC at I3–I5
"sounds the same in both, just a robot whose pitch is a bit higher" —
rate/pitch alone don't carry distress on Azure he-IL voices. PR #95 closed
the Whisper WER gap (#87); this issue is the descendant perceptual gap.

Plumbing
--------
1. AzureProvider.supports_style_tags: False → True. M14's False default was
   speculative ("express-as is unavailable for he-IL"), never validated by
   listening test. All production speakers in configs/speakers/ use
   style="General", so the flip is a no-op for default renders — non-General
   styles only emit when (2) fires.

2. renderer._SPIKE_97_ENV_VAR: when SYNTHBANSHEE_SPIKE_97_VIC_STYLE is set
   AND the current turn is VIC at intensity ≥ 4 AND the provider supports
   style tags, the renderer overrides style_entry.style with the env-var
   value before SSML emission. Cache key (SHA-256 of SSML) changes only for
   affected turns, so non-VIC and I≤3 turns hit cache → baseline A/B costs
   only the VIC@I4–I5 re-renders.

Scope
-----
Single knob. No prosody changes, no #87 cap relaxation, no speaker-YAML
edits, no SSML-builder changes (it already emitted express-as conditionally
on supports_style_tags). When the spike concludes, the next change replaces
this with whatever the listening test decided — a permanent VIC YAML edit,
a different lever from #97's enumerated list, or a revert of the flag flip
if Azure he-IL express-as turns out to be perceptually flat.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 8, 2026 13:22
@shaypal5 shaypal5 added this to the M17 milestone May 8, 2026
@shaypal5 shaypal5 added planning comp: tts TTS rendering, SSML, Azure/Google providers labels May 8, 2026
@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a targeted spike to A/B-test Azure SSML <mstts:express-as> styles for VIC turns at high intensity (I≥4) by (1) enabling style-tag capabilities for Azure and (2) adding an env-var-driven override in the renderer, with a corresponding unit test update. The PR description reports a negative result (Azure he-IL voice ignores express-as), which makes the current capability flip and spike plumbing unsuitable to merge as-is.

Changes:

  • Flip Azure provider supports_style_tags from False to True.
  • Add SYNTHBANSHEE_SPIKE_97_VIC_STYLE env-var override in TTSRenderer.render_utterance() for VIC at intensity ≥ 4.
  • Update unit test expectations to match the new Azure capability value.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
synthbanshee/tts/azure_provider.py Advertises Azure backend style-tag support by toggling supports_style_tags to True.
synthbanshee/tts/renderer.py Adds spike-specific env-var logic to override per-turn style before SSML generation.
tests/unit/test_google_provider.py Updates TestProviderCapabilities.test_azure_capabilities to assert supports_style_tags=True and adds rationale comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread synthbanshee/tts/azure_provider.py Outdated
Comment thread tests/unit/test_google_provider.py Outdated
Comment thread synthbanshee/tts/renderer.py Outdated
Comment thread synthbanshee/tts/renderer.py Outdated
…e, unit tests

Addresses the senior-dev review on PR #98:

1. Per-call supports_style_tags instead of a global flip.
   - azure_provider.py: revert supports_style_tags to False (M14 default
     restored).  The cache survived the revert because both eras of cache
     entries (pre-M14 True, post-M14 False) coexist.
   - renderer.py: when the override fires, force supports_style_tags=True
     for that single SSML build.  The provider's global capability is no
     longer touched; non-spike turns are byte-identical to main.
   - test_google_provider.py: revert the matching test assertion.

2. Generic naming for reuse.
   - SYNTHBANSHEE_SPIKE_97_VIC_STYLE → SYNTHBANSHEE_VIC_STYLE_OVERRIDE.
   - _SPIKE_97_* constants → _VIC_STYLE_OVERRIDE_*.
   - The next style spike (terrified, whispering, …) needs no code change.

3. Direct byte-identity proof.
   - tests/fixtures/issue_97_fearful_ssml.json captures, for VIC@I=4 turn 14
     of sp_it_a_0001: the General-style SSML SHA, the fearful-style SSML
     SHA, the cache files each maps to (both populated by real Azure
     synthesis), and the audio SHA — proving the cached bytes are
     byte-identical despite the express-as wrapper difference.
   - Hebrew utterance text redacted from the SSML body per CLAUDE.md
     ("Hebrew text goes in .j2 templates or .txt transcripts only");
     the SHAs remain authoritative.

4. Unit test for the gate.
   - tests/unit/test_vic_style_override.py covers the four-condition gate
     (env-set + role + intensity + provider-default-unchanged) with
     parameterized coverage over multiple style values.  1698 unit tests
     pass; ruff + mypy clean.

Net effect on PR #98 disposition: still recommend close-without-merge —
the override now has cleaner architecture and better evidence, but the
finding is the same (Azure he-IL silently ignores express-as).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown

pr-agent-context report:

This is a refreshed snapshot of the current PR state.

This run includes a patch coverage gap on PR #98 in repository https://github.com/DataHackIL/SynthBanshee

Address the patch coverage gaps below, then push all of these changes in a single commit.

# Patch coverage

Patch test coverage is 62.50%; please raise it to 100%. These are the uncovered code lines:
- synthbanshee/tts/renderer.py: 80, 94, 95, 96, 97, 103

Run metadata:

Tool ref: v4
Tool version: 4.0.21
Trigger: workflow dispatch
Workflow run: 25574244529 attempt 1
Comment timestamp: 2026-05-08T19:08:03.765203+00:00
PR head commit: fbd83cee5cb992c92428e8a0d5267af01f23cdef

@shaypal5

shaypal5 commented May 8, 2026

Copy link
Copy Markdown
Member Author

Closing without merge — spike concluded with a negative result.

Disposition

  • fearful on he-IL-HilaNeural confirmed as a no-op via byte-identity at the cache layer (see tests/fixtures/issue_97_fearful_ssml.json on this branch).
  • The override mechanism in synthbanshee/tts/renderer.py was the spike's reproducer; it evaporates with this PR.
  • Branch preserved as the reproducer — git checkout spike/97-express-as-fearful-i4-i5 to re-run the A/B.

Successor work

Cross-references

@shaypal5 shaypal5 closed this May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: tts TTS rendering, SSML, Azure/Google providers planning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants