From 8a71013247dbd700cbaf9805a62456e610a47611 Mon Sep 17 00:00:00 2001 From: Ethanfel Date: Wed, 29 Jul 2026 19:45:53 +0200 Subject: [PATCH] feat(models): add bulk model deletion controls --- README.md | 1 + __init__.py | 26 ++++++++++ js/nodes_stats.js | 102 ++++++++++++++++++++++++++++++++++--- mapper.py | 70 +++++++++++++++++++++++++ tests/test_model_delete.py | 66 ++++++++++++++++++++++++ 5 files changed, 259 insertions(+), 6 deletions(-) create mode 100644 tests/test_model_delete.py diff --git a/README.md b/README.md index 6588a36..1d5fadb 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,7 @@ a clear message) when ComfyUI Manager is absent or there are no disabled package - Summary bar with counts for each tier across all model types - Sections per model type (checkpoints, vae, controlnet, …) - Per-model table showing execution count, last used date, and status +- Select one or more installed models and permanently delete them after confirmation ### API diff --git a/__init__.py b/__init__.py index cf66a7e..87bca5e 100644 --- a/__init__.py +++ b/__init__.py @@ -112,6 +112,32 @@ async def get_model_stats(request): return web.json_response({"error": "internal error"}, status=500) +@routes.post("/nodes-stats/models/delete") +async def delete_models(request): + """Delete one or more currently installed model files. + + The mapper validates each supplied (type, name) pair against ComfyUI's + current model folders before unlinking it, rather than trusting a path sent + by the browser. + """ + try: + data = await request.json() + models = data.get("models") + if not isinstance(models, list) or not models: + return web.json_response({"error": "models must be a non-empty list"}, status=400) + if len(models) > 500: + return web.json_response({"error": "too many models"}, status=400) + results = await asyncio.get_event_loop().run_in_executor( + None, model_mapper.delete_models, models + ) + deleted = [r for r in results if r.get("status") == "deleted"] + failed = [r for r in results if r.get("status") != "deleted"] + return web.json_response({"deleted": deleted, "failed": failed}) + except Exception: + logger.error("nodes-stats: error deleting models", exc_info=True) + return web.json_response({"error": "internal error"}, status=500) + + @routes.post("/nodes-stats/reset") async def reset_stats(request): try: diff --git a/js/nodes_stats.js b/js/nodes_stats.js index 6a0d7a6..c107f71 100644 --- a/js/nodes_stats.js +++ b/js/nodes_stats.js @@ -258,6 +258,7 @@ async function showStatsDialog(initialTab = "nodes") { wireWorkflowButtons(dialog); wireWhitelistButtons(dialog); wireNativeEnableButtons(dialog); + wireModelDeleteControls(dialog); switchTab(TABS.includes(initialTab) ? initialTab : "nodes"); @@ -432,6 +433,11 @@ function buildModelsTabContent(modelData) { return html; } + html += `
+ No models selected + +
`; + for (const group of modelData) { if (group.models.length === 0) continue; const title = group.model_type.charAt(0).toUpperCase() + group.model_type.slice(1).replace(/_/g, " "); @@ -443,30 +449,114 @@ function buildModelsTabContent(modelData) { } function buildModelTable(models) { - let html = ` + let html = `
+ - - - + + + + `; for (const m of models) { const meta = STATUS_META[m.status] || STATUS_META.used; const lastSeen = m.last_seen ? new Date(m.last_seen).toLocaleDateString() : "—"; + const selectable = m.installed ? "" : " disabled"; + const action = m.installed + ? `` + : `—`; + html += ` - + + + `; } - html += `
ModelExecutionsLast UsedStatusExecutionsLast UsedStatus
${escapeHtml(m.model_name)}${escapeHtml(m.model_name)} ${m.count} ${lastSeen} ${meta.label}${action}
`; + html += ``; return html; } +// Model deletion is intentionally separate from package disable: the selected +// file is deleted permanently, so every request is confirmed and the backend +// validates that it belongs to a configured ComfyUI model folder. +function wireModelDeleteControls(dialog) { + const selectionCount = dialog.querySelector("#ns-model-selection-count"); + const deleteSelected = dialog.querySelector("#ns-delete-selected-models"); + if (!selectionCount || !deleteSelected) return; + + const selectedModels = () => [...dialog.querySelectorAll(".ns-model-select:checked")].map((box) => ({ + model_type: box.dataset.modelType, + model_name: box.dataset.modelName, + })); + + const updateSelection = () => { + const selected = dialog.querySelectorAll(".ns-model-select:checked").length; + selectionCount.textContent = selected ? `${selected} model${selected === 1 ? "" : "s"} selected` : "No models selected"; + deleteSelected.disabled = selected === 0; + dialog.querySelectorAll(".ns-model-select-all").forEach((master) => { + const choices = [...master.closest("table").querySelectorAll(".ns-model-select:not(:disabled)")]; + const checked = choices.filter((box) => box.checked).length; + master.checked = choices.length > 0 && checked === choices.length; + master.indeterminate = checked > 0 && checked < choices.length; + master.disabled = choices.length === 0; + }); + }; + + dialog.querySelectorAll(".ns-model-select").forEach((box) => box.addEventListener("change", updateSelection)); + dialog.querySelectorAll(".ns-model-select-all").forEach((master) => master.addEventListener("change", () => { + master.closest("table").querySelectorAll(".ns-model-select:not(:disabled)").forEach((box) => { + box.checked = master.checked; + }); + updateSelection(); + })); + dialog.querySelectorAll(".ns-delete-model-btn").forEach((button) => button.addEventListener("click", (event) => { + event.stopPropagation(); + deleteModels([{ model_type: button.dataset.modelType, model_name: button.dataset.modelName }], dialog); + })); + deleteSelected.addEventListener("click", () => deleteModels(selectedModels(), dialog)); + updateSelection(); +} + +async function deleteModels(models, dialog) { + if (!models.length) return; + const label = models.length === 1 ? `"${models[0].model_name}"` : `${models.length} selected models`; + if (!confirm(`Permanently delete ${label}?\n\nThis removes the model files from disk and cannot be undone.`)) return; + + const controls = dialog.querySelectorAll(".ns-model-select, .ns-model-select-all, .ns-delete-model-btn, #ns-delete-selected-models"); + controls.forEach((control) => { control.disabled = true; }); + try { + const response = await fetch("/nodes-stats/models/delete", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ models }), + }); + let result = {}; + try { result = await response.json(); } catch { /* use the HTTP status below */ } + if (!response.ok) throw new Error(result.error || `HTTP ${response.status}`); + + const deleted = result.deleted || []; + const failed = result.failed || []; + if (deleted.length) { + notify(`Deleted ${deleted.length} model${deleted.length === 1 ? "" : "s"}.`, "success"); + showStatsDialog("models"); // Re-read folders; deleted files remain as historical entries. + } + if (failed.length) { + const names = failed.map((item) => item.model_name || item.message || "unknown").join(", "); + notify(`Could not delete: ${names}`, "error"); + } + } catch (error) { + notify("Failed to delete model: " + error.message, "error"); + } finally { + if (dialog.isConnected) controls.forEach((control) => { control.disabled = false; }); + } +} + // Render the Workflow tab from a classification result. `disabled` entries get // re-enable actions (temporary trial or permanent); `missing` entries get an // Install button that defers to ComfyUI Manager. diff --git a/mapper.py b/mapper.py index 8630bb7..6910fa8 100644 --- a/mapper.py +++ b/mapper.py @@ -132,6 +132,76 @@ class ModelMapper: self._ensure() return {k: sorted(v) for k, v in self._folder_files.items()} + def delete_models(self, models): + """Delete verified model files and return per-model results. + + Each requested file must still be in the mapper's known model list and + resolve beneath the configured root for its model type. This keeps the + HTTP endpoint from being usable to remove arbitrary files. + """ + self._ensure() + try: + import folder_paths + except ImportError: + return [{"status": "error", "message": "folder_paths unavailable"}] + + results = [] + seen = set() + for item in models: + if not isinstance(item, dict): + results.append({"status": "error", "message": "invalid model entry"}) + continue + model_type = item.get("model_type") + model_name = item.get("model_name") + if not isinstance(model_type, str) or not isinstance(model_name, str): + results.append({ + "model_type": model_type, + "model_name": model_name, + "status": "error", + "message": "invalid model entry", + }) + continue + key = (model_type, model_name) + if key in seen: + continue + seen.add(key) + result = {"model_type": model_type, "model_name": model_name} + + known = self._folder_files.get(model_type, frozenset()) + if model_name not in known: + result.update(status="error", message="model is not installed") + results.append(result) + continue + + try: + path = folder_paths.get_full_path(model_type, model_name) + roots = folder_paths.get_folder_paths(model_type) + path = os.path.abspath(path) if path else None + allowed = any( + os.path.commonpath([path, os.path.abspath(root)]) == os.path.abspath(root) + for root in roots + ) if path else False + except (OSError, TypeError, ValueError): + path = None + allowed = False + + if not path or not allowed or not os.path.isfile(path): + result.update(status="error", message="model file not found in its configured folder") + results.append(result) + continue + + try: + os.remove(path) + result["status"] = "deleted" + except OSError as exc: + result.update(status="error", message=str(exc)) + results.append(result) + + # The model-folder listing is now stale whether any deletion succeeded + # or not (another process could have changed it), so rebuild next time. + self.invalidate() + return results + def extract_models_from_prompt(self, prompt): """Scan a prompt dict and return (model_name, model_type) pairs. diff --git a/tests/test_model_delete.py b/tests/test_model_delete.py new file mode 100644 index 0000000..36969f8 --- /dev/null +++ b/tests/test_model_delete.py @@ -0,0 +1,66 @@ +from mapper import ModelMapper + + +def _configure_model_paths(monkeypatch, tmp_path, filenames): + import folder_paths + + models_root = tmp_path / "models" + models_root.mkdir() + for filename in filenames: + target = models_root / filename + target.parent.mkdir(parents=True, exist_ok=True) + target.write_bytes(b"model") + + monkeypatch.setattr(folder_paths, "folder_names_and_paths", {"checkpoints": ([], {})}) + monkeypatch.setattr(folder_paths, "get_filename_list", lambda model_type: filenames if model_type == "checkpoints" else []) + monkeypatch.setattr(folder_paths, "get_folder_paths", lambda model_type: [str(models_root)] if model_type == "checkpoints" else []) + monkeypatch.setattr(folder_paths, "get_full_path", lambda model_type, filename: str(models_root / filename)) + return models_root + + +def test_delete_models_removes_only_a_known_model(monkeypatch, tmp_path): + models_root = _configure_model_paths(monkeypatch, tmp_path, ["nested/model.safetensors"]) + mapper = ModelMapper() + + result = mapper.delete_models([{ + "model_type": "checkpoints", + "model_name": "nested/model.safetensors", + }]) + + assert result == [{ + "model_type": "checkpoints", + "model_name": "nested/model.safetensors", + "status": "deleted", + }] + assert not (models_root / "nested/model.safetensors").exists() + + +def test_delete_models_rejects_unknown_name(monkeypatch, tmp_path): + models_root = _configure_model_paths(monkeypatch, tmp_path, ["known.safetensors"]) + mapper = ModelMapper() + + result = mapper.delete_models([{ + "model_type": "checkpoints", + "model_name": "../outside.safetensors", + }]) + + assert result[0]["status"] == "error" + assert (models_root / "known.safetensors").exists() + + +def test_delete_models_rejects_path_outside_model_root(monkeypatch, tmp_path): + models_root = _configure_model_paths(monkeypatch, tmp_path, ["known.safetensors"]) + outside = tmp_path / "outside.safetensors" + outside.write_bytes(b"do not remove") + import folder_paths + monkeypatch.setattr(folder_paths, "get_full_path", lambda *_: str(outside)) + mapper = ModelMapper() + + result = mapper.delete_models([{ + "model_type": "checkpoints", + "model_name": "known.safetensors", + }]) + + assert result[0]["status"] == "error" + assert (models_root / "known.safetensors").exists() + assert outside.exists()