|
| 1 | +# Node Sizing and Pin Positioning Fix Plan |
| 2 | + |
| 3 | +## Problem Summary |
| 4 | + |
| 5 | +There are two related bugs affecting node display and layout: |
| 6 | + |
| 7 | +1. **Pin Positioning Bug**: When nodes are created, deleted, and undone, pins don't position correctly until the node is manually resized |
| 8 | +2. **Node Sizing Bug**: When loading nodes with size smaller than minimum required for GUI + pins, the node is crushed and GUI elements are compressed |
| 9 | + |
| 10 | +## Root Cause Analysis |
| 11 | + |
| 12 | +### Pin Positioning Issue |
| 13 | +- Located in `src/node.py` `_update_layout()` method (lines 218-269) |
| 14 | +- Pin positioning is calculated correctly but visual update doesn't trigger properly |
| 15 | +- `pin.update_label_pos()` is called but pin visual refresh may not occur |
| 16 | +- Issue manifests during undo operations when nodes are recreated from serialized state |
| 17 | + |
| 18 | +### Node Sizing Issue |
| 19 | +- Located in `src/node.py` `_calculate_minimum_height()` and `fit_size_to_content()` methods |
| 20 | +- Minimum size calculation occurs but enforcement is inconsistent |
| 21 | +- During undo restoration in `src/commands/node_commands.py` DeleteNodeCommand.undo() (lines 250-384) |
| 22 | +- Size is set multiple times but may not respect GUI content minimum requirements |
| 23 | + |
| 24 | +## Comprehensive Fix Plan |
| 25 | + |
| 26 | +### Phase 1: Core Layout System Fixes |
| 27 | + |
| 28 | +#### Task 1.1: Improve Pin Position Update Mechanism |
| 29 | +**Location**: `src/pin.py` |
| 30 | +- **Issue**: `update_label_pos()` method (lines 61-68) only updates label position, not pin visual state |
| 31 | +- **Fix**: Add explicit pin visual refresh after position updates |
| 32 | +- **Implementation**: |
| 33 | + - Add `update_visual_state()` method to Pin class |
| 34 | + - Call `self.update()` to trigger Qt repaint |
| 35 | + - Ensure pin connections are also updated (`update_connections()`) |
| 36 | + |
| 37 | +#### Task 1.2: Enhance Node Layout Update Process |
| 38 | +**Location**: `src/node.py` `_update_layout()` method |
| 39 | +- **Issue**: Layout calculation is correct but visual update chain is incomplete |
| 40 | +- **Fix**: Ensure complete visual refresh after layout changes |
| 41 | +- **Implementation**: |
| 42 | + - Call `self.prepareGeometryChange()` before any position changes |
| 43 | + - Force pin visual updates after positioning |
| 44 | + - Trigger `self.update()` to refresh node visual state |
| 45 | + - Update all pin connections after layout changes |
| 46 | + |
| 47 | +#### Task 1.3: Fix Minimum Size Enforcement |
| 48 | +**Location**: `src/node.py` `_calculate_minimum_height()` and `fit_size_to_content()` |
| 49 | +- **Issue**: Minimum size calculation doesn't account for all content properly |
| 50 | +- **Fix**: Improve minimum size calculation and enforcement |
| 51 | +- **Implementation**: |
| 52 | + - Include proxy widget minimum size requirements |
| 53 | + - Add safety margins for GUI content |
| 54 | + - Ensure width calculation includes pin labels and content |
| 55 | + - Prevent size from being set below calculated minimum |
| 56 | + |
| 57 | +#### Task 1.4: Comprehensive Minimum Size Calculation System |
| 58 | +**Location**: `src/node.py` |
| 59 | +- **Issue**: No comprehensive method to calculate absolute minimum node size for all content |
| 60 | +- **Fix**: Create robust minimum size calculation that accounts for all node components |
| 61 | +- **Implementation**: |
| 62 | + - Add `calculate_absolute_minimum_size()` method that returns (min_width, min_height) |
| 63 | + - Calculate minimum width based on: |
| 64 | + - Title text width |
| 65 | + - Longest pin label width (input and output sides) |
| 66 | + - GUI content minimum width |
| 67 | + - Minimum node padding and margins |
| 68 | + - Calculate minimum height based on: |
| 69 | + - Title bar height |
| 70 | + - Pin area height (max of input/output pin counts × pin_spacing) |
| 71 | + - GUI content minimum height |
| 72 | + - Required spacing and margins |
| 73 | + - Include safety margins for visual clarity |
| 74 | + - Account for resize handle area |
| 75 | + |
| 76 | +### Phase 2: File Loading and Undo/Redo System Fixes |
| 77 | + |
| 78 | +#### Task 2.1: Add Minimum Size Validation on Node Loading |
| 79 | +**Location**: `src/file_operations.py` and node creation/loading functions |
| 80 | +- **Issue**: Nodes can be loaded with sizes smaller than their minimum requirements, causing layout issues |
| 81 | +- **Fix**: Validate and correct node sizes during loading operations |
| 82 | +- **Implementation**: |
| 83 | + - Add validation check in node loading/deserialization functions |
| 84 | + - Call `calculate_absolute_minimum_size()` for each loaded node |
| 85 | + - Compare loaded size against calculated minimum size |
| 86 | + - If loaded size is smaller than minimum, automatically adjust to minimum |
| 87 | + - Log size corrections for debugging purposes |
| 88 | + - Apply this validation in: |
| 89 | + - Graph file loading (.md and .json formats) |
| 90 | + - Node creation from templates |
| 91 | + - Import operations |
| 92 | + - Any node deserialization process |
| 93 | + |
| 94 | +#### Task 2.2: Improve Node Restoration Process |
| 95 | +**Location**: `src/commands/node_commands.py` DeleteNodeCommand.undo() |
| 96 | +- **Issue**: Node recreation process doesn't properly trigger layout updates |
| 97 | +- **Fix**: Ensure proper initialization sequence during node restoration |
| 98 | +- **Implementation**: |
| 99 | + - Call `fit_size_to_content()` after all properties are set |
| 100 | + - Force `_update_layout()` after pin creation |
| 101 | + - Add explicit visual refresh after restoration |
| 102 | + - Ensure GUI state is applied before size calculations |
| 103 | + - Validate restored size against minimum requirements using new `calculate_absolute_minimum_size()` |
| 104 | + |
| 105 | +#### Task 2.3: Add Post-Restoration Layout Validation |
| 106 | +**Location**: `src/commands/node_commands.py` |
| 107 | +- **Issue**: No validation that restored node layout is correct |
| 108 | +- **Fix**: Add validation and correction step after node restoration |
| 109 | +- **Implementation**: |
| 110 | + - Check if node size meets minimum requirements using `calculate_absolute_minimum_size()` |
| 111 | + - Verify pin positions are within node bounds |
| 112 | + - Validate GUI content fits within allocated space |
| 113 | + - Force layout recalculation if validation fails |
| 114 | + - Apply minimum size corrections if necessary |
| 115 | + |
| 116 | +### Phase 3: Proactive Layout Management |
| 117 | + |
| 118 | +#### Task 3.1: Add Layout Refresh Method |
| 119 | +**Location**: `src/node.py` |
| 120 | +- **Issue**: No centralized way to force complete layout refresh |
| 121 | +- **Fix**: Create comprehensive refresh method |
| 122 | +- **Implementation**: |
| 123 | + - Add `refresh_layout()` method to Node class |
| 124 | + - Include pin positioning, size validation, and visual updates |
| 125 | + - Call from critical points: after undo, after loading, after code changes |
| 126 | + - Incorporate minimum size validation using `calculate_absolute_minimum_size()` |
| 127 | + - Auto-correct size if it's below minimum requirements |
| 128 | + |
| 129 | +#### Task 3.2: Improve Content Widget Sizing |
| 130 | +**Location**: `src/node.py` `_update_layout()` method |
| 131 | +- **Issue**: Proxy widget sizing logic is fragile (lines 256-264) |
| 132 | +- **Fix**: Make widget sizing more robust |
| 133 | +- **Implementation**: |
| 134 | + - Calculate content area more precisely |
| 135 | + - Add minimum content height enforcement |
| 136 | + - Handle edge cases where content is larger than available space |
| 137 | + |
| 138 | +### Phase 4: Integration and Testing |
| 139 | + |
| 140 | +#### Task 4.1: Integration Testing |
| 141 | +- **Goal**: Ensure all components work together correctly |
| 142 | +- **Tests**: |
| 143 | + - Create node → delete → undo sequence |
| 144 | + - Load graphs with small node sizes |
| 145 | + - Resize nodes with different content types |
| 146 | + - Test with nodes containing GUI elements |
| 147 | + |
| 148 | +#### Task 4.2: Performance Optimization |
| 149 | +- **Goal**: Ensure layout updates don't impact performance |
| 150 | +- **Implementation**: |
| 151 | + - Batch layout updates when possible |
| 152 | + - Avoid redundant calculations |
| 153 | + - Use lazy evaluation for expensive operations |
| 154 | + |
| 155 | +## Implementation Priority |
| 156 | + |
| 157 | +### High Priority (Fix Immediately) |
| 158 | +1. **Task 1.4**: Comprehensive Minimum Size Calculation System |
| 159 | +2. **Task 2.1**: Add Minimum Size Validation on Node Loading |
| 160 | +3. **Task 1.2**: Enhanced Node Layout Update Process |
| 161 | +4. **Task 2.2**: Improved Node Restoration Process |
| 162 | + |
| 163 | +### Medium Priority (Fix Soon) |
| 164 | +5. **Task 1.1**: Pin Position Update Mechanism |
| 165 | +6. **Task 1.3**: Minimum Size Enforcement |
| 166 | +7. **Task 3.1**: Layout Refresh Method |
| 167 | +8. **Task 2.3**: Post-Restoration Validation |
| 168 | + |
| 169 | +### Low Priority (Quality of Life) |
| 170 | +9. **Task 3.2**: Content Widget Sizing Improvements |
| 171 | +10. **Task 4.2**: Performance Optimization |
| 172 | + |
| 173 | +## Expected Outcomes |
| 174 | + |
| 175 | +### Bug Resolution |
| 176 | +- Pins will position correctly immediately after undo operations |
| 177 | +- Nodes will maintain proper minimum size during all operations |
| 178 | +- GUI elements will never be crushed or compressed |
| 179 | +- Nodes loaded from files will automatically resize to minimum requirements if saved too small |
| 180 | +- Comprehensive minimum size calculation prevents layout issues across all node types |
| 181 | + |
| 182 | +### Code Quality Improvements |
| 183 | +- More robust layout calculation system |
| 184 | +- Better separation of concerns between layout and visual updates |
| 185 | +- Improved error handling and validation |
| 186 | + |
| 187 | +### User Experience |
| 188 | +- Eliminated need for manual node resizing to fix layout |
| 189 | +- Consistent node appearance across all operations |
| 190 | +- More reliable undo/redo functionality |
| 191 | + |
| 192 | +## Technical Implementation Details |
| 193 | + |
| 194 | +### Minimum Size Calculation Algorithm |
| 195 | +The `calculate_absolute_minimum_size()` method should implement the following logic: |
| 196 | + |
| 197 | +```python |
| 198 | +def calculate_absolute_minimum_size(self) -> tuple[int, int]: |
| 199 | + """Calculate the absolute minimum size needed for this node's content.""" |
| 200 | + |
| 201 | + # Base measurements |
| 202 | + title_height = 32 |
| 203 | + pin_spacing = 25 |
| 204 | + pin_margin_top = 15 |
| 205 | + node_padding = 10 |
| 206 | + resize_handle_size = 15 |
| 207 | + |
| 208 | + # Calculate minimum width |
| 209 | + title_width = self._title_item.boundingRect().width() + 20 # Title + padding |
| 210 | + |
| 211 | + # Pin label widths (find longest on each side) |
| 212 | + max_input_label_width = max([pin.label.boundingRect().width() |
| 213 | + for pin in self.input_pins] or [0]) |
| 214 | + max_output_label_width = max([pin.label.boundingRect().width() |
| 215 | + for pin in self.output_pins] or [0]) |
| 216 | + |
| 217 | + pin_label_width = max_input_label_width + max_output_label_width + 40 # Labels + pin spacing |
| 218 | + |
| 219 | + # GUI content minimum width |
| 220 | + gui_min_width = 0 |
| 221 | + if self.content_container: |
| 222 | + gui_min_width = self.content_container.minimumSizeHint().width() |
| 223 | + |
| 224 | + min_width = max( |
| 225 | + self.base_width, # Default base width |
| 226 | + title_width, |
| 227 | + pin_label_width, |
| 228 | + gui_min_width + node_padding |
| 229 | + ) |
| 230 | + |
| 231 | + # Calculate minimum height |
| 232 | + max_pins = max(len(self.input_pins), len(self.output_pins)) |
| 233 | + pin_area_height = (max_pins * pin_spacing) if max_pins > 0 else 0 |
| 234 | + |
| 235 | + # GUI content minimum height |
| 236 | + gui_min_height = 0 |
| 237 | + if self.content_container: |
| 238 | + gui_min_height = self.content_container.minimumSizeHint().height() |
| 239 | + |
| 240 | + min_height = (title_height + |
| 241 | + pin_margin_top + |
| 242 | + pin_area_height + |
| 243 | + gui_min_height + |
| 244 | + resize_handle_size + |
| 245 | + node_padding) |
| 246 | + |
| 247 | + return (min_width, min_height) |
| 248 | +``` |
| 249 | + |
| 250 | +### Load-Time Size Validation |
| 251 | +During node loading, implement this validation: |
| 252 | + |
| 253 | +```python |
| 254 | +def validate_and_correct_node_size(node_data: dict) -> dict: |
| 255 | + """Validate node size against minimum requirements and correct if needed.""" |
| 256 | + |
| 257 | + # Create temporary node to calculate minimum size |
| 258 | + temp_node = create_node_from_data(node_data) |
| 259 | + min_width, min_height = temp_node.calculate_absolute_minimum_size() |
| 260 | + |
| 261 | + loaded_width = node_data.get('size', [200, 150])[0] |
| 262 | + loaded_height = node_data.get('size', [200, 150])[1] |
| 263 | + |
| 264 | + corrected_width = max(loaded_width, min_width) |
| 265 | + corrected_height = max(loaded_height, min_height) |
| 266 | + |
| 267 | + if corrected_width != loaded_width or corrected_height != loaded_height: |
| 268 | + print(f"Node '{node_data['title']}' size corrected from " |
| 269 | + f"{loaded_width}x{loaded_height} to {corrected_width}x{corrected_height}") |
| 270 | + node_data['size'] = [corrected_width, corrected_height] |
| 271 | + |
| 272 | + return node_data |
| 273 | +``` |
| 274 | + |
| 275 | +## Implementation Notes |
| 276 | + |
| 277 | +### Code Patterns to Follow |
| 278 | +- Always call `prepareGeometryChange()` before modifying positions/sizes |
| 279 | +- Use consistent method naming: `update_*()` for calculations, `refresh_*()` for visual updates |
| 280 | +- Include proper error handling and fallback behavior |
| 281 | +- Follow existing code style and commenting patterns |
| 282 | + |
| 283 | +### Debugging Strategy |
| 284 | +**Important**: These issues are highly dependent on GUI rendering, Qt layout systems, and real-time visual updates. Traditional unit tests are insufficient for debugging these problems. |
| 285 | + |
| 286 | +#### Primary Debugging Approach: Debug Print Statements |
| 287 | +- **Add comprehensive debug prints** throughout the layout and sizing methods |
| 288 | +- Focus on key methods: |
| 289 | + - `Node._update_layout()` - track pin positioning calculations |
| 290 | + - `Node.calculate_absolute_minimum_size()` - verify size calculations |
| 291 | + - `Node.fit_size_to_content()` - monitor size adjustments |
| 292 | + - `Pin.update_label_pos()` - track pin position updates |
| 293 | + - `DeleteNodeCommand.undo()` - monitor restoration sequence |
| 294 | + |
| 295 | +#### Debug Print Examples |
| 296 | +```python |
| 297 | +def _update_layout(self): |
| 298 | + print(f"DEBUG: _update_layout() called for node '{self.title}'") |
| 299 | + print(f"DEBUG: Current size: {self.width}x{self.height}") |
| 300 | + print(f"DEBUG: Pin counts - input: {len(self.input_pins)}, output: {len(self.output_pins)}") |
| 301 | + |
| 302 | + # ... existing layout code ... |
| 303 | + |
| 304 | + for i, pin in enumerate(self.input_pins): |
| 305 | + print(f"DEBUG: Input pin {i} positioned at {pin.pos()}") |
| 306 | + |
| 307 | + print(f"DEBUG: _update_layout() completed") |
| 308 | +``` |
| 309 | + |
| 310 | +#### Strategic Debug Points |
| 311 | +1. **Size Validation Points**: |
| 312 | + - Before and after `fit_size_to_content()` |
| 313 | + - During node loading/restoration |
| 314 | + - When size constraints are applied |
| 315 | + |
| 316 | +2. **Pin Positioning Points**: |
| 317 | + - Before and after pin position calculations |
| 318 | + - During visual updates |
| 319 | + - After undo operations |
| 320 | + |
| 321 | +3. **Layout Trigger Points**: |
| 322 | + - When `_update_layout()` is called |
| 323 | + - During GUI widget creation/rebuilding |
| 324 | + - After property changes |
| 325 | + |
| 326 | +#### Live Testing Approach |
| 327 | +- Run the application with debug prints enabled |
| 328 | +- Perform the exact user scenario: create node → delete → undo |
| 329 | +- Monitor console output for layout sequence issues |
| 330 | +- Manually resize node to trigger correct layout, compare debug output |
| 331 | +- Use debug prints to identify where the layout chain breaks |
| 332 | + |
| 333 | +#### Debug Print Management |
| 334 | +- **During Development**: Use extensive debug prints to trace execution flow |
| 335 | +- **Conditional Debugging**: Consider using a debug flag to enable/disable prints |
| 336 | +```python |
| 337 | +DEBUG_LAYOUT = True # Set to False for production |
| 338 | + |
| 339 | +def _update_layout(self): |
| 340 | + if DEBUG_LAYOUT: |
| 341 | + print(f"DEBUG: _update_layout() called for node '{self.title}'") |
| 342 | + # ... rest of method |
| 343 | +``` |
| 344 | +- **Post-Fix Cleanup**: Remove or disable debug prints once issues are resolved |
| 345 | +- **Keep Key Diagnostics**: Retain essential debug prints that could help with future issues |
| 346 | + |
| 347 | +### Testing Strategy (Secondary) |
| 348 | +While debug prints are primary, these tests support the debugging process: |
| 349 | +- Create unit tests for layout calculation methods (pure calculation testing) |
| 350 | +- Add integration tests for undo/redo scenarios |
| 351 | +- Include visual regression tests for node appearance |
| 352 | +- Test with various node types: code-only, GUI-enabled, different sizes |
| 353 | +- **New minimum size tests**: |
| 354 | + - Test `calculate_absolute_minimum_size()` with various content types |
| 355 | + - Load graphs with intentionally small node sizes and verify auto-correction |
| 356 | + - Test nodes with complex GUI content (many widgets, large content) |
| 357 | + - Verify minimum size calculations with different pin configurations |
| 358 | + - Test edge cases: no pins, many pins, long pin labels, wide titles |
| 359 | + |
| 360 | +## Maintenance Considerations |
| 361 | + |
| 362 | +This plan addresses both immediate bugs and underlying architectural issues that could cause similar problems in the future. The proposed changes create a more robust foundation for node layout management while maintaining backward compatibility with existing functionality. |
| 363 | + |
| 364 | +Regular testing of the undo/redo system and node layout should be performed, especially when making changes to the node system, pin system, or command system. |
0 commit comments