Skip to content
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
notes(2025-03-02) Add architecture notes
  • Loading branch information
tony committed Apr 6, 2025
commit b26ed1d9f21a9989d9f12b06ee25b49ed88084e9
149 changes: 149 additions & 0 deletions notes/2025-03-02-architecture-plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
<think>Let me analyze the `snapshot.py` file for its architecture, typing, and implementation details, as well as identify any flaws or opportunities for improvement in its API.

First, let's understand what this module is doing:

1. This module implements a snapshot mechanism for tmux objects (Server, Session, Window, Pane)
2. It uses frozen_dataclass_sealable to create immutable snapshots of these objects
3. It provides utilities to filter snapshots, convert them to dictionaries, etc.

Key classes in this module:
- PaneSnapshot, WindowSnapshot, SessionSnapshot, ServerSnapshot - these create immutable snapshots of their respective tmux objects
- Each snapshot class inherits from both the respective tmux class (Pane, Window, etc.) and Sealable
- Each has a from_X factory method to create a snapshot from a live object

Let's identify any architectural or typing issues:

1. Type annotations: The module uses Python's typing system extensively, with TypeVars, Union types, etc.
2. Immutability pattern: It uses frozen_dataclass_sealable to implement immutability
3. Circular references: The snapshots can have circular references (e.g., a WindowSnapshot has SessionSnapshot which has the same WindowSnapshot)

Now, let's look for flaws or areas for improvement:

1. **Complex Inheritance**: Multiple inheritance from both tmux objects and Sealable might cause confusion
2. **Code Duplication**: The from_X factory methods have a lot of duplicated logic
3. **Excessive Type Annotations**: Some of the type annotations might be overly complex
4. **Error Handling**: There's a mix of error suppression and re-raising
5. **Memory Usage**: Snapshots might use a lot of memory, especially with capture_content=True

API simplification opportunities:

1. **Factory Method Consistency**: Consider simplifying the factory methods
2. **Context Manager**: Consider adding a context manager for creating and using snapshots
3. **Fluent Interface**: Consider making the filter and conversion methods chainable
4. **Simplify Type Annotations**: Make type annotations more consistent and simpler where possible
5. **Reduce Circular References**: Consider alternative designs to reduce circular references

Let's discuss each of these in detail in the response.</think># Analysis of Snapshot Architecture

After examining the `snapshot.py` module, I can provide an analysis of its architecture, typing approach, and potential areas for improvement.

## Overall Architecture

The module implements a hierarchical snapshot system for tmux objects with these key components:

1. Four snapshot classes that mirror the tmux object hierarchy:
- `ServerSnapshot`
- `SessionSnapshot`
- `WindowSnapshot`
- `PaneSnapshot`

2. Each class inherits from both:
- The corresponding tmux class (Server, Session, etc.)
- A `Sealable` base class to provide immutability

3. Utility functions for:
- Filtering snapshots (`filter_snapshot`)
- Converting to dictionaries (`snapshot_to_dict`)
- Creating active-only views (`snapshot_active_only`)

## Typing Approach

The module makes excellent use of Python's modern typing features:

- Type variables with covariance (`PaneT = t.TypeVar("PaneT", bound=Pane, covariant=True)`)
- Proper return type annotations with Union types
- Type checking guards (`if t.TYPE_CHECKING:`)
- Type casts for better type safety (`t.cast("ServerSnapshot", filtered)`)

## Strengths

1. **Immutability Pattern**: Using `frozen_dataclass_sealable` provides a robust way to create immutable snapshots
2. **Complete Object Graph**: Snapshots maintain the full hierarchy of relationships
3. **Type Safety**: Strong typing throughout the codebase

## Potential Flaws

1. **Complex Factory Methods**: The `from_X` methods contain complex logic for finding server references, with multiple fallback strategies:
```python
if source_server is None and window_snapshot is not None:
source_server = window_snapshot.server
# ...more fallbacks...
```

2. **Circular References**: The bi-directional references (window_snapshot -> session_snapshot -> window_snapshot) could create complexity for serialization and garbage collection.

3. **Error Handling Inconsistencies**: There's a mix of:
```python
with contextlib.suppress(Exception):
# silent failure
```
and explicit error raising:
```python
if source_server is None:
error_msg = "Cannot create snapshot..."
raise ValueError(error_msg)
```

4. **Memory Overhead**: Snapshots duplicate a lot of data, especially with `capture_content=True`.

5. **Attribute Access Pattern**: The code uses direct attribute access in some places but property methods in others.

## API Simplification Opportunities

1. **Unified Factory Method**: Consider a single entry point for snapshot creation:
```python
def create_snapshot(tmux_object, capture_content=False, depth=None):
"""Create a snapshot of a tmux object."""
# Dispatch to appropriate snapshot class based on type
```

2. **Context Manager Support**:
```python
@contextlib.contextmanager
def tmux_snapshot(server, capture_content=False):
"""Create a snapshot and yield it as a context manager."""
snapshot = ServerSnapshot.from_server(server, capture_content)
try:
yield snapshot
finally:
# Cleanup if needed
```

3. **Fluent Interface** for chaining operations:
```python
snapshot = (
ServerSnapshot.from_server(server)
.filter(lambda obj: obj.name.startswith("dev"))
.active_only()
.to_dict()
)
```

4. **Focused Snapshot Classes**: The snapshots try to maintain compatibility with their tmux counterparts but override behaviors. Consider a cleaner separation with composition instead of inheritance.

5. **More Targeted Snapshot Creation**: Allow for creating more targeted snapshots:
```python
# Only capturing active session/window/pane hierarchy
snapshot = create_snapshot(server, include='active')

# Capturing only specified sessions
snapshot = create_snapshot(server, include_sessions=['dev', 'prod'])
```

## Type Safety Improvements

1. The use of `type: ignore[override]` comments suggests there might be a better way to handle property overrides in the typing system. Consider using Protocol classes or structural typing.

2. The `filter_snapshot` function's return type is quite complex - it might be worth creating a generic type for this.

Overall, the module is well-designed but could benefit from some API simplifications to make it more intuitive for users while maintaining its strong typing and immutability guarantees.