fix(widget+voice): carry, run & persist chat session-state across CHAT↔VOICE#835
fix(widget+voice): carry, run & persist chat session-state across CHAT↔VOICE#835swaroopvarma1 wants to merge 1 commit into
Conversation
WalkthroughThe PR propagates the accumulated ChangesCHAT→VOICE agent_state propagation
Sequence Diagram(s)sequenceDiagram
participant Widget as Widget Client
participant Handler as voice_connect_handler
participant SessionStore as get_agent_session_state
participant VoiceAgent as BreezyBuddyVoiceAgent
Widget->>Handler: voice connect (session_id)
Handler->>SessionStore: get_agent_session_state(session_id)
SessionStore-->>Handler: agent_session_state dict
Handler->>VoiceAgent: meta_data_seed { agent_state: {...} }
VoiceAgent->>VoiceAgent: capture agent_state into _widget_resume_seed
VoiceAgent->>VoiceAgent: merge agent_state keys into template_vars (missing keys only)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/agent/__init__.py`:
- Around line 367-376: The widget resume state merge is happening after
load_template_config consumes the lead payload, which means placeholder
resolution for cart_id and checkout_id still uses the old payload. Move the
logic that merges agent_state keys from self._widget_resume_seed into
self.lead.payload to execute before the load_template_config call (not after),
so that the loader sees the updated identifiers during placeholder resolution.
Keep the current template_vars merge as a fallback mechanism for any remaining
missing keys. Find where load_template_config is invoked and relocate the
agent_state hydration logic to occur before that call.
🪄 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: 0e7069d4-4470-482d-a6b2-f3c002dd3e96
📒 Files selected for processing (2)
app/ai/voice/agents/breeze_buddy/agent/__init__.pyapp/api/routers/breeze_buddy/widget/handlers.py
| # Widget resume: thread the chat session's accumulated agent_state | ||
| # (cart_id/checkout_id/client-pushed facts) into template_vars so | ||
| # {placeholder} resolution in the resumed voice flow uses the | ||
| # chat-built identifiers instead of losing them on the flip. | ||
| # only_if_missing — never clobber an explicitly-rendered call var. | ||
| if self._widget_resume_seed: | ||
| resumed_state = self._widget_resume_seed.get("agent_state") or {} | ||
| merged_keys = [k for k in resumed_state if k not in self.template_vars] | ||
| for k in merged_keys: | ||
| self.template_vars[k] = resumed_state[k] |
There was a problem hiding this comment.
Merge resume state before load_template_config consumes the lead payload.
load_template_config(self.lead) runs before this merge, so loader-time {cart_id} / {checkout_id} placeholder resolution still sees the old lead.payload. Hydrate missing agent_state keys into self.lead.payload before the loader, then keep the template_vars merge as a fallback.
🐛 Proposed ordering fix
+ if self._widget_resume_seed:
+ resumed_state = self._widget_resume_seed.get("agent_state") or {}
+ if isinstance(resumed_state, dict):
+ lead_payload = (
+ self.lead.payload if isinstance(self.lead.payload, dict) else {}
+ )
+ missing_payload_keys = [
+ k for k in resumed_state if k not in lead_payload
+ ]
+ if missing_payload_keys:
+ self.lead.payload = {
+ **lead_payload,
+ **{
+ k: resumed_state[k]
+ for k in missing_payload_keys
+ },
+ }
+
try:
(
self.template,
self.configurations,
self.template_vars,
) = await load_template_config(self.lead)🤖 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/agent/__init__.py` around lines 367 - 376,
The widget resume state merge is happening after load_template_config consumes
the lead payload, which means placeholder resolution for cart_id and checkout_id
still uses the old payload. Move the logic that merges agent_state keys from
self._widget_resume_seed into self.lead.payload to execute before the
load_template_config call (not after), so that the loader sees the updated
identifiers during placeholder resolution. Keep the current template_vars merge
as a fallback mechanism for any remaining missing keys. Find where
load_template_config is invoked and relocate the agent_state hydration logic to
occur before that call.
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Review Summary
Files reviewed: 2
New issues raised: 0
Status: ✅ APPROVED
Analysis
This PR fixes a data-loss issue when transitioning from CHAT → VOICE mode in widget sessions by carrying the agent_session_state (containing reducer-built identifiers like cart_id, checkout_id) across the channel flip.
Changes Verified:
-
app/api/routers/breeze_buddy/widget/handlers.py(lines ~603-613)- Fetches
agent_session_stateviaget_agent_session_state()using parameterized SQL ($1 placeholder) ✓ - Passes state data through
meta_data_seed["agent_state"]to voice runtime ✓ - Properly handles
Nonecase with empty dict fallback ✓
- Fetches
-
app/ai/voice/agents/breeze_buddy/agent/__init__.py(lines ~335-338, ~367-382)- Re-hydrates
agent_statefrom_widget_resume_seed✓ - Merges into
template_varswith "only_if_missing" strategy (never clobbers explicitly-rendered call vars) ✓ - Logs only key names (
sorted(merged_keys)), not values — no PII exposure ✓
- Re-hydrates
Security Assessment:
| Criteria | Status |
|---|---|
| SQL Injection | ✅ Safe — uses $1 parameterized queries via get_agent_session_state_query() |
| Auth/Authorization | ✅ Safe — session ownership verified via assert_widget_session_ownership() before state access |
| PII Exposure | ✅ Safe — logging only key names, not values |
| Data Integrity | ✅ Safe — only_if_missing merge preserves explicit template vars |
| Secrets | ✅ Safe — no hardcoded credentials |
Architecture Notes:
The implementation correctly follows the established pattern in loader.py:195-203 for threading payload fields into template variables. The state data originates from server-side reducers (not client-manipulable), making this a secure data-flow.
The PR description accurately reflects the scope: this closes the data-loss seam; full chat-style dynamic inject_tool_args parity in voice remains a separate follow-up as noted.
Reviewed per project standards: SQL safety, auth/authz, PII protection, async resilience, and migration integrity.
swaroopvarma1
force-pushed
the
feat/widget-voice-carry-agent-state
branch
from
16cabfb to
d78a2e2
Compare
swaroopvarma1
changed the title
…T↔VOICE
Three related correctness fixes so a widget CHAT↔VOICE flip keeps the session's
reducer-built state (cart_id, checkout_id, client-pushed facts) instead of
silently dropping it. Before this, voice had ZERO state-reducer /
tool_arg_injection code, so a template's state config worked in chat and
silently no-op'd on the actual voice call.
1. CARRY (CHAT→VOICE seed). The widget voice resume seed carried prior_history
+ start_node but NOT agent_session_state. Read it into the seed
(widget/handlers.py) and merge into the voice runtime's template_vars
(only_if_missing) so chat-built identifiers thread through voice's existing
{placeholder} resolution. The voice agent now holds self.agent_state, seeded
from the resume.
2. RUN (SessionStatePolicy on voice). inject_tool_args runs before each voice
tool and apply_state_reducers after it, reading/writing bot.agent_state —
the same pure engines chat runs in _cycle_loop — at BOTH voice tool seams:
global functions (_make_global_wrapper) and MCP tools (the mcp loader's
_state_wrap_mcp_handler, applied outside the approval gate). Gated by a new
handles_state_externally flag: chat sets it (wrapper skips, chat does it
itself — no double-application); voice doesn't. The two voice-state helpers
live in session_state.py next to the pure engines so both seams share them.
No-op for templates with no state config.
3. PERSIST (VOICE→CHAT drain). end_conversation writes the voice-accumulated
agent_state back to the chat_session, so a later chat turn sees what voice
changed — completing the round-trip.
The principled completion of the agent_state seed-fix: voice now READS and
UPDATES the shared state via the chat engines, for global functions AND MCP
tools, with no new state machinery.
Tests: tests/test_voice_session_state.py (global-function hook + gating) and
tests/test_mcp_approval.py (MCP state wrap). pyrefly 0 errors; full suite 452
passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
swaroopvarma1
force-pushed
the
feat/widget-voice-carry-agent-state
branch
from
d78a2e2 to
fe2dee7
Compare
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Review Summary
Files reviewed: 9
New issues raised: 1 (minor suggestion)
Status: ✅ APPROVED
Security Assessment
| Criteria | Status |
|---|---|
| SQL Injection | ✅ Safe — all queries use $1, $2 parameterized placeholders via queries→accessor→decoder layers |
| Auth/Authorization | ✅ Safe — assert_widget_session_ownership() verifies session ownership before state access |
| PII Exposure | ✅ Safe — logging only key names (sorted(merged_keys), sorted(self.agent_state)), never values |
| Secrets | ✅ Safe — no hardcoded credentials; secrets flow through existing KMS-encrypted credential system |
| Data Integrity | ✅ Safe — only_if_missing merge strategy preserves explicit template vars |
Architecture Review
The implementation correctly follows established patterns:
- Voice state management mirrors chat's
_cycle_loopbehavior viahandles_state_externallygating - MCP tools and global functions both receive state injection/reduction via shared helpers
- Database migrations are append-only (no existing migration files modified)
- Test coverage added for voice session state (8 tests) and MCP state wrapping (3 tests)
Existing CodeRabbitAI Comment
The author's PR description already addresses the ordering concern: _render_direct_mode_flow renders only the system_prompt — function {placeholder}s resolve at call time from the already-merged template_vars. The inject_tool_args addition provides the robust call-time path. This is a deliberate design choice, not an oversight.
Minor Suggestion Raised
One inline comment on end_conversation.py regarding if voice_state: vs is not None — non-blocking.
Reviewed per project standards: SQL safety, auth/authz, PII protection, async resilience, and migration integrity.
| if voice_state: | ||
| await upsert_agent_session_state( | ||
| chat_session_id=str(widget_session_id), | ||
| data=voice_state, |
There was a problem hiding this comment.
💬 SUGGESTION: Consider using if voice_state is not None: instead of if voice_state: to allow persisting an empty dict {} if explicitly set. Current logic skips persistence when agent_state is empty, which may be intentional (nothing to persist) but could miss clearing state scenarios.
3 participants
What
So a widget CHAT↔VOICE flip keeps the session's reducer-built state (
cart_id,checkout_id, client facts) instead of silently dropping it. Root cause: the voice runtime had zero state-reducer /tool_arg_injectioncode — a template's state config worked in chat and silently no-op'd on the real voice call.1. Carry (CHAT→VOICE seed)
Seed
agent_session_state(widget/handlers.py) → merge intotemplate_vars(only_if_missing) so chat-built IDs thread through voice's existing{placeholder}resolution. Voice agent holdsself.agent_state.2. Run (SessionStatePolicy on voice)
inject_tool_argsbefore each voice tool,apply_state_reducersafter — the same pure engines chat runs in_cycle_loop— at both voice tool seams: global functions (_make_global_wrapper) and MCP tools (the mcp loader's_state_wrap_mcp_handler, applied outside the approval gate). Gated by a newhandles_state_externallyflag (chat sets it → wrapper skips; voice applies). The two helpers live insession_state.pynext to the pure engines so both seams share them. No-op without state config.3. Persist (VOICE→CHAT drain)
end_conversationwrites voice's accumulatedagent_stateback to the chat session — completes the round-trip.Re: the AI review comment (merge timing)
Validated — not applicable, so not changed.
_render_direct_mode_flow(loader.py:72-82) renders only thesystem_promptat load; "Function descriptions are not rendered." So function{placeholder}s (where these IDs live) resolve at call time from the already-mergedtemplate_vars. And part #2 addsinject_tool_argsas the robust call-time path anyway.Verification
tests/test_voice_session_state.py(8) — global-function inject/reduce + gating;tests/test_mcp_approval.py(+3) — MCP state wrap.Manual repro
Chat adds an item (builds
cart_id) → tap mic → voice acts on the same cart → back in chat, cart persists.