test(sdk_tests): cover the stdlib json type in type_shapes#3937
Conversation
Adds a `ns_json_values` fixture whose functions are all declared to return `json` (`baml.json.json`) but each return a differently-shaped value — null, bool, int, float, string, array, object, and a nested object — plus a `round_trip_json` encode/decode pair and a `JsonContainer` class with a `json` field. Host-language roundtrip tests assert each shape survives the encode → FFI → decode round trip in both the python/pydantic2 and typescript/node SDKs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
📝 WalkthroughWalkthroughAdds a new BAML fixture defining a recursive ChangesJSON Type Shape Support and Tests
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
baml_language/sdk_tests/fixtures/type_shapes/baml_src/ns_json_values/types.baml (1)
11-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding falsy-value shape coverage.
Current shapes cover null/true/42/3.14/"hello"/array/object/nested, but omit falsy edge cases:
0,"",[],{}. These are valuable because decode paths that mistakenly check truthiness instead ofis not None/presence would silently pass all current tests while failing on these values.♻️ Suggested additional functions
function return_json_nested() -> json { {"a": 1, "b": [2, 3], "c": {"nested": null}} } + +function return_json_zero() -> json { + 0 +} + +function return_json_empty_string() -> json { + "" +} + +function return_json_empty_array() -> json { + [] +} + +function return_json_empty_object() -> json { + {} +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdk_tests/fixtures/type_shapes/baml_src/ns_json_values/types.baml` around lines 11 - 41, Add shape coverage for falsy JSON values in the ns_json_values fixtures by extending the existing return_json_* functions in types.baml with cases for 0, empty string, empty array, and empty object. Keep the current naming/style alongside return_json_null, return_json_bool, and return_json_object, and ensure the fixture exercises these values so decode logic in the json type path is verified against truthiness bugs.baml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/roundtrip_tests/test_json_values.py (1)
26-90: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFalsy-value roundtrip coverage gap (downstream of fixture).
Same gap as the fixture: no test exercises
0,"",[], or{}throughround_trip_json/return_json_*, which are the values most likely to expose truthiness-check bugs in decode logic. Once the corresponding fixture functions exist, mirror the existing test pattern for them (e.g.,assert round_trip_json(j=0) == 0andassert round_trip_json(j=0) is not None).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/roundtrip_tests/test_json_values.py` around lines 26 - 90, Add roundtrip tests in test_json_values for falsy JSON values that are currently uncovered, using the existing round_trip_json and return_json_* helpers. After the corresponding fixture functions are available, verify 0, empty string, empty array, and empty object survive decoding/encoding without being treated as missing values, following the same assertion style as the existing test_round_trip_json_* cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@baml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/roundtrip_tests/test_json_values.py`:
- Around line 26-90: Add roundtrip tests in test_json_values for falsy JSON
values that are currently uncovered, using the existing round_trip_json and
return_json_* helpers. After the corresponding fixture functions are available,
verify 0, empty string, empty array, and empty object survive decoding/encoding
without being treated as missing values, following the same assertion style as
the existing test_round_trip_json_* cases.
In
`@baml_language/sdk_tests/fixtures/type_shapes/baml_src/ns_json_values/types.baml`:
- Around line 11-41: Add shape coverage for falsy JSON values in the
ns_json_values fixtures by extending the existing return_json_* functions in
types.baml with cases for 0, empty string, empty array, and empty object. Keep
the current naming/style alongside return_json_null, return_json_bool, and
return_json_object, and ensure the fixture exercises these values so decode
logic in the json type path is verified against truthiness bugs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 025c3a58-f85b-4a05-bc67-1ffdcfda5afb
📒 Files selected for processing (3)
baml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/roundtrip_tests/test_json_values.pybaml_language/sdk_tests/crates/typescript_node/type_shapes/customizable/roundtrip_json_values.test.tsbaml_language/sdk_tests/fixtures/type_shapes/baml_src/ns_json_values/types.baml
Binary size checks passed✅ 7 passed
Generated by |
Adds SDK e2e coverage for the stdlib
jsontype (baml.json.json).Changes
sdk_tests/fixtures/type_shapes/baml_src/ns_json_values/types.baml: every function is declared to returnjson, but each returns a differently-shaped value —null,bool,int,float,string, array, object, and a nested object. Also includes around_trip_json(j: json) -> jsonencode/decode pair and aJsonContainerclass with ajsonfield.crates/python_pydantic2/type_shapes/customizable/roundtrip_tests/test_json_values.py): asserts each shape decodes to the expected Python value and survives the encode → FFI → decode round trip, including a heterogeneous array.crates/typescript_node/type_shapes/customizable/roundtrip_json_values.test.ts): same coverage ported to vitest.The namespace is
ns_json_values(→baml_sdk.json_values) rather thanns_jsonto avoid colliding with the stdlibbaml.jsonnamespace and the Python stdlibjsonmodule.Test plan
cargo nextest run -p sdk_test_python_pydantic2 -p sdk_test_typescript_node— 29/29 pass locally (ruff, pyright, pytest, tsc, esm_output, vitest across all fixtures).🤖 Generated with Claude Code
Summary by CodeRabbit