Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,14 @@
^pkgdown$
^\.github$
^\.git$
^CLAUDE\.md$
^planning$
^scripts$
^data-raw$
^dev$
^logs$
^\.claude$
^\.lintr$
^CITATION\.cff$
^data$
^research$
25 changes: 25 additions & 0 deletions planning/archive/2026-10-issue-100-rbuildignore/README.md
Original file line number Diff line number Diff line change
@@ -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
78 changes: 78 additions & 0 deletions planning/archive/2026-10-issue-100-rbuildignore/findings.md
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 12 additions & 0 deletions planning/archive/2026-10-issue-100-rbuildignore/progress.md
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions planning/archive/2026-10-issue-100-rbuildignore/review-round1.md
Original file line number Diff line number Diff line change
@@ -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.
13 changes: 13 additions & 0 deletions planning/archive/2026-10-issue-100-rbuildignore/review-round2.md
Original file line number Diff line number Diff line change
@@ -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`.
51 changes: 51 additions & 0 deletions planning/archive/2026-10-issue-100-rbuildignore/review-round3.md
Original file line number Diff line number Diff line change
@@ -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.
37 changes: 37 additions & 0 deletions planning/archive/2026-10-issue-100-rbuildignore/task_plan.md
Original file line number Diff line number Diff line change
@@ -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
Loading