Skip to content

Quality audit PR 2/6: cooperative super() initialization and dispatch#320

Merged
wolph merged 4 commits into
developfrom
quality-audit-2-super
Jul 6, 2026
Merged

Quality audit PR 2/6: cooperative super() initialization and dispatch#320
wolph merged 4 commits into
developfrom
quality-audit-2-super

Conversation

@wolph

@wolph wolph commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Second of six audit PRs (PR 1: #319). Migrates the widget hierarchy and the ProgressBar mixin tower from hand-simulated MRO (explicit Parent.__init__(self, ...) / Parent.update(self, ...) calls) to cooperative super() — the root cause of a class of silent kwarg-swallowing bugs and hidden double-initialization.

What changed

  • widgets.py (619d289): every chain-breaking mixin (FormatWidgetMixin, WidthWidgetMixin, VariableMixin, SamplesMixin) is now cooperative; ~20 widget constructors collapse their multiple explicit parent calls into one super().__init__(...). A private _WidgetKwargsSink terminates the chains, silently absorbing stray kwargs for backwards compatibility (third-party widgets have leaked kwargs to parents for years). abc.ABC dropped from the two mixins that declared no abstract methods.
  • bar.py __init__ (d27d5c5): ResizableMixin/StdRedirectMixin chain to super(); ProgressBar.__init__'s three explicit parent calls become one. ResizableMixin.__init__ used to run twice per bar via the dynamic-super overlap — now exactly once (WeakSet install idempotent, identical end state). ProgressBarBase assigns its index guarded so old-style subclasses still consume exactly one index.
  • bar.py dispatch (d3793ea): _update_parents' triple no-op dispatch and the start/finish chains collapse to single super() calls (value always passed by keyword so (*args, **kwargs) and (value=None) signatures interoperate). Only semantic shift: SIGWINCH uninstall now runs before stream unwrap on finish — independent subsystems.

Compatibility guarantees (test-enforced, from PR 1's characterization suite)

Both historical subclassing styles keep working and render byte-identically:

  • Old style (explicit unbound parent __init__ calls, copying the library's former pattern) — including the historic format=-leak variant.
  • Super style (single cooperative call) — now fully works: kwargs finally reach every base. The strict xfail documenting the old chain break flipped to a required pass.

New regression guards: no-double-init probes (widgets + bar tower), the uses_colors stale-cache hazard for two-phase old-style construction, exactly-once update dispatch, finish(end='') kwarg safety. Public __init__ signatures byte-identical (API-surface snapshot untouched); render goldens byte-identical.

Verification

567 passed / 100.00% branch coverage on 3.14; CI-parity runs (TERM=dumb) green on 3.10 and 3.12; ruff clean; pyright 0 errors; perf budget passes.

wolph added 3 commits July 6, 2026 03:02
Convert widgets.py from explicit unbound-parent __init__ calls to
cooperative super() initialization. Add a private _WidgetKwargsSink base
that terminates the cooperative chains (absorbing stray kwargs third-party
widgets have passed for years), and make FormatWidgetMixin,
WidthWidgetMixin, VariableMixin and SamplesMixin cooperative. Both
WidthWidgetMixin and VariableMixin inherit the sink so it lands after both
in every MRO, letting `name` reach VariableMixin in the MultiRangeBar /
JobStatusBar diamonds.

Every widget constructor that hand-called multiple parents now issues a
single super().__init__(...), forwarding its named params as keywords.
Drop the abc.ABC base from FormatWidgetMixin / WidthWidgetMixin (neither
declares an abstractmethod); WidgetBase keeps ABCMeta.

WidgetBase.__init__ now drops any cached `uses_colors` before re-checking,
so old-style code that calls parents with different kwargs per pass cannot
keep a stale uses_colors=False and silently lose color rendering.

Public constructor signatures and rendering output are unchanged
(render goldens byte-identical). Flip the previously-strict-xfail
test_super_style_widget_constructs_and_renders to passing and add
no-double-init and two-phase-color-kwargs regression tests.
Replace explicit class-qualified parent calls in the update/start/finish
chains with cooperative super() dispatch, mirroring the __init__ migration.

- ProgressBar._update_parents: three explicit parent update() calls (two of
  which resolved to the no-op ProgressBarMixinBase.update) collapse to a
  single super().update(value=value).
- ProgressBar.start / .finish: three explicit parent calls each collapse to
  one super() call.
- StdRedirectMixin/DefaultFdMixin/ResizableMixin internal explicit parent
  calls converted to super().

value is passed by keyword through the chain so the intermediate
`*args, **kwargs` and `value=None` signatures interoperate.

Ordering note: the SIGWINCH uninstall in ResizableMixin.finish now runs
before the stream unwrap in StdRedirectMixin.finish (previously after).
The two subsystems are independent, so behavior is unchanged.

Add two characterization tests: a super()-style update override is entered
exactly once per update() call (guards the collapsed chain against
re-dispatch), and finish(end='') threads end through the collapsed chain.
Copilot AI review requested due to automatic review settings July 6, 2026 01:27
Comment thread tests/test_subclass_compat.py Dismissed
Comment thread tests/test_subclass_compat.py Dismissed
Comment thread tests/test_subclass_compat.py Dismissed

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the codebase to use cooperative super() calls instead of explicit unbound parent class calls across progressbar/bar.py and progressbar/widgets.py. It also introduces a _WidgetKwargsSink to safely terminate cooperative chains and adds comprehensive tests to ensure backwards compatibility. The feedback suggests two improvements: first, checking for the presence of __dict__ before calling vars(self) in widgets.py to prevent crashes on subclasses using __slots__; second, checking if 'index' is in self.__dict__ rather than comparing against -1 in bar.py to make the initialization guard more robust.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread progressbar/widgets.py
Comment thread progressbar/bar.py

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR continues the audit series by converting the widget hierarchy and progress bar mixin tower from explicit parent calls to cooperative super() initialization and dispatch, aiming to eliminate MRO chain breaks, kwarg swallowing, and double-initialization hazards while preserving backwards compatibility for third-party subclasses.

Changes:

  • Removed the xfail that documented the former super()-style widget chain break and added new regression tests to enforce post-migration invariants (no double-init, correct kwarg threading, and exactly-once dispatch).
  • Made key widget mixins and many widget constructors cooperative via super().__init__(...), introducing a private kwargs sink to terminate init chains safely.
  • Made ProgressBar’s init and update/start/finish dispatch cooperative, including a guard to ensure ProgressBarBase consumes exactly one index even under old-style multiple explicit parent __init__ calls.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/test_subclass_compat.py Updates characterization/regression tests to enforce cooperative-super guarantees and remove the prior strict xfail.
progressbar/widgets.py Migrates widget mixins/constructors to cooperative super() and adds a kwargs sink to preserve backwards compatibility.
progressbar/bar.py Migrates bar init and lifecycle/update dispatch to cooperative super() and guards index assignment against multiple init entrypoints.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread progressbar/bar.py Outdated
DefaultFdMixin.start accepted **kwargs but called super().start()
without them, silently dropping start-time kwargs from downstream
mixins in the now-cooperative chain (flagged by review on #320).
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.

3 participants