From d0d724c4c88fe35c5c24f0a479f7209abd4a34d9 Mon Sep 17 00:00:00 2001 From: Jared Knowles Date: Wed, 9 Sep 2026 11:28:01 -0400 Subject: [PATCH] fix(suggestions): guard empty candidates and restore year typing in .query_covered_years() Addresses roborev jobs 205/208 (reviewing pre-merge draft/original commits of #33/#34, now split and merged as #69/#70): - Restore `res$year <- as.integer(res$year)` in .query_covered_years(), dropped during the #33 rebuild as apparently-dead code. Its own roxygen promises an integer `year` column; without the coercion the implementation no longer matches that documented contract. - Guard .query_covered_years() against empty `candidates`, mirroring the existing `gap_years` guard. Not reachable via .build_suggestions() today (candidates is checked non-empty before this is called), but the extracted helper is independently callable and previously built a malformed `WHERE r.recipe_id IN ()` clause for a hypothetical direct caller with no candidates. - Add a direct fixture-only unit test of .query_candidate_recipes()'s category_type filtering (#34) in both flow directions -- queries only summary_categories/harmonization_recipes metadata, so it runs without skip_if_no_corpus(), unlike the two existing end-to-end tests. 1080 tests pass (2 skipped live-corpus). Co-Authored-By: Claude Sonnet 5 --- R/suggestions.R | 10 ++++++---- tests/testthat/test-recipes.R | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/R/suggestions.R b/R/suggestions.R index 7ce4487..b0299fa 100644 --- a/R/suggestions.R +++ b/R/suggestions.R @@ -300,14 +300,14 @@ #' result. #' @return Data frame with columns `recipe_id` (character) and `year` #' (integer). Returns an empty data frame (`recipe_id = character(0)`, -#' `year = integer(0)`) when `gap_years` is empty, so callers can safely -#' reference `$recipe_id`. +#' `year = integer(0)`) when `gap_years` or `candidates` is empty, so +#' callers can safely reference `$recipe_id`. #' @noRd .query_covered_years <- function(con, candidates, cohort, gap_years) { - if (length(gap_years) == 0L) { + if (length(gap_years) == 0L || length(candidates) == 0L) { return(data.frame(recipe_id = character(0), year = integer(0))) } - DBI::dbGetQuery(con, sprintf( + res <- DBI::dbGetQuery(con, sprintf( "SELECT DISTINCT r.recipe_id, l.year FROM long l JOIN harmonization_recipes r @@ -322,6 +322,8 @@ .sql_lit_chr(candidates), .cohort_sql(cohort, "l.canonical_govid"), paste(gap_years, collapse = ",") )) + res$year <- as.integer(res$year) + res } #' Query recipe metadata: labels and year spans for a set of candidate diff --git a/tests/testthat/test-recipes.R b/tests/testthat/test-recipes.R index fcb950b..18f3064 100644 --- a/tests/testthat/test-recipes.R +++ b/tests/testthat/test-recipes.R @@ -392,6 +392,26 @@ test_that("I1 + #34: cog_revenue never suggests expenditure-only recipes", { expect_false("corrections_combined" %in% ids) }) +test_that(".query_candidate_recipes() scopes candidates by category_type (#34)", { + # Direct assertion on the mechanism the two tests above exercise + # end-to-end: corrections_combined's own components (E04/E05) are + # category_type = 'expenditure' in summary_categories, so an + # expenditure-flavored flow_prefixes call must surface it and a + # revenue-flavored one must not. This queries only summary_categories/ + # harmonization_recipes (no government data), so it runs against the + # bundled fixture with no skip_if_no_corpus() needed. + con <- uscogdata:::.ensure_session() + + expenditure <- uscogdata:::.query_candidate_recipes( + con, category = "Corrections", flow_prefixes = c("E", "F", "G")) + expect_true("corrections_combined" %in% expenditure) + + revenue <- uscogdata:::.query_candidate_recipes( + con, category = "Corrections", + flow_prefixes = c("T", "A", "U", "B", "C", "D")) + expect_false("corrections_combined" %in% revenue) +}) + test_that("uscogdata#9: no partial-coverage fire in a modern year", { skip_if_no_corpus() r <- cog_spending("061037123085", years = 2019L, category = "Public Welfare")