fix: literal name search, units docs, peer-summary semantics (#16, #15, #14) #22

2026-07-30T11:24:19-04:00

jared commented

2026-07-30 11:24:19 -04:00

Closes #16, #15, #14 — the three kodor/fix issues, taken over after a day with no branch, PR or comment on any of them. Batched because each is single-file with a committed acceptance test, and two share documentation surfaces.

#16 (F-025) — `cog_gov_search()` matched as an unescaped regex

Utility mode interpolated name straight into regexp_matches(), while basket mode in the same file already routed it through .escape_regex(). Two failure modes, both HTTP 200 through the API:

a government could not be found by its own complete name when that name contains a metacharacter — FREDONIA (BRISCOE) CITY returned nothing;
a bare . matched all 608 Wisconsin cities.

Malformed pattern text also reached the engine as an error, which cog-api surfaced as a 500 — reachable by typing a real name one character at a time (Athens-Clarke County (bal).

Utility mode now calls the escaper that already existed. Roxygen updated so neither mode still advertises regex semantics.

⚠️ Behaviour change: anchored exact-match searches stop working — there is no regex left to anchor. Two existing tests used "^BROWARD COUNTY$" and "^FLORIDA$" as their exact-match idiom; both now search those characters literally. Updated to bare names, which still resolve to exactly one row once scoped by state/type (verified against the fixture, not assumed). Utility mode no longer offers any exact-match option — a real if small capability loss, noted on the issue.

#15 (F-004) — the ×1000 conversion was missing where readers meet the package

Amounts are returned in full US dollars. Already stated in ?cog_spending/?cog_revenue @return, in provenance, and in cog-api's data-dictionary — absent from every surface a reader meets first. Added to README.md as its own section and to both vignettes' openings.

The dangerous one is cog_explorer/CLAUDE.md, which stated the opposite rule ("All raw amt values are in $1,000s") without scoping it to the raw column. A reader applying that to amt_nominal overstates by 1000× and gets a plausible-looking number rather than an obvious error. Fixed there too — but that directory has no git remote, so it rides in no PR and is left uncommitted for you.

#14 (F-021) — `summary_*` rows are per-category quantiles

stats::quantile() runs separately inside each (year, spend_subtype, category) cell, so summary_p50 is the median peer's value in that one category, never the value of the median peer's total. Summing across categories misstated a total-spending band by −32.7% to +251.0% across 24 years, with a sign flip at FY2012.

The verb is right and its documented use (facet by role and category) is unaffected, so the fix is @return prose plus a worked snippet showing the correct computation. This is the R-side counterpart of cog-api#9, fixed on the API surface earlier today; the wording is deliberately consistent across the two.

Two notes on the mechanics

The phrase "not additive" must stay on one roxygen source line — the test greps the generated .Rd, where a line wrap turns it into not additive and stops matching. Cost one red run to find.
man/ was regenerated with roxygen 8.0.0 against a repo built with 7.3.3, so cog_spending.Rd and DESCRIPTION were reverted — their entire diff was version churn (reindentation, RoxygenNote → Config/roxygen2/version) with no content change. The two .Rd files kept carry only the edits above. Worth deciding separately whether to bump the repo to roxygen 8.

Verification

	before	after
uscogdata suite	606 / 0 / 6	629 / 0 / 3

The three remaining skips are #11, #12 and #13.

Closes #16, #15, #14 — the three `kodor/fix` issues, taken over after a day with no branch, PR or comment on any of them. Batched because each is single-file with a committed acceptance test, and two share documentation surfaces. ## #16 (F-025) — `cog_gov_search()` matched as an unescaped regex Utility mode interpolated `name` straight into `regexp_matches()`, while **basket mode in the same file already** routed it through `.escape_regex()`. Two failure modes, both HTTP 200 through the API: - a government could not be found by **its own complete name** when that name contains a metacharacter — `FREDONIA (BRISCOE) CITY` returned nothing; - a bare `.` matched **all 608** Wisconsin cities. Malformed pattern text also reached the engine as an error, which cog-api surfaced as a **500** — reachable by typing a real name one character at a time (`Athens-Clarke County (bal`). Utility mode now calls the escaper that already existed. Roxygen updated so neither mode still advertises regex semantics. > ⚠️ **Behaviour change:** anchored exact-match searches stop working — there is no regex left to anchor. Two existing tests used `"^BROWARD COUNTY$"` and `"^FLORIDA$"` as their exact-match idiom; both now search those characters literally. Updated to bare names, which still resolve to exactly one row once scoped by state/type (verified against the fixture, not assumed). **Utility mode no longer offers any exact-match option** — a real if small capability loss, [noted on the issue](https://gitea.civilytics.org/Civilytics/uscogdata/issues/16). ## #15 (F-004) — the ×1000 conversion was missing where readers meet the package Amounts are returned in **full US dollars**. Already stated in `?cog_spending`/`?cog_revenue` `@return`, in provenance, and in cog-api's data-dictionary — absent from every surface a reader meets *first*. Added to `README.md` as its own section and to both vignettes' openings. The dangerous one is **`cog_explorer/CLAUDE.md`**, which stated the opposite rule (*"All raw `amt` values are in $1,000s"*) without scoping it to the raw column. A reader applying that to `amt_nominal` overstates by 1000× and gets a plausible-looking number rather than an obvious error. Fixed there too — but that directory has no git remote, so it rides in no PR and is **left uncommitted for you**. ## #14 (F-021) — `summary_*` rows are per-category quantiles `stats::quantile()` runs separately inside each `(year, spend_subtype, category)` cell, so `summary_p50` is *the median peer's value in that one category*, never *the value of the median peer's total*. Summing across categories misstated a total-spending band by **−32.7% to +251.0%** across 24 years, with a sign flip at FY2012. The verb is right and its documented use (facet by `role` **and** `category`) is unaffected, so the fix is `@return` prose plus a worked snippet showing the correct computation. This is the R-side counterpart of **cog-api#9**, fixed on the API surface earlier today; the wording is deliberately consistent across the two. ## Two notes on the mechanics - The phrase **"not additive" must stay on one roxygen source line** — the test greps the generated `.Rd`, where a line wrap turns it into `not additive` and stops matching. Cost one red run to find. - `man/` was regenerated with **roxygen 8.0.0 against a repo built with 7.3.3**, so `cog_spending.Rd` and `DESCRIPTION` were reverted — their entire diff was version churn (reindentation, `RoxygenNote` → `Config/roxygen2/version`) with no content change. The two `.Rd` files kept carry only the edits above. **Worth deciding separately whether to bump the repo to roxygen 8.** ## Verification | | before | after | |---|---|---| | uscogdata suite | 606 / 0 / 6 | **629 / 0 / 3** | The three remaining skips are #11, #12 and #13.

jared added 1 commit 2026-07-30 11:24:19 -04:00

fix: literal name search, units docs, peer-summary semantics (#16 , #15 , #14 )

R-CMD-check / check (push) Failing after 3m4s

Details

R-CMD-check / check (pull_request) Failing after 3m4s

Details

d006dea6e4

The three kodor/fix issues, taken over after a day with no branch, PR or
comment on any of them. Batched because each is single-file with a committed
acceptance test, and two share documentation surfaces.

#16 (F-025) -- cog_gov_search() utility mode interpolated `name` straight
into regexp_matches() unescaped, while basket mode in the same file already
routed it through .escape_regex() with the comment "so `name` is treated as
a literal substring". Two failure modes, both HTTP 200 through the API:
a government could not be found by its own complete name when that name
contains a metacharacter (FREDONIA (BRISCOE) CITY returned nothing), and a
bare "." matched all 608 Wisconsin cities. Malformed pattern text reached
the engine as an error, which cog-api surfaced as a 500 -- reachable by
typing a real name one character at a time ("Athens-Clarke County (bal").

Utility mode now calls the escaper that already existed. Roxygen updated:
utility mode is documented as a literal case-insensitive substring match,
and the basket-mode "substring fallback" step no longer describes itself as
a regex either.

  BEHAVIOUR CHANGE worth flagging: anchored exact-match searches stop
  working, because there is no regex left to anchor. Two existing tests used
  "^BROWARD COUNTY$" and "^FLORIDA$" as their exact-match idiom; both now
  search for those characters literally. Updated to the bare names, which
  still resolve to exactly one row each once scoped by state/type (verified,
  not assumed). There is no exact-match option in utility mode any more --
  noted on the issue, since that is a real if small capability loss.

#15 (F-004) -- the raw Census files report thousands of dollars; this
package multiplies by 1000 and returns full US dollars. Correct, and already
stated in ?cog_spending / ?cog_revenue @return, in provenance, and in
cog-api's data-dictionary. Absent from every surface a reader meets FIRST.
Added to README.md as its own section and to both vignettes' openings.

The dangerous one is cog_explorer/CLAUDE.md, which states the opposite rule
("All raw `amt` values are in $1,000s") without scoping it to the raw column
-- a reader applying that to amt_nominal overstates by 1000x and gets a
plausible-looking number rather than an obvious error. Fixed there too; that
directory has no git remote, so it rides in no PR and is left uncommitted
for the owner.

#14 (F-021) -- .peer_summary_rows() computes stats::quantile() separately
inside each (year, spend_subtype, category) cell, so a summary_p50 row is
"the median peer's value in that one category", never "the value of the
median peer's total" -- the median peer for Police and for Fire are usually
different governments. Summing them across categories misstated a
total-spending band by -32.7% to +251.0% across 24 years, with a sign flip
at FY2012. The verb is right and its documented use (facet by role AND
category) is unaffected, so the fix is @return prose plus a worked snippet
showing the correct computation: sum each peer's own categories first, then
take the quantile of those per-government totals.

This is the R-side counterpart of cog-api#9, fixed on the API surface
earlier today; the wording is deliberately consistent across the two.

Note the phrase "not additive" has to stay on one roxygen source line --
the test greps the generated Rd, where a line wrap turns it into
"not   additive" and stops matching. Cost one red run to find.

man/ regenerated with roxygen 8.0.0 against a repo built with 7.3.3, so
cog_spending.Rd and DESCRIPTION were reverted -- their entire diff was
version churn (reindentation, RoxygenNote -> Config/roxygen2/version) with
no content change. The two Rd files kept carry only the edits above.

Suite: 629 pass / 0 fail / 3 skip (was 606/0/6). The three remaining skips
are #11, #12 and #13.

jared referenced this pull request

2026-07-30 11:24:36 -04:00

cog_gov_search() utility mode interpolates name into regexp_matches() unescaped #16

jared added 1 commit 2026-07-30 11:31:40 -04:00

fix: let the doc-content tests survive R CMD check

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

Details

R-CMD-check / check (pull_request) Successful in 3m11s

Details

2e8383b098

CI failed on the previous commit. testthat::test_local() from a checkout was
green, but rcmdcheck was not: under R CMD check the suite runs against the
INSTALLED package, where README.md, vignettes/ and man/ do not exist. Both
newly-activated tests read them through test_path("..", "..", ...) and died
on `cannot open the connection`.

The defect was latent in the committed tests, not introduced here -- they
shipped skip()ped, so CI had never executed either one. Removing the skips
is what exposed it, which is the mechanism working as intended.

Guarded with skip_if_no_source_tree(), so they skip in the installed-package
context that structurally cannot satisfy them. They are NOT thereby unchecked
in CI: the workflow runs testthat::test_local() from the checkout as its own
step before rcmdcheck, and there the paths resolve and the assertions run.

Deliberately not split: test-peer-summary-scope.R's numeric pin needs only
the corpus and would survive check on its own, but it exists to protect the
sentence above it. Separating them would let the prose drift while the pin
kept passing.

Verified locally: test_local 629 pass / 0 fail / 3 skip; rcmdcheck
0 errors / 0 warnings / 0 notes.

jared merged commit 8db944e4a0 into main

2026-07-30 11:37:08 -04:00

jared referenced this pull request

2026-07-30 12:01:43 -04:00

cog_peer_compare() summary_* rows are per-category quantiles, not per-government totals, and this is undocumented #14

jared referenced this pull request

2026-07-30 12:01:43 -04:00

Returned amounts are full US dollars; the x1000 conversion is undocumented in reader-facing docs #15

Sign in to join this conversation.