|
| 1 | +# Snapshot Module Redesign Proposal |
| 2 | + |
| 3 | +**Date**: March 2, 2025 |
| 4 | +**Author**: Development Team |
| 5 | +**Status**: Draft Proposal |
| 6 | + |
| 7 | +## Executive Summary |
| 8 | + |
| 9 | +This document proposes refactoring the current monolithic `snapshot.py` module (approximately 650 lines) into a structured package to improve maintainability, testability, and extensibility. The primary goal is to separate concerns, reduce file size, and establish a clear API boundary while maintaining backward compatibility. |
| 10 | + |
| 11 | +## Current State Analysis |
| 12 | + |
| 13 | +### Structure |
| 14 | + |
| 15 | +The current `snapshot.py` module contains: |
| 16 | + |
| 17 | +- 4 base classes with sealable mixin functionality |
| 18 | +- 4 concrete snapshot classes (Server, Session, Window, Pane) |
| 19 | +- Several utility functions for filtering and transformations |
| 20 | +- Type definitions and aliases |
| 21 | +- Complex inter-dependencies between classes |
| 22 | + |
| 23 | +### Pain Points |
| 24 | + |
| 25 | +1. **Size**: The file is large (~650 lines) and challenging to navigate |
| 26 | +2. **Tight coupling**: Classes reference each other directly, creating complex dependencies |
| 27 | +3. **Mixed concerns**: Type definitions, base classes, implementations, and utilities are intermingled |
| 28 | +4. **Testing complexity**: Testing specific components requires loading the entire module |
| 29 | +5. **Future maintenance**: Adding new features or making changes affects the entire module |
| 30 | + |
| 31 | +## Proposed Structure |
| 32 | + |
| 33 | +We propose refactoring into a dedicated package with this structure: |
| 34 | + |
| 35 | +``` |
| 36 | +src/libtmux/snapshot/ |
| 37 | +├── __init__.py # Bare init, documenting package |
| 38 | +├── base.py # Base classes with Sealable mixins |
| 39 | +├── types.py # Type definitions, exports, and annotations |
| 40 | +├── models/ |
| 41 | +│ ├── __init__.py # Exports all models |
| 42 | +│ ├── pane.py # PaneSnapshot implementation |
| 43 | +│ ├── window.py # WindowSnapshot implementation |
| 44 | +│ ├── session.py # SessionSnapshot implementation |
| 45 | +│ └── server.py # ServerSnapshot implementation |
| 46 | +└── utils.py # Utility functions (filter_snapshot, snapshot_to_dict, etc.) |
| 47 | +``` |
| 48 | + |
| 49 | +### Key Components |
| 50 | + |
| 51 | +1. **`__init__.py`**: Declare module |
| 52 | + |
| 53 | +2. **`types.py`**: Centralizes all type definitions |
| 54 | + ```python |
| 55 | + from __future__ import annotations |
| 56 | + |
| 57 | + import typing as t |
| 58 | + |
| 59 | + from libtmux.pane import Pane |
| 60 | + from libtmux.server import Server |
| 61 | + from libtmux.session import Session |
| 62 | + from libtmux.window import Window |
| 63 | + |
| 64 | + # Type variables for generic typing |
| 65 | + PaneT = t.TypeVar("PaneT", bound=Pane, covariant=True) |
| 66 | + WindowT = t.TypeVar("WindowT", bound=Window, covariant=True) |
| 67 | + SessionT = t.TypeVar("SessionT", bound=Session, covariant=True) |
| 68 | + ServerT = t.TypeVar("ServerT", bound=Server, covariant=True) |
| 69 | + |
| 70 | + # Forward references for snapshot classes |
| 71 | + if t.TYPE_CHECKING: |
| 72 | + from libtmux.snapshot.models.pane import PaneSnapshot |
| 73 | + from libtmux.snapshot.models.window import WindowSnapshot |
| 74 | + from libtmux.snapshot.models.session import SessionSnapshot |
| 75 | + from libtmux.snapshot.models.server import ServerSnapshot |
| 76 | + |
| 77 | + # Union type for snapshot classes |
| 78 | + SnapshotType = t.Union[ServerSnapshot, SessionSnapshot, WindowSnapshot, PaneSnapshot] |
| 79 | + else: |
| 80 | + # Runtime placeholder - will be properly defined after imports |
| 81 | + SnapshotType = t.Any |
| 82 | + ``` |
| 83 | + |
| 84 | +3. **`base.py`**: Base classes that implement sealable behavior |
| 85 | + ```python |
| 86 | + from __future__ import annotations |
| 87 | + |
| 88 | + import typing as t |
| 89 | + |
| 90 | + from libtmux._internal.frozen_dataclass_sealable import Sealable |
| 91 | + from libtmux._internal.query_list import QueryList |
| 92 | + from libtmux.pane import Pane |
| 93 | + from libtmux.server import Server |
| 94 | + from libtmux.session import Session |
| 95 | + from libtmux.window import Window |
| 96 | + |
| 97 | + from libtmux.snapshot.types import PaneT, WindowT, SessionT, PaneT |
| 98 | + |
| 99 | + class SealablePaneBase(Pane, Sealable): |
| 100 | + """Base class for sealable pane classes.""" |
| 101 | + |
| 102 | + class SealableWindowBase(Window, Sealable, t.Generic[PaneT]): |
| 103 | + """Base class for sealable window classes with generic pane type.""" |
| 104 | + |
| 105 | + # Implementation of properties with proper typing |
| 106 | + |
| 107 | + class SealableSessionBase(Session, Sealable, t.Generic[WindowT, PaneT]): |
| 108 | + """Base class for sealable session classes with generic window and pane types.""" |
| 109 | + |
| 110 | + # Implementation of properties with proper typing |
| 111 | + |
| 112 | + class SealableServerBase(Server, Sealable, t.Generic[SessionT, WindowT, PaneT]): |
| 113 | + """Generic base for sealable server with typed session, window, and pane.""" |
| 114 | + |
| 115 | + # Implementation of properties with proper typing |
| 116 | + ``` |
| 117 | + |
| 118 | +4. **Model classes**: Individual implementations in separate files |
| 119 | + - Each file contains a single snapshot class with focused responsibility |
| 120 | + - Clear imports and dependencies between modules |
| 121 | + - Proper type annotations |
| 122 | + |
| 123 | +5. **`utils.py`**: Utility functions separated from model implementations |
| 124 | + ```python |
| 125 | + from __future__ import annotations |
| 126 | + |
| 127 | + import copy |
| 128 | + import datetime |
| 129 | + import typing as t |
| 130 | + |
| 131 | + from libtmux.snapshot.types import SnapshotType |
| 132 | + from libtmux.snapshot.models import ( |
| 133 | + ServerSnapshot, SessionSnapshot, WindowSnapshot, PaneSnapshot |
| 134 | + ) |
| 135 | + |
| 136 | + def filter_snapshot( |
| 137 | + snapshot: SnapshotType, |
| 138 | + filter_func: t.Callable[[SnapshotType], bool], |
| 139 | + ) -> SnapshotType | None: |
| 140 | + """Filter a snapshot hierarchy based on a filter function.""" |
| 141 | + # Implementation... |
| 142 | + |
| 143 | + def snapshot_to_dict( |
| 144 | + snapshot: SnapshotType | t.Any, |
| 145 | + ) -> dict[str, t.Any]: |
| 146 | + """Convert a snapshot to a dictionary, avoiding circular references.""" |
| 147 | + # Implementation... |
| 148 | + |
| 149 | + def snapshot_active_only( |
| 150 | + full_snapshot: ServerSnapshot, |
| 151 | + ) -> ServerSnapshot: |
| 152 | + """Return a filtered snapshot containing only active sessions, windows, and panes.""" |
| 153 | + # Implementation... |
| 154 | + ``` |
| 155 | + |
| 156 | +## Implementation Plan |
| 157 | + |
| 158 | +We propose a phased approach with the following steps: |
| 159 | + |
| 160 | +### Phase 1: Setup Package Structure (Week 1) |
| 161 | + |
| 162 | +1. Create the package directory structure |
| 163 | +2. Set up empty module files with appropriate imports |
| 164 | +3. Create the types.py module with all type definitions |
| 165 | +4. Draft the base.py module with base classes |
| 166 | + |
| 167 | +### Phase 2: Migrate Models (Week 2-3) |
| 168 | + |
| 169 | +1. Move PaneSnapshot to its own module |
| 170 | +2. Move WindowSnapshot to its own module |
| 171 | +3. Move SessionSnapshot to its own module |
| 172 | +4. Move ServerSnapshot to its own module |
| 173 | +5. Update imports and references between modules |
| 174 | + |
| 175 | +### Phase 3: Extract Utilities (Week 3) |
| 176 | + |
| 177 | +1. Move utility functions to utils.py |
| 178 | +2. Update imports and references |
| 179 | + |
| 180 | +### Phase 4: Testing and Finalization (Week 4) |
| 181 | + |
| 182 | +1. Add/update tests to verify all functionality works correctly |
| 183 | +2. Update documentation |
| 184 | +3. Final code review |
| 185 | +4. Merge to main branch |
| 186 | + |
| 187 | +## Benefits and Tradeoffs |
| 188 | + |
| 189 | +### Benefits |
| 190 | + |
| 191 | +1. **Improved maintainability**: Smaller, focused files with clear responsibilities |
| 192 | +2. **Better organization**: Separation of concerns between different components |
| 193 | +3. **Simplified testing**: Ability to test components in isolation |
| 194 | +4. **Enhanced discoverability**: Easier for new developers to understand the codebase |
| 195 | +5. **Clearer API boundary**: Explicit exports from __init__.py define the public API |
| 196 | +6. **Future extensibility**: Easier to add new snapshot types or modify existing ones |
| 197 | + |
| 198 | +### Tradeoffs |
| 199 | + |
| 200 | +1. **Initial effort**: Significant upfront work to refactor and test |
| 201 | +2. **Complexity**: More files to navigate and understand |
| 202 | +3. **Risk**: Potential for regressions during refactoring |
| 203 | +4. **Import overhead**: Slightly more complex import statements |
| 204 | +5. **Learning curve**: Team needs to adapt to the new structure |
| 205 | + |
| 206 | +## Backward Compatibility |
| 207 | + |
| 208 | +The proposed changes maintain backward compatibility through: |
| 209 | + |
| 210 | +1. **Identical public API**: All classes and functions re-exported from __init__.py |
| 211 | +2. **Same behavior**: No functional changes to how snapshots work |
| 212 | +3. **Same type definitions**: Type hints remain compatible with existing code |
| 213 | + |
| 214 | +## Success Metrics |
| 215 | + |
| 216 | +The success of this refactoring will be measured by: |
| 217 | + |
| 218 | +1. **Code coverage**: Maintain or improve current test coverage |
| 219 | +2. **File sizes**: No file should exceed 200 lines |
| 220 | +3. **Import clarity**: Clear and direct imports between modules |
| 221 | +4. **Maintainability**: Reduction in complexity metrics |
| 222 | +5. **Developer feedback**: Team survey on code navigability |
| 223 | + |
| 224 | +## Conclusion |
| 225 | + |
| 226 | +This redesign addresses the current maintainability issues with the snapshot module while preserving all functionality and backward compatibility. The modular approach will make future maintenance easier and allow for more focused testing. We recommend proceeding with this refactoring as outlined in the implementation plan. |
| 227 | + |
| 228 | +## Next Steps |
| 229 | + |
| 230 | +1. Review and finalize this proposal |
| 231 | +2. Create implementation tickets in the issue tracker |
| 232 | +3. Assign resources for implementation |
| 233 | +4. Schedule code reviews at each phase completion |
0 commit comments