Skip to content

Fix compatibility on benchmark repos#20

Merged
alvinwan merged 2 commits intomainfrom
dev/alvin/repo-compat-fixes
Apr 8, 2026
Merged

Fix compatibility on benchmark repos#20
alvinwan merged 2 commits intomainfrom
dev/alvin/repo-compat-fixes

Conversation

@alvinwan
Copy link
Copy Markdown
Owner

@alvinwan alvinwan commented Apr 8, 2026

Summary

  • fix scope rewriting so global and nonlocal declarations stay aligned with renamed bindings
  • preserve helper insertion order around from __future__ imports and stop whitespace compaction from corrupting f-strings or match / case
  • keep placeholder ... bodies valid after docstring stripping and add regression tests for the new compatibility cases

Validation

  • .venv/bin/python -m pytest -q
  • aggressive package-mode compile sweep over .bench-repos/{attrs,click,flask,jinja,pyminifier-tool,pytest,requests} with --rename-modules --rename-global-variables --rename-arguments

@alvinwan alvinwan marked this pull request as ready for review April 8, 2026 06:08
@alvinwan alvinwan merged commit f01f5b8 into main Apr 8, 2026
10 checks passed
@alvinwan alvinwan deleted the dev/alvin/repo-compat-fixes branch April 8, 2026 06:08
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1d60b8e88a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread pymini/pymini.py
Comment on lines +1022 to +1024
if self.local_rename_scopes and not self._is_node_global(node):
node.name = self._rename_local_identifier(node.name)
elif node.name not in self.mapping_values:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep nested function names unique across module

Using _rename_local_identifier for non-global FunctionDefs allows a nested function to reuse a top-level renamed name (for example both becoming a). Because call-signature metadata is stored in self.callable_argument_infos by identifier, the later nested definition overwrites the top-level entry, so subsequent keyword calls to the top-level function are no longer rewritten correctly and can fail at runtime with TypeError when rename_arguments=True.

Useful? React with 👍 / 👎.

Comment thread pymini/pymini.py
Comment on lines +883 to +885
if self.local_rename_scopes and not self._is_node_global(node):
node.name = self._rename_local_identifier(node.name)
elif node.name not in self.mapping_values:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Prevent nested class renames from shadowing global class metadata

Renaming non-global ClassDefs via _rename_local_identifier can reuse a renamed top-level class name, but class method/attribute metadata is keyed only by class identifier (class_method_argument_infos / class_member_mappings). When a nested class later overwrites that key, method-call rewriting for the top-level class can use the wrong signature and emit invalid calls (e.g., unconverted keywords), causing runtime errors under rename_arguments=True.

Useful? React with 👍 / 👎.

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