From 24b2ff7d8c9769adbb129bbcef9911384d657809 Mon Sep 17 00:00:00 2001 From: Jared Knowles Date: Mon, 27 Jul 2026 11:08:57 -0400 Subject: [PATCH] fix: gate IG-counterpart matching to the direct-expenditure flow family Review found the suffix-set match alone is unsafe: revenue-side recipes (ig_federal_b47_wide, ig_state_c47_wide, ig_local_d47_wide, and their *_89 siblings) coincidentally share exact suffix sets with M/L expenditure recipes despite representing a different flow direction. Reachable today via a mis-scoped cog_spending(category = "IG Federal") call, not just cog_revenue(). Thread flow_prefixes (same parameter .build_harmonization_block already uses) through .build_suggestions()/.attach_ig_counterparts() and require a firing recipe's own prefixes to be both in the calling verb's flow family and within {E,F,G} before searching the M/L catalog. --- R/spending.R | 3 +- R/suggestions.R | 77 ++++++++++++++++++----- tests/testthat/test-expenditure-concept.R | 60 ++++++++++++++++++ 3 files changed, 124 insertions(+), 16 deletions(-) diff --git a/R/spending.R b/R/spending.R index 582cb18..1ec6044 100644 --- a/R/spending.R +++ b/R/spending.R @@ -217,7 +217,8 @@ cog_spending <- function(govid, years, category = NULL, harmonization <- .build_harmonization_block( con, govid, years, resolved, flow_prefixes ) - suggestions <- .build_suggestions(con, govid, years, category, result, resolved$basis) + suggestions <- .build_suggestions(con, govid, years, category, result, + resolved$basis, flow_prefixes) } # Determine expenditure_concept_note: only non-empty for "total", explains diff --git a/R/suggestions.R b/R/suggestions.R index 6e30d4b..cf40044 100644 --- a/R/suggestions.R +++ b/R/suggestions.R @@ -35,10 +35,16 @@ #' @param result The verb's already-computed result tibble (post basis #' query, pre per_capita/adjust_to_year). #' @param basis The *resolved* basis (`"harmonized"` or `"raw"`). -#' @return List of `list(recipe_id, label, available_years, hint)`, possibly -#' empty. +#' @param flow_prefixes The calling verb's own flow-type prefixes (e.g. +#' `c("E", "F", "G")` for `cog_spending()`, `c("T", "A", "U", "B", "C", +#' "D")` for `cog_revenue()` -- see `.verb_spendrev()`). Passed through to +#' `.attach_ig_counterparts()` to keep the intergovernmental-counterpart +#' lookup scoped to the calling verb's own flow family. +#' @return List of `list(recipe_id, label, available_years, hint, +#' ig_recipe_id)`, possibly empty. #' @noRd -.build_suggestions <- function(con, govid, years, category, result, basis) { +.build_suggestions <- function(con, govid, years, category, result, basis, + flow_prefixes) { if (!identical(basis, "harmonized") || is.null(category)) return(list()) candidates <- DBI::dbGetQuery(con, sprintf( @@ -98,7 +104,7 @@ hint = sprintf("re-run with recipe = '%s'", rid) ) } - .attach_ig_counterparts(con, suggestions) + .attach_ig_counterparts(con, suggestions, flow_prefixes) } #' Attach `ig_recipe_id` to each suggestion: the intergovernmental-expenditure @@ -116,29 +122,70 @@ #' all use "04"/"05" for corrections). M/L "combined other" codes (47/89/ #' 91-94) reuse digits for an unrelated catch-all construct, so e.g. #' `general_gov_e89_wide`'s {E85, E89} -> {"85", "89"} must NOT match -#' `ige_local_m89_wide`'s {"89", "91", "92", "93"} on the shared "89" alone -- -#' verified against the fixture's full `harmonization_recipes` catalog (see -#' task-6-report.md): only the corrections family (E/F/G/M, suffixes 04/05) -#' has an exact-set match in this corpus. +#' `ige_local_m89_wide`'s {"89", "91", "92", "93"} on the shared "89" alone. +#' Checked by hand against the full harmonization_recipes catalog: only the +#' corrections family (E/F/G/M, suffixes 04/05) has an exact-set match in +#' this corpus. +#' +#' Exact-set suffix matching is NOT enough on its own, though: the same +#' reused-digit problem exists ACROSS the revenue-side IG families too. +#' `ig_local_d47_wide` (D47/D94, suffixes {"47","94"}) is an exact-set match +#' for `ige_local_m47_wide` (M47/M94, same suffixes) even though one is +#' intergovernmental REVENUE received from local governments and the other is +#' intergovernmental EXPENDITURE paid to local governments -- unrelated flows +#' that happen to reuse "47"/"94" for their own "transit/utilities" and +#' "other/combined" catch-alls. `ig_federal_b47_wide`, `ig_state_c47_wide`, +#' and their `*_89` siblings all collide the same way. None of this is +#' reachable via `cog_revenue()` in the bundled fixture today (its B/C/D +#' recipes never happen to have a covered gap year for any fixture govid), +#' but it IS reachable via a mis-scoped `cog_spending()` call on a +#' revenue-only category, e.g. `cog_spending(gov, category = "IG Federal")` +#' fires `ig_federal_b47_wide`/`ig_federal_b89_wide` for real in the fixture +#' -- so this is a live, not merely theoretical, gap. +#' +#' Two flow-family checks close this, both required (see +#' `tests/testthat/test-expenditure-concept.R`, "revenue-flavored ... never +#' receives an M/L counterpart" tests, for the pairwise verification): +#' 1. `own_prefix %in% flow_prefixes`: the firing recipe's own component +#' codes must belong to the calling verb's own flow family (the same +#' `flow_prefixes` `.build_harmonization_block()` uses, see +#' `R/basis.R`). This blocks a recipe surfaced through a mis-scoped +#' category from ever reaching the M/L search, e.g. `cog_spending()`'s +#' flow_prefixes are `c("E","F","G")`, which `ig_federal_b47_wide`'s own +#' `"B"` is not part of. +#' 2. `own_prefix %in% c("E","F","G")`: M/L only ever pairs with the +#' DIRECT-expenditure family, never with revenue (`cog_revenue()`'s +#' flow_prefixes already fold B/C/D in as ordinary revenue -- there is +#' no separate "Total" bolt-on for revenue the way `expenditure_concept` +#' adds one for spending) and never with ANOTHER M/L recipe (without +#' this check, `ige_local_m47_wide` would wrongly match sibling +#' `ige_state_l47_wide` on their shared {"47","94"} suffix set). +#' Condition 1 alone does not catch this: under `cog_revenue()`, +#' `ig_federal_b47_wide`'s own `"B"` IS inside revenue's own +#' `flow_prefixes`, so only this second, family-specific check blocks +#' the search. #' @noRd -.attach_ig_counterparts <- function(con, suggestions) { +.attach_ig_counterparts <- function(con, suggestions, flow_prefixes) { if (length(suggestions) == 0L) return(suggestions) comp <- DBI::dbGetQuery(con, "SELECT recipe_id, component_code FROM harmonization_recipes") + comp$prefix <- substr(comp$component_code, 1L, 1L) comp$suffix <- substr(comp$component_code, 2L, nchar(comp$component_code)) suffix_sets <- lapply(split(comp$suffix, comp$recipe_id), function(x) sort(unique(x))) + prefix_sets <- lapply(split(comp$prefix, comp$recipe_id), function(x) sort(unique(x))) - ig_recipe_ids <- unique( - comp$recipe_id[substr(comp$component_code, 1L, 1L) %in% c("M", "L")] - ) + ig_recipe_ids <- unique(comp$recipe_id[comp$prefix %in% c("M", "L")]) find_counterpart <- function(rid) { - own <- suffix_sets[[rid]] - if (is.null(own)) return(NULL) + own_prefix <- prefix_sets[[rid]] + own_suffix <- suffix_sets[[rid]] + if (is.null(own_prefix) || is.null(own_suffix)) return(NULL) + if (!all(own_prefix %in% flow_prefixes)) return(NULL) + if (!all(own_prefix %in% c("E", "F", "G"))) return(NULL) for (cand in ig_recipe_ids) { if (identical(cand, rid)) next - if (setequal(suffix_sets[[cand]], own)) return(cand) + if (setequal(suffix_sets[[cand]], own_suffix)) return(cand) } NULL } diff --git a/tests/testthat/test-expenditure-concept.R b/tests/testthat/test-expenditure-concept.R index edde077..cd473a1 100644 --- a/tests/testthat/test-expenditure-concept.R +++ b/tests/testthat/test-expenditure-concept.R @@ -283,3 +283,63 @@ test_that("no suggestion fires for a healthy query", { r <- cog_spending("010000226085", years = 2019, category = "Police") expect_length(attr(r, "provenance")$suggestions, 0L) }) + +test_that("a mis-scoped cog_spending() call never attaches an M/L counterpart to a revenue-flavored recipe", { + # "IG Federal" is a revenue-only category (summary_categories maps it to + # B-prefixed component codes only; its recipes are ig_federal_b47_wide / + # ig_federal_b89_wide). A cog_spending() call scoped to it returns zero + # spending rows for every requested year -- there is no spending + # component in this category at all -- so the coverage-gap machinery + # fires for real (not hypothetically) even though this isn't the kind of + # format-boundary gap the recipe catalog is meant to signpost. This is + # exactly the live-corpus risk flagged in review: ig_federal_b47_wide's + # own component codes (B47/B94, suffixes {"47","94"}) are an EXACT + # suffix-set match for the expenditure recipe ige_local_m47_wide + # (M47/M94, same suffixes) -- a coincidence of reused digits, not a real + # Direct/Total pairing. The flow-family gate in + # .attach_ig_counterparts() must keep ig_recipe_id NULL here. + r <- suppressMessages( + cog_spending("010000226085", years = c(2005, 2011), category = "IG Federal") + ) + sugg <- attr(r, "provenance")$suggestions + expect_gt(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(".attach_ig_counterparts() never pairs a revenue-side recipe with its coincidental M/L suffix twin", { + # Broader version of the case above, run at the matching-helper level + # (the same level code review's pairwise enumeration was done at) rather + # than end-to-end: the fixture has no (govid, year) combination where + # cog_revenue() itself produces a covered gap for any B/C/D recipe, so an + # end-to-end repro for THIS specific set of recipes isn't reachable + # today. Each of these six recipes shares an exact suffix set with an + # M/L expenditure recipe purely by reused-digit coincidence: + # ig_federal_b47_wide {"47","94"} == ige_local_m47_wide / ige_state_l47_wide + # ig_federal_b89_wide {"89","91","92","93"} == ige_local_m89_wide / ige_state_l89_wide + # ig_state_c47_wide {"47","94"} == ige_local_m47_wide / ige_state_l47_wide + # ig_state_c89_wide {"89","91","92","93"} == ige_local_m89_wide / ige_state_l89_wide + # ig_local_d47_wide {"47","94"} == ige_local_m47_wide / ige_state_l47_wide + # ig_local_d89_wide {"89","91","92","93"} == ige_local_m89_wide / ige_state_l89_wide + # None of them may receive an ig_recipe_id under cog_revenue()'s own + # flow_prefixes, since M/L only ever pairs with the direct-expenditure + # (E/F/G) family. + con <- uscogdata:::.ensure_session() + fake_suggestion <- function(rid) { + list(recipe_id = rid, label = "x", available_years = c(1967L, 2023L), + hint = "h") + } + fake_suggestions <- lapply( + c("ig_federal_b47_wide", "ig_federal_b89_wide", + "ig_state_c47_wide", "ig_state_c89_wide", + "ig_local_d47_wide", "ig_local_d89_wide"), + fake_suggestion + ) + out <- uscogdata:::.attach_ig_counterparts( + con, fake_suggestions, c("T", "A", "U", "B", "C", "D") + ) + ig <- unlist(lapply(out, function(s) s$ig_recipe_id)) + expect_length(ig, 0L) +})