voice agents refactoring #207
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughWalkthroughRefactors TTS voice configuration from a single string ( Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client UI
participant API as Voice Agent Controller
participant Service as Voice Agent Service
participant DB as Database
participant Pipecat as Pipecat Service
participant LLM as LLM Service
UI->>UI: User selects supported_languages and enters per-language voice IDs
UI->>API: POST/PUT agent payload with `tts_voice_ids` dict
API->>Service: create_agent/update_agent with `tts_voice_ids`
Service->>Service: Validate `tts_voice_ids` covers supported_languages
Service->>DB: Persist `tts_voice_ids` (JSONB)
DB-->>Service: Persisted
Service-->>API: Success
API-->>UI: Agent created/updated
Note over Pipecat,DB: Runtime conversation flow
Pipecat->>DB: Fetch agent config (includes `tts_voice_ids`)
Pipecat->>Pipecat: Detect language -> select voice_id = tts_voice_ids[lang] or default
Pipecat->>LLM: Build prompt (prefixed with customer_number)
Pipecat->>Pipecat: Initialize per-language STT/TTS services using selected voice_id
Pipecat-->>UI: Stream conversation audio/text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! 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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision: str = 'b92161a34bfc' |
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision: str = 'b92161a34bfc' | ||
| down_revision: Union[str, None] = 'a5b3c4d5e6f7' |
| # revision identifiers, used by Alembic. | ||
| revision: str = 'b92161a34bfc' | ||
| down_revision: Union[str, None] = 'a5b3c4d5e6f7' | ||
| branch_labels: Union[str, Sequence[str], None] = None |
| revision: str = 'b92161a34bfc' | ||
| down_revision: Union[str, None] = 'a5b3c4d5e6f7' | ||
| branch_labels: Union[str, Sequence[str], None] = None | ||
| depends_on: Union[str, Sequence[str], None] = None |
| ) | ||
|
|
||
| @validator('tts_voice_ids') | ||
| def validate_tts_voice_ids_keys(cls, v, values): |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In
`@wavefront/client/src/pages/apps/`[appId]/voice-agents/CreateVoiceAgentDialog.tsx:
- Around line 59-61: The tts_voice_ids Zod record currently only checks for at
least one key but allows empty string values; update the schema for
tts_voice_ids so the record's value schema requires non-empty strings (e.g.,
replace the current z.string() value type with a non-empty string validator such
as z.string().min(1) or z.string().nonempty()) and adjust the validation message
accordingly so entries like { en: '' } are rejected; locate the tts_voice_ids
declaration in the schema and modify the z.record(valueSchema) portion to
enforce non-empty voice IDs.
- Around line 139-151: The effect that syncs voiceIdState when
watchedSupportedLanguages or isOpen changes (the useEffect block updating
setVoiceIdState) must also update the form state so removed languages are
cleared; after computing the newState inside that effect, call
form.setValue('tts_voice_ids', newState) to mirror voiceIdState in the form (use
the same key 'tts_voice_ids' as used elsewhere) so the form no longer retains
voice IDs for languages that were removed.
In
`@wavefront/client/src/pages/apps/`[appId]/voice-agents/EditVoiceAgentDialog.tsx:
- Around line 167-179: The useEffect that syncs voiceIdState (useEffect with
dependencies watchedSupportedLanguages and isOpen) only updates the local state
via setVoiceIdState and does not update the form's tts_voice_ids field, leaving
stale entries for removed languages; update the effect to also call
form.setValue('tts_voice_ids', newState) (or setValue with the same key used in
the form) after computing newState so the form stays in sync with
setVoiceIdState and removed languages are cleared before submit.
In
`@wavefront/server/apps/call_processing/call_processing/controllers/webhook_controller.py`:
- Around line 232-233: customer_number is pulled with
body_data.get('customer_number') and can be None but is passed into
run_conversation (and used in the system prompt), causing "Customer phone
number: None"; update webhook_controller to validate/normalize customer_number
after extraction: if missing or not a string return a 400/422 response (or set a
clear fallback like empty string and log a warning), trim/format the number, and
only call run_conversation when customer_number is a valid string; reference the
extracted variable customer_number and the call to run_conversation in
webhook_controller to locate where to add the check and error response/logging.
- Around line 188-189: The twiml_endpoint is incorrectly swapping Twilio
parameters: change the stream.parameter assignments so customer_number uses From
and agent_number uses To (mirror the correct mapping used in inbound_webhook);
locate the stream.parameter calls inside the twiml_endpoint function and swap
the values for customer_number and agent_number (replace customer_number=To with
customer_number=From and agent_number=From with agent_number=To).
In
`@wavefront/server/apps/call_processing/call_processing/services/llm_service.py`:
- Around line 54-58: The code directly accesses llm_config['base_url'] in the
azure_openai branch which can raise KeyError; update the branch in
LLMServiceFactory (the elif handling llm_type == 'azure_openai' in
llm_service.py) to use llm_config.get('base_url') with a sensible default or
validate and raise a clear error if missing, then pass that value into
_create_azure_llm(api_key, model, parameters, base_url) so missing configs are
handled consistently with other config keys.
In
`@wavefront/server/apps/call_processing/call_processing/services/pipecat_service.py`:
- Around line 183-184: The code currently falls back to the literal string
'default' when default_language is absent from tts_voice_ids_dict, which may be
invalid for the TTS provider; change the logic in the block that sets
default_voice_id (referencing default_voice_id, tts_voice_ids_dict,
default_language) to not use the string 'default' — either retrieve a
provider-specific default (e.g., call a get_provider_default_voice() helper) or
set default_voice_id = None and raise a clear ValueError/KeyError indicating the
missing mapping so callers can handle it; also ensure any downstream use of
default_voice_id validates it and logs a descriptive error if unresolved.
In
`@wavefront/server/modules/db_repo_module/db_repo_module/alembic/versions/2026_01_24_1447-b92161a34bfc_refactor_tts_voice_id_to_jsonb.py`:
- Around line 50-77: The migration currently uses old_voice_id directly so when
old_voice_id is None or an empty string it writes invalid/empty values into
voice_ids_dict; update the loop so that before building voice_ids_dict you
normalize old_voice_id (e.g., if not old_voice_id or old_voice_id == "" set
normalized_voice_id = DEFAULT_TTS_VOICE_ID or another non-empty fallback) and
then build voice_ids_dict = {lang: normalized_voice_id for lang in
supported_langs}; ensure you reference old_voice_id, voice_ids_dict and the
tts_voice_ids update so the DB gets non-empty values for each language.
🧹 Nitpick comments (1)
wavefront/server/apps/call_processing/call_processing/services/llm_service.py (1)
150-153: Fix implicitOptionaltype hint.Per PEP 484,
base_url: str = Noneshould explicitly declare theNonetype. This was flagged by Ruff (RUF013).♻️ Suggested fix
`@staticmethod` def _create_azure_llm( - api_key: str, model: str, parameters: Dict[str, Any], base_url: str = None + api_key: str, model: str, parameters: Dict[str, Any], base_url: str | None = None ):
Comment on lines
+59
to
+61
| tts_voice_ids: z.record(z.string(), z.string()).refine((val) => Object.keys(val).length > 0, { | ||
| message: 'At least one voice ID is required', | ||
| }), |
There was a problem hiding this comment.
Zod schema doesn't validate that voice IDs are non-empty strings.
The current schema only checks that at least one key exists in the record, but allows empty string values like { en: '' }. This could lead to agents being created with empty voice IDs, which would fail at runtime.
Proposed fix to validate non-empty voice IDs
- tts_voice_ids: z.record(z.string(), z.string()).refine((val) => Object.keys(val).length > 0, {
- message: 'At least one voice ID is required',
- }),
+ tts_voice_ids: z.record(z.string(), z.string().min(1, 'Voice ID cannot be empty')).refine(
+ (val) => Object.keys(val).length > 0 && Object.values(val).every((v) => v.trim().length > 0),
+ {
+ message: 'At least one non-empty voice ID is required',
+ }
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| tts_voice_ids: z.record(z.string(), z.string()).refine((val) => Object.keys(val).length > 0, { | |
| message: 'At least one voice ID is required', | |
| }), | |
| tts_voice_ids: z.record(z.string(), z.string().min(1, 'Voice ID cannot be empty')).refine( | |
| (val) => Object.keys(val).length > 0 && Object.values(val).every((v) => v.trim().length > 0), | |
| { | |
| message: 'At least one non-empty voice ID is required', | |
| } | |
| ), |
🤖 Prompt for AI Agents
In
`@wavefront/client/src/pages/apps/`[appId]/voice-agents/CreateVoiceAgentDialog.tsx
around lines 59 - 61, The tts_voice_ids Zod record currently only checks for at
least one key but allows empty string values; update the schema for
tts_voice_ids so the record's value schema requires non-empty strings (e.g.,
replace the current z.string() value type with a non-empty string validator such
as z.string().min(1) or z.string().nonempty()) and adjust the validation message
accordingly so entries like { en: '' } are rejected; locate the tts_voice_ids
declaration in the schema and modify the z.record(valueSchema) portion to
enforce non-empty voice IDs.
Comment on lines
+139
to
+151
| // Sync voice ID state with language changes | ||
| useEffect(() => { | ||
| if (isOpen && watchedSupportedLanguages) { | ||
| setVoiceIdState((prev) => { | ||
| const newState: Record<string, string> = {}; | ||
| // Preserve existing voice IDs for languages still selected | ||
| watchedSupportedLanguages.forEach((lang) => { | ||
| newState[lang] = prev[lang] || ''; | ||
| }); | ||
| return newState; | ||
| }); | ||
| } | ||
| }, [watchedSupportedLanguages, isOpen]); |
There was a problem hiding this comment.
Form field not updated when languages are removed.
The useEffect updates voiceIdState when supported languages change, but it doesn't call form.setValue('tts_voice_ids', newState) to sync the form. This means removed languages' voice IDs remain in the form state even though they're not visible in the UI, potentially causing validation issues with the server-side validator that rejects extra languages.
Proposed fix to sync form state with voiceIdState
useEffect(() => {
if (isOpen && watchedSupportedLanguages) {
setVoiceIdState((prev) => {
const newState: Record<string, string> = {};
// Preserve existing voice IDs for languages still selected
watchedSupportedLanguages.forEach((lang) => {
newState[lang] = prev[lang] || '';
});
+ // Sync form state to remove voice IDs for deselected languages
+ form.setValue('tts_voice_ids', newState);
return newState;
});
}
- }, [watchedSupportedLanguages, isOpen]);
+ }, [watchedSupportedLanguages, isOpen, form]);🤖 Prompt for AI Agents
In
`@wavefront/client/src/pages/apps/`[appId]/voice-agents/CreateVoiceAgentDialog.tsx
around lines 139 - 151, The effect that syncs voiceIdState when
watchedSupportedLanguages or isOpen changes (the useEffect block updating
setVoiceIdState) must also update the form state so removed languages are
cleared; after computing the newState inside that effect, call
form.setValue('tts_voice_ids', newState) to mirror voiceIdState in the form (use
the same key 'tts_voice_ids' as used elsewhere) so the form no longer retains
voice IDs for languages that were removed.
Comment on lines
+167
to
+179
| // Sync voice ID state with language changes | ||
| useEffect(() => { | ||
| if (isOpen && watchedSupportedLanguages) { | ||
| setVoiceIdState((prev) => { | ||
| const newState: Record<string, string> = {}; | ||
| // Preserve existing voice IDs for languages still selected | ||
| watchedSupportedLanguages.forEach((lang) => { | ||
| newState[lang] = prev[lang] || ''; | ||
| }); | ||
| return newState; | ||
| }); | ||
| } | ||
| }, [watchedSupportedLanguages, isOpen]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "EditVoiceAgentDialog.tsx" -type fRepository: rootflo/wavefront
Length of output: 140
🏁 Script executed:
cat -n ./wavefront/client/src/pages/apps/[appId]/voice-agents/EditVoiceAgentDialog.tsxRepository: rootflo/wavefront
Length of output: 50373
🏁 Script executed:
sed -n '167,300p' ./wavefront/client/src/pages/apps/[appId]/voice-agents/EditVoiceAgentDialog.tsxRepository: rootflo/wavefront
Length of output: 5033
🏁 Script executed:
sed -n '775,806p' ./wavefront/client/src/pages/apps/[appId]/voice-agents/EditVoiceAgentDialog.tsxRepository: rootflo/wavefront
Length of output: 1739
🏁 Script executed:
sed -n '286,288p' ./wavefront/client/src/pages/apps/[appId]/voice-agents/EditVoiceAgentDialog.tsxRepository: rootflo/wavefront
Length of output: 212
🏁 Script executed:
sed -n '54,54p' ./wavefront/client/src/pages/apps/[appId]/voice-agents/EditVoiceAgentDialog.tsxRepository: rootflo/wavefront
Length of output: 122
🏁 Script executed:
sed -n '95,180p' ./wavefront/client/src/pages/apps/[appId]/voice-agents/EditVoiceAgentDialog.tsxRepository: rootflo/wavefront
Length of output: 3672
🏁 Script executed:
rg "tts_voice_ids" ./wavefront/client/src/pages/apps/[appId]/voice-agents/EditVoiceAgentDialog.tsxRepository: rootflo/wavefront
Length of output: 562
Sync form field when supported languages change.
The effect updates voiceIdState but not the form field. When a language is removed without manually editing voice IDs, the form submission includes stale tts_voice_ids with removed languages, causing server validation to fail. Call form.setValue() to keep them in sync.
🛠️ Suggested fix
useEffect(() => {
if (isOpen && watchedSupportedLanguages) {
setVoiceIdState((prev) => {
const newState: Record<string, string> = {};
// Preserve existing voice IDs for languages still selected
watchedSupportedLanguages.forEach((lang) => {
newState[lang] = prev[lang] || '';
});
+ form.setValue('tts_voice_ids', newState, { shouldDirty: true });
return newState;
});
}
- }, [watchedSupportedLanguages, isOpen]);
+ }, [watchedSupportedLanguages, isOpen, form]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Sync voice ID state with language changes | |
| useEffect(() => { | |
| if (isOpen && watchedSupportedLanguages) { | |
| setVoiceIdState((prev) => { | |
| const newState: Record<string, string> = {}; | |
| // Preserve existing voice IDs for languages still selected | |
| watchedSupportedLanguages.forEach((lang) => { | |
| newState[lang] = prev[lang] || ''; | |
| }); | |
| return newState; | |
| }); | |
| } | |
| }, [watchedSupportedLanguages, isOpen]); | |
| // Sync voice ID state with language changes | |
| useEffect(() => { | |
| if (isOpen && watchedSupportedLanguages) { | |
| setVoiceIdState((prev) => { | |
| const newState: Record<string, string> = {}; | |
| // Preserve existing voice IDs for languages still selected | |
| watchedSupportedLanguages.forEach((lang) => { | |
| newState[lang] = prev[lang] || ''; | |
| }); | |
| form.setValue('tts_voice_ids', newState, { shouldDirty: true }); | |
| return newState; | |
| }); | |
| } | |
| }, [watchedSupportedLanguages, isOpen, form]); |
🤖 Prompt for AI Agents
In
`@wavefront/client/src/pages/apps/`[appId]/voice-agents/EditVoiceAgentDialog.tsx
around lines 167 - 179, The useEffect that syncs voiceIdState (useEffect with
dependencies watchedSupportedLanguages and isOpen) only updates the local state
via setVoiceIdState and does not update the form's tts_voice_ids field, leaving
stale entries for removed languages; update the effect to also call
form.setValue('tts_voice_ids', newState) (or setValue with the same key used in
the form) after computing newState so the form stays in sync with
setVoiceIdState and removed languages are cleared before submit.
Comment on lines
+188
to
+189
| stream.parameter(name='customer_number', value=To) | ||
| stream.parameter(name='agent_number', value=From) |
There was a problem hiding this comment.
Critical: From/To mapping is swapped compared to inbound_webhook.
In the twiml_endpoint, the customer/agent number assignment is reversed:
- Line 188:
customer_number=To(should beFrom) - Line 189:
agent_number=From(should beTo)
In Twilio webhooks, From is the caller (customer) and To is the number being called (agent). Compare with inbound_webhook at lines 122-123 which correctly uses customer_number=From and agent_number=To.
🐛 Fix the swapped parameters
# Pass parameters to WebSocket stream
stream.parameter(name='voice_agent_id', value=voice_agent_id)
- stream.parameter(name='customer_number', value=To)
- stream.parameter(name='agent_number', value=From)
+ stream.parameter(name='customer_number', value=From)
+ stream.parameter(name='agent_number', value=To)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| stream.parameter(name='customer_number', value=To) | |
| stream.parameter(name='agent_number', value=From) | |
| stream.parameter(name='customer_number', value=From) | |
| stream.parameter(name='agent_number', value=To) |
🤖 Prompt for AI Agents
In
`@wavefront/server/apps/call_processing/call_processing/controllers/webhook_controller.py`
around lines 188 - 189, The twiml_endpoint is incorrectly swapping Twilio
parameters: change the stream.parameter assignments so customer_number uses From
and agent_number uses To (mirror the correct mapping used in inbound_webhook);
locate the stream.parameter calls inside the twiml_endpoint function and swap
the values for customer_number and agent_number (replace customer_number=To with
customer_number=From and agent_number=From with agent_number=To).
Comment on lines
+232
to
+233
| customer_number = body_data.get('customer_number') | ||
| # agent_number = body_data.get('agent_number') |
There was a problem hiding this comment.
Missing validation for customer_number.
customer_number is extracted with .get() (line 232) which can return None, but it's passed directly to run_conversation (line 293) which expects str. If missing, this will result in "Customer phone number: None" in the system prompt. Consider adding validation or a fallback.
🛠️ Suggested fix
voice_agent_id = body_data.get('voice_agent_id')
customer_number = body_data.get('customer_number')
# agent_number = body_data.get('agent_number')
if not voice_agent_id:
logger.error('voice_agent_id not found in stream parameters')
await websocket.close(code=1008, reason='Missing voice_agent_id')
return
+ if not customer_number:
+ logger.warning('customer_number not found in stream parameters, using empty string')
+ customer_number = ''
+
logger.info(f'Voice agent ID: {voice_agent_id}')Also applies to: 293-293
🤖 Prompt for AI Agents
In
`@wavefront/server/apps/call_processing/call_processing/controllers/webhook_controller.py`
around lines 232 - 233, customer_number is pulled with
body_data.get('customer_number') and can be None but is passed into
run_conversation (and used in the system prompt), causing "Customer phone
number: None"; update webhook_controller to validate/normalize customer_number
after extraction: if missing or not a string return a 400/422 response (or set a
clear fallback like empty string and log a warning), trim/format the number, and
only call run_conversation when customer_number is a valid string; reference the
extracted variable customer_number and the call to run_conversation in
webhook_controller to locate where to add the check and error response/logging.
Comment on lines
+54
to
+58
| elif llm_type == 'azure_openai': | ||
| base_url = llm_config['base_url'] | ||
| return LLMServiceFactory._create_azure_llm( | ||
| api_key, model, parameters, base_url | ||
| ) |
There was a problem hiding this comment.
Missing key access will raise KeyError if base_url is not provided.
Line 55 uses direct dictionary access llm_config['base_url'] which will raise a KeyError if the key is missing. Other config values use .get() with defaults. Consider validating or providing a fallback.
🛠️ Suggested fix
elif llm_type == 'azure_openai':
- base_url = llm_config['base_url']
+ base_url = llm_config.get('base_url')
+ if not base_url:
+ raise ValueError('Azure OpenAI requires base_url (endpoint) to be configured')
return LLMServiceFactory._create_azure_llm(
api_key, model, parameters, base_url
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| elif llm_type == 'azure_openai': | |
| base_url = llm_config['base_url'] | |
| return LLMServiceFactory._create_azure_llm( | |
| api_key, model, parameters, base_url | |
| ) | |
| elif llm_type == 'azure_openai': | |
| base_url = llm_config.get('base_url') | |
| if not base_url: | |
| raise ValueError('Azure OpenAI requires base_url (endpoint) to be configured') | |
| return LLMServiceFactory._create_azure_llm( | |
| api_key, model, parameters, base_url | |
| ) |
🤖 Prompt for AI Agents
In
`@wavefront/server/apps/call_processing/call_processing/services/llm_service.py`
around lines 54 - 58, The code directly accesses llm_config['base_url'] in the
azure_openai branch which can raise KeyError; update the branch in
LLMServiceFactory (the elif handling llm_type == 'azure_openai' in
llm_service.py) to use llm_config.get('base_url') with a sensible default or
validate and raise a clear error if missing, then pass that value into
_create_azure_llm(api_key, model, parameters, base_url) so missing configs are
handled consistently with other config keys.
Comment on lines
+183
to
+184
| # Get voice ID for default language | ||
| default_voice_id = tts_voice_ids_dict.get(default_language, 'default') |
There was a problem hiding this comment.
Fallback voice ID 'default' may not be valid.
If the default_language is missing from tts_voice_ids_dict, the fallback is the literal string 'default'. This may cause TTS provider errors since 'default' is unlikely to be a valid voice ID. Consider failing explicitly or using a provider-specific default.
🛠️ Suggested fix
# Get voice ID for default language
- default_voice_id = tts_voice_ids_dict.get(default_language, 'default')
+ default_voice_id = tts_voice_ids_dict.get(default_language)
+ if not default_voice_id:
+ logger.warning(
+ f'No voice ID configured for default language {default_language}'
+ )
+ # Use first available voice ID or raise error
+ if tts_voice_ids_dict:
+ default_voice_id = next(iter(tts_voice_ids_dict.values()))
+ else:
+ raise ValueError('No TTS voice IDs configured for agent')🤖 Prompt for AI Agents
In
`@wavefront/server/apps/call_processing/call_processing/services/pipecat_service.py`
around lines 183 - 184, The code currently falls back to the literal string
'default' when default_language is absent from tts_voice_ids_dict, which may be
invalid for the TTS provider; change the logic in the block that sets
default_voice_id (referencing default_voice_id, tts_voice_ids_dict,
default_language) to not use the string 'default' — either retrieve a
provider-specific default (e.g., call a get_provider_default_voice() helper) or
set default_voice_id = None and raise a clear ValueError/KeyError indicating the
missing mapping so callers can handle it; also ensure any downstream use of
default_voice_id validates it and logs a descriptive error if unresolved.
Comment on lines
+50
to
+77
| for agent in agents: | ||
| old_voice_id = agent.tts_voice_id | ||
|
|
||
| # Handle null/empty supported_languages - default to ["en"] | ||
| if agent.supported_languages: | ||
| if isinstance(agent.supported_languages, list): | ||
| supported_langs = agent.supported_languages | ||
| else: | ||
| # In case it's stored as JSON string | ||
| try: | ||
| supported_langs = json.loads(agent.supported_languages) | ||
| except (json.JSONDecodeError, TypeError): | ||
| supported_langs = ['en'] | ||
| else: | ||
| supported_langs = ['en'] | ||
|
|
||
| # Build dict: apply old voice_id to all supported languages | ||
| voice_ids_dict = {lang: old_voice_id for lang in supported_langs} | ||
|
|
||
| # Update the agent with new JSONB dict | ||
| connection.execute( | ||
| text(""" | ||
| UPDATE voice_agents | ||
| SET tts_voice_ids = :voice_dict | ||
| WHERE id = :agent_id | ||
| """), | ||
| {'voice_dict': json.dumps(voice_ids_dict), 'agent_id': str(agent.id)}, | ||
| ) |
There was a problem hiding this comment.
Handle null/empty tts_voice_id values during migration.
If old_voice_id is None or an empty string, the migration creates a dictionary with None/empty values for all languages (line 67). This could cause issues when the application tries to use these voice IDs, as the new validation expects non-empty values.
Proposed fix to handle null/empty voice IDs
for agent in agents:
old_voice_id = agent.tts_voice_id
+
+ # Handle null/empty voice_id - use 'default' as fallback
+ if not old_voice_id or not str(old_voice_id).strip():
+ old_voice_id = 'default'
# Handle null/empty supported_languages - default to ["en"]
if agent.supported_languages:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for agent in agents: | |
| old_voice_id = agent.tts_voice_id | |
| # Handle null/empty supported_languages - default to ["en"] | |
| if agent.supported_languages: | |
| if isinstance(agent.supported_languages, list): | |
| supported_langs = agent.supported_languages | |
| else: | |
| # In case it's stored as JSON string | |
| try: | |
| supported_langs = json.loads(agent.supported_languages) | |
| except (json.JSONDecodeError, TypeError): | |
| supported_langs = ['en'] | |
| else: | |
| supported_langs = ['en'] | |
| # Build dict: apply old voice_id to all supported languages | |
| voice_ids_dict = {lang: old_voice_id for lang in supported_langs} | |
| # Update the agent with new JSONB dict | |
| connection.execute( | |
| text(""" | |
| UPDATE voice_agents | |
| SET tts_voice_ids = :voice_dict | |
| WHERE id = :agent_id | |
| """), | |
| {'voice_dict': json.dumps(voice_ids_dict), 'agent_id': str(agent.id)}, | |
| ) | |
| for agent in agents: | |
| old_voice_id = agent.tts_voice_id | |
| # Handle null/empty voice_id - use 'default' as fallback | |
| if not old_voice_id or not str(old_voice_id).strip(): | |
| old_voice_id = 'default' | |
| # Handle null/empty supported_languages - default to ["en"] | |
| if agent.supported_languages: | |
| if isinstance(agent.supported_languages, list): | |
| supported_langs = agent.supported_languages | |
| else: | |
| # In case it's stored as JSON string | |
| try: | |
| supported_langs = json.loads(agent.supported_languages) | |
| except (json.JSONDecodeError, TypeError): | |
| supported_langs = ['en'] | |
| else: | |
| supported_langs = ['en'] | |
| # Build dict: apply old voice_id to all supported languages | |
| voice_ids_dict = {lang: old_voice_id for lang in supported_langs} | |
| # Update the agent with new JSONB dict | |
| connection.execute( | |
| text(""" | |
| UPDATE voice_agents | |
| SET tts_voice_ids = :voice_dict | |
| WHERE id = :agent_id | |
| """), | |
| {'voice_dict': json.dumps(voice_ids_dict), 'agent_id': str(agent.id)}, | |
| ) |
🤖 Prompt for AI Agents
In
`@wavefront/server/modules/db_repo_module/db_repo_module/alembic/versions/2026_01_24_1447-b92161a34bfc_refactor_tts_voice_id_to_jsonb.py`
around lines 50 - 77, The migration currently uses old_voice_id directly so when
old_voice_id is None or an empty string it writes invalid/empty values into
voice_ids_dict; update the loop so that before building voice_ids_dict you
normalize old_voice_id (e.g., if not old_voice_id or old_voice_id == "" set
normalized_voice_id = DEFAULT_TTS_VOICE_ID or another non-empty fallback) and
then build voice_ids_dict = {lang: normalized_voice_id for lang in
supported_langs}; ensure you reference old_voice_id, voice_ids_dict and the
tts_voice_ids update so the DB gets non-empty values for each language.
- also set stop_secs=0.2 in VADParams as we are using smart turn detection. - removed depereceated interruption strategy setting
Comment on lines
+78
to
+79
| # if 'endpointing' in parameters: # using pipecat VAD + smart turn detection | ||
| # options_dict['endpointing'] = parameters['endpointing'] |
Comment on lines
+88
to
+89
| # if 'vad_events' in parameters: # depreceated in pipecat 0.99+ | ||
| # options_dict['vad_events'] = parameters['vad_events'] |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wavefront/server/apps/call_processing/call_processing/services/pipecat_service.py (1)
292-298: Phone number in system prompt is PII—mask or gate it.Embedding the full customer number into the LLM system prompt risks PII exposure to external providers and logs. Consider masking (e.g., last 4 digits) or making this opt-in with explicit consent.
🛠️ Example masking approach
- 'content': f'Customer phone number: {customer_number}\n' - + agent_config['system_prompt'], + 'content': f'Customer phone number (last4): ' + f'{customer_number[-4:] if customer_number else ""}\n' + + agent_config['system_prompt'],
🤖 Fix all issues with AI agents
In
`@wavefront/server/apps/call_processing/call_processing/services/tool_wrapper_service.py`:
- Line 26: Update the tool schema defaults so they match the wrapper service's
new default timeout: change the timeout Field defaults on ApiToolConfig and
PythonToolConfig in tool_schemas.py from 30 and 60 to 6 (or, if the wrapper
intentionally overrides schemas, add a clear docstring/comment in
tool_schemas.py explaining that ToolWrapperService uses a 6s runtime default and
that schemas only provide a suggested value). Ensure you modify the timeout
Field definitions in ApiToolConfig and PythonToolConfig (or add the explanatory
comment next to those classes) so schema expectations align with the wrapper's
timeout parameter (timeout: int = 6) used in the wrapper service.
🧹 Nitpick comments (1)
wavefront/server/apps/call_processing/call_processing/services/stt_service.py (1)
78-95: Add warning when Deepgramendpointingorvad_eventsare passed but ignored.The schema documents
endpointingas a configurable STT parameter, but the code intentionally disables bothendpointing(in favor of Pipecat VAD) andvad_events(deprecated in Pipecat 0.99+). If callers pass these viastt_parameters, they will be silently ignored, leading to unexpected behavior. Add logging to surface this mismatch:if 'endpointing' in parameters or 'vad_events' in parameters: logger.warning( "Deepgram endpointing/vad_events are ignored; using Pipecat VAD for detection" )
| auth_type: Optional[str] = None, | ||
| auth_credentials: Optional[Dict[str, str]] = None, | ||
| timeout: int = 30, | ||
| timeout: int = 6, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's examine the file structure and locate the tool wrapper service
find . -name "tool_wrapper_service.py" -type f
# Check if the file exists and examine it
if [ -f "wavefront/server/apps/call_processing/call_processing/services/tool_wrapper_service.py" ]; then
echo "=== File content (first 50 lines) ==="
head -50 "wavefront/server/apps/call_processing/call_processing/services/tool_wrapper_service.py"
fiRepository: rootflo/wavefront
Length of output: 1725
🏁 Script executed:
#!/bin/bash
# Search for timeout configurations in tool-related code
echo "=== Searching for timeout configurations in tool-related code ==="
rg -n 'timeout' --type=py -g '*tool*' -C 3
echo -e "\n=== Searching for tool config files ==="
fd -e json -e yaml -e yml | grep -i tool | head -20Repository: rootflo/wavefront
Length of output: 14520
🏁 Script executed:
#!/bin/bash
# Check the actual file around lines 26 and 200
if [ -f "wavefront/server/apps/call_processing/call_processing/services/tool_wrapper_service.py" ]; then
echo "=== Context around line 26 ==="
sed -n '15,35p' "wavefront/server/apps/call_processing/call_processing/services/tool_wrapper_service.py"
echo -e "\n=== Context around line 200 ==="
sed -n '190,210p' "wavefront/server/apps/call_processing/call_processing/services/tool_wrapper_service.py"
fiRepository: rootflo/wavefront
Length of output: 1524
🏁 Script executed:
#!/bin/bash
# Search for how tools are instantiated and configured
echo "=== Searching for tool instantiation patterns ==="
rg -n 'tool_wrapper|timeout.*=' --type=py | head -30
echo -e "\n=== Searching for config.get or config.timeout patterns ==="
rg -n "config\.get\(|config\.timeout" --type=pyRepository: rootflo/wavefront
Length of output: 10573
Update tool schema defaults to match wrapper service timeout change.
The 6-second timeout reduction is problematic because it creates a schema-implementation mismatch. The tool schema definitions in tool_schemas.py still specify:
ApiToolConfig:timeout: int = Field(default=30, ...)PythonToolConfig:timeout: int = Field(default=60, ...)
But the wrapper service now uses 6 seconds as default. This inconsistency means:
- Users creating tools via the schema expect 30s/60s timeouts
- Actual execution uses 6s, causing unexpected failures
Update wavefront/server/modules/voice_agents_module/voice_agents_module/models/tool_schemas.py to align with the 6-second default, or document why the wrapper intentionally overrides schema expectations.
Users can still override per-tool via config.get('timeout', 6), but the defaults must be consistent across layers.
🤖 Prompt for AI Agents
In
`@wavefront/server/apps/call_processing/call_processing/services/tool_wrapper_service.py`
at line 26, Update the tool schema defaults so they match the wrapper service's
new default timeout: change the timeout Field defaults on ApiToolConfig and
PythonToolConfig in tool_schemas.py from 30 and 60 to 6 (or, if the wrapper
intentionally overrides schemas, add a clear docstring/comment in
tool_schemas.py explaining that ToolWrapperService uses a 6s runtime default and
that schemas only provide a suggested value). Ensure you modify the timeout
Field definitions in ApiToolConfig and PythonToolConfig (or add the explanatory
comment next to those classes) so schema expectations align with the wrapper's
timeout parameter (timeout: int = 6) used in the wrapper service.
…otflo/flo-ai into fix/gcs-signed-urls
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wavefront/server/packages/flo_cloud/flo_cloud/gcp/gcs.py`:
- Around line 155-158: Remove the debug print statements that output credentials
in gcs.py: eliminate the prints for service_account_email and
self.credentials.token (the token value) inside the code path that accesses
these variables; if you need observability keep only non-sensitive,
purpose-driven logging (e.g., a single sanitized debug message that a credential
was loaded without printing the email or token) and use the module's logger
instead of print to avoid leaking secrets. Locate the occurrences referencing
service_account_email and self.credentials.token and remove or replace them with
a redacted/log-safe message in the surrounding method where credentials are
loaded/used.
🧹 Nitpick comments (2)
wavefront/server/packages/flo_cloud/flo_cloud/gcp/gcs.py (2)
25-25: Unused attributesigning_credentials.
self.signing_credentialsis initialized but never read or written anywhere else in this class. Consider removing it if not needed.🧹 Proposed fix
- self.signing_credentials = None
166-169: Simplify redundant conditionals.The ternary expressions are unnecessary since
service_account_emailandtokenare already initialized toNoneand remainNoneif the attributes don't exist.♻️ Proposed simplification
presigned_url = blob.generate_signed_url( version='v4', expiration=datetime.now(UTC) + timedelta(seconds=expiresIn), method=type, - service_account_email=service_account_email - if service_account_email - else None, - access_token=token if token else None, + service_account_email=service_account_email, + access_token=token, )
Comment on lines
+155
to
+158
| print(f'service_account_email: {service_account_email}') | ||
| if hasattr(self.credentials, 'token'): | ||
| token = self.credentials.token | ||
| print(f'token: {token}') |
There was a problem hiding this comment.
Remove debug print statements that leak sensitive credentials.
Printing the access token (line 158) is a security risk—tokens can end up in logs, CI outputs, or monitoring systems. The service account email (line 155) is less sensitive but still unnecessary to log in production.
🔒 Proposed fix
if hasattr(self.credentials, 'service_account_email'):
service_account_email = self.credentials.service_account_email
- print(f'service_account_email: {service_account_email}')
if hasattr(self.credentials, 'token'):
token = self.credentials.token
- print(f'token: {token}')🤖 Prompt for AI Agents
In `@wavefront/server/packages/flo_cloud/flo_cloud/gcp/gcs.py` around lines 155 -
158, Remove the debug print statements that output credentials in gcs.py:
eliminate the prints for service_account_email and self.credentials.token (the
token value) inside the code path that accesses these variables; if you need
observability keep only non-sensitive, purpose-driven logging (e.g., a single
sanitized debug message that a credential was loaded without printing the email
or token) and use the module's logger instead of print to avoid leaking secrets.
Locate the occurrences referencing service_account_email and
self.credentials.token and remove or replace them with a redacted/log-safe
message in the surrounding method where credentials are loaded/used.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores
Bug Fixes / Tweaks
✏️ Tip: You can customize this high-level summary in your review settings.