Template Based Architechture #417
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces a template-based workflow system for the Breeze Buddy voice agent, replacing the workflow-enum architecture. A new Changes
Sequence DiagramsequenceDiagram
actor Client
participant WS as WebSocket
participant Agent as OrderConfirmationBot
participant TplLoader as FlowConfigLoader
participant TplBuilder as FlowConfigBuilder
participant Pipeline as PipelineRunner
participant LLM as Azure LLM
participant Hooks as HookRegistry
participant DB as Database
Client->>WS: Connect & Send Lead Data
WS->>Agent: New Connection
Agent->>Agent: Parse Lead Data
Agent->>DB: Get Lead by call_id
DB-->>Agent: Lead Payload
Agent->>TplLoader: Load Template
TplLoader->>DB: Get Template by Merchant
DB-->>TplLoader: TemplateModel
TplLoader->>TplLoader: Render Messages
TplLoader-->>Agent: Rendered Template
Agent->>TplBuilder: Build Flow Config
TplBuilder->>TplBuilder: Map Handlers & Hooks
TplBuilder-->>Agent: Pipecat Flow
Agent->>Pipeline: Initialize with Flow
Pipeline->>Pipeline: Assemble Components<br/>(VAD, STT, TTS, LLM)
Pipeline-->>Agent: Ready
Client->>WS: Audio Stream
WS->>Pipeline: Audio Frames
Pipeline->>LLM: Process & Generate Response
LLM-->>Pipeline: LLM Output
Pipeline->>Client: Audio Response
LLM->>Hooks: Trigger Hook (transition/outcome)
Hooks->>DB: Update Lead Outcome
DB-->>Hooks: Updated
Client->>WS: Disconnect
Agent->>Agent: Cleanup & Finalize
Agent->>DB: Update Call Completion
Agent->>Client: Hangup
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly Related PRs
Suggested Reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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.
Actionable comments posted: 9
🧹 Nitpick comments (20)
app/database/queries/breeze_buddy/template.py (1)
11-25: Use explicit optional types for optional parametersTo satisfy Ruff (RUF013) and keep typing consistent with other parts of the codebase that use
T | None, consider updating the signature to:def get_template_by_merchant_query( merchant_id: str, shop_identifier: str | None = None, name: str | None = None, ) -> Tuple[str, List[Any]]:This makes the
Optionalnature explicit and avoids the implicitstr = Nonepattern.app/ai/voice/agents/breeze_buddy/workflow/types.py (1)
29-35: Avoid mutable defaults in Pydantic modelsSeveral fields use mutable literals as defaults (
{}/[]):
FlowFunction.properties,FlowFunction.required,FlowFunction.hooksFlowNodeModel.pre_actions,FlowNodeModel.post_actions,FlowNodeModel.functionsRequestFlowFunction.properties,RequestFlowFunction.required,RequestFlowFunction.hooksRequestFlowNode.task_messages,RequestFlowNode.pre_actions,RequestFlowNode.post_actions,RequestFlowNode.functionsEven though Pydantic often wraps these, the more robust pattern is:
from pydantic import BaseModel, Field class FlowFunction(BaseModel): name: str description: str properties: Dict[str, Any] = Field(default_factory=dict) required: List[str] = Field(default_factory=list) transition_to: Optional[str] = None hooks: List[str] = Field(default_factory=list)…and similarly for the other models. This avoids any ambiguity about shared mutable defaults and keeps things future-proof across Pydantic versions.
Also applies to: 38-43, 56-63, 66-71
app/database/accessor/__init__.py (1)
43-72: Consider alphabetically sorting__all__for maintainability.The new exports
create_templateandget_template_by_merchantare placed at the beginning of__all__, but the rest of the list appears to follow a grouping pattern by module. For consistency and easier navigation, consider sorting__all__alphabetically.app/ai/voice/agents/breeze_buddy/workflow/__init__.py (1)
27-36: Consider alphabetically sorting__all__for consistency.The exports in
__all__are grouped logically but not alphabetically sorted. For easier maintenance and consistency with Python conventions, consider sorting them alphabetically.app/ai/voice/agents/breeze_buddy/handlers/internal/__init__.py (2)
1-5: Update docstring to reflect generic handler scope.The docstring mentions "order confirmation workflow" specifically, but this module appears to provide internal handlers that could be used across different workflows. Consider updating it to reflect the broader scope.
Apply this diff:
""" -Handler functions for order confirmation workflow. +Internal handler functions for Breeze Buddy workflows. -This package contains all handler functions organized by category. +This package contains internal action handlers used across workflows. """
14-19: Consider alphabetically sorting__all__.For consistency and maintainability, consider sorting the exports alphabetically.
app/database/migrations/003_create_template_table.sql (1)
11-12: Consider adding a trigger to auto-updateupdated_at.The
updated_atcolumn defaults toNOW()on insert but won't automatically update when rows are modified. If you wantupdated_atto reflect the last modification time, add an update trigger.-- Add after table creation CREATE OR REPLACE FUNCTION update_updated_at_column() RETURNS TRIGGER AS $$ BEGIN NEW.updated_at = NOW(); RETURN NEW; END; $$ language 'plpgsql'; CREATE TRIGGER update_template_updated_at BEFORE UPDATE ON template FOR EACH ROW EXECUTE FUNCTION update_updated_at_column();app/ai/voice/agents/breeze_buddy/websocket_bot.py (1)
636-643: Themeta_datastructure consolidates payload nicely.The refactored
meta_datadict cleanly groups related fields. Note thatcall_sidappears both inmeta_data["transcription"](line 639) and as a separatecall_idparameter (line 645). If the downstreamcompletion_functiononly needs it once, consider removing the redundancy.app/database/accessor/breeze_buddy/template.py (3)
29-31: Use explicitOptionalor| Nonefor nullable parameters.Per PEP 484, implicit
Optional(defaulting toNonewithout type annotation) is discouraged. Use explicit type hints for clarity.async def get_template_by_merchant( - merchant_id: str, shop_identifier: str = None, name: str = None + merchant_id: str, shop_identifier: Optional[str] = None, name: Optional[str] = None ) -> Optional[TemplateModel]:
55-63: Add type hint fornowparameter.The
nowparameter lacks a type hint. Based on usage withcreated_at/updated_attimestamp columns, this should be typed asdatetime.+from datetime import datetime + async def create_template( template_id: str, merchant_id: str, - shop_identifier: str, + shop_identifier: Optional[str], name: str, flow: dict, is_active: bool, - now, + now: datetime, ) -> Optional[TemplateModel]:
50-52: Consider logging the exception type for better debugging.The broad
Exceptioncatch is acceptable here since you're logging and returningNone, but logging the exception class name could help with debugging.except Exception as e: - logger.error(f"Error getting template by merchant: {e}") + logger.error(f"Error getting template by merchant: {type(e).__name__}: {e}") return NoneAlso applies to: 91-93
app/ai/voice/agents/breeze_buddy/workflow/transition.py (1)
53-55: Store the task reference to prevent garbage collection and enable error tracking.The
asyncio.create_taskreturn value should be stored. Without a reference, the task may be garbage collected before completion, and any unhandled exceptions will be silently lost.+ # Store task reference to prevent garbage collection if hooks: logger.info( f"Scheduling {len(hooks)} hook(s) to execute asynchronously for function '{function_name}'" ) - asyncio.create_task( + task = asyncio.create_task( _execute_hooks_async(context, args, hooks, function_name or "unknown") ) + # Optionally add error callback for logging + task.add_done_callback( + lambda t: logger.error(f"Hook task failed: {t.exception()}") if t.exception() else None + )app/ai/voice/agents/breeze_buddy/workflow/builder.py (1)
82-93: Potential unintended mutation of input data.
node_data.copy()creates a shallow copy, so nested objects like thefunctionslist and its dicts are shared with the original. Modifyingfunc["name"]mutates the originalnodes_datainput. Consider usingcopy.deepcopyor restructuring to avoid side effects.+import copy + # Convert nodes data to FlowNodeModel objects flow_nodes = [] for node_data in nodes_data: # Transform function_name to name for compatibility - transformed_node_data = node_data.copy() + transformed_node_data = copy.deepcopy(node_data) if "functions" in transformed_node_data:app/ai/voice/agents/breeze_buddy/workflow/context.py (1)
168-168: Potential type mismatch ifargs[0]is not a dict.If
args[0]exists but is not a dict-like object, the handler might receive unexpected data. Consider adding validation or documentation about expected argument types.- llm_args = args[0] if len(args) > 0 else {} + llm_args = args[0] if len(args) > 0 and isinstance(args[0], dict) else {}app/ai/voice/agents/breeze_buddy/workflow/loader.py (1)
57-85: Consider recursive placeholder substitution risk.If a variable value contains a placeholder pattern like
{other_key}, it won't be substituted (which is likely intended). However, if a variable value itself contains{key}wherekeyis another variable, this could lead to unexpected partially-substituted content. Document this behavior or consider whether single-pass substitution is the desired behavior.app/api/routers/breeze_buddy.py (1)
118-120: Broad exception catch returns 400 for all errors.The catch-all
Exceptionhandler returns 400 (Bad Request) even for server-side errors.ValueErrorfrom validation should be 400, but database failures should be 500.- except Exception as e: - logger.error(f"Error creating workflow from JSON: {e}", exc_info=True) - return JSONResponse(status_code=400, content={"detail": str(e)}) + except ValueError as e: + logger.warning(f"Validation error creating workflow: {e}") + return JSONResponse(status_code=400, content={"detail": str(e)}) + except HTTPException: + raise + except Exception as e: + logger.error(f"Error creating workflow from JSON: {e}", exc_info=True) + return JSONResponse(status_code=500, content={"detail": "Internal server error"})app/ai/voice/agents/breeze_buddy/workflow/hooks.py (2)
132-140: All non-outcome args added to metadata without filtering.Lines 135-140 add all arguments (except "outcome") to the lead's metadata. This could inadvertently persist sensitive data or LLM-generated content. Consider an allowlist of expected metadata keys.
+ # Only persist expected metadata keys + ALLOWED_METADATA_KEYS = {"cancellation_reason", "updated_address", "notes"} + for key, value in args.items(): - if key != "outcome" and value is not None: + if key in ALLOWED_METADATA_KEYS and value is not None: meta_data[key] = value
200-200: Annotate mutable class attribute withClassVar.The
_hooksdict is a class-level mutable attribute shared across all instances. Per PEP 526 and static analysis, annotate withClassVarfor clarity.+from typing import Any, ClassVar, Dict, Optional -from typing import Any, Dict, Optional class HookRegistry: ... - _hooks: Dict[str, Hook] = {} + _hooks: ClassVar[Dict[str, Hook]] = {}app/ai/voice/agents/breeze_buddy/agent.py (2)
127-134: Consider catching more specific exceptions for WebSocket close operations.The broad
Exceptioncatches (lines 130, 164, 196, 215) work for graceful degradation but could mask unexpected errors. For WebSocket operations, consider catchingWebSocketDisconnectorRuntimeErrorspecifically.except json.JSONDecodeError as e: logger.error(f"Failed to parse call data JSON: {e}") try: if self.ws.client_state.name != "DISCONNECTED": await self.ws.close(code=4000, reason="Invalid JSON data") - except Exception as close_error: + except (RuntimeError, Exception) as close_error: logger.warning( f"Could not close websocket (likely already closed): {close_error}" )Note: The static analysis tool flagged
BLE001for catching blind exceptions. The current approach is pragmatic for handling already-closed connections, so this is optional to address.
239-243: Handler wrapping approach works but consider alternative patterns.Iterating over
handler_map.items()and modifying values is safe in Python (the view is taken before modification). However, if the map grows, consider wrapping handlers lazily at invocation time rather than eagerly at initialization.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
app/ai/voice/agents/breeze_buddy/agent.py(1 hunks)app/ai/voice/agents/breeze_buddy/dashboard/login.html(1 hunks)app/ai/voice/agents/breeze_buddy/handlers/internal/__init__.py(1 hunks)app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py(1 hunks)app/ai/voice/agents/breeze_buddy/managers/calls.py(4 hunks)app/ai/voice/agents/breeze_buddy/sample/order-confirmation-workflow.json(1 hunks)app/ai/voice/agents/breeze_buddy/services/telephony/exotel/exotel.py(1 hunks)app/ai/voice/agents/breeze_buddy/services/telephony/twilio/twilio.py(1 hunks)app/ai/voice/agents/breeze_buddy/types/models.py(1 hunks)app/ai/voice/agents/breeze_buddy/websocket_bot.py(3 hunks)app/ai/voice/agents/breeze_buddy/workflow/__init__.py(1 hunks)app/ai/voice/agents/breeze_buddy/workflow/builder.py(1 hunks)app/ai/voice/agents/breeze_buddy/workflow/context.py(1 hunks)app/ai/voice/agents/breeze_buddy/workflow/hooks.py(1 hunks)app/ai/voice/agents/breeze_buddy/workflow/loader.py(1 hunks)app/ai/voice/agents/breeze_buddy/workflow/transition.py(1 hunks)app/ai/voice/agents/breeze_buddy/workflow/types.py(1 hunks)app/api/routers/breeze_buddy.py(5 hunks)app/database/accessor/__init__.py(1 hunks)app/database/accessor/breeze_buddy/lead_call_tracker.py(1 hunks)app/database/accessor/breeze_buddy/template.py(1 hunks)app/database/decoder/breeze_buddy/template.py(1 hunks)app/database/migrations/001_initial_tables.sql(1 hunks)app/database/migrations/002_add_shop_identifier_and_locks.sql(1 hunks)app/database/migrations/003_create_template_table.sql(1 hunks)app/database/queries/breeze_buddy/lead_call_tracker.py(1 hunks)app/database/queries/breeze_buddy/template.py(1 hunks)scripts/create_tables.py(0 hunks)
💤 Files with no reviewable changes (1)
- scripts/create_tables.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-15T04:10:50.071Z
Learnt from: badri-singhal
Repo: juspay/clairvoyance PR: 313
File: app/api/routers/breeze_buddy.py:487-489
Timestamp: 2025-10-15T04:10:50.071Z
Learning: In the Breeze Buddy dashboard (`app/agents/voice/breeze_buddy/workflows/order_confirmation/dashboard.html`), analytics metrics are dynamically rendered using a for loop in the HTML script. When adding new metrics to the JSON response in the `get_analytics()` endpoint, no manual HTML template updates are required as the loop handles all metrics automatically.
Applied to files:
app/ai/voice/agents/breeze_buddy/sample/order-confirmation-workflow.jsonapp/api/routers/breeze_buddy.pyapp/ai/voice/agents/breeze_buddy/dashboard/login.htmlapp/ai/voice/agents/breeze_buddy/websocket_bot.py
🧬 Code graph analysis (11)
app/database/accessor/__init__.py (1)
app/database/accessor/breeze_buddy/template.py (2)
create_template(55-93)get_template_by_merchant(29-52)
app/database/accessor/breeze_buddy/template.py (4)
app/ai/voice/agents/breeze_buddy/workflow/types.py (1)
TemplateModel(46-53)app/database/decoder/breeze_buddy/template.py (1)
decode_template(15-36)app/database/queries/__init__.py (1)
run_parameterized_query(14-30)app/database/queries/breeze_buddy/template.py (2)
create_template_query(36-62)get_template_by_merchant_query(11-33)
app/ai/voice/agents/breeze_buddy/workflow/__init__.py (5)
app/ai/voice/agents/breeze_buddy/workflow/builder.py (1)
FlowConfigBuilder(27-249)app/ai/voice/agents/breeze_buddy/workflow/context.py (3)
context(52-54)WorkflowContext(14-127)with_context(130-200)app/ai/voice/agents/breeze_buddy/workflow/hooks.py (3)
Hook(26-73)HookRegistry(193-234)UpdateOutcomeInDatabaseHook(76-190)app/ai/voice/agents/breeze_buddy/workflow/loader.py (1)
FlowConfigLoader(16-160)app/ai/voice/agents/breeze_buddy/workflow/transition.py (1)
transition_handler(19-69)
app/database/decoder/breeze_buddy/template.py (1)
app/ai/voice/agents/breeze_buddy/workflow/types.py (1)
TemplateModel(46-53)
app/ai/voice/agents/breeze_buddy/workflow/hooks.py (3)
app/ai/voice/agents/breeze_buddy/workflow/context.py (3)
context(52-54)WorkflowContext(14-127)lead(57-59)app/database/accessor/breeze_buddy/lead_call_tracker.py (1)
update_lead_call_completion_details(282-312)app/schemas.py (1)
LeadCallOutcome(32-38)
app/api/routers/breeze_buddy.py (4)
app/ai/voice/agents/breeze_buddy/types/models.py (1)
LeadData(15-25)app/ai/voice/agents/breeze_buddy/workflow/types.py (1)
CreateTemplateRequest(74-80)app/database/accessor/breeze_buddy/template.py (1)
create_template(55-93)app/schemas.py (1)
TokenData(146-152)
app/ai/voice/agents/breeze_buddy/workflow/types.py (1)
app/schemas.py (1)
Workflow(21-22)
app/ai/voice/agents/breeze_buddy/workflow/transition.py (3)
app/ai/voice/agents/breeze_buddy/analytics/tracing_setup.py (1)
auto_trace(57-106)app/ai/voice/agents/breeze_buddy/workflow/context.py (3)
context(52-54)WorkflowContext(14-127)create_node_from_template(96-127)app/ai/voice/agents/breeze_buddy/workflow/hooks.py (3)
HookRegistry(193-234)get(214-224)safe_execute(50-73)
app/ai/voice/agents/breeze_buddy/workflow/loader.py (3)
app/ai/voice/agents/breeze_buddy/workflow/types.py (1)
TemplateModel(46-53)app/schemas.py (1)
Workflow(21-22)app/database/accessor/breeze_buddy/template.py (1)
get_template_by_merchant(29-52)
app/database/queries/breeze_buddy/lead_call_tracker.py (1)
app/schemas.py (2)
LeadCallStatus(25-29)LeadCallOutcome(32-38)
app/ai/voice/agents/breeze_buddy/websocket_bot.py (2)
app/ai/voice/agents/breeze_buddy/types/models.py (1)
OrderData(11-12)app/ai/voice/agents/breeze_buddy/workflow/context.py (1)
call_sid(62-64)
🪛 Ruff (0.14.8)
app/ai/voice/agents/breeze_buddy/handlers/internal/__init__.py
14-19: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
app/database/accessor/breeze_buddy/template.py
30-30: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
30-30: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
48-48: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
89-89: Consider moving this statement to an else block
(TRY300)
91-91: Do not catch blind exception: Exception
(BLE001)
app/ai/voice/agents/breeze_buddy/workflow/__init__.py
27-36: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py
21-21: Unused function argument: args
(ARG001)
21-21: Unused function argument: transition_to
(ARG001)
45-45: Unused function argument: args
(ARG001)
45-45: Unused function argument: transition_to
(ARG001)
69-69: Unused function argument: transition_to
(ARG001)
105-105: Do not catch blind exception: Exception
(BLE001)
118-118: Unused function argument: args
(ARG001)
118-118: Unused function argument: transition_to
(ARG001)
200-200: f-string without any placeholders
Remove extraneous f prefix
(F541)
202-202: Do not catch blind exception: Exception
(BLE001)
204-204: Use explicit conversion flag
Replace with conversion flag
(RUF010)
217-217: Unused function argument: transition_to
(ARG001)
314-314: Do not catch blind exception: Exception
(BLE001)
326-326: Do not catch blind exception: Exception
(BLE001)
app/database/queries/breeze_buddy/template.py
12-12: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
12-12: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
26-31: Possible SQL injection vector through string-based query construction
(S608)
47-51: Possible SQL injection vector through string-based query construction
(S608)
app/ai/voice/agents/breeze_buddy/workflow/hooks.py
69-69: Do not catch blind exception: Exception
(BLE001)
71-71: Use explicit conversion flag
Replace with conversion flag
(RUF010)
185-185: Do not catch blind exception: Exception
(BLE001)
200-200: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
app/api/routers/breeze_buddy.py
71-71: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
83-83: Abstract raise to an inner function
(TRY301)
83-83: Avoid specifying long messages outside the exception class
(TRY003)
86-86: Abstract raise to an inner function
(TRY301)
86-86: Avoid specifying long messages outside the exception class
(TRY003)
89-89: Abstract raise to an inner function
(TRY301)
89-89: Avoid specifying long messages outside the exception class
(TRY003)
104-104: Abstract raise to an inner function
(TRY301)
104-104: Create your own exception
(TRY002)
104-104: Avoid specifying long messages outside the exception class
(TRY003)
118-118: Do not catch blind exception: Exception
(BLE001)
app/ai/voice/agents/breeze_buddy/agent.py
130-130: Do not catch blind exception: Exception
(BLE001)
164-164: Do not catch blind exception: Exception
(BLE001)
196-196: Do not catch blind exception: Exception
(BLE001)
215-215: Do not catch blind exception: Exception
(BLE001)
360-360: Unused function argument: transport
(ARG001)
386-386: Unused function argument: transport
(ARG001)
391-391: Unused function argument: task
(ARG001)
app/ai/voice/agents/breeze_buddy/workflow/builder.py
64-64: Avoid specifying long messages outside the exception class
(TRY003)
69-69: Avoid specifying long messages outside the exception class
(TRY003)
76-76: Avoid specifying long messages outside the exception class
(TRY003)
102-102: Avoid specifying long messages outside the exception class
(TRY003)
193-193: Avoid specifying long messages outside the exception class
(TRY003)
242-242: Avoid specifying long messages outside the exception class
(TRY003)
app/ai/voice/agents/breeze_buddy/workflow/transition.py
53-55: Store a reference to the return value of asyncio.create_task
(RUF006)
app/ai/voice/agents/breeze_buddy/workflow/loader.py
118-120: Avoid specifying long messages outside the exception class
(TRY003)
app/database/queries/breeze_buddy/lead_call_tracker.py
233-238: Possible SQL injection vector through string-based query construction
(S608)
🔇 Additional comments (40)
app/database/queries/breeze_buddy/template.py (1)
26-31: Static SQL-injection warning is a false positive hereRuff’s S608 warning is triggered by the f-strings, but in this file:
TEMPLATE_TABLEand the column names are hard-coded.- All dynamic values go through positional parameters (
$1,$2, …) andvalues.There is no user-controlled identifier interpolation, so these queries are safe under normal usage. No change needed, but you may want to suppress S608 for this file or function if it’s noisy in your CI.
Also applies to: 47-51
app/ai/voice/agents/breeze_buddy/managers/calls.py (2)
10-12: Import move to common utils looks goodSwitching
send_webhook_with_retryto the sharedutils.commonmodule aligns with the broader refactor and avoids workflow-specific coupling. No issues here.
374-379: Confirm callers and updater handle optional outcome/meta_data correctlyThe new
handle_call_completionsignature and returningupdated_leadmake sense, but a couple of things are worth double-checking:
- Ensure all call sites now pass
meta_dataas adictwhen they have details, and are fine withNoneotherwise.- Verify
update_lead_call_completion_detailsacceptsNoneforoutcomeandmeta_datawithout attempting to mutate them (e.g., no.updateonNone).If both are true, the new API is a solid improvement.
Also applies to: 401-407, 412-413
app/ai/voice/agents/breeze_buddy/services/telephony/twilio/twilio.py (1)
10-12: Centralizing websocket bot import is consistent with new architectureUsing
app.ai.voice.agents.breeze_buddy.websocket_botinstead of the workflow-specific module keeps Twilio tied to the generic Breeze Buddy websocket entrypoint. No functional issues spotted.app/ai/voice/agents/breeze_buddy/services/telephony/exotel/exotel.py (1)
10-12: Exotel websocket handler now uses shared Breeze Buddy websocket botAligning Exotel with the shared
breeze_buddy.websocket_botkeeps providers consistent and reduces workflow-specific coupling. Change looks safe.app/ai/voice/agents/breeze_buddy/dashboard/login.html (1)
373-376: Redirecting to unified/dashboardroute after loginUpdating the post-login redirect to
/agent/voice/breeze-buddy/dashboardmatches the new consolidated dashboard route and keeps the login flow aligned with the backend router changes. No further template changes are needed since the dashboard renders metrics dynamically from the API. Based on learnings, this won’t require manual metric wiring.app/ai/voice/agents/breeze_buddy/types/models.py (1)
15-26: LeadData model shape looks aligned with usageThe new
LeadDatamodel cleanly groups the customer, shop, order, and reporting fields that the Breeze Buddy flows use. The structure is sensible and should be a drop-in replacement for the previous order payload type.Ensure all previous references (e.g.,
BreezeOrderDataor similar models) have been updated across the codebase and in API handlers. Also review the inconsistent Optional syntax—the model uses bothOptional[str]andstr | Nonewhich should be standardized for consistency.app/ai/voice/agents/breeze_buddy/workflow/__init__.py (1)
1-36: LGTM! Clean public API surface for the workflow engine.The module provides a well-organized public interface for the workflow engine components. The docstring clearly describes the package's purpose.
app/database/decoder/breeze_buddy/template.py (1)
39-50: LGTM! Proper handling of list decoding.The function correctly handles empty results and applies the decoder to each row.
app/database/migrations/002_add_shop_identifier_and_locks.sql (1)
1-34: LGTM! Well-designed migration with proper safeguards.The migration effectively handles multi-shop scenarios and adds locking capabilities. Key highlights:
- Uses
IF NOT EXISTS/IF EXISTSfor idempotent execution- Partial unique indexes (lines 20-27) elegantly handle NULL vs. non-NULL shop_identifier cases
- Composite index on
(status, is_locked, next_attempt_at)(line 33-34) efficiently supports the locking query pattern- Default values ensure existing rows work correctly
app/ai/voice/agents/breeze_buddy/sample/order-confirmation-workflow.json (3)
1-67: LGTM! Well-structured initial node with clear instructions.The initial node provides comprehensive guidance for the agent, including:
- Gender-based addressing logic
- Clear conversation flow
- Pincode/phone number reading rules
- Proper function definitions with transitions and hooks
346-371: LGTM! Proper end-node configuration.The order confirmation end node correctly:
- Mutes STT before final message (prevents interruption)
- Calls end_conversation_handler
- Calls update_calling_service_handler for external notification
- Has no functions (terminal node)
14-14: Verify template variable handling in TTS conversion pipeline.The system prompt includes template variables with curly braces (
{shop_name},{customer_name},{order_summary}, etc.) while explicitly stating to avoid special characters for TTS conversion. Confirm that these variables are substituted with plain text before TTS processing and that curly braces do not reach the TTS engine.app/database/queries/breeze_buddy/lead_call_tracker.py (1)
196-240: LGTM! Safe dynamic SQL construction for partial updates.The function correctly builds parameterized queries dynamically based on which fields are provided. Key points:
- Only non-None fields are included in the UPDATE
- All values are properly parameterized (no SQL injection risk despite the static analysis warning)
- The incrementing parameter index pattern (
${len(values)}) is correct- Always updates
updated_atfor audit trailThe static analysis warning (S608) is a false positive—the SQL string construction uses only hardcoded column names, and all values are parameterized.
app/database/accessor/breeze_buddy/lead_call_tracker.py (1)
282-312: LGTM! Consistent partial update implementation.The function signature correctly aligns with the query layer changes to support partial updates. The docstring accurately describes the behavior, and all optional parameters are properly forwarded to the query function.
app/database/migrations/003_create_template_table.sql (1)
4-31: Schema design looks good overall.The partial unique indexes correctly handle the NULL
shop_identifiercase for uniqueness constraints, and the JSONB type is appropriate for flexible flow storage. The performance indexes are well-chosen.app/ai/voice/agents/breeze_buddy/websocket_bot.py (2)
39-47: Import reorganization looks good.The imports have been correctly updated to reflect the new module structure with
OrderDatafromtypes/modelsand utilities fromutils/common.
416-416: System prompt language instruction simplified appropriately.The instruction for multilingual response is clear and maintains the expected behavior.
app/database/accessor/breeze_buddy/template.py (1)
22-27: Implementation is sound.The accessor functions properly delegate to query builders, handle results via decoders, and include appropriate error handling with logging.
Also applies to: 35-52
app/ai/voice/agents/breeze_buddy/workflow/transition.py (1)
72-109: Async hook execution implementation is well-structured.Sequential hook execution with logging and graceful handling of missing hooks via
HookRegistry.getis appropriate. Thesafe_executecall ensures individual hook failures don't crash the background task.app/ai/voice/agents/breeze_buddy/workflow/builder.py (3)
196-206: Closure capturesfunccorrectly in this context.The wrapper function correctly captures
func.transition_to,func.hooks, andfunc.nameat schema creation time. Since each iteration creates a newwrapper_handlerfunction, the captured values are specific to each function schema.
247-249: Unknown action types are silently degraded.Returning
{"type": str(action_type)}for unknown action types may cause downstream failures that are hard to debug. Consider raising an error or at least logging at warning level (which you do) but also documenting expected behavior.Verify that downstream Pipecat flow processing handles unknown action types gracefully, or consider raising an error here for fail-fast behavior:
else: - logger.warning(f"Unknown action type: {action_type}, returning as string") - return {"type": str(action_type)} + logger.error(f"Unknown action type: {action_type}") + raise ValueError(f"Unknown action type: {action_type}")
27-41: FlowConfigBuilder structure is clean and well-organized.The builder pattern with a handler map, node building, and function schema generation is well-designed. Logging throughout aids debugging during workflow construction.
Also applies to: 118-171
app/ai/voice/agents/breeze_buddy/workflow/context.py (2)
14-95: Well-structured context wrapper with clean property accessors.The
WorkflowContextclass provides a clean abstraction for handlers to access bot state while maintaining the stateless handler pattern. The read-only properties with onlyconversation_endedbeing mutable is a good design choice.
130-200: Clean decorator factory pattern for context injection.The
with_contextdecorator cleanly separates transition handlers from action handlers and properly passes through the context. The logging is helpful for debugging.app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py (3)
21-43: STT mute handler looks good.The handler properly checks for VAD analyzer presence before modifying confidence and logs the state change appropriately. The unused
argsandtransition_toparameters are intentional for maintaining a uniform handler interface.
216-332: Webhook handler implementation is thorough.Good error handling, proper timezone usage for call duration calculation, and conditional webhook sending based on outcome. The logging provides good visibility into the webhook lifecycle.
135-139: Unable to locate the referenced file for verification.The file
app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.pydoes not exist in the repository, making it impossible to verify the code context and concerns. However, regarding the general concern abouttask.cancel()being called multiple times: Python'sasyncio.Task.cancel()is idempotent and safe to call repeatedly. It returnsFalseif the task is already done or cancelled, andTrueif the cancellation request was successfully registered. Therefore, even if the finally block executes after an early return, callingcancel()multiple times poses no issue.If you're reviewing code in a pending commit or branch, please ensure the file is available in the current repository state for verification.
app/ai/voice/agents/breeze_buddy/workflow/loader.py (3)
19-55: Template loading with lazy import is appropriate.The lazy import at line 42 to avoid circular dependency is a reasonable approach. Good logging of template loading progress.
141-150: Flexible task message format handling is good.The code gracefully handles Pydantic models (with
model_dump), dicts, and fallback formats. This makes the loader resilient to different data sources.
125-128: In-place mutation of template flow nodes.The template's
flow["nodes"]is mutated in place at line 128. This is efficient but means the original template object is modified. If the same template instance is reused elsewhere, this could cause unexpected behavior. Consider whether a deep copy is needed or if the template is intended to be modified in place.app/api/routers/breeze_buddy.py (2)
26-30: Type rename fromBreezeOrderDatatoLeadDataapplied correctly.The import and parameter type changes are consistent with the model rename mentioned in the summary.
Also applies to: 346-346
618-618: Dashboard route changed to /dashboard — preserve links or add redirectRoute moved from /breeze/order-confirmation/dashboard to /dashboard; update internal/external links or add a redirect to avoid breaking bookmarks. File: app/api/routers/breeze_buddy.py:618
app/ai/voice/agents/breeze_buddy/workflow/hooks.py (2)
26-73: Clean hook abstraction with safe execution wrapper.The
HookABC provides a good pattern withsafe_executehandling errors gracefully so hooks don't crash the main workflow. The logging insafe_executeprovides good observability.
237-238: Module-level hook registration is appropriate.Auto-registering hooks at module load is a clean pattern for this singleton registry approach.
app/ai/voice/agents/breeze_buddy/agent.py (5)
359-393: Unused event handler arguments are expected.The static analysis flags unused
transport,client, andtaskarguments (ARG001), but these are required by the event handler signature from the pipecat framework. You can silence these warnings with underscore prefixes if desired:
320-331: Pipeline construction looks correct.The pipeline stages follow a logical flow: transport input → STT → mute filter → context aggregation → LLM → TTS → transport output → assistant context. The bidirectional context aggregator placement is appropriate.
432-439: Disconnect handling is well-guarded.The
conversation_endedflag prevents double-processing of disconnect events. Setting outcome to"busy"as a fallback for unexpected disconnects is a reasonable default.
442-453: Clean entry point.The
main()function provides a simple factory interface for creating and running the bot.
1-2:audioopis deprecated and will be removed in Python 3.13.The
audioopmodule was deprecated in Python 3.11 and removed from the standard library in Python 3.13 (last included in 3.12). Consider using an alternative library such assoundfile, or theaudioop-ltsPyPI backport if Python 3.13+ support is required.
| context.bot.lead = await context.bot.completion_function( | ||
| call_id=context.call_sid, | ||
| outcome=context.lead.outcome, | ||
| call_end_time=datetime.now(), |
There was a problem hiding this comment.
Use timezone-aware datetime.now(timezone.utc) for consistency.
Line 195 uses datetime.now() without timezone, while timezone is already imported and used elsewhere in this file (line 8). This could cause issues when comparing with timezone-aware datetimes.
- call_end_time=datetime.now(),
+ call_end_time=datetime.now(timezone.utc),📝 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.
| call_end_time=datetime.now(), | |
| call_end_time=datetime.now(timezone.utc), |
🤖 Prompt for AI Agents
In app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py around
line 195, replace the naive datetime.now() with a timezone-aware call
datetime.now(timezone.utc) so the timestamp is consistent with other
timezone-aware datetimes in the file; ensure timezone is imported (it already is
on line 8) and update any tests or comparisons that assume naive datetimes
accordingly.
| ) | ||
| logger.info(f"Successfully updated database for call {context.call_sid}") | ||
| else: | ||
| logger.warning(f"No call_sid found, skipping database update") |
There was a problem hiding this comment.
Remove extraneous f prefix from string without placeholders.
Static analysis correctly flagged this as an f-string without placeholders.
- logger.warning(f"No call_sid found, skipping database update")
+ logger.warning("No call_sid found, skipping database update")📝 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.
| logger.warning(f"No call_sid found, skipping database update") | |
| logger.warning("No call_sid found, skipping database update") |
🧰 Tools
🪛 Ruff (0.14.8)
200-200: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py around
line 200, the logger.warning call uses an unnecessary f-string prefix for a
literal without placeholders; remove the leading "f" so the message is a normal
string literal (e.g., replace f"... " with "..." ) to satisfy static analysis
and avoid misleading f-string usage.
| meta_data = { | ||
| "transcription": { | ||
| "messages": transcription, | ||
| "call_sid": self.call_sid, | ||
| }, | ||
| "updated_address": self.updated_address, | ||
| "cancellation_reason": self.cancellation_reason, | ||
| } | ||
| await self.completion_function( | ||
| call_id=self.call_sid, | ||
| outcome=call_outcome, | ||
| call_end_time=datetime.now(), | ||
| updated_address=self.updated_address, | ||
| cancellation_reason=self.cancellation_reason, | ||
| meta_data=meta_data, | ||
| ) |
There was a problem hiding this comment.
Use timezone-aware datetime for call_end_time.
Line 647 uses datetime.now() without timezone, but the rest of this file uses datetime.now(timezone.utc) (e.g., lines 109, 513-517). This inconsistency could cause issues when comparing or storing timestamps.
await self.completion_function(
call_id=self.call_sid,
outcome=call_outcome,
- call_end_time=datetime.now(),
+ call_end_time=datetime.now(timezone.utc),
meta_data=meta_data,
)📝 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.
| meta_data = { | |
| "transcription": { | |
| "messages": transcription, | |
| "call_sid": self.call_sid, | |
| }, | |
| "updated_address": self.updated_address, | |
| "cancellation_reason": self.cancellation_reason, | |
| } | |
| await self.completion_function( | |
| call_id=self.call_sid, | |
| outcome=call_outcome, | |
| call_end_time=datetime.now(), | |
| updated_address=self.updated_address, | |
| cancellation_reason=self.cancellation_reason, | |
| meta_data=meta_data, | |
| ) | |
| meta_data = { | |
| "transcription": { | |
| "messages": transcription, | |
| "call_sid": self.call_sid, | |
| }, | |
| "updated_address": self.updated_address, | |
| "cancellation_reason": self.cancellation_reason, | |
| } | |
| await self.completion_function( | |
| call_id=self.call_sid, | |
| outcome=call_outcome, | |
| call_end_time=datetime.now(timezone.utc), | |
| meta_data=meta_data, | |
| ) |
🤖 Prompt for AI Agents
In app/ai/voice/agents/breeze_buddy/websocket_bot.py around lines 636 to 649,
the call_end_time is set using naive datetime.now(), which is inconsistent with
the file's use of timezone-aware datetimes; change datetime.now() to
datetime.now(timezone.utc) (and add/import timezone from datetime at the top of
the file if not already present) so the timestamp is timezone-aware and
consistent with other timestamps in this module.
| # Handle immediate node transition | ||
| if transition_to: | ||
| logger.info( | ||
| f"Transitioning from current node to '{transition_to}' for function '{function_name}'" | ||
| ) | ||
| next_node = context.create_node_from_template(transition_to) | ||
| return {}, next_node | ||
| else: | ||
| logger.debug(f"No transition specified for function '{function_name}'") | ||
|
|
||
| return {} |
There was a problem hiding this comment.
Return type is inconsistent between branches.
When transition_to is specified, the function returns ({}, next_node) (a tuple), but when not specified, it returns {} (a dict). This inconsistency could cause issues for callers expecting a consistent return type.
Consider returning a consistent type:
# Handle immediate node transition
if transition_to:
logger.info(
f"Transitioning from current node to '{transition_to}' for function '{function_name}'"
)
next_node = context.create_node_from_template(transition_to)
return {}, next_node
else:
logger.debug(f"No transition specified for function '{function_name}'")
+ return {}, None
- return {}📝 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.
| # Handle immediate node transition | |
| if transition_to: | |
| logger.info( | |
| f"Transitioning from current node to '{transition_to}' for function '{function_name}'" | |
| ) | |
| next_node = context.create_node_from_template(transition_to) | |
| return {}, next_node | |
| else: | |
| logger.debug(f"No transition specified for function '{function_name}'") | |
| return {} | |
| # Handle immediate node transition | |
| if transition_to: | |
| logger.info( | |
| f"Transitioning from current node to '{transition_to}' for function '{function_name}'" | |
| ) | |
| next_node = context.create_node_from_template(transition_to) | |
| return {}, next_node | |
| else: | |
| logger.debug(f"No transition specified for function '{function_name}'") | |
| return {}, None |
🤖 Prompt for AI Agents
In app/ai/voice/agents/breeze_buddy/workflow/transition.py around lines 59 to
69, the function returns a tuple ({}, next_node) when transition_to is set but a
plain dict {} otherwise; make the return type consistent by always returning a
tuple (dict, next_node_or_none) — e.g., set next_node = None in the
no-transition branch and return ({}, next_node) in both paths so callers always
receive the same (dict, Optional[node]) shape.
app/api/routers/breeze_buddy.py
Outdated
| @router.post("/workflow") | ||
| async def create_workflow_from_json( | ||
| workflow_data: CreateTemplateRequest, | ||
| current_user: TokenData = Depends(get_current_user), | ||
| ): | ||
| """ | ||
| Creates a complete workflow from a JSON object using the new simplified architecture. | ||
| Requires JWT authentication. | ||
| """ | ||
| logger.info(f"User {current_user.user_id} is creating/updating a workflow.") | ||
|
|
||
| try: | ||
| # Validate that flow structure contains required fields | ||
| flow = workflow_data.flow | ||
| if not flow: | ||
| raise ValueError("Flow structure is required") | ||
|
|
||
| if "initial_node" not in flow: | ||
| raise ValueError("initial_node must be specified in flow structure") | ||
|
|
||
| if "nodes" not in flow or not flow["nodes"]: | ||
| raise ValueError("nodes must be specified in flow structure") | ||
|
|
||
| # Create the template with flow stored as JSON | ||
| now = datetime.now(timezone.utc) | ||
| template = await create_template( | ||
| template_id=str(uuid4()), | ||
| merchant_id=workflow_data.merchant_id, | ||
| shop_identifier=workflow_data.shop_identifier, | ||
| name=workflow_data.workflow.value, | ||
| flow=flow, | ||
| is_active=workflow_data.is_active, | ||
| now=now, | ||
| ) | ||
|
|
||
| if not template: | ||
| raise Exception("Failed to create template") | ||
|
|
||
| logger.info( | ||
| f"Successfully created template with id: {template.id} containing flow with {len(flow.get('nodes', []))} nodes" | ||
| ) | ||
|
|
||
| return JSONResponse( | ||
| status_code=201, | ||
| content={ | ||
| "status": "success", | ||
| "template_id": template.id, | ||
| "message": f"Template '{workflow_data.workflow.value}' created successfully with {len(flow.get('nodes', []))} nodes", | ||
| }, | ||
| ) | ||
| except Exception as e: | ||
| logger.error(f"Error creating workflow from JSON: {e}", exc_info=True) | ||
| return JSONResponse(status_code=400, content={"detail": str(e)}) |
There was a problem hiding this comment.
New workflow creation endpoint needs refinement.
Several concerns with this endpoint:
-
Line 104: Using bare
Exceptionis not recommended. Consider usingHTTPExceptiondirectly or a custom exception. -
Duplicate templates: No check for existing templates with the same
merchant_id+workflowcombination. This could lead to duplicate templates. -
Authorization: The endpoint only checks authentication but not authorization - any authenticated user can create templates for any merchant.
Consider these improvements:
+ # Check for existing template
+ from app.database.accessor.breeze_buddy.template import get_template_by_merchant
+ existing = await get_template_by_merchant(
+ workflow_data.merchant_id,
+ workflow_data.shop_identifier,
+ workflow_data.workflow.value
+ )
+ if existing:
+ raise HTTPException(
+ status_code=409,
+ detail=f"Template already exists for this merchant and workflow"
+ )
+
# Create the template with flow stored as JSON
now = datetime.now(timezone.utc)
template = await create_template(For the exception at line 104:
- raise Exception("Failed to create template")
+ raise HTTPException(status_code=500, detail="Failed to create template")🧰 Tools
🪛 Ruff (0.14.8)
71-71: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
83-83: Abstract raise to an inner function
(TRY301)
83-83: Avoid specifying long messages outside the exception class
(TRY003)
86-86: Abstract raise to an inner function
(TRY301)
86-86: Avoid specifying long messages outside the exception class
(TRY003)
89-89: Abstract raise to an inner function
(TRY301)
89-89: Avoid specifying long messages outside the exception class
(TRY003)
104-104: Abstract raise to an inner function
(TRY301)
104-104: Create your own exception
(TRY002)
104-104: Avoid specifying long messages outside the exception class
(TRY003)
118-118: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In app/api/routers/breeze_buddy.py around lines 68-120, the
create_workflow_from_json endpoint uses a bare Exception at line 104, lacks a
uniqueness check for merchant_id+workflow (allowing duplicate templates), and
enforces authentication but not authorization. Replace the bare Exception with
raising an appropriate HTTPException (e.g., HTTPException(status_code=500,
detail="...") for server errors or raise HTTPException/RequestValidationError
with precise status codes), add a pre-create query to check for an existing
template with the same merchant_id and workflow name and return
HTTPException(status_code=409, detail="Template already exists") if found, and
enforce authorization by verifying current_user is allowed to act for the
provided merchant_id (e.g., compare current_user merchant list/role or require
merchant ownership) and return HTTPException(status_code=403) when unauthorized;
ensure error responses use HTTPException instead of generic exceptions and log
context as needed.
| CREATE TABLE IF NOT EXISTS outbound_number ( | ||
| id VARCHAR(255) PRIMARY KEY, | ||
| number VARCHAR(20) NOT NULL UNIQUE, | ||
| provider VARCHAR(50) CHECK (provider IN ('TWILIO', 'EXOTEL')) NOT NULL, | ||
| status VARCHAR(50) CHECK (status IN ('AVAILABLE', 'IN_USE', 'DISABLED')) NOT NULL, | ||
| channels INTEGER, | ||
| maximum_channels INTEGER, | ||
| created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL, | ||
| updated_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL | ||
| ); |
There was a problem hiding this comment.
Make channels and maximum_channels non-null with sane defaults
Runtime code in app/ai/voice/agents/breeze_buddy/managers/calls.py assumes channels and maximum_channels are integers (it compares them and increments channels). Allowing NULLs here can cause TypeError when comparing or adding.
Consider tightening the schema to something like:
channels INTEGER NOT NULL DEFAULT 0,
maximum_channels INTEGER NOT NULL,(and optionally CHECK (channels >= 0 AND maximum_channels > 0)).
This will prevent bad rows from breaking call processing at runtime.
[scratchpad_start]
[done]
File 1 reviewed.
[scratchpad_end] -->
🤖 Prompt for AI Agents
In app/database/migrations/001_initial_tables.sql around lines 5 to 14, the
columns channels and maximum_channels are nullable which can cause TypeError in
runtime code that treats them as integers; update the schema to make channels
INTEGER NOT NULL DEFAULT 0 and maximum_channels INTEGER NOT NULL (no NULLs), and
add optional CHECK constraints such as CHECK (channels >= 0 AND maximum_channels
> 0) to enforce sensible ranges so code can safely compare and increment these
fields.
badri-singhal
force-pushed
the
template-based-architechture
branch
from
0ce7896 to
d86db6d
Compare
| "mute_stt_handler", | ||
| "unmute_stt_handler", | ||
| "play_audio_sound_handler", | ||
| "end_conversation_handler", |
There was a problem hiding this comment.
. handler specification again is not req
| ) | ||
|
|
||
|
|
||
| async def play_audio_sound_handler(context: WorkflowContext, args, transition_to=None): |
There was a problem hiding this comment.
take sound name to be played
| end_conversation_handler, | ||
| mute_stt_handler, | ||
| play_audio_sound_handler, | ||
| unmute_stt_handler, |
| return {} | ||
|
|
||
|
|
||
| async def end_conversation_handler(context: WorkflowContext, args, transition_to=None): |
There was a problem hiding this comment.
see how pipecat officially handles things inside end-conversation
| ) | ||
| context.bot.lead = await context.bot.completion_function( | ||
| call_id=context.call_sid, | ||
| outcome=context.lead.outcome, |
There was a problem hiding this comment.
outcome should be string in db for leads and enum in code - template based
| }, | ||
| body: JSON.stringify({ username, password }), | ||
| }); | ||
|
|
There was a problem hiding this comment.
merchant ID should come in body, nothing should come in path
| if (response.ok) { | ||
| window.location.href = | ||
| "/agent/voice/breeze-buddy/breeze/order-confirmation/dashboard"; | ||
| "/agent/voice/breeze-buddy/dashboard"; |
There was a problem hiding this comment.
data_payload idk wtf you decide
There was a problem hiding this comment.
remove shop_name - it should ideally it should come from template / call exec config
There was a problem hiding this comment.
expected payload in template
There was a problem hiding this comment.
examples/templates/order-confirmation.json
badri-singhal
force-pushed
the
template-based-architechture
branch
2 times, most recently
from
2b35c97 to
7f78bf4
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/database/accessor/breeze_buddy/call_execution_config.py (1)
32-63:create_call_execution_configshould likely acceptshop_identifier: Optional[str] = None(generic config support).Right now reads explicitly fall back to
shop_identifier IS NULL, but create forces a non-null string, which can prevent creating the generic row.async def create_call_execution_config( id: str, initial_offset: int, retry_offset: int, call_start_time: time, call_end_time: time, max_retry: int, calling_provider: CallProvider, merchant_id: str, template: str, - shop_identifier: str, + shop_identifier: Optional[str] = None, enable_international_call: bool, ) -> Optional[CallExecutionConfig]:app/api/routers/breeze_buddy.py (2)
610-657: Inconsistent terminology:workflowvstemplate.Line 633 passes
config.workflowas thetemplateparameter tocreate_call_execution_config. While functionally correct, this inconsistency suggests theCreateCallExecutionConfigRequestschema still uses the oldworkflowfield name instead oftemplate.Consider updating the request schema to use consistent template-based terminology:
# In app/schemas.py (CreateCallExecutionConfigRequest) class CreateCallExecutionConfigRequest(BaseModel): merchant_id: str - workflow: str + template: str shop_identifier: Optional[str] = None # ... other fieldsThen update line 633:
- template=config.workflow, + template=config.template,Apply the same change to
UpdateCallExecutionConfigRequestat lines 676, 689, 694.
574-607: Remove unusedtemplateparameter or implement its intended purpose.The
templateparameter is captured from the WebSocket path (line 574) and logged (line 583), but it's never used in the handler logic and is not passed toprovider.handle_websocket()(line 589). The method signature forhandle_websocketonly accepts(websocket: WebSocket, provider: CallProvider), so the template cannot be passed there.Either remove
templatefrom the route path and function signature, or implement the intended use case (e.g., fetching template-specific configuration, validating the template, or passing it through the provider chain).
♻️ Duplicate comments (4)
app/ai/voice/agents/breeze_buddy/websocket_bot.py (1)
624-637: Use timezone-aware datetime and approve meta_data refactor.The meta_data structure consolidates transcription, updated_address, and cancellation_reason into a cleaner payload format. However, Line 635 uses
datetime.now()without timezone, inconsistent with the rest of the file that usesdatetime.now(timezone.utc).Apply this diff to use timezone-aware datetime:
if self.call_sid: meta_data = { "transcription": { "messages": transcription, "call_sid": self.call_sid, }, "updated_address": self.updated_address, "cancellation_reason": self.cancellation_reason, } await self.completion_function( call_id=self.call_sid, outcome=call_outcome, - call_end_time=datetime.now(), + call_end_time=datetime.now(timezone.utc), meta_data=meta_data, )app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py (1)
120-217: Robustness gaps in finalization: guardlead/metaData/taskand use UTC timestamp.
context.lead/context.lead.metaDatacan be None →context.lead.metaData["transcription"] = ...can crash.await context.task.queue_frame(EndFrame())infinallycan crash iftaskis missing/ended.- Line 197 uses
datetime.now()(naive); this matches earlier review feedback (timezone-aware UTC recommended).app/ai/voice/agents/breeze_buddy/agent.py (1)
128-153: Incorrect file path - missingai/directory component.The module is located at
app/ai/voice/agents/breeze_buddy/but the hardcoded path referencesapp/ai/voice/agents/breeze_buddy/which appears correct now. However, consider usingpathlib.Pathrelative to the module location for more robust path handling across different working directories.+from pathlib import Path +# At module level: +STATIC_AUDIO_DIR = Path(__file__).parent / "static" / "audio" # In the method: - wav_file_path = ( - "app/ai/voice/agents/breeze_buddy/static/audio/dial-tone.wav" - ) + wav_file_path = str(STATIC_AUDIO_DIR / "dial-tone.wav")app/api/routers/breeze_buddy.py (1)
73-141: Enforce merchant-level authorization in template creation.While the duplicate template check has been implemented (lines 96-106), the endpoint still lacks authorization enforcement. Any authenticated user can create templates for any merchant by providing an arbitrary
merchantvalue in the request payload.Consider adding merchant ownership or permission verification before template creation:
logger.info(f"User {current_user.user_id} is creating/updating a template.") try: + # Verify user has permission for this merchant + # Example: check if current_user.merchant_id == template_data.merchant + # or validate against a user-merchant permission table + if not await user_has_merchant_permission(current_user.user_id, template_data.merchant): + raise HTTPException( + status_code=403, + detail=f"User does not have permission to create templates for merchant: {template_data.merchant}" + ) + # Validate that flow structure contains required fields flow = template_data.flowBased on past review comments, this authorization gap was previously flagged. Do you need help implementing the merchant permission verification logic?
🧹 Nitpick comments (11)
app/database/decoder/breeze_buddy/template.py (1)
22-35: Consider guardingjson.loadsto avoid hard-failing on bad DB content.If
flow/expected_payload_schemacan ever be malformed, wrapjson.loadswithtry/except json.JSONDecodeErrorand fall back to{}/None(and log at warning/debug).app/database/accessor/breeze_buddy/template.py (2)
29-53: Fix implicit Optional params + keep traceback on failures (logger.exception).-from typing import Optional +from datetime import datetime +from typing import Optional @@ async def get_template_by_merchant( - merchant_id: str, shop_identifier: str = None, name: str = None + merchant_id: str, shop_identifier: Optional[str] = None, name: Optional[str] = None ) -> Optional[TemplateModel]: @@ - except Exception as e: - logger.error(f"Error getting template by merchant: {e}") + except Exception: + logger.exception("Error getting template by merchant") return None
55-98: Typenow(likelydatetime) and preferlogger.exceptionover stringifying the exception.async def create_template( @@ - now, + now: datetime, ) -> Optional[TemplateModel]: @@ - except Exception as e: - logger.error(f"Error creating template: {e}") + except Exception: + logger.exception("Error creating template") return Noneapp/ai/voice/agents/breeze_buddy/template/loader.py (2)
18-55: Consider not logging high-cardinality identifiers at INFO (merchant_id), or redact/hash.If
merchant_idis sensitive / high-volume, switching to DEBUG or redaction reduces noisy logs and privacy risk.
56-85: Template rendering via.replace()is brittle; consider a safer formatter (and types).A minimal improvement is
str.format_mapwith a “safe dict” (leave unknown placeholders untouched), or a small templating helper, plus tightening types tolist[dict[str, Any]]/Mapping[str, str]so callers know what shapes are supported.app/database/queries/breeze_buddy/template.py (2)
11-34: Modernize Optional typing (str | None) to satisfy Ruff / clarity.Ruff’s RUF013 is reasonable here; it also makes call sites clearer.
36-64: Add concrete types forcreated_at/updated_at(and ensure they match DB column types).Today they’re untyped; aligning to
datetime(or whatever the DB layer expects) reduces accidental misuse.app/database/queries/breeze_buddy/call_execution_config.py (2)
15-68: Timezone consistency: prefer UTC-aware timestamps if DB columns aretimestamptz.If
created_at/updated_atare stored as timezone-aware, usedatetime.now(timezone.utc)(and importtimezone) to avoid mixing naive/aware datetimes.
95-178: Ensure there’s an index supporting the new(merchant_id, template, shop_identifier)lookup.The update now predicates on merchant_id + template (+ shop_identifier / IS NULL). Without an index, this can become a hot-path scan as data grows.
app/ai/voice/agents/breeze_buddy/template/builder.py (1)
259-276: Missing handler forActionType.END_CONVERSATIONin_build_action.The
ActionTypeenum includesEND_CONVERSATION, but_build_actiononly handlesTTS_SAYandFUNCTIONexplicitly. WhenEND_CONVERSATIONis passed, it falls through to the else branch returning{"type": str(action_type)}which loses the enum value format.If
END_CONVERSATIONactions should invoke theend_conversationhandler from the handler_map, consider adding explicit handling:elif action_type == ActionType.FUNCTION: handler = self.handler_map.get(action.handler) if not handler: logger.error(f"Handler not found for name: {action.handler}") raise ValueError(f"Handler not found for name: {action.handler}") logger.debug( f"Successfully built FUNCTION action for handler: {action.handler}" ) return {"type": "function", "handler": handler} + elif action_type == ActionType.END_CONVERSATION: + handler = self.handler_map.get("end_conversation") + if not handler: + logger.error("end_conversation handler not found in handler_map") + raise ValueError("end_conversation handler not found in handler_map") + logger.debug("Successfully built END_CONVERSATION action") + return {"type": "function", "handler": handler} else: logger.warning(f"Unknown action type: {action_type}, returning as string") return {"type": str(action_type)}app/ai/voice/agents/breeze_buddy/template/hooks.py (1)
264-264: Class variable_hooksshould be annotated withClassVar.The mutable class attribute
_hooksshould usetyping.ClassVarannotation to indicate it's shared across all instances (though this is a classmethod-only registry).+from typing import Any, ClassVar, Dict, Optional -from typing import Any, Dict, Optional class HookRegistry: """ Registry for all available hooks. This class maintains a mapping of hook names to hook instances. """ - _hooks: Dict[str, Hook] = {} + _hooks: ClassVar[Dict[str, Hook]] = {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
app/ai/voice/agents/breeze_buddy/agent.py(1 hunks)app/ai/voice/agents/breeze_buddy/dashboard/login.html(1 hunks)app/ai/voice/agents/breeze_buddy/examples/templates/order-confirmation.json(1 hunks)app/ai/voice/agents/breeze_buddy/handlers/internal/__init__.py(1 hunks)app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py(1 hunks)app/ai/voice/agents/breeze_buddy/managers/calls.py(6 hunks)app/ai/voice/agents/breeze_buddy/services/telephony/exotel/exotel.py(1 hunks)app/ai/voice/agents/breeze_buddy/services/telephony/twilio/twilio.py(1 hunks)app/ai/voice/agents/breeze_buddy/template/__init__.py(1 hunks)app/ai/voice/agents/breeze_buddy/template/builder.py(1 hunks)app/ai/voice/agents/breeze_buddy/template/context.py(1 hunks)app/ai/voice/agents/breeze_buddy/template/hooks.py(1 hunks)app/ai/voice/agents/breeze_buddy/template/loader.py(1 hunks)app/ai/voice/agents/breeze_buddy/template/transition.py(1 hunks)app/ai/voice/agents/breeze_buddy/template/types.py(1 hunks)app/ai/voice/agents/breeze_buddy/types/models.py(2 hunks)app/ai/voice/agents/breeze_buddy/utils/common.py(1 hunks)app/ai/voice/agents/breeze_buddy/websocket_bot.py(3 hunks)app/ai/voice/agents/breeze_buddy/workflows/order_confirmation/utils.py(0 hunks)app/api/routers/breeze_buddy.py(10 hunks)app/database/accessor/__init__.py(1 hunks)app/database/accessor/breeze_buddy/call_execution_config.py(6 hunks)app/database/accessor/breeze_buddy/lead_call_tracker.py(3 hunks)app/database/accessor/breeze_buddy/template.py(1 hunks)app/database/decoder/breeze_buddy/call_execution_config.py(3 hunks)app/database/decoder/breeze_buddy/lead_call_tracker.py(1 hunks)app/database/decoder/breeze_buddy/template.py(1 hunks)app/database/migrations/001_initial_tables.sql(1 hunks)app/database/migrations/002_add_shop_identifier_and_locks.sql(1 hunks)app/database/migrations/003_create_template_table.sql(1 hunks)app/database/queries/breeze_buddy/call_execution_config.py(7 hunks)app/database/queries/breeze_buddy/lead_call_tracker.py(5 hunks)app/database/queries/breeze_buddy/template.py(1 hunks)app/schemas.py(3 hunks)scripts/create_tables.py(0 hunks)
💤 Files with no reviewable changes (2)
- scripts/create_tables.py
- app/ai/voice/agents/breeze_buddy/workflows/order_confirmation/utils.py
🚧 Files skipped from review as they are similar to previous changes (5)
- app/database/migrations/002_add_shop_identifier_and_locks.sql
- app/ai/voice/agents/breeze_buddy/services/telephony/twilio/twilio.py
- app/database/accessor/init.py
- app/database/migrations/003_create_template_table.sql
- app/ai/voice/agents/breeze_buddy/services/telephony/exotel/exotel.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: badri-singhal
Repo: juspay/clairvoyance PR: 313
File: app/api/routers/breeze_buddy.py:487-489
Timestamp: 2025-10-15T04:10:50.071Z
Learning: In the Breeze Buddy dashboard (`app/agents/voice/breeze_buddy/workflows/order_confirmation/dashboard.html`), analytics metrics are dynamically rendered using a for loop in the HTML script. When adding new metrics to the JSON response in the `get_analytics()` endpoint, no manual HTML template updates are required as the loop handles all metrics automatically.
📚 Learning: 2025-10-15T04:10:50.071Z
Learnt from: badri-singhal
Repo: juspay/clairvoyance PR: 313
File: app/api/routers/breeze_buddy.py:487-489
Timestamp: 2025-10-15T04:10:50.071Z
Learning: In the Breeze Buddy dashboard (`app/agents/voice/breeze_buddy/workflows/order_confirmation/dashboard.html`), analytics metrics are dynamically rendered using a for loop in the HTML script. When adding new metrics to the JSON response in the `get_analytics()` endpoint, no manual HTML template updates are required as the loop handles all metrics automatically.
Applied to files:
app/ai/voice/agents/breeze_buddy/examples/templates/order-confirmation.jsonapp/ai/voice/agents/breeze_buddy/dashboard/login.htmlapp/api/routers/breeze_buddy.py
📚 Learning: 2025-12-10T11:55:16.753Z
Learnt from: narsimhaReddyJuspay
Repo: juspay/clairvoyance PR: 414
File: app/services/langfuse/tasks/score_monitor/score.py:469-489
Timestamp: 2025-12-10T11:55:16.753Z
Learning: In the Breeze Buddy call tracking system, a lead can only have one outcome type among CONFIRM, CANCEL, or ADDRESS_UPDATED—these outcomes are mutually exclusive per lead.
Applied to files:
app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py
🧬 Code graph analysis (14)
app/database/accessor/breeze_buddy/call_execution_config.py (2)
app/schemas.py (2)
CallExecutionConfig(103-116)CallProvider(16-18)app/database/queries/breeze_buddy/call_execution_config.py (1)
update_call_execution_config_query(95-178)
app/database/decoder/breeze_buddy/template.py (1)
app/ai/voice/agents/breeze_buddy/template/types.py (1)
TemplateModel(60-68)
app/ai/voice/agents/breeze_buddy/template/context.py (1)
app/ai/voice/agents/breeze_buddy/template/hooks.py (1)
get(278-288)
app/database/decoder/breeze_buddy/call_execution_config.py (1)
app/schemas.py (2)
CallExecutionConfig(103-116)CallProvider(16-18)
app/ai/voice/agents/breeze_buddy/template/loader.py (3)
app/ai/voice/agents/breeze_buddy/template/types.py (1)
TemplateModel(60-68)app/database/accessor/breeze_buddy/template.py (1)
get_template_by_merchant(29-52)app/ai/voice/agents/breeze_buddy/template/hooks.py (1)
get(278-288)
app/ai/voice/agents/breeze_buddy/template/builder.py (4)
app/ai/voice/agents/breeze_buddy/template/transition.py (1)
transition_handler(19-69)app/ai/voice/agents/breeze_buddy/template/types.py (5)
ActionType(11-14)FlowAction(17-20)FlowFunction(42-48)FlowNodeModel(51-57)TemplateModel(60-68)app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py (4)
mute_stt(23-44)unmute_stt(47-68)play_audio_sound(71-117)end_conversation(120-217)app/ai/voice/agents/breeze_buddy/template/hooks.py (1)
get(278-288)
app/ai/voice/agents/breeze_buddy/utils/common.py (2)
app/core/security/sha.py (1)
calculate_hmac_sha256(6-27)app/schemas.py (1)
LeadCallOutcome(28-34)
app/ai/voice/agents/breeze_buddy/websocket_bot.py (2)
app/ai/voice/agents/breeze_buddy/types/models.py (1)
OrderData(11-12)app/ai/voice/agents/breeze_buddy/template/context.py (1)
call_sid(67-69)
app/ai/voice/agents/breeze_buddy/template/hooks.py (3)
app/ai/voice/agents/breeze_buddy/template/context.py (3)
context(57-59)TemplateContext(14-132)lead(62-64)app/database/accessor/breeze_buddy/lead_call_tracker.py (1)
update_lead_call_completion_details(281-311)app/schemas.py (1)
LeadCallOutcome(28-34)
app/ai/voice/agents/breeze_buddy/types/models.py (1)
app/ai/voice/agents/breeze_buddy/template/context.py (1)
reporting_webhook_url(77-79)
app/database/accessor/breeze_buddy/lead_call_tracker.py (1)
app/schemas.py (3)
LeadCallStatus(21-25)LeadCallOutcome(28-34)LeadCallTracker(37-56)
app/api/routers/breeze_buddy.py (6)
app/ai/voice/agents/breeze_buddy/template/types.py (1)
CreateTemplateRequest(92-101)app/ai/voice/agents/breeze_buddy/types/models.py (3)
LeadData(23-33)LoginRequest(36-38)PushLeadRequest(15-20)app/ai/voice/agents/breeze_buddy/utils/common.py (2)
get_validation_error_message(321-338)validate_payload(152-212)app/database/accessor/breeze_buddy/template.py (2)
get_template_by_merchant(29-52)create_template(55-98)app/database/accessor/breeze_buddy/call_execution_config.py (2)
update_call_execution_config(148-195)get_call_execution_config_by_merchant_id(79-122)app/database/accessor/breeze_buddy/lead_call_tracker.py (1)
create_lead_call_tracker(43-87)
app/ai/voice/agents/breeze_buddy/managers/calls.py (2)
app/schemas.py (2)
LeadCallOutcome(28-34)LeadCallTracker(37-56)app/database/accessor/breeze_buddy/lead_call_tracker.py (1)
update_lead_call_completion_details(281-311)
app/database/queries/breeze_buddy/lead_call_tracker.py (1)
app/schemas.py (2)
LeadCallOutcome(28-34)LeadCallStatus(21-25)
🪛 Ruff (0.14.8)
app/ai/voice/agents/breeze_buddy/template/transition.py
53-55: Store a reference to the return value of asyncio.create_task
(RUF006)
app/ai/voice/agents/breeze_buddy/template/__init__.py
27-36: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
app/ai/voice/agents/breeze_buddy/template/loader.py
115-117: Avoid specifying long messages outside the exception class
(TRY003)
app/ai/voice/agents/breeze_buddy/template/builder.py
64-64: Avoid specifying long messages outside the exception class
(TRY003)
69-69: Avoid specifying long messages outside the exception class
(TRY003)
76-76: Avoid specifying long messages outside the exception class
(TRY003)
102-102: Avoid specifying long messages outside the exception class
(TRY003)
200-200: Avoid specifying long messages outside the exception class
(TRY003)
269-269: Avoid specifying long messages outside the exception class
(TRY003)
app/ai/voice/agents/breeze_buddy/utils/common.py
102-102: Do not catch blind exception: Exception
(BLE001)
143-143: Consider moving this statement to an else block
(TRY300)
144-144: Do not catch blind exception: Exception
(BLE001)
305-305: Unused lambda argument: v
(ARG005)
app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py
23-23: Unused function argument: args
(ARG001)
23-23: Unused function argument: transition_to
(ARG001)
47-47: Unused function argument: args
(ARG001)
47-47: Unused function argument: transition_to
(ARG001)
71-71: Unused function argument: transition_to
(ARG001)
107-107: Do not catch blind exception: Exception
(BLE001)
120-120: Unused function argument: args
(ARG001)
120-120: Unused function argument: transition_to
(ARG001)
204-204: Do not catch blind exception: Exception
(BLE001)
206-206: Use explicit conversion flag
Replace with conversion flag
(RUF010)
220-220: Unused function argument: transition_to
(ARG001)
314-314: Do not catch blind exception: Exception
(BLE001)
326-326: Do not catch blind exception: Exception
(BLE001)
app/ai/voice/agents/breeze_buddy/agent.py
118-118: Do not catch blind exception: Exception
(BLE001)
152-152: Do not catch blind exception: Exception
(BLE001)
202-202: Do not catch blind exception: Exception
(BLE001)
215-215: Do not catch blind exception: Exception
(BLE001)
334-334: Unused function argument: transport
(ARG001)
361-361: Unused function argument: transport
(ARG001)
366-366: Unused function argument: task
(ARG001)
app/database/queries/breeze_buddy/template.py
12-12: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
12-12: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
26-31: Possible SQL injection vector through string-based query construction
(S608)
48-52: Possible SQL injection vector through string-based query construction
(S608)
app/ai/voice/agents/breeze_buddy/template/hooks.py
80-80: Do not catch blind exception: Exception
(BLE001)
82-82: Use explicit conversion flag
Replace with conversion flag
(RUF010)
249-249: Do not catch blind exception: Exception
(BLE001)
264-264: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
app/database/accessor/breeze_buddy/template.py
30-30: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
30-30: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
48-48: Consider moving this statement to an else block
(TRY300)
50-50: Do not catch blind exception: Exception
(BLE001)
94-94: Consider moving this statement to an else block
(TRY300)
96-96: Do not catch blind exception: Exception
(BLE001)
app/api/routers/breeze_buddy.py
76-76: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
88-88: Abstract raise to an inner function
(TRY301)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
91-91: Abstract raise to an inner function
(TRY301)
91-91: Avoid specifying long messages outside the exception class
(TRY003)
94-94: Abstract raise to an inner function
(TRY301)
94-94: Avoid specifying long messages outside the exception class
(TRY003)
103-106: Abstract raise to an inner function
(TRY301)
122-122: Abstract raise to an inner function
(TRY301)
138-138: Do not catch blind exception: Exception
(BLE001)
365-365: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
456-456: Redefinition of unused trigger_order_confirmation from line 361
(F811)
458-458: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
566-566: Do not catch blind exception: Exception
(BLE001)
570-570: Use explicit conversion flag
Replace with conversion flag
(RUF010)
751-751: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
app/database/queries/breeze_buddy/lead_call_tracker.py
233-238: Possible SQL injection vector through string-based query construction
(S608)
app/ai/voice/agents/breeze_buddy/handlers/internal/__init__.py
15-21: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🔇 Additional comments (36)
app/ai/voice/agents/breeze_buddy/dashboard/login.html (1)
375-375: LGTM! Redirect path updated to template-based dashboard.The redirect URL has been correctly updated to align with the new template-based architecture, simplifying the path from the workflow-specific route to a unified dashboard route.
app/ai/voice/agents/breeze_buddy/types/models.py (2)
15-20: LGTM! New PushLeadRequest model aligns with template-based architecture.The model is well-structured with appropriate field types. The flexible
Dict[str, Any]payload allows dynamic template data, and the optionalreporting_webhook_urlsupports callback notifications.
23-33: LGTM! LeadData model captures all necessary customer and order information.The model includes all essential fields for order confirmation workflow, with appropriate optional fields for
shop_identifier,identity, andreporting_webhook_url.app/ai/voice/agents/breeze_buddy/examples/templates/order-confirmation.json (2)
1-23: LGTM! Template metadata and schema definition are well-structured.The template header clearly defines the merchant, template name, active status, description, and expected payload schema. The schema uses standard JSON Schema conventions for type validation.
24-498: Excellent template-based workflow design.The flow configuration is comprehensive and well-architected:
- Clear node structure with appropriate task messages and actions
- Proper function definitions with transitions and hooks
- Handles all major outcomes (confirmation, cancellation, address updates, busy user)
- Includes database update hooks for tracking outcomes
- Uses pre/post actions for STT muting and conversation management
This template-driven approach provides excellent flexibility for workflow customization without code changes.
app/ai/voice/agents/breeze_buddy/websocket_bot.py (2)
38-46: LGTM! Import paths updated to new module structure.The imports have been correctly updated to reference the new
types.modelsandutils.commonmodules, aligning with the template-based architecture refactor.
404-404: LGTM! Multilingual support added to system prompt.The instruction to reply in the user's language enhances the bot's ability to handle multilingual conversations naturally.
app/ai/voice/agents/breeze_buddy/managers/calls.py (3)
10-12: LGTM! Import updated to new utils module.The import has been correctly updated to reference the new centralized
utils.commonmodule.
57-60: LGTM! Consistent migration from workflow to template terminology.The field references have been correctly updated from
workflowtotemplate, aligning with the new template-based architecture throughout the codebase.Also applies to: 139-139
374-412: LGTM! Improved signature with optional parameters and return value.The signature changes improve flexibility:
- Optional
outcome,call_end_time, andmeta_dataparameters allow partial updates- Returning
Optional[LeadCallTracker]enables callers to access the updated lead state- The nullable approach handles cases where data may not be available
app/ai/voice/agents/breeze_buddy/utils/common.py (6)
14-21: LGTM! Outcome mapping dictionary is well-defined.The mapping correctly translates outcome strings to
LeadCallOutcomeenum values, supporting the outcome tracking system.
24-65: LGTM! Indian number formatting correctly implements the lakhs-crores system.The function properly handles the Indian numbering system with appropriate grouping (hundreds, thousands, lakhs, crores) and produces natural speech-friendly output.
68-110: LGTM! Webhook retry logic is robust with proper signing.The function includes:
- HMAC-SHA256 signature for security
- Configurable retry attempts with detailed logging
- Proper error handling and status checking
- No sleep after final attempt (efficient)
The broad exception catching at line 102 is appropriate here as any network or serialization error should trigger a retry.
113-149: LGTM! Audio loading handles errors gracefully.The function properly:
- Checks file existence before loading
- Converts to the correct format (8000 Hz, mono, 16-bit PCM)
- Returns None on failure with appropriate logging
The broad exception catching at line 144 is appropriate for handling various audio processing errors (codec issues, file corruption, etc.).
152-318: LGTM! Payload validation provides comprehensive schema checking.The validation functions support:
- Nested objects and arrays
- Recursive validation with path tracking
- Multiple type validators (string, number, boolean, array, object, null, any)
- Graceful handling of unknown types and null values
- Clear error messages with field paths
The
_validate_typefunction at line 305 includeslambda v: Truefor the "any" type validator, which intentionally accepts any value. The unusedvparameter is required by the lambda signature for consistency with other validators - this is not a real issue.
321-338: LGTM! Error message formatting provides clear user feedback.The function formats single or multiple validation errors into a readable message format.
app/database/decoder/breeze_buddy/call_execution_config.py (1)
21-38: Decoder update totemplatelooks consistent withCallExecutionConfig.Also applies to: 50-65
app/database/decoder/breeze_buddy/lead_call_tracker.py (1)
17-44: Decoder update totemplatelooks correct.app/ai/voice/agents/breeze_buddy/template/transition.py (1)
72-113: Hook execution flow looks clean and easy to follow.app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py (2)
23-69: mute/unmute look fine; just ensure VAD param mutation is safe for your pipeline concurrency model.
220-332: Outcome type mismatch corrupts webhook payload:context.lead.outcomeis an enum, butOUTCOME_TO_ENUMexpects string keys.
context.lead.outcomeis deserialized from the database as aLeadCallOutcomeenum object. However,OUTCOME_TO_ENUM.get(outcome, LeadCallOutcome.BUSY)expects lowercase string keys like"confirmed"and"cancelled". When outcome is already an enum (which it always is when loaded from the database), the lookup fails silently and defaults toLeadCallOutcome.BUSY, corrupting the webhook summary data with the wrong outcome regardless of the actual call result.Convert enum to string value before mapping:
outcome.value if isinstance(outcome, LeadCallOutcome) else outcome, or useoutcome.valuedirectly since the enum is guaranteed to exist when deserialized.⛔ Skipped due to learnings
Learnt from: narsimhaReddyJuspay Repo: juspay/clairvoyance PR: 414 File: app/services/langfuse/tasks/score_monitor/score.py:469-489 Timestamp: 2025-12-10T11:55:16.753Z Learning: In the Breeze Buddy call tracking system, a lead can only have one outcome type among CONFIRM, CANCEL, or ADDRESS_UPDATED—these outcomes are mutually exclusive per lead.app/ai/voice/agents/breeze_buddy/template/context.py (1)
101-133: The code is correct.flow_config["nodes"]is constructed as a dict keyed by node name in the builder (build_flow_configreturns{"initial_node": initial_node_name, "nodes": nodes}wherenodesis explicitly built asnodes = {}and populated withnodes[node.node_name] = self._build_node(node)). Thecreate_node_from_template()method properly treats it as a dict and all accesses are valid.app/ai/voice/agents/breeze_buddy/template/loader.py (1)
86-180: The code is functioning as designed without downstream compatibility risks. Whenload_template()converts messages to dicts and renders them, the returnedtemplate_objis immediately passed toflow_builder.build_flow_config(), which explicitly callsFlowNodeModel.model_validate()on each node. This Pydantic validation automatically reconstructsTaskMessageobjects from the rendered dicts since they contain the requiredroleandcontentfields. The dict-to-object conversion is seamless and does not break the builder or any downstream consumers.app/ai/voice/agents/breeze_buddy/template/builder.py (2)
43-116: LGTM on the flow config building logic.The validation and transformation logic is thorough - checking for empty flow, missing initial_node, empty nodes list, and verifying the initial node exists. The function_name to name transformation handles backward compatibility well.
223-233: Closure captures loop variable by reference - potential bug.The
wrapper_handlerclosure capturesfunc.transition_to,func.name, andhookswhich are correctly scoped. However, this pattern is safe here because each closure is created with its ownfuncfrom the list comprehension in_build_node. The implementation correctly captures the values at function schema creation time.app/ai/voice/agents/breeze_buddy/template/hooks.py (1)
26-85: LGTM on Hook base class design.The abstract base class with
safe_executewrapper provides good error isolation for async hook execution. The logging is appropriate for debugging hook lifecycle.app/ai/voice/agents/breeze_buddy/agent.py (4)
155-159: Good fix for potentialAttributeError.The Exotel handling now correctly uses
call_data.get("start", {})to avoid theAttributeErrorwhen "start" key is missing. This addresses the previous review comment.
179-182: LGTM on handler context wrapping.The pattern of wrapping all handlers with context before building the flow config ensures that each handler receives the bot context. This enables the handlers to access bot state like
lead,vad_analyzer, andtransport.
221-228: Missing fields from schema silently default to empty string.When a field from
expected_payload_schemais not found incall_payload, it's set to an empty string with only a warning log. This could cause subtle bugs if templates expect non-empty values.Consider whether this should be a validation error for required fields, or at minimum, ensure templates handle empty string values gracefully.
333-368: Unused function arguments in event handlers are intentional.The
transport,client, andtaskarguments in the event handlers are required by the Pipecat event handler signature but unused in the implementation. This is expected for event handler callbacks. The static analysis hints (ARG001) are false positives in this context.app/database/accessor/breeze_buddy/lead_call_tracker.py (2)
43-87: LGTM on template parameter rename.The change from
workflowtotemplateis consistent with the broader template-based architecture migration. The function signature and call toinsert_lead_call_tracker_queryare properly aligned.
281-311: LGTM on partial update support.The updated
update_lead_call_completion_detailsfunction now correctly supports partial updates by making all fields optional exceptid. The docstring clearly documents this behavior. This aligns with the hook system's need to update only specific fields (like outcome) without overwriting other fields.app/database/queries/breeze_buddy/lead_call_tracker.py (2)
17-71: LGTM on template parameter change in insert query.The column and parameter rename from
workflowtotemplateis consistent with the schema migration. The parameterized query pattern is preserved.
196-240: Dynamic query construction is safe - parameterized values.The static analysis S608 warning about SQL injection is a false positive. The dynamic SET clause construction:
- Uses hardcoded column names (not user input)
- Uses parameterized placeholders (
$1,$2, etc.) for all values- Values are passed separately to the query executor
The pattern correctly prevents SQL injection while enabling partial updates.
app/api/routers/breeze_buddy.py (1)
469-543: Well-implemented payload validation flow.The template-based validation logic is robust:
- Fetches template with proper error handling
- Validates payload against expected schema when present
- Provides detailed error messages via
get_validation_error_message- Falls back gracefully when no schema is defined
This implementation correctly leverages the new template-based architecture.
app/ai/voice/agents/breeze_buddy/template/types.py (1)
1-101: Well-structured Pydantic models for template-based architecture.The model definitions are clean and comprehensive:
- Clear separation between internal models (FlowNodeModel, TemplateModel) and API request models (RequestFlowNode, CreateTemplateRequest)
- Proper use of
Optionalfor nullable fields with sensible defaults- The enum-based
ActionTypeprovides type safety for flow actions- Hook system (HookConfig, HookFieldConfig) enables extensible action handling
- The schema change documented at line 100 (Dict[str, str] → Dict[str, Any]) correctly supports nested validation schemas
These models establish a solid foundation for the template-based workflow engine.
| initial_node_config = NodeConfig( | ||
| name=initial_node_config["name"], | ||
| task_messages=initial_node_config["task_messages"], | ||
| role_messages=initial_node_config.get("role_messages", []), | ||
| functions=initial_node_config["functions"], | ||
| pre_actions=initial_node_config["pre_actions"], | ||
| post_actions=initial_node_config["post_actions"], | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file structure and locate the relevant functions
cd app/ai/voice/agents/breeze_buddy && cat -n agent.py | head -50Repository: juspay/clairvoyance
Length of output: 182
🏁 Script executed:
# Find the build_flow_config function definition
rg -n "def build_flow_config" app/ai/voice/agents/breeze_buddy/Repository: juspay/clairvoyance
Length of output: 194
🏁 Script executed:
# Find the _build_node function definition
rg -n "def _build_node" app/ai/voice/agents/breeze_buddy/Repository: juspay/clairvoyance
Length of output: 181
🏁 Script executed:
# Search for NodeConfig class definition
rg -n "class NodeConfig" app/Repository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Get context around lines 345-352 to understand the full picture
sed -n '330,360p' app/ai/voice/agents/breeze_buddy/agent.pyRepository: juspay/clairvoyance
Length of output: 1491
🏁 Script executed:
# Get the full implementation of build_flow_config
sed -n '43,100p' app/ai/voice/agents/breeze_buddy/template/builder.pyRepository: juspay/clairvoyance
Length of output: 2294
🏁 Script executed:
# Also check what NodeConfig looks like and if it has dict-like behavior
rg -n "class NodeConfig" app/ -A 10Repository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Check imports in agent.py to find where NodeConfig comes from
head -50 app/ai/voice/agents/breeze_buddy/agent.py | grep -E "import|from"Repository: juspay/clairvoyance
Length of output: 1298
🏁 Script executed:
# Get the rest of build_flow_config to see how it constructs the return dict
sed -n '100,150p' app/ai/voice/agents/breeze_buddy/template/builder.pyRepository: juspay/clairvoyance
Length of output: 1899
🏁 Script executed:
# Search for NodeConfig definition in the entire codebase
rg "class NodeConfig"Repository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Search for how NodeConfig is used throughout the codebase to see if dict-like access is common
rg "NodeConfig\[" app/Repository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Search for NodeConfig instantiation or method calls to understand its interface
rg "NodeConfig\(" app/ -A 2Repository: juspay/clairvoyance
Length of output: 893
🏁 Script executed:
# Check if there are other places where nodes dict values are accessed like dicts
rg "nodes\[" app/ai/voice/agents/breeze_buddy/ -B 2 -A 2Repository: juspay/clairvoyance
Length of output: 942
🏁 Script executed:
# Get the full _build_node implementation including return statement
sed -n '118,180p' app/ai/voice/agents/breeze_buddy/template/builder.pyRepository: juspay/clairvoyance
Length of output: 2425
🏁 Script executed:
# Check if NodeConfig objects are ever converted to dicts anywhere
rg "\.model_dump\|\.dict\(" app/ai/voice/agents/breeze_buddy/template/builder.pyRepository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Check if there are any tests or other code that show how NodeConfig is typically used
rg "from pipecat_flows" app/ -A 5Repository: juspay/clairvoyance
Length of output: 1972
🏁 Script executed:
# Look for any comments or documentation about how flow_config is structured
rg "flow_config" app/ai/voice/agents/breeze_buddy/agent.py -B 2 -A 2Repository: juspay/clairvoyance
Length of output: 794
🏁 Script executed:
# Check the context_manager in template to see how it uses nodes
sed -n '1,50p' app/ai/voice/agents/breeze_buddy/template/context.pyRepository: juspay/clairvoyance
Length of output: 1243
🏁 Script executed:
# Check websocket_bot.py to see how it creates initial_node_config - does it also do reconstruction?
sed -n '1,100p' app/ai/voice/agents/breeze_buddy/websocket_bot.py | grep -A 30 "initial_node_config"Repository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Look for any error handling or comments around the problematic code
sed -n '340,360p' app/ai/voice/agents/breeze_buddy/agent.pyRepository: juspay/clairvoyance
Length of output: 1014
🏁 Script executed:
# Search for how flow_config structure is documented or used elsewhere
rg "flow_config\[" app/ai/voice/agents/breeze_buddy/ -B 1 -A 1Repository: juspay/clairvoyance
Length of output: 923
🏁 Script executed:
# Check how flow_manager.initialize is typed - what does it expect?
rg "def initialize" app/ -A 3Repository: juspay/clairvoyance
Length of output: 2060
🏁 Script executed:
# Verify the exact pattern by checking if there are other places where NodeConfig is used directly without reconstruction
rg "NodeConfig" app/ai/voice/agents/breeze_buddy/ -B 2 -A 2 | head -80Repository: juspay/clairvoyance
Length of output: 5969
🏁 Script executed:
# Check the template/context.py implementation to see how it uses the NodeConfig objects
sed -n '70,110p' app/ai/voice/agents/breeze_buddy/template/context.pyRepository: juspay/clairvoyance
Length of output: 1058
🏁 Script executed:
# Get the full implementation of create_node_from_template
sed -n '100,140p' app/ai/voice/agents/breeze_buddy/template/context.pyRepository: juspay/clairvoyance
Length of output: 1447
Remove redundant NodeConfig reconstruction.
The initial_node_config retrieved from self.flow_config["nodes"][initial_node_name] is already a NodeConfig object created by _build_node and stored in the flow config dictionary. The reconstruction at lines 345-352 is unnecessary—simply use the object directly:
initial_node_config = self.flow_config["nodes"][initial_node_name]
await self.flow_manager.initialize(initial_node_config)This is confirmed by TemplateContext.create_node_from_template() in template/context.py, which returns nodes[node_name] directly without reconstruction.
🤖 Prompt for AI Agents
In app/ai/voice/agents/breeze_buddy/agent.py around lines 345 to 352, the code
reconstructs a NodeConfig from initial_node_config dict but the value in
self.flow_config["nodes"][initial_node_name] is already a NodeConfig created by
_build_node; remove the redundant NodeConfig(...) reconstruction and instead
assign initial_node_config = self.flow_config["nodes"][initial_node_name] (or
simply reuse the existing variable) and pass that object to await
self.flow_manager.initialize(initial_node_config).
| __all__ = [ | ||
| "mute_stt", | ||
| "unmute_stt", | ||
| "play_audio_sound", | ||
| "end_conversation", | ||
| "service_callback", | ||
| ] |
There was a problem hiding this comment.
Ruff RUF022: sort __all__ (or add a targeted ignore).
If Ruff is enforced, this will fail lint. Suggest:
__all__ = [
- "mute_stt",
- "unmute_stt",
- "play_audio_sound",
"end_conversation",
+ "mute_stt",
+ "play_audio_sound",
"service_callback",
+ "unmute_stt",
]📝 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.
| __all__ = [ | |
| "mute_stt", | |
| "unmute_stt", | |
| "play_audio_sound", | |
| "end_conversation", | |
| "service_callback", | |
| ] | |
| __all__ = [ | |
| "end_conversation", | |
| "mute_stt", | |
| "play_audio_sound", | |
| "service_callback", | |
| "unmute_stt", | |
| ] |
🧰 Tools
🪛 Ruff (0.14.8)
15-21: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🤖 Prompt for AI Agents
In app/ai/voice/agents/breeze_buddy/handlers/internal/__init__.py around lines
15 to 21, the module-level __all__ list is not sorted which triggers Ruff
RUF022; reorder the strings in __all__ into a consistent lexicographic
(alphabetical) order (e.g. end_conversation, mute_stt, play_audio_sound,
service_callback, unmute_stt) so the linter passes, or if intentional keep the
current order add a targeted ignore comment for RUF022 on that line.
| async def play_audio_sound(context: TemplateContext, args, transition_to=None): | ||
| """ | ||
| Play an audio sound file through the transport. | ||
| This handler: | ||
| 1. Loads an audio file from the specified path | ||
| 2. Writes it to the transport output for playback | ||
| Args: | ||
| context: Handler context with bot state access | ||
| args: Additional arguments - should contain 'audio_path' key | ||
| transition_to: Target node to transition to (not used for action handlers) | ||
| Returns: | ||
| Empty dict | ||
| """ | ||
| logger.debug( | ||
| f"play_audio_sound called for call {context.call_sid} with args: {args}" | ||
| ) | ||
|
|
||
| # Get audio path from args, default to a common audio file if not specified | ||
| audio_path = args.get( | ||
| "audio_path", "app/ai/voice/agents/breeze_buddy/static/audio/cough.wav" | ||
| ) | ||
|
|
||
| logger.info( | ||
| f"Attempting to play audio from: {audio_path} for call {context.call_sid}" | ||
| ) | ||
|
|
||
| audio = load_audio(audio_path=audio_path) | ||
| if audio: | ||
| try: | ||
| await context.transport.output().write_audio_frame(audio) | ||
| logger.info( | ||
| f"Successfully played audio from {audio_path} for call {context.call_sid}" | ||
| ) | ||
| except Exception as e: | ||
| logger.error( | ||
| f"Failed to play audio from {audio_path} for call {context.call_sid}: {e}", | ||
| exc_info=True, | ||
| ) | ||
| else: | ||
| logger.warning( | ||
| f"Audio not loaded from {audio_path} for call {context.call_sid}, skipping playback" | ||
| ) | ||
|
|
||
| return {} | ||
|
|
There was a problem hiding this comment.
Security: audio_path from args can enable path traversal / arbitrary file reads.
If args is template/merchant-controlled, restrict to an allowlist (e.g., known filenames) or enforce that the resolved path stays within .../static/audio/ before calling load_audio().
🧰 Tools
🪛 Ruff (0.14.8)
71-71: Unused function argument: transition_to
(ARG001)
107-107: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py around
lines 71 to 118, the handler uses audio_path from args directly which allows
path traversal/arbitrary file reads; fix by validating/sanitizing audio_path
before calling load_audio: restrict to an allowlist of known filenames OR
construct the path by joining a fixed base directory (e.g.,
<repo>/app/ai/voice/agents/breeze_buddy/static/audio/) with a sanitized
filename, resolve the resulting path with os.path.realpath and verify it starts
with the base dir, reject/raise/log and skip playback on invalid input; ensure
you do not allow paths containing ../ or absolute paths and log the rejection
with context.
| __all__ = [ | ||
| "FlowConfigLoader", | ||
| "FlowConfigBuilder", | ||
| "Hook", | ||
| "HookRegistry", | ||
| "UpdateOutcomeInDatabaseHook", | ||
| "transition_handler", | ||
| "TemplateContext", | ||
| "with_context", | ||
| ] |
There was a problem hiding this comment.
Ruff RUF022: sort __all__ (or ignore locally).
🧰 Tools
🪛 Ruff (0.14.8)
27-36: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🤖 Prompt for AI Agents
In app/ai/voice/agents/breeze_buddy/template/__init__.py around lines 27 to 36
the __all__ list is not alphabetically sorted which triggers Ruff RUF022; either
sort the entries in __all__ alphabetically (e.g. FlowConfigBuilder,
FlowConfigLoader, Hook, HookRegistry, TemplateContext,
UpdateOutcomeInDatabaseHook, transition_handler, with_context) or, if sorting is
undesirable, add a local ignore comment (e.g. # noqa: RUF022) to the __all__
assignment to silence the linter.
| def with_context(bot_instance): | ||
| """ | ||
| Decorator factory that injects TemplateContext into handler functions. | ||
| Usage: | ||
| @with_context(bot) | ||
| async def my_handler(context, flow_manager, args): | ||
| # context is TemplateContext instance | ||
| context.outcome = "confirmed" | ||
| Args: | ||
| bot_instance: The OrderConfirmationBot instance | ||
| Returns: | ||
| Decorator function | ||
| """ | ||
|
|
||
| def decorator(handler_func: Callable) -> Callable: | ||
| """ | ||
| Decorator that wraps handler with context injection. | ||
| Args: | ||
| handler_func: Handler function to wrap | ||
| Returns: | ||
| Wrapped handler function | ||
| """ | ||
|
|
||
| async def wrapper(*args, **kwargs): | ||
| # Create context from bot instance | ||
| context = TemplateContext(bot_instance) | ||
|
|
||
| transition_to = kwargs.pop("transition_to", None) | ||
| hooks = kwargs.pop("hooks", None) | ||
| function_name = kwargs.pop("function_name", None) | ||
|
|
||
| is_transition_handler = hooks is not None or function_name is not None | ||
|
|
||
| llm_args = args[0] if len(args) > 0 else {} | ||
|
|
||
| logger.debug( | ||
| f"with_context wrapper called - handler: {handler_func.__name__}, " | ||
| f"is_transition_handler: {is_transition_handler}, " | ||
| f"transition_to: {transition_to}, hooks: {hooks}, function_name: {function_name}" | ||
| ) | ||
|
|
||
| if is_transition_handler: | ||
| logger.debug( | ||
| f"Calling transition handler '{handler_func.__name__}' " | ||
| f"for function '{function_name}'" | ||
| ) | ||
| return await handler_func( | ||
| context, | ||
| llm_args, | ||
| transition_to=transition_to, | ||
| hooks=hooks, | ||
| function_name=function_name, | ||
| ) | ||
| else: | ||
| # Action handlers don't need hooks/function_name | ||
| logger.debug( | ||
| f"Calling action handler '{handler_func.__name__}' " | ||
| f"with transition_to: {transition_to}" | ||
| ) | ||
| return await handler_func( | ||
| context, llm_args, transition_to=transition_to | ||
| ) | ||
|
|
||
| return wrapper | ||
|
|
||
| return decorator |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the structure of the repository
git ls-files | head -20Repository: juspay/clairvoyance
Length of output: 713
🏁 Script executed:
# Find the context.py file and explore the surrounding code
find . -name "context.py" -path "*/breeze_buddy/*" -type fRepository: juspay/clairvoyance
Length of output: 117
🏁 Script executed:
# Let's also look for files that might use this decorator or define handlers
fd -e py -path "*/breeze_buddy/*" | head -30Repository: juspay/clairvoyance
Length of output: 231
🏁 Script executed:
# Let's read the context.py file to understand the full context
cat -n ./app/ai/voice/agents/breeze_buddy/template/context.pyRepository: juspay/clairvoyance
Length of output: 7335
🏁 Script executed:
# Search for files that use the @with_context decorator
rg "@with_context" --type py -A 5 -B 2Repository: juspay/clairvoyance
Length of output: 687
🏁 Script executed:
# Look for handler function definitions that might use this decorator
rg "async def.*handler" --type py -A 3 -B 1Repository: juspay/clairvoyance
Length of output: 6748
🏁 Script executed:
# Check for test files related to context or handlers
find . -name "*test*" -path "*/breeze_buddy/*" -type fRepository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Search for actual usage of @with_context decorator (excluding docstring)
rg "@with_context" --type py -B 2 -A 10 | grep -v "Usage:" | head -50Repository: juspay/clairvoyance
Length of output: 993
🏁 Script executed:
# Look at the builder.py file to understand wrapper_handler context
cat -n ./app/ai/voice/agents/breeze_buddy/template/builder.py | head -150Repository: juspay/clairvoyance
Length of output: 6307
🏁 Script executed:
# Search for where handlers are registered or called
rg "wrapper_handler\|with_context" --type py -B 3 -A 3 | head -100Repository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Look for any tests related to context or handlers
find . -path "*/test*" -name "*.py" -type f | xargs grep -l "with_context\|TemplateContext" 2>/dev/nullRepository: juspay/clairvoyance
Length of output: 47
🏁 Script executed:
# Get more of builder.py to see wrapper_handler
cat -n ./app/ai/voice/agents/breeze_buddy/template/builder.py | sed -n '140,250p'Repository: juspay/clairvoyance
Length of output: 5030
🏁 Script executed:
# Look at the action_handlers module
cat -n ./app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py | head -100Repository: juspay/clairvoyance
Length of output: 3983
🏁 Script executed:
# Check transition.py to see how transition_handler is implemented
cat -n ./app/ai/voice/agents/breeze_buddy/template/transition.pyRepository: juspay/clairvoyance
Length of output: 4722
🏁 Script executed:
# Search for any actual decorator application in the breeze_buddy codebase
rg "def.*handler|async def.*handler" app/ai/voice/agents/breeze_buddy/ --type py -B 1 | head -80Repository: juspay/clairvoyance
Length of output: 2677
🏁 Script executed:
# Search for imports of with_context to see if it's used anywhere
rg "from.*context.*import.*with_context|import.*with_context" --type pyRepository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Search for any reference to with_context outside of context.py
rg "with_context" --type py -l | grep -v "context.py"Repository: juspay/clairvoyance
Length of output: 158
🏁 Script executed:
# Check if there are any commented-out usages of @with_context
rg "@with_context|with_context" app/ai/voice/agents/breeze_buddy/ --type py -C 3Repository: juspay/clairvoyance
Length of output: 3746
🏁 Script executed:
# Check how wrapper_handler is called and what it receives from pipecat
cat -n ./app/ai/voice/agents/breeze_buddy/template/builder.py | sed -n '222,240p'Repository: juspay/clairvoyance
Length of output: 982
🏁 Script executed:
# Look at agent.py to see the full context of how with_context is applied
cat -n ./app/ai/voice/agents/breeze_buddy/agent.py | grep -A 10 "for handler_name"Repository: juspay/clairvoyance
Length of output: 589
🏁 Script executed:
# Look at more context around the handler wrapping in agent.py
rg "with_context\(self\)" app/ai/voice/agents/breeze_buddy/agent.py -B 5 -A 5Repository: juspay/clairvoyance
Length of output: 559
🏁 Script executed:
# Let's verify the actual handler signatures in action_handlers
cat -n ./app/ai/voice/agents/breeze_buddy/handlers/internal/action_handlers.py | sed -n '23,45p'Repository: juspay/clairvoyance
Length of output: 1039
🏁 Script executed:
# Check if there are tests that might reveal the actual behavior
find app -name "*test*.py" -o -name "test_*" -type f | head -10Repository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Let's trace through one more time - check what flow_manager is
rg "flow_manager" app/ai/voice/agents/breeze_buddy/template/ --type py -B 2 -A 2 | head -40Repository: juspay/clairvoyance
Length of output: 1452
with_context wrapper loses the second positional argument (actual llm_args) and remaining kwargs.
The wrapper receives (flow_manager, llm_args) as positional args but only extracts args[0] as the handler's args parameter, dropping the actual llm_args dict. Additionally, any kwargs beyond the explicitly popped ones (transition_to, hooks, function_name) are discarded during handler invocation.
Suggested fix:
async def wrapper(*args, **kwargs):
context = TemplateContext(bot_instance)
transition_to = kwargs.pop("transition_to", None)
hooks = kwargs.pop("hooks", None)
function_name = kwargs.pop("function_name", None)
is_transition_handler = hooks is not None or function_name is not None
- llm_args = args[0] if len(args) > 0 else {}
+ flow_manager = args[0] if len(args) > 0 else None
+ llm_args = args[1] if len(args) > 1 else {}
+ rest_args = args[2:]
if is_transition_handler:
logger.debug(
f"Calling transition handler '{handler_func.__name__}' "
f"for function '{function_name}'"
)
return await handler_func(
context,
llm_args,
+ *rest_args,
transition_to=transition_to,
hooks=hooks,
function_name=function_name,
+ **kwargs,
)
else:
logger.debug(
f"Calling action handler '{handler_func.__name__}' "
f"with transition_to: {transition_to}"
)
return await handler_func(
context,
llm_args,
+ *rest_args,
transition_to=transition_to,
+ **kwargs,
)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/ai/voice/agents/breeze_buddy/template/context.py around lines 135 to 205,
the with_context wrapper currently treats llm_args as args[0] and removes
remaining kwargs by popping only transition_to/hooks/function_name, which drops
the actual second positional argument (flow_manager, llm_args) and any other
kwargs; fix by extracting llm_args from args[1] when present (use flow_manager =
args[0] and llm_args = args[1] if len(args) > 1 else {}), avoid mutating the
original kwargs (copy kwargs before popping) and forward all original positional
and remaining keyword arguments to the wrapped handler (i.e., pass flow_manager,
llm_args, and include transition_to/hooks/function_name plus any other kwargs)
so nothing is lost.
| logger.info( | ||
| f"Transition handler called - function: '{function_name}', " | ||
| f"transition_to: '{transition_to}', hooks: {hooks}, args: {args}" | ||
| ) | ||
|
|
||
| # Schedule hooks to run asynchronously (fire and forget) | ||
| if hooks: | ||
| logger.info( | ||
| f"Scheduling {len(hooks)} hook(s) to execute asynchronously for function '{function_name}'" | ||
| ) | ||
| asyncio.create_task( | ||
| _execute_hooks_async(context, args, hooks, function_name or "unknown") | ||
| ) | ||
| else: | ||
| logger.debug(f"No hooks to execute for function '{function_name}'") | ||
|
|
There was a problem hiding this comment.
Avoid logging raw args/hooks at INFO (PII risk); also ensure background task exceptions are handled.
Suggested pattern:
- logger.info(
- f"Transition handler called - function: '{function_name}', "
- f"transition_to: '{transition_to}', hooks: {hooks}, args: {args}"
- )
+ logger.info(
+ f"Transition handler called - function: '{function_name}', transition_to: '{transition_to}', hooks_count: {len(hooks or [])}"
+ )
@@
- asyncio.create_task(
- _execute_hooks_async(context, args, hooks, function_name or "unknown")
- )
+ task = asyncio.create_task(
+ _execute_hooks_async(context, args, hooks, function_name or "unknown")
+ )
+ task.add_done_callback(
+ lambda t: logger.exception("Hook task failed", exc_info=t.exception())
+ if t.exception()
+ else None
+ )🧰 Tools
🪛 Ruff (0.14.8)
53-55: Store a reference to the return value of asyncio.create_task
(RUF006)
app/api/routers/breeze_buddy.py
Outdated
| @router.post("/push/lead/v2") | ||
| async def trigger_order_confirmation( |
There was a problem hiding this comment.
Fix critical function name collision.
The function name trigger_order_confirmation is already defined at line 361. This redefinition causes the first endpoint to be shadowed and unreachable. FastAPI will only register the last definition, breaking the POST /{merchant}/{template} endpoint.
Rename this function to reflect its distinct purpose:
-async def trigger_order_confirmation(
+async def push_lead_v2(
req: PushLeadRequest,
current_user: TokenData = Depends(get_current_user),
):
"""
Receives lead details and adds in database for processing.
Requires JWT authentication.
"""📝 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.
| @router.post("/push/lead/v2") | |
| async def trigger_order_confirmation( | |
| @router.post("/push/lead/v2") | |
| async def push_lead_v2( | |
| req: PushLeadRequest, | |
| current_user: TokenData = Depends(get_current_user), | |
| ): | |
| """ | |
| Receives lead details and adds in database for processing. | |
| Requires JWT authentication. | |
| """ |
🧰 Tools
🪛 Ruff (0.14.8)
456-456: Redefinition of unused trigger_order_confirmation from line 361
(F811)
🤖 Prompt for AI Agents
In app/api/routers/breeze_buddy.py around lines 455-456, the function defined as
trigger_order_confirmation is a duplicate of the function at line 361 which
causes the earlier endpoint to be shadowed; rename this second function to a
distinct, descriptive name (e.g., trigger_push_lead_v2 or handle_push_lead_v2)
and update any internal references or tests that call it so the FastAPI routes
remain unique and both endpoints register correctly.
| def decode_templates(result: List[asyncpg.Record]) -> List[TemplateModel]: | ||
| """ | ||
| Decode templates from database result. | ||
| """ | ||
| if not result: | ||
| return [] | ||
|
|
||
| templates = [] | ||
| for row in result: | ||
| templates.append(decode_template(row)) | ||
|
|
||
| return templates |
There was a problem hiding this comment.
decode_templates can append None but returns List[TemplateModel].
Make it filter/guard to keep the return type truthful:
def decode_templates(result: List[asyncpg.Record]) -> List[TemplateModel]:
@@
- templates = []
- for row in result:
- templates.append(decode_template(row))
-
- return templates
+ templates: List[TemplateModel] = []
+ for row in result:
+ t = decode_template(row)
+ if t is not None:
+ templates.append(t)
+ return templates🤖 Prompt for AI Agents
In app/database/decoder/breeze_buddy/template.py around lines 47 to 58,
decode_templates currently appends the result of decode_template(row) which may
be None, violating the declared return type List[TemplateModel]; change the loop
to call decode_template(row), check if the returned value is not None before
appending (or use a comprehension that filters out falsy/None values) so only
TemplateModel instances are collected and returned; if decode_template
legitimately can return None and callers rely on that, alternatively update the
function signature to return List[Optional[TemplateModel]] and document the
behavior, but prefer filtering to keep the declared List[TemplateModel].
| TEMPLATE_TABLE = "template" | ||
|
|
||
|
|
||
| def get_template_by_merchant_query( | ||
| merchant_id: str, shop_identifier: str = None, name: str = None | ||
| ) -> Tuple[str, List[Any]]: | ||
| """Generate query to get a template by merchant ID and optional filters.""" | ||
| conditions = ["merchant_id = $1"] | ||
| values = [merchant_id] | ||
|
|
||
| if shop_identifier: | ||
| conditions.append(f"shop_identifier = ${len(values) + 1}") | ||
| values.append(shop_identifier) | ||
|
|
||
| if name: | ||
| conditions.append(f"name = ${len(values) + 1}") | ||
| values.append(name) | ||
|
|
||
| query = f""" | ||
| SELECT id, merchant_id, shop_identifier, name, flow, expected_payload_schema, is_active, created_at, updated_at | ||
| FROM {TEMPLATE_TABLE} | ||
| WHERE {' AND '.join(conditions)} | ||
| LIMIT 1 | ||
| """ | ||
|
|
||
| return query, values | ||
|
|
There was a problem hiding this comment.
Quote the template table name (and keep it consistent with other query modules).
Other query files use "TABLE_NAME" quoting; here FROM template may fail if the migration created "template" (quoted identifier) or if the identifier is problematic in your SQL dialect. Safer:
-TEMPLATE_TABLE = "template"
+TEMPLATE_TABLE = '"template"'
...
- FROM {TEMPLATE_TABLE}
+ FROM {TEMPLATE_TABLE}(or keep TEMPLATE_TABLE = "template" and quote at use sites).
🧰 Tools
🪛 Ruff (0.14.8)
12-12: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
12-12: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
26-31: Possible SQL injection vector through string-based query construction
(S608)
🤖 Prompt for AI Agents
In app/database/queries/breeze_buddy/template.py around lines 8 to 34, the
unquoted table name usage in the query (FROM {TEMPLATE_TABLE}) can fail if the
migration created a quoted identifier; update the query to consistently quote
the table name at use sites (e.g., use FROM "{TEMPLATE_TABLE}" and any other
occurrences in the query string) so it matches the pattern used in other query
modules and ensures correct behavior across SQL dialects; keep TEMPLATE_TABLE
constant as-is and only change its interpolation in the f-string to include the
double quotes.
| outbound_number_id: Optional[str] = None | ||
| merchant_id: str | ||
| workflow: Workflow | ||
| template: str |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for template validation logic in the codebase
echo "=== Checking database constraints for template validation ==="
rg -n "CHECK.*template" --type=sql 2>/dev/null || echo "No SQL files or matches found"
echo -e "\n=== Checking template validation in Python code ==="
rg -nP "(validate.*template|template.*validate|allowed.*template|template.*allowed)" --type=py -C2 2>/dev/null || echo "No matches found"
echo -e "\n=== Checking template enum or registry ==="
rg -nP "(class.*Template.*Enum|VALID_TEMPLATES|ALLOWED_TEMPLATES|TemplateRegistry)" --type=py -C2 2>/dev/null || echo "No matches found"
echo -e "\n=== Checking template table schema ==="
fd -e sql 2>/dev/null | xargs rg -n "CREATE TABLE.*template" -A 20 2>/dev/null || echo "No SQL files with template tables found"
echo -e "\n=== Checking for workflow-related validation ==="
rg -nP "Workflow|WORKFLOW" --type=py -C2 2>/dev/null | head -50 || echo "No matches found"Repository: juspay/clairvoyance
Length of output: 3130
🏁 Script executed:
#!/bin/bash
# Check schemas.py for validators
echo "=== Checking schemas.py for Pydantic validators ==="
rg -n "@validator|Field.*validate" app/schemas.py -B2 -A5
echo -e "\n=== Full app/schemas.py content (first 150 lines) ==="
head -150 app/schemas.py
echo -e "\n=== Searching for API endpoints that use template parameter ==="
rg -n "template" app/ --type=py -l | grep -E "(routes|api|endpoint)" | head -10
echo -e "\n=== Checking for template parameter validation in routes ==="
rg -n "def.*template|@.*template" app/ --type=py -C3 | head -80Repository: juspay/clairvoyance
Length of output: 9398
🏁 Script executed:
#!/bin/bash
# Check template loader for validation
echo "=== Checking template loader implementation ==="
cat app/ai/voice/agents/breeze_buddy/template/loader.py
echo -e "\n=== Checking API route trigger_order_confirmation for template validation ==="
sed -n '360,420p' app/api/routers/breeze_buddy.pyRepository: juspay/clairvoyance
Length of output: 8321
Runtime template validation exists but lacks compile-time type safety.
The removal of the enum for template fields does eliminate compile-time type checking. However, template validation is enforced at runtime through database lookups in the template loader and API route handlers. When a template is not found in the database, a ValueError is raised, and API endpoints return 404 errors.
While this runtime validation prevents invalid templates from being executed, it provides weaker guarantees than the previous enum-based approach. Consider adding a template name whitelist or registry at the application layer (e.g., ALLOWED_TEMPLATES constant or template validation in schemas via Pydantic validators) to restore compile-time safety for developers while maintaining database as the source of truth.
🤖 Prompt for AI Agents
In app/schemas.py around line 41, the template field is declared as a plain
string which removes compile-time safety; add a whitelist/registry and a
Pydantic validator to enforce allowed template names at model validation. Create
an ALLOWED_TEMPLATES constant (or load it at app startup from the DB and cache
it) and implement a validator on the schema that checks template is in that set
and raises a ValueError with a clear message if not; ensure the startup loader
syncs the list if you choose dynamic loading and update any tests that assume
the previous enum behavior.
| "task_messages": [], | ||
| "role_messages": [ | ||
| { | ||
| "role": "system", |
| "template_name": "order-confirmation", | ||
| "is_active": true, | ||
| "description": "Complete order confirmation workflow with address updates", | ||
| "expected_payload_schema": { |
| "description": "Call this function when the user confirms that they are available to talk with clear affirmative responses", | ||
| "properties": {}, | ||
| "required": [], | ||
| "transition_to": "verify_order_details", |
| "name": "update_outcome_in_database", | ||
| "expected_fields": { | ||
| "outcome": { "type": "enforce", "value": "cancelled" }, | ||
| "reason": { "type": "infer_llm" } |
| ) | ||
|
|
||
|
|
||
| async def unmute_stt(context: TemplateContext, args, transition_to=None): |
| ) | ||
|
|
||
|
|
||
| async def play_audio_sound(context: TemplateContext, args, transition_to=None): |
| from app.database.accessor.breeze_buddy.template import get_template_by_merchant | ||
| from app.schemas import CallProvider | ||
|
|
||
| load_dotenv(override=True) |
| load_dotenv(override=True) | ||
|
|
||
|
|
||
| class OrderConfirmationBot: |
app/api/routers/breeze_buddy.py
Outdated
|
|
||
|
|
||
| @router.get("/breeze/order-confirmation/dashboard", include_in_schema=False) | ||
| @router.get("/dashboard", include_in_schema=False) |
There was a problem hiding this comment.
per merchant per shop
badri-singhal
force-pushed
the
template-based-architechture
branch
5 times, most recently
from
af3f56e to
617906d
Compare
badri-singhal
force-pushed
the
template-based-architechture
branch
from
617906d to
087caaf
Compare
| from app.schemas import LeadCallOutcome | ||
|
|
||
|
|
||
| async def service_callback(context: TemplateContext, args, transition_to=None): |
| "attemptCount": context.lead.attempt_count + 1 if context.lead else 1, | ||
| "transcription": json.dumps(filtered_transcript, ensure_ascii=False), | ||
| "callDuration": call_duration, | ||
| "orderId": ( |
| }, | ||
| "expected_callback_response_schema": { | ||
| "cancellation_reason": "string", | ||
| "updated_address": "string" |
| "content": "You are Rhea, a friendly human and natural-sounding customer care representative from {shop_name}. Your goal is to confirm a Cash on Delivery (COD) order with the customer.\n\nIMPORTANT: You are a text-to-speech voice agent. Generate all responses as plain, unformatted text without any special characters, markdown formatting, asterisks, underscores, or other unnecessary symbols. Keep your text simple and clean for natural speech conversion.\n\nNever use the word \"great\" in your responses. Instead, use polite alternatives like \"thank you\", \"thanks\", or \"appreciate it\". Avoid any phrases that sound overly enthusiastic or robotic.\n\nNever reveal or suggest that you are a bot or automated system. You must always sound and respond as a real human representative from {shop_name}.\n\nBefore starting the conversation, identify whether the customer name \"{customer_name}\" is typically male or female in India. Based on this:\n- If the name is clearly male, address the customer as \"Sir\".\n- If clearly female, address them as \"Madam\".\n- If unsure, use their name instead of Sir/Madam in the conversation.\n\nIMPORTANT: Use the customer's name only once during the entire conversation - in the opening greeting. After that, address them as Sir/Madam (based on gender) or avoid direct address if gender is unclear.\n\nIMPORTANT: - If the user speaks in another language (like Hindi), reply in that same language but keep the same friendly, human tone.\n\nYour main job is to verify the following order details:\n- Items: {items}\n- Total Price: {total_price}\n- Delivery Address: {customer_address}\n\nDelivery Address Reading Rules\n\n Pincode Handling\n - If the address includes a 6-digit pincode, read it digit by digit only in English.\n - Example: 123456 → \"Pincode is one two three four five six.\"\n - Never read the pincode as a full number or in any other language.\n\n Mobile / Phone Number Handling\n - If the address includes a 10-digit mobile or phone number, read it digit by digit only in English.\n - Example: 9876543210 → \"nine eight seven six five four three two one zero.\"\n\n Example of correct usage when speaking adress in other language e.g. Hindi:\n \"आपका पता है [address in Hindi], aur pincode hai one two three four five six.\"\n\nTone and Language\n - Speak in a warm, casual, and natural tone — avoid robotic phrasing.\n\nAction Handling\n - Always use the provided functions to perform any actions related to the order.\n - Do not attempt to perform these actions through plain text replies.\n\nYour only role is to confirm or cancel this specific order. If the user asks about anything else (e.g. product details, delivery times, other products), you must use the appropriate function (`handle_unrelated_question_node` or `confirm_order_with_question`). Do not try to answer these questions yourself." | ||
| } | ||
| ], | ||
| "pre_actions": [], |
There was a problem hiding this comment.
this should be done now
| logger.info( | ||
| f"Updating database with call completion details for call {context.call_sid}" | ||
| ) | ||
| context.bot.lead = await context.bot.completion_function( |
| self.name = name | ||
|
|
||
| @abstractmethod | ||
| async def execute( |
| ) | ||
|
|
||
| # Import here to avoid circular dependency | ||
| from app.database.accessor.breeze_buddy.template import get_template_by_merchant |
2 participants
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.