Skip to content

Fix Python codegen synthetic permission approval names - #1652

Merged
SteveSandersonMS merged 4 commits into
github:mainfrom
abhinavgautam01:fix-python-codegen-long-type-names
Jul 31, 2026
Merged

Fix Python codegen synthetic permission approval names#1652
SteveSandersonMS merged 4 commits into
github:mainfrom
abhinavgautam01:fix-python-codegen-long-type-names

Conversation

@abhinavgautam01

Copy link
Copy Markdown
Contributor

Summary

Fixes Python codegen emitting an unintended quicktype-synthesized permission approval class name, PermissionDecisionApproveForIonApproval, caused by structural merging of similar permission approval schemas.

The generator now removes unreferenced synthetic dataclasses after union post-processing, preserving the intended schema-named union aliases such as PermissionDecisionApproveForSessionApproval and PermissionDecisionApproveForLocationApproval.

Fixes #1125.

Changes

  • Added Python codegen cleanup for unused synthetic dataclasses
  • Regenerated python/copilot/generated/rpc.py
  • Added regression coverage to ensure the bad synthesized name is not exported while the intended long names remain available

Tests

  • npm run generate:python
  • python3 -m pytest python/test_codegen_type_names.py
  • PATH=/Users/gautam/.nvm/versions/node/v22.14.0/bin:$PATH python3 -m pytest python/test_*.py
  • git diff --check

@abhinavgautam01
abhinavgautam01 requested a review from a team as a code owner June 13, 2026 09:05
Copilot AI review requested due to automatic review settings June 13, 2026 09:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates the Python codegen post-processing to remove unused “synthetic” Quicktype-emitted @dataclass classes so they no longer appear in the generated module exports.

Changes:

  • Added a post-processing pass to delete unreferenced synthetic @dataclass blocks from generated Python.
  • Wired the new pass into the Python codegen pipeline using the schema’s known definition names.
  • Added a Python regression test and updated the generated rpc.py output accordingly.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File Description
scripts/codegen/python.ts Adds removeUnusedSyntheticPythonDataclasses() and applies it during final Python code post-processing.
python/test_codegen_type_names.py Adds a regression test asserting a known synthetic type is not exported while real types remain exported.
python/copilot/generated/rpc.py Regenerated output removing the unused synthetic dataclass and its __all__ export entry.

Comment thread scripts/codegen/python.ts Outdated
Comment thread scripts/codegen/python.ts Outdated
Comment thread scripts/codegen/python.ts Outdated
@SteveSandersonMS
SteveSandersonMS force-pushed the fix-python-codegen-long-type-names branch from 5755075 to 4aa1f83 Compare July 31, 2026 14:57
Copilot AI review requested due to automatic review settings July 31, 2026 14:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 2/3 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 31, 2026 15:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

Suppressed comments (1)

python/test_codegen_type_names.py:13

  • This only proves that at least one matching export exists. The loader test below can still pass when either schema-named alias is missing from __all__, because it calls separate _load_* functions and never accesses the aliases themselves. Assert both exact session and location alias names here so the regression test covers the stated public exports.
    assert approval_exports
  • Files reviewed: 2/3 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Comment thread scripts/codegen/python.ts Dismissed

@SteveSandersonMS SteveSandersonMS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@SteveSandersonMS
SteveSandersonMS added this pull request to the merge queue Jul 31, 2026
Merged via the queue into github:main with commit 20be013 Jul 31, 2026
55 of 57 checks passed
@mohamadhosenzademohamad-oss

Copy link
Copy Markdown

Summary

Fixes Python codegen emitting an unintended quicktype-synthesized permission approval class name, PermissionDecisionApproveForIonApproval, caused by structural merging of similar permission approval schemas.

The generator now removes unreferenced synthetic dataclasses after union post-processing, preserving the intended schema-named union aliases such as PermissionDecisionApproveForSessionApproval and PermissionDecisionApproveForLocationApproval.

Fixes #1125.

Changes

  • Added Python codegen cleanup for unused synthetic dataclasses
  • Regenerated python/copilot/generated/rpc.py
  • Added regression coverage to ensure the bad synthesized name is not exported while the intended long names remain available

Tests

  • npm run generate:python
  • python3 -m pytest python/test_codegen_type_names.py
  • PATH=/Users/gautam/.nvm/versions/node/v22.14.0/bin:$PATH python3 -m pytest python/test_*.py
  • git diff --check

edburns pushed a commit that referenced this pull request Aug 4, 2026
* Fix Python codegen synthetic permission approval names

* Address Python codegen cleanup review feedback

* Regenerate python rpc after rebase

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Replace brittle python codegen symbol test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
edburns pushed a commit that referenced this pull request Aug 5, 2026
* Fix Python codegen synthetic permission approval names

* Address Python codegen cleanup review feedback

* Regenerate python rpc after rebase

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Replace brittle python codegen symbol test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

Python codegen: quicktype mangles long type names

5 participants