Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94d4b6bc0a
ℹ️ 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".
| ): | ||
| if argument is not None and argument.annotation is not None: | ||
| self.visit(argument.annotation) |
There was a problem hiding this comment.
Reserve nested function params before reusing outer local names
The new local-scope allocator only visits nested function argument annotations here, so nested parameter names themselves are never reserved. In rename_arguments=True mode this lets an outer binding be renamed to a nested parameter name, which breaks closures when the inner body references the outer variable (e.g. outer(long_value) with inner(a): return long_value + a is rewritten so both names become a, changing results). This regression is introduced by the new per-scope reuse logic and can silently alter runtime behavior.
Useful? React with 👍 / 👎.
| def visit_Lambda(self, node): | ||
| return None |
There was a problem hiding this comment.
Include lambda/comprehension bindings in local-name reservation
By returning early for expression scopes, the collector ignores lambda/comprehension-local bindings when building used_names. That allows an enclosing variable to be renamed to the same identifier used inside a lambda/comprehension, which changes name resolution and output (for example, (lambda a: long_value + a) can become (lambda a: a + a)). This is a semantic regression triggered by local-name reuse in functions that contain these expressions.
Useful? React with 👍 / 👎.
Summary
Why
The previous package-mode compression work still left a lot of gzip redundancy on the table because local names stayed unnecessarily unique across functions. Reusing short locals per scope improves both raw source size and compressed package size. Folding in the unresolved review fixes at the same time keeps the aggressive renaming path correct.
Impact
98,181bytes to24,288bytes (75.3%smaller).tar.gznow goes from23,101bytes to8,355bytes (63.8%smaller)Validation
PYTHONPATH=. .venv/bin/python -m pytestPYTHONPATH=. .venv/bin/python scripts/regenerate_examples.py --check78tests passed