PTAL @richardliaw @alexeykudinkin @owenowenisme !

Im seeing the failures in #3f5f2ce in other microcheck runs as well, so might need to merge master when that is fixed (might be due to flaky CI, as I've seen some cache misses and weird builds on buildkite, so I reran with empty commit). Preliminary manual inspection, at least to me, seems unrelated (reason being 1, failure logs are unrelated, 2, the tests passed for multiple prior commits, while almost nothing changed except for logging order and some bug fixes)

btw, core test failure also fails on current master: https://buildkite.com/ray-project/microcheck/builds/30043

Hey @kyuds, do u think u can rebase from master?
cc: @bveeramani

@iamjustinhsu next action is on me to provide guidance on how to do this refactor

@kyuds I finally dug into this.

The spaghetti has gotten pretty deep over the years for the progress bar code, and I think we should clean it up in steps.

Here are two immediate issues I think we should clean up:

Fix 1: Two Subprogress Bar Interfaces

Current state:

• Interface 1 (AllToAllOperator, HashShuffleOperator)

  • initialize_sub_progress_bars(), close_sub_progress_bars(), set_sub_progress_bars()
  • Operators manage ProgressBar instances directly
  • Used with old TQDM display

• Interface 2 (HashShuffleOperator)

  • SubProgressBarMixin with get_sub_progress_bar_names() / set_sub_progress_bar()
  • Operators declare needs; RichExecutionProgressManager creates the bars
  • Used with new Rich display

Problem: Confusing and increases maintenance — which interface should new subprogress bars use?

Proposed solution:
Unify on SubProgressBarMixin:

• Refactor it to work for both TQDM and Rich, and for both operators
• Remove initialize_sub_progress_bars(), close_sub_progress_bars(), set_sub_progress_bars()

Fix 2: Operators Managing Progress Bar Lifecycle

Problem:

• Tightly couples operators to progress bar rendering, making it harder to change display/output (e.g., logs vs terminal).
• Complicates operator interface: they handle execution and progress visualization.

Proposed solution:

• Operators should only describe progress, e.g., via a simple dict[str, int] like {"shuffle": 4}.
• Progress rendering/management should be handled elsewhere.

will be rebasing to master soon. Im a bit clogged this week, but should be able to get to this soon

to respond to comments from @bveeramani , this specific PR actually is the proposed fix for problem 1. Problem 2 is also sort of addressed here. Because BaseProgressManager is an abstract class and all progress managers implement that class, we are able to just switch out the implementation for BaseProgressManager and progress can be changed easily.

Discussed with @kyuds over Slack -- we're going to park this PR for now, and come back to it later, maybe after we've de-spaghettified some of our code

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

superceded by follow up prs