scripts/dis_dump.py (1)

395-400: Minor: consider contextlib.ExitStack or a small helper for cleaner stdout/file fallback.

The current try/finally is correct, but the two-branch if args.output on lines 395 and 399 can be consolidated using contextlib.nullcontext(sys.stdout) so that the with semantics handle closing uniformly.

♻️ Proposed refactor

-    output = open(args.output, "w", encoding="utf-8") if args.output else sys.stdout
-    try:
-        json.dump(results, output, ensure_ascii=False, separators=(",", ":"))
-    finally:
-        if args.output:
-            output.close()
+    import contextlib
+    output_cm = (
+        open(args.output, "w", encoding="utf-8")
+        if args.output
+        else contextlib.nullcontext(sys.stdout)
+    )
+    with output_cm as output:
+        json.dump(results, output, ensure_ascii=False, separators=(",", ":"))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/dis_dump.py` around lines 395 - 400, Replace the try/finally
file-handle pattern around json.dump with a context manager that unifies stdout
and file handling: use contextlib.nullcontext(sys.stdout) (or
contextlib.ExitStack) to open args.output when present and yield sys.stdout
otherwise, then perform json.dump(results, output, ...) inside a single with
block; reference the variables/functions output, args.output, and json.dump to
locate and modify the existing block.

scripts/compare_bytecode.py (1)

194-199: stdout referenced via locals() after a timeout path that never sets it.

On the timeout branch (lines 179–186) data = None is assigned but stdout is not, hence the "stdout" in locals() guard on line 197. This works, but the flow is fragile — any refactor that adds another exception path will need to remember this invariant. A plain stdout = b"" at the top of the function (mirroring the pattern in _run_sync_dump) would eliminate the locals() check and make the intent explicit.

♻️ Suggested tidy

 def _finish_one(job):
     """Wait for a single dis_dump.py process and return parsed JSON."""
     proc = job["proc"]
     expected = {relpath for relpath, _ in job["targets"]}
+    stdout = b""
     try:
         stdout = proc.communicate(timeout=600)[0]
     except subprocess.TimeoutExpired:
         ...
         data = None
     else:
         data = _load_dump_output(job["output_file"])
     finally:
         ...
 
-    stray = stdout.decode(errors="replace").strip() if "stdout" in locals() else ""
+    stray = stdout.decode(errors="replace").strip()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/compare_bytecode.py` around lines 194 - 199, The function currently
uses "stdout" only in some branches and guards it with '"stdout" in locals()'
which is fragile; initialize stdout = b"" at the top of the function (same
pattern as _run_sync_dump) so stdout is always defined, remove the locals()
check and simply decode stdout (e.g. stray =
stdout.decode(errors="replace").strip()) and check if stray to emit the warning;
update references in the timeout/error handling in this function accordingly
(keep the proc.returncode check and stray warning logic) so no code paths leave
stdout undefined.

crates/codegen/src/symboltable.rs (2)

2227-2237: Minor: comment narrower than the actual gate.

The comment states the exclusion is for "annotation scopes nested in classes that can see class scope", but the check is parent_can_see_class_scope, which is also set on TypeParams scopes inside a class (see enter_type_param_block, line 1079) and on scan_type_param_bound_or_default scopes. So generic-class type-parameter scopes and TypeVar bound/default scopes nested in a class will also skip inlining. If that is intentional (it matches CPython), consider broadening the comment to avoid future confusion.

📝 Suggested wording

         // PEP 709: Mark non-generator comprehensions for inlining.
-        // CPython's symtable marks all non-generator comprehensions for
-        // inlining, except annotation scopes nested in classes that can see
-        // class scope.
+        // CPython's symtable marks all non-generator comprehensions for
+        // inlining, except when the enclosing scope can see class scope
+        // (annotation, type-parameter, and TypeVar bound/default scopes
+        // nested in a class).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/codegen/src/symboltable.rs` around lines 2227 - 2237, The comment is
too narrow: the gate checking parent_can_see_class_scope (used with
is_generator) also applies to TypeParams and type-var bound/default scopes (see
enter_type_param_block and scan_type_param_bound_or_default), so update the
comment above the inlining logic to state that any nested annotation-like scopes
that can see class scope (including generic-class type-parameter scopes and
TypeVar bound/default scopes) are excluded from comp_inlined rather than only
“annotation scopes nested in classes that can see class scope”; leave the logic
using parent_can_see_class_scope, is_generator, and setting comp_inlined
unchanged.

---

570-607: Optional: fuse the two post-analysis passes over symbol_table.symbols.

The pass at 572-576 must run before analyze_symbol (line 579) so that the Unknown + COMP_CELL branch at 689-701 sees the flag. That's fine. However, the third pass at 600-607 (LOCAL→CELL promotion) can be folded into the analyze_symbol loop at 579-586 since its decision depends only on the just-resolved symbol.scope, inlined_cells, and promote_inlined_cells_to_cell (which is constant per call). This also lets the free-var collection at 583-585 see the promoted Cell scope in one pass, which may matter if a future change relies on Cell symbols appearing in newfree consistently.

♻️ Sketch

-        for symbol in symbol_table.symbols.values_mut() {
-            if inlined_cells.contains(&symbol.name) {
-                symbol.flags.insert(SymbolFlags::COMP_CELL);
-            }
-        }
-
-        // Analyze symbols in current scope
-        for symbol in symbol_table.symbols.values_mut() {
-            self.analyze_symbol(symbol, symbol_table.typ, sub_tables, class_entry)?;
-
-            // Collect free variables from this scope
-            if symbol.scope == SymbolScope::Free || symbol.flags.contains(SymbolFlags::FREE_CLASS) {
-                newfree.insert(symbol.name.clone());
-            }
-        }
-
-        // PEP 709 / CPython symtable.c:
-        // - only promote LOCAL -> CELL in function-like scopes, where
-        //   analyze_cells() runs. Module and class scopes keep their normal
-        //   scope and rely on DEF_COMP_CELL for comprehension-only cells.
-        let promote_inlined_cells_to_cell = matches!(
+        for symbol in symbol_table.symbols.values_mut() {
+            if inlined_cells.contains(&symbol.name) {
+                symbol.flags.insert(SymbolFlags::COMP_CELL);
+            }
+        }
+
+        // PEP 709 / CPython symtable.c: only promote LOCAL -> CELL in
+        // function-like scopes where analyze_cells() runs.
+        let promote_inlined_cells_to_cell = matches!(
             symbol_table.typ,
             CompilerScope::Function
                 | CompilerScope::AsyncFunction
                 | CompilerScope::Lambda
                 | CompilerScope::Comprehension
                 | CompilerScope::Annotation
         );
-        for symbol in symbol_table.symbols.values_mut() {
-            if inlined_cells.contains(&symbol.name)
-                && promote_inlined_cells_to_cell
-                && symbol.scope == SymbolScope::Local
-            {
-                symbol.scope = SymbolScope::Cell;
-            }
-        }
+        for symbol in symbol_table.symbols.values_mut() {
+            self.analyze_symbol(symbol, symbol_table.typ, sub_tables, class_entry)?;
+
+            if promote_inlined_cells_to_cell
+                && symbol.scope == SymbolScope::Local
+                && inlined_cells.contains(&symbol.name)
+            {
+                symbol.scope = SymbolScope::Cell;
+            }
+
+            if symbol.scope == SymbolScope::Free || symbol.flags.contains(SymbolFlags::FREE_CLASS) {
+                newfree.insert(symbol.name.clone());
+            }
+        }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/codegen/src/symboltable.rs` around lines 570 - 607, The third pass
that promotes LOCAL→CELL based on inlined_cells and
promote_inlined_cells_to_cell should be folded into the loop that calls
self.analyze_symbol so you only iterate symbol_table.symbols once; after calling
self.analyze_symbol(symbol, ...), compute promote_inlined_cells_to_cell (as
already defined) and if inlined_cells.contains(&symbol.name) &&
promote_inlined_cells_to_cell && symbol.scope == SymbolScope::Local then set
symbol.scope = SymbolScope::Cell before you collect free vars into newfree,
ensuring the COMP_CELL bit-setting pass (which must run first) remains unchanged
and newfree sees the promoted scope in the same pass.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

scripts/compare_bytecode.py (1)

172-196: ⚠️ Potential issue | 🟡 Minor

Preserve failed reruns in the result map.

failed/empty targets are only logged, then omitted from recovered. If both interpreters lose the same target, the final report can under-count it instead of reporting an error.

Suggested fix

 def _rerun_missing_targets(interpreter, targets, base_dir):
     recovered = {}
     failed = []
     empty = []
     for target in targets:
+        relpath = target[0]
         data = _run_sync_dump(interpreter, [target], base_dir)
         if data is None:
-            failed.append(target[0])
+            failed.append(relpath)
+            recovered[relpath] = {
+                "status": "error",
+                "error": "dump helper failed while rerunning target",
+            }
         elif data:
             recovered.update(data)
         else:
-            empty.append(target[0])
+            empty.append(relpath)
+            recovered[relpath] = {
+                "status": "error",
+                "error": "dump helper produced no data while rerunning target",
+            }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/compare_bytecode.py` around lines 172 - 196, The
_rerun_missing_targets function currently logs failed/empty targets but omits
them from the returned recovered map, causing under-reporting; after computing
failed and empty (use the same keying as recovered, i.e. target[0]), add entries
for each failed and empty target into recovered (e.g., recovered[failed_name] =
None) before returning so the caller sees these missing items instead of
silently dropping them; update _rerun_missing_targets to insert those sentinel
entries after the warning prints and then return recovered.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes