Skip to content

fix(desktop): harden read aloud synthesis and playback - #76757

Open
hermz580 wants to merge 1 commit into
NousResearch:mainfrom
hermz580:fix/read-aloud-code-block-summary
Open

fix(desktop): harden read aloud synthesis and playback#76757
hermz580 wants to merge 1 commit into
NousResearch:mainfrom
hermz580:fix/read-aloud-code-block-summary

Conversation

@hermz580

@hermz580 hermz580 commented Aug 2, 2026

Copy link
Copy Markdown

Summary

  • summarize fenced code blocks before desktop Read Aloud so TTS does not narrate large code dumps literally
  • bound /api/audio/speak synthesis with a server-side timeout and return a clean 504 when a provider stalls
  • fail open when streaming WebAudio cannot initialize by falling back to the plain audio playback path

Why

Desktop Read Aloud could fail in two user-visible ways: long/code-heavy assistant replies could make provider synthesis stall, and Windows/Electron AudioContext failures could throw before playback had a chance to fall back. This keeps optional WebAudio streaming off the critical path and gives the POST path a bounded failure mode.

Test plan

  • npm test --workspace apps/desktop -- hermes.test.ts voice-playback.test.ts speech-text.test.ts
  • python -m pytest tests/hermes_cli/test_web_server_speak_stream.py tests/hermes_cli/test_web_server.py::TestWebServerEndpoints::test_speak_text_times_out_stalled_provider tests/hermes_cli/test_web_server.py::TestWebServerEndpoints::test_speak_text_returns_base64_data_url tests/hermes_cli/test_web_server.py::TestWebServerEndpoints::test_speak_text_requires_nonempty_text -q -o 'addopts='\n- npm run typecheck --workspace apps/desktop\n- python -m py_compile hermes_cli/web_server.py\n- npm run build --workspace apps/desktop

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) comp/cli CLI entry point, hermes_cli/, setup wizard tool/tts Text-to-speech and transcription labels Aug 2, 2026

@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 the focused desktop hardening. The AudioContext fallback addresses a current unguarded constructor at apps/desktop/src/lib/voice-playback.ts:275; the fenced-code Read Aloud change is already on main as f02d41cb2161.

Problems

  • The POST patch dispatches text_to_speech_tool directly, but current main scopes this endpoint through _speak_scoped / _config_profile_scope(profile) at hermes_cli/web_server.py:4400-4409 (5f1c400e72). Retain that wrapper or non-default desktop profiles will resolve the dashboard profile's TTS configuration.
  • asyncio.wait_for ends the HTTP wait but does not provide cleanup for a synthesis worker that finishes later. The endpoint only unlinks after result_json returns (hermes_cli/web_server.py:4418-4450), while the default TTS path creates a cache file (tools/tts_tool.py:2907-2923). The added delayed-provider test returns after the timeout without checking that late output is removed.

Suggested changes

  • Wrap run_in_executor(None, _speak_scoped) with the timeout.
  • Use a request-owned output path and arrange late-worker cleanup; extend the timeout test to verify it.

Automated hermes-sweeper review.

Comment thread hermes_cli/web_server.py
result_json = await loop.run_in_executor(None, text_to_speech_tool, text)
result_json = await asyncio.wait_for(
loop.run_in_executor(None, text_to_speech_tool, text),
timeout=DESKTOP_TTS_TIMEOUT_SECONDS,

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.

Current main runs this endpoint through _speak_scoped so ?profile= selects that profile's TTS configuration (hermes_cli/web_server.py:4400-4409, added by 5f1c400e72). Keep the timeout around loop.run_in_executor(None, _speak_scoped) rather than calling text_to_speech_tool directly.

Comment thread hermes_cli/web_server.py Outdated
@@ -4524,7 +4527,17 @@ async def speak_text(payload: TTSSpeakRequest):
try:
from tools.tts_tool import text_to_speech_tool
loop = asyncio.get_running_loop()
result_json = await loop.run_in_executor(None, text_to_speech_tool, text)
result_json = await asyncio.wait_for(
loop.run_in_executor(None, text_to_speech_tool, text),

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.

wait_for returns the 504 while the executor worker may still finish. Because normal cleanup only happens after result_json is processed and default TTS writes a cache file, late successful synthesis can leave audio behind. Use a request-owned output path and arrange completion-time cleanup for timed-out work; extend the delayed-provider test to assert it.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Aug 2, 2026
@hermz580
hermz580 force-pushed the fix/read-aloud-code-block-summary branch from 221cf46 to 0b67d41 Compare August 2, 2026 13:54
@andrexibiza

Copy link
Copy Markdown
Contributor

Vox Lockin lane 09 verification receipt — mergeable, no gaps found in the desktop class.

  • Merge-clean against current main (70db671): git merge-tree reports no conflicts.
  • Reviewed the desktop delta: unlockAutoplay and new AudioContext() in openSpeechStream are wrapped so an unavailable AudioContext degrades to the whole-text fallback path (settle('fallback')) with a console warning instead of a silent 'speaking forever' wedge — matches the stall-hardening intent of the read-aloud class.
  • Tests: apps/desktop/src/lib/voice-playback.test.ts passes on PR head (1/1).
  • The hermes_cli/web_server.py portion is outside the desktop hook lane (gateway/API-server territory) but is additive and doesn't change the desktop contract.

Recommend merge.

@hermz580

hermz580 commented Aug 8, 2026

Copy link
Copy Markdown
Author

Hi @teknium1 — gentle follow-up r feedback (Aug 2) was addressed in the force-push to 0b67d41: the timeout now wraps run_in_executor(None, _speak_scoped) so ?profile= still resolves the profile-scoped TTS config, timed-out synthesis uses a request-owned output path, and a done-callback removes late audio from timed-out requests (delayed-provider test extended to assert the cleanup). @andrexibiza's Vox Lockin verification (Aug 4) confirmed mergeable — no gaps found in the desktop class — and recommended merge. Is there anything else needed from our side before this can merge? Happy to adjust.

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 comp/desktop Electron desktop app (apps/desktop/*) 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 sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows 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