diff --git a/pkg-r/DESCRIPTION b/pkg-r/DESCRIPTION index 9b770cbf..67a487c6 100644 --- a/pkg-r/DESCRIPTION +++ b/pkg-r/DESCRIPTION @@ -27,6 +27,7 @@ Imports: duckdb (>= 1.5.4.2), ellmer (>= 0.4.1), evaluate, + filelock, glue, highr, htmltools, @@ -52,6 +53,7 @@ Suggests: dplyr, ggplot2, gt, + nanoparquet, odbc, otel (>= 0.2.0), otelsdk (>= 0.2.0), diff --git a/pkg-r/NAMESPACE b/pkg-r/NAMESPACE index c9004cba..94ac6310 100644 --- a/pkg-r/NAMESPACE +++ b/pkg-r/NAMESPACE @@ -2,6 +2,7 @@ export(commons) export(commons_app) +export(commons_prewarm) export(commons_server) export(commons_theme) export(context_layer) diff --git a/pkg-r/R/chat.R b/pkg-r/R/chat.R index 7d1bba23..0161450d 100644 --- a/pkg-r/R/chat.R +++ b/pkg-r/R/chat.R @@ -111,14 +111,7 @@ commons_server <- function(id, client, ...) { attributes = list("commons.server.id" = id) ) - # Build the context index and start the background pin-cache download - # during post-startup idle time (while the user reads the welcome message). - # Errors are swallowed: the first search retries the index build and - # surfaces the failure to the model, and an unwarmed pin is simply - # downloaded at its first use. - later::later(function() { - tryCatch(client$prewarm(), error = function(err) NULL) - }) + commons_prewarm(client) chat <- shinychat::chat_server(id, client = client, ...) # shinychat owns the conversation identity (it sets the client's @@ -130,6 +123,99 @@ commons_server <- function(id, client, ...) { chat } +#' Pre-warm a commons agent during post-startup idle time +#' +#' A [commons()] agent defers two kinds of setup to first use, and exposes a +#' `prewarm()` method for each so you can move the cost off the first +#' question: +#' +#' * `agent$prewarm_context()` builds the context index (the store behind +#' `search_context`). The index is a persistent, content-addressed file, +#' so the build happens once per content version: later sessions open it +#' in milliseconds, and it can be built offline ahead of deployment. The +#' cache root resolves from the `commons.context_cache` option, the +#' `COMMONS_CONTEXT_CACHE` or `CONNECT_CONTENT_DATA_DIR` environment +#' variables, an `app_cache/commons` directory beside a Shiny app, or the +#' per-user cache directory, in that order. This resolution ladder (and +#' the content-addressed, age-pruned cache design generally) follows the +#' file cache in the sass package, which has run in production Shiny +#' deployments for years. Note that `app_cache/` is +#' excluded from deployed bundles (rsconnect treats it as server-side +#' scratch), so it is shared across the sessions of one deployment but +#' rebuilt after a redeploy; to ship a pre-built store with the app, +#' point `commons.context_cache` at a directory inside the app and run +#' `prewarm_context()` before deploying. The cache is capped at 256 MB +#' with least-recently-used eviction (option +#' `commons.context_cache_max_size`, in bytes; a single store larger than +#' the cap is kept, with a warning). Set +#' `options(commons.context_cache = FALSE)` (or the +#' `COMMONS_CONTEXT_CACHE` environment variable to `false`) to disable +#' persistence entirely. +#' * `agent$prewarm_sources()` starts a background process that downloads +#' any uncached pins into the local pins cache (see [data_source()]). +#' Because the pins cache is on disk, this can also run ahead of +#' deployment — outside the Shiny runtime entirely — and the deployed +#' app reads the warmed cache. +#' +#' `agent$prewarm()` calls both. Call `commons_prewarm()` in a Shiny server +#' function to defer warming to post-startup idle time, so it happens while +#' the user reads the welcome message. Outside a running Shiny app (e.g. a +#' pre-deploy warm-up script) there is no [later::later()] event loop, so +#' `commons_prewarm()` warms synchronously instead. Note that +#' `commons_prewarm()` always downgrades failures to warnings (see below), +#' even on this synchronous path — a pre-deploy script that should fail the +#' deploy on a cold cache must call `agent$prewarm()` directly. +#' +#' `prewarm()` lets failures propagate, since a direct call is typically +#' warming caches ahead of deployment and a mere warning would sail through +#' a deploy script. `commons_prewarm()` downgrades such failures to +#' warnings: pre-warming is a pure optimization — everything it builds is +#' rebuilt lazily at first use — and an error escaping the [later::later()] +#' callback would stop the app. +#' +#' @param client A [commons()] agent. +#' +#' @return `NULL`, invisibly. +#' +#' @examples +#' \dontrun{ +#' server <- function(input, output, session) { +#' agent <- commons( +#' ellmer::chat_anthropic(), +#' data_sources = data_source(sales = sales) +#' ) +#' commons_prewarm(agent) +#' shinychat::chat_server("chat", client = agent) +#' } +#' } +#' +#' @export +commons_prewarm <- function(client) { + check_commons_client(client) + # An error escaping a later::later() callback stops the Shiny app, and + # pre-warming is a pure optimization, so downgrade failures to warnings. + warm <- function() { + tryCatch( + client$prewarm(), + error = function(err) { + # Assign first: the raw message can contain braces (DuckDB errors + # embed JSON), which cli would try to interpolate -- and an error + # escaping this handler would stop the app. + msg <- conditionMessage(err) + cli::cli_warn("{msg}") + } + ) + } + # later::later() only fires while an event loop is running; outside Shiny + # (e.g. a pre-deploy warm-up script) the callback would never run. + if (is_shiny_app()) { + later::later(warm) + } else { + warm() + } + invisible(NULL) +} + check_chat_packages <- function(call = rlang::caller_env()) { missing <- c("htmltools", "shiny", "shinychat")[ !vapply( diff --git a/pkg-r/R/commons.R b/pkg-r/R/commons.R index 92da5671..17769129 100644 --- a/pkg-r/R/commons.R +++ b/pkg-r/R/commons.R @@ -390,6 +390,18 @@ Commons <- R6::R6Class( }, prewarm = function() { + # Pre-warming is a pure optimization (everything it builds is rebuilt + # or downloaded lazily at first use), but a direct call is typically + # warming caches ahead of deployment, so failures propagate: a cold + # cache should fail the deploy. commons_prewarm() downgrades failures + # to warnings for the Shiny idle-time path, where an escaping error + # would stop the app. + self$prewarm_context() + self$prewarm_sources() + invisible(self) + }, + + prewarm_context = function() { layer <- private$context_layer layer_state <- if (is.null(layer)) NULL else context_layer_state(layer) if (!is.null(layer_state) && length(layer_state$docs) > 0) { @@ -397,11 +409,27 @@ Commons <- R6::R6Class( "commons_context_prewarm", attributes = list( "commons.context.n_docs" = length(layer_state$docs), - "commons.context.cache_hit" = !is.null(layer_state$store) + # Probe with the side-effect-free resolver so recording telemetry + # can't create directories or trigger the fallback warning, and + # tryCatch so a resolution failure can't abort prewarming. + "commons.context.cache_hit" = + !is.null(layer_state$store) || + isTRUE(tryCatch( + context_cache_enabled() && + file.exists(context_store_path( + layer_state$docs, + cache_dir = context_cache_dir() + )), + error = function(err) FALSE + )) ) ) context_store(layer) } + invisible(self) + }, + + prewarm_sources = function() { for (source in private$sources) { source_prewarm(source) } diff --git a/pkg-r/R/context-layer.R b/pkg-r/R/context-layer.R index 2c352ccd..4d34b526 100644 --- a/pkg-r/R/context-layer.R +++ b/pkg-r/R/context-layer.R @@ -106,25 +106,316 @@ strip_frontmatter <- function(md) { sub("(?s)^---\r?\n.*?\r?\n---(\r?\n|$)", "", md, perl = TRUE) } -# Store setup (duckdb creation, chunk insertion, FTS indexing) is the most -# expensive part of building an agent and many conversations never search, so -# it's deferred to the first search. Aliases of one layer share its store; -# augmenting its documents creates a layer with a fresh store. +# The context store is a persistent, content-addressed DuckDB file: the key +# hashes the layer's docs plus the ragnar/duckdb versions (whose file format +# the store depends on), so a build happens once per content version per +# cache root and every process afterwards opens it read-only. Store setup is +# still deferred to the first search (or prewarm_context()) since many +# conversations never search, but on a warm cache that first search only +# pays for opening a file. Aliases of one layer share its store; augmenting +# its documents creates a layer with a fresh store. +# Housekeeping state for the persistent context cache: prune throttling, the +# fallback tempdir, and the one-time warning about an unusable cache dir. +context_cache_state <- new.env(parent = emptyenv()) + context_store <- function(layer) { state <- context_layer_state(layer) - if (is.null(state$store)) { - local_commons_span( - "commons_context_store_build", - attributes = list("commons.context.n_docs" = length(state$docs)) + if (!is.null(state$store)) { + return(state$store) + } + if (!context_cache_enabled()) { + store <- build_context_store_memory(state$docs) + state$store <- store + return(store) + } + path <- context_store_path(state$docs) + if (!file.exists(path)) { + build_context_store(state$docs, path) + } else { + # Touch the mtime so size-cap eviction is LRU: stores in active use + # stay young. Best-effort -- a read-only cache dir still opens fine. + tryCatch(Sys.setFileTime(path, Sys.time()), error = function(err) NULL) + } + store <- tryCatch( + ragnar::ragnar_store_connect(path), + error = function(err) err + ) + if (inherits(store, "error")) { + # Typically a concurrent pruner unlinked the store between our + # file.exists() and the connect; a corrupt store file is also possible. + # Warn (and notify in Shiny, where a warning is easy to miss), then + # rebuild once. A second failure propagates, so persistent corruption + # still surfaces rather than being silently rebuilt every session. + context_store_connect_warning(path, store) + unlink(path) + build_context_store(state$docs, path) + store <- ragnar::ragnar_store_connect(path) + } + state$store <- store + store +} + +context_store_connect_warning <- function(path, err) { + # Assign first: the raw message can contain braces (DuckDB errors embed + # JSON), which cli would try to interpolate. + detail <- conditionMessage(err) + cli::cli_warn(c( + "Failed to open the cached context store at {.path {path}}; rebuilding it.", + i = "{detail}" + )) + if (is_shiny_app()) { + tryCatch( + shiny::showNotification( + "The context index is being rebuilt; the first search may be slow.", + type = "warning", + duration = 8 + ), + error = function(err) NULL + ) + } +} + +# options(commons.context_cache = FALSE) disables the persistent store (the +# index is built in memory per layer instead) -- an escape hatch for +# development loops over context files. +context_cache_enabled <- function() { + if (identical(getOption("commons.context_cache"), FALSE)) { + return(FALSE) + } + # Env vars can't express FALSE; accept the usual spellings. + val <- Sys.getenv("COMMONS_CONTEXT_CACHE", unset = NA_character_) + if (!is.na(val) && tolower(val) %in% c("false", "0", "no")) { + return(FALSE) + } + TRUE +} + +build_context_store_memory <- function(docs) { + local_commons_span( + "commons_context_store_build", + attributes = list( + "commons.context.n_docs" = length(docs), + "commons.context.persistent" = FALSE ) - store <- ragnar::ragnar_store_create(embed = NULL) - for (doc in state$docs) { - ragnar::ragnar_store_insert(store, ragnar::markdown_chunk(doc)) + ) + store <- ragnar::ragnar_store_create(embed = NULL) + for (doc in docs) { + ragnar::ragnar_store_insert(store, ragnar::markdown_chunk(doc)) + } + ragnar::ragnar_store_build_index(store, type = "fts") + store +} + +# Build to a temp file in the same directory, then rename into place +# atomically, so a concurrent reader or builder never observes a partial +# store. If another builder wins the race, its store is equivalent content; +# discard ours and open theirs. +build_context_store <- function(docs, path) { + local_commons_span( + "commons_context_store_build", + attributes = list( + "commons.context.n_docs" = length(docs), + "commons.context.persistent" = TRUE + ) + ) + dir.create(dirname(path), recursive = TRUE, showWarnings = FALSE) + tmp <- tempfile(pattern = ".build-", tmpdir = dirname(path)) + on.exit(unlink(tmp, recursive = TRUE), add = TRUE) + + store <- ragnar::ragnar_store_create(tmp, embed = NULL) + for (doc in docs) { + ragnar::ragnar_store_insert(store, ragnar::markdown_chunk(doc)) + } + ragnar::ragnar_store_build_index(store, type = "fts") + DBI::dbDisconnect(store@con, shutdown = TRUE) + + if (!file.exists(path)) { + file.rename(tmp, path) + } + if (!file.exists(path)) { + cli::cli_abort("Failed to build the context store at {.path {path}}.") + } + prune_context_cache(dirname(path), protect = path) + invisible(path) +} + +# Content-addressed stores accumulate one file per content version, so the +# cache is capped by total size and pruned LRU: the mtime touch on open +# keeps actively used stores young, so eviction deletes least-recently-used +# stores first. The store just built is protected explicitly (not just by +# its young mtime) so a store larger than the cap survives: like cachem, a +# single oversized store is kept, with a one-time warning, rather than +# evicted into a rebuild loop. Throttled like cachem (at most once per 20 +# builds or per 5 seconds) since stat-ing the directory on every build is +# needlessly slow; concurrent pruners may double-delete, which unlink +# tolerates with a warning. +prune_context_cache <- function( + dir, + max_size = getOption("commons.context_cache_max_size", 256 * 1024^2), + protect = NULL +) { + now <- Sys.time() + context_cache_state$n_builds <- (context_cache_state$n_builds %||% 0) + 1 + last <- context_cache_state$last_prune + throttled <- context_cache_state$n_builds %% 20 != 0 && + !is.null(last) && + difftime(now, last, units = "secs") < 5 + if (throttled) { + return(invisible()) + } + context_cache_state$last_prune <- now + + reap_stale_build_files(dir, now) + + stores <- list.files(dir, pattern = "[.]duckdb$", full.names = TRUE) + total <- sum(file.size(stores)) + if (total > max_size) { + evictable <- setdiff(stores, protect) + evictable <- evictable[order(file.mtime(evictable))] + for (victim in evictable) { + if (total <= max_size) { + break + } + size <- file.size(victim) + # Only count the eviction when the unlink actually happened -- on + # Windows, deleting a store another process holds open fails. + if (suppressWarnings(unlink(victim)) == 0) { + total <- total - size + } } - ragnar::ragnar_store_build_index(store, type = "fts") - state$store <- store + if (total > max_size && is.null(context_cache_state$warned_size)) { + context_cache_state$warned_size <- TRUE + cli::cli_warn(c( + "The context cache exceeds its size cap ({format_size(max_size)}) with only protected or in-use stores remaining.", + i = "A single store larger than the cap is kept; raise {.code options(commons.context_cache_max_size)} if this is expected." + )) + } + } + invisible() +} + +# build_context_store() builds at a `.build-*` temp file that the size-cap +# pruner never sees (it lists `*.duckdb`), so a crashed or killed build would +# otherwise leak its partial store forever. Reap temp files older than a day: +# young enough to clear debris promptly, old enough to never delete a build +# that is still in flight. +reap_stale_build_files <- function(dir, now, max_age = 24 * 60 * 60) { + stale <- list.files( + dir, + pattern = "^[.]build-", + all.files = TRUE, + full.names = TRUE + ) + age <- difftime(now, file.mtime(stale), units = "secs") + # file.mtime() is NA for a file a concurrent process just deleted + old <- stale[!is.na(age) & age > max_age] + suppressWarnings(unlink(old, recursive = TRUE)) + invisible() +} + +format_size <- function(bytes) { + if (bytes >= 1024^2) { + sprintf("%.0f MB", bytes / 1024^2) + } else { + sprintf("%.0f KB", bytes / 1024) + } +} + +# `cache_dir` is injectable so read-only probes (e.g. the cache_hit span +# attribute in prewarm_context()) can pass context_cache_dir() and avoid the +# fallback tempdir's side effects (directory creation, one-time warning). +context_store_path <- function(docs, cache_dir = context_cache_dir_safe()) { + key <- rlang::hash(c( + docs, + paste0("ragnar:", utils::packageVersion("ragnar")), + paste0("duckdb:", utils::packageVersion("duckdb")) + )) + file.path(cache_dir, "context", paste0(key, ".duckdb")) +} + +# Cache root resolution: an explicit override, then Connect's persistent +# data directory (survives deployments when the server enables it), then -- +# for Shiny apps -- an app_cache/ directory beside the app (sass's +# convention: per-app scoping on hosted platforms, used locally only if it +# already exists), then the per-user cache dir. Wherever the root is +# ephemeral (e.g. Connect Cloud, which resets disk to the deployed bundle), +# the store simply rebuilds once per cache lifetime instead of once per +# process. +context_cache_dir <- function() { + opt <- getOption("commons.context_cache") + if (!is.null(opt) && !identical(opt, FALSE)) { + if (!rlang::is_string(opt)) { + cli::cli_abort( + "{.code options(commons.context_cache)} must be a path to a cache directory or {.code FALSE}." + ) + } + return(opt) + } + for (env in c("COMMONS_CONTEXT_CACHE", "CONNECT_CONTENT_DATA_DIR")) { + val <- Sys.getenv(env, unset = NA_character_) + false_like <- !is.na(val) && tolower(val) %in% c("false", "0", "no") + if (!is.na(val) && nzchar(val) && !false_like) { + return(val) + } + } + if (is_shiny_app()) { + app_dir <- shiny::getShinyOption("appDir") + if (!is.null(app_dir)) { + app_cache <- file.path(app_dir, "app_cache", "commons") + if ( + is_hosted_shiny_app() || + dir.exists(app_cache) || + dir.exists(dirname(app_cache)) + ) { + return(app_cache) + } + } + } + tools::R_user_dir("commons", "cache") +} + +is_shiny_app <- function() { + isNamespaceLoaded("shiny") && shiny::isRunning() +} + +# Connect and Shiny Server both set SHINY_SERVER_VERSION for content. +is_hosted_shiny_app <- function() { + nzchar(Sys.getenv("SHINY_SERVER_VERSION")) && is_shiny_app() +} + +# Caching must never take down the app: if the resolved cache dir can't be +# created or written, warn once and fall back to a per-session tempdir (the +# store becomes per-process, as if persistence were disabled). +context_cache_dir_safe <- function() { + dir <- context_cache_dir() + # Probe with an actual write: file.access() checks DOS attributes rather + # than ACLs on Windows, so it can report an unwritable dir as writable. + ok <- tryCatch( + { + dir.create(dir, recursive = TRUE, showWarnings = FALSE) + # tempfile() warns (not errors) when dir isn't a directory + dir.exists(dir) && { + probe <- tempfile(tmpdir = dir) + file.create(probe) && unlink(probe) == 0 + } + }, + error = function(err) FALSE + ) + if (ok) { + return(dir) + } + if (is.null(context_cache_state$warned)) { + context_cache_state$warned <- TRUE + cli::cli_warn(c( + "Cannot write to the context cache directory {.path {dir}}.", + i = "Falling back to a per-session temporary directory; the context index will be rebuilt in each process." + )) + } + if (is.null(context_cache_state$fallback_dir)) { + context_cache_state$fallback_dir <- tempfile("commons-context-cache-") + dir.create(context_cache_state$fallback_dir, recursive = TRUE) } - state$store + context_cache_state$fallback_dir } context_search <- function(layer, query, n = 3) { diff --git a/pkg-r/R/data-source.R b/pkg-r/R/data-source.R index c1437429..a4dda4c7 100644 --- a/pkg-r/R/data-source.R +++ b/pkg-r/R/data-source.R @@ -12,13 +12,21 @@ #' * A `pins` board, e.g. [pins::board_connect()], is read into the same #' in-process database: each pin in `tables` becomes a table. Pin names are #' validated against the board at construction (a single listing call), but -#' each pin is downloaded only when its table is first used. -#' [commons_server()] starts a background process right after startup that -#' downloads the remaining pins into the local pins cache, so a first use -#' typically only reads an already-downloaded file. A table reflects the pin's -#' value at first use and is not refreshed for the lifetime of the data -#' source; if a pin can't be read (e.g. a network failure), the error surfaces -#' at that first use and the read is retried on the next one. +#' each pin is downloaded only when its table is first used. Parquet and +#' CSV pins are then queried in place -- a DuckDB view over the downloaded +#' file, with no copy into the database -- while other formats (e.g. RDS) +#' are read and loaded at first use. Calling the agent's +#' `prewarm_sources()` method (see [commons_prewarm()]) starts a background +#' process that downloads the remaining pins into the local pins cache, so +#' a first use typically only reads an already-downloaded file. Since the +#' pins cache is on disk, `prewarm_sources()` can also run ahead of +#' deployment to warm the cache the deployed app will read. A table +#' reflects the pin version resolved at first use and is not refreshed for +#' the lifetime of the data source; if the board deletes that version +#' (e.g. a rewrite on a non-versioned board), the next query re-resolves +#' the latest version. If a pin can't be read (e.g. a network failure), +#' the error surfaces at that first use and the read is retried on the +#' next one. #' #' @param ... A single DBI connection, a single `pins` board, or named data #' frames to register as tables. When passing data frames, each name becomes @@ -330,9 +338,12 @@ data_source_board <- function( # Lock the connection down before any writes; lock_configuration() only # freezes SET statements, so later dbWriteTable() from a deferred read still - # works. + # works. Views over pin files read straight from the board's directory + # (folder boards) or local cache (remote boards), so allowlist exactly that + # root: the connection can read pin files and nothing else. + root <- board_pin_root(board) con <- duckdb_connect() - duckdb_lock_down(con) + duckdb_lock_down(con, allow_dirs = root) check_labels_free(con, names(tables), call = call) new_data_source( @@ -340,19 +351,38 @@ data_source_board <- function( names(tables), owned = TRUE, dictionary = dictionary, - pending = new_pending_pins(board, tables) + pending = new_pending_pins(board, tables, root) ) } +# The directory pin files for this board live under: the local cache for +# remote boards (connect, s3, ...), the board's own directory for folder +# boards. NULL when the board's layout is unknown, which disables the +# zero-copy view path (pins load eagerly instead). The field order matters: +# `cache` wins because remote boards keep downloaded files there, while +# folder boards have no cache and only set `path`. +board_pin_root <- function(board) { + for (field in c("cache", "path")) { + val <- board[[field]] + if (!is.null(val) && !is.na(val) && nzchar(val)) { + return(val) + } + } + NULL +} + # The deferred-read state a board source carries: the board plus the pins not # yet loaded (named character: table label -> pin name). Shared by every alias # of the source, so a read through one is seen by all. source_prewarm() also # stores its background downloader's handle here ($process), so all aliases see # at most one live warmer. -new_pending_pins <- function(board, tables) { +new_pending_pins <- function(board, tables, root = NULL) { pending <- new.env(parent = emptyenv()) pending$board <- board pending$pins <- tables + pending$root <- root + # Tables registered as views over pin files: table label -> list(pin, path). + pending$views <- list() pending } @@ -454,9 +484,16 @@ source_ensure_tables <- function(source, tables, call = rlang::caller_env()) { ) for (table in todo) { pin <- pending$pins[[table]] - value <- tryCatch( - pins::pin_read(pending$board, pin), + tryCatch( + with_pin_lock( + pending$board, + pin, + source_load_pin(source, table, pin, call = call) + ), error = function(err) { + if (inherits(err, "commons_pin_not_data_frame")) { + stop(err) + } cli::cli_abort( "Failed to read pin {.val {pin}} for table {.val {table}}.", parent = err, @@ -464,26 +501,141 @@ source_ensure_tables <- function(source, tables, call = rlang::caller_env()) { ) } ) - if (!is.data.frame(value)) { - cli::cli_abort( - c( - "Pin {.val {pin}} for table {.val {table}} is not a data frame.", - i = "It is {.obj_type_friendly {value}}." - ), - call = call - ) - } - DBI::dbWriteTable(state$con, table, as.data.frame(value), overwrite = TRUE) pending$pins <- pending$pins[setdiff(names(pending$pins), table)] } invisible(source) } +# Register a pinned table in DuckDB. When the pin is a single file DuckDB +# reads natively (parquet/csv/json), register a view over the downloaded +# file instead of copying the data in: the load is free, the data stays in +# the pins cache, and queries get DuckDB's column and predicate pushdown. +# The view references the resolved version's file, so it is as stable as +# that version; refresh_dangling_views() covers boards that delete old +# versions. Other formats (rds, qs2, arrow, uploads) take the eager path. +source_load_pin <- function(source, table, pin, call = rlang::caller_env()) { + state <- data_source_state(source) + board <- state$pending$board + meta <- pins::pin_meta(board, pin) + version <- meta$local$version + path <- pins::pin_download(board, pin, version = version) + + reader <- if (length(path) == 1 && !is.null(state$pending$root)) { + pin_view_reader(meta$type) + } + if (!is.null(reader)) { + create_pin_view(state$con, table, path, reader) + state$pending$views[[table]] <- list(pin = pin, path = path) + return(invisible(source)) + } + + value <- pins::pin_read(board, pin, version = version) + if (!is.data.frame(value)) { + cli::cli_abort( + c( + "Pin {.val {pin}} for table {.val {table}} is not a data frame.", + i = "It is {.obj_type_friendly {value}}." + ), + class = "commons_pin_not_data_frame", + call = call + ) + } + # A previous registration may have been a view (e.g. the pin's format + # changed across versions); dbWriteTable can't overwrite a view. + DBI::dbExecute( + state$con, + sprintf("DROP VIEW IF EXISTS %s", DBI::dbQuoteIdentifier(state$con, table)) + ) + DBI::dbWriteTable(state$con, table, as.data.frame(value), overwrite = TRUE) + invisible(source) +} + +# Only formats DuckDB reads with built-in (non-extension) table functions: +# the connection is locked down with extension autoload disabled, so +# extension-backed readers (e.g. read_json_auto) are unavailable. +pin_view_reader <- function(type) { + switch( + type, + parquet = "read_parquet", + csv = "read_csv_auto" + ) +} + +create_pin_view <- function(con, table, path, reader) { + DBI::dbExecute( + con, + sprintf( + "CREATE OR REPLACE VIEW %s AS SELECT * FROM %s(%s)", + DBI::dbQuoteIdentifier(con, table), + reader, + DBI::dbQuoteString(con, path) + ) + ) +} + +# A view over a pin file dangles when the file disappears after registration: +# a rewrite on a non-versioned board deletes the old version's directory, and +# cache pruning can remove files too. Re-resolve the pin's latest version and +# register it afresh (a view again, or an eager load if the format changed). +# Returns TRUE only when a view was actually refreshed, so callers can retry +# the failed query once without risking an unbounded loop. +refresh_dangling_views <- function(source) { + pending <- data_source_state(source)$pending + if (is.null(pending) || length(pending$views) == 0) { + return(FALSE) + } + refreshed <- FALSE + for (table in names(pending$views)) { + view <- pending$views[[table]] + if (file.exists(view$path)) { + next + } + tryCatch( + { + with_pin_lock(pending$board, view$pin, { + pending$views[[table]] <- NULL + source_load_pin(source, table, view$pin) + }) + refreshed <- TRUE + }, + error = function(err) NULL + ) + } + refreshed +} + source_ensure_all <- function(source, call = rlang::caller_env()) { state <- data_source_state(source) source_ensure_tables(source, state$tables, call = call) } +# pins has no cache locking, so a background prewarm downloading a pin can +# race a first-use pin_read() of the same pin and leave a truncated cache +# entry that poisons later reads. Both sides take an exclusive lock keyed by +# the board's cache path and pin name, making the cache single-writer: the +# reader waits out an in-flight download instead of duplicating it. Lock +# files are left in the cache after unlock: unlinking one while another +# process waits on it would break the mutual exclusion, and they cost one +# tiny file per pin. +with_pin_lock <- function(board, pin, expr) { + # `cache` is a pins implementation detail (verified against pins 1.4.x); + # the guards below fail open to an unlocked read if it ever goes away. + cache <- board$cache + # Boards without a download cache (e.g. board_folder) never download, so + # there is no race to guard against. + if (is.null(cache) || is.na(cache) || !nzchar(cache)) { + return(force(expr)) + } + # Sanitized names can collide ("a/b" vs "a_b"), which merely serializes + # two pins on one lock. Lock files are never removed, but they're empty + # and there is at most one per pin. + name <- gsub("[^A-Za-z0-9._-]", "_", pin) + dir.create(cache, recursive = TRUE, showWarnings = FALSE) + lock <- filelock::lock(file.path(cache, paste0("commons-", name, ".lock"))) + on.exit(filelock::unlock(lock), add = TRUE) + force(expr) +} + # Warm the pins on-disk cache in a background process rather than loading into # DuckDB: dbWriteTable() must run in this process (where the DuckDB lives) and # would block every question asked during the load. The board is serialized to @@ -528,7 +680,7 @@ prewarm_downloads <- function(board, pins) { function(pin) { tryCatch( { - pins::pin_download(board, pin) + with_pin_lock(board, pin, pins::pin_download(board, pin)) TRUE }, error = function(err) FALSE @@ -590,13 +742,20 @@ source_describe <- function( catalog_ensure_queryable(source, table, call = call) source_ensure_tables(source, table) - sample <- DBI::dbGetQuery( - state$con, - sprintf( - "SELECT * FROM %s LIMIT %d", - DBI::dbQuoteIdentifier(state$con, id), - n_sample - ) + sample_sql <- sprintf( + "SELECT * FROM %s LIMIT %d", + DBI::dbQuoteIdentifier(state$con, id), + n_sample + ) + sample <- tryCatch( + DBI::dbGetQuery(state$con, sample_sql), + error = function(err) { + if (refresh_dangling_views(source)) { + DBI::dbGetQuery(state$con, sample_sql) + } else { + stop(err) + } + } ) relation <- source_relation(source, table) if (is.null(state$relations)) { @@ -694,10 +853,14 @@ source_query <- function(source, sql) { return(result) } todo <- pending_tables_in_error(source, result) - if (length(todo) == 0) { - stop(result) + if (length(todo) > 0) { + source_ensure_tables(source, todo) + next + } + if (refresh_dangling_views(source)) { + next } - source_ensure_tables(source, todo) + stop(result) } } @@ -776,18 +939,37 @@ check_query <- function(sql, call = rlang::caller_env()) { # DuckDB-specific hardening for the connection we own: no extension loading, # no filesystem or external access, and the configuration locked thereafter. -duckdb_lock_down <- function(con) { +duckdb_lock_down <- function(con, allow_dirs = character(0)) { + # allowed_directories must be set while external access is still enabled, + # and is incompatible with disabling LocalFileSystem (the filesystem-level + # block wins over the path allowlist). With an allowlist, external access + # stays disabled and only the listed directories remain readable. + if (length(allow_dirs) > 0) { + DBI::dbExecute( + con, + sprintf( + "SET allowed_directories = [%s]", + paste(DBI::dbQuoteString(con, allow_dirs), collapse = ", ") + ) + ) + } DBI::dbExecute( con, - " + sprintf( + " SET allow_community_extensions = false; SET allow_unsigned_extensions = false; SET autoinstall_known_extensions = false; SET autoload_known_extensions = false; SET enable_external_access = false; -SET disabled_filesystems = 'LocalFileSystem'; -SET lock_configuration = true; -" +%sSET lock_configuration = true; +", + if (length(allow_dirs) == 0) { + "SET disabled_filesystems = 'LocalFileSystem';\n" + } else { + "" + } + ) ) invisible(con) } diff --git a/pkg-r/man/commons_prewarm.Rd b/pkg-r/man/commons_prewarm.Rd new file mode 100644 index 00000000..03121c33 --- /dev/null +++ b/pkg-r/man/commons_prewarm.Rd @@ -0,0 +1,79 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/chat.R +\name{commons_prewarm} +\alias{commons_prewarm} +\title{Pre-warm a commons agent during post-startup idle time} +\usage{ +commons_prewarm(client) +} +\arguments{ +\item{client}{A \code{\link[=commons]{commons()}} agent.} +} +\value{ +\code{NULL}, invisibly. +} +\description{ +A \code{\link[=commons]{commons()}} agent defers two kinds of setup to first use, and exposes a +\code{prewarm()} method for each so you can move the cost off the first +question: +} +\details{ +\itemize{ +\item \code{agent$prewarm_context()} builds the context index (the store behind +\code{search_context}). The index is a persistent, content-addressed file, +so the build happens once per content version: later sessions open it +in milliseconds, and it can be built offline ahead of deployment. The +cache root resolves from the \code{commons.context_cache} option, the +\code{COMMONS_CONTEXT_CACHE} or \code{CONNECT_CONTENT_DATA_DIR} environment +variables, an \code{app_cache/commons} directory beside a Shiny app, or the +per-user cache directory, in that order. This resolution ladder (and +the content-addressed, age-pruned cache design generally) follows the +file cache in the sass package, which has run in production Shiny +deployments for years. Note that \verb{app_cache/} is +excluded from deployed bundles (rsconnect treats it as server-side +scratch), so it is shared across the sessions of one deployment but +rebuilt after a redeploy; to ship a pre-built store with the app, +point \code{commons.context_cache} at a directory inside the app and run +\code{prewarm_context()} before deploying. The cache is capped at 256 MB +with least-recently-used eviction (option +\code{commons.context_cache_max_size}, in bytes; a single store larger than +the cap is kept, with a warning). Set +\code{options(commons.context_cache = FALSE)} (or the +\code{COMMONS_CONTEXT_CACHE} environment variable to \code{false}) to disable +persistence entirely. +\item \code{agent$prewarm_sources()} starts a background process that downloads +any uncached pins into the local pins cache (see \code{\link[=data_source]{data_source()}}). +Because the pins cache is on disk, this can also run ahead of +deployment — outside the Shiny runtime entirely — and the deployed +app reads the warmed cache. +} + +\code{agent$prewarm()} calls both. Call \code{commons_prewarm()} in a Shiny server +function to defer warming to post-startup idle time, so it happens while +the user reads the welcome message. Outside a running Shiny app (e.g. a +pre-deploy warm-up script) there is no \code{\link[later:later]{later::later()}} event loop, so +\code{commons_prewarm()} warms synchronously instead. Note that +\code{commons_prewarm()} always downgrades failures to warnings (see below), +even on this synchronous path — a pre-deploy script that should fail the +deploy on a cold cache must call \code{agent$prewarm()} directly. + +\code{prewarm()} lets failures propagate, since a direct call is typically +warming caches ahead of deployment and a mere warning would sail through +a deploy script. \code{commons_prewarm()} downgrades such failures to +warnings: pre-warming is a pure optimization — everything it builds is +rebuilt lazily at first use — and an error escaping the \code{\link[later:later]{later::later()}} +callback would stop the app. +} +\examples{ +\dontrun{ +server <- function(input, output, session) { + agent <- commons( + ellmer::chat_anthropic(), + data_sources = data_source(sales = sales) + ) + commons_prewarm(agent) + shinychat::chat_server("chat", client = agent) +} +} + +} diff --git a/pkg-r/man/data_source.Rd b/pkg-r/man/data_source.Rd index ad45e24d..2fbbdfa1 100644 --- a/pkg-r/man/data_source.Rd +++ b/pkg-r/man/data_source.Rd @@ -61,13 +61,21 @@ when the data isn't already in a database. \item A \code{pins} board, e.g. \code{\link[pins:board_connect]{pins::board_connect()}}, is read into the same in-process database: each pin in \code{tables} becomes a table. Pin names are validated against the board at construction (a single listing call), but -each pin is downloaded only when its table is first used. -\code{\link[=commons_server]{commons_server()}} starts a background process right after startup that -downloads the remaining pins into the local pins cache, so a first use -typically only reads an already-downloaded file. A table reflects the pin's -value at first use and is not refreshed for the lifetime of the data -source; if a pin can't be read (e.g. a network failure), the error surfaces -at that first use and the read is retried on the next one. +each pin is downloaded only when its table is first used. Parquet and +CSV pins are then queried in place -- a DuckDB view over the downloaded +file, with no copy into the database -- while other formats (e.g. RDS) +are read and loaded at first use. Calling the agent's +\code{prewarm_sources()} method (see \code{\link[=commons_prewarm]{commons_prewarm()}}) starts a background +process that downloads the remaining pins into the local pins cache, so +a first use typically only reads an already-downloaded file. Since the +pins cache is on disk, \code{prewarm_sources()} can also run ahead of +deployment to warm the cache the deployed app will read. A table +reflects the pin version resolved at first use and is not refreshed for +the lifetime of the data source; if the board deletes that version +(e.g. a rewrite on a non-versioned board), the next query re-resolves +the latest version. If a pin can't be read (e.g. a network failure), +the error surfaces at that first use and the read is retried on the +next one. } } \section{Data dictionaries}{ diff --git a/pkg-r/tests/testthat/_snaps/layer-objects.md b/pkg-r/tests/testthat/_snaps/layer-objects.md index a3755fc8..af340f03 100644 --- a/pkg-r/tests/testthat/_snaps/layer-objects.md +++ b/pkg-r/tests/testthat/_snaps/layer-objects.md @@ -36,3 +36,4 @@ print(context_two) Message A commons context layer with 2 documents. + diff --git a/pkg-r/tests/testthat/setup-context-cache.R b/pkg-r/tests/testthat/setup-context-cache.R new file mode 100644 index 00000000..63922b97 --- /dev/null +++ b/pkg-r/tests/testthat/setup-context-cache.R @@ -0,0 +1,7 @@ +# Isolate the persistent context store cache per test session so stores +# built by one test session don't mask build spans (or leak) in another. +options(commons.context_cache = tempfile("commons-context-cache-")) +withr::defer( + unlink(getOption("commons.context_cache"), recursive = TRUE), + testthat::teardown_env() +) diff --git a/pkg-r/tests/testthat/test-chat.R b/pkg-r/tests/testthat/test-chat.R index 8d67dcab..832347ce 100644 --- a/pkg-r/tests/testthat/test-chat.R +++ b/pkg-r/tests/testthat/test-chat.R @@ -105,3 +105,46 @@ test_that("commons_server requires a commons agent", { error = TRUE ) }) + +test_that("commons_prewarm() downgrades prewarm failures to warnings", { + path <- withr::local_tempfile(fileext = ".md") + writeLines(c("# Revenue", "", "Revenue means booked revenue."), path) + agent <- test_agent(context_layer = context_layer(files = path)) + + local_mocked_bindings( + context_store = function(...) stop("index build exploded"), + .package = "commons" + ) + # Outside a running Shiny app, commons_prewarm() warms synchronously. + expect_warning(commons_prewarm(agent), "index build exploded") +}) + +test_that("commons_prewarm() warns on failures with braces in the message", { + path <- withr::local_tempfile(fileext = ".md") + writeLines(c("# Revenue", "", "Revenue means booked revenue."), path) + agent <- test_agent(context_layer = context_layer(files = path)) + + # DuckDB errors embed JSON; cli must not interpolate the raw message + local_mocked_bindings( + context_store = function(...) stop('bad store: {"code": 1}'), + .package = "commons" + ) + expect_warning(commons_prewarm(agent), "bad store", fixed = TRUE) +}) + +test_that("commons_app() prewarms the agent on idle", { + skip_if_not_installed("shiny") + skip_if_not_installed("shinychat") + + app <- commons_app(test_agent()) + app_env <- environment(app$serverFuncSource) + prewarmed <- FALSE + testthat::local_mocked_bindings( + commons_prewarm = function(client) prewarmed <<- TRUE, + .package = "commons" + ) + shiny::testServer(app_env$server, { + session$flushReact() + }) + expect_true(prewarmed) +}) diff --git a/pkg-r/tests/testthat/test-commons.R b/pkg-r/tests/testthat/test-commons.R index fb2e4b6d..ec371cfe 100644 --- a/pkg-r/tests/testthat/test-commons.R +++ b/pkg-r/tests/testthat/test-commons.R @@ -418,7 +418,7 @@ test_that("commons() errors on injection parameters matching no name", { }) -test_that("prewarm() builds the context store ahead of the first search", { +test_that("prewarm_context() builds the context store ahead of the first search", { path <- withr::local_tempfile(fileext = ".md") writeLines(c("# Revenue", "", "Revenue means booked revenue."), path) layer <- context_layer(files = path) @@ -428,23 +428,50 @@ test_that("prewarm() builds the context store ahead of the first search", { agent <- test_agent(context_layer = layer) expect_null(context_layer_state(layer)$store) - agent$prewarm() + agent$prewarm_context() expect_false(is.null(context_layer_state(layer)$store)) expect_match(context_search(layer, "revenue")[[1]], "booked") }) +test_that("prewarm() warms both context and sources", { + path <- withr::local_tempfile(fileext = ".md") + writeLines(c("# Revenue", "", "Revenue means booked revenue."), path) + layer <- context_layer(files = path) + agent <- test_agent(context_layer = layer) + + agent$prewarm() + expect_false(is.null(context_layer_state(layer)$store)) +}) + test_that("prewarm() without a context layer is a no-op", { expect_no_error(test_agent()$prewarm()) + expect_no_error(test_agent()$prewarm_context()) + expect_no_error(test_agent()$prewarm_sources()) }) -test_that("prewarm() records a cache-miss build and its own span", { +test_that("prewarm() propagates failures", { + path <- withr::local_tempfile(fileext = ".md") + writeLines(c("# Revenue", "", "Revenue means booked revenue."), path) + agent <- test_agent(context_layer = context_layer(files = path)) + + local_mocked_bindings( + context_store = function(...) stop("index build exploded"), + .package = "commons" + ) + expect_error(agent$prewarm(), "index build exploded") + expect_error(agent$prewarm_context(), "index build exploded") +}) + +test_that("prewarm_context() records a cache-miss build and its own span", { skip_if_not_installed("otelsdk") + # A fresh cache root guarantees a cold build regardless of test order. + withr::local_options(commons.context_cache = withr::local_tempdir()) path <- withr::local_tempfile(fileext = ".md") writeLines(c("# Revenue", "", "Revenue means booked revenue."), path) agent <- test_agent(context_layer = context_layer(files = path)) - recorded <- otelsdk::with_otel_record(agent$prewarm()) + recorded <- otelsdk::with_otel_record(agent$prewarm_context()) names <- vapply(recorded$traces, `[[`, character(1), "name") expect_true("commons_context_store_build" %in% names) @@ -456,15 +483,16 @@ test_that("prewarm() records a cache-miss build and its own span", { expect_equal(prewarm_span$attributes[["commons.context.cache_hit"]], FALSE) }) -test_that("prewarm() records a cache hit without a build span", { +test_that("prewarm_context() records a cache hit without a build span", { skip_if_not_installed("otelsdk") + withr::local_options(commons.context_cache = withr::local_tempdir()) path <- withr::local_tempfile(fileext = ".md") writeLines(c("# Revenue", "", "Revenue means booked revenue."), path) agent <- test_agent(context_layer = context_layer(files = path)) - agent$prewarm() + agent$prewarm_context() - recorded <- otelsdk::with_otel_record(agent$prewarm()) + recorded <- otelsdk::with_otel_record(agent$prewarm_context()) names <- vapply(recorded$traces, `[[`, character(1), "name") expect_false("commons_context_store_build" %in% names) @@ -472,7 +500,7 @@ test_that("prewarm() records a cache hit without a build span", { expect_equal(prewarm_span$attributes[["commons.context.cache_hit"]], TRUE) }) -test_that("prewarm() warms board pins in the background without loading them", { +test_that("prewarm_sources() warms board pins in the background without loading them", { skip_if_not_installed("pins") board <- board_with_pins( @@ -487,7 +515,7 @@ test_that("prewarm() warms board pins in the background without loading them", { expect_length(DBI::dbListTables(data_source_state(src)$con), 0) - agent$prewarm() + agent$prewarm_sources() p <- data_source_state(src)$pending$process expect_s3_class(p, "r_process") withr::defer(p$kill()) diff --git a/pkg-r/tests/testthat/test-context-layer.R b/pkg-r/tests/testthat/test-context-layer.R index d4f2019b..a13eea2a 100644 --- a/pkg-r/tests/testthat/test-context-layer.R +++ b/pkg-r/tests/testthat/test-context-layer.R @@ -77,3 +77,219 @@ test_that("context_layer skips a frontmatter-only file", { layer <- context_layer(files = path) expect_length(context_search(layer, "provenance"), 0) }) + +test_that("the context store persists on disk and is shared across layers", { + withr::local_options(commons.context_cache = withr::local_tempdir()) + path <- withr::local_tempfile(fileext = ".md") + writeLines(c("# Revenue", "", "Revenue means booked revenue."), path) + + layer1 <- context_layer(files = path) + store_path <- context_store_path(context_layer_state(layer1)$docs) + expect_false(file.exists(store_path)) + + expect_match(context_search(layer1, "revenue")[[1]], "booked") + expect_true(file.exists(store_path)) + + # A distinct layer with the same docs opens the same on-disk store + layer2 <- context_layer(files = path) + expect_identical( + context_store_path(context_layer_state(layer2)$docs), + store_path + ) + expect_match(context_search(layer2, "revenue")[[1]], "booked") + + # Different docs key a different store + expect_false(identical(context_store_path("other docs"), store_path)) +}) + +test_that("commons.context_cache = FALSE builds the store in memory", { + withr::local_options(commons.context_cache = FALSE) + path <- withr::local_tempfile(fileext = ".md") + writeLines(c("# Revenue", "", "Revenue means booked revenue."), path) + + layer <- context_layer(files = path) + expect_match(context_search(layer, "revenue")[[1]], "booked") + expect_identical( + DBI::dbGetInfo(context_layer_state(layer)$store@con)$dbname, + ":memory:" + ) +}) + +# Reset the package-level cache housekeeping state on test exit, so tests +# that poke it don't leak order-dependence into later tests. +local_context_cache_state <- function(env = parent.frame()) { + old <- as.list(context_cache_state) + withr::defer( + { + rm(list = ls(context_cache_state), envir = context_cache_state) + list2env(old, envir = context_cache_state) + }, + env + ) +} + +test_that("an unwritable cache dir warns once and falls back to a tempdir", { + # A file where the cache dir should be makes dir.create() fail. + blocker <- withr::local_tempfile() + writeLines("occupied", blocker) + withr::local_options(commons.context_cache = file.path(blocker, "cache")) + + local_context_cache_state() + context_cache_state$warned <- NULL + context_cache_state$fallback_dir <- NULL + + path <- withr::local_tempfile(fileext = ".md") + writeLines(c("# Revenue", "", "Revenue means booked revenue."), path) + layer <- context_layer(files = path) + + expect_warning( + expect_match(context_search(layer, "revenue")[[1]], "booked"), + "Falling back to a per-session temporary directory" + ) + # The fallback is stable within the session and warns only once + expect_no_warning(context_cache_dir_safe()) + expect_identical(context_cache_dir_safe(), context_cache_state$fallback_dir) +}) + +test_that("prune_context_cache() is throttled across builds", { + dir <- withr::local_tempdir() + stale <- file.path(dir, "stale.duckdb") + writeLines(strrep("x", 1000), stale) + + # A prune just happened, and the build count isn't at a multiple of 20 + local_context_cache_state() + context_cache_state$n_builds <- 1 + context_cache_state$last_prune <- Sys.time() + prune_context_cache(dir, max_size = 1) + expect_true(file.exists(stale)) + + # Twenty builds since the throttle reset forces a prune + context_cache_state$n_builds <- 19 + prune_context_cache(dir, max_size = 1) + expect_false(file.exists(stale)) +}) + +test_that("prune_context_cache() evicts least-recently-used stores over the size cap", { + dir <- withr::local_tempdir() + oldest <- file.path(dir, "oldest.duckdb") + middle <- file.path(dir, "middle.duckdb") + newest <- file.path(dir, "newest.duckdb") + for (f in c(oldest, middle, newest)) { + writeLines(strrep("x", 1000), f) + } + now <- Sys.time() + Sys.setFileTime(oldest, now - 300) + Sys.setFileTime(middle, now - 200) + Sys.setFileTime(newest, now - 100) + + local_context_cache_state() + context_cache_state$n_builds <- 0 + context_cache_state$last_prune <- NULL + # Cap fits two stores; the oldest is evicted + cap <- 2 * file.size(newest) + prune_context_cache(dir, max_size = cap) + + expect_false(file.exists(oldest)) + expect_true(file.exists(middle)) + expect_true(file.exists(newest)) +}) + +test_that("prune_context_cache() keeps a single store larger than the cap", { + dir <- withr::local_tempdir() + big <- file.path(dir, "big.duckdb") + writeLines(strrep("x", 10000), big) + + local_context_cache_state() + context_cache_state$n_builds <- 0 + context_cache_state$last_prune <- NULL + context_cache_state$warned_size <- NULL + + expect_warning( + prune_context_cache(dir, max_size = 1, protect = big), + "exceeds its size cap" + ) + expect_true(file.exists(big)) + + # Warns only once per session + context_cache_state$n_builds <- 0 + context_cache_state$last_prune <- NULL + expect_no_warning( + prune_context_cache(dir, max_size = 1, protect = big) + ) + expect_true(file.exists(big)) +}) + +test_that("prune_context_cache() reaps stale .build-* temp files only", { + dir <- withr::local_tempdir() + old <- file.path(dir, ".build-old") + fresh <- file.path(dir, ".build-fresh") + store <- file.path(dir, "store.duckdb") + for (f in c(old, fresh, store)) { + writeLines("x", f) + } + Sys.setFileTime(old, Sys.time() - 25 * 60 * 60) + + local_context_cache_state() + context_cache_state$n_builds <- 0 + context_cache_state$last_prune <- NULL + prune_context_cache(dir) + + expect_false(file.exists(old)) + expect_true(file.exists(fresh)) + expect_true(file.exists(store)) +}) + +test_that("context_store() warns and rebuilds when the cached store won't open", { + layer <- new_context_layer(c("Some context about widgets.")) + path <- context_store_path(context_layer_state(layer)$docs) + dir.create(dirname(path), recursive = TRUE, showWarnings = FALSE) + # Stand in for a store unlinked or corrupted between file.exists() and + # connect (e.g. by a concurrent pruner) + writeLines("not a duckdb file", path) + + expect_warning( + store <- context_store(layer), + "rebuilding" + ) + expect_identical(store, context_layer_state(layer)$store) + expect_equal( + context_search(layer, "widgets"), + "Some context about widgets." + ) +}) + +test_that("cache root prefers the option, then env vars", { + withr::local_options(commons.context_cache = NULL) + withr::local_envvar( + COMMONS_CONTEXT_CACHE = NA, + CONNECT_CONTENT_DATA_DIR = NA, + SHINY_SERVER_VERSION = NA + ) + expect_identical( + context_cache_dir(), + tools::R_user_dir("commons", "cache") + ) + + withr::local_envvar(CONNECT_CONTENT_DATA_DIR = "/connect/data") + expect_identical(context_cache_dir(), "/connect/data") + + withr::local_envvar(COMMONS_CONTEXT_CACHE = "/explicit/cache") + expect_identical(context_cache_dir(), "/explicit/cache") + + withr::local_options(commons.context_cache = "/option/cache") + expect_identical(context_cache_dir(), "/option/cache") +}) + +test_that("the context_cache option must be a path or FALSE", { + withr::local_options(commons.context_cache = TRUE) + expect_error(context_cache_dir(), "must be a path") +}) + +test_that("COMMONS_CONTEXT_CACHE can disable the cache", { + withr::local_options(commons.context_cache = NULL) + withr::local_envvar(COMMONS_CONTEXT_CACHE = "FALSE") + expect_false(context_cache_enabled()) + # A false-like value is never mistaken for a path + withr::local_envvar(CONNECT_CONTENT_DATA_DIR = "/connect/data") + expect_identical(context_cache_dir(), "/connect/data") +}) diff --git a/pkg-r/tests/testthat/test-data-source.R b/pkg-r/tests/testthat/test-data-source.R index 361a81a7..ab6dc471 100644 --- a/pkg-r/tests/testthat/test-data-source.R +++ b/pkg-r/tests/testthat/test-data-source.R @@ -498,3 +498,72 @@ test_that("as_data_sources validates its input", { error = TRUE ) }) + +test_that("parquet/csv pins register as zero-copy views over the pins cache", { + skip_if_not_installed("pins") + skip_if_not_installed("nanoparquet") + + board <- pins::board_temp() + suppressMessages({ + pins::pin_write(board, data.frame(id = 1:3, v = letters[1:3]), "p-parquet", type = "parquet") + pins::pin_write(board, data.frame(id = 4:6, v = letters[4:6]), "p-csv", type = "csv") + }) + src <- data_source( + board, + tables = c(parquet_t = "p-parquet", csv_t = "p-csv") + ) + + state <- data_source_state(src) + for (table in c("parquet_t", "csv_t")) { + res <- source_query(src, sprintf("SELECT * FROM %s", table)) + expect_equal(nrow(res), 3) + # Registered as a view over the cache file, not a copied table + catalog <- DBI::dbGetQuery( + state$con, + "SELECT table_type FROM information_schema.tables WHERE table_name = $1", + params = list(table) + ) + expect_equal(catalog$table_type, "VIEW") + # The view reads the pin's versioned file directly + expect_true(file.exists(state$pending$views[[table]]$path)) + } +}) + +test_that("rds pins still load eagerly as tables", { + skip_if_not_installed("pins") + + board <- board_with_pins("team-orders" = data.frame(id = 1:3)) + src <- data_source(board, tables = c(orders = "team-orders")) + + res <- source_query(src, "SELECT * FROM orders") + expect_equal(nrow(res), 3) + catalog <- DBI::dbGetQuery( + data_source_state(src)$con, + "SELECT table_type FROM information_schema.tables WHERE table_name = 'orders'" + ) + expect_equal(catalog$table_type, "BASE TABLE") +}) + +test_that("a dangling pin view is re-resolved and retried transparently", { + skip_if_not_installed("pins") + skip_if_not_installed("nanoparquet") + + board <- pins::board_folder(withr::local_tempdir(), versioned = FALSE) + suppressMessages( + pins::pin_write(board, data.frame(id = 1L), "orders", type = "parquet") + ) + src <- data_source(board, tables = c(orders = "orders")) + + expect_equal(source_query(src, "SELECT * FROM orders")$id, 1L) + view_path <- data_source_state(src)$pending$views$orders$path + + # A rewrite on a non-versioned board deletes the old version's directory + suppressMessages( + pins::pin_write(board, data.frame(id = 1:2), "orders", type = "parquet") + ) + expect_false(file.exists(view_path)) + + # The next query re-resolves the latest version and succeeds + expect_equal(source_query(src, "SELECT * FROM orders")$id, 1:2) + expect_true(file.exists(data_source_state(src)$pending$views$orders$path)) +}) diff --git a/pkg-r/vignettes/commons.Rmd b/pkg-r/vignettes/commons.Rmd index 56bf6e2f..55016e40 100644 --- a/pkg-r/vignettes/commons.Rmd +++ b/pkg-r/vignettes/commons.Rmd @@ -318,4 +318,4 @@ agent <- commons( commons_app(agent) ``` -Use `commons_app()` to run the agent in a local or single-user Shiny app. For multi-user deployments, compose shinychat's UI with `commons_theme()` on the page and `commons_server()` in the server, and create a new agent for each Shiny session. This example assumes that `observations` and `site_area` are data frames loaded when the app starts. +Use `commons_app()` to run the agent in a local or single-user Shiny app. For multi-user deployments, compose shinychat's UI with `commons_theme()` on the page and `commons_server()` in the server, and create a new agent for each Shiny session. `commons_server()` warms the agent during post-startup idle time (see `commons_prewarm()`); `agent$prewarm_context()` and `agent$prewarm_sources()` warm the context index and pins cache individually. This example assumes that `observations` and `site_area` are data frames loaded when the app starts.