From 8a8303fc3e927afae3804cf661b476e5392eb090 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Sun, 18 Oct 2026 20:49:02 -0700 Subject: [PATCH 01/10] Initialize PWF baseline for #348 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WJE2szSCkeV9R3qKhuSwyx --- planning/active/findings.md | 47 ++++++++++++++++++++++++++++++++++++ planning/active/progress.md | 8 ++++++ planning/active/task_plan.md | 42 ++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 planning/active/findings.md create mode 100644 planning/active/progress.md create mode 100644 planning/active/task_plan.md diff --git a/planning/active/findings.md b/planning/active/findings.md new file mode 100644 index 00000000..f4ddb50c --- /dev/null +++ b/planning/active/findings.md @@ -0,0 +1,47 @@ +# Findings — One failed group skips the post-consolidate recompute for every group (#348) + +## Context and design (from plan mode) + + +In `data-raw/study_area_run.sh` the coverage check after consolidate (`:1394-1410`) `exit 1`s on the first run group that has no `.streams` rows. The recompute (`lnk_access(merge = TRUE)`, `:1425-1545`) and the compare come after it, so one failed group costs every other group its recompute. The check also asks only whether a group has rows, not whether they come from this run, so a failed group that still holds an earlier run's rows passes, the recompute covers it, and nothing at that point says its rows are stale. + +What exploration settled: +- **"Finished this run" is already defined** in `data-raw/study_area_hosts.sh` (#347): `host_finished` = `.log` row with `run_uid = RUN_UID AND date_end IS NOT NULL`, with log `done` markers only adding. `.lnk_log_run_fail()` (`R/lnk_log.R:1055`) leaves `date_end` NULL, so a failed group has an *open* row. Since #354, consolidate copies each group's log rows to the dispatcher, so after consolidate the dispatcher's `log` names cypher groups too. +- **Reuse:** `.hosts_query` (sentinel-framed local/remote query; unknown never reads as empty) and the set helpers (`.hosts_union/minus/inter/csv`) in `study_area_hosts.sh`. +- **The pool functions** (`csv_lines`, `run_recompute_pool`, etc., `:1127-1280`) are pulled out of `study_area_run.sh` by `pool_probe.sh`, `recompute_parity.sh` and `recompute_sweep.sh` with `sed`/`awk` pattern matches. They must stay in the driver. Moving them within the file is safe because the matches do not depend on position. +- **A fixture schema test already exists:** `test-schema_consolidate.R` uses `skip_if_no_db()` plus `.lnk_log_create_tables()` on a scratch schema. `test-study_area_hosts.R` runs a bash lib with stubs on PATH, and that is the pattern followed here. +- A cypher group that failed leaves its open log row **on the cypher** (consolidate takes finished groups only). So in `--recompute-only`, reading the expected set from the dispatcher's log cannot see such a group. That is why `--wsgs=` is offered. + +## Design + +New sourced lib **`data-raw/study_area_finish.sh`** (sourced after `study_area_hosts.sh`; uses its helpers). It holds everything from coverage onwards, so the normal run and `--recompute-only` share one path and tests can reach it: + +- `run_coverage `: one strict local `streams` query and one `log` query for `RUN_UID`. Sets: + - `COV_RECOMPUTE` = expected ∩ has rows ∩ (closed log row this run ∪ signals); + - `COV_NOROWS` = no rows at all; + - `COV_STALE` = rows, but not from a build this run finished (an earlier run's, or this run's partial pre-persist); + - `COV_LOST` = finished this run, no rows on the dispatcher (consolidate lost them). + + Prints one line per class. Returns 1 when the database does not answer, and the driver then FATALs as before, because it cannot verify. +- `run_expected_from_log`: the groups with **any** `log` row for `RUN_UID`, open or closed (for `--recompute-only` without `--wsgs=`). +- `run_finish `: coverage, then the log-tables / barrier-views prep, the pool over `COV_RECOMPUTE`, the judge, the compare over `COV_RECOMPUTE`, and the final report. This is today's `:1383-1620`, moved and changed only as follows: + - **Missing groups never exit.** They set `RUN_INCOMPLETE=1`, and the report names each by class. + - **An empty `COV_RECOMPUTE` skips the recompute and the compare**, and the run prints why ("no run group has rows from a build this run finished"). + - The report gives the recovery command: `LNK_RUN_UID= LNK_SCHEMA=… Rscript data-raw/wsg_run_one.R `, then `study_area_run.sh --recompute-only --run-uid= …`. + - It returns the status, and the driver `exit`s with it. + +Driver changes (`study_area_run.sh`): +- Flags: + - `--recompute-only` and `--run-uid=`, both required together. `` must match `^[0-9]{8}_[0-9]{6}-[0-9a-f]{8}$`, the format minted at `:330`. + - Optional `--wsgs=A,B` (recompute-only only). + - `--recompute-only` is refused with `--focal`, `--cy-workspaces`, `--preflight-only`, `--refresh-primitives` or `--auto-install`. + - The focal-count check is skipped in recompute-only, as it already is for preflight-only. +- In recompute-only, `RUN_UID` is the given uid (exported as `LNK_RUN_UID`, so `log_recompute` rows carry it), and the header says `MODE: --recompute-only`. +- The pool function block moves above the pre-flight. Right after the trap, `--recompute-only` checks `pg_isready`, takes its expected set (`--wsgs=`, or else `run_expected_from_log`), and calls `run_finish "$expected" ""`. It skips the pre-flight, dispatch and consolidate, and says why: it reads persisted tables only. Zero expected groups means a non-zero exit naming the run_uid. +- Normal path: `RUN_WSGS` = union of buckets minus every host's SKIP set. Signals = dispatcher `ACC_FINISHED ∪ ACC_DONE_LOG` ∪ each cypher's `CY_BUCKET_DONE`. Then `run_finish "$RUN_WSGS" "$signals"`. + + +## Errors Encountered + +| Error | Resolution | +|-------|------------| diff --git a/planning/active/progress.md b/planning/active/progress.md new file mode 100644 index 00000000..d9d1b987 --- /dev/null +++ b/planning/active/progress.md @@ -0,0 +1,8 @@ +# Progress — One failed group skips the post-consolidate recompute for every group (#348) + +## Session 2026-10-18 + +- Plan-mode exploration — phases approved by user ("Go all phases to PR") +- Created branch `348-one-failed-group-skips-the-post-consolid` off main +- Scaffolded PWF baseline from issue #348 with approved phases +- Next: start Phase 1 diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md new file mode 100644 index 00000000..509658cf --- /dev/null +++ b/planning/active/task_plan.md @@ -0,0 +1,42 @@ +# Task: One failed group skips the post-consolidate recompute for every group (#348) + +In `study_area_run.sh`, one failed group skips the post-consolidate recompute for every group in the run. The coverage check after consolidate asks whether every run group has rows in `.streams`, and `exit 1`s on the first miss. The recompute (`lnk_access(merge = TRUE)`, the correctness guarantee for cross-group access) comes after it. So when one group fails to model, the groups that did model lose their recompute and the compare. The gate also tests for rows, not rows from this run, so a failed group that still holds an earlier run's rows passes it unremarked. + +Design: `/Users/airvine/.claude/plans/quiet-hopping-umbrella.md` (approved 2026-10-18), summarised in findings.md. + +## Phase 1: Tests first (fixture schema, failing) +- [ ] Move `hosts_bash()` to `tests/testthat/helper-bash.R`, so both shell-lib test files share it; `test-study_area_hosts.R` still passes +- [ ] `tests/testthat/test-study_area_finish.R`: build a scratch schema with `skip_if_no_db()` + `.lnk_log_create_tables()` + a minimal `streams`. Groups: + - AAAA, BBBB closed this run, with rows; + - CCCC open row this run, no rows; + - DDDD rows plus another run's row; + - EEEE closed this run, no rows. +- [ ] Test: `run_coverage` splits into recompute / norows / stale / lost exactly +- [ ] Test: a `done` signal admits a group with rows and no closed row +- [ ] Test: a missing `streams` table is unknown (non-zero), never "all missing" +- [ ] Test: `run_expected_from_log` returns AAAA, BBBB, CCCC, EEEE +- [ ] Test (acceptance 1): `run_finish` with one missing group calls the stubbed pool and compare with exactly the recompute set, returns 1, and names the missing group +- [ ] Test (acceptance 2): zero groups with rows: the pool and compare are not called, it returns 1, and it says why +- [ ] Test: the driver refuses `--run-uid` alone, `--recompute-only` alone, a malformed uid, and `--recompute-only --focal=…` (all exit before any Rscript) + +## Phase 2: `data-raw/study_area_finish.sh` + driver wiring +- [ ] Write the lib (`run_coverage`, `run_expected_from_log`, `run_finish`) and move the tail into `run_finish` with the changes above +- [ ] Driver: new flags and validation, move the pool function block above the pre-flight, add the `--recompute-only` branch, normal path calls `run_finish`; update the header usage and "Post-conditions" comment +- [ ] `bash -n` both scripts; Phase 1 tests pass; `pool_probe.sh data-raw/study_area_run.sh` still passes (the extraction still works) + +## Phase 3: Docs +- [ ] RUNBOOK §8d: the coverage classes, recompute over this run's finished groups, `--recompute-only --run-uid` recovery (replace the "coverage FATAL" wording at ~`:1291`) +- [ ] `CLAUDE.md` #246 status fact on #348: state the new behaviour +- [ ] `research/study_area_run.md`: one line where the run's recovery is described, if it names the coverage exit + +## Phase 4: Verification +- [ ] Mutation: restore `exit 1` on missing in a copy of the lib (`LNK_TEST_FINISH_LIB`), and confirm acceptance test 1 goes red +- [ ] Live, read-only: `run_coverage` against `fresh_default` for run_uid `20261017_004619-3f4c713b` with the 241 groups gives 241 recompute and 0 missing (no recompute run) +- [ ] `devtools::test()` full suite, `lintr` on the new test file + +## Validation + +- [ ] Tests pass +- [ ] `/code-check` clean (each commit, or once over the branch with `/code-check branch`) +- [ ] PWF checkboxes match landed work +- [ ] `/planning-archive` on completion From d122584bdd2d47feac577b2ff5e6d0ca796d0a28 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Sun, 18 Oct 2026 20:55:55 -0700 Subject: [PATCH 02/10] Test coverage, recompute set and recompute-only flags on a fixture schema (#348) Scratch persist schema on the local fwapg: groups finished this run, one failed (open log row), one with an earlier run's rows, one lost at consolidate. The driver's pool functions run for real; Rscript is a stub that records calls. hosts_bash() moves to a shared helper. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WJE2szSCkeV9R3qKhuSwyx --- planning/active/task_plan.md | 18 +- tests/testthat/helper-bash.R | 20 ++ tests/testthat/test-study_area_finish.R | 252 ++++++++++++++++++++++++ tests/testthat/test-study_area_hosts.R | 18 -- 4 files changed, 281 insertions(+), 27 deletions(-) create mode 100644 tests/testthat/helper-bash.R create mode 100644 tests/testthat/test-study_area_finish.R diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index 509658cf..92082ab7 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -5,19 +5,19 @@ In `study_area_run.sh`, one failed group skips the post-consolidate recompute fo Design: `/Users/airvine/.claude/plans/quiet-hopping-umbrella.md` (approved 2026-10-18), summarised in findings.md. ## Phase 1: Tests first (fixture schema, failing) -- [ ] Move `hosts_bash()` to `tests/testthat/helper-bash.R`, so both shell-lib test files share it; `test-study_area_hosts.R` still passes -- [ ] `tests/testthat/test-study_area_finish.R`: build a scratch schema with `skip_if_no_db()` + `.lnk_log_create_tables()` + a minimal `streams`. Groups: +- [x] Move `hosts_bash()` to `tests/testthat/helper-bash.R`, so both shell-lib test files share it; `test-study_area_hosts.R` still passes +- [x] `tests/testthat/test-study_area_finish.R`: build a scratch schema with `skip_if_no_db()` + `.lnk_log_create_tables()` + a minimal `streams`. Groups: - AAAA, BBBB closed this run, with rows; - CCCC open row this run, no rows; - DDDD rows plus another run's row; - EEEE closed this run, no rows. -- [ ] Test: `run_coverage` splits into recompute / norows / stale / lost exactly -- [ ] Test: a `done` signal admits a group with rows and no closed row -- [ ] Test: a missing `streams` table is unknown (non-zero), never "all missing" -- [ ] Test: `run_expected_from_log` returns AAAA, BBBB, CCCC, EEEE -- [ ] Test (acceptance 1): `run_finish` with one missing group calls the stubbed pool and compare with exactly the recompute set, returns 1, and names the missing group -- [ ] Test (acceptance 2): zero groups with rows: the pool and compare are not called, it returns 1, and it says why -- [ ] Test: the driver refuses `--run-uid` alone, `--recompute-only` alone, a malformed uid, and `--recompute-only --focal=…` (all exit before any Rscript) +- [x] Test: `run_coverage` splits into recompute / norows / stale / lost exactly +- [x] Test: a `done` signal admits a group with rows and no closed row +- [x] Test: a missing `streams` table is unknown (non-zero), never "all missing" +- [x] Test: `run_expected_from_log` returns AAAA, BBBB, CCCC, EEEE +- [x] Test (acceptance 1): `run_finish` with one missing group calls the stubbed pool and compare with exactly the recompute set, returns 1, and names the missing group +- [x] Test (acceptance 2): zero groups with rows: the pool and compare are not called, it returns 1, and it says why +- [x] Test: the driver refuses `--run-uid` alone, `--recompute-only` alone, a malformed uid, and `--recompute-only --focal=…` (all exit before any Rscript) ## Phase 2: `data-raw/study_area_finish.sh` + driver wiring - [ ] Write the lib (`run_coverage`, `run_expected_from_log`, `run_finish`) and move the tail into `run_finish` with the changes above diff --git a/tests/testthat/helper-bash.R b/tests/testthat/helper-bash.R new file mode 100644 index 00000000..1a2cbce7 --- /dev/null +++ b/tests/testthat/helper-bash.R @@ -0,0 +1,20 @@ +# Shared by the tests that run data-raw/ bash libs (test-study_area_hosts.R, +# test-study_area_finish.R). + +# The libs need bash >= 4 (assoc arrays); macOS /bin/bash is 3.2. +hosts_bash <- function() { + cands <- unique(c( + "/opt/homebrew/bin/bash", "/usr/local/bin/bash", + unname(Sys.which("bash")) + )) + for (b in cands[nzchar(cands) & file.exists(cands)]) { + v <- suppressWarnings(system2(b, c("-c", shQuote("echo ${BASH_VERSINFO[0]}")), + stdout = TRUE, stderr = FALSE + )) + n <- suppressWarnings(as.integer(v)) + if (length(n) == 1L && !is.na(n) && n >= 4L) { + return(b) + } + } + "" +} diff --git a/tests/testthat/test-study_area_finish.R b/tests/testthat/test-study_area_finish.R new file mode 100644 index 00000000..52934441 --- /dev/null +++ b/tests/testthat/test-study_area_finish.R @@ -0,0 +1,252 @@ +# data-raw/study_area_finish.sh: which groups have rows from this run, and the +# recompute and compare over those (link#348). The lib is bash, so each test +# runs it in a bash process against a scratch schema on the local docker +# fwapg, with the driver's own pool functions and an Rscript stub on PATH that +# records its calls instead of running anything. +# +# data-raw/ is .Rbuildignore'd, so under R CMD check the scripts are absent and +# these skip; devtools::test() from the source tree runs them. + +# LNK_TEST_FINISH_LIB points the tests at a mutant copy (proving they fail). +finish_lib <- function() { + p <- Sys.getenv( + "LNK_TEST_FINISH_LIB", + testthat::test_path("..", "..", "data-raw", "study_area_finish.sh") + ) + normalizePath(p, mustWork = FALSE) +} +finish_hosts_lib <- function() { + normalizePath(testthat::test_path("..", "..", "data-raw", "study_area_hosts.sh"), + mustWork = FALSE) +} +finish_driver <- function() { + normalizePath(testthat::test_path("..", "..", "data-raw", "study_area_run.sh"), + mustWork = FALSE) +} + +skip_if_no_finish_lib <- function() { + testthat::skip_if_not(file.exists(finish_lib()), "data-raw/study_area_finish.sh absent") + if (!nzchar(hosts_bash())) { + testthat::fail("no bash >= 4 found; the study_area_finish.sh tests cannot run") + } +} + +finish_uid <- "20261018_000000-0000abcd" + +# A scratch persist schema: the log tables, and a streams table holding only +# what coverage reads. AAAA and BBBB finished this run; CCCC opened a log row +# this run and failed; DDDD holds an earlier run's rows; EEEE finished this +# run and its rows are gone (lost at consolidate). +finish_fixture <- function(conn, env = parent.frame(), streams = TRUE) { + psql <- Sys.which("psql") + testthat::skip_if(!nzchar(psql), "psql not on PATH") + info <- DBI::dbGetInfo(conn) + testthat::skip_if(!identical(as.character(info$port), "5432") || + !info$host %in% c("localhost", "127.0.0.1"), + "not the local docker fwapg the lib reads") + sc <- paste0("lnk_test_finish_", Sys.getpid(), if (streams) "" else "_nostreams") + DBI::dbExecute(conn, "SET client_min_messages TO warning") + DBI::dbExecute(conn, sprintf("DROP SCHEMA IF EXISTS %s CASCADE", sc)) + link:::.lnk_log_create_tables(conn, sc) + withr::defer( + DBI::dbExecute(conn, sprintf("DROP SCHEMA IF EXISTS %s CASCADE", sc)), + envir = env + ) + DBI::dbExecute(conn, sprintf( + "INSERT INTO %s.log (run_id, watershed_group_code, date_start, date_end, run_uid) + VALUES ('a', 'AAAA', '2026-10-18', '2026-10-18', '%s'), + ('b', 'BBBB', '2026-10-18', '2026-10-18', '%s'), + ('c', 'CCCC', '2026-10-18', NULL, '%s'), + ('d', 'DDDD', '2026-08-01', '2026-08-01', '20260801_000000-00000000'), + ('e', 'EEEE', '2026-10-18', '2026-10-18', '%s')", + sc, finish_uid, finish_uid, finish_uid, finish_uid)) + if (streams) { + DBI::dbExecute(conn, sprintf( + "CREATE TABLE %s.streams (watershed_group_code varchar(4) NOT NULL)", sc)) + DBI::dbExecute(conn, sprintf( + "INSERT INTO %s.streams VALUES ('AAAA'), ('AAAA'), ('BBBB'), ('DDDD'), ('FFFF')", + sc)) + } + sc +} + +# A scratch world: an Rscript stub recording every call, and the dirs the +# driver writes logs and per-job files into. +finish_world <- function(env = parent.frame()) { + d <- withr::local_tempdir(.local_envir = env) + for (sub in c("bin", "logs")) dir.create(file.path(d, sub)) + p <- file.path(d, "bin", "Rscript") + writeLines(c("#!/bin/sh", sprintf('echo "$*" >> "%s/rscript_calls"', d), "exit 0"), p) + Sys.chmod(p, "755") + d +} + +rscript_calls <- function(world) { + p <- file.path(world, "rscript_calls") + if (file.exists(p)) readLines(p) else character(0) +} + +# Run `body` with both libs sourced, the driver's pool functions extracted +# from it as pool_probe.sh does, and the driver's globals set. +run_finish_sh <- function(world, schema, body) { + script <- file.path(world, "run.sh") + drv <- shQuote(finish_driver()) + writeLines(c( + "set -euo pipefail", + sprintf("source %s", shQuote(finish_hosts_lib())), + sprintf("source %s", shQuote(finish_lib())), + sprintf("eval \"$(sed -n '/^csv_lines() {/p' %s)\"", drv), + sprintf("eval \"$(sed -n '/^csv_count() {/p' %s)\"", drv), + sprintf("eval \"$(awk '/^recompute_one\\(\\)/,/^}/' %s)\"", drv), + sprintf("eval \"$(awk '/^recompute_order\\(\\)/,/^}/' %s)\"", drv), + sprintf("eval \"$(awk '/^run_recompute_pool\\(\\)/,/^}/' %s)\"", drv), + sprintf("SCHEMA=%s; RUN_UID=%s; CONFIG=default; TS=T0", schema, finish_uid), + sprintf("LOG_DIR=%s; REPO_ROOT=%s; RECOMPUTE_JOBS=2", + shQuote(file.path(world, "logs")), shQuote(world)), + "RECOMPUTE_FAIL=0; RECOMPUTE_FAIL_STAGE=''; RUN_INCOMPLETE=0", + body + ), script) + out <- withr::with_envvar( + c(PATH = paste(file.path(world, "bin"), Sys.getenv("PATH"), sep = ":")), + suppressWarnings(system2(hosts_bash(), script, stdout = TRUE, stderr = TRUE)) + ) + list(out = paste(out, collapse = "\n"), status = attr(out, "status") %||% 0L) +} + +# The value a `name=` line printed by a test body carries. +printed <- function(r, name) { + m <- regmatches(r$out, regexpr(sprintf("%s=[A-Z,]*", name), r$out)) + if (length(m) == 0L) NA_character_ else sub("^[^=]*=", "", m) +} + +cov_body <- function(expected, signals = "") { + c( + sprintf("run_coverage %s %s", shQuote(expected), shQuote(signals)), + paste0("echo \"STATE=$COV_STATE\"; echo \"REC=$COV_RECOMPUTE\"; ", + "echo \"NOROWS=$COV_NOROWS\"; echo \"STALE=$COV_STALE\"; ", + "echo \"LOST=$COV_LOST\"; echo \"MISSING=$COV_MISSING\"") + ) +} + +test_that("coverage splits the run's groups by what this run built", { + skip_if_no_finish_lib() + conn <- skip_if_no_db() + sc <- finish_fixture(conn) + w <- finish_world() + r <- run_finish_sh(w, sc, cov_body("AAAA,BBBB,CCCC,DDDD,EEEE,GGGG")) + expect_equal(r$status, 0L, info = r$out) + expect_identical(printed(r, "REC"), "AAAA,BBBB") + expect_identical(printed(r, "NOROWS"), "CCCC,GGGG") + expect_identical(printed(r, "STALE"), "DDDD") + expect_identical(printed(r, "LOST"), "EEEE") + expect_identical(printed(r, "MISSING"), "CCCC,DDDD,EEEE,GGGG") + # FFFF has rows and is not a run group: it is in no class. + expect_false(grepl("FFFF", r$out)) +}) + +test_that("a done signal admits a group with rows and no closed log row", { + skip_if_no_finish_lib() + conn <- skip_if_no_db() + sc <- finish_fixture(conn) + w <- finish_world() + r <- run_finish_sh(w, sc, cov_body("AAAA,DDDD", "DDDD")) + expect_equal(r$status, 0L, info = r$out) + expect_identical(printed(r, "REC"), "AAAA,DDDD") + expect_identical(printed(r, "MISSING"), "") +}) + +test_that("a persist with no streams table is unknown, never 'nothing has rows'", { + skip_if_no_finish_lib() + conn <- skip_if_no_db() + sc <- finish_fixture(conn, streams = FALSE) + w <- finish_world() + r <- run_finish_sh(w, sc, c( + "rc=0; run_coverage AAAA,CCCC '' || rc=$?", + "echo \"RC=$rc STATE=$COV_STATE\"; echo \"NOROWS=$COV_NOROWS\"" + )) + expect_match(r$out, "RC=1 STATE=unknown", fixed = TRUE) + expect_identical(printed(r, "NOROWS"), "") +}) + +test_that("--recompute-only's expected set is every group with a log row for the run", { + skip_if_no_finish_lib() + conn <- skip_if_no_db() + sc <- finish_fixture(conn) + w <- finish_world() + r <- run_finish_sh(w, sc, "echo \"EXP=$(run_expected_from_log)\"") + expect_equal(r$status, 0L, info = r$out) + expect_identical(printed(r, "EXP"), "AAAA,BBBB,CCCC,EEEE") +}) + +test_that("one missing group: the rest are recomputed and compared, and the run exits non-zero naming it", { + skip_if_no_finish_lib() + conn <- skip_if_no_db() + sc <- finish_fixture(conn) + w <- finish_world() + r <- run_finish_sh(w, sc, "run_finish AAAA,BBBB,CCCC ''") + expect_equal(r$status, 1L, info = r$out) + calls <- rscript_calls(w) + recomputed <- sort(sub( + "^data-raw/wsg_recompute_one\\.R ([A-Z]{4}) default$", "\\1", + grep("wsg_recompute_one", calls, value = TRUE) + )) + expect_identical(recomputed, c("AAAA", "BBBB")) + compare <- grep("study_area_compare", calls, value = TRUE) + expect_length(compare, 1L) + expect_match(compare, " AAAA,BBBB default$") + expect_match(r$out, "were NOT recomputed or compared: CCCC", fixed = TRUE) + expect_match(r$out, "--recompute-only --run-uid=20261018_000000-0000abcd", fixed = TRUE) + expect_match(r$out, "study_area_run INCOMPLETE", fixed = TRUE) +}) + +test_that("no group with rows from this run: no recompute, no compare, and it says why", { + skip_if_no_finish_lib() + conn <- skip_if_no_db() + sc <- finish_fixture(conn) + w <- finish_world() + r <- run_finish_sh(w, sc, "run_finish CCCC,DDDD ''") + expect_equal(r$status, 1L, info = r$out) + calls <- rscript_calls(w) + expect_length(grep("wsg_recompute_one|study_area_compare|barriers_views_build", calls), 0L) + expect_match(r$out, "No run group has rows from a build this run finished", fixed = TRUE) + expect_match(r$out, "no rows: CCCC", fixed = TRUE) + expect_match(r$out, "an earlier run's or partial rows: DDDD", fixed = TRUE) +}) + +test_that("a complete run recomputes every group and exits 0", { + skip_if_no_finish_lib() + conn <- skip_if_no_db() + sc <- finish_fixture(conn) + w <- finish_world() + r <- run_finish_sh(w, sc, "run_finish AAAA,BBBB ''") + expect_equal(r$status, 0L, info = r$out) + expect_match(r$out, "study_area_run done", fixed = TRUE) + expect_length(grep("wsg_recompute_one", rscript_calls(w)), 2L) +}) + +test_that("the driver refuses --run-uid / --wsgs without --recompute-only, and dispatch flags with it", { + skip_if_no_finish_lib() + run_drv <- function(...) { + out <- suppressWarnings(system2(hosts_bash(), c(shQuote(finish_driver()), ...), + stdout = TRUE, stderr = TRUE)) + list(out = paste(out, collapse = "\n"), status = attr(out, "status") %||% 0L) + } + r <- run_drv(paste0("--run-uid=", finish_uid)) + expect_equal(r$status, 1L) + expect_match(r$out, "--run-uid= is for --recompute-only", fixed = TRUE) + r <- run_drv("--wsgs=AAAA") + expect_equal(r$status, 1L) + expect_match(r$out, "--wsgs= is for --recompute-only", fixed = TRUE) + r <- run_drv("--recompute-only") + expect_equal(r$status, 1L) + expect_match(r$out, "needs --run-uid=", fixed = TRUE) + r <- run_drv("--recompute-only", "--run-uid=20261018-bad") + expect_equal(r$status, 1L) + expect_match(r$out, "--run-uid= must look like", fixed = TRUE) + r <- run_drv("--recompute-only", paste0("--run-uid=", finish_uid), "--focal=ADMS") + expect_equal(r$status, 1L) + expect_match(r$out, "--recompute-only takes no --focal", fixed = TRUE) + r <- run_drv("--recompute-only", paste0("--run-uid=", finish_uid), "--wsgs=ADMS,bulk") + expect_equal(r$status, 1L) + expect_match(r$out, "--wsgs= takes comma-separated", fixed = TRUE) +}) diff --git a/tests/testthat/test-study_area_hosts.R b/tests/testthat/test-study_area_hosts.R index 2aaaa0f1..6e8c83db 100644 --- a/tests/testthat/test-study_area_hosts.R +++ b/tests/testthat/test-study_area_hosts.R @@ -15,24 +15,6 @@ hosts_lib <- function() { normalizePath(p, mustWork = FALSE) } -# The lib needs bash >= 4 (assoc arrays); macOS /bin/bash is 3.2. -hosts_bash <- function() { - cands <- unique(c( - "/opt/homebrew/bin/bash", "/usr/local/bin/bash", - unname(Sys.which("bash")) - )) - for (b in cands[nzchar(cands) & file.exists(cands)]) { - v <- suppressWarnings(system2(b, c("-c", shQuote("echo ${BASH_VERSINFO[0]}")), - stdout = TRUE, stderr = FALSE - )) - n <- suppressWarnings(as.integer(v)) - if (length(n) == 1L && !is.na(n) && n >= 4L) { - return(b) - } - } - "" -} - # Absent lib (R CMD check, installed tests) skips. A present lib with no # bash >= 4 FAILS: the driver needs bash >= 4 too, and nothing in CI runs # these, so a silent skip would read as coverage. From abe43c9a62c25115c427760ffebb3cb6d19dd5f6 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Sun, 18 Oct 2026 20:55:55 -0700 Subject: [PATCH 03/10] Recompute the groups a run built when others fail; add --recompute-only (#348) Coverage no longer exits on the first run group without rows. It splits the run's groups by what the dispatcher's database says about this run_uid (rows from a finished build; no rows; an earlier run's or partial rows; lost at consolidate), the recompute and compare run over the first class, and the rest are named at the end with a non-zero exit. With no group in the first class neither runs, and the report says why. --recompute-only --run-uid= [--wsgs=] finishes a run whose gaps were re-modelled by hand under the same run_uid. The tail lives in data-raw/study_area_finish.sh so both paths share it and tests reach it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WJE2szSCkeV9R3qKhuSwyx --- data-raw/study_area_finish.sh | 348 +++++++++++++++++ data-raw/study_area_run.sh | 689 ++++++++++++++-------------------- planning/active/progress.md | 5 + planning/active/task_plan.md | 6 +- 4 files changed, 634 insertions(+), 414 deletions(-) create mode 100644 data-raw/study_area_finish.sh diff --git a/data-raw/study_area_finish.sh b/data-raw/study_area_finish.sh new file mode 100644 index 00000000..925c4d16 --- /dev/null +++ b/data-raw/study_area_finish.sh @@ -0,0 +1,348 @@ +# data-raw/study_area_finish.sh — finishing a run once its groups are on the +# dispatcher: which groups have rows from this run, the post-consolidate +# recompute over those, the compare, and the final report (link#348). Sourced +# by study_area_run.sh after study_area_hosts.sh, and by +# tests/testthat/test-study_area_finish.R. Sourcing defines functions only. +# +# The rule (link#348): one failed group does not cost the others their +# recompute. Coverage used to `exit 1` on the first run group with no +# .streams rows, before the recompute, so one failure left every other +# group's access computed against an incomplete barrier set. It also asked +# only "are there rows", so a failed group holding an EARLIER run's rows +# passed and was recomputed as if it were this run's. +# +# Now coverage splits the run's groups by what the dispatcher's database says +# about THIS run_uid, and the recompute and compare run over the groups whose +# rows a build of this run wrote. The rest are named at the end and the run +# exits non-zero; their rows, earlier or partial, are left as they are. +# +# Globals read: SCHEMA RUN_UID CONFIG TS LOG_DIR REPO_ROOT RECOMPUTE_JOBS +# RUN_INCOMPLETE RECOMPUTE_FAIL RECOMPUTE_FAIL_STAGE (initialised by the +# driver); with a default: KEPT_CAUSE_N RECOMPUTE_ONLY +# Functions read: .hosts_query host_finished .hosts_* (study_area_hosts.sh); +# csv_count recompute_order run_recompute_pool (study_area_run.sh) +# Globals written: COV_STATE COV_RECOMPUTE COV_NOROWS COV_STALE COV_LOST +# COV_MISSING RUN_WSGS ALL_WSGS COMPARE_CSV RUN_INCOMPLETE RECOMPUTE_FAIL +# RECOMPUTE_FAIL_STAGE RECOMPUTE_RAN +# +# Needs bash >= 4, as the driver does. + +# run_coverage [signals csv]: split the expected groups by what +# the dispatcher's database holds for this run. +# +# COV_RECOMPUTE rows in .streams, and finished this run: a log row +# for RUN_UID with date_end set, or a signal the driver +# passes (a host's `done` marker, a consolidated bucket). +# The log's date_end is a soft UPDATE that can fail +# (.lnk_log_run_finish), so signals only ever add. +# COV_NOROWS no rows at all. +# COV_STALE rows, but no finish signal of this run: an earlier run's +# rows, or this run's pre-persist of a build that then failed. +# COV_LOST finished this run, no rows here: consolidate lost them. +# +# Returns 1, with COV_STATE=unknown and every set empty, when the database +# does not answer or has no .streams (strict, as the hosts are): an +# unknown must never read as "nothing has rows". +run_coverage() { + local expected signals in rows closed fin + expected=$(.hosts_union "${1:-}") + signals="${2:-}" + COV_STATE=unknown + COV_RECOMPUTE=""; COV_NOROWS=""; COV_STALE=""; COV_LOST=""; COV_MISSING="" + in=$(.hosts_lines "$expected" | sed "s/.*/'&'/" | paste -sd, -) + rows=$(.hosts_query local streams "watershed_group_code IN (${in:-NULL})" 1) \ + || return 1 + closed=$(host_finished local) || return 1 + COV_STATE=known + rows=$(printf '%s\n' "$rows" | .hosts_csv) + fin=$(.hosts_union "$(printf '%s\n' "$closed" | .hosts_csv)" "$signals") + fin=$(.hosts_inter "$fin" "$expected") + COV_RECOMPUTE=$(.hosts_inter "$rows" "$fin") + COV_LOST=$(.hosts_minus "$fin" "$rows") + COV_STALE=$(.hosts_minus "$rows" "$fin") + COV_NOROWS=$(.hosts_minus "$(.hosts_minus "$expected" "$rows")" "$COV_LOST") + COV_MISSING=$(.hosts_union "$COV_NOROWS" "$COV_STALE" "$COV_LOST") + echo " $(.hosts_n "$COV_RECOMPUTE")/$(.hosts_n "$expected") run WSGs have rows from a build this run finished" + [ -z "$COV_NOROWS" ] || echo " ✗ no rows in $SCHEMA.streams: $COV_NOROWS" + [ -z "$COV_STALE" ] || echo " ✗ rows NOT from a build this run finished (an earlier run's, or this run's partial build): $COV_STALE" + [ -z "$COV_LOST" ] || echo " ✗ finished this run, but no rows in $SCHEMA.streams (lost at consolidate): $COV_LOST" + return 0 +} + +# run_expected_from_log: the groups with ANY .log row for RUN_UID on +# the dispatcher, open or closed, as CSV. The --recompute-only expected set +# when --wsgs= is not given. A group that failed on a cloud host and was never +# re-run here has no row here (consolidate copies finished groups only), so it +# cannot be seen this way. Strict: a missing log table returns 1. +run_expected_from_log() { + local out + out=$(.hosts_query local log "run_uid = '${RUN_UID}'" 1) || return 1 + printf '%s\n' "$out" | .hosts_csv +} + +# run_finish [signals csv]: coverage, recompute, compare and the +# final report. EXITS the shell on an incomplete or failed run, as the driver +# always has, and returns 0 only for a complete one. Call it bare: under +# `set -e`, a function called in an `if` or `||` runs with errexit off. +run_finish() { + RUN_WSGS=$(.hosts_union "${1:-}") + local signals="${2:-}" + COMPARE_CSV="$LOG_DIR/${TS}_compare.csv" + + # --- coverage post-condition (link#246, link#348) ------------------------- + # Detection rather than prevention: this fires whatever the cause. In a full + # run it sits AFTER the burn so a failure cannot leak spend, and BEFORE the + # recompute so the recompute covers only what this run built. + # + # Necessary because schema_consolidate DELETEs the destination bucket from + # every persist table (its step 3.5) and then COPYs (step 4). A source that + # produced nothing therefore removes the destination's prior rows for those + # WSGs and still returns ok = TRUE (COV_LOST). The log tables keep their + # rows (#354), so rows are read from streams and finishing from the log. + # + # A gap no longer exits here (link#348): it would take every other group's + # recompute with it. It is named at the end, with an exit status to match. + echo "=== verify WSG coverage in $SCHEMA (run_uid $RUN_UID) ===" + if ! run_coverage "$RUN_WSGS" "$signals"; then + echo "FATAL: could not verify WSG coverage in $SCHEMA"; exit 1 + fi + if [ -n "$COV_MISSING" ]; then + RUN_INCOMPLETE=1 + echo " NOTE: recomputing only the $(.hosts_n "$COV_RECOMPUTE") WSGs above; the rest keep" + echo " whatever rows they hold, and the run will exit non-zero." + fi + ALL_WSGS="$COV_RECOMPUTE" + + if [ -z "$ALL_WSGS" ]; then + echo "=== post-consolidate recompute: SKIPPED ===" + echo " No run group has rows from a build this run finished, so there is" + echo " nothing the recompute could settle and nothing valid to compare." + else + run_finish_recompute + run_finish_compare + fi + run_finish_report +} + +# --- post-consolidate recompute: settle cross-WSG access (link#205) --- +# Drainage-closed + DS-first per-host is NOT sufficient: a WSG's downstream +# barriers can be cross-bucket or arrive late in DS-first order, so its access +# (hence token1/token2) is computed against an incomplete barrier set. +# Caught 2026-05-25: FINA 75% / PARA 69% per-host -> both 99% only after +# re-modelling on the full consolidated barrier set. The recompute is the +# correctness guarantee REGARDLESS of bucketing. We use lnk_access(merge=TRUE) +# — the cheap access-only recompute that reuses the persisted streams / +# habitat / barriers / barrier_overrides (link#205, ~10 s/WSG vs ~1.5 min for +# a full pipeline rebuild). Because it is cheap, we recompute ALL run WSGs +# this run built unconditionally rather than threshold-filtering by parity — +# bucketing is now a speed knob, not a correctness lever. +# +# Parallel since link#250. Two things previously made the serial loop +# mandatory, and both are now gone: +# +# 1. lnk_access() rebuilt the SCHEMA-scoped barrier views on every call -- +# 38 DDL statements against names every sibling WSG reads. CREATE OR +# REPLACE VIEW takes an AccessExclusiveLock, and a QUEUED exclusive +# request blocks every AccessShareLock behind it, so one job's DDL would +# stall every sibling's read until lock_timeout killed someone. The build +# is now hoisted below and each job runs LNK_VIEWS_PREBUILT=1. Everything +# else a job touches is WSG-scoped (zz_lnk_streams_, +# zz_lnk_access_scratch_, zz_lnk_mc_scratch_) or row-scoped +# (UPDATE ... WHERE watershed_group_code; DELETE+INSERT in a transaction). +# +# 2. A failed WSG was invisible. `|| echo` sat INSIDE the loop, so the +# subshell always exited 0 and the success line below was unconditional; +# RUN_INCOMPLETE is assigned BEFORE this block and nothing after raised +# it. A run in which every recompute failed exited 0 and wrote a compare +# CSV. The .rc files + lnk_fanout_judge() are what fix that. +run_finish_recompute() { + RECOMPUTE_LOG="$LOG_DIR/${TS}_recompute.log" + RC_DIR="$LOG_DIR/${TS}_recompute.d" + RC_TSV="$LOG_DIR/${TS}_recompute.tsv" + N_RECOMPUTE=$(csv_count "$ALL_WSGS") + + echo "=== post-consolidate recompute (lnk_access, ${N_RECOMPUTE} WSGs, -j${RECOMPUTE_JOBS}) ===" + + # Ensure the log tables exist ONCE, single-threaded, before any job starts + # (link#262). wsg_recompute_one.R deliberately refuses to run DDL — it runs + # inside this pool, and schema DDL belongs at init, not in N concurrent jobs. + # That leaves one edge case with the worst possible timing: if every focal WSG + # on the dispatcher species-skips (wsg_run_one.R exits 0 without calling + # lnk_pipeline_run), nothing ever creates log_recompute, and the pool would + # then hard-stop on all N WSGs — at the end of a paid multi-host run, after + # consolidate. One idempotent call here removes the class for nothing. + if ( cd "$REPO_ROOT" && LNK_LOAD=loadall Rscript -e ' +suppressPackageStartupMessages(pkgload::load_all(quiet = TRUE)) +cfg <- lnk_config(commandArgs(TRUE)[1]) +s <- Sys.getenv("LNK_SCHEMA"); if (nzchar(s)) cfg$pipeline$schema <- s +conn <- lnk_db_conn(dbname = "fwapg", host = "localhost", port = 5432L, + user = "postgres", password = "postgres") +on.exit(try(DBI::dbDisconnect(conn), silent = TRUE), add = TRUE) +link:::.lnk_log_create_tables(conn, cfg$pipeline$schema) +' "$CONFIG" ) > "$LOG_DIR/${TS}_recompute_logtables.log" 2>&1; then + echo " ✓ log tables present in $SCHEMA" + else + echo " ✗ could not ensure log tables; see $LOG_DIR/${TS}_recompute_logtables.log" + echo " the recompute would fail per-WSG on a missing log_recompute" + RECOMPUTE_FAIL=1 + # Its OWN stage token. Reusing "views" made the final gate print "the barrier + # views could not be built" and point at ${TS}_recompute_views.log -- a file + # the views step never wrote, because it is gated on RECOMPUTE_FAIL=0 and + # never ran. Two prep steps, two names, two pieces of evidence. + RECOMPUTE_FAIL_STAGE="logtables" + fi + + # Build the barrier views ONCE, single-threaded, before any job starts. + # Nested rather than `[ "$RECOMPUTE_FAIL" = "0" ] && ( build )`: with the + # short-circuit form, an already-failed log-table step would fall into this + # else and report "barrier view build failed", naming a log file that was + # never written. Two prep steps, two diagnoses. + if [ "$RECOMPUTE_FAIL" = "0" ]; then + if ( cd "$REPO_ROOT" && LNK_LOAD=loadall \ + Rscript data-raw/barriers_views_build.R "$CONFIG" ) \ + > "$LOG_DIR/${TS}_recompute_views.log" 2>&1; then + echo " ✓ barrier views built once for $SCHEMA" + else + # Deliberately NOT falling back to per-WSG builds. At -j>1 that is exactly + # the race the hoist removes, and it would turn one loud pre-build failure + # into N quiet lock_timeouts attributed to individual WSGs. + echo " ✗ barrier view build failed; see $LOG_DIR/${TS}_recompute_views.log" + echo " skipping the recompute — the run will exit non-zero" + RECOMPUTE_FAIL=1 + # Which stage failed decides which evidence exists. The pool never ran, so + # RECOMPUTE_LOG and RC_TSV were never created -- naming them at the final + # gate would send the operator to two absent files. + RECOMPUTE_FAIL_STAGE="views" + fi + fi + + if [ "$RECOMPUTE_FAIL" = "0" ]; then + rm -rf "$RC_DIR"; mkdir -p "$RC_DIR" + # Longest-first. Ordering is the CALLER's business -- run_recompute_pool + # consumes ALL_WSGS as given, which keeps it testable by pool_probe.sh and + # lets recompute_parity.sh shuffle deliberately for its invariance pass. + ALL_WSGS=$(recompute_order "$ALL_WSGS" | paste -sd, -) + run_recompute_pool "$RECOMPUTE_JOBS" + # Read by the kept-hosts report: whether a recovery must recompute only the + # kept groups and those upstream, or every group of the run (link#347). + RECOMPUTE_RAN=1 + + # Never `cat "$RC_DIR"/*` — ARG_MAX at provincial scope. `find -exec ... +` + # also exits 0 on zero matches, leaving an EMPTY tsv rather than aborting; + # that empty case is a real branch and is judged in R, not here. + : > "$RC_TSV" + find "$RC_DIR" -maxdepth 1 -name '*.rc' -exec cat {} + >> "$RC_TSV" + : > "$RECOMPUTE_LOG" + find "$RC_DIR" -maxdepth 1 -name '*.log' -exec cat {} + >> "$RECOMPUTE_LOG" + + if ( cd "$REPO_ROOT" && LNK_LOAD=loadall \ + Rscript data-raw/fanout_judge.R "$RC_TSV" "$ALL_WSGS" recompute ); then + echo " ✓ recompute: ${N_RECOMPUTE}/${N_RECOMPUTE} WSGs" + rm -rf "$RC_DIR" + else + echo " ✗ recompute incomplete; see $RECOMPUTE_LOG and $RC_DIR/" + RECOMPUTE_FAIL_STAGE="pool" + # Per-job logs KEPT on failure: the concatenation is the artifact, the + # per-job files are what you actually grep. + RECOMPUTE_FAIL=1 + fi + fi +} + +# --- compare (tunnel-free) -> CSV --- +# Over the recomputed groups only: a group this run did not build has nothing +# of this run's to compare (link#348). +run_finish_compare() { + echo "=== compare (tunnel-free) ===" + ( cd "$REPO_ROOT" && LNK_LOAD=loadall Rscript data-raw/study_area_compare.R \ + "$COMPARE_CSV" "$ALL_WSGS" "$CONFIG" ) > "$LOG_DIR/${TS}_compare.log" 2>&1 \ + || { echo " ✗ compare failed; see $LOG_DIR/${TS}_compare.log"; exit 1; } + echo " ✓ compare CSV: $COMPARE_CSV" +} + +run_finish_report() { + echo "=== summary ===" + echo " run WSGs: ${RUN_WSGS:-}" + echo " recomputed WSGs: ${ALL_WSGS:-}" + if [ -n "$ALL_WSGS" ]; then + echo " compare CSV: $COMPARE_CSV" + tail -40 "$LOG_DIR/${TS}_compare.log" || true + fi + + # An incomplete run must not exit 0. The failure is reported HERE rather than + # at the point of detection so that everything already computed is + # consolidated, recomputed, compared and written out first — the operator gets + # the artifacts AND an accurate exit status, instead of one at the cost of the + # other. A caller that only checks the exit code still learns the truth. + # Two independent causes, reported separately. Merging them into one flag + # would print "some host did not account for its bucket" over a recompute + # failure, which is the wrong diagnosis and sends the operator to the wrong + # host. They are also different KINDS of failure: an incomplete bucket means + # output is missing, an incomplete recompute means output is silently WRONG. + # A third cause (link#347): a cypher still up because it may hold work that + # was not consolidated, or whose burn did not complete. --keep-cyphers alone is + # a choice, not a failure; a host it keeps that is ALSO held for cause still + # counts, so the flag cannot turn an unconsolidated host into an exit 0. + local hosts_kept=0 + [ "${KEPT_CAUSE_N:-0}" -gt 0 ] && hosts_kept=1 + if [ "${RUN_INCOMPLETE:-0}" != "0" ] || [ "${RECOMPUTE_FAIL:-0}" != "0" ] \ + || [ "$hosts_kept" != "0" ]; then + echo "=== study_area_run INCOMPLETE ===" + if [ "$hosts_kept" != "0" ]; then + echo " $KEPT_CAUSE_N cypher(s) were NOT burned for cause: each may hold groups" + echo " that are not on the dispatcher, and is still running and billed." + echo " Consolidate only each host's groups column, and only where" + echo " groups_measured = yes; never groups_partial (consolidating a group the" + echo " host no longer holds, or holds in part, deletes the dispatcher's copy)." + if [ "${RECOMPUTE_RAN:-0}" = "1" ]; then + echo " The recompute above ran WITHOUT those groups, so recompute each one and" + echo " each run group upstream of it afterwards. Then burn the host." + else + echo " The recompute did not run, so recompute every group of the run" + echo " afterwards. Then burn the host." + fi + echo " Hosts, reasons and commands: $LOG_DIR/${TS}_kept_hosts.tsv" + fi + if [ "${RUN_INCOMPLETE:-0}" != "0" ]; then + if [ -n "${COV_MISSING:-}" ]; then + echo " These run WSGs have no rows from a build this run finished, so they" + echo " were NOT recomputed or compared: $COV_MISSING" + [ -z "${COV_NOROWS:-}" ] || echo " no rows: $COV_NOROWS" + [ -z "${COV_STALE:-}" ] || echo " an earlier run's or partial rows: $COV_STALE" + [ -z "${COV_LOST:-}" ] || echo " lost at consolidate: $COV_LOST" + echo " The recompute above ran without them. Re-model each under this run_uid," + echo " then recompute the whole run (every group, since a re-modelled group's" + echo " barriers change access upstream of it):" + echo " LNK_RUN_UID=$RUN_UID LNK_SCHEMA=$SCHEMA LNK_LOAD=loadall \\" + echo " Rscript data-raw/wsg_run_one.R $CONFIG" + echo " bash data-raw/study_area_run.sh --recompute-only --run-uid=$RUN_UID \\" + echo " --config=$CONFIG --schema=$SCHEMA --wsgs=$RUN_WSGS" + else + echo " At least one host did not account for its whole bucket; only the" + echo " WSGs it finished were consolidated. Artifacts above are valid for" + echo " those WSGs. Re-run the missing ones before trusting the compare." + fi + fi + if [ "${RECOMPUTE_FAIL:-0}" != "0" ]; then + echo " The post-consolidate recompute did not complete for every WSG." + echo " Those WSGs' streams_access / streams_mapping_code still hold their" + echo " PRE-consolidate values, so cross-WSG access — hence token1/token2" + echo " and ;DAM — is WRONG for them. This is bad output, not missing" + echo " output: the compare CSV above will look complete." + if [ "${RECOMPUTE_FAIL_STAGE:-pool}" = "logtables" ]; then + echo " The provenance log tables could not be created, so NO WSG was" + echo " recomputed. See $LOG_DIR/${TS}_recompute_logtables.log." + elif [ "${RECOMPUTE_FAIL_STAGE:-pool}" = "views" ]; then + echo " The barrier views could not be built, so NO WSG was recomputed." + echo " See $LOG_DIR/${TS}_recompute_views.log." + else + echo " See $RECOMPUTE_LOG (and ${RC_TSV} for per-WSG exit status)." + fi + echo " Re-run just those, no full run needed:" + echo " LNK_SCHEMA=$SCHEMA LNK_LOAD=loadall \\" + echo " Rscript data-raw/wsg_recompute_one.R $CONFIG" + fi + exit 1 + fi + echo "=== study_area_run done ===" +} diff --git a/data-raw/study_area_run.sh b/data-raw/study_area_run.sh index 5a751e5f..fe7cc560 100755 --- a/data-raw/study_area_run.sh +++ b/data-raw/study_area_run.sh @@ -52,7 +52,9 @@ # # Post-conditions: every host must account for its whole bucket before # consolidate (read from its database, not its log), and every run WSG must -# have rows in the persist afterwards. +# have rows from a build this run finished in the persist afterwards. A group +# that does not is named and the run exits non-zero, but the recompute and the +# compare still run over the rest (link#348; data-raw/study_area_finish.sh). # # Extra flags: # --preflight-only run the local gates and exit (no spend) @@ -77,6 +79,15 @@ # --preflight-note="why" downgrade ONLY vintage + parity to warnings, # and only with a written reason. There is no # global bypass on purpose. +# --recompute-only --run-uid= [--wsgs=A,B] +# finish an existing run on the dispatcher: coverage +# for that run_uid, the recompute over every group +# with rows from a finished build of it, the compare +# and the report. No droplet, no pre-flight. For a +# run whose missing groups were re-modelled by hand +# under the same run_uid (link#348). Without --wsgs= +# the expected groups are those with a log row for +# the run_uid on the dispatcher. # # Run identity (link#262). Three levels, and only the middle one is new: # @@ -132,6 +143,14 @@ RECOMPUTE_JOBS=4 # at the recompute, so `set -u` cannot bite on any earlier exit path. RECOMPUTE_FAIL=0 RECOMPUTE_FAIL_STAGE="" +# Set when a run group has no rows from a build this run finished, or a host +# did not account for its bucket (link#348). Read by the final report. +RUN_INCOMPLETE=0 +# --recompute-only finishes an EXISTING run: its run_uid, and optionally the +# run's groups (link#348). +RECOMPUTE_ONLY=0 +RUN_UID_ARG="" +WSGS_ARG="" FOCAL_ARR=() for arg in "$@"; do case "$arg" in @@ -144,6 +163,15 @@ for arg in "$@"; do --preflight-only) PREFLIGHT_ONLY=1 ;; --refresh-primitives) REFRESH_PRIMITIVES=1 ;; --auto-install) AUTO_INSTALL=1 ;; + --recompute-only) RECOMPUTE_ONLY=1 ;; + # Interpolated into SQL (the coverage and log queries), so the format the + # driver mints is the only one accepted. + --run-uid=*) RUN_UID_ARG="${arg#--run-uid=}" + [[ "$RUN_UID_ARG" =~ ^[0-9]{8}_[0-9]{6}-[0-9a-f]{8}$ ]] \ + || { echo "FATAL: --run-uid= must look like 20261017_004619-3f4c713b (got '$RUN_UID_ARG')" >&2; exit 1; } ;; + --wsgs=*) WSGS_ARG="${arg#--wsgs=}" + [[ "$WSGS_ARG" =~ ^[A-Z]{4}(,[A-Z]{4})*$ ]] \ + || { echo "FATAL: --wsgs= takes comma-separated four-letter group codes (got '$WSGS_ARG')" >&2; exit 1; } ;; --prep-ssh-wait=*) PREP_SSH_WAIT_S="${arg#--prep-ssh-wait=}" case "$PREP_SSH_WAIT_S" in ''|*[!0-9]*) echo "FATAL: --prep-ssh-wait= needs a positive integer (got '$PREP_SSH_WAIT_S')" >&2; exit 1 ;; @@ -203,6 +231,25 @@ for arg in "$@"; do esac done +# --recompute-only finishes one existing run on the dispatcher, so it needs +# that run's uid and takes nothing that dispatches. --run-uid and --wsgs +# without it would be silently ignored, so they are refused instead. +if [ "$RECOMPUTE_ONLY" = "1" ]; then + [ -n "$RUN_UID_ARG" ] || { + echo "FATAL: --recompute-only needs --run-uid= of the run to finish" >&2; exit 1; } + if [ "${#FOCAL_ARR[@]}" -gt 0 ] || [ -n "$CY_WS" ] || [ "$PREFLIGHT_ONLY" = "1" ] \ + || [ "$REFRESH_PRIMITIVES" = "1" ] || [ "$AUTO_INSTALL" = "1" ]; then + echo "FATAL: --recompute-only takes no --focal, --cy-workspaces, --preflight-only," >&2 + echo " --refresh-primitives or --auto-install: it dispatches nothing." >&2 + exit 1 + fi +else + [ -z "$RUN_UID_ARG" ] || { + echo "FATAL: --run-uid= is for --recompute-only; a run mints its own" >&2; exit 1; } + [ -z "$WSGS_ARG" ] || { + echo "FATAL: --wsgs= is for --recompute-only; a run's groups come from --focal" >&2; exit 1; } +fi + # --config and --schema are interpolated into the cypher ssh command strings # inside '...', like --run-label: refuse anything that could close the quote # (link#246 code-check). @@ -215,7 +262,7 @@ done # --preflight-only needs no --focal set: it exits before the buckets are # resolved, and demanding them would make the cheap dry check as awkward as # the real thing. -if [ "$PREFLIGHT_ONLY" = "0" ]; then +if [ "$PREFLIGHT_ONLY" = "0" ] && [ "$RECOMPUTE_ONLY" = "0" ]; then IFS=',' read -ra CY_WS_ARR <<< "$CY_WS" [ -n "$CY_WS" ] || CY_WS_ARR=() N_CY=${#CY_WS_ARR[@]} @@ -327,7 +374,13 @@ fi # # $RANDOM is fine for that job (collision avoidance within a second, not # secrecy). Padded to a fixed width so the ids sort and eyeball consistently. -RUN_UID="${TS}-$(printf '%04x%04x' "$RANDOM" "$RANDOM")" +# --recompute-only takes the uid of the run it finishes, so its recompute rows +# land under that run (link#348). +if [ "$RECOMPUTE_ONLY" = "1" ]; then + RUN_UID="$RUN_UID_ARG" +else + RUN_UID="${TS}-$(printf '%04x%04x' "$RANDOM" "$RANDOM")" +fi export LNK_RUN_UID="$RUN_UID" export LNK_RUN_LABEL="$RUN_LABEL" @@ -342,6 +395,7 @@ echo " log dir: $LOG_DIR" echo " vintage max: ${VINTAGE_MAX_DAYS} d" echo " recompute -j: $RECOMPUTE_JOBS" [ "$PREFLIGHT_ONLY" = "0" ] || echo " MODE: --preflight-only (no spend, no writes)" +[ "$RECOMPUTE_ONLY" = "0" ] || echo " MODE: --recompute-only (finish run_uid $RUN_UID; no spend)" [ -z "$PREFLIGHT_NOTE" ] || echo " OVERRIDE: vintage+parity downgraded to warnings — $PREFLIGHT_NOTE" # --- trap: burn cyphers on exit (safety net; explicit burn after consolidate) --- @@ -352,6 +406,9 @@ echo " recompute -j: $RECOMPUTE_JOBS" # and is listed in ${TS}_kept_hosts.tsv with the command that burns it. # shellcheck source=study_area_hosts.sh source "$REPO_ROOT/data-raw/study_area_hosts.sh" +# Coverage, recompute, compare and the final report (link#348). +# shellcheck source=study_area_finish.sh +source "$REPO_ROOT/data-raw/study_area_finish.sh" CYPHERS_UP=0 KEPT_N=0 KEPT_CAUSE_N=0 @@ -398,6 +455,205 @@ on_exit() { } trap on_exit EXIT +# Split a CSV into lines, dropping blanks. `sed` deleting every line still +# exits 0, where `grep -v '^$'` exits 1 — and under `set -euo pipefail` that +# aborts the script from inside a plain assignment. That trap has now bitten +# this diff twice (cypher_prep's ~/.Renviron filter, then the consolidate +# bucket builder below), so the safe form lives in one helper rather than +# being remembered at each call site. +# +# `printf '%s\n'`, not `printf '%s'`: without the trailing newline `wc -l` +# counts separators rather than items and reports one fewer than there is, so +# a host that completed its whole bucket would be reported incomplete. The +# empty case still yields 0, because sed drops the resulting blank line. +csv_lines() { printf '%s\n' "${1:-}" | tr ',' '\n' | sed '/^[[:space:]]*$/d'; } +csv_count() { csv_lines "${1:-}" | wc -l | tr -d ' '; } + +# --- post-consolidate recompute pool (link#250) ---------------------------- +# recompute_one runs ONE WSG and never returns non-zero: the pool must not be +# able to abort under `set -e`, and the exit status is carried by the .rc file +# rather than by this function's return. +# +# One log per job, not a shared fd. Parallel appends to one file interleave +# mid-record once a record exceeds the stdio buffer (~64 KB), which a full R +# traceback comfortably does. +# +# The .rc file holds the finished TSV ROW, not a bare number, so collection is +# one `find -exec cat` with no basename loop. It is also written LAST, so a +# job killed partway leaves no row and is judged "never reported" rather than +# silently counted as fine. +recompute_one() { # $1 = WSG + local w="$1" rc=0 + ( cd "$REPO_ROOT" + LNK_LOAD=loadall LNK_VIEWS_PREBUILT=1 \ + Rscript data-raw/wsg_recompute_one.R "$w" "$CONFIG" + ) > "$RC_DIR/$w.log" 2>&1 || rc=$? + [ "$rc" = "0" ] || echo "[WARN] recompute WSG $w failed (rc=$rc; continuing)" \ + >> "$RC_DIR/$w.log" + printf '%s\t%s\n' "$w" "$rc" > "$RC_DIR/$w.rc" + return 0 +} + +# Order the pool's work list LONGEST-FIRST from previously recorded times. +# +# A pool's makespan is bounded below by its slowest single job, so a long job +# that starts last extends the whole phase. Measured 2026-09-01: CHWK is 226 s +# against 11 s for SALR, and one WSG was 74% of a four-WSG set. LPT is already +# this repo's answer for packing components onto hosts (study_area_buckets.R, +# wsgs_dispatch.sh); this is the same rule one level down, inside one host. +# +# Ordered on prior RECOMPUTE times, never on segment count. Segments are a +# reasonable proxy for modelling work (R^2 0.64 over 104 WSGs) and a bad one +# here: SALR has 20% MORE segments than CHWK and finishes 20x faster, so a +# segment ordering would actively run the cheap job first. +# +# Times come from previous runs' committed ${TS}_recompute.log files, newest +# wins. A WSG with no recorded time gets the MEDIAN of those that have one -- +# the same rule study_area_buckets.R uses for a WSG missing from the network +# table, so an unknown never sorts as free. +# +# With NO prior times at all this returns the input order and SAYS SO. The +# ops-hardening record (planning/archive/2026-05-ops-hardening-20260514) has +# the precedent: an LPT fallback that silently degraded to a naive split and +# ignored host speeds went unnoticed across a 217-WSG run. +recompute_order() { # $1 = csv of WSGs -> prints WSGs, longest-first + local wsgs="$1" tf med n + tf=$(mktemp "${TMPDIR:-/tmp}/lnk_rc_times.XXXXXX") || { csv_lines "$wsgs"; return 0; } + # sort -r on TS-prefixed names is newest-first; awk keeps the first sighting + # of each WSG, so a later run's time wins. + find "$LOG_DIR" -maxdepth 1 -name '*_recompute.log' -type f 2>/dev/null \ + | sort -r \ + | while IFS= read -r f; do + sed -nE 's/^\[wsg_recompute_one\] ([A-Z]{4}) recomputed in ([0-9.]+) min.*/\1 \2/p' "$f" + done | awk '!seen[$1]++' > "$tf" + n=$(wc -l < "$tf" | tr -d ' ') + if [ "${n:-0}" -eq 0 ]; then + echo " note: no prior recompute times in $LOG_DIR — running in input order" >&2 + rm -f "$tf"; csv_lines "$wsgs"; return 0 + fi + med=$(cut -d' ' -f2 "$tf" | sort -n \ + | awk '{a[NR]=$1} END{print (NR%2) ? a[(NR+1)/2] : (a[NR/2]+a[NR/2+1])/2}') + echo " ordering ${n} known WSG time(s) longest-first (median ${med} min for the rest)" >&2 + csv_lines "$wsgs" \ + | awk -v tf="$tf" -v med="$med" ' + BEGIN { while ((getline l < tf) > 0) { split(l, a, " "); t[a[1]] = a[2] } } + { printf "%s %s\n", ($1 in t ? t[$1] : med), $1 }' \ + | sort -k1,1nr -k2,2 \ + | cut -d' ' -f2 + rm -f "$tf" +} + +# Bounded-width pool. `wait -n` is bash 4.3+ and /bin/bash on macOS is 3.2.57, +# so a free slot is found by polling liveness with `kill -0` and reaping the +# finished child with `wait ` (which returns immediately for a child that +# has already exited). We deliberately do NOT read the rc from `wait`: the .rc +# file is the single source of truth, which also means the evidence survives a +# killed dispatcher. +run_recompute_pool() { # $1 = width + local width="$1" slot pid w all_pids="" + local SLOT_PID + # Width must be a positive integer or this HANGS rather than failing: + # `seq 0 -1` is empty, so the slot scan never runs, `break 2` is + # unreachable, and the outer `while :; sleep 1` spins forever. + # --recompute-jobs validates its own input, but recompute_parity.sh and + # recompute_sweep.sh pass a width straight through, so the guard belongs + # HERE where all three callers meet it. + # + # SHAPE FIRST, then value. `[ abc -lt 1 ]` exits 2 ("integer expression + # expected") rather than returning true, so a numeric-only guard treats a + # non-numeric width as "condition false", falls through, and hangs -- which + # is the exact failure it was written to prevent. Measured: 0 and -1 were + # refused while abc and 2x still hung. + case "${width:-}" in + ''|*[!0-9]*) + echo "FATAL: run_recompute_pool needs a positive integer width (got '${width:-}')" >&2 + return 1 ;; + esac + # NORMALISE THE BASE ONCE, before anything consumes it. `test` and `case` + # read base 10; `$(( ))` reads a leading zero as OCTAL. So "08" passes both + # validators and then dies in `$((width - 1))` with "value too great for + # base" -> empty seq -> the same hang; and "010" quietly becomes 8, giving + # a pool two slots narrower than the operator asked for and than the banner + # reports. Three rounds of this bug were three predicates disagreeing about + # the grammar; one normalisation ends the class. + width=$((10#$width)) + # Upper bound HERE, not only on the CLI flag. `10#` wraps silently on + # overflow -- 10#99999999999999999999 is 7766279631452241919, which passes + # the shape check and the >= 1 test and then sends `seq` off for the rest of + # the afternoon. --recompute-jobs is saved by its own <= 16, but + # recompute_parity.sh and recompute_sweep.sh pass an operator width straight + # through, and this function is what all three callers meet. + if [ "$width" -lt 1 ] || [ "$width" -gt 64 ]; then + echo "FATAL: run_recompute_pool needs 1 <= width <= 64 (got '$width')" >&2 + return 1 + fi + # Pre-seed every slot. Referencing an unset array element under `set -u` is + # an unbound-variable error on bash 3.2. + SLOT_PID=() + for slot in $(seq 0 $((width - 1))); do SLOT_PID[$slot]=0; done + + for w in $(csv_lines "$ALL_WSGS"); do + while :; do + for slot in $(seq 0 $((width - 1))); do + pid=${SLOT_PID[$slot]} + [ "$pid" = "0" ] && break 2 + if ! kill -0 "$pid" 2>/dev/null; then + wait "$pid" 2>/dev/null || true + SLOT_PID[$slot]=0 + break 2 + fi + done + sleep 1 + done + recompute_one "$w" & + SLOT_PID[$slot]=$! + all_pids="$all_pids ${SLOT_PID[$slot]}" + done + # Drain the last partial wave, waiting on OUR children only. + # + # A bare `wait` waits for every background job in the calling shell, not + # just the ones this pool started -- so it silently couples the pool to + # whatever else the caller has backgrounded, and hangs outright if any of + # those is long-lived. Measured 2026-09-01: a 2-second sampler loop + # backgrounded by data-raw/recompute_sweep.sh wedged the pool indefinitely, + # with every job already finished and nothing to show for it. + # + # An empty WSG list means zero iterations and no pids -- that emptiness is + # NOT judged here, it is judged in R by lnk_fanout_judge(), where the + # branch can be tested. + for pid in $all_pids; do wait "$pid" 2>/dev/null || true; done +} + +# --- --recompute-only: finish a run whose gaps were filled by hand (link#348) +# A run that ends with groups missing names them; once each is re-modelled +# under the same run_uid (LNK_RUN_UID= ... wsg_run_one.R), this runs the +# rest of a run's tail and nothing else: coverage for that run_uid, the +# recompute over every group with rows from a finished build of it, the +# compare and the report. No droplet, no pre-flight (the recompute reads only +# the persisted tables), no consolidate. +if [ "$RECOMPUTE_ONLY" = "1" ]; then + echo "=== --recompute-only: run_uid $RUN_UID ===" + echo " pre-flight, dispatch and consolidate SKIPPED: this reads only the" + echo " persisted tables in $SCHEMA, and spins up nothing." + pg_isready -h localhost -p 5432 >/dev/null 2>&1 \ + || { echo "FATAL: local fwapg down (:5432)"; exit 1; } + if [ -n "$WSGS_ARG" ]; then + RO_EXPECT=$(csv_lines "$WSGS_ARG" | sort -u | paste -sd, -) + echo " expected WSGs: $(csv_count "$RO_EXPECT"), from --wsgs=" + else + RO_EXPECT=$(run_expected_from_log) \ + || { echo "FATAL: could not read $SCHEMA.log"; exit 1; } + echo " expected WSGs: the $(csv_count "$RO_EXPECT") with a $SCHEMA.log row for this run_uid." + echo " A group that failed on a cloud host and was not re-run here has no row" + echo " here; pass the run's groups with --wsgs= to have it named." + fi + [ -n "$RO_EXPECT" ] || { + echo "FATAL: no $SCHEMA.log row for run_uid $RUN_UID, and no --wsgs=: nothing to recompute" + exit 1; } + run_finish "$RO_EXPECT" "" + exit 0 +fi + # --- pre-flight, local: everything answerable before a droplet exists ------ # # A cypher's software is PREDICTABLE from here: its link comes from @@ -1113,174 +1369,6 @@ echo " ✓ host runs finished (per-WSG soft-fail; gaps surface in compare)" # about to be re-COPYed cannot delete anything it does not replace. The gap # then surfaces at the coverage post-condition after the burn, by which point # the successful work is safely on the dispatcher. -# Split a CSV into lines, dropping blanks. `sed` deleting every line still -# exits 0, where `grep -v '^$'` exits 1 — and under `set -euo pipefail` that -# aborts the script from inside a plain assignment. That trap has now bitten -# this diff twice (cypher_prep's ~/.Renviron filter, then the consolidate -# bucket builder below), so the safe form lives in one helper rather than -# being remembered at each call site. -# -# `printf '%s\n'`, not `printf '%s'`: without the trailing newline `wc -l` -# counts separators rather than items and reports one fewer than there is, so -# a host that completed its whole bucket would be reported incomplete. The -# empty case still yields 0, because sed drops the resulting blank line. -csv_lines() { printf '%s\n' "${1:-}" | tr ',' '\n' | sed '/^[[:space:]]*$/d'; } -csv_count() { csv_lines "${1:-}" | wc -l | tr -d ' '; } - -# --- post-consolidate recompute pool (link#250) ---------------------------- -# recompute_one runs ONE WSG and never returns non-zero: the pool must not be -# able to abort under `set -e`, and the exit status is carried by the .rc file -# rather than by this function's return. -# -# One log per job, not a shared fd. Parallel appends to one file interleave -# mid-record once a record exceeds the stdio buffer (~64 KB), which a full R -# traceback comfortably does. -# -# The .rc file holds the finished TSV ROW, not a bare number, so collection is -# one `find -exec cat` with no basename loop. It is also written LAST, so a -# job killed partway leaves no row and is judged "never reported" rather than -# silently counted as fine. -recompute_one() { # $1 = WSG - local w="$1" rc=0 - ( cd "$REPO_ROOT" - LNK_LOAD=loadall LNK_VIEWS_PREBUILT=1 \ - Rscript data-raw/wsg_recompute_one.R "$w" "$CONFIG" - ) > "$RC_DIR/$w.log" 2>&1 || rc=$? - [ "$rc" = "0" ] || echo "[WARN] recompute WSG $w failed (rc=$rc; continuing)" \ - >> "$RC_DIR/$w.log" - printf '%s\t%s\n' "$w" "$rc" > "$RC_DIR/$w.rc" - return 0 -} - -# Order the pool's work list LONGEST-FIRST from previously recorded times. -# -# A pool's makespan is bounded below by its slowest single job, so a long job -# that starts last extends the whole phase. Measured 2026-09-01: CHWK is 226 s -# against 11 s for SALR, and one WSG was 74% of a four-WSG set. LPT is already -# this repo's answer for packing components onto hosts (study_area_buckets.R, -# wsgs_dispatch.sh); this is the same rule one level down, inside one host. -# -# Ordered on prior RECOMPUTE times, never on segment count. Segments are a -# reasonable proxy for modelling work (R^2 0.64 over 104 WSGs) and a bad one -# here: SALR has 20% MORE segments than CHWK and finishes 20x faster, so a -# segment ordering would actively run the cheap job first. -# -# Times come from previous runs' committed ${TS}_recompute.log files, newest -# wins. A WSG with no recorded time gets the MEDIAN of those that have one -- -# the same rule study_area_buckets.R uses for a WSG missing from the network -# table, so an unknown never sorts as free. -# -# With NO prior times at all this returns the input order and SAYS SO. The -# ops-hardening record (planning/archive/2026-05-ops-hardening-20260514) has -# the precedent: an LPT fallback that silently degraded to a naive split and -# ignored host speeds went unnoticed across a 217-WSG run. -recompute_order() { # $1 = csv of WSGs -> prints WSGs, longest-first - local wsgs="$1" tf med n - tf=$(mktemp "${TMPDIR:-/tmp}/lnk_rc_times.XXXXXX") || { csv_lines "$wsgs"; return 0; } - # sort -r on TS-prefixed names is newest-first; awk keeps the first sighting - # of each WSG, so a later run's time wins. - find "$LOG_DIR" -maxdepth 1 -name '*_recompute.log' -type f 2>/dev/null \ - | sort -r \ - | while IFS= read -r f; do - sed -nE 's/^\[wsg_recompute_one\] ([A-Z]{4}) recomputed in ([0-9.]+) min.*/\1 \2/p' "$f" - done | awk '!seen[$1]++' > "$tf" - n=$(wc -l < "$tf" | tr -d ' ') - if [ "${n:-0}" -eq 0 ]; then - echo " note: no prior recompute times in $LOG_DIR — running in input order" >&2 - rm -f "$tf"; csv_lines "$wsgs"; return 0 - fi - med=$(cut -d' ' -f2 "$tf" | sort -n \ - | awk '{a[NR]=$1} END{print (NR%2) ? a[(NR+1)/2] : (a[NR/2]+a[NR/2+1])/2}') - echo " ordering ${n} known WSG time(s) longest-first (median ${med} min for the rest)" >&2 - csv_lines "$wsgs" \ - | awk -v tf="$tf" -v med="$med" ' - BEGIN { while ((getline l < tf) > 0) { split(l, a, " "); t[a[1]] = a[2] } } - { printf "%s %s\n", ($1 in t ? t[$1] : med), $1 }' \ - | sort -k1,1nr -k2,2 \ - | cut -d' ' -f2 - rm -f "$tf" -} - -# Bounded-width pool. `wait -n` is bash 4.3+ and /bin/bash on macOS is 3.2.57, -# so a free slot is found by polling liveness with `kill -0` and reaping the -# finished child with `wait ` (which returns immediately for a child that -# has already exited). We deliberately do NOT read the rc from `wait`: the .rc -# file is the single source of truth, which also means the evidence survives a -# killed dispatcher. -run_recompute_pool() { # $1 = width - local width="$1" slot pid w all_pids="" - local SLOT_PID - # Width must be a positive integer or this HANGS rather than failing: - # `seq 0 -1` is empty, so the slot scan never runs, `break 2` is - # unreachable, and the outer `while :; sleep 1` spins forever. - # --recompute-jobs validates its own input, but recompute_parity.sh and - # recompute_sweep.sh pass a width straight through, so the guard belongs - # HERE where all three callers meet it. - # - # SHAPE FIRST, then value. `[ abc -lt 1 ]` exits 2 ("integer expression - # expected") rather than returning true, so a numeric-only guard treats a - # non-numeric width as "condition false", falls through, and hangs -- which - # is the exact failure it was written to prevent. Measured: 0 and -1 were - # refused while abc and 2x still hung. - case "${width:-}" in - ''|*[!0-9]*) - echo "FATAL: run_recompute_pool needs a positive integer width (got '${width:-}')" >&2 - return 1 ;; - esac - # NORMALISE THE BASE ONCE, before anything consumes it. `test` and `case` - # read base 10; `$(( ))` reads a leading zero as OCTAL. So "08" passes both - # validators and then dies in `$((width - 1))` with "value too great for - # base" -> empty seq -> the same hang; and "010" quietly becomes 8, giving - # a pool two slots narrower than the operator asked for and than the banner - # reports. Three rounds of this bug were three predicates disagreeing about - # the grammar; one normalisation ends the class. - width=$((10#$width)) - # Upper bound HERE, not only on the CLI flag. `10#` wraps silently on - # overflow -- 10#99999999999999999999 is 7766279631452241919, which passes - # the shape check and the >= 1 test and then sends `seq` off for the rest of - # the afternoon. --recompute-jobs is saved by its own <= 16, but - # recompute_parity.sh and recompute_sweep.sh pass an operator width straight - # through, and this function is what all three callers meet. - if [ "$width" -lt 1 ] || [ "$width" -gt 64 ]; then - echo "FATAL: run_recompute_pool needs 1 <= width <= 64 (got '$width')" >&2 - return 1 - fi - # Pre-seed every slot. Referencing an unset array element under `set -u` is - # an unbound-variable error on bash 3.2. - SLOT_PID=() - for slot in $(seq 0 $((width - 1))); do SLOT_PID[$slot]=0; done - - for w in $(csv_lines "$ALL_WSGS"); do - while :; do - for slot in $(seq 0 $((width - 1))); do - pid=${SLOT_PID[$slot]} - [ "$pid" = "0" ] && break 2 - if ! kill -0 "$pid" 2>/dev/null; then - wait "$pid" 2>/dev/null || true - SLOT_PID[$slot]=0 - break 2 - fi - done - sleep 1 - done - recompute_one "$w" & - SLOT_PID[$slot]=$! - all_pids="$all_pids ${SLOT_PID[$slot]}" - done - # Drain the last partial wave, waiting on OUR children only. - # - # A bare `wait` waits for every background job in the calling shell, not - # just the ones this pool started -- so it silently couples the pool to - # whatever else the caller has backgrounded, and hangs outright if any of - # those is long-lived. Measured 2026-09-01: a 2-second sampler loop - # backgrounded by data-raw/recompute_sweep.sh wedged the pool indefinitely, - # with every job already finished and nothing to show for it. - # - # An empty WSG list means zero iterations and no pids -- that emptiness is - # NOT judged here, it is judged in R by lnk_fanout_judge(), where the - # branch can be tested. - for pid in $all_pids; do wait "$pid" 2>/dev/null || true; done -} # What each host finished comes from its DATABASE (host_account, in # study_area_hosts.sh): .log rows for this run_uid with date_end set, @@ -1374,248 +1462,27 @@ fi # --- burn cyphers now (released hosts only; minimise idle) --- burn_cyphers || true -# WSG set across all hosts. -ALL_WSGS=$( { echo "$DISP_BUCKET" | tr ',' '\n' +# The run's groups: every host's bucket, less the groups a host SKIPped (no +# modelled species: wsg_run_one.R writes no row for them, so they cannot have +# any). Finished signals beyond the dispatcher's log rows for this run_uid: +# its own `done` markers, and each cypher's consolidated bucket (database ∪ +# log done), which is what consolidate took. They only add (run_coverage). +RUN_SKIP="${ACC_SKIP[dispatcher]:-}" +RUN_SIGNALS=$(.hosts_union "${ACC_FINISHED[dispatcher]:-}" "${ACC_DONE_LOG[dispatcher]:-}") +for WS in "${CY_WS_ARR[@]}"; do + RUN_SKIP=$(.hosts_union "$RUN_SKIP" "${ACC_SKIP[$WS]:-}") + RUN_SIGNALS=$(.hosts_union "$RUN_SIGNALS" "${CY_BUCKET_DONE[$WS]:-}") +done +RUN_WSGS=$( { echo "$DISP_BUCKET" | tr ',' '\n' for WS in "${CY_WS_ARR[@]}"; do echo "${CY_BUCKET[$WS]}" | tr ',' '\n'; done } | sed '/^[[:space:]]*$/d' | sort -u | paste -sd, - ) -COMPARE_CSV="$LOG_DIR/${TS}_compare.csv" - -# --- coverage post-condition (link#246) ------------------------------------ -# Detection rather than prevention: this fires whatever the cause, including -# causes nobody has thought of yet. Placed AFTER the burn so a failure cannot -# leak spend, and BEFORE the recompute so a partial result is never painted -# as complete. -# -# Necessary because schema_consolidate DELETEs the destination bucket from -# every persist table (its step 3.5) and then COPYs (step 4). A source that -# produced nothing therefore removes the destination's prior rows for those -# WSGs and still returns ok = TRUE. The log tables keep their rows (#354), so -# this check reads streams, never the log. -echo "=== verify WSG coverage in $SCHEMA ===" -if MISSING=$(PGPASSWORD=postgres psql -h localhost -p 5432 -U postgres -d fwapg -t -A -c " - WITH expect(w) AS (SELECT unnest(string_to_array('$ALL_WSGS', ','))) - SELECT coalesce(string_agg(e.w, ',' ORDER BY e.w), '') - FROM expect e - LEFT JOIN (SELECT DISTINCT watershed_group_code w FROM ${SCHEMA}.streams) g - ON g.w = e.w - WHERE g.w IS NULL") -then - # coalesce(...,'') so zero missing returns an empty string rather than - # NULL — and the empty string is only trusted on this success branch. - [ -z "$MISSING" ] || { - echo "FATAL: consolidate left $SCHEMA with no rows for: $MISSING"; exit 1; } - echo " ✓ every run WSG has rows in $SCHEMA.streams" -else - echo "FATAL: could not verify WSG coverage in $SCHEMA"; exit 1 -fi +RUN_WSGS=$(.hosts_minus "$RUN_WSGS" "$RUN_SKIP") +[ -z "$RUN_SKIP" ] || echo " note: SKIPped (no modelled species), so not expected to have rows: $RUN_SKIP" -# This check asks "are there rows", NOT "are they from this run". The persist -# accumulates across runs and consolidate's DELETE is bucket-scoped, so a WSG -# that was excluded from a narrowed bucket keeps its PREVIOUS run's rows and -# passes here. It is a guard against consolidate destroying data, not a -# substitute for the completeness accounting — an earlier revision of this -# script leaned on it as the backstop for an incomplete run, and it cannot -# carry that. `$RUN_INCOMPLETE` is what carries it, at the very end. +# A host that did not account for its whole bucket. Coverage can only add to +# it: a group with no rows from this run sets it too (link#348). RUN_INCOMPLETE="$complete_fail" -if [ "$RUN_INCOMPLETE" != "0" ]; then - echo " NOTE: some WSGs were excluded from consolidate this run; any rows" - echo " they show above are from an EARLIER run, not this one." -fi - -# --- post-consolidate recompute: settle cross-WSG access (link#205) --- -# Drainage-closed + DS-first per-host is NOT sufficient: a WSG's downstream -# barriers can be cross-bucket or arrive late in DS-first order, so its access -# (hence token1/token2) is computed against an incomplete barrier set. -# Caught 2026-05-25: FINA 75% / PARA 69% per-host -> both 99% only after -# re-modelling on the full consolidated barrier set. The recompute is the -# correctness guarantee REGARDLESS of bucketing. We use lnk_access(merge=TRUE) -# — the cheap access-only recompute that reuses the persisted streams / -# habitat / barriers / barrier_overrides (link#205, ~10 s/WSG vs ~1.5 min for -# a full pipeline rebuild). Because it is cheap, we recompute ALL run WSGs -# unconditionally rather than threshold-filtering by parity — bucketing is -# now a speed knob, not a correctness lever. -# -# Parallel since link#250. Two things previously made the serial loop -# mandatory, and both are now gone: -# -# 1. lnk_access() rebuilt the SCHEMA-scoped barrier views on every call -- -# 38 DDL statements against names every sibling WSG reads. CREATE OR -# REPLACE VIEW takes an AccessExclusiveLock, and a QUEUED exclusive -# request blocks every AccessShareLock behind it, so one job's DDL would -# stall every sibling's read until lock_timeout killed someone. The build -# is now hoisted below and each job runs LNK_VIEWS_PREBUILT=1. Everything -# else a job touches is WSG-scoped (zz_lnk_streams_, -# zz_lnk_access_scratch_, zz_lnk_mc_scratch_) or row-scoped -# (UPDATE ... WHERE watershed_group_code; DELETE+INSERT in a transaction). -# -# 2. A failed WSG was invisible. `|| echo` sat INSIDE the loop, so the -# subshell always exited 0 and the success line below was unconditional; -# RUN_INCOMPLETE is assigned BEFORE this block and nothing after raised -# it. A run in which every recompute failed exited 0 and wrote a compare -# CSV. The .rc files + lnk_fanout_judge() are what fix that. -RECOMPUTE_LOG="$LOG_DIR/${TS}_recompute.log" -RC_DIR="$LOG_DIR/${TS}_recompute.d" -RC_TSV="$LOG_DIR/${TS}_recompute.tsv" -N_RECOMPUTE=$(csv_count "$ALL_WSGS") - -echo "=== post-consolidate recompute (lnk_access, ${N_RECOMPUTE} WSGs, -j${RECOMPUTE_JOBS}) ===" - -# Ensure the log tables exist ONCE, single-threaded, before any job starts -# (link#262). wsg_recompute_one.R deliberately refuses to run DDL — it runs -# inside this pool, and schema DDL belongs at init, not in N concurrent jobs. -# That leaves one edge case with the worst possible timing: if every focal WSG -# on the dispatcher species-skips (wsg_run_one.R exits 0 without calling -# lnk_pipeline_run), nothing ever creates log_recompute, and the pool would -# then hard-stop on all N WSGs — at the end of a paid multi-host run, after -# consolidate. One idempotent call here removes the class for nothing. -if ( cd "$REPO_ROOT" && LNK_LOAD=loadall Rscript -e ' -suppressPackageStartupMessages(pkgload::load_all(quiet = TRUE)) -cfg <- lnk_config(commandArgs(TRUE)[1]) -s <- Sys.getenv("LNK_SCHEMA"); if (nzchar(s)) cfg$pipeline$schema <- s -conn <- lnk_db_conn(dbname = "fwapg", host = "localhost", port = 5432L, - user = "postgres", password = "postgres") -on.exit(try(DBI::dbDisconnect(conn), silent = TRUE), add = TRUE) -link:::.lnk_log_create_tables(conn, cfg$pipeline$schema) -' "$CONFIG" ) > "$LOG_DIR/${TS}_recompute_logtables.log" 2>&1; then - echo " ✓ log tables present in $SCHEMA" -else - echo " ✗ could not ensure log tables; see $LOG_DIR/${TS}_recompute_logtables.log" - echo " the recompute would fail per-WSG on a missing log_recompute" - RECOMPUTE_FAIL=1 - # Its OWN stage token. Reusing "views" made the final gate print "the barrier - # views could not be built" and point at ${TS}_recompute_views.log -- a file - # the views step never wrote, because it is gated on RECOMPUTE_FAIL=0 and - # never ran. Two prep steps, two names, two pieces of evidence. - RECOMPUTE_FAIL_STAGE="logtables" -fi - -# Build the barrier views ONCE, single-threaded, before any job starts. -# Nested rather than `[ "$RECOMPUTE_FAIL" = "0" ] && ( build )`: with the -# short-circuit form, an already-failed log-table step would fall into this -# else and report "barrier view build failed", naming a log file that was -# never written. Two prep steps, two diagnoses. -if [ "$RECOMPUTE_FAIL" = "0" ]; then - if ( cd "$REPO_ROOT" && LNK_LOAD=loadall \ - Rscript data-raw/barriers_views_build.R "$CONFIG" ) \ - > "$LOG_DIR/${TS}_recompute_views.log" 2>&1; then - echo " ✓ barrier views built once for $SCHEMA" - else - # Deliberately NOT falling back to per-WSG builds. At -j>1 that is exactly - # the race the hoist removes, and it would turn one loud pre-build failure - # into N quiet lock_timeouts attributed to individual WSGs. - echo " ✗ barrier view build failed; see $LOG_DIR/${TS}_recompute_views.log" - echo " skipping the recompute — the run will exit non-zero" - RECOMPUTE_FAIL=1 - # Which stage failed decides which evidence exists. The pool never ran, so - # RECOMPUTE_LOG and RC_TSV were never created -- naming them at the final - # gate would send the operator to two absent files. - RECOMPUTE_FAIL_STAGE="views" - fi -fi - -if [ "$RECOMPUTE_FAIL" = "0" ]; then - rm -rf "$RC_DIR"; mkdir -p "$RC_DIR" - # Longest-first. Ordering is the CALLER's business -- run_recompute_pool - # consumes ALL_WSGS as given, which keeps it testable by pool_probe.sh and - # lets recompute_parity.sh shuffle deliberately for its invariance pass. - ALL_WSGS=$(recompute_order "$ALL_WSGS" | paste -sd, -) - run_recompute_pool "$RECOMPUTE_JOBS" - # Read by the kept-hosts report: whether a recovery must recompute only the - # kept groups and those upstream, or every group of the run (link#347). - RECOMPUTE_RAN=1 - - # Never `cat "$RC_DIR"/*` — ARG_MAX at provincial scope. `find -exec ... +` - # also exits 0 on zero matches, leaving an EMPTY tsv rather than aborting; - # that empty case is a real branch and is judged in R, not here. - : > "$RC_TSV" - find "$RC_DIR" -maxdepth 1 -name '*.rc' -exec cat {} + >> "$RC_TSV" - : > "$RECOMPUTE_LOG" - find "$RC_DIR" -maxdepth 1 -name '*.log' -exec cat {} + >> "$RECOMPUTE_LOG" - - if ( cd "$REPO_ROOT" && LNK_LOAD=loadall \ - Rscript data-raw/fanout_judge.R "$RC_TSV" "$ALL_WSGS" recompute ); then - echo " ✓ recompute: ${N_RECOMPUTE}/${N_RECOMPUTE} WSGs" - rm -rf "$RC_DIR" - else - echo " ✗ recompute incomplete; see $RECOMPUTE_LOG and $RC_DIR/" - RECOMPUTE_FAIL_STAGE="pool" - # Per-job logs KEPT on failure: the concatenation is the artifact, the - # per-job files are what you actually grep. - RECOMPUTE_FAIL=1 - fi -fi - -# --- compare (tunnel-free) -> CSV --- -echo "=== compare (tunnel-free) ===" -( cd "$REPO_ROOT" && LNK_LOAD=loadall Rscript data-raw/study_area_compare.R \ - "$COMPARE_CSV" "$ALL_WSGS" "$CONFIG" ) > "$LOG_DIR/${TS}_compare.log" 2>&1 \ - || { echo " ✗ compare failed; see $LOG_DIR/${TS}_compare.log"; exit 1; } -echo " ✓ compare CSV: $COMPARE_CSV" - -# --- report --- -echo "=== summary ===" -echo " run WSGs: $ALL_WSGS" -echo " compare CSV: $COMPARE_CSV" -tail -40 "$LOG_DIR/${TS}_compare.log" || true -# An incomplete run must not exit 0. The failure is reported HERE rather than -# at the point of detection so that everything already computed is -# consolidated, recomputed, compared and written out first — the operator gets -# the artifacts AND an accurate exit status, instead of one at the cost of the -# other. A caller that only checks the exit code still learns the truth. -# Two independent causes, reported separately. Merging them into one flag -# would print "some host did not account for its bucket" over a recompute -# failure, which is the wrong diagnosis and sends the operator to the wrong -# host. They are also different KINDS of failure: an incomplete bucket means -# output is missing, an incomplete recompute means output is silently WRONG. -# A third cause (link#347): a cypher still up because it may hold work that -# was not consolidated, or whose burn did not complete. --keep-cyphers alone is -# a choice, not a failure; a host it keeps that is ALSO held for cause still -# counts, so the flag cannot turn an unconsolidated host into an exit 0. -HOSTS_KEPT=0 -[ "${KEPT_CAUSE_N:-0}" -gt 0 ] && HOSTS_KEPT=1 -if [ "${RUN_INCOMPLETE:-0}" != "0" ] || [ "${RECOMPUTE_FAIL:-0}" != "0" ] \ - || [ "$HOSTS_KEPT" != "0" ]; then - echo "=== study_area_run INCOMPLETE ===" - if [ "$HOSTS_KEPT" != "0" ]; then - echo " $KEPT_CAUSE_N cypher(s) were NOT burned for cause: each may hold groups" - echo " that are not on the dispatcher, and is still running and billed." - echo " Consolidate only each host's groups column, and only where" - echo " groups_measured = yes; never groups_partial (consolidating a group the" - echo " host no longer holds, or holds in part, deletes the dispatcher's copy)." - if [ "${RECOMPUTE_RAN:-0}" = "1" ]; then - echo " The recompute above ran WITHOUT those groups, so recompute each one and" - echo " each run group upstream of it afterwards. Then burn the host." - else - echo " The recompute did not run, so recompute every group of the run" - echo " afterwards. Then burn the host." - fi - echo " Hosts, reasons and commands: $LOG_DIR/${TS}_kept_hosts.tsv" - fi - if [ "${RUN_INCOMPLETE:-0}" != "0" ]; then - echo " At least one host did not account for its whole bucket; only the" - echo " WSGs it finished were consolidated. Artifacts above are valid for" - echo " those WSGs. Re-run the missing ones before trusting the compare." - fi - if [ "${RECOMPUTE_FAIL:-0}" != "0" ]; then - echo " The post-consolidate recompute did not complete for every WSG." - echo " Those WSGs' streams_access / streams_mapping_code still hold their" - echo " PRE-consolidate values, so cross-WSG access — hence token1/token2" - echo " and ;DAM — is WRONG for them. This is bad output, not missing" - echo " output: the compare CSV above will look complete." - if [ "${RECOMPUTE_FAIL_STAGE:-pool}" = "logtables" ]; then - echo " The provenance log tables could not be created, so NO WSG was" - echo " recomputed. See $LOG_DIR/${TS}_recompute_logtables.log." - elif [ "${RECOMPUTE_FAIL_STAGE:-pool}" = "views" ]; then - echo " The barrier views could not be built, so NO WSG was recomputed." - echo " See $LOG_DIR/${TS}_recompute_views.log." - else - echo " See $RECOMPUTE_LOG (and ${RC_TSV} for per-WSG exit status)." - fi - echo " Re-run just those, no full run needed:" - echo " LNK_SCHEMA=$SCHEMA LNK_LOAD=loadall \\" - echo " Rscript data-raw/wsg_recompute_one.R $CONFIG" - fi - exit 1 -fi -echo "=== study_area_run done ===" +# Coverage, recompute over the groups this run built, compare, report. Exits +# non-zero on an incomplete run. Bare call: see study_area_finish.sh. +run_finish "$RUN_WSGS" "$RUN_SIGNALS" diff --git a/planning/active/progress.md b/planning/active/progress.md index d9d1b987..4becf35c 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -6,3 +6,8 @@ - Created branch `348-one-failed-group-skips-the-post-consolid` off main - Scaffolded PWF baseline from issue #348 with approved phases - Next: start Phase 1 +- Phase 1: `tests/testthat/test-study_area_finish.R` (8 tests, fixture schema + Rscript stub, real pool functions extracted from the driver); `hosts_bash()` moved to `helper-bash.R` +- Phase 2: `data-raw/study_area_finish.sh` (`run_coverage`, `run_expected_from_log`, `run_finish`); driver gains `--recompute-only --run-uid= [--wsgs=]`, pool block moved above the pre-flight, tail replaced by `run_finish` +- Verified: finish tests 8/8 and hosts tests pass; `bash -n` both; `pool_probe.sh` PASS=37 FAIL=0; recompute_parity/sweep extraction still resolves +- Mutation: a copy of the lib restoring the exit on a missing group fails acceptance tests 1 (6 failures) and 2 (3 failures) +- Live, read-only: `run_coverage` on `fresh_default` for run_uid `20261017_004619-3f4c713b`: expected 241 (from the log), 241/241 recompute, 0 missing diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index 92082ab7..04400404 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -20,9 +20,9 @@ Design: `/Users/airvine/.claude/plans/quiet-hopping-umbrella.md` (approved 2026- - [x] Test: the driver refuses `--run-uid` alone, `--recompute-only` alone, a malformed uid, and `--recompute-only --focal=…` (all exit before any Rscript) ## Phase 2: `data-raw/study_area_finish.sh` + driver wiring -- [ ] Write the lib (`run_coverage`, `run_expected_from_log`, `run_finish`) and move the tail into `run_finish` with the changes above -- [ ] Driver: new flags and validation, move the pool function block above the pre-flight, add the `--recompute-only` branch, normal path calls `run_finish`; update the header usage and "Post-conditions" comment -- [ ] `bash -n` both scripts; Phase 1 tests pass; `pool_probe.sh data-raw/study_area_run.sh` still passes (the extraction still works) +- [x] Write the lib (`run_coverage`, `run_expected_from_log`, `run_finish`) and move the tail into `run_finish` with the changes above +- [x] Driver: new flags and validation, move the pool function block above the pre-flight, add the `--recompute-only` branch, normal path calls `run_finish`; update the header usage and "Post-conditions" comment +- [x] `bash -n` both scripts; Phase 1 tests pass; `pool_probe.sh data-raw/study_area_run.sh` still passes (the extraction still works) ## Phase 3: Docs - [ ] RUNBOOK §8d: the coverage classes, recompute over this run's finished groups, `--recompute-only --run-uid` recovery (replace the "coverage FATAL" wording at ~`:1291`) From ed441f0c9c842e6351658b1b620a495aaa91c1ee Mon Sep 17 00:00:00 2001 From: almac2022 Date: Sun, 18 Oct 2026 20:56:37 -0700 Subject: [PATCH 04/10] Document coverage classes and --recompute-only recovery (#348) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WJE2szSCkeV9R3qKhuSwyx --- CLAUDE.md | 2 +- RUNBOOK.md | 50 +++++++++++++++++++++++++++++++++--- planning/active/progress.md | 1 + planning/active/task_plan.md | 10 ++++---- research/study_area_run.md | 4 ++- 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cb269871..da8686a1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ Experimental package — breaking all the time and loving the learning curve. St **Facts not worth re-deriving:** - **One run_uid, more than one link SHA.** In `log`, 239 groups stamp `ee21eb13` and KITR and UPET `a17e9177`, the fix for the two bugs that failed them, re-run on m1 under the same run_uid. In `log_recompute`, all 241 stamp `a17e9177`, so the shipped `streams_access` / `streams_mapping_code` were written at it. UPET was recomputed again after `91a968ab` (the mapping-code fix, below), from a clean tree at `eb93f343`; one stray row between those is `link_dirty` TRUE (an uncommitted review file), and the clean re-run follows it. - **The driver read completeness from log text, and it missed 18 finished groups that way** (#347): an R warning put the `done` marker mid-line, the driver consolidated only what it counted, and its burn would have destroyed the rest; the operator held it with `kill -STOP` and consolidated by hand. Since #347 the count is each host's database, a cypher is held from dispatch until no `streams` rows are left on it after consolidate, and a kept host is listed in `_kept_hosts.tsv` (RUNBOOK §8d, "Which host may be burned"). -- **A failed group with no earlier rows skips the recompute for every group** (#348): the coverage gate exits before it. A failed group that still has an earlier run's rows passes the gate, and its stale rows go unnoticed there. +- **A failed group no longer skips the recompute for every group** (#348). Until then the coverage gate exited before the recompute, and a failed group holding an earlier run's rows passed it unnoticed. Coverage now reads which groups have rows from a build this run finished, the recompute and compare run over those, and the rest are named with a non-zero exit. `--recompute-only --run-uid=` finishes a run once they are re-modelled under that uid (RUNBOOK §8d, "A missing group costs only itself"). - **The recompute takes 154 min at 241 groups and `-j4`** (median 2.0 min per group, longest 16 min), not the ~1 h the plan assumed. - **A present species with no barrier in a group is accessible** (`access_` 1 or 2) **and has `has_barriers__dnstr` FALSE**, so its mapping code is built. Zero barrier rows mean NA ("no data") only when nothing says the species is present. Before `91a968ab` the flag stayed NA and UPET RB shipped with `mapping_code_rb` "" on every segment; UPET RB is the only such case in the 241. - **`fresh_default` is 31 GB** (14 GB tables, 17 GB indexes; 11.39 M segments), up from 7.3 GB. Summing `pg_total_relation_size()` over every `pg_class` row counts each index twice (48 GB). The fwapg volume is 600 GiB with 253 G free after the run. diff --git a/RUNBOOK.md b/RUNBOOK.md index 5f8cf83d..b486e690 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -1209,11 +1209,55 @@ that DELETE, so a group's log history survives it; link#354.) So: - before consolidate, every host must account for its whole bucket, read from its **database**: `.log` rows for the run_uid with `date_end` set, plus `SKIP` markers from the log, which write no row (link#347); -- after consolidate, every run WSG must have rows in `.streams`. +- after consolidate, every run WSG must have rows in `.streams` from a + build **this run finished**. The second is detection rather than prevention, and it is the one that would have caught link#246 on day one regardless of cause. +### A missing group costs only itself (link#348) + +The coverage check used to `exit 1` on the first run group with no rows, +before the recompute. One failed group then left every other group's access +computed against an incomplete barrier set, with no recompute rows at all. +It also asked only whether rows existed, so a failed group holding an earlier +run's rows passed and was recomputed as this run's. + +Now `run_coverage` (`data-raw/study_area_finish.sh`) splits the run's groups +(every bucket, less SKIPs) into four classes, read on the dispatcher after +consolidate: + +| class | rows in `streams` | finished this run | what happens | +|---|---|---|---| +| recompute | yes | yes | recomputed and compared | +| no rows | no | no | named at the end | +| earlier or partial | yes | no | named; its rows are left as they are | +| lost at consolidate | no | yes | named | + +"Finished this run" is a `.log` row for the run_uid with `date_end` +set (consolidate copies a group's log rows home, link#354), and the driver adds +the dispatcher's `done` markers and each cypher's consolidated bucket. A +failed build leaves its row open (`.lnk_log_run_fail` sets only `notes`), and +its pre-persist can leave `streams` rows, which is why "rows" alone is not +"this run's". The recompute and the compare run over the first class only. If +that class is empty, neither runs and the report says why. Any other class +makes the run exit non-zero, and the report names the groups and the recovery: + +```bash +LNK_RUN_UID= LNK_SCHEMA= LNK_LOAD=loadall \ + Rscript data-raw/wsg_run_one.R # each missing group +bash data-raw/study_area_run.sh --recompute-only --run-uid= \ + --config= --schema= [--wsgs=] +``` + +`--recompute-only` runs the driver's tail and nothing else: no droplet, no +pre-flight, no consolidate. It recomputes every group of the run, not just +the re-modelled ones, because a re-modelled group's barriers change access +upstream of it. Without `--wsgs=`, the expected set is the groups with any log row +for the run_uid on the dispatcher. A group that failed on a cloud host and was +never re-run here has no such row, so only `--wsgs=` gets it named. +Tests: `tests/testthat/test-study_area_finish.R`, on a scratch schema. + ### Which host may be burned (link#347) The count used to be `[wsg_run_one] … done` lines, anchored at line start. R @@ -1288,8 +1332,8 @@ older build. A group with persist rows but **no** log row on the dispatcher no longer holds it, not replaced. Where it says `no`, find out what is left first. If the run's recompute ran, it ran without the kept groups: recompute each one and each run group upstream of it (`wsg_recompute_one.R`). If it did not run -(consolidate failed, coverage FATAL), recompute every group of the run; the -banner says which. A missing +(consolidate failed, or no run group had rows from this run), recompute every +group of the run; the banner says which. A missing `.streams` or `streams_mapping_code` on a host reads as unknown, not empty: every dispatched cypher has both. Tests: `tests/testthat/test-study_area_hosts.R` runs the lib in bash with stubbed ssh, psql, tofu and doctl. diff --git a/planning/active/progress.md b/planning/active/progress.md index 4becf35c..103aee32 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -11,3 +11,4 @@ - Verified: finish tests 8/8 and hosts tests pass; `bash -n` both; `pool_probe.sh` PASS=37 FAIL=0; recompute_parity/sweep extraction still resolves - Mutation: a copy of the lib restoring the exit on a missing group fails acceptance tests 1 (6 failures) and 2 (3 failures) - Live, read-only: `run_coverage` on `fresh_default` for run_uid `20261017_004619-3f4c713b`: expected 241 (from the log), 241/241 recompute, 0 missing +- Phase 3: RUNBOOK §8d gains "A missing group costs only itself" (classes table, recovery commands); CLAUDE.md #348 fact and research/study_area_run.md recovery line updated diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index 04400404..f388386b 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -25,13 +25,13 @@ Design: `/Users/airvine/.claude/plans/quiet-hopping-umbrella.md` (approved 2026- - [x] `bash -n` both scripts; Phase 1 tests pass; `pool_probe.sh data-raw/study_area_run.sh` still passes (the extraction still works) ## Phase 3: Docs -- [ ] RUNBOOK §8d: the coverage classes, recompute over this run's finished groups, `--recompute-only --run-uid` recovery (replace the "coverage FATAL" wording at ~`:1291`) -- [ ] `CLAUDE.md` #246 status fact on #348: state the new behaviour -- [ ] `research/study_area_run.md`: one line where the run's recovery is described, if it names the coverage exit +- [x] RUNBOOK §8d: the coverage classes, recompute over this run's finished groups, `--recompute-only --run-uid` recovery (replace the "coverage FATAL" wording at ~`:1291`) +- [x] `CLAUDE.md` #246 status fact on #348: state the new behaviour +- [x] `research/study_area_run.md`: one line where the run's recovery is described, if it names the coverage exit ## Phase 4: Verification -- [ ] Mutation: restore `exit 1` on missing in a copy of the lib (`LNK_TEST_FINISH_LIB`), and confirm acceptance test 1 goes red -- [ ] Live, read-only: `run_coverage` against `fresh_default` for run_uid `20261017_004619-3f4c713b` with the 241 groups gives 241 recompute and 0 missing (no recompute run) +- [x] Mutation: restore `exit 1` on missing in a copy of the lib (`LNK_TEST_FINISH_LIB`), and confirm acceptance test 1 goes red +- [x] Live, read-only: `run_coverage` against `fresh_default` for run_uid `20261017_004619-3f4c713b` with the 241 groups gives 241 recompute and 0 missing (no recompute run) - [ ] `devtools::test()` full suite, `lintr` on the new test file ## Validation diff --git a/research/study_area_run.md b/research/study_area_run.md index aaaddadc..197302f0 100644 --- a/research/study_area_run.md +++ b/research/study_area_run.md @@ -242,7 +242,9 @@ cloud hosts, `--refresh-primitives`, link `ee21eb13`. Both verifiers pass at their only copy. Two groups failed on bugs fixed in `a17e9177` (a duplicate PSCIS row from an xref that sets both ids; a logical `access_` for a present species with no barrier). One failed group would have skipped the - recompute for all of them (#348). The operator paused the driver, the 18 were + recompute for all of them (#348; since fixed: coverage now recomputes the + groups the run built and names the rest, and `--recompute-only + --run-uid=` finishes the run once those are re-modelled, RUNBOOK §8d). The operator paused the driver, the 18 were consolidated by hand, KITR and UPET re-ran on m1 under the same run_uid, and the driver resumed. So the run carries more than one link SHA: in `log`, 239 groups `ee21eb13` and 2 `a17e9177`; in `log_recompute`, all 241 `a17e9177`, From 9e719bdc2d6951d45918680c314b47f540f00333 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Sun, 18 Oct 2026 21:08:43 -0700 Subject: [PATCH 05/10] Fold in plan review: config check, newest-row rule, compare reaches the report (#348) - --recompute-only refuses a --config other than the one the run's log rows name. - A group counts as finished this run only when its newest log row is the run's and closed (schema_consolidate.R's rule), so a later rebuild or a failed hand re-run under the same uid is not recomputed as this run's. - A failed compare falls through to the report instead of hiding it. - The full run's expected set and signals come from a tested lib function; a SKIP another host finished is kept. - _coverage.tsv records each group's class; discharge resolution is skipped in recompute-only; recovery text names LNK_GUARD_DOWNSTREAM=warn and --recompute-only, including the kept-host and consolidate banners. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WJE2szSCkeV9R3qKhuSwyx --- RUNBOOK.md | 33 ++++-- data-raw/study_area_finish.sh | 127 ++++++++++++++++++++---- data-raw/study_area_hosts.sh | 4 +- data-raw/study_area_run.sh | 50 ++++++---- planning/active/findings.md | 12 ++- planning/active/progress.md | 3 + planning/active/review-plan.md | 25 +++++ planning/active/task_plan.md | 4 +- tests/testthat/helper-bash.R | 20 ---- tests/testthat/setup.R | 20 ++++ tests/testthat/test-study_area_finish.R | 104 +++++++++++++++---- 11 files changed, 315 insertions(+), 87 deletions(-) create mode 100644 planning/active/review-plan.md delete mode 100644 tests/testthat/helper-bash.R diff --git a/RUNBOOK.md b/RUNBOOK.md index b486e690..a93526bc 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -1234,12 +1234,18 @@ consolidate: | earlier or partial | yes | no | named; its rows are left as they are | | lost at consolidate | no | yes | named | -"Finished this run" is a `.log` row for the run_uid with `date_end` -set (consolidate copies a group's log rows home, link#354), and the driver adds -the dispatcher's `done` markers and each cypher's consolidated bucket. A -failed build leaves its row open (`.lnk_log_run_fail` sets only `notes`), and -its pre-persist can leave `streams` rows, which is why "rows" alone is not -"this run's". The recompute and the compare run over the first class only. If +"Finished this run" means the group's **newest** `.log` row (max +`date_start`, the rule `schema_consolidate.R` uses for which build a group's +persist rows hold) is the run_uid's and has `date_end` set. Consolidate +copies a group's log rows home (link#354), and the driver adds the +dispatcher's `done` markers and each cypher's consolidated bucket. A failed +build leaves its row open (`.lnk_log_run_fail` sets only `notes`), and its +pre-persist can leave `streams` rows, so rows alone do not make a group +"this run's". "Newest" matters for a `--recompute-only` days later: a group a +later run rebuilt, or a hand re-run under the same uid that failed after its +pre-persist, has a closed row for the uid that no longer names its rows. The +classes are written to `_coverage.tsv`, beside the compare CSV, which +covers the first class only. The recompute and the compare run over the first class only. If that class is empty, neither runs and the report says why. Any other class makes the run exit non-zero, and the report names the groups and the recovery: @@ -1255,7 +1261,20 @@ pre-flight, no consolidate. It recomputes every group of the run, not just the re-modelled ones, because a re-modelled group's barriers change access upstream of it. Without `--wsgs=`, the expected set is the groups with any log row for the run_uid on the dispatcher. A group that failed on a cloud host and was -never re-run here has no such row, so only `--wsgs=` gets it named. +never re-run here has no such row, so only `--wsgs=` gets it named. A +`--wsgs=` that leaves out logged groups warns. It refuses a `--config` other +than the one the run's log rows name, since a forgotten `--config` would +recompute every group under `bcfishpass` and stamp the rows with the run's +uid. It skips the discharge resolution as well as the pre-flight, but still +checks for `fresh.streams_vw_bcfp`, which the compare reads. + +Two things it does not do. It takes no lock, so do not run it while another +driver on the same schema can resume: a driver paused with `kill -STOP`, as in +#347's recovery, would run a second barrier-view build and pool on the schema. +And `study_area_verify.sql` stays red for a group whose failed attempt left an +open log row **on the dispatcher**. That row is never closed, so verify reports +the group as started and never finished, even after the re-run succeeds. In +#246 the open rows were on the cloud hosts, so it did not show. Tests: `tests/testthat/test-study_area_finish.R`, on a scratch schema. ### Which host may be burned (link#347) diff --git a/data-raw/study_area_finish.sh b/data-raw/study_area_finish.sh index 925c4d16..5eca70dc 100644 --- a/data-raw/study_area_finish.sh +++ b/data-raw/study_area_finish.sh @@ -2,7 +2,8 @@ # dispatcher: which groups have rows from this run, the post-consolidate # recompute over those, the compare, and the final report (link#348). Sourced # by study_area_run.sh after study_area_hosts.sh, and by -# tests/testthat/test-study_area_finish.R. Sourcing defines functions only. +# tests/testthat/test-study_area_finish.R. Sourcing defines functions and +# declares CY_BUCKET_DONE (re-declaring keeps its contents), nothing else. # # The rule (link#348): one failed group does not cost the others their # recompute. Coverage used to `exit 1` on the first run group with no @@ -18,23 +19,80 @@ # # Globals read: SCHEMA RUN_UID CONFIG TS LOG_DIR REPO_ROOT RECOMPUTE_JOBS # RUN_INCOMPLETE RECOMPUTE_FAIL RECOMPUTE_FAIL_STAGE (initialised by the -# driver); with a default: KEPT_CAUSE_N RECOMPUTE_ONLY +# driver); with a default: KEPT_CAUSE_N RUN_LABEL; run_expected_from_hosts +# also reads DISP_BUCKET CY_WS_ARR CY_BUCKET CY_BUCKET_DONE ACC_FINISHED +# ACC_DONE_LOG ACC_SKIP # Functions read: .hosts_query host_finished .hosts_* (study_area_hosts.sh); # csv_count recompute_order run_recompute_pool (study_area_run.sh) # Globals written: COV_STATE COV_RECOMPUTE COV_NOROWS COV_STALE COV_LOST -# COV_MISSING RUN_WSGS ALL_WSGS COMPARE_CSV RUN_INCOMPLETE RECOMPUTE_FAIL -# RECOMPUTE_FAIL_STAGE RECOMPUTE_RAN +# COV_MISSING RUN_WSGS ALL_WSGS COMPARE_CSV COMPARE_FAIL RUN_INCOMPLETE +# RECOMPUTE_FAIL RECOMPUTE_FAIL_STAGE RECOMPUTE_RAN; run_expected_from_hosts +# writes RUN_EXPECT RUN_SIGNALS RUN_SKIP # # Needs bash >= 4, as the driver does. +declare -A CY_BUCKET_DONE + +# run_expected_from_hosts: what a full run expects, and what it already knows +# finished, from the per-host accounting (study_area_hosts.sh). Sets: +# RUN_EXPECT every host's bucket, less RUN_SKIP; +# RUN_SKIP groups a host SKIPped (no modelled species: wsg_run_one.R +# writes no row for them) and no host finished. Buckets come +# from the same species filter, so one here means a host's +# config or overrides disagree with the dispatcher's: a WARN; +# RUN_SIGNALS finish signals beyond the dispatcher's log rows: its `done` +# markers and each cypher's consolidated bucket (database ∪ +# log done), which is what consolidate took. They only add. +run_expected_from_hosts() { + local ws all skip + all="${DISP_BUCKET:-}" + skip="${ACC_SKIP[dispatcher]:-}" + RUN_SIGNALS=$(.hosts_union "${ACC_FINISHED[dispatcher]:-}" "${ACC_DONE_LOG[dispatcher]:-}") + for ws in "${CY_WS_ARR[@]}"; do + all=$(.hosts_union "$all" "${CY_BUCKET[$ws]:-}") + skip=$(.hosts_union "$skip" "${ACC_SKIP[$ws]:-}") + RUN_SIGNALS=$(.hosts_union "$RUN_SIGNALS" "${CY_BUCKET_DONE[$ws]:-}") + done + RUN_SKIP=$(.hosts_minus "$skip" "$RUN_SIGNALS") + RUN_EXPECT=$(.hosts_minus "$all" "$RUN_SKIP") + [ -z "$RUN_SKIP" ] || \ + echo " WARN: SKIPped by a host (no modelled species) though its bucket expected them; not expected to have rows: $RUN_SKIP" + return 0 +} + +# run_config_check: the run's log rows on the dispatcher name exactly one +# config, and it is $CONFIG. --recompute-only with a forgotten --config would +# otherwise recompute every group under another bundle and stamp the rows with +# the run's uid. Returns 1, saying why, otherwise. +run_config_check() { + local out + out=$(PGPASSWORD=postgres psql -h localhost -p 5432 -U postgres -d fwapg \ + -X -q -t -A -v ON_ERROR_STOP=1 -c " + SELECT coalesce(string_agg(DISTINCT coalesce(config_name, ''), ',' + ORDER BY coalesce(config_name, '')), '') + FROM ${SCHEMA}.log WHERE run_uid = '${RUN_UID}'" 2>/dev/null) || { + echo " ✗ could not read config_name from $SCHEMA.log"; return 1; } + if [ "$out" = "$CONFIG" ]; then + echo " ✓ run_uid $RUN_UID was built with --config=$CONFIG" + return 0 + fi + echo " ✗ run_uid $RUN_UID was built with config '${out:-}', not --config=$CONFIG" + return 1 +} + # run_coverage [signals csv]: split the expected groups by what # the dispatcher's database holds for this run. # -# COV_RECOMPUTE rows in .streams, and finished this run: a log row -# for RUN_UID with date_end set, or a signal the driver -# passes (a host's `done` marker, a consolidated bucket). -# The log's date_end is a soft UPDATE that can fail -# (.lnk_log_run_finish), so signals only ever add. +# COV_RECOMPUTE rows in .streams, and finished this run: the +# group's NEWEST log row (max date_start, the rule +# schema_consolidate.R uses for which build the persist rows +# hold) is RUN_UID's and has date_end set, or the driver +# passes a signal (a host's `done` marker, a consolidated +# bucket). The log's date_end is a soft UPDATE that can fail +# (.lnk_log_run_finish), so signals only ever add. Newest, +# not any: a group a later run rebuilt, or a hand re-run under +# this uid that failed after its pre-persist, holds rows that +# are not this run's finished build. # COV_NOROWS no rows at all. # COV_STALE rows, but no finish signal of this run: an earlier run's # rows, or this run's pre-persist of a build that then failed. @@ -52,7 +110,10 @@ run_coverage() { in=$(.hosts_lines "$expected" | sed "s/.*/'&'/" | paste -sd, -) rows=$(.hosts_query local streams "watershed_group_code IN (${in:-NULL})" 1) \ || return 1 - closed=$(host_finished local) || return 1 + closed=$(.hosts_query local log "run_uid = '${RUN_UID}' AND date_end IS NOT NULL + AND (watershed_group_code, date_start) IN ( + SELECT watershed_group_code, max(date_start) FROM ${SCHEMA}.log + GROUP BY watershed_group_code)") || return 1 COV_STATE=known rows=$(printf '%s\n' "$rows" | .hosts_csv) fin=$(.hosts_union "$(printf '%s\n' "$closed" | .hosts_csv)" "$signals") @@ -86,8 +147,9 @@ run_expected_from_log() { # `set -e`, a function called in an `if` or `||` runs with errexit off. run_finish() { RUN_WSGS=$(.hosts_union "${1:-}") - local signals="${2:-}" + local signals="${2:-}" w COMPARE_CSV="$LOG_DIR/${TS}_compare.csv" + COMPARE_FAIL=0 # --- coverage post-condition (link#246, link#348) ------------------------- # Detection rather than prevention: this fires whatever the cause. In a full @@ -106,6 +168,16 @@ run_finish() { if ! run_coverage "$RUN_WSGS" "$signals"; then echo "FATAL: could not verify WSG coverage in $SCHEMA"; exit 1 fi + # One row per run group and its class: the compare CSV covers only the + # first class, so this is what says what it leaves out. + { + printf 'wsg\tclass\n' + for w in $(.hosts_lines "$COV_RECOMPUTE"); do printf '%s\trecompute\n' "$w"; done + for w in $(.hosts_lines "$COV_NOROWS"); do printf '%s\tno_rows\n' "$w"; done + for w in $(.hosts_lines "$COV_STALE"); do printf '%s\tnot_this_run\n' "$w"; done + for w in $(.hosts_lines "$COV_LOST"); do printf '%s\tlost_at_consolidate\n' "$w"; done + } > "$LOG_DIR/${TS}_coverage.tsv" + echo " classes: $LOG_DIR/${TS}_coverage.tsv" if [ -n "$COV_MISSING" ]; then RUN_INCOMPLETE=1 echo " NOTE: recomputing only the $(.hosts_n "$COV_RECOMPUTE") WSGs above; the rest keep" @@ -251,20 +323,24 @@ link:::.lnk_log_create_tables(conn, cfg$pipeline$schema) # --- compare (tunnel-free) -> CSV --- # Over the recomputed groups only: a group this run did not build has nothing -# of this run's to compare (link#348). +# of this run's to compare (link#348). A failure no longer exits here: it +# would hide the report, which names the groups that are missing. run_finish_compare() { echo "=== compare (tunnel-free) ===" - ( cd "$REPO_ROOT" && LNK_LOAD=loadall Rscript data-raw/study_area_compare.R \ - "$COMPARE_CSV" "$ALL_WSGS" "$CONFIG" ) > "$LOG_DIR/${TS}_compare.log" 2>&1 \ - || { echo " ✗ compare failed; see $LOG_DIR/${TS}_compare.log"; exit 1; } - echo " ✓ compare CSV: $COMPARE_CSV" + if ( cd "$REPO_ROOT" && LNK_LOAD=loadall Rscript data-raw/study_area_compare.R \ + "$COMPARE_CSV" "$ALL_WSGS" "$CONFIG" ) > "$LOG_DIR/${TS}_compare.log" 2>&1; then + echo " ✓ compare CSV: $COMPARE_CSV" + else + echo " ✗ compare failed; see $LOG_DIR/${TS}_compare.log" + COMPARE_FAIL=1 + fi } run_finish_report() { echo "=== summary ===" echo " run WSGs: ${RUN_WSGS:-}" echo " recomputed WSGs: ${ALL_WSGS:-}" - if [ -n "$ALL_WSGS" ]; then + if [ -n "$ALL_WSGS" ] && [ "${COMPARE_FAIL:-0}" = "0" ]; then echo " compare CSV: $COMPARE_CSV" tail -40 "$LOG_DIR/${TS}_compare.log" || true fi @@ -286,7 +362,7 @@ run_finish_report() { local hosts_kept=0 [ "${KEPT_CAUSE_N:-0}" -gt 0 ] && hosts_kept=1 if [ "${RUN_INCOMPLETE:-0}" != "0" ] || [ "${RECOMPUTE_FAIL:-0}" != "0" ] \ - || [ "$hosts_kept" != "0" ]; then + || [ "$hosts_kept" != "0" ] || [ "${COMPARE_FAIL:-0}" != "0" ]; then echo "=== study_area_run INCOMPLETE ===" if [ "$hosts_kept" != "0" ]; then echo " $KEPT_CAUSE_N cypher(s) were NOT burned for cause: each may hold groups" @@ -301,6 +377,9 @@ run_finish_report() { echo " The recompute did not run, so recompute every group of the run" echo " afterwards. Then burn the host." fi + echo " Every group of the run, once consolidated:" + echo " bash data-raw/study_area_run.sh --recompute-only --run-uid=$RUN_UID \\" + echo " --config=$CONFIG --schema=$SCHEMA" echo " Hosts, reasons and commands: $LOG_DIR/${TS}_kept_hosts.tsv" fi if [ "${RUN_INCOMPLETE:-0}" != "0" ]; then @@ -313,7 +392,11 @@ run_finish_report() { echo " The recompute above ran without them. Re-model each under this run_uid," echo " then recompute the whole run (every group, since a re-modelled group's" echo " barriers change access upstream of it):" - echo " LNK_RUN_UID=$RUN_UID LNK_SCHEMA=$SCHEMA LNK_LOAD=loadall \\" + # warn, as the driver runs it: a group whose downstream neighbour is + # missing too would otherwise stop at the guard (error by default), + # and the recompute re-runs it in error mode once all are back. + echo " LNK_RUN_UID=$RUN_UID ${RUN_LABEL:+LNK_RUN_LABEL=$RUN_LABEL }LNK_SCHEMA=$SCHEMA \\" + echo " LNK_GUARD_DOWNSTREAM=warn LNK_LOAD=loadall \\" echo " Rscript data-raw/wsg_run_one.R $CONFIG" echo " bash data-raw/study_area_run.sh --recompute-only --run-uid=$RUN_UID \\" echo " --config=$CONFIG --schema=$SCHEMA --wsgs=$RUN_WSGS" @@ -323,6 +406,12 @@ run_finish_report() { echo " those WSGs. Re-run the missing ones before trusting the compare." fi fi + if [ "${COMPARE_FAIL:-0}" != "0" ]; then + echo " The compare failed (see $LOG_DIR/${TS}_compare.log); the recompute" + echo " above stands. Re-run it alone:" + echo " LNK_SCHEMA=$SCHEMA LNK_LOAD=loadall Rscript data-raw/study_area_compare.R \\" + echo " $COMPARE_CSV ${ALL_WSGS:-} $CONFIG" + fi if [ "${RECOMPUTE_FAIL:-0}" != "0" ]; then echo " The post-consolidate recompute did not complete for every WSG." echo " Those WSGs' streams_access / streams_mapping_code still hold their" diff --git a/data-raw/study_area_hosts.sh b/data-raw/study_area_hosts.sh index dffccca9..978bada6 100644 --- a/data-raw/study_area_hosts.sh +++ b/data-raw/study_area_hosts.sh @@ -534,7 +534,9 @@ hosts_kept_report() { echo " The run's recompute did NOT run: after consolidating, recompute EVERY" echo " group of the run, not only these" fi - echo " (LNK_SCHEMA=$SCHEMA LNK_LOAD=loadall Rscript data-raw/wsg_recompute_one.R )." + echo " (every group: bash data-raw/study_area_run.sh --recompute-only" + echo " --run-uid=$RUN_UID --config= --schema=$SCHEMA; one group:" + echo " LNK_SCHEMA=$SCHEMA LNK_LOAD=loadall Rscript data-raw/wsg_recompute_one.R )." echo " Then burn:" for ws in "$@"; do echo " cd $CYPHER_DIR && ./cypher_down.sh --workspace $ws"; done echo " Recorded in $tsv" diff --git a/data-raw/study_area_run.sh b/data-raw/study_area_run.sh index fe7cc560..20c4e734 100755 --- a/data-raw/study_area_run.sh +++ b/data-raw/study_area_run.sh @@ -335,6 +335,11 @@ export LNK_SCHEMA="$SCHEMA" # Where the config reads discharge (link#331): every host needs that table, # because prepare joins discharge for every group. The dispatcher's gate # checks its own database; cypher_prep loads wet's on each cypher. +# --recompute-only models nothing and reads no discharge (wsg_recompute_one.R), +# so it resolves neither and cannot be stopped by them (link#348). +DISCHARGE="" +WET_LOAD="" +if [ "$RECOMPUTE_ONLY" = "0" ]; then DISCHARGE=$(cd "$REPO_ROOT" && LNK_LOAD=loadall Rscript -e ' suppressPackageStartupMessages(pkgload::load_all(quiet = TRUE)) cat(link:::.lnk_discharge_source(lnk_config(commandArgs(TRUE)[1])))' "$CONFIG" 2>/dev/null || true) @@ -345,7 +350,6 @@ esac # The wet load the dispatcher holds; every cypher must hold the same one # (cypher_prep.sh), or hosts classify on different discharge. Empty when the # dispatcher has none, which its discharge gate then fails. -WET_LOAD="" if [ "$DISCHARGE" = "wet" ]; then WET_LOAD=$(cd "$REPO_ROOT" && LNK_LOAD=loadall Rscript -e ' suppressPackageStartupMessages(pkgload::load_all(quiet = TRUE)) @@ -362,6 +366,7 @@ if (res$ok) cat(link:::.lnk_discharge_wet_load_id(res$comment))' \ *"'"*) echo "FATAL: the wet table comment holds a quote: $WET_LOAD"; exit 1 ;; esac fi +fi # --- run identity (link#262) --- # Minted ONCE, here, before anything forks. $TS is already the run's own @@ -637,12 +642,24 @@ if [ "$RECOMPUTE_ONLY" = "1" ]; then echo " persisted tables in $SCHEMA, and spins up nothing." pg_isready -h localhost -p 5432 >/dev/null 2>&1 \ || { echo "FATAL: local fwapg down (:5432)"; exit 1; } + # The compare reads the local bcfp snapshot, as in a full run. + [ "$(PGPASSWORD=postgres psql -h localhost -p 5432 -U postgres -d fwapg -t -A -c \ + "SELECT 1 FROM information_schema.tables WHERE table_schema='fresh' AND table_name='streams_vw_bcfp'" 2>/dev/null || true)" = "1" ] \ + || { echo "FATAL: fresh.streams_vw_bcfp missing (run snapshot_bcfp.sh --with-bcfp-views)"; exit 1; } + run_config_check || { echo "FATAL: refusing to recompute run_uid $RUN_UID under another config"; exit 1; } + RO_LOGGED=$(run_expected_from_log) \ + || { echo "FATAL: could not read $SCHEMA.log"; exit 1; } if [ -n "$WSGS_ARG" ]; then RO_EXPECT=$(csv_lines "$WSGS_ARG" | sort -u | paste -sd, -) echo " expected WSGs: $(csv_count "$RO_EXPECT"), from --wsgs=" + # A subset leaves the groups upstream of a re-modelled one on access + # computed before its barriers changed. + RO_LEFT=$(.hosts_minus "$RO_LOGGED" "$RO_EXPECT") + [ -z "$RO_LEFT" ] || { + echo " WARN: --wsgs= leaves out $(csv_count "$RO_LEFT") group(s) with log rows for this run_uid;" + echo " those upstream of a re-modelled group keep stale access: $RO_LEFT"; } else - RO_EXPECT=$(run_expected_from_log) \ - || { echo "FATAL: could not read $SCHEMA.log"; exit 1; } + RO_EXPECT="$RO_LOGGED" echo " expected WSGs: the $(csv_count "$RO_EXPECT") with a $SCHEMA.log row for this run_uid." echo " A group that failed on a cloud host and was not re-run here has no row" echo " here; pass the run's groups with --wsgs= to have it named." @@ -1443,7 +1460,11 @@ quit(status = if (ok) 0 else 1) " ) > "$LOG_DIR/${TS}_consolidate.log" 2>&1 || crc=$? CONSOLIDATING=0 [ "$crc" = "0" ] \ - || { echo " ✗ consolidate failed; see $LOG_DIR/${TS}_consolidate.log — cyphers KEPT"; exit 1; } + || { echo " ✗ consolidate failed; see $LOG_DIR/${TS}_consolidate.log — cyphers KEPT" + echo " No group was recomputed. Once the hosts' groups are consolidated:" + echo " bash data-raw/study_area_run.sh --recompute-only --run-uid=$RUN_UID \\" + echo " --config=$CONFIG --schema=$SCHEMA" + exit 1; } echo " ✓ consolidated $n_src/$N_CY cypher(s) (see $LOG_DIR/${TS}_consolidate.log)" fi @@ -1462,22 +1483,9 @@ fi # --- burn cyphers now (released hosts only; minimise idle) --- burn_cyphers || true -# The run's groups: every host's bucket, less the groups a host SKIPped (no -# modelled species: wsg_run_one.R writes no row for them, so they cannot have -# any). Finished signals beyond the dispatcher's log rows for this run_uid: -# its own `done` markers, and each cypher's consolidated bucket (database ∪ -# log done), which is what consolidate took. They only add (run_coverage). -RUN_SKIP="${ACC_SKIP[dispatcher]:-}" -RUN_SIGNALS=$(.hosts_union "${ACC_FINISHED[dispatcher]:-}" "${ACC_DONE_LOG[dispatcher]:-}") -for WS in "${CY_WS_ARR[@]}"; do - RUN_SKIP=$(.hosts_union "$RUN_SKIP" "${ACC_SKIP[$WS]:-}") - RUN_SIGNALS=$(.hosts_union "$RUN_SIGNALS" "${CY_BUCKET_DONE[$WS]:-}") -done -RUN_WSGS=$( { echo "$DISP_BUCKET" | tr ',' '\n' - for WS in "${CY_WS_ARR[@]}"; do echo "${CY_BUCKET[$WS]}" | tr ',' '\n'; done -} | sed '/^[[:space:]]*$/d' | sort -u | paste -sd, - ) -RUN_WSGS=$(.hosts_minus "$RUN_WSGS" "$RUN_SKIP") -[ -z "$RUN_SKIP" ] || echo " note: SKIPped (no modelled species), so not expected to have rows: $RUN_SKIP" +# The run's groups, less SKIPs, and the finish signals the per-host accounting +# already holds (run_expected_from_hosts, study_area_finish.sh). +run_expected_from_hosts # A host that did not account for its whole bucket. Coverage can only add to # it: a group with no rows from this run sets it too (link#348). @@ -1485,4 +1493,4 @@ RUN_INCOMPLETE="$complete_fail" # Coverage, recompute over the groups this run built, compare, report. Exits # non-zero on an incomplete run. Bare call: see study_area_finish.sh. -run_finish "$RUN_WSGS" "$RUN_SIGNALS" +run_finish "$RUN_EXPECT" "$RUN_SIGNALS" diff --git a/planning/active/findings.md b/planning/active/findings.md index f4ddb50c..9be257f9 100644 --- a/planning/active/findings.md +++ b/planning/active/findings.md @@ -28,7 +28,7 @@ New sourced lib **`data-raw/study_area_finish.sh`** (sourced after `study_area_h - **Missing groups never exit.** They set `RUN_INCOMPLETE=1`, and the report names each by class. - **An empty `COV_RECOMPUTE` skips the recompute and the compare**, and the run prints why ("no run group has rows from a build this run finished"). - The report gives the recovery command: `LNK_RUN_UID= LNK_SCHEMA=… Rscript data-raw/wsg_run_one.R `, then `study_area_run.sh --recompute-only --run-uid= …`. - - It returns the status, and the driver `exit`s with it. + - It exits 1 on an incomplete run, as the tail always did, so it is called bare (revised in build: a function called under `if` or `||` runs with errexit off). Driver changes (`study_area_run.sh`): - Flags: @@ -41,6 +41,16 @@ Driver changes (`study_area_run.sh`): - Normal path: `RUN_WSGS` = union of buckets minus every host's SKIP set. Signals = dispatcher `ACC_FINISHED ∪ ACC_DONE_LOG` ∪ each cypher's `CY_BUCKET_DONE`. Then `run_finish "$RUN_WSGS" "$signals"`. +## Plan review (2026-10-19) + +Triage table in `review-plan.md`. Probes before acting: +- `fresh_default.log` for run `20261017_004619-3f4c713b` names one config (`default`), so the config check passes there and refuses `bcfishpass`. +- 241 groups have a newest log row that is closed and from that run, so the newest-row rule changes nothing for #246 (live `run_coverage`: 241/241, 0 missing). + +## Suite baseline + +`devtools::test()` on this branch: 3195 pass, 2 failed and 2 errors in `test-lnk_pipeline_prepare.R` (golden dams pin, known stale per CLAUDE.md: 8 pinned, `cabd.dams` gives 7) and `test-lnk_wsg_resolve.R` (closure). No `R/` file changed on this branch and neither file reads `data-raw/`. + ## Errors Encountered | Error | Resolution | diff --git a/planning/active/progress.md b/planning/active/progress.md index 103aee32..69b281ba 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -12,3 +12,6 @@ - Mutation: a copy of the lib restoring the exit on a missing group fails acceptance tests 1 (6 failures) and 2 (3 failures) - Live, read-only: `run_coverage` on `fresh_default` for run_uid `20261017_004619-3f4c713b`: expected 241 (from the log), 241/241 recompute, 0 missing - Phase 3: RUNBOOK §8d gains "A missing group costs only itself" (classes table, recovery commands); CLAUDE.md #348 fact and research/study_area_run.md recovery line updated +- Plan review folded in (`review-plan.md`): run_config_check, newest-row rule, compare failure falls through to the report, run_expected_from_hosts (SKIP minus signals, WARN), `_coverage.tsv`, discharge skipped in recompute-only, recovery text with LNK_GUARD_DOWNSTREAM=warn, kept-host text points at --recompute-only, consolidate-failure banner prints the recovery +- Tests 56/56 (11 tests); hosts 120/120; mutants m_exit / m_newest / m_noskip each red; pool_probe PASS=37; live probe on fresh_default 241/241 with the newest rule, config check refuses bcfishpass +- `hosts_bash()` now lives in `tests/testthat/setup.R` beside `skip_if_no_db()` (helper-bash.R dropped) diff --git a/planning/active/review-plan.md b/planning/active/review-plan.md new file mode 100644 index 00000000..a10cd710 --- /dev/null +++ b/planning/active/review-plan.md @@ -0,0 +1,25 @@ +# Plan review — #348 (Plan agent, 2026-10-19 ~04:00 UTC) + +Reviewer read the plan, the issue and the code while Phase 1-2 were being written, +so it reviewed the draft as well as the plan. The findings and what was done with each: + +| # | Finding | Probe | Disposition | +|---|---|---|---| +| Blocker 1 | `--recompute-only` never checks that `--config` is the run's config; a forgotten `--config` recomputes 241 groups under `bcfishpass` and stamps them with the run's uid | `fresh_default.log` for the run: `config_name` = `default` only | **Fix:** refuse unless the run's log names exactly `$CONFIG` | +| A1 | "Closed row for run_uid" ≠ "these rows are this run's" when a later run rebuilt the group, or a hand re-run under the same uid failed after pre-persist | 241 groups in `fresh_default` have a newest row that is closed and from this run | **Fix:** a group counts as finished only if its newest `log` row (max `date_start`, the same rule `schema_consolidate.R` uses) is closed and has this run_uid; signals still add. Fixture cases for both. | +| Gap 1 | The log-derived expected set misses groups that failed before the log opened, and whole runs that died before `run_finish` | — | Deferred: `--wsgs=` covers it; follow-up issue (persist the run's group list at bucket time) | +| Gap 2 | A `--wsgs=` subset leaves upstream groups unsettled | — | **Fix:** WARN when `--wsgs=` is a strict subset of the run's log groups | +| Gap 3 | A compare failure exits before the report, hiding the missing-group names | — | **Fix:** flag it and fall through to the report | +| Gap 4 | Discharge / wet-load resolution runs before the recompute-only branch and can FATAL a recompute that never reads discharge | `wsg_recompute_one.R` reads no discharge | **Fix:** gate on `RECOMPUTE_ONLY=0` | +| Gap 5 | Driver glue (expected minus SKIP, signals) is untested inline code | — | **Fix:** `run_expected_from_hosts` in the lib, tested with plain maps. Subtract only SKIPs no host finished; WARN (drift), not note. | +| Gap 6 | Coverage leaves no artifact | — | **Fix:** `${TS}_coverage.tsv` (wsg, class) | +| Gap 7 | `study_area_verify.sql` stays red after the recovery for a group whose failed attempt left an open row on the dispatcher | — | **Doc:** RUNBOOK §8d says so | +| Gap 8 | Recovery command omits `LNK_GUARD_DOWNSTREAM=warn` and `LNK_RUN_LABEL` | `wsg_run_one.R` guard defaults to `error` | **Fix** | +| Gap 9 | Recompute-only skips all pre-flight | — | Partial: add the `fresh.streams_vw_bcfp` check (the compare needs it). SHA exports fall back to the local checkouts, the same source the pre-flight reads; dirty check not added. | +| Gap 10 | Nothing stops two finishes on one schema (a paused driver plus `--recompute-only`) | — | **Doc:** RUNBOOK warning; no lock (deferred) | +| Gap 11 | Kept-host recovery text still says "recompute every group … wsg_recompute_one.R" | — | **Fix:** point at `--recompute-only --run-uid=` (finish lib, hosts lib, RUNBOOK) | +| Ordering 1 | Plan says run_finish returns the status; it exits | — | Plan text corrected | +| Ordering 2 | Plan says stubbed pool; tests run the real pool against an Rscript stub | — | Plan text corrected | +| Ordering 3 | Add mutants: newest-row term, empty-set skip | — | **Done** in Phase 4 | +| Scope 1 | Consolidate failure still exits before coverage (same failure class) | — | Banner now prints the `--recompute-only` recovery; follow-up issue for the rest | +| Acceptance 1 | Acceptance says "driver exits"; tests exercise `run_finish` (driver `LOG_DIR` is under the repo) | — | Stated in the plan: the lib-level test stands in for the driver | diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index f388386b..2b9e76da 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -15,7 +15,7 @@ Design: `/Users/airvine/.claude/plans/quiet-hopping-umbrella.md` (approved 2026- - [x] Test: a `done` signal admits a group with rows and no closed row - [x] Test: a missing `streams` table is unknown (non-zero), never "all missing" - [x] Test: `run_expected_from_log` returns AAAA, BBBB, CCCC, EEEE -- [x] Test (acceptance 1): `run_finish` with one missing group calls the stubbed pool and compare with exactly the recompute set, returns 1, and names the missing group +- [x] Test (acceptance 1): `run_finish` with one missing group runs the real pool (an Rscript stub records calls) and the compare over exactly the recompute set, exits 1, and names the missing group. The lib-level test stands in for the driver, whose `LOG_DIR` is under the repo. - [x] Test (acceptance 2): zero groups with rows: the pool and compare are not called, it returns 1, and it says why - [x] Test: the driver refuses `--run-uid` alone, `--recompute-only` alone, a malformed uid, and `--recompute-only --focal=…` (all exit before any Rscript) @@ -33,6 +33,8 @@ Design: `/Users/airvine/.claude/plans/quiet-hopping-umbrella.md` (approved 2026- - [x] Mutation: restore `exit 1` on missing in a copy of the lib (`LNK_TEST_FINISH_LIB`), and confirm acceptance test 1 goes red - [x] Live, read-only: `run_coverage` against `fresh_default` for run_uid `20261017_004619-3f4c713b` with the 241 groups gives 241 recompute and 0 missing (no recompute run) - [ ] `devtools::test()` full suite, `lintr` on the new test file +- [x] Plan review (`review-plan.md`) folded in: config check, newest-row rule, compare failure reaches the report, `run_expected_from_hosts` (tested), coverage TSV, discharge skipped in recompute-only, recovery text, `--wsgs=` subset warning +- [x] Extra mutants: dropping the newest-row term, and the empty-set skip, each turn their test red ## Validation diff --git a/tests/testthat/helper-bash.R b/tests/testthat/helper-bash.R deleted file mode 100644 index 1a2cbce7..00000000 --- a/tests/testthat/helper-bash.R +++ /dev/null @@ -1,20 +0,0 @@ -# Shared by the tests that run data-raw/ bash libs (test-study_area_hosts.R, -# test-study_area_finish.R). - -# The libs need bash >= 4 (assoc arrays); macOS /bin/bash is 3.2. -hosts_bash <- function() { - cands <- unique(c( - "/opt/homebrew/bin/bash", "/usr/local/bin/bash", - unname(Sys.which("bash")) - )) - for (b in cands[nzchar(cands) & file.exists(cands)]) { - v <- suppressWarnings(system2(b, c("-c", shQuote("echo ${BASH_VERSINFO[0]}")), - stdout = TRUE, stderr = FALSE - )) - n <- suppressWarnings(as.integer(v)) - if (length(n) == 1L && !is.na(n) && n >= 4L) { - return(b) - } - } - "" -} diff --git a/tests/testthat/setup.R b/tests/testthat/setup.R index 34b8c8ed..b0062926 100644 --- a/tests/testthat/setup.R +++ b/tests/testthat/setup.R @@ -72,3 +72,23 @@ capture_write <- function(expr, probe_rows = 0L, query_result = NULL) { ) captured } + +# Shared by the tests that run data-raw/ bash libs (test-study_area_hosts.R, +# test-study_area_finish.R). +# The libs need bash >= 4 (assoc arrays); macOS /bin/bash is 3.2. +hosts_bash <- function() { + cands <- unique(c( + "/opt/homebrew/bin/bash", "/usr/local/bin/bash", + unname(Sys.which("bash")) + )) + for (b in cands[nzchar(cands) & file.exists(cands)]) { + v <- suppressWarnings(system2(b, c("-c", shQuote("echo ${BASH_VERSINFO[0]}")), + stdout = TRUE, stderr = FALSE + )) + n <- suppressWarnings(as.integer(v)) + if (length(n) == 1L && !is.na(n) && n >= 4L) { + return(b) + } + } + "" +} diff --git a/tests/testthat/test-study_area_finish.R b/tests/testthat/test-study_area_finish.R index 52934441..f868ee27 100644 --- a/tests/testthat/test-study_area_finish.R +++ b/tests/testthat/test-study_area_finish.R @@ -36,7 +36,9 @@ finish_uid <- "20261018_000000-0000abcd" # A scratch persist schema: the log tables, and a streams table holding only # what coverage reads. AAAA and BBBB finished this run; CCCC opened a log row # this run and failed; DDDD holds an earlier run's rows; EEEE finished this -# run and its rows are gone (lost at consolidate). +# run and its rows are gone (lost at consolidate); HHHH finished this run and +# a later run rebuilt it; IIII finished this run, then a hand re-run under the +# same uid failed after its pre-persist. finish_fixture <- function(conn, env = parent.frame(), streams = TRUE) { psql <- Sys.which("psql") testthat::skip_if(!nzchar(psql), "psql not on PATH") @@ -52,20 +54,30 @@ finish_fixture <- function(conn, env = parent.frame(), streams = TRUE) { DBI::dbExecute(conn, sprintf("DROP SCHEMA IF EXISTS %s CASCADE", sc)), envir = env ) - DBI::dbExecute(conn, sprintf( - "INSERT INTO %s.log (run_id, watershed_group_code, date_start, date_end, run_uid) - VALUES ('a', 'AAAA', '2026-10-18', '2026-10-18', '%s'), - ('b', 'BBBB', '2026-10-18', '2026-10-18', '%s'), - ('c', 'CCCC', '2026-10-18', NULL, '%s'), - ('d', 'DDDD', '2026-08-01', '2026-08-01', '20260801_000000-00000000'), - ('e', 'EEEE', '2026-10-18', '2026-10-18', '%s')", - sc, finish_uid, finish_uid, finish_uid, finish_uid)) + u <- finish_uid + later <- "20261101_000000-0000beef" + sql_log <- sprintf(paste( + "INSERT INTO %s.log", + "(run_id, watershed_group_code, date_start, date_end, run_uid, config_name)", + "VALUES ('a', 'AAAA', '2026-10-18', '2026-10-18', '%s', 'default'),", + "('b', 'BBBB', '2026-10-18', '2026-10-18', '%s', 'default'),", + "('c', 'CCCC', '2026-10-18', NULL, '%s', 'default'),", + "('d', 'DDDD', '2026-08-01', '2026-08-01', '20260801_000000-00000000', 'default'),", + "('e', 'EEEE', '2026-10-18', '2026-10-18', '%s', 'default'),", + "('h1', 'HHHH', '2026-10-18', '2026-10-18', '%s', 'default'),", + "('h2', 'HHHH', '2026-11-01', '2026-11-01', '%s', 'default'),", + "('i1', 'IIII', '2026-10-18', '2026-10-18', '%s', 'default'),", + "('i2', 'IIII', '2026-10-19', NULL, '%s', 'default')" + ), sc, u, u, u, u, u, later, u, u) + DBI::dbExecute(conn, sql_log) if (streams) { DBI::dbExecute(conn, sprintf( - "CREATE TABLE %s.streams (watershed_group_code varchar(4) NOT NULL)", sc)) - DBI::dbExecute(conn, sprintf( - "INSERT INTO %s.streams VALUES ('AAAA'), ('AAAA'), ('BBBB'), ('DDDD'), ('FFFF')", - sc)) + "CREATE TABLE %s.streams (watershed_group_code varchar(4) NOT NULL)", sc + )) + DBI::dbExecute(conn, sprintf(paste( + "INSERT INTO %s.streams VALUES", + "('AAAA'), ('AAAA'), ('BBBB'), ('DDDD'), ('FFFF'), ('HHHH'), ('IIII')" + ), sc)) } sc } @@ -133,13 +145,15 @@ test_that("coverage splits the run's groups by what this run built", { conn <- skip_if_no_db() sc <- finish_fixture(conn) w <- finish_world() - r <- run_finish_sh(w, sc, cov_body("AAAA,BBBB,CCCC,DDDD,EEEE,GGGG")) + r <- run_finish_sh(w, sc, cov_body("AAAA,BBBB,CCCC,DDDD,EEEE,GGGG,HHHH,IIII")) expect_equal(r$status, 0L, info = r$out) expect_identical(printed(r, "REC"), "AAAA,BBBB") expect_identical(printed(r, "NOROWS"), "CCCC,GGGG") - expect_identical(printed(r, "STALE"), "DDDD") + # A group's NEWEST log row decides: HHHH was rebuilt by a later run, and + # IIII's newest build under this uid never closed. + expect_identical(printed(r, "STALE"), "DDDD,HHHH,IIII") expect_identical(printed(r, "LOST"), "EEEE") - expect_identical(printed(r, "MISSING"), "CCCC,DDDD,EEEE,GGGG") + expect_identical(printed(r, "MISSING"), "CCCC,DDDD,EEEE,GGGG,HHHH,IIII") # FFFF has rows and is not a run group: it is in no class. expect_false(grepl("FFFF", r$out)) }) @@ -175,7 +189,7 @@ test_that("--recompute-only's expected set is every group with a log row for the w <- finish_world() r <- run_finish_sh(w, sc, "echo \"EXP=$(run_expected_from_log)\"") expect_equal(r$status, 0L, info = r$out) - expect_identical(printed(r, "EXP"), "AAAA,BBBB,CCCC,EEEE") + expect_identical(printed(r, "EXP"), "AAAA,BBBB,CCCC,EEEE,HHHH,IIII") }) test_that("one missing group: the rest are recomputed and compared, and the run exits non-zero naming it", { @@ -196,7 +210,63 @@ test_that("one missing group: the rest are recomputed and compared, and the run expect_match(compare, " AAAA,BBBB default$") expect_match(r$out, "were NOT recomputed or compared: CCCC", fixed = TRUE) expect_match(r$out, "--recompute-only --run-uid=20261018_000000-0000abcd", fixed = TRUE) + expect_match(r$out, "LNK_GUARD_DOWNSTREAM=warn", fixed = TRUE) expect_match(r$out, "study_area_run INCOMPLETE", fixed = TRUE) + cov <- utils::read.delim(file.path(w, "logs", "T0_coverage.tsv"), colClasses = "character") + expect_identical(cov$class[order(cov$wsg)], c("recompute", "recompute", "no_rows")) +}) + +test_that("a failed compare still reaches the report, which names the missing groups", { + skip_if_no_finish_lib() + conn <- skip_if_no_db() + sc <- finish_fixture(conn) + w <- finish_world() + writeLines(c( + "#!/bin/sh", sprintf('echo "$*" >> "%s/rscript_calls"', w), + 'case "$*" in *study_area_compare*) exit 1 ;; esac', "exit 0" + ), file.path(w, "bin", "Rscript")) + r <- run_finish_sh(w, sc, "run_finish AAAA,BBBB,CCCC ''") + expect_equal(r$status, 1L, info = r$out) + expect_match(r$out, "compare failed", fixed = TRUE) + expect_match(r$out, "were NOT recomputed or compared: CCCC", fixed = TRUE) +}) + +test_that("--recompute-only refuses a config the run was not built with", { + skip_if_no_finish_lib() + conn <- skip_if_no_db() + sc <- finish_fixture(conn) + w <- finish_world() + ok <- run_finish_sh(w, sc, "run_config_check") + expect_equal(ok$status, 0L, info = ok$out) + bad <- run_finish_sh(w, sc, "CONFIG=bcfishpass; run_config_check") + expect_equal(bad$status, 1L) + expect_match(bad$out, "built with config 'default', not --config=bcfishpass", fixed = TRUE) + DBI::dbExecute(conn, sprintf( + "UPDATE %s.log SET config_name = 'default_tuned' WHERE run_id = 'b'", sc + )) + mixed <- run_finish_sh(w, sc, "run_config_check") + expect_equal(mixed$status, 1L) + expect_match(mixed$out, "'default,default_tuned'", fixed = TRUE) +}) + +test_that("a full run expects every bucket less SKIPs no host finished, with every host's signals", { + skip_if_no_finish_lib() + w <- finish_world() + r <- run_finish_sh(w, "unused", c( + "DISP_BUCKET=AAAA,BBBB,SSSS; CY_WS_ARR=(a b)", + "CY_BUCKET[a]=CCCC,DDDD; CY_BUCKET[b]=EEEE,TTTT", + "ACC_FINISHED[dispatcher]=AAAA; ACC_DONE_LOG[dispatcher]=BBBB", + "ACC_SKIP[dispatcher]=SSSS; ACC_SKIP[a]=EEEE; ACC_SKIP[b]=TTTT", + "CY_BUCKET_DONE[a]=CCCC; CY_BUCKET_DONE[b]=EEEE", + "run_expected_from_hosts", + "echo \"EXP=$RUN_EXPECT\"; echo \"SIG=$RUN_SIGNALS\"; echo \"SKIP=$RUN_SKIP\"" + )) + expect_equal(r$status, 0L, info = r$out) + # EEEE was SKIPped by a, which does not run it, and finished by b: kept. + expect_identical(printed(r, "EXP"), "AAAA,BBBB,CCCC,DDDD,EEEE") + expect_identical(printed(r, "SIG"), "AAAA,BBBB,CCCC,EEEE") + expect_identical(printed(r, "SKIP"), "SSSS,TTTT") + expect_match(r$out, "WARN: SKIPped by a host", fixed = TRUE) }) test_that("no group with rows from this run: no recompute, no compare, and it says why", { From 5709d87bcf210231c63bdb572f57b06edf5edb0f Mon Sep 17 00:00:00 2001 From: almac2022 Date: Sun, 18 Oct 2026 21:20:26 -0700 Subject: [PATCH 06/10] Code-check round 1: kept-host groups are consolidated, not re-modelled; carry --run-label (#348) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WJE2szSCkeV9R3qKhuSwyx --- RUNBOOK.md | 9 +++- data-raw/study_area_finish.sh | 37 +++++++++++---- data-raw/study_area_hosts.sh | 2 +- data-raw/study_area_run.sh | 2 +- planning/active/review-round1.md | 63 +++++++++++++++++++++++++ tests/testthat/test-study_area_finish.R | 17 +++++++ 6 files changed, 117 insertions(+), 13 deletions(-) create mode 100644 planning/active/review-round1.md diff --git a/RUNBOOK.md b/RUNBOOK.md index a93526bc..f5138692 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -1253,7 +1253,7 @@ makes the run exit non-zero, and the report names the groups and the recovery: LNK_RUN_UID= LNK_SCHEMA= LNK_LOAD=loadall \ Rscript data-raw/wsg_run_one.R # each missing group bash data-raw/study_area_run.sh --recompute-only --run-uid= \ - --config= --schema= [--wsgs=] + --config= --schema= [--run-label=