-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(channels): extract quoted DingTalk text, richText, and card replies #9537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,13 +73,22 @@ import type { | |
|
|
||
| interface DingTalkRichTextPart { | ||
| type?: string; | ||
| msgType?: string; | ||
| text?: string; | ||
| content?: string; | ||
| downloadCode?: string; | ||
| atName?: string; | ||
| } | ||
|
|
||
| interface DingTalkCardElement { | ||
| elementType?: string; | ||
| value?: string; | ||
| children?: DingTalkCardElement[]; | ||
| } | ||
|
|
||
| interface DingTalkMessageContent { | ||
| text?: string; | ||
| content?: string; | ||
| richText?: DingTalkRichTextPart[]; | ||
| downloadCode?: string; | ||
| fileName?: string; | ||
|
|
@@ -89,6 +98,7 @@ interface DingTalkMessageContent { | |
| chatRecord?: unknown; | ||
| records?: unknown; | ||
| messages?: unknown; | ||
| cardContent?: DingTalkCardElement[]; | ||
| } | ||
|
|
||
| interface DingTalkRepliedMsg { | ||
|
|
@@ -2094,8 +2104,6 @@ export class DingtalkChannel extends ChannelBase { | |
| const isReplyToBot = | ||
| !!data.chatbotUserId && replied.senderId === data.chatbotUserId; | ||
|
|
||
| // Note: DingTalk doesn't include content for interactiveCard replies | ||
| // (bot responses sent via webhook). Only user message quotes have text. | ||
| const text = this.summarizeRepliedContent(replied); | ||
| const downloadCode = replied.content?.downloadCode; | ||
| const mediaType = this.mediaTypeFromMsgType(replied.msgType); | ||
|
|
@@ -2159,25 +2167,34 @@ export class DingtalkChannel extends ChannelBase { | |
| } | ||
|
|
||
| /** | ||
| * Build a text summary from a repliedMsg, handling text, richText, chat | ||
| * records, and media message types with placeholders. | ||
| * Build a text summary from a repliedMsg, handling text, markdown, | ||
| * richText, chat records, interactiveCard, and media message types with | ||
| * placeholders. | ||
| */ | ||
| private summarizeRepliedContent(replied: DingTalkRepliedMsg): string { | ||
| const msgType = replied.msgType; | ||
| const msgType = replied.msgType?.toLowerCase(); | ||
| const content = replied.content; | ||
|
|
||
| // Text quotes carry the body in content.content; markdown uses | ||
| // content.text, which the generic fallback below handles. | ||
| if (msgType === 'text' && content?.content?.trim()) { | ||
| return content.content.trim(); | ||
| } | ||
|
|
||
| // Direct text content | ||
| if (content?.text?.trim()) { | ||
| return content.text.trim(); | ||
| } | ||
|
|
||
| // RichText: concatenate text parts, placeholder for images | ||
| // RichText: concatenate text parts, placeholder for images. Quoted | ||
| // richText segments use {msgType, content} rather than {type, text}. | ||
| if (content?.richText && Array.isArray(content.richText)) { | ||
| const parts: string[] = []; | ||
| for (const part of content.richText) { | ||
| const partType = part.type || 'text'; | ||
| if (partType === 'text' && part.text) { | ||
| parts.push(part.text); | ||
| const partType = (part.type || part.msgType || 'text').toLowerCase(); | ||
| const partText = part.text ?? part.content; | ||
| if (partType === 'text' && partText?.trim()) { | ||
| parts.push(partText.trim()); | ||
| } else if (partType === 'picture') { | ||
| parts.push('[image]'); | ||
| } else if (partType === 'at' && part.atName) { | ||
|
|
@@ -2188,7 +2205,7 @@ export class DingtalkChannel extends ChannelBase { | |
| if (summary) return summary; | ||
| } | ||
|
|
||
| if (msgType === 'chatRecord') { | ||
| if (msgType === 'chatrecord') { | ||
| // The quote budget, not the record budget: this text becomes | ||
| // `envelope.referencedText`, which `ChannelBase` renders through | ||
| // `sanitizeQuotedText(..., 500)`. Rendered to 4000 the quote arrives cut | ||
|
|
@@ -2203,11 +2220,33 @@ export class DingtalkChannel extends ChannelBase { | |
| return text; | ||
| } | ||
|
|
||
| // Interactive cards (usually quoted bot replies) have no flat text | ||
| // field; the body lives in TEXT nodes of the cardContent element tree. | ||
| if (msgType === 'interactivecard') { | ||
| return this.collectCardText(content?.cardContent); | ||
| } | ||
|
Comment on lines
+2225
to
+2227
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The empty/absent Add one test: an interactiveCard quote with 中文说明这个新分支的空/缺失 补一个测试:interactiveCard 引用且 — qwen3.8-max via Qwen Code /review (v0.21.14) |
||
|
|
||
| // Media type placeholders. Shared with the chat-record entry formatter so | ||
| // the same message type is never described two ways to the model. | ||
| return mediaTypePlaceholder(msgType, content?.fileName) ?? ''; | ||
| } | ||
|
|
||
| private collectCardText(nodes: DingTalkCardElement[] | undefined): string { | ||
| const segments: string[] = []; | ||
| const walk = (items: DingTalkCardElement[]): void => { | ||
| for (const node of items) { | ||
| if (!node || typeof node !== 'object') continue; | ||
| if (node.elementType === 'TEXT' && typeof node.value === 'string') { | ||
| const trimmed = node.value.trim(); | ||
| if (trimmed) segments.push(trimmed); | ||
| } | ||
| if (Array.isArray(node.children)) walk(node.children); | ||
| } | ||
| }; | ||
| if (Array.isArray(nodes)) walk(nodes); | ||
| return segments.join('\n'); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Add a card fixture with two TEXT nodes and assert the joined output, e.g. 中文说明
补一个含两个 TEXT 节点的卡片用例,断言拼接结果,例如 — qwen3.8-max via Qwen Code /review (v0.21.14) |
||
| } | ||
|
|
||
| /** | ||
| * Map a DingTalk message type to the media type used for downloads. Shared | ||
| * by the direct-media (`extractContent`) and quoted-media | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] The richText loop still supports the legacy
{type, text}part shape via thepart.type ||fallback, but no test pins that backward-compatible branch — the new quoted-richText tests use only the{msgType, content}shape. Droppingpart.type ||(mutating to(part.msgType || 'text')) leaves the full package suite green (verified: 320/320 pass after the mutation), so a future refactor could silently break legacy-shape quotes — a{type:'picture'}part losing its[image]placeholder,{type:'text'}text skipped — while the suite stays green.Consider adding one legacy-shaped fixture to the new
quoted message contextblock, e.g.and asserting
referencedText === 'legacy body[image]'.中文说明
richText 循环目前仍通过
part.type ||回退兼容旧的{type, text}段形态,但没有任何测试固定这条向后兼容分支——新增的引用 richText 测试只用了{msgType, content}形态。删掉part.type ||(突变为(part.msgType || 'text'))后整个包的测试套件依然全绿(已验证:突变后 320/320 通过),因此未来的重构可能在测试全绿时悄悄破坏旧形态的引用——{type:'picture'}段丢失[image]占位符、{type:'text'}文本被跳过。建议在新的
quoted message context用例组补一个旧形态用例:并断言
referencedText === 'legacy body[image]'。— qwen3.8-max via Qwen Code /review (v0.21.14)