feat: Add LanguageModelMixin and update LanguageModelComponent to use it#10858
feat: Add LanguageModelMixin and update LanguageModelComponent to use it#10858ogabrielluiz wants to merge 3 commits into
Conversation
WalkthroughThis PR extracts language model provider handling logic (input generation, model fetching, LLM instantiation, dynamic configuration) from LanguageModelComponent into a new reusable LanguageModelMixin, consolidating multi-provider support (OpenAI, Anthropic, Google, IBM watsonx.ai, Ollama) while maintaining functional equivalence. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 3 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/lfx/src/lfx/base/models/language_model_mixin.py (2)
85-98: Synchronous HTTP call may block when invoked from async context.
fetch_ibm_modelsuses synchronousrequests.get, but it's called from the asyncupdate_llm_provider_configmethod (line 454). While the fallback behavior is resilient, the blocking call could delay other async tasks.Consider using
httpx.AsyncClient(already used for Ollama inmodel_utils.py) or wrapping inasyncio.to_threadfor consistency with the async pattern.
298-299: RedundantSecretStrwrapping and unwrapping.The expression
SecretStr(api_key).get_secret_value()wrapsapi_keyin aSecretStronly to immediately extract the secret value. Sinceapi_keyis already a string at this point, you can pass it directly:return ChatWatsonx( - apikey=SecretStr(api_key).get_secret_value(), + apikey=api_key, url=base_url_ibm,src/lfx/tests/unit/base/models/test_language_model_mixin.py (1)
96-139: Consider consolidating import-check helpers.The five
_has_langchain_*functions follow an identical pattern. While the current approach is explicit, you could reduce duplication:def _has_package(package_name: str) -> bool: import importlib try: importlib.import_module(package_name) return True except ImportError: return False # Usage: pytest.mark.skipif(not _has_package("langchain_openai"), ...)This is a minor nit—the current explicit approach works fine.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lfx/src/lfx/base/models/__init__.py(1 hunks)src/lfx/src/lfx/base/models/language_model_mixin.py(1 hunks)src/lfx/src/lfx/components/models_and_agents/language_model.py(2 hunks)src/lfx/tests/unit/base/models/test_language_model_mixin.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/test_*.py
📄 CodeRabbit inference engine (Custom checks)
**/test_*.py: Review test files for excessive use of mocks that may indicate poor test design - check if tests have too many mock objects that obscure what's actually being tested
Warn when mocks are used instead of testing real behavior and interactions, and suggest using real objects or test doubles when mocks become excessive
Ensure mocks are used appropriately for external dependencies only, not for core logic
Backend test files should follow the naming convention test_*.py with proper pytest structure
Test files should have descriptive test function names that explain what is being tested
Tests should be organized logically with proper setup and teardown
Consider including edge cases and error conditions for comprehensive test coverage
Verify tests cover both positive and negative scenarios where appropriate
For async functions in backend tests, ensure proper async testing patterns are used with pytest
For API endpoints, verify both success and error response testing
Files:
src/lfx/tests/unit/base/models/test_language_model_mixin.py
🧠 Learnings (5)
📚 Learning: 2025-11-24T19:46:09.104Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/backend_development.mdc:0-0
Timestamp: 2025-11-24T19:46:09.104Z
Learning: Applies to src/backend/base/langflow/components/**/__init__.py : Update `__init__.py` with alphabetically sorted imports when adding new components
Applied to files:
src/lfx/src/lfx/base/models/__init__.py
📚 Learning: 2025-11-24T19:46:09.104Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/backend_development.mdc:0-0
Timestamp: 2025-11-24T19:46:09.104Z
Learning: Applies to src/backend/base/langflow/components/**/*.py : Add new components to the appropriate subdirectory under `src/backend/base/langflow/components/` (agents/, data/, embeddings/, input_output/, models/, processing/, prompts/, tools/, or vectorstores/)
Applied to files:
src/lfx/src/lfx/components/models_and_agents/language_model.pysrc/lfx/tests/unit/base/models/test_language_model_mixin.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Use `pytest.mark.api_key_required` and `pytest.mark.no_blockbuster` markers for components that need external APIs; use `MockLanguageModel` from `tests.unit.mock_language_model` for testing without external API keys
Applied to files:
src/lfx/tests/unit/base/models/test_language_model_mixin.py
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Test component versioning and backward compatibility using `file_names_mapping` fixture with `VersionComponentMapping` objects mapping component files across Langflow versions
Applied to files:
src/lfx/tests/unit/base/models/test_language_model_mixin.py
📚 Learning: 2025-08-05T22:51:27.961Z
Learnt from: edwinjosechittilappilly
Repo: langflow-ai/langflow PR: 0
File: :0-0
Timestamp: 2025-08-05T22:51:27.961Z
Learning: The TestComposioComponentAuth test in src/backend/tests/unit/components/bundles/composio/test_base_composio.py demonstrates proper integration testing patterns for external API components, including real API calls with mocking for OAuth completion, comprehensive resource cleanup, and proper environment variable handling with pytest.skip() fallbacks.
Applied to files:
src/lfx/tests/unit/base/models/test_language_model_mixin.py
🧬 Code graph analysis (3)
src/lfx/src/lfx/base/models/__init__.py (1)
src/lfx/src/lfx/base/models/language_model_mixin.py (1)
LanguageModelMixin(76-519)
src/lfx/src/lfx/base/models/language_model_mixin.py (2)
src/lfx/src/lfx/base/models/model_utils.py (1)
get_ollama_models(39-108)src/lfx/src/lfx/utils/util.py (1)
transform_localhost_url(119-161)
src/lfx/tests/unit/base/models/test_language_model_mixin.py (1)
src/lfx/src/lfx/components/models_and_agents/language_model.py (1)
LanguageModelComponent(10-44)
⏰ 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). (2)
- GitHub Check: Update Starter Projects
- GitHub Check: Run Ruff Check and Format
🔇 Additional comments (9)
src/lfx/src/lfx/base/models/__init__.py (1)
1-4: LGTM!Imports are alphabetically sorted and the new
LanguageModelMixinis properly exported alongsideLCModelComponent.src/lfx/src/lfx/base/models/language_model_mixin.py (4)
1-46: Well-documented module with clear usage examples.The docstring provides helpful guidance on how to use the mixin, including code examples for input composition and update_build_config integration.
47-74: LGTM!Constants are well-organized and properly grouped by provider.
100-218: LGTM!Clean API design with keyword-only arguments and sensible defaults. The conditional inclusion of optional inputs provides good flexibility for different use cases.
343-518: LGTM!The dynamic configuration logic handles provider switching comprehensively with appropriate fallback behavior when model fetching fails. The o1 model system_message visibility toggle is a nice touch for OpenAI's reasoning model constraints.
src/lfx/src/lfx/components/models_and_agents/language_model.py (1)
10-44: LGTM!Clean refactoring that delegates provider logic to the mixin while maintaining explicit control over the
input_valueandsystem_messagefield placement. The separation of concerns is well-executed.src/lfx/tests/unit/base/models/test_language_model_mixin.py (3)
15-94: LGTM!Tests thoroughly validate input configuration defaults and optional field inclusion without relying on mocks.
141-261: LGTM!Comprehensive coverage of validation error cases for each provider. The use of
pytest.mark.skipiffor optional dependencies follows best practices from the testing guidelines.
375-385: LGTM!Good integration test verifying input ordering expectations for the component.
| # Check that build_model delegates to build_llm | ||
| assert comp.build_model == comp.build_llm or callable(comp.build_model) |
There was a problem hiding this comment.
Assertion does not verify delegation.
The condition comp.build_model == comp.build_llm or callable(comp.build_model) is always true because any method is callable. This doesn't actually verify that build_model delegates to build_llm.
Consider testing the actual delegation behavior:
# Check that build_model delegates to build_llm
- assert comp.build_model == comp.build_llm or callable(comp.build_model)
+ # Verify they're both methods (minimal check since actual call requires API keys)
+ assert callable(comp.build_model)
+ assert callable(comp.build_llm)
+ # The implementation of build_model should call build_llm
+ # This is verified by code inspection: build_model returns self.build_llm()Alternatively, you could use unittest.mock.patch to verify the call if stronger verification is needed.
📝 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.
| # Check that build_model delegates to build_llm | |
| assert comp.build_model == comp.build_llm or callable(comp.build_model) | |
| # Check that build_model delegates to build_llm | |
| # Verify they're both methods (minimal check since actual call requires API keys) | |
| assert callable(comp.build_model) | |
| assert callable(comp.build_llm) | |
| # The implementation of build_model should call build_llm | |
| # This is verified by code inspection: build_model returns self.build_llm() |
🤖 Prompt for AI Agents
In src/lfx/tests/unit/base/models/test_language_model_mixin.py around lines
365-366, the assertion `comp.build_model == comp.build_llm or
callable(comp.build_model)` is ineffective because methods are always callable
and doesn't verify delegation; replace it with a test that patches/mocks
comp.build_llm (e.g., with unittest.mock.patch.object or a monkeypatch) to
assert that calling comp.build_model(...) invokes comp.build_llm(...) with the
same arguments and returns its result (or alternatively directly assert
comp.build_model is comp.build_llm when delegation is implemented by aliasing);
ensure the mock verifies the call count and arguments to confirm actual
delegation.
1 participant
Introduce LanguageModelMixin to facilitate dynamic integration of LLM providers, simplify the LanguageModelComponent by removing unused code, and add unit tests to ensure functionality.
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.