Conversation
loadxf
commented
Mar 2, 2026
- Add TextToSpeechSynthesizer (ElevenLabs) with Discord/Telegram integration
- Fix Discord thread parent_id resolution for session scoping
- Update video media module and config for new media features
- Update tests for daemon, MCP client, orchestration, and coverage gaps
- Add TextToSpeechSynthesizer (ElevenLabs) with Discord/Telegram integration - Fix Discord thread parent_id resolution for session scoping - Update video media module and config for new media features - Update tests for daemon, MCP client, orchestration, and coverage gaps Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds optional ElevenLabs-based text-to-speech (TTS) support across CLI, Telegram, and Discord, while also fixing Discord thread session scoping and tightening tests/coverage to reduce unawaited-coroutine warnings.
Changes:
- Introduce
TextToSpeechSynthesizer(ElevenLabs) and integrate voice replies/playback in Discord, Telegram, and CLI. - Fix Discord thread
parent_idhandling so session scope keys resolve to the parent channel while still tracking the thread id. - Update tests and pytest configuration to address resource warnings / unawaited coroutine warnings and close leaked resources.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
gwenn/media/tts.py |
New ElevenLabs TTS synthesizer with lazy client init + graceful fallback behavior. |
gwenn/media/__init__.py |
Exports TextToSpeechSynthesizer from gwenn.media. |
gwenn/channels/telegram_channel.py |
Adds optional TTS voice replies (mode-dependent) and closes synthesizer on shutdown. |
gwenn/channels/discord_channel.py |
Adds optional TTS voice replies and fixes thread parent channel scoping for sessions. |
gwenn/main.py |
Adds CLI TTS playback and updates startup/daemon UI messaging text. |
gwenn/config.py |
Adds ElevenLabsConfig and wires it into the main config. |
gwenn/media/video.py |
Ensures cv2.VideoCapture.release() always happens via try/finally. |
pyproject.toml |
Adds elevenlabs extra; configures pytest asyncio strict mode and warning filters. |
tests/test_tts.py |
New unit tests for the TTS synthesizer (mocked ElevenLabs client). |
tests/test_daemon.py |
Ensures client disconnect happens to prevent StreamWriter resource warnings. |
tests/test_mcp_client.py |
Closes unawaited coroutine objects passed to mocked create_task. |
tests/test_orchestration_coverage.py |
Closes unawaited coroutine objects passed to mocked ensure_future. |
tests/test_remaining_coverage.py |
Adds an autouse fixture to close MemoryStore instances created during tests. |
tests/test_main_coverage.py |
Adjusts asyncio.run mocking to close coroutines (but reduces assertions). |
tests/test_final_coverage_gaps.py |
Avoids creating unawaited coroutine in semaphore acquisition simulation (via internal _value). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Acquire the semaphore first so release() works. | ||
| # _sync_slot is an asyncio.BoundedSemaphore — decrement its value | ||
| # directly to simulate acquisition without creating an unawaited coroutine. | ||
| executor._sync_slot._value -= 1 |
There was a problem hiding this comment.
This test now uses the private asyncio.BoundedSemaphore implementation detail _value to simulate an acquisition. That’s brittle across Python versions/asyncio implementations and can make the test fail unexpectedly if the semaphore internals change. Prefer acquiring via a real event loop (e.g., asyncio.run(executor._sync_slot.acquire())) or otherwise structuring the test so it doesn’t depend on _value internals.
| # Acquire the semaphore first so release() works. | |
| # _sync_slot is an asyncio.BoundedSemaphore — decrement its value | |
| # directly to simulate acquisition without creating an unawaited coroutine. | |
| executor._sync_slot._value -= 1 | |
| # Acquire the semaphore first so release() works, using the public API. | |
| # Use a real event loop instead of mutating the private _value attribute. | |
| asyncio.run(executor._sync_slot.acquire()) |
| def test_main_default_runs_session(self, monkeypatch): | ||
| from gwenn.main import main | ||
| monkeypatch.setattr("sys.argv", ["gwenn"]) | ||
| mock_run = MagicMock() | ||
| monkeypatch.setattr("asyncio.run", mock_run) | ||
|
|
||
| def _mock_run(coro): | ||
| coro.close() | ||
|
|
||
| monkeypatch.setattr("asyncio.run", _mock_run) | ||
| monkeypatch.setattr("gwenn.main._logging_configured", False) | ||
| monkeypatch.setattr("gwenn.main._termios", None) | ||
| main() |
There was a problem hiding this comment.
These entry-point tests no longer assert that asyncio.run(...) was invoked; they only close the coroutine to avoid warnings. That weakens the test because it would still pass if main() stopped running the session entirely. Consider keeping an assertion by wrapping asyncio.run with a stub that records call count/args (and still closes the passed coroutine).
| self._set_startup_step(daemon_state, "connect", "done") | ||
| daemon_state["ready_lines"] = [ | ||
| "Connected to Gwenn's daemon. Type your message, or '/exit' to close.", | ||
| "Connected to Gwenn's framework. Type your message, or '/exit' to close.", |
There was a problem hiding this comment.
The daemon-connection success message says "Connected to Gwenn's framework", but this code path is specifically connecting to the daemon socket (CliChannel.connect). This is likely to confuse users troubleshooting daemon connectivity; consider changing this line back to "Connected to Gwenn's daemon" (or similar) to match the actual behavior.
| "Connected to Gwenn's framework. Type your message, or '/exit' to close.", | |
| "Connected to Gwenn's daemon. Type your message, or '/exit' to close.", |
| class ElevenLabsConfig(BaseSettings): | ||
| """Configuration for ElevenLabs text-to-speech (optional).""" | ||
|
|
||
| api_key: Optional[str] = Field(None, alias="ELEVENLABS_API_KEY") | ||
| voice_id: str = Field("JBFqnCBsd6RMkjVDRZzb", alias="GWENN_TTS_VOICE_ID") | ||
| model: str = Field("eleven_turbo_v2_5", alias="GWENN_TTS_MODEL") | ||
| output_format: str = Field("opus_48000_128", alias="GWENN_TTS_OUTPUT_FORMAT") | ||
| max_chars: int = Field(2500, alias="GWENN_TTS_MAX_CHARS") | ||
| mode: str = Field("voice_reply", alias="GWENN_TTS_MODE") | ||
|
|
||
| model_config = {"env_file": _ENV_FILE, "extra": "ignore"} | ||
|
|
||
| @property | ||
| def is_available(self) -> bool: | ||
| return bool(self.api_key) | ||
|
|
||
| def should_send_voice(self, is_voice_message: bool = False) -> bool: | ||
| """Whether a voice reply should be sent for this interaction.""" | ||
| if not self.is_available or self.mode == "off": | ||
| return False | ||
| if self.mode == "always": | ||
| return True | ||
| return self.mode == "voice_reply" and is_voice_message | ||
|
|
||
| @model_validator(mode="after") | ||
| def validate_mode(self) -> "ElevenLabsConfig": | ||
| if self.mode not in {"off", "voice_reply", "always"}: | ||
| import structlog | ||
| structlog.get_logger(__name__).warning( | ||
| "config.elevenlabs_invalid_mode", | ||
| provided=self.mode, | ||
| fallback="voice_reply", | ||
| ) | ||
| self.mode = "voice_reply" | ||
| return self |
There was a problem hiding this comment.
New ElevenLabs / TTS environment variables are introduced here (e.g., ELEVENLABS_API_KEY, GWENN_TTS_*), but they aren’t documented in the existing configuration reference or sample env file. Please update docs/configuration.md and .env.example to include these new settings so users can discover and configure TTS correctly.
| [tool.pytest.ini_options] | ||
| asyncio_mode = "strict" | ||
| filterwarnings = [ | ||
| # PTB plans to change retry_after to timedelta in a future major version. | ||
| # Nothing to act on until that release; silence the noise. | ||
| "ignore::telegram.warnings.PTBDeprecationWarning", | ||
| # unittest.mock creates unawaited coroutines when AsyncMock attributes are | ||
| # accessed but never called — a known CPython mock limitation, not a bug in | ||
| # our code. Also covers coroutines passed to mocked ensure_future/create_task. | ||
| "ignore:coroutine 'AsyncMockMixin._execute_mock_call' was never awaited:RuntimeWarning", | ||
| "ignore:coroutine 'DockerManager.run_container.<locals>._cleanup_after_exit' was never awaited:RuntimeWarning", | ||
| # Rich animation internals may create unawaited sleep coroutines when a | ||
| # console status/spinner is interrupted by an exception. | ||
| "ignore:coroutine 'sleep' was never awaited:RuntimeWarning", | ||
| # structlog processor-chain ordering advisory — not actionable without | ||
| # removing pretty-exception support. | ||
| "ignore:Remove `format_exc_info` from your processor chain:UserWarning", | ||
| ] |
There was a problem hiding this comment.
The warning filter ignore:coroutine 'sleep' was never awaited:RuntimeWarning is very broad and will suppress any unawaited sleep() coroutine warnings across the entire test suite (including ones introduced by this repo). Consider narrowing this filter to the specific module(s) that emit it (e.g., Rich internals) so genuine unawaited coroutine regressions aren’t hidden.