Skip to content

Commit 8921f36

Browse files
bbestclaude
andcommitted
feat(check): the cruise that leaves obs and keeps its casts (3.12.0)
Release v2026.08.08 shipped 10 calcofi_ctd-cast cruises holding all 1,186 of their casts and none of their 874,000 observations. Nothing in the pipeline said a word, and nothing was going to: PK/FK validation runs child -> parent, so every surviving obs row still had a parent cast, and a parent with NO CHILDREN violates no constraint. The bounds backstop inspects obs, which these cruises had entirely left. No check anywhere looked at the parent side. check_cruise_coverage() is that check. Three things it has to get right: - The grain is the CRUISE, not the sample. A CTD `sample` row is one physical cast per DIRECTION while obs keeps one direction, so ~half of that dataset's cast rows legitimately carry no observations; a per-sample assertion is wrong on arrival. A whole cruise with none never is. - It joins through sample_key, never obs.cruise_key — that denormalized column is NULL on 59,274 swfsc_cufes rows and 14,170 euphausiid ones and would invent orphans that do not exist. A test drops the column outright so an implementation that reaches for it fails rather than passing quietly. - A dataset emitting no observations at all is exempt. sio_pic-zooplankton is a net-tow registry whose biovolumes are pending from the provider: 587 sample-only cruises that must not fail. The rule is relative, so no allowlist is needed to say so. max_orphan_cruises takes a named per-dataset vector, so the release can ratchet a documented backlog while a new orphan still fails; an ingest that knows its own correct answer passes 0. Also fixes a stale fixture in test-in_release.R that declared a directory `output:` and had been failing since check_nested_outputs() landed in 3.11.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b39c3c7 commit 8921f36

7 files changed

Lines changed: 267 additions & 2 deletions

File tree

DESCRIPTION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Package: calcofi4db
22
Title: CalCOFI Database Tools
3-
Version: 3.11.0
3+
Version: 3.12.0
44
URL: https://calcofi.io/calcofi4db
55
BugReports: https://github.com/calcofi/calcofi4db/issues
66
Authors@R:

NAMESPACE

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export(build_taxon_table)
2929
export(cc_stage_dir)
3030
export(cc_stage_path)
3131
export(changed_inputs)
32+
export(check_cruise_coverage)
3233
export(check_data_integrity)
3334
export(check_measurement_bounds)
3435
export(check_multiple_datasets)

NEWS.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,38 @@
1+
# calcofi4db 3.12.0
2+
3+
## `check_cruise_coverage()` — the cruise that leaves `obs` and keeps its samples
4+
5+
Release `v2026.08.08` shipped 10 `calcofi_ctd-cast` cruises that had lost **every
6+
one of their 874,000 observations** while keeping all 1,186 of their casts. The
7+
CTD transects app went from 142 cruises to 132 overnight; nothing in the pipeline
8+
said a word.
9+
10+
Nothing was going to. PK/FK validation runs child -> parent, so every `obs` row
11+
that remained still had a parent cast — and a parent with **no children** violates
12+
no constraint. The bounds backstop only inspects `obs`, which these cruises had
13+
entirely left. There was no check anywhere that looked at the parent side.
14+
15+
`check_cruise_coverage(con)` is that check: one row per `dataset_key` with
16+
`cruises`, `cruises_no_obs` and `orphan_samples`, halting when a dataset exceeds
17+
its allowance. Three things it gets right that a first cut would not:
18+
19+
- **The grain is the cruise, not the sample.** A CTD `sample` row is one physical
20+
cast *per direction* while `obs` keeps one direction, so ~half of
21+
`calcofi_ctd-cast`'s cast rows legitimately carry no observations. A per-sample
22+
assertion is wrong on arrival; a whole cruise with none never is.
23+
- **It joins through `sample_key`, never `obs.cruise_key`.** That denormalized
24+
column is NULL on 59,274 `swfsc_cufes` rows and 14,170 euphausiid ones, which
25+
would invent orphans that do not exist.
26+
- **A dataset emitting no observations at all is exempt.** `sio_pic-zooplankton`
27+
is a net-tow registry whose biovolumes are pending from the provider, so
28+
`sample`-only is its designed state — 587 cruises that must not fail. The rule
29+
is relative ("if a dataset contributes observations, every one of its cruises
30+
must"), so it needs no allowlist to say so.
31+
32+
`max_orphan_cruises` takes a named per-dataset vector so the release can ratchet a
33+
documented backlog while a *new* orphan still fails; an ingest that knows its own
34+
correct answer passes `0`.
35+
136
# calcofi4db 3.11.0
237

338
## `build_targets_list()` refuses a directory `output:`

R/check.R

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,3 +483,104 @@ check_taxon_ids <- function(con, allow = character(), halt = TRUE,
483483

484484
if (verbose) rpt else invisible(rpt)
485485
}
486+
# check_cruise_coverage --------------------------------------------------------
487+
488+
#' Cruises that carry samples but no observations — the silent-loss guard
489+
#'
490+
#' A cruise can leave `obs` without leaving `sample`, and nothing about that
491+
#' violates a foreign key: FK validation runs child -> parent, so every surviving
492+
#' `obs` row still has a parent, and a parent with **no children** breaks no
493+
#' constraint. Release `v2026.08.08` shipped in exactly that state — 10
494+
#' `calcofi_ctd-cast` cruises kept all 1,186 of their casts and lost all 874,000
495+
#' of their observations, because a Google Drive placeholder read as zero rows and
496+
#' the direction letter the thinning step needs came from the filename of a
497+
#' conflict copy. No check anywhere looked at the parent side.
498+
#'
499+
#' The grain is the **cruise**, deliberately, and it is not the sample. A CTD
500+
#' `sample` row is one physical cast *per direction* while `obs` keeps a single
501+
#' direction, so about half of `calcofi_ctd-cast`'s cast rows legitimately carry
502+
#' no observations and a per-sample assertion would be wrong on arrival. A whole
503+
#' cruise with none is never legitimate.
504+
#'
505+
#' A dataset that emits **no** observations at all is exempt rather than failing
506+
#' 587 times: `sio_pic-zooplankton` is a net-tow registry whose biovolumes are
507+
#' still pending from the provider, so contributing `sample` alone is its designed
508+
#' state. The rule is therefore relative — *if a dataset contributes observations,
509+
#' every one of its cruises must* — which needs no allowlist to say so.
510+
#'
511+
#' @param con a DBI connection holding `sample` and `obs`
512+
#' @param obs_tbl name of the observation table (default `"obs"`)
513+
#' @param max_orphan_cruises integer allowance, or a named integer vector keyed by
514+
#' `dataset_key` for a per-dataset ratchet. Use `0` where the correct answer is
515+
#' known to be zero (an ingest asserting its own output); use the current counts
516+
#' as a ratchet at release time so a *new* orphan fails while a documented
517+
#' backlog does not. May only ever be lowered.
518+
#' @param halt logical; `stop()` when the allowance is exceeded (default `TRUE`)
519+
#' @param verbose logical; message the summary
520+
#' @return a data.frame, one row per `dataset_key`, with `cruises`,
521+
#' `cruises_no_obs`, `orphan_samples` and `emits_obs` (invisibly when
522+
#' `verbose = FALSE`)
523+
#' @export
524+
#' @concept check
525+
check_cruise_coverage <- function(con, obs_tbl = "obs",
526+
max_orphan_cruises = 0L,
527+
halt = TRUE, verbose = TRUE) {
528+
present <- DBI::dbListTables(con)
529+
if (!all(c("sample", obs_tbl) %in% present))
530+
stop(glue::glue(
531+
"check_cruise_coverage(): needs `sample` and `{obs_tbl}` in `con`."))
532+
533+
# join through sample_key, never through obs.cruise_key: the denormalized
534+
# cruise_key on obs is NULL for 59,274 swfsc_cufes rows and 14,170 euphausiid
535+
# ones, which would invent orphans that do not exist.
536+
rpt <- DBI::dbGetQuery(con, glue::glue("
537+
WITH s AS (
538+
SELECT dataset_key, cruise_key, sample_key
539+
FROM sample WHERE cruise_key IS NOT NULL),
540+
o AS (SELECT DISTINCT sample_key FROM {obs_tbl}),
541+
j AS (
542+
SELECT s.dataset_key, s.cruise_key, COUNT(*) AS samples,
543+
COUNT(*) FILTER (WHERE o.sample_key IS NOT NULL) AS samples_with_obs
544+
FROM s LEFT JOIN o USING (sample_key) GROUP BY 1, 2)
545+
SELECT dataset_key,
546+
COUNT(*) AS cruises,
547+
COUNT(*) FILTER (WHERE samples_with_obs = 0) AS cruises_no_obs,
548+
COALESCE(SUM(samples) FILTER (WHERE samples_with_obs = 0), 0)
549+
AS orphan_samples
550+
FROM j GROUP BY 1 ORDER BY 1"))
551+
rpt$emits_obs <- rpt$cruises_no_obs < rpt$cruises
552+
# a registry-only dataset has no observations to lose
553+
rpt$cruises_no_obs[!rpt$emits_obs] <- 0L
554+
rpt$orphan_samples[!rpt$emits_obs] <- 0L
555+
556+
allow <- if (is.null(names(max_orphan_cruises))) {
557+
stats::setNames(rep(as.integer(max_orphan_cruises)[1], nrow(rpt)),
558+
rpt$dataset_key)
559+
} else {
560+
a <- stats::setNames(rep(0L, nrow(rpt)), rpt$dataset_key)
561+
a[names(max_orphan_cruises)] <- as.integer(max_orphan_cruises)
562+
a
563+
}
564+
over <- rpt[rpt$cruises_no_obs > allow[rpt$dataset_key], , drop = FALSE]
565+
566+
if (verbose)
567+
message(glue::glue(
568+
"cruise coverage: {sum(rpt$cruises_no_obs)} cruise(s) with samples and no ",
569+
"{obs_tbl} across {sum(rpt$emits_obs)} observing dataset(s) ",
570+
"({sum(!rpt$emits_obs)} registry-only, exempt)"))
571+
572+
if (nrow(over)) {
573+
detail <- paste(sprintf(
574+
" %s: %d of %d cruise(s), %d orphan sample(s) — allowance %d",
575+
over$dataset_key, over$cruises_no_obs, over$cruises,
576+
over$orphan_samples, allow[over$dataset_key]), collapse = "\n")
577+
msg <- paste0(
578+
"cruise(s) carry samples but no ", obs_tbl, ":\n", detail,
579+
"\n The casts survive and every FK still resolves, so nothing else will\n",
580+
" report this. Find where the observations were dropped before releasing;\n",
581+
" raising the allowance to make the check pass republishes the loss.")
582+
if (halt) stop(msg, call. = FALSE) else warning(msg, call. = FALSE)
583+
}
584+
585+
if (verbose) rpt else invisible(rpt)
586+
}

man/check_cruise_coverage.Rd

Lines changed: 58 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# A cruise can leave `obs` without leaving `sample`, and no foreign key notices:
2+
# FK validation runs child -> parent, so a parent with no children is silent.
3+
# These pin the four behaviors that make the check usable as a release gate.
4+
5+
# two datasets: `aa` observes 2 of its 3 cruises, `rr` is a registry (no obs)
6+
make_con <- function(env = parent.frame()) {
7+
con <- get_duckdb_con(":memory:")
8+
withr::defer(DBI::dbDisconnect(con, shutdown = TRUE), envir = env)
9+
10+
smpl <- data.frame(
11+
dataset_key = c(rep("aa", 5), rep("rr", 2)),
12+
cruise_key = c("c1", "c1", "c2", "c3", "c3", "r1", "r2"),
13+
sample_key = c("aa:1", "aa:2", "aa:3", "aa:4", "aa:5", "rr:1", "rr:2"))
14+
# c3's two samples have no obs at all; c1 has one observed and one bare sample
15+
# (the CTD up/down-cast case, which must NOT count as a loss)
16+
obs <- data.frame(
17+
sample_key = c("aa:1", "aa:3", "aa:3"),
18+
measurement_type = c("t", "t", "s"))
19+
20+
DBI::dbWriteTable(con, "sample", smpl)
21+
DBI::dbWriteTable(con, "obs", obs)
22+
con
23+
}
24+
25+
test_that("a cruise with samples and no obs is found, and a half-observed one is not", {
26+
con <- make_con()
27+
rpt <- check_cruise_coverage(con, halt = FALSE, verbose = FALSE) |>
28+
suppressWarnings()
29+
30+
aa <- rpt[rpt$dataset_key == "aa", ]
31+
expect_equal(aa$cruises, 3L)
32+
# c3 only — c1 keeps one bare sample beside an observed one and is fine
33+
expect_equal(aa$cruises_no_obs, 1L)
34+
expect_equal(aa$orphan_samples, 2)
35+
})
36+
37+
test_that("a dataset that emits no observations at all is exempt, not 587 failures", {
38+
con <- make_con()
39+
rpt <- check_cruise_coverage(con, halt = FALSE, verbose = FALSE) |>
40+
suppressWarnings()
41+
42+
rr <- rpt[rpt$dataset_key == "rr", ]
43+
expect_false(rr$emits_obs)
44+
expect_equal(rr$cruises_no_obs, 0L) # sio_pic-zooplankton is a tow registry
45+
expect_true(rpt$emits_obs[rpt$dataset_key == "aa"])
46+
})
47+
48+
test_that("it halts by default and the per-dataset allowance ratchets it", {
49+
con <- make_con()
50+
expect_error(check_cruise_coverage(con, verbose = FALSE), "samples but no obs")
51+
expect_silent(
52+
check_cruise_coverage(con, max_orphan_cruises = c(aa = 1L), verbose = FALSE))
53+
# the ratchet is per dataset: an allowance for another key does not cover `aa`
54+
expect_error(
55+
check_cruise_coverage(con, max_orphan_cruises = c(zz = 9L), verbose = FALSE),
56+
"aa: 1 of 3")
57+
})
58+
59+
test_that("a NULL cruise_key on obs cannot invent an orphan", {
60+
# obs.cruise_key is denormalized and NULL for tens of thousands of real rows,
61+
# so the join must go through sample_key. Drop the column entirely: if the
62+
# implementation ever reaches for it, this fails rather than silently passing.
63+
con <- make_con()
64+
DBI::dbExecute(con, "ALTER TABLE obs ADD COLUMN cruise_key VARCHAR")
65+
rpt <- check_cruise_coverage(con, halt = FALSE, verbose = FALSE) |>
66+
suppressWarnings()
67+
expect_equal(rpt$cruises_no_obs[rpt$dataset_key == "aa"], 1L)
68+
})

tests/testthat/test-in_release.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,9 @@ test_that("a flagged-out ingest is not an [auto] dependency of the release", {
116116
writeLines(c(
117117
"---", "title: Release", "calcofi:", " target_name: release_database",
118118
" workflow_type: release", " dependency:", " - auto",
119-
" output: data/releases", "---"),
119+
# a FILE, not `data/releases`: check_nested_outputs() (3.11.0) rejects a
120+
# directory `output:`, and this fixture predates it
121+
" output: data/releases/_release_stamp.json", "---"),
120122
file.path(dir, "release_database.qmd"))
121123

122124
tl <- build_targets_list(dir, verbose = FALSE)

0 commit comments

Comments
 (0)