Skip to content

#233: config data dictionaries + fresh<->link column ownership boundary (v0.44.3) - #234

Merged
NewGraphEnvironment merged 10 commits into
mainfrom
233-rename-dimensions-columns-csv-dictionary
Jul 31, 2026
Merged

#233: config data dictionaries + fresh<->link column ownership boundary (v0.44.3)#234
NewGraphEnvironment merged 10 commits into
mainfrom
233-rename-dimensions-columns-csv-dictionary

Conversation

@NewGraphEnvironment

Copy link
Copy Markdown
Owner

Summary

  • Renames configs/dimensions_columns.csvdictionary_dimensions.csv and adds the missing sibling dictionary_parameters_fresh.csv (19 rows: type, group, owner, consumed_by, default, description). The nine cluster_* columns had no documentation anywhere outside fresh's source.
  • Ends the re-derivation of the fresh↔link column-ownership boundary. It was settled by Replace observations parameter with barrier_overrides from link fresh#129 (fresh 0.12.7 removed observation_*: "fish passage interpretation belongs in link, not the network engine") but was findable only by archaeology through two repos' planning archives. Now encoded as the owner column — 14 fresh-owned engine params, 5 link-owned observation_* — and written up in RUNBOOK.md §7.
  • Makes the dictionary load-bearing. audit_configs.R §3b reads owner instead of hardcoding grepl("^observation_", ...), so adding a link-owned column is a dictionary edit rather than a regex edit.
  • Removes audit_configs.R's hardcoded setwd("/Users/airvine/...") — it now derives its repo root from its own script location and resolves paths via repo_path().

Two findings from verifying rather than assuming

Every consumed_by is a machine-verified file:line (24/24 resolve). That surfaced two things the issue body had wrong or missing:

  • link never reads the cluster_* columns. It only passes the frame through at R/lnk_pipeline_connect.R:107 to fresh's .frs_run_connectivity(); the real readers are fresh/R/frs_habitat.R:1164-1230. The issue body's "→ lnk_pipeline_connect.R" was imprecise and is corrected in the dictionary.
  • rear_gradient_min is read by no code in either package. Header-only in fresh's canonical CSV and every link bundle. Recorded as unused rather than dropped — fresh owns that schema, so removal is a fresh-side call.

Gotcha worth knowing

The bundles carry different column subsets — bcfishpass dimensions.csv has 30 columns to the three default* bundles' 32. Any coverage check must assert against the union across bundles; a per-bundle equality assertion would be wrong.

Related Issues

Test plan

  • devtools::test()1317 PASS (+23 new), 1 pre-existing failure
  • Rscript data-raw/audit_configs.R — 0 findings, exit 0
  • Audit runs clean from /tmp (outside the repo), and source()ing it leaves cwd untouched — the properties the hardcoded setwd was hiding
  • Coverage guard negative-tested: dropping one dictionary row makes the audit exit 1 and fire both flags; file restored byte-identical
  • lintr — no new lints (test-dictionaries.R 0; audit_configs.R 13, unchanged from baseline)
  • Old-name sweep clean (only the historical NEWS.md entry and the archived findings retain it, both intentional)

Notes

One test fails and it is not from this branch. test-lnk_wsg_resolve.R:138 needs DB table public.wsg_outlet, whose builder is open follow-up #227. It is red on main too — a CSV rename cannot create a Postgres table.

Guarding needed two layers because data-raw/ is .Rbuildignored and the audit never runs for an installed package: tests/testthat/test-dictionaries.R is the CI-side guard; the audit is the pre-trifecta gate.

Generated with Claude Code

NewGraphEnvironment and others added 10 commits July 29, 2026 16:42
Establishes the dictionary_<file> naming convention ahead of adding
dictionary_parameters_fresh.csv. Pure rename — no code reads the file;
the only live reference was the #75 entry in CLAUDE.md.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_0166UFHKi483jSLDJi9tUhd7
Guards both config dictionaries against the CSVs they describe: coverage
in each direction, plus the fresh<->link owner partition settled by
NewGraphEnvironment/fresh#129. data-raw/audit_configs.R is .Rbuildignore'd
and never runs for an installed package, so testthat is the durable guard.

Red until Phase 3 authors dictionary_parameters_fresh.csv: 6 FAIL / 9 PASS.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_0166UFHKi483jSLDJi9tUhd7
19 rows documenting every parameters_fresh.csv column: type, group, owner,
consumed_by, default, description. Turns the Phase 2 contract green.

The owner column encodes the partition settled by NewGraphEnvironment/fresh#129
rather than restating it: 14 fresh-owned engine params, 5 link-owned
observation_* interpretation params.

Every consumed_by file:line machine-verified against source (24/24). Records
that link only passes the frame through for cluster_* (the real readers are in
fresh), and that rear_gradient_min has no reader in either package.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_0166UFHKi483jSLDJi9tUhd7
Section 3b hardcoded the fresh<->link partition as grepl("^observation_").
It now reads the owner column from dictionary_parameters_fresh.csv, so a new
link-owned column is documented once rather than taught to a regex.

Adds coverage (undocumented column flags), a reverse consistency check
(dictionary says link-owned but fresh ships it = stale dictionary), and a
missing-dictionary guard. Negative-tested: dropping a row exits 1.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_0166UFHKi483jSLDJi9tUhd7
RUNBOOK section 7 gains the dictionary row and a 'Who owns which
parameters_fresh column' subsection: the 14/5 split, fresh#129 as the
deciding issue, both enforcement points, and the opposite directions of
travel for rules.yaml vs the column schema. Also records rear_gradient_min
as a third 'gap worth knowing'.

The boundary was previously findable only by archaeology through two repos'
planning archives, which is why it kept getting re-derived.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_0166UFHKi483jSLDJi9tUhd7
The script setwd()'d to /Users/airvine/Projects/repo/link, making it
unrunnable for anyone else and silently wrong from a second checkout.

It now derives the repo root from its own script location (--file= under
Rscript, sourcing-frame ofile when source()d) and resolves all 11 config
paths through a repo_path() helper. No setwd at all, so a source() leaves
the caller's working directory untouched.

Verified: exits 0 with no findings when run from /tmp; cwd unchanged
across a source(). No new lints.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_0166UFHKi483jSLDJi9tUhd7
@NewGraphEnvironment
NewGraphEnvironment merged commit c5fe1ae into main Jul 31, 2026
1 check passed
@NewGraphEnvironment
NewGraphEnvironment deleted the 233-rename-dimensions-columns-csv-dictionary branch July 31, 2026 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rename dimensions_columns.csv → dictionary_dimensions.csv; add dictionary_parameters_fresh.csv

1 participant