fix(openai-embedding): add dimensions request parameter toggle#8526
Open
kawayiYokami wants to merge 1 commit into
Open
fix(openai-embedding): add dimensions request parameter toggle#8526kawayiYokami wants to merge 1 commit into
kawayiYokami wants to merge 1 commit into
Conversation
dosubot
Bot
added
size:S
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/test_openai_embedding_source.py" line_range="27-28" />
<code_context>
+ return SimpleNamespace(data=data)
+
+
+@pytest.mark.asyncio
+async def test_openai_embedding_does_not_send_configured_dimensions():
+ provider = OpenAIEmbeddingProvider(
+ {
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test case for when `embedding_dimensions` is not configured at all.
In addition to the configured case, please add an async test where `embedding_dimensions` is omitted from `provider_config`. That test should assert that `get_dim()` uses the default dimension and that `embeddings.create` is called only with `input` and `model` (no `dimensions` or extra kwargs).
Suggested implementation:
```python
fake_embeddings = FakeEmbeddingsClient()
provider.client = SimpleNamespace(embeddings=fake_embeddings)
@pytest.mark.asyncio
async def test_openai_embedding_uses_default_dimensions_when_not_configured(mocker):
provider = OpenAIEmbeddingProvider(
{
"id": "openai-compatible-embedding",
"embedding_api_key": "test-key",
"embedding_api_base": "https://example.com/v1",
"embedding_model": "BAAI/bge-m3",
},
{},
)
# Use a mock embeddings client so we can inspect the call signature
fake_embeddings = SimpleNamespace()
fake_embeddings.create = mocker.AsyncMock(
return_value=SimpleNamespace(
data=[SimpleNamespace(embedding=[1.0, 2.0, 3.0])]
)
)
provider.client = SimpleNamespace(embeddings=fake_embeddings)
# get_dim() should fall back to the provider's default dimension
default_dim = provider.get_dim()
assert isinstance(default_dim, int)
assert default_dim > 0
# Trigger an embedding call; dimensions should not be passed explicitly
inputs = ["hello", "world"]
await provider.get_embeddings(inputs)
assert fake_embeddings.create.call_count == 1
call_args, call_kwargs = fake_embeddings.create.call_args
# OpenAI embeddings API should only receive input and model
# (no `dimensions` or other extra kwargs)
assert "input" in call_kwargs
assert "model" in call_kwargs
assert call_kwargs["input"] == inputs
assert call_kwargs["model"] == provider.config["embedding_model"]
assert "dimensions" not in call_kwargs
assert len(call_kwargs) == 2
```
To make this test compile and pass, you may need to:
1. Ensure `pytest-mock` is available so that the `mocker` fixture works, or replace `mocker.AsyncMock` with `unittest.mock.AsyncMock` and import it at the top of the file: `from unittest.mock import AsyncMock`.
2. Confirm that `OpenAIEmbeddingProvider` exposes:
- a `get_dim()` method returning the effective embedding dimension (using a default when `embedding_dimensions` is not configured),
- a `get_embeddings(inputs)` coroutine that ultimately calls `provider.client.embeddings.create(input=inputs, model=...)`,
- a `config` (or similarly named) attribute containing `"embedding_model"`.
3. If the provider uses different method names (e.g. `embed_documents` or `aembed` instead of `get_embeddings`), update the test to call the correct coroutine and adjust the final assertions accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+27
to
+28
| @pytest.mark.asyncio | ||
| async def test_openai_embedding_does_not_send_configured_dimensions(): |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add a test case for when embedding_dimensions is not configured at all.
In addition to the configured case, please add an async test where embedding_dimensions is omitted from provider_config. That test should assert that get_dim() uses the default dimension and that embeddings.create is called only with input and model (no dimensions or extra kwargs).
Suggested implementation:
fake_embeddings = FakeEmbeddingsClient()
provider.client = SimpleNamespace(embeddings=fake_embeddings)
@pytest.mark.asyncio
async def test_openai_embedding_uses_default_dimensions_when_not_configured(mocker):
provider = OpenAIEmbeddingProvider(
{
"id": "openai-compatible-embedding",
"embedding_api_key": "test-key",
"embedding_api_base": "https://example.com/v1",
"embedding_model": "BAAI/bge-m3",
},
{},
)
# Use a mock embeddings client so we can inspect the call signature
fake_embeddings = SimpleNamespace()
fake_embeddings.create = mocker.AsyncMock(
return_value=SimpleNamespace(
data=[SimpleNamespace(embedding=[1.0, 2.0, 3.0])]
)
)
provider.client = SimpleNamespace(embeddings=fake_embeddings)
# get_dim() should fall back to the provider's default dimension
default_dim = provider.get_dim()
assert isinstance(default_dim, int)
assert default_dim > 0
# Trigger an embedding call; dimensions should not be passed explicitly
inputs = ["hello", "world"]
await provider.get_embeddings(inputs)
assert fake_embeddings.create.call_count == 1
call_args, call_kwargs = fake_embeddings.create.call_args
# OpenAI embeddings API should only receive input and model
# (no `dimensions` or other extra kwargs)
assert "input" in call_kwargs
assert "model" in call_kwargs
assert call_kwargs["input"] == inputs
assert call_kwargs["model"] == provider.config["embedding_model"]
assert "dimensions" not in call_kwargs
assert len(call_kwargs) == 2To make this test compile and pass, you may need to:
- Ensure
pytest-mockis available so that themockerfixture works, or replacemocker.AsyncMockwithunittest.mock.AsyncMockand import it at the top of the file:from unittest.mock import AsyncMock. - Confirm that
OpenAIEmbeddingProviderexposes:- a
get_dim()method returning the effective embedding dimension (using a default whenembedding_dimensionsis not configured), - a
get_embeddings(inputs)coroutine that ultimately callsprovider.client.embeddings.create(input=inputs, model=...), - a
config(or similarly named) attribute containing"embedding_model".
- a
- If the provider uses different method names (e.g.
embed_documentsoraembedinstead ofget_embeddings), update the test to call the correct coroutine and adjust the final assertions accordingly.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes the dimensions parameter from OpenAI embedding requests to prevent errors with providers that do not support it, and adds a test suite for this behavior. However, completely removing this parameter breaks support for shortened embeddings in OpenAI's text-embedding-3 models, which can cause dimension mismatches and crashes in vector databases. The feedback suggests conditionally sending the dimensions parameter only for text-embedding-3 models when a shortened dimension is configured, and updating the tests to cover both scenarios.
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 on lines
45
to
60
| async def get_embedding(self, text: str) -> list[float]: | ||
| """获取文本的嵌入""" | ||
| kwargs = self._embedding_kwargs() | ||
| embedding = await self.client.embeddings.create( | ||
| input=text, | ||
| model=self.model, | ||
| **kwargs, | ||
| ) | ||
| return embedding.data[0].embedding | ||
|
|
||
| async def get_embeddings(self, text: list[str]) -> list[list[float]]: | ||
| """批量获取文本的嵌入""" | ||
| kwargs = self._embedding_kwargs() | ||
| embeddings = await self.client.embeddings.create( | ||
| input=text, | ||
| model=self.model, | ||
| **kwargs, | ||
| ) | ||
| return [item.embedding for item in embeddings.data] | ||
|
|
Contributor
There was a problem hiding this comment.
Completely removing the dimensions parameter breaks the ability to use shortened embeddings for OpenAI's text-embedding-3 models (e.g., requesting 512 dimensions instead of 1536).
If a user configures embedding_dimensions to 512 for text-embedding-3-small, the API will now return the default 1536 dimensions because the parameter is not sent. However, get_dim() will still return 512. This mismatch will cause a crash (e.g., dimension mismatch) when inserting the 1536-dimensional vector into a FAISS index or vector database configured for 512 dimensions.
To fix this, we should keep the dimensions parameter but only send it when the model is text-embedding-3 and the configured dimension is explicitly smaller than the default (e.g., < 1536 for small, < 3072 for large). For other models/providers (like bge-m3 or text-embedding-ada-002), the configured dimension matches the model's natural dimension, so we do not send the dimensions parameter, avoiding the 400 errors.
async def get_embedding(self, text: str) -> list[float]:
"""获取文本的嵌入"""
kwargs = self._embedding_kwargs()
embedding = await self.client.embeddings.create(
input=text,
model=self.model,
**kwargs,
)
return embedding.data[0].embedding
async def get_embeddings(self, text: list[str]) -> list[list[float]]:
"""批量获取文本的嵌入"""
kwargs = self._embedding_kwargs()
embeddings = await self.client.embeddings.create(
input=text,
model=self.model,
**kwargs,
)
return [item.embedding for item in embeddings.data]
def _embedding_kwargs(self) -> dict:
"""构建嵌入请求的可选参数"""
kwargs = {}
if "embedding_dimensions" in self.provider_config:
try:
dims = int(self.provider_config["embedding_dimensions"])
if "text-embedding-3-small" in self.model and dims < 1536:
kwargs["dimensions"] = dims
elif "text-embedding-3-large" in self.model and dims < 3072:
kwargs["dimensions"] = dims
except (ValueError, TypeError):
pass
return kwargs
Comment on lines
+27
to
+51
| @pytest.mark.asyncio | ||
| async def test_openai_embedding_does_not_send_configured_dimensions(): | ||
| provider = OpenAIEmbeddingProvider( | ||
| { | ||
| "id": "openai-compatible-embedding", | ||
| "embedding_api_key": "test-key", | ||
| "embedding_api_base": "https://example.com/v1", | ||
| "embedding_model": "BAAI/bge-m3", | ||
| "embedding_dimensions": 1024, | ||
| }, | ||
| {}, | ||
| ) | ||
| fake_embeddings = FakeEmbeddingsClient() | ||
| provider.client = SimpleNamespace(embeddings=fake_embeddings) | ||
|
|
||
| embedding = await provider.get_embedding("hello") | ||
| embeddings = await provider.get_embeddings(["hello", "world"]) | ||
|
|
||
| assert embedding == [1.0, 2.0, 3.0] | ||
| assert embeddings == [[0.0, 0.0, 1.0], [1.0, 0.0, 1.0]] | ||
| assert provider.get_dim() == 1024 | ||
| assert fake_embeddings.calls == [ | ||
| {"input": "hello", "model": "BAAI/bge-m3"}, | ||
| {"input": ["hello", "world"], "model": "BAAI/bge-m3"}, | ||
| ] |
Contributor
There was a problem hiding this comment.
Let's update the test to cover both cases:
- Non-OpenAI models or default dimensions (where
dimensionsshould NOT be sent). - OpenAI
text-embedding-3models with shortened dimensions (wheredimensionsSHOULD be sent).
@pytest.mark.asyncio
async def test_openai_embedding_dimensions_handling():
# Case 1: Non-OpenAI model or default dimension -> should NOT send dimensions parameter
provider_non_openai = OpenAIEmbeddingProvider(
{
"id": "openai-compatible-embedding",
"embedding_api_key": "test-key",
"embedding_api_base": "https://example.com/v1",
"embedding_model": "BAAI/bge-m3",
"embedding_dimensions": 1024,
},
{},
)
fake_embeddings_1 = FakeEmbeddingsClient()
provider_non_openai.client = SimpleNamespace(embeddings=fake_embeddings_1)
embedding = await provider_non_openai.get_embedding("hello")
embeddings = await provider_non_openai.get_embeddings(["hello", "world"])
assert embedding == [1.0, 2.0, 3.0]
assert embeddings == [[0.0, 0.0, 1.0], [1.0, 0.0, 1.0]]
assert provider_non_openai.get_dim() == 1024
assert fake_embeddings_1.calls == [
{"input": "hello", "model": "BAAI/bge-m3"},
{"input": ["hello", "world"], "model": "BAAI/bge-m3"},
]
# Case 2: OpenAI text-embedding-3 model with shortened dimension -> SHOULD send dimensions parameter
provider_openai = OpenAIEmbeddingProvider(
{
"id": "openai-embedding",
"embedding_api_key": "test-key",
"embedding_api_base": "https://api.openai.com/v1",
"embedding_model": "text-embedding-3-small",
"embedding_dimensions": 512,
},
{},
)
fake_embeddings_2 = FakeEmbeddingsClient()
provider_openai.client = SimpleNamespace(embeddings=fake_embeddings_2)
await provider_openai.get_embedding("hello")
assert fake_embeddings_2.calls == [
{"input": "hello", "model": "text-embedding-3-small", "dimensions": 512},
]
kawayiYokami
force-pushed
the
fix/embedding-dimensions-param
branch
from
60cbeaf to
857dc7f
Compare
dosubot
Bot
added
size:M
kawayiYokami
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
修复 #8506。
本 PR 为 OpenAI-compatible embedding provider 新增一个嵌入维度请求参数开关，默认关闭。
当前配置项
embedding_dimensions的主要语义是本地向量维度元数据：它用于get_dim()，并供 FAISS / 数据库 / UI 等本地逻辑判断向量长度。它不应该在默认情况下被自动转换成 API 请求参数dimensions发送给上游。dimensions对 OpenAI-compatible embedding 接口来说不是通用参数。很多 embedding 模型只接受input和model，额外发送dimensions会导致 400 invalid parameter。硅基流动的BAAI/bge-m3就是典型例子。同时，OpenAI
text-embedding-3系列等模型确实支持通过dimensions请求降维。因此本 PR 不彻底删除该能力，而是把它改成显式开关：dimensions，避免误伤不支持该参数的模型。embedding_dimensions作为 APIdimensions参数发送，用于支持明确支持降维的模型。Modifications / 改动点
新增 OpenAI Embedding 配置项
embedding_dimensions_as_request_param，默认false。在 provider 编辑界面的“嵌入维度 / 自动检测”旁边增加“请求参数”开关。
修改
astrbot/core/provider/sources/openai_embedding_source.py：只有开关开启时才发送dimensions。保留
embedding_dimensions在get_dim()中作为本地向量维度元数据使用。新增回归测试，覆盖：
dimensions。dimensions。embedding_dimensions时即使开关打开也不会发送无效参数。This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
运行
uv run时，本地.venv中存在旧包 metadata 警告，uv 随后自动重新安装/修复了包环境；测试本身通过。关于知识库多嵌入模型兼容性的说明
本 PR 只修复当前 embedding 请求参数问题，不直接改造知识库架构。但这里必须强调：使用向量维度来标记或判断知识库索引兼容性，是原理上错误的实现。
向量维度只能说明向量数组长度一致，最多只能判断它们能否被写入同一个 FAISS index。它不能说明两个 embedding 模型产生的向量处在同一个语义空间中。
两个不同 embedding 模型即使输出完全相同的维度，也可能有完全不同的坐标系、向量分布和相似度语义。把它们混用到同一个知识库向量索引中，通常不会报维度错误，但检索结果会失真。这类错误不会以“插入失败”的形式暴露，而是以更隐蔽的检索质量退化形式出现。
因此，知识库的多 embedding 模型兼容不能只依赖
embedding_dimensions。正确方向应该是按 embedding provider / model 等向量空间身份隔离索引，或者在切换 embedding 模型时触发索引重建。此前 #6394 中曾提交过更完整的 RAG / 知识库改造方案，其中包含 provider 隔离的 FAISS 索引与在线重建能力，可作为后续实现方向的参考。本 PR 先保持较小范围，只处理dimensions请求参数开关问题。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.txtandpyproject.toml./ 我确保没有引入新依赖库，或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。