Repository navigation
v0.40.3: persist per-source flags in streams_access (#196) - #199
Merged
Merged
Conversation
…cess (#196) v0.40.0/v0.40.1/v0.40.2 (#187/#192/#194) persisted streams_access with per-species access columns but dropped the per-barrier-source flag columns lnk_pipeline_mapping_code needs to classify the second mapping_code token. Phase 2 of #187 dropped them with incorrect reasoning (claimed conditional on remediations/observations; actually only conditional on barrier_sources which lnk_pipeline_run always passes). Effect: lnk_mapping_code reads SELECT * FROM persist streams_access → has() probes return FALSE for missing cols → second token defaults to NONE for every segment. PARS BT 2026-05-19: link: ACCESS;NONE (4412) SPAWN;NONE (4773) bcfp: ACCESS;DAM (4411) SPAWN;DAM (5263) ACCESS;MODELLED (1775) REAR;DAM (2191) ACCESS;ASSESSED (900) ... Match_pct vs bcfp drops from historic 980 ~30-50
…cross-WSG visibility (#196) Sibling fix to the per-source-flag persistence: even with the flag columns now persisted, lnk_pipeline_access wrote them all FALSE because lnk_barriers_views was pointed at WORKING schema's barriers (per-WSG local only) instead of PERSIST (province-wide). #187 Phase 4 set barriers_table = paste0(schema, '.barriers') as a workaround for the ordering issue that lnk_pipeline_persist runs AFTER the mapping_code phase, so persist barriers wouldn't yet have current WSG. But that loses cross-WSG dam visibility (PARS BT through Bennett/ Peace Canyon dams in PCEA/UPCE — the link#152 fix). Fix: pre-persist current WSG's barriers (and streams, habitat) BEFORE the mapping_code phase. lnk_barriers_views then defaults to persist and sees current + all previously-persisted WSGs. Existing persist call after the mapping_code phase re-runs idempotently (DELETE-WHERE- WSG + INSERT) and picks up streams_access + streams_mapping_code. Cost: ~5s extra per WSG (the redundant streams/habitat/barriers re- write). Worth it for cross-WSG visibility. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ss INSERT (#196) Third #196 commit. lnk_persist_init (commit 91f3f90) added the per-source flag columns to the streams_access DDL, but lnk_pipeline_persist built its INSERT projection from cols_streams_access_base + .lnk_cols_streams_access_per_sp(species) ONLY -- missing .lnk_cols_streams_access_source_flags(). So the DDL had the columns but the INSERT never populated them: they stayed NULL/FALSE in persist. lnk_mapping_code then read NULL flags -> second token defaulted to NONE. Verified in isolation against a surviving working schema: the in-memory access tibble + working streams_access both had the flags populated (anthropogenic 48559, dams 48559, dam_ind 32406 of 48559), but persist streams_access came back all-FALSE. After adding the source-flags generator to access_cols_v, persist streams_access populates: anth 48559, dams 48559 (cross-WSG dams visible), dam_ind 32406. The code-check on the earlier commit asserted the INSERT picks up new columns automatically (iterates names(access_cols_v)) -- wrong, because access_cols_v itself omitted the source flags. Lesson: DDL changes and INSERT-projection changes must be made + verified together. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…revert barriers_per_sp to _unified The mapping_code/access mechanism is documented authoritatively from smnorris/bcfishpass@v0.7.15: per-species access set is natural-only + observation/habitat override; dams are descriptors, never in the access set. link's barriers_per_sp carries dams (known-divergent) — reverted the broken _min swap to runnable _unified; the real fix is scoped as Phase 4d. CLAUDE.md points to RUNBOOK.md + planning/active/HANDOFF.md. (#196) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ies redesign) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…fresh-machine modelling Co-Authored-By: Claude Opus 4.7 <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
streams_access(has_barriers_{anthropogenic,pscis,dams,remediations}_dnstr,dam_dnstr_ind,remediated_dnstr_ind) solnk_pipeline_mapping_code's second token populates instead of defaulting toNONE. Three coupled fixes: DDL (lnk_persist_init), pre-persist barriers for cross-WSG dam visibility (lnk_pipeline_run), INSERT projection (lnk_pipeline_persist— the missing projection was the actual bug).RUNBOOK.md— durable mental model of the barrier → access → mapping_code machinery + the authoritative bcfp access-set mechanism (read fromsmnorris/bcfishpass@v0.7.15).Known follow-up (not in this PR)
The per-species accessibility set still carries dams; it should be natural-only + observation/habitat-overridden (bcfp's model). So dam-downstream segments still emit a bare habitat token, not
SPAWN;DAM. Fully characterized in RUNBOOK §5 with a scoped fix — seeplanning/active/phase4d_plan_draft.md+issue_phase4d_draft.md.Handoff
Mid-stream handoff to M1 in progress — see
planning/active/HANDOFF.md.Test plan
devtools::test()+check()on M1 before merge🤖 Generated with Claude Code