feat(mod): Add set_client() and status to chat_mod_server() return value#227
feat(mod): Add set_client() and status to chat_mod_server() return value#227gadenbuie wants to merge 5 commits into
set_client() and status to chat_mod_server() return value#227Conversation
Allow replacing the chat client after module initialization. The new `set_client(new_client, sync = TRUE)` function swaps the internal client reference used by all module closures. When `sync` is TRUE, turns, system prompt, and tools are copied from the old client to the new one. If called while a stream is in progress, the swap is deferred until the stream completes. Bookmarking is re-registered with the new client. The return value is now a locked environment with an active binding for `client`, so `chat$client` always reflects the current client.
Add `restore_ui` param to `chat_restore()` to control whether existing client turns are rendered into the chat UI on registration. `do_swap()` passes `restore_ui = FALSE` to avoid duplicating the already-displayed conversation when re-registering bookmark callbacks. Rename the `"update_last_turn"` observer to `"on_stream_complete"` to reflect that it now also handles deferred client swaps.
There was a problem hiding this comment.
Pull request overview
Adds the ability to hot-swap the ellmer chat client in chat_mod_server() and exposes a status reactive describing whether a stream is in progress. The module's return value also changes from a list to a locked environment with client as an active binding, and chat_restore() gains a restore_ui flag so re-registration after a swap can skip rendering the existing turns.
Changes:
- New
set_client(new_client, sync = TRUE)andstatusexposed bychat_mod_server(), with mid-stream swaps deferred via apending_swapreactive. chat_mod_server()now returns a locked environment;clientis exposed viamakeActiveBinding()so callers always see the current client.chat_restore()gains arestore_uiargument used by the swap path to re-register bookmark callbacks without re-rendering UI.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg-r/R/chat_app.R | Implements set_client, swap_client, status reactive, deferred-swap observer, and switches return type to a locked environment with client as active binding. |
| pkg-r/R/chat_restore.R | Adds restore_ui parameter to allow skipping initial UI rendering when re-registering after a client swap. |
| pkg-r/man/chat_app.Rd | Regenerated docs reflecting the new environment return value, status, and set_client(). |
| pkg-r/man/chat_restore.Rd | Regenerated docs for the new restore_ui parameter. |
Files not reviewed (2)
- pkg-r/man/chat_app.Rd: Language not supported
- pkg-r/man/chat_restore.Rd: Language not supported
Comments suppressed due to low confidence (1)
pkg-r/R/chat_app.R:267
- After
swap_client()runs (either inline fromset_client()or via the deferred path in this observer), it setsclient <<- new_clientand changespending_swap()toNULL. Because this observer takes a reactive dependency onpending_swap(), it will re-execute. At that point,append_stream_task$status()is still"success", solast_turn(client$last_turn())runs again — this time against the new client. Whensync = FALSE, the new client has no turns, solast_turnwill be silently overwritten withNULL, losing the last assistant turn that was just published to consumers. Consider gating thelast_turnupdate so it only fires when the task status actually transitions to success (e.g. track the previously-seen status), or only run the swap branch when status is not "success" / when there is no fresh result to publish.
shiny::observe(label = "on_stream_complete", {
status <- append_stream_task$status()
swap <- pending_swap()
if (status == "success") {
last_turn(client$last_turn())
}
if (!is.null(swap) && status != "running") {
pending_swap(NULL)
swap_client(swap$client, swap$sync)
}
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set_client <- function(new_client, sync = TRUE) { | ||
| check_ellmer_chat(new_client) | ||
|
|
||
| if (append_stream_task$status() == "running") { | ||
| pending_swap(list(client = new_client, sync = sync)) | ||
| return(invisible()) | ||
| } | ||
|
|
||
| swap_client(new_client, sync) | ||
| } |
| swap_client <- function(new_client, sync) { | ||
| if (sync) { | ||
| new_client$set_turns(client$get_turns()) | ||
| new_client$set_system_prompt(client$get_system_prompt()) | ||
| new_client$set_tools(client$get_tools()) | ||
| } | ||
| client <<- new_client | ||
| chat_restore( | ||
| "chat", | ||
| client, | ||
| session = session, | ||
| bookmark_on_input = bookmark_on_input, | ||
| bookmark_on_response = bookmark_on_response, | ||
| restore_ui = FALSE | ||
| ) | ||
| invisible() | ||
| } |
| ret$last_turn <- shiny::reactive(last_turn(), label = "mod_last_turn") | ||
| ret$last_input <- shiny::reactive(last_input(), label = "mod_last_input") | ||
| ret$status <- shiny::reactive(label = "mod_status", { | ||
| if (append_stream_task$status() == "running") "streaming" else "idle" |
There was a problem hiding this comment.
I considered this during the design phase and decided that we only need idle and streaming for now
| chat_restore( | ||
| "chat", | ||
| client, | ||
| session = session, | ||
| bookmark_on_input = bookmark_on_input, | ||
| bookmark_on_response = bookmark_on_response, | ||
| restore_ui = FALSE | ||
| ) |
Summary
set_client(new_client, sync = TRUE)to the environment returned bychat_mod_server(), allowing the parent server to hot-swap the chat client (e.g. when a user switches models). Whensync = TRUE, conversation turns, system prompt, and tools are copied from the old client to the new one. Mid-stream swaps are safely deferred until the stream completes.statusreactive ("idle"/"streaming") so the parent can observe the chat interaction state.clientfrom a plain list element to an active binding that always reflects the current client, and changes the return type fromlist()to a lockedenvironment.restore_uiparameter tochat_restore()to support re-registering bookmark callbacks after a client swap without re-rendering the full conversation UI.Verification