Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2206 +/- ##
==========================================
+ Coverage 92.59% 92.62% +0.03%
==========================================
Files 125 126 +1
Lines 26587 26784 +197
==========================================
+ Hits 24617 24808 +191
- Misses 1970 1976 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8f313707a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class support for running R hooks in prek, integrating R into the existing language framework (install/health-check/run), CI matrix, and documentation so R hooks can be installed and executed similarly to other supported languages.
Changes:
- Implement R language backend (
Rscriptexecution, env activation, remoterenvrestore, local additional deps install). - Add R integration tests and update CI/nextest configuration to provision R and schedule R tests.
- Update docs to reflect R support and adjust language support messaging.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/generate-ci-matrix.py | Adds r to the CI language test matrix. |
| README.md | Removes outdated note about some languages being unsupported. |
| docs/workspace.md | Updates example output to show Markdownlint passing. |
| docs/reference/configuration.md | Updates r language entry semantics to reflect the new backend. |
| docs/proposals/hook-shell.md | Updates proposal docs for r entry semantics. |
| docs/languages.md | Documents R support (Rscript + renv) and removes repetitive “Status” lines across languages. |
| docs/index.md | Removes outdated note about unsupported languages. |
| crates/prek/tests/languages/unimplemented.rs | Removes R “unimplemented language” test now that R is implemented. |
| crates/prek/tests/languages/r.rs | Adds R integration tests (local hook, local deps, remote renv repo, language_version rejection). |
| crates/prek/tests/languages/main.rs | Registers the new R test module. |
| crates/prek/src/languages/r.rs | New R language implementation (install/check/run + entry validation). |
| crates/prek/src/languages/mod.rs | Wires R into the language dispatcher and updates shell support messaging. |
| crates/prek/src/cli/run/run.rs | Removes the “unimplemented language” skip path/status now that R is implemented. |
| crates/prek-consts/src/env_vars.rs | Adds R-related env var constants (R_HOME, R_PROFILE_USER, RENV_PROJECT). |
| .github/workflows/ci.yml | Installs R in CI when the r language group is selected. |
| .config/nextest.toml | Adds CI override to serialize R tests due to shared cache/lock state. |
Comment on lines
+282
to
+290
| if entry[1] == "-e" { | ||
| if entry.len() > 3 { | ||
| anyhow::bail!("You can supply at most one expression"); | ||
| } | ||
| } else if entry.len() > 2 { | ||
| anyhow::bail!( | ||
| "The only valid syntax is `Rscript -e {{expr}}` or `Rscript path/to/hook/script`" | ||
| ); | ||
| } |
📦 Cargo Bloat ComparisonBinary size change: +0.00% (26.6 MiB → 26.6 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
⚡️ Hyperfine BenchmarksSummary: 0 regressions, 1 improvements above the 10% threshold. Environment
CLI CommandsBenchmarking basic commands in the main repo:
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base --version |
2.3 ± 0.1 | 2.1 | 2.8 | 1.05 ± 0.07 |
prek-head --version |
2.1 ± 0.1 | 2.0 | 2.5 | 1.00 |
prek list
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base list |
9.6 ± 0.3 | 9.2 | 10.3 | 1.04 ± 0.04 |
prek-head list |
9.2 ± 0.2 | 8.8 | 10.1 | 1.00 |
prek validate-config .pre-commit-config.yaml
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base validate-config .pre-commit-config.yaml |
3.2 ± 0.1 | 3.1 | 3.3 | 1.08 ± 0.12 |
prek-head validate-config .pre-commit-config.yaml |
3.0 ± 0.3 | 2.8 | 5.1 | 1.00 |
prek sample-config
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base sample-config |
2.5 ± 0.0 | 2.4 | 2.7 | 1.05 ± 0.03 |
prek-head sample-config |
2.4 ± 0.0 | 2.3 | 2.5 | 1.00 |
Cold vs Warm Runs
Comparing first run (cold) vs subsequent runs (warm cache):
prek run --all-files (cold - no cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
75.5 ± 2.0 | 73.1 | 78.7 | 1.01 ± 0.05 |
prek-head run --all-files |
74.8 ± 3.3 | 71.7 | 80.6 | 1.00 |
prek run --all-files (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
75.7 ± 3.3 | 72.3 | 84.1 | 1.02 ± 0.06 |
prek-head run --all-files |
74.4 ± 2.9 | 71.3 | 81.5 | 1.00 |
Full Hook Suite
Running the builtin hook suite on the benchmark workspace:
prek run --all-files (full builtin hook suite)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
74.6 ± 3.0 | 70.6 | 81.8 | 1.00 |
prek-head run --all-files |
75.0 ± 3.2 | 70.5 | 81.7 | 1.01 ± 0.06 |
Individual Hook Performance
Benchmarking each hook individually on the test repo:
prek run trailing-whitespace --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run trailing-whitespace --all-files |
20.4 ± 0.4 | 19.6 | 21.1 | 1.02 ± 0.04 |
prek-head run trailing-whitespace --all-files |
20.0 ± 0.6 | 19.2 | 22.0 | 1.00 |
prek run end-of-file-fixer --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run end-of-file-fixer --all-files |
25.3 ± 1.5 | 23.4 | 29.2 | 1.00 |
prek-head run end-of-file-fixer --all-files |
26.0 ± 2.0 | 23.3 | 30.6 | 1.03 ± 0.10 |
prek run check-json --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-json --all-files |
7.7 ± 0.2 | 7.4 | 8.0 | 1.02 ± 0.04 |
prek-head run check-json --all-files |
7.5 ± 0.2 | 7.2 | 7.9 | 1.00 |
prek run check-yaml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-yaml --all-files |
7.6 ± 0.1 | 7.4 | 7.8 | 1.03 ± 0.03 |
prek-head run check-yaml --all-files |
7.4 ± 0.2 | 7.2 | 7.9 | 1.00 |
prek run check-toml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-toml --all-files |
7.8 ± 0.3 | 7.2 | 8.3 | 1.03 ± 0.05 |
prek-head run check-toml --all-files |
7.5 ± 0.3 | 7.2 | 8.3 | 1.00 |
prek run check-xml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-xml --all-files |
7.8 ± 0.2 | 7.3 | 8.1 | 1.01 ± 0.06 |
prek-head run check-xml --all-files |
7.7 ± 0.4 | 7.1 | 9.2 | 1.00 |
prek run detect-private-key --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run detect-private-key --all-files |
14.1 ± 1.3 | 12.4 | 16.9 | 1.03 ± 0.13 |
prek-head run detect-private-key --all-files |
13.7 ± 1.2 | 11.7 | 16.0 | 1.00 |
prek run fix-byte-order-marker --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run fix-byte-order-marker --all-files |
18.9 ± 0.9 | 17.3 | 20.3 | 1.00 |
prek-head run fix-byte-order-marker --all-files |
19.0 ± 0.9 | 17.5 | 20.2 | 1.00 ± 0.07 |
Installation Performance
Benchmarking hook installation (fast path hooks skip Python setup):
prek install-hooks (cold - no cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
4.6 ± 0.0 | 4.5 | 4.6 | 1.03 ± 0.02 |
prek-head install-hooks |
4.4 ± 0.1 | 4.4 | 4.5 | 1.00 |
prek install-hooks (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
4.6 ± 0.1 | 4.5 | 4.7 | 1.01 ± 0.03 |
prek-head install-hooks |
4.6 ± 0.1 | 4.4 | 4.8 | 1.00 |
File Filtering/Scoping Performance
Testing different file selection modes:
prek run (staged files only)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run |
37.2 ± 0.8 | 36.3 | 39.5 | 1.00 |
prek-head run |
37.6 ± 1.4 | 36.0 | 40.1 | 1.01 ± 0.04 |
prek run --files '*.json' (specific file type)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --files '*.json' |
8.0 ± 0.1 | 7.8 | 8.3 | 1.02 ± 0.03 |
prek-head run --files '*.json' |
7.8 ± 0.2 | 7.6 | 8.2 | 1.00 |
Workspace Discovery & Initialization
Benchmarking hook discovery and initialization overhead:
prek run --dry-run --all-files (measures init overhead)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --dry-run --all-files |
6.9 ± 0.1 | 6.8 | 7.1 | 1.04 ± 0.02 |
prek-head run --dry-run --all-files |
6.6 ± 0.1 | 6.5 | 6.7 | 1.00 |
Meta Hooks Performance
Benchmarking meta hooks separately:
prek run check-hooks-apply --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-hooks-apply --all-files |
12.0 ± 0.1 | 11.8 | 12.2 | 1.02 ± 0.03 |
prek-head run check-hooks-apply --all-files |
11.8 ± 0.3 | 11.6 | 12.5 | 1.00 |
prek run check-useless-excludes --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-useless-excludes --all-files |
19.2 ± 22.2 | 10.7 | 93.4 | 1.84 ± 2.13 |
prek-head run check-useless-excludes --all-files |
10.4 ± 0.1 | 10.2 | 10.7 | 1.00 |
✅ Performance improvement for prek run check-useless-excludes --all-files: 45.6300% faster
prek run identity --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run identity --all-files |
9.8 ± 0.1 | 9.7 | 9.9 | 1.00 ± 0.03 |
prek-head run identity --all-files |
9.8 ± 0.2 | 9.5 | 10.2 | 1.00 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a596e4c79f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+108
to
+112
| Cmd::new(rscript_executable(), "check Rscript version") | ||
| .arg("--version") | ||
| .check(true) | ||
| .output() | ||
| .await |
There was a problem hiding this comment.
When an R hook environment has already been installed and the system R selected by PATH/R_HOME is later upgraded, this health check still reports the cache as healthy as long as some Rscript exists. The environment was restored into renv's R-versioned library for the old interpreter, so compiled packages can be reused with an incompatible R instead of triggering a reinstall; install should persist getRversion() and compare it here before reusing the env.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d28ebfb45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+136
to
+139
| let activate = env_path.join("activate.R"); | ||
| if activate.exists() { | ||
| cmd.env(EnvVars::R_PROFILE_USER, activate); | ||
| } |
There was a problem hiding this comment.
For local R hooks that do not declare additional_dependencies, no activate.R is written during install, so this branch leaves R_PROFILE_USER unset while still omitting --no-init-file from the Rscript command. In that case Rscript can source the repository's .Rprofile (or the user's home profile), even though the R backend is meant to emulate a clean startup; a project profile that changes .libPaths(), options, or exits can make a bare language: r hook behave differently from the same hook under pre-commit. Set R_PROFILE_USER to a controlled empty/activation file or pass --no-init-file when there is no activation profile.
Useful? React with 👍 / 👎.
Comment on lines
+126
to
+131
| let entry = r_hook_entry(hook)?; | ||
| let env_path = hook.env_path().expect("R must have env path"); | ||
|
|
||
| let run = async |batch: &[&Path]| { | ||
| let mut cmd = Cmd::new(&entry[0], "run R hook"); | ||
| cmd.current_dir(hook.work_dir()) |
Comment on lines
+289
to
+293
| if entry[1] == "-e" { | ||
| if entry.len() > 3 { | ||
| anyhow::bail!("You can supply at most one expression"); | ||
| } | ||
| } else if entry.len() > 2 { |
Comment on lines
+350
to
+354
| #[test] | ||
| fn validate_r_entry_allows_supported_forms() { | ||
| validate_r_entry(&["Rscript".into(), "-e".into(), "1+1".into()]).unwrap(); | ||
| validate_r_entry(&["Rscript".into(), "hook.R".into()]).unwrap(); | ||
| } |
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.
Closes #42