Fix Python codegen synthetic permission approval names - #1652
Conversation
There was a problem hiding this comment.
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
@dataclassblocks 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.pyoutput 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. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
5755075 to
4aa1f83
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
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
SteveSandersonMS
left a comment
There was a problem hiding this comment.
Thanks @abhinavgautam01!
|
* 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>
* 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>
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
PermissionDecisionApproveForSessionApprovalandPermissionDecisionApproveForLocationApproval.Fixes #1125.
Changes
python/copilot/generated/rpc.pyTests
npm run generate:pythonpython3 -m pytest python/test_codegen_type_names.pyPATH=/Users/gautam/.nvm/versions/node/v22.14.0/bin:$PATH python3 -m pytest python/test_*.pygit diff --check