-
-
Notifications
You must be signed in to change notification settings - Fork 41
[Refactor] Migrate to Node.js crypto APIs and enhance prompt context #590
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
Conversation
- Remove unused @types/uuid, @types/md5, @types/jsdom, @types/katex dependencies across all packages - Move uuid and md5 from core dependencies to devDependencies where appropriate - Replace uuid v4 with Node.js built-in crypto.randomUUID() in core middleware - Standardize JSON formatting in root package.json - Move @koishijs/plugin-notifier from dependencies to devDependencies in core package This cleanup reduces the dependency footprint and leverages Node.js built-in APIs where possible.
- Replace uuid with crypto.randomUUID() in database_history, chat service, and related files
- Replace md5 library with crypto.createHash('md5') in platform config and preset service
- Refactor messageTransformer.transform() to use options object instead of boolean quote parameter
- Update all transform() call sites to pass options object with quote and includeQuoteReply flags
- Improve API consistency and maintainability across message transformation logic
This change eliminates external dependencies in favor of Node.js built-in crypto module
and modernizes the message transformer API for better extensibility.
…in execution - Pass session through configurable parameter in chat chains and browsing chain - Add prompt variable extraction from message content to chain requests - Update ChatLunaChatPrompt to accept and forward configurable with session context - Enable long-memory plugin to access session data via configurable in prompt functions - Add Guild layer support to long-memory configuration and layer engines - Improve error handling in long-memory prompt variable and tool implementations - Fix chat history content extraction to use getMessageContent for complex content This change enables prompt templates and extensions to access session context during rendering, allowing for more dynamic and context-aware prompt generation.
Summary of ChangesHello @dingyi222666, 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! This pull request significantly modernizes the codebase by integrating Node.js's native crypto functionalities, thereby reducing reliance on third-party libraries and enhancing overall security. It also introduces a more dynamic and context-aware prompt rendering system by incorporating session data and automatically extracting prompt content. These changes collectively aim to streamline development, improve performance, and provide a more robust foundation for future features. Highlights
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
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (26)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough此变更将 session 作为 configurable 贯穿 LLM 调用链与提示渲染;对 message content 统一使用 getMessageContent 生成 prompt;MessageTransformer 改为接收 options 对象以控制引用处理;UUID/MD5 由第三方库迁移到 Node crypto;长记忆扩展重构记忆聚合与输出格式并新增 Guild 层。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as 用户 / 会话
participant MW as 中间件
participant MT as MessageTransformer
participant CH as Chat Chain
participant PR as Prompt Renderer
participant LLM as 模型
U->>MW: 发消息
MW->>MT: transform(elements, model, options{quote:false, includeQuoteReply})
MT-->>MW: 标准化 Message(content)
MW->>CH: call({ message, variables, session, ... })
CH->>PR: formatMessages({ variables + prompt:getMessageContent(content), configurable:{session} })
PR-->>CH: 渲染后的消息/系统提示
CH->>LLM: 请求(包含 configurable.session)
LLM-->>CH: 响应
CH-->>MW: ChainValues
MW-->>U: 输出结果
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
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
This pull request is a solid piece of work that modernizes the codebase and enhances its capabilities. The migration from external crypto dependencies like uuid and md5 to Node.js's built-in crypto module is a commendable move towards reducing the dependency footprint and leveraging standard APIs. The introduction of session context into the prompt rendering system is a significant feature enhancement, enabling more dynamic and personalized interactions. I also appreciate the dependency cleanup across various packages and the refactoring of messageTransformer.transform to use an options object, which improves API design and extensibility. I've found one minor issue regarding logging which I've commented on. Overall, these changes contribute positively to the project's maintainability and functionality.
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/llm-core/memory/message/database_history.ts (1)
318-336: 潜在 NPE:_saveMessage 在未加载会话时直接访问 _serializedChatHistory首次调用 addUserMessage/addAIChatMessage 时,若未先调用 getMessages()/loadConversation(),_serializedChatHistory 可能为 undefined,.find 会抛错。建议在 _saveMessage 入口确保已加载会话。
应用以下补丁在方法开头加载会话,避免空引用:
private async _saveMessage(message: BaseMessage) { + await this.loadConversation() const lastedMessage = this._serializedChatHistory.find( (item) => item.id === this._latestId )packages/core/src/middlewares/room/resolve_room.ts (1)
66-76: 使用 splitContent.join(' ') 替换 splitContent.concat(' ')
h.text 需接收字符串,splitContent.concat 返回数组,应改为 splitContent.join 返回字符串;transform 签名支持传入 undefined 以使用默认 message,无需变更。- context.options.inputMessage = - await ctx.chatluna.messageTransformer.transform( - session, - [h.text(splitContent.concat(' '))], - matchedRoom.model, - undefined, - { - quote: false, - includeQuoteReply: config.includeQuoteReply - } - ) + context.options.inputMessage = + await ctx.chatluna.messageTransformer.transform( + session, + [h.text(splitContent.join(' '))], + matchedRoom.model, + undefined, + { + quote: false, + includeQuoteReply: config.includeQuoteReply + } + )
🧹 Nitpick comments (4)
packages/core/src/llm-core/memory/message/database_history.ts (1)
24-33: 类型改进:_latestId 允许为 null代码多处将 _latestId 赋值为 null(如 clear、removeAllToolAndFunctionMessages),当前声明为 string 会造成类型不一致。建议显式标注为 string | null。
- private _latestId: string + private _latestId: string | nullpackages/extension-long-memory/src/plugins/tool.ts (1)
128-131: 为错误路径补充日志是正确的,但建议统一日志接口与上下文目前本文件工具类有的使用 this.ctx.logger,有的使用外部 logger。建议统一为同一记录器,并附加最小上下文(tool 名称、会话标识)便于排查,同时避免记录可能包含敏感文本的详细错误信息。
packages/core/src/llm-core/platform/config.ts (1)
125-131: MD5 哈希迁移到 Node crypto 正确;建议稳定序列化以降低碰撞风险当前仅对 values.join('') 求哈希,理论上存在字符串拼接歧义导致的碰撞风险。建议对有序值数组进行 JSON 序列化后再哈希,更稳健。
- return createHash('md5').update(values.join('')).digest('hex') + return createHash('md5').update(JSON.stringify(values)).digest('hex')packages/extension-long-memory/src/plugins/prompt_varaiable.ts (1)
41-45: 避免在日志中输出完整文档内容,改为输出计数或指纹完整日志可能包含敏感信息。建议仅记录数量或摘要。
- logger.debug(`Searched documents: ${documents}`) + logger.debug(`Searched documents count: ${documents.length}`)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
package.jsonis excluded by!**/*.jsonpackages/adapter-ollama/package.jsonis excluded by!**/*.jsonpackages/adapter-rwkv/package.jsonis excluded by!**/*.jsonpackages/core/package.jsonis excluded by!**/*.jsonpackages/extension-long-memory/package.jsonis excluded by!**/*.jsonpackages/extension-mcp/package.jsonis excluded by!**/*.jsonpackages/extension-tools/package.jsonis excluded by!**/*.jsonpackages/extension-variable/package.jsonis excluded by!**/*.jsonpackages/renderer-image/package.jsonis excluded by!**/*.jsonpackages/service-embeddings/package.jsonis excluded by!**/*.jsonpackages/service-image/package.jsonis excluded by!**/*.jsonpackages/service-search/package.jsonis excluded by!**/*.jsonpackages/service-vector-store/package.jsonis excluded by!**/*.json
📒 Files selected for processing (17)
packages/core/src/llm-core/chain/chat_chain.ts(3 hunks)packages/core/src/llm-core/chain/plugin_chat_chain.ts(2 hunks)packages/core/src/llm-core/chain/prompt.ts(9 hunks)packages/core/src/llm-core/memory/message/database_history.ts(2 hunks)packages/core/src/llm-core/platform/config.ts(2 hunks)packages/core/src/middlewares/chat/read_chat_message.ts(1 hunks)packages/core/src/middlewares/chat/rollback_chat.ts(1 hunks)packages/core/src/middlewares/model/request_model.ts(2 hunks)packages/core/src/middlewares/room/resolve_room.ts(3 hunks)packages/core/src/preset.ts(2 hunks)packages/core/src/services/chat.ts(5 hunks)packages/core/src/services/message_transform.ts(5 hunks)packages/extension-long-memory/src/index.ts(2 hunks)packages/extension-long-memory/src/plugins/prompt_varaiable.ts(1 hunks)packages/extension-long-memory/src/plugins/tool.ts(1 hunks)packages/extension-long-memory/src/utils/chat-history.ts(1 hunks)packages/service-search/src/chain/browsing_chain.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
packages/core/src/llm-core/chain/plugin_chat_chain.ts (1)
packages/core/src/utils/string.ts (1)
getMessageContent(150-166)
packages/core/src/llm-core/chain/chat_chain.ts (1)
packages/core/src/utils/string.ts (1)
getMessageContent(150-166)
packages/core/src/llm-core/chain/prompt.ts (2)
packages/core/src/services/types.ts (1)
RenderConfigurable(38-40)packages/shared-prompt-renderer/src/types.ts (1)
RenderConfigurable(19-21)
packages/extension-long-memory/src/utils/chat-history.ts (1)
packages/core/src/utils/string.ts (1)
getMessageContent(150-166)
packages/service-search/src/chain/browsing_chain.ts (1)
packages/core/src/utils/string.ts (1)
getMessageContent(150-166)
packages/extension-long-memory/src/plugins/prompt_varaiable.ts (3)
packages/extension-long-memory/src/index.ts (3)
Config(31-62)Config(64-175)logger(9-9)packages/extension-long-memory/src/service/memory.ts (1)
Context(338-340)packages/shared-prompt-renderer/src/types.ts (1)
FunctionProvider(4-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (24)
packages/extension-long-memory/src/index.ts (2)
54-56: LGTM! 类型扩展正确。
Guild层已正确添加到layer联合类型中,与 schema 定义保持一致。
66-109: LGTM! Schema 配置已正确更新。
Guild层已正确添加到:
enabledLayers的 schema 定义和默认值中layerEngines的 schema 定义和默认配置中配置结构合理,Guild 层默认使用 HippoRAG 引擎与 User 和 Preset 层保持一致。
packages/core/src/llm-core/memory/message/database_history.ts (1)
18-18: 使用 Node.js crypto.randomUUID 的迁移正确ID 生成切换到 crypto.randomUUID() 合理且与全局迁移保持一致。无需额外改动。
Also applies to: 333-349
packages/extension-long-memory/src/utils/chat-history.ts (1)
85-89: 从 JSON.stringify 切换到 getMessageContent 是正确修复更健壮地提取非字符串 content 的纯文本,避免出现序列化噪音。
packages/core/src/middlewares/room/resolve_room.ts (1)
173-175: conversationId 使用 crypto.randomUUID 合理克隆房间生成新会话 ID 的方式与全局迁移一致。
packages/core/src/preset.ts (1)
18-18: 改用 Node crypto 计算文件 MD5 正确使用 createHash('md5') 对文件 Buffer 求哈希是更原生的做法,逻辑清晰。
Also applies to: 161-164
packages/core/src/middlewares/model/request_model.ts (1)
35-36: 确认生产环境 Node 版本支持 crypto.randomUUID
当前 package.json 未指定 engines.node,请确保运行时 Node 版本 ≥14.17.0。packages/core/src/llm-core/chain/prompt.ts (4)
47-47: LGTM!新增的 configurable 字段设计合理向
ChatLunaChatPromptFormat接口添加可选的configurable字段,使得会话上下文可以贯穿整个提示渲染流程,这是一个良好的架构改进。
124-127: LGTM!configurable 参数传递正确
_formatSystemPrompts方法正确地接收configurable参数并将其传递给renderPresetTemplate调用。默认值设置为空对象确保了向后兼容性。Also applies to: 147-153
434-442: LGTM!authors note 渲染支持 configurable
_counterAuthorsNote方法正确地接收并传递configurable参数到renderTemplate调用。使用configurable ?? {}确保了即使传入 undefined 也能提供有效的空对象。
26-29: 不需要统一 RenderConfigurable 定义
prompt.ts中导入的RenderConfigurable来自外部插件,与core/src/services/types.ts中同名接口属于不同模块,无冲突。Likely an incorrect or invalid review comment.
packages/core/src/middlewares/chat/read_chat_message.ts (1)
30-40: LGTM!消息转换器调用已更新为选项对象模式对
messageTransformer.transform的调用已正确更新为接受选项对象,包括quote: false和从配置中获取的includeQuoteReply。这与message_transform.ts中的重构保持一致。传递
undefined作为第 4 个参数(message)是合理的,因为此处正在创建新消息。packages/core/src/middlewares/chat/rollback_chat.ts (1)
104-114: LGTM!回滚功能中的消息转换器调用已正确更新此处对
messageTransformer.transform的调用与read_chat_message.ts中的更改保持一致,正确使用了选项对象模式。packages/core/src/llm-core/chain/plugin_chat_chain.ts (2)
29-29: LGTM!导入 getMessageContent 工具函数从
koishi-plugin-chatluna/utils/string导入getMessageContent是合理的,这个工具函数用于从消息内容中提取纯文本。
173-178: LGTM!请求对象增强符合 PR 目标将
prompt字段添加到variables并引入包含session的configurable对象,使得会话上下文可以贯穿整个 LLM 调用链。这与 PR 的目标一致:将 session 作为 configurable 参数传递,并自动提取提示内容作为链变量。packages/core/src/services/chat.ts (2)
196-196: LGTM!chat 方法默认参数已更新
chat方法的requestId参数默认值已正确更新为randomUUID(),与迁移到 Node.js crypto API 保持一致。
961-961: LGTM!内部方法已统一使用 randomUUID()
clearChatHistory、clearCache和delete方法中的 requestId 生成已统一更新为使用randomUUID()。Also applies to: 977-977, 1003-1003
packages/core/src/llm-core/chain/chat_chain.ts (2)
19-19: LGTM!导入 getMessageContent 工具函数与
plugin_chat_chain.ts一致,导入getMessageContent用于提取消息文本内容。
90-112: LGTM!call 方法增强与插件链保持一致
call方法的更新与plugin_chat_chain.ts中的实现保持一致:
- 接受
session参数- 将
prompt添加到variables- 引入包含
session的configurable对象这种一致性确保了不同链类型的行为统一。
packages/service-search/src/chain/browsing_chain.ts (2)
228-230: LGTM!浏览链变量增强与核心链实现保持一致,将
prompt添加到variables对象中。
277-279: LGTM!浏览链调用传递 session 配置向
callChatLunaChain传递configurable: { session }确保了会话上下文在搜索链中的传播,与其他链的实现保持一致。packages/core/src/services/message_transform.ts (3)
20-33: LGTM!重构为选项对象提升了扩展性将
transform方法的参数从布尔值重构为选项对象是一个很好的设计改进:
- 提高了可读性(命名参数)
- 便于未来添加新的配置选项
- 保持了向后兼容的默认值
46-65: LGTM!引用消息处理逻辑正确更新条件检查和递归调用已正确更新为使用
options.quote和options.includeQuoteReply。递归调用transform时正确传递了选项对象。
176-233: LGTM!_processElement 方法中的递归调用已全部更新所有递归调用
transform的地方都已正确更新为传递选项对象,确保了引用处理行为在整个元素树中的一致性。
…eval layers - Change retrievalLayer type from string to MemoryRetrievalLayerType enum - Use enum default value instead of 'Unknown' string - Remove debug logging statement for searched documents - Update .claude settings to include gh pr view command approval
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
♻️ Duplicate comments (1)
packages/extension-long-memory/src/plugins/prompt_varaiable.ts (1)
47-64: 记忆层级分组可能存在运行时类型不匹配虽然
memoriesByLayer的 Map 现在使用MemoryRetrievalLayerType作为键类型(第 47-48 行),但第 56-57 行直接使用memory.retrievalLayer而未进行类型转换或验证。这可能导致以下问题:
- 运行时类型不一致:如果记忆数据从数据库反序列化时,
retrievalLayer可能是普通字符串而非枚举值- 大小写敏感:如果存储的值是
"User"但枚举值是"user",Map 查找会失败- 空分组风险:类型不匹配会导致后续的
memoriesByLayer.get(key)返回undefined,sections 为空参考之前的评审意见,建议添加类型规范化逻辑:
validMemories.forEach((memory, index) => { - const layerName = - memory.retrievalLayer || MemoryRetrievalLayerType.USER + // 规范化层级类型:如果是字符串则转换为枚举值 + let layerName: MemoryRetrievalLayerType + if (typeof memory.retrievalLayer === 'string') { + const enumKey = memory.retrievalLayer.toUpperCase() as keyof typeof MemoryRetrievalLayerType + layerName = MemoryRetrievalLayerType[enumKey] ?? MemoryRetrievalLayerType.USER + } else { + layerName = memory.retrievalLayer ?? MemoryRetrievalLayerType.USER + } if (!memoriesByLayer.has(layerName)) { memoriesByLayer.set(layerName, []) }或者,如果确认
memory.retrievalLayer在运行时始终是正确的枚举值,请添加运行时断言或测试来验证这一点。
🧹 Nitpick comments (3)
packages/extension-long-memory/src/plugins/prompt_varaiable.ts (2)
104-112: 到期日期计算可以简化日期计算逻辑正确,但可以使用更简洁的表达式。
可以考虑重构为:
if (memory.expirationDate) { - const daysUntilExpiration = Math.ceil( - (memory.expirationDate.getTime() - Date.now()) / - (1000 * 60 * 60 * 24) - ) + const daysUntilExpiration = Math.ceil( + (memory.expirationDate.getTime() - Date.now()) / 86400000 + ) if (daysUntilExpiration > 0) {使用常量
86400000(毫秒/天) 代替1000 * 60 * 60 * 24可提高可读性。
131-141: 错误处理可以提供更多上下文信息虽然 try-catch 包装有效防止了异常传播,但返回的错误消息过于简单,不利于调试。
建议在错误消息中包含更多上下文:
ctx.chatluna.promptRenderer.registerFunctionProvider( 'long_memory', async (args, variables, configurable) => { try { return await handler(args, variables, configurable) } catch (error) { - logger.error(error) - return 'Error retrieving long-term memory' + logger.error('Failed to retrieve long-term memory:', error) + return `Error retrieving long-term memory: ${error instanceof Error ? error.message : 'Unknown error'}` } } )这样可以在提示中提供错误提示,帮助用户和开发者了解问题所在。
packages/extension-long-memory/src/types.ts (1)
27-27: 类型约束增强,提升类型安全
- 将
retrievalLayer从string改为MemoryRetrievalLayerType枚举,限定可用值范围、提升编译期检查。- 反序列化位置(如
packages/extension-long-memory/src/utils/chat-history.ts中YAML.load后)尚未对retrievalLayer做运行时合法性校验,建议在外部输入反序列化后增加枚举值验证,防止无效值进入系统。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.claude/settings.local.jsonis excluded by!**/*.json
📒 Files selected for processing (2)
packages/extension-long-memory/src/plugins/prompt_varaiable.ts(1 hunks)packages/extension-long-memory/src/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/extension-long-memory/src/plugins/prompt_varaiable.ts (3)
packages/extension-long-memory/src/index.ts (2)
Config(31-62)Config(64-175)packages/extension-long-memory/src/service/memory.ts (1)
Context(338-340)packages/shared-prompt-renderer/src/types.ts (1)
FunctionProvider(4-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (3)
packages/extension-long-memory/src/plugins/prompt_varaiable.ts (3)
4-4: 引入类型化的函数提供者接口导入
FunctionProvider类型为处理函数提供了明确的类型定义,这有助于确保参数签名的一致性,并能更好地与 prompt renderer 集成。
10-17: 层级类型转换逻辑正确通过
.toUpperCase()将字符串层级名称转换为枚举键名,并使用.filter()过滤掉无效值,这种处理方式既能容错又能保证类型安全。
68-79: 层级显示顺序已包含新的 Guild 层
layerOrder数组正确地包含了新添加的GUILD层级(第 73 行),并按照 Global → Guild → Preset → User 的优先级排序,符合记忆检索的语义层次结构。
- Update peerDependencies from ^1.3.0-alpha.61 to ^1.3.0-alpha.62 across all packages - Reformat package.json files with consistent 4-space indentation
This PR modernizes the codebase by migrating from external dependencies to Node.js built-in crypto APIs and enhances the prompt rendering system with session context support.
New Features
Bug Fixes
getMessageContentOther Changes
@types/uuid,@types/md5,@types/jsdom,@types/katex) across all packagesuuidpackage with Node.jscrypto.randomUUID()andmd5package withcrypto.createHash('md5')messageTransformer.transform()to use options object instead of boolean parameter for better API extensibility@koishijs/plugin-notifier)These changes reduce the dependency footprint, leverage Node.js built-in APIs, and enable more dynamic and context-aware prompt generation.