Skip to content

fix(yuanbao): evict stale entries from _member_cache on TTL expiry - #23384

Closed
AhmetArif0 wants to merge 1 commit into
NousResearch:mainfrom
AhmetArif0:fix/yuanbao-member-cache-eviction
Closed

fix(yuanbao): evict stale entries from _member_cache on TTL expiry#23384
AhmetArif0 wants to merge 1 commit into
NousResearch:mainfrom
AhmetArif0:fix/yuanbao-member-cache-eviction

Conversation

@AhmetArif0

Copy link
Copy Markdown
Contributor

Problem

_build_msg_body_with_mentions() in MessageSender checks whether a cached member list has expired, but never removes the stale entry:

# yuanbao.py L4112-4117 (before fix)
cached = self._adapter._member_cache.get(group_code)
if cached:
    ts, member_list = cached
    members = member_list if (time.time() - ts < self._adapter.MEMBER_CACHE_TTL_S) else []

When the TTL has elapsed, members is set to [] and the stale entry is silently kept in the dict. Every group_code the bot has ever queried retains a permanent entry in _member_cache — holding the full member list (potentially thousands of records per group) until disconnect(). In a long-running bot serving many groups, this becomes an unbounded memory leak.

get_group_member_list_raw() overwrites the entry on a fresh fetch for the same group, so active groups don't accumulate duplicates. But inactive or departed groups never get a fresh fetch, so their entry is never overwritten or removed.

Fix

Delete the stale entry at the point it is detected as expired:

cached = self._adapter._member_cache.get(group_code)
if cached:
    ts, member_list = cached
    if time.time() - ts < self._adapter.MEMBER_CACHE_TTL_S:
        members = member_list
    else:
        del self._adapter._member_cache[group_code]
        members = []
else:
    members = []

The next call to get_group_member_list_raw() for the same group will repopulate the cache with fresh data, as it did before.

Notes

  • _build_msg_body_with_mentions is the only read path for _member_cache (one call site at L4104). The only write path is get_group_member_list_raw() at L3615. No concurrency concern — asyncio is single-threaded.
  • Symmetric with MessageDeduplicator (used for inbound dedup on the same adapter), which also evicts on access when TTL expires.
  • MEMBER_CACHE_TTL_S = 300.0 (5 minutes), same value as MessageDeduplicator(ttl_seconds=300).

Testing

Tested on: macOS

_build_msg_body_with_mentions() checks the TTL of each _member_cache
entry and returns an empty member list when the entry is stale, but
never removes the entry from the dict.  Over time every group_code the
bot has ever queried accumulates a permanent entry, retaining the full
member list (potentially thousands of records per group) until
disconnect().

Fix: delete the stale entry at the point it is detected as expired.
The next call to get_group_member_list_raw() for the same group will
repopulate the cache with fresh data as before.

Symmetric with the existing TTL pattern in MessageDeduplicator, which
evicts on access.
@alt-glitch alt-glitch added type/perf Performance improvement or optimization comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have labels May 10, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused cache-lifetime fix. The premise remains valid on current main: gateway/platforms/yuanbao.py:4864-4867 treats an expired member-cache entry as empty but leaves it stored, while :4367 writes non-empty member lists into that cache.

Problems

  • The PR adds no regression coverage. The current Yuanbao tests do not exercise _member_cache or _build_msg_body_with_mentions.

Suggested changes

  • Add a unit test that seeds an expired member-cache entry, calls adapter._outbound.sender._build_msg_body_with_mentions(...), and asserts the key is evicted while the existing plain-text fallback remains intact.

This is an automated hermes-sweeper review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants