Conversation
Reviewer's GuideThis PR refactors the connection actor’s event loop by moving polling and shutdown logic into dedicated async helper methods and consolidating event dispatch into a new handle_event function. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughRefactor the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ConnectionActor
participant ShutdownToken
participant PriorityQueue
participant ResponseStream
Client->>ConnectionActor: poll_sources()
ConnectionActor->>ConnectionActor: next_event(state)
alt Await shutdown
ConnectionActor->>ShutdownToken: await_shutdown()
ShutdownToken-->>ConnectionActor: Shutdown event
else Poll priority
ConnectionActor->>PriorityQueue: poll_priority()
PriorityQueue-->>ConnectionActor: Priority event
else Poll response
ConnectionActor->>ResponseStream: poll_response()
ResponseStream-->>ConnectionActor: Response event
end
ConnectionActor->>ConnectionActor: dispatch_event(event, state, out)
ConnectionActor-->>Client: Result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit Inference Engine (AGENTS.md)
Files:
⚙️ CodeRabbit Configuration File
Files:
🔇 Additional comments (9)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/connection.rs:486` </location>
<code_context>
+ async fn await_shutdown(token: CancellationToken) { Self::wait_shutdown(token).await; }
+
+ /// Poll the high-priority queue.
+ async fn poll_high_priority(rx: Option<&mut mpsc::Receiver<F>>) -> Option<F> {
+ Self::poll_optional(rx, Self::recv_push).await
+ }
+
</code_context>
<issue_to_address>
poll_high_priority and poll_low_priority are nearly identical; consider deduplication.
Since both functions wrap poll_optional with identical arguments, consider merging them into one with a priority parameter, unless there's a specific reason to keep them separate.
Suggested implementation:
```rust
/// Poll the priority queue.
async fn poll_priority(
rx_high: Option<&mut mpsc::Receiver<F>>,
rx_low: Option<&mut mpsc::Receiver<F>>,
high: bool,
) -> Option<F> {
if high {
Self::poll_optional(rx_high, Self::recv_push).await
} else {
Self::poll_optional(rx_low, Self::recv_push).await
}
}
```
```rust
res = Self::poll_priority(self.high_rx.as_mut(), self.low_rx.as_mut(), true), if high_available => Event::High(res),
```
1. If there is a `poll_low_priority` function, remove it and update any of its call sites to use `poll_priority(..., false)`.
2. If the function signatures or receiver types differ, adjust the parameters to match your actual codebase.
3. If you want to further generalize, you could use an enum for priority instead of a bool.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Extract polling helpers for shutdown and push queues to reduce complexity.
760f612 to
93d1a1f
Compare
Summary
next_eventby moving shutdown and queue polling to helpershandle_eventfunctionTesting
make fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_688bf57d7b7883229d7adce6ad9c1693
Summary by Sourcery
Refactor connection actor event polling to simplify next_event logic and centralize event dispatch
Enhancements: