Add Linux TTS support (espeak/spd-say)#1
Conversation
Voice coaching was hardcoded to macOS `say`. This adds platform detection so Linux users get TTS via `espeak` or `spd-say` while preserving the existing macOS path unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's accessibility by introducing Text-to-Speech (TTS) capabilities for Linux users, mirroring the existing functionality for macOS. It refactors the voice coaching logic to intelligently detect the operating system and available TTS engines, providing a seamless experience across platforms while gracefully handling cases where no TTS is installed. This change broadens the user base who can benefit from audible coaching cues. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds text-to-speech support for Linux using espeak or spd-say, complementing the existing macOS support. The changes in coach.py introduce platform detection for TTS commands, and plank.sh is updated with a wrapper for cross-platform speech. The README is also updated accordingly.
My review found a critical bug in coach.py that would cause a NameError at runtime, along with a suggestion to use a more robust standard library function. I also identified a minor logic issue in the new say_sync behavior and suggested a refactoring for the plank.sh script to improve efficiency and robustness. Overall, a great addition with a few areas for refinement.
| def _which(cmd: str) -> bool: | ||
| """Check whether *cmd* is on PATH (cached).""" | ||
| if cmd not in _which_cache: | ||
| _which_cache[cmd] = ( | ||
| subprocess.call( | ||
| ["which", cmd], | ||
| stdout=subprocess.DEVNULL, | ||
| stderr=subprocess.DEVNULL, | ||
| ) | ||
| == 0 | ||
| ) | ||
| return _which_cache[cmd] | ||
|
|
||
|
|
||
| _which_cache: dict[str, bool] = {} |
There was a problem hiding this comment.
There are two issues in this block:
_which_cacheis used in_whichbefore it is defined, which will cause aNameErrorat runtime.- The implementation of
_whichusingsubprocess.call(['which', ...])is not robust. If thewhichcommand is not available on the system, it will raise an unhandledFileNotFoundError.
I suggest moving the definition of _which_cache before _which and using the more robust and standard shutil.which to find executables. This will fix the bug and make the code cleaner.
Note: You'll need to add import shutil at the top of the file for this suggestion to work.
_which_cache: dict[str, bool] = {}
def _which(cmd: str) -> bool:
"""Check whether *cmd* is on PATH (cached)."""
if cmd not in _which_cache:
_which_cache[cmd] = shutil.which(cmd) is not None
return _which_cache[cmd]| if not cmd: | ||
| if wait > 0: | ||
| time.sleep(wait) | ||
| return |
There was a problem hiding this comment.
This logic causes say_sync to wait for wait seconds even if no TTS command is available and nothing is spoken. The docstring says 'optionally wait after it finishes', which implies that if speech doesn't happen, the wait should also be skipped. This could lead to unexpected delays if say_sync is called with a wait value on a system without TTS. This behavior is also inconsistent with the previous implementation which would not wait if the say command was not found.
if not cmd:
return| speak() { | ||
| if command -v say &>/dev/null; then | ||
| say "$1" | ||
| elif command -v espeak &>/dev/null; then | ||
| espeak "$1" | ||
| elif command -v spd-say &>/dev/null; then | ||
| spd-say "$1" | ||
| fi | ||
| } |
There was a problem hiding this comment.
This implementation has two areas for improvement:
- It only passes the first argument (
$1) to the TTS command. Using"$@"would make it more robust by handling all arguments. - It checks for the existence of
say,espeak, andspd-sayon every call. It would be more efficient to perform this check only once when the script starts.
Here is a suggested refactoring that addresses both points by finding the appropriate TTS command once and defining a speak function that uses it.
| speak() { | |
| if command -v say &>/dev/null; then | |
| say "$1" | |
| elif command -v espeak &>/dev/null; then | |
| espeak "$1" | |
| elif command -v spd-say &>/dev/null; then | |
| spd-say "$1" | |
| fi | |
| } | |
| _tts_command="" | |
| if command -v say &>/dev/null; then | |
| _tts_command="say" | |
| elif command -v espeak &>/dev/null; then | |
| _tts_command="espeak" | |
| elif command -v spd-say &>/dev/null; then | |
| _tts_command="spd-say" | |
| fi | |
| speak() { | |
| if [ -n "$_tts_command" ]; then | |
| "$_tts_command" "$@" | |
| fi | |
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a531898256
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| subprocess.call( | ||
| ["which", cmd], | ||
| stdout=subprocess.DEVNULL, | ||
| stderr=subprocess.DEVNULL, |
There was a problem hiding this comment.
which on unsupported platforms
This helper now invokes which before say()/say_sync() reach their FileNotFoundError guard. On Windows, which itself is not present, so _tts_cmd() raises on the first voice cue and the app crashes instead of silently skipping speech as it did before this change. Using shutil.which() or catching lookup failures inside _which() would preserve the advertised no-TTS fallback.
Useful? React with 👍 / 👎.
| for cmd in ("espeak", "spd-say"): | ||
| if _which(cmd): | ||
| return [cmd, text] |
There was a problem hiding this comment.
-w to spd-say in the synchronous path
When spd-say is the only Linux backend, say_sync() no longer blocks until the prompt finishes. The spd-say(1) docs say -w, --wait will "Wait till the message is spoken or discarded" (https://manpages.debian.org/testing/speech-dispatcher/spd-say.1.en.html), but _tts_cmd() currently returns only ['spd-say', text]. In timed_hold(), that makes the 3-2-1 countdown start while "Get in position" is still being spoken, so timed sets begin early on those machines.
Useful? React with 👍 / 👎.
| elif command -v spd-say &>/dev/null; then | ||
| spd-say "$1" |
There was a problem hiding this comment.
plank.sh wait for spd-say before sleeping
The new spd-say branch changes this script's timing because spd-say returns as soon as it submits the utterance unless -w/--wait is passed; spd-say(1) documents that flag as "Wait till the message is spoken or discarded" (https://manpages.debian.org/testing/speech-dispatcher/spd-say.1.en.html). On Linux systems that have spd-say but not espeak, the following sleep 3, sleep 40, and sleep 60 start before the cue finishes, so spoken prompts overlap the countdown and the hold/rest intervals become shorter than advertised.
Useful? React with 👍 / 👎.
mikkel
left a comment
mikkel
left a comment
There was a problem hiding this comment.
@/ml2/nanobot/.pr-review-state/review-body.tmp
1 participant
Summary
coach.py: uses macOSsayon Darwin,espeakorspd-sayon Linux, silently skips if none availableplank.shwith aspeak()wrapper that picks the right TTS command per platformThe existing macOS
saypath is unchanged — Linux detection only activates on non-Darwin platforms.Test plan
saystill works as beforeespeakinstalled — verify voice cues playspd-sayinstalled — verify fallback works🤖 Generated with Claude Code