Skip to content

Conversation

@ocean
Copy link
Owner

@ocean ocean commented Jan 15, 2026

  • Add tests for map defaults with JSON encoding
  • Add tests for list defaults with JSON encoding
  • Add tests for empty map and list defaults
  • Add tests for complex nested map defaults
  • Add tests for maps with various JSON types (string, number, bool, null)
  • Update existing empty map/list tests to reflect new behaviour

Further fixes for #57

Summary by CodeRabbit

  • Bug Fixes

    • Broader default value support in migrations: temporal types emitted as ISO8601 strings, decimals as string literals, maps/lists JSON-encoded when possible; :null treated as no default. Unencodable or unsupported defaults now omit DEFAULT and emit a warning.
  • Tests

    • Expanded coverage for JSON-encoded maps/lists, nested structures, temporal/decimal defaults, edge numeric/time cases and unencodable-value warning behaviour.
  • Chores

    • Test discovery configuration added to project settings.

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

- Add tests for map defaults with JSON encoding
- Add tests for list defaults with JSON encoding
- Add tests for empty map and list defaults
- Add tests for complex nested map defaults
- Add tests for maps with various JSON types (string, number, bool, null)
- Update existing empty map/list tests to reflect new behavior
- All 43 migration tests + 54 connection tests pass
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 15, 2026

Walkthrough

Adds JSON-encoding for map/list column defaults, converts temporal and Decimal defaults to quoted ISO8601/string literals, treats :null as no DEFAULT, logs and omits unsupported or unencodable defaults; test suite and project metadata updated accordingly.

Changes

Cohort / File(s) Summary
LibSQL Connection Adapter
lib/ecto/adapters/libsql/connection.ex
New encode_json_default/2; column_default/1 extended to JSON-encode maps/lists into DEFAULT '<json>'; converts DateTime/NaiveDateTime/Date/Time to ISO8601 quoted strings and Decimal to quoted string defaults; :null yields no DEFAULT; logs warnings and omits DEFAULT for unsupported or unencodable values.
Migration Tests
test/ecto_migration_test.exs
Expanded tests to assert JSON-encoded defaults for empty, nested and mixed maps/lists; verifies temporal and Decimal defaults are emitted as quoted literals; adds tests for warnings and omission of DEFAULT when map/list values are unencodable (e.g., PID, function); additional numeric/time edge cases covered.
Project metadata & manifest
mix.exs, manifest_file
Added test_pattern: "**/*_test.exs" and test_paths: ["test"] to project/0 metadata; manifest updated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibble keys and hop through trees,
I curl up maps and whisk the breeze,
Dates wear collars, decimals shine,
JSON crumbs and SQL thyme.
Hooray — defaults snug and fine! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically addresses the main changes in the pull request, which centre on adding support for map and list default values in the Ecto LibSQL adapter.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-defaults-handling


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 159f334 and 506cc2e.

📒 Files selected for processing (1)
  • test/ecto_migration_test.exs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ex,exs,rs,md}

📄 CodeRabbit inference engine (CLAUDE.md)

Use British/Australian English for all code, comments, and documentation (except SQL keywords and compatibility requirements)

Files:

  • test/ecto_migration_test.exs
test/**/*.{exs,ex}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.{exs,ex}: For state threading in tests, use consistent variable naming: state for connection scope, trx_state for transaction scope, cursor for cursor scope, stmt_id for prepared statement ID scope
When an error operation returns updated state in tests that IS needed for subsequent operations, rebind the state variable and add clarifying comments
When an error operation returns state in tests that is NOT needed for subsequent operations, discard it with underscore
For terminal test operations, use underscore variable names and assert error tuples with the pattern assert {:error, %EctoLibSql.Error{}, _conn}

Files:

  • test/ecto_migration_test.exs
**/*.{ex,exs}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ex,exs}: Use named parameters (:name, @name, or $name syntax) in SQL queries instead of positional ? parameters for improved readability and maintainability in Elixir code
Use prepared statements with EctoLibSql.Native.prepare/2 and statement caching for repeated queries to achieve 10-15x performance improvement
Always use transactions (handle_begin/handle_commit/handle_rollback) for multi-step database operations to ensure data consistency
When working with maps or lists containing temporal types (DateTime, NaiveDateTime, Date) for JSON storage, manually pre-encode them to ISO8601 strings before JSON serialization
For batch operations, prefer EctoLibSql.Native.batch_transactional/2 over individual inserts within explicit transactions for better atomicity and performance
Use cursors via DBConnection.stream/3 with max_rows parameter for memory-efficient processing of large result sets instead of loading all rows into memory
Implement proper error handling with pattern matching on {:ok, ...} and {:error, ...} tuples, use supervision trees for database worker processes, and log errors with context
In Ecto queries with JSON extraction, use fragment with inline SQL operators like data ->> 'key' for simpler syntax instead of the arrow_fragment helper for static fields
Use EctoLibSql.JSON module functions for JSON manipulation (extract, type, set, remove, patch, etc.) instead of raw SQL JSON functions for consistency and safety
Always wrap vector similarity search queries with limit clause and filter metadata using standard WHERE conditions before ordering by distance
Avoid using await on freeze_replica/1 function - it is not supported. Use backup/restore or data replication workarounds for disaster recovery scenarios

Files:

  • test/ecto_migration_test.exs
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/*.{ex,exs} : When working with maps or lists containing temporal types (DateTime, NaiveDateTime, Date) for JSON storage, manually pre-encode them to ISO8601 strings before JSON serialization
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/*.{ex,exs} : Use EctoLibSql.JSON module functions for JSON manipulation (extract, type, set, remove, patch, etc.) instead of raw SQL JSON functions for consistency and safety

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT table option in Ecto migrations for type safety, and ensure migrations define appropriate indexes (especially for frequently queried columns) and use table constraints

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/*.{ex,exs} : When working with maps or lists containing temporal types (DateTime, NaiveDateTime, Date) for JSON storage, manually pre-encode them to ISO8601 strings before JSON serialization

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/**/*.ex : Verify schema types match database columns and check custom types have loaders/dumpers to fix type conversion errors

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Update type loaders and dumpers in `lib/ecto/adapters/libsql.ex` when fixing type conversion issues

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to test/**/*.{exs,ex} : For terminal test operations, use underscore variable names and assert error tuples with the pattern `assert {:error, %EctoLibSql.Error{}, _conn}`

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/schemas/**/*.ex : When using Ecto changesets with LibSQL, always define unique_constraint/2 for email and other unique fields, and use on_conflict handling for upsert operations

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Update SQL generation in `lib/ecto/adapters/libsql/connection.ex` when adding SQLite function support

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to priv/repo/migrations/*.exs : For R*Tree spatial indexing, ensure table has odd number of columns (id + 1-5 dimension pairs), use proper min/max column naming convention, and create with rtree: true option in migrations

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/*.{ex,exs} : In Ecto queries with JSON extraction, use fragment with inline SQL operators like `data ->> 'key'` for simpler syntax instead of the arrow_fragment helper for static fields

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to priv/repo/migrations/*.exs : For SQLite migrations requiring column type changes, use table recreation pattern: create new table with desired schema, copy data with transformation, drop old table, rename new table, recreate indexes (SQLite doesn't support ALTER COLUMN; SQLite < 3.35.0 doesn't support DROP COLUMN)

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/**/*.ex : Use `cast/3` in changesets for automatic type conversion in Elixir

Applied to files:

  • test/ecto_migration_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 (7)
test/ecto_migration_test.exs (7)

939-952: LGTM!

The test correctly validates that empty maps are now JSON-encoded to '{}' in the DEFAULT clause, aligning with the updated handling of map defaults.


954-966: LGTM!

The test correctly validates that empty lists are now JSON-encoded to '[]' in the DEFAULT clause.


981-1075: Excellent implementation of JSON structure validation.

These tests properly address the previous review feedback by extracting and decoding the JSON literal from the DEFAULT clause, ensuring complete structure validation including null values. The Jason.decode!/1 approach catches missing keys and malformed JSON that substring checks would miss.


1077-1119: LGTM! Good coverage of error handling paths.

These tests properly verify the graceful degradation behaviour when JSON encoding fails for unencodable values (PIDs, functions). The use of ExUnit.CaptureLog.capture_log/1 to verify warning messages alongside the assertion that no DEFAULT clause is generated provides comprehensive validation.


1121-1140: LGTM!

The :null atom test correctly validates it behaves the same as nil (no DEFAULT clause). The Decimal test properly verifies string representation in the DEFAULT clause, preserving precision.


1142-1188: LGTM! Comprehensive temporal type coverage.

These tests correctly validate that DateTime, NaiveDateTime, Date, and Time defaults are converted to ISO8601 string representations in the DEFAULT clause. The Time test with microseconds (14:30:45.123456) provides good precision coverage. Based on learnings, this aligns with the approach of pre-encoding temporal types to ISO8601 strings before storage.


1190-1208: LGTM!

Good edge case coverage for Decimal defaults, testing both high precision values and negative numbers to ensure proper string conversion without truncation or sign issues.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

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

ocean added 4 commits January 16, 2026 08:37
- Log descriptive warnings when Jason.encode fails for map defaults
- Log descriptive warnings when Jason.encode fails for list defaults
- Provides visibility into why DEFAULT clause is not generated
- Matches existing error handling pattern from catch-all clause
- Includes tests verifying warning is logged for unencodable values (PIDs, functions)

Addresses feedback to improve debuggability when JSON encoding fails
- Combine column_default clauses for maps and lists using 'or' guard
- Extract common JSON encoding logic into encode_json_default/2 helper
- Reduces code duplication while maintaining readability
- Helper function handles both map and list type names for accurate logging
- All column_default clauses now grouped together per Elixir style guide

Addresses feedback to consolidate duplicate logic
- Replace String.contains?(sql, ["DEFAULT '[]'"]) with sql =~ "DEFAULT '[]'"
- More idiomatic Elixir pattern matching for simple string checks
- Reduces unnecessary list wrapping
- Maintains test clarity and intent

Addresses feedback to simplify unnecessarily verbose assertions
…ll defaults

Support for missing DEFAULT types:
- Decimal: Converts to string representation (e.g., '19.99')
- DateTime: Converts to ISO8601 string (e.g., '2026-01-16T14:30:00Z')
- NaiveDateTime: Converts to ISO8601 string (e.g., '2026-01-16T14:30:00')
- Date: Converts to ISO8601 string (e.g., '2026-01-16')
- Time: Converts to ISO8601 string (e.g., '14:30:45.123456')
- :null atom: Treated same as nil (no DEFAULT clause)

All conversions match the parameter encoding behavior in query.ex for consistency.

Add comprehensive test coverage:
- Basic Decimal defaults with various precision levels
- DateTime defaults with microsecond precision
- NaiveDateTime, Date, and Time defaults
- Edge cases: negative decimals, many decimal places
- :null atom behavior parity with nil

This closes the gap between what types the library accepts in parameters
and what types it accepts as column defaults in migrations.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@test/ecto_migration_test.exs`:
- Around line 1049-1052: The current assertion uses String.contains?(sql,
["string", "number", "bool"]) which only checks for any match; change it to
assert that all three JSON keys are present by checking each explicitly (for
example using Enum.all?/2 with String.contains?/2 or three separate asserts)
against the sql variable; update the test around the assertions that reference
sql and the String.contains? call so it ensures "string", "number", and "bool"
are all contained.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c1b2f38 and e6202ca.

📒 Files selected for processing (2)
  • lib/ecto/adapters/libsql/connection.ex
  • test/ecto_migration_test.exs
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ex,exs,rs,md}

📄 CodeRabbit inference engine (CLAUDE.md)

Use British/Australian English for all code, comments, and documentation (except SQL keywords and compatibility requirements)

Files:

  • test/ecto_migration_test.exs
  • lib/ecto/adapters/libsql/connection.ex
test/**/*.{exs,ex}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.{exs,ex}: For state threading in tests, use consistent variable naming: state for connection scope, trx_state for transaction scope, cursor for cursor scope, stmt_id for prepared statement ID scope
When an error operation returns updated state in tests that IS needed for subsequent operations, rebind the state variable and add clarifying comments
When an error operation returns state in tests that is NOT needed for subsequent operations, discard it with underscore
For terminal test operations, use underscore variable names and assert error tuples with the pattern assert {:error, %EctoLibSql.Error{}, _conn}

Files:

  • test/ecto_migration_test.exs
**/*.{ex,exs}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ex,exs}: Use named parameters (:name, @name, or $name syntax) in SQL queries instead of positional ? parameters for improved readability and maintainability in Elixir code
Use prepared statements with EctoLibSql.Native.prepare/2 and statement caching for repeated queries to achieve 10-15x performance improvement
Always use transactions (handle_begin/handle_commit/handle_rollback) for multi-step database operations to ensure data consistency
When working with maps or lists containing temporal types (DateTime, NaiveDateTime, Date) for JSON storage, manually pre-encode them to ISO8601 strings before JSON serialization
For batch operations, prefer EctoLibSql.Native.batch_transactional/2 over individual inserts within explicit transactions for better atomicity and performance
Use cursors via DBConnection.stream/3 with max_rows parameter for memory-efficient processing of large result sets instead of loading all rows into memory
Implement proper error handling with pattern matching on {:ok, ...} and {:error, ...} tuples, use supervision trees for database worker processes, and log errors with context
In Ecto queries with JSON extraction, use fragment with inline SQL operators like data ->> 'key' for simpler syntax instead of the arrow_fragment helper for static fields
Use EctoLibSql.JSON module functions for JSON manipulation (extract, type, set, remove, patch, etc.) instead of raw SQL JSON functions for consistency and safety
Always wrap vector similarity search queries with limit clause and filter metadata using standard WHERE conditions before ordering by distance
Avoid using await on freeze_replica/1 function - it is not supported. Use backup/restore or data replication workarounds for disaster recovery scenarios

Files:

  • test/ecto_migration_test.exs
  • lib/ecto/adapters/libsql/connection.ex
lib/ecto/adapters/libsql/connection.ex

📄 CodeRabbit inference engine (CLAUDE.md)

Update SQL generation in lib/ecto/adapters/libsql/connection.ex when adding SQLite function support

Files:

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

📄 CodeRabbit inference engine (CLAUDE.md)

lib/**/*.ex: Use proper transactions with timeout for database operations: Repo.transaction(fn -> ... end, timeout: 15_000) to avoid database locked errors
Ensure connections are closed in try/after blocks to prevent connection leaks
Use immediate transactions for write operations: begin(state, behavior: :immediate) for write-heavy workloads
Verify schema types match database columns and check custom types have loaders/dumpers to fix type conversion errors
Use cast/3 in changesets for automatic type conversion in Elixir

Files:

  • lib/ecto/adapters/libsql/connection.ex
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/*.{ex,exs} : Use EctoLibSql.JSON module functions for JSON manipulation (extract, type, set, remove, patch, etc.) instead of raw SQL JSON functions for consistency and safety
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/*.{ex,exs} : When working with maps or lists containing temporal types (DateTime, NaiveDateTime, Date) for JSON storage, manually pre-encode them to ISO8601 strings before JSON serialization
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/*.{ex,exs} : Use EctoLibSql.JSON module functions for JSON manipulation (extract, type, set, remove, patch, etc.) instead of raw SQL JSON functions for consistency and safety

Applied to files:

  • test/ecto_migration_test.exs
  • lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT table option in Ecto migrations for type safety, and ensure migrations define appropriate indexes (especially for frequently queried columns) and use table constraints

Applied to files:

  • test/ecto_migration_test.exs
  • lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Update type loaders and dumpers in `lib/ecto/adapters/libsql.ex` when fixing type conversion issues

Applied to files:

  • test/ecto_migration_test.exs
  • lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/**/*.ex : Verify schema types match database columns and check custom types have loaders/dumpers to fix type conversion errors

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to priv/repo/migrations/*.exs : For SQLite migrations requiring column type changes, use table recreation pattern: create new table with desired schema, copy data with transformation, drop old table, rename new table, recreate indexes (SQLite doesn't support ALTER COLUMN; SQLite < 3.35.0 doesn't support DROP COLUMN)

Applied to files:

  • test/ecto_migration_test.exs
  • lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/**/*.ex : Use `cast/3` in changesets for automatic type conversion in Elixir

Applied to files:

  • test/ecto_migration_test.exs
  • lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to test/**/*.{exs,ex} : For terminal test operations, use underscore variable names and assert error tuples with the pattern `assert {:error, %EctoLibSql.Error{}, _conn}`

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Update SQL generation in `lib/ecto/adapters/libsql/connection.ex` when adding SQLite function support

Applied to files:

  • test/ecto_migration_test.exs
  • lib/ecto/adapters/libsql/connection.ex
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to priv/repo/migrations/*.exs : For R*Tree spatial indexing, ensure table has odd number of columns (id + 1-5 dimension pairs), use proper min/max column naming convention, and create with rtree: true option in migrations

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/schemas/**/*.ex : When using Ecto changesets with LibSQL, always define unique_constraint/2 for email and other unique fields, and use on_conflict handling for upsert operations

Applied to files:

  • test/ecto_migration_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 (3)
lib/ecto/adapters/libsql/connection.ex (1)

476-512: Well-implemented JSON encoding for map/list defaults.

The implementation correctly handles JSON encoding with proper SQL escaping and graceful error handling. The escape_string/1 call ensures single quotes in JSON are properly escaped to prevent SQL injection.

Minor note: require Logger inside the error branch is evaluated at runtime on each failure. Since this only occurs during migrations and on encoding errors (both rare), this is acceptable.

test/ecto_migration_test.exs (2)

939-966: Tests correctly updated to reflect new JSON encoding behaviour.

The assertions now verify that empty maps encode to '{}' and empty lists encode to '[]', which aligns with the implementation changes.


1054-1096: Excellent coverage for unencodable value edge cases.

The tests properly verify both the absence of the DEFAULT clause and the warning messages using ExUnit.CaptureLog. Good use of real unencodable values (PID, function) rather than mocks.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

ocean added 3 commits January 16, 2026 08:44
Changed assertion from:
  assert String.contains?(sql, ["string", "number", "bool"])

To explicit assertions:
  assert sql =~ "string"
  assert sql =~ "number"
  assert sql =~ "bool"

The original assertion only verified that ANY of the keys were present.
The new approach ensures ALL three JSON keys are present in the DEFAULT clause,
providing stronger validation that the JSON encoding is complete.
Use DateTime.from_iso8601/1 instead of DateTime.new!/4 to avoid timezone
database issues. The ISO8601 parser is more reliable and doesn't require
timezone configuration in tests.
Add test_pattern and test_paths to mix.exs to explicitly configure which
files are test files. This eliminates the warning about support files
(test/support/*.ex) not matching test patterns, while ensuring all test files
matching **/*_test.exs are correctly identified and run.
@ocean
Copy link
Owner Author

ocean commented Jan 16, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@test/ecto_migration_test.exs`:
- Around line 981-1054: Update each JSON-default test (e.g., "handles map
defaults (JSON encoding)", "handles list defaults", "handles empty list
defaults", "handles complex nested map defaults", "handles map with various JSON
types") to locate the DEFAULT literal in the SQL returned by
Connection.execute_ddl, extract the quoted JSON string (the DEFAULT '...') and
decode it with Jason.decode!/1, then assert the decoded map/list equals the
expected structure (including asserting presence of keys like "null" => nil).
Use the Table struct and Connection.execute_ddl output as the source, parse the
DEFAULT clause for the column name (e.g.,
"preferences","tags","settings","metadata"), unescape the JSON string, decode
and compare to the exact expected Elixir map/list values to strengthen the
tests.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 645a518 and 159f334.

⛔ Files ignored due to path filters (1)
  • mix.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • mix.exs
  • test/ecto_migration_test.exs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ex,exs,rs,md}

📄 CodeRabbit inference engine (CLAUDE.md)

Use British/Australian English for all code, comments, and documentation (except SQL keywords and compatibility requirements)

Files:

  • mix.exs
  • test/ecto_migration_test.exs
**/*.{ex,exs}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ex,exs}: Use named parameters (:name, @name, or $name syntax) in SQL queries instead of positional ? parameters for improved readability and maintainability in Elixir code
Use prepared statements with EctoLibSql.Native.prepare/2 and statement caching for repeated queries to achieve 10-15x performance improvement
Always use transactions (handle_begin/handle_commit/handle_rollback) for multi-step database operations to ensure data consistency
When working with maps or lists containing temporal types (DateTime, NaiveDateTime, Date) for JSON storage, manually pre-encode them to ISO8601 strings before JSON serialization
For batch operations, prefer EctoLibSql.Native.batch_transactional/2 over individual inserts within explicit transactions for better atomicity and performance
Use cursors via DBConnection.stream/3 with max_rows parameter for memory-efficient processing of large result sets instead of loading all rows into memory
Implement proper error handling with pattern matching on {:ok, ...} and {:error, ...} tuples, use supervision trees for database worker processes, and log errors with context
In Ecto queries with JSON extraction, use fragment with inline SQL operators like data ->> 'key' for simpler syntax instead of the arrow_fragment helper for static fields
Use EctoLibSql.JSON module functions for JSON manipulation (extract, type, set, remove, patch, etc.) instead of raw SQL JSON functions for consistency and safety
Always wrap vector similarity search queries with limit clause and filter metadata using standard WHERE conditions before ordering by distance
Avoid using await on freeze_replica/1 function - it is not supported. Use backup/restore or data replication workarounds for disaster recovery scenarios

Files:

  • mix.exs
  • test/ecto_migration_test.exs
test/**/*.{exs,ex}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.{exs,ex}: For state threading in tests, use consistent variable naming: state for connection scope, trx_state for transaction scope, cursor for cursor scope, stmt_id for prepared statement ID scope
When an error operation returns updated state in tests that IS needed for subsequent operations, rebind the state variable and add clarifying comments
When an error operation returns state in tests that is NOT needed for subsequent operations, discard it with underscore
For terminal test operations, use underscore variable names and assert error tuples with the pattern assert {:error, %EctoLibSql.Error{}, _conn}

Files:

  • test/ecto_migration_test.exs
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/*.{ex,exs} : When working with maps or lists containing temporal types (DateTime, NaiveDateTime, Date) for JSON storage, manually pre-encode them to ISO8601 strings before JSON serialization
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/*.{ex,exs} : Implement proper error handling with pattern matching on {:ok, ...} and {:error, ...} tuples, use supervision trees for database worker processes, and log errors with context

Applied to files:

  • mix.exs
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/*.{ex,exs} : Use EctoLibSql.JSON module functions for JSON manipulation (extract, type, set, remove, patch, etc.) instead of raw SQL JSON functions for consistency and safety

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT table option in Ecto migrations for type safety, and ensure migrations define appropriate indexes (especially for frequently queried columns) and use table constraints

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Update type loaders and dumpers in `lib/ecto/adapters/libsql.ex` when fixing type conversion issues

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to test/**/*.{exs,ex} : For terminal test operations, use underscore variable names and assert error tuples with the pattern `assert {:error, %EctoLibSql.Error{}, _conn}`

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to **/schemas/**/*.ex : When using Ecto changesets with LibSQL, always define unique_constraint/2 for email and other unique fields, and use on_conflict handling for upsert operations

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-15T00:20:50.130Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-15T00:20:50.130Z
Learning: Applies to priv/repo/migrations/*.exs : For R*Tree spatial indexing, ensure table has odd number of columns (id + 1-5 dimension pairs), use proper min/max column naming convention, and create with rtree: true option in migrations

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Update SQL generation in `lib/ecto/adapters/libsql/connection.ex` when adding SQLite function support

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/**/*.ex : Verify schema types match database columns and check custom types have loaders/dumpers to fix type conversion errors

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to priv/repo/migrations/*.exs : For SQLite migrations requiring column type changes, use table recreation pattern: create new table with desired schema, copy data with transformation, drop old table, rename new table, recreate indexes (SQLite doesn't support ALTER COLUMN; SQLite < 3.35.0 doesn't support DROP COLUMN)

Applied to files:

  • test/ecto_migration_test.exs
📚 Learning: 2026-01-12T07:35:10.447Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T07:35:10.447Z
Learning: Applies to lib/**/*.ex : Use `cast/3` in changesets for automatic type conversion in Elixir

Applied to files:

  • test/ecto_migration_test.exs
🔇 Additional comments (4)
mix.exs (1)

20-21: Nice explicit test discovery settings.
Keeps ExUnit discovery consistent in CI and locally.

test/ecto_migration_test.exs (3)

939-966: Good coverage for empty map/list defaults.
Assertions align with the JSON‑encoded behaviour.


1056-1098: Nice warning coverage for unencodable defaults.
Capturing logs keeps the behaviour explicit.


1100-1187: Good coverage for :null, Decimal, and temporal defaults.
These cases now exercise the quoting/ISO‑8601 paths well.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Replace weak substring-based checks with proper extraction and decoding of JSON literals from DEFAULT clauses. This ensures:

- Exact structure validation via Jason.decode!
- Detection of missing keys (including null values)
- Verification that JSON is valid and complete
- No false positives from partial matches

Applied pattern to 5 JSON default tests:
- handles map defaults (JSON encoding)
- handles list defaults (JSON encoding)
- handles empty list defaults
- handles complex nested map defaults
- handles map with various JSON types

Tests now extract the JSON literal using regex and validate the decoded
structure matches exactly, including null values that would be silently
dropped by substring assertions.
@ocean ocean merged commit a4057eb into main Jan 16, 2026
14 checks passed
@ocean ocean deleted the fix-defaults-handling branch January 16, 2026 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants