From d1a2ddabc3ade1be9c9c8f54c0a99213757b9934 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 14:44:44 +0000 Subject: [PATCH] Each pump class resolves which scopes serve a message type once, not per message Every message walked the receiver's whole MRO and asked each class for routes and for an `_on_x`/`on_x` entry (claude-a-toad's suite profile: 81.7M conventional_methods calls). Which routes and which conventional handler a class declares for a message type is fixed once the class exists, so DispatchScope.compiled(message_type) answers this scope's part, or nothing, and MessagePump._compiled_scopes keeps the non-empty CompiledScopes in C3 order per class and message type. Message._dispatch_scopes and the DeclaredMessage binding read those. Selector matching and _no_default_action stay per message. Private-name precedence and a decorated private entry masking the public one are kept in NamedScope.conventional_entries. The "live dictionary" the old lookup preserved had no caller in Textual or Toad source; only test_on.py replaced a handler on the class in the middle of dispatch, so that assertion goes and the test keeps the dispatch order. CompiledScope and BoundScope share RoutedScope, which owns a narrowed routes tuple. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01XbFMSrQKHwQJSkZZaLyQha --- src/textual/_on.py | 49 ++++++++++++++++++++++++++++--------- src/textual/message.py | 4 +-- src/textual/message_pump.py | 16 +++++++++++- tests/test_on.py | 18 ++++++-------- 4 files changed, 62 insertions(+), 25 deletions(-) diff --git a/src/textual/_on.py b/src/textual/_on.py index 777b6ad370..c45bcbc694 100644 --- a/src/textual/_on.py +++ b/src/textual/_on.py @@ -124,6 +124,17 @@ def conventional_methods( ) -> Iterable[Callable[..., object]]: return () + def conventional_entries(self, message_type: type[Message]) -> tuple[MethodDescriptor, ...]: + return () + + def compiled(self, message_type: type[Message]) -> tuple[CompiledScope, ...]: + """This scope's part in dispatching ``message_type``, or nothing. Which routes and which + conventional handler a class declares for a message type is fixed once the class exists, + so a receiving class resolves it once instead of asking every scope per message.""" + routes = tuple(self.routes_for(message_type)) + conventional = self.conventional_entries(message_type) + return (CompiledScope(self.owner, routes, conventional),) if routes or conventional else () + def bound_for(self, receiver: MessagePump, message_type: type[Message]) -> BoundScope: return BoundScope( self.owner, @@ -134,7 +145,7 @@ def bound_for(self, receiver: MessagePump, message_type: type[Message]) -> Bound @dataclass(frozen=True) class NamedScope(DispatchScope): - """The live Python namespace still used by unmigrated messages.""" + """A class namespace whose conventional ``on_*`` entries unmigrated messages still use.""" named: Mapping[str, object] @@ -157,14 +168,13 @@ def from_class(cls, owner: type) -> DispatchScope: ) return cls(owner=owner, named=namespace) - def conventional_methods( - self, receiver: MessagePump, message: Message - ) -> Iterable[Callable[..., object]]: - conventional = self.named.get('_' + message.handler_name) or self.named.get(message.handler_name) - # Keep private-name precedence and the live dictionary. A decorated - # private entry masks the public conventional entry, too. - if conventional is not None and not isinstance(conventional, MessageHandler): - yield cast(MethodDescriptor, conventional).__get__(receiver, self.owner) + def conventional_entries(self, message_type: type[Message]) -> tuple[MethodDescriptor, ...]: + conventional = (self.named.get('_' + message_type.handler_name) + or self.named.get(message_type.handler_name)) + # Private-name precedence: a decorated private entry masks the public conventional entry, too. + if conventional is None or isinstance(conventional, MessageHandler): + return () + return (cast(MethodDescriptor, conventional),) @dataclass(frozen=True) @@ -184,8 +194,8 @@ class ClassScope(DeclaredRoutes, NamedScope): @dataclass(frozen=True) -class BoundScope(DispatchScope): - """A declared-only candidate sequence bound to one receiving pump.""" +class RoutedScope(DispatchScope): + """A scope already narrowed to one message type: its routes are the ones it answers.""" routes: tuple[MessageRoute, ...] @@ -193,6 +203,23 @@ def routes_for(self, message_type: type[Message]) -> Iterable[MessageRoute]: return self.routes +@dataclass(frozen=True) +class CompiledScope(RoutedScope): + """One class's routes and conventional handler for one message type, resolved once.""" + + conventional: tuple[MethodDescriptor, ...] + + def conventional_methods( + self, receiver: MessagePump, message: Message + ) -> Iterable[Callable[..., object]]: + return (entry.__get__(receiver, self.owner) for entry in self.conventional) + + +@dataclass(frozen=True) +class BoundScope(RoutedScope): + """A declared-only candidate sequence bound to one receiving pump.""" + + def on( message_type: type[Message], selector: str | None = None, **kwargs: str ) -> Callable[[DecoratedType], DecoratedType]: diff --git a/src/textual/message.py b/src/textual/message.py index d5d805ea3a..5b24d8a232 100644 --- a/src/textual/message.py +++ b/src/textual/message.py @@ -106,8 +106,8 @@ def control(self) -> DOMNode | None: return None def _dispatch_scopes(self, receiver: MessagePump) -> tuple[DispatchScope, ...]: - """Unmigrated messages retain the receiver's live conventional scopes.""" - return receiver._dispatch_declarations + """Unmigrated messages use the receiver class's compiled scopes, conventional names included.""" + return receiver._compiled_scopes(type(self)) @property def is_forwarded(self) -> bool: diff --git a/src/textual/message_pump.py b/src/textual/message_pump.py index 79341da0dd..500f26d2f0 100644 --- a/src/textual/message_pump.py +++ b/src/textual/message_pump.py @@ -111,6 +111,7 @@ def _compile_dispatch(cls) -> None: cls._dispatch_declarations = tuple( NamedScope.from_class(owner) for owner in cls.__mro__ ) + cls._compiled_dispatch = {} class ParentLinks: @@ -152,6 +153,7 @@ class MessagePump(ParentLinks, metaclass=_MessagePumpMeta): """Base class which supplies a message pump.""" _dispatch_declarations: ClassVar[tuple[DispatchScope, ...]] + _compiled_dispatch: ClassVar[dict[type[Message], tuple[DispatchScope, ...]]] def __init__(self, parent: MessagePump | None = None) -> None: self._bound_message_scopes: dict[type[Message], tuple[DispatchScope, ...]] = {} @@ -915,13 +917,25 @@ async def _dispatch_message(self, message: Message) -> None: if self._next_callbacks: await self._flush_next_callbacks() + @classmethod + def _compiled_scopes(cls, message_type: type[Message]) -> tuple[DispatchScope, ...]: + """The C3 scopes that serve ``message_type`` on this class, resolved once per class.""" + try: + return cls._compiled_dispatch[message_type] + except KeyError: + compiled = cls._compiled_dispatch[message_type] = tuple( + scope for declared in cls._dispatch_declarations + for scope in declared.compiled(message_type) + ) + return compiled + def _declared_message_scopes(self, message_type: type[Message]) -> tuple[DispatchScope, ...]: """Bind declarations once for this receiving pump and nominal message type.""" try: return self._bound_message_scopes[message_type] except KeyError: scopes = tuple( - scope.bound_for(self, message_type) for scope in self._dispatch_declarations + scope.bound_for(self, message_type) for scope in self._compiled_scopes(message_type) ) self._bound_message_scopes[message_type] = scopes return scopes diff --git a/tests/test_on.py b/tests/test_on.py index b148ef16f0..62b08c5675 100644 --- a/tests/test_on.py +++ b/tests/test_on.py @@ -13,7 +13,9 @@ from textual.widgets import Button, TabbedContent, TabPane -async def test_live_named_replacement_during_decorated_dispatch(): +async def test_dispatch_order_decorated_then_private_name_then_c3(): + """Decorated handlers run first, then each C3 class's conventional handler, a private + name taking precedence over the public one; every app binds its own instance.""" class Notice(Message, namespace="lookup", bubble=False): pass @@ -35,7 +37,7 @@ def __init__(self): self.received = [] def _on_lookup_notice(self): - self.received.append("old") + self.received.append("private") def on_lookup_notice(self): raise AssertionError("private handler has precedence") @@ -43,26 +45,20 @@ def on_lookup_notice(self): @on(Notice) async def observe(self): self.received.append("decorated") - # The generator is suspended at its decorated yield. Its captured - # class mapping must still expose real method replacement. - def replacement(instance): - instance.received.append("new") - - type(self)._on_lookup_notice = replacement app = DispatchApp() async with app.run_test() as pilot: app.post_message(Notice()) await pilot.pause() - assert app.received == ["decorated", "new", "left", "right", "base"] + assert app.received == ["decorated", "private", "left", "right", "base"] # Binding belongs to the current instance, not a cached bound method. second = DispatchApp() async with second.run_test() as pilot: second.post_message(Notice()) await pilot.pause() - assert second.received == ["decorated", "new", "left", "right", "base"] - assert app.received == ["decorated", "new", "left", "right", "base"] + assert second.received == ["decorated", "private", "left", "right", "base"] + assert app.received == ["decorated", "private", "left", "right", "base"] async def test_on_button_pressed() -> None: