Skip to content

fix(feishu): handle quote field for quoted reply messages - #22935

Open
sicnuyudidi wants to merge 1 commit into
NousResearch:mainfrom
sicnuyudidi:fix/feishu-quote-field
Open

fix(feishu): handle quote field for quoted reply messages#22935
sicnuyudidi wants to merge 1 commit into
NousResearch:mainfrom
sicnuyudidi:fix/feishu-quote-field

Conversation

@sicnuyudidi

Copy link
Copy Markdown
Contributor

Summary

Extract reply_to_message_id and reply_to_text from the Feishu quote field in message events, similar to how Signal adapter handles it.

Previously only parent_id, upper_message_id, and root_id were checked, which are empty for quoted replies outside of threads.

Changes

In gateway/platforms/feishu.py:

  1. Extract quote field from message: quote = getattr(message, "quote", None) or {}
  2. Get quote_id and quote_text from the quote field
  3. Use quote_id as fallback when parent_id/upper_message_id/root_id are empty
  4. Use quote_text as fallback for reply_to_text

Fixes

Fixes #22934

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter comp/gateway Gateway runner, session dispatch, delivery labels May 10, 2026
@sicnuyudidi
sicnuyudidi force-pushed the fix/feishu-quote-field branch from 9088e9a to 55d37f3 Compare May 10, 2026 05:50
@Bartok9

Bartok9 commented May 10, 2026

Copy link
Copy Markdown
Contributor

Reviewed — correct fix. Closing my duplicate #23079. The quote.get('message_id') or quote.get('id') fallback chain covers both old and new Feishu SDK versions, which is more defensive than a single key lookup.

@sicnuyudidi
sicnuyudidi force-pushed the fix/feishu-quote-field branch 2 times, most recently from a556475 to 1e0e6ab Compare May 12, 2026 09:28
Extract reply_to_message_id and reply_to_text from the Feishu quote field in message events, similar to how Signal adapter handles it.

Previously only parent_id, upper_message_id, and root_id were checked, which are empty for quoted replies outside of threads.

Also initialize reply_to_text before use to avoid UnboundLocalError.

Fixes NousResearch#22934
@sicnuyudidi
sicnuyudidi force-pushed the fix/feishu-quote-field branch from 1e0e6ab to 609c226 Compare May 12, 2026 09:29
@sicnuyudidi

Copy link
Copy Markdown
Contributor Author

Hi! I noticed the CI checks are failing (test, e2e, Windows footguns, and supply chain scan).

My change is very small — just 17 lines added / 2 deleted in gateway/platforms/feishu.py to extract the quote field for quoted reply messages, similar to how Signal adapter already handles it.

The test/e2e failures don't appear to be related to my changes. Could you help me understand:

  1. Are the test/e2e failures known flaky tests on main?
  2. Is there anything I can do to help get this PR merged?

Thanks!

@sicnuyudidi

Copy link
Copy Markdown
Contributor Author

Hi @teknium1 @OutThisLife, this PR fixes the quote field handling for quoted reply messages in Feishu. Ready for review, could you take a look? Thanks!

@Bartok9

Bartok9 commented May 18, 2026

Copy link
Copy Markdown
Contributor

@teknium1 vouching for this one — I reviewed the change on May 10 (closed my duplicate #23079 in its favor). It's a 17-line defensive fallback (quote.get('message_id') or quote.get('id')) in gateway/platforms/feishu.py that handles the Feishu SDK's old/new field names for quoted reply messages. Same pattern the Signal adapter already uses.

The CI failures on this PR (test, e2e, Windows footguns, supply-chain scan) are unrelated — they're known-red on main right now and don't touch the Feishu code path. Safe to merge.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the missing quoted-reply context. The current adapter still does not inspect message.quote (plugins/platforms/feishu/adapter.py:3253-3259), so the underlying request remains relevant.

Problems

  • The active adapter moved from gateway/platforms/feishu.py to plugins/platforms/feishu/adapter.py in 552adbe0827c32df8ed9bb19e908c26eff43add7; this PR must be ported there.
  • Please retain the current root_idthread_id fallback at plugins/platforms/feishu/adapter.py:3252. It was intentionally introduced by a79b0ec46157efc91537e634a3dcc44a76f6dc7e for Feishu topic routing.
  • The proposed quote_text fallback is overwritten if quote_id exists and _fetch_message_text() fails; that helper returns None for failed lookups at plugins/platforms/feishu/adapter.py:4171-4175.
  • Add regression coverage: existing coverage only exercises parent_id (tests/gateway/test_feishu.py:2000-2036).

Suggested changes

  • Port quote extraction to the plugin adapter, preserve topic routing, retain quote text after a failed lookup, and test both quote ID keys plus the lookup-failure path.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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
@Bartok9

Bartok9 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Thanks for the detailed review @teknium1 — all four points are actionable. I'll port the quote extraction to plugins/platforms/feishu/adapter.py, preserve the intentional root_idthread_id topic-routing fallback (a79b0ec), keep the quote_text value even when _fetch_message_text() returns None, and add regression coverage for both quote ID keys plus the lookup-failure path. Pushing an updated commit shortly.

@Bartok9

Bartok9 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Maintainer review items addressed (port)

Could not push to sicnuyudidi:fix/feishu-quote-field (403 from Bartok9 to the head fork). Ported commit is ready to cherry-pick or re-PR:

What changed vs review

  1. Ported quote extraction into plugins/platforms/feishu/adapter.py (~3252+) — active adapter path after 552adbe.
  2. Preserved root_idthread_id fallback (topic routing from a79b0ec).
  3. Kept literal quote_text when _fetch_message_text() returns None (failed lookup no longer blanks quote text).
  4. Coverage in tests/gateway/test_feishu.py (TestAdapterBehavior):
    • test_process_inbound_message_uses_quote_message_id_key
    • test_process_inbound_message_uses_quote_id_key
    • test_process_inbound_message_retains_quote_text_when_lookup_fails
  5. No edit to obsolete-only gateway/platforms/feishu.py on current main (adapter lives under plugins).

Tests

python3 -m pytest tests/gateway/test_feishu.py::TestAdapterBehavior::test_process_inbound_message_fetches_reply_to_text \
  tests/gateway/test_feishu.py::TestAdapterBehavior::test_process_inbound_message_uses_quote_message_id_key \
  tests/gateway/test_feishu.py::TestAdapterBehavior::test_process_inbound_message_uses_quote_id_key \
  tests/gateway/test_feishu.py::TestAdapterBehavior::test_process_inbound_message_retains_quote_text_when_lookup_fails -q
# 4 passed

@sicnuyudidi if you can pull this commit onto fix/feishu-quote-field (or enable a collaborator push), CI can re-run on #22935. Otherwise I can open a small follow-up PR against main that supersedes this one with credit.

@alt-glitch alt-glitch added P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins needs-decision Awaiting maintainer decision before any implementation and removed comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists labels Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have 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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feishu adapter doesn't handle quote field for quoted replies

4 participants