fix: Composio base to handle dataframe response correctly#9427
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 ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
Bot
added
bug
![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 (2)
src/backend/base/langflow/base/composio/composio_base.py (2)
138-146: Defensive rename is good; add a small guard for non-pandas DataFrame typesRenaming a "data" column to avoid attribute collisions is sensible. To harden this, also guard on the presence of a rename method so custom DataFrame implementations without pandas-like API don’t trigger the try/except path unnecessarily.
- if hasattr(result_dataframe, "columns"): + if hasattr(result_dataframe, "columns") and hasattr(result_dataframe, "rename"): try: if "data" in result_dataframe.columns: result_dataframe = result_dataframe.rename(columns={"data": "_data"}) except (AttributeError, TypeError, ValueError, KeyError) as e: logger.debug(f"Failed to rename 'data' column: {e}")
1261-1262: Good: maps aliased '_status' back to API 'status'This closes the loop between UI/schema aliasing and API execution. To keep requiredness logic consistent during execution, also remap the required_fields set to aliased names when comparing against param_fields:
# Outside the selected lines (~lines 1225-1227), after computing required_fields renamed_required_fields = set() for name in required_fields: if name == "user_id": renamed_required_fields.add(f"{self.app_name}_user_id") elif name == "status": renamed_required_fields.add(f"{self.app_name}_status") else: renamed_required_fields.add(name) required_fields = renamed_required_fieldsOptionally, replace the startswith/endswith pattern with exact equality for clarity:
- if field.endswith("_user_id") and field.startswith(self.app_name): + if field == f"{self.app_name}_user_id": final_field_name = "user_id" - elif field.endswith("_status") and field.startswith(self.app_name): + elif field == f"{self.app_name}_status": final_field_name = "status"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/backend/base/langflow/base/composio/composio_base.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{src/backend/**/*.py,tests/**/*.py,Makefile}
📄 CodeRabbit Inference Engine (.cursor/rules/backend_development.mdc)
{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code
Files:
src/backend/base/langflow/base/composio/composio_base.py
🔇 Additional comments (1)
src/backend/base/langflow/base/composio/composio_base.py (1)
390-392: Ensure required‐fields list uses the aliased “status” fieldIn src/backend/base/langflow/base/composio/composio_base.py (around lines 390–392), you’ve aliased:
elif clean_field == "status": clean_field = f"{self.app_name}_status"To keep a field marked required in the UI, you must also remap any occurrences of
"status"in your required‐fields list tof"{self.app_name}_status"during schema preparation and execution. Otherwise the UI may drop “status” from its required checks.• Review your schema‐processing block and confirm that every entry in the required list referencing
"status"is replaced with the aliased name.
• If you don’t yet have that mapping, please add it so that required “status” remains required.Let me know if you’d like me to draft a patch for remapping the required fields.
| # Update | ||
| field_schema_copy = field_schema.copy() | ||
| field_schema_copy["description"] = ( | ||
| f"User ID for {self.app_name.title()}: " + field_schema["description"] | ||
| ) | ||
| elif clean_field_name == "status": | ||
| clean_field_name = f"{self.app_name}_status" | ||
| # Update | ||
| field_schema_copy = field_schema.copy() | ||
| field_schema_copy["description"] = ( | ||
| f"Status for {self.app_name.title()}: " + field_schema["description"] | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential KeyError when building descriptions for renamed fields
Both the "user_id" and "status" branches access field_schema["description"] unconditionally. If the source schema lacks a description (common), this will raise a KeyError and abort input generation.
Apply this diff to make the description handling safe:
- field_schema_copy = field_schema.copy()
- field_schema_copy["description"] = (
- f"User ID for {self.app_name.title()}: " + field_schema["description"]
- )
+ field_schema_copy = field_schema.copy()
+ base_desc = field_schema.get("description")
+ field_schema_copy["description"] = (
+ f"User ID for {self.app_name.title()}: {base_desc}"
+ if base_desc
+ else f"User ID for {self.app_name.title()}"
+ )
@@
- field_schema_copy = field_schema.copy()
- field_schema_copy["description"] = (
- f"Status for {self.app_name.title()}: " + field_schema["description"]
- )
+ field_schema_copy = field_schema.copy()
+ base_desc = field_schema.get("description")
+ field_schema_copy["description"] = (
+ f"Status for {self.app_name.title()}: {base_desc}"
+ if base_desc
+ else f"Status for {self.app_name.title()}"
+ )Additionally, ensure the "required" list reflects the same renames to keep requiredness in sync with the aliased field names:
# Outside the selected lines, right after cleaned_required is computed (~lines 572-575)
if flat_schema.get("required"):
cleaned_required = [field.replace("[0]", "") for field in flat_schema["required"]]
# Reflect field renames (e.g., 'user_id' -> '<app>_user_id', 'status' -> '<app>_status')
mapped_required = []
for field in cleaned_required:
if field == "user_id":
mapped_required.append(f"{self.app_name}_user_id")
elif field == "status":
mapped_required.append(f"{self.app_name}_status")
else:
mapped_required.append(field)
flat_schema["required"] = mapped_requiredThis avoids mislabeling required inputs as optional in the UI.
🤖 Prompt for AI Agents
In src/backend/base/langflow/base/composio/composio_base.py around lines 539 to
550, the code unconditionally accesses field_schema["description"] when renaming
"user_id" and "status", which can raise KeyError if description is missing;
change those accesses to use field_schema.get("description", "") (or check for
presence) before concatenating so description building is safe. Also, after
cleaned_required is computed (around lines ~572-575), map renamed fields into
the required list so requiredness stays in sync: iterate cleaned_required,
replace "user_id" with f"{self.app_name}_user_id" and "status" with
f"{self.app_name}_status", and assign the resulting list back to
flat_schema["required"].
github-actions
Bot
added
bug
github-actions
Bot
added
bug
|
* fix: df handling * fix: format --------- Co-authored-by: Edwin Jose <edwin.jose@datastax.com>
* fix: df handling * fix: format --------- Co-authored-by: Edwin Jose <edwin.jose@datastax.com>
2 participants
Fixed output Dataframe handling in
/src/backend/base/langflow/base/composio/composio_base.pySummary by CodeRabbit
New Features
Bug Fixes