Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

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

238-248: ⚠️ Potential issue | 🟠 Major

Check exception type before overriding decode errors.

The map_err(|_| ...) closure masks all decode failures as "unknown encoding", including legitimate decode errors on known encodings (e.g., invalid bytes for the declared encoding). Only LookupError should map to "unknown encoding"; other errors (decode failures) should be preserved or converted to an appropriate SyntaxError with decode details.

🐛 Proposed fix to distinguish error types

         let decoded = vm
             .state
             .codec_registry
             .decode_text(bytes_obj.into(), enc, None, vm)
-            .map_err(|_| {
-                vm.new_exception_msg(
-                    vm.ctx.exceptions.syntax_error.to_owned(),
-                    format!("unknown encoding for '{filename}': {enc}").into(),
-                )
+            .map_err(|exc| {
+                if exc.fast_isinstance(&vm.ctx.exceptions.lookup_error) {
+                    vm.new_exception_msg(
+                        vm.ctx.exceptions.syntax_error.to_owned(),
+                        format!("unknown encoding for '{filename}': {enc}").into(),
+                    )
+                } else {
+                    vm.new_exception_msg(
+                        vm.ctx.exceptions.syntax_error.to_owned(),
+                        format!("'{filename}': {}", exc.to_string()).into(),
+                    )
+                }
             })?;

🤖 Prompt for AI Agents

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

In `@crates/vm/src/stdlib/builtins.rs` around lines 238 - 248, The current map_err
on vm.state.codec_registry.decode_text hides all decode failures as "unknown
encoding"; change the error handling in the call to decode_text (the map_err
closure around decode_text) to inspect the underlying error: if it is a
LookupError (i.e., unknown encoding) convert to
vm.new_exception_msg(vm.ctx.exceptions.syntax_error, format!("unknown encoding
for '{filename}': {enc}")), otherwise preserve or convert the real decode error
into a SyntaxError (include the original error message/details) or re-raise it
so real decoding failures (invalid byte sequences) are not masked; update the
closure that wraps decode_text accordingly instead of unconditionally mapping
all errors.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

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

239-244: ⚠️ Potential issue | 🟠 Major

Preserve non-lookup decode failures instead of rewriting all errors.

This closure currently rewrites all decode failures as “unknown encoding”, which masks real decode/type errors for known codecs. Only LookupError should be translated; other exceptions should pass through unchanged.

Proposed fix

             let decoded = vm
                 .state
                 .codec_registry
                 .decode_text(bytes_obj.into(), enc, None, vm)
-                .map_err(|_| {
-                    vm.new_exception_msg(
-                        vm.ctx.exceptions.syntax_error.to_owned(),
-                        format!("unknown encoding for '{filename}': {enc}").into(),
-                    )
-                })?;
+                .map_err(|exc| {
+                    if exc.isinstance(vm.ctx.exceptions.lookup_error) {
+                        vm.new_exception_msg(
+                            vm.ctx.exceptions.syntax_error.to_owned(),
+                            format!("unknown encoding for '{filename}': {enc}").into(),
+                        )
+                    } else {
+                        exc
+                    }
+                })?;

#!/bin/bash
# Verify current error remapping site and exception-type check patterns.
rg -nP --type=rust -C4 '\.decode_text\(' crates/vm/src/stdlib/builtins.rs
rg -nP --type=rust -C2 '\.isinstance\(' crates/vm/src | head -n 40

🤖 Prompt for AI Agents

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

In `@crates/vm/src/stdlib/builtins.rs` around lines 239 - 244, The current map_err
closure unconditionally rewrites any decode failure into a SyntaxError "unknown
encoding", which masks real decode/type errors; change the closure used around
the decode call so it only converts the error when it is a LookupError from the
encoder lookup and otherwise returns the original error unchanged — e.g. inspect
the error value inside the map_err (or use match/if let) and only call
vm.new_exception_msg(..., format!("unknown encoding for '{filename}':
{enc}").into()) when the err is a LookupError; for any other error, propagate
the original error (do not rewrap) so real decode/type errors are preserved
(referencing the existing vm.new_exception_msg, vm.ctx.exceptions.syntax_error
and the decode call surrounding .map_err).

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

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

235-244: ⚠️ Potential issue | 🟠 Major

Preserve non-lookup codec errors instead of rewriting all failures.

At Line 239, map_err(|_| ...) converts every decode failure into unknown encoding, which hides real decode/type errors from codec execution. Only LookupError should be rewritten; everything else should propagate unchanged.

Proposed fix

             let decoded = vm
                 .state
                 .codec_registry
                 .decode_text(bytes_obj.into(), enc, None, vm)
-                .map_err(|_| {
-                    vm.new_exception_msg(
-                        vm.ctx.exceptions.syntax_error.to_owned(),
-                        format!("unknown encoding for '{filename}': {enc}").into(),
-                    )
-                })?;
+                .map_err(|exc| {
+                    if exc.isinstance(vm.ctx.exceptions.lookup_error) {
+                        vm.new_exception_msg(
+                            vm.ctx.exceptions.syntax_error.to_owned(),
+                            format!("unknown encoding for '{filename}': {enc}").into(),
+                        )
+                    } else {
+                        exc
+                    }
+                })?;

#!/bin/bash
# Verify how decode_text errors are produced and ensure only LookupError is remapped.
# Expected: decode_text can return non-LookupError exceptions, which should not be masked.
rg -n --type=rust -C4 '\bdecode_text\s*\(' crates/vm/src
rg -n --type=rust -C6 'pub fn decode_text|fn decode_text|lookup_error' crates/vm/src/codecs.rs crates/vm/src/stdlib/builtins.rs

🤖 Prompt for AI Agents

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

In `@crates/vm/src/stdlib/builtins.rs` around lines 235 - 244, The current map_err
on vm.state.codec_registry.decode_text(...) swallows all decoder errors and
rewraps them as a syntax_error with "unknown encoding", but only LookupError
should be remapped; preserve and propagate any other errors from decode_text.
Change the map_err closure to match on the error variant returned by decode_text
(check for LookupError) and convert only that case into
vm.new_exception_msg(vm.ctx.exceptions.syntax_error, format!("unknown encoding
for '{filename}': {enc}")), otherwise return the original error unchanged so
non-lookup decode/type errors propagate; locate this change around the call to
decode_text in builtins.rs and update the map_err handling accordingly.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro