-
Notifications
You must be signed in to change notification settings - Fork 0
Add dedicated VLM model configuration and wire it through multi-tenant VLM creation #10
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
BukeLy
left a 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.
按照Comment的来改进
src/config.py
Outdated
| api_key: str = Field(..., description="LLM API Key") | ||
| base_url: str = Field(..., description="LLM API Base URL") | ||
| model: str = Field(default="seed-1-6-250615", description="LLM Model Name") | ||
| vlm_model: str = Field(default="seed-1-6-250615", description="VLM Model Name") |
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.
这个还是默认回退到seed了,user不设置不一定用这个.直接去掉默认行为,不设置就报错
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.
已改为必填且仅使用 VLM_MODEL 配置,不再回退默认值(e5548d2)。
src/config.py
Outdated
| def _load_vlm_model(cls, v): | ||
| """ | ||
| 优先读取独立的 VLM_MODEL(保持与 LLM 文本模型解耦) | ||
| 回退顺序: | ||
| 1) 显式传入值 | ||
| 2) 环境变量 VLM_MODEL(独立配置) | ||
| 3) 环境变量 LLM_VLM_MODEL(前缀形式) | ||
| 4) 默认值 | ||
| """ | ||
| return v or os.getenv("VLM_MODEL") or os.getenv("LLM_VLM_MODEL") or "seed-1-6-250615" |
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.
为什么要多级回退?我们只兼容一个配置,不引入额外的复杂性
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.
已去掉多级回退,仅接受独立的 VLM_MODEL(e5548d2)。
src/multi_tenant.py
Outdated
|
|
||
| # 从配置中提取参数(支持租户覆盖) | ||
| model = llm_config.get("model", self.ark_model) | ||
| model = llm_config.get("vlm_model", self.ark_vlm_model) |
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.
ark_model是什么?你还在用火山的命名方式吗?
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.
已移除 ark 命名,统一使用 llm/vlm 前缀并保持含义清晰(e5548d2)。
| LLM_BASE_URL="https://ark.ap-southeast.bytepluses.com/api/v3" | ||
| # 使用的模型名称 | ||
| LLM_MODEL=seed-1-6-250615 | ||
| # 用于多模态图片理解的 VLM 模型(与 LLM_MODEL 解耦) | ||
| VLM_MODEL=seed-1-6-250615 | ||
| # LLM 供应商标识(ark/openai/claude) | ||
| LLM_PROVIDER=ark | ||
| # VLM 图片理解 API 超时时间(秒,默认 120 秒) |
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.
LLM和VLM难道不可以换供应商吗?难道一定要用LLM的Base URL吗?
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.
已支持 VLM 独立的 API Key/Base URL(env 示例已添加 VLM_API_KEY/VLM_BASE_URL),不再强制复用 LLM Base URL(e5548d2)。
|
@copilot 修改Review出来的问题 #10 (review) |
Co-authored-by: BukeLy <19304666+BukeLy@users.noreply.github.com>
Co-authored-by: BukeLy <19304666+BukeLy@users.noreply.github.com>
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.
Pull request overview
This PR decouples VLM (Vision Language Model) configuration from text LLM configuration, enabling independent configuration of vision models through environment variables. Previously, VLM model selection was hardcoded and tied to LLM settings, preventing users from configuring different models or API endpoints for vision tasks.
Key changes:
- Introduced dedicated VLM configuration fields (
vlm_model,vlm_api_key,vlm_base_url) with fallback to LLM credentials when VLM-specific values are not provided - Updated multi-tenant manager to use VLM-specific configuration for vision model creation, replacing hardcoded model references
- Enhanced environment template with VLM configuration options and documentation
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/config.py | Added required vlm_model field and optional vlm_api_key/vlm_base_url fields to LLMConfig; removed unused os import |
| src/multi_tenant.py | Renamed ark_* variables to llm_* for clarity; added VLM-specific variables with fallback logic; updated _create_vision_model_func to use dedicated VLM configuration |
| src/tenant_config.py | Extended _merge_llm_config to include VLM fields in tenant configuration merge, enabling per-tenant VLM overrides |
| env.example | Added VLM_MODEL as required configuration; documented optional VLM_API_KEY and VLM_BASE_URL with fallback behavior explanation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| vlm_model: str = Field(..., description="VLM Model Name", alias="VLM_MODEL") | ||
| vlm_api_key: Optional[str] = Field(default=None, description="VLM API Key", alias="VLM_API_KEY") | ||
| vlm_base_url: Optional[str] = Field(default=None, description="VLM API Base URL", alias="VLM_BASE_URL") |
Copilot
AI
Dec 15, 2025
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.
The VLM fields use explicit aliases (VLM_MODEL, VLM_API_KEY, VLM_BASE_URL) that don't follow the LLM_ prefix defined in the Config class. For these aliases to work correctly, the LLMConfig.Config class needs populate_by_name = True. Without this setting, pydantic-settings will not be able to load these environment variables. Other config classes in this file that use aliases (like DeepSeekOCRConfig at line 158) already include this setting.
| # 用于多模态图片理解的 VLM 模型(必填,独立于 LLM_MODEL) | ||
| VLM_MODEL=seed-1-6-250615 |
Copilot
AI
Dec 15, 2025
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.
The documentation says VLM_MODEL is required (必填), but in the code at src/config.py:24, vlm_model uses Field(...) which makes it required at the pydantic validation level. However, the comment also says it should be "independent from LLM_MODEL" (独立于 LLM_MODEL), but the default value for LLM_MODEL and the example value for VLM_MODEL are both the same: seed-1-6-250615. This creates confusion about whether they should actually be different models. Consider clarifying whether VLM_MODEL can use the same model as LLM_MODEL (for models that support both text and vision), or if they must be different models.
| # 用于多模态图片理解的 VLM 模型(必填,独立于 LLM_MODEL) | |
| VLM_MODEL=seed-1-6-250615 | |
| # 用于多模态图片理解的 VLM 模型(必填,独立于 LLM_MODEL,需支持图片输入) | |
| VLM_MODEL=seed-vlm-1-6-250615 # 示例:与 LLM_MODEL 不同,需为支持视觉的模型 |
VLM model choice was hardcoded and tied to the text LLM model, preventing .env configuration and clear LLM/VLM separation.
vlm_modeltoLLMConfigviaVLM_MODEL(no fallback defaults).VLM_API_KEYandVLM_BASE_URLfor vendor-independent VLM access; falls back to LLM credentials only when VLM-specific values are absent.vlm_timeout.VLM_MODELas mandatory and documented optionalVLM_API_KEY/VLM_BASE_URLinenv.exampleto expose VLM selection and vendor choice.Example:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.