Merge pull request 'fix(suggestions): scope candidate recipes by category_type (#34)' (#70) from fix/category-type-34 into main

.query_candidate_recipes() (extracted in #33) now filters candidates by
category_type ('expenditure' vs 'revenue'), derived from the calling
verb's own flow_prefixes (E/F/G -> 'expenditure', else 'revenue').

Without this, a category shared across both flow families in
summary_categories leaked cross-family recipes: cog_revenue(category =
"Corrections") surfaced the expenditure-only corrections_combined recipe
(E04/E05) merely because "Corrections" is also a spending category name,
and cog_spending(category = "IG Federal") surfaced the revenue-only
ig_federal_b47_wide recipe. Both are wrong: following either hint would
attribute dollars to the wrong flow, or (IG Federal) fire the
coverage-gap machinery for a category the calling verb structurally
cannot report on at all.

Updates the two tests this changes the expected behavior of:
- "a mis-scoped cog_spending() call never attaches an M/L counterpart to
  a revenue-flavored recipe" (test-expenditure-concept.R): IG Federal is
  revenue-only, so a spending call now finds zero candidates outright
  rather than firing the suggestion and then blocking its M/L
  counterpart as a second-order check.
- "cog_revenue never suggests expenditure-only recipes"
  (test-recipes.R, was "I1: ... never fabricates suppressed dollars"):
  corrections_combined is expenditure-only, so a revenue call now never
  considers it as a candidate, rather than considering it and reporting
  zero suppressed dollars.

All 1078 tests pass (2 skipped live-corpus), measured devtools::test()
against this commit in a clean worktree stacked on the #33 refactor.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

.Rbuildignore

@@ -3,6 +3,7 @@
 ^\.Rproj\.user$
 ^_pkgdown\.yml$
 ^docs$
+^pm$
 ^Meta$
 ^doc$
 ^pkgdown$

.gitignore

@@ -4,17 +4,11 @@
 .Ruserdata
 *.Rproj
 inst/doc
-# pkgdown output. Listed as children rather than `docs/` so compass's
-# docs/pm/ and docs/decisions/ can be re-included -- git cannot re-include
-# anything beneath an excluded directory.
-#
-# Note the anchoring change this forces: a bare `docs/` matches a directory of
-# that name at ANY depth, while `/docs/*` matches only at the repo root. The
-# fixture corpus's own docs/ therefore needs its own rule to stay excluded.
-/docs/*
-!/docs/pm/
-!/docs/decisions/
-inst/extdata/fixture_corpus/docs/
+# pkgdown output. Compass used to keep its files in docs/pm/ and
+# docs/decisions/, which forced this to be written as children with two
+# re-includes -- git cannot re-include anything beneath an excluded directory.
+# Compass lives in pm/ now, so the whole directory can be excluded again.
+docs/
 /doc/
 /Meta/
 .DS_Store

.roborev.toml

@@ -2,7 +2,7 @@
 # Reviews are queued to a background daemon -- they never block a commit.
 
 post_commit_review = 'commit'
-excluded_commit_patterns = ['WIP', 'chore:', 'docs:', 'Merge ']
+excluded_commit_patterns = ['WIP', 'chore:', 'chore(', 'docs:', 'Merge ']
 
 review_guidelines = '''
 # --- compass:begin (generated -- edit the sources, not this) ---
@@ -19,6 +19,10 @@ review_guidelines = '''
   deeper than four levels.
 - No magic numbers or hardcoded paths -- name them as constants or read them from config.
 - New behaviour needs a test. A bug fix needs a test that fails without the fix.
+- Prose a person reads -- an issue title or body, a journal entry, a decision record,
+  the narrative on the status board -- names the action or the thing, not the shape of
+  the machinery. Flag "gate", "seam", "surface area", "load-bearing", "first-class",
+  "primitive", "blast radius". A project's own defined vocabulary is not the target.
 - Use the native pipe `|>`, not magrittr `%>%`.
 - snake_case for objects and functions; UPPER_SNAKE for constants. Never use `.` as a
   word separator in a function name -- it collides with S3 dispatch.

R/suggestions.R

@@ -84,6 +84,21 @@
 #' @return List of `list(recipe_id, label, available_years, hint,
 #'   ig_recipe_id, trigger, suppressed_amount, suppressed_years,
 #'   suppressed_codes)`, possibly empty.
+#'
+#' Decomposed (Issue #33) into three extracted helpers to stay within the
+#' project's "functions under 50 lines" convention:
+#' \itemize{
+#'   \item `.query_candidate_recipes()` -- candidate recipe lookup by
+#'     category/subtype scope + `category_type` filter (#34) + M/L exclusion.
+#'   \item `.query_recipe_meta()` -- metadata (label, year spans).
+#'   \item `.query_covered_years()` -- Path 1 gap-year coverage via the
+#'     recipe's own generic join.
+#' }
+#' The for-loop that merges covered-years + suppressed-components into
+#' suggestion objects stays inline here because it interleaves
+#' empty_hit/supp_hit precedence with field assembly. Likewise kept inline:
+#' the M/L-exclusion design-comment block and the final
+#' `.attach_ig_counterparts()` call.
 #' @noRd
 .build_suggestions <- function(con, cohort, years, category, result, basis,
                                 flow_prefixes, long_view,
@@ -111,28 +126,16 @@
   # by `category` (`.ALL_CATEGORIES` is never a row in
   # `summary_categories.category`, so a category-keyed sub-select always
   # came back empty here). The M/L exclusion below is unchanged either way.
-  candidate_scope_sql <- if (isTRUE(all_categories)) {
-    sprintf(
-      "SELECT DISTINCT item_code FROM summary_categories WHERE %s IN (%s)",
-      subtype_col, .sql_lit_chr(subtype_scope)
-    )
-  } else {
-    sprintf(
-      "SELECT DISTINCT item_code FROM summary_categories WHERE category IN (%s)",
-      .sql_lit_chr(category)
-    )
-  }
-  candidates <- DBI::dbGetQuery(con, sprintf(
-    "SELECT DISTINCT recipe_id FROM harmonization_recipes
-     WHERE component_code IN (
-       %s
-     )
-     AND recipe_id NOT IN (
-       SELECT DISTINCT recipe_id FROM harmonization_recipes
-       WHERE LEFT(component_code, 1) IN ('M', 'L')
-     )",
-    candidate_scope_sql
-  ))$recipe_id
+  #
+  # Issue #34: scope the candidate query by `category_type` ('expenditure'
+  # vs 'revenue') to prevent cross-flow-family leakage -- e.g.
+  # `cog_revenue(category = "Corrections")` must not surface
+  # expenditure-only recipes (E04/E05) merely because they share the same
+  # category name in summary_categories. The type is derived from
+  # flow_prefixes: E/F/G -> 'expenditure', anything else -> 'revenue'.
+  candidates <- .query_candidate_recipes(con, category, flow_prefixes,
+                                          all_categories, subtype_col,
+                                          subtype_scope)
   if (length(candidates) == 0L) return(list())
 
   result_years <- if (is.null(result) || nrow(result) == 0L) {
@@ -164,36 +167,11 @@
 
   if (length(gap_years) == 0L && nrow(supp) == 0L) return(list())
 
-  meta <- tibble::as_tibble(DBI::dbGetQuery(con, sprintf(
-    "SELECT recipe_id, any_value(label) AS label,
-            MIN(year_min) AS year_min, MAX(year_max) AS year_max
-     FROM harmonization_recipes
-     WHERE recipe_id IN (%s)
-     GROUP BY recipe_id",
-    .sql_lit_chr(candidates)
-  )))
+  meta <- .query_recipe_meta(con, candidates)
 
   # Path 1 (unchanged): (recipe, year) pairs the recipe's own generic join
   # covers for this government, restricted to the gap years.
-  covered <- if (length(gap_years) == 0L) {
-    data.frame(recipe_id = character(0), year = integer(0))
-  } else {
-    DBI::dbGetQuery(con, sprintf(
-      "SELECT DISTINCT r.recipe_id, l.year
-       FROM long l
-       JOIN harmonization_recipes r
-         ON l.item_code = r.component_code
-        AND l.year BETWEEN r.year_min AND r.year_max
-        AND (r.gov_type_scope = 'all'
-             OR (r.gov_type_scope = 'state' AND l.type = 0)
-             OR (r.gov_type_scope = 'local' AND l.type BETWEEN 1 AND 3))
-       WHERE r.recipe_id IN (%s)
-         AND %s
-         AND l.year IN (%s)",
-      .sql_lit_chr(candidates), .cohort_sql(cohort, "l.canonical_govid"),
-      paste(gap_years, collapse = ",")
-    ))
-  }
+  covered <- .query_covered_years(con, candidates, cohort, gap_years)
 
   suggestions <- list()
   for (rid in candidates) {
@@ -227,6 +205,144 @@
   .attach_ig_counterparts(con, suggestions, flow_prefixes)
 }
 
+#' Query candidate harmonization recipe IDs for a coverage-gap suggestion.
+#'
+#' Selects recipes whose component codes fall within the requested scope
+#' (category or subtype allowlist), excluding any recipe that is ITSELF an
+#' intergovernmental (M/L) recipe -- i.e. every one of its own component
+#' codes is M/L-prefixed. Without this exclusion, a category whose
+#' summary_categories rows span both a Direct family (e.g. E04/E05,
+#' "Corrections") and its M/L counterpart (M04/M05) makes the M/L recipe
+#' itself a raw top-level candidate for a plain `cog_spending()` call --
+#' following that hint would silently return intergovernmental dollars
+#' under `expenditure_concept = "direct"` provenance.
+#'
+#' In all-categories mode (`all_categories = TRUE`) the inner sub-select is
+#' scoped by `subtype_col`/`subtype_scope` -- the same allowlist
+#' `.build_verb_sql()` applies as a WHERE predicate to make the summed
+#' result a *concept* (see R/spending.R), not by `category`.
+#' `.ALL_CATEGORIES` ("All Categories") is never itself a row in
+#' `summary_categories.category`, so a category-keyed sub-select always
+#' returns zero candidates and silently disables signposting.
+#'
+#' Scope is also by `category_type` ('expenditure' vs 'revenue', Issue #34)
+#' to prevent cross-flow-family leakage: `cog_revenue(category =
+#' "Corrections")` must not surface expenditure-only recipes (E04/E05)
+#' merely because they share the same category name in summary_categories.
+#' The type is derived from flow_prefixes: E/F/G -> 'expenditure', anything
+#' else -> 'revenue'.
+#'
+#' @param con Active DuckDB connection.
+#' @param category Category name, or `NULL`.
+#' @param flow_prefixes The calling verb's own flow-type prefixes (see
+#'   `.build_suggestions()`). Used to derive `category_type` (#34).
+#' @param all_categories `TRUE` when the caller used `.ALL_CATEGORIES`.
+#' @param subtype_col Name of the summary_categories subtype column to
+#'   scope by when `all_categories = TRUE`; ignored otherwise.
+#' @param subtype_scope Character vector of subtype values to scope by
+#'   when `all_categories = TRUE`; ignored otherwise.
+#' @return Character vector of recipe IDs (possibly empty).
+#' @noRd
+.query_candidate_recipes <- function(con, category, flow_prefixes,
+                                      all_categories = FALSE,
+                                      subtype_col = NULL,
+                                      subtype_scope = NULL) {
+  # Issue #34: derive category_type from flow_prefixes to prevent
+  # cross-flow-family leakage -- e.g. cog_revenue(category = "Corrections")
+  # must not surface expenditure-only recipes merely because they share the
+  # same category name in summary_categories.
+  category_type <- if (all(flow_prefixes %in% c("E", "F", "G"))) {
+    "expenditure"
+  } else {
+    "revenue"
+  }
+
+  candidate_scope_sql <- if (isTRUE(all_categories)) {
+    sprintf(
+      "SELECT DISTINCT item_code FROM summary_categories
+       WHERE %s IN (%s) AND category_type = '%s'",
+      subtype_col, .sql_lit_chr(subtype_scope), category_type
+    )
+  } else {
+    sprintf(
+      "SELECT DISTINCT item_code FROM summary_categories
+       WHERE category IN (%s) AND category_type = '%s'",
+      .sql_lit_chr(category), category_type
+    )
+  }
+
+  DBI::dbGetQuery(con, sprintf(
+    "SELECT DISTINCT recipe_id FROM harmonization_recipes
+     WHERE component_code IN (
+       %s
+     )
+     AND recipe_id NOT IN (
+       SELECT DISTINCT recipe_id FROM harmonization_recipes
+       WHERE LEFT(component_code, 1) IN ('M', 'L')
+     )",
+    candidate_scope_sql
+  ))$recipe_id
+}
+
+#' Query gap-year coverage: which (recipe, year) pairs the recipe's own
+#' generic join covers for this government, restricted to `gap_years`.
+#'
+#' This is Path 1 of a suggestion (unchanged): it finds recipes whose
+#' component codes' generic join produces at least one row for this
+#' government in each gap year -- i.e. the category returned nothing in
+#' that year but a recipe would fill it.
+#'
+#' @param con Active DuckDB connection.
+#' @param candidates Character vector of recipe IDs to check coverage for.
+#' @param cohort The verb's cohort object (see `.make_cohort()`), rendered
+#'   into the govid predicate on the joined `long` scan via `.cohort_sql()`.
+#' @param gap_years Integer vector of requested years absent from the
+#'   result.
+#' @return Data frame with columns `recipe_id` (character) and `year`
+#'   (integer). Returns an empty data frame (`recipe_id = character(0)`,
+#'   `year = integer(0)`) when `gap_years` is empty, so callers can safely
+#'   reference `$recipe_id`.
+#' @noRd
+.query_covered_years <- function(con, candidates, cohort, gap_years) {
+  if (length(gap_years) == 0L) {
+    return(data.frame(recipe_id = character(0), year = integer(0)))
+  }
+  DBI::dbGetQuery(con, sprintf(
+    "SELECT DISTINCT r.recipe_id, l.year
+     FROM long l
+     JOIN harmonization_recipes r
+       ON l.item_code = r.component_code
+      AND l.year BETWEEN r.year_min AND r.year_max
+      AND (r.gov_type_scope = 'all'
+           OR (r.gov_type_scope = 'state' AND l.type = 0)
+           OR (r.gov_type_scope = 'local' AND l.type BETWEEN 1 AND 3))
+     WHERE r.recipe_id IN (%s)
+       AND %s
+       AND l.year IN (%s)",
+    .sql_lit_chr(candidates), .cohort_sql(cohort, "l.canonical_govid"),
+    paste(gap_years, collapse = ",")
+  ))
+}
+
+#' Query recipe metadata: labels and year spans for a set of candidate
+#' recipes.
+#'
+#' @param con Active DuckDB connection.
+#' @param candidates Character vector of recipe IDs to look up.
+#' @return Tibble with columns `recipe_id`, `label`, `year_min` (int), and
+#'   `year_max` (int).
+#' @noRd
+.query_recipe_meta <- function(con, candidates) {
+  tibble::as_tibble(DBI::dbGetQuery(con, sprintf(
+    "SELECT recipe_id, any_value(label) AS label,
+            MIN(year_min) AS year_min, MAX(year_max) AS year_max
+     FROM harmonization_recipes
+     WHERE recipe_id IN (%s)
+     GROUP BY recipe_id",
+    .sql_lit_chr(candidates)
+  )))
+}
+
 #' Attach `ig_recipe_id` to each suggestion: the intergovernmental-expenditure
 #' recipe (an M-to-local or L-to-state recipe) whose component codes cover
 #' exactly the same set of function suffixes as the firing recipe's own
@@ -272,7 +388,7 @@
 #'      `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.
+#'      "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
@@ -280,10 +396,6 @@
 #'      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, flow_prefixes) {
   if (length(suggestions) == 0L) return(suggestions)

pm/STATUS.md

@@ -14,14 +14,23 @@ The six open issues split cleanly. Two are API work carried out of the #9 review
 and deliberately deferred there rather than fixed in that branch. Three concern the
 corpus layer, and the largest of them, partition-level caching, was named the single
 highest-leverage change on the remote path before being deferred. One, the
-data-correction intake, is a decision rather than a task: it was parked during the
-0.3.0 design and it gates the API announcement, because without it the corpus cannot
-make the "traceable and correctable" claim that most distinguishes it from Census's
-own files.
+data-correction intake (#52), is a decision rather than a task: it was parked during
+the 0.3.0 design, and the API announcement waits on it, because without it the corpus
+cannot make the "traceable and correctable" claim that most distinguishes it from
+Census's own files.
 
 Nothing here is blocked on anything else, so the ordering is a judgement about value
 rather than a dependency graph.
 
+Compass's own files moved out of `docs/` this session. They were sitting inside
+pkgdown's output directory, and `pkgdown::clean_site()` deletes every top-level entry
+there except `CNAME` and `dev` — asked directly, it listed `docs/pm` and
+`docs/decisions` among the 28 it would remove, with the guard that would have stopped
+it satisfied by `docs/pkgdown.yml`. They are in `pm/` now. Nothing was lost: the
+journal had no entries and there were no decision records yet, which made this the
+cheapest moment to move. The `.gitignore` workaround that re-included two children of
+an excluded `docs/` is gone with it.
+
 ## Ready to work on next
 
 - **#34** cog_revenue() offers expenditure recipes as suggestions: scope the candidate query by category_type · `ws/api` — nothing is blocking it; something is currently wrong
@@ -47,25 +56,10 @@ rather than a dependency graph.
 <details>
 <summary>Dependency graph and detail</summary>
 
-```mermaid
-graph TD
-  I34["#34 cog_revenue() offers expenditure recipes as sug…"]
-  I36["#36 n_units_reporting is category-conditional and c…"]
-  I2["#2 Extend population data to be households as an a…"]
-  I33["#33 Decompose .build_suggestions() (106 lines) into…"]
-  I52["#52 Release 11/11: design the data-correction intak…"]
-  I64["#64 Partition-level caching: R/cache.R is still a s…"]
-  class I34 ready;
-  class I36 ready;
-  class I2 ready;
-  class I33 ready;
-  class I52 ready;
-  class I64 ready;
-  classDef ready fill:#dafbe1,stroke:#2da44e;
-```
+_Nothing blocks anything else, so there is no graph to draw._
 
 - Marker: `none` (no journal entry yet)
-- Commits since: 164
+- Commits since: 165
 - Open issues: 6
 
 </details>

tests/testthat/test-expenditure-concept.R

@@ -315,15 +315,17 @@ test_that("a mis-scoped cog_spending() call never attaches an M/L counterpart to
   # (SB194, cog_pipeline#64), so the recipe stopped being a candidate there.
   # FL state carries a real FY2011 B47 amount, so this exercises the guard
   # against a suggestion that genuinely fires.
+  #
+  # Issue #34: "IG Federal" maps to B-prefixed codes in summary_categories
+  # with category_type = 'revenue'. A spending verb (flow_prefixes E/F/G)
+  # now scopes its candidate query by category_type = 'expenditure', so it
+  # correctly finds NO candidates for this revenue-only category -- the
+  # suggestion machinery cannot fire, and no M/L counterpart is attached.
   r <- suppressMessages(
     cog_spending("120000226351", 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)
+  expect_length(sugg, 0L)
 })
 
 test_that("C1: 'total' on a legacy aggregate-only family reports the IG-only figure honestly, not as Direct + IG", {

tests/testthat/test-recipes.R

@@ -371,34 +371,25 @@ test_that("uscogdata#9: the revenue verb inherits the same trigger", {
   expect_null(sugg[[1]]$ig_recipe_id)
 })
 
-test_that("I1: cog_revenue never fabricates suppressed dollars for an expenditure-only recipe", {
+test_that("I1 + #34: cog_revenue never suggests expenditure-only recipes", {
   # uscogdata#9 review, finding I1: Corrections is an expenditure-only
-  # category (E04/E05). cog_revenue() naturally returns zero rows for it, so
-  # corrections_combined still fires as an empty_year suggestion (its own
-  # generic join finds real E04/E05 data for this government) -- but before
-  # the flow_prefixes fix, .suppressed_components() measured E04/E05 against
-  # cog_revenue()'s OWN view (which can never contain an E-coded row by
-  # construction) and reported the full $3,631,945,000 as "suppressed",
-  # when cog_spending() for the same gov/years/category actually returns
-  # $3,691,029,000 -- nothing was suppressed at all.
+  # category (E04/E05). Before the flow_prefixes fix (#9), .suppressed_components()
+  # measured E04/E05 against cog_revenue()'s OWN view and reported $3.6B as
+  # "suppressed" -- nothing was suppressed at all.
+  #
+  # Issue #34 builds on that: the candidate query now also filters by
+  # category_type ('revenue'), so expenditure-only recipes like corrections_combined
+  # (whose components E04/E05 are classified as 'expenditure' in summary_categories)
+  # are never even considered for a revenue verb. This is stronger than just
+  # suppressing the dollar claim -- it prevents the suggestion from firing at all.
   skip_if_no_corpus()
   r <- suppressMessages(
     cog_revenue("061037123085", years = 2019:2020, category = "Corrections"))
   sugg <- attr(r, "provenance")$suggestions
-  ids <- vapply(sugg, function(s) s$recipe_id, character(1))
-  expect_true("corrections_combined" %in% ids)
+  ids <- vapply(sugg, function(s) s$recipe_id %||% "", character(1))
 
-  hit <- sugg[[which(ids == "corrections_combined")]]
-  expect_equal(hit$suppressed_amount, 0)
-  expect_equal(hit$suppressed_years, integer(0))
-  expect_equal(hit$suppressed_codes, character(0))
-
-  # And cog_spending() for the identical gov/years/category is unaffected --
-  # it actually finds the E04/E05 dollars the buggy measurement claimed were
-  # excluded.
-  sp <- suppressMessages(
-    cog_spending("061037123085", years = 2019:2020, category = "Corrections"))
-  expect_equal(sum(sp$amt_nominal), 3691029000)
+  # corrections_combined should NOT appear -- its components are expenditure-only.
+  expect_false("corrections_combined" %in% ids)
 })
 
 test_that("uscogdata#9: no partial-coverage fire in a modern year", {