feat: add default call execution config fallback support#754
feat: add default call execution config fallback support#754Swetha-160303 wants to merge 1 commit into
Conversation
WalkthroughThis PR enables call execution configs to have a NULL ChangesDefault Call Execution Config Support
Sequence Diagram(s)The conditions for generating sequence diagrams are not met for this change. While the PR introduces fallback logic, it is a straightforward addition of one more search step to an existing selection pattern rather than a new multi-component interaction requiring visualization. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/api/routers/breeze_buddy/leads/handlers.py (1)
230-264: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winDuplicated 3-step config-resolution logic with
_get_lead_config.Lines 233–259 implement exactly the same three-step (
template_id→ name → default) resolution as_get_lead_configinapp/ai/voice/agents/breeze_buddy/managers/calls.py. Extracting this into a shared helper (e.g.,select_call_execution_config(configs, template_id, template_name)) would prevent future divergence — for example, only one site getting a fix if the resolution rules evolve again.The same reseller-level-default edge case I noted on
_get_lead_configapplies here as well (root cause is in the accessor's fallback strategy, not duplicated comment needed).♻️ Sketch of shared helper
# e.g., app/database/accessor/breeze_buddy/call_execution_config.py or a utils module def select_matching_config( configs: list[CallExecutionConfig], template_id: Optional[str], template_name: Optional[str], ) -> Optional[CallExecutionConfig]: if template_id: match = next( (c for c in configs if c.template_id and c.template_id == template_id), None, ) if match: return match if template_name: match = next( ( c for c in configs if c.template == template_name and (not template_id or not c.template_id) ), None, ) if match: return match # Default fallback (template IS NULL) return next((c for c in configs if c.template is None), None)🤖 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/leads/handlers.py` around lines 230 - 264, The three-step config resolution duplicated here (lines using call_execution_configs and template.id/template.name) should be extracted to a shared helper to avoid drift with _get_lead_config; create a function (e.g., select_matching_config or select_call_execution_config) that accepts configs, template_id and template_name and implements the same template_id → name (only when config.template_id is unset) → default (template is None) logic, then replace the inline block in this handler with a call to that helper and remove the duplicated logic so both this handler and _get_lead_config call the same function.app/ai/voice/agents/breeze_buddy/managers/calls.py (1)
80-118:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftEdge case: reseller-level default may be skipped when merchant has template-specific configs.
get_call_execution_config_by_merchant_idreturns all configs for a merchant regardless of template. If a merchant has e.g. a template-specific config for templateAbut no merchant-level default, and a lead arrives with templateB, the accessor returns the template-Aconfig. The fallback to reseller-level configs (merchant_id NULL) only triggers when the merchant query returns zero rows, so it won't activate. Back in_get_lead_config, steps 1–3 won't match templateA, returningNoneand silently skipping any reseller-level default config at (merchant_id IS NULL, template IS NULL).If the intent is to always fall back to reseller-level defaults when merchant-specific configs don't match, a fourth lookup is needed (or an accessor change to unconditionally include reseller-level rows). If this behavior is intentional (merchant manages own configs, no implicit reseller fallback), document it to clarify the precedence.
🤖 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/ai/voice/agents/breeze_buddy/managers/calls.py` around lines 80 - 118, The current matching flow (in the code that calls get_call_execution_config_by_merchant_id and selects a CallExecutionConfig by template/template_id) can miss a reseller-level default when merchant-specific configs exist but none match the lead's template; add an explicit fourth lookup for reseller-level defaults: if no config is found after checking template_id, template match, and merchant default (template IS NULL), call get_call_execution_config_by_merchant_id with the same reseller_id and a NULL/None merchant_id (or use an accessor that returns reseller-level configs) and pick the reseller default (template IS NULL) if present; alternatively, update the accessor to always include reseller-level rows so _get_lead_config can find them without a second call—ensure you reference get_call_execution_config_by_merchant_id and the config selection logic (template_id/template matching and template IS NULL) when making the change.
🧹 Nitpick comments (1)
app/database/queries/breeze_buddy/call_execution_config.py (1)
14-31: 💤 Low valueON CONFLICT targets correctly mirror the four partial indexes.
I cross-checked each of the four returned target strings against migration 029, and the index columns/predicates match exactly, which is required for PostgreSQL's index inference.
Minor consistency note: the helper uses
template is None/merchant_id is not Nonesemantics (correct), whileupdate_call_execution_config_queryat line 346 uses a truthy check (if merchant_id:). Today both yield the same result becausemerchant_idis eitherNoneor a non-empty UUID, but standardizing onis not Nonewould prevent surprises if an empty string ever leaks in. Optional.🤖 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/database/queries/breeze_buddy/call_execution_config.py` around lines 14 - 31, Standardize null checks to use explicit "is None" / "is not None" semantics to match _conflict_target: update the conditional in update_call_execution_config_query (and any related locals there) to check merchant_id is not None (and template is None/is not None where applicable) instead of a truthy check like "if merchant_id:" so behavior won't change if an empty string or other falsy-but-not-None value appears; mirror the same null-check logic used in _conflict_target for consistency.
🤖 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.
Outside diff comments:
In `@app/ai/voice/agents/breeze_buddy/managers/calls.py`:
- Around line 80-118: The current matching flow (in the code that calls
get_call_execution_config_by_merchant_id and selects a CallExecutionConfig by
template/template_id) can miss a reseller-level default when merchant-specific
configs exist but none match the lead's template; add an explicit fourth lookup
for reseller-level defaults: if no config is found after checking template_id,
template match, and merchant default (template IS NULL), call
get_call_execution_config_by_merchant_id with the same reseller_id and a
NULL/None merchant_id (or use an accessor that returns reseller-level configs)
and pick the reseller default (template IS NULL) if present; alternatively,
update the accessor to always include reseller-level rows so _get_lead_config
can find them without a second call—ensure you reference
get_call_execution_config_by_merchant_id and the config selection logic
(template_id/template matching and template IS NULL) when making the change.
In `@app/api/routers/breeze_buddy/leads/handlers.py`:
- Around line 230-264: The three-step config resolution duplicated here (lines
using call_execution_configs and template.id/template.name) should be extracted
to a shared helper to avoid drift with _get_lead_config; create a function
(e.g., select_matching_config or select_call_execution_config) that accepts
configs, template_id and template_name and implements the same template_id →
name (only when config.template_id is unset) → default (template is None) logic,
then replace the inline block in this handler with a call to that helper and
remove the duplicated logic so both this handler and _get_lead_config call the
same function.
---
Nitpick comments:
In `@app/database/queries/breeze_buddy/call_execution_config.py`:
- Around line 14-31: Standardize null checks to use explicit "is None" / "is not
None" semantics to match _conflict_target: update the conditional in
update_call_execution_config_query (and any related locals there) to check
merchant_id is not None (and template is None/is not None where applicable)
instead of a truthy check like "if merchant_id:" so behavior won't change if an
empty string or other falsy-but-not-None value appears; mirror the same
null-check logic used in _conflict_target for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b303a028-8900-4931-88c4-dade48ef7944
📒 Files selected for processing (6)
app/ai/voice/agents/breeze_buddy/managers/calls.pyapp/api/routers/breeze_buddy/leads/handlers.pyapp/database/accessor/breeze_buddy/call_execution_config.pyapp/database/migrations/029_nullable_template_in_call_execution_config.sqlapp/database/queries/breeze_buddy/call_execution_config.pyapp/schemas/breeze_buddy/core.py
There was a problem hiding this comment.
Pull request overview
This PR adds support for “default” Breeze Buddy call execution configs by allowing call_execution_config.template to be NULL, and then falling back to that default config when a template-specific config is not found.
Changes:
- Make
templateoptional in Breeze Buddy call execution config schemas and DB insert/update query APIs. - Add migration to allow
templateto be nullable and replace uniqueness enforcement with partial unique indexes for all(template, merchant_id)nullability combinations. - Add runtime fallback logic in lead handling and call execution to use a default config where
template IS NULL.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/schemas/breeze_buddy/core.py | Makes template optional on request/response models to support default configs. |
| app/database/queries/breeze_buddy/call_execution_config.py | Adds partial-index-aware upsert conflict targeting and updates update-query WHERE logic for nullable templates. |
| app/database/migrations/029_nullable_template_in_call_execution_config.sql | Makes template nullable and introduces partial unique indexes for correct deduplication with NULLs. |
| app/database/accessor/breeze_buddy/call_execution_config.py | Updates accessor signatures/docs to accept nullable template and describe default-config behavior. |
| app/api/routers/breeze_buddy/leads/handlers.py | Adds fallback to select a default call execution config when no template match is found. |
| app/ai/voice/agents/breeze_buddy/managers/calls.py | Adds fallback to select a default call execution config during call processing. |
| # Step 3: fall back to the default config (template IS NULL) for this merchant | ||
| config = next( | ||
| (c for c in call_execution_configs if c.template is None), | ||
| None, | ||
| ) | ||
| if not config: |
| # Step 3: fall back to the default config (template IS NULL) for this merchant | ||
| config = next( | ||
| (c for c in configs if c.template is None), | ||
| None, | ||
| ) | ||
| if not config: |
| if merchant_id: | ||
| values.append(merchant_id) | ||
| merchant_identifier_param = param_count | ||
| where_clause = f'reseller_id = ${reseller_id_param} AND "template" = ${template_param} AND merchant_id = ${merchant_identifier_param}' | ||
| where_clause = f"reseller_id = ${reseller_id_param} AND {template_clause} AND merchant_id = ${merchant_identifier_param}" | ||
| else: | ||
| where_clause = f'reseller_id = ${reseller_id_param} AND "template" = ${template_param} AND merchant_id IS NULL' | ||
| where_clause = f"reseller_id = ${reseller_id_param} AND {template_clause} AND merchant_id IS NULL" |
Summary by CodeRabbit
New Features
Improvements