Fix generated client wire names for renamed parameters - #51
Merged
Volv-G merged 1 commit intoAug 28, 2026
Merged
Conversation
The static generator emitted the collision-avoided Python local name as the
JSON/query/path key. A request-body field literally named `body` is renamed to
the local `body_2`, so `admin_post_notices("text", ...)` sent `{"body_2": ...}`
and omitted the required `body` field; `admin_patch_notices` had the same
defect. The dynamic transport was already correct because it keys the payload
off `CliParameter.original_name`.
Thread `original_name` through `_param_signature` as a `_ParamName`
(local_name, wire_name) pair so the emitted `json_data`, `params`, and
`path_params` literals all use the schema wire name while the method signature
keeps the collision-free local. Only a missing name falls back to the local
identifier, so schema-valid empty property names keep their exact spelling.
Also emit `# type: ignore[misc, assignment]` for the pydantic v1 `ConfigDict`
fallback in the runtime template; `[assignment]` alone left downstream
regenerations reporting `runtime.py: Cannot assign to a type [misc]`.
Regenerated the checked-in runtime module from the snapshot.
Assisted-By: devx/9c659b46-4731-49d5-8fa1-b00487ef84e8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two generator defects surfaced by a Binks review of a downstream codegen-only regeneration (Shopify/discovery#33919, follow-up r3880721261). Both are generator-owned: the downstream
generated/tree is only ever produced by regeneration, so hand-edits there would be reverted.1. Renamed parameters were serialized with the Python local name.
_request_body_parametersreservesbodyinused_names, so a schema field actually namedbodybecomes the Python identifierbody_2.api_transport.pyhandles this correctly - it keys the wire payload offCliParameter.original_namewhile matching arguments againstlocal_name. The static generator did not:_param_signature/_dict_literalusedlocal_namefor both the parameter and the JSON key. A normal call likeadmin_post_notices("text", title=..., variant=...)therefore sent{"body_2": ...}and omitted the server's requiredbody.admin_patch_noticeshad the same defect for body updates.The same mistake applied to the emitted
params=andpath_params=literals; it just was not observable in the current schema becausebody_2is the only renamed parameter today.2. The runtime template's
type: ignoredid not cover the required mypy error codes.generate_runtime()emittedConfigDict = None # type: ignore[assignment], which leaves downstream regenerations reportingruntime.py: Cannot assign to a type [misc]against an otherwise clean base.Fix
original_namethrough_param_signatureas a_ParamName(local_name, wire_name)pair, so the generatedjson_data,params, andpath_paramsliterals all use the OpenAPI wire name while the method signature keeps the collision-free Python local. Only a genuinely missing name falls back to the local identifier, so schema-valid empty property names keep their exact wire spelling.# type: ignore[misc, assignment]in the runtime template and regenerate the checked-intangle_api/generated/runtime.py.Non-renamed parameters emit byte-identical literals, so the only change to the checked-in generated output is the one-line
type: ignore.Generated output for a colliding schema now reads:
Tests
Three focused generator regressions in
tests/test_codegen.py:test_generate_operations_uses_wire_names_for_renamed_parameters- renamed body (body->body_2), query (order.by,token->token_2) and path (notice-id) names; asserts both the emitted literals and the actual runtime call payloads through an imported generated module.test_generate_runtime_type_ignore_covers_required_mypy_error_codes- pins the runtime template's error codes and thatgenerate()writes it verbatim.test_generate_operations_preserves_empty_wire_names- an empty JSON property name is schema-valid and must not be replaced by the generated local.Checks
uv run pytest- 1093 passedgit diff --checkclean,uv lock --checkcleanruntime.py(with pydantic installed): clean, including under--warn-unused-ignores. Reverting to[assignment]alone reproducesCannot assign to a type [misc].Downstream consumers need a submodule pin bump plus a regeneration to pick this up.