Improve WireframeProtocol docs and fragment tests header

[leynos]

Summary

• Improves documentation for WireframeProtocol and its usage in with_protocol
• Adds descriptive header for fragmentation tests in fragment/tests.rs

Changes

Documentation

• src/hooks.rs: Expanded WireframeProtocol documentation to explain that while a custom ProtocolError type is allowed, WireframeApp::with_protocol currently requires ProtocolError = () to enable dynamic dispatch with a uniform interface across connections. Includes note about potential future relaxation.
• src/app/builder_protocol.rs: Updated with_protocol method docs to clearly state the current constraint of ProtocolError = () and the rationale for keeping the error type uniform and safe for the builder API.
• src/fragment/tests.rs: Added module-level documentation describing the fragmentation/reassembly test scope and coverage (e.g., FragmentHeader access, FragmentSeries ordering/validation, Fragmenter splitting and IDs, and Reassembler assembly with limits/expiry).

Tests

• No functional changes; documentation-focused updates only.

Test plan

• cargo test to ensure builds/tests are unaffected
• cargo doc to verify documentation compiles cleanly

◳ Generated by DevBoxer ◰

---

ℹ️ Tag @devboxerhub to ask questions and address PR feedback

📎 Task: https://www.devboxer.com/task/96d25cd1-2470-49fd-a839-d9c46ff4608c

📝 Closes #435

Summary by Sourcery

Document current ProtocolError = () constraint for pluggable protocols and clarify fragmentation test coverage.

Enhancements:

• Clarify WireframeProtocol documentation around use of custom ProtocolError and the current ProtocolError = () requirement when used with WireframeApp::with_protocol.
• Update WireframeApp::with_protocol builder docs to explain the rationale for enforcing a unit ProtocolError for dynamic dispatch and a uniform interface.
• Document the scope and behavior covered by fragmentation and reassembly tests in fragment/tests.rs without changing test logic.

Documentation:

• Expand protocol-related API documentation to better describe error type constraints and fragmentation test responsibilities.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Improves internal documentation around WireframeProtocol error type constraints and clarifies the scope of fragmentation/reassembly tests, without changing runtime behavior.

Class diagram for WireframeProtocol and WireframeApp with_protocol documentation constraint

classDiagram
    class WireframeProtocol {
        <<trait>>
        +type Frame
        +type ProtocolError
        +on_connect(ctx: ConnectionContext) void
        +before_send_frame(frame: Frame, ctx: ConnectionContext) void
        +on_frame(frame: Frame, ctx: ConnectionContext) void
        +on_command_complete(ctx: ConnectionContext) void
        +handle_error(error: ProtocolError, ctx: ConnectionContext) void
        +on_eof(error: EofError, partial_data: bytes, ctx: ConnectionContext) void
    }

    class WireframeApp {
        +builder() WireframeAppBuilder
    }

    class WireframeAppBuilder {
        +with_protocol(protocol: P) WireframeAppBuilder
    }

    WireframeAppBuilder --> WireframeApp : builds
    WireframeAppBuilder ..> WireframeProtocol : with_protocol<P>

    note for WireframeProtocol "When installed via WireframeApp.with_protocol, ProtocolError must currently be () for dynamic dispatch and a uniform interface across connections"

Loading

File-Level Changes

Change	Details	Files
Clarify WireframeProtocol error type usage and examples to match current WireframeApp::with_protocol constraints.	Expanded WireframeProtocol trait docs to explain that WireframeApp::with_protocol currently requires ProtocolError = () for dynamic dispatch and a uniform interface. Added note that the ProtocolError constraint may be relaxed in a future release. Updated documentation examples to use ProtocolError = () instead of a custom error type.	src/hooks.rs
Clarify builder with_protocol API constraints and rationale for ProtocolError = () requirement.	Augmented with_protocol method documentation to state that protocols must use ProtocolError = (). Explained safety and API design reasons for keeping error types uniform and not exposing application-specific errors via the builder.	src/app/builder_protocol.rs
Document the scope and coverage of fragmentation and reassembly tests.	Added module-level documentation describing what fragmentation tests cover, including FragmentHeader access, FragmentSeries ordering/validation, Fragmenter splitting/IDs, and Reassembler assembly with limits/expiry.	src/fragment/tests.rs

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#435	Clarify and correct documentation around WireframeProtocol and WireframeApp::with_protocol so that the documented ProtocolError behavior matches the actual constraint (ProtocolError = ()) or the code is generalized to support arbitrary protocol error types.	✅
#435	Add appropriate module-level //! documentation to src/fragment/tests.rs describing the fragmentation/reassembly test module’s purpose and coverage.	✅

Possibly linked issues

• Fix documentation inconsistencies and missing module docs #435: PR updates WireframeProtocol/with_protocol docs to note ProtocolError = () and adds fragment tests module docs as requested.

---

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]

Summary by CodeRabbit

• Documentation
  • Clarified protocol builder docs and updated examples to show the required unit-style error type for dynamic dispatch.
  • Expanded test module documentation covering fragmentation, reassembly, ID handling, size and expiry behaviours.

Walkthrough

State that documentation-only changes were applied: add a doc comment to with_protocol clarifying the ProtocolError = () requirement for dynamic dispatch, update WireframeProtocol-related docs and examples to use ProtocolError = (), and add module-level documentation to fragmentation tests. No public signatures or behaviour changed.

Changes

Cohort / File(s)	Summary
Protocol documentation src/app/builder_protocol.rs, src/hooks.rs	Added and updated documentation and examples to clarify that when using dynamic dispatch the protocol error type must be (). No API signature or behavioural changes.
Test module documentation src/fragment/tests.rs	Added module-level //! documentation describing the fragmentation and reassembly test coverage and intent. No test logic changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

✨ Docs aligned, the comments sing tonight,
Declare the error empty, tidy and bright,
Tests now framed with a guiding line,
Keep the interfaces neat and fine,
Ship the clarity — let the code take flight!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarises the main changes: improving WireframeProtocol documentation and adding fragmentation test module documentation.
Description check	✅ Passed	The description clearly relates to the changeset, detailing documentation improvements to hooks.rs, builder_protocol.rs, and tests.rs with specific rationale for each change.
Linked Issues check	✅ Passed	The PR fully addresses both objectives from issue #435: clarifying WireframeProtocol documentation regarding ProtocolError = () constraint [#435], and adding module-level documentation to fragment/tests.rs [#435].
Out of Scope Changes check	✅ Passed	All changes directly address the objectives from #435; documentation additions to hooks.rs, builder_protocol.rs, and fragment/tests.rs are entirely within scope with no extraneous modifications.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ 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 docs-fix-wireframeprotocol-fragment-tests-v0xtck

---

No actionable comments were generated in the recent review. 🎉

---

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

[leynos]

@coderabbitai Have the following now been resolved?

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src/hooks.rs:57-59` </location>
<code_context>
     /// impl WireframeProtocol for MyProtocol {
     ///     type Frame = Vec<u8>;
-    ///     type ProtocolError = String;
+    ///     type ProtocolError = ();
     ///
     ///     fn handle_error(&self, error: Self::ProtocolError, _ctx: &mut ConnectionContext) {
     ///         tracing::error!(error = %error, "protocol error");
</code_context>

<issue_to_address>
**issue (bug_risk):** The example using `ProtocolError = ()` conflicts with logging the error value.

With `type ProtocolError = ()`, the `error` parameter is always unit, so `tracing::error!(error = %error, ...)` is misleading and won’t compile because `()` doesn’t implement `Display`. To keep the example realistic and compilable, either:

* Use a concrete printable error type again (e.g. `String`) and add a brief note that `with_protocol` currently requires `ProtocolError = ()`, or
* Keep `ProtocolError = ()` but change the handler to log only a static message (without `%error`).
</issue_to_address>