From f8aba393fee1ee57db16a19b7995e2a56eeb1411 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Sat, 3 Oct 2026 09:46:46 +0800 Subject: [PATCH 1/2] feat(models): show Civitai model/version ids in model modal, always-on hash/id search - Model modal hash footnote now shows Civitai model id and version id (right-aligned, quick-copy buttons); hidden for non-Civitai models - Hash/id exact search (sha256/autov2/autov3/civitai ids) is now always on: the search-options "hash" toggle is removed and the search_hash query param is silently ignored for API compatibility - Footnote render condition relaxed so autov3-only and id-only models still show the line - i18n: 4 new keys translated in all 9 locales; filters.hash key removed --- docs/i18n-translation-guidelines.md | 26 ++++ locales/de.json | 5 +- locales/en.json | 5 +- locales/es.json | 5 +- locales/fr.json | 5 +- locales/he.json | 5 +- locales/ja.json | 5 +- locales/ko.json | 5 +- locales/ru.json | 5 +- locales/zh-CN.json | 5 +- locales/zh-TW.json | 5 +- py/routes/handlers/model_handlers.py | 1 - py/services/model_query.py | 28 ++-- .../css/components/lora-modal/lora-modal.css | 9 ++ static/js/api/baseModelApi.js | 3 - static/js/components/shared/ModelModal.js | 49 ++++++- static/js/managers/SearchManager.js | 1 - static/js/state/index.js | 4 - templates/components/header.html | 3 - .../api/baseModelApi.searchParams.test.js | 120 ------------------ .../components/modelModal.hashes.test.js | 66 +++++++++- tests/services/test_model_query_search.py | 61 +++++++-- 22 files changed, 254 insertions(+), 167 deletions(-) delete mode 100644 tests/frontend/api/baseModelApi.searchParams.test.js diff --git a/docs/i18n-translation-guidelines.md b/docs/i18n-translation-guidelines.md index aced6bad..8ac194a0 100644 --- a/docs/i18n-translation-guidelines.md +++ b/docs/i18n-translation-guidelines.md @@ -120,6 +120,14 @@ Locales: `en`, `zh-CN`, `zh-TW`, `ja`, `ko`, `fr`, `de`, `es`, `ru`, `he` (RTL). > style. All 9 locales are translated (terminology in §2, "Download routing feature"), > so the "no remaining placeholders" claim holds again. +> **Status (2026-10, Civitai ids in model modal):** the model modal's hash footnote now +> shows the Civitai model id and version id (right-aligned, with copy buttons), adding +> 4 keys (`modals.model.metadata.civitaiModelId` / `.civitaiVersionId`, +> `modals.model.actions.copyCivitaiId` / `.civitaiIdCopied`). The same pass removed the +> search-options "hash" toggle (`header.search.filters.hash`) because hash/id search is +> now always on. All 9 locales are translated (terminology in §2, "Civitai ids feature"), +> so the "no remaining placeholders" claim holds again. + --- ## 1. Hard rules (do not violate) @@ -586,6 +594,24 @@ directly (model-type names, R3). The tooltip quotes the `modals.download.useDefa label verbatim with each locale's UI-label quoting style (zh-CN “ ”, zh-TW/ja 「 」, ko `' '`, fr « … », de „ … “, es/ru/he «…»). +### Civitai ids feature (model/version id in the model modal) + +The model modal's hash footnote shows the Civitai **model id** and **version id** with +copy buttons (`modals.model.metadata.civitaiModelId` / `.civitaiVersionId` labels, +`modals.model.actions.copyCivitaiId` tooltip, `.civitaiIdCopied` toast). **"ID" stays +Latin in every locale** (same precedent as `recipes.*.copyId`), and `Civitai` is the +brand (R3) — it is never translated or transliterated; the casing mirrors `en.json` +verbatim (R9). The copy/copied strings reuse each locale's existing clipboard patterns +(`modals.model.actions.copyHash` / `openFileLocation.copied`). + +| Term | Rendering | +|---|---| +| Model ID (label) | zh-CN 模型 ID · zh-TW 模型 ID · ja モデル ID · ko 모델 ID · fr ID du modèle · de Modell-ID · es ID del modelo · ru ID модели · he מזהה מודל | +| Version ID (label) | zh-CN 版本 ID · zh-TW 版本 ID · ja バージョン ID · ko 버전 ID · fr ID de version · de Versions-ID · es ID de versión · ru ID версии · he מזהה גרסה | + +Hebrew uses its established מזהה ("identifier") noun instead of Latin `ID` in these +labels, matching `recipes.*.copyId` (העתק מזהה מתכון). + --- ## 3. Cross-cutting confusion hot-spots (must-fix list) diff --git a/locales/de.json b/locales/de.json index 99b92321..27d813f7 100644 --- a/locales/de.json +++ b/locales/de.json @@ -251,7 +251,6 @@ "modelname": "Modellname", "tags": "Tags", "creator": "Ersteller", - "hash": "Hash", "title": "Rezept-Titel", "loraName": "LoRA-Dateiname", "loraModel": "LoRA-Modellname", @@ -1837,6 +1836,8 @@ "sendToWorkflow": "An ComfyUI senden", "sendToWorkflowText": "An ComfyUI senden", "copyHash": "Hash kopieren", + "copyCivitaiId": "Civitai-ID kopieren", + "civitaiIdCopied": "Civitai-ID in die Zwischenablage kopiert", "deleteModelWithShortcut": "Modell löschen (Del)" }, "openFileLocation": { @@ -1855,6 +1856,8 @@ "baseModel": "Basismodell", "size": "Größe", "hashes": "Hashes", + "civitaiModelId": "Modell-ID", + "civitaiVersionId": "Versions-ID", "unknown": "Unbekannt", "usageTips": "Nutzungstipps", "additionalNotes": "Zusätzliche Notizen", diff --git a/locales/en.json b/locales/en.json index 9b83bf13..19eb7b77 100644 --- a/locales/en.json +++ b/locales/en.json @@ -251,7 +251,6 @@ "modelname": "Model Name", "tags": "Tags", "creator": "Creator", - "hash": "Hash", "title": "Recipe Title", "loraName": "LoRA Filename", "loraModel": "LoRA Model Name", @@ -1837,6 +1836,8 @@ "sendToWorkflow": "Send to ComfyUI", "sendToWorkflowText": "Send to ComfyUI", "copyHash": "Copy hash", + "copyCivitaiId": "Copy Civitai ID", + "civitaiIdCopied": "Civitai ID copied to clipboard", "deleteModelWithShortcut": "Delete model (Del)" }, "openFileLocation": { @@ -1855,6 +1856,8 @@ "baseModel": "Base Model", "size": "Size", "hashes": "Hashes", + "civitaiModelId": "Model ID", + "civitaiVersionId": "Version ID", "unknown": "Unknown", "usageTips": "Usage Tips", "additionalNotes": "Additional Notes", diff --git a/locales/es.json b/locales/es.json index bfc1e6de..65f05b1f 100644 --- a/locales/es.json +++ b/locales/es.json @@ -251,7 +251,6 @@ "modelname": "Nombre del modelo", "tags": "Etiquetas", "creator": "Creador", - "hash": "Hash", "title": "Título de la receta", "loraName": "Nombre de archivo LoRA", "loraModel": "Nombre del modelo LoRA", @@ -1837,6 +1836,8 @@ "sendToWorkflow": "Enviar a ComfyUI", "sendToWorkflowText": "Enviar a ComfyUI", "copyHash": "Copiar hash", + "copyCivitaiId": "Copiar ID de Civitai", + "civitaiIdCopied": "ID de Civitai copiado al portapapeles", "deleteModelWithShortcut": "Eliminar modelo (Del)" }, "openFileLocation": { @@ -1855,6 +1856,8 @@ "baseModel": "Modelo base", "size": "Tamaño", "hashes": "Hashes", + "civitaiModelId": "ID del modelo", + "civitaiVersionId": "ID de versión", "unknown": "Desconocido", "usageTips": "Consejos de uso", "additionalNotes": "Notas adicionales", diff --git a/locales/fr.json b/locales/fr.json index 4dbfb441..0174265a 100644 --- a/locales/fr.json +++ b/locales/fr.json @@ -251,7 +251,6 @@ "modelname": "Nom du modèle", "tags": "Tags", "creator": "Créateur", - "hash": "Hash", "title": "Titre de la recipe", "loraName": "Nom de fichier LoRA", "loraModel": "Nom du modèle LoRA", @@ -1837,6 +1836,8 @@ "sendToWorkflow": "Envoyer vers ComfyUI", "sendToWorkflowText": "Envoyer vers ComfyUI", "copyHash": "Copier le hash", + "copyCivitaiId": "Copier l’ID Civitai", + "civitaiIdCopied": "ID Civitai copié dans le presse-papiers", "deleteModelWithShortcut": "Supprimer le modèle (Del)" }, "openFileLocation": { @@ -1855,6 +1856,8 @@ "baseModel": "Modèle de base", "size": "Taille", "hashes": "Hashes", + "civitaiModelId": "ID du modèle", + "civitaiVersionId": "ID de version", "unknown": "Inconnu", "usageTips": "Conseils d'utilisation", "additionalNotes": "Notes supplémentaires", diff --git a/locales/he.json b/locales/he.json index 7ba591c0..d70d9980 100644 --- a/locales/he.json +++ b/locales/he.json @@ -251,7 +251,6 @@ "modelname": "שם מודל", "tags": "תגיות", "creator": "יוצר", - "hash": "hash", "title": "כותרת מתכון", "loraName": "שם קובץ LoRA", "loraModel": "שם מודל LoRA", @@ -1837,6 +1836,8 @@ "sendToWorkflow": "שלח ל-ComfyUI", "sendToWorkflowText": "שלח ל-ComfyUI", "copyHash": "העתק hash", + "copyCivitaiId": "העתק מזהה Civitai", + "civitaiIdCopied": "מזהה Civitai הועתק ללוח העריכה", "deleteModelWithShortcut": "מחק מודל (Del)" }, "openFileLocation": { @@ -1855,6 +1856,8 @@ "baseModel": "מודל בסיס", "size": "גודל", "hashes": "hashes", + "civitaiModelId": "מזהה מודל", + "civitaiVersionId": "מזהה גרסה", "unknown": "לא ידוע", "usageTips": "טיפים לשימוש", "additionalNotes": "הערות נוספות", diff --git a/locales/ja.json b/locales/ja.json index 521ae066..91a8b874 100644 --- a/locales/ja.json +++ b/locales/ja.json @@ -251,7 +251,6 @@ "modelname": "モデル名", "tags": "タグ", "creator": "作成者", - "hash": "ハッシュ", "title": "レシピタイトル", "loraName": "LoRAファイル名", "loraModel": "LoRAモデル名", @@ -1837,6 +1836,8 @@ "sendToWorkflow": "ComfyUI に送信", "sendToWorkflowText": "ComfyUI に送信", "copyHash": "ハッシュをコピー", + "copyCivitaiId": "Civitai IDをコピー", + "civitaiIdCopied": "Civitai IDをクリップボードにコピーしました", "deleteModelWithShortcut": "モデルを削除（Del）" }, "openFileLocation": { @@ -1855,6 +1856,8 @@ "baseModel": "ベースモデル", "size": "サイズ", "hashes": "ハッシュ", + "civitaiModelId": "モデル ID", + "civitaiVersionId": "バージョン ID", "unknown": "不明", "usageTips": "使用のヒント", "additionalNotes": "追加メモ", diff --git a/locales/ko.json b/locales/ko.json index fb197ae7..3c034f75 100644 --- a/locales/ko.json +++ b/locales/ko.json @@ -251,7 +251,6 @@ "modelname": "모델명", "tags": "태그", "creator": "제작자", - "hash": "해시", "title": "레시피 제목", "loraName": "LoRA 파일명", "loraModel": "LoRA 모델명", @@ -1837,6 +1836,8 @@ "sendToWorkflow": "ComfyUI로 보내기", "sendToWorkflowText": "ComfyUI로 보내기", "copyHash": "해시 복사", + "copyCivitaiId": "Civitai ID 복사", + "civitaiIdCopied": "Civitai ID가 클립보드에 복사되었습니다", "deleteModelWithShortcut": "모델 삭제(Del)" }, "openFileLocation": { @@ -1855,6 +1856,8 @@ "baseModel": "베이스 모델", "size": "크기", "hashes": "해시", + "civitaiModelId": "모델 ID", + "civitaiVersionId": "버전 ID", "unknown": "알 수 없음", "usageTips": "사용 팁", "additionalNotes": "추가 메모", diff --git a/locales/ru.json b/locales/ru.json index 353085ae..1a1a975c 100644 --- a/locales/ru.json +++ b/locales/ru.json @@ -251,7 +251,6 @@ "modelname": "Название модели", "tags": "Теги", "creator": "Автор", - "hash": "Хэш", "title": "Название рецепта", "loraName": "Имя файла LoRA", "loraModel": "Название модели LoRA", @@ -1837,6 +1836,8 @@ "sendToWorkflow": "Отправить в ComfyUI", "sendToWorkflowText": "Отправить в ComfyUI", "copyHash": "Копировать хеш", + "copyCivitaiId": "Копировать ID Civitai", + "civitaiIdCopied": "ID Civitai скопирован в буфер обмена", "deleteModelWithShortcut": "Удалить модель (Del)" }, "openFileLocation": { @@ -1855,6 +1856,8 @@ "baseModel": "Базовая модель", "size": "Размер", "hashes": "Хэши", + "civitaiModelId": "ID модели", + "civitaiVersionId": "ID версии", "unknown": "Неизвестно", "usageTips": "Советы по использованию", "additionalNotes": "Дополнительные заметки", diff --git a/locales/zh-CN.json b/locales/zh-CN.json index 01d30be8..db9c25ba 100644 --- a/locales/zh-CN.json +++ b/locales/zh-CN.json @@ -251,7 +251,6 @@ "modelname": "模型名称", "tags": "标签", "creator": "创作者", - "hash": "哈希", "title": "配方标题", "loraName": "LoRA 文件名", "loraModel": "LoRA 模型名称", @@ -1837,6 +1836,8 @@ "sendToWorkflow": "发送到 ComfyUI", "sendToWorkflowText": "发送到 ComfyUI", "copyHash": "复制哈希值", + "copyCivitaiId": "复制 Civitai ID", + "civitaiIdCopied": "Civitai ID 已复制到剪贴板", "deleteModelWithShortcut": "删除模型（Del）" }, "openFileLocation": { @@ -1855,6 +1856,8 @@ "baseModel": "基础模型", "size": "大小", "hashes": "哈希值", + "civitaiModelId": "模型 ID", + "civitaiVersionId": "版本 ID", "unknown": "未知", "usageTips": "使用提示", "additionalNotes": "附加备注", diff --git a/locales/zh-TW.json b/locales/zh-TW.json index 4454e8db..f9689df6 100644 --- a/locales/zh-TW.json +++ b/locales/zh-TW.json @@ -251,7 +251,6 @@ "modelname": "模型名稱", "tags": "標籤", "creator": "創作者", - "hash": "雜湊", "title": "配方標題", "loraName": "LoRA 檔案名稱", "loraModel": "LoRA 模型名稱", @@ -1837,6 +1836,8 @@ "sendToWorkflow": "傳送到 ComfyUI", "sendToWorkflowText": "傳送到 ComfyUI", "copyHash": "複製雜湊值", + "copyCivitaiId": "複製 Civitai ID", + "civitaiIdCopied": "Civitai ID 已複製到剪貼簿", "deleteModelWithShortcut": "刪除模型（Del）" }, "openFileLocation": { @@ -1855,6 +1856,8 @@ "baseModel": "基礎模型", "size": "大小", "hashes": "雜湊值", + "civitaiModelId": "模型 ID", + "civitaiVersionId": "版本 ID", "unknown": "未知", "usageTips": "使用提示", "additionalNotes": "附加備註", diff --git a/py/routes/handlers/model_handlers.py b/py/routes/handlers/model_handlers.py index a2287ef7..d2fe867b 100644 --- a/py/routes/handlers/model_handlers.py +++ b/py/routes/handlers/model_handlers.py @@ -387,7 +387,6 @@ class ModelListingHandler: == "true", "tags": request.query.get("search_tags", "false").lower() == "true", "creator": request.query.get("search_creator", "false").lower() == "true", - "hash": request.query.get("search_hash", "false").lower() == "true", "recursive": request.query.get("recursive", "true").lower() == "true", } diff --git a/py/services/model_query.py b/py/services/model_query.py index 29d5c544..a15ea05e 100644 --- a/py/services/model_query.py +++ b/py/services/model_query.py @@ -432,7 +432,6 @@ class SearchStrategy: "tags": False, "recursive": True, "creator": False, - "hash": False, } def __init__( @@ -495,13 +494,14 @@ class SearchStrategy: results.append(item) continue - # Hash search is always exact (never fuzzy): match the full - # sha256, its autov2 prefix (first 10 chars), or the autov3 hash. - if options.get("hash", False): - hash_query = search_lower.strip() - if hash_query and self._matches_hash(item, hash_query): - results.append(item) - continue + # Hash/id search is always exact (never fuzzy) and always on: it + # matches the full sha256, its autov2 prefix (first 10 chars), + # the autov3 hash, or the Civitai model/version ids. Exact-match + # semantics mean it adds no noise to ordinary keyword searches. + hash_query = search_lower.strip() + if hash_query and self._matches_hash(item, hash_query): + results.append(item) + continue return results @@ -515,6 +515,18 @@ class SearchStrategy: autov3 = item.get("autov3") if isinstance(autov3, str) and autov3 and hash_query == autov3.lower(): return True + civitai = item.get("civitai") + if isinstance(civitai, dict): + # A model card corresponds to one Civitai version: `modelId` is + # the model id (may match several cards when the library holds + # multiple versions), `id` is the version id (unique per card). + for key in ("modelId", "model_id", "id"): + value = civitai.get(key) + if value is None: + continue + value_str = str(value).strip() + if value_str and value_str != "0" and hash_query == value_str: + return True return False def _matches( diff --git a/static/css/components/lora-modal/lora-modal.css b/static/css/components/lora-modal/lora-modal.css index d5fcca7e..4604fea2 100644 --- a/static/css/components/lora-modal/lora-modal.css +++ b/static/css/components/lora-modal/lora-modal.css @@ -292,6 +292,15 @@ font-size: 0.8em; } +/* Civitai model/version ids — pushed to the right end of the footnote line */ +.hash-footnote .civitai-id-group { + display: inline-flex; + align-items: baseline; + flex-wrap: wrap; + gap: 4px 8px; + margin-left: auto; +} + .hash-footnote .hash-copy-btn { display: inline-flex; align-items: center; diff --git a/static/js/api/baseModelApi.js b/static/js/api/baseModelApi.js index 28aa6867..5030869d 100644 --- a/static/js/api/baseModelApi.js +++ b/static/js/api/baseModelApi.js @@ -1566,9 +1566,6 @@ export class BaseModelApiClient { if (pageState.searchOptions.creator !== undefined) { params.append('search_creator', pageState.searchOptions.creator.toString()); } - if (pageState.searchOptions.hash !== undefined) { - params.append('search_hash', pageState.searchOptions.hash.toString()); - } } } diff --git a/static/js/components/shared/ModelModal.js b/static/js/components/shared/ModelModal.js index 73a84515..e66c480d 100644 --- a/static/js/components/shared/ModelModal.js +++ b/static/js/components/shared/ModelModal.js @@ -355,7 +355,9 @@ export async function showModelModal(model, modelType) { const escapedFolderPath = escapeHtml((modelWithFullData.file_path || '').replace(/[^/]+$/, '') || 'N/A'); // De-emphasized hash display: a borderless full-width footnote line below // the info grid — sha256 middle-truncated (first 10 + last 6), autov3 in - // full (12 chars); the full value is copied via data-hash. + // full (12 chars); the full value is copied via data-hash. Civitai model / + // version ids join the same line on the right when the model comes from + // the Civitai ecosystem (a model card corresponds to one Civitai version). const modelSha256 = modelWithFullData.sha256 || ''; const modelAutov3 = modelWithFullData.autov3 || ''; const truncatedSha256 = modelSha256.length > 16 @@ -383,8 +385,47 @@ export async function showModelModal(model, modelType) { `); } - const hashesMarkup = modelSha256 && hashEntries.length ? ` -
${hashEntries.join('·')} + const normalizeCivitaiId = (value) => { + if (value === undefined || value === null) return ''; + const normalized = String(value).trim(); + // "0" is used as a placeholder for unknown ids in some metadata sources + return normalized && normalized !== '0' ? normalized : ''; + }; + const civitaiInfo = modelWithFullData.civitai || {}; + const footnoteModelId = normalizeCivitaiId(civitaiInfo.modelId ?? civitaiInfo.model_id); + const footnoteVersionId = normalizeCivitaiId(civitaiInfo.id); + const copyCivitaiIdTitle = translate('modals.model.actions.copyCivitaiId', {}, 'Copy Civitai ID'); + const civitaiIdCopiedToast = escapeAttribute(translate('modals.model.actions.civitaiIdCopied', {}, 'Civitai ID copied to clipboard')); + const civitaiIdEntries = []; + if (footnoteModelId) { + civitaiIdEntries.push(` + + ${translate('modals.model.metadata.civitaiModelId', {}, 'Model ID')} + ${escapeHtml(footnoteModelId)} + + `); + } + if (footnoteVersionId) { + civitaiIdEntries.push(` + + ${translate('modals.model.metadata.civitaiVersionId', {}, 'Version ID')} + ${escapeHtml(footnoteVersionId)} + + `); + } + const footnoteParts = []; + if (hashEntries.length) { + footnoteParts.push(hashEntries.join('·')); + } + if (civitaiIdEntries.length) { + footnoteParts.push(`${civitaiIdEntries.join('·')}`); + } + const hashesMarkup = footnoteParts.length ? ` +
${footnoteParts.join('')}
` : ''; const useNewIcons = state.global.settings.use_new_license_icons !== false; const licenseIcons = useNewIcons @@ -975,7 +1016,7 @@ function setupEventHandlers(filePath, modelType) { break; case 'copy-hash': if (target.dataset.hash) { - copyToClipboard(target.dataset.hash, 'Hash copied to clipboard'); + copyToClipboard(target.dataset.hash, target.dataset.toast || 'Hash copied to clipboard'); } break; } diff --git a/static/js/managers/SearchManager.js b/static/js/managers/SearchManager.js index c610618d..4929caf8 100644 --- a/static/js/managers/SearchManager.js +++ b/static/js/managers/SearchManager.js @@ -304,7 +304,6 @@ export class SearchManager { pageState.searchOptions.modelname = options.modelname || false; pageState.searchOptions.tags = options.tags || false; pageState.searchOptions.creator = options.creator || false; - pageState.searchOptions.hash = options.hash || false; } } diff --git a/static/js/state/index.js b/static/js/state/index.js index acf76dd3..8442e793 100644 --- a/static/js/state/index.js +++ b/static/js/state/index.js @@ -123,7 +123,6 @@ export const state = { modelname: true, tags: false, creator: false, - hash: false, recursive: getStorageItem(`${MODEL_TYPES.LORA}_recursiveSearch`, true), }, filters: { @@ -190,7 +189,6 @@ export const state = { filename: true, modelname: true, creator: false, - hash: false, recursive: getStorageItem(`${MODEL_TYPES.CHECKPOINT}_recursiveSearch`, true), }, filters: { @@ -230,7 +228,6 @@ export const state = { modelname: true, tags: false, creator: false, - hash: false, recursive: getStorageItem(`${MODEL_TYPES.EMBEDDING}_recursiveSearch`, true), }, filters: { @@ -268,7 +265,6 @@ export const state = { modelname: true, tags: false, creator: false, - hash: false, recursive: getStorageItem(`${MODEL_TYPES.OTHER}_recursiveSearch`, true), }, filters: { diff --git a/templates/components/header.html b/templates/components/header.html index be61399b..de9d5a36 100644 --- a/templates/components/header.html +++ b/templates/components/header.html @@ -203,20 +203,17 @@
{{ t('header.search.filters.modelname') }}
{{ t('header.search.filters.tags') }}
{{ t('header.search.filters.creator') }}
-
{{ t('header.search.filters.hash') }}
{% elif request.path == '/embeddings' or request.path == '/other' %}
{{ t('header.search.filters.filename') }}
{{ t('header.search.filters.modelname') }}
{{ t('header.search.filters.tags') }}
{{ t('header.search.filters.creator') }}
-
{{ t('header.search.filters.hash') }}
{% else %}
{{ t('header.search.filters.filename') }}
{{ t('header.search.filters.modelname') }}
{{ t('header.search.filters.tags') }}
{{ t('header.search.filters.creator') }}
-
{{ t('header.search.filters.hash') }}
{% endif %}
diff --git a/tests/frontend/api/baseModelApi.searchParams.test.js b/tests/frontend/api/baseModelApi.searchParams.test.js deleted file mode 100644 index f1977d1f..00000000 --- a/tests/frontend/api/baseModelApi.searchParams.test.js +++ /dev/null @@ -1,120 +0,0 @@ -import { describe, it, expect, vi } from 'vitest'; - -const { - BASE_MODEL_API_MODULE, - STATE_MODULE, - UI_HELPERS_MODULE, - I18N_MODULE, - STORAGE_MODULE, - API_CONFIG_MODULE, - API_FACTORY_MODULE, - SIDEBAR_MANAGER_MODULE, -} = vi.hoisted(() => ({ - BASE_MODEL_API_MODULE: new URL('../../../static/js/api/baseModelApi.js', import.meta.url).pathname, - STATE_MODULE: new URL('../../../static/js/state/index.js', import.meta.url).pathname, - UI_HELPERS_MODULE: new URL('../../../static/js/utils/uiHelpers.js', import.meta.url).pathname, - I18N_MODULE: new URL('../../../static/js/utils/i18nHelpers.js', import.meta.url).pathname, - STORAGE_MODULE: new URL('../../../static/js/utils/storageHelpers.js', import.meta.url).pathname, - API_CONFIG_MODULE: new URL('../../../static/js/api/apiConfig.js', import.meta.url).pathname, - API_FACTORY_MODULE: new URL('../../../static/js/api/modelApiFactory.js', import.meta.url).pathname, - SIDEBAR_MANAGER_MODULE: new URL('../../../static/js/components/SidebarManager.js', import.meta.url).pathname, -})); - -vi.mock(STATE_MODULE, () => ({ - state: { - global: { settings: {} }, - }, - getCurrentPageState: vi.fn(() => ({})), -})); - -vi.mock(UI_HELPERS_MODULE, () => ({ - showToast: vi.fn(), -})); - -vi.mock(I18N_MODULE, () => ({ - translate: vi.fn((key) => key), -})); - -vi.mock(STORAGE_MODULE, () => ({ - getStorageItem: vi.fn(), - getSessionItem: vi.fn(() => null), - removeSessionItem: vi.fn(), - saveMapToStorage: vi.fn(), -})); - -vi.mock(API_CONFIG_MODULE, () => ({ - getCompleteApiConfig: vi.fn(() => ({ - endpoints: {}, - config: { displayName: 'LoRA', singularName: 'LoRA', supportsLetterFilter: false }, - })), - getCurrentModelType: vi.fn(() => 'loras'), - isValidModelType: vi.fn(() => true), - DOWNLOAD_ENDPOINTS: {}, - HF_ENDPOINTS: {}, - WS_ENDPOINTS: {}, -})); - -vi.mock(API_FACTORY_MODULE, () => ({ - resetAndReload: vi.fn(), -})); - -vi.mock(SIDEBAR_MANAGER_MODULE, () => ({ - sidebarManager: { refresh: vi.fn() }, -})); - -async function createClient() { - const { BaseModelApiClient } = await import(BASE_MODEL_API_MODULE); - class TestClient extends BaseModelApiClient {} - return new TestClient('loras'); -} - -function makePageState(searchOptions) { - return { - viewMode: 'active', - activeFolder: null, - showFavoritesOnly: false, - showUpdateAvailableOnly: false, - filters: { search: 'abc123' }, - searchOptions: { - filename: true, - modelname: true, - tags: false, - creator: false, - recursive: true, - ...searchOptions, - }, - }; -} - -describe('BaseModelApiClient._buildQueryParams hash search option', () => { - it('appends search_hash=true when the hash option is enabled', async () => { - const client = await createClient(); - const params = client._buildQueryParams({}, makePageState({ hash: true })); - - expect(params.get('search_hash')).toBe('true'); - expect(params.get('search')).toBe('abc123'); - }); - - it('appends search_hash=false when the hash option is disabled', async () => { - const client = await createClient(); - const params = client._buildQueryParams({}, makePageState({ hash: false })); - - expect(params.get('search_hash')).toBe('false'); - }); - - it('omits search_hash when the option is absent (backend defaults to false)', async () => { - const client = await createClient(); - const params = client._buildQueryParams({}, makePageState({})); - - expect(params.get('search_hash')).toBeNull(); - }); - - it('does not send search_hash without an active search term', async () => { - const client = await createClient(); - const pageState = makePageState({ hash: true }); - pageState.filters.search = ''; - const params = client._buildQueryParams({}, pageState); - - expect(params.get('search_hash')).toBeNull(); - }); -}); diff --git a/tests/frontend/components/modelModal.hashes.test.js b/tests/frontend/components/modelModal.hashes.test.js index cb4ce66b..85d70430 100644 --- a/tests/frontend/components/modelModal.hashes.test.js +++ b/tests/frontend/components/modelModal.hashes.test.js @@ -165,12 +165,76 @@ describe('Model modal hash rendering', () => { expect(rows[0].querySelector('.hash-kind').textContent).toBe('SHA256'); }); - it('hides the hashes item entirely when sha256 is empty', async () => { + it('renders autov3 alone when sha256 is empty', async () => { await renderModal(makeModel({ sha256: '', autov3: AUTOV3 })); + const rows = document.querySelectorAll('.hash-footnote .hash-entry'); + expect(rows).toHaveLength(1); + expect(rows[0].querySelector('.hash-kind').textContent).toBe('AutoV3'); + }); + + it('hides the footnote entirely when there are no hashes and no civitai ids', async () => { + await renderModal(makeModel({ sha256: '', autov3: '', civitai: {} })); + expect(document.querySelector('.hash-footnote')).toBeNull(); }); + it('renders civitai model and version ids with copy buttons', async () => { + await renderModal(makeModel({ civitai: { modelId: 12345, id: 67890 } })); + + const group = document.querySelector('.hash-footnote .civitai-id-group'); + expect(group).not.toBeNull(); + + const entries = group.querySelectorAll('.civitai-id-entry'); + expect(entries).toHaveLength(2); + + expect(entries[0].querySelector('.hash-kind').textContent).toBe('Model ID'); + expect(entries[0].querySelector('.model-hash-value').textContent).toBe('12345'); + expect(entries[0].querySelector('[data-action="copy-hash"]').dataset.hash).toBe('12345'); + + expect(entries[1].querySelector('.hash-kind').textContent).toBe('Version ID'); + expect(entries[1].querySelector('.model-hash-value').textContent).toBe('67890'); + expect(entries[1].querySelector('[data-action="copy-hash"]').dataset.hash).toBe('67890'); + }); + + it('hides civitai ids for models without civitai data', async () => { + await renderModal(makeModel({ civitai: {} })); + + expect(document.querySelector('.hash-footnote .civitai-id-group')).toBeNull(); + }); + + it.each([0, '0', null, undefined, ''])('treats civitai id %s as missing', async (missingId) => { + await renderModal(makeModel({ civitai: { modelId: missingId, id: missingId } })); + + expect(document.querySelector('.hash-footnote .civitai-id-group')).toBeNull(); + }); + + it('falls back to snake_case model_id', async () => { + await renderModal(makeModel({ civitai: { model_id: 777, id: 888 } })); + + const group = document.querySelector('.hash-footnote .civitai-id-group'); + const entries = group.querySelectorAll('.civitai-id-entry'); + expect(entries[0].querySelector('.model-hash-value').textContent).toBe('777'); + expect(entries[1].querySelector('.model-hash-value').textContent).toBe('888'); + }); + + it('still renders the footnote when only civitai ids exist', async () => { + await renderModal(makeModel({ sha256: '', autov3: '', civitai: { modelId: 12345, id: 67890 } })); + + const footnote = document.querySelector('.hash-footnote'); + expect(footnote).not.toBeNull(); + expect(footnote.querySelectorAll('.civitai-id-entry')).toHaveLength(2); + }); + + it('copies the version id with the civitai toast when its copy button is clicked', async () => { + await renderModal(makeModel({ civitai: { modelId: 12345, id: 67890 } })); + + const entries = document.querySelectorAll('.civitai-id-entry [data-action="copy-hash"]'); + entries[1].click(); + + expect(copyToClipboard).toHaveBeenCalledWith('67890', 'Civitai ID copied to clipboard'); + }); + it('copies the full hash when the copy button is clicked', async () => { await renderModal(makeModel()); diff --git a/tests/services/test_model_query_search.py b/tests/services/test_model_query_search.py index 89f0aa9a..34934609 100644 --- a/tests/services/test_model_query_search.py +++ b/tests/services/test_model_query_search.py @@ -13,20 +13,13 @@ SHA256 = "abcdef1234567890" + "f" * 48 # 64-char hex AUTOV2 = SHA256[:10] AUTOV3 = "0123456789ab" +# Hash/id exact matching is always on; these options only disable the +# scope-based branches so tests exercise the hash branch in isolation. HASH_ONLY_OPTIONS = { "filename": False, "modelname": False, "tags": False, "creator": False, - "hash": True, -} - -HASH_OFF_OPTIONS = { - "filename": False, - "modelname": False, - "tags": False, - "creator": False, - "hash": False, } @@ -88,9 +81,11 @@ class TestSearchStrategyHash: item = make_item(autov3="") assert strategy.apply([item], AUTOV3, HASH_ONLY_OPTIONS) == [] - def test_hash_option_disabled(self, strategy): - assert strategy.apply([make_item()], SHA256, HASH_OFF_OPTIONS) == [] - assert strategy.apply([make_item()], AUTOV3, HASH_OFF_OPTIONS) == [] + def test_hash_matching_is_always_on(self, strategy): + # There is no hash option anymore: exact hash/id matching happens + # regardless of the search scope options. + assert len(strategy.apply([make_item()], SHA256, HASH_ONLY_OPTIONS)) == 1 + assert len(strategy.apply([make_item()], AUTOV3, HASH_ONLY_OPTIONS)) == 1 def test_fuzzy_mode_still_exact(self, strategy): # Fuzzy matching must never apply to the hash field @@ -104,6 +99,48 @@ class TestSearchStrategyHash: assert strategy.apply([item], SHA256, HASH_ONLY_OPTIONS) == [] +class TestSearchStrategyCivitaiIds: + """Hash search also exact-matches Civitai model/version ids.""" + + def test_version_id_matches(self, strategy): + item = make_item(civitai={"modelId": 12345, "id": 67890}) + assert len(strategy.apply([item], "67890", HASH_ONLY_OPTIONS)) == 1 + + def test_model_id_matches(self, strategy): + item = make_item(civitai={"modelId": 12345, "id": 67890}) + assert len(strategy.apply([item], "12345", HASH_ONLY_OPTIONS)) == 1 + + def test_model_id_matches_all_versions_of_the_model(self, strategy): + v1 = make_item(file_name="v1.safetensors", civitai={"modelId": 12345, "id": 111}) + v2 = make_item(file_name="v2.safetensors", civitai={"modelId": 12345, "id": 222}) + other = make_item(file_name="other.safetensors", civitai={"modelId": 999, "id": 333}) + result = strategy.apply([v1, v2, other], "12345", HASH_ONLY_OPTIONS) + assert {r["file_name"] for r in result} == {"v1.safetensors", "v2.safetensors"} + + def test_snake_case_model_id_matches(self, strategy): + item = make_item(civitai={"model_id": 12345, "id": 67890}) + assert len(strategy.apply([item], "12345", HASH_ONLY_OPTIONS)) == 1 + + def test_string_ids_match(self, strategy): + item = make_item(civitai={"modelId": "12345", "id": "67890"}) + assert len(strategy.apply([item], "67890", HASH_ONLY_OPTIONS)) == 1 + + def test_unknown_id_does_not_match(self, strategy): + item = make_item(civitai={"modelId": 12345, "id": 67890}) + assert strategy.apply([item], "555", HASH_ONLY_OPTIONS) == [] + + @pytest.mark.parametrize("missing", [0, "0", None, ""]) + def test_placeholder_ids_do_not_match(self, strategy, missing): + item = make_item(sha256="", autov3=None, civitai={"modelId": missing, "id": missing}) + assert strategy.apply([item], "0", HASH_ONLY_OPTIONS) == [] + + def test_missing_civitai_data_does_not_match(self, strategy): + item = make_item(sha256="", autov3=None, civitai=None) + assert strategy.apply([item], "12345", HASH_ONLY_OPTIONS) == [] + item = make_item(sha256="", autov3=None) + assert strategy.apply([item], "12345", HASH_ONLY_OPTIONS) == [] + + class TestFormatResponseAutov3: """format_response should pass the autov3 field through unchanged.""" From 896ce5eddb0eb1ac91d30dd276bbcd622a56439e Mon Sep 17 00:00:00 2001 From: Will Miao Date: Sat, 3 Oct 2026 14:31:10 +0800 Subject: [PATCH 2/2] perf(rename): make bulk filename-template apply O(n) instead of O(n^2) Applying a filename template to a large library re-did O(library) work for every renamed file: a full natsort resort plus whole-table SQLite rewrite and download-history resync after each rename, and a full scan plus resort of the entire recipe collection per renamed LoRA. On a 20k-model library with 300k recipes on a HDD this pushed "Apply to Library" into multi-day runs. - ModelScanner.defer_cache_persist(): bulk loops update the in-memory entry and indexes only; resort + persist + download-history sync run once at context exit, forced even on cancellation/error since files are already renamed on disk. Single-rename callers keep immediate per-call behavior. - RecipeScanner.build_lora_hash_index(): one-shot hash -> recipes index so per-file lookups are O(1); update_lora_filename_by_hash gains hash_index / defer_maintenance params, with a single finalize_bulk_filename_updates() resort at the end of a bulk session. - ModelLifecycleService.bulk_rename_session() / BulkRenameContext wire the deferred path through rename_model (hash index built lazily on first recipe-touching rename). - Blocking os.rename sequence offloaded via asyncio.to_thread so one file's HDD I/O no longer stalls the event loop (no cross-file parallelism). - Skip logic, per-batch WebSocket progress, cancellation, and result counters unchanged. --- py/services/model_lifecycle_service.py | 136 +++- py/services/model_scanner.py | 133 +++- py/services/recipe_scanner.py | 72 +- .../use_cases/filename_template_use_case.py | 36 +- tests/conftest.py | 7 +- tests/services/test_bulk_rename_apply.py | 639 ++++++++++++++++++ tests/services/test_recipe_scanner.py | 109 +++ tests/services/test_use_cases.py | 11 +- 8 files changed, 1080 insertions(+), 63 deletions(-) create mode 100644 tests/services/test_bulk_rename_apply.py diff --git a/py/services/model_lifecycle_service.py b/py/services/model_lifecycle_service.py index f5c8fb4b..b6a3fb4c 100644 --- a/py/services/model_lifecycle_service.py +++ b/py/services/model_lifecycle_service.py @@ -2,10 +2,12 @@ from __future__ import annotations +import asyncio import json import logging import os -from typing import Any, Awaitable, Callable, Dict, Iterable, List, Mapping, Optional, TYPE_CHECKING, cast +from contextlib import asynccontextmanager +from typing import Any, AsyncIterator, Awaitable, Callable, Dict, Iterable, List, Mapping, Optional, TYPE_CHECKING, cast from ..services.service_registry import ServiceRegistry from ..services.pending_delete_service import get_pending_delete_service @@ -107,6 +109,36 @@ def _require_path_in_library_roots(file_path: str, scanner, *, label: str = "pat ) +class BulkRenameContext: + """Per-session state threaded through ``rename_model`` calls of a bulk rename. + + Holds the lazily built recipe hash index so a bulk rename loop pays the + O(recipes) index build at most once (on the first recipe-touching rename) + instead of rescanning every recipe per renamed file. Also tracks whether + any recipe was re-pointed so the session finalizes recipe maintenance only + when needed. + """ + + def __init__(self, recipe_scanner: Any) -> None: + self._recipe_scanner = recipe_scanner + self._recipe_hash_index: Optional[Dict[str, List[Dict[str, Any]]]] = None + self.recipes_touched = False + + @property + def recipe_scanner(self) -> Any: + return self._recipe_scanner + + async def get_recipe_hash_index(self) -> Optional[Dict[str, List[Dict[str, Any]]]]: + """Return the lora-hash → recipes index, building it on first use.""" + if self._recipe_scanner is None: + return None + if self._recipe_hash_index is None: + self._recipe_hash_index = ( + await self._recipe_scanner.build_lora_hash_index() + ) + return self._recipe_hash_index + + class ModelLifecycleService: """Co-ordinate destructive and mutating model operations.""" @@ -365,10 +397,45 @@ class ModelLifecycleService: return await self._scanner.bulk_delete_models(file_paths) + @asynccontextmanager + async def bulk_rename_session(self) -> AsyncIterator[BulkRenameContext]: + """Context for bulk rename loops (filename-template "Apply to Library"). + + While active, the per-file ``update_single_model_cache`` resort/persist + chain and the per-file recipe folder-metadata refresh/resort are + deferred; both run exactly once when the outermost session exits — see + ``ModelScanner.defer_cache_persist`` and + ``RecipeScanner.finalize_bulk_filename_updates``. The finalize steps run + even on cancellation or mid-loop errors, because files are already + renamed on disk and the caches must not be left diverging. + + Yields a :class:`BulkRenameContext` to pass as ``bulk_context`` into + each ``rename_model`` call of the loop. + """ + recipe_scanner = await self._recipe_scanner_factory() + context = BulkRenameContext(recipe_scanner) + async with self._scanner.defer_cache_persist(): + try: + yield context + finally: + if recipe_scanner is not None and context.recipes_touched: + try: + await recipe_scanner.finalize_bulk_filename_updates() + except Exception as exc: # pragma: no cover - defensive logging + logger.error( + "Error finalizing bulk recipe updates: %s", exc + ) + async def rename_model( - self, *, file_path: str, new_file_name: str + self, *, file_path: str, new_file_name: str, bulk_context: Optional[BulkRenameContext] = None ) -> Dict[str, object]: - """Rename a model and its companion artefacts.""" + """Rename a model and its companion artefacts. + + When ``bulk_context`` is given (bulk rename loop), the recipe + re-pointing uses the session's prebuilt hash index and defers recipe + maintenance to the session finalize; the scanner cache persist is + likewise deferred by the surrounding ``bulk_rename_session``. + """ if not file_path or not new_file_name: raise ValueError("File path and new file name are required") @@ -419,20 +486,11 @@ class ModelLifecycleService: raw_hash = metadata.get("sha256") if isinstance(metadata, dict) else None hash_value = raw_hash if isinstance(raw_hash, str) else None - renamed_files: List[str] = [] - new_metadata_path: Optional[str] = None new_preview: Optional[str] = None - for old_path, pattern in existing_files: - ext = self._get_multipart_ext(pattern) - new_path = os.path.join( - os.path.dirname(old_path), f"{new_file_name}{ext}" - ).replace(os.sep, "/") - os.rename(old_path, new_path) - renamed_files.append(new_path) - - if ext == ".metadata.json": - new_metadata_path = new_path + renamed_files, new_metadata_path = await asyncio.to_thread( + self._rename_companion_files, existing_files, new_file_name + ) if metadata and new_metadata_path: metadata["file_name"] = new_file_name @@ -457,12 +515,26 @@ class ModelLifecycleService: ) if hash_value and getattr(self._scanner, "model_type", "") == "lora": - recipe_scanner = await self._recipe_scanner_factory() + if bulk_context is not None: + recipe_scanner = bulk_context.recipe_scanner + hash_index = await bulk_context.get_recipe_hash_index() + defer_maintenance = True + else: + recipe_scanner = await self._recipe_scanner_factory() + hash_index = None + defer_maintenance = False if recipe_scanner: try: - await recipe_scanner.update_lora_filename_by_hash( - hash_value, new_file_name + file_count, cache_count = ( + await recipe_scanner.update_lora_filename_by_hash( + hash_value, + new_file_name, + hash_index=hash_index, + defer_maintenance=defer_maintenance, + ) ) + if bulk_context is not None and (file_count or cache_count): + bulk_context.recipes_touched = True except Exception as exc: # pragma: no cover - defensive logging logger.error( "Error updating recipe references for %s: %s", @@ -478,6 +550,34 @@ class ModelLifecycleService: "reload_required": False, } + def _rename_companion_files( + self, + existing_files: List[tuple[str, str]], + new_file_name: str, + ) -> tuple[List[str], Optional[str]]: + """Rename all companion files, off the event loop thread. + + Runs the blocking ``os.rename`` sequence for one model in a worker + thread so a single file's HDD I/O does not stall the event loop. + Never parallelized across files: one model's renames stay sequential + and the helper holds no locks. + """ + renamed_files: List[str] = [] + new_metadata_path: Optional[str] = None + + for old_path, pattern in existing_files: + ext = self._get_multipart_ext(pattern) + new_path = os.path.join( + os.path.dirname(old_path), f"{new_file_name}{ext}" + ).replace(os.sep, "/") + os.rename(old_path, new_path) + renamed_files.append(new_path) + + if ext == ".metadata.json": + new_metadata_path = new_path + + return renamed_files, new_metadata_path + @staticmethod def _get_multipart_ext(filename: str) -> str: """Return the extension for files with compound suffixes.""" diff --git a/py/services/model_scanner.py b/py/services/model_scanner.py index bf5d430e..7724e1cb 100644 --- a/py/services/model_scanner.py +++ b/py/services/model_scanner.py @@ -4,6 +4,7 @@ import logging import asyncio import time import shutil +from contextlib import asynccontextmanager from dataclasses import dataclass from typing import Any, Awaitable, Callable, Dict, List, Mapping, Optional, Sequence, Set, Tuple, Type, Union, cast @@ -162,6 +163,13 @@ class ModelScanner: self._name_display_mode = self._resolve_name_display_mode() self._cancel_requested = False # Flag for cancellation self._move_locks: Dict[str, asyncio.Lock] = {} # Per-source-file move locks + # Bulk-operation deferral: while _defer_persist_depth > 0, + # update_single_model_cache() skips the per-call resort/persist and + # only marks _deferred_persist_pending; the exit of the outermost + # defer_cache_persist() context finalizes once (see + # _finalize_deferred_cache_persist). + self._defer_persist_depth = 0 + self._deferred_persist_pending = False self._autov3_backfill_scheduled = False # One-time AutoV3 backfill trigger per process # Guard against concurrent all-folders backfill walks (cold fallback # for persisted snapshots that predate folder recording). @@ -773,11 +781,11 @@ class ModelScanner: except Exception as exc: logger.warning("AutoV3 backfill failed: %s", exc) - async def _save_persistent_cache(self, scan_result: CacheBuildResult) -> None: + async def _save_persistent_cache(self, scan_result: CacheBuildResult, *, force: bool = False) -> None: if not scan_result or not getattr(self, '_persistent_cache', None): return - if self.is_cancelled(): + if self.is_cancelled() and not force: logger.info( f"{self.model_type.capitalize()} Scanner: Skipping _save_persistent_cache " "after cancellation" @@ -836,7 +844,7 @@ class ModelScanner: bucket.append(path) return snapshot - async def _persist_current_cache(self) -> None: + async def _persist_current_cache(self, *, force: bool = False) -> None: if self._cache is None or not getattr(self, '_persistent_cache', None): return @@ -851,7 +859,7 @@ class ModelScanner: else None ), ) - await self._save_persistent_cache(snapshot) + await self._save_persistent_cache(snapshot, force=force) await self._sync_download_history(snapshot.raw_data, source='scan') def _count_model_files(self) -> int: """Count all model files with supported extensions in all roots @@ -2646,11 +2654,85 @@ class ModelScanner: logger.error(f"Error updating metadata paths: {e}", exc_info=True) return None + @asynccontextmanager + async def defer_cache_persist(self): + """Defer heavyweight cache maintenance for a bulk operation. + + While at least one ``defer_cache_persist`` context is active, + :meth:`update_single_model_cache` performs only the in-memory entry + swap plus incremental index updates — it skips the full version-index + rebuild, the natsort resort, and the whole-table SQLite persist plus + download-history sync that normally run per call. When the outermost + context exits, the pending maintenance runs **once** (resort, persist, + download-history sync). + + The final persist is forced: it runs even when the scanner's + cancellation flag is set or the wrapped block raised, because callers + use this around operations that already mutated files on disk and the + cache must not be left diverging from reality. + + Intended for bulk rename/move loops (e.g. the filename-template "Apply + to Library" flow). Single-shot callers keep the immediate per-call + behavior by not entering this context. + """ + self._defer_persist_depth = getattr(self, "_defer_persist_depth", 0) + 1 + try: + yield + finally: + self._defer_persist_depth -= 1 + if self._defer_persist_depth == 0: + await self._finalize_deferred_cache_persist() + + @property + def _cache_persist_deferred(self) -> bool: + """True while cache resort/persist is deferred to a bulk finalize.""" + return getattr(self, "_defer_persist_depth", 0) > 0 + + async def _finalize_deferred_cache_persist(self) -> None: + """Run the resort + persist deferred by ``defer_cache_persist``. + + Best-effort: failures are logged, never raised, so an error here + cannot mask the outcome of the bulk operation itself (including + cancellation). + """ + if not getattr(self, "_deferred_persist_pending", False): + return + self._deferred_persist_pending = False + if self._cache is None: + return + try: + # resort() rebuilds the version index and folder list, so the + # per-call rebuilds skipped during deferral are covered here. + await self._cache.resort() + await self._persist_current_cache(force=True) + self.bump_cache_version() + except Exception: + logger.error( + "%s Scanner: failed to finalize deferred cache persist", + self.model_type.capitalize(), + exc_info=True, + ) + async def update_single_model_cache(self, original_path: str, new_path: str, metadata: Optional[Dict[str, Any]], recalculate_type: bool = False) -> Union[bool, Dict[str, Any]]: - """Update cache after a model has been moved or modified""" + """Update cache after a model has been moved or modified. + + Performs the full maintenance chain (version-index rebuild, resort, + whole-table persist, download-history sync) unless the scanner is + inside a :meth:`defer_cache_persist` context, in which case only + the in-memory entry swap and incremental index updates run and the + heavy chain executes once at context exit. + """ + deferred = self._cache_persist_deferred cache = await self.get_cached_data() - existing_item = next((item for item in cache.raw_data if item['file_path'] == original_path), None) + existing_index: Optional[int] = None + existing_item = None + for idx, item in enumerate(cache.raw_data): + if item['file_path'] == original_path: + existing_item = item + existing_index = idx + break + if existing_item: cache.remove_from_version_index(existing_item) @@ -2662,11 +2744,18 @@ class ModelScanner: del self._tags_count[tag] self._hash_index.remove_by_path(original_path) - - cache.raw_data = [ - item for item in cache.raw_data - if item['file_path'] != original_path - ] + + if deferred: + # In-place swap avoids the O(n) list rebuild per renamed file; + # indexes were already updated incrementally above/below, and the + # folder recompute happens in the single finalize resort(). + if existing_index is not None: + cache.raw_data.pop(existing_index) + else: + cache.raw_data = [ + item for item in cache.raw_data + if item['file_path'] != original_path + ] cache_modified = bool(existing_item) or bool(metadata) cache_entry: Optional[Dict[str, Any]] = None @@ -2707,8 +2796,11 @@ class ModelScanner: cache_entry.get('autov3') or None, ) - all_folders = set(item['folder'] for item in cache.raw_data) - cache.folders = sorted(list(all_folders), key=lambda x: x.lower()) + if not deferred: + # O(n) over raw_data; the finalize resort() recomputes the + # folder list once, so bulk callers skip it per file. + all_folders = set(item['folder'] for item in cache.raw_data) + cache.folders = sorted(list(all_folders), key=lambda x: x.lower()) # The move target may live in directories the last scan never saw; # record the destination folder (and its parents) in the known @@ -2723,13 +2815,18 @@ class ModelScanner: for tag in cache_entry.get('tags', []): self._tags_count[tag] = self._tags_count.get(tag, 0) + 1 - cache.rebuild_version_index() + if deferred: + if cache_modified: + self._deferred_persist_pending = True + self.bump_cache_version() + else: + cache.rebuild_version_index() - await cache.resort() + await cache.resort() - if cache_modified: - await self._persist_current_cache() - self.bump_cache_version() + if cache_modified: + await self._persist_current_cache() + self.bump_cache_version() if metadata and cache_entry is not None: return cache_entry diff --git a/py/services/recipe_scanner.py b/py/services/recipe_scanner.py index 13e3650e..9d118102 100644 --- a/py/services/recipe_scanner.py +++ b/py/services/recipe_scanner.py @@ -4580,14 +4580,64 @@ class RecipeScanner: return syntax_parts + async def build_lora_hash_index(self) -> Dict[str, List[Dict[str, Any]]]: + """Build a one-shot lowercase-LoRA-hash → recipes index. + + Scans the recipe cache exactly once (O(recipes × loras)) and returns + a mapping of lowercase lora ``hash`` to the list of recipe dicts + containing it. Bulk rename loops pass this index to + :meth:`update_lora_filename_by_hash` so per-file lookups are O(1) + instead of rescanning every recipe for each renamed LoRA. + """ + cache = await self.get_cached_data() + index: Dict[str, List[Dict[str, Any]]] = {} + if not cache or not cache.raw_data: + return index + + for recipe in cache.raw_data: + loras = recipe.get("loras", []) + if not isinstance(loras, list): + continue + for lora in loras: + if not isinstance(lora, dict): + continue + hash_value = (lora.get("hash") or "").lower() + if hash_value: + index.setdefault(hash_value, []).append(recipe) + return index + + async def finalize_bulk_filename_updates(self) -> None: + """Run once after a bulk rename session that deferred maintenance. + + Refreshes folder metadata and schedules a single re-sort. Filename-only + renames never change recipe folders, so the deferred refresh is + redundant but cheap; skipping it per file is what makes bulk renames + O(1)-per-file. + """ + if self._cache is None: + return + self._schedule_resort() + async def update_lora_filename_by_hash( - self, hash_value: str, new_file_name: str + self, + hash_value: str, + new_file_name: str, + *, + hash_index: Optional[Dict[str, List[Dict[str, Any]]]] = None, + defer_maintenance: bool = False, ) -> Tuple[int, int]: """Update file_name in all recipes that contain a LoRA with the specified hash. Args: hash_value: The SHA256 hash value of the LoRA new_file_name: The new file_name to set + hash_index: Optional prebuilt index from + :meth:`build_lora_hash_index`. When given, the O(recipes) + cache scan (and its folder-metadata walk) is skipped and the + affected recipes are looked up directly — the bulk rename path. + defer_maintenance: When True, skip the folder-metadata refresh and + resort scheduling. The caller MUST run + :meth:`finalize_bulk_filename_updates` exactly once afterwards. Returns: Tuple[int, int]: (number of recipes updated in files, number of recipes updated in cache) @@ -4598,17 +4648,21 @@ class RecipeScanner: # Always use lowercase hash for consistency hash_value = hash_value.lower() - # Get cache - cache = await self.get_cached_data() - if not cache or not cache.raw_data: - return 0, 0 + if hash_index is not None: + candidate_recipes = hash_index.get(hash_value, []) + else: + # Get cache + cache = await self.get_cached_data() + if not cache or not cache.raw_data: + return 0, 0 + candidate_recipes = cache.raw_data file_updated_count = 0 cache_updated_count = 0 - # Find recipes that need updating from the cache + # Find recipes that need updating recipes_to_update = [] - for recipe in cache.raw_data: + for recipe in candidate_recipes: loras = recipe.get("loras", []) if not isinstance(loras, list): continue @@ -4654,7 +4708,9 @@ class RecipeScanner: # We don't necessarily need to resort because LoRA file_name isn't a sort key, # but we might want to schedule a resort if we're paranoid or if searching relies on sorted state. # Given it's a rename of a dependency, search results might change if searching by LoRA name. - self._schedule_resort() + # Bulk callers defer this to a single finalize_bulk_filename_updates() call. + if not defer_maintenance: + self._schedule_resort() return file_updated_count, cache_updated_count diff --git a/py/services/use_cases/filename_template_use_case.py b/py/services/use_cases/filename_template_use_case.py index e569d681..659e7c61 100644 --- a/py/services/use_cases/filename_template_use_case.py +++ b/py/services/use_cases/filename_template_use_case.py @@ -33,7 +33,9 @@ class FilenameTemplateUseCase: An empty template restores the recorded original filename instead of rendering a template. Shares the auto-organize lock (and its in-progress error) so a bulk rename never runs concurrently with an auto-organize - operation. + operation. The whole loop runs inside a bulk rename session so cache + persist/resort and recipe maintenance happen once at the end instead of + per renamed file. """ def __init__( @@ -106,23 +108,24 @@ class FilenameTemplateUseCase: await self._emit_progress(progress_callback, result, "started") - for index in range(0, result.total, AUTO_ORGANIZE_BATCH_SIZE): - if self._scanner.is_cancelled(): - logger.info( - "Filename template apply cancelled for %s", self._model_type - ) - break - - batch = models[index : index + AUTO_ORGANIZE_BATCH_SIZE] - for model in batch: + async with self._lifecycle_service.bulk_rename_session() as bulk_context: + for index in range(0, result.total, AUTO_ORGANIZE_BATCH_SIZE): if self._scanner.is_cancelled(): + logger.info( + "Filename template apply cancelled for %s", self._model_type + ) break - await self._process_model(model, template, result) - result.processed += 1 - await self._emit_progress(progress_callback, result, "processing") - # Yield between batches so the server stays responsive. - await asyncio.sleep(0.1) + batch = models[index : index + AUTO_ORGANIZE_BATCH_SIZE] + for model in batch: + if self._scanner.is_cancelled(): + break + await self._process_model(model, template, result, bulk_context) + result.processed += 1 + + await self._emit_progress(progress_callback, result, "processing") + # Yield between batches so the server stays responsive. + await asyncio.sleep(0.1) if self._scanner.is_cancelled(): result.status = "cancelled" @@ -150,6 +153,7 @@ class FilenameTemplateUseCase: model: Dict[str, Any], template: str, result: AutoOrganizeResult, + bulk_context: Any = None, ) -> None: model_name = model.get("model_name", "Unknown") try: @@ -177,7 +181,7 @@ class FilenameTemplateUseCase: return await self._lifecycle_service.rename_model( - file_path=file_path, new_file_name=new_stem + file_path=file_path, new_file_name=new_stem, bulk_context=bulk_context ) result.success_count += 1 diff --git a/tests/conftest.py b/tests/conftest.py index d4c44e5f..04238b74 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,9 +3,10 @@ import importlib.util import inspect import sys import types +from contextlib import asynccontextmanager from dataclasses import dataclass, field from pathlib import Path -from typing import Any, Dict, List, Optional, Sequence +from typing import Any, AsyncIterator, Dict, List, Optional, Sequence from unittest import mock import pytest @@ -177,6 +178,10 @@ class MockScanner: def reset_cancellation(self) -> None: self._cancelled = False + @asynccontextmanager + async def defer_cache_persist(self) -> AsyncIterator[None]: + yield None + async def get_cached_data(self, force_refresh: bool = False): return self._cache diff --git a/tests/services/test_bulk_rename_apply.py b/tests/services/test_bulk_rename_apply.py new file mode 100644 index 00000000..3f0d94e0 --- /dev/null +++ b/tests/services/test_bulk_rename_apply.py @@ -0,0 +1,639 @@ +"""Performance-regression tests for the bulk filename-template rename path. + +The bulk "Apply to Library" rename must be O(1)-per-file: the heavyweight +cache persist/resort chain and the recipe maintenance run exactly once per +bulk operation (even on cancellation or mid-loop errors), while single-shot +renames keep their immediate per-call behavior. +""" + +from __future__ import annotations + +import asyncio +import json +from contextlib import asynccontextmanager +from pathlib import Path +from typing import Any, AsyncIterator, Dict, List, Optional + +import pytest + +from py.services import model_scanner as model_scanner_module +from py.services.model_cache import ModelCache +from py.services.model_lifecycle_service import ModelLifecycleService +from py.services.model_scanner import ModelScanner +from py.services.settings_manager import get_settings_manager +from py.services.use_cases.filename_template_use_case import FilenameTemplateUseCase +from py.utils.models import BaseModelMetadata + + +class BulkDummyScanner(ModelScanner): + """Minimal concrete scanner for cache-behavior tests.""" + + def __init__(self, root: Path): + self._root = str(root) + super().__init__( + model_type="dummy", + model_class=BaseModelMetadata, + file_extensions={".txt"}, + ) + + def get_model_roots(self) -> List[str]: + return [self._root] + + +@pytest.fixture(autouse=True) +def _reset_model_scanner_singletons(): + ModelScanner._instances.clear() + ModelScanner._locks.clear() + yield + ModelScanner._instances.clear() + ModelScanner._locks.clear() + + +@pytest.fixture(autouse=True) +def _disable_persistent_cache_env(monkeypatch: pytest.MonkeyPatch): + monkeypatch.setenv("LORA_MANAGER_DISABLE_PERSISTENT_CACHE", "1") + + +@pytest.fixture(autouse=True) +def _stub_register_service(monkeypatch: pytest.MonkeyPatch): + async def noop(*_args: Any, **_kwargs: Any) -> None: + return None + + monkeypatch.setattr( + model_scanner_module.ServiceRegistry, "register_service", noop + ) + + +def _metadata(stem: str, path: str, sha256: str, civitai_id: int = 1) -> Dict[str, Any]: + return { + "file_name": stem, + "file_path": path, + "model_name": stem, + "sha256": sha256, + "folder": "", + "tags": [], + "size": 1, + "modified": 1.0, + "civitai": {"id": civitai_id, "modelId": 2}, + } + + +class _SpiedCache: + """Scanner cache with call counters for resort/persist/sync.""" + + def __init__(self, scanner: BulkDummyScanner, entries: List[Dict[str, Any]]): + self.cache = ModelCache(raw_data=[dict(e) for e in entries], folders=[]) + scanner._cache = self.cache + self.resort_calls = 0 + self.save_calls: List[bool] = [] + self.sync_calls = 0 + self._original_resort = self.cache.resort + + async def resort(self) -> None: + self.resort_calls += 1 + await self._original_resort() + + async def fake_save(self, scan_result: Any, *, force: bool = False) -> None: + self.save_calls.append(force) + + async def fake_sync(self, raw_data: Any, *, source: str) -> None: + self.sync_calls += 1 + + +async def _make_spied_scanner( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, entries: List[Dict[str, Any]] +) -> tuple[BulkDummyScanner, _SpiedCache]: + scanner = BulkDummyScanner(tmp_path) + spied = _SpiedCache(scanner, entries) + # Truthy stand-in so _persist_current_cache() does not early-return. + scanner._persistent_cache = object() + monkeypatch.setattr(scanner, "_save_persistent_cache", spied.fake_save) + monkeypatch.setattr(scanner, "_sync_download_history", spied.fake_sync) + # Flush the resort task scheduled by ModelCache.__post_init__ before + # installing the counting wrapper. + await asyncio.sleep(0) + await asyncio.sleep(0) + monkeypatch.setattr(spied.cache, "resort", spied.resort) + return scanner, spied + + +# --------------------------------------------------------------------------- +# ModelScanner deferred persist +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_update_single_model_cache_persists_immediately_by_default( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + path_a = (tmp_path / "a.txt").as_posix() + scanner, spied = await _make_spied_scanner( + tmp_path, monkeypatch, [_metadata("a", path_a, "hash-a")] + ) + + await scanner.update_single_model_cache( + path_a, path_a, _metadata("a", path_a, "hash-a") + ) + + assert spied.save_calls == [False] + assert spied.sync_calls == 1 + assert spied.resort_calls == 1 + + +@pytest.mark.asyncio +async def test_deferred_cache_persist_persists_once_for_many_updates( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + entries = [] + for index, stem in enumerate(("a", "b", "c")): + path = (tmp_path / f"{stem}.txt").as_posix() + entries.append(_metadata(stem, path, f"hash-{stem}", civitai_id=index + 1)) + scanner, spied = await _make_spied_scanner(tmp_path, monkeypatch, entries) + + async with scanner.defer_cache_persist(): + for index, stem in enumerate(("a", "b", "c")): + old_path = (tmp_path / f"{stem}.txt").as_posix() + new_path = (tmp_path / f"{stem}-renamed.txt").as_posix() + await scanner.update_single_model_cache( + old_path, + new_path, + _metadata( + f"{stem}-renamed", new_path, f"hash-{stem}", civitai_id=index + 1 + ), + ) + # Nothing heavy may run mid-loop. + assert spied.save_calls == [] + assert spied.sync_calls == 0 + assert spied.resort_calls == 0 + + # Exactly one heavyweight finalize for the whole bulk operation. + assert spied.save_calls == [True] + assert spied.sync_calls == 1 + assert spied.resort_calls == 1 + + # In-memory state is correct, including the incremental version index. + cache = await scanner.get_cached_data() + cached_paths = {item["file_path"] for item in cache.raw_data} + for stem in ("a", "b", "c"): + assert (tmp_path / f"{stem}.txt").as_posix() not in cached_paths + assert (tmp_path / f"{stem}-renamed.txt").as_posix() in cached_paths + assert cache.version_index[1]["file_path"].endswith("a-renamed.txt") + assert scanner._hash_index.get_path("hash-a").endswith("a-renamed.txt") + + +@pytest.mark.asyncio +async def test_deferred_cache_persist_finalizes_despite_cancellation( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + path_a = (tmp_path / "a.txt").as_posix() + scanner, spied = await _make_spied_scanner( + tmp_path, monkeypatch, [_metadata("a", path_a, "hash-a")] + ) + + scanner.cancel_task() + async with scanner.defer_cache_persist(): + new_path = (tmp_path / "a-renamed.txt").as_posix() + await scanner.update_single_model_cache( + path_a, new_path, _metadata("a-renamed", new_path, "hash-a") + ) + + # Files are already renamed on disk, so the persist must be forced even + # though the cancellation flag is set. + assert spied.save_calls == [True] + assert spied.sync_calls == 1 + assert spied.resort_calls == 1 + + +@pytest.mark.asyncio +async def test_deferred_cache_persist_finalizes_despite_mid_loop_error( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + path_a = (tmp_path / "a.txt").as_posix() + scanner, spied = await _make_spied_scanner( + tmp_path, monkeypatch, [_metadata("a", path_a, "hash-a")] + ) + + with pytest.raises(RuntimeError, match="boom"): + async with scanner.defer_cache_persist(): + new_path = (tmp_path / "a-renamed.txt").as_posix() + await scanner.update_single_model_cache( + path_a, new_path, _metadata("a-renamed", new_path, "hash-a") + ) + raise RuntimeError("boom") + + assert spied.save_calls == [True] + assert spied.sync_calls == 1 + + +@pytest.mark.asyncio +async def test_deferred_cache_persist_nested_contexts_finalize_once( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + path_a = (tmp_path / "a.txt").as_posix() + scanner, spied = await _make_spied_scanner( + tmp_path, monkeypatch, [_metadata("a", path_a, "hash-a")] + ) + + async with scanner.defer_cache_persist(): + async with scanner.defer_cache_persist(): + new_path = (tmp_path / "a-renamed.txt").as_posix() + await scanner.update_single_model_cache( + path_a, new_path, _metadata("a-renamed", new_path, "hash-a") + ) + + assert spied.save_calls == [True] + assert spied.resort_calls == 1 + + +@pytest.mark.asyncio +async def test_deferred_cache_persist_without_updates_persists_nothing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + scanner, spied = await _make_spied_scanner(tmp_path, monkeypatch, []) + + async with scanner.defer_cache_persist(): + pass + + assert spied.save_calls == [] + assert spied.resort_calls == 0 + + +# --------------------------------------------------------------------------- +# ModelLifecycleService bulk rename session +# --------------------------------------------------------------------------- + + +class _SessionScanner: + model_type = "lora" + + def __init__(self, root: Path): + self._root = str(root) + self.cache_updates: List[tuple[str, str]] = [] + self.defer_enters = 0 + self.defer_exits = 0 + + def get_model_roots(self) -> List[str]: + return [self._root] + + @asynccontextmanager + async def defer_cache_persist(self) -> AsyncIterator[None]: + self.defer_enters += 1 + try: + yield + finally: + self.defer_exits += 1 + + async def update_single_model_cache( + self, old_path: str, new_path: str, metadata: Dict[str, Any] + ) -> bool: + self.cache_updates.append((old_path, new_path)) + return True + + +class _PassthroughMetadataManager: + def __init__(self) -> None: + self.saved: List[str] = [] + + async def save_metadata(self, path: str, metadata: Dict[str, Any]) -> bool: + self.saved.append(path) + return True + + +class RecordingRecipeScanner: + """Records bulk-mode recipe calls and applies filename changes.""" + + def __init__(self) -> None: + self.index_builds = 0 + self.updates: List[Dict[str, Any]] = [] + self.finalizes = 0 + self._recipes: Dict[str, Dict[str, Any]] = {} + + def add_recipe(self, recipe_id: str, lora_hash: str, file_name: str) -> None: + self._recipes[recipe_id] = { + "id": recipe_id, + "loras": [{"hash": lora_hash, "file_name": file_name}], + } + + async def build_lora_hash_index(self) -> Dict[str, List[Dict[str, Any]]]: + self.index_builds += 1 + index: Dict[str, List[Dict[str, Any]]] = {} + for recipe in self._recipes.values(): + for lora in recipe["loras"]: + index.setdefault(lora["hash"].lower(), []).append(recipe) + return index + + async def update_lora_filename_by_hash( + self, + hash_value: str, + new_file_name: str, + *, + hash_index: Optional[Dict[str, List[Dict[str, Any]]]] = None, + defer_maintenance: bool = False, + ) -> tuple[int, int]: + self.updates.append( + { + "hash_value": hash_value, + "new_file_name": new_file_name, + "hash_index": hash_index, + "defer_maintenance": defer_maintenance, + } + ) + matched = 0 + for recipe in (self._recipes.values() if hash_index is None else hash_index.get(hash_value.lower(), [])): + for lora in recipe["loras"]: + if lora["hash"].lower() == hash_value.lower(): + lora["file_name"] = new_file_name + matched += 1 + return (matched, matched) + + async def finalize_bulk_filename_updates(self) -> None: + self.finalizes += 1 + + +def _write_model_with_sidecar( + root: Path, stem: str, sha256: str, model_name: Optional[str] = None +) -> str: + model_path = root / f"{stem}.safetensors" + model_path.write_bytes(b"model") + (root / f"{stem}.metadata.json").write_text( + json.dumps( + { + "file_name": stem, + "file_path": model_path.as_posix(), + "model_name": model_name or stem, + "sha256": sha256, + } + ) + ) + return model_path.as_posix() + + +def _make_lifecycle_service( + scanner: _SessionScanner, recipe_scanner: RecordingRecipeScanner +) -> ModelLifecycleService: + async def metadata_loader(path: str) -> Dict[str, Any]: + with open(path, "r", encoding="utf-8") as handle: + return json.load(handle) + + async def recipe_scanner_factory() -> RecordingRecipeScanner: + return recipe_scanner + + return ModelLifecycleService( + scanner=scanner, # pyright: ignore[reportArgumentType] + metadata_manager=_PassthroughMetadataManager(), # pyright: ignore[reportArgumentType] + metadata_loader=metadata_loader, + recipe_scanner_factory=recipe_scanner_factory, + ) + + +@pytest.mark.asyncio +async def test_bulk_rename_session_defers_and_finalizes_once(tmp_path: Path): + scanner = _SessionScanner(tmp_path) + recipe_scanner = RecordingRecipeScanner() + recipe_scanner.add_recipe("r1", "aa" * 32, "old_a") + recipe_scanner.add_recipe("r2", "bb" * 32, "old_b") + service = _make_lifecycle_service(scanner, recipe_scanner) + + path_a = _write_model_with_sidecar(tmp_path, "model-a", "aa" * 32) + path_b = _write_model_with_sidecar(tmp_path, "model-b", "bb" * 32) + + async with service.bulk_rename_session() as bulk_context: + await service.rename_model( + file_path=path_a, new_file_name="renamed-a", bulk_context=bulk_context + ) + await service.rename_model( + file_path=path_b, new_file_name="renamed-b", bulk_context=bulk_context + ) + # No finalize may run before the session ends. + assert recipe_scanner.finalizes == 0 + assert scanner.defer_enters == 1 + assert scanner.defer_exits == 0 + + assert scanner.defer_exits == 1 + # Hash index built at most once for the whole session. + assert recipe_scanner.index_builds == 1 + assert recipe_scanner.finalizes == 1 + assert len(recipe_scanner.updates) == 2 + for update in recipe_scanner.updates: + assert update["hash_index"] is not None + assert update["defer_maintenance"] is True + + # Recipes were re-pointed. + assert recipe_scanner._recipes["r1"]["loras"][0]["file_name"] == "renamed-a" + assert recipe_scanner._recipes["r2"]["loras"][0]["file_name"] == "renamed-b" + + +@pytest.mark.asyncio +async def test_bulk_rename_session_finalizes_recipes_despite_error(tmp_path: Path): + scanner = _SessionScanner(tmp_path) + recipe_scanner = RecordingRecipeScanner() + recipe_scanner.add_recipe("r1", "aa" * 32, "old_a") + service = _make_lifecycle_service(scanner, recipe_scanner) + + path_a = _write_model_with_sidecar(tmp_path, "model-a", "aa" * 32) + + with pytest.raises(RuntimeError, match="mid-loop"): + async with service.bulk_rename_session() as bulk_context: + await service.rename_model( + file_path=path_a, new_file_name="renamed-a", bulk_context=bulk_context + ) + raise RuntimeError("mid-loop") + + assert recipe_scanner.finalizes == 1 + assert scanner.defer_exits == 1 + + +@pytest.mark.asyncio +async def test_bulk_rename_session_skips_recipe_finalize_when_untouched(tmp_path: Path): + scanner = _SessionScanner(tmp_path) + recipe_scanner = RecordingRecipeScanner() + recipe_scanner.add_recipe("r1", "cc" * 32, "old_c") + service = _make_lifecycle_service(scanner, recipe_scanner) + + # Model whose hash matches no recipe: the lookup runs but no recipe is + # touched, so no recipe maintenance is needed at finalize. + path_a = _write_model_with_sidecar(tmp_path, "model-a", "aa" * 32) + + async with service.bulk_rename_session() as bulk_context: + await service.rename_model( + file_path=path_a, new_file_name="renamed-a", bulk_context=bulk_context + ) + + assert len(recipe_scanner.updates) == 1 + assert recipe_scanner.updates[0]["defer_maintenance"] is True + assert recipe_scanner.finalizes == 0 + assert recipe_scanner._recipes["r1"]["loras"][0]["file_name"] == "old_c" + + +@pytest.mark.asyncio +async def test_single_rename_keeps_immediate_recipe_behavior(tmp_path: Path): + scanner = _SessionScanner(tmp_path) + recipe_scanner = RecordingRecipeScanner() + recipe_scanner.add_recipe("r1", "aa" * 32, "old_a") + service = _make_lifecycle_service(scanner, recipe_scanner) + + path_a = _write_model_with_sidecar(tmp_path, "model-a", "aa" * 32) + + await service.rename_model(file_path=path_a, new_file_name="renamed-a") + + assert len(recipe_scanner.updates) == 1 + update = recipe_scanner.updates[0] + assert update["hash_index"] is None + assert update["defer_maintenance"] is False + assert recipe_scanner.finalizes == 0 + assert recipe_scanner._recipes["r1"]["loras"][0]["file_name"] == "renamed-a" + + +# --------------------------------------------------------------------------- +# Use case end-to-end: exactly one heavyweight persist for the whole loop +# --------------------------------------------------------------------------- + + +class _UseCaseScanner(ModelScanner): + def __init__(self, root: Path): + self._root = str(root) + super().__init__( + model_type="lora", + model_class=BaseModelMetadata, + file_extensions={".safetensors"}, + ) + + def get_model_roots(self) -> List[str]: + return [self._root] + + +class _UseCaseLockProvider: + def __init__(self) -> None: + self._lock = asyncio.Lock() + + def is_auto_organize_running(self) -> bool: + return False + + async def get_auto_organize_lock(self) -> asyncio.Lock: + return self._lock + + +def _set_filename_template(template: str, model_type: str = "lora") -> None: + manager = get_settings_manager() + templates = dict(manager.settings.get("download_filename_templates") or {}) + templates[model_type] = template + manager.settings["download_filename_templates"] = templates + + +@pytest.mark.asyncio +async def test_filename_template_bulk_apply_persists_and_repoints_once( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + _set_filename_template("{model_name}") + + entries = [] + for index, (stem, model_name, sha) in enumerate( + (("model-a", "alpha", "aa" * 32), ("model-b", "beta", "bb" * 32)) + ): + path = _write_model_with_sidecar(tmp_path, stem, sha, model_name=model_name) + entries.append(_metadata(model_name, path, sha, civitai_id=index + 1)) + + scanner = _UseCaseScanner(tmp_path) + spied = _SpiedCache(scanner, entries) + scanner._persistent_cache = object() + monkeypatch.setattr(scanner, "_save_persistent_cache", spied.fake_save) + monkeypatch.setattr(scanner, "_sync_download_history", spied.fake_sync) + await asyncio.sleep(0) + await asyncio.sleep(0) + monkeypatch.setattr(spied.cache, "resort", spied.resort) + + recipe_scanner = RecordingRecipeScanner() + recipe_scanner.add_recipe("r1", "aa" * 32, "alpha") + recipe_scanner.add_recipe("r2", "bb" * 32, "beta") + + service = _make_lifecycle_service( # type: ignore[arg-type] + scanner, recipe_scanner # pyright: ignore[reportArgumentType] + ) + use_case = FilenameTemplateUseCase( + scanner=scanner, + lifecycle_service=service, + lock_provider=_UseCaseLockProvider(), + model_type="lora", + ) + + result = await use_case.execute(progress_callback=None) + + assert result.status == "success" + assert result.success_count == 2 + + # One heavyweight persist/resort for the entire bulk operation, not two. + assert spied.save_calls == [True] + assert spied.sync_calls == 1 + assert spied.resort_calls == 1 + + # Recipes re-pointed via a single lazily built hash index, maintenance + # finalized once. + assert recipe_scanner.index_builds == 1 + assert recipe_scanner.finalizes == 1 + assert recipe_scanner._recipes["r1"]["loras"][0]["file_name"] == "alpha" + assert recipe_scanner._recipes["r2"]["loras"][0]["file_name"] == "beta" + + # Cache reflects the new paths. + cache = await scanner.get_cached_data() + cached_paths = {item["file_path"] for item in cache.raw_data} + assert (tmp_path / "alpha.safetensors").as_posix() in cached_paths + assert (tmp_path / "beta.safetensors").as_posix() in cached_paths + + # Sidecars moved alongside the model files. + assert (tmp_path / "alpha.metadata.json").exists() + assert (tmp_path / "beta.metadata.json").exists() + + +@pytest.mark.asyncio +async def test_filename_template_bulk_apply_finalizes_persist_on_cancellation( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + _set_filename_template("{model_name}") + + entries = [] + paths = [] + for index, (stem, model_name, sha) in enumerate( + (("model-a", "alpha", "aa" * 32), ("model-b", "beta", "bb" * 32)) + ): + path = _write_model_with_sidecar(tmp_path, stem, sha, model_name=model_name) + paths.append(path) + entries.append(_metadata(model_name, path, sha, civitai_id=index + 1)) + + scanner = _UseCaseScanner(tmp_path) + spied = _SpiedCache(scanner, entries) + scanner._persistent_cache = object() + monkeypatch.setattr(scanner, "_save_persistent_cache", spied.fake_save) + monkeypatch.setattr(scanner, "_sync_download_history", spied.fake_sync) + await asyncio.sleep(0) + await asyncio.sleep(0) + monkeypatch.setattr(spied.cache, "resort", spied.resort) + + service = _make_lifecycle_service( + scanner, # pyright: ignore[reportArgumentType] + RecordingRecipeScanner(), + ) + + original_rename = service.rename_model + + async def cancelling_rename(**kwargs: Any) -> Dict[str, object]: + result = await original_rename(**kwargs) + scanner.cancel_task() + return result + + monkeypatch.setattr(service, "rename_model", cancelling_rename) + + use_case = FilenameTemplateUseCase( + scanner=scanner, + lifecycle_service=service, + lock_provider=_UseCaseLockProvider(), + model_type="lora", + ) + + result = await use_case.execute(progress_callback=None) + + assert result.status == "cancelled" + # The one file renamed before cancellation is on disk; the cache must + # still be persisted exactly once. + assert spied.save_calls == [True] + assert spied.sync_calls == 1 + assert spied.resort_calls == 1 diff --git a/tests/services/test_recipe_scanner.py b/tests/services/test_recipe_scanner.py index 88b6f47b..2a234b0c 100644 --- a/tests/services/test_recipe_scanner.py +++ b/tests/services/test_recipe_scanner.py @@ -1903,6 +1903,115 @@ async def test_update_lora_filename_by_hash_updates_affected_recipes( assert cached1["loras"][0]["file_name"] == new_name +@pytest.mark.asyncio +async def test_update_lora_filename_by_hash_bulk_mode_skips_scan_and_defers_resort( + tmp_path: Path, recipe_scanner, monkeypatch: pytest.MonkeyPatch +): + """Bulk rename path: prebuilt hash index, no per-call cache walk/resort.""" + scanner, _ = recipe_scanner + recipes_dir = Path(config.loras_roots[0]) / "recipes" + recipes_dir.mkdir(parents=True, exist_ok=True) + + recipe1_id = "recipe-bulk-1" + recipe1_path = recipes_dir / f"{recipe1_id}.recipe.json" + recipe1_data = { + "id": recipe1_id, + "file_path": str(tmp_path / "bulk1.png"), + "title": "Bulk 1", + "modified": 0.0, + "created_date": 0.0, + "loras": [{"file_name": "old_name", "hash": "hash1"}], + } + recipe1_path.write_text(json.dumps(recipe1_data)) + await scanner.add_recipe(dict(recipe1_data)) + + # Build the index once (O(recipes)), as a bulk rename session does. + hash_index = await scanner.build_lora_hash_index() + assert "hash1" in hash_index + + # Spies: the bulk call must not walk the recipe cache again, and must not + # schedule a resort per call. + get_cached_calls = 0 + original_get_cached = scanner.get_cached_data + + async def counting_get_cached_data(*args, **kwargs): + nonlocal get_cached_calls + get_cached_calls += 1 + return await original_get_cached(*args, **kwargs) + + monkeypatch.setattr(scanner, "get_cached_data", counting_get_cached_data) + + resort_calls = 0 + original_schedule = scanner._schedule_resort + + def counting_schedule_resort(**kwargs): + nonlocal resort_calls + resort_calls += 1 + original_schedule(**kwargs) + + monkeypatch.setattr(scanner, "_schedule_resort", counting_schedule_resort) + + file_count, cache_count = await scanner.update_lora_filename_by_hash( + "HASH1", "new_name", hash_index=hash_index, defer_maintenance=True + ) + + assert (file_count, cache_count) == (1, 1) + assert get_cached_calls == 0 + assert resort_calls == 0 + + # The per-match recipe JSON rewrite must still happen. + persisted1 = json.loads(recipe1_path.read_text()) + assert persisted1["loras"][0]["file_name"] == "new_name" + cached1 = next(r for r in hash_index["hash1"] if r["id"] == recipe1_id) + assert cached1["loras"][0]["file_name"] == "new_name" + + # Deferred maintenance runs exactly once at finalize. + await scanner.finalize_bulk_filename_updates() + await asyncio.sleep(0) + assert resort_calls == 1 + + +@pytest.mark.asyncio +async def test_update_lora_filename_by_hash_with_index_still_resorts_when_not_deferred( + tmp_path: Path, recipe_scanner, monkeypatch: pytest.MonkeyPatch +): + """hash_index without defer_maintenance: O(1) lookup, immediate resort.""" + scanner, _ = recipe_scanner + recipes_dir = Path(config.loras_roots[0]) / "recipes" + recipes_dir.mkdir(parents=True, exist_ok=True) + + recipe1_id = "recipe-idx-1" + recipe1_path = recipes_dir / f"{recipe1_id}.recipe.json" + recipe1_data = { + "id": recipe1_id, + "file_path": str(tmp_path / "idx1.png"), + "title": "Index 1", + "modified": 0.0, + "created_date": 0.0, + "loras": [{"file_name": "old_name", "hash": "hash1"}], + } + recipe1_path.write_text(json.dumps(recipe1_data)) + await scanner.add_recipe(dict(recipe1_data)) + + hash_index = await scanner.build_lora_hash_index() + + resort_calls = 0 + original_schedule = scanner._schedule_resort + + def counting_schedule_resort(**kwargs): + nonlocal resort_calls + resort_calls += 1 + original_schedule(**kwargs) + + monkeypatch.setattr(scanner, "_schedule_resort", counting_schedule_resort) + + file_count, cache_count = await scanner.update_lora_filename_by_hash( + "hash1", "new_name", hash_index=hash_index + ) + + assert (file_count, cache_count) == (1, 1) + assert resort_calls == 1 + @pytest.mark.asyncio async def test_get_paginated_data_filters_by_favorite(recipe_scanner): scanner, _ = recipe_scanner diff --git a/tests/services/test_use_cases.py b/tests/services/test_use_cases.py index e24d4817..bafbbeba 100644 --- a/tests/services/test_use_cases.py +++ b/tests/services/test_use_cases.py @@ -1,8 +1,9 @@ import asyncio import logging +from contextlib import asynccontextmanager from dataclasses import dataclass from types import SimpleNamespace -from typing import Any, Dict, List, Optional +from typing import Any, AsyncIterator, Dict, List, Optional import pytest @@ -547,7 +548,13 @@ class StubLifecycleService: self.cancel_on_rename = False self._scanner = scanner - async def rename_model(self, *, file_path: str, new_file_name: str) -> Dict[str, Any]: + @asynccontextmanager + async def bulk_rename_session(self) -> AsyncIterator[None]: + yield None + + async def rename_model( + self, *, file_path: str, new_file_name: str, bulk_context: Any = None + ) -> Dict[str, Any]: if self.error is not None: raise self.error self.renames.append({"file_path": file_path, "new_file_name": new_file_name})