Pin executable Hugging Face model downloads to revisions#1688
Conversation
|
CLA Assistant Lite bot CLA Assistant bot All Contributors have signed the CLA. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request pins several Hugging Face model URLs to specific commit hashes instead of using the mutable 'main' branch, ensuring stability and security. It also introduces a regression test to verify that executable Hugging Face models are pinned. The reviewer suggested using reflection in the test to scan all public static properties of RemoteModels for downloadable resources, which would make the test comprehensive and future-proof against any new model collections.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| using StabilityMatrix.Core.Helper; | ||
|
|
||
| namespace StabilityMatrix.Tests.Core; | ||
|
|
||
| public class RemoteModelsTests | ||
| { | ||
| [Fact] | ||
| public void ExecutableHuggingFaceModelsUsePinnedRevisions() | ||
| { | ||
| var executableExtensions = new HashSet<string>(StringComparer.OrdinalIgnoreCase) | ||
| { | ||
| ".bin", | ||
| ".ckpt", | ||
| ".pt", | ||
| ".pth", | ||
| }; | ||
|
|
||
| var unpinnedModels = RemoteModels | ||
| .UltralyticsModelFiles | ||
| .Where(model => model.DownloadableResource?.Url.Host == "huggingface.co") | ||
| .Where(model => executableExtensions.Contains(Path.GetExtension(model.DownloadableResource!.Value.Url.AbsolutePath))) | ||
| .Where(model => model.DownloadableResource!.Value.Url.AbsolutePath.Contains("/resolve/main/", StringComparison.Ordinal)) | ||
| .Select(model => model.DownloadableResource!.Value.Url) | ||
| .ToArray(); | ||
|
|
||
| Assert.Empty(unpinnedModels); | ||
| } |
There was a problem hiding this comment.
The current regression test only validates RemoteModels.UltralyticsModelFiles. To make this test comprehensive and future-proof against any new model collections or properties added to RemoteModels, we can use reflection to scan all public static properties for downloadable resources (RemoteResource and HybridModelFile). This ensures that any executable Hugging Face models added anywhere in RemoteModels in the future will automatically be validated.
using System.Reflection;
using StabilityMatrix.Core.Helper;
using StabilityMatrix.Core.Models;
namespace StabilityMatrix.Tests.Core;
public class RemoteModelsTests
{
[Fact]
public void ExecutableHuggingFaceModelsUsePinnedRevisions()
{
var executableExtensions = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
{
".bin",
".ckpt",
".pt",
".pth",
};
var properties = typeof(RemoteModels).GetProperties(BindingFlags.Public | BindingFlags.Static);
var resources = new List<RemoteResource>();
foreach (var prop in properties)
{
var value = prop.GetValue(null);
if (value is IEnumerable<RemoteResource> remoteResources)
{
resources.AddRange(remoteResources);
}
else if (value is IEnumerable<HybridModelFile> hybridModels)
{
resources.AddRange(hybridModels
.Where(m => m.DownloadableResource.HasValue)
.Select(m => m.DownloadableResource!.Value));
}
else if (value is HybridModelFile singleHybrid && singleHybrid.DownloadableResource.HasValue)
{
resources.Add(singleHybrid.DownloadableResource.Value);
}
}
var unpinnedModels = resources
.Where(resource => resource.Url.Host == "huggingface.co")
.Where(resource => executableExtensions.Contains(Path.GetExtension(resource.Url.AbsolutePath)))
.Where(resource => resource.Url.AbsolutePath.Contains("/resolve/main/", StringComparison.Ordinal))
.Select(resource => resource.Url)
.ToArray();
Assert.Empty(unpinnedModels);
}There was a problem hiding this comment.
I have read the CLA Document and I hereby sign the CLA
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
|
Hi @ungrav, thanks for the submission! Seems reasonable, though we use MSTest in the |
|
|
||
| public class RemoteModelsTests | ||
| { | ||
| [Fact] |
There was a problem hiding this comment.
Hi, I'm not sure this PR needs tests exactly, but you will need to change to MSTest format, we're not using xunit and this isn't building right now.
3 participants
Summary
.ptdownloads to the exact Hugging Face commit that contains the already-declared SHA-256 objectsresolve/mainURLs for executable Hugging Face model formats in the built-in Ultralytics model listWhy
PyTorch checkpoint formats are executable deserialization surfaces. Stability Matrix already verifies SHA-256 values, which prevents a changed object from being accepted, but resolving a mutable branch can still turn a routine download into a failure after upstream changes. Pinning the revision makes provenance and availability deterministic and matches the pattern already used by built-in upscaler and ControlNet resources.
The selected revision is
b0a075fd35454c86bb453a1ca06b29ffee704c20. Its four Git LFS pointers match the SHA-256 values already present inRemoteModels.cs.Validation
HashSha256valuesresolve/maingit diff --checkA local .NET SDK was unavailable, so the focused xUnit test was not executed locally; CI should provide the compile/runtime check.