Accept state=/type= predicates on the money verbs instead of a 40k-id IN list #58

New Issue

2026-08-09T11:20:44-04:00

jared commented

2026-08-09 11:20:44 -04:00

The flat routes in cog-api resolve a cohort with cog_gov_search() and then pass the
resulting ids to cog_spending()/cog_revenue() as a govid vector. .sql_lit_chr()
renders that into a quoted IN (...) list, and .verb_spendrev() embeds it into 5–8
separate SQL statements per request (scope check, main aggregate, per-capita join,
harmonization block, suggestion queries).

For state=CA&type=city that is ~1,500 quoted ids repeated 5–8 times. For a nationwide
query it approaches 40,000 — a multi-hundred-kilobyte SQL string, re-parsed and re-planned
per statement.

Ask

Accept state and type directly on cog_spending() / cog_revenue() (and ideally
cog_balances()), as an alternative to an explicit govid vector, so the cohort is
expressed as a join or subquery against canonical_fips_xwalk inside the same query
rather than round-tripped through R.

Semantics should match cog_gov_search()'s exactly — note state there is a postal
abbreviation translated internally, while the crosswalk column fips_state holds a FIPS
code. That mismatch is worth preserving in one place rather than reimplementing: filtering
fips_state == "WI" in R silently matches nothing, which is a trap cog-api hit while
optimizing this same path.

Additive (both stay NULL by default, govid keeps working), so it can be adopted behind
a formals probe.

The flat routes in cog-api resolve a cohort with `cog_gov_search()` and then pass the resulting ids to `cog_spending()`/`cog_revenue()` as a `govid` vector. `.sql_lit_chr()` renders that into a quoted `IN (...)` list, and `.verb_spendrev()` embeds it into **5–8 separate SQL statements** per request (scope check, main aggregate, per-capita join, harmonization block, suggestion queries). For `state=CA&type=city` that is ~1,500 quoted ids repeated 5–8 times. For a nationwide query it approaches 40,000 — a multi-hundred-kilobyte SQL string, re-parsed and re-planned per statement. ## Ask Accept `state` and `type` directly on `cog_spending()` / `cog_revenue()` (and ideally `cog_balances()`), as an alternative to an explicit `govid` vector, so the cohort is expressed as a join or subquery against `canonical_fips_xwalk` inside the same query rather than round-tripped through R. Semantics should match `cog_gov_search()`'s exactly — note `state` there is a **postal abbreviation** translated internally, while the crosswalk column `fips_state` holds a FIPS code. That mismatch is worth preserving in one place rather than reimplementing: filtering `fips_state == "WI"` in R silently matches nothing, which is a trap cog-api hit while optimizing this same path. Additive (both stay `NULL` by default, `govid` keeps working), so it can be adopted behind a formals probe.

jared referenced this issue

2026-08-09 13:46:20 -04:00

cog_geographic_rollup(): push aggregation and pagination into SQL #59

jared commented

2026-08-09 13:46:20 -04:00

Measured: this is worth more than the issue claims, and it is the fix for #59 too

Same aggregate over the 20,106-government type = city cohort (FY2022), with the cohort
expressed four ways against the production corpus:

cohort expressed as	time
`IN (20,106 literals)` — what happens today	449 ms
join against a temp cohort table	99 ms
predicate on `canonical_fips_xwalk` — this issue	94 ms
no cohort filter at all (floor)	88 ms

The rendered IN list is 301,591 characters, and .verb_spendrev() embeds it in 5–8
separate statements per call, so the real saving is likely larger than the 355 ms a single
query shows.

4.8x on this query, and within 7% of the no-filter floor. I had estimated 20–40% for
this issue earlier from a state-sized cohort; that was too conservative — the penalty is
superlinear in cohort size, so it only becomes visible at fleet scale.

This also makes #58 the fix for #59: profiling showed /rollups spends ~1% of its time in
the R-side join that #59 targets, and effectively all of it inside cog_spending() over a
wide cohort. See my note there.

Worth keeping in mind for the implementation: cog_gov_search(state = ) takes a postal
abbreviation while canonical_fips_xwalk.fips_state holds a FIPS code, so the
predicate needs the same translation the search verb does internally. cog-api hit exactly
this trap while optimizing the same path — an R-side fips_state == "WI" filter silently
matches nothing.

## Measured: this is worth more than the issue claims, and it is the fix for #59 too Same aggregate over the 20,106-government `type = city` cohort (FY2022), with the cohort expressed four ways against the production corpus: | cohort expressed as | time | |---|---:| | `IN (20,106 literals)` — what happens today | **449 ms** | | join against a temp cohort table | 99 ms | | predicate on `canonical_fips_xwalk` — this issue | **94 ms** | | no cohort filter at all (floor) | 88 ms | The rendered `IN` list is **301,591 characters**, and `.verb_spendrev()` embeds it in 5–8 separate statements per call, so the real saving is likely larger than the 355 ms a single query shows. **4.8x on this query, and within 7% of the no-filter floor.** I had estimated 20–40% for this issue earlier from a state-sized cohort; that was too conservative — the penalty is superlinear in cohort size, so it only becomes visible at fleet scale. This also makes #58 the fix for #59: profiling showed `/rollups` spends ~1% of its time in the R-side join that #59 targets, and effectively all of it inside `cog_spending()` over a wide cohort. See my note there. Worth keeping in mind for the implementation: `cog_gov_search(state = )` takes a **postal abbreviation** while `canonical_fips_xwalk.fips_state` holds a **FIPS code**, so the predicate needs the same translation the search verb does internally. cog-api hit exactly this trap while optimizing the same path — an R-side `fips_state == "WI"` filter silently matches nothing.

jared referenced this issue

2026-08-09 13:48:34 -04:00

cog_geographic_rollup(): push aggregation and pagination into SQL #59

jared commented

2026-08-09 13:49:19 -04:00

Implementation notes (from the profiling above)

Where the IN list is built today

.sql_lit_chr() — R/spending.R:737. Renders 'a','b',...; harmless in itself.
.build_verb_sql() — R/spending.R:743, takes govid and embeds it in the WHERE clause.
.check_govids_in_scope() — R/session.R:71-73, embeds the SAME list again.
R/complete.R:79 — again, inside the completion grid.
The suggestion and suppression queries repeat it too.

That repetition is why the measured saving is likely larger than the 355 ms a single
aggregate showed: one 301,591-character string is parsed and planned 5–8 times per call.

Suggested shape

Add optional state / type arguments to cog_spending() / cog_revenue() (and ideally
cog_balances()), mutually compatible with govid being NULL. When set, express the
cohort as a subquery instead of a literal list:

canonical_govid IN (
  SELECT canonical_govid FROM canonical_fips_xwalk
  WHERE fips_state = '55' AND govs_type = 2
)

Measured: 94 ms versus 449 ms for the literal list, within 7% of the no-filter floor.

Reuse, do not reimplement

.coerce_state_to_fips() and .coerce_type() already exist (R/search.R:117, :121) and
are exactly what cog_gov_search() uses. The API takes a postal abbreviation ("WI")
while canonical_fips_xwalk.fips_state holds a FIPS code ("55") — a predicate written
against the raw parameter silently matches nothing and returns an empty result that looks
like "this government reported nothing." cog-api hit this exact trap optimizing the same
path. Reusing the existing coercers keeps one definition of the translation.

Constraints

Additive only. uscogdata is public at 0.3.0. state/type default to NULL; every
existing govid-based call must be byte-identical in behaviour.
Decide and document what govid and state/type together means — either an error
or an intersection, but not a silent precedence rule.
Consumers feature-detect via formals() (see verb_supports_pagination() in cog-api,
api/R/session.R), so nothing needs to deploy in lockstep.

Verifying the win

Civilytics/cog-api has scripts/bench.sh; the fleet routes (/spending?state=..&type=..,
/rollups) are where this should show. Point it at a local server reading the production
corpus. Also worth re-running the raw comparison in the profiling comment above, since it
isolates the predicate from everything else the verb does.

## Implementation notes (from the profiling above) ### Where the IN list is built today - `.sql_lit_chr()` — `R/spending.R:737`. Renders `'a','b',...`; harmless in itself. - `.build_verb_sql()` — `R/spending.R:743`, takes `govid` and embeds it in the WHERE clause. - `.check_govids_in_scope()` — `R/session.R:71-73`, embeds the SAME list again. - `R/complete.R:79` — again, inside the completion grid. - The suggestion and suppression queries repeat it too. That repetition is why the measured saving is likely larger than the 355 ms a single aggregate showed: one 301,591-character string is parsed and planned 5–8 times per call. ### Suggested shape Add optional `state` / `type` arguments to `cog_spending()` / `cog_revenue()` (and ideally `cog_balances()`), mutually compatible with `govid` being `NULL`. When set, express the cohort as a subquery instead of a literal list: ```sql canonical_govid IN ( SELECT canonical_govid FROM canonical_fips_xwalk WHERE fips_state = '55' AND govs_type = 2 ) ``` Measured: **94 ms** versus **449 ms** for the literal list, within 7% of the no-filter floor. ### Reuse, do not reimplement `.coerce_state_to_fips()` and `.coerce_type()` already exist (`R/search.R:117`, `:121`) and are exactly what `cog_gov_search()` uses. **The API takes a postal abbreviation (`"WI"`) while `canonical_fips_xwalk.fips_state` holds a FIPS code (`"55"`)** — a predicate written against the raw parameter silently matches nothing and returns an empty result that looks like "this government reported nothing." cog-api hit this exact trap optimizing the same path. Reusing the existing coercers keeps one definition of the translation. ### Constraints - **Additive only.** uscogdata is public at 0.3.0. `state`/`type` default to `NULL`; every existing `govid`-based call must be byte-identical in behaviour. - Decide and document what `govid` **and** `state`/`type` together means — either an error or an intersection, but not a silent precedence rule. - Consumers feature-detect via `formals()` (see `verb_supports_pagination()` in cog-api, `api/R/session.R`), so nothing needs to deploy in lockstep. ### Verifying the win `Civilytics/cog-api` has `scripts/bench.sh`; the fleet routes (`/spending?state=..&type=..`, `/rollups`) are where this should show. Point it at a local server reading the production corpus. Also worth re-running the raw comparison in the profiling comment above, since it isolates the predicate from everything else the verb does.

jared referenced a pull request that will close this issue

2026-08-09 14:16:05 -04:00

feat: cohort predicates instead of a 40k-id IN list (#58) #61

jared commented

2026-08-09 14:16:36 -04:00

Implemented in #61 — measured, and it reaches the floor

Same four-way comparison re-run against the production corpus with the shipped
implementation as one of the arms. DUCKDB_THREADS=2, median of 5 reps after a
warm-up, FY2022 aggregate over the 20,106-government type = "city" cohort.
Your original figures in brackets:

cohort expressed as	time
`IN (20,106 literals)` — 0.3.0	432 ms [449]
join against a temp cohort table	132 ms [99]
predicate on `canonical_fips_xwalk` — this issue	102 ms [94]
no cohort filter at all (the floor)	105 ms [88]

The rendered IN list measured 301,589 characters, matching your 301,591.

The predicate arm is at the floor — 102 ms against 105 ms for querying with
no cohort restriction whatsoever. The two are inside run-to-run noise of each
other, so the cohort filter is now effectively free rather than merely cheaper.

End to end through the verb

Your prediction that the real saving exceeds what a single query shows, because
.verb_spendrev() embeds the list 5–8 times, holds:

`cog_spending(category = "Police")`	time
`govid = <20,106 ids>`	1080 ms
`type = "city"`	271 ms

3.99x, an 809 ms saving against the 330 ms the single aggregate accounts
for. The rest is exactly the repetition you identified.

Equivalence

Verified on the production corpus rather than only the fixture: both spellings
of the cohort return 19,236 identical rows with identical amt_nominal
sums. The suite covers the same property for all three verbs, and under
per_capita / adjust_to_year / pagination.

The two decisions the issue asked to be made explicitly

govid + state/type intersect. An error would have been the
conservative call, but it could never be relaxed later without breaking
callers, whereas the intersection can be tightened by deprecation. A disjoint
intersection returns zero rows, and there is a test asserting that rather
than one of the two silently winning.
Provenance. A predicate cohort leaves scope$govids_found /
govids_missing empty and adds scope$cohort with state, type and
n_governments. Resolving the ids purely to report them would have put
20,000 govids into every fleet-scale response body — cog-api passes
provenance through verbatim — which is the cost this issue exists to remove.
govid-named calls report exactly as before.

Two things worth knowing

#59 is not closed by this. The note above says #58 is also the fix for
/rollups, and the query-level premise is right, but
cog_geographic_rollup() takes govids as a named list of per-layer id
vectors and uses it to build the govid→layer map in its own output — so it
structurally needs the ids today. Its layers are already exactly
state/county/city, the same vocabulary as type, so a predicate-named
layer is very doable; it just changes that verb's public argument shape and
needs its own issue. I did not fold it in here.

A latent bug surfaced and is fixed in the PR. .state_abbrev_to_fips is a
named character vector, so [[ on an absent name throws base R's "subscript
out of bounds" — which made the curated "Unknown state abbreviation" abort
below it unreachable dead code. cog_gov_search(state = "ZZ") has always given
the cryptic error. Fixed because the new state= argument routes into the same
coercer.

## Implemented in #61 — measured, and it reaches the floor Same four-way comparison re-run against the production corpus with the shipped implementation as one of the arms. `DUCKDB_THREADS=2`, median of 5 reps after a warm-up, FY2022 aggregate over the 20,106-government `type = "city"` cohort. Your original figures in brackets: | cohort expressed as | time | |---|---:| | `IN (20,106 literals)` — 0.3.0 | 432 ms [449] | | join against a temp cohort table | 132 ms [99] | | predicate on `canonical_fips_xwalk` — this issue | **102 ms** [94] | | no cohort filter at all (the floor) | 105 ms [88] | The rendered `IN` list measured 301,589 characters, matching your 301,591. **The predicate arm is at the floor** — 102 ms against 105 ms for querying with no cohort restriction whatsoever. The two are inside run-to-run noise of each other, so the cohort filter is now effectively free rather than merely cheaper. ### End to end through the verb Your prediction that the real saving exceeds what a single query shows, because `.verb_spendrev()` embeds the list 5–8 times, holds: | `cog_spending(category = "Police")` | time | |---|---:| | `govid = <20,106 ids>` | 1080 ms | | `type = "city"` | **271 ms** | **3.99x**, an 809 ms saving against the 330 ms the single aggregate accounts for. The rest is exactly the repetition you identified. ### Equivalence Verified on the production corpus rather than only the fixture: both spellings of the cohort return **19,236 identical rows** with identical `amt_nominal` sums. The suite covers the same property for all three verbs, and under `per_capita` / `adjust_to_year` / pagination. ### The two decisions the issue asked to be made explicitly - **`govid` + `state`/`type` intersect.** An error would have been the conservative call, but it could never be relaxed later without breaking callers, whereas the intersection can be tightened by deprecation. A disjoint intersection returns zero rows, and there is a test asserting that rather than one of the two silently winning. - **Provenance.** A predicate cohort leaves `scope$govids_found` / `govids_missing` empty and adds `scope$cohort` with `state`, `type` and `n_governments`. Resolving the ids purely to report them would have put 20,000 govids into every fleet-scale response body — cog-api passes provenance through verbatim — which is the cost this issue exists to remove. `govid`-named calls report exactly as before. ### Two things worth knowing **#59 is not closed by this.** The note above says #58 is also the fix for `/rollups`, and the query-level premise is right, but `cog_geographic_rollup()` takes `govids` as a named list of per-layer id vectors and uses it to build the govid→layer map in its own output — so it structurally needs the ids today. Its layers are already exactly `state`/`county`/`city`, the same vocabulary as `type`, so a predicate-named layer is very doable; it just changes that verb's public argument shape and needs its own issue. I did not fold it in here. **A latent bug surfaced and is fixed in the PR.** `.state_abbrev_to_fips` is a named *character vector*, so `[[` on an absent name throws base R's "subscript out of bounds" — which made the curated `"Unknown state abbreviation"` abort below it unreachable dead code. `cog_gov_search(state = "ZZ")` has always given the cryptic error. Fixed because the new `state=` argument routes into the same coercer.

jared closed this issue

2026-08-09 14:31:54 -04:00

jared referenced this issue from a commit

2026-08-09 14:31:55 -04:00

Merge pull request 'feat: cohort predicates instead of a 40k-id IN list (#58)' (#61) from feat/cohort-predicates-58 into main

jared referenced this issue

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

Release 7/11: tag v0.4.0 and register with r-universe #47

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