feat(core): refine quote and forward reply config schema#780
feat(core): refine quote and forward reply config schema#780PinkElysiaDev wants to merge 2 commits into
Conversation
Walkthrough在聊天链中新增并传播可选的 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatChain
participant Middleware
participant Sender
participant Downstream
Client->>ChatChain: receiveMessage/receiveCommand(payload)
ChatChain->>Middleware: initialize ChainMiddlewareContext (includes requestStartedAt)
Middleware->>ChatChain: invoke send handlers with context
ChatChain->>Sender: send(session, messages, context)
Sender->>Sender: buildMessageFragment(messages, context)
Sender->>Downstream: forward/send message (includes context)
Downstream-->>Sender: ack
Sender-->>ChatChain: resolved
ChatChain-->>Client: response delivered
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
📝 Coding Plan
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 Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
Summary of ChangesHello, 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 refines the message handling configuration by introducing more dynamic control over how the bot replies and forwards messages. It moves beyond simple boolean toggles to a more sophisticated system where quoting and forwarding can be conditionally applied based on factors like response time or message length. This enhancement provides greater flexibility and precision in managing bot interactions, ensuring more contextually appropriate responses. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/chains/chain.ts (1)
356-371:⚠️ Potential issue | 🟡 Minor错误/中止消息把
context丢掉了，引用阈值在失败路径上不会生效。Line 356 和 Line 371 这里显式传
undefined，所以一旦replyQuoteThreshold > 0，错误或中止回复就永远命不中新增的耗时引用逻辑。这个 PR 已经把成功路径和 stop 路径都接上了context，失败路径也应该保持一致。🔧 建议修改
- await this._handleMiddlewareError( - session, - result.middlewareName!, - result.error! - ) + await this._handleMiddlewareError( + session, + context, + result.middlewareName!, + result.error! + ) return false } @@ private async _handleMiddlewareError( session: Session, + context: ChainMiddlewareContext, middlewareName: string, error: Error ) { if (error instanceof ChatLunaError) { const message = error.errorCode === ChatLunaErrorCode.ABORTED ? session.text('chatluna.aborted') : error.message - await this.sendMessage(session, message, undefined) + await this.sendMessage(session, message, context) return } @@ await this.sendMessage( session, session.text('chatluna.middleware_error', [ middlewareName, error.message ]), - undefined + context ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/chains/chain.ts` around lines 356 - 371, The error/abort paths call this.sendMessage(session, ..., undefined) and thus drop the context so replyQuoteThreshold logic never triggers; update these calls to forward the existing context (i.e., pass context instead of undefined) where sendMessage is invoked in the failure/abort branches so the reply quoting logic (replyQuoteThreshold) and downstream middleware (middlewareName and the 'chatluna.middleware_error' response) receive the same context as the success path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/config.ts`:
- Around line 82-97: The schema branch for isForwardMsg===false currently uses
an empty Schema.object({}) which allows forwardMsgMinLength to be omitted,
mismatching the exported Config interface (which requires forwardMsgMinLength).
Update the false-branch object in the Schema.union to include
forwardMsgMinLength with the same numeric constraints but marked hidden (e.g.,
forwardMsgMinLength:
Schema.number().min(0).max(400).step(1).default(0).hidden()) so the field always
exists at runtime while remaining hidden from public validation; keep the
isForwardMsg field handling as-is and ensure the Config type
(forwardMsgMinLength, isForwardMsg) stays consistent with this schema.
---
Outside diff comments:
In `@packages/core/src/chains/chain.ts`:
- Around line 356-371: The error/abort paths call this.sendMessage(session, ...,
undefined) and thus drop the context so replyQuoteThreshold logic never
triggers; update these calls to forward the existing context (i.e., pass context
instead of undefined) where sendMessage is invoked in the failure/abort branches
so the reply quoting logic (replyQuoteThreshold) and downstream middleware
(middlewareName and the 'chatluna.middleware_error' response) receive the same
context as the success path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 237af1b2-18b2-4aed-a32f-f0e1945c1f6b
⛔ Files ignored due to path filters (2)
packages/core/src/locales/en-US.schema.ymlis excluded by!**/*.ymlpackages/core/src/locales/zh-CN.schema.ymlis excluded by!**/*.yml
📒 Files selected for processing (2)
packages/core/src/chains/chain.tspackages/core/src/config.ts
| Schema.intersect([ | ||
| Schema.object({ | ||
| isForwardMsg: Schema.boolean().default(false) | ||
| }), | ||
| Schema.union([ | ||
| Schema.object({ | ||
| isForwardMsg: Schema.const(true).required(), | ||
| forwardMsgMinLength: Schema.number() | ||
| .min(0) | ||
| .max(400) | ||
| .step(1) | ||
| .default(0) | ||
| }), | ||
| Schema.object({}) | ||
| ]) | ||
| ]), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/core && cat -n src/config.ts | head -120Repository: ChatLunaLab/chatluna
Length of output: 4493
🏁 Script executed:
cd packages/core && grep -n "forwardMsgMinLength" src/chains/chain.ts | head -20Repository: ChatLunaLab/chatluna
Length of output: 115
🏁 Script executed:
cd packages/core && sed -n '740,755n' src/chains/chain.tsRepository: ChatLunaLab/chatluna
Length of output: 46
🏁 Script executed:
cd packages/core && sed -n '740,755p' src/chains/chain.tsRepository: ChatLunaLab/chatluna
Length of output: 453
🏁 Script executed:
cd packages/core && rg -n "forwardMsgMinLength" --type tsRepository: ChatLunaLab/chatluna
Length of output: 253
🏁 Script executed:
cd packages/core && rg -B5 -A5 "config.forwardMsgMinLength" --type tsRepository: ChatLunaLab/chatluna
Length of output: 544
forwardMsgMinLength 的 schema 与导出的 Config 类型不一致
Config 接口（第 9 行）声明 forwardMsgMinLength: number 为必填字段，但第 95 行的空对象分支允许在 isForwardMsg === false 时该字段完全缺失，导致运行时可能返回 undefined。虽然目前在 chains/chain.ts 第 743 行的访问被 isForwardMsg 守卫所保护，但这个类型/运行时契约的不匹配会增加未来维护风险。
建议在 schema 的 false 分支中保留该字段并标记为 .hidden()，保持公共类型签名的一致性：
修复方案
Schema.union([
Schema.object({
isForwardMsg: Schema.const(true).required(),
forwardMsgMinLength: Schema.number()
.min(0)
.max(400)
.step(1)
.default(0)
}),
- Schema.object({})
+ Schema.object({
+ isForwardMsg: Schema.const(false).required(),
+ forwardMsgMinLength: Schema.number()
+ .min(0)
+ .max(400)
+ .step(1)
+ .default(0)
+ .hidden()
+ })
])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Schema.intersect([ | |
| Schema.object({ | |
| isForwardMsg: Schema.boolean().default(false) | |
| }), | |
| Schema.union([ | |
| Schema.object({ | |
| isForwardMsg: Schema.const(true).required(), | |
| forwardMsgMinLength: Schema.number() | |
| .min(0) | |
| .max(400) | |
| .step(1) | |
| .default(0) | |
| }), | |
| Schema.object({}) | |
| ]) | |
| ]), | |
| Schema.intersect([ | |
| Schema.object({ | |
| isForwardMsg: Schema.boolean().default(false) | |
| }), | |
| Schema.union([ | |
| Schema.object({ | |
| isForwardMsg: Schema.const(true).required(), | |
| forwardMsgMinLength: Schema.number() | |
| .min(0) | |
| .max(400) | |
| .step(1) | |
| .default(0) | |
| }), | |
| Schema.object({ | |
| isForwardMsg: Schema.const(false).required(), | |
| forwardMsgMinLength: Schema.number() | |
| .min(0) | |
| .max(400) | |
| .step(1) | |
| .default(0) | |
| .hidden() | |
| }) | |
| ]) | |
| ]), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/config.ts` around lines 82 - 97, The schema branch for
isForwardMsg===false currently uses an empty Schema.object({}) which allows
forwardMsgMinLength to be omitted, mismatching the exported Config interface
(which requires forwardMsgMinLength). Update the false-branch object in the
Schema.union to include forwardMsgMinLength with the same numeric constraints
but marked hidden (e.g., forwardMsgMinLength:
Schema.number().min(0).max(400).step(1).default(0).hidden()) so the field always
exists at runtime while remaining hidden from public validation; keep the
isForwardMsg field handling as-is and ensure the Config type
(forwardMsgMinLength, isForwardMsg) stays consistent with this schema.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/service-multimodal/src/plugin.ts (1)
7-22:⚠️ Potential issue | 🟡 Minor将导入别名
read_files改为 camelCase第 7 行和第 22 行使用的导入别名
read_files违反 TypeScript 命名规范。变量和函数别名应采用 camelCase，改为readFiles。修改建议
-import { apply as read_files } from './plugins/read_files' // import end +import { apply as readFiles } from './plugins/read_files' // import end @@ - [audio, image, read_files] // middleware end + [audio, image, readFiles] // middleware end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/service-multimodal/src/plugin.ts` around lines 7 - 22, The import alias read_files violates camelCase; update the import in plugin.ts to use readFiles and rename all usages accordingly (the import statement and the middleware array entry inside the plugins function), ensuring the symbol referenced by the import (from './plugins/read_files') and the middleware array const middlewares = [audio, image, readFiles] use the new camelCase name so TypeScript naming conventions are followed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/service-multimodal/src/plugin.ts`:
- Around line 7-22: The import alias read_files violates camelCase; update the
import in plugin.ts to use readFiles and rename all usages accordingly (the
import statement and the middleware array entry inside the plugins function),
ensuring the symbol referenced by the import (from './plugins/read_files') and
the middleware array const middlewares = [audio, image, readFiles] use the new
camelCase name so TypeScript naming conventions are followed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 29e7e16e-ae05-4a62-9704-a2fc496d150e
📒 Files selected for processing (2)
packages/service-multimodal/src/plugin.tspackages/shared-adapter/src/utils.ts
✅ Files skipped from review due to trivial changes (1)
- packages/shared-adapter/src/utils.ts
1 participant
isReplyWithAt从单纯布尔开关改为“开关 + 阈值分支”配置replyQuoteThreshold，用于按响应耗时决定是否引用原消息isForwardMsg同样改为“开关 + 阈值分支”配置forwardMsgMinLength仅在启用合并消息时显示includeQuoteReply与replyQuoteThreshold的中文描述，使其语义更准确、表达更统一