Skip to content

fix(gateway): honor auto TTS for text replies - #51196

Closed
55nx954gn6-debug wants to merge 1 commit into
NousResearch:mainfrom
55nx954gn6-debug:fix/auto-tts-text-replies
Closed

fix(gateway): honor auto TTS for text replies#51196
55nx954gn6-debug wants to merge 1 commit into
NousResearch:mainfrom
55nx954gn6-debug:fix/auto-tts-text-replies

Conversation

@55nx954gn6-debug

Copy link
Copy Markdown
Contributor

Summary

  • honor platform adapter auto-TTS defaults when deciding whether final text replies should also receive voice output
  • preserve explicit per-chat /voice off as a hard override
  • keep voice_only limited to voice input replies

Validation

  • ./venv/bin/python -m pytest tests/gateway/test_auto_voice_reply_format.py -q -o 'addopts='

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery tool/tts Text-to-speech and transcription P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jun 23, 2026

@tonydwb tonydwb 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.

Code Review Summary

Verdict: Approved

Overview

Fixes _should_send_voice_reply to honor the adapter._should_auto_tts_for_chat() hook so that text replies trigger TTS when voice.auto_tts is enabled at the adapter level.

Analysis

  • Targeted bug fix with clear behavioral description
  • Adds adapter-level auto_tts check alongside the existing _voice_mode check
  • Explicit off override still respected
  • Debug logging added for skipped replies
  • 3 new unit tests covering the key scenarios

✅ Looks Good

  • Clean, minimal change
  • Good test coverage
  • Debug logging won't impact production (uses logger.debug)

Reviewed by Hermes Agent

@tonydwb tonydwb 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.

Code Review Summary

Verdict: Approved

Fixes the gateway auto-TTS logic to properly honor auto_tts: true for text replies (not just voice/speech replies). Previously, text-mode replies with auto_tts: true would skip the TTS generation.

Changes

  • gateway/run.py: +20/-1
  • tests/gateway/test_auto_voice_reply_format.py: +35/-3

Quality

  • Small, targeted fix (2 files)
  • Good test coverage added
  • No security concerns

Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for identifying the missing final-text-reply path; current main still lacks adapter/default auto-TTS in gateway/run.py:13108-13113.

Problems

  • gateway/run.py:10949 also enables TTS for an explicit /voice on chat. That command records voice_only and adds the chat to the adapter enabled set (gateway/slash_commands.py:2469-2474), while _should_auto_tts_for_chat() returns true for enabled chats (gateway/platforms/base.py:2677-2687). A text input would therefore receive audio, contrary to the existing voice_only contract in tests/gateway/test_voice_command.py:352-354.
  • The new voice_only test at tests/gateway/test_auto_voice_reply_format.py:95-103 has no adapter configured, so it does not exercise that state.

Suggested changes

  • Apply the adapter-default branch only when voice_mode is None, retaining the existing explicit all and voice-input voice_only cases.
  • Add a text-input regression test with voice_only and an adapter predicate returning true.

Automated hermes-sweeper review.

Comment thread gateway/run.py
# explicitly turned off. The base adapter's own auto-TTS path only
# covers voice-input replies, so final text replies need the runner
# path here.
or (voice_mode != "off" and adapter_auto_tts)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_should_auto_tts_for_chat() is also true for a chat explicitly enabled by /voice on, whose persisted mode is voice_only. This branch would then send TTS for text input, violating the existing voice_only contract. Restrict the adapter-default branch to voice_mode is None and add a regression test with a true adapter predicate.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
teknium1 pushed a commit that referenced this pull request Jul 28, 2026
Salvaged from PR #51196 (@55nx954gn6-debug). _should_send_voice_reply only
consulted the runner's _voice_mode dict (/voice on|voice_only|all), so the
global voice.auto_tts config default — which is synced into each adapter's
_auto_tts_default on gateway connect — was invisible to the runner path.
Net effect: with streaming enabled and only global auto-TTS configured
(no per-chat /voice opt-in), the streamed reply consumed the text, the
base adapter's auto-TTS got text_content=None, and no voice reply was
ever sent (#51867/#23983 remainder).

The runner now also asks the adapter's _should_auto_tts_for_chat(chat_id)
(which encodes per-chat /voice on|off overrides over the global default);
an explicit /voice off chat mode remains a hard override.

Refs #51867 #23983 #51282 #13126
teknium1 pushed a commit that referenced this pull request Jul 28, 2026
Salvaged from PR #51196 (@55nx954gn6-debug). _should_send_voice_reply only
consulted the runner's _voice_mode dict (/voice on|voice_only|all), so the
global voice.auto_tts config default — which is synced into each adapter's
_auto_tts_default on gateway connect — was invisible to the runner path.
Net effect: with streaming enabled and only global auto-TTS configured
(no per-chat /voice opt-in), the streamed reply consumed the text, the
base adapter's auto-TTS got text_content=None, and no voice reply was
ever sent (#51867/#23983 remainder).

The runner now also asks the adapter's _should_auto_tts_for_chat(chat_id)
(which encodes per-chat /voice on|off overrides over the global default);
an explicit /voice off chat mode remains a hard override.

Refs #51867 #23983 #51282 #13126
@teknium1

Copy link
Copy Markdown
Contributor

Merged into main via consolidated salvage PR #73508 (merge f440a44753). Your adapter auto-TTS check in _should_send_voice_reply — closing the streamed/global auto-TTS gap — was cherry-picked with your authorship.

Your contribution is credited to you in git history. Thank you! Closing this PR as merged-via-salvage.

@teknium1 teknium1 closed this Jul 29, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
Salvaged from PR NousResearch#51196 (@55nx954gn6-debug). _should_send_voice_reply only
consulted the runner's _voice_mode dict (/voice on|voice_only|all), so the
global voice.auto_tts config default — which is synced into each adapter's
_auto_tts_default on gateway connect — was invisible to the runner path.
Net effect: with streaming enabled and only global auto-TTS configured
(no per-chat /voice opt-in), the streamed reply consumed the text, the
base adapter's auto-TTS got text_content=None, and no voice reply was
ever sent (NousResearch#51867/NousResearch#23983 remainder).

The runner now also asks the adapter's _should_auto_tts_for_chat(chat_id)
(which encodes per-chat /voice on|off overrides over the global default);
an explicit /voice off chat mode remains a hard override.

Refs NousResearch#51867 NousResearch#23983 NousResearch#51282 NousResearch#13126
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages tool/tts Text-to-speech and transcription type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants