[unbalancedparentheses]

Review-only view of the final v0.8 performance delta for #240. Do not merge this PR. The base is the previously tested 7f609bf; the head is the same branch as #240. This keeps the diff small enough for the Claude workflow to read without truncation. Close this review view once its findings are resolved; merge/release happens only through #240 and #271.

Please review reply/turn token ownership, the no-drop-glue immediate dispatch adapter and macro compatibility, destructor and panic ordering, causal tracing, and precise clock behavior. The +3% instruction-count threshold and all guarantees remain unchanged.

Regression checks: 45 production-core reply/turn/job Loom tests, 270 simulated tests, 94 accepted lifecycle/reply/job traces and 133 expected model counterexamples; compile-pass/fail macro checks, future layout and panic regressions. Final source CI, full release performance/soak/model gates and both client ports remain required independently of this review.

[unbalancedparentheses]

/claude

Please review this complete, bounded delta against 7f609bf. The earlier #240 review was truncated before the important files, so this review-only PR exposes just the final changes; do not merge it. The diff is about 1,300 changed lines and should fit without truncation.

Review token ownership/retirement and the no-side-state slot fast path; await-free generated dispatch (only messages without drop glue), macro source compatibility, lazy direct calls, receiver/context lifetimes, destructors, panic/death fences and actor tracing. Also review JobState::finish retaining the unused later factory outside trace::atomic. That defect was reproduced by a cross-thread destructor timeout and its new regression now passes; CI explicitly runs it with lifecycle-trace. Please flag any remaining user callback/destructor under locks.

The +3% instruction gate is unchanged and passes. The release is still blocked on wall-clock regressions, and neither this PR nor #240 should merge until final CI, models, soak, clients and performance pass. Uncommitted handoff experiments are not in this review.

[github-actions]

🤖 Claude Code Review

Review: PR #272

The PR has three main parts:

• Immediate dispatch. Await-free generated handlers can run directly during dispatch, without allocating a boxed future.
• Turn-end fast path. A counter of externally owned ReplyTo tokens lets end_turn skip an atomic exchange when none exist.
• Trace-lock fix. Unused job completion factories are now dropped outside the trace lock.

The overall direction is sound and well tested. A few correctness and maintainability points need attention.

Correctness

1. Method generics are not checked before inlining (macros/src/lib.rs, await_free_handler)

await_free_handler rejects many cases, but it never looks at method.sig.generics. Consider a handler like:

async fn h<'a>(&mut self, m: M, ctx: &'a Context<Self>) -> u64

or one with a method-level where clause. The body and the ctx_ty get copied into a closure inside __spawned_handle_immediate, where 'a and T are not in scope. Code that compiled before this PR would now fail with "undeclared lifetime".

Suggested fix: add || !method.sig.generics.params.is_empty() || method.sig.generics.where_clause.is_some() to the bail-out condition.

2. The fast path assumes only the owner writes the phase (reply/turn.rs:129-141)

When external == 0, end_turn does load followed by store(RUNNING). That is only safe if no other thread can write phase at that moment. Token drops are covered by the counter. Please confirm the same holds for exiting() and the transition to DEAD, i.e. that they can only run on the owner (run-loop/cleanup) thread while a turn is active.

If a link or exit path, or runtime-drop cleanup, can move the phase concurrently, the plain store could turn DEAD or exiting back into RUNNING. The Loom cases only cover token droppers racing with end_turn.

3. Every external owner of a slot must hold the count (reply/mod.rs:496-509, ReplyTo::drop)

Ownership is now counted per ReplyTo. Please audit any path that moves the slot out of a ReplyTo and stores it elsewhere, for example pipe_reply, or a job that keeps the slot after the ReplyTo is consumed. In such a path, the ReplyTo's drop decrements the counter, and the later registry.dropped(slot) from the job thread then races a load/store end_turn.

Either:

• carry the hold along with the slot, or
• document why those paths cannot reach the phase.

4. Inconsistent destructor boundary (macros/src/lib.rs, needs_drop guard)

The guard checks only needs_drop::<M>(). If the reply is refused because the caller is gone, the M::Result value is still dropped inside direct dispatch.

If the "DeathFirst fence" rationale applies to message destructors, it seems to apply equally to result destructors. Please either also check needs_drop::<M::Result>(), or explain in the comment why the two are different.

5. Turn bookkeeping on the Ok((None, _)) => Ok(()) branch (tasks/actor.rs:2325)

This branch discards caller and skips whatever the Some branch does around polling. Please verify that the following still bracket a direct-dispatched handler:

• begin_turn / end_turn
• turn-latency instrumentation
• in-turn drop commits

A PendingReply that is dropped un-replied must still be committed with the turn's outcome. direct_dispatch_panic_answers_queued_callers_and_runs_stopped covers the panic case. A test with a non-panicking handler that drops its PendingReply without replying would be worthwhile.

Macro / maintainability

6. Method-level lint attributes are lost

allow, expect, deny and warn are accepted as "inert", but they are not propagated to the copied closure body. As a result:

• A method marked #[allow(deprecated)] or #[allow(unused_variables)] will now emit those warnings from the generated adapter.
• This breaks -D warnings builds.

Suggested fix: re-emit the method's lint attributes on the closure. Note that #[expect] needs care, because it is now evaluated on two copies of the body.

7. Diagnostic regression (actor_handler_ctx_wrong_type.stderr)

The error used to point at the user's parameter. It now points at #[actor], because __spawned_ctx uses Span::mixed_site().

Suggested fix: use Span::mixed_site().located_at(<user arg span>). This keeps the hygiene while restoring the useful error location.

8. Minor macro points

• await_free_handler(method) is evaluated twice (once in the call match, once for handler_impl). Compute it once into a local.
• nth(1).unwrap() / nth(2).unwrap() rely on earlier arity validation. A short comment or an expect("validated arity") would make that dependency explicit.
• visit_macro blocks every macro, including format!, vec!, tracing::debug! and assert!. Most real handlers will therefore never take the fast path. This is fine as a conservative first step, but it is worth stating in the GUIDE so users don't expect broad wins.
• Every await-free handler body is now compiled twice (as the inherent method and as the adapter closure). Small code-size and compile-time cost.

Benchmarks / CI

• Benches now use Deadline::Never (designs.rs, runtime_isolation.rs, workload_research.rs).
  • This changes what is measured compared with earlier campaigns: there is no per-request timer. Results from before and after this PR should not be compared directly. Consider noting this in the bench README or CHANGELOG.
  • A hang now relies entirely on the runner's 90s watchdog.
• benchmarks/README.md:22-24: the new sentence was not rewrapped, leaving one overlong line.
• fuzz.yml: RUSTUP_TOOLCHAIN moved from the job level to the workflow level. It now applies to every job in this workflow, not just scenario. If another job is added later (e.g. a stable replay or format check), it will silently run on nightly. Consider keeping it at the job level.

Minor

• reply/slot.rs:707-713: The relaxed check now depends on the invariant "side is never removed", which is only enforced by a debug_assert. Consider a // INVARIANT: note at the side field definition so future edits see the dependency.
• reply/jobs.rs finish: The later.take().expect(...) wrapper looks correct.
  • The regression test also deadlock-tests the refused branch. It is worth asserting that later is not invoked in that branch: if it were, drop(later_probe) would run inside the section and the test would be checking the wrong thing.
• RequestBudget refactor: Looks good. request_with_timeout stays lazy, and the explicit + Send bound matches the prior inferred auto trait.

---

Automated review by Claude · claude-opus-5-5 · custom prompt

[unbalancedparentheses]

/claude

Please review the current complete delta, including the new compact slot commit 3acec30. This review-only PR still fits without truncation; do not merge it. Focus on value/side-state promotion, terminal refusal preserving the first reply, destructors outside locks, parking_lot/standard-Loom mutex equivalence and Send/Sync behavior. Core crates still forbid unsafe; no coarse clock or deadline change. All 47 production-core Loom tests pass, plus unit/layout, macro and paused-clock regressions; focused Codex review found no actionable regressions. Final CI/performance/soak/model gates remain required.

Responses to your earlier findings:

• Method generics were already rejected by the macro. The immediate selector now additionally rejects method where clauses, and computes its decision once. Generated context identifiers keep mixed-site hygiene while pointing diagnostics at the user's parameter.
• Exiting/dead phase writes occur only in run-loop/owned cleanup; a runtime cannot drop a future while polling it. Jobs retaining slot Arcs after consuming ReplyTo do not call Registry::dropped. Only borrowed PendingReply drops and counted ReplyTo drops reach that phase; comments record this audit.
• Lint attributes were already propagated to the generated impl. New deny-warnings UI coverage verifies allow/expect(deprecated), and a constrained-method fixture uses the fallback.
• Refused result destructors run in reply() after the method returned in either adapter. A new regression verifies that their panic remains a handler panic and runs stopped(). Another direct-adapter regression drops an unanswered obligation on normal return and observes ReplyDropped, proving turn commit still brackets the direct branch.
• Fuzz CI found an oracle defect: a Normal Transient exit with a state-destructor cleanup failure is required to restart. The oracle now accounts for that documented policy. The original CI tape replays strictly; 64 additional seeds and negative checks still rejecting unjustified clean-Normal restarts pass. No production supervision behavior was relaxed.

[github-actions]

🤖 Claude Code Review

Review: PR #272

The core protocol changes look careful and well tested:

• The slot payload enum and the terminal pre-check in publish
• The later factory moved outside trace::atomic
• The Loom coverage for the external-token fast path

Most of the risk is in the macro's body duplication and the new ownership counter.

Correctness

1. macros/src/lib.rs (immediate branch): duplicated bodies duplicate static items

Inlining #body into the dispatch closure compiles the user body twice: once in the inherent method, once in the closure. Any item declared inside the body becomes two distinct items. Examples:

• static CALLS: AtomicUsize
• static INIT: OnceLock<…>
• a nested fn containing a static

As a result, calls through the mailbox and direct actor.compute(...) calls see different state. The Check visitor doesn't block items.

Suggested fix: add visit_item_static, or simply visit_item for any nested item, to set blocked = true.

2. concurrency/src/reply/turn.rs end_turn fast path: plain store can overwrite a concurrent phase change

The plain store(RUNNING) is only sound if every non-owner phase writer is a counted token drop. The comment asserts that exiting()/death transitions belong to the run-loop owner.

Please double-check that exiting() is never called from:

• an exit-signal or link path on another actor's thread, or
• runtime-drop cleanup that races a turn.

A foreign EXITING store between self.load() and store(RUNNING) would be silently lost. A debug_assert on the CAS result would catch this cheaply. For example, use compare_exchange(previous, RUNNING, Release, Relaxed) in debug builds.

3. reply/mod.rs / registry.rs: external counter balance depends on every ReplyTo path

• Construction: token_added is called only in PendingReply::defer. Any other ReplyTo construction site would underflow in external_finished. That is a debug panic, and in release it wraps to usize::MAX.
• Consumption without Drop: any path that consumes a ReplyTo without running Drop leaks a count. This includes mem::forget, ManuallyDrop, or destructuring into a linked job in pipe_reply*. The leak permanently disables the fast path, a silent perf regression.

Suggestions:

• Grep for every ReplyTo { … } literal and every forget/ManuallyDrop use.
• Consider a test asserting external == 0 after a pipe_reply round-trip.

4. reply/mod.rs: register/track after token construction may hit an unregistered slot

If register panics, ReplyTo::drop now calls registry.dropped(slot) for a slot that was never registered. Confirm dropped tolerates that, with no double release and no debug assert. Previously, the Arc<Slot> was simply dropped.

5. macros/src/lib.rs: inconsistent indexing of the context parameter

ctx_name locates its span via nth(ctx_index), but the immediate path hardcodes nth(2) for ctx_pat/ctx_ty. If ctx_index can ever differ from 2, the immediate path would bind the wrong parameter or panic in the macro. Use ctx_index in both places.

6. tasks/actor.rs, Ok((None, _)) => Ok(())

Verify the direct path doesn't skip per-turn bookkeeping that previously lived in the polling branch, such as:

• the cooperative yield/budget accounting
• instrumentation turn hooks
• begin_turn/end_turn placement relative to dispatch

The unanswered-obligation test passing suggests begin_turn precedes dispatch. Fairness under a stream of immediate messages is worth a targeted test, though: a mailbox-flood actor must still yield to the runtime.

API / ergonomics

7. request_with_timeout signature change

It changed from async fn to -> impl Future + Send + '_. That explicitly commits to Send in the public API, and any non-Send M::Result now fails at the definition rather than at the use site. This is probably intended, but call it out in the CHANGELOG as an API-visible change.

8. UI regressions in tests/ui/*.stderr

• actor_async_handler_threads now leads with E0407: __spawned_handle_immediate is not a member of trait Handler. That leaks a hidden internal name to users who simply used the wrong backend.
• The ctx-type error now points into the macro expansion.

Consider generating the hook only when the impl targets tasks::Handler, or emit it in a way that fails silently for threads.

9. JobKey retention

Unbounded per-distinct-key history is now documented but not bounded. Callers keying on request IDs leak until shutdown. A debug_assert or tracing::warn! past a high-water mark would make misuse visible.

Minor

• benches/designs.rs: after replacing Duration::from_secs(10), check whether Duration is still used. An unused import will warn under -D warnings.
• fuzz.yml: RUSTUP_TOOLCHAIN moved to workflow-level env, so any future job in this file silently runs on that nightly. Keeping it job-scoped was safer.
• slot.rs into_reply: drop(waker) is fine. The Payload::Extended arm's explicit drop(side) is redundant.
• SlotMutex::lock: it returns an infallible LockResult purely for Loom parity. A one-line comment at fn lock noting the unwrap_or_else is dead in production would help readers.

Tests

The new Loom cases are good:

• two_publishers_preserve_the_winning_value
• last_external_drop_is_visible_to_the_next_turn
• in_turn_reply_racing_defer_returns_the_credit_once

So is the trace-lock destructor regression. Two additions are worth making:

• a test for item Improve State handling #1 (a static inside a handler body observed via both paths), and
• a pipe_reply external-count balance test for item Changes and additions made for ethrex integration #3.

---

Automated review by Claude · claude-opus-5-5 · custom prompt

[unbalancedparentheses]

/claude

Please review the latest fixes at dcf3091 in this bounded review-only PR; do not merge it. Your local-static finding was reproduced, then fixed by excluding nested items from immediate specialization. A follow-up reproduction showed closure/async-block TypeId identity also changed when copied, so those now keep the async adapter too. The regression checks direct and mailbox calls across local statics, nested functions, closures and async blocks. All 94 UI cases and 14 immediate-dispatch tests pass, including a flooded-mailbox cooperative-budget regression. Production handler benchmarks contain none of these excluded constructs.

The requested pipe_reply ownership-balance test also passes: external_count remains one while the token is in the job completion factory, and returns to zero after reply consumption. Grepping production reveals one ReplyTo constructor, in defer(), and no forget/ManuallyDrop use on ReplyTo (PendingReply's ManuallyDrop is a separate borrowed obligation). Job completion retains the token itself, so it retains the count. register may prune terminal slots and thus run a destructor; if that panics, the newly constructed token's Drop records its slot in the turn and releases its hold, without requiring registration to have succeeded. The exiting/dead writers remain only the owned run-loop/ActorCleanup, or Registry::drop when no handles exist; signal paths request lifecycle exit, they do not mutate this phase.

The SlotMutex parity comment is clarified; the standalone thread-only example lockfile now includes the new dependency without upgrading existing packages. Native/async Clippy on pinned 1.93.0 and the Tokio-free dependency check pass. Message and Message::Result already require Send, so explicit future Send does not introduce the non-Send-result change hypothesized in your previous review. Key retention remains documented, without adding hot-path warnings. Final CI, versioned performance, soak and full model gates remain required before release.

[github-actions]

🤖 Claude Code Review

Review of PR #272

This PR bundles six changes:

• inline reply slots with a cold Extended box and parking_lot
• a turn-end fast path driven by an external-token counter
• await-free immediate dispatch generated by #[actor]
• moving the job-factory destructor outside the trace lock
• unbounded requests in the benchmarks
• a fix to the fuzz oracle

The concurrency reasoning is mostly sound. The main gaps are in test coverage and in how robust the token counter is.

Correctness / concurrency

reply/slot.rs – reply() terminal pre-check (~L812)

• This fixes a real bug. Previously, a losing publisher overwrote the winner's value and then took it back out.
• The check is race-free because both publishers write the value and do their CAS while holding the inner lock.
• Mapping REPLIED to Delivery::CallerGone for the second publisher is semantically odd. The caller isn't gone; the slot was already answered. It does match the existing Err(lost) mapping, though.
• Consider a dedicated variant, or at least a doc note on Delivery::CallerGone.

reply_in_turn (~L753)

• Replacing credits(current) != 0 with a debug_assert is safe, for two reasons:
  • TOKEN/credits are only set after side_or_init installs the side.
  • A stray credit would fail the exact-word CAS and fall through to publish.
• The Loom cases in_turn_reply_racing_defer… and late_side_promotion… cover the promotion race well.

reply/turn.rs end_turn fast path + registry.rs / ReplyTo::drop

• The memory ordering is correct:
  • The Relaxed increment is only performed by the owner thread.
  • The Release fetch_sub operations form a release sequence, so an Acquire read of 0 synchronizes with every prior drop.
• The fragility is accounting:
  • The decrement happens in every ReplyTo::drop.
  • The increment happens only at one site in PendingReply::defer (L518).
• Any other construction path would under-count. Examples:
  • re-wrapping a token in pipe_reply
  • ReplyTo { slot: x.slot.take(), .. }
  • future helpers
• Under-counting is the unsafe direction. In release builds the debug_assert vanishes, the counter wraps, and the fast path can race a live drop.
• Suggestion: move external_added() into a single ReplyTo::new constructor, and make that the only way to build the struct.
• Over-counting (for example via mem::forget) is benign; it just pins the slow path. A comment saying so would help.

reply/jobs.rs finish (L473)

• Correct: the unused later is now dropped after trace::atomic returns.
• The new test only passes if core.finish never invokes later in either the refused=false or refused=true case. If it did, later_probe would drop under the lock and the test would hit its 10s timeout.
• Worth a comment stating that assumption, so a future change to finish doesn't turn this into a confusing timeout.

Immediate dispatch (macros/src/lib.rs, tasks/actor.rs)

No test proves the fast path is actually taken.

• Every test in immediate_dispatch.rs would pass identically if await_free_handler always returned false. That could happen, for example, if the #[request_handler] attribute still sat in method.attrs and failed inert_attribute.
• Add a positive assertion along these lines:

  assert!(<Worker as Handler<Compute>>::__spawned_handle_immediate(&mut w, Compute{value:1}, detached_reply(), &ctx).is_none());

Drop-order tests are misnamed.

• In ignored_parameters_drop_after_the_handler_body_in_both_paths, both Ignored and Partial have drop glue (DropOrder, Arc). Because of the needs_drop guard, neither ever reaches the closure adapter.
• The same applies to ignored_parameter_destructor_panics_after_body….
• The "both paths" claim and the closure-parameter comment at macros L1785 ("Function/closure parameters retain ignored fields…") are therefore untested.
• Either rename the tests, or add a drop-glue-free pattern case.

Body duplication

• Each eligible handler body is now type-checked and compiled twice: once in the inherent method and once in the closure. Costs:
  • more compile time and code size
  • user type and borrow errors in the body may be reported twice
• The actor_handler_ctx_wrong_type.stderr churn hints at this. Worth confirming with a UI test that has a type error inside an eligible body.

actor_async_handler_threads.stderr

• The macro now emits __spawned_handle_immediate for the threads Handler, which has no such method. The result is a new leading E0407 that obscures the real error.
• Gate emission on the tasks backend.

Lint attributes

• Method-level #[allow]/#[expect] need to reach the copied closure body. Otherwise the copy will warn, and an #[expect] could fire unfulfilled in one copy.
• The ui-pass test covers this, so confirm extra_attrs carries those attributes and not only cfg.

Scope of the optimisation

• needs_drop::<M>() excludes any message containing String, Vec, Arc, and so on, so the optimisation only applies to POD messages.
• The guard is fine, but the CHANGELOG/GUIDE should set expectations about how narrow the win is.

API

tasks/actor.rs request_with_timeout (L903, L1525)

• Changing from async fn to fn -> impl Future + Send + '_ adds an explicit Send bound to a public signature.
• Previously Send-ness leaked from the body.
• This is likely equivalent today, but it is now a hard promise. Note it in the CHANGELOG as an API-visible change.

CI / misc

fuzz.yml

• RUSTUP_TOOLCHAIN is now workflow-level, so it overrides rust-toolchain.toml for every job in this workflow, including any added later.
• If only scenario needs nightly, keep it job-scoped. The dtolnay step can still reference a workflow-level FUZZ_NIGHTLY.

concurrency/benches/designs.rs

• Check whether the Duration import is now unused.

Cargo.toml (parking_lot = "0.12.5")

• This is a new unconditional dependency for all users, including thread-only builds. That is acceptable, but worth noting in the CHANGELOG.

fuzz/src/oracles.rs

• The oracle change and its unit tests look right.
• The docstring line for restart_justified exceeds the surrounding wrap width (cosmetic).

Summary

Area	Assessment
Slot refactor and two-publisher fix	Solid
Turn-end fast path	Correctly ordered, but relies on unenforced construction discipline; centralize the increment in a constructor
Immediate dispatch	Lacks a test proving it activates; several "direct path" tests exercise only the fallback

---

Automated review by Claude · claude-opus-5-5 · custom prompt

[unbalancedparentheses]

/claude

Final follow-up to your latest review: ReplyTo construction and its ownership increment now live together in one private ReplyTo::new. The increment runs after instrumentation construction, so a stamp panic cannot leak a hold. Forgetting a token is documented as conservatively retaining the slow path. The pipe_reply round-trip verifies count 1 -> 0. New eligible_handler_uses_the_immediate_hook positively asserts None from the hook (it fails if specialization is disabled); the drop-order test name now says mailbox/inherent rather than implying it tests the optimized branch. A mailbox-flood test verifies cooperative scheduling. The fuzz nightly literal is job-scoped, with setup referencing that job's env. Please review this small follow-up and confirm whether any release-blocking correctness issue remains.

All earlier local-static and anonymous-type findings have reproducing regressions; their adapter exclusions are conservative. The remaining invalid-backend UI fixture intentionally imports threads::Handler for an async method; its extra hidden-hook error is diagnostic quality, not valid-source breakage. Existing lint UI cases confirm attributes reach the generated impl. Message::Result already requires Send. The parking_lot dependency is described in the release notes. No timeout semantics, safety guarantee, benchmark threshold, unsafe policy, or scheduling budget was weakened.

[github-actions]

🤖 Claude Code Review

Review: PR #272

The core protocol changes look careful and are well covered by tests. These are the most solid parts of the PR:

• Publisher pre-check in publish: without it, a second publisher could overwrite and destroy the winning value under the lock. The fix is correct, and two_publishers_preserve_the_winning_value would have caught the old bug.
• Unused-factory drop in JobState::finish: the unused later factory is now dropped outside trace::atomic. The regression test reproduces the deadlock.
• Turn fast-path memory ordering: Release on fetch_sub and Acquire when reading zero are correct. Two concurrent droppers are handled through the release sequence, and the Loom cases cover it.

Some items below need verification or tightening.

Correctness / concurrency

1. slot.rs, reply_in_turn: keep the credits(current) != 0 check.

if state(current) != OPEN || !matches!(inner.payload, Payload::Empty) {
...
debug_assert_eq!(credits(current), 0);

• current is already loaded, so the runtime check costs one AND and a compare. That is negligible next to the mutex.
• The invariant ("side.pool is installed before any credit") is non-local. In release builds the new code silently takes the fast path if it is ever violated.
• I recommend keeping the runtime check as defense-in-depth.

2. turn.rs, end_turn fast path: the invariant is broad and only commented. The non-atomic load followed by store(RUNNING) is sound only if no thread other than the owner writes phase while external == 0. That includes exiting(), dead, and any link or exit path. The comment asserts this, but nothing enforces it.

• Consider a #[cfg(debug_assertions)] variant that uses compare_exchange(previous, RUNNING) and asserts success.
• That would turn a future violation into a test failure instead of a lost drop.

3. ReplyTo construction must always go through ReplyTo::new. Drop now unconditionally calls external_finished().

• If any other site builds ReplyTo { slot, registry, stamp } directly (type conversion, re-wrapping, a job hand-back), the counter underflows. That panics in debug and wraps to usize::MAX in release, permanently disabling the fast path.
• Conversely, any mem::forget(self) pattern inside ReplyTo methods permanently pins the actor to the slow path. The doc comment acknowledges this.
• Please confirm there are no other struct-literal constructions. Making the fields private to a submodule would enforce it.

4. tasks/actor.rs, run_actor_lifecycle: Ok((None, _)) => Ok(()) skips the whole future branch. User handler code now runs inside dispatch(). Please confirm:

• begin_turn() is called before dispatch(), not just before the first poll. Otherwise a PendingReply dropped in the direct body is recorded outside a turn.
• Whatever the Some(turn) branch does with caller is not needed when the turn completes synchronously. Examples: watcher registration, end-of-turn trace events, the in_actor wrapper's bookkeeping.
• The tests suggest panic and drop semantics hold. A short comment at the match arm explaining why caller can be ignored would help.

5. reply/mod.rs, defer: Drop can now run on an unregistered slot. The token is constructed before register and track_deadline. If either panics, ReplyTo::drop calls registry.dropped(slot) for a slot that was never registered. Please verify dropped tolerates that.

Macro / immediate dispatch

6. macros/src/lib.rs: the handler body is compiled twice. It is compiled once in the inherent method and once in the __spawned_handle_immediate closure, even when needs_drop::<M>() makes the copy dead code.

• Consequences: more compile time and code size per handler, and possibly duplicated lint diagnostics (e.g. unused_variables, clippy) from the copied tokens.
• Consider emitting the immediate impl only when the message type is syntactically known to be trivially droppable. Alternatively, accept this and document it.

7. Narrow applicability. Every macro blocks the optimization, including info!, debug!, assert! and format!, and so does any message with drop glue (String, Vec, Arc). Real-world handlers will rarely qualify.

• This is fine, but the CHANGELOG and GUIDE should make clear it is a niche path.
• It also argues for justifying the duplicated-body cost in (6) with benchmark numbers.

8. UI diagnostics regression (actor_async_handler_threads.stderr). Users who mix #[actor] async handlers with threads::Handler now get an extra confusing E0407: __spawned_handle_immediate is not a member error. Consider gating generation so the original error stays the only one.

API / perf

9. request_with_timeout: signature change and code bloat.

• Public API change: it changed from async fn to fn -> impl Future + Send + '_. The explicit + Send is a new public promise. That is fine, but note it in the CHANGELOG as an API-visible change.
• Two monomorphizations: request_with_budget<M, B> now instantiates the full request state machine for both DefaultRequestBudget and Duration, per message type.
• Unverified saving: if duration is live across an .await, the future stores 16 bytes regardless, and the "zero-sized budget" saving disappears.
• Suggestion: add a size_of_val regression like the slot one, or drop the generic.

10. Payload::put on the Value variant. *self = Self::Value(value) silently drops a prior value under the lock. With the new pre-check this should be unreachable, so a debug_assert!(!matches!(self, Self::Value(_))) would document and enforce it.

Minor

• concurrency/benches/designs.rs:319,324: check whether Duration is still used elsewhere in the file to avoid an unused-import warning.
• benches/workload_research.rs: most of this hunk is a rustfmt reflow. Splitting formatting-only changes (also in fuzz/src/driver.rs and oracles.rs) into a separate commit would ease review.
• slot.rs publish pre-check: mapping REPLIED to Delivery::CallerGone for a refused second publisher is misleading, since the caller isn't gone. This was pre-existing, but now there is a dedicated branch where it could be distinguished.
• fuzz/src/oracles.rs: relaxing V1/V2 so a Transient child with a Normal reason restarts on destructor panic changes the oracle to match observed behavior. Please confirm this is the documented supervision contract (the MIGRATION reference) and not a masked bug. The bidirectional unit tests are good.
• parking_lot in the core crate: the dependency is acceptable. The comment "payloads/wakers are never dropped under it" is worth restating as a hard rule, since the lock is non-poisoning and a panic under it now leaves the state silently in use.

---

Automated review by Claude · claude-opus-5-5 · custom prompt