fix: round-trip serialization of Callable types with an explicit parameter list#12122
fix: round-trip serialization of Callable types with an explicit parameter list#12122vidigoat wants to merge 1 commit into
Conversation
…meter list serialize_type() corrupted Callable[[X, Y], R] into 'typing.Callable[, R]' because typing.get_args returns the parameter list as a Python list (not a type), which the arg serializer dropped; deserialize_type() then raised DeserializationError. This broke to_dict/from_dict for any component whose schema uses such a Callable (ConditionalRouter, OutputAdapter, etc.): silent corruption on save, hard failure on load. Handle the parameter-list arg explicitly on both sides (render/parse it as [X, Y]) so these types round-trip. A prior fix only covered the Callable[..., R] (Ellipsis) form.
|
@vidigoat is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
rkfshakti
left a comment
There was a problem hiding this comment.
This is a well-scoped fix. The Callable[[X, Y], R] parameter list is indeed a special case in Python's typing system — typing.get_args returns it as a plain list, not a type, so the existing serialize_type path can't handle it.
The approach of adding _serialize_type_arg / _deserialize_type_arg helper functions is clean and minimal — it isolates the special case without touching the main serialization logic. The bracket-delimited format [X, Y] round-trips correctly because _parse_generic_args already splits on commas at the top level, so nested brackets are preserved.
The test coverage is thorough: all common Callable shapes are tested (with params, empty params, nested generics, nested Callables, Optional Callables, and Dict/List of Callables). The release note is well-written.
One suggestion: consider adding a test for Callable[..., int] (ellipsis) to confirm it still works after the change, since _parse_generic_args would see ... as a single argument. But this is not a blocker — the existing test_output_type_deserialization_legacy_ellipsis_literal test covers that path on the deserialization side.
Problem
serialize_type/deserialize_typecorruptCallabletypes that declare an explicit parameter list, so components that serialize such a type can't round-trip throughto_dict/from_dict.typing.get_args(Callable[[int, str], bool])returns([int, str], bool)— the first arg is a Python list, not a type. The arg serializer passed it toserialize_type, whose name handling turns a value starting with[into an empty string, so the whole parameter list vanished. The result then fails to deserialize.This affects any component whose serialized schema uses a
Callablewith a parameter list — e.g.ConditionalRouter/OutputAdapterwith aCallable[[...], ...]output type: silent corruption on save, hard failure on load. (A previous fix, #12062, only handled theCallable[..., R]Ellipsis form.)Fix
Handle the parameter-list argument explicitly on both sides — render it as
[X, Y]on serialize and read it back into a Python list on deserialize — so theseCallabletypes round-trip. Everything else delegates to the existing functions, so behaviour for all other types is unchanged.Added tests for the explicit-parameter-list case (serialize, deserialize, round-trip); they fail before this change and pass after. Full
test_type_serialization.pyis green (192 passed), andruffis clean. Release note included.Disclosure: I used an AI assistant while working on this; I reproduced the bug, verified the fix, and reviewed every line myself.