Skip to content

Add safe recovery choices after resolution cancellation - #1237

Draft
AdvancedImagingUTSW wants to merge 4 commits into
kdean/issue-486-stop-resolution-changefrom
kdean/issue-486-resolution-recovery
Draft

Add safe recovery choices after resolution cancellation#1237
AdvancedImagingUTSW wants to merge 4 commits into
kdean/issue-486-stop-resolution-changefrom
kdean/issue-486-resolution-recovery

Conversation

@AdvancedImagingUTSW

Copy link
Copy Markdown
Collaborator

Summary

This stacked PR adds the explicit recovery decision requested for a cancelled resolution change:

  • show a modal Resolution Change Cancelled dialog only after the safety stop has quiesced the resolution worker;
  • make Close and Escape select Keep Current Position without further stage motion;
  • enable Return to Previous Position only when the literal pre-movement coordinates are complete, the stopped position was read successfully, all stop attempts succeeded, and every saved axis passes the active stage driver's strict limit validation;
  • run an approved return as a separate model-owned task that remains cancellable through Stop Stage;
  • disable ordinary stage movement controls during the return while leaving the separate Stop Stage control available;
  • always restore stage controls on successful, cancelled, failed, or stale return attempts.

The core collision-prevention fix is intentionally isolated in draft PR #1236. This PR is based on that branch so reviewers can test and merge the safety stop independently from the optional recovery UX.

Related to #486.

Safety behavior

The saved position is copied into a private recovery snapshot tied to the cancelled task and selected microscope. A new resolution task invalidates any older snapshot. Return uses the existing cancellation-aware Model.move_stage(..., wait_until_done=True) path and the existing Stop Stage lifecycle; it does not bypass device limit checks or create a second stage executor.

The review pass also caught and fixed a stale-return edge case: if the saved position becomes unavailable between opening the dialog and selecting Return, the model refuses motion, emits a terminal failure event, and the controller re-enables ordinary stage controls.

Reuse analysis

Validation

  • black --check passed on all recovery-modified Python files.
  • ruff check passed on all recovery-modified Python files.
  • 34 focused resolution, cancellation, return, controller, popup, concurrency-guard, and existing-resolution tests passed.
  • 20 broader test/model/test_model.py and test/model/test_microscope.py tests passed.
  • git diff kdean/issue-486-stop-resolution-change --check passed.
  • One existing skimage/NumPy deprecation warning was emitted.

Popup behavior is tested without constructing Tk locally. Although Homebrew Xvfb is installed, Navigate's current conda _tkinter links to macOS AppKit rather than X11, so Xvfb cannot isolate a real window in this environment. Native Windows CI or Linux X11 Tk under xvfb-run -a should perform the final constructed-window smoke check.

Stack

Reuse analysis:
- Return movement uses Model.move_stage and the existing Stop Stage lifecycle.
- Existing stage verification enforces the saved coordinates and configured limits.
- No recovery-specific stage executor, limit validator, or stop API was added.
Reuse analysis: the dialog uses Navigate's existing PopUp, event queue, model thread pool, stage-control disablement, and Stop Stage lifecycle. It introduces no parallel movement or stop executor.
Reuse analysis: rejected and completed returns publish the existing resolution_return_complete event. Validation continues through each stage's existing strict verify_abs_position contract; no alternate limit or UI recovery path was added.
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.20863% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.24%. Comparing base (2573bfa) to head (64ae85d).

Files with missing lines Patch % Lines
src/navigate/model/model.py 88.40% 8 Missing ⚠️
src/navigate/controller/controller.py 80.55% 7 Missing ⚠️
Additional details and impacted files
@@                            Coverage Diff                             @@
##           kdean/issue-486-stop-resolution-change    #1237      +/-   ##
==========================================================================
+ Coverage                                   65.06%   65.24%   +0.17%     
==========================================================================
  Files                                         191      192       +1     
  Lines                                       26829    26967     +138     
==========================================================================
+ Hits                                        17456    17594     +138     
  Misses                                       9373     9373              
Flag Coverage Δ
unittests 65.24% <89.20%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant