Skip to content

Fix relabelling, triple flush and lost text regressions from 4.7.0 - #336

Open
wolph wants to merge 2 commits into
developfrom
fix/stream-multibar-regressions
Open

wolph wants to merge 2 commits into
developfrom
fix/stream-multibar-regressions

Conversation

@wolph

@wolph wolph commented Nov 6, 2026

Copy link
Copy Markdown
Owner

Three behaviours that 4.7.0 changed against 4.6.0 without a changelog line. Each one was found by running the public API of both releases side by side:

behaviour 4.6.0 4.7.0 this PR
MultiBar: pop or clear a bar, then add it again 1 label 2 labels 1 label
target.flush() calls for one start_capturing + stop_capturing 1 3 1
a WrappingIO.flush() override raises once: captured text kept lost kept

What changed

MultiBar labels leave with the bar. _label_bar records the label objects it inserts, and _forget_bar (pop, del, clear, a replacement, remove_finished) removes exactly those objects from bar.widgets, matched by identity, so a label of the bar's own that renders the same text stays. A bar that is removed while its labels are being formatted ends up unlabelled. 4.6.0 only avoided the double label on pop by accident: pop did not forget the bar at all, del did and labelled twice too.

One flush when capturing stops. stop_capturing() no longer drains and delivers itself when update_capturing is the default hook: the hook drains and delivers through flush(), as it did in 4.6.0. A replaced hook, which may not call super(), still gets the stop's own delivery first, so the error chaining and the hook order pinned by the existing tests are unchanged. Flag propagation is split out of _apply_capturing into _propagate_capturing, with the private signatures left as they were.

A failing flush() costs no text. WrappingIO._deliver calls the public flush() after the buffer was drained. When that call raises, the taken text now goes back in front of anything written since, and the next flush delivers it in order. A failed target.write() still drops its text, so a retry cannot write it twice.

Verification

Every number below comes from this tree.

  • pytest with coverage: Python 3.10 and 3.14t, both 100 percent (see the CI runs for the full matrix).
  • ruff check and format, pyright on Linux and Windows, codespell, sphinx-build -W: clean.
  • The 4.6.0 test files against this tree: the same three colour-default failures as 4.7.0, nothing new.
  • README benchmarks interleaved with 4.7.0 (min of two rounds each):
metric 4.7.0 this PR
A iterator overhead, plain (ns/iter) 26.10 27.32
A iterator overhead, fast (ns/iter) 3.51 3.44
B forced redraw, plain (us) 17.98 18.01
B forced redraw, FastProgressBar (us) 4.89 4.89
FormatLabel render (us) 1.23 1.25
Timer widget (us) 0.99 1.01
captured write (us) 0.12 0.13
C import, plain (ms) 3.55 3.54
C import, fast (ms) 3.49 3.48

The iterator paths are untouched by this change and the two rounds disagree on the direction there, so that spread is run-to-run noise.

  • Stream micro benchmarks against 4.7.0: captured write, flush, start plus stop capturing and a MultiBar render of three bars are at parity, a wrap plus capture plus unwrap cycle went from 4.5 to 3.8 us.

The compatibility script of the release skill could not run (its scripts directory is gone from ~/.claude/skills), so the compatibility result above rests on the 4.6.0 test files, 4.7.0's own test suite and the probes.

Where this comes from

A Codex session (29 Oct to 3 Nov) left an 8700-line uncommitted rewrite of the stream wrapper and MultiBar in the main checkout, pinned to these three regressions among 395 failing asserts, most of them bound to private names and hostile subclasses. That rewrite made start_capturing plus stop_capturing 27 times slower and MultiBar.render twice as slow, so it was declined as a whole. The three regressions it was right about are fixed here in 60 lines.

wolph added 2 commits November 6, 2026 11:33
A bar that leaves a MultiBar takes its generated label widgets with it, so adding it again labels it once. The stop_capturing() that brings the count to zero flushes the original stream once, as in 4.6.0. A WrappingIO.flush() replacement that raises no longer costs the captured text.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant