Skip to content

fix(openai-embedding): temporarily fix invalid paramater for SiliconFlow provider's non-Qwen embedding models#8508

Open
Allenyou1126 wants to merge 2 commits into
AstrBotDevs:masterfrom
Allenyou1126:master
Open

fix(openai-embedding): temporarily fix invalid paramater for SiliconFlow provider's non-Qwen embedding models#8508
Allenyou1126 wants to merge 2 commits into
AstrBotDevs:masterfrom
Allenyou1126:master

Conversation

@Allenyou1126
Copy link
Copy Markdown

@Allenyou1126 Allenyou1126 commented Jun 2, 2026

Fix #8506.

This is only a temporary workaround! / 这是一个临时的修复!

由于 SiliconFlow / 硅基流动的 Embedding 模型 API 调用中,只有 Qwen/Qwen3 系列模型支持 dimensions 参数,其他模型传入该参数会导致 400 错误,因此加入一个特判 Case 作为 Workaround。

后续更实用的修复可以考虑在前端添加一个设置项,选择在调用 Embedding 模型时,是否需要传递该参数。

Modifications / 改动点

修改了 astrbot/core/provider/sources/openai_embedding_source.py,向其中用于构建 API 请求可选参数的 OpenAIEmbeddingProvider._embedding_kwargs() 函数加入了特判,根据 API Base 和模型名称判断,符合以下要求时:

  • 调用的是硅基流动提供的 API:API Base 以 https://api.siliconflow.cn 起始。
  • 调用的模型不是 Qwen 系列:模型名称不以 Qwen 起始。

将从返回的 kwargs 字典中删除 dimensions 参数。

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Verification Steps / 验证步骤

在模型提供商中添加 OpenAI Embedding Provider,API Base 使用 https://api.siliconflow.cn/v1, 模型使用非 Qwen 系列的任意 Embedding Model,保存后点击测试。

Screenshots or Test Results / 运行截图或测试结果

image image

Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

Summary by Sourcery

Bug Fixes:

  • Avoid sending the unsupported dimensions parameter when calling SiliconFlow embedding models that are not part of the Qwen series.

@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. area:provider The bug / feature is about AI Provider, Models, LLM Agent, LLM Agent Runner. labels Jun 2, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The SiliconFlow-specific workaround logic is hardcoded via startswith("https://api.siliconflow.cn") and self.model.startswith("Qwen"); consider centralizing provider/model capability flags (e.g., in provider config or a capabilities map) so that future provider-specific quirks don’t accumulate as scattered string checks.
  • To make the model check more robust, consider normalizing the model name (e.g., self.model.lower().startswith("qwen")) or using a more explicit capability flag instead of relying on a case-sensitive prefix string.
  • If SiliconFlow ever changes domains or paths (e.g., adds more subdomains or versions), the startswith("https://api.siliconflow.cn") check may become brittle; you might want to match on host only or make this configurable in the provider settings.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The SiliconFlow-specific workaround logic is hardcoded via `startswith("https://api.siliconflow.cn")` and `self.model.startswith("Qwen")`; consider centralizing provider/model capability flags (e.g., in provider config or a capabilities map) so that future provider-specific quirks don’t accumulate as scattered string checks.
- To make the model check more robust, consider normalizing the model name (e.g., `self.model.lower().startswith("qwen")`) or using a more explicit capability flag instead of relying on a case-sensitive prefix string.
- If SiliconFlow ever changes domains or paths (e.g., adds more subdomains or versions), the `startswith("https://api.siliconflow.cn")` check may become brittle; you might want to match on host only or make this configurable in the provider settings.

## Individual Comments

### Comment 1
<location path="astrbot/core/provider/sources/openai_embedding_source.py" line_range="83-84" />
<code_context>
+            and provider_api_base.startswith("https://api.siliconflow.cn")
+            and not self.model.startswith("Qwen")
+        ):
+            # For SiliconFlow and Non-Qwen models, dimensions parameter is not supported. so remove it.
+            kwargs.pop("dimensions", None)
         return kwargs

</code_context>
<issue_to_address>
**suggestion:** Consider emitting a log when stripping `dimensions` to aid debugging and configuration transparency.

This silent removal could confuse callers who expect a specific embedding size or configuration to be applied. Please add a debug/info log when `dimensions` is popped (including model and provider) so this behavior is visible in production without changing semantics.

Suggested implementation:

```python
        # Fix: SiliconFlow provider does not support dimensions parameter, except for Qwen models.
        provider_api_base = self.provider_config.get("embedding_api_base")
        if (
            provider_api_base
            and provider_api_base.startswith("https://api.siliconflow.cn")
            and not self.model.startswith("Qwen")
        ):
            # For SiliconFlow and Non-Qwen models, dimensions parameter is not supported. so remove it.
            removed_dimensions = kwargs.pop("dimensions", None)
            if removed_dimensions is not None:
                logger.info(
                    "Stripped unsupported 'dimensions' parameter for SiliconFlow embeddings "
                    "(provider_api_base=%s, model=%s, requested_dimensions=%s)",
                    provider_api_base,
                    self.model,
                    removed_dimensions,
                )
        return kwargs

```

1. Ensure this module has a logger defined, following existing project conventions, e.g. near the top of the file:
   `import logging` and `logger = logging.getLogger(__name__)` (or reuse an existing logger if one is already present).
2. If your logging policy prefers `debug` over `info` for this kind of configuration detail, adjust `logger.info` to `logger.debug` accordingly.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread astrbot/core/provider/sources/openai_embedding_source.py Outdated
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 workaround for the SiliconFlow provider in openai_embedding_source.py by removing the dimensions parameter for non-Qwen models. The reviewer suggested making the API base URL check more robust against protocol variations and whitespaces, and pointed out a potential inconsistency where get_dim() might still return the user-configured dimension instead of the actual returned vector size.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread astrbot/core/provider/sources/openai_embedding_source.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:provider The bug / feature is about AI Provider, Models, LLM Agent, LLM Agent Runner. size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]嵌入模型报错

1 participant