tests: Consolidate and simplify Elixir tests #56
Conversation
WalkthroughCentralises test DB cleanup via new helpers, removes several large legacy Elixir test modules, and adds many new Elixir and Rust test suites (prepared statements, concurrency, streaming, replication, error handling). Introduces Rust TestDbGuard and Elixir EctoLibSql.TestHelpers, plus minor docs/.gitignore/.beads edits. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @test/smoke_test.exs:
- Around line 76-80: The failing CI is due to formatting in the test block
around the EctoLibSql.connect/1 and EctoLibSql.handle_execute/4 assertions; run
mix format on test/smoke_test.exs (or run mix format for the repo) to apply the
expected formatting so the block containing {:ok, state} =
EctoLibSql.connect(state[:opts]), the Query struct construction
%EctoLibSql.Query{statement: "SELECT * FROM not_existing_table"}, and the assert
line are properly formatted to satisfy mix format --check-formatted.
- Around line 119-126: Run mix format on the test file to fix formatting issues:
format test/smoke_test.exs (or run mix format --check-formatted to see failures
and then mix format to apply fixes). Ensure the block around the
EctoLibSql.handle_execute and EctoLibSql.handle_commit calls follows standard
Elixir formatting (correct indentation, spacing, and line breaks) so that the
test compiles and passes mix format checks.
🧹 Nitpick comments (2)
test/smoke_test.exs (1)
104-126: Consider adding a transaction rollback test.Whilst the smoke tests are intentionally minimal, adding a basic transaction rollback scenario would strengthen confidence in core functionality. This would help catch issues early before running the more comprehensive test suites.
Based on learnings and coding guidelines, tests should cover transaction rollback scenarios.
Example rollback test
Add this test to the "basic transaction" describe block:
test "can rollback transaction", state do {:ok, state} = EctoLibSql.connect(state[:opts]) # Create table first create = %EctoLibSql.Query{ statement: "CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, email TEXT)" } {:ok, _query, _result, state} = EctoLibSql.handle_execute(create, [], [], state) # Begin transaction {:ok, _begin_result, state} = EctoLibSql.handle_begin([], state) # Insert data insert = %EctoLibSql.Query{statement: "INSERT INTO users (name, email) VALUES (?, ?)"} {:ok, _query, _result, state} = EctoLibSql.handle_execute(insert, ["Bob", "bob@example.com"], [], state) # Rollback instead of commit {:ok, _rollback_result, state} = EctoLibSql.handle_rollback([], state) # Verify data was not persisted select = %EctoLibSql.Query{statement: "SELECT COUNT(*) FROM users"} {:ok, _query, result, _state} = EctoLibSql.handle_execute(select, [], [], state) assert [[0]] = result.rows endtest/prepared_statement_test.exs (1)
1-739: Consider adding concurrent prepared statement tests.Whilst the current test coverage is excellent, adding tests for concurrent usage of prepared statements would strengthen confidence in thread-safety and statement isolation. This could include scenarios where multiple processes use the same or different prepared statements simultaneously.
Based on coding guidelines, tests should cover concurrent operations where applicable.
Example concurrent test structure
@tag :concurrent test "concurrent prepared statement executions", %{state: state} do # Insert test data {:ok, _query, _result, state} = exec_sql(state, "INSERT INTO users (id, name, email) VALUES (?, ?, ?)", [ 1, "Alice", "alice@example.com" ]) {:ok, _query, _result, state} = exec_sql(state, "INSERT INTO users (id, name, email) VALUES (?, ?, ?)", [ 2, "Bob", "bob@example.com" ]) {:ok, stmt_id} = Native.prepare(state, "SELECT * FROM users WHERE id = ?") # Execute concurrently from multiple tasks tasks = Enum.map(1..10, fn _ -> Task.async(fn -> {:ok, result1} = Native.query_stmt(state, stmt_id, [1]) {:ok, result2} = Native.query_stmt(state, stmt_id, [2]) {result1, result2} end) end) # All tasks should complete successfully results = Enum.map(tasks, &Task.await/1) assert length(results) == 10 Native.close_stmt(stmt_id) end
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.beads/last-touchedtest/ecto_libsql_test.exstest/error_demo_test.exstest/explain_simple_test.exstest/prepared_statement_test.exstest/smoke_test.exstest/statement_features_test.exstest/stmt_caching_performance_test.exs
💤 Files with no reviewable changes (4)
- test/error_demo_test.exs
- test/statement_features_test.exs
- test/ecto_libsql_test.exs
- test/explain_simple_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/prepared_statement_test.exstest/stmt_caching_performance_test.exstest/smoke_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/prepared_statement_test.exstest/stmt_caching_performance_test.exstest/smoke_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/prepared_statement_test.exstest/stmt_caching_performance_test.exstest/smoke_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/prepared_statement_test.exstest/stmt_caching_performance_test.exstest/smoke_test.exs
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use prepared statements with automatic statement caching for repeated queries to achieve ~10-15x performance improvement
Applied to files:
test/prepared_statement_test.exstest/stmt_caching_performance_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/prepared_statement_test.exstest/smoke_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/prepared_statement_test.exstest/stmt_caching_performance_test.exstest/smoke_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
test/prepared_statement_test.exstest/smoke_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/smoke_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/smoke_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Use the Repository pattern for data access in EctoLibSql applications
Applied to files:
test/smoke_test.exs
🧬 Code graph analysis (2)
test/prepared_statement_test.exs (3)
test/stmt_caching_performance_test.exs (1)
exec_sql(15-18)test/savepoint_test.exs (1)
exec_sql(14-17)lib/ecto_libsql/native.ex (3)
prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)
test/smoke_test.exs (1)
lib/ecto_libsql.ex (3)
disconnect(131-133)handle_execute(148-188)handle_begin(312-317)
🪛 GitHub Actions: CI
test/smoke_test.exs
[error] 76-80: Mix format --check-formatted failed. The following regions in test/smoke_test.exs are not properly formatted.
[error] 119-126: Mix format --check-formatted failed. The following regions in test/smoke_test.exs are not properly formatted.
⏰ 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 (5)
.beads/last-touched (1)
1-1: Metadata update looks good.The metadata tracking update is straightforward with no concerns.
test/stmt_caching_performance_test.exs (1)
1-8: LGTM! Clear module rename and documentation.The module rename from
BenchmarkTesttoPerformanceTestbetter reflects the test's purpose, and the added documentation clearly explains that these tests verify performance characteristics without being strict gates. The documentation follows British/Australian English conventions as required.test/prepared_statement_test.exs (3)
1-11: Excellent test module structure and documentation.The module is well-organised with clear documentation linking to Phase 1 roadmap features. The comprehensive test coverage across preparation, execution, introspection, lifecycle, and performance aspects demonstrates thorough validation of prepared statement functionality.
313-366: Well-implemented automatic reset tests.These tests properly verify that statement caching handles reset automatically between executions, ensuring parameter bindings don't leak across calls. The verification of row counts confirms correctness.
588-738: Comprehensive binding behaviour tests with appropriate performance validation.The tests ported from ecto_sql provide excellent coverage of binding behaviour, parameter type handling, and performance characteristics. The performance test is appropriately tagged as
:flakyand includes informative output whilst always passing, which is the correct approach for CI environments where performance can vary significantly.The explicit notes about CI variability and the focus on API functionality rather than strict performance gates show good engineering judgement.
- Adds comprehensive rollback scenario alongside existing commit test - Verifies rollback correctly undoes in-transaction changes - Confirms pre-transaction data is preserved - Strengthens confidence in core transaction functionality - All 8 smoke tests pass
Adds 5 comprehensive concurrent tests to verify thread-safety and statement isolation: 1. Multiple processes using different prepared statements concurrently - Verifies independent statements don't interfere with each other - Tests both ID-based and name-based lookups in parallel 2. Single prepared statement safely used by multiple processes - Confirms statement reuse across concurrent tasks is safe - Validates parameter isolation and correct results - Tests with Process.sleep to simulate real-world timing 3. Concurrent writes with prepared statements maintain consistency - Verifies INSERT operations through prepared statements are atomic - Confirms final count matches expected value (1 initial + 5 new = 6) - Tests concurrent write scenarios 4. Prepared statements handle parameter isolation across concurrent tasks - Verifies parameters from one task don't leak to another - Tests custom parameter values with detailed assertions - Validates both parameter AND data isolation 5. Prepared statements maintain isolation when reset concurrently - Tests explicit reset_stmt() during concurrent access - Verifies statement can be reset while other tasks use it - Confirms isolation is maintained after reset All 44 tests pass including these new concurrent scenarios.
ocean
force-pushed
the
consolidate-tests
branch
from
e57c504 to
cd0209f
Compare
ocean
force-pushed
the
consolidate-tests
branch
from
c8bd634 to
737517d
Compare
…REPORT Created 9 new issues based on TEST_AUDIT_REPORT.md recommendations: **P2 (Medium priority - should do soon)**: - el-doo: Test cursor streaming with large result sets - el-fd8: Test connection pool behavior under load - el-d63: Test connection error recovery - el-crt: Test savepoint + replication interaction - el-wtl: Test JSONB binary format operations - el-d3o: Add Rust tests for error scenarios **P3 (Low priority - nice to have)**: - el-cbv: Add performance benchmark tests - el-1p2: Document test layering strategy - el-v3v: Reduce redundant parameter binding tests These issues capture all missing test coverage areas identified in TEST_AUDIT_REPORT.md
ocean
force-pushed
the
consolidate-tests
branch
from
56e148a to
5b6afe8
Compare
… load - test/savepoint_replication_test.exs: 5 tests for savepoint+replication interaction * Basic savepoint operations with replica sync enabled * Savepoint rollback with remote sync preservation * Nested savepoints with remote sync * Error recovery patterns with savepoints * Constraint violation handling - test/pool_load_test.exs: 8 tests for concurrent connection behavior * Multiple concurrent independent connections * Rapid burst handling * Long-running transaction robustness * Error recovery and isolation under load * Resource cleanup (prepared statements) * Transaction isolation guarantees Both test suites designed to test critical integration points without over-testing. Tests are isolated, skippable (replication tests), and pass 100% (13/13 tests). Closes: el-crt, el-fd8
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @test/savepoint_replication_test.exs:
- Around line 85-92: Run mix format to fix formatting in
test/savepoint_replication_test.exs; specifically format the block that calls
EctoLibSql.handle_execute and the subsequent assertion (the code using "SELECT
COUNT(*) FROM #{table}", the variables state/result, and the assert [[1]] =
result.rows) so it adheres to project formatting rules (e.g., run `mix format`
or apply your editor’s formatter to that file).
🧹 Nitpick comments (1)
test/savepoint_replication_test.exs (1)
1-19: Minor spelling issue: Use British/Australian EnglishPer the coding guidelines, British/Australian English should be used in documentation and comments. Line 7 uses "behavior" which should be "behaviour".
Suggested fix
Focused on critical integration scenarios: 1. Savepoints work correctly in replica mode with sync enabled - 2. Savepoint rollback doesn't interfere with remote sync + 2. Savepoint rollback doesn't interfere with remote synchronisation 3. Error recovery with savepoints in replicated transactions
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.beads/last-touched.gitignoreCLAUDE.mdtest/pool_load_test.exstest/prepared_statement_test.exstest/savepoint_replication_test.exstest/smoke_test.exs
🚧 Files skipped from review as they are similar to previous changes (1)
- .beads/last-touched
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/prepared_statement_test.exstest/pool_load_test.exstest/savepoint_replication_test.exstest/smoke_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/prepared_statement_test.exstest/pool_load_test.exstest/savepoint_replication_test.exstest/smoke_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/prepared_statement_test.exstest/pool_load_test.exstest/savepoint_replication_test.exstest/smoke_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/prepared_statement_test.exstest/pool_load_test.exstest/savepoint_replication_test.exsCLAUDE.mdtest/smoke_test.exs
**/*.{md,markdown}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British/Australian English for all documentation files (except SQL keywords and compatibility requirements)
Files:
CLAUDE.md
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use prepared statements with automatic statement caching for repeated queries to achieve ~10-15x performance improvement
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/prepared_statement_test.exstest/pool_load_test.exstest/savepoint_replication_test.exstest/smoke_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/prepared_statement_test.exstest/pool_load_test.exstest/savepoint_replication_test.exstest/smoke_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
test/prepared_statement_test.exsCLAUDE.mdtest/smoke_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/prepared_statement_test.exstest/pool_load_test.exstest/savepoint_replication_test.exstest/smoke_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/pool_load_test.exstest/smoke_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/turso_remote_test.exs : Turso remote tests should be skipped by default if credentials are missing and only run when explicitly needed due to cost implications
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to AGENTS.md : Update AGENTS.md API reference when adding new public NIF functions or Ecto features that users should know about
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to README.md : Update README.md when adding new features or changing user-facing functionality
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use Beads (`bd` command) for multi-session issue tracking with persistence across sessions and for work with dependencies between tasks
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : Always format Rust code before committing using `cargo fmt`
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs,rs,md} : When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with `git commit`, clean up stashes, and provide context for next session
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Always format Elixir code before committing using `mix format --check-formatted`
Applied to files:
test/smoke_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Use the Repository pattern for data access in EctoLibSql applications
Applied to files:
test/smoke_test.exs
🧬 Code graph analysis (3)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (5)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)
test/savepoint_replication_test.exs (2)
lib/ecto_libsql.ex (1)
handle_execute(148-188)lib/ecto_libsql/native.ex (5)
begin(828-838)create_savepoint(1805-1807)release_savepoint_by_name(1836-1838)commit(853-857)rollback_to_savepoint_by_name(1877-1879)
test/smoke_test.exs (1)
lib/ecto_libsql.ex (4)
disconnect(131-133)handle_execute(148-188)handle_begin(312-317)handle_rollback(350-358)
🪛 GitHub Actions: CI
test/savepoint_replication_test.exs
[error] Mix format check failed. The following files are not formatted.
⏰ 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 (28)
.gitignore (1)
41-43: LGTM!Appropriate addition to ignore local beads viewer configuration and caches. This aligns with the project's use of Beads for issue tracking as documented in CLAUDE.md.
test/smoke_test.exs (5)
1-14: LGTM!Well-structured smoke test module with clear documentation referencing specialised test files for comprehensive coverage. The use of
doctest EctoLibSqlensures documentation examples stay valid.
15-50: LGTM!Solid test isolation strategy with unique database files per test and thorough cleanup in both
setupandon_exitcallbacks. The cleanup of WAL and SHM files alongside the main database file is a good practice for SQLite.
52-66: LGTM!Basic connectivity tests cover the essential lifecycle operations (connect, ping, disconnect) with proper pattern matching.
68-104: LGTM!Good coverage of query scenarios including error handling for invalid SQL. The multi-statement test (lines 83-103) verifies compound statement execution which is important for SQLite compatibility.
106-171: LGTM!Excellent transaction test coverage with proper verification of rollback semantics. The rollback test (lines 131-170) correctly validates that:
- Pre-transaction data (Bob) persists
- Transaction data (Charlie) is rolled back
- Final count confirms isolation
This aligns with the coding guidelines for transaction rollback scenarios.
test/savepoint_replication_test.exs (3)
21-62: LGTM!Good setup with proper test isolation via unique database names and conditional Turso connection based on environment variables. The try/rescue in
on_exitis appropriate for handling cleanup of potentially already-closed connections.
64-183: LGTM!Comprehensive savepoint tests covering:
- Basic savepoint operations with replica sync (lines 65-93)
- Rollback preserving outer transaction data (lines 95-132)
- Nested savepoints with partial rollback (lines 134-183)
The tests properly verify data integrity after each operation.
203-214: The test correctly uses the state from the error tuple for savepoint recovery.The
{:error, _reason, trx_state}tuple at line 211 contains the original, unmodified transaction state. Error conditions do not corrupt the state; it remains safe to use for subsequent operations including savepoint rollback. The Rust layer validates transaction ownership (checkingentry.conn_id == conn_id) before executingrollback_to_savepoint_by_name, and the transaction is automatically re-inserted after the operation. This test correctly implements the error recovery pattern using savepoints as documented.CLAUDE.md (3)
49-49: LGTM!Good addition of the Table of Contents entry for easy navigation to the Beads section.
262-262: LGTM!Minor punctuation improvement for readability.
298-304: LGTM!Useful best practices section with actionable guidance for session workflow. The bullet points provide clear, concise instructions for using Beads effectively.
test/pool_load_test.exs (7)
1-18: LGTM!Well-documented test module covering critical concurrent connection scenarios. The moduledoc clearly outlines the test objectives.
19-41: LGTM!Good test isolation with unique database per test run. Each test creates its own connections, and cleanup handles the setup connection appropriately.
43-109: LGTM!Good concurrent connection tests verifying that multiple independent connections can successfully execute operations without interference. The verification of final row counts (lines 80, 107) ensures data integrity.
111-177: LGTM!Good tests for long-running operations with appropriate use of
busy_timeoutoption and verification that concurrent transactions complete successfully. TheProcess.sleepcalls simulate realistic workloads.
179-267: LGTM!Excellent error recovery tests demonstrating connection resilience. The tests verify that:
- A connection recovers after a query error (lines 179-216)
- Multiple connections recover independently (lines 218-267)
This matches the coding guidelines requirement for error case coverage.
270-308: LGTM!Good test for prepared statement lifecycle under concurrent load. Properly exercises prepare, execute, and close operations across multiple concurrent tasks.
311-355: LGTM!Solid transaction isolation test with concurrent transactions and slight delays to increase operation overlap. Verification of final count confirms all transactions committed successfully without interference.
test/prepared_statement_test.exs (9)
313-339: LGTM!Good test for statement reuse with automatic binding reset. The test properly verifies that prepared statements can be executed multiple times without re-preparation, and that all insertions succeed.
341-425: LGTM!Comprehensive reset tests covering:
- Automatic binding reset between executions (lines 341-366)
- Explicit
reset_stmtusage (lines 368-396)- Multiple consecutive resets (lines 398-419)
- Error handling for invalid statements (lines 421-424)
Good coverage of both happy path and error cases as per coding guidelines.
427-507: LGTM!Excellent column metadata tests covering:
- Basic column retrieval with origin names and declared types
- Aliased columns preserving alias names
- Expression columns (COUNT, MAX)
- Error handling for invalid statements
509-586: LGTM!Comprehensive parameter introspection tests covering all SQLite parameter styles:
- Colon-style (
:name)- Dollar-style (
$name)- At-style (
@name)- Positional (
?)- Mixed positional and named
This is valuable for ensuring the adapter correctly handles various SQL authoring patterns.
740-784: LGTM!Good concurrent access test verifying that multiple processes can safely use different prepared statements simultaneously. The verification that both queries return identical data (line 771) is a nice touch.
786-830: LGTM!Important test validating that a single prepared statement can be safely shared across multiple processes. The verification of data integrity after concurrent access (lines 825-826) ensures the statement remains in a consistent state.
832-874: LGTM!Good test for concurrent write operations using prepared statements. Verifies that multiple concurrent INSERT operations maintain data consistency and the final count is correct.
876-916: LGTM!Critical test for parameter isolation across concurrent tasks. The assertion at lines 901-902 explicitly verifies no parameter contamination occurs between concurrent executions.
918-972: LGTM!Excellent edge case test for concurrent reset operations. Testing
reset_stmtduring concurrent access is important for validating thread safety of the reset functionality.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/json_helpers_test.exs (2)
5-25: Addon_exitdisconnect to avoid leaking connections across a large suite.
This suite is big enough that leaving connections to GC can become flaky.Proposed fix
setup do {:ok, state} = EctoLibSql.connect(database: ":memory:") + on_exit(fn -> EctoLibSql.disconnect([], state) end) # Create a test table with JSON columns {:ok, _, _, state} = EctoLibSql.handle_execute( """ CREATE TABLE IF NOT EXISTS json_test ( id INTEGER PRIMARY KEY, data TEXT, data_jsonb BLOB, metadata TEXT ) """, [], [], state ) {:ok, state: state} end
288-305: Relax JSONB “must be smaller” assertions to avoid cross-version flakiness.
JSONB being smaller is common, but not a stable guarantee; strict</<=can fail depending on implementation details.Proposed fix
test "converts to JSONB binary format", %{state: state} do json = ~s({"a":1}) {:ok, result} = JSON.convert(state, json, :jsonb) # Should be binary assert is_binary(result) - # JSONB is smaller/different than text JSON - assert byte_size(result) < byte_size(json) + # JSONB is binary; size characteristics are implementation-dependent + assert byte_size(result) > 0 end- test "JSONB storage is 5-10% smaller than text JSON", %{state: state} do + test "JSONB storage is not pathologically larger than text JSON", %{state: state} do # Create a reasonably sized JSON object json_text = ~s({"user":{"id":1,"name":"Alice","email":"alice@example.com","profile":{"bio":"Software engineer","location":"San Francisco","interests":["Elixir","Rust","Go"]},"settings":{"theme":"dark","notifications":true,"language":"en"}}}) # Convert to JSONB {:ok, jsonb} = JSON.convert(state, json_text, :jsonb) text_size = byte_size(json_text) jsonb_size = byte_size(jsonb) - # JSONB should be smaller (5-10% is typical, but may vary) - # We check for general size improvement (not overly strict) - assert jsonb_size <= text_size, - "JSONB (#{jsonb_size} bytes) should be <= text JSON (#{text_size} bytes)" - - # Most of the time JSONB is noticeably smaller - # but we don't enforce a strict percentage due to variation + # Don’t assert “must be smaller”; just ensure it’s within a reasonable bound. + assert jsonb_size <= trunc(text_size * 1.5), + "JSONB (#{jsonb_size} bytes) unexpectedly large vs text JSON (#{text_size} bytes)" endAlso applies to: 786-805
🤖 Fix all issues with AI agents
In @native/ecto_libsql/src/tests/error_handling_tests.rs:
- Around line 485-496: The test test_create_db_invalid_permissions uses a
NUL-containing path ("\0invalid\0path.db") which is not portable to Windows;
guard the test with a platform cfg or use platform-specific invalid-path logic:
wrap the test function (test_create_db_invalid_permissions) with #[cfg(unix)] so
it only runs on Unix, or implement conditional behavior inside the test using
cfg!(windows) to choose a different invalid-path strategy, keeping the assertion
that Builder::new_local(invalid_path).build().await must not panic.
In @test/connection_recovery_test.exs:
- Around line 8-11: Add an on_exit hook in the setup block that calls the
disconnect function for the in-memory connection to avoid leaking resources:
after {:ok, state} = EctoLibSql.connect(database: ":memory:") register on_exit
and call EctoLibSql.disconnect(state) (or the appropriate teardown function for
EctoLibSql) so the connection is cleaned up when the test finishes, then return
{:ok, state: state}.
In @test/cursor_streaming_large_test.exs:
- Around line 33-110: Tests drop updated state from insert_rows/4 and call
helpers that use a hard-coded query instead of the declared cursor, causing
state and query/cursor mismatches; fix by threading the returned state through
every insert_rows/4 call and into subsequent calls to EctoLibSql.handle_declare
and your fetch helpers, and change fetch_all_rows and fetch_remaining_rows (and
any helper that calls EctoLibSql.handle_fetch) to accept and use the declared
Query/cursor returned by handle_declare rather than using a new hard-coded
Query; ensure every test captures the updated state (e.g., state =
insert_rows(state, ...)) and passes that same state and the declared
cursor/query into fetch_* helpers so handle_fetch/4 receives the exact cursor
and state that handle_declare produced.
In @test/pool_load_test.exs:
- Around line 43-109: The concurrency tests are flaky due to SQLite writer
contention and default Task await timeouts; update each concurrent test that
spawns Task.async to pass a busy_timeout when connecting (e.g., call
EctoLibSql.connect(database: test_db, busy_timeout: 30_000) or similar to the
connect API) and replace Task.await_many(tasks) with Task.await_many(tasks,
30_000) so tasks have a longer timeout; apply this pattern in the described
tests that call EctoLibSql.connect and Task.await_many (including the blocks
using EctoLibSql.handle_execute and Task.await_many at the shown ranges).
In @test/prepared_statement_test.exs:
- Around line 918-960: The test allows empty or nil results after resetting the
prepared statement, masking regressions; tighten the assertions in the "prepared
statements maintain isolation when reset concurrently" test so that after
calling Native.reset_stmt(state, stmt_id) a subsequent Native.query_stmt(state,
stmt_id, [task_num + 5]) must return the row inserted earlier (IDs 6..10), not
[] or nil. Replace the lenient case block with a strict assertion that
result2.rows contains a single row whose id equals task_num + 5 and whose
name/email match "User#{task_num + 5}" and "user#{task_num + 5}@example.com",
and remove the unreachable new_id == nil branch.
🧹 Nitpick comments (10)
test/savepoint_replication_test.exs (1)
1-62: Skip + teardown strategy looks sound for remote-cost control.
One small hardening:File.rm/1returns{:error, :enoent}and is currently ignored; if you want quieter teardown, useFile.rm/1+ ignore return (explicit) orFile.rm/1wrapped in a helper.test/json_helpers_test.exs (1)
467-545::skipdoesn’t skip tests; considerExUnitskipping so it’s visible.
Right now those branches “pass” silently. If that’s intended, all good; if not, useskip/1(or tag) so CI output is explicit.Also applies to: 647-686
native/ecto_libsql/src/tests/error_handling_tests.rs (2)
1-28: Module intent + unwrap exception are consistent with test guidance.
One optional tidy-up:setup_test_db/cleanup_test_dbare duplicated withnative/ecto_libsql/src/tests/integration_tests.rs; consider factoring into a shared test util if you keep growing test modules. Based on learnings, unwrap usage in tests is acceptable.
239-288: “No panic” tests should still assert something meaningful where feasible.
For parameter mismatch onexecute/preparedquery, consider assertingis_err()(or checking a stable error class) if libsql behaviour is expected to be consistent in your supported versions—otherwise these tests can pass while masking regressions.Also applies to: 451-479
test/pool_load_test.exs (1)
270-309: Ensure statement/connection cleanup even on failures inside tasks.
If any{:ok, ...} = ...match fails, the task crashes and may skipclose_stmt/disconnect. Wrapping task bodies withtry/afterwould make this more robust under load.test/connection_recovery_test.exs (1)
13-451: Assert the error tuple in “failure” steps to make recovery guarantees explicit.
E.g. replace_result = ...withassert {:error, _reason, _state} = ...(or the exact shape you expect). That keeps the tests honest and aligns with the “handle errors via pattern matching” guidance. Based on learnings, this is worth tightening.test/cursor_streaming_large_test.exs (3)
367-381:insert_rows/4is O(n) round-trips; consider a faster insert path if CI gets slow.
10k individual inserts (plus other tests) can drag. A prepared insert statement (or a single multi-rowINSERT) would keep coverage while reducing runtime. Based on learnings, prepared statements are a good fit for repeated queries.
383-427: Avoid non-tail recursion +++for ID collection (cheap to harden now).
fetch_all_ids/3builds lists withids ++ ..., which is quadratic; and several helpers aren’t tail-recursive. For current sizes it’s probably fine, but a small accumulator-based helper keeps this robust.
398-412:fetch_remaining_rows/3duplicatesfetch_all_rows/3.
Oncefetch_all_rows/4is query-aware, you can dropfetch_remaining_rows/3(or make it a thin wrapper) to reduce maintenance surface.test/prepared_statement_test.exs (1)
740-784: Concurrency suite looks useful; consider tagging/timeouts to reduce CI flake risk.
Task.await_many(..., 5000)can be tight under load; a slightly higher timeout and/or a@tag :concurrency/:slowwould make failures more actionable.Also applies to: 786-830, 832-874, 876-916
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
AGENTS.mdnative/ecto_libsql/src/tests/error_handling_tests.rsnative/ecto_libsql/src/tests/integration_tests.rsnative/ecto_libsql/src/tests/mod.rsnative/ecto_libsql/src/transaction.rstest/connection_recovery_test.exstest/cursor_streaming_large_test.exstest/json_helpers_test.exstest/pool_load_test.exstest/prepared_statement_test.exstest/savepoint_replication_test.exs
✅ Files skipped from review due to trivial changes (2)
- AGENTS.md
- native/ecto_libsql/src/transaction.rs
🧰 Additional context used
📓 Path-based instructions (7)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Always format Rust code before committing usingcargo fmt
NEVER use.unwrap()in production Rust code—usesafe_lockhelpers and proper error handling instead (tests may use.unwrap()for simplicity)
Files:
native/ecto_libsql/src/tests/mod.rsnative/ecto_libsql/src/tests/error_handling_tests.rsnative/ecto_libsql/src/tests/integration_tests.rs
native/ecto_libsql/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/**/*.rs: Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Validate transaction ownership by checking thatentry.conn_id == conn_idbefore performing transaction operations
Add#[rustler::nif]attribute to NIF functions—modern Rustler auto-detects them without manual registration
Files:
native/ecto_libsql/src/tests/mod.rsnative/ecto_libsql/src/tests/error_handling_tests.rsnative/ecto_libsql/src/tests/integration_tests.rs
native/ecto_libsql/src/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
native/ecto_libsql/src/tests/mod.rsnative/ecto_libsql/src/tests/error_handling_tests.rsnative/ecto_libsql/src/tests/integration_tests.rs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
native/ecto_libsql/src/tests/mod.rsnative/ecto_libsql/src/tests/error_handling_tests.rstest/pool_load_test.exstest/json_helpers_test.exstest/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/savepoint_replication_test.exsnative/ecto_libsql/src/tests/integration_tests.rs
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/pool_load_test.exstest/json_helpers_test.exstest/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/savepoint_replication_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/pool_load_test.exstest/json_helpers_test.exstest/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/savepoint_replication_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/pool_load_test.exstest/json_helpers_test.exstest/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/savepoint_replication_test.exs
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*.exs : Tests may use `.unwrap()` for simplicity when handling test data and setup
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
native/ecto_libsql/src/tests/mod.rsnative/ecto_libsql/src/tests/error_handling_tests.rstest/pool_load_test.exstest/json_helpers_test.exstest/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/savepoint_replication_test.exsnative/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
native/ecto_libsql/src/tests/mod.rsnative/ecto_libsql/src/tests/error_handling_tests.rsnative/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
native/ecto_libsql/src/tests/mod.rsnative/ecto_libsql/src/tests/error_handling_tests.rsnative/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
native/ecto_libsql/src/tests/mod.rsnative/ecto_libsql/src/tests/error_handling_tests.rstest/prepared_statement_test.exsnative/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
native/ecto_libsql/src/tests/mod.rsnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rstest/pool_load_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rstest/pool_load_test.exstest/json_helpers_test.exstest/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement type loaders for converting SQLite values to Ecto types using `loaders/2` function
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
test/pool_load_test.exstest/prepared_statement_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/pool_load_test.exstest/connection_recovery_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : JSON functions (extract, type, set, replace, insert, remove, patch) should be used for efficient JSON/JSONB manipulation in LibSQL 3.45.1+
Applied to files:
test/json_helpers_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use cursor streaming for large result sets to avoid loading entire datasets into memory
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*.exs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/connection_recovery_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/connection_recovery_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Use the Repository pattern for data access in EctoLibSql applications
Applied to files:
test/connection_recovery_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use prepared statements with automatic statement caching for repeated queries to achieve ~10-15x performance improvement
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Always format Elixir code before committing using `mix format --check-formatted`
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-08T04:56:27.393Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.393Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT tables in migrations for type safety when database schema stability is critical
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/turso_remote_test.exs : Turso remote tests should be skipped by default if credentials are missing and only run when explicitly needed due to cost implications
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
native/ecto_libsql/src/tests/integration_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
native/ecto_libsql/src/tests/integration_tests.rs
🧬 Code graph analysis (5)
native/ecto_libsql/src/tests/mod.rs (1)
native/ecto_libsql/src/lib.rs (1)
tests(30-30)
native/ecto_libsql/src/tests/error_handling_tests.rs (3)
native/ecto_libsql/src/tests/integration_tests.rs (4)
setup_test_db(14-16)cleanup_test_db(18-20)row(108-108)row(281-281)lib/ecto_libsql/query.ex (1)
to_string(49-51)native/ecto_libsql/src/transaction.rs (1)
drop(141-150)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (5)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)
test/connection_recovery_test.exs (2)
lib/ecto_libsql.ex (2)
handle_begin(312-317)handle_rollback(350-358)lib/ecto_libsql/native.ex (5)
prepare(951-959)batch(1208-1212)create_savepoint(1805-1807)rollback_to_savepoint_by_name(1877-1879)reset(1312-1314)
test/savepoint_replication_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (4)
begin(828-838)create_savepoint(1805-1807)release_savepoint_by_name(1836-1838)rollback_to_savepoint_by_name(1877-1879)
⏰ 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 (7)
native/ecto_libsql/src/tests/integration_tests.rs (1)
241-241: Spelling change aligns with AU/UK docs guideline.
No functional impact; looks good.test/savepoint_replication_test.exs (1)
64-251: Coverage hits the key savepoint/rollback/error-recovery paths.
The assertions are appropriately outcome-focused (rows/counts) rather than brittle error-string matching. Based on learnings, this is in the right direction for savepoint recovery patterns.native/ecto_libsql/src/tests/mod.rs (1)
6-10: Test module wiring looks correct.test/pool_load_test.exs (1)
19-41: Setup/teardown is good (per-test DB + cleanup).
Nice that this isolates state and cleans WAL/SHM artefacts.test/cursor_streaming_large_test.exs (1)
5-8: Guidelines check: en-AU spelling is good here; please also ensure formatting gate passes.
Comments read en-AU already; just make suremix format --check-formattedis clean before merge.Also applies to: 363-366
test/prepared_statement_test.exs (2)
313-586: Nice coverage expansion for reset, metadata, and parameter introspection.
The new scenarios are well-targeted and align with the test coverage learnings (happy path + edge/error cases).
13-17: Repo guideline mismatch: “camelCase function names in Elixir” vs current snake_case helpers.
This file uses idiomatic snake_case (exec_sql/3etc.). If the guideline is intentional, it should be applied consistently; if it’s a typo, it should be corrected to avoid churn.Also applies to: 313-425
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @TEST_AUDIT_REPORT.md:
- Line 3: Update the document header's Date value from "2024-01-08" to the
correct PR creation date "2026-01-08" by editing the Date line in
TEST_AUDIT_REPORT.md so the header matches the actual PR date.
🧹 Nitpick comments (3)
TEST_AUDIT_REPORT.md (2)
199-202: Add language specifier to fenced code block.Per markdownlint, fenced code blocks should have a language specified for proper syntax highlighting.
-``` +```text explain_query_test.exs: 262 lines, uses full Ecto setup explain_simple_test.exs: 115 lines, simpler test setup--- `453-458`: **Add language specifiers to directory tree code blocks.** The directory tree blocks at lines 453 and 462 lack language specifiers. Consider using `text` or `plaintext` for consistency with markdown linting. </blockquote></details> <details> <summary>test/cursor_streaming_large_test.exs (1)</summary><blockquote> `317-416`: **Consider consolidating similar helper functions.** The helper functions `fetch_all_rows`, `fetch_remaining_rows`, `fetch_all_ids`, `fetch_all_binary_rows`, `fetch_all_computed_rows`, `fetch_all_group_rows`, and `fetch_all_distinct_rows` share a similar pattern. Consider extracting a generic fetch function that accepts a row transformer. <details> <summary>♻️ Suggested refactor</summary> ```elixir defp fetch_all_with_transform(state, cursor, query, opts, transform_fn) do case EctoLibSql.handle_fetch(query, cursor, opts, state) do {:cont, result, next_state} -> transformed = transform_fn.(result.rows) transformed ++ fetch_all_with_transform(next_state, cursor, query, opts, transform_fn) {:halt, result, _state} -> transform_fn.(result.rows) end end # Usage examples: defp fetch_all_rows(state, cursor, query, opts) do fetch_all_with_transform(state, cursor, query, opts, &length/1) |> Enum.sum() end defp fetch_all_ids(state, cursor, query, opts) do fetch_all_with_transform(state, cursor, query, opts, fn rows -> Enum.map(rows, fn [id] -> id end) end) end
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
TEST_AUDIT_REPORT.mdTEST_COVERAGE_ISSUES_CREATED.mdnative/ecto_libsql/src/tests/error_handling_tests.rstest/connection_recovery_test.exstest/cursor_streaming_large_test.exstest/pool_load_test.exstest/prepared_statement_test.exs
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{md,markdown}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British/Australian English for all documentation files (except SQL keywords and compatibility requirements)
Files:
TEST_AUDIT_REPORT.mdTEST_COVERAGE_ISSUES_CREATED.md
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
TEST_AUDIT_REPORT.mdTEST_COVERAGE_ISSUES_CREATED.mdnative/ecto_libsql/src/tests/error_handling_tests.rstest/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/pool_load_test.exs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Always format Rust code before committing usingcargo fmt
NEVER use.unwrap()in production Rust code—usesafe_lockhelpers and proper error handling instead (tests may use.unwrap()for simplicity)
Files:
native/ecto_libsql/src/tests/error_handling_tests.rs
native/ecto_libsql/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/**/*.rs: Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Validate transaction ownership by checking thatentry.conn_id == conn_idbefore performing transaction operations
Add#[rustler::nif]attribute to NIF functions—modern Rustler auto-detects them without manual registration
Files:
native/ecto_libsql/src/tests/error_handling_tests.rs
native/ecto_libsql/src/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
native/ecto_libsql/src/tests/error_handling_tests.rs
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/pool_load_test.exs
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*.exs : Tests may use `.unwrap()` for simplicity when handling test data and setup
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
TEST_AUDIT_REPORT.mdTEST_COVERAGE_ISSUES_CREATED.mdnative/ecto_libsql/src/tests/error_handling_tests.rstest/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rstest/cursor_streaming_large_test.exstest/connection_recovery_test.exstest/prepared_statement_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rstest/prepared_statement_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rstest/connection_recovery_test.exstest/prepared_statement_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : NEVER use `.unwrap()` in production Rust code—use `safe_lock` helpers and proper error handling instead (tests may use `.unwrap()` for simplicity)
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use cursor streaming for large result sets to avoid loading entire datasets into memory
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/connection_recovery_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*.exs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/connection_recovery_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/connection_recovery_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Use the Repository pattern for data access in EctoLibSql applications
Applied to files:
test/connection_recovery_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/turso_remote_test.exs : Turso remote tests should be skipped by default if credentials are missing and only run when explicitly needed due to cost implications
Applied to files:
test/connection_recovery_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use prepared statements with automatic statement caching for repeated queries to achieve ~10-15x performance improvement
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2026-01-08T04:56:27.393Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.393Z
Learning: Applies to priv/repo/migrations/*.exs : Use RAW ROWID tables with the `random_rowid: true` option to prevent ID enumeration attacks in production
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
test/prepared_statement_test.exs
🧬 Code graph analysis (4)
test/cursor_streaming_large_test.exs (1)
lib/ecto_libsql.ex (3)
handle_execute(148-188)handle_declare(466-484)handle_fetch(415-442)
test/connection_recovery_test.exs (2)
lib/ecto_libsql.ex (4)
disconnect(131-133)handle_execute(148-188)handle_begin(312-317)handle_rollback(350-358)lib/ecto_libsql/native.ex (5)
prepare(951-959)batch(1208-1212)create_savepoint(1805-1807)rollback_to_savepoint_by_name(1877-1879)reset(1312-1314)
test/prepared_statement_test.exs (3)
test/stmt_caching_performance_test.exs (1)
exec_sql(15-18)test/savepoint_test.exs (1)
exec_sql(14-17)lib/ecto_libsql/native.ex (4)
prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)query_stmt(1041-1067)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (5)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)
🪛 LanguageTool
TEST_AUDIT_REPORT.md
[typographical] ~362-~362: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...is_err()); } ``` Estimated effort: 1-2 hours Impact: - Verifies Rust lay...
(HYPHEN_TO_EN)
[typographical] ~420-~420: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ormat operations Estimated effort: 3-4 hours Impact: More robust confiden...
(HYPHEN_TO_EN)
[typographical] ~548-~548: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ntributors Estimated total effort: 2-3 hours for high-priority items
(HYPHEN_TO_EN)
🪛 markdownlint-cli2 (0.18.1)
TEST_AUDIT_REPORT.md
17-17: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
453-453: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
462-462: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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)
TEST_AUDIT_REPORT.md (1)
1-549: Comprehensive and well-structured test audit documentation.This document provides excellent guidance for test consolidation efforts. The tiered categorisation, clear recommendations with effort estimates, and implementation checklists are valuable for tracking progress. The analysis of Rust vs Elixir test separation and identification of duplication aligns well with the PR objectives.
TEST_COVERAGE_ISSUES_CREATED.md (1)
1-127: Well-organised issue tracking documentation.This document provides excellent traceability from the TEST_AUDIT_REPORT.md recommendations to actionable issues. The priority breakdown, effort estimates, and mapping table make it easy to track progress on test coverage improvements.
native/ecto_libsql/src/tests/error_handling_tests.rs (7)
1-16: Excellent module documentation and appropriate clippy configuration.The module documentation clearly explains the purpose (BEAM VM stability) and the focus areas for error handling tests. The
#![allow(clippy::unwrap_used)]is appropriate here as per the project guidelines that allow.unwrap()in test code for simplicity.
22-28: Test isolation with unique database files.Good pattern using UUID-based database filenames to ensure test isolation. The cleanup function properly removes the test database file.
34-171: Comprehensive constraint violation tests.These tests cover all major SQLite constraint types (NOT NULL, UNIQUE, PRIMARY KEY, CHECK) and verify that violations result in errors rather than panics. This is critical for BEAM VM stability.
239-314: Parameter binding error tests with pragmatic assertions.The approach of not asserting on specific success/failure for edge cases (parameter count mismatch) is sensible, as libsql behaviour may vary. The key assertion that it doesn't panic is the correct focus for BEAM stability.
485-511: Platform-specific tests for invalid paths.Good use of
#[cfg(unix)]and#[cfg(windows)]to provide platform-appropriate invalid path tests. This ensures the test suite works correctly across platforms.
599-635: Unicode handling and SQL injection prevention tests.Excellent tests for Unicode data preservation and SQL injection prevention via parameterisation. The Unicode test includes characters from multiple scripts (Latin, Chinese, Arabic) providing good coverage.
1-673: Comprehensive error handling test suite.This new test file addresses the recommendations from TEST_AUDIT_REPORT.md for Rust-side error handling tests. It covers constraint violations, syntax errors, parameter binding, transaction errors, prepared statements, file errors, and edge cases. The tests verify graceful error returns rather than panics, which is critical for BEAM VM stability. Based on learnings, the use of
.unwrap()in test setup is appropriate.test/connection_recovery_test.exs (5)
1-16: Clean test module setup with in-memory database.Good use of in-memory database for test isolation and proper cleanup in
on_exitcallback. The alias forEctoLibSqlkeeps the test code concise.
18-91: Thorough connection recovery tests for query and constraint errors.These tests verify that the connection remains usable after failed queries and constraint violations. The pattern of executing an error-causing operation and then verifying subsequent operations succeed is appropriate for testing resilience.
133-176: Transaction error recovery test.Good test verifying that a transaction can still be rolled back after a query error within the transaction. This is important for ensuring data integrity in error scenarios.
324-385: Savepoint error recovery with partial rollback.Excellent test demonstrating savepoint functionality for targeted rollback within a transaction. The test correctly verifies that:
- Data before the savepoint is preserved
- Data after savepoint rollback is lost
- New data can be inserted after rollback
- Final commit works correctly
This covers an important error recovery pattern as per the coding guidelines.
1-457: Comprehensive connection recovery test suite.This test module provides excellent coverage of connection resilience scenarios including failed queries, constraint violations, syntax errors, transaction errors, prepared statement errors, batch failures, savepoint recovery, and connection resets. The tests follow the pattern of causing an error and verifying the connection remains usable, which is critical for production reliability.
test/pool_load_test.exs (5)
1-41: Well-documented test module with proper setup.The moduledoc clearly explains the test scenarios and the rationale for using separate connections. The setup properly creates a unique database file and includes comprehensive cleanup of SQLite WAL mode files (-shm, -wal).
43-109: Concurrent connection tests with appropriate timeouts.Good use of
Task.async/Task.await_manyfor testing concurrent connections. Thebusy_timeout: 30_000setting is appropriate for SQLite concurrent access where write locks may cause contention. The tests verify all concurrent operations succeed.
111-177: Long-running operation tests.These tests verify that transactions held for longer periods don't cause timeout issues. The use of
Process.sleepto simulate work within transactions is appropriate for this scenario.
270-308: Prepared statement cleanup under concurrent load.Good test verifying that prepared statements are properly created, executed, and closed under concurrent load. The explicit
close_stmtcall ensures resources are cleaned up.
1-356: Comprehensive pool/load test suite addressing identified gaps.This test module addresses the "Connection pool behavior under load" gap identified in TEST_AUDIT_REPORT.md. It covers concurrent independent connections, long-running operations, connection recovery, resource cleanup, and transaction isolation. The use of file-based databases with WAL mode and appropriate busy timeouts is well-suited for testing concurrent SQLite access.
test/cursor_streaming_large_test.exs (4)
1-30: Clear test module setup with comprehensive table schema.Good setup with in-memory database and a table schema that supports various test scenarios (batch_id for grouping, sequence for ordering, data BLOB for binary tests).
32-59: Large dataset streaming tests.These tests verify cursor streaming with 1K and 10K rows, demonstrating that large datasets can be processed in batches without loading everything into memory at once. This addresses the "Large result sets with streaming" gap identified in TEST_AUDIT_REPORT.md.
110-155: BLOB data preservation through cursor streaming.Excellent test verifying that binary data is correctly preserved when streamed through cursors. The test creates 1KB binary data blocks and verifies both the content structure and size are maintained.
1-417: Comprehensive cursor streaming test suite.This test module provides excellent coverage of cursor-based streaming scenarios including large datasets, batch sizes, filtering, ordering, BLOB handling, JOINs, computed columns, aggregations, and more. This directly addresses the "Large result set streaming (cursors)" gap identified in TEST_AUDIT_REPORT.md. The tests verify that data can be streamed in batches without loading entire datasets into memory.
test/prepared_statement_test.exs (8)
313-366: Statement reset and caching tests.Good tests verifying that prepared statements can be reused multiple times with automatic binding reset between executions. This demonstrates the ~10-15x performance benefit of prepared statements mentioned in the learnings.
368-425: Explicit reset tests with error handling.These tests verify the explicit
reset_stmtAPI, including error handling for invalid statement IDs. Good coverage of the reset lifecycle.
427-507: Full column metadata introspection tests.Comprehensive tests for
get_stmt_columnscovering basic columns, aliased columns, and expression columns. Good verification of the metadata tuple structure{name, origin_name, decl_type}.
509-586: Parameter naming tests for different styles.Excellent coverage of SQLite's parameter naming conventions including colon-style (
:name), dollar-style ($name), at-style (@name), and mixed positional/named parameters.
740-975: Concurrent prepared statement usage tests.These tests verify thread-safety of prepared statements under concurrent access. Good coverage of:
- Multiple processes using different statements
- Single statement shared across processes
- Concurrent writes maintaining consistency
- Parameter isolation across tasks
- Concurrent resets maintaining isolation
This addresses an important concern for production use where multiple processes may share prepared statements.
734-734: Good use of British English spelling.The comment correctly uses "behaviour" (British spelling) as per the coding guidelines.
1-976: Significantly expanded prepared statement test suite.This test file now provides comprehensive coverage of prepared statement functionality including:
- Basic preparation and execution
- Statement introspection (columns, parameters)
- Statement lifecycle (close, use-after-close)
- Reset and caching behaviour
- Parameter naming conventions
- Binding behaviour (auto-reset, type coercion)
- Concurrent usage across multiple processes
The additions address the consolidation goals from TEST_AUDIT_REPORT.md by merging functionality previously in
statement_features_test.exsinto this canonical file.
7-7: No action required—async: trueis safely implemented.The test module uses
async: truewith proper database isolation: each test creates a unique database file via:erlang.unique_integer([:positive])and cleans it up after execution. This isolation pattern prevents interference between concurrent test runs. The concurrent task spawning within individual tests (lines 740–975) operates against each test's own isolated database, confirming that async execution is safe.
TEST_AUDIT_REPORT.md
Outdated
| @@ -0,0 +1,549 @@ | |||
| # Comprehensive Test Audit: Elixir vs Rust Tests | |||
|
|
|||
| **Date**: 2024-01-08 | |||
There was a problem hiding this comment.
Inconsistent date in document header.
The date shows 2024-01-08 but the PR was created on 2026-01-08. This appears to be a typo.
-**Date**: 2024-01-08
+**Date**: 2026-01-08 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **Date**: 2024-01-08 | |
| **Date**: 2026-01-08 |
🤖 Prompt for AI Agents
In @TEST_AUDIT_REPORT.md at line 3, Update the document header's Date value from
"2024-01-08" to the correct PR creation date "2026-01-08" by editing the Date
line in TEST_AUDIT_REPORT.md so the header matches the actual PR date.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/cursor_streaming_large_test.exs (2)
232-236: Consider tightening batch count assertion.For 1000 rows with a batch size of 100, you should get exactly 10 batches. The assertion
>= 9is conservative and could mask off-by-one errors.Suggested fix
- assert batch_count >= 9, "Should have at least 9 batches for 1000 rows with batch size 100" + assert batch_count == 10, "Should have exactly 10 batches for 1000 rows with batch size 100"
372-420: Consider consolidating duplicate helper functions.
fetch_all_binary_rows,fetch_all_computed_rows,fetch_all_group_rows, andfetch_all_distinct_rowsare functionally identical—they all collectresult.rows. A single generic helper would reduce duplication.Suggested consolidation
- defp fetch_all_binary_rows(state, cursor, query, opts) do - case EctoLibSql.handle_fetch(query, cursor, opts, state) do - {:cont, result, next_state} -> - result.rows ++ fetch_all_binary_rows(next_state, cursor, query, opts) - - {:halt, result, _state} -> - result.rows - end - end - - defp fetch_all_computed_rows(state, cursor, query, opts) do - case EctoLibSql.handle_fetch(query, cursor, opts, state) do - {:cont, result, next_state} -> - result.rows ++ fetch_all_computed_rows(next_state, cursor, query, opts) - - {:halt, result, _state} -> - result.rows - end - end - - defp count_batches(state, cursor, query, opts) do - case EctoLibSql.handle_fetch(query, cursor, opts, state) do - {:cont, _result, next_state} -> - 1 + count_batches(next_state, cursor, query, opts) - - {:halt, _result, _state} -> - 1 - end - end - - defp fetch_all_group_rows(state, cursor, query, opts) do - case EctoLibSql.handle_fetch(query, cursor, opts, state) do - {:cont, result, next_state} -> - result.rows ++ fetch_all_group_rows(next_state, cursor, query, opts) - - {:halt, result, _state} -> - result.rows - end - end - - defp fetch_all_distinct_rows(state, cursor, query, opts) do - case EctoLibSql.handle_fetch(query, cursor, opts, state) do - {:cont, result, next_state} -> - result.rows ++ fetch_all_distinct_rows(next_state, cursor, query, opts) - - {:halt, result, _state} -> - result.rows - end - end + defp fetch_all_raw_rows(state, cursor, query, opts) do + case EctoLibSql.handle_fetch(query, cursor, opts, state) do + {:cont, result, next_state} -> + result.rows ++ fetch_all_raw_rows(next_state, cursor, query, opts) + + {:halt, result, _state} -> + result.rows + end + endThen update callers to use
fetch_all_raw_rows/4.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/cursor_streaming_large_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/cursor_streaming_large_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/cursor_streaming_large_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/cursor_streaming_large_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/cursor_streaming_large_test.exs
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Always format Elixir code before committing using `mix format --check-formatted`
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use cursor streaming for large result sets to avoid loading entire datasets into memory
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/cursor_streaming_large_test.exs
🧬 Code graph analysis (1)
test/cursor_streaming_large_test.exs (1)
lib/ecto_libsql.ex (4)
handle_execute(148-188)disconnect(131-133)handle_declare(466-484)handle_fetch(415-442)
⏰ 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). (2)
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (10)
test/cursor_streaming_large_test.exs (10)
1-34: Setup block and module structure look good.The test module is well-structured with an in-memory database setup, proper cleanup via
on_exit, and a table schema that supports the various test scenarios (including BLOB data via thedatacolumn).
37-63: Good coverage of basic large dataset streaming.These tests effectively validate that the cursor can stream large datasets (1K and 10K rows) without loading all data into memory at once, with configurable batch sizes.
65-82: Batch size validation is correct.The test properly verifies that the cursor respects the
max_rowsoption by checking the first batch size and then counting remaining rows to confirm the total.
100-112: Order verification test is well-designed.The test correctly verifies that cursor streaming preserves row ordering by comparing fetched IDs against the expected sequence.
114-159: BLOB handling test is thorough.Good test for binary data preservation through the cursor streaming path. The test creates known binary patterns and verifies size integrity.
253-277: GROUP BY aggregation test is well-designed.The test correctly validates grouped aggregation results by verifying both the number of groups and the count per group.
279-295: OFFSET/LIMIT boundary validation is correct.Good test for verifying that cursor streaming respects SQL-level pagination, checking both the count and the exact ID range.
325-339: Helper functioninsert_rowsis correct.The row-by-row insertion is straightforward and works well for test purposes. Whilst batch inserts would be faster for large datasets, the current approach prioritises clarity.
36-319: Consider complementary test coverage for cursor streaming error and edge cases.The test suite provides comprehensive happy-path coverage for cursor streaming with large datasets (various SQL clauses, binary data, computed columns). Per the coding guidelines, tests should cover error cases, edge cases (NULL values, empty result sets), transaction rollback scenarios, and concurrent operations. Verify whether these scenarios are covered in companion test files before addressing this review.
246-250: No action required — the test assumption is correct.The
handle_fetchimplementation returns{:halt, ...}only when the rows list is empty. For an aggregation query returning a single row,rowswill be non-empty and{:cont, ...}will be returned, as expected by the test pattern match.
…nd fix formatting
ocean
force-pushed
the
consolidate-tests
branch
from
fa26f8f to
efbeacb
Compare
- Changed format string from {}, i to {i} syntax
- Resolves clippy::uninlined-format-args warning
- All tests pass, no formatting issues
…cation - Tighten batch count assertion to exactly 11 (10 full batches + 1 final empty batch) - Replace 4 nearly identical fetch_all_* functions with single generic helper - Keep semantic aliases for backwards compatibility - All tests pass
- Add explicit CRITICAL warning in Quick Rules section - Detail exact 5-step pre-commit workflow with strict ordering - Emphasise that check-formatted must PASS before any commit - Clarify Cargo fmt --check verification step - Add warning about never committing with failing checks
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/json_helpers_test.exs (1)
5-29: Consider assertingdisconnect/2succeeds (or intentionally ignoring failures).
on_exitcleanup is a good addition, but right now any disconnect failure will be silent. IfEctoLibSql.disconnect/2returns a tagged tuple, pattern-match it for a cleaner failure mode.Proposed tweak
on_exit(fn -> - EctoLibSql.disconnect([], state) + _ = EctoLibSql.disconnect([], state) end)
🤖 Fix all issues with AI agents
In @native/ecto_libsql/src/tests/error_handling_tests.rs:
- Around line 291-313: The assertion `result.is_ok() || result.is_err()` is a
tautology and tests nothing; replace it with a real check in the
`test_type_coercion_integer_to_text` test: ensure the `conn.execute(...)` call
stored in `result` does not panic by asserting success (e.g., assert that
`result.is_ok()` or unwrap the result) so the insert using `Value::Integer(123)`
for the TEXT column is verified to complete; update the assertion on the
`result` variable (from the `conn.execute` call) accordingly.
- Around line 1-14: The file uses US spelling in Rust comments/docs (e.g.,
“parameterized”); update those to Australian/British variants across the test
file (native/ecto_libsql/src/tests/error_handling_tests.rs) including the header
and other occurrences around the Parameter validation section and lines
~647-670; specifically replace "parameterized" → "parameterised",
"parameterization" → "parameterisation", and any similar US spellings in
comments/docs while leaving code identifiers and string literals unchanged so
function names (tests, modules) and runtime behavior are not modified.
- Around line 22-28: Tests call cleanup_test_db(&db_path) while DB handles
(conn/db) are still in scope and they don't remove SQLite WAL/SHM files, causing
Windows file-lock flakes; change setup_test_db to create the DB path under
std::env::temp_dir(), implement an RAII guard type (e.g., TestDbGuard) declared
before any db/conn so its Drop runs after handles are dropped, and move deletion
logic into its Drop to remove the main .db and the associated .db-wal and
.db-shm files; update tests to instantiate TestDbGuard using the path produced
by setup_test_db and remove direct cleanup_test_db calls so cleanup is reliable
across platforms.
In @test/cursor_streaming_large_test.exs:
- Around line 341-359: Both fetch_all_rows and fetch_remaining_rows call
EctoLibSql.handle_fetch/4 but lack a clause for {:error, reason, state}, causing
a CaseClauseError on failures; add a third match in each case to handle {:error,
reason, _state} and call ExUnit.flunk/1 with a descriptive message (e.g., "fetch
error: #{inspect(reason)}") so tests fail with a clear error instead of a
CaseClauseError.
In @test/pool_load_test.exs:
- Around line 44-109: Tasks open DB connections with EctoLibSql.connect then
call EctoLibSql.handle_execute but call EctoLibSql.disconnect only on the
success path, so if a task crashes the DB stays locked; wrap the task body in a
try ... after (or try do ... rescue ... after ...) so that
EctoLibSql.disconnect([], state) is invoked in the after block regardless of
success/failure and still return the original result/exception, updating all
task bodies that call EctoLibSql.connect, handle_execute, and disconnect (e.g.,
inside the blocks that currently use Task.async in the tests).
🧹 Nitpick comments (5)
native/ecto_libsql/src/tests/error_handling_tests.rs (1)
513-544: Rename or refactortest_readonly_database_insert— it doesn't actually open a read-only connectionThe test reopens the database using
Builder::new_local(), which creates a standard read-write connection, not a read-only one. In libsql v0.9.29, read-only connections requireBuilder::new_local_replica(). Either rename the test to reflect what it actually does (e.g.test_reopen_and_query_database) or refactor it to properly set up read-only mode using the replication feature if that's the intent.test/pool_load_test.exs (2)
19-41:on_exitfile cleanup should tolerate:enoent(and optionally assert unexpected errors).
File.rm/1returns{:error, :enoent}when the file doesn’t exist; ignoring is fine, but if you want stronger hygiene, handle only:enoentand surface other failures.
43-356: Consider tagging these as:slow/:flaky(or reducing counts) to keep CI stable.These tests intentionally stress concurrency and locking; on constrained CI they may intermittently time out. Tagging lets you keep them valuable without making the default suite brittle.
test/cursor_streaming_large_test.exs (2)
325-339: Bulk insert (or wrap inserts in a transaction) to keep runtime under control.
insert_rows/4issues one insert per row; with 10k+ rows across multiple tests this can dominate CI time. Even a single explicit transaction around the loop would help.
361-381: Avoid++in recursive fetch helpers (can go O(n²) and inflate memory).
ids ++ fetch_all_ids(...)andresult.rows ++ ...repeatedly copy lists. Prefer an accumulator (reverse at the end) orStream/Enum.reduce.Sketch: accumulator approach for IDs
- defp fetch_all_ids(state, cursor, query, opts) do - case EctoLibSql.handle_fetch(query, cursor, opts, state) do - {:cont, result, next_state} -> - ids = Enum.map(result.rows, fn [id] -> id end) - ids ++ fetch_all_ids(next_state, cursor, query, opts) - - {:halt, result, _state} -> - Enum.map(result.rows, fn [id] -> id end) - end - end + defp fetch_all_ids(state, cursor, query, opts) do + fetch_all_ids(state, cursor, query, opts, []) + end + + defp fetch_all_ids(state, cursor, query, opts, acc) do + case EctoLibSql.handle_fetch(query, cursor, opts, state) do + {:cont, result, next_state} -> + batch_ids = Enum.map(result.rows, fn [id] -> id end) + fetch_all_ids(next_state, cursor, query, opts, [batch_ids | acc]) + + {:halt, result, _state} -> + batch_ids = Enum.map(result.rows, fn [id] -> id end) + acc = [batch_ids | acc] + acc |> Enum.reverse() |> List.flatten() + end + endAlso applies to: 372-399
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.gitignorenative/ecto_libsql/src/tests/error_handling_tests.rstest/connection_recovery_test.exstest/cursor_streaming_large_test.exstest/json_helpers_test.exstest/pool_load_test.exstest/prepared_statement_test.exs
🚧 Files skipped from review as they are similar to previous changes (2)
- .gitignore
- test/connection_recovery_test.exs
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/cursor_streaming_large_test.exstest/json_helpers_test.exstest/prepared_statement_test.exstest/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/cursor_streaming_large_test.exstest/json_helpers_test.exstest/prepared_statement_test.exstest/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/cursor_streaming_large_test.exstest/json_helpers_test.exstest/prepared_statement_test.exstest/pool_load_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/cursor_streaming_large_test.exstest/json_helpers_test.exsnative/ecto_libsql/src/tests/error_handling_tests.rstest/prepared_statement_test.exstest/pool_load_test.exs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Always format Rust code before committing usingcargo fmt
NEVER use.unwrap()in production Rust code—usesafe_lockhelpers and proper error handling instead (tests may use.unwrap()for simplicity)
Files:
native/ecto_libsql/src/tests/error_handling_tests.rs
native/ecto_libsql/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/**/*.rs: Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Validate transaction ownership by checking thatentry.conn_id == conn_idbefore performing transaction operations
Add#[rustler::nif]attribute to NIF functions—modern Rustler auto-detects them without manual registration
Files:
native/ecto_libsql/src/tests/error_handling_tests.rs
native/ecto_libsql/src/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
native/ecto_libsql/src/tests/error_handling_tests.rs
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use cursor streaming for large result sets to avoid loading entire datasets into memory
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/cursor_streaming_large_test.exstest/json_helpers_test.exsnative/ecto_libsql/src/tests/error_handling_tests.rstest/prepared_statement_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/cursor_streaming_large_test.exstest/json_helpers_test.exsnative/ecto_libsql/src/tests/error_handling_tests.rstest/prepared_statement_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : JSON functions (extract, type, set, replace, insert, remove, patch) should be used for efficient JSON/JSONB manipulation in LibSQL 3.45.1+
Applied to files:
test/json_helpers_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/json_helpers_test.exsnative/ecto_libsql/src/tests/error_handling_tests.rstest/prepared_statement_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/json_helpers_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
test/json_helpers_test.exsnative/ecto_libsql/src/tests/error_handling_tests.rstest/prepared_statement_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : NEVER use `.unwrap()` in production Rust code—use `safe_lock` helpers and proper error handling instead (tests may use `.unwrap()` for simplicity)
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use prepared statements with automatic statement caching for repeated queries to achieve ~10-15x performance improvement
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2026-01-08T04:56:27.393Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.393Z
Learning: Applies to priv/repo/migrations/*.exs : Use RAW ROWID tables with the `random_rowid: true` option to prevent ID enumeration attacks in production
Applied to files:
test/prepared_statement_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
test/prepared_statement_test.exs
🧬 Code graph analysis (3)
test/cursor_streaming_large_test.exs (1)
lib/ecto_libsql.ex (3)
handle_execute(148-188)disconnect(131-133)handle_fetch(415-442)
test/json_helpers_test.exs (2)
lib/ecto_libsql.ex (2)
disconnect(131-133)handle_execute(148-188)lib/ecto_libsql/query.ex (1)
to_string(49-51)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (5)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)
⏰ 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 (5)
native/ecto_libsql/src/tests/error_handling_tests.rs (1)
34-673: Nice breadth of negative-path coverageThe suite hits a good spread of constraint, syntax, transaction-lifecycle, prepared-statement, and edge-case scenarios, and the tests consistently avoid
unwrap()on the operation-under-test (matching the “no panic” goal). Based on learnings, allowingunwrap()for setup innative/ecto_libsql/src/tests/**/*.rsis fine.test/json_helpers_test.exs (1)
292-300: Good move dropping JSONB size assumptions.
assert result != jsonis a much more stable property than comparing sizes across libSQL/SQLite versions.test/prepared_statement_test.exs (3)
313-586: Nice coverage of statement reset + metadata + parameter name variants.This rounds out the lifecycle surface area well (reuse, explicit reset, columns, and mixed parameter styles).
509-586: The test assumptions aboutstmt_parameter_nameindexing are correct.SQLite and libSQL use 1-based indexing for parameter lookup, and anonymous positional parameters (
?) returnnil. The test cases correctly validate this behaviour across colon-style (:name), dollar-style ($name), and at-style (@name) named parameters, as well as mixed parameter types. No changes required.
740-975: Concurrent prepared statement usage is fully supported and thread-safe.The NIF implementation uses thread-safe registries (
Mutex<HashMap>) to manage connections and statements, with each statement stored asArc<Mutex<stmt>>for safe concurrent access. Bothquery_preparedandexecute_preparedvalidate statement ownership (decode::verify_statement_ownership) before operations, and statement bindings are reset before each execution to ensure parameter isolation. These tests are excellent regression tests for verifying the concurrency guarantees and can be kept as-is.
- Changed 'parameterized' to 'parameterised' in comments (3 occurrences) - Applies CLAUDE.md guidance on British/Australian English - No functional changes to code or test identifiers - All tests pass
- test/pool_load_test.exs: Handle :enoent in file cleanup gracefully - Changed on_exit/1 to tolerate missing files with proper error handling - Surfaces unexpected errors with IO.warn for better diagnostics - test/pool_load_test.exs: Tag all stress tests as :slow and :flaky - These concurrency/load tests can be flaky on constrained CI systems - Tagged to keep default test suite fast and stable - Tests still run when explicitly included - test/test_helper.exs: Update exclusion logic for CI - Default: exclude :ci_only, :slow, :flaky tests locally - On CI: only exclude :flaky tests to maintain stability - Reduces CI brittleness while keeping valuable tests available
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
native/ecto_libsql/src/tests/error_handling_tests.rs (1)
309-312: Tautological assertion - always passes.
result.is_ok() || result.is_err()is always true for anyResult. This assertion doesn't verify anything meaningful. For consistency with similar tests in this file (e.g., lines 259, 286), use thelet _ = result;pattern instead.♻️ Suggested fix
- assert!( - result.is_ok() || result.is_err(), - "Should handle type coercion without panic" - ); + // SQLite permits this due to type affinity, but should not panic + let _ = result;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{md,markdown}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British/Australian English for all documentation files (except SQL keywords and compatibility requirements)
Files:
CLAUDE.md
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Always format Rust code before committing usingcargo fmt
NEVER use.unwrap()in production Rust code—usesafe_lockhelpers and proper error handling instead (tests may use.unwrap()for simplicity)
Files:
native/ecto_libsql/src/tests/error_handling_tests.rs
native/ecto_libsql/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/**/*.rs: Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Validate transaction ownership by checking thatentry.conn_id == conn_idbefore performing transaction operations
Add#[rustler::nif]attribute to NIF functions—modern Rustler auto-detects them without manual registration
Files:
native/ecto_libsql/src/tests/error_handling_tests.rs
native/ecto_libsql/src/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
native/ecto_libsql/src/tests/error_handling_tests.rs
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to AGENTS.md : Update AGENTS.md API reference when adding new public NIF functions or Ecto features that users should know about
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to README.md : Update README.md when adding new features or changing user-facing functionality
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : NEVER use `.unwrap()` in production Rust code—use `safe_lock` helpers and proper error handling instead (tests may use `.unwrap()` for simplicity)
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : Always format Rust code before committing using `cargo fmt`
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Always format Elixir code before committing using `mix format --check-formatted`
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use Beads (`bd` command) for multi-session issue tracking with persistence across sessions and for work with dependencies between tasks
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs,rs,md} : When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with `git commit`, clean up stashes, and provide context for next session
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-08T04:56:27.393Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.393Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT tables in migrations for type safety when database schema stability is critical
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
CLAUDE.mdnative/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{md,markdown} : Use British/Australian English for all documentation files (except SQL keywords and compatibility requirements)
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/tests/error_handling_tests.rs
⏰ 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: Rust Fuzz Testing
- 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 (11)
native/ecto_libsql/src/tests/error_handling_tests.rs (7)
1-28: LGTM!The module documentation clearly explains the test purpose and focus areas. The
#![allow(clippy::unwrap_used)]is appropriate here since tests may use.unwrap()for simplicity per project guidelines. The UUID-based test database setup ensures test isolation.
34-171: LGTM!Comprehensive coverage of constraint violation scenarios (NOT NULL, UNIQUE, PRIMARY KEY, CHECK). Each test properly isolates the database, validates that violations return errors rather than panicking, and cleans up afterwards.
177-233: LGTM!Good coverage of SQL syntax and semantic errors. Testing typos, non-existent tables/columns, and malformed SQL ensures the NIF layer handles these gracefully without panicking.
320-430: LGTM!Excellent transaction error coverage including double commit/rollback scenarios and verification that rollback actually works. The
test_query_after_rollbacktest properly validates that data is not persisted after rollback.
436-479: LGTM!Good coverage of prepared statement error scenarios. The parameter mismatch test appropriately uses the
let _ = result;pattern since libsql behaviour may vary.
485-544: LGTM!Good platform-specific handling with
#[cfg(unix)]and#[cfg(windows)]attributes. The invalid path tests (null bytes on Unix, reserved device name on Windows) are reasonable edge cases to verify no panics occur.
550-673: LGTM!Excellent edge case coverage including empty/whitespace SQL, very long queries, Unicode handling (good multi-script example), and SQL injection safety verification. The injection test properly validates that the malicious string is stored literally rather than executed, confirming parameterised query safety.
CLAUDE.md (4)
13-17: LGTM!Good addition of explicit formatting workflow in Quick Rules. The three-step process (format → verify → commit) with emphasis on catching issues early is clear and actionable.
53-53: LGTM!TOC correctly updated to include the Issue Tracking with Beads section.
302-308: LGTM!Practical best practices for Beads workflow that complement the command reference. The emphasis on session hygiene (checking
bd readyat start, syncing before end) aligns well with the mandatory session completion workflow.
565-584: LGTM!The strict ordering and explicit step numbering in the pre-commit checklist provides clear guidance. The critical warning about not proceeding with failing checks reinforces the project's quality standards.
Two key performance improvements: 1. insert_rows/4 - Use prepared statements instead of individual inserts - Was: 1000+ unprepared INSERT statements per test - Now: Reuses single prepared statement with parameter binding - Benefit: Reduces statement compilation overhead, faster inserts - Note: Can't wrap in transaction (cursors require no active transaction) 2. fetch_all_ids/4 and fetch_all_cursor_rows/4 - Fix O(n²) list concatenation - Was: ids ++ recursive_call() and result.rows ++ recursive_call() - Now: Accumulator pattern with single reversal at end - Benefit: Prevents expensive list copies, scales to 10k+ rows - Example: 10k rows went from ~O(n²) to O(n) operations These optimizations keep CI runtime predictable with large dataset tests.
Ensure close_stmt is always called regardless of execute_stmt success/failure by wrapping the execution loop in try/after block.
Apply same try/after pattern to ensure close_stmt is always called.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @test/cursor_streaming_large_test.exs:
- Line 494: Change any American English spelling in the test descriptions and
documentation to British English; specifically rename the describe block "cursor
transaction behavior" to "cursor transaction behaviour" (and update any other
comments/strings in this file that use American spellings to their
British/Australian equivalents).
In @test/pool_load_test.exs:
- Around line 305-307: The test is asserting the number of result rows instead
of the COUNT(*) value — verify_result.rows will always be a single row; extract
the count from verify_result.rows (the first row's first column), convert it to
an integer if it's returned as a string or numeric type, and assert that that
extracted count is > 0; reference verify_result and verify_result.rows when
locating the code to change.
- Around line 1279-1289: The test case is matching the wrong error tuple shape;
change the pattern matching on result from a 4-tuple to the 3-tuple shape
produced by handle_execute so the error branch actually matches. Update the
clause that currently reads {:error, _query, _reason, trx_state} to match
{:error, _reason, trx_state} and ensure you still call
EctoLibSql.Native.rollback(trx_state) and return {:ok, :correctly_rolled_back};
leave the success branch ({:ok, _query, _result, trx_state}) unchanged.
🧹 Nitpick comments (7)
test/savepoint_replication_test.exs (5)
19-19: Consider using||for consistency with Elixir conventions.Whilst
orworks correctly here,||is more commonly used in Elixir for boolean operations and would be more idiomatic.♻️ Proposed refactor
- @moduletag skip: is_nil(@turso_uri) || is_nil(@turso_token) + @moduletag skip: is_nil(@turso_uri) || is_nil(@turso_token)
27-27: Consider using||for consistency with Elixir conventions.Similar to line 19, using
||instead oforwould be more idiomatic for boolean operations in Elixir.♻️ Proposed refactor
- if not (is_nil(@turso_uri) or is_nil(@turso_token)) do + if not (is_nil(@turso_uri) || is_nil(@turso_token)) do
49-69: Consider consolidating cleanup logic for better readability.Whilst the current cleanup approach is functional and correctly handles failures, consolidating the multiple try/rescue blocks could improve maintainability.
♻️ Proposed refactor
on_exit(fn -> - # Drop remote table to clean up Turso database - try do - EctoLibSql.handle_execute( - "DROP TABLE IF EXISTS #{test_table}", - [], - [], - state - ) - rescue - _ -> :ok - end - - try do - EctoLibSql.disconnect([], state) - rescue - _ -> :ok - end - + # Clean up remote table and disconnect + catch_error = fn fun -> + try do + fun.() + rescue + _ -> :ok + end + end + + catch_error.(fn -> + EctoLibSql.handle_execute( + "DROP TABLE IF EXISTS #{test_table}", + [], + [], + state + ) + end) + + catch_error.(fn -> EctoLibSql.disconnect([], state) end) + EctoLibSql.TestHelpers.cleanup_db_files(test_db) end)
74-203: Consider adding verification that remote sync actually occurred.Whilst these tests validate savepoint behavior, they don't verify that data was actually synchronized to the remote Turso database. The tests assume sync succeeded but don't confirm it. Consider adding a verification step that queries the remote database directly to ensure replication occurred.
225-234: Consider asserting on the specific error reason.Whilst the test correctly handles the error scenario, it only verifies that an error occurred without checking the error reason. Consider asserting on the expected constraint violation error to make the test more robust.
♻️ Proposed enhancement
result = EctoLibSql.handle_execute( "INSERT INTO #{table} (id, name) VALUES (?, ?)", [100, "Duplicate"], [], trx_state ) # Rebind trx_state - error tuple contains updated transaction state needed for recovery - assert {:error, _reason, trx_state} = result + assert {:error, %{sqlite: %{message: message}}, trx_state} = result + assert message =~ ~r/UNIQUE constraint failed|PRIMARY KEY/itest/pool_load_test.exs (2)
17-17: Unused alias declaration.The
alias EctoLibSqlon line 17 is not used anywhere in the module — all references use the full module nameEctoLibSql. Consider removing it or actually using the alias throughout the file for consistency.♻️ Suggested fix
- alias EctoLibSql
800-810: State not properly chained through sequential inserts.Each call to
insert_edge_case_value(trx_state, value)uses the originaltrx_staterather than threading the updated state from the previous insert. While this may work due to how the underlying connection tracks transaction state by ID, it's inconsistent with the pattern used elsewhere in these tests and could mask issues if state tracking changes.Consider threading the state through the fold:
♻️ Suggested improvement
- insert_results = - Enum.map(edge_values, fn value -> - {:ok, _query, _result, new_state} = insert_edge_case_value(trx_state, value) - new_state - end) - - # Use final state after all inserts - final_trx_state = List.last(insert_results) || trx_state + final_trx_state = + Enum.reduce(edge_values, trx_state, fn value, current_state -> + {:ok, _query, _result, new_state} = insert_edge_case_value(current_state, value) + new_state + end)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
test/cursor_streaming_large_test.exstest/pool_load_test.exstest/savepoint_replication_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/cursor_streaming_large_test.exstest/pool_load_test.exstest/savepoint_replication_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/cursor_streaming_large_test.exstest/pool_load_test.exstest/savepoint_replication_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/cursor_streaming_large_test.exstest/pool_load_test.exstest/savepoint_replication_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/cursor_streaming_large_test.exstest/pool_load_test.exstest/savepoint_replication_test.exs
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.393Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT tables in migrations for type safety when database schema stability is critical
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use cursor streaming for large result sets to avoid loading entire datasets into memory
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/pool_load_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/pool_load_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/pool_load_test.exstest/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Always format Elixir code before committing using `mix format --check-formatted`
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*.exs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-08T04:56:27.393Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.393Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT tables in migrations for type safety when database schema stability is critical
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/turso_remote_test.exs : Turso remote tests should be skipped by default if credentials are missing and only run when explicitly needed due to cost implications
Applied to files:
test/savepoint_replication_test.exs
🧬 Code graph analysis (2)
test/cursor_streaming_large_test.exs (1)
lib/ecto_libsql.ex (6)
handle_execute(148-188)disconnect(131-133)handle_declare(466-484)handle_fetch(415-442)handle_begin(312-317)handle_rollback(350-358)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (6)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)rollback(870-874)
⏰ 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 (15)
test/savepoint_replication_test.exs (3)
1-12: LGTM: Clear documentation following British English standards.The module documentation clearly articulates the test focus areas and credential requirements, using proper British English spelling ("behaviour").
74-203: Well-structured test coverage for savepoint scenarios with replication.The three tests comprehensively cover the essential savepoint behaviors: basic operations, rollback preservation of outer transactions, and nested savepoint handling. The test logic correctly uses the original connection state for verification queries after commit.
206-261: Excellent error recovery pattern with correct state handling.This test demonstrates the proper error recovery pattern using savepoints. The critical rebinding of
trx_statefrom the error tuple (line 234) is correctly implemented, allowing the transaction to continue after rolling back to the savepoint. This follows the error recovery pattern described in the learnings.Based on learnings, savepoints enable error recovery patterns in batch operations.
test/cursor_streaming_large_test.exs (6)
1-34: Excellent test setup and isolation strategy.The use of an in-memory database with proper cleanup in
on_exitensures test isolation. The table schema appropriately supports the various test scenarios (filtering, ordering, binary data, etc.).
36-319: Comprehensive coverage of cursor streaming scenarios.This test suite thoroughly validates cursor streaming across diverse SQL operations: large result sets, varied batch sizes, filters, joins, aggregations, computed columns, and query lifecycle. The progression from simple to complex queries demonstrates robust testing methodology.
321-492: Thorough error handling and edge case validation.The error handling tests properly use pattern matching on
{:error, _reason, _state}tuples, and the edge case coverage (NULL values, empty strings, mixed scenarios) aligns excellently with the testing requirements. The distinction between NULL and empty strings is particularly well-validated.
495-560: Excellent transaction rollback and isolation coverage.The transaction tests properly validate cursor behaviour after rollback (with graceful handling of multiple possible outcomes) and verify that cursors see uncommitted changes within the same transaction. This aligns with the transaction rollback scenario requirements from the learnings.
562-676: Comprehensive concurrent operation validation.The concurrent cursor tests thoroughly validate multiple cursors with interleaved fetches, separate queries, and cursor isolation from data modifications. This meets the concurrent operations requirement from the learnings and demonstrates robust multi-cursor handling.
682-802: Well-designed helper functions with efficiency optimisations.The helper functions demonstrate excellent engineering:
insert_rowsuses prepared statements to reduce overhead (lines 684-688, cleanup at 706)- Accumulator patterns in
fetch_all_ids(736-755) andfetch_all_cursor_rows(757-774) avoid O(n²) list concatenation- Pattern matching on
{:ok, _}at line 694 is acceptable for test simplicity per the learnings- Semantic alias functions (776-791) improve readability
test/pool_load_test.exs (6)
45-91: LGTM!The helper functions provide good coverage of edge cases including NULL values, empty strings, large strings, special characters, and various Unicode encodings. The separation of concerns is clear.
311-400: LGTM!The long-running operation tests properly handle transactions with explicit commit result checking and appropriate timeouts. The error handling using
casestatements andflunk/1provides clear failure messages.
402-576: LGTM!The connection recovery tests comprehensively verify that connections remain functional after query errors. The use of pin operators to confirm state consistency is a good practice.
578-721: LGTM!The resource cleanup tests properly verify prepared statement lifecycle management under concurrent load, including creation, execution with edge-case data, and explicit cleanup via
close_stmt.
873-1101: LGTM!The concurrent load edge case tests provide excellent coverage including NULL-only inserts, empty strings, large datasets (500 rows), and type conversion with boundary values. The extended timeouts for large dataset tests are appropriate.
1103-1370: Transaction rollback tests provide good coverage.The rollback tests comprehensively cover: pure rollback scenarios, mixed commit/rollback patterns, constraint violation handling, and edge-case data rollback. The test at lines 1230-1313 has an error tuple pattern issue (flagged separately), but the overall structure is sound.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
test/pool_load_test.exs (3)
291-308: Consider more specific Unicode data verification.Line 307 only verifies that some rows exist (
length(verify_result.rows) > 0), but doesn't confirm the Unicode characters are correctly preserved. Consider asserting a specific count or verifying that specific Unicode patterns were retrieved correctly.♻️ More specific verification
- # Should retrieve some of the Unicode values - # (exact count depends on LIKE behavior with Unicode) - assert length(verify_result.rows) > 0 + # Should retrieve rows matching Unicode patterns + # Expected: 5 tasks × at least 4 matching patterns = at least 20 rows + [[count]] = verify_result.rows + assert count >= 20, "Expected at least 20 rows with Unicode patterns, got #{count}"
816-826: Consider using Enum.reduce for cleaner state threading.The current pattern collects all intermediate states into a list and then takes the last one. This works but is somewhat awkward. Using
Enum.reducewould more clearly express the intent of threading state through multiple operations.♻️ Cleaner state threading with Enum.reduce
- # Insert edge-case values within transaction - edge_values = generate_edge_case_values(task_num) - - insert_results = - Enum.map(edge_values, fn value -> - {:ok, _query, _result, new_state} = insert_edge_case_value(trx_state, value) - new_state - end) - - # Use final state after all inserts - final_trx_state = List.last(insert_results) || trx_state + # Insert edge-case values within transaction, threading state through + edge_values = generate_edge_case_values(task_num) + + final_trx_state = + Enum.reduce(edge_values, trx_state, fn value, acc_state -> + {:ok, _query, _result, new_state} = insert_edge_case_value(acc_state, value) + new_state + end)
1347-1356: State should be threaded through transaction operations.Lines 1350-1353 reuse the same
trx_statefor all inserts without threading the updated state through. Whilst this works in a rollback scenario, it's not following best practices for state management in Elixir.♻️ Thread state through inserts
# Insert edge-case values in transaction edge_values = generate_edge_case_values(task_num) - _insert_results = - Enum.map(edge_values, fn value -> - insert_edge_case_value(trx_state, value) + final_trx_state = + Enum.reduce(edge_values, trx_state, fn value, acc_state -> + {:ok, _query, _result, new_state} = insert_edge_case_value(acc_state, value) + new_state end) # Always rollback - edge-case data should not persist - case EctoLibSql.Native.rollback(trx_state) do + case EctoLibSql.Native.rollback(final_trx_state) do
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/pool_load_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/pool_load_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/pool_load_test.exs
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/pool_load_test.exs
🧬 Code graph analysis (1)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (6)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)rollback(870-874)
⏰ 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)
test/pool_load_test.exs (7)
1-39: LGTM! Excellent test setup and documentation.The module documentation clearly outlines critical scenarios, and the setup properly creates isolated test databases with cleanup via
TestHelpers.cleanup_db_files. The use ofon_exitensures resources are cleaned up even if tests fail.
45-91: LGTM! Comprehensive edge case helpers.The helper functions provide excellent coverage of edge cases including NULL values, empty strings, large datasets, and Unicode data (Chinese, Arabic, emoji). This aligns well with testing requirements.
311-400: LGTM! Robust long-running operation tests.The tests properly handle transaction duration scenarios with explicit error handling (lines 361-367, 377-388) and clear failure messages using
flunk. The pattern of explicitly matching commit results is excellent for debugging.
402-576: LGTM! Thorough connection recovery testing.The tests comprehensively verify that connections remain functional after errors, including with edge-case data (NULL, empty strings, large values). The pattern of explicit error matching and recovery verification (lines 531-542) ensures robust error handling.
578-738: LGTM! Excellent prepared statement resource management.The tests properly verify prepared statement cleanup under concurrent load with defensive error handling (lines 606-610, 686-690). Testing with edge-case data ensures the prepared statement path handles NULL and special values correctly.
889-1117: LGTM! Comprehensive concurrent load edge case testing.Excellent coverage of edge cases under concurrent load:
- NULL values vs empty strings properly distinguished (lines 971-982)
- Large datasets (500 rows) validate scalability
- Type conversions verify data integrity across int/float/text types
- Maximum value boundaries tested (line 1079)
This thoroughly addresses the edge case testing requirements.
1246-1329: LGTM! Excellent constraint violation handling.The test properly verifies that rollback occurs after constraint violations (lines 1295-1299), and confirms that only the original row persists (line 1328). This validates proper error handling and transactional integrity.
- cursor_streaming_large_test.exs: 'behavior' -> 'behaviour' - pool_load_test.exs: Extract COUNT(*) value from result.rows properly - pool_load_test.exs: Fix error tuple from 4-tuple to 3-tuple pattern
- Verify specific Unicode patterns (café, 中文, العربية, emoji) are preserved - Use Enum.reduce for cleaner state threading in concurrent transactions - Thread state properly through transaction operations before rollback
- Use || instead of 'or' for idiomatic boolean operations - Consolidate cleanup logic into single for loop with try/rescue - Add sync verification via max_write_replication_index after commits - Assert specific constraint violation error reason
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
test/savepoint_replication_test.exs (1)
1-277: Consider expanding test coverage for edge cases.The current tests effectively cover savepoint replication behaviour with happy paths, constraint violations, and rollback scenarios. However, per the coding guidelines, consider adding test cases for:
- NULL values in savepoint operations (e.g., inserting NULL into TEXT columns)
- Empty strings in replicated transactions
- Type conversions during savepoint operations
- Large datasets to verify replication frame handling under load
These additions would provide more comprehensive coverage of edge cases that could surface issues in production replication scenarios.
As per coding guidelines, tests should cover edge cases including NULL values, empty strings, large datasets, type conversions, and concurrent operations.
test/cursor_streaming_large_test.exs (1)
759-774: Consider matching the efficiency pattern offetch_all_ids_acc.The
fetch_all_cursor_rows_accfunction usesEnum.reverse(result.rows) ++ accon each iteration (line 768, 772), which is correct but less efficient than the nested list pattern used infetch_all_ids_acc(lines 744-755). For consistency and a slight performance improvement, consider collecting batches as nested lists (like[result.rows | acc]) and flattening once at the end.♻️ Optional refactor to match fetch_all_ids_acc pattern
defp fetch_all_cursor_rows_acc(state, cursor, query, opts, acc) do case EctoLibSql.handle_fetch(query, cursor, opts, state) do {:cont, result, next_state} -> - # Prepend reversed batch to accumulator to maintain order - new_acc = Enum.reverse(result.rows) ++ acc - fetch_all_cursor_rows_acc(next_state, cursor, query, opts, new_acc) + # Collect batches as nested lists to avoid intermediate reversals + fetch_all_cursor_rows_acc(next_state, cursor, query, opts, [result.rows | acc]) {:halt, result, _state} -> - Enum.reverse(result.rows) ++ acc + [result.rows | acc] |> Enum.reverse() |> List.flatten() end endThen update the calling function:
defp fetch_all_cursor_rows(state, cursor, query, opts) do fetch_all_cursor_rows_acc(state, cursor, query, opts, []) - |> Enum.reverse() endtest/pool_load_test.exs (1)
242-245: Consider handling the DELETE result for test robustness.While it's acceptable to skip error handling in test setup, capturing the result might help diagnose issues if the cleanup operation fails:
♻️ Optional improvement
- {:ok, state} = EctoLibSql.connect(database: test_db, busy_timeout: 30_000) - EctoLibSql.handle_execute("DELETE FROM test_data", [], [], state) - EctoLibSql.disconnect([], state) + {:ok, state} = EctoLibSql.connect(database: test_db, busy_timeout: 30_000) + {:ok, _query, _result, _state} = EctoLibSql.handle_execute("DELETE FROM test_data", [], [], state) + EctoLibSql.disconnect([], state)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
test/cursor_streaming_large_test.exstest/pool_load_test.exstest/savepoint_replication_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/cursor_streaming_large_test.exstest/savepoint_replication_test.exstest/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/cursor_streaming_large_test.exstest/savepoint_replication_test.exstest/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/cursor_streaming_large_test.exstest/savepoint_replication_test.exstest/pool_load_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/cursor_streaming_large_test.exstest/savepoint_replication_test.exstest/pool_load_test.exs
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use cursor streaming for large result sets to avoid loading entire datasets into memory
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/cursor_streaming_large_test.exstest/savepoint_replication_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/cursor_streaming_large_test.exstest/savepoint_replication_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/cursor_streaming_large_test.exstest/savepoint_replication_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/savepoint_replication_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Always format Elixir code before committing using `mix format --check-formatted`
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*.exs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-08T04:56:27.393Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.393Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT tables in migrations for type safety when database schema stability is critical
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/turso_remote_test.exs : Turso remote tests should be skipped by default if credentials are missing and only run when explicitly needed due to cost implications
Applied to files:
test/savepoint_replication_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/savepoint_replication_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/savepoint_replication_test.exstest/pool_load_test.exs
🧬 Code graph analysis (1)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (6)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)rollback(870-874)
⏰ 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 (15)
test/savepoint_replication_test.exs (3)
1-19: LGTM! Excellent test documentation and skip logic.The module documentation clearly describes the test scenarios using proper British English ("behaviour"), and the skip logic correctly prevents tests from running when Turso credentials are missing, aligning with the guideline to skip remote tests by default when credentials are unavailable.
21-68: Solid setup and cleanup pattern.The setup correctly creates unique databases and tables per test for isolation, and the cleanup logic properly orders operations (drop table → disconnect → cleanup files) with error handling to ensure cleanup never blocks subsequent tests.
213-275: Excellent demonstration of savepoint error recovery pattern.This test correctly demonstrates the error recovery pattern using savepoints in batch operations. The pattern matching on line 243 properly captures the updated transaction state from the error tuple (
{:error, reason, trx_state}), enabling recovery via rollback to savepoint. This aligns with the learning about using savepoints for partial rollback within transactions.test/cursor_streaming_large_test.exs (5)
1-34: LGTM! Excellent test setup.The module structure and setup block are well-designed. The in-memory database with proper cleanup on exit, clear comments explaining test intent, and appropriate use of British English ("behaviour" in line 494) all follow best practices.
36-319: LGTM! Comprehensive streaming test coverage.This test suite excellently validates cursor streaming behaviour across varied scenarios including large datasets (1K, 10K rows), batch size handling, filtering, JOINs, aggregations, and computed columns. The tests properly verify memory-conscious streaming with batched fetches, which aligns with the learning that cursor streaming should be used for large result sets to avoid loading entire datasets into memory.
Based on learnings, these tests appropriately cover the happy path with successful operations across multiple SQL constructs.
321-492: LGTM! Thorough error handling and edge case coverage.The error handling tests properly validate malformed SQL, syntax errors, and non-existent tables with appropriate pattern matching on
{:error, _reason, _state}tuples. Edge case coverage for NULL values, empty strings, and their combinations is excellent, meeting the learnings requirement for comprehensive edge case testing including NULL values and empty strings.Based on learnings, this properly handles database errors with pattern matching on error tuples rather than using bare unwrap operations.
494-560: LGTM! Robust transaction rollback testing.These tests properly validate cursor behaviour within transactions and after rollback, meeting the learnings requirement for transaction rollback scenario coverage. The defensive handling of multiple possible outcomes at lines 519-531 demonstrates good testing practice by accommodating different valid implementation behaviours.
562-676: LGTM! Excellent concurrent operations coverage.The concurrent cursor tests thoroughly validate multiple cursors with interleaved fetches, separate cursors for concurrent tasks, and cursor isolation when data is modified. This meets the learnings requirement for concurrent operations testing and demonstrates proper handling of cursor state across concurrent scenarios.
test/pool_load_test.exs (7)
1-39: LGTM! Well-structured test module with proper setup and cleanup.The module documentation clearly outlines critical scenarios, and the setup block properly creates isolated test databases with unique names to prevent conflicts. The use of
on_exitwithTestHelpers.cleanup_db_filesensures proper resource cleanup.
45-92: LGTM! Comprehensive edge case and Unicode data generators.The helper functions provide excellent coverage of edge cases including NULL values, empty strings, large strings (1KB), special characters, and various Unicode character sets (Latin with accents, Chinese, Arabic, emoji, and mixed). This aligns well with the testing requirements.
310-399: LGTM! Well-designed long-running transaction tests.The tests appropriately validate that long-running operations don't cause timeout issues. The shorter
busy_timeout: 5000in the first test appears intentional for testing timeout behavior, while the concurrent transaction test includes robust error handling with explicitflunkon commit failures.
401-575: LGTM! Comprehensive connection recovery tests.The tests thoroughly validate that connections remain functional after SQL errors. Proper use of pattern matching with the pin operator (
^state) ensures state immutability after errors, and the concurrent recovery test demonstrates that multiple connections can recover independently. Excellent coverage of edge cases including NULL values.
577-737: LGTM! Excellent resource cleanup patterns.The tests demonstrate proper resource management with nested
try/afterblocks ensuring prepared statements are always closed, even rescuing potential errors during cleanup. The edge-case test thoroughly validates NULL value handling and proper data persistence. This is a model for resource cleanup under concurrent load.
739-1113: LGTM! Comprehensive coverage of transaction isolation and edge cases.The tests thoroughly validate transaction isolation under concurrent load and cover extensive edge cases including NULL-only data, empty strings, large datasets (500 rows), and type conversions with boundary values (max int64, max float64). The 60-second timeout for the large dataset test is appropriately sized for the workload.
1115-1383: LGTM! Thorough transaction rollback coverage.The tests comprehensively validate rollback behaviour under concurrent load, including:
- Concurrent rollbacks leaving no data
- Mixed commit/rollback scenarios with proper counting verification
- Rollback after constraint violations (UNIQUE constraint)
- Rollback with edge-case data
The DELETE operations in test setup (lines 1121, 1179, 1332) without error checking are acceptable for test initialisation. This aligns perfectly with the requirement to test transaction rollback scenarios.
Based on learnings, the tests thoroughly cover all required scenarios.
- Refactor fetch_all_cursor_rows_acc to use nested list pattern (matching fetch_all_ids_acc) for better performance - Add pattern match on DELETE result in pool_load_test cleanup for earlier failure detection
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @test/cursor_streaming_large_test.exs:
- Around line 744-755: The helper loop fetch_all_ids_acc/5 currently only
handles {:cont, result, next_state} and {:halt, result, _state} and will raise a
CaseClauseError on {:error, reason, state}; update fetch_all_ids_acc/5 (and the
other helpers fetch_all_cursor_rows_acc/5 and count_batches/4) to explicitly
match {:error, reason, state} and call ExUnit.flunk/1 (or flunk/1) with a
descriptive message that includes the reason (and optionally state) so test
failures are reported clearly instead of raising a case-clause error. Ensure the
new clause returns or halts appropriately after flunking.
- Around line 682-708: The insert_rows function can leak the prepared statement
if an insert raises mid-loop; wrap the loop that calls
EctoLibSql.Native.execute_stmt in a try...after (or try do ... rescue ... after)
so that EctoLibSql.Native.close_stmt(stmt) always runs regardless of errors,
e.g., prepare the statement with EctoLibSql.Native.prepare, then in a try do
perform the Enum.reduce/Enum.each executing execute_stmt, and in the after block
call EctoLibSql.Native.close_stmt(stmt) to ensure cleanup even on failure.
- Around line 9-34: The on_exit currently closes using the original setup
`state`, which can become stale; instead extract and store the connection
identifier from the initial `state` (e.g., `conn_id = state.conn_id` or
pattern-match the tuple returned by EctoLibSql.connect) and call
EctoLibSql.disconnect([conn_id]) inside on_exit so you always teardown the
specific connection created by setup (update the `setup` block to return the
conn_id and use that in on_exit rather than the entire `state` variable).
In @test/pool_load_test.exs:
- Around line 19-39: The test setup ignores the updated connection state
returned by EctoLibSql.handle_execute/4 and later calls EctoLibSql.disconnect/2
with the original state; capture the fourth element from handle_execute/4 (e.g.
pattern match {:ok, _query, _result, state} or rename to new_state) and use that
updated state in the on_exit callback when calling EctoLibSql.disconnect([],
state) so disconnect gets the latest connection state.
🧹 Nitpick comments (3)
test/cursor_streaming_large_test.exs (1)
650-675: “Cursor isolation” assertion doesn’t assert isolation.
assert total_fetched >= 50will pass even if the cursor is badly broken; it also doesn’t distinguish “snapshot” vs “sees new rows” semantics. Consider asserting an allowed range (e.g.,100..200) or splitting into two explicit expectations depending on intended behaviour.test/pool_load_test.exs (2)
75-91: Minor DRY: duplicated insert helpers can be one function.
insert_unicode_edge_case_value/2andinsert_edge_case_value/2are identical; consider keeping just one.
93-311: Consider enabling WAL mode in the test setup to reduce flakiness under concurrent load.File-based SQLite benefits significantly from Write-Ahead Logging when handling simultaneous writers. The current setup relies solely on per-connection
busy_timeout(30,000 ms), which doesn't address the underlying lock contention with multiple concurrent writers. Enable WAL mode in the setup block usingEctoLibSql.Pragma.set_journal_mode(state, :wal)before test table creation, and optionally pair it withEctoLibSql.Pragma.set_synchronous(state, :normal)for the test environment. This approach is already used intest/ecto_sql_transaction_compat_test.exsand aligns with the codebase's concurrency patterns.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/cursor_streaming_large_test.exstest/pool_load_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use cursor streaming for large result sets to avoid loading entire datasets into memory
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/cursor_streaming_large_test.exstest/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/pool_load_test.exs
🧬 Code graph analysis (1)
test/cursor_streaming_large_test.exs (2)
lib/ecto_libsql.ex (6)
handle_execute(148-188)disconnect(131-133)handle_declare(466-484)handle_fetch(415-442)handle_begin(312-317)handle_rollback(350-358)lib/ecto_libsql/native.ex (3)
prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)
⏰ 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: Rust Fuzz Testing
- 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 (1)
test/pool_load_test.exs (1)
2-14: Module doc looks good (AU spelling, clear intent).This reads well and matches the stated test scope.
cursor_streaming_large_test.exs: - Add error handling to fetch_all_ids_acc, fetch_all_cursor_rows_acc, and count_batches to report failures clearly instead of CaseClauseError - Wrap insert_rows in try/after to ensure prepared statement cleanup on error - Fix on_exit to use conn_id instead of potentially stale state pool_load_test.exs: - Capture updated state from handle_execute and use conn_id in on_exit for reliable connection cleanup
- Use proper EctoLibSql.State struct in on_exit disconnect calls - Replace List.flatten with Enum.concat for single-level flattening in fetch_all_cursor_rows and fetch_all_ids helpers (List.flatten was incorrectly flattening row data like [[1, 'a'], [2, 'b']] into [1, 'a', 2, 'b'])
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @test/cursor_streaming_large_test.exs:
- Around line 762-782: fetch_all_cursor_rows/4 currently reverses the
accumulated batches then calls List.flatten/1 which deep-flattens each row;
instead preserve row structure by concatenating only the outer batch level.
Change fetch_all_cursor_rows to reverse the accumulator from
fetch_all_cursor_rows_acc and then use a one-level concat (e.g. Enum.concat or
similar) rather than List.flatten so callers still receive [[id, value], ...];
update references to fetch_all_cursor_rows and fetch_all_cursor_rows_acc
accordingly.
- Around line 684-812: The helper functions (insert_rows/4, fetch_all_rows/4,
fetch_remaining_rows/4, fetch_all_ids/4, fetch_all_ids_acc/5,
fetch_all_cursor_rows/4, fetch_all_cursor_rows_acc/5, fetch_all_binary_rows/4,
fetch_all_computed_rows/4, fetch_all_group_rows/4, fetch_all_distinct_rows/4,
count_batches/4) follow Elixir snake_case; align the repo guideline to the
language convention by updating the style guide to require snake_case for Elixir
helpers (or, if the project truly prefers camelCase, rename these functions
consistently across the test suite and any callers)—pick one approach and make
the naming consistent across the codebase and the guideline.
- Around line 30-33: The on_exit teardown currently passes a plain map to
EctoLibSql.disconnect which expects a %EctoLibSql.State{} struct; replace the
map argument with an actual %EctoLibSql.State{conn_id: conn_id} (or obtain the
real state used in the test) so the pattern match in EctoLibSql.disconnect/2
succeeds and teardown won't crash.
In @test/pool_load_test.exs:
- Around line 48-95: The helper names use snake_case and duplicate logic: rename
generate_edge_case_values/1 -> generateEdgeCaseValues/1 and
generate_unicode_edge_case_values/1 -> generateUnicodeEdgeCaseValues/1 to follow
the camelCase guideline, and consolidate the identical insert_edge_case_value/2
and insert_unicode_edge_case_value/2 into a single insertEdgeCaseValue/2 (or
similarly camelCased) function; replace all callers to use the new camelCase
names and the single insert function so there’s no duplicated implementation.
- Around line 316-405: The test names claim “timeout issues” but the bodies only
sleep inside transactions and assert commits; either rename the two tests to
reflect they check long-held transactions/commit behavior (change "long
transaction doesn't cause timeout issues" to e.g. "long transaction doesn't
prevent commit" and change "multiple concurrent transactions complete despite
duration" to e.g. "multiple concurrent transactions complete despite holding"),
or if you actually want to test timeout behavior, modify the tests to set a
small busy_timeout on the connection and perform a DB-blocking operation that
exceeds that timeout (then assert the connection returns a timeout/error),
updating the test bodies around EctoLibSql.connect, EctoLibSql.Native.begin, and
EctoLibSql.handle_execute accordingly.
- Around line 19-42: The setup opens a DB connection and leaves it open until
on_exit, skewing load tests and misusing disconnect/2 by passing a map; after
creating the schema with EctoLibSql.handle_execute in setup, call
EctoLibSql.disconnect([], state) immediately to close the connection, and change
the on_exit callback to only call
EctoLibSql.TestHelpers.cleanup_db_files(test_db); remove or ignore the
%{conn_id: conn_id} disconnect and ensure you pass the full state struct
(variable state) to disconnect when closing the connection.
- Around line 1125-1130: The test is discarding results from DELETE operations
and ignoring EctoLibSql.rollback results, which can mask adapter failures and
leave transactions open; update each call to EctoLibSql.handle_execute("DELETE
FROM test_data", [], [], state) to capture and assert its result (e.g.,
pattern-match on {:ok, _} or assert success) instead of discarding it, and in
the constraint-violation test capture the result of EctoLibSql.rollback (in both
case branches) and assert it succeeds (or handle {:error, reason} explicitly),
ensuring you also propagate or handle any error so transactions are not left
open; locate these calls by the function names EctoLibSql.handle_execute and
EctoLibSql.rollback in test/pool_load_test.exs and replace the ignored calls
with proper pattern matches or assertions.
🧹 Nitpick comments (4)
test/cursor_streaming_large_test.exs (3)
615-650: Test name says “concurrent tasks”, but there’s no concurrency here.This test is currently sequential cursor usage, not Task-based concurrency (Line 615). Either rename it to reflect interleaving/sequential behaviour, or make it genuinely concurrent (likely needing a shared DB that supports multiple connections, not
:memory:per-connection semantics). Based on learnings, concurrency coverage is expected for tests.
54-65: Potentially slow CI: 10k row inserts are per-row NIF calls; consider a transaction and/or a:slowtag.The 10k insert loop (Line 54 onward; implemented in
insert_rows/4Lines 684-710) may dominate runtime. Wrapping inserts in a transaction (or reducing dataset sizes while keeping cursor-batching assertions) will usually help.Also applies to: 684-710
712-736: Duplicate helpers:fetch_all_rows/4andfetch_remaining_rows/4are identical.You can keep one and alias the other, or parameterise the small behaviour difference if you intended one.
test/pool_load_test.exs (1)
96-314: Consider threading state updates through sequential operations usingEnum.reducerather thanEnum.mapfor consistency with adapter semantics.While the current code functions correctly (no transactions are active, so state updates are identity operations), the pattern of ignoring returned state from
handle_execute/4is fragile. The adapter contract specifies thathandle_execute/4returns{:ok, query, result, updated_state}or{:error, error, updated_state}, and state carries mutable fields (trx_id,mode,sync) beyondconn_id. If the connection state ever needs to track mutable changes (such as when transactions are introduced), this pattern will fail silently. For example, line 825 elsewhere in the test file demonstrates the correct pattern usingEnum.reduceto thread state through sequential operations. Applying the same pattern here would make the code more robust and consistent with the rest of the codebase.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/cursor_streaming_large_test.exstest/pool_load_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/pool_load_test.exstest/cursor_streaming_large_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use cursor streaming for large result sets to avoid loading entire datasets into memory
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
test/cursor_streaming_large_test.exs
🧬 Code graph analysis (1)
test/cursor_streaming_large_test.exs (1)
lib/ecto_libsql.ex (6)
handle_execute(148-188)disconnect(131-133)handle_declare(466-484)handle_fetch(415-442)handle_begin(312-317)handle_rollback(350-358)
⏰ 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 (2)
test/pool_load_test.exs (2)
1-18: Good test intent and clear module-level scope.
The moduledoc reads well and uses “behaviour” (AU spelling) consistently.
407-450: The state assertion with^stateis correct.The API contract in
handle_executeexplicitly guarantees that on error, the identical state is returned. All error paths in the implementation—whether throughdo_query,do_execute_with_trx, orformat_query_result—return the original state unchanged. Pinning with the pin operator (^state) is therefore the right approach and validates the expected API contract.Likely an incorrect or invalid review comment.
| defp insert_rows(state, start_id, end_id, batch_id) do | ||
| # Use a prepared statement to reduce overhead per insert | ||
| {:ok, stmt} = | ||
| EctoLibSql.Native.prepare( | ||
| state, | ||
| "INSERT INTO large_data (id, batch_id, sequence, value) VALUES (?, ?, ?, ?)" | ||
| ) | ||
|
|
||
| try do | ||
| Enum.reduce(start_id..end_id, state, fn id, acc_state -> | ||
| value = "value_#{id}_batch_#{batch_id}" | ||
|
|
||
| {:ok, _changes} = | ||
| EctoLibSql.Native.execute_stmt( | ||
| acc_state, | ||
| stmt, | ||
| "INSERT INTO large_data (id, batch_id, sequence, value) VALUES (?, ?, ?, ?)", | ||
| [id, batch_id, id - start_id + 1, value] | ||
| ) | ||
|
|
||
| acc_state | ||
| end) | ||
| after | ||
| # Always clean up prepared statement, even on error | ||
| EctoLibSql.Native.close_stmt(stmt) | ||
| end | ||
| end | ||
|
|
||
| defp fetch_all_rows(state, cursor, query, opts) do | ||
| case EctoLibSql.handle_fetch(query, cursor, opts, state) do | ||
| {:cont, result, next_state} -> | ||
| result.num_rows + fetch_all_rows(next_state, cursor, query, opts) | ||
|
|
||
| {:halt, result, _state} -> | ||
| result.num_rows | ||
|
|
||
| {:error, reason, _state} -> | ||
| flunk("Cursor fetch failed with error: #{inspect(reason)}") | ||
| end | ||
| end | ||
|
|
||
| defp fetch_remaining_rows(state, cursor, query, opts) do | ||
| case EctoLibSql.handle_fetch(query, cursor, opts, state) do | ||
| {:cont, result, next_state} -> | ||
| result.num_rows + fetch_remaining_rows(next_state, cursor, query, opts) | ||
|
|
||
| {:halt, result, _state} -> | ||
| result.num_rows | ||
|
|
||
| {:error, reason, _state} -> | ||
| flunk("Cursor fetch failed with error: #{inspect(reason)}") | ||
| end | ||
| end | ||
|
|
||
| defp fetch_all_ids(state, cursor, query, opts) do | ||
| # Use accumulator to avoid O(n²) list concatenation. | ||
| # Collect batches in reverse order, then flatten with nested reverses for correctness. | ||
| fetch_all_ids_acc(state, cursor, query, opts, []) | ||
| |> Enum.reverse() | ||
| |> List.flatten() | ||
| end | ||
|
|
||
| defp fetch_all_ids_acc(state, cursor, query, opts, acc) do | ||
| case EctoLibSql.handle_fetch(query, cursor, opts, state) do | ||
| {:cont, result, next_state} -> | ||
| ids = Enum.map(result.rows, fn [id] -> id end) | ||
| # Collect batches as nested lists to avoid intermediate reversals | ||
| fetch_all_ids_acc(next_state, cursor, query, opts, [ids | acc]) | ||
|
|
||
| {:halt, result, _state} -> | ||
| ids = Enum.map(result.rows, fn [id] -> id end) | ||
| [ids | acc] | ||
|
|
||
| {:error, reason, _state} -> | ||
| flunk("Cursor fetch failed in fetch_all_ids_acc: #{inspect(reason)}") | ||
| end | ||
| end | ||
|
|
||
| # Generic helper to collect all rows from a cursor by repeatedly fetching batches | ||
| # Uses nested list pattern to avoid O(n²) list concatenation with ++ | ||
| defp fetch_all_cursor_rows(state, cursor, query, opts) do | ||
| fetch_all_cursor_rows_acc(state, cursor, query, opts, []) | ||
| |> Enum.reverse() | ||
| |> List.flatten() | ||
| end | ||
|
|
||
| defp fetch_all_cursor_rows_acc(state, cursor, query, opts, acc) do | ||
| case EctoLibSql.handle_fetch(query, cursor, opts, state) do | ||
| {:cont, result, next_state} -> | ||
| # Collect batches as nested lists to avoid intermediate reversals | ||
| fetch_all_cursor_rows_acc(next_state, cursor, query, opts, [result.rows | acc]) | ||
|
|
||
| {:halt, result, _state} -> | ||
| [result.rows | acc] | ||
|
|
||
| {:error, reason, _state} -> | ||
| flunk("Cursor fetch failed in fetch_all_cursor_rows_acc: #{inspect(reason)}") | ||
| end | ||
| end | ||
|
|
||
| # Aliases for backwards compatibility and semantic clarity | ||
| defp fetch_all_binary_rows(state, cursor, query, opts) do | ||
| fetch_all_cursor_rows(state, cursor, query, opts) | ||
| end | ||
|
|
||
| defp fetch_all_computed_rows(state, cursor, query, opts) do | ||
| fetch_all_cursor_rows(state, cursor, query, opts) | ||
| end | ||
|
|
||
| defp fetch_all_group_rows(state, cursor, query, opts) do | ||
| fetch_all_cursor_rows(state, cursor, query, opts) | ||
| end | ||
|
|
||
| defp fetch_all_distinct_rows(state, cursor, query, opts) do | ||
| fetch_all_cursor_rows(state, cursor, query, opts) | ||
| end | ||
|
|
||
| defp count_batches(state, cursor, query, opts) do | ||
| case EctoLibSql.handle_fetch(query, cursor, opts, state) do | ||
| {:cont, _result, next_state} -> | ||
| 1 + count_batches(next_state, cursor, query, opts) | ||
|
|
||
| {:halt, _result, _state} -> | ||
| 1 | ||
|
|
||
| {:error, reason, _state} -> | ||
| flunk("Cursor fetch failed in count_batches: #{inspect(reason)}") | ||
| end | ||
| end |
There was a problem hiding this comment.
Naming style: helpers use snake_case, but repo guidelines mention camelCase for Elixir functions.
If the guideline is intentional, these helpers (insert_rows/4, fetch_all_rows/4, etc.) don’t comply; otherwise, consider correcting the guideline (Elixir convention is snake_case). As per coding guidelines, please align one way or the other.
🤖 Prompt for AI Agents
In @test/cursor_streaming_large_test.exs around lines 684 - 812, The helper
functions (insert_rows/4, fetch_all_rows/4, fetch_remaining_rows/4,
fetch_all_ids/4, fetch_all_ids_acc/5, fetch_all_cursor_rows/4,
fetch_all_cursor_rows_acc/5, fetch_all_binary_rows/4, fetch_all_computed_rows/4,
fetch_all_group_rows/4, fetch_all_distinct_rows/4, count_batches/4) follow
Elixir snake_case; align the repo guideline to the language convention by
updating the style guide to require snake_case for Elixir helpers (or, if the
project truly prefers camelCase, rename these functions consistently across the
test suite and any callers)—pick one approach and make the naming consistent
across the codebase and the guideline.
| defp generate_edge_case_values(task_num) do | ||
| [ | ||
| # Normal string | ||
| "normal_value_#{task_num}", | ||
| # NULL value | ||
| nil, | ||
| # Empty string | ||
| "", | ||
| # Large string (1KB) | ||
| String.duplicate("x", 1000), | ||
| # Special characters | ||
| "special_chars_!@#$%^&*()_+-=[]{};" | ||
| ] | ||
| end | ||
|
|
||
| defp generate_unicode_edge_case_values(task_num) do | ||
| [ | ||
| # Latin with accents (ê, á, ü) | ||
| "café_#{task_num}", | ||
| # Chinese characters (中文) | ||
| "chinese_中文_#{task_num}", | ||
| # Arabic characters (العربية) | ||
| "arabic_العربية_#{task_num}", | ||
| # Emoji (😀, 🎉, ❤️) | ||
| "emoji_😀🎉❤️_#{task_num}", | ||
| # Mixed: combining all above | ||
| "mixed_café_中文_العربية_😀_#{task_num}" | ||
| ] | ||
| end | ||
|
|
||
| defp insert_unicode_edge_case_value(state, value) do | ||
| EctoLibSql.handle_execute( | ||
| "INSERT INTO test_data (value) VALUES (?)", | ||
| [value], | ||
| [], | ||
| state | ||
| ) | ||
| end | ||
|
|
||
| defp insert_edge_case_value(state, value) do | ||
| EctoLibSql.handle_execute( | ||
| "INSERT INTO test_data (value) VALUES (?)", | ||
| [value], | ||
| [], | ||
| state | ||
| ) | ||
| end | ||
|
|
There was a problem hiding this comment.
Helper naming doesn’t match the repo guideline (camelCase) and there’s easy dedupe.
If that guideline is intentional, generate_edge_case_values/1, generate_unicode_edge_case_values/1, etc. should be renamed. Also insert_unicode_edge_case_value/2 and insert_edge_case_value/2 are identical.
🤖 Prompt for AI Agents
In @test/pool_load_test.exs around lines 48 - 95, The helper names use
snake_case and duplicate logic: rename generate_edge_case_values/1 ->
generateEdgeCaseValues/1 and generate_unicode_edge_case_values/1 ->
generateUnicodeEdgeCaseValues/1 to follow the camelCase guideline, and
consolidate the identical insert_edge_case_value/2 and
insert_unicode_edge_case_value/2 into a single insertEdgeCaseValue/2 (or
similarly camelCased) function; replace all callers to use the new camelCase
names and the single insert function so there’s no duplicated implementation.
| describe "long-running operations" do | ||
| @tag :slow | ||
| @tag :flaky | ||
| test "long transaction doesn't cause timeout issues", %{test_db: test_db} do | ||
| {:ok, state} = EctoLibSql.connect(database: test_db, busy_timeout: 5000) | ||
|
|
||
| try do | ||
| # Start longer transaction | ||
| {:ok, trx_state} = EctoLibSql.Native.begin(state) | ||
|
|
||
| {:ok, _query, _result, trx_state} = | ||
| EctoLibSql.handle_execute( | ||
| "INSERT INTO test_data (value, duration) VALUES (?, ?)", | ||
| ["long", 100], | ||
| [], | ||
| trx_state | ||
| ) | ||
|
|
||
| # Simulate some work | ||
| Process.sleep(100) | ||
|
|
||
| {:ok, _committed_state} = EctoLibSql.Native.commit(trx_state) | ||
| after | ||
| EctoLibSql.disconnect([], state) | ||
| end | ||
| end | ||
|
|
||
| @tag :slow | ||
| @tag :flaky | ||
| test "multiple concurrent transactions complete despite duration", %{test_db: test_db} do | ||
| tasks = | ||
| Enum.map(1..3, fn i -> | ||
| Task.async(fn -> | ||
| {:ok, state} = EctoLibSql.connect(database: test_db, busy_timeout: 30_000) | ||
|
|
||
| try do | ||
| {:ok, trx_state} = EctoLibSql.Native.begin(state) | ||
|
|
||
| {:ok, _query, _result, trx_state} = | ||
| EctoLibSql.handle_execute( | ||
| "INSERT INTO test_data (value) VALUES (?)", | ||
| ["trx_#{i}"], | ||
| [], | ||
| trx_state | ||
| ) | ||
|
|
||
| # Hold transaction | ||
| Process.sleep(50) | ||
|
|
||
| # Explicitly handle commit result to catch errors | ||
| case EctoLibSql.Native.commit(trx_state) do | ||
| {:ok, _committed_state} -> | ||
| {:ok, :committed} | ||
|
|
||
| {:error, reason} -> | ||
| {:error, {:commit_failed, reason}} | ||
| end | ||
| after | ||
| EctoLibSql.disconnect([], state) | ||
| end | ||
| end) | ||
| end) | ||
|
|
||
| results = Task.await_many(tasks, 30_000) | ||
|
|
||
| # All commits should succeed; fail test if any error occurred | ||
| Enum.each(results, fn result -> | ||
| case result do | ||
| {:ok, :committed} -> | ||
| :ok | ||
|
|
||
| {:error, {:commit_failed, reason}} -> | ||
| flunk("Transaction commit failed: #{inspect(reason)}") | ||
|
|
||
| other -> | ||
| flunk("Unexpected result from concurrent transaction: #{inspect(other)}") | ||
| end | ||
| end) | ||
|
|
||
| # Verify all inserts | ||
| {:ok, state} = EctoLibSql.connect(database: test_db, busy_timeout: 30_000) | ||
|
|
||
| {:ok, _query, result, _state} = | ||
| EctoLibSql.handle_execute("SELECT COUNT(*) FROM test_data", [], [], state) | ||
|
|
||
| EctoLibSql.disconnect([], state) | ||
|
|
||
| assert [[3]] = result.rows | ||
| end | ||
| end |
There was a problem hiding this comment.
Test names over-claim “timeout issues” vs what’s exercised.
Process.sleep/1 inside a transaction is fine, but it’s not really a long-running query/timeout scenario. Consider renaming to match what’s asserted, or extend the test to actually cover the timeout behaviour you care about.
🤖 Prompt for AI Agents
In @test/pool_load_test.exs around lines 316 - 405, The test names claim
“timeout issues” but the bodies only sleep inside transactions and assert
commits; either rename the two tests to reflect they check long-held
transactions/commit behavior (change "long transaction doesn't cause timeout
issues" to e.g. "long transaction doesn't prevent commit" and change "multiple
concurrent transactions complete despite duration" to e.g. "multiple concurrent
transactions complete despite holding"), or if you actually want to test timeout
behavior, modify the tests to set a small busy_timeout on the connection and
perform a DB-blocking operation that exceeds that timeout (then assert the
connection returns a timeout/error), updating the test bodies around
EctoLibSql.connect, EctoLibSql.Native.begin, and EctoLibSql.handle_execute
accordingly.
- Change concurrent mutex test from :skip to :flaky (it passes) - Standardize SQLite limitation tests with :sqlite_limitation tag - Add :sqlite_limitation to excluded tags in test_helper.exs - Create issue el-6r5 to document SQLite/PostgreSQL differences Tests affected: - error_handling_test.exs:178 - now :flaky (runs on CI exclude) - ecto_sql_transaction_compat_test.exs:218,228 - :sqlite_limitation - ecto_sql_compatibility_test.exs:86 - :sqlite_limitation
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/cursor_streaming_large_test.exs (1)
784-799: Consider removing or documenting the alias functions.These four alias functions (
fetch_all_binary_rows,fetch_all_computed_rows,fetch_all_group_rows,fetch_all_distinct_rows) are identical wrappers aroundfetch_all_cursor_rows. Whilst the comment mentions "backwards compatibility and semantic clarity", they may add confusion. Consider either:
- Removing them and using
fetch_all_cursor_rowsdirectly- Adding @doc annotations explaining their purpose
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.beads/last-touchedtest/cursor_streaming_large_test.exstest/ecto_sql_compatibility_test.exstest/ecto_sql_transaction_compat_test.exstest/error_handling_test.exstest/pool_load_test.exstest/test_helper.exs
🚧 Files skipped from review as they are similar to previous changes (3)
- test/ecto_sql_transaction_compat_test.exs
- test/pool_load_test.exs
- .beads/last-touched
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/test_helper.exstest/cursor_streaming_large_test.exstest/error_handling_test.exstest/ecto_sql_compatibility_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/test_helper.exstest/cursor_streaming_large_test.exstest/error_handling_test.exstest/ecto_sql_compatibility_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/test_helper.exstest/cursor_streaming_large_test.exstest/error_handling_test.exstest/ecto_sql_compatibility_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/cursor_streaming_large_test.exstest/error_handling_test.exstest/ecto_sql_compatibility_test.exs
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/test_helper.exstest/cursor_streaming_large_test.exstest/error_handling_test.exstest/ecto_sql_compatibility_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/test_helper.exstest/cursor_streaming_large_test.exstest/error_handling_test.exstest/ecto_sql_compatibility_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*.exs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/test_helper.exstest/error_handling_test.exstest/ecto_sql_compatibility_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/turso_remote_test.exs : Turso remote tests should be skipped by default if credentials are missing and only run when explicitly needed due to cost implications
Applied to files:
test/test_helper.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use cursor streaming for large result sets to avoid loading entire datasets into memory
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/cursor_streaming_large_test.exstest/error_handling_test.exstest/ecto_sql_compatibility_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.{ex,exs} : Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to **/*.rs : Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/cursor_streaming_large_test.exs
📚 Learning: 2026-01-08T04:56:27.393Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.393Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT tables in migrations for type safety when database schema stability is critical
Applied to files:
test/ecto_sql_compatibility_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement type dumpers for converting Ecto types to SQLite values using `dumpers/2` function
Applied to files:
test/ecto_sql_compatibility_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
test/ecto_sql_compatibility_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
test/ecto_sql_compatibility_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 (19)
test/error_handling_test.exs (2)
178-179: LGTM! Appropriate tag for concurrency test.Changing from
@tag :skipto@tag :flakyis sensible - this allows the test to run on CI whilst acknowledging its timing-sensitive nature. The test verifies mutex safety under concurrent stress, which is valuable but inherently prone to occasional timing issues.
223-223: LGTM! Clean consolidation of database cleanup.Using the centralised
EctoLibSql.TestHelpers.cleanup_db_files/1helper simplifies cleanup and ensures consistency with other tests in the suite.test/ecto_sql_compatibility_test.exs (2)
54-56: LGTM! Consistent cleanup pattern.The
on_exitcallback now uses the centralised cleanup helper, which handles all associated database files (WAL, SHM, journal, etc.) in one call.
86-88: LGTM! Well-documented limitation.Replacing
@tag :skipwith@tag :sqlite_limitationand adding an explanatory comment provides better documentation of why this test behaves differently. This makes it clear the limitation is inherent to SQLite rather than a bug to fix.test/test_helper.exs (4)
1-5: LGTM! Clear documentation of test exclusion categories.The comments provide good context for why different test categories exist and when they're excluded.
12-19: LGTM! Sensible test exclusion strategy.The exclusion logic appropriately differentiates between CI and local runs, ensuring CI skips flaky tests whilst still running more comprehensive tests than local development.
47-62: LGTM! Well-structured cleanup helper.The function correctly handles the common database auxiliary files. Ignoring
File.rmreturn values is appropriate here since files may not exist (e.g.,-walonly exists in WAL mode), and cleanup failures during tests are rarely critical.
75-85: LGTM! Comprehensive pattern-based cleanup.The two-step approach is sensible:
Path.wildcard(pattern)finds matching DB files, thencleanup_db_files/1handles each and its auxiliary files- The secondary
pattern <> "-*"catches orphaned auxiliary files that may exist without a main DB fileThis provides thorough cleanup for test databases with dynamic names.
test/cursor_streaming_large_test.exs (11)
9-36: LGTM! Clean test setup with proper cleanup.The setup correctly establishes an in-memory database, creates the test table, and ensures disconnection in
on_exit. Using:memory:avoids file cleanup concerns.
38-115: LGTM! Comprehensive streaming tests.Good coverage of cursor streaming fundamentals: varying dataset sizes, batch size enforcement, filtered queries, and ordering verification. The tests properly exercise the cursor lifecycle with
handle_declareandhandle_fetch.
116-161: LGTM! Thorough binary data handling test.The test verifies that BLOB data is correctly streamed through cursors, including preservation of binary size. The 1KB payload per row is a reasonable size to test binary handling without excessive memory usage.
255-320: LGTM! Good SQL feature coverage through cursors.These tests verify that complex SQL operations (GROUP BY, OFFSET/LIMIT, DISTINCT) work correctly when results are streamed through cursors. The assertions validate both row counts and data correctness.
323-374: LGTM! Proper error handling coverage.The tests verify that cursor operations return appropriate error tuples for invalid SQL, syntax errors, and non-existent tables. Empty result sets are also handled correctly. This aligns with the project's error handling guidelines.
376-493: LGTM! Excellent edge case coverage for NULL handling.These tests verify the important distinction between NULL and empty strings, which is a common source of bugs. The test at line 492 correctly expects SQLite's 1/0 representation for boolean IS NULL results.
496-562: LGTM! Transaction behaviour tests are well-structured.The rollback test correctly handles multiple possible outcomes (error, halt, or cont), acknowledging that cursor behaviour after rollback may vary by implementation. The uncommitted changes test verifies read-your-writes within a transaction.
564-678: LGTM! Cursor isolation tests are comprehensive.The tests verify that multiple cursors maintain independent state when interleaved, and that cursors are isolated from concurrent data modifications. Note that these tests run sequentially within the same process, testing logical isolation rather than true parallelism.
738-760: LGTM! Efficient list accumulation pattern.Good use of the accumulator pattern with
Enum.reverse() |> Enum.concat()to avoid O(n²) list concatenation. The comments clearly explain the performance rationale.
225-238: Batch count expectation is correct.The assertion for 11 batches is accurate. The cursor implementation returns
{:halt, result, state}when rows are empty (lib/ecto_libsql.ex line 434), and thecount_batchesfunction counts each fetch result—including the final empty batch—as one batch. With 1000 rows and batch size 100, this yields 10 full batches plus 1 final batch with 0 rows = 11 total.
684-710: No action needed. The code correctly uses the Native module's wrapper API.The
prepare/2function is intentionally designed to accept the State struct, not a raw connection ID. All wrapper functions in the Native module follow this pattern—they accept State as the first parameter, extract theconn_idvia pattern matching, and pass it to the underlying NIF functions. The test code at lines 686–690 correctly passesstatetoprepare/2, and the subsequent calls toexecute_stmt/4andclose_stmt/1are consistent with the established API design.Likely an incorrect or invalid review comment.
- Close setup connection immediately after schema creation - Consolidate duplicate insert_unicode_edge_case_value into insert_edge_case_value - Assert DELETE results instead of discarding them - Handle rollback results properly in constraint violation test
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @test/pool_load_test.exs:
- Around line 682-686: The blanket rescue around the call to
Native.close_stmt(stmt) masks unexpected errors; change it to catch only the
expected exception types (for example rescue e in [ArgumentError, RuntimeError]
-> :ok) or pattern-match on the specific exception(s) your Native.close_stmt/1
can raise, and re-raise or log any other exceptions instead of swallowing them;
ensure the code refers to Native.close_stmt/1 and the stmt variable so
unexpected failures surface for debugging.
🧹 Nitpick comments (1)
test/pool_load_test.exs (1)
815-819: State threading pattern is correct but assumes success.The
Enum.reduce/3pattern correctly threads the transaction state through multiple operations. However, it assumes all inserts succeed (the pattern match{:ok, ...}will raise aMatchErroron failure).This is acceptable for tests where failures should be immediately visible, but consider whether explicit error handling would make test failures clearer:
Optional: Explicit error handling
final_trx_state = Enum.reduce_while(edge_values, {:ok, trx_state}, fn value, {:ok, acc_state} -> case insert_edge_case_value(acc_state, value) do {:ok, _query, _result, new_state} -> {:cont, {:ok, new_state}} {:error, reason, _state} -> {:halt, {:error, reason}} end end) case final_trx_state do {:ok, state} -> # continue with commit {:error, reason} -> # handle error endHowever, the current approach (fail-fast with pattern match) is clearer for tests.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/pool_load_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/pool_load_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/pool_load_test.exs
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement type loaders for converting SQLite values to Ecto types using `loaders/2` function
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/pool_load_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 / macos-latest
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (5)
test/pool_load_test.exs (5)
1-41: Excellent test isolation and cleanup setup!The per-test database creation with unique naming and centralized cleanup using
EctoLibSql.TestHelpers.cleanup_db_files/1provides proper test isolation. The setup connection is correctly closed before tests run.
47-85: Well-structured edge case helpers!The helper functions provide comprehensive coverage of edge cases (NULL, empty strings, large values, Unicode variants) and promote code reuse throughout the test suite. The inline comments clearly document each edge case type.
95-95: Verify timeout configuration is appropriate.The
busy_timeout: 30_000(30 seconds) is used consistently throughout the tests. Whilst this helps avoid spurious failures under load, it may also mask genuine performance issues or deadlock scenarios.Consider:
- Whether this timeout is representative of production configuration
- If tests should also verify operations complete within a more aggressive timeout (e.g., 5 seconds) to catch performance regressions
- Adding a test variant with shorter timeouts to detect lock contention issues earlier
1-1388: Comprehensive test coverage aligning with requirements!This test suite excellently covers all required scenarios per the learnings:
- ✅ Happy path (successful operations)
- ✅ Error cases (syntax errors, constraint violations)
- ✅ Edge cases (NULL values, empty strings, large datasets, Unicode)
- ✅ Transaction rollback scenarios
- ✅ Type conversions (integers, floats, strings, timestamps)
- ✅ Concurrent operations (multiple connections, transactions)
The test structure with helper functions, proper resource cleanup, and consistent error handling patterns demonstrates good test design. The main improvements needed are addressing the flaky test behaviour and narrowing rescue clauses.
Based on learnings, this test suite meets the coverage requirements for test/**/*test.exs files.
88-90: The @flaky tag on these tests is intentional, not indicative of problematic code. Pertest_helper.exs, these tests are deliberately excluded from normal test runs ("excluded by default to avoid CI brittleness"), as they are load/stress tests designed for concurrent access pattern validation. The current implementation already includes proper task synchronisation viaTask.await_many, generous timeouts (30–60 seconds), and resource cleanup with try/after blocks. A recent commit ("fix: improve pool_load_test.exs robustness") indicates flakiness concerns have already been addressed. No changes required.
Two critical improvements to test/pool_load_test.exs: 1. Replace blanket rescue clauses with specific exception handling (lines 602-623, 695-722) - Changed 'rescue _ -> :ok' to pattern match only ArgumentError and RuntimeError - Unexpected exceptions are logged with ERROR level and re-raised for visibility - Expected exceptions are logged with DEBUG level to aid troubleshooting - Makes failures in close_stmt/1 immediately visible for debugging - Added 'require Logger' to module 2. Add explicit error handling to state threading in transaction test (lines 848-879) - Replaced implicit success assumption in Enum.reduce with Enum.reduce_while - Now explicitly handles insert_edge_case_value/2 failures instead of letting MatchError occur - Captures errors with context (insert_failed reason) instead of crashing - Wrapped entire sequence in 'with' clause to handle errors from either inserts or commit - Makes test failures clearer and prevents masking of real issues Benefits: - Unexpected exceptions now surface for debugging instead of being silently swallowed - Test errors are more informative with explicit error capture and context - Code is more maintainable by showing what exceptions are explicitly expected
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @test/pool_load_test.exs:
- Around line 849-864: The reduce_while error clause is pattern-matching a
4-tuple {:error, _query, reason, _state} but the underlying call path
(handle_execute) returns a 3-tuple {:error, reason, state}; update the failing
branch inside Enum.reduce_while (the clause handling insert_edge_case_value
errors) to match the actual 3-tuple (e.g. {:error, reason, _state} or {:error,
reason, state}) and then convert/forward the reason as {:halt, {:error,
{:insert_failed, reason}}} so errors are caught and propagated without causing a
MatchError; reference the reduce block around Enum.reduce_while and the
insert_edge_case_value/handle_execute error return shapes when making the
change.
🧹 Nitpick comments (2)
test/pool_load_test.exs (2)
1396-1402: Missing error handling in reduce will crash on insert failure.The pattern match
{:ok, _query, _result, new_state} = insert_edge_case_value(acc_state, value)will raise aMatchErrorif the insert fails. Compare with lines 849-864 which usesreduce_whilewith proper error handling. While test code can be more lenient, this inconsistency could cause confusing test failures.♻️ Suggested improvement using reduce_while
- final_trx_state = - Enum.reduce(edge_values, trx_state, fn value, acc_state -> - {:ok, _query, _result, new_state} = insert_edge_case_value(acc_state, value) - new_state - end) + {:ok, final_trx_state} = + Enum.reduce_while(edge_values, {:ok, trx_state}, fn value, {:ok, acc_state} -> + case insert_edge_case_value(acc_state, value) do + {:ok, _query, _result, new_state} -> {:cont, {:ok, new_state}} + {:error, _reason, _state} -> {:halt, {:error, :insert_failed}} + end + end)
602-624: Consider documenting why close_stmt errors are expected.The rescue block treats
ArgumentErrorandRuntimeErroras expected errors when closing prepared statements. If these exceptions are genuinely expected during normal operation, a brief comment explaining the root cause would help maintainers understand this is intentional rather than masking a bug.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/pool_load_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/pool_load_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/pool_load_test.exs
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement type loaders for converting SQLite values to Ecto types using `loaders/2` function
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
test/pool_load_test.exs
🧬 Code graph analysis (1)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (6)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)rollback(870-874)
⏰ 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). (2)
- GitHub Check: Rust Fuzz Testing
- GitHub Check: Analyze (rust)
🔇 Additional comments (6)
test/pool_load_test.exs (6)
1-42: Well-structured test module with proper setup and cleanup.The module documentation clearly outlines critical scenarios, and the setup correctly creates isolated test databases with unique names and proper cleanup via
TestHelpers.cleanup_db_files. The use of "behaviour" follows British/Australian English as per coding guidelines.
44-86: Good coverage of edge-case data generators.The helper functions provide comprehensive test data including NULL values, empty strings, large strings, Unicode (Chinese, Arabic, emoji), and special characters. This aligns well with the testing requirements for edge cases. Based on learnings, tests should cover edge cases like NULL values, empty strings, and large datasets.
88-158: Concurrent connection tests are well-designed.Good use of
try/afterblocks to ensure connection cleanup, appropriatebusy_timeout: 30_000to handle SQLite locking under concurrent load, and proper verification of results via count queries. The flaky/slow tags appropriately signal test characteristics.
1073-1158: Excellent type conversion edge-case coverage.Good testing of boundary values including max int64 (
9_223_372_036_854_775_807), max float64, negative numbers, and zero values. The verification that types are correctly preserved after round-trip through the database is valuable. Based on learnings, this aligns with the requirement to test type conversions.
1160-1217: Transaction rollback tests are comprehensive.Good coverage of rollback scenarios including concurrent rollbacks, mixed commit/rollback patterns, and constraint violation handling. The tests properly verify that rolled-back data does not persist, which is critical for transaction integrity. This aligns with the learning requirement to test transaction rollback scenarios.
1285-1378: Constraint violation rollback test is well-designed.The test properly creates a UNIQUE constraint scenario, attempts to violate it within a transaction, and verifies that rollback correctly discards all changes including the valid insert that preceded the constraint violation. This is important edge-case coverage for transaction atomicity.
Correct the 3-tuple vs 4-tuple mismatch in the reduce_while error branch:
- handle_execute returns {:ok, query, result, state} (4-tuple) on success
- handle_execute returns {:error, reason, state} (3-tuple) on error
- The error clause was incorrectly matching a 4-tuple with {:error, _query, reason, _state}
- Updated to match the actual error return shape: {:error, reason, _state}
This prevents a MatchError when an insert operation fails during the transaction,
allowing the error to be properly caught and propagated as {:halt, {:error, {:insert_failed, reason}}}.
The fix ensures that insert failures are explicitly handled rather than causing
an unhandled pattern match exception.
Apply mix format to correct indentation that was introduced during the previous refactoring of error handling and rescue clauses. No functional changes - formatting only.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @test/pool_load_test.exs:
- Around line 601-624: The current try/rescue around
EctoLibSql.Native.close_stmt(stmt) is incorrect because close_stmt returns :ok
or {:error, reason} instead of raising; replace the try/rescue with a case on
the return value of EctoLibSql.Native.close_stmt(stmt): handle :ok by returning
:ok and handle {:error, _reason} by logging via Logger.debug (include useful
context like inspect(stmt) or the reason) and then return :ok; remove the rescue
block and any exception-specific branches (%ArgumentError, %RuntimeError) since
they won't occur.
🧹 Nitpick comments (2)
test/pool_load_test.exs (2)
237-243: Consider test isolation: this test clears the table while others don't.This test explicitly deletes all rows before running (lines 240-241), whereas other tests in this describe block accumulate data. This creates implicit test ordering dependency - if tests run in parallel or different order, results could vary.
Consider either:
- Clearing the table in
setupso all tests start fresh, or- Using a test-specific value marker to filter only this test's data in assertions
This is a minor concern given the
@tag :flakymarker already indicates awareness of timing sensitivities.
1398-1402: Consider consistent error handling pattern.This uses a simple
Enum.reducewith pattern matching that will crash on insert failure:{:ok, _query, _result, new_state} = insert_edge_case_value(acc_state, value)Compare to the more robust
reduce_whilepattern used in lines 849-864 which handles errors explicitly. While crashing on unexpected errors is acceptable in tests per learnings, the inconsistency may make debugging harder if failures occur.This is a minor observation - the current approach is valid for test code.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/pool_load_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/pool_load_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/pool_load_test.exs
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement type loaders for converting SQLite values to Ecto types using `loaders/2` function
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
test/pool_load_test.exs
🧬 Code graph analysis (1)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (6)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)rollback(870-874)
⏰ 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). (2)
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (8)
test/pool_load_test.exs (8)
1-42: LGTM! Well-structured test setup with proper isolation and cleanup.The setup correctly creates a unique database per test run and uses the centralised
TestHelpers.cleanup_db_files/1for cleanup. The British English "behaviour" in the moduledoc aligns with coding guidelines.
48-86: LGTM! Comprehensive edge-case value generators.Good coverage of edge cases including NULL values, empty strings, large strings (1KB), special characters, and various Unicode scripts (Latin accents, Chinese, Arabic, emoji). This aligns well with the test coverage requirements from the learnings.
308-397: LGTM! Good coverage of long-running transaction scenarios.The tests correctly verify that transactions held open for extended periods (50-100ms) complete successfully under concurrent load. The explicit
casematching on commit results (lines 358-364, 374-385) provides clear error reporting on failures.
399-573: LGTM! Thorough connection recovery testing.Excellent coverage of recovery scenarios including:
- Single connection recovery after syntax errors (lines 402-442)
- Recovery with edge-case data preservation (lines 446-502)
- Concurrent independent recovery (lines 506-572)
The use of pin operator
^state(lines 417, 461) to verify the connection state remains unchanged after errors is a good practice for ensuring connection integrity.
849-879: LGTM! Well-structured state threading with proper error handling.The
reduce_whilepattern (lines 850-864) correctly threads transaction state through multiple inserts while handling errors explicitly. Thewithblock ensures commit only happens if all inserts succeed, and theelseclause properly propagates errors.
1118-1124: Good boundary value testing with max int64 and float64.Testing with
9_223_372_036_854_775_807(max int64) and1.7976931348623157e308(near max float64) is excellent for verifying SQLite handles extreme values correctly through the NIF layer.
1287-1377: Good constraint violation testing, butunique_testtable may persist.The test creates a
unique_testtable (line 1293) for testing UNIQUE constraint violations. This table isn't explicitly dropped in cleanup. Whilecleanup_db_filesremoves the entire database file, it's worth noting for consistency.The constraint violation handling (lines 1336-1353) correctly distinguishes between expected failure and unexpected success scenarios.
88-91: Strong test coverage aligned with requirements.The test suite comprehensively covers the required scenarios from the learnings:
- ✓ Happy path (successful operations)
- ✓ Error cases (syntax errors, constraint violations)
- ✓ Edge cases (NULL, empty strings, large datasets, Unicode)
- ✓ Transaction rollback scenarios
- ✓ Type conversions (int, float, text, timestamp)
- ✓ Concurrent operations
The
@tag :slowand@tag :flakymarkers appropriately indicate test characteristics for CI configuration.
- Fix Unicode assertion counts (café/中文/العربية appear 10× not 5×)
- Add async: false to prevent race conditions
- Fix close_stmt error handling to use case instead of try/rescue
(close_stmt returns :ok/{:error, reason}, doesn't raise)
- Remove inappropriate :slow/:flaky tags from sequential tests
- Add explanatory comments for test categorisation
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @test/pool_load_test.exs:
- Around line 1380-1384: The reduce over edge_values that builds final_trx_state
uses Enum.reduce with a pattern match {:ok, _query, _result, new_state} =
insert_edge_case_value(acc_state, value) which will raise MatchError on
failures; replace this with a defensive Enum.reduce_while (or switch to the same
reduce_while pattern used in the transaction isolation test) that calls
insert_edge_case_value(acc_state, value) and explicitly handles {:ok, _query,
_result, new_state} -> {:cont, new_state} and any {:error, reason} (or
unexpected tuple) -> {:halt, {:error, %{value: value, reason: reason, state:
acc_state}}} so failures return a clear error payload which you can then assert
or raise with a descriptive message before proceeding to use final_trx_state.
🧹 Nitpick comments (1)
test/pool_load_test.exs (1)
1099-1106: Consider potential precision issues with max float value.Using
1.7976931348623157e308(near Float64 max) is excellent edge-case coverage, but be aware this may exhibit platform-specific rounding behaviour. If tests become flaky on specific platforms, consider using a slightly smaller value like1.0e307.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/pool_load_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/pool_load_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/pool_load_test.exs
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement type loaders for converting SQLite values to Ecto types using `loaders/2` function
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/pool_load_test.exs
🧬 Code graph analysis (1)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (6)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)rollback(870-874)
⏰ 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 / ubuntu-latest
- GitHub Check: Analyze (rust)
- GitHub Check: Rust Checks (macos-latest, stable)
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
🔇 Additional comments (12)
test/pool_load_test.exs (12)
1-42: Well-structured test setup with proper isolation and cleanup.The setup correctly creates a unique database per test run, establishes the test schema, and properly delegates cleanup to the centralised helper. The
on_exitcallback ensures resources are cleaned up even if tests fail.
48-86: Good coverage of edge-case data generation helpers.The helpers appropriately cover NULL values, empty strings, large strings, special characters, and comprehensive Unicode (Latin accents, CJK, Arabic, emoji). This aligns well with the test coverage requirements for edge cases. Based on learnings, tests should cover NULL values, empty strings, and large datasets.
299-307: Verify Unicode pattern count assertions.The comments state that
café,中文, andالعربيةappear in both individual patterns and "mixed_..." patterns (10 each), but let me verify:
- 5 tasks × 1 "café_N" = 5
- 5 tasks × 1 "mixed_café_中文_العربية_😀_N" = 5
- Total containing "café" = 10 ✓
The logic appears correct. The
😀🎉❤️sequence only appears in "emoji_..." (not mixed, which has just😀), hence 5. Good edge-case verification.
88-129: Concurrent connection tests are well-structured.The tests properly use
try/afterto ensure connections are closed even on failure, use appropriate timeouts, and verify results with assertions. The:slowand:flakytags appropriately mark these tests for conditional execution.
311-336: Transaction handling pattern is correct.The use of
stateindisconnectafter committingtrx_stateis correct since both share the sameconn_id. The transaction state (trx_state) is just an enriched version of the connection state withtrx_idadded.
402-445: Proper error recovery verification pattern.The use of pin matching (
^state) in error assertions correctly verifies that the connection state is unchanged after query errors, ensuring the connection can continue to be used. This complements the connection_recovery_test.exs by using file-based database.
580-648: Robust resource cleanup pattern with nested try/after.The nested
try/afterblocks ensure both prepared statements and connections are properly cleaned up. The error handling onclose_stmt(logging but not failing) is appropriate since cleanup failures shouldn't mask test results.
830-861: Correct state threading through transactional inserts.The
reduce_whilewithwithpattern properly threads state through sequential inserts within a transaction. This correctly handles both the successful path (accumulating state) and error path (halting with error). The explicit error handling surfaces failures clearly.
912-954: Good edge-case coverage for NULL and empty string handling under concurrent load.These tests verify that NULL values and empty strings are handled correctly under concurrent conditions and that NULL vs empty string semantics are preserved (IS NULL vs = ''). This aligns with the test coverage requirements for edge cases.
1269-1359: Excellent constraint violation rollback test.This test properly verifies transaction rollback behaviour when a unique constraint is violated. The test:
- Sets up a table with a UNIQUE constraint
- Inserts an initial row
- Attempts concurrent transactions that each try to insert a duplicate
- Verifies all transactions correctly roll back
- Confirms only the original row remains
This covers the "constraint violations" requirement from the test coverage learnings.
1203-1265: Good test for mixed commit/rollback consistency.The deterministic even/odd commit/rollback pattern makes the expected outcome clear (5 rows). This effectively tests that commits and rollbacks don't interfere with each other under concurrent conditions.
1-17: Comprehensive pool/load test suite with excellent coverage.This test module provides thorough coverage of concurrent database access patterns including:
- Concurrent independent connections
- Long-running transactions
- Connection recovery after errors
- Resource cleanup under load
- Transaction isolation
- Edge-case data handling (NULL, empty, large, Unicode)
- Transaction rollback scenarios
The documentation uses British English appropriately, and the test structure follows good patterns with proper resource cleanup.
- Replace unsafe max Float64 value (1.7976931348623157e308) with platform-safe 1.0e307 to prevent flaky tests from platform-specific rounding - Replace unsafe Enum.reduce with defensive Enum.reduce_while in concurrent transaction test to gracefully handle insert failures instead of crashing - Add explicit error handling in reduce_while with clear error payloads for debugging - Improves test reliability and diagnostic value
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @test/pool_load_test.exs:
- Around line 434-445: The test assumes an isolated table but asserts exactly 2
rows; to fix, clear the shared test_data table at the start of the test before
inserting: after obtaining the DB connection via EctoLibSql.connect(...) run a
SQL clear (e.g., TRUNCATE or DELETE FROM test_data) using
EctoLibSql.handle_execute/4 so the subsequent inserts are the only rows counted,
then proceed with the existing insert and SELECT COUNT(*) assertion.
🧹 Nitpick comments (1)
test/pool_load_test.exs (1)
234-243: Consider adding table cleanup to all tests in this describe block for consistency.The Unicode test clears the table (line 241), but earlier tests in this describe block (lines 91-230) do not. This creates an implicit execution order dependency. While
async: falseensures sequential execution, addingDELETE FROM test_dataat the start of each test (or in a shared setup) would make each test fully independent and easier to debug in isolation.♻️ Optional: Add cleanup at the start of each test
test "multiple concurrent connections execute successfully", %{test_db: test_db} do + # Clean table for independent test + {:ok, state} = EctoLibSql.connect(database: test_db, busy_timeout: 30_000) + {:ok, _, _, _} = EctoLibSql.handle_execute("DELETE FROM test_data", [], [], state) + EctoLibSql.disconnect([], state) + # Spawn 5 concurrent connections
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/pool_load_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/pool_load_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/pool_load_test.exs
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement type loaders for converting SQLite values to Ecto types using `loaders/2` function
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Applied to files:
test/pool_load_test.exs
🧬 Code graph analysis (1)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (6)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)rollback(870-874)
⏰ 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 (10)
test/pool_load_test.exs (10)
1-42: Well-structured test setup with proper isolation and cleanup.The module correctly uses
async: falseto prevent test interference, creates unique per-test databases, and leverages the centralisedTestHelpers.cleanup_db_files/1for teardown. The setup pattern of creating the table and immediately disconnecting before tests create their own connections is appropriate for simulating independent concurrent access.
48-86: Good coverage of edge-case data scenarios.The helper functions provide comprehensive test data including NULL values, empty strings, large strings (1KB), special characters, and Unicode (Chinese, Arabic, emoji). This aligns well with the testing requirements for edge cases. Based on learnings, tests should cover NULL values, empty strings, and large datasets.
88-129: Solid concurrent connection test with proper cleanup.The test correctly uses
try/afterto ensuredisconnectis called even on failure. Thebusy_timeout: 30_000is appropriate for concurrent SQLite access. The verification query confirms all inserts succeeded.
580-648: Exemplary resource cleanup pattern with nested try/after.The nested
try/afterblocks (lines 586-619) correctly ensure both the prepared statement and the connection are cleaned up, even if the statement execution fails. The graceful handling ofclose_stmterrors (lines 605-615) is appropriate—logging instead of failing prevents test flakiness from cleanup race conditions.The row count assertion on line 647 has the same accumulation issue noted earlier.
830-862: Well-implemented state threading through transactional inserts.The
reduce_whilecombined withwith/else(lines 831-861) correctly threads the transaction state through multiple insert operations while handling errors gracefully. This pattern ensures that if any insert fails, the error is surfaced rather than silently swallowed.
1008-1051: Good stress test with appropriate timeout adjustment.The large dataset test (500 rows across 5 concurrent connections) correctly increases the timeout to 60 seconds on both
connect(line 1014) andTask.await_many(line 1036). This prevents false failures under load.
1053-1140: Type conversion test properly uses isolated table.Creating a separate
typed_datatable (line 1061) for type conversion testing avoids the row count accumulation issues present in other tests. The coverage of integer bounds (including max i64), floats, and timestamps is comprehensive.
1142-1199: Correct approach: clearing table before tests with absolute count assertions.This test properly clears the
test_datatable (lines 1147-1149) before running, ensuring the assertion[[0]] = result.rows(line 1198) is reliable. This pattern should be applied to other tests with absolute count assertions.
1267-1359: Comprehensive constraint violation and rollback test.The test correctly:
- Creates a table with a UNIQUE constraint
- Inserts a baseline row
- Attempts duplicate inserts within transactions
- Verifies rollback succeeds on constraint violation
- Confirms only the original row persists
The case pattern on lines 1318-1334 handles both expected failure and unexpected success paths, ensuring proper cleanup in either case.
1-1433: Overall: Well-structured load test suite with comprehensive coverage.This test file provides excellent coverage of concurrent connection behaviour, including:
- Happy path concurrent operations
- Edge cases (NULL, empty strings, large values, Unicode)
- Transaction isolation and rollback scenarios
- Constraint violation handling
- Resource cleanup under load
The main recurring issue is that several tests assert exact row counts without clearing the
test_datatable first, which can cause failures if tests run in a different order or if the test database persists between runs. The rollback tests (lines 1142+) demonstrate the correct pattern by clearing the table before assertions.Based on learnings, the test coverage aligns well with requirements for happy path, error cases, edge cases, transaction rollback scenarios, type conversions, and concurrent operations.
- Add DELETE FROM test_data before concurrent load tests to ensure exact row count assertions - Affects three tests: 'connection recovers after query error', 'concurrent load with only NULL values', 'concurrent load with only empty strings' - Prevents test failures when previous tests leave residual data in shared database - Ensures each test has a predictable, clean starting state for row count verification
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/pool_load_test.exs (1)
18-18: Redundant alias can be removed.
alias EctoLibSqlis a no-op since the module name doesn't change. This line can be safely removed.Suggested fix
- alias EctoLibSql - setup do
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/pool_load_test.exs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formattedUse camelCase for function names in Elixir (e.g.,
defmodule,def handle_execute)
Files:
test/pool_load_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/pool_load_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/pool_load_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/pool_load_test.exs
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-08T04:56:27.394Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.394Z
Learning: Applies to **/*.ex : All database connection errors and query failures must be handled gracefully with pattern matching on `{:ok, ...}` and `{:error, ...}` tuples - never use bare unwrap operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Use proper error handling patterns with case matching and with clauses to handle both success and error tuples from NIF operations
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
test/pool_load_test.exs
📚 Learning: 2026-01-06T06:36:01.585Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
test/pool_load_test.exs
🧬 Code graph analysis (1)
test/pool_load_test.exs (2)
lib/ecto_libsql.ex (2)
handle_execute(148-188)disconnect(131-133)lib/ecto_libsql/native.ex (6)
begin(828-838)commit(853-857)prepare(951-959)execute_stmt(991-1020)close_stmt(1080-1082)rollback(870-874)
⏰ 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)
test/pool_load_test.exs (7)
48-86: Well-structured edge case helpers.Good coverage of edge cases including NULL, empty strings, large values (1KB), special characters, and comprehensive Unicode support (Latin accents, CJK, Arabic, emoji). This aligns with test coverage requirements for edge cases.
88-128: Good concurrent connection test structure.Properly uses
try/afterfor cleanup, extendedbusy_timeoutfor concurrent access, andTask.await_manywith appropriate timeout. The verification of all inserts via COUNT query is a solid pattern.
360-388: Thorough commit error handling.Good practice to explicitly handle commit results with case statements rather than assuming success. The
flunk/1calls withinspect/1provide clear diagnostic information on failure.
607-620: Robust cleanup pattern with graceful error handling.Good defensive programming: the prepared statement close is wrapped in case matching, logging errors at debug level rather than failing the test. This prevents cleanup issues from masking actual test failures.
835-865: Elegant state threading with reduce_while and with clause.The combination of
reduce_whilefor sequential inserts with explicit state threading, plus the outerwithclause for error handling, is a clean pattern that properly propagates both success and failure cases through the transaction.
1289-1379: Excellent constraint violation test.This test properly exercises the constraint violation scenario per test coverage requirements. It creates a unique constraint, attempts duplicate inserts within transactions, and verifies that:
- The constraint violation is detected
- The transaction correctly rolls back
- Only the original row persists
1-16: Comprehensive test module with excellent coverage.This test module provides thorough coverage of concurrent connection behaviour, including:
- Happy path operations (successful concurrent inserts)
- Error cases (constraint violations, syntax errors)
- Edge cases (NULL, empty strings, large data, Unicode)
- Transaction rollback scenarios (including mixed commit/rollback)
- Type conversions (integers, floats, timestamps)
- Concurrent operations throughout
The use of per-test unique databases and centralised cleanup via
TestHelpers.cleanup_db_filesaligns well with the PR's consolidation objectives. Based on learnings, test coverage requirements are satisfied.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.