From 57729375b6b3f6b20d4a99c3163d573507c22096 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Sat, 12 Sep 2026 14:24:35 +0800 Subject: [PATCH] docs: detail Phase 2 download design for Other Models page --- docs/plans/other-models-page.md | 98 ++++++++++++++++++++++++++++++--- 1 file changed, 90 insertions(+), 8 deletions(-) diff --git a/docs/plans/other-models-page.md b/docs/plans/other-models-page.md index 1c2a1847..97d038aa 100644 --- a/docs/plans/other-models-page.md +++ b/docs/plans/other-models-page.md @@ -1,7 +1,7 @@ # Plan: "Other Models" Page — Unified Management for VAE / Upscaler / Text Encoder / etc. -**Status:** v1 — decisions locked with feature owner (2026-09-12); ready for implementation. -**Scope (Phase 1):** scan + manage (list, search, filter, tags, folders, preview, rename, move, delete/exclude, CivitAI metadata fetch) for a new model type `other`, exposed as a new web page. **Phase 2 (out of scope here, outlined in §9):** one-click download from CivitAI for these types. +**Status:** v2 — **Phase 1 implemented** (2026-09-12, commits `27da7b3c` backend + `fa7ce725` frontend; verified live against a running ComfyUI instance: scan/hash/sub_type-derivation/fetch/previews all green). **Phase 2 designed** (§9, decisions pending owner sign-off). +**Scope (Phase 1):** scan + manage (list, search, filter, tags, folders, preview, rename, move, delete/exclude, CivitAI metadata fetch) for a new model type `other`, exposed as a new web page. **Phase 2 (§9):** one-click download from CivitAI for these types. ## 1. Goal @@ -152,14 +152,87 @@ Follow existing conventions (`pytest.ini`, `tests/frontend/` vitest): 6. i18n keys + sync script 7. `pytest` + `npm test` full runs; hand to user for manual UI check -## 9. Phase 2 Outline (NOT in this implementation) +## 9. Phase 2 Detailed Design — CivitAI Downloads for `other` -CivitAI downloads for `other` types: +Designed 2026-09-12 against the Phase-1 code on this branch; decisions marked **[locked]** follow the same recommendations the feature owner approved for Phase 1. -- `py/services/download_manager.py`: extend the type map (`:1496-1507` — currently rejects unknown types), `_get_scanner_for_model_type` (`:230-236`), `_build_metadata_for_resume` (`:977-981`). -- **Default-root selection** (`:1699-1727`): needs per-sub_type settings keys (`default_vae_root`, `default_upscaler_root`, …) — this is what pulls in `settings_manager.py` (library default keys `:82-85`, `_check_and_auto_set` ~`:890`, `set()` dispatch `:1621-1628`, `upsert_library` params, library-switch scanner notifications `:1795-1799`/`:2150-2155`). -- File-type routing: consult CivitAI's `primaryFileTypesByModelType` (e.g. CLIP-type models ship `Text Encoder`/`Vision Encoder` primary files, not `Model`); a `file.type → target folder` map analogous to `download_routing.py:is_diffusion_model_download`. -- Settings UI for the new default roots. +### 9.1 Download pipeline touch points + +Flow: `POST /api/lm/download-model` (`py/routes/model_route_registrar.py:104`; GET variant `:105` for the browser extension) → `ModelDownloadHandler.download_model` (`model_handlers.py:1740`) → `DownloadModelUseCase.execute` → `DownloadCoordinator.schedule_download` → `DownloadManager.download_from_civitai` (`download_manager.py:386`) → `_execute_original_download` (`:1415`). Inside, seven scatter points need an `other` branch: + +1. **Type map** (`:1496-1507`): accept `model.type.lower() in VALID_OTHER_CIVITAI_TYPES` → `model_type = "other"` (reuses the Phase-1 set, incl. `"other"` itself). +2. **Early version-exists gate** (`:1436-1463`): add `other_scanner.check_model_version_exists`. +3. **File-level exists gate** (`:1640-1655` → `_find_local_file_entry` `:320-346` → `_get_scanner_for_model_type` `:230-236`): add explicit `other` branch. **Trap**: the function currently falls through to the lora scanner for unknown types — `"other"` would silently dedupe against loras. Also narrow the fall-through to `"lora"` only / raise on unknown. +4. **Version-level fallback gate** (`:1656-1688`): add `elif model_type == "other"`. +5. **Default-root selection** (`:1690-1727`): for `other`, first resolve sub_type (§9.2), then read `default_other_roots[sub_type]` (§9.3); if sub_type is undecidable or no default root configured → error guiding the user to pick a folder explicitly. +6. **Metadata class selection** (`:1909-1928`) + `_build_metadata_for_resume` (`:969-981`): add `OtherModelMetadata.from_civitai_info` branches. +7. **Post-download cache write** (`_execute_download_pipeline` `:2622-2679`): add `other` scanner branch; `adjust_metadata` re-derives sub_type from the on-disk root automatically. `_get_supported_extensions_for_type` (`:2720-2744`): `other` reuses the checkpoint extension set. + +Hooks: `_record_downloaded_version_history` (model_type is free text — zero change); `_sync_downloaded_version` (`:1984` → scanner dispatch `:2130-2135`) add `other`; `py/utils/example_images_download_manager.py` scanner dispatch at `:411-421`, `:591-601`, `:1089+` — add `other` at all three (silent no-scanner otherwise). + +Path templates: `get_download_path_template("other")` already falls back to `"{base_model}/{first_tag}"` — works with zero change; optional settings-UI row (§9.4). + +### 9.2 File-level routing (model.type / file.type → sub_type) **[locked]** + +Table-driven, mirroring Phase 1. New in `py/utils/constants.py`: + +```python +CIVITAI_FILE_TYPE_TO_OTHER_SUB_TYPE = { + "VAE": "vae", "Upscaler": "upscaler", "Text Encoder": "text_encoder", + "Vision Encoder": "clip_vision", "CLIPVision": "clip_vision", + "ControlNet": "controlnet", +} +``` + +`download_routing.py` gains `resolve_other_download_sub_type(civitai_model_type, file_types, selected_file_type=None)` with fixed priority: + +1. **Explicit user file pick** (`file_params` from #1058's `_resolve_target_file`) — if the picked file's type maps, it wins even when model.type is `Checkpoint`. +2. **model.type** via the existing `CIVITAI_TYPE_TO_OTHER_SUB_TYPE` (`constants.py:120-127`). +3. **file.type fallback** — only when model.type maps to nothing (e.g. model.type `Other` or retired `CLIP`). MUST NOT override a mapped model.type: checkpoint models routinely bundle VAE/Text Encoder component files, and unconditional file-type routing would misroute them. +4. Still undecidable → `None`; `use_default_paths` errors and the UI offers all other roots for manual selection. + +HTTP: extend `DownloadRoutingHandler.get_download_routing` (`download_routing_handlers.py:23`) with an `other` branch returning `{root_kind: "other", sub_type: ...}`; add `GET /api/lm/other/roots_by_subtype` in `OtherRoutes.setup_specific_routes` (data from `config._prepare_other_paths`'s per-key roots, aggregating `text_encoders` + legacy `clip` under `text_encoder`). + +### 9.3 Settings: single dict key `default_other_roots` **[locked]** + +Rejected: four flat keys (`default_vae_root`…) — each flat key costs ~13 touch points in `settings_manager.py` (defaults `:82-85`, `_check_and_auto_set` `:890-895`, `set()` `:1621-1628`, `_update_active_library_entry` `:738-805`, upsert/create signatures `:1953-2132`, `_build_library_payload` `:552-612`, `_sync_active_library_to_root` `:519-547`, three library constructors, frontend `DEFAULT_SETTINGS_BASE`), repeated per future sub_type. + +Chosen: one mapping key `default_other_roots: {sub_type: path}`, copying the `extra_folder_paths` precedent (generic Mapping handling at `:533-535`, `:573-578`, `:763-767`). `_check_and_auto_set` generalizes to per-sub_type candidates (union over that sub_type's folder keys — `text_encoder` → `text_encoders` + `clip`). `set()` validates keys against `VALID_OTHER_SUB_TYPES`. + +Also fix the Phase-1 omission: add `"other_scanner"` to `_notify_library_change` (`:2150-2156`) and `_notify_model_name_display_change` (`:1795-1800`) — otherwise switching libraries leaves the other page stale. + +### 9.4 Settings UI + +- `templates/components/modals/settings/library.html:34-40`: sub_type selectors after the existing four `setting_select`s (Jinja loop; controlnet selector only when `enabled_other_folders` includes it). Dict-subkey save helper `saveOtherRootSetting(subType, value)` alongside the flat `saveSelectSetting`. +- `static/js/managers/SettingsManager.js:1547-1697`: `loadOtherRoots()` mirroring `loadUnetRoots()`, fed by `/api/lm/other/roots_by_subtype`; current values from `state.global.settings.default_other_roots`. `state/index.js:24` `DEFAULT_SETTINGS_BASE` += `default_other_roots: {}`. +- Optional: one `other` row in the download-path-template block (`library.html:153-211`). +- i18n: `settings.folderSettings.*` keys into `locales/en.json` + sync script; other locales keep `[TODO: Translate]`. +- Settings GET (`misc_handlers.py:1528-1536`) already returns all non-sensitive keys — new key reaches the frontend for free. + +### 9.5 Frontend download entry + +- `templates/components/controls.html:83`: drop the `page_id != 'other'` exclusion on the download button (keyboard shortcut D self-enables via `PageControls.js:196-198`). +- `OtherControls.js:22-55`: add `showDownloadModal: () => downloadManager.showDownloadModal()` (mirror `EmbeddingsControls.js:43-45`). +- `DownloadManager.js` `proceedToLocationContent` (`:955-1017`): add `_resolveOtherSubType()` (mirror `_resolveIsDiffusionModel` `:1026`): selected file type → `/api/lm/download/routing` → `otherApiClient.fetchModelRoots(subType)` (new); default-root preselect reads `default_other_roots[subType]` instead of `` `default_${singularType}_root` `` (`:974`). Undecidable → list all other roots (`/api/lm/other/roots`) for manual pick; an explicit save_dir skips backend default-root logic, so the two paths cannot disagree. +- `ModelVersionsTab` download buttons are modelType-generic and already work via `getModelApiClient('other')`; context menu has no CivitAI download entry — no change. +- Version-list type validation (`get_civitai_versions` → `_validate_civitai_model_type`) already accepts `VALID_OTHER_CIVITAI_TYPES` from Phase 1. + +### 9.6 CivitAI type mapping decisions **[locked]** + +- Download accepts exactly `VALID_OTHER_CIVITAI_TYPES` (`VAE, Upscaler, TextEncoder, CLIP, CLIPVision, Controlnet, Other`) — reuse the Phase-1 tables; do NOT create new ones. +- Extend `CIVITAI_USER_MODEL_TYPES` (`constants.py:133-137`) with the 7 aliases, and point them at the other scanner / `"other"` history bucket in `misc_handlers.py` (`type_scanner_map` `:2793-2797`, `downloaded_version_map` `:2821-2827`) — otherwise creator pages silently filter these models while downloads claim support. +- Fix (small Phase-1 bug): `OtherModelMetadata.from_civitai_info` (`py/utils/models.py:343`) reads `version_info.get("type")`, but the type lives at `version["model"]["type"]` — the mapping never fires and always degrades to the placeholder. Read `version_info.get("model", {}).get("type")` instead. (`CheckpointMetadata:290` has the same shape; leave it alone here.) + +### 9.7 Tests + +Existing base: `tests/services/test_download_manager_basic.py` (incl. `test_download_rejects_unsupported_model_type` `:1336`), `test_download_manager_error.py`, `test_download_manager_concurrent.py`, `tests/integration/test_download_flow.py`, `tests/services/test_settings_manager.py`; frontend `tests/frontend/managers/downloadManager.routing.test.js`, `settingsManager.library.test.js`. + +Add: (1) `resolve_other_download_sub_type` unit tests — every priority tier, bundled-component anti-misrouting, undecidable → None, civarchive-shaped payload; (2) download_manager — six model.types accepted → other scanner (mock), unknown still rejected, no lora-scanner fall-through, per-sub_type default roots + unconfigured error, resume metadata, extension set; (3) settings_manager — `default_other_roots` defaults/auto-set (incl. text_encoder dual-key union)/library sync/upsert passthrough/illegal sub_type rejection; (4) routes — `/api/lm/download/routing` other branch, `roots_by_subtype` shape; (5) example-images dispatch accepts `other` (3 sites); (6) vitest — `_resolveOtherSubType` + root select + default preselect, `loadOtherRoots`; (7) user-models existsLocally for VAE. + +### 9.8 Phase 2 file list + +Backend: `py/utils/constants.py`, `py/services/download_routing.py`, `py/routes/handlers/download_routing_handlers.py`, `py/services/download_manager.py`, `py/utils/example_images_download_manager.py`, `py/services/settings_manager.py`, `py/utils/models.py`, `py/routes/other_routes.py`, `py/routes/handlers/misc_handlers.py`, `settings.json.example`. +Frontend/templates: `templates/components/controls.html`, `static/js/components/controls/OtherControls.js`, `static/js/managers/DownloadManager.js`, `static/js/api/otherApi.js`, `templates/components/modals/settings/library.html`, `static/js/managers/SettingsManager.js`, `static/js/state/index.js`, `locales/en.json` + sync. ## 10. Risks / Open Questions @@ -168,3 +241,12 @@ CivitAI downloads for `other` types: - **Retired CivitAI types**: `CLIP`/`CLIPVision` are retired upstream (grandfathered for existing models); metadata fetch must tolerate both retired and current types — `VALID_OTHER_CIVITAI_TYPES` includes them deliberately. - **Standalone users** must add the new `folder_paths` keys to `settings.json` themselves; document in `settings.json.example` and the feature doc. - **Page display name** is i18n-only; if "Other Models" tests poorly, rename `other.title` without code changes. + +### Phase 2 risks + +- **Bundled component files**: checkpoint models routinely ship VAE/Text Encoder component files — file.type routing must stay a fallback (or explicit user pick), never an override (§9.2 priority is load-bearing; test it). +- **`_get_scanner_for_model_type` lora fall-through** (`download_manager.py:236`): without an explicit `other` branch, dedupe checks run against the lora scanner — the most insidious trap in Phase 2. +- **text_encoder dual folder keys** (`text_encoders` + legacy `clip`): default-root candidates, `roots_by_subtype`, and auto-set must all merge both keys; miss one and the default-root dropdown comes up empty. +- **Undecidable sub_type** (model.type `Other` + unknown file types): must error and ask, never silently default to the vae folder. +- **Lazy hash after download**: downloads carry CivitAI SHA256 (no recompute needed) — ensure the post-download cache write doesn't leave `hash_status="pending"`, or the next metadata fetch re-hashes a 10 GB file. +- **CivArchive source**: same `_execute_original_download` path, same payload shape — cover it once in tests.