Export testkit utilities behind wireframe::testkit feature (Stage A)

[leynos]

Summary

• Implement Stage A for 8.5.4: export the phase-8.5 testkit utilities as a root surface behind a dedicated Cargo feature named testkit and gate it behind the feature, i.e. wireframe::testkit.
• Introduce a root testkit surface and in-crate driving helpers, reassembly/assertion utilities, and slow-I/O helpers behind the testkit feature. This includes root-level integration and behavioural tests referencing the new surface.
• Retain wireframe_testing as a compatibility facade to preserve source compatibility while migrating helpers into the root surface in future steps.
• Update ExecPlan and ADR references, and align with the 8.5.4 roadmap item.
• Update user-facing docs to describe wireframe::testkit and the testkit feature gating, while maintaining existing wireframe_testing usage.

Changes

• Root crate
  • Add a new Cargo feature testkit:
    [features]
    testkit = []
  • Gate the new public surface behind the feature in src/lib.rs:

    #[cfg(feature = "testkit")]
    pub mod testkit;

• Root module surface
  • Introduced a new src/testkit/ suite with modules for fragment driving, partial-frame driving, reassembly assertions, slow-I/O helpers, result types, and shared support.
  • Exported the testkit surface via wireframe::testkit behind the testkit feature, including:
    • Partial frame/fragment driving helpers
    • Slow I/O helpers and pacing configurations
    • Reassembly and message-assembly assertion primitives
    • Shared TestResult / TestError types
• Tests & integration
  • Added rstest and rstest-bdd based integration tests that target the new root surface.
  • Added testkit-specific fixtures and scenarios (tests/features/testkit_export.feature, tests/fixtures/testkit_export.rs, etc.).
• Documentation
  • ExecPlan: Added docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md (new).
  • ADR references updated to reflect the testkit surface behind a feature gate and compatibility facade strategy.
  • docs/roadmap.md: Mark 8.5.4 as completed.
  • docs/users-guide.md: Updated to show wireframe::testkit exposure behind the testkit feature and updated usage examples.
• Compatibility facade
  • wireframe_testing Cargo.toml updated to enable the new testkit feature on the main crate (wireframe = { path = "..", features = ["test-support", "pool", "testkit"] }).
  • wireframe_testing public exports still compile and route through the new root testkit surface.

Rationale

• Cargo cycles: Exporting wireframe_testing directly from the main crate would create a cycle. By gating a dedicated testkit surface behind a feature, we provide a first-party opt-in path while avoiding cycles.
• Stabilization: Existing wireframe_testing behavior remains available, preserving compatibility as we migrate helpers into the root testkit surface in future steps.
• Traceability: The change aligns with ADR 0002 and the 8.5.4 roadmap, enabling staged migration with focused tests and docs.

Plan / Milestones

• Stage A (this PR): Establish gated surface and feature flag; add documentation plan. Implement root wireframe::testkit and gate behind testkit.
• Stage B–G (subsequent PRs): Move 8.5 helpers into wireframe::testkit, convert wireframe_testing exports to a compatibility facade, add rstest/rstest-bdd coverage, and update ADRs/docs.

Testing plan

• Local builds:
  • cargo build --features testkit
  • cargo test --features testkit
• Verification:
  • Ensure wireframe_testing consumers compile without enabling testkit.
  • Validate that wireframe::testkit surface compiles and tests exercise the new API.
• Documentation/tests:
  • Validate that docs reflect the new surface and gating.

Acceptance criteria

• wireframe::testkit exists only with feature = "testkit".
• The exported surface covers the phase-8.5 helper families planned for 8.5.4.
• wireframe_testing remains source-compatible for the covered helpers.
• Focused tests (rstest/rstest-bdd) reference wireframe::testkit surface.
• Documentation reflects when to use wireframe::testkit vs wireframe_testing.
• docs/roadmap.md marks 8.5.4 as done.

Risks & mitigations

• Risk: potential cargo cycles if re-export strategies are expanded prematurely. Mitigation: keep the surface strictly gated behind testkit and avoid direct re-exports from the default surface.
• Risk: feature gating adds divergence between crates. Mitigation: align future changes with ADR 0002 and update tests/docs accordingly.

📎 Task: https://www.devboxer.com/task/b714b319-2386-437b-bbfc-e9ab37542426

[sourcery-ai]

Reviewer's Guide

Adds a new feature-gated wireframe::testkit root module that centralizes phase-8.5 testing helpers (partial-frame/fragment drivers, slow I/O helpers, and reassembly assertions), converts wireframe_testing into a thin compatibility facade over this surface, and wires in targeted tests and documentation updates to validate and describe the new export path.

Sequence diagram for drive_with_slow_codec_payloads using wireframe::testkit

sequenceDiagram
    participant T as TestCode
    participant TK as testkit::slow_io
    participant SUP as testkit::support
    participant APP as WireframeApp
    participant TOK as tokio::duplex
    participant COD as FrameCodec

    T->>TK: drive_with_slow_codec_payloads(app, codec, payloads, config)
    TK->>TK: drive_with_slow_codec_frames(app, codec, payloads, config)

    TK->>SUP: encode_payloads_with_codec(codec, payloads)
    SUP->>COD: encoder(), wrap_payload(), encode(...)
    COD-->>SUP: encoded frames
    SUP-->>TK: Vec<Vec<u8>> encoded

    TK->>TK: flatten frames to wire_bytes

    TK->>TK: drive_slow_internal(|server| run_owned_app(app, server), wire_bytes, config)
    TK->>TOK: duplex(config.capacity)
    TOK-->>TK: client, server streams

    par server_task
        TK->>SUP: run_owned_app(app, server)
        SUP->>APP: handle_connection(server)
        APP-->>SUP: completes
    and writer_task
        TK->>TOK: write_with_optional_pacing(client writer, wire_bytes, config.writer_pacing)
        loop chunks
            TOK-->>APP: chunk bytes
            TK->>TK: pause_between_chunks(config.writer_pacing)
        end
        TK->>TOK: shutdown writer
    and reader_task
        TK->>TOK: read_with_optional_pacing(client reader, config.reader_pacing)
        loop chunks
            TOK-->>TK: chunk bytes
            TK->>TK: pause_between_chunks(config.reader_pacing)
        end
        TK-->>TK: raw_bytes
    end

    TK-->>TK: decode_frames_with_codec(codec, raw_bytes)
    TK->>SUP: decode_frames_with_codec(codec, raw_bytes)
    SUP->>COD: decoder(), decode()/decode_eof()
    COD-->>SUP: Vec<F::Frame>
    SUP-->>TK: Vec<F::Frame>

    TK-->>T: io::Result<Vec<Vec<u8>>> (extract_payloads)

Loading

Class diagram for wireframe::testkit module structure and key types

classDiagram
    class testkit {
    }
    class fragment_drive {
    }
    class partial_frame {
    }
    class slow_io {
    }
    class reassembly {
    }
    class result {
    }
    class support {
    }

    testkit --> fragment_drive
    testkit --> partial_frame
    testkit --> slow_io
    testkit --> reassembly
    testkit --> result
    testkit --> support

    class SlowIoPacing {
        +NonZeroUsize chunk_size
        +Duration delay
        +new(chunk_size: NonZeroUsize, delay: Duration) SlowIoPacing
    }

    class SlowIoConfig {
        +Option~SlowIoPacing~ writer_pacing
        +Option~SlowIoPacing~ reader_pacing
        +usize capacity
        +new() SlowIoConfig
        +with_writer_pacing(pacing: SlowIoPacing) SlowIoConfig
        +with_reader_pacing(pacing: SlowIoPacing) SlowIoConfig
        +with_capacity(capacity: usize) SlowIoConfig
        -validate(self) io::Result~SlowIoConfig~
    }

    class slow_io {
        +MAX_SLOW_IO_CAPACITY: usize
        +drive_with_slow_frames(app: WireframeApp, frames: Vec~Vec~u8~~, config: SlowIoConfig) async io::Result~Vec~u8~~
        +drive_with_slow_payloads(app: WireframeApp, payloads: Vec~Vec~u8~~, config: SlowIoConfig) async io::Result~Vec~u8~~
        +drive_with_slow_codec_frames(app: WireframeApp, codec: FrameCodec, payloads: Vec~Vec~u8~~, config: SlowIoConfig) async io::Result~Vec~F::Frame~~
        +drive_with_slow_codec_payloads(app: WireframeApp, codec: FrameCodec, payloads: Vec~Vec~u8~~, config: SlowIoConfig) async io::Result~Vec~Vec~u8~~~
    }

    slow_io --> SlowIoPacing
    slow_io --> SlowIoConfig

    class partial_frame {
        +drive_with_partial_frames(app: WireframeApp, codec: FrameCodec, payloads: Vec~Vec~u8~~, chunk_size: NonZeroUsize) async io::Result~Vec~Vec~u8~~~
        +drive_with_partial_frames_with_capacity(app: WireframeApp, codec: FrameCodec, payloads: Vec~Vec~u8~~, chunk_size: NonZeroUsize, capacity: usize) async io::Result~Vec~Vec~u8~~~
        +drive_with_partial_frames_mut(app: &mut WireframeApp, codec: FrameCodec, payloads: Vec~Vec~u8~~, chunk_size: NonZeroUsize) async io::Result~Vec~Vec~u8~~~
        +drive_with_partial_codec_frames(app: WireframeApp, codec: FrameCodec, payloads: Vec~Vec~u8~~, chunk_size: NonZeroUsize) async io::Result~Vec~F::Frame~~
    }

    class fragment_drive {
        +drive_with_fragments(app: WireframeApp, codec: FrameCodec, fragmenter: Fragmenter, payload: Vec~u8~) async io::Result~Vec~Vec~u8~~~
        +drive_with_fragments_with_capacity(app: WireframeApp, codec: FrameCodec, fragmenter: Fragmenter, payload: Vec~u8~, capacity: usize) async io::Result~Vec~Vec~u8~~~
        +drive_with_fragments_mut(app: &mut WireframeApp, codec: FrameCodec, fragmenter: Fragmenter, payload: Vec~u8~) async io::Result~Vec~Vec~u8~~~
        +drive_with_fragment_frames(app: WireframeApp, codec: FrameCodec, fragmenter: Fragmenter, payload: Vec~u8~) async io::Result~Vec~F::Frame~~
        +drive_with_partial_fragments(app: WireframeApp, codec: FrameCodec, fragmenter: Fragmenter, payload: Vec~u8~, chunk_size: NonZeroUsize) async io::Result~Vec~Vec~u8~~~
    }

    fragment_drive --> Fragmenter

    class TestError {
        <<enum>>
        +Io(std::io::Error)
        +Msg(String)
        +Wireframe(WireframeError)
        +Client(crate::client::ClientError) *cfg(not(loom))*
        +Server(crate::server::ServerError) *cfg(not(loom))*
        +Push(PushError)
        +PushConfig(PushConfigError)
        +ConnectionState(ConnectionStateError)
        +Reassembly(ReassemblyError)
        +Fragmentation(FragmentationError)
        +Codec(CodecError)
        +Encode(bincode::error::EncodeError)
        +Decode(bincode::error::DecodeError)
        +Join(tokio::task::JoinError)
        +Timeout(tokio::time::error::Elapsed)
        +OneshotRecv(tokio::sync::oneshot::error::RecvError)
        +OneshotTryRecv(tokio::sync::oneshot::error::TryRecvError)
        +MpscTryRecv(tokio::sync::mpsc::error::TryRecvError)
        +ParseInt(std::num::ParseIntError)
        +TryFromInt(std::num::TryFromIntError)
        +Utf8(std::str::Utf8Error)
        +FromUtf8(std::string::FromUtf8Error)
        +AddrParse(std::net::AddrParseError)
        +from(value: String) TestError
        +from(value: &str) TestError
    }

    class TestResult {
        <<type alias>>
        +Result~T, TestError~
    }

    result --> TestError
    result --> TestResult

    class FragmentReassemblySnapshot {
        +Option~&ReassembledMessage~ last_reassembled
        +Option~&ReassemblyError~ last_error
        +&[MessageId] evicted_ids
        +usize buffered_messages
        +new(last_reassembled: Option~&ReassembledMessage~, last_error: Option~&ReassemblyError~, evicted_ids: &[MessageId], buffered_messages: usize) FragmentReassemblySnapshot
    }

    class FragmentReassemblyErrorExpectation {
        <<enum>>
        +MessageTooLarge(message_id: MessageId)
        +MessageTooLargeAny
        +IndexMismatch
        +MessageMismatch
        +SeriesComplete
        +IndexOverflow
    }

    class MessageAssemblySnapshot {
        +Option~&Result~Option~AssembledMessage~, MessageAssemblyError~~ last_result
        +&[AssembledMessage] completed_messages
        +&[MessageKey] evicted_keys
        +usize buffered_count
        +usize total_buffered_bytes
        +new(last_result: Option~&Result~Option~AssembledMessage~, MessageAssemblyError~~, completed_messages: &[AssembledMessage], evicted_keys: &[MessageKey], buffered_count: usize, total_buffered_bytes: usize) MessageAssemblySnapshot
        +last_completed() Option~&AssembledMessage~
        +completed_for_key(key: MessageKey) Option~&AssembledMessage~
    }

    class MessageAssemblyErrorExpectation {
        <<enum>>
        +SequenceMismatch(expected: FrameSequence, found: FrameSequence)
        +DuplicateFrame(key: MessageKey, sequence: FrameSequence)
        +MissingFirstFrame(key: MessageKey)
        +DuplicateFirstFrame(key: MessageKey)
        +MessageTooLarge(key: MessageKey)
        +ConnectionBudgetExceeded(key: MessageKey)
        +InFlightBudgetExceeded(key: MessageKey)
    }

    class reassembly_assertions {
        +assert_fragment_reassembly_absent(snapshot: FragmentReassemblySnapshot) TestResult
        +assert_fragment_reassembly_completed_len(snapshot: FragmentReassemblySnapshot, expected_len: usize) TestResult
        +assert_fragment_reassembly_completed_bytes(snapshot: FragmentReassemblySnapshot, expected: &[u8]) TestResult
        +assert_fragment_reassembly_error(snapshot: FragmentReassemblySnapshot, expected: FragmentReassemblyErrorExpectation) TestResult
        +assert_fragment_reassembly_buffered_messages(snapshot: FragmentReassemblySnapshot, expected: usize) TestResult
        +assert_fragment_reassembly_evicted(snapshot: FragmentReassemblySnapshot, message_id: MessageId) TestResult
        +assert_message_assembly_incomplete(snapshot: MessageAssemblySnapshot) TestResult
        +assert_message_assembly_completed(snapshot: MessageAssemblySnapshot, expected: &[u8]) TestResult
        +assert_message_assembly_completed_for_key(snapshot: MessageAssemblySnapshot, key: MessageKey, expected: &[u8]) TestResult
        +assert_message_assembly_error(snapshot: MessageAssemblySnapshot, expected: MessageAssemblyErrorExpectation) TestResult
        +assert_message_assembly_buffered_count(snapshot: MessageAssemblySnapshot, expected: usize) TestResult
        +assert_message_assembly_total_buffered_bytes(snapshot: MessageAssemblySnapshot, expected: usize) TestResult
        +assert_message_assembly_evicted(snapshot: MessageAssemblySnapshot, key: MessageKey) TestResult
    }

    reassembly --> FragmentReassemblySnapshot
    reassembly --> FragmentReassemblyErrorExpectation
    reassembly --> MessageAssemblySnapshot
    reassembly --> MessageAssemblyErrorExpectation
    reassembly --> reassembly_assertions
    reassembly_assertions --> TestResult

    class support {
        <<internal helpers>>
        +DEFAULT_CAPACITY: usize
        +encode_payloads_with_codec(codec: FrameCodec, payloads: Vec~Vec~u8~~) io::Result~Vec~Vec~u8~~~
        +decode_frames_with_codec(codec: FrameCodec, bytes: &[u8]) io::Result~Vec~F::Frame~~
        +extract_payloads(frames: &[F::Frame]) Vec~Vec~u8~~
        +drive_internal(server_fn: FnOnce(DuplexStream), frames: Vec~Vec~u8~~, capacity: usize) async io::Result~Vec~u8~~
        +drive_chunked_internal(server_fn: FnOnce(DuplexStream), wire_bytes: Vec~u8~, chunk_size: NonZeroUsize, capacity: usize) async io::Result~Vec~u8~~
        +run_owned_app(app: WireframeApp, server: DuplexStream) async
    }

    support ..> WireframeApp
    support ..> FrameCodec

    testkit ..> TestResult
    testkit ..> TestError

Loading

File-Level Changes

Change	Details	Files
Introduce testkit Cargo feature and root wireframe::testkit module implementing the consolidated test helper surface.	Add testkit feature flag in the root Cargo.toml, including docs.rs metadata and dev-dependency feature set. Gate a new pub mod testkit; in src/lib.rs behind #[cfg(feature = "testkit")]. Implement src/testkit/ subtree with shared support utilities, partial-frame drivers, fragment drivers, slow-I/O helpers, and a shared TestError/TestResult type. Expose the testkit API via src/testkit/mod.rs re-exporting driving helpers, slow I/O helpers, and reassembly assertion primitives.	Cargo.toml src/lib.rs src/testkit/mod.rs src/testkit/support.rs src/testkit/partial_frame.rs src/testkit/fragment_drive.rs src/testkit/slow_io.rs src/testkit/result.rs src/testkit/reassembly/mod.rs src/testkit/reassembly/assert_helpers.rs src/testkit/reassembly/fragment.rs src/testkit/reassembly/message.rs src/testkit/reassembly/message_error.rs
Refactor wireframe_testing to act as a compatibility facade over wireframe::testkit for phase-8.5 helpers.	Enable the testkit feature on the wireframe dependency in wireframe_testing/Cargo.toml. Replace the local TestError/TestResult definitions with the shared versions in wireframe::testkit. Re-export reassembly helpers from wireframe::testkit::reassembly instead of local modules, and drop now-redundant module implementations. Update wireframe_testing helper exports so slow-I/O, partial-frame, and fragment-driving helpers are forwarded from wireframe::testkit while keeping existing public names and types.	wireframe_testing/Cargo.toml wireframe_testing/src/integration_helpers.rs wireframe_testing/src/reassembly/mod.rs wireframe_testing/src/helpers.rs
Add focused tests and BDD scenarios that exercise the new wireframe::testkit root export directly.	Introduce tests/testkit_exports.rs covering partial-frame/fragment drivers, slow I/O helpers, and reassembly assertions via wireframe::testkit. Add a new BDD world fixture, steps, and scenarios (testkit_export_world, testkit_export_steps, testkit_export_scenarios) plus a .feature file that drives payloads and reassembly assertions through the root testkit surface. Register the new fixture, steps, and scenarios in the existing test harness module registries.	tests/testkit_exports.rs tests/fixtures/testkit_export.rs tests/steps/testkit_export_steps.rs tests/scenarios/testkit_export_scenarios.rs tests/features/testkit_export.feature tests/fixtures/mod.rs tests/steps/mod.rs tests/scenarios/mod.rs
Update documentation and design records to describe the feature-gated testkit surface and mark roadmap progress.	Add ExecPlan docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md describing constraints, risks, and implementation plan for the testkit export. Update ADR 0002 to record that phase-8.5 helpers now live behind the testkit feature in the main crate and that wireframe_testing is a compatibility facade. Update docs/users-guide.md examples and narrative to show wireframe::testkit usage, including enabling the testkit Cargo feature and noting ongoing wireframe_testing compatibility. Mark roadmap item 8.5.4 as completed in docs/roadmap.md. Ensure docs.rs builds include the testkit feature via [package.metadata.docs.rs].	docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md docs/adr-002-streaming-requests-and-shared-message-assembly.md docs/users-guide.md docs/roadmap.md Cargo.toml

---

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.

[coderabbitai]

Walkthrough

Introduce an opt-in Cargo feature testkit that exposes wireframe::testkit, moving in-memory drivers (fragment/partial-frame/slow-I/O), deterministic reassembly assertion utilities, and shared test error/result types into the main crate; keep wireframe_testing as a compatibility façade that re-exports the new testkit surface.

Changes

Cohort / File(s)	Summary
Feature Flag & Core Export Cargo.toml, src/lib.rs	Add testkit Cargo feature; enable docs/dev to build with testkit; expose pub mod testkit gated by #[cfg(feature = "testkit")].
Testkit Root & Re-exports src/testkit/mod.rs	Add testkit root module that publicly re-exports driver helpers, reassembly assertions, result types and slow‑I/O primitives.
Support & Result Types src/testkit/support.rs, src/testkit/result.rs	Add duplex harness, panic-guard wrapper, encode/decode helpers, DEFAULT_CAPACITY, shared TestError/TestResult and conversion impls.
Fragment Driving src/testkit/fragment_drive.rs	Add fragment-aware drivers and helpers: fragment → encode → envelope → bincode serialize → drive over duplex; provide owned/mut/capacity/chunked variants returning payloads or frames.
Partial Frame Driving src/testkit/partial_frame.rs	Add chunked-write helpers to drive apps with partial codec frames; capacity and codec-frame variants returning payloads or frames.
Slow I/O Simulation src/testkit/slow_io.rs	Add SlowIoPacing/SlowIoConfig, MAX_SLOW_IO_CAPACITY, paced read/write primitives, validation, and drive_with_slow_* APIs.
Reassembly Assertions src/testkit/reassembly/... src/testkit/reassembly/mod.rs, fragment.rs, message.rs, message_error.rs, assert_helpers.rs	Add snapshot types, expectation enums and deterministic assertion helpers for fragment reassembly and message assembly; expose assertion helpers and expectation types.
Test Support Utilities src/testkit/support.rs	Provide TestSerializer trait, client read/write strategies, drive_internal/drive_chunked_internal, codec encode/decode helpers, and run_owned_app.
Docs & Roadmap docs/*	Document relocation of phase‑8.5 test helpers to wireframe::testkit, add ExecPlan 8.5.4, update user‑guide examples, mark roadmap item complete.
Tests, BDD & Fixtures tests/*, tests/features/*, tests/fixtures/*, tests/scenarios/*, tests/steps/*	Add BDD feature, fixtures, scenarios, steps and integration tests exercising wireframe::testkit exports (partial‑frame, fragment, slow‑I/O, reassembly assertions).
wireframe_testing Compatibility wireframe_testing/Cargo.toml, wireframe_testing/src/helpers.rs, wireframe_testing/src/integration_helpers.rs, wireframe_testing/src/reassembly/mod.rs	Enable testkit feature on wireframe dependency; remove local helper implementations and TestError; re-export wireframe::testkit APIs and TestError/TestResult as a compatibility façade.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test harness
    participant Codec as FrameCodec
    participant Fragmenter as Fragmenter
    participant Client as Client writer
    participant Duplex as DuplexStream
    participant Server as WireframeApp
    participant Decoder as Frame decoder

    Test->>Codec: Encode payloads -> frames
    alt fragmentation
        Codec->>Fragmenter: Produce fragments
        Fragmenter->>Client: Emit fragment bytes
    else no fragmentation
        Codec->>Client: Emit framed bytes
    end
    Client->>Duplex: Write bytes (possibly chunked/paced)
    Duplex->>Server: Deliver bytes to server handler
    Server->>Duplex: Write response bytes
    Duplex->>Decoder: Collect response bytes
    Decoder->>Codec: Decode frames
    Decoder->>Test: Extract payloads / snapshots

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Add slow reader/writer back-pressure testkit, tests, and docs #495 — Moves slow‑I/O helpers (SlowIoPacing, SlowIoConfig, MAX_SLOW_IO_CAPACITY and drive_with_slow_*) into the testkit and updates re‑exports.
• 8.5.3: Remove duplicate accessors in message assembly #499 — Moves deterministic reassembly assertion helpers into wireframe::testkit and makes wireframe_testing a compatibility façade.

Poem

Fragments flow through testkit’s careful door,
Reassembly hums and counts the bytes once more.
Slow I/O ticks in patient, measured streams,
Compatibility waves, and BDD sings its dreams. 🎭

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title directly and clearly summarises the main objective: exporting testkit utilities behind a new wireframe::testkit feature in Stage A.
Description check	✅ Passed	The description comprehensively covers the changeset, including the feature flag, new modules, tests, documentation updates, compatibility facade strategy, and rationale.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch export-wireframe-testkit-feature-s3553j

---

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

[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. Include the file and symbol names indicated in the issue at the head of your response.

src/testkit/reassembly/message.rs

Comment on file

pub fn assert_message_assembly_buffered_count(
    snapshot: MessageAssemblySnapshot<'_>,
    expected: usize,
) -> TestResult {
    assert_usize_field(snapshot.buffered_count, expected, "buffered_count")
}

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: assert_message_assembly_buffered_count,assert_message_assembly_total_buffered_bytes

[coderabbitai]

Actionable comments posted: 8

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md`:
- Around line 379-383: Update the documentation to use the singular
`testkit_export` names consistently: replace references to
`tests/features/testkit_exports.feature`, `tests/fixtures/testkit_exports.rs`,
`tests/steps/testkit_exports_steps.rs`, and
`tests/scenarios/testkit_exports_scenarios.rs` with
`tests/features/testkit_export.feature`, `tests/fixtures/testkit_export.rs`,
`tests/steps/testkit_export_steps.rs`, and
`tests/scenarios/testkit_export_scenarios.rs`, and update the test run example
filter from `-- testkit_exports` to `-- testkit_export` (also apply the same
changes at the locations mentioned around lines 430-433).
- Around line 193-194: Remove the stale opener "Not started yet." from the
section that documents the exported wireframe testkit feature (the line
containing the exact text "Not started yet." in
docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md) so the document
matches its Status: IMPLEMENTED and retains the recorded API, compatibility,
behavioural proof, and validation content without contradicting language.

In `@src/testkit/result.rs`:
- Around line 13-15: Add the #[non_exhaustive] attribute to the public enum
TestError so future variants can be added without breaking downstream exhaustive
matches; locate the TestError enum declaration (the pub enum TestError in this
file) and place #[non_exhaustive] directly above the #[derive(...)]/pub enum
line, then run cargo check to ensure no warnings from exhaustive matches in this
crate need addressing.

In `@src/testkit/support.rs`:
- Around line 18-38: The TestSerializer trait is currently defined in the
private support module and used in public APIs, triggering the private_bounds
lint; fix this by adding a public re-export of TestSerializer from the testkit
root module so the trait becomes part of the public API surface (i.e., in the
testkit module export support::TestSerializer as pub so S: TestSerializer in
slow_io, fragment_drive, and partial_frame is nameable publicly). Ensure the
exported symbol is exactly TestSerializer to match existing generic bounds.

In `@tests/fixtures/testkit_export.rs`:
- Around line 83-101: The test helper seed_completed_message_snapshot currently
takes a key but assert_root_reassembly_helper asserts MessageKey(7) explicitly,
causing a mismatch; fix by persisting the seeded key and using it in the
assertion: add a field (e.g., seeded_key: Option<u64>) to the fixture, set it
inside seed_completed_message_snapshot(key) when creating AssembledMessage, and
then change assert_root_reassembly_helper to build the expected key from that
stored value (use MessageKey(self.seeded_key.unwrap()) instead of
MessageKey(7)); alternatively remove the key parameter from
seed_completed_message_snapshot and always seed a fixed key then keep the
assertion as-is, but ensure seed_completed_message_snapshot and
assert_root_reassembly_helper reference the same identifier (seeded_key,
seed_completed_message_snapshot, assert_root_reassembly_helper, MessageKey).

In `@tests/scenarios/testkit_export_scenarios.rs`:
- Around line 11-25: Remove the #[expect(unused_variables, ...)] attributes from
the two test functions and underscore the unused fixture parameter instead:
rename the parameter testkit_export_world to _testkit_export_world in
root_testkit_partial_frame_export and root_testkit_reassembly_export so the
tests remain valid stubs without silencing lints.

In `@tests/testkit_exports.rs`:
- Around line 92-97: The test currently sends a vector of zeroes and ignores the
response, so it never verifies that drive_with_fragments actually hits the echo
route (route 1) or returns the echoed payload; replace the raw vec![0; 64] with
a valid envelope payload (e.g. a non-empty byte sequence or serialized envelope
expected by build_echo_app/route 1), await and capture the result of
drive_with_fragments, and assert that the reply matches the original payload
(use Fragmenter::new(...) and drive_with_fragments(app, &codec, &fragmenter,
payload). Ensure you assert success and equality of the echoed payload rather
than dropping the result.
- Around line 116-128: The fixed
tokio::time::advance(Duration::from_millis(30)).await is brittle; replace it
with an incremental advance loop that keeps advancing small steps until the
spawned task (the future returned by tokio::spawn that runs
drive_with_slow_codec_payloads) actually finishes or a reasonable timeout is
hit. Concretely, poll task.is_finished() in a loop, calling
tokio::time::advance(small_step).await (e.g., 20ms) repeatedly and breaking when
task.is_finished() (or after a max total duration computed from payload size if
available), then await the task as before to get response_payloads; this ensures
larger encoded frames/pacing delays finish reliably.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4a48512f-7e87-4ca8-8750-5e07da926b4d

📥 Commits

Reviewing files that changed from the base of the PR and between 317a3a4 and 32c930c.

📒 Files selected for processing (29)

• Cargo.toml
• docs/adr-002-streaming-requests-and-shared-message-assembly.md
• docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md
• docs/roadmap.md
• docs/users-guide.md
• src/lib.rs
• src/testkit/fragment_drive.rs
• src/testkit/mod.rs
• src/testkit/partial_frame.rs
• src/testkit/reassembly/assert_helpers.rs
• src/testkit/reassembly/fragment.rs
• src/testkit/reassembly/message.rs
• src/testkit/reassembly/message_error.rs
• src/testkit/reassembly/mod.rs
• src/testkit/result.rs
• src/testkit/slow_io.rs
• src/testkit/support.rs
• tests/features/testkit_export.feature
• tests/fixtures/mod.rs
• tests/fixtures/testkit_export.rs
• tests/scenarios/mod.rs
• tests/scenarios/testkit_export_scenarios.rs
• tests/steps/mod.rs
• tests/steps/testkit_export_steps.rs
• tests/testkit_exports.rs
• wireframe_testing/Cargo.toml
• wireframe_testing/src/helpers.rs
• wireframe_testing/src/integration_helpers.rs
• wireframe_testing/src/reassembly/mod.rs

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/testkit_exports.rs`:
- Around line 181-259: The four tests
slow_io_config_validate_rejects_zero_capacity,
slow_io_config_validate_rejects_capacity_over_max,
slow_io_config_validate_rejects_writer_chunk_exceeding_capacity, and
slow_io_config_validate_rejects_reader_chunk_exceeding_capacity duplicate the
same assertion logic; extract a private helper fn
assert_validate_fails_with_invalid_input(config: SlowIoConfig, context: &str) ->
io::Result<()> that calls config.validate(), expects an error, and checks
err.kind() == io::ErrorKind::InvalidInput (using that context in the error
messages), then replace each test body with a one-line call to that helper
passing the constructed SlowIoConfig and a context string; this will centralize
the assertion around SlowIoConfig::validate and ErrorKind::InvalidInput.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 27ae899d-690c-47a0-9881-74cf160f18bb

📥 Commits

Reviewing files that changed from the base of the PR and between 32c930c and eb69a57.

📒 Files selected for processing (6)

• docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md
• src/testkit/fragment_drive.rs
• src/testkit/partial_frame.rs
• src/testkit/slow_io.rs
• src/testkit/support.rs
• tests/testkit_exports.rs

[leynos]

@coderabbitai Have the following now been resolved?

Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity.

Please address the comments from this code review:

## Overall Comments
- The slow I/O machinery in `src/testkit/slow_io.rs` reimplements a lot of the connection setup and panic-wrapping logic already present in `src/testkit/support.rs::drive_internal`/`drive_chunked_internal`; consider factoring the shared `DuplexStream` + server task + panic handling into a single reusable helper to reduce duplication and keep future changes consistent.
- Several of the new helpers (e.g. in `partial_frame.rs`, `fragment_drive.rs`, and `slow_io.rs`) construct `io::Error` with formatted strings for argument validation or internal invariants; you might want to centralize these validations (e.g. capacity/chunk-size checks and slice-bounds checks) into small shared functions so the error messages and behavior stay aligned as the testkit evolves.

## Individual Comments

### Comment 1
<location path="src/testkit/support.rs" line_range="40-49" />
<code_context>
+pub(crate) async fn drive_internal<F, Fut>(
</code_context>
<issue_to_address>
**nitpick:** drive_internal and drive_chunked_internal duplicate server task setup logic

Since both functions build nearly the same `server_fut` (duplex setup, panic catching, error mapping), consider extracting that logic into a shared helper. This will reduce duplication and ensure panic/IO handling stays consistent across current and future in-memory driver variants.
</issue_to_address>

### Comment 2
<location path="tests/testkit_exports.rs" line_range="105-114" />
<code_context>
+    let codec = hotline_codec();
+    let app = build_echo_app(codec.clone())?;
+    let payload = serialize_envelope(b"slow-io")?;
+    let config = SlowIoConfig::new()
+        .with_writer_pacing(SlowIoPacing::new(
+            NonZeroUsize::new(2).ok_or_else(|| io::Error::other("chunk must be non-zero"))?,
+            Duration::from_millis(5),
+        ))
+        .with_reader_pacing(SlowIoPacing::new(
+            NonZeroUsize::new(4).ok_or_else(|| io::Error::other("chunk must be non-zero"))?,
+            Duration::from_millis(5),
+        ))
+        .with_capacity(64);
+
+    let task = tokio::spawn(async move {
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests that cover `SlowIoConfig::validate` failure paths (capacity and chunk-size bounds).

Current tests only cover the happy path for `SlowIoConfig` and pacing. Please add tests that exercise the validation error cases in `slow_io.rs` (e.g., `capacity = 0`, `capacity > MAX_SLOW_IO_CAPACITY`, pacing chunk size > capacity) and assert `io::ErrorKind::InvalidInput`, so regressions in this validation logic are caught.

Suggested implementation:

```rust
#[test]
fn slow_io_config_validate_errors_on_zero_capacity() {
    let config = SlowIoConfig::new().with_capacity(0);

    let err = config
        .validate()
        .expect_err("expected validate to fail for zero capacity");

    assert_eq!(err.kind(), io::ErrorKind::InvalidInput);
}

#[test]
fn slow_io_config_validate_errors_on_capacity_over_max() {
    let config = SlowIoConfig::new().with_capacity(MAX_SLOW_IO_CAPACITY + 1);

    let err = config
        .validate()
        .expect_err("expected validate to fail for capacity over MAX_SLOW_IO_CAPACITY");

    assert_eq!(err.kind(), io::ErrorKind::InvalidInput);
}

#[test]
fn slow_io_config_validate_errors_when_chunk_size_exceeds_capacity() {
    // capacity smaller than pacing chunk size
    let capacity = 4;
    let chunk_size = NonZeroUsize::new(capacity * 2).expect("non-zero chunk size");

    let config = SlowIoConfig::new()
        .with_capacity(capacity)
        .with_writer_pacing(SlowIoPacing::new(chunk_size, Duration::from_millis(1)));

    let err = config
        .validate()
        .expect_err("expected validate to fail when chunk size exceeds capacity");

    assert_eq!(err.kind(), io::ErrorKind::InvalidInput);
}

#[tokio::test(flavor = "current_thread", start_paused = true)]

```

If these symbols are not already imported at the top of `tests/testkit_exports.rs`, add the following to your existing `use` block:

```rust
use std::{io, num::NonZeroUsize, time::Duration};
use crate::slow_io::{SlowIoConfig, SlowIoPacing, MAX_SLOW_IO_CAPACITY};
```

Adjust the module path for `SlowIoConfig`, `SlowIoPacing`, and `MAX_SLOW_IO_CAPACITY` to match how they are imported elsewhere in the test file.
</issue_to_address>

### Comment 3
<location path="docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md" line_range="191-195" />
<code_context>
+  contract while existing tests continue guarding the helper behaviour and
+  compatibility path. Date/Author: 2026-03-21 / Codex
+
+## Outcomes & Retrospective
+
+Not started yet. This section must be updated after implementation with:
+
+- Final shipped API:
</code_context>
<issue_to_address>
**suggestion:** Opening sentence of Outcomes & Retrospective is now stale and contradicts the implemented status.

This section now documents concrete outcomes and validation, but the opening line still says "Not started yet...". Please update or remove that line so the section consistently reflects the completed implementation and recorded outcomes.

```suggestion
## Outcomes & Retrospective

This section records the concrete outcomes and validation for the implemented plan:

- Final shipped API:
```
</issue_to_address>

### Comment 4
<location path="docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md" line_range="424-428" />
<code_context>
+
+```sh
+set -o pipefail
+cargo test --test bdd --features "advanced-tests testkit" -- testkit_exports \
+  2>&1 | tee /tmp/8-5-4-bdd.log
+```
</code_context>
<issue_to_address>
**issue (typo):** Possible typo in BDD test selector: `testkit_exports` vs. `testkit_export`.

The new files use the singular `testkit_export`, but this command filters on `testkit_exports`. If this is meant to target the new flow, update the selector to match the file naming to avoid confusion when running the command.

```suggestion
```sh
set -o pipefail
cargo test --test testkit_export --features testkit \
  2>&1 | tee /tmp/8-5-4-testkit-export.log
```
```
</issue_to_address>

### Comment 5
<location path="src/testkit/slow_io.rs" line_range="195" />
<code_context>
+    }
+}
+
+async fn drive_slow_internal<F, Fut>(
+    server_fn: F,
+    wire_bytes: Vec<u8>,
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the common duplex/panic/try_join orchestration into a single shared driver that accepts read/write strategy closures instead of adding a separate `drive_slow_internal` implementation.

You can reduce the added complexity by centralizing the transport orchestration and parameterizing only the read/write behavior, instead of introducing a full third driver (`drive_slow_internal`) with its own panic/duplex/`try_join` machinery.

Right now `drive_slow_internal` is structurally the same as `support::drive_internal` and `support::drive_chunked_internal` (duplex creation, split, panic-wrapped server future, writer future, reader future, `try_join!`). That logic can live in a single reusable helper that takes *strategy closures* for client write/read. The slow I/O helpers would then only define pacing-aware strategies and call into the shared driver.

Example of how you could structure this (sketch, types may need to be adjusted to match the existing `support` module):

```rust
// in support module (or wherever the other drivers live)
pub async fn drive_with_strategies<F, Fut, WFn, RFn, WOut, ROut>(
    server_fn: F,
    capacity: usize,
    write_strategy: WFn,
    read_strategy: RFn,
) -> io::Result<ROut>
where
    F: FnOnce(DuplexStream) -> Fut,
    Fut: Future<Output = ()>,
    WFn: for<'a> FnOnce(&'a mut (impl AsyncWrite + Unpin)) -> WOut + Send,
    WOut: Future<Output = io::Result<()>> + Send,
    RFn: for<'a> FnOnce(&'a mut (impl AsyncRead + Unpin)) -> ROut + Send,
    ROut: Future<Output = io::Result<ROut>> + Send,
{
    let (client, server) = tokio::io::duplex(capacity);
    let (mut reader, mut writer) = split(client);

    let server_fut = async {
        use futures::FutureExt as _;
        let result = std::panic::AssertUnwindSafe(server_fn(server))
            .catch_unwind()
            .await;
        match result {
            Ok(()) => Ok(()),
            Err(panic) => {
                let panic_msg = crate::panic::format_panic(&panic);
                Err(io::Error::other(format!("server task failed: {panic_msg}")))
            }
        }
    };

    let writer_fut = async {
        write_strategy(&mut writer).await?;
        writer.shutdown().await?;
        Ok(())
    };

    let reader_fut = read_strategy(&mut reader);

    let ((), (), out) = tokio::try_join!(server_fut, writer_fut, reader_fut)?;
    Ok(out)
}
```

Then `drive_slow_internal` becomes a thin wrapper that only wires up pacing-aware strategies, and you can keep *all* existing behavior:

```rust
async fn drive_slow_internal<F, Fut>(
    server_fn: F,
    wire_bytes: Vec<u8>,
    config: SlowIoConfig,
) -> io::Result<Vec<u8>>
where
    F: FnOnce(DuplexStream) -> Fut,
    Fut: Future<Output = ()>,
{
    let config = config.validate()?;

    drive_with_strategies(
        server_fn,
        config.capacity,
        |writer| write_with_optional_pacing(writer, &wire_bytes, config.writer_pacing),
        |reader| read_with_optional_pacing(reader, config.reader_pacing),
    )
    .await
}
```

You can then refactor the existing `drive_internal` and `drive_chunked_internal` to call `drive_with_strategies` with appropriate immediate / chunked strategies instead of re-implementing the same orchestration. That:

- Removes the third custom driver implementation.
- Keeps all pacing logic localized to `write_with_optional_pacing`/`read_with_optional_pacing`.
- Centralizes panic handling and `try_join!` behavior, reducing maintenance and cognitive load.
</issue_to_address>

### Comment 6
<location path="src/testkit/partial_frame.rs" line_range="21" />
<code_context>
+    codec::FrameCodec,
+};
+
+#[derive(Debug, Clone, Copy)]
+struct ChunkConfig {
+    chunk_size: NonZeroUsize,
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing the `ChunkConfig` wrapper and reuse a shared internal codec driver so this helper only wires parameters and doesn’t reimplement the encode/drive/decode pipeline.

You can drop `ChunkConfig` and the extra indirection while keeping all behavior, and also share the driver logic with other helpers.

Right now `ChunkConfig` just carries two integers into `drive_partial_frames_internal`, and that internal function mirrors the other drivers (`fragment_drive`, `slow_io`) that already do: encode → flatten → `drive_chunked_internal` → decode. You can:

1. Remove `ChunkConfig` and pass `chunk_size`/`capacity` directly.
2. Extract a shared internal driver in `support` and use it from all three helpers, so `partial_frames` doesn’t reimplement the pattern.

For example, in `support` you could introduce a generic codec driver:

```rust
// in support.rs
pub async fn drive_via_codec_internal<F, H, Fut>(
    handler: H,
    codec: &F,
    payloads: Vec<Vec<u8>>,
    chunk_size: NonZeroUsize,
    capacity: usize,
) -> io::Result<Vec<F::Frame>>
where
    F: FrameCodec,
    H: FnOnce(DuplexStream) -> Fut,
    Fut: Future<Output = ()> + Send,
{
    let encoded = encode_payloads_with_codec(codec, payloads)?;
    let wire_bytes: Vec<u8> = encoded.into_iter().flatten().collect();
    let raw = drive_chunked_internal(handler, wire_bytes, chunk_size, capacity).await?;
    decode_frames_with_codec(codec, &raw)
}
```

Then `partial_frames` becomes a thin wrapper, no `ChunkConfig`/extra internal fn needed:

```rust
pub async fn drive_with_partial_frames_with_capacity<S, C, E, F>(
    app: WireframeApp<S, C, E, F>,
    codec: &F,
    payloads: Vec<Vec<u8>>,
    chunk_size: NonZeroUsize,
    capacity: usize,
) -> io::Result<Vec<Vec<u8>>>
where
    S: TestSerializer,
    C: Send + 'static,
    E: Packet,
    F: FrameCodec,
{
    let frames = drive_via_codec_internal(
        |server| run_owned_app(app, server),
        codec,
        payloads,
        chunk_size,
        capacity,
    )
    .await?;
    Ok(extract_payloads::<F>(&frames))
}

pub async fn drive_with_partial_frames_mut<S, C, E, F>(
    app: &mut WireframeApp<S, C, E, F>,
    codec: &F,
    payloads: Vec<Vec<u8>>,
    chunk_size: NonZeroUsize,
) -> io::Result<Vec<Vec<u8>>>
where
    S: TestSerializer,
    C: Send + 'static,
    E: Packet,
    F: FrameCodec,
{
    let frames = drive_via_codec_internal(
        |server| async move { app.handle_connection(server).await },
        codec,
        payloads,
        chunk_size,
        DEFAULT_CAPACITY,
    )
    .await?;
    Ok(extract_payloads::<F>(&frames))
}

pub async fn drive_with_partial_codec_frames<S, C, E, F>(
    app: WireframeApp<S, C, E, F>,
    codec: &F,
    payloads: Vec<Vec<u8>>,
    chunk_size: NonZeroUsize,
) -> io::Result<Vec<F::Frame>>
where
    S: TestSerializer,
    C: Send + 'static,
    E: Packet,
    F: FrameCodec,
{
    drive_via_codec_internal(
        |server| run_owned_app(app, server),
        codec,
        payloads,
        chunk_size,
        DEFAULT_CAPACITY,
    )
    .await
}
```

You’d also update `fragment_drive`/`slow_io` to call `drive_via_codec_internal` instead of their own encode/flatten/drive/decode pipeline. This removes the extra concept (`ChunkConfig` + a dedicated internal fn) and centralizes the driver behavior in one place while preserving all existing public APIs.
</issue_to_address>

### Comment 7
<location path="src/testkit/fragment_drive.rs" line_range="59" />
<code_context>
+
+const DEFAULT_FRAGMENT_ROUTE_ID: u32 = 1;
+
+struct FragmentRequest<'a> {
+    fragmenter: &'a Fragmenter,
+    payload: Vec<u8>,
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the `FragmentRequest` abstraction and `drive_fragments_internal` with a generic `drive_preencoded` helper so fragment helpers just preprocess payloads then call a shared driver function.

You can drop `FragmentRequest` and simplify `drive_fragments_internal` into a reusable helper that takes pre-encoded payloads, then thin out the public functions to just “preprocess → call driver”. That removes the extra request abstraction while keeping behavior identical.

For example, in this module (or `support` if you want it shared more widely), introduce a generic internal driver:

```rust
async fn drive_preencoded<F, H, Fut>(
    handler: H,
    codec: &F,
    payloads: Vec<Vec<u8>>,
    capacity: usize,
) -> io::Result<Vec<F::Frame>>
where
    F: FrameCodec,
    H: FnOnce(DuplexStream) -> Fut,
    Fut: std::future::Future<Output = ()> + Send,
{
    let encoded = encode_payloads_with_codec(codec, payloads)?;
    let raw = drive_internal(handler, encoded, capacity).await?;
    decode_frames_with_codec(codec, &raw)
}
```

Then all fragment-specific logic stays in a small preprocessing step, and the public functions become straightforward:

```rust
pub async fn drive_with_fragments_with_capacity<S, C, E, F>(
    app: WireframeApp<S, C, E, F>,
    codec: &F,
    fragmenter: &Fragmenter,
    payload: Vec<u8>,
    capacity: usize,
) -> io::Result<Vec<Vec<u8>>>
where
    S: TestSerializer,
    C: Send + 'static,
    E: Packet,
    F: FrameCodec,
{
    let preprocessed = fragment_and_encode(fragmenter, payload, DEFAULT_FRAGMENT_ROUTE_ID)?;
    let frames = drive_preencoded(
        |server| run_owned_app(app, server),
        codec,
        preprocessed,
        capacity,
    )
    .await?;
    Ok(extract_payloads::<F>(&frames))
}
```

```rust
pub async fn drive_with_fragments_mut<S, C, E, F>(
    app: &mut WireframeApp<S, C, E, F>,
    codec: &F,
    fragmenter: &Fragmenter,
    payload: Vec<u8>,
) -> io::Result<Vec<Vec<u8>>>
where
    S: TestSerializer,
    C: Send + 'static,
    E: Packet,
    F: FrameCodec,
{
    let preprocessed = fragment_and_encode(fragmenter, payload, DEFAULT_FRAGMENT_ROUTE_ID)?;
    let frames = drive_preencoded(
        |server| async move { app.handle_connection(server).await },
        codec,
        preprocessed,
        DEFAULT_CAPACITY,
    )
    .await?;
    Ok(extract_payloads::<F>(&frames))
}
```

```rust
pub async fn drive_with_fragment_frames<S, C, E, F>(
    app: WireframeApp<S, C, E, F>,
    codec: &F,
    fragmenter: &Fragmenter,
    payload: Vec<u8>,
) -> io::Result<Vec<F::Frame>>
where
    S: TestSerializer,
    C: Send + 'static,
    E: Packet,
    F: FrameCodec,
{
    let preprocessed = fragment_and_encode(fragmenter, payload, DEFAULT_FRAGMENT_ROUTE_ID)?;
    drive_preencoded(
        |server| run_owned_app(app, server),
        codec,
        preprocessed,
        DEFAULT_CAPACITY,
    )
    .await
}
```

With this, you can delete `FragmentRequest` and `drive_fragments_internal`; the structure now mirrors the other helpers (partial frame, etc.) but avoids carrying an extra request type while still centralizing the encode/drive/decode logic in a single internal helper.
</issue_to_address>

[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. Include the file and symbol names indicated in the issue at the head of your response.

src/testkit/support.rs

Comment on file

pub(crate) async fn drive_internal<F, Fut>(
    server_fn: F,
    frames: Vec<Vec<u8>>,
    capacity: usize,
) -> io::Result<Vec<u8>>
where
    F: FnOnce(DuplexStream) -> Fut,
    Fut: std::future::Future<Output = ()> + Send,
{
    drive_with_strategies(
        server_fn,
        capacity,
        |mut writer| async {
            write_frames(&mut writer, &frames).await?;
            Ok(writer)
        },
        read_all,
    )
    .await
}

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: drive_chunked_internal,drive_internal

[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. Include the file and symbol names indicated in the issue at the head of your response.

tests/testkit_exports.rs

Comment on file

fn slow_io_config_validate_rejects_reader_chunk_exceeding_capacity() -> io::Result<()> {
    let capacity = 4;
    let chunk_size =
        NonZeroUsize::new(capacity * 2).ok_or_else(|| io::Error::other("non-zero chunk size"))?;

    let config = SlowIoConfig::new()
        .with_capacity(capacity)
        .with_reader_pacing(SlowIoPacing::new(chunk_size, Duration::from_millis(1)));

    let err = config
        .validate()
        .expect_err("expected validate to fail when reader chunk exceeds capacity");

    if err.kind() != io::ErrorKind::InvalidInput {
        return Err(io::Error::other(format!(
            "expected InvalidInput, got {:?}",
            err.kind()
        )));
    }
    Ok(())
}

❌ New issue: Code Duplication
The module contains 4 functions with similar structure: slow_io_config_validate_rejects_capacity_over_max,slow_io_config_validate_rejects_reader_chunk_exceeding_capacity,slow_io_config_validate_rejects_writer_chunk_exceeding_capacity,slow_io_config_validate_rejects_zero_capacity

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Comment 3: Outcomes & Retrospective section ==="
grep -n -A 10 "Outcomes & Retrospective" docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md | head -20

echo ""
echo "=== Comment 4: BDD test selector ==="
grep -n "testkit_export" docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md | tail -20

echo ""
echo "=== ChunkConfig still present? ==="
rg -n "ChunkConfig" src/testkit/

echo ""
echo "=== FragmentRequest still present? ==="
rg -n "FragmentRequest" src/testkit/

echo ""
echo "=== drive_with_strategies in support.rs ==="
rg -n "drive_with_strategies" src/testkit/support.rs | head -10

echo ""
echo "=== SlowIoConfig validate tests ==="
grep -n "slow_io_config_validate" tests/testkit_exports.rs

echo ""
echo "=== drive_slow_internal still present? ==="
rg -n "drive_slow_internal" src/testkit/slow_io.rs | head -5

Length of output: 2473

---

🏁 Script executed:

#!/bin/bash
echo "=== drive_slow_internal body ==="
sed -n '205,240p' src/testkit/slow_io.rs

echo ""
echo "=== execplan lines 370-435 (comment 4 context) ==="
sed -n '370,435p' docs/execplans/8-5-4-export-wireframe-testkit-behind-feature.md

Length of output: 3839

---

@leynos Here is the status of each item. No annulments are required — none of the comments request en-GB spelling changes, and the complexity-related comments all reduce rather than increase cyclomatic complexity.

---

Overall Comments

	Status
Slow I/O reimplements duplex/panic logic from support.rs	✅ Resolved — drive_slow_internal now delegates to drive_with_strategies (confirmed below)
Centralise io::Error construction for argument validation	✅ Resolved — SlowIoConfig::validate centralises those checks; individual drivers call it

---

Individual Comments

#	Location	Status
1	support.rs — drive_internal/drive_chunked_internal duplication	✅ Resolved — drive_with_strategies introduced at line 81; both functions delegate to it at lines 171 and 193
2	tests/testkit_exports.rs — SlowIoConfig::validate failure-path tests	✅ Resolved — four tests present: slow_io_config_validate_rejects_zero_capacity (line 182), …rejects_capacity_over_max (199), …rejects_writer_chunk_exceeding_capacity (216), …rejects_reader_chunk_exceeding_capacity (239)
3	docs/execplans/… — stale "Not started yet" opening	✅ Resolved — replaced with "This section records the concrete outcomes and validation for the implemented plan:"
4	docs/execplans/… — BDD test selector typo	⚠️ Partially resolved — Stage G cargo commands are now correct (testkit_exports targets the integration test at tests/testkit_exports.rs; the BDD command correctly uses singular testkit_export). However, Stage E (the plan text) still lists stale plural filenames that do not match the implemented artefacts:

# Stage E still says:          # Actual implemented files:
testkit_exports.feature    →   testkit_export.feature
testkit_exports.rs         →   testkit_export.rs
testkit_exports_steps.rs   →   testkit_export_steps.rs
testkit_exports_scenarios.rs → testkit_export_scenarios.rs

#	Location	Status
5	src/testkit/slow_io.rs — drive_slow_internal complexity	✅ Resolved — drive_slow_internal now calls drive_with_strategies, passing write_with_optional_pacing and read_with_optional_pacing as strategy closures; the doc-comment confirms this explicitly
6	src/testkit/partial_frame.rs — ChunkConfig wrapper	✅ Resolved — ChunkConfig is absent from src/testkit/
7	src/testkit/fragment_drive.rs — FragmentRequest abstraction	✅ Resolved — FragmentRequest is absent from src/testkit/

---

The one remaining item is the stale plural filenames in the Stage E plan text of the execplan document. Update those four lines to the singular names to keep the plan consistent with what was actually delivered.

[coderabbitai]

♻️ Duplicate comments (3)

tests/scenarios/testkit_export_scenarios.rs (1)

11-13: 🧹 Nitpick | 🔵 Trivial

Remove no-op bindings in scenario stubs.

Rename the fixture parameter to _testkit_export_world and delete the no-op statements on Line 12 and Line 20.

♻️ Proposed cleanup

 fn root_testkit_partial_frame_export(testkit_export_world: TestkitExportWorld) {
-    let _ = &testkit_export_world;
+fn root_testkit_partial_frame_export(_testkit_export_world: TestkitExportWorld) {
 }
@@
-fn root_testkit_reassembly_export(testkit_export_world: TestkitExportWorld) {
-    let _ = &testkit_export_world;
+fn root_testkit_reassembly_export(_testkit_export_world: TestkitExportWorld) {
 }

Also applies to: 19-21

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/scenarios/testkit_export_scenarios.rs` around lines 11 - 13, Rename the
fixture parameter in the scenario stub function
root_testkit_partial_frame_export from testkit_export_world to
_testkit_export_world and remove the unused no-op binding statements (the "let _
= &testkit_export_world;" and the other identical no-op around lines 19-21) so
the parameter is clearly marked as intentionally unused and the redundant
statements are deleted; update the function signature only
(root_testkit_partial_frame_export) and delete the two no-op lines referenced.

tests/testkit_exports.rs (2)

97-109: ⚠️ Potential issue | 🟡 Minor

Assert fragmented round-trip payload equality.

Decode and compare the echoed payload to b"fragment-test" in this branch. Keep non-empty checks as guards, but stop treating them as sufficient correctness proof.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/testkit_exports.rs` around lines 97 - 109, The test currently only
checks for non-empty frames but must assert the round-trip payload equals
b"fragment-test"; after obtaining first_payload (from
drive_with_fragments/app/codec/fragmenter/serialize_envelope), decode or
deserialize the envelope/frame (e.g., via the test helper used elsewhere such as
deserialize_envelope or the codec's decode method) and assert its inner payload
equals b"fragment-test" (keeping the existing non-empty guards). Ensure you
reference serialize_envelope, drive_with_fragments, codec, fragmenter, and
first_payload when locating where to insert the decode + equality assertion.

---

243-269: 🧹 Nitpick | 🔵 Trivial

Replace these twin validation tests with one parameterized case.

Collapse writer/reader chunk-exceeding cases into a single parameterized test path with a pacing-applier input to remove the remaining duplication.

As per coding guidelines, "Replace duplicated tests with #[rstest(...)] parameterised cases."

Run this read-only check to confirm the duplicated structure before refactoring:

#!/bin/bash
rg -n -C 4 'slow_io_config_validate_rejects_(writer|reader)_chunk_exceeding_capacity|with_(writer|reader)_pacing' tests/testkit_exports.rs

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/testkit_exports.rs` around lines 243 - 269, The two nearly identical
tests slow_io_config_validate_rejects_writer_chunk_exceeding_capacity and
slow_io_config_validate_rejects_reader_chunk_exceeding_capacity should be
collapsed into a single parameterized rstest that takes a pacing-applier
function (e.g., a FnOnce(SlowIoConfig, NonZeroUsize) -> SlowIoConfig) and runs
the same validation logic; create a NonZeroUsize chunk_size from capacity*2,
build a base SlowIoConfig::new().with_capacity(capacity), apply the
pacing-applier to set either with_writer_pacing or with_reader_pacing using
SlowIoPacing::new(chunk_size, Duration::from_millis(1)), then call
assert_slow_io_config_rejects(config, "..."); annotate with #[rstest(...)] to
pass both appliers (one that calls with_writer_pacing, one that calls
with_reader_pacing) to the single test function.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@tests/scenarios/testkit_export_scenarios.rs`:
- Around line 11-13: Rename the fixture parameter in the scenario stub function
root_testkit_partial_frame_export from testkit_export_world to
_testkit_export_world and remove the unused no-op binding statements (the "let _
= &testkit_export_world;" and the other identical no-op around lines 19-21) so
the parameter is clearly marked as intentionally unused and the redundant
statements are deleted; update the function signature only
(root_testkit_partial_frame_export) and delete the two no-op lines referenced.

In `@tests/testkit_exports.rs`:
- Around line 97-109: The test currently only checks for non-empty frames but
must assert the round-trip payload equals b"fragment-test"; after obtaining
first_payload (from
drive_with_fragments/app/codec/fragmenter/serialize_envelope), decode or
deserialize the envelope/frame (e.g., via the test helper used elsewhere such as
deserialize_envelope or the codec's decode method) and assert its inner payload
equals b"fragment-test" (keeping the existing non-empty guards). Ensure you
reference serialize_envelope, drive_with_fragments, codec, fragmenter, and
first_payload when locating where to insert the decode + equality assertion.
- Around line 243-269: The two nearly identical tests
slow_io_config_validate_rejects_writer_chunk_exceeding_capacity and
slow_io_config_validate_rejects_reader_chunk_exceeding_capacity should be
collapsed into a single parameterized rstest that takes a pacing-applier
function (e.g., a FnOnce(SlowIoConfig, NonZeroUsize) -> SlowIoConfig) and runs
the same validation logic; create a NonZeroUsize chunk_size from capacity*2,
build a base SlowIoConfig::new().with_capacity(capacity), apply the
pacing-applier to set either with_writer_pacing or with_reader_pacing using
SlowIoPacing::new(chunk_size, Duration::from_millis(1)), then call
assert_slow_io_config_rejects(config, "..."); annotate with #[rstest(...)] to
pass both appliers (one that calls with_writer_pacing, one that calls
with_reader_pacing) to the single test function.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4a7fd138-459c-4693-8906-e9b3a2a6b6ee

📥 Commits

Reviewing files that changed from the base of the PR and between 906f49f and eebc143.

📒 Files selected for processing (2)

• tests/scenarios/testkit_export_scenarios.rs
• tests/testkit_exports.rs

[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. Include the file and symbol names indicated in the issue at the head of your response.

Code Duplication

src/testkit/support.rs:

What lead to degradation?

The module contains 2 functions with similar structure: drive_chunked_internal,drive_internal

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.

@leynos This issue was already addressed earlier in this comment thread. The fix and AI coding agent prompt are in my reply immediately above the final leynos: @coderabbitai Please suggest a fix… comment (the one referencing the CodeScene report).

To summarise what was proposed there:

File: src/testkit/support.rs
Symbols: drive_internal, drive_chunked_internal

Extract two file-private async strategy adapters — write_frames_strategy and write_chunked_strategy — that take WriteHalf<DuplexStream> by value, delegate to the existing write_frames/write_chunked &mut helpers, and return io::Result<WriteHalf<DuplexStream>>. Both drive_internal and drive_chunked_internal then reduce to a single drive_with_strategies(…).await call, with no inline closure boilerplate repeated.

The full diff and the verbatim AI coding agent prompt are in that earlier reply — please refer to it directly to avoid duplication here.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

tests/testkit_exports.rs (1)

97-109: ⚠️ Potential issue | 🟡 Minor

Assert fragmented round-trip payload equality.

Replace the non-empty-only checks on Lines 99-109 with a decode-and-compare assertion against b"fragment-test" so this test proves route correctness instead of mere output presence.

🛠️ Suggested patch

     let payload = serialize_envelope(b"fragment-test")?;
     let response_payloads = drive_with_fragments(app, &codec, &fragmenter, payload).await?;
-    if response_payloads.is_empty() {
-        return Err(io::Error::other("expected non-empty response payloads"));
-    }
     let first_payload = response_payloads
         .first()
         .ok_or_else(|| io::Error::other("expected response payload"))?;
-    // Verify response contains data - drive_with_fragments returns wire frames which may include
-    // fragment headers for fragmented payloads
-    if first_payload.is_empty() {
-        return Err(io::Error::other("expected non-empty response frame"));
+    let echoed_payload = deserialize_payload(first_payload)?;
+    if echoed_payload != b"fragment-test" {
+        return Err(io::Error::other(format!(
+            "unexpected fragmented echoed payload: {echoed_payload:?}"
+        )));
     }
     Ok(())
 }

As per coding guidelines, "Write unit and behavioural tests for new Rust functionality."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/testkit_exports.rs` around lines 97 - 109, The test currently only
checks for non-empty response frames; instead decode the returned frame(s) and
assert the payload equals b"fragment-test". Use the same codec/serializer used
to send the request (symbols: serialize_envelope, drive_with_fragments, codec,
fragmenter, response_payloads, first_payload) to decode/reconstruct the envelope
from first_payload (or reassemble fragments if necessary) and compare the
decoded bytes to b"fragment-test" with an assertion rather than returning on
emptiness.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/testkit_exports.rs`:
- Around line 243-245: The function signature for
assert_slow_io_rejects_chunk_exceeding_capacity is not formatted per rustfmt;
run the repository formatter (make fmt) to apply the canonical formatting to
tests/testkit_exports.rs (so the signature and surrounding lines match rustfmt
rules), then verify with make check-fmt and commit the updated file; ensure the
change only adjusts whitespace/formatting around
assert_slow_io_rejects_chunk_exceeding_capacity and does not modify logic.

---

Duplicate comments:
In `@tests/testkit_exports.rs`:
- Around line 97-109: The test currently only checks for non-empty response
frames; instead decode the returned frame(s) and assert the payload equals
b"fragment-test". Use the same codec/serializer used to send the request
(symbols: serialize_envelope, drive_with_fragments, codec, fragmenter,
response_payloads, first_payload) to decode/reconstruct the envelope from
first_payload (or reassemble fragments if necessary) and compare the decoded
bytes to b"fragment-test" with an assertion rather than returning on emptiness.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 62f9a5d9-312e-47d8-8be6-b4b63a8e4b3d

📥 Commits

Reviewing files that changed from the base of the PR and between eebc143 and ca05683.

📒 Files selected for processing (1)

• tests/testkit_exports.rs

[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. Include the file and symbol names indicated in the issue at the head of your response.

Code Duplication

src/testkit/support.rs:

What lead to degradation?

The module contains 2 functions with similar structure: drive_chunked_internal,drive_internal

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
cat src/testkit/support.rs

Length of output: 8688

---

@leynos This fix is already in place. src/testkit/support.rs currently contains both write_frames_strategy and write_chunked_strategy as file-private async strategy adapters, and both drive_internal and drive_chunked_internal already delegate to drive_with_strategies via those adapters — exactly as proposed earlier in this thread. No further changes are needed.

(ᵔ◡ᵔ)∫

[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. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

Code Duplication

src/testkit/support.rs:

What lead to degradation?

The module contains 2 functions with similar structure: drive_chunked_internal,drive_internal

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
cat src/testkit/support.rs

Length of output: 8688

---

@leynos

File: src/testkit/support.rs
Symbols: drive_internal, drive_chunked_internal

---

The fix is already fully applied. Both functions delegate to drive_with_strategies via the file-private write_frames_strategy / write_chunked_strategy adapters introduced earlier in this thread. The residual structural similarity CodeScene detects is:

• identical where F: FnOnce(DuplexStream) -> Fut, Fut: Future<Output = ()> + Send bounds, and
• identical drive_with_strategies(server_fn, capacity, |writer| …, read_all).await call shape.

Further unification would be deleterious. Merging the two into a single function would require either removing them and pushing strategy-closure construction into every call site (scattering complexity), or introducing a runtime discriminator (enum/Option) to select the write strategy — adding indirection with no semantic benefit. Both functions are already minimal, single-expression convenience shims whose sole purpose is to hide the strategy wiring. The similarity is inherent, not accidental, because they wrap the same underlying abstraction with legitimately distinct parameters.

---

CodeScene suppression message

drive_internal and drive_chunked_internal are intentional, minimal convenience wrappers over the shared drive_with_strategies orchestrator. They differ only in write strategy: drive_internal passes batched frames via write_frames_strategy, while drive_chunked_internal passes a byte slice in fixed-size chunks via write_chunked_strategy. The write strategies themselves were extracted precisely to remove the previously duplicated closure boilerplate; what remains is the irreducible structural skeleton shared by any two functions that delegate to the same orchestrator with different arguments.

Further unification — either by removing both shims and pushing strategy-closure construction to every call site, or by introducing a runtime discriminator to select the write strategy — would increase call-site complexity or add indirection without benefit. The similarity is inherent to the abstraction, not a DRY violation, and suppressing this diagnostic is appropriate.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/testkit_exports.rs`:
- Line 247: The error message for NonZeroUsize::new(capacity *
2).ok_or_else(...) is a stale literal ("8 is non-zero"); replace it with a
value-agnostic message that reflects the actual check, e.g. change the string to
something like "capacity * 2 is zero" or "zero capacity" so the failure text
remains correct if capacity changes, leaving the call to
NonZeroUsize::new(capacity * 2) and the surrounding logic unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 010bcfa3-5e11-4e54-be3e-ad04bbf606d5

📥 Commits

Reviewing files that changed from the base of the PR and between 2661364 and 5c5211b.

📒 Files selected for processing (1)

• tests/testkit_exports.rs

[leynos]

@coderabbitai should we make WireframeError an implementation of std::error::Error? This feels like a gap in the library architecture.

2. ❌ Cannot fix: #[from] attribute for Wireframe variant in src/testkit/result.rs

I attempted to add #[from] to the Wireframe(WireframeError) variant and remove the manual From implementation, but this fails to compile because:

• WireframeError is a generic type: WireframeError<E = ()>
• The #[from] attribute requires that the type implements StdError
• Due to its generic parameter, WireframeError does not satisfy the trait bounds required by thiserror's #[from] derivation

This is exactly what was noted in the session summary - the #[from] attribute cannot be used with WireframeError due to trait bound restrictions, so the manual From implementation must remain.

[leynos]

@coderabbitai is this something we should devote further time to?

2. Fragment test payload validation (tests/testkit_exports.rs)

⚠️ Cannot implement as specified - The review comment requested deserializing the fragment response and asserting it equals b"fragment-test". However, drive_with_fragments returns codec wire frames that contain FRAG headers (fragment protocol frames), not bincode-serialized envelopes.

The test response bytes are [70, 82, 65, 71, ...] ("FRAG..."), which cannot be deserialized by deserialize_payload (which expects bincode-serialized Envelope structs). The partial-frame test works because drive_with_partial_frames returns complete envelopes without fragmentation protocol overhead.

Current implementation: Validates that response payloads are non-empty, which is the correct assertion for fragmented wire frames.

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== WireframeError definition ==="
rg -n 'WireframeError' src/ --type rust -A 3 -B 1 | head -80

echo ""
echo "=== Does WireframeError derive/impl Error? ==="
rg -n 'impl.*Error.*WireframeError\|WireframeError.*Error\|derive.*Error' src/ --type rust | head -20

echo ""
echo "=== Full WireframeError struct/enum definition ==="
ast-grep --pattern 'pub enum WireframeError<$$$> {
  $$$
}'

echo ""
echo "=== Check if thiserror is used ==="
rg -n 'thiserror' Cargo.toml wireframe_testing/Cargo.toml

Length of output: 4868

---

@leynos — yes, this is a genuine gap worth addressing. Here is the analysis and a concrete recommendation.

---

The gap

WireframeError<E> is a public library error type but derives only Debug. Without Display and std::error::Error, it cannot participate in the standard error ecosystem: it cannot be wrapped by anyhow, returned through Box<dyn Error>, used with thiserror's #[from] or #[source], or inspected via std::error::Error::source.

The blocking constraint is the Protocol(E) variant. std::error::Error requires Display, and Display requires E: Display. The default E = () satisfies neither, so a single blanket impl cannot cover the full type.

---

Recommended approach: two complementary impls

1. Blanket impl covering all real protocol-error types:

use std::fmt;

impl<E: fmt::Display + fmt::Debug> fmt::Display for WireframeError<E> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            Self::DuplicateRoute(id) => write!(f, "duplicate route: {id}"),
            Self::Io(e)             => write!(f, "I/O error: {e}"),
            Self::Protocol(e)       => write!(f, "protocol error: {e}"),
            Self::Codec(e)          => write!(f, "codec error: {e}"),
        }
    }
}

impl<E: std::error::Error + 'static> std::error::Error for WireframeError<E> {
    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
        match self {
            Self::Io(e)    => Some(e),
            Self::Codec(e) => Some(e),
            Self::Protocol(e) => Some(e),
            Self::DuplicateRoute(_) => None,
        }
    }
}

2. Specific impl for the E = () default (unlocks #[from] in TestError):

impl fmt::Display for WireframeError<()> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            Self::DuplicateRoute(id) => write!(f, "duplicate route: {id}"),
            Self::Io(e)             => write!(f, "I/O error: {e}"),
            Self::Protocol(())      => write!(f, "protocol error"),
            Self::Codec(e)          => write!(f, "codec error: {e}"),
        }
    }
}

impl std::error::Error for WireframeError<()> {
    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
        match self {
            Self::Io(e)    => Some(e),
            Self::Codec(e) => Some(e),
            _              => None,
        }
    }
}

These two impls are not overlapping because () does not implement std::error::Error.

---

Downstream effect on TestError

Once WireframeError<()> implements std::error::Error, the manual From impl in src/testkit/result.rs can be replaced with #[from]:

#[error(transparent)]
Wireframe(#[from] WireframeError),

and the impl From<WireframeError> for TestError block deleted entirely.

---

Suggested path

This change is small, additive, and non-breaking (adding new trait impls cannot break existing callers). It is worth doing as a follow-up to this PR rather than in-scope, to keep this PR focused. Would you like me to open a GitHub issue to track it?

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== drive_with_fragments return type and implementation ==="
rg -n 'pub async fn drive_with_fragments' src/testkit/fragment_drive.rs -A 15

echo ""
echo "=== Current fragment test in testkit_exports.rs ==="
rg -n 'fragment' tests/testkit_exports.rs -A 10 -B 2

echo ""
echo "=== What does drive_with_fragments return - trace through extract_payloads ==="
rg -n 'extract_payloads\|decode_frames_with_codec' src/testkit/support.rs -A 10

echo ""
echo "=== Fragment reassembly helpers exported from testkit ==="
rg -n 'assert_fragment_reassembly\|drive_with_fragment' src/testkit/mod.rs

Length of output: 5549

---

@leynos — No, this does not warrant further time.

Here is the reasoning:

The echo app echoes the raw FRAG-encoded fragment envelopes back across the wire. drive_with_fragments decodes those responses using the frame codec and then calls extract_payloads, yielding the FRAG wire bytes as payload content — not reassembled application data. Asserting equality to b"fragment-test" would therefore require collecting those FRAG frames and running them through the full Fragmenter reassembly machinery before comparing. That machinery is precisely what the assert_fragment_reassembly_completed_bytes helper and FragmentReassemblySnapshot are for — and they are exercised directly in root_testkit_exports_reassembly_assertions.

The test's purpose is narrow and correct: validate that drive_with_fragments is exported and produces output. The non-empty check is the right assertion at that boundary. Wiring up a full reassembly pass here would test the Fragmenter crate logic rather than the testkit export surface, conflate two concerns in a single test, and substantially increase complexity for no net gain in coverage.

Leave the test as-is.