[mununki]

Follow-up to #9217, for points 2 and 3 of the review there. Builds on #9288 and #9283, which must be merged first; until then this PR also shows their commits.

This PR changes how three kinds of external arguments are evaluated: @int and @as arguments, event-style polymorphic variant arguments, and unit and @ignore arguments. The JS call does not get any of them as they are. Each is now evaluated once, in its place among the arguments. It is bound to a temporary only when reading it in place would give a different value.

• An @int argument, or a @string argument with @as, is passed to the function that switches on it, so setLevel(t, f()) compiles to t.setLevel((x => { switch (x) { … } })(f())). The argument is evaluated in the caller, so setLevel(t, await p) compiles to (x => …)(await p) without a temporary. The arguments to its left are no longer bound either. The function captures no variable of the caller. The zero-parameter function used before read the argument inside itself, so V8 kept everything the argument reads alive for as long as any closure created in the same call. In the review of this PR, twenty callbacks kept 160 MB alive. 11.1 and 12.3 have this problem too, so the fix has a changelog entry.
• A unit or @ignore argument is not passed. Its effects are now a statement in its place. They used to run before the whole call or object, with every kind of external, in 11.1 and 12.3 as well, so setX(t, f(), g()) called g() first. It now compiles to let tmp = f(); g(); t.setX(tmp);. For an optional @ignore argument, only the payload is evaluated, so an absent one no longer goes through Primitive_option.valFromOption. An external whose arguments are all @ignore is now called without them, like any other external. A @module external of the module itself keeps their effects instead of dropping them. The object of a @send call is used as it is, whatever its spec.
• An event-style argument is read twice, once for the tag and once for the payload. A @val global is read in place again, like a @module value, so on(t, ev) compiles to t.on(ev.NAME, ev.VAL). A JS property read is bound to a temporary. That covers a @get getter, a @val or @module value with @scope, and a @val with a dotted name such as "window.ev". An argument with side effects is bound too, and so is one that a later argument's statements could change.

How it works: Js_of_lam_variant.dispatch builds the function with a parameter (E.ocaml_fun [x], as wrap_then_path does). Lam_compile_external_call.hold_arg binds an event-style argument unless eval_as_event_in_place holds for it. It also turns the effects of a unit or @ignore argument into a statement of that argument. sequence_arguments then evaluates the arguments to the left of such a statement first and binds those the statement could change, as it does for any argument with statements. hold_args leaves the object of a @send call alone. eval_as_event_in_place uses Js_analyzer.is_okay_to_duplicate ~property_reads:false from #9283. On top of that, it accepts array elements, conditionals and fields of them, which would otherwise need a temporary that also binds the arguments to their left. Nothing is hoisted any more. So the effect lists of ocaml_to_js_eff (now ocaml_to_js), assemble_args_* and assemble_obj_args are gone, along with add_eff, Js_exp_make.fuse_to_seq and Js_exp_make.of_block.

What stays the same:

• An argument with side effects is still evaluated once, so the Evaluate a polymorphic variant argument of an external once, in place #9217 entry of 13.0.0-alpha.7 still holds.
• The runtime compiles to the same JavaScript, and the .gen.tsx outputs do not change.

Where a statement cannot go, the output grows:

• A while condition with an effectful unit argument becomes a while (true) loop that breaks.
• Such a call used as an array element or as an argument of another call binds the elements or arguments to its left.

Two known limitations remain:

• A @val getter can give the tag of one event and the payload of another, as with compilers before Evaluate a polymorphic variant argument of an external once, in place #9217.
• %raw code that ends in a line comment still breaks the module as an @int or @as argument, as it does with 12.3. The comment hides the parenthesis that closes the call, which happens for any argument (t.setName("light" // default mode)). That belongs in a separate printer fix.

Checked-in outputs that change:

• polyvar_external_arg_test.mjs and bs_ignore_effect_test.mjs.
• The dispatch functions in Evaluate optional @obj arguments in source order and read properties once #9283's optional_external_arg_none_test.mjs and optional_external_arg_once_test.mjs. The latter also loses a temporary, and one of its sequences becomes statements.
• ImportJsValue.res.js in the gentype tests (two dispatch functions).

Tests: polyvar_external_arg_test.res checks:

• with a WeakRef and gc(), that a closure created next to an @int argument does not keep what the argument reads alive
• the order of the arguments around synchronous and awaited @string and @int arguments
• the order of unit and @ignore arguments after them, labelled, optional and in an @obj literal
• a unit argument that changes an event-style @val global
• that a @val getter is read for the tag and again for the payload, while a dotted @val name is read once

bs_ignore_effect_test.res checks:

• the order of unit and @ignore arguments after plain ones
• a unit argument that changes an earlier one
• an absent optional @ignore argument
• externals whose arguments are all @ignore
• a @send external whose object is marked @ignore

Compiled with #9283, the retention test, the getter test and four of the tests in bs_ignore_effect_test.res fail.

Checklist

• Added a changelog fragment (see changes/README.md), or no release notes are needed and a maintainer should apply changelog:skip.
• Added tests for the fix or feature.
• Gave every commit a description below its subject, saying why it is made (see "Commit messages" in CONTRIBUTING.md).
• Signed off every commit for the DCO (git commit -s).

[github-actions]

Changelog

🐛 Bug fix

• Fix an @int or @as argument of an external keeping what it reads alive for as long as a closure created next to it.
• Evaluate a unit or @ignore argument of an external in source order, as in setX(t, f(), g()).
• Call an external whose arguments are all @ignore without them, like other externals.
• Keep the effects of the @ignore arguments of a @module external that is the module itself.
• Fix a TypeError when an optional external argument such as ~x=?dict->Dict.get(k), or an optional JSX key, is None.
• Evaluate an optional @obj argument such as ~x=?f() in source order.
• Read an optional external argument that is a JS property, such as a @get getter, only once.

Semantic Diff

6 commits · Full page

✨ Pass undefined for an optional external argument or JSX key that is None

_{c907500 · mununki · Nov 09, 2026, 15:21 UTC · new}

Why

An optional argument of an external passed as ~x=?e, where e is a call, a dict or array lookup, a conditional or anything else that cannot be read twice to test it first, is compiled to Primitive_option.valFromOption(e). valFromOption reads a property of its argument to find a nested None, so it threw a TypeError when e was None, though its doc comment says that it keeps undefined. #8380 replaced Primitive_option.toUndefined, which tested for undefined before calling valFromOption, on that premise. The same call is emitted for a JSX key, also with -bs-jsx-preserve, for the argument of an inlined function that forwards ~x? to an external, and for an optional @int argument, or @string argument with @as, which switches on it. valFromOption now returns undefined for undefined, as it does for null, which fixes all of them.

JS modified (Primitive_option.valFromOption)

js

 function valFromOption(x) {
-  if (x === null || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
+  if ((x == null) || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
     return x;
   }
   …
 }

Output modified (optional argument that is None)

%%raw(`function greet(name) { return name === undefined ? "Hello" : "Hello " + name }`)
@val external greet: (~name: string=?, unit) => string = "greet"

let greetUser = (names: dict<string>, id) => greet(~name=?names->Dict.get(id), ())
Console.log(greetUser(dict{"1": "Ann"}, "2"))

js · Before and After are equal

function greetUser(names, id) {
  return greet(Primitive_option.valFromOption(names[id]));
}

text

-TypeError: Cannot read properties of undefined (reading 'BS_PRIVATE_NESTED_SOME_NONE')
+Hello

Review

• Why change the runtime and not the compiler? Every path that emits the unchecked call goes through valFromOption: Arg_optional in ocaml_to_js_eff, the JSX key in Lam_compile_primitive, inlined functions, and @int and @as arguments, which switch on its result. get_default_undefined_from_optional returns an expression, so a test in the compiler needs the argument bound to a temporary: a statement for each such argument, through hold_arg for calls and separately for JSX keys, plus temporaries for the arguments to its left (see sequence_arguments). Bringing back toUndefined would undo Remove Primitive_option.toUndefined; use valFromOption for optional ffi args #8380 for one comparison. This way the generated code stays as it is, and code that 13.0.0-alpha.5 or later already compiled works once the runtime is updated.
• Does the extra test cost the other callers anything? They pass a value known to be Some: a match on a nested option (Pval_from_option), and the branch after x !== undefined of an @obj assignment or of an optional argument that can be read twice. For them x == null replaces x === null, still one comparison; in a micro-benchmark of 2×10^8 calls the difference is within noise for objects or numbers alone and under 0.3 ns a call for mixed inputs. The runtime grows by one character, and no other compiled module of the runtime, the stdlib or the tests changes.
• Could an argument that can be read twice now be compiled to valFromOption(x) alone, without x !== undefined ? … : undefined? It could, but that would change much output and call the runtime for None. The next commit does it only for a JS property read, which may run a getter.

✨ Evaluate optional @obj arguments in source order and read properties once

_{308b456 · mununki · Nov 09, 2026, 15:21 UTC · new}

Why

An @obj external builds its object as a literal and assigns an optional argument whose presence is not known after it, under if (tmp !== undefined). So that argument was evaluated after the arguments that follow it: make(~size=?size(), ~mode=mode(), ()) called mode() first, and a later argument with side effects could change what the optional argument read, such as a @get getter, a @val global, Dict.get, an array element, a mutable field or a ref. hold_obj_args now binds such an argument to a temporary after its statements, unless evaluating it after the literal gives the same values and effects.

As with the temporaries of #9217, sequence_arguments then evaluates the arguments to the left of the temporary before it, with evaluate_now. That bound an optional argument known to be present as a whole, so that it lost its presence and was assigned after the literal too, and it bound variables and fields of records although defining a temporary as such a value cannot change them. evaluate_now now evaluates the value in an Optional_block and keeps the block, and sequence_arguments leaves an argument that runs no code, a variable, a constant or a field of a record, in place when the statements after it only define variables as such values. A JS property read may run a getter, so it does not count.

An optional argument that is a JS property read, as from a @get external or a @val name with a dot, was read twice, to test it for None and to unwrap it, by a call, a JSX key and an @obj external alike. A getter ran twice, and one that returned a value and then undefined made the call throw a TypeError. Such an argument is now read once: a call and a JSX key give it to Primitive_option.valFromOption, which returns undefined for None since the previous commit, and an @obj external binds it. Js_analyzer.is_okay_to_duplicate ~property_reads:false tells these arguments apart.

JS modified (optional argument with side effects)

@obj external make: (~size: int=?, ~mode: string, unit) => {..} = ""

let o = (size, mode) => make(~size=?size(), ~mode=mode(), ())

js

 function o(size, mode) {
-  let tmp = {
+  let tmp = size();
+  let tmp$1 = {
     mode: mode()
   };
-  let tmp$1 = size();
-  if (tmp$1 !== undefined) {
-    tmp.size = tmp$1;
+  if (tmp !== undefined) {
+    tmp$1.size = tmp;
   }
-  return tmp;
+  return tmp$1;
 }

JS modified (optional argument that a later argument changes)

type source
@get external name: source => option<string> = "name"
@obj external makeName: (~name: string=?, ~mode: string, unit) => {..} = ""

let n = (s, rename) => makeName(~name=?name(s), ~mode=rename(s), ())

js

 function n(s, rename) {
-  let tmp = {
+  let tmp = s.name;
+  let tmp$1 = {
     mode: rename(s)
   };
-  if (s.name !== undefined) {
-    tmp.name = s.name;
+  if (tmp !== undefined) {
+    tmp$1.name = tmp;
   }
-  return tmp;
+  return tmp$1;
 }

JS modified (arguments before a bound optional argument)

type props = {color: string, width?: string}
@obj
external style: (~color: string=?, ~icon: string=?, ~width: string=?, ~height: string, unit) => {..} = ""

let a = (props, height) => style(~color=props.color, ~width=?props.width, ~height=height(), ())
let b = (icon, width, height) => style(~icon=icon(), ~width=?width(), ~height=height(), ())

js

 function a(props, height) {
-  let tmp = {
+  let tmp = props.width;
+  let tmp$1 = {
     color: props.color,
     height: height()
   };
-  if (props.width !== undefined) {
-    tmp.width = props.width;
+  if (tmp !== undefined) {
+    tmp$1.width = tmp;
   }
-  return tmp;
+  return tmp$1;
 }
 
 function b(icon, width, height) {
-  let tmp = {
-    icon: icon(),
+  let tmp = icon();
+  let tmp$1 = width();
+  let tmp$2 = {
+    icon: tmp,
     height: height()
   };
-  let tmp$1 = width();
   if (tmp$1 !== undefined) {
-    tmp.width = tmp$1;
+    tmp$2.width = tmp$1;
   }
-  return tmp;
+  return tmp$2;
 }

JS modified (optional argument known to be present before a temporary)

type item = {id: int}
@obj external mk: (~item: item=?, ~level: @int [#low | @as(10) #high], unit) => {..} = ""

let k = async (f, p) => mk(~item=f(), ~level=await p, ())

js

 async function k(f, p) {
   let tmp = f();
   let tmp$1 = await p;
-  let tmp$2 = {
+  return {
+    item: tmp,
     level: (() => {
       switch (tmp$1) {
         …
       }
     })()
   };
-  if (tmp !== undefined) {
-    tmp$2.item = Primitive_option.valFromOption(tmp);
-  }
-  return tmp$2;
 }

JS modified (optional property read)

type source
@get external name: source => option<string> = "name"
@val external greet: (~name: string=?, unit) => string = "greet"
@val external userName: option<string> = "window.userName"
@obj external makeName: (~name: string=?, ~mode: string, unit) => {..} = ""

let g = s => greet(~name=?name(s), ())
let u = () => greet(~name=?userName, ())
let m = (o: {"name": option<string>}) => makeName(~name=?o["name"], ~mode="dark", ())

js

 function g(s) {
-  return greet(s.name !== undefined ? Primitive_option.valFromOption(s.name) : undefined);
+  return greet(Primitive_option.valFromOption(s.name));
 }
 
 function u() {
-  return greet(window.userName !== undefined ? Primitive_option.valFromOption(window.userName) : undefined);
+  return greet(Primitive_option.valFromOption(window.userName));
 }
 
 function m(o) {
   let tmp = {
     mode: "dark"
   };
-  if (o.name !== undefined) {
-    tmp.name = o.name;
+  let tmp$1 = o.name;
+  if (tmp$1 !== undefined) {
+    tmp.name = tmp$1;
   }
   return tmp;
 }

JS modified (temporaries for optional arguments of an @obj external)

representation · Before: Lam_compile_external_obj.hold_obj_args

-~x=?f(), ~m=g()    ↦ after the literal
-~x=?p.x, ~m=g()    ↦ after the literal
-~x=?f(), ~m=p.m    ↦ after the literal
-~x=?f(), g()       ↦ after the literal
+~x=?f(), ~m=g()    ↦ temporary
+~x=?p.x, ~m=g()    ↦ temporary
+~x=?f(), ~m=p.m    ↦ temporary
+~x=?f(), g()       ↦ temporary
 ~x=?p.x, ~m=p.m    ↦ after the literal
 ~x=?f(), ~m="dark" ↦ after the literal
 ~x=?v, ~m=g()      ↦ after the literal
 ~x=?f(), ~y=?g()   ↦ after the literal
 note: p.x stands for a field, a global or a lookup, and v for an immutable variable

JS modified (arguments before a statement)

representation · Before: Lam_compile.sequence_arguments

-r.c, then let tmp = r.x  ↦ temporary
+r.c, then let tmp = r.x  ↦ in place
 r.c, then let tmp = s.x  ↦ temporary
 s.c, then let tmp = r.x  ↦ temporary
 f(), then let tmp = r.x  ↦ temporary
 r.c, then let tmp = g()  ↦ temporary
-~c=f(), then a statement ↦ temporary for the option
+~c=f(), then a statement ↦ temporary for f(), option in place
 note: r.c is a variable or a field of a record, s.x a JS property read, and ~c=f() an optional argument known to be present

JS modified (optional arguments read twice)

representation · Before: Js_analyzer.is_okay_to_duplicate · After: Js_analyzer.is_okay_to_duplicate ~property_reads:false

 x, M.x, globalName          ↦ test, then unwrap
 r.field                     ↦ test, then unwrap
-s.prop, o["prop"], window.x ↦ test, then unwrap
+s.prop, o["prop"], window.x ↦ unwrap once
 f(), a[i], c ? x : y        ↦ unwrap once
+note: r.field is a field of a record, s.prop a JS property read, such as a `@get` getter, and window.x a `@val` name with a dot

Review

• Which calls keep their output? An @obj call in which nothing that the literal evaluates after the optional argument can change it or be changed by it: constants, immutable variables and functions, reads without side effects when the optional argument has none either, as in make(~x=?p.x, ~m=p.m, ()), and other optional arguments whose presence is not known, which are assigned after it, in order. Variables and fields of records are still tested and unwrapped in place. No checked-in output changes besides the new tests.
• Which arguments get a temporary they do not need? A later argument is taken to change an optional one unless it is delay_safe or neither has side effects, and the JS IR does not say whether a field is mutable or a call pure. So make(~a=?props.a, ~b=Int.toString(n), ()) binds props.a, as a above binds props.width. A field of a JS object type, o["x"], cannot be told from a getter, so an @obj external binds it even when no later argument can change it, as m above, while a call no longer tests it inline.
• Why in hold_obj_args and not in hold_arg? Whether the argument has to be bound depends on the arguments after it, which hold_arg does not see, and only an object-creating external evaluates an argument after the ones that follow it. The binding does not depend on the type of the argument, so an optional polymorphic variant argument keeps its place whether or not hold_arg binds a later argument.
• Why is delay_safe passed to hold_obj_args? It is Lam_compile.delay_safe, which needs the variables the function assigns, and Lam_compile depends on this module.
• Why change evaluate_now and sequence_arguments here? Before this commit only the temporaries of Evaluate a polymorphic variant argument of an external once, in place #9217 for polymorphic variant arguments made sequence_arguments bind the arguments to their left, as in k above, where item lost its place in Object.keys. Binding optional arguments of any type would make that common: without the two changes, b above would assign icon after the literal as well, and a would bind props.color. Both functions are in lam_compile.ml, which Evaluate @int, @as, unit and @ignore arguments of externals in their place #9231 does not change.
• Why a flag on is_okay_to_duplicate and not another predicate? The two differ only in property reads, and its other callers, min and max of booleans and the object of a Pjs_object_get, keep the default. Js_ast_util.named_expression takes the same flag, which assemble_obj_args passes.
• Is a @val global still read twice? Yes, a plain one is, like a value of a JS module: only a property read is taken to run a getter, as a @get external is there to do. A @val name with a dot and a global with @scope compile to a property read and are read once.
• What still runs out of order? The side effects of a unit or @ignore argument are put before the literal or the call, so mk(~a=f(), g()) calls g() first, as before. A getter is taken to have no side effects, so make(~name=?name(s), ~count=count(s), ()) with two @get externals still reads count first.

✏️ Read @val globals and synchronous dispatch arguments of externals in place

_{8a914c9 · mununki · Nov 04, 2026, 20:50 UTC · edited}

Why

The review of #9217 asked for two changes. A @val global passed as an event-style argument was bound to a temporary because it may run a getter, while a @module value, which may be a getter as well, was read in place. Reading both in place is simpler and keeps the output globals had before #9217. And an @int argument, or a @string argument with @as, was bound whenever it had side effects, along with the arguments to its left, though the arrow function that switches on it is called where the argument is. Only an await, a syntax error in that function, has to be evaluated before it.

JS modified (@val global as an event-style argument)

type t
@val external ev: [#click(int => unit) | #keydown(int => unit)] = "ev"
@send external on: (t, @string [#click(int => unit) | #keydown(int => unit)]) => unit = "on"

let listen = t => on(t, ev)

js

 function listen(t) {
-  let tmp = ev;
-  t.on(tmp.NAME, tmp.VAL);
+  t.on(ev.NAME, ev.VAL);
 }

JS modified (@int argument with side effects between others)

type t
@send external set: (t, string, @int [#low | #high], string) => unit = "set"

let update = (t, g, f, h) => set(t, g(), f(), h())

js

 function update(t, g, f, h) {
-  let tmp = g();
-  let tmp$1 = f();
-  t.set(tmp, (() => {
-    switch (tmp$1) {
+  t.set(g(), (() => {
+    switch (f()) {
       case "low" :
         return 0;
       case "high" :
         return 1;
     }
   })(), h());
 }

JS modified (temporaries for @int and @as arguments)

representation · Lam_compile_external_call.hold_arg

-f()                         ↦ temporary
-pick(async () => await p)   ↦ temporary
-%raw(`"low"`)               ↦ temporary
+f()                         ↦ in place
+pick(async () => await p)   ↦ in place
+%raw(`"low"`)               ↦ in place
 await p                     ↦ temporary
 %raw(`await p`)             ↦ temporary
-~l=?f(), presence not known ↦ temporary
+~l=?f(), presence not known ↦ in place
 x, x.contents, b ? x : y    ↦ in place

Tests: polyvar_external_arg_test.res now expects a @val global passed as an event-style argument to be read for the tag and again for the payload. It adds tests for the order of the arguments around synchronous and awaited @string and @int arguments; an optional @int argument and an optional @obj field whose presence is not known, including None; and an async function and raw code in an @int or @as argument. Compiled with the previous commit, only the getter test fails.

Review

• What is still bound? An @int or @as argument with an await outside the functions it contains, as setLevel(t, await p), and the arguments to its left, so that they still run first. Raw JS code counts as awaiting unless it is a function or a literal, since the compiler cannot see what it does; that is the output Evaluate a polymorphic variant argument of an external once, in place #9217 gave it. An event-style argument with side effects is still bound, since it is read twice, and so is a JS property read: a @get getter, and a @val or @module value with @scope, which compiles to the same property read and cannot be told from it.
• Why is an optional argument whose presence is not known read in place too? Evaluate optional @obj arguments in source order and read properties once #9283, which this branch builds on, made the temporary unnecessary. lvlOpt(t, ~l=?f(), ()) compiles to Primitive_option.valFromOption(f()), which returns undefined for None since then instead of throwing a TypeError, and hold_obj_args binds such a field of an @obj external itself when evaluating it after the literal could change a value or the order of effects.
• Why is there no changelog fragment? Evaluate a polymorphic variant argument of an external once, in place #9217 was released in 13.0.0-alpha.7 with the entry "Fix an @string or @int argument of an external being evaluated twice, as in on(t, mk()), or breaking await", which still holds: an argument with side effects is still evaluated once, and an awaited one is still bound. This commit brings back the output that compilers before Evaluate a polymorphic variant argument of an external once, in place #9217 gave a @val global, and otherwise only drops temporaries.

✨ Pass @int and @as arguments of externals to the function that switches on them

_{c4e1528 · mununki · Nov 09, 2026, 15:32 UTC · new}

Why

The arrow function that switches on an @int argument, or a @string argument with @as, took no parameter and read the argument inside, so it captured the variables the argument reads. V8 keeps a captured variable alive for as long as any closure created in the same call is, so in the review of #9231 twenty callbacks kept 160 MB alive. 11.1 and 12.3 do this too, with pure arguments as well. Reading the argument inside the function also made an await in it a syntax error, which is why #9217 bound awaited arguments to temporaries. Calling the function with the argument evaluates it in the caller, where an await is legal, and captures nothing.

JS modified (@int argument)

type t
@send external setLevel: (t, @int [#low | @as(10) #high]) => unit = "setLevel"

let setup = (t, classify, big) => {
  setLevel(t, classify(big))
  () => t
}
let later = async (t, p) => setLevel(t, await p)

js

 function setup(t, classify, big) {
-  t.setLevel((() => {
-    switch (classify(big)) {
+  t.setLevel((x => {
+    switch (x) {
       case "low" :
         return 0;
       case "high" :
         return 10;
     }
-  })());
+  })(classify(big)));
   return () => t;
 }
 async function later(t, p) {
-  let tmp = await p;
-  t.setLevel((() => {
-    switch (tmp) {
+  t.setLevel((x => {
+    switch (x) {
       …
     }
-  })());
+  })(await p));
 }

JS modified (where an @int or @as argument is evaluated)

representation · Lam_compile_external_call.hold_arg

-setLevel(t, f())                ↦ read inside the function
-setLevel(t, await p)            ↦ temporary
-setLevel(t, %raw(`await p`))    ↦ temporary
-setLevelOpt(t, ~level=?f(), ()) ↦ read inside the function
+setLevel(t, f())                ↦ passed to the function
+setLevel(t, await p)            ↦ passed to the function
+setLevel(t, %raw(`await p`))    ↦ passed to the function
+setLevelOpt(t, ~level=?f(), ()) ↦ passed to the function

Tests: polyvar_external_arg_test.res checks with a WeakRef and gc() that a closure returned next to an @int argument does not keep the record the argument reads alive. Compiled with #9217 or the previous commit, that test fails. Its output, the outputs of the tests of #9283 and ImportJsValue.res.js in the gentype tests show the function called with the argument.

Review

• How does the test call gc()? v8.setFlagsFromString with --expose-gc gives new contexts a gc function, which vm.runInNewContext("gc") returns, as Jest's leak detection does. A WeakRef keeps its target until the end of the current task, so the test waits for a timer before it collects. It failed 20 runs out of 20 with the previous commit and passed 20 out of 20 with this one.
• Why does raw code that ends in a line comment, such as "light" // default mode, still break the module as an @int or @as argument? The line comment hides the parenthesis that closes the call, as it does for any argument: a string argument given that raw code compiles to t.setName("light" // default mode) with every compiler. Evaluate a polymorphic variant argument of an external once, in place #9217 put such an argument in a temporary on a line of its own, which hid the bug. The previous commit, like compilers before Evaluate a polymorphic variant argument of an external once, in place #9217, left it inside the switch, where it hid the rest of the line, as the review of Evaluate @int, @as, unit and @ignore arguments of externals in their place #9231 found. Ending raw code that ends in a line comment with a line break belongs in the printer, as a fix of its own.
• What goes? Js_of_lam_variant.awaits and eval_in_place, the temporaries for awaited arguments, and Js_exp_make.of_block, which had no other use. hold_arg now binds only an event-style argument, which cannot be optional, so it loses its arg_label parameter.

✨ Evaluate the effects of unit and @ignore arguments of externals in their place

_{f9f501b · mununki · Nov 09, 2026, 15:38 UTC · new}

Why

An external leaves a unit or @ignore argument out of the JS call, and translate_ffi and assemble_obj_args evaluated the effects of one before the whole call or object. So setX(t, f(), g()) called g() before f(), with 11.1 and 12.3 and with every kind of external. The temporaries of #9217 hid it only after a polymorphic variant argument with side effects, and the previous commits brought it back there (regressions 2 and 5 in the review of #9231). hold_arg now makes the effects a statement of the argument, so sequence_arguments evaluates the arguments to its left first, binding only those that the statement could change, as it does for any argument with statements. Nothing is hoisted any more, so the effect lists of ocaml_to_js_eff, assemble_args_* and assemble_obj_args go, with add_eff and Js_exp_make.fuse_to_seq, which had no other use, and ocaml_to_js_eff becomes ocaml_to_js.

JS modified (unit argument)

type t
@send external setX: (t, int, unit) => unit = "setX"

let update = (t, f, g) => setX(t, f(), g())

js

 function update(t, f, g) {
+  let tmp = f();
   g();
-  t.setX(f());
+  t.setX(tmp);
 }

JS modified (unit argument in a while condition)

type t
@send external ready: (t, unit) => bool = "ready"

let wait = (t, g) =>
  while ready(t, g()) {
    ()
  }

js

 function wait(t, g) {
-  while (g(), t.ready()) {
+  while (true) {
+    g();
+    if (!t.ready()) {
+      break;
+    }
   };
 }

JS modified (optional @ignore argument)

type t
@send external push: (t, int, ~extra: @ignore int=?, unit) => unit = "push"

let add = (t, g) => push(t, 1, ~extra=?g(), ())

js

 function add(t, g) {
-  Primitive_option.valFromOption(g());
+  g();
   t.push(1);
 }

JS modified (external whose arguments are all @ignore)

@val external count: (@ignore int, @ignore int) => int = "count"
@module external path: @ignore int => {..} = "node:path"

let counted = (f, g) => count(f(), g())
let loaded = g => path(g())

js

 function counted(f, g) {
-  return count(f(), g());
+  f();
+  g();
+  return count();
 }
 function loaded(g) {
+  g();
   return Nodepath;
 }

JS modified (where the effects of a unit or @ignore argument run)

representation · Before: Lam_compile_external_call.ocaml_to_js_eff · After: Lam_compile_external_call.hold_arg

-setX(t, f(), g()), the unit argument g()     ↦ before the whole call
-makeThen(~a=f(), g()), the unit argument g() ↦ before the object
-push(t, 1, ~extra=?g(), ())                  ↦ valFromOption(g()), before the call
-count(f(), g())                              ↦ passed to count
-path(g())                                    ↦ dropped
+setX(t, f(), g()), the unit argument g()     ↦ statement in its place
+makeThen(~a=f(), g()), the unit argument g() ↦ statement in its place
+push(t, 1, ~extra=?g(), ())                  ↦ g(), statement in its place
+count(f(), g())                              ↦ statements in their place, not passed
+path(g())                                    ↦ statement in its place

Tests: bs_ignore_effect_test.res checks the order of unit and @ignore arguments after others, with @send and @obj externals, a unit argument that changes an earlier one, an absent optional @ignore argument, and externals whose arguments are all @ignore. polyvar_external_arg_test.res checks the order after @string and @int arguments, labelled, optional and in an @obj literal, whose optional field known to be present stays in the literal, and a unit argument that changes an event-style @val global before it is read. Compiled with #9217 or the previous commit, these tests fail. bs_ignore_effect_test.res also checks that a @send external whose object is marked @ignore still calls the method on that object.

Review

• Why does some output get bigger? A unit argument with side effects now has a statement. A while condition with statements becomes a loop that breaks, and a call with one, as an element of an array or an argument of another call, evaluates the elements or arguments to its left first, binding those with side effects: [ready(t, g()), ready(t, h())] compiles to g(); let tmp = t.ready(); h(); return [tmp, t.ready()]; instead of [(g(), t.ready()), (h(), t.ready())]. Hoisting the effects as before is right only when every argument to their left can be read after them, which would keep the effect lists that this commit removes; the old order was wrong whenever that did not hold. Outside the new tests, three such sequences in checked-in outputs become statements, two in bs_ignore_effect_test.mjs and one in optional_external_arg_once_test.mjs of Evaluate optional @obj arguments in source order and read properties once #9283.
• Why does hold_args take the declaration of the external? translate_ffi calls the method of a @send external on its first argument whatever that argument's spec says, and the frontend accepts @ignore there, as in a @send external push of type (@ignore array<int>, int) => int. So hold_args leaves that argument as it is, where it would replace an @ignore one with undefined, and push(arr, 1) still compiles to arr.push(1).
• Why are the arguments of an external whose arguments are all @ignore no longer passed? Every other external leaves @ignore arguments out. translate_ffi took such an external for a JS value and called it with the arguments it got, which hold_arg now replaces with undefined. A JS value whose type is a function type it does not spell out, such as @val external v: u = "v" with type u = (int, int) => int, has no argument specs and still gets its arguments, as ExternalArity.res checks. A @module external of the module itself still is the module, but it no longer drops the effects of its arguments.
• What about an optional @ignore argument? Only its payload is evaluated, for its effects, so it no longer goes through Primitive_option.valFromOption, whose result was dropped. One without side effects, such as a variable, a field or a @get property, is not read at all, where it was tested for None and unwrapped, or given to valFromOption.

✨ Bind dotted @val event arguments of externals

_{28b9c60 · mununki · Nov 09, 2026, 15:40 UTC · new}

Why

The first commit reads a @val global in place for the tag and for the payload of an event-style argument, like a @module value. A @val with a dotted name such as "window.ev" compiles to a variable printed as window.ev, so it was read in place too, though it is the same property read as @val @scope("window") external ev: … = "ev", which keeps its temporary (a lower-severity regression in the review of #9231). Js_analyzer.is_okay_to_duplicate ~property_reads:false, which #9283 added for optional arguments, already tells such a name from a variable, so duplicable now defers to it, as the review suggested, and adds array elements and conditionals, and fields of them.

JS modified (@val with a dotted name as an event-style argument)

type t
@val external ev: [#click(int => unit) | #keydown(int => unit)] = "window.ev"
@send external on: (t, @string [#click(int => unit) | #keydown(int => unit)]) => unit = "on"

let listen = t => on(t, ev)

js

 function listen(t) {
-  t.on(window.ev.NAME, window.ev.VAL);
+  let tmp = window.ev;
+  t.on(tmp.NAME, tmp.VAL);
 }

JS modified (event-style arguments read twice)

representation · Js_of_lam_variant.eval_as_event_in_place

 x, r.ev, ev, M.ev                        ↦ read twice
-window.ev, as @val "window.ev"           ↦ read twice
+window.ev, as @val "window.ev"           ↦ temporary
 window.ev, as @val @scope("window") "ev" ↦ temporary
 s.ev, as @get                            ↦ temporary
 arr[i], c ? x : y, rs[i].ev              ↦ read twice
 f()                                      ↦ temporary
 note: ev is a @val global and M.ev a @module value

Tests: polyvar_external_arg_test.res checks that a @val with a dotted name is read once, like the same name with @scope. Compiled with the previous commit, it fails.

Review

• Why does duplicable add array elements and conditionals, and fields of them, to is_okay_to_duplicate? That predicate leaves them out, and the other callers that pass ~property_reads:false read such an argument once instead: an optional argument of a call goes to Primitive_option.valFromOption, and one of an @obj external to a temporary after the literal. The alternative for an event-style argument is a temporary before the call, a statement that makes sequence_arguments bind the arguments to its left that have side effects, so these reads without side effects stay in place, as Evaluate a polymorphic variant argument of an external once, in place #9217 left them. A field of a variable is already in is_okay_to_duplicate; duplicable takes fields itself only to reach the array element or conditional under them, as in rs[i].ev. No other output changes.
• Why is a @val global without a dot still read twice, though a getter may then give the tag of one event and the payload of another? The review of Evaluate a polymorphic variant argument of an external once, in place #9217 asked to treat a @val global like a @module value, which Babel or TypeScript may compile to a getter as well, and to read both in place, as compilers before Evaluate a polymorphic variant argument of an external once, in place #9217 did. A later argument that changes the global is no longer a problem since the previous commit: its effects are statements, before which the global is bound.
• Why no changelog fragment? A dotted @val was bound in 13.0.0-alpha.7 as well, so this only takes back what the first commit changed for it.

[codecov]

Codecov Report

❌ Patch coverage is 95.90164% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.93%. Comparing base (e941d27) to head (28b9c60).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
compiler/core/lam_compile_external_obj.ml	92.30%	3 Missing ⚠️
compiler/core/js_analyzer.ml	85.71%	1 Missing ⚠️
compiler/core/lam_compile_external_call.ml	98.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9231      +/-   ##
==========================================
+ Coverage   81.92%   81.93%   +0.01%     
==========================================
  Files         456      456              
  Lines       65469    65437      -32     
==========================================
- Hits        53635    53618      -17     
+ Misses      11834    11819      -15     

Files with missing lines	Coverage Δ
compiler/core/js_ast_util.ml	100.00% <100.00%> (ø)
compiler/core/js_exp_make.ml	66.32% <ø> (-0.01%)	⬇️
compiler/core/js_of_lam_option.ml	96.66% <100.00%> (ø)
compiler/core/js_of_lam_variant.ml	86.84% <100.00%> (+1.42%)	⬆️
compiler/core/lam_compile.ml	90.40% <100.00%> (+0.47%)	⬆️
compiler/core/js_analyzer.ml	80.76% <85.71%> (+0.65%)	⬆️
compiler/core/lam_compile_external_call.ml	97.14% <98.00%> (+4.16%)	⬆️
compiler/core/lam_compile_external_obj.ml	95.58% <92.30%> (+0.13%)	⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pkg-pr-new]

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@9231

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@9231

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@9231

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@9231

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@9231

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@9231

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@9231

commit: 28b9c60

[github-actions]

Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-9231

[mununki]

@codex review

[chatgpt-codex-connector]

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-09T17:31:13.999124Z	28b9c60	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 04bd2b4fe0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cknitt]

🤖 This review was written by Claude (Claude Code), not by a human reviewer. I built bsc for this PR and for its base, #9217, compiled small programs with both, and ran the behavioural cases in node. Each finding below was checked separately; two are marked plausible (not confirmed).

Summary

Five confirmed regressions against #9217: wrong values, effects running out of order, a crash, and a memory leak. There are also three lower-severity regressions, one bug that already exists (#9226), and a few cleanups. CI is green, the checked-in .mjs matches what this PR's compiler produces, and the PR's own claim holds: compiled with #9217, only the getter test fails.

Common cause. Binding a side-effecting dispatch argument to a temporary in #9217 did more than its stated job: it also kept evaluation in source order. It made sequence_arguments bind the arguments to its left, and it ran before effects that the external translation hoists in front of the call. Removing the temporary brings back the older orders. So "correctness only needs this for await" doesn't hold in the cases below.

Regressions against #9217

1. @obj optional fields are read after a later side-effecting field (lam_compile_external_call.ml:139)

In an @obj external, an optional field whose presence isn't known and that reads mutable state is now evaluated after a later side-effecting @int/@as field. That's because assemble_obj_args assigns those fields after the object literal.

makeModeFirst(~mode=?r.contents, ~level=bump(), ()), where bump sets r.contents = Some(#dark):

• Evaluate a polymorphic variant argument of an external once, in place #9217: let tmp = r.contents; let tmp$1 = bump(); … gives {level: 10, mode: "light-mode"}
• PR: let tmp = {level: (() => { switch (bump()) … })()}; if (r.contents !== undefined) { tmp.mode = … } gives mode: "dark-mode". If bump sets None, mode is dropped.

This also happens with a @get getter, a @val global, Dict.get, a[0], and a plain ~name: string=?.
Fix: in hold_obj_args, bind every presence-unknown Obj_optional argument that isn't delay_safe, whatever its type.

2. Effects of unit/@ignore arguments now run before an earlier dispatch argument (js_of_lam_variant.ml:60)

ocaml_to_js_eff puts these effects in eff, and add_eff emits them before the whole call. So a dispatch argument that is no longer bound runs after effects that come later in the source.

@send external setLevelU: (t, @int [#low | #high], unit) => unit = "setLevel" with setLevelU(t, f(), g()):

• Evaluate a polymorphic variant argument of an external once, in place #9217: let tmp = f(); g(); t.setLevel(…tmp…)
• PR: g(); t.setLevel((() => { switch (f()) … })()), so g runs before f.

The same happens with @ignore, @obj makeOptions(~level=f(), g()), @new/@module/@variadic, and labelled or optional dispatch arguments. When the later effect changes what the argument reads, the value passed changes too. The new ordering test only has plain neighbours.
Fix: emit these effects as statements in their own position instead of hoisting them.

3. Memory retention from the zero-parameter arrow (js_of_lam_variant.ml:60)

(() => { switch (arg) … })() captures the free variables of arg. V8 keeps them in the function's shared context, so any longer-lived closure created in the same function keeps them alive.

let setup = (t, big) => { setLevel(t, classify(big)); () => t }, 20 callbacks retained, each call given an 8 MB array: #9217's output retains 0 MB, this PR's retains 160 MB (node / V8 12.4, at every tier). Building it as (x => { switch (x) … })(classify(big)) retains 0 MB.

4. An optional property-read argument runs its getter twice and can throw (lam_compile_external_call.ml:139)

setLevelOptThen(t, ~level=?currentLevel(s), g(), ()), where currentLevel is a @get and g() is a later @int argument:

• Evaluate a polymorphic variant argument of an external once, in place #9217: let tmp = s.currentLevel; let tmp$1 = g(); …: one getter call
• PR: switch (s.currentLevel !== undefined ? Primitive_option.valFromOption(s.currentLevel) : undefined): two calls. With a getter that returns "high" and then undefined, this throws TypeError: Cannot read properties of undefined (reading 'BS_PRIVATE_NESTED_SOME_NONE').

This contradicts the PR's own rule that a @get getter must not be read twice.

5. A later argument can change an event-style @val global before it is read (js_of_lam_variant.ml:73)

Treating every Var as duplicable drops #9217's temporary, and that temporary was what read the global before the hoisted effects ran.

onU(t, currentEvent, setCurrent(#keydown(2))), with the global holding #click(1):

• Evaluate a polymorphic variant argument of an external once, in place #9217: let tmp = polyvarCurrentEvent; setCurrent(…); t.onU(tmp.NAME, tmp.VAL) passes click 1
• PR: setCurrent({NAME: "keydown", VAL: 2}); t.onU(polyvarCurrentEvent.NAME, polyvarCurrentEvent.VAL) passes keydown 2

Lower-severity regressions

• Dotted @val names (js_of_lam_variant.ml:73): @val external ev: … = "window.ev" compiles to a Var that prints as window.ev, so it is read twice: t.on(window.ev.NAME, window.ev.VAL). The identical @scope("window") … = "ev" keeps its temporary. Dotted names like "Math.PI" and "window.location.search" are common. Fix: | Var (Id id) when Ext_ident.is_js id && String.contains id.name '.' -> false.
• %raw classified as Js_literal (js_of_lam_variant.ml:43): classify_function has no end-of-input check, so %raw("light" // default mode) counts as a literal and is printed as switch ("light" // default mode) {. The module then doesn't parse (SyntaxError: Unexpected token 'case'). This is rare, and compilers before Evaluate a polymorphic variant argument of an external once, in place #9217 emitted the same code. Fix: drop Js_literal from the exemption.
• Known-present optional @int/@as arguments with a side-effecting payload (lam_compile_external_call.ml:148): when a later argument has statements, evaluate_now binds the whole Optional_block and its presence is lost. With mk(~mode=f(), ~level=await p, ()), Object.keys order becomes [level, mode], and calls get an extra Primitive_option require. Values stay correct. Fix: add an Optional_block (a, id) case to evaluate_now that evaluates only a.
• (plausible) Mismatched tag and payload: a @val getter that returns a different value on each read can pair one event's tag with another's payload, so a handler of the wrong type is registered (e.g. s => s.toUpperCase() under "click").

Bug that already exists (#9226)

The new comment (lines 125–129) gives valFromOption failing on undefined as the reason to bind presence-unknown optional arguments, but the binding is only done for side effects. Pure lookups still crash on None: setLevelOpt(t, ~level=?arr->Array.get(i), ()) or ~level=?(c ? x : y) emit switch (Primitive_option.valFromOption(arr[i])), which throws. Base output is identical.

Cleanups

• Simpler shape: the awaits/eval_in_place machinery, the raw-code case and the await temporaries are only needed because eval and eval_as_int build a zero-parameter arrow. Building (x => { switch (x) … })(arg) with E.call (E.ocaml_fun ~one_unit_arg:false [x] […]) [arg], as wrap_then_path already does, evaluates the argument in the caller, where await (and a future yield) is legal. When all 31 dispatch functions in polyvar_external_arg_test.mjs are rewritten this way, mocha still passes 14/14 and retention drops to 0 MB. This also fixes regression 3.
• Reuse: duplicable is a second "safe to read twice" predicate next to Js_analyzer.is_okay_to_duplicate, and the two disagree. duplicable adds Undefined, Array_index and Cond, and drops JS property reads. A shared predicate would keep event and option arguments consistent.
• Simplification (lam_compile_external_call.ml:138): when presence_known is false, eval_in_place e is implied by no_side_effect_expression e. So the arm reduces to if presence_known then not (eval_in_place e) else not (no_side_effect_expression e). The two branches at 152–155 differ only by a flag that can be derived from arg_label.
• (plausible) Docs: the hold_arg doc in the .mli doesn't mention that a presence-unknown optional argument is bound when it has side effects.
• Minor: the awaits iterator record and closure are rebuilt on every call. They could be built once at module level, like Js_analyzer.no_side_effect_obj.

Suggested direction

1. Use the one-parameter form (x => switch …)(arg). This fixes the retention, removes the need for awaits and the await temporaries, and evaluates the argument in the caller.
2. Bind every presence-unknown optional argument that isn't delay_safe. This fixes the @obj order and the double getter read.
3. Emit unit/@ignore effects in their own position.
4. Don't treat dotted @val names as duplicable.

[mununki]

Thanks for the review. Regressions 1 and 4 are fixed in #9283, and the #9226 section in #9288. This PR is now rebased onto both and shows their commits until they are merged. In the review's order:

Regressions against #9217

1. @obj optional field read after a later field: fixed in Evaluate optional @obj arguments in source order and read properties once #9283.
2. unit/@ignore effects hoisted before the call: fixed in f9f501b. They are now a statement in the argument's place, for every kind of external, and the effect lists and add_eff are gone.
3. Retention: fixed in c4e1528 with (x => { switch (x) … })(arg), as you suggested. A test checks it with a WeakRef and gc().
4. Optional property read twice: fixed in Evaluate optional @obj arguments in source order and read properties once #9283.
5. Later argument changing an event-style @val global: fixed by f9f501b, which binds the global before the later argument's statement. Tested.

Lower-severity regressions

• Dotted @val names: 28b9c60 binds them like the @scope form. Tested.
• %raw classified as Js_literal: the exemption is gone (c4e1528). Raw code that ends in a line comment still breaks, as with 12.3, because the comment hides the closing ) of the call. That happens for any argument (t.setName("light" // default mode)), so I'll fix it in the printer separately.
• Known-present optional argument losing its presence: fixed in Evaluate optional @obj arguments in source order and read properties once #9283. There, evaluate_now evaluates only the value inside an Optional_block.
• (plausible) Mismatched tag and payload: kept for @val names without a dot, because the review of Evaluate a polymorphic variant argument of an external once, in place #9217 asked to read globals in place like @module values. The case where a later argument changes the global is item 5.

#9226: fixed in #9288.

Cleanups

• Simpler shape: done in c4e1528. awaits, eval_in_place, the raw-code case, the await temporaries and E.of_block are gone.
• Reuse: done in 28b9c60. duplicable now uses Js_analyzer.is_okay_to_duplicate ~property_reads:false from Evaluate optional @obj arguments in source order and read properties once #9283. It only adds array elements, conditionals and fields of them, which an event-style argument may read twice instead of binding them.
• Simplification: hold_arg now only binds event-style arguments and turns unit/@ignore effects into statements. presence_known and its arg_label parameter are gone.
• Docs: the .mli now says what hold_arg binds and where unit/@ignore effects go.
• awaits iterator: no longer applies.

f9f501b also calls an external whose arguments are all @ignore without them, like other externals. A @module external of the module itself now keeps their effects. The PR now has changelog fragments, so changelog:skip will be removed.

[github-actions]

✅ Compiler benchmarks

Allocated words

In total: 683,552,907 → 683,440,958 (-0.02%).

Benchmark	Base	Head	Change
packages/@rescript/runtime/Primitive_option.res (inputs changed)	315,661	314,597	-0.34%

(inputs changed): this pull request changes runtime or Belt sources, so these benchmarks compile different code than on the base, and their changes don't count as regressions.

Instructions

In total: 9,664,672,601 → 9,664,735,979 (±0.00%).

None of the 33 benchmarks changed by 0.1% or more.

Output size

In total: 25,569,598 → 25,569,243 (±0.00%).

None of the 13 benchmarks changed by 0.1% or more.

Pull request 28b9c60 compared with base b82d944 in this run.

[mununki]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 28b9c605b6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".