crates/vm/src/frame.rs (1)

2568-2584: Consider consolidating identical jump instrumentation arms.

InstrumentedJumpForward and InstrumentedJumpBackward are identical aside from the label variable. You can merge them to keep the match smaller and avoid duplication.

Refactor sketch

-            Instruction::InstrumentedJumpForward => {
-                let src_offset = (self.lasti() - 1) * 2;
-                let target = bytecode::Label::from(u32::from(arg));
-                self.jump(target);
-                if self.monitoring_mask & monitoring::EVENT_JUMP != 0 {
-                    monitoring::fire_jump(vm, self.code, src_offset, target.0 * 2)?;
-                }
-                Ok(None)
-            }
-            Instruction::InstrumentedJumpBackward => {
-                let src_offset = (self.lasti() - 1) * 2;
-                let dest = bytecode::Label::from(u32::from(arg));
-                self.jump(dest);
-                if self.monitoring_mask & monitoring::EVENT_JUMP != 0 {
-                    monitoring::fire_jump(vm, self.code, src_offset, dest.0 * 2)?;
-                }
-                Ok(None)
-            }
+            Instruction::InstrumentedJumpForward | Instruction::InstrumentedJumpBackward => {
+                let src_offset = (self.lasti() - 1) * 2;
+                let target = bytecode::Label::from(u32::from(arg));
+                self.jump(target);
+                if self.monitoring_mask & monitoring::EVENT_JUMP != 0 {
+                    monitoring::fire_jump(vm, self.code, src_offset, target.0 * 2)?;
+                }
+                Ok(None)
+            }

As per coding guidelines, "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code."

🤖 Prompt for AI Agents

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

In `@crates/vm/src/frame.rs` around lines 2568 - 2584, The two match arms for
Instruction::InstrumentedJumpForward and Instruction::InstrumentedJumpBackward
are identical except for the label variable; consolidate them by matching both
variants together (Instruction::InstrumentedJumpForward |
Instruction::InstrumentedJumpBackward) and compute the label once from arg
(e.g., let label = bytecode::Label::from(u32::from(arg))), then reuse the shared
logic: compute src_offset via self.lasti(), call self.jump(label), and
conditionally call monitoring::fire_jump(vm, self.code, src_offset, label.0 * 2)
using the existing monitoring_mask check to avoid duplication.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

crates/compiler-core/src/bytecode.rs (1)

119-125: ⚠️ Potential issue | 🟡 Minor

Unchecked start + size can wrap on malformed input.

Line 121 computes end: start + size without overflow protection. On a crafted/corrupted varint table, this can produce end < start, yielding a nonsensical entry. The sibling function find_exception_handler (line 85) has the same issue, but since decode_exception_table returns entries for external consumption, a bogus end is more dangerous here.

Suggested fix

+        let end = match start.checked_add(size) {
+            Some(end) => end,
+            None => break,
+        };
         entries.push(ExceptionTableEntry {
             start,
-            end: start + size,
+            end,
             target,

🤖 Prompt for AI Agents

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

In `@crates/compiler-core/src/bytecode.rs` around lines 119 - 125, The code
unconditionally computes end = start + size which can overflow on malformed
input; in decode_exception_table (and similarly in find_exception_handler)
replace the unchecked addition with a checked addition (use
start.checked_add(size)) and handle the None case by returning an error (or
skipping the malformed entry) instead of producing an end < start; ensure the
handling is consistent for ExceptionTableEntry creation (and its
depth/push_lasti logic) so no bogus entries are emitted to external consumers.

crates/vm/src/frame.rs (1)

261-270: FrameState.lasti still not synced in threading builds.

set_lasti only stores to the atomic self.lasti, leaving self.state.lasti (the value read by ExecutingFrame::lasti()) stale in threading builds.

🤖 Prompt for AI Agents

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

In `@crates/vm/src/frame.rs` around lines 261 - 270, In threading builds set_lasti
currently only writes the atomic self.lasti, leaving self.state.lasti stale;
update set_lasti to write both places so ExecutingFrame::lasti() (which reads
self.state.lasti) sees the new value — i.e., in the #[cfg(feature =
"threading")] branch keep the atomic store(self.lasti.store(val, ...)) and also
update the backing FrameState.lasti (self.state.lasti.set(val) or assign
accordingly) so both the atomic and self.state.lasti are kept in sync.

crates/vm/src/frame.rs (3)

395-397: monitoring_mask and prev_line set but never read in yield_from_target.

The ExecutingFrame built here only calls exec.yield_from_target() which neither reads monitoring_mask nor prev_line. These initializations are dead code in this context.

♻️ Proposed cleanup

-            monitoring_mask: 0,
-            prev_line: 0,

Remove the two dead fields, or if ExecutingFrame requires them to be set (struct exhaustiveness), document why.

🤖 Prompt for AI Agents

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

In `@crates/vm/src/frame.rs` around lines 395 - 397, The ExecutingFrame
initialization sets monitoring_mask and prev_line but yield_from_target never
reads them, so drop these dead initializations: remove monitoring_mask and
prev_line from the ExecutingFrame literal (or from the struct if they truly are
unused) and rebuild; if ExecutingFrame must maintain those fields for
exhaustiveness, either derive/implement Default and use ..Default::default() or
add a clear comment in the ExecutingFrame/ yield_from_target caller explaining
why the fields are intentionally set despite not being read. Ensure references
to monitoring_mask, prev_line, ExecutingFrame, and yield_from_target are updated
accordingly.

---

503-511: Redundant prev_line update when tracing already set it.

When the tracing block (lines 490-497) fires and updates self.prev_line, the block at lines 503-511 unconditionally writes the same value again. The second write is always a no-op but adds unnecessary noise.

♻️ Proposed cleanup

-        if !matches!(
-            op,
-            Instruction::Resume { .. }
-                | Instruction::ExtendedArg
-                | Instruction::InstrumentedLine
-        ) && let Some((loc, _)) = self.code.locations.get(idx)
-        {
-            self.prev_line = loc.line.get() as u32;
-        }
+        if !vm.use_tracing.get()
+            && !matches!(
+                op,
+                Instruction::Resume { .. }
+                    | Instruction::ExtendedArg
+                    | Instruction::InstrumentedLine
+            )
+        {
+            if let Some((loc, _)) = self.code.locations.get(idx) {
+                self.prev_line = loc.line.get() as u32;
+            }
+        }

Or simply remove the else-redundant guard; both writes are idempotent. Consider adding a short comment explaining the write's purpose (LINE event deduplication in InstrumentedLine).

🤖 Prompt for AI Agents

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

In `@crates/vm/src/frame.rs` around lines 503 - 511, The block that
unconditionally sets self.prev_line using self.code.locations.get(idx) after the
tracing branch is redundant because the tracing branch (which also updates
self.prev_line) already wrote the same value; remove the second update in the
matching block (the code that checks !matches!(op, Instruction::Resume |
Instruction::ExtendedArg | Instruction::InstrumentedLine) && let Some((loc, _))
= self.code.locations.get(idx)) so self.prev_line is only set in the tracing
branch, or convert that assignment into an else-branch of the tracing logic;
also add a short comment by the remaining write (referencing self.prev_line and
InstrumentedLine) explaining it's for LINE event deduplication.

---

2673-2684: offset.saturating_sub(2) in InstrumentedNotTaken assumes preceding instruction is always the conditional jump.

The src_offset is computed as (lasti - 1) * 2 (this instruction's position), then reduced by 2 to point at the preceding instruction. This implicitly requires the compiler to always emit NOT_TAKEN immediately after its paired conditional jump with no intervening opcodes, and that the instruction width is always 2 bytes.

This is currently guaranteed by the codegen, but it's worth a brief comment explaining the invariant to prevent future breakage.

📝 Suggested comment

 Instruction::InstrumentedNotTaken => {
     if self.monitoring_mask & monitoring::EVENT_BRANCH_LEFT != 0 {
         let offset = (self.lasti() - 1) * 2;
         let dest_offset = self.lasti() * 2;
+        // NOT_TAKEN is always emitted immediately after its paired POP_JUMP_IF_*,
+        // so src_offset points at that conditional jump (offset - 2).
         monitoring::fire_branch_left(
             vm,
             self.code,
             offset.saturating_sub(2),
             dest_offset,
         )?;

🤖 Prompt for AI Agents

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

In `@crates/vm/src/frame.rs` around lines 2673 - 2684, Add a short explanatory
comment immediately above the InstrumentedNotTaken handling block
(Instruction::InstrumentedNotTaken) that documents the invariant: the NOT_TAKEN
marker is emitted directly after its paired conditional jump with no intervening
opcodes and all instruction slots are 2 bytes, which is why the src_offset is
computed as (lasti - 1) * 2 and then adjusted with .saturating_sub(2) before
calling monitoring::fire_branch_left; additionally consider a debug-time check
(e.g., using debug_assert!) that self.lasti() >= 1 to catch violations early.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

crates/vm/src/frame.rs (2)

261-270: ⚠️ Potential issue | 🟠 Major

set_lasti still leaves FrameState.lasti stale in threading mode.

Line 264 updates only the atomic field. But ExecutingFrame::lasti() reads state.lasti (Line 469), so external f_lasti writes can be ignored while executing.

Suggested fix

 pub fn set_lasti(&self, val: u32) {
     #[cfg(feature = "threading")]
     {
         self.lasti.store(val, atomic::Ordering::Relaxed);
+        self.state.lock().lasti = val;
     }
     #[cfg(not(feature = "threading"))]
     {
         self.lasti.set(val);
     }
 }

🤖 Prompt for AI Agents

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

In `@crates/vm/src/frame.rs` around lines 261 - 270, The threading build currently
updates only the atomic lasti in Frame::set_lasti, leaving FrameState.lasti
stale and causing ExecutingFrame::lasti to read an outdated value; change
Frame::set_lasti so that in the #[cfg(feature = "threading")] branch it writes
the atomic (self.lasti.store(...)) and also updates the non-atomic backing field
(state.lasti or self.state.lasti.set/assign) so both places remain consistent,
or alternatively change ExecutingFrame::lasti to read the atomic lasti; update
the code paths involving set_lasti, FrameState.lasti, and ExecutingFrame::lasti
accordingly.

---

3271-3271: ⚠️ Potential issue | 🟠 Major

is_python_call still misclassifies wrapped Python callables as C calls.

Line 3271 only matches bare PyFunction. Wrappers (e.g. bound/static/partial-like delegates) can still execute Python frames, which can lead to wrong C_* event classification.

🤖 Prompt for AI Agents

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

In `@crates/vm/src/frame.rs` at line 3271, is_python_call currently only checks
callable.downcast_ref::<PyFunction>(), misclassifying wrapped Python callables;
change the detection to treat any object whose __call__ is a PyFunction as a
Python call too by replacing the single downcast_ref check with a two-step
check: first keep callable.downcast_ref::<PyFunction>().is_some(), and if not,
attempt to retrieve callable.getattr("__call__") and test whether that attribute
downcasts to PyFunction (handling/propagating errors safely), so
bound/static/partial-like wrappers are classified as Python calls; reference the
local variables is_python_call and callable when making the change.

crates/vm/src/builtins/code.rs (1)

916-987: LGTM! Previous NOT_TAKEN concern addressed; minor robustness consideration.

The implementation correctly handles de-instrumentation via to_base() and properly checks for NotTaken presence (lines 958-965) before computing the fall-through offset.

One minor consideration: the EndAsyncFor branch (lines 970-971) performs next_i - oparg as usize which could panic on subtraction underflow if the bytecode is malformed. While valid bytecode should never trigger this, using saturating_sub or checked_sub would make this more defensive against corrupted inputs.

Optional defensive fix

 Instruction::EndAsyncFor => {
     // src = END_SEND position (next_i - oparg)
     let next_i = i + 1;
-    let src_i = next_i - oparg as usize;
+    let src_i = next_i.saturating_sub(oparg as usize);
     (src_i * 2, (src_i + 2) * 2, next_i * 2)
 }

🤖 Prompt for AI Agents

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

In `@crates/vm/src/builtins/code.rs` around lines 916 - 987, co_branches currently
computes src_i for the EndAsyncFor case as next_i - oparg as usize which can
underflow on malformed bytecode; update the EndAsyncFor branch in co_branches to
use a safe subtraction (e.g., saturating_sub or checked_sub with an early
continue or error) when deriving src_i from next_i and oparg so you avoid panics
on underflow while preserving existing behavior for valid bytecode.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

crates/vm/src/frame.rs (2)

3269-3269: ⚠️ Potential issue | 🟠 Major

is_python_call misclassifies wrapped Python callables.

Line 3269 only detects bare PyFunction, so wrappers (e.g. bound/static/partial wrappers around Python functions) can incorrectly emit C_RETURN/C_RAISE in addition to Python-level events.

[suggested fix: unwrap known wrapper callables before classification, then check the terminal callable type.]

🤖 Prompt for AI Agents

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

In `@crates/vm/src/frame.rs` at line 3269, The current is_python_call check only
tests for PyFunction and thus misclassifies wrapped callables; add logic to
unwrap known wrapper types (e.g. bound method wrappers, functools.partial-like
wrappers, staticmethod/classmethod wrappers) before testing the terminal
callable type. Implement or call a helper (e.g. unwrap_callable or similar) that
strips PyMethod/partial/descriptor wrappers and then replace the direct
downcast_ref::<PyFunction>() check with a downcast on the unwrapped object so
is_python_call reflects the terminal callable type.

---

2531-2554: ⚠️ Potential issue | 🟠 Major

C_RETURN/C_RAISE emission is incorrectly tied to EVENT_CALL.

At Line 3272 and Line 2531, arg capture is guarded by EVENT_CALL; when a tool enables only EVENT_C_RETURN/EVENT_C_RAISE, those events can be dropped.

Suggested fix pattern

- let call_arg0 = if self.monitoring_mask & monitoring::EVENT_CALL != 0 {
+ let monitor_call = self.monitoring_mask & monitoring::EVENT_CALL != 0;
+ let monitor_c_return = self.monitoring_mask & monitoring::EVENT_C_RETURN != 0;
+ let monitor_c_raise = self.monitoring_mask & monitoring::EVENT_C_RAISE != 0;
+ let need_arg0 = monitor_call || (monitor_c_return || monitor_c_raise);
+ let call_arg0 = if need_arg0 {
     ...
   } else {
     None
   };

- if let Some(arg0) = call_arg0 && !is_python_call {
+ if let Some(arg0) = call_arg0.clone() && !is_python_call && monitor_c_return {
     ...
   }

- if let Some(arg0) = call_arg0 && !is_python_call {
+ if let Some(arg0) = call_arg0 && !is_python_call && monitor_c_raise {
     ...
   }

Also applies to: 3271-3303

🤖 Prompt for AI Agents

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

In `@crates/vm/src/frame.rs` around lines 2531 - 2554, The code currently ties
capturing call-site arguments to monitoring::EVENT_CALL (via monitoring_mask and
call_args), so handlers that enable only EVENT_C_RETURN or EVENT_C_RAISE will
not receive the captured args; change the logic so capturing of
global_super/arg0 is performed whenever EVENT_C_RETURN or EVENT_C_RAISE is
enabled (i.e. check monitoring::EVENT_C_RETURN|monitoring::EVENT_C_RAISE in
addition to or instead of EVENT_CALL), ensure
nth_value(2)/nth_value(1)/monitoring::get_missing are used to populate the tuple
used by monitoring::fire_c_return and monitoring::fire_c_raise, and keep
fire_call only under EVENT_CALL while using the newly-captured call_args for the
C_RETURN/C_RAISE paths in load_super_attr success and Err branches (symbols:
monitoring_mask, EVENT_CALL, EVENT_C_RETURN, EVENT_C_RAISE, call_args,
fire_call, fire_c_return, fire_c_raise, load_super_attr, nth_value,
get_missing).

crates/compiler-core/src/bytecode/instruction.rs (1)

464-466: ⚠️ Potential issue | 🟡 Minor

Clarify to_base() None semantics.

Line 465 currently implies None means “not instrumented,” but Instruction::InstrumentedLine and Instruction::InstrumentedInstruction are instrumented and also return None. Please document that None can also mean “instrumented opcode without a base equivalent.”

🤖 Prompt for AI Agents

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

In `@crates/compiler-core/src/bytecode/instruction.rs` around lines 464 - 466,
Update the doc comment for the to_base() method to clarify the semantics of a
None return: state that None is returned both for opcodes that are not
instrumented and for instrumented opcodes that do not have a base equivalent
(specifically mention Instruction::InstrumentedLine and
Instruction::InstrumentedInstruction as examples of instrumented variants that
also return None), so callers know None can mean either “not instrumented” or
“instrumented without a base variant.”

crates/compiler-core/src/bytecode.rs (1)

420-423: ⚠️ Potential issue | 🟡 Minor

Unify the safety contract between Deref and replace_op.

Lines 420-423 say shared reads are valid while replace_op mutates, but Lines 433-434 require exclusive access with no concurrent reads/writes. These guarantees conflict; please keep a single contract (exclusive access) to avoid implying racy reads are permitted.

#!/bin/bash
# Verify the conflicting safety statements in CodeUnits.
rg -n "Shared references to the slice are valid even while replace_op|exclusive access to the instruction buffer" crates/compiler-core/src/bytecode.rs -C2

Also applies to: 433-435

🤖 Prompt for AI Agents

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

In `@crates/compiler-core/src/bytecode.rs` around lines 420 - 423, Unify the
safety contract by requiring exclusive access for both the Deref path and
replace_op: update the unsafe comment in the Deref implementation (the unsafe {
&*self.0.get() } block) to state that callers must ensure exclusive,
no-concurrent-reads/writes access to the underlying instruction buffer (remove
the line claiming shared reads are valid), and make the replace_op
documentation/unsafe contract match (i.e., both must assert exclusive access to
the instruction buffer); reference the Deref impl around the unsafe get() and
the replace_op function so the two comments are consistent and no longer imply
racy shared reads are permitted.

crates/vm/src/stdlib/sys/monitoring.rs (1)

341-347: Redundant .take(len) in iterator chain.

The iterator code.code.instructions.iter() already yields exactly len items (since len = code.code.instructions.len()), making .take(len) unnecessary.

Suggested simplification

         for (i, unit) in code
             .code
             .instructions
             .iter()
             .enumerate()
-            .take(len)
             .skip(first_traceable)
         {

The same applies at line 437-442.

🤖 Prompt for AI Agents

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

In `@crates/vm/src/stdlib/sys/monitoring.rs` around lines 341 - 347, Remove the
redundant .take(len) from the iterator chain(s) that enumerate
code.code.instructions (where len = code.code.instructions.len())—e.g., in the
loop using
code.code.instructions.iter().enumerate().take(len).skip(first_traceable) remove
.take(len) so the chain becomes enumerate().skip(first_traceable); do the same
for the second occurrence referenced around the later loop (the other enumerate
over code.code.instructions with .take(len)). This removes unnecessary
truncation while preserving use of len and first_traceable.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

crates/vm/src/builtins/code.rs (1)

925-927: ⚠️ Potential issue | 🟠 Major

Restore displaced opcodes from monitoring_data in co_branches().

Line 926 only uses to_base(), so InstrumentedLine / InstrumentedInstruction cannot be mapped back to their original opcodes. That makes branch extraction incomplete on instrumented code, including target checks at Line 942 and NOT_TAKEN fallthrough detection at Line 961.

Proposed fix

 pub fn co_branches(&self, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
     use bytecode::Instruction;

     let instructions = &self.code.instructions;
+    let monitoring = self.monitoring_data.lock();
+    let original_op = |idx: usize, op: Instruction| -> Instruction {
+        match op {
+            Instruction::InstrumentedLine => monitoring
+                .as_ref()
+                .and_then(|m| m.line_opcodes.get(idx).copied())
+                .filter(|b| *b != 0)
+                .and_then(|b| Instruction::try_from(b).ok())
+                .unwrap_or(op),
+            Instruction::InstrumentedInstruction => monitoring
+                .as_ref()
+                .and_then(|m| m.per_instruction_opcodes.get(idx).copied())
+                .filter(|b| *b != 0)
+                .and_then(|b| Instruction::try_from(b).ok())
+                .unwrap_or(op),
+            _ => op.to_base().unwrap_or(op),
+        }
+    };
     let mut branches = Vec::new();
     let mut extended_arg: u32 = 0;

     for (i, unit) in instructions.iter().enumerate() {
-        let op = unit.op.to_base().unwrap_or(unit.op);
+        let op = original_op(i, unit.op);
         let raw_arg = u32::from(u8::from(unit.arg));
@@
-                    let right = if matches!(
-                        instructions.get(target).map(|u| u.op),
+                    let right = if matches!(
+                        instructions.get(target).map(|u| original_op(target, u.op)),
                         Some(Instruction::EndFor) | Some(Instruction::InstrumentedEndFor)
                     ) {
@@
                     let next_op = instructions
                         .get(i + 1)
-                        .map(|u| u.op.to_base().unwrap_or(u.op));
+                        .map(|u| original_op(i + 1, u.op));

Also applies to: 942-945, 958-961

🤖 Prompt for AI Agents

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

In `@crates/vm/src/builtins/code.rs` around lines 925 - 927, In co_branches(),
when de-instrumenting opcodes (currently using
unit.op.to_base().unwrap_or(unit.op)), consult monitoring_data to restore any
displaced InstrumentedLine/InstrumentedInstruction back to their original opcode
first (e.g., look up the original opcode by the unit's offset/index in
monitoring_data and use it), then fall back to to_base() and finally unit.op;
apply the same restoration logic at the other locations that compute branch
targets and NOT_TAKEN fallthrough (the checks around the target computation and
fallthrough detection currently at the later blocks referenced in the comment)
so instrumented instructions map correctly to their original opcodes.

crates/vm/src/frame.rs (1)

3378-3379: ⚠️ Potential issue | 🟠 Major

Unwrap delegated Python callables before deciding C-return/C-raise emission.

Line 3378 only recognizes bare PyFunction. Wrapper callables that delegate to Python functions can be misclassified as C calls, which can produce incorrect/duplicate monitoring events on Line 3396 and Line 3406.

Also applies to: 3396-3410

🤖 Prompt for AI Agents

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

In `@crates/vm/src/frame.rs` around lines 3378 - 3379, The code currently sets
is_python_call via callable.downcast_ref::<PyFunction>().is_some(), which
misclassifies wrapper/delegating callables as non-Python and causes incorrect
duplicate monitoring events at the C-return/C-raise emission sites; change the
logic that computes is_python_call to first unwrap delegated Python callables
(detect known wrapper/proxy types and obtain their inner target, or use the
existing unwrap utility if present) and then test the unwrapped value with
downcast_ref::<PyFunction>() so the subsequent C-return/C-raise emission
branches (the code paths that emit the monitoring events) correctly treat
delegated Python functions as Python calls.

crates/compiler-core/src/bytecode/instruction.rs (1)

435-513: Consider a single source of truth for opcode pair mappings.

Line 440 onward and Line 473 onward duplicate the same mapping pairs in opposite directions. This is easy to desync on future opcode additions. Consider defining pairs once (macro/table) and deriving both to_instrumented and to_base matches from it.

As per coding guidelines: "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code."

🤖 Prompt for AI Agents

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

In `@crates/compiler-core/src/bytecode/instruction.rs` around lines 435 - 513, The
two match lists in to_instrumented and to_base duplicate opcode pair mappings
and should be centralized; create a single source-of-truth mapping (e.g., a
static array, const slice, or macro) that lists tuples of (base_variant,
instrumented_variant) and then implement to_instrumented and to_base by looking
up that mapping (construct the base variants with Arg::marker() where needed and
match on variant discriminants or pattern keys). Update to_instrumented to
return the instrumented variant from the shared table and to_base to return the
base variant (using Arg::marker() for typed fields) so new opcode pairs are
added only once and both functions derive their behavior from the same mapping.

crates/vm/src/stdlib/sys/monitoring.rs (1)

230-230: Replace decorative section separators with plain comments.

Please remove // === ... === separators and keep short plain comments/doc-comments only.

As per coding guidelines "Do not add decorative section separators (e.g. // -----------, // ===, /* *** */). Use /// doc-comments or short // comments only when they add value".

Also applies to: 277-277

🤖 Prompt for AI Agents

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

In `@crates/vm/src/stdlib/sys/monitoring.rs` at line 230, Replace the decorative
section separator comment "// === Phase 1-3: De-instrument all layers (outermost
first) ===" (and the similar "// === ... ===" at the other occurrence) with a
short plain comment or doc-comment—e.g., change to "// Phase 1-3: De-instrument
all layers (outermost first)" or "/// Phase 1-3: De-instrument all layers"—so
remove the "===...===" decoration; update the comment in the surrounding
function/section in monitoring.rs where that text appears to follow the
project's short/plain comment guideline.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro