Fix: make two merged guards say what they actually guard

[ChaoWao]

Summary

Four corrections left behind by #1845 and #1857. No behavior changes — one header declaration, one static_assert, two comment blocks and one doc paragraph.

Both merged PRs left a guard that does not say what it guards. That matters more than usual here: one of the files exists specifically to record what is proven, and the other holds a compile-time contract.

From #1857

The _ARM_CALLABLE_ID comment got the causality wrong

It reached the right conclusion from the wrong mechanism:

The L2 st_worker is session-scoped, and the framework's own default test_run registers this class's callable at id 0 and keeps it there for the session.

The sibling conftest.py overrides st_worker to scope="class" — deliberately, so this white-box stress gets a worker whose slot table starts empty and does not enter _l2_worker_pool. So the collision is inside the class, not across the session.

The actual chain, verified:

Step	Evidence
the worker is class-scoped	conftest.py:27 @pytest.fixture(scope="class")
test_run registers via Worker.register	scene_test.py:1506
that takes the lowest free slot, i.e. 0	task_interface.py:1353 _allocate_slot_locked
and never gives it back during the class	the handle is cached on type(self)._st_l2_handle; nothing unregisters it

The merged PR description also explained the original test's survival as "sorting alphabetically before test_run". That is not how pytest collects (definition order within a class), and it does not fit the evidence either: test_serial_submission_… sorts alphabetically after test_run and passes. register_callable failed with code -1 was a real observation; the story attached to it was not.

The robustness claim in the old comment — off id 0 in any test order — is correct and is kept, since the arms bypass _callable_registry via _register_callable_at_slot.

The diagnostics arm had no expiry condition

That arm asserts a fallback the project intends to remove. concurrent_native_prepare_supported_impl says so itself:

// The common C API keeps collector-bearing configurations on
// the sequential path until their state is per-epoch.

So when collector state becomes per-epoch and diagnostics_any() leaves allow_prepared_successor, a diagnostic config will overlap like any other and the arm turns red with the very did not overlap it currently demands. The correct response then is to delete the arm, not restore the serialization — and nothing in the test or the doc said so, which is how a temporary fallback gets cemented by its own test.

The two negative arms are not equally durable, and that asymmetry is what decides the response:

Arm	Coupled to	Lifetime
serial submission (inflight_limit=1)	nothing — one run in flight cannot overlap under any admission policy	permanent
diagnostics config	the silent diagnostics_any() fallback	retires with it

Stated in both the arm's docstring and docs/dfx/host-trace.md, since its value and its lifetime have the same source.

From #1845

set_host_log_state had no declaration anywhere

The sim AICPU backend defines it and both sim/host/device_runner.cpp files resolve it by name — grep finds three references and zero declarations. Its sibling set_log_level, resolved by the same load_sym call two lines above, is declared in aicpu/device_log.h. The previous revision of #1845 declared both under SIMPLER_CPU_SIM_HOST_LOG_BRIDGE; deleting that macro (correctly) took the declaration with it.

Declared next to set_log_level, with the state struct only forward-declared so neither <dlfcn.h> nor the state layout reaches device targets. The dlsym caller stays string-resolved, exactly as set_log_level's does — the point is that the definition now has something to disagree with.

KeepsExistingInitArgumentOrder asserted a tautology

TEST(ChipWorkerApi, KeepsExistingInitArgumentOrder) { EXPECT_NE(&call_existing_chip_worker_init, nullptr); }

A function's address is never null. The real guard was that the helper compiled; the runtime assertion tested nothing, and gcc can warn on the comparison itself.

The intent — pin the parameter order, because the Python binding passes positionally and an inserted parameter silently changes what an existing call means — is worth keeping, so it is now expressed as what it is:

using ExpectedChipWorkerInit = void (ChipWorker::*)(
    const std::string &, const std::string &, const std::string &, const std::string &, int, const CallConfig *,
    uint32_t, const std::string &
);
static_assert(
    std::is_same_v<decltype(&ChipWorker::init), ExpectedChipWorkerInit>,
    "ChipWorker::init signature changed: append new parameters, never insert"
);

std::is_invocable_v on the five leading arguments would not work — a pointer-to-member type carries no default arguments — and pinning the full type is the stricter, more useful guard anyway: any signature change becomes deliberate.

Verification

• ctest --test-dir tests/ut/cpp/build -LE requires_hardware -j4 --output-on-failure --timeout 300 — 100/100 pass
• Inversion check on the new guard: reordering two parameters in the alias makes the build stop with the intended message, then reverted. A guard that cannot fail is the defect this PR is fixing, so it gets the same treatment.

  error: static assertion failed: ChipWorker::init signature changed: append new parameters, never insert
  

• Both aicpu/device_log.cpp backends syntax-check against the new declaration (g++ -fsyntax-only), including the onboard one that only ever sees the declaration.
• The scene-test change is comment-only, so onboard behavior is unchanged; ruff check and py_compile clean.
• pre-commit clean on all four files except clang-tidy, whose hook venv cannot import simpler and fails identically on untouched files.

Not in scope

• docs/dfx/host-trace.md is now 306 lines, so this change is what pushes it past doc-consistency.md §6's ~300-line soft target (it already had 6 H2 sections). A split — most naturally lifting ## Async pipeline proof and its negative-controls subsection into docs/dfx/native-overlap-proof.md — is a bigger change than these corrections warrant, so it is flagged rather than bundled.
• Fix: make host logging consumers self-contained #1845's shared CMake source list is only half adopted. The six platform CMakeLists.txt use SIMPLER_HOST_LOG_SOURCES, but python/bindings/CMakeLists.txt, sim_context/CMakeLists.txt and tests/ut/cpp/CMakeLists.txt (7 sites) still hardcode the two paths. A partial dedup keeps the "remember the other places" problem, so it wants its own change.

🤖 Generated with Claude Code

[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: e9aa008d-5dc7-4c1b-b051-343d4bac483c

📥 Commits

Reviewing files that changed from the base of the PR and between f4cd9cb and 1299612.

📒 Files selected for processing (4)

• docs/dfx/host-trace.md
• src/common/platform/include/aicpu/device_log.h
• tests/st/a2a3/host_build_graph/concurrent_prepare_stress/test_concurrent_prepare_stress.py
• tests/ut/cpp/hierarchical/test_pipeline_contract.cpp

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

---

📝 Walkthrough

Walkthrough

The changes add a host logging state setter declaration, strengthen the ChipWorker::init signature test, and clarify concurrent preparation test policies and fallback lifetimes.

Changes

Host logging API

Layer / File(s)	Summary
Host logging state contract src/common/platform/include/aicpu/device_log.h	The header declares SimplerHostLogState and the C-linkage set_host_log_state setter.

Async pipeline contracts

Layer / File(s)	Summary
Worker initialization signature contract tests/ut/cpp/hierarchical/test_pipeline_contract.cpp	The test uses a compile-time assertion to verify the complete ChipWorker::init signature and removes the helper-based runtime test.
Concurrent preparation policy documentation tests/st/a2a3/host_build_graph/concurrent_prepare_stress/test_concurrent_prepare_stress.py, docs/dfx/host-trace.md	Comments and documentation define worker slot ownership, permanent serial rejection behavior, and the temporary diagnostics serialization fallback.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 12996

This change corrects documentation and strengthens compile-time contract checks without changing runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

• hw-native-sys/simpler#1811: Introduced the async pipeline proof and its temporary diagnostics-serialization fallback.

Poem

A rabbit checks the slots in line,
And guards the worker’s type design.
Host logs cross the backend gate,
While temporary locks await their fate.
Hop, hop—contracts now align!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the corrections to the two guard comments and assertions, which are central changes in the pull request.
Description check	✅ Passed	The description directly explains all four corrections, their purpose, verification, and out-of-scope items.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

---

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