Repository navigation
Preserve hook config deserialization error locations - #2573
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2573 +/- ##
==========================================
+ Coverage 93.71% 93.77% +0.06%
==========================================
Files 135 135
Lines 28939 29048 +109
==========================================
+ Hits 27121 27241 +120
+ Misses 1818 1807 -11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adjusts hook deserialization to preserve precise source locations for nested/flattened hook option errors, so YAML/TOML diagnostics point at the invalid value rather than a parent object.
Changes:
- Introduces a flat
HookWireserde representation and converts it intoRemoteHook/LocalHook/ManifestHookviaFrom/TryFromto avoid span loss from nestedflatten. - Moves option-field deserialization/validation (e.g.,
minimum_prek_version,groups) onto the wire representation and reconstructsHookOptionsexplicitly. - Updates and adds snapshot tests to assert improved error locations for invalid/missing hook fields and invalid option values.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb4dd728a2
ℹ️ 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".
📦 Cargo Bloat ComparisonBinary size change: -0.66% (30.5 MiB → 30.3 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.6 ± 0.1 | 2.4 | 2.7 | 1.07 ± 0.05 |
prek-head --version |
2.4 ± 0.1 | 2.2 | 2.9 | 1.00 |
prek list
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base list |
10.3 ± 0.2 | 10.0 | 10.8 | 1.03 ± 0.04 |
prek-head list |
10.0 ± 0.3 | 9.4 | 10.9 | 1.00 |
prek validate-config .pre-commit-config.yaml
⏭️ Skipped: .pre-commit-config.yaml not found
prek sample-config
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base sample-config |
3.0 ± 0.1 | 2.9 | 3.1 | 1.08 ± 0.03 |
prek-head sample-config |
2.8 ± 0.1 | 2.6 | 2.9 | 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 |
39.2 ± 1.3 | 37.7 | 42.1 | 1.00 |
prek-head run --all-files |
39.8 ± 0.7 | 39.0 | 41.1 | 1.02 ± 0.04 |
prek run --all-files (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
40.2 ± 0.8 | 38.9 | 41.3 | 1.00 |
prek-head run --all-files |
40.3 ± 1.0 | 38.4 | 42.0 | 1.00 ± 0.03 |
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 |
39.5 ± 1.0 | 36.9 | 41.5 | 1.01 ± 0.04 |
prek-head run --all-files |
39.1 ± 1.2 | 36.6 | 41.5 | 1.00 |
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 |
13.7 ± 0.6 | 12.8 | 15.8 | 1.01 ± 0.06 |
prek-head run trailing-whitespace --all-files |
13.5 ± 0.4 | 12.5 | 14.3 | 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 |
12.3 ± 0.5 | 11.5 | 13.4 | 1.05 ± 0.05 |
prek-head run end-of-file-fixer --all-files |
11.7 ± 0.3 | 11.2 | 12.4 | 1.00 |
prek run check-json --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-json --all-files |
8.8 ± 0.3 | 8.3 | 9.3 | 1.04 ± 0.04 |
prek-head run check-json --all-files |
8.5 ± 0.2 | 8.1 | 9.1 | 1.00 |
prek run check-yaml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-yaml --all-files |
8.4 ± 0.2 | 8.2 | 9.3 | 1.03 ± 0.04 |
prek-head run check-yaml --all-files |
8.2 ± 0.2 | 7.8 | 8.7 | 1.00 |
prek run check-toml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-toml --all-files |
8.9 ± 0.2 | 8.5 | 9.5 | 1.05 ± 0.04 |
prek-head run check-toml --all-files |
8.5 ± 0.3 | 7.9 | 9.1 | 1.00 |
prek run check-xml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-xml --all-files |
8.6 ± 0.3 | 8.2 | 9.3 | 1.04 ± 0.05 |
prek-head run check-xml --all-files |
8.3 ± 0.3 | 7.9 | 9.0 | 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 |
12.1 ± 0.3 | 11.5 | 13.1 | 1.01 ± 0.05 |
prek-head run detect-private-key --all-files |
12.0 ± 0.5 | 11.0 | 13.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 |
14.0 ± 0.9 | 12.9 | 15.9 | 1.02 ± 0.10 |
prek-head run fix-byte-order-marker --all-files |
13.8 ± 0.9 | 12.5 | 16.2 | 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 |
5.2 ± 0.1 | 5.1 | 5.3 | 1.06 ± 0.04 |
prek-head install-hooks |
4.9 ± 0.2 | 4.7 | 5.1 | 1.00 |
prek install-hooks (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base install-hooks |
5.2 ± 0.1 | 5.1 | 5.3 | 1.04 ± 0.02 |
prek-head install-hooks |
5.0 ± 0.1 | 4.9 | 5.1 | 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 |
26.7 ± 0.4 | 26.1 | 27.5 | 1.00 |
prek-head run |
26.8 ± 2.0 | 25.7 | 33.4 | 1.01 ± 0.08 |
prek run --files '*.json' (specific file type)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --files '*.json' |
5.9 ± 0.1 | 5.8 | 6.1 | 1.06 ± 0.02 |
prek-head run --files '*.json' |
5.6 ± 0.1 | 5.5 | 5.7 | 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 |
7.8 ± 0.1 | 7.6 | 8.0 | 1.03 ± 0.02 |
prek-head run --dry-run --all-files |
7.5 ± 0.1 | 7.4 | 7.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 |
9.1 ± 0.1 | 8.9 | 9.4 | 1.04 ± 0.02 |
prek-head run check-hooks-apply --all-files |
8.7 ± 0.1 | 8.6 | 8.9 | 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 |
9.1 ± 0.1 | 8.9 | 9.3 | 1.03 ± 0.02 |
prek-head run check-useless-excludes --all-files |
8.8 ± 0.1 | 8.7 | 9.0 | 1.00 |
prek run identity --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run identity --all-files |
8.2 ± 0.1 | 8.1 | 8.4 | 1.03 ± 0.02 |
prek-head run identity --all-files |
8.0 ± 0.1 | 7.9 | 8.2 | 1.00 |
Nested flattened hook options lost source spans during deserialization, causing errors to point at a parent hook or repository. Deserialize hook fields through a flat wire representation so YAML and TOML errors identify the invalid value.