test: add failing tests for Madison walkthrough findings #17

2026-07-29T00:30:34-04:00

jared commented

2026-07-29 00:30:34 -04:00

Six tests, one per issue opened from the Madison walkthrough audit (cog_explorer/docs/walkthroughs/FINDINGS.md, 2026-07-28). No production code changes — this PR only makes the tracked work mechanically verifiable.

Issues these tests correspond to

Test file	Issue	Findings	Severity	Verdict
`test-expenditure-concepts.R`	#11	F-012, F-017, F-018	high	F-017 defect; F-012/F-018 definitional
`test-revenue-concept-insurance-trust.R`	#12	F-014	medium	definitional
`test-coverage-disclosure.R`	#13	F-020, F-023	high	definitional
`test-peer-summary-scope.R`	#14	F-021	high	definitional
`test-amount-units-documented.R`	#15	F-004	high	definitional
`test-gov-search-literal-match.R`	#16	F-025	high	defect

Safe to merge — CI stays green

Every test is skipped, each by a single testthat::skip() on the first line of the test body naming its issue and finding IDs:

testthat::skip("Blocked on uscogdata#16 (finding F-025)")

The real assertions sit beneath it, so activating a test when its fix lands is a one-line deletion.

Verified locally against the bundled fixture corpus:

with the skips in place — full suite 576 pass / 0 fail / 6 skip;
with the skips removed — all six fail or error, for the reasons the findings describe (e.g. test-gov-search-literal-match.R: cog_gov_search("FREDONIA (BRISCOE) CITY") returns 0 rows for a government that exists, and cog_gov_search("[") raises Invalid Input Error: missing ]).

Notes on how the tests are built

helper-walkthrough-raw.R opens its own DuckDB connection straight onto the corpus's long parquet partitions, bypassing this package's SQL views. Every expected amount comes from there rather than from the verb under test — verifying an absence through the filter that creates it proves nothing, and that was the single most common defect in the audit itself.
Fixture year window. The audit's headline reconciliations are FY2022 (Madison I89 = 46,609 thousands, 7.1% below Census's published Direct Expenditure) and FY1970–FY1986 (Madison's prefix-X revenue). Both are outside the fixture's 2011/2012/2019/2020, so the same invariants are asserted where the fixture does carry them — Madison FY2020 (I89 = 27,704) and Wisconsin state FY2012 (X01+X05+X08 = 2,038,800) — with the out-of-window figures recorded in comments.
The coverage test reproduces both findings on the fixture exactly: Wisconsin's 608-city universe rolls up 597 governments in FY2012 (census year) against 152/112/114 in FY2011/2019/2020, and Chilton City's 15-peer cohort taken at FY2012 collapses to 3 of 15 in FY2019 and FY2020 — the same 3-of-15 the audit measured on the live corpus.
Two tests deliberately assert less than they could, and say so inline:
- test-revenue-concept-insurance-trust.R calls a revenue_concept = "total" argument that is this test's proposal. The owner's 2026-07-28 resolution covers expenditure concepts only; no revenue-side naming has been ruled on. The asserted dollar invariants are independent of the eventual name.
- test-gov-search-literal-match.R does not assert on the finding's q=St. Louis example: under correct literal matching that search still returns 0 rows, because the stored name is ST LOUIS CITY with no period.
test-coverage-disclosure.R accepts the always-on coverage metadata either as columns on the returned tibble or as provenance$coverage — the settled design fixes the three field names (n_units_reporting, n_units_expected, is_census_year) and that they reach the caller, not the container.

Six tests, one per issue opened from the **Madison walkthrough audit** (`cog_explorer/docs/walkthroughs/FINDINGS.md`, 2026-07-28). No production code changes — this PR only makes the tracked work mechanically verifiable. ## Issues these tests correspond to | Test file | Issue | Findings | Severity | Verdict | |---|---|---|---|---| | `test-expenditure-concepts.R` | #11 | F-012, F-017, F-018 | high | F-017 defect; F-012/F-018 definitional | | `test-revenue-concept-insurance-trust.R` | #12 | F-014 | medium | definitional | | `test-coverage-disclosure.R` | #13 | F-020, F-023 | high | definitional | | `test-peer-summary-scope.R` | #14 | F-021 | high | definitional | | `test-amount-units-documented.R` | #15 | F-004 | high | definitional | | `test-gov-search-literal-match.R` | #16 | F-025 | high | defect | ## Safe to merge — CI stays green **Every test is skipped**, each by a single `testthat::skip()` on the first line of the test body naming its issue and finding IDs: ```r testthat::skip("Blocked on uscogdata#16 (finding F-025)") ``` The real assertions sit beneath it, so activating a test when its fix lands is a one-line deletion. Verified locally against the bundled fixture corpus: * **with the skips in place** — full suite `576 pass / 0 fail / 6 skip`; * **with the skips removed** — all six fail or error, for the reasons the findings describe (e.g. `test-gov-search-literal-match.R`: `cog_gov_search("FREDONIA (BRISCOE) CITY")` returns 0 rows for a government that exists, and `cog_gov_search("[")` raises `Invalid Input Error: missing ]`). ## Notes on how the tests are built * **`helper-walkthrough-raw.R`** opens its own DuckDB connection straight onto the corpus's `long` parquet partitions, bypassing this package's SQL views. Every expected amount comes from there rather than from the verb under test — verifying an absence through the filter that creates it proves nothing, and that was the single most common defect in the audit itself. * **Fixture year window.** The audit's headline reconciliations are FY2022 (Madison `I89 = 46,609` thousands, 7.1% below Census's published Direct Expenditure) and FY1970–FY1986 (Madison's prefix-`X` revenue). Both are outside the fixture's `2011/2012/2019/2020`, so the same invariants are asserted where the fixture does carry them — Madison FY2020 (`I89 = 27,704`) and Wisconsin state FY2012 (`X01+X05+X08 = 2,038,800`) — with the out-of-window figures recorded in comments. * **The coverage test reproduces both findings on the fixture exactly**: Wisconsin's 608-city universe rolls up 597 governments in FY2012 (census year) against 152/112/114 in FY2011/2019/2020, and Chilton City's 15-peer cohort taken at FY2012 collapses to **3 of 15** in FY2019 and FY2020 — the same 3-of-15 the audit measured on the live corpus. * **Two tests deliberately assert less than they could**, and say so inline: * `test-revenue-concept-insurance-trust.R` calls a `revenue_concept = "total"` argument that is this test's *proposal*. The owner's 2026-07-28 resolution covers expenditure concepts only; no revenue-side naming has been ruled on. The asserted dollar invariants are independent of the eventual name. * `test-gov-search-literal-match.R` does not assert on the finding's `q=St. Louis` example: under correct literal matching that search still returns 0 rows, because the stored name is `ST LOUIS CITY` with no period. * `test-coverage-disclosure.R` accepts the always-on coverage metadata either as columns on the returned tibble or as `provenance$coverage` — the settled design fixes the three field names (`n_units_reporting`, `n_units_expected`, `is_census_year`) and that they reach the caller, not the container.

jared added 1 commit 2026-07-29 00:30:34 -04:00

test: add failing tests for Madison walkthrough findings

R-CMD-check / check (push) Successful in 3m3s

Details

R-CMD-check / check (pull_request) Successful in 2m58s

Details

9233c3d18e

Six skipped tests, one per issue opened from the Madison walkthrough audit
(docs/walkthroughs/FINDINGS.md in cog_explorer). Each asserts the desired
behaviour, so it fails today and goes green when the fix lands; each is
guarded by a single skip() naming its issue and finding IDs, so the suite
stays green and activating a test is a one-line deletion.

  test-expenditure-concepts.R             #11  F-012, F-017, F-018
  test-revenue-concept-insurance-trust.R  #12  F-014
  test-coverage-disclosure.R              #13  F-020, F-023
  test-peer-summary-scope.R               #14  F-021
  test-amount-units-documented.R          #15  F-004
  test-gov-search-literal-match.R         #16  F-025

helper-walkthrough-raw.R reads the corpus's long parquet partitions directly,
bypassing uscogdata's SQL views. Every expected amount comes from there rather
than from the verb under test - verifying an absence through the filter that
creates it proves nothing, which was the most common defect in the audit itself.

Verified: with the skips removed all six fail (or error) against the bundled
fixture; with them in place the full suite is 576 pass / 0 fail / 6 skip.

kodor reviewed

2026-07-29 01:08:45 -04:00

kodor left a comment

COMMENT ## Kodor PR Review — #17: test: add failing tests for Madison walkthrough findings

Verdict: ✅ Looks good — clean, well-structured test scaffolding.

Strengths

Independent ground truth via raw DuckDB queries — The helper () bypasses the package's SQL views and reads directly from parquet partitions. This is exactly right: you can't verify a filter by running the same filter.
Every test is properly skipped with pointing to the specific issue/finding ID. Activation is a one-line deletion when fixes land.
Clear mapping table in PR description linking each test file to its issue number, finding IDs, severity, and verdict.
Good fixture-aware assertions — Tests use FY2012/FY2020 from the bundled fixture while documenting the out-of-window FY2022 headline numbers in comments.
**** covers both correctness (parentheses, metacharacters) and robustness (malformed regex that errors). The raw parquet cross-check for period-containing names is a nice touch.
**** reproduces the exact 3-of-15 peer cohort collapse on the fixture — strong regression anchor.

Minor Notes (non-blocking)

**** proposes before the argument name is settled. You note this inline — good practice. Just means whoever picks up #12 will need to update the two calls if naming differs.
**** uses with skip if unset. Consider adding a comment or note that CI needs this env var set, so future contributors don't get confused by silent skips.

No Issues Found

No security concerns (test-only code, no production changes)
No style issues — consistent formatting and thorough inline documentation
Logic is sound across all 7 files

Recommendation: Safe to merge. CI stays green (576 pass / 0 fail / 6 skip).

COMMENT ## Kodor PR Review — #17: test: add failing tests for Madison walkthrough findings **Verdict: ✅ Looks good — clean, well-structured test scaffolding.** ### Strengths 1. **Independent ground truth via raw DuckDB queries** — The helper () bypasses the package's SQL views and reads directly from parquet partitions. This is exactly right: you can't verify a filter by running the same filter. 2. **Every test is properly skipped** with pointing to the specific issue/finding ID. Activation is a one-line deletion when fixes land. 3. **Clear mapping table in PR description** linking each test file to its issue number, finding IDs, severity, and verdict. 4. **Good fixture-aware assertions** — Tests use FY2012/FY2020 from the bundled fixture while documenting the out-of-window FY2022 headline numbers in comments. 5. **** covers both correctness (parentheses, metacharacters) and robustness (malformed regex that errors). The raw parquet cross-check for period-containing names is a nice touch. 6. **** reproduces the exact 3-of-15 peer cohort collapse on the fixture — strong regression anchor. ### Minor Notes (non-blocking) - **** proposes before the argument name is settled. You note this inline — good practice. Just means whoever picks up #12 will need to update the two calls if naming differs. - **** uses with skip if unset. Consider adding a comment or note that CI needs this env var set, so future contributors don't get confused by silent skips. ### No Issues Found - No security concerns (test-only code, no production changes) - No style issues — consistent formatting and thorough inline documentation - Logic is sound across all 7 files **Recommendation: Safe to merge. CI stays green (576 pass / 0 fail / 6 skip).**

jared merged commit 82acda6f93 into main

2026-07-29 10:35:32 -04:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Civilytics/uscogdata#17