7
High
// The Bug
A bug in pallets/click's ProgressBar (#3571, patch-ready) causes incomplete display on finish() as _completed_intervals are discarded; show_pos only…
// Root Cause
The edge case lives in `class ProgressBar(t.Generic[V]):` at `src/click/_termui_impl.py`. When the progress bar finishes with accumulated steps that were never shown, the `finish()` method calls `self.make_step(0)` — or nothing at all if `_completed_intervals` was zero. Partial progress accumulated between the last `update()` call and `finish()` gets silently discarded.
In Python, this pattern slips through because standard test suites rarely cover boundary conditions around partial step accumulation [1]. The `_completed_intervals` counter tracks how many step increments have been made since the last display update. Since `show_pos` only renders when `make_step` receives a non-zero value, the last batch of steps vanishes on finish.
### Reproduction Steps
Create a `ProgressBar` over a known length, call `bar.update(n)` where `n < pbar.length - current_pos`, then call `bar.finish()`. The displayed progress shows the last update position, not 100%, because `_completed_intervals` was never flushed through `make_step`. [1]
### Impact Assessment
The bug affects users who rely on `show_pos` to display full completion after `finish()`. For click, a one-line fix resolves a scenario where progress bars show incomplete progress even after the operation has ended — a cosmetic regression that erodes user confidence in the progress indicator. This matters most in long-running CLI operations where users watch the progress bar to estimate completion time.
In Python, this pattern slips through because standard test suites rarely cover boundary conditions around partial step accumulation [1]. The `_completed_intervals` counter tracks how many step increments have been made since the last display update. Since `show_pos` only renders when `make_step` receives a non-zero value, the last batch of steps vanishes on finish.
### Reproduction Steps
Create a `ProgressBar` over a known length, call `bar.update(n)` where `n < pbar.length - current_pos`, then call `bar.finish()`. The displayed progress shows the last update position, not 100%, because `_completed_intervals` was never flushed through `make_step`. [1]
### Impact Assessment
The bug affects users who rely on `show_pos` to display full completion after `finish()`. For click, a one-line fix resolves a scenario where progress bars show incomplete progress even after the operation has ended — a cosmetic regression that erodes user confidence in the progress indicator. This matters most in long-running CLI operations where users watch the progress bar to estimate completion time.
// The Fix
Diff showing the exact changes made to fix the bug.
@@ -348,6 +348,7 @@ class ProgressBar(t.Generic[V]):
self._completed_intervals = 0
def finish(self) -> None:
+ self.make_step(self._completed_intervals)
self.eta_known = False
self.current_item = None
self.finished = True// Pattern & Takeaways
- **Pattern**: Accumulated state in `_completed_intervals` was never drained at the termination boundary. The fix changes only the minimum necessary code.
- **Key insight**: The most predictable bugs live at boundary conditions — empty/null input, iteration boundaries, unexpected types, and operation transitions [1]. Here the boundary was the switch from "actively updating" to "finished".
- **Practical check**: When reviewing code that maintains internal counters or accumulators, trace what happens at every exit point. If state is only read in one code path but modified in another, you've found a candidate for this pattern.
### Key Takeaways
1. Progress bar implementations with lazy display updates are vulnerable to accumulated-but-unflushed state at termination.
2. The fix pattern — flush internal state before the termination sequence — applies to any UI component that batches updates for performance.
3. When reviewing library code, trace the full lifecycle of internal state counters. If they're only drained in one code path, there's likely a missing drain at another.
### How to Verify the Fix
To confirm the fix works in your own codebase:
- Create a `ProgressBar` with a known length (e.g., `bar = ProgressBar(range(100))`)
- Call `bar.update(50)` to advance partially
- Call `bar.finish()` without a final `update()` call
- Assert that `show_pos` displays 100% completion [2]
- Confirm no regression on normal usage: call `bar.update(100)` then `bar.finish()` still works
- **Key insight**: The most predictable bugs live at boundary conditions — empty/null input, iteration boundaries, unexpected types, and operation transitions [1]. Here the boundary was the switch from "actively updating" to "finished".
- **Practical check**: When reviewing code that maintains internal counters or accumulators, trace what happens at every exit point. If state is only read in one code path but modified in another, you've found a candidate for this pattern.
### Key Takeaways
1. Progress bar implementations with lazy display updates are vulnerable to accumulated-but-unflushed state at termination.
2. The fix pattern — flush internal state before the termination sequence — applies to any UI component that batches updates for performance.
3. When reviewing library code, trace the full lifecycle of internal state counters. If they're only drained in one code path, there's likely a missing drain at another.
### How to Verify the Fix
To confirm the fix works in your own codebase:
- Create a `ProgressBar` with a known length (e.g., `bar = ProgressBar(range(100))`)
- Call `bar.update(50)` to advance partially
- Call `bar.finish()` without a final `update()` call
- Assert that `show_pos` displays 100% completion [2]
- Confirm no regression on normal usage: call `bar.update(100)` then `bar.finish()` still works