diff --git a/docs/agent_skills.md b/docs/agent_skills.md index c45152d2..184d981e 100644 --- a/docs/agent_skills.md +++ b/docs/agent_skills.md @@ -104,10 +104,12 @@ boilerplate README and put everything useful in `Description`, so reading only the README yields almost nothing. Providers opt in by overriding `ModelSource.fetch_model_card_context()`, which -returns a `ModelCardContext`. Example images are matched to the model's -**basename**, so each checkpoint in a collection repo gets its own images. -Sites with no such extras inherit an empty context, and the pipeline behaves -exactly as before. +returns a `ModelCardContext`. The wanted file is identified by its sha256 when +the caller knows it (the scanner already records one) and by **basename** +otherwise, so each checkpoint in a collection repo gets its own images — and +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 file, so `execute_skill()` creates a `ModelSourceCache` for the duration of a diff --git a/py/services/agent/agent_service.py b/py/services/agent/agent_service.py index 10cf3461..d671261f 100644 --- a/py/services/agent/agent_service.py +++ b/py/services/agent/agent_service.py @@ -474,9 +474,13 @@ class AgentService: cache.readmes[cache_key] = readme # 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( - 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_base_model"] = card_context.base_model diff --git a/py/services/model_sources/base.py b/py/services/model_sources/base.py index 49f63bd9..02793f9d 100644 --- a/py/services/model_sources/base.py +++ b/py/services/model_sources/base.py @@ -307,19 +307,24 @@ class ModelSource: source_id: str, filename: str = "", *, + sha256: str = "", cache: Optional["ModelSourceCache"] = None, ) -> ModelCardContext: """Return the card extras the site keeps outside the README. - *filename* is the model file's basename (no directory) and selects - the right entry when a repository holds several models. Sites whose - model card is fully described by :meth:`fetch_model_card` need no - override and inherit this empty context. + *filename* is the model file's basename (no directory) and *sha256* + its content hash; between them they select the right entry when a + repository holds several models. A site that records per-file hashes + 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`) that lets a provider avoid re-fetching repository-wide data for every 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 context as "the site had nothing extra to say". """ diff --git a/py/services/model_sources/modelscope.py b/py/services/model_sources/modelscope.py index f8e53008..8990580b 100644 --- a/py/services/model_sources/modelscope.py +++ b/py/services/model_sources/modelscope.py @@ -117,6 +117,7 @@ class ModelScopeSource(ModelSource): source_id: str, filename: str = "", *, + sha256: str = "", cache: Optional["ModelSourceCache"] = None, ) -> ModelCardContext: """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``, so enrichment that reads only the README comes back nearly empty. - Example images are matched to *filename* through each version's - ``stats.fileList``, which means the images returned belong to the - exact ``.safetensors`` being enriched — essential for collection - repositories, where every checkpoint has its own sample image. + The wanted file is identified by its sha256 when the caller knows it + and by *filename* otherwise; see :func:`_matching_versions`. The + images and trigger words returned belong to that exact + ``.safetensors`` — essential for collection repositories, where every + checkpoint has its own sample image. The detail payload describes the whole repository and is therefore 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) if data is None: return ModelCardContext() - return _build_card_context(data, filename) + return _build_card_context(data, filename, sha256) async def _fetch_detail( self, @@ -245,7 +247,9 @@ def _first_string(value: Any) -> str: 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`. 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")), ) - versions = _matching_versions(data.get("MuseInfo"), filename) + versions = _matching_versions( + data.get("MuseInfo"), + filename, + digests=_file_digests(data), + sha256=sha256, + ) if versions: context.example_images = _cover_image_urls(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() -def _matching_versions(muse_info: Any, filename: str) -> list[dict[str, Any]]: - """Return the ``versions`` entries that publish *filename*. +def _file_digests(data: dict[str, Any]) -> dict[str, str]: + """Return ``basename -> sha256`` for every published weight file. - Matching is by exact basename first, then by the version's ``showName`` - appearing in the file stem (which absorbs the naming drift ModelScope - sometimes applies to uploaded weights). All matches are returned so a - 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 - to the wrong file. + ``ModelInfos`` groups the repository's files by kind (``safetensor``, + …) and records a real sha256 for each, which is what makes it possible to + recognise a file the user has renamed. + """ + + digests: dict[str, str] = {} + 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): @@ -371,6 +420,18 @@ def _matching_versions(muse_info: Any, filename: str) -> list[dict[str, Any]]: if not entries: 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: return entries if len(entries) == 1 else [] diff --git a/tests/services/test_model_sources.py b/tests/services/test_model_sources.py index db59370f..70361276 100644 --- a/tests/services/test_model_sources.py +++ b/tests/services/test_model_sources.py @@ -7,9 +7,12 @@ each provider's model-card fetching and capability flags. from __future__ import annotations +from unittest import mock import pytest +from py.services.agent.agent_service import AgentService from py.services.model_sources import ( + ModelCardContext, ModelSourceCache, HuggingFaceSource, 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") 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