Conversation
Documents all breaking changes and new capabilities introduced between the `v0.2.0` tag and HEAD, based on a semantic diff of the Rust source. Breaking changes covered: - `test-helpers` feature removed; `test-support` is the sole alias - `pool`, `testkit`, and `serializer-serde` features added - `wireframe::app::error::WireframeError` and `wireframe::response::WireframeError` removed; unified `wireframe::WireframeError<E>` at crate root - All root re-exports removed; module paths and `wireframe::prelude` required - `PacketParts::payload` / `FragmentParts::payload` → `into_payload` - `BackoffConfig::normalised` → `normalized` - `Serializer` trait bounds updated to `EncodeWith<S>` / `DecodeWith<S>` - `WireframeApp::new()` now returns `Result<Self>` - Server factory now expressed via `AppFactory` / `FactoryResult` traits - `AppDataStore` moved to `wireframe::app_data_store` - `EnvelopeId` and `CorrelationId` newtypes added to `wireframe::message_assembler` New capabilities covered: - `MemoryBudgets` / `BudgetBytes` with `memory_budgets()` and `enable_fragmentation()` builder methods - `wireframe::prelude` convenience import module - `wireframe::testkit` testing utilities module Client capabilities (new domain coverage, separate section): - Streaming responses via `call_streaming`, `ResponseStream`, `StreamingResponseExt`, and `TypedResponseStream` - Outbound streaming via `send_streaming` and `SendStreamingConfig` - Connection pooling via `WireframeClientPool`, `ClientPoolConfig`, `PoolHandle`, `PooledClientLease`, and `PoolFairnessPolicy` - Per-frame request hooks via `before_send` and `after_receive` - Structured tracing instrumentation via `TracingConfig`
Two sections were missing from the initial draft:
- **Codec improvements**: documents `FrameCodec::frame_payload_bytes`,
the new default method for zero-copy payload extraction. No existing
codec implementation requires change; the method is additive with a
copy-based default. Includes an override example for codecs that store
the frame payload as `Bytes` internally.
- **wireframe_testing additions**: documents three areas of new test
infrastructure in the companion crate:
- `WireframePair` / `spawn_wireframe_pair` – in-process server+client
pair harness over real loopback TCP
- `ObservabilityHandle` / `obs_handle` / `Labels` – combined log and
metrics capture fixture for codec error and connection metric
assertions
- Codec-aware drive helpers (`drive_with_codec_frames`,
`decode_frames_with_codec`, etc.) and Hotline protocol fixtures
(`valid_hotline_wire`, `oversized_hotline_wire`, `correlated_hotline_
wire`, and related functions) for codec regression tests
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Migration Guide for v0.2.0 → v0.3.0This PR adds a comprehensive migration guide at docs/v0-2-0-to-v0-3-0-migration-guide.md documenting breaking changes and new capabilities introduced in Wireframe v0.3.0, and a small ADR update (docs/adr-001-multi-packet-streaming-response-api.md) narrowing that ADR’s “Consequences” requirement to server-side multi-packet response handler documentation. Breaking changes documented
New capabilities documented
Documentation & reviewer feedback
Related design documents and execplans
Lines changed: +1193/-0 (documentation-only change). Estimated review effort: Low. WalkthroughDocument v0.2.0→v0.3.0 migration: add a comprehensive migration guide detailing Cargo feature changes, unified Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Pool as WireframeClientPool
participant Lease as PooledClientLease
participant Server as WireframeApp
participant Assembler as MessageAssembler
participant Store as AppDataStore
Client->>Pool: Acquire lease
Pool-->>Client: Return PooledClientLease
Client->>Lease: call_streaming / send_streaming
Lease->>Server: Send frames (with EnvelopeId/CorrelationId)
Server->>Assembler: Route/assemble frames (EnvelopeRouting)
Assembler->>Store: Persist/read fragments
Store-->>Assembler: Ack / fragments
Assembler-->>Server: AssembledMessage
Server-->>Lease: Streamed responses (ResponseStream)
Client->>Lease: receive_streaming / map via StreamingResponseExt
Lease->>Pool: Release lease
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
Reviewer's GuideAdd a comprehensive v0.2.0 → v0.3.0 migration guide documenting all breaking API changes, new configuration options, and expanded client/testing capabilities across core, client, codec, and companion crates. Sequence diagram for client streaming response consumptionsequenceDiagram
actor User
participant Client as WireframeClient
participant Server as WireframeServer
participant Stream as ResponseStream
User->>Client: call_streaming(envelope)
Client->>Server: send request frame(s)
Server-->>Client: stream response frames
Client-->>User: ResponseStream
loop consume stream
User->>Stream: try_next()
Stream->>Client: poll_next_frame()
Client->>Server: receive next frame
Server-->>Client: next frame or terminator
Client-->>Stream: frame or None
Stream-->>User: frame or None
end
User->>Stream: drop ResponseStream
Stream->>Client: release exclusive borrow
Sequence diagram for pooled client lease acquisition and requestsequenceDiagram
actor User
participant Builder as WireframeClientBuilder
participant Pool as WireframeClientPool
participant Handle as PoolHandle
participant Lease as PooledClientLease
participant Client as WireframeClient
participant Server as WireframeServer
User->>Builder: connect_pool(addr, ClientPoolConfig)
Builder->>Pool: create pool with sockets
Pool-->>User: WireframeClientPool
User->>Pool: handle()
Pool-->>Handle: PoolHandle
User->>Handle: acquire()
Handle->>Pool: acquire_socket()
Pool-->>Handle: PooledClientLease
Handle-->>User: PooledClientLease
User->>Lease: call(request_envelope)
Lease->>Client: call(request_envelope)
Client->>Server: send request
Server-->>Client: send response
Client-->>Lease: Response
Lease-->>User: Response
User->>Lease: drop lease
Lease->>Pool: return client to pool
Class diagram for core error, app factory, and memory budget changesclassDiagram
class WireframeError~E~ {
+DuplicateRoute code: u32
+Io error: std_io_Error
+Protocol error: E
+Codec error: CodecError
}
class Result~T~ {
<<type alias>>
}
class CodecError {
}
class WireframeApp {
+memory_budgets: MemoryBudgets
+new() Result~WireframeApp~
+memory_budgets(budgets: MemoryBudgets) WireframeApp
+enable_fragmentation() WireframeApp
}
class MemoryBudgets {
+per_message: BudgetBytes
+per_connection: BudgetBytes
+in_flight: BudgetBytes
+new(per_message: BudgetBytes, per_connection: BudgetBytes, in_flight: BudgetBytes) MemoryBudgets
}
class BudgetBytes {
+value: NonZeroUsize
+new(value: NonZeroUsize) BudgetBytes
}
class AppFactory~Ser, Ctx, E, Codec~ {
<<trait>>
+call() R
}
class FactoryResult~App~ {
<<trait>>
}
class WireframeServer {
+bind(addr: SocketAddr) WireframeServer
+run_with_shutdown(factory: AppFactory, signal: ShutdownSignal) Result~()~
}
class AppDataStore {
}
class EnvelopeId {
+value: u32
}
class CorrelationId {
+value: u64
}
class AssembledMessage {
+routing: EnvelopeRouting
}
class EnvelopeRouting {
+envelope_id: EnvelopeId
+correlation_id: CorrelationId
}
class BackoffConfig {
+normalized(initial: Duration, max: Duration) BackoffConfig
}
class PacketParts {
+into_payload() Bytes
}
class FragmentParts {
+into_payload() Bytes
}
class Serializer~S~ {
<<trait>>
+serialize(message: EncodeWith~S~) Result~Vec_u8~~
+deserialize(bytes: &[u8]) Result~DecodeWith~S~~
}
class EncodeWith~S~ {
<<trait>>
+encode_with(serializer: S) Result~Vec_u8~~
}
class DecodeWith~S~ {
<<trait>>
+decode_with(serializer: S, bytes: &[u8]) Result~Self~
}
class SerdeSerializerBridge {
<<trait>>
}
class SerdeMessage~T~ {
}
WireframeError --> CodecError
Result --> WireframeError
WireframeApp --> MemoryBudgets
MemoryBudgets --> BudgetBytes
AppFactory ..|> Fn
AppFactory ..> FactoryResult
WireframeServer --> AppFactory
FactoryResult <|.. WireframeApp
FactoryResult <|.. Result
AssembledMessage --> EnvelopeRouting
EnvelopeRouting --> EnvelopeId
EnvelopeRouting --> CorrelationId
AppDataStore ..> WireframeApp
Serializer --> EncodeWith
Serializer --> DecodeWith
SerdeMessage ..> SerdeSerializerBridge
Class diagram for client streaming, hooks, and tracing changesclassDiagram
class WireframeClient {
+call(request: Envelope) Result~Response~
+call_streaming(request: Envelope) Result~ResponseStream~
+receive_streaming() Result~ResponseStream~
+send_streaming(header: FrameHeader, reader: AsyncRead, config: SendStreamingConfig) Result~SendStreamingOutcome~
}
class ResponseStream {
<<AsyncIterator>>
-client: &mut WireframeClient
+try_next() Result~Option~Frame~~
}
class StreamingResponseExt {
<<trait>>
+typed_with(mapper: Mapper) TypedResponseStream
}
class TypedResponseStream~S, Mapper, P, Item~ {
<<AsyncIterator>>
-inner: S
-mapper: Mapper
+try_next() Result~Option~Item~~
}
class SendStreamingConfig {
+chunk_size: Option~usize~
+timeout: Option~Duration~
+with_chunk_size(size: usize) SendStreamingConfig
+with_timeout(timeout: Duration) SendStreamingConfig
}
class SendStreamingOutcome {
+frames_sent() usize
+bytes_sent() usize
}
class WireframeClientBuilder {
+before_send(f: FnMutableVecU8) WireframeClientBuilder
+after_receive(f: FnMutableBytesMut) WireframeClientBuilder
+tracing_config(config: TracingConfig) WireframeClientBuilder
+connect(addr: SocketAddr) Result~WireframeClient~
+connect_pool(addr: SocketAddr, config: ClientPoolConfig) Result~WireframeClientPool~
}
class TracingConfig {
+with_all_levels(level: Level) TracingConfig
+with_all_timing(enabled: bool) TracingConfig
+with_call_level(level: Level) TracingConfig
+with_send_timing(enabled: bool) TracingConfig
+with_streaming_level(level: Level) TracingConfig
}
class FnMutableVecU8 {
<<closure>>
+call(bytes: Vec_u8_ref_mut)
}
class FnMutableBytesMut {
<<closure>>
+call(bytes: BytesMut_ref_mut)
}
class Envelope {
}
class FrameHeader {
}
class Frame {
}
class AsyncRead {
<<trait>>
}
class Response {
}
class ClientPoolConfig {
}
class WireframeClientPool {
}
WireframeClient --> ResponseStream
WireframeClient --> SendStreamingConfig
WireframeClient --> SendStreamingOutcome
ResponseStream --> WireframeClient
StreamingResponseExt <|.. ResponseStream
StreamingResponseExt --> TypedResponseStream
TypedResponseStream --> ResponseStream
WireframeClientBuilder --> WireframeClient
WireframeClientBuilder --> WireframeClientPool
WireframeClientBuilder --> ClientPoolConfig
WireframeClientBuilder --> TracingConfig
WireframeClientBuilder --> FnMutableVecU8
WireframeClientBuilder --> FnMutableBytesMut
SendStreamingConfig --> SendStreamingOutcome
Class diagram for client connection pooling changesclassDiagram
class WireframeClientBuilder {
+connect_pool(addr: SocketAddr, config: ClientPoolConfig) Result~WireframeClientPool~
}
class WireframeClientPool {
+handle() PoolHandle
}
class PoolHandle {
+acquire() Result~PooledClientLease~
}
class PooledClientLease {
<<Deref_to_WireframeClient>>
-client: WireframeClient
+call(request: Envelope) Result~Response~
+send(frame: Frame) Result~()~
+receive() Result~Frame~
}
class ClientPoolConfig {
+pool_size(value: usize) ClientPoolConfig
+max_in_flight_per_socket(value: usize) ClientPoolConfig
+idle_timeout(timeout: Duration) ClientPoolConfig
+fairness_policy(policy: PoolFairnessPolicy) ClientPoolConfig
}
class PoolFairnessPolicy {
<<enum>>
+RoundRobin
+Lifo
}
class WireframeClient {
+call(request: Envelope) Result~Response~
}
class Envelope {
}
class Response {
}
class Frame {
}
WireframeClientBuilder --> ClientPoolConfig
WireframeClientBuilder --> WireframeClientPool
WireframeClientPool --> PoolHandle
PoolHandle --> PooledClientLease
PooledClientLease --> WireframeClient
ClientPoolConfig --> PoolFairnessPolicy
Class diagram for testkit and wireframe_testing additionsclassDiagram
class TestkitModule {
<<module wireframe_testkit>>
}
class WireframeApp {
}
class TestSerializer {
}
class TestResult~T~ {
<<type alias>>
}
class TestError {
}
class SlowIoConfig {
+payload_chunk_size: usize
+frame_delay: Duration
}
class SlowIoPacing {
+interval: Duration
}
class MessageAssemblySnapshot {
}
class FragmentReassemblySnapshot {
}
class TestkitHelpers {
+drive_with_partial_frames(app: WireframeApp, frames: Vec~Bytes~)
+drive_with_fragments(app: WireframeApp, fragments: Vec~Bytes~)
+drive_with_fragment_frames(app: WireframeApp, frames: Vec~Bytes~)
+drive_with_slow_frames(app: WireframeApp, config: SlowIoConfig)
+drive_with_slow_payloads(app: WireframeApp, config: SlowIoConfig)
+assert_message_assembly_completed(snapshot: MessageAssemblySnapshot)
+assert_fragment_reassembly_error(snapshot: FragmentReassemblySnapshot)
}
class WireframePair {
+client_mut() WireframeClient_ref_mut
+local_addr() SocketAddr
+shutdown() TestResult~()~
}
class WireframeClient {
}
class ObservabilityHandle {
+new() ObservabilityHandle
+clear() void
+recorder() MetricsRecorder
+snapshot() void
+codec_error_counter(kind: &str, action: &str) u64
}
class Labels {
+with_label(key: &str, value: &str) Labels
}
class MetricsRecorder {
}
class HotlineFrameCodec {
}
class HotlineFixtures {
+valid_hotline_wire(payload: Bytes, transaction_id: u32) Bytes
+valid_hotline_frame(payload: Bytes, transaction_id: u32) HotlineFrame
+oversized_hotline_wire(max_frame_length: usize) Bytes
+mismatched_total_size_wire(payload: Bytes) Bytes
+truncated_hotline_header() Bytes
+truncated_hotline_payload(payload_len: usize) Bytes
+correlated_hotline_wire(transaction_id: u32, payloads: Vec~Bytes~) Bytes
+sequential_hotline_wire(base_transaction_id: u32, payloads: Vec~Bytes~) Bytes
+new_test_codec() HotlineFrameCodec
}
class HotlineFrame {
}
class Bytes {
}
class WireframeTestingCrate {
<<crate wireframe_testing>>
}
TestkitModule --> WireframeApp
TestkitModule --> TestSerializer
TestkitModule --> TestResult
TestkitModule --> TestError
TestkitModule --> SlowIoConfig
TestkitModule --> SlowIoPacing
TestkitModule --> MessageAssemblySnapshot
TestkitModule --> FragmentReassemblySnapshot
TestkitModule --> TestkitHelpers
WireframeTestingCrate --> WireframePair
WireframeTestingCrate --> ObservabilityHandle
WireframeTestingCrate --> Labels
WireframeTestingCrate --> HotlineFixtures
WireframePair --> WireframeClient
ObservabilityHandle --> MetricsRecorder
ObservabilityHandle --> Labels
HotlineFixtures --> HotlineFrame
HotlineFixtures --> HotlineFrameCodec
HotlineFixtures --> Bytes
Class diagram for FrameCodec zero-copy payload improvementsclassDiagram
class FrameCodec {
<<trait>>
+Frame
+frame_payload(frame: Frame_ref) &[u8]
+frame_payload_bytes(frame: Frame_ref) Bytes
+wrap_payload(payload: Bytes) Frame
+max_frame_length() usize
}
class LengthDelimitedFrameCodec {
+Frame = Bytes
+frame_payload(frame: Bytes_ref) &[u8]
+frame_payload_bytes(frame: Bytes_ref) Bytes
+wrap_payload(payload: Bytes) Bytes
+max_frame_length() usize
}
class MyCodec {
+limit: usize
+Frame = Bytes
+frame_payload(frame: Bytes_ref) &[u8]
+frame_payload_bytes(frame: Bytes_ref) Bytes
+wrap_payload(payload: Bytes) Bytes
+max_frame_length() usize
}
class Bytes {
}
FrameCodec <|.. LengthDelimitedFrameCodec
FrameCodec <|.. MyCodec
LengthDelimitedFrameCodec --> Bytes
MyCodec --> Bytes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2274d86070
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 444-453: Update the "Streaming responses" and the
outbound-streaming section to describe the tuple-based streaming API: explain
that multi-packet streams are represented as (header, payload) tuples and show
how the client consumes them via WireframeClient.call_streaming(envelope)
returning a ResponseStream that yields tuples until a terminator, and how
receive_streaming() is used for pushed streams; also document the helper
constructors (channel-pair preparers) that create the send/receive channel pair
used to initiate outbound streams, and align wording in both response-streaming
and outbound-streaming sections to reference the tuple format, the
ResponseStream behavior, and the channel-pair helper constructors by name so
readers know how to prepare and consume the tuple-based streams.
- Around line 64-67: Update the migration guide sentence to clarify that the
crate root re-exports only wireframe::Result<T> and
wireframe::WireframeError<E>, and that other types were moved into their owning
modules but those module namespaces remain publicly exported (i.e., you must
update import paths to reference the module directly rather than implying those
types are removed from the public API). Mention the distinction between
"re-exported at root" and "public modules still available" so readers know to
import from the module namespaces rather than expecting full re-exports at crate
root.
- Around line 454-455: Update the documentation text so it refers to the correct
streaming trait: replace the phrase "ResponseStream is an
AsyncIterator-compatible type" with wording that "ResponseStream implements
futures::Stream to yield typed frames" (it should mention ResponseStream,
futures::Stream, and the existing Stream usage like .try_next() / StreamExt to
make the intent clear). Ensure the sentence emphasizes the exclusive borrow of
the client for the duration of the stream and uses "Stream" terminology
consistently.
🪄 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: f19d45b9-b686-4b70-b577-aa6bb819ad30
📒 Files selected for processing (1)
docs/v0-2-0-to-v0-3-0-migration-guide.md
Corrections based on verification against current source: - **Root re-exports section**: Clarify that module namespaces remain fully public (`pub mod` at crate root). The previous wording implied types were removed from the public API; only the root shorthand re- exports are gone. Readers importing from `wireframe::serializer::*` and similar module paths are unaffected. - **Streaming responses section**: Replace "AsyncIterator-compatible" with the correct trait (`ResponseStream` implements `futures::Stream` with `Item = Result<P, ClientError>`). Document that `receive_streaming` takes a `correlation_id: u64` parameter and is the lower-level variant for callers who have already sent the request manually. Update the code example to use `StreamExt::next` with explicit `result?` unwrap, matching the actual rustdoc pattern and removing the untyped `WireframeClient` placeholder. Finding 1 from the inline comments (tuple-based streaming / channel-pair helpers) was not applied: no such API exists in the codebase. `ResponseStream` yields typed packets, not tuples.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
`normalized` is a consuming instance method (`fn normalized(self) -> Self`), not a constructor. The previous snippet `BackoffConfig::normalized(initial, max)` would not compile. Replace with the correct pattern: construct a `BackoffConfig` struct literal and call `.normalized()` on it, matching the usage shown in the method's own rustdoc.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
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 `@docs/v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 446-500: The migration guide currently documents ResponseStream,
call_streaming, receive_streaming and StreamingResponseExt but conflicts with
the coding guideline that mandates a "tuple-based API" and channel-pair helpers;
resolve the conflict by either (A) removing or narrowing the tuple-based API
requirement from the coding guideline text so it no longer mandates an API that
doesn't exist, or (B) implementing and documenting the tuple-based API
(including helper constructors that prepare the channel pair and updated docs
for ResponseStream/call_streaming/receive_streaming/TypedResponseStream), or (C)
explicitly clarifying in the guideline that the tuple-based rule only applies to
specified design docs and not to migration guides; update the guideline or
migration-guide text accordingly so the wording and examples for ResponseStream,
call_streaming, receive_streaming, StreamingResponseExt, and TypedResponseStream
are consistent with the chosen approach.
🪄 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: 7779480c-8978-4446-a824-0a9ee10cd872
📒 Files selected for processing (1)
docs/v0-2-0-to-v0-3-0-migration-guide.md
The server factory section showed `run_with_shutdown(|| build_app(), signal)`, implying the factory is passed to `run_with_shutdown`. The actual API is: - `WireframeServer::new(factory)` – factory supplied at construction - `.bind(addr).await?.run_with_shutdown(shutdown)` – shutdown future only Rewrite both examples to use the correct call chain. Drop the misleading before/after pair that showed identical broken code; replace with an infallible and a fallible example, each using the right constructor.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Two navigation improvements to the v0.2.0 → v0.3.0 migration guide: - Add a `## Contents` section after the intro paragraph, split into "Breaking changes" and "New capabilities" groups, linking to every major section by anchor. - Replace bare prose references to the v0.1.0 → v0.2.0 guide with explicit relative Markdown links (`v0-1-0-to-v0-2-0-migration-guide.md`) so readers unfamiliar with the docs layout can navigate directly.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Add a Mermaid class diagram to the connection pooling section showing the `WireframeClientBuilder` → `WireframeClientPool` → `PoolHandle` → `PooledClientLease` → `WireframeClient` chain, with `ClientPoolConfig` and `PoolFairnessPolicy` as configuration nodes. Includes an accessible italic caption describing the diagram structure. - Convert the `**Breaking changes**` and `**New capabilities**` labels in the Contents section from bold emphasis to `###` subheadings, resolving the MD036/no-emphasis-as-heading markdownlint errors.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/v0-2-0-to-v0-3-0-migration-guide.md (1)
459-536:⚠️ Potential issue | 🟠 MajorDocument the tuple-based streaming model and channel-pair helper constructors.
Update the streaming sections to explicitly describe the tuple item shape for
multi-packet streams and name the helper constructors that prepare the
send/receive channel pair before outbound streaming.Triage:
[type:docstyle]As per coding guidelines, "Documentation describing multi-packet streaming MUST reflect the tuple-based API and explain helper constructors that prepare the channel pair".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/v0-2-0-to-v0-3-0-migration-guide.md` around lines 459 - 536, Update the Streaming sections to explicitly describe the tuple-based item shape used for multi-packet streams (mention the exact tuple fields used by the API) and add documentation for the helper constructors that prepare the send/receive channel pair used by outbound streaming; specifically mention WireframeClient::call_streaming, WireframeClient::receive_streaming, ResponseStream, StreamingResponseExt::typed_with, WireframeClient::send_streaming and SendStreamingConfig, and include short examples showing the tuple item structure and the exact helper constructor names and calls that produce the channel pair before calling send_streaming.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 459-536: Update the Streaming sections to explicitly describe the
tuple-based item shape used for multi-packet streams (mention the exact tuple
fields used by the API) and add documentation for the helper constructors that
prepare the send/receive channel pair used by outbound streaming; specifically
mention WireframeClient::call_streaming, WireframeClient::receive_streaming,
ResponseStream, StreamingResponseExt::typed_with,
WireframeClient::send_streaming and SendStreamingConfig, and include short
examples showing the tuple item structure and the exact helper constructor names
and calls that produce the channel pair before calling send_streaming.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e2c0b3ba-7a08-4053-8d0e-32f1b7c068f5
📒 Files selected for processing (1)
docs/v0-2-0-to-v0-3-0-migration-guide.md
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Insert a Mermaid class diagram at the top of the "Client: new capabilities" section showing the complete client API surface: `WireframeClientBuilder` construction paths, `WireframeClient` streaming methods, `ResponseStream` and `TypedResponseStream` iteration, the `StreamingResponseExt` trait relationship, `SendStreamingConfig` / `SendStreamingOutcome` for outbound streaming, `TracingConfig`, and the `before_send` / `after_receive` hook closure types. An accessible italic caption summarises the diagram in prose for readers who cannot render Mermaid.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Insert a Mermaid class diagram between the Contents section and the first breaking-change section, providing an at-a-glance overview of the server and protocol types affected in v0.3.0: the unified `WireframeError<E>` and `Result<T>` alias, `WireframeApp` with its memory budget builder methods, the `AppFactory` / `FactoryResult` server factory model, `AssembledMessage` with typed `EnvelopeId` and `CorrelationId` routing, the updated `Serializer<S>` trait bounds (`EncodeWith<S>`, `DecodeWith<S>`), `SerdeMessage<T>` / `SerdeSerializerBridge`, and the renamed payload accessors on `PacketParts` and `FragmentParts`. Includes an accessible italic caption summarising the diagram in prose.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Insert a Mermaid sequence diagram in the "Streaming responses" section after the call_streaming code example. The diagram shows the full ResponseStream lifecycle: the initial call_streaming request/response exchange, the try_next() poll loop consuming server frames until the terminator arrives, and the exclusive borrow release on drop. Includes an accessible italic caption summarising the flow in prose.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Have the following now been resolved? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 31803
Both comments are resolved. ✅ |
Insert a Mermaid sequence diagram in the "Connection pooling" section after the existing class diagram, showing the runtime flow: connect_pool builds the pool, handle() produces a PoolHandle, acquire() checks out a PooledClientLease (blocking at the in-flight limit if needed), the lease delegates the call through WireframeClient to the server, and dropping the lease returns the socket to the pool. Includes an accessible italic caption summarising the flow in prose.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Insert a Mermaid class diagram at the top of the "Testkit module" section, covering both testing layers at a glance: the wireframe::testkit module (drive helpers, slow-IO simulation, assembly assertions, TestSerializer, TestResult/TestError) and the wireframe_testing companion crate (WireframePair, ObservabilityHandle, Labels, HotlineFixtures with codec regression helpers and new_test_codec). Placing the diagram before both sections gives readers a visual map of the full testing infrastructure before they read the per-section prose. Includes an accessible italic caption describing both layers.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
ADR 0001's consequence stated "Documentation describing multi-packet streaming MUST reflect the tuple-based API" without scoping the rule to server-side handler documentation. This created an apparent conflict with the migration guide, which documents the client-side consumption API (ResponseStream, call_streaming, receive_streaming, StreamingResponseExt, TypedResponseStream) — a distinct concern that the ADR decision never addressed. Resolution (C): scope the ADR consequence explicitly to server-side handler-authoring documentation, and list the client-side types that fall outside it. Add a scope note to the migration guide's "Streaming responses" section that links back to ADR 0001 and draws the boundary between the two APIs clearly for readers arriving from either direction.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The Mermaid class diagram showed `receive_streaming()` with no
parameters. The actual API and the prose both require a `correlation_id`
argument. Update the method signature to match:
+receive_streaming(correlation_id: CorrelationId) Result~ResponseStream~
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/v0-2-0-to-v0-3-0-migration-guide.md (1)
757-768:⚠️ Potential issue | 🟠 MajorReplace
AsyncIteratorlabels withStreamterminology.Edit the class diagram to match the documented and implemented Rust API surface.
Use<<Stream>>forResponseStreamandTypedResponseStream.Triage:
[type:docstyle]#!/bin/bash set -euo pipefail echo "Check Rust implementations:" rg -n -C2 --type=rust '\bimpl\b.*\bStream\b.*\bResponseStream\b|\bstruct\s+ResponseStream\b|\bTypedResponseStream\b' src echo echo "Check migration guide terminology in diagrams:" rg -n -C2 'AsyncIterator|futures::Stream|ResponseStream|TypedResponseStream' docs/v0-2-0-to-v0-3-0-migration-guide.mdAs per coding guidelines, "Verify
docs/users-guide.mdand migration notes describe the same meaning as code symbols".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/v0-2-0-to-v0-3-0-migration-guide.md` around lines 757 - 768, The class diagram uses <<AsyncIterator>> labels but the Rust API exposes Stream types; update the diagram to use <<Stream>> instead of <<AsyncIterator>> for both ResponseStream and TypedResponseStream and ensure related method/type labels match the Rust symbols (e.g., ResponseStream, TypedResponseStream, StreamingResponseExt::typed_with, and the try_next() signature) so the diagram terminology aligns with the implemented Rust types and methods.
🤖 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/v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 29-30: Several Mermaid diagrams (e.g., the ```mermaid``` block
starting with `classDiagram`) are missing a one- or two-sentence screen-reader
description immediately before the diagram; add a short plain-text description
line (one or two sentences) directly above each ```mermaid``` block (for example
above the `classDiagram` blocks referenced) while keeping existing captions
after the diagram unchanged so screen readers get context before the diagram is
rendered.
- Around line 752-753: Update the class diagram entry for the receive_streaming
method so its signature matches the API: change receive_streaming() to
receive_streaming(correlation_id: u64) Result~ResponseStream~; ensure the
correlation_id parameter is present next to the existing send_streaming(header:
FrameHeader, reader: AsyncRead, config: SendStreamingConfig) entry and that the
diagram formatting remains consistent with other method signatures.
---
Duplicate comments:
In `@docs/v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 757-768: The class diagram uses <<AsyncIterator>> labels but the
Rust API exposes Stream types; update the diagram to use <<Stream>> instead of
<<AsyncIterator>> for both ResponseStream and TypedResponseStream and ensure
related method/type labels match the Rust symbols (e.g., ResponseStream,
TypedResponseStream, StreamingResponseExt::typed_with, and the try_next()
signature) so the diagram terminology aligns with the implemented Rust types and
methods.
🪄 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: fe6e6d03-e753-4215-86ad-44f0cd5b5fc5
📒 Files selected for processing (2)
docs/adr-001-multi-packet-streaming-response-api.mddocs/v0-2-0-to-v0-3-0-migration-guide.md
All six Mermaid diagrams in the v0.2.0 → v0.3.0 migration guide lacked a plain-text description immediately before the opening fence. Screen readers encounter the diagram source before any post-diagram caption, so they had no context. Add a one- to two-sentence description directly above each ```mermaid block: - Server/protocol type overview (class diagram before Cargo features) - Testing infrastructure overview (class diagram at Testkit module) - Client API type overview (class diagram at Client: new capabilities) - call_streaming lifecycle (sequence diagram in Streaming responses) - Connection pool type hierarchy (class diagram in Connection pooling) - Connection pool usage flow (sequence diagram in Connection pooling) Existing post-diagram captions are unchanged.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 102-104: Update the diagram signature for
BackoffConfig::normalized in the migration guide so it matches the actual
no-argument builder method: change the signature shown for class BackoffConfig
from "normalized(initial: Duration, max: Duration) BackoffConfig" to
"normalized() BackoffConfig" (this ensures consistency with the example usage
`.normalized()` and the implementation in BackoffConfig/normalized).
- Around line 768-781: The class diagram uses the wrong async trait
label—replace occurrences of AsyncIterator with Stream and annotate them as
futures::Stream semantics: update the ResponseStream class (where AsyncIterator
appears), the StreamingResponseExt trait entry, and the TypedResponseStream
generic block to use futures::Stream instead of AsyncIterator so the labels at
lines near ResponseStream, StreamingResponseExt, and TypedResponseStream match
the semantics used at lines 893–896.
- Around line 77-80: Update the class diagram entry for WireframeServer: remove
the `factory: AppFactory` parameter from the `run_with_shutdown` method
signature so it reads `run_with_shutdown(signal: ShutdownSignal) -> Result<()>`;
the `factory` parameter only belongs to `WireframeServer::new(...)` (as used in
the examples) so ensure `WireframeServer::new` retains the factory argument
while `run_with_shutdown` does not.
🪄 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: d9937af8-e0f1-4661-80c3-eb707fc230f8
📒 Files selected for processing (1)
docs/v0-2-0-to-v0-3-0-migration-guide.md
All three findings verified against source before applying: - `BackoffConfig::normalized`: actual signature is `fn normalized(mut self) -> Self` (no arguments). Remove the spurious `initial` and `max` parameters from the class diagram entry. - `ResponseStream` and `TypedResponseStream`: both implement `futures::Stream` (confirmed at response_stream.rs:160 and streaming_helpers.rs:175). Replace `<<AsyncIterator>>` with `<<futures::Stream>>` on both classes. - `WireframeServer`: `run_with_shutdown` takes only a shutdown future (`src/server/runtime.rs:142`); the factory is supplied to `WireframeServer::new(factory)` (`src/server/config/mod.rs:76`). Add `new(factory: AppFactory)` to the class and remove the `factory` parameter from `run_with_shutdown`.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
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 `@docs/v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 495-496: The diagram label uses the non-canonical module
identifier "<<module wireframe_testkit>>"; update it to the canonical symbol
"<<module wireframe::testkit>>" so the Testkit diagram matches the
public/documented module name—locate the diagram label containing "<<module
wireframe_testkit>>" and replace it with "<<module wireframe::testkit>>".
🪄 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: 2bf185ba-dfc4-491d-82c0-dd004fc234db
📒 Files selected for processing (1)
docs/v0-2-0-to-v0-3-0-migration-guide.md
The class diagram used `<<module wireframe_testkit>>` (underscore) while the actual public module path is `wireframe::testkit` (confirmed at src/lib.rs:49 and used consistently throughout the guide prose). Update the label to `<<module wireframe::testkit>>`.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
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 `@docs/v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 77-81: The class diagram for WireframeServer is incorrect: update
the WireframeServer::bind signature to reflect that bind is awaitable and
fallible by changing its return path to a Result (e.g., Result<WireframeServer,
Error>) instead of returning WireframeServer directly; locate the diagram block
containing "class WireframeServer" and replace the bind line so it shows a
Result-returning/awaited bind (keeping new(factory: AppFactory) and
run_with_shutdown(signal: ShutdownSignal) as-is).
🪄 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: fe3345a8-3224-4e57-b496-152825a4801e
📒 Files selected for processing (1)
docs/v0-2-0-to-v0-3-0-migration-guide.md
`bind(self, addr: SocketAddr) -> Result<BoundServer, ServerError>` (src/server/config/binding.rs:146) is fallible and must show a Result return type in the diagram. Update from `WireframeServer` to `Result~WireframeServer~`. The method is synchronous (not async), so no `.await` annotation is needed. `new` and `run_with_shutdown` are correct.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
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 `@docs/v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 77-81: Update the diagram and examples so WireframeServer::bind
accurately shows it returns Result<BoundServer<...>, ServerError> (use
BoundServer in the class diagram instead of WireframeServer) and remove the
erroneous .await? calls in the factory examples that call WireframeServer::bind
(since bind is synchronous and returns Result directly); ensure the examples
instead use ? to propagate the Result before calling
.run_with_shutdown(...).await? on the run_with_shutdown future.
🪄 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: da406081-a639-41c4-a1d3-9079f469dcdd
📒 Files selected for processing (1)
docs/v0-2-0-to-v0-3-0-migration-guide.md
`bind` is synchronous and returns `Result<BoundServer, ServerError>` (src/server/config/binding.rs:149), where `BoundServer` is a type alias for `WireframeServer` with a `Bound` state marker. Two corrections: - Class diagram: change return type from `Result~WireframeServer~` to `Result~BoundServer~` to match the actual declared return type. - Factory code examples: replace `.bind(addr).await?` (invalid – bind is not a future) with `.bind(addr)?` so the `Result` is propagated synchronously before `.run_with_shutdown(...).await?` is called on the resulting `BoundServer`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Documents all breaking changes and new capabilities introduced between
the
v0.2.0tag and HEAD, based on a semantic diff of the Rust source.Breaking changes covered:
test-helpersfeature removed;test-supportis the sole aliaspool,testkit, andserializer-serdefeatures addedwireframe::app::error::WireframeErrorandwireframe::response::WireframeErrorremoved; unifiedwireframe::WireframeError<E>at crate rootwireframe::preluderequired
PacketParts::payload/FragmentParts::payload→into_payloadBackoffConfig::normalised→normalizedSerializertrait bounds updated toEncodeWith<S>/DecodeWith<S>WireframeApp::new()now returnsResult<Self>AppFactory/FactoryResulttraitsAppDataStoremoved towireframe::app_data_storeEnvelopeIdandCorrelationIdnewtypes added towireframe::message_assemblerNew capabilities covered:
MemoryBudgets/BudgetByteswithmemory_budgets()andenable_fragmentation()builder methodswireframe::preludeconvenience import modulewireframe::testkittesting utilities moduleClient capabilities (new domain coverage, separate section):
call_streaming,ResponseStream,StreamingResponseExt, andTypedResponseStreamsend_streamingandSendStreamingConfigWireframeClientPool,ClientPoolConfig,PoolHandle,PooledClientLease, andPoolFairnessPolicybefore_sendandafter_receiveTracingConfigTwo sections were missing from the initial draft:
Codec improvements: documents
FrameCodec::frame_payload_bytes,the new default method for zero-copy payload extraction. No existing
codec implementation requires change; the method is additive with a
copy-based default. Includes an override example for codecs that store
the frame payload as
Bytesinternally.wireframe_testing additions: documents three areas of new test
infrastructure in the companion crate:
WireframePair/spawn_wireframe_pair– in-process server+clientpair harness over real loopback TCP
ObservabilityHandle/obs_handle/Labels– combined log andmetrics capture fixture for codec error and connection metric
assertions
drive_with_codec_frames,decode_frames_with_codec, etc.) and Hotline protocol fixtures(
valid_hotline_wire,oversized_hotline_wire,correlated_hotline_ wire, and related functions) for codec regression testsSummary by Sourcery
Add a comprehensive migration guide documenting breaking changes and new capabilities between wireframe v0.2.0 and v0.3.0, including client, server, codec, and testing updates.
Documentation: