Skip to content

fix: play returned TTS audio path in CLI voice mode - #43131

Closed
valda wants to merge 3 commits into
NousResearch:mainfrom
valda:fix/cli-voice-returned-tts-path
Closed

fix: play returned TTS audio path in CLI voice mode#43131
valda wants to merge 3 commits into
NousResearch:mainfrom
valda:fix/cli-voice-returned-tts-path

Conversation

@valda

@valda valda commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Prefer the requested MP3 for CLI/TUI voice playback when it exists and is non-empty.
  • Fall back to the actual file_path returned by text_to_speech_tool() when the requested MP3 was not produced.
  • Add regression coverage for both classic CLI voice playback and the shared TUI/CLI voice wrapper.

Motivation

CLI/TUI voice playback requested an MP3 path from text_to_speech_tool(), but then ignored the tool response and attempted to play only that originally requested path.

That works for providers that write exactly the requested MP3 file, but breaks for command providers or native-format providers that return a different output file via file_path, such as FLAC/WAV/OGG output.

In those cases the TTS tool can successfully synthesize audio, but CLI/TUI playback skips it because the requested MP3 path does not exist.

At the same time, some voice-compatible providers leave the requested MP3 in place while returning an adjacent OGG generated for messaging compatibility. Local playback should keep using the existing MP3 in that case because players such as afplay do not reliably handle OGG.

Changes

  • Parse the JSON response from text_to_speech_tool().
  • Prefer the requested MP3 when it exists and is non-empty.
  • Fall back to result["file_path"] when the requested MP3 is missing or empty.
  • Clean up both the returned audio path and requested MP3 path, plus adjacent OGG files created by the existing TTS conversion path.
  • Add regression tests covering both returned-path fallback and MP3-over-OGG preference in:
    • HermesCLI._voice_speak_response()
    • hermes_cli.voice.speak_text()

Out of scope

This PR intentionally does not change when the TTS tool converts audio to OGG/Opus.

There appears to be a separate behavior where voice-compatible command providers may be converted to OGG/Opus even for local CLI/TUI playback, even though that conversion should not be necessary when the provider already produced a locally playable native format such as FLAC or WAV. This PR keeps that existing behavior unchanged, but the conversion trigger may be worth revisiting in a follow-up: for local CLI/TUI playback, preserving provider-native audio should be enough when no platform-specific voice-bubble format is required.

This PR only fixes the CLI/TUI caller contract: use the requested MP3 when available, otherwise use the concrete file_path returned by the TTS tool.

Test plan

scripts/run_tests.sh tests/tools/test_voice_cli_integration.py tests/hermes_cli/test_voice_wrapper.py

Result:

132 tests passed, 0 failed

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard tool/tts Text-to-speech and transcription labels Jun 9, 2026
valda added 2 commits June 27, 2026 20:54
Add current email to AUTHOR_MAP and remove a stale duplicate
self-entry. Fixes check-attribution failure for this PR.
@valda
valda force-pushed the fix/cli-voice-returned-tts-path branch from 9478d2d to 4df1f53 Compare June 27, 2026 11:54
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the returned-path contract; the underlying issue is present on current main in both callers.

Problems

  • cli.py:11390-11401 and hermes_cli/voice.py:799-814 intentionally retain the requested MP3 because local afplay has unreliable OGG support. However, tools/tts_tool.py:2398-2407 converts voice_compatible command-provider output to OGG and returns that OGG path even outside a Telegram-specific branch. The proposed unconditional returned-path preference would replace a working requested MP3 with that OGG path.

Suggested changes

  • Prefer the requested MP3 when it exists and is non-empty; use returned file_path only when the requested MP3 is missing. Add CLI and wrapper coverage for returned OGG plus an existing requested MP3, alongside the existing alternate-format case.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 14, 2026
@valda

valda commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for catching this. Updated in 52bf4c060.

Both CLI playback paths now prefer the requested MP3 only when it exists and is non-empty, and fall back to the file_path returned by text_to_speech_tool() when the MP3 was not produced.

This preserves returned-path playback for providers that produce only an alternate format such as FLAC or WAV, while avoiding replacing a usable local MP3 with an OGG generated for messaging compatibility.

I added regression coverage for both paths covering:

  • a returned alternate-format path when the requested MP3 is missing;
  • a returned OGG path when the requested MP3 also exists, verifying that the MP3 is played.

The OGG/Opus conversion behavior itself remains unchanged and out of scope for this PR.

Validation:
scripts/run_tests.sh tests/tools/test_voice_cli_integration.py tests/hermes_cli/test_voice_wrapper.py
— 132 tests passed, 0 failed.

@teknium1

Copy link
Copy Markdown
Contributor

Merged into main via consolidated salvage PR #73520 (merge e04c2a9ebd). Your auto-play of text_to_speech tool-result audio in CLI voice mode (2 commits) was cherry-picked with your authorship.

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

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

3 participants