Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cleaned up tts tool, detect if tts server isnt running #404

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

ErikBjare
Copy link
Owner

@ErikBjare ErikBjare commented Jan 15, 2025

Important

Improved TTS tool by integrating content cleaning, detecting server availability, and adding tests for text processing.

  • Behavior:
    • Removed _clean_content_for_speech from gptme/chat.py and integrated its functionality into clean_for_speech in gptme/tools/tts.py.
    • speak function in gptme/tools/tts.py now checks TTS server availability and logs a warning if unavailable.
    • Default speaking speed increased to 1.3 in gptme/tools/tts.py.
  • TTS Server Detection:
    • TTS server availability is checked using a socket connection in gptme/tools/tts.py.
    • Logs TTS server status (enabled/disabled) based on availability.
  • Tests:
    • Added tests/test_tools_tts.py to test split_text and clean_for_speech functions.
    • Moved text splitting tests from gptme/tools/tts.py to tests/test_tools_tts.py.

This description was created by Ellipsis for fbf5369. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to fbf5369 in 1 minute and 44 seconds

More details
  • Looked at 374 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. gptme/chat.py:253
  • Draft comment:
    The removal of _clean_content_for_speech means that content is no longer cleaned before being spoken. Consider integrating the cleaning logic into the speak function call to avoid undesired speech output.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment raises a valid concern - we're no longer cleaning the content before speaking it. However, I cannot see the implementation of speak() - it's possible that speak() already handles cleaning the content. Without seeing the implementation of speak(), I cannot be certain whether this is actually an issue or not.
    I don't have visibility into the speak() function implementation. The comment could be wrong if speak() already handles content cleaning.
    While the concern seems reasonable, without seeing the speak() implementation, I cannot be confident enough that this is definitely an issue that needs to be fixed.
    Since I cannot verify whether this is actually an issue without seeing the speak() implementation, I should err on the side of removing the comment.
2. gptme/tools/tts.py:267
  • Draft comment:
    Consider explicitly setting the clean parameter to True in the speak function call for clarity, even though it defaults to True.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The speak function in gptme/tools/tts.py has a clean parameter that defaults to True, which means it will clean the text before speaking. This aligns with the intent of cleaning content before speaking, as was done previously in gptme/chat.py. However, the clean parameter should be explicitly set to True in the speak call to ensure clarity.

Workflow ID: wflow_T4D5pQSPkuoVLvqt


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@codecov-commenter
Copy link

codecov-commenter commented Jan 15, 2025

Codecov Report

Attention: Patch coverage is 34.48276% with 19 lines in your changes missing coverage. Please review.

Project coverage is 69.93%. Comparing base (91bdce6) to head (fbf5369).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
gptme/tools/tts.py 35.71% 18 Missing ⚠️
gptme/chat.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #404      +/-   ##
==========================================
+ Coverage   68.84%   69.93%   +1.09%     
==========================================
  Files          70       70              
  Lines        5771     5761      -10     
==========================================
+ Hits         3973     4029      +56     
+ Misses       1798     1732      -66     
Flag Coverage Δ
anthropic/claude-3-haiku-20240307 68.82% <34.48%> (+1.08%) ⬆️
openai/gpt-4o-mini 67.74% <34.48%> (+0.86%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ErikBjare ErikBjare merged commit d27da5e into master Jan 15, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants