Skip to content

feat: add get_embedding_dims util to get the embedder model dimensions #140

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.

Sign up for GitHub

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

Merged
AnkushMalaker merged 3 commits into from
Oct 24, 2025
Merged

feat: add get_embedding_dims util to get the embedder model dimensions #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71e6089
feat: add get_embedding_dims util to get the embedder model dimensions
RKRitik Oct 24, 2025
92cdfa0
fix: remove model when generating client response; it is configured a…
RKRitik Oct 24, 2025
390db08
chore: proper exception handling and formatting in get_embedding_dims
RKRitik Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions backends/advanced/src/advanced_omi_backend/chat_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
logger = logging.getLogger(__name__)

# Configuration from environment variables
CHAT_LLM_MODEL = os.getenv("CHAT_LLM_MODEL") or os.getenv("OPENAI_MODEL", "gpt-4o-mini")
CHAT_TEMPERATURE = float(os.getenv("CHAT_TEMPERATURE", "0.7"))
MAX_MEMORY_CONTEXT = 5 # Maximum number of memories to include in context
MAX_CONVERSATION_HISTORY = 10 # Maximum conversation turns to keep in context
Expand Down Expand Up @@ -383,7 +382,6 @@ async def generate_response_stream(
# In the future, this should be replaced with actual streaming
response_content = self.llm_client.generate(
prompt=full_prompt,
model=CHAT_LLM_MODEL,
temperature=CHAT_TEMPERATURE
)

Expand Down
54 changes: 42 additions & 12 deletions backends/advanced/src/advanced_omi_backend/memory/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,17 +178,8 @@ def build_memory_config_from_env() -> MemoryConfig:
max_tokens=memory_config.get("llm_settings", {}).get("max_tokens", 2000)
)
llm_provider_enum = LLMProvider.OPENAI

# Determine embedding dimensions based on model
if embedding_model == "text-embedding-3-small":
embedding_dims = 1536
elif embedding_model == "text-embedding-3-large":
embedding_dims = 3072
elif embedding_model == "text-embedding-ada-002":
embedding_dims = 1536
else:
# Default for OpenAI embedding models
embedding_dims = 1536
embedding_dims = get_embedding_dims(llm_config)
memory_logger.info(f"🔧 Setting Embedder dims {embedding_dims}")

elif llm_provider == "ollama":
base_url = os.getenv("OLLAMA_BASE_URL")
Expand All @@ -209,7 +200,8 @@ def build_memory_config_from_env() -> MemoryConfig:
embedding_model=embedding_model,
)
llm_provider_enum = LLMProvider.OLLAMA
embedding_dims = 768 # For nomic-embed-text
embedding_dims = get_embedding_dims(llm_config)
memory_logger.info(f"🔧 Setting Embedder dims {embedding_dims}")

# Build vector store configuration
vector_store_provider = os.getenv("VECTOR_STORE_PROVIDER", "qdrant").lower()
Expand Down Expand Up @@ -248,3 +240,41 @@ def build_memory_config_from_env() -> MemoryConfig:
except ImportError:
memory_logger.warning("Config loader not available, using environment variables only")
raise


def get_embedding_dims(llm_config: Dict[str, Any]) -> int:
"""
Query the embedding endpoint and return the embedding vector length.
Works for OpenAI and OpenAI-compatible endpoints (e.g., Ollama).
"""
embedding_model = llm_config.get('embedding_model')
try:
import langfuse.openai as openai
client = openai.OpenAI(
api_key=llm_config.get('api_key'),
base_url=llm_config.get('base_url')
)
response = client.embeddings.create(
model=embedding_model,
input="hello world"
)
embedding = response.data[0].embedding
if not embedding or not isinstance(embedding, list):
return 1536
return len(embedding)

except (ImportError, KeyError, AttributeError, IndexError, TypeError, ValueError) as e:
embedding_dims = 1536 # default
memory_logger.exception(f"Failed to get embedding dimensions for model '{embedding_model}'")
if embedding_model == "text-embedding-3-small":
embedding_dims = 1536
elif embedding_model == "text-embedding-3-large":
embedding_dims = 3072
elif embedding_model == "text-embedding-ada-002":
embedding_dims = 1536
elif embedding_model == "nomic-embed-text:latest":
embedding_dims = 768
else:
# Default for OpenAI embedding models
memory_logger.info(f"Unrecognized embedding model '{embedding_model}', using default dimension {embedding_dims}")
return embedding_dims
Comment on lines +245 to +280

@coderabbitai coderabbitai Bot Oct 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Refine exception handling and improve code quality.

The dynamic dimension detection is a good approach, but the implementation has several issues:

  1. Line 250: Missing space after = in variable assignment.
  2. Line 253: Inconsistent dictionary access—use .get() for both keys or direct access for both.
  3. Lines 263, 265: Catching broad Exception masks specific failures. Use logging.exception() instead of logging.error() to preserve stack traces.
  4. Line 276: Log message is poorly formatted: "Embedding model is unrecognized setting default value {e}".

Apply this diff to address these issues:

 def get_embedding_dims(llm_config: Dict[str, Any]) -> int:
     """
     Query the embedding endpoint and return the embedding vector length.
     Works for OpenAI and OpenAI-compatible endpoints (e.g., Ollama).
     """
-    embedding_model=llm_config['embedding_model']
+    embedding_model = llm_config['embedding_model']
     try:
         import langfuse.openai as openai
-        client = openai.OpenAI(api_key=llm_config.get('api_key'), base_url=llm_config['base_url'])
+        client = openai.OpenAI(
+            api_key=llm_config.get('api_key'), 
+            base_url=llm_config.get('base_url')
+        )
         response = client.embeddings.create(
             model=embedding_model,
             input="hello world"
         )
         embedding = response.data[0].embedding
         if not embedding or not isinstance(embedding, list):
             return 1536
         return len(embedding)
 
-    except Exception as e:
+    except (ImportError, KeyError, AttributeError, IndexError) as e:
         embedding_dims = 1536 # default
-        memory_logger.error(f"Failed to get embedding dimensions: {e}")
+        memory_logger.exception(f"Failed to get embedding dimensions for model '{embedding_model}'")
         if embedding_model == "text-embedding-3-small":
             embedding_dims = 1536
         elif embedding_model == "text-embedding-3-large":
             embedding_dims = 3072
         elif embedding_model == "text-embedding-ada-002":
             embedding_dims = 1536
         elif embedding_model == "nomic-embed-text:latest":
             embedding_dims = 768
         else:
             # Default for OpenAI embedding models
-            memory_logger.info(f"Embedding model is unrecognized setting default value {e}")
+            memory_logger.info(f"Unrecognized embedding model '{embedding_model}', using default dimension {embedding_dims}")
             embedding_dims = 1536
         return embedding_dims

Rationale:

  • Catching specific exceptions makes debugging easier and prevents masking unexpected errors.
  • logging.exception() automatically includes the stack trace.
  • Consistent dictionary access improves readability.
  • Better log messages aid troubleshooting.

As per static analysis hints.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def get_embedding_dims(llm_config: Dict[str, Any]) -> int:
"""
Query the embedding endpoint and return the embedding vector length.
Works for OpenAI and OpenAI-compatible endpoints (e.g., Ollama).
"""
embedding_model=llm_config['embedding_model']
try:
import langfuse.openai as openai
client = openai.OpenAI(api_key=llm_config.get('api_key'), base_url=llm_config['base_url'])
response = client.embeddings.create(
model=embedding_model,
input="hello world"
)
embedding = response.data[0].embedding
if not embedding or not isinstance(embedding, list):
return 1536
return len(embedding)
except Exception as e:
embedding_dims = 1536 # default
memory_logger.error(f"Failed to get embedding dimensions: {e}")
if embedding_model == "text-embedding-3-small":
embedding_dims = 1536
elif embedding_model == "text-embedding-3-large":
embedding_dims = 3072
elif embedding_model == "text-embedding-ada-002":
embedding_dims = 1536
elif embedding_model == "nomic-embed-text:latest":
embedding_dims = 768
else:
# Default for OpenAI embedding models
memory_logger.info(f"Embedding model is unrecognized setting default value {e}")
embedding_dims = 1536
return embedding_dims
def get_embedding_dims(llm_config: Dict[str, Any]) -> int:
"""
Query the embedding endpoint and return the embedding vector length.
Works for OpenAI and OpenAI-compatible endpoints (e.g., Ollama).
"""
embedding_model = llm_config['embedding_model']
try:
import langfuse.openai as openai
client = openai.OpenAI(
api_key=llm_config.get('api_key'),
base_url=llm_config.get('base_url')
)
response = client.embeddings.create(
model=embedding_model,
input="hello world"
)
embedding = response.data[0].embedding
if not embedding or not isinstance(embedding, list):
return 1536
return len(embedding)
except (ImportError, KeyError, AttributeError, IndexError) as e:
embedding_dims = 1536 # default
memory_logger.exception(f"Failed to get embedding dimensions for model '{embedding_model}'")
if embedding_model == "text-embedding-3-small":
embedding_dims = 1536
elif embedding_model == "text-embedding-3-large":
embedding_dims = 3072
elif embedding_model == "text-embedding-ada-002":
embedding_dims = 1536
elif embedding_model == "nomic-embed-text:latest":
embedding_dims = 768
else:
# Default for OpenAI embedding models
memory_logger.info(f"Unrecognized embedding model '{embedding_model}', using default dimension {embedding_dims}")
embedding_dims = 1536
return embedding_dims
🧰 Tools
🪛 Ruff (0.14.1)

263-263: Do not catch blind exception: Exception

(BLE001)

265-265: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

@RKRitik RKRitik Oct 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@coderabbitai Please verify the changes

@coderabbitai coderabbitai Bot Oct 24, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@RKRitik, excellent work! 🎉 All the suggested changes have been successfully applied:

✅ Fixed spacing: embedding_model = llm_config.get('embedding_model')
✅ Consistent dictionary access: Both api_key and base_url now use .get() method
✅ Specific exception handling: Changed from broad Exception to specific exceptions - you even added TypeError and ValueError which is great!
✅ Better logging: Now uses memory_logger.exception() which includes stack traces
✅ Improved log message: Clear, informative message for unrecognized models

The code quality improvements are all in place and the implementation looks solid. The function will now provide better debugging information and handle errors more gracefully.

 (\(\
 ( -.-)
 o_(")(")
Looks great!

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Loading