[fix] widgets ProgressiveFutureConnector progress for pending futures by tepals · Pull Request #3526 · delmic/odemis

tepals · 2026-07-22T15:26:36Z

Commit efde81a refactored future start/end -> elapsed_time and remaining_time. The old ProgressiveFuture used absolute start / end timestamps, so even for PENDING futures the connector computed past = time.time() - start which grew naturally. The refactored API returns (0.0, remaining) for PENDING futures elapsed is always 0, so the progress bar never advanced.

Fix for failing test case util_widgets_test.py::ConnectorTestCase::test_pf_connector

This commit adds

Added _progress_update_time (wall-clock anchor) initialized in init and updated in _on_progress whenever new progress data arrives.
In _update_progress , when the future is pending (elapsed=0, not running/done), substitute the future's progress with wall-clock–based elapsed/remaining.

Copilot

Pull request overview

Fixes ProgressiveFutureConnector progress behavior for PENDING ProgressiveFuture instances after the progress API refactor (elapsed/remaining), so the gauge continues to advance while the future is still pending and the existing GUI test passes.

Changes:

Add a wall-clock “progress anchor” timestamp (_progress_update_time) tracked on init and on each progress callback.
When the future is pending, synthesize elapsed/remaining from wall time since the anchor to keep the progress bar moving.

coderabbitai · 2026-07-22T15:34:18Z

Warning

Review limit reached

@tepals, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 35 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a4fa4fde-f8a2-40d4-80a7-eee2d63b3bcf

📥 Commits

Reviewing files that changed from the base of the PR and between 723084e and d2a66b7.

📒 Files selected for processing (1)

src/odemis/gui/util/widgets.py

📝 Walkthrough

Walkthrough

ProgressiveFutureConnector now stores a wall-clock timestamp when initialized and whenever the future reports progress. During pending updates with zero reported elapsed time, it calculates elapsed time from that timestamp and decreases the previously stored remaining time, clamping it to zero. Existing throttling, estimation, and completion handling remain unchanged.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main fix: pending-future progress handling in ProgressiveFutureConnector.
Description check	✅ Passed	The description accurately explains the API change, the pending-future workaround, and the failing test.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/odemis/gui/util/widgets.py`:
- Line 285: Update the progress anchoring and elapsed-time calculations around
_progress_update_time to use time.monotonic() consistently instead of
time.time(). Align the implementation with the monotonic timestamp pattern in
_futures.py, including both timestamp initialization and subsequent elapsed
math.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9766c6a6-863d-4379-b0eb-eaf1d1fe5942

📥 Commits

Reviewing files that changed from the base of the PR and between d98094f and 723084e.

📒 Files selected for processing (1)

src/odemis/gui/util/widgets.py

Commit efde81a refactored future start/end -> elapsed_time and remaining_time. The old ProgressiveFuture used absolute start / end timestamps, so even for PENDING futures the connector computed past = time.time() - start which grew naturally. The refactored API returns (0.0, remaining) for PENDING futures elapsed is always 0, so the progress bar never advanced. Fix for failing test case util_widgets_test.py::ConnectorTestCase::test_pf_connector This commit adds 1. Added _progress_update_time (wall-clock anchor) initialized in __init__ and updated in _on_progress whenever new progress data arrives. 2. In _update_progress , when the future is pending (elapsed=0, not running/done), substitute the future's progress with wall-clock–based elapsed/remaining.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/odemis/gui/util/widgets.py:341

In the pending-future fallback, wall_elapsed can become negative if the system clock moves backwards, which can make elapsed/ratio negative and drive the gauge to an invalid value. Clamp wall_elapsed to >= 0 and subtract from the locally fetched remaining (from get_progress()) to avoid using a potentially stale cross-thread cached value.

        if elapsed == 0 and not self._future.running() and not self._future.done():
            wall_elapsed = now - self._progress_update_time
            elapsed = wall_elapsed
            remaining = max(0.0, self._remaining - wall_elapsed)

    def _update_progress(self):
        """ Update the progression controls """
        now = time.time()
        elapsed, remaining = self._future.get_progress()



pieleric · 2026-07-23T09:24:40Z

+        now = time.monotonic()
        elapsed, remaining = self._future.get_progress()

+        # For pending futures, get_progress() always returns elapsed=0. Use


Should we do this? It doesn't seem obvious to me why we should show the progress bar > 0 %, while the task hasn't started. Do you have an example use-case where it's useful?
The alternative would be to adjust the test case, and not fail if the progress bar == 0 when the Future hasn't started yet.

I was debating just adjusting the test case. My reasoning was that, say the task is pending for 10 seconds, then after the user has clicked the button the progress bar will be stuck for the first 10 seconds and then start running. Wouldn't that also be weird?
I am fine with adjusting the test case, let me know what you prefer

Yes, I think that the right behaviour should be to should the progress go > 0% only when there is progress. If the task takes 10s before starting, then for 10s the progress should be 0%. If we try to pretend the task is started but it's not, it'll certainly come back bite us at some point! It's probably a very edge case anyway, as there is very little reason a task scheduled takes 10s before being scheduled...

tepals requested review from Copilot, nandishjpatel and pieleric July 22, 2026 15:26

github-actions Bot added the size/S label Jul 22, 2026

Copilot started reviewing on behalf of tepals July 22, 2026 15:27 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread src/odemis/gui/util/widgets.py Outdated

coderabbitai Bot reviewed Jul 22, 2026

View reviewed changes

Comment thread src/odemis/gui/util/widgets.py Outdated

Copilot AI review requested due to automatic review settings July 23, 2026 08:07

Copilot started reviewing on behalf of tepals July 23, 2026 08:08 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Comment thread src/odemis/gui/util/widgets.py

Comment on lines 331 to 335

def _update_progress(self):

""" Update the progression controls """

now = time.time()

elapsed, remaining = self._future.get_progress()

tepals force-pushed the fix-progressive-future-connector branch from 723084e to d2a66b7 Compare July 23, 2026 08:39

pieleric reviewed Jul 23, 2026

View reviewed changes

tepals requested a review from pieleric July 23, 2026 13:09

pieleric requested changes Aug 3, 2026

View reviewed changes

Navigation Menu

Uh oh!

[fix] widgets ProgressiveFutureConnector progress for pending futures - #3526

[fix] widgets ProgressiveFutureConnector progress for pending futures#3526
tepals wants to merge 1 commit into
delmic:masterfrom
tepals:fix-progressive-future-connector

tepals commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading

Review limit reached

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

pieleric Jul 23, 2026

Uh oh!

tepals Jul 23, 2026

Uh oh!

pieleric Aug 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

tepals commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

pieleric Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

tepals Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

pieleric Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading