Add: carry an endpoint deployment on Global CommDomain members

[sunkaixuan2018]

Summary

A GlobalDomainMember was four tree positions — node and local worker ids, global device rank, domain rank — none of which said what the endpoint is deployed on. The backing a member's window is carved from is derived from its deployment (comm_endpoints._backend_kind_for_provider), so without the field a member's semantics are welded to one tree level, and a HOST_CPU participant or the two deployment views of one chip have no spelling at all.

Placeholder-only, as the issue specifies: the dimension is added, exactly one value is accepted, behavior does not change.

• GlobalDomainMember gains an EndpointDeployment, imported from comm_endpoints rather than redefined. It is normalized at construction because EndpointDeployment subclasses str — an un-normalized value would be found by the wire-id table yet rejected by validate_member_table's identity check, so the encoder and the validator would disagree about the same value.
• The member wire carries it as a u32 ordinal from an explicit table, mirroring GLOBAL_DOMAIN_PROFILE_IDS. Enum declaration order is not a wire contract, so the mapping is stated rather than derived.
• validate_member_table admits only DEVICE_AICORE, and says so in terms of backing rather than level: no host-mappable window backing is implemented, because halHostRegister refuses a VMM VA.
• Worker resolves the deployment from the registered L3 node rather than the caller, so allocate_global_domain(members=[(l3_id, l2_index), ...]) keeps its signature and no example changes.
• docs/comm-domain.md states the invariant instead of leaving a reader to infer member semantics from the tuple shape.

Why a second version constant

GLOBAL_DOMAIN_VERSION guarded four unrelated layouts at once: the L4↔L3 commands, the LOCAL_* L3→L2 mailbox structs, the release/copy commands, and the descriptor — which the platform backend stamps with COMM_GLOBAL_DOMAIN_VERSION in src/common/platform_comm/comm.h and Python checks on decode. Advancing that constant for a Python-only field addition would have failed every allocation at PREPARE with global domain descriptor version mismatch, unless the C++ macro moved in lockstep and every runtime was rebuilt.

The L4↔L3 commands now carry their own GLOBAL_DOMAIN_COMMAND_VERSION = 2, so a stale peer reports a version mismatch instead of a confusing truncated-wire error, and the backend-paired constant stays at 1. No C++ changes — src/ is untouched, and the member table never reaches the device (the L3→L2 LOCAL_PREPARE_REQUEST carries only domain_rank and rank_count).

Testing

Validated on an Ascend910 V1 host; onboard runs went through task-submit.

• Unit tests — pytest tests/ut -m "not requires_hardware": 1486 passed
• Cross-process end-to-end — 8 cases spawning real simpler.remote_l3_worker daemons over TCP, plus the mixed local/remote and MPI-group paths
• Simulation tests — full tests/st --platform a2a3sim: 64 passed, 0 failed
• Hardware tests — pytest tests/ut -m requires_hardware --platform a2a3: 6/6 passed, including test_two_rank_comm_lifecycle and test_two_rank_allocate_release_round_trip
• Hardware scene tests — tests/st/worker --platform a2a3: 12/12 PASS, including TestAsyncNotifyDemo, which declares no sim variant
• Lint — ruff check/format, pyright, check-headers, check-english-only, markdownlint

New unit cases cover the deployment round-tripping on the wire, construction-time normalization, rejection of a non-DEVICE_AICORE member, rejection of an unknown deployment id arriving on the wire, and the command version being independent of the backend-stamped descriptor version.

Scope

Deliberately excluded, per the issue: no EndpointId / session_id / worker_path, no member selector, no capability computation. The two follow-ups that share this root cause — CommBufferSpec being an offset slice of one window rather than an independent Buffer, and comm_profile being one node-level string — are breaking changes that need the endpoint registry to be load-bearing first.

Fixes #1836

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 19808e5a-569e-4fa8-878c-c90d80bbe46c

📥 Commits

Reviewing files that changed from the base of the PR and between f4ed104 and 3d4ba91.

📒 Files selected for processing (4)

• docs/comm-domain.md
• python/simpler/global_comm_domain.py
• python/simpler/worker.py
• tests/ut/py/test_global_comm_domain.py

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Global CommDomain members now include EndpointDeployment metadata. Worker topology propagates deployment values into members and topology hashes. Member encoding carries deployment IDs. Command codecs use an independent command version. Validation and tests cover supported deployments and wire errors.

Changes

Global CommDomain deployment metadata

Layer / File(s)	Summary
Member contract and command wire format python/simpler/global_comm_domain.py	GlobalDomainMember stores and normalizes EndpointDeployment. Validation accepts only DEVICE_AICORE. Member wire data includes a deployment ID. Command codecs use GLOBAL_DOMAIN_COMMAND_VERSION.
Worker deployment propagation python/simpler/worker.py	_GlobalNodeRuntime records deployment metadata. Worker allocation copies deployment values into members and topology-hash inputs.
Validation tests and contract documentation tests/ut/py/test_global_comm_domain.py, docs/comm-domain.md	Tests cover normalization, round trips, invalid deployment IDs, unsupported host backing, and independent command versions. Documentation describes endpoint deployment and device-backed windows.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 3d4ba

This localized change adds endpoint deployment metadata without changing behavior, and the reported validation is passing. No actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant Worker
  participant GlobalDomainMember
  participant MemberTable
  participant GlobalDomainWireCodec
  Worker->>GlobalDomainMember: assign registered EndpointDeployment
  GlobalDomainMember->>MemberTable: validate deployment
  MemberTable-->>GlobalDomainMember: accept DEVICE_AICORE
  GlobalDomainMember->>GlobalDomainWireCodec: encode deployment ID
  GlobalDomainWireCodec-->>GlobalDomainMember: decode deployment ID

Loading

Poem

A rabbit checks the device-bound trail,
With deployment tucked into every detail.
The wire learns one more ID,
While command versions stand free.
DEVICE_AICORE passes the gate—
Clean topology seals its fate.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.52% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: adding endpoint deployment to Global CommDomain members.
Description check	✅ Passed	The description directly explains the deployment field, wire format, validation, worker behavior, versioning, testing, and scope.
Linked Issues check	✅ Passed	The changes satisfy issue #1836 by adding the shared deployment enum, wire support, validation, worker resolution, and documentation.
Out of Scope Changes check	✅ Passed	The changes remain within the linked issue scope; versioning, tests, and documentation support the deployment field addition.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[ChaoWao]

Reviewed at 3d4ba915 (merge-base f4ed1045, 2 behind main). CI 18/1-skip green, and I reproduced tests/ut/py locally: 1531 passed, 13 skipped.

Before anything else: the premise this PR implements was overturned four days ago, and that is my fault, not yours. #1836's body still says "add a deployment to members" because I never pushed the correction. Our internal notes have recorded since 2026-08-14 that the issue body is half-wrong and that corrected wording was pending my action — and you did the honest thing, which is implement the issue as written. The design ruling landed after I filed it. So please read what follows as "the target moved", not "you misread the target".

What the ruling says, and why I think it holds

The conclusion was that the dimension does not belong on members. Three reasons; I re-verified each on this branch rather than quoting:

① members is the rank table, not a participant list. Adding a field that can only ever hold one value documents nothing and invites a wrong inference. On this branch that is literally true — I traced every write:

• _GlobalNodeRuntime.deployment defaults to DEVICE_AICORE (worker.py:910)
• _GlobalNodeRuntime(...) is constructed at exactly one site (:5048) and does not pass deployment; deployment= appears exactly once in the whole 10k-line file, at :9574, which is GlobalDomainMember(deployment=node.deployment) — i.e. it forwards the default
• validate_member_table (global_comm_domain.py:297) rejects anything that is not DEVICE_AICORE

So the field is set to a constant, validated to be that constant, encoded, decoded, and — this is the part that matters — never read to make a decision. Its only consumers in the repo are the four sites that maintain it: normalize (:90), validate (:297), encode (:333), decode (:356).

② The _backend_kind_for_provider citation doesn't reach this path. This is the strongest argument in the PR body and the one I most wanted to be true, because if the deployment selected the backing, it would be load-bearing. It does do that — for the region planner. But the Global CommDomain path never asks:

grep -n "_backend_kind_for_provider|plan_region|RegionAccessService|BackendResolver" global_comm_domain.py
→ (no hits)

The window is carved by the platform backend via comm_global_domain_prepare, not by the planner. So the sentence "the backing a member's window is carved from is derived from its deployment" is true of comm_endpoints, and not true of the code this PR changes. That gap is subtle and I don't think it was careless — it's exactly the kind of thing that only shows up when you go looking for the consumer.

③ What's actually missing is an attachment axis. #1770 already landed the right shape: MemberAttachmentPlan (comm_endpoints.py:540) carries member + role + adapter_kind + adapter_profile. A host participant is a CONSUMER with an owner-delegated or host-VMM copy adapter — it does not occupy a rank, it attaches to somebody else's window. The domain wire carries members and no attachments; that is the gap. A HOST_CPU participant still has no spelling after this PR, because the validator correctly refuses to let it into the rank table — which is the ruling's point restated.

Authority is .docs/cpu-npu-shared-memory/domain-member-vs-attachment.md (internal, not in-repo). Its reason ① also retires the second motivation I put in #1836: DEVICE_AICORE vs DEVICE_AICPU needs no distinction here, because the one place that actually evaluates capability — DefaultRegionAccessService._evaluate_host_vmm_copy (comm_endpoints.py:648) — groups the two device values together (if query.provider.deployment not in (DEVICE_AICORE, DEVICE_AICPU), message "host VMM copy requires a device provider"). I verified that line. So the three-value enum collapses to two wherever a decision is made.

The version split is genuinely good and should survive

Independent of all of the above, splitting GLOBAL_DOMAIN_COMMAND_VERSION out of GLOBAL_DOMAIN_VERSION is a real improvement and I'd like it kept even if the deployment field goes. I verified the boundary is drawn correctly:

• GLOBAL_DOMAIN_VERSION = 1 still guards the descriptor and the LOCAL_* L3→L2 mailbox structs (worker.py:2549, :2612, :2662, :2731, :9173), and stays paired with COMM_GLOBAL_DOMAIN_VERSION 1U (comm.h:40) — I confirmed the C++ side is untouched at 1
• GLOBAL_DOMAIN_COMMAND_VERSION = 2 guards only the L4↔L3 commands, where Python owns both ends (8 sites, all in global_comm_domain.py)

Your reasoning in the body is right and worth preserving in a comment: one constant was guarding four unrelated layouts, and bumping it for a Python-only change would have failed every allocation at PREPARE with a descriptor-version mismatch unless the C++ macro moved in lockstep. That was a trap and you disarmed it.

Two more things I want to credit specifically:

• The normalization bug is a real find. EndpointDeployment subclasses str, so an un-normalized value would be matched by the wire-id dict (which compares by value) yet fail validate_member_table's is not identity check — encoder and validator disagreeing about the same input. Normalizing in __post_init__ is the right fix, and the test that pins it (..._normalizes_its_deployment_to_the_enum_member) states the reasoning in its docstring.
• Explicit wire-id table rather than enum ordinal. Correct — declaration order is not a wire contract, and saying so out loud is what stops the next person from IntEnum-ing it.

The test coverage is also better than the change strictly needed: round-trip, normalization, non-device rejection, unknown-wire-id rejection, and command-version independence.

What I suggest

1. Keep the version split, the GLOBAL_DOMAIN_COMMAND_VERSION test, and the docs paragraph's framing that a member is an endpoint rather than a tree position.
2. Drop the deployment field, its wire id table, its validator branch, and the three deployment-specific tests — they pin a constant.
3. Retarget to the attachment axis: a DomainAttachment record parallel to members, reusing Add: materialize planned comm endpoint regions #1770's frozen role / adapter_kind / adapter_profile and the existing AddressSpace, no new enum. First version can carry host attachments only.

⚠️ But do not start (3) on my word alone. The attachment work has a second prerequisite that neither #1836 nor the ruling's first draft accounted for: _prepare_global_domain_node stores each card's window base in _GlobalNodeDomainState.local_window_bases, and that base is a device VA a host cannot dereference. Adding vocabulary without the access primitive gets you a base address nobody can use. The access primitive is a separate platform-layer item (macro-selected ld/st vs rtMemcpy). I'd rather scope that properly with you than hand you another issue that turns out to be aimed at the wrong thing.

I will correct #1836 — retitle it to the attachment axis, drop the overturned motivation, and rewrite the proposed fix — before anyone else picks it up.

Verdict

Needs discussion, on the premise rather than the code. If we only had the version split and the normalization insight, this would be a clean approve.

I'm sorry for the wasted round. The specific process failure was mine: our notes flagged #1836's body as publicly wrong and pending correction, and I left it sitting there where you could reasonably act on it.

[ChaoWao]

I've corrected #1836 — retitled to the attachment axis, dropped the overturned motivation, rewrote the proposed fix, and added a changelog comment explaining the delta. Before doing that I re-verified the design ruling's three reasons against upstream/main@3cfd894f rather than carrying them over, because you deserve a decision made against today's code and not a four-day-old note. All three still hold:

	Verified on 3cfd894f
Device engines are not distinguished here	_evaluate_host_vmm_copy (comm_endpoints.py:648) still groups DEVICE_AICORE and DEVICE_AICPU — "host VMM copy requires a device provider"
members is a rank table	windowsIn/windowsOut are fixed ctypes.c_uint64 * 64 (task_interface.py:907-908); domain_rank load-bearing in 17 places; a CPU cannot be an HCCL rank
The gap is an attachment axis	MemberAttachmentPlan exists (comm_endpoints.py:540) with AdapterProfile.HOST_VMM_COPY / HOST_SVM_MAP already named; attachment in global_comm_domain.py = 0 hits

I also went looking for a reason to keep the field, and specifically checked whether removing it would cost a version bump — it would not. GLOBAL_DOMAIN_COMMAND_VERSION is compared with strict equality at every decode entry (:387, :493, :563, :612), no negotiation, and every node comes from one launch of the same build. So there is no version skew to manage in either direction, and "we already bumped to 2, may as well keep it" doesn't hold.

Two ways forward — your call, and I'd take either

A — Narrow this PR. Keep the version split, the normalization fix, and the docs framing that a member is an endpoint rather than a tree position. Drop the deployment field, GLOBAL_DOMAIN_DEPLOYMENT_IDS, the validator branch, and the three deployment-specific tests. Retitle (it currently says "carry an endpoint deployment"), and change Fixes #1836 to a plain reference so the attachment work keeps its anchor.

B — Split into two. Land the version split as its own PR; close the deployment half.

I lean B. The version split is independently valuable and I'd approve it on sight — it disarms a real trap (one constant guarding four unrelated layouts, where a Python-only bump would have failed every allocation at PREPARE with a descriptor-version mismatch). It shouldn't have to wait on a premise argument it has nothing to do with. And as its own PR the commit message can say what it fixes, instead of being a subsection of a change that got retargeted.

Either way, please move Fixes #1836 — if this merges as-is it auto-closes an issue whose actual content is now the attachment axis, and that requirement would lose its only anchor while remaining unsolved.

If you want the follow-up

The attachment work is described in the rewritten #1836, but read the prerequisites section before starting: it has two prerequisites, not one. local_window_bases holds a device VA that a host process cannot dereference — on main that dict has exactly two hits, the declaration and the write, and zero readers. Adding attachment vocabulary without a host access primitive gets you a base address nobody can use, and no host-mappable window backing exists yet (halHostRegister refuses a VMM VA). I'd rather scope that with you than hand you another issue aimed slightly off.

Nothing about the quality of this PR is in question. 1531 unit tests, sim, real hardware through task-submit, and two findings that were yours rather than the issue's — the version-constant trap and the str-subclass normalization hazard, both of which I'd have missed. The premise moved; the work didn't fail.

[sunkaixuan2018]

Closing in favour of the split you suggested (option B), and thank you for the review — the three reasons hold and I verified each on the branch rather than taking them on trust.

The _backend_kind_for_provider citation in my PR body was wrong, and it was the load-bearing claim. I grepped global_comm_domain.py for the planner symbols and got 0 hits, exactly as you did. I had read _backend_kind_for_provider while working out why a host member needs a different backing, and then wrote it up as though the Global CommDomain path consulted it. It does not — the window is carved by comm_global_domain_prepare in the platform backend. That was a claim about the endpoint model presented as a claim about the code I was changing, and it should not have survived my own review of the PR body.

Your ① is also right in a way I should have caught before opening: _GlobalNodeRuntime(...) is constructed at one site that never passes deployment, so the field only ever forwards its default, and nothing reads it to make a decision. A field with one possible value and no consumer documents nothing.

I've since seen that #1879 landed the attachment axis, so the follow-up is already covered — good, and it makes the case for retargeting rather than narrowing even clearer.

The version split is now #1885, rebased onto 55556ba0f. Two notes on what changed from the version you reviewed here:

• Both constants are 2, not 1 and 2. You pointed out that removing the field costs no version bump, and Add: carry Global CommDomain attachment metadata #1879 has since moved the shared constant to 2 in lockstep with comm.h. So the split changes no wire layout at all — it only separates the two namespaces.
• The pairing is now an executable assertion. The new PR parses COMM_GLOBAL_DOMAIN_VERSION out of comm.h and asserts it equals GLOBAL_DOMAIN_VERSION. I verified it fails when only the Python side moves. Add: carry Global CommDomain attachment metadata #1879 got that lockstep right by hand; this makes the next one fail at the edit instead of at PREPARE.

One thing I deliberately left out of #1885, in case you disagree: the LOCAL_* L3→L2 mailbox structs still ride GLOBAL_DOMAIN_VERSION. They are Python-owned at both ends like the commands are, so in principle they want a third constant. I kept the two-way boundary you had already verified rather than widening the change, and the comment states the residue instead of hiding it. Happy to split it three ways if you'd rather.

No follow-up needed on the attachment work from me — but thank you for the prerequisites writeup on local_window_bases. I confirmed it has zero readers on main, which is the kind of thing that would have cost a day to discover the hard way.