Skip to content

Fix generated client wire names for renamed parameters - #51

Merged
Volv-G merged 1 commit into
masterfrom
piforge/tangle-pipeline-crud/fix-openapi-codegen-wire-name-al-e2e892c
Aug 28, 2026
Merged

Fix generated client wire names for renamed parameters#51
Volv-G merged 1 commit into
masterfrom
piforge/tangle-pipeline-crud/fix-openapi-codegen-wire-name-al-e2e892c

Conversation

@Volv-G

@Volv-G Volv-G commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

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_parameters reserves body in used_names, so a schema field actually named body becomes the Python identifier body_2. api_transport.py handles this correctly - it keys the wire payload off CliParameter.original_name while matching arguments against local_name. The static generator did not: _param_signature/_dict_literal used local_name for both the parameter and the JSON key. A normal call like admin_post_notices("text", title=..., variant=...) therefore sent {"body_2": ...} and omitted the server's required body. admin_patch_notices had the same defect for body updates.

The same mistake applied to the emitted params= and path_params= literals; it just was not observable in the current schema because body_2 is the only renamed parameter today.

2. The runtime template's type: ignore did not cover the required mypy error codes.

generate_runtime() emitted ConfigDict = None # type: ignore[assignment], which leaves downstream regenerations reporting runtime.py: Cannot assign to a type [misc] against an otherwise clean base.

Fix

  • Thread original_name through _param_signature as a _ParamName(local_name, wire_name) pair, so the generated json_data, params, and path_params literals 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.
  • Emit # type: ignore[misc, assignment] in the runtime template and regenerate the checked-in tangle_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:

def admin_notices(self, body_2: Any, order_by: Any = None, ...):
    return self._request_json(
        'POST', '/api/admin/notices',
        path_params=None,
        params={'order.by': order_by, 'token': token_2},
        json_data={**(body or {}), **{**{'body': body_2}, ...}},
        ...
    )

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 that generate() 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 passed
  • git diff --check clean, uv lock --check clean
  • mypy on the regenerated runtime.py (with pydantic installed): clean, including under --warn-unused-ignores. Reverting to [assignment] alone reproduces Cannot assign to a type [misc].
  • Independent local review completed; the empty-wire-name edge case above was found and fixed in review.

Downstream consumers need a submodule pin bump plus a regeneration to pick this up.

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
@Volv-G
Volv-G requested a review from Ark-kun as a code owner August 28, 2026 13:27
@Volv-G
Volv-G merged commit 1897416 into master Aug 28, 2026
6 checks passed
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