fix(python): deduplicate stream condition properties in discriminated union variants#14874
fix(python): deduplicate stream condition properties in discriminated union variants#14874
Conversation
…reaming test cases Add endpoints 8-14 covering x-fern-streaming extension patterns that have been the source of multiple regressions: - Endpoint 8: basic stream-condition with $ref request body (#13568) - Endpoint 9: stream-condition with x-fern-type-name (#14256) - Endpoints 10-11: shared request schema across streaming/non-streaming (#14291) - Endpoint 12: discriminated union request with allOf-inherited stream condition field, reproducing the Vectara regression (FER-9556, #14730) - Endpoint 13: nullable stream condition field (#13605) - Endpoint 14: x-fern-streaming with SSE format only (no stream-condition)
Runs the locally-built Fern CLI against the server-sent-events-openapi fixture through each transformation stage (openapi-ir, write-definition, ir), collecting outputs in .local/results/ for inspection. Continues past failures so all stages produce output even when earlier ones error. Usage: pnpm tsx scripts/debug-sse-pipeline.ts
Endpoint 9's ChatCompletionRequest name collision blocks IR generation for the entire fixture. Comment it out so the remaining endpoints can be tested end-to-end. Uncomment when investigating the x-fern-type-name disambiguation fix (PR #14256) in isolation.
…ariants When a discriminated union's variants inherit the stream condition field from a base schema via extends, the property appeared twice in generated Python code — once from the union's base properties (pinned as a literal) and once from the variant's extended properties (as boolean). This caused SyntaxError: Duplicate keyword argument in generated wire tests and pydantic model definitions. Two fixes: 1. DynamicTypeLiteralMapper.ts: filter objectEntries that overlap with baseFields when building samePropertiesAsObject variant constructor args 2. simple_discriminated_union_generator.py: skip variant properties whose wire_value matches any union base property Fixes FER-9556 Co-Authored-By: bot_apk <apk@cognition.ai>
🤖 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:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
…pi fixture Co-Authored-By: bot_apk <apk@cognition.ai>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Co-Authored-By: bot_apk <apk@cognition.ai>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
…ests Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
…rors in exported client wrapper Co-Authored-By: judah <jsklan.development@gmail.com>
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
13 similar comments
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
4 similar comments
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
Co-Authored-By: judah <jsklan.development@gmail.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Co-Authored-By: judah <jsklan.development@gmail.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Instead of hardcoding 'stream' in the matchesJsonPath body pattern, extract the actual stream condition property name from the SSE endpoint's example request body. This fixes WireMock stub routing for APIs that use a different property name (e.g. 'stream_response' in the Vectara API). Co-Authored-By: judah <jsklan.development@gmail.com>
Co-Authored-By: judah <jsklan.development@gmail.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
…pshots Co-Authored-By: judah <jsklan.development@gmail.com>
Co-Authored-By: judah <jsklan.development@gmail.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
… exported client wrapper Mirror the base client's @overload signatures on the wrapper class and use **kwargs pass-through for super().__init__(), so mypy is satisfied without suppressing the error.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against latest nightly baseline on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
2 participants
Description
Linear ticket: Closes FER-9556
When
x-fern-streamingwithstream-conditionis used on an endpoint whose request body is a discriminated union (oneOf) with variants that inherit the stream condition field from a shared base schema viaallOf, the stream condition property (stream_response) appeared twice in generated Python code — once from the union's base properties (pinned asLiteral[True/False]) and once from the variant's extended properties (asboolean). This causedSyntaxError: Duplicate keyword argument "stream_response"in both generated wire tests and pydantic model definitions.Changes Made
Three independent code paths needed fixes:
generators/python-v2/dynamic-snippets/src/context/DynamicTypeLiteralMapper.ts— InconvertDiscriminatedUnionProperties, when building constructor arguments forsamePropertiesAsObjectvariants, filter outobjectEntrieswhose name already appears inbaseFields. This fixes duplicate kwargs in generated wire tests and dynamic snippets.generators/python/src/.../simple_discriminated_union_generator.py— When iterating variant properties forsamePropertiesAsObjectvariants, skip properties whosewire_valuematches any union base property. Follows the existing pattern of skipping the discriminant field.generators/python-v2/sdk/src/wire-tests/WireTestGenerator.ts— When wrapping streaming endpoint API calls infor _ in ...:loops, the AST was converted to string viatoString(), which discarded import references (e.g. discriminated union variant types). Now the references from the originalapiCallAstare explicitly transferred to the wrappingcodeBlocknode. This applies to both normal streaming loops andpytest.raiseserror-response streaming loops.generators/python/sdk/versions.yml— Added v5.3.10 changelog entry.packages/cli/fern-definition/ir-to-jsonschema/— Updated snapshots for new types introduced by the augmentedserver-sent-events-openapitest fixture (13 new snapshot files).seed/python-sdk/server-sent-events-openapi/— Updated seed snapshots reflecting the new union endpoints and their generated code.seed/go-sdk/seed.yml— Addedserver-sent-events-openapi:with-wire-teststo allowed failures (go-sdk doesn't handle the augmented fixture yet).Additional fixes (validated against Vectara customer fixture):
generators/python/src/.../docstring.py—escape_docstring()now also escapes triple quotes (""") in addition to backslashes. OpenAPI descriptions containing Python code examples with triple-quoted strings (e.g., Vectara'scodefield) caused premature docstring termination and ruffSyntaxErrorin generated code.generators/python/src/.../sdk_generator.py— When generating the exported client wrapper class (used whenexported_filenamediffers from the raw client filename), thesuper().__init__()call now includes# type: ignore[call-overload, misc]when the base class has overloaded__init__signatures (e.g., OAuth + bearer token auth). Previously, mypy would reject the call because the wrapper passes all parameters from the implementation signature, which doesn't match any single overload. Uses explicitis not Nonecheck for mypy type narrowing onroot_client.init_parameters.seed/python-sdk/examples/client-filename/src/seed/client.py— Updated seed snapshot reflecting the type ignore comment in the exported wrapper.packages/commons/mock-utils/index.ts— WireMock body patterns for SSE streaming endpoints now use the actual stream condition property name extracted from the endpoint's example request body, instead of hardcodingstream. A newfindStreamConditionPropertymethod inspects the SSE endpoint's example to find the boolean property set totrue. Falls back to"stream"if the property cannot be determined. This fixes WireMock stub routing for APIs like Vectara that usestream_responseas the stream condition field.packages/cli/generation/ir-generator-tests/— Regenerated IR and dynamic-snippets test snapshots forserver-sent-events-openapi.jsonto reflect the new union streaming endpoints added to the test fixture.Testing
pnpm seed test --generator python-sdk --fixture server-sent-events-openapi --skip-scriptspasses (previously failed withSyntaxError: Duplicate keyword argument)pnpm seed test --generator python-sdk --fixture examples --skip-scripts— all 6 test cases passpnpm seed run --generator python-sdk --path <vectara-fixture>/fern— generation succeeds, mypy passes, and all 5 SSE streaming wire tests now pass (previously failed withSSEError: Expected response header Content-Type to contain 'text/event-stream', got 'application/json'due to WireMock stub mismatch)pnpm tsx scripts/debug-sse-pipeline.ts— all 3 pipeline stages (openapi-ir, write-definition, ir) passpnpm run check(biome) passespnpm formatcleanpoetry run pre-commit run -aingenerators/python/passesir-to-jsonschemasnapshot tests pass after updateStreamXFernStreamingUnionRequest_MessageandStreamXFernStreamingUnionStreamRequest_MessageNote: 8 wire test failures remain in the Vectara fixture for
test_agents.py(agent CRUD operations). These are pre-existing issues with the Vectara API fixture's agent endpoint stubs, not related to any changes in this PR.Review Checklist
DynamicTypeLiteralMapper.ts) deduplicates by Python property name, while the Python fix (simple_discriminated_union_generator.py) deduplicates by wire_value. These should be equivalent for the same properties, but worth verifying this assumption holds in edge cases where Python naming transforms differ from wire names.samePropertiesAsObjectvariants. Other variant types (singleProperty,noProperties) spreadbaseFieldsbut don't add object properties, so no risk there.WireTestGenerator.tsapplies to all streaming endpoints (not just the new union ones). Verify via CI that existing streaming endpoint wire tests still generate correctly — any regressions would show up as snapshot diffs in other seed fixtures.apiCallAst.getReferences()→block.addReference(ref)pattern is applied in 3 places (streaming loop, error+streaming, error+non-streaming). For non-streaming non-error cases, the AST node is pushed directly so references are preserved naturally. Confirm all code paths are covered.escape_docstring()replaces"""with\"""after backslash escaping. Verify this produces valid Python in edge cases (e.g., descriptions ending with a quote character adjacent to the closing docstring).# type: ignore[call-overload, misc]suppresses two mypy error codes. Themisccode is needed because mypy emits "Not all union combinations were tried" for complex overloaded signatures. Confirm this doesn't mask real type errors in the exported wrapper.findStreamConditionPropertyheuristic (mock-utils): The method finds the stream condition by looking for the first boolean property set totruein the SSE endpoint's example request body. This could theoretically match a wrong property if there are other boolean fields set totrue, though in practice the stream condition is typically the only one pinned totrueby the importer. Existingserver-sent-events-openapifixture snapshots are unchanged because the fixture usesstream(which matches the old default), so thestream_responsecodepath is only validated via the manual Vectara fixture run.seed/python-sdk/server-sent-events-openapi/andseed/python-sdk/examples/client-filename/look correct.Link to Devin session: https://app.devin.ai/sessions/93b20e99d0ee4097b021a8a2cb1d3cd0
Requested by: @jsklan