Feat: add launch-acceptance flight fence

[Crane-Liu]

Summary

Separate the point at which a run's dispatches have been launched from the point
at which they complete, so a later submit() can build its graph while the
previous run is still executing on the device.

• count a per-run launch-acceptance fence, raised by the dispatch's group size and lowered once per endpoint dispatch
• publish acceptance on A2A3 onboard after both the AICore and AICPU launches succeed, into a sticky mailbox word — mailbox ownership still stays with the endpoint until TASK_DONE
• elect acceptance separately from completion on RunHandle, so a thread parked in wait() does not turn every acceptance drain into a completion drain
• keep remote, SUB and A5 on the conservative completion fallback
• K=1 throughout: one mailbox frame, no generation-safe frame reuse, no slot lease

Dependency

Stacked on #1539 (A0b, code-image-safe AICore stream generations). Merge after #1539.

#1466 is merged, so the run-resource work is already in the base. The branch
still carries #1539's commit below the acceptance commit; once #1539 lands this
PR rebases down to the single acceptance change.

Why acceptance is a sticky word, not a MailboxState

A state word carrying TASK_ACCEPTED loses the ACK whenever the child reaches
TASK_DONE between two parent polls — precisely the short-task case the fence
exists to pipeline. The loss is silent: the fence falls back to completion and
the pipeline engages or not depending on a race.

The child sets MAILBOX_OFF_ACCEPTED before TASK_DONE; the parent clears it
only when publishing the next TASK_READY, and reads it after the state, so
observing TASK_DONE implies the word is visible. MailboxState::TASK_ACCEPTED
is gone and MAILBOX_ARGS_CAPACITY drops by 8 on both sides of the wire.

Correctness properties this carries

Property	Why it is load-bearing
The accept path never throws	It advances from a WorkerThread, where an escaping exception ends the process. An unknown run or slot is ignored; a count mismatch is recorded as the run's error, the way decrement_run_tasks already does; the post-dispatch fallback cannot propagate out of the thread.
The fence advances even when the dispatch failed	Otherwise a waiter on wait_run_accepted blocks forever.
The decrement runs under completion_mu	It is the mutex a waiter evaluates its predicate on. Without it the notify lands between that check and the block and is lost.
Acceptance is elected separately from completion	worker.run() is submit().wait(), so a thread parked on the previous handle's completion is the normal case.
A released run reads as accepted	A waiter that races the run's release returns instead of raising.
WorkerManager::start takes on_accept with no default	An omitted callback leaves pending_accepts non-zero forever, which is a hang, not a degraded mode.

Scope

An acceptance bridge, not multi-run admission. No dual frames, no generation-safe
frame reuse, no K=2 slot lease, no prepared FIFO. For a dependent DAG, acceptance
can land close to completion.

Validation

Against cdcbb71c.

• changed-file pre-commit: passed
• C++ unit tests: 62/62
• Python unit tests: 837 passed, 2 skipped
• a2a3 onboard, the exact CI command (examples tests/st minus the two SDMA demos, 4 devices under task-submit): 118 passed, 1 skipped, 0 failed — the mailbox layout moved, so this is the check that matters

Mutation-checked, each against its own regression:

Property	Test	Mutation that fails it
acceptance readable after TASK_DONE	AcceptanceIsReadableAfterTaskDone	production reads the state word instead of the sticky one
accept path never throws	AcceptanceFenceNeverThrowsOutOfAWorkerThread	get_run / slot_state restored on the accept path
released run reads as accepted	AcceptanceWaitOnAReleasedRunReturns	get_run restored in wait_run_accepted
acceptance not queued behind completion	test_acceptance_wait_does_not_block_completion_waiter	acceptance drain waits on _wait_in_progress

Two gaps stated rather than papered over:

• The lost-wakeup fix (decrement under completion_mu) has no regression test. The window is the few instructions between the waiter's predicate check and its block, with the mutex held throughout; 3000 forced attempts never hit it. A test that passes on the defect is worse than none, so it was removed. The fix rests on the structural argument: mutate the state a predicate reads under the mutex that predicate is evaluated on — the discipline finish_run_if_ready already follows.
• AcceptanceIsReadableAfterTaskDone does not force the parent to skip a poll between the child's two writes; the parent is already spinning and nothing in the test can stop it. It pins the property that makes that interleaving harmless. A genuinely forced ordering needs a poll seam in LocalMailboxEndpoint, which was judged a worse trade than naming the test honestly.

ut was red on AppleClang only — arithmetic on a pointer to void in the test
mock, which GCC accepts as an extension. Fixed; verified with
-Werror=pointer-arith as a local clang proxy.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change introduces an explicit run-acceptance boundary between dispatch and completion. Acceptance is tracked per run, published by supported device paths, propagated through worker callbacks, exposed through bindings, and used to gate subsequent Python submissions independently of cleanup.

Changes

Run acceptance lifecycle

Layer / File(s)	Summary
Orchestrator acceptance accounting src/common/hierarchical/orchestrator.*, src/common/hierarchical/types.h, tests/ut/cpp/hierarchical/test_orchestrator.cpp	Runs track pending acceptance obligations and expose wait_run_accepted, run_accepted, and mark_task_accepted APIs with coverage for dispatched and failed runs.
Worker dispatch acceptance propagation src/common/hierarchical/worker_manager.*, src/common/hierarchical/worker.cpp, tests/ut/cpp/hierarchical/test_scheduler.cpp, docs/worker-manager.md	Worker endpoints propagate acceptance callbacks, LocalMailboxEndpoint observes TASK_ACCEPTED before TASK_DONE, and worker startup wiring forwards acceptance notifications.
Runtime acceptance publication src/common/platform/onboard/host/*, src/common/worker/*, src/a2a3/platform/onboard/host/device_runner.cpp, python/bindings/task_interface.cpp	The runtime binds and atomically publishes an acceptance state after onboard kernels are enqueued, while ChipWorker and bindings forward optional acceptance parameters.
Python submission coordination python/simpler/worker.py, python/simpler/orchestrator.py, python/bindings/worker_bind.h, tests/ut/py/test_worker/test_host_worker.py, docs/orchestrator.md, docs/task-flow.md	RunHandle separates acceptance waits from completion waits, later submissions wait for acceptance, and Python wrappers and lifecycle documentation describe the new boundary.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PythonWorker
  participant Orchestrator
  participant WorkerManager
  participant DeviceRunner
  PythonWorker->>Orchestrator: submit run and register acceptance obligations
  WorkerManager->>DeviceRunner: dispatch task
  DeviceRunner->>DeviceRunner: enqueue kernels
  DeviceRunner->>WorkerManager: publish TASK_ACCEPTED
  WorkerManager->>Orchestrator: mark_task_accepted
  PythonWorker->>Orchestrator: wait for run acceptance
  Orchestrator-->>PythonWorker: acceptance boundary reached

Loading

Possibly related PRs

• hw-native-sys/simpler#1459: Introduces the run lifecycle mechanics extended here with acceptance tracking.
• hw-native-sys/simpler#1462: Changes the onboard DeviceRunner launch/reap path where acceptance is now published.
• hw-native-sys/simpler#1466: Provides the run-handle and per-run resource model extended with acceptance waiting.

Poem

A rabbit watched kernels hop in a row,
“Accepted!” twinkled before they could go.
Runs kept their cleanup, each snug in its nest,
While the next DAG began without waiting to rest.
Ears up for TASK_ACCEPTED—what a quest!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% 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
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: adding a launch-acceptance fence.
Description check	✅ Passed	The description clearly matches the changeset and explains the new acceptance fence, callbacks, and fallback behavior.

---

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.}

[Crane-Liu]

Rewrote this PR on current main (f44c715c) to make launch acceptance a first-class boundary without weakening completion ownership.

Key changes:

• Added a per-run acceptance counter in the orchestrator. A grouped run becomes accepted only after every member has crossed its launch boundary.
• Added a distinct TASK_ACCEPTED mailbox signal. The local endpoint observes it while retaining the existing tight-poll child-liveness checks.
• Kept acceptance wait and completion wait independently synchronized in RunHandle, including the native-run lifetime race when both waits overlap.
• Added completion-time fallback for backends that do not publish an earlier boundary, and terminal fallback for failed/poisoned runs.
• Wired A2/A3 onboard acceptance after both AICore and AICPU work have been enqueued, before reap/sync.
• Updated orchestrator, worker-manager, and task-flow documentation.

Validation for commit c2e19ed975a105cacf8528ea8a6476238cfc2e3f:

• Remote Python UT: 833 passed, 6 skipped.
• Remote native non-hardware CTest: 62/62 passed.
• Full pre-commit diff gate passed, including clang-tidy, cpplint, markdownlint, Ruff, and pyright.
• A2/A3 architecture precheck passed.
• Real two-device A2/A3 validation passed: task_20260727_054905_353545413752, devices 5/7, test_multi_chip_dispatch.py passed.

This rewrite intentionally does not add or alter the GitHub simulation workflow; hardware validation remains the evidence for this stage.

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@docs/worker-manager.md`:
- Around line 175-181: Update LocalMailboxEndpoint::run(...) and its callers to
accept and forward the acceptance hook consistently with run_with_accept(...).
In the dispatch polling loop, track whether TASK_ACCEPTED has already been
observed and invoke on_accept() only on the first observation per dispatch,
while preserving TASK_DONE as the loop termination condition.

In `@tests/ut/cpp/hierarchical/test_scheduler.cpp`:
- Around line 384-401: In the test around the caller thread and child lifecycle,
replace fatal ASSERT_TRUE/ASSERT_EQ checks executed after creating caller with
non-fatal expectations, ensuring execution always reaches child.complete() and
caller.join(). Preserve the existing timeout and success assertions while
guaranteeing caller is joined even when an expectation fails.

🪄 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 Plus

Run ID: afbe5c14-f8c5-419b-9945-e6d9f510056b

📥 Commits

Reviewing files that changed from the base of the PR and between f44c715 and c2e19ed.

📒 Files selected for processing (23)

• docs/orchestrator.md
• docs/task-flow.md
• docs/worker-manager.md
• python/bindings/task_interface.cpp
• python/bindings/worker_bind.h
• python/simpler/orchestrator.py
• python/simpler/worker.py
• src/a2a3/platform/onboard/host/device_runner.cpp
• src/common/hierarchical/orchestrator.cpp
• src/common/hierarchical/orchestrator.h
• src/common/hierarchical/types.h
• src/common/hierarchical/worker.cpp
• src/common/hierarchical/worker_manager.cpp
• src/common/hierarchical/worker_manager.h
• src/common/platform/onboard/host/c_api_shared.cpp
• src/common/platform/onboard/host/device_runner_base.cpp
• src/common/platform/onboard/host/device_runner_base.h
• src/common/worker/chip_worker.cpp
• src/common/worker/chip_worker.h
• src/common/worker/pto_runtime_c_api.h
• tests/ut/cpp/hierarchical/test_orchestrator.cpp
• tests/ut/cpp/hierarchical/test_scheduler.cpp
• tests/ut/py/test_worker/test_host_worker.py

[Crane-Liu]

Stack update: this PR is now rebased onto #1539 (A0b: code-image-safe AICore stream generations), matching the intended dependency order A0b -> A1.

Combined head: 8023d9140e68cf79abf3ad18144c38781e91b24f

Validation on the combined stack:

• pre-commit: all hooks passed
• Python acceptance/worker UT: 83 passed
• focused C++ orchestrator/scheduler UT: 3 passed
• a2a3 architecture precheck: task_20260727_062539_392909523438 (exit 0)
• a2a3 one-device stream generation/reuse ST: task_20260727_062713_39419017530 (3 passed)
• a2a3 two-device multi-chip acceptance ST: task_20260727_062713_39419029943 (1 passed)

The acceptance commit remains a single commit above #1539. Once #1539 merges, this PR contracts to the acceptance-flight change only.

[ChaoWao]

Revision: acceptance is a sticky mailbox word, not a MailboxState

Pushed d9d81e5d, rebased onto cdcbb71c.

The transient-state race is real, and the PR admitted it

TASK_ACCEPTED lived in the state word, so TASK_DONE overwrote it whenever the child finished between two parent polls. The old comment said as much — "A very short task may advance straight to TASK_DONE between polls; WorkerThread's completion fallback then satisfies the accepted fence conservatively." That fallback is correct for endpoints with no ACK at all, but using it to paper over a lost ACK means the pipeline engages or not depending on a race. A pipeline whose engagement is nondeterministic is not a pipeline.

Fix

A dedicated sticky word, carved out of the args region so the error message keeps the tail:

static constexpr int32_t MAILBOX_TASK_ACCEPTED = 1;
static constexpr ptrdiff_t MAILBOX_OFF_ACCEPTED = MAILBOX_OFF_ERROR_MSG - 8;

• the child sets it after both launches, before TASK_DONE;
• the parent clears it when it publishes the next TASK_READY, so a previous task's ACK can never satisfy this one's fence;
• the parent reads it after the state, so observing TASK_DONE implies the word is visible — the ordering that makes a fast task unable to lose its ACK.

MailboxState::TASK_ACCEPTED is gone; MAILBOX_ARGS_CAPACITY drops by 8 on both sides of the wire.

Deterministic test

AcceptanceSurvivesATaskThatCompletesBeforeTheParentPolls — the child publishes the ACK and TASK_DONE back to back with no parent poll in between, and the test asserts the accept callback still fired.

Mutation-checked by regressing only the production read back to the state word (the mock keeps writing the sticky word, as the real child does): both this test and the pre-existing LocalMailboxPublishesAcceptanceBeforeCompletion fail. My first attempt at this mutation also changed the mock, which made it pass — worth stating, because that version proved nothing.

Doc mismatch

docs/worker-manager.md showed on_accept = {}. The header deliberately has no default: an omitted callback leaves pending_accepts non-zero forever, which is a hang, not a degraded mode. The doc now matches, and its poll snippet shows the sticky read.

Validation

• C++ unit: 62/62
• Python unit: 837 passed, 2 skipped
• a2a3 onboard, the exact CI command on 4 devices: 118 passed, 1 skipped, 0 failed — the mailbox layout moved, so this was the check that mattered
• changed-file pre-commit: passed

One process note: the first onboard attempt aborted at collection with _task_interface was built from 7eb46cb9, but this source tree is at d9d81e5d. That is #1523's guard doing its job — I had built before amending the commit. Rebuilt and re-ran.

Still carried, unchanged

The acceptance fence's lost-wakeup fix (decrementing under completion_mu) has no regression test. I wrote one and could not make it fail on the unfixed code — the window is the few instructions between the waiter's predicate check and its block, with the mutex held throughout — so I removed it rather than ship a test that passes on the defect. That fix rests on the structural argument: mutate the state a predicate reads under the mutex that predicate is evaluated on, which is the discipline finish_run_if_ready already follows.

[ChaoWao]

All three addressed — pushed a89fb11a

1. void* arithmetic (must fix) — this was the red ut

mailbox_ptr() + MAILBOX_OFF_ACCEPTED in the mock. GCC accepts it as an extension, AppleClang does not, which is exactly the failure I had been unable to read — the run kept reporting "still in progress" so gh would not serve the log:

tests/ut/cpp/hierarchical/test_scheduler.cpp:121:66: error: arithmetic on a pointer to void

Fixed as suggested — cast to char * first. Also confirmed nothing else in the C++ tree does the same.

Since there is no clang on this box, I used -Werror=pointer-arith as a proxy and checked it actually bites: re-introducing the bug gives error: pointer of type ‘void *’ used in arithmetic [-Werror=pointer-arith], and the fixed code builds clean under it. Worth noting for anyone else developing here — the default GCC build will not catch this class at all.

2. Stack note in the PR body

Corrected to "Stacked on #1539. Merge after #1539", with a line saying #1466 is already in the base and this branch still carries #1539's commit below the acceptance commit until #1539 lands.

3. The back-to-back test overstated what it forces

Right, and this is the more useful of the two. The parent is already spinning by the time the mock writes, so nothing in the test prevents a poll landing between the two writes — the name claimed a scenario the test does not create.

I did not add a poll gate: the poll lives inside LocalMailboxEndpoint::run_with_accept, so gating it means a test-only seam in production code, which seemed a worse trade than naming the test honestly. Renamed to AcceptanceIsReadableAfterTaskDone, and the comment now says plainly what it pins and what it does not:

This does not force the parent to skip a poll between the two writes: the parent is already spinning by then and nothing here can stop it. What it pins is the property that makes that interleaving harmless — acceptance is readable after TASK_DONE, so losing a poll cannot lose the ACK.

The mock still publishes acceptance only into the sticky word, never into the state, so an endpoint that looks in the state word observes none at all — that part is deterministic and is what the mutation check exercises.

Say the word if you would rather have the seam and a genuinely forced ordering; it is a small addition to the endpoint.

Validation

C++ 62/62, Python 837 passed / 2 skipped, pre-commit clean. Both CodeRabbit threads replied to and resolved.