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.
This commit is contained in:
+2
-1
@@ -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
|
||||
|
||||
+62
-15
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user