diff --git a/NEWS.md b/NEWS.md index ca4aaf8..6e3b47d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -17,10 +17,21 @@ FY2011 `Miscellaneous Revenue` reported $943,842,000 while dropping $1,899,995,000 of aggregate-published `U4-` rents and royalties. * The trigger stays recipe-driven, so it only fires where a harmonization - recipe actually exists to name the fix. Measured on the bundled fixture, - every fire lands in the wide era; `higher_ed_e18_wide` and - `general_gov_e89_wide` stay silent, because their components are ordinary - classified leaves even pre-2012. + recipe actually exists to name the fix. `higher_ed_e18_wide` and + `general_gov_e89_wide` stay silent in every year measured on the bundled + fixture, because their components are ordinary classified leaves even + pre-2012. +* The `suppressed_component` trigger (and any `suppressed_amount`/ + `suppressed_codes` an `empty_year` fire also carries) is scoped to the + calling verb's own flow family: `cog_spending()` only ever measures E/F/G + component dollars, `cog_revenue()` only T/A/U/B/C/D. A component from the + OTHER flow family reports `suppressed_amount = 0` rather than a fabricated + claim. The `empty_year` trigger itself is not flow-scoped -- a category + belonging to the other flow (e.g. `cog_spending(category = "IG Local")`) + still returns zero rows and can still fire, in any year including modern + ones, naming the recipe whose own generic join finds real data for this + government. That is a mis-scoped query, not a corpus-format gap, so its + `suppressed_amount` is correctly 0. ## New: `cog_balances()` for cash-and-security holdings diff --git a/R/suggestions.R b/R/suggestions.R index 4bfa6cf..3f6eb9b 100644 --- a/R/suggestions.R +++ b/R/suggestions.R @@ -103,7 +103,42 @@ # verb's own view structurally excludes. Measured across ALL requested # years, not just gap years -- the whole point is that a year with rows can # still be missing dollars. - supp <- .suppressed_components(con, candidates, govid, years, long_view) + # + # I3(b): the anti-join inside `.suppressed_components()` is real DuckDB + # work, and unconditionally running it here regressed the common healthy + # path -- pre-#9, a fully-covered category query returned right after the + # cheap `candidates` query above. `.needs_suppression_query()` is a free + # (in-memory), EXACT (not heuristic) pre-check built from `result$ + # codes_included`, which the verb's own basis query already computed: it + # never skips a call that could have found something (see its own + # roxygen), so a recipe still qualifies on suppression alone with zero gap + # years -- it just avoids the round trip when that already-in-memory + # evidence rules it out. + # + # The pre-check needs a candidate's FULL component set, not just the + # component(s) that got it INTO `candidates` above -- a recipe with a + # modern leaf component (e.g. `welfare_cash_e67_wide`'s J67, a + # summary_categories member) also carries a wide-era aggregate component + # (E67) that is absent from summary_categories entirely and so would never + # surface via that query, yet is exactly the component this feature exists + # to catch. This is a second query, but a cheap one: metadata only, + # `recipe_id IN ()`, no `long`/govid/year involvement -- the + # same class of query as `meta` below. + comp_rows <- DBI::dbGetQuery(con, sprintf( + "SELECT DISTINCT recipe_id, component_code FROM harmonization_recipes + WHERE recipe_id IN (%s)", + .sql_lit_chr(candidates) + )) + flow_components <- unique(comp_rows$component_code[ + substr(comp_rows$component_code, 1L, 1L) %in% flow_prefixes]) + supp <- if (.needs_suppression_query(flow_components, result, govid, years)) { + .suppressed_components(con, candidates, govid, years, long_view, flow_prefixes) + } else { + tibble::tibble( + recipe_id = character(0), year = numeric(0), + suppressed_amount = numeric(0), suppressed_codes = character(0) + ) + } if (length(gap_years) == 0L && nrow(supp) == 0L) return(list()) @@ -170,6 +205,60 @@ .attach_ig_counterparts(con, suggestions, flow_prefixes) } +#' Cheap (no SQL), exact pre-check gating the `.suppressed_components()` +#' round trip (uscogdata#9 review, finding I3(b)). +#' +#' Reuses `result`, which the verb's own basis query already computed and +#' which carries `codes_included` -- the DISTINCT item codes the verb's view +#' actually returned -- grouped by exactly `(year, canonical_govid, +#' category)`. A component code appearing there for a given (govid, year) +#' can only have come from the view, so it is -- by construction -- NOT +#' excluded for that (govid, year, item_code) key, which is exactly +#' `.suppressed_components()`'s own anti-join key. So: if every requested +#' (govid, year) pair already accounts for every one of the candidates' +#' flow-scoped component codes this way, the real measurement is guaranteed +#' to return zero rows for every one of them, and can be skipped outright. +#' +#' This is exact, not a heuristic approximation: it only ever returns `FALSE` +#' (skip) when the answer is provably "nothing to find", so it never +#' silences a genuine suppression fire. Any (govid, year) pair this cheaply +#' available evidence does not positively cover -- including a pair with +#' zero rows at all (a gap year), or one government of many in a large +#' batch call whose result happens to omit that year -- is conservatively +#' treated as "might be suppressed", so the real query still runs whenever +#' there is genuine doubt. In particular this does NOT special-case +#' `gap_years`: a recipe with zero gap years can still need the real query, +#' and one with every requested year a gap still gets `TRUE` here (the +#' `is.null(result) || nrow(result) == 0L` branch) rather than being +#' skipped. +#' +#' @param component_codes Character vector of candidate component codes, +#' already restricted to the calling verb's own `flow_prefixes` (I1) -- +#' see `.build_suggestions()`'s `flow_components`. +#' @param result Same `result` `.build_suggestions()` was passed. +#' @param govid Character vector of canonical_govid values. +#' @param years Integer vector of requested years. +#' @return `TRUE` if `.suppressed_components()` must actually run; `FALSE` +#' if it is already provably going to return zero rows. +#' @noRd +.needs_suppression_query <- function(component_codes, result, govid, years) { + if (length(component_codes) == 0L) return(FALSE) + if (is.null(result) || nrow(result) == 0L) return(TRUE) + + req_key <- paste(rep(govid, times = length(years)), + rep(as.integer(years), each = length(govid))) + res_key <- paste(result$canonical_govid, as.integer(result$year)) + codes_by_key <- split(result$codes_included, res_key) + + for (k in unique(req_key)) { + codes_here <- codes_by_key[[k]] + if (is.null(codes_here)) return(TRUE) + present <- unique(unlist(strsplit(codes_here, ",", fixed = TRUE))) + if (!all(component_codes %in% present)) return(TRUE) + } + FALSE +} + #' Measure, per (recipe, year), the component dollars this government holds #' that the calling verb's own long view structurally excludes. #' @@ -193,21 +282,38 @@ #' excluded from the RESULT for scoping reasons -- because it belongs to a #' different `category`, or because `expenditure_concept` narrowed the #' subtypes -- is still present in the view, so it never fires. Suggesting a -#' recipe is a coverage fix, not a category redefinition. Measured on the -#' bundled fixture, this keeps `higher_ed_e18_wide` and `general_gov_e89_wide` -#' silent (E18/E89 are leaf-and-classified even in the wide era) and confines -#' every fire to 2011. +#' recipe is a coverage fix, not a category redefinition. +#' +#' `flow_prefixes` (uscogdata#9 review, finding I1) restricts the measured +#' components to the CALLING VERB's own flow family (`c("E","F","G")` for +#' spending, `c("T","A","U","B","C","D")` for revenue). Without this, a +#' candidate recipe belonging to the OTHER flow family is always absent from +#' this verb's view (by construction -- `cog_revenue()`'s view never carries +#' an E-coded row) and so was always reported as "suppressed", fabricating a +#' dollar claim across flow families (`cog_revenue(category = "Corrections")` +#' claimed $3.63B excluded that `cog_spending()` reports and fully accounts +#' for). Filtering on `LEFT(r.component_code, 1)` also drops M/L-prefixed +#' components from measurement under `cog_spending()` (`flow_prefixes` never +#' includes "M"/"L") -- harmless today, because a recipe's own M/L components +#' (e.g. `corrections_ig_local_combined`'s M04/M05) are present in the view +#' in every year they exist and so never fired as suppressed anyway, but +#' worth recording since this filter is now the thing relied on to prevent +#' it. #' #' @param con Active DuckDB connection. #' @param candidates Character vector of recipe ids to measure. #' @param govid Character vector of canonical_govid values. #' @param years Integer vector of requested years. #' @param long_view Name of the verb's long view, from `.select_long_view()`. +#' @param flow_prefixes The calling verb's own flow-type prefixes (see +#' `.build_suggestions()`). Only recipe components whose first character is +#' in this set are measured. #' @return Tibble of `recipe_id`, `year`, `suppressed_amount` (full US #' dollars), `suppressed_codes` (comma-joined, sorted). Zero rows when #' nothing is suppressed. #' @noRd -.suppressed_components <- function(con, candidates, govid, years, long_view) { +.suppressed_components <- function(con, candidates, govid, years, long_view, + flow_prefixes) { empty <- tibble::tibble( recipe_id = character(0), year = numeric(0), suppressed_amount = numeric(0), suppressed_codes = character(0) @@ -242,16 +348,20 @@ AND l.canonical_govid IN (%2$s) AND l.year IN (%3$s) AND l.amt <> 0 + AND LEFT(r.component_code, 1) IN (%5$s) AND NOT EXISTS ( SELECT 1 FROM %4$s v WHERE v.canonical_govid = l.canonical_govid AND v.year = l.year AND v.item_code = l.item_code + AND v.year IN (%3$s) -- restated: enables partition pruning (I3a) + AND v.canonical_govid IN (%2$s) -- restated: pushes the govid filter (I3a) ) GROUP BY 1, 2 ORDER BY 1, 2", .sql_lit_chr(candidates), .sql_lit_chr(govid), - paste(as.integer(years), collapse = ","), long_view + paste(as.integer(years), collapse = ","), long_view, + .sql_lit_chr(flow_prefixes) ) tibble::as_tibble(DBI::dbGetQuery(con, sql)) } diff --git a/inst/schemas/provenance-v1.json b/inst/schemas/provenance-v1.json index 6bf15ec..a47945d 100644 --- a/inst/schemas/provenance-v1.json +++ b/inst/schemas/provenance-v1.json @@ -38,8 +38,8 @@ "description": "Harmonization recipes that would fill incomplete coverage in the requested years for this government. Empty on a healthy query, on an un-scoped (category = NULL) query, on basis = 'raw', and on a recipe = query (which resolves its own coverage).", "items": { "type": "object", - "required": ["recipe_id", "label", "available_years", "hint", "trigger", - "suppressed_amount", "suppressed_years", "suppressed_codes"], + "required": ["recipe_id", "label", "available_years", "hint", "ig_recipe_id", + "trigger", "suppressed_amount", "suppressed_years", "suppressed_codes"], "properties": { "recipe_id": { "type": "string" }, "label": { "type": "string" }, @@ -56,11 +56,11 @@ "trigger": { "type": "string", "enum": ["empty_year", "suppressed_component"], - "description": "Why this fired. 'empty_year': the result has no rows at all in a requested year. 'suppressed_component': the result HAS rows, but a component code carries dollars the verb's long view structurally excludes -- aggregate-published, or absent from summary_categories. 'empty_year' wins when both apply, being the stronger claim; the suppressed_* fields are populated either way." + "description": "Why this fired. 'empty_year': the result has no rows at all in a requested year. 'suppressed_component': the result HAS rows, but a component code carries dollars this government reports in the requested years that the verb's underlying long view structurally excludes -- aggregate-published, carrying no harmonized code, or absent from summary_categories. This is NOT the same thing as 'excluded from the result': a component present in the view under a different category (a scoping choice, e.g. a different `category` or a narrower `expenditure_concept`) contributes 0 and never fires. 'empty_year' wins when both apply, being the stronger claim; the suppressed_* fields are populated either way, using the same underlying-view measurement, and can be 0 even on an 'empty_year' fire." }, "suppressed_amount": { "type": "number", - "description": "Full US dollars this government holds in the recipe's component codes that the result excludes, summed across the requested years. 0 when nothing is suppressed." + "description": "Full US dollars this government reports, in the recipe's component codes, in the requested years, that the verb's underlying long view structurally excludes (aggregate-published, carrying no harmonized code, or absent from summary_categories) -- summed across those years. This is NOT the same quantity as 'what the result excludes': a component present in the view under a different category or a narrower `expenditure_concept` is scoped out on purpose, counts as 0 here, and is not suppression. 0 does not always mean full coverage -- see 'trigger' and 'empty_year'. May be negative where Census publishes a negative `amt` for the excluded rows." }, "suppressed_years": { "type": "array", diff --git a/tests/testthat/test-recipes.R b/tests/testthat/test-recipes.R index 9d4ec1f..0db8562 100644 --- a/tests/testthat/test-recipes.R +++ b/tests/testthat/test-recipes.R @@ -245,6 +245,68 @@ test_that(".select_long_view maps annotated view bases to their long views", { "spending_long") }) +# --- I3(b): the free pre-check gating .suppressed_components() ------------- + +test_that(".needs_suppression_query skips only when the evidence rules out suppression", { + # Every requested (govid, year) already accounts for every component code: + # .suppressed_components() is guaranteed to find nothing, so it is safe to + # skip the round trip. + result_full <- tibble::tibble( + year = c(2019L, 2019L, 2020L, 2020L), + canonical_govid = c("A", "B", "A", "B"), + codes_included = c("E01,E02", "E01,E02,E03", "E01,E02", "E01,E02") + ) + expect_false(uscogdata:::.needs_suppression_query( + c("E01", "E02"), result_full, govid = c("A", "B"), years = c(2019L, 2020L))) + + # One (govid, year) is missing a component -- cannot rule out suppression, + # so the real measurement must still run. + result_gap <- result_full + result_gap$codes_included[result_gap$canonical_govid == "B" & result_gap$year == 2020L] <- "E01" + expect_true(uscogdata:::.needs_suppression_query( + c("E01", "E02"), result_gap, govid = c("A", "B"), years = c(2019L, 2020L))) + + # A requested (govid, year) is entirely absent from `result` (e.g. a gap + # year, or one government of many in a batch call) -- conservatively TRUE. + result_absent <- result_full[!(result_full$canonical_govid == "B" & result_full$year == 2020L), ] + expect_true(uscogdata:::.needs_suppression_query( + c("E01", "E02"), result_absent, govid = c("A", "B"), years = c(2019L, 2020L))) + + # No candidate component belongs to the calling verb's own flow family (the + # I1 cross-flow-family case) -- nothing could ever be measured, so skip. + expect_false(uscogdata:::.needs_suppression_query( + character(0), result_full, govid = c("A", "B"), years = c(2019L, 2020L))) + + # An empty result (e.g. every requested year is a gap) can never positively + # rule out suppression -- conservatively TRUE. + expect_true(uscogdata:::.needs_suppression_query( + c("E01"), result_full[0, ], govid = "A", years = 2019L)) +}) + +test_that("I3(b): a suppression-only fire (zero gap years) still runs the real measurement", { + # Public Welfare FY2011 for LA County has rows in every requested year (no + # gap_years), so this exercises exactly the path I3(b) must not break: the + # pre-check must return TRUE here, and the real .suppressed_components() + # round trip must actually execute, or the whole uscogdata#9 feature would + # go dark on its own motivating case. + skip_if_no_corpus() + called <- FALSE + orig <- uscogdata:::.suppressed_components + testthat::local_mocked_bindings( + .suppressed_components = function(...) { + called <<- TRUE + orig(...) + }, + .package = "uscogdata" + ) + r <- suppressMessages( + cog_spending("061037123085", years = 2011L, category = "Public Welfare")) + expect_true(called) + sugg <- attr(r, "provenance")$suggestions + triggers <- vapply(sugg, function(s) s$trigger, character(1)) + expect_true(all(triggers == "suppressed_component")) +}) + test_that(".suppressed_components measures the E67/E68 dollars Public Welfare drops", { skip_if_no_corpus() con <- uscogdata:::.ensure_session() @@ -252,7 +314,8 @@ test_that(".suppressed_components measures the E67/E68 dollars Public Welfare dr con, candidates = c("welfare_cash_e67_wide", "welfare_cash_e68_wide"), govid = "061037123085", years = 2011L, - long_view = "spending_long_harmonized") + long_view = "spending_long_harmonized", + flow_prefixes = c("E", "F", "G")) expect_s3_class(s, "tbl_df") expect_equal(nrow(s), 2L) @@ -269,7 +332,8 @@ test_that(".suppressed_components finds nothing in a modern year", { con, candidates = c("welfare_cash_e67_wide", "welfare_cash_e68_wide"), govid = "061037123085", years = 2019L, - long_view = "spending_long_harmonized") + long_view = "spending_long_harmonized", + flow_prefixes = c("E", "F", "G")) expect_equal(nrow(s), 0L) }) @@ -279,10 +343,29 @@ test_that(".suppressed_components rejects a long_view outside the allowlist", { expect_error( uscogdata:::.suppressed_components( con, candidates = "welfare_cash_e67_wide", govid = "061037123085", - years = 2011L, long_view = "long; DROP TABLE x"), + years = 2011L, long_view = "long; DROP TABLE x", + flow_prefixes = c("E", "F", "G")), class = "uscogdata_internal_error") }) +test_that(".suppressed_components never measures a component from the other flow family (I1)", { + # uscogdata#9 review, finding I1: without the flow_prefixes filter, a + # candidate recipe entirely outside the calling verb's own flow family is + # ALWAYS absent from that verb's view (by construction), so it was always + # reported as "suppressed" -- fabricating a dollar claim. E67/E68 are + # Public Welfare EXPENDITURE codes; scoping the measurement to revenue's + # own flow_prefixes must find nothing for them. + skip_if_no_corpus() + con <- uscogdata:::.ensure_session() + s <- uscogdata:::.suppressed_components( + con, + candidates = c("welfare_cash_e67_wide", "welfare_cash_e68_wide"), + govid = "061037123085", years = 2011L, + long_view = "revenue_long_harmonized", + flow_prefixes = c("T", "A", "U", "B", "C", "D")) + expect_equal(nrow(s), 0L) +}) + test_that("uscogdata#9: Public Welfare signposts its suppressed E67/E68 dollars", { # The bug: E74/E79 return rows for FY2011, so there is no row-absence gap, # so nothing fired -- while E67 ($1,803,872,000) and E68 ($271,589,000) were @@ -350,6 +433,36 @@ 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", { + # uscogdata#9 review, finding I1: Corrections is an expenditure-only + # category (E04/E05). cog_revenue() naturally returns zero rows for it, so + # corrections_combined still fires as an empty_year suggestion (its own + # generic join finds real E04/E05 data for this government) -- but before + # the flow_prefixes fix, .suppressed_components() measured E04/E05 against + # cog_revenue()'s OWN view (which can never contain an E-coded row by + # construction) and reported the full $3,631,945,000 as "suppressed", + # when cog_spending() for the same gov/years/category actually returns + # $3,691,029,000 -- nothing was suppressed 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)) + expect_true("corrections_combined" %in% ids) + + hit <- sugg[[which(ids == "corrections_combined")]] + expect_equal(hit$suppressed_amount, 0) + 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", { skip_if_no_corpus() r <- cog_spending("061037123085", years = 2019L, category = "Public Welfare")