diff --git a/R/api_openrouter.R b/R/api_openrouter.R index 18517d8..2a116db 100644 --- a/R/api_openrouter.R +++ b/R/api_openrouter.R @@ -90,7 +90,7 @@ extract_message_content <- function(msg) { # Reasoning models (e.g. gpt-5-nano) put output in $reasoning, $content is NULL - if (is.null(content) || (is.character(content) && nchar(content) == 0)) { + if (is.null(content) || (is.character(content) && (length(content) == 0 || all(nchar(content) == 0)))) { if (!is.null(msg$reasoning)) { return(msg$reasoning) } diff --git a/R/config.R b/R/config.R index e9c73b1..95ff443 100644 --- a/R/config.R +++ b/R/config.R @@ -1,6 +1,9 @@ # App version — single source of truth SERAPEUM_VERSION <- "18.0.0" +# Cost estimate for OpenAlex content API PDF download (USD per request) +OA_CONTENT_DOWNLOAD_COST_USD <- 0.01 + #' Load configuration from YAML file or environment variables #' @param path Path to config file #' @return List of config values (from file, env vars, or NULL) diff --git a/R/import_paper.R b/R/import_paper.R index 3c84101..71b37a8 100644 --- a/R/import_paper.R +++ b/R/import_paper.R @@ -61,7 +61,7 @@ import_single_paper <- function(con, notebook_id, abstract_row, tryCatch( log_oa_usage(con, "content_download", paste0("content/works/", abs$paper_id, ".pdf"), - dl$usage, cost_usd = 0.01), + dl$usage, cost_usd = OA_CONTENT_DOWNLOAD_COST_USD), error = function(e) message("[import] Failed to log OA usage: ", e$message) ) } diff --git a/R/mod_cost_tracker.R b/R/mod_cost_tracker.R index a931c80..4b1d56c 100644 --- a/R/mod_cost_tracker.R +++ b/R/mod_cost_tracker.R @@ -314,9 +314,10 @@ build_latency_sparkline <- function(trend) { if (nrow(trend) < 2) return(NULL) max_ms <- max(trend$avg_latency_ms, na.rm = TRUE) - if (max_ms == 0) return(NULL) + if (!is.finite(max_ms) || max_ms <= 0) return(NULL) bar_heights <- (trend$avg_latency_ms / max_ms) * 40 + bar_heights[is.na(bar_heights)] <- 0 tags$div( class = "d-flex align-items-end gap-1", diff --git a/R/mod_query_builder.R b/R/mod_query_builder.R index 4cb0e80..c2c1e99 100644 --- a/R/mod_query_builder.R +++ b/R/mod_query_builder.R @@ -79,7 +79,7 @@ OUTPUT (valid JSON only, no markdown, no code fences): model <- resolve_model_for_operation(cfg, "query_build") req(provider, model) - if ((is.null(provider$api_key) || nchar(provider$api_key) == 0) && !is_local_provider(provider)) { + if ((is.null(provider$api_key) || is.na(provider$api_key) || !nzchar(provider$api_key)) && !is_local_provider(provider)) { showNotification( "OpenRouter API key not configured. Please go to Settings.", type = "warning", diff --git a/R/mod_search_notebook.R b/R/mod_search_notebook.R index cb2b694..4fd8f4e 100644 --- a/R/mod_search_notebook.R +++ b/R/mod_search_notebook.R @@ -555,6 +555,7 @@ mod_search_notebook_server <- function(id, con, notebook_id, config, notebook_re source("R/db_migrations.R") source("R/db.R") source("R/interrupt.R") + source("R/config.R") # For DEFAULT_CONFIG (used by import_single_paper) source("R/bulk_import.R") # For write_import_progress / read_import_progress source("R/_ragnar.R") # For chunk_with_ragnar() source("R/pdf.R") # For download_pdf_from_url(), process_pdf(), sanitize_filename() @@ -2872,7 +2873,7 @@ mod_search_notebook_server <- function(id, con, notebook_id, config, notebook_re provider_or <- provider_from_config(cfg, con()) embed_model <- resolve_model_for_operation(cfg, "embedding") - if ((is.null(provider_or$api_key) || nchar(provider_or$api_key) == 0) && !is_local_provider(provider_or)) { + if ((is.null(provider_or$api_key) || is.na(provider_or$api_key) || !nzchar(provider_or$api_key)) && !is_local_provider(provider_or)) { showNotification("API key required for embedding (unless using a local provider)", type = "error") return() } diff --git a/tests/testthat/test-cost-tracking.R b/tests/testthat/test-cost-tracking.R index 130c0be..10912bb 100644 --- a/tests/testthat/test-cost-tracking.R +++ b/tests/testthat/test-cost-tracking.R @@ -1,4 +1,5 @@ library(testthat) +library(shiny) source_app("config.R") @@ -265,3 +266,57 @@ test_that("log_cost returns NULL and warns on INSERT failure", { expect_null(result) }) + +# --- #223: build_latency_sparkline edge cases --- + +test_that("build_latency_sparkline returns NULL for all-NA latencies", { + trend <- data.frame( + date = Sys.Date() - 0:2, + avg_latency_ms = c(NA_real_, NA_real_, NA_real_), + call_count = c(1L, 1L, 1L) + ) + expect_null(build_latency_sparkline(trend)) +}) + +test_that("build_latency_sparkline returns NULL for all-zero latencies", { + trend <- data.frame( + date = Sys.Date() - 0:2, + avg_latency_ms = c(0, 0, 0), + call_count = c(1L, 1L, 1L) + ) + expect_null(build_latency_sparkline(trend)) +}) + +test_that("build_latency_sparkline handles mixed NA and valid values", { + trend <- data.frame( + date = Sys.Date() - 0:2, + avg_latency_ms = c(NA_real_, 500, 1000), + call_count = c(1L, 2L, 3L) + ) + result <- build_latency_sparkline(trend) + expect_false(is.null(result)) + expect_s3_class(result, "shiny.tag") + expect_false(grepl("NApx", as.character(result))) +}) + +test_that("build_latency_sparkline returns sparkline for normal input", { + trend <- data.frame( + date = Sys.Date() - 0:3, + avg_latency_ms = c(200, 500, 300, 800), + call_count = c(5L, 3L, 4L, 2L) + ) + result <- build_latency_sparkline(trend) + expect_false(is.null(result)) + expect_s3_class(result, "shiny.tag") + rendered <- as.character(result) + expect_match(rendered, "d-flex") +}) + +test_that("build_latency_sparkline returns NULL for single-row trend", { + trend <- data.frame( + date = Sys.Date(), + avg_latency_ms = 500, + call_count = 3L + ) + expect_null(build_latency_sparkline(trend)) +}) diff --git a/tests/testthat/test-extract-message-content.R b/tests/testthat/test-extract-message-content.R new file mode 100644 index 0000000..6133a52 --- /dev/null +++ b/tests/testthat/test-extract-message-content.R @@ -0,0 +1,62 @@ +library(testthat) + +source_app("api_openrouter.R") + +test_that("extract_message_content returns normal string content", { + msg <- list(content = "Hello, world!") + expect_equal(extract_message_content(msg), "Hello, world!") +}) + +test_that("extract_message_content returns reasoning when content is NULL", { + msg <- list(content = NULL, reasoning = "I reasoned this.") + expect_equal(extract_message_content(msg), "I reasoned this.") +}) + +test_that("extract_message_content returns NULL when content and reasoning are NULL", { + msg <- list(content = NULL, reasoning = NULL) + expect_null(extract_message_content(msg)) +}) + +test_that("extract_message_content falls through empty string to reasoning", { + msg <- list(content = "", reasoning = "fallback reasoning") + expect_equal(extract_message_content(msg), "fallback reasoning") +}) + +test_that("extract_message_content returns empty string when no reasoning fallback", { + msg <- list(content = "") + # Content is empty, no reasoning → falls through and returns content as-is + expect_equal(extract_message_content(msg), "") +}) + +test_that("extract_message_content falls through character(0) to reasoning", { + msg <- list(content = character(0), reasoning = "fallback reasoning") + expect_equal(extract_message_content(msg), "fallback reasoning") +}) + +test_that("extract_message_content handles character(0) with no reasoning", { + msg <- list(content = character(0)) + # character(0) is empty, no reasoning → returns content as-is + expect_equal(extract_message_content(msg), character(0)) +}) + +test_that("extract_message_content handles multi-element character vector", { + # This was the bug — nchar() on a vector yields a vector, if() errors + msg <- list(content = c("Hello", "World")) + expect_equal(extract_message_content(msg), c("Hello", "World")) +}) + +test_that("extract_message_content concatenates list-of-parts content", { + msg <- list(content = list( + list(text = "Part one"), + list(text = "Part two") + )) + expect_equal(extract_message_content(msg), "Part one\nPart two") +}) + +test_that("extract_message_content handles mixed list parts", { + msg <- list(content = list( + list(text = "Structured"), + "plain string" + )) + expect_equal(extract_message_content(msg), "Structured\nplain string") +})