cog-api's paginate() sliced an already-fully-materialized result: every
page of a deep sweep re-ran the whole cog_spending()/cog_revenue() query
and re-listified every row, just to keep up to 1000 and discard the
rest. A 193,105-row/194-page fleet-wide sweep (cog_explorer's Southern
guide, corpus summary build) repeated that full cost 194 times and
wedged the production server for hours on 2026-08-06 -- single request,
CPU-bound, single-threaded plumber process, no other request could get
through, not even /health.
cog_spending()/cog_revenue() gain optional limit/offset, pushed into
.build_verb_sql() as SQL LIMIT/OFFSET behind the existing (already
deterministic) ORDER BY. The full unpaginated row count rides along via
COUNT(*) OVER() in the same scan -- exposed as a total_rows attribute --
so a caller walking pages never needs a second round trip to ask how
many there are. A page now costs O(limit), not O(full result).
Mutually exclusive with complete = TRUE (which fills a grid over the
FULL requested (year, category) space -- pagination over a partial slice
of already-grouped rows has no defined meaning for the cells it would
fill) and with recipe (whose result comes from a separate, not-yet-wired
query path). Both abort with a clear classed condition rather than
silently ignoring the parameter.
limit/offset default to NULL; every existing call site is unaffected.
.build_suggestions()'s recipe-candidate sub-select was keyed on
`WHERE category IN (<category>)`. The reserved pseudo-category
"All Categories" is never itself a row in summary_categories.category, so
in all-categories mode `candidates` always came back empty and coverage
signposting (uscogdata#9) was structurally impossible for the one mode
whose entire premise is "you cannot sum the wrong scope" -- measured on
Los Angeles County FY2011: category = "Public Welfare" reports 2
suggestions (incl. $271,589,000 excluded E68), category = "All Categories"
reported 0, silently losing that same signal.
Apply the branch's own design principle: the concept boundary is subtype,
not category. .build_suggestions() now accepts all_categories/subtype_col/
subtype_scope (all optional, default off, so no other caller's behaviour
changes) and, when all-categories mode is active, scopes the candidate
sub-select by `<subtype_col> IN (<subtype_scope>)` instead -- symmetric
with .build_verb_sql()'s own WHERE predicate. The M/L recipe exclusion and
the is.null(category) early return are unchanged.
After the fix, LA County FY2011 "All Categories" reports 5 suggestions,
including welfare_cash_e68_wide for the exact $271,589,000 gap.
Adds two covering tests to test-all-categories.R using the bundled fixture
(AL state gov, FY2011, "Corrections"): one end-to-end (per-category and
all-categories both signpost the same recipe) and one direct on
.build_suggestions() proving the subtype-vs-category branch is what
changes the query. Updates the 0.2.0 NEWS entry.
- .detect_direct_suppressed() keys on (year, canonical_govid, category);
all-categories mode collapses category to one literal value, so the key
collides and the detector silently reports FALSE instead of "unknown".
Report NA there instead, and stop isTRUE() in .build_provenance() from
collapsing that NA back to FALSE. Schema widened to allow null.
- Refuse complete = TRUE + category = "All Categories": the completion grid
has no per-category cells left to fill once categories are collapsed,
so the prior silent 0-rows-filled result was never actually checked.
- cog_balances(category = "All Categories") returned zero rows with no
error. .validate_verb_inputs() gains allow_all_categories (default
FALSE); .verb_spendrev() passes TRUE, cog_balances() does not, so the
three verbs share one place to reject it instead of drifting again.
- Fix the false `subtype = "operations"` argument claim (no such argument
exists) in NEWS.md and an internal spending.R comment.
Adds three covering tests to test-all-categories.R for the three
behaviour changes above.
Bumps the minor version because 'All Categories' adds public surface
without breaking any existing call.
The bump is load-bearing, not cosmetic: cog-api installs this package
with install_local(), which no-ops when the version already matches.
Without it, Phase 1 would silently test against the 0.1.0 reader and
pass while proving nothing.
Closes uscogdata#36. It counts governments with rows for the requested
category, so a surveyed government that genuinely spends nothing there
is indistinguishable from one never surveyed. In FY2022, a complete
census year, Georgia reports 393 of 567 cities for Police -- the gap is
cities that contract to the sheriff.
Documents the comparison that IS valid: same category, census year vs
sample year.
Geographic totals are the expensive case cog-api#37 was filed about --
without this a caller issues one rollup per category and sums them.
The pass-through was expected to work by construction; this asserts it
rather than assuming it, including under per_capita and inflation
adjustment.
A reserved value nobody can discover is a trap, and this is the view
the API's /categories endpoint is built from. Emitted for the two flow
vocabularies only -- cog_balances() returns a stock and has no concept
to sum within.
Also fix test-categories.R to exclude pseudo-category rows from
crosswalk-specific assertions (one row per (category, subtype) pair,
non-empty item_codes, valid subtypes).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The @param category text on cog_spending()/cog_revenue() told users to
"Combine with subtype = ..." but neither verb has a subtype argument.
Replace with accurate guidance: filter the returned frame's
spend_subtype/revenue_subtype column.
Returns one summed row per (year, govid, subtype) across every
category in the requested concept's subtype scope, so a caller never
sums categories client-side and cannot sum the wrong scope.
Combining it with other category names is an error rather than a
silent partial sum.
The concept boundary in this package is subtype, not category, so a
total is the existing query with the category dimension collapsed and
no category predicate applied. subtype is deliberately kept in the
grouping: subtype=operations plus all-categories is 'operating
expenditure', which is the measure a fiscal comparison wants.
Named 'All Categories' rather than 'Total' because category='Total'
would sit one argument from expenditure_concept='total' and mean
something different.
R/suggestions.R crossed the project's 400-line limit (424 lines).
Pure move of .suppressed_components() and its roxygen block per the
plan's Task 5 Step 2 remedy; no logic, SQL, or wording changed.
Scoped re-review measured .needs_suppression_query() against the fixture
and found it doesn't pay for itself: it skips the round trip on ~3% of
healthy candidate-bearing calls, ~0% of the multi-govid batch shape
(cog_geographic_rollup()/cog_peer_compare()) it was meant to help, and
reaching the gate costs an unconditional metadata query that on its own
roughly cancels the expected saving -- net slower on the fixture. The
gate was also correct (0 unsound skips) but left an untested exactness
invariant (result$codes_included and the anti-join sharing the harmonized
item_code space) whose silent violation would kill signposting, which is
the exact failure class uscogdata#9 exists to prevent.
Owner's call: revert it and keep the code simple. A batch-aware
optimization, if warranted, is a separate issue.
Removes .needs_suppression_query() entirely (function, roxygen, call
site, comp_rows/flow_components), restoring .build_suggestions() to call
.suppressed_components() directly -- unchanged from f77adb6 except that
it still threads flow_prefixes through (I1, kept). Also removes the two
tests that existed solely to exercise the gate (the five-branch synthetic
test and the local_mocked_bindings call-counter test); no test asserting
real signposting behavior was touched.
I1 (flow_prefixes filter), I2 (schema wording + ig_recipe_id required),
I3(a) (restated NOT EXISTS literals for partition pruning), and M4
(reworded scope claims) are all untouched by this revert.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Final whole-branch review fix wave for the partial-coverage signposting
feature:
- I1: .suppressed_components() now filters measured recipe components to
the calling verb's own flow_prefixes. Without this, a candidate recipe
from the OTHER flow family was always absent from the verb's own view by
construction and so was always reported as "suppressed" -- fabricating a
dollar claim across flow families (cog_revenue(category = "Corrections")
claimed $3.63B excluded that cog_spending() actually reports in full).
- I2: reworded provenance-v1.json's trigger/suppressed_amount descriptions
to describe what the code actually measures (the verb's underlying long
view, not "the result"), and to note suppressed_amount can be negative.
Added ig_recipe_id to the suggestions items' required list, matching the
key's always-set/nullable runtime behavior.
- I3(a): restated the year/govid literals inside .suppressed_components()'s
NOT EXISTS subquery so DuckDB can partition-prune that side too (verified
via EXPLAIN: Scanning Files 1/4 instead of an unfiltered full scan;
all.equal(old, new) results confirmed unchanged).
- I3(b): added .needs_suppression_query(), a free, exact pre-check reusing
the verb's own already-computed result$codes_included to skip the anti-
join round trip on the common fully-covered path, without weakening the
"suppression can fire with zero gap years" guarantee.
- M4: corrected the overbroad "confines every fire to 2011" scope claim in
R/suggestions.R and NEWS.md -- the suppressed-dollar measurement is now
flow-scoped (post-I1), but the empty_year trigger itself is not, and can
still fire in modern years for a mis-scoped cross-flow-family category.
Added a regression test for I1 plus direct unit-test coverage for the new
flow-family filter and the I3(b) pre-check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
devtools::document() should not change DESCRIPTION when only JSON/Markdown/test files are edited. Restore the original RoxygenNote: 7.3.3 and remove the Config/roxygen2/version auto-generated line that resulted from running devtools::document() locally.
.inform_suggestions() and cog_explain() now render suppressed_amount /
suppressed_years / suppressed_codes as a continuation line on each
suggestion bullet whenever suppressed_amount > 0 (an empty_year fire can
carry them too, so this keys off the amount, not trigger). Also renames
the cli header from "Coverage gap detected" to "Incomplete coverage" --
a partial-coverage fire is not a gap, the year has rows, they're just
short.
The merged schema-v7 fix (b59b79b) widened .validate_schema()'s allow-list but
left this test asserting that 7 is REJECTED, so main went red. CI had been
hanging on the apt step before ever reaching the suite, which is why the
failure only surfaced once the HTTPS fix let the job get that far.
Flips 7L from expect_error to expect_silent, and ADDS an 8L rejection case.
That second part is the point: simply deleting the 7L expectation would have
left the test unable to prove any upper bound is enforced at all, so a future
v8 corpus with a genuinely breaking change would pass validation silently. The
test should assert the boundary moved, not that it disappeared.
Suite: 796 PASS, 0 FAIL, 0 WARN, 0 SKIP.
The "Install system libraries" step was stalling indefinitely. It was not
deadlocked on a config prompt and not slow-but-progressing: measured inside the
live runner container, /var/cache/apt/archives stayed at 0 .deb files after 3+
minutes, with apt's http workers parked in S state waiting on the network.
Root cause is the http:// mirror path being pathologically slow from this
runner, not broken. Measured 2026-08-04 from inside the CI container, same
index file, back to back:
http://archive.ubuntu.com/ubuntu/dists/noble/Release 20.1s
https://archive.ubuntu.com/ubuntu/dists/noble/Release 3.1s
apt fetches many indexes serially, so ~20s apiece compounds into what looks
like a hang. Rewriting the deb822 sources to https makes the step complete.
Verified before committing, in the running CI container (rocker/r-ver:4.4):
- ca-certificates present and apt 2.8.3 ships the https method built in, so
nothing has to be installed over http first to bootstrap TLS
- the sed rewrites both URIs (archive + security); the only remaining http://
is an inert comment line
- '#' is used as the sed delimiter deliberately: '|' collides with the
alternation and fails with "unknown option to `s'"
- the regex survives YAML block-scalar parsing with backslashes intact
`|| true` guards each sed because the step runs under `sh -e`, so a
missing-sources-file on some other base image must not kill the job.
The balance work added category_type = "balance" rows to the corpus and
cog_balances() to read them, but left cog_categories() -- the discovery
surface -- unable to describe them:
- subtype COALESCEd only spend_subtype and revenue_subtype, so every balance
row came back with subtype = NA
- type rejected "balance", so there was no way to ask for the holdings
taxonomy at all
Both matter downstream: cog-api derives its subtype vocabulary from
cog_categories(), so an NA subtype becomes an unusable API parameter. Found
while implementing cog-api#26.
Note cog_balances() itself still takes no subtype argument -- for holdings
category is a strict coarsening of balance_subtype -- but the value belongs
in the discovery surface regardless.
Tests read the expected subtype set independently from the crosswalk parquet
rather than from the function under test.
Both were settled during implementation and are easy to get wrong from
outside the package:
- coverage_window is corpus-scoped, not result-scoped. It reports the observed
year extent of every balance subtype, not only those a query returned. The
sibling field `truncated` is the result-scoped one.
- balance_caveats is present only on cog_balances() results; an API layer that
assumes it is universal will read NULL from the money verbs.
F-7: inst/schemas/provenance-v1.json described coverage_window as mapping
each *observed* balance_subtype, but the query at R/balance_caveats.R has
no predicate tied to the query's codes and always returns every subtype in
the mounted corpus. Took option (b) of the two the review offered -- change
the doc, not the code. Reporting all windows is the better product
behaviour (it answers 'is there a family I missed?'), it is what cog-api#26
already forwards verbatim, and option (a) would make the block empty for a
0-row result. Reworded to say the windows are corpus-wide and that
'truncated' is the query-scoped field. Pinned by a new test either way.
F-10: the 'Current State' block was self-contradictory after a partial
update -- headed 2026-04-27, claiming branch main @ d65e9fe, with a
2026-08-03 test count measured on feat/cog-balances-25 underneath it, and
listing README.md / _pkgdown.yml as outstanding when both exist and
_pkgdown.yml was edited by this branch. All numbers below re-measured on
the final tree after every other fix in this wave, not before:
788 tests (testthat::test_local()), 14 exports (NAMESPACE), 14 man/*.Rd,
2 vignettes, no docs/ (pkgdown::build_site() genuinely still outstanding,
as is the .Rbuildignore fixture entry -- both kept in the list).
The related deferred README.md item is closed with no change, per the
review's ruling: README.md enumerates no verbs at all, so naming
cog_balances would make it the only non-cog_spending verb mentioned.
Findings F-1..F-8. Every assertion below was verified to FAIL before its
fix (or under mutation, where the behaviour already worked) and pass after.
F-3: 'an unknown recipe id is rejected' used a bare expect_error(). Deleting
.validate_recipe_id() leaves .recipe_components() returning 0 rows and
comps$label[[1]] throwing 'subscript out of bounds' -- still an error, so
the test passed on the regression while the user lost the curated message.
Now asserts class = 'uscogdata_unknown_recipe'. Mutation-checked.
F-4: no test ever set per_capita and adjust_to_year together, so the
load-bearing ordering comment at R/balances.R was unverified. Reversing
those two calls silently drops amt_per_capita_real (.attach_real_dollars()
no-ops when amt_per_capita_nominal does not exist yet). New test asserts
presence AND that the per-capita column is deflated by the same factor as
the level column; mutation-checked by reversing the order (2 failures).
F-5: the spec's 'Gating' requirement had no test -- nothing ever called
cog_balances() on a corpus without balance_subtype. Extended the existing
with_corpus_missing_balance_subtype() block to assert class =
'uscogdata_no_balance_support'; mutation-checked by dropping the guard.
F-1/F-2: added mutual-exclusivity and four-argument validation tests, each
pinned to the message or class (all four inputs already produced *some*
error or *some* quiet wrong answer, so bare expect_error() was useless
here). Plus an ordering guard: a data-frame govid must still work, which
is what fails if validation is put before .coerce_govid_input().
F-6: asserts on the RENDERED cog_explain() text (both streams -- cli
routes through conditions that land on stderr), with a negative case
proving money-verb output is unaffected and that the capture is not vacuous.
F-7: pins that coverage_window is corpus-scoped while truncated is
query-scoped; mutation-checked by scoping the windows to observed subtypes.
F-8: pins the memo slot is populated on first call and cleared by
cog_close().
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.
Re-measured CLAUDE.md's test count on the final tree (764, not 763 --
the earlier number predated the balance_caveats schema test). Removed
notes from cog_balances()'s @return block: it was copied from
cog_spending()'s @return style without checking cog_balances() never
calls .verb_spendrev(), the only place that sets notes. Verified the
remaining documented columns against colnames() observed across every
argument combination (bare, per_capita, adjust_to_year, both, recipe,
category filter).
Adds the NEWS entry, a Financial data pkgdown reference section (none
existed for cog_spending/cog_revenue), and corrects CLAUDE.md's SQL-layer
claim, view count, test count and fixture-year description against
measured values. Also documents balance_caveats in
inst/schemas/provenance-v1.json (test-first: added a schema-documentation
test to test-balances.R, confirmed it failed, then fixed the schema) and
fleshes out cog_balances()'s @return roxygen to enumerate its conditional
columns, regenerating man/cog_balances.Rd.
Task 4's implementer found that SB195 does not surface for a
recipe query spanning only 2011-2012. .build_series_break_refs() matches
break_year BETWEEN min(years) AND max(years), and SB195's break_year is 2002.
That is correct behaviour rather than a gap: a series lying entirely after the
book -> market change sits on one consistent basis, so disclosing a break it
never crosses would be noise. .build_corpus_break_refs() applies the same rule
deliberately.
The spec's caveat table overclaimed by omitting the span condition. Corrected.
Mirrors R/spending.R:465-466 -- .check_govids_in_scope()'s return was
previously captured only for its message side effect. Also drops a
redundant duplicate assertion in the flow-code guard test.
The bundled fixture has no balance item_code with is_aggregate = TRUE, so
asserting COUNT(*) FROM balance_long WHERE is_aggregate = 0 passed whether
or not the view's AND NOT is_aggregate predicate existed. Follows the
synthetic hive-partitioned parquet pattern already used for the 22-/23-
and 24-/25- view predicates in test-views.R: reads the real
inst/sql/26-balance_long.sql text off disk and executes it against a
synthetic corpus containing both an aggregate and non-aggregate row under
a real balance item_code (W01).
Standardises on the identifier other agents use for state governments, which
is stable across corpus vintages and is the same id used against the live API.
Verified in the bundled fixture, and it is strictly better coverage than the
previous pick: Wisconsin reaches four of the five balance subtypes (adds
workers_comp_trust via Y21) and carries BOTH wide->modern recipe bridges
(X40->Z77 and X41->Z78), so a second recipe test is added. Y61
(other_insurance_trust) is absent for Wisconsin; no test depends on it.
Also notes not to assert on gov_name -- the fixture carries both "WISCONSIN"
and "WISCONSIN STATE GOVT" and the verb COALESCEs them.
Six TDD tasks: the two views + registration gate, the core verb, per_capita
and adjust_to_year, recipe=, balance_caveats provenance, docs.
Every internal the plan calls was verified to exist with the signature used
(.build_verb_sql, .shape_recipe_result, .attach_per_capita, .run_recipe,
.require_schema_v5, ...), so the tasks reuse the shared machinery rather than
reimplementing it. The verb deliberately does not route through
.verb_spendrev(), whose concept scoping, IG leg and complete= grid are all
flow-specific.
Test government is ALABAMA STATE GOVT (010000226085), which covers every case
in the bundled fixture: W01/W31/W61 in 2012/2019/2020, X21+Z77 in 2012,
Y07/Y08 throughout, and X40 in 2011 -- so the wide-era recipe bridge is
testable offline.
Corrects this spec. The earlier draft deferred recipe= and proposed adding
summary_categories rows for X40/X41. Both were wrong, and the pipeline state
they were meant to fix is correct and documented.
cog_pipeline/docs/phase_r_harmonization_review.md records the decisions:
- Sec 0.2: the wide era exposes these split families ONLY as aggregates, so
the recipe join deliberately does NOT filter is_aggregate. Safe by
construction -- wide rows are aggregate-only, modern rows leaf-only, every
component year-scoped.
- Sec 1: the planned X40->Z77 harmonization MAP rows were dropped on purpose;
continuity ships as recipes instead. That is why harmonization_map carries
no balance-code rows.
The reader already implements this (R/recipes.R, R/spending.R). Verified
against the live corpus rather than trusting the comment: corrections_combined
FY2007, whose wide leg E05 is likewise aggregate-only, returns $906,743,000.
Also withdraws the claim that SB155/156 and SB195/196 contradict each other.
X40 rows after FY2002 are the wide-era SAS column persisting through the era
boundary; they say nothing about a classification-level rename. The two sets
describe different layers.
What survives is one narrow, non-blocking gap: no series_breaks row exists at
2016/2017 for Z77/Z78/X30, though review doc Sec 2 recommended exactly that.
Recorded as out-of-scope item 1 with the SB197-SB202 precedent.
jared
