reface the interface of online models#1074
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a centralized parameter resolution utility, resolve_online_params, to standardize the handling of model, source, and URL aliases across various online modules, including Chat, Embedding, and MultiModal. It also renames base_url to url for consistency and improves error reporting during type inference. The review feedback identifies a logic error in the parameter swapping mechanism within resolve_online_params that could interfere with dynamic routing. Additionally, it suggests including missing model aliases in the Chat and MultiModal modules to ensure consistent behavior across the library.
| if source is not None and _in(source): | ||
| raise ValueError(f'`{model!r}` and `{source!r}` are both recognised source names; ' | ||
| f'provide exactly one via `source=`.') | ||
| source, model = model, source |
There was a problem hiding this comment.
The logic for swapping model and source is incorrect when source is already provided but not present in the source_registry (e.g., when using source='dynamic'). Currently, if model is a recognized source name and source is set to something else (like 'dynamic'), the function will swap them because _in(source) will be false. This breaks dynamic routing by setting source to the model name and model to 'dynamic'. The swap should only occur if source is None. If both are provided and model is a recognized source, it should raise a ValueError to avoid ambiguity, consistent with the check added in automodel.py.
| if source is not None and _in(source): | |
| raise ValueError(f'`{model!r}` and `{source!r}` are both recognised source names; ' | |
| f'provide exactly one via `source=`.') | |
| source, model = model, source | |
| if source is not None: | |
| raise ValueError(f'{model!r} is a recognised source name; pass it as source= and ' | |
| f'do not also set source={source!r}.') | |
| source, model = model, None |
| model, source, url, kwargs = resolve_online_params( | ||
| model, source, url, kwargs, url_aliases='base_url', source_registry=lazyllm.online.chat) |
There was a problem hiding this comment.
For consistency with AutoModel and other online modules, OnlineChatModule should also resolve model_name and base_model aliases into the model parameter. Currently, these remain in kwargs, which might lead to unexpected behavior or duplicate parameters being passed to the underlying supplier.
model, source, url, kwargs = resolve_online_params(
model, source, url, kwargs, model_aliases=('model_name', 'base_model'),
url_aliases='base_url', source_registry=lazyllm.online.chat)| model, source, url, kwargs = resolve_online_params( | ||
| model, source, url, kwargs, url_aliases='base_url') |
There was a problem hiding this comment.
Similar to the OnlineChatModule, OnlineMultiModalModule is missing common model aliases like model_name and base_model. Additionally, there is a redundant call to resolve_online_params inside _validate_parameters (line 60). While the second call is needed to check the source_registry (which depends on the resolved type), the first call in new could be optimized or combined to avoid multiple passes over kwargs.
| model, source, url, kwargs = resolve_online_params( | |
| model, source, url, kwargs, url_aliases='base_url') | |
| model, source, url, kwargs = resolve_online_params( | |
| model, source, url, kwargs, model_aliases=('model_name', 'base_model'), url_aliases='base_url') |
📌 PR 内容 / PR Description
统一并规范
OnlineChatModule、OnlineEmbeddingModule、OnlineMultiModalModule、OnlineModule、AutoModel五个在线模型入口的参数接口,抽象公共参数解析逻辑,消除历史命名不一致问题,并增强冲突检测与错误提示。Unifies the parameter interface across all five online model entry points (
OnlineChatModule,OnlineEmbeddingModule,OnlineMultiModalModule,OnlineModule,AutoModel), extracts a shared parameter resolver, eliminates historical naming inconsistencies, and strengthens conflict detection with clear error messages.✅ 变更类型 / Type of Change
🔍 核心变更 / Key Changes
1. 新增公共参数解析函数
resolve_online_params(base/utils.py)抽象出统一的参数解析器,供所有模块的
__new__、__init__、forward共用:embed_model_name、model_name、embed_name、base_url、embed_url)映射到规范名(model、url)ValueError'openai'),自动与source互换;若source也是合法 source 名则报错;若source是模型名则直接交换model_aliases/url_aliases支持传str(单别名)或tuple(多别名)2. 统一构造函数参数名
OnlineEmbeddingModulesource,embed_model_name,embed_urlmodel,url(旧名保持 kw 兼容)OnlineChatModulebase_urlurl(base_url保持 kw 兼容)OnlineMultiModalModulebase_urlurl(base_url保持 kw 兼容)3. 统一
forward参数名所有 forward 方法统一支持
url、model作为显式参数,历史字段(base_url、embed_url、model_name、embed_model_name、embed_name)通过resolve_online_params自动兼容,同时检测冲突。4. 修复错误类型与校验方式
OnlineChatModule/OnlineEmbeddingModule:skip_auth无 url 时由KeyError改为ValueErrorOnlineMultiModalModule._resolve_type_name/_validate_parameters:assert改为raise ValueError,错误信息更清晰OnlineMultiModalBase.forward:去掉无意义的try/except Exception包装5. 其他修复
dynamic_router._merge_dynamic_forward_kwargs:补充对embed_url、embed_name等历史别名的感知,避免重复注入AutoModel:补充model_name别名支持,并增加 model-as-source 冲突检测resolve_online_params中使用hasattr(__contains__)而非callable()判断 source_registry 类型,修复LazyDict既是容器又是 Callable 导致被误当函数调用的 bug⚡ 更新后的用法示例 / Usage After Update
🔄 重构前 / 重构后对比 / Before & After
重构前 / Before:
重构后 / After:
base_url、embed_url、embed_model_name、model_name、embed_name等旧字段在构造和 forward 中均可继续使用OnlineEmbeddingModule第一个位置参数语义变化:旧版是source,新版是model;由于 source-as-model 自动交换机制,旧式OnlineEmbeddingModule('openai')行为不变