Skip to content

fix(cli): sort jobs by stage completion ratio#1947

Open
QuakeWang wants to merge 2 commits into
apache:mainfrom
QuakeWang:fix-jobs-stage-sort
Open

fix(cli): sort jobs by stage completion ratio#1947
QuakeWang wants to merge 2 commits into
apache:mainfrom
QuakeWang:fix-jobs-stage-sort

Conversation

@QuakeWang

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #.

Rationale for this change

The TUI jobs table sorted the stage completion column with integer division:

completed_stages / num_stages

This could panic when num_stages was zero, and it also collapsed partial ratios such as 1/4, 2/4, and 3/4 to the same value.

What changes are included in this PR?

This PR updates the jobs table stage-completion sorting to compare the actual completion ratio safely, treating zero-stage jobs as 0% complete. It also adds regression tests for ratio-based sorting and zero-stage jobs.

Are there any user-facing changes?

Yes. Sorting the TUI jobs table by stage completion now reflects the real completion ratio and no longer panics for jobs with zero stages.

Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
Comment thread ballista-cli/src/tui/domain/jobs.rs Outdated
if job.num_stages == 0 {
(0_u128, 1_u128)
} else {
(job.completed_stages as u128, job.num_stages as u128)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit : Perhaps 128bit int is an overkill here ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@coderfender Good point. I switched this to compare f64 completion ratios with total_cmp while keeping the zero-stage guard. The exact u128 fraction comparison was more defensive than this TUI sorting path needs.

@coderfender coderfender left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

left comments

Signed-off-by: QuakeWang <wangfuzheng0814@foxmail.com>
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.

2 participants