Skip to content

feat(typescript): generate stream/non-stream overloads for stream-condition endpoints - #17339

Open
rishabh-fern wants to merge 1 commit into
mainfrom
devin/1786026438-ts-stream-condition-overloads
Open

feat(typescript): generate stream/non-stream overloads for stream-condition endpoints#17339
rishabh-fern wants to merge 1 commit into
mainfrom
devin/1786026438-ts-stream-condition-overloads

Conversation

@rishabh-fern

@rishabh-fern rishabh-fern commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Description
Linear ticket: Refs Pylon #22706 (Truefoundry)

Endpoints declared with stream-condition: $request.stream (IR HttpResponseBody.streamParameter) have only ever generated the streaming variant in TypeScript — a TODO to that effect has been in GeneratedSdkClientClassImpl since #4071 (Jul 2024), and GeneratedSdkEndpointTypeSchemasImpl threw on the response type. Python generates the OpenAI shape (@typing.overload on Literal[True]/Literal[False] + one implementation); this brings TypeScript to parity:

public generate(request: GenerateRequest & { stream: true }, opts?): HttpResponsePromise<core.Stream>;
public generate(request: GenerateRequest & { stream: false }, opts?): HttpResponsePromise;
public generate(request: GenerateRequest, opts?): HttpResponsePromise<core.Stream | RegularResponse> {
if (request.stream) { /* existing streaming path / } else { / ordinary fetcher path */ }
}
Intersection types keep the exported GenerateRequest interface unchanged instead of splitting it into a discriminated union.

Gated behind a new generateStreamConditionOverloads custom config, defaulting to false, per the breaking-changes policy in CLAUDE.md: today generate(req) unconditionally returns Stream, and after the change a caller passing a non-literal boolean matches neither overload, which is a compile error in their code. The default-config seed output is byte-identical to what is on main.

Changes Made
New GeneratedStreamParameterEndpointImplementation composes the existing streaming and default endpoint implementations rather than duplicating transport logic: it returns the union signature, emits the two narrowed overloads, and branches at runtime on the condition property (resolved through the generated request wrapper, so query and inlined-body properties both work, with element access when the key is not a bare identifier).
Overloads are now attached to the public method; previously getOverloads output only reached the private __method, so it was invisible to SDK consumers.
GeneratedSdkEndpointTypeSchemasImpl normalizes streamParameter into its stream/non-stream halves: the stream schema still drives deserializeStreamData, and the non-stream response schema is now generated and used by deserializeResponse (it previously threw).
Config plumbing: generateStreamConditionOverloads through TypescriptCustomConfigSchema -> SdkCustomConfig -> SdkGeneratorCli -> SdkGenerator -> SdkClientClassGenerator.
Seed fixture streaming-parameter split into no-custom-config and stream-condition-overloads outputs.
Changelog entry under generators/typescript/sdk/changes/unreleased/.
[ ] Updated README.md generator (if applicable)
Deliberately out of scope, falling back to today’s stream-only behavior: nested propertyPath conditions (an intersection type cannot narrow a nested property) and bytes/fileDownload non-stream responses. Wire tests still skip streamParameter in TestGenerator.ts.

Testing
pnpm compile — 160/160 successful; pnpm check — clean.
pnpm turbo run test for @fern-typescript/sdk-client-class-generator (605 tests) and @fern-typescript/sdk-endpoint-type-schemas-generator (62 tests) pass. The schemas test that asserted streamParameter throws was replaced with one asserting it deserializes.
The generated stream-condition-overloads fixture was installed and typechecked with tsc: client.dummy.generate({ stream: true, ... }) assigns to core.Stream and { stream: false, ... } assigns to RegularResponse; swapping the two deliberately fails with Type 'RegularResponse' is missing the following properties from type 'Stream', confirming the narrowing is real rather than union widening.
seed/ts-sdk/streaming-parameter/no-custom-config is byte-identical to the previous fixture output (rename-only diff), confirming no change for existing users. Seed’s Docker validator step could not run in this environment (no Docker Hub access), so the generated projects were typechecked directly instead.
[x] Unit tests added/updated
[x] Manual testing completed
Co-authored by Devin: https://app.devin.ai/sessions/cb5d3903f06345a98a604bafacbd98ec (requested by @rishabh.dhadda)


Open in Devin Review

…dition endpoints

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

@nitpickybot nitpickybot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the changes — everything looks good. No issues found.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment on lines +122 to +126
return {
...parameter,
type: `(${parameter.type.toString()}) & { ${getPropertyKey(propertyKey)}: ${isStreaming} }`,
initializer: undefined
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Generated client method can no longer be called without arguments when every request field is optional

The default value of the request argument is stripped when the two narrowed variants of the method are declared (initializer: undefined at generators/typescript/sdk/client-class-generator/src/endpoints/GeneratedStreamParameterEndpointImplementation.ts:125) without marking the argument optional, so users of the generated SDK can no longer call the method with no arguments even though the implementation still allows it.
Impact: For APIs where all request fields (including the stream flag) are optional, previously valid call sites in user code stop compiling once the new option is enabled.

How the request parameter's optionality is lost when building the overloads

RequestWrapperParameter.getParameterType (generators/typescript/sdk/client-class-generator/src/request-parameter/RequestWrapperParameter.ts:18-29) emits hasQuestionToken: false together with initializer: {} when areAllPropertiesOptional is true, i.e. optionality is expressed purely through the default value. getNarrowedParameters correctly removes the initializer (initializers are illegal in overload signatures) but keeps hasQuestionToken at its original false, so both emitted overload signatures declare request as a required parameter while the implementation signature keeps request: X = {}. Calls such as client.generate() then match no overload.

A fix would be to set hasQuestionToken: true whenever the original parameter had an initializer (or was already optional).

Suggested change
return {
...parameter,
type: `(${parameter.type.toString()}) & { ${getPropertyKey(propertyKey)}: ${isStreaming} }`,
initializer: undefined
};
return {
...parameter,
type: `(${parameter.type.toString()}) & { ${getPropertyKey(propertyKey)}: ${isStreaming} }`,
hasQuestionToken: parameter.hasQuestionToken || parameter.initializer != null,
initializer: undefined
};
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +93 to +95
const nonStreamResponseBody = getNonStreamResponse(endpoint);
if (nonStreamResponseBody?.type === "json") {
switch (nonStreamResponseBody.value.responseBodyType.type) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Serialization file for condition-based streaming endpoints gains an unused schema even when the new option is off

A serializer for the non-streaming response is now built for every condition-based streaming endpoint (getNonStreamResponse(endpoint) at generators/typescript/sdk/sdk-endpoint-type-schemas-generator/src/GeneratedSdkEndpointTypeSchemasImpl.ts:93) regardless of whether the new opt-in option is enabled, so SDKs that did not opt in get an extra, never-used piece of generated code.
Impact: Users who did not enable the new feature see their generated output change, gaining dead exported code in the endpoint serialization files.

Why the schema is emitted for users who did not opt in

GeneratedSdkEndpointTypeSchemasImpl has no knowledge of generateStreamConditionOverloads. When the serde layer is enabled and the stream-condition endpoint's non-stream response is a primitive/container JSON type, this.generatedResponseSchema is now constructed and written by writeToFile (generators/typescript/sdk/sdk-endpoint-type-schemas-generator/src/GeneratedSdkEndpointTypeSchemasImpl.ts:189-192). With the option disabled, GeneratedSdkClientClassImpl still returns only the streaming implementation (generators/typescript/sdk/client-class-generator/src/GeneratedSdkClientClassImpl.ts:342-351), so deserializeResponse is never called and the emitted Response schema is unreferenced. The claim that non-opted-in output is unchanged only holds when the non-stream response is a named type (for which no schema is generated).

A fix would be to thread the flag (or the endpoint's chosen implementation) into the schemas generator so the extra schema is only generated when the non-stream variant is actually emitted.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

SDK Generation Benchmark Results

Comparing PR branch against median of 5 nightly run(s) on main (latest: 2026-08-06T05:01:41Z).

Full benchmark table (click to expand)
Generator Spec main (generator) main (E2E) PR (generator) Delta
ts-sdk square 177s (n=5) 181s (n=5) 152s -25s (-14.1%)

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 fern generate). main (E2E): full customer-observable time including build/test scripts (nightly baseline, informational). Delta is computed against generator-only baseline.
⚠️ = generation exited with a non-zero exit code (timing may not reflect a successful run).
Baseline from nightly runs on main (latest: 2026-08-06T05:01:41Z). Trigger benchmark-baseline to refresh.
Last updated: 2026-08-06 14:53 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant