Fix progress collapse ordering#2291
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2291 +/- ##
=======================================
Coverage 93.11% 93.12%
=======================================
Files 127 127
Lines 28051 28094 +43
=======================================
+ Hits 26121 26163 +42
- Misses 1930 1931 +1 ☔ 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 fixes a progress-renderer edge case where completed hook rows could be collapsed in completion order rather than visual insertion order, which can leave the underlying indicatif::MultiProgress in an invalid state and trigger a panic after hooks finish (Issue #2289).
Changes:
- Track a monotonic “visual insertion order” (
line_order) for each hook’s main progress line and store completed bars keyed by that order. - Refactor “hide one line” into
collapse_one_line()returning an explicit result{ anchor, bars }so the caller can reliably insert/update the summary line before removing bars. - Add a regression test covering out-of-order completion while ensuring collapsing removes a true visual prefix.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📦 Cargo Bloat ComparisonBinary size change: +0.75% (26.7 MiB → 26.9 MiB) Expand for cargo-bloat outputHead Branch ResultsBase Branch Results |
⚡️ Hyperfine BenchmarksSummary: 1 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.6 | 1.07 ± 0.06 |
prek-head --version |
2.2 ± 0.1 | 2.1 | 2.4 | 1.00 |
prek list
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base list |
9.9 ± 0.1 | 9.4 | 10.2 | 1.01 ± 0.03 |
prek-head list |
9.7 ± 0.2 | 9.4 | 11.2 | 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.4 ± 0.1 | 3.2 | 3.7 | 1.07 ± 0.04 |
prek-head validate-config .pre-commit-config.yaml |
3.2 ± 0.1 | 3.0 | 3.4 | 1.00 |
prek sample-config
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base sample-config |
2.6 ± 0.0 | 2.5 | 2.7 | 1.06 ± 0.03 |
prek-head sample-config |
2.5 ± 0.1 | 2.4 | 2.7 | 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 |
78.1 ± 2.5 | 73.5 | 81.7 | 1.02 ± 0.05 |
prek-head run --all-files |
76.7 ± 3.0 | 73.0 | 82.5 | 1.00 |
prek run --all-files (warm - with cache)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --all-files |
78.6 ± 2.3 | 73.6 | 82.2 | 1.02 ± 0.04 |
prek-head run --all-files |
77.1 ± 2.5 | 71.5 | 81.2 | 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 |
77.5 ± 2.7 | 72.3 | 82.6 | 1.00 |
prek-head run --all-files |
78.0 ± 2.6 | 72.8 | 85.6 | 1.01 ± 0.05 |
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.9 ± 0.6 | 20.0 | 23.3 | 1.01 ± 0.03 |
prek-head run trailing-whitespace --all-files |
20.6 ± 0.4 | 19.9 | 21.6 | 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 |
27.3 ± 1.8 | 25.2 | 30.5 | 1.02 ± 0.10 |
prek-head run end-of-file-fixer --all-files |
26.8 ± 2.0 | 24.3 | 30.2 | 1.00 |
prek run check-json --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-json --all-files |
8.1 ± 0.3 | 7.6 | 9.1 | 1.02 ± 0.05 |
prek-head run check-json --all-files |
7.9 ± 0.2 | 7.4 | 8.4 | 1.00 |
prek run check-yaml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-yaml --all-files |
7.9 ± 0.2 | 7.5 | 8.3 | 1.03 ± 0.03 |
prek-head run check-yaml --all-files |
7.6 ± 0.1 | 7.3 | 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 |
8.2 ± 0.3 | 7.6 | 8.8 | 1.00 |
prek-head run check-toml --all-files |
8.4 ± 1.3 | 7.3 | 13.4 | 1.03 ± 0.17 |
prek run check-xml --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-xml --all-files |
8.2 ± 0.3 | 7.7 | 9.0 | 1.05 ± 0.06 |
prek-head run check-xml --all-files |
7.9 ± 0.3 | 7.2 | 8.3 | 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.1 | 12.6 | 16.2 | 1.01 ± 0.11 |
prek-head run detect-private-key --all-files |
13.9 ± 1.0 | 12.4 | 16.5 | 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.5 ± 0.9 | 17.4 | 20.8 | 1.00 |
prek-head run fix-byte-order-marker --all-files |
19.6 ± 1.2 | 17.7 | 23.6 | 1.06 ± 0.08 |
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.1 ± 0.2 | 4.9 | 5.3 | 1.07 ± 0.04 |
prek-head install-hooks |
4.8 ± 0.1 | 4.7 | 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.1 | 4.7 | 4.9 | 1.06 ± 0.02 |
prek-head install-hooks |
4.6 ± 0.1 | 4.5 | 4.7 | 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 |
39.1 ± 1.2 | 37.4 | 41.8 | 1.03 ± 0.04 |
prek-head run |
37.8 ± 1.1 | 36.6 | 41.1 | 1.00 |
prek run --files '*.json' (specific file type)
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run --files '*.json' |
8.4 ± 0.1 | 8.2 | 8.7 | 1.00 |
prek-head run --files '*.json' |
8.4 ± 0.3 | 8.1 | 8.9 | 1.00 ± 0.04 |
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 |
8.7 ± 1.2 | 7.3 | 10.6 | 1.22 ± 0.17 |
prek-head run --dry-run --all-files |
7.1 ± 0.1 | 6.9 | 7.4 | 1.00 |
✅ Performance improvement for prek run --dry-run --all-files (measures init overhead): 18.1500% faster
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.4 ± 0.1 | 12.2 | 12.5 | 1.00 |
prek-head run check-hooks-apply --all-files |
21.3 ± 16.3 | 10.8 | 57.1 | 1.72 ± 1.32 |
prek run check-hooks-apply --all-files: 72.2400% slower
prek run check-useless-excludes --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run check-useless-excludes --all-files |
10.9 ± 0.2 | 10.7 | 11.2 | 1.00 ± 0.02 |
prek-head run check-useless-excludes --all-files |
10.9 ± 0.2 | 10.7 | 11.3 | 1.00 |
prek run identity --all-files
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
prek-base run identity --all-files |
10.2 ± 0.1 | 10.1 | 10.4 | 1.00 |
prek-head run identity --all-files |
10.3 ± 0.2 | 10.0 | 10.7 | 1.01 ± 0.03 |
9c8b750 to
ebdaf29
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebdaf299e2
ℹ️ 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".
| group.active_tail = Some(id); | ||
|
|
||
| running.insert(id, HookBar::new(hook, progress)); | ||
| running.insert(id, HookBar::new(hook, id, progress)); |
There was a problem hiding this comment.
Track actual insertion order after summary reanchoring
When the terminal is small enough to collapse completed rows while an older hook in the same project is still running, update_group_summary can set group.last_line to the summary because no completed bars remain visible. This start path then inserts the next hook after that summary, visually above the still-running older hook, but stores the larger id as its line_order; if the older hook completes later, the BTreeMap collapses by id and removes the lower visual row first, reintroducing the ordering corruption this change is meant to prevent.
Useful? React with 👍 / 👎.
## Summary - Keep project insertion anchors based on live running tails and visible completed rows. - Place hidden summaries at the collapsed row anchor. - Update output and completion layout state before removing progress bars. ## Context Follow-up to #2291. A collapsed summary could become the next insertion anchor while an older hook in the same project was still running, which allowed later hooks to render above older running rows and corrupt later collapse order.
Summary
Keep completed hook progress rows ordered by visual insertion order before collapsing.
Fixes #2289.