fix: add ModelInput to listed components#11886
Merged
HimavarshaVS merged 32 commits intomainfrom Feb 27, 2026
Merged
Conversation
Contributor
|
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 Use the checkbox below for a quick retry:
WalkthroughReplaces the LLM input field with a provider-aware model selection system in the CSV Agent component. Adds IBM WatsonX-specific configuration fields (API key, endpoint URL, project ID) with dynamic visibility based on the selected model provider. Implements a new _get_llm method to resolve LLM instances and an update_build_config method to manage field visibility. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CSVAgentComponent
participant UpdateBuildConfig as update_build_config
participant ModelResolver as _get_llm
participant LLMProvider as LLM Provider<br/>(WatsonX/Others)
User->>CSVAgentComponent: Select model provider
CSVAgentComponent->>UpdateBuildConfig: Field visibility change triggered
UpdateBuildConfig->>UpdateBuildConfig: Determine if WatsonX selected
alt Provider is IBM WatsonX
UpdateBuildConfig->>CSVAgentComponent: Show api_key, base_url_ibm_watsonx, project_id
else Provider is Other
UpdateBuildConfig->>CSVAgentComponent: Hide WatsonX-specific fields
end
User->>CSVAgentComponent: Trigger build_agent
CSVAgentComponent->>ModelResolver: Call _get_llm()
ModelResolver->>LLMProvider: Initialize LLM with model, credentials, endpoint
LLMProvider-->>ModelResolver: Return LLM instance
ModelResolver-->>CSVAgentComponent: Return resolved LLM
CSVAgentComponent->>CSVAgentComponent: Create CSV agent with LLM
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 6❌ Failed checks (1 error, 3 warnings, 2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
bug
Contributor
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
github-actions
bot
added
lgtm
github-actions
bot
added
bug
github-actions
bot
added
bug
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11886 +/- ##
==========================================
+ Coverage 35.71% 35.76% +0.05%
==========================================
Files 1528 1528
Lines 73960 73960
Branches 11154 11154
==========================================
+ Hits 26417 26455 +38
+ Misses 46106 46067 -39
- Partials 1437 1438 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lfx/src/lfx/components/langchain_utilities/csv_agent.py (1)
194-228:⚠️ Potential issue | 🟡 Minor
build_agentleaks the temp file on error — add try/finally parity withbuild_agent_response.
build_agent_responsewraps its body in atry/finallyto ensure_cleanup_temp_file()runs.build_agenthas no such guard: if_get_llm()orcreate_csv_agent(...)raises, the temp file created by_get_local_path()is never deleted.🛡️ Proposed fix
# Get local path (downloads from S3 if needed) local_path = self._get_local_path() - llm = self._get_llm() - - agent_csv = create_csv_agent( - llm=llm, - path=local_path, - agent_type=self.agent_type, - handle_parsing_errors=self.handle_parsing_errors, - pandas_kwargs=self.pandas_kwargs, - **agent_kwargs, - ) - - self.status = Message(text=str(agent_csv)) - - # Note: Temp file will be cleaned up when the component is destroyed or - # when build_agent_response is called - return agent_csv + try: + llm = self._get_llm() + + agent_csv = create_csv_agent( + llm=llm, + path=local_path, + agent_type=self.agent_type, + handle_parsing_errors=self.handle_parsing_errors, + pandas_kwargs=self.pandas_kwargs, + **agent_kwargs, + ) + + self.status = Message(text=str(agent_csv)) + return agent_csv + except Exception: + self._cleanup_temp_file() + raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lfx/src/lfx/components/langchain_utilities/csv_agent.py` around lines 194 - 228, build_agent currently calls self._get_local_path() then _get_llm() and create_csv_agent(...) without ensuring cleanup on exceptions, which can leak the temp file; wrap the body after obtaining local_path in a try/finally and call self._cleanup_temp_file() in the finally block (mirroring build_agent_response) so that any exception from _get_llm or create_csv_agent still triggers cleanup; reference the build_agent method, self._get_local_path, self._get_llm, create_csv_agent, and self._cleanup_temp_file when applying the change.
🧹 Nitpick comments (3)
src/lfx/src/lfx/components/langchain_utilities/csv_agent.py (2)
128-129: Inner functionget_tool_calling_model_optionsis re-created on every call.The closure adds no value over a one-liner
lambdaor a direct call; hoisting the function or inlining it avoids the unnecessary per-call object creation.♻️ Simplified version
- def get_tool_calling_model_options(user_id=None): - return get_language_model_options(user_id=user_id, tool_calling=True) - build_config = update_model_options_in_build_config( component=self, build_config=dict(build_config), cache_key_prefix="language_model_options_tool_calling", - get_options_func=get_tool_calling_model_options, + get_options_func=lambda user_id=None: get_language_model_options(user_id=user_id, tool_calling=True), field_name=field_name, field_value=field_value, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lfx/src/lfx/components/langchain_utilities/csv_agent.py` around lines 128 - 129, The inner function get_tool_calling_model_options is recreated on every call; replace it with a direct call or a hoisted helper to avoid allocating a closure each time — e.g., remove the nested def and either call get_language_model_options(user_id=user_id, tool_calling=True) inline where used or define a top-level helper function named get_tool_calling_model_options that simply delegates to get_language_model_options(user_id, tool_calling=True); update all references to use that single function instead of creating a new closure.
115-123:getattris unnecessary for declared input fields.
api_key,base_url_ibm_watsonx, andproject_idare declared inputs, soself.api_keyetc. are always attributes. Usinggetattr(..., None)just obscures that fact and could silently swallow aNameErrorfrom a typo.♻️ Simplified version
def _get_llm(self): """Resolve the language model from dropdown selection or connected component.""" return get_llm( model=self.model, user_id=self.user_id, - api_key=getattr(self, "api_key", None), - watsonx_url=getattr(self, "base_url_ibm_watsonx", None), - watsonx_project_id=getattr(self, "project_id", None), + api_key=self.api_key, + watsonx_url=self.base_url_ibm_watsonx, + watsonx_project_id=self.project_id, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lfx/src/lfx/components/langchain_utilities/csv_agent.py` around lines 115 - 123, The _get_llm method uses getattr(self, "...", None) for inputs that are declared attributes (api_key, base_url_ibm_watsonx, project_id), which is unnecessary and can hide typos; update _get_llm to pass the direct attributes (self.api_key, self.base_url_ibm_watsonx, self.project_id) into get_llm so attribute access is explicit and errors from misspelled names surface immediately.src/lfx/src/lfx/_assets/component_index.json (1)
79655-79655: Duplicated agent creation logic betweenbuild_agent_responseandbuild_agent.Both methods repeat the same
ImportErrorguard,allow_dangerousguard,agent_kwargsdict,_get_local_path(),_get_llm(), andcreate_csv_agent(...)call. Extracting into a shared_create_agent() -> AgentExecutorhelper eliminates the duplication and makes the resource-leak fix (above) a single-site change.♻️ Proposed refactor
+ def _create_agent(self) -> AgentExecutor: + try: + from langchain_experimental.agents.agent_toolkits.csv.base import create_csv_agent + except ImportError as e: + msg = ( + "langchain-experimental is not installed. " + "Please install it with `pip install langchain-experimental`." + ) + raise ImportError(msg) from e + + allow_dangerous = getattr(self, "allow_dangerous_code", False) or False + agent_kwargs = { + "verbose": self.verbose, + "allow_dangerous_code": allow_dangerous, + } + local_path = self._get_local_path() + llm = self._get_llm() + return create_csv_agent( + llm=llm, + path=local_path, + agent_type=self.agent_type, + handle_parsing_errors=self.handle_parsing_errors, + pandas_kwargs=self.pandas_kwargs, + **agent_kwargs, + ), local_path # return local_path so callers can clean up + def build_agent_response(self) -> Message: + agent_csv, _ = self._create_agent() try: - from langchain_experimental.agents.agent_toolkits.csv.base import create_csv_agent - ... - local_path = self._get_local_path() - llm = self._get_llm() - agent_csv = create_csv_agent(...) result = agent_csv.invoke({"input": self.input_value}) return Message(text=str(result["output"])) finally: self._cleanup_temp_file() def build_agent(self) -> AgentExecutor: + agent_csv, _ = self._create_agent() try: - from langchain_experimental.agents.agent_toolkits.csv.base import create_csv_agent - ... - local_path = self._get_local_path() - llm = self._get_llm() - agent_csv = create_csv_agent(...) self.status = Message(text=str(agent_csv)) return agent_csv + finally: + self._cleanup_temp_file()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lfx/src/lfx/_assets/component_index.json` at line 79655, build_agent_response and build_agent duplicate the same agent-creation logic (ImportError guard, allow_dangerous handling, agent_kwargs, _get_local_path(), _get_llm(), and create_csv_agent call); extract this into a new private helper _create_agent(self) -> AgentExecutor that performs the import guard, resolves allow_dangerous, builds agent_kwargs, calls _get_local_path() and _get_llm(), and returns the created agent_csv from create_csv_agent, then have build_agent_response call _create_agent() and invoke it, and have build_agent call _create_agent() and set self.status and return the agent; ensure cleanup behavior still calls _cleanup_temp_file() from build_agent_response as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lfx/src/lfx/_assets/component_index.json`:
- Line 79655: update_build_config must enforce a WatsonX-safe agent_type: inside
the is_watsonx branch (in update_build_config) locate the build_config entries
for "agent_type" and, when is_watsonx is True, set
build_config["agent_type"]["value"] to "zero-shot-react-description" (or a
WatsonX-compatible default) and update build_config["agent_type"]["options"] to
remove or disable "openai-tools" and "openai-functions"; conversely, when
is_watsonx is False ensure the original options (including
"openai-tools"/"openai-functions") are present. Use the symbols
update_build_config, is_watsonx, and "agent_type" to find the code and ensure
create_csv_agent will not be called with an incompatible agent_type.
- Line 79655: The build_agent method leaks temp files because _get_local_path
may set self._temp_file_path for S3 downloads but build_agent lacks a finally
cleanup; update build_agent (the method that creates agent_csv) to mirror
build_agent_response by wrapping the core logic in try...finally and call
self._cleanup_temp_file() in the finally block so any temp file created by
_get_local_path is removed; reference build_agent, _get_local_path,
_cleanup_temp_file, and build_agent_response when making the change.
In `@src/lfx/src/lfx/components/langchain_utilities/csv_agent.py`:
- Around line 125-151: The type annotation for update_build_config's parameter
field_value is too narrow (currently str) given the runtime handling of list
values for model selections; update the signature of
update_build_config(field_value: ...) to accept the actual types emitted by
ModelInput (e.g., str | list[dict] or a more specific Sequence[Mapping] / list
of model choice dicts) and adjust any imports (typing.Sequence/Mapping) as
needed so the isinstance(current_model_value, list) branch works when field_name
== "model"; keep the rest of the function logic unchanged and ensure any
callers/tests use the widened type or are updated to match.
- Line 52: The code currently uses IBM_WATSONX_URLS[0] at class-definition time
which will raise IndexError if IBM_WATSONX_URLS is empty; change the value
expression to safely handle an empty list (e.g., use IBM_WATSONX_URLS[0] if
IBM_WATSONX_URLS else None or use next(iter(IBM_WATSONX_URLS), None)) so the
module can import even when the constant is empty; update the place where
value=IBM_WATSONX_URLS[0] is defined to use the safe expression and ensure any
downstream code handles the None/default appropriately.
---
Outside diff comments:
In `@src/lfx/src/lfx/components/langchain_utilities/csv_agent.py`:
- Around line 194-228: build_agent currently calls self._get_local_path() then
_get_llm() and create_csv_agent(...) without ensuring cleanup on exceptions,
which can leak the temp file; wrap the body after obtaining local_path in a
try/finally and call self._cleanup_temp_file() in the finally block (mirroring
build_agent_response) so that any exception from _get_llm or create_csv_agent
still triggers cleanup; reference the build_agent method, self._get_local_path,
self._get_llm, create_csv_agent, and self._cleanup_temp_file when applying the
change.
---
Nitpick comments:
In `@src/lfx/src/lfx/_assets/component_index.json`:
- Line 79655: build_agent_response and build_agent duplicate the same
agent-creation logic (ImportError guard, allow_dangerous handling, agent_kwargs,
_get_local_path(), _get_llm(), and create_csv_agent call); extract this into a
new private helper _create_agent(self) -> AgentExecutor that performs the import
guard, resolves allow_dangerous, builds agent_kwargs, calls _get_local_path()
and _get_llm(), and returns the created agent_csv from create_csv_agent, then
have build_agent_response call _create_agent() and invoke it, and have
build_agent call _create_agent() and set self.status and return the agent;
ensure cleanup behavior still calls _cleanup_temp_file() from
build_agent_response as before.
In `@src/lfx/src/lfx/components/langchain_utilities/csv_agent.py`:
- Around line 128-129: The inner function get_tool_calling_model_options is
recreated on every call; replace it with a direct call or a hoisted helper to
avoid allocating a closure each time — e.g., remove the nested def and either
call get_language_model_options(user_id=user_id, tool_calling=True) inline where
used or define a top-level helper function named get_tool_calling_model_options
that simply delegates to get_language_model_options(user_id, tool_calling=True);
update all references to use that single function instead of creating a new
closure.
- Around line 115-123: The _get_llm method uses getattr(self, "...", None) for
inputs that are declared attributes (api_key, base_url_ibm_watsonx, project_id),
which is unnecessary and can hide typos; update _get_llm to pass the direct
attributes (self.api_key, self.base_url_ibm_watsonx, self.project_id) into
get_llm so attribute access is explicit and errors from misspelled names surface
immediately.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lfx/src/lfx/_assets/component_index.jsonsrc/lfx/src/lfx/_assets/stable_hash_history.jsonsrc/lfx/src/lfx/components/langchain_utilities/csv_agent.py
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The CSV Agent and other components' Language Model field was a connection-only handle with no UI. It’s now a ModelInput so the UI shows a Language Model dropdown (and “Connect other models”) plus API key and Watsonx fields when needed, matching the main Agent component.
Components Updated:
Summary by CodeRabbit