Skip to content

feat(compiler2): mathematical baml.ops operator interfaces (B-289) - #4042

Merged
codeshaunted merged 12 commits into
canaryfrom
avery/b-289
Jul 16, 2026
Merged

codeshaunted merged 12 commits into
canaryfrom
avery/b-289

Conversation

@codeshaunted

@codeshaunted codeshaunted commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Implements B-289: + - * / % and unary - dispatch through new baml.ops.{Add,Subtract,Multiply,Divide,Remainder,Negate} interfaces, so user types can overload arithmetic operators. Takes over kai/math-ops and merges current canary into it.

  • baml.ops interfaces with Rhs extends root.Concrete and type Output extends root.Concrete = Self, plus the int/float/bigint impl matrix (mixed pairs widen; int wraps at i63; integer / % by zero panics DivisionByZero; float stays IEEE-754). New baml.Concrete marker interface with a blanket impl.
  • TIR: primitives keep the fast infer_arithmetic classifier; anything else resolves <lhs as Op<rhs>>::Output via associated-type projection. Union operands take the cartesian product of alternatives (all pairs must be implemented; result is the union of their Outputs).
  • MIR: statically-primitive operands keep specialized opcodes; everything else lowers to a call to the baml.ops.__union_* runtime drivers.
  • VM: drive_binary_op double-dispatches on both operands' runtime concrete types through resolve_implements_rule and tail-calls the impl method.

Merge adaptations vs kai/math-ops: ops driver ported to the RealizedTy resolver API; kai's dual-package implements fix dropped (canary's implements_interface already searches all involved packages); interfaces declare requires root.Concrete so the symbolic Self default for Output discharges its bound; associated types referenced as Self.Output.

Deferred: string + T / string * int impls (commented out), compound-assignment coverage for user types.

Closes B-289

Summary by CodeRabbit

  • New Features
    • Added numeric baml.ops interfaces and native implementations for + - * / % and unary - across int, float, and bigint, including union-aware runtime dispatch.
    • Introduced a “concrete-only” constraint to improve operator implementation binding/type checking.
  • Bug Fixes
    • Improved operator and arithmetic/compound-assignment lowering to keep opcode and interface-driver behavior consistent.
    • Refined interface-implementation selection to prefer exact interface-argument matches.
  • Tests
    • Expanded dispatch/interface typing and panic/parity coverage for user types, unions, and primitives.

2kai2kai2 and others added 5 commits June 17, 2026 17:11
Currently just normal interfaces with no magic operator overloading
behavior
- Primitives are special-pathed to bytecode
- Other types are run via a driver (TODO: same-concrete-type can emit
  direct function call). This includes unions: `A | B + C | D` will be
  valid if all pairs implement `baml.ops.Add` and the output type is the
  union of their outputs.
Conflict resolution beyond textual merges:
- bex_vm ops.rs driver: value_concrete_runtime_ty is gone; use
  value_concrete_ty + RealizedTy resolver API, baked method fqn pointers,
  and fallible realize_frame
- TIR builder.rs: drop kai's dual-package implements fix (canary's
  implements_interface already searches all involved packages); route
  operator Output projection through TypeContext::normalize/project
- check.rs: take canary's side (implements validation moved to TIR
  impl_rules phase-5 conformance)
- math.baml: interfaces now 'requires root.Concrete' so the symbolic
  Self default for Output discharges its bound; associated types are
  referenced as Self.Output per current syntax
Conflict-resolved to canary's side during the merge; regenerated against
the merged stdlib (baml.ops.{Add..Negate,__union_*} + baml.Concrete)
@linear

linear Bot commented Jul 15, 2026

Copy link
Copy Markdown

B-289

@vercel

vercel Bot commented Jul 15, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
beps Ready Ready Preview, Comment Jul 16, 2026 7:23pm
promptfiddle Ready Ready Preview, Comment Jul 16, 2026 7:23pm
promptfiddle2 Ready Ready Preview, Comment Jul 16, 2026 7:23pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds interface-based arithmetic dispatch for user-defined types and unions, compiler type inference and lowering support, runtime implementation resolution, native numeric operators, and compile-time/runtime tests.

Changes

Arithmetic Interface Dispatch

Layer / File(s) Summary
Operator contracts and builtin extraction
baml_language/crates/baml_builtins2/..., baml_language/crates/baml_builtins2_codegen/src/extract.rs
Defines Concrete, arithmetic interfaces, numeric implementations, union driver declarations, builtin registration, and method-level native directives.
Compiler operator resolution
baml_language/crates/baml_compiler2_tir/src/builder.rs, baml_language/crates/baml_compiler2_mir/src/lower.rs
Resolves operator interfaces during type inference and lowers non-primitive arithmetic, negation, and compound assignments through runtime drivers.
Native numeric operations
baml_language/crates/bex_vm/src/package_baml/ops_math.rs
Implements arithmetic and negation for int, float, bigint, and mixed numeric operands with VM-compatible numeric and panic semantics.
Runtime union dispatch
baml_language/crates/bex_vm/src/package_baml/{mod.rs,ops.rs,resolve.rs}
Registers operator entrypoints, performs exact interface-rule resolution, and invokes concrete implementations.
Operator validation
baml_language/crates/baml_tests/...
Tests user-defined operators, negation, unions, associated outputs, existentials, numeric parity, panic behavior, compound assignment, and associated-type projection behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BAMLSource
  participant TIRBuilder
  participant MIRLowerer
  participant PackageBamlOps
  participant ImplementsResolver
  participant NativeOperator
  BAMLSource->>TIRBuilder: infer operator interface and Output
  TIRBuilder->>MIRLowerer: lower non-primitive operator
  MIRLowerer->>PackageBamlOps: call __union_add or __union_neg
  PackageBamlOps->>ImplementsResolver: resolve concrete implementation
  ImplementsResolver->>NativeOperator: invoke resolved operator
  NativeOperator-->>PackageBamlOps: return arithmetic result
  PackageBamlOps-->>BAMLSource: forward operator result
Loading

Possibly related PRs

  • BoundaryML/baml#3455: Relates to the primitive arithmetic opcode paths that the new driver lowering parallels.
  • BoundaryML/baml#3821: Relates to the integer overflow and catchable panic behavior used by native arithmetic.
  • BoundaryML/baml#4046: Also changes implements-rule resolution used by runtime operator dispatch.

Suggested reviewers: 2kai2kai2, aaronvg, hellovai

Poem

A rabbit hops through plus and minus,
Union paths now choose with kindness.
Bigints grow and floats glide,
Native panics stay inside.
“Hop!” says the bunny, “ops now flow!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding mathematical baml.ops operator interfaces for compiler2.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch avery/b-289

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.

@github-actions

Copy link
Copy Markdown

⏭️ Performance benchmarks were skipped

Perf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to canary/main.

To run them on this PR, do any of the following, then push a commit (or re-run CI):

  • Add RUN_CODSPEED=1 to the PR description, or
  • Include run-perf or /perf in the PR title or any commit message.

@codeshaunted
codeshaunted marked this pull request as ready for review July 15, 2026 20:01
@github-actions

github-actions Bot commented Jul 15, 2026 •

Copy link
Copy Markdown

Binary size checks passed

✅ 7 passed

Artifact Platform File Gzip Gated on Baseline Delta Status
✅ baml-cli Linux 🔒 23.1 MB 9.8 MB file 22.8 MB +371.2 KB (+1.6%) OK
✅ packed-program Linux 🔒 16.4 MB 6.9 MB file 16.3 MB +178.6 KB (+1.1%) OK
✅ baml-cli macOS 🔒 17.8 MB 8.6 MB file 17.5 MB +302.3 KB (+1.7%) OK
✅ packed-program macOS 🔒 12.8 MB 6.0 MB file 12.6 MB +116.4 KB (+0.9%) OK
✅ baml-cli Windows 🔒 19.3 MB 8.7 MB file 18.7 MB +539.1 KB (+2.9%) OK
✅ packed-program Windows 🔒 13.7 MB 6.1 MB file 13.5 MB +140.2 KB (+1.0%) OK
✅ bridge_wasm WASM 15.3 MB 🔒 4.3 MB gzip 4.3 MB +30.1 KB (+0.7%) OK

🔒 = the size this artifact is GATED on (ceiling + delta). Binaries gate on file size (installed binary); WASM gates on gzip (download size). The other size is shown for information only.


Generated by cargo size-gate · workflow run

…r plumbing

- math.baml: delete the commented-out string Add/Multiply impls; string
  concat is deliberately fast-path only and string + <non-string> stays
  a type error (user-decided)
- bex_vm ops.rs: drop ForwardResult, reuse the existing
  PassThroughContinuation
- MIR lower.rs: extract lower_via_ops_driver as the shared shape of
  equals_equals/__union_* driver calls; merge the two is_primitive
  closures into arith_primitive(ty, include_string). Emitted MIR is
  unchanged (bytecode snapshots pass untouched)
@vercel
vercel Bot temporarily deployed to Preview – beps July 15, 2026 20:23 Inactive

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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/baml_std/baml/ns_ops/math.baml`:
- Around line 52-55: Update the Negate interface’s neg method to constrain its
self type to root.Concrete, matching the requirements of the other arithmetic
operator interfaces while keeping neg available only for concrete types.

In `@baml_language/crates/bex_vm/src/package_baml/ops_math.rs`:
- Around line 85-92: Adjust checked_bigint_mul so its size estimate does not
reject products that actually fit within MAX_BIGINT_BITS, including multiplying
a boundary-sized bigint by 1. Preserve rejection for products exceeding the
limit, and add a regression test covering the boundary case.

In `@baml_language/crates/bex_vm/src/package_baml/ops.rs`:
- Around line 223-227: Update the dispatch flow around resolve_implements_rule
to require an exact match between the requested RHS interface arguments and the
selected implementation before invoking it. Prevent fallback to the first
applicable specialization for non-empty iface_args; if no exact match exists,
return unresolved_op(iface, method) so the result is UnresolvedVirtualCall.
🪄 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: b1ea49a3-7dc8-4de9-a9a1-bcb0686c591e

📥 Commits

Reviewing files that changed from the base of the PR and between 91495a4 and 3984d25.

⛔ Files ignored due to path filters (9)
  • baml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/snapshots/baml_src/operators.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snap is excluded by !**/*.snap
📒 Files selected for processing (11)
  • baml_language/crates/baml_builtins2/baml_std/baml/core.baml
  • baml_language/crates/baml_builtins2/baml_std/baml/ns_ops/math.baml
  • baml_language/crates/baml_builtins2/src/lib.rs
  • baml_language/crates/baml_builtins2_codegen/src/extract.rs
  • baml_language/crates/baml_compiler2_mir/src/lower.rs
  • baml_language/crates/baml_compiler2_tir/src/builder.rs
  • baml_language/crates/baml_tests/baml_src/ns_operators/operators.baml
  • baml_language/crates/baml_tests/tests/interfaces.rs
  • baml_language/crates/bex_vm/src/package_baml/mod.rs
  • baml_language/crates/bex_vm/src/package_baml/ops.rs
  • baml_language/crates/bex_vm/src/package_baml/ops_math.rs

@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 July 15, 2026 20:32 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle July 15, 2026 20:43 Inactive
- operator_operand_members: dedup alternatives by equivalence after
  literal widening so (1 | 2) + x resolves the int pair once
- ops_qtn(name) replaces the two inline baml.ops QualifiedTypeName
  constructions
@vercel
vercel Bot temporarily deployed to Preview – beps July 15, 2026 20:49 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 July 15, 2026 20:56 Inactive
- Negate requires root.Concrete, matching the binary op interfaces
- operator dispatch resolves via resolve_implements_rule_exact: on a
  proof/registry desync a near-miss specialization must surface as
  UnresolvedVirtualCall, not be silently called with a mismatched rhs
  (the shared resolve_implements_rule keeps its defensive fallback for
  empty-request callers)

checked_bigint_mul's one-bit over-estimate is deliberate: it mirrors the
Mul opcode preflight in vm.rs so operator and interface dispatch agree
@vercel
vercel Bot temporarily deployed to Preview – beps July 15, 2026 21:09 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 July 15, 2026 21:17 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle July 15, 2026 21:29 Inactive
…n and existential soundness

Four bugs from the B-289 test audit, each with its regression test:

- compound assignment: TIR now requires the operator result to be
  assignable back to the target (Counter += 1 with Output = int was
  accepted, leaving an int in a Counter slot), and MIR routes OP= on
  non-primitive operands through the __union_* drivers via a shared
  emit_assign_op (v += w on a user class compiled to a raw opcode and
  died with a VM internal error)
- opcode parity: the ops_math natives were written against pre-B-266
  wrapping semantics; int add/sub/mul/div/neg now throw catchable
  IntegerOverflow (incl. INT_MIN / -1 and -INT_MIN) and float / by zero
  throws DivisionByZero, exactly like the opcodes; stale docs rewritten
- primitive fast path: infer_arithmetic's wildcard arms claimed
  user_class + float/string for the fast path (typing it by the
  primitive operand alone), and mixed-kind primitive unions promoted
  statically while the runtime value could be the other kind (UB in the
  specialized opcodes via unreachable_unchecked); both TIR and MIR now
  keep an operand on the fast path only when it is single-kind (builtin
  wrapper classes like baml.Float count as their primitive; null union
  members from chain-narrowed compound assigns stay transparent)
- unpinned Output: an operand whose <l as Op<r>>::Output realizes to the
  = Self default existential (bare baml.ops.Add<int> param, or a
  typevar bound without an Output pin) is rejected instead of unsoundly
  typing (x + 1) + 1

Coverage backfill from the audit: user-type Sub/Mul/Div/Rem dispatch,
multi-instantiation selection by rhs runtime type (Add<int> + Add<float>),
Output != Self at runtime, driver-vs-opcode parity grid (values and all
catchable panics via bounded-generic dispatch), mixed-union runtime test.
string + uint8array/media now errors statically (it was already broken at
runtime on this branch: MIR routed it to a driver with no impl)
@vercel
vercel Bot temporarily deployed to Preview – beps July 16, 2026 01:59 Inactive

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
baml_language/crates/baml_compiler2_tir/src/builder.rs (2)

13735-13758: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Unsound primitive filtering permits crashing VM operations.

The is_non_object_primitive helper fails to match Ty::Union, Ty::Class (e.g., baml.Int), and Ty::TypeAlias. As a result, it returns false for these types.

When lhs is Ty::String (where is_non_object_primitive correctly yields false) and rhs is Ty::Union(Int, Int(1)), the union also yields false. Both !is_non_object_primitive checks evaluate to true, mistakenly allowing string + (int | 1) to compile. The VM will then abort when attempting to invoke as_string on the tagged integer.

We can completely remove is_non_object_primitive and cleanly close this loophole by strictly matching (Some(PrimitiveType::String), Some(PrimitiveType::String)) on the already-resolved base types.

🛡️ Proposed fix to ensure type safety
-            (Some(PrimitiveType::String), _) | (_, Some(PrimitiveType::String)) => {
-                // String concatenation, Add only, and only string + string:
-                // both operands are known primitive-based here (the None arm
-                // above already sent everything else to the interface path),
-                // so the non-object-primitive check rejects exactly `string +
-                // int/float/bigint` — the VM concatenates objects via
-                // `as_string`, and a tagged primitive has no object
-                // representation. `string` deliberately has no `baml.ops`
-                // impls (see math.baml), so no interface fallback applies.
-                if matches!(op, baml_compiler2_ast::BinaryOp::Add)
-                    && !Self::is_non_object_primitive(lhs)
-                    && !Self::is_non_object_primitive(rhs)
-                {
-                    Ty::String {
-                        attr: TyAttr::default(),
-                    }
-                } else {
-                    Ty::Unknown {
-                        attr: TyAttr::default(),
-                    }
-                }
-            }
+            (Some(PrimitiveType::String), Some(PrimitiveType::String)) => {
+                // String concatenation, Add only, and only string + string.
+                // `string` deliberately has no `baml.ops` impls (see math.baml),
+                // so no interface fallback applies.
+                if matches!(op, baml_compiler2_ast::BinaryOp::Add) {
+                    Ty::String {
+                        attr: TyAttr::default(),
+                    }
+                } else {
+                    Ty::Unknown {
+                        attr: TyAttr::default(),
+                    }
+                }
+            }
+            (Some(PrimitiveType::String), _) | (_, Some(PrimitiveType::String)) => {
+                Ty::Unknown {
+                    attr: TyAttr::default(),
+                }
+            }

(You can also completely remove the now-unused is_non_object_primitive helper defined at lines 13630–13647).

🤖 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_compiler2_tir/src/builder.rs` around lines 13735 -
13758, Replace the `is_non_object_primitive` checks in the string-concatenation
branch with a strict match requiring both already-resolved operand base types to
be `Some(PrimitiveType::String)`, returning `Ty::String` only for
string-plus-string. Remove the now-unused `is_non_object_primitive` helper and
preserve `Ty::Unknown` for all other operand types, including unions, classes,
and type aliases.

7365-7388: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Update the tracked local type in AssignOp. AssignOp should mirror plain Assign for simple local paths and call assign_local(...); otherwise a flow-typed local like let x = 1; x += 2; stays recorded as Literal(1) and can mislead later inference/narrowing.

🤖 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_compiler2_tir/src/builder.rs` around lines 7365 -
7388, The AssignOp handling around the result-type compatibility check must
update tracked local types like plain Assign. For simple local assignment
targets, call assign_local(...) with the computed result type; preserve the
existing compatibility validation and avoid applying local tracking for
non-local or complex paths.

Source: Coding guidelines

🧹 Nitpick comments (2)
baml_language/crates/baml_tests/baml_src/ns_operators/operators.baml (2)

368-374: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise specialization selection through a union-typed RHS.

These monomorphic wrappers do not verify runtime RHS selection. Route both values through one int | float parameter so a broken union double-dispatch path cannot pass.

Proposed regression coverage
-function ops_poly_add_int(p: Poly, n: int) -> Poly { p + n }
-function ops_poly_add_float(p: Poly, f: float) -> Poly { p + f }
+function ops_poly_add(p: Poly, rhs: int | float) -> Poly { p + rhs }

 test "operator_multi_instantiation_selects_by_rhs_type" {
-    assert.is_true(baml.deep_equals(ops_poly_add_int(Poly { v: 1 }, 5), Poly { v: 6 }))
-    assert.is_true(baml.deep_equals(ops_poly_add_float(Poly { v: 1 }, 0.5), Poly { v: 1001 }))
+    assert.is_true(baml.deep_equals(ops_poly_add(Poly { v: 1 }, 5), Poly { v: 6 }))
+    assert.is_true(baml.deep_equals(ops_poly_add(Poly { v: 1 }, 0.5), Poly { v: 1001 }))
 }

The PR objectives explicitly call out runtime RHS selection and mixed-union behavior.

🤖 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_tests/baml_src/ns_operators/operators.baml` around
lines 368 - 374, Update the operator specialization test around ops_poly_add_int
and ops_poly_add_float to add a single wrapper accepting an int | float RHS,
then invoke it with both integer and float values. Assert the expected Poly
results through this union-typed path so runtime RHS specialization and
mixed-union dispatch are exercised rather than only monomorphic wrappers.

432-446: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Complete the primitive driver parity matrix.

Float omits -, *, and %; bigint omits -, *, %, and unary -; mixed widening only tests +. Add driver assertions for these paths, especially float % 0.0, whose required result is NaN rather than a panic.

The PR objectives claim native numeric parity across the arithmetic interfaces.

🤖 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_tests/baml_src/ns_operators/operators.baml` around
lines 432 - 446, Expand the arithmetic parity tests in
driver_float_arithmetic_matches_opcodes,
driver_bigint_arithmetic_matches_opcodes, and
driver_mixed_widening_matches_opcodes to cover subtraction, multiplication,
modulo, bigint unary negation, and mixed widening for each supported operator.
Include the float modulo-by-zero case and assert that drv_float_mod returns NaN
without panicking, while preserving the existing deep-equality checks and
operand types.
🤖 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.

Outside diff comments:
In `@baml_language/crates/baml_compiler2_tir/src/builder.rs`:
- Around line 13735-13758: Replace the `is_non_object_primitive` checks in the
string-concatenation branch with a strict match requiring both already-resolved
operand base types to be `Some(PrimitiveType::String)`, returning `Ty::String`
only for string-plus-string. Remove the now-unused `is_non_object_primitive`
helper and preserve `Ty::Unknown` for all other operand types, including unions,
classes, and type aliases.
- Around line 7365-7388: The AssignOp handling around the result-type
compatibility check must update tracked local types like plain Assign. For
simple local assignment targets, call assign_local(...) with the computed result
type; preserve the existing compatibility validation and avoid applying local
tracking for non-local or complex paths.

---

Nitpick comments:
In `@baml_language/crates/baml_tests/baml_src/ns_operators/operators.baml`:
- Around line 368-374: Update the operator specialization test around
ops_poly_add_int and ops_poly_add_float to add a single wrapper accepting an int
| float RHS, then invoke it with both integer and float values. Assert the
expected Poly results through this union-typed path so runtime RHS
specialization and mixed-union dispatch are exercised rather than only
monomorphic wrappers.
- Around line 432-446: Expand the arithmetic parity tests in
driver_float_arithmetic_matches_opcodes,
driver_bigint_arithmetic_matches_opcodes, and
driver_mixed_widening_matches_opcodes to cover subtraction, multiplication,
modulo, bigint unary negation, and mixed widening for each supported operator.
Include the float modulo-by-zero case and assert that drv_float_mod returns NaN
without panicking, while preserving the existing deep-equality checks and
operand types.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 69b335cc-f3a9-4b14-a5bb-19aea64de372

📥 Commits

Reviewing files that changed from the base of the PR and between c843064 and 1b42408.

⛔ Files ignored due to path filters (3)
  • baml_language/crates/baml_tests/snapshots/baml_src/operators.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snap is excluded by !**/*.snap
📒 Files selected for processing (6)
  • baml_language/crates/baml_builtins2/baml_std/baml/ns_ops/math.baml
  • baml_language/crates/baml_compiler2_mir/src/lower.rs
  • baml_language/crates/baml_compiler2_tir/src/builder.rs
  • baml_language/crates/baml_tests/baml_src/ns_operators/operators.baml
  • baml_language/crates/baml_tests/tests/interfaces.rs
  • baml_language/crates/bex_vm/src/package_baml/ops_math.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • baml_language/crates/baml_compiler2_mir/src/lower.rs
  • baml_language/crates/baml_builtins2/baml_std/baml/ns_ops/math.baml
  • baml_language/crates/baml_tests/tests/interfaces.rs

@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 July 16, 2026 02:06 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle July 16, 2026 02:19 Inactive
…ncat typing

- builtin package listing: line numbers shifted by the math.baml edits
- builtin_io corpus: uint8array + string now errors at compile time
  (E0004) instead of compiling to a doomed driver call; on canary it
  concatenated via the string wildcard arm, but B-289's MIR routing had
  already broken it at runtime, and the string decision (no baml.ops
  impls for string-likes) makes the static error the intended behavior
- interfaces_associated_types fixture: render the int | string catch
  results via to_string() - a mixed union is not a valid concat operand
@vercel
vercel Bot temporarily deployed to Preview – beps July 16, 2026 02:34 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 July 16, 2026 02:40 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle July 16, 2026 02:54 Inactive
An unreachable catch arm's binding is never-typed; 'prefix' + other in
such an arm previously slipped through the string wildcard arm and must
not become E0004 under the stricter fast path - bottom propagates with
no diagnostic
@vercel
vercel Bot temporarily deployed to Preview – beps July 16, 2026 18:23 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 July 16, 2026 18:30 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle July 16, 2026 18:44 Inactive
Negate now mirrors Rust's Neg and the binary op interfaces:
type Output extends root.Concrete = Self, so -x can change type
(class Debt { implements Negate { type Output = int ... } }). TIR
negate inference resolves each operand alternative's Output projection
and unions the results instead of returning the operand type; the
unpinned-Output rejection now applies to Negate too (a bare
T extends baml.ops.Negate bound must pin Output to use -x).
Existing impls are unaffected - the = Self default covers them
@vercel
vercel Bot temporarily deployed to Preview – beps July 16, 2026 19:02 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 July 16, 2026 19:09 Inactive

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
baml_language/crates/baml_compiler2_tir/src/builder.rs (1)

13979-14011: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Handle Ty::Never in unary Neg

infer_unary_op sends a never operand through infer_negate_via_interface, which falls back to InvalidUnaryOp. Expr::Throw produces Ty::Never, so -(throw e) gets a spurious diagnostic even though the path is unreachable. Mirror the binary arithmetic short-circuit here and return Ty::Never before interface dispatch.

🤖 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_compiler2_tir/src/builder.rs` around lines 13979 -
14011, Update infer_unary_op’s UnaryOp::Neg handling to match Ty::Never before
infer_negate_via_interface dispatch, returning Ty::Never directly with its
existing attributes. Preserve the current primitive, unknown/error,
interface-dispatch, and invalid-operation behavior for all other operand types.
🧹 Nitpick comments (1)
baml_language/crates/baml_compiler2_tir/src/builder.rs (1)

13874-13924: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Cartesian-product resolution for infer_arithmetic_via_interface / infer_negate_via_interface: correct, minor duplication.

Both helpers follow the same "resolve each alternative's Output, bail on any failure via ?, join successes" shape — differing only in whether rhs is threaded through. This is a small, understandable duplication; extracting a shared "join outputs across alternatives" helper would be a nice-to-have but isn't warranted given the modest size and the differing arity (binary vs. unary) of the two call sites.

🤖 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_compiler2_tir/src/builder.rs` around lines 13874 -
13924, No code change is required: the duplication between
infer_arithmetic_via_interface and infer_negate_via_interface is intentional and
acceptable. Preserve their current Cartesian-product and fail-fast resolution
behavior.
🤖 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.

Outside diff comments:
In `@baml_language/crates/baml_compiler2_tir/src/builder.rs`:
- Around line 13979-14011: Update infer_unary_op’s UnaryOp::Neg handling to
match Ty::Never before infer_negate_via_interface dispatch, returning Ty::Never
directly with its existing attributes. Preserve the current primitive,
unknown/error, interface-dispatch, and invalid-operation behavior for all other
operand types.

---

Nitpick comments:
In `@baml_language/crates/baml_compiler2_tir/src/builder.rs`:
- Around line 13874-13924: No code change is required: the duplication between
infer_arithmetic_via_interface and infer_negate_via_interface is intentional and
acceptable. Preserve their current Cartesian-product and fail-fast resolution
behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 58ac5311-46c2-4120-b429-4054616030e3

📥 Commits

Reviewing files that changed from the base of the PR and between 0bdab9a and 9ee0051.

⛔ Files ignored due to path filters (2)
  • baml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/snapshots/baml_src/operators.snap is excluded by !**/*.snap
📒 Files selected for processing (4)
  • baml_language/crates/baml_builtins2/baml_std/baml/ns_ops/math.baml
  • baml_language/crates/baml_compiler2_tir/src/builder.rs
  • baml_language/crates/baml_tests/baml_src/ns_operators/operators.baml
  • baml_language/crates/baml_tests/tests/interfaces.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • baml_language/crates/baml_tests/baml_src/ns_operators/operators.baml
  • baml_language/crates/baml_tests/tests/interfaces.rs
  • baml_language/crates/baml_builtins2/baml_std/baml/ns_ops/math.baml

@vercel
vercel Bot temporarily deployed to Preview – promptfiddle July 16, 2026 19:23 Inactive
@codeshaunted
codeshaunted added this pull request to the merge queue Jul 16, 2026
Merged via the queue into canary with commit 8af9bbc Jul 16, 2026
155 of 160 checks passed
@codeshaunted
codeshaunted deleted the avery/b-289 branch July 16, 2026 21:18

This branch was previously deployed

3 inactive deployments
Preview – promptfiddle — 9ee00517 Deployed Jul 16, 2026 by vercel[bot]
Preview – promptfiddle2 — 9ee00517 Deployed Jul 16, 2026 by vercel[bot]
Preview – beps — 9ee00517 Deployed Jul 16, 2026 by vercel[bot]
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