From c48feeddb62896f3420546efa5c9009231def616 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Fri, 25 Sep 2026 23:29:30 +0800 Subject: [PATCH] fix: stop grouping HF/ModelScope models by repository A repository is not a model identity: collection repos on Hugging Face and ModelScope host many unrelated models, which were wrongly shown as versions of each other. - Hugging Face models no longer auto-group (the Hub exposes no site-native model id) - ModelScope models group by the site's native published-model id (MuseInfo modelVersion.modelId), extracted during enrichment and persisted on the sidecar as source_model_id/source_version_id; unenriched models stay standalone instead of collapsing a whole repo into one group - TensorArt grouping unchanged (its URL id is already model-level) - Frontend group-key derivation mirrors the new backend semantics --- py/services/agent/post_processor.py | 9 ++ py/services/base_model_service.py | 16 ++-- py/services/model_scanner.py | 6 +- py/services/model_sources/base.py | 33 ++++++- py/services/model_sources/huggingface.py | 12 +++ py/services/model_sources/modelscope.py | 49 ++++++++++- py/services/model_sources/registry.py | 11 ++- py/services/persistent_model_cache.py | 10 +++ static/js/components/shared/ModelCard.js | 2 +- .../js/components/shared/ModelVersionsTab.js | 6 +- static/js/utils/modelSourceHelpers.js | 26 +++++- .../frontend/utils/modelSourceHelpers.test.js | 34 +++++++- tests/routes/test_model_source_handlers.py | 6 ++ tests/services/test_base_model_service.py | 85 ++++++++++--------- tests/services/test_model_scanner.py | 12 +++ tests/services/test_model_sources.py | 49 +++++++++-- tests/services/test_post_processor.py | 69 +++++++++++++++ 17 files changed, 357 insertions(+), 78 deletions(-) diff --git a/py/services/agent/post_processor.py b/py/services/agent/post_processor.py index cb6af7d8..cc80faf3 100644 --- a/py/services/agent/post_processor.py +++ b/py/services/agent/post_processor.py @@ -199,6 +199,15 @@ class PostProcessor: if is_source_model and site_version: self._merge_civitai(updates, metadata, name=site_version) + # Site-native identity ids (ModelScope's published model/version ids). + # They are what version grouping keys off, so they must reach the + # sidecar even when nothing else about the card changed. + if is_source_model and source_context is not None: + if source_context.source_model_id: + updates["source_model_id"] = source_context.source_model_id + if source_context.source_version_id: + updates["source_version_id"] = source_context.source_version_id + # gallery images → civitai.images (site example images, YAML frontmatter # widget entries, and Sample Gallery markdown tables in the README body) rec_width = llm_output.get("recommended_width") or 0 diff --git a/py/services/base_model_service.py b/py/services/base_model_service.py index 7a30f74b..228192aa 100644 --- a/py/services/base_model_service.py +++ b/py/services/base_model_service.py @@ -740,18 +740,14 @@ class BaseModelService(ABC): return annotated - @staticmethod - def _extract_hf_group_key(item: Dict[str, Any]) -> Optional[str]: - """Extract `hf:{owner}/{repo}` from item's ``hf_url``, or None.""" - key = BaseModelService._extract_source_group_key(item) - return key if key and key.startswith("hf:") else None - @staticmethod def _extract_source_group_key(item: Dict[str, Any]) -> Optional[str]: """Return the external-source group key for *item*, or None. - Hugging Face keeps the historical ``hf:{owner}/{repo}`` shape; other - platforms use their own short prefix (``ms:`` / ``ta:``). + Only sources with a site-native model identity yield a key: + ModelScope groups by its published-model id (``ms:{id}``), TensorArt + by its numeric model id (``ta:{id}``); Hugging Face models never + group (see :meth:`ModelSource.group_key`). """ return source_group_key(item) @@ -761,8 +757,8 @@ class BaseModelService(ABC): Preference order: 1. CivitAI ``modelId`` (int) - 2. External model source identity, e.g. ``hf:{owner}/{repo}``, - ``ms:{owner}/{repo}``, ``ta:{model_id}`` (str) + 2. External model source identity, e.g. ``ms:{model_id}``, + ``ta:{model_id}`` (str) 3. ``None`` (no known grouping source) """ mid = BaseModelService._extract_model_id(item) diff --git a/py/services/model_scanner.py b/py/services/model_scanner.py index e470dbd2..542671ae 100644 --- a/py/services/model_scanner.py +++ b/py/services/model_scanner.py @@ -399,10 +399,14 @@ class ModelScanner: 'skip_metadata_refresh': bool(get_value('skip_metadata_refresh', False)), # External model source (Hugging Face / ModelScope / TensorArt). # `source_url` + `source_platform` are canonical; `hf_url` stays in - # sync as a legacy alias (normalised below). + # sync as a legacy alias (normalised below). `source_model_id` / + # `source_version_id` are the site-native identity ids version + # grouping keys off (ModelScope; empty elsewhere). 'source_platform': get_value('source_platform', '') or '', 'source_url': get_value('source_url', '') or '', 'hf_url': get_value('hf_url', '') or '', + 'source_model_id': get_value('source_model_id', '') or '', + 'source_version_id': get_value('source_version_id', '') or '', } normalize_metadata_source(entry) diff --git a/py/services/model_sources/base.py b/py/services/model_sources/base.py index 6d097914..afd601a7 100644 --- a/py/services/model_sources/base.py +++ b/py/services/model_sources/base.py @@ -25,7 +25,7 @@ import logging import os import re from dataclasses import dataclass, field -from typing import Any, Dict, Iterable, Optional +from typing import Any, Dict, Iterable, Mapping, Optional import aiohttp @@ -123,6 +123,20 @@ class ModelCardContext: trigger_words: list[str] = field(default_factory=list) """Trigger words the site records for the requested model file.""" + source_model_id: str = "" + """Site-native id of the *published model* the requested file belongs to. + + Sites whose repository is not a model identity publish a separate, + stable id per model (ModelScope's ``modelVersion.modelId`` — identical + across every version of one published model, different between the + models of a collection repository). It is the version-grouping key, + persisted on the sidecar as ``source_model_id``. + """ + + source_version_id: str = "" + """Site-native id of the published version the requested file belongs to + (ModelScope's ``modelVersion.id``), persisted as ``source_version_id``.""" + def is_empty(self) -> bool: """Return ``True`` when the site contributed nothing extra.""" @@ -139,6 +153,8 @@ class ModelCardContext: self.official_tags, self.example_images, self.trigger_words, + self.source_model_id, + self.source_version_id, ) ) @@ -329,11 +345,20 @@ class ModelSource: return "" - def group_key(self, source_id: str) -> str: - """Return the version-group key for *source_id*.""" + def group_key(self, ref: SourceRef, item: Mapping[str, Any]) -> Optional[str]: + """Return the version-group key for the model described by *item*. + + The default groups by source id (``{prefix}:{owner}/{repo}``), which + is only correct when the source id already identifies a single + published model. Sources whose repository hosts many unrelated + models override this: they either derive the key from a site-native + model identity recorded in *item* (ModelScope's ``source_model_id``) + or return ``None`` when the platform has no reliable model identity + at all (Hugging Face), leaving the model ungrouped. + """ prefix = GROUP_PREFIXES.get(self.platform, self.platform) - return f"{prefix}:{source_id}" + return f"{prefix}:{ref.source_id}" async def fetch_model_card(self, source_id: str) -> str: """Fetch the raw model card (README) markdown for *source_id*.""" diff --git a/py/services/model_sources/huggingface.py b/py/services/model_sources/huggingface.py index f67f0b0f..2ccda732 100644 --- a/py/services/model_sources/huggingface.py +++ b/py/services/model_sources/huggingface.py @@ -4,10 +4,12 @@ from __future__ import annotations import logging import re +from typing import Any, Mapping, Optional from .base import ( ModelSource, ModelSourceError, + SourceRef, fetch_json, fetch_text, filter_weight_files, @@ -54,6 +56,16 @@ class HuggingFaceSource(ModelSource): def canonical_url(self, source_id: str) -> str: return f"https://huggingface.co/{source_id}" + def group_key(self, ref: SourceRef, item: Mapping[str, Any]) -> Optional[str]: + """Hugging Face models never auto-group. + + A repository is not a model identity — collection repos host many + unrelated models — and the Hub exposes no site-native published-model + id, so there is no reliable key to group by. + """ + + return None + def asset_base_url(self, source_id: str, revision: str = "") -> str: return f"https://huggingface.co/{source_id}/resolve/{self.resolve_revision(revision)}" diff --git a/py/services/model_sources/modelscope.py b/py/services/model_sources/modelscope.py index d2333dda..1f9f063e 100644 --- a/py/services/model_sources/modelscope.py +++ b/py/services/model_sources/modelscope.py @@ -44,12 +44,15 @@ import json import logging import os import re -from typing import TYPE_CHECKING, Any, Iterable, Optional +from typing import TYPE_CHECKING, Any, Iterable, Mapping, Optional from .base import ( + GROUP_PREFIXES, ModelCardContext, ModelSource, ModelSourceError, + SourceRef, + clean_source_url, fetch_json, fetch_text, filter_weight_files, @@ -111,6 +114,22 @@ class ModelScopeSource(ModelSource): def canonical_url(self, source_id: str) -> str: return f"{self.base_url}/models/{source_id}" + def group_key(self, ref: SourceRef, item: Mapping[str, Any]) -> Optional[str]: + """Group by ModelScope's published-model id, never by repository. + + A collection repository hosts many unrelated published models, so + the repo id is not a version-group identity. Only models whose + metadata carries the site-native ``source_model_id`` (recorded at + enrichment time from ``MuseInfo.versions[].modelVersion.modelId``) + group together; unenriched models stay standalone. + """ + + model_id = clean_source_url(item.get("source_model_id")) + if not model_id: + return None + prefix = GROUP_PREFIXES.get(self.platform, self.platform) + return f"{prefix}:{model_id}" + def asset_base_url(self, source_id: str, revision: str = "") -> str: return ( f"{self.base_url}/models/{source_id}/resolve/" @@ -350,9 +369,37 @@ def _build_card_context( context.version_name = _version_label(versions) context.example_images = _cover_image_urls(versions) context.trigger_words = _version_trigger_words(versions) + context.source_model_id, context.source_version_id = _version_identity( + versions + ) return context +def _version_identity(versions: list[dict[str, Any]]) -> tuple[str, str]: + """Return the site-native ``(model id, version id)`` of the first match. + + ``modelVersion.modelId`` is identical across every version of one + published model and differs between the models of a collection + repository, which makes it the version-grouping identity; + ``modelVersion.id`` identifies the version itself. Both are ints in + the payload and are stored as strings. + """ + + for version in versions: + model_version = version.get("modelVersion") + if not isinstance(model_version, dict): + continue + model_id = model_version.get("modelId") + version_id = model_version.get("id") + if model_id is None and version_id is None: + continue + return ( + str(model_id) if model_id is not None else "", + str(version_id) if version_id is not None else "", + ) + return "", "" + + def _base_model_aliases(data: dict[str, Any]) -> list[str]: """Return the site's own names for the base model. diff --git a/py/services/model_sources/registry.py b/py/services/model_sources/registry.py index ef563ad1..df568c42 100644 --- a/py/services/model_sources/registry.py +++ b/py/services/model_sources/registry.py @@ -196,8 +196,13 @@ def get_source_platform(item: Mapping[str, Any]) -> str: def source_group_key(item: Mapping[str, Any]) -> Optional[str]: """Return the version-group key for *item*, or ``None``. - Hugging Face keeps the historical ``hf:{owner}/{repo}`` shape; other - platforms use their own short prefix (see :data:`GROUP_PREFIXES`). + Only sources with a site-native model identity yield a key: TensorArt + groups by its numeric model id (``ta:``) and ModelScope by the + published-model id recorded at enrichment time (``ms:`` / + ``msai:``). Hugging Face yields no key at all — a repository is + not a model identity — and unenriched ModelScope models stay + standalone rather than collapsing a whole collection repository into + one group. """ ref = resolve_source_ref(item) @@ -206,7 +211,7 @@ def source_group_key(item: Mapping[str, Any]) -> Optional[str]: source = get_source(ref.platform) if source is None: return None - return source.group_key(ref.source_id) + return source.group_key(ref, item) __all__ = [ diff --git a/py/services/persistent_model_cache.py b/py/services/persistent_model_cache.py index 144f5027..8d9eefc2 100644 --- a/py/services/persistent_model_cache.py +++ b/py/services/persistent_model_cache.py @@ -68,6 +68,8 @@ class PersistentModelCache: "source_platform", "source_url", "hf_url", + "source_model_id", + "source_version_id", ) _MODEL_UPDATE_COLUMNS: Tuple[str, ...] = _MODEL_COLUMNS[2:] _instances: Dict[str, "PersistentModelCache"] = {} @@ -214,6 +216,8 @@ class PersistentModelCache: "source_platform": row["source_platform"] or "", "source_url": row["source_url"] or "", "hf_url": row["hf_url"] or "", + "source_model_id": row["source_model_id"] or "", + "source_version_id": row["source_version_id"] or "", } # Legacy rows only carry `hf_url`; derive the canonical pair so # every consumer sees the same shape. @@ -579,6 +583,8 @@ class PersistentModelCache: source_platform TEXT DEFAULT '', source_url TEXT DEFAULT '', hf_url TEXT DEFAULT '', + source_model_id TEXT DEFAULT '', + source_version_id TEXT DEFAULT '', PRIMARY KEY (model_type, file_path) ); @@ -648,6 +654,8 @@ class PersistentModelCache: "source_platform": "TEXT DEFAULT ''", "source_url": "TEXT DEFAULT ''", "hf_url": "TEXT DEFAULT ''", + "source_model_id": "TEXT DEFAULT ''", + "source_version_id": "TEXT DEFAULT ''", "autov3": "TEXT", } @@ -735,6 +743,8 @@ class PersistentModelCache: item.get("source_platform") or "", item.get("source_url") or "", item.get("hf_url") or "", + item.get("source_model_id") or "", + item.get("source_version_id") or "", ) def _insert_model_sql(self) -> str: diff --git a/static/js/components/shared/ModelCard.js b/static/js/components/shared/ModelCard.js index 25202558..bdffb3c7 100644 --- a/static/js/components/shared/ModelCard.js +++ b/static/js/components/shared/ModelCard.js @@ -522,7 +522,7 @@ export function createModelCard(model, modelType) { card.dataset.modelId = modelId; } else { // For externally-sourced models, derive a group key from the source - // URL for version grouping (hf:user/repo, ms:user/repo, ta:). + // identity for version grouping (ms:, ta:). const sourceGroupKey = getModelSourceGroupKey(model); if (sourceGroupKey) { card.dataset.modelId = sourceGroupKey; diff --git a/static/js/components/shared/ModelVersionsTab.js b/static/js/components/shared/ModelVersionsTab.js index cdeb0863..f14602b7 100644 --- a/static/js/components/shared/ModelVersionsTab.js +++ b/static/js/components/shared/ModelVersionsTab.js @@ -994,9 +994,9 @@ export function initVersionsTab({ renderErrorState(container, translate('modals.model.versions.missingModelId', {}, 'This model is missing a Civitai model id.')); return; } - // External source group keys (e.g. "hf:user/repo", "ms:user/repo", - // "ta:8278...") are not real CivitAI model IDs — skip the remote API - // call and show a helpful message instead. + // External source group keys (e.g. "ms:12345", "ta:8278...") are + // not real CivitAI model IDs — skip the remote API call and show a + // helpful message instead. const sourceGroup = parseModelSourceGroupKey(modelId); if (sourceGroup) { controller.isLoading = false; diff --git a/static/js/utils/modelSourceHelpers.js b/static/js/utils/modelSourceHelpers.js index 459f99e2..c23e2c0c 100644 --- a/static/js/utils/modelSourceHelpers.js +++ b/static/js/utils/modelSourceHelpers.js @@ -6,8 +6,7 @@ * support AI metadata enrichment. * * Models loaded from an older cache may only carry the legacy `hf_url` - * field; every helper here falls back to it, and to the legacy - * `hf:user/repo` group key shape. + * field; every helper here falls back to it. */ import { translate } from './i18nHelpers.js'; @@ -17,6 +16,9 @@ export const MODEL_SOURCES = [ platform: 'huggingface', label: 'Hugging Face', groupPrefix: 'hf', + // A repository hosts many unrelated models and the site exposes no + // model-level identity, so HF models never auto-group. + groupKey: 'none', supportsEnrichment: true, supportsDownload: true, defaultRevision: 'main', @@ -36,6 +38,9 @@ export const MODEL_SOURCES = [ platform: 'modelscope', label: 'ModelScope', groupPrefix: 'ms', + // Group by the site-native published-model id (`source_model_id`), + // recorded by enrichment — the repo id is not a model identity. + groupKey: 'modelId', supportsEnrichment: true, supportsDownload: true, defaultRevision: 'master', @@ -56,6 +61,7 @@ export const MODEL_SOURCES = [ platform: 'modelscope-ai', label: 'ModelScope (International)', groupPrefix: 'msai', + groupKey: 'modelId', supportsEnrichment: true, supportsDownload: true, defaultRevision: 'master', @@ -73,6 +79,8 @@ export const MODEL_SOURCES = [ platform: 'tensorart', label: 'TensorArt', groupPrefix: 'ta', + // The numeric id in a TensorArt URL already identifies a single model. + groupKey: 'repo', supportsEnrichment: false, supportsDownload: false, defaultRevision: '', @@ -152,11 +160,21 @@ export function getModelSourceInfo(model) { /** * Version-group key for a model, matching the backend's `_extract_group_key`. - * Returns `''` when the model has no external source. + * Returns `''` when the model has no external source, or when its source has + * no reliable model identity (Hugging Face, or a ModelScope model that has + * not been enriched with the site-native `source_model_id` yet). */ export function getModelSourceGroupKey(model) { const info = getModelSourceInfo(model); - if (!info || !info.sourceId) return ''; + if (!info) return ''; + const strategy = info.groupKey || 'repo'; + if (strategy === 'none') return ''; + if (strategy === 'modelId') { + const modelId = + model && typeof model.source_model_id === 'string' ? model.source_model_id.trim() : ''; + return modelId ? `${info.groupPrefix}:${modelId}` : ''; + } + if (!info.sourceId) return ''; return `${info.groupPrefix}:${info.sourceId}`; } diff --git a/tests/frontend/utils/modelSourceHelpers.test.js b/tests/frontend/utils/modelSourceHelpers.test.js index 224dd597..629aa896 100644 --- a/tests/frontend/utils/modelSourceHelpers.test.js +++ b/tests/frontend/utils/modelSourceHelpers.test.js @@ -105,13 +105,39 @@ describe('modelSourceHelpers', () => { describe('getModelSourceGroupKey', () => { it('matches the backend group-key shapes', () => { - expect(getModelSourceGroupKey({ hf_url: 'https://huggingface.co/u/r' })).toBe('hf:u/r'); - expect( - getModelSourceGroupKey({ source_url: 'https://modelscope.cn/models/u/r' }) - ).toBe('ms:u/r'); + // TensorArt's numeric id already identifies a single model. expect(getModelSourceGroupKey({ source_url: 'https://tensor.art/models/123' })).toBe( 'ta:123' ); + // ModelScope groups by the site-native published-model id. + expect( + getModelSourceGroupKey({ + source_url: 'https://modelscope.cn/models/u/r', + source_model_id: '555', + }) + ).toBe('ms:555'); + expect( + getModelSourceGroupKey({ + source_url: 'https://www.modelscope.ai/models/u/r', + source_model_id: '678', + }) + ).toBe('msai:678'); + }); + + it('returns an empty string for sources without a model identity', () => { + // Hugging Face repos are not a model identity: never grouped. + expect(getModelSourceGroupKey({ hf_url: 'https://huggingface.co/u/r' })).toBe(''); + // Unenriched ModelScope models stay standalone rather than collapsing + // a whole collection repo into one group. + expect( + getModelSourceGroupKey({ source_url: 'https://modelscope.cn/models/u/r' }) + ).toBe(''); + expect( + getModelSourceGroupKey({ + source_url: 'https://modelscope.cn/models/u/r', + source_model_id: ' ', + }) + ).toBe(''); }); it('returns an empty string without a source', () => { diff --git a/tests/routes/test_model_source_handlers.py b/tests/routes/test_model_source_handlers.py index afbb99c5..415b5afb 100644 --- a/tests/routes/test_model_source_handlers.py +++ b/tests/routes/test_model_source_handlers.py @@ -1027,6 +1027,8 @@ def _modelscope_card_payload() -> dict: "modelVersion": { "showName": "c1-st1000", "triggerWords": '["kreaface","kreamodel"]', + "id": 1002, + "modelId": 555, }, "coverImages": [ {"url": "https://resources.modelscope.cn/cover-images/b.png"}, @@ -1141,6 +1143,9 @@ async def test_download_hydrates_the_card_from_the_site(tmp_path, monkeypatch): '{"strength_min": 0.5, "strength_max": 1.2, "strength_range": "0.5-1.2"}' ) assert saved["metadata_source"] == "source:modelscope" + # The site-native identity ids are persisted for version grouping. + assert saved["source_model_id"] == "555" + assert saved["source_version_id"] == "1002" # No provider answered, so claiming an AI enrichment would be a lie. assert "llm_enriched_at" not in saved @@ -1148,6 +1153,7 @@ async def test_download_hydrates_the_card_from_the_site(tmp_path, monkeypatch): assert scanner.update_single_model_cache.await_count == 1 cached = scanner.update_single_model_cache.await_args.args[2] assert cached["model_name"] == "Krea-2-LORA" + assert cached["source_model_id"] == "555" @pytest.mark.asyncio diff --git a/tests/services/test_base_model_service.py b/tests/services/test_base_model_service.py index b4ee4ba1..4ee365cd 100644 --- a/tests/services/test_base_model_service.py +++ b/tests/services/test_base_model_service.py @@ -1263,39 +1263,8 @@ async def test_get_model_civitai_url_falls_back_when_host_setting_is_not_a_strin } -class TestHfGroupKey: - """Tests for _extract_hf_group_key and _extract_group_key.""" - - # --- _extract_hf_group_key --- - - def test_hf_group_key_valid_url(self): - """Standard HF URL returns hf:user/repo.""" - item = {"hf_url": "https://huggingface.co/unsloth/qwen-edit"} - assert BaseModelService._extract_hf_group_key(item) == "hf:unsloth/qwen-edit" - - def test_hf_group_key_url_with_subpath(self): - """URL with subpath still extracts just owner/repo.""" - item = {"hf_url": "https://huggingface.co/user/repo/resolve/main/file.safetensors"} - assert BaseModelService._extract_hf_group_key(item) == "hf:user/repo" - - def test_hf_group_key_empty_url(self): - """Empty hf_url returns None.""" - assert BaseModelService._extract_hf_group_key({"hf_url": ""}) is None - - def test_hf_group_key_no_url(self): - """Missing hf_url key returns None.""" - assert BaseModelService._extract_hf_group_key({}) is None - - def test_hf_group_key_none_url(self): - """None hf_url returns None.""" - assert BaseModelService._extract_hf_group_key({"hf_url": None}) is None - - def test_hf_group_key_invalid_url(self): - """Malformed HF URL returns None.""" - assert BaseModelService._extract_hf_group_key({"hf_url": "not-a-url"}) is None - assert BaseModelService._extract_hf_group_key({"hf_url": "https://example.com"}) is None - - # --- _extract_group_key --- +class TestSourceGroupKey: + """Tests for _extract_group_key (CivitAI id, then site-native source identity).""" def test_group_key_civitai_only(self): """CivitAI modelId returned as int.""" @@ -1303,30 +1272,64 @@ class TestHfGroupKey: assert BaseModelService._extract_group_key(item) == 123 def test_group_key_hf_only(self): - """HF-only item returns hf:user/repo string.""" + """HF-linked items never group: a repository is not a model identity.""" item = {"hf_url": "https://huggingface.co/user/repo"} - assert BaseModelService._extract_group_key(item) == "hf:user/repo" + assert BaseModelService._extract_group_key(item) is None def test_group_key_civitai_preferred(self): - """CivitAI modelId takes precedence over hf_url.""" + """CivitAI modelId takes precedence over any source identity.""" item = { "civitai": {"modelId": 456}, - "hf_url": "https://huggingface.co/other/repo", + "source_url": "https://tensor.art/models/789", } assert BaseModelService._extract_group_key(item) == 456 def test_group_key_neither(self): - """No CivitAI or HF returns None.""" + """No CivitAI or groupable source returns None.""" assert BaseModelService._extract_group_key({}) is None assert BaseModelService._extract_group_key({"some": "data"}) is None def test_group_key_civitai_none_model_id(self): - """civitai.modelId=None falls through to HF.""" + """civitai.modelId=None falls through to the source identity.""" item = { "civitai": {"modelId": None}, - "hf_url": "https://huggingface.co/user/repo", + "source_url": "https://tensor.art/models/789", } - assert BaseModelService._extract_group_key(item) == "hf:user/repo" + assert BaseModelService._extract_group_key(item) == "ta:789" + + def test_group_key_modelscope_uses_published_model_id(self): + """ModelScope groups under ms: once enrichment recorded it.""" + item = { + "source_platform": "modelscope", + "source_url": "https://modelscope.cn/models/u/r", + "source_model_id": "555", + } + assert BaseModelService._extract_group_key(item) == "ms:555" + + def test_group_key_modelscope_unenriched_stays_standalone(self): + """Without source_model_id there is no key — never repo-level grouping.""" + item = { + "source_platform": "modelscope", + "source_url": "https://modelscope.cn/models/u/r", + } + assert BaseModelService._extract_group_key(item) is None + + def test_group_key_modelscope_identity_crosses_repos(self): + """Same published-model id groups across repos; same repo does not.""" + + def ms_item(repo, model_id): + return { + "source_platform": "modelscope", + "source_url": f"https://modelscope.cn/models/{repo}", + "source_model_id": model_id, + } + + assert BaseModelService._extract_group_key( + ms_item("alice/collection", "555") + ) == BaseModelService._extract_group_key(ms_item("bob/mirror", "555")) + assert BaseModelService._extract_group_key( + ms_item("alice/collection", "555") + ) != BaseModelService._extract_group_key(ms_item("alice/collection", "777")) class TestApplyHashFilters: diff --git a/tests/services/test_model_scanner.py b/tests/services/test_model_scanner.py index 3a57109c..95778f24 100644 --- a/tests/services/test_model_scanner.py +++ b/tests/services/test_model_scanner.py @@ -967,6 +967,8 @@ def _make_cache_entry(**overrides) -> Dict[str, Any]: "source_platform": "", "source_url": "", "hf_url": "", + "source_model_id": "", + "source_version_id": "", "license_flags": 113, "hash_status": "completed", } @@ -1005,6 +1007,8 @@ async def test_sync_cache_no_change(tmp_path: Path): "tags": ["alpha"], "civitai": {"id": 111, "modelId": 222, "name": "v1"}, "hf_url": "", + "source_model_id": "", + "source_version_id": "", } changed = await scanner.sync_cache_from_metadata( @@ -1049,6 +1053,8 @@ async def test_sync_cache_in_place_update(tmp_path: Path): "tags": ["beta", "gamma"], "civitai": {"id": 111, "modelId": 222, "name": "v1"}, "hf_url": "", + "source_model_id": "", + "source_version_id": "", } changed = await scanner.sync_cache_from_metadata( @@ -1094,6 +1100,8 @@ async def test_sync_cache_not_in_cache_delegates(tmp_path: Path): "tags": [], "civitai": {}, "hf_url": "", + "source_model_id": "", + "source_version_id": "", } changed = await scanner.sync_cache_from_metadata( @@ -1147,6 +1155,8 @@ async def test_sync_cache_conditional_resort_skipped(tmp_path: Path, monkeypatch "tags": ["alpha"], "civitai": {"id": 111, "modelId": 222, "name": "v1"}, "hf_url": "", + "source_model_id": "", + "source_version_id": "", } changed = await scanner.sync_cache_from_metadata( @@ -1197,6 +1207,8 @@ async def test_sync_cache_conditional_resort_triggered(tmp_path: Path, monkeypat "tags": ["alpha"], "civitai": {"id": 111, "modelId": 222, "name": "v1"}, "hf_url": "", + "source_model_id": "", + "source_version_id": "", } changed = await scanner.sync_cache_from_metadata( diff --git a/tests/services/test_model_sources.py b/tests/services/test_model_sources.py index 9e720c4d..31819d5a 100644 --- a/tests/services/test_model_sources.py +++ b/tests/services/test_model_sources.py @@ -279,15 +279,39 @@ class TestHelpers: assert get_source_platform({"source_platform": "tensorart"}) == "tensorart" assert get_source_platform({}) == "" - def test_group_keys_match_legacy_hf_shape(self): - assert source_group_key({"hf_url": "https://huggingface.co/u/r"}) == "hf:u/r" - assert ( - source_group_key({"source_url": "https://modelscope.cn/models/u/r"}) == "ms:u/r" - ) + def test_group_keys_use_site_native_identity(self): + # Hugging Face has no site-native model identity: never grouped. + assert source_group_key({"hf_url": "https://huggingface.co/u/r"}) is None + # TensorArt's numeric id already identifies a single model. assert ( source_group_key({"source_url": "https://tensor.art/models/123"}) == "ta:123" ) + def test_modelscope_groups_by_published_model_id(self): + # Without an enriched source_model_id the model stays standalone — + # never grouped by repo, which would collapse a collection repo. + assert ( + source_group_key({"source_url": "https://modelscope.cn/models/u/r"}) is None + ) + assert ( + source_group_key( + { + "source_url": "https://modelscope.cn/models/u/r", + "source_model_id": "555", + } + ) + == "ms:555" + ) + assert ( + source_group_key( + { + "source_url": "https://www.modelscope.ai/models/u/r", + "source_model_id": "678", + } + ) + == "msai:678" + ) + def test_group_key_is_none_without_source(self): assert source_group_key({}) is None assert source_group_key({"hf_url": "https://example.com/x"}) is None @@ -457,7 +481,12 @@ def _modelscope_detail_payload() -> dict: "versions": [ { "stats": {"fileList": ["Krea-2-LORA_c1-st8000.safetensors"]}, - "modelVersion": {"showName": "c1-st8000", "triggerWords": '[""]'}, + "modelVersion": { + "showName": "c1-st8000", + "triggerWords": '[""]', + "id": 1001, + "modelId": 555, + }, "coverImages": [ {"url": "https://resources.modelscope.cn/cover-images/a.png"} ], @@ -467,6 +496,8 @@ def _modelscope_detail_payload() -> dict: "modelVersion": { "showName": "c1-st1000", "triggerWords": '["kreaface","kreamodel"]', + "id": 1002, + "modelId": 555, }, "coverImages": [ {"url": "https://resources.modelscope.cn/cover-images/b.png"}, @@ -533,6 +564,9 @@ class TestFetchModelCardContext: # The version label is taken from the file that was matched, not from # whichever version happens to come first in the payload. assert context.version_name == "c1-st1000" + # The site-native identity ids belong to the matched version too. + assert context.source_model_id == "555" + assert context.source_version_id == "1002" @pytest.mark.asyncio async def test_modelscope_version_label_is_empty_for_an_unknown_file( @@ -550,6 +584,9 @@ class TestFetchModelCardContext: ) assert context.version_name == "" + # No version matched, so there is no per-version identity either. + assert context.source_model_id == "" + assert context.source_version_id == "" # The repository-wide fields are still published. assert context.model_name == "Krea-2-LORA" diff --git a/tests/services/test_post_processor.py b/tests/services/test_post_processor.py index 44f0eba2..76a6aca4 100644 --- a/tests/services/test_post_processor.py +++ b/tests/services/test_post_processor.py @@ -817,6 +817,75 @@ class TestSiteProvidedContext: "https://huggingface.co/user/repo/resolve/main/images/cat.png" ] + @pytest.mark.asyncio + async def test_site_identity_ids_are_persisted(self, processor): + """source_model_id/source_version_id reach the sidecar for grouping.""" + context = ModelCardContext(source_model_id="555", source_version_id="1002") + + with ( + mock.patch("py.metadata_ops.apply_metadata_updates") as mock_apply, + mock.patch("py.metadata_ops.download_preview", return_value=None), + mock.patch("py.metadata_ops.refresh_cache"), + ): + await processor.process( + skill_name="enrich_hf_metadata", + model_path="/p.safetensors", + llm_output=self.LLM_OUTPUT, + metadata=dict(self.MODELSCOPE_METADATA), + readme_content="", + source_context=context, + ) + + applied = mock_apply.call_args[0][1] + assert applied["source_model_id"] == "555" + assert applied["source_version_id"] == "1002" + + @pytest.mark.asyncio + async def test_site_identity_ids_absent_without_context_values(self, processor): + """No identity keys are written when the site did not publish any.""" + with ( + mock.patch("py.metadata_ops.apply_metadata_updates") as mock_apply, + mock.patch("py.metadata_ops.download_preview", return_value=None), + mock.patch("py.metadata_ops.refresh_cache"), + ): + await processor.process( + skill_name="enrich_hf_metadata", + model_path="/p.safetensors", + llm_output=self.LLM_OUTPUT, + metadata=dict(self.MODELSCOPE_METADATA), + readme_content="", + source_context=ModelCardContext(description="summary only"), + ) + + applied = mock_apply.call_args[0][1] + assert "source_model_id" not in applied + assert "source_version_id" not in applied + + @pytest.mark.asyncio + async def test_site_identity_ids_skipped_for_a_model_with_no_external_source( + self, processor + ): + """A CivitAI-only model must not pick up source identity ids.""" + context = ModelCardContext(source_model_id="555", source_version_id="1002") + + with ( + mock.patch("py.metadata_ops.apply_metadata_updates") as mock_apply, + mock.patch("py.metadata_ops.download_preview", return_value=None), + mock.patch("py.metadata_ops.refresh_cache"), + ): + await processor.process( + skill_name="enrich_hf_metadata", + model_path="/p.safetensors", + llm_output=self.LLM_OUTPUT, + metadata={"from_civitai": True}, + readme_content="", + source_context=context, + ) + + applied = mock_apply.call_args[0][1] + assert "source_model_id" not in applied + assert "source_version_id" not in applied + # ======================================================================