Bring Your Own Model Language Bindings for C#, JavaScript, Python, and Rust SDKs - #1062
Selena Yang (selenayang888) wants to merge 24 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
🟡 Changes recommended
Borrowed Python metadata can outlive its native manager, C# metadata copying races caller mutation, and public API compatibility and Rust coverage need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Extends SDK v2 BYOM support across C#, JavaScript/TypeScript, Python, and Rust.
Changes:
- Adds local catalog selection and model registration APIs.
- Adds mutable metadata builders and ABI v2 bindings.
- Adds BYOM documentation and integration/API tests.
File summaries
| File | Description |
|---|---|
sdk_v2/rust/tests/public_api.rs |
Checks public BYOM API types. |
sdk_v2/rust/src/types.rs |
Adds catalog types and metadata builder. |
sdk_v2/rust/src/lib.rs |
Exports new Rust types. |
sdk_v2/rust/src/foundry_local_manager.rs |
Adds typed catalog access. |
sdk_v2/rust/src/detail/native.rs |
Implements native registration calls. |
sdk_v2/rust/src/detail/manager.rs |
Retrieves catalogs by type. |
sdk_v2/rust/src/detail/ffi.rs |
Adds ABI v2 definitions. |
sdk_v2/rust/src/catalog.rs |
Exposes BYOM operations. |
sdk_v2/rust/README.md |
Documents Rust BYOM usage. |
sdk_v2/python/test/unit/test_imports.py |
Verifies catalog type export. |
sdk_v2/python/test/unit/test_catalog_types.py |
Tests catalog enum defaults. |
sdk_v2/python/test/integration/test_byom.py |
Exercises Python BYOM behavior. |
sdk_v2/python/src/foundry_local_sdk/model_info.py |
Adds native mutable and borrowed metadata. |
sdk_v2/python/src/foundry_local_sdk/imodel.py |
Returns borrowed metadata views. |
sdk_v2/python/src/foundry_local_sdk/foundry_local_manager.py |
Adds typed catalog caching. |
sdk_v2/python/src/foundry_local_sdk/catalog.py |
Adds registration operations. |
sdk_v2/python/src/foundry_local_sdk/__init__.py |
Exports CatalogType. |
sdk_v2/python/README.md |
Documents Python BYOM usage. |
sdk_v2/js/test/catalog.test.ts |
Updates native catalog mocks. |
sdk_v2/js/test/byom.test.ts |
Tests JavaScript BYOM APIs. |
sdk_v2/js/src/types.ts |
Adds CatalogType. |
sdk_v2/js/src/modelInfo.ts |
Adds mutable metadata wrapper. |
sdk_v2/js/src/index.ts |
Exports BYOM types and constructors. |
sdk_v2/js/src/foundryLocalManager.ts |
Adds typed catalog caching. |
sdk_v2/js/src/detail/native.ts |
Extends native interfaces. |
sdk_v2/js/src/catalog.ts |
Adds sync and async BYOM methods. |
sdk_v2/js/README.md |
Documents JavaScript BYOM usage. |
sdk_v2/js/native/src/session.h |
Retains manager lifetimes. |
sdk_v2/js/native/src/session.cc |
Propagates manager ownership. |
sdk_v2/js/native/src/model.h |
Adds native manager keepalive. |
sdk_v2/js/native/src/model.cc |
Propagates model keepalives. |
sdk_v2/js/native/src/model_info.h |
Declares native metadata wrapper. |
sdk_v2/js/native/src/model_info.cc |
Implements metadata mutation and snapshots. |
sdk_v2/js/native/src/manager.h |
Uses shared manager ownership. |
sdk_v2/js/native/src/manager.cc |
Adds typed catalog retrieval. |
sdk_v2/js/native/src/catalog.h |
Declares native BYOM methods. |
sdk_v2/js/native/src/catalog.cc |
Implements native BYOM dispatch. |
sdk_v2/js/native/src/addon.cc |
Registers metadata constructor. |
sdk_v2/js/native/src/addon_data.h |
Stores metadata constructor reference. |
sdk_v2/js/docs/PortJsToSdkV2.md |
Marks BYOM implemented. |
sdk_v2/js/binding.gyp |
Builds metadata source. |
sdk_v2/cs/test/FoundryLocal.Tests/CatalogTests.cs |
Tests C# BYOM lifecycle. |
sdk_v2/cs/src/ICatalog.cs |
Adds registration contracts. |
sdk_v2/cs/src/FoundryModelInfo.cs |
Adds mutable metadata properties. |
sdk_v2/cs/src/FoundryLocalManager.cs |
Adds typed catalog access. |
sdk_v2/cs/src/Enums.cs |
Adds CatalogType. |
sdk_v2/cs/src/Detail/NativeMethods.cs |
Adds ABI v2 interop declarations. |
sdk_v2/cs/src/Detail/FoundryLocalApi.cs |
Implements native BYOM wrappers. |
sdk_v2/cs/src/Catalog.cs |
Implements C# registration operations. |
sdk_v2/cs/README.md |
Documents C# BYOM usage. |
Review details
- Files reviewed: 50/50 changed files
- Comments generated: 5
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Baiju Meswani (baijumeswani)
left a comment
There was a problem hiding this comment.
Thanks for adding BYOM support across the language SDKs. The basic path looks good, and the PR checks are green. I also built the native SDK and ran the focused tests locally: JS 34/34 passed, Python 17 passed with 4 environment skips, and the Rust BYOM round trip passed. I found a few cases that are not covered by those tests. The most important one is a native crash when a JS catalog is used after the manager is disposed. Python also loses custom metadata after the same model is fetched from the catalog again. I do not think this is ready for approval until the lifetime and metadata round-trip issues are fixed. I left simple inline comments with reproduction details and suggested tests.
Baiju Meswani (baijumeswani)
left a comment
There was a problem hiding this comment.
Fresh review against the current head, focused on the BYOM lifecycle and metadata issues found during qualification. The earlier JavaScript catalog-disposal and Python metadata-persistence concerns appear addressed. The inline comments below cover the remaining lifetime, API-semantics, metadata-parity, and FFI-boundary concerns.
Baiju Meswani (baijumeswani)
left a comment
There was a problem hiding this comment.
Thanks for addressing the earlier feedback. I found two remaining JavaScript lifetime gaps. Both can leave native work using a manager after dispose().
Baiju Meswani (baijumeswani)
left a comment
There was a problem hiding this comment.
Thanks for addressing the earlier manager lifetime comments. The manager lease and EP download ownership changes are good. I found two remaining session lifetime issues that should be fixed before merge.
Extends the Bring Your Own Model (BYOM) API introduced in BYOM to the C#, JavaScript/TypeScript, Python, and Rust SDK v2 bindings.
Applications can select the local catalog, register an existing local ONNX model directory, use the returned model through the standard inference APIs, and unregister it without deleting or taking ownership of the model assets.