-
Notifications
You must be signed in to change notification settings - Fork 30
feat: support openai embedding and add llm base_urls in client #182
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -117,8 +117,7 @@ const DatasourceDetail: React.FC = () => { | |||||
| if (!datasourceId) return; | ||||||
| if (yamlContent) { | ||||||
| const yamlResponse = validateDynamicQueryYaml(yamlContent); | ||||||
| if (yamlResponse.valid) { | ||||||
| } else { | ||||||
| if (!yamlResponse.valid) { | ||||||
| notifyError(yamlResponse.error); | ||||||
| return; | ||||||
| } | ||||||
|
|
@@ -190,6 +189,7 @@ const DatasourceDetail: React.FC = () => { | |||||
| notifySuccess('Datasource deleted successfully'); | ||||||
| navigate(`/apps/${appId}/datasources`); | ||||||
| } catch (error) { | ||||||
| console.error('Error deleting datasource'); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Include the error object in the log for debugging. Logging a static string without the actual error loses valuable debugging information when troubleshooting production issues. - console.error('Error deleting datasource');
+ console.error('Error deleting datasource:', error);Additionally, consider notifying the user about the failure since the error is currently swallowed silently: } catch (error) {
console.error('Error deleting datasource:', error);
+ notifyError('Failed to delete datasource');
} finally {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| } finally { | ||||||
| setDeleting(false); | ||||||
| setShowDeleteConfirm(false); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,12 +2,13 @@ | |||||||||||||||||||||||||||||||||||||||||||||
| import requests | ||||||||||||||||||||||||||||||||||||||||||||||
| from rag_ingestion.env import EMBEDDING_SERVICE_URL | ||||||||||||||||||||||||||||||||||||||||||||||
| from flo_utils.utils.log import logger | ||||||||||||||||||||||||||||||||||||||||||||||
| from rag_ingestion.env import OPENAI_API_KEY, EMBEDDING_MODEL | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| class EmbeddingFunc: | ||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| self.max_batch_size = 32 | ||||||||||||||||||||||||||||||||||||||||||||||
| self.bgm_url = f'{EMBEDDING_SERVICE_URL}/v1/embeddings' | ||||||||||||||||||||||||||||||||||||||||||||||
| self.bgm_url = f'{EMBEDDING_SERVICE_URL}' | ||||||||||||||||||||||||||||||||||||||||||||||
| logger.info(f'The embedding url is {EMBEDDING_SERVICE_URL}') | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def generate_document_embeddings(self, chunks): | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -33,12 +34,20 @@ def generate_chunk_embeddings(self, chunks): | |||||||||||||||||||||||||||||||||||||||||||||
| return embeddings | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def bgm_embedding(self, texts): | ||||||||||||||||||||||||||||||||||||||||||||||
| headers = { | ||||||||||||||||||||||||||||||||||||||||||||||
| 'Authorization': f'Bearer {OPENAI_API_KEY}', | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| response = requests.post( | ||||||||||||||||||||||||||||||||||||||||||||||
| self.bgm_url, | ||||||||||||||||||||||||||||||||||||||||||||||
| headers=headers if EMBEDDING_MODEL == 'text-embedding-3-small' else None, | ||||||||||||||||||||||||||||||||||||||||||||||
| json={ | ||||||||||||||||||||||||||||||||||||||||||||||
| 'model': 'BAAI/bge-m3', | ||||||||||||||||||||||||||||||||||||||||||||||
| 'model': EMBEDDING_MODEL, | ||||||||||||||||||||||||||||||||||||||||||||||
| 'input': texts, | ||||||||||||||||||||||||||||||||||||||||||||||
| 'encoding_format': 'float', | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| timeout=60, | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
41
to
50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add timeout to prevent indefinite hangs. The response = requests.post(
self.bgm_url,
headers={'Authorization': 'Bearer ' + openai_api_key},
json={
# 'model': 'BAAI/bge-m3',
'model': 'text-embedding-3-small',
'input': texts,
'encoding_format': 'float',
},
+ timeout=60,
)📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.14.8)38-38: Probable use of (S113) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| return response.json()['data'][0]['embedding'] | ||||||||||||||||||||||||||||||||||||||||||||||
| response.raise_for_status() | ||||||||||||||||||||||||||||||||||||||||||||||
| res = response.json() | ||||||||||||||||||||||||||||||||||||||||||||||
| return res['data'][0]['embedding'] | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+52
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returns only the first embedding; batch processing is broken. When Additionally, there's no error handling for HTTP failures or unexpected responses. - res = response.json()
- return res['data'][0]['embedding']
+ response.raise_for_status()
+ res = response.json()
+ return [item['embedding'] for item in res['data']]This returns all embeddings in order, which pairs with the fix needed at line 20. |
||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,9 @@ | |
|
|
||
| CLOUD_PROVIDER = os.getenv('CLOUD_PROVIDER', 'gcp') | ||
| RETRY_COUNT = os.getenv('RETRY_COUNT', 3) | ||
| OPENAI_API_KEY = os.getenv('OPENAI_API_KEY') | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find all usages of OPENAI_API_KEY in the codebase
echo "=== Searching for OPENAI_API_KEY usages ==="
rg -n "OPENAI_API_KEY" --type=py -C3
echo ""
echo "=== Searching for OpenAI client initialization ==="
rg -n "openai\.(Client|OpenAI|AzureOpenAI)" --type=py -C3
echo ""
echo "=== Searching for from_env or similar patterns ==="
rg -n "from_env|api_key.*=|Client\(" --type=py | head -50Repository: rootflo/wavefront Length of output: 22707 🏁 Script executed: cat -n wavefront/server/background_jobs/rag_ingestion/rag_ingestion/embeddings/embed.pyRepository: rootflo/wavefront Length of output: 2262 Add validation to ensure OPENAI_API_KEY is set when required. When 🤖 Prompt for AI Agents |
||
| EMBEDDING_SERVICE_URL = os.getenv('EMBEDDING_SERVICE_URL') | ||
| FLOWARE_SERVICE_URL = os.getenv('FLOWARE_SERVICE_URL') | ||
| APP_ENV = os.getenv('APP_ENV', 'dev') | ||
| PASSTHROUGH_SECRET = os.getenv('PASSTHROUGH_SECRET') | ||
| EMBEDDING_MODEL = os.getenv('EMBEDDING_MODEL', 'text-embedding-3-small') | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import requests | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import List | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from dataclasses import dataclass | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -13,7 +14,7 @@ class KnowledgeBaseEmbeddingObject: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class EmbeddingFunc: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self, embedding_url): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.max_batch_size = 32 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.bgm_url = f'{embedding_url}/v1/embeddings' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.bgm_url = f'{embedding_url}' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def generate_document_embeddings(self, chunks): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| contents = [v['content'] for v in chunks.values()] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -42,12 +43,16 @@ def generate_chunk_embeddings(self, chunks): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return embeddings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def bgm_embedding(self, texts): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| openai_api_key = os.getenv('OPENAI_API_KEY') or '' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response = requests.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.bgm_url, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers={'Authorization': 'Bearer ' + openai_api_key}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| json={ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'model': 'BAAI/bge-m3', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # 'model': 'BAAI/bge-m3', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'model': 'text-embedding-3-small', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+51
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Search for references to the old BAAI/bge-m3 model or its typical vector size
rg -n "BAAI/bge-m3|bge-m3" --type py
# Search for vector_size configurations that might need updating
rg -n "vector_size" --type py -C 2Repository: rootflo/wavefront Length of output: 25282 🏁 Script executed: # Read the embed.py file at the target lines to see context
head -70 wavefront/server/modules/knowledge_base_module/knowledge_base_module/embeddings/embed.py | tail -30
# Check if both embed.py files have similar changes
head -60 wavefront/server/background_jobs/rag_ingestion/rag_ingestion/embeddings/embed.py | tail -20Repository: rootflo/wavefront Length of output: 1727 🏁 Script executed: # Search for bgm_url definition
rg -n "bgm_url" --type py -B 3 -A 3
# Search for any migration-related code or documentation
rg -n "migration|migrate|regenerate" --type py -i | head -30
# Check for configuration or setup documentation
fd -e "README|SETUP|CONFIG|\.md" -i | head -20Repository: rootflo/wavefront Length of output: 7156 🏁 Script executed: # Search for embedding deletion or regeneration logic
rg -n "delete.*embedding|regenerate|re-embed" --type py -i
# Check if embeddings are generated on-demand or stored
rg -n "generate.*embedding|store.*embedding" --type py -A 2 | head -40
# Look for any data migration scripts
fd -e "py" -path "*/alembic/versions/*" -exec basename {} \; | grep -i embedRepository: rootflo/wavefront Length of output: 5787 🌐 Web query:
💡 Result: BAAI's BGE‑M3 produces 1024‑dimensional embeddings (output vector length = 1024). [1][2][3] Sources:
Verify embedding migration strategy before deploying the model change. The switch from
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'input': texts, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'encoding_format': 'float', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return response.json()['data'][0]['embedding'] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| res = response.json() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return res['data'][0]['embedding'] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
45
to
+58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add timeout, error handling, and validate API key. The current implementation has several issues:
Apply this diff to address these issues: def bgm_embedding(self, texts):
openai_api_key = os.getenv('OPENAI_API_KEY') or ''
+ if not openai_api_key:
+ raise ValueError('OPENAI_API_KEY environment variable is required')
+
response = requests.post(
self.bgm_url,
headers={'Authorization': 'Bearer ' + openai_api_key},
json={
# 'model': 'BAAI/bge-m3',
'model': 'text-embedding-3-small',
'input': texts,
'encoding_format': 'float',
},
+ timeout=30,
)
+ response.raise_for_status()
res = response.json()
return res['data'][0]['embedding']📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.14.8)47-47: Probable use of (S113) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Anthropic Claude API base URL official documentation💡 Result:
The official Anthropic Claude API base URL is: https://api.anthropic.com/v1/. [1][2]
Sources:
Add
/v1to the Anthropic base URL.The Anthropic base URL should be
https://api.anthropic.com/v1/according to official Anthropic API documentation. Update line 113 to include the/v1path segment.Also applies to: 113-113
🤖 Prompt for AI Agents