adding the direct proxy for the daily client #312
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 WalkthroughUpdates proxy setup to use environment variables in voice agent initialization; refines prompt texts and reduces discount cap; simplifies Breeze surcharge rule creation by setting payment_method to "*" unconditionally. No public API/signature changes. Control flow largely unchanged aside from standardized payment method value. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant VA as Voice Agent
participant ENV as Environment
participant DC as Daily CallClient
VA->>ENV: Set HTTP_PROXY/HTTPS_PROXY env vars
Note right of ENV: Env vars now source of proxy
VA->>DC: Initialize / operate
DC-->>VA: Uses proxy from environment
Note over VA,DC: On failure, logs and proceeds without proxy
sequenceDiagram
autonumber
participant Agent as Auto Agent
participant Breeze as Breeze Config
participant API as Breeze API
Agent->>Breeze: _handle_create_surcharge_rule(input)
Breeze->>Breeze: Set payment_method="*"
Note right of Breeze: Removed mapping by paymentMethodType
Breeze->>API: Create/validate rule with payment_method="*"
API-->>Breeze: Response (success/error)
Breeze-->>Agent: Result (unchanged flow)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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: 1
🧹 Nitpick comments (1)
app/agents/voice/automatic/prompts/system/performance_directives.py (1)
43-43: Replace ambiguous multiplication character and verify discount cap reduction.The Unicode multiplication sign
×(U+00D7) can cause confusion. Use ASCII*orxinstead.Additionally, the discount cap was reduced from 10% to 5% of AOV. Ensure this business logic change aligns with product requirements and that stakeholders have approved this reduction.
Apply this diff to use ASCII multiplication:
- 2. Calculate discount: (AOV × Gap%) ÷ 100, capped at 5% of AOV, minimum ₹5 + 2. Calculate discount: (AOV * Gap%) / 100, capped at 5% of AOV, minimum ₹5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/agents/voice/automatic/__init__.py(1 hunks)app/agents/voice/automatic/prompts/system/performance_directives.py(2 hunks)app/agents/voice/automatic/tools/breeze/configuration.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.0)
app/agents/voice/automatic/prompts/system/performance_directives.py
43-43: String contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF001)
🔇 Additional comments (2)
app/agents/voice/automatic/__init__.py (1)
346-357: LGTM! Proxy configuration via environment variables.The shift from a Daily-specific proxy API to setting standard environment variables (HTTP_PROXY, HTTPS_PROXY, and lowercase variants) is a sound approach. This ensures compatibility with various underlying libraries that may check different case variants of these variables.
app/agents/voice/automatic/prompts/system/performance_directives.py (1)
11-11: LGTM! Clearer function description.The added detail about finding "total Cash on Delivery and total prepaid amounts" makes the step more explicit and actionable.
| payment_method = "WALLET" | ||
| else: | ||
| payment_method = "CASH" | ||
| payment_method = "*" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify inconsistency: create always uses "*" but update uses conditional mapping.
The _handle_create_surcharge_rule function now unconditionally sets payment_method = "*" (line 116), but _handle_update_surcharge_rule (lines 348-360) still contains the conditional mapping logic:
if payment_method_type in ["CREDIT", "DEBIT"]:
payment_method = "CREDIT/DEBIT"
elif payment_method_type == "UPI":
payment_method = "UPI"
# ... etcThis inconsistency means:
- Create operations: Always set
payment_methodto"*"regardless ofpaymentMethodType - Update operations: Derive
payment_methodfrompaymentMethodTypeusing the mapping
Additionally, the function schema documentation (lines 791-792) states that paymentMethod is "automatically derived from paymentMethodType" with specific mappings (CASH→CASH, CREDIT/DEBIT→CREDIT/DEBIT, etc.), which contradicts the current implementation.
If setting payment_method = "*" universally is intentional, consider:
- Applying the same logic to the update function for consistency
- Updating the function schema documentation to reflect this behavior
- Adding a comment explaining why
"*"is always used
Run the following script to verify if there are other places where the conditional mapping is used:
🏁 Script executed:
#!/bin/bash
# Search for other instances of payment_method_type to payment_method mapping logic
rg -n -A 10 'payment_method_type in \["CREDIT", "DEBIT"\]' --type=pyLength of output: 1051
🏁 Script executed:
#!/bin/bash
# Extract create surcharge rule handler around line 116
rg -n -C3 'def _handle_create_surcharge_rule' app/agents/voice/automatic/tools/breeze/configuration.py
# Extract schema documentation around paymentMethod comments
rg -n -C2 'paymentMethod' app/agents/voice/automatic/tools/breeze/configuration.pyLength of output: 4308
Unify payment_method derivation in create and update operations
Replace the hardcoded payment_method = "*" in _handle_create_surcharge_rule (line 116) with the same conditional mapping used in _handle_update_surcharge_rule (lines 348–360), and update the schema documentation (lines 786–792) to reflect the actual mapping logic.
🤖 Prompt for AI Agents
In app/agents/voice/automatic/tools/breeze/configuration.py around line 116, the
create handler currently hardcodes payment_method = "*"; replace that with the
same conditional mapping logic used in the update handler (see lines 348–360) so
create derives payment_method from input the same way (including handling of
wildcard/default and any specific key translations). Also update the schema
documentation at lines 786–792 to describe the actual mapping rules (what inputs
map to "*" vs specific values and any special cases) so docs match the
implemented logic.
| f"Received paymentMethodType: '{payment_method_type}' (type: {type(payment_method_type)})" | ||
| ) | ||
|
|
||
| payment_method = "*" |
There was a problem hiding this comment.
why are these changes?
priyanshi-2003
force-pushed
the
adding-direct-proxy-in-the-case-of-daily-client
branch
from
b0788e2 to
b334f60
Compare
| os.environ.update( | ||
| { | ||
| "HTTP_PROXY": proxy_url, | ||
| "HTTPS_PROXY": proxy_url, | ||
| "http_proxy": proxy_url, | ||
| "https_proxy": proxy_url, | ||
| } | ||
| ) | ||
| logger.info( | ||
| "Daily WebRTC proxy configured successfully via environment variables" | ||
| ) |
There was a problem hiding this comment.
This is nothing but setting up environment variables. If this is required then you can just add these in env and then check
There was a problem hiding this comment.
In which file shall I add this, as this is cloud environment specific
3 participants
Summary by CodeRabbit
New Features
Documentation
Refactor