feat: Add chat_enable_bookmarking(id, client, ..., update_on_input = TRUE, update_on_response = TRUE)

[schloerke]

Fixes #27

Reprex app:

library(shiny)
library(bslib)
library(shinychat)

ui <- function(...) {
  page_fillable(
    chat_ui("chat", fill = TRUE)
  )
}

server <- function(input, output, session) {
  chat_client <- ellmer::chat_ollama(
    system_prompt = "Important: Always respond in a limerick",
    model = "qwen2.5-coder:1.5b",
    echo = TRUE
  )
  # Let the UI know about the client
  chat_enable_bookmarking("chat", chat_client)

  observeEvent(input$chat_user_input, {
    stream <- chat_client$stream_async(input$chat_user_input)
    chat_append("chat", stream)
  })
}

# Enable bookmarking!
shinyApp(ui, server, enableBookmarking = "url")

[schloerke]

Talking with @icarusz and @wch , I am in agreement that we should change the API:

This change below (set_chat_client(id, client, ..., bookmark = "auto") -> chat_bookmark(id, client, ..., update_on_input = TRUE, update_on_response = TRUE) will only really change the interface. The main logic will remain.

library(shiny)
library(bslib)
library(shinychat)

ui <- function(...) {
  page_fillable(
    chat_ui("chat", fill = TRUE)
  )
}

server <- function(input, output, session) {
  chat_client <- ellmer::chat_ollama(
    system_prompt = "Important: Always respond in a limerick",
    model = "qwen2.5-coder:1.5b",
    echo = TRUE
  )

  # Let the UI know that shinychat should bookmark the chat_client
  shinychat::chat_bookmark("chat", chat_client) # update_on_input = TRUE, update_on_response = TRUE

  observeEvent(input$chat_user_input, {
    stream_promise <- chat_client$stream_async(input$chat_user_input)
    shinychat::chat_append("chat", stream_promise)
  })

}

# Enable bookmarking!
shinyApp(ui, server, enableBookmarking = "server")

If other features were to be made, they should have their own shinychat::chat_FEATURE() method.

[schloerke]

Remaining question

[schloerke]

@wch Looking for a review on the API: shinychat::chat_bookmark(ID, CLIENT) with default parameters of update_on_input = TRUE and update_on_response = TRUE.

These default parameters hide away the multiple calls to session$doBookmark().

Ex equivalent server code:

  shinychat::chat_bookmark("chat", chat_client, update_on_input = FALSE, update_on_response = FALSE)
  
  observeEvent(input$chat_user_input, {
    session$doBookmark()
    stream_promise <- chat_client$stream_async(input$chat_user_input)
	stream_promise$then(function(stream_value) { session$doBookmark() })
    shinychat::chat_append("chat", stream_promise)
  })

Final server code:

  shinychat::chat_bookmark("chat", chat_client)
  
  observeEvent(input$chat_user_input, {
    stream_promise <- chat_client$stream_async(input$chat_user_input)
    shinychat::chat_append("chat", stream_promise)
  })

[wch]

The new API does look better. A few questions:

• The name chat_bookmark makes it sound like running that function will do the bookmarking. Maybe chat_enable_bookmarking?
• Another thing we should consider: We want to use this mechanism to (A) do bookmarking in the usual way that Shiny does it (B) allow the user to save/restore multiple chats, similar to the ChatGPT web interface. Will this API make sense for the second case as well?
• Suppose an app uses update_on_input=TRUE and there's a disconnection before the response is completed. When the chat is restored, is there be a way in the UI re-send the message to the server and get a response? If not, then restoring that conversation could leave the app in an unusable state, I think. (We should have the ability to re-send a message anyway, but that's outside the scope of this PR.)

[schloerke]

✅ chat_enable_bookmarking()

[schloerke]

Will this API make sense for the second case as well?

If the chat is done within a single chat object, no. The data storage required is more complex than what a single ellmer object wants.

If the chat is done within a module where given the sidebar's chat selected id, we could have a dynamic chat module that loads a chat given the chat ID. I believe this is a plausible approach

[schloerke]

We also need to save the UI here

[schloerke]

We need to independently restore the UI here (not directly from ellmer)

[schloerke]

This should ideally be a {s7} JSON "pack" method, similar to jsonlite:::pack()