-
-
Notifications
You must be signed in to change notification settings - Fork 41
[Refactor] Remove unused ctx parameter from platform client registration #586
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
…ent function - Update CreateClientFunction type signature to remove Context parameter - Remove unused ctx parameter from all adapter registerClient calls - Update platform service to call createClientFunction without context - Simplify client registration across all adapters: - adapter-azure-openai - adapter-claude - adapter-deepseek - adapter-dify - adapter-doubao - adapter-gemini - adapter-hunyuan - adapter-ollama - adapter-openai-like - adapter-openai - adapter-qwen - adapter-rwkv - adapter-spark - adapter-wenxin - adapter-zhipu - service-embeddings (huggingface) The context is already available in the client constructors, making the parameter in registerClient redundant.
- Bump core package to v1.3.0-alpha.58 - Update all adapter packages to reference new core version: - adapter-azure-openai: 1.3.0-alpha.11 → 1.3.0-alpha.12 - adapter-claude: 1.3.0-alpha.11 → 1.3.0-alpha.12 - adapter-deepseek: 1.3.0-alpha.11 → 1.3.0-alpha.12 - adapter-dify: 1.3.0-alpha.9 → 1.3.0-alpha.10 - adapter-doubao: 1.3.0-alpha.11 → 1.3.0-alpha.12 - adapter-gemini: 1.3.0-alpha.16 → 1.3.0-alpha.17 - adapter-hunyuan: 1.3.0-alpha.9 → 1.3.0-alpha.10 - adapter-ollama: 1.3.0-alpha.9 → 1.3.0-alpha.10 - adapter-openai-like: 1.3.0-alpha.12 → 1.3.0-alpha.13 - adapter-openai: 1.3.0-alpha.11 → 1.3.0-alpha.12 - adapter-qwen: 1.3.0-alpha.12 → 1.3.0-alpha.13 - adapter-rwkv: 1.3.0-alpha.9 → 1.3.0-alpha.10 - adapter-spark: 1.3.0-alpha.9 → 1.3.0-alpha.10 - adapter-wenxin: 1.3.0-alpha.9 → 1.3.0-alpha.10 - adapter-zhipu: 1.3.0-alpha.11 → 1.3.0-alpha.12 - Update extension packages peer dependencies - Update service packages including embeddings (1.2.7 → 1.3.0-alpha.1) - Update shared-adapter peer dependency All packages now reference koishi-plugin-chatluna ^1.3.0-alpha.58
|
Caution Review failedThe pull request is closed. Walkthrough将 registerClient 的工厂函数从接收 Context 参数改为零参数闭包:核心类型与服务调用相应调整,所有适配器与 Huggingface 注册改为捕获外层 ctx;平台服务调用由 createClientFunction(ctx) 改为 createClientFunction()。未改动公开导出接口除类型签名外无变化。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Host as Host/App
participant Plugin as ChatLunaPlugin
participant Core as PlatformService
participant Factory as createClientFunction()
participant Client as PlatformClient
Host->>Plugin: apply(ctx, config)
Plugin->>Plugin: registerClient(() => new Client(ctx, config, plugin))
Note right of Plugin #DFF2E1: 工厂函数不再接收 ctx 参数\n改为闭包捕获外层 ctx
Host->>Core: 请求获取/创建客户端
Core->>Factory: 调用零参工厂:createClientFunction()
Factory-->>Core: 返回 Client 实例
Core-->>Host: 提供已创建/缓存的 Client
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (28)
📒 Files selected for processing (1)
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 implements a significant refactoring to simplify the platform client registration system by eliminating a redundant 'ctx' parameter. The change improves code clarity and reduces cognitive overhead for developers by ensuring that the context is accessed directly within client constructors where it's already available, rather than being passed explicitly through the factory function. This leads to a cleaner and more intuitive API for client registration across the system. 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 well-executed refactoring across multiple packages to simplify the platform client registration. By removing the now-redundant ctx parameter from the CreateClientFunction and all its implementations, the API is cleaner and more intuitive. The changes correctly leverage closures to capture the plugin-specific context, which is a good improvement. The modifications are consistent and correctly applied throughout the codebase, including all adapters and the core services. Overall, this is a solid improvement to the codebase's clarity and maintainability.
- Bump core package to v1.3.0-alpha.59 - Update all packages to reference new core version dependency - Fix config schema: improve blackList field definition with proper chaining - Fix locale files: correct quote usage in test result messages All packages now reference koishi-plugin-chatluna ^1.3.0-alpha.59
Add @koishijs/plugin-notifier ^1.2.1 as a dependency to fix missing package error. This dependency is required for the notification feature added in the previous release.
This PR refactors the platform client registration system by removing the unused
ctxparameter from theCreateClientFunctiontype and all adapter implementations.Summary of Changes
This PR simplifies the platform client registration API by removing redundant context parameter passing. The context is already accessible within client constructors, making the parameter in
registerClientunnecessary.Other Changes
Core refactoring:
CreateClientFunctiontype signature from(ctx: Context) => BasePlatformClientto() => BasePlatformClientPlatformServiceto callcreateClientFunction()without passing contextChatLunaPlugin.registerClient()method signatureAdapter updates: Removed unused
ctxparameter fromregisterClientcalls in all 15 adapters:Service updates: Updated embeddings service (huggingface) to use new signature
Version bumps: Bumped all package versions to align with core v1.3.0-alpha.58
Technical Details
The refactoring improves code clarity by:
All clients already receive the context through their constructor parameters, making the additional context parameter in the factory function redundant.