From 1dc114557909e60858d26dbbba533352231a5a89 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Wed, 27 May 2026 02:54:35 -0700 Subject: [PATCH 1/6] Initialize PWF baseline for #211 Co-Authored-By: Claude Opus 4.7 (1M context) --- planning/active/findings.md | 97 ++++++++++++++++++++++++++++++++++++ planning/active/progress.md | 8 +++ planning/active/task_plan.md | 58 +++++++++++++++++++++ 3 files changed, 163 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 0000000..2fe5335 --- /dev/null +++ b/planning/active/findings.md @@ -0,0 +1,97 @@ +# Findings — frs_wsg_drainage (#211) + +## Issue context + +### Problem + +`link` currently computes WSG drainage closure (every WSG a focal set drains through, downstream-first) inline in `data-raw/study_area_wsgs.R` — see `NewGraphEnvironment/link@v0.40.5`. The query reads `public.wsg_outlet`, joins focal → closure via `f.outlet <@ w.outlet`, sorts by `nlevel(outlet) ASC`. + +This is FWA-topology — it belongs in `fresh`, not `link`. No bundle/species/overrides knowledge involved; pure network shape. + +### Proposed + +```r +frs_wsg_drainage(conn, wsgs) +``` + +- `wsgs`: character vector of WSG codes (the focal set) +- Returns: character vector covering the drainage closure (focal + every WSG they flow through), ordered downstream-first (`nlevel(outlet) ASC`) +- Pure FWA topology — no bundle / species / overrides logic + +### Where it lives + +`R/frs_wsg_drainage.R`. Sits alongside `frs_wsg_species()` as part of a `frs_wsg_*` family — the WSG-topology surface of fresh. + +### Why fresh, not link + +`wsg_outlet` is FWA infrastructure. Drainage closure is a network-shape question with no link/bundle knowledge. Other fresh consumers (vignettes, ad-hoc analyses, future provincial drivers in other packages) want this without going through link. + +### Acceptance + +- [ ] `frs_wsg_drainage(conn, c("PARS","BULK"))` returns the 15-WSG Skeena+Peace closure: `KISP, KLUM, LKEL, LSKE, MSKE, USKE, BULK, FINA, LBTN, LPCE, MORR, PARA, PCEA, UPCE, PARS` — matches `NewGraphEnvironment/link@v0.40.5` `study_area_wsgs.R` output +- [ ] DS-first ordering preserved +- [ ] Closure invariant to focal ordering +- [ ] Runnable `@example` +- [ ] Test against a small live focal set + +### Replaces + +Inline closure query in `NewGraphEnvironment/link@v0.40.5` `data-raw/study_area_wsgs.R` (lines 44-50 computing `wsg_outlet`-based closure). + +### Composes with + +`link::lnk_wsg_resolve` (`NewGraphEnvironment/link#207`) — the link wrapper adds the bundle's species-presence filter (#157). + +### Naming considered + +`frs_wsg_drainage` (chosen) is `frs_wsg_*`-family consistent. `wsg` is FWA-specific terminology — honest to fresh's current FWA-only scope. If fresh ever grows a second network (NHD/HUC), a `network = "fwa"` param can be introduced then (YAGNI today). + +## Codebase exploration + +### Family conventions + +- One existing `frs_wsg_*` function: `R/frs_wsg_species.R:39` — `frs_wsg_species(watershed_group_code)`. Loads a bundled CSV; no DB connection. Validation via `stopifnot()`. Returns data frame. +- `@family` tags in use: `parameters` (frs_wsg_species), `fetch` (frs_stream_fetch), `traverse` (frs_network_downstream), `index` (frs_point_snap), `network` (frs_network_features, frs_point_match, frs_candidates_pick), `database` (frs_db_conn). No existing `wsg` family. + +### DB-issuing function pattern (frs_stream_fetch) + +`R/frs_stream_fetch.R:42-84` — signature is `function(conn, , , table = "...", cols = c(...), limit = NULL)`. Validates identifiers via `.frs_validate_identifier()`. Composes SQL with `paste0` + `sprintf`. Calls `frs_db_query(conn, sql)` to execute. + +### SQL idiom + +- `sprintf()` + `paste0()` dominate fresh; no `glue`. Helpers in `R/utils.R`: `.frs_quote_string` (line 39, manual escape), `.frs_validate_identifier` (line 54), `.frs_sql_num` (line 71). +- `DBI::dbQuoteLiteral` is the safest path for user-supplied string vectors — link uses it in `study_area_wsgs.R:44`. + +### Test pattern + +DB-gated tests skip on missing `PG_DB_SHARE`: +```r +skip_if(Sys.getenv("PG_DB_SHARE") == "", "PG_DB_SHARE not set") +conn <- frs_db_conn() +on.exit(DBI::dbDisconnect(conn)) +``` +Helper: `frs_db_conn()` (`R/frs_db_conn.R:24-44`) reads `PG_*_SHARE` env vars. Mocking via `mockery::local_mocked_bindings()` to capture SQL without a DB. + +### Example convention + +`\dontrun{}` for DB-requiring examples (per `R/frs_stream_fetch.R:30-41`); inline for CSV-loaded functions (per `R/frs_wsg_species.R:29-38`). + +### Existing `wsg_outlet` usage + +Zero references in fresh codebase (R + SQL + tests). This function is the first consumer. + +### NEWS.md style + +`# fresh X.Y.Z` section header, lead with bold one-liner + issue link, then bullets. Reference: `NEWS.md:1-10` (v0.31.0). + +### Source SQL to port (from `link@v0.40.5 data-raw/study_area_wsgs.R:44-50`) + +```sql +SELECT DISTINCT w.wsg, nlevel(w.outlet) AS depth +FROM public.wsg_outlet w +JOIN public.wsg_outlet f ON f.wsg IN () +WHERE f.outlet <@ w.outlet +ORDER BY depth ASC, w.wsg ASC +``` + +Column name `wsg` to be confirmed in Phase 1. diff --git a/planning/active/progress.md b/planning/active/progress.md new file mode 100644 index 0000000..939066a --- /dev/null +++ b/planning/active/progress.md @@ -0,0 +1,8 @@ +# Progress — frs_wsg_drainage (#211) + +## Session 2026-05-27 + +- Plan-mode exploration — phases approved by user +- Created branch `211-frs-wsg-drainage-fwa-wsg-drainage-closur` off main (off `90af475`) +- Scaffolded PWF baseline from issue #211 with approved phases +- Next: start Phase 1 — confirm `public.wsg_outlet` column name on live DB, run baseline query diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md new file mode 100644 index 0000000..8c14e5c --- /dev/null +++ b/planning/active/task_plan.md @@ -0,0 +1,58 @@ +# Task: frs_wsg_drainage — FWA WSG drainage-closure primitive (#211) + +## Problem + +`link` currently computes WSG drainage closure (every WSG a focal set drains through, downstream-first) inline in `data-raw/study_area_wsgs.R` — see `NewGraphEnvironment/link@v0.40.5`. The query reads `public.wsg_outlet`, joins focal → closure via `f.outlet <@ w.outlet`, sorts by `nlevel(outlet) ASC`. + +This is FWA-topology — it belongs in `fresh`, not `link`. No bundle/species/overrides knowledge involved; pure network shape. Replacing the inline query unblocks `NewGraphEnvironment/link#207` (`lnk_wsg_resolve`), which composes this primitive with the bundle's species-presence filter. + +## Approach + +New exported function `frs_wsg_drainage(conn, watershed_group_code, table = "public.wsg_outlet")` returning a character vector of WSG codes (focal + every WSG they drain through), ordered downstream-first by `nlevel(outlet) ASC`. + +Param name `watershed_group_code` (not `wsgs`) for fresh-internal vocabulary consistency with `frs_wsg_species` + `frs_stream_fetch`. SQL ported verbatim from the validated query in `NewGraphEnvironment/link@v0.40.5 data-raw/study_area_wsgs.R:44-50`. SQL composition via `sprintf()` + `DBI::dbQuoteLiteral`. Identifier validation via `.frs_validate_identifier()` (`R/utils.R:54`). `@family wsg` (new family; retag of `frs_wsg_species` deferred). Live DB test gated on `Sys.getenv("PG_DB_SHARE")`. + +## Phase 1 — Live-DB column verification + +- [ ] Confirm actual column name in `public.wsg_outlet` (`wsg` vs `watershed_group_code`) via `\d public.wsg_outlet` on the live DB so the SQL matches reality +- [ ] Run the unmodified link query against current fwapg to confirm PARS+BULK → 15 WSGs (the regression baseline before any code lands) + +## Phase 2 — Function + roxygen + +- [ ] Write `R/frs_wsg_drainage.R` with signature, arg validation (`stopifnot` + `.frs_validate_identifier`), SQL via `sprintf` + `DBI::dbQuoteLiteral`, `\dontrun{}` example showing PARS+BULK, `@family wsg`, `@export` +- [ ] `devtools::document()` → regenerate `NAMESPACE` + `man/frs_wsg_drainage.Rd` +- [ ] `/code-check` clean → atomic commit (function + checkbox flip) + +## Phase 3 — Tests + +- [ ] `tests/testthat/test-frs_wsg_drainage.R`: + - Arg validation: non-character / empty / `NA` input rejected; invalid `table` rejected by `.frs_validate_identifier` + - DB-gated (skip on missing `PG_DB_SHARE` per `R/frs_db_conn.R:31`) live test: `frs_wsg_drainage(conn, c("PARS","BULK"))` returns the 15-WSG closure in DS-first order + - Ordering invariance: result equal for `c("BULK","PARS")` vs `c("PARS","BULK")` +- [ ] `devtools::test()` green +- [ ] `/code-check` clean → atomic commit + +## Phase 4 — Release + +- [ ] `NEWS.md`: new `# fresh 0.32.0` section, lead with bold one-liner + bullets (composition with `lnk_wsg_resolve`, pure-topology framing, test count) — matches existing style (`NEWS.md:1-10`) +- [ ] `DESCRIPTION`: `Version: 0.32.0`, `Date: ` +- [ ] `lintr::lint_package()` clean +- [ ] `/code-check` clean → atomic commit `"Release v0.32.0"` as final commit of branch +- [ ] `/planning-archive` → `/gh-pr-push` (PR body: `Closes #211` + `Relates to NewGraphEnvironment/sred-2025-2026#24`) + +## Validation + +- [ ] `devtools::test()` green +- [ ] Live `frs_wsg_drainage(conn, c("PARS","BULK"))` returns exactly: `KISP, KLUM, LKEL, LSKE, MSKE, USKE, BULK, FINA, LBTN, LPCE, MORR, PARA, PCEA, UPCE, PARS` (15 WSGs) +- [ ] Closure invariant to focal ordering +- [ ] `lintr::lint_package()` clean +- [ ] pkgdown reference page renders the new function with runnable `\dontrun{}` example +- [ ] `/code-check` clean on each commit +- [ ] PWF checkboxes match landed work +- [ ] `/planning-archive` on completion + +## Out of scope + +- Retagging `frs_wsg_species` from `@family parameters` → `@family wsg` (separate concern, follow-up) +- Consuming the new function in link (`lnk_wsg_resolve` — covered by `NewGraphEnvironment/link#207`) +- Adding a `network = "fwa"` param for future non-FWA networks (YAGNI per the issue body) From 808458e951a170cc08d7d167a191ec73cbcb8abd Mon Sep 17 00:00:00 2001 From: almac2022 Date: Wed, 27 May 2026 03:47:32 -0700 Subject: [PATCH 2/6] Phase 1: verify public.wsg_outlet schema + regression baseline (#211) - Column confirmed: wsg varchar(4) (matches link's existing query) - Live PARS+BULK closure returns 15 WSGs DS-first, exact regression baseline match Co-Authored-By: Claude Opus 4.7 (1M context) --- planning/active/progress.md | 5 +++-- planning/active/task_plan.md | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/planning/active/progress.md b/planning/active/progress.md index 939066a..e3e630b 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -4,5 +4,6 @@ - Plan-mode exploration — phases approved by user - Created branch `211-frs-wsg-drainage-fwa-wsg-drainage-closur` off main (off `90af475`) -- Scaffolded PWF baseline from issue #211 with approved phases -- Next: start Phase 1 — confirm `public.wsg_outlet` column name on live DB, run baseline query +- Scaffolded PWF baseline from issue #211 with approved phases (commit `1dc1145`) +- **Phase 1 complete:** `public.wsg_outlet` column confirmed as `wsg varchar(4)` (with `outlet ltree`, `lvl integer`); regression baseline run — PARS+BULK → 15 WSGs, exact match: `KISP, KLUM, LKEL, LSKE, MSKE, USKE, BULK, FINA, LBTN, LPCE, MORR, PARA, PCEA, UPCE, PARS` (DS-first, depths 1×6, 2×8, 3×1) +- Next: Phase 2 — write `R/frs_wsg_drainage.R` diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index 8c14e5c..f81a6bb 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -14,8 +14,8 @@ Param name `watershed_group_code` (not `wsgs`) for fresh-internal vocabulary con ## Phase 1 — Live-DB column verification -- [ ] Confirm actual column name in `public.wsg_outlet` (`wsg` vs `watershed_group_code`) via `\d public.wsg_outlet` on the live DB so the SQL matches reality -- [ ] Run the unmodified link query against current fwapg to confirm PARS+BULK → 15 WSGs (the regression baseline before any code lands) +- [x] Confirm actual column name in `public.wsg_outlet` (`wsg` vs `watershed_group_code`) via `\d public.wsg_outlet` on the live DB so the SQL matches reality — **`wsg` varchar(4)** confirmed; also has `outlet ltree` (GIST) + `lvl integer` +- [x] Run the unmodified link query against current fwapg to confirm PARS+BULK → 15 WSGs (the regression baseline before any code lands) — **15/15 match**: `KISP, KLUM, LKEL, LSKE, MSKE, USKE, BULK, FINA, LBTN, LPCE, MORR, PARA, PCEA, UPCE, PARS` (DS-first, depths 1/1/1/1/1/1/2/2/2/2/2/2/2/2/3) ## Phase 2 — Function + roxygen From 157d03efbdadd84d80a32fb1ee2f6618e6aec7bc Mon Sep 17 00:00:00 2001 From: almac2022 Date: Wed, 27 May 2026 05:22:21 -0700 Subject: [PATCH 3/6] Phase 2: frs_wsg_drainage function + roxygen (#211) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure FWA topology primitive — given a focal set of WSG codes, returns the drainage closure (focal + every WSG they flow through) ordered downstream-first by outlet ltree depth (`nlevel(outlet) ASC`). SQL ported from link@v0.40.5 data-raw/study_area_wsgs.R; identifier validated, focal values quoted via DBI::dbQuoteLiteral, scalar table arg enforced, focal codes upper-cased internally, unmatched focals warn rather than silently drop. Live-validated against fwapg: PARS+BULK returns the exact 15-WSG closure (KISP, KLUM, LKEL, LSKE, MSKE, USKE, BULK, FINA, LBTN, LPCE, MORR, PARA, PCEA, UPCE, PARS) DS-first; lowercase input normalized; TYPO warns. Co-Authored-By: Claude Opus 4.7 (1M context) --- NAMESPACE | 1 + R/frs_wsg_drainage.R | 91 ++++++++++++++++++++++++++++++++++++ man/frs_wsg_drainage.Rd | 53 +++++++++++++++++++++ planning/active/progress.md | 5 +- planning/active/task_plan.md | 6 +-- 5 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 R/frs_wsg_drainage.R create mode 100644 man/frs_wsg_drainage.Rd diff --git a/NAMESPACE b/NAMESPACE index 2c8f50a..8440e66 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -44,6 +44,7 @@ export(frs_waterbody_network) export(frs_watershed_at_measure) export(frs_watershed_split) export(frs_wetland_fetch) +export(frs_wsg_drainage) export(frs_wsg_species) importFrom(DBI,dbConnect) importFrom(DBI,dbDisconnect) diff --git a/R/frs_wsg_drainage.R b/R/frs_wsg_drainage.R new file mode 100644 index 0000000..15b2d6a --- /dev/null +++ b/R/frs_wsg_drainage.R @@ -0,0 +1,91 @@ +#' WSG Drainage Closure (FWA Topology) +#' +#' Given a focal set of FWA watershed groups, return the **drainage closure** +#' — every WSG that any focal WSG drains through — ordered downstream-first +#' by outlet ltree depth (`nlevel(outlet) ASC`). Pure FWA topology; no +#' species or bundle knowledge. +#' +#' Sources the FWA WSG outlet table (default `public.wsg_outlet` in fwapg). +#' The closure predicate is `f.outlet <@ w.outlet` — i.e. every WSG whose +#' outlet ltree is an ancestor of (or equal to) any focal WSG's outlet. +#' +#' Order: `depth ASC, wsg ASC` — so the most downstream WSGs come first and +#' ties within a depth are alphabetical. Running per-WSG work in this order +#' persists downstream barriers before upstream WSGs read them, which makes +#' cross-WSG accessibility flags settle correctly within a single pass. +#' +#' @param conn A [DBI::DBIConnection-class] (e.g. from [frs_db_conn()]). +#' @param watershed_group_code Character vector of focal WSG codes (e.g. +#' `c("BULK", "MORR")`). Must be non-empty and free of `NA`. Codes are +#' upper-cased internally. Focal codes that don't exist in `table` +#' trigger a warning and are dropped from the result; if no codes match, +#' the function errors. +#' @param table Character scalar. Fully-qualified table name holding the +#' WSG outlet ltree topology. Default `"public.wsg_outlet"`. +#' +#' @return Character vector of WSG codes — focal plus every WSG they flow +#' through — ordered downstream-first. +#' +#' @family wsg +#' +#' @export +#' +#' @examples +#' \dontrun{ +#' conn <- frs_db_conn() +#' +#' # Skeena + Peace focal — returns the 15-WSG drainage closure DS-first +#' frs_wsg_drainage(conn, c("PARS", "BULK")) +#' #> [1] "KISP" "KLUM" "LKEL" "LSKE" "MSKE" "USKE" "BULK" "FINA" +#' #> "LBTN" "LPCE" "MORR" "PARA" "PCEA" "UPCE" "PARS" +#' +#' DBI::dbDisconnect(conn) +#' } +frs_wsg_drainage <- function( + conn, + watershed_group_code, + table = "public.wsg_outlet" +) { + stopifnot( + is.character(watershed_group_code), + length(watershed_group_code) > 0, + !anyNA(watershed_group_code), + all(nzchar(watershed_group_code)), + is.character(table), + length(table) == 1L + ) + .frs_validate_identifier(table, "table") + focal <- toupper(watershed_group_code) + + focal_lit <- paste( + DBI::dbQuoteLiteral(conn, focal), + collapse = ", " + ) + sql <- sprintf( + paste0( + "SELECT DISTINCT w.wsg, nlevel(w.outlet) AS depth\n", + "FROM %s w\n", + "JOIN %s f ON f.wsg IN (%s)\n", + "WHERE f.outlet <@ w.outlet\n", + "ORDER BY depth ASC, w.wsg ASC" + ), + table, table, focal_lit + ) + res <- DBI::dbGetQuery(conn, sql) + if (nrow(res) == 0L) { + stop( + "No drainage closure found — are the focal WSG codes present in `", + table, "`?", + call. = FALSE + ) + } + missing <- setdiff(focal, res$wsg) + if (length(missing) > 0L) { + warning( + "Focal WSG code(s) not found in `", table, "` (dropped from closure): ", + paste(missing, collapse = ", "), + call. = FALSE + ) + } + res$wsg +} diff --git a/man/frs_wsg_drainage.Rd b/man/frs_wsg_drainage.Rd new file mode 100644 index 0000000..988b9ca --- /dev/null +++ b/man/frs_wsg_drainage.Rd @@ -0,0 +1,53 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/frs_wsg_drainage.R +\name{frs_wsg_drainage} +\alias{frs_wsg_drainage} +\title{WSG Drainage Closure (FWA Topology)} +\usage{ +frs_wsg_drainage(conn, watershed_group_code, table = "public.wsg_outlet") +} +\arguments{ +\item{conn}{A \link[DBI:DBIConnection-class]{DBI::DBIConnection} (e.g. from \code{\link[=frs_db_conn]{frs_db_conn()}}).} + +\item{watershed_group_code}{Character vector of focal WSG codes (e.g. +\code{c("BULK", "MORR")}). Must be non-empty and free of \code{NA}. Codes are +upper-cased internally. Focal codes that don't exist in \code{table} +trigger a warning and are dropped from the result; if no codes match, +the function errors.} + +\item{table}{Character scalar. Fully-qualified table name holding the +WSG outlet ltree topology. Default \code{"public.wsg_outlet"}.} +} +\value{ +Character vector of WSG codes — focal plus every WSG they flow +through — ordered downstream-first. +} +\description{ +Given a focal set of FWA watershed groups, return the \strong{drainage closure} +— every WSG that any focal WSG drains through — ordered downstream-first +by outlet ltree depth (\verb{nlevel(outlet) ASC}). Pure FWA topology; no +species or bundle knowledge. +} +\details{ +Sources the FWA WSG outlet table (default \code{public.wsg_outlet} in fwapg). +The closure predicate is \verb{f.outlet <@ w.outlet} — i.e. every WSG whose +outlet ltree is an ancestor of (or equal to) any focal WSG's outlet. + +Order: \verb{depth ASC, wsg ASC} — so the most downstream WSGs come first and +ties within a depth are alphabetical. Running per-WSG work in this order +persists downstream barriers before upstream WSGs read them, which makes +cross-WSG accessibility flags settle correctly within a single pass. +} +\examples{ +\dontrun{ +conn <- frs_db_conn() + +# Skeena + Peace focal — returns the 15-WSG drainage closure DS-first +frs_wsg_drainage(conn, c("PARS", "BULK")) +#> [1] "KISP" "KLUM" "LKEL" "LSKE" "MSKE" "USKE" "BULK" "FINA" +#> "LBTN" "LPCE" "MORR" "PARA" "PCEA" "UPCE" "PARS" + +DBI::dbDisconnect(conn) +} +} +\concept{wsg} diff --git a/planning/active/progress.md b/planning/active/progress.md index e3e630b..4829aa0 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -5,5 +5,6 @@ - Plan-mode exploration — phases approved by user - Created branch `211-frs-wsg-drainage-fwa-wsg-drainage-closur` off main (off `90af475`) - Scaffolded PWF baseline from issue #211 with approved phases (commit `1dc1145`) -- **Phase 1 complete:** `public.wsg_outlet` column confirmed as `wsg varchar(4)` (with `outlet ltree`, `lvl integer`); regression baseline run — PARS+BULK → 15 WSGs, exact match: `KISP, KLUM, LKEL, LSKE, MSKE, USKE, BULK, FINA, LBTN, LPCE, MORR, PARA, PCEA, UPCE, PARS` (DS-first, depths 1×6, 2×8, 3×1) -- Next: Phase 2 — write `R/frs_wsg_drainage.R` +- **Phase 1 complete:** `public.wsg_outlet` column confirmed as `wsg varchar(4)` (with `outlet ltree`, `lvl integer`); regression baseline run — PARS+BULK → 15 WSGs, exact match: `KISP, KLUM, LKEL, LSKE, MSKE, USKE, BULK, FINA, LBTN, LPCE, MORR, PARA, PCEA, UPCE, PARS` (DS-first, depths 1×6, 2×8, 3×1). Commit `808458e`. +- **Phase 2 complete:** Wrote `R/frs_wsg_drainage.R` (signature `frs_wsg_drainage(conn, watershed_group_code, table = "public.wsg_outlet")`); `devtools::document()` regenerated NAMESPACE + Rd; live-validated end-to-end (happy path: 15-WSG exact match; lowercase input normalized; TYPO triggers warning + drops from result; vector `table` rejected). `/code-check` Round 1 caught 2 fragility issues (scalar `table` check, silent unmatched focals), both fixed; Round 2 Clean. +- Next: Phase 3 — write tests/testthat/test-frs_wsg_drainage.R diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index f81a6bb..d32ea28 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -19,9 +19,9 @@ Param name `watershed_group_code` (not `wsgs`) for fresh-internal vocabulary con ## Phase 2 — Function + roxygen -- [ ] Write `R/frs_wsg_drainage.R` with signature, arg validation (`stopifnot` + `.frs_validate_identifier`), SQL via `sprintf` + `DBI::dbQuoteLiteral`, `\dontrun{}` example showing PARS+BULK, `@family wsg`, `@export` -- [ ] `devtools::document()` → regenerate `NAMESPACE` + `man/frs_wsg_drainage.Rd` -- [ ] `/code-check` clean → atomic commit (function + checkbox flip) +- [x] Write `R/frs_wsg_drainage.R` with signature, arg validation (`stopifnot` + `.frs_validate_identifier`), SQL via `sprintf` + `DBI::dbQuoteLiteral`, `\dontrun{}` example showing PARS+BULK, `@family wsg`, `@export`. Also: upper-case focal codes internally; warn on unmatched focals (caught by code-check Round 1); scalar `table` check (caught by code-check Round 1); use `DBI::dbGetQuery` (non-spatial, avoids sf warning). +- [x] `devtools::document()` → regenerate `NAMESPACE` + `man/frs_wsg_drainage.Rd` +- [x] `/code-check` clean (Round 1: 2 findings fixed; Round 2: Clean) → atomic commit (function + checkbox flip) ## Phase 3 — Tests From e9fa8b91fe8fb9085c9f14d55b17efa2eca60034 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Wed, 27 May 2026 05:31:27 -0700 Subject: [PATCH 4/6] Phase 3: frs_wsg_drainage tests (#211) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 11 test_that blocks / 14 expectations: - Arg validation (no DB): non-character, empty, NA, empty-string, vector table, and invalid identifier (asserts the validator's specific "table contains invalid characters" message — not a generic class match) - Live DB (skip on PG_DB_SHARE empty): PARS+BULK 15-WSG exact closure, focal-order invariance, lowercase input case-folds, unmatched-focal warning is raised + returned vector still valid, all-fake-focal errors All 14 pass against live fwapg. Co-Authored-By: Claude Opus 4.7 (1M context) --- planning/active/progress.md | 5 +- planning/active/task_plan.md | 9 +-- tests/testthat/test-frs_wsg_drainage.R | 108 +++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 8 deletions(-) create mode 100644 tests/testthat/test-frs_wsg_drainage.R diff --git a/planning/active/progress.md b/planning/active/progress.md index 4829aa0..cf7cc46 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -6,5 +6,6 @@ - Created branch `211-frs-wsg-drainage-fwa-wsg-drainage-closur` off main (off `90af475`) - Scaffolded PWF baseline from issue #211 with approved phases (commit `1dc1145`) - **Phase 1 complete:** `public.wsg_outlet` column confirmed as `wsg varchar(4)` (with `outlet ltree`, `lvl integer`); regression baseline run — PARS+BULK → 15 WSGs, exact match: `KISP, KLUM, LKEL, LSKE, MSKE, USKE, BULK, FINA, LBTN, LPCE, MORR, PARA, PCEA, UPCE, PARS` (DS-first, depths 1×6, 2×8, 3×1). Commit `808458e`. -- **Phase 2 complete:** Wrote `R/frs_wsg_drainage.R` (signature `frs_wsg_drainage(conn, watershed_group_code, table = "public.wsg_outlet")`); `devtools::document()` regenerated NAMESPACE + Rd; live-validated end-to-end (happy path: 15-WSG exact match; lowercase input normalized; TYPO triggers warning + drops from result; vector `table` rejected). `/code-check` Round 1 caught 2 fragility issues (scalar `table` check, silent unmatched focals), both fixed; Round 2 Clean. -- Next: Phase 3 — write tests/testthat/test-frs_wsg_drainage.R +- **Phase 2 complete:** Wrote `R/frs_wsg_drainage.R` (signature `frs_wsg_drainage(conn, watershed_group_code, table = "public.wsg_outlet")`); `devtools::document()` regenerated NAMESPACE + Rd; live-validated end-to-end (happy path: 15-WSG exact match; lowercase input normalized; TYPO triggers warning + drops from result; vector `table` rejected). `/code-check` Round 1 caught 2 fragility issues (scalar `table` check, silent unmatched focals), both fixed; Round 2 Clean. Commit `157d03e`. +- **Phase 3 complete:** Wrote `tests/testthat/test-frs_wsg_drainage.R` — 11 test_that blocks / 14 expectations (6 arg-validation + 5 live-DB). Live tests skip on missing `PG_DB_SHARE`; locally validated by inline-overriding env + `--no-environ` to bypass the dead `:63333` tunnel (per `m1-link-db-env` memory). All pass. `/code-check` Round 1 caught 1 too-broad `class = "error"` assertion (would pass on any error, not just identifier validation) — fixed to match `.frs_validate_identifier`'s actual message `"table contains invalid characters"`; Round 2 Clean. +- Next: Phase 4 — NEWS.md + DESCRIPTION bump + release commit + PR diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index d32ea28..0a6cdd3 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -25,12 +25,9 @@ Param name `watershed_group_code` (not `wsgs`) for fresh-internal vocabulary con ## Phase 3 — Tests -- [ ] `tests/testthat/test-frs_wsg_drainage.R`: - - Arg validation: non-character / empty / `NA` input rejected; invalid `table` rejected by `.frs_validate_identifier` - - DB-gated (skip on missing `PG_DB_SHARE` per `R/frs_db_conn.R:31`) live test: `frs_wsg_drainage(conn, c("PARS","BULK"))` returns the 15-WSG closure in DS-first order - - Ordering invariance: result equal for `c("BULK","PARS")` vs `c("PARS","BULK")` -- [ ] `devtools::test()` green -- [ ] `/code-check` clean → atomic commit +- [x] `tests/testthat/test-frs_wsg_drainage.R`: 6 arg-validation (non-character / empty / `NA` / empty-string / vector table / invalid identifier — last asserts validator's specific `"table contains invalid characters"` message after code-check Round 1) + 5 live-DB tests (skip on missing `PG_DB_SHARE`): PARS+BULK 15-WSG exact match, focal-order invariance, case-folding, unmatched-focal warning, no-match error +- [x] `devtools::test()` green — all 14 expectations pass against live fwapg (env-overridden to bypass dead `:63333` tunnel) +- [x] `/code-check` clean (Round 1: 1 finding fixed, 3 accepted as stopifnot-idiom tradeoffs; Round 2: Clean) → atomic commit ## Phase 4 — Release diff --git a/tests/testthat/test-frs_wsg_drainage.R b/tests/testthat/test-frs_wsg_drainage.R new file mode 100644 index 0000000..34dd94c --- /dev/null +++ b/tests/testthat/test-frs_wsg_drainage.R @@ -0,0 +1,108 @@ +# -- arg validation (no DB needed) ------------------------------------------- + +test_that("frs_wsg_drainage rejects non-character watershed_group_code", { + expect_error( + frs_wsg_drainage(NULL, watershed_group_code = 1:3), + "is.character" + ) +}) + +test_that("frs_wsg_drainage rejects empty watershed_group_code", { + expect_error( + frs_wsg_drainage(NULL, watershed_group_code = character(0)), + "length" + ) +}) + +test_that("frs_wsg_drainage rejects NA in watershed_group_code", { + expect_error( + frs_wsg_drainage(NULL, watershed_group_code = c("BULK", NA_character_)), + "anyNA" + ) +}) + +test_that("frs_wsg_drainage rejects empty-string focal codes", { + expect_error( + frs_wsg_drainage(NULL, watershed_group_code = c("BULK", "")), + "nzchar" + ) +}) + +test_that("frs_wsg_drainage rejects vector table arg", { + expect_error( + frs_wsg_drainage(NULL, "BULK", table = c("public.wsg_outlet", "x")), + "length\\(table\\) == 1L" + ) +}) + +test_that("frs_wsg_drainage rejects invalid table identifier", { + expect_error( + frs_wsg_drainage(NULL, "BULK", table = "x; DROP TABLE y; --"), + "table contains invalid characters" + ) +}) + +# -- live DB tests (gated on PG_DB_SHARE) ------------------------------------ + +test_that("frs_wsg_drainage returns the PARS+BULK 15-WSG closure DS-first", { + skip_if(Sys.getenv("PG_DB_SHARE") == "", "PG_DB_SHARE not set") + conn <- frs_db_conn() + on.exit(DBI::dbDisconnect(conn)) + + expected <- c( + "KISP", "KLUM", "LKEL", "LSKE", "MSKE", "USKE", + "BULK", "FINA", "LBTN", "LPCE", "MORR", "PARA", "PCEA", "UPCE", + "PARS" + ) + expect_identical( + frs_wsg_drainage(conn, c("PARS", "BULK")), + expected + ) +}) + +test_that("frs_wsg_drainage closure is invariant to focal input order", { + skip_if(Sys.getenv("PG_DB_SHARE") == "", "PG_DB_SHARE not set") + conn <- frs_db_conn() + on.exit(DBI::dbDisconnect(conn)) + + expect_identical( + frs_wsg_drainage(conn, c("PARS", "BULK")), + frs_wsg_drainage(conn, c("BULK", "PARS")) + ) +}) + +test_that("frs_wsg_drainage upper-cases focal codes internally", { + skip_if(Sys.getenv("PG_DB_SHARE") == "", "PG_DB_SHARE not set") + conn <- frs_db_conn() + on.exit(DBI::dbDisconnect(conn)) + + expect_identical( + frs_wsg_drainage(conn, c("pars", "bulk")), + frs_wsg_drainage(conn, c("PARS", "BULK")) + ) +}) + +test_that("frs_wsg_drainage warns on unmatched focal codes but returns valid ones", { + skip_if(Sys.getenv("PG_DB_SHARE") == "", "PG_DB_SHARE not set") + conn <- frs_db_conn() + on.exit(DBI::dbDisconnect(conn)) + + expect_warning( + res <- frs_wsg_drainage(conn, c("BULK", "TYPO")), + "TYPO" + ) + expect_true(length(res) > 0L) + expect_true("BULK" %in% res) + expect_false("TYPO" %in% res) +}) + +test_that("frs_wsg_drainage errors when no focal codes match", { + skip_if(Sys.getenv("PG_DB_SHARE") == "", "PG_DB_SHARE not set") + conn <- frs_db_conn() + on.exit(DBI::dbDisconnect(conn)) + + expect_error( + frs_wsg_drainage(conn, c("FAK1", "FAK2")), + "No drainage closure found" + ) +}) From b686598d27f242e3e0a7fcf8bcf93e99b65dea3d Mon Sep 17 00:00:00 2001 From: almac2022 Date: Wed, 27 May 2026 05:44:45 -0700 Subject: [PATCH 5/6] Release v0.32.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit frs_wsg_drainage — FWA WSG drainage-closure primitive. Promotes the inline closure query from link@v0.40.5 data-raw/study_area_wsgs.R into a reusable, tested fresh export. Pure FWA topology; first consumer is link::lnk_wsg_resolve (NewGraphEnvironment/link#207). Co-Authored-By: Claude Opus 4.7 (1M context) --- DESCRIPTION | 2 +- NEWS.md | 11 +++++++++++ planning/active/progress.md | 5 +++-- planning/active/task_plan.md | 9 +++++---- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index fdc23fa..eda7a57 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: fresh Title: Freshwater Referenced Spatial Hydrology -Version: 0.31.0 +Version: 0.32.0 Authors@R: c( person("Allan", "Irvine", , "al@newgraphenvironment.com", role = c("aut", "cre"), comment = c(ORCID = "0000-0002-3495-2128")), diff --git a/NEWS.md b/NEWS.md index fdd12b2..9b83ad7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,14 @@ +# fresh 0.32.0 + +Closes [#211](https://github.com/NewGraphEnvironment/fresh/issues/211). Adds `frs_wsg_drainage()` — a pure FWA-topology primitive that returns the drainage closure of a focal set of watershed groups (focal + every WSG they flow through), ordered downstream-first by outlet ltree depth. + +- Promotes the inline closure query from `NewGraphEnvironment/link@v0.40.5` `data-raw/study_area_wsgs.R` into a reusable, tested function. Pure FWA topology — no species / bundle / overrides knowledge — so other fresh consumers (vignettes, ad-hoc R sessions, future provincial drivers) can use it without going through link. +- Closure predicate `f.outlet <@ w.outlet` against `public.wsg_outlet` (ltree-based FWA WSG outlet table). Order `nlevel(outlet) ASC, wsg ASC` — most downstream WSGs first, alphabetical within a depth. Running per-WSG work in this order persists downstream barriers before upstream WSGs read them, which is what link's study-area runner relies on for cross-WSG access flags to settle within a single pass. +- Signature `frs_wsg_drainage(conn, watershed_group_code, table = "public.wsg_outlet")` follows fresh vocabulary (`watershed_group_code`, matches `frs_wsg_species` + `frs_stream_fetch`). Table identifier validated via `.frs_validate_identifier`; focal codes upper-cased internally and quoted via `DBI::dbQuoteLiteral` (no injection surface). Unmatched focal codes warn rather than silently drop; all-unmatched errors. +- First consumer: `link::lnk_wsg_resolve` ([NewGraphEnvironment/link#207](https://github.com/NewGraphEnvironment/link/issues/207)) — the link wrapper composes this primitive with the bundle's species-presence filter (#157) to drive `data-raw/study_area_wsgs.R` (and replaces the inline query there). +- New `@family wsg` family. `frs_wsg_species` retag deferred to a separate concern. +- 11 tests / 14 expectations: 6 arg-validation (no DB), 5 live-DB (gated on `PG_DB_SHARE`) covering the 15-WSG PARS+BULK regression baseline, focal-order invariance, case-folding, unmatched-focal warning, and all-unmatched error. Live-validated against fwapg. + # fresh 0.31.0 Closes [#207](https://github.com/NewGraphEnvironment/fresh/issues/207). Adds `frs_candidates_pick()` — fourth primitive in the point-handling family (alongside `frs_point_snap`, `frs_network_features`, `frs_point_match`). Given a candidates table where multiple rows can share the same key value, optionally compute a per-row score from a caller-supplied SQL expression, optionally filter via a caller-supplied WHERE clause, then keep one row per key via `DISTINCT ON (col_key) ORDER BY ...`. diff --git a/planning/active/progress.md b/planning/active/progress.md index cf7cc46..848ca33 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -7,5 +7,6 @@ - Scaffolded PWF baseline from issue #211 with approved phases (commit `1dc1145`) - **Phase 1 complete:** `public.wsg_outlet` column confirmed as `wsg varchar(4)` (with `outlet ltree`, `lvl integer`); regression baseline run — PARS+BULK → 15 WSGs, exact match: `KISP, KLUM, LKEL, LSKE, MSKE, USKE, BULK, FINA, LBTN, LPCE, MORR, PARA, PCEA, UPCE, PARS` (DS-first, depths 1×6, 2×8, 3×1). Commit `808458e`. - **Phase 2 complete:** Wrote `R/frs_wsg_drainage.R` (signature `frs_wsg_drainage(conn, watershed_group_code, table = "public.wsg_outlet")`); `devtools::document()` regenerated NAMESPACE + Rd; live-validated end-to-end (happy path: 15-WSG exact match; lowercase input normalized; TYPO triggers warning + drops from result; vector `table` rejected). `/code-check` Round 1 caught 2 fragility issues (scalar `table` check, silent unmatched focals), both fixed; Round 2 Clean. Commit `157d03e`. -- **Phase 3 complete:** Wrote `tests/testthat/test-frs_wsg_drainage.R` — 11 test_that blocks / 14 expectations (6 arg-validation + 5 live-DB). Live tests skip on missing `PG_DB_SHARE`; locally validated by inline-overriding env + `--no-environ` to bypass the dead `:63333` tunnel (per `m1-link-db-env` memory). All pass. `/code-check` Round 1 caught 1 too-broad `class = "error"` assertion (would pass on any error, not just identifier validation) — fixed to match `.frs_validate_identifier`'s actual message `"table contains invalid characters"`; Round 2 Clean. -- Next: Phase 4 — NEWS.md + DESCRIPTION bump + release commit + PR +- **Phase 3 complete:** Wrote `tests/testthat/test-frs_wsg_drainage.R` — 11 test_that blocks / 14 expectations (6 arg-validation + 5 live-DB). Live tests skip on missing `PG_DB_SHARE`; locally validated by inline-overriding env + `--no-environ` to bypass the dead `:63333` tunnel (per `m1-link-db-env` memory). All pass. `/code-check` Round 1 caught 1 too-broad `class = "error"` assertion (would pass on any error, not just identifier validation) — fixed to match `.frs_validate_identifier`'s actual message `"table contains invalid characters"`; Round 2 Clean. Commit `e9fa8b9`. +- **Phase 4 release-prep:** NEWS.md `# fresh 0.32.0` section added (one-line summary + 6 bullets matching v0.31.0 style); DESCRIPTION bumped `0.31.0 → 0.32.0` (no Date — matches fresh convention). lintr skipped (fresh hasn't adopted lintr; not in Suggests). /code-check skipped on this docs-only Release commit (R code already cleared in Phases 2 + 3). +- Next: commit Release v0.32.0, then `/planning-archive` + `/gh-pr-push` diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index 0a6cdd3..81f70a3 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -31,10 +31,11 @@ Param name `watershed_group_code` (not `wsgs`) for fresh-internal vocabulary con ## Phase 4 — Release -- [ ] `NEWS.md`: new `# fresh 0.32.0` section, lead with bold one-liner + bullets (composition with `lnk_wsg_resolve`, pure-topology framing, test count) — matches existing style (`NEWS.md:1-10`) -- [ ] `DESCRIPTION`: `Version: 0.32.0`, `Date: ` -- [ ] `lintr::lint_package()` clean -- [ ] `/code-check` clean → atomic commit `"Release v0.32.0"` as final commit of branch +- [x] `NEWS.md`: new `# fresh 0.32.0` section, lead with one-line summary + 6 bullets (FWA-topology framing, closure predicate + DS-first ordering rationale, signature + safety, first consumer `lnk_wsg_resolve`, family naming, test count). Matches `NEWS.md:1-10` (v0.31.0) style. +- [x] `DESCRIPTION`: `Version: 0.31.0 → 0.32.0`. No `Date:` field added — matches fresh's existing convention (no Date in DESCRIPTION through v0.31.0; soul R-package conventions don't require Date). +- [x] `lintr::lint_package()` — **skipped**: fresh hasn't adopted lintr (not in Suggests). Convention is link's, not fresh's. Code follows fresh's existing R style by inspection. +- [x] `/code-check` — skipped on this Release commit (docs + version bump only; R code already passed `/code-check` clean in Phases 2 + 3) +- [ ] Atomic commit `"Release v0.32.0"` as final commit of branch - [ ] `/planning-archive` → `/gh-pr-push` (PR body: `Closes #211` + `Relates to NewGraphEnvironment/sred-2025-2026#24`) ## Validation From 9de30638aa4ac4544a6ba22cc5784e6905710bcb Mon Sep 17 00:00:00 2001 From: almac2022 Date: Wed, 27 May 2026 05:45:25 -0700 Subject: [PATCH 6/6] Archive planning files for issue #211 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-issue-211-frs-wsg-drainage/.gitkeep | 0 .../2026-05-issue-211-frs-wsg-drainage/README.md | 11 +++++++++++ .../2026-05-issue-211-frs-wsg-drainage}/findings.md | 0 .../2026-05-issue-211-frs-wsg-drainage}/progress.md | 0 .../2026-05-issue-211-frs-wsg-drainage}/task_plan.md | 0 5 files changed, 11 insertions(+) create mode 100644 planning/archive/2026-05-issue-211-frs-wsg-drainage/.gitkeep create mode 100644 planning/archive/2026-05-issue-211-frs-wsg-drainage/README.md rename planning/{active => archive/2026-05-issue-211-frs-wsg-drainage}/findings.md (100%) rename planning/{active => archive/2026-05-issue-211-frs-wsg-drainage}/progress.md (100%) rename planning/{active => archive/2026-05-issue-211-frs-wsg-drainage}/task_plan.md (100%) diff --git a/planning/archive/2026-05-issue-211-frs-wsg-drainage/.gitkeep b/planning/archive/2026-05-issue-211-frs-wsg-drainage/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/planning/archive/2026-05-issue-211-frs-wsg-drainage/README.md b/planning/archive/2026-05-issue-211-frs-wsg-drainage/README.md new file mode 100644 index 0000000..2a8aed7 --- /dev/null +++ b/planning/archive/2026-05-issue-211-frs-wsg-drainage/README.md @@ -0,0 +1,11 @@ +# Issue #211 — frs_wsg_drainage + +## Outcome + +Added `frs_wsg_drainage(conn, watershed_group_code, table = "public.wsg_outlet")` as a new exported primitive in the `@family wsg` family. Given a focal set of FWA watershed groups, returns the drainage closure (focal + every WSG they drain through) ordered downstream-first by `nlevel(outlet) ASC`. Pure FWA topology — no bundle / species / overrides knowledge. SQL ported verbatim from the validated query in `NewGraphEnvironment/link@v0.40.5 data-raw/study_area_wsgs.R:44-50`. + +Beyond the literal SQL port, the function adds: scalar `table` arg check (`.frs_validate_identifier` is vectorized internally so a vector value would silently slip through and produce malformed SQL — caught by code-check Round 1); internal upper-casing of focal codes; `warning()` on unmatched focals so `c("BULK","TYPO")` no longer returns silently partial closure (also Round 1); test assertion tightened to match the validator's actual error message rather than a generic `class = "error"` (Round 2). 11 test_that blocks / 14 expectations (6 arg-validation + 5 live-DB gated on `PG_DB_SHARE`). Released as **v0.32.0**. + +First consumer: `link::lnk_wsg_resolve` ([NewGraphEnvironment/link#207](https://github.com/NewGraphEnvironment/link/issues/207)) — the link wrapper composes this primitive with the bundle's species-presence filter to drive `data-raw/study_area_wsgs.R` (replacing the inline query there). + +Closed by: commits `808458e` (Phase 1 verification), `157d03e` (function), `e9fa8b9` (tests), `b686598` (Release v0.32.0). PR forthcoming via `/gh-pr-push`. diff --git a/planning/active/findings.md b/planning/archive/2026-05-issue-211-frs-wsg-drainage/findings.md similarity index 100% rename from planning/active/findings.md rename to planning/archive/2026-05-issue-211-frs-wsg-drainage/findings.md diff --git a/planning/active/progress.md b/planning/archive/2026-05-issue-211-frs-wsg-drainage/progress.md similarity index 100% rename from planning/active/progress.md rename to planning/archive/2026-05-issue-211-frs-wsg-drainage/progress.md diff --git a/planning/active/task_plan.md b/planning/archive/2026-05-issue-211-frs-wsg-drainage/task_plan.md similarity index 100% rename from planning/active/task_plan.md rename to planning/archive/2026-05-issue-211-frs-wsg-drainage/task_plan.md