FragmentAdapter trait and DefaultFragmentAdapter; opt-in fragmentation#457
FragmentAdapter trait and DefaultFragmentAdapter; opt-in fragmentation#457
Conversation
…t hardening - Introduces a comprehensive ExecPlan draft for roadmap item 9.2.1 focused on the FragmentAdapter trait and fragmentation opt-in hardening. - Covers purpose, constraints, tolerances, risks, progress, decisions, and plans. - Defines public API shape, behavioral policies, and testing strategy. - Guides staged implementation and documentation updates. - Serves as a reference point for fragmentation-related development and QA. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Reviewer's GuideIntroduces a public FragmentAdapter trait and DefaultFragmentAdapter implementation, rewires app fragmentation to use this adapter with fragmentation disabled by default and explicitly opt-in via builder APIs, hardens fragment-series policies (duplicate suppression, out-of-order handling, zero-length support) and reassembler behaviour, expands unit/integration/BDD tests for the new contract (including interleaved streams), and updates documentation and roadmap to reflect the new adapter contract, opt-in semantics, purge ownership, and upgraded rstest-bdd dev-dependencies. Sequence diagram for inbound and outbound fragmentation with FragmentAdaptersequenceDiagram
actor Developer
participant Builder as WireframeAppBuilder
participant App as WireframeApp
participant Handler
participant Adapter as FragmentAdapter
participant Codec as FrameCodec
participant Socket
participant Timer as ReadTimeoutScheduler
rect rgb(235,235,255)
Developer->>Builder: new()
Builder-->>Developer: App (fragmentation None)
Developer->>Builder: enable_fragmentation()
Builder->>Builder: derive FragmentationConfig from codec.max_frame_length()
Builder->>Builder: create DefaultFragmentAdapter
Builder-->>Developer: App (fragmentation Some(DefaultFragmentAdapter))
end
rect rgb(235,255,235)
Handler->>App: send_response(envelope)
App->>Adapter: fragment(envelope)
Adapter-->>App: fragments Vec~Envelope~
loop for each fragment
App->>Codec: encode(fragment)
Codec-->>App: frame
App->>Socket: write(frame)
end
end
rect rgb(255,235,235)
Socket-->>Codec: frame
Codec-->>App: packet
App->>Adapter: reassemble(packet)
Adapter-->>App: Option~packet~
alt complete message
App-->>Handler: deliver reassembled envelope
else more fragments required
App-->>Handler: no callback yet
end
end
rect rgb(255,255,210)
Timer-->>App: read timeout tick
App->>Adapter: purge_expired()
Adapter-->>App: Vec~MessageId~
App->>App: log purged message ids
end
Class diagram for FragmentAdapter trait and DefaultFragmentAdapter implementationclassDiagram
direction LR
class Fragmentable {
<<trait>>
}
class FragmentAdapter {
<<trait>>
+fragment(packet: E) Result~Vec~E~~
+reassemble(packet: E) Result~Option~E~~
+purge_expired() Vec~MessageId~
}
class DefaultFragmentAdapter {
+fragmenter: Fragmenter
+reassembler: Reassembler
+new(config: FragmentationConfig) DefaultFragmentAdapter
+fragment(packet: E) Result~Vec~E~~
+reassemble(packet: E) Result~Option~E~~
+purge_expired() Vec~MessageId~
-fragment_inner(packet: E) Result~Vec~E~~
-reassemble_inner(packet: E) Result~Option~E~~
-purge_expired_inner() Vec~MessageId~
}
class Fragmenter {
+new(fragment_payload_cap: usize) Fragmenter
}
class Reassembler {
+new(max_message_size: usize, reassembly_timeout: Duration) Reassembler
+push(header: FragmentHeader, payload: &[u8]) Result~Option~ReassembledMessage~~
+purge_expired() Vec~MessageId~
}
class FragmentAdapterError {
<<enum>>
+Decode(DecodeError)
+Reassembly(ReassemblyError)
}
class FragmentStatus {
<<enum>>
+Incomplete
+Duplicate
+Complete
}
class FragmentSeries {
+message_id: MessageId
+next_index: FragmentIndex
+complete: bool
+accept(fragment: FragmentHeader) Result~FragmentStatus, FragmentError~
}
class FragmentationConfig {
+fragment_payload_cap: usize
+max_message_size: usize
+reassembly_timeout: Duration
}
class FragmentationState {
<<type_alias>>
}
class FragmentProcessError {
<<type_alias>>
}
FragmentAdapter <|.. DefaultFragmentAdapter : implements
DefaultFragmentAdapter --> Fragmenter : uses
DefaultFragmentAdapter --> Reassembler : uses
FragmentAdapter --> Fragmentable : E constraint
FragmentAdapter --> FragmentAdapterError
FragmentAdapter --> FragmentationError
FragmentAdapter --> MessageId
FragmentSeries --> FragmentStatus
DefaultFragmentAdapter --> FragmentationConfig
FragmentationState --> DefaultFragmentAdapter : alias of
FragmentProcessError --> FragmentAdapterError : alias of
Flow diagram for builder fragmentation configuration and codec changesflowchart LR
A[WireframeApp_default\nfragmentation None] --> B{Developer calls enable_fragmentation}
B -->|Yes| C[Compute FragmentationConfig\nfrom codec.max_frame_length]
C --> D[Create DefaultFragmentAdapter]
D --> E[Store Some_DefaultFragmentAdapter\ninto App.fragmentation]
B -->|No| F[Fragmentation remains disabled]
E --> G{Developer calls with_codec}
F --> G
G --> H[Rebuild app with new codec]
H --> I[Reset fragmentation to None\nrequires explicit reconfiguration]
I --> J{Developer calls buffer_capacity}
J --> K[Update LengthDelimitedFrameCodec\nwith new capacity]
K --> L[Fragmentation remains None until\nexplicit enable_fragmentation or fragmentation_Some_cfg]
subgraph Explicit_config
M[Developer calls fragmentation_Some_cfg]
M --> N[Create DefaultFragmentAdapter\nfrom provided config]
N --> O[Store Some_DefaultFragmentAdapter\ninto App.fragmentation]
end
I --> M
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduce a public Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as WireframeApp\n(builder/runtime)
participant Adapter as DefaultFragmentAdapter\n(FragmentAdapter)
participant Codec as Codec/Framing
participant Handler as ApplicationHandler
App->>Adapter: fragment(packet) [outbound when opt-in]
Adapter->>Adapter: split payload -> fragments
Adapter->>Codec: emit per-fragment frames
Codec->>App: send frames
%% inbound path
Codec->>Adapter: deliver frame -> decode header+payload
Adapter->>Adapter: reassemble(fragment) -> check duplicate / index
alt complete
Adapter->>Handler: deliver reassembled packet
else incomplete
Adapter-->>Adapter: buffer partial state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
…e FragmentAdapter API - Introduce a public `FragmentAdapter` trait and `DefaultFragmentAdapter` implementation to unify fragmentation and reassembly behavior. - Shift fragmentation from default-enabled to explicit opt-in via `enable_fragmentation()` or `fragmentation(Some(cfg))` on `WireframeApp` builder. - Make `with_codec()` clear fragmentation config to require reconfiguration aligned with codec frame budget. - Add explicit duplicate fragment suppression (non-fatal) and preserve out-of-order rejection with deterministic cleanup. - Expose caller-driven purge API via `FragmentAdapter::purge_expired()` for manual timeout eviction control. - Update related docs, design ADRs, and user guides to reflect new fragmentation control and policies. - Add comprehensive unit, integration, and behavioral tests verifying opt-in semantics, duplicate suppression, zero-length fragment support, interleaved reassembly, and purge API usage. This change improves resource control, clarity, and flexibility of transport fragmentation handling, preventing unintended overhead and enabling tailored fragment handling strategies. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
…t types - Updated FragmentAdapter trait to remove generic parameter on trait and instead use generic methods for fragment and reassemble. - Adjusted DefaultFragmentAdapter implementation accordingly. - Improved error handling with #[from] attributes on FragmentAdapterError variants. - Revised related documentation to use "adapter" instead of "adaptor" and align terminology. - Split large fragment tests module into focused submodules for clarity and maintainability. - Added new well-structured unit tests for fragment adapter, fragmenter, header series, and reassembler. - Updated builder and codec modules to require explicit opt-in for fragmentation when frame budgets or codec change. This refactor improves the fragment adapter API ergonomics and type safety, and enhances test coverage and documentation clarity. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
|
@coderabbitai Have the following now been resolved? |
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/execplans/9-1-3-fragment-adapter-trait.md`:
- Line 18: The sentences that read "public `FragmentAdapter` trait and aligns
runtime behaviour so fragmentation is" and the other occurrences join two
independent clauses with "so" but lack the required comma; update each sentence
(including the one mentioning the FragmentAdapter trait and the two other
occurrences later in the document) to insert a comma before "so" so they read
"...behaviour, so fragmentation..." (and similarly add a comma before "so" in
the other two sentences).
- Line 352: The phrase uses the non-standard "users guide"; update the text that
reads "Documentation parity: users guide and design docs describe the same
behaviour" to use the conventional "user guide" instead. Locate the exact string
"users guide" in the document (e.g., within the sentence starting "Documentation
parity:") and replace it with "user guide", preserving the rest of the sentence
and punctuation.
In `@docs/users-guide.md`:
- Around line 842-844: Replace the US spelling "deserialization" with the
British English "deserialisation" in the documentation text (specifically the
phrase "codec decode → fragment reassembly → deserialization" and any other
occurrences in this file) to comply with the project's en-GB-oxendict
requirement; update the inbound processing line to read "codec decode → fragment
reassembly → deserialisation" and run a quick search in docs/users-guide.md to
change any other instances of "deserialization".
…an docs Correct multiple instances of "deserialization" to "deserialisation" and "documentation" to "documentation" to align with UK English spelling conventions in users-guide.md and 9-1-3-fragment-adapter-trait.md. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/users-guide.md (1)
367-367:⚠️ Potential issue | 🟡 MinorUse consistent terminology: "adapter" not "adaptor".
Replace "adaptor" with "adapter" to match the
FragmentAdaptertype name.✏️ Proposed fix
-logging. This keeps transport experiments lightweight while the full adaptor +logging. This keeps transport experiments lightweight while the full adapter layer evolves. The helper is fallible—`FragmentationError` surfaces encoding
🤖 Fix all issues with AI agents
In `@docs/execplans/9-1-3-fragment-adapter-trait.md`:
- Around line 28-29: Add a comma before "while" in the sentence "interleaved
fragment streams reassemble correctly while duplicate and out-of-order series
follow documented policies" so the two independent clauses are correctly
punctuated; update the sentence in the
docs/execplans/9-1-3-fragment-adapter-trait.md content where that phrase appears
(look for the sentence starting with "interleaved fragment streams reassemble
correctly") to read "...reassemble correctly, while duplicate and out-of-order
series follow documented policies."
- Around line 45-46: The filename token
"generic-message-fragmentation-and-re-assembly-design.md" in the bullet is being
split across lines; update the bullet so the filename remains atomic by either
letting that filename exceed the 80‑column wrap or rephrasing the sentence to
keep the full filename on one line (for example: "Record decisions in the
companion doc generic-message-fragmentation-and-re-assembly-design.md and other
related composition guidance."). Ensure the exact filename string is not
hyphen-wrapped or broken into multiple lines in the markdown.
Corrected instances of "adaptor" to "adapter" for consistent terminology in the users guide documentation. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/users-guide.md`:
- Line 121: Replace the British -ise spelling "deserialisation" with
en-GB-oxendict "-ize" form "deserialization" everywhere in the document
(instances currently written as "deserialisation" at the noted locations),
ensuring consistency in surrounding punctuation and footnote markers (e.g., keep
"[^6][^7]" intact) and update all other occurrences mentioned (the other two
instances) to "deserialization".
- Corrected comma usage for clarity in fragmentation design doc. - Fixed spelling from 'deserialisation' to 'deserialization' in multiple places. - Improved wording and formatting in execution plans and user guide docs. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Summary
Changes
API surface
Rationale
Migration / Compatibility
Tests plan
Validation and gates
Documentation updates
Notes for reviewers
📎 Task: https://www.devboxer.com/task/63aac90e-7ba4-4cf7-9a8d-d3661b768afc
📎 Task: https://www.devboxer.com/task/5836b8c0-0e9b-4447-a190-84233efc8219