Skip to content

Honor dimensions param#131

Merged
jnu merged 2 commits into
mainfrom
dim
Apr 28, 2026
Merged

Honor dimensions param#131
jnu merged 2 commits into
mainfrom
dim

Conversation

@jnu

@jnu jnu commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Honors dimension param in embeddings client

Comment on lines +111 to +116
"input": text,
"model": self.config.model,
}
if self.config.dimensions:
params["dimensions"] = self.config.dimensions
result = self.client.embeddings.create(**params)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The embed_async method in OpenAIEmbeddingGenerator does not pass the dimensions parameter to the OpenAI API, unlike its synchronous counterpart embed.
Severity: MEDIUM

Suggested Fix

Update the embed_async method to construct a params dictionary and pass the dimensions parameter if it's set in self.config, mirroring the logic in the synchronous embed method. Also, add a test case to verify the behavior of embed_async with the dimensions parameter.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: bc2/lib/embedding/openai.py#L111-L116

Potential issue: The pull request updated the synchronous `embed` method to correctly
pass the `dimensions` parameter from `self.config` to the OpenAI API. However, the
asynchronous `embed_async` method was not updated. It still calls
`self.aclient.embeddings.create` without including the `dimensions` parameter. This will
cause users who configure a specific embedding dimension and use the async path to
receive embeddings of the model's default size (e.g., 3072) instead of the requested
size (e.g., 256), leading to a silent inconsistency and potential downstream errors.

Did we get this right? 👍 / 👎 to inform future reviews.

@jnu jnu merged commit 38fedd6 into main Apr 28, 2026
2 checks passed
@jnu jnu deleted the dim branch April 28, 2026 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant