-
-
Notifications
You must be signed in to change notification settings - Fork 44
feat(core): 可选地将合并转发聊天记录 ID 附加到上下文,并追加[聊天记录]标记 #714
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: v1-dev
Are you sure you want to change the base?
feat(core): 可选地将合并转发聊天记录 ID 附加到上下文,并追加[聊天记录]标记 #714
Conversation
Walkthrough在配置接口中新增布尔字段 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Incoming Message
participant Middleware as read_chat_message
participant Transformer as Message Transformer
participant Message as transformedMessage
Client->>Middleware: 发送包含元素的原始消息
Middleware->>Transformer: 拦截 forward/message 钩子
Transformer->>Middleware: 初始化/更新内部状态(state.ids)
Middleware->>Middleware: 遍历元素,检测 isForwardMessageElement\n调用 pickForwardMessageId -> normalizeForwardMessageId -> 收集到 state.ids
alt config.attachForwardMsgIdToContext && state.ids 非空
Middleware->>Message: 确保 additional_kwargs 存在
Middleware->>Message: 写入 additional_kwargs.forwardMessageIds = state.ids
Middleware->>Message: 调用 addMessageContent("[聊天记录]")
Middleware->>Message: 移除内部键 forwardHistoryInternalKey
end
Middleware->>Transformer: 将已处理的 transformedMessage 继续下游处理
预计代码审查工作量🎯 3 (中等) | ⏱️ ~20 分钟 诗篇
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @CookSleep, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在解决 ChatLuna 在处理合并转发聊天记录时可能丢失关键信息的问题。通过引入一个可选的配置项,它允许系统在检测到转发消息时,将其相关的 ID 附加到消息上下文中,并向消息内容追加一个 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
本次 PR 引入了一项处理转发聊天记录的新功能,这是一个很有价值的补充。实现结构清晰,通过增加新的配置项和相应的逻辑来检测、处理和标记转发消息。代码总体上是清晰的,并且遵循了 PR 描述中的逻辑。
我的主要反馈与改进新添加的辅助函数中的 TypeScript 类型有关。为消息元素使用 Koishi 的正确类型将增强类型安全性和代码可读性。对此我提供了一些具体的建议。
总的来说,这是一次不错的贡献。
| function hasForwardMessageElement(elements: h[]): boolean { | ||
| return elements.some((element) => { | ||
| if (!element) return false | ||
| if (isForwardMessageElement(element)) return true | ||
|
|
||
| const children = (element as unknown as { children?: unknown }).children | ||
| return ( | ||
| Array.isArray(children) && | ||
| children.length > 0 && | ||
| hasForwardMessageElement(children as h[]) | ||
| ) | ||
| }) | ||
| } |
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.
在 TypeScript 中,h 是 Koishi 框架中用于创建消息元素的函数,而不是一个类型。消息元素的正确类型应该是 h.Element。在此函数中,h 被错误地用作类型注解,这导致了需要使用 as unknown 这样的类型断言来访问 children 属性,降低了代码的类型安全性和可读性。
建议将类型注解中的 h 修正为 h.Element,并移除不必要的类型断言,使代码更加健壮和清晰。
| function hasForwardMessageElement(elements: h[]): boolean { | |
| return elements.some((element) => { | |
| if (!element) return false | |
| if (isForwardMessageElement(element)) return true | |
| const children = (element as unknown as { children?: unknown }).children | |
| return ( | |
| Array.isArray(children) && | |
| children.length > 0 && | |
| hasForwardMessageElement(children as h[]) | |
| ) | |
| }) | |
| } | |
| function hasForwardMessageElement(elements: h.Element[]): boolean { | |
| return elements.some((element) => { | |
| if (!element) return false | |
| if (isForwardMessageElement(element)) return true | |
| const children = element.children | |
| return ( | |
| Array.isArray(children) && | |
| children.length > 0 && | |
| hasForwardMessageElement(children) | |
| ) | |
| }) | |
| } |
| function collectForwardMessageIds(elements: h[]): string[] { | ||
| const forwardMessageIds = new Set<string>() | ||
|
|
||
| const visit = (element: h) => { | ||
| if (!element) return | ||
|
|
||
| if (isForwardMessageElement(element)) { | ||
| const attrs = element.attrs ?? {} | ||
| for (const key of [ | ||
| 'id', | ||
| 'message_id', | ||
| 'messageId', | ||
| 'res_id', | ||
| 'resId', | ||
| 'forward_id', | ||
| 'forwardId' | ||
| ]) { | ||
| const normalizedId = normalizeForwardMessageId( | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (attrs as any)[key] | ||
| ) | ||
| if (normalizedId) { | ||
| forwardMessageIds.add(normalizedId) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const children = (element as unknown as { children?: unknown }).children | ||
| if (Array.isArray(children) && children.length > 0) { | ||
| ;(children as h[]).forEach(visit) | ||
| } | ||
| } | ||
|
|
||
| elements.forEach(visit) | ||
| return Array.from(forwardMessageIds) | ||
| } |
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.
与 hasForwardMessageElement 函数类似,此函数也错误地将 h 用作类型。应使用 h.Element 作为正确的元素类型。这可以消除对 children 属性的不安全访问,并提高代码的整体质量。
修正类型后,可以安全地直接访问 element.children 并对其进行迭代,而无需 as unknown 或 as h[] 等类型断言。
function collectForwardMessageIds(elements: h.Element[]): string[] {
const forwardMessageIds = new Set<string>()
const visit = (element: h.Element) => {
if (!element) return
if (isForwardMessageElement(element)) {
const attrs = element.attrs ?? {}
for (const key of [
'id',
'message_id',
'messageId',
'res_id',
'resId',
'forward_id',
'forwardId'
]) {
const normalizedId = normalizeForwardMessageId(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(attrs as any)[key]
)
if (normalizedId) {
forwardMessageIds.add(normalizedId)
}
}
}
const children = element.children
if (Array.isArray(children) && children.length > 0) {
children.forEach(visit)
}
}
elements.forEach(visit)
return Array.from(forwardMessageIds)
}| function isForwardMessageElement(element: h): boolean { | ||
| if (!element) return false | ||
| if (element.type === 'forward') return true | ||
| if (element.type !== 'message') return false | ||
|
|
||
| const attrs = element.attrs ?? {} | ||
| const forward = attrs['forward'] | ||
|
|
||
| if ( | ||
| forward === true || | ||
| forward === 'true' || | ||
| forward === 1 || | ||
| forward === '1' | ||
| ) { | ||
| return true | ||
| } | ||
|
|
||
| return ( | ||
| attrs['forward_id'] != null || | ||
| attrs['forwardId'] != null || | ||
| attrs['res_id'] != null || | ||
| attrs['resId'] != null | ||
| ) | ||
| } |
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.
此函数同样存在将 h 误用为类型的问题,应修正为 h.Element。
此外,对 forward 属性的检查可以写得更简洁。通过将其转换为字符串进行比较,可以减少布尔表达式的复杂性,使其更易于阅读和维护。
function isForwardMessageElement(element: h.Element): boolean {
if (!element) return false
if (element.type === 'forward') return true
if (element.type !== 'message') return false
const attrs = element.attrs ?? {}
if (['true', '1'].includes(String(attrs.forward))) {
return true
}
return (
attrs['forward_id'] != null ||
attrs['forwardId'] != null ||
attrs['res_id'] != null ||
attrs['resId'] != null
)
}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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/core/src/middlewares/chat/read_chat_message.ts`:
- Around line 449-467: 当前采集逻辑在 collectForwardMessageIds(使用
normalizeForwardMessageId)里无差别地遍历包括 'id' 的所有键,导致当元素同时存在 forward=true 与自身 id
时可能误取元素自身 ID;为修复,要改为优先使用专用键
['forward_id','forwardId','res_id','resId','message_id','messageId'] 并在找到第一个有效
normalizedId 后停止(或仅在上述专用键全部缺失时才回退到 'id'),或者直接移除 'id' 并在 isForwardMessageElement
中同步将 'id' 纳入判断以保持语义一致;定位到使用 normalizeForwardMessageId 的循环并调整键优先级与短路逻辑(或移除 'id'
并更新 isForwardMessageElement)以避免采集到错误的 ID。
🧹 Nitpick comments (2)
packages/core/src/middlewares/chat/read_chat_message.ts (2)
29-35: 两次递归遍历可合并为一次。
hasForwardMessageElement和collectForwardMessageIds都递归遍历同一棵元素树。可以只调用collectForwardMessageIds,根据返回结果是否为空来判断hasForwardHistory,避免重复遍历。♻️ 建议合并
- const hasForwardHistory = - config.attachForwardMsgIdToContext && - hasForwardMessageElement(message as h[]) - - const forwardMessageIds = hasForwardHistory - ? collectForwardMessageIds(message as h[]) - : [] + const forwardMessageIds = config.attachForwardMsgIdToContext + ? collectForwardMessageIds(message as h[]) + : [] + + const hasForwardHistory = forwardMessageIds.length > 0注意:此修改会改变语义——如果存在转发元素但未收集到任何 ID,
hasForwardHistory将为false,[聊天记录]标记也不会被追加。如果希望"只要存在转发元素就追加标记",可以改为使用collectForwardMessageIds内部同时返回一个布尔标志。
434-438:children访问方式可以简化。koishi 的
h类型本身就有children: h[]属性,无需通过unknown中转。当前写法可以正常工作,但不够简洁。♻️ 简化 children 访问
- const children = (element as unknown as { children?: unknown }).children - return ( - Array.isArray(children) && - children.length > 0 && - hasForwardMessageElement(children as h[]) - ) + return ( + element.children?.length > 0 && + hasForwardMessageElement(element.children) + )
collectForwardMessageIds中同理:- const children = (element as unknown as { children?: unknown }).children - if (Array.isArray(children) && children.length > 0) { - ;(children as h[]).forEach(visit) - } + if (element.children?.length > 0) { + element.children.forEach(visit) + }Also applies to: 470-473
dingyi222666
left a comment
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.
使用 https://chatluna.chat/development/connect-to-core-services/message-read.html 相关 API 实现,稍加修改后,伪装也能支持读取。
ctx.chatluna.messageTransformer.intercept(
'at',
async (session, element, message) => {
const name = element.attrs['name']
const id = element.attrs['id']
if (id !== session.bot.selfId) {
addMessageContent(
message,
`<at ${name != null ? `name="${name}"` : ''} id="${id}"/>`
)
}
}
)```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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/core/src/middlewares/chat/read_chat_message.ts`:
- Around line 467-478: pickForwardMessageId currently only looks for
message_id/messageId while isForwardMessageElement treats
forward_id/forwardId/res_id/resId as valid forward markers; update
pickForwardMessageId to check the full set of keys (forward_id, forwardId,
res_id, resId, then fallback to message_id, messageId) in that priority order
and keep using normalizeForwardMessageId for each candidate; also apply the same
key-range change to the other similar extractor referenced around the 480-494
block so forward-detected elements have their IDs collected into state.ids
consistently with isForwardMessageElement.
| function pickForwardMessageId(element: h.Element): string | null { | ||
| const attrs = (element.attrs ?? {}) as Record<string, unknown> | ||
|
|
||
| // Only use the common message id field used across other message APIs. | ||
| // Keep both snake_case and camelCase for adapter compatibility. | ||
| for (const key of ['message_id', 'messageId']) { | ||
| const normalizedId = normalizeForwardMessageId(attrs[key]) | ||
| if (normalizedId) return normalizedId | ||
| } | ||
|
|
||
| return null | ||
| } |
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.
pickForwardMessageId 提取的键范围与 isForwardMessageElement 检测的键范围不一致。
isForwardMessageElement 通过 forward_id、forwardId、res_id、resId 判定元素为转发消息,但 pickForwardMessageId 仅从 message_id/messageId 提取 ID。这意味着仅凭 res_id 或 forward_id 识别的转发元素,其 ID 将无法被收集到 state.ids 中。
PR 描述中明确提到需要收集 forward_id、forwardId、res_id、resId 等字段,建议将这些键也加入 pickForwardMessageId 的提取范围,并按优先级排列(专用键优先,通用键兜底)。
🔧 建议的修复
function pickForwardMessageId(element: h.Element): string | null {
const attrs = (element.attrs ?? {}) as Record<string, unknown>
- // Only use the common message id field used across other message APIs.
- // Keep both snake_case and camelCase for adapter compatibility.
- for (const key of ['message_id', 'messageId']) {
+ // Prefer specific forward ID keys, fall back to general message ID.
+ // Keep both snake_case and camelCase for adapter compatibility.
+ for (const key of [
+ 'forward_id', 'forwardId',
+ 'res_id', 'resId',
+ 'message_id', 'messageId'
+ ]) {
const normalizedId = normalizeForwardMessageId(attrs[key])
if (normalizedId) return normalizedId
}
return null
}Also applies to: 480-494
🤖 Prompt for AI Agents
In `@packages/core/src/middlewares/chat/read_chat_message.ts` around lines 467 -
478, pickForwardMessageId currently only looks for message_id/messageId while
isForwardMessageElement treats forward_id/forwardId/res_id/resId as valid
forward markers; update pickForwardMessageId to check the full set of keys
(forward_id, forwardId, res_id, resId, then fallback to message_id, messageId)
in that priority order and keep using normalizeForwardMessageId for each
candidate; also apply the same key-range change to the other similar extractor
referenced around the 480-494 block so forward-detected elements have their IDs
collected into state.ids consistently with isForwardMessageElement.
背景
在使用 Napcat/合并转发聊天记录时,ChatLuna 的输入消息转换链路可能会过滤掉转发记录元素,导致上下文中无法保留“存在聊天记录”这一信息。
改动
attachForwardMsgIdToContext(位于“对话行为选项”末尾,默认关闭)。id、message_id、messageId、res_id、resId、forward_id、forwardId)。inputMessage.additional_kwargs.forwardMessageIds。[聊天记录],避免在过滤后消息为空而被中间件直接丢弃。兼容性