Skip to content

Commit c5fe1ae

Browse files
Merge pull request #234 from NewGraphEnvironment/233-rename-dimensions-columns-csv-dictionary
#233: config data dictionaries + fresh<->link column ownership boundary (v0.44.3)
2 parents b9f6285 + 4e0b30a commit c5fe1ae

13 files changed

Lines changed: 581 additions & 28 deletions

File tree

CLAUDE.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ Experimental package — breaking all the time and loving the learning curve. St
1515

1616
**Key correction (do NOT re-rabbit-hole):** the pipeline builds `<schema>.crossings` **from DB primitives** (`lnk_pipeline_crossings` → `.lnk_crossings_union`: PSCIS + `fresh.modelled_stream_crossings` + CABD), **not** from `crossings.csv`. The CSV read at `lnk_pipeline_load.R:100` is **vestigial** — the union drops + rebuilds the table before break/classify/mapping_code touch it. So #231 ("consume weekly crossings.csv; repoint pipeline off fresh") was **closed as misdirected**; lessons in `planning/archive/2026-07-issue-231-crossings-from-primitives/README.md` (+ memory `crossings-built-from-primitives`). Freshness lever = `data-raw/snapshot_bcfp.sh` reloading the primitives into the **`fwapg`** DB (last load ~2026-05-26; `lnk_db_conn()` defaults to a `bcfishpass` DB that LACKS them). Opened **#232** — confirm link's built crossings ≈ bcfp's complete `crossings_vw` (the parity reference). Aside: `crossings.csv` was published to `s3://newgraph` (db_newgraph#15, smnorris PR #57) before we realized the models don't consume it — **db_newgraph#16** tracks reconsidering that dump.
1717

18+
## Status (2026-07-31) — v0.44.3 shipped (#233 config dictionaries + ownership boundary)
19+
20+
Both config CSVs now have data dictionaries: `configs/dictionary_dimensions.csv` (renamed from `dimensions_columns.csv`) and the new `configs/dictionary_parameters_fresh.csv` (19 rows — type, group, `owner`, `consumed_by`, default, description). **The point was not documentation, it was stopping the re-derivation:** the fresh↔link `parameters_fresh` column-ownership split had been settled long ago by [fresh#129](https://github.com/NewGraphEnvironment/fresh/issues/129) (fresh 0.12.7 *removed* `observation_*` — "fish passage interpretation belongs in link, not the network engine") but was only findable by archaeology through two repos' planning archives, so it kept getting re-worked from scratch. It is now the `owner` column — **14 fresh-owned engine params, 5 link-owned `observation_*`** — read by `audit_configs.R` §3b instead of a hardcoded `grepl("^observation_", ...)`, and written up in [`RUNBOOK.md`](RUNBOOK.md) §7 "Who owns which `parameters_fresh` column". Adding a link-owned column is now a dictionary edit, not a regex edit.
21+
22+
**Two findings from machine-verifying every `consumed_by` file:line (24/24) rather than inferring them:** link never reads the nine `cluster_*` columns at all — it only passes the frame through (`lnk_pipeline_connect.R:107`) to fresh's `.frs_run_connectivity()`; and **`rear_gradient_min` is read by no code in either package** (recorded as unused, not dropped — fresh owns that schema). **Gotcha worth knowing:** the bundles carry *different* column subsets — bcfishpass `dimensions.csv` has 30 columns to the three `default*` bundles' 32 — so any dictionary/coverage check must assert against the **union**, never a single bundle. Guarded in two layers because `data-raw/` is `.Rbuildignore`d and never runs for an installed package: `tests/testthat/test-dictionaries.R` (+23, the CI-side guard) and the audit's coverage / reverse-consistency / missing-dictionary flags (negative-tested — dropping one row exits 1). Also removed `audit_configs.R`'s hardcoded `setwd("/Users/airvine/...")`: the script now derives its repo root from its own location and resolves paths via `repo_path()`, so it runs from any cwd and mutates none. Open follow-ups unchanged: **#224**, **#225**, **#227** (the `public.wsg_outlet` builder — its absence is the one standing test failure, `test-lnk_wsg_resolve.R:138`).
23+
1824
## Status (2026-07-04) — v0.44.1 shipped (#226 vignette accessible_km)
1925

2026
Extended the PARS vignette with an **Accessible habitat (km)** section proving `accessible_km` bcfp-equivalence (link 6,822.5 vs bcfp 6,822.9 km BT, **−0.01%**; table from cached `inst/vignette-data/pars_accessible.rds`). **Gotcha that bit hard:** "regenerate the vignette artifacts" was NOT docs-only — the two persist configs drift in segmentation because only WSGs re-modelled post-#223 are dense. `fresh` (bcfp config) had PARS at 97,538 segs but `fresh_default` (default/grayling) was still pre-#223 (48,558); the gpkg's single `streams` layer joins `fresh` geometry to `fresh_default` `mapping_code_gr` on `id_segment`, so a naive regen attaches grayling tokens to mismatched geometry → corrupt GR map. Fix: re-model the lagging config (`data-raw/wsg_run_one.R` + `merge=TRUE` recompute via `wsg_recompute_one.R` for cross-WSG `;DAM`) so both share segmentation; `wsg_vignette_data.R` now carries a **segmentation-parity guard** that refuses a mixed build. Any cross-config artifact joined on `id_segment` must verify both sides share segmentation first. mapping_code parity refreshed 99.04%→98.91% (denser post-#223). Open follow-ups unchanged: **#224**, **#225**, **#227**.
@@ -253,7 +259,7 @@ To run the entire province: loop over watershed groups. Or pass any AOI with `sp
253259
- #45 — Gradient classes cleanup (derive from `loaded$parameters_fresh$access_gradient_max`)
254260
- #52 — Channel-class break positions vs gradient thresholds (research)
255261
- #53 — Distribute tar_make across M4 + M1 + db_newgraph
256-
- #75`dimensions_columns.csv` as source-of-truth: auto-gen README + `lnk_rules_build()` validation (CSV seeded in v0.17.0)
262+
- #75`dictionary_dimensions.csv` as source-of-truth: auto-gen README + `lnk_rules_build()` validation (CSV seeded in v0.17.0)
257263

258264
## Recently closed
259265

DESCRIPTION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Package: link
22
Title: Stream Network Habitat Interpretation (Experimental)
3-
Version: 0.44.2
3+
Version: 0.44.3
44
Date: 2026-07-06
55
Authors@R: c(
66
person("Allan", "Irvine", , "airvine@newgraphenvironment.com",

NEWS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
# link 0.44.3
2+
3+
Data dictionaries for both config CSVs, and an end to re-deriving the fresh↔link column-ownership boundary ([#233](https://github.com/NewGraphEnvironment/link/issues/233)). `inst/extdata/configs/dimensions_columns.csv` is renamed `dictionary_dimensions.csv` (pure rename — no code read it; the only live reference was the #75 entry in `CLAUDE.md`), establishing a `dictionary_<file>` convention for its new sibling `dictionary_parameters_fresh.csv`. The new file documents all 19 `parameters_fresh.csv` columns — type, group, `owner`, `consumed_by`, default-when-absent, description — where previously the 9 `cluster_*` columns were undocumented outside fresh's source and the rest lived in scattered prose. Every `consumed_by` is a machine-verified `file:line` (24/24 resolve), which corrected two things worth knowing: link does not read the `cluster_*` columns at all, it only passes the frame through (`R/lnk_pipeline_connect.R:107`) to fresh's `.frs_run_connectivity()`, and `rear_gradient_min` is read by **no code in either package** — recorded as unused rather than dropped, since fresh owns that schema. The `owner` column encodes, rather than restates, the partition settled by [fresh#129](https://github.com/NewGraphEnvironment/fresh/issues/129) (shipped fresh 0.12.7, which removed `observation_*` from fresh: "fish passage interpretation belongs in link, not the network engine"): 14 fresh-owned engine params, 5 link-owned `observation_*` interpretation params. `data-raw/audit_configs.R` §3b now reads that column instead of hardcoding `grepl("^observation_", ...)`, so a new link-owned column is documented once rather than taught to a regex, and gains three guards — undocumented-column coverage, a reverse check (dictionary claims link-owned but fresh ships it = stale dictionary), and a missing-dictionary guard; negative-tested by dropping a row, which exits 1. Because `data-raw/` is `.Rbuildignore`d and never runs for an installed package, the durable guard is the new `tests/testthat/test-dictionaries.R` (+23 tests), which checks coverage in both directions against the **union** of bundle columns — bundles legitimately carry different subsets, bcfishpass's `dimensions.csv` having 30 columns to the defaults' 32. `RUNBOOK.md` §7 gains the ownership section so the boundary is findable without archaeology through two repos' planning archives. Separately, `audit_configs.R` no longer `setwd()`s to a hardcoded `/Users/airvine/...` path — it derives the repo root from its own script location and resolves every path through a new `repo_path()` helper, so the audit runs from any working directory (verified from `/tmp`), works for anyone's checkout, and mutates no global state when `source()`d.
4+
15
# link 0.44.2
26

37
Reader-facing prose cleanup on the PARS vignette (`vignettes/pars-habitat-connectivity.Rmd`), from review — no model, artifact, or numbers change. Dropped internal issue numbers and build-infrastructure ("tunnel") references, and cut the per-segment `mapping_code` disagreement detail (the top-diff-pattern columns and the "remaining disagreements" paragraph) since bull-trout parity is ~99%. The accessible / spawning / rearing km table stays but is reframed around `lnk_rollup_wsg()` as a standalone habitat-totals roll-up — link's own numbers, no reference required — that happens to match bcfishpass, rather than a proof-of-parity exercise. Removed the `mapping_code` symbology paragraph entirely: the colour semantics were easy to mis-state (e.g. red marks an accessible reach whose downstream PSCIS crossing is ranked a *barrier*, not merely "field-assessed"), and each map already carries its own colour legend.

RUNBOOK.md

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -417,9 +417,39 @@ direction. Not yet scoped; candidate issue.
417417
| Habitat dimensions (spawn/rear by gradient, channel width, lake/stream, …) | `configs/<name>/dimensions.csv``lnk_rules_build()``rules.yaml` | `frs_habitat_classify()` (token1 habitat) |
418418
| Species residence (resident/anadromous/spawn-only) | **hardcoded** defaults in `lnk_pipeline_mapping_code()` | which mc_barrier flavor + spawn-only token1 |
419419
| Dam / anthropogenic blocking | **nowhere** — universal `all species` in `lnk_barriers_unify` | `blocks_species` (§2a). Not rules-driven. |
420-
421-
Two gaps worth knowing: **species residence** is hardcoded (data-drive is
422-
follow-up #189), and **dam blocking is not rules-driven** at all (universal).
420+
| What each config column means | `configs/dictionary_dimensions.csv`, `configs/dictionary_parameters_fresh.csv` | data dictionaries — per-column type, group, default, description, and (for `parameters_fresh`) `owner` + `consumed_by` file:line |
421+
422+
### Who owns which `parameters_fresh` column
423+
424+
**Do not re-derive this.** `parameters_fresh.csv` is co-owned, and the split is
425+
settled:
426+
427+
- **fresh owns the 14 network-engine columns**`species_code`,
428+
`access_gradient_max`, the two `*_gradient_min`, and the nine `cluster_*`.
429+
fresh ships them in its own `inst/extdata/parameters_fresh.csv`; link's
430+
bundles are seeded from it.
431+
- **link owns the 5 `observation_*` columns** — fish-passage interpretation
432+
(counts, thresholds, date windows, species pooling, control veto).
433+
434+
The boundary was decided in
435+
[fresh#129](https://github.com/NewGraphEnvironment/fresh/issues/129) (shipped
436+
fresh 0.12.7), which *removed* `observation_*` from fresh after fresh#69 had
437+
added them: "fish passage interpretation belongs in link, not the network
438+
engine." Values may diverge freely per bundle — link tunes them — but the
439+
**column set** is contractual.
440+
441+
Enforced in two places, both of which read `dictionary_parameters_fresh.csv`'s
442+
`owner` column rather than hardcoding the rule:
443+
`data-raw/audit_configs.R` §3b (pre-trifecta gate) and
444+
`tests/testthat/test-dictionaries.R` (runs in CI, since `data-raw/` is
445+
`.Rbuildignore`d). Direction of travel is opposite for the two shared artifacts:
446+
`rules.yaml` flows **link → fresh** (link owns the generator, `lnk_rules_build()`);
447+
the `parameters_fresh` column schema flows **fresh → link**.
448+
449+
Three gaps worth knowing: **species residence** is hardcoded (data-drive is
450+
follow-up #189), **dam blocking is not rules-driven** at all (universal), and
451+
`rear_gradient_min` is carried in the schema but **read by no code in either
452+
package** — it is fresh-owned, so removing it is a fresh-side call.
423453
If dam blocking should ever become species-specific, it's a new
424454
per-source-per-species column + `lnk_barriers_unify` change — not a tweak.
425455

0 commit comments

Comments
 (0)