Skip to content

user_barriers_definite no longer eligible for override (#48)#49

Merged
NewGraphEnvironment merged 5 commits intomainfrom
48-user-barriers-definite-bypass
Apr 23, 2026
Merged

user_barriers_definite no longer eligible for override (#48)#49
NewGraphEnvironment merged 5 commits intomainfrom
48-user-barriers-definite-bypass

Conversation

@NewGraphEnvironment
Copy link
Copy Markdown
Owner

@NewGraphEnvironment NewGraphEnvironment commented Apr 23, 2026

Summary

  • user_barriers_definite rows (227 reviewer-added positions province-wide — EXCLUSION zones like Stump Lake Dam, MISC barriers the model doesn't detect from gradient/falls/subsurface) are no longer eligible for observation-based override. Matches bcfishpass's model_access_*.sql shape, which appends barriers_user_definite post-filter via UNION ALL — observations and habitat never re-open those positions.
  • Same-family defect as Wire barriers_definite_control into lnk_barrier_overrides #44 / PR Honour barriers_definite_control at the override step, per-species (#44) #47, different mechanism. Active on ELKR pre-fix: 4 rows in working_elkr.barrier_overrides matched working_elkr.barriers_definite positions (Erickson Creek exclusion + two Spillway MISC entries). Post-fix: 0 matches on all 5 WSGs.
  • Fix is one edit: .lnk_pipeline_prep_natural() drops the INSERT INTO natural_barriers SELECT ... FROM barriers_definite block. barriers_definite is still consumed separately — lnk_pipeline_break() applies it as a sequential break source (segment boundary at each user-definite position), and lnk_pipeline_classify() UNION ALLs it into fresh.streams_breaks (access-gating barrier set).
  • ELKR rollup shifts toward bcfishpass: BT spawning +3.4% → +2.8%, WCT spawning +4.0% → +2.6%, WCT rearing +1.6% → +0.3%. Other four WSGs unchanged: ADMS/BABL/DEAD have empty barriers_definite; BULK has 87 rows but none had observation counts clearing threshold.
  • Version 0.6.0 → 0.7.0. NEWS entry, DESCRIPTION bump, research/bcfishpass_comparison.md (ELKR parity table updated + new "Key fixes" row + new "user_barriers_definite bypass (user_barriers_definite bypasses override in bcfishpass but is eligible in link #48)" subsection), vignette text corrected — old bullet said user-definite barriers were "eligible for per-species override… when enough upstream observations clear the threshold" which accurately described the pre-fix code path but was wrong vs bcfishpass; new bullet says "always-blocking, always a break position, never eligible for observation-based override" with the bcfishpass UNION ALL reference.

Reproducibility: two consecutive tar_destroy + tar_make runs produce bit-identical 46-row rollup tibbles (digest 50908d234e2131fc0842dc3ab653ae78).

Test plan

  • devtools::test() — 279 PASS, 0 FAIL.
  • Test assertion flipped: .lnk_pipeline_prep_natural() now asserted to union gradient + falls only (no definite). expect_no_match with escaped \\. and word boundary \\b correctly excludes the match while still allowing barriers_definite_control to appear elsewhere in the SQL (underscore is a word char, so \b does not match between e and _).
  • tar_make() across 5 WSGs, two consecutive full rebuilds — bit-identical rollups via digest::digest().
  • Direct verification of working_<wsg>.barrier_overrides JOIN working_<wsg>.barriers_definite: 0 matches on all 5 WSGs post-fix (ELKR was 4 pre-fix).
  • ELKR rollup direction confirmed — BT/WCT spawning and rearing all moved toward bcfishpass reference, consistent with the four blocked-upstream user-definite positions.
  • /code-check over 2 rounds — surfaced an inline-comment misattribution and a stale roxygen docstring; both fixed before commit.

Fixes #48
Relates to NewGraphEnvironment/sred-2025-2026#24

🤖 Generated with Claude Code

@NewGraphEnvironment
PWF: archive #44, init #48 (user_barriers_definite bypass)
ca1f44b 
#47 merged as link 0.6.0 (squash 8eda3fb). #48 follows the same
family — user_barriers_definite rows currently eligible for override
in link; bcfishpass appends post-filter. ELKR active test case (4
defect rows).

Relates to #48
@NewGraphEnvironment
Drop user-definite from natural_barriers (bcfishpass parity)
35d262e 
Previously .lnk_pipeline_prep_natural() unioned barriers_definite into
natural_barriers. That table is the set passed to lnk_barrier_overrides()
as the 'barriers' argument, which emits per-species override rows when
observations upstream clear the threshold. Net effect: user-identified
definite barriers (EXCLUSION zones, MISC reviewer-added positions) could
be re-opened by historical observations. Confirmed active on ELKR at
pre-fix: 4 rows in working_elkr.barrier_overrides matched
working_elkr.barriers_definite positions (Erickson Creek exclusion +
two Spillway MISC entries).

bcfishpass treats user-definite barriers differently. Each
model_access_*.sql builds its barriers CTE from gradient + falls +
subsurfaceflow (no user-definite); observations and habitat filters
run only on that set. user_barriers_definite rows are then appended
post-filter to the final per-model table via UNION ALL. They always
block, never eligible for observation override.

Matching that here — dropped the INSERT INTO natural_barriers FROM
barriers_definite block. barriers_definite is still consumed separately:

  - lnk_pipeline_break() applies it as a sequential break source, so
    segmentation still places a boundary at each user-definite position
  - lnk_pipeline_classify() UNION ALLs it directly into
    fresh.streams_breaks, so it blocks access gating

Test assertion flipped from 'unions gradient + falls + definite' to
'unions gradient + falls only (no definite)'. Roxygen on the exported
lnk_pipeline_prepare() and on the @nord helper one-liner updated to
reflect the new shape; both note bcfishpass parity.

Relates to #48
@NewGraphEnvironment
Bump to 0.7.0: NEWS, DESCRIPTION, research doc, vignette, artifacts
3ef7e0c 
Phase 3 of #48. Numerical artifacts regenerated from the post-fix
pipeline; narrative updates document the user-definite bypass.

- DESCRIPTION: Version 0.6.0 -> 0.7.0
- NEWS.md: 0.7.0 entry covering the natural_barriers drop, why
  bcfishpass's post-filter UNION ALL is the correct shape, ELKR shift
  numbers, and the scope-of-fix summary on each WSG.
- inst/extdata/vignette-data/*.rds: regenerated from
  data-raw/vignette_reproducing_bcfishpass.R against the current
  tar_make rollup (46 rows, bit-identical across two full rebuilds;
  digest 50908d234e2131fc0842dc3ab653ae78).
- research/bcfishpass_comparison.md: updates ELKR parity table with
  post-fix numbers and a line of pre-fix context. Adds a row to the
  "Key fixes" table and a new subsection "user_barriers_definite
  bypass (#48)" mirroring the format used for the #44 subsection.
- vignettes/reproducing-bcfishpass.Rmd: corrects the user-identified-
  definite-barriers bullet. Old text said "eligible for per-species
  override via lnk_barrier_overrides when enough upstream observations
  clear the threshold" (accurate to pre-#48 code, wrong vs bcfishpass).
  New text: "always-blocking, always a break position, never eligible
  for observation-based override" with the bcfishpass UNION ALL
  reference.
- data-raw/logs/: 20260423_07 and _08 run logs.
- planning: Phase 1 checkbox in task_plan.md.

Reproducibility verified: two consecutive tar_destroy + tar_make runs
produce bit-identical 46-row rollups.

Relates to #48
@NewGraphEnvironment
Vignette: drop DEAD from parity pivot tables, note in prose
b8cd442 
DEAD was added (PR #47) as a targeted end-to-end test for the
barriers_definite_control filter, not a parity measurement. Mixing it
into the same pivot as ADMS/BULK/BABL/ELKR muddles the table's
purpose. Readers who want DEAD's diff_pct numbers can find them in
research/bcfishpass_comparison.md's dedicated DEAD subsection.

Restores the pivot's intersect() to the original four-WSG list;
adds a short paragraph below the tables pointing at the research
doc for DEAD detail.

Relates to #48
@NewGraphEnvironment
Revert "Vignette: drop DEAD from parity pivot tables, note in prose"
f14ae62 
This reverts commit b8cd442.
@NewGraphEnvironment NewGraphEnvironment merged commit 5cbd75d into main Apr 23, 2026
1 check passed
@NewGraphEnvironment NewGraphEnvironment deleted the 48-user-barriers-definite-bypass branch April 23, 2026 21:14
NewGraphEnvironment added a commit that referenced this pull request Apr 23, 2026
@NewGraphEnvironment
Migrate pipeline probes to manifest-driven gating (#46) (#50)
b2fc181 
* PWF: archive #48, init #46 (manifest-driven probes refactor)

#49 merged as 0.7.0 (5cbd75d), tagged v0.7.0. #46 follows as pure
refactor with bit-identical rollup expected.

Relates to #46

* Migrate pipeline probes to manifest-driven gating

Pure refactor — no behavior change. Two `information_schema.tables`
probes in `R/lnk_pipeline_prepare.R` replaced with direct checks of
the `lnk_config` manifest:

- `.lnk_pipeline_prep_gradient()` gains a `cfg` parameter. The probe
  for `<schema>.barriers_definite_control` before the gradient-barrier
  DELETE is now `!is.null(cfg$overrides$barriers_definite_control)`.
  Caller in `lnk_pipeline_prepare()` updated to pass `cfg`.
- `.lnk_pipeline_prep_overrides()` probe for
  `<schema>.user_habitat_classification` replaced with
  `!is.null(cfg$habitat_classification)`.

Rationale: the config manifest is the declarative contract for which
capabilities a pipeline variant activates. The probes worked because
`.lnk_pipeline_prep_load_aux()` writes each table exactly when the
manifest declares the key, but that indirection made empty-table edge
cases easier to miss (see the asymmetric-gating bug fixed in #44,
rooted in the same seam). Manifest-key gating is consistent with the
per-capability pattern introduced in #44 for the barrier-override
control wiring.

Code-check round-1 surfaced a matching asymmetry on the habitat side:
the new manifest gate at prep_overrides would return non-NULL for a
degenerate (header-only) habitat CSV, but `.lnk_pipeline_prep_load_aux()`
wrote no table in that case, so `lnk_barrier_overrides()` would error
on SELECT from a missing relation. Fixed by mirroring the
`barriers_definite_control` pattern: always create a schema-valid
`user_habitat_classification` table when the manifest declares the
key, empty or populated. Now the manifest key is the true contract.

Tests (`test-lnk_pipeline_prepare.R`):

- Replaced DBI::dbGetQuery mocks in prep_gradient tests with `cfg`
  stubs that either include or omit the manifest key. Two tests
  cover both branches (no-key → no DELETE emitted; key-present →
  DELETE emitted).
- Added two new tests for prep_overrides' habitat gate — manifest
  declares → `habitat = "<schema>.user_habitat_classification"`;
  manifest omits → `habitat = NULL`.

Verification:

- `devtools::test()` 282 PASS 0 FAIL.
- `tar_destroy + tar_make` produces rollup with digest
  `50908d234e2131fc0842dc3ab653ae78` (46 rows) — bit-identical to the
  post-#48 baseline. Second rebuild after the empty-stub fix also
  bit-identical. No behavior drift on the bcfishpass config.

Fixes #46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

user_barriers_definite bypasses override in bcfishpass but is eligible in link

1 participant

@NewGraphEnvironment