Преглед на файлове

Deliver the room roster on every channel turn, cap it at 20 members

The per-room dedup kept the roster out of follow-up dispatches once a
room was announced, but threads hold isolated sessions: a thread opened
after the announcement never saw an audience at all. Cache and delivery
had different scopes - the cache is room-scoped (one directory fetch,
presence patched live), but the consumer is turn-scoped.

Fix it by dropping dedup entirely: every channel turn now carries the
rendered roster line from the cached projection. Repetition is free of
API calls; membership events, /leave, stale-room pruning and reconnects
still evict a room so its next turn refetches once. ROSTER_MEMBER_LIMIT
drops from 100 to 20 to keep the prompt block small now that it rides
on every message.
Paul Klumpp преди 1 седмица
родител
ревизия
622560ac60
променени са 5 файла, в които са добавени 85 реда и са изтрити 128 реда
  1. 14 16
      AGENTS.md
  2. 4 4
      README.md
  3. 32 52
      adapter.py
  4. 5 4
      platform_config.py
  5. 30 52
      test_adapter.py

+ 14 - 16
AGENTS.md

@@ -95,22 +95,20 @@ changes must keep them, or consciously renegotiate the docs.**
 - **Threads stay threads.** An inbound thread reply keeps its thread context;
   a fresh room reply opens a thread under the incoming message unless
   `auto_thread` is disabled or the room is a DM.
-- **Room roster is room-scoped and presence-live.** The first turn in a room
-  carries the member roster in `channel_context` so the agent knows its
-  audience; the roster is **not** thread-scoped and not delivered on every
-  turn. Per announced room the adapter keeps a miniature projection of server
-  state — member IDs plus their cached users (`_user_cache`, one cache, one
-  truth) and the last rendered line. `presence_changed` patches the cached
-  user in place; the next turn in that room re-renders from cache and
-  re-delivers only when the line actually changed, so presence churn costs no
-  API calls. `user_joined_room`/`user_left_room` discard a room's projection
-  (its next turn refetches once), as does a reconnect — protocol v1 sends no
-  presence snapshot on subscribe, so discarding is what forces fresh data
-  after downtime; refetching is lazy, quiet rooms stay free of lookups. The
-  bot's own presence events are filtered out. A failed lookup leaves the room
-  unannounced and lets the next turn retry. This is the fix for the old
-  "roster frozen at thread start" flaw: presence moves (away/offline) within
-  a long-lived room now catch up on the next message.
+- **Room roster rides on every channel turn.** Threads hold isolated sessions
+  and the agent only sees `channel_context` per dispatch, so each channel turn
+  carries the current member roster — repetition is the point, deduplicating
+  it strands later threads without an audience (that bug shipped once). The
+  cost stays bounded because the roster is a miniature projection of server
+  state, modelled on the Chatto web frontend's presence overlay: per room,
+  member IDs plus their users in `_user_cache` (one cache, one truth),
+  fetched from the directory once. `presence_changed` patches the cached user
+  in place; `user_joined_room`/`user_left_room`, `/leave`, stale-room pruning
+  and reconnects discard a room's projection so its next turn refetches once —
+  protocol v1 sends no presence snapshot on subscribe, so discarding is what
+  forces fresh data after downtime; quiet rooms stay free of lookups. The bot's
+  own presence events are filtered out. A failed lookup leaves the room
+  unprojected and lets the next turn retry.
 - **Edits are corrections, not new traffic.** An inbound `message_edited`
   re-runs the admission gates against the new body: a message currently being
   processed is cancelled (🚫) and redispatched with the edited text; a queued

+ 4 - 4
README.md

@@ -15,7 +15,7 @@ Before setup, here's the part most people want to know: how Hermes behaves once
 | **DMs** | Hermes responds to every message. No `@mention` needed. Each DM has its own session. |
 | **Rooms** | Opt-in per room: a room listed in `CHATTO_REQUIRE_MENTION_ROOMS` gets answers only when Hermes is addressed (`@mention`, `@all`, `@here`); one listed in `CHATTO_OPTIONAL_MENTION_ROOMS` gets an answer for every message. A room on neither list stays silent — read-only. |
 | **Threads** | If you reply in a thread, Hermes keeps the thread context isolated from the parent room. The bot auto-follows threads it participates in. |
-| **Room awareness** | The first turn in a room comes with the member roster attached (who is in the room, with presence), so Hermes knows his audience without asking. Presence changes flow into the cached roster live and are re-delivered on the next turn only when the rendered line actually changed — going `away`/`offline` is reflected without extra directory lookups. DMs never carry one. |
+| **Room awareness** | Every channel turn comes with the member roster attached (who is in the room, with live presence), so Hermes always knows his audience — in every thread, every turn. The roster is fetched once per room and kept fresh from presence events without extra directory lookups; the first 20 members are listed, larger rooms get an "and N more" note. DMs never carry one. |
 | **Processing indicators** | Hermes adds a 👀 reaction when it starts processing a message, and replaces it with ✅ on success, ❌ on failure, or 🚫 when processing was cancelled. |
 | **Editing your messages** | An edit within 5 minutes of posting counts as a correction: while Hermes is still working on the original, he restarts with the edited text (🚫 → 👀); if he had ignored or not yet answered the message, the gates are re-checked against the new text — adding a forgotten `@mention` this way works. Edits to messages Hermes has already answered change nothing. Disable with `CHATTO_EDIT_DISPATCH=false`. |
 | **Typing indicators** | Hermes broadcasts persistent typing indicators while it's working, so users know the bot is active. |
@@ -44,7 +44,7 @@ Capabilities natively implemented by the Chatto plugin adapter:
 | room creation | yes |
 | room join/leave over DM | yes (`/join`, `/leave`) |
 | member directory | yes (cached) |
-| room member context | yes (roster on the first turn of each room, presence kept live from cached members) |
+| room member context | yes (roster on every channel turn, presence kept live from a per-room cache) |
 | presence broadcasting | yes (online, refreshed) |
 | custom status | yes |
 | read state management | yes |
@@ -208,9 +208,9 @@ When you reply to a message in Chatto (creating a thread), Hermes responds withi
 
 ### Room Member Roster
 
-Hermes cannot see who else is in a room: unaddressed channel messages are dropped by the mention gates long before they could teach him a name. So the first turn in a room carries the member roster alongside the message text — one entry per member as `@login (Display Name, presence)`, e.g. `@alice (Alice, online), @bob (Bob, offline)`. The bot's own account and deleted users are left out; rooms larger than 100 members end with `… and N more`. Presence is the Chatto status (`online`, `away`, `do not disturb`, `offline`).
+Hermes cannot see who else is in a room: unaddressed channel messages are dropped by the mention gates long before they could teach him a name. So every turn in a channel carries the member roster alongside the message text — one entry per member as `@login (Display Name, presence)`, e.g. `@alice (Alice, online), @bob (Bob, offline)`. The bot's own account and deleted users are left out; rooms larger than 20 members end with `… and N more`. Presence is the Chatto status (`online`, `away`, `do not disturb`, `offline`).
 
-The roster is room-scoped, not thread-scoped: the first turn in a room brings it, and later turns bring nothing again until the rendered line actually changes. The adapter keeps a small projection per announced room — member IDs plus their cached users — and `presence_changed` events patch those users in place, so someone going `away` or `offline` shows up on the next turn in that room without any extra directory lookup; churn that ends without a visible change repeats nothing. The bot's own presence refresh never enters the roster. Membership changes (`user_joined_room`/`user_left_room`) and reconnects discard the projection, and the room's next turn refetches once — protocol v1 sends no presence snapshot on subscribe, so a discarded cache is what forces fresh data after downtime. If the initial lookup fails, the turn proceeds without the roster and the next turn tries again.
+The roster rides on every channel turn — threads hold isolated sessions, so each turn needs its own copy. That repetition is free of API calls: the adapter keeps a small projection per room (member IDs plus their cached users), fetched once from the directory, and `presence_changed` events patch those users in place, so someone going `away` or `offline` shows up on the next turn without any extra lookup. The bot's own presence refresh never enters the roster. Membership changes (`user_joined_room`/`user_left_room`), leaving a room and reconnects discard the projection, and the room's next turn refetches once — protocol v1 sends no presence snapshot on subscribe, so a discarded cache is what forces fresh data after downtime. If the initial lookup fails, the turn proceeds without the roster and the next turn tries again.
 
 ### Reactions
 

+ 32 - 52
adapter.py

@@ -337,19 +337,15 @@ class ChattoAdapter(BasePlatformAdapter):
         # fresh turn — that is the lock against re-answering settled
         # conversations by editing old messages.
         self._dispatched_ids: deque[str] = deque(maxlen=ChattoConstants.SEEN_CAP)
-        # Rooms whose member roster has already been announced to the agent via
-        # channel_context — the roster is room-scoped, so the first turn in a
-        # room carries it. Together with _rosters/_roster_text this is our
-        # miniature projection of server state: presence changes patch the
-        # cached users in place and the next turn re-renders from cache;
-        # membership events and reconnects discard a room so its next turn
-        # refetches once.
+        # Rooms whose member roster has been projected from the directory —
+        # the roster is room-scoped and rides on every channel turn (threads
+        # hold isolated sessions, so each turn needs its own copy). Together
+        # with _rosters this is our miniature projection of server state:
+        # presence changes patch the cached users in place; membership events
+        # and reconnects discard a room so its next turn refetches once.
         self._roster_announced: deque[str] = deque(maxlen=ChattoConstants.SEEN_CAP)
         # Announced rooms' membership projection (see _RoomRoster).
         self._rosters: dict[str, _RoomRoster] = {}
-        # Announced rooms' last delivered roster text — the dedup key that
-        # keeps presence churn from repeating an unchanged channel_context.
-        self._roster_text: dict[str, str] = {}
         # session_key -> message ID currently being processed there. Written
         # by on_processing_start, cleared by on_processing_complete; an edit
         # landing on the recorded ID is a mid-run correction.
@@ -1096,8 +1092,8 @@ class ChattoAdapter(BasePlatformAdapter):
 
     async def _room_roster_context(
         self, client: ChattoClient, room_id: str
-    ) -> tuple[_RoomRoster, str] | None:
-        """Fetch the room's member roster as a fresh projection + rendered line.
+    ) -> _RoomRoster | None:
+        """Fetch the room's member roster as a fresh projection.
 
         The agent only ever sees its prompt: without this block it cannot know
         who else is in a channel, because unaddressed messages are dropped by
@@ -1144,11 +1140,7 @@ class ChattoAdapter(BasePlatformAdapter):
             return None
 
         unfetched = max(0, page.total_count - len(members))
-        more = f" … and {unfetched} more" if unfetched else ""
-        return (
-            _RoomRoster(member_ids=member_ids, unfetched=unfetched),
-            ", ".join(entries) + more,
-        )
+        return _RoomRoster(member_ids=member_ids, unfetched=unfetched)
 
     def _roster_entry(self, user: User) -> str:
         """One roster entry, e.g. ``@bob (Bob Example, online)``.
@@ -1189,8 +1181,8 @@ class ChattoAdapter(BasePlatformAdapter):
         more = f" … and {roster.unfetched} more" if roster.unfetched else ""
         return ", ".join(entries) + more
 
-    def _announce_roster(self, room_id: str, roster: _RoomRoster, text: str) -> None:
-        """Record a freshly fetched roster as the room's announced projection.
+    def _announce_roster(self, room_id: str, roster: _RoomRoster) -> None:
+        """Record a freshly fetched roster as the room's projection.
 
         Bounded like _dispatched_ids: appending past the cap drops the oldest
         room's projection alongside its deque entry.
@@ -1204,9 +1196,7 @@ class ChattoAdapter(BasePlatformAdapter):
             self._roster_announced.append(room_id)
             if oldest is not None:
                 self._rosters.pop(oldest, None)
-                self._roster_text.pop(oldest, None)
         self._rosters[room_id] = roster
-        self._roster_text[room_id] = text
 
     def _evict_roster(self, room_id: str) -> None:
         """Drop all roster state for a room.
@@ -1220,34 +1210,24 @@ class ChattoAdapter(BasePlatformAdapter):
         except ValueError:
             pass
         self._rosters.pop(room_id, None)
-        self._roster_text.pop(room_id, None)
 
     async def _roster_for_room(self, client: ChattoClient, *, room_id: str) -> str:
-        """Roster context for this room, kept fresh through the projection.
-
-        The first turn in a room fetches and announces the roster; afterwards
-        the cached members are re-rendered and re-delivered only when the line
-        actually changed — presence patches land in _user_cache without any
-        API call, so churn like rapid away/offline flips costs dictionary
-        writes only. Membership events or a reconnect evict the room, making
-        its next turn refetch once. An unannounced room retries on its next
-        turn until a lookup succeeds.
-        """
-        if room_id in self._roster_announced:
-            line = self._roster_line(room_id)
-            if line == self._roster_text.get(room_id):
-                logger.debug("Chatto: roster for room %s unchanged", room_id)
-                return ""
-            logger.debug("Chatto: roster for room %s changed - redelivering", room_id)
-            self._roster_text[room_id] = line
-            return line
+        """The room's roster line for this turn — delivered on every turn.
 
-        fetched = await self._room_roster_context(client, room_id)
-        if fetched is None:
-            return ""
-        roster, text = fetched
-        self._announce_roster(room_id, roster, text)
-        return text
+        Threads hold isolated sessions and the agent only sees
+        channel_context per dispatch, so each channel turn carries the current
+        audience rather than deduplicating it. The projection exists to spare
+        the directory, not the prompt: presence patches land in _user_cache
+        in place, membership events or a reconnect evict the room so its next
+        turn refetches once. An unprojected room retries on its next turn
+        until a lookup succeeds.
+        """
+        if room_id not in self._roster_announced:
+            fetched = await self._room_roster_context(client, room_id)
+            if fetched is None:
+                return ""
+            self._announce_roster(room_id, fetched)
+        return self._roster_line(room_id)
 
     async def _dispatch_message_posted(self, payload: MessagePostedPayload) -> None:
         # Respond-room gate first: read-only memberships must not cost a
@@ -1541,11 +1521,11 @@ class ChattoAdapter(BasePlatformAdapter):
             thread_id,
         )
 
-        # The first turn in a room is where the agent learns who is in the
-        # channel — later turns re-deliver it only when the cached projection
-        # renders a different line (see _roster_for_room). The gateway prepends
-        # channel_context above the message text, so the roster never mingles
-        # with what the user actually wrote.
+        # Every channel turn carries the room's roster: threads hold isolated
+        # sessions and the agent only sees channel_context per dispatch, so a
+        # repeat is not deduplication but the point (see _roster_for_room).
+        # The gateway prepends channel_context above the message text, so the
+        # roster never mingles with what the user actually wrote.
         roster = ""
         if room_kind != RoomKind.DM:
             logger.debug(
@@ -1556,7 +1536,7 @@ class ChattoAdapter(BasePlatformAdapter):
             if not roster:
                 logger.debug(
                     "Chatto: dispatching channel message without roster "
-                    "(lookup failed, empty, or unchanged)"
+                    "(lookup failed or empty)"
                 )
 
         source = self.build_source(

+ 5 - 4
platform_config.py

@@ -59,10 +59,11 @@ class ChattoConstants:
     SPLIT_THRESHOLD = 9900
     SEEN_CAP = 500
 
-    # Members fetched per roster announcement (the context block handed to the
-    # agent when a channel thread opens). Larger rooms get an "and N more"
-    # note instead of an unbounded prompt block.
-    ROSTER_MEMBER_LIMIT = 100
+    # Members fetched per roster delivery (the context block handed to the
+    # agent alongside every channel turn). The roster rides on each message,
+    # so the cap keeps the prompt block small; larger rooms get an "and N
+    # more" note instead.
+    ROSTER_MEMBER_LIMIT = 20
 
     # WebSocket reconnect backoff (the realtime transport itself lives in
     # chattolib.realtime, which owns protocol-level constants).

+ 30 - 52
test_adapter.py

@@ -1388,11 +1388,11 @@ def _roster_user(user_id, login, presence=PresenceStatus.UNSPECIFIED, deleted=Fa
 
 
 class TestRoomRoster:
-    """The first turn in a room carries the member roster in channel_context —
-    the agent's prompt is the only place it could learn who else is listening.
-    The roster is a room-scoped mini projection: presence changes patch the
-    cached users and re-render without any API call; membership events and
-    reconnects discard a room so its next turn refetches once."""
+    """Every channel turn carries the member roster in channel_context —
+    threads hold isolated sessions, so each turn needs its own copy. The
+    roster is a room-scoped mini projection: fetched once, presence changes
+    patch the cached users in place; membership events and reconnects discard
+    a room so its next turn refetches once."""
 
     def _adapter(self):
         adapter = _make_adapter()
@@ -1463,7 +1463,9 @@ class TestRoomRoster:
         assert "@hermes_bot" not in context
         assert "carol" not in context
 
-    async def test_thread_follow_up_does_not_repeat_it(self):
+    async def test_follow_up_carries_the_roster_too(self):
+        """Threads hold isolated sessions, so every channel turn needs its own
+        copy of the roster — repetition is the point, deduplication a bug."""
         adapter = self._adapter()
         self._seed_channel(adapter)
         adapter._chatto_client.list_room_members = AsyncMock(
@@ -1478,8 +1480,11 @@ class TestRoomRoster:
             thread_root="msg-1",
         )
 
+        # Still one directory lookup: the projection spares the API, not the
+        # prompt.
         assert adapter._chatto_client.list_room_members.await_count == 1
-        assert self._dispatched_event(adapter).channel_context is None
+        context = self._dispatched_event(adapter).channel_context
+        assert context is not None and "@alice" in context
 
     async def test_unspecified_room_kind_still_gets_one(self):
         """A server that never sets kind counts as a channel — roster too."""
@@ -1572,9 +1577,9 @@ class TestRoomRoster:
         assert adapter.handle_message.await_count == 1
         assert "@alice" in self._dispatched_event(adapter).channel_context
 
-    async def test_presence_change_redelivers_updated_roster_from_cache(self):
+    async def test_presence_change_shows_on_next_turn_from_cache(self):
         """Presence moves patch the cached user; the next turn in the room
-        re-renders the line from cache — no second directory lookup."""
+        renders the updated line — no second directory lookup."""
         adapter = self._adapter()
         self._seed_channel(adapter)
         # alice starts UNSPECIFIED (no presence label in the rendered entry).
@@ -1599,19 +1604,9 @@ class TestRoomRoster:
         context = self._dispatched_event(adapter).channel_context
         assert context is not None and "@alice (Alice, offline)" in context
 
-        # Unchanged since, a further turn delivers nothing.
-        await self._dispatch(
-            adapter,
-            "@hermes_bot third",
-            message_id="msg-3",
-            thread_root="msg-1",
-        )
-        assert adapter._chatto_client.list_room_members.await_count == 1
-        assert self._dispatched_event(adapter).channel_context is None
-
-    async def test_presence_churn_to_the_same_status_delivers_once(self):
-        """Rapid away/offline flapping that ends where it started costs
-        dictionary writes only and never repeats the channel_context."""
+    async def test_presence_churn_costs_no_extra_lookups(self):
+        """Rapid away/offline flapping is absorbed by the cache: the roster
+        still rides on every turn, but the directory is never re-asked."""
         adapter = self._adapter()
         self._seed_channel(adapter)
         adapter._chatto_client.list_room_members = AsyncMock(
@@ -1624,8 +1619,6 @@ class TestRoomRoster:
             await adapter._handle_realtime_event(
                 _make_presence_event(user_id="user-1", status=PresenceStatus.AWAY)
             )
-        adapter.handle_message.reset_mock()
-
         await self._dispatch(
             adapter,
             "@hermes_bot again",
@@ -1633,20 +1626,12 @@ class TestRoomRoster:
             thread_root="msg-1",
         )
         assert adapter._chatto_client.list_room_members.await_count == 1
-        assert self._dispatched_event(adapter).channel_context is not None
-        assert "(Alice, away)" in self._dispatched_event(adapter).channel_context
-
-        await self._dispatch(
-            adapter,
-            "@hermes_bot third",
-            message_id="msg-3",
-            thread_root="msg-1",
-        )
-        assert self._dispatched_event(adapter).channel_context is None
+        context = self._dispatched_event(adapter).channel_context
+        assert context is not None and "(Alice, away)" in context
 
-    async def test_non_member_presence_change_does_not_redeliver(self):
-        """A presence move by someone outside the announced roster renders the
-        same line — nothing goes out."""
+    async def test_non_member_presence_change_does_not_alter_the_line(self):
+        """A presence move by someone outside the roster projection leaves the
+        rendered line untouched — but the turn still carries it."""
         adapter = self._adapter()
         self._seed_channel(adapter)
         adapter._chatto_client.list_room_members = AsyncMock(
@@ -1666,10 +1651,12 @@ class TestRoomRoster:
             message_id="msg-2",
             thread_root="msg-1",
         )
-        assert self._dispatched_event(adapter).channel_context is None
+        context = self._dispatched_event(adapter).channel_context
+        assert context is not None and "mallory" not in context
 
     async def test_own_presence_change_is_ignored(self):
-        """Our 60s presence refresh must neither patch nor redeliver."""
+        """Our 60s presence refresh patches nothing — the agent must not see
+        the bot in its own audience."""
         adapter = self._adapter()
         self._seed_channel(adapter)
         adapter._chatto_client.list_room_members = AsyncMock(
@@ -1685,13 +1672,6 @@ class TestRoomRoster:
         )
 
         assert bot.presence_status == PresenceStatus.ONLINE
-        await self._dispatch(
-            adapter,
-            "@hermes_bot again",
-            message_id="msg-2",
-            thread_root="msg-1",
-        )
-        assert self._dispatched_event(adapter).channel_context is None
 
     async def test_presence_change_before_any_announcement_is_noop(self):
         """No room announced yet — a presence event must not raise or fetch,
@@ -1705,7 +1685,7 @@ class TestRoomRoster:
         assert adapter._chatto_client.list_room_members.await_count == 0
         assert "user-42" not in adapter._user_cache
 
-    async def test_membership_event_refetches_and_redelivers_once(self):
+    async def test_membership_event_refetches_once(self):
         """A join/leave makes the cached roster wrong: it is discarded and the
         next turn refetches once, delivering the updated line."""
         adapter = self._adapter()
@@ -1746,8 +1726,10 @@ class TestRoomRoster:
             message_id="msg-3",
             thread_root="msg-1",
         )
+        # Still one refetch only — but the roster keeps riding on every turn.
         assert adapter._chatto_client.list_room_members.await_count == 2
-        assert self._dispatched_event(adapter).channel_context is None
+        context = self._dispatched_event(adapter).channel_context
+        assert context is not None and "@bob" in context
 
     async def test_reconnect_discards_cached_rosters_for_a_fresh_snapshot(self):
         """Protocol v1 sends no presence snapshot on subscribe, so every
@@ -1766,7 +1748,6 @@ class TestRoomRoster:
         await adapter._refresh_rooms()
         assert not adapter._roster_announced
         assert not adapter._rosters
-        assert not adapter._roster_text
 
         await self._dispatch(
             adapter,
@@ -1796,7 +1777,6 @@ class TestRoomRoster:
 
         assert "gone-1" not in adapter._roster_announced
         assert "gone-1" not in adapter._rosters
-        assert "gone-1" not in adapter._roster_text
 
 
 # -- Inbound edits (edit-dispatch) --
@@ -2430,7 +2410,6 @@ class TestDmRoomCommands:
         adapter._joined_room_ids = ["room-7"]
         adapter._roster_announced.append("room-7")
         adapter._rosters["room-7"] = MagicMock()
-        adapter._roster_text["room-7"] = "@alice"
         state = _make_room_state(_make_room("room-7", "Deploy", RoomKind.CHANNEL), True)
         adapter._chatto_client.get_room = AsyncMock(return_value=state)
 
@@ -2441,7 +2420,6 @@ class TestDmRoomCommands:
         # Leaving the audience means keeping no roster projection for it.
         assert "room-7" not in adapter._roster_announced
         assert "room-7" not in adapter._rosters
-        assert "room-7" not in adapter._roster_text
         assert "Left 'Deploy' (room-7)" in self._reply(adapter)
 
     async def test_leave_refuses_direct_messages(self):