ui: prevent progress bar capacity overflow

[OllieinCanada]

Summary

Prevent the CLI progress renderer from panicking with capacity overflow when
its completed-stage count exceeds the planned-stage total.

This is independent of #339; that review command exposed an existing progress
display bug, but the V4L2 prompt change does not modify src/main.rs.

Root cause

render_progress() derived a filled bar width directly from
completed_stages / total_stages and then allocated the remainder with:

"░".repeat(width - filled)

If an inconsistent or duplicated progress event makes completed_stages
larger than total_stages, filled exceeds width. The subtraction
underflows in release builds, and repeat() attempts an impossible allocation.

The same input could also display a percentage above 100% and a completed
count larger than the total.

Fix

• Clamp the display-only completed count to the planned total.
• Use saturating multiplication for percentage and bar calculations.
• Bound the filled width and use saturating subtraction for the empty width.
• Leave review execution and progress events unchanged.
• Add a regression test for five completed stages against one planned stage,
  along with normal and zero-total cases.

Testing

• cargo fmt --all -- --check
• cargo test --all-features --bin sashiko (14 passed)
• cargo clippy --all-targets --all-features -- -D warnings
• git diff --check

[derekbarbosa]

Hi Ollie,

Thanks for the PR!

at a glance, this looks good. thank you for subbing in the "safer" saturating_* math functions and abstracting it away in a neat helper.

just for fun, I also ran it through the /review-pr skill:

  Analysis

   - 🟢 Bugs / Regressions: None found. saturating_mul prevents overflow on large stage counts. min(total_stages) and min(width) ensure visual boundaries are respected.
   - 🟢 Safety: Logic handles total_stages == 0 gracefully. usize operations are safe from underflow via saturating_sub.
   - 🟢 Tests: New unit test test_progress_metrics_clamp_completed_stages_to_total covers edge cases (zero total, completed > total). All 334 workspace tests passed.
   - 🟡 Complexity: Logic is simple and well-isolated.

  Findings

  🔵 Style / Nits / DRY: Logic extracted correctly.
  L1164-1180: Logic for metric calculation is now isolated and robust.

LGTM!

[mchehab]

Hi Ollie,

Thanks for the PR!

at a glance, this looks good. thank you for subbing in the "safer" saturating_* math functions and abstracting it away in a neat helper.
LGTM!

If you feel this approach is better, feel free to close #342 . I opened it just because I can't test Ollama without having a fix for this issue ;-)

[derekbarbosa]

Hi Ollie,
Thanks for the PR!
at a glance, this looks good. thank you for subbing in the "safer" saturating_* math functions and abstracting it away in a neat helper.
LGTM!

If you feel this approach is better, feel free to close #342 . I opened it just because I can't test Ollama without having a fix for this issue ;-)

I had planned to post your PR review and this comment at the same time, but my finger slipped ;)

thanks!

[mchehab]

IMO, a separate PR should try to solve the root cause of the issue, e.g. to ensure that total_stages is incremented when there is the need to run additional stages (for instance, on retries - not sure if are there other cases as well).

[rgushchin]

Is this ready to be merged? If not, what needs to be done?

[mchehab]

Is this ready to be merged? If not, what needs to be done?

LGTM

[derekbarbosa]

Hi @OllieinCanada - please remove this PR from "draft" once it is ready so that it may be merged.

[rgushchin]

Merged, thanks!