fix(r-pipeline): drop broken renv.lock; clean up python_version in artifact YAML#6950
Merged
Merged
Conversation
python_version was always intended as pipeline-audit metadata (the Python interpreter that ran the workflow), but on an R artifact it reads like a claim that the artifact runs on Python 3.13. With the language_version column from migration 3a7e1b5c0c4f now carrying the artifact's actual runtime, the python_version field has no place in artifact YAML — workflow_run is sufficient for pipeline audit. Existing rows keep their python_version in the DB (the migration backfilled language_version from python_version, and frontend already prefers language_version with python_version as fallback). sync_to_postgres.py handles missing python_version in YAML via the existing fallback chain. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The hand-written renv.lock listed only 15 top-level packages (ggplot2, scales, tibble, ...). renv::restore() does not auto-resolve transitive dependencies that aren't pinned in the lockfile, so munsell, gtable, colorspace, R6, RColorBrewer, isoband, farver, labeling, ... were never installed in the renv-managed library. Earlier runs only worked because the runner image's system R library also lived on .libPaths(); when the cache restoration scoped libpaths to the renv library, library(ggplot2) failed with "no package called munsell" during impl-repair (run 25973551110). Replace setup-renv@v2 with a direct install.packages step. Base R's installer resolves transitive deps automatically; pinning to a dated Posit Package Manager snapshot (noble/2025-01-15) keeps versions reproducible without needing a fully-populated lockfile. renv.lock is deleted — it was misleading rather than helpful, and no tooling outside the workflows referenced it. The renv-stdout-quieting in impl-generate.yml from PR #6948 stays as defensive insurance in case someone reintroduces a .Rprofile later; without renv it is a no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR cleans up the R/ggplot2 generation pipeline by replacing the incomplete renv.lock restore path with direct R package installation from a pinned Posit Package Manager snapshot, and stops writing pipeline python_version metadata into newly generated artifact YAML.
Changes:
- Removes the repository-level
renv.lock. - Updates the shared
setup-rcomposite action to install ggplot2-related packages directly and smoke-test rendering. - Updates
impl-generate.ymlmetadata generation to emitlanguage_versionwithoutpython_version.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
renv.lock |
Removes the incomplete R lockfile that was used by setup-renv. |
.github/workflows/impl-generate.yml |
Drops python_version from newly generated implementation metadata YAML. |
.github/actions/setup-r/action.yml |
Replaces setup-renv with direct install.packages() installation from a pinned RSPM snapshot. |
| c("ggplot2", "ragg", "tidyr", "dplyr", "viridis", | ||
| "palmerpenguins", "gapminder", "tibble", "scales", | ||
| "systemfonts", "textshaping"), | ||
| dependencies = TRUE |
Apply Copilot review feedback on #6950. `dependencies = TRUE` also pulls Suggests, which for ggplot2/dplyr/tidyr includes sf (needs libgeos/proj/gdal that we don't apt-install), Hmisc, mapproj, maps, quantreg, and other optional packages unused by the implementations. Restrict to c("Depends", "Imports", "LinkingTo") so transitive runtime deps (munsell, gtable, colorspace, etc.) are still resolved while skipping the optional stack. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related cleanups to the R/ggplot2 pipeline so it actually completes end-to-end.
1.
setup-r: install viainstall.packages(..., dependencies = TRUE)instead ofrenv::restore()Symptom
impl-repairforscatter-basic/ggplot2(run 25973551110) failed at the smoke-test step:Root cause
The hand-written
renv.locklisted only 15 top-level packages (ggplot2, scales, tibble, …).renv::restore()does not auto-resolve transitive dependencies that aren't pinned in the lockfile, somunsell(required byscales), plusgtable,colorspace,R6,RColorBrewer,isoband,farver,labeling,MASS,mgcv, … were never installed into the renv-managed library.Earlier runs only succeeded because the runner image's pre-baked R library was also on
.libPaths(). When the cache-restoration path scoped.libPaths()to the renv-managed library (/home/runner/work/_temp/Library),library(ggplot2)could no longer findmunselland the action failed.Adding individual missing entries to
renv.lockis a treadmill — every new R lib means more transitive deps to hunt down by hand. The setup-r action's existing comment already hinted at this being a transitional approach ("Once a successful CI run populates hashes viarenv::snapshot(), this flag can be dropped").Fix
Replace
r-lib/actions/setup-renv@v2with a directinstall.packages(c(...), dependencies = TRUE)call. Base R's installer resolves the transitive dep tree automatically; reproducibility comes from pinning to a dated Posit Package Manager snapshot (noble/2025-01-15) instead of a per-package hash table.setup-renv@v2step +RENV_CONFIG_INSTALL_HASHES=FALSEworkaroundrenv.lock(no tooling outside the workflows referenced it)working-directoryinput (no longer needed)install.packagesstep withdependencies = TRUE+ RSPM snapshot URLggsaveofgeom_point()to a PNG) for fast-fail detectionRENV_CONFIG_STARTUP_QUIET=TRUEinimpl-generate.yml's version probes stays as defensive insurance in case anyone reintroduces a.Rprofilelater — without renv it's a no-op.2.
impl-generate: stop writingpython_versionto new artifact YAMLspython_version: 3.13.13on an R YAML reads like a claim the artifact runs on Python — it's actually pipeline-audit metadata (the workflow runner's Python). The DB has hadlanguage_versionsince migration3a7e1b5c0c4f(backfilled frompython_versionfor legacy rows); the frontend already prefers it (PlotOfTheDay.tsx:267). New YAMLs droppython_versionentirely;workflow_runis sufficient pipeline audit.sync_to_postgres.pyhandles the missing field via its existing fallback chain.Test plan
bulk-generate.yml -f specification_id=scatter-basic -f library=ggplot2impl-generatesucceeds, smoke-test confirmsok: ... byteslanguage_version: 4.4.1,library_version: 3.5.1, nopython_versionimpl-reviewrequests a repair,impl-repairalso passeslibrary(ggplot2)cleanly🤖 Generated with Claude Code