Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion R/api_openrouter.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 3 additions & 0 deletions R/config.R
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
2 changes: 1 addition & 1 deletion R/import_paper.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment on lines 61 to 65
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OA_CONTENT_DOWNLOAD_COST_USD is defined in config.R, but import_single_paper() is sourced/used in at least one worker context that does not source config.R (e.g., batch_import_task in mod_search_notebook.R sources import_paper.R without config.R). In that path, evaluating cost_usd = OA_CONTENT_DOWNLOAD_COST_USD will fail and silently skip OA usage logging (caught by tryCatch). Consider either ensuring all entrypoints source config.R before import_paper.R, or adding a local fallback (e.g., defaulting to 0.01 when the constant is not defined) so usage logging works consistently.

Copilot uses AI. Check for mistakes.
)
}
Expand Down
3 changes: 2 additions & 1 deletion R/mod_cost_tracker.R
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion R/mod_query_builder.R
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion R/mod_search_notebook.R
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
}
Expand Down
55 changes: 55 additions & 0 deletions tests/testthat/test-cost-tracking.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
library(testthat)
library(shiny)


source_app("config.R")
Expand Down Expand Up @@ -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))
})
62 changes: 62 additions & 0 deletions tests/testthat/test-extract-message-content.R
Original file line number Diff line number Diff line change
@@ -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")
})