-
Notifications
You must be signed in to change notification settings - Fork 0
tests: Add more tests from the Ecto SQL adapter to strengthen our testing - SQL compatibility, streaming and transactions testing #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ting - SQL compatibility, streaming and transactions testing
WalkthroughAdds prepared-statement management (reset and column-metadata retrieval), enables Turso remote encryption via a Changes
Sequence Diagram(s)sequenceDiagram
participant App as Elixir App
participant Wrapper as EctoLibSql.Native
participant NIF as Rust NIF
participant Reg as Statement Registry
participant Conn as LibSQL Connection
rect rgb(220,235,255)
Note over App,Reg: Reset Statement Flow
App->>Wrapper: reset_stmt(state, stmt_id)
Wrapper->>NIF: reset_statement(conn_id, stmt_id)
NIF->>Reg: verify ownership & acquire lock
NIF->>Reg: clear/reset cached prepared statement
Reg-->>NIF: :ok
NIF-->>Wrapper: :ok
Wrapper-->>App: {:ok, :reset}
end
rect rgb(220,255,230)
Note over App,Reg: Get Statement Columns Flow
App->>Wrapper: get_stmt_columns(state, stmt_id)
Wrapper->>NIF: get_statement_columns(conn_id, stmt_id)
NIF->>Reg: verify ownership & acquire lock
NIF->>Reg: fetch prepared statement metadata
loop per column
NIF->>NIF: extract (name, origin_name, decl_type)
end
Reg-->>NIF: [column tuples]
NIF-->>Wrapper: {:ok, [columns]}
Wrapper-->>App: {:ok, columns}
end
rect rgb(255,245,215)
Note over App,Conn: Remote Encryption Setup Flow
App->>Wrapper: connect(uri, [remote_encryption_key: key])
Wrapper->>NIF: build connection with opts
NIF->>Conn: parse options, read remote_encryption_key
Conn->>Conn: create EncryptionContext (Base64Encoded)
Conn->>Conn: build LibSQL client with remote encryption
Conn-->>NIF: connected (conn_id)
NIF-->>Wrapper: {:ok, conn_id}
Wrapper-->>App: {:ok, conn_id}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ex,exs,rs,md}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ex,exs}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
test/**/*.exs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (11)📓 Common learnings📚 Learning: 2025-12-17T11:58:46.019ZApplied to files:
📚 Learning: 2025-12-17T11:58:46.019ZApplied to files:
📚 Learning: 2025-12-17T11:58:26.477ZApplied to files:
📚 Learning: 2025-12-17T11:58:26.477ZApplied to files:
📚 Learning: 2025-12-17T11:58:26.477ZApplied to files:
📚 Learning: 2025-12-17T11:58:26.477ZApplied to files:
📚 Learning: 2025-12-17T11:58:26.477ZApplied to files:
📚 Learning: 2025-12-17T11:58:26.477ZApplied to files:
📚 Learning: 2025-12-17T11:58:26.477ZApplied to files:
📚 Learning: 2025-12-17T11:58:26.477ZApplied to files:
⏰ 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)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this 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 (6)
test/prepared_statement_test.exs (1)
354-411: Consider adding a:flakytag to the performance test.Similar to the performance test in
test/statement_features_test.exs(line 186), this benchmark may exhibit variance in CI environments. The 2x threshold is conservative, but adding:flakywould align with the existing pattern.Proposed change
+ @tag :flaky @tag :performance test "prepared vs unprepared statement performance comparison", %{state: state} dotest/ecto_stream_compat_test.exs (3)
73-75: Consider cleaning up WAL journal files on exit.When using SQLite with WAL mode (which may be enabled by default or by other tests), companion files (
-waland-shm) may remain. These should also be removed for a clean test teardown.🔎 Suggested cleanup enhancement
on_exit(fn -> File.rm(@test_db) + File.rm(@test_db <> "-wal") + File.rm(@test_db <> "-shm") end)
151-168: Consider usinginsert_allfor consistency with the previous test.The first test (line 139) uses
insert_allfor efficient bulk insertion, but this test usesEnum.eachwith individual inserts. Usinginsert_allwould be more consistent and faster.🔎 Optional refactor for consistency
test "stream with query and transformations" do # Insert posts - Enum.each(1..50, fn i -> - TestRepo.insert!(%Post{title: "Post #{i}"}) - end) + posts = Enum.map(1..50, fn i -> %{title: "Post #{i}", body: nil} end) + TestRepo.insert_all(Post, posts)
193-211: Test title may be misleading: streams are sequential, not concurrent.The test is titled "multiple concurrent streams in same transaction", but the streams are consumed sequentially (
post_streamis fully consumed beforecomment_streamstarts). For truly concurrent behaviour, you'd interleave consumption (e.g., usingStream.zip). The current test still validates useful behaviour—perhaps rename to "multiple sequential streams" for clarity.test/ecto_sql_transaction_compat_test.exs (2)
56-58: Consider cleaning up WAL journal files on exit.Similar to the streaming test file, WAL companion files should be removed. Since
test_dbis a local variable, you'll need to capture it.🔎 Suggested cleanup enhancement
on_exit(fn -> File.rm(test_db) + File.rm(test_db <> "-wal") + File.rm(test_db <> "-shm") end)
63-89: Retry logic is pragmatic but could be more robust.The retry mechanism handles SQLite lock contention well. However, using bare
rescue _catches all exceptions, which could mask unexpected errors. Consider rescuing specific exception types likeEcto.Adapters.SQL.QueryErrororDBConnection.ConnectionError.🔎 More explicit error handling
setup do # Clean table before each test with a retry mechanism cleanup_with_retry = fn -> try do Ecto.Adapters.SQL.query!(TestRepo, "DELETE FROM transactions") :ok rescue - _ -> + e in [Ecto.Adapters.SQL.QueryError, DBConnection.ConnectionError] -> # If locked, wait and retry Process.sleep(200) try do Ecto.Adapters.SQL.query!(TestRepo, "DELETE FROM transactions") :ok rescue - _ -> + e in [Ecto.Adapters.SQL.QueryError, DBConnection.ConnectionError] -> # Final retry after longer wait Process.sleep(500) Ecto.Adapters.SQL.query!(TestRepo, "DELETE FROM transactions") :ok end end end
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
CHANGELOG.mdREADME.mdRESILIENCE_IMPROVEMENTS.mdTESTING_PLAN_COMPREHENSIVE.mdlib/ecto_libsql/native.exnative/ecto_libsql/src/connection.rsnative/ecto_libsql/src/statement.rstest/ecto_sql_compatibility_test.exstest/ecto_sql_transaction_compat_test.exstest/ecto_stream_compat_test.exstest/prepared_statement_test.exstest/statement_features_test.exs
💤 Files with no reviewable changes (2)
- RESILIENCE_IMPROVEMENTS.md
- TESTING_PLAN_COMPREHENSIVE.md
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS use British/Australian English spelling and grammar for code, comments, and documentation, except where required for function calls, SQL keywords, error messages, or compatibility with external systems that may use US English
Files:
CHANGELOG.mdnative/ecto_libsql/src/statement.rslib/ecto_libsql/native.extest/prepared_statement_test.exstest/ecto_stream_compat_test.exsnative/ecto_libsql/src/connection.rsREADME.mdtest/ecto_sql_compatibility_test.exstest/statement_features_test.exstest/ecto_sql_transaction_compat_test.exs
native/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Rust Cargo formatter (
cargo fmt) before committing changes and fix any issues
Files:
native/ecto_libsql/src/statement.rsnative/ecto_libsql/src/connection.rs
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Elixir formatter (
mix format --check-formatted) before committing changes and fix any issues
Files:
lib/ecto_libsql/native.extest/prepared_statement_test.exstest/ecto_stream_compat_test.exstest/ecto_sql_compatibility_test.exstest/statement_features_test.exstest/ecto_sql_transaction_compat_test.exs
lib/ecto_libsql/native.ex
📄 CodeRabbit inference engine (CLAUDE.md)
lib/ecto_libsql/native.ex: For each new Rust NIF function, add corresponding Elixir NIF stubs and safe wrapper functions in lib/ecto_libsql/native.ex, with proper state management via EctoLibSql.State
For explicitly unsupported functions, update the Elixir wrapper with comprehensive documentation explaining why the feature is unsupported, what architectural constraints prevent it, and what alternative approaches users can take
Files:
lib/ecto_libsql/native.ex
lib/**/*.ex
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.ex: When updating Elixir code, follow the pattern of case matching on NIF results:{:ok, _, result, new_state}for successful query execution or{:error, reason}for failures
In Elixir error handling, preferwithclauses for chaining multiple operations that return Result tuples, providing clear error handling with anelseclause
In Ecto changesets, use thecastfunction to automatically attempt type conversions for fields, which will raise ChangeError if types don't match schema expectations
Use transactions with appropriate isolation behaviours when executing database operations::deferredfor mixed workloads,:immediatefor write-heavy operations,:exclusivefor critical operations requiring exclusive locks,:read_onlyfor read-only queries
When working with database locks, use transactions with proper timeout configuration, ensure connections are properly closed in try-after blocks, and use immediate/exclusive transaction behaviours for write-heavy workloads to minimise lock contention
When handling vector search operations, verify LibSQL version supports vectors, use correct vector syntax withEctoLibSql.Native.vector()function, insert vectors withvector(?)SQL syntax, and query with vector distance functions likevector_distance_cos
Files:
lib/ecto_libsql/native.ex
**/*.ex
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ex: Use prepared statements viaEctoLibSql.Native.prepare/2andEctoLibSql.Native.query_stmt/3for repeated queries to improve performance by ~10-15x
Use batch operations withEctoLibSql.Native.batch_transactional/2for bulk inserts/updates instead of individual statements
Implement proper error handling by pattern matching on{:ok, ...}and{:error, ...}tuples from all EctoLibSql operations
Use transactions withEctoLibSql.handle_begin/2,EctoLibSql.handle_commit/2, andEctoLibSql.handle_rollback/2for multi-step database operations
Use cursor streaming withDBConnection.stream/3for memory-efficient processing of large result sets
Use vector search withEctoLibSql.Native.vector/1,EctoLibSql.Native.vector_type/2, andEctoLibSql.Native.vector_distance_cos/2for AI/ML features
UseEctoLibSql.Pragma.*functions to configure SQLite parameters such as foreign keys, journal mode, cache size, and synchronous level
Use savepoints withEctoLibSql.Native.create_savepoint/2,EctoLibSql.Native.release_savepoint_by_name/2, andEctoLibSql.Native.rollback_to_savepoint_by_name/2for nested transactions
Use Ecto changesets for data validation before inserting or updating records
Use Ecto query composition withEcto.Queryfor building complex queries incrementally
Preload associations withRepo.preload/2to avoid N+1 query problems in Ecto applications
For remote replicas in production, useEctoLibSql.Native.sync/1,EctoLibSql.Native.get_frame_number_for_replica/1,EctoLibSql.Native.sync_until_frame/2, andEctoLibSql.Native.flush_and_get_frame/1for replication management
Use busy timeout configuration withEctoLibSql.Native.busy_timeout/2to handle database locking conflicts gracefully
Use statement introspection functions (EctoLibSql.Native.stmt_parameter_count/2,EctoLibSql.Native.stmt_column_count/2,EctoLibSql.Native.stmt_column_name/3) to validate prepared statement structure
Handle connection errors gracefully by ch...
Files:
lib/ecto_libsql/native.ex
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.exs: Add tests for all new features in appropriate Elixir test files (test/ecto_*_test.exs) and Rust test modules (native/ecto_libsql/src/tests/), covering happy path, error cases, edge cases, and type conversions
Add comprehensive tests for unsupported functions asserting that they always return {:error, :unsupported} and that they don't modify database state or corrupt connections
Files:
test/prepared_statement_test.exstest/ecto_stream_compat_test.exstest/ecto_sql_compatibility_test.exstest/statement_features_test.exstest/ecto_sql_transaction_compat_test.exs
🧠 Learnings (37)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to test/**/*.exs : Add comprehensive tests for unsupported functions asserting that they always return {:error, :unsupported} and that they don't modify database state or corrupt connections
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to test/**/*.exs : Add tests for all new features in appropriate Elixir test files (test/ecto_*_test.exs) and Rust test modules (native/ecto_libsql/src/tests/), covering happy path, error cases, edge cases, and type conversions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/ecto/adapters/libsql.ex : When implementing Ecto storage operations (create, drop, status) in lib/ecto/adapters/libsql.ex, ensure they properly handle both local SQLite and remote Turso databases via the connection mode in EctoLibSql.State
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : Use prepared statements via `EctoLibSql.Native.prepare/2` and `EctoLibSql.Native.query_stmt/3` for repeated queries to improve performance by ~10-15x
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : Use statement introspection functions (`EctoLibSql.Native.stmt_parameter_count/2`, `EctoLibSql.Native.stmt_column_count/2`, `EctoLibSql.Native.stmt_column_name/3`) to validate prepared statement structure
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/ecto_libsql/native.ex : For explicitly unsupported functions, update the Elixir wrapper with comprehensive documentation explaining why the feature is unsupported, what architectural constraints prevent it, and what alternative approaches users can take
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/ecto_libsql/native.ex : For explicitly unsupported functions, update the Elixir wrapper with comprehensive documentation explaining why the feature is unsupported, what architectural constraints prevent it, and what alternative approaches users can take
Applied to files:
CHANGELOG.mdnative/ecto_libsql/src/statement.rslib/ecto_libsql/native.extest/prepared_statement_test.exstest/statement_features_test.exs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : Use prepared statements via `EctoLibSql.Native.prepare/2` and `EctoLibSql.Native.query_stmt/3` for repeated queries to improve performance by ~10-15x
Applied to files:
CHANGELOG.mdnative/ecto_libsql/src/statement.rslib/ecto_libsql/native.extest/prepared_statement_test.exstest/statement_features_test.exs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : Use statement introspection functions (`EctoLibSql.Native.stmt_parameter_count/2`, `EctoLibSql.Native.stmt_column_count/2`, `EctoLibSql.Native.stmt_column_name/3`) to validate prepared statement structure
Applied to files:
CHANGELOG.mdnative/ecto_libsql/src/statement.rslib/ecto_libsql/native.extest/prepared_statement_test.exstest/statement_features_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/ecto_libsql/native.ex : For each new Rust NIF function, add corresponding Elixir NIF stubs and safe wrapper functions in lib/ecto_libsql/native.ex, with proper state management via EctoLibSql.State
Applied to files:
CHANGELOG.mdnative/ecto_libsql/src/statement.rslib/ecto_libsql/native.extest/prepared_statement_test.exstest/statement_features_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/ecto/adapters/libsql.ex : When implementing Ecto storage operations (create, drop, status) in lib/ecto/adapters/libsql.ex, ensure they properly handle both local SQLite and remote Turso databases via the connection mode in EctoLibSql.State
Applied to files:
CHANGELOG.mdlib/ecto_libsql/native.extest/prepared_statement_test.exsREADME.mdtest/ecto_sql_compatibility_test.exstest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : In Rust, all non-test code must have no `.unwrap()` calls; this was a critical refactoring in v0.5.0 to eliminate 146 unwrap calls and prevent BEAM VM crashes from panics in native code
Applied to files:
CHANGELOG.mdnative/ecto_libsql/src/statement.rs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : In lib/ecto/adapters/libsql/connection.ex, implement SQL expression handlers for SQLite functions by pattern matching on function names and converting them to SQL fragments, respecting SQLite syntax and limitations
Applied to files:
CHANGELOG.mdlib/ecto_libsql/native.extest/prepared_statement_test.exstest/ecto_sql_compatibility_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/**/*.ex : When handling vector search operations, verify LibSQL version supports vectors, use correct vector syntax with `EctoLibSql.Native.vector()` function, insert vectors with `vector(?)` SQL syntax, and query with vector distance functions like `vector_distance_cos`
Applied to files:
CHANGELOG.mdlib/ecto_libsql/native.extest/ecto_stream_compat_test.exstest/ecto_sql_compatibility_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : In Rust async blocks within NIFs, drop registry locks before entering the `TOKIO_RUNTIME.block_on(async { ... })` call to prevent deadlocks, then re-acquire locks if needed after the async block completes
Applied to files:
CHANGELOG.mdnative/ecto_libsql/src/statement.rsnative/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : When adding a new NIF function in Rust, place it in the appropriate module based on functionality: connection.rs (lifecycle), query.rs (execution), transaction.rs (transactions), batch.rs (batch operations), statement.rs (prepared statements), cursor.rs (streaming), replication.rs (replica sync), metadata.rs (metadata access), or savepoint.rs (nested transactions)
Applied to files:
native/ecto_libsql/src/statement.rslib/ecto_libsql/native.extest/prepared_statement_test.exstest/ecto_stream_compat_test.exsnative/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to native/ecto_libsql/src/transaction.rs : Use the TransactionEntry struct in Rust transaction operations to track which connection owns each transaction, validating ownership in savepoint and transaction operations to prevent cross-connection misuse
Applied to files:
native/ecto_libsql/src/statement.rsnative/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : Include comprehensive Rust documentation comments (/// comments) on all public NIF functions explaining purpose, arguments, return values (success and error cases), and usage examples
Applied to files:
native/ecto_libsql/src/statement.rslib/ecto_libsql/native.ex
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to native/ecto_libsql/src/savepoint.rs : In savepoint operations in native/ecto_libsql/src/savepoint.rs, validate that the transaction belongs to the connection by comparing entry.conn_id with the provided conn_id parameter
Applied to files:
native/ecto_libsql/src/statement.rslib/ecto_libsql/native.ex
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/**/*.ex : When updating Elixir code, follow the pattern of case matching on NIF results: `{:ok, _, result, new_state}` for successful query execution or `{:error, reason}` for failures
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : Use savepoints with `EctoLibSql.Native.create_savepoint/2`, `EctoLibSql.Native.release_savepoint_by_name/2`, and `EctoLibSql.Native.rollback_to_savepoint_by_name/2` for nested transactions
Applied to files:
lib/ecto_libsql/native.extest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : For remote replicas in production, use `EctoLibSql.Native.sync/1`, `EctoLibSql.Native.get_frame_number_for_replica/1`, `EctoLibSql.Native.sync_until_frame/2`, and `EctoLibSql.Native.flush_and_get_frame/1` for replication management
Applied to files:
lib/ecto_libsql/native.exREADME.md
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to test/**/*.exs : Add comprehensive tests for unsupported functions asserting that they always return {:error, :unsupported} and that they don't modify database state or corrupt connections
Applied to files:
test/prepared_statement_test.exstest/ecto_stream_compat_test.exstest/ecto_sql_compatibility_test.exstest/statement_features_test.exstest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to test/**/*.exs : Add tests for all new features in appropriate Elixir test files (test/ecto_*_test.exs) and Rust test modules (native/ecto_libsql/src/tests/), covering happy path, error cases, edge cases, and type conversions
Applied to files:
test/prepared_statement_test.exstest/ecto_stream_compat_test.exstest/ecto_sql_compatibility_test.exstest/statement_features_test.exstest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to test/**/*migration*test.exs : For SQLite migrations that require column type changes or drops (which SQLite doesn't support via ALTER), use the table recreation pattern: create new table, copy data with transformations, swap tables, and recreate indexes
Applied to files:
test/prepared_statement_test.exstest/ecto_stream_compat_test.exstest/ecto_sql_compatibility_test.exstest/statement_features_test.exstest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : Use cursor streaming with `DBConnection.stream/3` for memory-efficient processing of large result sets
Applied to files:
test/ecto_stream_compat_test.exs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*_schema.{ex,exs} : Use Ecto schemas with proper type definitions for database models in Elixir applications
Applied to files:
test/ecto_stream_compat_test.exstest/ecto_sql_compatibility_test.exs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/repo.ex : Define custom Ecto repositories using `Ecto.Repo` with `Ecto.Adapters.LibSql` adapter for ecto_libsql projects
Applied to files:
test/ecto_stream_compat_test.exsREADME.mdtest/ecto_sql_compatibility_test.exstest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : Use transactions with `EctoLibSql.handle_begin/2`, `EctoLibSql.handle_commit/2`, and `EctoLibSql.handle_rollback/2` for multi-step database operations
Applied to files:
test/ecto_stream_compat_test.exsREADME.mdtest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : Implement proper error handling by pattern matching on `{:ok, ...}` and `{:error, ...}` tuples from all EctoLibSql operations
Applied to files:
test/ecto_stream_compat_test.exstest/ecto_sql_compatibility_test.exstest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : In Rust, use the `safe_lock_arc(&arc_mutex, "context_description")` helper function instead of directly calling `.lock().unwrap()` on Arc<Mutex<T>> types to safely acquire locks with proper error handling
Applied to files:
native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : In Rust, use the `safe_lock(®ISTRY, "context_description")` helper function instead of directly calling `.lock().unwrap()` on Mutex types to safely acquire locks with proper error handling
Applied to files:
native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : NEVER use `.unwrap()` in production Rust code; all Rust production code must use safe error handling via `safe_lock` helpers, `ok_or_else` for Option types, and the `?` operator for error propagation
Applied to files:
native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : In Rust production NIFs, use `ok_or_else` to convert Option types to Result, providing clear error messages via Term boxing: `.ok_or_else(|| rustler::Error::Term(Box::new("error message")))`
Applied to files:
native/ecto_libsql/src/connection.rs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to config/**.exs : Use remote replica mode (`database`, `uri`, `auth_token`, `sync: true`) for production deployments with Turso
Applied to files:
README.md
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to test/turso_remote_test.exs : For Turso remote tests, use environment variables TURSO_DB_URI and TURSO_AUTH_TOKEN, loading them from .env.local file before running tests; remote tests should be skipped by default if credentials are missing
Applied to files:
README.md
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/**/*.ex : When working with database locks, use transactions with proper timeout configuration, ensure connections are properly closed in try-after blocks, and use immediate/exclusive transaction behaviours for write-heavy workloads to minimise lock contention
Applied to files:
README.mdtest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/**/*.ex : Use transactions with appropriate isolation behaviours when executing database operations: `:deferred` for mixed workloads, `:immediate` for write-heavy operations, `:exclusive` for critical operations requiring exclusive locks, `:read_only` for read-only queries
Applied to files:
README.mdtest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : Use `EctoLibSql.Pragma.*` functions to configure SQLite parameters such as foreign keys, journal mode, cache size, and synchronous level
Applied to files:
README.mdtest/ecto_sql_compatibility_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: Applies to lib/ecto/adapters/libsql.ex : In type loaders/dumpers in lib/ecto/adapters/libsql.ex, ensure bidirectional conversion: loaders convert SQLite values to Elixir types on retrieval, dumpers convert Elixir types to SQLite on storage
Applied to files:
test/ecto_sql_compatibility_test.exs
📚 Learning: 2025-12-17T11:58:26.464Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.464Z
Learning: When debugging failing tests, run with trace (`mix test file.exs --trace`), verify NIF compilation (`File.exists?("priv/native/ecto_libsql.so")`), check Rust output with `cargo test -- --nocapture`, and run Rust tests independently to isolate issues
Applied to files:
test/ecto_sql_compatibility_test.exstest/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:46.002Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.002Z
Learning: Applies to **/*.ex : Use batch operations with `EctoLibSql.Native.batch_transactional/2` for bulk inserts/updates instead of individual statements
Applied to files:
test/ecto_sql_transaction_compat_test.exs
🧬 Code graph analysis (6)
native/ecto_libsql/src/statement.rs (3)
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/decode.rs (1)
verify_statement_ownership(47-54)
lib/ecto_libsql/native.ex (1)
native/ecto_libsql/src/statement.rs (2)
reset_statement(357-381)get_statement_columns(407-448)
test/prepared_statement_test.exs (2)
lib/ecto_libsql/native.ex (3)
prepare(478-486)query_stmt(530-549)close_stmt(562-564)lib/ecto_libsql.ex (1)
handle_execute(134-151)
test/ecto_sql_compatibility_test.exs (1)
lib/ecto/adapters/libsql/connection.ex (4)
from(663-666)all(435-464)update_all(467-476)delete_all(479-487)
test/statement_features_test.exs (2)
lib/ecto_libsql/native.ex (3)
prepare(478-486)execute_stmt(502-515)close_stmt(562-564)lib/ecto_libsql.ex (1)
handle_execute(134-151)
test/ecto_sql_transaction_compat_test.exs (4)
native/ecto_libsql/src/transaction.rs (1)
transaction(100-111)lib/ecto/adapters/libsql/connection.ex (3)
all(435-464)from(663-666)update_all(467-476)lib/ecto_libsql/native.ex (1)
rollback(441-445)lib/ecto_libsql.ex (1)
checkout(240-245)
🪛 LanguageTool
CHANGELOG.md
[duplication] ~33-~33: Possible typo: you repeated a word.
Context: ...ryption_key`) for at-rest database file encryption - Encryption types: - Local encryption: AE...
(ENGLISH_WORD_REPEAT_RULE)
⏰ 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 (31)
native/ecto_libsql/src/connection.rs (3)
86-101: LGTM – RemoteReplica encryption configuration.The implementation correctly applies:
- Local encryption via
EncryptionConfigfor the replica file- Remote encryption via
EncryptionContextwithBase64Encodedkey for Turso requestsBoth encryption types can be used simultaneously for end-to-end encryption, as documented.
105-119: LGTM – Remote mode encryption support.The remote encryption configuration is correctly applied before calling
builder.build().await. The builder reassignment pattern (builder = builder.remote_encryption(...)) is consistent with the existing local encryption pattern.
10-10: Well-structured remote encryption support.The new imports and documentation clearly explain the distinction between local encryption (
encryption_keyfor at-rest AES-256-CBC) and remote encryption (remote_encryption_keyfor Turso encrypted databases). The option parsing follows the established pattern, and the builder correctly applies bothEncryptionConfigfor local encryption andEncryptionContextwithEncryptionKey::Base64Encodedfor remote encryption.native/ecto_libsql/src/statement.rs (2)
327-381: Well-implementedreset_statementNIF.The implementation correctly:
- Validates connection ID and statement ownership before proceeding
- Follows the established lock management pattern (acquire, clone, drop, then access)
- Includes comprehensive documentation with usage examples
- Returns
:okatom on success, consistent with other similar functions
383-448: Well-implementedget_statement_columnsNIF.The column metadata extraction is thorough:
- Returns
(name, origin_name, decl_type)tuples for each column- Sensible fallback:
origin_namedefaults tonamewhen not availabledecl_typeis correctlyOption<String>since SQLite may not have type info for expressionsThe lock management follows the established pattern, and ownership verification prevents cross-connection access.
test/statement_features_test.exs (2)
237-298: Comprehensive tests forreset_stmtfunctionality.Good coverage including:
- Explicit reset followed by successful re-execution
- Multiple consecutive resets in a loop
- Error handling for invalid statement IDs
The tests verify the core use case of resetting statements for reuse without re-preparing.
304-384: Thorough tests forget_stmt_columnsmetadata retrieval.Excellent edge case coverage:
- Basic column metadata with declared types (INTEGER, TEXT)
- Aliased columns verify that
namereflects the alias- Expression columns (COUNT, MAX) confirm metadata is still accessible
The tuple destructuring pattern
{col1_name, col1_origin, col1_type}matches the Rust NIF return type.CHANGELOG.md (1)
8-43: Well-documented changelog entries for new features.The [Unreleased] section comprehensively covers:
- Statement Reset: Performance benefits (10-15x), usage pattern, and return values
- Statement Column Metadata: Use cases for type introspection and schema discovery
- Remote Encryption: Clear distinction between local and remote encryption modes
The static analysis hint about word repetition is a false positive – "encryption" appears in distinct contexts (file encryption vs encryption types).
test/ecto_sql_compatibility_test.exs (4)
1-66: Well-structured Ecto SQL compatibility test module.Good setup:
- Unique database naming with cleanup prevents test pollution
- Per-test table cleanup ensures test isolation
async: falseis appropriate for tests sharing a single database connection- Internal TestRepo and Post schema keep tests self-contained
86-97: Appropriately skipped test with clear rationale.The skip explanation correctly notes that schemaless
type()queries behave differently in SQLite/LibSQL compared to PostgreSQL. This documents a known compatibility gap.
158-200: Comprehensive escaping tests for SQL injection prevention.Good coverage of single quote escaping across all CRUD operations:
insert!,update!,insert_all,update_all,delete_allThese tests verify that LibSQL correctly escapes special characters to prevent SQL injection.
202-248: Thorough utility function tests.Coverage includes:
load/2converting raw query results to Ecto structstable_exists?/2for both existing and non-existent tablesformat_table/1with comprehensive edge cases (nil, empty columns/rows)README.md (2)
306-339: Clear and comprehensive encryption documentation.Excellent coverage of all encryption scenarios:
- Local-only encryption
- Remote Turso encryption
- Combined local + remote for embedded replicas
The Notes section helpfully clarifies:
- When to use each key type
- Base64 encoding requirement for remote keys
- Link to Turso documentation for key generation
425-426: Configuration options table updated correctly.The table now clearly distinguishes:
encryption_key: For local file encryption (AES-256-CBC)remote_encryption_key: For Turso encrypted databases (base64-encoded)lib/ecto_libsql/native.ex (3)
151-155: NIF stubs correctly declared.The new stubs
reset_statement/2andget_statement_columns/2follow the established pattern, raising:nif_not_loadeduntil the Rust NIF is loaded.
1267-1301: Well-documentedreset_stmt/2wrapper.The function correctly:
- Extracts
conn_idfrom state- Validates both parameters are binaries via guards
- Returns the NIF result directly (
:okor{:error, reason})- Includes performance note about 10-15x speedup
1336-1344: Defensive result normalisation inget_stmt_columns/2.The function handles multiple response formats:
{:ok, columns}– passed through- Raw list – wrapped in
{:ok, result}{:error, reason}– passed through- Unexpected responses – converted to error tuple
This defensive approach guards against potential NIF response format changes.
test/prepared_statement_test.exs (2)
311-332: Good test for binding isolation between executions.This test correctly verifies that parameter bindings are cleared between statement executions. The explicit
refuteassertion at line 329 clearly documents the expected behaviour.
413-441: LGTM – Memory efficiency test documents expected behaviour.The test appropriately:
- Avoids platform-specific memory assertions
- Documents expected behaviour for manual leak detection
- Uses realistic workload (1000 iterations)
test/ecto_stream_compat_test.exs (4)
1-13: LGTM! Clear module documentation and appropriate test configuration.The moduledoc clearly describes the source and purpose of these ported tests. Using
async: falseis correct for file-based SQLite testing to avoid concurrent access issues.
17-42: LGTM! Well-structured inline schemas with proper associations.The full module path references (
EctoLibSql.EctoStreamCompatTest.Commentand.Post) are correctly used for the nested schema associations.
87-129: LGTM! Good coverage of basic streaming scenarios.The tests correctly verify streaming with empty results, schemaless queries, and associations. The pattern of creating the stream outside the transaction (line 119) and consuming it inside (line 123) is valid and consistent with Ecto's streaming semantics.
214-235: LGTM! Good verification ofmax_rowsstreaming option.The test correctly validates that streaming with
max_rowsworks as expected and that the result types are correct. The comment on lines 221-222 helpfully clarifies thatmax_rowsaffects fetching, not the stream output format.test/ecto_sql_transaction_compat_test.exs (8)
1-14: LGTM! Excellent moduledoc with clear test scope.The documentation clearly outlines the source and the specific transaction behaviours being verified, including LibSQL-specific modes. Using
async: falseis appropriate for SQLite transaction testing.
91-145: LGTM! Comprehensive coverage of basic transaction behaviour.Tests correctly verify transaction return values, error propagation, successful commits, and rollback on exceptions. The use of
TestRepo.in_transaction?()to verify transaction state is a good practice.
147-202: LGTM! Excellent documentation of SQLite-specific nested transaction behaviour.The comments on lines 163-164 and the assertions clearly document that SQLite's savepoint semantics differ from PostgreSQL—nested rollbacks poison the outer transaction. This is valuable for users migrating from other databases.
204-259: Good documentation of SQLite concurrency limitations via skipped tests.Marking these tests with
@tag :skipand@tag :sqlite_concurrency_limitationappropriately documents that these PostgreSQL-compatible behaviours aren't supported in SQLite's single-writer model. Keeping the test code serves as reference documentation.
344-368: LGTM! Clear verification of checkout and transaction interaction.Tests correctly verify that
in_transaction?()reflects the expected state when nesting checkouts and transactions in both directions.
370-401: LGTM! Critical tests for error recovery and state cleanliness.These tests verify that the connection state is properly cleaned up after transaction errors and manual rollbacks, ensuring subsequent operations work correctly. This is essential for connection pool health.
403-451: LGTM! Excellent end-to-end verification of complex transactional workflows.These tests provide valuable coverage of realistic multi-operation transactions including inserts, updates, deletes, and queries within a single transaction, verifying the final data state is correct.
312-327: Add a negative test to verify writes fail in read-only transactions.The test currently verifies reads work in
:read_onlymode but doesn't test that write operations are rejected. Add an assertion to confirmTestRepo.insert!/1raises an error within a read-only transaction. However, the exact exception type requires verification—based on error patterns elsewhere in the test file (which useDBConnection.ConnectionErrorfor transaction-related errors), it may not beEcto.InvalidChangesetErroras originally suggested.
There was a problem hiding this 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 (3)
lib/ecto_libsql.ex (1)
220-232: Consider clearing staletrx_idwhen transaction is no longer active.The logic correctly distinguishes idle vs active transaction states. However, when
handle_status_transactionreturns{:error, _}, the state still contains the staletrx_idwhile reporting:idle. This could lead to confusion if subsequent code inspects the state directly.Consider returning the state with
trx_idcleared:🔎 Suggested refinement
trx_id when is_binary(trx_id) -> # Check if transaction is still active case EctoLibSql.Native.handle_status_transaction(trx_id) do :ok -> {:transaction, state} - {:error, _message} -> {:idle, state} + {:error, _message} -> {:idle, %{state | trx_id: nil}} endThis is a minor concern since
handle_statusis primarily informational and subsequent operations will fail appropriately at the native layer.test/prepared_statement_test.exs (1)
354-431: Performance test correctly tagged as:flakyfor CI stability.The test serves its purpose as API documentation and provides performance visibility via console output. The
assert trueat line 430 ensures CI stability while the:flakytag allows selective exclusion.Consider adding a comment near the
assert trueexplaining it's intentional:# Success! Prepared statement API is working, which is the main goal of this test - assert true + # Always pass - performance validation is informational, not a gate + assert true, "Prepared statement API is functional"test/ecto_sql_transaction_compat_test.exs (1)
91-98: Consider extracting the cleanup delay to a constant.The
Process.sleep(50)magic number works but could benefit from documentation or extraction:🔎 Suggested refinement
+ # Time to allow for SQLite file handles to be released after repo shutdown + @cleanup_delay_ms 50 + setup do # ... end # Wait a bit for cleanup - Process.sleep(50) + Process.sleep(@cleanup_delay_ms)This is a minor nitpick - the current implementation is functional.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/ecto_libsql.exnative/ecto_libsql/src/constants.rsnative/ecto_libsql/src/utils.rstest/ecto_sql_transaction_compat_test.exstest/ecto_stream_compat_test.exstest/prepared_statement_test.exs
🚧 Files skipped from review as they are similar to previous changes (1)
- test/ecto_stream_compat_test.exs
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS use British/Australian English spelling and grammar for code, comments, and documentation, except where required for function calls, SQL keywords, error messages, or compatibility with external systems that may use US English
Files:
native/ecto_libsql/src/constants.rsnative/ecto_libsql/src/utils.rstest/ecto_sql_transaction_compat_test.exslib/ecto_libsql.extest/prepared_statement_test.exs
native/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Rust Cargo formatter (
cargo fmt) before committing changes and fix any issues
Files:
native/ecto_libsql/src/constants.rsnative/ecto_libsql/src/utils.rs
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Elixir formatter (
mix format --check-formatted) before committing changes and fix any issues
Files:
test/ecto_sql_transaction_compat_test.exslib/ecto_libsql.extest/prepared_statement_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.exs: Add tests for all new features in appropriate Elixir test files (test/ecto_*_test.exs) and Rust test modules (native/ecto_libsql/src/tests/), covering happy path, error cases, edge cases, and type conversions
Add comprehensive tests for unsupported functions asserting that they always return {:error, :unsupported} and that they don't modify database state or corrupt connections
Files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
lib/**/*.ex
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.ex: When updating Elixir code, follow the pattern of case matching on NIF results:{:ok, _, result, new_state}for successful query execution or{:error, reason}for failures
In Elixir error handling, preferwithclauses for chaining multiple operations that return Result tuples, providing clear error handling with anelseclause
In Ecto changesets, use thecastfunction to automatically attempt type conversions for fields, which will raise ChangeError if types don't match schema expectations
Use transactions with appropriate isolation behaviours when executing database operations::deferredfor mixed workloads,:immediatefor write-heavy operations,:exclusivefor critical operations requiring exclusive locks,:read_onlyfor read-only queries
When working with database locks, use transactions with proper timeout configuration, ensure connections are properly closed in try-after blocks, and use immediate/exclusive transaction behaviours for write-heavy workloads to minimise lock contention
When handling vector search operations, verify LibSQL version supports vectors, use correct vector syntax withEctoLibSql.Native.vector()function, insert vectors withvector(?)SQL syntax, and query with vector distance functions likevector_distance_cos
Files:
lib/ecto_libsql.ex
**/*.ex
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ex: Use prepared statements viaEctoLibSql.Native.prepare/2andEctoLibSql.Native.query_stmt/3for repeated queries to improve performance by ~10-15x
Use batch operations withEctoLibSql.Native.batch_transactional/2for bulk inserts/updates instead of individual statements
Implement proper error handling by pattern matching on{:ok, ...}and{:error, ...}tuples from all EctoLibSql operations
Use transactions withEctoLibSql.handle_begin/2,EctoLibSql.handle_commit/2, andEctoLibSql.handle_rollback/2for multi-step database operations
Use cursor streaming withDBConnection.stream/3for memory-efficient processing of large result sets
Use vector search withEctoLibSql.Native.vector/1,EctoLibSql.Native.vector_type/2, andEctoLibSql.Native.vector_distance_cos/2for AI/ML features
UseEctoLibSql.Pragma.*functions to configure SQLite parameters such as foreign keys, journal mode, cache size, and synchronous level
Use savepoints withEctoLibSql.Native.create_savepoint/2,EctoLibSql.Native.release_savepoint_by_name/2, andEctoLibSql.Native.rollback_to_savepoint_by_name/2for nested transactions
Use Ecto changesets for data validation before inserting or updating records
Use Ecto query composition withEcto.Queryfor building complex queries incrementally
Preload associations withRepo.preload/2to avoid N+1 query problems in Ecto applications
For remote replicas in production, useEctoLibSql.Native.sync/1,EctoLibSql.Native.get_frame_number_for_replica/1,EctoLibSql.Native.sync_until_frame/2, andEctoLibSql.Native.flush_and_get_frame/1for replication management
Use busy timeout configuration withEctoLibSql.Native.busy_timeout/2to handle database locking conflicts gracefully
Use statement introspection functions (EctoLibSql.Native.stmt_parameter_count/2,EctoLibSql.Native.stmt_column_count/2,EctoLibSql.Native.stmt_column_name/3) to validate prepared statement structure
Handle connection errors gracefully by ch...
Files:
lib/ecto_libsql.ex
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to test/**/*.exs : Add comprehensive tests for unsupported functions asserting that they always return {:error, :unsupported} and that they don't modify database state or corrupt connections
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to test/**/*.exs : Add tests for all new features in appropriate Elixir test files (test/ecto_*_test.exs) and Rust test modules (native/ecto_libsql/src/tests/), covering happy path, error cases, edge cases, and type conversions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto/adapters/libsql.ex : When implementing Ecto storage operations (create, drop, status) in lib/ecto/adapters/libsql.ex, ensure they properly handle both local SQLite and remote Turso databases via the connection mode in EctoLibSql.State
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use prepared statements via `EctoLibSql.Native.prepare/2` and `EctoLibSql.Native.query_stmt/3` for repeated queries to improve performance by ~10-15x
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto_libsql/native.ex : For each new Rust NIF function, add corresponding Elixir NIF stubs and safe wrapper functions in lib/ecto_libsql/native.ex, with proper state management via EctoLibSql.State
Applied to files:
native/ecto_libsql/src/constants.rsnative/ecto_libsql/src/utils.rslib/ecto_libsql.extest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : In Rust, all non-test code must have no `.unwrap()` calls; this was a critical refactoring in v0.5.0 to eliminate 146 unwrap calls and prevent BEAM VM crashes from panics in native code
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : Include comprehensive Rust documentation comments (/// comments) on all public NIF functions explaining purpose, arguments, return values (success and error cases), and usage examples
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto_libsql/native.ex : For explicitly unsupported functions, update the Elixir wrapper with comprehensive documentation explaining why the feature is unsupported, what architectural constraints prevent it, and what alternative approaches users can take
Applied to files:
native/ecto_libsql/src/utils.rstest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto/adapters/libsql.ex : In type loaders/dumpers in lib/ecto/adapters/libsql.ex, ensure bidirectional conversion: loaders convert SQLite values to Elixir types on retrieval, dumpers convert Elixir types to SQLite on storage
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : In Rust NIF signatures, convert async/await Result<T, E> errors to String via `.map_err(|e| format!("{:?}", e))` before returning from async blocks within `block_on`
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : In Rust production NIFs, use `ok_or_else` to convert Option types to Result, providing clear error messages via Term boxing: `.ok_or_else(|| rustler::Error::Term(Box::new("error message")))`
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to test/**/*.exs : Add comprehensive tests for unsupported functions asserting that they always return {:error, :unsupported} and that they don't modify database state or corrupt connections
Applied to files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use transactions with `EctoLibSql.handle_begin/2`, `EctoLibSql.handle_commit/2`, and `EctoLibSql.handle_rollback/2` for multi-step database operations
Applied to files:
test/ecto_sql_transaction_compat_test.exslib/ecto_libsql.ex
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to test/**/*.exs : Add tests for all new features in appropriate Elixir test files (test/ecto_*_test.exs) and Rust test modules (native/ecto_libsql/src/tests/), covering happy path, error cases, edge cases, and type conversions
Applied to files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use savepoints with `EctoLibSql.Native.create_savepoint/2`, `EctoLibSql.Native.release_savepoint_by_name/2`, and `EctoLibSql.Native.rollback_to_savepoint_by_name/2` for nested transactions
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/**/*.ex : When working with database locks, use transactions with proper timeout configuration, ensure connections are properly closed in try-after blocks, and use immediate/exclusive transaction behaviours for write-heavy workloads to minimise lock contention
Applied to files:
test/ecto_sql_transaction_compat_test.exslib/ecto_libsql.ex
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/**/*.ex : Use transactions with appropriate isolation behaviours when executing database operations: `:deferred` for mixed workloads, `:immediate` for write-heavy operations, `:exclusive` for critical operations requiring exclusive locks, `:read_only` for read-only queries
Applied to files:
test/ecto_sql_transaction_compat_test.exslib/ecto_libsql.ex
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to test/**/*migration*test.exs : For SQLite migrations that require column type changes or drops (which SQLite doesn't support via ALTER), use the table recreation pattern: create new table, copy data with transformations, swap tables, and recreate indexes
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Implement proper error handling by pattern matching on `{:ok, ...}` and `{:error, ...}` tuples from all EctoLibSql operations
Applied to files:
test/ecto_sql_transaction_compat_test.exslib/ecto_libsql.ex
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto/adapters/libsql.ex : When implementing Ecto storage operations (create, drop, status) in lib/ecto/adapters/libsql.ex, ensure they properly handle both local SQLite and remote Turso databases via the connection mode in EctoLibSql.State
Applied to files:
test/ecto_sql_transaction_compat_test.exslib/ecto_libsql.extest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use batch operations with `EctoLibSql.Native.batch_transactional/2` for bulk inserts/updates instead of individual statements
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: When debugging failing tests, run with trace (`mix test file.exs --trace`), verify NIF compilation (`File.exists?("priv/native/ecto_libsql.so")`), check Rust output with `cargo test -- --nocapture`, and run Rust tests independently to isolate issues
Applied to files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/**/*.ex : When updating Elixir code, follow the pattern of case matching on NIF results: `{:ok, _, result, new_state}` for successful query execution or `{:error, reason}` for failures
Applied to files:
lib/ecto_libsql.ex
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : In lib/ecto/adapters/libsql/connection.ex, implement SQL expression handlers for SQLite functions by pattern matching on function names and converting them to SQL fragments, respecting SQLite syntax and limitations
Applied to files:
lib/ecto_libsql.extest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use busy timeout configuration with `EctoLibSql.Native.busy_timeout/2` to handle database locking conflicts gracefully
Applied to files:
lib/ecto_libsql.ex
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use prepared statements via `EctoLibSql.Native.prepare/2` and `EctoLibSql.Native.query_stmt/3` for repeated queries to improve performance by ~10-15x
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use statement introspection functions (`EctoLibSql.Native.stmt_parameter_count/2`, `EctoLibSql.Native.stmt_column_count/2`, `EctoLibSql.Native.stmt_column_name/3`) to validate prepared statement structure
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : When adding a new NIF function in Rust, place it in the appropriate module based on functionality: connection.rs (lifecycle), query.rs (execution), transaction.rs (transactions), batch.rs (batch operations), statement.rs (prepared statements), cursor.rs (streaming), replication.rs (replica sync), metadata.rs (metadata access), or savepoint.rs (nested transactions)
Applied to files:
test/prepared_statement_test.exs
🧬 Code graph analysis (5)
native/ecto_libsql/src/constants.rs (1)
lib/ecto/adapters/libsql.ex (1)
def autogenerate(:id), do: nil(91-91)
native/ecto_libsql/src/utils.rs (1)
native/ecto_libsql/src/cursor.rs (1)
val(309-324)
test/ecto_sql_transaction_compat_test.exs (3)
native/ecto_libsql/src/transaction.rs (1)
transaction(100-111)lib/ecto_libsql/native.ex (1)
rollback(441-445)lib/ecto_libsql.ex (1)
checkout(248-253)
lib/ecto_libsql.ex (1)
native/ecto_libsql/src/transaction.rs (1)
handle_status_transaction(398-406)
test/prepared_statement_test.exs (2)
lib/ecto_libsql/native.ex (3)
prepare(478-486)query_stmt(530-549)close_stmt(562-564)lib/ecto_libsql.ex (1)
handle_execute(134-151)
⏰ 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 (7)
native/ecto_libsql/src/constants.rs (1)
77-78: LGTM!The
blobandnilatom declarations correctly extend the NIF interface to support blob tuple handling and nil/NULL decoding. This aligns with the updateddecode_term_to_valuelogic inutils.rs.native/ecto_libsql/src/utils.rs (1)
396-407: LGTM!The nil handling is correctly implemented:
- Early check for nil atom before numeric/string decodings ensures NULL values are mapped to
Value::Nullimmediately.- The function-local import of
{blob, nil}fromcrate::constantsis appropriate for consistency with the blob handling pattern.- Boolean atoms (
true/false) will still be handled correctly at line 413 sincedecodeattempts are non-destructive.test/prepared_statement_test.exs (1)
311-352: LGTM!Excellent test coverage for prepared statement binding behaviour:
- The auto-reset test (lines 312-332) correctly verifies that parameter bindings don't leak between executions.
- The type flexibility test (lines 334-352) validates SQLite's dynamic typing with reused statements.
- Both tests include proper cleanup with
Native.close_stmt/1.Based on learnings, these tests align with the requirement to cover "happy path, error cases, edge cases, and type conversions" for new features.
test/ecto_sql_transaction_compat_test.exs (4)
1-39: LGTM!Well-structured test module with proper isolation:
- Per-test unique database files prevent cross-test contamination.
async: falsecorrectly prevents concurrent access to the sharedTestReponame.- The
UniqueErrorexception cleanly simulates constraint violations.- British English spelling ("behaviour") used consistently as per coding guidelines.
159-183: Excellent documentation of SQLite-specific nested transaction behaviour.The comment on lines 175-176 clearly documents that SQLite's savepoint behaviour differs from PostgreSQL - the outer transaction becomes poisoned after a nested rollback. This is valuable for users porting from PostgreSQL-based systems.
The test correctly verifies this behaviour by asserting both:
- The
DBConnection.ConnectionErroris raised on subsequent operations (line 177)- The entire transaction returns
{:error, :rollback}(line 180)- No data is persisted (line 182)
216-271: Appropriately tagged tests for SQLite concurrency limitations.Keeping these tests with
:skipand:sqlite_concurrency_limitationtags is the right approach - it documents the behavioural differences without causing false failures. These could potentially be re-enabled if future LibSQL/SQLite versions improve cross-process transaction isolation with WAL mode.
273-354: LGTM!Comprehensive coverage of LibSQL-specific transaction modes:
:deferred- lock acquired on first write:immediate- reserved lock acquired immediately:exclusive- blocks all other connections:read_only- no locks, read-only accessThis aligns with the coding guidelines: "Use transactions with appropriate isolation behaviours when executing database operations."
There was a problem hiding this 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)
test/prepared_statement_test.exs (2)
361-369: Consider using Native.query directly instead of handle_execute for test setup.Lines 361-369 use
EctoLibSql.handle_executefor bulk inserts in the performance test setup. For consistency with the rest of the test file (which usesexec_sqlhelper or directNativecalls), consider using the same pattern here.🔎 Proposed refactor for consistency
# Setup: Create 100 test users Enum.each(1..100, fn i -> - {:ok, _, _, _state} = - EctoLibSql.handle_execute( - "INSERT INTO users (id, name, email) VALUES (?, ?, ?)", - [i, "User#{i}", "user#{i}@example.com"], - [], - state - ) + {:ok, _query, _result, _} = + exec_sql(state, "INSERT INTO users (id, name, email) VALUES (?, ?, ?)", [ + i, + "User#{i}", + "user#{i}@example.com" + ]) end)
440-448: Consider using exec_sql helper for consistency in memory test.Similar to the performance test, the memory efficiency test at lines 440-448 uses
EctoLibSql.handle_executefor bulk inserts while the rest of the file usesexec_sqlor directNativecalls.🔎 Proposed refactor for consistency
# Insert 1000 test rows Enum.each(1..1000, fn i -> - {:ok, _, _, _state} = - EctoLibSql.handle_execute( - "INSERT INTO users (id, name, email) VALUES (?, ?, ?)", - [i, "User#{i}", "user#{i}@example.com"], - [], - state - ) + {:ok, _query, _result, _} = + exec_sql(state, "INSERT INTO users (id, name, email) VALUES (?, ?, ?)", [ + i, + "User#{i}", + "user#{i}@example.com" + ]) end)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/ecto_libsql.extest/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ecto_libsql.ex
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS use British/Australian English spelling and grammar for code, comments, and documentation, except where required for function calls, SQL keywords, error messages, or compatibility with external systems that may use US English
Files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Elixir formatter (
mix format --check-formatted) before committing changes and fix any issues
Files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.exs: Add tests for all new features in appropriate Elixir test files (test/ecto_*_test.exs) and Rust test modules (native/ecto_libsql/src/tests/), covering happy path, error cases, edge cases, and type conversions
Add comprehensive tests for unsupported functions asserting that they always return {:error, :unsupported} and that they don't modify database state or corrupt connections
Files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to test/**/*.exs : Add comprehensive tests for unsupported functions asserting that they always return {:error, :unsupported} and that they don't modify database state or corrupt connections
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to test/**/*.exs : Add tests for all new features in appropriate Elixir test files (test/ecto_*_test.exs) and Rust test modules (native/ecto_libsql/src/tests/), covering happy path, error cases, edge cases, and type conversions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto/adapters/libsql.ex : When implementing Ecto storage operations (create, drop, status) in lib/ecto/adapters/libsql.ex, ensure they properly handle both local SQLite and remote Turso databases via the connection mode in EctoLibSql.State
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use prepared statements via `EctoLibSql.Native.prepare/2` and `EctoLibSql.Native.query_stmt/3` for repeated queries to improve performance by ~10-15x
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use transactions with `EctoLibSql.handle_begin/2`, `EctoLibSql.handle_commit/2`, and `EctoLibSql.handle_rollback/2` for multi-step database operations
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to test/**/*.exs : Add comprehensive tests for unsupported functions asserting that they always return {:error, :unsupported} and that they don't modify database state or corrupt connections
Applied to files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to test/**/*.exs : Add tests for all new features in appropriate Elixir test files (test/ecto_*_test.exs) and Rust test modules (native/ecto_libsql/src/tests/), covering happy path, error cases, edge cases, and type conversions
Applied to files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use savepoints with `EctoLibSql.Native.create_savepoint/2`, `EctoLibSql.Native.release_savepoint_by_name/2`, and `EctoLibSql.Native.rollback_to_savepoint_by_name/2` for nested transactions
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/**/*.ex : Use transactions with appropriate isolation behaviours when executing database operations: `:deferred` for mixed workloads, `:immediate` for write-heavy operations, `:exclusive` for critical operations requiring exclusive locks, `:read_only` for read-only queries
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to test/**/*migration*test.exs : For SQLite migrations that require column type changes or drops (which SQLite doesn't support via ALTER), use the table recreation pattern: create new table, copy data with transformations, swap tables, and recreate indexes
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/**/*.ex : When working with database locks, use transactions with proper timeout configuration, ensure connections are properly closed in try-after blocks, and use immediate/exclusive transaction behaviours for write-heavy workloads to minimise lock contention
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto/adapters/libsql.ex : When implementing Ecto storage operations (create, drop, status) in lib/ecto/adapters/libsql.ex, ensure they properly handle both local SQLite and remote Turso databases via the connection mode in EctoLibSql.State
Applied to files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use batch operations with `EctoLibSql.Native.batch_transactional/2` for bulk inserts/updates instead of individual statements
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/repo.ex : Define custom Ecto repositories using `Ecto.Repo` with `Ecto.Adapters.LibSql` adapter for ecto_libsql projects
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Implement proper error handling by pattern matching on `{:ok, ...}` and `{:error, ...}` tuples from all EctoLibSql operations
Applied to files:
test/ecto_sql_transaction_compat_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: When debugging failing tests, run with trace (`mix test file.exs --trace`), verify NIF compilation (`File.exists?("priv/native/ecto_libsql.so")`), check Rust output with `cargo test -- --nocapture`, and run Rust tests independently to isolate issues
Applied to files:
test/ecto_sql_transaction_compat_test.exstest/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use prepared statements via `EctoLibSql.Native.prepare/2` and `EctoLibSql.Native.query_stmt/3` for repeated queries to improve performance by ~10-15x
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:46.019Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T11:58:46.019Z
Learning: Applies to **/*.ex : Use statement introspection functions (`EctoLibSql.Native.stmt_parameter_count/2`, `EctoLibSql.Native.stmt_column_count/2`, `EctoLibSql.Native.stmt_column_name/3`) to validate prepared statement structure
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto_libsql/native.ex : For each new Rust NIF function, add corresponding Elixir NIF stubs and safe wrapper functions in lib/ecto_libsql/native.ex, with proper state management via EctoLibSql.State
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto_libsql/native.ex : For explicitly unsupported functions, update the Elixir wrapper with comprehensive documentation explaining why the feature is unsupported, what architectural constraints prevent it, and what alternative approaches users can take
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : In lib/ecto/adapters/libsql/connection.ex, implement SQL expression handlers for SQLite functions by pattern matching on function names and converting them to SQL fragments, respecting SQLite syntax and limitations
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2025-12-17T11:58:26.477Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T11:58:26.477Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : When adding a new NIF function in Rust, place it in the appropriate module based on functionality: connection.rs (lifecycle), query.rs (execution), transaction.rs (transactions), batch.rs (batch operations), statement.rs (prepared statements), cursor.rs (streaming), replication.rs (replica sync), metadata.rs (metadata access), or savepoint.rs (nested transactions)
Applied to files:
test/prepared_statement_test.exs
🧬 Code graph analysis (1)
test/ecto_sql_transaction_compat_test.exs (3)
native/ecto_libsql/src/transaction.rs (1)
transaction(100-111)lib/ecto_libsql/native.ex (1)
rollback(441-445)lib/ecto_libsql.ex (1)
checkout(248-253)
⏰ 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: Analyze (rust)
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
🔇 Additional comments (10)
test/prepared_statement_test.exs (2)
311-463: LGTM! Excellent test coverage for prepared statement binding behaviour.The new test suite effectively validates:
- Binding reset between executions to prevent parameter leakage
- Multi-type parameter handling (integers, strings, mixed)
- Performance characteristics (with appropriate flaky tagging)
- Memory efficiency patterns
The performance test at lines 354-432 is particularly well-designed, with:
- Informational assertions rather than strict gates (avoiding CI variability)
- Clear logging of metrics for visibility
- Appropriate warnings for unexpected slowdowns
- Recognition that the test primarily validates API functionality
The British English spelling "behaviour" in the describe block title (line 311) correctly follows the coding guidelines.
371-383: Verify handle_execute is the intended function for unprepared benchmark.The unprepared benchmark at lines 371-383 uses
EctoLibSql.handle_execute, which appears to be testing a different code path than the directNative.query_stmtused in the prepared benchmark. This may introduce confounding variables in the performance comparison.If the intent is to compare the same query execution path with and without statement preparation, consider using
Native.query(viaexec_sql) for the unprepared benchmark to match the prepared path more closely. However, if testing the full Ecto adapter path vs prepared statements is intentional, the current approach is valid.Could you clarify whether the performance comparison should test:
- Same execution path (Native layer) with/without prepared statements, or
- Full Ecto adapter path vs prepared statement path?
test/ecto_sql_transaction_compat_test.exs (8)
1-104: Excellent test setup with proper isolation and cleanup.The test module demonstrates strong engineering practices:
- Unique database file per test to avoid locking issues
- Proper WAL mode and busy_timeout configuration
- Comprehensive on_exit cleanup with process lifecycle handling
- British English spelling ("behaviour" in line 106, 162)
The cleanup logic at lines 70-101 properly handles various exit scenarios and includes a delay for SQLite file handle release.
220-228: Verify rollback-outside-transaction behaviour is actually skipped.The test expects a RuntimeError with message "cannot call rollback outside of transaction", but this test is marked with
@skip. This suggests the current implementation may not enforce this constraint.Based on learnings, this should be tested once the constraint is implemented. Consider tracking this with an issue or TODO if the behaviour is planned for future implementation.
As per coding guidelines, ensure comprehensive test coverage for unsupported or not-yet-implemented functions.
230-273: Document the SQLite concurrency limitation preventing cross-process isolation tests.This test (lines 230-273) verifies transaction isolation across processes, which is a critical database guarantee. The
@skipand@sqlite_concurrency_limitationtags indicate this is not currently testable.Could you clarify:
- Is this limitation inherent to SQLite's file-based locking model?
- Does LibSQL (Turso) remote mode support proper cross-process isolation?
- Should these tests be enabled when using remote mode?
Consider adding a comment documenting the specific limitation and whether remote mode addresses it:
@tag :skip @tag :sqlite_concurrency_limitation # SQLite file-based locking prevents true cross-process isolation testing # with pool_size: 1. Consider testing with LibSQL remote mode or increasing pool_size. test "transactions are not shared across processes" do
163-186: Clarify nested transaction rollback "poison" behaviour for SQLite.The test at lines 163-186 includes an important comment at line 178: "After nested rollback, outer transaction is poisoned in SQLite. This behaviour differs from PostgreSQL."
This is valuable documentation of SQLite-specific semantics, but the assertion at line 180-182 expects a
DBConnection.ConnectionErrorwith message matchingtransaction rolling back.Please verify this assertion works correctly. If the "poisoned" state is SQLite-specific, ensure that:
- The error message is consistent across LibSQL local and remote modes
- The test accurately reflects the intended rollback propagation behaviour
The test correctly validates that the final state is empty (line 185), confirming the outer transaction rolled back.
277-357: Excellent coverage of LibSQL-specific transaction modes.The LibSQL transaction modes tests (lines 277-357) provide comprehensive coverage of DEFERRED, IMMEDIATE, EXCLUSIVE, and READ_ONLY modes with clear comments explaining the locking behaviour of each mode:
- DEFERRED: Lock acquired on first write (line 278)
- IMMEDIATE: Reserved lock acquired immediately (line 297)
- EXCLUSIVE: Exclusive lock blocks all other connections (line 313)
- READ_ONLY: No locks, read-only access (line 331)
The rollback test (lines 344-356) validates that transaction modes work correctly with manual rollback, which is essential for confirming proper cleanup.
Based on learnings, these tests align with the guidance to use appropriate isolation behaviours for different workloads.
359-383: LGTM! Checkout operations correctly validate transaction state.The checkout operations tests properly verify:
- Transaction inside checkout maintains correct
in_transaction?state- Checkout inside transaction preserves transaction context
These tests confirm that checkout operations (connection reservation) work correctly with the transaction lifecycle.
386-399: Good validation that transactions don't leak on query errors.The test at lines 386-399 verifies that failed queries within transactions properly clean up transaction state. The comment at line 389 correctly documents that EctoLibSql raises
EctoLibSql.Errorinstead ofEcto.QueryError.This aligns with the coding guidelines for comprehensive error handling tests.
419-467: Excellent complex transaction scenario coverage.The complex transaction tests (lines 419-467) validate:
- Multiple operations in a single transaction with update_all
- Mixed CRUD operations (query, insert, update, delete) in one transaction
These tests ensure the adapter handles realistic multi-operation workflows correctly, with proper assertions on intermediate and final states.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.