Introduce PacketParts and optional correlation IDs#282
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughConvert correlation IDs from u64 to Option, introduce a public PacketParts type, and update the Packet trait, Envelope, middleware, examples, docs and tests to use PacketParts and optional correlation IDs (None denotes unsolicited/server-initiated frames). Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware
participant Router
participant Handler
Client->>Middleware: Send Envelope(id, correlation_id: Option<u64>, payload)
Middleware->>Router: Build ServiceRequest(frame, correlation_id)
Router->>Handler: Route payload using PacketParts (id, correlation_id, payload)
Handler-->>Router: Return ServiceResponse(payload, correlation_id)
Router->>Middleware: Build Envelope from PacketParts (inherit correlation)
Middleware->>Client: Send Envelope(id, correlation_id, payload)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Reviewer's GuideRefactors correlation ID handling to be optional and generic by introducing PacketParts, flattens Envelope, propagates IDs through middleware and handlers, enhances logging context, and updates related tests and examples for the new API. Class diagram for Packet, PacketParts, and Envelope changesclassDiagram
class Packet {
+id() u32
+correlation_id() Option<u64>
+into_parts() PacketParts
+from_parts(parts: PacketParts) Self
}
class PacketParts {
+id: u32
+correlation_id: Option<u64>
+msg: Vec<u8>
}
class Envelope {
+id: u32
+correlation_id: Option<u64>
+msg: Vec<u8>
+new(id: u32, correlation_id: Option<u64>, msg: Vec<u8>)
+into_parts() PacketParts
}
Packet <|.. Envelope
PacketParts <.. Envelope : uses
PacketParts <.. Packet : uses
Class diagram for ServiceRequest and ServiceResponse changesclassDiagram
class ServiceRequest {
-inner: FrameContainer<Vec<u8>>
-correlation_id: Option<u64>
+new(frame: Vec<u8>, correlation_id: Option<u64>)
+frame() &[u8]
+correlation_id() Option<u64>
+frame_mut() &mut Vec<u8>
+into_inner() Vec<u8>
}
class ServiceResponse {
-inner: FrameContainer<Vec<u8>>
-correlation_id: Option<u64>
+new(frame: Vec<u8>, correlation_id: Option<u64>)
+frame() &[u8]
+frame_mut() &mut Vec<u8>
+correlation_id() Option<u64>
+into_inner() Vec<u8>
}
class FrameContainer~T~ {
+new(frame: T)
+frame() &T
+frame_mut() &mut T
+into_inner() T
}
ServiceRequest o-- FrameContainer
ServiceResponse o-- FrameContainer
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![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.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
src/middleware.rs (1)
47-52: Expose a way to set/overwrite correlation IDs in middlewareEnable middleware to attach or scrub correlation IDs by adding a setter on
ServiceRequest.Apply inside this impl:
impl ServiceRequest { // … /// Set or clear the correlation identifier. pub fn set_correlation_id(&mut self, correlation_id: Option<u64>) { self.correlation_id = correlation_id; } }
♻️ Duplicate comments (7)
src/app.rs (5)
189-197: Add unit tests for Packet::from_parts and into_partsAdd tests that round-trip via PacketParts for:
- correlation_id = None
- correlation_id = Some(value)
- empty payload vs non-empty payload
Prefer rstest with parameterised cases per guidelines.
I can draft rstest-based cases if helpful.
199-205: Introduce From/Into conversions between Envelope and PacketParts to remove boilerplateDefine canonical From conversions and switch Packet impl to them. This centralises mapping logic as previously suggested.
Apply this change:
+impl From<Envelope> for PacketParts { + fn from(e: Envelope) -> Self { + PacketParts { id: e.id, correlation_id: e.correlation_id, msg: e.msg } + } +} + +impl From<PacketParts> for Envelope { + fn from(p: PacketParts) -> Self { + Envelope::new(p.id, p.correlation_id, p.msg) + } +}Then simplify the Packet impl (see comment at lines 241-249).
229-237: Prefer centralised conversion in into_partsRoute Envelope::into_parts via the new From impl to avoid duplication and keep a single source of truth.
Apply:
- pub fn into_parts(self) -> PacketParts { - PacketParts { - id: self.id, - correlation_id: self.correlation_id, - msg: self.msg, - } - } + pub fn into_parts(self) -> PacketParts { self.into() }
241-249: Simplify Packet impl using the From/Into conversionsStop re-stating field mappings; delegate to conversions.
Apply:
- fn into_parts(self) -> PacketParts { Envelope::into_parts(self) } + fn into_parts(self) -> PacketParts { self.into() } - fn from_parts(parts: PacketParts) -> Self { - Envelope::new(parts.id, parts.correlation_id, parts.msg) - } + fn from_parts(parts: PacketParts) -> Self { parts.into() }
746-752: Log correlation_id mismatches explicitlyDetect and log mismatches between request and response correlation IDs. Deepwiki research confirms that optional correlation IDs propagate through PushHandle and WireframeProtocol; any divergence should surface in logs. Update the code in src/app.rs around line 750 as follows:
@@ src/app.rs:748-752 - let correlation_id = resp.correlation_id().or(env.correlation_id); + let req_cid = env.correlation_id; + let res_cid = resp.correlation_id(); + if let (Some(rq), Some(rs)) = (req_cid, res_cid) { + if rq != rs { + tracing::warn!( + id = env.id, + req_correlation_id = rq, + res_correlation_id = rs, + "correlation_id mismatch between request and response" + ); + } + } + let correlation_id = res_cid.or(req_cid);Confirmed no other
.or(env.correlation_id)usages without mismatch checks.tests/middleware_order.rs (1)
91-93: Assert correlation_id (and id) in response to validate propagationAdd assertions to verify the middleware preserves the correlation ID and message ID alongside the frame mutation.
- let parts = resp.into_parts(); - assert_eq!(parts.msg, vec![b'X', b'A', b'B', b'B', b'A']); + let parts = resp.into_parts(); + assert_eq!(parts.correlation_id, Some(7)); + assert_eq!(parts.id, 1); + assert_eq!(parts.msg, vec![b'X', b'A', b'B', b'B', b'A']);src/middleware.rs (1)
314-323: Reduce boilerplate with PacketParts::new constructorReplace the struct literal with a
PacketParts::new(...)constructor to slim call-sites. This repeats prior feedback.Use after adding the constructor:
- let parts = PacketParts { - id: self.id, - correlation_id: req.correlation_id(), - msg: req.into_inner(), - }; - let env = E::from_parts(parts); + let env = E::from_parts(PacketParts::new( + self.id, + req.correlation_id(), + req.into_inner(), + ));Add in
impl PacketParts(in the app module):impl PacketParts { #[must_use] pub fn new(id: u32, correlation_id: Option<u64>, msg: Vec<u8>) -> Self { Self { id, correlation_id, msg } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
README.md(1 hunks)examples/metadata_routing.rs(1 hunks)examples/ping_pong.rs(1 hunks)src/app.rs(4 hunks)src/middleware.rs(6 hunks)tests/correlation_id.rs(1 hunks)tests/lifecycle.rs(3 hunks)tests/metadata.rs(2 hunks)tests/middleware.rs(2 hunks)tests/middleware_order.rs(2 hunks)tests/routes.rs(6 hunks)tests/world.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/correlation_id.rstests/middleware_order.rstests/metadata.rstests/world.rstests/middleware.rsexamples/metadata_routing.rsexamples/ping_pong.rstests/lifecycle.rssrc/middleware.rstests/routes.rssrc/app.rs
**/*.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Paragraphs and bullets must be wrapped to 80 columns, except where a long URL would prevent this (in which case, silence MD013 for that line)
- Code blocks should be wrapped to 120 columns.
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
README.md
🧬 Code Graph Analysis (4)
tests/middleware_order.rs (1)
src/app.rs (2)
new(221-227)new(330-330)
tests/metadata.rs (1)
src/app.rs (2)
new(221-227)new(330-330)
examples/metadata_routing.rs (1)
src/app.rs (4)
new(221-227)new(330-330)id(187-187)id(241-241)
tests/lifecycle.rs (2)
src/app.rs (9)
id(187-187)id(241-241)correlation_id(190-190)correlation_id(243-243)into_parts(193-193)into_parts(231-237)into_parts(245-245)from_parts(196-196)from_parts(247-249)tests/routes.rs (4)
id(29-29)correlation_id(31-31)into_parts(33-39)from_parts(41-47)
🔍 MCP Research (1 server)
Deepwiki:
-
The asynchronous messaging system integrates with the updated packet and envelope types, propagating optional correlation IDs through
PushHandleandSessionRegistry. TheWireframeProtocoltrait hooks receive and manipulate frames with optional correlation IDs. (Protocol Customization) -
The
WireframeProtocoltrait andProtocolHooksstructure support the updated envelope and packet types with optional correlation IDs. Hooks such asbefore_sendandon_connection_setupoperate on frames that include optional correlation IDs. (Protocol Customization) -
The roadmap outlines the introduction of optional correlation IDs and the
PacketPartsstruct as part of the transition to multi-packet streaming responses and advanced protocol features. This change is foundational for supporting streaming responses with correlation and fragmentation. (Future Roadmap)
🔇 Additional comments (20)
src/app.rs (5)
166-175: Example Packet impl aligns with optional correlation IDsKeep this example minimal yet correct. The None correlation is appropriate for a custom envelope with no correlation.
176-182: from_parts example reconstructs correctlyThe example reconstructs all fields; fine for illustrating round-tripping with PacketParts.
213-215: Envelope fields updated to Option are correctField visibility and types align with the new contract.
221-227: Envelope::new signature matches the new semanticsConstructor now cleanly accepts Option.
189-197: Confirm Option migration complete
- No calls to
Envelope::newpass a raw integer; the only invocation atsrc/app.rs:751uses thecorrelation_idvariable of typeOption<u64>.- All
impl Packet for …definitions—including tests andEnvelopeinsrc/app.rs—implementfn correlation_id(&self) -> Option<u64>.tests/metadata.rs (1)
63-63: Ensure no raw integer correlation IDs in tests
- Ran
rg -n 'Envelope::new\([^,]+,\s*\d+\s*,\s*' tests– no matches found.- All tests now wrap correlation IDs in
Some(...).tests/correlation_id.rs (1)
15-17: Adopt Option correlation ids in streaming testYielding Some(cid) and asserting Some(cid) across frames validates propagation for streams. This complies with the optional correlation ID roadmap.
Also applies to: 23-23
tests/world.rs (1)
138-140: Propagate Option-based correlation ids in world helper and verificationThe updates correctly carry and assert Some(self.cid) in all frames.
Also applies to: 153-157
tests/middleware_order.rs (1)
67-67: Adopt Option correlation ID in Envelope construction — LGTMCreation with
Some(7)aligns with the updated API and optional correlation ID design.examples/ping_pong.rs (1)
60-60: Carry correlation ID on all error paths — LGTMError responses now include the request correlation ID (
Option<u64>), matching the updated API and ensuring end-to-end correlation.Also applies to: 65-69, 76-76, 82-86
tests/middleware.rs (1)
15-17: Echo service preserves correlation ID — LGTMExtracting
cidand passing it toServiceResponse::newcorrectly preserves correlation IDs through the middleware chain.README.md (1)
98-111: Update to PacketParts and optional correlation ID — LGTMThe example correctly reflects
PacketPartsandOption<u64>for correlation IDs, matching the refactoredPackettrait.tests/lifecycle.rs (1)
16-16: Migrate StateEnvelope to Option and PacketParts — LGTMThe trait implementation matches the updated
Packetinterface, and the envelope instance usesSome(0)as expected.Also applies to: 105-121, 122-129, 143-143
tests/routes.rs (2)
14-14: Align imports with new Packet/PacketParts APIsImport
PacketandPacketPartsfromappto match the refactored trait surface. LGTM.
91-91: Assert correlation ID preservationAssert against
Some(99)after round-trip to prove correlation ID is preserved. LGTM.src/middleware.rs (5)
39-42: Store optional correlation IDs in requestsMove to
Option<u64>at the type level. This matches the new contract and avoids sentinel values. LGTM.
58-61: Return Option from request accessorReturn
Option<u64>to reflect optionality. LGTM.
73-76: Store optional correlation IDs in responsesTrack correlation IDs on responses as
Option<u64>. LGTM.
81-86: Construct responses with optional correlation IDsPass through the optional correlation ID explicitly. LGTM.
11-12: Import PacketParts for struct-based envelope flow
Verified that middleware docs already reference optional correlation IDs and PacketParts (README.md 98, 106–109). LGTM.
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
src/middleware.rs (1)
41-61: Expose a setter on ServiceRequest to allow middleware to attach/update correlation IDs.Enable middleware to derive or amend correlation IDs (e.g., from headers/payload) before envelope construction.
impl ServiceRequest { @@ pub fn correlation_id(&self) -> Option<u64> { self.correlation_id } + + /// Set or clear the correlation identifier. + pub fn set_correlation_id(&mut self, correlation_id: Option<u64>) { + self.correlation_id = correlation_id; + }
♻️ Duplicate comments (9)
examples/metadata_routing.rs (4)
65-65: Replace unwrap with expect to follow linting guidelinesUse expect with a clear message.
- let app = WireframeApp::new() - .unwrap() + let app = WireframeApp::new() + .expect("failed to create app")
92-92: Replace unwrap with expect to follow linting guidelinesProvide actionable failure context.
- let payload = Ping.to_bytes().unwrap(); + let payload = Ping.to_bytes().expect("failed to serialise Ping");
101-101: Replace unwrap with expect to follow linting guidelinesProvide actionable failure context.
- .encode(&frame, &mut bytes) - .unwrap(); + .encode(&frame, &mut bytes) + .expect("failed to encode frame");
106-106: Replace unwrap with expect to follow linting guidelinesProvide actionable failure context.
- server_task.await.unwrap(); + server_task.await.expect("server task join failed");tests/middleware.rs (2)
58-61: Assert correlation_id in the first test to tighten guaranteesAdd an assertion to ensure the response preserves Some(0).
let response = wrapped.call(request).await.expect("middleware call failed"); assert_eq!(response.frame(), &[1, 2, 3, b'!', b'?']); + assert_eq!(response.correlation_id(), Some(0));
63-86: Parameterise duplicated tests with rstestReplace the duplicated None/Some cases with a single parameterised test.
use rstest::rstest; #[rstest(correlation_id => [None, Some(0), Some(42)])] #[tokio::test] async fn middleware_modifies_request_and_response_preserves_cid( correlation_id: Option<u64>, ) { let service = EchoService; let mw = ModifyMiddleware; let wrapped = mw.transform(service).await; let request = ServiceRequest::new(vec![1, 2, 3], correlation_id); let response = wrapped.call(request).await.expect("middleware call failed"); assert_eq!(response.frame(), &[1, 2, 3, b'!', b'?']); assert_eq!(response.correlation_id(), correlation_id); }README.md (1)
98-111: Document None correlation ID semantics alongside PacketPartsExplain that correlation_id: None denotes unsolicited or server‑initiated messages and cross‑link to the design doc. Add a short note under the Custom Envelopes example.
Example addition:
> Note: correlation_id: None denotes an unsolicited event or server‑initiated > push. Handlers and middleware must preserve None as None rather than > normalising to 0.tests/lifecycle.rs (1)
128-156: Assert correlation_id propagation end‑to‑endDecode the outgoing frame and assert the correlation ID is preserved as Some(0).
let out = run_app(app, vec![frame.to_vec()], None) .await .expect("app run failed"); assert!(!out.is_empty()); + // Decode the first frame and verify correlation_id propagation + let mut buf = BytesMut::from(&out[0][..]); + let frame = LengthPrefixedProcessor::default() + .decode(&mut buf) + .expect("decode failed") + .expect("frame missing"); + let (resp, _) = BincodeSerializer + .deserialize::<StateEnvelope>(&frame) + .expect("deserialize failed"); + assert_eq!(resp.correlation_id, Some(0)); assert_eq!(setup.load(Ordering::SeqCst), 1); assert_eq!(teardown.load(Ordering::SeqCst), 1);src/middleware.rs (1)
81-86: Add a setter on ServiceResponse for correlation updates in middleware.Make correlation updates explicit on the response path as well.
impl ServiceResponse { @@ pub fn correlation_id(&self) -> Option<u64> { self.correlation_id } + + /// Set or clear the correlation identifier. + pub fn set_correlation_id(&mut self, correlation_id: Option<u64>) { + self.correlation_id = correlation_id; + }Also applies to: 96-99
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
README.md(1 hunks)docs/rust-binary-router-library-design.md(1 hunks)examples/metadata_routing.rs(1 hunks)src/app.rs(4 hunks)src/middleware.rs(6 hunks)tests/lifecycle.rs(3 hunks)tests/middleware.rs(2 hunks)tests/middleware_order.rs(3 hunks)tests/routes.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Function documentation must include clear examples demonstrating the usage and outcome of the function. Test documentation should omit examples where the example serves only to reiterate the test logic.
No single code file may be longer than 400 lines. Long switch statements or dispatch tables should be broken up by feature and constituents colocated with targets. Large blocks of test data should be moved to external data files.
Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider using Arc to reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Every module must begin with a module level (//! ) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessary mut bindings.
Handle errors with the Result type instead of panicking where feasible.
Avoid unsafe code unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not use return in single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Prefer expect over allow.
Prefer .expect() over .unwrap().
Use concat!() to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums. Derive std::error::Error (via the thiserror crate) for any condition the caller might inspect, retry, or map to an HTTP status....
Files:
tests/middleware_order.rsexamples/metadata_routing.rstests/middleware.rssrc/middleware.rstests/routes.rstests/lifecycle.rssrc/app.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/middleware_order.rsexamples/metadata_routing.rstests/middleware.rssrc/middleware.rstests/routes.rstests/lifecycle.rssrc/app.rs
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Documentation must use en-GB-oxendict spelling and grammar. (EXCEPTION: the naming of the LICENSE file, which is to be left unchanged for community consistency.)
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown files must be wrapped at 120 columns.
Tables and headings in Markdown files must not be wrapped.
Use dashes (-) for list bullets in Markdown files.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Files:
README.mddocs/rust-binary-router-library-design.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
README.mddocs/rust-binary-router-library-design.md
docs/**/*.md
📄 CodeRabbit Inference Engine (docs/contents.md)
Conventions for writing project documentation as described in the documentation style guide
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-oxendict) for documentation text.
The word "outwith" is acceptable in documentation.
Keep US spelling when used in an API, for examplecolor.
Use the Oxford comma in documentation text.
Treat company names as collective nouns in documentation (e.g., "Lille Industries are expanding").
Write headings in sentence case in documentation.
Use Markdown headings (#,##,###, etc.) in order without skipping levels.
Follow markdownlint recommendations for Markdown files.
Provide code blocks and lists using standard Markdown syntax.
Always provide a language identifier for fenced code blocks; useplaintextfor non-code text.
Use-as the first level bullet and renumber lists when items change.
Prefer inline links using[text](url)or angle brackets around the URL; avoid reference-style links like[foo][bar].
Ensure blank lines before and after bulleted lists and fenced blocks in Markdown.
Ensure tables have a delimiter line below the header row in Markdown.
Expand any uncommon acronym on first use, for example, Continuous Integration (CI).
Wrap paragraphs at 80 columns in documentation.
Wrap code at 120 columns in documentation.
Do not wrap tables in documentation.
Use sequentially numbered footnotes referenced with[^1]and place definitions at the end of the file.
Where it adds clarity, include Mermaid diagrams in documentation.
When embedding figures, useand provide concise alt text describing the content.
Add a brief description before each Mermaid diagram in documentation for screen readers.Document examples showing how to deprecate old message versions gracefully
Write the official documentation for the new features. Create separate guides for "Duplex Messaging & Pushes", "Streaming Responses", and "Message Fragmentation". Each guide must include runna...
Files:
docs/rust-binary-router-library-design.md
docs/**/*.{md,rs}
📄 CodeRabbit Inference Engine (docs/multi-packet-and-streaming-responses-design.md)
docs/**/*.{md,rs}: The official documentation and examples must exclusively use the declarativeResponsemodel for handler responses.
The async-stream pattern must be documented as the canonical approach for dynamic stream generation.
Files:
docs/rust-binary-router-library-design.md
🧬 Code Graph Analysis (4)
tests/middleware_order.rs (3)
src/app.rs (5)
new(221-227)new(245-251)new(357-357)correlation_id(190-190)correlation_id(235-235)tests/lifecycle.rs (1)
correlation_id(112-112)tests/routes.rs (1)
correlation_id(33-33)
examples/metadata_routing.rs (1)
src/app.rs (5)
new(221-227)new(245-251)new(357-357)id(187-187)id(232-232)
src/middleware.rs (3)
src/app.rs (7)
new(221-227)new(245-251)new(357-357)correlation_id(190-190)correlation_id(235-235)from_parts(196-196)from_parts(239-239)src/extractor.rs (3)
new(120-120)new(210-210)req(269-269)src/rewind_stream.rs (1)
new(25-31)
tests/routes.rs (3)
src/app.rs (15)
id(187-187)id(232-232)correlation_id(190-190)correlation_id(235-235)into_parts(193-193)into_parts(237-237)new(221-227)new(245-251)new(357-357)from_parts(196-196)from_parts(239-239)default(297-310)from(143-143)from(272-272)from(276-276)wireframe_testing/src/helpers.rs (1)
drive_with_bincode(343-362)src/message.rs (1)
from_bytes(50-55)
🔍 MCP Research (1 server)
Deepwiki:
-
docs/rust-binary-router-library-design.md: Introduces a new subsection "4.3.1 Packet Abstraction" describing the
Packettrait representing transport frames, which can be decomposed intoPacketPartsfor efficient handling and re-assembly. The default implementation isEnvelope, which implementsPacketand carries payload and metadata. ThePackettrait defines methodsid(),correlation_id() -> Option<u64>,into_parts() -> PacketParts, andfrom_parts(parts: PacketParts) -> Self. ThePacketPartsstruct holdsid, optionalcorrelation_id, andpayload. TheEnvelopestruct contains these fields and implementsPacket, providing a constructor andinto_parts()method. This addition clarifies the abstraction for frame representation and decomposition, emphasizing reuse ofPacketPartsto avoid repetitive tuple unpacking during frame splitting. -
src/app.rs: The
Packettrait and related types have been refactored to useOption<u64>for the correlation ID instead of a plainu64. The trait method signatures were updated accordingly. ThePacketPartsstruct was introduced to encapsulateid, optionalcorrelation_id, andpayloadfields. TheEnvelopestruct was flattened by removing the nestedPacketHeaderand directly storingid,correlation_id: Option<u64>, andpayload. TheEnvelopeimplementsPacketby delegating to these fields and converting to/fromPacketParts. ThePacketPartsstruct includes a constructor and aninherit_correlationmethod that sets the correlation ID if missing or logs a warning if mismatched. Logging statements in connection handling now include bothidand optionalcorrelation_idfor improved context. -
src/middleware.rs: The
ServiceRequeststruct'scorrelation_idfield type was changed fromu64toOption<u64>, with constructor and accessor updated accordingly. TheServiceResponsestruct was extended to include an optionalcorrelation_idfield, with constructor and accessor updated. In theRouteService<E: Packet>implementation of theServicetrait, packet envelope creation now uses aPacketPartsstruct to encapsulate route ID, optional correlation ID, and payload bytes. Response construction extracts payload and correlation ID from the envelope's parts and passes both toServiceResponse::new. These changes propagate the optional correlation ID through request and response types and unify packet envelope construction/deconstruction aroundPacketParts. -
tests/routes.rs: The
TestEnvelopestruct was modified to change thecorrelation_idfield from au64to anOption<u64>, and themsgfield was renamed topayload. ThePackettrait implementation forTestEnvelopewas updated accordingly: thecorrelation_id()method now returnsOption<u64>;into_parts()returns aPacketPartsstruct instead of a tuple; andfrom_parts()now takes aPacketPartsstruct instead of separate parameters. All test code creatingTestEnvelopeinstances was updated to useSome(value)orNoneforcorrelation_idand to usepayloadinstead ofmsg. Assertions checkingcorrelation_idwere changed to expectOption<u64>. Several new tests were added: one verifies handling ofNonecorrelation ID in a handler echo; another parametrized test checks propagation of variousOption<u64>correlation IDs through a single frame; and a unit test verifies round-trip conversion betweenTestEnvelopeandPacketParts. The changes affect serialization, deserialization, and frame processing logic to accommodate optional correlation IDs and renamed payload field, but no changes to control flow or error handling were introduced.
🔇 Additional comments (12)
tests/middleware_order.rs (1)
67-94: LGTM: correct use of optional correlation ID and PacketPartsCreate Envelope with Some(7), round-trip via into_parts, and assert payload + correlation_id. This aligns with the new API.
tests/middleware.rs (1)
15-16: LGTM: propagate correlation_id through the serviceExtract the Option from the request and pass it to ServiceResponse::new. This matches the new API surface.
tests/lifecycle.rs (1)
105-107: LGTM: adopt PacketParts and optional correlation ID in StateEnvelopeUse Option fields and implement Packet via PacketParts::{new, ...}. This matches the refactor and keeps functions concise without unnecessary return.
Also applies to: 109-125
docs/rust-binary-router-library-design.md (1)
532-535: Good addition; clearly explains thePacket/PacketPartsintent.The narrative concisely motivates
PacketPartsand its reuse.src/middleware.rs (1)
314-322: Envelope construction via PacketParts is correct and future‑proof.Building
EfromPacketPartsand returningServiceResponsefrom parts aligns with the refactor goals.tests/routes.rs (4)
21-26: Update toOption<u64>andpayloadlooks correct; trait impl matches new shape.Accessors and parts conversions are sound; inlining small accessors is a nice touch.
Also applies to: 28-46
95-126: Solid None-path coverage.This validates end-to-end propagation for missing correlation IDs.
185-227: Parametrised propagation test adds valuable breadth.Good coverage across
Noneand multipleSomevalues.
228-239: Round‑trip parts test is tight and useful.This guards the conversion contract.
src/app.rs (3)
230-240: Accessor inlining and conversion viaFromare on point.The
Packetimpl forEnvelopeis minimal and leverages the new conversions well.
242-269:PacketParts::newandinherit_correlationare well‑scoped.Constructor and correlation inheritance (with a warning on mismatch) fit the design and logging guidance.
773-793: Preserve/validate correlation IDs on the response path.Constructing parts with
resp.correlation_id()and then calling.inherit_correlation(env.correlation_id)is the right ordering; logging includes bothidandcorrelation_id.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
src/app.rs (1)
265-271: Conversions implemented – reduces duplication
From<Envelope>andFrom<PacketParts>centralise mapping logic exactly as previously requested.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
README.md(2 hunks)docs/api.md(1 hunks)docs/roadmap.md(1 hunks)docs/rust-binary-router-library-design.md(1 hunks)src/app.rs(6 hunks)src/middleware.rs(6 hunks)tests/lifecycle.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
README.mddocs/roadmap.mddocs/api.mddocs/rust-binary-router-library-design.md
**/*.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/lifecycle.rssrc/app.rssrc/middleware.rs
🧬 Code Graph Analysis (3)
tests/lifecycle.rs (3)
tests/routes.rs (4)
id(30-30)correlation_id(33-33)into_parts(35-37)from_parts(39-45)src/app.rs (15)
id(181-181)id(226-226)correlation_id(184-184)correlation_id(229-229)into_parts(187-187)into_parts(231-231)new(215-221)new(239-245)new(347-347)from_parts(190-190)from_parts(233-233)from(143-143)from(266-266)from(270-270)default(291-304)wireframe_testing/src/helpers.rs (1)
processor(22-22)
src/app.rs (3)
tests/lifecycle.rs (4)
correlation_id(112-112)into_parts(114-116)from_parts(118-124)id(110-110)tests/routes.rs (4)
correlation_id(33-33)into_parts(35-37)from_parts(39-45)id(30-30)src/metrics.rs (2)
inc_handler_errors(78-78)inc_handler_errors(81-81)
src/middleware.rs (1)
src/app.rs (7)
new(215-221)new(239-245)new(347-347)correlation_id(184-184)correlation_id(229-229)from_parts(190-190)from_parts(233-233)
🪛 LanguageTool
docs/api.md
[typographical] ~11-~11: Consider using an em dash in dialogues and enumerations.
Context: ...relation_id, payload); ``` - id: u32 – frame identifier - `correlation_id: Opt...
(DASH_RULE)
🔇 Additional comments (10)
tests/lifecycle.rs (1)
155-165: Validaterun_appcontract before casting bytesThe test assumes
run_appreturns a single contiguousVec<u8>, yet the API previously supplied aVec<Vec<u8>>. Passing a slice of vectors toBytesMut::fromwill not compile. Confirm the return type and adapt:-let mut buf = BytesMut::from(&out[..]); +let mut buf = BytesMut::from(&out);or iterate if multiple frames are possible.
src/middleware.rs (4)
41-52: Optional correlation ID wiring looks correct
ServiceRequestnow stores anOption<u64>and the ctor initialises it cleanly.
58-60: Accessor is fineThe getter obeys naming and returns
Option<u64>as expected.
74-86: ServiceResponse gains correlation-id support correctlyConstructor and storage align with the request side.
319-327: Good use ofPacketParts::newSwitching to the helper collapses three lines of boiler-plate and keeps the call site readable.
src/app.rs (4)
166-174: Example now matches public APIDoc test uses
PacketParts::newand correct field names.
183-191: Trait signature update is soundReturning
Option<u64>lines up with wire format and avoids sentinel values.
622-624: Log context addedConnection-level error now carries a correlation placeholder, improving trace consistency.
751-783: Logging enrichments are consistentAll warn paths now include
idand optionalcorrelation_id, aiding diagnostics.docs/roadmap.md (1)
158-166: Record completion of packet decomposition and optional correlation identifiersAligns with the implemented API and uses consistent terminology (“server‑initiated push”). No further changes required.
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
src/app.rs (2)
153-176: Fix doctest imports for PacketParts in the Packet trait exampleImport
PacketPartsso the doctest compiles.-/// use wireframe::{app::Packet, message::Message}; +/// use wireframe::{app::{Packet, PacketParts}, message::Message};
324-349: Fix doctest imports in WireframeApp::new exampleImport
PacketPartsin the example so doctests compile.- /// use wireframe::app::{Packet, WireframeApp}; + /// use wireframe::app::{Packet, PacketParts, WireframeApp};
♻️ Duplicate comments (2)
src/app.rs (2)
1-792: Decompose this module to meet the 400-line limitSplit by responsibility (packet.rs, envelope.rs, builder.rs, protocol.rs, io.rs, connection.rs). An issue already tracks this.
Link: #291
193-199: Avoid exposing PacketParts fields publicly to protect invariantsHide fields and expose getters to prevent external mutation bypassing
inherit_correlationinvariants. This will require updating call sites (tests and examples).If you want to stage this without breaking external users, first add getters and deprecate direct field access, then make fields private in a follow-up release.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/app.rs(6 hunks)src/middleware.rs(6 hunks)tests/packet_parts.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Function documentation must include clear examples demonstrating the usage and outcome of the function. Test documentation should omit examples where the example serves only to reiterate the test logic.
No single code file may be longer than 400 lines. Long switch statements or dispatch tables should be broken up by feature and constituents colocated with targets. Large blocks of test data should be moved to external data files.
Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider using Arc to reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Every module must begin with a module level (//! ) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessary mut bindings.
Handle errors with the Result type instead of panicking where feasible.
Avoid unsafe code unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not use return in single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Prefer expect over allow.
Prefer .expect() over .unwrap().
Use concat!() to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums. Derive std::error::Error (via the thiserror crate) for any condition the caller might inspect, retry, or map to an HTTP status....
Files:
tests/packet_parts.rssrc/middleware.rssrc/app.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/packet_parts.rssrc/middleware.rssrc/app.rs
🧬 Code Graph Analysis (2)
tests/packet_parts.rs (1)
src/app.rs (10)
new(215-221)new(239-245)new(350-350)from(143-143)from(269-269)from(273-273)id(181-181)id(226-226)correlation_id(184-184)correlation_id(229-229)
src/app.rs (3)
tests/lifecycle.rs (4)
correlation_id(112-112)into_parts(114-116)from_parts(118-124)id(110-110)tests/routes.rs (4)
correlation_id(33-33)into_parts(35-37)from_parts(39-45)id(30-30)src/metrics.rs (2)
inc_handler_errors(78-78)inc_handler_errors(81-81)
🔍 MCP Research (1 server)
Deepwiki:
-
Document [docs/rust-binary-router-library-design.md] describes the introduction of a
Packettrait representing transport frames, decomposable intoPacketPartsfor efficient handling and re-assembly. The default implementation isEnvelope, which implementsPacketand carries payload and metadata.PacketPartsholdsid: u32,correlation_id: Option<u64>, andpayload: Vec<u8>. TheEnvelopestruct no longer contains a nestedPacketHeaderbut directly stores these fields. ThePackettrait's methodscorrelation_id(),into_parts(), andfrom_parts()are updated accordingly.PacketPartsincludes a constructor and aninherit_correlationmethod. (docs/rust-binary-router-library-design.md) -
Document [docs/api.md] adds API documentation for the
PacketPartsstruct, decomposing a packet intoid: u32,correlation_id: Option<u64>(whereNoneindicates unsolicited event or server push), andpayload: Vec<u8>. It includes example Rust code demonstrating creation and conversion to/fromPacketParts. It notes thatNonecorrelation IDs propagate to ensure server-originated packets do not carry unintended correlation identifiers. (docs/api.md) -
Document [src/app.rs] details updates to the
Packettrait and related types to useOption<u64>for correlation IDs instead ofu64. Trait method signatures forcorrelation_id(),into_parts(), andfrom_parts()useOption<u64>andPacketParts. TheEnvelopestruct fields removePacketHeaderand directly includeid,correlation_id: Option<u64>, andpayload.EnvelopeimplementsPacketby delegating to these fields and converting to/fromPacketParts. ThePacketPartsstruct is added with methodsnew()andinherit_correlation(). Logging and routing logic include optional correlation IDs. (src/app.rs) -
Document [src/middleware.rs] describes changes to
ServiceRequestandServiceResponsestructs to useOption<u64>for correlation IDs.ServiceRequest'scorrelation_idfield type changed fromu64toOption<u64>, with constructor and accessor updated accordingly.ServiceResponsewas extended with an optionalcorrelation_idfield, constructor, setter, and accessor updated to handle this. TheRouteService<E: Packet>implementation usesPacketPartsto encapsulate route ID, optional correlation ID, and payload bytes when creating envelopes and constructing responses. (src/middleware.rs)
🔇 Additional comments (9)
tests/packet_parts.rs (1)
5-14: LGTM: Round-trip conversion behaves correctlyThe assertions cover id, correlation_id and payload. This validates Envelope ↔ PacketParts conversions as intended.
src/middleware.rs (2)
41-61: Propagate optional correlation ID through requestsSwitching
ServiceRequesttoOption<u64>and exposingcorrelation_id()is correct and aligns with the updated Packet trait semantics.
75-106: Expose correlation on responses with a fluent setterThe new
ServiceResponsecorrelation field, accessor, and chainable#[must_use]setter are well-shaped for middleware.src/app.rs (6)
183-191: Packet trait refactor to Option and PacketParts is on pointThe trait surface matches the PR objective and the wider design. Method signatures and semantics are coherent.
236-245: PacketParts::new is correct and keeps construction consistentConstructor looks good and centralises initialisation.
268-274: Prefer From/Into conversions to slim Packet implsUsing
From<Envelope>andFrom<PacketParts>is the right shape; thePacketimpl forwarding to these reads cleanly.
625-627: Include correlation context in termination logsGood: termination warns include an explicit
correlation_id = ?None::<u64>for consistency.
753-755: Include correlation context in deserialisation failure logsGood: warning includes a correlation field (None) and the error. This aligns logging across the module.
766-786: Route through optional correlation correctly
- Build
ServiceRequestwith the incoming correlation, pass through handler, and construct the response viaPacketParts::inherit_correlationto ensure presence/correction.- Log handler and DLQ errors with id and correlation context.
This is the intended propagation model.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR introduces a significant architectural improvement to the Wireframe library by implementing optional correlation IDs and introducing the PacketParts struct. The changes replace raw u64 correlation_id fields with Option<u64> throughout the codebase, making the optional nature of correlation tracking explicit. This design allows the system to properly distinguish between solicited responses (with correlation IDs) and unsolicited events or server-initiated pushes (with None correlation IDs).
The PacketParts struct serves as a unified container for packet components (id, correlation_id, payload), replacing the previous tuple-based approach for packet decomposition. This provides better type safety and eliminates repetitive tuple unpacking. The Envelope struct was flattened by removing the nested PacketHeader and directly embedding the packet fields.
Key changes include:
- Updated the
Packettrait to usePacketPartsforinto_parts()andfrom_parts()methods - Enhanced
ServiceRequestandServiceResponseto propagate correlation IDs through the middleware pipeline - Added sophisticated correlation ID inheritance logic with
inherit_correlation()method - Enriched logging throughout the system to include correlation IDs alongside message IDs
- Updated all tests, examples, and documentation to work with the new optional correlation ID system
The changes maintain backward compatibility while providing cleaner abstractions and better debugging capabilities through improved traceability.
Important Files Changed
File Changes
| Filename | Score | Overview |
|---|---|---|
src/app.rs |
4/5 | Core implementation of PacketParts struct and optional correlation ID system with inheritance logic |
src/middleware.rs |
4/5 | Updated middleware system to propagate optional correlation IDs through request/response cycle |
tests/packet_parts.rs |
5/5 | New comprehensive test suite for PacketParts struct and correlation ID inheritance |
tests/routes.rs |
5/5 | Updated routing tests to support optional correlation IDs with comprehensive parameterized testing |
tests/lifecycle.rs |
4/5 | Updated lifecycle tests to use new PacketParts API and verify correlation ID propagation |
tests/middleware.rs |
5/5 | Enhanced middleware tests with parameterized correlation ID testing using rstest |
tests/middleware_order.rs |
5/5 | Updated middleware ordering tests to verify correlation ID preservation through middleware chains |
tests/correlation_id.rs |
5/5 | Simple test update to use optional correlation IDs in streaming scenarios |
tests/metadata.rs |
5/5 | Minimal safe updates to use Some(0) for correlation_id in metadata parsing tests |
tests/world.rs |
5/5 | Updated Cucumber BDD test infrastructure to work with optional correlation IDs |
examples/ping_pong.rs |
5/5 | Updated ping/pong example to extract and propagate correlation IDs through all response paths |
examples/metadata_routing.rs |
5/5 | Updated example to use None for correlation IDs and improved error handling with .expect() |
README.md |
5/5 | Updated documentation to demonstrate new PacketParts API and optional correlation ID usage |
docs/api.md |
5/5 | New documentation file explaining PacketParts struct usage and None correlation ID semantics |
docs/rust-binary-router-library-design.md |
5/5 | Added new section documenting the Packet abstraction with UML diagram |
docs/roadmap.md |
5/5 | Updated roadmap to mark Packet decomposition feature as complete with proper formatting |
Confidence score: 4/5
- This PR introduces significant architectural improvements with good test coverage and documentation, but requires careful attention due to the complexity of the changes
- Score reflects thorough implementation of optional correlation IDs and PacketParts with comprehensive testing, though the wide-reaching nature of changes warrants careful review
- Pay close attention to
src/app.rsandsrc/middleware.rsfor the core correlation ID inheritance logic and middleware propagation patterns
Context used:
Rule - Prefer .expect() over .unwrap() (link)
16 files reviewed, no comments
There was a problem hiding this comment.
Greptile Summary
This review covers only the changes made since the last review (commit 020adc9), not the entire PR.
The recent changes primarily focus on improving API encapsulation and consistency. The most significant change is making the PacketParts struct fields private and providing controlled access through public getter methods (id(), correlation_id(), payload()). This follows proper encapsulation principles and prevents external code from directly mutating internal state.
The changes also include updates to all dependent code that was using direct field access - tests, examples, and middleware code have been refactored to use the new accessor methods instead. Documentation has been enhanced throughout with better examples for the Packet trait and improved documentation for the inherit_correlation method. Minor cosmetic improvements include fixing hyphenation consistency ("re-assembly" → "reassembly") and standardizing dash usage in documentation lists from en-dashes to em-dashes.
These changes maintain backward compatibility through existing From trait implementations while establishing a more stable and maintainable public API that can evolve without breaking client code.
Important Files Changed
File Changes
| Filename | Score | Overview |
|---|---|---|
src/app.rs |
5/5 | Made PacketParts fields private with public accessor methods for better encapsulation |
tests/packet_parts.rs |
5/5 | Updated tests to use new PacketParts accessor methods instead of direct field access |
tests/routes.rs |
5/5 | Refactored TestEnvelope implementation to use PacketParts accessor methods |
tests/lifecycle.rs |
5/5 | Updated StateEnvelope to use new PacketParts API with getter methods |
tests/middleware_order.rs |
5/5 | Adapted test to use unified Packet::into_parts() API instead of deprecated Envelope methods |
src/middleware.rs |
5/5 | Refactored RouteService to use PacketParts accessor methods consistently |
docs/api.md |
5/5 | Minor typography improvement replacing en-dashes with em-dashes |
docs/rust-binary-router-library-design.md |
5/5 | Fixed hyphenation consistency and added None correlation ID semantics |
README.md |
5/5 | Updated documentation to demonstrate new PacketParts accessor methods |
Confidence score: 5/5
- This PR is extremely safe to merge with minimal risk of breaking existing functionality
- Score reflects proper encapsulation improvements and comprehensive test updates that maintain backward compatibility
- No files require special attention as all changes follow consistent patterns and maintain existing behavior
9 files reviewed, no comments
|
@coderabbitai review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
examples/metadata_routing.rs (2)
65-65: Good replacement of unwrap() with expect() and improved error messages.This follows the project guideline to prefer expect() with context.
Also applies to: 76-76, 85-85, 101-101, 106-106
92-92: Use of “serialize” matches en‑GB‑oxendict “-ize” spelling.Spelling now aligns with the documented style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
README.md(3 hunks)docs/rust-binary-router-library-design.md(1 hunks)examples/metadata_routing.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
examples/metadata_routing.rs
**/*.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/rust-binary-router-library-design.mdREADME.md
🧬 Code Graph Analysis (1)
examples/metadata_routing.rs (2)
src/app.rs (7)
new(215-221)new(239-245)new(376-376)id(181-181)id(226-226)id(248-248)payload(254-254)tests/world.rs (1)
spawn(32-64)
🔍 MCP Research (1 server)
Deepwiki:
- The repository roadmap describes a Phase 6 "Multi-Packet Streaming Responses" feature that adds a correlation_id field to the Frame header to link multi-packet responses to the original request and a mechanism to signal end-of-stream; source: https://deepwiki.com/search/introduce-packetparts-and-opti_0208fd06-46f5-4796-b85f-74c1e10f7791
🔇 Additional comments (3)
examples/metadata_routing.rs (1)
55-55: Boldly keep None for absent correlation IDs.This correctly aligns the example with the Packet API’s Option and with repository conventions where None means “no correlation”.
README.md (2)
123-126: Keep None semantics explicit.This note correctly standardises on None (not Some(0)) for “no correlation” and links to PacketParts details.
98-114: Update the accompanying snippet to use expect() instead of unwrap().The custom envelope implementation is correct. Also adjust subsequent chained calls to avoid unwrap() in examples, keeping examples consistent with the guideline to prefer expect().
Example adjustment (illustrative):
-let app = WireframeApp::<_, _, MyEnv>::new() - .unwrap() +let app = WireframeApp::<_, _, MyEnv>::new() + .expect("failed to create app") .route(1, std::sync::Arc::new(|env: &MyEnv| Box::pin(async move { /* ... */ }))) - .unwrap(); + .expect("failed to add route");Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/app.rs (1)
146-181: Update docs to match the PacketParts APINo tuple-based signatures remain, but examples still refer to a
msgfield where the trait now usespayload. Rename these to keep docs and code in sync:
- In docs/message-versioning.md (lines 48–50), change
toversion: Option<u16>, msg: Vec<u8>,version: Option<u16>, payload: Vec<u8>,- Search other docs for direct
msg:fields in envelope-type examples or PacketParts usage and update them topayload:.
♻️ Duplicate comments (1)
src/app.rs (1)
1-826: Decompose module exceeding 400-line limit (tracked)Split src/app.rs by responsibility as previously suggested. This file still violates the ≤400-line guideline.
Issue already created (Refactor app.rs: decompose module exceeding 400-line limit). Do not block this PR further on structure; follow up in the linked issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/app.rs(8 hunks)tests/lifecycle.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Use precise names; boolean names should start with is/has/should
Use en-GB-oxendict spelling and grammar in comments
Function documentation must include clear examples; test documentation should omit redundant examples
Keep code files ≤ 400 lines; split long switch/dispatch logic by feature; move large test data to external files
Disallow Clippy warnings
Fix warnings emitted during tests in code rather than silencing them
Extract helper functions for long functions; adhere to separation of concerns and CQRS
Group related parameters into meaningful structs when functions have many parameters
Consider using Arc for large error returns to reduce data size
Each Rust module must begin with a module-level //! comment describing purpose and utility
Document public APIs with Rustdoc /// comments to enable cargo doc generation
Prefer immutable data; avoid unnecessary mut
Handle errors with Result instead of panicking where feasible
Avoid unsafe code unless necessary and document any usage clearly
Place function attributes after doc comments
Do not use return in single-line functions
Use predicate functions for conditional criteria with more than two branches
Do not silence lints except as a last resort
Lint suppressions must be tightly scoped and include a clear reason
Prefer #[expect(..)] over #[allow(..)] for lints
Prefer .expect() over .unwrap()
Use concat!() to combine long string literals rather than escaping newlines
Prefer single-line function bodies where appropriate (e.g., pub fn new(id: u64) -> Self { Self(id) })
Prefer semantic error enums deriving std::error::Error via thiserror for inspectable conditions
Files:
tests/lifecycle.rssrc/app.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/lifecycle.rssrc/app.rs
{src,tests}/**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
Write unit and behavioural tests for new functionality
Files:
tests/lifecycle.rssrc/app.rs
tests/**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
tests/**/*.rs: Use rstest fixtures for shared setup
Replace duplicated tests with #[rstest(...)] parameterised cases
Prefer mockall for mocks/stubs
Files:
tests/lifecycle.rs
🧬 Code Graph Analysis (2)
tests/lifecycle.rs (3)
tests/routes.rs (9)
id(30-30)correlation_id(33-33)into_parts(35-37)from_parts(39-48)BincodeSerializer(89-90)BincodeSerializer(122-123)BincodeSerializer(170-171)BincodeSerializer(178-179)BincodeSerializer(224-225)src/app.rs (17)
id(184-184)id(229-229)id(251-251)correlation_id(187-187)correlation_id(232-232)correlation_id(254-254)into_parts(190-190)into_parts(234-234)new(218-224)new(242-248)new(379-379)from_parts(193-193)from_parts(236-236)payload(257-257)from(143-143)from(293-293)from(297-302)wireframe_testing/src/helpers.rs (1)
processor(22-22)
src/app.rs (3)
tests/lifecycle.rs (4)
correlation_id(112-112)into_parts(114-116)from_parts(118-127)id(110-110)src/middleware.rs (9)
correlation_id(60-60)correlation_id(105-105)new(22-22)new(47-52)new(88-93)new(151-151)new(202-202)new(288-298)id(312-312)tests/routes.rs (4)
correlation_id(33-33)into_parts(35-37)from_parts(39-48)id(30-30)
🔍 MCP Research (1 server)
Deepwiki:
- Page: Overview — The README example in the Overview page documents an older Packet trait API where into_parts() returns a raw tuple (u32, Vec) rather than PacketParts (see "Custom Envelope Types" snippet showing fn into_parts(self) -> (u32, Vec) and fn from_parts(id: u32, data: Vec) -> Self). (docs from leynos/wireframe: Page "Overview")
- Page: API Reference — The API Reference page describes the Packet trait and explicitly shows into_parts() -> (u32, Vec) and from_parts(id: u32, msg: Vec) -> Self in its "Message Processing APIs" / "Core Traits" section, indicating the repository's documented API includes the older tuple-based packet decomposition. (docs from leynos/wireframe: Page "API Reference")
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
🔇 Additional comments (7)
tests/lifecycle.rs (3)
102-107: Packet impl for test envelope aligns with new APIImplement Packet using PacketParts::new and accessors. The correlation_id: Option and payload naming are correct and consistent with the refactor.
Also applies to: 109-128
149-151: Encode with shared processor helper is correctUse processor() for encoding to keep symmetry with the app’s frame processor and avoid drift. Good.
153-167: Retain existing buffer conversion
run_app returnsio::Result<Vec<u8>>, sooutis a singleVec<u8>. Decoding withBytesMut::from(&out[..])is correct—drop the indexing suggestion.src/app.rs (4)
154-177: Update examples to PacketParts and payload is correctDemonstrate Packet::into_parts/from_parts using PacketParts::new and payload. This matches the new public surface.
187-194: Trait surface changes are coherentExpose correlation_id() -> Option, into_parts(self) -> PacketParts, and from_parts(parts) -> Self. This unifies decomposition across envelope types as intended.
210-223: Envelope flattening and conversions look correct
- Envelope stores id, correlation_id: Option, payload directly, matching docs/tests.
- Packet impl delegates via From conversions; From/From centralise mapping. Good.
Also applies to: 227-237, 292-303
654-656: Tracing now includes correlation context; keep it consistent
- Maintain correlation context consistently across warn! logs (done here).
- Good use of correlation_id = ?None:: where not yet known.
Also applies to: 783-791, 795-803, 814-820
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
src/app.rs (2)
153-181: Fix doctest imports to include PacketPartsImport
PacketPartsin the example; otherwise the doctest cannot resolve the symbol.-/// use wireframe::{ -/// app::{Packet, PacketParts}, -/// message::Message, -/// }; +/// use wireframe::{ +/// app::{Packet, PacketParts}, +/// message::Message, +/// };And in the later example:
-/// use wireframe::app::{Packet, WireframeApp}; +/// use wireframe::app::{Packet, PacketParts, WireframeApp};
321-338: Minor doc style nit: British English spelling in docsKeep British English with Oxford -ize/-our in documentation comments. “Initializes” is acceptable (Oxford -ize), but audit nearby comments for stray Americanisms in follow‑ups.
♻️ Duplicate comments (1)
src/app.rs (1)
1-200: Split the module to stay within the 400‑line limitDecompose this file by responsibility as already tracked in issue #291; keep
Packet/PacketParts,Envelope, builder, protocol hooks, I/O, and connection handling in separate modules.Also applies to: 315-395
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
README.md(3 hunks)docs/rust-binary-router-library-design.md(1 hunks)src/app.rs(8 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Use precise names; boolean names should start with is/has/should
Use en-GB-oxendict spelling and grammar in comments
Function documentation must include clear examples; test documentation should omit redundant examples
Keep code files ≤ 400 lines; split long switch/dispatch logic by feature; move large test data to external files
Disallow Clippy warnings
Fix warnings emitted during tests in code rather than silencing them
Extract helper functions for long functions; adhere to separation of concerns and CQRS
Group related parameters into meaningful structs when functions have many parameters
Consider using Arc for large error returns to reduce data size
Each Rust module must begin with a module-level //! comment describing purpose and utility
Document public APIs with Rustdoc /// comments to enable cargo doc generation
Prefer immutable data; avoid unnecessary mut
Handle errors with Result instead of panicking where feasible
Avoid unsafe code unless necessary and document any usage clearly
Place function attributes after doc comments
Do not use return in single-line functions
Use predicate functions for conditional criteria with more than two branches
Do not silence lints except as a last resort
Lint suppressions must be tightly scoped and include a clear reason
Prefer #[expect(..)] over #[allow(..)] for lints
Prefer .expect() over .unwrap()
Use concat!() to combine long string literals rather than escaping newlines
Prefer single-line function bodies where appropriate (e.g., pub fn new(id: u64) -> Self { Self(id) })
Prefer semantic error enums deriving std::error::Error via thiserror for inspectable conditions
Files:
src/app.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
src/app.rs
{src,tests}/**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
Write unit and behavioural tests for new functionality
Files:
src/app.rs
docs/**/*.md
📄 CodeRabbit Inference Engine (docs/documentation-style-guide.md)
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-oxendict) for documentation text.
The word "outwith" is acceptable in documentation.
Keep US spelling when used in an API, for examplecolor.
Use the Oxford comma in documentation text.
Treat company names as collective nouns in documentation (e.g., "Lille Industries are expanding").
Write headings in sentence case in documentation.
Use Markdown headings (#,##,###, etc.) in order without skipping levels.
Follow markdownlint recommendations for Markdown files.
Provide code blocks and lists using standard Markdown syntax.
Always provide a language identifier for fenced code blocks; useplaintextfor non-code text.
Use-as the first level bullet and renumber lists when items change.
Prefer inline links using[text](url)or angle brackets around the URL; avoid reference-style links like[foo][bar].
Ensure blank lines before and after bulleted lists and fenced blocks in Markdown.
Ensure tables have a delimiter line below the header row in Markdown.
Expand any uncommon acronym on first use, for example, Continuous Integration (CI).
Wrap paragraphs at 80 columns in documentation.
Wrap code at 120 columns in documentation.
Do not wrap tables in documentation.
Use sequentially numbered footnotes referenced with[^1]and place definitions at the end of the file.
Where it adds clarity, include Mermaid diagrams in documentation.
When embedding figures, useand provide concise alt text describing the content.
Add a brief description before each Mermaid diagram in documentation for screen readers.Document examples showing how to deprecate old message versions gracefully
Write the official documentation for the new features. Create separate guides for "Duplex Messaging & Pushes", "Streaming Responses", and "Message Fragmentation". Each guide must include runnable examples and explain the relevant concepts and APIs.
docs/**/*.md: Use docs/ markdown ...
Files:
docs/rust-binary-router-library-design.md
docs/**/*.{md,rs}
📄 CodeRabbit Inference Engine (docs/multi-packet-and-streaming-responses-design.md)
docs/**/*.{md,rs}: The official documentation and examples must exclusively use the declarativeResponsemodel for handler responses.
The async-stream pattern must be documented as the canonical approach for dynamic stream generation.
Files:
docs/rust-binary-router-library-design.md
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Markdown paragraphs and bullet points must be wrapped at 80 columns
Markdown code blocks must be wrapped at 120 columns
Do not wrap tables and headings in Markdown
Use dashes (-) for list bullets in Markdown
Use GitHub-flavoured Markdown footnotes ([^1])
Files:
docs/rust-binary-router-library-design.mdREADME.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/rust-binary-router-library-design.mdREADME.md
🧬 Code Graph Analysis (1)
src/app.rs (2)
tests/lifecycle.rs (4)
correlation_id(112-112)into_parts(114-116)from_parts(118-127)id(110-110)tests/routes.rs (4)
correlation_id(33-33)into_parts(35-37)from_parts(39-48)id(30-30)
🔍 MCP Research (2 servers)
Deepwiki:
- Document [CI/CD and Development Workflow]: The repository defines GitHub Actions workflows including .github/workflows/ci.yml (main CI) and .github/workflows/advanced-tests.yml (advanced/scheduled tests) used to run formatting, clippy, tests, and coverage steps. (docs: CI/CD and Development Workflow)
- Document [Observability and Monitoring]: The project exports Prometheus-ready metrics with concrete metric names such as wireframe_connections_active, wireframe_frames_processed_total, wireframe_push_queue_depth, wireframe_pushes_dropped_total, and wireframe_request_latency_seconds. (docs: Observability and Monitoring)
- Document [Asynchronous Messaging]: The SessionRegistry stores Weak entries (DashMap of ConnectionId → Weak) so lookups upgrade weak refs and prune dead connections; PushHandle uses token-bucket rate limiting and supports Dead Letter Queue routing for full queues. (docs: Asynchronous Messaging)
Context7:
Selected library ID: NONE — no relevant Context7 library match found
Reason: The resolve step returned no library matching the PR title or the wireframe repository. Please provide one of:
- the exact Context7-compatible library ID (format: /org/project or /org/project/version), or
- a clearer library/package name such as "leynos/wireframe" or the repository URL so I can resolve and fetch docs/issues for the specific codebase.
🔇 Additional comments (15)
docs/rust-binary-router-library-design.md (1)
537-544: LGTM: Correlation identifier semantics and streaming note are clearThe explanation that
Nonedenotes unsolicited events or server‑initiated pushes, plus the linkage/explicit EOS guidance for streaming, reads well and matches the updated API.src/app.rs (11)
186-194: Packet trait surface looks soundThe trait shape (id(), optional correlation_id(), into_parts()/from_parts()) is orthogonal and enables envelope-agnostic routing. No issues.
196-203: Keep PacketParts encapsulated; API matches docsPrivate fields with constructor/accessors are correct for invariants and future evolution. No issues.
209-224: Envelope flattening and Option correlation are correctFlattening
Envelopeand adoptingOption<u64>aligns with the PR objective and simplifies usage. Constructor shape is appropriate.
227-237: Delegate via From/Into to avoid duplicationUsing
From<Envelope> for PacketPartsand its inverse to implementPacketkeeps mapping centralised and tidy. Good.
259-290: Correlation inheritance behaviour is robust and well‑instrumentedThe function now fills missing CIDs and overwrites mismatches after a
warn!, including packetidfor traceability. Examples reflect behaviour. Good.
293-304: Conversions are clean and side‑effect freeUsing accessors in
From<PacketParts> for Envelopeavoids field leakage and keeps invariants intact.
626-637: Parse fast path with fallback is appropriateParsing via
FrameMetadata::parsewith a safe fallback to full deserialisation is correct. Error boxing is acceptable here.
655-657: Enrich connection error logs with correlation contextIncluding
correlation_id = ?None::<u64>ensures consistent log shape. This matches the logging guidance and previous feedback. Good.
777-794: Deserialisation failure handling is correct and boundedWarn, increment error metric, cap via
MAX_DESER_FAILURES, and continue. Including the correlation context (None) keeps log shape consistent.
796-804: Response correlation propagation is correctBuild
PacketPartswith handler output, inherit/overwrite CID from request as needed, then convert toEnvelope. Logging includes id and correlation_id. This addresses past ambiguity.
815-821: Good: include id and correlation_id in handler/no‑route warningsKeep these fields in all warnings for traceability.
README.md (3)
98-114: LGTM: Custom envelope example updated to PacketParts and optional CIDThe example now imports
PacketParts, usesOption<u64>, and constructs viaPacketParts::new(...). Access via accessors is correct.
123-126: LGTM: ClarifyNonecorrelation identifier semantics with cross‑referenceThe added note and link to
PacketPartsdocumentation are clear and consistent with the design.
293-295: LGTM: Link the example title to its fileThe hyperlink improves discoverability; keep this pattern for other examples.
|
@coderabbitai please create a new GitHub issue for the following: Name the consuming getter Follow Rust idioms for consuming accessors. Provide a non-breaking path by adding #[must_use]
- pub fn payload(self) -> Vec<u8> { self.payload }
+ pub fn into_payload(self) -> Vec<u8> { self.payload }Update internal examples and call sites to use |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll create a GitHub issue for this improvement to follow Rust naming conventions for consuming accessors. @leynos I've created a GitHub issue to track the improvement for renaming the consuming getter method to follow Rust idioms. The issue includes the proposed code change, implementation tasks, and references back to this PR and your comment. 🏁 Script executed: Length of output: 964 Perfect! I've successfully created GitHub issue #305 for renaming the consuming getter method to follow Rust idioms. The issue includes: ✅ Clear title and description with the proposed code change You can view the issue at: #305 The issue tracks the transition from |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/rust-binary-router-library-design.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.md
📄 CodeRabbit Inference Engine (docs/documentation-style-guide.md)
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-oxendict) for documentation text.
The word "outwith" is acceptable in documentation.
Keep US spelling when used in an API, for examplecolor.
Use the Oxford comma in documentation text.
Treat company names as collective nouns in documentation (e.g., "Lille Industries are expanding").
Write headings in sentence case in documentation.
Use Markdown headings (#,##,###, etc.) in order without skipping levels.
Follow markdownlint recommendations for Markdown files.
Provide code blocks and lists using standard Markdown syntax.
Always provide a language identifier for fenced code blocks; useplaintextfor non-code text.
Use-as the first level bullet and renumber lists when items change.
Prefer inline links using[text](url)or angle brackets around the URL; avoid reference-style links like[foo][bar].
Ensure blank lines before and after bulleted lists and fenced blocks in Markdown.
Ensure tables have a delimiter line below the header row in Markdown.
Expand any uncommon acronym on first use, for example, Continuous Integration (CI).
Wrap paragraphs at 80 columns in documentation.
Wrap code at 120 columns in documentation.
Do not wrap tables in documentation.
Use sequentially numbered footnotes referenced with[^1]and place definitions at the end of the file.
Where it adds clarity, include Mermaid diagrams in documentation.
When embedding figures, useand provide concise alt text describing the content.
Add a brief description before each Mermaid diagram in documentation for screen readers.Document examples showing how to deprecate old message versions gracefully
Write the official documentation for the new features. Create separate guides for "Duplex Messaging & Pushes", "Streaming Responses", and "Message Fragmentation". Each guide must include runnable examples and explain the relevant concepts and APIs.
docs/**/*.md: Use docs/ markdown ...
Files:
docs/rust-binary-router-library-design.md
docs/**/*.{md,rs}
📄 CodeRabbit Inference Engine (docs/multi-packet-and-streaming-responses-design.md)
docs/**/*.{md,rs}: The official documentation and examples must exclusively use the declarativeResponsemodel for handler responses.
The async-stream pattern must be documented as the canonical approach for dynamic stream generation.
Files:
docs/rust-binary-router-library-design.md
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Markdown paragraphs and bullet points must be wrapped at 80 columns
Markdown code blocks must be wrapped at 120 columns
Do not wrap tables and headings in Markdown
Use dashes (-) for list bullets in Markdown
Use GitHub-flavoured Markdown footnotes ([^1])
Files:
docs/rust-binary-router-library-design.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/rust-binary-router-library-design.md
🔍 MCP Research (1 server)
Deepwiki:
- "DeepWiki (search result): The project's roadmap mentions "Phase 6: Multi-Packet Streaming Responses" which plans to add a correlation_id to frame headers so multi-packet responses can reuse a request's correlation id; the Connection actor would send each chunk as a Frame that carries that correlation_id. (https://deepwiki.com/search/introduce-packetparts-and-opti_d0b58aa4-c2d4-423f-8d7c-6161102490b1)"
- "ask_question (leynos/wireframe): The Packet trait overview in project docs describes id(), into_parts(), and from_parts() as the packet decomposition/reassembly interface; into_parts/from_parts are used to extract/construct id and payload bytes for routing and custom Envelope implementations. (leynos/wireframe internal docs via ask_question)"
🔇 Additional comments (2)
docs/rust-binary-router-library-design.md (2)
501-506: Keep sentence case and scope statement — LGTMThe subsection heading and the brief overview follow the documentation style guide and correctly scope the abstraction to
Packet,PacketParts, andEnvelope.
537-544: Clarify streaming linkage — LGTM and compliant with roadmapThe text explicitly ties
correlation_idto multi‑packet streaming and EOS signalling. This complies with the roadmap’s guidance on reusing a request’s correlation identifier across streamed frames.
1 participant
Summary
correlation_idfields withOption<u64>and addPacketPartsEnvelopeand propagate correlation IDs through middlewarecorrelation_idalongside message IDs for easier debuggingTesting
make fmtmake lintmake testmake markdownlinthttps://chatgpt.com/codex/tasks/task_e_68953cfe46648322978c741264be93a3
Summary by Sourcery
Introduce optional correlation identifiers and a unified PacketParts struct for packet APIs, flatten Envelope, propagate correlation IDs through middleware and routing logic, and enrich logging and tests to support optional correlation IDs
New Features:
Enhancements:
Documentation:
Tests: