diff --git a/TESTING.md b/TESTING.md index bf6901f..3a62a6a 100644 --- a/TESTING.md +++ b/TESTING.md @@ -3587,7 +3587,8 @@ npm run --prefix tools/vendor test:workflows -- workspace-system-integrations.sp System › Auto-tagging (`web/js/workspace/system-tagging.mjs`, D132–D134) edits one server-side tagging configuration, `autotag.defaults` in config.json: `model_type` (`wd14-swinv2`, `wd14-vit`, `gguf`, `hf`), `existing` (`skip`, `merge`, -`replace_after_review`), `general_threshold` and `character_threshold` (0–1), +`replace_after_review`), `general_threshold` and `character_threshold` (0–1 in +steps of 0.05, #369), `template` (non-blank, at most 4,000 characters and 16 KiB) and `suggestion_limit` (1–100). They are host drafts (`autotag_model_type`, `autotag_existing`, `autotag_general_threshold`, `autotag_character_threshold`, `autotag_template`, @@ -3598,7 +3599,13 @@ naming the setting by its row label ("General-tag confidence must be between 0 a `settingRefusal` in system.mjs only for a save that carried `autotag_*` keys; every other section's plain 400 keeps the generic status) and the control is highlighted. The template's limit counts code points, as Python's `len` does. A hand-edited bad value -falls back to its default on load, one key at a time. `GET /autotag/models` reports the +falls back to its default on load, one key at a time. Every page shows the thresholds in +steps of 0.05 (the System rows, the Tag image dialogs, the legacy sliders), so a saved +threshold off that grid is refused ("General-tag confidence must be a multiple of +0.05.", `validate_saved_threshold`; `draftProblem` holds back Preview tagging review and +the batch Review with the same sentence) and falls back on load; an accepted one is +stored exactly on the grid (#369). A per-request threshold keeps its own check: `/image-annotations/suggest` and +`/jobs/autotag` take 0–1; the legacy `/autotag/single` and `/autotag/start` only require a number. `GET /autotag/models` reports the saved values (`wd14_general_threshold`, `wd14_character_threshold`, `suggestion_limit`, `default_prompt`, `model_type`, `existing`), so the Tag image and Auto-tag selected dialogs start from them; a preset (the bulk review, Preview tagging review) wins. diff --git a/py/api/settings_routes.py b/py/api/settings_routes.py index c38274c..eb3da04 100644 --- a/py/api/settings_routes.py +++ b/py/api/settings_routes.py @@ -173,8 +173,8 @@ def _validate_autotag_settings(data): EXISTING_POLICIES, MAX_SUGGESTION_LIMIT, MODEL_TYPES, + validate_saved_threshold, validate_template, - validate_threshold, ) updates = {} @@ -198,7 +198,7 @@ def _validate_autotag_settings(data): ): if key in data: try: - value = validate_threshold(label, data[key]) + value = validate_saved_threshold(label, data[key]) except ValueError as error: raise ValueError(f"{error}.") from None updates[key.upper()] = value diff --git a/py/config.py b/py/config.py index 2a14023..6bbf8ba 100644 --- a/py/config.py +++ b/py/config.py @@ -573,6 +573,7 @@ try: MAX_INSTRUCTION_CHARS, MODEL_TYPES, WD14_MODEL_TYPES, + validate_saved_threshold, validate_template, validate_threshold, ) @@ -585,6 +586,7 @@ except ImportError: MAX_INSTRUCTION_CHARS, MODEL_TYPES, WD14_MODEL_TYPES, + validate_saved_threshold, validate_template, validate_threshold, ) @@ -631,12 +633,12 @@ class AutotagConfig: "existing": ("EXISTING", _valid_existing, "skip"), "general_threshold": ( "GENERAL_THRESHOLD", - lambda v: validate_threshold("value", v), + lambda v: validate_saved_threshold("value", v), 0.35, ), "character_threshold": ( "CHARACTER_THRESHOLD", - lambda v: validate_threshold("value", v), + lambda v: validate_saved_threshold("value", v), 0.85, ), "template": ("TEMPLATE", validate_template, DEFAULT_TAG_TEMPLATE), diff --git a/py/tagging_defaults.py b/py/tagging_defaults.py index dedaf9b..be75c9e 100644 --- a/py/tagging_defaults.py +++ b/py/tagging_defaults.py @@ -39,6 +39,25 @@ def validate_threshold(name: str, value: Any) -> float: return float(value) +# Every page shows the thresholds in steps of 0.05 (the concept's System rows, +# the Tag image dialogs and the legacy sliders), so a saved one must be on that +# grid (#369). Per-request thresholds keep their own checks. +THRESHOLD_STEP = 0.05 +_STEPS_PER_UNIT = round(1 / THRESHOLD_STEP) +# A computed float can miss the grid by a hair: (0.1 + 0.2) * 20 is 6.000000000000001. +_STEP_TOLERANCE = 1e-12 # in steps; refuses 0.35000000001 + + +def validate_saved_threshold(name: str, value: Any) -> float: + """A saved WD14 threshold: in [0, 1] and a multiple of ``THRESHOLD_STEP``, + returned exactly on the grid; else ValueError naming ``name``.""" + value = validate_threshold(name, value) + steps = value * _STEPS_PER_UNIT + if abs(steps - round(steps)) > _STEP_TOLERANCE: + raise ValueError(f"{name} must be a multiple of {THRESHOLD_STEP}") + return round(steps) / _STEPS_PER_UNIT + + def validate_template(value: Any, name: str = "instructions") -> str: """Non-empty language-model instructions within the character and byte caps, or ValueError naming ``name``.""" diff --git a/tests/browser/workspace-system-tagging.spec.mjs b/tests/browser/workspace-system-tagging.spec.mjs index 2619135..a2720b1 100644 --- a/tests/browser/workspace-system-tagging.spec.mjs +++ b/tests/browser/workspace-system-tagging.spec.mjs @@ -188,6 +188,35 @@ test('a refused value shows the server\'s message', async ({ page, request }) => await expect(actionOf(page, 'Preview tagging review')).not.toHaveAttribute('aria-disabled', 'true'); }); +test('a value off the 0.05 step is refused and never reaches the Tag image dialog (#369)', async ({ page, request }) => { + const before = (await serverJson(request, SETTINGS_URL)).settings; + const STEP = 'General-tag confidence must be a multiple of 0.05.'; + await selectThenOpen(page, FIRST); + await page.locator('#threshold-general').fill('0.37'); + await page.locator('#threshold-general').press('Tab'); + // Preview tagging review explains instead of opening a dialog whose Generate the browser would block. + await expect(actionOf(page, 'Preview tagging review')).toHaveAttribute('aria-disabled', 'true'); + await actionOf(page, 'Preview tagging review').focus(); + await page.keyboard.press('Enter'); + await expect(toast(page)).toHaveText(STEP); + await expect(tagDialog(page)).toHaveCount(0); + // The server refuses the save with the same sentence and writes nothing. + await save(page).click(); + await expect(status(page)).toHaveText(STEP); + expect((await serverJson(request, SETTINGS_URL)).settings.autotag_general_threshold).toBe(before.autotag_general_threshold); + // A value on the step opens the dialog, and its Generate passes the browser's own check and sends. + await page.locator('#threshold-general').fill('0.4'); + await page.locator('#threshold-general').press('Tab'); + await expect(actionOf(page, 'Preview tagging review')).not.toHaveAttribute('aria-disabled', 'true'); + await actionOf(page, 'Preview tagging review').click(); + await expect(tagDialog(page).locator('#image-general-threshold')).toHaveValue('0.4'); + expect(await tagDialog(page).locator('#image-general-threshold').evaluate(input => input.checkValidity())).toBe(true); + const suggested = page.waitForResponse(item => item.url().endsWith('/image-annotations/suggest')); + await tagDialog(page).getByRole('button', { name: 'Generate suggestions', exact: true }).click(); + const answer = await suggested; + expect(answer.request().postDataJSON().options.general_threshold).toBe(0.4); +}); + test('Download starts the job and Activity shows it without Cancel', async ({ page, request }) => { await unavailable(request, ['wd14-vit']); await fakeDownload(request, 'hold'); diff --git a/tests/js/workspace-system-tagging.test.js b/tests/js/workspace-system-tagging.test.js index 5cf30e2..a8a9c6d 100644 --- a/tests/js/workspace-system-tagging.test.js +++ b/tests/js/workspace-system-tagging.test.js @@ -307,6 +307,16 @@ test('Preview tagging review opens the Tag image dialog with the section’s val assert.deepEqual(draftProblem({ ...HOST, autotag_general_threshold: 1.5 }), { key: 'autotag_general_threshold', message: 'General-tag confidence must be between 0 and 1.' }); assert.equal(draftProblem({ ...HOST, autotag_character_threshold: '' }).key, 'autotag_character_threshold'); + // #369: every page shows the thresholds in steps of 0.05, so a value off that grid cannot be saved or previewed. + assert.deepEqual(draftProblem({ ...HOST, autotag_general_threshold: 0.37 }), + { key: 'autotag_general_threshold', message: 'General-tag confidence must be a multiple of 0.05.' }); + assert.deepEqual(draftProblem({ ...HOST, autotag_character_threshold: 0.33 }), + { key: 'autotag_character_threshold', message: 'Character-tag confidence must be a multiple of 0.05.' }); + assert.equal(draftProblem({ ...HOST, autotag_general_threshold: 0.15, autotag_character_threshold: 0.95 }), null); + assert.equal(draftProblem({ ...HOST, autotag_general_threshold: 0, autotag_character_threshold: 1 }), null); + // A computed value a hair off the grid passes and is handed on snapped to it, as the server stores it. + assert.equal(draftProblem({ ...HOST, autotag_general_threshold: 0.1 + 0.2 }), null); + assert.equal(taggingPreset({ ...HOST, autotag_general_threshold: 0.1 + 0.2 }).options.general_threshold, 0.3); assert.equal(draftProblem({ ...HOST, autotag_suggestion_limit: 2.5 }).key, 'autotag_suggestion_limit'); assert.equal(draftProblem({ ...HOST, autotag_suggestion_limit: 0 }, { limit: false }), null); assert.deepEqual(draftProblem({ ...HOST, autotag_model_type: 'hf', autotag_template: ' ' }), @@ -342,6 +352,11 @@ test('Preview tagging review opens the Tag image dialog with the section’s val await button('Preview tagging review').dispatch('click'); assert.deepEqual(calls.toasts, ['General-tag confidence must be between 0 and 1.']); assert.equal(calls.opened.length, 1); + byId('threshold-general').value = '0.37'; await byId('threshold-general').dispatch('change'); + assert.equal(button('Preview tagging review').getAttribute('aria-disabled'), 'true'); + await button('Preview tagging review').dispatch('click'); + assert.deepEqual(calls.toasts.at(-1), 'General-tag confidence must be a multiple of 0.05.'); + assert.equal(calls.opened.length, 1); } finally { scope.dispose(); dom.restore(); } }); @@ -361,6 +376,8 @@ test('the batch review grid, body and Cancel-only reasons (D134, CUJ 4–5)', as assert.equal(existingLabel('odd'), 'odd'); assert.deepEqual(batchBody(HOST), { model_type: 'wd14-swinv2', existing: 'skip', options: { general_threshold: 0.35, character_threshold: 0.85 } }); + // #369: a threshold a hair off the grid is sent snapped to it. + assert.equal(batchBody({ ...HOST, autotag_character_threshold: 0.1 + 0.2 }).options.character_threshold, 0.3); assert.deepEqual(batchBody(llm), { model_type: 'gguf', existing: 'skip', options: { instructions: HOST.autotag_template } }); assert.equal(batchCloseNote(PLAN), null); assert.equal(batchCloseNote({ ...PLAN, review_required: true }), REPLACE_NOTE); diff --git a/tests/test_config.py b/tests/test_config.py index 7a9e8ba..d099bd6 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -516,8 +516,9 @@ class TestAutotagConfig(unittest.TestCase): bad_values = { "model_type": ["wd14-convnext", 3, None], "existing": ["replace", "", True], - "general_threshold": [1.5, -0.1, float("nan"), "0.4", True], - "character_threshold": [2, float("inf"), None], + # #369: a saved threshold off the 0.05 step no page can show. + "general_threshold": [1.5, -0.1, float("nan"), "0.4", True, 0.37], + "character_threshold": [2, float("inf"), None, 0.33], # 4000 characters are at most 16,000 UTF-8 bytes, so the 16 KiB # byte cap can only be met past the character cap. "template": ["", " ", "x" * 4001, "\U0001f600" * 4097, "\ud800", 7], @@ -567,6 +568,27 @@ class TestAutotagConfig(unittest.TestCase): config_module.MODEL_TYPES, {"gguf", "hf", "wd14-swinv2", "wd14-vit"} ) + def test_saved_threshold_is_a_multiple_of_the_step(self): + """#369: every page shows the thresholds in steps of 0.05, so a saved + one must be on that grid; it is stored exactly on it.""" + from py.tagging_defaults import validate_saved_threshold as validate + + for value in (0, 0.05, 0.15, 0.35, 0.85, 1): + with self.subTest(value=value): + self.assertEqual(validate("x", value), value) + self.assertEqual(validate("x", 0.1 + 0.2), 0.3) + for value, message in ( + (0.37, "x must be a multiple of 0.05"), + (0.351, "x must be a multiple of 0.05"), + (0.35000000001, "x must be a multiple of 0.05"), + (1.5, "x must be between 0 and 1"), + ("0.4", "x must be a finite number"), + ): + with self.subTest(value=value): + with self.assertRaises(ValueError) as caught: + validate("x", value) + self.assertEqual(str(caught.exception), message) + def test_suggestion_limit_setting_validated_on_load(self): for value in (0, 101, "10", float("nan"), 2.5, True, None, [10]): with self.subTest(value=value): diff --git a/tests/test_settings_sources.py b/tests/test_settings_sources.py index e85549f..3ab7b03 100644 --- a/tests/test_settings_sources.py +++ b/tests/test_settings_sources.py @@ -1037,6 +1037,23 @@ class TestAutotagDefaultsSettings(SourcesTestCase): settings = (await self._get())["settings"] self.assertEqual({key: settings[key] for key in self.SAVED}, self.SAVED) + async def test_threshold_near_the_grid_is_saved_on_it(self): + """#369: a computed value a hair off the 0.05 grid is stored on it, so + every page's step check accepts what /settings and the file return.""" + revision = (await self._get())["revision"] + status, data = await self._post( + "/prompt_manager/settings", + {"expected_revision": revision, "autotag_general_threshold": 0.1 + 0.2}, + ) + self.assertEqual(status, 200, data) + self.assertEqual( + repr(self._read_config()["autotag"]["defaults"]["general_threshold"]), + "0.3", + ) + self.assertEqual( + repr((await self._get())["settings"]["autotag_general_threshold"]), "0.3" + ) + async def test_stale_revision_changes_no_default(self): revision = (await self._get())["revision"] self._write_config({"custom": {"edited": "elsewhere"}}) @@ -1055,6 +1072,16 @@ class TestAutotagDefaultsSettings(SourcesTestCase): 1.5, "General-tag confidence must be between 0 and 1.", ), + ( + "autotag_general_threshold", + 0.37, + "General-tag confidence must be a multiple of 0.05.", + ), + ( + "autotag_character_threshold", + 0.33, + "Character-tag confidence must be a multiple of 0.05.", + ), ( "autotag_character_threshold", "0.8", diff --git a/web/js/workspace/system-tagging.mjs b/web/js/workspace/system-tagging.mjs index f667587..aaea542 100644 --- a/web/js/workspace/system-tagging.mjs +++ b/web/js/workspace/system-tagging.mjs @@ -85,6 +85,9 @@ export const STATUS_COPY = Object.freeze({ export const SETTING_PROBLEMS = Object.freeze({ autotag_general_threshold: 'General-tag confidence must be between 0 and 1.', autotag_character_threshold: 'Character-tag confidence must be between 0 and 1.', + // #369: every page shows the thresholds in steps of 0.05, so the server saves only values on that grid. + autotag_general_threshold_step: 'General-tag confidence must be a multiple of 0.05.', + autotag_character_threshold_step: 'Character-tag confidence must be a multiple of 0.05.', autotag_suggestion_limit: 'Suggestions per image must be a whole number from 1 to 100.', // The server's two template sentences (`validate_template`), blank first. autotag_template: 'LLM tag template must be non-empty text.', @@ -171,10 +174,15 @@ export function draftValue(key, text) { } const unit = value => typeof value === 'number' && Number.isFinite(value) && value >= 0 && value <= 1; +// A multiple of 0.05 within the server's tolerance (`validate_saved_threshold`): (0.1 + 0.2) * 20 is 6.000000000000001. +const onStep = value => Math.abs(value * 20 - Math.round(value * 20)) <= 1e-12; +// A threshold on the grid, handed on exactly as the server stores it (anything else is left for draftProblem to refuse). +const snapped = value => (unit(value) && onStep(value) ? Math.round(value * 20) / 20 : value); /** * The first setting the chosen model would use that the section cannot send, as `{key, message}`, else null: WD14 - * reads both thresholds, the language models the template; Generate also reads the suggestion count (`limit`). + * reads both thresholds (0 to 1 in steps of 0.05, #369), the language models the template; Generate also reads the + * suggestion count (`limit`). */ export function draftProblem(values, { limit = true } = {}) { const model = values.autotag_model_type; @@ -189,6 +197,7 @@ export function draftProblem(values, { limit = true } = {}) { } const valid = key === 'autotag_suggestion_limit' ? Number.isInteger(value) && value >= 1 && value <= 100 : unit(value); if (!valid) return { key, message: SETTING_PROBLEMS[key] }; + if (key !== 'autotag_suggestion_limit' && !onStep(value)) return { key, message: SETTING_PROBLEMS[`${key}_step`] }; } return null; } @@ -197,7 +206,8 @@ export function draftProblem(values, { limit = true } = {}) { export function taggingPreset(values) { const model = values.autotag_model_type; const options = CLASSIFIERS.has(model) - ? { general_threshold: values.autotag_general_threshold, character_threshold: values.autotag_character_threshold } + ? { general_threshold: snapped(values.autotag_general_threshold), + character_threshold: snapped(values.autotag_character_threshold) } : { instructions: values.autotag_template }; return { model, options: { ...options, suggestion_limit: values.autotag_suggestion_limit } }; } @@ -206,7 +216,8 @@ export function taggingPreset(values) { export function batchBody(values) { const model = values.autotag_model_type; const options = CLASSIFIERS.has(model) - ? { general_threshold: values.autotag_general_threshold, character_threshold: values.autotag_character_threshold } + ? { general_threshold: snapped(values.autotag_general_threshold), + character_threshold: snapped(values.autotag_character_threshold) } : { instructions: values.autotag_template }; return { model_type: model, existing: values.autotag_existing, options }; }