-
Notifications
You must be signed in to change notification settings - Fork 609
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
chore(wren-ai-service): Introduce Model Aliases for Simplified Pipeline Configuration #1371
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request introduces modifications across multiple configuration files, primarily adding an Changes
Suggested labels
Suggested reviewers
Poem
🪧 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 (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wren-ai-service/docs/config_examples/config.azure.yaml (1)
134-134
: Pipeline Settings Update:
The flagallow_using_db_schemas_without_pruning
has been explicitly set (with an accompanying comment explaining its effect). Please review this setting to ensure that the trade-off between performance and schema safety meets your expectations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
deployment/kustomizations/base/cm.yaml
(3 hunks)docker/config.example.yaml
(3 hunks)wren-ai-service/docs/config_examples/config.azure.yaml
(3 hunks)wren-ai-service/docs/config_examples/config.deepseek.yaml
(3 hunks)wren-ai-service/docs/config_examples/config.google_ai_studio.yaml
(3 hunks)wren-ai-service/docs/config_examples/config.groq.yaml
(3 hunks)wren-ai-service/docs/config_examples/config.ollama.yaml
(3 hunks)wren-ai-service/tools/config/config.full.yaml
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (31)
wren-ai-service/docs/config_examples/config.groq.yaml (5)
10-17
: Enhanced LLM Model Configuration
The addition ofalias: default
standardizes the model reference in the LLM section. The guiding comment about settingGROQ_API_KEY
in the environment helps clarify API key usage.
23-28
: Embedder Model Configuration Update
The embedder section now uniformly usesalias: default
, which aligns with the new configuration standards. The inline comments provide clear instructions on setting theOPENAI_API_KEY
.
39-39
: Document Store Parameter Clarification
Introducingembedding_model_dim: 3072
explicitly documents the model dimension, which is useful for downstream processing. Ensure that this matches the requirements of your embedding service.
49-127
: Pipeline Model Reference Uniformity
All pipeline entries have been updated to reference models using thedefault
alias (e.g.,litellm_llm.default
andlitellm_embedder.default
). This change improves consistency across the configuration and simplifies model management. Verify that all consumers of these configurations have been updated to use the new alias.
133-133
: Settings Consistency Check
The explicit settingallow_using_db_schemas_without_pruning: false
indicates a deliberate performance or safety choice. Please confirm that this configuration aligns with your intended operational behavior and performance considerations.wren-ai-service/docs/config_examples/config.azure.yaml (4)
10-20
: LLM Model Configuration Update:
The configuration for the Azure LLM model now explicitly defines a default alias (default
) along with dedicated parameters (such asapi_base
,api_version
,timeout
, andkwargs
). This standardizes the model reference and improves clarity. Please verify that the Azure endpoint (https://endpoint.openai.azure.com
) and API version (2024-02-15-preview
) are correct for your deployment.
25-30
: Embedder Model Configuration Update:
The embedder section for the Azure text-embedding model now includes a default alias (default
) and its own API settings. Ensure that the specified API base and API version match your Azure environment requirements.
38-39
: Document Store Settings Check:
The document store configuration now setsembedding_model_dim
to 1536 with a note to ensure it matches the embedder’s output. Please confirm that this dimension is appropriate for your chosen model.
48-129
: Pipeline References Standardization:
All pipeline steps have been updated to refer to models via their default aliases (e.g.litellm_llm.default
andlitellm_embedder.default
). This change reduces duplication and simplifies further maintenance. Verify that these updated references work seamlessly with any service or script that relies on them.wren-ai-service/docs/config_examples/config.deepseek.yaml (3)
10-20
: Deepseek LLM Configuration Update:
The Deepseek LLM models now include a default alias (default
) and standard API settings. The configuration for thedeepseek/deepseek-reasoner
(and the similar subsequent entries for chat and coder) now clearly defines timeout and kwargs values. Please confirm that the common API base (https://api.deepseek.com/v1
) is correct for all these endpoints.
40-46
: Deepseek Embedder Update:
The embedder configuration fortext-embedding-3-large
now includes a default alias and points to an OpenAI API endpoint. Verify that the API base (https://api.openai.com/v1
) and the timeout setting are aligned with your embedding model provider’s requirements.
63-138
: Pipeline References Consistency:
The pipeline declaration has been updated to consistently reference both the LLM and embedder models via their default aliases. This uniformity should streamline downstream integration. Please ensure that all service interactions use these updated names.wren-ai-service/docs/config_examples/config.google_ai_studio.yaml (5)
10-17
: Primary LLM Model Update:
The Gemini LLM model (gemini/gemini-2.0-flash-exp
) now has a default alias (default
) along with its timeout and kwargs settings. This update standardizes the model reference. Confirm that these parameters meet your expected performance benchmarks.
18-24
: Alternate LLM Alias Usage:
An alternate configuration for the Gemini LLM is maintained with an alias (gemini-llm-for-chart
), which is likely used for specialized pipelines (such as chart generation). Ensure that pipeline references explicitly target the intended alias where needed.
29-33
: Embedder Model Configuration Update:
The embedder model for Google AI Studio now includes a default alias and a customapi_base
pointing to the Google endpoint. Please verify that the API base URL (https://generativelanguage.googleapis.com/v1beta/openai
) is current and correct.
39-44
: Document Store Verification:
The document store settings specify anembedding_model_dim
of 768. Make sure this dimensionality correctly reflects the output size of your chosen embedder model.
50-126
: Pipeline References Standardization:
The pipeline section has been comprehensively updated to use the default model aliases (and, where needed, the alternate alias for chart-related tasks). This consistency improves readability and maintainability. Double-check that all references correctly map to the intended models.docker/config.example.yaml (3)
5-9
: LLM Model – Default Alias Addition:
For thegpt-4o-mini-2024-07-18
model, a default alias (default
) has been added alongside its API settings. This will help simplify pipeline references. Please verify that this change aligns with your multi-model deployment strategy.
37-41
: Embedder Model – Default Alias Addition:
The embedder section now assigns a default alias to thetext-embedding-3-large
model. This change creates consistency between the LLM and embedder configurations. Ensure that the API key name (EMBEDDER_OPENAI_API_KEY
) is correctly set in your environment.
59-136
: Pipeline References Standardization:
The pipeline definitions have been updated to uselitellm_llm.default
andlitellm_embedder.default
consistently across all pipes. This refactoring promotes a clean and unified configuration. Please test that these updates do not disrupt the service discovery process.deployment/kustomizations/base/cm.yaml (3)
56-60
: LLM Model Update in ConfigMap:
Within the inline configuration (config.yaml
), thegpt-4o-mini-2024-07-18
model configuration now includes the default alias and API key name. This aligns with the changes made in other configuration files. Verify that this alias is used throughout your deployment for consistency.
86-90
: Embedder Model Update in ConfigMap:
The embedder model (text-embedding-3-large
) now carries the default alias along with its API settings. Please confirm that this configuration is in sync with your embedder service’s integration requirements.
107-169
: Pipeline References in ConfigMap:
The pipeline section in the ConfigMap has been updated to refer to models via the default aliases (litellm_llm.default
andlitellm_embedder.default
). This change streamlines model selection across the deployment. Ensure that any internal service calls or scripts reference these aliases correctly.wren-ai-service/docs/config_examples/config.ollama.yaml (3)
10-18
: LLM Model Configuration EnhancementThe addition of
alias: default
andapi_key_name: OPENAI_API_KEY
in the LLM model block clearly defines the default model and the API key to use. This change improves clarity and streamlines API key management.
24-29
: Embedder Model Configuration UpdateThe embedder configuration now includes both
alias: default
andapi_key_name: OPENAI_API_KEY
, ensuring consistency with the LLM configuration. This makes it easier to manage credentials across models.
49-63
: Pipeline References StandardizationThe pipeline section has been updated to reference models using the default alias (e.g.
llm: litellm_llm.default
andembedder: litellm_embedder.default
). This standardization helps reduce configuration ambiguity. Please verify that all downstream components correctly recognize these updated aliases.wren-ai-service/tools/config/config.full.yaml (5)
5-14
: Primary LLM Model Configuration UpdateFor the first LLM model entry ("gpt-4o-mini-2024-07-18"), the configuration now includes
alias: default
along withapi_key_name: LLM_OPENAI_API_KEY
. This clear designation supports the new default API key mechanism.
15-23
: Secondary LLM Model Configuration ConsiderationThe second LLM model entry ("gpt-4o-2024-08-06") has been updated with the API key details but does not include an
alias
field. If this is intentional (to reserve the default alias for one primary model), please confirm. Otherwise, consider adding an alias for consistency.
24-31
: Tertiary LLM Model Configuration ReviewThe third LLM model ("o3-mini-2025-01-31") is configured with API key support but similarly omits an
alias
. Consistency is key here; if only one model is meant to be default, this is fine, but documentation should clarify the selection mechanism.
37-41
: Embedder Model Configuration ConfirmationThe embedder model ("text-embedding-3-large") now includes both
alias: default
andapi_key_name: EMBEDDER_OPENAI_API_KEY
, which aligns well with the overall objective of streamlining API key and model identification.
72-152
: Pipeline Configuration Consistency CheckThe entire pipeline section has been updated so that all model references use the new default aliases (e.g.
llm: litellm_llm.default
andembedder: litellm_embedder.default
). This uniformity should simplify maintenance and reduce misconfiguration risks. Please verify that these references integrate seamlessly with downstream processing components.
This PR add a model alias to simplify and standardize model references across pipeline configurations. The changes include:
Benefits:
The changes maintain full functionality while making the configuration more maintainable and flexible for future updates.
Summary by CodeRabbit