fix(gateway): thread auto-TTS reply + support voice_out_carries_text adapters - #32655
fix(gateway): thread auto-TTS reply + support voice_out_carries_text adapters#32655cristianmgm7 wants to merge 3 commits into
Conversation
…adapters
Two related bugs in the auto-TTS dispatch
(``gateway/platforms/base.py:_run_message_response``) that surface on
every voice-out platform but only became visible while wiring TTS
into a new adapter (Carbon Voice):
### Bug 1 — voice memo arrives as a top-level post, not a reply
The dispatch calls ``play_tts(...)`` to ship the synthesized audio,
but never passes ``reply_to``. The parallel text path (a few lines
below) computes ``_reply_anchor = _reply_anchor_for_event(event)``
and threads correctly; the TTS path doesn't, so the audio bubble
arrives as a top-level message even when the user clearly replied
to an existing thread.
Symptom on the recipient side: their inbound is in a thread; the
agent's voice memo answer arrives at the top of the conversation,
detached from context.
Fix: also pass ``reply_to=_reply_anchor_for_event(event)`` to
``play_tts``. Adapters whose ``send_voice`` ignores ``reply_to``
discard it harmlessly via ``**kwargs``.
### Bug 2 — duplicate text bubble after every TTS reply
``_tts_caption_delivered`` only fires for Telegram (via the
``telegram_tts_caption`` path, which actually carries the text as
the audio message's caption field). Every other voice-out platform
falls through to the text-send branch below and ships a duplicate
text bubble right after the voice memo.
For platforms that *re-render the spoken text inside the voice
bubble themselves* — Carbon Voice runs server-side STT on uploaded
audio and shows the transcript inline; future similar platforms —
that duplicate is pure noise.
Fix: introduce a new class attribute on ``BasePlatformAdapter``:
voice_out_carries_text: bool = False
Subclasses opt in by overriding to True. The
``_tts_caption_delivered`` check is widened to:
bool(
(telegram_tts_caption
or getattr(self, "voice_out_carries_text", False))
and getattr(tts_result, "success", False)
)
Default False keeps every existing adapter's behavior unchanged.
Only adapters that explicitly opt in suppress the duplicate text
send — same opt-in shape as ``REQUIRES_EDIT_FINALIZE``.
### Test plan
New ``tests/gateway/test_tts_voice_out.py`` (5 cases, all passing):
- ``voice_out_carries_text`` defaults to False on
``BasePlatformAdapter``.
- A subclass can override to True without affecting the base.
- A subclass that doesn't override inherits False.
- The TTS dispatch in ``base.py`` calls ``play_tts`` with
``reply_to=`` (source-grep guard).
- The TTS dispatch reads ``voice_out_carries_text`` (source-grep
guard).
The dispatch flow itself is exercised by existing per-platform
integration tests; the new unit tests just pin the contracts so
they can't quietly regress.
### Discovered while
…wiring TTS through the Carbon Voice plugin
(``PhononX/hermes-plugin-carbonvoice``). CV transcribes uploaded
audio server-side, so the voice memo bubble already shows the
spoken text — making the duplicate text-send obvious as soon as
the auto-TTS path engaged. The threading bug surfaced in the same
test: replies in DMs and group threads arrived top-level.
alt-glitch
added
P2
|
Hi @Zyrixtrex — your commit Also fixes a parallel asymmetry that lives in the same dispatch block: Discovered while wiring TTS through a Carbon Voice plugin (PhononX/hermes-plugin-carbonvoice) where both bugs surfaced together — CV transcribes uploaded audio server-side, so the duplicate text bubble was immediately visible, and the non-threaded audio was obvious in DM/group threads. Local patch validates fully end-to-end; the PR's 5 unit tests pin both contracts as source-grep guards plus class-attribute defaults. Would love your eyes on it whenever you have a moment 🙏 |
|
Thanks for identifying the auto-TTS reply-threading asymmetry. The first premise is still present on current main: Problems
Suggested changes
Automated hermes-sweeper review. |
teknium1
added
sweeper:risk-message-delivery
… full response Review feedback on NousResearch#32655: prepare_tts_text() strips markdown and truncates to 4000 chars, so an opted-in voice_out_carries_text adapter could suppress the full text bubble even when the voice transcript couldn't carry all of it. - Gate the suppression on _tts_speech_is_complete (speech_text equals the untouched text_content); prepared/truncated responses keep the follow-up text send. - Replace the source-grep assertions with behavioral tests in tests/gateway/test_base_topic_sessions.py driving _process_message_background end-to-end: reply_to anchor on play_tts, successful opt-in suppression, failed-TTS fallback, long (truncated) reply, formatted (stripped) reply, and non-opted-in default. - test_tts_voice_out.py now only pins the class-attribute contract.
…ce-out-carries-text # Conflicts: # gateway/platforms/base.py
|
Thanks for the review — both points addressed. Content loss on suppression → fixed in 26b6707. The dispatch now tracks Source-string assertions → replaced with behavioral tests in
Also merged current main (ad18a4a) to resolve the overlap with the |
…earch#32655 Drops this PR's own voice_out_carries_text implementation in base.py: the same contract already exists in PR NousResearch#32655, where it is further along in review and hardened per feedback (completeness guard so a truncated TTS rendition never suppresses the full text, plus reply_to threading for the voice memo and behavioral tests). The plugin keeps declaring voice_out_carries_text = True; until NousResearch#32655 lands the attribute is simply ignored (voice-out delivers audio plus a duplicate text bubble — degraded UX, never lost content). This PR is now strictly zero core changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Cross-reference: #43226 (Carbon Voice platform plugin) declares |
Verification note (Vox Lockin lane 02 — auto-TTS / voice-mode reply class)Verified against Finding: not mergeable as-is; out of the lane-02 corpus class; no duplicate/salvage from this lane.
Recommendation: rebase onto main, drop the superseded threading hunks, keep the |
4 participants
Summary
Two related bugs in the auto-TTS dispatch (`gateway/platforms/base.py:_run_message_response`) that surface on every voice-out platform but only became visible while wiring TTS into a new adapter (Carbon Voice). Default behavior for existing adapters is unchanged.
Bug 1 — voice memo arrives as a top-level post, not a reply
The dispatch calls `play_tts(...)` to ship the synthesized audio, but never passes `reply_to`. The parallel text path a few lines below computes `_reply_anchor = _reply_anchor_for_event(event)` and threads correctly; the TTS path doesn't, so the audio bubble arrives detached from context.
Repro on any platform whose `send_voice` honors `reply_to` (Telegram, Carbon Voice, Matrix, …):
Fix: also pass `reply_to=_reply_anchor_for_event(event)` to `play_tts`. Adapters whose `send_voice` ignores `reply_to` discard it harmlessly via `**kwargs`.
Bug 2 — duplicate text bubble after every TTS reply
`_tts_caption_delivered` only fires for Telegram (via the `telegram_tts_caption` path, which carries the text as the audio's caption field). Every other voice-out platform falls through to the text-send branch and ships a duplicate text bubble right after the voice memo.
For platforms that re-render the spoken text inside the voice bubble themselves — e.g. Carbon Voice runs server-side STT on uploaded audio and shows the transcript inline — that duplicate is pure noise.
Fix: introduce a class-level opt-in on `BasePlatformAdapter`:
```python
voice_out_carries_text: bool = False
```
Subclasses override to True when their voice messages inherently carry the spoken text. The `_tts_caption_delivered` check is widened to:
```python
_tts_caption_delivered = bool(
(
telegram_tts_caption
or getattr(self, "voice_out_carries_text", False)
)
and getattr(tts_result, "success", False)
)
```
Default False — every existing adapter is unaffected. Only adapters that explicitly opt in suppress the duplicate text send. Same opt-in shape as `REQUIRES_EDIT_FINALIZE` a few lines above.
Test plan
New `tests/gateway/test_tts_voice_out.py` (5 cases, all passing locally):
The dispatch flow itself is exercised by existing per-platform integration tests; the new unit tests pin the two contracts so they can't quietly regress.
Backwards compatibility
Discovered while
…wiring TTS through the Carbon Voice plugin (`PhononX/hermes-plugin-carbonvoice`). CV transcribes uploaded audio server-side, so the voice memo bubble already shows the spoken text — making the duplicate text-send obvious as soon as the auto-TTS path engaged. The threading bug surfaced in the same test: replies in DMs and group threads arrived top-level instead of nested under the user's message.