fix(python): handle explode=false for array query parameters#12731
fix(python): handle explode=false for array query parameters#12731
Conversation
Propagate the OpenAPI 'explode' field through the old importer pipeline (OpenAPI IR -> Fern Definition -> Fern IR) and generate comma-separated serialization in the Python SDK when explode=false. Co-Authored-By: unknown <>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
…ps://git-manager.devin.ai/proxy/github.com/fern-api/fern into devin/1771969395-fix-python-query-param-explode
Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
…uery params Co-Authored-By: unknown <>
…d output Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
…i fixture The streaming SSE endpoint was breaking java-spring and ts-express generators which don't support streaming responses. The comma-join logic for streaming endpoints was already verified to work since the code path is shared between streaming and non-streaming endpoints in the Python generator. Co-Authored-By: unknown <>
…71969395-fix-python-query-param-explode
Made-with: Cursor
…ps://github.com/fern-api/fern into devin/1771969395-fix-python-query-param-explode
Fix issue with `explode` field propagation in OpenAPI query parameters.
| def _wrap_with_comma_join( | ||
| self, reference: AST.Expression, query_parameter: ir_types.QueryParameter | ||
| ) -> AST.Expression: | ||
| """Wrap a query parameter reference with comma-joining for explode=False. | ||
|
|
||
| When explode=False, array values should be serialized as comma-separated | ||
| strings (e.g. "a,b,c") instead of repeated keys (e.g. "k=a&k=b&k=c"). | ||
|
|
||
| The parameter may be: | ||
| - A list/sequence value that needs joining | ||
| - Optional, in which case we need a None check | ||
| - A scalar with allow_multiple (Union[str, Sequence[str]]), which also | ||
| needs to handle both single values and sequences | ||
| """ | ||
| param_name = get_parameter_name(query_parameter.name.name) | ||
| value_type = query_parameter.value_type.get_as_union() | ||
| is_optional = value_type.type == "container" and value_type.container.get_as_union().type in ( | ||
| "optional", | ||
| "nullable", | ||
| ) | ||
|
|
||
| if query_parameter.allow_multiple: | ||
|
|
||
| def write_comma_join(writer: AST.NodeWriter) -> None: | ||
| writer.write( | ||
| f'",".join(map(str, {param_name})) if isinstance({param_name}, (list, tuple, set)) else {param_name}' | ||
| ) | ||
|
|
||
| return AST.Expression(AST.CodeWriter(write_comma_join)) | ||
| else: | ||
| # Direct list/set type | ||
| if is_optional: | ||
|
|
||
| def write_comma_join(writer: AST.NodeWriter) -> None: | ||
| writer.write(f'",".join(map(str, {param_name})) if {param_name} is not None else None') | ||
|
|
||
| return AST.Expression(AST.CodeWriter(write_comma_join)) | ||
| else: | ||
|
|
||
| def write_comma_join(writer: AST.NodeWriter) -> None: | ||
| writer.write(f'",".join(map(str, {param_name}))') | ||
|
|
||
| return AST.Expression(AST.CodeWriter(write_comma_join)) |
There was a problem hiding this comment.
🟡 _wrap_with_comma_join ignores the reference parameter, bypassing datetime/date/annotation serialization
The _wrap_with_comma_join method accepts a reference parameter (which is the output of _get_query_parameter_reference, containing datetime serialization, date formatting, and annotation metadata conversion), but completely ignores it. Instead, it reconstructs the raw parameter name via get_parameter_name(query_parameter.name.name) and uses that directly in the generated code.
Root Cause and Impact
At endpoint_function_generator.py:1497-1498, the caller passes self._get_query_parameter_reference(query_parameter) as reference, but then inside _wrap_with_comma_join at line 1461, the method uses param_name = get_parameter_name(query_parameter.name.name) instead of the reference expression.
This means if an explode: false array query parameter has a type that requires special serialization (e.g., datetime, date, or a Fern model type requiring convert_and_respect_annotation_metadata), those transformations will be silently skipped. The raw Python variable name will be used instead of the serialized form.
For the current test cases (string arrays), this doesn't matter since strings don't need special serialization. But if a user defines an explode: false array of datetime values or model objects, the generated code would pass raw Python objects instead of serialized values to the comma-join, producing incorrect wire output.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "tags": ",".join(map(str, tags)) if isinstance(tags, list) else tags, | ||
| "optionalTags": ",".join(map(str, optional_tags)) if isinstance(optional_tags, list) else optional_tags, |
There was a problem hiding this comment.
🟡 Generated code uses isinstance(x, list) instead of isinstance(x, (list, tuple, set)) due to stale seed snapshot
The generator code at endpoint_function_generator.py:1472 correctly generates isinstance({param_name}, (list, tuple, set)) for the allow_multiple branch, but the seed test snapshot at seed/python-sdk/query-parameters-openapi/no-custom-config/src/seed/raw_client.py:123 shows isinstance(tags, list) — checking only list.
Stale Snapshot Details
The seed snapshot was not regenerated after the generator code was updated to check (list, tuple, set). The snapshot file on disk shows:
"tags": ",".join(map(str, tags)) if isinstance(tags, list) else tags,But the generator would now produce:
"tags": ",".join(map(str, tags)) if isinstance(tags, (list, tuple, set)) else tags,This means the snapshot tests are passing against stale output. The stale snapshot itself (if shipped) would fail to comma-join tuple or set inputs, silently passing them through as-is to httpx, which would likely produce incorrect query string serialization.
Prompt for agents
Regenerate the seed snapshot for the query-parameters-openapi fixture by running: pnpm seed test --generator python-sdk --fixture query-parameters-openapi. The generated raw_client.py at seed/python-sdk/query-parameters-openapi/no-custom-config/src/seed/raw_client.py lines 123-124 and 252-253 should then show isinstance(tags, (list, tuple, set)) instead of isinstance(tags, list), matching the generator code at generators/python/src/fern_python/generators/sdk/client_generator/endpoint_function_generator.py line 1472.
Was this helpful? React with 👍 or 👎 to provide feedback.
Description
Refs: Requested by @jsklan
Link to Devin run: https://app.devin.ai/sessions/13529ce4c3a84afc96b17b669e2e27dc
When an OpenAPI spec sets
explode: falseon an array query parameter, the generated Python SDK now serializes values as comma-separated strings (tags=A,B,C) instead of repeated keys (tags=A&tags=B&tags=C). This applies to both regular and streaming endpoints (i.e. those usinghttpx_client.stream()for SSE).The root cause was that the
explodefield was silently dropped when flowing through the old OpenAPI importer pipeline (OpenAPI IR → Fern Definition → Fern IR). The field existed in the parse IR but was never carried into the final IR or downstream types.Note on cross-generator consistency: Neither the TypeScript nor Java SDK generators currently implement
explode: falsecomma-separated serialization. This PR makes Python the first generator to support it per the OpenAPI spec (style: form, explode: false).Changes Made
Pipeline propagation (CLI):
explodefield to the OpenAPI IR finalQueryParametertype (finalIr.yml+ regenerated TS SDK types)explodeduring parse IR → final IR conversion (generateIr.ts)explodefrom OpenAPI IR to Fern Definition (buildQueryParameter.ts)explodeto Fern DefinitionQueryParameterTypeReferenceDetailed(API + serialization types)explodefrom Fern Definition in IR generator (convertQueryParameter.ts) instead of hardcodingundefinedexplode: nooptovisitHttpService.tsvalidator visitorPython generator:
_should_comma_join_query_parameter()— checksexplode is False+allow_multipleor list/set type_wrap_with_comma_join()— wraps array values with",".join(map(str, x)) if isinstance(x, list) else x_get_query_parameters_for_endpoint()to apply comma-join wrapping_get_query_parameters_for_endpoint(), which is shared by both.request()and.stream()code paths — no separate streaming fix was neededTest fixture (
query-parameters-openapi):tags(required,explode: false) andoptionalTags(optional,explode: false) to the existingsearchendpoint/stream/eventswithtagsandidsquery params (bothexplode: false) to verify streaming supportStreamEventschema to componentsSnapshot updates:
openapi-ir-to-fern-testssnapshots forparameter-serialization-edge-casesandswitchboard(both openapi-ir and openapi variants)v3-importer-testsbaseline snapshot forparameter-style-deepobjectir-generator-testssnapshots forquery-parameters-openapiandquery-parameters-openapi-as-objects(both IR and dynamic-snippets)Testing
query-parameters-openapipasses (build + test scripts)poetry run pre-commit run -a)openapi-ir-to-fern-testssnapshots updated and passingv3-importer-testsbaseline snapshot updated and passingir-generator-testssnapshots updated and passing (313 passed)raw_client.pyconfirmed: streaming endpoint uses",".join(map(str, tags)) if isinstance(tags, list) else tagsinsidehttpx_client.stream()paramsUpdates Since Last Revision
/stream/eventsSSE endpoint to test fixture withexplode: falsequery params. Verified that the existing comma-join logic correctly applies to streaming endpoints (both sync and async) viahttpx_client.stream().query-parameters-openapiandquery-parameters-openapi-as-objectsfixtures to include the new streaming endpoint.Human Review Checklist
Critical items to verify:
isinstance(tags, list)check: The comma-join logic only handleslistbut the type hint isUnion[str, Sequence[str]]. If users pass atupleor otherSequence, it won't be comma-joined. Should this beisinstance(tags, (list, tuple))or check forSequenceprotocol?Code duplication in
_wrap_with_comma_join: Theallow_multiplebranch has identical code for bothis_optional=Trueandis_optional=Falsecases. Theisinstancecheck handles both, so the optional branch doesn't add a None guard. Is this intentional (httpx handles None)?WebSocket query parameters: The summary mentioned checking
websocket_connect_method_generator.pybut no changes were made. Do WebSocket connections need the same fix?Auto-generated type modifications:
QueryParameterTypeReferenceDetailed.tsfiles are marked auto-generated but were manually edited. Will this cause issues if someone regenerates the Fern Definition schema?Dead code path: The direct list/set type checking in
_should_comma_join_query_parameter(after theallow_multiplecheck) may never trigger in the old pipeline since it converts arrays to scalars withallow_multiple=True. Is this code path needed for the v3 importer?Streaming endpoint verification: The streaming endpoint test fixture proves the fix works, but the generated code shows the same
isinstance(tags, list)pattern. Confirm this is sufficient for real-world SSE streaming use cases.