fix(py): handle list-valued JSON Schema type in tool signatures#3754
fix(py): handle list-valued JSON Schema type in tool signatures#3754Anas Khan (anxkhn) wants to merge 1 commit into
Conversation
|
Anas Khan (@anxkhn) is attempting to deploy a commit to the Composio Team on Vercel. A member of the Team first needs to authorize it. |
eeshsaxena
left a comment
There was a problem hiding this comment.
This looks correct to me, and the placement is the important part.
The crux is that the new branch sits before elif param_type in PYDANTIC_TYPE_TO_PYTHON_TYPE: in the chain. That membership test hashes param_type as a dict key, and a list is unhashable, so a {"type": ["string", "null"]} schema would raise TypeError: unhashable type: 'list' there. Catching isinstance(param_type, list) first avoids that:
>>> ["string", "null"] in {"string": str}
TypeError: cannot use 'list' as a dict key (unhashable type: 'list')Worth noting it also sidesteps the same hazard in the final else branch, which does FALLBACK_VALUES[param_type] - that would blow up on a list too, and the new elif keeps lists from ever reaching it.
The mapping mirrors the existing anyOf/oneOf handling (PYDANTIC_TYPE_TO_PYTHON_TYPE.get(ptype, t.Any) then reduce into a Union, using param_schema.get("default", "") for the default), so it's consistent with the surrounding code. I reproduced the mapping and it matches the tests: ["string","null"] -> str | None, ["integer"] -> int, ["string","integer"] -> str | int, ["file","null"] -> Any | None. Nice coverage on all four cases including the unrecognized-member fallback.
Deferring to the maintainers for the final review.
JSON Schema Draft 2020-12 and OpenAPI 3.1 express a nullable field as a
list of types (e.g. {"type": ["string", "null"]}) rather than an anyOf.
In get_signature_format_from_schema_params a list is unhashable, so the
"param_type in PYDANTIC_TYPE_TO_PYTHON_TYPE" membership test raised
TypeError: unhashable type: 'list', breaking tool wrapping for the
langchain, langgraph, autogen and llamaindex providers.
Add a branch that maps each member type to a Python type (falling back to
t.Any for unrecognized types) and builds a Union, mirroring the existing
anyOf/oneOf handling. Add regression tests covering the nullable,
single-member, multi non-null, and unrecognized-member cases.
bc4706b to
b0d6c1a
Compare
This PR: - builds on top of #3754 - normalizes list-valued JSON Schema `type` fields before scalar type lookups - applies the handling to direct fields and `anyOf`/`oneOf` options - adds regressions for nullable, single, mixed, unknown, and combiner-nested type lists - credits @anxkhn as a Git co-author ## Context JSON Schema Draft 2020-12 and OpenAPI 3.1 allow `type` to be an array. The previous parser passed that list to a dictionary lookup, raising `TypeError: unhashable type: 'list'`. This replacement covers the direct case from #3754 and the same shape nested in a combiner. --------- Co-authored-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
|
Thank you for identifying and testing the list-valued JSON Schema type failure. The broader fix has now merged in #3815: it includes the direct case from this PR, covers the same shape inside anyOf/oneOf, and preserves scalar fallback defaults for single-item lists. Your contribution is credited as a co-author in the merged replacement. |
Summary
get_signature_format_from_schema_paramsinpython/composio/utils/shared.pybuilds the__signature__used to wrap tools for the agentic providers (langchain, langgraph,autogen, llamaindex). For each property it reads
param_type = param_schema.get("type")and then runs the dict-membership test
elif param_type in PYDANTIC_TYPE_TO_PYTHON_TYPE:.JSON Schema Draft 2020-12 and OpenAPI 3.1 (the dialect Composio toolkits are derived from)
express a nullable field as a list of types, e.g.
{"type": ["string", "null"]}, ratherthan an
anyOf. A list is unhashable, so that membership test raisesTypeError: unhashable type: 'list'at wrap time, which breaks the entire tool set forthose providers whenever any tool has such a parameter.
This is a follow-up to #3708 / #3710. Those made the
anyOf/oneOfcombiner branchdefensive, but the sibling top-level scalar
typebranch still assumedtypeis ahashable string.
Reproduction (on
next, before this change):Fixes #
Changes
python/composio/utils/shared.py: add anelif isinstance(param_type, list):branchbefore the crashing membership test. It maps each member via
PYDANTIC_TYPE_TO_PYTHON_TYPE.get(ptype, t.Any)(falling back tot.Anyforunrecognized member types), unwraps a single member, and otherwise
reduces the membersinto a
t.Union, mirroring the existinganyOf/oneOfhandling directly above it.python/tests/test_schema_parser.py: add four regression tests toTestGetSignatureFormatFromSchemaParamscovering the nullable (["string", "null"]),single-member (
["integer"]), multiple non-null (["string", "integer"]), andunrecognized-member (
["file", "null"]) cases.Type of change
How Has This Been Tested?
Python 3.11, from
python/:nextbefore the change (see the Summary snippet), andconfirmed it is resolved after: the same input now returns
branch: Union[str, Any, NoneType] = ''with no error.uv run pytest tests/test_schema_parser.py -q-> 79 passed (includes the 4 new tests).uv run pytest tests/test_openapi.py tests/test_files.py tests/test_json_schema.py tests/test_schema_parser.py -q-> 253 passed.
ruff check --config config/ruff.toml(+--select I+ruff format --check)on both changed files -> clean.
mypy --config-file config/mypy.ini composio/utils/shared.py tests/test_schema_parser.py-> Success, no issues.
Screenshots (if applicable)
N/A
Checklist
changesets are only for published TypeScript packages)
Additional context
The more robust
schema_converterpath (json_schema_to_pydantic_type) already handleslist-valued/nullable types; this change brings the older
get_signature_format_from_schema_paramspath (used by the langchain/langgraph/autogen/llamaindex providers) in line with it and with the #3708 pattern. The sibling
FALLBACK_VALUES[param_type]KeyErroron an unknown scalartypeis a separate issueand is intentionally left out of this PR to keep the diff surgical.