diff --git a/pkg-r/NEWS.md b/pkg-r/NEWS.md index d157e533..c9b28036 100644 --- a/pkg-r/NEWS.md +++ b/pkg-r/NEWS.md @@ -1,5 +1,7 @@ # querychat (development version) +* `querychat_app()` will now only automatically clean up the data source if QueryChat creates the data source internally from a data frame. (#164) + * **Breaking change:** The `$sql()` method now returns `NULL` instead of `""` (empty string) when no query has been set, aligning with the behavior of `$title()` for consistency. Most code using `isTruthy()` or similar falsy checks will continue working without changes. Code that explicitly checks `sql() == ""` should be updated to use falsy checks (e.g., `!isTruthy(sql())`) or explicit null checks (`is.null(sql())`). (#146) * Tool detail cards can now be expanded or collapsed by default when querychat runs a query or updates the dashboard via the `querychat.tool_details` R option or the `QUERYCHAT_TOOL_DETAILS` environment variable. Valid values are `"expanded"`, `"collapsed"`, or `"default"`. (#137) diff --git a/pkg-r/R/QueryChat.R b/pkg-r/R/QueryChat.R index 6ed12a8d..47e7338b 100644 --- a/pkg-r/R/QueryChat.R +++ b/pkg-r/R/QueryChat.R @@ -166,9 +166,6 @@ QueryChat <- R6::R6Class( private$.data_source <- normalize_data_source(data_source, table_name) - # Validate table name - check_sql_table_name(table_name) - self$id <- id %||% table_name if (!is.null(greeting) && file.exists(greeting)) { @@ -192,7 +189,7 @@ QueryChat <- R6::R6Class( # By default, only close automatically if a Shiny session is active if (is.na(cleanup)) { - cleanup <- !is.null(shiny::getDefaultReactiveDomain()) + cleanup <- shiny::isRunning() } if (cleanup) { @@ -585,8 +582,11 @@ QueryChat <- R6::R6Class( #' used. See the package prompts directory for the default template format. #' @param cleanup Whether or not to automatically run `$cleanup()` when the #' Shiny session/app stops. By default, cleanup only occurs if `QueryChat` -#' gets created within a Shiny session. Set to `TRUE` to always clean up, -#' or `FALSE` to never clean up automatically. +#' is created within a Shiny app. Set to `TRUE` to always clean up, or +#' `FALSE` to never clean up automatically. +#' +#' In `querychat_app()`, in-memory databases created for data frames are +#' always cleaned up. #' #' @return A `QueryChat` object. See [QueryChat] for available methods. #' @@ -668,13 +668,26 @@ querychat_app <- function( categorical_threshold = 20, extra_instructions = NULL, prompt_template = NULL, - cleanup = TRUE, + cleanup = NA, bookmark_store = "url" ) { + if (shiny::isRunning()) { + cli::cli_abort( + "{.fn querychat_app} cannot be called from within a Shiny app. Use {.fn querychat} instead." + ) + } + if (is_missing(table_name) && is.data.frame(data_source)) { table_name <- deparse1(substitute(data_source)) } + check_bool(cleanup, allow_na = TRUE) + if (is.data.frame(data_source)) { + cleanup <- TRUE + } else if (is.na(cleanup)) { + cleanup <- FALSE + } + qc <- QueryChat$new( data_source = data_source, table_name = table_name, @@ -697,6 +710,8 @@ normalize_data_source <- function(data_source, table_name) { return(data_source) } + check_sql_table_name(table_name, call = caller_env()) + if (is.data.frame(data_source)) { return(DataFrameSource$new(data_source, table_name)) } diff --git a/pkg-r/R/utils-shiny.R b/pkg-r/R/utils-shiny.R new file mode 100644 index 00000000..893a57a1 --- /dev/null +++ b/pkg-r/R/utils-shiny.R @@ -0,0 +1,3 @@ +in_shiny_session <- function() { + !is.null(shiny::getDefaultReactiveDomain()) # nocov +} diff --git a/pkg-r/man/querychat-convenience.Rd b/pkg-r/man/querychat-convenience.Rd index f07c03e4..2e176f08 100644 --- a/pkg-r/man/querychat-convenience.Rd +++ b/pkg-r/man/querychat-convenience.Rd @@ -30,7 +30,7 @@ querychat_app( categorical_threshold = 20, extra_instructions = NULL, prompt_template = NULL, - cleanup = TRUE, + cleanup = NA, bookmark_store = "url" ) } @@ -81,8 +81,11 @@ used. See the package prompts directory for the default template format.} \item{cleanup}{Whether or not to automatically run \verb{$cleanup()} when the Shiny session/app stops. By default, cleanup only occurs if \code{QueryChat} -gets created within a Shiny session. Set to \code{TRUE} to always clean up, -or \code{FALSE} to never clean up automatically.} +is created within a Shiny app. Set to \code{TRUE} to always clean up, or +\code{FALSE} to never clean up automatically. + +In \code{querychat_app()}, in-memory databases created for data frames are +always cleaned up.} \item{bookmark_store}{The bookmarking storage method. Passed to \code{\link[shiny:enableBookmarking]{shiny::enableBookmarking()}}. If \code{"url"} or \code{"server"}, the chat state diff --git a/pkg-r/man/querychat-package.Rd b/pkg-r/man/querychat-package.Rd index 63598eda..7c10e48a 100644 --- a/pkg-r/man/querychat-package.Rd +++ b/pkg-r/man/querychat-package.Rd @@ -75,11 +75,19 @@ Useful links: \itemize{ \item \url{https://posit-dev.github.io/querychat/pkg-r} \item \url{https://posit-dev.github.io/querychat} + \item \url{https://github.com/posit-dev/querychat} + \item Report bugs at \url{https://github.com/posit-dev/querychat/issues} } } \author{ -\strong{Maintainer}: Joe Cheng \email{joe@posit.co} +\strong{Maintainer}: Garrick Aden-Buie \email{garrick@posit.co} (\href{https://orcid.org/0000-0002-7111-0077}{ORCID}) + +Authors: +\itemize{ + \item Joe Cheng \email{joe@posit.co} [conceptor] + \item Carson Sievert \email{carson@posit.co} (\href{https://orcid.org/0000-0002-4958-2844}{ORCID}) +} Other contributors: \itemize{ diff --git a/pkg-r/tests/testthat/test-QueryChat.R b/pkg-r/tests/testthat/test-QueryChat.R index 91eb3847..526a41b6 100644 --- a/pkg-r/tests/testthat/test-QueryChat.R +++ b/pkg-r/tests/testthat/test-QueryChat.R @@ -306,3 +306,39 @@ describe("normalize_data_source()", { }) }) }) + +test_that("querychat_app() only cleans up data frame sources on exit", { + local_mocked_r6_class( + QueryChat, + public = list( + initialize = function(..., cleanup) { + # have to use an option because the code is evaluated in a far-away env + options(.test_cleanup = cleanup) + }, + app = function(...) {} + ) + ) + withr::local_options(rlang_interactive = TRUE) + + withr::with_options(list(.test_cleanup = NULL), { + test_df <- new_test_df() + querychat_app(test_df) + cleanup_result <- getOption(".test_cleanup") + expect_true(cleanup_result) + }) + + withr::with_options(list(.test_cleanup = NULL), { + test_ds <- local_data_frame_source(new_test_df()) + querychat_app(test_ds) + cleanup_result <- getOption(".test_cleanup") + expect_false(cleanup_result) + }) + + withr::with_options(list(.test_cleanup = NULL), { + con <- local_sqlite_connection(new_test_df()) + test_ds <- DBISource$new(con$conn, "test_table") + querychat_app(test_ds) + cleanup_result <- getOption(".test_cleanup") + expect_false(cleanup_result) + }) +})