refactor(comm): cleanups deferred from the transport×role split

[junjzhang]

Tracking issue for small items found while refactoring the transfer IR (SELF_COPY/SHADOW → transport ∈ {P2P, BROADCAST, LOCAL, NONE} + orthogonal is_source/is_target). They were intentionally left out of that PR to keep it scoped to the role/transport split and the self-copy bucket-finalize fix. Collecting them here to do in one pass.

1. execute() should own self.work

Transferable.execute() returns the dist.Work handle, and the only two callers immediately assign it back:

• src/etha/comm/execution.py:90 — chunk.work = chunk.execute()
• src/etha/comm/ir.py:218 — self.work = self.execute() (in Bucket.launch)

Move the assignment into execute() so it sets self.work directly; call sites become chunk.execute() / self.execute(). Trade-off: execute() mutates self instead of being side-effect-free — but since neither caller uses the return value for anything else, the "pure" property buys nothing. Net simplification.

2. Overlapping source/target meshes (a rank in both) are not correctly handled

Latent — production uses disjoint meshes, so none of these fire. The current single-tensor local (self-copy) chunk encodes "read and write the same tensor", which only holds when the source and target tensors coincide (as in the unit test).

• (a) Wrong-tensor read. The local chunk is built with tensor=target_tensor but reads src_slice (a source-grid slice) — src/etha/comm/get_chunks.py (the Transport.LOCAL branch, ~L100-110). When a rank holds distinct source and target tensors, this reads target_tensor[src_slice] instead of source_tensor[src_slice]. Proper fix: split the single tensor field into read_tensor/write_tensor (then is_source reads read_tensor[src_slice], is_target writes write_tensor[dst_slice], self-copy uses both).
• (b) Skipped Partial reduce. The local chunk doesn't carry source_partial_groups, so a rank that is both a Partial source and a target won't reduce before its local write.
• (c) P2P-to-self hang. A route whose sole dst is the source rank emits a P2P is_source chunk that isends to itself with no matching irecv (self-delivery goes through the LOCAL chunk).

All three disappear if overlapping meshes are either (i) properly supported — read/write tensor split + the self-cell routed only through LOCAL, never P2P, with the reduce applied — or (ii) explicitly rejected at map-build time. Decision needed: support vs reject.

Regression anchor: tests/test_self_copy_bucket.py covers the single-tensor LOCAL path. A distinct-tensor test would expose (a).

---

More to be appended as review continues.

[junjzhang]

Codex review of #106 surfaced a concrete instance of the overlapping-mesh problem (item 2 above):

tensor=None chunk crashes during bucketization (overlapping meshes only).
The agent's send-direction call builds chunks with target_tensor=None (agent.py:663) and the recv-direction call with source_tensor=None (agent.py:675). If a route's dst (or src) lands on the local rank — i.e. the source and target meshes share a rank — routes_to_chunks constructs a chunk whose tensor is None:

• a LOCAL chunk (no transfer_dtype passed) crashes in Chunk.__post_init__ on self.tensor.dtype;
• a recv chunk crashes later in _build_bucket on first_chunk.tensor.device (get_buckets.py).

This is latent: with disjoint meshes a send route's dsts are all remote, so dst_rank != rank and no such chunk is built. The now-removed execute_chunk_simple had if chunk.tensor is None: continue, which silently skipped these — i.e. it would have dropped the local delivery and produced a wrong result anyway. So the old path was silently-wrong and the new path is loud (crash) for the same unsupported config.

Reinforces the support-vs-reject decision for overlapping meshes: either properly support (read/write tensor split so a local rank's self-delivery reads source and writes target), or reject overlapping meshes explicitly at map-build time with a clear error.

[junjzhang]

Status update after #106:

• Item 1 (execute() should own self.work) — resolved by refactor(comm): Bucket is the sole transfer unit; Chunk is pure data #106. Bucket.execute was inlined into launch(), which now sets self.work directly; Chunk dropped both execute() and the work field. There is no longer a duplicated X.work = X.execute().

Still open:

• Item 2 — overlapping source/target meshes (the support-vs-reject decision, incl. the tensor=None bucketization crash noted above).
• Item 3 — Chunk.dst_idx dead field (still set in get_chunks but never read; trivial removal).

Keeping this issue open for 2 and 3.