[Code Health] MailboxState conflates task phase, init, control and directives in one 13-value enum

[ChaoWao]

Category

Technical Debt (cleanup, refactor)

Component

Host Runtime

Description

MailboxState is a single 13-value enum that carries four unrelated kinds of
thing in one shared-memory word. This is the unfinished half of the
async-preparation pipeline's E2 step: "let the mailbox express only phase, and
store the preparation disposition separately." The disposition half shipped in #1650
(48dc78bf, as a side effect of unrelated work rather than any E2 PR):
MailboxPreparationDisposition (worker_manager.h:84-88) got its own frame
offset, its own validation, and a poison path for out-of-enum values. The
phase half did not.

Today's members, grouped by what they actually are (hit counts across src/ +
python/):

Kind	Members	Uses
Task phase — the only kind E2 wants to keep here	IDLE=0 · TASK_READY=1 · FRAME_STAGED=8 · TASK_LAUNCHED=9 · TASK_DONE=2 · TASK_FAILED=10	—
Init lifecycle	INIT_READY=6 · INIT_FAILED=7	30 · 17
Control channel	CONTROL_REQUEST=4 · CONTROL_DONE=5	12 · 17
Directives written by the peer	SHUTDOWN=3 · ACTIVATE=11 · PREPARE_READY=12	32 · 10 · 8

ACTIVATE is not an observed state at all — it is CAS-written into the word
as a command (worker_manager.cpp:724 does FRAME_STAGED -> ACTIVATE).

Three consequences, none of which a rename fixes:

1. The phase values are interleaved, so "is this a phase?" needs a hand-written
set. They occupy 1, 2, 8, 9, 10, with non-phase values at 3–7, 11, 12. The
resulting predicate is spelled out by hand, e.g. worker.py:2702:

live_states = (_TASK_READY, _PREPARE_READY, _ACTIVATE, _FRAME_STAGED, _TASK_LAUNCHED)

Two directives mixed with three phases in one tuple — exactly the conflation E2
exists to remove. Every such set must be maintained by hand and can silently
disagree with the next one.

2. The TASK_ / FRAME_ prefixes are load-bearing, not noise. The step's
target vocabulary is READY / STAGED / LAUNCHED / DONE / FAILED; the code needs
the prefixes only to disambiguate from INIT_READY and CONTROL_DONE in the
same enum. So renaming before splitting produces a strictly worse state —
READY sitting next to INIT_READY. The rename is a consequence of the
split, not a substitute for it.

3. FRAME_STAGED already means two different things in one header.
MailboxState::FRAME_STAGED = 8 (:77) and
WorkerProgressKind::FRAME_STAGED = 0 (:291). Same identifier, same file,
different values.

The constraint that shapes any fix: this word is a cross-process wire
contract, not an internal enum. It is a volatile int32_t at
MAILBOX_OFF_STATE mutated by mailbox_compare_exchange_state, in shared
memory between a parent and its forked children — the POD boundary in
.claude/rules/codestyle.md §8. And the Python side re-declares all 13
values by hand as module constants (worker.py:310-329), with no generation
or assertion tying them to the C++ enum.

Therefore: renaming enumerators is free (compile-time only); renumbering is
not — an already-running child and a freshly started parent must agree on the
integers. Any fix has to keep the numeric values fixed or ship a synchronized
change on both sides.

Not on the critical path for anything today. Filing it so the scope is on record
rather than being re-estimated as "just a rename" — which is how it was
previously (mis)scoped.

Location

Verified against upstream/main@4739cbf4:

- `src/common/hierarchical/worker_manager.h:60-82` — `MailboxState`, the 13-value enum
- `src/common/hierarchical/worker_manager.h:77` — `MailboxState::FRAME_STAGED = 8`
- `src/common/hierarchical/worker_manager.h:290-294` — `WorkerProgressKind`, whose `FRAME_STAGED = 0` (`:291`) collides by name
- `src/common/hierarchical/worker_manager.h:117` — `MAILBOX_OFF_STATE`, the shared-memory offset of the word
- `src/common/hierarchical/worker_manager.cpp:724` — `ACTIVATE` CAS-written as a directive (`FRAME_STAGED -> ACTIVATE`)
- `python/simpler/worker.py:310-329` — all 13 values re-declared by hand, no single source of truth
- `python/simpler/worker.py:2702` — `live_states`, a hand-written set mixing directives with phases

Proposed Fix

Split by concern while keeping every numeric value unchanged, so nothing on
the wire moves:

• MailboxTaskPhase — IDLE, TASK_READY, FRAME_STAGED, TASK_LAUNCHED,
  TASK_DONE, TASK_FAILED
• MailboxInitState — INIT_READY, INIT_FAILED
• MailboxControlState — CONTROL_REQUEST, CONTROL_DONE
• MailboxDirective — SHUTDOWN, ACTIVATE, PREPARE_READY

They continue to share the one word, with value ranges that do not overlap, so
the split is source-level only and no running pair can disagree.

Then, and only then, drop the now-redundant TASK_ / FRAME_ prefixes, and
rename WorkerProgressKind::FRAME_STAGED or the phase member so the collision
in worker_manager.h goes away.

Independently worth doing and cheap: give the Python constants a generation step
or a start-up assertion against the C++ values, so the two cannot drift
silently. That is the part with real (if latent) risk today — a wrong constant
there is a silent cross-process protocol mismatch, not a compile error.

Priority

Low (no impact today, good to fix eventually)

Related: #1650 (delivered the disposition half) · #1728 and #1739 (E1/E1', the
sibling step, both complete)

[ChaoWao]

调查后不建议做这个拆分，关闭。结论和我在正文里写的提案相反 —— 记在这里，免得下次再推导一遍。

提案不成立的原因

正文说这四类「继续共享这一个字，取值范围不重叠，所以拆分只在源码层」。前半句对，后半句不成立：这四类不是分离使用的，而是在同一个变量、同一个函数、同一次原子操作里混流。

实测（main@0fc77f21）：

证据	位置
一次 CAS 跨两类：FRAME_STAGED(phase) → ACTIVATE(directive)	worker_manager.cpp:733
mailbox_compare_exchange_state(char*, MailboxState, MailboxState) —— 两个形参必须同类型	worker_manager.h:90
同一个 write_mailbox_state 写 CONTROL_REQUEST(control)、IDLE(phase)、SHUTDOWN(directive)	worker_manager.cpp 各处
MailboxState read_mailbox_state() 单一返回类型，调用方在一个 switch 里混判各类	worker_manager.h:469
Python 侧同一个 state_addr 轮流写 _CONTROL_DONE / _INIT_READY / _INIT_FAILED	worker.py:1871, 3071, 4070

规模：MailboxState:: 共 74 处引用，分布在 test_scheduler.cpp(51) · worker_manager.cpp(21) · worker_bind.h(14) 等。

拆成四个类型之后，read_mailbox_state() 只能返回其中一个，而调用方需要判别全部四类。结果只有两条路：到处 static_cast<int32_t> 再比较 —— 等于绕开刚建立的类型区分；或者给 CAS 和读写函数加重载与转换层 —— 净增代码，只为表达一件本来就成立的事。

MailboxState 是个判别联合：13 个互斥取值共用一个 32 位字。这是它本来的形状，不是缺陷。我把「这些成员语义上属于不同类别」读成了「它们应该是不同类型」—— 前者对，后者不跟着成立。

判据其实已经写在代码里

真正该从 state 字里分出去的东西，分出去的方式是给它自己的字，不是给它自己的类型。头文件里两处既有注释就是这么判的：

• worker_manager.h:136 —— acceptance 是 sticky，不是 MailboxState，因为放进 state 字会在子进程于两次父轮询之间到达 TASK_DONE 时丢掉 ACK
• worker_manager.h:151 —— termination 是 control frame 上的 sticky 单向字，不是 MailboxState，因为 MAILBOX_OFF_STATE 有三个写者，任何一个都会覆盖掉 SHUTDOWN

两处的理由都是并发写者会互相覆盖，不是「语义上属于不同类别」。这才是可拆与不可拆的分界线。

Issue 里成立的两半，都已交付

• disposition 拆分 —— 由 Refactor: centralize chip native run ownership #1650 (48dc78bf) 交付。它之所以能拆，正是因为它拿到了自己的 frame 偏移（MAILBOX_OFF_PREPARATION_DISPOSITION），带独立校验和越界 poison 路径；不是因为改了类型
• 漂移守卫 —— 由 Add: check the mailbox wire constants against the C++ enums at import #1765 (0fc77f21) 交付。这是正文里我判断为「唯一有真实（潜在）风险」的那一半，因为两侧声明跨语言、编译器抓不到，错值是合法但不同的状态，会表现为挂起或错路由而不是报错。写这个守卫时当场发现了一处不对称：MailboxPreparationDisposition::NONE C++ 有而 Python 没声明 —— 查证后确认 Python 拒绝把 NONE 当作已发布的 disposition 是对的（父进程认领帧时写它，子进程从不发布），所以补了常量并写明这层区别，而不是放宽检查

还剩什么

正文点到的 worker.py:2702 那个手写 live_states 元组（三个 phase 混两个 directive）确实读着别扭。但那是可读性问题，换成一个具名谓词就够了，取值和类型全不用动 —— 不需要这个 issue 撑着。

WorkerProgressKind::FRAME_STAGED 与 MailboxState::FRAME_STAGED 的重名也仍在，同样是纯改名、零 wire 影响，谁下次动到那个头文件顺手改掉即可。