💡 Verification agent

#!/usr/bin/env bash
set -euo pipefail

echo "All occurrences of DictUpdate with context (sanity check):"
rg -n --hidden --glob '!target' -A3 -B3 '\bDictUpdate\b'

echo
echo "Likely pattern matches not using struct form (manual review recommended):"
rg -n --hidden --glob '!target' -e '\bDictUpdate\s*=>' -e '\bInstruction::DictUpdate\s*[),]|,$'

echo
echo "Constructors not using struct form (look for missing braces):"
rg -n --hidden --glob '!target' -e '\bInstruction::DictUpdate\b(?!\s*\{)' -U

Add documentation for DictUpdate index semantics & ensure marshaling compatibility

Please update compiler/core/src/bytecode.rs around the DictUpdate variant (lines 667–672) as follows:

• Insert a doc comment explaining that index is a 0-based offset to the dict on the stack before popping the source mapping. For example:

  /// Pop a source mapping from the top of the stack and update
  /// the dict located `index` slots below it. `index` is a 0-based
  /// offset relative to the top-of-stack *before* the pop.
  DictUpdate {
      index: Arg<u32>,
  },

• Note in the doc that legacy bytecode (with no index field) will decode with index == 0; if you intend to break compatibility, bump the bytecode version or handle index == 0 explicitly in tests.

Next steps:

• No stale unit-variant constructors were found (grep output shows only struct-form usage), so no further code changes required there.
• Add unit tests in the bytecode test module asserting:
  • Instruction::DictUpdate { index } yields stack_effect() == -1
  • fmt_dis() prints DictUpdate { index }
• Add a marshaling test that serializes an old-format DictUpdate (without arg) and ensures it decodes to DictUpdate { index: 0 }.

In compiler/core/src/bytecode.rs around lines 667 to 672, add a doc comment to
the DictUpdate variant explaining that the index field is a 0-based offset to
the dict on the stack before popping the source mapping, clarifying the
semantics as described. Also note in the doc that legacy bytecode without the
index field decodes with index == 0 and mention the need to handle compatibility
accordingly. Then, add unit tests in the bytecode test module to assert that
Instruction::DictUpdate { index } has a stack_effect() of -1 and that fmt_dis()
outputs the expected string. Finally, add a marshaling test that serializes an
old-format DictUpdate (without the index argument) and verifies it decodes to
DictUpdate { index: 0 }.

⚠️ Potential issue

Don't mask attribute lookup errors when checking mapping protocol

vm.get_method(...) returns Option<PyResult<_>>; using is_none() ignores Some(Err(e)) and turns it into a TypeError, hiding the real exception. Propagate lookup errors instead.

-                if vm
-                    .get_method(source.clone(), vm.ctx.intern_str("keys"))
-                    .is_none()
-                {
-                    return Err(vm.new_type_error(format!(
-                        "'{}' object is not a mapping",
-                        source.class().name()
-                    )));
-                }
+                match vm.get_method(source.clone(), vm.ctx.intern_str("keys")) {
+                    Some(Ok(_)) => {}
+                    Some(Err(e)) => return Err(e),
+                    None => {
+                        return Err(vm.new_type_error(format!(
+                            "'{}' object is not a mapping",
+                            source.class().name()
+                        )));
+                    }
+                }

In vm/src/frame.rs around lines 826 to 833, the code uses is_none() on the
result of vm.get_method(...) which returns Option<PyResult<_>>, causing it to
ignore Some(Err(e)) and incorrectly convert it to a TypeError. To fix this,
handle the Option and PyResult separately: first check if the Option is None to
return the TypeError, but if it is Some, propagate any Err by returning it
instead of masking it.