Skip to content

Dhilan/swift bridge 1 - #4108

Closed
sxlijin wants to merge 16 commits into
canaryfrom
dhilan/swift-bridge-1
Closed

sxlijin wants to merge 16 commits into
canaryfrom
dhilan/swift-bridge-1

Conversation

@sxlijin

@sxlijin sxlijin commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Added Swift as a first-class code generation output target in the CLI, including end-to-end Swift SDK test support.
    • Introduced the Swift runtime bridge with streaming APIs, cancellation handling, media helpers, host callables, and structured BamlError/BamlPanic.
  • Documentation
    • Added/updated Swift bridge capability and union representation documentation.
  • Tests
    • Expanded Swift SDK coverage with new fixtures and replay-based streaming/E2E, ABI/layout smoke tests, and extensive round-trip suites.
  • Chores
    • Updated macOS Swift test runner setup and Swift build/packaging artifacts (xcframework + SwiftPM manifest).

ATX24 and others added 15 commits July 15, 2026 09:52
Stand up every layer of the Swift SDK skeleton end to end, with no real
capability yet — the working pipe Phase 1 pumps primitives through:

- bridge_swift: staticlib re-exporting bridge_cffi's C ABI; all 12
  symbols verified in the archive. New release-bridge-swift profile
  (panic=unwind) for eventual release builds.
- sdks/swift SwiftPM package (BamlBridge): cbindgen-generated C header
  + modulemap (with HostDispatchFn/HostReleaseFn typedefs injected —
  they live in dep crates cbindgen doesn't parse), checked-in
  swift-protobuf clients for the 4 wire protos, XCFramework build
  script (--host-only dev / --all release), and an FFI smoke test
  proving SwiftPM -> XCFramework -> Rust linking via version().
- sdkgen_swift: generator stub registered as output_type = "swift"
  (OutputType::Swift + baml_cli dispatch arm); emits the base64
  bytecode payload (_InlinedBaml.swift) and a stub Baml namespace.
- sdk_test_swift: full harness wiring per DEVELOPMENT.md — build.rs
  codegen soft-failing into build_diagnostics, per-fixture SwiftPM
  packages (path dep on sdks/swift; note SwiftPM identifies path deps
  by directory name), swift build/test scaffold with a one-line
  ENFORCED_FIXTURES dial (empty for now), macOS-only nextest
  setup-script binding that assembles the host XCFramework.

Co-Authored-By: Claude Fable 5 <[email protected]>
Pump real values through the Phase 0 pipe: free functions over the
primitive subset, sync + async, with the type_shapes and function_calls
fixtures enforced under `cargo nextest run -p sdk_test_swift` (4/4
green; failure propagation verified with a deliberate assert-break).

- BamlBridge runtime: idempotent bytecode init + completion-callback
  plumbing (locked callback-id -> waiter table; bytes copied off the
  FFI thread). Async via CheckedContinuation with Task cancellation
  forwarding to cancel_function_call; sync via semaphore park — safe
  because completions always arrive on an engine Tokio thread
  (call_function fully decodes its buffer before returning). Two IDs
  per call by design: u64 proto call_id (engine handle, cancellation)
  vs u32 callback correlation key (CallbackFn only carries u32).
- Encode.swift: BamlEncodable conformances (Int/Double/Bool/String/
  Data/BamlNull/Optional/Array/Dictionary<String,_>); empty containers
  still set their oneof (SetInParent analog — Python's "Bug A");
  BamlNull is the spelling for BAML's standalone null type.
- Decode.swift: BamlOutboundResult envelope (ok/error/panic; exit
  panic -> flush_events + exit), union/literal unwrapping, wire-driven
  BamlDecodable conformances with named typeMismatch errors.
- sdkgen_swift: translate_ty for the primitive subset (null-union ->
  T? collapse, literal -> base type, string-keyed maps), namespace
  routing into caseless-enum trees, real fn/fn_async body emission;
  functions with unsupported signature types are skipped so generated
  packages always compile. Bytecode payload emitted as ONE multiline
  string literal — a `"…" + "…"` chunk chain sent swiftc's expression
  type-checker super-linear (55+ min for multi-MB payloads; now ~35 s).
- Tests ported 1:1 from python_pydantic2 (TestPrimitives/Void/Lists/
  Maps/Main; per-file notes name the Python cases waiting on later
  phases). ENFORCED_FIXTURES dial flipped to type_shapes +
  function_calls. Scaffold now emits ONE test per fixture: a sibling
  `swift build` test contended for the SwiftPM .build lock and a
  killed run wedged every later run behind an orphaned lock holder.

Co-Authored-By: Claude Fable 5 <[email protected]>
…args)

Widen the bridge from primitives to the named-type core, with three
fixtures enforced (type_shapes, function_calls, docstrings_etc) and the
gate green first try (5/5, ~36s).

- BamlBridge: BamlOptional<T> (.unset omits the kwarg so the engine
  evaluates the BAML default; nil = explicit null — Python's UNSET
  design), BamlIndirect CoW property wrapper for recursive struct
  fields, baml_class/baml_enum inbound builders, classFields()/
  enumVariant() outbound helpers (missing field decodes as null).
- sdkgen_swift: classes -> Equatable/Sendable structs with memberwise
  inits + generated BamlEncodable/BamlDecodable conformances (FQN baked
  in; decode is wire-tolerant like Python); enums -> String-raw enums;
  non-recursive aliases -> typealias; a supported-types fixpoint drops
  classes with unsupported fields and everything referencing them (the
  generated package always compiles); a reachability pass over direct
  non-List/Map class references marks cycle-forming fields
  @BamlIndirect (self-recursion, mutual A/B, 3-class SCCs); optional
  args emit as BamlOptional<T> = .unset with conditional appends.
  Namespace trees now hold type decls too, with ancestor enums
  synthesized for deep paths (Baml.a.b.Thing).
- Tests ported 1:1: enums, optional, class_refs, recursion (both
  SCCs), aliases (string list), forward_refs, the optional-args
  UNSET/nil matrix (negative cases are compile errors in Swift, noted
  per file), previously-deferred class cases in primitives/maps,
  type_shapes namespace reachability, and docstrings_etc as
  generated-source assertions (Swift has no runtime __doc__; /// in
  source is the analog).

Deliberately deferred: Codable conformance on generated structs
(nothing in the parity suite needs it; revisit with serde delegation),
unions (Phase 3), generics + $stream companions (Phase 5), methods
(Phase 4).

Co-Authored-By: Claude Fable 5 <[email protected]>
BAML unions are structural; Swift's sum types are nominal. Instead of
generating a named enum per union shape (an earlier design — every name
an API promise, placement dependent on first-use, decode by structural
guessing), ship ONE reusable generic family in the runtime and never
synthesize a union type name. Cross-bridge aligned (C# BamlUnion<T0,T1>,
Java BamlUnion2<A,B>); design doc at sdks/swift/docs/unions-design.md.

- BamlBridge/BamlUnions.swift: BamlUnion2..8 as `indirect` generic
  enums — positional cases (t0/t1/... in canonical BAML arm order),
  type-directed init overloads (arm by argument type — insertion-
  stable construction, the C# implicit-conversion analog), anyValue,
  per-arm accessors, value(as:)/holds (std::variant analogs), labeled
  match (sync + async; exhaustive by signature), conditional
  Equatable/Hashable/Sendable. `int | string` is BamlUnion2<Int,String>
  everywhere: structural identity, zero generated types.
- Wire: encode passes the selected arm bare (unchanged). Decode is
  METADATA-FIRST: the union_variant_value wrapper's value_option_name
  names the selected arm and is matched against a new _bamlArmIdentity
  protocol REQUIREMENT (witness-dispatched; extension-only members
  would statically resolve to the nil default) — primitives declare
  canonical names, generated classes/enums their FQN; class-FQN match
  second; structural try-order last. Fixes the old design's flaw of
  discarding the wire's answer and guessing.
- Emitter: Ty::Union → normalize (null→Optional, dedup, singleton
  collapse) → literal arms to their base types (literal-only unions
  collapse entirely — "draft"|"sent"|"paid" is String; no raw-value
  enums) → BamlUnionN<...> inline. No registry, naming, or placement
  machinery exists. Non-recursive union aliases are typealiases;
  recursive ones (RecList = int | RecList[]) emit a nominal indirect
  enum under the USER'S name with the identical family surface
  (typealias can't self-reference — every nominal-language bridge
  needs this escape hatch). Cycle-boxer skips >=2-arm unions
  (indirect enums are already heap-boxed).
- Tests: TestUnions exercises all three consumption tiers (exhaustive
  switch, match, type-directed access) plus the normalization matrix;
  TestComplexModels asserts class-arm identity survives the round trip
  (payment .t0/.t1 selected by wire identity, not guessing); union
  cases unlocked in Lists/Optional/Aliases/ForwardRefs. Gate 5/5.

Co-Authored-By: Claude Fable 5 <[email protected]>
Gate 5/5 green. Three capability rows land, plus the stdlib package
opens to codegen.

- Methods: render_callable grows FnKind (Free/Static/Instance); class
  methods emit on their structs with class-scoped FQNs and the
  ("self", receiver) kwarg-0 convention. Greeter + OptBox suites.
- Error contract: BamlError/BamlPanic carry the raw thrown payload;
  `err.value(as: MyError.self)` is the typed analog of Python's
  `exc.value` + isinstance. Union-throws preserve className; engine
  traces assert; `- Throws:` doc lines are the Raises: analog.
- Stdlib emission: pkg filters open to `baml` (-> Baml.baml.*) and
  vendor pkgs (-> Baml.vendor.*), pruned by the existing fixpoint.
  Two real bugs flushed out: `baml.swift` clobbered `Baml.swift` on
  case-insensitive APFS (root file now BamlRoot.swift); stdlib classes
  named String/Int shadowed Swift's own inside generated scopes (all
  standard types now emit fully qualified: Swift.String,
  Foundation.Data, ... — including the family unions' _bamlArmIdentity
  and holds() spellings). Replay note: under BamlUnionN family unions,
  literal-only unions collapse to their base type, so the old literal
  raw-value enums (and their "r"/"r+" duplicate-case hazard) don't
  exist; nullable recursive union aliases (stdlib json) gained support
  (non-null arms in the nominal enum, ? at reference sites).
- Host callables (BAML -> Swift closures): generated closure params are
  uniformly `async throws` (sync closures coerce); an erased
  BamlHostCallable registers in the single per-process registry
  (callables + opaque thrown errors, bridge_python's design) and rides
  as HOST_VALUE_CALLABLE. Dispatch copies the BamlToHostCall bytes and
  hops to a detached Task (fire-and-return; no sync re-entrancy), then
  complete_host_call exactly once: ok InboundValue / typed throw via
  BamlThrownValue (matched against the declared contract) / opaque
  baml.errors.HostCallable envelope whose _handle rehydrates the
  ORIGINAL Swift error on the way back out. Wire lesson: the engine
  requires the envelope's `traceback string?` field present-as-null.
- Tests: TestMethodsOnClasses, TestErrors, TestHostCallables (19
  cases), OptBox matrix unlocked in TestOptionalArgs.

Remaining for Phase 4b: BamlHandle + $rust_type fields, media handle
tags, stdlib entrypoints (fs/sys/http) + test_handles.

Co-Authored-By: Claude Fable 5 <[email protected]>
Gate 5/5 green. Completes Phase 4: opaque engine resources round-trip
and the stdlib is callable end to end.

- BamlHandle: final class owning an engine handle-table key. deinit
  releases (EXCEPT host-value keyspaces — the per-bridge registry is
  disjoint from HANDLE_TABLE; Python's BamlPyHandle::Drop rule);
  encoding clones a fresh key for the wire (`_clone_key_for_wire`
  semantics) so instances stay independently droppable; equality is
  resource identity.
- Ty::RustType -> `BamlHandle?`, unlocking File/Response/Image and
  every other $rust_type-bearing stdlib class through the existing
  fixpoint — their read/seek/text/mime_type methods come free via the
  4a method emission.
- Media: constructors are VM-native ops that never enter the codegen
  pool, so BamlMedia.fromBase64 wraps the baml_media_* C ABI (the same
  reason Python routes construction through its PyO3 wrapper).
  Generated single-rust-field classes decode BOTH forms: class_value
  (normal instances) and the bare ADT_MEDIA_* tagged handle the engine
  emits for media values.
- Collision rule: BAML allows a function and a child namespace with
  the same name (Python separates module vs attribute lookup); Swift
  has one lookup space per scope, so `func id()` vs `enum id`
  (vendor boundary.id) is an invalid redeclaration. The namespace wins;
  colliding functions drop. Surfaced only now because RustType support
  made boundary.id()'s return type emittable.
- Tests: TestHandles (media payload round-trip, HTTP fetch against a
  minimal in-test socket server, file open/close, cursor state
  persisting across FFI calls) and TestStdlibEntrypoints (now_ms,
  fs.exists, compiler-intrinsics source assertions).

Co-Authored-By: Claude Fable 5 <[email protected]>
The final capability phase: every sdk_tests fixture is now enforced
(6/6, zero skipped), including the streaming e2e over the replay
server. No wire type_args anywhere — Swift's static generics plus the
engine's inbound inference cover every case Python needs runtime
machinery for.

- Generics: `class Wrapper<T>` -> `struct Wrapper<T: BamlCodableValue>`
  (Equatable+Sendable+codec constraint bundle); parameterized refs
  (`Wrapper<int>` -> `Wrapper<Swift.Int>`); bare TypeVar translation;
  boxing analysis covers parameterized self-references
  (GenericLinkedList<T>). Generic functions/methods emit a Swift
  generic signature when every TypeVar is inferable from a required
  argument VALUE (the engine re-infers server-side; nothing on the
  wire) — return-only TypeVars (parse_as) stay unemitted until a wire
  type-hint hook exists (Python requires _types= for those too), and
  the coverage check deliberately does not recurse into Callable
  params (host callables are opaque to inference; Python's apply<T,R>
  proves it). Python's subscript/_types=/reified-metadata surfaces
  collapse into compile-time facts.
- Streaming: a `$stream` companion is an ORDINARY pool function whose
  return type is baml.llm.Stream<Partial, Final> — translate maps it
  to the runtime BamlStream wrapper (never a generated struct);
  next/final reuse the standard call path with ("self", handle), and
  next() returns BamlStreamNext<Partial> (.value/.finished — the
  StreamFinished sentinel is distinct from a legitimately-nil
  partial). `$` maps to `_` wholesale (fn_stream, fn_build_request,
  fn_parse, fn_parse_stream); `$stream` CLASSES route under
  Baml.stream_types.<ns> (suffix stripped), FUNCTIONS beside their
  parent — Python's routing split, mirrored. Two real bugs found by
  the llm fixture: the decl-map sort key used bare_name(), so a base
  function silently overwrote its $stream companion; and
  namespace_for routed companion FUNCTIONS into stream_types.
- Replay harness: ports to ~60 lines because replay_serve_detached is
  itself a generated BAML function — call it, setenv the
  BAML_REPLAY_* vars, POST shutdown. No thread/addr-file dance.
- Media: Ty::Media -> the 4b-generated Baml.baml.media.* structs;
  ns_media roundtrips pass (engine-minted values through typed calls).
- Cancellation: engine baml.panics.Cancelled -> Swift
  CancellationError (async-only, Python parity); Task-cancel and
  timeout-race tests assert the 0.5s fast-cancel bound.
  BamlCallContext remains unported (structured concurrency owns
  cancellation in Swift; noted in tests).
- Tests: TestGenerics, TestGenericMethods, TestGenericInference (35
  portable cases from both Python generic suites, Python-specific
  cases documented), TestMedia, TestStreams (companion types as
  values), TestCancellation, llm TestMain ($build_request header
  pipeline against real env keys) + TestStreamingE2E (6 cases) +
  ReplayHarness. llm_functions joins ENFORCED_FIXTURES.

Co-Authored-By: Claude Fable 5 <[email protected]>
# Conflicts:
#	baml_language/Cargo.lock
#	baml_language/Cargo.toml
#	baml_language/crates/baml_cli/Cargo.toml
#	baml_language/crates/baml_cli/src/generate.rs
#	baml_language/crates/baml_codegen_types/src/generator_fields.rs
#	baml_language/sdk_tests/harness_runner/src/lib.rs
#	baml_language/sdk_tests/harness_setup/Cargo.toml
#	baml_language/sdk_tests/harness_setup/src/lib.rs
Canary standardized the bridge ABI onto a single versioned function
table (BamlApiV1) resolved through baml_get_api_v1(), replacing the
flat #[no_mangle] symbol surface the Swift runtime consumed in phases
0-5. Adopt it:

- Add BamlBridgeLanguage::Swift = 7 to bridge_cffi (registry, TryFrom,
  telemetry/display names, abi_layout discriminant assert) and
  regenerate the canonical header.
- New BamlBridge/Api.swift resolves the table once (fatalError on a
  null / not-V1-compatible library) and unwraps every required V1
  function pointer into a typed static; all native call sites
  (Runtime, Handle, Media, HostCallables, Decode) route through it.
- initialize() now calls register_bridge with the SDK's stamped
  version, so a generated SDK can never silently run against a
  different runtime release. sdkgen emits Baml.sdkVersion from
  baml_version::CANONICAL_VERSION and threads it into initialize().
- Swap the hand-maintained baml_bridge.h for a build-synced copy of
  the canonical crates/bridge_cffi/include/baml_cffi.h; the modulemap
  and xcframework script follow.
- Media.Kind values move onto the canonical BamlCffiMediaKind numbering
  (image=1..generic=5); status checks compare against BAML_CFFI_STATUS_OK.

Also fold in the canary merge fallout: the Ty family gained a per-variant
TyAttr, so alias/enum declaration names use bare_name() (fixing $stream
companion aliases) and `never` return types render as void functions.
Enforce the new unsupported_only fixture. Gate: 7/7, 0 skipped.
…mobile targets

The --all xcframework path failed linking aarch64-apple-ios: bex_cache
(new since the iOS feasibility spike) enabled reqwest's aws-lc-rs
provider, whose C objects target iOS 26.5 and reference
___chkstk_darwin. Give bex_cache the same aws-crypto/ring-crypto
feature split as sys_native/sys_llm, thread it through bex_project,
and have build-xcframework.sh build *-apple-ios* targets with
--no-default-features --features ring-crypto (the configuration the
on-device spike validated). macOS keeps the default backend.

Gate: 7/7. The glasses-vision demo app now builds for iOS simulator
against the full three-slice XCFramework.
…ped symbols

The fixpoint's skip posture meant an unsupported construct produced
absent API with no trace (the Ty::Never/DoPanic incident). Now every
skip is recorded with the concrete type that failed translation and
emitted into the generated output as _BamlSkipped.swift:

- reasons probe the failing signature in the renderer's own order
  (generic coverage, parameters, return type), so the reported cause
  is the one that made the renderer bail
- covers functions, classes, aliases, static/instance methods, and
  namespace-collision drops
- skips in the author's own (user) package raise a #warning at every
  build of the package; the stdlib/vendor baseline is listed under a
  separate section without warning — a warning that always fires
  would be ignored
- an empty manifest is emitted as the positive nothing-was-skipped
  signal

Gate: 7/7, 0 skipped.

Co-Authored-By: Claude Fable 5 <[email protected]>
@vercel

vercel Bot commented Jul 21, 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 21, 2026 6:48pm
promptfiddle Ready Ready Preview, Comment Jul 21, 2026 6:48pm
promptfiddle2 Ready Ready Preview, Comment Jul 21, 2026 6:48pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e6d1c7b0-0408-4b85-80a9-48f14ea157ad

📥 Commits

Reviewing files that changed from the base of the PR and between 0fc826a and 96e7dbe.

📒 Files selected for processing (1)
  • baml_language/sdks/swift/Sources/BamlBridge/HostCallables.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • baml_language/sdks/swift/Sources/BamlBridge/HostCallables.swift

📝 Walkthrough

Walkthrough

This PR adds Swift SDK generation, a versioned CFFI bridge, Swift runtime encoding/decoding and invocation APIs, SwiftPM/XCFramework packaging, and extensive fixture-based tests for calls, types, errors, streams, handles, and documentation.

Changes

Swift SDK bridge

Layer / File(s) Summary
ABI, protobuf models, and runtime bridge
baml_language/sdks/swift/Sources/CBamlBridge/*, baml_language/sdks/swift/Sources/BamlBridge/Proto/*, baml_language/sdks/swift/Sources/BamlBridge/*.swift
Adds the versioned native function-table ABI, generated protobuf models, Swift value encoding/decoding, unions, handles, errors, host callables, streams, and synchronous/asynchronous runtime invocation.
Swift code generation and CLI integration
baml_language/sdks/swift/rust/sdkgen_swift/*, baml_language/crates/baml_cli/*, baml_language/crates/baml_codegen_types/*
Adds Swift type translation and source emission, recursive type handling, skipped-symbol diagnostics, embedded bytecode initialization, and CLI dispatch for OutputType::Swift.
Packaging and test harness
baml_language/sdks/swift/Package.swift, baml_language/sdks/swift/scripts/*, baml_language/sdk_tests/harness_setup/*, baml_language/sdk_tests/crates/swift/*
Adds SwiftPM/XCFramework packaging, bridge build and generation scripts, fixture code generation, nextest setup, and generated Rust test scaffolding.
Swift SDK tests and documentation
baml_language/sdk_tests/crates/swift/**, baml_language/sdks/swift/docs/*
Adds XCTest coverage for generated calls, models, generics, unions, errors, cancellation, host callables, media, handles, streams, and documentation lowering, plus Swift bridge and union design documentation.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Poem

A bunny hops through Swift code bright,
With unions boxed just right.
Calls stream, errors, handles too,
Tests bloom in XCTest dew.
The bridge hums beneath the moon—
New SDK magic, hopping soon!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.35% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is too branch-like and vague; it doesn’t clearly describe the Swift bridge changes in the PR. Rename it to a concise summary of the main change, e.g. "Add Swift bridge and SDK generation support".
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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 dhilan/swift-bridge-1
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch dhilan/swift-bridge-1

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0fc826a994

ℹ️ 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".

}
let _ = write!(
out,
"\n\tpublic static var _bamlArmIdentity: Swift.String? {{ \"{fqn}\" }}\n\n\

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use the wire's enum identity for union arms

For unions of user-defined enums, this stamps _bamlArmIdentity with the canonical user... FQN, but the CFFI union metadata is populated from format!("{}", metadata.selected_option) and RuntimeTy display renders enum names with display_name() (eliding the reserved user package). In a supported return type like A | B where both enums have a raw value X, the metadata comparison misses and decoding falls through to structural try-order, so a wire-selected B.X can be returned as the first arm A.X. Please emit/compare the same wire enum name, or use the enum value's FQN before structural fallback.

Useful? React with 👍 / 👎.

Comment on lines +82 to +84
if let fqn = v.wireClassFQN() {
if let identity = T0._bamlArmIdentity, identity == fqn { return .t0(try T0._bamlDecode(v)) }
if let identity = T1._bamlArmIdentity, identity == fqn { return .t1(try T1._bamlDecode(v)) }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include generic args when matching class union arms

For a supported union such as Box<int> | Box<string>, wireClassFQN() returns only the nominal class name and both generated Box<T> instantiations expose the same _bamlArmIdentity, so this branch always selects T0; it then either throws while decoding the first instantiation or mislabels values that both instantiations can decode (for example nullable or empty fields). The outbound class value carries concrete typeArgs, so the class-arm match needs to include those args, or at least avoid returning permanently on a failed first-instantiation decode.

Useful? React with 👍 / 👎.

@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: 4

🧹 Nitpick comments (4)
baml_language/sdks/swift/rust/sdkgen_swift/src/translate_ty.rs (1)

92-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Match MediaKind directly
format!("{kind:?}") ties this branch to Debug output; a direct match kind keeps the supported media cases compiler-checked and leaves Generic on the unsupported path explicitly.

🤖 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/sdks/swift/rust/sdkgen_swift/src/translate_ty.rs` around lines
92 - 100, Update the Ty::Media branch to match the MediaKind value directly
instead of converting kind through Debug formatting. Map Image, Audio, Video,
and Pdf to their existing names, and explicitly return None for Generic or other
unsupported variants while preserving the Baml.baml.media.{name} output.
baml_language/sdks/swift/Sources/BamlBridge/Decode.swift (2)

273-285: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Doc comment overstates strictness of hex parsing.

The comment says "Strict lowercase-hex bigint parse," but UInt64(_:radix:) parses hex case-insensitively, so uppercase digits ("1F") would also succeed here despite the doc's claim. Harmless if the wire only ever emits lowercase, but the comment is misleading.

🤖 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/sdks/swift/Sources/BamlBridge/Decode.swift` around lines 273 -
285, The doc comment for parseHexBigintFittingInt inaccurately claims strict
lowercase-hex parsing. Update the comment to describe the actual
case-insensitive hexadecimal behavior, or add explicit validation if
lowercase-only parsing is required; keep the existing Int range and sign
handling unchanged.

343-361: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Union-unwrap loop duplicated instead of reusing BamlOutboundValue.normalized.

describeThrownValue re-implements the same while case .unionVariantValue unwrapping loop already defined on BamlOutboundValue.normalized (Lines 17-23). If the union-unwrap semantics ever change (e.g. additional peeling rules), this copy could silently drift out of sync.

♻️ Reuse `normalized` instead of duplicating the loop
 private func describeThrownValue(
     _ value: BamlBridge_Cffi_V1_BamlOutboundValue
 ) -> (message: String, className: String?) {
-    var current = value
-    while case .unionVariantValue(let variant) = current.value {
-        current = variant.value
-    }
-    switch current.value {
+    let current = BamlOutboundValue(value).normalized
+    switch current.value {
🤖 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/sdks/swift/Sources/BamlBridge/Decode.swift` around lines 343 -
361, Update describeThrownValue to use BamlOutboundValue.normalized for union
unwrapping instead of maintaining its local while loop, then apply the existing
switch logic to the normalized value. Preserve the current class, string, and
default formatting behavior.
baml_language/sdk_tests/crates/swift/llm_functions/customizable/ReplayHarness.swift (1)

41-66: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Blocking semaphore wait inside an async teardown path.

shutdown(addr) blocks the calling thread with DispatchSemaphore.wait for up to 6s, and it's invoked from withAsync's defer (line 51). Even though this compiles fine (shutdown is a plain sync function, not awaited), blocking synchronously here ties up a thread from Swift's bounded cooperative thread pool, which can slow down or stall concurrent async test teardowns.

Consider an async variant using URLSession.shared.data(for:) for the withAsync path so teardown doesn't block a pool thread.

♻️ Suggested async shutdown for the async path
+    private static func shutdownAsync(_ addr: String) async {
+        guard let url = URL(string: "http://\(addr)/__replay__/shutdown") else { return }
+        var request = URLRequest(url: url, timeoutInterval: 5)
+        request.httpMethod = "POST"
+        request.httpBody = Data()
+        _ = try? await URLSession.shared.data(for: request)
+    }
+
     static func withAsync<T>(
         recording: String,
         _ body: () async throws -> T
     ) async throws -> T {
         let addr = try await Baml.replay.replay_serve_detached_async(
             recording_path: recordingPath(recording)
         )
         setenv("BAML_REPLAY_BASE_URL", "http://\(addr)", 1)
         setenv("BAML_REPLAY_API_KEY", "replay-test-key", 1)
-        defer {
-            shutdown(addr)
-            unsetenv("BAML_REPLAY_BASE_URL")
-            unsetenv("BAML_REPLAY_API_KEY")
-        }
-        return try await body()
+        do {
+            let result = try await body()
+            await shutdownAsync(addr)
+            unsetenv("BAML_REPLAY_BASE_URL")
+            unsetenv("BAML_REPLAY_API_KEY")
+            return result
+        } catch {
+            await shutdownAsync(addr)
+            unsetenv("BAML_REPLAY_BASE_URL")
+            unsetenv("BAML_REPLAY_API_KEY")
+            throw error
+        }
     }
🤖 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/sdk_tests/crates/swift/llm_functions/customizable/ReplayHarness.swift`
around lines 41 - 66, Update withAsync to perform non-blocking asynchronous
teardown by awaiting an async shutdown helper that uses
URLSession.shared.data(for:) instead of DispatchSemaphore.wait. Preserve the
existing URL, POST request, timeout, and environment cleanup behavior, while
retaining shutdown for synchronous callers if needed.
🤖 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/swift/rust/sdkgen_swift/src/diagnostics.rs`:
- Around line 51-66: The argument representability probe diverges from the
renderer for required callable parameters. Update the loop over
`function.arguments` to mirror `render_callable`/`render_callable_param` for
`Ty::Function`, probing the callable parameter’s inner signature before falling
back to `translate_ty`, so diagnostics report the renderer’s actual first
failure.

In `@baml_language/sdks/swift/rust/sdkgen_swift/src/emit.rs`:
- Around line 552-561: Update the class-arm emission loop in the surrounding
union-generation function so each original `Ty::Class` arm resolves its matching
deduplicated type and variant index within `arm_tys` before generating the
`_bamlDecode` call and `.tN` case. Do not use the raw `arms` enumeration index
for either `arm_tys` lookup or variant selection, and preserve the existing
`wireClassFQN()` matching behavior.

In `@baml_language/sdks/swift/Sources/BamlBridge/Handle.swift`:
- Around line 30-38: Update BamlHandle’s Equatable implementation in static func
== so identity compares both handleType and key. Preserve equality for handles
from the same domain with matching keys, while ensuring host-value and
engine-owned handles with colliding numeric keys compare unequal.

In `@baml_language/sdks/swift/Sources/BamlBridge/Runtime.swift`:
- Around line 204-216: Update registerPending to select an unused, nonzero
callback ID before inserting into pending, looping through wrapped IDs while
pending already contains the candidate and preserving lock protection. Ensure
the selected id, rather than the initially read nextCallbackId, is stored in
pending and returned; do not leave the comment claiming occupied IDs are skipped
unless the implementation enforces it.

---

Nitpick comments:
In
`@baml_language/sdk_tests/crates/swift/llm_functions/customizable/ReplayHarness.swift`:
- Around line 41-66: Update withAsync to perform non-blocking asynchronous
teardown by awaiting an async shutdown helper that uses
URLSession.shared.data(for:) instead of DispatchSemaphore.wait. Preserve the
existing URL, POST request, timeout, and environment cleanup behavior, while
retaining shutdown for synchronous callers if needed.

In `@baml_language/sdks/swift/rust/sdkgen_swift/src/translate_ty.rs`:
- Around line 92-100: Update the Ty::Media branch to match the MediaKind value
directly instead of converting kind through Debug formatting. Map Image, Audio,
Video, and Pdf to their existing names, and explicitly return None for Generic
or other unsupported variants while preserving the Baml.baml.media.{name}
output.

In `@baml_language/sdks/swift/Sources/BamlBridge/Decode.swift`:
- Around line 273-285: The doc comment for parseHexBigintFittingInt inaccurately
claims strict lowercase-hex parsing. Update the comment to describe the actual
case-insensitive hexadecimal behavior, or add explicit validation if
lowercase-only parsing is required; keep the existing Int range and sign
handling unchanged.
- Around line 343-361: Update describeThrownValue to use
BamlOutboundValue.normalized for union unwrapping instead of maintaining its
local while loop, then apply the existing switch logic to the normalized value.
Preserve the current class, string, and default formatting behavior.
🪄 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: 6188e15a-3619-46fd-a715-fb5e3ae386fb

📥 Commits

Reviewing files that changed from the base of the PR and between cddb6ba and 0fc826a.

⛔ Files ignored due to path filters (2)
  • baml_language/Cargo.lock is excluded by !**/*.lock
  • baml_language/sdks/swift/Package.resolved is excluded by !**/Package.resolved
📒 Files selected for processing (85)
  • baml_language/.config/nextest.toml
  • baml_language/Cargo.toml
  • baml_language/crates/baml_cli/Cargo.toml
  • baml_language/crates/baml_cli/src/generate.rs
  • baml_language/crates/baml_codegen_types/src/generator_fields.rs
  • baml_language/crates/bex_cache/Cargo.toml
  • baml_language/crates/bex_project/Cargo.toml
  • baml_language/crates/bridge_cffi/include/baml_cffi.h
  • baml_language/crates/bridge_cffi/src/ffi/runtime.rs
  • baml_language/crates/bridge_cffi/tests/abi_layout.rs
  • baml_language/sdk_tests/crates/swift/Cargo.toml
  • baml_language/sdk_tests/crates/swift/build.rs
  • baml_language/sdk_tests/crates/swift/docstrings_etc/customizable/TestMain.swift
  • baml_language/sdk_tests/crates/swift/function_calls/customizable/TestCancellation.swift
  • baml_language/sdk_tests/crates/swift/function_calls/customizable/TestErrors.swift
  • baml_language/sdk_tests/crates/swift/function_calls/customizable/TestGenericInference.swift
  • baml_language/sdk_tests/crates/swift/function_calls/customizable/TestHostCallables.swift
  • baml_language/sdk_tests/crates/swift/function_calls/customizable/TestMain.swift
  • baml_language/sdk_tests/crates/swift/function_calls/customizable/TestMethodsOnClasses.swift
  • baml_language/sdk_tests/crates/swift/function_calls/customizable/TestOptionalArgs.swift
  • baml_language/sdk_tests/crates/swift/function_calls/customizable/TestStdlibEntrypoints.swift
  • baml_language/sdk_tests/crates/swift/llm_functions/customizable/ReplayHarness.swift
  • baml_language/sdk_tests/crates/swift/llm_functions/customizable/TestMain.swift
  • baml_language/sdk_tests/crates/swift/llm_functions/customizable/TestStreamingE2E.swift
  • baml_language/sdk_tests/crates/swift/setup.sh
  • baml_language/sdk_tests/crates/swift/src/lib.rs
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/TestComplexModels.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/TestGenericMethods.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/TestMain.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestAliases.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestClassRefs.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestEnums.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestForwardRefs.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestGenerics.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestHandles.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestLists.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestLiterals.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestMaps.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestMedia.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestOptional.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestPrimitives.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestRecursion.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestRouting.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestStreams.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestSymbolCollisions.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestUnions.swift
  • baml_language/sdk_tests/crates/swift/type_shapes/customizable/roundtrip_tests/TestVoid.swift
  • baml_language/sdk_tests/harness_runner/src/lib.rs
  • baml_language/sdk_tests/harness_setup/Cargo.toml
  • baml_language/sdk_tests/harness_setup/src/lib.rs
  • baml_language/sdk_tests/harness_setup/src/swift.rs
  • baml_language/sdk_tests/harness_setup/src/templates/Package.swift
  • baml_language/sdks/swift/.gitignore
  • baml_language/sdks/swift/Package.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Api.swift
  • baml_language/sdks/swift/Sources/BamlBridge/BamlUnions.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Decode.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Encode.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Errors.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Handle.swift
  • baml_language/sdks/swift/Sources/BamlBridge/HostCallables.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Indirect.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Media.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Optional.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Proto/baml_handle.pb.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Proto/baml_inbound.pb.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Proto/baml_outbound.pb.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Proto/baml_type.pb.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Runtime.swift
  • baml_language/sdks/swift/Sources/BamlBridge/Stream.swift
  • baml_language/sdks/swift/Sources/CBamlBridge/include/baml_cffi.h
  • baml_language/sdks/swift/Sources/CBamlBridge/include/module.modulemap
  • baml_language/sdks/swift/Tests/BamlBridgeTests/FFISmokeTests.swift
  • baml_language/sdks/swift/docs/state-of-baml-swift.md
  • baml_language/sdks/swift/docs/unions-design.md
  • baml_language/sdks/swift/rust/bridge_swift/Cargo.toml
  • baml_language/sdks/swift/rust/bridge_swift/src/lib.rs
  • baml_language/sdks/swift/rust/sdkgen_swift/Cargo.toml
  • baml_language/sdks/swift/rust/sdkgen_swift/src/diagnostics.rs
  • baml_language/sdks/swift/rust/sdkgen_swift/src/emit.rs
  • baml_language/sdks/swift/rust/sdkgen_swift/src/lib.rs
  • baml_language/sdks/swift/rust/sdkgen_swift/src/translate_ty.rs
  • baml_language/sdks/swift/scripts/build-xcframework.sh
  • baml_language/sdks/swift/scripts/generate-header.sh
  • baml_language/sdks/swift/scripts/generate-protos.sh

Comment on lines +51 to +66
for arg in &function.arguments {
let ok = if arg.default.is_some() {
translate_optional_arg_inner(&arg.ty, ctx).is_some()
} else {
// Callable params succeed iff every piece of their own
// signature translates — same probe either way.
translate_ty(&arg.ty, ctx).is_some()
};
if !ok {
return format!(
"parameter `{}`: type `{}` is not representable in Swift",
arg.name.as_str(),
arg.ty
);
}
}

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Callable-parameter probe diverges from the renderer, producing a misleading skip reason.

render_callable in emit.rs handles a Ty::Function argument specially via render_callable_param, but here the required-arg branch calls translate_ty(&arg.ty, ctx), which returns None for Ty::Function unconditionally. So a function whose host-callable param is actually representable, but which is skipped for a later cause (e.g. an unrepresentable return type), gets misreported as parameter ...: type ... is not representable — contradicting the invariant documented at Lines 11-14 that the first reported cause matches the renderer's. Consider mirroring the Ty::Function branch here (probe via render_callable_param's inner logic) before falling back to translate_ty.

🤖 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/sdks/swift/rust/sdkgen_swift/src/diagnostics.rs` around lines
51 - 66, The argument representability probe diverges from the renderer for
required callable parameters. Update the loop over `function.arguments` to
mirror `render_callable`/`render_callable_param` for `Ty::Function`, probing the
callable parameter’s inner signature before falling back to `translate_ty`, so
diagnostics report the renderer’s actual first failure.

Comment on lines +552 to +561
out.push_str("\t\tif let fqn = v.wireClassFQN() {\n");
for (i, arm) in arms.iter().enumerate() {
if let Ty::Class(class_name, _, _) = arm {
let _ = writeln!(
out,
"\t\t\tif fqn == \"{class_name}\" {{ return .t{i}(try {}._bamlDecode(v)) }}",
arm_tys[i]
);
}
}

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.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Index mismatch between arms and deduped arm_tys — panics / emits wrong cases.

arm_tys is built with structural dedup (Lines 488-494), so its indices no longer line up with the original arms. Here the loop enumerates the un-deduped arms but indexes arm_tys[i] and emits .t{i}. When an earlier arm collapsed (e.g. "draft" | "sent" | SomeClass | ... where the two string literals dedup to one Swift.String), a later Ty::Class arm sits at an arms index >= arm_tys.len(), so arm_tys[i] triggers an index-out-of-bounds panic during generation; below that threshold it still selects the wrong deduped type and wrong .t{i} case. Resolve the arm's index within arm_tys:

🐛 Proposed fix
     out.push_str("\t\tif let fqn = v.wireClassFQN() {\n");
-    for (i, arm) in arms.iter().enumerate() {
-        if let Ty::Class(class_name, _, _) = arm {
-            let _ = writeln!(
-                out,
-                "\t\t\tif fqn == \"{class_name}\" {{ return .t{i}(try {}._bamlDecode(v)) }}",
-                arm_tys[i]
-            );
-        }
-    }
+    for arm in arms {
+        if let Ty::Class(class_name, _, _) = arm {
+            let ty = translate_ty(arm, ctx)?;
+            if let Some(idx) = arm_tys.iter().position(|t| *t == ty) {
+                let _ = writeln!(
+                    out,
+                    "\t\t\tif fqn == \"{class_name}\" {{ return .t{idx}(try {ty}._bamlDecode(v)) }}"
+                );
+            }
+        }
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
out.push_str("\t\tif let fqn = v.wireClassFQN() {\n");
for (i, arm) in arms.iter().enumerate() {
if let Ty::Class(class_name, _, _) = arm {
let _ = writeln!(
out,
"\t\t\tif fqn == \"{class_name}\" {{ return .t{i}(try {}._bamlDecode(v)) }}",
arm_tys[i]
);
}
}
out.push_str("\t\tif let fqn = v.wireClassFQN() {\n");
for arm in arms {
if let Ty::Class(class_name, _, _) = arm {
let ty = translate_ty(arm, ctx)?;
if let Some(idx) = arm_tys.iter().position(|t| *t == ty) {
let _ = writeln!(
out,
"\t\t\tif fqn == \"{class_name}\" {{ return .t{idx}(try {ty}._bamlDecode(v)) }}"
);
}
}
}
🤖 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/sdks/swift/rust/sdkgen_swift/src/emit.rs` around lines 552 -
561, Update the class-arm emission loop in the surrounding union-generation
function so each original `Ty::Class` arm resolves its matching deduplicated
type and variant index within `arm_tys` before generating the `_bamlDecode` call
and `.tN` case. Do not use the raw `arms` enumeration index for either `arm_tys`
lookup or variant selection, and preserve the existing `wireClassFQN()` matching
behavior.

Comment on lines +30 to +38
extension BamlHandle: Equatable {
/// Identity is the table key. Two handles to the same resource
/// minted separately compare unequal — generated structs holding
/// handles compare by resource identity, mirroring Python where
/// private handle attrs sit outside pydantic equality.
public static func == (lhs: BamlHandle, rhs: BamlHandle) -> Bool {
lhs.key == rhs.key
}
}

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Equatable compares only key, but key spaces can numerically collide across handle domains.

The deinit comment two lines above explicitly warns that releasing a host-value key via handleRelease "would evict a numerically-colliding engine entry" — i.e. host-value registry keys and engine handle-table keys can share the same numeric value. == here ignores handleType, so two BamlHandle instances from different domains (e.g. one host-value, one engine-owned) with a colliding numeric key would incorrectly compare equal. This directly affects Equatable on any generated model that embeds a handle field.

🐛 Include handleType in the identity check
 public static func == (lhs: BamlHandle, rhs: BamlHandle) -> Bool {
-    lhs.key == rhs.key
+    lhs.key == rhs.key && lhs.handleType == rhs.handleType
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
extension BamlHandle: Equatable {
/// Identity is the table key. Two handles to the same resource
/// minted separately compare unequal — generated structs holding
/// handles compare by resource identity, mirroring Python where
/// private handle attrs sit outside pydantic equality.
public static func == (lhs: BamlHandle, rhs: BamlHandle) -> Bool {
lhs.key == rhs.key
}
}
extension BamlHandle: Equatable {
/// Identity is the table key. Two handles to the same resource
/// minted separately compare unequal — generated structs holding
/// handles compare by resource identity, mirroring Python where
/// private handle attrs sit outside pydantic equality.
public static func == (lhs: BamlHandle, rhs: BamlHandle) -> Bool {
lhs.key == rhs.key && lhs.handleType == rhs.handleType
}
}
🤖 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/sdks/swift/Sources/BamlBridge/Handle.swift` around lines 30 -
38, Update BamlHandle’s Equatable implementation in static func == so identity
compares both handleType and key. Preserve equality for handles from the same
domain with matching keys, while ensuring host-value and engine-owned handles
with colliding numeric keys compare unequal.

Comment on lines +204 to +216
private func registerPending(
_ completion: @escaping @Sendable (Result<Data, Error>) -> Void
) -> UInt32 {
lock.lock()
defer { lock.unlock() }
let id = nextCallbackId
// 32-bit wrap is theoretical (4B in-flight calls) but stay
// defensive: skip ids still pending.
nextCallbackId = nextCallbackId &+ 1
if nextCallbackId == 0 { nextCallbackId = 1 }
pending[id] = completion
return id
}

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.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

registerPending does not "skip ids still pending" as its comment claims. The comment states the wrap logic stays defensive by skipping in-flight ids, but the code only avoids 0 — it never checks pending[id]. On a 32-bit wrap, pending[id] = completion silently overwrites a live entry, so the overwritten call's semaphore/continuation is never resumed and that call hangs forever. Either skip occupied ids or drop the misleading comment.

🛠️ Skip ids still in flight
         let id = nextCallbackId
-        // 32-bit wrap is theoretical (4B in-flight calls) but stay
-        // defensive: skip ids still pending.
-        nextCallbackId = nextCallbackId &+ 1
-        if nextCallbackId == 0 { nextCallbackId = 1 }
+        // 32-bit wrap is theoretical (4B in-flight calls) but stay
+        // defensive: advance past ids still pending.
+        repeat {
+            nextCallbackId = nextCallbackId &+ 1
+            if nextCallbackId == 0 { nextCallbackId = 1 }
+        } while pending[nextCallbackId] != nil
         pending[id] = completion
         return id

Note: this still assumes id itself is free on wrap; if strict correctness is desired, pick the first free id for id as well.

🤖 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/sdks/swift/Sources/BamlBridge/Runtime.swift` around lines 204 -
216, Update registerPending to select an unused, nonzero callback ID before
inserting into pending, looping through wrapped IDs while pending already
contains the candidate and preserving lock protection. Ensure the selected id,
rather than the initially read nextCallbackId, is stored in pending and
returned; do not leave the comment claiming occupied IDs are skipped unless the
implementation enforces it.

Error implies Sendable; storing a bare `any BamlEncodable` fails
strict-concurrency checking on newer compilers (caught by the dist
repo's smoke gate on macos-15 — local toolchains let it slide). The
payload is now `any BamlEncodable & Sendable`: the honest constraint,
satisfied by every realistic thrown value (generated models are
Sendable value types), so no call-site changes.

Gate: 7/7.

Co-Authored-By: Claude Fable 5 <[email protected]>

This branch was previously deployed

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