Skip to content

fix(feishu): include replied media context - #27545

Open
jukzthebest wants to merge 1 commit into
NousResearch:mainfrom
jukzthebest:fix/feishu-reply-parent-media-context
Open

fix(feishu): include replied media context#27545
jukzthebest wants to merge 1 commit into
NousResearch:mainfrom
jukzthebest:fix/feishu-reply-parent-media-context

Conversation

@jukzthebest

Copy link
Copy Markdown

Summary

  • Fetch normalized context for replied-to Feishu messages, including downloadable media resources.
  • Attach replied parent media to inbound events so text replies to images/files keep the original attachment available to the agent.
  • Keep existing reply-to text behavior through the new context helper and add regression coverage for parent image replies.

Test Plan

  • python -m py_compile gateway/platforms/feishu.py tests/gateway/test_feishu.py
  • python -m pytest tests/gateway/test_feishu.py -q

@alt-glitch alt-glitch added type/bug Something isn't working platform/feishu Feishu / Lark adapter comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists labels May 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #13115 (fix(feishu): include parent media context for replies). Also related to #24285, #26097, #26063 — all competing PRs for the same Feishu reply-to-media fix. Closes #26037.

@teknium1

teknium1 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Thanks for the Feishu fix — I verified the bug still exists on current main, and the overall direction is useful.

Problems

  • Current main only fetches parent text in _process_inbound_message (gateway/platforms/feishu.py:3132) and passes only the current message’s media_urls into MessageEvent (gateway/platforms/feishu.py:3171), so the premise is real.
  • The patch adds _message_context_cache as a plain Dict on PR head (gateway/platforms/feishu.py:1453) and caches entries without eviction (gateway/platforms/feishu.py:3934). Current main already bounds reply text lookups with OrderedDict plus _FEISHU_MESSAGE_TEXT_CACHE_SIZE eviction (gateway/platforms/feishu.py:1458, gateway/platforms/feishu.py:4050). Salvaging this should preserve that bounded-cache behavior.

Suggested changes

  • Use an OrderedDict/LRU cap for the new parent context cache, including move_to_end on hits and popitem(last=False) on overflow.
  • Add coverage for parent file/audio media_refs and cache hit/eviction behavior; the downloader has separate image and media-ref paths (gateway/platforms/feishu.py:3643, gateway/platforms/feishu.py:3652).

Automated hermes-sweeper review.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the Feishu reply-context fix. The underlying bug is still present on current main: plugins/platforms/feishu/adapter.py:3259 fetches only parent text, while MessageEvent receives only the current inbound media at :3298-3299.

Problems

  • The new _message_context_cache in this diff is an unbounded plain Dict. It retains downloaded parent-media paths indefinitely. Current main's analogous _message_text_cache is LRU-bounded at plugins/platforms/feishu/adapter.py:4165-4189; the salvaged context cache should preserve that behavior.
  • The added test covers a parent image only. The downloader separately handles image keys and generic media references at plugins/platforms/feishu/adapter.py:3780-3799, so parent file/audio cases need coverage too.
  • The adapter was relocated to plugins/platforms/feishu/adapter.py by 5600105478ffde29d7566b45421b100eaa29c4ef, so this needs a targeted salvage port rather than applying the old-path diff directly.

Suggested changes

  • Port the helper and media merge to the bundled plugin, use a capped OrderedDict/LRU cache, and test image plus file/audio parent media and eviction behavior.

Automated hermes-sweeper review.

self._sent_message_id_order: List[str] = [] # LRU order for _sent_message_ids_to_chat
self._chat_info_cache: Dict[str, Dict[str, Any]] = {}
self._message_text_cache: Dict[str, Optional[str]] = {}
self._message_context_cache: Dict[str, tuple[Optional[str], List[str], List[str]]] = {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this a bounded OrderedDict/LRU cache. Parent contexts include downloaded media paths, so an unbounded Dict can grow for the lifetime of a busy adapter; current main's analogous reply-text cache promotes hits and evicts past _FEISHU_MESSAGE_TEXT_CACHE_SIZE.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
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 P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants