-
Notifications
You must be signed in to change notification settings - Fork 8.2k
chore: clean up watsonx urls usage #10562
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
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe PR consolidates hardcoded IBM Watsonx API endpoint lists across four component and model files by replacing them with a shared constant Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #10562 +/- ##
==========================================
- Coverage 39.03% 31.66% -7.37%
==========================================
Files 1482 1330 -152
Lines 85923 60416 -25507
Branches 10348 9030 -1318
==========================================
- Hits 33539 19132 -14407
+ Misses 51330 40374 -10956
+ Partials 1054 910 -144
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lfx/src/lfx/components/ibm/watsonx.py(2 hunks)src/lfx/src/lfx/components/ibm/watsonx_embeddings.py(2 hunks)src/lfx/src/lfx/components/models/embedding_model.py(1 hunks)src/lfx/src/lfx/components/models/language_model.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-21T14:16:14.125Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-21T14:16:14.125Z
Learning: Applies to src/backend/tests/**/*.py : Use 'MockLanguageModel' for testing language model components without external API calls in backend Python tests.
Applied to files:
src/lfx/src/lfx/components/models/language_model.py
📚 Learning: 2025-08-11T16:52:17.902Z
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/langflow PR: 9336
File: src/backend/base/langflow/base/models/openai_constants.py:5-33
Timestamp: 2025-08-11T16:52:17.902Z
Learning: In the Langflow codebase (src/backend/base/langflow/base/models/openai_constants.py), GPT-5 series models (gpt-5, gpt-5-mini, gpt-5-nano, gpt-5-chat-latest) are reasoning models and should have the reasoning=True flag set, which places them in OPENAI_REASONING_MODEL_NAMES rather than OPENAI_CHAT_MODEL_NAMES.
Applied to files:
src/lfx/src/lfx/components/models/language_model.py
📚 Learning: 2025-08-11T16:52:26.755Z
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/langflow PR: 9336
File: src/backend/base/langflow/base/models/openai_constants.py:29-33
Timestamp: 2025-08-11T16:52:26.755Z
Learning: The "gpt-5-chat-latest" model in the OpenAI models configuration does not support tool calling, so tool_calling should be set to False for this model in src/backend/base/langflow/base/models/openai_constants.py.
Applied to files:
src/lfx/src/lfx/components/models/language_model.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Update Component Index
🔇 Additional comments (5)
src/lfx/src/lfx/components/models/language_model.py (2)
16-16: LGTM! Clean refactoring to centralized URL constant.The import consolidates URL management by using a shared constant instead of duplicating the URL list across multiple files.
110-111: Correct usage of the imported constant.The dropdown correctly references the centralized
IBM_WATSONX_URLSconstant.src/lfx/src/lfx/components/models/embedding_model.py (1)
8-8: LGTM! Consistent refactoring.The import and usage of
IBM_WATSONX_URLSare consistent with the centralization effort. The fallback toIBM_WATSONX_URLS[0]on line 160 provides a sensible default.Also applies to: 50-51, 160-160
src/lfx/src/lfx/components/ibm/watsonx.py (1)
9-9: LGTM! Clean refactoring pattern.Replacing the inline hardcoded list with
IBM_WATSONX_URLScentralizes URL management effectively.Also applies to: 25-25
src/lfx/src/lfx/components/ibm/watsonx_embeddings.py (1)
10-10: IBM_WATSONX_URLS is properly defined and correctly exported.The constant is defined as a list of six valid Watson x regional endpoint URLs in
lfx.base.models.watsonx_constantsand is correctly importable from that module. The import statement is accurate.
| "https://jp-tok.ml.cloud.ibm.com", | ||
| "https://ca-tor.ml.cloud.ibm.com", | ||
| ], | ||
| options={IBM_WATSONX_URLS}, |
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.
Fix syntax error: remove curly braces around IBM_WATSONX_URLS.
The options={IBM_WATSONX_URLS} syntax creates a set containing the list object itself, not a list of URL strings. This will cause the dropdown to malfunction. All other files in this PR correctly use options=IBM_WATSONX_URLS without braces.
Apply this diff:
- options={IBM_WATSONX_URLS},
+ options=IBM_WATSONX_URLS,📝 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.
| options={IBM_WATSONX_URLS}, | |
| options=IBM_WATSONX_URLS, |
🤖 Prompt for AI Agents
In src/lfx/src/lfx/components/ibm/watsonx_embeddings.py around line 37, the
dropdown options are set using options={IBM_WATSONX_URLS} which incorrectly
creates a set containing the list object; change it to options=IBM_WATSONX_URLS
so the dropdown receives the actual list of URL strings. Make this
single-character removal of the curly braces and ensure no other surrounding
syntax is altered.
14126c2 to
9a5e54d
Compare
clean up watsonx urls
Summary by CodeRabbit
Release Notes