fix: Create correct constraint name/error message as per Ecto expectations and naming conventions #35
Conversation
…tions and naming conventions
|
Caution Review failedThe pull request is closed. WalkthroughAdds a public to_constraints/2 to the LibSql adapter that reconstructs Ecto-style index/constraint names from SQLite error messages; updates tests and an integration changeset to expect reconstructed names; adds Dialyzer config/ignores, .gitignore entry, changelog note, and a large suite of vector geospatial tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @ocean. * #35 (comment) The following files were modified: * `lib/ecto/adapters/libsql/connection.ex`
Removed unnecessary blank lines in the documentation for the to_constraints function.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/ecto/adapters/libsql/connection.ex (1)
137-177: Correct reconstruction logic following Ecto conventions!The helper properly handles all three cases:
- Multi-column constraints:
"table.col1, table.col2"→"table_col1_col2_index"- Single-column constraints:
"table.column"→"table_column_index"- Bare constraints:
"positive_age"→"positive_age"The backtick cleaning throughout ensures libSQL's formatting quirks don't affect the result.
As an optional improvement, the multi-column logic could be slightly simplified. At line 156,
clean.()is called before splitting, then again at line 159 afterList.last(). The first call is redundant:{table_col, _} -> # Subsequent columns: only take the column name table_col - |> clean.() |> String.split(".") |> List.last() |> clean.()Alternatively, you could refactor the multi-column case to use pattern matching for clarity:
[first | rest] = String.split(constraint, ", ") table_col = first |> clean.() |> String.replace(".", "_") cols = Enum.map(rest, fn col -> col |> clean.() |> String.split(".") |> List.last() |> clean.() end) [table_col | cols] |> Enum.concat(["index"]) |> Enum.join("_")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ecto/adapters/libsql/connection.ex(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ex
📄 CodeRabbit inference engine (AGENTS.md)
Use Ecto changesets with validation rules and constraints in schema definitions
Files:
lib/ecto/adapters/libsql/connection.ex
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Elixir formatter (
mix format --check-formatted) before committing changes and fix any issues
Files:
lib/ecto/adapters/libsql/connection.ex
lib/**/*.ex
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.ex: Use case expressions to handle {:ok, result} and {:error, reason} tuples returned from NIF functions, with proper error propagation using the ? operator or with clauses
SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Handle database locked errors by using proper transaction management: set appropriate transaction behaviour (:deferred, :immediate, :exclusive), ensure connections are properly closed, use short transactions, set adequate timeouts (15000ms recommended)
Files:
lib/ecto/adapters/libsql/connection.ex
lib/ecto/adapters/libsql/connection.ex
📄 CodeRabbit inference engine (CLAUDE.md)
lib/ecto/adapters/libsql/connection.ex: Query expressions in lib/ecto/adapters/libsql/connection.ex must handle all Ecto expression types: literals, columns, operators, functions, fragments, case expressions, type casts, and subqueries
DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Files:
lib/ecto/adapters/libsql/connection.ex
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to **/*.ex : Use Ecto changesets with validation rules and constraints in schema definitions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/integration_tests.rs : Rust integration tests must use temporary SQLite database files with UUID names, created via setup_test_db() and cleaned up via cleanup_test_db(path), testing real database operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Provide comprehensive documentation for all NIF functions including purpose, arguments with types, return values with success/error patterns, and usage examples
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `safe_lock(&CONNECTION_REGISTRY, "function_name context")` pattern for locking the CONNECTION_REGISTRY Mutex, always providing context string for debugging
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Type loaders and dumpers in lib/ecto/adapters/libsql.ex must convert between Ecto types and SQLite types: :boolean (0/1), :datetime (ISO8601 text), :date (ISO8601 text), :time (ISO8601 text), custom types via registered handlers
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Query expressions in lib/ecto/adapters/libsql/connection.ex must handle all Ecto expression types: literals, columns, operators, functions, fragments, case expressions, type casts, and subqueries
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to **/*.ex : Use Ecto changesets with validation rules and constraints in schema definitions
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `safe_lock(&CONNECTION_REGISTRY, "function_name context")` pattern for locking the CONNECTION_REGISTRY Mutex, always providing context string for debugging
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Handle NIF errors gracefully by pattern matching on {:ok, result} and {:error, reason} returns from EctoLibSql.Native functions
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `ok_or_else()` for Option types instead of `.unwrap()` or `.expect()`, providing descriptive error messages
Applied to files:
lib/ecto/adapters/libsql/connection.ex
⏰ 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 (2)
lib/ecto/adapters/libsql/connection.ex (2)
69-83: Excellent documentation addition!The docstring clearly explains the function's purpose, parameters, and return values with helpful examples. The type specification is also correct.
104-135: Well-structured constraint extraction logic!The two-phase approach is solid:
- First attempts to extract explicit index names from enhanced error messages (line 118)
- Falls back to reconstructing names from column information (lines 125-129)
The backtick stripping (line 128) properly handles libSQL's formatting quirks, and the fallback to "unknown" (line 132) provides sensible error handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/ecto/adapters/libsql/connection.ex (1)
137-169: Index name reconstruction logic is correct.The implementation correctly handles all three cases:
- Multi-column:
"users.email, users.slug"→"users_email_slug_index"✓- Single-column:
"users.email"→"users_email_index"✓- Bare constraint:
"positive_age"→"positive_age"✓The logic properly extracts the table name from the first column in multi-column constraints and follows Ecto's naming convention.
Minor refinement: Remove redundant clean calls.
Line 151 applies
clean.()twice—once before splitting and once onList.last(). The second call is redundant sinceList.last()returns a substring that was already cleaned. Similarly, line 161 mapscleanover parts that are already clean.Apply this diff to simplify:
cols = Enum.map(rest, fn col -> - col |> clean.() |> String.split(".") |> List.last() |> clean.() + col |> clean.() |> String.split(".") |> List.last() end)constraint |> clean.() |> String.split(".") - |> Enum.map(clean) |> Enum.concat(["index"]) |> Enum.join("_")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ecto/adapters/libsql/connection.ex(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ex
📄 CodeRabbit inference engine (AGENTS.md)
Use Ecto changesets with validation rules and constraints in schema definitions
Files:
lib/ecto/adapters/libsql/connection.ex
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Elixir formatter (
mix format --check-formatted) before committing changes and fix any issues
Files:
lib/ecto/adapters/libsql/connection.ex
lib/**/*.ex
📄 CodeRabbit inference engine (CLAUDE.md)
lib/**/*.ex: Use case expressions to handle {:ok, result} and {:error, reason} tuples returned from NIF functions, with proper error propagation using the ? operator or with clauses
SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Handle database locked errors by using proper transaction management: set appropriate transaction behaviour (:deferred, :immediate, :exclusive), ensure connections are properly closed, use short transactions, set adequate timeouts (15000ms recommended)
Files:
lib/ecto/adapters/libsql/connection.ex
lib/ecto/adapters/libsql/connection.ex
📄 CodeRabbit inference engine (CLAUDE.md)
lib/ecto/adapters/libsql/connection.ex: Query expressions in lib/ecto/adapters/libsql/connection.ex must handle all Ecto expression types: literals, columns, operators, functions, fragments, case expressions, type casts, and subqueries
DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Files:
lib/ecto/adapters/libsql/connection.ex
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to **/*.ex : Use Ecto changesets with validation rules and constraints in schema definitions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Provide comprehensive documentation for all NIF functions including purpose, arguments with types, return values with success/error patterns, and usage examples
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `safe_lock(&CONNECTION_REGISTRY, "function_name context")` pattern for locking the CONNECTION_REGISTRY Mutex, always providing context string for debugging
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Query expressions in lib/ecto/adapters/libsql/connection.ex must handle all Ecto expression types: literals, columns, operators, functions, fragments, case expressions, type casts, and subqueries
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `safe_lock(&CONNECTION_REGISTRY, "function_name context")` pattern for locking the CONNECTION_REGISTRY Mutex, always providing context string for debugging
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to **/*.ex : Use Ecto changesets with validation rules and constraints in schema definitions
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Handle NIF errors gracefully by pattern matching on {:ok, result} and {:error, reason} returns from EctoLibSql.Native functions
Applied to files:
lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `ok_or_else()` for Option types instead of `.unwrap()` or `.expect()`, providing descriptive error messages
Applied to files:
lib/ecto/adapters/libsql/connection.ex
⏰ 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 (3)
lib/ecto/adapters/libsql/connection.ex (3)
69-83: Well-documented public API!The docstring clearly explains the function's purpose, parameters, return values, and provides helpful examples of constraint tuples. The typespec correctly captures the function signature.
84-102: Constraint type detection logic looks correct.The pattern matching for different constraint types is sound, and the mapping of NOT NULL to check constraints aligns with Ecto conventions. The approach using
String.contains?for detecting constraint types is appropriate for SQLite error messages.
105-134: Implementation correctly handles backtick removal and error message parsing.The two-phase extraction logic with defence-in-depth backtick handling is sound and well-tested. The code properly:
- Strips trailing backticks via
String.trim_trailing("")` at line 128- Applies additional backtick removal on both sides within
constraint_name_hack/1- Implicitly prevents empty
constraint_partvia the regex quantifier(.+)$which requires at least one character- Handles enhanced error messages containing explicit index names via the initial
(index: ...)pattern matchTest coverage confirms correct behaviour: "UNIQUE constraint failed: users.email
" →users_email_index`.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/vector_geospatial_test.exs (2)
55-57: Consider cleaning up SQLite WAL/SHM files on exit.SQLite may create additional files (
*.db-wal,*.db-shm) that won't be removed byFile.rm(@test_db). The.gitignorepatternz_ecto_libsql_test-*.db-*suggests awareness of this.on_exit(fn -> File.rm(@test_db) + File.rm(@test_db <> "-wal") + File.rm(@test_db <> "-shm") end)
68-347: Consider adding error and edge case tests.The current test suite thoroughly covers happy path scenarios. Per coding guidelines, consider adding tests for:
- Edge cases: NULL embeddings, empty result sets, zero-distance calculations
- Error cases: Invalid vector dimensions, malformed embeddings
This could be addressed in a follow-up PR.
Would you like me to help generate additional test cases for error and edge scenarios?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
mix.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.dialyzer_ignore.exs(1 hunks).gitignore(1 hunks)lib/ecto/adapters/libsql/connection.ex(2 hunks)mix.exs(2 hunks)test/vector_geospatial_test.exs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/ecto/adapters/libsql/connection.ex
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Elixir formatter (
mix format --check-formatted) before committing changes and fix any issues
Files:
mix.exstest/vector_geospatial_test.exs
test/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Files:
test/vector_geospatial_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Each Ecto test must cover: happy path (successful operations), error cases (invalid IDs, missing resources, constraints), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations if applicable
Files:
test/vector_geospatial_test.exs
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to **/*.ex : Use Ecto changesets with validation rules and constraints in schema definitions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to lib/**/*repo.ex : Configure Ecto repository in lib/ directory with adapter: Ecto.Adapters.LibSql
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Provide comprehensive documentation for all NIF functions including purpose, arguments with types, return values with success/error patterns, and usage examples
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Type loaders and dumpers in lib/ecto/adapters/libsql.ex must convert between Ecto types and SQLite types: :boolean (0/1), :datetime (ISO8601 text), :date (ISO8601 text), :time (ISO8601 text), custom types via registered handlers
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `safe_lock(&CONNECTION_REGISTRY, "function_name context")` pattern for locking the CONNECTION_REGISTRY Mutex, always providing context string for debugging
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Query expressions in lib/ecto/adapters/libsql/connection.ex must handle all Ecto expression types: literals, columns, operators, functions, fragments, case expressions, type casts, and subqueries
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Provide comprehensive documentation for all NIF functions including purpose, arguments with types, return values with success/error patterns, and usage examples
Applied to files:
.dialyzer_ignore.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Applied to files:
.dialyzer_ignore.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Applied to files:
.dialyzer_ignore.exs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Handle NIF errors gracefully by pattern matching on {:ok, result} and {:error, reason} returns from EctoLibSql.Native functions
Applied to files:
.dialyzer_ignore.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Query expressions in lib/ecto/adapters/libsql/connection.ex must handle all Ecto expression types: literals, columns, operators, functions, fragments, case expressions, type casts, and subqueries
Applied to files:
.dialyzer_ignore.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Type loaders and dumpers in lib/ecto/adapters/libsql.ex must convert between Ecto types and SQLite types: :boolean (0/1), :datetime (ISO8601 text), :date (ISO8601 text), :time (ISO8601 text), custom types via registered handlers
Applied to files:
.dialyzer_ignore.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `ok_or_else()` for Option types instead of `.unwrap()` or `.expect()`, providing descriptive error messages
Applied to files:
.dialyzer_ignore.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : Handle database locked errors by using proper transaction management: set appropriate transaction behaviour (:deferred, :immediate, :exclusive), ensure connections are properly closed, use short transactions, set adequate timeouts (15000ms recommended)
Applied to files:
.dialyzer_ignore.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto_libsql/native.ex : Add safe wrapper functions in Elixir (lib/ecto_libsql/native.ex) around Rust NIFs that use EctoLibSql.State, pattern matching on state fields like `conn_id` to pass to the underlying NIF
Applied to files:
.dialyzer_ignore.exs
📚 Learning: 2025-12-14T21:02:57.456Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Wrap database operations in error handling using pattern matching on {:ok, result} and {:error, reason} tuples
Applied to files:
.dialyzer_ignore.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Rust test modules must be organised in tests/ subdirectory with focused modules: mod.rs (organisation), constants_tests.rs (registry operations), utils_tests.rs (utility functions), integration_tests.rs (end-to-end scenarios)
Applied to files:
.gitignoremix.exstest/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Applied to files:
.gitignoretest/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/integration_tests.rs : Rust integration tests must use temporary SQLite database files with UUID names, created via setup_test_db() and cleaned up via cleanup_test_db(path), testing real database operations
Applied to files:
.gitignore
📚 Learning: 2025-12-17T00:28:04.033Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.033Z
Learning: Release process must include: update version in mix.exs, update CHANGELOG.md with all changes, verify README.md if documentation changed, run full test suite (mix test && cargo test), verify Hex package includes correct files (lib/, priv/, native/, documentation)
Applied to files:
mix.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to **/*.{ex,exs} : ALWAYS run the Elixir formatter (`mix format --check-formatted`) before committing changes and fix any issues
Applied to files:
mix.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : All NIF functions must use `#[rustler::nif]` attribute annotation. Modern Rustler automatically discovers all NIFs, so `rustler::init!()` in lib.rs automatically registers all annotated functions
Applied to files:
mix.exs
📚 Learning: 2025-12-17T00:28:04.033Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.033Z
Learning: Connection options in Ecto config must include: :database for local SQLite (required for local/replica modes), :uri and :auth_token for Turso (required for remote/replica modes), :sync for replicas (boolean), :encryption_key (32+ chars), :pool_size, and :queue_target/:queue_interval for connection pooling
Applied to files:
mix.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/lib.rs : Rust module lib.rs must declare and export all submodules and perform NIF function registration via `rustler::init!`, keeping the file concise (around 29 lines)
Applied to files:
mix.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `#[rustler::nif(schedule = "DirtyIo")]` for I/O-bound NIF functions, ensuring Erlang scheduler is not blocked
Applied to files:
mix.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/**/*.exs : Each Ecto test must cover: happy path (successful operations), error cases (invalid IDs, missing resources, constraints), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations if applicable
Applied to files:
test/vector_geospatial_test.exs
🪛 GitHub Actions: CI
test/vector_geospatial_test.exs
[error] 138-365: Command failed: mix format --check-formatted. The file is not formatted according to Elixir formatter. Details show multiple formatting diffs in test/vector_geospatial_test.exs (lines around 138-365). Run 'mix format' to fix formatting.
⏰ 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)
.gitignore (1)
33-35: LGTM!The PLT directory ignore rule correctly matches the
plt_core_path: "priv/plts"configuration in mix.exs, preventing build artifacts from being committed.mix.exs (2)
19-29: Well-structured Dialyzer configuration.Good practices observed:
plt_core_pathaligns with the.gitignoreentrylist_unused_filters: truehelps identify stale suppressions- The
check.dialyzeralias provides a convenient shortcut
52-52: Dependency version is current and properly scoped.The latest stable version is 1.4.7 (released Nov 06, 2025), and the constraint ~> 1.4 correctly allows patch updates within the 1.4.x series. The scoping to dev-only with
runtime: falseis correct. No changes required.test/vector_geospatial_test.exs (1)
147-163: LGTM!Good practice guarding the pattern match with
assert result.num_rows == 3before destructuring. The assertions comprehensively verify ordering and distance properties..dialyzer_ignore.exs (1)
1-25: Line 25 suppression is correct—no investigation required.The improper list warning refers to intentional iodata construction at
connection.exline 610 where["EXPLAIN QUERY PLAN " | sql]builds iodata forIO.iodata_to_binary/1. This is idiomatic and correct Elixir code. The suppression should remain.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
test/vector_geospatial_test.exs (4)
55-57: Consider cleaning up SQLite WAL and SHM files.SQLite in WAL mode may create additional files (
.db-wal,.db-shm). The current cleanup only removes the main database file, which could leave artefacts.on_exit(fn -> File.rm(@test_db) + File.rm(@test_db <> "-wal") + File.rm(@test_db <> "-shm") end)
68-388: Consider adding error and edge case tests per coding guidelines.The happy path scenarios are well covered with good assertions. As per coding guidelines, consider adding tests for:
- Error cases: Invalid vector dimensions (e.g., 3D vector when 2D expected), malformed embeddings
- Edge cases: NULL embedding values, empty result sets, boundary coordinates (±90°/±180°)
- Type conversions: Verify behaviour when embedding field contains unexpected types
Example additional test:
test "handles NULL embedding gracefully" do TestRepo.insert!(%Location{ name: "No Embedding", latitude: 0.0, longitude: 0.0, city: "Test", country: "Test" }) # Verify query doesn't crash with NULL embeddings result = Ecto.Adapters.SQL.query!( TestRepo, """ SELECT name FROM locations WHERE embedding IS NULL """ ) assert result.num_rows == 1 end
334-339: Consider asserting the nearest USA city for completeness.The test verifies ordering but doesn't assert which USA city is closest to Sydney. Adding this would strengthen the test.
# Extract USA results usa_rows = Enum.filter(rows, fn [country, _, _] -> country == "USA" end) assert length(usa_rows) == 2 - [[usa_country, _usa_city, usa_dist], [_, _, second_usa_dist]] = usa_rows + [[usa_country, usa_city, usa_dist], [_, _, second_usa_dist]] = usa_rows assert usa_country == "USA" + # Los Angeles is geographically closer to Sydney than New York + assert usa_city == "Los Angeles" assert usa_dist < second_usa_dist
342-387: Test implementation uses scalar coordinates rather than vector ranges.The test name suggests "vector ranges" but the implementation filters on scalar
latitude/longitudecolumns rather than using vector operations. This is a valid approach for bounding box queries, but consider either:
- Renaming to "finds locations within geographic bounds" for clarity, or
- Using vector distance from a centre point if vector-based range search was intended
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/vector_geospatial_test.exs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Elixir formatter (
mix format --check-formatted) before committing changes and fix any issues
Files:
test/vector_geospatial_test.exs
test/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Files:
test/vector_geospatial_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Each Ecto test must cover: happy path (successful operations), error cases (invalid IDs, missing resources, constraints), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations if applicable
Files:
test/vector_geospatial_test.exs
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to lib/**/*repo.ex : Configure Ecto repository in lib/ directory with adapter: Ecto.Adapters.LibSql
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to **/*.ex : Use Ecto changesets with validation rules and constraints in schema definitions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Provide comprehensive documentation for all NIF functions including purpose, arguments with types, return values with success/error patterns, and usage examples
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/integration_tests.rs : Rust integration tests must use temporary SQLite database files with UUID names, created via setup_test_db() and cleaned up via cleanup_test_db(path), testing real database operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Type loaders and dumpers in lib/ecto/adapters/libsql.ex must convert between Ecto types and SQLite types: :boolean (0/1), :datetime (ISO8601 text), :date (ISO8601 text), :time (ISO8601 text), custom types via registered handlers
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `safe_lock(&CONNECTION_REGISTRY, "function_name context")` pattern for locking the CONNECTION_REGISTRY Mutex, always providing context string for debugging
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Rust test modules must be organised in tests/ subdirectory with focused modules: mod.rs (organisation), constants_tests.rs (registry operations), utils_tests.rs (utility functions), integration_tests.rs (end-to-end scenarios)
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Rust test modules must be organised in tests/ subdirectory with focused modules: mod.rs (organisation), constants_tests.rs (registry operations), utils_tests.rs (utility functions), integration_tests.rs (end-to-end scenarios)
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/**/*.exs : Each Ecto test must cover: happy path (successful operations), error cases (invalid IDs, missing resources, constraints), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations if applicable
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to **/*.{ex,exs} : ALWAYS run the Elixir formatter (`mix format --check-formatted`) before committing changes and fix any issues
Applied to files:
test/vector_geospatial_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 (2)
test/vector_geospatial_test.exs (2)
1-9: Module and TestRepo setup looks good.Using
async: falseis correct for database tests with shared state. The TestRepo configuration follows standard Ecto patterns for the LibSql adapter.
11-31: Schema and changeset are well-defined.The Location schema appropriately defines the embedding field as
:stringsinceEctoLibSql.Native.vectorreturns a string representation. The changeset validates the essential geospatial fields.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/vector_geospatial_test.exs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Elixir formatter (
mix format --check-formatted) before committing changes and fix any issues
Files:
test/vector_geospatial_test.exs
test/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Files:
test/vector_geospatial_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Each Ecto test must cover: happy path (successful operations), error cases (invalid IDs, missing resources, constraints), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations if applicable
Files:
test/vector_geospatial_test.exs
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to lib/**/*repo.ex : Configure Ecto repository in lib/ directory with adapter: Ecto.Adapters.LibSql
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to **/*.ex : Use Ecto changesets with validation rules and constraints in schema definitions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Provide comprehensive documentation for all NIF functions including purpose, arguments with types, return values with success/error patterns, and usage examples
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Type loaders and dumpers in lib/ecto/adapters/libsql.ex must convert between Ecto types and SQLite types: :boolean (0/1), :datetime (ISO8601 text), :date (ISO8601 text), :time (ISO8601 text), custom types via registered handlers
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/integration_tests.rs : Rust integration tests must use temporary SQLite database files with UUID names, created via setup_test_db() and cleaned up via cleanup_test_db(path), testing real database operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Use `safe_lock(&CONNECTION_REGISTRY, "function_name context")` pattern for locking the CONNECTION_REGISTRY Mutex, always providing context string for debugging
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Rust test modules must be organised in tests/ subdirectory with focused modules: mod.rs (organisation), constants_tests.rs (registry operations), utils_tests.rs (utility functions), integration_tests.rs (end-to-end scenarios)
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Rust test modules must be organised in tests/ subdirectory with focused modules: mod.rs (organisation), constants_tests.rs (registry operations), utils_tests.rs (utility functions), integration_tests.rs (end-to-end scenarios)
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/**/*.exs : Each Ecto test must cover: happy path (successful operations), error cases (invalid IDs, missing resources, constraints), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations if applicable
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to **/*.{ex,exs} : ALWAYS run the Elixir formatter (`mix format --check-formatted`) before committing changes and fix any issues
Applied to files:
test/vector_geospatial_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 (5)
test/vector_geospatial_test.exs (5)
71-175: LGTM! Comprehensive nearest-neighbour test.The test correctly validates cosine distance calculations with normalized coordinates, verifies self-similarity (distance ≈ 0), and confirms proper ordering of results.
177-389: LGTM! Thorough coverage of geospatial search scenarios.The tests effectively validate region filtering, distance thresholds, per-country aggregation, and approximate searches using coordinate ranges.
392-733: LGTM! Excellent edge case coverage.The tests comprehensively cover NULL handling, empty results, zero distances, extreme coordinates, large-dimensional vectors (128-dim), pagination, and mixed NULL/valid scenarios.
765-886: LGTM! Robust error case testing.The tests effectively validate handling of large thresholds, zero thresholds, negative distance comparisons (always return 0 results), and duplicate record behaviour.
19-19: Remove the type concern—the embedding field type is correct.The
embeddingfield is properly typed as:stringbecauseEctoLibSql.Native.vector()returns a string representation (e.g.,"[1.0,2.0]"), not binary data. The database stores this as F32_BLOB(2), and the dimension matching is exact—all test vectors use 2-dimensional coordinates matching the column definition.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/vector_geospatial_test.exs (1)
19-19: Consider adding a comment for the embedding field type.The
:stringtype for the embedding field is used to store binary vector data (F32_BLOB). Whilst this is correct for handling binary embeddings with LibSql, a brief comment would clarify this non-obvious choice for future maintainers.Example:
field(:longitude, :float) - field(:embedding, :string) + field(:embedding, :string) # Binary vector data stored as F32_BLOB field(:city, :string)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/vector_geospatial_test.exs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Elixir formatter (
mix format --check-formatted) before committing changes and fix any issues
Files:
test/vector_geospatial_test.exs
test/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Files:
test/vector_geospatial_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Each Ecto test must cover: happy path (successful operations), error cases (invalid IDs, missing resources, constraints), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations if applicable
Files:
test/vector_geospatial_test.exs
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to lib/**/*repo.ex : Configure Ecto repository in lib/ directory with adapter: Ecto.Adapters.LibSql
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to **/*.ex : Use Ecto changesets with validation rules and constraints in schema definitions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/integration_tests.rs : Rust integration tests must use temporary SQLite database files with UUID names, created via setup_test_db() and cleaned up via cleanup_test_db(path), testing real database operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Type loaders and dumpers in lib/ecto/adapters/libsql.ex must convert between Ecto types and SQLite types: :boolean (0/1), :datetime (ISO8601 text), :date (ISO8601 text), :time (ISO8601 text), custom types via registered handlers
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Rust test modules must be organised in tests/ subdirectory with focused modules: mod.rs (organisation), constants_tests.rs (registry operations), utils_tests.rs (utility functions), integration_tests.rs (end-to-end scenarios)
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/**/*.exs : Each Ecto test must cover: happy path (successful operations), error cases (invalid IDs, missing resources, constraints), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations if applicable
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to **/*.{ex,exs} : ALWAYS run the Elixir formatter (`mix format --check-formatted`) before committing changes and fix any issues
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : Handle database locked errors by using proper transaction management: set appropriate transaction behaviour (:deferred, :immediate, :exclusive), ensure connections are properly closed, use short transactions, set adequate timeouts (15000ms recommended)
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/integration_tests.rs : Rust integration tests must use temporary SQLite database files with UUID names, created via setup_test_db() and cleaned up via cleanup_test_db(path), testing real database operations
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/savepoint.rs : Savepoint operations (create, release, rollback) must validate that the transaction belongs to the current connection by checking `entry.conn_id == conn_id` before proceeding
Applied to files:
test/vector_geospatial_test.exs
🧬 Code graph analysis (1)
test/vector_geospatial_test.exs (2)
native/ecto_libsql/src/transaction.rs (1)
transaction(100-111)lib/ecto_libsql/native.ex (1)
rollback(435-439)
⏰ 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 (4)
test/vector_geospatial_test.exs (4)
70-390: Excellent test coverage for vector geospatial search operations.The test suite comprehensively covers:
- Nearest neighbour searches with cosine distance
- Region-based filtering
- Distance threshold queries
- Country-based aggregations
- Approximate location searches using coordinate ranges
All tests properly use parameterised queries and verify results with appropriate assertions.
392-749: Comprehensive edge case coverage.This test block thoroughly validates edge cases including:
- NULL embedding handling
- Empty result sets
- Zero-distance scenarios with identical embeddings
- Extreme coordinate values at poles and date line
- Large dimensional vectors (128-D) with proper table creation and cleanup
- Pagination with distance ordering
- Mixed NULL and valid embeddings
All tests include appropriate assertions and resource cleanup.
904-1065: Transaction rollback tests successfully address previous review feedback.Excellent implementation of the previously requested transaction rollback test coverage. The suite now includes:
- Single insertion rollback - Verifies a single vector insertion is properly rolled back
- Batch insertion rollback - Confirms multiple insertions within a transaction are rolled back whilst preserving pre-transaction data
- Savepoint rollback - Tests manual rollback behaviour within transactions
Each test properly verifies database state before, during, and after transactions, ensuring data integrity is maintained across rollback scenarios.
Based on learnings, transaction rollback scenarios are required test coverage for Ecto tests.
1067-1303: Concurrent operation tests successfully address previous review feedback.Outstanding implementation of the previously requested concurrent operation test coverage. The suite now includes:
- Concurrent distance queries - 10 parallel queries with varying vectors, verifying thread-safety of read operations
- Concurrent transactional inserts - 5 parallel insertions within separate transactions, validating write isolation
- Concurrent queries with varying dimensions - Tests multiple query points executed concurrently
- Mixed read/write concurrency - Combines 5 reader tasks with 3 writer tasks to validate concurrent access patterns
All tests properly use
Task.asyncandTask.await_many, verify operation success, and confirm data integrity across concurrent operations.Based on learnings, concurrent operation scenarios are required test coverage for Ecto tests when applicable.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/vector_geospatial_test.exs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS run the Elixir formatter (
mix format --check-formatted) before committing changes and fix any issues
Files:
test/vector_geospatial_test.exs
test/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Files:
test/vector_geospatial_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Each Ecto test must cover: happy path (successful operations), error cases (invalid IDs, missing resources, constraints), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations if applicable
Files:
test/vector_geospatial_test.exs
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : DDL operations in lib/ecto/adapters/libsql/connection.ex must support: CREATE/DROP TABLE/INDEX, ALTER TABLE (column add/rename/modify with constraints), RENAME, foreign key management, constraint generation (primary key, unique, not null, check, default)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to lib/**/*repo.ex : Configure Ecto repository in lib/ directory with adapter: Ecto.Adapters.LibSql
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : SQLite ALTER TABLE operations have limited support: no RENAME COLUMN (requires table recreation), no DROP COLUMN (SQLite < 3.35.0), no TYPE changes. Use table recreation pattern documented in ECTO_MIGRATION_GUIDE.md for schema changes
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/integration_tests.rs : Rust integration tests must use temporary SQLite database files with UUID names, created via setup_test_db() and cleaned up via cleanup_test_db(path), testing real database operations
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T21:02:57.456Z
Learning: Applies to **/*.ex : Use Ecto changesets with validation rules and constraints in schema definitions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Rust test modules must be organised in tests/ subdirectory with focused modules: mod.rs (organisation), constants_tests.rs (registry operations), utils_tests.rs (utility functions), integration_tests.rs (end-to-end scenarios)
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/*.exs : All Elixir test files must have descriptive names matching their purpose: ecto_adapter_test.exs (adapter functionality), ecto_connection_test.exs (SQL generation), ecto_integration_test.exs (full integration), ecto_libsql_test.exs (DBConnection), ecto_migration_test.exs (migrations), error_handling_test.exs (error handling), turso_remote_test.exs (Turso remote tests)
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Rust test modules must be organised in tests/ subdirectory with focused modules: mod.rs (organisation), constants_tests.rs (registry operations), utils_tests.rs (utility functions), integration_tests.rs (end-to-end scenarios)
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to test/**/*.exs : Each Ecto test must cover: happy path (successful operations), error cases (invalid IDs, missing resources, constraints), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations if applicable
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to **/*.{ex,exs} : ALWAYS run the Elixir formatter (`mix format --check-formatted`) before committing changes and fix any issues
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to lib/**/*.ex : Handle database locked errors by using proper transaction management: set appropriate transaction behaviour (:deferred, :immediate, :exclusive), ensure connections are properly closed, use short transactions, set adequate timeouts (15000ms recommended)
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/tests/integration_tests.rs : Rust integration tests must use temporary SQLite database files with UUID names, created via setup_test_db() and cleaned up via cleanup_test_db(path), testing real database operations
Applied to files:
test/vector_geospatial_test.exs
📚 Learning: 2025-12-17T00:28:04.032Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T00:28:04.032Z
Learning: Applies to native/ecto_libsql/src/savepoint.rs : Savepoint operations (create, release, rollback) must validate that the transaction belongs to the current connection by checking `entry.conn_id == conn_id` before proceeding
Applied to files:
test/vector_geospatial_test.exs
🧬 Code graph analysis (1)
test/vector_geospatial_test.exs (2)
native/ecto_libsql/src/transaction.rs (1)
transaction(100-111)lib/ecto_libsql/native.ex (1)
rollback(435-439)
⏰ 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 (5)
test/vector_geospatial_test.exs (5)
1-69: LGTM! Well-structured test setup.The module setup is comprehensive with appropriate
async: falsefor database tests, proper cleanup of SQLite WAL/SHM files, and table cleanup between tests. Theembeddingfield typed as:stringworks correctly withEctoLibSql.Native.vectoroutput.
71-391: Good coverage of geospatial search scenarios.The tests cover nearest-neighbour search, region filtering, distance thresholds, country aggregation, and coordinate range queries. The coordinate normalisation approach (lat/90, lon/180) is well-documented in comments.
393-750: Comprehensive edge case coverage.Good testing of NULL embeddings, empty results, identical embeddings, extreme coordinate values, large vectors (128-dim), and pagination. The temporary
locations_largetable is properly cleaned up.
905-1066: Transaction rollback scenarios well covered.Tests correctly verify that vector insertions are rolled back on transaction failure, including single inserts, batch inserts, and nested transaction contexts. The before/after count assertions properly validate rollback behaviour.
1068-1305: Good concurrent operation testing.The tests properly validate thread-safety of vector distance queries and concurrent insertions using
Task.async/Task.await_many. The mixed read/write test correctly handles SQLite's concurrency model.
Fixes:
Enhanced extract_constraint_name/1 to reconstruct full index names following Ecto's convention
Added constraint_name_hack/1 helper function for proper index name reconstruction
Properly handles single-column, multi-column, and bare constraint names
Strips backticks appended by libSQL to error messages
Updated existing test to expect reconstructed index name "users_email_index"
Added 4 new comprehensive test cases:
Updated User schema changeset to use new index name "users_email_index"
Ensures constraint validation matches the reconstructed index name from error messages
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.