Refactor src/connection.rs: exceeds 400-line coding guideline

[coderabbitai]

Issue Description

The file src/connection.rs currently contains 992 lines, significantly exceeding the 400-line threshold specified in the coding guidelines.

Requested Action

Decompose this module into smaller, domain-focused submodules in a follow-up PR. Suggested areas for extraction:

• ActorState
• MultiPacketContext
• Event handling

Context

This issue was identified during the review of PR #403.

Related PR: #403
Requested by: @leynos

[traycerai]

Plan

Observations

The file:src/connection.rs module contains 992 lines, significantly exceeding the 400-line coding guideline. The file already has a submodule structure with file:src/connection/test_support.rs, indicating the module is partially set up for directory-based organization. The codebase follows a consistent pattern where larger modules use a mod.rs file with separate files for domain-focused components (e.g., file:src/app/mod.rs, file:src/fragment/mod.rs, file:src/push/mod.rs). The issue was identified during PR #403 review and suggests extracting ActorState, MultiPacketContext, and Event handling into separate submodules.

Approach

Convert file:src/connection.rs to a directory-based module structure by creating file:src/connection/mod.rs and extracting three domain-focused submodules as suggested. The refactoring will maintain all public APIs unchanged while organizing private implementation details into cohesive units. Each extracted submodule will focus on a single responsibility: state management, multi-packet handling, and event types. The existing file:src/connection/test_support.rs will be updated to import from the new submodules. This approach follows the established patterns in file:src/app/, file:src/fragment/, and file:src/push/ modules.

Implementation Steps

1. Create Directory Structure

Create the new module structure by renaming and organizing files:

• Rename file:src/connection.rs to file:src/connection/mod.rs
• The file:src/connection/test_support.rs file already exists and will remain

2. Extract ActorState Module

Create file:src/connection/state.rs containing:

Components to move:

• RunState enum (lines 918-925 from original file)
• ActorState struct and its implementation (lines 927-988)

Module structure:

// Private enum and struct
enum RunState { Active, ShuttingDown, Finished }
struct ActorState { ... }
impl ActorState { ... }

Visibility: Keep all items private (no pub keyword) as they are internal implementation details used only within the connection module and by test_support.

3. Extract MultiPacketContext Module

Create file:src/connection/multi_packet.rs containing:

Components to move:

• MultiPacketStamp enum (lines 163-169)
• MultiPacketTerminationReason enum and its implementations (lines 175-197)
• MultiPacketContext<F> struct and its implementation (lines 199-243)

Module structure:

// Private enums and struct
enum MultiPacketStamp { Disabled, Enabled(Option<u64>) }
enum MultiPacketTerminationReason { Drained, Disconnected, Shutdown }
impl MultiPacketTerminationReason { ... }
impl fmt::Display for MultiPacketTerminationReason { ... }
struct MultiPacketContext<F> { ... }
impl<F> MultiPacketContext<F> { ... }

Visibility: Keep all items private as they are internal to the connection actor implementation.

4. Extract Event Module

Create file:src/connection/event.rs containing:

Components to move:

• Event<F, E> enum (lines 68-81)

Module structure:

use crate::response::WireframeError;

// Private enum
enum Event<F, E> {
    Shutdown,
    High(Option<F>),
    Low(Option<F>),
    MultiPacket(Option<F>),
    Response(Option<Result<F, WireframeError<E>>>),
    Idle,
}

Visibility: Keep private as it's an internal event type for the actor's select loop.

5. Update mod.rs File

Organize file:src/connection/mod.rs with the following structure:

Module declarations (at the top after imports):

mod state;
mod multi_packet;
mod event;

#[cfg(not(loom))]
pub mod test_support;

Use statements to import from submodules:

use state::{ActorState, RunState};
use multi_packet::{MultiPacketContext, MultiPacketStamp, MultiPacketTerminationReason};
use event::Event;

Remaining content in mod.rs:

• Module-level documentation
• Standard library and external crate imports
• ACTIVE_CONNECTIONS static and ActiveConnection RAII guard (lines 26-46)
• active_connection_count() public function (lines 48-51)
• FairnessConfig public struct (lines 83-117)
• ConnectionChannels<F> public struct (lines 98-108)
• ConnectionActor<F, E> public struct and all its implementations (lines 119-915)
• DrainContext<'a, F> private struct (lines 154-158)
• QueueKind private enum (lines 245-251)

6. Update test_support.rs Imports

Modify file:src/connection/test_support.rs to import from the new submodules:

Change the import statement from:

use super::{
    ActorState,
    ConnectionActor,
    ConnectionChannels,
    DrainContext,
    MultiPacketTerminationReason,
    ProtocolHooks,
    QueueKind,
};

To:

use super::{
    ConnectionActor,
    ConnectionChannels,
    DrainContext,
    QueueKind,
    state::ActorState,
    multi_packet::MultiPacketTerminationReason,
};

Note: ProtocolHooks is imported from crate::hooks, not from the connection module, so it remains unchanged.

7. Verify Public API Compatibility

Ensure the public API remains unchanged:

Public exports in file:src/lib.rs:

• pub use connection::ConnectionActor; - remains unchanged
• No other connection module items are re-exported at the crate level

Public items in connection module:

• active_connection_count() function - remains public
• FairnessConfig struct - remains public
• ConnectionChannels<F> struct - remains public
• ConnectionActor<F, E> struct - remains public
• test_support module - remains public under cfg(not(loom))

8. Update Internal Cross-References

Review and update any internal documentation links or references:

• Update rustdoc links in file:src/connection/mod.rs that reference moved types
• Ensure next_event documentation correctly references the Event enum
• Verify that all method documentation remains accurate after the refactoring

9. File Size Verification

After refactoring, verify the file sizes:

Expected breakdown:

• file:src/connection/mod.rs: ~700-750 lines (main actor implementation)
• file:src/connection/state.rs: ~70 lines (state machine)
• file:src/connection/multi_packet.rs: ~85 lines (multi-packet context)
• file:src/connection/event.rs: ~20 lines (event enum)
• file:src/connection/test_support.rs: ~260 lines (existing, with updated imports)

Total: Still ~1135 lines across all files, but each file is now under 800 lines, with the main mod.rs significantly reduced from 992 lines.

Module Organization Diagram

graph TD
    A[src/connection/mod.rs<br/>~750 lines] --> B[src/connection/state.rs<br/>~70 lines]
    A --> C[src/connection/multi_packet.rs<br/>~85 lines]
    A --> D[src/connection/event.rs<br/>~20 lines]
    A --> E[src/connection/test_support.rs<br/>~260 lines]
    
    B --> F[RunState enum<br/>ActorState struct]
    C --> G[MultiPacketStamp enum<br/>MultiPacketTerminationReason enum<br/>MultiPacketContext struct]
    D --> H[Event enum]
    E --> I[Test helpers<br/>ActorHarness<br/>ActorStateHarness]
    
    style A fill:#e1f5ff
    style B fill:#fff4e1
    style C fill:#fff4e1
    style D fill:#fff4e1
    style E fill:#f0f0f0

Loading

Component Relationships

sequenceDiagram
    participant M as mod.rs<br/>(ConnectionActor)
    participant S as state.rs<br/>(ActorState)
    participant MP as multi_packet.rs<br/>(MultiPacketContext)
    participant E as event.rs<br/>(Event)
    
    M->>E: next_event() returns Event
    M->>S: Check state.is_done()
    M->>MP: multi_packet.is_active()
    E->>M: Event::MultiPacket
    M->>MP: handle_multi_packet_closed()
    MP->>S: state.mark_closed()
    M->>S: state.start_shutdown()

Loading

Benefits

Aspect	Before	After
Main file size	992 lines	~750 lines
Largest file	992 lines	~750 lines
Module organization	Single file	5 focused files
State management	Mixed with actor	Isolated in state.rs
Multi-packet logic	Mixed with actor	Isolated in multi_packet.rs
Event types	Mixed with actor	Isolated in event.rs
Maintainability	Low (large file)	High (focused modules)
Testability	Moderate	High (clear boundaries)

Import In IDE

🤖 Prompt for AI Agents

## Observations

The `file:src/connection.rs` module contains 992 lines, significantly exceeding the 400-line coding guideline. The file already has a submodule structure with `file:src/connection/test_support.rs`, indicating the module is partially set up for directory-based organization. The codebase follows a consistent pattern where larger modules use a `mod.rs` file with separate files for domain-focused components (e.g., `file:src/app/mod.rs`, `file:src/fragment/mod.rs`, `file:src/push/mod.rs`). The issue was identified during PR #403 review and suggests extracting ActorState, MultiPacketContext, and Event handling into separate submodules.

## Approach

Convert `file:src/connection.rs` to a directory-based module structure by creating `file:src/connection/mod.rs` and extracting three domain-focused submodules as suggested. The refactoring will maintain all public APIs unchanged while organizing private implementation details into cohesive units. Each extracted submodule will focus on a single responsibility: state management, multi-packet handling, and event types. The existing `file:src/connection/test_support.rs` will be updated to import from the new submodules. This approach follows the established patterns in `file:src/app/`, `file:src/fragment/`, and `file:src/push/` modules.

## Implementation Steps

### 1. Create Directory Structure

Create the new module structure by renaming and organizing files:

- Rename `file:src/connection.rs` to `file:src/connection/mod.rs`
- The `file:src/connection/test_support.rs` file already exists and will remain

### 2. Extract ActorState Module

Create `file:src/connection/state.rs` containing:

**Components to move:**
- `RunState` enum (lines 918-925 from original file)
- `ActorState` struct and its implementation (lines 927-988)

**Module structure:**
```rust
// Private enum and struct
enum RunState { Active, ShuttingDown, Finished }
struct ActorState { ... }
impl ActorState { ... }
```

**Visibility:** Keep all items private (no `pub` keyword) as they are internal implementation details used only within the connection module and by test_support.

### 3. Extract MultiPacketContext Module

Create `file:src/connection/multi_packet.rs` containing:

**Components to move:**
- `MultiPacketStamp` enum (lines 163-169)
- `MultiPacketTerminationReason` enum and its implementations (lines 175-197)
- `MultiPacketContext<F>` struct and its implementation (lines 199-243)

**Module structure:**
```rust
// Private enums and struct
enum MultiPacketStamp { Disabled, Enabled(Option<u64>) }
enum MultiPacketTerminationReason { Drained, Disconnected, Shutdown }
impl MultiPacketTerminationReason { ... }
impl fmt::Display for MultiPacketTerminationReason { ... }
struct MultiPacketContext<F> { ... }
impl<F> MultiPacketContext<F> { ... }
```

**Visibility:** Keep all items private as they are internal to the connection actor implementation.

### 4. Extract Event Module

Create `file:src/connection/event.rs` containing:

**Components to move:**
- `Event<F, E>` enum (lines 68-81)

**Module structure:**
```rust
use crate::response::WireframeError;

// Private enum
enum Event<F, E> {
    Shutdown,
    High(Option<F>),
    Low(Option<F>),
    MultiPacket(Option<F>),
    Response(Option<Result<F, WireframeError<E>>>),
    Idle,
}
```

**Visibility:** Keep private as it's an internal event type for the actor's select loop.

### 5. Update mod.rs File

Organize `file:src/connection/mod.rs` with the following structure:

**Module declarations (at the top after imports):**
```rust
mod state;
mod multi_packet;
mod event;

#[cfg(not(loom))]
pub mod test_support;
```

**Use statements to import from submodules:**
```rust
use state::{ActorState, RunState};
use multi_packet::{MultiPacketContext, MultiPacketStamp, MultiPacketTerminationReason};
use event::Event;
```

**Remaining content in mod.rs:**
- Module-level documentation
- Standard library and external crate imports
- `ACTIVE_CONNECTIONS` static and `ActiveConnection` RAII guard (lines 26-46)
- `active_connection_count()` public function (lines 48-51)
- `FairnessConfig` public struct (lines 83-117)
- `ConnectionChannels<F>` public struct (lines 98-108)
- `ConnectionActor<F, E>` public struct and all its implementations (lines 119-915)
- `DrainContext<'a, F>` private struct (lines 154-158)
- `QueueKind` private enum (lines 245-251)

### 6. Update test_support.rs Imports

Modify `file:src/connection/test_support.rs` to import from the new submodules:

**Change the import statement from:**
```rust
use super::{
    ActorState,
    ConnectionActor,
    ConnectionChannels,
    DrainContext,
    MultiPacketTerminationReason,
    ProtocolHooks,
    QueueKind,
};
```

**To:**
```rust
use super::{
    ConnectionActor,
    ConnectionChannels,
    DrainContext,
    QueueKind,
    state::ActorState,
    multi_packet::MultiPacketTerminationReason,
};
```

**Note:** `ProtocolHooks` is imported from `crate::hooks`, not from the connection module, so it remains unchanged.

### 7. Verify Public API Compatibility

Ensure the public API remains unchanged:

**Public exports in `file:src/lib.rs`:**
- `pub use connection::ConnectionActor;` - remains unchanged
- No other connection module items are re-exported at the crate level

**Public items in connection module:**
- `active_connection_count()` function - remains public
- `FairnessConfig` struct - remains public
- `ConnectionChannels<F>` struct - remains public  
- `ConnectionActor<F, E>` struct - remains public
- `test_support` module - remains public under `cfg(not(loom))`

### 8. Update Internal Cross-References

Review and update any internal documentation links or references:

- Update rustdoc links in `file:src/connection/mod.rs` that reference moved types
- Ensure `next_event` documentation correctly references the `Event` enum
- Verify that all method documentation remains accurate after the refactoring

### 9. File Size Verification

After refactoring, verify the file sizes:

**Expected breakdown:**
- `file:src/connection/mod.rs`: ~700-750 lines (main actor implementation)
- `file:src/connection/state.rs`: ~70 lines (state machine)
- `file:src/connection/multi_packet.rs`: ~85 lines (multi-packet context)
- `file:src/connection/event.rs`: ~20 lines (event enum)
- `file:src/connection/test_support.rs`: ~260 lines (existing, with updated imports)

**Total:** Still ~1135 lines across all files, but each file is now under 800 lines, with the main mod.rs significantly reduced from 992 lines.

## Module Organization Diagram

```mermaid
graph TD
    A[src/connection/mod.rs<br/>~750 lines] --> B[src/connection/state.rs<br/>~70 lines]
    A --> C[src/connection/multi_packet.rs<br/>~85 lines]
    A --> D[src/connection/event.rs<br/>~20 lines]
    A --> E[src/connection/test_support.rs<br/>~260 lines]
    
    B --> F[RunState enum<br/>ActorState struct]
    C --> G[MultiPacketStamp enum<br/>MultiPacketTerminationReason enum<br/>MultiPacketContext struct]
    D --> H[Event enum]
    E --> I[Test helpers<br/>ActorHarness<br/>ActorStateHarness]
    
    style A fill:#e1f5ff
    style B fill:#fff4e1
    style C fill:#fff4e1
    style D fill:#fff4e1
    style E fill:#f0f0f0
```

## Component Relationships

```mermaid
sequenceDiagram
    participant M as mod.rs<br/>(ConnectionActor)
    participant S as state.rs<br/>(ActorState)
    participant MP as multi_packet.rs<br/>(MultiPacketContext)
    participant E as event.rs<br/>(Event)
    
    M->>E: next_event() returns Event
    M->>S: Check state.is_done()
    M->>MP: multi_packet.is_active()
    E->>M: Event::MultiPacket
    M->>MP: handle_multi_packet_closed()
    MP->>S: state.mark_closed()
    M->>S: state.start_shutdown()
```

## Benefits

| Aspect | Before | After |
|--------|--------|-------|
| **Main file size** | 992 lines | ~750 lines |
| **Largest file** | 992 lines | ~750 lines |
| **Module organization** | Single file | 5 focused files |
| **State management** | Mixed with actor | Isolated in `state.rs` |
| **Multi-packet logic** | Mixed with actor | Isolated in `multi_packet.rs` |
| **Event types** | Mixed with actor | Isolated in `event.rs` |
| **Maintainability** | Low (large file) | High (focused modules) |
| **Testability** | Moderate | High (clear boundaries) |

---

Execution Information

Branch: main
Commit: 920257e

---

💡 Tips

Supported Commands (Inside Comments)

• Use @traycerai generate to iterate on the previous version of the implementation plan.

Supported Commands (Inside Description)

• Add @traycerai ignore anywhere in the ticket description to prevent this ticket from being processed.
• Add @traycerai branch:<branch-name> anywhere in the ticket description to specify the target branch for the implementation plan.

Community

• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.