Add Python ↔ Rust roundtrip tests mirroring existing Python ↔ TS tests#10
Draft
Add Python ↔ Rust roundtrip tests mirroring existing Python ↔ TS tests#10
Conversation
Co-authored-by: pirate <511499+pirate@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Implement roundtrip test between Python and Rust
Add Python ↔ Rust roundtrip tests mirroring existing Python ↔ TS tests
Feb 20, 2026
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
30 issues found across 29 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="bubus-rust/tests/test_event_result.rs">
<violation number="1" location="bubus-rust/tests/test_event_result.rs:21">
P1: Custom agent: **Test quality checker**
This test only asserts 2 out of 10 fields set by `EventResult::new()`, making it a shallow test that doesn't verify behavioral correctness. Per the test quality rule: *"no 'fake' tests that simply check if attrs/methods are present or that calls run without error, tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct."*
The test should also assert:
- `result.event_id == "event-id"`
- `result.handler.id == "h1"` (and other handler fields)
- `result.result.is_none()`
- `result.error.is_none()`
- `result.started_at.is_none()`
- `result.completed_at.is_none()`
- `result.event_children.is_empty()`
- `result.id` is a valid UUIDv7 string</violation>
</file>
<file name="bubus-rust/tests/test_eventbus_on_off.rs">
<violation number="1" location="bubus-rust/tests/test_eventbus_on_off.rs:13">
P1: Custom agent: **Test quality checker**
Test only asserts `event_results.len()` but never verifies the actual result value or event status. The handler returns `Ok(json!("ok"))` but this value is never checked — the handler could return anything and the test would still pass. Per the test quality rule, tests must assert that all output values are behaviorally correct. Add assertions for the result value (e.g., `json!("ok")`), `event_status == Completed`, and handler metadata like `handler_name`.</violation>
</file>
<file name="bubus-rust/tests/test_eventbus_timeout.rs">
<violation number="1" location="bubus-rust/tests/test_eventbus_timeout.rs:105">
P1: Custom agent: **Test quality checker**
Weak assertion defeats the test's purpose. `assert!(has_error || is_completed)` will pass whether or not the parent timeout actually cancelled the child — it accepts *any* outcome. A test named `cancels_pending_or_started_children` must assert that the child was indeed cancelled/aborted (i.e., has an `Error` result with an appropriate error type like `EventHandlerAbortedError` or `EventHandlerCancelledError`), not merely that it has *some* result.
Per the rule: *"no 'fake' tests … tests must assert that all writes, mutations, side effects, and output values are actually behaviorally correct."*</violation>
</file>
<file name="bubus-rust/tests/test_eventbus_find.rs">
<violation number="1" location="bubus-rust/tests/test_eventbus_find.rs:17">
P1: Custom agent: **Test quality checker**
Test `test_find_future_waits_for_new_event` only asserts `found.is_some()` without verifying any properties of the found event (e.g., `event_type`). The rule prohibits tests that merely check a call returned *something* — tests must assert behavioral correctness of all output values. The equivalent TS and Python tests verify `event_type` at minimum. Add assertions on the found event's `event_type` and/or `event_id`.</violation>
</file>
<file name="bubus-rust/tests/event_bus_tests.rs">
<violation number="1" location="bubus-rust/tests/event_bus_tests.rs:53">
P1: Custom agent: **Test quality checker**
This test claims to verify parallel handler concurrency but only asserts that both handlers produced results (`len() == 2`), which would also pass under serial execution. To actually test parallelism, measure wall-clock time and assert it's less than the sum of individual sleeps (e.g., <40ms for two 20ms sleeps), or use synchronization primitives to prove concurrent entry. Also, the individual result values (`json!(1)` and `json!(2)`) are never asserted — a handler returning the wrong value would not be caught.</violation>
</file>
<file name="bubus-rust/tests/test_eventbus_dispatch_defaults.rs">
<violation number="1" location="bubus-rust/tests/test_eventbus_dispatch_defaults.rs:25">
P2: Test doesn't actually test bus defaults — it explicitly sets the same values on the event that the bus would use as defaults, so the `unwrap_or(self.default)` fallback path is never exercised. To test that bus defaults are applied, remove the block that sets `event_handler_concurrency` and `event_handler_completion` on the event, leaving them as `None`.</violation>
<violation number="2" location="bubus-rust/tests/test_eventbus_dispatch_defaults.rs:31">
P1: Custom agent: **Test quality checker**
This test is effectively a "fake" test—it claims to verify that default handler settings are applied, but the only assertion (`event_results.len() == 1`) proves nothing about whether `Serial` concurrency or `All` completion modes were actually enforced. With a single handler, every combination of concurrency/completion modes yields exactly one result.
To be a meaningful test, it should register **multiple handlers** and assert observable behavioral differences caused by the settings (e.g., verify serial execution order, verify that `All` completion waits for every handler vs. `First` returning after only one). The actual result values should also be asserted.</violation>
</file>
<file name="bubus-rust/tests/test_event_history_store.rs">
<violation number="1" location="bubus-rust/tests/test_event_history_store.rs:17">
P1: Custom agent: **Test quality checker**
This test violates the rule against "fake" tests — the assertion `history.iter().any(|id| id.contains('-'))` only checks UUID format, not which events were actually retained. For a test named `keeps_recent_entries`, it must verify that the 2 **most recent** events (`evt_1`, `evt_2`) are kept and the oldest (`evt_0`) is dropped. Store each event's ID before emitting and assert the history contains exactly the expected IDs. Also register at least one handler so `wait_completed()` exercises real processing, not a trivial no-op path.</violation>
<violation number="2" location="bubus-rust/tests/test_event_history_store.rs:33">
P2: Custom agent: **Test quality checker**
This test only asserts the rejected event's `event_path` is empty but never verifies the positive case — that the first event was successfully stored in history and processed. A robust test for `drop=false` rejection must assert both sides of the boundary: `bus.event_history_ids().len() == 1`, the first event's path is non-empty, and ideally the second event's status reflects rejection. Register a handler so `wait_completed()` exercises real processing.</violation>
</file>
<file name="bubus-rust/tests/test_eventbus_locking.rs">
<violation number="1" location="bubus-rust/tests/test_eventbus_locking.rs:8">
P1: Custom agent: **Test quality checker**
**Flaky test**: `test_queue_jump` has a race condition. The handler is trivially fast (no sleep/delay), so the worker thread can fully process `event1` before `event2` is even pushed to the front of the queue via `emit_with_options`. When that happens, `event1_started < event2_started` and the assertion `event2_started <= event1_started` fails.
The Python counterpart (`test_queue_jump_awaited_child_preempts_queued_sibling_on_same_bus`) avoids this by using the parent→child pattern with `asyncio.sleep(0.005)` in the child handler, ensuring both events are queued before processing completes, and asserting a deterministic operation order rather than comparing timestamps. This test should follow the same approach: add a non-trivial delay in the handler so both events are guaranteed to be in the queue before the first one finishes, and assert execution ordering rather than relying on timestamp comparison.</violation>
</file>
<file name="bubus-rust/tests/test_base_event.rs">
<violation number="1" location="bubus-rust/tests/test_base_event.rs:18">
P1: Custom agent: **Test quality checker**
This roundtrip test only checks `json == json` after serialize→deserialize, which is a tautological assertion — if serialization is broken but self-consistent, this passes. The test must assert concrete field values (`event_id` format, `event_type == "test_event"`, `event_status == "pending"`, payload `{"value": 1}`, timestamp presence/format) to verify behavioral correctness, as both the Python and TS test suites do for their equivalent `toJSON`/`fromJSON` roundtrip tests.</violation>
<violation number="2" location="bubus-rust/tests/test_base_event.rs:29">
P1: Custom agent: **Test quality checker**
`block_on(event.wait_completed())` at the end of this test runs without any subsequent assertion — it only checks the call doesn't error/hang, which the rule explicitly forbids ('no fake tests that simply check that calls run without error'). Additionally, the state-transition test never verifies that `mark_started()`/`mark_completed()` set the corresponding timestamp fields (`event_started_at`, `event_completed_at`), unlike the equivalent Python and TS lifecycle tests which assert those values are populated after each transition.</violation>
</file>
<file name="bubus-rust/tests/test_ids.rs">
<violation number="1" location="bubus-rust/tests/test_ids.rs:21">
P1: Custom agent: **Test quality checker**
Test claims Python/TS compatibility but doesn't share any test vectors with the Python test `test_event_handler_id_matches_typescript_uuidv5_algorithm`. The rule requires parity between Python and TS (and by extension Rust) test coverage. To prove cross-language compatibility, include the same test vector used in the Python test:
expected_seed = "018f8e40-1234-7000-8000-000000001234|pkg.module.handler|~/project/app.py:123|2025-01-02T03:04:05.678901000Z|StandaloneEvent"
expected_id = "0acdaf2c-a5b1-5785-8499-7c48b3c2c5d8"
and assert `compute_handler_id(...)` produces `expected_id` for those exact inputs.</violation>
<violation number="2" location="bubus-rust/tests/test_ids.rs:46">
P1: Custom agent: **Test quality checker**
This `from_callable` block is a near-"fake" test — it only checks `get_version_num() == 5` without verifying the actual handler ID value is correct. The rule prohibits tests that "simply check if attrs/methods are present" without asserting behavioral correctness.
The Python equivalent (`test_event_handler_id_matches_typescript_uuidv5_algorithm`) constructs an `EventHandler` with all fields controlled and asserts the *exact* expected ID. Here, `from_callable` internally uses `now_iso()` making the ID non-deterministic, so you can't assert the exact value. Instead, either:
1. Construct the `EventHandler` struct directly (like Python does) with controlled `handler_registered_at` and assert the exact ID, or
2. At minimum, verify `entry.id == compute_handler_id(&eventbus_id, &handler_name, None, &entry.handler_registered_at, &event_pattern)` to confirm `from_callable` correctly delegates to `compute_handler_id`.</violation>
</file>
<file name="bubus-rust/tests/test_typed_events.rs">
<violation number="1" location="bubus-rust/tests/test_typed_events.rs:42">
P2: Custom agent: **Test quality checker**
`test_on_typed_and_emit_typed_roundtrip` only asserts `first_result()` but doesn't verify result count or event completion metadata. Existing untyped tests (e.g., `test_emit_and_handler_result`) consistently verify `results.len() == 1` in addition to the result value. A bug producing duplicate results or failing to set completion timestamps would go undetected. Add assertions for result count and event status to match the rigor of existing tests.</violation>
<violation number="2" location="bubus-rust/tests/test_typed_events.rs:48">
P1: Custom agent: **Test quality checker**
`test_find_typed_returns_typed_payload` is too shallow: it registers no handler (so the typed result path is untested), doesn't verify the `event_type` of the found event (the primary semantic of `find_typed`), and doesn't assert event status/results/completion metadata. Compare with the existing untyped `test_find_past_match_returns_event` which at minimum verifies `event_type`. Per the rule, tests must assert that all side effects and output values are behaviorally correct — this test only checks two payload field values.
This test should register a handler, verify the found event's type and completion status, assert `event_results` count, and check the typed result value through the found event.</violation>
</file>
<file name="bubus-rust/tests/test_event_handler_ids.rs">
<violation number="1" location="bubus-rust/tests/test_event_handler_ids.rs:20">
P1: Custom agent: **Test quality checker**
Tautological test: the expected value is computed using the same algorithm as the function under test, so it can never catch a real bug. This violates the rule that tests must "assert that all writes, mutations, side effects, and output values are actually behaviorally correct" — not just that the same code produces the same output twice.
The assertion should use a **hardcoded known-good UUID** (ideally one also verified by the Python implementation to ensure cross-language parity). For example, compute the expected value once using a trusted implementation and hardcode it.</violation>
</file>
<file name="bubus-rust/tests/test_event_handler_first.rs">
<violation number="1" location="bubus-rust/tests/test_event_handler_first.rs:16">
P0: Custom agent: **Test quality checker**
This test file has drastically insufficient coverage compared to its TS counterpart (`bubus-ts/tests/event_handler_first.test.ts` which has 20+ tests). The rule requires parity between implementations and thorough edge-case coverage for all `event_handler_completion` config options. Missing tests include: `First` with parallel handlers, cancellation of remaining handlers, undefined/null/falsy return values, error paths (all throw, mixed), no handlers registered, single handler, child event cancellation, and `event_handler_completion` field visibility. These are critical for a system handling financial data at scale.</violation>
<violation number="2" location="bubus-rust/tests/test_event_handler_first.rs:21">
P1: Custom agent: **Test quality checker**
`thread::sleep` is a blocking call inside an `async move` block. With the single-threaded `futures::executor::block_on`, this blocks the entire executor if the handler ever runs. Use an async-compatible sleep (e.g., `tokio::time::sleep`) instead. Additionally, the test should track whether the second handler was actually invoked (like the TS test does with `second_handler_called = false`) to properly assert behavioral correctness rather than just checking result count.</violation>
</file>
<file name="bubus-rust/src/lock_manager.rs">
<violation number="1" location="bubus-rust/src/lock_manager.rs:7">
P0: Custom agent: **Make sure concurrency options work correctly and consistently**
`ReentrantLock` wraps `parking_lot::Mutex` which **will deadlock** on reentrant acquisition from the same thread. `parking_lot` provides `ReentrantMutex` specifically for recursive/reentrant locking. The Python implementation (`bubus/lock_manager.py`) correctly implements reentrant semantics with depth tracking via `ContextVar`, but this Rust equivalent has no such capability. Queue-jumping in `GlobalSerial` or `BusSerial` mode (where a handler awaits a child event that needs the same lock) will deadlock instead of re-entering the lock.
Use `parking_lot::ReentrantMutex<()>` instead of `parking_lot::Mutex<()>`, and return `parking_lot::ReentrantMutexGuard` from the `lock()` method.</violation>
</file>
<file name="bubus-rust/src/event_bus.rs">
<violation number="1" location="bubus-rust/src/event_bus.rs:101">
P0: Custom agent: **Make sure concurrency options work correctly and consistently**
The background event-loop thread in `start_loop` holds an `Arc<Self>`, preventing the bus from ever being deallocated without an explicit `stop()` call. Rule 1 requires: *"Make sure all memory is correctly freed / able to be garbage collected automatically without manual .stop() / .destroy() calls by the user. Most users aren't going to use the shutdown methods."* With the stated use case of *"millions of busses... creating and destroying busses per request"*, this is a severe memory leak.
Fix: Use a `Weak<Self>` in the thread (upgrading per iteration), and implement `Drop for EventBus` (or a wrapper) to set the stop flag so the thread exits when all strong references are dropped.</violation>
<violation number="2" location="bubus-rust/src/event_bus.rs:109">
P0: Custom agent: **Make sure concurrency options work correctly and consistently**
The `thread::sleep(Duration::from_millis(1))` on every loop iteration when the queue is non-empty adds 1ms of pure latency per event, exceeding the rule's 0.2ms/event/handler overhead budget by 5x. This sleep appears intended to yield the thread, but it fires on every event dequeue—even when events are waiting to be processed immediately. Remove this sleep; the `queue_notify.listen().await` on the empty-queue path already handles idle waiting correctly.</violation>
<violation number="3" location="bubus-rust/src/event_bus.rs:293">
P1: Bug: `?` operator causes `find_in_history` to return `None` prematurely when any event is missing from the map, instead of skipping it and continuing the search. If an event was evicted from `self.runtime.events` (due to history size limits) but its ID is still in the cloned `history_order` snapshot, this aborts the entire search. Use a `let ... else { continue }` pattern instead.</violation>
</file>
<file name="bubus-rust/src/base_event.rs">
<violation number="1" location="bubus-rust/src/base_event.rs:16">
P1: Custom agent: **Make sure concurrency options work correctly and consistently**
`now_iso()` lacks the monotonic timestamp guarantee that Python's `monotonic_datetime()` provides. Python uses a lock + last-timestamp tracking (`bubus/helpers.py:63-69`) to ensure every timestamp is strictly increasing even under concurrent access. The Rust version uses raw `SystemTime::now()` which can produce duplicate or out-of-order timestamps under high throughput, breaking the global event ordering invariant described in the README. This needs an `AtomicU64` or `Mutex`-guarded last-timestamp to match Python's semantics.</violation>
<violation number="2" location="bubus-rust/src/base_event.rs:21">
P0: Custom agent: **Make sure concurrency options work correctly and consistently**
`now_iso()` produces `{epoch_secs}.{nanos}Z` (e.g., `1708000000.123456789Z`) instead of proper ISO 8601 format (`2024-02-15T12:00:00.123456789Z`). Python's `monotonic_datetime()` in `bubus/helpers.py:32-41` uses `strftime("%Y-%m-%dT%H:%M:%S").{nanos:09d}Z`. This format mismatch will break any cross-language roundtrip because Python parses timestamps with `datetime.fromisoformat()`. The format should be `YYYY-MM-DDTHH:MM:SS.nnnnnnnnnZ`.</violation>
</file>
<file name="bubus-rust/src/event_result.rs">
<violation number="1" location="bubus-rust/src/event_result.rs:26">
P2: Missing `#[serde(default)]` on `event_children: Vec<String>`. This PR explicitly added `#[serde(default)]` to `event_results` in `BaseEventData` for JSON compatibility, but the same pattern is missing here. If a JSON payload omits the `event_children` key, deserialization will fail with a missing-field error. Add `#[serde(default)]` for consistency and robustness.</violation>
</file>
<file name="bubus-rust/src/typed.rs">
<violation number="1" location="bubus-rust/src/typed.rs:54">
P2: `first_result()` iterates over `HashMap::values()` which has non-deterministic ordering. When multiple handlers succeed, this returns an arbitrary result rather than a predictable "first" one. Consider sorting by handler key or using an ordered collection to ensure deterministic behavior consistent with the Python/TS implementations.</violation>
<violation number="2" location="bubus-rust/src/typed.rs:104">
P3: `payload_map_from_value` is dead code — it is defined but never called anywhere in the codebase. The same logic already exists inline in `TypedEvent::new()` (lines 23–26). Consider removing it or using it in `new()` to avoid duplication.</violation>
</file>
<file name="bubus-rust/Cargo.toml">
<violation number="1" location="bubus-rust/Cargo.toml:10">
P3: The `thread-pool` feature on the `futures` crate is unused — only `executor` (for `block_on`) is referenced anywhere in the source. Removing it trims an unnecessary transitive dependency and compile time.</violation>
</file>
<file name="bubus-rust/README.md">
<violation number="1" location="bubus-rust/README.md:22">
P2: Quickstart example won't compile: `event_bus::new(...)` and `base_event::new(...)` are not free functions — `new` is an associated function on the `EventBus` and `BaseEvent` structs respectively. The imports and constructor calls need to reference the structs directly.</violation>
</file>
Architecture diagram
sequenceDiagram
participant PY as Python Test (pytest)
participant FS as File System (JSON)
participant RU as Rust Binary (roundtrip_events)
participant BUS as Rust EventBus/BaseEvent
Note over PY,RU: NEW: Python ↔ Rust Cross-Runtime Roundtrip Flow
PY->>PY: Generate BaseEvent with Pydantic
PY->>FS: NEW: Write events to BUBUS_RUST_INPUT_PATH
PY->>RU: NEW: Execute via subprocess.run()
Note right of RU: Env: BUBUS_RUST_INPUT_PATH / OUTPUT_PATH
RU->>FS: Read JSON input
FS-->>RU: Event JSON list
loop For each event
RU->>BUS: NEW: BaseEvent::from_json_value(value)
Note right of BUS: Uses serde(default) for event_results
BUS-->>RU: Arc<BaseEvent>
RU->>BUS: NEW: event.to_json_value()
BUS-->>RU: Re-serialized JSON
end
alt Success
RU->>FS: NEW: Write results to BUBUS_RUST_OUTPUT_PATH
RU-->>PY: Exit Code 0
else Error
RU-->>PY: Exit Code 1 (stderr)
PY->>PY: pytest.fail()
end
PY->>FS: Read BUBUS_RUST_OUTPUT_PATH
FS-->>PY: Rust-serialized JSON
PY->>PY: NEW: BaseEvent.model_validate(rust_json)
Note over PY: test_python_to_rust_roundtrip_schema_enforcement_after_reload
PY->>PY: Load event into fresh Python EventBus
alt Valid payload
PY->>PY: Handler returns correct shape
PY-->>PY: EventStatus: completed
else Invalid payload
PY->>PY: NEW: Handler returns wrong shape
PY-->>PY: EventStatus: error (Pydantic ValidationError)
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,41 @@ | |||
| use std::{sync::Arc, thread, time::Duration}; | |||
Contributor
There was a problem hiding this comment.
P0: Custom agent: Test quality checker
This test file has drastically insufficient coverage compared to its TS counterpart (bubus-ts/tests/event_handler_first.test.ts which has 20+ tests). The rule requires parity between implementations and thorough edge-case coverage for all event_handler_completion config options. Missing tests include: First with parallel handlers, cancellation of remaining handlers, undefined/null/falsy return values, error paths (all throw, mixed), no handlers registered, single handler, child event cancellation, and event_handler_completion field visibility. These are critical for a system handling financial data at scale.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_event_handler_first.rs, line 16:
<comment>This test file has drastically insufficient coverage compared to its TS counterpart (`bubus-ts/tests/event_handler_first.test.ts` which has 20+ tests). The rule requires parity between implementations and thorough edge-case coverage for all `event_handler_completion` config options. Missing tests include: `First` with parallel handlers, cancellation of remaining handlers, undefined/null/falsy return values, error paths (all throw, mixed), no handlers registered, single handler, child event cancellation, and `event_handler_completion` field visibility. These are critical for a system handling financial data at scale.</comment>
<file context>
@@ -0,0 +1,41 @@
+}
+
+#[test]
+fn test_event_handler_first_serial_stops_after_first_success() {
+ let bus = EventBus::new(Some("BusFirstSerial".to_string()));
+
</file context>
|
|
||
| #[derive(Default, Clone)] | ||
| pub struct ReentrantLock { | ||
| lock: Arc<Mutex<()>>, |
Contributor
There was a problem hiding this comment.
P0: Custom agent: Make sure concurrency options work correctly and consistently
ReentrantLock wraps parking_lot::Mutex which will deadlock on reentrant acquisition from the same thread. parking_lot provides ReentrantMutex specifically for recursive/reentrant locking. The Python implementation (bubus/lock_manager.py) correctly implements reentrant semantics with depth tracking via ContextVar, but this Rust equivalent has no such capability. Queue-jumping in GlobalSerial or BusSerial mode (where a handler awaits a child event that needs the same lock) will deadlock instead of re-entering the lock.
Use parking_lot::ReentrantMutex<()> instead of parking_lot::Mutex<()>, and return parking_lot::ReentrantMutexGuard from the lock() method.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/lock_manager.rs, line 7:
<comment>`ReentrantLock` wraps `parking_lot::Mutex` which **will deadlock** on reentrant acquisition from the same thread. `parking_lot` provides `ReentrantMutex` specifically for recursive/reentrant locking. The Python implementation (`bubus/lock_manager.py`) correctly implements reentrant semantics with depth tracking via `ContextVar`, but this Rust equivalent has no such capability. Queue-jumping in `GlobalSerial` or `BusSerial` mode (where a handler awaits a child event that needs the same lock) will deadlock instead of re-entering the lock.
Use `parking_lot::ReentrantMutex<()>` instead of `parking_lot::Mutex<()>`, and return `parking_lot::ReentrantMutexGuard` from the `lock()` method.</comment>
<file context>
@@ -0,0 +1,29 @@
+
+#[derive(Default, Clone)]
+pub struct ReentrantLock {
+ lock: Arc<Mutex<()>>,
+}
+
</file context>
| thread::spawn(move || { | ||
| block_on(async move { | ||
| loop { | ||
| if !bus.runtime.queue.lock().is_empty() { |
Contributor
There was a problem hiding this comment.
P0: Custom agent: Make sure concurrency options work correctly and consistently
The thread::sleep(Duration::from_millis(1)) on every loop iteration when the queue is non-empty adds 1ms of pure latency per event, exceeding the rule's 0.2ms/event/handler overhead budget by 5x. This sleep appears intended to yield the thread, but it fires on every event dequeue—even when events are waiting to be processed immediately. Remove this sleep; the queue_notify.listen().await on the empty-queue path already handles idle waiting correctly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/event_bus.rs, line 109:
<comment>The `thread::sleep(Duration::from_millis(1))` on every loop iteration when the queue is non-empty adds 1ms of pure latency per event, exceeding the rule's 0.2ms/event/handler overhead budget by 5x. This sleep appears intended to yield the thread, but it fires on every event dequeue—even when events are waiting to be processed immediately. Remove this sleep; the `queue_notify.listen().await` on the empty-queue path already handles idle waiting correctly.</comment>
<file context>
@@ -0,0 +1,699 @@
+ thread::spawn(move || {
+ block_on(async move {
+ loop {
+ if !bus.runtime.queue.lock().is_empty() {
+ thread::sleep(Duration::from_millis(1));
+ }
</file context>
| }), | ||
| bus_serial_lock: Arc::new(ReentrantLock::default()), | ||
| }); | ||
| Self::start_loop(bus.clone()); |
Contributor
There was a problem hiding this comment.
P0: Custom agent: Make sure concurrency options work correctly and consistently
The background event-loop thread in start_loop holds an Arc<Self>, preventing the bus from ever being deallocated without an explicit stop() call. Rule 1 requires: "Make sure all memory is correctly freed / able to be garbage collected automatically without manual .stop() / .destroy() calls by the user. Most users aren't going to use the shutdown methods." With the stated use case of "millions of busses... creating and destroying busses per request", this is a severe memory leak.
Fix: Use a Weak<Self> in the thread (upgrading per iteration), and implement Drop for EventBus (or a wrapper) to set the stop flag so the thread exits when all strong references are dropped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/event_bus.rs, line 101:
<comment>The background event-loop thread in `start_loop` holds an `Arc<Self>`, preventing the bus from ever being deallocated without an explicit `stop()` call. Rule 1 requires: *"Make sure all memory is correctly freed / able to be garbage collected automatically without manual .stop() / .destroy() calls by the user. Most users aren't going to use the shutdown methods."* With the stated use case of *"millions of busses... creating and destroying busses per request"*, this is a severe memory leak.
Fix: Use a `Weak<Self>` in the thread (upgrading per iteration), and implement `Drop for EventBus` (or a wrapper) to set the stop flag so the thread exits when all strong references are dropped.</comment>
<file context>
@@ -0,0 +1,699 @@
+ }),
+ bus_serial_lock: Arc::new(ReentrantLock::default()),
+ });
+ Self::start_loop(bus.clone());
+ bus
+ }
</file context>
| let dur = SystemTime::now() | ||
| .duration_since(UNIX_EPOCH) | ||
| .unwrap_or_default(); | ||
| format!("{}.{:09}Z", dur.as_secs(), dur.subsec_nanos()) |
Contributor
There was a problem hiding this comment.
P0: Custom agent: Make sure concurrency options work correctly and consistently
now_iso() produces {epoch_secs}.{nanos}Z (e.g., 1708000000.123456789Z) instead of proper ISO 8601 format (2024-02-15T12:00:00.123456789Z). Python's monotonic_datetime() in bubus/helpers.py:32-41 uses strftime("%Y-%m-%dT%H:%M:%S").{nanos:09d}Z. This format mismatch will break any cross-language roundtrip because Python parses timestamps with datetime.fromisoformat(). The format should be YYYY-MM-DDTHH:MM:SS.nnnnnnnnnZ.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/base_event.rs, line 21:
<comment>`now_iso()` produces `{epoch_secs}.{nanos}Z` (e.g., `1708000000.123456789Z`) instead of proper ISO 8601 format (`2024-02-15T12:00:00.123456789Z`). Python's `monotonic_datetime()` in `bubus/helpers.py:32-41` uses `strftime("%Y-%m-%dT%H:%M:%S").{nanos:09d}Z`. This format mismatch will break any cross-language roundtrip because Python parses timestamps with `datetime.fromisoformat()`. The format should be `YYYY-MM-DDTHH:MM:SS.nnnnnnnnnZ`.</comment>
<file context>
@@ -0,0 +1,127 @@
+ let dur = SystemTime::now()
+ .duration_since(UNIX_EPOCH)
+ .unwrap_or_default();
+ format!("{}.{:09}Z", dur.as_secs(), dur.subsec_nanos())
+}
+
</file context>
| let event = mk_event("work"); | ||
| { | ||
| let mut inner = event.inner.lock(); | ||
| inner.event_handler_concurrency = Some(EventHandlerConcurrencyMode::Serial); |
Contributor
There was a problem hiding this comment.
P2: Test doesn't actually test bus defaults — it explicitly sets the same values on the event that the bus would use as defaults, so the unwrap_or(self.default) fallback path is never exercised. To test that bus defaults are applied, remove the block that sets event_handler_concurrency and event_handler_completion on the event, leaving them as None.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/tests/test_eventbus_dispatch_defaults.rs, line 25:
<comment>Test doesn't actually test bus defaults — it explicitly sets the same values on the event that the bus would use as defaults, so the `unwrap_or(self.default)` fallback path is never exercised. To test that bus defaults are applied, remove the block that sets `event_handler_concurrency` and `event_handler_completion` on the event, leaving them as `None`.</comment>
<file context>
@@ -0,0 +1,33 @@
+ let event = mk_event("work");
+ {
+ let mut inner = event.inner.lock();
+ inner.event_handler_concurrency = Some(EventHandlerConcurrencyMode::Serial);
+ inner.event_handler_completion = Some(EventHandlerCompletionMode::All);
+ }
</file context>
| pub fn first_result(&self) -> Option<E::Result> { | ||
| let results: HashMap<String, crate::event_result::EventResult> = | ||
| self.inner.inner.lock().event_results.clone(); | ||
| for result in results.values() { |
Contributor
There was a problem hiding this comment.
P2: first_result() iterates over HashMap::values() which has non-deterministic ordering. When multiple handlers succeed, this returns an arbitrary result rather than a predictable "first" one. Consider sorting by handler key or using an ordered collection to ensure deterministic behavior consistent with the Python/TS implementations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/typed.rs, line 54:
<comment>`first_result()` iterates over `HashMap::values()` which has non-deterministic ordering. When multiple handlers succeed, this returns an arbitrary result rather than a predictable "first" one. Consider sorting by handler key or using an ordered collection to ensure deterministic behavior consistent with the Python/TS implementations.</comment>
<file context>
@@ -0,0 +1,109 @@
+ pub fn first_result(&self) -> Option<E::Result> {
+ let results: HashMap<String, crate::event_result::EventResult> =
+ self.inner.inner.lock().event_results.clone();
+ for result in results.values() {
+ if result.error.is_none() {
+ if let Some(value) = &result.result {
</file context>
| ## Quickstart | ||
|
|
||
| ```rust | ||
| use bubus_rust::{base_event, event_bus}; |
Contributor
There was a problem hiding this comment.
P2: Quickstart example won't compile: event_bus::new(...) and base_event::new(...) are not free functions — new is an associated function on the EventBus and BaseEvent structs respectively. The imports and constructor calls need to reference the structs directly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/README.md, line 22:
<comment>Quickstart example won't compile: `event_bus::new(...)` and `base_event::new(...)` are not free functions — `new` is an associated function on the `EventBus` and `BaseEvent` structs respectively. The imports and constructor calls need to reference the structs directly.</comment>
<file context>
@@ -0,0 +1,40 @@
+## Quickstart
+
+```rust
+use bubus_rust::{base_event, event_bus};
+use futures::executor::block_on;
+use serde_json::{Map, json};
</file context>
| } | ||
| } | ||
|
|
||
| pub fn payload_map_from_value(value: Value) -> Map<String, Value> { |
Contributor
There was a problem hiding this comment.
P3: payload_map_from_value is dead code — it is defined but never called anywhere in the codebase. The same logic already exists inline in TypedEvent::new() (lines 23–26). Consider removing it or using it in new() to avoid duplication.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/src/typed.rs, line 104:
<comment>`payload_map_from_value` is dead code — it is defined but never called anywhere in the codebase. The same logic already exists inline in `TypedEvent::new()` (lines 23–26). Consider removing it or using it in `new()` to avoid duplication.</comment>
<file context>
@@ -0,0 +1,109 @@
+ }
+}
+
+pub fn payload_map_from_value(value: Value) -> Map<String, Value> {
+ match value {
+ Value::Object(map) => map,
</file context>
| serde = { version = "1", features = ["derive"] } | ||
| serde_json = "1" | ||
| uuid = { version = "1", features = ["v4", "v5", "v7", "serde"] } | ||
| futures = { version = "0.3", features = ["executor", "thread-pool"] } |
Contributor
There was a problem hiding this comment.
P3: The thread-pool feature on the futures crate is unused — only executor (for block_on) is referenced anywhere in the source. Removing it trims an unnecessary transitive dependency and compile time.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus-rust/Cargo.toml, line 10:
<comment>The `thread-pool` feature on the `futures` crate is unused — only `executor` (for `block_on`) is referenced anywhere in the source. Removing it trims an unnecessary transitive dependency and compile time.</comment>
<file context>
@@ -0,0 +1,18 @@
+serde = { version = "1", features = ["derive"] }
+serde_json = "1"
+uuid = { version = "1", features = ["v4", "v5", "v7", "serde"] }
+futures = { version = "0.3", features = ["executor", "thread-pool"] }
+event-listener = "5"
+parking_lot = "0.12"
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings in
bubus-rustfromcodex/implement-bubus-rust-equivalentand adds cross-runtime roundtrip tests between Python and Rust, matching the existing Python ↔ TypeScript roundtrip test pattern.Changes
bubus-rust/src/bin/roundtrip_events.rs— Rust binary that reads events JSON viaBUBUS_RUST_INPUT_PATH, round-trips throughBaseEvent::from_json_value→to_json_value, writes toBUBUS_RUST_OUTPUT_PATHbubus-rust/src/base_event.rs— Added#[serde(default)]onevent_resultsso Rust can deserialize Python events that omit the field when emptytests/test_cross_runtime_roundtrip.py— Two new tests:test_python_to_rust_roundtrip_preserves_event_fields_and_result_type_semantics— all 11 event/result-type combinations survive Python→Rust→Python with field equality and JSON Schema semantic equivalencetest_python_to_rust_roundtrip_schema_enforcement_after_reload— wrong-shape payloads are rejected, correct-shape payloads accepted after Rust roundtrip reload.gitignore—bubus-rust/target/Usage
Requires a one-time build before running cross-runtime tests:
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by cubic
Adds a Python↔Rust roundtrip test using a new Rust crate and binary to ensure event JSON stays consistent across runtimes. Also confirms the result schema is preserved and enforced when Python reloads Rust-produced events.
Written for commit 4de5e7b. Summary will update on new commits.