Actionable comments posted: 0

🧹 Nitpick comments (2)

stdlib/src/sqlite.rs (2)

900-910: Clamp timeout to avoid c_int overflow

let timeout = (args.timeout * 1000.0) as c_int; can overflow for large timeouts. Clamp and round before casting.

Apply within this block:

-            let timeout = (args.timeout * 1000.0) as c_int;
-            db.busy_timeout(timeout);
+            // Saturate to c_int::MAX and avoid negatives
+            let timeout_ms = (args.timeout * 1000.0).clamp(0.0, i32::MAX as f64).round() as c_int;
+            db.busy_timeout(timeout_ms);

---

911-922: Subclass init path: good; consider making sqlite.connect() call the factory so init runs

__init__ correctly performs one-time DB open for subclassed instances. However, sqlite.connect() still constructs via Connection::py_new(...), so a factory=SubClass won’t execute the subclass’ __init__ (unlike CPython, which calls factory(...)). Recommend updating connect() to invoke the factory type, letting Python dispatch call __new__ then __init__.

Proposed approach (conceptual; outside this hunk): build args tuple matching ConnectArgs order and call the type:

// Pseudocode: call the chosen factory so __init__ runs for subclasses
// let obj = vm.invoke(args.factory.clone().into_object(),
//     (args.database, args.timeout, args.detect_types, args.isolation_level,
//      args.check_same_thread, /* omit passing 'factory' itself */
//      args.cached_statements, args.uri))?;

Please verify no downstream assumptions rely on the current “py_new-only” construction.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fcf1969 and 38e9fc5.

⛔ Files ignored due to path filters (1)

• Lib/test/test_sqlite3/test_regression.py is excluded by !Lib/**

📒 Files selected for processing (1)

• stdlib/src/sqlite.rs (3 hunks)

🧰 Additional context used 📓 Path-based instructions (2) **/*.rs

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

**/*.rs: Format Rust code with the default rustfmt style (run cargo fmt)
Run clippy and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management

Files:

• stdlib/src/sqlite.rs

{vm,stdlib}/**/*.rs

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

Use RustPython macros (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

• stdlib/src/sqlite.rs

🧬 Code graph analysis (1) stdlib/src/sqlite.rs (3)

vm/src/builtins/type.rs (1)

• class (147-149)

vm/src/object/core.rs (1)

• class (671-673)

Lib/test/test_sqlite3/test_regression.py (1)

• Connection (229-231)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Run snippets and cpython tests (windows-latest)

🔇 Additional comments (2)

stdlib/src/sqlite.rs (2)

854-879: Eager base init + deferred subclass init is correct

Type check via cls.is(Connection::class(&vm.ctx).as_object()) and gating db initialization achieves the CPython-compatible behavior while preserving defaults (text_factory, thread id). Looks good.

---

936-940: Error text parity with CPython

Updated message “Base Connection.init not called.” matches expected test string and CPython behavior. Good.

Run the sqlite3 regression test to confirm: test_connection_constructor_call_check passes with this change.

Great, thank you!