fix: set in-memory outcome to prevent BUSY default in direct mode#765
fix: set in-memory outcome to prevent BUSY default in direct mode#765sharifajahanshaik wants to merge 1 commit into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughUpdateOutcomeInDatabaseHook.execute assigns ChangesOutcome State Race Condition Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent end_conversation_global from defaulting a call outcome to BUSY in Direct Mode by setting the lead outcome in-memory immediately, rather than waiting for the async database update to complete.
Changes:
- Set
context.lead.outcomebefore awaiting the DB write inUpdateOutcomeInDatabaseHook.execute. - Add an explanatory comment describing the Direct Mode race condition being addressed.
| # # Set in-memory outcome IMMEDIATELY before the async DB write. | ||
| # # This prevents a race condition in Direct Mode where | ||
| # # end_conversation_global checks context.lead.outcome and | ||
| # # defaults to BUSY because the async DB write hasn't completed yet. | ||
| context.lead.outcome = outcome |
| # # Set in-memory outcome IMMEDIATELY before the async DB write. | ||
| # # This prevents a race condition in Direct Mode where | ||
| # # end_conversation_global checks context.lead.outcome and | ||
| # # defaults to BUSY because the async DB write hasn't completed yet. |
![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 (2)
app/ai/voice/agents/breeze_buddy/template/hooks.py (2)
208-212: ⚡ Quick winConsider setting in-memory outcome after transfer check.
The in-memory assignment at line 212 uses the LLM-provided outcome, but the transfer override check at lines 244-250 may change the final outcome to
"TRANSFERRED". This causescontext.lead.outcometo be set to the LLM value initially, then replaced with the database version (which has the transfer override) at line 273.While the final state is correct, you could avoid setting the in-memory outcome twice by moving the transfer check before line 212. This would ensure
context.lead.outcomeis set once with the final correct value.♻️ Suggested reordering
Move the metadata building and transfer check before the in-memory assignment:
# Get lead from context if not context.lead or not context.lead.id: logger.info(...) return try: - # Set in-memory outcome IMMEDIATELY before the async DB write. - # This prevents a race condition in Direct Mode where - # end_conversation_global checks context.lead.outcome and - # defaults to BUSY because the async DB write hasn't completed yet. - context.lead.outcome = outcome - logger.debug(f"outcome '{outcome}' for lead {context.lead.id}") - # Initialize metadata with existing data meta_data = context.lead.metaData or {} # Initialize outcome object inside metadata if it doesn't exist if "outcome" not in meta_data: meta_data["outcome"] = {} # Add all other fields except outcome to the outcome object for key, value in final_data.items(): if key != "outcome" and value is not None: meta_data["outcome"][key] = value logger.debug(...) - # Log all metadata properties that were added - if meta_data: - logger.debug(...) - else: - logger.debug(...) - # Guard: if a transfer is in progress, preserve "TRANSFERRED" outcome if meta_data.get("transfer", {}).get("status") == "success": logger.info(...) outcome = "TRANSFERRED" + + # Set in-memory outcome IMMEDIATELY before the async DB write. + # This prevents a race condition in Direct Mode where + # end_conversation_global checks context.lead.outcome and + # defaults to BUSY because the async DB write hasn't completed yet. + context.lead.outcome = outcome + logger.debug(f"Set in-memory outcome '{outcome}' for lead {context.lead.id}") # Update lead in database with outcome logger.info(...)Also applies to: 244-250
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/ai/voice/agents/breeze_buddy/template/hooks.py` around lines 208 - 212, The in-memory assignment context.lead.outcome is being set to the LLM-provided outcome before the transfer-override logic runs, causing a redundant write and potential race; move the transfer override check (the block that forces outcome to "TRANSFERRED" based on transfer metadata) and the metadata construction so they run before assigning context.lead.outcome, then set context.lead.outcome exactly once to the final resolved outcome (preserve existing variable names and logic that reads/writes outcome and metadata).
208-211: ⚡ Quick winFix comment formatting.
The comment uses
# #(hash-space-hash) at the start of each line, which is non-standard. Python convention uses a single#for comments.✨ Proposed formatting fix
- # # Set in-memory outcome IMMEDIATELY before the async DB write. - # # This prevents a race condition in Direct Mode where - # # end_conversation_global checks context.lead.outcome and - # # defaults to BUSY because the async DB write hasn't completed yet. + # Set in-memory outcome IMMEDIATELY before the async DB write. + # This prevents a race condition in Direct Mode where + # end_conversation_global checks context.lead.outcome and + # defaults to BUSY because the async DB write hasn't completed yet.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/ai/voice/agents/breeze_buddy/template/hooks.py` around lines 208 - 211, The multi-line comment in the Breeze Buddy hook (the block describing setting the in-memory outcome before the async DB write and referencing end_conversation_global and context.lead.outcome) uses non-standard "# #" prefixes; change each line to a single "#" with a single space after it (e.g., "# Set in-memory outcome IMMEDIATELY before the async DB write.") so the comment follows normal Python style and remains clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/ai/voice/agents/breeze_buddy/template/hooks.py`:
- Line 213: The debug log message using logger.debug(f"outcome '{outcome}' for
lead {context.lead.id}") is ambiguous; update it to include an action verb and
clearer context (e.g., "Recorded outcome", "Processed outcome", or "Setting
outcome") so logs are self-descriptive. Locate the logger.debug call in the hook
where outcome and context.lead.id are available (the function in
template/hooks.py that references variables outcome and context.lead.id) and
change the message to something like "Recorded outcome '{outcome}' for lead
{context.lead.id}" to improve clarity.
---
Nitpick comments:
In `@app/ai/voice/agents/breeze_buddy/template/hooks.py`:
- Around line 208-212: The in-memory assignment context.lead.outcome is being
set to the LLM-provided outcome before the transfer-override logic runs, causing
a redundant write and potential race; move the transfer override check (the
block that forces outcome to "TRANSFERRED" based on transfer metadata) and the
metadata construction so they run before assigning context.lead.outcome, then
set context.lead.outcome exactly once to the final resolved outcome (preserve
existing variable names and logic that reads/writes outcome and metadata).
- Around line 208-211: The multi-line comment in the Breeze Buddy hook (the
block describing setting the in-memory outcome before the async DB write and
referencing end_conversation_global and context.lead.outcome) uses non-standard
"# #" prefixes; change each line to a single "#" with a single space after it
(e.g., "# Set in-memory outcome IMMEDIATELY before the async DB write.") so the
comment follows normal Python style and remains clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 065a3734-dc9a-4127-a2f8-754ebf331ff2
📒 Files selected for processing (1)
app/ai/voice/agents/breeze_buddy/template/hooks.py
| # # end_conversation_global checks context.lead.outcome and | ||
| # # defaults to BUSY because the async DB write hasn't completed yet. | ||
| context.lead.outcome = outcome | ||
| logger.debug(f"outcome '{outcome}' for lead {context.lead.id}") |
There was a problem hiding this comment.
Improve log message clarity.
The debug log at line 213 is missing an action verb. Messages like "outcome 'X' for lead Y" are ambiguous during debugging. Consider making the message self-descriptive.
📝 Proposed fix
- logger.debug(f"outcome '{outcome}' for lead {context.lead.id}")
+ logger.debug(f"Set in-memory outcome '{outcome}' for lead {context.lead.id}")📝 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.debug(f"outcome '{outcome}' for lead {context.lead.id}") | |
| logger.debug(f"Set in-memory outcome '{outcome}' for lead {context.lead.id}") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/ai/voice/agents/breeze_buddy/template/hooks.py` at line 213, The debug
log message using logger.debug(f"outcome '{outcome}' for lead
{context.lead.id}") is ambiguous; update it to include an action verb and
clearer context (e.g., "Recorded outcome", "Processed outcome", or "Setting
outcome") so logs are self-descriptive. Locate the logger.debug call in the hook
where outcome and context.lead.id are available (the function in
template/hooks.py that references variables outcome and context.lead.id) and
change the message to something like "Recorded outcome '{outcome}' for lead
{context.lead.id}" to improve clarity.
sharifajahanshaik
force-pushed
the
fix/outcome-race-condition-direct-mode
branch
from
fe07fe3 to
89b2156
Compare
Summary by CodeRabbit