feat(feishu): default to threaded replies in group chats - #22539
Open
ccccaqqqq wants to merge 4 commits into
Open
feat(feishu): default to threaded replies in group chats#22539ccccaqqqq wants to merge 4 commits into
ccccaqqqq wants to merge 4 commits into
Conversation
In Feishu group chats the bot currently posts every response into the
main message stream, which floods the chat. Default to in-thread replies
for regular group chats so each conversation collapses into its own
topic.
Behavior:
- Group chats (chat_mode="group"): replies are auto-threaded.
- Direct messages (chat_mode="p2p"): unchanged.
- Topic groups (chat_mode="topic"): unchanged — already topic-scoped.
- Replies that already carry a thread_id: unchanged.
- Sends without a parent message_id: unchanged (Feishu's API can't
create a topic from a top-level send).
Opt out via:
feishu:
reply_in_thread: false
The reply_in_thread key is already bridged to platform.extra by
gateway/config.py for parity with Slack.
Implementation notes:
- Adds a small _get_chat_mode helper that calls im.v1.chat.get and
caches the chat_mode field per-chat in a dedicated dict.
- Doesn't reuse _chat_info_cache["type"] because _map_chat_type maps
the wrong API field (chat_type — group visibility, "private"/"public")
as the chat-mode classifier and silently falls back to "dm" for
groups. That pre-existing inconsistency is intentionally left alone
in this patch and should be fixed separately.
Co-Authored-By: Claude <noreply@anthropic.com>
The previous commit only auto-threaded sends that already carried a
parent message id (effective_reply_to). In a fresh group chat the final
assistant reply went through the stream-consumer's reply path and was
threaded, but interim sends — tool-progress bubbles ("🐍 execute_code:
...", "💻 terminal: ..."), status callbacks, and approval cards — were
constructed in gateway/run.py with metadata that only included a reply
anchor when source.thread_id was already set. With no thread_id (the
common case for the first message in a new group), those sends arrived
at _send_raw_message with reply_to=None, bypassed the auto-thread
branch, and flooded the main feed alongside the threaded final reply.
Two-part fix:
1. FeishuAdapter remembers the most recent inbound user message id per
chat (_last_user_message_id, OrderedDict capped at 2048 entries,
skips bot-originated messages so the bot never anchors topics to
its own posts). The auto-thread branch in _send_raw_message now
falls back to this implicit anchor when no explicit reply_to was
passed, so any future send path that forgets to plumb metadata
still gets threaded in group chats. Also broadens the existing
metadata.reply_to_message_id fallback to fire regardless of
thread_id presence, so explicit anchors propagate too.
2. gateway/run.py: drop the `source.thread_id` precondition when
building _status_thread_metadata for Feishu. The anchor (event
message id) is now carried unconditionally so status callbacks and
approval cards have an explicit anchor in addition to the implicit
one above. Topic groups and DMs are unaffected — auto-threading
only fires for chat_mode == "group".
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
_reply_anchor_for_event had a Feishu special case that, whenever source.thread_id was set and the user's inbound message carried a reply_to_message_id (i.e. the user quoted some earlier message), would return the *quoted* message id as the bot's reply anchor instead of the user's own message id. The anchor flowed downstream as event_message_id (see gateway/run.py:9487) into the stream consumer's reply_to and the status/approval metadata, so every bot send ended up visually attached to the message the user quoted — not to the user's own request. Symptom: in a group with topic-style replies enabled, user quotes an unrelated meego link and @-mentions the bot to act on it; the bot's threaded reply appears anchored under the meego link rather than under the user's @-mention, which reads as the bot responding to the link. Removing the special case lets _reply_anchor_for_event fall through to the default (event.message_id, the user's own). The user's message is in the same topic if the user is in one, so reply_in_thread=true still keeps the bot inside the topic — the only thing that changes is the visual anchor. The original Feishu branch was added in passing by b323957 (fix(telegram): preserve DM topic routing via reply fallback) without a stated rationale or Feishu-specific test coverage, so removing it is safe; existing tests (only the Telegram branch is exercised) remain unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous patch (0148f0c) added a _last_user_message_id fallback in _send_raw_message's auto-thread branch, but gated it on ``not reply_in_thread`` — assuming sends that already carry a thread_id in metadata also carry an anchor. They don't always. Specifically, _deliver_media_from_response builds metadata via _thread_metadata_for_source which only emits ``{"thread_id": ...}`` for Feishu (it never adds reply_to_message_id; that branch is Telegram-only). A QR image sent through that path arrives at _send_raw_message with reply_to=None and metadata={thread_id: omt_xxx}. The old code computed ``reply_in_thread = True`` from the metadata, then skipped the entire auto-thread block (precondition was ``not reply_in_thread``), so the implicit anchor was never read. effective_reply_to stayed None, and the send fell through to the top-level branch where reply_in_thread is silently ignored — image landed flat in the main feed. Symptom: bytecli meego authorization QR (and any other media delivered via _deliver_media_from_response) appeared top-level even though the text/approval/tool-progress messages around it were correctly threaded. Fix: re-gate the fallback on ``not effective_reply_to`` instead of ``not reply_in_thread``. The two cases now collapse: (a) no thread_id, no reply_to → anchor + set reply_in_thread (classic auto-thread to start a topic). (b) thread_id present, no reply_to → anchor + reply_in_thread already True (drop into the existing topic via the reply API, which is the only way Feishu lets a send land inside a topic). Both paths now use the cached last user message id as the anchor and go through _build_reply_message_body, so the bot send always reaches the right topic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
alt-glitch
added
type/feature
Contributor
|
Thanks for the detailed Feishu threading work. The underlying regular-group behavior is still absent on current main, but this patch needs a targeted rework before it can be salvaged. Problems
Suggested changes
Automated hermes-sweeper review. |
teknium1
added
sweeper:risk-message-delivery
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In Feishu group chats the bot currently posts every response into the main message stream, which floods the chat. Default to in-thread replies for regular group chats so each conversation collapses into its own topic.
Behavior:
Opt out via:
The reply_in_thread key is already bridged to platform.extra by gateway/config.py for parity with Slack.
Implementation notes:
What does this PR do?
Related Issue
Fixes #
Type of Change
Changes Made
How to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
hermes --toolsets skills -q "Use the X skill to do Y"Screenshots / Logs