Skip to content

fix(desktop): respect voice.auto_tts setting in voice conversation mode (fixes #44263) - #44277

Open
Morad37 wants to merge 3 commits into
NousResearch:mainfrom
Morad37:fix/voice-auto-tts-44263
Open

fix(desktop): respect voice.auto_tts setting in voice conversation mode (fixes #44263)#44277
Morad37 wants to merge 3 commits into
NousResearch:mainfrom
Morad37:fix/voice-auto-tts-44263

Conversation

@Morad37

@Morad37 Morad37 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Voice chat was calling playSpeechText without checking voice.auto_tts. Now it reads the config once on first use and skips TTS when disabled. The response still appears as text either way.

@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 duplicate This issue or pull request already exists labels Jun 11, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #44263 — this fixes the bug half of that report (Desktop Voice Chat mode ignores voice.auto_tts and always reads replies aloud). #44263 also requests a real-time streaming voice mode, which this PR does not implement. Related: #43845 (auto-TTS toggle).

@Morad37

Morad37 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for taking a look. This PR is the fix for the bug portion of #44263 (auto_tts ignored in voice mode). The streaming feature request is separate — I'd leave that for another PR once the API supports it.

@harjothkhara

Copy link
Copy Markdown
Contributor

Nice — gating on voice.auto_tts is the right call, and routing the skip through speak() keeps the conversation loop moving when it's off. Two changes I'd suggest before merge, both because the gate currently fails open where #44263 wants it to fail closed ("must not read replies aloud unless explicitly enabled"):

1. Default to off. config.voice?.auto_tts !== false enables on a missing key (undefined !== falsetrue), and the catch sets true on a config-read failure — so an unset value or a transient read error both result in speech. Suggest === true, and false in the catch. The backend default is already auto_tts: False (hermes_cli/config.py), so strict-true matches it.

2. Don't cache for the hook's lifetime. It's read once via if (autoTtsRef.current === null), so toggling "Read Responses Aloud" in Settings or switching gateway profiles won't take effect until the view remounts — the gate silently lags the config.

For (2), one option is to hydrate a small shared value from useHermesConfig (it already fetches config on boot / settings-save / profile-change) and have the voice loop subscribe — it then refreshes whenever config does, with no per-hook cache to go stale. I have a tested version of that approach (fail-closed default, a generation guard so an old profile's in-flight fetch can't clobber a newer one, and vitest coverage including a streaming turn). Happy to share it for you to fold in or use as a starting point — whichever you'd prefer.

A test asserting playSpeechText is never called when auto_tts is unset/false would also lock in the security-relevant property.

- Change default from \!== false (fails open) to === true (fails closed)
- Stop caching auto_tts at the ref level for the hook's lifetime
- Store auto_tts in a nanostore () updated by useHermesConfig
  whenever config refreshes (boot, settings-save, profile-change)

Co-authored-by: harjothkhara <harjothkhara@users.noreply.github.com>
@Morad37

Morad37 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Good catches on both points. Fixed both in the push I just made:

  1. Fail-closed default: Changed to === true with false in the catch. An unset config or transient read error both result in silence now instead of speech.

  2. Cached config: Pulled the auto_tts read out of useRef entirely. Added a $autoTts nanostore to store/voice-playback.ts that gets updated by useHermesConfig.refreshHermesConfig() whenever config refreshes — boot, settings save, profile switch. The voice hook just calls $autoTts.get() each time speak() runs, so toggling "Read Responses Aloud" in Settings takes effect immediately.

The approach with a shared store felt cleaner than re-reading config on every speak() call since useHermesConfig already has the config and the refresh path.

Let me know if you'd prefer a different shape for the store integration.

@alt-glitch alt-glitch added comp/desktop Electron desktop app (apps/desktop/*) duplicate This issue or pull request already exists and removed duplicate This issue or pull request already exists comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 27, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Label correction on re-triage: this PR was previously tagged duplicate of #44263, but #44263 is an issue (the bug report) and a fix PR is never a duplicate of the issue it fixes. Re-tagged as related to #44263 instead (and to sibling auto-TTS PR #43845). The streaming-voice request in #44263 remains a separate scope, as the author noted.

@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 targeting the voice-conversation path specifically. The bug remains on current main: apps/desktop/src/app/chat/composer/hooks/use-voice-conversation.ts:228 still unconditionally calls playSpeechText, while hermes_cli/config.py:2153 defaults voice.auto_tts to False.

Problems

  • The PR's new $autoTts mirror now overlaps with main's canonical desktop store: apps/desktop/src/store/voice-prefs.ts:8 owns $autoSpeakReplies, and apps/desktop/src/app/session/hooks/use-hermes-config.ts:90 hydrates it from config. Use that existing store for the conversation gate rather than adding a second atom and refresh path.
  • The PR adds no regression coverage for this contract. Add a voice-conversation test proving disabled/unset config never invokes playSpeechText, while enabled config still does and the loop continues.

Suggested changes

  • Gate speak() with $autoSpeakReplies.get() from voice-prefs and retain the existing skip-to-idle/listening behavior.

Automated hermes-sweeper review.

}

/** Whether auto-TTS is enabled. Defaults to false (fail-closed). Updated
* when config refreshes so toggling the setting takes effect immediately. */

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 already has the fail-closed voice.auto_tts mirror in apps/desktop/src/store/voice-prefs.ts:8, hydrated by useHermesConfig at apps/desktop/src/app/session/hooks/use-hermes-config.ts:90. Please use that existing $autoSpeakReplies store for the voice-conversation gate instead of introducing a second mirror that can diverge.

@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 14, 2026
@andrexibiza

Copy link
Copy Markdown
Contributor

Adversarial verification (Vox Lockin lane 02 — auto-TTS / voice-mode reply class)

Verified against origin/main @ 70db671fac (2026-08-04). Head SHA tested: 2a68da86f4.

Verdict: logic is correct and matches house conventions for #44263; the branch needs a rebase before merge. No duplicate PR opened.

What I checked

  1. Premise on main: apps/desktop/src/app/chat/composer/hooks/use-voice-conversation.ts still speaks every reply in voice-conversation mode unconditionally (only a muted gate); it does not consult voice.auto_tts / the "Read replies aloud" toggle. Premise of ## 1. Bug: Voice Chat Mode Cannot Disable TTS #44263 confirmed on current main.
  2. PR logic (probes = static + cross-check with merged conventions):
    • speak() now gates on a $autoTts nanostore: if (!$autoTts.get()) { skip speech, keep conversation moving } — fail-closed default (atom(false)), matching the already-merged $autoSpeakReplies convention (applyAutoSpeakFromConfigBoolean(config?.voice?.auto_tts) in apps/desktop/src/store/voice-prefs.ts, PR feat(desktop): read replies aloud (auto-TTS) composer toggle #55154).
    • use-hermes-config.ts refreshes the store on every config refresh (setAutoTts(config.voice?.auto_tts === true)), so boot / settings-save / profile-change take effect without restart — this also addresses the "cached once per hook lifetime" flaw of the previous autoTtsRef approach.
    • No change to the STT/transcription side; TTS-off still keeps the loop moving (text-only conversation), which is the behavior the issue asks for.
  3. Mergeability: BLOCKED — conflicts with current main. Merge-base is e6de6dd559 (2026-06-06); git apply --3way of origin/main...2a68da86f4 conflicts on use-voice-conversation.ts and use-hermes-config.ts (the conversation hook was substantially refactored upstream since June — live speech streaming, barge-in monitor, thinking-sound, etc.). The store files (voice-playback.ts, types/hermes.ts) apply cleanly.
  4. Tests: the PR adds no automated tests; the repo's desktop hook tests (use-voice-conversation.test.tsx, use-voice-conversation-rearm.test.tsx) would need a rebase-run. The hook-level logic is simple enough to review statically, but a rebase + those suites are required before merge.

Recommended path for the author

Rebase fix/voice-conversation-auto-tts onto current main (the conflicts are contextual, not semantic — the speak() gate should transplant cleanly onto the refactored hook), re-run apps/desktop hook tests, and consider a unit test for the $autoTts gate. Happy to re-verify after rebase.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have 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 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.

5 participants