fix(feishu): preserve quoted reply ancestry (#67184) - #67222
Conversation
Follow the Feishu parent_id / upper_message_id chain for quoted replies to preserve nested message ancestry. Direct parents come first, ancestors follow via "[Earlier quoted message]" separator. - Add _fetch_message_item() with shared LRU cache (128 entries) so text and future media context consumers do not duplicate API calls. - Add _fetch_message_context_chain() with bounded traversal: max 6 messages deep, 500-char budget, cycle detection, and stops on API errors, chat-boundary changes, and thread mismatches. - Reuse the adapter-owned blocking-call executor for all Feishu SDK requests to stay within the gateway's async model. - Keep the existing gateway 500-character reply-context budget as the final transport boundary; direct non-nested replies retain their current behavior. Closes NousResearch#67184
Duplicate of #66207: both implement the same bounded Feishu quoted-reply ancestry and reply-context mechanism. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused Feishu reply-context work. The underlying gap is present on current main: plugins/platforms/feishu/adapter.py:3257-3263 resolves one target and :4173-4204 fetches only that message.
Problems
plugins/platforms/feishu/adapter.py:4302slicestextwithout a null guard (and:4309does the same for later ancestors)._extract_text_from_raw_contentreturnsOptional[str]at current-mainplugins/platforms/feishu/adapter.py:4206-4222; an empty or unknown quote can therefore raiseTypeErrorduring inbound processing. Please render only when text is present, while continuing traversal.- This duplicates open #66207, as noted in the member discussion. Its traversal places rendering inside
if text, and it includes a message-item cache reuse test.
Suggested changes
- Add an empty/unknown quote-item regression case, including a later textual ancestor.
- Consolidate with #66207 so the selected salvage has one maintained implementation.
Automated hermes-sweeper review.
| self._message_text_cache.popitem(last=False) | ||
|
|
||
| if not rendered: | ||
| rendered = text[:max_chars] |
There was a problem hiding this comment.
_extract_text_from_raw_content returns Optional[str]; for an unknown or empty quote item this slice raises TypeError (and the later text[:remaining] has the same issue). Render only inside if text, then continue ancestry traversal so a textless direct parent does not abort inbound processing.
What does this PR do?
Feishu quote replies currently fetch only the direct parent text. When that parent is itself a reply, short follow-ups such as "this one" or "agree" lose the original question that gives the parent its meaning.
This change follows the explicit
parent_id/upper_message_idancestry for quoted messages and supplies a bounded text chain through the existingreply_to_textfield.Related Issue
Fixes #67184.
Type of Change
Changes Made
plugins/platforms/feishu/adapter.pyparent_id,upper_message_id, and root-only ancestry while retaining cycle protection.tests/gateway/test_feishu.pyHow to Test
python -m pytest tests/gateway/test_feishu.py::TestFeishuFetchMessageText -q— 14 passed.python -m pytest tests/gateway/test_feishu.py -q— 173 passed, 47 skipped.python -m pytest tests/gateway/test_reply_to_injection.py -q— 6 passed.ruff check plugins/platforms/feishu/adapter.py tests/gateway/test_feishu.py— All checks passed.python -m py_compile plugins/platforms/feishu/adapter.py tests/gateway/test_feishu.py— OK.git diff --check— passed.