Skip to content

Commit 01553ef

Browse files
docs(CLAUDE.md): migrate durable memory to Working Conventions (soul#47)
Moves 14 operating rules out of machine-local Claude memory into the repo so they travel to every machine. Each carries the incident that produced it - link#112 filed on a half-answered "yes"; [x] /code-check clean ticked on #138 without running it (post-hoc run found three real fragility bugs); the CABD falls "gap" that dissolved in five minutes of psql across 187 WSGs; the BT rearing drift that was input state, not an extraction bug. Section sits above the soul-conventions marker; the appendix is untouched. Not migrated: 6 NGE-wide rules (soul candidates, not link's to hold), 3 already canonical in this file, 1 orphan about fresh internals, and 4 stale entries whose issues are closed (#117, #150, #151). Also corrects the header, which claimed v0.44.2 against DESCRIPTION 0.45.1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0166UFHKi483jSLDJi9tUhd7
1 parent 42c5de2 commit 01553ef

1 file changed

Lines changed: 114 additions & 1 deletion

File tree

CLAUDE.md

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Experimental package — breaking all the time and loving the learning curve. St
99
**Repository:** NewGraphEnvironment/link
1010
**Primary Language:** R
1111
**Prefix:** `lnk_`
12-
**Branch:** `main` (v0.44.2 as of 2026-07-06)
12+
**Branch:** `main` (v0.45.1 as of 2026-08-06)
1313

1414
## Status (2026-08-06) — v0.45.1 shipped (WSG drainage closure rebuilt; #227 re-scoped)
1515

@@ -309,6 +309,119 @@ When changing a `configs/<name>/dimensions.csv` or any file that feeds `lnk_rule
309309

310310
Relates to NewGraphEnvironment/sred#24 — crossing connectivity interpretation package.
311311

312+
## Working Conventions
313+
314+
Operating rules learned on this repo. Migrated from machine-local Claude memory
315+
so they travel to every machine (soul#47 recipe).
316+
317+
### Surface design decisions before writing code
318+
319+
For load-bearing choices — function naming, family vs singleton, prefix, scope — present two or three concrete options with tradeoffs and let the user pick.
320+
321+
**Why:** Twice in one session (#65 and the `schema_apply` naming) a design was implemented without consulting and had to be redone. Real design decisions sit *between* agreeing on direction and writing the diff; "auto mode" means execute the chosen path quickly, not skip the choice.
322+
323+
**How to apply:** After agreeing the *what*, ask the *how* — which prefix, one function or a family, what does the package already do? Check `soul/conventions/newgraph.md` for `noun_verb-detail` before inventing a name.
324+
325+
### Read the issue before re-deriving its design
326+
327+
When work references a function or feature that has an issue, read that issue body in full — and scan its closed predecessors — before exploring.
328+
329+
**Why:** An entire session went into re-deriving `frs_order_child` decisions that were documented verbatim in fresh#158, including a predecessor link to fresh#156 closed in its favour with its own analysis.
330+
331+
**How to apply:** `gh issue view <N> --repo <owner>/<repo>` first. Search closed issues for rejected predecessors — "closed in favor of" comments carry the rationale. Check `planning/archive/` for prior PWFs on the topic. If the issue contradicts a hypothesis you're about to test, say so before testing. Note that fresh#158 states link's `frs_order_child` is deliberately **not** chasing bcfp parity — don't assume parity is the goal for any link primitive without checking.
332+
333+
### Detective work before scoping a "gap"
334+
335+
When an issue claims data is missing relative to bcfishpass, run a count plus a row-level diff against the real data before scoping any implementation.
336+
337+
**Why:** link#102 (CABD waterfalls "completeness gap") dissolved in five minutes of `psql`: fresh's `falls.csv` per-WSG barrier counts are byte-identical to `bcfishpass.falls WHERE barrier_ind = true` across all 187 WSGs. The "missing famous falls" framing was wrong — those are mostly `barrier_ind = false` (fishways) or live in `cabd.dams`. A research-doc sentence had implied a divergence that didn't exist at row level.
338+
339+
**How to apply:** Counts from both sides → diff at WSG level → row-level spot-check on one representative WSG → only then scope. Costs 5–10 minutes; has closed issues as not-a-bug and avoided multi-day PRs. Even when data does differ, the count diff sizes the work correctly.
340+
341+
### Build abstract systems, not point solutions
342+
343+
Reuse first, hardcode last, compose. New functions join an existing `lnk_*` family; don't invent a family unless none fits.
344+
345+
**Why:** The pipeline had two near-identical helpers applying CSV-driven `barrier_status` overrides, and a third of the same shape was about to be added. The user wants the system rationalized, not extended — "don't be afraid to start over, might be smarter."
346+
347+
**How to apply:** Before adding a third helper of a shape, consolidate the two that exist. Mirror external systems by view or composition, never a hand-built table. **Don't hide diamonds** — a primitive with utility beyond its caller belongs as a public function at its natural altitude (a "what's downstream" SQL pattern serves water quality stations and sediment samples too, so it belongs in `fresh`'s `frs_network_*`, not a private link helper).
348+
349+
### Never file an issue or PR without body review
350+
351+
Draft the body, show it, and wait for explicit approval before `gh issue create` / `gh pr create`.
352+
353+
**Why:** "Yes write the issue" arrived in the same message as unanswered architectural questions; link#112 was filed prematurely and the user had to edit a public issue after the fact.
354+
355+
**How to apply:** Draft to a tempfile and show it in the response. Architectural questions in the same message mean the design isn't settled — answer those, get sign-off, then file. Only exception: a skill where filing *is* the explicit ask.
356+
357+
### PWF checkboxes are an integrity contract
358+
359+
`- [x]` means the named action ran and reported clean. If it didn't run, the box stays unchecked.
360+
361+
**Why:** `[x] /code-check clean` was ticked on #138 (v0.32.0) without invoking the skill. Run post-hoc, it surfaced three real fragility findings (int4 overflow, silent row loss on FWA-join NULLs, `pts.*` column collision) that should have been caught pre-merge.
362+
363+
**How to apply:** Leave it unchecked, or reword to what actually happened ("deferred — see follow-up"). For a `/code-check` missed on an already-merged PR, run it post-hoc and ship the findings as a follow-up patch.
364+
365+
### Fail loud; the release bar is the full sweep
366+
367+
Before any minor or major release: `devtools::test()`, `devtools::check()`, `lintr::lint_package()`, and live parity **beyond the happy-path case**.
368+
369+
**Why:** Each of these shipped or nearly shipped a quiet regression — a scratch primitive that matched 15613/15647 ADMS arrays but was lossy elsewhere; a consolidate driver that dropped source schemas even when restore failed (lost a cypher's data); a resume gate that conflated "RDS present" with "WSG done" and silently no-op'd a recovery run.
370+
371+
**How to apply:** If you tested ADMS, also test BULK or HORS. If you tested PSCIS barriers, also test dams. When the user says "ship it" and the sweep hasn't run, ask first — seconds versus days. Surface failures inline with a proposed fix rather than papering over them.
372+
373+
### Stamp the environment in every verification log
374+
375+
Verification runs record environment state, not just numbers.
376+
377+
**Why:** A refactor appeared to move BT rearing by 0.4 points against bcfishpass on ADMS. Hours went into hunting an extraction bug that didn't exist — the drift was entirely input state changing between two run dates. Without stamps you cannot tell which input moved.
378+
379+
**How to apply:** Header carries link version + SHA, fresh version + SHA, fwapg dump timestamp or schema hash, bcfishobs row count, bcfishpass reference row counts, and the reference data version. `lnk_stamp()` (#24) should drive this once it ships.
380+
381+
### How to talk about bcfishpass
382+
383+
Never position link or fresh as superseding or replacing bcfishpass. It is the system we learned from and build on.
384+
385+
**Why:** It's community infrastructure maintained by smnorris with contributions from many groups. Framing our work as a replacement misrepresents the relationship.
386+
387+
**How to apply:** "validates against", not "replaces". "builds on the foundation of", not "improves on". "community-maintained override CSVs", not "our data". Frame methodology differences (wetland rearing, intermittent streams) as *our biological defaults*, not corrections. The bcfishpass comparison is a validation step, not the package's goal.
388+
389+
### Never write into smnorris/* without explicit approval
390+
391+
No comments, issues, PRs or any write action in an upstream repo unless the user approves that specific action.
392+
393+
**Why:** Upstream is a third party; every write creates notifications and work for someone else. The user wants a deliberate decision each time, not a judgment call.
394+
395+
**How to apply:** Read-only `gh` calls are fine. Any write (`gh issue create`, `gh pr comment`, `gh api -X POST`, …) waits for "file it" on that specific action. Draft it in chat first. Applies to bcfishpass, bcfishobs, db_newgraph, fwapg, and external orgs generally.
396+
397+
### Sibling-repo work: branch and PR, no comms thread
398+
399+
Working directly in `fresh` or `crate` from a link session is fine when the user directs it. The older comms-first rule was explicitly relaxed.
400+
401+
**How to apply:** Proceed in the sibling repo — but still branch, still open a PR, **never push to a sibling's main**. Keep commits separate per repo; don't bundle a link change and a fresh change into one commit. Ask when an action is destructive or the intent is unclear.
402+
403+
### No issue references in vignettes
404+
405+
Vignettes describe what the package does today. Issue numbers go stale silently.
406+
407+
**Why:** A vignette line reading "seed for link#75, which will turn this CSV into…" would still promise a future state after #75 closed.
408+
409+
**How to apply:** Vignettes link to behaviour and source paths only — no `#NN`, no "will become". Issues are fine and encouraged in NEWS, PR bodies, commit messages, and `research/*.md`. Avoid them in README unless they describe a limitation users need today.
410+
411+
### No manual reference index in _pkgdown.yml
412+
413+
With consistent prefix naming, drop the manual `reference:` section and let pkgdown auto-generate.
414+
415+
**Why:** A manual index recreates groupings the naming already does, and breaks CI whenever a new export isn't added to it — exactly how v0.18.1's pkgdown build failed on `lnk_load_overrides`.
416+
417+
**How to apply:** Keep `_pkgdown.yml` to `url:`, `template:`, and `articles:`. Manual sections only earn their place in packages with mixed prefixes or weak naming.
418+
419+
### fresh tests itself against link's compare script
420+
421+
Both repos are local on the same machine, same Docker DB and tunnel. fresh can run link's compare script directly — we are not the middleman.
422+
423+
**How to apply:** A fresh issue should say `Rscript ~/Projects/repo/link/data-raw/compare_bcfishpass.R BULK` with target numbers, not "we'll test and report back". fresh installs link via `devtools::install_local()` and verifies before pushing.
424+
312425
<!-- BEGIN SOUL CONVENTIONS — DO NOT EDIT BELOW THIS LINE -->
313426

314427

0 commit comments

Comments
 (0)