From 50b5b07e200042f12a544a5bace9361f4b40a48b Mon Sep 17 00:00:00 2001 From: almac2022 Date: Thu, 27 Aug 2026 11:32:34 -0700 Subject: [PATCH] Purge stale public.wsg_outlet references; document the ltree-closure trap #227/v0.45.1 removed public.wsg_outlet as a concept, but references survived in places that still steer behaviour: - R/lnk_log.R listed public.wsg_outlet as a pipeline input primitive, so the run-provenance log (v0.45.0) fingerprints a table that should not exist. On a machine with the leftover table it records real numbers for a phantom dependency; on a clean one it records NULLs for a table that was never a dependency. Removed. - research/study_areas.md documented the *superseded* technique as current -- "closure and downstream-first order are derived from wscode_ltree ancestry (@>)" -- and pointed at #227 as the issue tracking its reproducible build, when #227 was re-scoped to delete it. Rewritten to point at lnk_wsg_resolve() with an explicit do-not. - research/study_area_run.md and data-raw/study_area_run.sh referenced the table in prose. Repointed. data-raw/study_area_wsgs.R was already a shim around lnk_wsg_resolve(), so no live code path was wrong. Adds RUNBOOK section 8b, which did not previously warn about any of this. The trap is that two watershed groups on the same stream share an outlet code, so `a.outlet @> b.outlet` is true in both directions and reports each as downstream of the other. Worked example is the Kootenay, where FWA carries the out-and-back through Montana and Idaho under one continuous wscode -- an ltree test puts KOTR/SMAR/BULL below Kootenay Lake when they are above it via the US, and the true closure of LARL/KOTL/SLOC is just those three. Found by falling into it: a hand-rolled @> query produced a five-WSG "outstanding downstream closure" that does not exist. --- R/lnk_log.R | 6 ++---- RUNBOOK.md | 37 +++++++++++++++++++++++++++++++++++++ data-raw/study_area_run.sh | 2 +- research/study_area_run.md | 2 +- research/study_areas.md | 8 +++++--- 5 files changed, 46 insertions(+), 9 deletions(-) diff --git a/R/lnk_log.R b/R/lnk_log.R index 220454d7..f296e92f 100644 --- a/R/lnk_log.R +++ b/R/lnk_log.R @@ -136,16 +136,14 @@ "bcfishobs.observations", "whse_fish.pscis_assessment_svw", "cabd.dams", - "fresh.modelled_stream_crossings", - "public.wsg_outlet" + "fresh.modelled_stream_crossings" ), source = c( rep(fwapg, 7L), "bcfishobs", "bcdata bc2pg", "CABD", - "snapshot_bcfp.sh <- bchamp objectstore", - "ad-hoc (link#227)" + "snapshot_bcfp.sh <- bchamp objectstore" ), stringsAsFactors = FALSE ) diff --git a/RUNBOOK.md b/RUNBOOK.md index 5aa214c5..35b8a1dd 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -519,6 +519,43 @@ per-source-per-species column + `lnk_barriers_unify` change — not a tweak. --- +## 8b. Drainage closure: never hand-roll it from ltree + +**Use `lnk_wsg_resolve(cfg, loaded, wsgs, conn = conn)`.** It delegates to +`fresh::frs_wsg_drainage()` (fresh >= 0.33.0), which tests per-group outlet +**points** (`blue_line_key` + `downstream_route_measure`) with the measure-aware +`whse_basemapping.fwa_downstream()`. + +**Do not** compute closure from `wscode_ltree` ancestry (`a.outlet @> b.outlet`). +That was the pre-#227 method and it silently over-includes, because **two watershed +groups on the same stream share an outlet code** — so `@>` is true in *both* +directions and calls each one downstream of the other. Closure is measure-aware, +not code-aware. This is why `c("PARS","BULK")` dropped from 15 WSGs to 9 when +#238 adopted fresh 0.33.0. + +**Worked example — the Kootenay, where it bites hardest.** FWA carries the whole +river under one continuous `wscode_ltree = 300.625474`, including the stretch that +leaves BC near Newgate, runs through Montana and Idaho, and re-enters at Creston. +Measures chain with no gap: + +``` +LARL 0–130 → KOTL 130–431,808 → BULL 431,808–495,206 → SMAR 495,206–608,735 → KOTR 608,735–773,149 +``` + +So KOTR/SMAR/BULL are **upstream** of Kootenay Lake, reached via the US loop — but +an ltree test sees them sharing KOTL's outlet code and reports them as downstream. +The correct closure of `c("LARL","KOTL","SLOC")` is **just those three**: LARL is +the terminal BC group (it holds the Kootenay's mouth at Castlegar and the Columbia +down to the border — hence Waneta and Seven Mile sitting in it), and below it is +the United States. + +**`public.wsg_outlet` is gone as a concept** (#227). If you find the table in a +database it is a leftover from before fresh 0.33.0 — it still answers queries, and +it answers them wrongly. Outlets now ship in fresh at `inst/extdata/wsg_outlet.csv` +and reach the DB as a `VALUES` list; no table is needed anywhere. + +--- + ## 8. Fast verification recipes ```bash diff --git a/data-raw/study_area_run.sh b/data-raw/study_area_run.sh index 04b2de5f..1f4dd8a3 100755 --- a/data-raw/study_area_run.sh +++ b/data-raw/study_area_run.sh @@ -14,7 +14,7 @@ # # Cross-WSG `;DAM` correctness WITHOUT a post-consolidate recompute: each # host gets a DRAINAGE-CLOSED bucket (focal WSGs + every WSG they drain -# through, via study_area_wsgs.R / public.wsg_outlet) run DOWNSTREAM-FIRST, +# through, via study_area_wsgs.R / lnk_wsg_resolve) run DOWNSTREAM-FIRST, # so a WSG's downstream dam barriers are persisted before its access / # mapping_code is computed. One study area (closed) per host. # diff --git a/research/study_area_run.md b/research/study_area_run.md index c851de04..329a19ea 100644 --- a/research/study_area_run.md +++ b/research/study_area_run.md @@ -30,7 +30,7 @@ cyphers in order. Dispatcher-only (no cyphers): omit `--cy-workspaces`, pass one 1. **Pre-flight** (tunnel-free): local fwapg up, `fresh.streams_vw_bcfp` present, doctl/tofu (only if cyphers). 2. **Drainage-closed DS-first buckets** (`study_area_wsgs.R`): each focal set → - its closure (every WSG it drains through, via `public.wsg_outlet`, + its closure (every WSG it drains through, via `lnk_wsg_resolve()`, `f.outlet <@ w.outlet`) ordered downstream-first (`nlevel(outlet) ASC`), then **filtered to bundle-species presence** (link#157). 3. **Spin + prep** cyphers (`cypher_up.sh`, `cypher_prep.sh` with diff --git a/research/study_areas.md b/research/study_areas.md index ee180081..2f65e600 100644 --- a/research/study_areas.md +++ b/research/study_areas.md @@ -19,9 +19,11 @@ The 3 FWCP regions and their watershed groups — the parity-run scope for link 29 focal WSGs → **~52 with downstream-closure** (every WSG each drains through — e.g. PARS → PCEA / UPCE / LPCE / FINA / PARA / LBTN). Closure + downstream-first -order are derived from `wscode_ltree` ancestry (`@>`) via the `public.wsg_outlet` -helper table (a per-WSG outlet `wscode_ltree`; see the Database Connection section -of `CLAUDE.md` and issue #227 for its reproducible build). Major drainages by root +order come from `lnk_wsg_resolve()`, which delegates to `fresh::frs_wsg_drainage()` +(fresh >= 0.33.0). **Do not hand-roll this from `wscode_ltree` ancestry (`@>`)** — +that was the pre-#227 method and it over-includes: two WSGs on the same stream share +an outlet code, so `@>` calls each downstream of the other. The closure is +measure-aware, not code-aware. See RUNBOOK section 8b. Major drainages by root wscode: Fraser `100` (68 WSGs), Peace `200` (65), Columbia `300`/ELKR (17 — NOT a study area), Skeena `400` (12).