Added support for noise cancellation model for daily#715
Added support for noise cancellation model for daily#715kanakagrawal-crypto wants to merge 1 commit into
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request adds transport-type-aware AIC model path selection to the Breeze Buddy agent. A new function selects the 16kHz model for daily transport and defaults otherwise. The audio input filter factory now accepts transport type and creates separate filter instances per transport variant, with appropriate model paths wired into the respective transport parameter factories. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/ai/voice/agents/breeze_buddy/agent/transport.py (2)
112-116: Avoid instantiating the unused AIC filter.Each call to
get_transport_params()now eagerly constructs both adailyand atelephonyAICFilter, but only one lambda in the returned dict is ever invoked per call (the transport is selected by the caller viatransport_params[transport_type]()). Loading the unused.aicmodeland holding the extra license-bound resource is wasted work on every setup path.Consider deferring filter creation into the per-transport lambdas so only the selected transport pays the cost:
♻️ Proposed refactor
- # Create filters per transport type for proper model selection - audio_in_filter_daily = _create_audio_input_filter( - configurations, TRANSPORT_TYPE_DAILY - ) - audio_in_filter_telephony = _create_audio_input_filter(configurations, "twilio") - return { "daily": lambda: DailyParams( audio_in_enabled=True, audio_out_enabled=True, - audio_in_filter=audio_in_filter_daily, + audio_in_filter=_create_audio_input_filter( + configurations, TRANSPORT_TYPE_DAILY + ), # Note: DailyParams does not support audio_out_mixer ), "twilio": lambda: FastAPIWebsocketParams( ... - audio_in_filter=audio_in_filter_telephony, + audio_in_filter=_create_audio_input_filter(configurations, "twilio"), ), ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/ai/voice/agents/breeze_buddy/agent/transport.py` around lines 112 - 116, get_transport_params currently eagerly constructs both audio_in_filter_daily and audio_in_filter_telephony by calling _create_audio_input_filter(...) even though the returned dict only invokes one lambda; move the _create_audio_input_filter(...) calls into the respective per-transport lambdas (the values in the dict returned by get_transport_params) so each AICFilter is created lazily when transport_params[transport_type]() is executed (use TRANSPORT_TYPE_DAILY for the daily branch and "twilio" for the telephony branch) to avoid loading the unused .aicmodel and consuming the license-bound resource.
116-116: Extract a telephony transport constant.The string
"twilio"is used here as a stand-in for "any non-daily / telephony transport" to drive 8kHz model selection in_get_aic_model_path. This is fragile — if someone renamesTRANSPORT_TYPE_DAILYsemantics or adds a new telephony transport that happens to match"daily", the selection silently breaks. Consider a dedicated constant (e.g.TRANSPORT_TYPE_TELEPHONY = "telephony") and branch_get_aic_model_pathexplicitly on daily vs telephony intent rather than on concrete transport names.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/ai/voice/agents/breeze_buddy/agent/transport.py` at line 116, Replace the hard-coded "twilio" transport string with a dedicated telephony constant and update model-selection logic: add a constant like TRANSPORT_TYPE_TELEPHONY = "telephony", change the call that sets audio_in_filter_telephony (currently using _create_audio_input_filter(..., "twilio")) to use that constant, and modify _get_aic_model_path to branch explicitly on TRANSPORT_TYPE_DAILY vs TRANSPORT_TYPE_TELEPHONY (instead of matching concrete transport names) so 8kHz model selection is driven by transport intent rather than a specific provider name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/ai/voice/agents/breeze_buddy/agent/transport.py`:
- Around line 112-116: get_transport_params currently eagerly constructs both
audio_in_filter_daily and audio_in_filter_telephony by calling
_create_audio_input_filter(...) even though the returned dict only invokes one
lambda; move the _create_audio_input_filter(...) calls into the respective
per-transport lambdas (the values in the dict returned by get_transport_params)
so each AICFilter is created lazily when transport_params[transport_type]() is
executed (use TRANSPORT_TYPE_DAILY for the daily branch and "twilio" for the
telephony branch) to avoid loading the unused .aicmodel and consuming the
license-bound resource.
- Line 116: Replace the hard-coded "twilio" transport string with a dedicated
telephony constant and update model-selection logic: add a constant like
TRANSPORT_TYPE_TELEPHONY = "telephony", change the call that sets
audio_in_filter_telephony (currently using _create_audio_input_filter(...,
"twilio")) to use that constant, and modify _get_aic_model_path to branch
explicitly on TRANSPORT_TYPE_DAILY vs TRANSPORT_TYPE_TELEPHONY (instead of
matching concrete transport names) so 8kHz model selection is driven by
transport intent rather than a specific provider name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a1d6d70-ac3f-47e7-98e7-5e7097e76e6d
📒 Files selected for processing (2)
app/ai/voice/agents/breeze_buddy/agent/transport.pyapp/core/config/static.py
There was a problem hiding this comment.
Pull request overview
Adds transport-aware selection of the ai-coustics (AIC) noise cancellation model so Breeze Buddy can use a 16kHz model for Daily (web) audio and keep using an 8kHz model for telephony transports.
Changes:
- Introduces a new
AIC_MODEL_PATH_16KHZstatic config entry with a default 16kHz model path. - Adds
_get_aic_model_path()and threadstransport_typeinto Breeze Buddy AIC filter creation to select 8kHz vs 16kHz models. - Updates transport parameter construction to use different input filters for Daily vs telephony transports.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/core/config/static.py | Adds a new env-configurable 16kHz AIC model path. |
| app/ai/voice/agents/breeze_buddy/agent/transport.py | Selects AIC model by transport and wires per-transport audio input filters. |
kanakagrawal-crypto
force-pushed
the
noise_cancel
branch
from
59aed40 to
147b073
Compare
kanakagrawal-crypto
force-pushed
the
noise_cancel
branch
from
147b073 to
75fda1c
Compare
kanakagrawal-crypto
force-pushed
the
noise_cancel
branch
from
75fda1c to
300cb21
Compare
kanakagrawal-crypto
force-pushed
the
noise_cancel
branch
from
300cb21 to
fa134e8
Compare
Summary by CodeRabbit