Remove commons_server() in favor of shinychat::chat_server() - #221
Draft
cpsievert wants to merge 2 commits into
Draft
Remove commons_server() in favor of shinychat::chat_server()#221cpsievert wants to merge 2 commits into
cpsievert wants to merge 2 commits into
Conversation
With shinychat managing conversation identity and history restores first-class, commons_server() no longer adds anything over shinychat::chat_server(). Remove it and make commons_theme() the sole building block for custom commons apps: - commons_app() composes page_chat(), commons_theme(), and chat_server() directly; the same composition is shown in the examples, vignette, and onboarding skill for custom deployments. - Conversation-id persistence (persist_conversation_id() and its history hooks) and the agent-side bookkeeping (get/set_conversation_id(), refresh_conversation_id(), last_streamed_turns) are deleted. ellmer stamps gen_ai.conversation.id on its own spans from the client's conversation_id binding, which shinychat allocates at first submission; span_conversation_id() still walks the ancestor chain so older traces with the id on commons' wrapper span remain readable. - check_chat_packages() goes too: htmltools and shinychat are in Imports, and shinychat itself Imports shiny, so every package the chat functions touch is guaranteed installed with commons. - commons_theme() moves from theme.R into chat.R.
commons_server() used to kick off pre-warming itself; with it gone, export a helper so custom apps get the same behavior with one call. commons_prewarm(agent) validates the agent and defers prewarm() to post-startup idle time, and is used by commons_app() and throughout the examples, vignette, and onboarding skill. The error contract is split by call site. A direct agent$prewarm() is typically warming caches ahead of deployment, so failures propagate: a cold cache should fail the deploy, and a warning would sail through a deploy script. commons_prewarm() downgrades failures to warnings, since pre-warming is a pure optimization (everything it builds is rebuilt lazily at first use) and an error escaping a later::later() callback would stop the Shiny app.
Collaborator
Author
|
@simonpcouch I think I'd also be open to just keeping |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #202.
With shinychat now managing conversation identity and history restores first-class,
commons_server()no longer adds much on top ofshinychat::chat_server(), so this PR removes it. Custom apps now composeshinychat::chat_server()directly, and the idle-time pre-warm it used to do is exposed as an exportedcommons_prewarm()helper:The two call sites differ in error handling: a direct
agent$prewarm()— typically cache warming ahead of deployment — lets failures propagate, so a cold cache fails the deploy;commons_prewarm()downgrades failures to warnings, since pre-warming is a pure optimization (everything it builds is rebuilt lazily at first use) and an error escaping alater::later()callback would stop the whole Shiny app.Testing
prewarm()propagates failures;commons_prewarm()downgrades a prewarm failure to a warning (driven throughlater::run_now());commons_app()prewarms the agent on idle; a commons agent runs undershinychat::chat_server()viashiny::testServer().commons_server()tests.