Fix sqlite autocommit lifecycle - #8387

crates/stdlib/src/_sqlite3.rs (1)

1133-1165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting the shared "exec + reopen BEGIN" logic in commit/rollback.

The Disabled arms of commit() (1140-1143) and rollback() (1160-1163) differ only in the first statement (COMMIT vs ROLLBACK) but share an identical trailing db._exec(b"BEGIN\0", vm). Per repo guidelines, extract the differing value and call the shared logic once.

♻️ Suggested refactor

+        fn exec_and_reopen(db: &Sqlite, stmt: &[u8], vm: &VirtualMachine) -> PyResult<()> {
+            db._exec(stmt, vm)?;
+            db._exec(b"BEGIN\0", vm)
+        }
+
         fn commit(&self, vm: &VirtualMachine) -> PyResult<()> {
             let db = self.db_lock(vm)?;
             let mode = *self.autocommit.lock();
             match mode {
                 AutocommitMode::Legacy => db.implicit_commit(vm),
                 AutocommitMode::Enabled => Ok(()),
-                AutocommitMode::Disabled => {
-                    db._exec(b"COMMIT\0", vm)?;
-                    db._exec(b"BEGIN\0", vm)
-                }
+                AutocommitMode::Disabled => Self::exec_and_reopen(&db, b"COMMIT\0", vm),
             }
         }

         fn rollback(&self, vm: &VirtualMachine) -> PyResult<()> {
             let db = self.db_lock(vm)?;
             let mode = *self.autocommit.lock();
             match mode {
                 AutocommitMode::Legacy => {
                     if db.is_autocommit() {
                         Ok(())
                     } else {
                         db._exec(b"ROLLBACK\0", vm)
                     }
                 }
                 AutocommitMode::Enabled => Ok(()),
-                AutocommitMode::Disabled => {
-                    db._exec(b"ROLLBACK\0", vm)?;
-                    db._exec(b"BEGIN\0", vm)
-                }
+                AutocommitMode::Disabled => Self::exec_and_reopen(&db, b"ROLLBACK\0", vm),
             }
         }

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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/stdlib/src/_sqlite3.rs` around lines 1133 - 1165, Extract the shared
Disabled-mode transaction flow from commit and rollback into a helper or common
local path that accepts the differing SQL command, COMMIT or ROLLBACK, executes
it through db._exec, then executes BEGIN once. Update both commit and rollback
to supply their respective command while preserving the existing Legacy and
Enabled behavior.

Source: Coding guidelines