Feat: add model registry#157
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new read-only model registry capability to tracel-core, exposing model metadata and artifact download functionality via Context::model_registry() and backend-specific ModelRegistryProvider implementations.
Changes:
- Introduces
model_registrymodule with DTOs (ModelInfo,ModelVersionInfo),ModelRegistryModule, provider trait, and error types, plus unit tests for module behavior. - Implements
ModelRegistryProviderforCloudBackendand (feature-gated)StationBackend, including checksum/size verification in Station download plans. - Refactors repeated
ArtifactDownloadFileconstruction into shared helpers (download_file.rs) and removes a deadcreate_cloud_experiment_runre-export.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/tracel-core/src/model_registry/mod.rs | Adds core model registry module API, errors, download flow, and unit tests. |
| crates/tracel-core/src/model_registry/cloud.rs | Implements model registry provider for cloud backend. |
| crates/tracel-core/src/model_registry/station.rs | Implements model registry provider for station backend (with verification fields). |
| crates/tracel-core/src/context.rs | Wires model registry access into Context via an optional provider/module. |
| crates/tracel-core/src/connection.rs | Extends provider wiring to supply model registry providers for Cloud/Station and None for Offline. |
| crates/tracel-core/src/download_file.rs | Adds helper constructors for ArtifactDownloadFile (with/without verification). |
| crates/tracel-core/src/lib.rs | Registers new modules and re-exports model-registry public types. |
| crates/tracel-core/src/experiment/remote/cloud/mod.rs | Switches artifact download file construction to shared helper; removes dead create_cloud_experiment_run. |
| crates/tracel-core/src/experiment/remote/station/mod.rs | Switches artifact download file construction to shared helper. |
| crates/tracel-core/src/experiment/mod.rs | Removes stale runtime re-export. |
| crates/tracel-artifact/src/lib.rs | Re-exports TransferError from tracel-artifact. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
…i/tracel into feat/add_model_registry
jwric
reviewed
Jul 7, 2026
| sha2.workspace = true | ||
|
|
||
| [dev-dependencies] | ||
| tempfile = "3.27.0" |
Member
There was a problem hiding this comment.
Add to workspace dependencies and reuse the same one as in tracel-artifacts
jwric
reviewed
Jul 7, 2026
Member
There was a problem hiding this comment.
Odd module choice, inline the construction of these structs where used
jwric
reviewed
Jul 7, 2026
| #[error("version {version} of model '{name}' not found")] | ||
| VersionNotFound { name: String, version: u32 }, | ||
| #[error("communication with the model registry failed: {0}")] | ||
| Client(#[from] ClientError), |
Member
There was a problem hiding this comment.
Client error not supposed to surface to the user api, box dyn error this
jwric
reviewed
Jul 7, 2026
Comment on lines
+36
to
+56
| pub(crate) fn map_not_found( | ||
| err: ClientError, | ||
| not_found: impl FnOnce() -> ModelRegistryError, | ||
| ) -> ModelRegistryError { | ||
| match err { | ||
| ClientError::NotFound => not_found(), | ||
| other => other.into(), | ||
| } | ||
| } | ||
|
|
||
| /// Runs a client call purely to check existence, discarding its response and mapping a | ||
| /// not-found result through `not_found`. Shared by every [`ModelRegistryProvider`] | ||
| /// implementation's `ensure_model_exists`/`ensure_version_exists` checks. | ||
| pub(crate) fn ensure_exists<T>( | ||
| result: Result<T, ClientError>, | ||
| not_found: impl FnOnce() -> ModelRegistryError, | ||
| ) -> Result<(), ModelRegistryError> { | ||
| result | ||
| .map(|_| ()) | ||
| .map_err(|err| map_not_found(err, not_found)) | ||
| } |
Member
There was a problem hiding this comment.
This is over functionification, its just confusing
jwric
reviewed
Jul 8, 2026
Comment on lines
+28
to
+30
| .join("models") | ||
| .join("station") | ||
| .join(station_id); |
Member
There was a problem hiding this comment.
the order should be: station -> station_id -> models
jwric
approved these changes
Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add read-only model registry module
Summary:
Adds a
ModelRegistryProvidertrait andModelRegistryModuletotracel-core, exposing read-only model registry access (load) throughContext::models(). Implemented for bothCloudBackendandStationBackend(station-only verifieschecksum/sizeon download); returnsNonefor the offline/local backend. Includes unit tests forModelRegistryModuleand a small refactor extracting ArtifactDownloadFile construction into shared helpers indownload_file.rs. It also includes a local cache.Changes:
model_registrymodule: trait, module, DTOs, errorsModelCachefeature = "station")get/version/download_tocreate_cloud_experiment_runre-export (used in tracel-runtime crate that is now deleted), dedupedArtifactDownloadFileconstruction