mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-09-26 13:34:08 -03:00
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
This commit is contained in:
@@ -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', () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:<modelId> 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:
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
# ======================================================================
|
||||
|
||||
Reference in New Issue
Block a user