Fix/google agent models #10597
Fix/google agent models #10597
Conversation
WalkthroughThe Google Generative AI component's default model selection was updated to dynamically reference the first available model from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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, 2 warnings, 1 inconclusive)
✅ 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 |
![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
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/google/google_generative_ai.py (1)
112-125: Fix pre-existing bugs in model filtering logic.While not introduced by this PR, there are critical bugs in the tool model filtering that could result in an empty
idslist (causing the IndexError flagged in line 140):
- Line 120: Uses
self.model_nameinstead of the loop variablemodel, causing the same model to be checked repeatedly instead of each model in the list.- Line 124: Modifying
model_idsduring iteration can skip elements and produce incorrect results.Apply this diff to fix both issues:
if tool_model_enabled: try: from langchain_google_genai.chat_models import ChatGoogleGenerativeAI except ImportError as e: msg = "langchain_google_genai is not installed." raise ImportError(msg) from e - for model in model_ids: + filtered_ids = [] + for model in model_ids: model_with_tool = ChatGoogleGenerativeAI( - model=self.model_name, + model=model, google_api_key=self.api_key, ) - if not self.supports_tool_calling(model_with_tool): - model_ids.remove(model) - return model_ids + if self.supports_tool_calling(model_with_tool): + filtered_ids.append(model) + return filtered_ids + return model_ids
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lfx/src/lfx/components/google/google_generative_ai.py(2 hunks)
🔇 Additional comments (1)
src/lfx/src/lfx/components/google/google_generative_ai.py (1)
27-35: ****The original concern about
IndexErroron line 32 is not valid.GOOGLE_GENERATIVE_AI_MODELSis derived from a statically initialized list (GOOGLE_GENERATIVE_AI_MODELS_DETAILED) with hardcoded model entries, guaranteeing it is never empty at module initialization. Additionally, even in theupdate_build_configmethod (line 140),ids[0]is protected: it references eitherGOOGLE_GENERATIVE_AI_MODELS(non-empty) or falls back to it on exception, making it safe.Likely an incorrect or invalid review comment.
| build_config.setdefault("model_name", {}) | ||
| build_config["model_name"]["options"] = ids | ||
| build_config["model_name"].setdefault("value", ids[0]) | ||
| build_config["model_name"]["value"] = ids[0] |
There was a problem hiding this comment.
Add bounds checking before accessing ids[0].
Direct assignment is correct for ensuring the value is always updated (fixing the setdefault issue). However, accessing ids[0] without verifying the list is non-empty will cause an IndexError if:
GOOGLE_GENERATIVE_AI_MODELSis empty- The API returns no models with
"generateContent"support - All models are filtered out when
tool_model_enabledisTrue
Apply this diff to add bounds checking:
build_config.setdefault("model_name", {})
build_config["model_name"]["options"] = ids
- build_config["model_name"]["value"] = ids[0]
+ if ids:
+ build_config["model_name"]["value"] = ids[0]📝 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.
| build_config.setdefault("model_name", {}) | |
| build_config["model_name"]["options"] = ids | |
| build_config["model_name"].setdefault("value", ids[0]) | |
| build_config["model_name"]["value"] = ids[0] | |
| build_config.setdefault("model_name", {}) | |
| build_config["model_name"]["options"] = ids | |
| if ids: | |
| build_config["model_name"]["value"] = ids[0] |
🤖 Prompt for AI Agents
In src/lfx/src/lfx/components/google/google_generative_ai.py around lines 138 to
140, the code assigns build_config["model_name"]["value"] = ids[0] without
checking ids length which can raise IndexError; update the code to check that
ids is a non-empty list before accessing ids[0] (e.g., if ids: set value to
ids[0]; else set value to None or an empty string or omit the "value" key) and
ensure build_config["model_name"]["options"] still receives the ids list so
downstream logic can handle the empty case.
1 participant
🔧 Summary
This PR fixes an issue in the Google Generative AI Agent component where the
model_namefield was not properly updated in the UI or in the internal component state.This PR relates to issue #10572.
There were two underlying problems:
"gemini-1.5-pro") was still being assigned, causing mismatches whenever the available model list changed.setdefault()does not override existing values, resulting in stale defaults persisting in the UI.✅ What has been changed
✔ Removed hard-coded
"gemini-1.5-pro"The default model is now dynamically assigned using the first entry from the model list in the
DropdownInput, following the same pattern used by all other providers in Langflow.✔ Removed
.setdefault("value", ids[0])and replaced it with an explicit assignment to ensure the value is always updatedSummary by CodeRabbit