docs: API alternatives summary with registry and macro analysis by ElFantasma · Pull Request #150 · lambdaclass/spawned

ElFantasma · 2026-02-13T21:36:21Z

Summary

Comprehensive comparison of 6 API approaches (Baseline, A–F) for the actor framework, using the same chat room example throughout
Updated Approach A code samples to reflect the new actor_api! macro and #[actor] improvements
Added Registry & Service Discovery section analyzing how the named registry differs across approaches (what's stored, type safety, discovery granularity)
Added Macro Improvement Potential section evaluating whether actor_api!-style macros could reduce boilerplate in unimplemented approaches (B–F)
Updated comparison matrices with registry and macro rows

Test plan

Review doc for accuracy against implemented code on feat/actor-macro-registry
Verify code samples compile (Approach A samples match actual chat_room example)

… API Introduces a unified API redesign addressing #144, #145, and #129: - Handler<M> trait with per-message typed results (RPITIT, not object-safe) - Receiver<M>/Recipient<M> for type-erased cross-actor messaging - #[actor] proc macro that generates Handler<M> impls from #[handler] methods - Any-based named registry (register/whereis/unregister) - messages! declarative macro for defining message structs - Context<A> replaces ActorRef<A> in handler signatures - New examples: chat_room (Recipient), service_discovery (registry) - All existing examples migrated to new API

…dler macros

…potential

github-actions · 2026-02-13T21:38:53Z

🤖 Codex Code Review

Review Summary
The refactor toward Message + Handler and the type-erased envelope is clean and the tests are solid. I found a few correctness/concurrency issues worth addressing.

Findings

Cancellation does not wake the actor loop (tasks + threads)
Context::stop() only flips the CancellationToken, but run_actor blocks on rx.recv() and only checks the token after a message arrives. That means an externally requested stop can hang forever if no new messages are sent, and join() can block indefinitely.

concurrency/src/tasks/actor.rs:392-405
concurrency/src/threads/actor.rs:368-381
Suggestion: select between rx.recv() and cancellation_token.cancelled() (async), or close the channel when stopping. For threads, use recv_timeout in a loop that also checks the token, or add a wake-up message on stop.

started() / stopped() panics can cause join() to hang (tasks backend)
run_actor does not guard started() or stopped() with catch_unwind. If either panics, the task unwinds before the completion watch is updated, so ActorRef::join() waits forever.

concurrency/src/tasks/actor.rs:352-355
concurrency/src/tasks/actor.rs:385-389
Suggestion: wrap started() and stopped() in catch_unwind and ensure the completion signal is sent via a scope guard even on panic.

send_message_on for threads ignores cancellation
The thread-based send_message_on always executes f() and only checks the token after it returns. If f() blocks (e.g., ctrl_c()), this thread can live forever even after the actor stops.

concurrency/src/threads/actor.rs:407-418
Suggestion: add a cancellation-aware variant (e.g., f(cancellation_token)), or periodically check cancellation while waiting.

Nice work
The message/handler refactor and the type-erased Recipient API are clean and feel idiomatic. The tests are extensive and useful.

Possible next steps

I can propose concrete patches for the cancellation/recv behavior in both backends.
I can add panic guards for started()/stopped() with a completion scope guard.
I can draft a cancellation-aware send_message_on for the threads backend.

Automated review by OpenAI Codex · custom prompt

greptile-apps · 2026-02-13T21:39:55Z

Greptile Overview

Greptile Summary

This PR adds comprehensive documentation comparing 6 different API design approaches for the actor framework, alongside the actual implementation of Approach A (Handler + Recipient). The implementation includes major refactoring of the actor system from enum-based to per-message handlers, new registry functionality, and the #[actor] proc macro.

Key Changes:

Added 1100-line API_ALTERNATIVES_SUMMARY.md comparing Baseline and Approaches A-F with the same chat room example
Implemented Handler trait with envelope-based type erasure (Approach A)
Added named registry with Recipient<M> storage for service discovery
Created #[actor] macro to generate Handler impls from annotated methods
Migrated examples (bank, chat_room, service_discovery) to new API
Added message definition macros (messages!, send_messages!, request_messages!)

Critical Issue:
The documentation extensively references an actor_api! macro (lines 224-256, 313-318, 376-383, 945-1020, and elsewhere) that does not exist in the codebase. The actual chat_room example uses manual trait definitions instead. This creates significant confusion about what's implemented vs. proposed. Either:

The actor_api! macro should be removed from the docs, or
The macro needs to be implemented before merging

The code samples showing "With #[actor] macro + actor_api!" don't match reality - only #[actor] exists.

Otherwise:
The implementation quality is excellent. The Handler pattern is well-executed, the registry works correctly, and examples demonstrate the concepts clearly. The documentation itself is thorough and well-structured for comparing approaches.

Confidence Score: 2/5

This PR should not be merged as-is due to documentation-implementation mismatch
The implementation is solid (would be 4/5), but the documentation extensively describes an actor_api! macro that doesn't exist, creating misleading expectations. The PR title says "API alternatives summary with registry and macro analysis" but documents unimplemented features as if they exist. This needs correction before merge.
docs/API_ALTERNATIVES_SUMMARY.md needs significant revision to clearly distinguish implemented features from proposals

Important Files Changed

Filename	Overview
docs/API_ALTERNATIVES_SUMMARY.md	New comprehensive documentation comparing 6 API approaches; however, extensively references `actor_api!` macro that doesn't exist in the codebase yet
concurrency/src/message.rs	New file introducing `Message` trait and declarative macros (`messages!`, `send_messages!`, `request_messages!`) for message definition
concurrency/src/registry.rs	New named registry implementation with Any-based type-erased storage, providing register/whereis/unregister API
concurrency/src/tasks/actor.rs	Major refactoring from enum-based Actor trait to Handler + Recipient pattern with envelope-based type erasure
macros/src/lib.rs	New `#[actor]` proc macro generating Handler implementations from `#[send_handler]`/`#[request_handler]` annotated methods
examples/chat_room/src/room.rs	New chat room example implementing Approach A with manual API trait (not using `actor_api!` macro mentioned in docs)
examples/service_discovery/src/main.rs	New example demonstrating named registry usage with Recipient for service discovery pattern

_{Last reviewed commit: 36d03b4}

greptile-apps

_{47 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-13T21:39:59Z

docs/API_ALTERNATIVES_SUMMARY.md

+### With `#[actor]` macro + `actor_api!`
+
+<details>
+<summary><b>room.rs</b> — macros eliminate both Handler and extension trait boilerplate</summary>
+
+```rust
+use spawned_concurrency::actor_api;
+use spawned_concurrency::send_messages;
+use spawned_concurrency::request_messages;
+use spawned_concurrency::tasks::{Actor, ActorRef, Context, Handler, Recipient};
+use spawned_macros::actor;
+
+// -- Messages --
+
+send_messages! {
+    Say { from: String, text: String };
+    Deliver { from: String, text: String };
+    Join { name: String, inbox: Recipient<Deliver> }
+}
+
+request_messages! {
+    Members -> Vec<String>
+}
+
+// -- API --
+
+actor_api! {
+    pub ChatRoomApi for ActorRef<ChatRoom> {
+        send fn say(from: String, text: String) => Say;
+        send fn add_member(name: String, inbox: Recipient<Deliver>) => Join;
+        request async fn members() -> Vec<String> => Members;
+    }
+}


The actor_api! macro referenced extensively in this section doesn't exist in the codebase. The actual implementation in examples/chat_room/src/room.rs uses manual trait definitions (lines 31-49), not the actor_api! macro shown here.

The documentation shows:

actor_api! { pub ChatRoomApi for ActorRef<ChatRoom> { send fn say(from: String, text: String) => Say; // ... } }

But the actual code manually defines the trait and impl without this macro.

Prompt To Fix With AI

This is a comment left during a code review. Path: docs/API_ALTERNATIVES_SUMMARY.md Line: 224:256 Comment: The `actor_api!` macro referenced extensively in this section doesn't exist in the codebase. The actual implementation in `examples/chat_room/src/room.rs` uses manual trait definitions (lines 31-49), not the `actor_api!` macro shown here. The documentation shows: ```rust actor_api! { pub ChatRoomApi for ActorRef<ChatRoom> { send fn say(from: String, text: String) => Say; // ... } } ``` But the actual code manually defines the trait and impl without this macro. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-13T21:40:00Z

docs/API_ALTERNATIVES_SUMMARY.md

+| Dimension | Non-macro | With `#[actor]` macro + `actor_api!` |
+|-----------|-----------|--------------------------------------|
+| **Readability** | Each `impl Handler<M>` block is self-contained. You see the message type and return type in the trait bound. But many small impl blocks can feel scattered. | `#[send_handler]`/`#[request_handler]` attributes inside a single `#[actor] impl` block group all handlers together. `actor_api!` declares the caller-facing API in a compact block. Files read top-to-bottom: Messages → API → Actor. |
+| **API at a glance** | Must scan all `impl Handler<M>` blocks to know what messages an actor handles. | The `actor_api!` block is the "at-a-glance" API surface — each line declares a method, its params, and the underlying message. |
+| **Boilerplate** | One `impl Handler<M>` block per message × per actor. Message structs need manual `impl Message`. | `send_messages!`/`request_messages!` macros eliminate `Message` impls. `#[actor]` eliminates `Handler` impls. `actor_api!` reduces the extension trait + impl (~15 lines) to ~5 lines. |
+| **main.rs expressivity** | Raw message structs: `room.send_request(Join { ... })` — explicit but verbose. | Extension traits: `alice.join_room(room.clone())` — reads like natural API calls. |
+| **Circular dep solution** | `Recipient<M>` — room stores `Recipient<Deliver>`, user stores `Recipient<Say>`. Neither knows the other's concrete type. | Same mechanism. The macros don't change how type erasure works. |
+| **Discoverability** | Standard Rust patterns. Any Rust developer can read `impl Handler<M>`. | `#[actor]` and `actor_api!` are custom — new developers need to learn what they do, but the patterns are common (Actix uses the same approach). |


The comparison table references actor_api! macro features that aren't implemented. The "With #[actor] macro + actor_api!" column describes functionality that doesn't exist in the codebase (grep shows actor_api only appears in this documentation file).

Prompt To Fix With AI

This is a comment left during a code review. Path: docs/API_ALTERNATIVES_SUMMARY.md Line: 376:383 Comment: The comparison table references `actor_api!` macro features that aren't implemented. The "With `#[actor]` macro + `actor_api!`" column describes functionality that doesn't exist in the codebase (grep shows `actor_api` only appears in this documentation file). How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-13T21:40:01Z

docs/API_ALTERNATIVES_SUMMARY.md

+## Macro Improvement Potential
+
+Approach A's `actor_api!` macro eliminates extension trait boilerplate by generating a trait + impl from a compact declaration. Could similar macros reduce boilerplate in the other approaches?
+
+### Approach B: Protocol Traits — YES, significant potential
+
+The bridge impls are structurally identical to what `actor_api!` already generates. Each bridge method just wraps `self.send(Msg { fields })`:
+
+```rust
+// Current bridge boilerplate (~10 lines per actor)
+impl ChatBroadcaster for ActorRef<ChatRoom> {
+    fn say(&self, from: String, text: String) -> Result<(), ActorError> {
+        self.send(Say { from, text })
+    }
+    fn add_member(&self, name: String, inbox: Arc<dyn ChatParticipant>) -> Result<(), ActorError> {
+        self.send(Join { name, inbox })
+    }
+}
+```
+
+A variant of `actor_api!` could generate bridge impls for an existing trait:
+
+```rust
+// Potential: impl-only mode for existing protocol traits
+actor_api! {
+    impl ChatBroadcaster for ActorRef<ChatRoom> {
+        send fn say(from: String, text: String) => Say;
+        send fn add_member(name: String, inbox: Arc<dyn ChatParticipant>) => Join;
+    }
+}
+```
+
+This would use the same syntax but `impl Trait for Type` (no `pub`, no new trait) signals that we're implementing an existing trait. The protocol trait itself remains user-defined — it IS the contract, so it should stay hand-written.
+
+**Impact:** Bridge boilerplate per actor drops from ~10 lines to ~4 lines. The protocol trait definition stays manual (by design). Combined with `#[actor]`, the total code for a protocol-based actor would be competitive with Approach A.
+
+### Approach C: Typed Wrappers — NO
+
+The fundamental problem is the dual-channel architecture, not boilerplate. The `Clone` bound incompatibility between enum messages and `Recipient<M>` creates a structural split that macros can't paper over. Typed wrappers still hide `unreachable!()` branches internally.
+
+### Approach D: Derive Macro — N/A
+
+This approach IS a macro. The `#[derive(ActorMessages)]` would generate message structs, `Message` impls, API wrappers, and `Handler<M>` delegation — subsuming what `actor_api!`, `send_messages!`, and `#[actor]` do separately. Adding `actor_api!` on top would be redundant.
+
+### Approach E: AnyActorRef — NO
+
+You could wrap `send_any(Box::new(...))` in typed helper methods, but this provides false safety — the runtime dispatch can still fail. The whole point of AnyActorRef is erasing types; adding typed wrappers on top contradicts that.
+
+### Approach F: PID — PARTIAL
+
+The registration boilerplate could be automated:
+
+```rust
+// Current: manual registration per message type
+room.register::<Say>();
+room.register::<Join>();
+room.register::<Members>();
+
+// Potential: derive-style auto-registration
+#[actor(register(Say, Join, Members))]
+impl ChatRoom { ... }
+```
+
+And `spawned::send(pid, Msg { ... })` could get ergonomic wrappers similar to `actor_api!`. But since `Pid` carries no type information, these wrappers can only provide ergonomics, not safety — a wrong Pid still causes a runtime error.
+
+### Summary
+
+| Approach | Macro potential | What it would eliminate | Worth implementing? |
+|----------|----------------|----------------------|---------------------|
+| **B: Protocol Traits** | High | Bridge impl boilerplate | Yes — `actor_api!` impl-only mode |
+| **C: Typed Wrappers** | None | N/A — structural problem | No |
+| **D: Derive Macro** | N/A | Already a macro | N/A |
+| **E: AnyActorRef** | None | Would add false safety | No |
+| **F: PID** | Low-Medium | Registration ceremony | Maybe — ergonomics only |
+
+**Takeaway:** Approach B is the only unimplemented approach that would meaningfully benefit from `actor_api!`-style macros. And the required change is small — adding an impl-only mode to the existing macro. This would make Approach B more competitive with Approach A on boilerplate, while retaining its testability advantage.


The entire "Macro Improvement Potential" section discusses how actor_api! could be extended to other approaches, but this macro doesn't exist yet. This creates confusion about what's actually implemented vs. what's proposed.

Prompt To Fix With AI

This is a comment left during a code review. Path: docs/API_ALTERNATIVES_SUMMARY.md Line: 945:1020 Comment: The entire "Macro Improvement Potential" section discusses how `actor_api!` could be extended to other approaches, but this macro doesn't exist yet. This creates confusion about what's actually implemented vs. what's proposed. How can I resolve this? If you propose a fix, please make it concise.

ElFantasma added 4 commits February 11, 2026 17:36

feat: rename send_request → request, add send/request message and han…

548008e

…dler macros

docs: add API alternatives summary with chat room comparison

73cda3b

docs: add registry analysis, actor_api! macro, and macro improvement …

36d03b4

…potential

greptile-apps bot reviewed Feb 13, 2026

View reviewed changes

ElFantasma added 3 commits February 13, 2026 18:50

docs: use permanent commit links instead of branch references

7844f54

docs: use full commit SHAs for permanent links

0ae9989

docs: use absolute GitHub URLs for cross-branch links

51f1961

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: API alternatives summary with registry and macro analysis#150

docs: API alternatives summary with registry and macro analysis#150
ElFantasma wants to merge 7 commits intomainfrom
docs/api-alternatives-summary

ElFantasma commented Feb 13, 2026

Uh oh!

github-actions bot commented Feb 13, 2026

Uh oh!

greptile-apps bot commented Feb 13, 2026

Important Files Changed

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Feb 13, 2026

Uh oh!

greptile-apps bot Feb 13, 2026

Uh oh!

greptile-apps bot Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ElFantasma commented Feb 13, 2026

Summary

Test plan

Uh oh!

github-actions bot commented Feb 13, 2026

🤖 Codex Code Review

Uh oh!

greptile-apps bot commented Feb 13, 2026

Greptile Overview

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant