Conversation
… migration - Introduced ADR 008, 009, and 010 to define architectural decisions for the new zero-copy public byte container, migration rollout strategy, and transport-frame boundary. - Added a detailed zero-copy frame and payload migration roadmap outlining phased implementation, benchmarking, migration plans, and release preparation. - Updated documentation links and inventory notes to incorporate these critical migration strategy documents. This lays the foundation for removing the final Vec<u8> copy in outbound paths and establishes a clear public API, rollout and performance validation strategy for the breaking change to zero-copy payloads. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
WalkthroughSummarise the addition of three ADRs and a detailed zero‑copy migration roadmap; renumber roadmap sections and update many execplans and supporting docs to reference new phase identifiers and local Epic 284 tracking. Report that all modifications are documentation-only. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Conn as ConnectionActor
participant Codec as CodecDriver/Serializer
participant Transport as Transport/FrameEmitter
rect rgba(200,220,255,0.5)
Client->>Conn: send Packet/Envelope (payload = Bytes-compatible)
Conn->>Codec: hand off packet without materialising Vec<u8>
end
rect rgba(200,255,200,0.5)
Codec->>Codec: apply wrap_payload (zero-copy)
alt caller needs mutation
Codec->>Codec: invoke edit-on-demand helper (copy-on-write)
end
Codec->>Transport: emit transport frame
end
rect rgba(255,220,200,0.5)
Transport->>Client: deliver frame/ack
end
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Reviewer's GuideDocuments and wires up the zero-copy frame and payload migration effort by adding ADRs 008–010, introducing a dedicated migration roadmap, and aligning the existing roadmap, exec plans, and inventory docs (including renumbering client-roadmap items from 10–14 to 16–19) to reference and track epic 284 coherently. Flow diagram for the zero-copy frame and payload migration roadmapflowchart TD
A["1 Decision closure and baseline"] --> B["2 Internal zero-copy foundations"]
B --> C["3 Public API migration"]
C --> D["4 Validation ecosystem updates and documentation"]
D --> E["5 Release rollout"]
subgraph Phase1_Decision_Closure
A1["1.1 Approve public byte container and edit-on-demand model<br/>ADR 008"]
A2["1.1 Approve compatibility and rollout policy<br/>ADR 009"]
A3["1.1 Approve actor and codec-driver boundary<br/>ADR 010"]
A4["1.2 Define benchmarks and thresholds"]
A5["1.2 Draft migration guide outline"]
end
subgraph Phase2_Internal_Foundations
B1["2.1 Introduce internal zero-copy payload representation"]
B2["2.1 Convert PacketParts and Envelope storage"]
B3["2.1 Update internal channels and buffers"]
B4["2.2 Implement actor and codec-driver boundary"]
B5["2.2 Remove core Vec<u8> frame bridges"]
B6["2.2 Add allocation and pointer-reuse regressions"]
end
subgraph Phase3_Public_API
C1["3.1 Migrate packet and middleware APIs off Vec<u8>"]
C2["3.1 Preserve edit-on-demand ergonomics"]
C3["3.1 Update server examples and behavioural tests"]
C4["3.2 Update before_send hook and client byte APIs"]
C5["3.2 Update Serializer::serialize contract"]
C6["3.2 Re-evaluate client preamble policy"]
end
subgraph Phase4_Validation_and_Docs
D1["4.1 Update testing helpers and examples to new byte model"]
D2["4.1 Publish migration guide and breaking-change summary"]
D3["4.1 Close or supersede ADRs 008 009 010"]
D4["4.2 Run benchmarks vs thresholds and record results"]
D5["4.2 Run downstream canaries and capture outcomes"]
end
subgraph Phase5_Release
E1["5.1 Finalize versioning plan for breaking release"]
E2["5.1 Publish release notes changelog and upgrade guidance"]
E3["5.1 Capture post-release follow-ups and compatibility helper review"]
end
A --> A1 --> A2 --> A3 --> A4 --> A5
B --> B1 --> B2 --> B3 --> B4 --> B5 --> B6
C --> C1 --> C2 --> C3 --> C4 --> C5 --> C6
D --> D1 --> D2 --> D3 --> D4 --> D5
E --> E1 --> E2 --> E3
File-Level Changes
Assessment against linked issues
Possibly linked issues
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}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Rename multiple execplan docs and their internal references to move roadmap items from the 10 and 11 series to the 16 and 17 series respectively. - Update all corresponding documentation and source comments to reflect the new roadmap item numbering. - Adjust examples, logs, test names, and documentation to maintain consistency with the new item numbers. - Ensure tests and quality gates refer to updated roadmap items. This change aligns roadmap tracking and documentation with the current iteration scheme, preparing for the next phase of development in the 16.x and 17.x series and maintaining internal consistency. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/contents.md`:
- Around line 32-34: Reflow the markdown bullet starting with "Zero-copy frame
and payload migration roadmap" so the list item and its following sentence are
wrapped to 80 columns per the repository rule; edit the bullet text and its
paragraph to split long lines (breaking after the link or logical word
boundaries) so no line exceeds 80 characters while preserving the link
`[Zero-copy frame and payload migration
roadmap](zero-copy-frame-and-payload-migration-roadmap.md)` and the sentence
"Phased plan for migrating from `Vec<u8>`-owned frame APIs to a zero-copy
alternative."
In `@docs/execplans/17-4-2-troubleshooting-section.md`:
- Around line 451-454: The checklist sentence is grammatically broken; revise
the line starting "Only after these pass should implementation:" so the
checklist flows correctly—e.g., change to "Only after these pass should the
implementation team:" or "Only after these pass should the following be
completed:" and ensure the two items read as complete actions (mark `17.4.2` as
done in `docs/roadmap.md` and update the `Progress` and `Outcomes &
Retrospective` sections). Update the surrounding text to match the chosen
phrasing so the closing checklist is clear and unambiguous.
In `@docs/frame-vec-u8-inventory.md`:
- Around line 359-361: The text in docs/frame-vec-u8-inventory.md references
"phases 10 through 14 of `roadmap.md`" which is stale; update the Epic 284
coordination sentence to either use the current phase identifiers from the
renumbered roadmap or point to the dedicated zero-copy roadmap document instead
of hard-coded phase numbers. Edit the sentence that mentions "Epic 284" and
"phases 10 through 14 of `roadmap.md`" to reference the correct phase names/IDs
or replace the link with the authoritative zero-copy roadmap link so the
source-of-truth mapping remains accurate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b794ac95-8965-4128-adba-a88d9e0401b7
📒 Files selected for processing (23)
docs/adr-008-zero-copy-public-byte-container.mddocs/adr-009-vec-u8-migration-rollout.mddocs/adr-010-transport-frame-boundary-for-zero-copy.mddocs/contents.mddocs/execplans/16-2-1-client-message-api.mddocs/execplans/16-2-2-client-decode-and-transport-failures.mddocs/execplans/16-3-1-client-response-stream-and-response-multipacket.mddocs/execplans/16-3-2-exercise-interleaved-queues.mddocs/execplans/16-4-1-client-documentation-and-examples.mddocs/execplans/17-1-1-outgoing-request-middleware-hooks.mddocs/execplans/17-1-2-structured-logging-and-tracing-spans.mddocs/execplans/17-2-1-configurable-connection-pool-preserving-preamble.mddocs/execplans/17-2-2-expose-pool-handle-api.mddocs/execplans/17-3-1-helpers-for-consuming-streaming-responses.mddocs/execplans/17-3-2-in-process-server-and-client-pair-test-harness.mddocs/execplans/17-4-2-troubleshooting-section.mddocs/frame-vec-u8-inventory.mddocs/multi-packet-and-streaming-responses-design.mddocs/roadmap.mddocs/users-guide.mddocs/wireframe-client-design.mddocs/wireframe-testing-crate.mddocs/zero-copy-frame-and-payload-migration-roadmap.md
…p and epic work Add detailed traceability sections to ADR-008, ADR-009, and ADR-010 to reference Epic 284 tasks, roadmap items, and relevant documentation. Update frame-vec-u8-inventory.md coordination notes to reflect roadmap tracking updates for zero-copy migration epic. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/adr-008-zero-copy-public-byte-container.md`:
- Around line 7-10: The ADR's Date line uses a trailing full stop and
inconsistent formatting; update the Date header in
docs/adr-008-zero-copy-public-byte-container.md to exactly "2026-04-12"
(YYYY-MM-DD) with no trailing period so it matches the ADR date syntax and
coding guidelines.
In `@docs/adr-009-vec-u8-migration-rollout.md`:
- Around line 7-10: Update the "Date" field in the ADR header to strictly follow
YYYY-MM-DD by removing the trailing period so the value reads exactly
`2026-04-12`; edit the "Date" line under the "## Date" heading in
docs/adr-009-vec-u8-migration-rollout.md to delete the full stop after the date
to comply with ADR formatting rules.
In `@docs/adr-010-transport-frame-boundary-for-zero-copy.md`:
- Around line 7-10: In the "## Date" section (the line currently containing
"2026-04-12."), remove the trailing period so the date is exactly "2026-04-12"
to match the required YYYY-MM-DD format; update the line that follows the "##
Date" header (the literal date string) accordingly and ensure no other
punctuation remains on that date line.
In `@docs/frame-vec-u8-inventory.md`:
- Around line 359-363: Remove the stray leading space at the start of the
paragraph continuation that begins with "with the combined
[`roadmap.md`](roadmap.md)..." in the paragraph mentioning
"zero-copy-frame-and-payload-migration-roadmap.md" so the line starts flush with
the previous sentence (no leading space) to preserve consistent Markdown
paragraph formatting and satisfy markdownlint/docstyle rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2b86da6a-2666-4d43-946c-ac42e7180216
📒 Files selected for processing (4)
docs/adr-008-zero-copy-public-byte-container.mddocs/adr-009-vec-u8-migration-rollout.mddocs/adr-010-transport-frame-boundary-for-zero-copy.mddocs/frame-vec-u8-inventory.md
Removed unnecessary trailing periods in the date fields of ADRs 008, 009, and 010 markdown files to maintain consistency and formatting correctness. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
docs/adr-009-vec-u8-migration-rollout.md (1)
5-5:⚠️ Potential issue | 🟡 MinorNormalize ADR status token.
Change Line 5 to
Proposedwithout the full stop to match the required ADR
status value format.Triage:
[type:docstyle]
As per coding guidelines: "ADR required sections: Include Status (one of
'Proposed', 'Accepted', 'Superseded', or 'Deprecated'...)".Suggested patch
-Proposed. +Proposed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr-009-vec-u8-migration-rollout.md` at line 5, Update the ADR status token on docs/adr-009-vec-u8-migration-rollout.md so it matches the required canonical values: replace the current "Proposed." text on the Status line with "Proposed" (remove the trailing period) to conform to the ADR required sections format.
🤖 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/adr-008-zero-copy-public-byte-container.md`:
- Line 5: Update the ADR status token on line 5 to exactly "Proposed" with no
trailing punctuation so the Status field is machine-checkable and matches
required tokens; locate the Status header in
docs/adr-008-zero-copy-public-byte-container.md (the ADR's Status line) and
replace the current text with the exact token Proposed.
- Around line 25-26: Rewrite the sentence that currently reads "The default
codec path wants shared, cheap-to-clone bytes to remove the final copy
identified in epic 284." to direct technical language: replace the nonstandard
"wants shared" with a precise verb such as "requires" or "uses", refer to "byte
buffers" or "bytes" explicitly (e.g., "requires shared, cheap-to-clone byte
buffers") and change "remove" to "avoid" or "eliminate" so the line reads
clearly and in en-GB spelling while still referencing the final copy discussed
in epic 284 and the "default codec path".
In `@docs/adr-010-transport-frame-boundary-for-zero-copy.md`:
- Line 5: Change the ADR status token on line 5 to exactly "Proposed" (no
trailing period or other punctuation) so the ADR metadata matches the required
set of statuses; update the status field in
docs/adr-010-transport-frame-boundary-for-zero-copy.md (the ADR Status line) to
the normalized value "Proposed".
- Around line 96-140: Add an explicit consolidation step to the Migration Plan
that declares the codec driver as the sole owner of transport frame emission and
mandates migrating all non-driver wrap_payload call sites into the codec-driver
boundary; specifically mention relocating any existing wrap_payload usages
(e.g., the runtime inbound emission path currently calling wrap_payload) into
the codec driver, update Phase 1 to include a concrete "consolidate non-driver
wrap_payload callers" task, and add a risk/control item that tracks these call
sites (e.g., by name or TODO tags) until they are removed, referencing
ConnectionActor, FrameCodec::wrap_payload, and the inbound emission path so
reviewers can locate and verify the changes.
---
Duplicate comments:
In `@docs/adr-009-vec-u8-migration-rollout.md`:
- Line 5: Update the ADR status token on
docs/adr-009-vec-u8-migration-rollout.md so it matches the required canonical
values: replace the current "Proposed." text on the Status line with "Proposed"
(remove the trailing period) to conform to the ADR required sections format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d733114b-0a36-4dc6-9fbe-c232ed1df95c
📒 Files selected for processing (3)
docs/adr-008-zero-copy-public-byte-container.mddocs/adr-009-vec-u8-migration-rollout.mddocs/adr-010-transport-frame-boundary-for-zero-copy.md
…me boundary - Corrected proposal statuses from "Proposed." to "Proposed" in ADR-008, ADR-009, and ADR-010. - Improved wording for byte buffer usage in ADR-008 to reflect shared, cheap-to-clone buffers. - In ADR-010, added clarification on consolidating FrameCodec::wrap_payload call sites into codec driver and tracking remaining call sites to enforce codec driver ownership of transport frame emission. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…proved formatting Reformatted the coordination notes section for better readability by explicitly stating that Epic 284 is tracked locally in a dedicated roadmap file. Improved sentence structure to distinctly separate details about the roadmap and the zero-copy migration phases. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/adr-008-zero-copy-public-byte-container.md`:
- Around line 32-50: In the ADR "Traceability" section add an explicit entry for
zero-copy-frame-and-payload-migration-roadmap.md so the ADR links directly to
the zero-copy migration roadmap (keeping ADR-to-roadmap navigation one hop and
auditable); update the list alongside the existing frame-vec-u8-inventory.md and
roadmap.md references so the new roadmap filename is clearly declared in that
Traceability block.
In `@docs/adr-009-vec-u8-migration-rollout.md`:
- Around line 28-48: Update the "Traceability" section to include a direct
reference to the zero-copy roadmap file by adding
`docs/zero-copy-frame-and-payload-migration-roadmap.md` to the listed tracking
artifacts so the rollout traceability explicitly couples migration policy and
roadmap milestones; modify the Traceability bulleted list (inside the same
"Traceability" section and near the existing roadmap.md entries and roadmap
items `10.1.2`–`14.2.1`) to include the new file and ensure the phrasing makes
it clear this roadmap is part of the migration publish phase described in "Phase
1".
In `@docs/adr-010-transport-frame-boundary-for-zero-copy.md`:
- Around line 31-48: Update the Traceability section of this ADR to add a
dedicated bullet linking the "Zero-Copy Frame and Payload Migration Roadmap"
document (the zero-copy migration roadmap) and explicitly state the relationship
between this ADR's boundary decision and the rollout workstream; keep the
wording that preserves navigation between the boundary decision and rollout
tasks and add the implementation note to monitor all non-driver
FrameCodec::wrap_payload call sites during review so the codec driver becomes
the sole owner of transport frame emission (reference FrameCodec::wrap_payload
and this ADR title).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b6648492-713a-4405-86cb-b549644999f1
📒 Files selected for processing (3)
docs/adr-008-zero-copy-public-byte-container.mddocs/adr-009-vec-u8-migration-rollout.mddocs/adr-010-transport-frame-boundary-for-zero-copy.md
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Added references to zero-copy migration roadmap document in ADRs 008, 009, and 010 to track migration phases, rollout policy, and boundary decisions related to Epic 284. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/adr-008-zero-copy-public-byte-container.md`:
- Around line 44-53: The ADR references outdated roadmap IDs (10.1.1, 11.1.1,
12.1.1, 12.1.2, 12.2.1, 12.2.3, 13.1.2); update those tokens to their renumbered
equivalents in the 16.x/17.x plan family so traceability links remain auditable
and clickable. Edit the listed bullets in
docs/adr-008-zero-copy-public-byte-container.md replacing each `10.x`–`13.x`
item reference with the correct new roadmap IDs (ensure the specific tokens
`10.1.1`, `11.1.1`, `12.1.1`, `12.1.2`, `12.2.1`, `12.2.3`, `13.1.2` are matched
and replaced), and verify any associated hyperlinks or cross-references still
resolve after the change.
In `@docs/adr-009-vec-u8-migration-rollout.md`:
- Around line 37-38: The traceability sentence in
docs/adr-009-vec-u8-migration-rollout.md uses the incorrect noun phrase "later
publish milestones"; update that phrase to "later publication milestones"
(maintaining en-GB-oxendict spelling) so the sentence reads "...including the
Phase 1 decision closure and the later publication milestones." Ensure you
replace the exact phrase in the traceability paragraph.
- Around line 40-51: Update the listed roadmap item IDs to the current 16.x/17.x
numbering so the ADR maps to the live migration plan; specifically replace the
old IDs `10.1.2`, `10.2.3`, `12.1.2`, `12.2.2`, `13.1.1`, `13.1.2`, `14.1.1`,
`14.1.2`, `14.1.3`, and `14.2.1` in the Vec<u8> migration rollout section with
their corresponding new 16.x/17.x IDs from the renumbered roadmap, verify each
new ID matches the roadmap title/description, and update any adjacent text that
assumes the old numbering so all references (the listed roadmap item
identifiers) are consistent and accurate.
In `@docs/adr-010-transport-frame-boundary-for-zero-copy.md`:
- Around line 45-53: Update the outdated roadmap ID references in the ADR text
by replacing the tokens `10.1.3`, `11.1.2`, `11.2.1`, `11.2.2`, `11.2.3`, and
`14.2.1` with the current active roadmap IDs so reviewers can trace
implementation checkpoints; locate the paragraph listing these roadmap items in
the ADR and substitute each old ID with the corresponding up-to-date roadmap
entry (verify against the current roadmap source), keeping the same phrasing and
bullets (`actor and codec-driver boundary`, `remove default-path Vec<u8> copy`,
`implement boundary and move Vec<u8>-specific runtime traits`,
`allocation/pointer-reuse regressions`, `review runtime-only compatibility
bridges`) to preserve intent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 857d4a98-08da-4601-ac05-36a53f6bb3f9
📒 Files selected for processing (3)
docs/adr-008-zero-copy-public-byte-container.mddocs/adr-009-vec-u8-migration-rollout.mddocs/adr-010-transport-frame-boundary-for-zero-copy.md
Corrected minor typos in docs/adr-009-vec-u8-migration-rollout.md and docs/execplans/17-4-2-troubleshooting-section.md for clarity and correctness. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Owner
Author
|
@coderabbitai Please suggest a fix for the following lint violation and provide an AI coding agent prompt for the fix: |
This comment was marked as resolved.
This comment was marked as resolved.
…uence validation logic - Extracted fill_prefix_buffer helper in frame conversion for clarity and reusability. - Simplified bytes_to_u64 by delegating buffer filling to the new helper. - Refactored MessageSeries sequence validation into handle_untracked_first_sequence and advance_tracked_sequence for clearer separation of concerns. - Improved error handling and explicit sequence flow in message assembly. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
changed the title
Owner
Author
|
@coderabbitai Please suggest a fix for the following lint violation and provide an AI coding agent prompt for the fix: |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
…offset Replaced the match arms and conditional checks in fill_prefix_buffer with a single calculated offset and direct slice copy. This reduces code complexity and removes redundant debug assertions, improving readability and maintainability while preserving functionality. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/frame/conversion.rs`:
- Around line 37-47: Change fill_prefix_buffer to derive size from prefix.len()
and enforce the precondition with a debug_assert! instead of leaving a
debug_assert!(false) path or lint suppression: in function
fill_prefix_buffer(prefix: &[u8], endianness: Endianness) compute let size =
prefix.len(); add debug_assert!(matches!(size, 1 | 2 | 4 | 8), "prefix must be
1, 2, 4, or 8 bytes"); compute offset as before and call buf[offset..offset +
size].copy_from_slice(prefix); return buf; remove the #[expect(...)] suppression
and the unused size parameter, and update the call site (the caller that
currently passes size, referenced at the previous line 76) to call
fill_prefix_buffer(prefix, endianness) with the new signature.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: abe557a7-3110-40ea-b762-6d3ae7c8b923
📒 Files selected for processing (2)
src/frame/conversion.rssrc/message_assembler/series.rs
Owner
Author
|
@coderabbitai Please suggest a fix for the following concern and provide an AI coding agent prompt for the fix: |
This comment was marked as resolved.
This comment was marked as resolved.
Removed the explicit size parameter from fill_prefix_buffer and instead infer size from the prefix slice length. Added debug assertions to validate prefix length and buffer slice correctness. Updated callers accordingly to streamline and clarify slice handling, improving code safety and readability. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…g verification logic Extracted the inline budget-related abort verification from assert_connection_aborted into a dedicated verify_abort_outcome method. This helps improve test code clarity and reuse by validating error kind, draining late payloads, and checking no payloads are observed before abort. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…on architecture Add detailed documentation of message continuation ordering in `src/message_assembler/series.rs`. Describe the public API `validate_and_advance_sequence()` and its two private helpers, `handle_untracked_first_sequence()` and `advance_tracked_sequence()`, clarifying their roles in managing state transitions and validation rules. This improves developer understanding of the sequence tracking design and aids future maintenance and refactoring. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
Owner
Author
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. ❌ Failed checks (2 warnings)Check name | Status | Explanation | Resolution -- | -- | -- | -- Developer Documentation | |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 9495
No further work is needed for this check. |
Owner
Author
|
@coderabbitai re:
Please provide an accurate title and summary that I can paste in |
This comment was marked as resolved.
This comment was marked as resolved.
leynos
changed the title
#284)
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
What this PR does
Documentation & architecture
docs/adr-008-zero-copy-public-byte-container.md): adoptsBytes-compatible shared storage as the canonical public payloadrepresentation, with an edit-on-demand copy-on-write mutation helper for
middleware and client-hook surfaces.
docs/adr-009-vec-u8-migration-rollout.md): selects astaged breaking-release rollout (Option C) for the
Vec<u8>→ zero-copymigration, with finite bounded compatibility helpers.
docs/adr-010-transport-frame-boundary-for-zero-copy.md):establishes
ConnectionActoras packet-oriented and the codec-driver as thesole owner of transport-frame emission, removing
Vec<u8>frame bridges fromthe core runtime once validated.
docs/zero-copy-frame-and-payload-migration-roadmap.md: a phasedprogramme (decision closure & baseline → internal foundations → public API
migration → validation → release rollout) with task checklists, success
measures, and explicit ADR cross-references.
docs/frame-vec-u8-inventory.mdanddocs/contents.mdto referencethe new ADRs and roadmap.
10.x/11.x/12.x/13.x/14.xfamily to16.x/17.x/18.x/19.xto accommodate the newzero-copy phases (10–14) ahead of the formal-verification and client-library
phases.
Code refactors (no public API changes)
src/frame/conversion.rs: extractsfill_prefix_buffer(prefix, size, endianness) -> [u8; 8]— computes a single byte offset and performs one flatcopy_from_slice, replacing the twoif let Some(dst) = buf.get_mut(...)clusters that triggered a
bumpy_road_functionDylint violation inbytes_to_u64.src/message_assembler/series.rs: extractshandle_untracked_first_sequenceand
advance_tracked_sequencefromvalidate_and_advance_sequence, reducingfive nested conditional clusters to two flat single-expression
matcharms.Traceability
Each ADR carries explicit item-level back-references to
roadmap.mdand thezero-copy migration roadmap:
Testing & validation
were modified.
Task