Fix: Flush remaining accumulated steps in ProgressBar.finish() so show_pos displays full completion

The Bug

Repo: pallets/click Issue: #3571 Status: submission-failed

Description: Flush remaining accumulated steps in ProgressBar.finish() so show_pos displays full completion

Fix scope: 1 line changed in src/click/_termui_impl.py

Root Cause

The bug lives in click’s ProgressBar class, specifically in the interaction between update() and finish() in _termui_impl.py.

How click tracks progress internally. The ProgressBar maintains two key state variables: self.pos (the number of items officially “completed”) and self._completed_intervals (an accumulator for steps that haven’t been committed to pos yet). The update_min_steps parameter controls the rendering cadence — when update(1) is called for each iteration, the step is added to _completed_intervals. Only when _completed_intervals reaches the update_min_steps threshold does the code call make_step(), which adds the accumulated count to self.pos, re-renders the bar, and resets the accumulator to zero. This batching mechanism was introduced to avoid expensive terminal redraws on every single iteration.

The edge case. When update_min_steps does not evenly divide the total iteration length, a remainder accumulates in _completed_intervals after the last render threshold is hit. In the reproduction case — length=20, update_min_steps=7 — the sequence is:

• Items 1–7: _completed_intervals reaches 7, make_step(7) fires, pos → 7, render shows 7/20
• Items 8–14: accumulator reaches 7 again, make_step(7) fires, pos → 14, render shows 14/20
• Items 15–20: only 6 steps accumulate in _completed_intervals, never reaching the threshold of 7

When the generator loop exits, finish() is called. Here’s the problem: finish() sets self.finished = True and clears ETA tracking, but it never flushes the remaining accumulated steps stored in _completed_intervals. The render_progress() call that immediately follows in the generator loop uses self.pos (still 14) to build the show_pos display. The bar itself renders at 100% because the pct property returns 1.0 when self.finished is True, but format_pos() uses self.pos directly, producing the misleading output 14/20.

The percentage-based display (show_percent=True, the default when show_pos is not set) masks this bug because pct short-circuits to 1.0 on finished. But show_pos=True reveals the discrepancy: the bar looks complete while the position counter shows a stale value. This is a classic state synchronization bug — self.finished is set to True before self.pos is updated to reflect the true total of items iterated.

Impact. This affects any user of click.progressbar who enables show_pos=True with an update_min_steps value that doesn’t evenly divide their iteration length. The visual output shows an incomplete position counter despite the bar being full, which is confusing and undermines user trust in the progress indicator. The fix is a single line added to finish() that flushes the remaining _completed_intervals into self.pos before marking the progress bar as finished.

The Fix

The fix is a surgical change — every line is deliberate and scoped to exactly the problem. In ProgressBar.finish(), the method currently sets self.finished = True without first committing the pending accumulated steps. The corrected version calls self.make_step(self._completed_intervals) before setting the finished flag, ensuring that self.pos reflects the true total items processed. The accumulator is then reset to zero, maintaining the invariant that _completed_intervals is clean after a make_step call.

This is a one-line insertion that neither refactors the rendering pipeline nor alters the public API. It simply closes the gap between the accumulated state and the committed state at the one point in the lifecycle where no more update() calls will arrive.

Pattern & Takeaways

Pattern. State accumulation with deferred commit — a common performance pattern in UI frameworks — is vulnerable to flush-at-teardown bugs. The accumulator acts as a write-back cache for self.pos, and finish() is the last-chance flush point. Forgetting to drain the buffer leaves the primary state variable stale. This is structurally identical to a file write buffer that isn’t flushed before the file handle is closed, or a metrics counter that isn’t published before the process exits.

Key lessons.

1. Accumulator semantics must be bounded by lifecycle events. Any batching mechanism that defers state updates must have a corresponding flush at every exit point (normal completion, error, early termination). In click’s case, both finish() and any exception handler in the generator loop are the potential exit points.

2. Property short-circuits can mask state bugs. The fact that pct returns 1.0 unconditionally when self.finished is True hid this bug for users of the default percentage display. Different outputs that derive from different state variables can diverge under the same condition — this is a signal that the state machine has a hole. When reviewing code, check that all output paths derive from the same ground truth state.

3. Parameter boundary testing. update_min_steps defines a rendering quantum. Any test suite for a progress bar should include length % update_min_steps != 0 as a mandatory test case — i.e., a total length that leaves a remainder. This is a classic off-by-one-in-time bug: the last batch of iterations is smaller than the threshold and never triggers the flush logic.

4. Surgical fix principle confirmed. A single line that flushes state at the right lifecycle point is more reliable than refactoring the accumulation logic, which would touch more code paths and risk introducing new bugs. The fix is narrowly scoped to the finish() method because that is the unique point where the invariant “all steps are committed” must hold.

Transfer Potential

Varies — edge case fixes are repo-specific in detail but universal in pattern. The minimal-change principle and boundary-condition thinking transfer to any codebase. Reading this post helps recognize similar patterns in your own projects.

---

Auto-generated from PR #3571. View all patches on GitHub.

References

[1] pallets/click [2] #3571 [3] View all patches