From c8c84bfc5487db5d325ab78d698573e085ba5cf6 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Sat, 19 Sep 2026 05:49:01 +0800 Subject: [PATCH] feat(loras): warn when widget strength leaves the usage-tips range The cycler-list payload now carries usage_tips, and the LORAS widget parses strength_min/strength_max/strength_range into a cached lookup. Strength inputs (model and clip) turn amber with an explanatory tooltip when dragged, typed, or stepped outside the recommended range. Related: https://github.com/willmiao/ComfyUI-Lora-Manager/issues/1090 --- py/services/lora_service.py | 16 +- .../lorasWidgetStrengthRange.test.js | 299 ++++++++++++++++++ tests/services/test_lora_cycler_list.py | 64 ++++ web/comfyui/lm_styles.css | 12 + web/comfyui/loras_widget.js | 4 + web/comfyui/loras_widget_events.js | 7 +- web/comfyui/loras_widget_utils.js | 190 ++++++++++- 7 files changed, 584 insertions(+), 8 deletions(-) create mode 100644 tests/frontend/components/lorasWidgetStrengthRange.test.js create mode 100644 tests/services/test_lora_cycler_list.py diff --git a/py/services/lora_service.py b/py/services/lora_service.py index 6167c549..d5e51858 100644 --- a/py/services/lora_service.py +++ b/py/services/lora_service.py @@ -714,12 +714,18 @@ class LoraService(BaseModelService): ), ) - # Return minimal data needed for cycling - return [ - { + # Return minimal data needed for cycling. usage_tips is only included + # when non-empty so widget consumers (recommended strength range cues) + # can build their lookup without inflating the payload. + result = [] + for lora in available_loras: + entry = { "file_name": f"{lora['folder']}/{lora['file_name']}" if lora.get("folder") else lora["file_name"], "model_name": lora.get("model_name", lora["file_name"]), "folder": lora.get("folder", ""), } - for lora in available_loras - ] + usage_tips = lora.get("usage_tips") + if usage_tips: + entry["usage_tips"] = usage_tips + result.append(entry) + return result diff --git a/tests/frontend/components/lorasWidgetStrengthRange.test.js b/tests/frontend/components/lorasWidgetStrengthRange.test.js new file mode 100644 index 00000000..57bc393b --- /dev/null +++ b/tests/frontend/components/lorasWidgetStrengthRange.test.js @@ -0,0 +1,299 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +const { + APP_MODULE, + API_MODULE, +} = vi.hoisted(() => ({ + APP_MODULE: new URL('../../../scripts/app.js', import.meta.url).pathname, + API_MODULE: new URL('../../../scripts/api.js', import.meta.url).pathname, +})); + +vi.mock(APP_MODULE, () => ({ + app: { graph: {} }, +})); + +const { fetchApiMock } = vi.hoisted(() => ({ fetchApiMock: vi.fn() })); +vi.mock(API_MODULE, () => ({ + api: { fetchApi: fetchApiMock }, +})); + +import { + parseStrengthRange, + describeStrengthRangeViolation, + applyStrengthRangeCue, + buildStrengthRangeMap, + getLoraStrengthRange, + getAvailableLoras, + resetAvailableLorasCache, +} from '../../../web/comfyui/loras_widget_utils.js'; + +describe('parseStrengthRange', () => { + it('parses explicit strength_min/strength_max keys', () => { + expect(parseStrengthRange('{"strength_min": 0.4, "strength_max": 0.8}')).toEqual({ + min: 0.4, + max: 0.8, + recommended: null, + }); + }); + + it('parses the strength_range shorthand', () => { + expect(parseStrengthRange('{"strength_range": "0.4-0.8"}')).toEqual({ + min: 0.4, + max: 0.8, + recommended: null, + }); + }); + + it('accepts camelCase variants', () => { + expect(parseStrengthRange('{"strengthMin": 0.2, "strengthMax": 1.5}')).toEqual({ + min: 0.2, + max: 1.5, + recommended: null, + }); + }); + + it('prefers explicit min/max over the range string per side', () => { + expect( + parseStrengthRange('{"strength_min": 0.1, "strength_range": "0.4-0.8"}') + ).toEqual({ min: 0.1, max: 0.8, recommended: null }); + }); + + it('supports open-ended ranges', () => { + expect(parseStrengthRange('{"strength_max": 1.0}')).toEqual({ + min: null, + max: 1.0, + recommended: null, + }); + }); + + it('captures the recommended strength when present', () => { + expect( + parseStrengthRange('{"strength": 0.6, "strength_min": 0.4, "strength_max": 0.8}') + ).toEqual({ min: 0.4, max: 0.8, recommended: 0.6 }); + }); + + it('accepts numeric strings for bounds', () => { + expect(parseStrengthRange('{"strength_min": "0.4", "strength_max": "0.8"}')).toEqual({ + min: 0.4, + max: 0.8, + recommended: null, + }); + }); + + it('parses negative bounds in range strings', () => { + expect(parseStrengthRange('{"strength_range": "-0.5-0.8"}')).toEqual({ + min: -0.5, + max: 0.8, + recommended: null, + }); + }); + + it('returns null when no range is configured', () => { + expect(parseStrengthRange('{"strength": 0.6}')).toBeNull(); + expect(parseStrengthRange('{}')).toBeNull(); + expect(parseStrengthRange('')).toBeNull(); + expect(parseStrengthRange(null)).toBeNull(); + expect(parseStrengthRange(undefined)).toBeNull(); + }); + + it('returns null for malformed JSON', () => { + expect(parseStrengthRange('{invalid')).toBeNull(); + }); + + it('returns null for inverted ranges', () => { + expect(parseStrengthRange('{"strength_min": 0.9, "strength_max": 0.2}')).toBeNull(); + }); + + it('accepts already-parsed objects', () => { + expect(parseStrengthRange({ strength_min: 0.3 })).toEqual({ + min: 0.3, + max: null, + recommended: null, + }); + }); +}); + +describe('describeStrengthRangeViolation', () => { + const range = { min: 0.4, max: 0.8, recommended: 0.6 }; + + it('returns null when the value is inside the range', () => { + expect(describeStrengthRangeViolation(0.6, range)).toBeNull(); + expect(describeStrengthRangeViolation(0.4, range)).toBeNull(); + expect(describeStrengthRangeViolation(0.8, range)).toBeNull(); + }); + + it('describes values below the range', () => { + expect(describeStrengthRangeViolation(0.2, range)).toBe( + 'Below recommended strength range (0.40\u20130.80); recommended: 0.60' + ); + }); + + it('describes values above the range', () => { + expect(describeStrengthRangeViolation('1.0', range)).toBe( + 'Above recommended strength range (0.40\u20130.80); recommended: 0.60' + ); + }); + + it('omits the recommended part when not configured', () => { + expect(describeStrengthRangeViolation(0.1, { min: 0.4, max: null, recommended: null })).toBe( + 'Below recommended strength range (\u2265 0.40)' + ); + expect(describeStrengthRangeViolation(1.5, { min: null, max: 1.0, recommended: null })).toBe( + 'Above recommended strength range (\u2264 1.00)' + ); + }); + + it('returns null without a range or with a non-numeric value', () => { + expect(describeStrengthRangeViolation(0.1, null)).toBeNull(); + expect(describeStrengthRangeViolation('abc', range)).toBeNull(); + }); +}); + +describe('applyStrengthRangeCue', () => { + const range = { min: 0.4, max: 0.8, recommended: 0.6 }; + + it('adds the cue class and tooltip for out-of-range values', () => { + const input = document.createElement('input'); + applyStrengthRangeCue(input, 1.5, range); + expect(input.classList.contains('lm-strength-out-of-range')).toBe(true); + expect(input.title).toContain('Above recommended strength range'); + }); + + it('clears the cue for in-range values', () => { + const input = document.createElement('input'); + applyStrengthRangeCue(input, 1.5, range); + applyStrengthRangeCue(input, 0.6, range); + expect(input.classList.contains('lm-strength-out-of-range')).toBe(false); + expect(input.hasAttribute('title')).toBe(false); + }); + + it('clears the cue when no range is configured', () => { + const input = document.createElement('input'); + input.classList.add('lm-strength-out-of-range'); + input.title = 'stale'; + applyStrengthRangeCue(input, 99, null); + expect(input.classList.contains('lm-strength-out-of-range')).toBe(false); + expect(input.hasAttribute('title')).toBe(false); + }); +}); + +describe('buildStrengthRangeMap', () => { + it('keys ranges by normalized path and basename', () => { + const map = buildStrengthRangeMap([ + { file_name: 'sub/a', usage_tips: '{"strength_min": 0.4, "strength_max": 0.8}' }, + ]); + expect(map.get('sub/a')).toEqual({ min: 0.4, max: 0.8, recommended: null }); + expect(map.get('a')).toEqual({ min: 0.4, max: 0.8, recommended: null }); + }); + + it('strips extensions from keys', () => { + const map = buildStrengthRangeMap([ + { file_name: 'sub/a.safetensors', usage_tips: '{"strength_max": 1.0}' }, + ]); + expect(map.get('sub/a')).toBeTruthy(); + }); + + it('skips entries without a valid range', () => { + const map = buildStrengthRangeMap([ + { file_name: 'a', usage_tips: '' }, + { file_name: 'b' }, + { file_name: 'c', usage_tips: '{"strength": 0.6}' }, + null, + ]); + expect(map.size).toBe(0); + }); +}); + +describe('getLoraStrengthRange', () => { + beforeEach(() => { + fetchApiMock.mockReset(); + resetAvailableLorasCache(); + }); + + it('returns null while the cache is not loaded', () => { + expect(getLoraStrengthRange('a')).toBeNull(); + }); + + it('resolves ranges from the cached cycler list', async () => { + fetchApiMock.mockResolvedValue({ + ok: true, + json: async () => ({ + success: true, + loras: [ + { + file_name: 'sub/a.safetensors', + usage_tips: '{"strength": 0.6, "strength_min": 0.4, "strength_max": 0.8}', + }, + { file_name: 'b.safetensors' }, + ], + }), + }); + + await getAvailableLoras(); + expect(getLoraStrengthRange('sub/a.safetensors')).toEqual({ + min: 0.4, + max: 0.8, + recommended: 0.6, + }); + // Extension-free and basename forms resolve to the same entry. + expect(getLoraStrengthRange('sub/a')).toEqual({ + min: 0.4, + max: 0.8, + recommended: 0.6, + }); + expect(getLoraStrengthRange('a')).toEqual({ + min: 0.4, + max: 0.8, + recommended: 0.6, + }); + }); + + it('falls back to the basename for folder-qualified names', async () => { + fetchApiMock.mockResolvedValue({ + ok: true, + json: async () => ({ + success: true, + loras: [ + { file_name: 'sub/a.safetensors', usage_tips: '{"strength_max": 1.0}' }, + ], + }), + }); + + await getAvailableLoras(); + expect(getLoraStrengthRange('any/folder/a.safetensors')).toEqual({ + min: null, + max: 1.0, + recommended: null, + }); + }); + + it('returns null for loras without a configured range', async () => { + fetchApiMock.mockResolvedValue({ + ok: true, + json: async () => ({ + success: true, + loras: [{ file_name: 'b.safetensors' }], + }), + }); + + await getAvailableLoras(); + expect(getLoraStrengthRange('b')).toBeNull(); + expect(getLoraStrengthRange('missing')).toBeNull(); + }); + + it('returns null for absolute paths', async () => { + fetchApiMock.mockResolvedValue({ + ok: true, + json: async () => ({ + success: true, + loras: [ + { file_name: 'a.safetensors', usage_tips: '{"strength_max": 1.0}' }, + ], + }), + }); + + await getAvailableLoras(); + expect(getLoraStrengthRange('/abs/path/a.safetensors')).toBeNull(); + expect(getLoraStrengthRange('C:/abs/path/a.safetensors')).toBeNull(); + }); +}); diff --git a/tests/services/test_lora_cycler_list.py b/tests/services/test_lora_cycler_list.py new file mode 100644 index 00000000..64ad234d --- /dev/null +++ b/tests/services/test_lora_cycler_list.py @@ -0,0 +1,64 @@ +"""Tests for LoraService.get_cycler_list usage_tips exposure.""" + +import pytest +from unittest.mock import Mock, AsyncMock + +from py.services.lora_service import LoraService + + +@pytest.fixture +def lora_service(): + """Create a LoraService instance with a mocked scanner cache.""" + scanner = Mock() + cache_mock = Mock() + cache_mock.raw_data = [ + { + "file_name": "with_tips.safetensors", + "folder": "sub", + "model_name": "With Tips", + "usage_tips": '{"strength": 0.6, "strength_min": 0.4, "strength_max": 0.8}', + }, + { + "file_name": "empty_tips.safetensors", + "folder": "", + "model_name": "Empty Tips", + "usage_tips": "", + }, + { + "file_name": "no_tips.safetensors", + "folder": "", + "model_name": "No Tips", + }, + ] + scanner.get_cached_data = AsyncMock(return_value=cache_mock) + return LoraService(scanner) + + +@pytest.mark.asyncio +async def test_cycler_list_includes_usage_tips_when_present(lora_service): + loras = await lora_service.get_cycler_list() + + with_tips = next(l for l in loras if l["file_name"] == "sub/with_tips.safetensors") + assert with_tips["usage_tips"] == ( + '{"strength": 0.6, "strength_min": 0.4, "strength_max": 0.8}' + ) + + +@pytest.mark.asyncio +async def test_cycler_list_omits_usage_tips_when_empty_or_missing(lora_service): + loras = await lora_service.get_cycler_list() + + empty_tips = next(l for l in loras if l["file_name"] == "empty_tips.safetensors") + no_tips = next(l for l in loras if l["file_name"] == "no_tips.safetensors") + assert "usage_tips" not in empty_tips + assert "usage_tips" not in no_tips + + +@pytest.mark.asyncio +async def test_cycler_list_keeps_existing_fields(lora_service): + loras = await lora_service.get_cycler_list() + + with_tips = next(l for l in loras if l["model_name"] == "With Tips") + assert with_tips["file_name"] == "sub/with_tips.safetensors" + assert with_tips["folder"] == "sub" + assert with_tips["model_name"] == "With Tips" diff --git a/web/comfyui/lm_styles.css b/web/comfyui/lm_styles.css index 235bba83..efdd6638 100644 --- a/web/comfyui/lm_styles.css +++ b/web/comfyui/lm_styles.css @@ -309,6 +309,18 @@ box-shadow: 0 0 0 2px rgba(66, 153, 225, 0.3); } +/* Advisory cue: strength is outside the usage-tips recommended range. + Amber to distinguish it from the red "missing LoRA" error state. */ +.lm-lora-strength-input.lm-strength-out-of-range { + border-color: rgba(245, 158, 11, 0.75); + color: rgba(252, 211, 77, 0.95); +} + +.lm-lora-strength-input.lm-strength-out-of-range:focus { + border-color: rgba(245, 158, 11, 0.9); + box-shadow: 0 0 0 2px rgba(245, 158, 11, 0.3); +} + .lm-lora-strength-buttons { display: flex; flex-direction: column; diff --git a/web/comfyui/loras_widget.js b/web/comfyui/loras_widget.js index cec0f426..3b1d5047 100644 --- a/web/comfyui/loras_widget.js +++ b/web/comfyui/loras_widget.js @@ -7,6 +7,8 @@ import { syncClipStrengthIfCollapsed, getAvailableLoras, getAvailableLorasSync, + getLoraStrengthRange, + applyStrengthRangeCue, isLoraNameAvailable, onLibraryChanged } from "./loras_widget_utils.js"; @@ -521,6 +523,7 @@ export function addLorasWidget(node, name, opts, callback) { strengthEl.classList.add("lm-lora-strength-input"); strengthEl.type = "text"; strengthEl.value = typeof strength === 'number' ? strength.toFixed(2) : Number(strength).toFixed(2); + applyStrengthRangeCue(strengthEl, strength, getLoraStrengthRange(name)); strengthEl.addEventListener('pointerdown', () => { pendingFocusTarget = { name, type: "strength" }; }); @@ -650,6 +653,7 @@ export function addLorasWidget(node, name, opts, callback) { clipStrengthEl.classList.add("lm-lora-strength-input", "lm-lora-clip-strength-input"); clipStrengthEl.type = "text"; clipStrengthEl.value = typeof clipStrength === 'number' ? clipStrength.toFixed(2) : Number(clipStrength).toFixed(2); + applyStrengthRangeCue(clipStrengthEl, clipStrength, getLoraStrengthRange(name)); clipStrengthEl.addEventListener('pointerdown', () => { pendingFocusTarget = { name, type: "clip" }; }); diff --git a/web/comfyui/loras_widget_events.js b/web/comfyui/loras_widget_events.js index ed0da25c..8037cd92 100644 --- a/web/comfyui/loras_widget_events.js +++ b/web/comfyui/loras_widget_events.js @@ -1,7 +1,7 @@ import { api } from "../../scripts/api.js"; import { app } from "../../scripts/app.js"; import { createMenuItem, createDropIndicator } from "./loras_widget_components.js"; -import { parseLoraValue, formatLoraValue, syncClipStrengthIfCollapsed, saveRecipeDirectly, copyToClipboard, showToast, moveLoraByDirection, getDropTargetIndex } from "./loras_widget_utils.js"; +import { parseLoraValue, formatLoraValue, syncClipStrengthIfCollapsed, saveRecipeDirectly, copyToClipboard, showToast, moveLoraByDirection, getDropTargetIndex, getLoraStrengthRange, applyStrengthRangeCue } from "./loras_widget_utils.js"; // Function to handle strength adjustment via dragging export function handleStrengthDrag(name, initialStrength, initialX, event, widget, isClipStrength = false, updateWidget = true) { @@ -194,6 +194,7 @@ export function initDrag( const strengthInput = currentDragElement.querySelector('.lm-lora-strength-input'); if (strengthInput && typeof newStrength === 'number') { strengthInput.value = newStrength.toFixed(2); + applyStrengthRangeCue(strengthInput, newStrength, getLoraStrengthRange(name)); } // Prevent showing the preview tooltip during drag @@ -335,6 +336,10 @@ export function initHeaderDrag(headerEl, widget, renderFunction) { if (lorasData[index]) { input.value = lorasData[index].strength.toFixed(2); } + const entryEl = input.closest('[data-lora-name]'); + if (entryEl) { + applyStrengthRangeCue(input, input.value, getLoraStrengthRange(entryEl.dataset.loraName)); + } }); }); diff --git a/web/comfyui/loras_widget_utils.js b/web/comfyui/loras_widget_utils.js index 4b23bfad..1af41b48 100644 --- a/web/comfyui/loras_widget_utils.js +++ b/web/comfyui/loras_widget_utils.js @@ -88,6 +88,158 @@ let availableLorasCache = null; let availableLorasPromise = null; let availabilityGeneration = 0; +/** + * Parse a numeric usage-tips value (accepts numbers and numeric strings). + */ +function parseUsageTipNumber(value) { + if (typeof value === "number" && Number.isFinite(value)) { + return value; + } + if (typeof value === "string") { + const parsed = parseFloat(value); + if (Number.isFinite(parsed)) { + return parsed; + } + } + return null; +} + +/** + * Parse a "x.x-y.y" range string, tolerating whitespace and negatives. + */ +function parseRangeString(value) { + if (typeof value !== "string") { + return null; + } + const match = value.trim().match(/^(-?\d+(?:\.\d+)?)\s*-\s*(-?\d+(?:\.\d+)?)$/); + if (!match) { + return null; + } + return { min: parseFloat(match[1]), max: parseFloat(match[2]) }; +} + +/** + * Extract the recommended strength range from a usage_tips payload (JSON + * string or already-parsed object). Explicit strength_min/strength_max keys + * take precedence over the strength_range "x.x-y.y" shorthand. Returns + * { min, max, recommended } with null bounds for open-ended ranges, or null + * when no valid range is configured. + */ +export function parseStrengthRange(usageTips) { + let tips = usageTips; + if (typeof tips === "string") { + if (!tips.trim()) { + return null; + } + try { + tips = JSON.parse(tips); + } catch { + return null; + } + } + if (!tips || typeof tips !== "object") { + return null; + } + + let min = parseUsageTipNumber(tips.strength_min ?? tips.strengthMin); + let max = parseUsageTipNumber(tips.strength_max ?? tips.strengthMax); + + if (min === null || max === null) { + const parsed = parseRangeString(tips.strength_range ?? tips.strengthRange); + if (parsed) { + if (min === null) min = parsed.min; + if (max === null) max = parsed.max; + } + } + + if (min === null && max === null) { + return null; + } + if (min !== null && max !== null && min > max) { + return null; + } + + return { min, max, recommended: parseUsageTipNumber(tips.strength) }; +} + +/** + * Describe how a strength value violates the recommended range, or return + * null when it is inside (or no range is configured). The returned string is + * used as the input's tooltip. + */ +export function describeStrengthRangeViolation(value, range) { + if (!range) { + return null; + } + const numeric = Number(value); + if (!Number.isFinite(numeric)) { + return null; + } + const { min, max, recommended } = range; + + let direction = null; + if (min !== null && numeric < min) { + direction = "Below"; + } else if (max !== null && numeric > max) { + direction = "Above"; + } + if (!direction) { + return null; + } + + let rangeText; + if (min !== null && max !== null) { + rangeText = `${min.toFixed(2)}\u2013${max.toFixed(2)}`; + } else if (min !== null) { + rangeText = `\u2265 ${min.toFixed(2)}`; + } else { + rangeText = `\u2264 ${max.toFixed(2)}`; + } + + let text = `${direction} recommended strength range (${rangeText})`; + if (recommended !== null) { + text += `; recommended: ${recommended.toFixed(2)}`; + } + return text; +} + +/** + * Toggle the out-of-range visual cue on a strength input. + */ +export function applyStrengthRangeCue(inputEl, value, range) { + const message = describeStrengthRangeViolation(value, range); + inputEl.classList.toggle("lm-strength-out-of-range", message !== null); + if (message) { + inputEl.title = message; + } else { + inputEl.removeAttribute("title"); + } +} + +/** + * Build the lookup map of recommended strength ranges from cycler-list + * entries. Keyed like the availability set (normalized path and basename). + */ +export function buildStrengthRangeMap(loras) { + const map = new Map(); + for (const lora of loras || []) { + const range = parseStrengthRange(lora?.usage_tips); + if (!range) { + continue; + } + const normalized = normalizeLoraNameKey(lora?.file_name); + if (!normalized) { + continue; + } + map.set(normalized, range); + const slash = normalized.lastIndexOf("/"); + if (slash >= 0) { + map.set(normalized.slice(slash + 1), range); + } + } + return map; +} + async function refreshAvailableLoras() { const generation = availabilityGeneration; try { @@ -100,16 +252,18 @@ async function refreshAvailableLoras() { return null; } const data = await response.json(); - const paths = (data?.loras || []) + const loras = data?.loras || []; + const paths = loras .map((lora) => lora?.file_name) .filter(Boolean); const set = buildAvailableLoraSet(paths); + const ranges = buildStrengthRangeMap(loras); if (generation !== availabilityGeneration) { // Stale response: the cache was invalidated while this fetch was in // flight, do not repopulate it with pre-change data. return null; } - availableLorasCache = { set, at: Date.now() }; + availableLorasCache = { set, ranges, at: Date.now() }; return set; } catch (error) { console.warn("Failed to fetch available LoRAs:", error); @@ -151,6 +305,38 @@ export function getAvailableLorasSync() { return null; } +/** + * Synchronous lookup of a LoRA's recommended strength range from the cached + * cycler-list data. Mirrors isLoraNameAvailable's matching: basename fallback + * for folder-qualified names, and null for absolute paths or while the cache + * is not loaded (no cue is shown in those cases). + */ +export function getLoraStrengthRange(name) { + if ( + !availableLorasCache || + Date.now() - availableLorasCache.at >= AVAILABLE_LORAS_TTL_MS + ) { + return null; + } + const ranges = availableLorasCache.ranges; + if (!ranges || ranges.size === 0) { + return null; + } + const normalized = String(name || "").replace(/\\/g, "/"); + if (normalized.startsWith("/") || /^[A-Za-z]:\//.test(normalized)) { + return null; + } + const key = normalizeLoraNameKey(name); + if (ranges.has(key)) { + return ranges.get(key); + } + const slash = key.lastIndexOf("/"); + if (slash >= 0) { + return ranges.get(key.slice(slash + 1)) || null; + } + return null; +} + /** * Drop the cached availability data (used by tests and by callers that need * a forced refresh of the local library state). In-flight fetches started