Skip to content

Commit a657b29

Browse files
author
Mike Johnson
committed
invariants: guard hydroseq routing-order completeness (audit finding)
Add .hf_hydroseq_check (flags >0.5% NA or any duplicate hydroseq), wired into the ngen and merge invariants. Audit finding: hydroseq is 0% NA per-VPU but the global merge leaves ~1.6% NA (cross-VPU / terminal reaches left unordered) -- the same silent-degradation class as the mainstem_id drop. Tests cover NA / duplicate / clean. Root-causing the merge's hydroseq recompute is a follow-up; this guard makes it visible.
1 parent cada8a3 commit a657b29

2 files changed

Lines changed: 36 additions & 0 deletions

File tree

R/hf_invariants.R

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ hf_check_invariants <- function(stage, ..., strict = TRUE,
265265
checks$ngen_fp_prefix <- .hf_ok(n_bad == 0L,
266266
sprintf("%d ngen flowpath_id(s) do not start with 'fp-'", n_bad))
267267
checks$mainstem_id_populated <- .hf_mainstem_check(flowpaths)
268+
checks$hydroseq_valid <- .hf_hydroseq_check(flowpaths)
268269
}
269270

270271
if (!is.null(divides)) {
@@ -344,6 +345,22 @@ hf_check_invariants <- function(stage, ..., strict = TRUE,
344345
if (frac > 0.05) " -- dropped/mis-mapped?" else ""))
345346
}
346347

348+
# Shared hydroseq integrity check. hydroseq is the topological routing order:
349+
# every flowpath needs one (per-VPU outputs are 0% NA) and they must be unique.
350+
# A NA fraction flags reaches left unordered (e.g. the global merge leaving
351+
# cross-VPU/terminal reaches without a recomputed hydroseq).
352+
.hf_hydroseq_check <- function(flowpaths) {
353+
if (!"hydroseq" %in% names(flowpaths)) {
354+
return(.hf_info("no hydroseq column on flowpaths"))
355+
}
356+
h <- flowpaths$hydroseq
357+
frac_na <- mean(is.na(h))
358+
n_dup <- sum(duplicated(h[!is.na(h)]))
359+
.hf_ok(frac_na <= 0.005 && n_dup == 0L,
360+
sprintf("hydroseq: %.2f%% NA, %d duplicate(s)%s", 100 * frac_na, n_dup,
361+
if (frac_na > 0.005 || n_dup > 0L) " -- routing order incomplete" else ""))
362+
}
363+
347364
.hf_ring_counts <- function(geoms) {
348365
vapply(geoms, function(g) {
349366
if (length(g) == 0L || sf::st_is_empty(g)) return(0L)
@@ -512,6 +529,8 @@ hf_check_merge_invariants <- function(merged, expected = NULL,
512529

513530
# mainstem_id (a global reference levelpath id) must survive the merge intact.
514531
checks$mainstem_id_populated <- .hf_mainstem_check(fp)
532+
# hydroseq routing order must be complete + unique after the global recompute.
533+
checks$hydroseq_valid <- .hf_hydroseq_check(fp)
515534

516535
# single CRS across all merged layers
517536
crs_of <- function(x) if (inherits(x, "sf")) sf::st_crs(x)$epsg else NA

tests/testthat/test-invariants.R

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,23 @@ test_that("ngen stage flags dropped/mis-mapped mainstem_id", {
7474
expect_true(res2$checks$mainstem_id_populated$ok)
7575
})
7676

77+
test_that("ngen stage flags incomplete/duplicate hydroseq routing order", {
78+
na_fp <- data.frame(flowpath_id = paste0("fp-", 1:10), flowpath_toid = "0",
79+
hydroseq = c(1:5, rep(NA, 5)), stringsAsFactors = FALSE) # 50% NA
80+
expect_false(suppressMessages(hf_check_invariants("ngen", flowpaths = na_fp,
81+
strict = FALSE))$checks$hydroseq_valid$ok)
82+
83+
dup_fp <- data.frame(flowpath_id = paste0("fp-", 1:5), flowpath_toid = "0",
84+
hydroseq = c(1, 2, 2, 3, 4), stringsAsFactors = FALSE) # duplicate
85+
expect_false(suppressMessages(hf_check_invariants("ngen", flowpaths = dup_fp,
86+
strict = FALSE))$checks$hydroseq_valid$ok)
87+
88+
ok_fp <- data.frame(flowpath_id = paste0("fp-", 1:5), flowpath_toid = "0",
89+
hydroseq = 1:5, stringsAsFactors = FALSE)
90+
expect_true(suppressMessages(hf_check_invariants("ngen", flowpaths = ok_fp,
91+
strict = FALSE))$checks$hydroseq_valid$ok)
92+
})
93+
7794
test_that("aggregated stage catches duplicate flowpath_id", {
7895
fp <- data.frame(flowpath_id = c("1", "1"), stringsAsFactors = FALSE)
7996
expect_error(

0 commit comments

Comments
 (0)