chore(wren-ai-service): minor updates#1236
Conversation
cyyeh
added
module/ai-service
WalkthroughThis pull request introduces several configuration modifications across multiple files, primarily focusing on removing Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
docker/docker-compose.yaml (1)
29-31: Consider data consistency between shared volume and host directory.The
wren-engineservice now has access to both the shareddatavolume and a host-mounted directory. This dual access pattern requires careful consideration:
- Ensure clear separation of concerns between data stored in shared volume vs. host directory
- Document any potential race conditions if multiple services access the same files
- Consider implementing file locking mechanisms if needed
wren-ai-service/src/pipelines/generation/sql_generation_reasoning.py (1)
Line range hint
89-92: Consider adding error handling for JSON parsing.The
post_processfunction directly parses the LLM response without any error handling. Consider adding try-catch to handle potential JSON parsing errors gracefully.@observe() def post_process( generate_sql_reasoning: dict, ) -> dict: - return orjson.loads(generate_sql_reasoning.get("replies")[0]) + try: + return orjson.loads(generate_sql_reasoning.get("replies")[0]) + except (orjson.JSONDecodeError, IndexError) as e: + logger.error(f"Failed to parse LLM response: {e}") + raise ValueError("Invalid response format from LLM") from ewren-ai-service/docs/config_examples/config.deepseek.yaml (3)
1-3: Enhance header documentation for better user guidance.Consider adding more specific instructions about:
- The exact steps to rename and place the file
- Prerequisites like API keys and environment setup
- Required permissions for the
.wrenaidirectory-# you should rename this file to config.yaml and put it in ~/.wrenai -# please pay attention to the comments starting with # and adjust the config accordingly +# Setup Instructions: +# 1. Create directory: mkdir -p ~/.wrenai +# 2. Copy this file: cp config.deepseek.yaml ~/.wrenai/config.yaml +# 3. Set required permissions: chmod 600 ~/.wrenai/config.yaml +# 4. Follow the comments below to configure your API keys and settings
36-36: Make service endpoints configurable via environment variables.The service endpoints are hardcoded:
http://wren-ui:3000http://qdrant:6333Consider making these configurable for different environments.
-endpoint: http://wren-ui:3000 +endpoint: ${WREN_UI_ENDPOINT:-http://wren-ui:3000} -location: http://qdrant:6333 +location: ${QDRANT_ENDPOINT:-http://qdrant:6333}Also applies to: 41-41
122-122: Fix trailing whitespace.Remove trailing space after
text-embedding-3-large.- embedder: openai_embedder.text-embedding-3-large + embedder: openai_embedder.text-embedding-3-large🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 122-122: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
deployment/kustomizations/base/cm.yaml(0 hunks)docker/config.example.yaml(0 hunks)docker/docker-compose.yaml(1 hunks)wren-ai-service/docs/config_examples/config.deepseek.yaml(1 hunks)wren-ai-service/docs/config_examples/config.google_ai_studio.yaml(1 hunks)wren-ai-service/docs/config_examples/config.groq.yaml(1 hunks)wren-ai-service/docs/config_examples/config.ollama.yaml(1 hunks)wren-ai-service/src/pipelines/generation/sql_generation_reasoning.py(1 hunks)wren-ai-service/src/providers/llm/litellm.py(1 hunks)wren-ai-service/tools/config/config.example.yaml(0 hunks)wren-ai-service/tools/config/config.full.yaml(0 hunks)
💤 Files with no reviewable changes (4)
- deployment/kustomizations/base/cm.yaml
- wren-ai-service/tools/config/config.full.yaml
- docker/config.example.yaml
- wren-ai-service/tools/config/config.example.yaml
✅ Files skipped from review due to trivial changes (3)
- wren-ai-service/docs/config_examples/config.groq.yaml
- wren-ai-service/docs/config_examples/config.ollama.yaml
- wren-ai-service/docs/config_examples/config.google_ai_studio.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
wren-ai-service/docs/config_examples/config.deepseek.yaml
[warning] 29-29: wrong indentation: expected 0 but found 2
(indentation)
[warning] 52-52: wrong indentation: expected 0 but found 2
(indentation)
[error] 122-122: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
wren-ai-service/src/pipelines/generation/sql_generation_reasoning.py (2)
28-34: LGTM! Clear JSON format specification added to the system prompt.The addition of the "FINAL ANSWER FORMAT" section clearly specifies the expected JSON structure for the reasoning plan output, which aligns with the PR objective of supporting
json_objectas output from LLM.
Line range hint
103-112: Verify ifresponse_formatspecification is still needed.The AI summary indicates that
response_formatspecifications are being removed from config files for specific AI models. Please verify if thisresponse_formatspecification inSQL_GENERATION_REASONING_MODEL_KWARGSshould also be removed for consistency.wren-ai-service/docs/config_examples/config.deepseek.yaml (2)
29-30: Verify embedding model dimensions.The document store configuration specifies 3072 dimensions. Please verify this matches the output dimensions of
text-embedding-3-large.✅ Verification successful
Embedding dimensions are correctly configured
The dimension value of 3072 specified in the configuration matches the official test configuration (wren-ai-service/tests/data/config.test.yaml) for the text-embedding-3-large model.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify embedding dimensions for text-embedding-3-large # Fetch model details from OpenAI API curl -s https://api.openai.com/v1/models/text-embedding-3-large \ -H "Authorization: Bearer $OPENAI_API_KEY" | jq '.dimensions'Length of output: 115
Script:
#!/bin/bash # Search for dimension configurations and embedding model references echo "=== Searching config files ===" rg -A 5 "dimension|text-embedding-3" --type yaml echo -e "\n=== Searching documentation ===" fd . -e md -e rst | xargs rg -l "text-embedding-3|embedding dimension"Length of output: 28152
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 29-29: wrong indentation: expected 0 but found 2
(indentation)
146-147: Review development mode settings.The configuration has:
- Logging level set to DEBUG
- Development mode enabled
These settings should be adjusted for production environments.
Consider adding environment-specific configurations or warning comments:
- logging_level: DEBUG - development: true + # Production environments should use: + # logging_level: INFO + # development: false + logging_level: ${LOGGING_LEVEL:-DEBUG} + development: ${DEVELOPMENT_MODE:-true}
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
wren-ai-service/docs/config_examples/config.deepseek.yaml (5)
1-3: Enhance configuration documentation.Consider adding more context to the setup instructions:
- Document the required DeepSeek API key format
- Add a link to DeepSeek API documentation
- Specify the minimum required WrenAI version
# you should rename this file to config.yaml and put it in ~/.wrenai +# Required environment variables: +# - DEEPSEEK_API_KEY: Your DeepSeek API key from https://platform.deepseek.ai +# Minimum WrenAI version: X.Y.Z +# For more details, see: https://platform.deepseek.ai/docs # please pay attention to the comments starting with # and adjust the config accordingly
40-44: Consider adding health check endpoint configuration.The engine configuration could benefit from a health check endpoint to ensure service availability.
type: engine provider: wren_ui endpoint: http://wren-ui:3000 +health_check_endpoint: /health # Add health check endpoint +health_check_interval: 60 # Add health check interval in seconds
45-52: Consider adding backup configuration.The Qdrant configuration could benefit from backup settings to prevent data loss.
type: document_store provider: qdrant location: http://qdrant:6333 embedding_model_dim: 3072 # put your embedding model dimension here timeout: 120 recreate_index: false +backup_enabled: true # Enable periodic backups +backup_path: /data/backups # Specify backup location +backup_interval: 86400 # Daily backup interval in seconds
53-142: Consider adding retry and timeout configurations for pipelines.While the pipeline configurations are comprehensive, they could benefit from task-specific settings.
type: pipeline +default_retry_count: 3 # Add default retry count +default_retry_delay: 5 # Add default retry delay in seconds pipes: - name: db_schema_indexing embedder: openai_embedder.text-embedding-3-large document_store: qdrant + retry_count: 5 # Override default retry count for specific tasks + timeout: 300 # Add task-specific timeout🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 59-59: wrong indentation: expected 0 but found 2
(indentation)
[error] 129-129: trailing spaces
(trailing-spaces)
129-129: Fix trailing whitespace.Remove the trailing whitespace at the end of line 129.
- embedder: openai_embedder.text-embedding-3-large + embedder: openai_embedder.text-embedding-3-large🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 129-129: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wren-ai-service/docs/config_examples/config.deepseek.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
wren-ai-service/docs/config_examples/config.deepseek.yaml
[warning] 36-36: wrong indentation: expected 0 but found 2
(indentation)
[warning] 59-59: wrong indentation: expected 0 but found 2
(indentation)
[error] 129-129: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
wren-ai-service/docs/config_examples/config.deepseek.yaml (3)
31-39: Verify embedding model dimension.The configuration uses
text-embedding-3-largewhich is correct, but we should verify:
- The embedding dimension (3072) matches the model's output
- The API endpoint compatibility
✅ Verification successful
Configuration is correct for text-embedding-3-large model
The embedding configuration is properly set up:
- The model dimension (3072) matches OpenAI's text-embedding-3-large specifications
- The vector store is correctly configured with the same dimension
- The API endpoint (api.openai.com/v1) is correct for OpenAI embeddings
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify OpenAI embedding model configuration # Check model details curl -s "https://api.openai.com/v1/models/text-embedding-3-large" \ -H "Authorization: Bearer $EMBEDDER_OPENAI_API_KEY" | jq '.dimensions' # Verify API compatibility curl -s "https://api.openai.com/v1/embeddings" \ -H "Authorization: Bearer $EMBEDDER_OPENAI_API_KEY" \ -H "Content-Type: application/json" \ -d '{ "model": "text-embedding-3-large", "input": "Test" }' | jq '.data[0].embedding | length'Length of output: 333
Script:
#!/bin/bash # Search for embedding dimension configuration and vector store setup rg -i "dimension|vector.*store|embedding.*size" -A 3 -B 3 # Search for database schema or table creation fd -e sql -e py | xargs rg -i "create.*table|vector|embedding" -A 3 -B 3 # Search for configuration files fd -e yaml -e yml | xargs rg -i "embedding|vector|dimension" -A 3 -B 3Length of output: 86058
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 36-36: wrong indentation: expected 0 but found 2
(indentation)
4-30: Verify DeepSeek model configurations.The LLM configurations look good with appropriate settings for each model. However, let's verify:
- The model endpoints and response formats
- The availability of these specific models
✅ Verification successful
DeepSeek model configurations are correctly structured
The configuration is valid with appropriate model selection for different tasks:
- deepseek-coder (json_object format) for SQL and technical tasks
- deepseek-chat (text format) for natural language interactions
- deepseek-reasoner (text format) for analytical reasoning
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify DeepSeek model configurations # Check model endpoints curl -s "https://api.deepseek.com/v1/models" \ -H "Authorization: Bearer $DEEPSEEK_API_KEY" | jq '.data[] | select(.id | contains("deepseek"))' # Verify response format compatibility for model in "deepseek-reasoner" "deepseek-chat" "deepseek-coder"; do echo "Checking $model..." curl -s "https://api.deepseek.com/v1/chat/completions" \ -H "Authorization: Bearer $DEEPSEEK_API_KEY" \ -H "Content-Type: application/json" \ -d "{ \"model\": \"$model\", \"messages\": [{\"role\": \"user\", \"content\": \"Hello\"}], \"response_format\": {\"type\": \"text\"} }" | jq '.error?' doneLength of output: 1595
Script:
#!/bin/bash # Search for DeepSeek related files and documentation echo "=== Searching for DeepSeek related files ===" fd -t f -e md -e rst -e txt . | grep -i deepseek || true fd -t f -e yaml -e yml . | grep -i deepseek || true echo -e "\n=== Searching for DeepSeek model references in code ===" rg -i "deepseek" --type py --type yaml --type json -C 2 || true echo -e "\n=== Looking for test files with DeepSeek ===" fd -t f -e py test | xargs rg -i "deepseek" || trueLength of output: 8862
143-154: 🛠️ Refactor suggestionReview and document performance-critical settings.
The settings section contains performance-critical values that should be documented and validated:
- Cache sizes and TTL values
- Batch sizes and retrieval limits
- Development mode in production
settings: - column_indexing_batch_size: 50 - table_retrieval_size: 10 - table_column_retrieval_size: 100 + # Indexing settings + column_indexing_batch_size: 50 # Adjust based on available memory + # Retrieval settings + table_retrieval_size: 10 # Maximum number of tables to retrieve + table_column_retrieval_size: 100 # Maximum number of columns to retrieve allow_using_db_schemas_without_pruning: false - query_cache_maxsize: 1000 - query_cache_ttl: 3600 + # Cache settings + query_cache_maxsize: 1000 # Maximum number of cached queries + query_cache_ttl: 3600 # Cache TTL in seconds langfuse_host: https://cloud.langfuse.com langfuse_enable: true logging_level: DEBUG - development: true + development: false # Should be false in production
wwwy3y3
deleted the
chore/ai-service/fix-json-object-and-minor-updates
branch
2 participants
Summary by CodeRabbit
Configuration Updates
response_formatspecification forgpt-4o-mini-2024-07-18andgpt-4o-2024-08-06models across multiple configuration fileswren-engineservice to mount project data directoryDocumentation
allow_using_db_schemas_without_pruningsetting in configuration files