Merge pull request 'fix(suggestions): scope candidate recipes by category_type (#34)' (#70) from fix/category-type-34 into main

R/suggestions.R

@@ -89,7 +89,7 @@
 #' project's "functions under 50 lines" convention:
 #' \itemize{
 #'   \item `.query_candidate_recipes()` -- candidate recipe lookup by
-#'     category/subtype scope + M/L exclusion.
+#'     category/subtype scope + `category_type` filter (#34) + M/L exclusion.
 #'   \item `.query_recipe_meta()` -- metadata (label, year spans).
 #'   \item `.query_covered_years()` -- Path 1 gap-year coverage via the
 #'     recipe's own generic join.
@@ -126,8 +126,16 @@
   # by `category` (`.ALL_CATEGORIES` is never a row in
   # `summary_categories.category`, so a category-keyed sub-select always
   # came back empty here). The M/L exclusion below is unchanged either way.
-  candidates <- .query_candidate_recipes(con, category, all_categories,
+  #
-                                          subtype_col, subtype_scope)
+  # Issue #34: scope the candidate query by `category_type` ('expenditure'
+  # vs 'revenue') to prevent cross-flow-family leakage -- e.g.
+  # `cog_revenue(category = "Corrections")` must not surface
+  # expenditure-only recipes (E04/E05) merely because they share the same
+  # category name in summary_categories. The type is derived from
+  # flow_prefixes: E/F/G -> 'expenditure', anything else -> 'revenue'.
+  candidates <- .query_candidate_recipes(con, category, flow_prefixes,
+                                          all_categories, subtype_col,
+                                          subtype_scope)
   if (length(candidates) == 0L) return(list())
 
   result_years <- if (is.null(result) || nrow(result) == 0L) {
@@ -217,8 +225,17 @@
 #' `summary_categories.category`, so a category-keyed sub-select always
 #' returns zero candidates and silently disables signposting.
 #'
+#' Scope is also by `category_type` ('expenditure' vs 'revenue', Issue #34)
+#' to prevent cross-flow-family leakage: `cog_revenue(category =
+#' "Corrections")` must not surface expenditure-only recipes (E04/E05)
+#' merely because they share the same category name in summary_categories.
+#' The type is derived from flow_prefixes: E/F/G -> 'expenditure', anything
+#' else -> 'revenue'.
+#'
 #' @param con Active DuckDB connection.
 #' @param category Category name, or `NULL`.
+#' @param flow_prefixes The calling verb's own flow-type prefixes (see
+#'   `.build_suggestions()`). Used to derive `category_type` (#34).
 #' @param all_categories `TRUE` when the caller used `.ALL_CATEGORIES`.
 #' @param subtype_col Name of the summary_categories subtype column to
 #'   scope by when `all_categories = TRUE`; ignored otherwise.
@@ -226,18 +243,31 @@
 #'   when `all_categories = TRUE`; ignored otherwise.
 #' @return Character vector of recipe IDs (possibly empty).
 #' @noRd
-.query_candidate_recipes <- function(con, category, all_categories = FALSE,
+.query_candidate_recipes <- function(con, category, flow_prefixes,
+                                      all_categories = FALSE,
                                       subtype_col = NULL,
                                       subtype_scope = NULL) {
+  # Issue #34: derive category_type from flow_prefixes to prevent
+  # cross-flow-family leakage -- e.g. cog_revenue(category = "Corrections")
+  # must not surface expenditure-only recipes merely because they share the
+  # same category name in summary_categories.
+  category_type <- if (all(flow_prefixes %in% c("E", "F", "G"))) {
+    "expenditure"
+  } else {
+    "revenue"
+  }
+
   candidate_scope_sql <- if (isTRUE(all_categories)) {
     sprintf(
-      "SELECT DISTINCT item_code FROM summary_categories WHERE %s IN (%s)",
+      "SELECT DISTINCT item_code FROM summary_categories
-      subtype_col, .sql_lit_chr(subtype_scope)
+       WHERE %s IN (%s) AND category_type = '%s'",
+      subtype_col, .sql_lit_chr(subtype_scope), category_type
     )
   } else {
     sprintf(
-      "SELECT DISTINCT item_code FROM summary_categories WHERE category IN (%s)",
+      "SELECT DISTINCT item_code FROM summary_categories
-      .sql_lit_chr(category)
+       WHERE category IN (%s) AND category_type = '%s'",
+      .sql_lit_chr(category), category_type
     )
   }
 

tests/testthat/test-expenditure-concept.R

@@ -315,15 +315,17 @@ test_that("a mis-scoped cog_spending() call never attaches an M/L counterpart to
   # (SB194, cog_pipeline#64), so the recipe stopped being a candidate there.
   # FL state carries a real FY2011 B47 amount, so this exercises the guard
   # against a suggestion that genuinely fires.
+  #
+  # Issue #34: "IG Federal" maps to B-prefixed codes in summary_categories
+  # with category_type = 'revenue'. A spending verb (flow_prefixes E/F/G)
+  # now scopes its candidate query by category_type = 'expenditure', so it
+  # correctly finds NO candidates for this revenue-only category -- the
+  # suggestion machinery cannot fire, and no M/L counterpart is attached.
   r <- suppressMessages(
     cog_spending("120000226351", years = c(2005, 2011), category = "IG Federal")
   )
   sugg <- attr(r, "provenance")$suggestions
-  expect_gt(length(sugg), 0L)
+  expect_length(sugg, 0L)
-  ids <- vapply(sugg, function(s) s$recipe_id %||% "", character(1))
-  expect_true("ig_federal_b47_wide" %in% ids)
-  ig <- unlist(lapply(sugg, function(s) s$ig_recipe_id))
-  expect_length(ig, 0L)
 })
 
 test_that("C1: 'total' on a legacy aggregate-only family reports the IG-only figure honestly, not as Direct + IG", {

tests/testthat/test-recipes.R

@@ -371,34 +371,25 @@ test_that("uscogdata#9: the revenue verb inherits the same trigger", {
   expect_null(sugg[[1]]$ig_recipe_id)
 })
 
-test_that("I1: cog_revenue never fabricates suppressed dollars for an expenditure-only recipe", {
+test_that("I1 + #34: cog_revenue never suggests expenditure-only recipes", {
   # uscogdata#9 review, finding I1: Corrections is an expenditure-only
-  # category (E04/E05). cog_revenue() naturally returns zero rows for it, so
+  # category (E04/E05). Before the flow_prefixes fix (#9), .suppressed_components()
-  # corrections_combined still fires as an empty_year suggestion (its own
+  # measured E04/E05 against cog_revenue()'s OWN view and reported $3.6B as
-  # generic join finds real E04/E05 data for this government) -- but before
+  # "suppressed" -- nothing was suppressed at all.
-  # the flow_prefixes fix, .suppressed_components() measured E04/E05 against
+  #
-  # cog_revenue()'s OWN view (which can never contain an E-coded row by
+  # Issue #34 builds on that: the candidate query now also filters by
-  # construction) and reported the full $3,631,945,000 as "suppressed",
+  # category_type ('revenue'), so expenditure-only recipes like corrections_combined
-  # when cog_spending() for the same gov/years/category actually returns
+  # (whose components E04/E05 are classified as 'expenditure' in summary_categories)
-  # $3,691,029,000 -- nothing was suppressed at all.
+  # are never even considered for a revenue verb. This is stronger than just
+  # suppressing the dollar claim -- it prevents the suggestion from firing at all.
   skip_if_no_corpus()
   r <- suppressMessages(
     cog_revenue("061037123085", years = 2019:2020, category = "Corrections"))
   sugg <- attr(r, "provenance")$suggestions
-  ids <- vapply(sugg, function(s) s$recipe_id, character(1))
+  ids <- vapply(sugg, function(s) s$recipe_id %||% "", character(1))
-  expect_true("corrections_combined" %in% ids)
 
-  hit <- sugg[[which(ids == "corrections_combined")]]
+  # corrections_combined should NOT appear -- its components are expenditure-only.
-  expect_equal(hit$suppressed_amount, 0)
+  expect_false("corrections_combined" %in% ids)
-  expect_equal(hit$suppressed_years, integer(0))
-  expect_equal(hit$suppressed_codes, character(0))
-
-  # And cog_spending() for the identical gov/years/category is unaffected --
-  # it actually finds the E04/E05 dollars the buggy measurement claimed were
-  # excluded.
-  sp <- suppressMessages(
-    cog_spending("061037123085", years = 2019:2020, category = "Corrections"))
-  expect_equal(sum(sp$amt_nominal), 3691029000)
 })
 
 test_that("uscogdata#9: no partial-coverage fire in a modern year", {