Browse Source

Review leftovers: room-kind helper, rename, except merge

- _room_kind_for() resolves a room's kind from cache or GetRoom,
  fulfilling the old Todo comment; the user-cache branch collapses
  into one guard clause, dropping the runtime-dead but type-load-bearing
  second None check.
- _is_respond_room renamed to _answers_in_room: the name outlived the
  CHATTO_RESPOND_ROOMS config it was coined for.
- send()'s identical ChattoError/Exception branches merge into one.
- _mark_seen/_remember_dispatched drop the first element with del
  instead of remove(self._seen[0]) — no value search needed.
- test docstring moved above the statements it describes.
Paul Klumpp 1 tuần trước cách đây
mục cha
commit
aa7f8a6c01
2 tập tin đã thay đổi với 40 bổ sung40 xóa
  1. 39 39
      adapter.py
  2. 1 1
      test_adapter.py

+ 39 - 39
adapter.py

@@ -597,7 +597,7 @@ class ChattoAdapter(BasePlatformAdapter):
     def _mark_seen(self, event_id: str) -> None:
         self._seen.append(event_id)
         while len(self._seen) > ChattoConstants.SEEN_CAP:
-            self._seen.remove(self._seen[0])  # fastest removal of first item in a list.
+            del self._seen[0]
 
     def _is_seen(self, event_id: str) -> bool:
         return event_id in self._seen
@@ -825,7 +825,7 @@ class ChattoAdapter(BasePlatformAdapter):
         if joined_room.id not in self._joined_room_ids:
             # Same rule as _refresh_rooms: silent rooms are joined read-only
             # — seeding history nothing will ever answer would be waste.
-            if self._is_respond_room(joined_room.id):
+            if self._answers_in_room(joined_room.id):
                 await self._seed_room(joined_room.id)
             else:
                 logger.info(
@@ -1015,6 +1015,24 @@ class ChattoAdapter(BasePlatformAdapter):
             return MessageType.AUDIO
         return MessageType.TEXT
 
+    async def _room_kind_for(
+        self, client: ChattoClient, room_id: str
+    ) -> RoomKind | None:
+        """The room's kind, from cache or a fresh GetRoom lookup.
+
+        Returns ``None`` when the room cannot be resolved — the caller treats
+        that as "not dispatchable" rather than guessing a kind.
+        """
+        kind = self._room_kinds.get(room_id)
+        if kind is not None:
+            return kind
+        room_viewer_state = await client.get_room(room_id)
+        if room_viewer_state is None or room_viewer_state.room is None:
+            return None
+        kind = room_viewer_state.room.kind or RoomKind.UNSPECIFIED
+        self._room_kinds[room_id] = kind
+        return kind
+
     def _room_policy(self, room_id: str) -> ChannelPolicy:
         """Which of the mention lists a channel-kind room is on.
 
@@ -1029,14 +1047,14 @@ class ChattoAdapter(BasePlatformAdapter):
             return ChannelPolicy.REQUIRE_MENTION
         return ChannelPolicy.SILENT
 
-    def _is_respond_room(self, room_id: str) -> bool:
+    def _answers_in_room(self, room_id: str) -> bool:
         """Whether inbound messages from this room reach the agent pipeline.
 
         Chatto knows only DMs and channels, so the split is ``kind == DM``:
-        a DM is always a respond room (``/join`` must stay reachable), every
-        other room — channel-kind or an unknown kind, which is how servers
-        that never set ``kind`` show up — opts in through the mention lists.
-        A room whose policy is SILENT stays read-only (marked as read, never
+        a DM always answers (``/join`` must stay reachable), every other
+        room — channel-kind or an unknown kind, which is how servers that
+        never set ``kind`` show up — opts in through the mention lists. A
+        room whose policy is SILENT stays read-only (marked as read, never
         seeded or answered).
         """
         if self._room_kinds.get(room_id) == RoomKind.DM:
@@ -1046,7 +1064,7 @@ class ChattoAdapter(BasePlatformAdapter):
     async def _dispatch_message_posted(self, payload: MessagePostedPayload) -> None:
         # Respond-room gate first: read-only memberships must not cost a
         # single API call, so this runs before fetch_message and _get_chatto_client.
-        if not self._is_respond_room(payload.room_id):
+        if not self._answers_in_room(payload.room_id):
             logger.debug(
                 "Chatto: message from read-only room %s ignored", payload.room_id
             )
@@ -1090,7 +1108,7 @@ class ChattoAdapter(BasePlatformAdapter):
             return
         self._dispatched_ids.append(message_id)
         while len(self._dispatched_ids) > ChattoConstants.SEEN_CAP:
-            self._dispatched_ids.remove(self._dispatched_ids[0])
+            del self._dispatched_ids[0]
 
     def _edit_is_fresh(self, message: Message) -> bool:
         """Whether this edit is young enough to still be processed.
@@ -1139,7 +1157,7 @@ class ChattoAdapter(BasePlatformAdapter):
         if not self.chatto_config.edit_dispatch.value:
             return
         # Read-only memberships cost no API call, mirroring the posted path.
-        if not self._is_respond_room(payload.room_id):
+        if not self._answers_in_room(payload.room_id):
             logger.debug("Chatto: edit from read-only room %s ignored", payload.room_id)
             return
 
@@ -1251,37 +1269,21 @@ class ChattoAdapter(BasePlatformAdapter):
         The caller owns dispatching: a non-None result still needs
         ``handle_message()``.
         """
-        if message.actor_id in self._user_cache:
-            # try the user cache.
-            user = self._user_cache.get(message.actor_id)
-        else:
-            # get the user and update cache.
+        user = self._user_cache.get(message.actor_id)
+        if user is None:
             directory_member = await client.get_user(user_id=message.actor_id)
-            if directory_member is None:
+            if directory_member is None or directory_member.user is None:
                 return None
             user = directory_member.user
-            if user is None:
-                return None
             self._user_cache[user.id] = user
 
-        if user is None:
-            return None
-
         if not self._check_auth(user):
             return None
 
-        # Todo: use a function that either reads from cache or gets room kind again.
-        if self._room_kinds.get(message.room_id) is None:
-            room_viewer_state = await client.get_room(message.room_id)
-            if room_viewer_state is None:
-                return None
-            if room_viewer_state.room is None:
-                return None
-            self._room_kinds[message.room_id] = (
-                room_viewer_state.room.kind or RoomKind.UNSPECIFIED
-            )
+        room_kind = await self._room_kind_for(client, message.room_id)
+        if room_kind is None:
+            return None
 
-        room_kind = self._room_kinds.get(message.room_id)
         message_body = message.body or ""
 
         logger.debug("message_body: %s room_kind: %s", message_body, room_kind)
@@ -1301,7 +1303,7 @@ class ChattoAdapter(BasePlatformAdapter):
         # the bot is one of many listeners and must be addressed, whereas a DM
         # is already addressed at it. Chatto has no group rooms — any
         # multi-participant surface is a channel, and a room whose kind the
-        # server never set counts as one too (see _is_respond_room). The
+        # server never set counts as one too (see _answers_in_room). The
         # shared _mentions_me gate keeps this path and the someone-else check
         # below on one definition of "addressed", broadcast handles included.
         if room_kind != RoomKind.DM:
@@ -1653,7 +1655,7 @@ class ChattoAdapter(BasePlatformAdapter):
                 # Membership came straight from the directory scan
                 # (viewer_state.is_member); natively invited rooms need no
                 # JoinRoom call — same rule as _run_join.
-                if self._is_respond_room(rid):
+                if self._answers_in_room(rid):
                     await self._seed_room(rid)
                 else:
                     logger.info(
@@ -1667,7 +1669,7 @@ class ChattoAdapter(BasePlatformAdapter):
             joined_room_names: list[str] = []
             for rid in self._joined_room_ids:
                 name = self._room_names[rid]
-                if not self._is_respond_room(rid):
+                if not self._answers_in_room(rid):
                     name += " [read-only]"
                 if rid in self._universal_room_ids:
                     name += " [universal]"
@@ -1782,11 +1784,9 @@ class ChattoAdapter(BasePlatformAdapter):
                     body=chunk,
                     thread_root_event_id=str(thread_id) if thread_id else "",
                 )
-            except ChattoError as e:
-                last_error = str(e)
-                retryable = True
-                break
             except Exception as e:
+                # ChattoError included: both read as "this chunk did not go
+                # out" and stop the batch — the SendResult carries the reason.
                 last_error = str(e)
                 retryable = True
                 break

+ 1 - 1
test_adapter.py

@@ -1905,9 +1905,9 @@ class TestDmRoomCommands:
         assert "CHATTO_REQUIRE_MENTION_ROOMS=grp-9" in reply
 
     async def test_join_skips_rpc_when_already_a_member(self):
-        adapter = self._adapter()
         """Natively invited accounts hold membership already — they only need
         seeding into the joined list (which silent channels skip)."""
+        adapter = self._adapter()
         adapter._seed_room = AsyncMock()
         state = _make_room_state(_make_room("room-9", "Deploy", RoomKind.CHANNEL), True)
         adapter._chatto_client.list_rooms = AsyncMock(return_value=[state])