fix(balances): validate the full signature, surface caveats in cog_explain, memoise coverage windows (#25)
Final-review findings F-1, F-2, F-6, F-8 (plus the F-9 @return reword,
which shares R/balances.R).
F-2: .validate_balance_inputs() checked 2 of cog_balances()' 7 arguments.
years = integer(0) leaked a raw DuckDB 'Parser Error ... AND year IN ()'
with the generated SQL echoed back; govid = character(0) and a non-character
category returned 0 rows with no error at all; recipe = c("a","b") threw
'the condition has length > 1' from inside .validate_recipe_id(). Replaced
with a call to the money verbs' own .validate_verb_inputs() (R/spending.R),
which validates the exact superset needed. Deleted the local copy rather
than extending it -- two validators is how they drift. Placed AFTER
.coerce_govid_input(), because .validate_verb_inputs() asserts
is.character(govid) and a data-frame govid is not unwrapped before that.
This is helper reuse of the same kind as .build_verb_sql()/.attach_per_capita();
the verb still does NOT route through .verb_spendrev().
F-1: falls out of F-2 for free -- the recipe/category mutual-exclusivity
guard lives inside .validate_verb_inputs(). Previously recipe silently
discarded category AND overwrote provenance$category with the recipe label,
so a caller asking for Fund Balances got X40/Z77 insurance-trust holdings
with no trace of the dropped filter.
F-6: cog_explain() rendered every provenance caveat block except
balance_caveats. Since .emit_balance_caveats() fires at most once per
session -- and is routinely consumed by a suppressMessages() call or an
unread knitr chunk -- cog_explain() is the only surface left for a caller
who deliberately audits the result. Added a 'Holdings caveats' section
guarded on !is.null(prov$balance_caveats). Also relabels the cosmetic
'Concept: NA' line on balance results as 'not applicable (holdings are a
stock, not a flow)'.
F-8: the coverage-window query has no govid and no year predicate -- its
answer depends only on the mounted corpus -- yet it scanned all of
balance_long on every call (35% of verb runtime on the fixture, and a
per-request throughput ceiling for cog-api#26). Memoised in
.uscogdata_env$balance_coverage_windows, invalidated by cog_close(), the
same pattern as .uscogdata_env$manifest.
This commit is contained in:
+20
-24
@@ -44,14 +44,17 @@
|
||||
#' @return Tibble with columns `year`, `canonical_govid`, `gov_name`,
|
||||
#' `balance_subtype`, `category`, `amt_nominal`, `codes_included`,
|
||||
#' `aggregate_fallback`, plus optional `amt_per_capita_nominal` and
|
||||
#' `pop_source` (when `per_capita = TRUE`), and optional `amt_real` and
|
||||
#' `amt_per_capita_real` (when `adjust_to_year` is set). Amounts are full
|
||||
#' US dollars.
|
||||
#' `pop_source` (when `per_capita = TRUE`), optional `amt_real` (when
|
||||
#' `adjust_to_year` is set), and optional `amt_per_capita_real` (only when
|
||||
#' **both** `per_capita = TRUE` and `adjust_to_year` are set -- there is no
|
||||
#' nominal per-capita column to deflate otherwise). Amounts are full US
|
||||
#' dollars.
|
||||
#'
|
||||
#' Carries a `provenance` attribute matching
|
||||
#' `inst/schemas/provenance-v1.json`, whose `balance_caveats` block reports
|
||||
#' `not_gaap`, `not_gaap_note`, `coverage_window` (measured per-subtype year
|
||||
#' extents) and `truncated` (subtypes whose coverage falls short of the
|
||||
#' `not_gaap`, `not_gaap_note`, `coverage_window` (measured year extents for
|
||||
#' every balance subtype in the mounted corpus, not only the observed ones)
|
||||
#' and `truncated` (the observed subtypes whose coverage falls short of the
|
||||
#' requested years). `expenditure_concept`/`revenue_concept` are `NA` --
|
||||
#' holdings are a stock, not a flow, so neither concept vocabulary applies.
|
||||
#' @export
|
||||
@@ -60,8 +63,19 @@ cog_balances <- function(govid, years, category = NULL,
|
||||
basis = c("harmonized", "raw"), recipe = NULL) {
|
||||
call <- match.call()
|
||||
basis <- match.arg(basis, c("harmonized", "raw"))
|
||||
.validate_balance_inputs(per_capita, adjust_to_year)
|
||||
# Coerce FIRST, validate second: .validate_verb_inputs() asserts
|
||||
# is.character(govid), and a data-frame govid (cog_gov_search() output) has
|
||||
# not been unwrapped yet at this point.
|
||||
govid <- .coerce_govid_input(govid)
|
||||
# The money verbs' validator, reused rather than re-implemented (R/spending.R).
|
||||
# It covers the exact superset cog_balances() needs -- including the
|
||||
# recipe/category mutual-exclusivity guard -- so a second local copy would
|
||||
# only be a place for the two to drift apart. This is the same kind of
|
||||
# helper reuse as .build_verb_sql()/.attach_per_capita() below; it does NOT
|
||||
# route the verb through .verb_spendrev(), which stays deliberately unused
|
||||
# here because its flow vocabulary is meaningless for a stock.
|
||||
.validate_verb_inputs(govid, years, category, per_capita, adjust_to_year,
|
||||
recipe)
|
||||
years <- as.integer(years)
|
||||
if (!is.null(adjust_to_year)) adjust_to_year <- as.integer(adjust_to_year)
|
||||
|
||||
@@ -129,24 +143,6 @@ cog_balances <- function(govid, years, category = NULL,
|
||||
result
|
||||
}
|
||||
|
||||
#' Cheap type validation for the two arguments cog_balances() shares with the
|
||||
#' money verbs. Mirrors the per_capita/adjust_to_year checks in
|
||||
#' .validate_verb_inputs() (R/spending.R) -- category/recipe validation is
|
||||
#' deliberately out of scope here (uscogdata#25 Task 3 review note).
|
||||
#' @noRd
|
||||
.validate_balance_inputs <- function(per_capita, adjust_to_year) {
|
||||
if (!is.logical(per_capita) || length(per_capita) != 1L) {
|
||||
cli::cli_abort("`per_capita` must be a length-1 logical.")
|
||||
}
|
||||
if (!is.null(adjust_to_year)) {
|
||||
if (!(is.integer(adjust_to_year) || is.numeric(adjust_to_year)) ||
|
||||
length(adjust_to_year) != 1L) {
|
||||
cli::cli_abort("`adjust_to_year` must be NULL or a length-1 integer.")
|
||||
}
|
||||
}
|
||||
invisible(TRUE)
|
||||
}
|
||||
|
||||
#' Abort unless the mounted corpus classifies balance codes.
|
||||
#'
|
||||
#' `balance_subtype` arrived with cog_pipeline #76/#77 without a
|
||||
|
||||
Reference in New Issue
Block a user