Skip to content

Conversation

@reiase
Copy link
Contributor

@reiase reiase commented Jan 25, 2026

Overview:

Details:

Where should the reviewer start?

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

- Removed deprecated methods from `ActorSystemCoreExt` and `ActorSystemAdvancedExt` to simplify the API.
- Consolidated actor spawning logic into a single internal method, enhancing clarity and reducing redundancy.
- Updated usage of actor spawning in tests and examples to utilize the new builder pattern for improved readability and maintainability.
- Adjusted imports and module structure for better organization and consistency across the codebase.
…ptions

- Simplified the handling of lazy actor references by delegating message sending to resolved references, reducing complexity and avoiding recursion.
- Introduced a new `Format` enum for message serialization, supporting binary and JSON formats, with auto-detection capabilities for improved flexibility.
- Updated the `SpawnOptions` and `ResolveOptions` structures to utilize default constructors, enhancing code clarity and consistency.
- Improved error handling in the `SpawnBuilder` for actor name validation, ensuring clearer feedback on invalid paths.
- Adjusted documentation and examples to reflect the new default options and serialization methods.
- Updated the ActorLifecycle struct to replace string-based identifiers with ActorId for both watchers and targets, enhancing type safety and consistency.
- Modified watch and unwatch methods to accept ActorId parameters, streamlining the registration and removal of watchers.
- Adjusted related methods and tests to reflect the new ActorId usage, ensuring proper functionality and improved clarity in actor relationships.
- Enhanced logging to utilize ActorId for better traceability during watch operations.
- Introduced UUID-based ActorId and NodeId, replacing previous numeric identifiers for improved uniqueness and traceability.
- Updated ActorAddress to support new UUID format, ensuring backward compatibility with legacy formats.
- Refactored tests and system components to utilize the new ActorId structure, enhancing clarity and consistency across the codebase.
- Added `uuid` dependency to the project for UUID generation and handling.
- Modified Justfile to include necessary build dependencies for Python and C++ integration.
- Introduced SystemActorProxy and PythonActorServiceProxy for direct method calls, improving interaction with system and service actors.
- Refactored BucketStorage and TopicBroker to utilize remote method support, streamlining actor communication.
- Updated tests to validate new proxy methods and ensure proper functionality across various scenarios.
- Enhanced message serialization and error handling for improved robustness and clarity in actor interactions.
… handling

- Converted StorageManager from an Actor to a remote class, enabling direct method calls.
- Replaced message-based communication with explicit method calls for bucket and topic management.
- Updated related methods to enhance clarity and maintainability, including get_bucket, get_topic, list_buckets, and get_stats.
- Adjusted tests to validate the new proxy method interactions, ensuring proper functionality across scenarios.
- Unified error handling by introducing a clear hierarchy of error types, including `PulsingError`, `RuntimeError`, and `ActorError`.
- Updated the `PulsingError` enum to categorize errors into `RuntimeError` for framework-level issues and `ActorError` for user-defined execution errors.
- Refactored error conversion logic to ensure seamless integration between Rust and Python, allowing for better error classification and handling.
- Enhanced error messages for clarity and consistency across the system, improving debugging and user feedback.
- Updated tests to validate the new error handling mechanisms and ensure proper functionality across various scenarios.
- Updated the Ray stress test script to utilize Ray Generators for streaming, improving the accuracy of performance comparisons with Pulsing.
- Modified the `StreamWorker` class to implement streaming via `yield`, allowing for asynchronous consumption of results.
- Adjusted the performance comparison report to reflect the new implementation, highlighting the significant performance differences between Ray and Pulsing, even with the use of generators.
- Revised metrics in the report to provide a clearer picture of latency and throughput improvements, emphasizing the advantages of Pulsing in low-latency scenarios.
…ing sections

- Added a comprehensive guide on communication patterns, detailing when to use sync, async, streaming, and fire-and-forget methods.
- Introduced a new section on error handling, categorizing errors into framework and actor execution types, with clear recovery strategies.
- Updated existing guides to reference the new communication patterns and error handling sections, improving overall documentation coherence.
- Enhanced examples to illustrate error handling and communication patterns in practical scenarios, aiding user understanding.
@reiase reiase merged commit 89ff331 into main Jan 25, 2026
8 checks passed
for _ in stream_items:
async for ref in worker.generate_stream.remote(count, delay):
# await the ObjectRef to get the actual value
item = await ref

Check notice

Code scanning / CodeQL

Unused local variable Note test

Variable item is not used.

Copilot Autofix

AI 9 days ago

In general, when a local variable is assigned but never used and the right-hand side has no side effects, either remove the variable or rename it to clearly indicate that it is intentionally unused. Here, the assignment item = await ref both awaits the ObjectRef (which is required to realize the stream elements and surface any exceptions) and binds the result to item. The await itself is needed, but the bound name is not. To preserve behavior while satisfying the unused-variable rule and keeping the code illustrative, we should keep the await ref call but bind it to a deliberately unused name.

The best minimal fix is to rename item on line 286 to something like _item (which contains unused-style semantics via the leading underscore convention) so that CodeQL recognizes it as intentionally unused, without altering any other logic. Concretely, in benchmarks/large_scale_stress_test_ray_single.py, inside StressTestClient.send_stream_request, change the line

item = await ref

to

_item = await ref

No imports or additional definitions are needed for this change.

Suggested changeset 1
benchmarks/large_scale_stress_test_ray_single.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/benchmarks/large_scale_stress_test_ray_single.py b/benchmarks/large_scale_stress_test_ray_single.py
--- a/benchmarks/large_scale_stress_test_ray_single.py
+++ b/benchmarks/large_scale_stress_test_ray_single.py
@@ -283,7 +283,7 @@
             chunk_count = 0
             async for ref in worker.generate_stream.remote(count, delay):
                 # await the ObjectRef to get the actual value
-                item = await ref
+                _item = await ref
                 chunk_count += 1
                 # Process item if needed (currently just counting)
 
EOF
@@ -283,7 +283,7 @@
chunk_count = 0
async for ref in worker.generate_stream.remote(count, delay):
# await the ObjectRef to get the actual value
item = await ref
_item = await ref
chunk_count += 1
# Process item if needed (currently just counting)

Copilot is powered by AI and may make mistakes. Always verify output.
import hashlib
import logging
from typing import Any
from typing import TYPE_CHECKING, Any

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'Any' is not used.

Copilot Autofix

AI 9 days ago

In general, to fix an unused import, you remove the specific unused name from the import statement (or delete the entire import if none of its names are used). This keeps the dependencies minimal and the code clearer.

For this file, TYPE_CHECKING is used in the if TYPE_CHECKING: block, while Any is never referenced. The best minimal-change fix is to modify the import line near the top of python/pulsing/queue/manager.py so that it only imports TYPE_CHECKING. Concretely, change line 6 from from typing import TYPE_CHECKING, Any to from typing import TYPE_CHECKING. No other lines, methods, or imports need to be added or changed.

Suggested changeset 1
python/pulsing/queue/manager.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/python/pulsing/queue/manager.py b/python/pulsing/queue/manager.py
--- a/python/pulsing/queue/manager.py
+++ b/python/pulsing/queue/manager.py
@@ -3,7 +3,7 @@
 import asyncio
 import hashlib
 import logging
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING
 
 from pulsing.actor import ActorId, ActorRef, ActorSystem, remote
 
EOF
@@ -3,7 +3,7 @@
import asyncio
import hashlib
import logging
from typing import TYPE_CHECKING, Any
from typing import TYPE_CHECKING

from pulsing.actor import ActorId, ActorRef, ActorSystem, remote

Copilot is powered by AI and may make mistakes. Always verify output.
from typing import Any, AsyncIterator

from pulsing.actor import Actor, ActorId, Message, StreamMessage
from pulsing.actor import ActorId, StreamMessage, remote

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'StreamMessage' is not used.

Copilot Autofix

AI 9 days ago

To fix the problem, the unused name should be removed from the import statement so that only the actually used names are imported. This keeps the module’s dependencies minimal and aligned with what the code uses.

Concretely, in python/pulsing/queue/storage.py, edit the import on line 7 to drop StreamMessage. Leave ActorId and remote intact, because they are used by on_start and as a decorator on BucketStorage. No additional imports, methods, or definitions are required.

Suggested changeset 1
python/pulsing/queue/storage.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/python/pulsing/queue/storage.py b/python/pulsing/queue/storage.py
--- a/python/pulsing/queue/storage.py
+++ b/python/pulsing/queue/storage.py
@@ -4,7 +4,7 @@
 import logging
 from typing import Any, AsyncIterator
 
-from pulsing.actor import ActorId, StreamMessage, remote
+from pulsing.actor import ActorId, remote
 
 from .backend import StorageBackend, get_backend_class
 
EOF
@@ -4,7 +4,7 @@
import logging
from typing import Any, AsyncIterator

from pulsing.actor import ActorId, StreamMessage, remote
from pulsing.actor import ActorId, remote

from .backend import StorageBackend, get_backend_class

Copilot is powered by AI and may make mistakes. Always verify output.
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 68.74266% with 266 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/pulsing-actor/src/error.rs 49.00% 77 Missing ⚠️
python/pulsing/actor/remote.py 44.23% 58 Missing ⚠️
python/pulsing/queue/manager.py 70.00% 21 Missing ⚠️
crates/pulsing-actor/src/transport/http2/client.rs 37.50% 15 Missing ⚠️
python/pulsing/exceptions.py 61.29% 12 Missing ⚠️
crates/pulsing-actor/src/actor/traits.rs 68.57% 11 Missing ⚠️
crates/pulsing-actor/src/system/traits.rs 72.50% 11 Missing ⚠️
python/pulsing/topic/broker.py 71.05% 11 Missing ⚠️
crates/pulsing-actor/src/system/resolve.rs 55.00% 9 Missing ⚠️
crates/pulsing-actor/src/actor/address.rs 88.52% 7 Missing ⚠️
... and 13 more
Files with missing lines Coverage Δ
crates/pulsing-actor/src/behavior/core.rs 71.79% <ø> (+0.90%) ⬆️
crates/pulsing-actor/src/metrics/mod.rs 74.07% <ø> (ø)
crates/pulsing-actor/src/system/config.rs 57.41% <100.00%> (-0.72%) ⬇️
crates/pulsing-actor/src/system_actor/messages.rs 78.57% <100.00%> (+78.57%) ⬆️
crates/pulsing-actor/src/system_actor/mod.rs 73.29% <100.00%> (ø)
crates/pulsing-actor/src/test_helper.rs 86.66% <ø> (ø)
python/pulsing/actor/__init__.py 86.27% <100.00%> (+0.56%) ⬆️
python/pulsing/actors/load_stream.py 0.00% <ø> (ø)
python/pulsing/topic/__init__.py 100.00% <ø> (ø)
crates/pulsing-actor/src/actor/context.rs 87.76% <98.27%> (+0.49%) ⬆️
... and 22 more

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants