feat(typescript,go,java): add clientDefault support to SDK generators#14541
feat(typescript,go,java): add clientDefault support to SDK generators#14541Swimburger wants to merge 32 commits intomainfrom
Conversation
…th params Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…th params Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 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:
|
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.
…olean literals with fmt.Sprintf Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…path params - Add utility methods to DefaultValueExtractor for clientDefault handling: hasClientDefault(), extractClientDefault(), extractEffectiveDefault(), hasAnyDefault() - Update WrappedRequestGenerator to use hasAnyDefault() for making params optional - Update HttpUrlBuilder to use extractEffectiveDefault() for query params and clientDefault fallback for path params via .orElse() - Update WrappedRequestEndpointWriter to handle headers with clientDefault via .orElse() - Bump Java IR SDK from 65.4.0 to 66.0.0 (includes clientDefault fields) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Add clientDefault: undefined to createPathParameter, createQueryParameter, and createHttpHeader factory functions in test-utils/ir-factories.ts to match IR SDK v66.0.0 type requirements. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Add isStringPathParameter() type guard so clientDefault fallback code (== "" check and fmt.Sprintf assignment) is only generated for string-typed path parameters. Non-string types would cause compile errors. Also fix indentation on path param and query param loops. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
For wrapped request path params with clientDefault, use a local variable (e.g., _region := request.Region) instead of modifying request.Region directly. This prevents silently mutating the caller's pointer-type request struct when applying clientDefault fallbacks. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…versions.lock Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…pport Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ed.yml irVersion, fix nullable header guard Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…o v61 - Java: Add isAlreadyOptional guard to prevent wrapping already-optional types in another Optional when clientDefault is set - Go: Revert seed.yml irVersion to v61 (Go IR SDK doesn't support v66 name compression yet) - Go: Set versions.yml irVersion to 61 for consistency Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| } | ||
| if header.ClientDefault != nil { | ||
| f.P("if options.", header.Name.Name.PascalCase.UnsafeName, ` == "" {`) | ||
| f.P("options. ", header.Name.Name.PascalCase.UnsafeName, ` = fmt.Sprintf("%v", `, literalToValue(header.ClientDefault), `)`) |
There was a problem hiding this comment.
Syntax error: extra space after "options." will generate invalid Go code like options. HeaderName = ... instead of options.HeaderName = ...
Fix:
f.P("options.", header.Name.Name.PascalCase.UnsafeName, ` = fmt.Sprintf("%v", `, literalToValue(header.ClientDefault), `)`)This matches the pattern used on line 1312 for the env variable case.
| f.P("options. ", header.Name.Name.PascalCase.UnsafeName, ` = fmt.Sprintf("%v", `, literalToValue(header.ClientDefault), `)`) | |
| f.P("options.", header.Name.Name.PascalCase.UnsafeName, ` = fmt.Sprintf("%v", `, literalToValue(header.ClientDefault), `)`) |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
…tringPathParameter to isStringType Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
….1.1) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 TypeScript request wrapper does not make parameters optional when they have clientDefault (generators/typescript/sdk/request-wrapper-generator/src/GeneratedRequestWrapperImpl.ts:898-903)
The TypeScript request wrapper generator's hasDefaultValue method (GeneratedRequestWrapperImpl.ts:898-903) only checks PrimitiveTypeV2 type-level defaults (gated behind the useDefaultRequestParameterValues config flag). It does not check clientDefault. This means that when a query parameter, header, or path parameter has a clientDefault but its IR type is not already optional and has no PrimitiveTypeV2 default, the parameter remains required in the generated request wrapper interface. The ?? fallback added by the client-class-generator (e.g., limit ?? "100" at generators/typescript/sdk/client-class-generator/src/endpoints/utils/GeneratedQueryParams.ts:110-116, or region ?? "us-east-1" at generators/typescript/sdk/client-class-generator/src/endpoints/utils/buildUrl.ts:83-89) becomes dead code because the user must always supply a value.
By contrast, the Java generator explicitly wraps non-optional parameters in Optional when they have a clientDefault (generators/java/sdk/src/main/java/com/fern/java/client/generators/WrappedRequestGenerator.java:121-123), and the Go generator uses zero-value semantics. The TypeScript generator is missing this step, so clientDefault has no effect for parameters whose type is not already optional.
View 24 additional findings in Devin Review.
…st wrapper Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Fixed the TypeScript request wrapper bug identified by Devin Review (BUG-0001: parameters with Fix in bd74942: Added |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| } | ||
| if header.ClientDefault != nil && isStringType(header.ValueType) { | ||
| f.P("if options.", header.Name.Name.PascalCase.UnsafeName, ` == "" {`) | ||
| f.P("options. ", header.Name.Name.PascalCase.UnsafeName, ` = fmt.Sprintf("%v", `, literalToValue(header.ClientDefault), `)`) | ||
| f.P("}") | ||
| } |
There was a problem hiding this comment.
🟡 Go generator: continue removed from header env-var loop changes behavior for headers without clientDefault
In generators/go/internal/generator/sdk.go:1328-1339, the continue statement was removed from the if header.Env != nil block in the client constructor's header initialization loop. The original code used continue to skip headers that already had an env-var fallback, preventing any further processing. While the new clientDefault block correctly handles the case where both Env and ClientDefault are present (env takes priority via the == "" guard), this structural change also means headers that have Env but no ClientDefault now fall through to the end of the loop body where there is no further processing — so behavior is unchanged for them.
However, the indentation of the clientDefault block (lines 1334-1338) is deeply inconsistent with the surrounding code, using extra tabs that make it appear nested inside the if header.Env != nil block when it is actually at the loop-body level. While Go ignores indentation, this makes the control flow misleading and fragile to future edits. A maintainer could easily misread the code as the clientDefault block being conditional on header.Env != nil, when it's actually unconditional.
Code showing misleading indentation
for _, header := range headers {
if header.Env != nil {
f.P("if options.", ..., ` == "" {`)
f.P("options. ", ..., ` = os.Getenv(...)`)
f.P("}")
}
if header.ClientDefault != nil && isStringType(header.ValueType) { // looks nested but isn't
f.P("if options.", ..., ` == "" {`)
...
}
}| } | |
| if header.ClientDefault != nil && isStringType(header.ValueType) { | |
| f.P("if options.", header.Name.Name.PascalCase.UnsafeName, ` == "" {`) | |
| f.P("options. ", header.Name.Name.PascalCase.UnsafeName, ` = fmt.Sprintf("%v", `, literalToValue(header.ClientDefault), `)`) | |
| f.P("}") | |
| } | |
| if header.ClientDefault != nil && isStringType(header.ValueType) { | |
| f.P("if options.", header.Name.Name.PascalCase.UnsafeName, ` == "" {`) | |
| f.P("options. ", header.Name.Name.PascalCase.UnsafeName, ` = fmt.Sprintf("%v", `, literalToValue(header.ClientDefault), `)`) | |
| f.P("}") | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch on the indentation — the clientDefault block is indeed at the loop-body level, not nested inside the if header.Env != nil block, but the extra tabs make it look nested. Will fix the indentation to align with the surrounding code.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…es service Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… 3.64.0) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ompile errors Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…eConverter -> case rename) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…aderClientDefault Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Description
Refs: Polytomic SDK upgrade — support
x-fern-defaultclient-side defaults in generated SDKs.When a header, query parameter, or path parameter has a
clientDefaultvalue (populated via thex-fern-defaultOpenAPI extension, merged in #14355 and #14516), the generated SDK code now uses that value as a fallback when the user doesn't explicitly provide one.Python support is in a separate PR (#14440).
Changes Made
TypeScript SDK Generator
getClientDefaultValue()helper extractsstring | booleanfrom the IRLiteralunionoptions?.headerName ?? "clientDefault"instead of bareoptions?.headerName?? "clientDefault"fallback; for overridable root headers chainsrequestOptions?.header ?? this._options?.header ?? "clientDefault"clientDefaultfallback when the header type is nullable, so explicitnullstill means "don't send the header" instead of being silently replaced by the default?? "clientDefault"fallback on the scalar expression?? "clientDefault"fallback before URL interpolationclientDefaultare made optional in the request interface (same as type-level defaults)YOUR_HEADER_NAMEplaceholderclientDefault: undefinedto allHttpHeaderobject literals in 5 test files66.0.0-alpha.1to66.0.0across all TS generator packages (32 files)Go IR SDK (
generators/go/internal/fern/ir/http.go)ClientDefault *Literalfield (bitmask bit 6) toHttpHeader,PathParameter, andQueryParameterstructs with getters/settersGo SDK Generator (
generators/go/internal/generator/sdk.go)ToHeader(): starts with default value, env var overrides, then user-provided value overridesclientDefaultfallback after env var initialization; guarded byisStringType()to skip non-string headerscontinueremoval: thecontinueafterheader.Envin the constructor loop was removed so headers with bothenvandclientDefaultfall through correctlyclientDefaultas fallback when request field is zero-valuepathParameterDefaultstruct; uses local variable to avoid mutating caller's wrapped request struct;isStringType()guard skips non-string path paramsisStringPathParameter→isStringType: now shared between path param and header guardsJava SDK Generator
irV65:65.4.0toirV65:65.7.0(v65-compatible types withclientDefaultfield). Note:irV65:66.0.0was tried first but has v66 name-compressed types (NameOrStringinstead ofName) that break the existing generator — avoid that version.versions.lock: constraint hash updated after irV65 upgrade (required by:verifyLocksGradle task)DefaultValueExtractor: newhasClientDefault(),extractClientDefault(),hasAnyDefault(), andextractEffectiveDefault()methods.clientDefaulttakes precedence over type-level defaults fromPrimitiveTypeV2.WrappedRequestGenerator: parameters withclientDefaultbecomeOptionalin the wrapped request (same as type-level defaults), usinghasAnyDefault()to check both. AddedisAlreadyOptional()guard to prevent double-wrapping types that are alreadyOptional<T>.WrappedRequestEndpointWriter: headers withclientDefaultuse.orElse(defaultValue)instead of conditionalif (isPresent())blocks. NewfindHeaderClientDefault()looks up the IRHttpHeaderby wire value across endpoint + service headers (endpoint first sofindFirst()prefers endpoint overrides).HttpUrlBuilder: query params useextractEffectiveDefault()(clientDefault > type-level). Path params withclientDefaultuse.orElse(defaultValue)for URL interpolation. NewfindQueryParamClientDefault()matches query params by wire key.Seed / Config
seed.yml: remains atirVersion: v61(Go IR SDK doesn't yet support v66 name compression)x-fern-defaultfixture (ts-sdk: 4 files, go-sdk: 1 file — note: Go snapshot was generated at v66 locally but is not in CI fixture list)Changelog Entries
versions.yml: v3.64.0 (feat: clientDefault support, irVersion 66)versions.yml: v1.34.0 (feat: clientDefault support, irVersion 61)versions.yml: v4.2.0 (feat: clientDefault support, irVersion 65)Human Review Checklist
Java
irV65:65.7.0is fully compatible with the existing Java generator. It was manually published and uses v65-compatible type shapes (e.g.,NameAndWireValuenotNameOrString).irV65:66.0.0under the same artifact caused pervasive compile failures due to v66 name compression — do not use that version.isAlreadyOptionalguard: New static helper prevents double-wrapping already-optional types whenclientDefaultis set. Applied at all three wrapping sites (query params, path params, headers). Verify theisContainer() && getContainer().get().isOptional()check covers nullable containers if those can appear here.findHeaderClientDefaulttype chain: Uses.flatMap(literal -> DefaultValueExtractor.extractClientDefault(Optional.of(literal)))becauseextractClientDefaultacceptsOptional<Literal>but is called afterflatMapalready unwrapped the Optional. Works correctly but is slightly awkward — consider adding an overload that acceptsLiteraldirectly.WrappedRequestEndpointWriterstoreshttpService: A newhttpServicefield was added to access service-level headers infindHeaderClientDefault(). Verify this is always non-null when the writer is constructed.clientDefaultprecedence over type-level defaults:extractEffectiveDefault()prefersclientDefaultoverPrimitiveTypeV2defaults. Verify this is the desired precedence.Go
ClientDefaultfields and bitmask bit1 << 6were hand-added. These will be overwritten on next IR SDK regeneration — verify the regenerated output matches.seed.ymlandversions.ymlboth use irVersion 61. TheclientDefaultfeature code is present but cannot be exercised by CI seed tests until the Go IR SDK is migrated to v66. The x-fern-default Go snapshot directory exists but is not in the CI fixture list.continueremoval (sdk.go ~L1333): Thecontinueafter theheader.Envblock was removed so that headers with bothenvandclientDefaultfall through correctly. Verify no regression for headers withenvbut noclientDefault.isStringTypeguard:isStringType(header.ValueType)was added to guard the constructor'sclientDefaultblock against non-string headers. Note:isStringTypecallsmaybePrimitive, which recurses through optionals — so it returnstrueforOptional<String>. This is safe in practice (constructor headers are always plain strings), but the guard is technically permissive.queryParamsby the time theclientDefaultfallback runs (user params may be serialized via a different mechanism).== ""): TheisStringType()guard skips non-string path params, but verify the check covers all relevant cases (e.g., aliased strings, named strings).pathParameterDefaultstruct introduces a local var (e.g.,_region) to prevent mutating the caller's wrapped request struct. Verify the variable naming doesn't collide with other generated variables.clientDefaultblock indentation (~L1334): TheclientDefaultblock inWriteClienthas inconsistent indentation relative to the surroundingheader.Envblock. Cosmetic only — does not affect behavior.TypeScript
clientDefault: Boolean defaults call.toString()to produce"true"/"false"for header values. Verify this is the desired wire format.clientDefault, the clientDefault is skipped entirely. This is defensive but means nullable headers with clientDefault will never use the default. Verify this trade-off is acceptable.Testing
irV65:65.7.0(verified locally with./gradlew compileJava):verifyLockspassing afterversions.lockconstraint hash updateclientDefault: undefinedon allHttpHeaderobject literals (5 files)x-fern-defaultfixture (ts-sdk, go-sdk)Link to Devin session: https://app.devin.ai/sessions/dae61f87717a46d781e579e47b4758e5
Requested by: @Swimburger