Suppression query runs on every category-scoped call; needs a batch-aware skip #35

New Issue

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

jared commented

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

Deferred finding I3(b) from the uscogdata#9 final review (PR #32). A first attempt shipped and was reverted after measurement — read this before trying again.

The cost

.build_suggestions() calls .suppressed_components() unconditionally whenever a category-scoped, basis = "harmonized" query has candidate recipes. Before #9 a healthy category query returned after one cheap candidate query; now it always pays for the anti-join. That includes the shape cog_geographic_rollup() and cog_peer_compare() issue with large govid lists, and cog-api may read the corpus over HTTP.

#9 did land the bigger half of this: the NOT EXISTS subquery now restates its year/govid literals, so it prunes to the requested year partitions instead of scanning all of them (Scanning Files: 1/4 on the fixture, against ~57 partitions on the live corpus). That fix stands and is not in question here.

What was tried and why it was reverted

A .needs_suppression_query() in-memory pre-check: skip the round trip when every flow-scoped component of every candidate recipe already appears in the result's codes_included. It was correct — a scoped re-review measured 260 (gov x year x category) combinations and found 0 unsound skips — but it did not pay for itself:

skipped 7/260 (2.7%) of candidate-bearing healthy queries
skipped 0/5 of the multi-govid batch shape that motivated the finding
added an unconditional comp_rows metadata query (~1.65 ms/call) to save an expected ~0.34 ms (the real query is ~12.45 ms at a 2.7% hit rate) — net slower on the fixture

The structural reason it cannot work in that form: it requires every sibling component present for every requested (govid, year). Public Welfare — the motivating category of #9 itself — has flow-scoped components {E67, E68}, neither of which ever appears in a modern year's codes_included, so it runs the full round trip on every call regardless. Ordinary reporting variance defeats the condition almost always.

It also left an untested exactness invariant: the gate's soundness rests on codes_included and the anti-join sharing the harmonized item_code space (inst/sql/22-spending_long_harmonized.sql's REPLACE (harmonized_code AS item_code)). If that ever changed, signposting would silently stop firing — the exact silent-failure class #9 exists to prevent.

Reverted in 7707462; a comment at the call site in R/suggestions.R records the attempt.

What a real fix probably needs

Work at the batch level rather than per (govid, year) — one query covering the whole govid list, or a corpus-level precomputed "which (code, year) pairs are ever suppressed" lookup that cheaply rules out most categories.
If any future gate reintroduces the codes_included coupling, ship a fixture property test asserting "gate says skip => the real query returns 0 rows" alongside it.

Deferred finding I3(b) from the uscogdata#9 final review (PR #32). A first attempt shipped and was reverted after measurement — read this before trying again. ## The cost `.build_suggestions()` calls `.suppressed_components()` unconditionally whenever a category-scoped, `basis = "harmonized"` query has candidate recipes. Before #9 a healthy category query returned after one cheap candidate query; now it always pays for the anti-join. That includes the shape `cog_geographic_rollup()` and `cog_peer_compare()` issue with large govid lists, and cog-api may read the corpus over HTTP. #9 did land the bigger half of this: the `NOT EXISTS` subquery now restates its year/govid literals, so it prunes to the requested year partitions instead of scanning all of them (`Scanning Files: 1/4` on the fixture, against ~57 partitions on the live corpus). That fix stands and is not in question here. ## What was tried and why it was reverted A `.needs_suppression_query()` in-memory pre-check: skip the round trip when every flow-scoped component of every candidate recipe already appears in the result's `codes_included`. It was **correct** — a scoped re-review measured 260 (gov x year x category) combinations and found 0 unsound skips — but it did not pay for itself: - skipped **7/260** (2.7%) of candidate-bearing healthy queries - skipped **0/5** of the multi-govid batch shape that motivated the finding - added an **unconditional** `comp_rows` metadata query (~1.65 ms/call) to save an expected ~0.34 ms (the real query is ~12.45 ms at a 2.7% hit rate) — **net slower on the fixture** The structural reason it cannot work in that form: it requires *every* sibling component present for *every* requested (govid, year). `Public Welfare` — the motivating category of #9 itself — has flow-scoped components {E67, E68}, neither of which ever appears in a modern year's `codes_included`, so it runs the full round trip on every call regardless. Ordinary reporting variance defeats the condition almost always. It also left an **untested exactness invariant**: the gate's soundness rests on `codes_included` and the anti-join sharing the harmonized `item_code` space (`inst/sql/22-spending_long_harmonized.sql`'s `REPLACE (harmonized_code AS item_code)`). If that ever changed, signposting would silently stop firing — the exact silent-failure class #9 exists to prevent. Reverted in `7707462`; a comment at the call site in `R/suggestions.R` records the attempt. ## What a real fix probably needs - Work at the batch level rather than per (govid, year) — one query covering the whole govid list, or a corpus-level precomputed "which (code, year) pairs are ever suppressed" lookup that cheaply rules out most categories. - If any future gate reintroduces the `codes_included` coupling, ship a fixture property test asserting "gate says skip => the real query returns 0 rows" alongside it.

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:02 -04:00

Closing: measured, and the premise no longer holds

This is the "absorb or kill" call that #56 step 3 asked for. Killing it.

This issue assumes the suppression round trip is a cost worth engineering around. The
profiling in #56 says otherwise, and the evidence is now specific rather than intuitive:

The whole R-side verb pipeline — session setup, provenance assembly, crosswalk joins,
suggestion and suppression machinery — sums to tens of milliseconds against a
local corpus. The same cog_spending() call measured 66 ms local vs 3.9 s remote.
The suppression query itself is ~12.45 ms, by this issue's own measurement.
So on the remote path it is well under 1% of the cost, and on the local path it is a
small slice of an already-cheap pipeline. There is no configuration in which fixing it
changes what a user experiences.

The stronger argument is the one already recorded here: a first attempt shipped and was
reverted after measurement. .needs_suppression_query() was correct (0 unsound skips
across 260 gov × year × category combinations) and still net-negative — it skipped 7/260
candidate-bearing queries, 0/5 of the multi-govid batch shape that motivated the finding,
and added an unconditional ~1.65 ms metadata query to save an expected ~0.34 ms.

That failure was structural, not an implementation detail: the skip condition needs every
sibling component present for every requested (govid, year), and Public Welfare — the
category that motivated #9 in the first place — has flow-scoped components {E67, E68},
neither of which appears in a modern year's codes_included. Ordinary reporting variance
defeats the condition. A second attempt would have to beat both the measurement and that
structure.

The half of this that mattered already landed in #9: the NOT EXISTS subquery restates
its year/govid literals, so it prunes to the requested partitions instead of scanning all
~57. That fix stands and is not what is being closed here.

Where the leverage actually is, from the same profiling: remote HTTP round-trips, the
corpus's parquet row-group layout (fixed in pipeline #93, published 2026-08-09), and
partition-level caching. Not the R pipeline.

Method and raw numbers: the profiling comment on #56, and docs/benchmarks/ in
Civilytics/cog-api (2026-08-baseline.md, 2026-08-phase3-replicas.md,
2026-08-final.md), reproducible via scripts/bench.sh there.

Reopen if a future measurement puts the R pipeline back on the critical path — but it
should start from a number, not from the intuition that a per-call query must be worth
removing.

## Closing: measured, and the premise no longer holds This is the "absorb or kill" call that #56 step 3 asked for. Killing it. This issue assumes the suppression round trip is a cost worth engineering around. The profiling in #56 says otherwise, and the evidence is now specific rather than intuitive: - The whole R-side verb pipeline — session setup, provenance assembly, crosswalk joins, suggestion *and* suppression machinery — sums to **tens of milliseconds** against a local corpus. The same `cog_spending()` call measured **66 ms local vs 3.9 s remote**. - The suppression query itself is **~12.45 ms**, by this issue's own measurement. - So on the remote path it is well under 1% of the cost, and on the local path it is a small slice of an already-cheap pipeline. There is no configuration in which fixing it changes what a user experiences. The stronger argument is the one already recorded here: **a first attempt shipped and was reverted after measurement.** `.needs_suppression_query()` was *correct* (0 unsound skips across 260 gov × year × category combinations) and still net-negative — it skipped 7/260 candidate-bearing queries, 0/5 of the multi-govid batch shape that motivated the finding, and added an unconditional ~1.65 ms metadata query to save an expected ~0.34 ms. That failure was structural, not an implementation detail: the skip condition needs *every* sibling component present for *every* requested (govid, year), and `Public Welfare` — the category that motivated #9 in the first place — has flow-scoped components {E67, E68}, neither of which appears in a modern year's `codes_included`. Ordinary reporting variance defeats the condition. A second attempt would have to beat both the measurement and that structure. The half of this that mattered already landed in #9: the `NOT EXISTS` subquery restates its year/govid literals, so it prunes to the requested partitions instead of scanning all ~57. That fix stands and is not what is being closed here. **Where the leverage actually is**, from the same profiling: remote HTTP round-trips, the corpus's parquet row-group layout (fixed in pipeline #93, published 2026-08-09), and partition-level caching. Not the R pipeline. Method and raw numbers: the profiling comment on #56, and `docs/benchmarks/` in `Civilytics/cog-api` (`2026-08-baseline.md`, `2026-08-phase3-replicas.md`, `2026-08-final.md`), reproducible via `scripts/bench.sh` there. Reopen if a future measurement puts the R pipeline back on the critical path — but it should start from a number, not from the intuition that a per-call query must be worth removing.

jared closed this issue

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

jared referenced this issue

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

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

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

Sign in to join this conversation.