Conversation
sairin1202
commented
Jan 9, 2026
sairin1202
commented
- change default embed size from 25 to 1
- openai api has the max embed size 25
There was a problem hiding this comment.
Pull request overview
This pull request changes the default embed_batch_size parameter from 25 to 1 across the codebase. According to the PR description, this is motivated by a claim that "openai api has the max embed size 25", however this appears to be incorrect based on OpenAI's actual API limits.
Key changes:
- Reduced default
embed_batch_sizefrom 25 to 1 in theOpenAISDKClientconstructor - Updated the corresponding default value in the
LLMConfigsettings class
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/memu/llm/openai_sdk.py | Changed default embed_batch_size parameter from 25 to 1 in OpenAISDKClient.init |
| src/memu/app/settings.py | Changed default embed_batch_size field from 25 to 1 in LLMConfig class |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| chat_model: str, | ||
| embed_model: str, | ||
| embed_batch_size: int = 25, | ||
| embed_batch_size: int = 1, |
There was a problem hiding this comment.
The change to reduce the default embed_batch_size from 25 to 1 appears problematic. According to the OpenAI API documentation, the embeddings endpoint supports batching up to 2048 inputs per request. The PR description states "openai api has the max embed size 25" which appears to be incorrect. Reducing the batch size to 1 would significantly increase the number of API calls required for embedding multiple texts, leading to increased latency and potentially higher costs. Unless there's a specific issue with batching that needs to be addressed, consider keeping the default at 25 or even increasing it for better performance.
| embed_batch_size: int = 1, | |
| embed_batch_size: int = 25, |
| ) | ||
| embed_batch_size: int = Field( | ||
| default=25, | ||
| default=1, |
There was a problem hiding this comment.
The change to reduce the default embed_batch_size from 25 to 1 appears problematic. According to the OpenAI API documentation, the embeddings endpoint supports batching up to 2048 inputs per request. The PR description states "openai api has the max embed size 25" which appears to be incorrect. Reducing the batch size to 1 would significantly increase the number of API calls required for embedding multiple texts, leading to increased latency and potentially higher costs. Unless there's a specific issue with batching that needs to be addressed, consider keeping the default at 25 or even increasing it for better performance.
| default=1, | |
| default=25, |
