diff --git a/.Rbuildignore b/.Rbuildignore index 8915c92..f294ed3 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -4,3 +4,14 @@ ^pkgdown$ ^\.github$ ^\.git$ +^CLAUDE\.md$ +^planning$ +^scripts$ +^data-raw$ +^dev$ +^logs$ +^\.claude$ +^\.lintr$ +^CITATION\.cff$ +^data$ +^research$ diff --git a/planning/archive/2026-10-issue-100-rbuildignore/README.md b/planning/archive/2026-10-issue-100-rbuildignore/README.md new file mode 100644 index 0000000..68f0e88 --- /dev/null +++ b/planning/archive/2026-10-issue-100-rbuildignore/README.md @@ -0,0 +1,25 @@ +## Outcome + +`.Rbuildignore` had held its six scaffold lines since the first commit, so every tarball and +every GitHub install shipped `planning/`, `scripts/`, `data-raw/`, `dev/`, `logs/`, `.claude/`, +`CLAUDE.md`, `CITATION.cff` and `.lintr`. Added an anchored pattern for each, plus `data/` +(gitignored working data, no package datasets) and `research/` (pre-emptive). Verified by +building and listing the tarball rather than by reading the file, and by checking each +pattern against the tree with `tools:::inRbuildignore()`. The `/gh-pr-merge` shipped-change +gate, which derives its pathspec from `.Rbuildignore`, now reads a planning-only or +CLAUDE.md-only merge as non-shipped. Three code-check rounds were all Clean. + +## Measurement + +R 4.5.2, `R CMD build --no-build-vignettes --no-manual` on clean copies of the tree: +tarball **307 → 92 files, 7.90 → 7.15 MB**; only `DESCRIPTION NAMESPACE NEWS.md README.md +LICENSE R man tests inst vignettes` remain. A build from the real working tree, with untracked +and ignored files included, gives the identical listing, so local builds no longer sweep +up `logs/*.log` (~2.3 MB) or `data/backfill`. `R CMD check --no-manual --ignore-vignettes`: +**5 NOTEs → 2**, tests passing in both. The first check attempt stopped at "package +dependencies" on both tarballs because Suggests `aws.s3` and `ecmwfr` are not installed +locally; rerun with `_R_CHECK_FORCE_SUGGESTS_=false`. The two remaining NOTEs predate this +work (#111). A reviewer's side finding, an unread 4.9 MB `inst/extdata/context_kotl.gpkg`, +was confirmed and filed as #112. Details in `findings.md`. + +Closed by: commit f42f30e / PR #113 diff --git a/planning/archive/2026-10-issue-100-rbuildignore/findings.md b/planning/archive/2026-10-issue-100-rbuildignore/findings.md new file mode 100644 index 0000000..5214eba --- /dev/null +++ b/planning/archive/2026-10-issue-100-rbuildignore/findings.md @@ -0,0 +1,78 @@ +# Findings — .Rbuildignore: planning/, CLAUDE.md, scripts/, data-raw/ ship in the package tarball (#100) + +## Issue context + +**If we do it:** `R CMD build` ships the package and nothing else. **If we never do:** every tarball built from this repo — and every install from GitHub — carries `planning/` (review files, PWF logs), `CLAUDE.md`, `scripts/` (producer pipeline, incl. `_lib.py`), `data-raw/`, `dev/`, `logs/` and `.claude/` into the user's library. + +## Problem + +`.Rbuildignore` has held six lines since the scaffold commit (`3ca80a4`, plus `^\.git$` in `3088af7`): + +``` +^LICENSE\.md$ ^_pkgdown\.yml$ ^docs$ ^pkgdown$ ^\.github$ ^\.git$ +``` + +Checked with `tools:::inRbuildignore()` on 2026-09-29: `CLAUDE.md`, `planning`, `scripts`, `data-raw`, `logs` all return `FALSE`. Surfaced by `/gh-pr-merge`'s shipped-change gate while releasing v0.5.1 — the gate reported `planning/archive/…` as shipped, so it cannot currently tell a docs-only merge from a code merge either. + +Measured 2026-09-30 (from #107, closed as a duplicate of this issue): `R CMD build --no-build-vignettes --no-manual` on `git archive HEAD` (v0.5.3 plus the #98 branch) produced a tarball with these top-level entries, about 230 internal files in all: + +``` +184 planning + 20 scripts + 16 data-raw + 5 logs + 2 dev + 2 .claude + 1 CLAUDE.md + 1 CITATION.cff + 1 .lintr +``` + +`devtools::check()` on v0.5.6 reports this as the "hidden files", "portable file names" and "top-level files" NOTEs. Add `^research$` too if a `research/` directory is ever created. + +## Proposed Solution + +- Add `^CLAUDE\.md$`, `^planning$`, `^scripts$`, `^data-raw$`, `^dev$`, `^logs$`, `^\.claude$`, `^\.lintr$`, `^CITATION\.cff$`, `^README\.Rmd$` if present, and `^data$` (gitignored working data), checking each against `ls -A`. +- Verify with `R CMD build` and `tar tzf` on the result, not by reading the file (`.Rbuildignore` has no comment syntax — every line is a live regex). +- `R CMD check` for NOTEs about non-standard top-level files, before and after. + +## Errors Encountered + +| Error | Resolution | +|-------|------------| + +## Phase 1 — pattern check (2026-10-01) + +`tools:::inRbuildignore()` against `ls -A`: each new pattern matches exactly its one top-level +entry (`^research$` matches nothing — no such directory yet). Kept top-level after the change: +`.gitignore DESCRIPTION inst LICENSE man NAMESPACE NEWS.md R README.md tests vignettes` +(`.gitignore` is dropped later by R's built-in excludes). Over every tracked file outside the +excluded directories, the only matches are `.Rbuildignore .lintr CITATION.cff CLAUDE.md +LICENSE.md _pkgdown.yml` — all intended; nothing under `R/ man/ tests/ inst/ vignettes/`. + +## Phase 2 — build and check, before vs after (2026-10-01, R 4.5.2) + +Both tarballs built with `R CMD build --no-build-vignettes --no-manual` from clean copies: +before = `git archive` of `1e5b450` (v0.5.6 + CLAUDE.md sync), after = `git checkout-index` +of the staged tree (`.Rbuildignore` byte-identical to `f42f30e`, checked with `cmp`). + +| | files | size | top-level entries | +|---|---|---|---| +| before | 307 | 7.90 MB | + planning 201, scripts 20, data-raw 16, logs 5, dev 2, .claude 2, CLAUDE.md, CITATION.cff, .lintr | +| after | 92 | 7.15 MB | DESCRIPTION NAMESPACE NEWS.md README.md LICENSE R man tests inst vignettes | + +`R CMD check --no-manual --ignore-vignettes` (`_R_CHECK_FORCE_SUGGESTS_=false` — Suggests +`aws.s3`, `ecmwfr` not installed here; the first attempt without it stopped at "package +dependencies" on both tarballs): **5 NOTEs → 2**, tests pass in both. Gone: "hidden files +and directories" (`.lintr`, `.claude`, `.gitkeep`s), "portable file names" (three +`planning/archive/` paths over 100 bytes), "CITATION file in a non-standard place" +(`CITATION.cff`). Remaining, pre-existing and unrelated — unused `sf` import and +`.data`/`.env` globals — filed as #111. + +Code-check round 3 also built from the real working tree (untracked + ignored files +included): identical listing, so `logs/*.log` (~2.3 MB), `data/backfill`, `data/update` and +`scripts/__pycache__` no longer reach a local build. It ran `/gh-pr-merge`'s shipped-change +pathspec loop against the new `.Rbuildignore`: `v0.5.6..HEAD` (CLAUDE.md sync, CITATION.cff +bot commit, PWF baseline) now reports nothing shipped; with the old file it listed all three. +Side finding, verified with `git grep`: `inst/extdata/context_kotl.gpkg` (4.9 MB) is read by +nothing — filed as #112. diff --git a/planning/archive/2026-10-issue-100-rbuildignore/progress.md b/planning/archive/2026-10-issue-100-rbuildignore/progress.md new file mode 100644 index 0000000..e26da80 --- /dev/null +++ b/planning/archive/2026-10-issue-100-rbuildignore/progress.md @@ -0,0 +1,12 @@ +# Progress — .Rbuildignore: planning/, CLAUDE.md, scripts/, data-raw/ ship in the package tarball (#100) + +## Session 2026-10-01 + +- Plan-mode exploration — phases approved by user +- Baseline measured: `R CMD build` of `git archive HEAD` ships 248 internal files +- Created branch `100-rbuildignore-planning-claude-md-scripts` off main +- Scaffolded PWF baseline from issue #100 with approved phases +- Next: start Phase 1 +- Phase 1: `.Rbuildignore` +11 patterns; `/code-check` 3 rounds, all Clean (`f42f30e`) +- Phase 2: tarball 307 → 92 files; R CMD check 5 → 2 NOTEs, tests pass; filed #111 (remaining NOTEs) and #112 (unused 4.9 MB gpkg) +- Next: archive, PR diff --git a/planning/archive/2026-10-issue-100-rbuildignore/review-round1.md b/planning/archive/2026-10-issue-100-rbuildignore/review-round1.md new file mode 100644 index 0000000..365eb2b --- /dev/null +++ b/planning/archive/2026-10-issue-100-rbuildignore/review-round1.md @@ -0,0 +1,11 @@ +# Review round 1 — #100 `.Rbuildignore` diff + +## Clean + +No issues found. + +Checked: +- Every new line is anchored `^...$`, dots escaped (`CLAUDE\.md`, `\.claude`, `\.lintr`, `CITATION\.cff`), no trailing whitespace or CR (`cat -et`). No comment lines. +- `inRbuildignore` matches against paths relative to the package root, so `^data$` / `^dev$` cannot hit `inst/extdata`, `inst/vignette-data` or any nested `data/`. +- Nothing in `R/`, `tests/`, `inst/` reads `scripts/`, `data-raw/`, `planning/`, `logs/`, `CITATION.cff` or `data/`; vignette mentions of `data-raw/` are comments/captions only. DESCRIPTION has no `LazyData`; `data/` holds only gitignored `backfill/` and `update/`, so excluding it removes local working data from local builds and breaks nothing. +- CI: `climate-update.yml` installs via `local::.` but runs `scripts/pipeline_update_edh.R` and writes `logs/` from the checkout, not from the tarball. `pkgdown.yaml` builds from the checkout (and already `rm -f CLAUDE.md` for the site surface). `update-citation-cff.yaml` reads/writes `CITATION.cff` in the checkout. `.lintr` is consumed by lintr from the source tree only. diff --git a/planning/archive/2026-10-issue-100-rbuildignore/review-round2.md b/planning/archive/2026-10-issue-100-rbuildignore/review-round2.md new file mode 100644 index 0000000..3e96868 --- /dev/null +++ b/planning/archive/2026-10-issue-100-rbuildignore/review-round2.md @@ -0,0 +1,13 @@ +# Review round 2 — #100 `.Rbuildignore` (consumers of the built package) + +## Clean + +No issues found. + +Angle: consumers of the built or installed package, not the source tree. + +- **Installed-path lookups.** I grepped `R/`, `tests/`, `vignettes/`, `inst/`, `scripts/`, `data-raw/` and `.github/` for `system.file`, `find.package`, `path.package`, `here::here`, `../../` and `source(`. Every `system.file()` call targets `inst/extdata` or `inst/vignette-data`, and both ship in the tarball. Nothing resolves `scripts/`, `logs/`, `data/`, `dev/`, `research/`, `planning/` or `CITATION.cff` through the installed package. The `here::here()` calls are in `scripts/rag_*.R`, which run from the checkout. In `R/`, the only `system()` call (`cd_s3_push.R:51`) shells out to the aws CLI, not to a repo script. +- **`^data$`.** This is the one exclusion that could have bitten, because `data/` is R's reserved dataset directory. Here it is safe. `data/` holds nothing tracked (only the local `backfill/` and `update/` working dirs), DESCRIPTION has no `LazyData`, and `R/` documents no datasets. Excluding it also keeps multi-GB local backfill data out of a locally built tarball. +- **`local::.` installs** (climate-update.yml, pkgdown.yaml). pak builds through `.Rbuildignore`, so the installed `cd` lacks `scripts/`, but the workflow runs `Rscript scripts/pipeline_update_edh.R` from the checkout. Both pipeline scripts only `library(cd)` or fall back to `load_all()`, and neither reads anything from the installed package's directory. +- **pkgdown.** It runs `build_site_github_pages(install = FALSE)`, which renders from the source checkout against the `local::.` install. `_pkgdown.yml` references none of the excluded paths. CITATION.cff is not read by pkgdown, which uses `inst/CITATION`. +- **R CMD check on the tarball.** It contains all 22 test files plus `tests/testthat.R`, the two vignette Rmds and `vignettes/references.bib`. In the vignettes, `data-raw/` appears only in an HTML comment, code comments and a caption string, never as a path that gets read. `example_catalog.json` hrefs are relative to `inst/extdata`. diff --git a/planning/archive/2026-10-issue-100-rbuildignore/review-round3.md b/planning/archive/2026-10-issue-100-rbuildignore/review-round3.md new file mode 100644 index 0000000..80ff110 --- /dev/null +++ b/planning/archive/2026-10-issue-100-rbuildignore/review-round3.md @@ -0,0 +1,51 @@ +# Review round 3: completeness (is anything internal still shipping?) + +## Clean + +No issues found in the diff. + +### What was checked (2026-10-01) + +1. **Working-tree build, not the clean copy.** I rsynced the real working tree (untracked and + gitignored files included, `.git` left out since `^\.git$` already covers it) to a temp dir and + ran `R CMD build --no-build-vignettes --no-manual` on it. `tar tzf` of the result matches the + clean `checkout-index` tarball in `scratchpad/after/` exactly (`diff` reports nothing). The + working tree's untracked and ignored contents all sit under directories that are now excluded: + - `scripts/__pycache__/*.pyc` → `^scripts$` + - `planning/active/review-round1.md` (untracked) → `^planning$` + - `logs/*.log` (~2.3 MB) → `^logs$` + - `data/backfill/`, `data/update/` → `^data$` + - `.claude/visibility` → `^\.claude$` + + `find` turned up no `.DS_Store`, `.Rhistory`, `.RData`, `.Rproj.user`, `*.Rproj`, `*_cache/`, + `*_files/` or stray `*.html` anywhere, and no `inst/doc`. A `devtools::build()` that builds + vignettes adds `inst/doc`, which is intended. +2. **Kept directories.** The tarball's non-`R/`, non-`man/*.Rd`, non-`tests/testthat/*` entries + are `DESCRIPTION LICENSE NAMESPACE NEWS.md README.md`, `man/figures/logo*.png`, + `tests/testthat.R`, the two vignette Rmds plus `references.bib`, and `inst/extdata/*` plus + `inst/vignette-data/*`. None of them is a note, a review file or scratch. `tests/testthat/` + holds only `test-*.R`, with no `_problems/` and no `testthat-problems.rds`. +3. **The `/gh-pr-merge` shipped-change gate.** I ran the gate's own loop + (`soul/skills/gh-pr-merge/SKILL.md` lines 309-330) against the staged `.Rbuildignore`. Each new + pattern turns into a working `:(exclude)` pathspec (`CLAUDE.md`, `planning`, `.claude`, `.lintr`, + `CITATION.cff`, `data`, `research`, …). Results: + - `v0.5.6..HEAD` (the CLAUDE.md sync, the CITATION.cff auto-update and the PWF baseline): empty + output with rc 0, so `SHIPPED_CHANGED=0`. With the old six-line `.Rbuildignore`, the same range + listed `CITATION.cff CLAUDE.md planning/...`, i.e. it counted as shipped. + - The index against `v0.5.6` (this PR) lists `.Rbuildignore`, so it counts as shipped. That is + correct, because it does change the tarball. + + The issue's use case holds: after this change, a docs-only merge that touches only `planning/` + or `CLAUDE.md` (or the post-release CITATION.cff bot commit) counts as non-shipped. + +### Out-of-scope observations (pre-existing, not introduced by this diff) + +- `inst/extdata/context_kotl.gpkg` (4.8 MB, about a third of the 12.5 MB installed size) ships, + but nothing in `R/`, `tests/`, `vignettes/` or `README.md` reads it. Only + `data-raw/example_context_kotl.R` and `data-raw/example_context_fwcp_peace.R` mention it. The + vignette uses `context_kootenay_lake.gpkg`. It looks like a superseded artifact, but it is not + internal and it breaks nothing. Worth a separate issue if package size matters. + (`example_aoi_kotl.gpkg` *is* used, by the README example.) +- The gate does not exclude `.gitignore`, which R drops by default, so a merge touching only + `.gitignore` would still count as shipped. It fails in the conservative direction (it releases), + so nothing is lost. diff --git a/planning/archive/2026-10-issue-100-rbuildignore/task_plan.md b/planning/archive/2026-10-issue-100-rbuildignore/task_plan.md new file mode 100644 index 0000000..b1fb015 --- /dev/null +++ b/planning/archive/2026-10-issue-100-rbuildignore/task_plan.md @@ -0,0 +1,37 @@ +# Task: .Rbuildignore: planning/, CLAUDE.md, scripts/, data-raw/ ship in the package tarball (#100) + +`.Rbuildignore` has held six lines since the scaffold commit (`3ca80a4`, plus `^\.git$` in `3088af7`). +Every tarball built from this repo — and every install from GitHub — carries `planning/`, `CLAUDE.md`, +`scripts/`, `data-raw/`, `dev/`, `logs/` and `.claude/` into the user's library. Measured 2026-10-01 on +`git archive HEAD` (v0.5.6): 201 planning, 20 scripts, 16 data-raw, 5 logs, 2 dev, 2 .claude, plus +`CLAUDE.md`, `CITATION.cff`, `.lintr`. + +`data/` holds no tracked files (gitignored `backfill/`, `update/` working data only — no package +datasets), so `^data$` is safe. No test, vignette or R file reads any excluded directory at runtime. + +## Phase 1: Exclude internal top-level entries +- [x] Append to `.Rbuildignore`, one live regex per line, no comments: + `^CLAUDE\.md$`, `^planning$`, `^scripts$`, `^data-raw$`, `^dev$`, `^logs$`, + `^\.claude$`, `^\.lintr$`, `^CITATION\.cff$`, `^data$`, `^research$` + (`^research$` pre-emptively, per the planning convention; `^README\.Rmd$` omitted — no such file) +- [x] Check each pattern matches its target and nothing else with `tools:::inRbuildignore()` + against `ls -A` + `git ls-files` + +## Phase 2: Verify by building, not by reading +- [x] `git archive` the branch into the scratchpad, `R CMD build --no-build-vignettes --no-manual`, + `tar tzf | cut -d/ -f2 | sort | uniq -c` — expect only `DESCRIPTION NAMESPACE NEWS.md + README.md LICENSE R man tests inst vignettes` (+ `build/` if produced) +- [x] `R CMD check --no-manual --ignore-vignettes` on the before and after tarballs; record the + "hidden files" / "portable file names" / "top-level files" NOTEs disappearing +- [x] Record before/after counts and NOTE diff in `findings.md` + +## Phase 3: Wrap up +- [x] `/planning-archive` with archive README (Measurement + Evidence) +- [x] `/gh-pr-push` — `Fixes #100`, SRED line in body. NEWS + version bump left to `/gh-pr-merge` + +## Validation + +- [x] Tests pass +- [x] `/code-check` clean on each commit +- [x] PWF checkboxes match landed work +- [x] `/planning-archive` on completion