GeezerVison

We don't have a strict rule for this, but conventionally using this to distinguish local patch from failing tests.

The notation itself looks good.
But we use search tools for this comment. Keeping consistency will be helpful

Old habits die hard 😅

If we had test-passing symtable before, can these tests be easily fixed?

I think so, I think the main issue is that "symbols" attribute used to return a list, now it's a dict. I'll investigate a bit more about the structure. For now I'll set the PR as a draft

Thank you so much!

Now more tests pass. But we still have a difference between our impl and cpython. but this is a change that needs to be made at compiler/codegen/src/symboltable.rs, I think it's a bit out of scope for this PR. I'll try to do a followup PR to fix it

💡 Verification agent

What does Python's symtable SymbolTable.id property return and are there security considerations?

Align SymbolTable.id() with CPython’s get_id(): avoid exposing raw pointers

The current implementation in vm/src/stdlib/symtable.rs returns the struct’s memory address as a usize, which diverges from CPython’s symtable.SymbolTable.get_id() (an internal, opaque identifier) and can leak pointer layouts.

Suggested changes:

• Replace the pointer cast in fn id(&self) -> usize with a dedicated, incrementing identifier (e.g., an AtomicUsize counter or per‐table UID stored on creation).
• Ensure IDs are unique within a run but do not expose raw addresses.
• Update documentation to note that id() returns an internal symbol-table ID, not a memory address.

Locations to update:

• vm/src/stdlib/symtable.rs, lines ~170–173 (the fn id(&self) definition)

Example diff sketch:

-   #[pygetset]
-   fn id(&self) -> usize {
-       self as *const Self as *const std::ffi::c_void as usize
-   }
+   // Use an internal counter instead of raw pointer to avoid leaking memory addresses
+   #[pygetset]
+   fn id(&self) -> usize {
+       self.inner_id  // populated from a global AtomicUsize at creation
+   }

Committable suggestion skipped: line range outside the PR's diff.

In vm/src/stdlib/symtable.rs around lines 170 to 173, the id() method currently
returns the raw memory address cast to usize, which exposes internal pointer
details and differs from CPython's opaque get_id(). To fix this, replace the
pointer cast with a unique internal identifier generated via a static
AtomicUsize counter incremented for each new SymbolTable instance or assign a
unique ID at creation stored in the struct. Update the id() method to return
this stored unique ID instead of the pointer. Also, revise the method's
documentation to clarify that id() returns an internal symbol table identifier,
not a memory address.

let's not delete these methods but change to getset, just like get_name and get_lineno.
Unlike other more complex cases, the deletion of these 2 methods cause unnecessary regression.

Fix this. And made type to return an int instead of a String, so now more tests pass

💡 Verification agent

#!/bin/bash
# Check CPython's SymbolTable.get_type() return values
python3 -c "
import symtable
st = symtable.symtable('def f(): pass', '<string>', 'exec')
print('Module type:', repr(st.get_type()))
for child in st.get_children():
    print('Function type:', repr(child.get_type()))
"

Ensure type property matches CPython’s string-based API

CPython’s SymbolTable.get_type() returns strings like "module" and "function", not integer codes. Our Rust typ() currently returns integer constants and falls back to -1 for any unhandled cases, which:

• Breaks compatibility with existing users expecting string values
• Omits several type names (e.g. comprehensions, lambdas, generators) that CPython exposes

Please update the implementation to either:

• Return Python strings matching CPython’s names, or
• If you intentionally switch to integers, document the change clearly and replace the -1 fallback with a dedicated TYPE_UNKNOWN constant—and add mappings for all types in CPython’s C implementation.

Points to address:

• Decide on integer vs. string return type and update docs accordingly
• Remove the // TODO and implement mappings for missing CPython types (generator, lambda, comprehension, etc.)
• Eliminate the -1 fallback or replace it with a well-defined default constant

In vm/src/stdlib/symtable.rs lines 154 to 163, the typ() function currently
returns integer constants and uses -1 as a fallback, which breaks compatibility
with CPython's string-based SymbolTable.get_type() API and omits several type
names. To fix this, change the return type to Python strings matching CPython's
type names (e.g., "module", "function", "class", "generator", "lambda",
"comprehension"), remove the -1 fallback, and implement mappings for all missing
CPython types. Also update any related documentation to reflect the string
return type and remove the TODO comment.

💡 Verification agent

#!/bin/bash
# Verify CPython's SymbolTable.get_symbols() return type and structure
python3 -c "
import symtable
st = symtable.symtable('x = 1', '<string>', 'exec')
symbols = st.get_symbols()
print('Type:', type(symbols))
if symbols:
    print('First symbol type:', type(symbols[0]))
    print('First symbol methods:', [m for m in dir(symbols[0]) if not m.startswith('_')])
"

Breaking change: symbols now returns dict instead of list

You’ve changed the symbols getter in vm/src/stdlib/symtable.rs (lines 197–205) from a list of Symbol objects to a dict[str, int]. In CPython, symtable.symtable(...).get_symbols() returns a list[Symbol] (each with methods like is_assigned(), get_name(), etc.), not a mapping of names to flag bits.

This will break any existing code that:

• Iterates over symbols as Symbol instances
• Calls methods on Symbol objects
• Relies on the return being a list

Please address this by one of the following:

• Revert symbols to return PyListRef of Symbol objects (e.g. vm.new_pyobj(symbol)) for compatibility
• Introduce a new method/property (e.g. symbol_flags) for the dict view, and preserve symbols as the list of Symbol
• If this breaking change is intentional, update the documentation/changelog to call it out clearly

In vm/src/stdlib/symtable.rs around lines 197 to 205, the symbols getter was
changed to return a dict mapping symbol names to flag bits, but in CPython it
returns a list of Symbol objects. To fix this, revert the symbols method to
return a PyListRef of Symbol objects by creating a list and adding each symbol
object (e.g. using vm.new_pyobj(symbol)) to it, preserving compatibility. If the
dict view is needed, add a new method or property like symbol_flags for that
purpose instead of changing symbols.