Conversation
Reviewer's GuideThis PR updates the length-prefixed framing example in the documentation to properly propagate I/O errors in both decoding and encoding, replaces panicking calls with error logic to avoid unexpected crashes, and revises the example heading and formatting to emphasize this behavior. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe update revises the example of a length-prefixed frame processor in the documentation, focusing on improved error handling by replacing Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Codec as LengthPrefixedCodec
participant Buffer as BytesMut
App->>Codec: decode(buffer)
Codec->>Buffer: read length prefix with error propagation (?)
Codec->>Buffer: check if enough bytes for frame
Codec-->>App: return frame or Ok(None) on incomplete data
App->>Codec: encode(frame, buffer)
Codec->>Buffer: check frame size <= MAX_FRAME_LEN
Codec->>Buffer: write length prefix and frame bytes
Possibly related issues
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (1)
`**/*.md`: * Avoid 2nd person or 1st person pronouns ("I", "you", "we") * Use en...
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-oxendic 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.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Consider using the
Buftrait’s get_u32 method directly onBytesMutinstead of creating a newCursorfor efficiency. - The example now enforces a max frame size on encode—update the surrounding docs to mention error propagation on the encoding side as well.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using the `Buf` trait’s get_u32 method directly on `BytesMut` instead of creating a new `Cursor` for efficiency.
- The example now enforces a max frame size on encode—update the surrounding docs to mention error propagation on the encoding side as well.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (7)
`docs/**/*.md`: Documentation must use en-GB-oxendict spelling and grammar (with the exception of "license" which is to be left unchanged for community consistency).
docs/**/*.md: Documentation must use en-GB-oxendict spelling and grammar (with the exception of "license" which is to be left unchanged for community consistency).
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`**/*.md`: Validate Markdown files using `markdownlint *.md **/*.md`. Run `mdfor...
**/*.md: Validate Markdown files usingmarkdownlint *.md **/*.md.
Runmdformat-allafter any documentation changes to format all Markdown files and fix table markup.
Validate Markdown Mermaid diagrams using thenixieCLI. The tool is already installed; runnixie *.md **/*.mddirectly instead of usingnpx.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks must be wrapped at 120 columns.
Tables and headings must not be wrapped.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Provide user guides and examples demonstrating server-initiated messaging.
docs/**/*.md: Provide user guides and examples demonstrating server-initiated messaging.
📄 Source: CodeRabbit Inference Engine (docs/asynchronous-outbound-messaging-roadmap.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Conventions for writing project documentation should follow the rules outlined in the documentation style guide.
docs/**/*.md: Conventions for writing project documentation should follow the rules outlined in the documentation style guide.
📄 Source: CodeRabbit Inference Engine (docs/contents.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Use British English based on the Oxford English Dictionary (en-o...
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-oxendict) for documentation.
The word "outwith" is acceptable in documentation.
Keep US spelling when used in an API, for examplecolor.
Use the Oxford comma in documentation.
Company names are treated as collective nouns (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 use fenced code blocks with a language identifier; 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 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 footnotes referenced with[^label]in documentation.
Include Mermaid diagrams in documentation where it adds clarity.
When embedding figures in documentation, useand provide concise alt text describing the content.
Add a short description before each Mermaid diagram in documentation so screen readers can understand it.
📄 Source: CodeRabbit Inference Engine (docs/documentation-style-guide.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Write the official documentation for the new features. Create se...
docs/**/*.md: 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.
📄 Source: CodeRabbit Inference Engine (docs/wireframe-1-0-detailed-development-roadmap.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`**/*.md`: * Avoid 2nd person or 1st person pronouns ("I", "you", "we") * Use en...
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-oxendic 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.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
⏰ Context from checks skipped due to timeout of 240000ms (1)
- GitHub Check: build-test
🔇 Additional comments (2)
docs/rust-binary-router-library-design.md (2)
1141-1146: Error-propagation change compiles cleanly – good fixReplacing
expectwith?and introducingCursorresolves the compile-time
type mismatch flagged in the earlier review, and correctly propagates I/O
errors to the caller. Implementation looks sound.
1164-1168: Oversize-frame guard added – previous concern resolvedThe
encodepath now mirrors thedecodelimit check, preventing peers from
forcing unbounded allocations. This addresses the earlier “frame too large”
comment perfectly.
| (Note: "wireframe" would abstract the direct use of `Encoder`/`Decoder` behind | ||
| its own `FrameProcessor` trait or provide helpers.) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Line length exceeds documentation guidelines
docs/**/*.md must wrap prose at 80 columns, but the note starting with
“Note: "wireframe" would abstract…” runs far longer. Please re-wrap or let
mdformat-all handle it so CI markdown-lint passes.
🤖 Prompt for AI Agents
In docs/rust-binary-router-library-design.md around lines 1178 to 1179, the note
about "wireframe" exceeds the 80-column line length limit for documentation.
Reformat the text to wrap lines at or below 80 characters, either manually or by
running the mdformat-all tool, to ensure it complies with markdown linting
rules.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (7)
`docs/**/*.md`: Documentation must use en-GB-oxendict spelling and grammar (with the exception of "license" which is to be left unchanged for community consistency).
docs/**/*.md: Documentation must use en-GB-oxendict spelling and grammar (with the exception of "license" which is to be left unchanged for community consistency).
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`**/*.md`: Validate Markdown files using `markdownlint *.md **/*.md`. Run `mdfor...
**/*.md: Validate Markdown files usingmarkdownlint *.md **/*.md.
Runmdformat-allafter any documentation changes to format all Markdown files and fix table markup.
Validate Markdown Mermaid diagrams using thenixieCLI. The tool is already installed; runnixie *.md **/*.mddirectly instead of usingnpx.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks must be wrapped at 120 columns.
Tables and headings must not be wrapped.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Provide user guides and examples demonstrating server-initiated messaging.
docs/**/*.md: Provide user guides and examples demonstrating server-initiated messaging.
📄 Source: CodeRabbit Inference Engine (docs/asynchronous-outbound-messaging-roadmap.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Conventions for writing project documentation should follow the rules outlined in the documentation style guide.
docs/**/*.md: Conventions for writing project documentation should follow the rules outlined in the documentation style guide.
📄 Source: CodeRabbit Inference Engine (docs/contents.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Use British English based on the Oxford English Dictionary (en-o...
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-oxendict) for documentation.
The word "outwith" is acceptable in documentation.
Keep US spelling when used in an API, for examplecolor.
Use the Oxford comma in documentation.
Company names are treated as collective nouns (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 use fenced code blocks with a language identifier; 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 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 footnotes referenced with[^label]in documentation.
Include Mermaid diagrams in documentation where it adds clarity.
When embedding figures in documentation, useand provide concise alt text describing the content.
Add a short description before each Mermaid diagram in documentation so screen readers can understand it.
📄 Source: CodeRabbit Inference Engine (docs/documentation-style-guide.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Write the official documentation for the new features. Create se...
docs/**/*.md: 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.
📄 Source: CodeRabbit Inference Engine (docs/wireframe-1-0-detailed-development-roadmap.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`**/*.md`: * Avoid 2nd person or 1st person pronouns ("I", "you", "we") * Use en...
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-oxendic 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.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
⏰ Context from checks skipped due to timeout of 240000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
🔇 Additional comments (1)
docs/rust-binary-router-library-design.md (1)
1162-1167: Oversize-frame guard inencodelooks spot-onNice addition—this mirrors the check already present in
decode, prevents unbounded allocations, and resolves the earlier review note.
| 1. **Frame Processor Implementation** (Simple length-prefixed framing using | ||
| `tokio-util`; invalid input or oversized frames return `io::Error` from both | ||
| decode and encode): | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Wrap prose at 80 columns to satisfy documentation linting
The bullet text introducing the codec example runs far beyond the 80-column limit mandated for docs/**/*.md. This will fail markdownlint/CI.
🤖 Prompt for AI Agents
In docs/rust-binary-router-library-design.md around lines 1123 to 1126, the
bullet point text exceeds the 80-column limit required by markdownlint. Reformat
the text by wrapping the prose so that no line exceeds 80 characters, ensuring
it meets the documentation linting standards without altering the content.
|
@sourcery-ai review |
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Advance the buffer by 4 bytes to consume the length prefix before splitting out the frame payload. (link)
General comments:
- The
decodeexample currently consumes the 4-byte length prefix viaget_u32()before checking if the full payload is available, so on a partial frame you lose the header—consider peeking the length without advancing (or only advancing after verifyingsrc.len() >= 4 + length). - The numbered list in the docs now has two ‘1.’ entries (for framing and server setup), so renumber the sections to maintain a proper sequence.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `decode` example currently consumes the 4-byte length prefix via `get_u32()` before checking if the full payload is available, so on a partial frame you lose the header—consider peeking the length without advancing (or only advancing after verifying `src.len() >= 4 + length`).
- The numbered list in the docs now has two ‘1.’ entries (for framing and server setup), so renumber the sections to maintain a proper sequence.
## Individual Comments
### Comment 1
<location> `docs/rust-binary-router-library-design.md:1158` </location>
<code_context>
+ }
- src.advance(4); // Consume length prefix
- Ok(Some(src.split_to(length)))
- }
- }
</code_context>
<issue_to_address>
Advance the buffer by 4 bytes to consume the length prefix before splitting out the frame payload.
Currently, you split out the payload without first advancing past the 4-byte length prefix, so the returned BytesMut will include the length prefix. You should call src.advance(4) before split_to(length).
</issue_to_address>Hi @leynos! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.| Ok(Some(src.split_to(length))) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
issue (review_instructions): Advance the buffer by 4 bytes to consume the length prefix before splitting out the frame payload.
Currently, you split out the payload without first advancing past the 4-byte length prefix, so the returned BytesMut will include the length prefix. You should call src.advance(4) before split_to(length).
Review instructions:
Path patterns: **/*
Instructions:
Create code-review comments for ALL issues. Avoid making general observations or non-specific feedback if at all possible.
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Do not use get_u32() here as it advances the buffer before checking if enough data is present, which can corrupt the buffer state. (link)
- Check for src.len() < 4 + length, not just < length, to ensure the full frame (including prefix) is present. (link)
- Advance the buffer by 4 bytes to consume the length prefix before splitting the payload. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `docs/rust-binary-router-library-design.md:1144` </location>
<code_context>
- let length = (&src[..4])
- .read_u32::<BigEndian>()
- .expect("slice length checked") as usize;
+ let length = src.get_u32() as usize;
- if length > MAX_FRAME_LEN {
</code_context>
<issue_to_address>
Do not use get_u32() here as it advances the buffer before checking if enough data is present, which can corrupt the buffer state.
You should first check that src.len() >= 4, then use something like src[..4] to read the length without advancing the buffer. Only advance the buffer after confirming the full frame is present. This prevents buffer corruption if the frame is incomplete.
</issue_to_address>
### Comment 2
<location> `docs/rust-binary-router-library-design.md:1150` </location>
<code_context>
- src.reserve(4 + length - src.len());
- return Ok(None); // Not enough data for full frame
- }
+ if src.len() < length {
+ src.reserve(length - src.len());
+ return Ok(None); // Not enough data for full frame
</code_context>
<issue_to_address>
Check for src.len() < 4 + length, not just < length, to ensure the full frame (including prefix) is present.
The check should be if src.len() < 4 + length, since the length prefix is part of the buffer. Otherwise, you may attempt to split or advance the buffer incorrectly, leading to panics or data loss.
</issue_to_address>
### Comment 3
<location> `docs/rust-binary-router-library-design.md:1158` </location>
<code_context>
+ }
- src.advance(4); // Consume length prefix
- Ok(Some(src.split_to(length)))
- }
- }
</code_context>
<issue_to_address>
Advance the buffer by 4 bytes to consume the length prefix before splitting the payload.
You must call src.advance(4) before src.split_to(length) to remove the length prefix from the buffer. Otherwise, the returned frame will include the length prefix, which is incorrect.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| let length = (&src[..4]) | ||
| .read_u32::<BigEndian>() | ||
| .expect("slice length checked") as usize; | ||
| let length = src.get_u32() as usize; |
There was a problem hiding this comment.
issue (review_instructions): Do not use get_u32() here as it advances the buffer before checking if enough data is present, which can corrupt the buffer state.
You should first check that src.len() >= 4, then use something like src[..4] to read the length without advancing the buffer. Only advance the buffer after confirming the full frame is present. This prevents buffer corruption if the frame is incomplete.
Review instructions:
Path patterns: **/*
Instructions:
Create code-review comments for ALL issues. Avoid making general observations or non-specific feedback if at all possible.
| src.reserve(4 + length - src.len()); | ||
| return Ok(None); // Not enough data for full frame | ||
| } | ||
| if src.len() < length { |
There was a problem hiding this comment.
issue (review_instructions): Check for src.len() < 4 + length, not just < length, to ensure the full frame (including prefix) is present.
The check should be if src.len() < 4 + length, since the length prefix is part of the buffer. Otherwise, you may attempt to split or advance the buffer incorrectly, leading to panics or data loss.
Review instructions:
Path patterns: **/*
Instructions:
Create code-review comments for ALL issues. Avoid making general observations or non-specific feedback if at all possible.
| Ok(Some(src.split_to(length))) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
issue (review_instructions): Advance the buffer by 4 bytes to consume the length prefix before splitting the payload.
You must call src.advance(4) before src.split_to(length) to remove the length prefix from the buffer. Otherwise, the returned frame will include the length prefix, which is incorrect.
Review instructions:
Path patterns: **/*
Instructions:
Create code-review comments for ALL issues. Avoid making general observations or non-specific feedback if at all possible.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (7)
`docs/**/*.md`: Documentation must use en-GB-oxendict spelling and grammar (with the exception of "license" which is to be left unchanged for community consistency).
docs/**/*.md: Documentation must use en-GB-oxendict spelling and grammar (with the exception of "license" which is to be left unchanged for community consistency).
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`**/*.md`: Validate Markdown files using `markdownlint *.md **/*.md`. Run `mdfor...
**/*.md: Validate Markdown files usingmarkdownlint *.md **/*.md.
Runmdformat-allafter any documentation changes to format all Markdown files and fix table markup.
Validate Markdown Mermaid diagrams using thenixieCLI. The tool is already installed; runnixie *.md **/*.mddirectly instead of usingnpx.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks must be wrapped at 120 columns.
Tables and headings must not be wrapped.
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Provide user guides and examples demonstrating server-initiated messaging.
docs/**/*.md: Provide user guides and examples demonstrating server-initiated messaging.
📄 Source: CodeRabbit Inference Engine (docs/asynchronous-outbound-messaging-roadmap.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Conventions for writing project documentation should follow the rules outlined in the documentation style guide.
docs/**/*.md: Conventions for writing project documentation should follow the rules outlined in the documentation style guide.
📄 Source: CodeRabbit Inference Engine (docs/contents.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Use British English based on the Oxford English Dictionary (en-o...
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-oxendict) for documentation.
The word "outwith" is acceptable in documentation.
Keep US spelling when used in an API, for examplecolor.
Use the Oxford comma in documentation.
Company names are treated as collective nouns (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 use fenced code blocks with a language identifier; 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 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 footnotes referenced with[^label]in documentation.
Include Mermaid diagrams in documentation where it adds clarity.
When embedding figures in documentation, useand provide concise alt text describing the content.
Add a short description before each Mermaid diagram in documentation so screen readers can understand it.
📄 Source: CodeRabbit Inference Engine (docs/documentation-style-guide.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`docs/**/*.md`: Write the official documentation for the new features. Create se...
docs/**/*.md: 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.
📄 Source: CodeRabbit Inference Engine (docs/wireframe-1-0-detailed-development-roadmap.md)
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
`**/*.md`: * Avoid 2nd person or 1st person pronouns ("I", "you", "we") * Use en...
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-oxendic 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.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
docs/rust-binary-router-library-design.md
🔇 Additional comments (1)
docs/rust-binary-router-library-design.md (1)
1123-1125: Re-runmdformat-all; this bullet still looks > 80 chars in CI.The documentation guidelines insist that prose in
docs/**/*.mdis wrapped at
80 columns. Please runmdformat-all(or wrap manually) so markdownlint does
not fail on these three lines.
| // Crate: my_frame_processor.rs | ||
| use bytes::{BytesMut, Buf, BufMut}; | ||
| use tokio_util::codec::{Decoder, Encoder}; | ||
| use byteorder::{BigEndian, ByteOrder}; | ||
| use std::io; | ||
|
|
||
| const MAX_FRAME_LEN: usize = 16 * 1024 * 1024; // 16 MiB upper limit | ||
| const MAX_FRAME_LEN: usize = 16 * 1024 * 1024; // 16 MiB upper limit | ||
|
|
||
| pub struct LengthPrefixedCodec; | ||
| pub struct LengthPrefixedCodec; | ||
|
|
||
| impl Decoder for LengthPrefixedCodec { | ||
| type Item = BytesMut; // Raw frame payload | ||
| type Error = io::Error; | ||
| impl Decoder for LengthPrefixedCodec { | ||
| type Item = BytesMut; // Raw frame payload | ||
| type Error = io::Error; | ||
|
|
||
| fn decode(&mut self, src: &mut BytesMut) -> Result<Option<Self::Item>, Self::Error> { | ||
| if src.len() < 4 { return Ok(None); } // Not enough data for length prefix | ||
| fn decode(&mut self, src: &mut BytesMut) -> Result<Option<Self::Item>, Self::Error> { | ||
| if src.len() < 4 { return Ok(None); } // Not enough data for length prefix | ||
|
|
||
| let length = u32::from_be_bytes( | ||
| src[..4].try_into().expect("slice length checked"), | ||
| ) as usize; | ||
| let length = BigEndian::read_u32(&src[..4]) as usize; | ||
|
|
||
| if length > MAX_FRAME_LEN { | ||
| return Err(io::Error::new(io::ErrorKind::InvalidInput, "frame too large")); | ||
| } | ||
| if length > MAX_FRAME_LEN { | ||
| return Err(io::Error::new(io::ErrorKind::InvalidInput, "frame too large")); | ||
| } | ||
|
|
||
| if src.len() < 4 + length { | ||
| src.reserve(4 + length - src.len()); | ||
| return Ok(None); // Not enough data for full frame | ||
| } | ||
| if src.len() < 4 + length { | ||
| src.reserve(4 + length - src.len()); | ||
| return Ok(None); // Not enough data for full frame | ||
| } | ||
| src.advance(4); // Consume length prefix | ||
| Ok(Some(src.split_to(length))) | ||
| } | ||
| } | ||
|
|
||
| src.advance(4); // Consume length prefix | ||
| Ok(Some(src.split_to(length))) | ||
| } | ||
| } | ||
| impl<T: AsRef<[u8]>> Encoder<T> for LengthPrefixedCodec { | ||
| type Error = io::Error; | ||
|
|
||
| impl<T: AsRef<[u8]>> Encoder<T> for LengthPrefixedCodec { | ||
| type Error = io::Error; | ||
|
|
||
| fn encode(&mut self, item: T, dst: &mut BytesMut) -> Result<(), Self::Error> { | ||
| let data = item.as_ref(); | ||
| let length = u32::try_from(data.len()).map_err(|_| { | ||
| io::Error::new( | ||
| io::ErrorKind::InvalidInput, | ||
| "payload exceeds 4 GiB limit", | ||
| ) | ||
| })?; | ||
| dst.reserve(4 + data.len()); | ||
| dst.put_u32(length); | ||
| dst.put_slice(data); | ||
| Ok(()) | ||
| fn encode(&mut self, item: T, dst: &mut BytesMut) -> Result<(), Self::Error> { | ||
| let data = item.as_ref(); | ||
| if data.len() > MAX_FRAME_LEN { | ||
| return Err(io::Error::new(io::ErrorKind::InvalidInput, "frame too large")); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| (Note: "wireframe" would abstract the direct use of `Encoder`/`Decoder` behind | ||
| its own `FrameProcessor` trait or provide helpers.) | ||
| dst.reserve(4 + data.len()); | ||
| dst.put_u32(data.len() as u32); | ||
| dst.put_slice(data); | ||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Introduce a LEN_PREFIX constant for clarity & easier maintenance.
The literal 4 appears several times and couples the logic tightly to a
32-bit length prefix. Replacing the magic number with a named constant improves
readability and makes it trivial to swap in a different prefix width later.
@@
- if src.len() < 4 { return Ok(None); } // Not enough data for length prefix
+ const LEN_PREFIX: usize = 4;
+ if src.len() < LEN_PREFIX {
+ return Ok(None); // Not enough data for length prefix
+ }
@@
- if src.len() < 4 + length {
+ if src.len() < LEN_PREFIX + length {
src.reserve(LEN_PREFIX + length - src.len());
return Ok(None); // Not enough data for full frame
}
- src.advance(4); // Consume length prefix
+ src.advance(LEN_PREFIX); // Consume length prefix
@@
- dst.reserve(4 + data.len());
- dst.put_u32(data.len() as u32);
+ dst.reserve(LEN_PREFIX + data.len());
+ dst.put_u32(data.len() as u32);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docs/rust-binary-router-library-design.md between lines 1128 and 1174,
replace all occurrences of the literal 4, which represents the length prefix
size, with a new constant named LEN_PREFIX. Define LEN_PREFIX as 4 at the top of
the file to improve code clarity and maintainability. Update all length prefix
related calculations and buffer operations to use LEN_PREFIX instead of the
hardcoded 4.
2 participants
Summary
Testing
mdformat-allmake fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_686a864d8fc08322a56644be2c248414
Summary by Sourcery
Revise the length-prefixed framing example to propagate IO errors on invalid or oversized frames in both decode and encode paths and document this behavior.
Enhancements:
Documentation: