cog_revenue() offers expenditure recipes as suggestions: scope the candidate query by category_type #34

New Issue

2026-08-05T08:23:05-04:00

jared commented

2026-08-05 08:23:05 -04:00

Deferred half of finding I1 from the uscogdata#9 final review (PR #32). Owner ruled: ship the minimal flow-prefix fix in #9, track the principled fix here.

What #9 fixed

.suppressed_components() measured every component of a candidate recipe against the calling verb's own view. Candidates were filtered only for M/L recipes, never for flow family, so a component belonging to the other verb was always absent from this verb's view and got reported as suppressed. Measured before the fix:

cog_revenue("061037123085", years = 2019:2020, category = "Corrections")
* corrections_combined: $3,631,945,000 excluded from FY2019, FY2020 (E04, E05)

Those dollars are fully reachable — cog_spending() for the same gov/years/category returns $3,691,029,000. Nothing was suppressed. #9 fixed the fabricated dollar figure by filtering the measurement on LEFT(r.component_code, 1) IN (<verb flow_prefixes>).

What is still wrong

The suggestion itself still fires. cog_revenue(category = "Corrections") still surfaces the three corrections recipes as trigger = "empty_year", because the candidate query at R/suggestions.R selects recipes by summary_categories membership without regard to category_type. A revenue verb should never have offered an expenditure category in the first place.

This is pre-existing behavior, older than #9.

The fix

Scope the candidate query's summary_categories lookup by the verb's own category_type.

Why it was deferred

It changes behavior beyond the #9 bug, and it breaks assertions in tests/testthat/test-expenditure-concept.R that deliberately use a mis-scoped cog_spending(category = "IG Federal") to exercise the M/L counterpart logic (ig_federal_b47_wide firing for real in the fixture is what makes those tests non-theoretical). Those tests need rethinking as part of this change, not as a side effect of a bug fix.

Also worth folding in: suppressed_codes under spending currently excludes J67/J68 as a side effect of the flow-prefix filter. Harmless today (they are in the view in every year they exist), but it is a wart the category_type approach would not have.

Deferred half of finding I1 from the uscogdata#9 final review (PR #32). Owner ruled: ship the minimal flow-prefix fix in #9, track the principled fix here. ## What #9 fixed `.suppressed_components()` measured every component of a candidate recipe against the calling verb's own view. Candidates were filtered only for M/L recipes, never for flow family, so a component belonging to the *other* verb was always absent from this verb's view and got reported as suppressed. Measured before the fix: ``` cog_revenue("061037123085", years = 2019:2020, category = "Corrections") * corrections_combined: $3,631,945,000 excluded from FY2019, FY2020 (E04, E05) ``` Those dollars are fully reachable — `cog_spending()` for the same gov/years/category returns $3,691,029,000. Nothing was suppressed. #9 fixed the fabricated **dollar figure** by filtering the measurement on `LEFT(r.component_code, 1) IN (<verb flow_prefixes>)`. ## What is still wrong The **suggestion itself** still fires. `cog_revenue(category = "Corrections")` still surfaces the three corrections recipes as `trigger = "empty_year"`, because the candidate query at `R/suggestions.R` selects recipes by `summary_categories` membership without regard to `category_type`. A revenue verb should never have offered an expenditure category in the first place. This is pre-existing behavior, older than #9. ## The fix Scope the candidate query's `summary_categories` lookup by the verb's own `category_type`. ## Why it was deferred It changes behavior beyond the #9 bug, and it breaks assertions in `tests/testthat/test-expenditure-concept.R` that **deliberately** use a mis-scoped `cog_spending(category = "IG Federal")` to exercise the M/L counterpart logic (`ig_federal_b47_wide` firing for real in the fixture is what makes those tests non-theoretical). Those tests need rethinking as part of this change, not as a side effect of a bug fix. Also worth folding in: `suppressed_codes` under spending currently excludes `J67`/`J68` as a side effect of the flow-prefix filter. Harmless today (they are in the view in every year they exist), but it is a wart the category_type approach would not have.

jared referenced this issue

2026-08-05 08:32:11 -04:00

Public Welfare understated 32-38% in legacy years with no coverage-gap signpost #9

jared referenced this issue

2026-08-09 09:18:47 -04:00

Performance pass over the reader before final release #56

jared commented

2026-08-10 19:10:27 -04:00

Re-scoping, not closing: the performance framing is dead, the defect is not

#56 step 3 asked that this be absorbed if profiling confirmed it, or closed as speculative
if not. Neither is quite right, because #56 mis-files what this issue is.

The performance framing is retired. #56 lists this under "performance findings filed
before anyone was measuring." The measurement is now in: the whole R-side verb pipeline
sums to tens of milliseconds against seconds of remote HTTP, so a too-broad candidate
query is not on anyone's critical path. Do not do this for speed. (#35 was the genuinely
perf-shaped sibling and has been closed on the same evidence.)

The defect is untouched by any of that. The body of this issue is a correctness
problem:

cog_revenue("061037123085", years = 2019:2020, category = "Corrections")
#> suggests three CORRECTIONS (expenditure) recipes, trigger = "empty_year"

A revenue verb should never offer an expenditure category's recipes. #9 fixed the
fabricated dollar figure by filtering the measurement on the verb's flow_prefixes;
the suggestion itself still fires, because the candidate query selects recipes by
summary_categories membership without regard to category_type. That is pre-existing
behaviour, older than #9, and it is wrong regardless of how fast it is.

It is also a trust problem more than a latency one, which is the real reason to keep
it open: a suggestion block that names categories from the wrong flow teaches a user that
provenance output is noise to skim past. That costs more than 12 ms.

Retitling accordingly

Scope the suggestion candidate query by the verb's category_type → the fix is unchanged,
but the justification is correctness, so it is not re-triaged as a performance nice-to-have
and dropped next time someone sorts by impact.

The deferral reason recorded here still stands and is the actual work: it breaks
assertions in tests/testthat/test-expenditure-concept.R that deliberately use a
mis-scoped cog_spending(category = "IG Federal") to exercise the M/L counterpart logic.
Those tests need rethinking as part of this change, which is why it was not folded into a
bug fix.

## Re-scoping, not closing: the performance framing is dead, the defect is not #56 step 3 asked that this be absorbed if profiling confirmed it, or closed as speculative if not. Neither is quite right, because #56 mis-files what this issue is. **The performance framing is retired.** #56 lists this under "performance findings filed before anyone was measuring." The measurement is now in: the whole R-side verb pipeline sums to tens of milliseconds against seconds of remote HTTP, so a too-broad candidate query is not on anyone's critical path. Do not do this for speed. (#35 was the genuinely perf-shaped sibling and has been closed on the same evidence.) **The defect is untouched by any of that.** The body of this issue is a correctness problem: ```r cog_revenue("061037123085", years = 2019:2020, category = "Corrections") #> suggests three CORRECTIONS (expenditure) recipes, trigger = "empty_year" ``` A revenue verb should never offer an expenditure category's recipes. #9 fixed the fabricated **dollar figure** by filtering the measurement on the verb's `flow_prefixes`; the **suggestion itself** still fires, because the candidate query selects recipes by `summary_categories` membership without regard to `category_type`. That is pre-existing behaviour, older than #9, and it is wrong regardless of how fast it is. It is also a **trust** problem more than a latency one, which is the real reason to keep it open: a suggestion block that names categories from the wrong flow teaches a user that provenance output is noise to skim past. That costs more than 12 ms. ## Retitling accordingly `Scope the suggestion candidate query by the verb's category_type` → the fix is unchanged, but the justification is correctness, so it is not re-triaged as a performance nice-to-have and dropped next time someone sorts by impact. The deferral reason recorded here still stands and is the actual work: it breaks assertions in `tests/testthat/test-expenditure-concept.R` that **deliberately** use a mis-scoped `cog_spending(category = "IG Federal")` to exercise the M/L counterpart logic. Those tests need rethinking as part of this change, which is why it was not folded into a bug fix.

jared changed title from ~~Scope the suggestion candidate query by the verb's category_type~~ to cog_revenue() offers expenditure recipes as suggestions: scope the candidate query by category_type

2026-08-10 19:10:32 -04:00

jared referenced this issue

2026-08-10 19:17:11 -04:00

Performance pass over the reader before final release #56

jared referenced this issue

2026-08-10 19:39:12 -04:00

Performance pass over the reader before final release #56

jared added the