feat(sdk): wire optional args through python and node bridges#3685
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds required-positional + optional-kwargs support and an exported Python Unset sentinel; updates Python and TypeScript emitters and runtimes to split required vs optional params, makes protoc writer usage safer, and adds fixtures plus static/runtime tests covering the new calling semantics. ChangesOptional arguments feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
sxlijin
marked this pull request as ready for review
June 4, 2026 07:58
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21e711540e
ℹ️ 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".
sxlijin
force-pushed
the
codex/optional-host-arg-coverage
branch
from
June 4, 2026 08:03
21e7115 to
8bf78b0
Compare
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: 4
🤖 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/sdks/nodejs/sdkgen_typescript_node/src/emit/mod.rs`:
- Line 166: The generated TypeScript function signature emitted by the closure
taking (name, fqn, mode, params, arg_tys, arg_defaults, return_ty) in
emit/mod.rs is missing the proper parameter-name/type list and the explicit cast
for the function type; update that emitter to produce an export like `export
const <name> = defineFunction("<fqn>", "<mode>", [<param names>]) as
(<paramName1>: <type1>, ...) => <return_ty>;` by assembling params with names
and their corresponding types from params and arg_tys, using return_ty for the
RHS cast, and keeping fqn and mode in the defineFunction call so the resulting
line exactly matches the expected form (e.g., `export const extract =
defineFunction("user.lorem.extract", "sync", ["text"]) as (text: string) =>
number;`).
In `@baml_language/sdks/nodejs/sdkgen_typescript_node/src/leaf.rs`:
- Line 826: The func_sym being constructed currently sets arg_defaults:
Vec::new(), which makes all parameters appear optional; instead initialize
arg_defaults with one None entry per parameter to match arity so fn_type_sig
computes required-positional correctly—locate the construction of func_sym (the
struct field arg_defaults in the function that builds func_sym) and replace
Vec::new() with a Vec populated with None for each parameter (e.g., iterating
over the parameters or using params.len()) so the length of arg_defaults equals
the number of parameters.
In `@baml_language/sdks/python/src/baml_core/__init__.py`:
- Around line 162-170: The Unset sentinel is instantiable so callers can create
non-canonical Unset() objects that `_build_kwargs` (which only checks `v is
UNSET`) will not treat as omitted; fix by making Unset a true singleton
(implement `def __new__(cls): if getattr(cls, "_instance", None) is None:
cls._instance = super().__new__(cls); return cls._instance`) so every `Unset()`
returns the same `UNSET` instance, and keep `UNSET = Unset()`; alternatively (or
as a simpler change) update `_build_kwargs` to treat any instance of Unset as
omitted by checking `isinstance(v, Unset)` instead of identity.
🪄 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: 918f31c3-5abf-4a27-930d-ee0af8aec0c6
⛔ Files ignored due to path filters (4)
baml_language/sdks/nodejs/bridge_nodejs/dist/define_function.d.tsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/define_function.d.ts.mapis excluded by!**/dist/**,!**/*.mapbaml_language/sdks/nodejs/bridge_nodejs/dist/define_function.jsis excluded by!**/dist/**baml_language/sdks/nodejs/bridge_nodejs/dist/define_function.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (17)
baml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/optional_args_static.pybaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_main.pybaml_language/sdk_tests/crates/python_pydantic2/function_calls/customizable/test_optional_args.pybaml_language/sdk_tests/crates/typescript_node/function_calls/customizable/main.test.tsbaml_language/sdk_tests/crates/typescript_node/function_calls/customizable/optional_args.static.tsbaml_language/sdk_tests/crates/typescript_node/function_calls/customizable/optional_args.test.tsbaml_language/sdk_tests/fixtures/function_calls/baml_src/main.bamlbaml_language/sdk_tests/harness_setup/src/python_pydantic2.rsbaml_language/sdk_tests/harness_setup/src/templates/pyproject.tomlbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/define_function.tsbaml_language/sdks/nodejs/bridge_nodejs/typescript_src/proto/baml_cffi.jsbaml_language/sdks/nodejs/sdkgen_typescript_node/src/emit/function.rsbaml_language/sdks/nodejs/sdkgen_typescript_node/src/emit/method.rsbaml_language/sdks/nodejs/sdkgen_typescript_node/src/emit/mod.rsbaml_language/sdks/nodejs/sdkgen_typescript_node/src/leaf.rsbaml_language/sdks/python/rust/sdkgen_python_pydantic2/src/leaf.rsbaml_language/sdks/python/src/baml_core/__init__.py
sxlijin
force-pushed
the
codex/optional-host-arg-coverage
branch
from
June 4, 2026 17:05
8bf78b0 to
8a9d2c0
Compare
sxlijin
force-pushed
the
codex/optional-host-arg-coverage
branch
from
June 4, 2026 18:15
8a9d2c0 to
c28c1a4
Compare
sxlijin
force-pushed
the
codex/optional-host-arg-coverage
branch
from
June 4, 2026 18:24
c28c1a4 to
56b719b
Compare
sxlijin
force-pushed
the
codex/optional-host-arg-coverage
branch
from
June 4, 2026 18:35
56b719b to
3467ff8
Compare
sxlijin
force-pushed
the
codex/optional-host-arg-coverage
branch
from
June 4, 2026 18:46
3467ff8 to
fa03f0b
Compare
sxlijin
force-pushed
the
codex/optional-host-arg-coverage
branch
from
June 4, 2026 19:09
fa03f0b to
36bc063
Compare
sxlijin
force-pushed
the
codex/optional-host-arg-coverage
branch
from
June 4, 2026 19:37
36bc063 to
b609cab
Compare
sxlijin
changed the title
sxlijin
enabled auto-merge
June 4, 2026 19:43
This was referenced Jun 18, 2026
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.
Summary
Implements the optional/defaulted host argument coverage spec for Python and TypeScript SDKs.
Validation
Summary by CodeRabbit
New Features
Tests
Bug Fixes
Chores