diff --git a/pkg-py/src/querychat/_viz_tools.py b/pkg-py/src/querychat/_viz_tools.py index c57b297c..46bbb324 100644 --- a/pkg-py/src/querychat/_viz_tools.py +++ b/pkg-py/src/querychat/_viz_tools.py @@ -5,6 +5,7 @@ import base64 import copy import io +import warnings from typing import TYPE_CHECKING, Any, TypedDict from uuid import uuid4 @@ -151,6 +152,35 @@ def __init__( # --------------------------------------------------------------------------- +def warn_viz_url_bookmarking() -> bool: + """ + Warn (once) when URL bookmarking is active. + + With URL bookmarking each chart embeds a base64 PNG preview (for LLM + feedback) that can push the bookmarked state past URL length limits and + corrupt it on restore. Server-side bookmarking has no such limit. This + check is best-effort and must never interrupt chart creation. + """ + try: + session = get_current_session() + is_url = session is not None and session.bookmark.store == "url" + except Exception: + return False + if not is_url: + return False + try: + warnings.warn( + "URL bookmarking may not reliably restore visualizations. Each " + "chart embeds a PNG preview that can exceed URL length limits and " + "corrupt the saved state on restore. Consider server-side " + 'bookmarking instead: App(..., bookmark_store="server").', + stacklevel=2, + ) + except Exception: + return False + return True + + def visualize_impl( executor: QueryExecutor, update_fn: Callable[[VisualizeData], None], @@ -188,6 +218,8 @@ def visualize( except Exception: png_bytes = None + warn_viz_url_bookmarking() + update_fn( {"ggsql": ggsql, "title": title, "widget_id": altair_widget.widget_id} ) diff --git a/pkg-r/R/querychat_module.R b/pkg-r/R/querychat_module.R index efc9904b..cb1c7ff8 100644 --- a/pkg-r/R/querychat_module.R +++ b/pkg-r/R/querychat_module.R @@ -267,7 +267,7 @@ mod_server <- function( if (!is.null(state$values$querychat_viz_widgets)) { restored <- restore_viz_widgets( executor, - state$values$querychat_viz_widgets, + restore_record_list(state$values$querychat_viz_widgets), session ) viz_widgets <<- restored @@ -329,6 +329,27 @@ GREETING_PROMPT <- paste( "using the suggestion card format from your instructions." ) +# A list of records (named lists) bookmarked to the URL comes back from Shiny's +# decoder as a data.frame, because jsonlite simplifies a JSON array of objects +# (simplifyDataFrame = TRUE). Rebuild the list-of-lists shape row by row, +# dropping absent (NA) optional fields. A value restored from a server-side +# store (or already a list) is passed through unchanged. +restore_record_list <- function(x) { + if (is.null(x)) { + return(NULL) + } + if (is.data.frame(x)) { + return(lapply(seq_len(nrow(x)), function(i) { + row <- as.list(x[i, , drop = FALSE]) + row <- lapply(row, function(v) { + if (length(v) == 1 && is.na(v)) NULL else v + }) + row[!vapply(row, is.null, logical(1))] + })) + } + as.list(x) +} + restore_viz_widgets <- function(executor, saved_widgets, session) { if (!rlang::is_installed("ggsql")) { warning( diff --git a/pkg-r/R/querychat_viz.R b/pkg-r/R/querychat_viz.R index a1b084fe..c84d15bd 100644 --- a/pkg-r/R/querychat_viz.R +++ b/pkg-r/R/querychat_viz.R @@ -78,6 +78,7 @@ visualize_result <- function( viz_container <- NULL if (!is.null(session)) { + warn_viz_url_bookmarking() session$output[[widget_id]] <- ggsql::renderGgsql(spec) viz_container <- htmltools::div( class = "querychat-viz-container", @@ -146,19 +147,47 @@ visualize_result <- function( ) extra <- list( display = list( - html = viz_container, + html = freeze_tags(viz_container), title = if (nzchar(title)) title else "Query Visualization", show_request = FALSE, open = querychat_tool_starts_open("visualize"), full_screen = TRUE, icon = viz_icon(), - footer = footer + footer = freeze_tags(footer) ) ) ellmer::ContentToolResult(value = value, extra = extra) } +# Best-effort, additive-only check: with URL bookmarking each chart embeds a +# base64 PNG preview (for LLM feedback) that can push the bookmarked state past +# URL length limits and corrupt it on restore. Recommend server-side +# bookmarking, which has no such limit. Detection and the warning are wrapped so +# this never interrupts chart creation, and the warning fires only once. +warn_viz_url_bookmarking <- function() { + is_url <- tryCatch( + identical(shiny::getShinyOption("bookmarkStore", ""), "url"), + error = function(e) FALSE + ) + if (!isTRUE(is_url)) { + return(invisible(FALSE)) + } + tryCatch( + cli::cli_warn( + c( + "URL bookmarking may not reliably restore visualizations.", + "i" = "Each chart embeds a PNG preview that can exceed URL length limits and corrupt the saved state on restore.", + "i" = "Consider server-side bookmarking instead: {.code shiny::enableBookmarking(\"server\")}." + ), + .frequency = "once", + .frequency_id = "querychat_viz_url_bookmarking" + ), + error = function(e) NULL + ) + invisible(TRUE) +} + collapse_validation_errors <- function(validated) { errors <- validated$errors if (is.null(errors) || !nrow(errors)) { diff --git a/pkg-r/R/utils-html.R b/pkg-r/R/utils-html.R new file mode 100644 index 00000000..2ef05b29 --- /dev/null +++ b/pkg-r/R/utils-html.R @@ -0,0 +1,22 @@ +# Resolve a tag's render hooks now and return inert HTML plus its html +# dependencies. shinychat bookmarks chat turns with jsonlite::serializeJSON() +# and restores them with unserializeJSON(), which rebuilds any embedded closure +# from deparsed text and drops its environment. A live bslib/htmltools tag (e.g. +# input_code_editor()) carries render hooks that close over package internals, +# so after the round-trip re-rendering fails with "could not find function". +# Freezing here -- while the hooks can still reach those internals -- keeps only +# inert HTML and html_dependency objects, which survive the round-trip. +# See posit-dev/shinychat#261. +freeze_tags <- function(tags) { + if (is.null(tags) || is.character(tags)) { + return(tags) + } + rendered <- htmltools::renderTags(tags) + htmltools::tagList( + htmltools::HTML(rendered$html), + if (nzchar(rendered$head)) { + htmltools::tags$head(htmltools::HTML(rendered$head)) + }, + rendered$dependencies + ) +}