feat(openapi): add x-fern-request-body-unwrap extension with generator support#16277
feat(openapi): add x-fern-request-body-unwrap extension with generator support#16277cadesark wants to merge 20 commits into
Conversation
Add support for an OpenAPI extension that tells generators to flatten nested request body properties into SDK method signatures while still serializing to the full nested wire format. Changes: - IR: Add optional unwrapPath field to InlinedRequestBody (v67.4.0) - Extension: Register x-fern-request-body-unwrap in fernExtensions.ts - New importer (openapi-to-ir): Parse extension in OperationConverter - Legacy parser: Parse extension in convertHttpOperation.ts - OpenAPI IR: Add requestBodyUnwrapPath to Endpoint types - Test fixture: Add request-body-unwrap test definition - CLI changelog entry for the new feature 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.
…uble quotes Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…Endpoint 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>
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) 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 |
…t, Python, Go) Implement codegen for x-fern-request-body-unwrap across three generators: - TypeScript: flattens leaf params into method signature, serializes nested wire format - Python: flattens params in sync/async clients, builds nested dict for serialization - Go: generates flat request struct with custom MarshalJSON/UnmarshalJSON for wire format Also fixes legacy OpenAPI pipeline to propagate unwrapPath through Fern Definition and adds seed test snapshots for all three generators. 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>
…pet mismatch) 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>
|
@claude review |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| // Top-level non-path properties | ||
| for _, prop := range inlinedRequestBody.Properties { | ||
| if prop.Name.WireValue == pathRoot { | ||
| continue | ||
| } | ||
| if isTypeReferenceLiteral(prop.ValueType) { | ||
| lit := extractLiteralValue(prop.ValueType) | ||
| if lit != nil { | ||
| f.P(`body["`, prop.Name.WireValue, `"] = `, literalToValue(lit)) | ||
| } | ||
| continue | ||
| } | ||
| fieldName := goExportedFieldName(prop.Name.Name.PascalCase.UnsafeName) | ||
| goType := typeReferenceToGoType(prop.ValueType, f.types, f.scope, f.baseImportPath, importPath, false) | ||
| if strings.HasPrefix(goType, "*") { | ||
| f.P("if ", receiver, ".", fieldName, " != nil {") | ||
| f.P(`body["`, prop.Name.WireValue, `"] = `, receiver, ".", fieldName) | ||
| f.P("}") | ||
| } else { | ||
| f.P(`body["`, prop.Name.WireValue, `"] = `, receiver, ".", fieldName) | ||
| } |
There was a problem hiding this comment.
🔴 writeUnwrappedMarshalJSON reimplements marshaling from scratch and silently breaks three guarantees the non-unwrapped path provides (generators/go/internal/generator/sdk.go:4773-4862). All three issues stem from sidestepping the existing marshal infrastructure: (1) the nil-guard strings.HasPrefix(goType, "*") (lines 4787, 4855) only catches pointer types, so optional map[K]V and []T properties unconditionally serialize as "key":null instead of being omitted — visible in the fixture's seed/go-sdk/.../transfer.go:66 where Metadata (tagged json:"metadata,omitempty") is written unconditionally; (2) receiver.explicitFields is never consulted, so the documented "Explicit Null" feature (calling SetX(nil) to emit "x": null) is silently broken for unwrapped requests — the non-unwrap path uses internal.HandleExplicitFields(marshaler, receiver.explicitFields) (sdk.go:3713-3717) but the unwrap branch bypasses it; (3) requestBody.dates is populated but ignored, so date-typed leaf fields skip the typed internal.NewDate wrapper (sdk.go:3698-3708) and emit Go's default RFC3339 time.Time format where the API expects YYYY-MM-DD. The architectural fix is to reuse the existing marshal infrastructure (struct tags for omitempty, HandleExplicitFields, date wrappers) rather than build a parallel implementation.
Extended reasoning...
Bug Summary
The newly introduced writeUnwrappedMarshalJSON function in generators/go/internal/generator/sdk.go (lines 4758-4895) reimplements JSON marshaling from scratch instead of composing with the existing marshal infrastructure. This reimplementation diverges from the non-unwrap path in three distinct ways, all of which silently break documented behavior contracts. Each is reachable today (issues 1 and 2) or as soon as a real API uses date fields with x-fern-request-body-unwrap (issue 3).
Issue 1: Optional maps/slices unconditionally emit null
The nil-guard at sdk.go:4787 (and again at 4855 for leaf properties) reads:
if strings.HasPrefix(goType, "*") {
f.P("if ", receiver, ".", fieldName, " != nil {")
f.P(`body["`, prop.Name.WireValue, `"] = `, receiver, ".", fieldName)
f.P("}")
} else {
f.P(`body["`, prop.Name.WireValue, `"] = `, receiver, ".", fieldName)
}The check strings.HasPrefix(goType, "*") only matches pointer types. Optional map[K]V and []T are nilable but their Go type strings start with map[ / [], so they fall through to unconditional assignment.
Step-by-step proof using the PR's own fixture:
test-definitions/fern/apis/request-body-unwrap/openapi.ymldeclaresmetadataasoptional<map<string, string>>at the/transfersendpoint.- The generator emits
seed/go-sdk/request-body-unwrap/no-custom-config/transfer.go. Line 20 shows the struct field:Metadata map[string]string+ "json:\"metadata,omitempty\" url:\"-\"" + ` - Line 66 shows the marshal output:
body[\"metadata\"] = c.Metadata— no nil guard. Theomitemptystruct tag is bypassed entirely because we are not marshaling the struct; we are marshaling amap[string]interface{}that already contains the value. - User code:
&CreateTransferRequest{Amount: 100, SourceAccountID: \"a\", DestinationAccountID: \"b\"}— metadata unset. json.Marshalof amap[string]interface{}where one value is a nilmap[string]stringproduces\"metadata\":null.- Wire output:
{\"metadata\":null,\"attributes\":{...}}instead of the expected{\"attributes\":{...}}.
APIs that treat null differently from absent — common with PATCH semantics or strict schemas where null means "clear the value" while absent means "leave unchanged" — will misbehave.
Issue 2: explicitFields bitmask is ignored — SetX(nil) does not emit explicit null
The non-unwrap path emits MarshalJSON like:
explicitMarshaler := internal.HandleExplicitFields(marshaler, receiver.explicitFields)
return json.Marshal(explicitMarshaler)(sdk.go:3713-3717). HandleExplicitFields walks the bitmask to strip omitempty from explicitly-set fields, producing the documented "Explicit Null" behavior advertised in seed/go-sdk/.../README.md.
The unwrap branch returns early at sdk.go:3630-3635 and never calls HandleExplicitFields. The generated writeUnwrappedMarshalJSON uses only if receiver.FieldName != nil checks (sdk.go:4787-4793, 4855-4861).
Step-by-step proof:
seed/go-sdk/.../payment.goline 26 declaresexplicitFields *big.Int.- Lines 50-55:
SetDescription(description *string)flipscreatePaymentRequestFieldDescriptionviac.require(...). - Lines 57-69 (MarshalJSON) never read
c.explicitFields. - User code:
req := &CreatePaymentRequest{...}; req.SetDescription(nil)— intent is to send\"description\": null. - MarshalJSON output: line 63 evaluates
if c.Description != nil→ false →descriptionis omitted. - Wire output:
{...}(no description key) instead of the expected{...,\"description\":null,...}.
The Explicit Null section of the generated SDK's README is silently broken for the exact code path this PR introduces.
Issue 3: Date-formatted fields lose their formatter
The non-unwrap path applies typed wrappers for date fields (sdk.go:3698-3708):
for _, date := range requestBody.dates {
f.P(date.Name.Name.PascalCase.UnsafeName, ": ", date.Constructor, "(", receiver, ".", date.Name.Name.PascalCase.UnsafeName, "),")
}where date.Constructor resolves to e.g. internal.NewDate (emits YYYY-MM-DD) or internal.NewDateTime (emits RFC3339). The requestBody.dates slice is populated by typeVisitor.visitObjectProperties against the synthetic unwrapped object (sdk.go:4002-4015).
The unwrap branch returns at sdk.go:3635 before reaching the date-wrap loop. writeUnwrappedMarshalJSON emits leaf[wire] = receiver.FieldName (sdk.go:4856-4862), putting the raw time.Time into map[string]interface{}. json.Marshal then uses Go's default time.Time.MarshalJSON which produces RFC3339Nano — wrong for Fern date primitives.
Step-by-step proof:
- Suppose an OpenAPI spec adds
birth_date: { type: string, format: date }to a leaf inside anx-fern-request-body-unwrappath. primitiveToGoType(model.go) emitstime.Timeas the Go type.requestBody.datesis still populated for the synthetic unwrapped object (typeVisitor runs unconditionally).- The unwrap branch ignores it.
leaf[\"birth_date\"] = c.BirthDatewrites the rawtime.Time. json.Marshalproduces\"birth_date\":\"2024-01-15T00:00:00Z\"— a datetime, where the API expects\"birth_date\":\"2024-01-15\".
The PR's seed fixture has no date-typed fields, so the bug is latent in the test suite but reachable for any real API using the new feature with a date primitive.
Why None of These Were Caught
The seed fixture exercises the happy path (all required fields set, optional metadata left unset which conveniently triggers issue 1 in the seed output but is not asserted as wire output). There are no tests asserting wire output, no explicit-null tests, and no date primitives in the unwrap-test schema.
Suggested Fix
The architectural fix is to reuse the non-unwrap marshal infrastructure instead of building a parallel implementation. One approach:
- Generate a flat struct with proper struct tags (
omitemptyfor optional non-pointers via,omitempty, typed wrappers for date fields). - Have MarshalJSON build a temporary nested struct with each leaf field assigned from the receiver, applying the same
HandleExplicitFieldsand date-constructor wrappers as the non-unwrap path. - Use
json.Marshalon that struct (so omitempty + custom MarshalJSON of typed wrappers work).
Concrete minimum patches for the current implementation:
- For issue 1, also guard
map[/[]prefixed types:if strings.HasPrefix(goType, \"*\") || strings.HasPrefix(goType, \"map[\") || strings.HasPrefix(goType, \"[]\"). (Optional value-type named aliases still leak — full fix requires the struct-tag approach.) - For issue 2, thread
explicitFieldsinto the body assignments: for each field, emitif c.explicitFields != nil && c.explicitFields.Bit(n) != 0 { body[wire] = field } else if field != nil { body[wire] = field }(mirroring HandleExplicitFields semantics). - For issue 3, when emitting
body[wire] = receiver.Fieldfor adateprimitive, wrap with the sameinternal.NewDate(...)/internal.NewDateTime(...)constructor used in the non-unwrap path (requestBody.datesis already populated by the visitor for the unwrapped object).
| return get_json_body_for_inlined_request( | ||
| self._context, | ||
| self._get_properties(), | ||
| ) | ||
|
|
||
| def _build_unwrapped_json_body(self, unwrap_path: List[str]) -> Optional[AST.Expression]: | ||
| context = self._context | ||
| all_top_level = self._get_raw_top_level_properties() | ||
| path_root = unwrap_path[0] | ||
| top_non_path = [p for p in all_top_level if get_wire_value(p.name) != path_root] | ||
|
|
||
| path_prop = next((p for p in all_top_level if get_wire_value(p.name) == path_root), None) | ||
| if path_prop is None: | ||
| return get_json_body_for_inlined_request(context, self._get_properties()) | ||
|
|
||
| intermediate_levels: List[Tuple[List[ir_types.ObjectProperty], str]] = [] | ||
| current_type_ref = path_prop.value_type |
There was a problem hiding this comment.
🔴 Generated docstrings, JSDoc @example blocks, reference.md, and snippet.json for endpoints with x-fern-request-body-unwrap render the original nested wire shape (data=CreatePaymentRequestData(type="payment", attributes=CreatePaymentRequestDataAttributes(amount=1, ...))) instead of the flat SDK signature the generators actually emit (create_payment(*, amount, currency, description)). Customers copy-pasting from the docs page (snippet.json), an IDE hover/help() (docstring/JSDoc), or reference.md get code that raises TypeError: unexpected keyword 'data' (Python) or fails TS2353 (TypeScript) against the SDK's own interface — the canonical fixture for the feature being introduced ships with non-functional documentation. The Go variant of this same defect was added to seed/go-sdk/seed.yml allowedFailures in this PR with the comment "v2 dynamic snippets use nested struct literal but v1 generates flat struct with unwrapPath" — i.e. the PR authors knew Go was broken; the Python/TS variants have no compile-time gate and so were not caught. Fix: when unwrapPath is set, the example-emission paths must navigate into example.request.value[path[0]][path[1]]... and surface the leaf-object key/value pairs as top-level kwargs (Python) / properties (TS) / struct fields (Go), lifting top-level non-path properties (e.g. metadata) directly.
Extended reasoning...
What the bug is
All three SDK generators were updated to flatten the method signature when x-fern-request-body-unwrap is set (_get_unwrapped_properties in Python, getUnwrappedProperties in TS, getUnwrappedBodyObjectProperties in Go). But the parallel paths that emit code examples — Python docstrings, TS JSDoc @example, reference.md, and the dynamic-snippet generator that produces snippet.json — were not updated. They still consume endpoint.examples[*].request.value straight from the IR, which is shaped to the wire (nested) format. The example values are never re-keyed onto the flat parameter names that the generator just emitted.
Verified directly in this PR's seed snapshots
-
Python docstring vs. signature —
seed/python-sdk/request-body-unwrap/no-custom-config/src/seed/payment/client.py. Lines 29-36 declare the function asdef create_payment(self, *, amount: int, currency: str, description: typing.Optional[str] = OMIT, request_options: ...)— nodataparameter. Lines 56-75 (Examplesblock in that same docstring) showclient.payment.create_payment(data=CreatePaymentRequestData(type="payment", attributes=CreatePaymentRequestDataAttributes(amount=1, currency="currency"))). The same broken docstring is duplicated in the async variant (lines 169-178) and intransfer/client.py:65-71. -
Python
reference.md— lists**data:** CreatePaymentRequestDataand**attributes:** CreateTransferRequestAttributesas the only parameters. Neither exists on the generated function. -
TypeScript JSDoc —
seed/ts-sdk/request-body-unwrap/no-custom-config/src/api/resources/payment/client/Client.tslines 28-37 show@example await client.payment.createPayment({ data: { type: "payment", attributes: { amount: 1, currency: "currency" } } }). But theCreatePaymentRequestinterface inrequests/CreatePaymentRequest.ts:15-21is{ amount: number; currency: string; description?: string }— nodatafield. The example does not type-check against the interface declared on the same PR. The standalone@exampleon the interface file itself (lines 4-13) has the same broken shape. -
snippet.json(customer-facing docs payload) —seed/python-sdk/.../snippet.jsonline 12 contains the brokencreate_payment(data=CreatePaymentRequestData(...))call, and line 38 containscreate_transfer(attributes=CreateTransferRequestAttributes(...))whereattributesis not a parameter on the generatedcreate_transfer.seed/ts-sdk/.../snippet.jsonline 11 contains the broken nested literal. This is what renders on every customer-facing API docs page. -
Go variant is already declared broken in the PR —
seed/go-sdk/seed.ymlline 380 addsrequest-body-unwraptoallowedFailureswith the literal commentv2 dynamic snippets use nested struct literal but v1 generates flat struct with unwrapPath. The PR authors knew the Go snippet doesn't compile against the flat struct and explicitly silenced it. The Python/TS variants are exactly the same defect, just lacking a compile-time gate.
Step-by-step proof
Concretely, what happens when a customer copy-pastes the Python snippet from snippet.json line 12:
from seed import SeedApi
from seed.payment import (
CreatePaymentRequestData,
CreatePaymentRequestDataAttributes,
)
client = SeedApi(base_url="https://yourhost.com/path/to/api")
client.payment.create_payment(
data=CreatePaymentRequestData(
type="payment",
attributes=CreatePaymentRequestDataAttributes(amount=1, currency="currency"),
),
)Step 1 — Python looks up client.payment.create_payment. Generator emitted signature: def create_payment(self, *, amount: int, currency: str, description: typing.Optional[str] = OMIT, request_options: ...) -> Payment.
Step 2 — Caller passes data=... as the only kwarg.
Step 3 — Python raises TypeError: create_payment() got an unexpected keyword argument 'data'. Required arg amount is also missing, but data is rejected first.
Same proof in TypeScript: the JSDoc/snippet.json shows { data: { type: "payment", attributes: { amount: 1, currency: "currency" } } } typed against CreatePaymentRequest = { amount: number; currency: string; description?: string }. tsc rejects with TS2353: Object literal may only specify known properties, and 'data' does not exist in type 'CreatePaymentRequest'. The actual call site in Client.ts:62-70 correctly nests for the wire — only the documentation examples are wrong.
Why existing safeguards do not catch this
- The seed snapshot tests assert generated text matches the frozen golden file. A broken example matching the broken golden file passes. The goldens were updated to the broken output rather than the output being fixed.
- Go is compiled as part of seed CI, so the broken Go snippet was caught and explicitly silenced via
allowedFailures(not fixed). - Python:
pyright/mypyis not run on dynamic-snippets per fixture. TS: the snippet examples are JSDoc comments — not type-checked bytsc. So the identical defect ships invisibly in the two languages without compile-time gates.
Impact
Every Fern Docs Registry page rendered from snippet.json shows non-compiling/non-runnable code. Every IDE hover (TS) and help() (Python) shows non-runnable code. Every reference.md entry lists parameters that don't exist. This is the canonical example for the very feature being introduced — copy-paste from the PR's own fixture produces TypeError / TS2353. The Go acknowledgement frames this as a real bug to fix, not as locked-in behavior.
Fix
When unwrapPath is set on an inlined request body, the example-rewriting step in each generator must walk into example.request.value[path[0]][path[1]]... and emit the resulting leaf-object key/value pairs as top-level kwargs (Python) / properties (TS) / struct fields (Go) on the call. Top-level non-path properties (e.g. metadata on createTransfer) lift out of example.request.value directly. The relevant paths are: Python EndpointFunctionGenerator._get_endpoint_example (and the snippet generator in packages/cli/generation/dynamic-snippets/python/), TypeScript GeneratedDefaultEndpointRequest's example writer and dynamic-snippets/typescript/, and Go's dynamic-snippet writer.
Single-value enums (e.g., enum: ['payment']) are now auto-filled at intermediate nesting levels during wire format reconstruction, alongside literal types. This ensures the full nested structure is correct on the wire when using x-fern-request-body-unwrap. Fixes TS, Python, and Go generators. Updates seed snapshots to include 'type: "payment"' at the data level. 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>
- Fix biome line-width formatting for ternary expression - resolveObjectProperties now walks Extends chain recursively - inlinedRequestBodyToUnwrappedObjectTypeDeclaration now sets Extends Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| if isTypeReferenceLiteral(prop.ValueType) { | ||
| lit := extractLiteralValue(prop.ValueType) | ||
| if lit != nil { | ||
| f.P(`body["`, prop.Name.WireValue, `"] = `, literalToValue(lit)) | ||
| } | ||
| continue |
There was a problem hiding this comment.
🔴 Go MarshalJSON/UnmarshalJSON reference non-existent struct fields for single-value enum properties
The struct definition for unwrapped request bodies excludes single-value enum properties (via getAutoFillValue in getUnwrappedBodyObjectProperties at generators/go/internal/generator/sdk.go:4729), but writeUnwrappedMarshalJSON only checks isTypeReferenceLiteral when iterating top-level and leaf properties. A single-value enum property passes through the literal check and falls into the branch that emits receiver.FieldName — but that field doesn't exist on the struct. This generates Go code that won't compile.
The same issue exists in writeUnwrappedUnmarshalJSON at lines 4975 and 5007. Intermediate levels correctly use getAutoFillValue (line 4879), showing the intent was to handle both cases.
Affected locations in MarshalJSON and UnmarshalJSON
- MarshalJSON top-level: line 4832 checks
isTypeReferenceLiteralbut should also handle single-value enums - MarshalJSON leaf: line 4897 same issue
- UnmarshalJSON top-level: line 4975 same issue
- UnmarshalJSON leaf: line 5007 same issue
The test definition includes CreatePaymentRequestDataType (a single-value enum with value "payment"), which would trigger this bug when the data.attributes unwrap path is used.
Prompt for agents
In writeUnwrappedMarshalJSON, the top-level property loop (lines 4828-4848) and the leaf property loop (lines 4896-4913) both only check isTypeReferenceLiteral to decide whether to auto-fill a value. They need to also handle single-value enums, since getUnwrappedBodyObjectProperties excludes those from the struct fields via getAutoFillValue.
The fix pattern: where the code currently does `if isTypeReferenceLiteral(prop.ValueType) { ... continue }`, it should instead check `if val := getAutoFillValue(prop.ValueType, f.types); val != "" { f.P(varName, ..., val); continue }`. This mirrors how intermediate levels already correctly use getAutoFillValue at line 4879.
The same fix is needed in writeUnwrappedUnmarshalJSON at lines 4975 and 5007 — single-value enum properties should be skipped (like literals are) since they have no corresponding struct field.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — fixed in 56d8b59. All four locations (MarshalJSON top-level + leaf, UnmarshalJSON top-level + leaf) now use getAutoFillValue instead of isTypeReferenceLiteral, matching the intermediate level pattern. Go seed test passes locally.
…Python import order - MarshalJSON/UnmarshalJSON top-level and leaf loops now use getAutoFillValue instead of isTypeReferenceLiteral, matching intermediate levels - Prevents referencing non-existent struct fields for single-value enums - Fix ruff import ordering in Python inlined_request_body_parameters.py Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| unwrap_path = getattr(self._request_body, "unwrapPath", None) | ||
| if unwrap_path is not None and len(unwrap_path) > 0: | ||
| return list(unwrap_path) | ||
| return None |
There was a problem hiding this comment.
🔴 Python _get_unwrap_path uses camelCase attribute name instead of snake_case, making the feature non-functional
The method _get_unwrap_path uses getattr(self._request_body, "unwrapPath", None) to read the new IR field. However, Fern-generated Python SDKs use snake_case for Python attribute names (with camelCase as the JSON wire alias). Looking at how other fields are accessed in the same class — self._request_body.properties and self._request_body.extends — they use snake_case. When the Python IR types are regenerated to include this field, the Pydantic model will define it as unwrap_path: Optional[List[str]] = Field(alias="unwrapPath"). Since getattr uses the Python attribute name (not the Pydantic alias), getattr(obj, "unwrapPath", None) will always return None, causing the entire unwrap feature to silently fall back to the non-unwrapped code path in the Python generator.
| unwrap_path = getattr(self._request_body, "unwrapPath", None) | |
| if unwrap_path is not None and len(unwrap_path) > 0: | |
| return list(unwrap_path) | |
| return None | |
| def _get_unwrap_path(self) -> Optional[List[str]]: | |
| unwrap_path = getattr(self._request_body, "unwrap_path", None) | |
| if unwrap_path is not None and len(unwrap_path) > 0: | |
| return list(unwrap_path) | |
| return None | |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is actually correct as-is. unwrapPath is not a defined field on the InlinedRequestBody Pydantic model — it's a new IR addition that hasn't been published to the fern IR package yet. The class uses extra="allow", so extra fields from JSON are stored under their original JSON key name (camelCase). Verified locally:
obj = MyModel.model_validate({'name': 'test', 'unwrapPath': ['data', 'attributes']})
getattr(obj, 'unwrapPath') # → ['data', 'attributes'] ✓
getattr(obj, 'unwrap_path') # → None ✗Once the IR package is republished with unwrap_path as a defined field, this would need updating — but for now "unwrapPath" is correct.
| for i := 1; i < len(unwrapPath); i++ { | ||
| objProps := resolveObjectProperties(leafTypeRef, f.types) | ||
| for _, p := range objProps { | ||
| if p.Name.WireValue == unwrapPath[i] { | ||
| leafTypeRef = p.ValueType | ||
| break | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Go UnmarshalJSON leaf type resolution doesn't verify segment was found, may use wrong type
In writeUnwrappedUnmarshalJSON at generators/go/internal/generator/sdk.go:4947-4955, the leaf type pre-computation loop walks the type graph to find the leaf type. The inner for _, p := range objProps loop breaks when it finds a matching segment, but if no matching segment is found (e.g., due to a stale or incorrect unwrapPath), leafTypeRef retains its previous value and the outer loop continues searching the WRONG type's properties for the next path segment. This contrasts with getUnwrappedBodyObjectProperties (generators/go/internal/generator/sdk.go:4757-4767) which properly tracks found and returns early. The consequence is that the generated UnmarshalJSON could unmarshal fields from a completely wrong type, producing silent data corruption at runtime.
| for i := 1; i < len(unwrapPath); i++ { | |
| objProps := resolveObjectProperties(leafTypeRef, f.types) | |
| for _, p := range objProps { | |
| if p.Name.WireValue == unwrapPath[i] { | |
| leafTypeRef = p.ValueType | |
| break | |
| } | |
| } | |
| } | |
| for i := 1; i < len(unwrapPath); i++ { | |
| objProps := resolveObjectProperties(leafTypeRef, f.types) | |
| found := false | |
| for _, p := range objProps { | |
| if p.Name.WireValue == unwrapPath[i] { | |
| leafTypeRef = p.ValueType | |
| found = true | |
| break | |
| } | |
| } | |
| if !found { | |
| break | |
| } | |
| } | |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — fixed in the next commit. Added found tracking with early break to match getUnwrappedBodyObjectProperties.
Prevents using wrong type if unwrapPath segment is not found during type graph traversal, matching getUnwrappedBodyObjectProperties pattern. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- write_auto_fill_prop now accepts Union[ObjectProperty, InlinedRequestBodyProperty] - Add explicit type annotation for leaf_props in _get_unwrapped_properties - Fix ruff import ordering Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Dynamic snippet generation doesn't support unwrapPath yet, so it produces code referencing the nested struct shape while the SDK flattens it. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| const resolvedType = context.type.resolveTypeReference(prop.valueType); | ||
| const isLiteral = resolvedType.type === "container" && resolvedType.container.type === "literal"; | ||
| if (!isLiteral) { | ||
| const requestProperty = this.getInlineProperty(inlinedRequestBody, prop, context); | ||
| properties.push(requestProperty); | ||
| } |
There was a problem hiding this comment.
🔴 TS exposes top-level and leaf-level single-value enums as required user params, diverging from Go/Python which auto-fill them. With a spec like attributes.kind: enum [transfer] (leaf) or a top-level fixed-value sibling next to the unwrap root, Python emits client.payments.create_payment(amount=100) and Go emits &CreatePaymentRequest{Amount: 100}, but TS requires the user to pass { kind: 'transfer', amount: 100 } on every call. Fix by replacing the isLiteral filter at GeneratedRequestWrapperImpl.ts:935-940 (top) and :986-998 (leaf) with a check that also matches single-value enums, and by calling getAutoFillExpression at the top + leaf paths in GeneratedDefaultEndpointRequest.ts (the intermediate level at line 466 already uses this pattern correctly).
Extended reasoning...
What the bug is
The TypeScript unwrap path treats single-value enums inconsistently across levels and inconsistently with the Go and Python generators. The Go counterpart was fixed in commit 56d8b59 of this PR; the TS counterpart still has the original defect at both the top-level and leaf-level of the unwrap path.
Code paths in TS
Request-wrapper interface generation (generators/typescript/sdk/request-wrapper-generator/src/GeneratedRequestWrapperImpl.ts):
- Top-level loop at lines 935-940:
const resolvedType = context.type.resolveTypeReference(prop.valueType); const isLiteral = resolvedType.type === "container" && resolvedType.container.type === "literal"; if (!isLiteral) { const requestProperty = this.getInlineProperty(inlinedRequestBody, prop, context); properties.push(requestProperty); }
- Leaf-level loop at lines 986-998: same
isLiteralfilter. Single-value Named enums pass through and become required user-facing parameters. - Intermediate-level loop at 961-981 correctly skips them entirely (they are part of the nesting structure).
Serialization (generators/typescript/sdk/client-class-generator/src/endpoint-request/GeneratedDefaultEndpointRequest.ts):
- Top-level (lines 489-510) and leaf-level (lines 429-447) only check
container.literal, falling through toreferenceToRequestBody.sdkNamefor single-value enums (i.e., reads the user-supplied value). - Intermediate-level (lines 462-475) correctly calls
this.getAutoFillExpression, which at lines 534-555 matches both literals and single-value enums.
Why Go and Python don't have this defect
- Go
sdk.go:getAutoFillValue(which covers both literals and single-value enums viagetSingleValueEnumWireValue) is called uniformly at top, intermediate, and leaf ingetUnwrappedBodyObjectProperties,writeUnwrappedMarshalJSON, andwriteUnwrappedUnmarshalJSONafter the fix in commit 56d8b59. - Python
inlined_request_body_parameters.py:_is_auto_fill_propertyat lines 101-104 checks both literals and single-value enums, and is applied uniformly at line 151 (top-level) and line 172 (leaf-level) of_get_unwrapped_properties, and viawrite_auto_fill_propin_build_unwrapped_json_body.
Why the seed fixture passes
The PR's only single-value enum in the unwrap fixture (CreatePaymentRequestDataType with value payment) sits at the intermediate level — it's a property of data, where the unwrap path is data.attributes. The intermediate-level handler in TS covers this case correctly, so seed snapshots produce identical output across the three generators. The asymmetry only surfaces when a single-value enum sits at the top level or inside the leaf object.
Step-by-step proof
Take a JSON:API-style spec where the discriminator sits inside the leaf object:
paths:
/payments:
post:
x-fern-request-body-unwrap: data.attributes
requestBody:
content:
application/json:
schema:
required: [data]
properties:
data:
type: object
required: [attributes]
properties:
attributes:
type: object
required: [kind, amount]
properties:
kind:
type: string
enum: [transfer] # single-value enum at LEAF
amount:
type: integer- Python:
_get_unwrapped_propertiescalls_is_auto_fill_propertyat line 172, which returnsTruefor the single-valuekindenum, sokindis excluded from the flat parameter list. The serialization (_build_unwrapped_json_body) callswrite_auto_fill_prop, which emits"kind": "transfer"inline. Generated SDK call:client.payments.create_payment(amount=100). - Go:
getUnwrappedBodyObjectPropertiescallsgetAutoFillValueat sdk.go:4763, which returns"transfer"for single-value enums, sokindis excluded from the struct.writeUnwrappedMarshalJSONemitsleaf["kind"] = "transfer"inline. Generated SDK call:&CreatePaymentRequest{Amount: 100}. - TypeScript:
getUnwrappedPropertiesreaches the leaf-level loop at lines 986-998.resolvedType.type === "container" && resolvedType.container.type === "literal"isfalsefor the single-value Named enum (it's anamedreference, notcontainer.literal), sokindfalls through and is pushed as a requiredPropertyonCreatePaymentRequest. The serialization at GeneratedDefaultEndpointRequest.ts:429-447 also only checkscontainer.literal, falling through toreferenceToRequestBody.sdkName. Generated SDK call:client.payments.createPayment({ kind: 'transfer', amount: 100 }).
Same IR → three different SDK surfaces. The TS user must hand-type kind: 'transfer' on every call, despite the value being fixed in the schema.
Why existing safeguards don't catch it
- Seed snapshot tests compare against frozen golden files. Since the seed's only single-value enum sits at the intermediate level (which TS handles correctly), the goldens are correct and pass.
- There is no cross-generator equivalence test for the unwrap output.
- TS compile (
tsc) doesn't notice this — the generated interface and call site are internally consistent; the defect is in what is exposed to the user, not in whether the code compiles.
How to fix
Mirror the intermediate-level handling at the top and leaf levels:
- In
GeneratedRequestWrapperImpl.tsgetUnwrappedProperties(lines 935-940 top, 986-998 leaf): replace theisLiteralfilter with a check that also matches single-value enums — e.g.,this.getAutoFillExpression(prop, context) != null, mirroring the existinggetAutoFillExpressionhelper that already lives at GeneratedDefaultEndpointRequest.ts:534-555. - In
GeneratedDefaultEndpointRequest.tsbuildUnwrappedRequestBodyExpression: at the top-level (around line 494) and leaf-level (around line 431), usegetAutoFillExpressionto emit the literal/enum value inline (same pattern as the intermediate level at line 466).
This is the same mechanical fix that commit 56d8b59 applied to Go.
| if (firstTypeDecl?.shape.type !== "object") { | ||
| return properties; | ||
| } | ||
| currentProps = firstTypeDecl.shape.properties; | ||
|
|
||
| for (let i = 1; i < unwrapPath.length; i++) { | ||
| const segment = unwrapPath[i]; | ||
| // Skip intermediate-level non-path properties — they are part of the | ||
| // nesting structure and handled automatically during serialization. | ||
|
|
||
| // Navigate to next level | ||
| const pathProp = currentProps?.find((p) => getWireValue(p.name) === segment); | ||
| if (pathProp == null || pathProp.valueType.type !== "named") { | ||
| return properties; | ||
| } | ||
| const typeDecl = context.type.getTypeDeclaration({ | ||
| typeId: pathProp.valueType.typeId, | ||
| fernFilepath: pathProp.valueType.fernFilepath, | ||
| name: pathProp.valueType.name, | ||
| displayName: pathProp.valueType.displayName | ||
| }); | ||
| if (typeDecl?.shape.type !== "object") { | ||
| return properties; | ||
| } | ||
| currentProps = typeDecl.shape.properties; | ||
| } |
There was a problem hiding this comment.
🔴 🔴 The new TS unwrap helpers walk the type chain using only shape.properties and never consult shape.extendedProperties or walk shape.extends recursively. As a result, for any spec where a type on the unwrap path uses Fern extends: or OpenAPI allOf, the TypeScript SDK silently drops inherited fields from BOTH the CreatePaymentRequest interface AND the wire payload, while Go (fixed in this PR) and Python (uses get_all_properties_including_extensions) correctly include them. Fix: walk shape.extends recursively (or use the precomputed shape.extendedProperties) at GeneratedRequestWrapperImpl.ts:959, 980, 985 and GeneratedDefaultEndpointRequest.ts:403, 410, 423, 427 — the same file already uses getAllPropertiesIncludingExtensions for the non-unwrap path at GeneratedRequestWrapperImpl.ts:663, so the pattern is available.
Extended reasoning...
What the bug is
The new TypeScript unwrap helpers introduced in this PR walk the request body type graph using only shape.properties, never consulting shape.extendedProperties (which the IR generator precomputes for this exact purpose) and never recursively walking shape.extends. The defect appears at every level of the unwrap path — top-level, intermediate, and leaf — and affects both the generated SDK interface and the wire serialization.
Code paths verified directly:
GeneratedRequestWrapperImpl.ts:959—currentProps = firstTypeDecl.shape.properties(first path level)GeneratedRequestWrapperImpl.ts:980—currentProps = typeDecl.shape.properties(subsequent path levels)GeneratedRequestWrapperImpl.ts:984-999— leaf level iteratescurrentProps(no extension walk)GeneratedDefaultEndpointRequest.ts:403, 410, 423, 427— sameshape.properties-only pattern in the serialization expression builder
Cross-generator divergence on the same IR
- Python
inlined_request_body_parameters.py:_resolve_object_propertiesdelegates toget_all_properties_including_extensions, which traverses the chain — inherited fields are included. - Go
sdk.go:resolveObjectPropertieswas fixed in this PR (commit visible in the diff at sdk.go:4634-4655) to walkExtendsrecursively, andinlinedRequestBodyToUnwrappedObjectTypeDeclarationnow setsExtends: inlinedRequestBody.Extendsmirroring sdk.go:4101. - TypeScript was not updated to match. Same IR → different SDKs.
The same file (GeneratedRequestWrapperImpl.ts:663) already uses getAllPropertiesIncludingExtensions for the non-unwrap path's required-check, so the pattern is available — the unwrap helpers were simply written without it.
Step-by-step proof
Take this Fern Definition (or the equivalent OpenAPI allOf):
types:
BaseAttributes:
properties:
reference: string
service:
endpoints:
createPayment:
method: POST
path: /payments
request:
name: CreatePaymentRequest
body:
properties:
data:
type: object
properties:
attributes:
extends: BaseAttributes
properties:
amount: integer
request-body-unwrap: data.attributesconvertHttpRequestBody.tspopulatesInlinedRequestBody.unwrapPath = ["data", "attributes"]. The leaf type forattributeshasproperties = [amount]andextends = [BaseAttributes].getUnwrappedPropertiesat GeneratedRequestWrapperImpl.ts:929+ walks down viatypeDecl.shape.propertiesonly. At theattributesleaf,currentProps = [amount]—reference(inherited fromBaseAttributes) is dropped.- The generated
CreatePaymentRequestinterface is{ amount: number }— noreferencefield. buildUnwrappedRequestBodyExpressionat GeneratedDefaultEndpointRequest.ts:367+ likewise usescurrentTypeDecl.shape.properties(line 403, 423, 427). The wire payload is{ data: { attributes: { amount } } }—referenceis silently omitted.- Python emits
client.payment.create_payment(amount=..., reference=...)and serializes{"data":{"attributes":{"amount":...,"reference":"..."}}}. - Go (post-fix) emits a
CreatePaymentRequeststruct includingReferenceand serializes the inherited field correctly.
Reachability
extends:on inlined request bodies and on referenced objects is a first-class Fern Definition feature; the IR (addExtendedPropertiesToIr.ts) flatMapsshape.extendsand stores the result inshape.extendedProperties.- OpenAPI
allOfmaterializes toextendsin the IR viaRequestBodyConverter.ts:178(extends: requestBodyTypeShape.extends). - The PR's seed fixture (
test-definitions/fern/apis/request-body-unwrap/openapi.yml) has noallOf/extendson the unwrap path, which is why seed snapshots pass while a real-world spec using either feature silently emits a broken TS SDK.
Fix
Mirror the Go fix and the existing in-file pattern. Either:
- Read
shape.extendedProperties(precomputed by the IR generator) in addition toshape.properties, or - Walk
shape.extendsrecursively, the wayresolveObjectPropertiesdoes insdk.go:4634-4655after this PR.
Apply at all four sites in GeneratedRequestWrapperImpl.ts and GeneratedDefaultEndpointRequest.ts listed above.
| if requestBody.unwrapPath != nil { | ||
| // For unwrapped request bodies, generate custom MarshalJSON/UnmarshalJSON | ||
| // that builds/parses the nested wire format from/to flat struct fields. | ||
| writeUnwrappedMarshalJSON(f, receiver, typeName, endpoint.RequestBody.InlinedRequestBody, requestBody.unwrapPath, importPath) | ||
| writeUnwrappedUnmarshalJSON(f, receiver, typeName, endpoint.RequestBody.InlinedRequestBody, requestBody.unwrapPath, importPath) | ||
| return nil |
There was a problem hiding this comment.
🔴 / silently drop the extra-properties feature (sdk.go:3630-3635). When an inlined request body combines x-fern-request-body-unwrap with extra-properties: true, the early return after the unwrap helpers bypasses both the writeExtractExtraProperties call at sdk.go:3677-3680 (UnmarshalJSON) and the internal.MarshalJSONWithExtraProperties(..., receiver.ExtraProperties) call at sdk.go:3713-3715 (MarshalJSON). The unwrap helpers themselves never reference receiver.ExtraProperties, never call internal.MarshalJSONWithExtraProperties, and never call internal.ExtractExtraProperties, yet the struct still has the ExtraProperties field (propagated by inlinedRequestBodyToUnwrappedObjectTypeDeclaration at sdk.go:4789-4793). Net effect: arbitrary fields the user sets on req.ExtraProperties are silently dropped from the wire on marshal, and unrecognized fields in incoming payloads are silently lost on unmarshal. Fix: extend writeUnwrappedMarshalJSON to invoke internal.MarshalJSONWithExtraProperties with the outer body map plus receiver.ExtraProperties, and extend writeUnwrappedUnmarshalJSON to call ExtractExtraProperties on the top-level raw map (excluding the known wire names) into receiver.ExtraProperties. This is a fourth observable failure of the same root cause flagged in the earlier marshal-infrastructure comment (omitempty / explicitFields / date wrappers).
Extended reasoning...
Bug Summary
The new unwrap codegen short-circuits WriteRequestType after emitting writeUnwrappedMarshalJSON/writeUnwrappedUnmarshalJSON (sdk.go:3630-3635), bypassing the existing infrastructure that wires the extra-properties: true feature into the non-unwrap path. The struct still carries an ExtraProperties map[string]interface{} field — but no generated code ever reads or writes it on the unwrap path.
Code paths verified directly
Non-unwrap path wires extra-properties correctly:
UnmarshalJSON(sdk.go:3677-3680):if requestBody.extraProperties { writeExtractExtraProperties(f, literals, receiver, ...) }
MarshalJSON(sdk.go:3713-3715):if requestBody.extraProperties { return internal.MarshalJSONWithExtraProperties(explicitMarshaler, receiver.ExtraProperties) }
Unwrap path bypasses both blocks:
if requestBody.unwrapPath != nil {
writeUnwrappedMarshalJSON(...)
writeUnwrappedUnmarshalJSON(...)
return nil // <-- never reaches the extra-properties branches above
}Unwrap helpers do not reference ExtraProperties:
writeUnwrappedMarshalJSON(sdk.go:~4812-4920) builds amap[string]interface{}from known properties and returnsjson.Marshal(body). It never callsinternal.MarshalJSONWithExtraPropertiesand never readsreceiver.ExtraProperties.writeUnwrappedUnmarshalJSON(sdk.go:~4926+) parses the raw map and known segments. It never callsinternal.ExtractExtraPropertiesand never writesreceiver.ExtraProperties.
The struct still has the field. inlinedRequestBodyToUnwrappedObjectTypeDeclaration (sdk.go:4789-4793) propagates ExtraProperties: inlinedRequestBody.ExtraProperties to the synthetic ObjectTypeDeclaration, so typeVisitor.visitObjectProperties emits the ExtraProperties map[string]interface{} field on the flat struct — but it is orphaned, with no marshal/unmarshal code referencing it.
Step-by-step proof
Consider this Fern Definition (or the equivalent OpenAPI with additionalProperties: true):
service:
endpoints:
createPayment:
method: POST
path: /payments
request:
name: CreatePaymentRequest
body:
extra-properties: true
properties:
data:
type: object
properties:
attributes:
type: object
properties:
amount: integer
request-body-unwrap: data.attributes- The IR populates
InlinedRequestBody.unwrapPath = ["data", "attributes"]ANDInlinedRequestBody.ExtraProperties = true. WriteRequestTypereaches sdk.go:3630-3635, emitswriteUnwrappedMarshalJSON/writeUnwrappedUnmarshalJSON, and returns. The sdk.go:3677-3680 and sdk.go:3713-3715 branches never execute.- The generated struct has
Amount intplusExtraProperties map[string]interface{}(from the synthetic type declaration). - User code:
req := &CreatePaymentRequest{Amount: 100, ExtraProperties: map[string]interface{}{"trace_id": "abc-123"}} resp, _ := client.Payment.CreatePayment(ctx, req)
MarshalJSONoutput:{"data":{"attributes":{"amount":100}}}—trace_idis silently absent. The user has no way to add arbitrary forward-compatible fields, which is the entire purpose ofextra-properties.- Symmetric on
UnmarshalJSON: a server response{"data":{"attributes":{"amount":100}},"server_trace_id":"xyz"}leavesreq.ExtraPropertiesnil — the unrecognized field is dropped on deserialization too.
In the non-unwrap path the same spec correctly produces {"data":{...},"amount":100,"trace_id":"abc-123"} because internal.MarshalJSONWithExtraProperties splices receiver.ExtraProperties into the JSON body.
Why the seed fixture passes
The PR fixture (test-definitions/fern/apis/request-body-unwrap/openapi.yml) does not combine additionalProperties: true with x-fern-request-body-unwrap. Seed snapshots therefore exercise only the non-extra-properties unwrap path and the non-unwrap extra-properties path independently. The cross-feature combination is latent.
Impact
Forward-compatible API design (sending and receiving unknown fields) is the canonical use case for extra-properties. Customers wanting that ergonomic plus the unwrap ergonomic get silent data loss in both directions. This is a feature-combination correctness issue distinct from the three issues already flagged in #16277 (comment) — same root cause (unwrap branch reimplementing marshal instead of composing with existing infrastructure), fourth concrete failure mode.
Suggested fix
Mirror the non-unwrap pattern. At the end of writeUnwrappedMarshalJSON, gate the return on inlinedRequestBody.ExtraProperties:
if inlinedRequestBody.ExtraProperties {
f.P("return internal.MarshalJSONWithExtraProperties(body, ", receiver, ".ExtraProperties)")
} else {
f.P("return json.Marshal(body)")
}This places extras at the outermost level of the wire object (matching the semantic intent of extra-properties: arbitrary unknown fields on the top-level body, not inside the unwrap path).
At the end of writeUnwrappedUnmarshalJSON, when inlinedRequestBody.ExtraProperties is true, emit a call to internal.ExtractExtraProperties over the top-level raw map, excluding the known top-level wire names (the unwrap path root plus any sibling top-level non-path properties) and write the result into receiver.ExtraProperties.
The architectural alternative is the one already suggested in the earlier comment: stop reimplementing marshal in the unwrap path and instead reuse HandleExplicitFields / MarshalJSONWithExtraProperties / date wrappers from the non-unwrap path, with a small wrapper that builds the nested body structure. That fixes all four symptoms at once.
🔬 also observed by claude-comment-3364652015
| } | ||
| return result | ||
| } | ||
|
|
||
| // getUnwrappedBodyObjectProperties returns the flat list of ObjectProperty for an unwrapped | ||
| // request body. This includes top-level non-path properties and leaf-level properties. | ||
| func getUnwrappedBodyObjectProperties( | ||
| inlinedRequestBody *ir.InlinedRequestBody, | ||
| unwrapPath []string, | ||
| types map[common.TypeId]*ir.TypeDeclaration, | ||
| ) []*ir.ObjectProperty { | ||
| var result []*ir.ObjectProperty | ||
| pathRoot := unwrapPath[0] | ||
|
|
||
| // Top-level non-path, non-auto-fill properties | ||
| for _, prop := range inlinedRequestBody.Properties { | ||
| if prop.Name.WireValue == pathRoot { | ||
| continue | ||
| } | ||
| if getAutoFillValue(prop.ValueType, types) != "" { | ||
| continue |
There was a problem hiding this comment.
🔴 Top-level and leaf-level properties with the same wire name (e.g. id at both the request-body root and inside the leaf object on a data.attributes unwrap path) are concatenated into a single property list with no dedup, producing duplicate Go struct fields, duplicate Python kwargs (SyntaxError), and duplicate TypeScript interface members (TS2300) — i.e. uncompilable SDK output across all three generators. The collision sites are getUnwrappedBodyObjectProperties (sdk.go:4716-4778), _get_unwrapped_properties (inlined_request_body_parameters.py:145-181), and getUnwrappedProperties (GeneratedRequestWrapperImpl.ts:923-1003). Fix: surface a validation error in the importer (preferred, symmetric with other unwrap-path misconfiguration handling) or deterministically rename one side.
Extended reasoning...
What the bug is
When the unwrap path is set, each generator builds a flat list of properties for the SDK signature by concatenating two sources: (a) top-level non-path properties of the inlined request body, and (b) the leaf object's properties at the end of the unwrap path. None of the three generators deduplicate against wire name when joining these lists. If both sources contain a property with the same wire name, the resulting flat list has two entries with the same name, and the downstream codegen unconditionally emits one declaration per entry.
Code paths
Go generators/go/internal/generator/sdk.go getUnwrappedBodyObjectProperties:
- lines 4725-4737 iterate
inlinedRequestBody.Propertiesand append each non-path, non-auto-fill property toresult - lines 4771-4777 iterate the leaf object's properties (via
resolveObjectProperties) and append each non-auto-fill property to the sameresultslice — no dedup against the wire names already inresult inlinedRequestBodyToUnwrappedObjectTypeDeclaration(sdk.go:4783) returns this as a syntheticObjectTypeDeclaration, whichvisitObjectProperties(model.go) iterates one-to-one to emitgoExportedFieldName(prop.Name.PascalCase.UnsafeName)fields on the generated struct.
Python generators/python/.../inlined_request_body_parameters.py _get_unwrapped_properties:
- lines 150-152 append top-level non-path, non-auto-fill props to
result - lines 169-179 append leaf-level non-auto-fill props to the same
resultlist with no dedup get_parametersthen creates oneNamedFunctionParameterper entry via_get_property_name→snake_case.safe_name, producing two parameters with the same Python name in the function signature
TypeScript generators/typescript/.../GeneratedRequestWrapperImpl.ts getUnwrappedProperties:
- lines 931-941 push top-level non-path, non-literal properties to
properties - lines 984-999 push leaf-level non-literal properties to the same array with no dedup
- the resulting
Property[]becomes the interface members ofCreatePaymentRequest— duplicate members triggerTS2300 Duplicate identifier
Step-by-step proof
Consider a JSON:API-style spec where a top-level idempotency key shares a name with a business-object id at the leaf:
post:
x-fern-request-body-unwrap: data.attributes
requestBody:
content:
application/json:
schema:
required: [id, data]
properties:
id: { type: string } # client-generated idempotency key
data:
type: object
required: [attributes]
properties:
attributes:
type: object
required: [id, amount]
properties:
id: { type: string } # business entity id
amount: { type: integer }getUnwrappedBodyObjectPropertieswalks top-level properties.idis non-path and not auto-fillable (it is a string primitive, not literal/single-value-enum), so it is appended toresult.datamatches the path root and is skipped.- The function then walks into
data.attributesand collects leaf properties.id(the business id) is also a non-auto-fillable string, so it is appended toresult.resultnow contains two entries withName.WireValue == "id". inlinedRequestBodyToUnwrappedObjectTypeDeclarationwraps this in anObjectTypeDeclarationwith twoidproperties.visitObjectPropertiesemits the struct literal:type CreatePaymentRequest struct { Id string `json:"id"` Id string `json:"id"` // duplicate field name — Go compile error Amount int `json:"amount"` }
go build ./...fails withduplicate field name 'Id' in struct literal.- The Python path produces
def create_payment(self, *, id: str, id: str, amount: int, ...) -> Payment:— Python's parser rejects this withSyntaxError: duplicate argument 'id' in function definitionat module import. - The TS path produces
interface CreatePaymentRequest { id: string; id: string; amount: number }—tscrejects withTS2300: Duplicate identifier 'id'.
Why existing safeguards do not catch it
The PR's seed fixture (test-definitions/fern/apis/request-body-unwrap/openapi.yml) declares top-level properties [data, metadata] and leaf properties [amount, currency, description, source_account_id, destination_account_id] — no overlapping wire names. Seed compile and snapshot tests pass because the collision condition is never exercised. There is no validation step that rejects the unwrap path when it would induce a collision, and the helpers concatenate the two lists without inspection.
Impact
Any real-world spec where a property name happens to repeat at the top level and inside the leaf object on the unwrap path produces an SDK that fails to compile in Go, fails to parse in Python, and fails type-checking in TypeScript. The JSON:API discriminator pattern (idempotency key at top, entity id at leaf), pagination cursors, and shared metadata fields are realistic triggers. The PR's own importer wires the extension fully, so any user encountering this pattern hits the failure on first fern generate.
Fix
Two viable approaches, matching the precedent set by other unwrap-path misconfiguration handling:
- Validate in the importer — detect collisions in
x-fern-request-body-unwrap.ts/buildEndpoint.tsand surface a clear error to the user (preferred; symmetric with how other unwrap-path misconfigurations are reported). - Deterministically rename one side — e.g., prefix top-level names with the request-body name on collision. More complex, no precedent in the existing codebase.
A minimal generator-side fix would be to detect collisions in getUnwrappedBodyObjectProperties / _get_unwrapped_properties / getUnwrappedProperties and error out instead of silently emitting duplicates. The importer-side validation is preferable because the diagnostic is more actionable and the same error surfaces uniformly across all three generators.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| const serverName = getExtension<string>(operation, FernOpenAPIExtension.SERVER_NAME_V2); | ||
| const rawUnwrapPath = getExtension<string>(operation, FernOpenAPIExtension.REQUEST_BODY_UNWRAP); | ||
| const requestBodyUnwrapPath = | ||
| rawUnwrapPath != null ? rawUnwrapPath.split(".").filter((segment) => segment.length > 0) : undefined; |
There was a problem hiding this comment.
🟡 The legacy OpenAPI importer crashes with an opaque TypeError: rawUnwrapPath.split is not a function when a user authors x-fern-request-body-unwrap as a YAML list (e.g. ["data", "attributes"]) instead of a dot-separated string. getExtension<string> is just a TypeScript-only cast — it returns whatever YAML/JSON value the user wrote, and the call to .split(".") at convertHttpOperation.ts:326-328 has no runtime guard. The 3.1 importer at x-fern-request-body-unwrap.ts:32-37 already validates typeof extensionValue !== "string" and routes to errorCollector — mirror that check here so the legacy and 3.1 paths behave consistently on malformed input.
Extended reasoning...
What the bug is
The legacy OpenAPI importer reads the new x-fern-request-body-unwrap extension and immediately calls .split(".") on the result with no runtime type validation:
// packages/cli/api-importers/openapi/openapi-ir-parser/src/openapi/v3/converters/operation/convertHttpOperation.ts:326-328
const rawUnwrapPath = getExtension<string>(operation, FernOpenAPIExtension.REQUEST_BODY_UNWRAP);
const requestBodyUnwrapPath =
rawUnwrapPath != null ? rawUnwrapPath.split(".").filter((segment) => segment.length > 0) : undefined;getExtension<T> (packages/cli/api-importers/openapi/openapi-ir-parser/src/getExtension.ts:25-35) does not validate the runtime type — the <string> parameter is purely a TypeScript cast over (object as any)[extension]. Whatever value the user authored in YAML/JSON flows through verbatim.
Why this matters
The 3.1 importer counterpart (packages/cli/api-importers/openapi-to-ir/src/extensions/x-fern-request-body-unwrap.ts:32-37) — added in the same PR — correctly validates the value:
if (typeof extensionValue !== "string") {
this.context.errorCollector.collect({
message: 'Expected a dot-separated string for x-fern-request-body-unwrap (e.g. "data.attributes")',
path: this.breadcrumbs
});
return undefined;
}So the same misconfigured spec produces a clean, actionable diagnostic in the 3.1 path and an opaque TypeError stack trace in the legacy path. The legacy pipeline is the path OpenAPI 3.0 specs (and 3.1 specs not routed to the new converter) go through, and this PR fully wires the legacy pipeline through buildEndpoint.ts:131-133, so the legacy code is live.
Step-by-step proof
Given an OpenAPI spec where a user — reasonably — authors the extension as a YAML list mirroring the IR's parsed-segment shape:
paths:
/payments:
post:
x-fern-request-body-unwrap:
- data
- attributes
requestBody: ...getExtension<string>(operation, FernOpenAPIExtension.REQUEST_BODY_UNWRAP)returns the raw JS array["data", "attributes"](not null, not a string).rawUnwrapPath != nullevaluates totrue.rawUnwrapPath.split(".")throwsTypeError: rawUnwrapPath.split is not a functionbecause arrays have nosplitmethod.- The exception aborts
convertHttpOperationand — because there is no enclosing try/catch around this property — aborts the entire OpenAPI import for the workspace.
The user sees a stack trace pointing into Fern internals with no indication of which extension was misconfigured. The 3.1 importer, by contrast, would surface Expected a dot-separated string for x-fern-request-body-unwrap (e.g. "data.attributes") with the operation's breadcrumb path.
How to fix
Mirror the 3.1 importer pattern. Before calling .split(...), validate the runtime type:
const rawUnwrapPath = getExtension<unknown>(operation, FernOpenAPIExtension.REQUEST_BODY_UNWRAP);
let requestBodyUnwrapPath: string[] | undefined;
if (rawUnwrapPath != null) {
if (typeof rawUnwrapPath !== "string") {
context.logger.error(
`Expected a dot-separated string for ${FernOpenAPIExtension.REQUEST_BODY_UNWRAP} (e.g. "data.attributes")`
);
} else {
requestBodyUnwrapPath = rawUnwrapPath.split(".").filter((segment) => segment.length > 0);
}
}(The exact reporting mechanism should match how this importer surfaces other malformed-extension diagnostics — context.logger or the relevant taskContext — rather than errorCollector which is specific to the 3.1 importer's plumbing.)
Severity
Marking this as a nit. The failure mode is a hard crash at import time rather than silent data corruption or wrong SDK output, and the trigger requires a user authoring error. But the fix is mechanical (one typeof guard) and brings the legacy importer to parity with the 3.1 path that this same PR introduces — the asymmetry is a meaningful DX gap.
| // Navigate to next level | ||
| for _, p := range objProps { | ||
| if p.Name.WireValue == segment { | ||
| currentTypeRef = p.ValueType | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 🟡 writeUnwrappedMarshalJSON intermediate-level navigation missing found-check. At sdk.go:4881-4887, the loop that walks one unwrapPath segment further sets currentTypeRef = p.ValueType on match and breaks, but never tracks whether the segment was found. If a path segment doesn't exist on objProps (user typo in x-fern-request-body-unwrap), currentTypeRef silently retains its previous value and the loop continues against the wrong type. This is asymmetric with the two peer sites in the same file: getUnwrappedBodyObjectProperties (sdk.go:4757-4767) returns early, and writeUnwrappedUnmarshalJSON (sdk.go:4946-4961, just fixed in 56d8b59) breaks early. Fix is mechanical — mirror the found := false / if !found { break } pattern from the other two sites.
Extended reasoning...
What the bug is
The intermediate-level navigation loop in writeUnwrappedMarshalJSON at sdk.go:4881-4887 reads:
// Navigate to next level
for _, p := range objProps {
if p.Name.WireValue == segment {
currentTypeRef = p.ValueType
break
}
}There is no found flag and no early exit from the outer for i := 1; i < len(unwrapPath); i++ loop. If segment is absent from objProps, currentTypeRef keeps pointing at the previous level's type and the outer loop continues to the next segment against the wrong type.
Asymmetry with the two peer sites
The same file has two other navigation loops over the unwrap path, both of which guard against a missing segment:
getUnwrappedBodyObjectPropertiesat sdk.go:4757-4767:found := false for _, p := range objProps { if p.Name.WireValue == segment { currentTypeRef = p.ValueType found = true break } } if !found { return result }
writeUnwrappedUnmarshalJSONat sdk.go:4946-4961 (added by commit 56d8b59 in this PR):found := false for _, p := range objProps { if p.Name.WireValue == unwrapPath[i] { leafTypeRef = p.ValueType found = true break } } if !found { break }
The MarshalJSON twin of that fix is missing.
Step-by-step proof
- User authors
x-fern-request-body-unwrap: "data.wrong.attributes"against a schema wheredatahas{attributes: object{amount, currency}}(typo:wrongdoesn't exist ondata). - The importer accepts the value as a non-empty string — the fern-definition validator visits
request-body-unwrapwithnoop(visitHttpService.ts:314) andFernRequestBodyUnwrapExtensiononly checkstypeof extensionValue === "string"and that the split has at least one segment. Path-vs-schema reachability is never checked. getUnwrappedBodyObjectPropertiesearly-returns ati=1with only top-level non-path properties. The syntheticObjectTypeDeclarationhas noAmount/Currencyfields, so the generatedCreatePaymentRequeststruct has no leaf fields.writeUnwrappedMarshalJSONwalks past the bad segment ati=1(loop at 4881-4887 doesn't exit). Ati=2,attributesdoes match ondata's actual type, socurrentTypeRefbecomesattributes' type. The leaf walk at 4892 then resolves to{amount, currency}and emitsleaf["amount"] = c.Amount/leaf["currency"] = c.Currencyfor fields that don't exist on the struct.go build ./...fails withundefined: c.Amount,undefined: c.Currency.
Why existing safeguards don't catch it
- The fern-definition validator is a noop for
request-body-unwrap. - The OpenAPI
FernRequestBodyUnwrapExtensiononly validates the path is a non-empty string after splitting on.. - The seed fixture's unwrap paths (
data.attributes,attributes) are all valid, so seed CI doesn't exercise the typo case. - The recently-fixed UnmarshalJSON path (56d8b59) and the original
getUnwrappedBodyObjectPropertiesboth early-exit, masking the asymmetry until the user actually misconfigures.
How to fix
Mirror the existing pattern from sdk.go:4946-4961 at sdk.go:4881-4887:
found := false
for _, p := range objProps {
if p.Name.WireValue == segment {
currentTypeRef = p.ValueType
found = true
break
}
}
if !found {
break
}This is the same mechanical change applied to UnmarshalJSON in 56d8b59, and aligns the three navigation sites in this file.
Severity
Nit-to-normal. Real defect — the trigger is a user typo, but the failure is a non-compiling Go SDK at the first fern generate, not silent data corruption. The fix is one-line mechanical and matches an in-file precedent that the PR author just established.
Description
Adds a new OpenAPI extension
x-fern-request-body-unwrapthat tells generators to flatten nested request body properties into the SDK method signature while still serializing to the full nested wire format.Example — an API that expects
{"data":{"attributes":{"amount":100}}}:Before:
client.payments.create(data={"attributes":{"amount":100}})After:
client.payments.create(amount=100, currency="USD")Changes Made
IR + OpenAPI importers
unwrapPath: list<string>toInlinedRequestBodyin IR schema (v67.4.0)x-fern-request-body-unwrapparsed in both OpenAPI-to-IR and legacy pipelinesundefinedrequest-body-unwrap) and IR generatorTypeScript SDK generator
GeneratedRequestWrapperImpl: walks type graph to expose only leaf + top-level non-path propertiesGeneratedDefaultEndpointRequest: builds nested object literal for wire serialization, auto-includes literal values at intermediate levelsPython SDK generator
InlinedRequestBodyParameters: whenunwrapPathpresent, resolves leaf type properties and exposes them as flat method paramsGo SDK generator
sdk.go:getUnwrapPath()extracts path from IR extra properties;getUnwrappedBodyObjectProperties()walks type graphCreatePaymentRequeststruct with customMarshalJSON/UnmarshalJSONthat reconstructs nested wire formatdata.attributes) and single-level paths (attributes)Test fixture + seed snapshots
test-definitions/fern/apis/request-body-unwrap/— OpenAPI spec with 2-level (data.attributes) and 1-level (attributes) unwrap endpointsts-sdk,python-sdk,go-sdkTesting
go-sdkverified locally,ts-sdk/python-sdksnapshots from prior runs)go build ./...compiles cleanlyLink to Devin session: https://app.devin.ai/sessions/8399553af5914efa85591ffb27621428
Requested by: @cade-fern