-
Notifications
You must be signed in to change notification settings - Fork 46
Fixed tts breaking on percentages #347
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
base: release
Are you sure you want to change the base?
Conversation
WalkthroughModified Changes
Sequence DiagramsequenceDiagram
autonumber
participant Old as Old Flow
participant New as New Flow
Old->>Old: Check if ELEVENLABS?
alt ELEVENLABS
Old->>Old: Return provider-specific string
Note over Old: Early exit
else Other Provider
Old->>Old: Return empty string
end
New->>New: Initialize general_instructions
New->>New: Add PERCENTAGE HANDLING guidance
New->>New: Initialize empty provider_specific_instructions
New->>New: Check if ELEVENLABS?
alt ELEVENLABS
New->>New: Assign provider-specific block
end
New->>New: Concatenate general + provider-specific
New->>New: Return combined instructions
Note over New: Unified return path
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/agents/voice/automatic/prompts/system/tts.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/agents/voice/automatic/prompts/system/tts.py (1)
app/agents/voice/automatic/types/models.py (1)
TTSProvider(8-10)
🔇 Additional comments (2)
app/agents/voice/automatic/prompts/system/tts.py (2)
9-17: LGTM! Clear percentage handling guidance.The general instructions provide comprehensive examples for handling percentages with decimals in TTS, directly addressing the PR objective. The guidance to replace decimal symbols with "point" is appropriate for TTS engines.
30-30: Behavioral change confirmed as intentional bug fix.The git history shows commit
3cdea02("Fixed tts breaking on percentages") deliberately refactored the function to extract general percentage handling instructions and return them for all TTS providers, while preserving ELEVENLABS-specific currency/number handling. This was done as a bug fix to address percentage pronunciation issues across all TTS engines—a general concern, not provider-specific.The change is correct and the code is ready.
| provider_specific_instructions = """ | ||
| CURRENCY & NUMBER HANDLING | ||
| Do not include any currency symbols (₹, $, etc.) in your spoken responses. | ||
| For any number with more than two digits, expand it using a **digit-word hybrid format** for natural speech. Say numbers using digits for major units and words for place values. | ||
| - Example: “322” → say “3 hundred 22 rupees” | ||
| - Example: “45,099” → say “45 thousand 99 rupees” | ||
| - Example: "322" → say "3 hundred 22 rupees" | ||
| - Example: "45,099" → say "45 thousand 99 rupees" | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Fix indentation in provider-specific instructions.
The multi-line string for provider_specific_instructions has inconsistent indentation that will include unnecessary leading whitespace in the output.
Apply this diff to fix the indentation:
- provider_specific_instructions = """
- CURRENCY & NUMBER HANDLING
- Do not include any currency symbols (₹, $, etc.) in your spoken responses.
-
- For any number with more than two digits, expand it using a **digit-word hybrid format** for natural speech. Say numbers using digits for major units and words for place values.
- - Example: "322" → say "3 hundred 22 rupees"
- - Example: "45,099" → say "45 thousand 99 rupees"
- """
+ provider_specific_instructions = """
+CURRENCY & NUMBER HANDLING
+Do not include any currency symbols (₹, $, etc.) in your spoken responses.
+
+For any number with more than two digits, expand it using a **digit-word hybrid format** for natural speech. Say numbers using digits for major units and words for place values.
+- Example: "322" → say "3 hundred 22 rupees"
+- Example: "45,099" → say "45 thousand 99 rupees"
+"""Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/agents/voice/automatic/prompts/system/tts.py around lines 21 to 28, the
multi-line string assigned to provider_specific_instructions contains
inconsistent leading indentation which will embed unwanted whitespace in the
string; reformat the triple-quoted string so its content lines are left-aligned
(no extra leading spaces), preserving line breaks and punctuation exactly as
intended, so the string contains clean text without leading indentation.
Summary by CodeRabbit