Conversation
…ons and color-coded borders
Implemented comprehensive scope-based visual feedback system that provides immediate visual
indication of configuration changes and hierarchical relationships across the GUI. The system
uses perceptually distinct colors to differentiate orchestrators (plates) and applies layered
borders with tints and patterns to distinguish steps within each orchestrator's pipeline.
Flash animations trigger when resolved configuration values change (not just raw values),
ensuring users see feedback only when actual effective values change after inheritance
resolution. This solves the false-positive flash problem where overridden step configs
would flash even though their resolved values didn't change.
Changes by functional area:
* Scope Visual Infrastructure: Created centralized configuration and color generation system
- scope_visual_config.py: Defines ScopeVisualConfig with tunable HSV parameters for
orchestrator/step colors, flash duration (300ms), and ScopeColorScheme dataclass
containing all derived colors. Implements ListItemType enum with polymorphic dispatch
pattern (following ProcessingContract design) to select correct background colors
without conditionals
- scope_color_utils.py: Generates perceptually distinct colors using distinctipy library
with WCAG AA contrast compliance checking (4.5:1 ratio). Implements deterministic
color assignment via MD5 hashing of scope_id. Supports layered border generation with
cycling tint factors [0.7, 1.0, 1.4] and patterns [solid, dashed, dotted] that cycle
through all 9 combinations before adding layers (step 0-8: 1 border, 9-17: 2 borders,
18-26: 3 borders). Extracts per-orchestrator step indexing from scope_id format
"plate_path::step_token@position" where @position enables independent step numbering
per orchestrator
- list_item_flash_animation.py: Manages flash animation for QListWidgetItem updates.
Stores (list_widget, row, scope_id, item_type) instead of item references to handle
item destruction during flash. Increases opacity to 100% for flash (from 15% for
orchestrators, 5% for steps), then restores correct color by recomputing from scope_id.
Implements global animator registry with cleanup on list rebuild
- widget_flash_animation.py: Manages flash animation for form widgets (QLineEdit,
QComboBox, etc.) using QPalette manipulation. Stores original palette, applies light
green flash (144, 238, 144 RGB at 100 alpha), restores after 300ms. Global registry
keyed by widget id with cleanup support
* Cross-Window Preview System: Enhanced to support flash detection via resolved value comparison
- cross_window_preview_mixin.py: Added _pending_changed_fields tracking (separate from
_pending_label_keys) to track ALL field changes for flash logic while only updating
labels for registered preview fields. Added _last_live_context_snapshot to capture
"before" state for comparison. Implemented _check_resolved_value_changed() that
compares resolved objects (not raw values) to detect actual effective changes.
Added _resolve_flash_field_value() hook for subclass-specific resolution.
Refactored _resolve_scope_targets() to eliminate duplicate scope mapping logic.
Flash and label updates now fully decoupled: flash triggers on any resolved value
change, labels update only for registered preview fields
* Widget Integration - Pipeline Editor: Integrated scope coloring and flash for step list items
- pipeline_editor.py: Modified _build_step_scope_id() to support optional position
parameter - position=None for cross-window updates (matches DualEditorWindow scope_id),
position=idx for visual styling (enables per-orchestrator step indexing). Implemented
_apply_step_item_styling() that builds scope_id with @position suffix and applies
background color + border layers from ScopeColorScheme. Added _flash_step_item()
using ListItemFlashAnimator. Enhanced _refresh_step_items_by_index() to separate
label updates (label_subset) from flash detection (changed_fields + live_context_before).
Implemented _resolve_step_flash_field() that resolves through context stack
(GlobalPipelineConfig → PipelineConfig → Step) using _get_pipeline_config_preview_instance()
and _get_global_config_preview_instance() to merge live values. Added
_path_depends_on_context() to check if step inherits value (returns True if None).
Modified QListWidget stylesheet to use transparent backgrounds (let delegate draw
scope colors) and border-left for selection indicator. Calls clear_all_animators()
before list rebuild to prevent flash timers accessing destroyed items
* Widget Integration - Plate Manager: Integrated scope coloring and flash for orchestrator list items
- plate_manager.py: Implemented _apply_orchestrator_item_styling() that applies
background color (15% opacity) and stores border data [(3, 1, 'solid')] for delegate.
Added _flash_plate_item() using ListItemFlashAnimator with ListItemType.ORCHESTRATOR.
Enhanced _update_single_plate_item() to check resolved value changes via
_check_resolved_value_changed() comparing pipeline config before/after with live
context. Implemented _resolve_pipeline_config_flash_field() that handles global_config
prefix and uses _path_depends_on_context() to check inheritance. Added dataclass
field validation to skip resolver when attribute not in __dataclass_fields__.
Modified QListWidget stylesheet for transparent backgrounds. Calls clear_all_animators()
before list rebuild. Added underline flag (UserRole + 2) for plate names in delegate
* Widget Integration - Step Editor Windows: Added layered border painting for scope-based styling
- dual_editor_window.py: Added step_position parameter to constructor for scope-based
styling. Implemented _apply_step_window_styling() that builds scope_id with @position
suffix (if available) and applies border stylesheet. Overrode paintEvent() to draw
layered borders with tint factors [0.7, 1.0, 1.4] and patterns [solid, dashed, dotted].
Each border layer drawn from outside to inside with proper inset calculation
(border_offset = inset + width/2 for centered pen drawing). Handles both old format
(width, tint_index) and new format (width, tint_index, pattern). Applies .darker(120)
to border colors for better contrast. Sets dash patterns [8, 6] for dashed and [2, 6]
for dotted with MORE OBVIOUS spacing
* Widget Integration - Pipeline Config Windows: Added simple orchestrator border
- config_window.py: Implemented _apply_config_window_styling() that uses orchestrator
border color (not layered step borders) for pipeline config windows. Applies 3px
solid border using scope_id to get color scheme
* Rendering & Styling: Enhanced delegate to draw scope backgrounds and layered borders
- list_item_delegate.py: Modified paint() to draw custom background FIRST (before
style draws selection) using BackgroundRole data, allowing scope colors to show
through. Added layered border rendering that reads border_layers (UserRole + 3)
and base_color_rgb (UserRole + 4) from item data. Draws each border layer from
outside to inside with tint calculation and pattern application (solid/dashed/dotted).
Added underline support for first line (UserRole + 2 flag) that underlines text
after last '▶ ' marker (for plate names). Moved QFont/QPen imports to top level
- widget_strategies.py: Added flash_widget() calls to all placeholder application
functions (_apply_lineedit_placeholder, _apply_spinbox_placeholder,
_apply_checkbox_placeholder, _apply_checkbox_group_placeholder,
_apply_path_widget_placeholder, _apply_combobox_placeholder) to provide immediate
visual feedback when inherited values update
* Dependencies: Added WCAG contrast compliance library
- pyproject.toml: Added wcag-contrast-ratio>=0.9 to gui extras for accessibility
compliance checking in scope_color_utils._ensure_wcag_compliant()
Technical implementation details:
Scope ID format:
- Orchestrator scope: "plate_path" (e.g., "/path/to/plate")
- Step scope (cross-window): "plate_path::step_token" (for matching DualEditorWindow)
- Step scope (visual): "plate_path::step_token@position" (for per-orchestrator indexing)
The @position suffix enables independent step numbering per orchestrator, ensuring
step 0 in orchestrator A gets different styling than step 0 in orchestrator B.
Flash detection logic:
1. Track ALL changed fields in _pending_changed_fields (not filtered)
2. Capture live context snapshot before and after changes
3. Get preview instances with merged live values for both snapshots
4. Compare resolved values (not raw values) via _check_resolved_value_changed()
5. Flash only if resolved value actually changed after inheritance resolution
This eliminates false positives where step overrides would flash even though their
resolved values didn't change (e.g., step.well_filter=3 stays 3 even when
pipeline.well_filter changes from 4 to 5).
Border layering pattern:
- Cycles through 9 tint+pattern combinations per layer (3 tints × 3 patterns)
- Step 0-2: 1 border with solid pattern, tints [dark, neutral, bright]
- Step 3-5: 1 border with dashed pattern, tints [dark, neutral, bright]
- Step 6-8: 1 border with dotted pattern, tints [dark, neutral, bright]
- Step 9-17: 2 borders (all combinations)
- Step 18-26: 3 borders (all combinations)
Tint factors [0.7, 1.0, 1.4] provide MORE DRASTIC visual distinction than previous
[0.85, 1.0, 1.15] values, making borders clearly distinguishable.
This commit fixes multiple critical bugs in the cross-window flash animation system that provides visual feedback when configuration values change. ## Bug Fixes ### 1. Invisible Flash Animation (CRITICAL) **Problem**: Flash animations were being called but not visible to users. **Root Cause**: Scope-based styling methods (_apply_orchestrator_item_styling and _apply_step_item_styling) were called AFTER flash, overwriting the flash background color with normal scope colors. **Solution**: Reordered operations in _refresh_step_items_by_index() and _update_single_plate_item() to apply styling BEFORE flash. **Files Changed**: - openhcs/pyqt_gui/widgets/pipeline_editor.py - openhcs/pyqt_gui/widgets/plate_manager.py ### 2. Wrong Flash Color **Problem**: Flash used hardcoded green color instead of scope colors. **Solution**: Changed flash logic to use same RGB as scope color with alpha=255 (full opacity) instead of normal low opacity (5% for steps, 15% for orchestrators). **Files Changed**: - openhcs/pyqt_gui/widgets/shared/list_item_flash_animation.py ### 3. Multiple Items Flashing When Only One Changed **Problem**: Changing a single step caused all steps to flash. **Root Cause**: Window close triggered both incremental update (via value_changed_handler with __WINDOW_CLOSED__) and full refresh (via trigger_global_cross_window_refresh). Full refresh timer cancelled incremental update timer, then flashed all items. **Solution**: Removed flash calls from _handle_full_preview_refresh() since it's used for window close/reset events (not actual value changes). Flash only happens in incremental updates where we know exactly which items changed. **Files Changed**: - openhcs/pyqt_gui/widgets/pipeline_editor.py - openhcs/pyqt_gui/widgets/plate_manager.py ### 4. Window Close with Unsaved Changes Not Flashing **Problem**: Closing editor windows with unsaved changes didn't flash affected items (even though values reverted to saved state). **Root Cause**: trigger_global_cross_window_refresh() in reject() methods cancelled incremental update timers before they could fire. **Solution**: Removed trigger_global_cross_window_refresh() calls from window reject() methods. Parameter form manager unregister already notifies listeners via value_changed_handler with __WINDOW_CLOSED__ marker, which triggers incremental updates. **Files Changed**: - openhcs/pyqt_gui/windows/dual_editor_window.py - openhcs/pyqt_gui/windows/config_window.py ## Code Cleanup ### Removed Complex Flash Detection Logic Temporarily removed flash detection that compared resolved values before/after changes. Currently flashing on ALL incremental updates to verify connectivity. Proper flash detection (comparing resolved step objects) will be implemented in follow-up commit. **Removed Methods**: - PipelineEditorWidget._resolve_flash_field_value() - PipelineEditorWidget._resolve_step_flash_field() - PlateManagerWidget._resolve_flash_field_value() - PlateManagerWidget._resolve_pipeline_config_flash_field() ### Refactored Display Text Formatting Extracted _format_resolved_step_for_display() from format_item_for_display() to separate resolution logic from formatting logic. This makes the code path clearer and enables reuse. **Files Changed**: - openhcs/pyqt_gui/widgets/pipeline_editor.py ### Improved Documentation - Added detailed docstrings to _check_resolved_value_changed() - Added docstrings to _resolve_flash_field_value() and _walk_object_path() - Clarified that _handle_full_preview_refresh() is for window close/reset - Removed obsolete _path_depends_on_context() method **Files Changed**: - openhcs/pyqt_gui/widgets/mixins/cross_window_preview_mixin.py ## Debug Logging Added extensive debug logging (🔥 emoji) throughout flash system for troubleshooting. This logging should be removed in a future commit once the system is stable. **Files Changed**: - openhcs/pyqt_gui/widgets/shared/list_item_flash_animation.py - openhcs/pyqt_gui/widgets/shared/list_item_delegate.py - openhcs/pyqt_gui/widgets/mixins/cross_window_preview_mixin.py - openhcs/pyqt_gui/widgets/pipeline_editor.py ## Testing Verified that: 1. Changing a single step flashes ONLY that step 2. Closing step editor with unsaved changes flashes ONLY that step 3. Closing step editor with NO unsaved changes does NOT flash 4. Flash color matches scope color (just with full opacity) 5. Flash is visible (not overwritten by styling) ## Follow-up Work - Implement proper flash detection by comparing resolved step objects - Remove debug logging once system is stable - Consider extracting flash logic into reusable mixin
…ntation Updated Sphinx documentation to match the flash animation system changes from the previous commit. ## Changes ### docs/source/architecture/scope_visual_feedback_system.rst **Updated Pipeline Editor Integration Example** (lines 287-332): - Replaced old flash detection logic with current implementation - Added critical ordering requirement: styling BEFORE flash - Showed current signature of _refresh_step_items_by_index() - Demonstrated use of _get_step_preview_instance() and _format_resolved_step_for_display() - Removed obsolete _check_resolved_value_changed() example (flash detection will be re-implemented in future commit) ### docs/source/development/visual_feedback_integration.rst **Rewrote Step 2: Implement Flash Detection** → **Step 2: Implement Incremental Updates with Flash** (lines 74-156): - Updated code example to match current _refresh_step_items_by_index() signature - Added CRITICAL ORDERING REQUIREMENT section explaining why styling must come before flash (prevents invisible flash bug) - Documented that flash detection is temporarily disabled (flashing on ALL incremental updates) - Explained the three-step ordering: styling → flash → label update **Added Step 3: Separate Full Refresh from Incremental Updates** (lines 158-173): - Documented _handle_full_preview_refresh() pattern - Explained why full refresh should NOT flash (window close/reset events) - Clarified that only incremental updates should flash **Removed Step 3: Implement Resolved Value Comparison**: - Removed obsolete _resolve_flash_field_value() documentation - This will be re-added when proper flash detection is implemented ## Rationale These documentation updates ensure developers understand: 1. **Critical ordering requirement**: Styling before flash prevents invisible flash bug 2. **Separation of concerns**: Full refresh vs incremental updates 3. **Current state**: Flash detection temporarily disabled while connectivity is verified 4. **Future work**: Proper flash detection will compare resolved step objects The updated examples now match the actual implementation in the codebase.
…tion - Add user guide for visual feedback system (color-coded borders, flash animations) - Document scope-based coloring, layered borders, and WCAG-compliant color generation - Add cross-references to architecture docs (scope_visual_feedback_system, gui_performance_patterns) - Update architecture index to include new UI development quick start path - Add visual_feedback_integration to development guides index The visual feedback system helps users understand: - Which orchestrator (plate) they're working with via unique colors - Which step belongs to which plate via inherited colors - When configuration values change via flash animations - Hierarchical relationships via layered borders Includes practical examples for editing steps, pipeline configs, and working with multiple plates.
… resolution Fixed severe performance issues where flash detection was creating massive overhead (29MB-31MB logs, thousands of function calls per second) when typing in config fields. Implemented content-based caching, batch resolution, and proper snapshot timing to achieve O(1) context setup for flash detection across all preview widgets. Changes by functional area: * Configuration Framework - Context Management: Add content-based caching for extract_all_configs() to handle frozen dataclasses recreated with dataclasses.replace(). Cache extracted configs in contextvars when setting context to avoid re-extraction on every attribute access. Reduces extract_all_configs calls from thousands per second to once per context setup. * Configuration Framework - Resolution: Fix lazy/non-lazy type matching in MRO inheritance (e.g., LazyStepWellFilterConfig matches StepWellFilterConfig). Add batch resolution method resolve_all_config_attrs() for O(1) context setup when resolving multiple attributes. Cache merged contexts to avoid recreating dataclass instances. Preserve inheritance hierarchy in lazy dataclass factory by making lazy versions inherit from lazy parents. * GUI - Cross-Window Preview Mixin: Implement resolved value comparison for flash detection instead of always flashing. Add identifier expansion for inheritance (e.g., "well_filter_config.well_filter" expands to check "step_well_filter_config.well_filter"). Add context-aware resolution through LiveContextResolver. Collect window close snapshot BEFORE form managers unregister to capture edited values that will be discarded. * GUI - Pipeline Editor: Build context stack (GlobalPipelineConfig → PipelineConfig → Step) for flash resolution. Only flash when resolved values actually changed (compare preview instances before/after). Update _last_live_context_snapshot AFTER flashes shown (not during incremental update) to enable subsequent edits to trigger flashes. Implement _handle_full_preview_refresh with flash detection for window close events. * GUI - Plate Manager: Build context stack (GlobalPipelineConfig → PipelineConfig) for flash resolution. Only flash when resolved values actually changed. Implement _handle_full_preview_refresh with flash detection for window close events. * GUI - Form Management: Move window close notification BEFORE removing from registry so external listeners can collect snapshot with edited values still present. Add object-to-manager mapping for retrieving window_open_snapshot. Set emit_signal=False when refreshing due to another window's value change to prevent cascading full refreshes. Performance improvements: - Typing in config fields: 29MB logs → minimal logging - Flash detection: Always flash → only flash when resolved values change - Context extraction: Thousands of calls/sec → once per context setup - Merged context creation: Every attribute access → cached and reused Architectural invariant enforced: "It should be impossible for a label to change but the item not flash, but the item can flash without a label updating."
…lution optimizations
Document critical implementation details missing from architecture docs:
* Configuration Framework:
- Content-based caching in extract_all_configs() for frozen dataclasses
- Extracted configs caching in contextvars to avoid re-extraction
- Lazy/non-lazy type matching in MRO inheritance resolution
- Inheritance preservation in lazy dataclass factory
- Batch resolution method resolve_all_config_attrs() for O(1) context setup
- Merged context caching to avoid recreating dataclass instances
* Scope Visual Feedback System:
- Identifier expansion for inheritance (e.g., well_filter_config.well_filter
expands to check step_well_filter_config.well_filter)
- Window close snapshot timing (collect BEFORE form managers unregister)
- Context-aware resolution through LiveContextResolver for flash detection
- Context stack building (GlobalPipelineConfig → PipelineConfig → Step)
These details explain the performance improvements and architectural decisions
in the previous commit (b53944e).
The batch flash detection was failing for non-dataclass objects like FunctionStep because: 1. LiveContextResolver.resolve_all_lazy_attrs() returned empty dict for non-dataclass objects 2. CrossWindowPreviewMixin._check_with_batch_resolution() had is_dataclass() checks that prevented batch resolution from running This caused window close flash detection to fail - when closing a step editor or PipelineConfig editor without saving, the steps wouldn't flash even though their inherited values changed. The fix unifies the code path for both dataclass and non-dataclass objects: - resolve_all_lazy_attrs() now introspects non-dataclass objects to find dataclass attributes (e.g., fiji_streaming_config, step_well_filter_config) - Removed is_dataclass() checks in _check_with_batch_resolution() so batch resolution works for all objects This ensures flash detection works correctly for window close events on both PipelineConfig and step editors.
Updated documentation to explain how LiveContextResolver.resolve_all_lazy_attrs() works for both dataclass and non-dataclass objects: - For dataclasses: uses fields() to get all field names - For non-dataclasses: introspects to find dataclass attributes This unified approach ensures flash detection works correctly for window close events on both PipelineConfig editors and step editors. Updated: - docs/source/architecture/configuration_framework.rst: Added example of resolve_all_lazy_attrs() usage - docs/source/architecture/scope_visual_feedback_system.rst: Added section on batch resolution for performance with explanation of dataclass vs non-dataclass handling
This commit implements comprehensive batch processing for flash detection and fixes window close snapshot timing to properly detect unsaved changes. ## Window Close Snapshot Handling **Problem**: Window close events need to compare 'before' (with unsaved edits) vs 'after' (without unsaved edits) to detect when values revert. **Solution**: Collect two snapshots with proper timing: 1. 'before' snapshot: Collected while form manager is still registered (contains unsaved edited values) 2. 'after' snapshot: Collected AFTER form manager is unregistered using QTimer.singleShot(0) to defer until next event loop iteration **Scope Filtering**: For window close events, detect which scope was closed: - Step scope (contains '::'): Only check the specific step that was closed - Plate scope (no '::'): Check ALL steps (PipelineConfig affects all steps) This prevents checking steps with empty snapshots and ensures correct flash detection for both step editors and PipelineConfig editors. ## Batch Flash Detection **Problem**: Sequential flash detection was slow and caused flashes to appear one-by-one instead of simultaneously. **Solution**: Implemented two-phase batch update process: **Phase 1 - Update labels/styling**: - Collect all items to update - Build before/after object pairs - Batch check which items should flash (single call) - Update ALL labels and styling **Phase 2 - Trigger flashes**: - Trigger ALL flashes simultaneously (not sequentially) - This ensures all flashes start at the same time **Performance**: ~6.8x speedup (314ms → 46ms for 7 steps) ## Changes **PipelineEditor**: - Use saved 'after' snapshot from window close event - Detect scope type (step vs plate) to filter which steps to check - Implement batch flash detection with two-phase update - Reuse preview instances for both flash detection and label updates - Add detailed logging for debugging window close events **PlateManager**: - Implement _update_plate_items_batch() for batch processing - Use saved 'after' snapshot from window close event - Two-phase update: labels first, then trigger all flashes simultaneously - Remove old _update_single_plate_item() method (replaced by batch version) Both widgets now use the unified batch processing approach from CrossWindowPreviewMixin._check_resolved_values_changed_batch().
## Config Preview Formatting **Problem**: When well_filter is None, the formatter was returning None instead of showing the config indicator (e.g., 'NAP', 'FIJI', 'MAT'). **Solution**: Show the base indicator even when well_filter is None. This provides visual feedback that the config is enabled but has no filter set. **Before**: well_filter=None → no indicator shown **After**: well_filter=None → 'NAP' (or 'FIJI', 'MAT', etc.) This makes it clear that the config is enabled (otherwise no indicator would show at all) but no specific well filter is configured. ## Flash Opacity **Change**: Reduced flash opacity from 255 (100%) to 127 (~50%). This makes flashes more subtle and less jarring while still providing clear visual feedback that values changed.
… method - Replace event-specific state storage on listeners with parameter passing - Add handle_window_close() method that receives before/after snapshots as parameters - Store snapshots temporarily in _pending_window_close_* for timer callback access - Fix expansion logic to use live_context_before (has form manager values) instead of live_context_after (empty) - Add type-based identifier expansion for parent type paths (e.g., 'PipelineConfig.well_filter_config') - Expand parent type paths to all nested fields in all dataclasses that inherit from field type - Use canonical root detection (uppercase or in _preview_scope_aliases) for type identification - Defer listener notification until after form manager unregistration using QTimer.singleShot(0) - Update Sphinx docs to document new window close handler and type-based expansion Architectural improvement: Window close is a form manager event, not listener state. Passing snapshots as parameters is cleaner than setting attributes on listeners.
Document critical implementation details discovered during debugging: LiveContextSnapshot Structure: - Explain values (global) vs scoped_values (plate/step-specific) distinction - Document scope_id format (plate vs step scope) - Show how to extract scoped values for flash detection Canonical Root Aliasing System: - Document _preview_scope_aliases dict mapping lowercase to type names - Explain why both uppercase (PipelineConfig) and lowercase (pipeline_config) must be handled - Note this as future refactoring opportunity Batch Resolution Performance: - Explain why batch resolution is O(1) vs O(N) for individual resolution - Document context stack building cost (GlobalPipelineConfig → PipelineConfig → Step) - Show when to use resolve_all_lazy_attrs vs resolve_all_config_attrs The Three Identifier Formats: - Simple field name: 'well_filter' - Nested field path: 'well_filter_config.well_filter' - Parent type path: 'PipelineConfig.well_filter_config' - Document expansion logic for each format with examples Window Close Snapshot Timing: - Explain WHY timing is critical (form manager adds/removes live values) - Document before snapshot = edited values, after snapshot = reverted values - Explain deferred notification with QTimer.singleShot(0) Scope ID Extraction Logic: - Document _extract_scope_id_for_preview() behavior for different object types - Show plate scope vs step scope extraction - Explain why scope determines which scoped_values to use Common Pitfalls and Maintenance Notes: - Using after snapshot for expansion (WRONG - has no values) - Storing event-specific state on listeners (WRONG - causes AttributeError) - Forgetting to use scoped values (WRONG - misses plate/step-specific values) - Hardcoding type names (WRONG - misses canonical roots) - Using resolve_all_lazy_attrs for non-lazy fields (WRONG - misses inherited values) - Document future refactoring opportunities Debugging Flash Detection Issues: - Log file locations and key grep patterns - How to verify snapshot contents - How to verify identifier expansion - How to verify batch resolution - Common symptoms and solutions This documentation ensures the system is maintainable and provides a foundation for future refactoring once the system is stable.
…ators Add critical design principle section explaining that all flashes and labels are based on LIVE values from open form editors, not saved values on disk. Design Principle: Live Values, Not Saved Values: - Explain that unsaved edits immediately affect flash detection and labels - Provide concrete example scenario showing instant feedback on edit/revert - Document why this design exists (instant feedback, what-if exploration) - Explain architectural implication for LiveContextSnapshot and window close timing Future Enhancements: - Propose visual indicators for labels showing values based on unsaved changes - Suggest implementation approaches (asterisk, color tint, tooltip, icon) - Provide example code for comparing live vs saved context - Document benefits, challenges, and recommendation for optional feature This addresses the fundamental question: 'What values are we showing?' Answer: Live values - what WOULD happen if you saved right now.
…ction call The unsaved changes indicator (†) was not appearing immediately in the PipelineEditor when editing PipelineConfig. Two issues were fixed: 1. Scope filter mismatch: When collecting saved context snapshot (without form managers), we weren't passing the same scope_filter that was used to collect the live context snapshot. This caused comparisons between different scopes, preventing change detection. - Added scope_filter parameter to check_config_has_unsaved_changes() - Added scope_filter parameter to check_step_has_unsaved_changes() - Updated PlateManager to pass orchestrator.plate_path as scope_filter - Updated PipelineEditor to pass self.current_plate as scope_filter 2. Missing function argument: _format_resolved_step_for_display() was being called with only 2 arguments instead of 3 in _handle_full_preview_refresh(), causing original_step to receive the live_context_snapshot value and breaking unsaved change detection. - Fixed function call on line 1568 to pass all 3 required arguments The indicator now appears immediately in both PlateManager and PipelineEditor when changes are made, without requiring focus changes.
Added comprehensive documentation for the unsaved changes indicator feature to docs/source/architecture/scope_visual_feedback_system.rst. Documentation covers: - Implementation status and visual indicator (dagger symbol †) - Core functions: check_config_has_unsaved_changes() and check_step_has_unsaved_changes() - Critical scope_filter requirement for correct change detection - Token increment technique for cache bypass when collecting saved snapshots - PlateManager integration with _check_pipeline_config_has_unsaved_changes() - PipelineEditor integration in _format_resolved_step_for_display() - Bug fix for missing original_step parameter in function call - Compile warning dialog implementation - Performance considerations (token-based caching, early returns, scope filtering) - Visual feedback rationale for choosing dagger symbol - Debugging support with extensive logging This documents the implementation from commit e42430c.
PERFORMANCE IMPROVEMENTS: 1. Collect saved context snapshot ONCE per step instead of once per config - Previously: 3 snapshot collections per step (one for each config) - Now: 1 snapshot collection per step (reused for all configs) - Reduces snapshot collection overhead by 66% 2. Early exit on first detected change - Stop comparing fields as soon as ANY difference is found - Avoids unnecessary field comparisons when change already detected 3. Reduce excessive logging - Changed INFO logs to DEBUG for field comparisons - Removed verbose debug logging in pipeline_editor.py - Only log when changes are actually detected - Reduces log spam from 100+ lines per update to minimal output CHANGES: - config_preview_formatters.py: - Added saved_context_snapshot parameter to check_config_has_unsaved_changes() - Modified check_step_has_unsaved_changes() to collect saved snapshot once - Changed comparison logging from INFO to DEBUG level - Early exit on first field difference - pipeline_editor.py: - Removed excessive debug logging (🔍 and 🔥 emojis) - Removed verbose snapshot token logging - Removed massive debug block in _handle_full_preview_refresh() IMPACT: - Dramatically reduces log output during reactive updates - Improves performance by avoiding redundant snapshot collections - Maintains same functionality with better efficiency
PERFORMANCE IMPROVEMENTS:
1. Changed all 🔍 (magnifying glass) logs to DEBUG level:
- cross_window_preview_mixin.py: 40 INFO -> DEBUG (field expansion, batch resolution)
- live_context_resolver.py: 3 INFO -> DEBUG (resolve_all_lazy_attrs)
- config_preview_formatters.py: Already done in previous commit
2. Changed all 🔥 (fire) logs to DEBUG level:
- list_item_flash_animation.py: 7 INFO -> DEBUG (flash animation details)
- pipeline_editor.py: Flash step details
- plate_manager.py: Flash plate details
- cross_window_preview_mixin.py: 9 INFO -> DEBUG (timer/schedule details)
3. Changed all 🎨 (palette) logs to DEBUG level:
- list_item_delegate.py: 1 INFO -> DEBUG (painting background)
RATIONALE:
- These logs were creating MASSIVE log spam (100+ lines per window close)
- The actual computation is necessary (7 steps × 2 contexts = 14 resolutions)
- The logging was the performance bottleneck, not the computation
- High-level INFO logs remain ("FLASHING X steps", "Changed field")
- Detailed logs still available via DEBUG level for troubleshooting
IMPACT:
- Logs are now clean and readable at INFO level
- Performance improved by avoiding excessive string formatting
- Still have full debugging capability when needed
…ere actually modified When PipelineConfig editor is open, it creates form managers for ALL nested configs. Previously, we checked all 16 configs for unsaved changes even when only 1 was edited. Now we check if the specific config field is in _last_emitted_values, not just if the dict is non-empty. This means when you reset well_filter, we only check well_filter_config instead of all 16 configs. Result: Reset operations are now fast (same speed as typing in a field).
Added documentation explaining the performance optimization that checks _last_emitted_values to skip configs that haven't been modified. This optimization reduced reset operation time from ~500ms to ~10-20ms by only checking configs that were actually edited instead of all 16 configs in PipelineConfig.
…anges detection When checking if a step has unsaved changes, we need to merge the step's scoped live values (from the step editor's form manager) into the step before building the context stack for resolution. The existing _get_step_preview_instance() method already does this: 1. Builds the step's scope_id 2. Extracts scoped values for that scope_id from live_context_snapshot 3. Merges the step's live values into the step The bug was that _resolve_config_attr() was using the original step instead of the merged step in the context stack, so step editor changes were not visible during resolution. This is the same pattern used by flash detection and other preview logic.
1. Add _get_preview_instance_generic to CrossWindowPreviewMixin - Single source of truth for extracting and merging live values - Supports both global values (GlobalPipelineConfig) and scoped values (PipelineConfig, FunctionStep) - Used by both PipelineEditor and PlateManager 2. Add _build_context_stack_with_live_values to PipelineEditor - Single source of truth for building context stacks with preview instances - Parameterized to accept either original steps or preview instances - Used by both flash detection and unsaved changes detection 3. Implement _merge_with_live_values in PipelineEditor - Handles both dataclass objects (via dataclasses.replace) and non-dataclass objects (via copy + setattr) - Required by CrossWindowPreviewMixin interface 4. Refactor all _get_*_preview_instance methods to use generic helper - _get_step_preview_instance: uses generic helper with scoped values - _get_pipeline_config_preview_instance: uses generic helper with scoped values - _get_global_config_preview_instance: uses generic helper with global values 5. Add comprehensive documentation to scope_hierarchy_live_context.rst - Critical Pattern: Always Use Preview Instances for Resolution - Single Source of Truth: _build_context_stack_with_live_values - Generic Helper: _get_preview_instance_generic - Historical Bug: Unsaved Changes Not Detected This refactoring prevents the bug from recurring by: - Centralizing the pattern in reusable helpers - Adding explicit documentation about the critical pattern - Reducing code duplication across PipelineEditor and PlateManager - Making the step_is_preview parameter explicit to avoid confusion
- Created TreeFormFlashMixin for widgets with tree + form (ConfigWindow, StepParameterEditorWidget) - Added tree_item_flash_animation module for flashing QTreeWidgetItems with background color and bold font - Enhanced widget_flash_animation to support custom colors and GroupBox stylesheet flashing - Integrated flash detection into ParameterFormManager via _apply_placeholder_text_with_flash_detection() - Flash animations trigger when: 1. Nested config placeholders change (cross-window updates) -> GroupBox + tree item flash 2. Double-clicking tree items to scroll -> GroupBox flashes - All flash animators use global registries to prevent overlapping flashes - Updated Sphinx documentation with new flash animation details
…e-based matching PROBLEM: - Fast-path optimization used hardcoded 'step_' prefix pattern matching - Only checked PipelineConfig -> FunctionStep inheritance - Missed configs without 'step_' prefix (napari_streaming_config, fiji_streaming_config, etc.) - Violated dual-axis resolution architecture (X-axis context + Y-axis MRO) - Performance regression: checked ALL steps on EVERY keystroke (~100ms per step) SOLUTION: 1. check_config_has_unsaved_changes: Use isinstance() for type-based matching - Direct field match: check if config_attr in _last_emitted_values - Type-based match: check if isinstance(config, type(field_value)) - No hardcoded field name patterns or type names - Leverages Python's MRO for inheritance detection 2. check_step_has_unsaved_changes: Add fast-path to skip irrelevant steps - Collect all config objects ONCE per step - Check if ANY emitted field matches ANY config (by name or type) - Skip step entirely if no relevant changes found - Only proceed to full resolution if potential match exists PERFORMANCE: - Before: O(n_steps * n_configs) = 49 checks per keystroke - After: O(n_steps) where only relevant steps checked (typically 0-1) CORRECTNESS: - Handles all inheritance: Global -> Pipeline -> Step - Works for all config types (with or without 'step_' prefix) - Uses framework's MRO-based type resolution - No magic strings or hardcoded assumptions
…rrides Fixed bug where closing a config window (e.g., PipelineConfig) would incorrectly flash steps that have their own overrides, even though their resolved values didn't actually change. Root Cause: When a config window closed, the before_snapshot only contained the closing window's values. When creating step preview instances for flash detection, they lacked other open windows' values (e.g., step overrides), causing resolution to use the wrong scope values. Example scenario: - PipelineConfig has well_filter=2 (plate scope) - Step_6 has well_filter=3 (step scope override) - User closes PipelineConfig without saving - Before: step_6 preview had NO override → resolved to 2 (plate scope) - After: step_6 preview had override=3 → resolved to 3 (step scope) - Result: Incorrectly flashed because 2 != 3 After fix: - Before: step_6 preview has override=3 → resolved to 3 (step scope wins) - After: step_6 preview has override=3 → resolved to 3 (step scope wins) - Result: No flash because 3 == 3 ✓ Changes by functional area: * Parameter Form Manager: Use collect_live_context() for before_snapshot instead of _create_snapshot_for_this_manager() to include ALL active form managers' values. Change _last_emitted_values to use full field paths as keys (e.g., 'GlobalPipelineConfig.step_materialization_config.well_filter') instead of just field names. Clear _last_emitted_values on window close to prevent stale fast-path matches. * Unsaved Changes Detection: Update fast-path logic to extract config attribute from full field paths and add scope matching to prevent step windows from affecting other steps' unsaved change detection. Add logging for debugging fast-path decisions. * Cross-Window Preview System: Add hasattr() checks before accessing _pending_window_close_* attributes to handle both window close and incremental update code paths. Add logging for snapshot scoped_values keys and per-object flash detection results. * Pipeline Editor: Fix attribute names from _window_close_* to _pending_window_close_* to match mixin's naming. Move snapshot cleanup to AFTER _refresh_step_items_by_index completes (was causing AttributeError). Add extensive logging for window close refresh logic. * Config Window: Document that _on_global_config_field_changed should NOT update thread-local global config (unsaved edits propagate via live context, not by mutating the saved baseline). The key insight is that preview instances need ALL live values from ALL open windows to resolve correctly through the scope hierarchy. Using only the closing window's values breaks scope precedence (step scope > plate scope > global scope).
… documentation Extended scope_hierarchy_live_context.rst with critical missing documentation that makes the reactive UI system transparent, debuggable, and maintainable. New sections added: * Window Close Flash Detection System: Documents the critical insight that before_snapshot must include ALL active form managers (not just the closing window) to correctly handle scope precedence when multiple windows are open. Includes detailed example showing why this matters (step overrides vs plate scope values). * LiveContextSnapshot Structure: Documents the difference between 'values' (global context for GlobalPipelineConfig) and 'scoped_values' (scoped context for PipelineConfig/FunctionStep). Explains how preview instance creation extracts values from the correct location based on use_global_values flag. * Token-Based Cache Invalidation: Documents how _live_context_token_counter provides global cache invalidation on every parameter change. Explains how caches use (object_id, token) as cache keys to detect stale entries. * Field Path Format and Fast-Path Optimization: Documents the full field path format (e.g., 'GlobalPipelineConfig.step_materialization_config.well_filter') and how _last_emitted_values enables fast-path optimization to skip expensive resolution comparisons. Includes scope matching logic to prevent step windows from affecting other steps, and cleanup requirements on window close. These gaps made the recent window close flash detection bug difficult to understand and debug. The new documentation provides the architectural context needed to reason about scope precedence, snapshot requirements, and performance optimizations in the reactive UI system. Related to commit 3965119 (window close flash detection fix).
CRITICAL FIXES: 1. Remove harmful Phase 1 (would add O(n_steps) work) 2. Fix scope_filter bug in Phase 2 (critical for multi-plate) 3. Fix fragile string matching in Phase 3 (use manager refs) 4. Add explicit cache invalidation rules PLAN OVERVIEW: - Phase 1-ALT: Type-based caching for unsaved changes (O(n_configs) not O(n_steps)) - Phase 2: Batch context collection within same update cycle - Phase 3: Batch cross-window updates to reduce signal frequency - Phase 4: Verify flash detection batching (already exists) All phases ready for implementation. Each phase is independently testable and revertible. See REBUTTAL_AND_CORRECTIONS.md for detailed response to reviewer feedback.
Problem: When editing nested config fields (e.g., GlobalPipelineConfig.well_filter_config.well_filter),
the type-based cache was only marking the parent config type (WellFilterConfig) with the parent field
name ('well_filter_config'), not the actual changed field name ('well_filter'). This caused MRO cache
lookups to fail because the cache has entries like (WellFilterConfig, 'well_filter'), not
(WellFilterConfig, 'well_filter_config').
Root cause: parameter_changed signal only emits the parent config name for nested changes, losing
information about which specific field changed inside the nested config.
Solution: Connect BOTH signals:
1. parameter_changed -> _emit_cross_window_change() (to emit context_value_changed)
2. context_value_changed -> marking function (to mark config types with full field paths)
The context_value_changed signal contains the full field path (e.g., 'PipelineConfig.well_filter_config.well_filter'),
allowing us to extract the actual changed field name ('well_filter') for accurate MRO cache lookups.
This fixes two critical bugs:
- Editing GlobalPipelineConfig.well_filter_config.well_filter while step editor open -> step now flashes
- Editing GlobalPipelineConfig while PipelineConfig editor open -> plate list items now flash
…ation Created comprehensive Sphinx documentation for the reactive UI performance optimizations implemented in the previous commits. This documents: Phase 1-ALT: Type-Based Caching - Problem: O(n_managers) iteration on every change - Solution: Type-based cache mapping config types to changed field names - Performance: O(1) cache lookup, 10-100x speedup for fast-path checks MRO Inheritance Cache - Problem: Type-based cache didn't account for MRO inheritance - Solution: Build cache at startup mapping (parent_type, field_name) → child types - Performance: O(1) lookup, built once at startup in <10ms Signal Architecture Fix - Problem: Disconnecting parameter_changed broke signal chain - Solution: Connect BOTH parameter_changed and context_value_changed - Result: Full field paths enable accurate MRO cache lookups Bugs Fixed: 1. Editing GlobalPipelineConfig.well_filter_config.well_filter while step editor open 2. Editing GlobalPipelineConfig while PipelineConfig editor open 3. Early return bug when live_context_snapshot=None Added to architecture index with cross-references to related systems.
…ontamination Problem: When multiple plates are open, editing PipelineConfig in Plate 1 would trigger flash detection for Plate 2's steps. The fast-path checks in check_config_has_unsaved_changes() and check_step_has_unsaved_changes() were iterating through ALL active form managers without filtering by scope, causing cross-plate contamination. Root cause: The fast-path optimization added in Phase 1-ALT checks _last_emitted_values from all managers globally, but didn't apply scope filtering. This meant changes in one plate's PipelineConfig editor would be detected as relevant for steps in a different plate. Solution: Add scope filtering using ParameterFormManager._is_scope_visible_static() to both fast-path checks. This ensures that only managers within the same scope (plate path) are considered when checking for unsaved changes. Changes: - check_config_has_unsaved_changes(): Added scope filter check before iterating _last_emitted_values - check_step_has_unsaved_changes(): Added plate-level scope filter to step-specific scope matching This fixes the multi-plate bug where things start breaking down with 2 plates open.
Changed logger.debug() to logger.info() for: - Scope filtering decisions (skipping/including managers) - Manager checking with scope_id and _last_emitted_values - Path match detection - Type match detection - No form managers with changes This will help debug the issue where having a PipelineConfig open excludes any refresh from being triggered by GlobalPipelineConfig changes in the plate manager.
…lues work for all objects - Modified get_user_modified_values() to extract raw values from nested dataclasses for ALL objects (lazy dataclasses, scoped objects like FunctionStep, etc.) - Removed early return for non-lazy-dataclass objects that was breaking sibling inheritance - Moved tuple reconstruction logic into _create_overlay_instance() to centralize it - Added dataclass check before reconstructing tuples to avoid errors with functions - Fixed parent overlay scope passing to use parent's scope_id for correct specificity - Made AbstractStep inherit from both ContextProvider and ScopedObject - Removed redundant ScopedObject inheritance from FunctionStep This fixes the regression where sibling inheritance stopped working in the step editor after commit 62e0676. Now when step_well_filter_config.well_filter is changed, sibling configs (step_materialization_config, etc.) correctly show the inherited value in their placeholders.
Created docs/source/architecture/sibling_inheritance_system.rst with: - Complete architecture explanation (parent overlay pattern) - Implementation details for all key components - Scoped objects vs lazy dataclasses distinction - Common pitfalls and debugging guide with 3 bug case studies - Code navigation guide for understanding the system - Mental model for horizontal vs vertical inheritance - Cross-references to related documentation Added cross-references in: - docs/source/architecture/index.rst (added to Configuration Systems toctree) - docs/source/architecture/configuration_framework.rst (sibling inheritance section) - docs/source/architecture/context_system.rst (ScopedObject section) - docs/source/architecture/parameter_form_lifecycle.rst (cross-window updates) - docs/source/development/scope_hierarchy_live_context.rst (scope specificity) This documentation provides the complete mental model needed to understand the bug fixed in commit 9d21d49 and how to navigate/debug similar issues.
BUG: ScopeProvider was stripping step suffix from scope strings, causing nested config managers in step editors to resolve placeholders at plate scope (specificity=1) instead of step scope (specificity=2). This broke sibling inheritance for auto-generated steps. ROOT CAUSE: - ScopeProvider.__init__() extracted only plate_path from scope_string by splitting on '::' and taking the first part - config_context() used str(context_provider.plate_path) as scope_id - Result: '/path/to/plate::step_7' became '/path/to/plate' - Resolver saw current_specificity=1 and skipped all step-scoped configs FIX: - Store full scope_string in ScopeProvider (preserve hierarchy) - Use scope_string instead of plate_path in config_context() - Remove unnecessary plate_path extraction (simpler code) IMPACT: - Sibling inheritance now works for auto-generated steps - Nested configs resolve at correct scope specificity - Parent overlay values are no longer skipped by resolver
BUG: PipelineConfig and FunctionStep build_scope_id() methods tried to access context_provider.plate_path, which doesn't exist on ScopeProvider. FIX: - Check if context_provider is a ScopeProvider - For PipelineConfig: extract plate path from scope_string (split on '::') - For FunctionStep: return scope_string directly (already has full scope) This allows ScopeProvider to only store scope_string without plate_path.
Make it clear that: - Orchestrator: builds scope from attributes - ScopeProvider: returns/extracts from scope_string
Make it clear that: - Orchestrator: builds scope from attributes - ScopeProvider: returns/extracts from scope_string
BEFORE: config_context() called build_scope_id() even with ScopeProvider, requiring build_scope_id() methods to check isinstance(ScopeProvider) and return scope_string. Redundant conditional logic in two places. AFTER: config_context() checks ScopeProvider FIRST and uses scope_string directly. build_scope_id() methods only handle orchestrator case. RESULT: Single responsibility - ScopeProvider handling in config_context(), scope building in build_scope_id(). Cleaner, simpler code.
CRITICAL BUG FIX: _emit_cross_window_change was returning early for 'enabled' fields BEFORE notifying parent manager, which prevented parent from reconstructing dataclass and storing in cache. This broke sibling inheritance for enabled fields while other bool fields (like persistent) worked correctly. ROOT CAUSE: - When enabled checkbox clicked, _emit_cross_window_change returned at line 4409 - This skipped calling parent._on_nested_parameter_changed at line 4418 - Parent never reconstructed dataclass, so get_user_modified_values() never saw updated enabled value - Result: sibling configs (napari/fiji) didn't inherit enabled changes FIX: - Move 'if param_name == enabled: return' check AFTER parent notification - Now parent gets notified, reconstructs dataclass, stores in cache - Sibling inheritance works for enabled fields like any other field SPECIAL HANDLING FOR 'enabled' FIELDS: The early return for 'enabled' prevents infinite placeholder refresh loops because enabled field changes trigger styling updates which can trigger more placeholder refreshes. The early return skips the root-level placeholder refresh (line 4420+) but must NOT skip parent notification (line 4415) otherwise sibling inheritance breaks.
The is_global_config_type() function checks for _is_global_config attribute, but auto_create_decorator never set it. This caused GlobalPipelineConfig to be incorrectly assigned plate-level scopes in collect_live_context(), breaking sibling inheritance for enabled fields in the pipeline config window. Root cause: is_global_config_type(base_type) returned False because the marker was never set, so the scope override logic was skipped.
When building the scopes dict for placeholder resolution, step-level managers were adding their scope entries (e.g., StepWellFilterConfig -> plate::step_6), which polluted plate-level resolution and broke sibling inheritance. The fix uses is_scope_at_or_above() instead of _is_scope_visible_static() when building scopes, preventing step-level managers from adding their scope entries when the filter is plate-level. Key distinction: - Values collection: Uses bidirectional matching (step values ARE collected) - Scopes dict: Uses strict filtering (step scopes are NOT added) This ensures pipeline editor can see step values for previews, but step-level scope assignments don't pollute plate-level placeholder resolution. Bug: When step editor was open, PipelineConfig's step_materialization_config couldn't see sibling step_well_filter_config values because the scopes dict had StepWellFilterConfig mapped to ::step_6 (specificity 2) instead of plate (specificity 1).
Added two centralized scope visibility functions:
1. is_scope_visible(manager_scope, filter_scope):
- Bidirectional matching - returns True if scopes are in same hierarchy
- Used for manager enumeration (e.g., finding step editors within a plate)
- Example: is_scope_visible('plate::step', 'plate') → True
2. is_scope_at_or_above(manager_scope, filter_scope):
- Strict matching - returns True only if manager is at same level or LESS specific
- Used for placeholder resolution to prevent scope contamination
- Example: is_scope_at_or_above('plate::step', 'plate') → False
These functions centralize scope logic that was previously duplicated across
parameter_form_manager.py with inconsistent implementations.
Also fixed cross_window_preview_mixin to return empty set instead of None
for unknown scopes (fail-safe default).
Added comprehensive documentation explaining the critical distinction between two scope filtering use cases: 1. Values Collection (bidirectional matching via is_scope_visible): - Used when collecting form values for preview/comparison - Step-level managers ARE included when filtering by plate-level - Enables pipeline editor to see unsaved step changes 2. Scopes Dict Building (strict filtering via is_scope_at_or_above): - Used when building the scopes dict for placeholder resolution - Step-level managers are NOT included when filtering by plate-level - Prevents step-level scopes from polluting plate-level resolution The documentation explains: - Why using the same filter for both causes scope contamination bugs - How scope contamination breaks sibling inheritance - Implementation details with code examples - The role of the scopes dict in placeholder resolution This documents the fix for the bug where step_materialization_config couldn't see sibling step_well_filter_config values when a step editor was open.
Added reference to scope_filtering_dual_use_cases doc in the sibling inheritance system documentation, as the scope filtering distinction is critical for understanding how sibling inheritance works correctly.
…fig closes
Root cause: In collect_live_context(), when scope_filter=None, all scoped
managers (including step editors) were being SKIPPED. This meant when a
PipelineConfig editor closed, the 'after' snapshot had scoped_values={},
causing the pipeline editor to incorrectly think step editors had no
unsaved changes.
Fix: Changed scope filtering logic so scope_filter=None means 'no filtering'
- include ALL managers (global + all scoped). This matches the comment that
was already in the code at multiple call sites.
- Add ScopeFilterMode enum with factory methods for_value_collection() and for_scopes_dict() - Polymorphic should_include() dispatches to predicate via dict lookup - Eliminates if/else branching at call sites - Callers now use factory methods instead of inline conditionals - Remove unused _is_scope_visible_static() method - Normalize Path→str conversion inside enum
… flash detection Window close was emitting identifiers in TypeName.field format (e.g., GlobalPipelineConfig.well_filter_config) while typing emitted field_name.nested_field format (e.g., well_filter_config.well_filter). The flash detection code couldn't walk TypeName.field paths because TypeName is a type name not an attribute on the object. This caused flashes to not trigger when going from unsaved→saved state on window close. Changes: - Add _collect_all_field_paths() to recursively collect paths from root + nested form managers - Window close now uses pre-collected field paths matching typing format - Remove _preview_fields filtering in flash detection (flash on ANY change, not just preview fields) - Add hasattr check in _expand_identifiers_for_inheritance() to prevent AttributeError on incompatible types - Fix PlateManager attribute naming (_window_close_* → _pending_window_close_*) - Add QApplication.processEvents() after flashes to ensure visibility before heavy work The architectural principle: window close should trigger the same code path as typing, just resetting all values back to saved state.
…h detection Documents the bug where window close emitted TypeName.field format identifiers that couldn't be walked, and the fix using recursive _collect_all_field_paths(). Covers: - The problem (TypeName.field vs field_name.nested_field) - Root cause analysis (root manager's field_id is type name) - The fix (recursive collection from nested managers) - hasattr guard for cross-type expansion - processEvents() for flash visibility - Testing instructions
- Use QVariantAnimation for 60fps color interpolation - Rapid fade-in (100ms) with OutQuad easing - Hold at max flash color while rapid updates continue (150ms timer reset) - Smooth fade-out (350ms) with InOutCubic easing when updates stop - Fix GroupBox animation using QPalette.ColorRole.Window - Add focus-instead-of-duplicate window management via scope_id registry
Pre-compute live and saved context snapshots ONCE in the coordinator (_execute_coordinated_updates) and share with all listeners via get_batch_snapshots() class method. Changes: - parameter_form_manager.py: Add _batch_live_context_snapshot and _batch_saved_context_snapshot class attributes, compute in _execute_coordinated_updates, add get_batch_snapshots() accessor - pipeline_editor.py: Use batch snapshots in _process_pending_preview_updates and _refresh_step_items_by_index - plate_manager.py: Use batch snapshots in _process_pending_preview_updates - config_preview_formatters.py: Bypass both fast-paths when saved_context_snapshot provided (batch operation) to ensure actual live vs saved comparison occurs Performance impact: - Before: ~800ms gap between PlateManager and PipelineEditor updates - After: Both components flash simultaneously (same batch execution) - Snapshot computation done 1x instead of 2x per batch Fixes unsaved marker not appearing on first change after reset by ensuring batch operations bypass cache fast-paths that incorrectly returned False when cache was empty.
Step-level form managers (specificity >= 2) now add their values ONLY to scoped_live_context, not to the global live_context dict. This fixes the bug where editing step_6 caused all steps (0-5) to flash because they all read from the same global live_context[FunctionStep]. Uses get_scope_specificity() for proper semantic scope level detection: - Specificity 0 (global): values go to live_context only - Specificity 1 (plate): values go to both live_context and scoped_live_context - Specificity 2+ (step): values go ONLY to scoped_live_context
… editors - Remove type-based matching in check_config_has_unsaved_changes that caused false positives due to deep inheritance hierarchy (e.g., LazyFijiStreamingConfig inherits from LazyWellFilterConfig via MRO) - Path-based matching is sufficient and correct - Add saved_context_snapshot parameter to _check_pipeline_config_has_unsaved_changes to bypass fast-path during batch operations (mirrors step-level fix from 647d61d)
# Conflicts: # openhcs/config_framework/lazy_factory.py
…resolution - config_window.py, function_list_editor.py: Call _reconstruct_tuples_to_instances() before creating config instances in save_config() to convert (type, dict) tuples back to proper dataclass instances - plate_manager.py: Fix _get_pipeline_config_preview_instance to skip None values from scoped configs during merge (None means inherit, not override) - plate_manager.py: Fix _merge_with_live_values to skip None values from live values so original saved values can resolve through context stack - plate_manager.py: Use _get_pipeline_config_preview_instance in _build_config_preview_labels instead of generic _get_preview_instance to ensure global values (like num_workers) are included in preview instance - parameter_form_manager.py: Add _reconstruct_tuples_to_instances classmethod to convert nested dataclass tuples back to instances
…t global config values For lazy dataclasses, getattr() triggers resolution which falls back to class defaults instead of checking if the raw value is None and inheriting from global config. This caused GlobalPipelineConfig values like path_planning_config.output_dir_suffix to be incorrectly overwritten with defaults during ZMQ execution. The fix uses __dict__.get() to access the raw stored value, allowing None values to properly trigger inheritance from the global config.
QListWidget.__bool__() returns False when empty (0 items), so the check 'if self.plate_list:' was failing when loading plates from code with no existing plates. This caused update_plate_list() to never be called, leaving the UI empty until another action triggered a refresh. Removed the unnecessary check since plate_list is always initialized in __init__ and will never be None.
…e key BUG: When step_well_filter_config.well_filter_mode was changed to EXCLUDE, only napari_streaming_config and fiji_streaming_config showed the inherited value. step_materialization_config and streaming_defaults still showed INCLUDE. ROOT CAUSE: The _lazy_resolution_cache key was (class_name, field_name, token) which didn't account for the context scope. Values resolved with scope_id=None (during PipelineConfig context) were being cached and incorrectly returned for step-scoped resolutions that should have inherited from StepWellFilterConfig. FIX: Cache key is now (class_name, field_name, token, scope_id) which ensures: - Values resolved with scope_id=None won't pollute step-scoped lookups - Different steps with different scope_ids get separate cache entries - Cross-scope cache pollution is prevented Also re-enables caching (was disabled for debugging).
- Update lazy resolution cache section to show new 4-tuple cache key (class_name, field_name, token, scope_id) instead of 3-tuple - Add Issue 5 documenting the cross-scope cache pollution bug and fix - Update line number references for access pattern
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scope-Based Visual Feedback System & Reactive UI Performance Optimizations
Overview
This PR implements a comprehensive scope-based visual feedback system and refactors the configuration framework to support generic N-level scope hierarchies. The work addresses fundamental architectural limitations in how the GUI represents configuration inheritance and propagates changes across windows.
Key Achievements:
GlobalPipelineConfig/PipelineConfigchecksNew Features
1. Scope-Based Visual Feedback System
Motivation: The GUI had no visual indication of which plate a step belonged to or how configuration changes propagated through the inheritance hierarchy. Users couldn't tell if a flash was due to their direct edit or an inherited value change.
Solution: Implemented a complete visual feedback system using perceptually distinct colors, layered borders, and flash animations that trigger only when resolved values change.
Scope-Based Coloring
distinctipylibraryLayered Borders
Steps within each orchestrator get layered borders to visually differentiate them:
"plate_path::step_token@position"Flash Animations
Critical Design Decision: Flash on resolved value changes, not raw value changes.
Why this matters: A step with an override shouldn't flash if you edit the inherited value, because the step's resolved value didn't change. This prevents false positives and makes flashes semantically meaningful.
Implementation:
Design Principle: Live Values, Not Saved Values
Critical concept: All flashes and labels are based on live values from open form editors, not saved values on disk.
When you open a config editor and make changes without saving:
When you close the editor without saving:
This provides instant feedback and enables "what-if" exploration without committing changes.
2. Centralized Cache Settings Toggle
Motivation: During development, cache-related bugs were difficult to isolate because caches were scattered across multiple modules with no unified control.
Solution:
openhcs/config_framework/cache_settings.pyprovides a single source of truth for cache toggles, controllable via environment variables. Useful for debugging correctness by forcing fresh resolution without modifying code.Architecture & Framework Changes
1. Generic Scope Hierarchy System
Problem: The configuration framework had hardcoded assumptions about a 2-level hierarchy (global → plate). Adding step-level configs required 50+ isinstance checks scattered across the codebase. This violated the open/closed principle and made the system brittle.
Solution: Refactored to a generic N-level scope hierarchy using the
ScopedObjectABC.New Abstractions
ScopedObjectABC (openhcs/config_framework/config.py):Implemented by:
GlobalPipelineConfig→ returnsNone(global scope)PipelineConfig→ returns plate pathFunctionStep→ returns"plate_path::step_token"GlobalConfigBaseVirtual Base Class:isinstance()checks without inheritanceisinstance(config, GlobalPipelineConfig)checksScopeProviderHelper:ScopeProvider(scope_id="...")Scope Contamination Fix
Critical Bug: Parent scopes (GlobalPipelineConfig) were incorrectly inheriting values from child scopes (PipelineConfig/FunctionStep). This violated the semantic meaning of "global" configuration.
Root Cause:
dual_axis_resolver.pyhad no concept of scope specificity - it would merge contexts from all scopes indiscriminately.Solution: Added scope specificity filtering:
context_manager.pyvia_calculate_scope_specificity()and scope filtering in context merging2. Framework-Level Cache Control
Problem: Cache-related bugs were recurring throughout development. When a bug appeared, it was unclear which cache was responsible, and disabling caches required code changes.
Solution:
FrameworkConfigclass inopenhcs/config_framework/config.pyprovides environment variable-based cache toggles:Environment Variables:
OPENHCS_DISABLE_TOKEN_CACHES=1- Disable all cachesOPENHCS_DISABLE_LAZY_RESOLUTION_CACHE=1- Disable lazy resolution cacheOPENHCS_DISABLE_PLACEHOLDER_CACHE=1- Disable placeholder text cacheOPENHCS_DISABLE_LIVE_CONTEXT_CACHE=1- Disable live context resolver cacheOPENHCS_DISABLE_UNSAVED_CHANGES_CACHE=1- Disable unsaved changes cacheIntegration: Caches check these flags before returning cached values, allowing runtime debugging without code modification.
3. Scope-Aware Configuration Priority
Problem: Unsaved changes cache was unscoped, causing cross-step contamination. Editing step 6 would incorrectly mark step 0 as having unsaved changes.
Root Cause: Cache used
Dict[Type, Set[str]]- keyed only by config type, not by scope. All steps shared the same cache entry.Solution:
Dict[Tuple[Type, Optional[str]], Set[str]]- keyed by(config_type, scope_id)context.tokento choose between preview instances (live values) and original instances (saved values)Files Changed:
openhcs/config_framework/context_manager.py- Scope tracking context varsopenhcs/config_framework/dual_axis_resolver.py- Scope specificity calculationopenhcs/pyqt_gui/widgets/config_preview_formatters.py- Scoped cache structureopenhcs/pyqt_gui/widgets/shared/parameter_form_manager.py- LiveContextSnapshot.scopes fieldPerformance Optimizations
1. Type-Based Caching (O(n_managers) → O(1))
Problem: Unsaved changes detection iterated through all form managers on every keystroke.
Solution: Type-based cache
Dict[Type, Set[str]]maps config type → changed field names. O(1) lookup to check if any manager has unsaved changes for a given config type.2. Batch Cross-Window Updates (John Carmack-style)
Problem: Cross-window updates used Qt signals, causing O(N×M) signal emissions (N managers × M steps).
Solution: Universal reactive update coordinator with direct widget updates. Batch all cross-window updates into single operation, eliminating signal emission overhead.
Files:
cross_window_preview_mixin.py,pipeline_editor.py,plate_manager.py3. Placeholder Refresh Filtering (50-90% reduction)
Problem: Every keystroke refreshed placeholders for ALL fields, even unchanged ones.
Solution: Track which fields changed and only refresh placeholders for those fields. 50-90% reduction in placeholder resolution calls.
4. Class-Level Lazy Cache (70-90% speedup)
Problem: Each lazy dataclass instance had its own resolution cache, causing redundant resolution.
Solution: Class-level cache shared across all instances. 70-90% speedup for lazy field resolution.
File:
openhcs/config_framework/lazy_factory.py5. Logging Spam Reduction
Problem: 140+ log messages per keystroke from flash animations, flash restorations, and batch processing.
Solution: Reduced logging verbosity, batched flash restorations via coordinator.
6. Content-Based Caching & Batch Resolution
Problem: Flash detection was creating 29MB-31MB logs per keystroke, thousands of function calls per second. Root cause:
extract_all_configs()was being called repeatedly for frozen dataclasses recreated withdataclasses.replace().Solution:
extract_all_configs()- cache by dataclass content, not identityresolve_all_config_attrs()- O(1) context setup when resolving multiple attributesLazyStepWellFilterConfigmatchesStepWellFilterConfig)Performance Impact:
Files:
context_manager.py,dual_axis_resolver.py,lazy_factory.py,cross_window_preview_mixin.py7. Batch Flash Detection (6.8x speedup)
Problem: Flash detection was sequential - check item 1, flash item 1, check item 2, flash item 2, etc.
Solution: Two-phase batch process:
Performance: 314ms → 46ms for 7 steps (6.8x speedup)
Files:
pipeline_editor.py,plate_manager.pyBug Fixes
1. Invisible Flash Animation
Problem: Flash animations were being called but not visible to users.
Root Cause: Scope-based styling methods were called AFTER flash, overwriting the flash background color with normal scope colors.
Solution: Reordered operations to apply styling BEFORE flash.
2. Flash Killed by Debounced Updates
Problem: Flash animations were invisible for isolated keystrokes. User types → flash starts (300ms duration) → 123ms later, debounce triggers preview update → styling overwrites flash color → flash becomes invisible.
Solution: Added
reapply_flash_if_active()helper to check if item is currently flashing and restore flash color if it was overwritten.3. Unsaved Changes Detection Asymmetry
Problem: Editing
PipelineConfig.well_filter_config.well_filtershowed unsaved changes for both plate AND steps, but editingPipelineConfig.step_well_filter_config.well_filteronly showed unsaved changes for plate.Root Cause: Cache was using inconsistent key formats - cache population used just
typeas key, but cache lookup used(type, scope)tuple as key.Solution: Changed cache key from
step_config_typeto(step_config_type, cache_scope_id)tuple to match the lookup pattern.4. Race Condition Requiring Typing Twice
Problem: First keystroke only showed unsaved changes for plate, second keystroke showed unsaved changes for both plate AND steps.
Root Cause: PipelineEditor processes BEFORE PlateManager, so when steps are checked on the first keystroke, the cache is empty (PlateManager hasn't populated it yet).
Solution: Changed active manager check to inspect field paths in
_last_emitted_valuesinstead of just checking if the dict is truthy.5. Async Placeholder Rendering Bug
Problem: Placeholders not rendering correctly in nested configuration forms.
Dual Root Causes:
_pending_nested_managersSolution: Pre-register nested managers with placeholder (None) BEFORE creation to prevent race condition; invalidate placeholder refresh cache before final refresh.
6. Nested Form Placeholder Bug
Problem: Lazy class
__init__not inheriting custom__init__from@global_pipeline_configdecorated base classes, causing fields to initialize with concrete values instead of None.Solution: Detect base class custom
__init__marker and apply same logic to lazy class; apply_fix_dataclass_field_defaults_post_processingto both base class (inherited fields only) and lazy class (ALL fields).7. Placeholder Rendering on Initial Window Open
Problem: Placeholders not rendering on initial window open (sync mode).
Root Cause: Widgets were created but not visible when placeholders were applied. Qt's
update()doesn't trigger repaint for invisible widgets. Sync widget creation path had only ONE event loop deferral, but widgets need TWO event loop ticks to become visible and paintable.Solution: Added second
QTimer.singleShot(0, ...)in sync path to match async behavior.8. Streaming Configs Incorrectly Resolving to None
Problem:
FijiStreamingConfig/NapariStreamingConfigwere None instead of havingenabled=False.Root Cause:
lazy_factory.py's_fix_dataclass_field_defaults_post_processing()was setting ALL fields to None default in the generated__init__, ignoring fields withdefault_factory.Solution: Modified generated
__init__to use_MISSINGsentinel fordefault_factoryfields and call factory if value is_MISSING.9. Nested Config Placeholders Not Inheriting from ui_hidden Fields
Problem:
FijiStreamingConfigcouldn't inherit fromFijiDisplayConfig(ui_hidden).Root Cause:
collect_live_context()was filtering out scoped managers whenscope_filter=None, preventing GlobalPipelineConfig from seeing nested config values from open windows.Solution: Changed
scope_filter=Noneto mean "no filtering" (include ALL scopes) instead of "only global scope".10. Placeholder Live Context Scoping
Problem: Placeholders not updating correctly across different scopes.
Solution: Fixed scope filtering logic in live context collection and cache refresh mechanisms.
Documentation
New Architecture Documentation (6 files, ~3,800 lines):
docs/source/architecture/scope_visual_feedback_system.rst(1,531 lines) - Complete documentation of visual feedback system architecturedocs/source/architecture/reactive_ui_performance_optimizations.rst(569 lines) - Performance optimization patterns and rationaledocs/source/architecture/caching_architecture.rst(365 lines) - Maps all 5 caching systems and their token-based invalidation pointsdocs/source/development/scope_hierarchy_live_context.rst(860 lines) - Scope hierarchy system and live context resolutiondocs/source/development/visual_feedback_integration.rst(343 lines) - Integration guide for adding visual feedback to new widgetsdocs/source/user_guide/visual_feedback.rst(191 lines) - User-facing documentationUpdated Documentation:
Breaking Changes
1. config_context() Signature Change
Before:
After:
Migration: Replace all
scope_idparameter calls withcontext_providerparameter. For objects implementingScopedObject, pass the object directly. For UI code without full objects, useScopeProvider(scope_id="...").Rationale: The old API required manually building scope_id strings. The new API uses the
ScopedObjectinterface, making it type-safe and eliminating string manipulation.2. LiveContextSnapshot.scopes Field
Change: Added new
scopes: Dict[str, str]field toLiveContextSnapshot.Impact: Backward compatible - old code ignores the new field. New code uses it for scope-aware resolution.
3. Scope Contamination Fixes
Change: Scope specificity filtering prevents parent scopes from seeing child scope values.
Impact: May change resolved values in edge cases where GlobalPipelineConfig was incorrectly inheriting from PipelineConfig. This is a correctness fix, not a regression.
Dependencies
Added:
wcag-contrast-ratio>=0.9- WCAG color contrast compliance checkingPurpose: Ensures all generated scope colors meet WCAG AA accessibility standards (4.5:1 contrast ratio) for users with visual impairments.
Testing
Testing Approach: Primarily manual testing due to the GUI-heavy nature of this work.
Manual Test Coverage:
enabled=Falseinstead of NoneIntegration Tests Updated:
tests/integration/test_main.py- Updated streaming config creation pattern to always-create-with-enabled-flag patternReview Guidance
Focus Areas for Review
Performance Profiling
cProfileorpy-spyto identify remaining bottlenecksFuture Refactoring Opportunities
antiducktype_ui_refactorbranch):parameter_form_manager.pyantiducktype_ui_refactorbranch implements a polymorphic widget strategy pattern that eliminates duck typingapply_scope_styling()methods currently check widget types withisinstance()Architecture Review
Code Quality
widget_flash_animation.py,tree_item_flash_animation.py,list_item_flash_animation.pyhave similar logic - could be unifiedscope_color_utils.pyandscope_visual_config.pyhave overlapping responsibilitiesDocumentation Completeness
Known Limitations
antiducktype_ui_refactorbranch)Suggested Next Steps
antiducktype_ui_refactorbranch to eliminate widget duck typingcaching_architecture.rst)