fix(llm): keep plugin-specific realtime handlers across a session swap - #6646
fix(llm): keep plugin-specific realtime handlers across a session swap#6646priyam-garg wants to merge 5 commits into
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
Some extra detail on the root cause and the one behavioral change reviewers will want to weigh in on. Root cause
self._forwarders: dict[EventTypes, Callable[[object], None]] = {
event: _make_forwarder(event) for event in _FORWARDED_EVENTS
}
The one behavioral changeHandlers have to be bound on the wrapper rather than on the child. Binding to rt_session = agent.realtime_llm_session
# before: bound to the child, lost on the next swap
rt_session._active.on("openai_server_event_received", handler)
# after: bound to the wrapper, re-attached to every new child
rt_session.on("openai_server_event_received", handler)Worth calling out explicitly since it's a call-site change for anyone already working around this, though Design notes
Test evidenceFour of the five new tests fail on The fifth, Scope of #6556Flagging this so the auto-close isn't a surprise: #6556 scopes the ask to resubscribing when falling back to the same plugin-type model. This implementation resubscribes on every swap, including across plugin types — an Possible follow-upThis makes the case for #6553 a bit stronger: callers now need Verified with |
`_FallbackRealtimeSession` only re-attached the fixed `_FORWARDED_EVENTS` allowlist to each new child, so handlers for provider-specific events (e.g. openai's `openai_server_event_received`) were silently dropped by `restart_session()` and by an availability fallback. Forwarders are now created on demand: subscribing on the wrapper registers a forwarder for that event, and `_bind` replays every forwarder onto each new child, so plugin-specific subscribers survive a swap like the generic ones already did. A `_child_bound` flag keeps a subscription made mid-swap off the child that is being discarded. Forwarders also became varargs, so a plugin event carrying more than one payload argument no longer loses all but the first. Fixes livekit#6559 Fixes livekit#6556
When `RealtimeModel.session()` itself raises, `_bring_up`'s failure path unbinds the outgoing child a second time -- and with on-demand forwarders that dict may hold an entry registered mid-swap that was never attached to it. `EventEmitter.off` discards rather than removes, so detaching a forwarder the child never had is a no-op and the swap still cascades. Adds a `session_error` hook to the realtime fake to reach that path.
The plugin-event handler was a no-op and the trailing emit asserted nothing, so the test passed even if the forwarder never reached the new child. Capture the payload and assert on it instead. Verified load-bearing: reverting the adapter change now fails this test.
`EventEmitter.emit` computes the varargs check and the arg slice inside its per-callback loop and never mutates `args`, so subscriber arities can't interact. Cover that directly: a varargs and a single-arg subscriber on the same event, asserting each gets its own share of a two-arg payload.
e35081c to
ef29f39
Compare
Forwarder creation was hooked only into `on()`. `EventEmitter.once`
delegates to `on`, so one-shot plugin subscriptions worked -- but only as
an undocumented consequence of rtc's internals. If `once` ever registered
directly instead, `session.once("openai_server_event_received", cb)` would
silently never fire.
Override `once()` to register the forwarder explicitly, making the
behaviour independent of how rtc implements it.
_FallbackRealtimeSessiononly re-attached the fixed_FORWARDED_EVENTSallowlist to each new child, so handlers for provider-specific events (e.g. openai'sopenai_server_event_received) were silently dropped byrestart_session()and by an availability fallback.Forwarders are now created on demand: subscribing on the wrapper registers a forwarder for that event, and
_bindreplays every forwarder onto each new child, so plugin-specific subscribers survive a swap like the generic ones already did. A_child_boundflag keeps a subscription made mid-swap off the child that is being discarded.Forwarders also became varargs, so a plugin event carrying more than one payload argument no longer loses all but the first.
Fixes #6559
Fixes #6556