fix(langflow): resolve #13231 by correcting Composio tool schema type normalization#13308
Open
gingeekrishna wants to merge 3 commits into
Open
fix(langflow): resolve #13231 by correcting Composio tool schema type normalization#13308gingeekrishna wants to merge 3 commits into
gingeekrishna wants to merge 3 commits into
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds JSON Schema sanitization utilities for Composio tools to repair missing ChangesComposio Tool Argument Schema Sanitization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (7 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR introduces a sanitization step for Composio tool argument JSON schemas to prevent downstream tool-calling failures when schemas are missing type declarations.
Changes:
- Add
sanitize_tool_args_schemautility to normalize/repair toolargsandargs_schemaJSON schemas. - Apply schema sanitization when selecting/configuring Composio tools in both the component API and base wrapper.
- Add unit tests validating schema repair behavior and no-op behavior for already-valid schemas.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/lfx/src/lfx/components/composio/composio_api.py | Sanitizes selected Composio tools’ args schemas after filtering actions. |
| src/lfx/src/lfx/base/composio/schema_utils.py | Adds schema inference + recursive sanitizer and a public sanitize_tool_args_schema helper. |
| src/lfx/src/lfx/base/composio/composio_base.py | Sanitizes each tool’s args schema during configure_tools. |
| src/backend/tests/unit/components/bundles/composio/test_schema_utils.py | Adds unit tests for schema sanitization behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
Bot
added
bug
![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: 2
🧹 Nitpick comments (3)
src/lfx/src/lfx/components/composio/composio_api.py (1)
260-263: ⚡ Quick winInclude exception details when sanitization is skipped.
The current log drops the error context, which makes root-cause analysis harder.
Suggested fix
- for tool in selected_tools: - try: - sanitize_tool_args_schema(tool) - except (TypeError, ValueError, AttributeError): - self.log(f"Skipping args schema sanitization for tool: {getattr(tool, 'name', 'unknown')}") + for tool in selected_tools: + try: + sanitize_tool_args_schema(tool) + except (TypeError, ValueError, AttributeError) as e: + self.log( + f"Skipping args schema sanitization for tool: {getattr(tool, 'name', 'unknown')} ({e})" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lfx/src/lfx/components/composio/composio_api.py` around lines 260 - 263, The log inside the except block for sanitize_tool_args_schema(tool) currently omits the exception details; update the except handling for (TypeError, ValueError, AttributeError) to capture the exception (e) and include its message/representation in the self.log call so the log reads something like "Skipping args schema sanitization for tool: <name> - <exception>" using getattr(tool, 'name', 'unknown') and the caught exception to aid debugging.src/lfx/src/lfx/base/composio/composio_base.py (1)
2364-2365: ⚡ Quick winAvoid silent suppression of sanitization failures.
This currently drops all sanitization exceptions with no signal. Logging at debug level here would make schema issues diagnosable without breaking tool configuration.
Suggested fix
- with suppress(ValueError, TypeError, AttributeError): - sanitize_tool_args_schema(tool) + try: + sanitize_tool_args_schema(tool) + except (ValueError, TypeError, AttributeError) as e: + logger.debug("Skipping args schema sanitization for tool '%s': %s", getattr(tool, "name", "unknown"), e)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lfx/src/lfx/base/composio/composio_base.py` around lines 2364 - 2365, Replace the silent suppression around sanitize_tool_args_schema(tool) (currently using suppress(ValueError, TypeError, AttributeError)) with an explicit try/except that catches ValueError, TypeError and AttributeError and logs the exception at debug level (including exc_info/stacktrace) along with a brief context (e.g., the tool identifier) before continuing; keep the same non-fatal behavior so configuration proceeds after logging.src/backend/tests/unit/components/bundles/composio/test_schema_utils.py (1)
39-72: ⚡ Quick winAdd one malformed-schema test case for the error path.
Current tests validate positive/no-op behavior, but they don’t assert behavior when
args_schema.model_json_schema()returns invalid data (e.g., non-dict). A small negative test here would lock in the best-effort contract.As per coding guidelines "
**/test_*.py: Consider including edge cases and error conditions for comprehensive test coverage" and "Verify tests cover both positive and negative scenarios where appropriate."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/tests/unit/components/bundles/composio/test_schema_utils.py` around lines 39 - 72, Add a negative test that ensures sanitize_tool_args_schema handles a malformed args_schema.model_json_schema() return (non-dict) without raising and treats it as a no-op: create a SimpleNamespace tool with args (e.g., subject/type string) and an args_schema object whose model_json_schema() returns an invalid type (like None or a string), call sanitize_tool_args_schema(tool), assert it returns False, and assert tool.args remains unchanged; reference sanitize_tool_args_schema, tool.args, and args_schema.model_json_schema in the test (suggested name: test_sanitize_tool_args_schema_handles_malformed_schema).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lfx/src/lfx/base/composio/schema_utils.py`:
- Around line 56-59: Ruff flags nested ifs in _sanitize_schema_in_place where
you check isinstance(properties, dict) then iterate and nest
isinstance(prop_schema, dict) and another conditional; refactor each nested
conditional (the blocks around properties.values(), the prop_schema dict check,
and the inner conditional at/near lines 56-59, 62-65, 68-69) into single
combined conditionals (e.g., combine isinstance checks and the inner predicate
with and/or short-circuiting) or return early to avoid nesting; update the logic
in the _sanitize_schema_in_place function so each branch uses a flat if with
combined conditions or early continue/return while preserving existing behavior
and references to properties and prop_schema.
- Around line 44-47: The code currently overwrites schema["type"] whenever
schema_type is not a str; change this to only overwrite when schema_type is
neither a str nor a valid list/tuple of str (e.g., preserve unions like
["string","null"]). In the block around schema_type and _infer_schema_type,
check isinstance(schema_type, (list, tuple)) and validate all elements are str
before leaving it alone; only call _infer_schema_type and set schema["type"] =
_infer_schema_type(schema) (and changed = True) when schema_type is invalid (not
a str and not a list/tuple of str).
---
Nitpick comments:
In `@src/backend/tests/unit/components/bundles/composio/test_schema_utils.py`:
- Around line 39-72: Add a negative test that ensures sanitize_tool_args_schema
handles a malformed args_schema.model_json_schema() return (non-dict) without
raising and treats it as a no-op: create a SimpleNamespace tool with args (e.g.,
subject/type string) and an args_schema object whose model_json_schema() returns
an invalid type (like None or a string), call sanitize_tool_args_schema(tool),
assert it returns False, and assert tool.args remains unchanged; reference
sanitize_tool_args_schema, tool.args, and args_schema.model_json_schema in the
test (suggested name: test_sanitize_tool_args_schema_handles_malformed_schema).
In `@src/lfx/src/lfx/base/composio/composio_base.py`:
- Around line 2364-2365: Replace the silent suppression around
sanitize_tool_args_schema(tool) (currently using suppress(ValueError, TypeError,
AttributeError)) with an explicit try/except that catches ValueError, TypeError
and AttributeError and logs the exception at debug level (including
exc_info/stacktrace) along with a brief context (e.g., the tool identifier)
before continuing; keep the same non-fatal behavior so configuration proceeds
after logging.
In `@src/lfx/src/lfx/components/composio/composio_api.py`:
- Around line 260-263: The log inside the except block for
sanitize_tool_args_schema(tool) currently omits the exception details; update
the except handling for (TypeError, ValueError, AttributeError) to capture the
exception (e) and include its message/representation in the self.log call so the
log reads something like "Skipping args schema sanitization for tool: <name> -
<exception>" using getattr(tool, 'name', 'unknown') and the caught exception to
aid debugging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dc4b9989-3a0a-4cad-98e5-647fa21ea063
📒 Files selected for processing (4)
src/backend/tests/unit/components/bundles/composio/test_schema_utils.pysrc/lfx/src/lfx/base/composio/composio_base.pysrc/lfx/src/lfx/base/composio/schema_utils.pysrc/lfx/src/lfx/components/composio/composio_api.py
github-actions
Bot
added
bug
gingeekrishna
changed the title
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
Comment on lines
+24
to
+39
| for union_key in ("anyOf", "oneOf", "allOf"): | ||
| variants = schema.get(union_key) | ||
| if isinstance(variants, list): | ||
| non_null_type: str | None = None | ||
| has_null_variant = False | ||
| for variant in variants: | ||
| if not isinstance(variant, dict): | ||
| continue | ||
| variant_type = variant.get("type") | ||
| if isinstance(variant_type, str): | ||
| if variant_type == "null": | ||
| has_null_variant = True | ||
| elif non_null_type is None: | ||
| non_null_type = variant_type | ||
| if non_null_type is not None: | ||
| return [non_null_type, "null"] if has_null_variant else non_null_type |
Comment on lines
+56
to
+62
| def _needs_schema_sanitization(schema: dict[str, Any]) -> bool: | ||
| """Check if a JSON schema needs sanitization (read-only traversal).""" | ||
| if "type" not in schema or schema["type"] is None: | ||
| return True | ||
|
|
||
| if schema.get("type") == "object" and "properties" in schema and schema["properties"] is None: | ||
| return True |
| }, | ||
| args_schema=None, | ||
| ) | ||
|
|
github-actions
Bot
added
bug
Comment on lines
+59
to
+69
| enum_values = schema.get("enum") | ||
| if isinstance(enum_values, list) and enum_values: | ||
| first = enum_values[0] | ||
| if isinstance(first, bool): | ||
| return "boolean" | ||
| if isinstance(first, int): | ||
| return "integer" | ||
| if isinstance(first, float): | ||
| return "number" | ||
| if isinstance(first, str): | ||
| return "string" |
Comment on lines
+26
to
+59
| for union_key in ("anyOf", "oneOf"): | ||
| variants = schema.get(union_key) | ||
| if isinstance(variants, list): | ||
| concrete_types: list[str] = [] | ||
| has_null_variant = False | ||
| for variant in variants: | ||
| if not isinstance(variant, dict): | ||
| continue | ||
| variant_type = variant.get("type") | ||
| if isinstance(variant_type, str): | ||
| if variant_type == "null": | ||
| has_null_variant = True | ||
| elif variant_type not in concrete_types: | ||
| concrete_types.append(variant_type) | ||
| elif isinstance(variant_type, list): | ||
| for t in variant_type: | ||
| if isinstance(t, str): | ||
| if t == "null": | ||
| has_null_variant = True | ||
| elif t not in concrete_types: | ||
| concrete_types.append(t) | ||
| else: | ||
| # No explicit type — infer from the variant's structure so that, | ||
| # e.g., enum-only variants like {"enum": ["a","b"]} are not | ||
| # silently treated as missing a concrete type. | ||
| inferred = _infer_schema_type(variant) | ||
| for t in [inferred] if isinstance(inferred, str) else inferred: | ||
| if t == "null": | ||
| has_null_variant = True | ||
| elif t not in concrete_types: | ||
| concrete_types.append(t) | ||
| if concrete_types: | ||
| result: list[str] = concrete_types + (["null"] if has_null_variant else []) | ||
| return result[0] if len(result) == 1 else result |
Comment on lines
+215
to
+224
| raw_schema = args_schema.model_json_schema() | ||
| if not isinstance(raw_schema, dict): | ||
| return changed | ||
|
|
||
| schema_copy = deepcopy(raw_schema) | ||
| schema_changed = _sanitize_schema_in_place(schema_copy) | ||
| if schema_changed: | ||
| tool.args_schema = create_input_schema_from_json_schema(schema_copy) | ||
| changed = True | ||
| return changed |
Comment on lines
+357
to
+359
| echo "npx playwright test ${{ inputs.tests_folder }} ${{ needs.determine-test-suite.outputs.test_grep }} --trace on --shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --workers 2 --pass-with-no-tests --retries=3" | ||
|
|
||
| npx playwright test ${{ inputs.tests_folder }} ${{ needs.determine-test-suite.outputs.test_grep }} --trace on --shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --workers 2 --retries=3 | ||
| npx playwright test ${{ inputs.tests_folder }} ${{ needs.determine-test-suite.outputs.test_grep }} --trace on --shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --workers 2 --pass-with-no-tests --retries=3 |
Comment on lines
+2357
to
+2361
| tools = list(composio.tools.get(user_id=self.entity_id, toolkits=[self.app_name.lower()], limit=limit)) | ||
| sanitized_tools, excluded_summary = sanitize_tools_with_fallback(tools, logger.debug) | ||
| failure_count = len(tools) - len(sanitized_tools) | ||
| configured_tools = [] | ||
| for tool in tools: | ||
| for tool in sanitized_tools: |
Comment on lines
+2370
to
+2375
| if failure_count > 0: | ||
| logger.warning( | ||
| "Excluded %d tool(s) due to args schema sanitization failures: %s", | ||
| failure_count, | ||
| excluded_summary, | ||
| ) |
| # Join tags with | for OR logic | ||
| REGEX_PATTERN=$(IFS='|'; echo "${TAGS[*]}") | ||
| TEST_GREP='--grep="${REGEX_PATTERN}"' | ||
| TEST_GREP="--grep \"${REGEX_PATTERN}\"" |
Comment on lines
+356
to
+364
| PASS_WITH_NO_TESTS="" | ||
| if [ "${{ matrix.shardTotal }}" -gt 1 ]; then | ||
| PASS_WITH_NO_TESTS="--pass-with-no-tests" | ||
| fi | ||
|
|
||
| # Echo command before running for easier debugging. | ||
| echo "npx playwright test ${{ inputs.tests_folder }} ${{ needs.determine-test-suite.outputs.test_grep }} --trace on --shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --workers 2 ${PASS_WITH_NO_TESTS} --retries=3" | ||
|
|
||
| npx playwright test ${{ inputs.tests_folder }} ${{ needs.determine-test-suite.outputs.test_grep }} --trace on --shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --workers 2 --retries=3 | ||
| npx playwright test ${{ inputs.tests_folder }} ${{ needs.determine-test-suite.outputs.test_grep }} --trace on --shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }} --workers 2 ${PASS_WITH_NO_TESTS} --retries=3 |
Comment on lines
+164
to
+172
| properties = schema.get("properties") | ||
| if isinstance(properties, dict): | ||
| for prop_schema in properties.values(): | ||
| if isinstance(prop_schema, dict) and _needs_schema_sanitization(prop_schema): | ||
| return True | ||
|
|
||
| items = schema.get("items") | ||
| if isinstance(items, dict) and _needs_schema_sanitization(items): | ||
| return True |
Contributor
Author
CI Failure Note:
|
…schema type normalization Adds schema_utils.py with sanitize_tool_args_schema / sanitize_tools_with_fallback helpers that infer missing JSON Schema 'type' fields for Composio tool arguments. Tools that fail sanitization are excluded and logged rather than crashing tool loading.
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
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.
… normalization
Summary by CodeRabbit
Bug Fixes
Tests