fix(suggestions): scope candidate recipes by category_type (#34)
.query_candidate_recipes() (extracted in #33) now filters candidates by category_type ('expenditure' vs 'revenue'), derived from the calling verb's own flow_prefixes (E/F/G -> 'expenditure', else 'revenue'). Without this, a category shared across both flow families in summary_categories leaked cross-family recipes: cog_revenue(category = "Corrections") surfaced the expenditure-only corrections_combined recipe (E04/E05) merely because "Corrections" is also a spending category name, and cog_spending(category = "IG Federal") surfaced the revenue-only ig_federal_b47_wide recipe. Both are wrong: following either hint would attribute dollars to the wrong flow, or (IG Federal) fire the coverage-gap machinery for a category the calling verb structurally cannot report on at all. Updates the two tests this changes the expected behavior of: - "a mis-scoped cog_spending() call never attaches an M/L counterpart to a revenue-flavored recipe" (test-expenditure-concept.R): IG Federal is revenue-only, so a spending call now finds zero candidates outright rather than firing the suggestion and then blocking its M/L counterpart as a second-order check. - "cog_revenue never suggests expenditure-only recipes" (test-recipes.R, was "I1: ... never fabricates suppressed dollars"): corrections_combined is expenditure-only, so a revenue call now never considers it as a candidate, rather than considering it and reporting zero suppressed dollars. All 1078 tests pass (2 skipped live-corpus), measured devtools::test() against this commit in a clean worktree stacked on the #33 refactor. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+38
-8
@@ -89,7 +89,7 @@
|
|||||||
#' project's "functions under 50 lines" convention:
|
#' project's "functions under 50 lines" convention:
|
||||||
#' \itemize{
|
#' \itemize{
|
||||||
#' \item `.query_candidate_recipes()` -- candidate recipe lookup by
|
#' \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_recipe_meta()` -- metadata (label, year spans).
|
||||||
#' \item `.query_covered_years()` -- Path 1 gap-year coverage via the
|
#' \item `.query_covered_years()` -- Path 1 gap-year coverage via the
|
||||||
#' recipe's own generic join.
|
#' recipe's own generic join.
|
||||||
@@ -126,8 +126,16 @@
|
|||||||
# by `category` (`.ALL_CATEGORIES` is never a row in
|
# by `category` (`.ALL_CATEGORIES` is never a row in
|
||||||
# `summary_categories.category`, so a category-keyed sub-select always
|
# `summary_categories.category`, so a category-keyed sub-select always
|
||||||
# came back empty here). The M/L exclusion below is unchanged either way.
|
# 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())
|
if (length(candidates) == 0L) return(list())
|
||||||
|
|
||||||
result_years <- if (is.null(result) || nrow(result) == 0L) {
|
result_years <- if (is.null(result) || nrow(result) == 0L) {
|
||||||
@@ -217,8 +225,17 @@
|
|||||||
#' `summary_categories.category`, so a category-keyed sub-select always
|
#' `summary_categories.category`, so a category-keyed sub-select always
|
||||||
#' returns zero candidates and silently disables signposting.
|
#' 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 con Active DuckDB connection.
|
||||||
#' @param category Category name, or `NULL`.
|
#' @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 all_categories `TRUE` when the caller used `.ALL_CATEGORIES`.
|
||||||
#' @param subtype_col Name of the summary_categories subtype column to
|
#' @param subtype_col Name of the summary_categories subtype column to
|
||||||
#' scope by when `all_categories = TRUE`; ignored otherwise.
|
#' scope by when `all_categories = TRUE`; ignored otherwise.
|
||||||
@@ -226,18 +243,31 @@
|
|||||||
#' when `all_categories = TRUE`; ignored otherwise.
|
#' when `all_categories = TRUE`; ignored otherwise.
|
||||||
#' @return Character vector of recipe IDs (possibly empty).
|
#' @return Character vector of recipe IDs (possibly empty).
|
||||||
#' @noRd
|
#' @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_col = NULL,
|
||||||
subtype_scope = 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)) {
|
candidate_scope_sql <- if (isTRUE(all_categories)) {
|
||||||
sprintf(
|
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 {
|
} else {
|
||||||
sprintf(
|
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
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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.
|
# (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
|
# FL state carries a real FY2011 B47 amount, so this exercises the guard
|
||||||
# against a suggestion that genuinely fires.
|
# 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(
|
r <- suppressMessages(
|
||||||
cog_spending("120000226351", years = c(2005, 2011), category = "IG Federal")
|
cog_spending("120000226351", years = c(2005, 2011), category = "IG Federal")
|
||||||
)
|
)
|
||||||
sugg <- attr(r, "provenance")$suggestions
|
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", {
|
test_that("C1: 'total' on a legacy aggregate-only family reports the IG-only figure honestly, not as Direct + IG", {
|
||||||
|
|||||||
@@ -371,34 +371,25 @@ test_that("uscogdata#9: the revenue verb inherits the same trigger", {
|
|||||||
expect_null(sugg[[1]]$ig_recipe_id)
|
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
|
# 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()
|
skip_if_no_corpus()
|
||||||
r <- suppressMessages(
|
r <- suppressMessages(
|
||||||
cog_revenue("061037123085", years = 2019:2020, category = "Corrections"))
|
cog_revenue("061037123085", years = 2019:2020, category = "Corrections"))
|
||||||
sugg <- attr(r, "provenance")$suggestions
|
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", {
|
test_that("uscogdata#9: no partial-coverage fire in a modern year", {
|
||||||
|
|||||||
Reference in New Issue
Block a user