Repository navigation
feat: add remote l3 wire codec - #1008
Merged
jvjhfhg merged 5 commits intoJun 11, 2026
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughImplements a binary wire codec for the Remote L3 protocol across Python and C++, defining constants, enums, and dataclasses for frame/message structures; providing frame codecs, socket transport helpers, and message-specific encoders/decoders for hello, task arguments, control, callable registration, and remote buffer lifecycle operations; and adding ordered command sequencing control with comprehensive validation and error handling. ChangesRemote L3 Wire Protocol
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the Remote L3 wire protocol codec in both Python and C++, establishing frame structures, serialization/deserialization logic, and validation rules for tasks, control messages, and remote buffer exports/imports. It also adds corresponding unit tests to verify the round-trip encoding and decoding. The review feedback highlights a potential bug in the Python implementation where slicing encoded UTF-8 error messages in encode_completion and encode_control_reply can split multi-byte characters and cause decoding failures; the reviewer suggests raising a ValueError instead of silently truncating to match the C++ behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
This was referenced Jun 8, 2026
jvjhfhg
pushed a commit
that referenced
this pull request
Jun 9, 2026
PR 1 of the remote L3 worker split stack (extracted from #866). Adds the design-contract documents and pre-split audit artifacts, and serves as the overview / source of truth for the implementation PRs that follow. Lands docs only — no runtime behavior; later PRs must be reviewed against this contract (or the contract updated). Split stack: - #1006 docs: remote L3 worker contract (this PR — umbrella/overview) - #1007 feat: remote import callable descriptors - #1008 feat: remote L3 wire codec - #1009 feat: remote endpoint eligibility scheduling - #1010 feat: remote L3 endpoint facade - #1011 feat: remote L3 Python session runtime Documents: - Remote L3 worker model and host/session/worker responsibilities. - Protocol surface: frames, TASK/COMPLETION/CONTROL/HEALTH, reserved fields. - Buffer and transport ownership, import, release, and future transport work. - Scheduler, orchestrator, worker-manager, task-flow, and hierarchical runtime doc updates so remote endpoints fit the existing runtime model. - Split-and-audit plan plus audit artifacts: compliance matrix, split map, risk register, verification checklist, and future-work list. Contract boundaries established for the stack: - Separates required behavior from explicitly unsupported behavior; reserved and unsupported protocol paths must fail explicitly. - Records the review boundaries for callable identity, wire codec, scheduler eligibility, C++ remote endpoint, and Python session runtime. Future work (out of this split unless a later dedicated PR changes the contract): A2 RoCE / A3 HCCS / A5 UB HCOMM hardware profiles, and Remote CommDomain support.
jvjhfhg
force-pushed
the
remote-l3-split-remote-wire
branch
from
June 9, 2026 03:32
83c595d to
9bc56e2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/common/hierarchical/types.h (1)
133-148: 💤 Low valueConsider documenting local vs wire-transmitted fields.
RemoteBufferHandlemixes fields that are transmitted over the wire (e.g.,buffer_id,remote_addr) with local runtime state (released,live_slot_refs). While the PR objectives specify explicit encode/decode without raw POD copying (so this is handled correctly), a brief comment would clarify which fields are local-only to prevent future maintenance confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/common/hierarchical/types.h` around lines 133 - 148, Add a brief clarifying comment above the RemoteBufferHandle struct that distinguishes which members are serialized/transmitted over the wire (e.g., buffer_id, generation, import_id, address_space, nbytes, offset, remote_addr, rkey_or_token, ub_ldst_va, access_flags) versus which are local/runtime-only state (e.g., released, live_slot_refs, endpoint_id/owner_endpoint_id if those are runtime-only in your design), and mention that explicit encode/decode routines handle the wire format rather than POD memcpy; reference the struct name RemoteBufferHandle and the specific field names in the comment so future maintainers know which fields must be included in encode/decode and which are local-only.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/simpler/remote_l3_protocol.py`:
- Around line 469-493: The non-HOST_INLINE validation is inconsistent: Python's
_validate_desc enforces buffer_id != 0 and generation != 0 but C++
validate_desc_against_inline_payload does not, causing wire incompatibility;
update the C++ validate_desc_against_inline_payload (the non-HOST_INLINE branch)
to also require desc.buffer_id != 0 and desc.generation != 0 and return/throw
the same kind of validation error when either is zero so the C++ acceptance
rules match the Python _validate_desc checks.
---
Nitpick comments:
In `@src/common/hierarchical/types.h`:
- Around line 133-148: Add a brief clarifying comment above the
RemoteBufferHandle struct that distinguishes which members are
serialized/transmitted over the wire (e.g., buffer_id, generation, import_id,
address_space, nbytes, offset, remote_addr, rkey_or_token, ub_ldst_va,
access_flags) versus which are local/runtime-only state (e.g., released,
live_slot_refs, endpoint_id/owner_endpoint_id if those are runtime-only in your
design), and mention that explicit encode/decode routines handle the wire format
rather than POD memcpy; reference the struct name RemoteBufferHandle and the
specific field names in the comment so future maintainers know which fields must
be included in encode/decode and which are local-only.
🪄 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: CHILL
Plan: Pro
Run ID: 03482917-e0d2-4360-b3f6-3d9d04781a7b
📒 Files selected for processing (7)
python/simpler/remote_l3_protocol.pysrc/common/hierarchical/remote_wire.cppsrc/common/hierarchical/remote_wire.hsrc/common/hierarchical/types.htests/ut/cpp/CMakeLists.txttests/ut/cpp/hierarchical/test_remote_wire.cpptests/ut/py/test_remote_l3_protocol.py
puddingfjz
force-pushed
the
remote-l3-split-remote-wire
branch
from
June 9, 2026 04:22
670eb11 to
b62522a
Compare
jvjhfhg
approved these changes
Jun 11, 2026
jvjhfhg
pushed a commit
that referenced
this pull request
Jun 25, 2026
Completes the L4 -> remote-L3 path: a Python session runtime that runs a remote L3 worker out-of-process and executes parent-submitted tasks against it over the SLR3 wire protocol (#1008) and the C++ endpoint facade (#1010). Wired end-to-end under transport="sim" (same-host: TCP control lane + POSIX shm data plane); HCOMM/RoCE/HCCS/UB remain Pending and are rejected at the manifest gate. Remote callable dispatch. RemoteCallable("module:qualname") registers a PYTHON_IMPORT target in a two-tier registry: the parent-facing REMOTE_TASK_DISPATCHER (re-imported fresh in the remote process, so closures never cross -- everything the remote function needs arrives via args) and the remote-internal INNER_L3_WORKER used by the embedded Worker(level=3) for its own children. Callable crosses by identity (import-path + digest), never by pickled closure. Remote memory API (RDMA-shaped, owner/imported handle model). remote_malloc / free / copy_to / copy_from / export / import / release_import on Worker, plus the public types RemoteAddressSpace, RemoteBufferHandle, RemoteBufferExport, and RemoteTensorRef. A buffer's owner is always the child worker that allocated it; the parent only holds handles. Owner allocations are REMOTE_DEVICE and consumable only on the owner; export converts a slice to a remotely-addressable REMOTE_WINDOW/UB_LDST grant (opaque, immutable, access- scoped) that import mints into an importer-local mapping. copy_to/copy_from are owner-only host<->buffer transfers driven by the parent; cross-worker sharing is zero-copy via import (importer maps the owner's segment), not a copy. Data crosses by reference. TaskArgs.add_tensor accepts RemoteTensorRef; Tensor.data stays zero on the wire and a sidecar descriptor carries owner identity (owner_worker_id, buffer_id, generation, offset, address_space, rkey). The descriptor is worker-agnostic -- the receiving session resolves owner identity to a local address against its own registry (own buffer table, or imported mapping). Dependency keys use owner identity, so producer and consumer tasks on different endpoints touching the same physical buffer serialize through the same TensorKey (cross-endpoint RAW ordering). HOST_INLINE is the small-payload bypass: no owner, no import, rides the TASK frame by value. Lifecycle. Deferred-free reference counting: remote_free on an owner handle defers physical free until no live slot refs and no live imported mappings remain; remote_release_import tears down only one importer mapping. The session daemon forks before threads start, owns physical shm and imported mappings, and validates the bootstrap manifest (remote_worker_level must be 3, transport must be sim). Tests: Python unit + sim daemon end-to-end coverage for callable identity, the task-interface remote types, and the remote-buffer lifecycle.
doraemonmj
pushed a commit
to doraemonmj/simpler_wc
that referenced
this pull request
Jul 1, 2026
Completes the L4 -> remote-L3 path: a Python session runtime that runs a remote L3 worker out-of-process and executes parent-submitted tasks against it over the SLR3 wire protocol (hw-native-sys#1008) and the C++ endpoint facade (hw-native-sys#1010). Wired end-to-end under transport="sim" (same-host: TCP control lane + POSIX shm data plane); HCOMM/RoCE/HCCS/UB remain Pending and are rejected at the manifest gate. Remote callable dispatch. RemoteCallable("module:qualname") registers a PYTHON_IMPORT target in a two-tier registry: the parent-facing REMOTE_TASK_DISPATCHER (re-imported fresh in the remote process, so closures never cross -- everything the remote function needs arrives via args) and the remote-internal INNER_L3_WORKER used by the embedded Worker(level=3) for its own children. Callable crosses by identity (import-path + digest), never by pickled closure. Remote memory API (RDMA-shaped, owner/imported handle model). remote_malloc / free / copy_to / copy_from / export / import / release_import on Worker, plus the public types RemoteAddressSpace, RemoteBufferHandle, RemoteBufferExport, and RemoteTensorRef. A buffer's owner is always the child worker that allocated it; the parent only holds handles. Owner allocations are REMOTE_DEVICE and consumable only on the owner; export converts a slice to a remotely-addressable REMOTE_WINDOW/UB_LDST grant (opaque, immutable, access- scoped) that import mints into an importer-local mapping. copy_to/copy_from are owner-only host<->buffer transfers driven by the parent; cross-worker sharing is zero-copy via import (importer maps the owner's segment), not a copy. Data crosses by reference. TaskArgs.add_tensor accepts RemoteTensorRef; Tensor.data stays zero on the wire and a sidecar descriptor carries owner identity (owner_worker_id, buffer_id, generation, offset, address_space, rkey). The descriptor is worker-agnostic -- the receiving session resolves owner identity to a local address against its own registry (own buffer table, or imported mapping). Dependency keys use owner identity, so producer and consumer tasks on different endpoints touching the same physical buffer serialize through the same TensorKey (cross-endpoint RAW ordering). HOST_INLINE is the small-payload bypass: no owner, no import, rides the TASK frame by value. Lifecycle. Deferred-free reference counting: remote_free on an owner handle defers physical free until no live slot refs and no live imported mappings remain; remote_release_import tears down only one importer mapping. The session daemon forks before threads start, owns physical shm and imported mappings, and validates the bootstrap manifest (remote_worker_level must be 3, transport must be sim). Tests: Python unit + sim daemon end-to-end coverage for callable identity, the task-interface remote types, and the remote-buffer lifecycle.
This was referenced Jul 24, 2026
Closed
nalinaly
pushed a commit
to nalinaly/simpler
that referenced
this pull request
Jul 31, 2026
PR 1 of the remote L3 worker split stack (extracted from hw-native-sys#866). Adds the design-contract documents and pre-split audit artifacts, and serves as the overview / source of truth for the implementation PRs that follow. Lands docs only — no runtime behavior; later PRs must be reviewed against this contract (or the contract updated). Split stack: - hw-native-sys#1006 docs: remote L3 worker contract (this PR — umbrella/overview) - hw-native-sys#1007 feat: remote import callable descriptors - hw-native-sys#1008 feat: remote L3 wire codec - hw-native-sys#1009 feat: remote endpoint eligibility scheduling - hw-native-sys#1010 feat: remote L3 endpoint facade - hw-native-sys#1011 feat: remote L3 Python session runtime Documents: - Remote L3 worker model and host/session/worker responsibilities. - Protocol surface: frames, TASK/COMPLETION/CONTROL/HEALTH, reserved fields. - Buffer and transport ownership, import, release, and future transport work. - Scheduler, orchestrator, worker-manager, task-flow, and hierarchical runtime doc updates so remote endpoints fit the existing runtime model. - Split-and-audit plan plus audit artifacts: compliance matrix, split map, risk register, verification checklist, and future-work list. Contract boundaries established for the stack: - Separates required behavior from explicitly unsupported behavior; reserved and unsupported protocol paths must fail explicitly. - Records the review boundaries for callable identity, wire codec, scheduler eligibility, C++ remote endpoint, and Python session runtime. Future work (out of this split unless a later dedicated PR changes the contract): A2 RoCE / A3 HCCS / A5 UB HCOMM hardware profiles, and Remote CommDomain support.
nalinaly
pushed a commit
to nalinaly/simpler
that referenced
this pull request
Jul 31, 2026
PR 3 of the remote L3 worker split stack. Adds the versioned cross-host frame protocol (codec only) that hw-native-sys#1010 (C++ endpoint) and hw-native-sys#1011 (Python session) build on. Transport-neutral and self-contained: encode/decode + struct definitions + tests; no sockets, endpoint, or session loop yet. Adds: - C++ `remote_wire.{h,cpp}`: SLR3 FrameHeader + payload codecs for HELLO, TASK, COMPLETION, CONTROL, CONTROL_REPLY, and remote buffer export/import/release. Canonical little-endian field encoding — no raw POD memcpy of CallConfig/ContinuousTensor onto the wire. - Python `remote_l3_protocol.py`: byte-symmetric codec plus thin read_frame/send_frame socket helpers. - `types.h`: wire-serialized Remote* structs (RemoteAddressSpace, RemoteBufferHandle/Export, RemoteTensorDesc/Ref/Sidecar, RemoteTaskArgsSidecar).
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
2 participants
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
This is PR 3 of the remote L3 worker split stack. It adds the transport-neutral
remote L3 wire codec in both C++ and Python.
This PR defines how remote L3 messages are encoded, decoded, validated, and
round-tripped. It does not add scheduling, a live remote endpoint, or a Python
session runtime.
Relationship to PR #866 and split stack
This PR is part of the split stack extracted from #866.
Stack:
The top of the stack was verified to match the corrected
remote-l3-worker-designbranch content from #866, excluding empty CI retrigger commits.
Stack position
remote-l3-split-callable-identityremote-l3-split-remote-wireremote-l3-split-callable-identityremote-l3-split-scheduler-eligibilityScope
This PR adds or updates:
remote_wireencode/decode helpers.remote_l3_protocolencode/decode helpers.Requirements covered
This PR covers the protocol requirements from the remote L3 contract:
contract.
sequence matching.
enable_scope_stats.staged blobs are explicitly rejected in this split.
Why this is separate
The scheduler and endpoint PRs need a stable protocol layer, but they should not
hide codec review inside higher-level runtime behavior. Keeping the codec in
its own PR makes bounds checks, reserved-field handling, and C++/Python parity
straightforward to review.
Important exclusions
This PR intentionally does not add:
RemoteL3Endpoint.Those are handled by later PRs.
Verification
Targeted verification already run during the split:
PYTHON_SERIALIZEDandSTAGED_BLOBpayload coverage: passed.Reviewer notes
Please focus on wire compatibility, strict validation, and whether all reserved
fields/future extensions are either rejected now or clearly documented as
future work.