Skip to content

Refactor ConnectionActor queue handling#229

Closed
leynos wants to merge 3 commits intomainfrom
codex/refactor-duplicate-logic-in-connectionactor
Closed

Refactor ConnectionActor queue handling#229
leynos wants to merge 3 commits intomainfrom
codex/refactor-duplicate-logic-in-connectionactor

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented Jul 31, 2025

Summary

  • extract common queue processing to process_push

Testing

  • make lint
  • make test

https://chatgpt.com/codex/tasks/task_e_688bc8736a9c8322ad21af8787ebebb0

Summary by Sourcery

Enhancements:

  • Unify high- and low-priority push queue processing into a single helper method using a Queue enum

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Jul 31, 2025

Reviewer's Guide

Extract and consolidate high-priority and low-priority queue handling into a single process_push method backed by a new Queue enum to reduce duplication in ConnectionActor.

Class diagram for refactored queue handling in ConnectionActor

classDiagram
    class ConnectionActor {
        - high_rx
        - low_rx
        + process_high(res: Option<F>, state: &mut ActorState, out: &mut Vec<F>)
        + process_low(res: Option<F>, state: &mut ActorState, out: &mut Vec<F>)
        + process_push(res: Option<F>, queue: Queue, state: &mut ActorState, out: &mut Vec<F>)
        + process_frame_common(frame, out)
        + after_high(out, state)
        + after_low()
        + reset_high_counter()
        + handle_closed_receiver(rx, state)
    }

    class Queue {
        <<enum>>
        High
        Low
    }

    ConnectionActor --> Queue
Loading

Class diagram for new Queue enum

classDiagram
    class Queue {
        <<enum>>
        High
        Low
    }
Loading

File-Level Changes

Change Details Files
Define a new Queue enum to distinguish high- and low-priority queues
  • Add #[derive(Clone, Copy)]
  • Declare enum Queue with variants High and Low
src/connection.rs
Extract common logic into a unified process_push method
  • Remove duplicated if-let handling from process_high and process_low
  • Implement process_push(frame, queue, state, out) with match over Option and Queue
  • Delegate process_high and process_low to call process_push
src/connection.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 31, 2025

Summary by CodeRabbit

  • Refactor

    • Reduced code duplication and improved clarity in connection handling by introducing a context struct and consolidating logic for processing frames.
    • Reformatted function and method bodies across multiple files to use multi-line blocks for improved readability.
    • Updated lint attributes for better documentation and clarity in example and test code.
  • Style

    • Applied consistent formatting and expanded lint attributes with reasons throughout the codebase.

Walkthrough

Refactor connection frame processing in src/connection.rs by introducing a ProcessContext struct to encapsulate mutable state and output vector references. Replace duplicated logic in process_high and process_low with a generic helper method process_push. Minor formatting and lint directive adjustments were applied in example and server files.

Changes

Cohort / File(s) Change Summary
Connection Frame Processing Refactor
src/connection.rs
Introduce ProcessContext struct to hold mutable references to ActorState and output vector. Add generic process_push method to unify frame processing logic for high and low priority queues. Refactor poll_sources, process_high, and process_low to use the new context and helper method. Minor formatting improvements.
Lint and Formatting Adjustments
examples/metadata_routing.rs
Change lint attribute on Pong struct from #[expect(dead_code)] to #[allow(dead_code)] with explanatory comment.
Formatting Refinements
src/push.rs
Reformat single-line expressions in PushHandle methods from_arc and downgrade to multi-line blocks. Rewrite conditional in bounded_with_rate_dlq to nested if let and if statements without logic change.
Formatting and Lint Updates
src/server.rs
Reformat worker_count method to multi-line block. Change pattern matching in process_stream to nested if let for clarity. Replace #[expect(dead_code)] with #[allow(dead_code)] on EmptyPreamble struct in tests.
Consistent Multi-line Formatting
src/middleware.rs
Convert all single-expression function bodies into multi-line blocks with explicit braces and line breaks. Expand #[allow] attributes with reasons and multi-line formatting. No logic changes.
Minor Formatting and Attribute Enhancements
examples/ping_pong.rs, wireframe_testing/src/helpers.rs
Add reason to lint allowance on ping_handler async function and run_app_with_frames async function. Reformat single-line expressions into multi-line blocks without logic changes.

Sequence Diagram(s)

sequenceDiagram
    participant ConnectionActor
    participant ProcessContext
    participant ActorState
    participant OutVec

    ConnectionActor->>ProcessContext: Create with mutable state and out vec
    ConnectionActor->>ConnectionActor: process_push(frame_opt, ctx, on_some, on_none)
    alt Frame present
        ConnectionActor->>ActorState: Process frame
        ConnectionActor->>OutVec: Append output frames
    else No frame (receiver closed)
        ConnectionActor->>ActorState: Handle closed receiver
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15 minutes

Possibly related PRs

  • Implement run state consolidation in connection actor #146: The main PR refactors the poll_sources, process_high, and process_low methods in src/connection.rs by introducing a ProcessContext struct and a generic process_push helper to consolidate duplicated logic, building directly on the refactored connection actor structure and run state consolidation introduced in the retrieved PR Implement run state consolidation in connection actor #146, which originally split the queue receivers and unified run state tracking; thus, the main PR is a continuation and refinement of the connection actor's frame processing logic established in the retrieved PR.
  • Simplify polling logic in ConnectionActor #171: The main PR refactors poll_sources and frame processing in ConnectionActor by introducing ProcessContext and a generic process_push method to reduce duplication, while the retrieved PR restructures poll_sources by splitting event polling into a separate next_event method and dispatching via an Event enum; both modify poll_sources and related processing but use different approaches without overlapping code elements or helper methods, so their changes are related by targeting the same functionality but are distinct in implementation details.
  • Refactor connection polling #157: The main PR refactors frame processing by introducing a ProcessContext struct and a generic process_push method to unify process_high and process_low, while the retrieved PR refactors connection polling by extracting polling logic into helper async functions and unifying frame processing with different helper methods; both modify process_high and process_low but use distinct approaches and helper abstractions, indicating related refactoring efforts on connection frame processing.

Poem

In connection’s heart, a context born,
To hold the state and frames adorn.
One method now to rule them all,
High or low, it heeds the call.
Clearer paths and less repeat,
Refactor’s rhythm, oh so neat! 🎶✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/refactor-duplicate-logic-in-connectionactor

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.

Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments

### Comment 1
<location> `src/connection.rs:69` </location>
<code_context>
 }

+#[derive(Clone, Copy)]
+enum Queue {
+    High,
+    Low,
</code_context>

<issue_to_address>
Consider replacing the Queue enum and match logic with a callback-based helper to simplify the code.

You can collapse the extra `Queue`‐enum + `match` entirely by turning your helper into a small “run one of two callbacks” routine.  This lets you keep the `process_high`/`process_low` entry points very simple, get rid of `Queue`, and preserve *exactly* the same behavior:

```rust
impl YourActor<F, E> {
    // helper that just picks one of two callbacks instead of matching on Queue
    fn process_push<OnSome, OnNone>(
        &mut self,
        res: Option<F>,
        state: &mut ActorState,
        out: &mut Vec<F>,
        on_some: OnSome,
        on_none: OnNone,
    )
    where
        OnSome: FnOnce(&mut Self, &mut Vec<F>, &mut ActorState),
        OnNone: FnOnce(&mut Self, &mut ActorState),
    {
        if let Some(frame) = res {
            self.process_frame_common(frame, out);
            on_some(self, out, state);
        } else {
            on_none(self, state);
        }
    }

    fn process_high(&mut self, res: Option<F>, state: &mut ActorState, out: &mut Vec<F>) {
        self.process_push(
            res,
            state,
            out,
            // Some(frame):
            |this, out, state| this.after_high(out, state),
            // None:
            |this, state| {
                Self::handle_closed_receiver(&mut this.high_rx, state);
                this.reset_high_counter();
            },
        );
    }

    fn process_low(&mut self, res: Option<F>, state: &mut ActorState, out: &mut Vec<F>) {
        self.process_push(
            res,
            state,
            out,
            // Some(frame):
            |this, _, _| this.after_low(),
            // None:
            |this, state| Self::handle_closed_receiver(&mut this.low_rx, state),
        );
    }
}

// you can now remove `enum Queue { High, Low }`
```

This keeps exactly the same branch logic but removes both the enum and the nested matches, reducing cognitive overhead.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/connection.rs Outdated
}

#[derive(Clone, Copy)]
enum Queue {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider replacing the Queue enum and match logic with a callback-based helper to simplify the code.

You can collapse the extra Queue‐enum + match entirely by turning your helper into a small “run one of two callbacks” routine. This lets you keep the process_high/process_low entry points very simple, get rid of Queue, and preserve exactly the same behavior:

impl YourActor<F, E> {
    // helper that just picks one of two callbacks instead of matching on Queue
    fn process_push<OnSome, OnNone>(
        &mut self,
        res: Option<F>,
        state: &mut ActorState,
        out: &mut Vec<F>,
        on_some: OnSome,
        on_none: OnNone,
    )
    where
        OnSome: FnOnce(&mut Self, &mut Vec<F>, &mut ActorState),
        OnNone: FnOnce(&mut Self, &mut ActorState),
    {
        if let Some(frame) = res {
            self.process_frame_common(frame, out);
            on_some(self, out, state);
        } else {
            on_none(self, state);
        }
    }

    fn process_high(&mut self, res: Option<F>, state: &mut ActorState, out: &mut Vec<F>) {
        self.process_push(
            res,
            state,
            out,
            // Some(frame):
            |this, out, state| this.after_high(out, state),
            // None:
            |this, state| {
                Self::handle_closed_receiver(&mut this.high_rx, state);
                this.reset_high_counter();
            },
        );
    }

    fn process_low(&mut self, res: Option<F>, state: &mut ActorState, out: &mut Vec<F>) {
        self.process_push(
            res,
            state,
            out,
            // Some(frame):
            |this, _, _| this.after_low(),
            // None:
            |this, state| Self::handle_closed_receiver(&mut this.low_rx, state),
        );
    }
}

// you can now remove `enum Queue { High, Low }`

This keeps exactly the same branch logic but removes both the enum and the nested matches, reducing cognitive overhead.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this now been resolved?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this has been resolved! The latest code no longer defines or uses the Queue enum. Instead, the queue processing logic has been unified into the process_push helper, which now takes callbacks for the high- and low-priority cases, just as suggested in the review. This reduces complexity and matches the recommended approach. If you have any further questions or want a code walkthrough, let me know!

Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gates Failed
Enforce advisory code health rules (1 file with Excess Number of Function Arguments)

Gates Passed
5 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce advisory code health rules Violations Code Health Impact
connection.rs 1 advisory rule 10.00 → 9.69 Suppress

Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

Comment thread src/connection.rs Outdated
Comment thread src/connection.rs Outdated
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Aug 2, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 2, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Improved (1 files improve in Code Health)

Gates Passed
6 Quality Gates Passed

See analysis details in CodeScene

View Improvements
File Code Health Impact Categories Improved
push.rs 9.69 → 10.00 Complex Conditional

Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 710b2d6 and ba5da2e.

📒 Files selected for processing (4)
  • examples/metadata_routing.rs (1 hunks)
  • src/connection.rs (9 hunks)
  • src/push.rs (3 hunks)
  • src/server.rs (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit Inference Engine (AGENTS.md)

**/*.rs: Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider using Arc to reduce the amount of data returned.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessary mut bindings.
Handle errors with the Result type instead of panicking where feasible.
Avoid unsafe code unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not use return in single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Prefer expect over allow.
Prefer .expect() over .unwrap().
Use concat!() to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums: Derive std::error::Error (via the thiserror crate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Use eyre::Report for human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and to eyre only in the main main() entrypoint or top-level async task.

Files:

  • examples/metadata_routing.rs
  • src/server.rs
  • src/push.rs
  • src/connection.rs

⚙️ CodeRabbit Configuration File

**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.

  • Adhere to single responsibility and CQRS

  • Place function attributes after doc comments.

  • Do not use return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless absolutely necessary.

  • Every module must begin with a //! doc comment that explains the module's purpose and utility.

  • Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar

  • Lints must not be silenced except as a last resort.

    • #[allow] is forbidden.
    • Only narrowly scoped #[expect(lint, reason = "...")] is allowed.
    • No lint groups, no blanket or file-wide suppression.
    • Include FIXME: with link if a fix is expected.
  • Use rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for mocks/stubs.

  • Prefer .expect() over .unwrap()

  • Ensure that any API or behavioural changes are reflected in the documentation in docs/

  • Ensure that any completed roadmap steps are recorded in the appropriate roadmap in docs/

  • Files must not exceed 400 lines in length

    • Large modules must be decomposed
    • Long match statements or dispatch tables should be decomposed by domain and collocated with targets
    • Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.

Files:

  • examples/metadata_routing.rs
  • src/server.rs
  • src/push.rs
  • src/connection.rs
🔇 Additional comments (16)
src/server.rs (2)

232-234: LGTM!

The reformatting from single-line to multi-line block improves readability whilst maintaining the same behaviour.


474-478: LGTM!

The refactoring to nested if let statements improves code clarity whilst preserving the original logic.

src/push.rs (3)

100-102: LGTM!

The reformatting from single-line to multi-line block improves readability whilst maintaining the same behaviour.


258-260: LGTM!

The reformatting from single-line to multi-line block improves readability whilst maintaining the same behaviour.


389-393: LGTM!

The refactoring to nested conditional statements improves code clarity whilst preserving the original validation logic.

src/connection.rs (11)

46-48: LGTM!

The reformatting from single-line to multi-line block improves readability whilst maintaining the same behaviour.


70-80: Excellent refactoring to address function argument concerns.

The ProcessContext struct effectively reduces parameter passing complexity and addresses the previous review feedback about excessive function arguments.


205-207: LGTM!

The reformatting from single-line to multi-line block improves readability whilst maintaining the same behaviour.


210-212: LGTM!

The reformatting from single-line to multi-line block improves readability whilst maintaining the same behaviour.


216-218: LGTM!

The reformatting from single-line to multi-line block improves readability whilst maintaining the same behaviour.


306-314: LGTM!

The integration of ProcessContext simplifies parameter passing and improves code organisation whilst maintaining the same functionality.


326-361: Excellent consolidation of duplicate queue processing logic.

The refactoring successfully eliminates code duplication by introducing the generic process_push helper method with callback parameters. This addresses both the previous concerns about function argument count and code duplication whilst maintaining identical behaviour.


416-431: LGTM!

The nested conditional structure improves code clarity whilst preserving the original logic for yielding to low-priority processing.


447-449: LGTM!

The reformatting from single-line to multi-line block improves readability whilst maintaining the same behaviour.


492-500: LGTM!

The reformatting from single-line to multi-line blocks improves readability whilst maintaining the same behaviour.


583-595: LGTM!

The reformatting from single-line to multi-line blocks improves readability whilst maintaining the same behaviour.

Comment thread examples/metadata_routing.rs Outdated

#[derive(bincode::Decode, bincode::Encode)]
#[expect(dead_code, reason = "used only in documentation example")]
#[allow(dead_code)] // used only in documentation example
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Revert to #[expect] directive to comply with coding guidelines.

Replace #[allow(dead_code)] with the previous #[expect] form as the coding guidelines explicitly forbid #[allow] directives.

Apply this diff to fix the compliance issue:

-#[allow(dead_code)] // used only in documentation example
+#[expect(dead_code, reason = "used only in documentation example")]
🤖 Prompt for AI Agents
In examples/metadata_routing.rs at line 63, replace the attribute
#[allow(dead_code)] with the previously used #[expect(dead_code)] directive to
comply with the coding guidelines that forbid #[allow] directives. This change
ensures adherence to the project's coding standards.

Comment thread src/server.rs Outdated
/// Test helper preamble carrying no data.
#[derive(Debug, Clone, PartialEq, Encode, Decode)]
#[expect(dead_code, reason = "test helper for unused preamble type")]
#[allow(dead_code)] // test helper for unused preamble type
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Revert to #[expect] directive to comply with coding guidelines.

Replace #[allow(dead_code)] with the previous #[expect] form as the coding guidelines explicitly forbid #[allow] directives.

Apply this diff to fix the compliance issue:

-#[allow(dead_code)] // test helper for unused preamble type
+#[expect(dead_code, reason = "test helper for unused preamble type")]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[allow(dead_code)] // test helper for unused preamble type
#[expect(dead_code, reason = "test helper for unused preamble type")]
🤖 Prompt for AI Agents
In src/server.rs at line 525, replace the attribute directive
#[allow(dead_code)] with the previously used #[expect(dead_code)] to comply with
the coding guidelines that forbid #[allow] directives. This change ensures
adherence to the project's coding standards.

Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Health Improved (1 files improve in Code Health)

Gates Passed
6 Quality Gates Passed

See analysis details in CodeScene

View Improvements
File Code Health Impact Categories Improved
push.rs 9.69 → 10.00 Complex Conditional

Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (2)
examples/metadata_routing.rs (1)

63-63: Revert to #[expect] directive to comply with coding guidelines.

Replace #[allow(dead_code)] with the previous #[expect] form as the coding guidelines explicitly forbid #[allow] directives.

Apply this diff to fix the compliance issue:

-#[allow(dead_code, reason = "used only in documentation example")]
+#[expect(dead_code, reason = "used only in documentation example")]
src/server.rs (1)

525-525: Revert to #[expect] directive to comply with coding guidelines.

Replace #[allow(dead_code)] with the previous #[expect] form as the coding guidelines explicitly forbid #[allow] directives.

Apply this diff to fix the compliance issue:

-#[allow(dead_code, reason = "test helper for unused preamble type")]
+#[expect(dead_code, reason = "test helper for unused preamble type")]
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba5da2e and 44e5c2c.

📒 Files selected for processing (5)
  • examples/metadata_routing.rs (1 hunks)
  • examples/ping_pong.rs (1 hunks)
  • src/middleware.rs (8 hunks)
  • src/server.rs (3 hunks)
  • wireframe_testing/src/helpers.rs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit Inference Engine (AGENTS.md)

**/*.rs: Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider using Arc to reduce the amount of data returned.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessary mut bindings.
Handle errors with the Result type instead of panicking where feasible.
Avoid unsafe code unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not use return in single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Prefer expect over allow.
Prefer .expect() over .unwrap().
Use concat!() to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums: Derive std::error::Error (via the thiserror crate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Use eyre::Report for human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and to eyre only in the main main() entrypoint or top-level async task.

Files:

  • wireframe_testing/src/helpers.rs
  • examples/metadata_routing.rs
  • examples/ping_pong.rs
  • src/server.rs
  • src/middleware.rs

⚙️ CodeRabbit Configuration File

**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.

  • Adhere to single responsibility and CQRS

  • Place function attributes after doc comments.

  • Do not use return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless absolutely necessary.

  • Every module must begin with a //! doc comment that explains the module's purpose and utility.

  • Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar

  • Lints must not be silenced except as a last resort.

    • #[allow] is forbidden.
    • Only narrowly scoped #[expect(lint, reason = "...")] is allowed.
    • No lint groups, no blanket or file-wide suppression.
    • Include FIXME: with link if a fix is expected.
  • Use rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for mocks/stubs.

  • Prefer .expect() over .unwrap()

  • Ensure that any API or behavioural changes are reflected in the documentation in docs/

  • Ensure that any completed roadmap steps are recorded in the appropriate roadmap in docs/

  • Files must not exceed 400 lines in length

    • Large modules must be decomposed
    • Long match statements or dispatch tables should be decomposed by domain and collocated with targets
    • Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.

Files:

  • wireframe_testing/src/helpers.rs
  • examples/metadata_routing.rs
  • examples/ping_pong.rs
  • src/server.rs
  • src/middleware.rs
🔇 Additional comments (8)
wireframe_testing/src/helpers.rs (1)

22-24: LGTM!

The formatting change from single-line to multi-line function body improves readability.

src/server.rs (2)

232-234: LGTM!

The formatting change from single-line to multi-line function body improves consistency with the codebase style.


474-478: LGTM!

The refactoring from combined if let with logical AND to nested if let statements improves clarity whilst preserving the same logic.

src/middleware.rs (5)

20-22: LGTM!

The formatting change from single-line to multi-line constructor improves consistency with codebase style.


26-28: LGTM!

The formatting change improves readability by expanding the single-line accessor to a multi-line block.


32-34: LGTM!

The formatting change improves consistency by expanding the mutable accessor to a multi-line block.


38-40: LGTM!

The formatting change improves readability by expanding the consuming method to a multi-line block.


60-62: LGTM!

The consistent formatting changes from single-line to multi-line function bodies across all accessor methods, constructors, and simple getters improve codebase consistency and readability.

Also applies to: 66-68, 72-74, 94-96, 100-102, 106-108, 143-145, 202-204, 233-235, 316-318

Comment thread examples/ping_pong.rs
Comment on lines +43 to +46
#[allow(
clippy::unused_async,
reason = "example handler intentionally performs no work"
)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Replace #[allow] with #[expect] to comply with coding guidelines.

The coding guidelines explicitly forbid #[allow] directives. Use #[expect] instead with the same reason.

Apply this diff to fix the compliance issue:

-#[allow(
-    clippy::unused_async,
-    reason = "example handler intentionally performs no work"
-)]
+#[expect(
+    clippy::unused_async,
+    reason = "example handler intentionally performs no work"
+)]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[allow(
clippy::unused_async,
reason = "example handler intentionally performs no work"
)]
#[expect(
clippy::unused_async,
reason = "example handler intentionally performs no work"
)]
🤖 Prompt for AI Agents
In examples/ping_pong.rs around lines 43 to 46, replace the attribute macro
#[allow(clippy::unused_async, reason = "example handler intentionally performs
no work")] with #[expect(clippy::unused_async, reason = "example handler
intentionally performs no work")] to comply with coding guidelines that forbid
#[allow] directives.

Comment thread src/middleware.rs
Comment on lines +182 to +186
#[allow(
clippy::inline_fn_without_body,
unused_attributes,
reason = "future-proof attribute and inline hint without body"
)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Replace #[allow] with #[expect] to comply with coding guidelines.

The coding guidelines explicitly forbid #[allow] directives. Use #[expect] instead with the same reason.

Apply this diff to fix the compliance issue:

-#[allow(
-    clippy::inline_fn_without_body,
-    unused_attributes,
-    reason = "future-proof attribute and inline hint without body"
-)]
+#[expect(
+    clippy::inline_fn_without_body,
+    unused_attributes,
+    reason = "future-proof attribute and inline hint without body"
+)]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[allow(
clippy::inline_fn_without_body,
unused_attributes,
reason = "future-proof attribute and inline hint without body"
)]
#[expect(
clippy::inline_fn_without_body,
unused_attributes,
reason = "future-proof attribute and inline hint without body"
)]
🤖 Prompt for AI Agents
In src/middleware.rs around lines 182 to 186, replace the #[allow] attribute
with #[expect] while keeping the same lint names and reason string to comply
with coding guidelines that forbid #[allow] directives.

/// Returns any I/O errors encountered while interacting with the in-memory
/// duplex stream.
#[allow(dead_code)]
#[allow(dead_code, reason = "unused outside integration tests")]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Replace #[allow] with #[expect] to comply with coding guidelines.

The coding guidelines explicitly forbid #[allow] directives. Use #[expect] instead.

Apply this diff to fix the compliance issue:

-#[allow(dead_code, reason = "unused outside integration tests")]
+#[expect(dead_code, reason = "unused outside integration tests")]
🤖 Prompt for AI Agents
In wireframe_testing/src/helpers.rs at line 239, replace the attribute directive
#[allow(dead_code, reason = "unused outside integration tests")] with
#[expect(dead_code, reason = "unused outside integration tests")] to comply with
the coding guidelines that forbid #[allow] directives.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Aug 3, 2025

Stale

@leynos leynos closed this Aug 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant