feat: report the coarse-vs-per-code subset relation in the signposting harness
The coarse and per-code signposting checks are partly DISJOINT, not nested: coarse fires on queries per-code does not, so the coarse -> percode move both adds and removes signposting. Every `*_delta_pp` the harness reports is therefore a NET that can mask a coverage loss in either direction. The staged-corpus headline (+1.875 pp, coarse 1/640 -> percode 13/640) sits on top of Corrections losing coverage outright (0.05 -> 0.00, -5 pp). The cause is structural, not sampling: coarse's coverage test is at recipe grain and self-coverage-permissive, while per-code requires a DIFFERENT component of the same recipe. When a whole category is empty in a year -- coarse's own trigger -- and the only covering evidence is the gapped component's own wide-era aggregate row, per-code cannot fire by construction. That case is already pinned as intended behaviour in test-recipes.R; this change measures what it costs, it does not change it. Measurement and disclosure only. R/suggestions.R is untouched -- which arm ships is the human ruling at Checkpoint R3. - header: replace the "noise trade" framing with an explicit statement that the checks are partly disjoint and every delta is a net - .measure_subset_relation(): split the disagreement into violations (coarse fired, per-code silent -- coverage LOST) and additions, returning the offending rows, not just counts. No assertion: the violation set is genuinely non-empty and a stopifnot() would only break the harness that is supposed to surface it - .measure_format_subset_report(): prominent HOLDS / *** VIOLATED *** section naming each offending (category, government, year) - detail gains coarse_gap_years / coarse_recipes / percode_recipes; by_category gains n_coarse_only / n_percode_only so the two netted flows are visible per category - new test-signposting-harness.R pins the reporting, including inversion guards and an end-to-end case (Broward FY2011 Corrections) where coarse fires and per-code does not Tests: 518 PASS / 0 FAIL / 0 WARN / 0 SKIP (was 476). Mutation-checked: inverting the violation direction fails 18 assertions, removing the violation reporting fails 9.
This commit is contained in:
@@ -24,9 +24,41 @@
|
||||
# sibling component to supply the covering evidence -- the shipped,
|
||||
# corrected implementation.
|
||||
#
|
||||
# This script MEASURES the deltas; it does not decide whether the added
|
||||
# signposting "noise" going coarse -> percode is acceptable. That is
|
||||
# Jared's ruling at Checkpoint R3 (see
|
||||
# READ THIS BEFORE QUOTING ANY DELTA FROM THIS SCRIPT
|
||||
# -----------------------------------------------------
|
||||
# The coarse and per-code checks are PARTLY DISJOINT, not nested. Per-code
|
||||
# is NOT a strict widening of coarse: there are queries coarse fires on that
|
||||
# per-code does not, so moving coarse -> percode both ADDS and REMOVES
|
||||
# signposting. Every `*_delta_pp` figure this script reports -- overall and
|
||||
# per category -- is therefore a NET of those two flows and can mask a
|
||||
# coverage loss in either direction. A headline "+X pp" can sit on top of
|
||||
# categories that lost coverage outright (a NEGATIVE corrected_delta_pp),
|
||||
# and a category-level zero can be an add and a loss cancelling. Read
|
||||
# `$subset_relation` (printed under "Subset relation" below) alongside any
|
||||
# delta; that section is where the two flows are separated.
|
||||
#
|
||||
# The disjointness is structural, not a sampling artifact. Both arms pair a
|
||||
# gap test with a coverage test, and it is the COVERAGE test that differs:
|
||||
# - coarse: gap = the WHOLE category result has zero rows that year;
|
||||
# covered = the recipe's generic join has ANY row that year
|
||||
# (unioned across all components -- a component's own
|
||||
# aggregate row counts).
|
||||
# - percode: gap = one specific component has no non-aggregate row that
|
||||
# year; covered = a DIFFERENT component of the SAME recipe has
|
||||
# a row that year (self-coverage explicitly excluded, per
|
||||
# review-doc S: 0.3's "...but OTHER components do").
|
||||
# So when a whole category is empty in a year -- exactly coarse's trigger --
|
||||
# and the only covering evidence is the gapped component's own wide-era
|
||||
# aggregate row, coarse fires and per-code CANNOT: there is by construction
|
||||
# no other component to supply the evidence. That case is already pinned as
|
||||
# intended behaviour in tests/testthat/test-recipes.R ("per-code gap does
|
||||
# NOT fire when a code's only coverage is its own aggregate row"). This
|
||||
# script's job is to say how often it costs coverage, not to relitigate it.
|
||||
#
|
||||
# This script MEASURES the deltas; it does not decide whether the resulting
|
||||
# signposting trade -- added "noise" in one direction, lost whole-category
|
||||
# gap coverage in the other -- is acceptable. That is Jared's ruling at
|
||||
# Checkpoint R3 (see
|
||||
# cog_pipeline/.superpowers/sdd/phase-r-task-19c-brief.md). The selfcov arm
|
||||
# exists purely to answer a narrower, mechanical question for that ruling:
|
||||
# how much of the coarse -> percode delta was ever attributable to the
|
||||
@@ -145,9 +177,27 @@
|
||||
sort(sample(pool, n))
|
||||
}
|
||||
|
||||
#' Comma-join a suggestion list's recipe ids (stable order) for the detail
|
||||
#' frame's audit columns; `""` when nothing fired.
|
||||
#' @noRd
|
||||
.measure_recipe_ids <- function(suggestions) {
|
||||
if (length(suggestions) == 0L) return("")
|
||||
paste(sort(vapply(suggestions, function(s) s$recipe_id, character(1))),
|
||||
collapse = ",")
|
||||
}
|
||||
|
||||
#' Run one (category, government) query through the coarse (`coarse_env`),
|
||||
#' self-coverage-allowed (`selfcov_env`), and live per-code
|
||||
#' `.build_suggestions()` and return a one-row summary of what each fired.
|
||||
#'
|
||||
#' Also records the coarse arm's OWN trigger evidence -- `coarse_gap_years`,
|
||||
#' the requested years in which the whole category result has zero rows --
|
||||
#' so a coarse-fired/per-code-silent disagreement can be named down to
|
||||
#' (category, government, year) instead of just counted. Per-code's gap
|
||||
#' years are deliberately NOT re-derived here: that would mean
|
||||
#' reimplementing `.recipe_component_gapped()`'s set arithmetic in the
|
||||
#' measurement harness, where it could silently drift from the code under
|
||||
#' measurement. Per-code rows are identified by the recipe ids they fired.
|
||||
#' @noRd
|
||||
.measure_one_query <- function(con, coarse_env, selfcov_env, category,
|
||||
category_type, govid, years) {
|
||||
@@ -172,21 +222,140 @@
|
||||
)
|
||||
percode_sugg <- .build_suggestions(con, govid, years, category, "harmonized")
|
||||
|
||||
result_years <- if (nrow(result) == 0L) integer(0) else unique(as.integer(result$year))
|
||||
gap_years <- sort(setdiff(as.integer(years), result_years))
|
||||
|
||||
data.frame(
|
||||
category = category,
|
||||
category_type = category_type,
|
||||
canonical_govid = govid,
|
||||
n_result_rows = nrow(result),
|
||||
n_coarse = length(coarse_sugg),
|
||||
n_selfcov = length(selfcov_sugg),
|
||||
n_percode = length(percode_sugg),
|
||||
fired_coarse = length(coarse_sugg) > 0L,
|
||||
fired_selfcov = length(selfcov_sugg) > 0L,
|
||||
fired_percode = length(percode_sugg) > 0L,
|
||||
category = category,
|
||||
category_type = category_type,
|
||||
canonical_govid = govid,
|
||||
n_result_rows = nrow(result),
|
||||
coarse_gap_years = paste(gap_years, collapse = ","),
|
||||
n_coarse = length(coarse_sugg),
|
||||
n_selfcov = length(selfcov_sugg),
|
||||
n_percode = length(percode_sugg),
|
||||
fired_coarse = length(coarse_sugg) > 0L,
|
||||
fired_selfcov = length(selfcov_sugg) > 0L,
|
||||
fired_percode = length(percode_sugg) > 0L,
|
||||
coarse_recipes = .measure_recipe_ids(coarse_sugg),
|
||||
percode_recipes = .measure_recipe_ids(percode_sugg),
|
||||
stringsAsFactors = FALSE
|
||||
)
|
||||
}
|
||||
|
||||
#' Columns that identify a disagreeing query well enough for a human to go
|
||||
#' and inspect it, in print order. Intersected with what `detail` actually
|
||||
#' has, so this works on a minimal hand-built frame too.
|
||||
#' @noRd
|
||||
.MEASURE_IDENTITY_COLS <- c(
|
||||
"category", "category_type", "canonical_govid", "coarse_gap_years",
|
||||
"n_result_rows", "coarse_recipes", "percode_recipes"
|
||||
)
|
||||
|
||||
#' Separate the two flows the `*_delta_pp` figures net together.
|
||||
#'
|
||||
#' Per-code is NOT a widening of coarse (see this file's header): the two
|
||||
#' checks pair different gap tests with different coverage tests, so moving
|
||||
#' coarse -> percode both adds and removes firings. This splits the
|
||||
#' disagreement into:
|
||||
#' - `violations`: coarse fired, per-code did NOT -- signposting coverage
|
||||
#' LOST. These are what a net delta hides. `holds` is FALSE whenever
|
||||
#' this is non-empty, i.e. whenever coarse is not a subset of per-code.
|
||||
#' - `additions`: per-code fired, coarse did NOT -- the expected gain.
|
||||
#'
|
||||
#' Deliberately returns the offending rows, not just counts, so the
|
||||
#' Checkpoint R3 ruling can be made against named (category, government,
|
||||
#' year) cases. Deliberately does NOT assert -- the violation set is really
|
||||
#' non-empty on the staged corpus, and a hard assertion here would only
|
||||
#' break the harness that is supposed to report it.
|
||||
#' @noRd
|
||||
.measure_subset_relation <- function(detail) {
|
||||
required <- c("category", "canonical_govid", "fired_coarse", "fired_percode")
|
||||
absent <- if (is.data.frame(detail)) setdiff(required, names(detail)) else required
|
||||
if (!is.data.frame(detail) || length(absent) > 0L) {
|
||||
stop("`detail` must be a data frame with columns ",
|
||||
paste(required, collapse = ", "), " (missing: ",
|
||||
paste(absent, collapse = ", "), ").", call. = FALSE)
|
||||
}
|
||||
fired_coarse <- as.logical(detail$fired_coarse)
|
||||
fired_percode <- as.logical(detail$fired_percode)
|
||||
if (anyNA(fired_coarse) || anyNA(fired_percode)) {
|
||||
stop("`fired_coarse`/`fired_percode` must be non-NA logicals.", call. = FALSE)
|
||||
}
|
||||
|
||||
keep <- intersect(.MEASURE_IDENTITY_COLS, names(detail))
|
||||
viol_idx <- which(fired_coarse & !fired_percode)
|
||||
add_idx <- which(fired_percode & !fired_coarse)
|
||||
|
||||
list(
|
||||
holds = length(viol_idx) == 0L,
|
||||
n_queries = nrow(detail),
|
||||
n_coarse_fired = sum(fired_coarse),
|
||||
n_percode_fired = sum(fired_percode),
|
||||
n_both = sum(fired_coarse & fired_percode),
|
||||
n_violations = length(viol_idx),
|
||||
n_additions = length(add_idx),
|
||||
violations = detail[viol_idx, keep, drop = FALSE],
|
||||
additions = detail[add_idx, keep, drop = FALSE]
|
||||
)
|
||||
}
|
||||
|
||||
#' Render a data frame of disagreeing queries as indented report lines,
|
||||
#' capped at `max_rows` with an explicit note about what was withheld (the
|
||||
#' full set is always in the returned `$subset_relation`).
|
||||
#' @noRd
|
||||
.measure_fmt_rows <- function(df, max_rows = 50L) {
|
||||
if (nrow(df) == 0L) return(" (none)")
|
||||
shown <- utils::head(df, max_rows)
|
||||
out <- paste0(" ", utils::capture.output(print(shown, row.names = FALSE)))
|
||||
if (nrow(df) > max_rows) {
|
||||
out <- c(out, sprintf(" ... %d more row(s) not shown; full set in $subset_relation.",
|
||||
nrow(df) - max_rows))
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
#' Format `.measure_subset_relation()` as a prominent, clearly-labelled
|
||||
#' report section. States plainly whether coarse is a subset of per-code
|
||||
#' and, when it is not, exactly where it breaks.
|
||||
#' @noRd
|
||||
.measure_format_subset_report <- function(rel, max_rows = 50L) {
|
||||
lines <- c(
|
||||
"==== Subset relation: is COARSE a subset of PER-CODE? ====",
|
||||
sprintf("Queries: %d | coarse fired: %d | per-code fired: %d | both: %d",
|
||||
rel$n_queries, rel$n_coarse_fired, rel$n_percode_fired, rel$n_both)
|
||||
)
|
||||
if (rel$holds) {
|
||||
lines <- c(lines, sprintf(paste(
|
||||
"HOLDS: coarse IS a subset of per-code -- 0 of %d queries fire under",
|
||||
"coarse but not per-code. On THIS battery the delta is a pure",
|
||||
"addition of %d query/queries, with no coverage lost."
|
||||
), rel$n_queries, rel$n_additions))
|
||||
} else {
|
||||
lines <- c(lines,
|
||||
"*** VIOLATED: coarse is NOT a subset of per-code. ***",
|
||||
sprintf(paste(
|
||||
"%d of %d queries fire under COARSE but NOT under PER-CODE:",
|
||||
"signposting coverage the move LOSES."
|
||||
), rel$n_violations, rel$n_queries),
|
||||
sprintf(paste(
|
||||
"Every delta reported above is therefore a NET of %d addition(s)",
|
||||
"MINUS %d loss(es), and understates both. Do not read it as",
|
||||
"'per-code fires wherever coarse did, plus more'."
|
||||
), rel$n_additions, rel$n_violations),
|
||||
"",
|
||||
paste(" COVERAGE LOST -- coarse fired, per-code silent.",
|
||||
"`coarse_gap_years` is the requested year(s) in which the whole",
|
||||
"category result was empty (coarse's own trigger evidence):"),
|
||||
.measure_fmt_rows(rel$violations, max_rows)
|
||||
)
|
||||
}
|
||||
c(lines, "",
|
||||
sprintf(" COVERAGE ADDED -- per-code fired, coarse silent (%d query/queries):",
|
||||
rel$n_additions),
|
||||
.measure_fmt_rows(rel$additions, max_rows))
|
||||
}
|
||||
|
||||
#' Measure the coarse-vs-per-code signposting suggestion rate over a
|
||||
#' realistic query battery (every category x a pre/post-boundary_year span
|
||||
#' x up to n_gov sampled governments).
|
||||
@@ -290,6 +459,14 @@ measure_signposting_rate <- function(corpus_url = Sys.getenv("USCOGDATA_URL", un
|
||||
stopifnot(all(detail$fired_percode <= detail$fired_selfcov))
|
||||
detail$fired_selfcov_only <- detail$fired_selfcov & !detail$fired_percode
|
||||
|
||||
# coarse vs percode is NOT a subset relation the way percode vs selfcov
|
||||
# is (see header). Measured and REPORTED, never asserted: the violation
|
||||
# set is genuinely non-empty on the staged corpus, and a stopifnot() here
|
||||
# would break the harness whose whole job is to surface it.
|
||||
subset_relation <- .measure_subset_relation(detail)
|
||||
detail$coarse_only <- detail$fired_coarse & !detail$fired_percode
|
||||
detail$percode_only <- detail$fired_percode & !detail$fired_coarse
|
||||
|
||||
by_category <- dplyr::summarise(
|
||||
dplyr::group_by(detail, category, category_type),
|
||||
n_queries = dplyr::n(),
|
||||
@@ -299,6 +476,9 @@ measure_signposting_rate <- function(corpus_url = Sys.getenv("USCOGDATA_URL", un
|
||||
original_delta_pp = (mean(fired_selfcov) - mean(fired_coarse)) * 100,
|
||||
corrected_delta_pp = (mean(fired_percode) - mean(fired_coarse)) * 100,
|
||||
selfcov_share_pp = mean(fired_selfcov_only) * 100,
|
||||
# the two flows corrected_delta_pp nets together, per category
|
||||
n_coarse_only = sum(coarse_only),
|
||||
n_percode_only = sum(percode_only),
|
||||
.groups = "drop"
|
||||
)
|
||||
by_category <- dplyr::arrange(by_category, dplyr::desc(corrected_delta_pp))
|
||||
@@ -336,19 +516,27 @@ measure_signposting_rate <- function(corpus_url = Sys.getenv("USCOGDATA_URL", un
|
||||
summary_overall$selfcov_share_pp,
|
||||
sum(detail$fired_selfcov_only), summary_overall$n_queries
|
||||
))
|
||||
message(if (subset_relation$holds) {
|
||||
sprintf("Subset relation coarse <= percode HOLDS (0 coarse-only firings); delta is a pure addition of %d.",
|
||||
subset_relation$n_additions)
|
||||
} else {
|
||||
sprintf("*** Subset relation coarse <= percode VIOLATED: %d coarse-only firing(s) LOST vs %d percode-only added. Deltas above are NETS. ***",
|
||||
subset_relation$n_violations, subset_relation$n_additions)
|
||||
})
|
||||
}
|
||||
|
||||
invisible(list(
|
||||
corpus_url = corpus_url,
|
||||
years = years,
|
||||
span_note = span$note,
|
||||
full_design = span$full_design,
|
||||
govids = govids,
|
||||
seed = seed,
|
||||
n_categories = nrow(categories),
|
||||
detail = detail,
|
||||
by_category = by_category,
|
||||
summary = summary_overall
|
||||
corpus_url = corpus_url,
|
||||
years = years,
|
||||
span_note = span$note,
|
||||
full_design = span$full_design,
|
||||
govids = govids,
|
||||
seed = seed,
|
||||
n_categories = nrow(categories),
|
||||
detail = detail,
|
||||
by_category = by_category,
|
||||
summary = summary_overall,
|
||||
subset_relation = subset_relation
|
||||
))
|
||||
}
|
||||
|
||||
@@ -365,5 +553,11 @@ if (identical(environment(), globalenv()) && sys.nframe() == 0L) {
|
||||
print(res$summary)
|
||||
|
||||
cat("\n==== By category (sorted by corrected delta, descending) ====\n")
|
||||
cat("NOTE: corrected_delta_pp is a NET. n_coarse_only = firings LOST going\n")
|
||||
cat("coarse -> percode; n_percode_only = firings ADDED. A category can be\n")
|
||||
cat("negative (net coverage loss) even when the overall figure is positive.\n")
|
||||
print(as.data.frame(res$by_category), row.names = FALSE)
|
||||
|
||||
cat("\n")
|
||||
cat(paste(.measure_format_subset_report(res$subset_relation), collapse = "\n"), "\n")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user