Refactor: separate A2/A3 device launch and reap

[Crane-Liu]

Summary

• split the A2/A3 onboard DeviceRunner::run() kernel submission boundary into launch_run()
• move stream synchronization, device-error recovery, profiling export, and timing collection into reap_run()
• keep run() strictly synchronous as launch_run() followed immediately by reap_run()
• correct the run() step list, which named the AICPU kernel before the AICore kernel while the launch has submitted AICore first
• do not add ACK states, resource banks, background reaping, or execution overlap

This is behavior-equivalent groundwork: the effective statement sequence, every
error return code, and every recovery action are unchanged — including the
collector teardown on the sync-failure path and print_handshake_results() on
the success path only. reap_run() deliberately takes no arguments and reads
only members, so a later change can defer it without re-splitting its inputs.

Series context

Part of the worker async-preparation series, following #1459 (per-run identity
and completion fence) and #1460 (Worker.submit and run handles), both merged.
Asynchronous reaping, resource banks and device overlap come later.

Validation

• changed-file pre-commit hooks: passed
• A2/A3 onboard libhost_runtime.so rebuilt for both host_build_graph and
  tensormap_and_ringbuffer — the real compile check, since this code is
  onboard-only
• C++ unit tests: 60/60 passed
• Python unit tests: 821 passed, 2 skipped

Neither unit-test suite exercises the onboard DeviceRunner path, so the
onboard CI job is the meaningful check. The A2/A3 hardware runs quoted in
earlier revisions were against a commit that is no longer in this branch's
history and have not been re-run.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR replaces drain-based orchestration with per-run lifecycle APIs, introduces asynchronous Python RunHandle completion, scopes tensor tracking and errors by RunId, updates deferred resource release to run fences, and separates device launch from reap/teardown phases.

Changes

Run lifecycle and completion fencing

Layer / File(s)	Summary
Run state and run-scoped bookkeeping src/common/hierarchical/{types,orchestrator,tensormap,ring}.*	Adds RunId and RunState, per-run task/error tracking, run-scoped TensorMap keys, and guarded run release.
Scheduler failure reporting and worker ownership src/common/hierarchical/{scheduler,worker,worker_manager}.*	Routes task failures through Orchestrator callbacks and removes WorkerManager-owned error state.
Python RunHandle submission and cleanup python/simpler/{worker,orchestrator}.py, python/bindings/worker_bind.h	Adds Worker.submit() and RunHandle, replaces _drain/_clear_error, and coordinates wait, cleanup, domain release, and close.
Lifecycle documentation and validation docs/*, python/simpler/task_interface.py, tests/ut/*	Updates completion-fence semantics and tests run isolation, failure propagation, TensorMap behavior, waiting, and close races.

Device runner phase separation

Layer / File(s)	Summary
Launch and reap helpers src/a2a3/platform/onboard/host/device_runner.*	Extracts kernel submission into launch_run and synchronization, recovery, teardown, and dependency output into reap_run.

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

Possibly related issues

• [Bug] sync_start drain can deadlock when generation-less ack/election state is reused across retry attempts #1455 — Both changes update drain/completion synchronization semantics around run-fence behavior.

Possibly related PRs

• hw-native-sys/simpler#1379 — Its concurrent Worker.run batching depends on lifecycle behavior replaced here.
• hw-native-sys/simpler#1398 — Both modify Python Worker close and lifecycle handling.
• hw-native-sys/simpler#1459 — It overlaps directly on per-run Orchestrator lifecycle, TensorMap namespacing, and removal of drain/error clearing.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Worker
  participant RunHandle
  participant Orchestrator
  participant Scheduler
  Caller->>Worker: submit(callable)
  Worker->>Orchestrator: begin_run()
  Worker->>Orchestrator: close_run_submission(run_id)
  Scheduler->>Orchestrator: report_task_error(slot, message)
  Caller->>RunHandle: wait()
  RunHandle->>Orchestrator: wait_run(run_id)
  RunHandle->>Worker: finalize run cleanup
  Worker->>Orchestrator: release_run(run_id)

Loading

Poem

A rabbit submits with a hop and a cheer,
A run fence says, “Completion is near!”
Tensors find homes by run-scoped decree,
Old drains make way for handles with glee.
Launch, reap, and cleanup now dance in a line—
“Squeak!” says the bunny, “The lifecycle is fine!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.07% 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 accurately summarizes the main change: splitting A2/A3 device launch and reap phases.
Description check	✅ Passed	The description is clearly related and matches the refactor to separate launch and reap 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.}

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/a2a3/platform/onboard/host/device_runner.cpp (1)

475-492: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Finalize dep_gen on launch failures too.

launch_run returns on AICore/AICPU launch failures before reap_run() runs, so the sync-error/path that already calls teardown_shared_collectors_after_run() bypasses the dep_gen_collector_.stop() + counter reconciliation + deps.json replay. Add an idempotent per-run terminal cleanup helper and invoke it from both launch-error returns and the success path to avoid emitting twice.

🤖 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/a2a3/platform/onboard/host/device_runner.cpp` around lines 475 - 492, Add
an idempotent per-run terminal cleanup helper that stops dep_gen collection,
reconciles its counters, and replays deps.json, then invoke it from both
AICore/AICPU launch-failure returns in launch_run and the existing successful
reap_run path. Ensure the helper coordinates with
teardown_shared_collectors_after_run() and prevents duplicate emission during
the same run.

🧹 Nitpick comments (2)

python/simpler/worker.py (2)

5840-5847: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Secondary cleanup errors in _finalize_run_handle are silently dropped.

_step() only records the first exception into result; any later step's exception (once result is already set) is swallowed with no diagnostic at all — unlike other best-effort cleanup helpers in this file (e.g. _flush_pending_remote_frees, _release_all_live_domains) which write the dropped errors to stderr. Worth at least a stderr note for observability parity.

🤖 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 `@python/simpler/worker.py` around lines 5840 - 5847, The exception path in
_finalize_run_handle’s nested _step should report subsequent cleanup failures
when result is already set. Preserve the first exception in result, but write
later exceptions to stderr, matching the diagnostic behavior of
_flush_pending_remote_frees and _release_all_live_domains.

---

5738-5821: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

callable shadows the Python builtin across the new submit/run API and its tests. The new Worker.submit/run/_submit_locked/_submit_l3_locked methods name their callback parameter callable, which Ruff flags (A002); the pattern is then echoed in test doubles/locals.

• python/simpler/worker.py#L5738-L5821: rename the callable parameter (e.g. orch_fn/fn) in submit, run, _submit_locked, and _submit_l3_locked.
• tests/ut/py/test_worker/test_host_worker.py#L1568-L1587: rename fake_submit's callable parameter to match the renamed API.
• tests/ut/py/test_worker/test_startup_readiness.py#L764-L777: rename the local variable callable in test_l2_submit_returns_completed_handle.

🤖 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 `@python/simpler/worker.py` around lines 5738 - 5821, Rename the callable
parameter in Worker.submit, run, _submit_locked, and _submit_l3_locked to a
non-shadowing name, updating all internal references and preserving behavior.
Also rename the callable parameter in
tests/ut/py/test_worker/test_host_worker.py lines 1568-1587 and the local
callable variable in tests/ut/py/test_worker/test_startup_readiness.py lines
764-777; no other changes are needed.

Source: Linters/SAST tools

🤖 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/orchestrator.md`:
- Around line 54-61: Update docs/orchestrator.md lines 54-61 to document
fail_run_submission(RunId run_id) without the exception_ptr parameter, matching
the binding contract. Also update docs/orchestrator.md lines 193-195 so the
pseudocode calls tensormap_.lookup(current_run_id, key) and
tensormap_.insert(current_run_id, key, sid), preserving the documented
run-scoped TensorMap API.

In `@python/simpler/worker.py`:
- Around line 1921-1968: Protect the `_finalize_run_handle` call in
`RunHandle.wait()` so any `BaseException` during finalization still records
terminal state, clears `_wait_in_progress`, releases run resources, notifies
waiters, and preserves the exception as the handle error. Ensure the
terminal-state assignments occur before any condition-variable acquisition,
mirroring the `Worker.close()` stranded-joiner protection, while retaining
normal native-error propagation.

In `@src/a2a3/platform/onboard/host/device_runner.h`:
- Around line 222-225: Update the public run() documentation to describe the
actual launch order: AICore submission via launch_run() occurs before AICPU
submission. Keep the surrounding synchronization and teardown documentation
unchanged.

In `@src/common/hierarchical/orchestrator.cpp`:
- Around line 204-207: Update Orchestrator::report_task_error to check
task.run_id before calling record_run_error: return safely when it equals
INVALID_RUN_ID, and handle or explicitly preserve the live-run assumption so an
unknown run id cannot unwind the scheduler loop. Keep normal error recording
unchanged for valid, existing run ids.

In `@tests/ut/cpp/hierarchical/test_tensormap.cpp`:
- Around line 105-117: Update TensorMap output cleanup to accept the producing
slot and erase a key only when its current mapping for the run still points to
that slot, preserving newer same-run producers. Modify the cleanup call sites
and the relevant TensorMap method, then extend SameAddressIsNamespacedByRun with
a same-run write-after-write regression covering producer overwrite and
subsequent cleanup.

---

Outside diff comments:
In `@src/a2a3/platform/onboard/host/device_runner.cpp`:
- Around line 475-492: Add an idempotent per-run terminal cleanup helper that
stops dep_gen collection, reconciles its counters, and replays deps.json, then
invoke it from both AICore/AICPU launch-failure returns in launch_run and the
existing successful reap_run path. Ensure the helper coordinates with
teardown_shared_collectors_after_run() and prevents duplicate emission during
the same run.

---

Nitpick comments:
In `@python/simpler/worker.py`:
- Around line 5840-5847: The exception path in _finalize_run_handle’s nested
_step should report subsequent cleanup failures when result is already set.
Preserve the first exception in result, but write later exceptions to stderr,
matching the diagnostic behavior of _flush_pending_remote_frees and
_release_all_live_domains.
- Around line 5738-5821: Rename the callable parameter in Worker.submit, run,
_submit_locked, and _submit_l3_locked to a non-shadowing name, updating all
internal references and preserving behavior. Also rename the callable parameter
in tests/ut/py/test_worker/test_host_worker.py lines 1568-1587 and the local
callable variable in tests/ut/py/test_worker/test_startup_readiness.py lines
764-777; no other changes are needed.

🪄 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: b5f4444d-ede8-4a58-91a7-0c0106dcf340

📥 Commits

Reviewing files that changed from the base of the PR and between d0bc661 and 0aec39c.

📒 Files selected for processing (32)

• docs/comm-domain.md
• docs/hierarchical_level_runtime.md
• docs/orchestrator.md
• docs/remote-l3-worker-design.md
• docs/scheduler.md
• docs/task-flow.md
• python/bindings/worker_bind.h
• python/simpler/orchestrator.py
• python/simpler/task_interface.py
• python/simpler/worker.py
• src/a2a3/platform/onboard/host/device_runner.cpp
• src/a2a3/platform/onboard/host/device_runner.h
• src/common/hierarchical/orchestrator.cpp
• src/common/hierarchical/orchestrator.h
• src/common/hierarchical/ring.cpp
• src/common/hierarchical/ring.h
• src/common/hierarchical/scheduler.cpp
• src/common/hierarchical/scheduler.h
• src/common/hierarchical/tensormap.cpp
• src/common/hierarchical/tensormap.h
• src/common/hierarchical/types.cpp
• src/common/hierarchical/types.h
• src/common/hierarchical/worker.cpp
• src/common/hierarchical/worker_manager.cpp
• src/common/hierarchical/worker_manager.h
• tests/ut/cpp/hierarchical/test_orchestrator.cpp
• tests/ut/cpp/hierarchical/test_scheduler.cpp
• tests/ut/cpp/hierarchical/test_tensormap.cpp
• tests/ut/py/test_worker/test_error_propagation.py
• tests/ut/py/test_worker/test_host_worker.py
• tests/ut/py/test_worker/test_l3_l2_orch_comm.py
• tests/ut/py/test_worker/test_startup_readiness.py