From 1b2294e3a0b597f7aefc8c5baa5ea88a7302d25f Mon Sep 17 00:00:00 2001 From: Jared Knowles Date: Sun, 19 Jul 2026 20:14:16 -0400 Subject: [PATCH] fix: require a DIFFERENT recipe component to cover a per-code gap .recipe_coverage()'s covered_years were computed once per recipe as a union across ALL of its components (aggregate rows included), without excluding the component currently being tested for a gap. So a code whose only representation in a year was its own wide-era aggregate row satisfied its own "covered" check -- self-coverage, not the "other components" review-doc 0.3's criterion actually specifies ("...has no rows ... but other components do"). .recipe_coverage() now returns (recipe_id, component_code, year) triples instead of collapsing across components, and .recipe_component_gapped() excludes the component under test before checking coverage, so a gap only fires when a genuinely different sibling component has data in that year. Adds the boundary test this gap in coverage let slip through untested: Broward FY2011 alone, where E05/F05/G05 each report solely as their own wide-era aggregate row and E04/F04/G04 don't exist as codes before 2012 corpus-wide, so none of the three Corrections recipes have any OTHER component to cover them -- must produce zero suggestions. The existing 2011-2012 combined test still passes, now firing because of the 2012 E05-gapped/E04-covers pair rather than 2011's self-coverage. Updates the header comment to state the other-component requirement explicitly. --- R/suggestions.R | 76 +++++++++++++++++++++-------------- tests/testthat/test-recipes.R | 19 +++++++++ 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/R/suggestions.R b/R/suggestions.R index f075f82..354fabc 100644 --- a/R/suggestions.R +++ b/R/suggestions.R @@ -20,21 +20,27 @@ # EACH recipe component that is itself a category member individually, # rather than asking whether the whole category *result* has zero rows # that year. A recipe fires when one of its own components has zero rows -# for this government in a requested year the recipe's own generic join -# (same join .run_recipe() uses, aggregate rows included) otherwise covers -# -- even if OTHER, unrelated codes in the same category have full data -# that year and the overall result looks complete. That is a deliberate -# narrowing of the R2-era false-positive guard: most governments don't use -# every sibling code in a multi-code category every year, and per-code -# detection WILL flag some of that as a "gap" even though it's really just -# a government not having that particular sub-type of spending, not a -# format-boundary artifact. The remaining guard against ordinary reporting -# variance is the per-government `covered` check below (a component is -# only flagged when the recipe's OWN join -- not some unrelated code -- +# for this government in a requested year, AND SOME OTHER component of that +# SAME recipe -- excluding the gapped one itself -- has a row (same join +# .run_recipe() uses, aggregate rows included) for that year. This is the +# literal review-doc § 0.3 criterion: "...has no rows ... but other +# components do." A component's OWN aggregate-only row does not satisfy +# its own gap (self-coverage is not "other components"); only a genuinely +# different sibling component can. This fires even if OTHER, unrelated +# codes in the same category have full data that year and the overall +# result looks complete. That is a deliberate narrowing of the R2-era +# false-positive guard: most governments don't use every sibling code in a +# multi-code category every year, and per-code detection WILL flag some of +# that as a "gap" even though it's really just a government not having +# that particular sub-type of spending, not a format-boundary artifact. +# The remaining guard against ordinary reporting variance is the +# per-government, per-OTHER-component `covered` check below (a component +# is only flagged when a DIFFERENT component of the SAME recipe -- not +# some unrelated code, and not the gapped component's own aggregate row -- # actually has something to offer in that year); it no longer tries to # avoid noise from sibling *codes*, only from a recipe with genuinely -# nothing to contribute. The acceptable noise level this trade produces is -# a product decision, measured (not tuned here) by +# nothing else to contribute. The acceptable noise level this trade +# produces is a product decision, measured (not tuned here) by # data-raw/measure_signposting_rate.R and ruled on at Checkpoint R3. #' Recipe components that are classified under the requested category -- @@ -93,19 +99,21 @@ )) } -#' Which (recipe_id, year) pairs the recipe's own generic join actually -#' covers for these governments -- the same join .run_recipe() uses -#' (component year_min/year_max + gov_type_scope, no is_aggregate filter), -#' just checking existence instead of summing. This is the per-government, -#' whole-recipe guard against ordinary reporting variance: unlike -#' .component_presence(), it is aggregate-inclusive and unioned across ALL -#' of a recipe's components, not just the one requested code being tested, -#' so a recipe with genuinely nothing to offer (no component, aggregate or -#' leaf, has ever reported) never fires. +#' Which (recipe_id, component_code, year) triples have at least one row +#' (aggregate rows included) for these governments -- the same scoping +#' .run_recipe()'s join uses (component year_min/year_max + gov_type_scope), +#' just checking existence instead of summing. Kept at per-component grain +#' (not unioned across the whole recipe, unlike the R2/R3-pre-fix version of +#' this function) so a gap check can require the covering evidence to come +#' from a DIFFERENT component -- review-doc § 0.3's "other components", not +#' the gapped component's own aggregate row. This is the per-government +#' guard against ordinary reporting variance: a recipe with genuinely +#' nothing to offer from any OTHER component (aggregate or leaf) never +#' fires. #' @noRd .recipe_coverage <- function(con, candidates, govid, years_lit) { DBI::dbGetQuery(con, sprintf( - "SELECT DISTINCT r.recipe_id, l.year + "SELECT DISTINCT r.recipe_id, r.component_code, l.year FROM long l JOIN harmonization_recipes r ON l.item_code = r.component_code @@ -121,23 +129,29 @@ } #' TRUE if recipe `rid` has at least one requested component with an -#' in-scope requested year that has no data (`present`), in a year the -#' recipe's own generic join is otherwise fillable (`covered`) -- the -#' per-code gap the R2 whole-result check couldn't see. +#' in-scope requested year that has no data (`present`), in a year some +#' OTHER component of the same recipe is otherwise fillable (`covered`, +#' excluding the component under test) -- the per-code gap the R2 +#' whole-result check couldn't see, covered by another component the way +#' review-doc § 0.3 specifies (not by the gapped component's own aggregate +#' row -- that is self-coverage, not "other components", and must not +#' count). #' @noRd .recipe_component_gapped <- function(rid, requested, present, covered, years) { - covered_years <- covered$year[covered$recipe_id == rid] - if (length(covered_years) == 0L) return(FALSE) - comps <- requested[requested$recipe_id == rid, , drop = FALSE] for (i in seq_len(nrow(comps))) { + this_code <- comps$component_code[i] in_scope <- years[years >= comps$year_min[i] & years <= comps$year_max[i]] if (length(in_scope) == 0L) next has_data <- present$year[ - present$recipe_id == rid & present$component_code == comps$component_code[i] + present$recipe_id == rid & present$component_code == this_code ] gap_years <- setdiff(in_scope, has_data) - if (any(gap_years %in% covered_years)) return(TRUE) + if (length(gap_years) == 0L) next + other_covered_years <- covered$year[ + covered$recipe_id == rid & covered$component_code != this_code + ] + if (any(gap_years %in% other_covered_years)) return(TRUE) } FALSE } diff --git a/tests/testthat/test-recipes.R b/tests/testthat/test-recipes.R index 87b2318..540d14d 100644 --- a/tests/testthat/test-recipes.R +++ b/tests/testthat/test-recipes.R @@ -203,6 +203,25 @@ test_that("signposting suggests corrections_combined across the 2011->2012 gap", expect_equal(hit$available_years, c(1967L, 2023L)) }) +test_that("per-code gap does NOT fire when a code's only coverage is its own aggregate row (self-coverage is not \"other components\")", { + skip_if_no_corpus() + # Broward, FY2011 ONLY (isolating the 2011 half of the query above): E05, + # F05, and G05 each report SOLELY as a wide-era AGGREGATE row that year + # (216088, 1453, 270 respectively); E04/F04/G04 -- their modern-only + # siblings -- don't exist as codes at all before 2012, corpus-wide (zero + # rows for any government). Each component's own aggregate row would + # trivially satisfy a same-component "covered" check, but review-doc + # § 0.3's criterion is explicit that a gap must be covered by "OTHER + # components", not the gapped component's own aggregate form. With no + # OTHER component present for any of the three Corrections recipes in + # 2011, none of them should fire -- this is what the combined + # 2011-2012 test above actually relies on 2012 (E05 gapped, E04 -- a + # genuinely different component -- covers) to fire, not 2011. + r <- cog_spending("121011212191", years = 2011L, category = "Corrections") + prov <- attr(r, "provenance") + expect_length(prov$suggestions, 0L) +}) + test_that("per-code gap fires even when a sibling code masks the whole-result check (Cleburne County, FY2012)", { skip_if_no_corpus() # Cleburne County, AL (canonical_govid 011029122489), FY2012: E04 ($854)