mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-09-21 03:01:27 -03:00
fix(modelscope): identify a model file by hash before filename
A file was matched to its published version by comparing basenames against each version's `stats.fileList`. Renaming the weights — routine once a model is filed away, and the reason the scanner records a sha256 at all — made the match fail silently, so the file lost its example images and its preview with no indication why. The detail payload's `ModelInfos.safetensor.files[]` carries a real sha256 per published file, and the local hash is already on disk, so match on that first: it is the one identifier a rename cannot invalidate. Exact basename and `showName` matching remain as fallbacks, and an unknown hash falls through to them rather than giving up, so a re-encoded file still resolves. Verified against the live repository: a renamed `c1-st1000` file with its hash yields the c1-st1000 image, the same rename without a hash yields nothing, and supplying c1-st2000's hash resolves to the c1-st2000 image even when the filename claims otherwise.
This commit is contained in:
@@ -104,10 +104,12 @@ boilerplate README and put everything useful in `Description`, so reading only
|
|||||||
the README yields almost nothing.
|
the README yields almost nothing.
|
||||||
|
|
||||||
Providers opt in by overriding `ModelSource.fetch_model_card_context()`, which
|
Providers opt in by overriding `ModelSource.fetch_model_card_context()`, which
|
||||||
returns a `ModelCardContext`. Example images are matched to the model's
|
returns a `ModelCardContext`. The wanted file is identified by its sha256 when
|
||||||
**basename**, so each checkpoint in a collection repo gets its own images.
|
the caller knows it (the scanner already records one) and by **basename**
|
||||||
Sites with no such extras inherit an empty context, and the pipeline behaves
|
otherwise, so each checkpoint in a collection repo gets its own images — and
|
||||||
exactly as before.
|
keeps getting them after the user renames the weights, which is the only
|
||||||
|
identifier a rename cannot invalidate. Sites with no such extras inherit an
|
||||||
|
empty context, and the pipeline behaves exactly as before.
|
||||||
|
|
||||||
The README and the repository metadata describe the whole repository, not one
|
The README and the repository metadata describe the whole repository, not one
|
||||||
file, so `execute_skill()` creates a `ModelSourceCache` for the duration of a
|
file, so `execute_skill()` creates a `ModelSourceCache` for the duration of a
|
||||||
|
|||||||
@@ -474,9 +474,13 @@ class AgentService:
|
|||||||
cache.readmes[cache_key] = readme
|
cache.readmes[cache_key] = readme
|
||||||
|
|
||||||
# Sites such as ModelScope keep part of the model card outside the
|
# Sites such as ModelScope keep part of the model card outside the
|
||||||
# README (author summary, curated tags, per-file example images).
|
# README (author summary, curated tags, per-file example images). The
|
||||||
|
# recorded hash identifies the file even after the user renames it.
|
||||||
card_context = await source.fetch_model_card_context(
|
card_context = await source.fetch_model_card_context(
|
||||||
ref.source_id, os.path.basename(model_path), cache=cache,
|
ref.source_id,
|
||||||
|
os.path.basename(model_path),
|
||||||
|
sha256=(metadata.get("sha256") or "").strip(),
|
||||||
|
cache=cache,
|
||||||
)
|
)
|
||||||
variables["source_description"] = card_context.description
|
variables["source_description"] = card_context.description
|
||||||
variables["source_base_model"] = card_context.base_model
|
variables["source_base_model"] = card_context.base_model
|
||||||
|
|||||||
@@ -307,19 +307,24 @@ class ModelSource:
|
|||||||
source_id: str,
|
source_id: str,
|
||||||
filename: str = "",
|
filename: str = "",
|
||||||
*,
|
*,
|
||||||
|
sha256: str = "",
|
||||||
cache: Optional["ModelSourceCache"] = None,
|
cache: Optional["ModelSourceCache"] = None,
|
||||||
) -> ModelCardContext:
|
) -> ModelCardContext:
|
||||||
"""Return the card extras the site keeps outside the README.
|
"""Return the card extras the site keeps outside the README.
|
||||||
|
|
||||||
*filename* is the model file's basename (no directory) and selects
|
*filename* is the model file's basename (no directory) and *sha256*
|
||||||
the right entry when a repository holds several models. Sites whose
|
its content hash; between them they select the right entry when a
|
||||||
model card is fully described by :meth:`fetch_model_card` need no
|
repository holds several models. A site that records per-file hashes
|
||||||
override and inherit this empty context.
|
should prefer *sha256*, because it is the only identifier that
|
||||||
|
survives the user renaming the weights.
|
||||||
|
|
||||||
*cache* is an optional per-run memo (see :class:`ModelSourceCache`)
|
*cache* is an optional per-run memo (see :class:`ModelSourceCache`)
|
||||||
that lets a provider avoid re-fetching repository-wide data for every
|
that lets a provider avoid re-fetching repository-wide data for every
|
||||||
file in a collection repository.
|
file in a collection repository.
|
||||||
|
|
||||||
|
Sites whose model card is fully described by :meth:`fetch_model_card`
|
||||||
|
need no override and inherit this empty context.
|
||||||
|
|
||||||
Implementations must never raise: enrichment treats a missing
|
Implementations must never raise: enrichment treats a missing
|
||||||
context as "the site had nothing extra to say".
|
context as "the site had nothing extra to say".
|
||||||
"""
|
"""
|
||||||
|
|||||||
@@ -117,6 +117,7 @@ class ModelScopeSource(ModelSource):
|
|||||||
source_id: str,
|
source_id: str,
|
||||||
filename: str = "",
|
filename: str = "",
|
||||||
*,
|
*,
|
||||||
|
sha256: str = "",
|
||||||
cache: Optional["ModelSourceCache"] = None,
|
cache: Optional["ModelSourceCache"] = None,
|
||||||
) -> ModelCardContext:
|
) -> ModelCardContext:
|
||||||
"""Read the model-detail API that backs the ModelScope model page.
|
"""Read the model-detail API that backs the ModelScope model page.
|
||||||
@@ -128,10 +129,11 @@ class ModelScopeSource(ModelSource):
|
|||||||
no further description") and put everything useful in ``Description``,
|
no further description") and put everything useful in ``Description``,
|
||||||
so enrichment that reads only the README comes back nearly empty.
|
so enrichment that reads only the README comes back nearly empty.
|
||||||
|
|
||||||
Example images are matched to *filename* through each version's
|
The wanted file is identified by its sha256 when the caller knows it
|
||||||
``stats.fileList``, which means the images returned belong to the
|
and by *filename* otherwise; see :func:`_matching_versions`. The
|
||||||
exact ``.safetensors`` being enriched — essential for collection
|
images and trigger words returned belong to that exact
|
||||||
repositories, where every checkpoint has its own sample image.
|
``.safetensors`` — essential for collection repositories, where every
|
||||||
|
checkpoint has its own sample image.
|
||||||
|
|
||||||
The detail payload describes the whole repository and is therefore
|
The detail payload describes the whole repository and is therefore
|
||||||
shared across every file in it, so it is read through *cache* when the
|
shared across every file in it, so it is read through *cache* when the
|
||||||
@@ -141,7 +143,7 @@ class ModelScopeSource(ModelSource):
|
|||||||
data = await self._fetch_detail(source_id, cache=cache)
|
data = await self._fetch_detail(source_id, cache=cache)
|
||||||
if data is None:
|
if data is None:
|
||||||
return ModelCardContext()
|
return ModelCardContext()
|
||||||
return _build_card_context(data, filename)
|
return _build_card_context(data, filename, sha256)
|
||||||
|
|
||||||
async def _fetch_detail(
|
async def _fetch_detail(
|
||||||
self,
|
self,
|
||||||
@@ -245,7 +247,9 @@ def _first_string(value: Any) -> str:
|
|||||||
return ""
|
return ""
|
||||||
|
|
||||||
|
|
||||||
def _build_card_context(data: dict[str, Any], filename: str) -> ModelCardContext:
|
def _build_card_context(
|
||||||
|
data: dict[str, Any], filename: str, sha256: str = ""
|
||||||
|
) -> ModelCardContext:
|
||||||
"""Turn a model-detail payload into a :class:`ModelCardContext`.
|
"""Turn a model-detail payload into a :class:`ModelCardContext`.
|
||||||
|
|
||||||
Separated from the HTTP fetch so the repository-wide payload can be cached
|
Separated from the HTTP fetch so the repository-wide payload can be cached
|
||||||
@@ -260,7 +264,12 @@ def _build_card_context(data: dict[str, Any], filename: str) -> ModelCardContext
|
|||||||
official_tags=_official_tags(data.get("OfficialTags")),
|
official_tags=_official_tags(data.get("OfficialTags")),
|
||||||
)
|
)
|
||||||
|
|
||||||
versions = _matching_versions(data.get("MuseInfo"), filename)
|
versions = _matching_versions(
|
||||||
|
data.get("MuseInfo"),
|
||||||
|
filename,
|
||||||
|
digests=_file_digests(data),
|
||||||
|
sha256=sha256,
|
||||||
|
)
|
||||||
if versions:
|
if versions:
|
||||||
context.example_images = _cover_image_urls(versions)
|
context.example_images = _cover_image_urls(versions)
|
||||||
context.trigger_words = _version_trigger_words(versions)
|
context.trigger_words = _version_trigger_words(versions)
|
||||||
@@ -350,16 +359,56 @@ def _version_show_name(version: dict[str, Any]) -> str:
|
|||||||
return _clean_text(model_version.get("showName")).lower()
|
return _clean_text(model_version.get("showName")).lower()
|
||||||
|
|
||||||
|
|
||||||
def _matching_versions(muse_info: Any, filename: str) -> list[dict[str, Any]]:
|
def _file_digests(data: dict[str, Any]) -> dict[str, str]:
|
||||||
"""Return the ``versions`` entries that publish *filename*.
|
"""Return ``basename -> sha256`` for every published weight file.
|
||||||
|
|
||||||
Matching is by exact basename first, then by the version's ``showName``
|
``ModelInfos`` groups the repository's files by kind (``safetensor``,
|
||||||
appearing in the file stem (which absorbs the naming drift ModelScope
|
…) and records a real sha256 for each, which is what makes it possible to
|
||||||
sometimes applies to uploaded weights). All matches are returned so a
|
recognise a file the user has renamed.
|
||||||
file re-published across several versions contributes all of its
|
"""
|
||||||
example images. With no *filename* only an unambiguous single-version
|
|
||||||
repository is used, because a per-file image must never be attributed
|
digests: dict[str, str] = {}
|
||||||
to the wrong file.
|
model_infos = data.get("ModelInfos")
|
||||||
|
if not isinstance(model_infos, dict):
|
||||||
|
return digests
|
||||||
|
for info in model_infos.values():
|
||||||
|
files = info.get("files") if isinstance(info, dict) else None
|
||||||
|
if not isinstance(files, list):
|
||||||
|
continue
|
||||||
|
for entry in files:
|
||||||
|
if not isinstance(entry, dict):
|
||||||
|
continue
|
||||||
|
name = _clean_text(entry.get("name"))
|
||||||
|
digest = _clean_text(entry.get("sha256"))
|
||||||
|
if name and digest:
|
||||||
|
digests.setdefault(os.path.basename(name).lower(), digest.lower())
|
||||||
|
return digests
|
||||||
|
|
||||||
|
|
||||||
|
def _matching_versions(
|
||||||
|
muse_info: Any,
|
||||||
|
filename: str,
|
||||||
|
*,
|
||||||
|
digests: dict[str, str] | None = None,
|
||||||
|
sha256: str = "",
|
||||||
|
) -> list[dict[str, Any]]:
|
||||||
|
"""Return the ``versions`` entries that publish the wanted model file.
|
||||||
|
|
||||||
|
Strategies, in order:
|
||||||
|
|
||||||
|
1. **sha256** — the file's content hash, looked up through
|
||||||
|
:func:`_file_digests`. This is the only strategy that survives the
|
||||||
|
user renaming the weights, which is common once a model is filed away.
|
||||||
|
2. **Exact basename** against each version's ``stats.fileList``.
|
||||||
|
3. **``showName`` inside the file stem**, which absorbs the naming drift
|
||||||
|
ModelScope sometimes applies to uploaded weights.
|
||||||
|
|
||||||
|
A known-but-unmatched hash falls through to the filename strategies
|
||||||
|
rather than giving up, in case the local file was re-encoded. All matches
|
||||||
|
are returned so a file re-published across several versions contributes
|
||||||
|
all of its example images. With no *filename* and no *sha256*, only an
|
||||||
|
unambiguous single-version repository is used, because a per-file image
|
||||||
|
must never be attributed to the wrong file.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
if not isinstance(muse_info, dict):
|
if not isinstance(muse_info, dict):
|
||||||
@@ -371,6 +420,18 @@ def _matching_versions(muse_info: Any, filename: str) -> list[dict[str, Any]]:
|
|||||||
if not entries:
|
if not entries:
|
||||||
return []
|
return []
|
||||||
|
|
||||||
|
target_hash = (sha256 or "").strip().lower()
|
||||||
|
if target_hash:
|
||||||
|
known = digests or {}
|
||||||
|
by_hash: list[dict[str, Any]] = []
|
||||||
|
for version in entries:
|
||||||
|
for path in _version_files(version):
|
||||||
|
if known.get(os.path.basename(path).lower()) == target_hash:
|
||||||
|
by_hash.append(version)
|
||||||
|
break
|
||||||
|
if by_hash:
|
||||||
|
return by_hash
|
||||||
|
|
||||||
if not filename:
|
if not filename:
|
||||||
return entries if len(entries) == 1 else []
|
return entries if len(entries) == 1 else []
|
||||||
|
|
||||||
|
|||||||
@@ -7,9 +7,12 @@ each provider's model-card fetching and capability flags.
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from unittest import mock
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
|
from py.services.agent.agent_service import AgentService
|
||||||
from py.services.model_sources import (
|
from py.services.model_sources import (
|
||||||
|
ModelCardContext,
|
||||||
ModelSourceCache,
|
ModelSourceCache,
|
||||||
HuggingFaceSource,
|
HuggingFaceSource,
|
||||||
ModelScopeSource,
|
ModelScopeSource,
|
||||||
@@ -381,6 +384,22 @@ def _modelscope_detail_payload() -> dict:
|
|||||||
},
|
},
|
||||||
]
|
]
|
||||||
},
|
},
|
||||||
|
"ModelInfos": {
|
||||||
|
"safetensor": {
|
||||||
|
"files": [
|
||||||
|
{
|
||||||
|
"name": "Krea-2-LORA_c1-st8000.safetensors",
|
||||||
|
"sha256": "a" * 64,
|
||||||
|
"size": 234680568,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"name": "Krea-2-LORA_c1-st1000.safetensors",
|
||||||
|
"sha256": "b" * 64,
|
||||||
|
"size": 234680568,
|
||||||
|
},
|
||||||
|
]
|
||||||
|
}
|
||||||
|
},
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -799,3 +818,141 @@ class TestCachedModelCardFetch:
|
|||||||
await source.fetch_model_card_context("u/r", "a.safetensors")
|
await source.fetch_model_card_context("u/r", "a.safetensors")
|
||||||
|
|
||||||
assert len(calls) == 2
|
assert len(calls) == 2
|
||||||
|
|
||||||
|
|
||||||
|
class TestHashBasedVersionMatching:
|
||||||
|
"""A renamed file must still find its own example images."""
|
||||||
|
|
||||||
|
ST1000_HASH = "b" * 64
|
||||||
|
ST8000_HASH = "a" * 64
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _patch(monkeypatch):
|
||||||
|
async def fake_fetch_json(url, **_kwargs):
|
||||||
|
return 200, _modelscope_detail_payload()
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"py.services.model_sources.modelscope.fetch_json", fake_fetch_json
|
||||||
|
)
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_renamed_file_is_matched_by_sha256(self, monkeypatch):
|
||||||
|
self._patch(monkeypatch)
|
||||||
|
|
||||||
|
context = await ModelScopeSource().fetch_model_card_context(
|
||||||
|
"u/r", "krea脸模-st1000-我改的名字.safetensors", sha256=self.ST1000_HASH
|
||||||
|
)
|
||||||
|
|
||||||
|
assert context.example_images == [
|
||||||
|
"https://resources.modelscope.cn/cover-images/b.png",
|
||||||
|
"https://resources.modelscope.cn/cover-images/c.png",
|
||||||
|
]
|
||||||
|
assert context.trigger_words == ["kreaface", "kreamodel"]
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_renamed_file_without_a_hash_finds_nothing(self, monkeypatch):
|
||||||
|
"""Pins the behaviour the hash match exists to fix."""
|
||||||
|
self._patch(monkeypatch)
|
||||||
|
|
||||||
|
context = await ModelScopeSource().fetch_model_card_context(
|
||||||
|
"u/r", "krea脸模-st1000-我改的名字.safetensors"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert context.example_images == []
|
||||||
|
# Repo-wide fields are unaffected by the miss.
|
||||||
|
assert context.base_model == "krea/Krea-2-Turbo"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_hash_wins_over_a_filename_that_matches_another_version(
|
||||||
|
self, monkeypatch
|
||||||
|
):
|
||||||
|
"""An inconsistent name/hash pair trusts the content hash."""
|
||||||
|
self._patch(monkeypatch)
|
||||||
|
|
||||||
|
context = await ModelScopeSource().fetch_model_card_context(
|
||||||
|
"u/r", "Krea-2-LORA_c1-st8000.safetensors", sha256=self.ST1000_HASH
|
||||||
|
)
|
||||||
|
|
||||||
|
assert context.example_images == [
|
||||||
|
"https://resources.modelscope.cn/cover-images/b.png",
|
||||||
|
"https://resources.modelscope.cn/cover-images/c.png",
|
||||||
|
]
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_unknown_hash_falls_back_to_the_filename(self, monkeypatch):
|
||||||
|
"""A re-encoded file still matches by name rather than losing its images."""
|
||||||
|
self._patch(monkeypatch)
|
||||||
|
|
||||||
|
context = await ModelScopeSource().fetch_model_card_context(
|
||||||
|
"u/r", "Krea-2-LORA_c1-st1000.safetensors", sha256="f" * 64
|
||||||
|
)
|
||||||
|
|
||||||
|
assert context.example_images == [
|
||||||
|
"https://resources.modelscope.cn/cover-images/b.png",
|
||||||
|
"https://resources.modelscope.cn/cover-images/c.png",
|
||||||
|
]
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_hash_is_case_insensitive(self, monkeypatch):
|
||||||
|
self._patch(monkeypatch)
|
||||||
|
|
||||||
|
context = await ModelScopeSource().fetch_model_card_context(
|
||||||
|
"u/r", "renamed.safetensors", sha256=self.ST1000_HASH.upper()
|
||||||
|
)
|
||||||
|
|
||||||
|
assert len(context.example_images) == 2
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_blank_hash_is_ignored(self, monkeypatch):
|
||||||
|
self._patch(monkeypatch)
|
||||||
|
|
||||||
|
context = await ModelScopeSource().fetch_model_card_context(
|
||||||
|
"u/r", "Krea-2-LORA_c1-st8000.safetensors", sha256=" "
|
||||||
|
)
|
||||||
|
|
||||||
|
assert context.example_images == [
|
||||||
|
"https://resources.modelscope.cn/cover-images/a.png"
|
||||||
|
]
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_missing_model_infos_degrades_to_filename_matching(self, monkeypatch):
|
||||||
|
payload = _modelscope_detail_payload()
|
||||||
|
del payload["Data"]["ModelInfos"]
|
||||||
|
|
||||||
|
async def fake_fetch_json(url, **_kwargs):
|
||||||
|
return 200, payload
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"py.services.model_sources.modelscope.fetch_json", fake_fetch_json
|
||||||
|
)
|
||||||
|
|
||||||
|
context = await ModelScopeSource().fetch_model_card_context(
|
||||||
|
"u/r", "Krea-2-LORA_c1-st1000.safetensors", sha256=self.ST1000_HASH
|
||||||
|
)
|
||||||
|
|
||||||
|
assert len(context.example_images) == 2
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_real_published_hashes_are_used_by_the_agent(self):
|
||||||
|
"""The agent must pass the recorded hash, not just the filename."""
|
||||||
|
service = AgentService()
|
||||||
|
with (
|
||||||
|
mock.patch(
|
||||||
|
"py.services.model_sources.modelscope.ModelScopeSource.fetch_model_card",
|
||||||
|
new=mock.AsyncMock(return_value="# card"),
|
||||||
|
),
|
||||||
|
mock.patch(
|
||||||
|
"py.services.model_sources.modelscope.ModelScopeSource.fetch_model_card_context",
|
||||||
|
new=mock.AsyncMock(return_value=ModelCardContext()),
|
||||||
|
) as mock_ctx,
|
||||||
|
):
|
||||||
|
await service._load_source_card(
|
||||||
|
"/models/loras/renamed.safetensors",
|
||||||
|
{
|
||||||
|
"source_platform": "modelscope",
|
||||||
|
"source_url": "https://modelscope.cn/models/u/r",
|
||||||
|
"sha256": "c" * 64,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert mock_ctx.call_args.kwargs["sha256"] == "c" * 64
|
||||||
|
|||||||
Reference in New Issue
Block a user