fix(events): add unsubscribe() to EventBus protocol and implementations (#10356) #11235

Closed
HAL9000 wants to merge 1 commits from agents/fix-eventbus-unsubscribe into master
4 changed files with 90 additions and 0 deletions
+1
View File
@@ -267,6 +267,7 @@ def step_protocol_stubs_callable(ctx: Context) -> None:
Review

Test Quality — BLOCKING

This PR adds unsubscribe() to the codebase but does not include any Behave BDD scenarios (Gherkin Given/When/Then) for it. Only a stub exercise line was added in step_protocol_stubs_callable(), which is insufficient.

Per project contributing guidelines: "Are there Behave BDD scenarios for all new behavior?" — this must be answered affirmatively.

Required scenarios:

  • subscribe then unsubscribe → returns True, handler no longer fires
  • unsubscribe from non-existent event type → returns False
  • unsubscribe with handler never registered → returns False
  • TypeError when event_type is not EventType (matching subscribe behavior)
  • TypeError when handler is not callable (matching subscribe behavior)

Please create a new feature file or add to the existing event_bus.feature:

Feature: EventBus unsubscribe
  As a developer using EventBus
  I want to unsubscribe handlers from specific event types
  So that I can manage subscription lifecycle.
  
  @tdd_issue_10356
  Scenario: Unsubscribe returns True and stops handler execution
    Given a ReactiveEventBus
    When I subscribe two handlers to "plan.created" events
    And the plan has not been created yet
    Then handler 0 should have received 0 event
    When I unsubscribe "plan.created" handler 0
    Then unsubscribe returns True
    When I emit a "plan.created" DomainEvent
    Then handler 0 should have received 0 event
    And handler 1 should have received 1 event

Suggestion: Use the @tdd_issue_10356 tag since this tests issue #10356.

## Test Quality — BLOCKING ❌ This PR adds `unsubscribe()` to the codebase but does not include any Behave BDD scenarios (Gherkin Given/When/Then) for it. Only a stub exercise line was added in step_protocol_stubs_callable(), which is insufficient. Per project contributing guidelines: "Are there Behave BDD scenarios for all new behavior?" — this must be answered affirmatively. **Required scenarios:** - subscribe then unsubscribe → returns True, handler no longer fires - unsubscribe from non-existent event type → returns False - unsubscribe with handler never registered → returns False - TypeError when event_type is not EventType (matching subscribe behavior) - TypeError when handler is not callable (matching subscribe behavior) Please create a new feature file or add to the existing event_bus.feature: ``` Feature: EventBus unsubscribe As a developer using EventBus I want to unsubscribe handlers from specific event types So that I can manage subscription lifecycle. @tdd_issue_10356 Scenario: Unsubscribe returns True and stops handler execution Given a ReactiveEventBus When I subscribe two handlers to "plan.created" events And the plan has not been created yet Then handler 0 should have received 0 event When I unsubscribe "plan.created" handler 0 Then unsubscribe returns True When I emit a "plan.created" DomainEvent Then handler 0 should have received 0 event And handler 1 should have received 1 event ``` Suggestion: Use the @tdd_issue_10356 tag since this tests issue #10356.
Review

Question: The stub exercise in step_protocol_stubs_callable() adds a single unsubscribe call to ensure the protocol stub is callable. However, this does not test actual unsubscribe behavior (removing from subscription list, return value, etc.). Could you add dedicated Behave scenarios that exercise the full subscribe/unsubscribe lifecycle with real instances (ReactiveEventBus/LoggingEventBus), not just protocol stubs? A stub call alone leaves the new behavior largely untested.

**Question:** The stub exercise in step_protocol_stubs_callable() adds a single unsubscribe call to ensure the protocol stub is callable. However, this does not test actual unsubscribe behavior (removing from subscription list, return value, etc.). Could you add dedicated Behave scenarios that exercise the full subscribe/unsubscribe lifecycle with real instances (ReactiveEventBus/LoggingEventBus), not just protocol stubs? A stub call alone leaves the new behavior largely untested.
Review

Test Quality — BLOCKING

This PR adds unsubscribe() to the codebase but does not include any Behave BDD scenarios (Gherkin Given/When/Then) for it. Only a stub exercise line was added in step_protocol_stubs_callable(), which is insufficient.

Per project contributing guidelines: "Are there Behave BDD scenarios for all new behavior?" — this must be answered affirmatively.

Required scenarios:

  • subscribe then unsubscribe → returns True, handler no longer fires
  • unsubscribe from non-existent event type → returns False
  • unsubscribe with handler never registered → returns False
  • TypeError when event_type is not EventType (matching subscribe behavior)
  • TypeError when handler is not callable (matching subscribe behavior)

Please create a new feature file or add to the existing event_bus.feature:

Feature: EventBus unsubscribe
  As a developer using EventBus
  I want to unsubscribe handlers from specific event types
  So that I can manage subscription lifecycle.
  
  @tdd_issue_10356
  Scenario: Unsubscribe returns True and stops handler execution
    Given a ReactiveEventBus
    When I subscribe two handlers to "plan.created" events
    And the plan has not been created yet
    Then handler 0 should have received 0 event
    When I unsubscribe "plan.created" handler 0
    Then unsubscribe returns True
    When I emit a "plan.created" DomainEvent
    Then handler 0 should have received 0 event
    And handler 1 should have received 1 event

Suggestion: Use the @tdd_issue_10356 tag since this tests issue #10356.

## Test Quality — BLOCKING ❌ This PR adds `unsubscribe()` to the codebase but does not include any Behave BDD scenarios (Gherkin Given/When/Then) for it. Only a stub exercise line was added in step_protocol_stubs_callable(), which is insufficient. Per project contributing guidelines: "Are there Behave BDD scenarios for all new behavior?" — this must be answered affirmatively. **Required scenarios:** - subscribe then unsubscribe → returns True, handler no longer fires - unsubscribe from non-existent event type → returns False - unsubscribe with handler never registered → returns False - TypeError when event_type is not EventType (matching subscribe behavior) - TypeError when handler is not callable (matching subscribe behavior) Please create a new feature file or add to the existing event_bus.feature: ``` Feature: EventBus unsubscribe As a developer using EventBus I want to unsubscribe handlers from specific event types So that I can manage subscription lifecycle. @tdd_issue_10356 Scenario: Unsubscribe returns True and stops handler execution Given a ReactiveEventBus When I subscribe two handlers to "plan.created" events And the plan has not been created yet Then handler 0 should have received 0 event When I unsubscribe "plan.created" handler 0 Then unsubscribe returns True When I emit a "plan.created" DomainEvent Then handler 0 should have received 0 event And handler 1 should have received 1 event ``` Suggestion: Use the @tdd_issue_10356 tag since this tests issue #10356.
Review

Question: The stub exercise in step_protocol_stubs_callable() adds a single unsubscribe call to ensure the protocol stub is callable. However, this does not test actual unsubscribe behavior (removing from subscription list, return value, etc.). Could you add dedicated Behave scenarios that exercise the full subscribe/unsubscribe lifecycle with real instances (ReactiveEventBus/LoggingEventBus), not just protocol stubs? A stub call alone leaves the new behavior largely untested.

**Question:** The stub exercise in step_protocol_stubs_callable() adds a single unsubscribe call to ensure the protocol stub is callable. However, this does not test actual unsubscribe behavior (removing from subscription list, return value, etc.). Could you add dedicated Behave scenarios that exercise the full subscribe/unsubscribe lifecycle with real instances (ReactiveEventBus/LoggingEventBus), not just protocol stubs? A stub call alone leaves the new behavior largely untested.
Review

Non-blocking: The _BareImpl Protocol stub passes for all methods (since Protocol structural typing only checks interface shape, not method bodies). Subscribing a new lambda then unsubscricing it won't exercise the removal logic — consider subscribing a named handler function first, then unsubscribing it and verifying True is returned.

Non-blocking: The `_BareImpl` Protocol stub passes for all methods (since Protocol structural typing only checks interface shape, not method bodies). Subscribing a new lambda then unsubscricing it won't exercise the removal logic — consider subscribing a named handler function first, then unsubscribing it and verifying `True` is returned.
Review

Non-blocking: The _BareImpl Protocol stub passes for all methods (since Protocol structural typing only checks interface shape, not method bodies). Subscribing a new lambda then unsubscricing it won't exercise the removal logic — consider subscribing a named handler function first, then unsubscribing it and verifying True is returned.

Non-blocking: The `_BareImpl` Protocol stub passes for all methods (since Protocol structural typing only checks interface shape, not method bodies). Subscribing a new lambda then unsubscricing it won't exercise the removal logic — consider subscribing a named handler function first, then unsubscribing it and verifying `True` is returned.
event = _make_event("plan.created")
obj.emit(event)
obj.subscribe(EventType.PLAN_CREATED, lambda e: None)
Review

The only unsubscribe() call in this file is the protocol stub exercise (line near 269) calling obj.unsubscribe(EventType.PLAN_CREATED, lambda e: None) on a bare _BareImpl subclass. This provides no meaningful behavioral verification.

Consider adding:
result = obj.unsubscribe(EventType.PLAN_CREATED, handler)
assert result is False # No subscriptions exist


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

The only unsubscribe() call in this file is the protocol stub exercise (line near 269) calling obj.unsubscribe(EventType.PLAN_CREATED, lambda e: None) on a bare _BareImpl subclass. This provides no meaningful behavioral verification. Consider adding: result = obj.unsubscribe(EventType.PLAN_CREATED, handler) assert result is False # No subscriptions exist --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
obj.unsubscribe(EventType.PLAN_CREATED, lambda e: None)
Review

Test Quality — BLOCKING

This PR adds unsubscribe() to the codebase but does not include any Behave BDD scenarios (Gherkin Given/When/Then) for it. Only a stub exercise line was added in step_protocol_stubs_callable(), which is insufficient.

Per project contributing guidelines: "Are there Behave BDD scenarios for all new behavior?" — this must be answered affirmatively.

Required scenarios:

  • subscribe then unsubscribe → returns True, handler no longer fires
  • unsubscribe from non-existent event type → returns False
  • unsubscribe with handler never registered → returns False
  • TypeError when event_type is not EventType (matching subscribe behavior)
  • TypeError when handler is not callable (matching subscribe behavior)

Please create a new feature file or add to the existing event_bus.feature:

Feature: EventBus unsubscribe
  As a developer using EventBus
  I want to unsubscribe handlers from specific event types
  So that I can manage subscription lifecycle.
  
  @tdd_issue_10356
  Scenario: Unsubscribe returns True and stops handler execution
    Given a ReactiveEventBus
    When I subscribe two handlers to "plan.created" events
    And the plan has not been created yet
    Then handler 0 should have received 0 event
    When I unsubscribe "plan.created" handler 0
    Then unsubscribe returns True
    When I emit a "plan.created" DomainEvent
    Then handler 0 should have received 0 event
    And handler 1 should have received 1 event

Suggestion: Use the @tdd_issue_10356 tag since this tests issue #10356.

## Test Quality — BLOCKING ❌ This PR adds `unsubscribe()` to the codebase but does not include any Behave BDD scenarios (Gherkin Given/When/Then) for it. Only a stub exercise line was added in step_protocol_stubs_callable(), which is insufficient. Per project contributing guidelines: "Are there Behave BDD scenarios for all new behavior?" — this must be answered affirmatively. **Required scenarios:** - subscribe then unsubscribe → returns True, handler no longer fires - unsubscribe from non-existent event type → returns False - unsubscribe with handler never registered → returns False - TypeError when event_type is not EventType (matching subscribe behavior) - TypeError when handler is not callable (matching subscribe behavior) Please create a new feature file or add to the existing event_bus.feature: ``` Feature: EventBus unsubscribe As a developer using EventBus I want to unsubscribe handlers from specific event types So that I can manage subscription lifecycle. @tdd_issue_10356 Scenario: Unsubscribe returns True and stops handler execution Given a ReactiveEventBus When I subscribe two handlers to "plan.created" events And the plan has not been created yet Then handler 0 should have received 0 event When I unsubscribe "plan.created" handler 0 Then unsubscribe returns True When I emit a "plan.created" DomainEvent Then handler 0 should have received 0 event And handler 1 should have received 1 event ``` Suggestion: Use the @tdd_issue_10356 tag since this tests issue #10356.
Review

Question: The stub exercise in step_protocol_stubs_callable() adds a single unsubscribe call to ensure the protocol stub is callable. However, this does not test actual unsubscribe behavior (removing from subscription list, return value, etc.). Could you add dedicated Behave scenarios that exercise the full subscribe/unsubscribe lifecycle with real instances (ReactiveEventBus/LoggingEventBus), not just protocol stubs? A stub call alone leaves the new behavior largely untested.

**Question:** The stub exercise in step_protocol_stubs_callable() adds a single unsubscribe call to ensure the protocol stub is callable. However, this does not test actual unsubscribe behavior (removing from subscription list, return value, etc.). Could you add dedicated Behave scenarios that exercise the full subscribe/unsubscribe lifecycle with real instances (ReactiveEventBus/LoggingEventBus), not just protocol stubs? A stub call alone leaves the new behavior largely untested.
Review

Non-blocking: The _BareImpl Protocol stub passes for all methods (since Protocol structural typing only checks interface shape, not method bodies). Subscribing a new lambda then unsubscricing it won't exercise the removal logic — consider subscribing a named handler function first, then unsubscribing it and verifying True is returned.

Non-blocking: The `_BareImpl` Protocol stub passes for all methods (since Protocol structural typing only checks interface shape, not method bodies). Subscribing a new lambda then unsubscricing it won't exercise the removal logic — consider subscribing a named handler function first, then unsubscribing it and verifying `True` is returned.
Review

BLOCKING - The _BareImpl(EventBus) class does not implement unsubscribe() (or any other Protocol methods). Calling obj.unsubscribe() on it will raise an error because Ellipsis (...) is not callable.

Fix: Add stubs for all Protocol methods:

class _BareImpl(EventBus):
def emit(self, event): ...
def subscribe(self, event_type, handler): ...
def unsubscribe(self, event_type, handler) -> bool: ...

BLOCKING - The _BareImpl(EventBus) class does not implement unsubscribe() (or any other Protocol methods). Calling obj.unsubscribe() on it will raise an error because Ellipsis (...) is not callable. Fix: Add stubs for all Protocol methods: class _BareImpl(EventBus): def emit(self, event): ... def subscribe(self, event_type, handler): ... def unsubscribe(self, event_type, handler) -> bool: ...
Review

BLOCKING - No Behave scenarios verify the core behavior of unsubscribe(). The acceptance criteria from issue #10356 require:

  • Unsubscribed handlers are not called on subsequent emit() calls
  • Unsubscribing a non-registered handler is a no-op (no exception)

Suggestion: Consider adding a new feature or extending event_bus.feature with Gherkin scenarios exercising these behaviors using the existing _EventCollector and ctx.bus patterns.

BLOCKING - No Behave scenarios verify the core behavior of unsubscribe(). The acceptance criteria from issue #10356 require: - Unsubscribed handlers are not called on subsequent emit() calls - Unsubscribing a non-registered handler is a no-op (no exception) Suggestion: Consider adding a new feature or extending event_bus.feature with Gherkin scenarios exercising these behaviors using the existing _EventCollector and ctx.bus patterns.
# ---------------------------------------------------------------------------
Review

Test Quality — BLOCKING

This PR adds unsubscribe() to the codebase but does not include any Behave BDD scenarios (Gherkin Given/When/Then) for it. Only a stub exercise line was added in step_protocol_stubs_callable(), which is insufficient.

Per project contributing guidelines: "Are there Behave BDD scenarios for all new behavior?" — this must be answered affirmatively.

Required scenarios:

  • subscribe then unsubscribe → returns True, handler no longer fires
  • unsubscribe from non-existent event type → returns False
  • unsubscribe with handler never registered → returns False
  • TypeError when event_type is not EventType (matching subscribe behavior)
  • TypeError when handler is not callable (matching subscribe behavior)

Please create a new feature file or add to the existing event_bus.feature:

Feature: EventBus unsubscribe
  As a developer using EventBus
  I want to unsubscribe handlers from specific event types
  So that I can manage subscription lifecycle.
  
  @tdd_issue_10356
  Scenario: Unsubscribe returns True and stops handler execution
    Given a ReactiveEventBus
    When I subscribe two handlers to "plan.created" events
    And the plan has not been created yet
    Then handler 0 should have received 0 event
    When I unsubscribe "plan.created" handler 0
    Then unsubscribe returns True
    When I emit a "plan.created" DomainEvent
    Then handler 0 should have received 0 event
    And handler 1 should have received 1 event

Suggestion: Use the @tdd_issue_10356 tag since this tests issue #10356.

## Test Quality — BLOCKING ❌ This PR adds `unsubscribe()` to the codebase but does not include any Behave BDD scenarios (Gherkin Given/When/Then) for it. Only a stub exercise line was added in step_protocol_stubs_callable(), which is insufficient. Per project contributing guidelines: "Are there Behave BDD scenarios for all new behavior?" — this must be answered affirmatively. **Required scenarios:** - subscribe then unsubscribe → returns True, handler no longer fires - unsubscribe from non-existent event type → returns False - unsubscribe with handler never registered → returns False - TypeError when event_type is not EventType (matching subscribe behavior) - TypeError when handler is not callable (matching subscribe behavior) Please create a new feature file or add to the existing event_bus.feature: ``` Feature: EventBus unsubscribe As a developer using EventBus I want to unsubscribe handlers from specific event types So that I can manage subscription lifecycle. @tdd_issue_10356 Scenario: Unsubscribe returns True and stops handler execution Given a ReactiveEventBus When I subscribe two handlers to "plan.created" events And the plan has not been created yet Then handler 0 should have received 0 event When I unsubscribe "plan.created" handler 0 Then unsubscribe returns True When I emit a "plan.created" DomainEvent Then handler 0 should have received 0 event And handler 1 should have received 1 event ``` Suggestion: Use the @tdd_issue_10356 tag since this tests issue #10356.
Review

Question: The stub exercise in step_protocol_stubs_callable() adds a single unsubscribe call to ensure the protocol stub is callable. However, this does not test actual unsubscribe behavior (removing from subscription list, return value, etc.). Could you add dedicated Behave scenarios that exercise the full subscribe/unsubscribe lifecycle with real instances (ReactiveEventBus/LoggingEventBus), not just protocol stubs? A stub call alone leaves the new behavior largely untested.

**Question:** The stub exercise in step_protocol_stubs_callable() adds a single unsubscribe call to ensure the protocol stub is callable. However, this does not test actual unsubscribe behavior (removing from subscription list, return value, etc.). Could you add dedicated Behave scenarios that exercise the full subscribe/unsubscribe lifecycle with real instances (ReactiveEventBus/LoggingEventBus), not just protocol stubs? A stub call alone leaves the new behavior largely untested.
Review

Test Quality — BLOCKING

This PR adds unsubscribe() to the codebase but does not include any Behave BDD scenarios (Gherkin Given/When/Then) for it. Only a stub exercise line was added in step_protocol_stubs_callable(), which is insufficient.

Per project contributing guidelines: "Are there Behave BDD scenarios for all new behavior?" — this must be answered affirmatively.

Required scenarios:

  • subscribe then unsubscribe → returns True, handler no longer fires
  • unsubscribe from non-existent event type → returns False
  • unsubscribe with handler never registered → returns False
  • TypeError when event_type is not EventType (matching subscribe behavior)
  • TypeError when handler is not callable (matching subscribe behavior)

Please create a new feature file or add to the existing event_bus.feature:

Feature: EventBus unsubscribe
  As a developer using EventBus
  I want to unsubscribe handlers from specific event types
  So that I can manage subscription lifecycle.
  
  @tdd_issue_10356
  Scenario: Unsubscribe returns True and stops handler execution
    Given a ReactiveEventBus
    When I subscribe two handlers to "plan.created" events
    And the plan has not been created yet
    Then handler 0 should have received 0 event
    When I unsubscribe "plan.created" handler 0
    Then unsubscribe returns True
    When I emit a "plan.created" DomainEvent
    Then handler 0 should have received 0 event
    And handler 1 should have received 1 event

Suggestion: Use the @tdd_issue_10356 tag since this tests issue #10356.

## Test Quality — BLOCKING ❌ This PR adds `unsubscribe()` to the codebase but does not include any Behave BDD scenarios (Gherkin Given/When/Then) for it. Only a stub exercise line was added in step_protocol_stubs_callable(), which is insufficient. Per project contributing guidelines: "Are there Behave BDD scenarios for all new behavior?" — this must be answered affirmatively. **Required scenarios:** - subscribe then unsubscribe → returns True, handler no longer fires - unsubscribe from non-existent event type → returns False - unsubscribe with handler never registered → returns False - TypeError when event_type is not EventType (matching subscribe behavior) - TypeError when handler is not callable (matching subscribe behavior) Please create a new feature file or add to the existing event_bus.feature: ``` Feature: EventBus unsubscribe As a developer using EventBus I want to unsubscribe handlers from specific event types So that I can manage subscription lifecycle. @tdd_issue_10356 Scenario: Unsubscribe returns True and stops handler execution Given a ReactiveEventBus When I subscribe two handlers to "plan.created" events And the plan has not been created yet Then handler 0 should have received 0 event When I unsubscribe "plan.created" handler 0 Then unsubscribe returns True When I emit a "plan.created" DomainEvent Then handler 0 should have received 0 event And handler 1 should have received 1 event ``` Suggestion: Use the @tdd_issue_10356 tag since this tests issue #10356.
Review

Question: The stub exercise in step_protocol_stubs_callable() adds a single unsubscribe call to ensure the protocol stub is callable. However, this does not test actual unsubscribe behavior (removing from subscription list, return value, etc.). Could you add dedicated Behave scenarios that exercise the full subscribe/unsubscribe lifecycle with real instances (ReactiveEventBus/LoggingEventBus), not just protocol stubs? A stub call alone leaves the new behavior largely untested.

**Question:** The stub exercise in step_protocol_stubs_callable() adds a single unsubscribe call to ensure the protocol stub is callable. However, this does not test actual unsubscribe behavior (removing from subscription list, return value, etc.). Could you add dedicated Behave scenarios that exercise the full subscribe/unsubscribe lifecycle with real instances (ReactiveEventBus/LoggingEventBus), not just protocol stubs? A stub call alone leaves the new behavior largely untested.
Review

Non-blocking: The _BareImpl Protocol stub passes for all methods (since Protocol structural typing only checks interface shape, not method bodies). Subscribing a new lambda then unsubscricing it won't exercise the removal logic — consider subscribing a named handler function first, then unsubscribing it and verifying True is returned.

Non-blocking: The `_BareImpl` Protocol stub passes for all methods (since Protocol structural typing only checks interface shape, not method bodies). Subscribing a new lambda then unsubscricing it won't exercise the removal logic — consider subscribing a named handler function first, then unsubscribing it and verifying `True` is returned.
Review

Non-blocking: The _BareImpl Protocol stub passes for all methods (since Protocol structural typing only checks interface shape, not method bodies). Subscribing a new lambda then unsubscricing it won't exercise the removal logic — consider subscribing a named handler function first, then unsubscribing it and verifying True is returned.

Non-blocking: The `_BareImpl` Protocol stub passes for all methods (since Protocol structural typing only checks interface shape, not method bodies). Subscribing a new lambda then unsubscricing it won't exercise the removal logic — consider subscribing a named handler function first, then unsubscribing it and verifying `True` is returned.
@@ -105,5 +105,38 @@ class LoggingEventBus:
raise TypeError("handler must be callable")
self._subscriptions.setdefault(event_type, []).append(handler)
def unsubscribe(
self,
event_type: EventType,
handler: Callable[[DomainEvent], None],
) -> bool:
"""Remove *handler* from events of *event_type*.
Removes only the **first** occurrence of *handler* (in case it was
registered multiple times). Returns ``True`` if a matching handler
was found and removed, ``False`` otherwise.
Args:
event_type: The :class:`EventType` whose handler list to modify.
handler: The previously-registered callable to remove.
Returns:
``True`` if a handler was removed; ``False`` if no match found.
"""
if not isinstance(event_type, EventType):
raise TypeError(
f"event_type must be an EventType, got {type(event_type).__name__!r}"
)
if not callable(handler):
raise TypeError("handler must be callable")
handlers = self._subscriptions.get(event_type)
if handlers is None:
return False
try:
handlers.remove(handler)
return True
except ValueError:
return False
__all__ = ["LoggingEventBus"]
@@ -49,5 +49,26 @@ class EventBus(Protocol):
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
"""
...
def unsubscribe(
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
self,
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
event_type: EventType,
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
handler: Callable[[DomainEvent], None],
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
) -> bool:
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
Review

SUGGESTION - For consistency with subscribe() which documents Raises, consider adding the same Raises section to the Protocol stub docstring. The implementations already do this.

SUGGESTION - For consistency with subscribe() which documents Raises, consider adding the same Raises section to the Protocol stub docstring. The implementations already do this.
"""Remove *handler* from the subscriber list for *event_type*.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
Removes only the **first** occurrence of *handler* in the list for
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
*event_type* (in case it was registered multiple times). Returns
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
``True`` if a matching handler was found and removed, ``False``
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
otherwise.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
Args:
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
event_type: The :class:`EventType` whose handler list to modify.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
handler: The previously-registered callable to remove.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
Returns:
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
``True`` if a handler was removed; ``False`` if no match found.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
"""
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
...
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
__all__ = ["EventBus"]
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
Review

Suggestion: Consider adding a RemovedOnceOnly behavioral note to align the Protocol docstring with what both implementations do.

Suggestion: Consider adding a `RemovedOnceOnly` behavioral note to align the Protocol docstring with what both implementations do.
@@ -184,6 +184,41 @@ class ReactiveEventBus:
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
"""
return self._stream
def unsubscribe(
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
self,
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
event_type: EventType,
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
handler: Callable[[DomainEvent], None],
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
) -> bool:
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
"""Remove *handler* from events of *event_type*.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
Removes only the **first** occurrence of *handler* (in case it was
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
registered multiple times). Returns ``True`` if a matching handler
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
was found and removed, ``False`` otherwise.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
Args:
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
event_type: The :class:`EventType` whose handler list to modify.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
handler: The previously-registered callable to remove.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
Returns:
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
``True`` if a handler was removed; ``False`` if no match found.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
"""
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
if not isinstance(event_type, EventType):
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
raise TypeError(
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
f"event_type must be an EventType, got {type(event_type).__name__!r}"
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
)
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
if not callable(handler):
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
raise TypeError("handler must be callable")
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
handlers = self._subscriptions.get(event_type)
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
if handlers is None:
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
return False
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
try:
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
handlers.remove(handler)
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
return True
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
except ValueError:
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
# Handler was not found in list (duplicates registered but
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
# this specific reference is not present).
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
return False
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
def close(self) -> None:
"""Complete the RxPY stream and prevent further event emission.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.
Review

Note: The close() method clears all subscriptions before this could be called. If a user calls unsubscribe() after close(), it returns False since the subscription list has already been cleared — this is reasonable but worth documenting in the close() docstring if not already.

Note: The `close()` method clears all subscriptions before this could be called. If a user calls `unsubscribe()` after `close()`, it returns `False` since the subscription list has already been cleared — this is reasonable but worth documenting in the `close()` docstring if not already.