Add falls to break_order — fix segmentation drift (Fixes #96)#99
Merged
NewGraphEnvironment merged 4 commits intomainfrom May 2, 2026
Merged
Add falls to break_order — fix segmentation drift (Fixes #96)#99NewGraphEnvironment merged 4 commits intomainfrom
NewGraphEnvironment merged 4 commits intomainfrom
Conversation
Methodology decision parked pending BABL inspection — see archive README. Clears planning/active for link#96 (falls in break_order) work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Falls not used as segmentation break source — implementation drift from documented break_order. Five-phase plan: fix + tests + HORS verification + 4-WSG regression + research doc + ship. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
R/lnk_pipeline_break.R lines 10-13 documented the bcfp break order as observations → gradient_minimal → falls → barriers_definite → habitat_endpoints → crossings, but the source_tables list and break_order default both omitted falls. Result: the FWA stream network was never broken at fall positions. Where two falls sit close together (no other break source in between), the resulting segment spanned across the second fall, leaving its upper portion incorrectly classified as accessible. Fix: - Add `falls = paste0(schema, ".falls")` to source_tables. - Add "falls" to the break_order default vector. - Add "falls" to both bundle configs' pipeline.break_order (bcfishpass + default). - Update the # Break sources documentation table to include the falls entry (each fall is its own barrier; not minimal-reduced). HORS verification (the issue's evidence case): BLK 356357296 segment 12671 (1447m straddling the second fall at DRM 67565) split into 12677 (17m below) + 12678 (1429m above, now accessible=FALSE). Total rearing_stream unchanged on HORS (the affected segment is edge_type 1250 Horsefly River construction line, excluded from the rearing_stream metric); broader `rearing` total dropped 1.43 km. HARR verification: BLK 356361157 (7 falls in 13km range) — all 7 fall positions now have segment breaks. Rollup diff vs pre-#96 baseline: BT rearing -0.63 km, BT rearing_stream -0.64 km. Other species/metrics unchanged. Map cache helper hardened — stale 0-row caches (left behind when the pipeline runs for one WSG and the map is rendered for another) now refetch instead of erroring on missing CRS. PWF Phase 1 + 3 checkbox-flipped, Phase 4 (4-WSG regression) next. Fixes #96 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 2 (tests) folded into Phase 5 ship commit:
- 2 new unit tests in tests/testthat/test-lnk_pipeline_break.R:
1. Falls is wired into source_tables — calling the pipeline with
break_order=c("falls","observations") hits w_bulk.falls then
w_bulk.observations_breaks in that order
2. Default break_order (cfg$pipeline$break_order = NULL) includes
falls between gradient_minimal and barriers_definite per the
docstring's documented order
- 33 PASS in test-lnk_pipeline_break.R (was 29 — 4 new expectations
across 2 test_that blocks)
Research doc:
- New § "falls in break_order (#96)" in research/bcfishpass_comparison.md
with evidence trace (HORS BLK 356357296 segment 12671 split at
DRM 67565) and 4-WSG regression table
- Added row to "Key fixes during comparison" table
DESCRIPTION + NEWS bumped 0.22.0 → 0.23.0.
Closes the implementation drift surfaced in link#96. PWF Phase 1+3
already shipped at commit 743e1f8; this commit closes Phase 2 + 5.
Fixes #96
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NewGraphEnvironment
added a commit
that referenced
this pull request
May 2, 2026
PR #99 merged. Clears planning/active for the next task. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fallswas loaded into<schema>.fallsand consumed for access gating + obs/habitat lift but never used as a segmentation break source, despiteR/lnk_pipeline_break.Rlines 10-13 documenting bcfp's order asobservations → gradient_minimal → falls → barriers_definite → habitat_endpoints → crossings. When two falls sat close together with no other break source between them, the resultingfresh.streamssegment spanned the second fall, leaving its upper portion incorrectly classified as accessible.Fix
falls = paste0(schema, ".falls")added tosource_tablesinR/lnk_pipeline_break.R"falls"added tobreak_orderdefault vector (betweengradient_minimalandbarriers_definite)"falls"added topipeline.break_orderin bothinst/extdata/configs/{bcfishpass,default}/config.yamlfrs_barriers_minimal)Verification
HORS BLK 356357296 evidence case: segment 12671 (1447 m straddling fall #2 at DRM 67565) split into 12677 (17 m below, accessible) + 12678 (1429 m above,
accessible=FALSE). Log:data-raw/logs/20260501_27_preflight_hors_falls_break.txt.HARR BLK 356361157 (7 falls in 13 km): all 7 fall positions now have segment breaks. Log:
data-raw/logs/20260501_28_preflight_harr_falls_break.txt.4-WSG regression (HARR/HORS/LFRA/BABL) vs pre-#96 baseline at
data-raw/logs/provincial_parity/:rearing_streammetric unchangedAll deltas negative — fix correctly removes segments above falls. Direction is structurally right regardless of whether each removal closes or slightly deepens the bcfp gap.
Test plan
tests/testthat/test-lnk_pipeline_break.R(33 PASS, was 29)research/bcfishpass_comparison.md§ "falls in break_order (Falls not used as segmentation break source — implementation drift from documented break_order #96)" with the evidence trace + regression tableMap cache helper
data-raw/maps/_lnk_map_compare.Rhardened alongside — stale 0-row caches (left when the pipeline runs for one WSG and the map is rendered for another) now refetch instead of erroring on missing CRS.🤖 Generated with Claude Code