Skip to content

fix(dingtalk): preserve inbound quoted message context - #9464

Open
chinatsu1124 wants to merge 3 commits into
AstrBotDevs:masterfrom
chinatsu1124:fix/9463-dingtalk-quoted-message
Open

fix(dingtalk): preserve inbound quoted message context#9464
chinatsu1124 wants to merge 3 commits into
AstrBotDevs:masterfrom
chinatsu1124:fix/9463-dingtalk-quoted-message

Conversation

@chinatsu1124

@chinatsu1124 chinatsu1124 commented Jul 30, 2026

Copy link
Copy Markdown

Fixes #9463.

DingTalk reply callbacks expose quoted messages through text.isReplyMsg and text.repliedMsg, but the adapter previously converted only the new message body. Quoted text was discarded, while quoted images were represented only by placeholders and never reached the model as image components.

Modifications / 改动点

  • Convert current DingTalk text.repliedMsg payloads into AstrBot Reply components.

  • Retain compatibility with legacy top-level quoteMessage payloads.

  • Preserve the quoted message ID, text, sender metadata, and timestamp when available.

  • Download standalone quoted pictures with downloadCode and robotCode, then attach them as Image components in Reply.chain.

  • Preserve the original text/image order for quoted rich-text messages and resolve embedded pictures into Image components.

  • Fall back to readable image placeholders when media download information is unavailable or the download fails.

  • Add adapter tests for current text replies, legacy quote payloads, standalone picture replies, and rich-text replies containing pictures.

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

$ uv run pytest tests/test_dingtalk_adapter.py -q
14 passed, 1 warning in 76.51s

$ uv run ruff check astrbot/core/platform/sources/dingtalk/dingtalk_adapter.py
All checks passed!

The warning is the pre-existing audioop deprecation warning.


Checklist / 检查清单

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. area:platform The bug / feature is about IM platform adapter, such as QQ, Lark, Telegram, WebChat and so on. labels Jul 30, 2026
@dosubot

dosubot Bot commented Jul 30, 2026

Copy link
Copy Markdown

📄 Knowledge review

Dosu skipped reviewing this PR because your organization has used its 200 included credits for the month. Your usage will reset on 2026-08-01. To have Dosu review this PR before then, ask your organization admin to upgrade to a pro account.


Leave Feedback Ask Dosu about AstrBot Add Dosu to your team

@sourcery-ai sourcery-ai Bot 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.

Hey - I've found 1 issue, and left some high level feedback:

  • The hard-coded placeholder labels for non-text content (e.g. "[Image]", "[File]", "[Chat history]") could be centralized or made configurable/localizable to avoid scattering user-facing strings inside the adapter.
  • Inserting the Reply component at index 0 of abm.message changes the message ordering; if other consumers rely on the first element being the leading At/Plain, consider documenting this behavior or appending the Reply and handling ordering at a higher layer.
  • The timestamp normalization logic in _parse_reply uses a bare > 10_000_000_000 heuristic; extracting this into a small helper or clarifying the expected units (ms vs s) would make the intent easier to understand and maintain.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The hard-coded placeholder labels for non-text content (e.g. "[Image]", "[File]", "[Chat history]") could be centralized or made configurable/localizable to avoid scattering user-facing strings inside the adapter.
- Inserting the Reply component at index 0 of `abm.message` changes the message ordering; if other consumers rely on the first element being the leading At/Plain, consider documenting this behavior or appending the Reply and handling ordering at a higher layer.
- The timestamp normalization logic in `_parse_reply` uses a bare `> 10_000_000_000` heuristic; extracting this into a small helper or clarifying the expected units (ms vs s) would make the intent easier to understand and maintain.

## Individual Comments

### Comment 1
<location path="astrbot/core/platform/sources/dingtalk/dingtalk_adapter.py" line_range="111" />
<code_context>
             return dingtalk_id[len(prefix) :]
         return dingtalk_id or "unknown"

+    def _parse_reply(
+        self,
+        message: dingtalk_stream.ChatbotMessage,
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring `_parse_reply` by extracting repeated defensive dict handling and quote-processing into small helper methods to simplify the control flow and clarify responsibilities.

You can reduce complexity by extracting the repeated defensive patterns and separating responsibilities into small helpers, while preserving behaviour.

### 1. Encapsulate defensive dict access

The `getattr` + `isinstance` pattern is repeated; a small helper keeps the flow cleaner:

```python
def _safe_dict(self, obj: object | None) -> dict:
    return obj if isinstance(obj, dict) else {}
```

Then at the top of `_parse_reply`:

```python
text_content = getattr(message, "text", None)
text_extensions = self._safe_dict(getattr(text_content, "extensions", None))
message_extensions = self._safe_dict(getattr(message, "extensions", None))
```

### 2. Split `_parse_reply` into focused helpers

You can keep `_parse_reply` as an orchestration method and move the logic into small helpers:

```python
def _extract_quote(self, text_ext: dict, msg_ext: dict) -> dict | None:
    replied_message = text_ext.get("repliedMsg")
    legacy_quote = msg_ext.get("quoteMessage")

    if text_ext.get("isReplyMsg") and isinstance(replied_message, dict):
        return replied_message
    if isinstance(legacy_quote, dict):
        return legacy_quote
    return None
```

```python
def _extract_quoted_text(self, quote: dict) -> str:
    message_type = str(quote.get("msgType") or quote.get("msgtype") or "text").strip()
    content = quote.get("content")
    if not isinstance(content, (dict, str)):
        content = quote.get("text")
    if isinstance(content, str):
        content = {"text": content}
    if not isinstance(content, dict):
        content = {}

    # main text / content keys
    for key in ("text", "content"):
        value = content.get(key)
        if isinstance(value, str) and value.strip():
            return value.strip()

    # richText handling
    if message_type == "richText":
        parts: list[str] = []
        rich_text = content.get("richText")
        if isinstance(rich_text, list):
            for item in rich_text:
                if not isinstance(item, dict):
                    continue
                item_text = item.get("content") or item.get("text")
                if isinstance(item_text, str) and item_text.strip():
                    parts.append(item_text.strip())
                elif (item.get("msgType") or item.get("type")) == "picture":
                    parts.append("[Image]")
        return "".join(parts)

    # generic placeholders / file
    placeholders = {
        "picture": "[Image]",
        "audio": "[Audio]",
        "voice": "[Audio]",
        "video": "[Video]",
        "interactiveCard": "[Card]",
        "chatRecord": "[Chat history]",
    }
    if message_type == "file":
        file_name = content.get("fileName")
        if isinstance(file_name, str) and file_name:
            return f"[File: {file_name}]"
        return "[File]"

    return placeholders.get(message_type, "")
```

```python
def _extract_quote_metadata(self, quote: dict, msg_ext: dict) -> tuple[str, str, int]:
    quote_id = str(quote.get("msgId") or msg_ext.get("originalMsgId") or "").strip()

    sender_id = str(quote.get("senderId") or "")
    if sender_id:
        sender_id = self._id_to_sid(sender_id)

    created_at = quote.get("createdAt") or 0
    try:
        quote_time = int(created_at)
        if quote_time > 10_000_000_000:
            quote_time //= 1000
    except (TypeError, ValueError):
        quote_time = 0

    return quote_id, sender_id, quote_time
```

Then `_parse_reply` becomes a linear, high-level flow:

```python
def _parse_reply(
    self,
    message: dingtalk_stream.ChatbotMessage,
) -> Reply | None:
    text_content = getattr(message, "text", None)
    text_extensions = self._safe_dict(getattr(text_content, "extensions", None))
    message_extensions = self._safe_dict(getattr(message, "extensions", None))

    quote = self._extract_quote(text_extensions, message_extensions)
    if quote is None:
        return None

    quoted_text = self._extract_quoted_text(quote)
    quote_id, sender_id, quote_time = self._extract_quote_metadata(quote, message_extensions)

    if not quote_id and not quoted_text:
        return None

    return Reply(
        id=quote_id,
        chain=[Plain(quoted_text)] if quoted_text else [],
        sender_id=sender_id,
        sender_nickname=str(quote.get("senderNick") or ""),
        time=quote_time,
        message_str=quoted_text,
        text=quoted_text,
    )
```

This keeps all your current behaviours (including edge-case handling) but lowers nesting, makes responsibilities clearer, and should address the complexity concerns without changing functionality.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread astrbot/core/platform/sources/dingtalk/dingtalk_adapter.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:platform The bug / feature is about IM platform adapter, such as QQ, Lark, Telegram, WebChat and so on. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] 钉钉引用消息的文本及图片内容未被完整解析

2 participants