feat(go): add generated SDK and standalone runtime release - #4067
Conversation
…e-primitives # Conflicts: # .github/workflows/release-baml-language.yml # scripts/baml-language-version
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
|
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:
📝 WalkthroughWalkthroughThis PR adds Go SDK generation, a native Go runtime and value codecs, extensive Go SDK fixtures and harness coverage, cross-platform CI support, and release automation that mirrors, publishes, and smoke-tests the Go module. ChangesGo SDK generation
Go runtime and codecs
SDK test harness
CI and releases
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as baml-cli
participant Generator as sdkgen_go
participant SDK as Generated Go SDK
participant Runtime as baml_go runtime
participant Native as bridge_cffi
CLI->>Generator: generate Go source with bytecode
Generator-->>SDK: write functions, types, codecs, bootstrap
SDK->>Runtime: call generated function
Runtime->>Native: load runtime and invoke function
Native-->>Runtime: callback result envelope
Runtime-->>SDK: decoded Go value
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Binary size checks passed✅ 7 passed
Generated by |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build2-bridge-cffi.reusable.yaml (1)
81-106: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict the matrix job’s token to read-only access.
This checkout-only job currently inherits the caller’s default permissions. Declare the minimum permission explicitly.
Proposed fix
matrix: name: Compute target matrix runs-on: ubuntu-latest + permissions: + contents: read🤖 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 @.github/workflows/build2-bridge-cffi.reusable.yaml around lines 81 - 106, Restrict the matrix job’s GitHub token to read-only access by adding an explicit permissions declaration to the matrix job containing only contents: read. Update the job identified by the matrix name “Compute target matrix”; leave its checkout and matrix-generation steps unchanged.Source: Linters/SAST tools
🧹 Nitpick comments (1)
.github/workflows/cargo-tests.reusable.yaml (1)
748-753: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrevent confusing workspace builds when WASM package discovery fails.
If
cargo metadataorjqfails,readarraysilently swallows the error and produces an empty array. This causescargo buildto execute without any-pflags, inadvertently building the entire workspace for WASM and failing with extremely misleading compilation errors.Adding a check for an empty array improves CI debuggability by pinpointing the failure source immediately.
💡 Proposed check for empty array
readarray -t wasm_packages < <(cargo metadata --no-deps --format-version 1 | jq -r '.packages[] | select(.source==null) | select(if .metadata.ci.wasm_support == null then true else .metadata.ci.wasm_support end) | .name') package_args=() for package in "${wasm_packages[@]}"; do package_args+=(-p "$package") done + if [[ ${`#package_args`[@]} -eq 0 ]]; then + echo "Error: No WASM packages found. Did 'cargo metadata' or 'jq' fail?" >&2 + exit 1 + fi cargo build "${package_args[@]}" --target wasm32-unknown-unknown --no-default-features --release --timings🤖 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 @.github/workflows/cargo-tests.reusable.yaml around lines 748 - 753, Validate the wasm_packages result immediately after the cargo metadata/jq readarray in the WASM build step, and fail with a clear error before constructing or invoking cargo build when the array is empty. Keep the existing package_args construction and targeted build behavior unchanged for successfully discovered packages.
🤖 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 @.github/workflows/build2-bridge-cffi.reusable.yaml:
- Around line 81-106: Restrict the matrix job’s GitHub token to read-only access
by adding an explicit permissions declaration to the matrix job containing only
contents: read. Update the job identified by the matrix name “Compute target
matrix”; leave its checkout and matrix-generation steps unchanged.
---
Nitpick comments:
In @.github/workflows/cargo-tests.reusable.yaml:
- Around line 748-753: Validate the wasm_packages result immediately after the
cargo metadata/jq readarray in the WASM build step, and fail with a clear error
before constructing or invoking cargo build when the array is empty. Keep the
existing package_args construction and targeted build behavior unchanged for
successfully discovered packages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c9279f9f-0657-4db1-afd7-51620a77bc1c
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.github/workflows/build2-bridge-cffi.reusable.yaml.github/workflows/cargo-tests.reusable.yaml.github/workflows/release-baml-language.ymlbaml_language/.config/nextest.tomlbaml_language/Cargo.tomlbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/generate.rsbaml_language/crates/baml_codegen_types/src/generator_fields.rsbaml_language/sdk_tests/harness_runner/src/lib.rsbaml_language/sdk_tests/harness_setup/Cargo.tomlbaml_language/sdk_tests/harness_setup/src/lib.rsscripts/baml-language-versionscripts/baml-release-manifests
🚧 Files skipped from review as they are similar to previous changes (9)
- baml_language/crates/baml_codegen_types/src/generator_fields.rs
- baml_language/sdk_tests/harness_setup/src/lib.rs
- baml_language/Cargo.toml
- baml_language/.config/nextest.toml
- scripts/baml-release-manifests
- baml_language/sdk_tests/harness_runner/src/lib.rs
- baml_language/sdk_tests/harness_setup/Cargo.toml
- baml_language/crates/baml_cli/src/generate.rs
- baml_language/crates/baml_cli/Cargo.toml
|
@coderabbitai review |
✅ Action performedReview finished.
|
…e-primitives # Conflicts: # .github/workflows/release-baml-language.yml # baml_language/.config/nextest.toml # baml_language/Cargo.lock # baml_language/crates/baml_cli/Cargo.toml # baml_language/crates/baml_cli/src/generate.rs # baml_language/sdk_tests/harness_setup/src/lib.rs
Superseded by fixes in 26d4e22 and the subsequent successful CodeRabbit review. All actionable threads are resolved; two withdrawn findings were verified against the current ABI/package model.
The pydantic2 SDK generator emitted BAML identifiers verbatim, so an enum member `None`, a class field `pass`, or a keyword class/enum/alias name produced Python that fails to import with a SyntaxError (issue BoundaryML#4059). Only free-function and method names were escaped previously. Generalize the existing `escape_python_keyword` helper to every Python-visible identifier site, mirroring the Go generator (PR BoundaryML#4067): keep the raw BAML name on the wire, escape only the language-facing spelling. - enum members: `None` renders `None_ = "None"` (value stays the wire identity, so decode by value keeps working with no alias needed). - class fields: `pass` renders `pass_: T = pydantic.Field(alias="pass")` and the class gains `populate_by_name=True` so it validates from both the escaped attribute and the raw JSON/wire key. Classes with no escaped field render byte-identically to before. - class / enum / type-alias names, plus TypeVars, escape at the definition site AND at every cross-reference (translate_ty render_name_ref, Ty::TypeVar) so references stay in lockstep. - collisions inside one scope resolve deterministically: a class or enum declaring both `pass` and `pass_` yields `pass__` and `pass_`. - a debug-assert render-boundary guard panics on any identifier that reaches the emitter still a hard keyword (mirrors Go's assertion). Out of scope (documented follow-ups): function/method parameter renaming (wire-coupled, needs bridge alias plumbing) and routing.rs module-segment generalization. Adds exhaustive 35-keyword unit tests for enum members and class fields, name/typevar escape + cross-reference agreement, collision resolution, soft-keyword controls, and a byte-identical no-keyword control. Keeps all 117 existing tests green (128 total). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
bridge_cffiGo runtime with verified release-manifest artifact resolutionbaml-cli generateand port the initial compile/runtime SDK fixturesgithub.com/boundaryml/baml-goas a read-only module mirror with matching Canary/nightlyv<language-version>tagsRelease behavior
The shared BAML release plan stamps the Go module's required runtime version. For every future Canary or nightly release, the release workflow:
BoundaryML/baml-gov<canonical-version>tag atomically withmainbaml_bridge_goin the public version manifest only after the tag succeedsreplacedirective or Rust toolchainBAML_GO_DEPLOY_KEYand the write deploy key onBoundaryML/baml-goare configured. The already-published BAML0.15.0release is intentionally not backfilled with a new Go tag; the first Go release will be the next nightly, followed by the next bumped Canary.Local validation
cargo test -p sdkgen_go(35 passed)cargo nextest run -p sdk_test_go(6 passed)go test ./...go vet ./...scripts/baml-language-version checkactionlint .github/workflows/release-baml-language.ymlSummary by CodeRabbit