fix: scope all-categories suggestion candidates by subtype, not category (finding 6)
.build_suggestions()'s recipe-candidate sub-select was keyed on `WHERE category IN (<category>)`. The reserved pseudo-category "All Categories" is never itself a row in summary_categories.category, so in all-categories mode `candidates` always came back empty and coverage signposting (uscogdata#9) was structurally impossible for the one mode whose entire premise is "you cannot sum the wrong scope" -- measured on Los Angeles County FY2011: category = "Public Welfare" reports 2 suggestions (incl. $271,589,000 excluded E68), category = "All Categories" reported 0, silently losing that same signal. Apply the branch's own design principle: the concept boundary is subtype, not category. .build_suggestions() now accepts all_categories/subtype_col/ subtype_scope (all optional, default off, so no other caller's behaviour changes) and, when all-categories mode is active, scopes the candidate sub-select by `<subtype_col> IN (<subtype_scope>)` instead -- symmetric with .build_verb_sql()'s own WHERE predicate. The M/L recipe exclusion and the is.null(category) early return are unchanged. After the fix, LA County FY2011 "All Categories" reports 5 suggestions, including welfare_cash_e68_wide for the exact $271,589,000 gap. Adds two covering tests to test-all-categories.R using the bundled fixture (AL state gov, FY2011, "Corrections"): one end-to-end (per-category and all-categories both signpost the same recipe) and one direct on .build_suggestions() proving the subtype-vs-category branch is what changes the query. Updates the 0.2.0 NEWS entry.
This commit is contained in:
@@ -19,6 +19,19 @@
|
|||||||
* `cog_categories()` advertises `"All Categories"` for the expenditure and
|
* `cog_categories()` advertises `"All Categories"` for the expenditure and
|
||||||
revenue vocabularies, so the reserved value is discoverable.
|
revenue vocabularies, so the reserved value is discoverable.
|
||||||
|
|
||||||
|
* Coverage signposting (see "Signposting now catches partially-suppressed
|
||||||
|
categories" below) now also works in `category = "All Categories"` mode.
|
||||||
|
The recipe-suggestion candidate query used to be scoped by `category`,
|
||||||
|
which is never a match for the reserved `"All Categories"` value, so
|
||||||
|
`provenance$suggestions` always came back empty there — the one mode whose
|
||||||
|
whole point is "you cannot sum the wrong scope" was silently unable to
|
||||||
|
signal a wrong scope. The candidate query is now scoped by the concept's
|
||||||
|
subtype allowlist instead, symmetric with how `.build_verb_sql()` itself
|
||||||
|
scopes the summed total: Los Angeles County FY2011, `category = "All
|
||||||
|
Categories"` still excludes $271,589,000 of aggregate-published Public
|
||||||
|
Welfare (`E68`), but now names `recipe = "welfare_cash_e68_wide"` to
|
||||||
|
recover it instead of reporting zero suggestions.
|
||||||
|
|
||||||
## Documentation
|
## Documentation
|
||||||
|
|
||||||
* `cog_geographic_rollup()` and `cog_peer_compare()` now document that
|
* `cog_geographic_rollup()` and `cog_peer_compare()` now document that
|
||||||
|
|||||||
+4
-1
@@ -442,7 +442,10 @@ cog_spending <- function(govid, years, category = NULL,
|
|||||||
suggestions <- .build_suggestions(con, govid, years, category,
|
suggestions <- .build_suggestions(con, govid, years, category,
|
||||||
direct_leg_result,
|
direct_leg_result,
|
||||||
resolved$basis, flow_prefixes,
|
resolved$basis, flow_prefixes,
|
||||||
.select_long_view(view_base, resolved$basis))
|
.select_long_view(view_base, resolved$basis),
|
||||||
|
all_categories = all_categories,
|
||||||
|
subtype_col = subtype_col,
|
||||||
|
subtype_scope = subtype_scope)
|
||||||
}
|
}
|
||||||
|
|
||||||
# C1(b): when expenditure_concept = "total", flag any row where the IG
|
# C1(b): when expenditure_concept = "total", flag any row where the IG
|
||||||
|
|||||||
+46
-3
@@ -59,12 +59,36 @@
|
|||||||
#' @param long_view Name of the verb's own long view (from
|
#' @param long_view Name of the verb's own long view (from
|
||||||
#' `.select_long_view()`), passed through to `.suppressed_components()` to
|
#' `.select_long_view()`), passed through to `.suppressed_components()` to
|
||||||
#' measure the second qualifying path (uscogdata#9).
|
#' measure the second qualifying path (uscogdata#9).
|
||||||
|
#' @param all_categories `TRUE` when the caller's `category` is the reserved
|
||||||
|
#' pseudo-category (`.ALL_CATEGORIES`). Defaults to `FALSE` so no other
|
||||||
|
#' caller's behaviour changes. When `TRUE`, the candidate-recipe sub-select
|
||||||
|
#' is scoped by `subtype_col`/`subtype_scope` instead of by `category` --
|
||||||
|
#' symmetric with `.build_verb_sql()`'s own all-categories branch (see
|
||||||
|
#' R/spending.R): the concept's subtype allowlist is the real scope
|
||||||
|
#' boundary, not any literal category value, and
|
||||||
|
#' `.ALL_CATEGORIES` ("All Categories") is never itself a row in
|
||||||
|
#' `summary_categories.category`, so leaving the category-keyed sub-select
|
||||||
|
#' in place here always returned zero candidates and silently disabled
|
||||||
|
#' signposting in all-categories mode (final whole-branch review, finding
|
||||||
|
#' 6).
|
||||||
|
#' @param subtype_col Name of the `summary_categories` subtype column to
|
||||||
|
#' scope by when `all_categories = TRUE` (`"spend_subtype"` or
|
||||||
|
#' `"revenue_subtype"` -- the same value `.build_verb_sql()` already
|
||||||
|
#' receives as its own `subtype_col`). Ignored when `all_categories =
|
||||||
|
#' FALSE`. `NULL` by default.
|
||||||
|
#' @param subtype_scope Character vector of subtype values to scope by when
|
||||||
|
#' `all_categories = TRUE` (the same value `.build_verb_sql()` already
|
||||||
|
#' receives as its own `subtype_scope` -- the concept's subtype allowlist,
|
||||||
|
#' e.g. `.expenditure_concept_subtypes(expenditure_concept)`). Ignored when
|
||||||
|
#' `all_categories = FALSE`. `NULL` by default.
|
||||||
#' @return List of `list(recipe_id, label, available_years, hint,
|
#' @return List of `list(recipe_id, label, available_years, hint,
|
||||||
#' ig_recipe_id, trigger, suppressed_amount, suppressed_years,
|
#' ig_recipe_id, trigger, suppressed_amount, suppressed_years,
|
||||||
#' suppressed_codes)`, possibly empty.
|
#' suppressed_codes)`, possibly empty.
|
||||||
#' @noRd
|
#' @noRd
|
||||||
.build_suggestions <- function(con, govid, years, category, result, basis,
|
.build_suggestions <- function(con, govid, years, category, result, basis,
|
||||||
flow_prefixes, long_view) {
|
flow_prefixes, long_view,
|
||||||
|
all_categories = FALSE,
|
||||||
|
subtype_col = NULL, subtype_scope = NULL) {
|
||||||
if (!identical(basis, "harmonized") || is.null(category)) return(list())
|
if (!identical(basis, "harmonized") || is.null(category)) return(list())
|
||||||
|
|
||||||
# Exclude any recipe that is ITSELF an intergovernmental (M/L) recipe --
|
# Exclude any recipe that is ITSELF an intergovernmental (M/L) recipe --
|
||||||
@@ -79,16 +103,35 @@
|
|||||||
# flow-prefix gate below/in `.attach_ig_counterparts()`: an M/L recipe
|
# flow-prefix gate below/in `.attach_ig_counterparts()`: an M/L recipe
|
||||||
# should never be suggested as a coverage-gap filler for EITHER verb, not
|
# should never be suggested as a coverage-gap filler for EITHER verb, not
|
||||||
# just kept from being named as the *counterpart* of another suggestion.
|
# just kept from being named as the *counterpart* of another suggestion.
|
||||||
|
#
|
||||||
|
# The inner sub-select is the concept boundary (finding 6, final
|
||||||
|
# whole-branch review): in all-categories mode it is scoped by
|
||||||
|
# `subtype_col`/`subtype_scope` -- the same allowlist `.build_verb_sql()`
|
||||||
|
# applies as a WHERE predicate to make the summed result a *concept*, not
|
||||||
|
# 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.
|
||||||
|
candidate_scope_sql <- if (isTRUE(all_categories)) {
|
||||||
|
sprintf(
|
||||||
|
"SELECT DISTINCT item_code FROM summary_categories WHERE %s IN (%s)",
|
||||||
|
subtype_col, .sql_lit_chr(subtype_scope)
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
sprintf(
|
||||||
|
"SELECT DISTINCT item_code FROM summary_categories WHERE category IN (%s)",
|
||||||
|
.sql_lit_chr(category)
|
||||||
|
)
|
||||||
|
}
|
||||||
candidates <- DBI::dbGetQuery(con, sprintf(
|
candidates <- DBI::dbGetQuery(con, sprintf(
|
||||||
"SELECT DISTINCT recipe_id FROM harmonization_recipes
|
"SELECT DISTINCT recipe_id FROM harmonization_recipes
|
||||||
WHERE component_code IN (
|
WHERE component_code IN (
|
||||||
SELECT DISTINCT item_code FROM summary_categories WHERE category IN (%s)
|
%s
|
||||||
)
|
)
|
||||||
AND recipe_id NOT IN (
|
AND recipe_id NOT IN (
|
||||||
SELECT DISTINCT recipe_id FROM harmonization_recipes
|
SELECT DISTINCT recipe_id FROM harmonization_recipes
|
||||||
WHERE LEFT(component_code, 1) IN ('M', 'L')
|
WHERE LEFT(component_code, 1) IN ('M', 'L')
|
||||||
)",
|
)",
|
||||||
.sql_lit_chr(category)
|
candidate_scope_sql
|
||||||
))$recipe_id
|
))$recipe_id
|
||||||
if (length(candidates) == 0L) return(list())
|
if (length(candidates) == 0L) return(list())
|
||||||
|
|
||||||
|
|||||||
@@ -189,3 +189,79 @@ test_that('expenditure_concept_direct_suppressed is NA, not FALSE, when categori
|
|||||||
prov_by_cat <- cog_explain(t_by_cat, format = "list")
|
prov_by_cat <- cog_explain(t_by_cat, format = "list")
|
||||||
expect_false(is.na(prov_by_cat$expenditure_concept_direct_suppressed))
|
expect_false(is.na(prov_by_cat$expenditure_concept_direct_suppressed))
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test_that('"All Categories" still signposts coverage gaps (finding 6, final whole-branch review)', {
|
||||||
|
# .build_suggestions()'s candidate sub-select used to be keyed on
|
||||||
|
# `category`, e.g. `WHERE category IN ('All Categories')`. Since
|
||||||
|
# .ALL_CATEGORIES is never itself a row in summary_categories.category,
|
||||||
|
# that sub-select always came back empty in all-categories mode, so
|
||||||
|
# `candidates` was empty and .build_suggestions() short-circuited to
|
||||||
|
# list() -- coverage signposting was structurally impossible for the one
|
||||||
|
# mode whose whole selling point is "you cannot sum the wrong scope"
|
||||||
|
# (uscogdata#9's entire point, silently defeated).
|
||||||
|
#
|
||||||
|
# AL state government, FY2011, category = "Corrections": this category has
|
||||||
|
# no legacy leaf rows in FY2011 (aggregate-flagged E04/E05 family), so the
|
||||||
|
# per-category query returns 0 rows and 3 recipe-hint suggestions fire
|
||||||
|
# (empty_year path). All-categories mode does not have an empty year --
|
||||||
|
# the government has other primary spending in FY2011 -- but the same
|
||||||
|
# suppressed Corrections dollars are still excluded from the summed total,
|
||||||
|
# so the fix (scoping the candidate sub-select by subtype_col/subtype_scope
|
||||||
|
# instead of by category, symmetric with .build_verb_sql()) must still
|
||||||
|
# surface them via the suppressed_component path.
|
||||||
|
gov <- "010000226085"
|
||||||
|
|
||||||
|
by_cat <- suppressMessages(cog_spending(gov, 2011L, category = "Corrections"))
|
||||||
|
sugg_by_cat <- cog_explain(by_cat, format = "list")$suggestions
|
||||||
|
expect_gt(length(sugg_by_cat), 0L)
|
||||||
|
|
||||||
|
all_cat <- suppressMessages(cog_spending(gov, 2011L, category = "All Categories"))
|
||||||
|
sugg_all_cat <- cog_explain(all_cat, format = "list")$suggestions
|
||||||
|
expect_gt(length(sugg_all_cat), 0L)
|
||||||
|
|
||||||
|
# The same Corrections recipe that fired per-category must also fire in
|
||||||
|
# all-categories mode -- not just some unrelated recipe.
|
||||||
|
ids_by_cat <- vapply(sugg_by_cat, function(s) s$recipe_id %||% "", character(1))
|
||||||
|
ids_all_cat <- vapply(sugg_all_cat, function(s) s$recipe_id %||% "", character(1))
|
||||||
|
expect_true("corrections_combined" %in% ids_by_cat)
|
||||||
|
expect_true("corrections_combined" %in% ids_all_cat)
|
||||||
|
|
||||||
|
# In all-categories mode the government DOES have other primary spending
|
||||||
|
# in FY2011 (the year itself is not a gap), so the suggestion can only have
|
||||||
|
# fired via the suppressed_component path, not empty_year.
|
||||||
|
corr_all <- sugg_all_cat[[which(ids_all_cat == "corrections_combined")]]
|
||||||
|
expect_identical(corr_all$trigger, "suppressed_component")
|
||||||
|
expect_gt(corr_all$suppressed_amount, 0)
|
||||||
|
})
|
||||||
|
|
||||||
|
test_that('"All Categories" candidate scoping is symmetric with .build_verb_sql() -- subtype, not category', {
|
||||||
|
# Direct assertion on the mechanism itself (finding 6): in all-categories
|
||||||
|
# mode .build_suggestions() must scope its candidate recipe sub-select by
|
||||||
|
# subtype_col/subtype_scope, not by the literal "All Categories" value.
|
||||||
|
# Passing all_categories = FALSE with the identical category value proves
|
||||||
|
# the branch -- not merely the subtype_col/subtype_scope arguments' mere
|
||||||
|
# presence -- is what changes the query.
|
||||||
|
con <- uscogdata:::.ensure_session()
|
||||||
|
|
||||||
|
none <- uscogdata:::.build_suggestions(
|
||||||
|
con, govid = "010000226085", years = 2011L,
|
||||||
|
category = "All Categories", result = NULL, basis = "harmonized",
|
||||||
|
flow_prefixes = c("E", "F", "G"),
|
||||||
|
long_view = "spending_long_harmonized",
|
||||||
|
all_categories = FALSE,
|
||||||
|
subtype_col = "spend_subtype",
|
||||||
|
subtype_scope = c("operations", "capital", "assistance")
|
||||||
|
)
|
||||||
|
expect_length(none, 0L)
|
||||||
|
|
||||||
|
scoped <- uscogdata:::.build_suggestions(
|
||||||
|
con, govid = "010000226085", years = 2011L,
|
||||||
|
category = "All Categories", result = NULL, basis = "harmonized",
|
||||||
|
flow_prefixes = c("E", "F", "G"),
|
||||||
|
long_view = "spending_long_harmonized",
|
||||||
|
all_categories = TRUE,
|
||||||
|
subtype_col = "spend_subtype",
|
||||||
|
subtype_scope = c("operations", "capital", "assistance")
|
||||||
|
)
|
||||||
|
expect_gt(length(scoped), 0L)
|
||||||
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user