Skip to content

Rust refactoring, fix constraint error message handling#33

Merged
ocean merged 11 commits intomainfrom
constraint-fixes
Dec 17, 2025
Merged

Rust refactoring, fix constraint error message handling#33
ocean merged 11 commits intomainfrom
constraint-fixes

Conversation

@ocean
Copy link
Copy Markdown
Owner

@ocean ocean commented Dec 16, 2025

Summary by CodeRabbit

  • New Features

    • Batch execution, prepared-statement support, cursor streaming, savepoints, replication controls, connection controls (busy timeout, reset, interrupt), metadata/PRAGMA queries, manual sync and statement introspection.
  • Bug Fixes

    • Improved constraint error messages, thread-safety and scheduler handling, binary/allocation error handling, SQL quoting and replication/connection performance hardening.
  • Documentation

    • CHANGELOG and contributor/editorial docs reorganised and standardised.
  • Tests

    • Test suite modularised, expanded and hardened; remote metadata tests enabled; statement column metadata tests added.
  • Chores

    • .gitignore updated; local settings allow WebSearch.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 16, 2025

Walkthrough

Modularised the Rust NIF into multiple modules, added shared Tokio runtime and global registries, implemented NIFs for connections, queries, transactions, statements, cursors, batching, replication and metadata, added utilities/decoders/tests, adjusted Elixir constraint-name extraction, and updated docs/config.

Changes

Cohort / File(s) Summary
Config & Ignore
\.claude/settings.local.json, \.gitignore
Added WebSearch to allow-list in settings and added .env.local to .gitignore.
Changelog & Docs
CHANGELOG.md, CLAUDE.md
Added an Unreleased changelog describing modularisation, hardening and fixes; rewrote editorial/formatting guidance and dev workflow.
Elixir adapter tweak
lib/ecto/adapters/libsql/connection.ex
Enhanced extract_constraint_name/1 to parse (index: <name>) and fallback to previous constraint extraction logic.
Root NIF modularisation
native/ecto_libsql/src/lib.rs
Replaced monolithic lib.rs with modular public modules and selective re-exports; retained rustler init and test scaffold.
Constants & registries
native/ecto_libsql/src/constants.rs
Added shared TOKIO_RUNTIME, DEFAULT_SYNC_TIMEOUT_SECS, NIF atoms and global registries: CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY.
Models / Resources
native/ecto_libsql/src/models.rs
Added LibSQLConn, CursorData, TransactionEntry structs and Mode enum; implemented resource wrapper for LibSQLConn.
Decode & validation
native/ecto_libsql/src/decode.rs
Added mode/transaction-behaviour decoders, ownership verifiers and savepoint name validation.
Utilities & routing
native/ecto_libsql/src/utils.rs
Added safe-lock helpers, timeouted sync helper, result builders, enhanced constraint error lookup, row collection/encoding, query-type enum and detection (detect_query_type, should_use_query), and term→Value decoding.
Connection NIFs
native/ecto_libsql/src/connection.rs
Implemented connect/ping/close/set_busy_timeout/reset_connection/interrupt_connection NIFs with registry management and tokio-backed async calls.
Query & PRAGMA
native/ecto_libsql/src/query.rs
Added query_args, do_sync, pragma_query NIFs; includes constraint error enhancement and sync behaviour.
Metadata
native/ecto_libsql/src/metadata.rs
Added last_insert_rowid, changes, total_changes (DirtyIo), and is_autocommit NIFs.
Prepared statements
native/ecto_libsql/src/statement.rs
Added prepare/query/execute prepared-statement lifecycle, parameter/column metadata APIs (stmt_column_count, stmt_column_name, stmt_parameter_count) and ownership checks.
Cursors
native/ecto_libsql/src/cursor.rs
Added declare_cursor, declare_cursor_with_context and fetch_cursor implementations with registry-backed cursor state and ownership validation.
Transactions & savepoints
native/ecto_libsql/src/transaction.rs, native/ecto_libsql/src/savepoint.rs
Added TransactionEntryGuard, begin/commit/rollback flows, transactional query/execute APIs, savepoint/release/rollback NIFs and ownership/guarding semantics.
Batching
native/ecto_libsql/src/batch.rs
Added non-transactional and transactional batch execution NIFs plus native-script variants, returning per-statement results.
Replication
native/ecto_libsql/src/replication.rs
Added replication NIFs: get_frame_number, sync_until, flush_replicator, max_write_replication_index and freeze_database (unsupported stub).
Tests reorganisation
native/ecto_libsql/src/tests.rs (removed), native/ecto_libsql/src/tests/*
Removed monolithic tests.rs; added tests/mod.rs, constants_tests.rs, integration_tests.rs, utils_tests.rs with focused unit and integration tests.
Elixir tests updated
test/turso_remote_test.exs, test/statement_features_test.exs
Re-enabled and relaxed remote metadata tests; replaced get_statement_columns usage with stmt_column_count + stmt_column_name calls, added out-of-bounds checks and adjusted assertions.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Elixir as Elixir client
    participant NIF as Rust NIF entry
    participant REG as Registries (CONNECTION / TXN / STMT / CURSOR)
    participant TOKIO as Tokio runtime
    participant DB as libsql DB

    Elixir->>NIF: begin_transaction(conn_id)
    NIF->>REG: lookup CONNECTION_REGISTRY(conn_id)
    NIF->>TOKIO: spawn async begin transaction
    TOKIO->>DB: db.begin_transaction()
    DB-->>TOKIO: Transaction
    TOKIO-->>NIF: trx created / id
    NIF->>REG: insert TXN_REGISTRY[trx_id]
    NIF-->>Elixir: {:ok, trx_id}

    Elixir->>NIF: query_with_trx_args(trx_id, conn_id, sql, args)
    NIF->>REG: take TXN_REGISTRY[trx_id] (guard)
    NIF->>NIF: should_use_query(sql) -> route decision
    alt SELECT/RETURNING path
      NIF->>TOKIO: transaction.query()
    else execute path
      NIF->>TOKIO: transaction.execute()
    end
    TOKIO->>DB: execute SQL
    DB-->>TOKIO: rows / affected
    TOKIO-->>NIF: result
    NIF->>REG: re-insert TXN_REGISTRY[trx_id] (guard Drop)
    NIF-->>Elixir: result map (columns, rows, num_rows)

    Elixir->>NIF: commit_or_rollback_transaction(trx_id,...)
    NIF->>REG: take & consume TXN_REGISTRY[trx_id]
    NIF->>TOKIO: commit/rollback
    TOKIO->>DB: transaction.commit()/rollback()
    DB-->>TOKIO: status
    TOKIO-->>NIF: status
    NIF-->>Elixir: {:ok, "committed"} / {:error, reason}
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Areas needing careful review:
    • Registry locking order and potential deadlocks across CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY
    • TransactionEntryGuard ownership/take/consume semantics and error-path correctness
    • Ownership checks for statements, cursors and transactions across NIF entry points
    • Interaction of shared TOKIO_RUNTIME with rustler scheduling (DirtyIo annotations) and timeout handling
    • Query routing heuristic (should_use_query) correctness and encoding/decoding edge cases

Possibly related PRs

Poem

🐇
I hopped through crates and split the burrow wide,
Registries snug where connections hide.
Guards hold the greens, cursors nibble rows,
Batches bound, transactions compose.
A small rabbit cheers — modules tidy, code spry.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately reflects the main changes: Rust code refactoring (modularisation of monolithic lib.rs into 13 modules) and constraint error message handling fixes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch constraint-fixes

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8bb1e64 and a65ab6b.

📒 Files selected for processing (1)
  • test/statement_features_test.exs (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/statement_features_test.exs
⏰ 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). (4)
  • GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
  • GitHub Check: Rust Checks (macos-latest, stable)
  • GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
  • GitHub Check: Analyze (rust)

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (19)
.claude/settings.local.json (1)

34-34: Minor observation: Unrelated change in scope.

The addition of the "WebSearch" permission doesn't appear to relate to the PR objectives (Rust refactoring, constraint error fixes, Turso test improvements). This seems to be a developer tooling preference that could be committed separately or mentioned in the PR description for clarity.

CLAUDE.md (4)

23-25: Reduce repetition in formatting rules for improved readability.

Lines 23-25 start three consecutive instructions with "ALWAYS", which creates redundant phrasing. The static analysis tool flagged this as excessive word repetition at the document start.

Apply this refactoring to consolidate and vary the phrasing:

- ALWAYS use British/Australian English spelling and grammar for code, comments, and documentation, except where required for function calls etc that may be in US English, such as SQL keywords or error messages, or where required for compatibility with external systems.
- ALWAYS run the Elixir formatter (`mix format --check-formatted`) before committing changes and fix any issues.
- ALWAYS run the Rust Cargo formatter (`cargo fmt`) before committing changes and fix any issues.
+ Use British/Australian English spelling and grammar for code, comments, and documentation (except where required for function calls, SQL keywords, or external system compatibility).
+ Before committing, ensure both formatters pass:
+   - Elixir: `mix format --check-formatted`
+   - Rust: `cargo fmt`

539-539: Add hyphen to fix grammar in "Must have tests for" heading.

Line 539 uses "Must have tests for" as a heading, but the static analysis tool correctly identified this needs a hyphen for proper compound adjective grammar.

- **Must have tests for**:
+ **Must-have tests for**:

1067-1073: Update important commands section to reflect new formatting requirements.

Lines 1067-1073 show the commands section but should be updated to reflect the consolidated formatting guidance from lines 23-25. Currently shows separate commands; could be consolidated.

After refactoring lines 23-25, update this section to:

  # Format & checks (ALWAYS before commit)
- mix format --check-formatted && cd native/ecto_libsql && cargo fmt
+ mix format && cd native/ecto_libsql && cargo fmt
+ mix format --check-formatted

(Only if lines 23-25 are refactored as suggested above.)


1144-1144: Simplify word choice in summary for improved conciseness.

Line 1144 uses "lots of tests" which static analysis suggests could be more concise and formal. In technical documentation, a more direct term would strengthen the statement.

- EctoLibSql is a production-ready Ecto adapter for LibSQL/Turso with full Ecto support, three connection modes, advanced features (vector search, encryption, streaming), zero panic risk, lots of tests, and comprehensive documentation.
+ EctoLibSql is a production-ready Ecto adapter for LibSQL/Turso with full Ecto support, three connection modes, advanced features (vector search, encryption, streaming), zero panic risk, extensive test coverage, and comprehensive documentation.
native/ecto_libsql/src/lib.rs (1)

1-4: Consider using inner doc comments for module-level documentation.

The module-level documentation uses /// (outer doc comments) instead of //! (inner doc comments). While this works, inner doc comments are the idiomatic Rust style for documenting the current module.

-/// EctoLibSql: Ecto adapter for LibSQL/Turso databases
-///
-/// This is the root module for the EctoLibSql NIF (Native Implemented Function) library.
-/// It declares and organizes all submodules handling different aspects of database operations.
+//! EctoLibSql: Ecto adapter for LibSQL/Turso databases
+//!
+//! This is the root module for the EctoLibSql NIF (Native Implemented Function) library.
+//! It declares and organizes all submodules handling different aspects of database operations.
native/ecto_libsql/src/cursor.rs (1)

312-318: Blob allocation failure silently returns nil.

When OwnedBinary::new(b.len()) returns None (allocation failure), the code silently returns nil instead of propagating an error. For large blobs, this could lead to unexpected nil values.

Consider logging or returning an error for allocation failures:

                     Value::Blob(b) => match OwnedBinary::new(b.len()) {
                         Some(mut owned) => {
                             owned.as_mut_slice().copy_from_slice(b);
                             Binary::from_owned(owned, env).encode(env)
                         }
-                        None => rustler::types::atom::nil().encode(env),
+                        None => {
+                            // Log or handle allocation failure appropriately
+                            // For now, return nil but consider returning an error atom
+                            rustler::types::atom::nil().encode(env)
+                        }
                     },
native/ecto_libsql/src/savepoint.rs (3)

35-55: Mutex held across async block_on call may cause contention.

The txn_registry lock is acquired at line 35 but held through the TOKIO_RUNTIME.block_on() call at lines 50-52. As per the coding guidelines, locks should be dropped before async blocks to prevent holding locks across await points.

Consider restructuring to extract the transaction and drop the lock before the async operation:

-    let mut txn_registry = safe_lock(&TXN_REGISTRY, "savepoint")?;
-
-    let entry = txn_registry
-        .get_mut(trx_id)
-        .ok_or_else(|| rustler::Error::Term(Box::new("Transaction not found")))?;
-
-    // Verify that the transaction belongs to the requesting connection
-    if entry.conn_id != conn_id {
-        return Err(rustler::Error::Term(Box::new(
-            "Transaction does not belong to this connection",
-        )));
-    }
-
-    let sql = format!("SAVEPOINT {}", name);
-
-    TOKIO_RUNTIME
-        .block_on(async { entry.transaction.execute(&sql, Vec::<Value>::new()).await })
-        .map_err(|e| rustler::Error::Term(Box::new(format!("Savepoint failed: {}", e))))?;
+    let sql = format!("SAVEPOINT {}", name);
+    
+    // Scope the lock to validation only
+    let txn_clone = {
+        let txn_registry = safe_lock(&TXN_REGISTRY, "savepoint")?;
+        let entry = txn_registry
+            .get(trx_id)
+            .ok_or_else(|| rustler::Error::Term(Box::new("Transaction not found")))?;
+        
+        if entry.conn_id != conn_id {
+            return Err(rustler::Error::Term(Box::new(
+                "Transaction does not belong to this connection",
+            )));
+        }
+        // Clone/Arc the transaction reference if possible
+        entry.transaction.clone()
+    }; // Lock dropped here
+    
+    TOKIO_RUNTIME
+        .block_on(async { txn_clone.execute(&sql, Vec::<Value>::new()).await })
+        .map_err(|e| rustler::Error::Term(Box::new(format!("Savepoint failed: {}", e))))?;

Note: This refactor depends on whether entry.transaction can be cloned or wrapped in an Arc. If not feasible due to the libsql Transaction type's ownership semantics, document why the lock must be held.

Based on learnings, for Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points.


70-94: Same lock contention pattern as savepoint function.

The release_savepoint function exhibits the same pattern where the TXN_REGISTRY lock is held across the async block_on call. Apply the same restructuring as suggested for savepoint if feasible.


109-135: Same lock contention pattern as other savepoint functions.

The rollback_to_savepoint function also holds the registry lock across the async operation. Consider applying the same refactoring pattern to all three functions for consistency.

Overall, the savepoint module correctly implements:

  • Savepoint name validation
  • Transaction ownership verification
  • Proper error handling with descriptive messages
  • Correct NIF annotations with DirtyIo scheduling
native/ecto_libsql/src/replication.rs (1)

157-180: Consider removing unnecessary async wrapper.

The max_write_replication_index call at line 171 appears to be synchronous (no .await). Wrapping it in TOKIO_RUNTIME.block_on() adds unnecessary overhead.

 #[rustler::nif(schedule = "DirtyIo")]
 pub fn max_write_replication_index(conn_id: &str) -> NifResult<u64> {
     let conn_map = safe_lock(&CONNECTION_REGISTRY, "max_write_replication_index conn_map")?;
     let client = conn_map
         .get(conn_id)
         .ok_or_else(|| rustler::Error::Term(Box::new("Connection not found")))?
         .clone();
     drop(conn_map);

-    let result = TOKIO_RUNTIME.block_on(async {
-        let client_guard = safe_lock_arc(&client, "max_write_replication_index client")
-            .map_err(|e| format!("Failed to lock client: {:?}", e))?;
-
-        // Call max_write_replication_index() which returns Option<FrameNo>
-        let max_write_frame = client_guard.db.max_write_replication_index();
-
-        Ok::<_, String>(max_write_frame.unwrap_or(0))
-    });
+    let client_guard = safe_lock_arc(&client, "max_write_replication_index client")?;
+    let max_write_frame = client_guard.db.max_write_replication_index();
+    Ok(max_write_frame.unwrap_or(0))
-
-    match result {
-        Ok(frame_no) => Ok(frame_no),
-        Err(e) => Err(rustler::Error::Term(Box::new(e))),
-    }
 }
native/ecto_libsql/src/connection.rs (1)

56-57: Consider using the global TOKIO_RUNTIME instead of creating a new runtime per connection.

The connect function creates a new tokio::runtime::Runtime for each connection attempt, while other functions in this codebase (e.g., ping, set_busy_timeout) use the shared TOKIO_RUNTIME from constants.rs. Creating a runtime is expensive and may lead to resource exhaustion under high connection rates.

-    let rt = tokio::runtime::Runtime::new()
-        .map_err(|e| rustler::Error::Term(Box::new(format!("Tokio runtime err {}", e))))?;
-
-    // Wrap the entire connection process with a timeout.
-    rt.block_on(async {
+    // Wrap the entire connection process with a timeout.
+    TOKIO_RUNTIME.block_on(async {
native/ecto_libsql/src/models.rs (1)

40-48: Consider adding #[derive(Debug)] to TransactionEntry for consistency.

Other structs in this file derive Debug for logging and debugging purposes, but TransactionEntry does not. This may complicate debugging transaction-related issues.

 /// Transaction entry with ownership tracking
 ///
 /// Tracks which connection owns a transaction and holds the transaction reference.
+#[derive(Debug)]
 pub struct TransactionEntry {
     /// Connection ID that created this transaction
     pub conn_id: String,
     /// The actual transaction object
     pub transaction: Transaction,
 }
native/ecto_libsql/src/statement.rs (2)

38-49: Locks acquired inside async block may cause issues.

The client and conn locks are acquired inside the TOKIO_RUNTIME.block_on async block (lines 42-43). As per the coding guidelines, locks should be dropped before async blocks to prevent holding locks across await points. While block_on itself is synchronous, the pattern here differs from other functions in this file that explicitly drop locks before block_on.

Consider restructuring to acquire the connection reference before the async block, similar to the pattern used in query_prepared:

     {
         let sql_to_prepare = sql.to_string();
+        let client_clone = client.clone();
 
         let stmt_result = TOKIO_RUNTIME.block_on(async {
-            let client_guard = utils::safe_lock_arc(&client, "prepare_statement client")?;
-            let conn_guard = utils::safe_lock_arc(&client_guard.client, "prepare_statement conn")?;
+            let client_guard = utils::safe_lock_arc(&client_clone, "prepare_statement client")?;
+            let conn_guard = utils::safe_lock_arc(&client_guard.client, "prepare_statement conn")?;
 
             conn_guard
                 .prepare(&sql_to_prepare)

Based on learnings, locks should be dropped before async blocks.


183-186: Dead code: _is_sync is computed but never used.

The _is_sync variable is computed on line 183 but never used. The comment on lines 200-201 explains that LibSQL handles sync automatically. Consider removing this computation entirely to avoid confusion.

-    let _is_sync = !matches!(utils::detect_query_type(sql_hint), utils::QueryType::Select);
-
     drop(stmt_registry); // Release lock before async operation
     drop(conn_map); // Release lock before async operation
native/ecto_libsql/src/transaction.rs (2)

161-169: Locks held during async operation.

The conn_map, conn_guard, and client_guard locks are held while calling client_guard.transaction().await inside block_on. Per coding guidelines, locks should be dropped before async blocks to prevent holding locks across await points.

Consider cloning the necessary references and dropping locks before the async operation, similar to patterns in other modules.

Based on learnings, locks should be dropped before async blocks.


200-214: Same lock pattern issue as begin_transaction.

This function holds multiple locks while executing the async transaction creation, similar to begin_transaction. Consider refactoring to drop locks before the async block.

native/ecto_libsql/src/utils.rs (2)

121-127: Consider quoting identifiers in PRAGMA queries.

The table_name and index_name (line 148) are interpolated directly into PRAGMA queries. While these values originate from SQLite's own error messages and PRAGMA results, consider quoting them for defensive programming:

-        let pragma_query = format!("PRAGMA index_list({})", table_name);
+        let pragma_query = format!("PRAGMA index_list(\"{}\")", table_name.replace("\"", "\"\""));

This protects against table names with special characters or spaces.


209-222: Silent error handling may hide data issues.

Line 221 converts row value errors to nil() silently. While this provides robustness, it could mask underlying data corruption or type mismatch issues. Consider logging these errors or returning a distinct error value if debugging becomes difficult.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b6adb9 and 712e58d.

📒 Files selected for processing (25)
  • .claude/settings.local.json (1 hunks)
  • .gitignore (1 hunks)
  • CHANGELOG.md (1 hunks)
  • CLAUDE.md (14 hunks)
  • lib/ecto/adapters/libsql/connection.ex (1 hunks)
  • native/ecto_libsql/src/batch.rs (1 hunks)
  • native/ecto_libsql/src/connection.rs (1 hunks)
  • native/ecto_libsql/src/constants.rs (1 hunks)
  • native/ecto_libsql/src/cursor.rs (1 hunks)
  • native/ecto_libsql/src/decode.rs (1 hunks)
  • native/ecto_libsql/src/lib.rs (1 hunks)
  • native/ecto_libsql/src/metadata.rs (1 hunks)
  • native/ecto_libsql/src/models.rs (1 hunks)
  • native/ecto_libsql/src/query.rs (1 hunks)
  • native/ecto_libsql/src/replication.rs (1 hunks)
  • native/ecto_libsql/src/savepoint.rs (1 hunks)
  • native/ecto_libsql/src/statement.rs (1 hunks)
  • native/ecto_libsql/src/tests.rs (0 hunks)
  • native/ecto_libsql/src/tests/constants_tests.rs (1 hunks)
  • native/ecto_libsql/src/tests/integration_tests.rs (1 hunks)
  • native/ecto_libsql/src/tests/mod.rs (1 hunks)
  • native/ecto_libsql/src/tests/utils_tests.rs (1 hunks)
  • native/ecto_libsql/src/transaction.rs (1 hunks)
  • native/ecto_libsql/src/utils.rs (1 hunks)
  • test/turso_remote_test.exs (2 hunks)
💤 Files with no reviewable changes (1)
  • native/ecto_libsql/src/tests.rs
🧰 Additional context used
📓 Path-based instructions (8)
lib/ecto/adapters/libsql/connection.ex

📄 CodeRabbit inference engine (CLAUDE.md)

lib/ecto/adapters/libsql/connection.ex: Generate SQL DDL operations in Connection module for CREATE TABLE, DROP TABLE, ALTER TABLE, CREATE INDEX, DROP INDEX, RENAME TABLE with proper constraint support
Implement query compilation in Connection module for SELECT, INSERT, UPDATE, DELETE statements with WHERE, JOIN, ORDER BY, LIMIT support
Use quote_name/1 function to safely quote identifiers (table names, column names, aliases) in generated SQL to prevent injection attacks

Files:

  • lib/ecto/adapters/libsql/connection.ex
lib/**/*.ex

📄 CodeRabbit inference engine (CLAUDE.md)

lib/**/*.ex: Include examples in docstrings for public Elixir functions using ## Examples section with doctests
Use Ecto changesets with proper validation and constraints for all schema operations
Use fragment/1 in Ecto queries for SQLite-specific functions like RANDOM(), ROWID, or custom SQL expressions
Document SQLite limitations in code comments, especially regarding ALTER TABLE, DROP COLUMN, and column type changes which require table recreation
Use transaction behaviours (:deferred, :immediate, :exclusive, :read_only) appropriately based on operation type - :immediate for write-heavy, :deferred for read-heavy
Use proper error matching in Elixir to handle EctoLibSql.Error exception type from failed database operations
Use sync/1 function to manually trigger replica synchronization with Turso cloud in embedded replica mode

Files:

  • lib/ecto/adapters/libsql/connection.ex
**/*.ex

📄 CodeRabbit inference engine (AGENTS.md)

Use Ecto changesets with validation rules and constraints in schema definitions

Files:

  • lib/ecto/adapters/libsql/connection.ex
*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Use Markdown files for documentation: AGENTS.md for API reference, CLAUDE.md for AI agent development guide, README.md for user documentation, CHANGELOG.md for version history, ECTO_MIGRATION_GUIDE.md for migration guidance, RUST_ERROR_HANDLING.md for error patterns

Files:

  • CHANGELOG.md
  • CLAUDE.md
CHANGELOG.md

📄 CodeRabbit inference engine (CLAUDE.md)

Update CHANGELOG.md with changes for each release following semantic versioning

Files:

  • CHANGELOG.md
native/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

native/**/*.rs: Never use .unwrap() in production Rust code - use safe_lock helper for Mutex, safe_lock_arc helper for Arc<Mutex>, ok_or_else for Option types, and ? operator for error propagation to prevent VM panics
Use safe_lock(&REGISTRY, "function_name context") helper function instead of .lock().unwrap() when accessing Mutex-protected registries in Rust
Use safe_lock_arc(&arc_mutex, "function_name context") helper function for locking Arc<Mutex> in Rust to prevent panics
Handle Option types using ok_or_else() instead of .unwrap() to return proper error tuples in Rust NIFs
For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points
In Rust async blocks, convert errors to String using .map_err(|e| format!("{:?}", e)) when async runtime errors are encountered
Define new Rust NIF functions with the #[rustler::nif(schedule = "DirtyIo")] attribute and implement them without manual function list in rustler::init!() - Rustler automatically detects annotated functions
Return NifResult<T> from all Rust NIF functions to enable proper error handling and prevent panics
Use the safe_lock helper in Rust to access the CONNECTION_REGISTRY without panicking on lock acquisition failure
Track transaction ownership in Rust using TransactionEntry struct containing both conn_id and transaction fields, and validate ownership in transaction operations
Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections
Implement proper error handling in all NIFs returning NifResult with descriptive error messages
In Rust code, validate query types (SELECT, INSERT, UPDATE, DELETE) to determine appropriate return format (rows vs. affected count)
Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and...

Files:

  • native/ecto_libsql/src/decode.rs
  • native/ecto_libsql/src/tests/mod.rs
  • native/ecto_libsql/src/tests/utils_tests.rs
  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/tests/constants_tests.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/constants.rs
  • native/ecto_libsql/src/tests/integration_tests.rs
  • native/ecto_libsql/src/lib.rs
test/**/*.exs

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.exs: Write Elixir tests in test/ directory following the pattern: setup database, create test data, run test, verify results, cleanup in on_exit callback
Organize test files by functionality: ecto_adapter_test.exs for storage operations, ecto_connection_test.exs for SQL generation, ecto_integration_test.exs for full workflows, ecto_migration_test.exs for migrations, error_handling_test.exs for error cases
Use ExUnit.Case for Elixir unit and integration tests with setup callbacks for test isolation and on_exit for cleanup
Test database locking scenarios with connection pool exhaustion and timeout configurations to ensure proper error handling

Files:

  • test/turso_remote_test.exs
test/turso_remote_test.exs

📄 CodeRabbit inference engine (CLAUDE.md)

Test Turso remote connections with valid credentials in separate test marked with :turso_remote tag to allow exclusion in CI

Files:

  • test/turso_remote_test.exs
🧠 Learnings (57)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : In Rust code, validate query types (SELECT, INSERT, UPDATE, DELETE) to determine appropriate return format (rows vs. affected count)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement proper error handling in all NIFs returning NifResult with descriptive error messages
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and transaction state
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Track transaction ownership in Rust using TransactionEntry struct containing both conn_id and transaction fields, and validate ownership in transaction operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use the safe_lock helper in Rust to access the CONNECTION_REGISTRY without panicking on lock acquisition failure
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Implement query compilation in Connection module for SELECT, INSERT, UPDATE, DELETE statements with WHERE, JOIN, ORDER BY, LIMIT support
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to test/**/*.exs : Write Elixir tests in test/ directory following the pattern: setup database, create test data, run test, verify results, cleanup in on_exit callback

Applied to files:

  • .gitignore
  • native/ecto_libsql/src/tests/mod.rs
  • CLAUDE.md
  • native/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to test/**/*.exs : Test database locking scenarios with connection pool exhaustion and timeout configurations to ensure proper error handling

Applied to files:

  • .gitignore
  • test/turso_remote_test.exs
  • native/ecto_libsql/src/tests/mod.rs
  • CLAUDE.md
  • native/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Generate SQL DDL operations in Connection module for CREATE TABLE, DROP TABLE, ALTER TABLE, CREATE INDEX, DROP INDEX, RENAME TABLE with proper constraint support

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/connection.rs
  • CLAUDE.md
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Document SQLite limitations in code comments, especially regarding ALTER TABLE, DROP COLUMN, and column type changes which require table recreation

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
  • CHANGELOG.md
  • test/turso_remote_test.exs
  • CLAUDE.md
  • native/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use proper error matching in Elixir to handle EctoLibSql.Error exception type from failed database operations

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
  • native/ecto_libsql/src/tests/mod.rs
  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Use `quote_name/1` function to safely quote identifiers (table names, column names, aliases) in generated SQL to prevent injection attacks

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement Ecto type loaders and dumpers in `lib/ecto/adapters/libsql.ex` for custom type conversions between Ecto and SQLite types

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
  • native/ecto_libsql/src/tests/mod.rs
  • CLAUDE.md
  • native/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/*.ex : Use the EctoLibSql.State struct to track connection state including conn_id, trx_id, mode (:local, :remote, :remote_replica), and sync setting

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
  • CHANGELOG.md
  • native/ecto_libsql/src/decode.rs
  • test/turso_remote_test.exs
  • native/ecto_libsql/src/tests/mod.rs
  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
  • native/ecto_libsql/src/constants.rs
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Implement query compilation in Connection module for SELECT, INSERT, UPDATE, DELETE statements with WHERE, JOIN, ORDER BY, LIMIT support

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/statement.rs
  • CLAUDE.md
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use Ecto changesets with proper validation and constraints for all schema operations

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Use DBConnection protocol implementation for connection lifecycle (connect, disconnect, ping) and query execution (execute, prepare, fetch)

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
  • native/ecto_libsql/src/tests/mod.rs
  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/statement.rs
  • CLAUDE.md
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
  • native/ecto_libsql/src/decode.rs
  • native/ecto_libsql/src/tests/mod.rs
  • native/ecto_libsql/src/tests/utils_tests.rs
  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/constants.rs
  • native/ecto_libsql/src/tests/integration_tests.rs
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Handle NIF errors gracefully by pattern matching on {:ok, result} and {:error, reason} returns from EctoLibSql.Native functions

Applied to files:

  • lib/ecto/adapters/libsql/connection.ex
  • native/ecto_libsql/src/tests/mod.rs
  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to CHANGELOG.md : Update CHANGELOG.md with changes for each release following semantic versioning

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to *.md : Use Markdown files for documentation: AGENTS.md for API reference, CLAUDE.md for AI agent development guide, README.md for user documentation, CHANGELOG.md for version history, ECTO_MIGRATION_GUIDE.md for migration guidance, RUST_ERROR_HANDLING.md for error patterns

Applied to files:

  • CHANGELOG.md
  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Track transaction ownership in Rust using TransactionEntry struct containing both conn_id and transaction fields, and validate ownership in transaction operations

Applied to files:

  • native/ecto_libsql/src/decode.rs
  • native/ecto_libsql/src/tests/utils_tests.rs
  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/tests/constants_tests.rs
  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
  • native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : In Rust code, validate query types (SELECT, INSERT, UPDATE, DELETE) to determine appropriate return format (rows vs. affected count)

Applied to files:

  • native/ecto_libsql/src/decode.rs
  • native/ecto_libsql/src/tests/utils_tests.rs
  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/tests/constants_tests.rs
  • CLAUDE.md
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/tests/integration_tests.rs
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections

Applied to files:

  • native/ecto_libsql/src/decode.rs
  • test/turso_remote_test.exs
  • native/ecto_libsql/src/tests/utils_tests.rs
  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
  • native/ecto_libsql/src/tests/integration_tests.rs
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use the safe_lock helper in Rust to access the CONNECTION_REGISTRY without panicking on lock acquisition failure

Applied to files:

  • native/ecto_libsql/src/decode.rs
  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/tests/constants_tests.rs
  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/constants.rs
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to test/turso_remote_test.exs : Test Turso remote connections with valid credentials in separate test marked with :turso_remote tag to allow exclusion in CI

Applied to files:

  • test/turso_remote_test.exs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/*.ex : Detect connection mode based on provided options: :local mode for database file paths, :remote mode for uri+auth_token, :remote_replica mode for database+uri+auth_token+sync:true

Applied to files:

  • test/turso_remote_test.exs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/connection.rs
  • CLAUDE.md
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to test/**/*.exs : Organize test files by functionality: ecto_adapter_test.exs for storage operations, ecto_connection_test.exs for SQL generation, ecto_integration_test.exs for full workflows, ecto_migration_test.exs for migrations, error_handling_test.exs for error cases

Applied to files:

  • native/ecto_libsql/src/tests/mod.rs
  • CLAUDE.md
  • native/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/tests.rs : Write Rust tests using #[tokio::test] for async operations with setup_test_db() and cleanup_test_db() helpers, tests may use .unwrap() for simplicity

Applied to files:

  • native/ecto_libsql/src/tests/mod.rs
  • native/ecto_libsql/src/tests/utils_tests.rs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/tests/constants_tests.rs
  • CLAUDE.md
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/constants.rs
  • native/ecto_libsql/src/tests/integration_tests.rs
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/tests.rs : Test code in Rust `#[cfg(test)]` modules may use `.unwrap()` for simplicity

Applied to files:

  • native/ecto_libsql/src/tests/mod.rs
  • native/ecto_libsql/src/tests/utils_tests.rs
  • native/ecto_libsql/src/tests/constants_tests.rs
  • CLAUDE.md
  • native/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to test/**/*.exs : Use ExUnit.Case for Elixir unit and integration tests with setup callbacks for test isolation and on_exit for cleanup

Applied to files:

  • native/ecto_libsql/src/tests/mod.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/constants.rs
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Define new Rust NIF functions with the `#[rustler::nif(schedule = "DirtyIo")]` attribute and implement them without manual function list in `rustler::init!()` - Rustler automatically detects annotated functions

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/batch.rs
  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and transaction state

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/tests/constants_tests.rs
  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/constants.rs
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use savepoints with create_savepoint and rollback_to_savepoint_by_name for partial rollback within transactions

Applied to files:

  • native/ecto_libsql/src/savepoint.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use sync/1 function to manually trigger replica synchronization with Turso cloud in embedded replica mode

Applied to files:

  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use max_write_replication_index to ensure read-your-writes consistency in remote replica mode

Applied to files:

  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Implement handle_begin/2, handle_commit/2, handle_rollback/2 callbacks for transaction management following DBConnection protocol

Applied to files:

  • native/ecto_libsql/src/models.rs
  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use batch operations (batch_transactional or execute_batch_sql) for bulk inserts instead of individual insert statements

Applied to files:

  • native/ecto_libsql/src/batch.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Implement handle_declare/4, handle_fetch/4, handle_deallocate/4 callbacks for cursor streaming operations

Applied to files:

  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/statement.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: For large result sets, use cursor streaming with DBConnection.stream() instead of loading all rows into memory

Applied to files:

  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Set busy_timeout with EctoLibSql.Native.busy_timeout for write-heavy workloads to handle database locks gracefully

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/constants.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use prepared statements for queries that will be executed multiple times, as statements are automatically cached after preparation

Applied to files:

  • native/ecto_libsql/src/statement.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use statement introspection functions (stmt_parameter_count, stmt_column_count, stmt_column_name) to validate prepared statement structure

Applied to files:

  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use `safe_lock(&REGISTRY, "function_name context")` helper function instead of `.lock().unwrap()` when accessing Mutex-protected registries in Rust

Applied to files:

  • native/ecto_libsql/src/tests/constants_tests.rs
  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/constants.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use transaction behaviours (:deferred, :immediate, :exclusive, :read_only) appropriately based on operation type - :immediate for write-heavy, :deferred for read-heavy

Applied to files:

  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
📚 Learning: 2025-12-05T22:12:22.789Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENT.md:0-0
Timestamp: 2025-12-05T22:12:22.789Z
Learning: Applies to AGENTS.md : Document agent architecture and design patterns in AGENTS.md

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to lib/**/*repo.ex : Configure Ecto repository in lib/ directory with adapter: Ecto.Adapters.LibSql

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement Ecto adapter callbacks: storage_up/1, storage_down/1, storage_status/1 for database lifecycle management

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Run both `mix test` for Elixir tests and `cargo test` for Rust tests before committing - pre-commit checklist should verify all tests pass

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Always format Elixir code using `mix format` before committing - unformatted code will fail CI

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement proper error handling in all NIFs returning NifResult with descriptive error messages

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Map Ecto :boolean type to INTEGER (0=false, 1=true) in SQLite with corresponding loaders and dumpers

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Document unsupported features by returning clear :unsupported atom errors, include explanation in docstring, and add tests asserting unsupported behavior

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Include examples in docstrings for public Elixir functions using `## Examples` section with doctests

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use Ecto.Query for building type-safe queries instead of raw SQL strings when possible

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use `fragment/1` in Ecto queries for SQLite-specific functions like `RANDOM()`, `ROWID`, or custom SQL expressions

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-05T22:12:22.789Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENT.md:0-0
Timestamp: 2025-12-05T22:12:22.789Z
Learning: Maintain agent implementation documentation standards

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: For Turso remote connections, include `libsql://` prefix in URI configuration

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use `safe_lock_arc(&arc_mutex, "function_name context")` helper function for locking Arc<Mutex<T>> in Rust to prevent panics

Applied to files:

  • native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Never use `.unwrap()` in production Rust code - use `safe_lock` helper for Mutex<T>, `safe_lock_arc` helper for Arc<Mutex<T>>, `ok_or_else` for Option types, and `?` operator for error propagation to prevent VM panics

Applied to files:

  • native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Return `NifResult<T>` from all Rust NIF functions to enable proper error handling and prevent panics

Applied to files:

  • native/ecto_libsql/src/lib.rs
🧬 Code graph analysis (11)
native/ecto_libsql/src/decode.rs (1)
lib/ecto_libsql/result.ex (1)
  • new (67-74)
test/turso_remote_test.exs (1)
lib/ecto_libsql/native.ex (1)
  • get_total_changes (597-599)
native/ecto_libsql/src/tests/utils_tests.rs (1)
native/ecto_libsql/src/utils.rs (2)
  • detect_query_type (261-282)
  • should_use_query (304-369)
native/ecto_libsql/src/query.rs (3)
native/ecto_libsql/src/utils.rs (8)
  • build_empty_result (67-73)
  • collect_rows (186-240)
  • enhance_constraint_error (85-181)
  • safe_lock (16-23)
  • safe_lock_arc (28-38)
  • should_use_query (304-369)
  • decode_term_to_value (374-400)
  • sync_with_timeout (43-61)
native/ecto_libsql/src/connection.rs (4)
  • t (47-47)
  • t (50-50)
  • t (51-51)
  • t (54-54)
lib/ecto_libsql/native.ex (1)
  • sync (249-251)
native/ecto_libsql/src/metadata.rs (2)
native/ecto_libsql/src/utils.rs (2)
  • safe_lock (16-23)
  • safe_lock_arc (28-38)
lib/ecto_libsql/result.ex (1)
  • new (67-74)
native/ecto_libsql/src/savepoint.rs (3)
native/ecto_libsql/src/decode.rs (1)
  • validate_savepoint_name (74-84)
native/ecto_libsql/src/utils.rs (1)
  • safe_lock (16-23)
lib/ecto_libsql/result.ex (1)
  • new (67-74)
native/ecto_libsql/src/replication.rs (1)
native/ecto_libsql/src/utils.rs (2)
  • safe_lock (16-23)
  • safe_lock_arc (28-38)
native/ecto_libsql/src/models.rs (1)
lib/ecto_libsql/native.ex (1)
  • sync (249-251)
native/ecto_libsql/src/batch.rs (1)
native/ecto_libsql/src/utils.rs (4)
  • collect_rows (186-240)
  • decode_term_to_value (374-400)
  • safe_lock (16-23)
  • safe_lock_arc (28-38)
native/ecto_libsql/src/cursor.rs (3)
native/ecto_libsql/src/transaction.rs (2)
  • transaction (96-107)
  • take (69-90)
native/ecto_libsql/src/utils.rs (3)
  • safe_lock (16-23)
  • decode_term_to_value (374-400)
  • safe_lock_arc (28-38)
native/ecto_libsql/src/decode.rs (1)
  • verify_cursor_ownership (59-66)
native/ecto_libsql/src/lib.rs (4)
lib/ecto_libsql/native.ex (2)
  • batch (681-706)
  • query (267-313)
native/ecto_libsql/src/savepoint.rs (1)
  • savepoint (32-55)
native/ecto_libsql/src/transaction.rs (1)
  • transaction (96-107)
native/ecto_libsql/src/utils.rs (2)
  • detect_query_type (261-282)
  • should_use_query (304-369)
🪛 LanguageTool
CLAUDE.md

[style] ~24-~24: Consider using a different verb for a more formal wording.
Context: ...rmatted`) before committing changes and fix any issues. - ALWAYS run the Rust Cargo...

(FIX_RESOLVE)


[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ommitting changes and fix any issues. - ALWAYS run the Rust Cargo formatter (`cargo fm...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~25-~25: Consider using a different verb for a more formal wording.
Context: ...rgo fmt`) before committing changes and fix any issues. --- ## Project Overview ...

(FIX_RESOLVE)


[grammar] ~539-~539: It appears that a hyphen is missing in this expression.
Context: ...o tests ``` ### Test Coverage Areas Must have tests for: - Happy path (successful o...

(MUST_HAVE)


[style] ~1144-~1144: Consider using a synonym to be more concise.
Context: ...ncryption, streaming), zero panic risk, lots of tests, and comprehensive documentation....

(A_LOT_OF)

⏰ 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). (3)
  • GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
  • GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
  • GitHub Check: Analyze (rust)

Comment thread CHANGELOG.md Outdated
Comment thread native/ecto_libsql/src/batch.rs Outdated
Comment thread native/ecto_libsql/src/batch.rs
Comment thread native/ecto_libsql/src/cursor.rs
Comment thread native/ecto_libsql/src/decode.rs
Comment thread native/ecto_libsql/src/metadata.rs Outdated
Comment thread native/ecto_libsql/src/metadata.rs Outdated
Comment thread native/ecto_libsql/src/metadata.rs Outdated
Comment thread native/ecto_libsql/src/query.rs
Comment thread native/ecto_libsql/src/transaction.rs
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
native/ecto_libsql/src/batch.rs (1)

106-177: Lock handling issue has been addressed.

Same fix applied here - the conn_map lock is now properly dropped on Line 121 before entering the async block, resolving the issue flagged in the previous review.

🧹 Nitpick comments (11)
native/ecto_libsql/src/cursor.rs (1)

270-331: Consider adding schedule = "DirtyIo" for consistency.

While fetch_cursor doesn't use TOKIO_RUNTIME.block_on(), it does acquire a mutex lock on CURSOR_REGISTRY which could potentially block. For consistency with other NIFs in the codebase and to avoid blocking the BEAM scheduler on lock contention, consider adding the DirtyIo scheduler annotation.

-#[rustler::nif]
+#[rustler::nif(schedule = "DirtyIo")]
 pub fn fetch_cursor<'a>(
native/ecto_libsql/src/query.rs (1)

127-152: Lock management issue from previous review has been addressed.

The conn_map lock is now explicitly dropped at line 135 before entering TOKIO_RUNTIME.block_on().

Minor: Consider using a string literal instead of format!() for the static success message to avoid unnecessary allocation:

-        Ok(()) => Ok((rustler::types::atom::ok(), format!("success sync"))),
+        Ok(()) => Ok((rustler::types::atom::ok(), "success sync".to_string())),
native/ecto_libsql/src/connection.rs (1)

199-228: Consider adding schedule = "DirtyIo" for consistency.

The close function lacks the #[rustler::nif(schedule = "DirtyIo")] attribute that other NIF functions use. While this function only performs registry operations (no network I/O), the safe_lock calls could potentially block if the mutex is contended. For consistency with other NIFs in this codebase, consider adding the scheduling attribute.

-#[rustler::nif]
+#[rustler::nif(schedule = "DirtyIo")]
 pub fn close(id: &str, opt: Atom) -> NifResult<Atom> {
native/ecto_libsql/src/statement.rs (1)

81-89: Consider simplifying the return type.

The return type Result<NifResult<Term<'a>>, rustler::Error> is redundant since NifResult is already Result<T, rustler::Error>. This could be simplified to NifResult<Term<'a>> for clarity.

 #[rustler::nif(schedule = "DirtyIo")]
 pub fn query_prepared<'a>(
     env: Env<'a>,
     conn_id: &str,
     stmt_id: &str,
     _mode: Atom,
     _syncx: Atom,
     args: Vec<Term<'a>>,
-) -> Result<NifResult<Term<'a>>, rustler::Error> {
+) -> NifResult<Term<'a>> {

And adjust the return statements accordingly (e.g., Ok(Ok(collected))Ok(collected)).

native/ecto_libsql/src/replication.rs (3)

34-45: Lock held across await point in async block.

Per coding guidelines, locks should be dropped before async blocks to prevent holding them across await points. Here client_guard is held while awaiting replication_index().

Consider restructuring to drop the lock before the await:

     let result = TOKIO_RUNTIME.block_on(async {
-        let client_guard = safe_lock_arc(&client, "get_frame_number client")
-            .map_err(|e| format!("Failed to lock client: {:?}", e))?;
-
-        let frame_no = client_guard
-            .db
-            .replication_index()
-            .await
-            .map_err(|e| format!("replication_index failed: {}", e))?;
-
-        Ok::<_, String>(frame_no.unwrap_or(0))
+        let db = {
+            let client_guard = safe_lock_arc(&client, "get_frame_number client")
+                .map_err(|e| format!("Failed to lock client: {:?}", e))?;
+            client_guard.db.clone()
+        };
+
+        let frame_no = db
+            .replication_index()
+            .await
+            .map_err(|e| format!("replication_index failed: {}", e))?;
+
+        Ok::<_, String>(frame_no.unwrap_or(0))
     });

This assumes db can be cloned; if not, the current approach may be acceptable if the lock duration is brief.


75-91: Lock held across await point with timeout.

Same concern as get_frame_number - the client_guard lock is held while awaiting sync_until. Consider cloning/extracting the db reference before the await if possible. The timeout mitigates indefinite blocking but doesn't address the lock contention.


119-136: Consistent pattern, same lock-across-await consideration.

This function follows the same pattern. If the db reference can be cloned to avoid holding the lock during the network operation, it would improve concurrency.

native/ecto_libsql/src/transaction.rs (4)

174-180: Lock held across await in transaction start.

The conn_guard is held while awaiting transaction(). Consider extracting or cloning the connection reference before the await if the connection type supports it.


207-209: Silent fallback to Deferred behaviour.

If decode_transaction_behavior returns None for an unrecognised atom, the function silently defaults to Deferred. Consider logging a warning or returning an error for invalid behaviour atoms to help debugging.


279-281: Redundant error wrapping.

guard.transaction() already returns rustler::Error, but then it's converted with format!("{:?}", e) and wrapped again:

     let trx = guard
         .transaction()
-        .map_err(|e| rustler::Error::Term(Box::new(format!("Guard error: {:?}", e))))?;
+        ?;

The original error is already a rustler::Error and can be propagated directly.


324-326: Same redundant error wrapping.

Apply the same fix as suggested for execute_with_transaction:

     let trx = guard
         .transaction()
-        .map_err(|e| rustler::Error::Term(Box::new(format!("Guard error: {:?}", e))))?;
+        ?;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 712e58d and f9efa6e.

📒 Files selected for processing (15)
  • CHANGELOG.md (1 hunks)
  • CLAUDE.md (14 hunks)
  • native/ecto_libsql/src/batch.rs (1 hunks)
  • native/ecto_libsql/src/connection.rs (1 hunks)
  • native/ecto_libsql/src/constants.rs (1 hunks)
  • native/ecto_libsql/src/cursor.rs (1 hunks)
  • native/ecto_libsql/src/decode.rs (1 hunks)
  • native/ecto_libsql/src/lib.rs (1 hunks)
  • native/ecto_libsql/src/metadata.rs (1 hunks)
  • native/ecto_libsql/src/query.rs (1 hunks)
  • native/ecto_libsql/src/replication.rs (1 hunks)
  • native/ecto_libsql/src/savepoint.rs (1 hunks)
  • native/ecto_libsql/src/statement.rs (1 hunks)
  • native/ecto_libsql/src/transaction.rs (1 hunks)
  • native/ecto_libsql/src/utils.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • native/ecto_libsql/src/decode.rs
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/constants.rs
🧰 Additional context used
📓 Path-based instructions (3)
native/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

native/**/*.rs: Never use .unwrap() in production Rust code - use safe_lock helper for Mutex, safe_lock_arc helper for Arc<Mutex>, ok_or_else for Option types, and ? operator for error propagation to prevent VM panics
Use safe_lock(&REGISTRY, "function_name context") helper function instead of .lock().unwrap() when accessing Mutex-protected registries in Rust
Use safe_lock_arc(&arc_mutex, "function_name context") helper function for locking Arc<Mutex> in Rust to prevent panics
Handle Option types using ok_or_else() instead of .unwrap() to return proper error tuples in Rust NIFs
For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points
In Rust async blocks, convert errors to String using .map_err(|e| format!("{:?}", e)) when async runtime errors are encountered
Define new Rust NIF functions with the #[rustler::nif(schedule = "DirtyIo")] attribute and implement them without manual function list in rustler::init!() - Rustler automatically detects annotated functions
Return NifResult<T> from all Rust NIF functions to enable proper error handling and prevent panics
Use the safe_lock helper in Rust to access the CONNECTION_REGISTRY without panicking on lock acquisition failure
Track transaction ownership in Rust using TransactionEntry struct containing both conn_id and transaction fields, and validate ownership in transaction operations
Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections
Implement proper error handling in all NIFs returning NifResult with descriptive error messages
In Rust code, validate query types (SELECT, INSERT, UPDATE, DELETE) to determine appropriate return format (rows vs. affected count)
Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and...

Files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/cursor.rs
*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Use Markdown files for documentation: AGENTS.md for API reference, CLAUDE.md for AI agent development guide, README.md for user documentation, CHANGELOG.md for version history, ECTO_MIGRATION_GUIDE.md for migration guidance, RUST_ERROR_HANDLING.md for error patterns

Files:

  • CLAUDE.md
  • CHANGELOG.md
CHANGELOG.md

📄 CodeRabbit inference engine (CLAUDE.md)

Update CHANGELOG.md with changes for each release following semantic versioning

Files:

  • CHANGELOG.md
🧠 Learnings (53)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and transaction state
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Define new Rust NIF functions with the `#[rustler::nif(schedule = "DirtyIo")]` attribute and implement them without manual function list in `rustler::init!()` - Rustler automatically detects annotated functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Return `NifResult<T>` from all Rust NIF functions to enable proper error handling and prevent panics
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Implement query compilation in Connection module for SELECT, INSERT, UPDATE, DELETE statements with WHERE, JOIN, ORDER BY, LIMIT support
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
  • native/ecto_libsql/src/cursor.rs
  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : In Rust code, validate query types (SELECT, INSERT, UPDATE, DELETE) to determine appropriate return format (rows vs. affected count)

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Implement query compilation in Connection module for SELECT, INSERT, UPDATE, DELETE statements with WHERE, JOIN, ORDER BY, LIMIT support

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/statement.rs
  • CLAUDE.md
  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Define new Rust NIF functions with the `#[rustler::nif(schedule = "DirtyIo")]` attribute and implement them without manual function list in `rustler::init!()` - Rustler automatically detects annotated functions

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use the safe_lock helper in Rust to access the CONNECTION_REGISTRY without panicking on lock acquisition failure

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and transaction state

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Track transaction ownership in Rust using TransactionEntry struct containing both conn_id and transaction fields, and validate ownership in transaction operations

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/savepoint.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/*.ex : Use the EctoLibSql.State struct to track connection state including conn_id, trx_id, mode (:local, :remote, :remote_replica), and sync setting

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/replication.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use `safe_lock(&REGISTRY, "function_name context")` helper function instead of `.lock().unwrap()` when accessing Mutex-protected registries in Rust

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/metadata.rs
  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use `safe_lock_arc(&arc_mutex, "function_name context")` helper function for locking Arc<Mutex<T>> in Rust to prevent panics

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/batch.rs
  • native/ecto_libsql/src/savepoint.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Never use `.unwrap()` in production Rust code - use `safe_lock` helper for Mutex<T>, `safe_lock_arc` helper for Arc<Mutex<T>>, `ok_or_else` for Option types, and `?` operator for error propagation to prevent VM panics

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/batch.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Use DBConnection protocol implementation for connection lifecycle (connect, disconnect, ping) and query execution (execute, prepare, fetch)

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Generate SQL DDL operations in Connection module for CREATE TABLE, DROP TABLE, ALTER TABLE, CREATE INDEX, DROP INDEX, RENAME TABLE with proper constraint support

Applied to files:

  • native/ecto_libsql/src/query.rs
  • native/ecto_libsql/src/lib.rs
  • CLAUDE.md
  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Implement handle_begin/2, handle_commit/2, handle_rollback/2 callbacks for transaction management following DBConnection protocol

Applied to files:

  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use transaction behaviours (:deferred, :immediate, :exclusive, :read_only) appropriately based on operation type - :immediate for write-heavy, :deferred for read-heavy

Applied to files:

  • native/ecto_libsql/src/transaction.rs
  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : In Rust async blocks, convert errors to String using `.map_err(|e| format!("{:?}", e))` when async runtime errors are encountered

Applied to files:

  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/*.ex : Detect connection mode based on provided options: :local mode for database file paths, :remote mode for uri+auth_token, :remote_replica mode for database+uri+auth_token+sync:true

Applied to files:

  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/connection.rs
  • CLAUDE.md
  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use statement introspection functions (stmt_parameter_count, stmt_column_count, stmt_column_name) to validate prepared statement structure

Applied to files:

  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/statement.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/tests.rs : Write Rust tests using #[tokio::test] for async operations with setup_test_db() and cleanup_test_db() helpers, tests may use .unwrap() for simplicity

Applied to files:

  • native/ecto_libsql/src/lib.rs
  • native/ecto_libsql/src/batch.rs
  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Return `NifResult<T>` from all Rust NIF functions to enable proper error handling and prevent panics

Applied to files:

  • native/ecto_libsql/src/lib.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use batch operations (batch_transactional or execute_batch_sql) for bulk inserts instead of individual insert statements

Applied to files:

  • native/ecto_libsql/src/batch.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use savepoints with create_savepoint and rollback_to_savepoint_by_name for partial rollback within transactions

Applied to files:

  • native/ecto_libsql/src/savepoint.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Set busy_timeout with EctoLibSql.Native.busy_timeout for write-heavy workloads to handle database locks gracefully

Applied to files:

  • native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use sync/1 function to manually trigger replica synchronization with Turso cloud in embedded replica mode

Applied to files:

  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use max_write_replication_index to ensure read-your-writes consistency in remote replica mode

Applied to files:

  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use prepared statements for queries that will be executed multiple times, as statements are automatically cached after preparation

Applied to files:

  • native/ecto_libsql/src/statement.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Document SQLite limitations in code comments, especially regarding ALTER TABLE, DROP COLUMN, and column type changes which require table recreation

Applied to files:

  • native/ecto_libsql/src/metadata.rs
  • CLAUDE.md
  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to *.md : Use Markdown files for documentation: AGENTS.md for API reference, CLAUDE.md for AI agent development guide, README.md for user documentation, CHANGELOG.md for version history, ECTO_MIGRATION_GUIDE.md for migration guidance, RUST_ERROR_HANDLING.md for error patterns

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-05T22:12:22.789Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENT.md:0-0
Timestamp: 2025-12-05T22:12:22.789Z
Learning: Applies to AGENTS.md : Document agent architecture and design patterns in AGENTS.md

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to lib/**/*repo.ex : Configure Ecto repository in lib/ directory with adapter: Ecto.Adapters.LibSql

Applied to files:

  • CLAUDE.md
  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to test/**/*.exs : Organize test files by functionality: ecto_adapter_test.exs for storage operations, ecto_connection_test.exs for SQL generation, ecto_integration_test.exs for full workflows, ecto_migration_test.exs for migrations, error_handling_test.exs for error cases

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to test/**/*.exs : Write Elixir tests in test/ directory following the pattern: setup database, create test data, run test, verify results, cleanup in on_exit callback

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to test/**/*.exs : Test database locking scenarios with connection pool exhaustion and timeout configurations to ensure proper error handling

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement Ecto type loaders and dumpers in `lib/ecto/adapters/libsql.ex` for custom type conversions between Ecto and SQLite types

Applied to files:

  • CLAUDE.md
  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement Ecto adapter callbacks: storage_up/1, storage_down/1, storage_status/1 for database lifecycle management

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/tests.rs : Test code in Rust `#[cfg(test)]` modules may use `.unwrap()` for simplicity

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Run both `mix test` for Elixir tests and `cargo test` for Rust tests before committing - pre-commit checklist should verify all tests pass

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Always format Elixir code using `mix format` before committing - unformatted code will fail CI

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement proper error handling in all NIFs returning NifResult with descriptive error messages

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use proper error matching in Elixir to handle EctoLibSql.Error exception type from failed database operations

Applied to files:

  • CLAUDE.md
  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Handle NIF errors gracefully by pattern matching on {:ok, result} and {:error, reason} returns from EctoLibSql.Native functions

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Map Ecto :boolean type to INTEGER (0=false, 1=true) in SQLite with corresponding loaders and dumpers

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Document unsupported features by returning clear :unsupported atom errors, include explanation in docstring, and add tests asserting unsupported behavior

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Include examples in docstrings for public Elixir functions using `## Examples` section with doctests

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use Ecto.Query for building type-safe queries instead of raw SQL strings when possible

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use `fragment/1` in Ecto queries for SQLite-specific functions like `RANDOM()`, `ROWID`, or custom SQL expressions

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-05T22:12:22.789Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENT.md:0-0
Timestamp: 2025-12-05T22:12:22.789Z
Learning: Maintain agent implementation documentation standards

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: For Turso remote connections, include `libsql://` prefix in URI configuration

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Implement handle_declare/4, handle_fetch/4, handle_deallocate/4 callbacks for cursor streaming operations

Applied to files:

  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: For large result sets, use cursor streaming with DBConnection.stream() instead of loading all rows into memory

Applied to files:

  • native/ecto_libsql/src/cursor.rs
🧬 Code graph analysis (4)
native/ecto_libsql/src/query.rs (1)
native/ecto_libsql/src/utils.rs (8)
  • build_empty_result (67-73)
  • collect_rows (192-249)
  • enhance_constraint_error (85-187)
  • safe_lock (16-23)
  • safe_lock_arc (28-38)
  • should_use_query (313-378)
  • decode_term_to_value (383-409)
  • sync_with_timeout (43-61)
native/ecto_libsql/src/lib.rs (3)
native/ecto_libsql/src/savepoint.rs (1)
  • savepoint (32-50)
native/ecto_libsql/src/transaction.rs (1)
  • transaction (96-107)
native/ecto_libsql/src/utils.rs (2)
  • detect_query_type (270-291)
  • should_use_query (313-378)
native/ecto_libsql/src/connection.rs (5)
native/ecto_libsql/src/utils.rs (2)
  • safe_lock_arc (28-38)
  • safe_lock (16-23)
lib/ecto_libsql/native.ex (1)
  • sync (249-251)
lib/ecto_libsql/result.ex (1)
  • new (67-74)
lib/ecto/adapters/libsql/connection.ex (1)
  • from (613-616)
native/ecto_libsql/src/transaction.rs (1)
  • drop (137-146)
native/ecto_libsql/src/replication.rs (1)
native/ecto_libsql/src/utils.rs (2)
  • safe_lock (16-23)
  • safe_lock_arc (28-38)
🪛 LanguageTool
CLAUDE.md

[style] ~24-~24: Consider using a different verb for a more formal wording.
Context: ...rmatted`) before committing changes and fix any issues. - ALWAYS run the Rust Cargo...

(FIX_RESOLVE)


[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ommitting changes and fix any issues. - ALWAYS run the Rust Cargo formatter (`cargo fm...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~25-~25: Consider using a different verb for a more formal wording.
Context: ...rgo fmt`) before committing changes and fix any issues. --- ## Project Overview ...

(FIX_RESOLVE)


[grammar] ~539-~539: It appears that a hyphen is missing in this expression.
Context: ...o tests ``` ### Test Coverage Areas Must have tests for: - Happy path (successful o...

(MUST_HAVE)

⏰ 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). (3)
  • GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
  • GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
  • GitHub Check: Analyze (rust)
🔇 Additional comments (31)
CHANGELOG.md (1)

8-37: LGTM! The Unreleased section is well-documented.

The module count has been corrected to 13, matching the enumerated list. The changelog follows the Keep a Changelog format and provides comprehensive documentation of the refactoring, including clear categorisation of changes, fixes, and their impact.

native/ecto_libsql/src/lib.rs (1)

1-29: Clean modular structure with appropriate re-exports.

The root module correctly declares all 13 submodules, provides sensible re-exports for internal use, and uses rustler::init! with automatic NIF discovery. The test module is properly gated behind #[cfg(test)]. This aligns well with the project's modularisation goals.

native/ecto_libsql/src/cursor.rs (2)

32-101: Lock management correctly addresses previous feedback.

The conn_map lock is now explicitly dropped at line 41 before entering TOKIO_RUNTIME.block_on(), preventing locks from being held across await points. This follows the coding guidelines for async NIF operations.


116-255: Well-structured context-aware cursor declaration with proper ownership validation.

The function correctly:

  • Uses TransactionEntryGuard for transaction ownership verification
  • Scopes the connection registry lock properly (lines 186-193)
  • Clones the inner Arc and drops the outer lock before async operations (lines 196-199)
  • Validates connection ID matching for connection-type cursors
native/ecto_libsql/src/metadata.rs (3)

25-44: DirtyIo annotations now correctly applied.

All metadata NIFs (last_insert_rowid, changes, total_changes, is_autocommit) now have the schedule = "DirtyIo" attribute, addressing the previous review feedback. Lock management is correct with the registry lock being dropped before TOKIO_RUNTIME.block_on().


61-116: Consistent implementation pattern across metadata functions.

The changes and total_changes functions follow the same safe pattern: acquire lock, clone client, drop lock, then perform async operation. Error handling returns proper error terms for invalid connection IDs.


132-151: LGTM!

The is_autocommit function correctly implements the same pattern with proper DirtyIo scheduling and lock management.

native/ecto_libsql/src/query.rs (2)

32-110: Well-structured query execution with proper routing and error enhancement.

The query_args function correctly:

  • Drops the registry lock before async operations (lines 41-47, 61-64)
  • Uses should_use_query to route between query() and execute() based on statement type
  • Enhances constraint errors with index names using a safe unwrap_or fallback
  • Documents the LibSQL automatic sync behaviour clearly

175-199: Proper PRAGMA query implementation with correct lock scoping.

The function correctly manages locks, dropping the registry lock before async operations (line 181). Error handling provides clear context for PRAGMA failures.

native/ecto_libsql/src/savepoint.rs (3)

1-50: Well-structured savepoint implementation with proper ownership validation.

The savepoint function correctly:

  • Validates the savepoint name before use (Line 33)
  • Uses TransactionEntryGuard::take for ownership verification with RAII cleanup (Line 36)
  • Properly schedules as DirtyIo for blocking I/O operations
  • Returns descriptive errors on failure

The guard pattern ensures the transaction is re-inserted into the registry even if the async block fails, which is good for reliability.


52-84: LGTM!

The release_savepoint function follows the same safe patterns as savepoint with proper validation, ownership verification, and error handling.


86-120: LGTM!

The rollback_to_savepoint function completes the savepoint trio with consistent error handling and ownership validation. As per coding guidelines, all functions properly implement transaction ownership verification with conn_id parameter validation.

native/ecto_libsql/src/connection.rs (5)

32-152: Solid connection establishment with proper timeout handling.

The connect function correctly:

  • Decodes keyword list options with proper error handling
  • Uses a 30-second timeout to prevent hanging connections
  • Supports all three connection modes (local, remote, remote_replica)
  • Performs a health check ping for remote connections
  • Uses safe_lock for registry access

The nested async timeout structure naturally scopes the connection work appropriately.


154-188: LGTM!

The ping function correctly drops the registry lock before entering the async block (Line 165), following the coding guidelines for preventing locks held across await points.


241-266: LGTM!

The set_busy_timeout function properly releases the registry lock before the blocking operation (Line 247), following the coding guidelines.


277-298: LGTM!

The reset_connection function correctly drops the registry lock before the async block (Line 283). The reset() call is properly awaited.


309-329: LGTM!

The interrupt_connection function correctly drops the registry lock before acquiring client-specific locks. Since interrupt() is a synchronous operation (not async), holding the client locks during the call is acceptable.

native/ecto_libsql/src/statement.rs (3)

29-67: Well-implemented prepared statement caching.

The prepare_statement function correctly:

  • Releases the outer lock before async operations (Lines 43-45)
  • Stores both connection ID and statement in registry for ownership tracking
  • Uses safe_lock and safe_lock_arc helpers throughout

154-209: LGTM!

The execute_prepared function properly releases locks before async operations (Lines 187-188) and includes helpful documentation about LibSQL's automatic sync behaviour. As per coding guidelines, ownership verification is correctly implemented.


211-326: LGTM!

The statement introspection functions (statement_column_count, statement_column_name, statement_parameter_count) consistently:

  • Verify ownership before access
  • Drop registry locks before accessing the statement
  • Include proper bounds checking for column index access (Lines 279-285)
native/ecto_libsql/src/batch.rs (3)

28-87: Lock handling issue has been addressed.

The previous review flagged that conn_map was held across the async block. This has been correctly fixed with the addition of drop(conn_map); on Line 43 before entering TOKIO_RUNTIME.block_on.


194-234: LGTM!

The execute_batch_native function correctly releases the registry lock (Line 200) before entering the async block, following the coding guidelines.


252-303: LGTM!

The execute_transactional_batch_native function follows the same correct pattern, dropping the lock on Line 265 before async execution.

CLAUDE.md (3)

98-117: Excellent documentation update for the modular Rust structure.

The file structure documentation now accurately reflects the refactored module organisation, clearly showing the responsibility of each module (connection, query, batch, statement, transaction, savepoint, cursor, replication, metadata, utils, constants, models, decode) and the test structure.


350-408: Clear, actionable guidance for adding new NIFs.

The updated workflow correctly emphasises that Rustler automatically discovers NIFs via the #[rustler::nif] attribute, and provides a clear module selection guide. This aligns with the coding guidelines from retrieved learnings.


1138-1155: LGTM!

The summary and contributing guidelines accurately capture the key principles: safety-first Rust error handling, comprehensive testing, and proper documentation. The updated timestamp reflects this PR's changes.

native/ecto_libsql/src/replication.rs (2)

157-173: LGTM!

Correctly implemented as a synchronous call. Uses safe_lock and safe_lock_arc helpers properly, and unwrap_or(0) for the Option type as per coding guidelines.


186-198: LGTM!

Good defensive validation of connection existence before returning the unsupported error. Clear documentation explains the architectural constraints preventing implementation.

native/ecto_libsql/src/transaction.rs (3)

54-147: Well-designed RAII guard pattern.

The TransactionEntryGuard correctly implements:

  • Ownership verification on take
  • Defensive re-insertion on ownership mismatch
  • Consumed state tracking to prevent double-use
  • Best-effort re-insertion in Drop (graceful lock failure handling)

This aligns with the coding guidelines for transaction ownership tracking using TransactionEntry with conn_id validation.


385-394: LGTM!

Simple and correct implementation for checking transaction status.


445-451: LGTM - previous issues addressed.

The double space typo and redundant .to_string() from the previous review have been fixed. The commit/rollback pattern correctly uses consume() to prevent re-insertion.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
native/ecto_libsql/src/cursor.rs (1)

313-322: Binary allocation failure returns error atom silently within row data.

When OwnedBinary::new(b.len()) fails (returns None), the code returns an :error atom as the cell value. This could lead to subtle data corruption if the caller doesn't specifically check for error atoms within row values.

Consider returning an error from the entire function instead, or at minimum document this behaviour:

                     Value::Blob(b) => match OwnedBinary::new(b.len()) {
                         Some(mut owned) => {
                             owned.as_mut_slice().copy_from_slice(b);
                             Binary::from_owned(owned, env).encode(env)
                         }
                         None => {
-                            // Binary allocation failed - return error atom
-                            crate::constants::error().encode(env)
+                            // Binary allocation failed - this is a serious error
+                            return Err(rustler::Error::Term(Box::new(format!(
+                                "Failed to allocate binary of size {} bytes",
+                                b.len()
+                            ))));
                         }
                     },
native/ecto_libsql/src/transaction.rs (1)

218-225: Consider alternative to eprintln for logging in NIF context.

Using eprintln! in a NIF may not surface warnings effectively to the Elixir application. Consider using Rustler's logging integration or returning the warning information to Elixir where it can be logged properly.

     let trx_behavior = match decode::decode_transaction_behavior(behavior) {
         Some(b) => b,
         None => {
-            // Log warning for unrecognized behavior atom
-            eprintln!(
-                "WARNING: Unrecognized transaction behavior atom: {:?}. Defaulting to Deferred.",
-                behavior
-            );
+            // Unrecognized behavior - default to Deferred
+            // Consider: return Err to force explicit handling on Elixir side
             TransactionBehavior::Deferred
         }
     };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 95fafa7 and 4c6b8c9.

📒 Files selected for processing (6)
  • native/ecto_libsql/src/connection.rs (1 hunks)
  • native/ecto_libsql/src/cursor.rs (1 hunks)
  • native/ecto_libsql/src/query.rs (1 hunks)
  • native/ecto_libsql/src/replication.rs (1 hunks)
  • native/ecto_libsql/src/statement.rs (1 hunks)
  • native/ecto_libsql/src/transaction.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • native/ecto_libsql/src/query.rs
🧰 Additional context used
📓 Path-based instructions (1)
native/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

native/**/*.rs: Never use .unwrap() in production Rust code - use safe_lock helper for Mutex, safe_lock_arc helper for Arc<Mutex>, ok_or_else for Option types, and ? operator for error propagation to prevent VM panics
Use safe_lock(&REGISTRY, "function_name context") helper function instead of .lock().unwrap() when accessing Mutex-protected registries in Rust
Use safe_lock_arc(&arc_mutex, "function_name context") helper function for locking Arc<Mutex> in Rust to prevent panics
Handle Option types using ok_or_else() instead of .unwrap() to return proper error tuples in Rust NIFs
For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points
In Rust async blocks, convert errors to String using .map_err(|e| format!("{:?}", e)) when async runtime errors are encountered
Define new Rust NIF functions with the #[rustler::nif(schedule = "DirtyIo")] attribute and implement them without manual function list in rustler::init!() - Rustler automatically detects annotated functions
Return NifResult<T> from all Rust NIF functions to enable proper error handling and prevent panics
Use the safe_lock helper in Rust to access the CONNECTION_REGISTRY without panicking on lock acquisition failure
Track transaction ownership in Rust using TransactionEntry struct containing both conn_id and transaction fields, and validate ownership in transaction operations
Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections
Implement proper error handling in all NIFs returning NifResult with descriptive error messages
In Rust code, validate query types (SELECT, INSERT, UPDATE, DELETE) to determine appropriate return format (rows vs. affected count)
Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and...

Files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/replication.rs
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and transaction state
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Define new Rust NIF functions with the `#[rustler::nif(schedule = "DirtyIo")]` attribute and implement them without manual function list in `rustler::init!()` - Rustler automatically detects annotated functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement proper error handling in all NIFs returning NifResult with descriptive error messages
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use the safe_lock helper in Rust to access the CONNECTION_REGISTRY without panicking on lock acquisition failure

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and transaction state

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Track transaction ownership in Rust using TransactionEntry struct containing both conn_id and transaction fields, and validate ownership in transaction operations

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/*.ex : Use the EctoLibSql.State struct to track connection state including conn_id, trx_id, mode (:local, :remote, :remote_replica), and sync setting

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Use DBConnection protocol implementation for connection lifecycle (connect, disconnect, ping) and query execution (execute, prepare, fetch)

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/statement.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Define new Rust NIF functions with the `#[rustler::nif(schedule = "DirtyIo")]` attribute and implement them without manual function list in `rustler::init!()` - Rustler automatically detects annotated functions

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Generate SQL DDL operations in Connection module for CREATE TABLE, DROP TABLE, ALTER TABLE, CREATE INDEX, DROP INDEX, RENAME TABLE with proper constraint support

Applied to files:

  • native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Set busy_timeout with EctoLibSql.Native.busy_timeout for write-heavy workloads to handle database locks gracefully

Applied to files:

  • native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/*.ex : Detect connection mode based on provided options: :local mode for database file paths, :remote mode for uri+auth_token, :remote_replica mode for database+uri+auth_token+sync:true

Applied to files:

  • native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Implement handle_declare/4, handle_fetch/4, handle_deallocate/4 callbacks for cursor streaming operations

Applied to files:

  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : In Rust code, validate query types (SELECT, INSERT, UPDATE, DELETE) to determine appropriate return format (rows vs. affected count)

Applied to files:

  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/statement.rs
  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: For large result sets, use cursor streaming with DBConnection.stream() instead of loading all rows into memory

Applied to files:

  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Implement query compilation in Connection module for SELECT, INSERT, UPDATE, DELETE statements with WHERE, JOIN, ORDER BY, LIMIT support

Applied to files:

  • native/ecto_libsql/src/statement.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use statement introspection functions (stmt_parameter_count, stmt_column_count, stmt_column_name) to validate prepared statement structure

Applied to files:

  • native/ecto_libsql/src/statement.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use prepared statements for queries that will be executed multiple times, as statements are automatically cached after preparation

Applied to files:

  • native/ecto_libsql/src/statement.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Implement handle_begin/2, handle_commit/2, handle_rollback/2 callbacks for transaction management following DBConnection protocol

Applied to files:

  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use transaction behaviours (:deferred, :immediate, :exclusive, :read_only) appropriately based on operation type - :immediate for write-heavy, :deferred for read-heavy

Applied to files:

  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use `safe_lock(&REGISTRY, "function_name context")` helper function instead of `.lock().unwrap()` when accessing Mutex-protected registries in Rust

Applied to files:

  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : In Rust async blocks, convert errors to String using `.map_err(|e| format!("{:?}", e))` when async runtime errors are encountered

Applied to files:

  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use sync/1 function to manually trigger replica synchronization with Turso cloud in embedded replica mode

Applied to files:

  • native/ecto_libsql/src/replication.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use max_write_replication_index to ensure read-your-writes consistency in remote replica mode

Applied to files:

  • native/ecto_libsql/src/replication.rs
🧬 Code graph analysis (2)
native/ecto_libsql/src/connection.rs (2)
native/ecto_libsql/src/utils.rs (2)
  • safe_lock_arc (28-38)
  • safe_lock (16-23)
native/ecto_libsql/src/transaction.rs (1)
  • drop (142-151)
native/ecto_libsql/src/replication.rs (1)
native/ecto_libsql/src/utils.rs (2)
  • safe_lock (16-23)
  • safe_lock_arc (28-38)
⏰ 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: Analyze (rust)
🔇 Additional comments (16)
native/ecto_libsql/src/connection.rs (4)

1-152: Well-structured connection lifecycle management.

The connect function correctly implements:

  • Proper lock handling with safe_lock
  • Lock release before async operations (line 133-140 uses safe_lock within the async block but this is the final registry insert)
  • Timeout wrapping with tokio::time::timeout
  • Comprehensive error handling with descriptive messages

The nested async pattern with timeout is appropriate for connection establishment. As per coding guidelines, all mutex access uses the safe_lock helper.


154-188: LGTM - ping follows correct async locking pattern.

The connection map lock is properly dropped on line 165 before entering the async block, preventing potential deadlock scenarios. The inner locks within the async block are appropriately scoped.


199-228: LGTM - resource cleanup handles all registry types correctly.

The close function properly uses safe_lock for each registry type and provides clear error messages for missing resources.


230-329: Consistent pattern across utility functions.

All three functions (set_busy_timeout, reset_connection, interrupt_connection) correctly:

  • Acquire and drop locks before async operations
  • Clone the client reference to avoid holding locks across await points
  • Use safe_lock_arc for nested mutex access

The implementation aligns well with the coding guidelines.

native/ecto_libsql/src/cursor.rs (2)

32-101: Correctly addresses previous review feedback.

The lock is now properly dropped on line 41 before entering the async block, following the pattern: "For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points." This resolves the issue flagged in past review comments.


116-255: Well-designed context-aware cursor declaration.

The function properly handles both transaction and connection contexts:

  • Transaction path uses TransactionEntryGuard for RAII safety (line 132)
  • Connection path validates ownership (lines 179-183) before proceeding
  • Lock scopes are correctly limited using block expressions (lines 186-199)

This follows the coding guidelines for transaction ownership validation.

native/ecto_libsql/src/replication.rs (4)

31-58: LGTM - clean async pattern for replication index retrieval.

Lock is properly dropped (line 38) before the async block. The function correctly handles the Option<u64> return from replication_index() by providing a sensible default of 0.


73-105: Good timeout handling in sync_until.

The function correctly:

  • Drops the connection map lock before async operations (line 80)
  • Wraps the sync operation with a timeout (lines 87-95)
  • Provides clear error messages distinguishing timeout from sync failure

166-182: Correctly implemented as synchronous operation.

Since max_write_replication_index() is a synchronous method on the Database type, this function appropriately avoids the async block pattern. The lock handling is correct for synchronous access.


195-207: Clear indication of unsupported functionality.

The function validates the connection exists before returning the :unsupported error. The documentation clearly explains why this feature is not implemented (architectural constraints around Arc ownership).

native/ecto_libsql/src/statement.rs (3)

29-67: Well-structured prepared statement caching.

The function correctly:

  • Scopes locks with block expressions (lines 31-37 and 42-45)
  • Stores the connection ID alongside the statement for ownership verification
  • Uses Arc<Mutex<Statement>> for thread-safe statement access

81-137: Correct prepared statement reuse pattern.

The reset() call on line 120 before executing the query is the correct pattern for reusing prepared statements - it clears previous bindings. Both locks are properly dropped (lines 112-113) before the async block.


255-290: Good bounds checking for column name access.

The function properly validates the column index (lines 279-285) before accessing the columns array, returning a descriptive error message that includes both the requested index and the actual column count.

native/ecto_libsql/src/transaction.rs (3)

59-152: Excellent RAII pattern for transaction safety.

The TransactionEntryGuard implementation is well-designed:

  • take() removes entry and verifies ownership, re-inserting on ownership failure (lines 82-88)
  • transaction() and consume() both check the consumed flag to prevent misuse
  • Drop implementation provides best-effort re-insertion with graceful handling of lock failures

This pattern ensures transaction entries are never lost due to panics or early returns. Based on learnings, this aligns with the requirement to track transaction ownership using TransactionEntry struct.


318-394: Well-structured transaction query execution with error enhancement.

The function correctly:

  • Uses TransactionEntryGuard for RAII safety
  • Distinguishes between query and execute paths using should_use_query()
  • Enhances constraint errors with additional context from the connection (lines 365-368, 382-385)

The guard automatically re-inserts the transaction entry on drop (line 391 comment), ensuring transaction state is preserved even on error paths.


424-466: Transaction finalization correctly consumes the guard.

The function properly:

  • Takes ownership with TransactionEntryGuard::take() (line 433)
  • Consumes the entry via consume() (line 436) to prevent re-insertion after commit/rollback
  • Handles both commit and rollback paths with appropriate error messages

Previous review feedback regarding double space and redundant .to_string() has been addressed in this version.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
test/statement_features_test.exs (1)

1-241: Critical: Fix formatting to pass CI pipeline.

The CI pipeline is failing because the file is not formatted according to mix format standards. Run mix format test/statement_features_test.exs to resolve this issue.

#!/bin/bash
# Verify the formatting issue and show what would change
mix format --check-formatted test/statement_features_test.exs || echo "Formatting required"
🧹 Nitpick comments (2)
test/statement_features_test.exs (1)

45-56: Correct implementation of granular column introspection API.

The test properly uses the new stmt_column_count/2 and stmt_column_name/3 NIFs to validate column metadata. The approach aligns with the statement introspection learnings.

Consider extracting a helper function to reduce duplication when retrieving multiple column names:

defp get_column_names(state, stmt_id, count) do
  for i <- 0..(count - 1) do
    {:ok, name} = EctoLibSql.Native.stmt_column_name(state, stmt_id, i)
    name
  end
end

Then use it as:

{:ok, count} = EctoLibSql.Native.stmt_column_count(state, stmt_id)
assert count == 3

names = get_column_names(state, stmt_id, count)
assert names == ["id", "name", "age"]

Additionally, consider testing error cases (invalid column indices):

# Negative index should return error
assert {:error, _} = EctoLibSql.Native.stmt_column_name(state, stmt_id, -1)

# Index >= count should return error  
assert {:error, _} = EctoLibSql.Native.stmt_column_name(state, stmt_id, 3)
native/ecto_libsql/src/connection.rs (1)

61-101: Consider using the decode_mode helper for consistency.

The function performs string-based mode matching (mode_str == "remote_replica", etc.) rather than using the decode_mode() function from the decode module. Whilst the current implementation works correctly, using the existing decoder would:

  • Centralise mode validation logic
  • Reduce code duplication
  • Ensure consistency with other modules that use decode_mode()

Apply this pattern:

-let db = match mode.atom_to_string() {
-    Ok(mode_str) => {
-        if mode_str == "remote_replica" {
+let mode_enum = decode::decode_mode(mode)
+    .ok_or_else(|| rustler::Error::Term(Box::new("Unknown mode")))?;
+
+let db = match mode_enum {
+    Mode::RemoteReplica => {
+        if mode_str == "remote_replica" {
             // ... remote_replica logic
-        } else if mode_str == "remote" {
+    }
+    Mode::Remote => {
             // ... remote logic
-        } else if mode_str == "local" {
+    }
+    Mode::Local => {
             // ... local logic
-        } else {
-            return Err(rustler::Error::Term(Box::new("Unknown mode".to_string())));
-        }
-    }
-    Err(other) => {
-        return Err(rustler::Error::Term(Box::new(format!("Unknown mode: {:?}", other))))
-    }
-}
+}
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c6b8c9 and a733560.

📒 Files selected for processing (8)
  • CHANGELOG.md (1 hunks)
  • native/ecto_libsql/src/connection.rs (1 hunks)
  • native/ecto_libsql/src/constants.rs (1 hunks)
  • native/ecto_libsql/src/cursor.rs (1 hunks)
  • native/ecto_libsql/src/decode.rs (1 hunks)
  • native/ecto_libsql/src/transaction.rs (1 hunks)
  • native/ecto_libsql/src/utils.rs (1 hunks)
  • test/statement_features_test.exs (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • native/ecto_libsql/src/constants.rs
🧰 Additional context used
📓 Path-based instructions (4)
native/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

native/**/*.rs: Never use .unwrap() in production Rust code - use safe_lock helper for Mutex, safe_lock_arc helper for Arc<Mutex>, ok_or_else for Option types, and ? operator for error propagation to prevent VM panics
Use safe_lock(&REGISTRY, "function_name context") helper function instead of .lock().unwrap() when accessing Mutex-protected registries in Rust
Use safe_lock_arc(&arc_mutex, "function_name context") helper function for locking Arc<Mutex> in Rust to prevent panics
Handle Option types using ok_or_else() instead of .unwrap() to return proper error tuples in Rust NIFs
For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points
In Rust async blocks, convert errors to String using .map_err(|e| format!("{:?}", e)) when async runtime errors are encountered
Define new Rust NIF functions with the #[rustler::nif(schedule = "DirtyIo")] attribute and implement them without manual function list in rustler::init!() - Rustler automatically detects annotated functions
Return NifResult<T> from all Rust NIF functions to enable proper error handling and prevent panics
Use the safe_lock helper in Rust to access the CONNECTION_REGISTRY without panicking on lock acquisition failure
Track transaction ownership in Rust using TransactionEntry struct containing both conn_id and transaction fields, and validate ownership in transaction operations
Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections
Implement proper error handling in all NIFs returning NifResult with descriptive error messages
In Rust code, validate query types (SELECT, INSERT, UPDATE, DELETE) to determine appropriate return format (rows vs. affected count)
Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and...

Files:

  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/decode.rs
*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Use Markdown files for documentation: AGENTS.md for API reference, CLAUDE.md for AI agent development guide, README.md for user documentation, CHANGELOG.md for version history, ECTO_MIGRATION_GUIDE.md for migration guidance, RUST_ERROR_HANDLING.md for error patterns

Files:

  • CHANGELOG.md
CHANGELOG.md

📄 CodeRabbit inference engine (CLAUDE.md)

Update CHANGELOG.md with changes for each release following semantic versioning

Files:

  • CHANGELOG.md
test/**/*.exs

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.exs: Write Elixir tests in test/ directory following the pattern: setup database, create test data, run test, verify results, cleanup in on_exit callback
Organize test files by functionality: ecto_adapter_test.exs for storage operations, ecto_connection_test.exs for SQL generation, ecto_integration_test.exs for full workflows, ecto_migration_test.exs for migrations, error_handling_test.exs for error cases
Use ExUnit.Case for Elixir unit and integration tests with setup callbacks for test isolation and on_exit for cleanup
Test database locking scenarios with connection pool exhaustion and timeout configurations to ensure proper error handling

Files:

  • test/statement_features_test.exs
🧠 Learnings (28)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and transaction state
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Define new Rust NIF functions with the `#[rustler::nif(schedule = "DirtyIo")]` attribute and implement them without manual function list in `rustler::init!()` - Rustler automatically detects annotated functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement proper error handling in all NIFs returning NifResult with descriptive error messages
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Implement handle_declare/4, handle_fetch/4, handle_deallocate/4 callbacks for cursor streaming operations

Applied to files:

  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : In Rust code, validate query types (SELECT, INSERT, UPDATE, DELETE) to determine appropriate return format (rows vs. affected count)

Applied to files:

  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/decode.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State

Applied to files:

  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/decode.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and transaction state

Applied to files:

  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Track transaction ownership in Rust using TransactionEntry struct containing both conn_id and transaction fields, and validate ownership in transaction operations

Applied to files:

  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/decode.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Implement transaction operations (savepoint, release_savepoint, rollback_to_savepoint) with conn_id parameter validation to ensure transactions are not accessed from wrong connections

Applied to files:

  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/decode.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points

Applied to files:

  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use the safe_lock helper in Rust to access the CONNECTION_REGISTRY without panicking on lock acquisition failure

Applied to files:

  • native/ecto_libsql/src/cursor.rs
  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/decode.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: For large result sets, use cursor streaming with DBConnection.stream() instead of loading all rows into memory

Applied to files:

  • native/ecto_libsql/src/cursor.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/*.ex : Use the EctoLibSql.State struct to track connection state including conn_id, trx_id, mode (:local, :remote, :remote_replica), and sync setting

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/transaction.rs
  • native/ecto_libsql/src/decode.rs
  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Use DBConnection protocol implementation for connection lifecycle (connect, disconnect, ping) and query execution (execute, prepare, fetch)

Applied to files:

  • native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Define new Rust NIF functions with the `#[rustler::nif(schedule = "DirtyIo")]` attribute and implement them without manual function list in `rustler::init!()` - Rustler automatically detects annotated functions

Applied to files:

  • native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Generate SQL DDL operations in Connection module for CREATE TABLE, DROP TABLE, ALTER TABLE, CREATE INDEX, DROP INDEX, RENAME TABLE with proper constraint support

Applied to files:

  • native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Set busy_timeout with EctoLibSql.Native.busy_timeout for write-heavy workloads to handle database locks gracefully

Applied to files:

  • native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/*.ex : Detect connection mode based on provided options: :local mode for database file paths, :remote mode for uri+auth_token, :remote_replica mode for database+uri+auth_token+sync:true

Applied to files:

  • native/ecto_libsql/src/connection.rs
  • native/ecto_libsql/src/decode.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Document SQLite limitations in code comments, especially regarding ALTER TABLE, DROP COLUMN, and column type changes which require table recreation

Applied to files:

  • CHANGELOG.md
  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to CHANGELOG.md : Update CHANGELOG.md with changes for each release following semantic versioning

Applied to files:

  • CHANGELOG.md
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use `safe_lock_arc(&arc_mutex, "function_name context")` helper function for locking Arc<Mutex<T>> in Rust to prevent panics

Applied to files:

  • native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use `safe_lock(&REGISTRY, "function_name context")` helper function instead of `.lock().unwrap()` when accessing Mutex-protected registries in Rust

Applied to files:

  • native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Never use `.unwrap()` in production Rust code - use `safe_lock` helper for Mutex<T>, `safe_lock_arc` helper for Arc<Mutex<T>>, `ok_or_else` for Option types, and `?` operator for error propagation to prevent VM panics

Applied to files:

  • native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/tests.rs : Write Rust tests using #[tokio::test] for async operations with setup_test_db() and cleanup_test_db() helpers, tests may use .unwrap() for simplicity

Applied to files:

  • native/ecto_libsql/src/utils.rs
  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Implement handle_begin/2, handle_commit/2, handle_rollback/2 callbacks for transaction management following DBConnection protocol

Applied to files:

  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Use transaction behaviours (:deferred, :immediate, :exclusive, :read_only) appropriately based on operation type - :immediate for write-heavy, :deferred for read-heavy

Applied to files:

  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : In Rust async blocks, convert errors to String using `.map_err(|e| format!("{:?}", e))` when async runtime errors are encountered

Applied to files:

  • native/ecto_libsql/src/transaction.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Handle Option types using `ok_or_else()` instead of `.unwrap()` to return proper error tuples in Rust NIFs

Applied to files:

  • native/ecto_libsql/src/decode.rs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Implement query compilation in Connection module for SELECT, INSERT, UPDATE, DELETE statements with WHERE, JOIN, ORDER BY, LIMIT support

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use statement introspection functions (stmt_parameter_count, stmt_column_count, stmt_column_name) to validate prepared statement structure

Applied to files:

  • test/statement_features_test.exs
🧬 Code graph analysis (2)
native/ecto_libsql/src/cursor.rs (3)
native/ecto_libsql/src/transaction.rs (3)
  • transaction (100-111)
  • drop (141-150)
  • take (73-94)
native/ecto_libsql/src/utils.rs (3)
  • safe_lock (16-23)
  • decode_term_to_value (397-423)
  • safe_lock_arc (28-38)
native/ecto_libsql/src/decode.rs (1)
  • verify_cursor_ownership (59-66)
test/statement_features_test.exs (1)
lib/ecto_libsql/native.ex (1)
  • prepare (472-480)
🪛 GitHub Actions: CI
test/statement_features_test.exs

[error] 39-41: mix format failed due to --check-formatted. The following files are not formatted: /home/runner/work/ecto_libsql/ecto_libsql/test/statement_features_test.exs

⏰ 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: Analyze (rust)
🔇 Additional comments (13)
CHANGELOG.md (1)

8-88: Changelog is well-structured and accurately reflects all major PR changes.

The module count issue flagged in the previous review has been resolved—line 13 correctly states "13 focused, single-responsibility modules," and all 13 modules are properly enumerated in lines 15–27. The [Unreleased] section comprehensively documents:

  • The Rust refactoring with clear module organisation
  • Thread safety and scheduler fixes with specific file/function references
  • Constraint error message handling improvements
  • Remote Turso test reliability enhancements

Format complies with Keep a Changelog, semantic versioning is correct, and content accurately reflects the PR objectives and AI summary. The changelog provides appropriate technical depth for the developer audience.

test/statement_features_test.exs (3)

36-39: LGTM! Status update accurately reflects new functionality.

The comment update from "NOT IMPLEMENTED" to "SUPPORTED ✅" correctly documents that statement column introspection is now functional.


87-99: Excellent test coverage for complex query scenarios.

The test correctly validates that column introspection works with JOINs, column aliases (as user_id), and aggregate functions (COUNT(p.id) as post_count). This ensures the API handles real-world query patterns.


13-33: Well-structured test setup following best practices.

The test setup correctly follows the coding guidelines:

  • Unique database per test run ensures isolation
  • Table creation in setup
  • Proper cleanup in on_exit callback with disconnect and file removal
native/ecto_libsql/src/decode.rs (1)

1-84: Well-implemented decoding and validation module.

The module follows all coding guidelines:

  • No .unwrap() calls
  • Proper Result and Option return types
  • Defensive validation in validate_savepoint_name prevents SQL injection by enforcing valid identifier syntax

The atom-based decoding functions and ownership verification helpers provide clean abstractions for the NIF layer.

native/ecto_libsql/src/connection.rs (1)

159-329: Excellent lock management across all connection lifecycle operations.

All functions correctly:

  • Drop registry locks before async operations (lines 165, 247, 283, 315)
  • Use safe_lock and safe_lock_arc helpers consistently
  • Provide descriptive error messages with context
  • Follow the DirtyIo scheduling pattern for I/O-bound operations

The implementation aligns perfectly with the coding guidelines.

native/ecto_libsql/src/cursor.rs (1)

33-333: Well-structured cursor streaming implementation.

The module correctly implements cursor-based result streaming:

  • Lock management follows guidelines (dropped before async operations on lines 41, 193)
  • .unwrap_or(Value::Null) usage (lines 80, 165, 229) provides safe defaults for missing column values and won't panic
  • Binary allocation (lines 313-322) uses proper error handling with ok_or_else
  • Ownership verification via decode::verify_cursor_ownership prevents cross-connection misuse
  • The guard pattern in declare_cursor_with_context ensures transaction re-insertion
native/ecto_libsql/src/utils.rs (4)

16-38: Locking helpers follow best practices.

Both safe_lock and safe_lock_arc properly handle poisoned mutexes by converting errors to descriptive rustler::Error messages with context. This prevents VM panics and aligns with coding guidelines.


85-187: Constraint error enhancement is well-implemented with proper SQL safety.

The function enhances SQLite constraint errors by querying metadata to find index names. SQL injection is prevented via the quote_identifier helper (line 122-125) which properly escapes double quotes before wrapping identifiers. This aligns with SQLite's identifier quoting requirements.

The complex logic correctly:

  • Parses constraint error messages
  • Queries PRAGMA index_list and PRAGMA index_info with escaped identifiers
  • Matches index columns to constraint violations
  • Provides enhanced error messages compatible with Ecto's unique_constraint/3

327-392: Efficient query type detection with acceptable trade-offs.

The zero-allocation byte-scanning implementation provides excellent performance for a hot-path function. The documented limitation (lines 317-326) regarding string literals and comments is acceptable because:

  • False positives (using query() unnecessarily) are safe
  • False negatives (missing SELECT/RETURNING) would cause failures, but the logic prevents this
  • Full SQL parsing would be prohibitively expensive

The implementation correctly checks for whitespace boundaries and handles case-insensitive matching.


43-73: Remaining utility functions are well-implemented.

All helper functions follow coding guidelines:

  • sync_with_timeout: Proper async timeout handling with descriptive errors
  • build_empty_result: Clean result construction for write operations
  • collect_rows: Comprehensive row collection with proper binary allocation using ok_or_else pattern; .unwrap_or() usage for error message formatting is safe
  • detect_query_type and QueryType: Clean enum-based query classification
  • decode_term_to_value: Handles multiple Elixir term types without .unwrap(), proper error messages

Also applies to: 192-305, 397-423

native/ecto_libsql/src/transaction.rs (2)

58-151: Excellent RAII guard implementation for transaction safety.

The TransactionEntryGuard pattern provides robust transaction lifecycle management:

  • Ownership verification with automatic re-insertion on failure (lines 81-86)
  • Defensive checks in transaction() and consume() prevent misuse
  • Drop implementation guarantees re-insertion unless explicitly consumed
  • Handles lock poisoning gracefully in Drop (line 145)
  • Comprehensive documentation explains usage patterns

This is a textbook example of Rust's RAII pattern for resource management.


164-464: Transaction NIFs demonstrate proper async and locking patterns.

All transaction functions correctly implement the guard pattern and follow coding guidelines:

  • Locks consistently dropped before async operations (lines 170, 233, etc.)
  • Guard's Drop trait ensures transaction re-insertion on all paths except consume
  • Comments document intentional lock-across-await cases (lines 179-180, 242-243) where LibSQL's API requires it
  • Constraint error enhancement in query_with_trx_args provides better Ecto integration
  • commit_or_rollback_transaction correctly consumes the guard to prevent re-insertion (line 434)

The implementation provides robust transaction safety with proper ownership tracking.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
test/statement_features_test.exs (1)

2-10: Update the module documentation to reflect current implementation status.

The module doc states that statement introspection and reset are "not implemented", but the tests below show these features are now supported (lines 36, 116, 230 all marked with ✅).

Apply this diff to update the documentation:

   @moduledoc """
   Tests for prepared statement features.
 
   Includes:
   - Basic prepare/execute (implemented)
-  - Statement introspection: columns(), parameter_count() (not implemented)
-  - Statement reset() for reuse (not implemented)
-  - query_row() for single-row queries (not implemented)
+  - Statement introspection: columns(), parameter_count() (implemented ✅)
+  - Statement reset() for reuse (implemented ✅)
   """
🧹 Nitpick comments (1)
test/statement_features_test.exs (1)

40-59: LGTM! The per-field column metadata retrieval is working correctly.

The test properly validates the new API by checking column count first, then retrieving individual column names by index. The logic correctly expects 3 columns (id, name, age) from the users table.

Optional: Consider adding edge case tests for robustness:

  • Invalid index (e.g., index 3 when only 3 columns exist, or negative index)
  • Statements that don't return columns (INSERT, UPDATE, DELETE)

Example test:

test "stmt_column_name returns error for invalid index", %{state: state} do
  {:ok, stmt_id} = EctoLibSql.Native.prepare(state, "SELECT * FROM users")
  {:ok, count} = EctoLibSql.Native.stmt_column_count(state, stmt_id)
  
  # Out of bounds should return error
  assert {:error, _} = EctoLibSql.Native.stmt_column_name(state, stmt_id, count)
  
  EctoLibSql.Native.close_stmt(stmt_id)
end
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a733560 and 3c3a547.

📒 Files selected for processing (1)
  • test/statement_features_test.exs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
test/**/*.exs

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.exs: Write Elixir tests in test/ directory following the pattern: setup database, create test data, run test, verify results, cleanup in on_exit callback
Organize test files by functionality: ecto_adapter_test.exs for storage operations, ecto_connection_test.exs for SQL generation, ecto_integration_test.exs for full workflows, ecto_migration_test.exs for migrations, error_handling_test.exs for error cases
Use ExUnit.Case for Elixir unit and integration tests with setup callbacks for test isolation and on_exit for cleanup
Test database locking scenarios with connection pool exhaustion and timeout configurations to ensure proper error handling

Files:

  • test/statement_features_test.exs
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and transaction state
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Define new Rust NIF functions with the `#[rustler::nif(schedule = "DirtyIo")]` attribute and implement them without manual function list in `rustler::init!()` - Rustler automatically detects annotated functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Document SQLite limitations in code comments, especially regarding ALTER TABLE, DROP COLUMN, and column type changes which require table recreation

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Implement query compilation in Connection module for SELECT, INSERT, UPDATE, DELETE statements with WHERE, JOIN, ORDER BY, LIMIT support

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/*.ex : Use the EctoLibSql.State struct to track connection state including conn_id, trx_id, mode (:local, :remote, :remote_replica), and sync setting

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use statement introspection functions (stmt_parameter_count, stmt_column_count, stmt_column_name) to validate prepared statement structure

Applied to files:

  • test/statement_features_test.exs
⏰ 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). (3)
  • GitHub Check: Elixir 1.17.0 / OTP 26.2 / ubuntu-latest
  • GitHub Check: Elixir 1.17.0 / OTP 26.2 / macos-latest
  • GitHub Check: Analyze (rust)
🔇 Additional comments (2)
test/statement_features_test.exs (2)

61-103: LGTM! Complex query column metadata retrieval works correctly.

The test properly validates that the per-field API works with complex queries including JOINs, column aliases, and aggregate functions. The expected column names (user_id, name, post_count) match the query structure.


230-239: LGTM! Parameter count introspection test is correct.

The test properly validates that stmt_parameter_count/2 returns the correct number of parameter placeholders in the prepared statement.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
test/statement_features_test.exs (1)

88-119: Consider consolidating duplicate out-of-bounds tests.

The tests "column_name returns error for out-of-bounds indices" (lines 88-109) and "stmt_column_name returns error for invalid index" (lines 111-119) both verify out-of-bounds behaviour for stmt_column_name/3. The first test already covers the invalid index case comprehensively by testing both high values (3, 100) and valid boundary cases (0, 2).

You could remove the second test to reduce duplication:

-    test "stmt_column_name returns error for invalid index", %{state: state} do
-      {:ok, stmt_id} = EctoLibSql.Native.prepare(state, "SELECT * FROM users")
-      {:ok, count} = EctoLibSql.Native.stmt_column_count(state, stmt_id)
-
-      # Out of bounds should return error
-      assert {:error, _} = EctoLibSql.Native.stmt_column_name(state, stmt_id, count)
-
-      EctoLibSql.Native.close_stmt(stmt_id)
-    end
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c3a547 and 8bb1e64.

📒 Files selected for processing (2)
  • native/ecto_libsql/src/connection.rs (1 hunks)
  • test/statement_features_test.exs (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • native/ecto_libsql/src/connection.rs
🧰 Additional context used
📓 Path-based instructions (1)
test/**/*.exs

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.exs: Write Elixir tests in test/ directory following the pattern: setup database, create test data, run test, verify results, cleanup in on_exit callback
Organize test files by functionality: ecto_adapter_test.exs for storage operations, ecto_connection_test.exs for SQL generation, ecto_integration_test.exs for full workflows, ecto_migration_test.exs for migrations, error_handling_test.exs for error cases
Use ExUnit.Case for Elixir unit and integration tests with setup callbacks for test isolation and on_exit for cleanup
Test database locking scenarios with connection pool exhaustion and timeout configurations to ensure proper error handling

Files:

  • test/statement_features_test.exs
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Use lazy static Mutex for global registries (CONNECTION_REGISTRY, TXN_REGISTRY, STMT_REGISTRY, CURSOR_REGISTRY) in Rust to manage connection and transaction state
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : Define new Rust NIF functions with the `#[rustler::nif(schedule = "DirtyIo")]` attribute and implement them without manual function list in `rustler::init!()` - Rustler automatically detects annotated functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to native/**/*.rs : For Rust NIFs using async/await, drop locks before async blocks to prevent holding locks across await points
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/**/*.ex : Document SQLite limitations in code comments, especially regarding ALTER TABLE, DROP COLUMN, and column type changes which require table recreation

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/native.ex : Add Elixir NIF stub functions to `lib/ecto_libsql/native.ex` with `:erlang.nif_error(:nif_not_loaded)` for each new Rust NIF, and provide safe wrapper functions that use EctoLibSql.State

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql/*.ex : Use the EctoLibSql.State struct to track connection state including conn_id, trx_id, mode (:local, :remote, :remote_replica), and sync setting

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Implement query compilation in Connection module for SELECT, INSERT, UPDATE, DELETE statements with WHERE, JOIN, ORDER BY, LIMIT support

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Use statement introspection functions (stmt_parameter_count, stmt_column_count, stmt_column_name) to validate prepared statement structure

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Handle NIF errors gracefully by pattern matching on {:ok, result} and {:error, reason} returns from EctoLibSql.Native functions

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Use `quote_name/1` function to safely quote identifiers (table names, column names, aliases) in generated SQL to prevent injection attacks

Applied to files:

  • test/statement_features_test.exs
📚 Learning: 2025-12-14T21:02:26.998Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T21:02:26.998Z
Learning: Applies to lib/ecto_libsql.ex : Implement handle_declare/4, handle_fetch/4, handle_deallocate/4 callbacks for cursor streaming operations

Applied to files:

  • test/statement_features_test.exs
⏰ 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). (3)
  • GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
  • GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
  • GitHub Check: Analyze (rust)
🔇 Additional comments (4)
test/statement_features_test.exs (4)

1-9: LGTM!

The module documentation clearly describes the test scope and features being tested.


35-49: LGTM!

The test properly validates statement column introspection using the two-step approach (stmt_column_countstmt_column_name via helper). Test structure follows best practices with proper setup and cleanup.


51-86: Good coverage of complex query scenarios.

This test validates that column introspection works correctly with joins, aggregates, and aliased columns. This is important for ensuring the NIF handles non-trivial SQL properly.


266-271: LGTM!

The helper function effectively reduces code duplication across tests. The assertive pattern matching ({:ok, name} = ...) is appropriate for test helpers, as tests should fail fast on unexpected errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant