The Implementation for Bring Your Own Local Model (BYOM) API by selenayang888 · Pull Request #928 · microsoft/foundry-local

Selena Yang (selenayang888) · 2026-07-30T08:58:47Z

The implementation to register a local ONNX model from an arbitrary filesystem path and make it available through the standard Catalog/Model/Inference api.

vercel · 2026-07-30T08:58:53Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
foundry-local	Ready	Preview	Aug 14, 2026 1:44am

Introduce ABI v3 with Info_Clone while preserving v1/v2 tables, restore independent C++ ModelInfo copy construction and assignment, and keep explicit CPU model loading on OGA's default provider. Add C ABI and C++ copy-semantics tests.

Copilot

Pull request overview

Adds persistent registration and inference support for local ONNX models through new local catalog and C/C++ APIs.

Changes:

Adds local model registration, persistence, metadata, and lifecycle handling.
Extends versioned C/C++ APIs with catalog selection and mutable ModelInfo.
Adds unit tests and separates legacy cached models from explicitly registered BYOM models.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`sdk_v2/cpp/test/internal_api/model_info_test.cc`	Tests property-bag persistence.
`sdk_v2/cpp/test/internal_api/model_info_accessors_test.cc`	Tests `ModelInfo` copy semantics.
`sdk_v2/cpp/test/internal_api/local_model_catalog_test.cc`	Tests local catalog behavior.
`sdk_v2/cpp/test/internal_api/c_api_test.cc`	Tests versioned cloning API.
`sdk_v2/cpp/test/internal_api/azure_catalog_test.cc`	Updates unresolved-cache expectations.
`sdk_v2/cpp/test/CMakeLists.txt`	Adds local catalog tests.
`sdk_v2/cpp/src/model.h`	Adds local-model lifecycle state.
`sdk_v2/cpp/src/model.cc`	Implements local loading and unregistering.
`sdk_v2/cpp/src/model_info.h`	Declares property-bag utilities.
`sdk_v2/cpp/src/model_info.cc`	Implements property persistence.
`sdk_v2/cpp/src/manager.h`	Adds multiple catalog support.
`sdk_v2/cpp/src/manager.cc`	Creates public and local catalogs.
`sdk_v2/cpp/src/inferencing/session/session.cc`	Uses collision-safe runtime IDs.
`sdk_v2/cpp/src/inferencing/generative/genai_model_instance.cc`	Revises provider overrides.
`sdk_v2/cpp/src/catalog/local_model_catalog.h`	Defines the local catalog.
`sdk_v2/cpp/src/catalog/local_model_catalog.cc`	Implements registration persistence.
`sdk_v2/cpp/src/catalog/catalog_client.h`	Documents explicit BYOM registration.
`sdk_v2/cpp/src/catalog/catalog_client.cc`	Removes synthesized BYOM entries.
`sdk_v2/cpp/src/catalog/base_model_catalog.h`	Adds catalog types and tombstones.
`sdk_v2/cpp/src/catalog/base_model_catalog.cc`	Implements model deactivation.
`sdk_v2/cpp/src/catalog/azure_model_catalog.cc`	Filters legacy local entries.
`sdk_v2/cpp/src/catalog.h`	Extends the catalog interface.
`sdk_v2/cpp/src/c_api.cc`	Implements versioned BYOM C APIs.
`sdk_v2/cpp/include/foundry_local/foundry_local_cpp.inline.h`	Implements C++ wrappers.
`sdk_v2/cpp/include/foundry_local/foundry_local_cpp.h`	Exposes new C++ APIs.
`sdk_v2/cpp/include/foundry_local/foundry_local_c.h`	Defines C API v3.
`sdk_v2/cpp/CMakeLists.txt`	Builds the local catalog source.

…model

Copilot

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

sdk_v2/cpp/src/model.cc:351

For a deferred registration whose directory/config is still missing, this reports 100% and succeeds even though IsCached() remains false. That breaks the usual Download() contract and lets callers proceed as if the model were available. Check availability and return an error until the externally managed assets exist.

  if (external_registration_) {
    if (progress_cb) {
      progress_cb(100.0f);
    }
    return;

sdk_v2/cpp/src/catalog/base_model_catalog.cc:66

On refresh this only appends aliases absent from the current active set; it never deactivates entries removed from the fetched source. Because the local registration index is cross-process and file-locked, an unregister/re-register in another process remains stale here forever (even after the four-hour refresh). Reconcile local entries by stable registration ID, tombstoning missing/replaced registrations while preserving pointer safety.

        models_.push_back({std::make_unique<Model>(std::move(model)), true});

sdk_v2/cpp/src/catalog/local_model_catalog.cc:476

The registration index is documented as authoritative, yet failure to create this optional sidecar aborts registration. Consequently, a valid model on a read-only/shared filesystem cannot be registered from its arbitrary path. Make sidecar creation best-effort (with a warning) or store generated metadata under app data instead.

    std::ofstream stream(temp_path, std::ios::binary | std::ios::trunc);
    if (!stream) {
      FL_THROW(FOUNDRY_LOCAL_ERROR_INTERNAL,
               "failed to write model_metadata.yml beside BYOM assets: " + registration.model_path);

sdk_v2/cpp/src/c_api.cc:763

The new public C entry points are not exercised by c_api_test.cc; the added catalog tests call LocalModelCatalog directly. Add an end-to-end C API test that obtains the local catalog through the versioned manager table, creates metadata, registers/lists/unregisters a model, and verifies handle lifetime and asset preservation.

  *out_model = AsHandle<flModel>(catalog->impl.RegisterModel(*AsImpl(model_info)));

Nat Kershaw (MSFT) (natke)

Confirming that the SDKs also need adding to this PR

Nat Kershaw (MSFT) (natke) · 2026-08-12T20:03:08Z

+  virtual std::unique_ptr<IModel> RegisterModel(const ModelInfo&) {
+    throw Error("models can only be registered in a local catalog", FOUNDRY_LOCAL_ERROR_INVALID_ARGUMENT);
+  }
+  virtual void UnregisterModel(const std::string&) {


Suggest "Deregister"

Nat Kershaw (MSFT) (natke) · 2026-08-12T20:22:30Z

+      }
+      // Keep the default provider in genai_config.json authoritative when the caller did not supply one. Some OGA
+      // providers such as DML are not represented by the SDK's explicit ExecutionProvider enum and use kDefault.
+      std::string task = "chat-completion";


This will overwrite user supplied metadata?

We need to define a set of known task values, and the user needs to specify. The task is used to determine which session types are valid for the model, so needs to match the model.

Baiju Meswani (baijumeswani)

Will add more comments soon.

Baiju Meswani (baijumeswani) · 2026-08-12T20:43:10Z

+  API_IMPL_END
+}
+
+static const flCatalogApi g_catalog_api_v1 = {


Why do we need g_catalog_api_v1? The API changes are purely additive and do not break ABI. So we should be ok just expanding g_catalog_api?

Agree. Should be exceedingly rare that we create a copy of an existing API to expand.

Baiju Meswani (baijumeswani) · 2026-08-12T20:44:15Z

+  API_IMPL_END
+}
+
+static const flModelApi g_model_api_v1 = {


Same here. The api changes should be purely additive. We should not need to define g_model_api_v1.

Baiju Meswani (baijumeswani) · 2026-08-12T20:45:06Z

    Manager_IsShutdownRequestedImpl,
+  };
+
+  static const flApi g_api_v2 = {


Same here. We do not need this. Any reason we need a new g_api_v2?

Scott McKay (skottmckay) · 2026-08-12T23:54:00Z

+#define FOUNDRY_LOCAL_REG_MODEL_PATH "model_path"
+#define FOUNDRY_LOCAL_REG_ALIAS "alias"


Should these two values be provided via metadata properties or directly to the RegisterModel call?

Instead of 'alias' should the user provide 'id' in format <alias>:<version>? Simplifies the input and maintains consistency with our catalog entries where id must be unique and in that format.

Scott McKay (skottmckay) · 2026-08-12T23:56:50Z

 #define FOUNDRY_LOCAL_MODEL_PROP_IS_TEST_MODEL_INT "is_test_model"                  ///< bool (0=false, 1=true)
 #define FOUNDRY_LOCAL_MODEL_PROP_CONTEXT_LENGTH_INT "context_length"                ///< optional int64_t
+#define FOUNDRY_LOCAL_MODEL_PROP_VERSION_INT "version"                              ///< optional non-negative integer
+#define FOUNDRY_LOCAL_MODEL_PROP_FILESIZE_BYTES_INT "file_size_bytes"                ///< optional int64_t


Why bytes if the 'real' catalog property is fileSizeMb?

Scott McKay (skottmckay) · 2026-08-13T00:02:01Z

+  FL_API_STATUS(Manager_GetCatalogByType, _In_ const flManager* manager, flCatalogType catalog_type,
+                _Outptr_ flCatalog** out_catalog);
+  FL_API_STATUS(Manager_GetCatalogByName, _In_ const flManager* manager, _In_ const char* catalog_name,
+                _Outptr_ flCatalog** out_catalog);


Is this something the user needs to know about?

Internally we could have a local catalog that we automatically use for RegisterModel, but that seems like it could stay an implementation detail and there's just a single user facing flCatalog.

Ideally the internals of FL aggregate across the various catalogs available (public/private/local) and the user doesn't need to care about that detail. V1 had a catalog aggregator class that I didn't port across as it hadn't been used yet. We could potentially add that concept back in.

I slightly prefer having separate catalogs per type (public, local, private - if that materializes). Because aggregate views can cause id/name conflicts and it becomes harder for consumers to tell which catalog a particular model came from. We then would need to expose some other model attributes or query-able catalog arguments.

Separate catalogs at the public API level adds overhead to the SDK user as they can't simply deal with a unified meta-catalog. Do we think that cost is outweighed by the benefit of manually being able to resolve a clash with the model id if it occurred and a default priority of local > private > cloud was used?

I could understand adding a metadata entry to the model for the catalog type. Low cost, ignored by vast majority of SDK users, available if needed using existing API.

Thinking through potential clashes:

local with cloud

local is higher priority as manually added

if you later added that to a cloud repo and want to prefer the cloud version you can unregister the local model

local model instance will be the one you're getting by default, you can check the metadata for catalog type if needed (although model location is likely just as good a signal as we're not putting those in our cloud model download cache currently) to know it's a local model that can be unregistered

but equally the unregister can return an error saying 'not a local model' as internally we'd still have separate catalogs

private with public

private is higher priority

if you put something in your private catalog assumably it should be preferred over public

remove from private catalog if that's not the case

And I would expect an id clash to be an edge case so we should try not to complicate the API for that if we have other ways of handling it.

There may be scenarios where the application wants all three versions of the same model from {public, private, local} and present to their users 1 or all 3 for different scenarios.
There may also be a scenario where the application has a model in the private model catalog before we make it available in the public catalog. Which could then force application to change the name (if they wanted to use both).

Separate catalogs at the public API level adds overhead to the SDK user as they can't simply deal with a unified meta-catalog.

I like the idea of an aggregate catalog, but only as a view of the 3 distinct catalogs. Not as a catalog itself. For example, we could create a helper/function called manager.GetAggregateCatalogView() which would then aggregate the 3 catalogs and present them based on some priority strategy.

This was one of the problems I thought the aggregate catalog had before in the v1 core which got surfaced when we did the private model catalog development.

There is also the scenario where one could have multiple private catalogs. Then the aggregation would become more involved and conflict resolution may require configurable strategy.

Scott McKay (skottmckay) · 2026-08-13T00:02:35Z

+  /// Unregister by alias or model ID without deleting model assets.
+  FL_API_STATUS(UnregisterModel, _In_ flCatalog* catalog, _In_ const char* alias_or_model_id);
+  /// List models explicitly registered in this local catalog.
+  FL_API_STATUS(GetLocalModels, _In_ const flCatalog* catalog, _Outptr_ flModelList** out_models);


Is this needed? Does a user need to know/care that a model was added directly?

We generally want to keep the public API as minimal as possible. Anything we add here is permanent so it must have a clear requirement to justify expanding the API surface.

Scott McKay (skottmckay) · 2026-08-13T00:04:46Z

+  FL_API_STATUS(Info_SerializeToFile, _In_ const flModelInfo* info, _In_ const char* file_path);
+  FL_API_STATUS(Info_DeserializeFromFile, _In_ const char* file_path, _Outptr_ flModelInfo** out_info);
+  /// Create a caller-owned deep copy. Release it with ReleaseModelInfo.
+  FL_API_STATUS(Info_Clone, _In_ const flModelInfo* info, _Outptr_ flModelInfo** out_info);


Naively I would have expected the user adding a model to create the model info in memory and provide that in a RegisterModel call. Why does the user need to serialize/deserialize or clone?

Scott McKay (skottmckay) · 2026-08-13T00:51:22Z

+      if (existing.info.alias == *alias_value) {
+        FL_THROW(FOUNDRY_LOCAL_ERROR_INVALID_ARGUMENT,
+                 "a model with alias '" + *alias_value + "' is already registered");
+      }


Would have expected this to be based on id not alias.

Scott McKay (skottmckay) · 2026-08-13T00:52:06Z

+    registration = {ResolveMetadata(model_info, nullptr, model_path, *alias_value, &assets_inspected), model_path,
+            assets_inspected};


nit: run clang-format to fix alignment

Scott McKay (skottmckay) · 2026-08-13T01:00:10Z

+                                 registration.info.GetPropertyWithDefault(kRegistrationIdProperty, std::string{});
+                        }),
+                        registrations.end());
+    SaveRegistrations(registrations);


Feels like too much is entangled here. Do we want to remove the registration if the activate fails or just if the CreateModel fails?

Or neither? if we validated the registration info before saving originally wouldn't it only be a runtime error that is likely transient here so automated unregistration isn't desirable?

Scott McKay (skottmckay) · 2026-08-13T01:02:04Z

+            current->Info().GetPropertyWithDefault(kRegistrationIdProperty, std::string{}) != registration_id) {
+          FL_THROW(FOUNDRY_LOCAL_ERROR_INVALID_USAGE, "model is no longer registered");
+        }
+        const_cast<LocalModelCatalog*>(this)->UnregisterModel(model_id);


const_cast is a design smell. Why are we calling UnregisterModel in CreateModel?

I see now it's in an unregister delegate. Can we split those out. The intention of the model factory is to be able to create a Model from the catalog info. unregister seems like a separate concern that could be untangled from a model creation factory.

Scott McKay (skottmckay) · 2026-08-13T01:03:51Z

+  using ModelFactory = std::function<Model(ModelInfo, std::string, std::function<void(const std::string&)>,
+                                           std::function<std::optional<ModelInfo>()>)>;


Define the 2 function args with separate using statements to provide some context as to what they represent via the name. Would also be helpful to include the arg names in the std::function usage as well. Whilst they're not strictly required it helps a user understand the function input.

Scott McKay (skottmckay) · 2026-08-13T07:42:00Z

+                                std::move(prepare_callback));
+      },
+      *logger_);
+  local_catalog_->ListModels();


do we need to call ListModels() here?

Scott McKay (skottmckay) · 2026-08-13T07:45:28Z

+            current->Info().GetPropertyWithDefault(kRegistrationIdProperty, std::string{}) != registration_id) {
+          FL_THROW(FOUNDRY_LOCAL_ERROR_INVALID_USAGE, "model is no longer registered");
+        }
+        const_cast<LocalModelCatalog*>(this)->UnregisterModel(model_id);


I see now it's in an unregister delegate. Can we split those out. The intention of the model factory is to be able to create a Model from the catalog info. unregister seems like a separate concern that could be untangled from a model creation factory.

Scott McKay (skottmckay) · 2026-08-13T07:55:05Z

+  mutable std::mutex metadata_mutex_;
+  mutable std::vector<std::unique_ptr<const ModelInfo>> info_snapshots_;
+  mutable std::atomic<const ModelInfo*> current_info_{nullptr};
  std::atomic<bool> cached_{false};
+  std::atomic<bool> active_{true};
  std::string local_path_;
+  std::string runtime_model_id_;
+  bool external_registration_ = false;
+  std::function<void(const std::string&)> unregister_callback_;
+  std::function<std::optional<ModelInfo>()> prepare_callback_;
+  mutable std::atomic<bool> metadata_prepared_{false};


This feels like a lot of complexity to be adding for BYOM which conceptually doesn't seem like it should be that different to the current model usage. If we require the model to be present at registration time does that simplify a lot of this?

Scott McKay (skottmckay) · 2026-08-13T07:56:07Z

+  /// Mark this model and its variants inactive while retaining pointer validity.
+  void Deactivate();
+  void BeginUnregister();
+  void CancelUnregister();


Do we need to support cancellation of unregister?

Scott McKay (skottmckay) · 2026-08-13T07:57:58Z

+                                   std::function<std::optional<ModelInfo>()> prepare_callback) {
+  auto model = FromModelInfo(std::move(info), std::move(local_path), download_manager, model_load_manager);
+  model.external_registration_ = true;
+  model.runtime_model_id_ = "local/" + model.Info().model_id;


do we need to treat this differently to any other model id? the more differences we have between how models from public/private/local are treated the more complicated the system is.

Scott McKay (skottmckay) · 2026-08-13T08:09:08Z

+  bool IsActive() const {
+    Model* selected = selected_variant_.load(std::memory_order_acquire);
+    return selected ? selected->IsActive() : active_.load();
+  }


I'm assuming deregistration to be rare so models will generally never become inactive. Given that, is this providing much value or would it be sufficient to document that unregistering a model potentially invalidates a model list the user has?

Scott McKay (skottmckay) · 2026-08-13T08:11:23Z

+void SetModelInfoStringProperty(ModelInfo& info, std::string key, std::string value);
+void SetModelInfoIntProperty(ModelInfo& info, std::string key, int64_t value);


Can these be methods like the existing getters?

Scott McKay (skottmckay) · 2026-08-13T08:24:13Z

+      {"alias", registration.info.alias},
+      {"model_path", registration.model_path},
+      {"registered_at", registration.info.GetPropertyWithDefault(FOUNDRY_LOCAL_MODEL_PROP_CREATION_TIME_STR, {})},
+      {"properties", ModelInfoToPropertyBagJson(registration.info)},


can we serialize the ModelInfo as is (we serialize ModelInfo instances to model_info.json I believe) so we avoid going back to an untyped property bag?

Scott McKay (skottmckay) · 2026-08-13T08:26:06Z

+  void SerializeToFile(const std::string& file_path) const;
+  static ModelInfo DeserializeFromFile(const std::string& file_path);


Why does a user need to de/serialize the model info?

Scott McKay (skottmckay) · 2026-08-13T08:31:22Z

+/// Returns whether the provider is DirectML, which is supplied by WinML rather than a downloadable EP bootstrapper.
+bool IsDmlProvider(std::string_view provider) {
+  return provider == "dml" || provider == "DML" || provider == "DmlExecutionProvider" ||
+         provider == "DMLExecutionProvider";
+}


Is this related to the PR?

Baiju Meswani (baijumeswani) · 2026-08-13T18:58:43Z

+  FL_API_STATUS(Manager_GetCatalogByType, _In_ const flManager* manager, flCatalogType catalog_type,
+                _Outptr_ flCatalog** out_catalog);
+  FL_API_STATUS(Manager_GetCatalogByName, _In_ const flManager* manager, _In_ const char* catalog_name,
+                _Outptr_ flCatalog** out_catalog);


I slightly prefer having separate catalogs per type (public, local, private - if that materializes). Because aggregate views can cause id/name conflicts and it becomes harder for consumers to tell which catalog a particular model came from. We then would need to expose some other model attributes or query-able catalog arguments.

Baiju Meswani (baijumeswani) · 2026-08-13T20:52:13Z

-  const flModelInfo* info_;
+  detail::Base<flModelInfo> handle_;
+
+  friend class Catalog;


Why does Catalog need to be a friend of ModelInfo?

Baiju Meswani (baijumeswani) · 2026-08-13T20:54:01Z

+  virtual void UnregisterModel(const std::string&) {
+    throw Error("models can only be unregistered from a local catalog", FOUNDRY_LOCAL_ERROR_INVALID_ARGUMENT);
+  }
+  virtual ModelList GetLocalModels() const {


Why do we need GetLocalModels when we have GetModels? Assuming local and public catalogs are different. GetModels on a local catalog should return the local models.

Baiju Meswani (baijumeswani) · 2026-08-13T20:56:31Z

  detail::Base<flManager> handle_;
  Configuration config_;
  mutable std::unique_ptr<Catalog> catalog_;
+  mutable std::unique_ptr<Catalog> local_catalog_;


Should we define a struct like:

struct CatalogCollection { mutable std::unique_ptr<Catalog> public_; mutable std::unique_ptr<Catalog> local_; } catalogs_;

And then access them using catalogs_.public_ and catalogs_.local_?

…model

the implementation for BYOM

29a1bad

Add deep-copy semantics for ModelInfo

15e1dde

Introduce ABI v3 with Info_Clone while preserving v1/v2 tables, restore independent C++ ModelInfo copy construction and assignment, and keep explicit CPU model loading on OGA's default provider. Add C ABI and C++ copy-semantics tests.

vercel Bot deployed to Preview August 6, 2026 01:21 View deployment

Selena Yang (selenayang888) marked this pull request as ready for review August 6, 2026 01:41

Copilot AI balanced review requested due to automatic review settings August 6, 2026 01:41

Copilot started reviewing on behalf of Selena Yang (selenayang888) August 6, 2026 01:42 View session

Copilot AI reviewed Aug 6, 2026

View reviewed changes

Comment thread sdk_v2/cpp/src/catalog/local_model_catalog.cc Outdated

Comment thread sdk_v2/cpp/src/inferencing/generative/genai_model_instance.cc Outdated

Resolve conflicts

f1cb59a

vercel Bot deployed to Preview August 6, 2026 08:02 View deployment

Selena Yang (selenayang888) added 2 commits August 7, 2026 16:24

Fix the comment about "Deferred registrations"

c1f1b8b

fixed execution-provider override bug

0c6a729

vercel Bot deployed to Preview August 7, 2026 23:38 View deployment

Merge remote-tracking branch 'origin/main' into syang/bring-your-own-…

a2dd9b2

…model

Selena Yang (selenayang888) requested review from Baiju Meswani (baijumeswani) and Scott McKay (skottmckay) August 8, 2026 06:00

vercel Bot deployed to Preview August 8, 2026 06:00 View deployment