Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9055186240
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR improves the user-facing error message produced when parsing invalid files/exclude file-pattern values in .pre-commit-config.yaml, making the expected formats clearer within prek’s config parsing layer.
Changes:
- Add a custom
serdedeserializer forFilePatternWireto provide clearer “expected …” parse errors for file patterns. - Extend the
invalid_configintegration test snapshot to assert the new, more specific parse error text. - Update this repo’s
.pre-commit-config.yamltop-levelexcludeto use prek’s glob-mapping form.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
crates/prek/src/config.rs |
Implements a custom Deserialize for file-pattern wire types to improve parse error messaging. |
crates/prek/tests/run.rs |
Adds a snapshot assertion covering invalid files pattern type errors. |
.pre-commit-config.yaml |
Switches snapshot exclusion from a regex string to a glob: mapping. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1829 +/- ##
==========================================
- Coverage 91.81% 91.78% -0.04%
==========================================
Files 98 98
Lines 20059 20119 +60
==========================================
+ Hits 18418 18467 +49
- Misses 1641 1652 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📦 Cargo Bloat ComparisonBinary size change: +0.00% (24.8 MiB → 24.8 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
⚡️ Hyperfine BenchmarksSummary: 0 regressions, 0 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.5 ± 0.2 | 2.2 | 3.2 | 1.04 ± 0.08 |
prek-head --version |
2.4 ± 0.1 | 2.2 | 2.6 | 1.00 |
prek list
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base list |
9.1 ± 0.2 | 8.8 | 9.8 | 1.01 ± 0.03 |
prek-head list |
9.0 ± 0.2 | 8.8 | 11.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.4 | 1.02 ± 0.02 |
prek-head validate-config .pre-commit-config.yaml |
3.2 ± 0.0 | 3.0 | 3.3 | 1.00 |
prek sample-config
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base sample-config |
2.6 ± 0.1 | 2.6 | 3.2 | 1.00 ± 0.04 |
prek-head sample-config |
2.6 ± 0.0 | 2.5 | 2.8 | 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 |
156.8 ± 2.1 | 153.9 | 160.2 | 1.00 |
prek-head run --all-files |
157.9 ± 2.5 | 154.0 | 161.2 | 1.01 ± 0.02 |
prek run --all-files (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
158.6 ± 2.7 | 153.4 | 164.8 | 1.01 ± 0.02 |
prek-head run --all-files |
156.2 ± 2.4 | 151.9 | 162.4 | 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 |
157.5 ± 3.7 | 152.6 | 170.5 | 1.00 |
prek-head run --all-files |
161.4 ± 23.3 | 153.5 | 321.1 | 1.02 ± 0.15 |
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 |
22.2 ± 0.4 | 21.3 | 23.0 | 1.01 ± 0.03 |
prek-head run trailing-whitespace --all-files |
21.9 ± 0.4 | 21.1 | 22.8 | 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 |
28.9 ± 1.9 | 26.1 | 32.8 | 1.00 ± 0.10 |
prek-head run end-of-file-fixer --all-files |
28.8 ± 2.0 | 25.9 | 35.5 | 1.00 |
prek run check-json --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-json --all-files |
12.3 ± 0.4 | 11.8 | 13.4 | 1.00 ± 0.04 |
prek-head run check-json --all-files |
12.2 ± 0.3 | 11.7 | 12.7 | 1.00 |
prek run check-yaml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-yaml --all-files |
11.9 ± 0.2 | 11.7 | 12.8 | 1.00 ± 0.03 |
prek-head run check-yaml --all-files |
11.9 ± 0.3 | 11.5 | 12.8 | 1.00 |
prek run check-toml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-toml --all-files |
12.2 ± 0.3 | 11.7 | 13.0 | 1.01 ± 0.03 |
prek-head run check-toml --all-files |
12.1 ± 0.2 | 11.7 | 12.5 | 1.00 |
prek run check-xml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-xml --all-files |
12.3 ± 0.3 | 11.9 | 13.1 | 1.01 ± 0.03 |
prek-head run check-xml --all-files |
12.2 ± 0.2 | 11.7 | 12.6 | 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 |
18.9 ± 1.1 | 16.7 | 21.6 | 1.01 ± 0.08 |
prek-head run detect-private-key --all-files |
18.8 ± 1.2 | 16.6 | 21.4 | 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 |
24.2 ± 1.4 | 21.4 | 27.2 | 1.01 ± 0.08 |
prek-head run fix-byte-order-marker --all-files |
24.0 ± 1.4 | 21.1 | 26.8 | 1.00 |
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.9 ± 0.1 | 4.8 | 5.0 | 1.00 ± 0.02 |
prek-head install-hooks |
4.9 ± 0.1 | 4.8 | 4.9 | 1.00 |
prek install-hooks (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
4.8 ± 0.0 | 4.8 | 4.9 | 1.00 ± 0.01 |
prek-head install-hooks |
4.8 ± 0.0 | 4.7 | 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 |
53.2 ± 0.8 | 52.0 | 54.9 | 1.00 |
prek-head run |
53.5 ± 1.1 | 52.0 | 55.6 | 1.01 ± 0.02 |
prek run --files '*.json' (specific file type)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --files '*.json' |
9.1 ± 0.1 | 8.9 | 9.3 | 1.01 ± 0.02 |
prek-head run --files '*.json' |
9.0 ± 0.1 | 8.8 | 9.3 | 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 |
15.3 ± 1.8 | 14.0 | 19.9 | 1.07 ± 0.12 |
prek-head run --dry-run --all-files |
14.3 ± 0.2 | 14.0 | 14.6 | 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 |
14.1 ± 0.1 | 13.8 | 14.2 | 1.00 |
prek-head run check-hooks-apply --all-files |
14.2 ± 0.1 | 14.0 | 14.5 | 1.01 ± 0.01 |
prek run check-useless-excludes --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-useless-excludes --all-files |
13.7 ± 0.6 | 12.5 | 14.2 | 1.10 ± 0.05 |
prek-head run check-useless-excludes --all-files |
12.5 ± 0.2 | 12.3 | 13.0 | 1.00 |
prek run identity --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run identity --all-files |
11.0 ± 0.1 | 10.9 | 11.2 | 1.00 ± 0.01 |
prek-head run identity --all-files |
11.0 ± 0.1 | 10.9 | 11.2 | 1.00 |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves how prek reports configuration parse errors for file pattern fields (e.g., files / exclude) by customizing deserialization and adding coverage to lock in the new error message behavior.
Changes:
- Implement custom
serdedeserialization forFilePatternWireto produce clearer “expected …” errors for invalid types. - Add an integration test snapshot asserting the improved parse error when
fileshas an invalid type. - Update this repo’s
.pre-commit-config.yamlto use theglobmapping form forexclude.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
crates/prek/src/config.rs |
Adds a custom Deserialize impl for FilePatternWire to improve parse error messages for file patterns. |
crates/prek/tests/run.rs |
Adds a snapshot test covering invalid files type error messaging. |
.pre-commit-config.yaml |
Switches exclude to the glob mapping form (prek-only glob support). |
No description provided.