Open
Conversation
- Introduced `docs/developers-guide.md` defining architectural vocabulary and naming invariants. - Updated `docs/users-guide.md`, `docs/api.md`, and rustdoc to clarify terms per canonical layering model. - Normalized 'frame' to transport codec units, 'packet/envelope' to routing wrappers, 'message' to domain payloads, and 'fragment' to transport subdivisions. - Updated migration guide with vocabulary normalization details and usage guidance. - Modified example types and docs to align with new naming conventions, improving clarity without breaking API. - Completed vocabulary normalization plan and validated with full quality gates. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
Reviewer's GuideNormalizes vocabulary across documentation and rustdoc to align with a canonical architectural layer model (Frame, Packet/Envelope, Message, Fragment) without changing public APIs, and marks the vocabulary normalization execution plan as complete. Class diagram for Packet, Envelope, and PacketParts vocabulary alignmentclassDiagram
class CorrelatableFrame {
<<trait>>
+correlation_id() Option_u64_
+set_correlation_id(cid Option_u64_)
}
class Message {
<<trait>>
}
class Packet {
<<trait>>
+id() u32
+into_parts() PacketParts
+from_parts(parts PacketParts) Self
+is_stream_terminator() bool
}
class PacketParts {
<<struct>>
-id u32
-correlation_id Option_u64_
-payload Vec_u8_
+new(id u32, correlation_id Option_u64_, payload Vec_u8_) PacketParts
+id() u32
+correlation_id() Option_u64_
+into_payload() Vec_u8_
}
class Envelope {
<<struct>>
-id u32
-correlation_id Option_u64_
-payload Vec_u8_
}
class WireframeApp {
+handle_connection()
}
Packet <|.. Envelope
CorrelatableFrame <|.. Envelope
Message <|.. Envelope
Packet o-- PacketParts
WireframeApp --> Envelope
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Summary by CodeRabbit
WalkthroughSummarise documentation vocabulary to canonical layer terms (Frame, Packet/Envelope, Message, Fragment), add a developers' guide and migration notes, and update examples and docs wording; do not change public API signatures or runtime behaviour. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Several places still use dual terms like
Packet / Envelopeorpacket/envelope identifier(e.g., in the glossary tables anddocs/api.md); consider choosing a single canonical term per context and explicitly noting the other as an alias to keep the mental model tighter. - In
docs/developers-guide.md, the allowed aliasfragment frameforFragmentrisks reintroducing confusion around the very specificFrameterm; it may be clearer to dropfragment frameor qualify it more strongly soFrameremains strictly transport-wired.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several places still use dual terms like `Packet / Envelope` or `packet/envelope identifier` (e.g., in the glossary tables and `docs/api.md`); consider choosing a single canonical term per context and explicitly noting the other as an alias to keep the mental model tighter.
- In `docs/developers-guide.md`, the allowed alias `fragment frame` for `Fragment` risks reintroducing confusion around the very specific `Frame` term; it may be clearer to drop `fragment frame` or qualify it more strongly so `Frame` remains strictly transport-wired.
## Individual Comments
### Comment 1
<location> `docs/execplans/vocabulary-normalization.md:102-103` </location>
<code_context>
+- Added a canonical layered glossary to `docs/users-guide.md`.
+- Created `docs/developers-guide.md` to define naming invariants and
+ allowed/disallowed term mixing.
+- Updated `docs/v0-1-0-to-v0-2-0-migration-guide.md` with explicit
+ terminology mapping and a no-additional-public-rename note.
+- Updated terminology wording in public rustdoc and references
+ (`src/app/envelope.rs`, `docs/api.md`, `docs/contents.md`).
</code_context>
<issue_to_address>
**nitpick (typo):** The phrase "a no-additional-public-rename note" reads awkwardly; consider rephrasing for clarity.
The multiple hyphens make this hard to read. Consider something like “a note that no additional public renames were made” or “a no additional public renames note” for clarity.
```suggestion
- Updated `docs/v0-1-0-to-v0-2-0-migration-guide.md` with explicit
- terminology mapping and a note that no additional public renames were made.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/developers-guide.md`:
- Around line 39-43: The three checklist bullets all start with "Confirm"
causing repetitive phrasing; update the three lines to use varied verbs and
structures (e.g., "Ensure the identifier name matches the owning layer," "Verify
rustdoc examples use the same term as the symbol being documented," and
"Document the meaning consistently in docs/users-guide.md and migration notes")
and keep the cross-layer example (`correlation_id`) explicitly labeled as shared
metadata; adjust the three checklist items around rustdoc, docs/users-guide.md,
and identifier naming to follow this varied wording.
In `@src/app/envelope.rs`:
- Around line 147-148: Doc comment above the Envelope type uses the en-US
spelling "deserialised"; update that documentation to the en-GB-oxendict form
"deserialized". Locate the comment string containing "Incoming packets are
deserialised into an `Envelope`..." in src/app/envelope.rs (the doc for the
Envelope type) and replace the single word "deserialised" with "deserialized" so
the documentation conforms to the required spelling standard.
docs/developers-guide.md
Outdated
Comment on lines
39
to
43
| - Confirm the identifier name matches the owning layer. | ||
| - Confirm rustdoc examples use the same term as the symbol being documented. | ||
| - Confirm `docs/users-guide.md` and migration notes describe the same meaning. | ||
| - Confirm cross-layer terms (for example, `correlation_id`) are explicitly | ||
| labelled as shared metadata. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Vary the sentence structure in the checklist.
The static analysis tool flagged three successive bullet points beginning with "Confirm". Rephrase for variety:
✏️ Suggested rewording
Use this checklist before merging API naming changes:
-- Confirm the identifier name matches the owning layer.
-- Confirm rustdoc examples use the same term as the symbol being documented.
-- Confirm `docs/users-guide.md` and migration notes describe the same meaning.
-- Confirm cross-layer terms (for example, `correlation_id`) are explicitly
+- Verify the identifier name matches the owning layer.
+- Check rustdoc examples use the same term as the symbol being documented.
+- Ensure `docs/users-guide.md` and migration notes describe the same meaning.
+- Label cross-layer terms (for example, `correlation_id`) explicitly as
labelled as shared metadata.🧰 Tools
🪛 LanguageTool
[style] ~41-~41: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... term as the symbol being documented. - Confirm docs/users-guide.md and migration not...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~42-~42: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tion notes describe the same meaning. - Confirm cross-layer terms (for example, `correl...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/developers-guide.md` around lines 39 - 43, The three checklist bullets
all start with "Confirm" causing repetitive phrasing; update the three lines to
use varied verbs and structures (e.g., "Ensure the identifier name matches the
owning layer," "Verify rustdoc examples use the same term as the symbol being
documented," and "Document the meaning consistently in docs/users-guide.md and
migration notes") and keep the cross-layer example (`correlation_id`) explicitly
labeled as shared metadata; adjust the three checklist items around rustdoc,
docs/users-guide.md, and identifier naming to follow this varied wording.
…lines - Updated allowed aliases and prohibited mixing table for consistency - Improved checklist wording for clarity and parallel structure - Corrected terminology in docs and comments for uniformity - Fixed typographical error from 'deserialised' to 'deserialized' in envelope.rs documentation Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/envelope.rs`:
- Around line 3-5: Update the doc-comment wording to use en-GB-oxendict "-ize"
spellings: replace "serialisation" with "serialization" and "deserialisation"
with "deserialization" in the comment that references
crate::app::builder::WireframeApp (and any other occurrences in this module),
ensuring the sentence remains grammatically correct after the substitutions.
Comment on lines
3
to
+5
| //! These types decouple serialisation from routing by wrapping raw payloads in | ||
| //! identifiers understood by [`crate::app::builder::WireframeApp`]. This | ||
| //! allows the builder to route frames before full deserialisation. See | ||
| //! allows the builder to route packets before full deserialisation. See |
Contributor
There was a problem hiding this comment.
Use -ize spelling for serialization terms.
Replace "serialisation" and "deserialisation" with "serialization" and
"deserialization" to match en-GB-oxendict spelling.
✏️ Proposed fix
-//! These types decouple serialisation from routing by wrapping raw payloads in
+//! These types decouple serialization from routing by wrapping raw payloads in
-//! allows the builder to route packets before full deserialisation. See
+//! allows the builder to route packets before full deserialization. SeeAs per coding guidelines, Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| //! These types decouple serialisation from routing by wrapping raw payloads in | |
| //! identifiers understood by [`crate::app::builder::WireframeApp`]. This | |
| //! allows the builder to route frames before full deserialisation. See | |
| //! allows the builder to route packets before full deserialisation. See | |
| //! These types decouple serialization from routing by wrapping raw payloads in | |
| //! identifiers understood by [`crate::app::builder::WireframeApp`]. This | |
| //! allows the builder to route packets before full deserialization. See |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/envelope.rs` around lines 3 - 5, Update the doc-comment wording to
use en-GB-oxendict "-ize" spellings: replace "serialisation" with
"serialization" and "deserialisation" with "deserialization" in the comment that
references crate::app::builder::WireframeApp (and any other occurrences in this
module), ensuring the sentence remains grammatically correct after the
substitutions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements vocabulary normalization documentation across guides, updates public-facing wording in docs, and aligns code commentary with the canonical layering model. No public API renames were introduced; changes focus on terminology and documentation.
Changes
Documentation
Code
Migration & Verification
Rationale
Testing and validation
Impact
◳ Generated by DevBoxer ◰
ℹ️ Tag @devboxerhub to ask questions and address PR feedback
📎 Task: https://www.devboxer.com/task/bcb3ccec-3029-4ca5-bb83-ab1f5e6bfd00
Summary by Sourcery
Normalize architectural vocabulary across documentation and packet-related rustdoc without changing public APIs.
Enhancements:
Documentation:
Tests: