-
Notifications
You must be signed in to change notification settings - Fork 1
JSON and JSONB schema helpers #50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Create EctoLibSql.JSON module with helpers for JSON/JSONB functions - Implement json_extract, json_type, json_is_valid, json_array, json_object - Implement json_each, json_tree for recursive iteration - Implement convert() for JSONB binary format support - Add arrow_fragment() helper for -> and ->> operators - Comprehensive test suite with 54 passing tests - Support for both text JSON and JSONB binary format - All functions handle error cases gracefully
- Add comprehensive JSON schema helpers section under Advanced Features - Document all EctoLibSql.JSON functions with examples - Include JSONB binary format support and usage - Add arrow operators (-> and ->>) documentation - Include real-world settings management example - Add API reference for all JSON helper functions - Include performance notes for JSON operations
WalkthroughAdds a new Elixir JSON helper module Changes
Sequence Diagram(s)(omitted — changes are new APIs, tests and metadata; not a multi-component control-flow change requiring a sequence diagram.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/ecto_libsql/json.ex (3)
623-644: Consider renamingquote/2to avoid shadowingKernel.quote/2.The function name shadows Elixir's fundamental
quote/2macro. While not breaking, users importing this module may encounter confusion. A name likejson_quote/2would be clearer and consistent with SQLite's function name.🔎 Proposed rename
- @spec quote(State.t(), term()) :: {:ok, String.t()} | {:error, term()} - def quote(%State{} = state, value) do + @spec json_quote(State.t(), term()) :: {:ok, String.t()} | {:error, term()} + def json_quote(%State{} = state, value) do
670-695: Consider renaminglength/2,3to avoid shadowingKernel.length/1.Similar to
quote/2, this shadows Elixir'sKernel.length/1. Considerjson_length/2,3for consistency with SQLite's function name.
113-134: Consider extracting common result handling into a helper.The same result-handling pattern is repeated across all functions. A private helper could reduce duplication:
🔎 Example helper extraction
defp handle_single_result(result) do case result do %{"rows" => [[value]]} -> {:ok, value} %{"rows" => []} -> {:ok, nil} %{"error" => reason} -> {:error, reason} {:error, reason} -> {:error, reason} other -> {:error, {:unexpected_response, other}} end endThen each function simplifies to:
def extract(%State{} = state, json, path) when is_binary(json) and is_binary(path) do Native.query_args(state.conn_id, state.mode, :disable_sync, "SELECT json_extract(?, ?)", [json, path]) |> handle_single_result() end
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.beads/issues.jsonl.beads/last-touchedAGENTS.mdCLAUDE.mdlib/ecto_libsql/json.extest/json_helpers_test.exs
🧰 Additional context used
📓 Path-based instructions (1)
AGENTS.md
📄 CodeRabbit inference engine (AGENT.md)
Document agent architecture and design patterns in AGENTS.md
Files:
AGENTS.md
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: When adding SQLite function support, update `lib/ecto/adapters/libsql/connection.ex` with the expression handler and add corresponding tests
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: For type conversion issues, update loaders/dumpers in `lib/ecto/adapters/libsql.ex` following the pattern `loaders/dumpers` functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: Elixir modules must follow the structure: EctoLibSql (DBConnection protocol), EctoLibSql.Native (NIF wrappers), EctoLibSql.State (connection state), Ecto.Adapters.LibSql (main adapter), Ecto.Adapters.LibSql.Connection (SQL generation)
📚 Learning: 2025-12-31T08:05:13.161Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: Elixir modules must follow the structure: EctoLibSql (DBConnection protocol), EctoLibSql.Native (NIF wrappers), EctoLibSql.State (connection state), Ecto.Adapters.LibSql (main adapter), Ecto.Adapters.LibSql.Connection (SQL generation)
Applied to files:
AGENTS.mdlib/ecto_libsql/json.ex
📚 Learning: 2026-01-02T08:55:37.150Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T08:55:37.150Z
Learning: Prefer prepared statements (EctoLibSql.Native.prepare/2) for repeated queries to achieve ~10-15x performance improvement through automatic statement caching
Applied to files:
AGENTS.md.beads/issues.jsonl
📚 Learning: 2025-12-31T08:05:13.161Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: When adding SQLite function support, update `lib/ecto/adapters/libsql/connection.ex` with the expression handler and add corresponding tests
Applied to files:
AGENTS.mdtest/json_helpers_test.exs.beads/issues.jsonllib/ecto_libsql/json.ex
📚 Learning: 2025-12-31T08:05:13.161Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: For type conversion issues, update loaders/dumpers in `lib/ecto/adapters/libsql.ex` following the pattern `loaders/dumpers` functions
Applied to files:
AGENTS.mdlib/ecto_libsql/json.ex
📚 Learning: 2025-12-31T08:05:13.161Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: Work is NOT complete until `git commit` succeeds; mandatory workflow includes: file issues for remaining work, run quality gates, update issue status, commit, clean up, verify, and hand off context
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-31T08:05:13.160Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.160Z
Learning: Modern Rustler auto-detects all `#[rustler::nif]` functions; no manual registration needed. Place new NIF functions in the appropriate module (connection, query, transaction, batch, statement, cursor, replication, metadata, savepoint, or utils)
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-31T08:05:13.161Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: Test coverage must include: happy path, error cases, edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
.beads/issues.jsonl
📚 Learning: 2026-01-02T08:55:37.150Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T08:55:37.150Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT table option when creating migrations to enforce type safety and prevent accidental type mismatches in SQLite
Applied to files:
.beads/issues.jsonl
📚 Learning: 2026-01-02T08:55:37.150Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T08:55:37.150Z
Learning: Applies to priv/repo/migrations/*.exs : Use foreign key constraints and unique indices in migrations to enforce data integrity at the database level
Applied to files:
.beads/issues.jsonl
📚 Learning: 2026-01-02T08:55:37.150Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T08:55:37.150Z
Learning: Applies to priv/repo/migrations/*.exs : Use RANDOM ROWID option for tables where sequential IDs could leak business information or pose security risks (e.g., sessions, API keys)
Applied to files:
.beads/issues.jsonl
📚 Learning: 2025-12-31T08:05:13.160Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.160Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : NEVER use `.unwrap()` in production Rust code - use `safe_lock` helpers instead. Tests MAY use `.unwrap()` for simplicity
Applied to files:
.beads/issues.jsonl
📚 Learning: 2025-12-31T08:05:13.160Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.160Z
Learning: Applies to native/ecto_libsql/src/!(tests)/**/*.rs : In Rust, use `safe_lock(®ISTRY, "function_name context")` for locking registries instead of `.lock().unwrap()`
Applied to files:
.beads/issues.jsonl
🧬 Code graph analysis (2)
test/json_helpers_test.exs (4)
lib/ecto_libsql.ex (1)
handle_execute(148-160)lib/ecto_libsql/query.ex (1)
to_string(49-51)lib/ecto_libsql/json.ex (1)
quote(624-644)lib/ecto/adapters/libsql/connection.ex (1)
insert(563-584)
lib/ecto_libsql/json.ex (2)
native/ecto_libsql/src/query.rs (1)
query_args(33-116)lib/ecto/adapters/libsql/connection.ex (4)
boolean(938-949)join(792-801)replace(647-649)insert(563-584)
🪛 LanguageTool
AGENTS.md
[uncategorized] ~1293-~1293: Possible missing comma found.
Context: ... LibSQL 3.45.1 has JSON1 built into the core with support for both text JSON and eff...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~2765-~2765: If ‘type’ is a classification term, ‘a’ is not necessary. Use “type of”. (The phrases ‘kind of’ and ‘sort of’ are informal if they mean ‘to some extent’.)
Context: ...andEctoLibSql.JSON.type/3` Get the type of a value in JSON at the specified path. *...
(KIND_OF_A)
[typographical] ~2772-~2772: Do not use a colon (:) before a series that is introduced by a preposition (‘of’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...urns:** {:ok, type} where type is one of: null, true, false, integer, real, text,...
(RP_COLON)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (15)
CLAUDE.md (1)
253-295: Excellent documentation for Beads workflow!The added issue tracking guidance is comprehensive and practical. The clear distinction between Beads and TodoWrite, along with the typical workflow examples, will help developers use the system effectively.
test/json_helpers_test.exs (1)
1-696: Excellent comprehensive test coverage for JSON helpers!The test suite is well-structured with 54+ tests covering all aspects of JSON/JSONB functionality:
- ✅ Extraction, type detection, validation
- ✅ Array/object construction
- ✅ Iteration and tree traversal
- ✅ JSONB binary format conversion
- ✅ JSON modification operations (set, replace, insert, remove, patch)
- ✅ Edge cases (unicode, deep nesting, large arrays, special characters)
- ✅ Ecto integration and round-trip scenarios
- ✅ Environment-aware skipping for version-dependent features
The chained modification tests (lines 651-695) are particularly valuable for verifying real-world usage patterns.
AGENTS.md (2)
1291-1574: Outstanding documentation for JSON schema helpers!The JSON Schema Helpers section is comprehensive and production-ready:
- ✅ Clear introduction with context about LibSQL 3.45.1 JSON1 support
- ✅ Practical examples for all functions
- ✅ JSONB binary format well explained (5-10% storage savings)
- ✅ Arrow operator distinction clear (-> vs ->>)
- ✅ Excellent Ecto integration patterns
- ✅ Real-world settings management example is production-quality (lines 1447-1536)
- ✅ Clear comparison of set/replace/insert behaviours
- ✅ Actionable performance notes
The settings management example particularly stands out as a complete, practical pattern that developers can adapt for their applications.
2748-2846: Comprehensive API reference for JSON helpers.The API documentation follows the established format and provides clear descriptions for all 16+ JSON functions with proper parameter and return value documentation.
lib/ecto_libsql/json.ex (11)
1-79: LGTM! Comprehensive module documentation.The moduledoc provides excellent coverage of the JSON1 extension capabilities, usage patterns with Ecto, and JSONB benefits.
110-135: LGTM! Safe parameterised query with comprehensive error handling.The function uses placeholders for user input and handles all response patterns appropriately.
160-184: LGTM!Consistent implementation with safe parameterised queries.
206-230: LGTM!Correctly maps SQLite's integer return (1/0) to Elixir booleans.
259-282: LGTM! Safe dynamic placeholder construction.User values are correctly passed as parameterised arguments, not interpolated into SQL.
320-348: LGTM! Good input validation.The even-argument check before execution prevents confusing database errors.
381-410: LGTM!Clean transformation of query results into structured tuples.
507-536: LGTM!Good fail-fast behaviour with
ArgumentErrorfor invalid format values.
771-801: LGTM! Flexible API with safe parameterised queries.Good design supporting both single path and list of paths.
829-851: LGTM!Clean implementation with proper parameterisation.
1024-1052: LGTM!Good handling of the empty/NULL result edge case with the
%{"rows" => [[]]}pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Fix all issues with AI Agents 🤖
In @AGENTS.md:
- Around line 1391-1393: The Ecto example incorrectly calls fragment/1 with
multiple arguments; replace it by calling fragment with a SQL template string
containing placeholders (e.g., '? = ?') and pass JSON.arrow_fragment("settings",
"theme", :double_arrow) as the first placeholder argument and the literal or
bound "dark" as the second argument so the fragment invocation uses the proper
fragment("...?", arg1, arg2) pattern and JSON.arrow_fragment remains the first
argument to that fragment.
- Around line 1362-1365: The example incorrectly calls Ecto.fragment with
JSON.arrow_fragment as if fragment/1 accepted arguments; instead interpolate the
string returned by JSON.arrow_fragment or simply use an inline SQL fragment.
Update the example to either interpolate JSON.arrow_fragment("data", "active",
:double_arrow) into fragment/1 or replace the call with an inline SQL fragment
such as "data ->> 'active' = ?" and pass the value as a bound parameter so the
where clause uses fragment/2 correctly.
In @lib/ecto_libsql/json.ex:
- Line 706: The docstring for json_remove incorrectly claims removing "$[0]" and
"$[2]" from [1,2,3,4,5] yields [2,4,5]; update the json_remove documentation
(docstring for function json_remove) to either show the correct result [2,3,5]
or clarify the behavior that removal is applied in order and changes subsequent
indices; alternatively show the recommended approach (sort/remove paths in
descending index order) to achieve removals based on original indices.
♻️ Duplicate comments (1)
lib/ecto_libsql/json.ex (1)
468-509: SQL injection protections are properly implemented.The
arrow_fragment/3functions now include:
validate_identifier!/1to ensurejson_columnmatches safe identifier pattern^[a-zA-Z_][a-zA-Z0-9_]*$escape_sql_string/1to escape single quotes in path strings by doubling themThis addresses the SQL injection vulnerability flagged in the previous review.
🧹 Nitpick comments (6)
test/json_helpers_test.exs (4)
64-70: Consider a more robust assertion for array extraction.The assertion
String.contains?(result, ["1", "2", "3"])may match unintended substrings. For example, it would match"10"containing"1". Consider parsing the JSON result for a more precise assertion.🔎 Proposed improvement
test "extracts array as JSON", %{state: state} do json = ~s({"items":[1,2,3]}) {:ok, result} = JSON.extract(state, json, "$.items") # Arrays are returned as JSON text assert is_binary(result) - assert String.contains?(result, ["1", "2", "3"]) + assert result == "[1,2,3]" end
181-186: Consider more specific assertions for special character handling.The test verifies presence of substrings but doesn't validate that escape sequences are correctly formed. For thorough testing of special character escaping, assert the exact JSON output structure.
429-436: Potential false positive in large array test.The assertion
String.contains?(json, Integer.to_string(v))could yield false positives. For instance, checking for"1"would also match within"10","21", etc. Consider parsing the JSON array or using a more precise matching strategy.🔎 Proposed improvement
test "handles large JSON arrays", %{state: state} do values = Enum.to_list(1..100) {:ok, json} = JSON.array(state, values) assert is_binary(json) - # Should contain all values - Enum.each(values, fn v -> - assert String.contains?(json, Integer.to_string(v)) - end) + # Parse and verify the array contents + {:ok, parsed} = Jason.decode(json) + assert parsed == values end
553-556: Assertion may be brittle due to whitespace/formatting.The assertion
String.contains?(result, "[1,2,4,5]")relies on exact formatting. SQLite's JSON output is typically canonical, but for robustness, consider parsing and comparing the decoded arrays.lib/ecto_libsql/json.ex (2)
516-524: Consider handling additional response shapes from Native.query_args.The
handle_single_result/1function handles%{"rows" => [[value]]}and%{"rows" => []}, but based on the Rust implementation innative/ecto_libsql/src/query.rs, the response could also include a"columns"key. While this doesn't affect correctness, documenting the expected response shape would aid maintainability.
110-120: Type spec could be more precise for extract return value.The
@specshows{:ok, term()}but the function can return specific types based on JSON value types (string, integer, float, nil, or JSON string for nested structures). This is acceptable given JSON's dynamic nature, but documenting the possible return types in the spec or docs would be helpful.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
AGENTS.mdlib/ecto_libsql/json.extest/json_helpers_test.exs
🧰 Additional context used
📓 Path-based instructions (1)
AGENTS.md
📄 CodeRabbit inference engine (AGENT.md)
Document agent architecture and design patterns in AGENTS.md
Files:
AGENTS.md
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: When adding SQLite function support, update `lib/ecto/adapters/libsql/connection.ex` with the expression handler and add corresponding tests
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: Elixir modules must follow the structure: EctoLibSql (DBConnection protocol), EctoLibSql.Native (NIF wrappers), EctoLibSql.State (connection state), Ecto.Adapters.LibSql (main adapter), Ecto.Adapters.LibSql.Connection (SQL generation)
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: For type conversion issues, update loaders/dumpers in `lib/ecto/adapters/libsql.ex` following the pattern `loaders/dumpers` functions
📚 Learning: 2025-12-31T08:05:13.161Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: When adding SQLite function support, update `lib/ecto/adapters/libsql/connection.ex` with the expression handler and add corresponding tests
Applied to files:
test/json_helpers_test.exsAGENTS.mdlib/ecto_libsql/json.ex
📚 Learning: 2025-12-31T08:05:13.161Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: Elixir modules must follow the structure: EctoLibSql (DBConnection protocol), EctoLibSql.Native (NIF wrappers), EctoLibSql.State (connection state), Ecto.Adapters.LibSql (main adapter), Ecto.Adapters.LibSql.Connection (SQL generation)
Applied to files:
AGENTS.mdlib/ecto_libsql/json.ex
📚 Learning: 2026-01-02T08:55:37.150Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T08:55:37.150Z
Learning: Prefer prepared statements (EctoLibSql.Native.prepare/2) for repeated queries to achieve ~10-15x performance improvement through automatic statement caching
Applied to files:
AGENTS.md
📚 Learning: 2025-12-31T08:05:13.161Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-31T08:05:13.161Z
Learning: For type conversion issues, update loaders/dumpers in `lib/ecto/adapters/libsql.ex` following the pattern `loaders/dumpers` functions
Applied to files:
AGENTS.mdlib/ecto_libsql/json.ex
🧬 Code graph analysis (2)
test/json_helpers_test.exs (4)
lib/ecto_libsql.ex (1)
handle_execute(148-160)lib/ecto_libsql/query.ex (1)
to_string(49-51)lib/ecto_libsql/json.ex (1)
json_quote(595-604)lib/ecto/adapters/libsql/connection.ex (2)
replace(647-649)insert(563-584)
lib/ecto_libsql/json.ex (2)
native/ecto_libsql/src/query.rs (1)
query_args(33-116)lib/ecto/adapters/libsql/connection.ex (3)
boolean(938-949)join(792-801)replace(647-649)
🪛 LanguageTool
AGENTS.md
[uncategorized] ~1293-~1293: Possible missing comma found.
Context: ... LibSQL 3.45.1 has JSON1 built into the core with support for both text JSON and eff...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~2765-~2765: If ‘type’ is a classification term, ‘a’ is not necessary. Use “type of”. (The phrases ‘kind of’ and ‘sort of’ are informal if they mean ‘to some extent’.)
Context: ...andEctoLibSql.JSON.type/3` Get the type of a value in JSON at the specified path. *...
(KIND_OF_A)
[typographical] ~2772-~2772: Do not use a colon (:) before a series that is introduced by a preposition (‘of’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...urns:** {:ok, type} where type is one of: null, true, false, integer, real, text,...
(RP_COLON)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Rust Fuzz Testing
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (8)
test/json_helpers_test.exs (2)
1-25: Well-structured test setup with comprehensive coverage.The test module is well-organised with a clean setup that creates an in-memory database and test table. The use of
describeblocks for logical grouping makes the tests maintainable.
463-497: Good use of environment-dependent test handling.The
casepattern for handlingjson_lengthavailability across different SQLite/libSQL versions is a sensible approach. This prevents test failures on systems where the function isn't available while still testing the functionality where it is.AGENTS.md (3)
1291-1318: Comprehensive documentation for JSON helpers.The JSON Schema Helpers section provides clear examples and explanations. The documentation accurately reflects the implementation in
lib/ecto_libsql/json.ex.
1410-1416: Documentation shows incorrect depth for nested object.The example shows
depthreturning 4 for{"a":{"b":{"c":1}}}, but according to the implementation and test at line 525-530, this should return 4 (root=1, a=2, b=3, c=4). However, looking at the test for{"a":{"b":1}}expecting depth 3, the nesting{"a":{"b":{"c":1}}}should indeed be depth 4. This appears correct.
2748-2847: API reference section is well-documented.The API reference provides clear parameter descriptions, return types, and concise explanations for each JSON helper function. This aligns well with the implementation.
lib/ecto_libsql/json.ex (3)
1-79: Well-documented module with comprehensive JSON1 wrapper.The module documentation is thorough, covering the JSON1 extension features, usage patterns, and examples. The structure follows Elixir conventions and the learnings about module structure.
906-917:keys/3useshandle_nullable_result/1.I see that
handle_nullable_result/1is actually used here at line 916 for thekeys/3function. My earlier observation about it being unused was incorrect - this is the correct handler for functions that can return NULL for non-object inputs.
537-547: This function is actually called at line 916 of the same file via the pipe operator, so it is not unused code.Likely an incorrect or invalid review comment.
Add JSON and JSONB support, tests and documentation
Summary by CodeRabbit
New Features
Tests
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.