feat: Workflow Runner - Edit button and multiple Acquisition sequences with config paths#493
Conversation
Previously, changing a workflow sequence required removing it and adding a new one. This adds an Edit button that opens the AddSequenceDialog pre-populated with existing sequence data, allowing in-place edits. Changes: - AddSequenceDialog now accepts optional edit_data parameter - Added Edit button between Insert Below and Remove buttons - Edit button is disabled during workflow execution - Acquisition sequences cannot be edited (shows warning) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Acquisition sequences can now: - Be added multiple times in a workflow (e.g., Pre-scan, Main scan) - Be edited via the Edit button (name and config file path) - Be removed like any other sequence - Optionally load settings from an acquisition.yaml file Changes: - SequenceItem: Added config_path field for acquisition YAML file - WorkflowRunner: signal_request_acquisition now passes config_path - gui_hcs: _run_acquisition_for_workflow loads YAML settings if provided - widgets_workflow: Added AddAcquisitionDialog for acquisition sequences - widgets_workflow: _insert_sequence now prompts for Script vs Acquisition - widgets_workflow: _edit_sequence handles both Script and Acquisition - widgets_workflow: _remove_sequence allows removing any sequence - Removed ensure_acquisition_exists() call on workflow load - Updated tests for new behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move test workflow files to tests/scripts/ for tracking: - test_workflow_script.py: Mock script that sleeps and prints progress - test_workflow.yaml: Basic workflow with pre/post scripts + acquisition - test_workflow_cycles.yaml: Workflow demonstrating per-cycle arguments Updated script paths and added config_path field to YAML files. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR enhances the Workflow Runner feature set by enabling in-place editing of sequences, supporting workflows with zero/one/many acquisition steps, and allowing acquisition steps to load settings from an optional acquisition.yaml config path.
Changes:
- Added UI support to insert/edit/remove both Script and Acquisition sequences, including an “Edit” action and a new acquisition configuration dialog.
- Extended the workflow data model + YAML serialization to include
config_pathper sequence and removed the “auto-add Acquisition” behavior. - Updated acquisition execution flow so the runner can request acquisition with an optional
config_pathfor the GUI to load before starting.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| software/control/workflow_runner.py | Adds config_path to SequenceItem, serializes it, and passes it through the acquisition request signal. |
| software/control/widgets_workflow.py | Adds “Edit” button and dialogs for inserting/editing acquisition sequences (name + optional config file). |
| software/control/gui_hcs.py | Accepts config_path from the workflow runner and attempts to load acquisition YAML before starting acquisition. |
| software/tests/control/test_workflow_runner.py | Updates/extends unit tests for serialization roundtrip and “no auto-add acquisition” behavior. |
| software/tests/scripts/test_workflow_script.py | Adds a manual test script used by workflow YAML fixtures. |
| software/tests/scripts/test_workflow.yaml | Adds a sample workflow YAML fixture demonstrating script + acquisition sequencing. |
| software/tests/scripts/test_workflow_cycles.yaml | Adds a sample workflow YAML fixture demonstrating per-cycle script arguments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.log.error(error_msg) | ||
| if self.workflowRunner: | ||
| self.workflowRunner.signal_error.emit(error_msg) | ||
| self.workflowRunner.on_acquisition_finished() |
There was a problem hiding this comment.
_load_acquisition_yaml() handles many failure cases by showing a QMessageBox and returning (it typically doesn’t raise). In those cases this code will still proceed to start acquisition, meaning a provided config_path can be silently ignored and the workflow continues with whatever settings were already in the GUI. Consider changing the YAML load path to return a success/failure signal (e.g., have _load_acquisition_yaml return bool or raise), and abort/fail the workflow step when loading fails (also avoid calling on_acquisition_finished() in a way that makes the runner treat the step as successful).
| self.workflowRunner.on_acquisition_finished() |
| self.log.info(f"Workflow requesting acquisition start (config_path={config_path or 'None'})") | ||
| widget = self.recordTabWidget.currentWidget() | ||
|
|
There was a problem hiding this comment.
With multiple acquisition sequences and editable acquisition names, any post-acquisition UI handling should not rely on the sequence name being exactly "Acquisition". Currently the main window disables the acquisition widget after completion only when name == "Acquisition", so acquisitions named "Pre-scan"/"Post-scan" may leave the acquisition tab enabled while subsequent script sequences run. Prefer checking the sequence type (via the index to look up workflow.sequences[index]) rather than the name string.
| print(f"[{args.name}] Starting...") | ||
| if args.port is not None: | ||
| print(f"[{args.name}] Port argument received: {args.port}") | ||
|
|
||
| print(f"[{args.name}] Sleeping for {args.duration} seconds...") | ||
|
|
||
| # Sleep in small increments to show progress | ||
| elapsed = 0.0 | ||
| interval = 0.5 | ||
| while elapsed < args.duration: | ||
| time.sleep(min(interval, args.duration - elapsed)) | ||
| elapsed += interval | ||
| if elapsed < args.duration: | ||
| print(f"[{args.name}] Progress: {elapsed:.1f}/{args.duration:.1f}s") | ||
|
|
There was a problem hiding this comment.
When this script is run under WorkflowRunner, stdout is piped; Python will typically buffer prints, so progress lines may not appear in the UI until the script exits. Consider using print(..., flush=True) (or otherwise forcing unbuffered output) so the workflow log reflects real-time progress during manual testing.
| time.sleep(min(interval, args.duration - elapsed)) | ||
| elapsed += interval |
There was a problem hiding this comment.
elapsed is always incremented by interval even when the final sleep uses a smaller remainder (min(interval, args.duration - elapsed)), which can overshoot args.duration and produce slightly incorrect progress reporting. Consider incrementing elapsed by the actual sleep duration (the min(...) value).
| time.sleep(min(interval, args.duration - elapsed)) | |
| elapsed += interval | |
| sleep_duration = min(interval, args.duration - elapsed) | |
| time.sleep(sleep_duration) | |
| elapsed += sleep_duration |
Add pre-flight check when user clicks Run: if any acquisition sequence has a config_path, verify the current tab supports _load_acquisition_yaml. Shows a clear error dialog with remediation steps if incompatible. Also updated the runtime check as a fallback (e.g., if user switches tabs during workflow) to fail with error instead of silent warning. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1. _load_acquisition_yaml returns bool for success/failure checking 2. _on_sequence_finished checks sequence type instead of name 3. Test script: flush=True for real-time output, fix progress calc 4. Extract _fail_workflow_acquisition helper to reduce duplication Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Extract _confirm_missing_file() helper to reduce file existence validation duplication in AddSequenceDialog and AddAcquisitionDialog - Extract _prompt_sequence_type() for cleaner sequence type selection - Extract _create_sequence_from_dialog() to simplify _insert_sequence - Simplify _edit_sequence by combining shared code paths - Use early returns to reduce nesting - Reduce line count while maintaining same functionality Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
UI fixes: - Fix button order in sequence type prompt: Script, Acquisition, Cancel - Rename column header from "Command" to "Command/Path" - Simplify edit confirmation message to "Changes saved" Documentation updates: - Document Edit button functionality - Document multiple acquisition sequences with config files - Update table column description - Add Acquisition Sequences section - Update YAML example with config_path field - Update tips section Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Extract _confirm_missing_file to module-level function (removes duplication) - Add _set_status helper for label text + color updates - Simplify _populate_from_data with field mappings - Use early returns in save/load methods - Clean up to_dict to omit None values (cleaner YAML output) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
acquisition.yamlfileChanges
UI Improvements:
AddAcquisitionDialogfor configuring acquisition sequences (name + optional config file)Data Model:
config_pathfield toSequenceItemfor acquisition YAML file pathensure_acquisition_exists()auto-add behavior - workflows can have 0, 1, or many acquisitionsconfig_pathExecution:
signal_request_acquisitionnow passesconfig_pathto the GUI_run_acquisition_for_workflowloads YAML settings before starting acquisition if config providedTest plan
pytest tests/control/test_workflow_runner.py)🤖 Generated with Claude Code