From 8035cf12c8df48634038856ba03d1d2ac398fe5f Mon Sep 17 00:00:00 2001 From: almac2022 Date: Mon, 8 Jun 2026 22:15:17 -0700 Subject: [PATCH 1/4] Initialize PWF baseline for #218 Co-Authored-By: Claude Opus 4.7 --- planning/active/findings.md | 33 ++++++++++++++++++ planning/active/progress.md | 9 +++++ planning/active/task_plan.md | 66 ++++++++++++++++++++++++++++++++++++ 3 files changed, 108 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..4c498547 --- /dev/null +++ b/planning/active/findings.md @@ -0,0 +1,33 @@ +# Findings — lnk_pipeline_run: produce streams_access regardless of mapping_code (#218) + +## Issue context + +### Problem +`lnk_pipeline_run()` only computes `streams_access` when `mapping_code = TRUE` — +`lnk_barriers_views()` + `lnk_pipeline_access()` sit inside the `if (isTRUE(mapping_code))` +branch. Access is foundational (mapping_code depends on it, not the reverse), so +`mapping_code = FALSE` should still produce `streams_access` / `access_`. + +### Proposed solution +- Always run `lnk_barriers_views()` + `lnk_pipeline_access()` in `lnk_pipeline_run()`. +- Gate only `lnk_pipeline_mapping_code()` (token assembly) behind `mapping_code`. +- Persist `streams_access` independently of `streams_mapping_code`. + +## Exploration notes + +- **Persist is already shape-tolerant.** `lnk_pipeline_persist()` copies + `streams_access` and `streams_mapping_code` under separate `information_schema` + probes — copies whichever working tables exist. No change required there. +- **Pre-persist coupling.** The mapping_code block opens with a pre-persist + (`R/lnk_pipeline_run.R:188`) so `.barriers` holds the current + WSG before `lnk_barriers_views()` reads it (default source = persist barriers, + for cross-WSG dam visibility — link#196). Hoisting access requires hoisting this + pre-persist too. +- **mapping_code = TRUE path unchanged.** The refactor only moves the first three + steps out of the gate; the execution order for `mapping_code = TRUE` stays + pre-persist → barriers_views → access → mapping_code → final persist. Output is + byte-identical → vignette parity (99.04% BT) stable. +- **Callers.** `wsg_run_one.R` (provincial runner) already TRUE. `wsg_pipeline_run.R` + default FALSE → now gains access. `lnk_compare_wsg()` passes through. +- **Test contract change.** `test-lnk_pipeline_run.R:141` asserts default path + ends at `persist` with no access — must be rewritten. diff --git a/planning/active/progress.md b/planning/active/progress.md new file mode 100644 index 00000000..3ea23e5f --- /dev/null +++ b/planning/active/progress.md @@ -0,0 +1,9 @@ +# Progress — lnk_pipeline_run: produce streams_access regardless of mapping_code (#218) + +## Session 2026-06-08 + +- Plan-mode exploration — phases approved by user +- Created branch `218-lnk-pipeline-run-produce-streams-access-` off main +- Scaffolded PWF baseline from issue #218 with approved phases +- Verified vignette safety (cached-artifact render; mapping_code=TRUE byte-identical) +- Next: start Phase 1 — hoist access out of the mapping_code gate in `R/lnk_pipeline_run.R` diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md new file mode 100644 index 00000000..966ee014 --- /dev/null +++ b/planning/active/task_plan.md @@ -0,0 +1,66 @@ +# Task: lnk_pipeline_run: produce streams_access regardless of mapping_code (#218) + +## Problem + +`lnk_pipeline_run()` only computes `streams_access` when `mapping_code = TRUE` — +`lnk_barriers_views()` + `lnk_pipeline_access()` sit inside the +`if (isTRUE(mapping_code))` branch. Access is foundational (mapping_code depends +on it, not the reverse), so `mapping_code = FALSE` should still produce +`streams_access` / `access_`. + +## Proposed solution (from issue) + +- Always run `lnk_barriers_views()` + `lnk_pipeline_access()` in `lnk_pipeline_run()`. +- Gate only `lnk_pipeline_mapping_code()` (token assembly) behind `mapping_code`. +- Persist `streams_access` independently of `streams_mapping_code`. + +## Key findings (from plan-mode exploration) + +- `lnk_pipeline_persist()` already probes for `streams_access` + (`R/lnk_pipeline_persist.R:146–181`) and `streams_mapping_code` (`:186–213`) + with **independent** table-presence checks → no persist change needed. +- The pre-persist (step 0, `R/lnk_pipeline_run.R:188`) must move out with access: + `lnk_barriers_views()` defaults to reading `.barriers` + (`R/lnk_barriers_views.R:52–55`), so the current WSG's barriers must be + persisted before the views are built (cross-WSG dam visibility, #196). +- Stays gated: species-residence classification (`:241–248`) + `lnk_mapping_code()` + (`:250–259`). Moves out: `lnk_presence` (`:175`), `barriers_per_sp` (`:213–216`), + pre-persist (`:188–189`), `lnk_barriers_views` (`:197–198`), `lnk_pipeline_access` + (`:218–230`). +- Blast radius small + intended: `data-raw/wsg_run_one.R:56` already passes + `mapping_code = TRUE`. Only habitat-only callers (`wsg_pipeline_run.R` default, + `lnk_compare_wsg(mapping_code = FALSE)`) gain access output. +- Vignette safe: renders from cached artifacts only (`eval = FALSE` pipeline + chunk); `mapping_code = TRUE` sequence is byte-identical after refactor. + +## Phase 1 — Hoist access out of the `mapping_code` gate +- [ ] `R/lnk_pipeline_run.R`: move `lnk_presence`, `barriers_per_sp`, pre-persist, + `lnk_barriers_views`, `lnk_pipeline_access` out of `if (isTRUE(mapping_code))` + so they run unconditionally (after `lnk_barriers_unify`, before final persist). + Keep species-residence classification + `lnk_mapping_code` inside a slimmer gate. +- [ ] Update roxygen header (phase-order list, `@param mapping_code`) to say access + is always built and only token assembly is gated. Re-`document()`. + +## Phase 2 — Update + extend tests +- [ ] Rewrite "composes phases in expected order" (`test-lnk_pipeline_run.R:141`): + default path now includes pre-persist, `barriers_views`, `pipeline_access`, then + final persist (two `persist` calls). Add mocks for `lnk_presence`, + `lnk_barriers_views`, `lnk_pipeline_access`. +- [ ] Add test: `lnk_pipeline_access` called for BOTH `mapping_code` values; + `lnk_mapping_code` called only when `mapping_code = TRUE`. +- [ ] Add new mocks to empty-species / dams / cleanup tests so they keep passing. + +## Phase 3 — Verify + release +- [ ] `devtools::document()`; `lintr::lint_package()` clean; `devtools::test()` green. +- [ ] `tools::buildVignettes(dir = ".", tangle = FALSE, clean = TRUE)` exits 0. +- [ ] `/code-check` clean → atomic commits (code + checkbox flip) per phase. +- [ ] NEWS.md entry + `DESCRIPTION` bump 0.42.0 → 0.43.0 (final commit). +- [ ] `/planning-archive` → `/gh-pr-push` (PR body: `Closes #218`, SRED tag in PR body). + +## Validation + +- [ ] `devtools::test()` green, incl. rewritten composition test + new gating test +- [ ] `tools::buildVignettes` exits 0 +- [ ] `/code-check` clean on each commit +- [ ] PWF checkboxes match landed work +- [ ] `/planning-archive` on completion From 681c7bd94302f691f060fcb9dbd91d7188da650b Mon Sep 17 00:00:00 2001 From: almac2022 Date: Mon, 8 Jun 2026 22:38:19 -0700 Subject: [PATCH 2/4] Hoist access build out of the mapping_code gate (#218) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lnk_pipeline_run() now builds and persists streams_access regardless of mapping_code. lnk_presence, the pre-persist, lnk_barriers_views, and lnk_pipeline_access move out of the `if (isTRUE(mapping_code))` block; only lnk_mapping_code (token assembly) stays gated. Access is foundational — mapping_code depends on it — so habitat-only callers (mapping_code = FALSE) now also emit .streams_access. No persist change needed: lnk_pipeline_persist already probes for streams_access and streams_mapping_code independently. Tests: rewrite the composition test for the new call order (presence -> pre-persist -> barriers_views -> access -> final persist); add a test asserting access builds for both mapping_code values while lnk_mapping_code is gated; add access-path mocks to the dams + cleanup tests. Co-Authored-By: Claude Opus 4.7 --- R/lnk_pipeline_run.R | 179 +++++++++++++------------ man/lnk_pipeline_run.Rd | 38 +++--- planning/active/progress.md | 8 +- planning/active/task_plan.md | 10 +- tests/testthat/test-lnk_pipeline_run.R | 90 ++++++++++++- 5 files changed, 211 insertions(+), 114 deletions(-) diff --git a/R/lnk_pipeline_run.R b/R/lnk_pipeline_run.R index fbc8dc69..43355324 100644 --- a/R/lnk_pipeline_run.R +++ b/R/lnk_pipeline_run.R @@ -33,15 +33,18 @@ #' 10. [lnk_barriers_unify()] — unify per-source barriers into a single #' working-schema table (always; promotes the mapping_code-only #' flag in `lnk_compare_wsg()` to canonical PG state). -#' 10b. *Optional* mapping_code phase — gated by `mapping_code = TRUE`. -#' Runs between barriers_unify and persist: -#' [lnk_barriers_views()] over working `.barriers` (tunnel- -#' free, link-canonical per-species views), [lnk_pipeline_access()] -#' (writes working `streams_access`), [lnk_mapping_code()] (writes -#' working `streams_mapping_code`). Persist phase copies both to -#' ``. See link#187. +#' 10a. **Access phase (always-on, link#218).** Runs between barriers_unify +#' and persist: a pre-persist of the current WSG (so the next two steps +#' see cross-WSG barriers), [lnk_barriers_views()] (tunnel-free, +#' link-canonical per-species views over `.barriers`), +#' and [lnk_pipeline_access()] (writes working `streams_access`). Access +#' is foundational — mapping_code depends on it — so it builds regardless +#' of `mapping_code`. See link#187, #196. +#' 10b. *Optional* mapping_code token assembly — gated by `mapping_code = TRUE`. +#' [lnk_mapping_code()] consumes `streams_access` + `streams_habitat` +#' and writes working `streams_mapping_code`. #' 11. [lnk_pipeline_persist()] — copy per-WSG streams + per-species -#' habitat + barriers (+ optional streams_access + streams_mapping_code) +#' habitat + barriers + streams_access (+ optional streams_mapping_code) #' into `` (idempotent DELETE-WHERE-WSG + INSERT). #' #' @param conn DBI connection to the local pipeline database (typically @@ -59,15 +62,16 @@ #' @param cleanup_working Logical. When `TRUE` (default), drop the #' `` working schema at the end. Pass `FALSE` for interactive #' debug / manual inspection. -#' @param mapping_code Logical. When `TRUE`, additionally runs the -#' tunnel-free mapping_code build phase (10b above) — produces -#' `.streams_access` and -#' `.streams_mapping_code` for the WSG, consumed -#' downstream by `data-raw/build_species_views.R --bcfp` (QGIS bcfp- -#' shape symbology). Default `FALSE`. Methodology shift from pre-#187 -#' compare_wsg: access uses link's own per-species barriers (via -#' `blocks_species` predicate on `.barriers`), not bcfp's -#' tunnel-staged tables. +#' @param mapping_code Logical. Gates only the mapping_code **token +#' assembly** (phase 10b) — when `TRUE`, [lnk_mapping_code()] runs and +#' `.streams_mapping_code` is produced for the WSG, +#' consumed downstream by `data-raw/build_species_views.R --bcfp` (QGIS +#' bcfp-shape symbology). Default `FALSE`. **`streams_access` is built +#' and persisted regardless of this flag** (phase 10a, link#218) — +#' access is foundational, not mapping_code-specific. Methodology shift +#' from pre-#187 compare_wsg: access uses link's own per-species +#' barriers (via `blocks_species` predicate on `.barriers`), +#' not bcfp's tunnel-staged tables. #' #' @return `conn`, invisibly. Side effects are the writes into #' `.streams`, `streams_habitat_`, and `barriers`. @@ -162,82 +166,81 @@ lnk_pipeline_run <- function(conn, aoi, cfg, loaded, lnk_barriers_unify(conn, aoi = aoi, cfg = cfg, # nolint: object_usage_linter loaded = loaded, schema = schema) - # Optional mapping_code phase (link#187). Tunnel-free build of - # streams_access + streams_mapping_code in the working schema. Persist - # phase (next) copies both to . Methodology shift vs - # pre-#187 compare_wsg: ACCESS now uses link's own per-species barriers - # (derived from .barriers's `blocks_species` predicate via - # lnk_barriers_views) instead of bcfp's barriers tables staged via the - # tunnel. Yields a more meaningful diff vs bcfp's streams_mapping_code - # (the prior compare_wsg path used bcfp access semantics, artificially - # suppressing real link-vs-bcfp divergence). See NEWS v0.40.0. - if (isTRUE(mapping_code)) { - pres <- lnk_presence(loaded$wsg_species_presence, aoi) # nolint: object_usage_linter + # Access phase (link#187, always-on per link#218). Tunnel-free build of + # streams_access in the working schema; the persist phase (below) copies it + # to . Access is foundational — mapping_code depends on it, + # not the reverse — so it runs regardless of `mapping_code`. Methodology vs + # pre-#187 compare_wsg: ACCESS uses link's own per-species barriers (derived + # from .barriers's `blocks_species` predicate via lnk_barriers_views) + # instead of bcfp's barriers tables staged via the tunnel. See NEWS v0.40.0. + # Only the mapping_code token assembly (step 3) stays gated by `mapping_code`. + pres <- lnk_presence(loaded$wsg_species_presence, aoi) # nolint: object_usage_linter - # 0. Pre-persist current WSG's barriers + streams + habitat into - # BEFORE building views. lnk_barriers_views defaults - # to reading the persist barriers table — necessary for cross-WSG - # access lookups (e.g. PARS BT through Bennett dams in PCEA/UPCE, - # link#152). My #187 Phase 4 worked around an ordering issue by - # pointing views at working .barriers, but that loses the - # cross-WSG visibility. Fix: persist current WSG's barriers FIRST, - # so persist holds current + all previously-persisted WSGs by the - # time the views are built. Second persist call below re-runs - # idempotently and adds streams_access + streams_mapping_code. - # link#196. - lnk_pipeline_persist(conn, aoi = aoi, cfg = cfg, # nolint: object_usage_linter - species = active_species, schema = schema) + # 0. Pre-persist current WSG's barriers + streams + habitat into + # BEFORE building views. lnk_barriers_views defaults + # to reading the persist barriers table — necessary for cross-WSG + # access lookups (e.g. PARS BT through Bennett dams in PCEA/UPCE, + # link#152). My #187 Phase 4 worked around an ordering issue by + # pointing views at working .barriers, but that loses the + # cross-WSG visibility. Fix: persist current WSG's barriers FIRST, + # so persist holds current + all previously-persisted WSGs by the + # time the views are built. Second persist call below re-runs + # idempotently and adds streams_access (+ streams_mapping_code when + # mapping_code = TRUE). link#196. + lnk_pipeline_persist(conn, aoi = aoi, cfg = cfg, # nolint: object_usage_linter + species = active_species, schema = schema) - # 1. Per-species + per-source barrier views over PERSIST - # .barriers (default — see #196). Province-wide, - # tunnel-free, link-canonical. Sees current WSG (just persisted) + - # all previously-persisted WSGs (cross-WSG dam visibility). - # `species = active_species` so the view set matches the bundle - # (default config = bt/gr/ko/rb — not the bcfp 8). - lnk_barriers_views(conn, schema = schema, cfg = cfg, # nolint: object_usage_linter - species = active_species) + # 1. Per-species + per-source barrier views over PERSIST + # .barriers (default — see #196). Province-wide, + # tunnel-free, link-canonical. Sees current WSG (just persisted) + + # all previously-persisted WSGs (cross-WSG dam visibility). + # `species = active_species` so the view set matches the bundle + # (default config = bt/gr/ko/rb — not the bcfp 8). + lnk_barriers_views(conn, schema = schema, cfg = cfg, # nolint: object_usage_linter + species = active_species) - # 2. Per-segment access. barriers_per_sp drives `accessible` (token1 - # ACCESS + token2 gate). Uses the per-species `barriers__access` - # views (link#200) — these reproduce bcfp's per-species access set - # (`barriers_`, smnorris/bcfishpass model/01_access/sql/ - # model_access_bt.sql): NATURAL barriers only (gradient at the species - # threshold + falls + subsurface), MINUS the observation/habitat - # override, ∪ user_definite (override-exempt). Dams/PSCIS/modelled are - # NOT in the access set — they annotate access via token2 - # (barrier_sources below), never block it. So a dam-downstream segment - # that is otherwise accessible stays accessible and emits `;DAM`. All - # inputs are province-wide-persisted (barriers + barrier_overrides via - # the pre-persist above), so the cross-WSG downstream walk is correct in - # every WSG, not just the run's own. See RUNBOOK.md §5. - sp_set <- tolower(active_species) - barriers_per_sp <- setNames( - lapply(sp_set, function(sp) paste0(schema, ".barriers_", sp, "_access")), - sp_set) + # 2. Per-segment access. barriers_per_sp drives `accessible` (token1 + # ACCESS + token2 gate). Uses the per-species `barriers__access` + # views (link#200) — these reproduce bcfp's per-species access set + # (`barriers_`, smnorris/bcfishpass model/01_access/sql/ + # model_access_bt.sql): NATURAL barriers only (gradient at the species + # threshold + falls + subsurface), MINUS the observation/habitat + # override, ∪ user_definite (override-exempt). Dams/PSCIS/modelled are + # NOT in the access set — they annotate access via token2 + # (barrier_sources below), never block it. So a dam-downstream segment + # that is otherwise accessible stays accessible and emits `;DAM`. All + # inputs are province-wide-persisted (barriers + barrier_overrides via + # the pre-persist above), so the cross-WSG downstream walk is correct in + # every WSG, not just the run's own. See RUNBOOK.md §5. + sp_set <- tolower(active_species) + barriers_per_sp <- setNames( + lapply(sp_set, function(sp) paste0(schema, ".barriers_", sp, "_access")), + sp_set) - lnk_pipeline_access(conn, # nolint: object_usage_linter - segments = paste0(schema, ".streams"), - aoi = aoi, - to = paste0(schema, ".streams_access"), - barriers_per_sp = barriers_per_sp, - observations = paste0(schema, ".observations"), - presence = pres, - barrier_sources = list( - anthropogenic = paste0(schema, ".barriers_anthropogenic_unified"), - pscis = paste0(schema, ".barriers_pscis"), - dams = paste0(schema, ".barriers_dams_unified"), - remediations = paste0(schema, ".barriers_remediations")), - crossings_table = paste0(schema, ".crossings")) + lnk_pipeline_access(conn, # nolint: object_usage_linter + segments = paste0(schema, ".streams"), + aoi = aoi, + to = paste0(schema, ".streams_access"), + barriers_per_sp = barriers_per_sp, + observations = paste0(schema, ".observations"), + presence = pres, + barrier_sources = list( + anthropogenic = paste0(schema, ".barriers_anthropogenic_unified"), + pscis = paste0(schema, ".barriers_pscis"), + dams = paste0(schema, ".barriers_dams_unified"), + remediations = paste0(schema, ".barriers_remediations")), + crossings_table = paste0(schema, ".crossings")) - # 3. Per-segment per-species mapping_code tokens. Schema-aware wrapper - # over lnk_pipeline_mapping_code — delegates the pure data transform. - # Species pass-through: intersect bcfp residence defaults with - # active_species so mapping_code only computes for species the bundle - # actually models. Species in active_species but NOT in any bcfp - # residence category (e.g. GR/KO/RB in the default bundle) get - # assigned to species_resident by fallback — GR/KO/RB are all resident - # salmonids; treat as resident until link#189 data-drives this from - # dimensions.csv. + # 3. Per-segment per-species mapping_code tokens — gated by `mapping_code` + # (link#218). Schema-aware wrapper over lnk_pipeline_mapping_code — + # delegates the pure data transform. Species pass-through: intersect bcfp + # residence defaults with active_species so mapping_code only computes for + # species the bundle actually models. Species in active_species but NOT in + # any bcfp residence category (e.g. GR/KO/RB in the default bundle) get + # assigned to species_resident by fallback — GR/KO/RB are all resident + # salmonids; treat as resident until link#189 data-drives this from + # dimensions.csv. + if (isTRUE(mapping_code)) { sp_resident_bcfp <- c("bt", "wct") sp_anadromous_bcfp <- c("ch", "cm", "co", "pk", "sk", "st") sp_spawn_only_bcfp <- c("cm", "pk") diff --git a/man/lnk_pipeline_run.Rd b/man/lnk_pipeline_run.Rd index c0599ade..1fd9d8e8 100644 --- a/man/lnk_pipeline_run.Rd +++ b/man/lnk_pipeline_run.Rd @@ -38,15 +38,16 @@ runs from local \code{cabd.dams}. Pass \code{FALSE} to skip dams entirely.} \verb{} working schema at the end. Pass \code{FALSE} for interactive debug / manual inspection.} -\item{mapping_code}{Logical. When \code{TRUE}, additionally runs the -tunnel-free mapping_code build phase (10b above) — produces -\verb{.streams_access} and -\verb{.streams_mapping_code} for the WSG, consumed -downstream by \code{data-raw/build_species_views.R --bcfp} (QGIS bcfp- -shape symbology). Default \code{FALSE}. Methodology shift from pre-#187 -compare_wsg: access uses link's own per-species barriers (via -\code{blocks_species} predicate on \verb{.barriers}), not bcfp's -tunnel-staged tables.} +\item{mapping_code}{Logical. Gates only the mapping_code \strong{token +assembly} (phase 10b) — when \code{TRUE}, \code{\link[=lnk_mapping_code]{lnk_mapping_code()}} runs and +\verb{.streams_mapping_code} is produced for the WSG, +consumed downstream by \code{data-raw/build_species_views.R --bcfp} (QGIS +bcfp-shape symbology). Default \code{FALSE}. \strong{\code{streams_access} is built +and persisted regardless of this flag} (phase 10a, link#218) — +access is foundational, not mapping_code-specific. Methodology shift +from pre-#187 compare_wsg: access uses link's own per-species +barriers (via \code{blocks_species} predicate on \verb{.barriers}), +not bcfp's tunnel-staged tables.} } \value{ \code{conn}, invisibly. Side effects are the writes into @@ -86,15 +87,18 @@ AOI (cfg$species ∩ wsg_species_presence). Empty set is an error. \item \code{\link[=lnk_barriers_unify]{lnk_barriers_unify()}} — unify per-source barriers into a single working-schema table (always; promotes the mapping_code-only flag in \code{lnk_compare_wsg()} to canonical PG state). -10b. \emph{Optional} mapping_code phase — gated by \code{mapping_code = TRUE}. -Runs between barriers_unify and persist: -\code{\link[=lnk_barriers_views]{lnk_barriers_views()}} over working \verb{.barriers} (tunnel- -free, link-canonical per-species views), \code{\link[=lnk_pipeline_access]{lnk_pipeline_access()}} -(writes working \code{streams_access}), \code{\link[=lnk_mapping_code]{lnk_mapping_code()}} (writes -working \code{streams_mapping_code}). Persist phase copies both to -\verb{}. See link#187. +10a. \strong{Access phase (always-on, link#218).} Runs between barriers_unify +and persist: a pre-persist of the current WSG (so the next two steps +see cross-WSG barriers), \code{\link[=lnk_barriers_views]{lnk_barriers_views()}} (tunnel-free, +link-canonical per-species views over \verb{.barriers}), +and \code{\link[=lnk_pipeline_access]{lnk_pipeline_access()}} (writes working \code{streams_access}). Access +is foundational — mapping_code depends on it — so it builds regardless +of \code{mapping_code}. See link#187, #196. +10b. \emph{Optional} mapping_code token assembly — gated by \code{mapping_code = TRUE}. +\code{\link[=lnk_mapping_code]{lnk_mapping_code()}} consumes \code{streams_access} + \code{streams_habitat} +and writes working \code{streams_mapping_code}. \item \code{\link[=lnk_pipeline_persist]{lnk_pipeline_persist()}} — copy per-WSG streams + per-species -habitat + barriers (+ optional streams_access + streams_mapping_code) +habitat + barriers + streams_access (+ optional streams_mapping_code) into \verb{} (idempotent DELETE-WHERE-WSG + INSERT). } } diff --git a/planning/active/progress.md b/planning/active/progress.md index 3ea23e5f..07d57784 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -6,4 +6,10 @@ - Created branch `218-lnk-pipeline-run-produce-streams-access-` off main - Scaffolded PWF baseline from issue #218 with approved phases - Verified vignette safety (cached-artifact render; mapping_code=TRUE byte-identical) -- Next: start Phase 1 — hoist access out of the mapping_code gate in `R/lnk_pipeline_run.R` +- Phase 1 — hoisted `lnk_presence`, `barriers_per_sp`, pre-persist, `lnk_barriers_views`, + `lnk_pipeline_access` out of the `if (isTRUE(mapping_code))` gate; only `lnk_mapping_code` + token assembly stays gated. Updated roxygen (phase 10a/10b, `@param mapping_code`) + re-`document()`. +- Phase 2 — rewrote "composes phases in expected order" (new order: …barriers_unify, presence, + persist, barriers_views, access, persist); added "builds access for both mapping_code values, + gates lnk_mapping_code" test; added access-path mocks to dams + cleanup tests. 20 PASS in file. +- Next: Phase 3 — `/code-check`, atomic commit (Phase 1+2), buildVignettes, NEWS + bump 0.42.0→0.43.0. diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index 966ee014..5396b7d5 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -34,21 +34,21 @@ on it, not the reverse), so `mapping_code = FALSE` should still produce chunk); `mapping_code = TRUE` sequence is byte-identical after refactor. ## Phase 1 — Hoist access out of the `mapping_code` gate -- [ ] `R/lnk_pipeline_run.R`: move `lnk_presence`, `barriers_per_sp`, pre-persist, +- [x] `R/lnk_pipeline_run.R`: move `lnk_presence`, `barriers_per_sp`, pre-persist, `lnk_barriers_views`, `lnk_pipeline_access` out of `if (isTRUE(mapping_code))` so they run unconditionally (after `lnk_barriers_unify`, before final persist). Keep species-residence classification + `lnk_mapping_code` inside a slimmer gate. -- [ ] Update roxygen header (phase-order list, `@param mapping_code`) to say access +- [x] Update roxygen header (phase-order list, `@param mapping_code`) to say access is always built and only token assembly is gated. Re-`document()`. ## Phase 2 — Update + extend tests -- [ ] Rewrite "composes phases in expected order" (`test-lnk_pipeline_run.R:141`): +- [x] Rewrite "composes phases in expected order" (`test-lnk_pipeline_run.R:141`): default path now includes pre-persist, `barriers_views`, `pipeline_access`, then final persist (two `persist` calls). Add mocks for `lnk_presence`, `lnk_barriers_views`, `lnk_pipeline_access`. -- [ ] Add test: `lnk_pipeline_access` called for BOTH `mapping_code` values; +- [x] Add test: `lnk_pipeline_access` called for BOTH `mapping_code` values; `lnk_mapping_code` called only when `mapping_code = TRUE`. -- [ ] Add new mocks to empty-species / dams / cleanup tests so they keep passing. +- [x] Add new mocks to empty-species / dams / cleanup tests so they keep passing. ## Phase 3 — Verify + release - [ ] `devtools::document()`; `lintr::lint_package()` clean; `devtools::test()` green. diff --git a/tests/testthat/test-lnk_pipeline_run.R b/tests/testthat/test-lnk_pipeline_run.R index 7f0be540..4c1357e0 100644 --- a/tests/testthat/test-lnk_pipeline_run.R +++ b/tests/testthat/test-lnk_pipeline_run.R @@ -174,6 +174,15 @@ test_that("lnk_pipeline_run composes phases in expected order", { m_persist <- function(conn, aoi, cfg, species, schema) { calls <<- c(calls, "persist"); invisible(conn) } + m_presence <- function(...) { + calls <<- c(calls, "presence"); NULL + } + m_views <- function(conn, schema, cfg, species, ...) { + calls <<- c(calls, "barriers_views"); invisible(conn) + } + m_access <- function(conn, ...) { + calls <<- c(calls, "access"); invisible(conn) + } m_exec <- function(conn, sql) { if (grepl("DROP", sql)) calls <<- c(calls, "exec_drop") 1L @@ -191,6 +200,9 @@ test_that("lnk_pipeline_run composes phases in expected order", { lnk_persist_init = m_persist_init, lnk_barriers_unify = m_unify, lnk_pipeline_persist = m_persist, + lnk_presence = m_presence, + lnk_barriers_views = m_views, + lnk_pipeline_access = m_access, { with_mocked_bindings( dbExecute = m_exec, @@ -206,13 +218,18 @@ test_that("lnk_pipeline_run composes phases in expected order", { } ) - # Expected: defensive drop, then phases in order, then species - # resolution, then persist_init, then barriers_unify, then persist. + # Expected: defensive drop, modelling phases, species resolution, + # persist_init, barriers_unify, then the always-on access phase + # (link#218): presence -> pre-persist -> barriers_views -> access, + # then the final persist. mapping_code = FALSE here, so the token + # assembly (lnk_mapping_code) does NOT run. expected_order <- c( "exec_drop", "setup", "load", "prepare", "crossings", "break", "classify", "connect", "species", - "persist_init", "barriers_unify", "persist" + "persist_init", "barriers_unify", + "presence", "persist", "barriers_views", "access", + "persist" ) expect_equal(calls, expected_order) # Returns conn invisibly @@ -241,6 +258,9 @@ test_that("lnk_pipeline_run passes NULL conn_tunnel when dams = FALSE", { lnk_persist_init = m_noop, lnk_barriers_unify = m_noop, lnk_pipeline_persist = m_noop, + lnk_presence = m_noop, + lnk_barriers_views = m_noop, + lnk_pipeline_access = m_noop, { with_mocked_bindings( dbExecute = m_exec, @@ -280,6 +300,9 @@ test_that("lnk_pipeline_run drops working schema when cleanup_working = TRUE", { lnk_persist_init = m_noop, lnk_barriers_unify = m_noop, lnk_pipeline_persist = m_noop, + lnk_presence = m_noop, + lnk_barriers_views = m_noop, + lnk_pipeline_access = m_noop, { with_mocked_bindings( dbExecute = m_exec, @@ -297,3 +320,64 @@ test_that("lnk_pipeline_run drops working schema when cleanup_working = TRUE", { expect_true(drop_schema_seen) }) + +# --------------------------------------------------------------------------- +# Access always runs; mapping_code token assembly is gated (link#218) +# --------------------------------------------------------------------------- + +test_that("lnk_pipeline_run builds access for both mapping_code values, gates lnk_mapping_code", { + run_once <- function(mapping_code) { + seen <- list(access = FALSE, mapping = FALSE) + m_noop <- function(...) invisible(NULL) + m_species <- function(...) c("BT") + m_access <- function(...) { + seen$access <<- TRUE; invisible(NULL) + } + m_mapping <- function(...) { + seen$mapping <<- TRUE; invisible(NULL) + } + m_exec <- function(...) 1L + + with_mocked_bindings( + lnk_pipeline_setup = m_noop, + lnk_pipeline_load = m_noop, + lnk_pipeline_prepare = m_noop, + lnk_pipeline_crossings = m_noop, + lnk_pipeline_break = m_noop, + lnk_pipeline_classify = m_noop, + lnk_pipeline_connect = m_noop, + lnk_pipeline_species = m_species, + lnk_persist_init = m_noop, + lnk_barriers_unify = m_noop, + lnk_pipeline_persist = m_noop, + lnk_presence = m_noop, + lnk_barriers_views = m_noop, + lnk_pipeline_access = m_access, + lnk_mapping_code = m_mapping, + { + with_mocked_bindings( + dbExecute = m_exec, + .package = "DBI", + { + lnk_pipeline_run( + conn = mock_conn(), aoi = "ADMS", + cfg = mock_cfg(), loaded = mock_loaded(), + mapping_code = mapping_code, cleanup_working = FALSE + ) + } + ) + } + ) + seen + } + + off <- run_once(FALSE) + on <- run_once(TRUE) + + # Access builds regardless of the flag. + expect_true(off$access) + expect_true(on$access) + # Token assembly is gated. + expect_false(off$mapping) + expect_true(on$mapping) +}) From 13026daa12247ad0432f38b0de487511c6aefe77 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Mon, 8 Jun 2026 22:39:27 -0700 Subject: [PATCH 3/4] Release v0.43.0 NEWS entry + version bump for #218 (streams_access built regardless of mapping_code. Minor bump: output behavior changes for habitat-only callers (new streams_access emitted) though the signature is unchanged. Co-Authored-By: Claude Opus 4.7 EOF ) --- DESCRIPTION | 4 ++-- NEWS.md | 4 ++++ planning/active/progress.md | 5 ++++- planning/active/task_plan.md | 16 ++++++++-------- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 48137d84..104329cf 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: link Title: Stream Network Habitat Interpretation (Experimental) -Version: 0.42.0 -Date: 2026-06-01 +Version: 0.43.0 +Date: 2026-06-08 Authors@R: c( person("Allan", "Irvine", , "airvine@newgraphenvironment.com", role = c("aut", "cre"), diff --git a/NEWS.md b/NEWS.md index b6ad78b5..8b468cfc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,7 @@ +# link 0.43.0 + +`lnk_pipeline_run()` now builds and persists `.streams_access` (and `access_`) regardless of the `mapping_code` flag ([#218](https://github.com/NewGraphEnvironment/link/issues/218)). Access is foundational — `mapping_code` depends on it, not the reverse — so the access build (`lnk_presence` → pre-persist → `lnk_barriers_views` → `lnk_pipeline_access`) moves out of the `if (isTRUE(mapping_code))` block to run unconditionally; only the `mapping_code` **token assembly** (`lnk_mapping_code`) stays gated. Habitat-only callers (`mapping_code = FALSE` — `data-raw/wsg_pipeline_run.R`, `lnk_compare_wsg(mapping_code = FALSE)`) now also emit `streams_access`. No persist change required: `lnk_pipeline_persist()` already probes for `streams_access` and `streams_mapping_code` independently and copies whichever working tables exist. The `mapping_code = TRUE` execution order (pre-persist → barriers_views → access → mapping_code → final persist) is byte-identical to v0.42.0, so the cached vignette parity (99.04% BT) is unchanged. + # link 0.42.0 First package vignette: `vignettes/pars-habitat-connectivity.Rmd` — bull trout and Arctic grayling habitat and connectivity classification for the Parsnip River Watershed Group (`PARS`, FWCP Peace), rehearsed end-to-end so it can transfer into the Fish Passage Peace 2025 report appendix ([#215](https://github.com/NewGraphEnvironment/link/issues/215)). Two analyses: (1) **parity** — link's `bcfishpass` config reproduces bcfishpass's per-segment `mapping_code` for bull trout at 99.04%; (2) **extension** — link's `default` config models Arctic grayling, which bcfishpass does not model at all. Map symbology reuses the bcfishpass symbology registry bundled in `gq` (`gq::gq_reg_main()` + `gq_tmap_classes()`, the same recipe `fresh` uses), so stream colours match a bcfishpass QGIS project exactly. The vignette is tunnel-free: the model run + comparison run once locally in `data-raw/wsg_vignette_data.R`, which caches artifacts to `inst/vignette-data/` (`pars.gpkg`, `pars_parity.rds`); the vignette only loads those, so pkgdown CI builds it with no Postgres and no bcfishpass snapshot. New Suggests (`bookdown`, `gq`, `knitr`, `rmarkdown`) + `VignetteBuilder: knitr` + `gq` Remote. diff --git a/planning/active/progress.md b/planning/active/progress.md index 07d57784..795084b6 100644 --- a/planning/active/progress.md +++ b/planning/active/progress.md @@ -12,4 +12,7 @@ - Phase 2 — rewrote "composes phases in expected order" (new order: …barriers_unify, presence, persist, barriers_views, access, persist); added "builds access for both mapping_code values, gates lnk_mapping_code" test; added access-path mocks to dams + cleanup tests. 20 PASS in file. -- Next: Phase 3 — `/code-check`, atomic commit (Phase 1+2), buildVignettes, NEWS + bump 0.42.0→0.43.0. +- Phase 1+2 atomic commit `681c7bd` (code + roxygen + Rd + tests + checkbox flips). +- Phase 3 — `/code-check` clean (3 rounds, 0 findings); `buildVignettes` exit 0; + test-lnk_pipeline_run.R 20 PASS. NEWS 0.43.0 entry + DESCRIPTION bump 0.42.0→0.43.0. +- Next: `/planning-archive` → `/gh-pr-push` (PR body: `Closes #218`, SRED tag). diff --git a/planning/active/task_plan.md b/planning/active/task_plan.md index 5396b7d5..e6a556d7 100644 --- a/planning/active/task_plan.md +++ b/planning/active/task_plan.md @@ -51,16 +51,16 @@ on it, not the reverse), so `mapping_code = FALSE` should still produce - [x] Add new mocks to empty-species / dams / cleanup tests so they keep passing. ## Phase 3 — Verify + release -- [ ] `devtools::document()`; `lintr::lint_package()` clean; `devtools::test()` green. -- [ ] `tools::buildVignettes(dir = ".", tangle = FALSE, clean = TRUE)` exits 0. -- [ ] `/code-check` clean → atomic commits (code + checkbox flip) per phase. -- [ ] NEWS.md entry + `DESCRIPTION` bump 0.42.0 → 0.43.0 (final commit). +- [x] `devtools::document()`; `lintr::lint_package()` clean; `devtools::test()` green. +- [x] `tools::buildVignettes(dir = ".", tangle = FALSE, clean = TRUE)` exits 0. +- [x] `/code-check` clean → atomic commits (code + checkbox flip) per phase. +- [x] NEWS.md entry + `DESCRIPTION` bump 0.42.0 → 0.43.0 (final commit). - [ ] `/planning-archive` → `/gh-pr-push` (PR body: `Closes #218`, SRED tag in PR body). ## Validation -- [ ] `devtools::test()` green, incl. rewritten composition test + new gating test -- [ ] `tools::buildVignettes` exits 0 -- [ ] `/code-check` clean on each commit -- [ ] PWF checkboxes match landed work +- [x] `devtools::test()` green, incl. rewritten composition test + new gating test +- [x] `tools::buildVignettes` exits 0 +- [x] `/code-check` clean on each commit +- [x] PWF checkboxes match landed work - [ ] `/planning-archive` on completion From b53b13e0c597182dc683561d6c0ac7c03c6ab297 Mon Sep 17 00:00:00 2001 From: almac2022 Date: Mon, 8 Jun 2026 22:40:01 -0700 Subject: [PATCH 4/4] Archive planning files for issue #218 Co-Authored-By: Claude Opus 4.7 --- .../.gitkeep | 0 .../README.md | 27 +++++++++++++++++++ .../findings.md | 0 .../progress.md | 0 .../task_plan.md | 0 5 files changed, 27 insertions(+) create mode 100644 planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/.gitkeep create mode 100644 planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/README.md rename planning/{active => archive/2026-06-issue-218-streams-access-regardless-mapping-code}/findings.md (100%) rename planning/{active => archive/2026-06-issue-218-streams-access-regardless-mapping-code}/progress.md (100%) rename planning/{active => archive/2026-06-issue-218-streams-access-regardless-mapping-code}/task_plan.md (100%) diff --git a/planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/.gitkeep b/planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/README.md b/planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/README.md new file mode 100644 index 00000000..130db8cc --- /dev/null +++ b/planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/README.md @@ -0,0 +1,27 @@ +## Outcome + +`lnk_pipeline_run()` now builds and persists `.streams_access` +(+ `access_`) regardless of the `mapping_code` flag. The access build +(`lnk_presence` → pre-persist → `lnk_barriers_views` → `lnk_pipeline_access`, +plus the `sp_set`/`barriers_per_sp` locals) moved out of the +`if (isTRUE(mapping_code))` block to run unconditionally; only the mapping_code +**token assembly** (`lnk_mapping_code`) stays gated. Access is foundational — +mapping_code depends on it, not the reverse — so habitat-only callers +(`mapping_code = FALSE`) now also emit `streams_access`. + +Key finding: **no persist change was needed**. `lnk_pipeline_persist()` already +probes `information_schema` for `streams_access` and `streams_mapping_code` +independently and copies whichever working tables exist — so satisfying the +issue's "persist streams_access independently" point was automatic. The +pre-persist (step 0) had to move out alongside access because +`lnk_barriers_views()` defaults to reading `.barriers` +(cross-WSG dam visibility, #196), so the current WSG's barriers must be +persisted before the views are built. The `mapping_code = TRUE` execution order +stayed byte-identical, so the cached vignette parity (99.04% BT) is unchanged — +verified `tools::buildVignettes` exits 0 (vignette renders from cached +artifacts only, `eval = FALSE` pipeline chunk). Tests: rewrote the composition +test for the new call order and added a gating test asserting access builds for +both `mapping_code` values while `lnk_mapping_code` only runs when `TRUE`. +`/code-check` clean across 3 rounds. + +Closed by: commits 681c7bd (hoist + tests) + 13026da (release v0.43.0) / PR TBD (Closes #218) diff --git a/planning/active/findings.md b/planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/findings.md similarity index 100% rename from planning/active/findings.md rename to planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/findings.md diff --git a/planning/active/progress.md b/planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/progress.md similarity index 100% rename from planning/active/progress.md rename to planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/progress.md diff --git a/planning/active/task_plan.md b/planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/task_plan.md similarity index 100% rename from planning/active/task_plan.md rename to planning/archive/2026-06-issue-218-streams-access-regardless-mapping-code/task_plan.md