Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (119)
📒 Files selected for processing (48)
💤 Files with no reviewable changes (12)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (32)
📝 WalkthroughWalkthroughThis PR adds a new baml.time API (Duration and Instant), marks many existing builtins as non-throwing by adding ChangesTime namespace and builtin modernization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Binary size checks passed✅ 7 passed
Generated by |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@baml_language/crates/baml_builtins2_codegen/src/codegen.rs`:
- Around line 1521-1524: The instance-backed receiver branch only handles
!needs_owned and therefore when extraction produced an owned Value for a
VmUsage::MutRef receiver the call-site can pass the Value by value instead of
&Value; update the condition around args.push(format!("&{name}")) to also cover
the MutRef usage. Specifically, in the block using recv.instance_backed and
needs_owned, change the test so it pushes "&{name}" when recv.instance_backed &&
(!needs_owned || recv.usage == VmUsage::MutRef) (or equivalent) so MutRef
receivers extracted as owned Values are passed by reference; keep the existing
branch names (recv.instance_backed, needs_owned, VmUsage::MutRef,
args.push(format!("&{name}"))).
In `@baml_language/crates/baml_compiler2_ast/src/lower_cst.rs`:
- Around line 1183-1195: init_test_key_from_path currently collapses different
paths to the same key by mapping non-alphanumerics to '_', causing duplicate
$init_test_<key> symbols; change it to produce a collision-resistant identifier
by keeping the sanitized base name but appending a short hex hash (e.g., first 8
chars of SHA-1/SHA-256) of the original path (or using a reversible encoding of
path separators) and ensure the final string consists only of ASCII
alphanumerics and underscores; update the function init_test_key_from_path to
compute the hash from the full path string and append it (with an underscore
separator) and apply the same change where this helper is used around the other
occurrence (lines referenced in the review) so each path yields a unique test
key.
In `@baml_language/crates/bex_vm/src/package_baml/time.rs`:
- Around line 38-43: The code currently calls
OffsetDateTime::format(&Rfc3339).unwrap(), which can panic for out-of-range
years; change this to handle the Result and map any formatting error into a
VmRustFnError::BamlError(VmBamlError::InvalidArgument) instead of panicking.
Replace the unwrap with a match/map_err (e.g.,
datetime.format(&Rfc3339).map_err(|e|
VmRustFnError::BamlError(VmBamlError::InvalidArgument { message: format!("Could
not serialize Instant as RFC3339: {}", e) }))? or an equivalent match) so the
function returns Err(...) on formatting failure; target the code around
OffsetDateTime::format(&Rfc3339), Rfc3339, VmRustFnError::BamlError and
VmBamlError::InvalidArgument.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 99735841-5277-40b4-9a57-ed91e94f440c
⛔ Files ignored due to path filters (119)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snapbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_describe_builtin_deep_copy.snapis excluded by!**/*.snapbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_describe_builtin_item_by_definition.snapis excluded by!**/*.snapbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_describe_builtin_string.snapis excluded by!**/*.snapbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_describe_truncation_hint.snapis excluded by!**/*.snapbaml_language/crates/baml_lsp2_actions/src/snapshots/baml_lsp2_actions__describe_tests__describe_builtin_deep_copy_with_compiler2_visible_files.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/_root.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/arrays.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/assignments.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/bigints.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/builtins.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/byte_strings.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/cancel_cascade.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/class_type_args_at_runtime.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/classes.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/closures.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/deep_copy.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/enums.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/env.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/exceptions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/floats.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/for_loops.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/fs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/functions.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/future_methods.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/glob.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/if_else.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/ints.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/is_operator.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/json_alias.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/json_auto_derive.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/json_parse_stringify.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/json_to_from_string.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/lambdas.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/lexical_scoping.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/maps.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/match_basics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/match_optimization.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/match_types.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/null_handling.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/operators.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/optional_function_parameters.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/patterns_new_runtime.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/reflect_type_of.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/reflect_type_of_generic.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/shell.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/soundness.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/spawn_basic.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/spawn_name_object.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/spawn_semantics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/spawn_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/strings.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/time.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/type_error_repro.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/type_reflection.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/typed_inputs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/typed_outputs.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/watch.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/while_loops.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_basic/baml_tests__compiles__test_expr_basic__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_basic/baml_tests__compiles__test_expr_basic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_basic/baml_tests__compiles__test_expr_basic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_basic/baml_tests__compiles__test_expr_basic__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_name_concat/baml_tests__compiles__test_expr_name_concat__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_name_concat/baml_tests__compiles__test_expr_name_concat__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_name_concat/baml_tests__compiles__test_expr_name_concat__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_name_concat/baml_tests__compiles__test_expr_name_concat__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_throwing_body/baml_tests__compiles__test_expr_throwing_body__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_throwing_body/baml_tests__compiles__test_expr_throwing_body__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_throwing_body/baml_tests__compiles__test_expr_throwing_body__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_expr_throwing_body/baml_tests__compiles__test_expr_throwing_body__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_old_and_new/baml_tests__compiles__test_old_and_new__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_old_and_new/baml_tests__compiles__test_old_and_new__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_old_and_new/baml_tests__compiles__test_old_and_new__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_old_and_new/baml_tests__compiles__test_old_and_new__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_raw_string_name/baml_tests__compiles__test_raw_string_name__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_raw_string_name/baml_tests__compiles__test_raw_string_name__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_raw_string_name/baml_tests__compiles__test_raw_string_name__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_raw_string_name/baml_tests__compiles__test_raw_string_name__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_with_not_keyword/baml_tests__compiles__test_with_not_keyword__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_with_not_keyword/baml_tests__compiles__test_with_not_keyword__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_with_not_keyword/baml_tests__compiles__test_with_not_keyword__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/test_with_not_keyword/baml_tests__compiles__test_with_not_keyword__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_basic/baml_tests__compiles__testset_basic__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_basic/baml_tests__compiles__testset_basic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_basic/baml_tests__compiles__testset_basic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_basic/baml_tests__compiles__testset_basic__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_dynamic/baml_tests__compiles__testset_dynamic__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_dynamic/baml_tests__compiles__testset_dynamic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_dynamic/baml_tests__compiles__testset_dynamic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_dynamic/baml_tests__compiles__testset_dynamic__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_nested/baml_tests__compiles__testset_nested__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_nested/baml_tests__compiles__testset_nested__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_nested/baml_tests__compiles__testset_nested__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_nested/baml_tests__compiles__testset_nested__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_vibes_nested/baml_tests__compiles__testset_vibes_nested__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_vibes_nested/baml_tests__compiles__testset_vibes_nested__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_vibes_nested/baml_tests__compiles__testset_vibes_nested__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_vibes_nested/baml_tests__compiles__testset_vibes_nested__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_with_setup/baml_tests__compiles__testset_with_setup__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_with_setup/baml_tests__compiles__testset_with_setup__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_with_setup/baml_tests__compiles__testset_with_setup__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/testset_with_setup/baml_tests__compiles__testset_with_setup__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_name_type_error/baml_tests__diagnostic_errors__test_expr_name_type_error__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_name_type_error/baml_tests__diagnostic_errors__test_expr_name_type_error__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_with_runner/baml_tests__diagnostic_errors__test_expr_with_runner__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_with_runner/baml_tests__diagnostic_errors__test_expr_with_runner__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_wrong_runner/baml_tests__diagnostic_errors__test_expr_wrong_runner__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_expr_wrong_runner/baml_tests__diagnostic_errors__test_expr_wrong_runner__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_with_runner_ambiguity/baml_tests__diagnostic_errors__test_with_runner_ambiguity__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/test_with_runner_ambiguity/baml_tests__diagnostic_errors__test_with_runner_ambiguity__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snap
📒 Files selected for processing (48)
baml_language/Cargo.tomlbaml_language/crates/baml_builtins2/baml_std/baml/bigint.bamlbaml_language/crates/baml_builtins2/baml_std/baml/bool.bamlbaml_language/crates/baml_builtins2/baml_std/baml/containers.bamlbaml_language/crates/baml_builtins2/baml_std/baml/core.bamlbaml_language/crates/baml_builtins2/baml_std/baml/float.bamlbaml_language/crates/baml_builtins2/baml_std/baml/int.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_errors/stack_trace.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_events/events.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_future/future.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_http/http.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_io/io.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_json/json.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_llm/llm_types.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_math/math.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_media/media.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_sys/sys.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_time/duration.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_time/instant.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_unstable/unstable.bamlbaml_language/crates/baml_builtins2/baml_std/baml/null.bamlbaml_language/crates/baml_builtins2/baml_std/baml/string.bamlbaml_language/crates/baml_builtins2/baml_std/baml/type_class.bamlbaml_language/crates/baml_builtins2/baml_std/baml/uint8array.bamlbaml_language/crates/baml_builtins2/src/lib.rsbaml_language/crates/baml_builtins2_codegen/src/codegen.rsbaml_language/crates/baml_builtins2_codegen/src/codegen_io.rsbaml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_builtins2_codegen/src/types.rsbaml_language/crates/baml_compiler2_ast/src/lib.rsbaml_language/crates/baml_compiler2_ast/src/lower_cst.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_hir/src/lib.rsbaml_language/crates/baml_compiler2_ppir/src/lib.rsbaml_language/crates/baml_tests/baml_src/ns_time/time.bamlbaml_language/crates/bex_vm/Cargo.tomlbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/package_baml/stack_trace.rsbaml_language/crates/bex_vm/src/package_baml/time.rsbaml_language/crates/bridge_wasm/Cargo.tomlbaml_language/crates/bridge_wasm/src/lib.rsbaml_language/crates/bridge_wasm/src/wasm_time.rsbaml_language/crates/sys_native/Cargo.tomlbaml_language/crates/sys_native/src/io_impls.rsbaml_language/crates/sys_ops/Cargo.tomlbaml_language/crates/sys_ops/src/lib.rsbaml_language/crates/sys_types/Cargo.toml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
baml_language/crates/baml_builtins2_codegen/src/extract.rs (1)
278-281: ⚡ Quick winExtract the dedicated-variant class list to avoid divergence.
The set
"Array" | "Map" | "String" | "Uint8Array"here must stay in lockstep with the identical match inextract_class_fields(Line 343).instance_backedis only correct while both lists agree; editing one without the other would mark a class asinstance_backedwith no generatedview::struct (or vice versa). A single shared predicate removes that risk.♻️ Proposed helper + call sites
/// Classes that keep a dedicated `Object` variant (not `Object::Instance`), /// so they get neither a `view::` struct nor `instance_backed` receivers. fn is_dedicated_variant(class_name: &str) -> bool { matches!(class_name, "Array" | "Map" | "String" | "Uint8Array") }- // Mirrors `extract_class_fields`: dedicated-variant types and - // field-less (opaque/marker) classes get no `view::` struct. - instance_backed: !matches!(class_name, "Array" | "Map" | "String" | "Uint8Array") - && !class_def.fields.is_empty(), + // Mirrors `extract_class_fields`: dedicated-variant types and + // field-less (opaque/marker) classes get no `view::` struct. + instance_backed: !is_dedicated_variant(class_name) && !class_def.fields.is_empty(),- // Skip classes with dedicated Object variants — they are not Instance-based. - match class_name { - "Array" | "Map" | "String" | "Uint8Array" => return None, - _ => {} - } + // Skip classes with dedicated Object variants — they are not Instance-based. + if is_dedicated_variant(class_name) { + return 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/crates/baml_builtins2_codegen/src/extract.rs` around lines 278 - 281, Extract the duplicate specialized-class match into a single helper to keep `instance_backed` and the logic in `extract_class_fields` consistent: add a function (e.g., `is_dedicated_variant(class_name: &str) -> bool`) that encapsulates the current matches!(class_name, "Array" | "Map" | "String" | "Uint8Array"), then replace the inline matches! usage in the `instance_backed` calculation and the identical match in `extract_class_fields` with calls to `is_dedicated_variant`; ensure the helper name is used in both places so future edits affect both behaviors simultaneously.
🤖 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/crates/baml_builtins2_codegen/src/extract.rs`:
- Around line 278-281: Extract the duplicate specialized-class match into a
single helper to keep `instance_backed` and the logic in `extract_class_fields`
consistent: add a function (e.g., `is_dedicated_variant(class_name: &str) ->
bool`) that encapsulates the current matches!(class_name, "Array" | "Map" |
"String" | "Uint8Array"), then replace the inline matches! usage in the
`instance_backed` calculation and the identical match in `extract_class_fields`
with calls to `is_dedicated_variant`; ensure the helper name is used in both
places so future edits affect both behaviors simultaneously.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: aaf3b7f5-fd5b-4d3a-8657-6f98e5c7d62d
⛔ Files ignored due to path filters (11)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/_root.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/time.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snap
📒 Files selected for processing (21)
baml_language/Cargo.tomlbaml_language/crates/baml_builtins2/baml_std/baml/ns_time/duration.bamlbaml_language/crates/baml_builtins2/baml_std/baml/ns_time/instant.bamlbaml_language/crates/baml_builtins2/src/lib.rsbaml_language/crates/baml_builtins2_codegen/src/codegen.rsbaml_language/crates/baml_builtins2_codegen/src/codegen_io.rsbaml_language/crates/baml_builtins2_codegen/src/extract.rsbaml_language/crates/baml_builtins2_codegen/src/types.rsbaml_language/crates/baml_tests/baml_src/ns_time/time.bamlbaml_language/crates/bex_vm/Cargo.tomlbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/package_baml/stack_trace.rsbaml_language/crates/bex_vm/src/package_baml/time.rsbaml_language/crates/bridge_wasm/Cargo.tomlbaml_language/crates/bridge_wasm/src/lib.rsbaml_language/crates/bridge_wasm/src/wasm_time.rsbaml_language/crates/sys_native/Cargo.tomlbaml_language/crates/sys_native/src/io_impls.rsbaml_language/crates/sys_ops/Cargo.tomlbaml_language/crates/sys_ops/src/lib.rsbaml_language/crates/sys_types/Cargo.toml
✅ Files skipped from review due to trivial changes (3)
- baml_language/crates/bex_vm/Cargo.toml
- baml_language/Cargo.toml
- baml_language/crates/sys_ops/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (14)
- baml_language/crates/bridge_wasm/Cargo.toml
- baml_language/crates/bex_vm/src/package_baml/mod.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_time/duration.baml
- baml_language/crates/baml_builtins2/src/lib.rs
- baml_language/crates/sys_native/Cargo.toml
- baml_language/crates/sys_native/src/io_impls.rs
- baml_language/crates/bridge_wasm/src/lib.rs
- baml_language/crates/baml_tests/baml_src/ns_time/time.baml
- baml_language/crates/bex_vm/src/package_baml/time.rs
- baml_language/crates/bridge_wasm/src/wasm_time.rs
- baml_language/crates/baml_builtins2_codegen/src/codegen_io.rs
- baml_language/crates/sys_ops/src/lib.rs
- baml_language/crates/baml_builtins2/baml_std/baml/ns_time/instant.baml
- baml_language/crates/baml_builtins2_codegen/src/codegen.rs
We previously had massive churn every time a new test file was added, as the `$init_test_<file_id>` was updated for on average half the test files. We now use the relative file path instead of the file id, which makes the init identifier stable if not moved.
We previously allowed `$rust_function` and `$rust_io_function` to not declare an explicit `throws` type. We now require it. Also changed `is_fallible` to use `//baml:fallible` annotations instead of just being a single Rust function that matched on method paths.
Adds the `baml.time` namespace with initial absolute time representations. These currently lack special behaviors like operator overloading or SAP coercion, which rely on either hard-coding or interface implementation.
2kai2kai2
enabled auto-merge
May 29, 2026 00:01
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
baml.time.Instant, representing an absolute point in time with nanosecond granularity. Canonical serialization is an RFC3339 timestamp.baml.time.Duration, representing a difference between twoInstants (a time span).$init_test_<file_id>identifiers to be path-based instead of using an unstable index so adding new files doesn't churn every testthrowsdeclaration (e.g.throws neverorthrows MyErrorType)Note that at this time, neither
InstantnorDurationimplement operator overloading (since the language does not support it currently) andInstantdoes not override SAP coercion (also not currently supported).Summary by CodeRabbit