Skip to content

Implement WireframeClient with Configurable Runtime#407

Merged
leynos merged 8 commits intomainfrom
terragon/implement-wireframe-client-foundation-cc7eo8
Dec 23, 2025
Merged

Implement WireframeClient with Configurable Runtime#407
leynos merged 8 commits intomainfrom
terragon/implement-wireframe-client-foundation-cc7eo8

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented Dec 20, 2025

Summary

Adds a fully configurable Wireframe client runtime, including a builder, codec and socket configuration, error handling, and a runtime driver that mirrors the server's framing/serialization. This enables caller-configurable serializers, frame length, and socket options before establishing a connection, plus end-to-end request/response flows.

Changes

Client runtime

  • Introduced a complete client runtime module with:
    • WireframeClientBuilder for configuring serializer, codec, and socket options
    • ClientCodecConfig and SocketOptions for runtime configuration
    • ClientError enum for transport, serialization, and deserialization errors
    • WireframeClient runtime with send, receive, call, and codec inspection

Builder pattern

  • Added WireframeClientBuilder with fluent APIs to:
    • set a custom Serializer (default BincodeSerializer)
    • configure ClientCodecConfig (max_frame_length, length format)
    • configure SocketOptions (nodelay, keepalive, linger, buffer sizes, reuse settings)
    • connect via a pre-configured TcpSocket with options applied

Configuration

  • New src/client/config.rs (ClientCodecConfig) implementing:
    • max_frame_length with clamping to [64, 16 MiB]
    • length_format with Endianness and byte-length handling
    • build_codec() to produce a LengthDelimitedCodec tailored to settings
  • New src/client/mod.rs to expose WireframeClientBuilder, ClientCodecConfig, SocketOptions, and WireframeClient
  • New src/client/error.rs with ClientError variants for Io, Serialize, Deserialize, and Disconnected

Runtime implementation

  • New src/client/runtime.rs with:
    • WireframeClient struct holding framed I/O, serializer, and codec config
    • async fn send<M: Message>(&mut self, message) to serialize and transmit
    • async fn receive<M: Message>(&mut self) to decode and deserialize
    • async fn call<Req: Message, Resp: Message>(&mut self, request) to send and await response
    • accessor for codec_config
  • WireframeClient::builder() helper for ergonomic client construction

Tests

  • New unit tests in src/client/tests.rs for codec clamping and defaults
  • Builder integration test to verify Nagle (nodelay) option is applied
  • Behaviour-driven tests wired into cucumber world (client_runtime feature) with steps in tests/steps/client_steps.rs
  • New cucumber world at tests/worlds/client_runtime.rs to start a simple echo server and verify payload echoing under configured frame lengths

Public API exposure

  • lib.rs re-exports WireframeClient, ClientCodecConfig, SocketOptions, and ClientError for ergonomic usage by downstream code

Usage

use std::net::SocketAddr;
use wireframe::{client::{ClientCodecConfig, SocketOptions}, WireframeClient};
use wireframe::BincodeSerializer; // or another Serializer impl

let addr: SocketAddr = "127.0.0.1:7878".parse().unwrap();
let codec = ClientCodecConfig::default().max_frame_length(2048);
let socket_opts = SocketOptions::default().nodelay(true).keepalive(true);

let mut client = WireframeClient::builder()
    .serializer(BincodeSerializer)
    .codec_config(codec)
    .socket_options(socket_opts)
    .connect(addr)
    .await?;

// define and send a request type that implements Message
let req = YourRequest { /* fields */ };
let resp: YourResponse = client.call(&req).await?;

Test plan

  • Unit tests for codec clamping and defaults (src/client/tests.rs)
  • Integration test verifying builder-applied transport options (nodelay)
  • Behavioural tests for client runtime using cucumber (client_runtime.feature, world, and steps)

Notes

  • The client runtime is designed to mirror server framing and serialization layers, enabling consistent end-to-end behavior while allowing configuration before connection.
  • Default max_frame_length remains aligned with server defaults, with clamping to sane bounds to avoid oversized frames.

🌿 Generated by Terry


ℹ️ Tag @terragon-labs to ask questions and address PR feedback

📎 Task: https://www.terragonlabs.com/task/5ead8d98-3ae6-4da9-8d7f-e581ea2ece4c

Summary by Sourcery

Add a configurable Wireframe client runtime with builder-based codec and socket configuration, expose it via the public API, document its usage, and cover it with unit and behaviour-driven tests.

New Features:

  • Introduce a WireframeClient runtime with send, receive, and call operations over a framed TCP connection.
  • Add ClientCodecConfig and SocketOptions types to configure frame encoding and TCP parameters before connecting.
  • Provide a WireframeClientBuilder for fluent configuration of serializer, codec settings, and socket options.

Enhancements:

  • Expose the new client module and its types (WireframeClient, ClientCodecConfig, SocketOptions, ClientError) from the library root.
  • Extend design and user documentation to describe the client runtime, configuration, and example usage.
  • Mark the roadmap item for implementing WireframeClient and its builder as completed.

Documentation:

  • Document the client runtime in the user guide with a full configuration and usage example, and link back to implementation details.
  • Update the client design document with concrete builder examples, socket configuration rationale, and implementation notes.

Tests:

  • Add unit tests for client codec defaults and frame length clamping, plus a builder integration test validating TCP_NODELAY application.
  • Introduce a Cucumber client runtime world, steps, and feature to validate end-to-end request/response behavior with configurable frame lengths.

Introduce a new WireframeClient runtime module that mirrors the server's framing and serialization layers. This includes:

- A WireframeClientBuilder with methods to configure serializers, codec settings (max frame length, length prefix format), and socket options (TCP_NODELAY, keepalive, linger, buffer sizes, reuseaddr, reuseport).
- ClientCodecConfig for length-delimited codec configuration with clamped max frame length.
- SocketOptions struct for TCP socket tuning applied before connecting.
- Detailed async runtime methods on WireframeClient for sending, receiving, and request-response call patterns.
- ClientError enum covering transport and serialization errors.
- Extensive documentation, usage examples, and integration in user guides and design docs.
- Behavioral tests with cucumber scenarios, test world, and unit tests validating configuration clamping and socket options.

This addition enables flexible, well-typed client connections aligned with server configuration for robust wireframe protocol communication.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 20, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced Wireframe client runtime enabling connections to remote servers with builder-based configuration
    • Socket option configuration for TCP tuning (buffer sizes, keepalive, linger, nodelay, reuse settings)
    • Configurable frame length and encoding format for protocol communication
  • Documentation

    • Added comprehensive client runtime documentation with usage examples and configuration guidelines
  • Chores

    • Added socket2 dependency (v0.6.0) for socket operations

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Add a Wireframe client runtime with builder, codec and socket configuration, error types, runtime send/receive/call APIs, unit and integration tests including Cucumber scenarios, and add the socket2 = "0.6.0" dependency; update docs and examples accordingly.

Changes

Cohort / File(s) Summary
Dependency & Docs
Cargo.toml, docs/roadmap.md, docs/users-guide.md, docs/wireframe-client-design.md
Add socket2 = "0.6.0"; mark client runtime roadmap item complete; insert client runtime section and examples in user guide; update design docs to use SocketAddr, borrowed call(&T) and new max_frame_length builder option.
Client API & Runtime
src/client/mod.rs, src/client/builder.rs, src/client/config.rs, src/client/error.rs, src/client/runtime.rs, src/client/codec_config.rs
Add WireframeClientBuilder<S> with fluent codec/socket options and async connect(SocketAddr); add ClientCodecConfig and SocketOptions with apply-to-socket logic; introduce ClientError; add generic WireframeClient<S> with send, receive, call and builder entrypoint.
Client Unit Tests
src/client/tests.rs
Add unit tests for codec config clamping/defaults, end-to-end length-delimited encoding, and propagation/application of socket options (platform-gated reuseport).
Crate Exports
src/lib.rs
Export new client module and re-export ClientCodecConfig, ClientError, SocketOptions, WireframeClient at crate root.
Integration & Cucumber Tests
tests/client_runtime.rs, tests/features/client_runtime.feature, tests/steps/client_steps.rs, tests/steps/mod.rs, tests/worlds/client_runtime.rs, tests/worlds/mod.rs, tests/world.rs, tests/cucumber.rs
Add integration test for frame-size error propagation; add Cucumber feature, steps and test world for echo and framing-error scenarios; wire ClientRuntimeWorld into test runner.
Log Test Adjustment
tests/connection.rs
Filter log records by correlation_id string before validating level and reason; remove redundant correlation assertion.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Dev as User/Caller
    participant Builder as WireframeClientBuilder
    participant Socket as TcpSocket (socket2/Tokio)
    participant Codec as LengthDelimitedCodec
    participant Framed as Framed<TcpStream, Codec>
    participant Client as WireframeClient
    participant Serializer as Serializer (e.g. Bincode)

    Dev->>Builder: new() / configure(serializer, codec_config, socket_options)
    Dev->>Builder: connect(addr: SocketAddr)
    activate Builder
    Builder->>Socket: create (v4/v6) & apply SocketOptions
    Builder->>Socket: connect(addr)
    Socket-->>Builder: connected TcpStream
    Builder->>Codec: build with max_frame_length, length_format, endianness
    Builder->>Framed: wrap TcpStream with Codec
    Builder->>Client: construct WireframeClient with Framed + Serializer
    deactivate Builder

    Dev->>Client: send(&message)
    activate Client
    Client->>Serializer: encode(&message)
    Serializer->>Framed: write framed bytes
    Framed->>Server: transmit frame
    deactivate Client

    Dev->>Client: receive()
    activate Client
    Framed->>Client: read frame
    Client->>Serializer: decode frame
    Serializer-->>Dev: return Message
    deactivate Client

    Dev->>Client: call(&req)
    activate Client
    Client->>Client: send(&req)
    Client->>Client: receive() -> response
    Client-->>Dev: Result<Response> / ClientError
    deactivate Client
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

✨ A socket stitched, a builder tuned,
Codec set and frames buffooned,
Client speaks and server sings,
Errors caught and test bells ring,
New runtime dances — code attuned 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarises the main addition: a fully configurable WireframeClient with builder pattern and runtime capabilities.
Description check ✅ Passed The description comprehensively covers all changes, from the client runtime module to builder pattern, configuration options, tests, and public API exposure, directly mirroring the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch terragon/implement-wireframe-client-foundation-cc7eo8

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c5a83f0 and f0edde8.

📒 Files selected for processing (6)
  • docs/users-guide.md
  • src/client/codec_config.rs
  • src/client/config.rs
  • src/client/mod.rs
  • src/client/tests.rs
  • tests/worlds/client_runtime.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Run make check-fmt (cargo fmt --workspace -- --check) before committing to validate Rust formatting across the entire workspace without modifying files.
Run make lint (cargo clippy --workspace --all-targets --all-features -- -D warnings) before committing to lint every target with all features enabled and deny all Clippy warnings.
Run make test (cargo test --workspace) before committing to run the full workspace test suite.
Clippy warnings MUST be disallowed. Fix any warnings emitted during tests in the code itself rather than silencing them.
Use make fmt (cargo fmt --workspace) to apply formatting fixes reported by the formatter check.
Where a Rust function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a Rust function has too many parameters, group related parameters in meaningfully named structs.
Where a Rust function is returning a large error, consider using Arc to reduce the amount of data returned.
Write unit and behavioural tests for new Rust functionality. Run both before and after making any change.
Every Rust module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public Rust APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessary mut bindings in Rust code.
Avoid unsafe code in Rust unless absolutely necessary, and document any usage clearly with a 'SAFETY' comment.
Place function attributes after doc comments in Rust code.
Do not use return in single-line Rust functions.
Use predicate functions for Rust conditional criteria with more than two branches.
Lints must not be silenced in Rust except as a last resort. Lint rule suppressions must be tightly scoped and include a clear reason.
Use concat!() to combine long string literals in Rust rather than escaping newlines with a backslash.
Prefer single-line versions of Rust functions where...

Files:

  • src/client/codec_config.rs
  • tests/worlds/client_runtime.rs
  • src/client/config.rs
  • src/client/tests.rs
  • src/client/mod.rs

⚙️ CodeRabbit configuration file

**/*.rs: * Seek to keep the cognitive complexity of functions no more than 9.

  • 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 / -yse / -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.
  • Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
  • 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() in tests.
  • .expect() and .unwrap() are forbidden outside of tests. Errors must be propagated.
  • 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.
  • Environment access (env::set_var and env::remove_var) are always unsafe in Rust 2024 and MUST be marked as such
    • For testing of functionality depending upon environment variables, dependency injection and...

Files:

  • src/client/codec_config.rs
  • tests/worlds/client_runtime.rs
  • src/client/config.rs
  • src/client/tests.rs
  • src/client/mod.rs
docs/**/*.md

📄 CodeRabbit inference engine (docs/contents.md)

Follow the documentation style guide conventions when writing project documentation

docs/**/*.md: Use British English based on the Oxford English Dictionary (en-GB-oxendict), including: -ize suffix in words like 'realize' and 'organization', -lyse suffix in words like 'analyse', -our suffix in words like 'colour', -re suffix in words like 'centre', double 'l' in words like 'cancelled', maintain 'e' in words like 'likeable', -ogue suffix in words like 'catalogue'
The word 'outwith' is acceptable in British English documentation
Keep US spelling when used in an API (for example, 'color')
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns: 'Lille Industries are expanding'
Write headings in sentence case in documentation
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Follow markdownlint recommendations for Markdown files
Always provide a language identifier for fenced code blocks in documentation; use 'plaintext' for non-code text
Use '-' as the first level bullet and renumber lists when items change in documentation
Prefer inline links using 'text' or angle brackets around the URL in documentation
Ensure blank lines before and after bulleted lists and fenced blocks in documentation
Ensure tables have a delimiter line below the header row in documentation
Expand any uncommon acronym on first use in documentation (for example, Continuous Integration (CI))
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with '[^label]' in documentation
Include Mermaid diagrams where they add clarity to documentation
When embedding figures in documentation, use 'alt text' and provide brief alt text describing the content
Add a short description before each Mermaid diagram so screen readers can understand it

Write official documentation with se...

Files:

  • docs/users-guide.md
docs/**/*.{rs,md}

📄 CodeRabbit inference engine (docs/rust-doctest-dry-guide.md)

Every doctest should validate the public API of a crate from the perspective of an external user, treating each documentation test as a separate temporary crate that imports the library as an external dependency

Files:

  • docs/users-guide.md
**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

**/*.md: Validate Markdown files using make markdownlint.
Run make fmt after any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by running make nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes in Markdown.

Files:

  • docs/users-guide.md

⚙️ CodeRabbit configuration file

**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")

  • Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
  • Headings must not be wrapped.
  • Documents must start with a level 1 heading
  • Headings must correctly increase or decrease by no more than one level at a time
  • Use GitHub-flavoured Markdown style for footnotes and endnotes.
  • Numbered footnotes must be numbered by order of appearance in the document.

Files:

  • docs/users-guide.md
🧬 Code graph analysis (3)
tests/worlds/client_runtime.rs (3)
src/client/codec_config.rs (1)
  • max_frame_length (50-53)
src/client/builder.rs (3)
  • max_frame_length (109-112)
  • new (42-48)
  • codec_config (93-96)
src/client/runtime.rs (2)
  • builder (64-64)
  • codec_config (200-200)
src/client/config.rs (1)
src/client/builder.rs (8)
  • nodelay (158-161)
  • keepalive (176-179)
  • linger (194-197)
  • send_buffer_size (210-213)
  • recv_buffer_size (226-229)
  • reuseaddr (242-245)
  • reuseport (264-267)
  • new (42-48)
src/client/mod.rs (2)
src/client/runtime.rs (2)
  • builder (64-64)
  • codec_config (200-200)
src/client/builder.rs (1)
  • codec_config (93-96)
🔍 Remote MCP

Now let me search for more specific information about error handling patterns and codec best practices in Rust:Based on my search results, I have gathered sufficient context to provide a comprehensive summary of relevant information for reviewing this pull request.

Summary of Relevant Context for PR #407 Review

Socket Options Configuration & TCP Settings

TcpSocket wraps an operating system socket and enables the caller to configure the socket before establishing a TCP connection or accepting inbound connections, with the ability to set socket options and explicitly bind the socket with a socket address. The PR's SocketOptions implementation aligns with this pattern by providing pre-connection configuration. Key considerations:

  • Most operating systems have minimum and maximum allowed sizes for send/recv buffers that clamp provided values, and Linux doubles the buffer size to account for internal bookkeeping data. This is relevant for the PR's send_buffer_size and recv_buffer_size configuration methods.

  • Some socket2 methods that set options require two system calls to set their options without overwriting previously set options by first getting current settings, applying changes, and updating—meaning the operation is not atomic and can lead to data races when two threads change options in parallel. This is important for reviewing the SocketOptions::apply implementation.

Length-Delimited Codec Configuration

The PR's ClientCodecConfig and its build_codec() method work with LengthDelimitedCodec. Key insights:

  • The max_frame_length is the largest size a codec will accept from the wire, with larger frames being rejected, and changes take effect on the next frame decode. The PR's clamping of max_frame_length to [64 bytes, 16 MiB] is a reasonable bounds-check pattern.

  • The default LengthDelimitedCodec setting is big endian, and this configuration option applies to both encoding and decoding. This confirms the PR's approach to configurable endianness via LengthFormat.

Error Handling with Boxed Errors

The PR uses Box<dyn std::error::Error + Send + Sync> for serialization/deserialization errors. Best practices:

  • Trait objects are unsized and don't implement the Sized marker trait, so a Box stores the trait object itself on the heap while storing the pointer on the stack, solving the sizing problem. This justifies the PR's use of boxed errors for serialization failures.

  • For library code, errors are part of the API so consumers know what errors to expect and recover from, and they need to be structured and easy to perform exhaustive matching on. The PR's approach with distinct ClientError enum variants for Io, Serialize, Deserialize, and Disconnected follows this pattern well.

Async Error Handling Patterns

  • Error handling in async Rust code is fundamentally different from synchronous code, as traditional try-catch or simple error returns aren't enough when juggling multiple concurrent operations. The PR's use of Result<T, ClientError> and map_err conversions in async methods addresses this.

Builder Pattern for Configuration

The WireframeClientBuilder pattern with fluent APIs is standard practice:

  • Many protocols delimit frames by prefacing data with a head specifying length, and the length_delimited module provides utilities for handling this, allowing consumers to work with entire frames without worrying about buffering or framing logic. The PR's builder approach simplifies this configuration.

[::web_search::] provides TCP socket configuration best practices and codec documentation context.

🔇 Additional comments (29)
src/client/mod.rs (1)

1-19: LGTM: clean module structure.

The module correctly declares submodules, re-exports the public API surface (WireframeClient, builder, config types, and error), and includes a test module as required.

src/client/tests.rs (7)

1-13: LGTM: module documentation and imports.

The module doc comment explains the purpose, and imports are well-organized.


14-21: LGTM: well-defined test constants.

Constants improve test readability and align with the codec configuration boundaries.


22-32: LGTM: helper reduces test duplication.

The spawn_listener helper cleanly abstracts listener setup. Use of .expect() is acceptable in test code.


34-50: LGTM: excellent abstraction for testing builder options.

The assert_builder_option helper eliminates duplication across socket option tests. The generic approach is clean and reusable.


52-57: LGTM: macro reduces test boilerplate.

The socket_option_test! macro cleanly wraps the test pattern, further eliminating duplication.


59-116: LGTM: comprehensive codec configuration tests.

The parameterized clamping test covers boundary cases, and the build_codec_configures_length_delimited_codec test validates actual codec behaviour through encode/decode round-trip, addressing previous review feedback.


118-204: LGTM: comprehensive socket option test coverage.

The socket option tests cover all builder methods (nodelay, keepalive, linger, buffer sizes, reuseaddr, reuseport) with appropriate platform guards for reuseport. Buffer size assertions correctly use >= to accommodate OS rounding.

docs/users-guide.md (3)

102-107: LGTM: wording improvements remove second-person pronouns.

The changes correctly eliminate second-person language ("you") in favour of passive constructions, aligning with documentation guidelines.


381-425: LGTM: well-structured client runtime documentation.

The new section clearly introduces WireframeClient, its builder, and configuration options. The code example demonstrates key patterns (codec config, socket options, connect, call) and correctly uses the no_run attribute to prevent doc-test failures whilst preserving syntax validation.


672-673: LGTM: footnote correctly documents implementation files.

The new footnote [^44] appropriately references the client runtime implementation files.

src/client/codec_config.rs (5)

1-25: LGTM: well-documented codec configuration structure.

The module doc comment, struct definition, and constants are clean. Doc examples demonstrate usage appropriately.


27-34: LGTM: sensible default configuration.

The default implementation provides reasonable values (4-byte big-endian length format, 1024-byte max frame).


36-73: LGTM: builder methods with appropriate bounds checking.

The max_frame_length method correctly clamps input to [64 bytes, 16 MiB], and both methods properly use #[must_use] for the builder pattern.


75-100: LGTM: clean accessor methods.

The accessor methods correctly use const fn and provide clear documentation with examples.


102-116: LGTM: codec construction correctly applies configuration.

The build_codec method properly configures the LengthDelimitedCodec with the stored length format, endianness, and max frame length.

src/client/config.rs (7)

1-40: LGTM: well-designed socket options structure.

The module doc comment and struct definition are clean. Platform-conditional compilation for reuseport is correctly applied for Unix-like systems (excluding Solaris, illumos, Cygwin).


42-70: LGTM: consistent internal enum design.

Both LingerSetting and KeepAliveSetting follow the same pattern with to_option helpers, providing clean conversions to Option<Duration>.


72-110: LGTM: clean builder methods for TCP options.

The nodelay and keepalive methods properly implement the builder pattern with appropriate documentation and the #[must_use] attribute.


112-166: LGTM: buffer size and linger configuration methods.

The linger, send_buffer_size, and recv_buffer_size methods follow the established builder pattern with clear documentation.


168-206: LGTM: reuse address/port configuration methods.

The reuseaddr and reuseport methods correctly implement platform-conditional compilation for reuseport, aligning with socket2's platform support.


208-269: LGTM: well-structured option application logic.

The apply method cleanly delegates to per-option helpers, reducing complexity as suggested in previous reviews. The apply_keepalive helper correctly uses socket2::TcpKeepalive for duration configuration.


271-291: LGTM: platform-conditional reuseport application.

The dual implementations of apply_reuseport correctly handle platform differences, with a no-op for unsupported platforms.

tests/worlds/client_runtime.rs (6)

1-29: LGTM: well-structured test world.

The module doc comment, cfg guard, and struct definitions are clean. The ClientRuntimeWorld correctly derives World for cucumber integration.


31-64: LGTM: echo server setup with proper error logging.

The start_server method correctly spawns an echo server task and now includes warn! logging for error paths, addressing previous feedback about silently swallowed errors.


66-79: LGTM: clean client connection setup.

The connect_client method correctly configures the codec and uses the builder pattern to establish the connection.


81-112: LGTM: payload sending methods cover success and error cases.

The send_payload method handles successful round-trips, whilst send_payload_expect_error correctly verifies that oversized payloads trigger errors, exercising framing limits.


114-142: LGTM: verification methods with extracted cleanup helper.

The verify_echo and verify_error methods correctly validate test outcomes and share the extracted await_server helper, adhering to DRY principles per previous feedback.


144-151: LGTM: clean server cleanup helper.

The await_server helper eliminates duplication between verification methods, as requested in previous reviews.


Comment @coderabbitai help to get the list of available commands and usage tips.

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Dec 20, 2025

Reviewer's Guide

Introduce a configurable Wireframe client runtime that mirrors the server’s framing/serialization, including a builder with codec and socket options, a dedicated client error type, request/response operations, docs, and end-to-end tests (unit, integration, and cucumber).

Sequence diagram for WireframeClient connect and call flow

sequenceDiagram
    actor Developer
    participant WireframeClientBuilder
    participant ClientCodecConfig
    participant SocketOptions
    participant TcpSocket
    participant LengthDelimitedCodec
    participant WireframeClient
    participant Serializer
    participant Server

    Developer->>WireframeClientBuilder: builder()
    Developer->>WireframeClientBuilder: serializer(BincodeSerializer)
    Developer->>WireframeClientBuilder: codec_config(ClientCodecConfig)
    Developer->>WireframeClientBuilder: socket_options(SocketOptions)
    Developer->>WireframeClientBuilder: connect(addr)

    WireframeClientBuilder->>TcpSocket: new_v4 or new_v6()
    WireframeClientBuilder->>SocketOptions: apply(TcpSocket)
    SocketOptions-->>WireframeClientBuilder: configured socket
    WireframeClientBuilder->>TcpSocket: connect(addr)
    TcpSocket-->>WireframeClientBuilder: TcpStream

    WireframeClientBuilder->>ClientCodecConfig: build_codec()
    ClientCodecConfig-->>WireframeClientBuilder: LengthDelimitedCodec
    WireframeClientBuilder->>LengthDelimitedCodec: wrap TcpStream as Framed
    LengthDelimitedCodec-->>WireframeClientBuilder: Framed
    WireframeClientBuilder-->>Developer: WireframeClient

    Developer->>WireframeClient: call(request)
    WireframeClient->>WireframeClient: send(request)
    WireframeClient->>Serializer: serialize(request)
    Serializer-->>WireframeClient: bytes
    WireframeClient->>LengthDelimitedCodec: send(bytes)
    LengthDelimitedCodec->>Server: TCP frame

    Server-->>LengthDelimitedCodec: response frame
    LengthDelimitedCodec-->>WireframeClient: bytes
    WireframeClient->>Serializer: deserialize(bytes)
    Serializer-->>WireframeClient: response
    WireframeClient-->>Developer: response
Loading

Updated class diagram for Wireframe client runtime types

classDiagram
    class WireframeClient {
        - Framed framed
        - Serializer serializer
        - ClientCodecConfig codec_config
        + send(message) Result
        + receive() Result
        + call(request) Result
        + codec_config() ClientCodecConfig
        + builder() WireframeClientBuilder
    }

    class WireframeClientBuilder {
        - Serializer serializer
        - ClientCodecConfig codec_config
        - SocketOptions socket_options
        + new() WireframeClientBuilder
        + serializer(serializer) WireframeClientBuilder
        + codec_config(codec_config) WireframeClientBuilder
        + max_frame_length(max_frame_length) WireframeClientBuilder
        + length_format(length_format) WireframeClientBuilder
        + socket_options(socket_options) WireframeClientBuilder
        + nodelay(enabled) WireframeClientBuilder
        + keepalive(enabled) WireframeClientBuilder
        + linger(duration) WireframeClientBuilder
        + send_buffer_size(size) WireframeClientBuilder
        + recv_buffer_size(size) WireframeClientBuilder
        + reuseaddr(enabled) WireframeClientBuilder
        + connect(addr) Result
    }

    class ClientCodecConfig {
        - LengthFormat length_format
        - usize max_frame_length
        + max_frame_length(max_frame_length) ClientCodecConfig
        + length_format(length_format) ClientCodecConfig
        + max_frame_length_value() usize
        + length_format_value() LengthFormat
        + build_codec() LengthDelimitedCodec
    }

    class SocketOptions {
        - Option~bool~ nodelay
        - Option~bool~ keepalive
        - Option~LingerSetting~ linger
        - Option~u32~ send_buffer_size
        - Option~u32~ recv_buffer_size
        - Option~bool~ reuseaddr
        - Option~bool~ reuseport
        + nodelay(enabled) SocketOptions
        + keepalive(enabled) SocketOptions
        + linger(duration) SocketOptions
        + send_buffer_size(size) SocketOptions
        + recv_buffer_size(size) SocketOptions
        + reuseaddr(enabled) SocketOptions
        + reuseport(enabled) SocketOptions
        + apply(socket) Result
    }

    class LingerSetting {
        <<enum>>
        Disabled
        Duration
        + to_option() Option
    }

    class ClientError {
        <<enum>>
        Io
        Serialize
        Deserialize
        Disconnected
    }

    class Serializer {
        <<trait>>
        + serialize(message) Result
        + deserialize(bytes) Result
    }

    class BincodeSerializer {
    }

    class LengthFormat {
    }

    class LengthDelimitedCodec {
    }

    class TcpSocket {
    }

    class TcpStream {
    }

    WireframeClientBuilder --> WireframeClient : builds
    WireframeClient --> ClientCodecConfig : uses
    WireframeClient --> Serializer : uses
    WireframeClient --> LengthDelimitedCodec : framed_io
    WireframeClientBuilder --> ClientCodecConfig : configures
    WireframeClientBuilder --> SocketOptions : configures
    SocketOptions --> TcpSocket : applies
    TcpSocket --> TcpStream : connects
    ClientCodecConfig --> LengthDelimitedCodec : builds
    BincodeSerializer ..|> Serializer
    WireframeClient --> ClientError : returns
    WireframeClientBuilder --> ClientError : returns

    ClientError --> Io
    ClientError --> Serialize
    ClientError --> Deserialize
    ClientError --> Disconnected
Loading

File-Level Changes

Change Details Files
Add configurable client codec and TCP socket configuration layer used by the client runtime and builder.
  • Introduce ClientCodecConfig with clamped max_frame_length, configurable LengthFormat (endianness and length bytes), and a builder for LengthDelimitedCodec.
  • Define SocketOptions with fluent setters for TCP_NODELAY, SO_KEEPALIVE, linger, send/recv buffer sizes, SO_REUSEADDR, and SO_REUSEPORT (where supported), plus an apply() helper to configure TcpSocket before connect.
src/client/config.rs
Implement WireframeClientBuilder to construct a configured client over TCP using serializer, codec config, and socket options.
  • Create WireframeClientBuilder with default BincodeSerializer, ClientCodecConfig, and SocketOptions, plus fluent methods for serializer replacement, codec_config, max_frame_length, length_format, and all socket options.
  • Implement async connect(SocketAddr) which creates an appropriately typed TcpSocket, applies SocketOptions, connects, wraps the stream in a LengthDelimitedCodec Framed with reserved read buffer, and returns a WireframeClient populated with serializer and codec_config.
src/client/builder.rs
Implement WireframeClient runtime with async send/receive/call APIs and codec inspection, plus a dedicated ClientError type.
  • Define WireframeClient struct holding Framed<TcpStream, LengthDelimitedCodec>, serializer, and ClientCodecConfig, with Debug that exposes codec_config.
  • Add WireframeClient::builder() that returns a default WireframeClientBuilder, tying runtime and builder together.
  • Implement send() to serialize Message values via the configured Serializer and write bytes into the framed sink, mapping errors into ClientError.
  • Implement receive() to await the next frame, handle EOF as Disconnected, and deserialize into a typed Message, mapping decode errors into ClientError.
  • Implement call() which composes send() and receive() into a request/response helper, and codec_config() accessor for inspection.
  • Introduce ClientError enum with Io (transport/codec), Serialize, Deserialize, and Disconnected variants using thiserror for display and source chaining.
src/client/runtime.rs
src/client/error.rs
Expose the client runtime module and types as part of the public crate API and wire up unit and integration tests for client behavior.
  • Add src/client/mod.rs to wire together builder, config, error, runtime, and expose WireframeClientBuilder, ClientCodecConfig, SocketOptions, ClientError, and WireframeClient; add an internal tests module.
  • Update lib.rs to declare the client module and re-export WireframeClient, ClientCodecConfig, SocketOptions, and ClientError at the crate root for ergonomic imports.
  • Add unit tests for ClientCodecConfig clamping and defaults and for builder-applied TCP_NODELAY using an actual TcpListener/TcpStream pair.
  • Introduce cucumber ClientRuntimeWorld, feature file, and step definitions that spin up a simple echo server, construct a WireframeClient with matching codec settings, send a payload, and assert echo behavior; register the new world in the cucumber main and world modules.
src/client/mod.rs
src/client/tests.rs
tests/worlds/client_runtime.rs
tests/steps/client_steps.rs
tests/features/client_runtime.feature
tests/cucumber.rs
tests/world.rs
tests/worlds/mod.rs
tests/steps/mod.rs
Update documentation and roadmap to describe and reference the new client runtime and mark the connection runtime task as complete.
  • Extend the user guide with a new "Client runtime" section describing WireframeClient, ClientCodecConfig, and SocketOptions usage with a concrete Login/LoginAck example, and cross-reference the implementation files via a new footnote.
  • Update the wireframe client design doc example to use SocketAddr-based connect, builder max_frame_length configuration, by-ref call(&request), and document key implementation decisions (SocketAddr connect, codec clamping, defaults).
  • Mark the roadmap connection runtime item for WireframeClient and builder as complete (checkbox from [ ] to [x]).
docs/users-guide.md
docs/wireframe-client-design.md
docs/roadmap.md

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

@leynos leynos marked this pull request as ready for review December 20, 2025 21:11
sourcery-ai[bot]

This comment was marked as resolved.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/client/config.rs Outdated
…ent builder

- Updated SocketOptions to allow keepalive duration configuration instead of boolean.
- WireframeClientBuilder.keepalive now accepts Option<Duration> to configure keepalive.
- Applied keepalive setting to TCP stream socket options.
- Added unit and integration tests for socket options including keepalive.
- Added client runtime feature test for error reporting on oversized frames.
- Updated usage docs and examples to demonstrate keepalive duration usage.
- Introduced new method to access underlying TcpStream from WireframeClient.

This enables TCP keepalive tuning for clients, improving connection reliability.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Dec 21, 2025

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

src/client/tests.rs

Comment on lines +212 to +228

async fn builder_applies_reuseport_option() {
    let (addr, accept) = spawn_listener().await;

    let client = WireframeClient::builder()
        .reuseport(true)
        .connect(addr)
        .await
        .expect("connect client");

    let sock_ref = SockRef::from(client.tcp_stream());
    assert!(
        sock_ref.reuse_port().expect("query SO_REUSEPORT"),
        "SO_REUSEPORT should be enabled when configured via builder"
    );

    let _server_stream = accept.await.expect("join accept task");
}

❌ New issue: Code Duplication
The module contains 6 functions with similar structure: builder_applies_keepalive_option,builder_applies_linger_option,builder_applies_recv_buffer_size_option,builder_applies_reuseaddr_option and 2 more functions

@leynos

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

Corrected misplaced or inconsistent doc comment lines in src/client/builder.rs and src/client/config.rs to improve example code readability and maintain proper Rust doc comment style.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@coderabbitai

This comment was marked as resolved.

This adds an integration test in `tests/client_runtime.rs` to verify that the wireframe client correctly surfaces errors when sending frames that exceed the server's maximum frame length. It ensures the client receives the appropriate transport or disconnection error upon server rejection of oversized frames.

Additionally, client socket option tests in `src/client/tests.rs` were refactored to use a shared helper function for cleaner and more concise test code.

Minor error handling improvements wrapping serializer errors with Box were also applied in `src/client/runtime.rs`.

Small documentation wording clarifications were made in `docs/users-guide.md`.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

…wlines

Refactored the client_runtime.rs test to consolidate method calls onto single lines, improving readability without changing behavior.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 21, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Checking Comment 1: Error wrapping in src/client/runtime.rs ==="
if [ -f src/client/runtime.rs ]; then
  echo "Checking send method:"
  rg -A 8 "pub async fn send" src/client/runtime.rs | head -20
  echo -e "\nChecking receive method:"
  rg -A 8 "pub async fn receive" src/client/runtime.rs | head -20
else
  echo "File not found: src/client/runtime.rs"
fi

Length of output: 809


🏁 Script executed:

#!/bin/bash
echo "=== Checking Comment 5: Oversized frame test ==="
if [ -f tests/worlds/client_runtime.rs ]; then
  # Check for test that validates error handling for oversized frames
  rg -n "exceeds.*max.*length|oversized.*frame|frame.*too.*large" tests/worlds/client_runtime.rs
  echo -e "\n=== Listing all test functions ==="
  rg -n "^(async )?fn test_|^pub async fn|#\[tokio::test\]" tests/worlds/client_runtime.rs
else
  echo "File not found: tests/worlds/client_runtime.rs"
fi

Length of output: 84


🏁 Script executed:

#!/bin/bash
echo "=== Checking Comment 9: Second-person pronouns in docs/users-guide.md ==="
if [ -f docs/users-guide.md ]; then
  echo "Line 106 context:"
  sed -n '104,108p' docs/users-guide.md | cat -n
  echo -e "\nLine 387 context:"
  sed -n '385,390p' docs/users-guide.md | cat -n
  echo -e "\nSearching for remaining 'you need' occurrences:"
  rg -n "you need|when you" docs/users-guide.md
else
  echo "File not found: docs/users-guide.md"
fi

Length of output: 759


🏁 Script executed:

#!/bin/bash
echo "=== Additional check: Look for integration/unit tests for oversized frames ==="
# Check if there are any tests in the test directory
fd -e rs . tests/ --exec rg -l "exceeds.*max|oversized|frame.*too.*large" {}
# Also check client tests
if [ -f src/client/tests.rs ]; then
  rg -n "exceeds.*max|oversized|frame.*too.*large" src/client/tests.rs
fi

Length of output: 179


🏁 Script executed:

#!/bin/bash
echo "=== Checking tests/client_runtime.rs for oversized frame test ==="
if [ -f tests/client_runtime.rs ]; then
  cat tests/client_runtime.rs
else
  echo "File not found"
fi

Length of output: 1937


🏁 Script executed:

#!/bin/bash
echo "=== Checking tests/steps/client_steps.rs for oversized frame scenarios ==="
if [ -f tests/steps/client_steps.rs ]; then
  # Look for steps related to frame size errors
  rg -n -C 5 "exceeds|oversized|too.*large|frame.*error" tests/steps/client_steps.rs
else
  echo "File not found"
fi

Length of output: 642


🏁 Script executed:

#!/bin/bash
echo "=== Checking feature files for oversized frame scenarios ==="
fd -e feature . tests/ --exec cat {}

Length of output: 6349


@leynos: Excellent news! All 9 comments have now been fully resolved. Here's the comprehensive status:

✅ All Comments Resolved

Previously Unresolved - Now Fixed:

Comment 1 (Error wrapping in src/client/runtime.rs): ✅ RESOLVED

  • Both send and receive methods now properly wrap errors in Box::new():
    .map_err(|e| ClientError::Serialize(Box::new(e)))?;
    .map_err(|e| ClientError::Deserialize(Box::new(e)))?;

Comment 5 (Oversized frame error test): ✅ RESOLVED

  • Added dedicated integration test client_surfaces_error_when_frame_exceeds_server_max_length in tests/client_runtime.rs
  • Also added BDD scenario in tests/features/client_runtime.feature: "Client reports errors when server frame limit is exceeded"
  • Test properly validates that oversized frames (128 bytes with server limit of 64 bytes) surface ClientError::Disconnected or ClientError::Io(_)

Comment 9 (Second-person pronouns in documentation): ✅ RESOLVED

  • Line 387: Changed to "when TCP tuning is required" (removed "you need")
  • Line 106: Changed to "when a different encoding strategy is required" (removed "you need")
  • No remaining second-person pronouns found in the file

Previously Resolved:

Comment 2 ✅ Conservative read buffer allocation implemented
Comment 3build_codec_configures_length_delimited_codec test added
Comment 4 ✅ All socket option tests added (keepalive, linger, buffer sizes, reuseaddr, reuseport)
Comment 6 ✅ Comprehensive unit tests in src/client/tests.rs (228 lines)
Comment 7 ✅ All doc examples in builder.rs use .expect() instead of .unwrap()
Comment 8 ✅ All doc examples in runtime.rs use .expect() instead of .unwrap()

All feedback from the code review has been successfully addressed! 🎉

- Added `socket2` dependency to enable detailed TCP keepalive configuration.
- Updated SocketOptions to use `socket2::TcpKeepalive` for keepalive duration setting.
- Support explicit enable/disable of TCP keepalive with duration control.
- Cleaned up error wrapping in client runtime serialization/deserialization.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

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: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/cucumber.rs (1)

1-24: Update module documentation to include ClientRuntimeWorld.

The module doc comment lists five test suites (lines 4-9) but omits the newly added ClientRuntimeWorld. Add an entry describing the client runtime test suite to maintain documentation accuracy.

🔎 Proposed fix
 //! Cucumber test runner for integration tests.
 //!
-//! Orchestrates five distinct test suites:
+//! Orchestrates six distinct test suites:
 //! - `PanicWorld`: Tests server resilience during connection panics
 //! - `CorrelationWorld`: Tests correlation ID propagation in multi-frame responses
 //! - `StreamEndWorld`: Verifies end-of-stream signalling
 //! - `MultiPacketWorld`: Tests channel-backed multi-packet response delivery
 //! - `FragmentWorld`: Tests fragment metadata enforcement and reassembly primitives
+//! - `ClientRuntimeWorld`: Tests client runtime configuration and framing behaviour
 //!
 //! # Example
 //!
 //! The runner executes feature files sequentially:
 //! ```text
 //! tests/features/connection_panic.feature    -> PanicWorld context
 //! tests/features/correlation_id.feature      -> CorrelationWorld context
 //! tests/features/stream_end.feature          -> StreamEndWorld context
 //! tests/features/multi_packet.feature        -> MultiPacketWorld context
 //! tests/features/fragment.feature            -> FragmentWorld context
+//! tests/features/client_runtime.feature      -> ClientRuntimeWorld context
 //! ```
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 59c4faf and 49f0301.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (19)
  • Cargo.toml (1 hunks)
  • docs/roadmap.md (1 hunks)
  • docs/users-guide.md (3 hunks)
  • docs/wireframe-client-design.md (4 hunks)
  • src/client/builder.rs (1 hunks)
  • src/client/config.rs (1 hunks)
  • src/client/error.rs (1 hunks)
  • src/client/mod.rs (1 hunks)
  • src/client/runtime.rs (1 hunks)
  • src/client/tests.rs (1 hunks)
  • src/lib.rs (2 hunks)
  • tests/client_runtime.rs (1 hunks)
  • tests/cucumber.rs (2 hunks)
  • tests/features/client_runtime.feature (1 hunks)
  • tests/steps/client_steps.rs (1 hunks)
  • tests/steps/mod.rs (1 hunks)
  • tests/world.rs (1 hunks)
  • tests/worlds/client_runtime.rs (1 hunks)
  • tests/worlds/mod.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
docs/**/*.md

📄 CodeRabbit inference engine (docs/contents.md)

Follow the documentation style guide conventions when writing project documentation

docs/**/*.md: Use British English based on the Oxford English Dictionary (en-GB-oxendict), including: -ize suffix in words like 'realize' and 'organization', -lyse suffix in words like 'analyse', -our suffix in words like 'colour', -re suffix in words like 'centre', double 'l' in words like 'cancelled', maintain 'e' in words like 'likeable', -ogue suffix in words like 'catalogue'
The word 'outwith' is acceptable in British English documentation
Keep US spelling when used in an API (for example, 'color')
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns: 'Lille Industries are expanding'
Write headings in sentence case in documentation
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Follow markdownlint recommendations for Markdown files
Always provide a language identifier for fenced code blocks in documentation; use 'plaintext' for non-code text
Use '-' as the first level bullet and renumber lists when items change in documentation
Prefer inline links using 'text' or angle brackets around the URL in documentation
Ensure blank lines before and after bulleted lists and fenced blocks in documentation
Ensure tables have a delimiter line below the header row in documentation
Expand any uncommon acronym on first use in documentation (for example, Continuous Integration (CI))
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with '[^label]' in documentation
Include Mermaid diagrams where they add clarity to documentation
When embedding figures in documentation, use 'alt text' and provide brief alt text describing the content
Add a short description before each Mermaid diagram so screen readers can understand it

Write official documentation with se...

Files:

  • docs/roadmap.md
  • docs/users-guide.md
  • docs/wireframe-client-design.md
docs/**/*.{rs,md}

📄 CodeRabbit inference engine (docs/rust-doctest-dry-guide.md)

Every doctest should validate the public API of a crate from the perspective of an external user, treating each documentation test as a separate temporary crate that imports the library as an external dependency

Files:

  • docs/roadmap.md
  • docs/users-guide.md
  • docs/wireframe-client-design.md
**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

**/*.md: Validate Markdown files using make markdownlint.
Run make fmt after any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by running make nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes in Markdown.

Files:

  • docs/roadmap.md
  • docs/users-guide.md
  • docs/wireframe-client-design.md

⚙️ CodeRabbit configuration file

**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")

  • Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
  • Headings must not be wrapped.
  • Documents must start with a level 1 heading
  • Headings must correctly increase or decrease by no more than one level at a time
  • Use GitHub-flavoured Markdown style for footnotes and endnotes.
  • Numbered footnotes must be numbered by order of appearance in the document.

Files:

  • docs/roadmap.md
  • docs/users-guide.md
  • docs/wireframe-client-design.md
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Run make check-fmt (cargo fmt --workspace -- --check) before committing to validate Rust formatting across the entire workspace without modifying files.
Run make lint (cargo clippy --workspace --all-targets --all-features -- -D warnings) before committing to lint every target with all features enabled and deny all Clippy warnings.
Run make test (cargo test --workspace) before committing to run the full workspace test suite.
Clippy warnings MUST be disallowed. Fix any warnings emitted during tests in the code itself rather than silencing them.
Use make fmt (cargo fmt --workspace) to apply formatting fixes reported by the formatter check.
Where a Rust function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a Rust function has too many parameters, group related parameters in meaningfully named structs.
Where a Rust function is returning a large error, consider using Arc to reduce the amount of data returned.
Write unit and behavioural tests for new Rust functionality. Run both before and after making any change.
Every Rust module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public Rust APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessary mut bindings in Rust code.
Avoid unsafe code in Rust unless absolutely necessary, and document any usage clearly with a 'SAFETY' comment.
Place function attributes after doc comments in Rust code.
Do not use return in single-line Rust functions.
Use predicate functions for Rust conditional criteria with more than two branches.
Lints must not be silenced in Rust except as a last resort. Lint rule suppressions must be tightly scoped and include a clear reason.
Use concat!() to combine long string literals in Rust rather than escaping newlines with a backslash.
Prefer single-line versions of Rust functions where...

Files:

  • tests/worlds/mod.rs
  • tests/world.rs
  • src/lib.rs
  • src/client/tests.rs
  • src/client/error.rs
  • src/client/mod.rs
  • tests/steps/mod.rs
  • tests/client_runtime.rs
  • tests/steps/client_steps.rs
  • tests/cucumber.rs
  • src/client/runtime.rs
  • tests/worlds/client_runtime.rs
  • src/client/builder.rs
  • src/client/config.rs

⚙️ CodeRabbit configuration file

**/*.rs: * Seek to keep the cognitive complexity of functions no more than 9.

  • 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 / -yse / -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.
  • Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
  • 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() in tests.
  • .expect() and .unwrap() are forbidden outside of tests. Errors must be propagated.
  • 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.
  • Environment access (env::set_var and env::remove_var) are always unsafe in Rust 2024 and MUST be marked as such
    • For testing of functionality depending upon environment variables, dependency injection and...

Files:

  • tests/worlds/mod.rs
  • tests/world.rs
  • src/lib.rs
  • src/client/tests.rs
  • src/client/error.rs
  • src/client/mod.rs
  • tests/steps/mod.rs
  • tests/client_runtime.rs
  • tests/steps/client_steps.rs
  • tests/cucumber.rs
  • src/client/runtime.rs
  • tests/worlds/client_runtime.rs
  • src/client/builder.rs
  • src/client/config.rs
Cargo.toml

📄 CodeRabbit inference engine (AGENTS.md)

Cargo.toml: Use explicit version ranges in Cargo.toml and keep dependencies up-to-date.
All crate versions specified in Cargo.toml must use SemVer-compatible caret requirements (e.g., some-crate = "1.2.3"). Wildcard (*) or open-ended inequality (>=) version requirements are strictly forbidden. Tilde requirements (~) should only be used where a dependency must be locked to patch-level updates for a specific, documented reason.

Files:

  • Cargo.toml
🧬 Code graph analysis (7)
src/client/tests.rs (4)
src/frame/format.rs (3)
  • bytes (54-54)
  • endianness (58-58)
  • u32_be (70-70)
src/client/runtime.rs (1)
  • builder (64-64)
src/client/config.rs (3)
  • default (32-37)
  • max_frame_length_value (90-90)
  • length_format_value (104-104)
src/client/builder.rs (2)
  • default (52-52)
  • new (42-48)
src/client/mod.rs (1)
src/client/runtime.rs (1)
  • builder (64-64)
tests/client_runtime.rs (1)
src/client/runtime.rs (1)
  • builder (64-64)
tests/steps/client_steps.rs (2)
src/client/config.rs (1)
  • max_frame_length (54-57)
src/client/builder.rs (1)
  • max_frame_length (109-112)
src/client/runtime.rs (1)
src/client/builder.rs (3)
  • serializer (70-79)
  • new (42-48)
  • codec_config (93-96)
tests/worlds/client_runtime.rs (3)
src/client/config.rs (2)
  • max_frame_length (54-57)
  • default (32-37)
src/client/builder.rs (4)
  • max_frame_length (109-112)
  • new (42-48)
  • codec_config (93-96)
  • default (52-52)
src/client/runtime.rs (2)
  • builder (64-64)
  • codec_config (200-200)
src/client/builder.rs (2)
src/client/config.rs (10)
  • default (32-37)
  • max_frame_length (54-57)
  • length_format (74-77)
  • nodelay (190-193)
  • keepalive (209-215)
  • linger (231-237)
  • send_buffer_size (251-254)
  • recv_buffer_size (268-271)
  • reuseaddr (285-288)
  • reuseport (308-311)
src/client/runtime.rs (1)
  • codec_config (200-200)
🪛 GitHub Actions: CI
src/client/runtime.rs

[error] 98-98: Clippy: redundant closure found. Remove the unnecessary closure at src/client/runtime.rs:98:22.

🪛 GitHub Check: build-test
src/client/runtime.rs

[failure] 135-135:
redundant closure


[failure] 98-98:
redundant closure

🪛 LanguageTool
docs/roadmap.md

[uncategorized] ~295-~295: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...lement WireframeClient and its builder so callers can configure serializers, ...

(COMMA_COMPOUND_SENTENCE_2)

🔍 Remote MCP

Let me try a web search to find relevant technical information about the socket2 crate and best practices for TCP socket configuration that would be applicable to this PR review.## Summary of Relevant Context for Pull Request Review

Based on my research into the technologies and patterns used in this PR, here is the relevant context for reviewing the WireframeClient implementation:

Socket Configuration via socket2

The socket2 crate mirrors raw socket semantics closely, with almost all methods corresponding precisely to one libc or OS API call. The PR uses socket2 to configure TCP socket options, particularly TCP keepalive. On Unix, setting TCP keepalive sets both SO_KEEPALIVE and TCP_KEEPALIVE/TCP_KEEPIDLE options (depending on platform), while on Windows it sets the SIO_KEEPALIVE_VALS option.

Important caveat: On Windows, unconfigured keepalive parameters may be overwritten with defaults, so the function should either be called once per socket or with identical parameters each time.

TCP Socket Configuration in tokio

TcpSocket should only be used directly when the default configuration doesn't meet requirements, and such sockets are typically used together with socket2 to configure socket options not available on TcpSocket.

Length-Delimited Framing

Length-delimited framing prefixes frame data with a frame head specifying frame length, providing utilities that allow consumers to work with entire frames without manual buffering or framing logic. The default format uses a big-endian u32 header with the frame payload.

Key Review Points

Socket Options: The PR's socket option configuration aligns with standard practice, using socket2 for cross-platform TCP keepalive and other low-level options before connection.

Builder Pattern: The fluent builder design allows composition of socket options before connection, which is a common pattern in async Rust for pre-connection configuration.

Frame Length Constraints: LengthDelimitedCodec enforces maximum frame sizes, rejecting frames larger than configured limits, supporting the PR's max_frame_length clamping and validation.

[::web_search::]

⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Sourcery review
🔇 Additional comments (29)
Cargo.toml (1)

47-47: LGTM!

The socket2 dependency addition is appropriate for the client runtime socket configuration features and follows the implicit caret versioning guideline.

src/client/tests.rs (3)

1-46: LGTM!

The module documentation is clear, and the helper functions effectively reduce test duplication as requested in previous reviews. The assert_builder_option helper provides a clean abstraction for testing socket configuration.


48-105: LGTM!

The codec configuration tests comprehensively address the previous review feedback. The build_codec_configures_length_delimited_codec test properly verifies that the configured length field format and endianness are applied to the codec through encode/decode round-tripping and wire-format inspection.


107-218: LGTM!

The socket option tests comprehensively cover all builder configuration options as requested in previous reviews. The use of the shared helper function eliminates duplication, and platform-specific guards for SO_REUSEPORT are correctly applied.

tests/worlds/mod.rs (1)

29-29: LGTM!

The module declaration properly exposes the client runtime world for use in cucumber tests.

tests/world.rs (1)

8-8: LGTM!

The re-export follows the established pattern for exposing test worlds.

tests/steps/mod.rs (1)

6-6: LGTM!

The module declaration properly wires the client step definitions into the cucumber test framework.

tests/cucumber.rs (1)

29-46: LGTM!

The ClientRuntimeWorld is properly imported and integrated into the cucumber test runner.

src/lib.rs (2)

12-12: LGTM!

The client module is properly exposed as part of the public API.


34-34: LGTM!

The client type re-exports follow the established pattern and provide a clean public API surface for the client runtime.

tests/features/client_runtime.feature (1)

1-12: Well-structured BDD scenarios covering both happy and error paths.

The feature file provides good coverage for the client runtime: the first scenario validates normal echo behaviour within frame limits, whilst the second scenario exercises the error path when frames exceed the server's configured maximum. This aligns well with the PR objectives.

src/client/error.rs (1)

1-20: LGTM!

The error enum is well-structured with appropriate variants for transport, serialization, and connection states. Using Box<dyn Error + Send + Sync> for serialization errors provides flexibility across different serializer implementations, and the #[source] attribute ensures proper error chaining.

tests/client_runtime.rs (1)

13-55: Good integration test covering oversized frame behaviour.

The test correctly validates that the client surfaces an appropriate error when sending frames that exceed the server's configured maximum. The assertion covering both Disconnected and Io variants is sensible since the exact error depends on timing and transport behaviour.

docs/wireframe-client-design.md (1)

1-118: Design document accurately reflects the implemented API.

The updates correctly document the API changes: SocketAddr parameter for connect, borrowed request in call(&request), and max_frame_length builder method. The implementation decisions section provides useful context on clamping and defaults.

src/client/mod.rs (1)

1-17: LGTM!

The module structure is clean and follows Rust conventions. The re-exports provide a well-organised public API surface that aligns with the crate's top-level exports.

tests/steps/client_steps.rs (1)

1-35: LGTM!

The step definitions are appropriately thin, delegating to the world's methods for test logic. This keeps the step definitions focused on mapping Gherkin syntax to test actions whilst maintaining good separation of concerns.

src/client/runtime.rs (2)

32-44: Good use of finish_non_exhaustive() in Debug implementation.

This correctly signals that the struct has additional fields not shown in the debug output, which is appropriate for omitting the framed stream and serializer.


173-179: Clean composition of send and receive.

The call method elegantly composes the two lower-level operations, providing a convenient request-response pattern for callers.

tests/worlds/client_runtime.rs (4)

35-57: LGTM!

The echo server setup correctly binds to an ephemeral port, spawns an async task with appropriate early-return handling, and stores the address and handle for later verification.


64-72: LGTM!

The client connection correctly retrieves the server address, configures the codec, and establishes the connection via the builder pattern with proper error propagation.


79-106: LGTM!

Both payload methods correctly implement the test scenarios—send_payload for successful echo and send_payload_expect_error for the oversized frame rejection case mentioned in the PR objectives.


112-144: LGTM!

The verification methods correctly validate echoed payloads and error variants, with proper server task cleanup. The duplicated cleanup logic is acceptable given the test context and clarity benefits.

src/client/config.rs (4)

11-120: LGTM!

ClientCodecConfig correctly implements frame length clamping between 64 bytes and 16 MiB, and build_codec() properly configures the LengthDelimitedCodec with the specified endianness and length field format.


139-175: LGTM!

The SocketOptions struct correctly uses platform-conditional compilation for reuseport, and the internal LingerSetting/KeepAliveSetting enums appropriately model the tri-state configuration (unset/disabled/duration).


177-311: LGTM!

The SocketOptions builder methods are consistent, properly annotated with #[must_use], and include appropriate platform guards for reuseport.


313-352: LGTM!

The apply() method correctly configures all socket options, including the two-step TCP keepalive setup via socket2::SockRef and TcpKeepalive that addresses the earlier API mismatch concern.

src/client/builder.rs (3)

24-53: LGTM!

The builder struct correctly uses a default type parameter for the serializer, and the Default implementation properly delegates to new().


55-267: LGTM!

The builder methods correctly delegate to the underlying ClientCodecConfig and SocketOptions types, and the generic type transformation in serializer() is properly implemented.


288-309: LGTM!

The connect() method correctly handles IPv4/IPv6 socket creation, applies socket options before connection, and uses a conservative initial buffer allocation (capped at 64 KiB) as per the resolved performance concern. The doc example appropriately uses .expect().

Comment thread docs/roadmap.md
Comment thread docs/users-guide.md
Comment thread src/client/runtime.rs
Comment thread src/client/runtime.rs
Comment thread src/client/tests.rs Outdated
Comment thread src/client/tests.rs Outdated
Comment thread tests/worlds/client_runtime.rs Outdated
…clarity

Refactored the SocketOptions struct's apply method to delegate to
individual methods for each socket option (e.g., nodelay, keepalive,
linger) improving readability and maintenance. Added corresponding
unit tests to verify socket option behaviors. Also improved error
handling simplification and minor related test updates.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

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: 4

♻️ Duplicate comments (2)
docs/roadmap.md (1)

290-297: Add comma before coordinating conjunction.

Insert a comma before "so" on line 295 when it connects two independent clauses.

🔎 Proposed fix
-  - [x] Implement `WireframeClient` and its builder so callers can configure
+  - [x] Implement `WireframeClient` and its builder, so callers can configure
     serializers, codec settings (including `max_frame_length` parity), and
     socket options before connecting.
docs/users-guide.md (1)

390-425: Add no_run attribute to the code example.

This code example connects to 127.0.0.1:7878, which will fail at doc-test time since no server is running. Mark the code block with no_run to prevent doc-test failures whilst preserving syntax validation.

🔎 Proposed fix
-```rust
+```rust,no_run
 use std::{net::SocketAddr, time::Duration};
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 49f0301 and c5a83f0.

📒 Files selected for processing (8)
  • docs/roadmap.md
  • docs/users-guide.md
  • src/client/config.rs
  • src/client/runtime.rs
  • src/client/tests.rs
  • tests/connection.rs
  • tests/cucumber.rs
  • tests/worlds/client_runtime.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Run make check-fmt (cargo fmt --workspace -- --check) before committing to validate Rust formatting across the entire workspace without modifying files.
Run make lint (cargo clippy --workspace --all-targets --all-features -- -D warnings) before committing to lint every target with all features enabled and deny all Clippy warnings.
Run make test (cargo test --workspace) before committing to run the full workspace test suite.
Clippy warnings MUST be disallowed. Fix any warnings emitted during tests in the code itself rather than silencing them.
Use make fmt (cargo fmt --workspace) to apply formatting fixes reported by the formatter check.
Where a Rust function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a Rust function has too many parameters, group related parameters in meaningfully named structs.
Where a Rust function is returning a large error, consider using Arc to reduce the amount of data returned.
Write unit and behavioural tests for new Rust functionality. Run both before and after making any change.
Every Rust module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public Rust APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessary mut bindings in Rust code.
Avoid unsafe code in Rust unless absolutely necessary, and document any usage clearly with a 'SAFETY' comment.
Place function attributes after doc comments in Rust code.
Do not use return in single-line Rust functions.
Use predicate functions for Rust conditional criteria with more than two branches.
Lints must not be silenced in Rust except as a last resort. Lint rule suppressions must be tightly scoped and include a clear reason.
Use concat!() to combine long string literals in Rust rather than escaping newlines with a backslash.
Prefer single-line versions of Rust functions where...

Files:

  • tests/connection.rs
  • tests/cucumber.rs
  • src/client/tests.rs
  • src/client/runtime.rs
  • tests/worlds/client_runtime.rs
  • src/client/config.rs

⚙️ CodeRabbit configuration file

**/*.rs: * Seek to keep the cognitive complexity of functions no more than 9.

  • 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 / -yse / -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.
  • Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
  • 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() in tests.
  • .expect() and .unwrap() are forbidden outside of tests. Errors must be propagated.
  • 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.
  • Environment access (env::set_var and env::remove_var) are always unsafe in Rust 2024 and MUST be marked as such
    • For testing of functionality depending upon environment variables, dependency injection and...

Files:

  • tests/connection.rs
  • tests/cucumber.rs
  • src/client/tests.rs
  • src/client/runtime.rs
  • tests/worlds/client_runtime.rs
  • src/client/config.rs
docs/**/*.md

📄 CodeRabbit inference engine (docs/contents.md)

Follow the documentation style guide conventions when writing project documentation

docs/**/*.md: Use British English based on the Oxford English Dictionary (en-GB-oxendict), including: -ize suffix in words like 'realize' and 'organization', -lyse suffix in words like 'analyse', -our suffix in words like 'colour', -re suffix in words like 'centre', double 'l' in words like 'cancelled', maintain 'e' in words like 'likeable', -ogue suffix in words like 'catalogue'
The word 'outwith' is acceptable in British English documentation
Keep US spelling when used in an API (for example, 'color')
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns: 'Lille Industries are expanding'
Write headings in sentence case in documentation
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Follow markdownlint recommendations for Markdown files
Always provide a language identifier for fenced code blocks in documentation; use 'plaintext' for non-code text
Use '-' as the first level bullet and renumber lists when items change in documentation
Prefer inline links using 'text' or angle brackets around the URL in documentation
Ensure blank lines before and after bulleted lists and fenced blocks in documentation
Ensure tables have a delimiter line below the header row in documentation
Expand any uncommon acronym on first use in documentation (for example, Continuous Integration (CI))
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with '[^label]' in documentation
Include Mermaid diagrams where they add clarity to documentation
When embedding figures in documentation, use 'alt text' and provide brief alt text describing the content
Add a short description before each Mermaid diagram so screen readers can understand it

Write official documentation with se...

Files:

  • docs/users-guide.md
  • docs/roadmap.md
docs/**/*.{rs,md}

📄 CodeRabbit inference engine (docs/rust-doctest-dry-guide.md)

Every doctest should validate the public API of a crate from the perspective of an external user, treating each documentation test as a separate temporary crate that imports the library as an external dependency

Files:

  • docs/users-guide.md
  • docs/roadmap.md
**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

**/*.md: Validate Markdown files using make markdownlint.
Run make fmt after any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by running make nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes in Markdown.

Files:

  • docs/users-guide.md
  • docs/roadmap.md

⚙️ CodeRabbit configuration file

**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")

  • Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
  • Headings must not be wrapped.
  • Documents must start with a level 1 heading
  • Headings must correctly increase or decrease by no more than one level at a time
  • Use GitHub-flavoured Markdown style for footnotes and endnotes.
  • Numbered footnotes must be numbered by order of appearance in the document.

Files:

  • docs/users-guide.md
  • docs/roadmap.md
🧬 Code graph analysis (5)
tests/connection.rs (2)
src/app/frame_handling.rs (1)
  • record (31-47)
wireframe_testing/src/logging.rs (1)
  • logger (82-82)
src/client/tests.rs (3)
src/frame/format.rs (3)
  • bytes (54-54)
  • endianness (58-58)
  • u32_be (70-70)
src/client/config.rs (7)
  • default (32-37)
  • max_frame_length_value (90-90)
  • length_format_value (104-104)
  • keepalive (209-215)
  • linger (231-237)
  • send_buffer_size (251-254)
  • recv_buffer_size (268-271)
src/client/builder.rs (6)
  • default (52-52)
  • new (42-48)
  • keepalive (176-179)
  • linger (194-197)
  • send_buffer_size (210-213)
  • recv_buffer_size (226-229)
src/client/runtime.rs (1)
src/client/builder.rs (3)
  • serializer (70-79)
  • new (42-48)
  • codec_config (93-96)
tests/worlds/client_runtime.rs (3)
src/client/config.rs (2)
  • max_frame_length (54-57)
  • default (32-37)
src/client/builder.rs (4)
  • max_frame_length (109-112)
  • new (42-48)
  • codec_config (93-96)
  • default (52-52)
src/client/runtime.rs (2)
  • builder (64-64)
  • codec_config (200-200)
src/client/config.rs (2)
src/client/builder.rs (11)
  • default (52-52)
  • max_frame_length (109-112)
  • length_format (125-128)
  • nodelay (158-161)
  • keepalive (176-179)
  • linger (194-197)
  • send_buffer_size (210-213)
  • recv_buffer_size (226-229)
  • reuseaddr (242-245)
  • reuseport (264-267)
  • new (42-48)
src/client/runtime.rs (1)
  • builder (64-64)
🪛 LanguageTool
docs/roadmap.md

[uncategorized] ~290-~290: Do not mix variants of the same word (‘serialisation’ and ‘serialization’) within a single text.
Context: ...time that mirrors the server's framing, serialisation, and lifecycle layers, so both sides sh...

(EN_WORD_COHERENCY)


[uncategorized] ~295-~295: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...lement WireframeClient and its builder so callers can configure serializers, ...

(COMMA_COMPOUND_SENTENCE_2)

⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Sourcery review
  • GitHub Check: build-test
🔇 Additional comments (26)
tests/cucumber.rs (2)

1-38: LGTM!

Module documentation correctly updated to reflect six test suites, and the import structure is clean.


40-48: LGTM!

The new ClientRuntimeWorld test suite is correctly wired into the sequential test runner.

tests/connection.rs (1)

209-238: LGTM!

The addition of correlation filtering strengthens the test by ensuring assertions only apply to logs with the expected correlation_id. This prevents false matches against unrelated log entries.

docs/users-guide.md (3)

15-35: LGTM!

The no_run attribute correctly prevents doc-test execution for this example that requires a runtime context.


101-107: LGTM!

The wording changes correctly remove second-person pronouns whilst maintaining clarity.


672-673: LGTM!

The footnote correctly references the implementation files for the client runtime.

src/client/tests.rs (7)

1-16: LGTM!

Module documentation, imports, and constants are appropriate for the test module.


18-28: LGTM!

The spawn_listener helper is clean and uses .expect() appropriately for test assertions.


30-46: LGTM!

The assert_builder_option helper effectively reduces test duplication whilst maintaining readability. Proper cleanup is ensured by awaiting the accept task.


48-105: LGTM!

The codec configuration tests comprehensively cover clamping behaviour, defaults, and actual codec round-trip encoding/decoding. The build_codec_configures_length_delimited_codec test addresses the past review request for validating LengthDelimitedCodec configuration.


107-133: LGTM!

The nodelay and keepalive socket option tests correctly verify the options are applied to the underlying TCP socket via the builder.


135-184: LGTM!

The linger and buffer size tests correctly verify socket options. The >= comparison for buffer sizes is appropriate since the OS may adjust requested values upward.


186-220: LGTM!

The reuseaddr test is straightforward, and the reuseport test correctly includes platform guards to exclude unsupported operating systems.

src/client/runtime.rs (6)

1-14: LGTM!

Module documentation and imports are appropriately structured.


16-44: LGTM!

The WireframeClient struct is well-designed with appropriate visibility for fields. The Debug implementation sensibly excludes I/O types whilst exposing the codec configuration.


46-65: LGTM!

The builder() method correctly provides a specialised entry point for the default serializer, with appropriate #[must_use] annotation.


127-137: LGTM!

The receive method correctly handles connection closure by returning ClientError::Disconnected. The same error type verification applies as noted for send.


139-221: LGTM!

The call method cleanly composes send and receive. The codec_config and tcp_stream accessors are appropriately designed, with const fn where possible and #[must_use] annotations.


94-101: The error type compatibility is correct. The Serializer::serialize() method returns Result<Vec<u8>, Box<dyn Error + Send + Sync>>, which matches the type expected by ClientError::Serialize. No error wrapping or type conversion is required; the code compiles and functions correctly as written.

tests/worlds/client_runtime.rs (2)

1-23: LGTM on structure and World setup.

The module doc comment is present, the doc comment correctly precedes the derive attributes, and the ClientRuntimeWorld struct is well-organised with appropriate optional fields for tracking test state.


64-73: LGTM on client connection setup.

The builder usage and codec configuration follow the expected pattern. Error propagation is correct.

src/client/config.rs (5)

15-120: LGTM on ClientCodecConfig implementation.

The struct is well-documented with examples, uses fluent setters with #[must_use], and provides const accessors. The build_codec method correctly maps LengthFormat to the underlying codec configuration.


139-154: LGTM on SocketOptions struct definition.

The platform-conditional reuseport field is correctly guarded with #[cfg] attributes for supported Unix platforms.


177-311: LGTM on SocketOptions setter methods.

All setters follow a consistent fluent pattern with #[must_use], and each is documented with examples. The reuseport method correctly applies the same #[cfg] guard as the field.


313-328: Decomposition into helper methods addresses complexity concern.

The apply method delegates to single-responsibility helpers, keeping the cyclomatic complexity manageable. The past CodeScene warning about complexity 9 (at threshold) is mitigated by this structure.


337-352: LGTM on keepalive handling.

The implementation correctly uses socket.set_keepalive(bool) to enable/disable keepalive, then applies the duration via socket2::SockRef and TcpKeepalive. This resolves the previously flagged type mismatch.

Comment thread src/client/config.rs Outdated
Comment thread src/client/config.rs
Comment thread tests/worlds/client_runtime.rs
Comment thread tests/worlds/client_runtime.rs
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Dec 23, 2025

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

src/client/config.rs

Comment on lines +313 to +352

    pub(crate) fn apply(&self, socket: &TcpSocket) -> io::Result<()> {
        if let Some(enabled) = self.nodelay {
            socket.set_nodelay(enabled)?;
        }
        if let Some(keepalive) = self.keepalive {
            match keepalive {
                KeepAliveSetting::Disabled => {
                    socket.set_keepalive(false)?;
                }
                KeepAliveSetting::Duration(duration) => {
                    socket.set_keepalive(true)?;
                    let sock_ref = SockRef::from(socket);
                    let config = TcpKeepalive::new().with_time(duration);
                    sock_ref.set_tcp_keepalive(&config)?;
                }
            }
        }
        if let Some(linger) = self.linger {
            socket.set_linger(linger.to_option())?;
        }
        if let Some(size) = self.send_buffer_size {
            socket.set_send_buffer_size(size)?;
        }
        if let Some(size) = self.recv_buffer_size {
            socket.set_recv_buffer_size(size)?;
        }
        if let Some(enabled) = self.reuseaddr {
            socket.set_reuseaddr(enabled)?;
        }
        #[cfg(all(
            unix,
            not(target_os = "solaris"),
            not(target_os = "illumos"),
            not(target_os = "cygwin"),
        ))]
        if let Some(enabled) = self.reuseport {
            socket.set_reuseport(enabled)?;
        }
        Ok(())
    }

❌ New issue: Complex Method
SocketOptions.apply has a cyclomatic complexity of 9, threshold = 9

@coderabbitai

This comment was marked as resolved.

- Moved ClientCodecConfig from src/client/config.rs to a new
  module src/client/codec_config.rs for better separation of concerns.
- Updated src/client/mod.rs to reflect new module structure.
- Removed codec-related code from src/client/config.rs which now
  focuses solely on socket options.
- Adjusted tests and imports accordingly.

This refactoring improves code organization and maintainability.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Dec 23, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 23, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant