Actionable comments posted: 2

In `@crates/vm/src/builtins/template.rs`:
- Around line 297-304: The __reduce__ for PyTemplateIter (method reduce)
currently returns (TemplateIterType, (template,)) which fails because
PyTemplateIter lacks a Constructor; update reduce to return the same
reconstruction pattern used by PyTemplate by returning the helper function
_template_unpickle as the callable and the template tuple as its args (i.e.,
return (_template_unpickle, (zelf.template.clone(),))) so unpickling calls the
helper instead of trying to call TemplateIterType directly.

In `@crates/vm/src/frame.rs`:
- Around line 706-755: PyTemplate::new and PyInterpolation::new currently skip
the invariant checks performed in their py_new implementations; update these
simple constructors to validate the same invariants: in PyTemplate::new, assert
strings.len() == interpolations.len() + 1 (use a debug_assert! or panic with a
clear message if you prefer non-debug enforcement) and in PyInterpolation::new
validate that conversion is either vm.ctx.none() or a single-character string in
{'s','r','a'} (mirror the checks and error text from interpolation.rs:py_new and
template.rs:py_new), failing fast with a clear message when violated so
directly-constructed objects cannot be malformed.

crates/vm/src/stdlib/ast/string.rs (2)

329-356: Avoid quadratic iteration when consuming f-string parts.

iter_mut().nth(i) inside the loop re-traverses the slice each iteration. Iterating directly keeps this O(n) and reads cleaner.

♻️ Proposed refactor

-    for i in 0..value.as_slice().len() {
-        let part = core::mem::replace(value.iter_mut().nth(i).unwrap(), default_part.clone());
+    for part in value.iter_mut() {
+        let part = core::mem::replace(part, default_part.clone());

---

575-589: Avoid quadratic iteration in tstring_to_object.

Same pattern as above—direct iteration over value.iter_mut() avoids O(n²) traversal.

♻️ Proposed refactor

-    for i in 0..value.as_slice().len() {
-        let tstring = core::mem::replace(value.iter_mut().nth(i).unwrap(), default_tstring.clone());
+    for tstring in value.iter_mut() {
+        let tstring = core::mem::replace(tstring, default_tstring.clone());

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between c934265 and b1114b4.

• Lib/_opcode_metadata.py is excluded by !Lib/**

• crates/codegen/src/compile.rs
• crates/codegen/src/symboltable.rs
• crates/compiler-core/src/bytecode/instruction.rs
• crates/vm/src/builtins/interpolation.rs
• crates/vm/src/builtins/mod.rs
• crates/vm/src/builtins/template.rs
• crates/vm/src/frame.rs
• crates/vm/src/stdlib/ast/expression.rs
• crates/vm/src/stdlib/ast/pyast.rs
• crates/vm/src/stdlib/ast/string.rs
• crates/vm/src/types/zoo.rs
• crates/vm/src/vm/vm_ops.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style using cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

• crates/vm/src/types/zoo.rs
• crates/vm/src/stdlib/ast/expression.rs
• crates/codegen/src/symboltable.rs
• crates/vm/src/builtins/mod.rs
• crates/vm/src/vm/vm_ops.rs
• crates/vm/src/stdlib/ast/pyast.rs
• crates/vm/src/builtins/interpolation.rs
• crates/compiler-core/src/bytecode/instruction.rs
• crates/vm/src/builtins/template.rs
• crates/vm/src/frame.rs
• crates/codegen/src/compile.rs
• crates/vm/src/stdlib/ast/string.rs

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

• crates/vm/src/builtins/mod.rs
• crates/vm/src/frame.rs

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

• iter (234-236)

crates/vm/src/stdlib/ast/string.rs (1)

• tstring_to_object (565-596)

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

• format_spec (128-130)

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

• element (650-650)

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

• template (140-165)

crates/vm/src/protocol/mapping.rs (1)

• slots (108-110)

crates/vm/src/protocol/sequence.rs (1)

• slots (134-136)

crates/vm/src/types/zoo.rs (1)

• init (109-203)

crates/compiler-core/src/bytecode/oparg.rs (2)

• from (25-27)
• from (68-70)

crates/vm/src/types/slot.rs (1)

• sequence_downcast (1982-1984)

crates/vm/src/builtins/template.rs (2)

• interpolations (103-105)
• strings (98-100)

crates/vm/src/builtins/interpolation.rs (4)

• format_spec (128-130)
• expression (118-120)
• value (113-115)
• conversion (123-125)

crates/vm/src/stdlib/ast/expression.rs (16)

• ast_to_object (11-53)
• ast_to_object (148-165)
• ast_to_object (191-208)
• ast_to_object (234-254)
• ast_to_object (285-302)
• ast_to_object (327-344)
• ast_to_object (370-390)
• ast_to_object (421-445)
• ast_to_object (477-491)
• ast_to_object (511-528)
• ast_to_object (554-571)
• ast_to_object (597-617)
• ast_to_object (648-666)
• ast_to_object (694-708)
• ast_to_object (728-742)
• ast_to_object (761-775)

crates/vm/src/stdlib/ast/expression.rs (1)

39-39: TString conversion is now wired in cleanly.

This removes the unimplemented path and routes t-strings through the dedicated converter as expected.

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

1444-1456: T-string interpolation scanning mirrors f-strings correctly.

Nice alignment with the f-string path, including format-spec traversal.

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

267-279: Opcode definitions and decode registration are consistent.

Good to see BuildTemplate/BuildInterpolation added to both the enum and custom opcode list.

Also applies to: 441-445

---

802-809: Stack-effect and disassembly updates look consistent.

The stack deltas and disassembly names align with the new template/interpolation ops.

Also applies to: 1004-1005

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

49-203: Interpolation constructor and protocol impls look solid.

Validation, reduce, equality, hashing, and repr are consistent and well-scoped.

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

17-197: Template normalization, concat, and iteration logic look consistent.

Constructor normalization and concat boundary handling read clean, and the iterator skips empty strings as intended.

Also applies to: 199-267, 269-293, 308-341, 343-346

crates/vm/src/stdlib/ast/string.rs (1)

366-563: TemplateStr/Interpolation AST wiring looks consistent.

Nice alignment with the existing JoinedStr/FormattedValue flow.

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

39-41: New interpolation/template exports are wired cleanly.

Also applies to: 81-82

crates/vm/src/types/zoo.rs (1)

6-9: TypeZoo registration for template/interpolation looks complete.

Also applies to: 96-98, 194-196, 252-253

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

383-387: sequence_unchecked() is safe for non-sequence operands—it's a thin wrapper without assertions.

The method simply wraps the object in a PySequence struct without validation. Safety is guaranteed by subsequent .slots().concat.load() checks, which return Option<T> and are guarded by if let Some(f) conditionals. Non-sequences safely return None from .load(), preventing execution.

crates/vm/src/stdlib/ast/pyast.rs (2)

935-936: LGTM! Module registration is correctly placed and follows established patterns.

The new nodes are appropriately registered after JoinedStr and FormattedValue, grouping related string expression types together.

---

451-463: LGTM! New AST nodes correctly implement PEP 750 template string support.

The TemplateStr and Interpolation node definitions follow the established impl_node! macro pattern and correctly mirror their f-string counterparts (JoinedStr and FormattedValue). Field definitions align with PEP 750:

• TemplateStr.values: sequence of constants and interpolations
• Interpolation: includes value, str (original expression source text), conversion, and format_spec fields, matching the PEP 750 specification exactly

crates/codegen/src/compile.rs (4)

27-33: Import updates look good.
The added ExprTString/TString imports align with the new t-string codegen path.

---

6285-6287: TString expression dispatch is wired correctly.
Routing Expr::TString into the new compiler path is the right integration point.

---

7495-7514: Stack contract and oparg encoding are correctly implemented.

The BuildTemplate/BuildInterpolation stack order and bit layout match across compiler and VM:

• BuildTemplate receives [strings_tuple, interpolations_tuple] after the swap (line 7511)
• BuildInterpolation oparg encoding (conversion << 2) | has_format_spec matches VM expectations
• Conversion bits 2+ encode None (0), Str (1), Repr (2), Ascii (3); has_format_spec is bit 0

---

7526-7531: The suggested code references a non-existent field and will not compile.

InterpolatedStringLiteralElement (the type of lit) has only three fields: node_index, range, and value. It does not have a flags field. The flags information is stored on the parent TString struct, not on individual literal elements.

For this change to work correctly, compile_tstring_into would need to be modified to:

1. Accept the tstring.flags parameter
2. Use tstring.flags (not lit.flags) when calling parse_fstring_literal_element
3. Update the function signature accordingly

The core concern about parity with f-string surrogate handling is reasonable, but the proposed implementation is incorrect.

Likely an incorrect or invalid review comment.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.