diff --git a/R/spending.R b/R/spending.R index 70582b6..bb4f641 100644 --- a/R/spending.R +++ b/R/spending.R @@ -28,13 +28,20 @@ #' folding). On a corpus with `schema_version < 5` (no harmonization #' tables), `basis` silently resolves to `"raw"` when left at its default #' and the resolution is recorded in the provenance; explicitly passing -#' `basis = "harmonized"` on such a corpus aborts. +#' `basis = "harmonized"` on such a corpus aborts. Ignored when `recipe` +#' is set (see below). #' @param recipe Optional harmonization recipe id (see [cog_recipes()]) for #' multi-code cross-vintage series that a 1:1 harmonized_code mapping #' can't express (e.g. a wide-era aggregate that only splits into leaf #' codes in the modern era). Mutually exclusive with `category`. The #' result's subtype column reads `"recipe"` and `category` reads the -#' recipe's label. Requires `schema_version >= 5`. +#' recipe's label. Requires `schema_version >= 5`. A recipe query bypasses +#' `basis` entirely (it joins `long` directly rather than going through +#' the `*_annotated`/`*_annotated_harmonized` views), so the `basis` +#' argument is ignored and the result's provenance reports +#' `basis = "recipe"` with an inert `harmonization` block (`applied = +#' FALSE`, pointing at the `recipe` block instead) rather than a +#' possibly-misleading `"harmonized"`/`"raw"` value. #' @return Tibble with columns `year`, `canonical_govid`, `gov_name`, #' `spend_subtype`, `category`, `amt_nominal`, optional `amt_real`, #' optional `amt_per_capita_nominal`, optional `amt_per_capita_real`, @@ -109,14 +116,32 @@ cog_spending <- function(govid, years, category = NULL, result$notes <- .notes_column(result) - harmonization <- .build_harmonization_block( - con, govid, years, resolved, flow_prefixes - ) - - suggestions <- if (is.null(recipe)) { - .build_suggestions(con, govid, years, category, result, resolved$basis) + # A recipe result doesn't go through spending_annotated(_harmonized) / + # revenue_annotated(_harmonized) at all -- .run_recipe()'s generic join + # reads `long` directly -- so `basis` and the `harmonization` exclusion + # count (which is itself computed from `long`, independent of which view + # a non-recipe query used) would describe a code path this result never + # took. Rather than report a technically-still-computed but misleading + # basis = "harmonized"/"raw" + harmonization$applied combo, recipe + # results report basis = "recipe" and an explicit, inert harmonization + # block pointing at the `recipe` block instead. Task 12 (cog-api) passes + # provenance through verbatim, so this needs to be unambiguous rather + # than technically-defensible-but-confusing. + if (!is.null(recipe)) { + basis_for_prov <- "recipe" + basis_note_for_prov <- NA_character_ + harmonization <- list( + applied = FALSE, na_rows_excluded = 0L, na_amount_excluded = 0, + note = "basis/harmonization not applicable to recipe results; see the recipe block instead" + ) + suggestions <- list() } else { - list() + basis_for_prov <- resolved$basis + basis_note_for_prov <- resolved$note + harmonization <- .build_harmonization_block( + con, govid, years, resolved, flow_prefixes + ) + suggestions <- .build_suggestions(con, govid, years, category, result, resolved$basis) } prov <- .build_provenance( @@ -130,8 +155,8 @@ cog_spending <- function(govid, years, category = NULL, result = result, sql = sql, subtype_col = subtype_col, - basis = resolved$basis, - basis_note = resolved$note, + basis = basis_for_prov, + basis_note = basis_note_for_prov, harmonization = harmonization, recipe = recipe_block, suggestions = suggestions diff --git a/man/cog_revenue.Rd b/man/cog_revenue.Rd index 18b77df..843d48e 100644 --- a/man/cog_revenue.Rd +++ b/man/cog_revenue.Rd @@ -40,14 +40,21 @@ reproduces the pre-Phase-R2 behavior (published item codes, no folding). On a corpus with `schema_version < 5` (no harmonization tables), `basis` silently resolves to `"raw"` when left at its default and the resolution is recorded in the provenance; explicitly passing -`basis = "harmonized"` on such a corpus aborts.} +`basis = "harmonized"` on such a corpus aborts. Ignored when `recipe` +is set (see below).} \item{recipe}{Optional harmonization recipe id (see [cog_recipes()]) for multi-code cross-vintage series that a 1:1 harmonized_code mapping can't express (e.g. a wide-era aggregate that only splits into leaf codes in the modern era). Mutually exclusive with `category`. The result's subtype column reads `"recipe"` and `category` reads the -recipe's label. Requires `schema_version >= 5`.} +recipe's label. Requires `schema_version >= 5`. A recipe query bypasses +`basis` entirely (it joins `long` directly rather than going through +the `*_annotated`/`*_annotated_harmonized` views), so the `basis` +argument is ignored and the result's provenance reports +`basis = "recipe"` with an inert `harmonization` block (`applied = +FALSE`, pointing at the `recipe` block instead) rather than a +possibly-misleading `"harmonized"`/`"raw"` value.} } \value{ Tibble with columns `year`, `canonical_govid`, `gov_name`, diff --git a/man/cog_spending.Rd b/man/cog_spending.Rd index 8e4c52a..f1016ec 100644 --- a/man/cog_spending.Rd +++ b/man/cog_spending.Rd @@ -40,14 +40,21 @@ reproduces the pre-Phase-R2 behavior (published item codes, no folding). On a corpus with `schema_version < 5` (no harmonization tables), `basis` silently resolves to `"raw"` when left at its default and the resolution is recorded in the provenance; explicitly passing -`basis = "harmonized"` on such a corpus aborts.} +`basis = "harmonized"` on such a corpus aborts. Ignored when `recipe` +is set (see below).} \item{recipe}{Optional harmonization recipe id (see [cog_recipes()]) for multi-code cross-vintage series that a 1:1 harmonized_code mapping can't express (e.g. a wide-era aggregate that only splits into leaf codes in the modern era). Mutually exclusive with `category`. The result's subtype column reads `"recipe"` and `category` reads the -recipe's label. Requires `schema_version >= 5`.} +recipe's label. Requires `schema_version >= 5`. A recipe query bypasses +`basis` entirely (it joins `long` directly rather than going through +the `*_annotated`/`*_annotated_harmonized` views), so the `basis` +argument is ignored and the result's provenance reports +`basis = "recipe"` with an inert `harmonization` block (`applied = +FALSE`, pointing at the `recipe` block instead) rather than a +possibly-misleading `"harmonized"`/`"raw"` value.} } \value{ Tibble with columns `year`, `canonical_govid`, `gov_name`, diff --git a/tests/testthat/test-recipes.R b/tests/testthat/test-recipes.R index 44a1bf3..0bf9518 100644 --- a/tests/testthat/test-recipes.R +++ b/tests/testthat/test-recipes.R @@ -69,7 +69,7 @@ test_that("recipe result carries a recipe provenance block with component rows", r <- cog_spending("121011212191", years = c(2011L, 2012L), recipe = "corrections_combined") prov <- attr(r, "provenance") - expect_equal(prov$basis, "harmonized") + expect_equal(prov$basis, "recipe") expect_equal(prov$category, "Corrections (functions 04+05 combined)") expect_type(prov$recipe, "list") expect_equal(prov$recipe$recipe_id, "corrections_combined") @@ -82,6 +82,34 @@ test_that("recipe result carries a recipe provenance block with component rows", expect_length(prov$suggestions, 0L) }) +test_that("recipe results report an unambiguous basis/harmonization, ignoring basis=", { + skip_if_no_corpus() + # A recipe query bypasses spending_annotated(_harmonized) entirely -- + # .run_recipe() joins `long` directly -- so `basis` must never read + # "harmonized"/"raw" (which would describe a code path this query never + # took) regardless of what the caller passed for `basis`. Task 12 + # consumes provenance verbatim, so this needs to be unambiguous. + r_default <- cog_spending("121011212191", years = c(2011L, 2012L), + recipe = "corrections_combined") + r_raw <- cog_spending("121011212191", years = c(2011L, 2012L), + recipe = "corrections_combined", basis = "raw") + r_harm <- cog_spending("121011212191", years = c(2011L, 2012L), + recipe = "corrections_combined", basis = "harmonized") + + for (r in list(r_default, r_raw, r_harm)) { + prov <- attr(r, "provenance") + expect_equal(prov$basis, "recipe") + expect_true(is.na(prov$basis_note)) + expect_false(prov$harmonization$applied) + expect_equal(prov$harmonization$na_rows_excluded, 0L) + expect_match(prov$harmonization$note, "recipe", ignore.case = TRUE) + } + + # basis= truly has zero effect on a recipe query's actual numbers. + expect_equal(r_raw$amt_nominal, r_harm$amt_nominal) + expect_equal(r_default$amt_nominal, r_raw$amt_nominal) +}) + test_that("recipe = 't19_selective_sales_wide' sums the local T11/T14 legs when present", { skip_if_no_corpus() # Westminster City, CA (canonical_govid 082001211654): T11 = 0 in 2011, diff --git a/tests/testthat/test-views.R b/tests/testthat/test-views.R index fe30aad..75eaa5b 100644 --- a/tests/testthat/test-views.R +++ b/tests/testthat/test-views.R @@ -14,41 +14,100 @@ test_that("all expected views register on session open", { expect_true(all(expected %in% views$table_name)) }) -test_that("the REPLACE(harmonized_code AS item_code) pattern folds a collapsed code", { - # spending_long_harmonized / revenue_long_harmonized (inst/sql/22-, 23-) - # are defined as: - # SELECT * REPLACE (harmonized_code AS item_code) FROM long WHERE ... +test_that("inst/sql/22- and 23- harmonized views enforce every WHERE predicate (real SQL text, synthetic parquet)", { + # spending_long_harmonized / revenue_long_harmonized are three-predicate + # views: + # SELECT * REPLACE (harmonized_code AS item_code) + # FROM long + # WHERE NOT is_aggregate + # AND harmonized_code IS NOT NULL + # AND LEFT(harmonized_code, 1) IN () # None of the curated harmonization_map's `collapse` rulings land inside # the bundled fixture's 2011-2020 window for spending/revenue-prefixed # codes (see the "basis = 'harmonized' (default) matches 'raw'" test in # test-spending.R and docs/phase_r_harmonization_review.md § 0.2/§ 2), so - # there is no real fixture row that exercises a nonzero fold. This test - # proves the REPLACE mechanism itself is correct against a synthetic - # long-shaped table with a deliberate E38 -> E36 collapse, independent of - # whether the bundled data happens to contain one right now. + # there is no real fixture row that exercises a nonzero fold or a + # predicate-excluded row. Rather than re-implement the WHERE clause by + # hand against an in-memory VALUES table (which would only prove the SQL + # *pattern* works, not that the deployed inst/sql/22-/23- text actually + # applies it), this test reads the real SQL files off disk, substitutes + # {url} exactly as .register_views() does, and executes them -- plus + # their 10-long.sql dependency -- against a synthetic hive-partitioned + # parquet tree written to a temp dir. A regression in any predicate (e.g. + # `NOT is_aggregate` dropped, the prefix list changed, the NULL guard + # removed) would change which of the rows below survive. + # + # The synthetic parquet is written with DuckDB's own COPY ... TO (FORMAT + # PARQUET) rather than the arrow package: this package has no arrow + # dependency (CLAUDE.md "No arrow dependency -- DuckDB reads parquet + # natively"), and DuckDB can round-trip its own parquet writer/reader + # without adding one for tests either. skip_if_no_corpus() - con <- cog_open() - on.exit(cog_close()) - DBI::dbExecute(con, " - CREATE OR REPLACE TEMP TABLE synthetic_long AS - SELECT * FROM (VALUES - ('121011212191', 2004, 'E36', 70942668, false, 'E36'), - ('121011212191', 2004, 'E38', 837485, false, 'E36'), - ('121011212191', 2004, 'E62', 100000, false, 'E62') - ) AS t(canonical_govid, year, item_code, amt, is_aggregate, harmonized_code) - ") + tmp <- withr::local_tempdir() + part_dir <- file.path(tmp, "data", "long", "year=2004") + dir.create(part_dir, recursive = TRUE) + part_path <- file.path(part_dir, "part-0.parquet") - folded <- DBI::dbGetQuery(con, " - SELECT item_code, SUM(amt) AS amt - FROM (SELECT * REPLACE (harmonized_code AS item_code) FROM synthetic_long) - GROUP BY item_code ORDER BY item_code - ") - expect_setequal(folded$item_code, c("E36", "E62")) - expect_equal(folded$amt[folded$item_code == "E36"], 70942668 + 837485) - expect_equal(folded$amt[folded$item_code == "E62"], 100000) + write_con <- DBI::dbConnect(duckdb::duckdb()) + on.exit(DBI::dbDisconnect(write_con, shutdown = TRUE), add = TRUE) + DBI::dbExecute(write_con, sprintf(" + COPY ( + SELECT * FROM (VALUES + -- Spending (E/F/G/K) rows, exercised against spending_long_harmonized: + ('spend-A', 'E36', 100, false, 'E36'), -- control: passes every predicate as-is + ('spend-B', 'E38', 50, false, 'E36'), -- collapse-fold: passes every predicate, renamed to E36 + ('spend-C', 'E05', 999999, true, 'E05'), -- excluded ONLY by `NOT is_aggregate` + ('spend-D', 'E99', 888888, false, NULL), -- excluded by `harmonized_code IS NOT NULL` + -- 'S74' is outside BOTH flow families (E/F/G/K spending and + -- T/A/U/B/C/D revenue -- it mirrors the real corpus's own + -- non-flow-type codes like S74/Z61), so it can only leak into + -- EITHER view via the E/F/G/K or T/A/U/B/C/D prefix filter, never + -- both at once -- a prefix drawn from the other view's own family + -- (e.g. a real T-code for the spending row) would incorrectly + -- leak into the other view's assertion below and not discriminate + -- the predicate under test. + ('spend-E', 'S74', 777777, false, 'S74'), -- excluded ONLY by the E/F/G/K prefix filter + -- Revenue (T/A/U/B/C/D) rows, exercised against revenue_long_harmonized: + ('rev-A', 'U11', 200, false, 'U11'), -- control: passes every predicate as-is + ('rev-B', 'U10', 25, false, 'U11'), -- collapse-fold: passes every predicate, renamed to U11 + ('rev-C', 'T29', 555555, true, 'T29'), -- excluded ONLY by `NOT is_aggregate` + ('rev-D', 'T88', 444444, false, NULL), -- excluded by `harmonized_code IS NOT NULL` + ('rev-E', 'Z61', 333333, false, 'Z61') -- excluded ONLY by the T/A/U/B/C/D prefix filter + ) AS t(canonical_govid, item_code, amt, is_aggregate, harmonized_code) + ) TO %s (FORMAT PARQUET) + ", uscogdata:::.sql_lit_chr(part_path))) - DBI::dbExecute(con, "DROP TABLE synthetic_long") + sql_dir <- system.file("sql", package = "uscogdata") + .read_view_sql <- function(filename) { + txt <- paste(readLines(file.path(sql_dir, filename), warn = FALSE), collapse = "\n") + gsub("\\{url\\}", paste0(tmp, "/"), txt, fixed = FALSE) + } + + con <- DBI::dbConnect(duckdb::duckdb()) + on.exit(DBI::dbDisconnect(con, shutdown = TRUE), add = TRUE) + DBI::dbExecute(con, .read_view_sql("10-long.sql")) + DBI::dbExecute(con, .read_view_sql("22-spending_long_harmonized.sql")) + DBI::dbExecute(con, .read_view_sql("23-revenue_long_harmonized.sql")) + + spend <- DBI::dbGetQuery(con, + "SELECT item_code, SUM(amt) AS amt FROM spending_long_harmonized + GROUP BY item_code ORDER BY item_code" + ) + # Exactly one surviving row: spend-C (aggregate), spend-D (NULL + # harmonized_code), and spend-E (wrong prefix family) must all be gone, + # and spend-A + spend-B must be folded together under E36. + expect_equal(nrow(spend), 1L) + expect_equal(spend$item_code, "E36") + expect_equal(spend$amt, 150) + + rev <- DBI::dbGetQuery(con, + "SELECT item_code, SUM(amt) AS amt FROM revenue_long_harmonized + GROUP BY item_code ORDER BY item_code" + ) + expect_equal(nrow(rev), 1L) + expect_equal(rev$item_code, "U11") + expect_equal(rev$amt, 225) }) test_that(".build_series_break_refs matches fin_code + break_year window", {