-
-
Notifications
You must be signed in to change notification settings - Fork 41
[Feature] Add notification support for model configuration guidance #585
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
- Add notification system to warn users when no models are configured - Display success notification showing total loaded models count - Include helpful link to model configuration documentation - Fix watcher cleanup in ChatInterface to properly dispose watchers on effect cleanup - Enable notification feature in default factory configuration The notification updates dynamically as models are added or removed, providing real-time feedback to users about their model configuration status.
Remove required() constraint from API key schema definitions across all adapters, allowing users to install and configure adapters without immediate API key setup. This change enables: - Better onboarding experience with notification guidance - Deferred API key configuration until users are ready - Compatibility with the new notification system for model configuration status Affected adapters: - azure-openai, claude, deepseek, doubao, gemini - hunyuan, openai, openai-like, qwen, wenxin, zhipu
|
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 Walkthrough放宽多适配器配置中 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as 调用者
participant Chat as ChatApp(ctx)
participant LLM as llm
participant EMB as embeddings
participant CH as chain
UI->>Chat: init()
Note over Chat: 在 ctx.effect 内注册两个独立 watcher(llm, embeddings)
par Watch llm
LLM-->>Chat: change(newValue)
alt newValue == null
Chat->>CH: set undefined
else
Chat->>Chat: createChain()
Chat->>CH: replace with new chain
end
and Watch embeddings
EMB-->>Chat: change(newEmb)
Chat->>Chat: _embeddings = newEmb
Chat->>Chat: recreate chain
end
UI-->>Chat: dispose()
Chat->>Chat: 调用各 watcher 的 cleanup(停止监听)
Chat->>CH: 释放链路
sequenceDiagram
autonumber
participant Ctx as Context
participant MS as modelSchema(ctx, createNotification)
participant Noti as notifier
participant Sch as schema(model)
Ctx->>MS: call with createNotification = true
opt createNotification
MS->>Noti: create warning "未配置模型"
end
loop watch modelNames (immediate)
MS->>Sch: 更新选项基于 modelNames
alt count > 1 and notification exists
MS->>Noti: update 成功信息(模型数量)
else
opt notification exists
MS->>Noti: update 警告,提示配置模型
end
end
end
Ctx-->>MS: dispose
MS->>Noti: 清理通知(如存在)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
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 enhances the user onboarding and system stability by introducing a dynamic notification system for model configuration, making API key setup optional across numerous adapter packages, and resolving a critical memory leak in the 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
|
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 introduces a valuable notification system for model configuration, makes API keys optional across various adapters to improve user onboarding, and fixes a memory leak by correctly managing watcher disposal. The changes are well-structured and align with the stated goals. I've identified a couple of issues in the new notification logic within packages/core/src/utils/schema.ts where the notification link is broken and the model count is incorrect. I've provided a single suggestion to address both. Overall, this is a solid improvement to the project.
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: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
packages/adapter-azure-openai/src/index.ts(1 hunks)packages/adapter-claude/src/index.ts(1 hunks)packages/adapter-deepseek/src/index.ts(1 hunks)packages/adapter-doubao/src/index.ts(1 hunks)packages/adapter-gemini/src/index.ts(1 hunks)packages/adapter-hunyuan/src/index.ts(1 hunks)packages/adapter-openai-like/src/index.ts(1 hunks)packages/adapter-openai/src/index.ts(1 hunks)packages/adapter-qwen/src/index.ts(1 hunks)packages/adapter-wenxin/src/index.ts(1 hunks)packages/adapter-zhipu/src/index.ts(1 hunks)packages/core/src/llm-core/chat/app.ts(1 hunks)packages/core/src/llm-core/chat/default.ts(1 hunks)packages/core/src/utils/schema.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-17T00:25:27.195Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#548
File: packages/core/src/llm-core/chat/app.ts:0-0
Timestamp: 2025-09-17T00:25:27.195Z
Learning: 在 ChatInterface 类中,响应式 watch 调用已经通过 ctx.effect() 包装来自动处理清理工作,避免内存泄漏。字段 _chain 和 _embeddings 的类型已更新为可空类型。
Applied to files:
packages/core/src/llm-core/chat/app.ts
📚 Learning: 2025-09-17T00:25:27.195Z
Learnt from: dingyi222666
PR: ChatLunaLab/chatluna#548
File: packages/core/src/llm-core/chat/app.ts:0-0
Timestamp: 2025-09-17T00:25:27.195Z
Learning: 在 ChatInterface 类中,响应式 watch 调用通过 ctx.effect() 包装来自动处理清理工作,避免内存泄漏。字段 _chain 和 _embeddings 的类型已更新为可空类型 (| undefined),并添加 ctx.on('dispose') 处理器提供额外的清理保障。这种使用 Koishi effect 系统的方式比手动管理 stop 句柄更优雅。
Applied to files:
packages/core/src/llm-core/chat/app.ts
🧬 Code graph analysis (1)
packages/core/src/llm-core/chat/default.ts (1)
packages/core/src/utils/schema.ts (1)
modelSchema(15-53)
⏰ 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 (5)
packages/adapter-hunyuan/src/index.ts (1)
55-55: API 密钥可选化符合 PR 目标。在 11 个适配器包中统一将
apiKeys元组的第一个元素从必需改为可选,允许用户在配置凭据之前安装适配器。运行时过滤逻辑(第 16-17 行)仍然确保只有非空密钥被使用。packages/core/src/utils/schema.ts (3)
1-1: LGTM!添加
h导入以支持通知内容的 HTML 解析。
7-7: LGTM!添加
@koishijs/plugin-notifier的类型导入以支持通知系统。
27-48: 确认通知对象在上下文销毁时被正确清理
请验证 @koishijs/plugin-notifier 是否在 ctx.stop 时自动释放通知实例;如无,请在 stop 回调中添加notification?.dispose()以避免内存泄漏。packages/adapter-qwen/src/index.ts (1)
52-52: 所有适配器均已移除 .required(),变更符合预期
已验证包括 qwen 在内的 11 个适配器中的 API 密钥模式已统一更新,安全且兼容。
…ification URL
- Add .default('') to API key schema definitions across all adapters to prevent
undefined values when fields are left empty
- Fix malformed URL in notification message (remove stray "> characters)
- Ensure consistent schema behavior across all adapter packages
Affected adapters:
- azure-openai, claude, deepseek, dify, doubao, gemini, hunyuan
- openai, openai-like, qwen, rwkv, wenxin, zhipu
This complements the previous commits by providing proper default values for
optional API keys, improving the user experience when configuring adapters.
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: 4
♻️ Duplicate comments (1)
packages/adapter-openai-like/src/index.ts (1)
102-102: 此问题在先前评审中已标记先前的评审已经指出:移除
.required()后,元组首项可能为undefined,但parseConfig(第 31 行)的过滤逻辑仍直接访问apiKey.length。当默认值是[[]]或用户新增行但尚未填写时,这里会得到undefined,在ready钩子里立即抛出TypeError: Cannot read properties of undefined (reading 'length'),插件无法启动。请在过滤时为解构项提供默认值或显式判断类型,避免访问空值。建议的修复方案:
- return config.apiKeys - .filter(([apiKey, _, enabled]) => { - return apiKey.length > 0 && enabled - }) - .map(([apiKey, apiEndpoint]) => { + return config.apiKeys + .filter(([apiKey = '', , enabled = true]) => { + return apiKey.length > 0 && enabled + }) + .map(([apiKey = '', apiEndpoint]) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
packages/adapter-azure-openai/src/index.ts(1 hunks)packages/adapter-claude/src/index.ts(1 hunks)packages/adapter-deepseek/src/index.ts(1 hunks)packages/adapter-dify/src/index.ts(1 hunks)packages/adapter-doubao/src/index.ts(1 hunks)packages/adapter-gemini/src/index.ts(1 hunks)packages/adapter-hunyuan/src/index.ts(1 hunks)packages/adapter-openai-like/src/index.ts(1 hunks)packages/adapter-openai/src/index.ts(1 hunks)packages/adapter-qwen/src/index.ts(1 hunks)packages/adapter-rwkv/src/index.ts(1 hunks)packages/adapter-wenxin/src/index.ts(1 hunks)packages/adapter-zhipu/src/index.ts(1 hunks)packages/core/src/utils/schema.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/adapter-hunyuan/src/index.ts
- packages/adapter-claude/src/index.ts
- packages/adapter-azure-openai/src/index.ts
- packages/adapter-wenxin/src/index.ts
- packages/adapter-openai/src/index.ts
- packages/adapter-zhipu/src/index.ts
- packages/adapter-deepseek/src/index.ts
- packages/core/src/utils/schema.ts
⏰ 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 (1)
packages/adapter-dify/src/index.ts (1)
59-59: 变更安全为
additionalModels中的apiKey添加默认值是安全的。此文件的parseConfig逻辑不涉及元组解构和长度访问,不存在运行时错误风险。
Update core package version and synchronize peer dependency versions across all adapter, extension, and service packages to 1.3.0-alpha.57. Updated packages: - Core: 1.3.0-alpha.56 → 1.3.0-alpha.57 - Adapters: azure-openai, claude, deepseek, dify, doubao, gemini, hunyuan, ollama, openai, openai-like, qwen, rwkv, spark, wenxin, zhipu (peer deps) - Extensions: long-memory, mcp, tools, variable (peer deps) - Services: embeddings, image, search, vector-store (peer deps) - Renderers: image (peer deps) - Shared: adapter (peer deps) This release includes the notification system for model configuration guidance.
This PR adds a notification system to improve the user onboarding experience by providing real-time feedback about model configuration status.
New Features
Bug fixes
Other Changes