Repository navigation
(feat):add dragon tts admin controls and better errors - #979
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughDragonTTS now supports cache search and audio management through bounded APIs and admin proxy routes. ElevenLabs adds language-aware synthesis. Streaming and HTTP synthesis errors now return structured provider details. ChangesDragonTTS provider and error handling
Cache management
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AdminClient
participant BreezeBuddyRouter
participant DragonTTSCacheAPI
participant CacheService
participant TTSProvider
AdminClient->>BreezeBuddyRouter: Submit cache operation
BreezeBuddyRouter->>DragonTTSCacheAPI: Forward authenticated request
DragonTTSCacheAPI->>CacheService: Resolve or update cache entry
CacheService->>TTSProvider: Re-synthesize when required
TTSProvider-->>CacheService: Return audio
CacheService-->>DragonTTSCacheAPI: Return cache metadata
DragonTTSCacheAPI-->>BreezeBuddyRouter: Return status and body
BreezeBuddyRouter-->>AdminClient: Return proxied response
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
dragontts/app/providers/elevenlabs.py (1)
96-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
Optional[str]for the new nullable parameters.The coding guideline requires
Optional[T]for nullable type annotations.
dragontts/app/providers/elevenlabs.py#L96-L102: replacelanguage: str | Nonewithlanguage: Optional[str].dragontts/app/providers/elevenlabs_pool.py#L207-L209: replacelanguage: str | Nonewithlanguage: Optional[str].As per coding guidelines: “Use Optional[T], List[T], Dict[str, Any], Union for type annotations.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dragontts/app/providers/elevenlabs.py` around lines 96 - 102, Update the nullable language annotations to use Optional[str] instead of str | None in _get_pool within dragontts/app/providers/elevenlabs.py (lines 96-102) and the corresponding declaration in dragontts/app/providers/elevenlabs_pool.py (lines 207-209), ensuring Optional is imported where required.Source: Coding guidelines
app/api/routers/breeze_buddy/dragontts.py (1)
22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLoad the DragonTTS URL from static configuration.
The new proxy imports
DRAGONTTS_URLfrom dynamic configuration. The Python guideline requires configuration to come fromapp/core/config/static.pythroughget_required_env()for mandatory values.Move the mandatory service URL to static configuration, or update the guideline if runtime mutability is required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/routers/breeze_buddy/dragontts.py` at line 22, Update the DragonTTS URL configuration used by the proxy importing DRAGONTTS_URL so the mandatory value is defined in app/core/config/static.py and loaded through get_required_env(); remove its dependency on app.core.config.dynamic while preserving the existing configuration key and service behavior.Source: Coding guidelines
dragontts/app/schemas/cache.py (1)
77-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the required type annotations to new interfaces.
The changed code adds incomplete collection types and omits required function return annotations.
dragontts/app/schemas/cache.py#L77-L90: useDict[str, Any]forparamsandList[CacheEntryDetail]forentries.dragontts/app/api/v1/cache.py#L35-L67: annotate_parse.dragontts/app/api/v1/cache.py#L87-L109: annotate the cache-record input type.dragontts/app/api/v1/cache.py#L325-L434: annotate endpoint return values.dragontts/app/cache/service.py#L1168-L1214: annotateresynth_by_keyandreplace_audio_by_keyreturn values.app/api/routers/breeze_buddy/dragontts.py#L69-L157: annotate_forwardand endpoint return values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dragontts/app/schemas/cache.py` around lines 77 - 90, Complete the type annotations across all affected interfaces: in dragontts/app/schemas/cache.py lines 77-90, use Dict[str, Any] for CacheEntryDetail.params and List[CacheEntryDetail] for ByTextResponse.entries; annotate _parse and the cache-record input in dragontts/app/api/v1/cache.py lines 35-67 and 87-109, and add return annotations to its endpoints at lines 325-434; annotate resynth_by_key and replace_audio_by_key returns in dragontts/app/cache/service.py lines 1168-1214; and annotate _forward plus endpoint return values in app/api/routers/breeze_buddy/dragontts.py lines 69-157, using the existing domain types and response models.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dragontts/app/api/v1/cache.py`:
- Around line 329-349: Validate the match parameter in the cache lookup endpoint
before calling metadata.list, accepting only "exact" and "substring". Reject any
other value with the endpoint’s standard validation error response, while
preserving the existing matching behavior and limit handling for valid values.
In `@dragontts/app/cache/service.py`:
- Around line 71-86: Update the WAV validation in replace_audio_by_key to reject
channel counts greater than two by extending the existing nchannels condition.
Preserve mono and stereo handling, and ensure unsupported multichannel files
raise the same ValueError before audioop.tomono is called.
- Around line 60-62: Update the shared Python runtime constraint and CI image
configuration to support only Python versions >=3.11 and <3.13, preventing
deployment with the removed audioop dependency used by service.py. Preserve the
existing audioop-based WAV decoding behavior.
- Around line 1200-1207: Update the uploaded-audio conversion in the async
request path to call the existing _convert_audio helper instead of synchronous
convert_audio, preserving the current arguments and result assignment. Leave
_decode_upload_to_pcm and the surrounding processing unchanged.
In `@dragontts/app/providers/elevenlabs.py`:
- Line 116: Update the _pools declaration annotation to match the four-member
key constructed in the provider logic: voice_id, model_id, enable_ssml_parsing,
and language. Use the appropriate tuple element types so the annotation
accurately accepts the key assigned at the key construction site and passes
Pyrefly.
---
Nitpick comments:
In `@app/api/routers/breeze_buddy/dragontts.py`:
- Line 22: Update the DragonTTS URL configuration used by the proxy importing
DRAGONTTS_URL so the mandatory value is defined in app/core/config/static.py and
loaded through get_required_env(); remove its dependency on
app.core.config.dynamic while preserving the existing configuration key and
service behavior.
In `@dragontts/app/providers/elevenlabs.py`:
- Around line 96-102: Update the nullable language annotations to use
Optional[str] instead of str | None in _get_pool within
dragontts/app/providers/elevenlabs.py (lines 96-102) and the corresponding
declaration in dragontts/app/providers/elevenlabs_pool.py (lines 207-209),
ensuring Optional is imported where required.
In `@dragontts/app/schemas/cache.py`:
- Around line 77-90: Complete the type annotations across all affected
interfaces: in dragontts/app/schemas/cache.py lines 77-90, use Dict[str, Any]
for CacheEntryDetail.params and List[CacheEntryDetail] for
ByTextResponse.entries; annotate _parse and the cache-record input in
dragontts/app/api/v1/cache.py lines 35-67 and 87-109, and add return annotations
to its endpoints at lines 325-434; annotate resynth_by_key and
replace_audio_by_key returns in dragontts/app/cache/service.py lines 1168-1214;
and annotate _forward plus endpoint return values in
app/api/routers/breeze_buddy/dragontts.py lines 69-157, using the existing
domain types and response models.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8fc89946-8c83-4849-835b-27cddbbd4bdf
📒 Files selected for processing (10)
app/ai/voice/tts/dragontts.pyapp/api/routers/breeze_buddy/dragontts.pydragontts/app/api/v1/cache.pydragontts/app/api/v1/tts.pydragontts/app/cache/service.pydragontts/app/core/config.pydragontts/app/providers/elevenlabs.pydragontts/app/providers/elevenlabs_pool.pydragontts/app/schemas/cache.pydragontts/app/storage/sqlite.py
There was a problem hiding this comment.
Pull request overview
This PR extends the DragonTTS service and Breeze Buddy admin API to support an admin-only cache browser/management workflow (by-text lookup, resynth, and audio replacement), while improving upstream error surfacing and tightening some operational safeguards (analytics range bounds, cache lookup indexing, and ElevenLabs language handling).
Changes:
- Added DragonTTS cache browse/manage endpoints (
/cache/by-text,/cache/{key}/resynth,/cache/{key}/audio) plus corresponding Breeze Buddy admin proxy routes. - Improved error visibility by surfacing provider/library exceptions as 502s and preserving streaming error bodies.
- Enhanced performance/ops behavior via a new SQLite index on
cache_entries.text, bounded analytics date ranges, and ElevenLabslanguage_codesupport for multilingual models.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| dragontts/app/storage/sqlite.py | Adds an index on cache_entries(text) to speed exact by-text cache lookups. |
| dragontts/app/schemas/cache.py | Introduces detailed cache-entry response models for by-text browsing (optionally including inline audio). |
| dragontts/app/providers/elevenlabs.py | Adds multilingual model language handling, ensures speed canonicalization, and extends pool keying to include SSML/language. |
| dragontts/app/providers/elevenlabs_pool.py | Supports language_code as a connect-time WS query param (pool-keyed). |
| dragontts/app/core/config.py | Adjusts provider default speeds and adds an analytics_max_range_days guardrail. |
| dragontts/app/cache/service.py | Adds WAV upload decoding + admin operations to resynth or replace audio by cache key. |
| dragontts/app/api/v1/tts.py | Improves upstream error reporting by mapping unexpected exceptions to 502 with details. |
| dragontts/app/api/v1/cache.py | Adds bounded analytics ranges and new cache browse/manage endpoints (by-text, resynth, replace-audio). |
| app/api/routers/breeze_buddy/dragontts.py | Adds admin-only proxy routes for the DragonTTS cache browser/management API. |
bf59326 to
d79c6be
Compare
d79c6be to
a773344
Compare
|
Thanks for the review — all items addressed in the latest push. Quick rundown:
black / isort / autoflake + behavior checks pass on both copies. |
| "POST", f"{self._url}/tts/stream", json=body | ||
| ) as response: | ||
| response.raise_for_status() | ||
| if response.status_code >= 400: |
Summary by CodeRabbit