Add MessageAssembler hook trait, headers, tests, and app integration

[leynos]

Summary

• Fully implemented the MessageAssembler hook trait and per-frame header models under wireframe::message_assembler.
• Added a new module wireframe::message_assembler with header types and a minimal trait; publicly re-exported for end users.
• WireframeApp builder updated to accept and expose a MessageAssembler instance via with_message_assembler and message_assembler.
• Implemented unit tests for header parsing and error handling in src/message_assembler/tests.rs; added cucumber-based behavioural tests scaffolding (worlds/steps/features) to exercise the API surface.
• ExecPlan documentation added at docs/execplans/8-2-1-message-assembler-hook-trait.md; user guide and roadmap references updated to reflect the surface and integration plan.

Changes

Public API

• New module wireframe::message_assembler containing:
  • pub trait MessageAssembler: Send + Sync + 'static with parse_frame_header(&self, payload: &[u8]) -> Result<ParsedFrameHeader, std::io::Error>.
  • Public header models:
    • pub enum FrameHeader { First(FirstFrameHeader), Continuation(ContinuationFrameHeader) }
    • pub struct FirstFrameHeader { pub message_key: MessageKey, pub metadata_len: usize, pub body_len: usize, pub total_body_len: Option<usize>, pub is_last: bool }
    • pub struct ContinuationFrameHeader { pub message_key: MessageKey, pub sequence: Option<FrameSequence>, pub body_len: usize, pub is_last: bool }
  • Newtypes:
    • pub struct MessageKey(pub u64);
    • pub struct FrameSequence(pub u32);
  • pub struct ParsedFrameHeader { header: FrameHeader, header_len: usize } with constructor and accessors.
• Re-exports: wireframe::message_assembler publicly via crate root exports.

Builder / Runtime Integration

• Updated src/app/builder.rs to support an optional message assembler:
  • Added field: message_assembler: Option<Arc<dyn MessageAssembler>>
  • Threaded through builder transitions and defaults.
  • Added builder methods:
    • with_message_assembler(...) -> Self
    • message_assembler() -> Option<Arc<dyn MessageAssembler>>
• Ensures future runtime integration points can invoke the assembler during inbound framing.

Testing & Documentation

• Scaffolding replaced with implemented tests:
  • Unit tests for header parsing and error paths in src/message_assembler/tests.rs.
  • Cucumber / behavioural tests scaffolding with features, worlds, and steps under tests/ (including tests/features/message_assembler.feature, tests/worlds/message_assembler.rs, and tests/steps/message_assembler_steps.rs).
• ExecPlan documentation added at docs/execplans/8-2-1-message-assembler-hook-trait.md describing purpose, decisions, and progress.
• Documentation updates planned/partially applied in docs/users-guide.md and docs/roadmap.md to reflect surface and caveats around inbound wiring (8.2.5 target for full integration).

Artifacts

• New directory/module: src/message_assembler/ with mod.rs and header.rs.
• Exposed in public crate surface via pub mod message_assembler and re-exports.
• Updated roadmap and design references to reflect the new interface.

Validation and Acceptance

• Public MessageAssembler trait and header types compile with examples.
• Unit tests covering first/continuation parsing and error paths.
• Cucumber/behavioural tests scaffolding wired to the new API surface.
• Documentation updates reflect the new interface and usage notes.
• Build, lint, and test gates pass locally.

📎 Task: https://www.terragonlabs.com/task/2c810261-b92d-441c-aa45-872cae5665c1

Summary by Sourcery

Introduce a public MessageAssembler hook and header model, integrate it into the WireframeApp builder, and add tests and docs to support protocol-specific multi-frame parsing.

New Features:

• Add wireframe::message_assembler module exposing the MessageAssembler trait and frame header types for protocol-level message assembly.
• Allow WireframeApp to accept and expose an optional MessageAssembler via builder methods for future inbound pipeline integration.

Enhancements:

• Re-export message assembler types from the crate root to provide a stable public API surface.

Documentation:

• Document the MessageAssembler hook, header model, and configuration flow in the user guide, design docs, exec plan, and roadmap.

Tests:

• Add unit tests and Cucumber-based behavioural tests to validate frame header parsing, error handling, and message assembler behaviour.

[coderabbitai]

Summary by CodeRabbit

• New Features

  • Added a message assembler hook interface so apps can register custom per-frame header parsing for multi-frame messages.
  • Builder API extended to configure and retrieve the message assembler.

• Documentation

  • Added user guide, design docs and roadmap updates describing the assembler hook and usage examples.

• Tests

  • End-to-end and unit test suites expanded with feature scenarios and step definitions validating header parsing and registration.

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

Walkthrough

Introduce a public MessageAssembler trait and typed per-frame header model; wire an optional Arc through the WireframeApp builder; add test-only parsing helpers, unit and Cucumber tests, feature docs and test-helpers exports to support protocol-agnostic frame-header parsing.

Changes

Cohort / File(s)	Summary
Documentation & Planning docs/adr-002-streaming-requests-and-shared-message-assembly.md, docs/execplans/8-2-1-message-assembler-hook-trait.md, docs/generic-message-fragmentation-and-re-assembly-design.md, docs/multi-packet-and-streaming-responses-design.md, docs/roadmap.md, docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md, docs/users-guide.md	Add design, execplan and user-guide text describing the MessageAssembler hook, public surface, registration via WireframeApp::with_message_assembler, and roadmap/task status updates.
Header model & Core Trait src/message_assembler/mod.rs, src/message_assembler/header.rs	Add MessageAssembler trait with parse_frame_header(&[u8]) -> Result<ParsedFrameHeader, io::Error>; introduce MessageKey, FrameSequence, FrameHeader (First/Continuation), FirstFrameHeader, ContinuationFrameHeader, and ParsedFrameHeader types with constructors and accessors.
Builder Integration src/app/builder.rs	Add message_assembler: Option<Arc<dyn MessageAssembler>> to WireframeApp; implement with_message_assembler() and message_assembler(); thread assembler through rebuild_with_params, with_codec, with_serializer, and on_connection_setup.
Crate Exports src/lib.rs	Add pub mod message_assembler; re-export MessageAssembler, FrameHeader, FirstFrameHeader, ContinuationFrameHeader, MessageKey, FrameSequence, ParsedFrameHeader; add test_helpers module behind feature flag and crate alias.
Test Helpers & Parsing src/test_helpers.rs, src/message_assembler/tests.rs	Add test-only TestAssembler and parse_frame_header implementation with safe byte reads, flag interpretation and errors; add unit tests covering parsing, header lengths and error cases.
Cucumber Tests & Worlds tests/features/message_assembler.feature, tests/steps/message_assembler_steps.rs, tests/steps/mod.rs, tests/worlds/message_assembler.rs, tests/worlds/mod.rs, tests/world.rs, tests/cucumber.rs	Add feature, step implementations and MessageAssemblerWorld with header-spec builders and assertions; integrate world into test runner.
Configuration Cargo.toml	Add local dev-dependency on wireframe with test-helpers feature; add test-helpers feature and enable it for tests.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Builder as WireframeApp Builder
    participant Assembler as MessageAssembler Hook
    participant Frame as Inbound Frame

    App->>Builder: with_message_assembler(impl)
    Builder->>Builder: store Arc<dyn MessageAssembler>
    Builder->>Builder: thread through transformations
    Note over Builder: with_codec / with_serializer / on_connection_setup
    Builder-->>App: configured WireframeApp

    Note over Frame: Inbound frame arrives
    Frame->>Assembler: parse_frame_header(payload)
    Assembler->>Assembler: read kind, flags, message_key
    alt First frame (kind == 0x01)
        Assembler->>Assembler: parse metadata_len, body_len, optional total_body_len
    else Continuation (kind == 0x02)
        Assembler->>Assembler: parse body_len, optional sequence
    else Unknown
        Assembler-->>Frame: return io::Error (InvalidData)
    end
    Assembler-->>Frame: Result<ParsedFrameHeader, io::Error>

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~28 minutes

Poem

✨ Add the trait, then stitch the frames with care,
Builder holds the hook and threads it everywhere,
Tests step through the stories, headers checked and true,
Keys and sequences line up, bytes reveal their cue,
Reassemble the message — neatly, frame by frame.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarises the main changes: introduction of MessageAssembler hook trait, header types, tests, and integration into the WireframeApp builder.
Description check	✅ Passed	The description comprehensively covers the public API surface, builder integration, testing approach, and documentation updates—all changes present in 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-messageassembler-hook-rmwz3c

---

📜 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 19798f4 and 61be42f.

📒 Files selected for processing (1)

• tests/worlds/message_assembler.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.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/message_assembler.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/message_assembler.rs

🧠 Learnings (1)

📚 Learning: 2026-01-03T17:44:28.792Z

Learnt from: leynos
Repo: leynos/wireframe PR: 412
File: tests/streaming_request.rs:85-107
Timestamp: 2026-01-03T17:44:28.792Z
Learning: In Rust code (e.g., the wireframe project), avoid direct indexing like vec[0] because it can panic. Prefer safe accessors that return Option, such as vec.get(0), vec.first(), or patterns that handle Option safely (e.g., match or if let Some). This should be applied across Rust files, including tests (e.g., tests/streaming_request.rs), regardless of prior length checks, to ensure safety and avoid panics from out-of-bounds access.

Applied to files:

• tests/worlds/message_assembler.rs

🧬 Code graph analysis (1)

tests/worlds/message_assembler.rs (4)

src/message_assembler/header.rs (6)

• new (109-109)
• header (134-134)
• from (13-13)
• from (17-17)
• from (29-29)
• from (33-33)

src/frame/format.rs (1)

• bytes (54-54)

src/app/builder.rs (2)

• message_assembler (368-370)
• new (107-107)

src/message_assembler/tests.rs (1)

• parse_header (185-189)

⏰ 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 (1)

tests/worlds/message_assembler.rs (1)

1-374: Clean test world implementation—previous refinements properly applied.

This file demonstrates solid test infrastructure:

• Module and public APIs thoroughly documented
• Consistent error-handling patterns throughout
• Assertion helpers properly consolidated (assert_common_field for shared fields, assert_header_field for type-specific extraction)
• Encoding duplication eliminated via set_payload_with_header
• HeaderEnvelope and DebugDisplay abstractions are clean and focused
• All functions remain within reasonable complexity bounds
• Bit manipulation, length conversions, and pattern matching are correct

The file adheres to coding guidelines (module doc, no unsafe, no panics, proper Result propagation, file length within limits) and incorporates feedback from prior review rounds.

---

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

[sourcery-ai]

Reviewer's Guide

Implements a new public MessageAssembler hook and header model, wires it into the WireframeApp builder API, and adds unit/behavioural tests plus documentation and roadmap updates for protocol-level message assembly.

Sequence diagram for registering a MessageAssembler via WireframeApp builder

sequenceDiagram
    actor Developer
    participant WireframeAppBuilder as WireframeApp
    participant DemoAssembler as DemoMessageAssembler

    Developer->>WireframeAppBuilder: new()
    activate WireframeAppBuilder
    WireframeAppBuilder-->>Developer: WireframeApp { message_assembler: None }
    deactivate WireframeAppBuilder

    Developer->>DemoAssembler: implement MessageAssembler
    activate DemoAssembler
    DemoAssembler-->>Developer: DemoMessageAssembler instance
    deactivate DemoAssembler

    Developer->>WireframeAppBuilder: with_message_assembler(DemoMessageAssembler)
    activate WireframeAppBuilder
    WireframeAppBuilder->>WireframeAppBuilder: store Arc_dyn_MessageAssembler in message_assembler
    WireframeAppBuilder-->>Developer: updated WireframeApp
    deactivate WireframeAppBuilder

    Developer->>WireframeAppBuilder: message_assembler()
    activate WireframeAppBuilder
    WireframeAppBuilder-->>Developer: Option_Arc_dyn_MessageAssembler(Some)
    deactivate WireframeAppBuilder

Loading

Class diagram for message_assembler hook and header model

classDiagram
    namespace message_assembler {
        class MessageKey {
            +u64 0
            +from_u64(value u64)
            +into_u64() u64
        }

        class FrameSequence {
            +u32 0
            +from_u32(value u32)
            +into_u32() u32
        }

        class FirstFrameHeader {
            +MessageKey message_key
            +usize metadata_len
            +usize body_len
            +Option_usize total_body_len
            +bool is_last
        }

        class ContinuationFrameHeader {
            +MessageKey message_key
            +Option_FrameSequence sequence
            +usize body_len
            +bool is_last
        }

        class FrameHeader {
            <<enum>>
            +First(FirstFrameHeader)
            +Continuation(ContinuationFrameHeader)
        }

        class ParsedFrameHeader {
            -FrameHeader header
            -usize header_len
            +new(header FrameHeader, header_len usize) ParsedFrameHeader
            +header() &FrameHeader
            +header_len() usize
            +into_header() FrameHeader
        }

        class MessageAssembler {
            <<trait>>
            +parse_frame_header(payload &[u8]) Result_ParsedFrameHeader_io_Error
        }
    }

    class WireframeApp {
        +message_assembler Option_Arc_dyn_MessageAssembler
        +with_message_assembler(assembler MessageAssembler) WireframeApp
        +message_assembler() Option_Arc_dyn_MessageAssembler
    }

    MessageKey <.. FirstFrameHeader : uses
    MessageKey <.. ContinuationFrameHeader : uses
    FrameSequence <.. ContinuationFrameHeader : uses

    FirstFrameHeader <|-- FrameHeader : First
    ContinuationFrameHeader <|-- FrameHeader : Continuation

    FrameHeader <.. ParsedFrameHeader : contains

    MessageAssembler <.. ParsedFrameHeader : returns

    MessageAssembler <.. WireframeApp : stored_as_Arc_dyn

Loading

File-Level Changes

Change	Details	Files
Introduce a public message_assembler module with trait, header types, and parsed header wrapper, re-exported from the crate root.	Add src/message_assembler/mod.rs defining the MessageAssembler trait and re-exporting header types Add src/message_assembler/header.rs with MessageKey, FrameSequence, FrameHeader, FirstFrameHeader, ContinuationFrameHeader, and ParsedFrameHeader including constructors and accessors Re-export message_assembler module and its types from src/lib.rs for public consumption	src/message_assembler/mod.rs src/message_assembler/header.rs src/lib.rs
Extend WireframeApp builder to carry an optional MessageAssembler instance and expose configuration/accessors.	Add message_assembler: Option<Arc> field to WireframeApp and initialize it in Default Thread message_assembler through rebuild_with_params and builder transitions, preserving it across codec/serializer rebuilds Add with_message_assembler(self, impl MessageAssembler + 'static) -> Self to install an assembler Add message_assembler(&self) -> Option<Arc> accessor cloning the Arc	src/app/builder.rs
Add unit tests for header parsing plus Cucumber-based behavioural tests and world/step wiring for message assembler behaviour.	Implement TestAssembler-based parsing tests in src/message_assembler/tests.rs validating first/continuation headers, header_len accounting, and error cases Add MessageAssemblerWorld, header spec helpers, and parsing/assertion helpers under tests/worlds/message_assembler.rs Add Cucumber steps in tests/steps/message_assembler_steps.rs and a new feature file tests/features/message_assembler.feature Wire MessageAssemblerWorld and steps into the existing cucumber test harness and world exports	src/message_assembler/tests.rs tests/worlds/message_assembler.rs tests/steps/message_assembler_steps.rs tests/features/message_assembler.feature tests/worlds/mod.rs tests/world.rs tests/steps/mod.rs tests/cucumber.rs
Document the MessageAssembler hook, its design, and update roadmap and design docs to reference the new interface and progress.	Extend docs/users-guide.md with a section explaining the MessageAssembler hook, example implementation, and current non-integrated status Update ADR and design docs to show the concrete trait and header API surface and how it composes with fragmentation/streaming designs Mark roadmap items 8.2.1 and 8.2.2 as complete in docs/roadmap.md and mention the hook surface in capability-maturity docs Add an ExecPlan document detailing the plan, progress, decisions, and validation for the MessageAssembler hook trait work	docs/users-guide.md docs/adr-002-streaming-requests-and-shared-message-assembly.md docs/generic-message-fragmentation-and-re-assembly-design.md docs/multi-packet-and-streaming-responses-design.md docs/roadmap.md docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md docs/execplans/8-2-1-message-assembler-hook-trait.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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[leynos]

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

Code Duplication

Location:

src/message_assembler/tests.rs:
The module contains 4 functions with similar structure: parse_continuation_header_with_sequence,parse_continuation_header_without_sequence,parse_first_frame_header_with_total_and_last,parse_first_frame_header_without_total

tests/steps/message_assembler_steps.rs:
The module contains 4 functions with similar structure: given_continuation_header,given_continuation_header_with_sequence,given_first_header,given_first_header_with_total

tests/worlds/message_assembler.rs:
The module contains 6 functions with similar structure: MessageAssemblerWorld.assert_body_len,MessageAssemblerWorld.assert_is_last,MessageAssemblerWorld.assert_message_key,MessageAssemblerWorld.assert_metadata_len and 2 more functions

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

[leynos]

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

tests/worlds/message_assembler.rs

Comment on lines +184 to +197

    pub fn assert_metadata_len(&self, expected: usize) -> TestResult {
        let parsed = self.parsed.as_ref().ok_or("no parsed header")?;
        let FrameHeader::First(header) = parsed.header() else {
            return Err("expected first header".into());
        };
        if header.metadata_len != expected {
            return Err(format!(
                "expected metadata length {expected}, got {}",
                header.metadata_len
            )
            .into());
        }
        Ok(())
    }

❌ New issue: Code Duplication
The module contains 3 functions with similar structure: MessageAssemblerWorld.assert_metadata_len,MessageAssemblerWorld.assert_sequence,MessageAssemblerWorld.assert_total_len

[leynos]

@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/message_assembler/tests.rs

Comment on lines +75 to +157

fn parse_frame_headers() {
    let cases = vec![
        HeaderCase {
            name: "first frame without total",
            build_payload: Box::new(|| {
                build_first_header_payload(FirstHeaderSpec {
                    flags: 0b0,
                    message_key: 9,
                    metadata_len: 2,
                    body_len: 12,
                    total_body_len: None,
                })
            }),
            expected_header: FrameHeader::First(FirstFrameHeader {
                message_key: MessageKey(9),
                metadata_len: 2,
                body_len: 12,
                total_body_len: None,
                is_last: false,
            }),
        },
        HeaderCase {
            name: "first frame with total and last",
            build_payload: Box::new(|| {
                build_first_header_payload(FirstHeaderSpec {
                    flags: 0b11,
                    message_key: 42,
                    metadata_len: 0,
                    body_len: 8,
                    total_body_len: Some(64),
                })
            }),
            expected_header: FrameHeader::First(FirstFrameHeader {
                message_key: MessageKey(42),
                metadata_len: 0,
                body_len: 8,
                total_body_len: Some(64),
                is_last: true,
            }),
        },
        HeaderCase {
            name: "continuation frame with sequence",
            build_payload: Box::new(|| {
                build_continuation_header_payload(ContinuationHeaderSpec {
                    flags: 0b10,
                    message_key: 7,
                    body_len: 16,
                    sequence: Some(3),
                })
            }),
            expected_header: FrameHeader::Continuation(ContinuationFrameHeader {
                message_key: MessageKey(7),
                sequence: Some(FrameSequence(3)),
                body_len: 16,
                is_last: false,
            }),
        },
        HeaderCase {
            name: "continuation frame without sequence",
            build_payload: Box::new(|| {
                build_continuation_header_payload(ContinuationHeaderSpec {
                    flags: 0b1,
                    message_key: 11,
                    body_len: 5,
                    sequence: None,
                })
            }),
            expected_header: FrameHeader::Continuation(ContinuationFrameHeader {
                message_key: MessageKey(11),
                sequence: None,
                body_len: 5,
                is_last: true,
            }),
        },
    ];

    for case in cases {
        let payload = (case.build_payload)();
        let parsed = parse_header(&payload);
        assert_eq!(parsed.header(), &case.expected_header, "case: {}", case.name);
        assert_eq!(parsed.header_len(), payload.len(), "case: {}", case.name);
    }
}

❌ New issue: Large Method
parse_frame_headers has 82 lines, threshold = 70

[leynos]

@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/message_assembler/tests.rs

Comment on lines +185 to +260

fn build_test_cases() -> Vec<HeaderCase> {
    vec![
        HeaderCase {
            name: "first frame without total",
            build_payload: Box::new(|| {
                build_first_header_payload(FirstHeaderSpec {
                    flags: 0b0,
                    message_key: 9,
                    metadata_len: 2,
                    body_len: 12,
                    total_body_len: None,
                })
            }),
            expected_header: FrameHeader::First(FirstFrameHeader {
                message_key: MessageKey(9),
                metadata_len: 2,
                body_len: 12,
                total_body_len: None,
                is_last: false,
            }),
        },
        HeaderCase {
            name: "first frame with total and last",
            build_payload: Box::new(|| {
                build_first_header_payload(FirstHeaderSpec {
                    flags: 0b11,
                    message_key: 42,
                    metadata_len: 0,
                    body_len: 8,
                    total_body_len: Some(64),
                })
            }),
            expected_header: FrameHeader::First(FirstFrameHeader {
                message_key: MessageKey(42),
                metadata_len: 0,
                body_len: 8,
                total_body_len: Some(64),
                is_last: true,
            }),
        },
        HeaderCase {
            name: "continuation frame with sequence",
            build_payload: Box::new(|| {
                build_continuation_header_payload(ContinuationHeaderSpec {
                    flags: 0b10,
                    message_key: 7,
                    body_len: 16,
                    sequence: Some(3),
                })
            }),
            expected_header: FrameHeader::Continuation(ContinuationFrameHeader {
                message_key: MessageKey(7),
                sequence: Some(FrameSequence(3)),
                body_len: 16,
                is_last: false,
            }),
        },
        HeaderCase {
            name: "continuation frame without sequence",
            build_payload: Box::new(|| {
                build_continuation_header_payload(ContinuationHeaderSpec {
                    flags: 0b1,
                    message_key: 11,
                    body_len: 5,
                    sequence: None,
                })
            }),
            expected_header: FrameHeader::Continuation(ContinuationFrameHeader {
                message_key: MessageKey(11),
                sequence: None,
                body_len: 5,
                is_last: true,
            }),
        },
    ]
}

❌ New issue: Large Method
build_test_cases has 76 lines, threshold = 70

[leynos]

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

Code Duplication

tests/steps/message_assembler_steps.rs:

What lead to degradation?

The module contains 4 functions with similar structure: given_continuation_header,given_continuation_header_with_sequence,given_first_header,given_first_header_with_total

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read all relevant files to check resolution status

echo "=== src/message_assembler/tests.rs ==="
cat src/message_assembler/tests.rs

echo -e "\n\n=== tests/worlds/message_assembler.rs ==="
cat tests/worlds/message_assembler.rs

echo -e "\n\n=== src/app/builder.rs (message_assembler method only) ==="
rg -n -A 10 "pub fn message_assembler" src/app/builder.rs

echo -e "\n\n=== docs/execplans/8-2-1-message-assembler-hook-trait.md (ADR references) ==="
rg -n -C 2 "ADR" docs/execplans/8-2-1-message-assembler-hook-trait.md

echo -e "\n\n=== tests/features/message_assembler.feature ==="
cat tests/features/message_assembler.feature

echo -e "\n\n=== Check for shared test helper ==="
if [ -f tests/common/message_assembler.rs ]; then
    echo "Found tests/common/message_assembler.rs"
    cat tests/common/message_assembler.rs
else
    echo "No tests/common/message_assembler.rs found"
fi

# Check if there's a test_helpers module
echo -e "\n\n=== Check for test_helpers in src ==="
if [ -f src/test_helpers.rs ]; then
    echo "Found src/test_helpers.rs"
    cat src/test_helpers.rs
else
    echo "No src/test_helpers.rs found"
fi

echo -e "\n\n=== Check src/lib.rs for test_helpers module ==="
rg -n "test_helpers" src/lib.rs || echo "No test_helpers reference in lib.rs"

echo -e "\n\n=== Check tests/common/mod.rs ==="
if [ -f tests/common/mod.rs ]; then
    cat tests/common/mod.rs
else
    echo "No tests/common/mod.rs found"
fi

Length of output: 25080

---

@leynos, yes! All the review comments have been resolved. Here's the status of each:

✅ Overall Comment 1: TestAssembler duplication

RESOLVED – A shared helper module exists at tests/common/message_assembler.rs with both TestAssembler and parse_frame_header. Both src/message_assembler/tests.rs and tests/worlds/message_assembler.rs import this shared implementation, ensuring unit and behavioural tests exercise identical parsing logic.

✅ Overall Comment 2: Arc cloning in message_assembler()

RESOLVED – The signature is now pub fn message_assembler(&self) -> Option<&Arc<dyn MessageAssembler>> (line 368), returning a reference to the Arc rather than cloning it.

✅ Comment 1: unknown_header_kind_errors test validity

RESOLVED – The test now properly constructs a payload with sufficient bytes for kind (0xff), flags (0x00), and message_key (u64) using BytesMut (lines 136–147). It also asserts the error message: assert_eq!(err.to_string(), "unknown header kind");

✅ Comment 2: Tests with body bytes after header

RESOLVED – The parameterised test now appends body bytes [0xaa, 0xbb, 0xcc] to each test case and validates that:

• The parsed header remains identical
• header_len covers only the header region
• header_len < payload_with_body.len() holds (lines 103–116)

✅ Comment 3: TestAssembler duplication (duplicate of Overall 1)

RESOLVED – See Overall Comment 1 above.

✅ Comment 4: Unit tests exist

RESOLVED – src/message_assembler/tests.rs contains comprehensive unit tests:

• Parameterised tests using rstest for 4 frame types
• Body byte validation for each case
• Error path tests (short_header_errors, unknown_header_kind_errors)
• Helper functions for payload construction

✅ Comment 5: Behavioural tests implemented

RESOLVED – tests/features/message_assembler.feature includes 6 complete scenarios:

1. Builder exposes a configured message assembler
2. Parsing first frame without total length
3. Parsing first frame with total length
4. Parsing continuation with sequence
5. Parsing continuation without sequence
6. Invalid header payload error handling

These cover the builder API (with_message_assembler, message_assembler()) and validate end-to-end parsing behaviour.

✅ Comment 6: ADR acronym expansion

RESOLVED – Line 88 of docs/execplans/8-2-1-message-assembler-hook-trait.md now reads: integration (see Architecture Decision Record (ADR) 0002). The acronym is expanded on first use.

---

Summary: All 8 comments (6 individual + 2 overall) have been successfully addressed. The code now has proper test coverage, shared test utilities, efficient API design, and clear documentation.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI Agents

In @src/message_assembler/tests.rs:
- Around line 8-10: The TestAssembler helper is imported via #[path] from
tests/common which creates a backward dependency; move the implementation into
src/test_helpers.rs (or src/test_helpers/message_assembler.rs) and gate it with
#[cfg(test)], add a #[cfg(test)] pub mod test_helpers; declaration in src/lib.rs
so the helper is exported, then replace the #[path =
"../../tests/common/message_assembler.rs"] usage in
src/message_assembler/tests.rs with a crate-relative import like use
crate::test_helpers::TestAssembler; and update all integration tests under
tests/ to import with use wireframe::test_helpers::TestAssembler;, remove the
old tests/common/message_assembler.rs file, and run make test to validate.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1d2aa2 and 2e8fc88.

📒 Files selected for processing (1)

• src/message_assembler/tests.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.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/message_assembler/tests.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/message_assembler/tests.rs

🧠 Learnings (1)

📚 Learning: 2026-01-03T17:44:28.792Z

Learnt from: leynos
Repo: leynos/wireframe PR: 412
File: tests/streaming_request.rs:85-107
Timestamp: 2026-01-03T17:44:28.792Z
Learning: In Rust code (e.g., the wireframe project), avoid direct indexing like vec[0] because it can panic. Prefer safe accessors that return Option, such as vec.get(0), vec.first(), or patterns that handle Option safely (e.g., match or if let Some). This should be applied across Rust files, including tests (e.g., tests/streaming_request.rs), regardless of prior length checks, to ensure safety and avoid panics from out-of-bounds access.

Applied to files:

• src/message_assembler/tests.rs

🧬 Code graph analysis (1)

src/message_assembler/tests.rs (3)

src/frame/format.rs (1)

• bytes (54-54)

tests/worlds/message_assembler.rs (1)

• parse_header (188-209)

src/message_assembler/header.rs (3)

• header (134-134)
• header_len (159-159)
• new (109-109)

🔍 Remote MCP Deepwiki

Excellent! Now I have comprehensive information about the existing patterns. Let me summarize the relevant context found:

Summary of Additional Context for PR Review

Based on the leynos/wireframe repository architecture, here is the relevant context to help review this PR:

1. Existing Builder Pattern Consistency

The PR's integration of MessageAssembler into WireframeApp follows established patterns in the codebase:

• Hook Registration Pattern: The codebase uses a consistent builder pattern for adding protocol-level hooks via methods like with_protocol, on_connection_setup, and on_connection_teardown. The new with_message_assembler(...) method aligns with this convention.

• Configuration Threading: The builder transparently threads configuration through state transformations (via rebuild_with_params), which the PR correctly implements for message_assembler across codec and serializer pathway transitions. This matches how existing fields like push_dlq and fragmentation are propagated.

• Getter Methods: The pattern of exposing getters (e.g., message_assembler()) to retrieve configured hooks is consistent with existing builder APIs.

2. Test Architecture Alignment

The PR's multi-layered testing approach (unit tests, Cucumber features, step definitions, and world objects) follows the repository's established testing strategy:

• Cucumber Framework: The project uses Cucumber with Gherkin syntax for behavioral tests, organized into:

  • Feature files (tests/features/*.feature)
  • World objects for test state (tests/worlds/*.rs)
  • Step definitions (tests/steps/*.rs)
  • Test runner integration (tests/cucumber.rs)

• Test Isolation: Each scenario gets its own World instance, consistent with the MessageAssemblerWorld approach in the PR.

• Multi-layered Testing: The combination of unit tests (src/message_assembler/tests.rs), integration tests (world/steps), and behavioral specifications is the established pattern for validating new features in wireframe.

3. Trait-Based Protocol Hooks

The MessageAssembler trait follows the repository's design philosophy of protocol customization through Send + Sync + 'static traits (similar to WireframeProtocol, FrameProcessor, and Serializer). This enables pluggable protocol implementations without runtime overhead.

4. Module Organization

The new src/message_assembler/ module structure (with mod.rs, header.rs, and tests.rs) aligns with the crate's organizational patterns for feature modules.

⏰ 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 (1)

src/message_assembler/tests.rs (1)

22-192: Excellent test coverage and parameterization.

The rstest-based parameterized tests comprehensively cover first-frame and continuation-frame variants (with/without optional fields), validate header-length accuracy for both header-only and body-appended payloads (addressing previous feedback about header_len < payload.len()), and properly assert error kind and message for both short-header and unknown-kind paths. The helper specs and builders are clear and maintainable.

All previous review feedback has been addressed:

• ✅ rstest migration completed
• ✅ Error message assertions included (lines 128, 143)
• ✅ Body bytes scenario tested (lines 100-117)
• ✅ TestAssembler duplication eliminated via shared import

[leynos]

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

Code Duplication

tests/worlds/message_assembler.rs:

What lead to degradation?

The module contains 2 functions with similar structure: MessageAssemblerWorld.assert_continuation_field,MessageAssemblerWorld.assert_first_field

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI Agents

In @src/message_assembler/tests.rs:
- Around line 1-18: CI reports rustfmt formatting diffs in this test file
(imports and use-grouping near the top including statements like use std::io;
and the multi-item import from super { ContinuationFrameHeader,
FirstFrameHeader, ... }). Fix by running the workspace formatter (e.g., run make
fmt or cargo fmt --workspace) to apply rustfmt changes, stage the updated
file(s), and re-run tests/CI to ensure formatting issues are resolved before
committing.

In @tests/worlds/message_assembler.rs:
- Around line 1-19: The file has CI formatting violations around the
module/header and the use/import block (the top-of-file items including the
module doc comment, cfg attribute, and imports such as BytesMut, BytesMut,
cucumber::World, and MessageAssembler); run the repository formatter (make fmt
or cargo fmt) to reformat this test file so the module doc, attributes, and use
statements comply with rustfmt rules and commit the resulting changes.
- Around line 85-120: The two helpers assert_first_field and
assert_continuation_field are duplicated; replace them with a single generic
assert_header_field<T, F>(&self, field_name: &str, expected: &T, extractor: F)
where T: PartialEq + fmt::Display + Copy and F: FnOnce(&FrameHeader) ->
Result<T, &'static str>, which retrieves parsed, calls
extractor(parsed.header()) and maps Err(&str) to the same error strings
("expected first header" / "expected continuation header") and the comparison
error ("expected {field_name} {expected}, got {actual}") exactly as before; then
update all call sites (assert_metadata_len, assert_total_len, assert_sequence)
to pass closures that pattern-match the FrameHeader, return Ok(field) on success
or Err("expected first header") / Err("expected continuation header") on
mismatch; keep TestResult returns and error messages unchanged and run tests.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e8fc88 and aa272ca.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (5)

• Cargo.toml
• src/lib.rs
• src/message_assembler/tests.rs
• src/test_helpers.rs
• tests/worlds/message_assembler.rs

🧰 Additional context used

📓 Path-based instructions (2)

**/*.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/lib.rs
• src/message_assembler/tests.rs
• src/test_helpers.rs
• tests/worlds/message_assembler.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/lib.rs
• src/message_assembler/tests.rs
• src/test_helpers.rs
• tests/worlds/message_assembler.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

🧠 Learnings (1)

📚 Learning: 2026-01-03T17:44:28.792Z

Learnt from: leynos
Repo: leynos/wireframe PR: 412
File: tests/streaming_request.rs:85-107
Timestamp: 2026-01-03T17:44:28.792Z
Learning: In Rust code (e.g., the wireframe project), avoid direct indexing like vec[0] because it can panic. Prefer safe accessors that return Option, such as vec.get(0), vec.first(), or patterns that handle Option safely (e.g., match or if let Some). This should be applied across Rust files, including tests (e.g., tests/streaming_request.rs), regardless of prior length checks, to ensure safety and avoid panics from out-of-bounds access.

Applied to files:

• src/lib.rs
• src/message_assembler/tests.rs
• src/test_helpers.rs
• tests/worlds/message_assembler.rs

🧬 Code graph analysis (3)

src/lib.rs (1)

src/app/builder.rs (1)

• message_assembler (368-370)

src/message_assembler/tests.rs (2)

tests/worlds/message_assembler.rs (1)

• parse_header (186-207)

src/message_assembler/header.rs (3)

• header (134-134)
• header_len (159-159)
• new (109-109)

tests/worlds/message_assembler.rs (1)

src/message_assembler/header.rs (8)

• fmt (21-21)
• fmt (37-37)
• header (134-134)
• new (109-109)
• from (13-13)
• from (17-17)
• from (29-29)
• from (33-33)

🪛 GitHub Actions: CI

src/message_assembler/tests.rs

[warning] 5-5: cargo fmt --check reported formatting changes needed (diff detected in test file).

---

[warning] 16-16: cargo fmt --check reported formatting changes needed (diff detected in test file).

tests/worlds/message_assembler.rs

[warning] 5-5: cargo fmt --check reported formatting changes needed (diff detected in world tests).

---

[warning] 18-18: cargo fmt --check reported formatting changes needed (diff detected in world tests).

🪛 GitHub Check: build-test

src/message_assembler/tests.rs

[warning] 16-16:
Diff in /home/runner/work/wireframe/wireframe/src/message_assembler/tests.rs

---

[warning] 5-5:
Diff in /home/runner/work/wireframe/wireframe/src/message_assembler/tests.rs

tests/worlds/message_assembler.rs

[warning] 18-18:
Diff in /home/runner/work/wireframe/wireframe/tests/worlds/message_assembler.rs

---

[warning] 5-5:
Diff in /home/runner/work/wireframe/wireframe/tests/worlds/message_assembler.rs

---

[warning] 18-18:
Diff in /home/runner/work/wireframe/wireframe/tests/worlds/message_assembler.rs

---

[warning] 5-5:
Diff in /home/runner/work/wireframe/wireframe/tests/worlds/message_assembler.rs

🔍 Remote MCP Deepwiki

Summary of additional, review-relevant facts found

• WireframeApp follows an established fluent builder pattern for protocol hooks and threads configuration through rebuild helpers; adding message_assembler via the same approach matches existing design (WireframeApp builder, with_protocol, on_connection_setup patterns).

• The codebase already uses trait-based protocol hooks constrained by Send + Sync + 'static (WireframeProtocol, before_send/on_connection_setup/etc.); MessageAssembler as a trait hook aligns with this extensibility model.

• Testing strategy is multi-layered (unit tests, rstest fixtures, wireframe_testing helpers, and Cucumber BDD). New unit tests + Cucumber worlds/steps should integrate with existing test utilities (wireframe_testing drive_with_*, LoggerHandle, rstest fixtures) and follow established patterns (world per scenario, feature/steps/worlds layout).

• The repo provides a test-only helpers pattern via cfg(any(test, feature = "test-helpers")) and a wireframe_testing crate with drive_with_bincode/drive_with_frame helpers — consolidating TestAssembler parsing logic into test helpers (as suggested in PR comments) would follow existing conventions and reduce duplication.

• ConnectionActor and protocol hook points (on_connection_setup, before_send, on_command_end) are the established integration surfaces for per-connection behavior; the builder's message_assembler field will need eventual propagation into connection-time hooks consistent with how WireframeProtocol is wired.

• CI/dev expectations: commits must include unit and behavioural tests, zero Clippy warnings, and formatting checks — add/modify tests and public API should respect those gates (make test, make lint, make check-fmt).

Sources/tools used

• Deepwiki_read_wiki_structure (leynos/wireframe) — repo page index listing relevant docs.
• Deepwiki_read_wiki_contents (leynos/wireframe) — detailed pages on WireframeApp, protocol hooks, testing, testing utilities, and CI/roadmap referenced above.

⏰ 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 (14)

Cargo.toml (1)

52-52: LGTM: Test-helpers feature integration is correct.

The self-referential dev-dependency with the test-helpers feature and the corresponding feature flag definition follow established patterns for exposing test utilities. The structure allows test code to import wireframe::test_helpers while keeping production builds clean.

Also applies to: 89-89

src/lib.rs (2)

7-7: LGTM: Self-reference declaration.

The extern crate self as wireframe; declaration enables internal code to reference the crate uniformly, which is particularly useful for feature-gated modules and test code.

---

27-27: LGTM: Module structure and re-exports.

The public message_assembler module, feature-gated test_helpers module, and comprehensive re-exports correctly expose the new API surface whilst keeping test utilities behind the appropriate feature gate.

Also applies to: 39-40, 66-74

src/message_assembler/tests.rs (3)

20-116: LGTM: Comprehensive parameterised test coverage.

The rstest-based test structure provides excellent coverage of header parsing scenarios, including both header-only payloads and payloads with appended body bytes. The assertions correctly verify that header_len covers only the header region regardless of trailing data.

---

118-142: LGTM: Error path validation.

Both error tests correctly verify the error kind and message string, ensuring clear diagnostics when parsing fails.

---

144-190: LGTM: Well-structured test helpers.

The spec structs and payload builders provide a clean abstraction for test case construction, whilst the parse_header helper appropriately delegates to the shared TestAssembler implementation.

src/test_helpers.rs (4)

1-3: LGTM: Appropriate feature gating.

The module-level feature gate correctly restricts this code to test and test-helpers builds, preventing inclusion in production artifacts.

---

18-26: LGTM: Clean test assembler implementation.

The TestAssembler struct appropriately delegates to the shared parse_frame_header function, enabling consistent parsing logic across unit and BDD tests.

---

28-80: LGTM: Robust header parsing implementation.

The parsing logic correctly handles both header kinds with appropriate error checking, safe byte extraction, and accurate header_len computation. The use of helper functions ensures bounds checking without panicking.

---

82-111: LGTM: Safe byte extraction helpers.

The helper functions consistently enforce bounds checking before extracting values, preventing panics from insufficient buffer data.

tests/worlds/message_assembler.rs (4)

20-46: LGTM: Clear test specification structs.

The FirstHeaderSpec and ContinuationHeaderSpec structs provide well-documented interfaces for constructing test headers in Cucumber scenarios.

---

48-69: LGTM: Well-structured Cucumber world.

The MessageAssemblerWorld struct appropriately manages test state, and the custom Debug implementation prevents excessive output from the opaque WireframeApp.

---

122-336: LGTM: Comprehensive world implementation.

The world methods provide thorough coverage of header construction, parsing, and assertion scenarios required for Cucumber tests. The builder integration methods correctly wire the TestAssembler into the test app.

---

338-343: LGTM: Utility wrapper for display formatting.

The DebugDisplay wrapper elegantly enables Display-bounded generic helpers to work with types that only implement Debug, particularly useful for Option<T> assertions.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI Agents

In @tests/worlds/message_assembler.rs:
- Around line 1-335: The file fails rustfmt checks; run the project's formatter
(e.g. `make fmt` or `cargo fmt`) to reformat tests/worlds/message_assembler.rs
and commit the changes so the file (including structs like FirstHeaderSpec,
ContinuationHeaderSpec and impl MessageAssemblerWorld and DebugDisplay) matches
the repository's rustfmt style.
- Around line 105-154: Both set_first_header and set_continuation_header
duplicate header encoding logic; extract that into a private helper
encode_header(&mut self, kind: u8, build_flags: impl FnOnce() -> u8, key: u64,
encode_body: impl FnOnce(&mut BytesMut) -> Result<(), &'static str>) ->
TestResult that creates a BytesMut, writes the kind byte, calls build_flags to
compute and write the flags byte, writes the key, calls encode_body to append
variant-specific length/optional fields (returning any validation errors),
stores bytes.to_vec() into self.payload and returns Ok(()). Refactor
set_first_header to call encode_header with kind 0x01 and a closure that encodes
metadata_len, body_len and optional total_len (preserving the existing try_from
error messages), and refactor set_continuation_header to call encode_header with
kind 0x02 and a closure that encodes body_len and optional sequence; keep all
existing validation logic and error strings intact.

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa272ca and d155fdc.

📒 Files selected for processing (1)

• tests/worlds/message_assembler.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.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/message_assembler.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/message_assembler.rs

🧠 Learnings (1)

📚 Learning: 2026-01-03T17:44:28.792Z

Learnt from: leynos
Repo: leynos/wireframe PR: 412
File: tests/streaming_request.rs:85-107
Timestamp: 2026-01-03T17:44:28.792Z
Learning: In Rust code (e.g., the wireframe project), avoid direct indexing like vec[0] because it can panic. Prefer safe accessors that return Option, such as vec.get(0), vec.first(), or patterns that handle Option safely (e.g., match or if let Some). This should be applied across Rust files, including tests (e.g., tests/streaming_request.rs), regardless of prior length checks, to ensure safety and avoid panics from out-of-bounds access.

Applied to files:

• tests/worlds/message_assembler.rs

🧬 Code graph analysis (1)

tests/worlds/message_assembler.rs (3)

src/message_assembler/header.rs (2)

• new (109-109)
• header (134-134)

src/app/builder.rs (2)

• message_assembler (368-370)
• new (107-107)

src/message_assembler/tests.rs (1)

• parse_header (186-190)

🪛 GitHub Actions: CI

tests/worlds/message_assembler.rs

[error] 1-1: cargo fmt check failed: Diff in source files detected. Run 'cargo fmt' (or 'cargo fmt --all') to format the code.

🪛 GitHub Check: build-test

tests/worlds/message_assembler.rs

[warning] 16-16:
Diff in /home/runner/work/wireframe/wireframe/tests/worlds/message_assembler.rs

---

[warning] 5-5:
Diff in /home/runner/work/wireframe/wireframe/tests/worlds/message_assembler.rs

---

[warning] 16-16:
Diff in /home/runner/work/wireframe/wireframe/tests/worlds/message_assembler.rs

---

[warning] 5-5:
Diff in /home/runner/work/wireframe/wireframe/tests/worlds/message_assembler.rs

🔍 Remote MCP Deepwiki

Summary of additional repo context relevant to this PR (short, review-focused)

• Design & builder pattern alignment: WireframeApp already follows the project's fluent builder pattern and threads protocol hooks through rebuild helpers; adding an Option<Arc> on the builder matches established patterns used for other hooks (with_protocol, on_connection_setup) — see WireframeApp documentation and builder notes.

• Protocol hook model: The project uses trait-based protocol hooks (WireframeProtocol) constrained by Send + Sync + 'static and invokes hooks from ConnectionActor at on_connection_setup, before_send, on_command_end. The new MessageAssembler trait (Send + Sync + 'static) fits this extensibility model.

• Frame/header parsing & FrameMetadata precedent: The codebase already uses FrameMetadata parsing (parse header w/o full deserialization) and LengthPrefixedProcessor; the new ParsedFrameHeader / header types follow that optimisation and will integrate with existing header-first routing patterns.

• Testing conventions & helpers:

  • The repo uses a layered testing strategy (unit, integration, cucumber BDD) and provides wireframe_testing helpers (drive_with_bincode/drive_with_frame) and a test-helpers cfg(feature) pattern. The added TestAssembler + parse_frame_header in test_helpers aligns with existing test-helper practices but duplication should be checked against wireframe_testing helpers to avoid redundant code.
  • Cucumber/world/steps layout is the established behavioural test pattern; new feature/World/steps files should follow the same isolation and fixtures (World per scenario, logger/runtime fixtures).

• Connection actor & runtime impact:

  • ConnectionActor uses a biased tokio::select! loop and invokes protocol hooks before sending frames. Any future runtime integration of MessageAssembler (when used at connection time) must preserve that ordering and respect shutdown/fairness semantics.

• Production/resilience expectations: CI and repo quality gates require unit + behavioural tests, zero Clippy warnings, and formatting checks. New public API additions (pub module and re-exports) must be documented and satisfy these gates.

References

• Repo design, WireframeApp, protocol hooks, FrameMetadata, testing, and CI details: Deepwiki generated docs for leynos/wireframe — Overview, Core Architecture, WireframeApp, Message Processing Pipeline, Testing, and CI/CD sections.

⏰ 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 (1)

tests/worlds/message_assembler.rs (1)

83-98: Excellent consolidation of assertion helpers.

The generic assert_header_field helper successfully consolidates the previously duplicated type-specific assertion methods. The extractor closure approach (returning Result<T, &'static str>) elegantly combines type-checking and field extraction whilst maintaining clear error messages.

[leynos]

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

tests/worlds/message_assembler.rs

Comment on lines +70 to +81

    fn assert_common_field<T, F>(&self, field: &str, expected: &T, extractor: F) -> TestResult
    where
        T: PartialEq + fmt::Display + Copy,
        F: FnOnce(&FrameHeader) -> T,
    {
        let parsed = self.parsed.as_ref().ok_or("no parsed header")?;
        let actual = extractor(parsed.header());
        if actual != *expected {
            return Err(format!("expected {field} {expected}, got {actual}").into());
        }
        Ok(())
    }

❌ New issue: Code Duplication
The module contains 5 functions with similar structure: MessageAssemblerWorld.assert_common_field,MessageAssemblerWorld.assert_header_field,MessageAssemblerWorld.assert_metadata_len,MessageAssemblerWorld.assert_sequence and 1 more functions

[leynos]

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

tests/worlds/message_assembler.rs

Comment on lines +236 to +244

    pub fn assert_metadata_len(&self, expected: usize) -> TestResult {
        self.assert_header_field("metadata length", &expected, |header| {
            if let FrameHeader::First(header) = header {
                Ok(header.metadata_len)
            } else {
                Err("expected first header")
            }
        })
    }

❌ New issue: Code Duplication
The module contains 3 functions with similar structure: MessageAssemblerWorld.assert_metadata_len,MessageAssemblerWorld.assert_sequence,MessageAssemblerWorld.assert_total_len

[leynos]

@coderabbitai review

[coderabbitai]

✅ 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.