From 8a2ea9bd978b913c316daffed3687229350c4a04 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Sat, 31 Oct 2026 19:32:37 -0700 Subject: [PATCH] feat(tagging): saved instructions with a default per engine (#431) Squashed from PR #587. --- TESTING.md | 82 +++- py/api/autotag_routes.py | 26 +- py/api/settings_routes.py | 114 +++++- py/config.py | 177 +++++++- py/tagging_defaults.py | 94 +++++ tests/browser/system-tagging-concept.spec.mjs | 89 +++- .../browser/workspace-system-tagging.spec.mjs | 78 +++- tests/browser/workspace_tagging_fixture.py | 25 +- tests/fixtures/legacy_contract_snapshot.json | 3 + .../js/workspace-system-instructions.test.js | 257 ++++++++++++ tests/js/workspace-system-tagging.test.js | 53 ++- tests/test_api_annotations.py | 2 + tests/test_api_autotag.py | 30 +- tests/test_autotag_legacy.py | 25 +- tests/test_config.py | 24 +- tests/test_saved_instructions.py | 379 ++++++++++++++++++ tests/test_saved_instructions_routes.py | 224 +++++++++++ tests/test_settings_sources.py | 15 +- tools/vendor/frontend-coverage-baseline.json | 6 + web/js/workspace/system-instructions.mjs | 351 ++++++++++++++++ web/js/workspace/system-tagging.mjs | 65 +-- web/js/workspace/workspace.css | 21 + 22 files changed, 2004 insertions(+), 136 deletions(-) create mode 100644 tests/js/workspace-system-instructions.test.js create mode 100644 tests/test_saved_instructions.py create mode 100644 tests/test_saved_instructions_routes.py create mode 100644 web/js/workspace/system-instructions.mjs diff --git a/TESTING.md b/TESTING.md index 22679d64..7408c522 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1886,7 +1886,8 @@ Inspect plan keeps D60–D66 (plan D60 → D83, D61 → D84, D65 → D88, D68 engines in the mock's order with legacy labels and sizes (`WD14 SwinV2 · ~400 MB`, `WD14 ViT · ~400 MB`, `GGUF JoyCaption (Recommended) · ~4.5 GB` in the mock's Ollama slot, `HuggingFace JoyCaption · ~16 GB`); the instructions start - from the server's `default_prompt`. + from the server's `default_prompt` (since #431 the saved engine's default + instructions, GGUF's for a WD14 engine). - **D91** (plan D68, ruling T3-bulk): a **Suggestions** number field (1–100) for every model, prefilled from `autotag.defaults.suggestion_limit` (10); a model change keeps it. @@ -4613,9 +4614,9 @@ the per-section hand-off panel, was retired by S12 (#329): every section is nati engine and policy (read-only) and opens "Review tag output images" in the concept's record-changing review (its `maintenancePlan` shell): `h3` "Tag output images", the `dl.meta-grid` Model, Existing tags, Images, Already auto-tagged, Images to tag - ("about N"), Time, Settings ("General · Character " or "Template of N - characters", then " · up to L tags per image" with the preview's saved - `suggestion_limit`) and Backup, the estimate sentence ("Nothing is written until you + ("about N"), Time, Settings ("General · Character ", then " · up to L tags + per image" with the preview's saved `suggestion_limit`; for a language model, since + D205, an "Instructions" row and "Up to L tags per image") and Backup, the estimate sentence ("Nothing is written until you start. The number to tag is an estimate: images changed since they were tagged are tagged again, and missing files are skipped. Activity reports what was tagged.") in the `p.small`, the D55 backup box and Cancel / "Start tagging", which starts the @@ -4657,6 +4658,31 @@ the per-section hand-off panel, was retired by S12 (#329): every section is nati concept row's), `skipped tags add, preview, remove, save and reload (D202)`, `skipped tags preview checks library tags only …` (workspace-system-tagging.spec) and `suggestions omit skipped tags and say how many were hidden` (workspace-tagging.spec). +- **D203** (Auto-tagging, #431, beyond the concept): "Saved instructions" replaces the + concept's "LLM tag template" field (the mock's state 2): a `.setting-block` + (`data-setting="tag-instructions"`, `data-key="autotag_instructions"`) with the label, + "Instructions for the language-model engines. Built-in can’t be changed; duplicate it + to make your own. N of 20 saved · Built‑in is not counted.", a `role="listbox"` list + (Built-in first with a "Read-only" chip, then the saved entries; "Default for GGUF" / + "Default for HuggingFace" chips), "+ New" ("New instructions", "(2)" if taken), + "Duplicate" (" copy"), "Delete" (`.danger`, disabled for Built-in), "At most 20 + saved instructions." at the cap, and the editor beside the list (under it below 700px): + "Name" ("N of 60") and "Text" ("N of 4,000 characters"), read-only for Built-in with + "Built-in instructions can’t be changed. Duplicate them to make your own.", else the + entry's problem ("The name “X” is already used.", "The text must not be empty."…). + Then two concept select rows, "Default for GGUF JoyCaption" and "Default for + HuggingFace JoyCaption" ("The Tag image dialog and batches start from this.", + `data-key="autotag_default_instructions"`), and the note a deleted default leaves + ("GGUF now uses Built-in."; nothing is written until Save preferences). Pinned by the + comparison's mapping (system-tagging-concept.spec: the block in the concept row's + tokens, the two rows on the concept's select row) and `saved instructions: new, edit, + per-engine default, delete, save and reload (D203)`, `a refused instructions save names + the entry and highlights the block (D203)` (workspace-system-tagging.spec). +- **D205** (Auto-tagging, #431; amends D134): for a language-model engine the batch + review adds a read-only "Instructions" row with the drafted engine default's name + before "Settings", which reads "Up to L tags per image"; the batch and Preview tagging + review send that entry's text. WD14 is unchanged. Pinned by `the batch review names the + language model's default instructions (D205)` (workspace-system-tagging.spec). - **D158** (Sources, #247 D-Q4): the concept has no switch for prompts from workflows without a PromptManager node; the maintainer chose a visible switch over a config key. A `.setting-row` in the concept's toggle markup follows "Watch for new outputs": @@ -5283,16 +5309,16 @@ server-side tagging configuration, `autotag.defaults` in config.json: `model_typ (`wd14-swinv2`, `wd14-vit`, `gguf`, `hf`), `existing` (`skip`, `merge`, `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`, -`autotag_suggestion_limit`) saved by the one Save row through `POST +`suggestion_limit` (1–100), and since #431 `skipped_tags`, `instructions` and +`default_instructions` (below). They are host drafts (`autotag_model_type`, `autotag_existing`, +`autotag_general_threshold`, `autotag_character_threshold`, `autotag_suggestion_limit`, +`autotag_skipped_tags`, `autotag_instructions`, `autotag_default_instructions`) saved by the one Save row through `POST /prompt_manager/settings` with `expected_revision`; a bad value is refused with a 400 naming the setting by its row label ("General-tag confidence must be between 0 and 1."), nothing is written, the Save row shows that sentence (`invalid-setting`, set by `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 +Instruction text limits count code points, as Python's `len` does. A hand-edited bad value 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 @@ -5303,8 +5329,42 @@ stored exactly on the grid (#369). A per-request threshold keeps its own check: `validate_options` and starts the `autotag` job titled "Tag output images (legacy page)"; the legacy `/autotag/single` only requires 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. +`default_prompt`, `model_type`, `existing`, and since #431 `instructions` and +`default_instructions`), so the Tag image and Auto-tag selected dialogs start from them; +a preset (the bulk review, Preview tagging review) wins. + +**Saved instructions** (#431, D203, D205): `autotag.defaults.instructions` holds the +saved entries `[{id, name, text}]` in order (at most 20; `id` 12 lowercase hex made by +the client, unique, `builtin` reserved; `name` trimmed, 1–60 characters, no control +character, unique case-folded, not "Built-in"; `text` as `validate_template`), and +`autotag.defaults.default_instructions` is `{gguf, hf}`, each `builtin` or a saved id. +Built-in (`{id: "builtin", name: "Built-in", text: DEFAULT_TAG_TEMPLATE}`) is never +stored; readers put it first (`AutotagConfig.all_instructions`), so a later release's +built-in text reaches everyone who uses it. Refusals (400, the Save row shows them and +highlights the editor): "Saved instructions: at most 20 entries besides Built-in.", +"Saved instructions: entry N has an invalid id.", "… repeats the id of entry M.", "… +needs a name.", "… name must be at most 60 characters.", "… name must not contain a line +break.", "… name “X” is already used.", "Saved instructions: the name “Built-in” is +reserved.", "Saved instructions: entry N text must be …", "Default for GGUF JoyCaption +must be one of the saved instructions." (and HuggingFace). A save carrying either key +without `expected_revision` is refused ("Reload the page and try again."); a save +carrying one is paired with the other from the running configuration and both are +written, plus the old `template` key as the GGUF default's text (deleted when that +default is Built-in), so an older build reads that default as its one template. `POST` +with `autotag_template` is refused ("LLM tag template was replaced by Saved instructions. +Reload the page."); `GET` keeps returning it (the GGUF default's text, read-only) for one +release and adds `autotag_builtin_instructions` (Built-in's text, read-only). Load: each +key alone (a damaged list → Built-in only, a default naming nothing → Built-in, each with +one warning), then the in-memory migration of the old `template`: with no `instructions` +key, a template other than Built-in becomes "Custom template" (id = first 12 hex of its +SHA-256), the default of both engines unless `default_instructions` is stored; with the +key, a template matching no entry (edited on an older build) is appended as "Custom +template (n)" with one INFO line (a full list: one WARNING, nothing imported). Nothing is +written at load; the next save that carries either key writes the list. +`AutotagConfig.default_instructions_text(engine)` feeds the legacy batch and single +blank-prompt fallbacks (by the request's engine) and `default_prompt`. Tests: +`tests/test_saved_instructions.py`, `tests/test_saved_instructions_routes.py`, +`tests/js/workspace-system-instructions.test.js`. **Skipped tags** (#431, D202): `autotag.defaults.skipped_tags`, host draft `autotag_skipped_tags`, a list of at most 200 entries, each trimmed, 1–50 code points, diff --git a/py/api/autotag_routes.py b/py/api/autotag_routes.py index bcd46f1d..ee676e87 100644 --- a/py/api/autotag_routes.py +++ b/py/api/autotag_routes.py @@ -78,6 +78,12 @@ MODEL_DOWNLOAD_BODY_MAX = 1024 _download_titles = {} +def _instruction_engine(model_type: str) -> str: + """The language-model engine whose default instructions ``model_type`` + reads: itself, or GGUF for a WD14 engine (which takes none).""" + return model_type if model_type in ("gguf", "hf") else "gguf" + + def _holder_label(holder: Optional[str]) -> str: if not holder: return ANOTHER_OPERATION @@ -294,7 +300,15 @@ class AutotagRoutesMixin: "success": True, "models": public_models(status, loaded_type), # The saved tagging defaults (#328, System › Auto-tagging). - "default_prompt": AutotagConfig.TEMPLATE, + # The saved engine's default instructions (GGUF for a + # WD14 engine); kept one release for cached pages (#431). + "default_prompt": AutotagConfig.default_instructions_text( + _instruction_engine(AutotagConfig.MODEL_TYPE) + ), + # #431: Built-in first, with every text, and each + # language-model engine's default. + "instructions": AutotagConfig.all_instructions(), + "default_instructions": dict(AutotagConfig.DEFAULT_INSTRUCTIONS), "model_loaded": loaded, "loaded_model_type": loaded_type, "wd14_general_threshold": AutotagConfig.GENERAL_THRESHOLD, @@ -549,7 +563,10 @@ class AutotagRoutesMixin: else: prompt = params.get("prompt") template = prompt if isinstance(prompt, str) and prompt.strip() else None - options = {"instructions": template or AutotagConfig.TEMPLATE} + options = { + "instructions": template + or AutotagConfig.default_instructions_text(model_type) + } try: _model, options = validate_options( model_type, @@ -1092,7 +1109,10 @@ class AutotagRoutesMixin: else: prompt = data.get("prompt") template = prompt if isinstance(prompt, str) and prompt.strip() else None - options = {"instructions": template or AutotagConfig.TEMPLATE} + options = { + "instructions": template + or AutotagConfig.default_instructions_text(model_type) + } use_gpu = data.get("use_gpu", True) options["use_gpu"] = use_gpu if isinstance(use_gpu, bool) else True return validate_options( diff --git a/py/api/settings_routes.py b/py/api/settings_routes.py index d3d9fb93..4cccf0ae 100644 --- a/py/api/settings_routes.py +++ b/py/api/settings_routes.py @@ -54,13 +54,24 @@ _SETTINGS_FILE_KEYS = { "AUTOTAG_EXISTING": ("autotag", "defaults", "existing"), "AUTOTAG_GENERAL_THRESHOLD": ("autotag", "defaults", "general_threshold"), "AUTOTAG_CHARACTER_THRESHOLD": ("autotag", "defaults", "character_threshold"), - "AUTOTAG_TEMPLATE": ("autotag", "defaults", "template"), "AUTOTAG_SUGGESTION_LIMIT": ("autotag", "defaults", "suggestion_limit"), "AUTOTAG_SKIPPED_TAGS": ("autotag", "defaults", "skipped_tags"), + # #431: the saved instructions and each engine's default, always written + # together (server-side pairing). + "AUTOTAG_INSTRUCTIONS": ("autotag", "defaults", "instructions"), + "AUTOTAG_DEFAULT_INSTRUCTIONS": ("autotag", "defaults", "default_instructions"), } +# The old single template: written only as the GGUF default's mirror for a +# downgrade (#431 B1.1), deleted when that default is Built-in. +_TEMPLATE_MIRROR_KEY = ("autotag", "defaults", "template") +TEMPLATE_REPLACED_ERROR = ( + "LLM tag template was replaced by Saved instructions. Reload the page." +) # Whole-list settings (#431): a save without ``expected_revision`` would merge # a stale copy over a list changed elsewhere, so it is refused. -_REVISION_REQUIRED = frozenset({"autotag_skipped_tags"}) +_REVISION_REQUIRED = frozenset( + {"autotag_skipped_tags", "autotag_instructions", "autotag_default_instructions"} +) REVISION_REQUIRED_ERROR = "Reload the page and try again." # The thumbnail settings a running Convert outdated job fixes (#317). _THUMBNAIL_SETTINGS = ("THUMBNAIL_FORMAT", "THUMBNAIL_QUALITY", "THUMBNAIL_EDGE") @@ -142,13 +153,20 @@ def _settings_mutation(updates, new_roots): """The config commit for a settings save: only the submitted keys change. Every other section and key (integrations and the CivitAI key, database, - unknown sections) is kept as it is on disk. + unknown sections) is kept as it is on disk. A save of the instructions + also writes the old ``template`` key as the GGUF default's mirror. """ def mutate(config): for name, value in updates.items(): *sections, key = _SETTINGS_FILE_KEYS[name] - _set_nested(config, sections, key, value) + _set_nested(config, sections, key, _jsonable(value)) + if "AUTOTAG_INSTRUCTIONS" in updates: + *sections, key = _TEMPLATE_MIRROR_KEY + mirror = _mirror_text( + updates["AUTOTAG_INSTRUCTIONS"], updates["AUTOTAG_DEFAULT_INSTRUCTIONS"] + ) + _set_nested(config, sections, key, _DELETE if mirror is None else mirror) if new_roots is not None: gallery = _section(config, "gallery") monitoring = _section(gallery, "monitoring") @@ -160,10 +178,37 @@ def _settings_mutation(updates, new_roots): return mutate +# ``_set_nested``'s value that removes the key instead of setting it. +_DELETE = object() + + +def _jsonable(value): + """A runtime value as config.json holds it (tuples become lists).""" + if isinstance(value, (list, tuple)): + return [_jsonable(item) for item in value] + if isinstance(value, dict): + return {key: _jsonable(item) for key, item in value.items()} + return value + + +def _mirror_text(entries, defaults): + """The GGUF default's text, or None when it is Built-in (#431 B1.1).""" + from ..tagging_defaults import BUILTIN_INSTRUCTIONS_ID + + for item in entries: + if item["id"] == defaults["gguf"] != BUILTIN_INSTRUCTIONS_ID: + return item["text"] + return None + + def _set_nested(config, sections, key, value): - """``config[s1][s2]...[key] = value``, each section a fresh dict.""" + """``config[s1][s2]...[key] = value``, each section a fresh dict; + ``_DELETE`` removes the key.""" if not sections: - config[key] = value + if value is _DELETE: + config.pop(key, None) + else: + config[key] = value return merged = _section(config, sections[0]) _set_nested(merged, sections[1:], key, value) @@ -182,7 +227,6 @@ def _validate_autotag_settings(data): MODEL_TYPES, validate_saved_threshold, validate_skipped_tags, - validate_template, ) updates = {} @@ -211,12 +255,7 @@ def _validate_autotag_settings(data): raise ValueError(f"{error}.") from None updates[key.upper()] = value if "autotag_template" in data: - try: - updates["AUTOTAG_TEMPLATE"] = validate_template( - data["autotag_template"], "LLM tag template" - ) - except ValueError as error: - raise ValueError(f"{error}.") from None + raise ValueError(TEMPLATE_REPLACED_ERROR) if "autotag_suggestion_limit" in data: value = data["autotag_suggestion_limit"] if type(value) is not int or not 1 <= value <= MAX_SUGGESTION_LIMIT: @@ -229,9 +268,37 @@ def _validate_autotag_settings(data): updates["AUTOTAG_SKIPPED_TAGS"] = tuple( validate_skipped_tags(data["autotag_skipped_tags"]) ) + updates.update(_validate_instruction_settings(data)) return updates +def _validate_instruction_settings(data): + """Server-side pairing (#431): a save that carries either instruction key + gets the other from the running configuration (which holds the migrated + list), the defaults are checked against the merged list, and both are + written. Empty when the save carries neither.""" + from ..config import AutotagConfig + from ..tagging_defaults import ( + validate_default_instructions, + validate_instructions, + ) + + if not {"autotag_instructions", "autotag_default_instructions"} & set(data): + return {} + if "autotag_instructions" in data: + entries = validate_instructions(data["autotag_instructions"]) + else: + entries = [dict(item) for item in AutotagConfig.INSTRUCTIONS] + defaults = data.get( + "autotag_default_instructions", dict(AutotagConfig.DEFAULT_INSTRUCTIONS) + ) + defaults = validate_default_instructions(defaults, entries) + return { + "AUTOTAG_INSTRUCTIONS": tuple(entries), + "AUTOTAG_DEFAULT_INSTRUCTIONS": defaults, + } + + async def _read_json(request): """Parse a JSON object body through the package's capped reader.""" from . import _read_json_body @@ -259,6 +326,7 @@ class SettingsRoutesMixin: """Get current settings.""" try: from ..config import AutotagConfig, PromptManagerConfig, GalleryConfig + from ..tagging_defaults import DEFAULT_TAG_TEMPLATE root_paths = [ self._public_path(d) for d in GalleryConfig.MONITORING_DIRECTORIES @@ -291,7 +359,19 @@ class SettingsRoutesMixin: "autotag_character_threshold": ( AutotagConfig.CHARACTER_THRESHOLD ), - "autotag_template": AutotagConfig.TEMPLATE, + # Read-only for one release (#431): a page cached + # from before the upgrade still renders. + "autotag_template": AutotagConfig.default_instructions_text( + "gguf" + ), + "autotag_instructions": [ + dict(item) for item in AutotagConfig.INSTRUCTIONS + ], + "autotag_default_instructions": dict( + AutotagConfig.DEFAULT_INSTRUCTIONS + ), + # Built-in's text, which System shows read-only. + "autotag_builtin_instructions": DEFAULT_TAG_TEMPLATE, "autotag_suggestion_limit": AutotagConfig.SUGGESTION_LIMIT, "autotag_skipped_tags": list(AutotagConfig.SKIPPED_TAGS), "cpu_count": PromptManagerConfig.max_worker_threads(), @@ -526,6 +606,12 @@ class SettingsRoutesMixin: if name in _AUTOTAG_SETTINGS: setattr(AutotagConfig, name[len(_AUTOTAG_PREFIX) :], value) continue + if "AUTOTAG_INSTRUCTIONS" in updates: + # What config.json now holds as the old key (the mirror). + AutotagConfig._LEGACY_TEMPLATE = AutotagConfig.mirror_template() + for name, value in updates.items(): + if name in _AUTOTAG_SETTINGS: + continue owner = GalleryConfig if name in _GALLERY_SETTINGS else PromptManagerConfig setattr(owner, name, value) restart_fields = [] diff --git a/py/config.py b/py/config.py index ab7c1071..4af38a5a 100644 --- a/py/config.py +++ b/py/config.py @@ -30,6 +30,7 @@ routes = server_instance.routes # Extension info extension_uri = None # Will be set in __init__.py +import hashlib # noqa: E402 import os # noqa: E402 from typing import Any, Dict, List, Optional, Tuple # noqa: E402 @@ -590,6 +591,12 @@ try: validate_skipped_tags, validate_template, validate_threshold, + BUILTIN_INSTRUCTIONS_ID, + INSTRUCTION_ENGINES, + MAX_SAVED_INSTRUCTIONS, + builtin_instructions, + validate_default_instructions, + validate_instructions, ) except ImportError: from py.tagging_defaults import ( # noqa: F401 - re-exported @@ -604,6 +611,12 @@ except ImportError: validate_skipped_tags, validate_template, validate_threshold, + BUILTIN_INSTRUCTIONS_ID, + INSTRUCTION_ENGINES, + MAX_SAVED_INSTRUCTIONS, + builtin_instructions, + validate_default_instructions, + validate_instructions, ) @@ -623,13 +636,24 @@ def _valid_skipped_tags(value: Any) -> Tuple[str, ...]: return tuple(validate_skipped_tags(value)) +def _reason(error: ValueError) -> str: + """A settings sentence as a load warning's reason ("entry 2 is empty").""" + text = str(error).rstrip(".") + for prefix in ("Skipped tags", "Saved instructions"): + if text.startswith(prefix): + return text.removeprefix(prefix).lstrip(": ") + return text + + +def _text_id(text: str) -> str: + """A migrated entry's id: deterministic, so every load gives the same.""" + return hashlib.sha256(text.encode("utf-8")).hexdigest()[:12] + + def _fallback_warning(key: str, error: ValueError, default: Any) -> str: """The one load warning of a saved tagging default that fails its check.""" - if key == "template": - return f"autotag.defaults.template {error}; using the built-in one" if key == "skipped_tags": - reason = str(error).removeprefix("Skipped tags").lstrip(": ").rstrip(".") - return f"autotag.defaults.skipped_tags {reason}; using no skipped tags" + return f"autotag.defaults.skipped_tags {_reason(error)}; using no skipped tags" return f"autotag.defaults.{key} {error}; using {default!r}" @@ -653,9 +677,18 @@ class AutotagConfig: EXISTING = "skip" GENERAL_THRESHOLD = 0.35 CHARACTER_THRESHOLD = 0.85 - TEMPLATE = DEFAULT_TAG_TEMPLATE SUGGESTION_LIMIT = DEFAULT_SUGGESTION_LIMIT SKIPPED_TAGS: Tuple[str, ...] = () + # #431: the saved instructions (Built-in is never stored) and the default + # of each language-model engine, by id. + INSTRUCTIONS: Tuple[Dict[str, str], ...] = () + DEFAULT_INSTRUCTIONS: Dict[str, str] = { + "gguf": BUILTIN_INSTRUCTIONS_ID, + "hf": BUILTIN_INSTRUCTIONS_ID, + } + # The old single template as read at load (None when absent or invalid); + # only the migration reads it. Saves keep it as the GGUF default's mirror. + _LEGACY_TEMPLATE: Optional[str] = None # config.json key -> (attribute, validator, default) _KEYS = { @@ -671,7 +704,6 @@ class AutotagConfig: lambda v: validate_saved_threshold("value", v), 0.85, ), - "template": ("TEMPLATE", validate_template, DEFAULT_TAG_TEMPLATE), "suggestion_limit": ( "SUGGESTION_LIMIT", _valid_suggestion_limit, @@ -691,18 +723,142 @@ class AutotagConfig: type(value) is int and 1 <= value <= MAX_SUGGESTION_LIMIT ) # bool, float and str are rejected + @classmethod + def all_instructions(cls) -> List[Dict[str, str]]: + """Built-in first, then the saved entries (copies).""" + return [builtin_instructions()] + [dict(item) for item in cls.INSTRUCTIONS] + + @classmethod + def default_instructions_text(cls, engine: str) -> str: + """The text of ``engine``'s (``gguf`` or ``hf``) default instructions.""" + if engine not in INSTRUCTION_ENGINES: + raise ValueError(f"{engine} takes no instructions") + wanted = cls.DEFAULT_INSTRUCTIONS.get(engine, BUILTIN_INSTRUCTIONS_ID) + for item in cls.INSTRUCTIONS: + if item["id"] == wanted: + return item["text"] + return DEFAULT_TAG_TEMPLATE + + @classmethod + def mirror_template(cls) -> Optional[str]: + """What an older build reads as its one template (#431 B1.1): the + GGUF default's text, or None (no key) when that default is Built-in.""" + if cls.DEFAULT_INSTRUCTIONS.get("gguf") == BUILTIN_INSTRUCTIONS_ID: + return None + return cls.default_instructions_text("gguf") + @classmethod def get_config(cls) -> Dict[str, Any]: - """The saved defaults; the template only when it is not the built-in - one, so a later built-in template still reaches who never set one.""" + """The saved defaults; ``template`` only as the GGUF default's mirror.""" defaults = { key: getattr(cls, attr) for key, (attr, _v, _d) in cls._KEYS.items() } - if defaults["template"] == DEFAULT_TAG_TEMPLATE: - del defaults["template"] defaults["skipped_tags"] = list(defaults["skipped_tags"]) + defaults["instructions"] = [dict(item) for item in cls.INSTRUCTIONS] + defaults["default_instructions"] = dict(cls.DEFAULT_INSTRUCTIONS) + mirror = cls.mirror_template() + if mirror is not None: + defaults["template"] = mirror return {"defaults": defaults} + @classmethod + def _load_instructions(cls, defaults: Dict[str, Any]) -> None: + """Each instruction key alone, then the cross-key default check, then + the in-memory migration of the old template (#431). Nothing is + written here, so the revision never changes at load.""" + cls._LEGACY_TEMPLATE = None + if "template" in defaults: + try: + cls._LEGACY_TEMPLATE = validate_template(defaults["template"]) + except ValueError as error: + config_logger.warning( + f"autotag.defaults.template {error}; using the built-in one" + ) + entries: List[Dict[str, str]] = [] + damaged = False + if "instructions" in defaults: + try: + entries = validate_instructions(defaults["instructions"]) + except ValueError as error: + damaged = True + config_logger.warning( + f"autotag.defaults.instructions {_reason(error)}; " + "using Built-in only" + ) + chosen = {engine: BUILTIN_INSTRUCTIONS_ID for engine in INSTRUCTION_ENGINES} + stored = defaults.get("default_instructions") + if damaged: + # Both defaults fall back with the list (its one warning says so); + # the old template is migrated as on a first upgrade. + pass + elif "default_instructions" in defaults and ( + not isinstance(stored, dict) or set(stored) != set(INSTRUCTION_ENGINES) + ): + config_logger.warning( + "autotag.defaults.default_instructions must name gguf and hf; " + "using Built-in" + ) + elif isinstance(stored, dict): + ids = {item["id"] for item in entries} + for engine in INSTRUCTION_ENGINES: + value = stored[engine] + if isinstance(value, str) and ( + value == BUILTIN_INSTRUCTIONS_ID or value in ids + ): + chosen[engine] = value + else: + config_logger.warning( + f"autotag.defaults.default_instructions.{engine} names no " + "saved instructions; using Built-in" + ) + cls._migrate_template( + entries, + chosen, + first=damaged or "instructions" not in defaults, + keep_defaults=not damaged and "default_instructions" in defaults, + ) + cls.INSTRUCTIONS = tuple(entries) + cls.DEFAULT_INSTRUCTIONS = chosen + + @classmethod + def _migrate_template(cls, entries, chosen, *, first, keep_defaults) -> None: + """Bring a template saved by an older build in as a named entry.""" + legacy = cls._LEGACY_TEMPLATE + if legacy is None or legacy == DEFAULT_TAG_TEMPLATE: + return + if first: + # The first upgrade: it becomes "Custom template", the default for + # both engines unless defaults are stored already (20A). + entries.append( + {"id": _text_id(legacy), "name": "Custom template", "text": legacy} + ) + if not keep_defaults: + for engine in INSTRUCTION_ENGINES: + chosen[engine] = entries[-1]["id"] + return + if any(item["text"] == legacy for item in entries): + return # the mirror of a saved default + # Edited on an older build after an upgrade (B1.2): import it. + if len(entries) >= MAX_SAVED_INSTRUCTIONS: + config_logger.warning( + "The tagging template edited on an older version was not imported: " + f"the saved instructions are full ({MAX_SAVED_INSTRUCTIONS})" + ) + return + names = {item["name"].casefold() for item in entries} + number = 2 + while f"custom template ({number})" in names: + number += 1 + name = f"Custom template ({number})" + ids = {item["id"] for item in entries} + new_id, n = _text_id(legacy), 2 + while new_id in ids: + new_id, n = _text_id(f"{legacy}\n{n}"), n + 1 + entries.append({"id": new_id, "name": name, "text": legacy}) + config_logger.info( + "Imported the tagging template edited on an older version as " f"'{name}'" + ) + @classmethod def update_config(cls, new_config: Any) -> None: if not isinstance(new_config, dict): @@ -720,6 +876,7 @@ class AutotagConfig: except ValueError as error: config_logger.warning(_fallback_warning(key, error, default)) setattr(cls, attr, default) + cls._load_instructions(defaults) class PromptManagerConfig: diff --git a/py/tagging_defaults.py b/py/tagging_defaults.py index b1181372..9d00cd96 100644 --- a/py/tagging_defaults.py +++ b/py/tagging_defaults.py @@ -226,3 +226,97 @@ def wd14_label_names(path) -> Tuple[str, ...]: with _label_cache_lock: _label_cache[key] = (stamp, names) return names + + +# Saved instructions (#431): named language-model instructions with a default +# per engine. Built-in is never stored; readers put it first. +BUILTIN_INSTRUCTIONS_ID = "builtin" +BUILTIN_INSTRUCTIONS_NAME = "Built-in" +MAX_SAVED_INSTRUCTIONS = 20 +MAX_INSTRUCTION_NAME_CHARS = 60 +INSTRUCTION_ENGINES = ("gguf", "hf") +ENGINE_LABELS = {"gguf": "GGUF JoyCaption", "hf": "HuggingFace JoyCaption"} +_INSTRUCTION_ID = re.compile(r"[0-9a-f]{12}") +_CONTROL = re.compile(r"[\x00-\x1f\x7f-\x9f

]") + + +def builtin_instructions() -> Dict[str, str]: + """The read-only Built-in entry, always the shipped template.""" + return { + "id": BUILTIN_INSTRUCTIONS_ID, + "name": BUILTIN_INSTRUCTIONS_NAME, + "text": DEFAULT_TAG_TEMPLATE, + } + + +def _instruction_entry(raw: Any, number: int) -> Dict[str, str]: + prefix = f"Saved instructions: entry {number}" + if ( + not isinstance(raw, dict) + or set(raw) != {"id", "name", "text"} + or not isinstance(raw["id"], str) + or _INSTRUCTION_ID.fullmatch(raw["id"]) is None + ): + raise ValueError(f"{prefix} has an invalid id.") + name = raw["name"].strip() if isinstance(raw["name"], str) else "" + if not name: + raise ValueError(f"{prefix} needs a name.") + if len(name) > MAX_INSTRUCTION_NAME_CHARS: + raise ValueError( + f"{prefix} name must be at most {MAX_INSTRUCTION_NAME_CHARS} characters." + ) + if _CONTROL.search(name): + raise ValueError(f"{prefix} name must not contain a line break.") + if name.casefold() == BUILTIN_INSTRUCTIONS_NAME.casefold(): + raise ValueError("Saved instructions: the name “Built-in” is reserved.") + try: + text = validate_template(raw["text"], f"{prefix} text") + except ValueError as error: + raise ValueError(f"{error}.") from None + return {"id": raw["id"], "name": name, "text": text} + + +def validate_instructions(value: Any) -> List[Dict[str, str]]: + """The saved entries ``[{id, name, text}]`` in order (names trimmed), or + ValueError with the sentence System shows (Built-in is not counted).""" + if not isinstance(value, list): + raise ValueError("Saved instructions must be a list.") + if len(value) > MAX_SAVED_INSTRUCTIONS: + raise ValueError( + f"Saved instructions: at most {MAX_SAVED_INSTRUCTIONS} entries besides " + "Built-in." + ) + entries: List[Dict[str, str]] = [] + ids: Dict[str, int] = {} + names: Dict[str, int] = {} + for number, raw in enumerate(value, 1): + item = _instruction_entry(raw, number) + if item["id"] in ids: + raise ValueError( + f"Saved instructions: entry {number} repeats the id of entry " + f"{ids[item['id']]}." + ) + key = item["name"].casefold() + if key in names: + raise ValueError( + f"Saved instructions: entry {number} name “{item['name']}” is " + "already used." + ) + ids[item["id"]] = number + names[key] = number + entries.append(item) + return entries + + +def validate_default_instructions(value: Any, entries) -> Dict[str, str]: + """``{gguf: id, hf: id}``, each Built-in or an id of ``entries``.""" + if not isinstance(value, dict) or set(value) != set(INSTRUCTION_ENGINES): + raise ValueError("Default instructions must name GGUF and HuggingFace.") + ids = {BUILTIN_INSTRUCTIONS_ID} | {item["id"] for item in entries} + for engine in INSTRUCTION_ENGINES: + if not isinstance(value[engine], str) or value[engine] not in ids: + raise ValueError( + f"Default for {ENGINE_LABELS[engine]} must be one of the saved " + "instructions." + ) + return {engine: value[engine] for engine in INSTRUCTION_ENGINES} diff --git a/tests/browser/system-tagging-concept.spec.mjs b/tests/browser/system-tagging-concept.spec.mjs index 865d8b14..9162e8bd 100644 --- a/tests/browser/system-tagging-concept.spec.mjs +++ b/tests/browser/system-tagging-concept.spec.mjs @@ -35,6 +35,8 @@ const MODEL_OPTIONS = ['WD14 SwinV2 · ~400 MB', 'WD14 ViT · ~400 MB', 'GGUF Jo const SUGGESTIONS = 'From 1 to 100: the most suggestions Generate shows for one image.'; const SKIPPED_DESCRIPTION = 'Suggested tags that contain one of these words are hidden. Words are separated by spaces, _ or -; ' + 'other characters such as ( ) belong to the word.'; +const DEFAULT_DESCRIPTION = 'The Tag image dialog and batches start from this.'; +const INSTRUCTIONS_DESCRIPTION = 'Instructions for the language-model engines. Built-in can’t be changed; duplicate it to make your own.'; const BATCH = 'Tag each output image from its own pixels with the model above. Prompts are not changed. Review the scope first.'; /** Runs in the page: Auto-tagging's surface under `mainSelector`: rows, field, actions, operation, geometry and tokens. */ @@ -64,7 +66,7 @@ function measureTagging(mainSelector) { text: [...row.firstElementChild.children].map(node => [name(node), text(node), node.localName === 'label' ? node.htmlFor : null]), control: control(row), flexWrap: getComputedStyle(row).flexWrap, controlBelow: row.lastElementChild.getBoundingClientRect().top >= row.firstElementChild.getBoundingClientRect().bottom - 0.5 })), - field: { tree: [...field.children].map(name), label: [text(field.querySelector('label')), field.querySelector('label').htmlFor], + field: field && { tree: [...field.children].map(name), label: [text(field.querySelector('label')), field.querySelector('label').htmlFor], textarea: { id: field.querySelector('textarea').id, value: field.querySelector('textarea').value, ...css(field.querySelector('textarea'), 'fontSize', 'minHeight', 'backgroundColor', 'color', 'borderTopColor') }, ...css(field, 'display', 'rowGap', 'fontSize', 'marginTop', 'marginBottom') }, @@ -81,7 +83,22 @@ function measureTagging(mainSelector) { label: css(block.querySelector('label'), 'fontSize', 'fontWeight', 'color'), description: css(block.querySelector('.setting-block-text p'), 'fontSize', 'color', 'marginTop'), input: css(block.querySelector('input'), 'fontSize', 'backgroundColor', 'color', 'borderTopColor') })( - surface.querySelector(':scope > .setting-block')), + surface.querySelector(':scope > .setting-block[data-setting="tag-skipped"]')), + // D203 (#431): Saved instructions (the concept has a template field instead). + instructions: (block => block && { tree: [...block.children].map(name), key: block.dataset.key, + text: [...block.firstElementChild.children].map(node => [node.localName, text(node)]), + list: [...block.querySelectorAll('[role="option"]')].map(option => [text(option.firstElementChild), + [...option.querySelectorAll('.badges > .chip')].map(chip => [text(chip), chip.className]), option.getAttribute('aria-selected')]), + listRole: block.querySelector('.instruction-list').getAttribute('role'), + actions: [...block.querySelectorAll('.instruction-actions > button')].map(item => [text(item), item.className, item.disabled]), + editor: [...block.querySelectorAll('.instruction-editor .field')].map(item => [text(item.querySelector('label > span')), + item.querySelector('input, textarea').value, item.querySelector('input, textarea').readOnly]), + note: text(block.querySelector('#instruction-note')), + layout: getComputedStyle(block.querySelector('.instructions-layout')).gridTemplateColumns.split(' ').length, + ...css(block, 'paddingTop', 'paddingBottom', 'borderTopWidth', 'borderTopColor'), + label: css(block.querySelector('label'), 'fontSize', 'fontWeight', 'color'), + textarea: css(block.querySelector('textarea'), 'fontSize', 'backgroundColor', 'borderTopColor') })( + surface.querySelector(':scope > .setting-block[data-setting="tag-instructions"]')), operations: [...surface.querySelectorAll(':scope > .operation')].map(row => ({ tree: [...row.children].map(name), title: text(row.querySelector('h3')), description: text(row.querySelector('p')), actions: [...row.querySelectorAll('.row-actions > button')].map(item => [text(item), item.className]) })), @@ -115,14 +132,21 @@ function measureTagging(mainSelector) { * The concept's Auto-tagging with the recorded deviations applied, for the fixture (all engines on disk, none loaded). * `phone` is a viewport at the concept's 700px breakpoint or below. */ -function mapTaggingConcept(reference, { template, phone }) { +function mapTaggingConcept(reference, { builtin, saved, phone }) { const [model, existing, general, character] = reference.rows; const [preview] = reference.actions.buttons; + // D203: Built-in first, then the saved entries, in each default select. + const names = ['Built-in', ...saved.map(entry => entry.name)]; + const defaultRow = (engine, long) => ({ ...existing, + text: [['label', `Default for ${long}`, `instruction-default-${engine}`], ['p', DEFAULT_DESCRIPTION, null]], + control: { ...existing.control, id: `instruction-default-${engine}`, options: names.map(label => [label, false]), + selected: names[1] } }); return { ...reference, - // D133 adds a row after the thresholds, D202 the Skipped tags block after it; D134 adds the operation after the - // actions. - tree: [...reference.tree.slice(0, 6), 'div.setting-row', 'div.setting-block', ...reference.tree.slice(6), 'div.operation'], + // D133 adds a row after the thresholds, D202 the Skipped tags block after it; D203 replaces the template field + // with Saved instructions, the two default rows and their note; D134 adds the operation after the actions. + tree: [...reference.tree.slice(0, 6), 'div.setting-row', 'div.setting-block', 'div.setting-block', 'div.setting-row', + 'div.setting-row', 'p.instruction-reset.small', reference.tree[7], 'div.operation'], rows: [ // D132 (OQ5, OQ6): the four real engines with their download size, and the chosen one's live status. { ...model, text: [model.text[0], ['p', 'Downloaded. Not loaded.', null]], @@ -133,7 +157,26 @@ function mapTaggingConcept(reference, { template, phone }) { // D133: the persisted suggestion count, on the concept's number row. { ...general, text: [['label', 'Suggestions per image', 'tag-suggestions'], ['p', SUGGESTIONS, null]], control: { ...general.control, id: 'tag-suggestions', value: '10' } }, + // D203: the default of each language-model engine (the fixture's saved entry is both), on the concept's + // select row. + defaultRow('gguf', 'GGUF JoyCaption'), defaultRow('hf', 'HuggingFace JoyCaption'), ], + // D203: Saved instructions in the concept's setting-row tokens (as D202), Built-in selected and read-only. + instructions: { tree: ['div.setting-block-text', 'div.instructions-layout'], key: 'autotag_instructions', + text: [['label', 'Saved instructions'], ['p', `${INSTRUCTIONS_DESCRIPTION} ${saved.length} of 20 saved · Built\u2011in is not counted.`]], + list: [['Built-in', [['Read-only', 'chip']], 'true'], + ...saved.map((entry, index) => [entry.name, index === 0 + ? [['Default for GGUF', 'chip blue'], ['Default for HuggingFace', 'chip blue']] : [], 'false'])], + listRole: 'listbox', + actions: [['+ New', '', false], ['Duplicate', '', false], ['Delete', 'danger', true]], + editor: [['Name', 'Built-in', true], ['Text', builtin, true]], + note: 'Built-in instructions can’t be changed. Duplicate them to make your own.', + layout: phone ? 1 : 2, + paddingTop: reference.row.paddingTop, paddingBottom: reference.row.paddingBottom, + borderTopWidth: reference.row.borderTopWidth, borderTopColor: reference.row.borderTopColor, + label: reference.label, + textarea: { fontSize: reference.field.textarea.fontSize, backgroundColor: reference.field.textarea.backgroundColor, + borderTopColor: reference.field.textarea.borderTopColor } }, // D202 (#431): the Skipped tags block in the concept's setting-row tokens: its label and description as a row's, // the row's padding and rule above, the concept's field input; an empty list says so. skipped: { tree: ['div.setting-block-text', 'div.skip-add', 'p.skip-problem.small', 'div.skip-chips.tags', 'p.skip-preview.small'], @@ -148,15 +191,15 @@ function mapTaggingConcept(reference, { template, phone }) { marginTop: reference.description.marginTop }, input: { fontSize: '12px', backgroundColor: reference.field.textarea.backgroundColor, color: reference.field.textarea.color, borderTopColor: reference.field.textarea.borderTopColor } }, - // The saved template (the server's default until one is saved) replaces the concept's sample text. - field: { ...reference.field, textarea: { ...reference.field.textarea, value: template } }, + // D203: no template field. + field: null, // D132: Download (for an engine already on disk, "Download again") and Unload replace the two simulated jobs. actions: { ...reference.actions, buttons: [preview, ['Download again', ''], ['Unload', '']] }, // D134: the prompt-tag batch, reviewed first. operations: [{ tree: ['div', 'div.row-actions'], title: 'Tag output images', description: BATCH, actions: [['Review', '']] }], // D132 (#398): every select shows its whole chosen option at every width (at 390 the concept's 120px cap cuts // them; production lets each grow past it). - selectsFit: reference.selectsFit.map(() => true), + selectsFit: [...reference.selectsFit, true, true].map(() => true), }; } @@ -180,7 +223,9 @@ async function productionTagging(page, theme) { async function savedTemplate(request, page) { const response = await request.get('/prompt_manager/settings'); await expect(response).toBeOK(); - return { template: (await response.json()).settings.autotag_template, phone: page.viewportSize().width <= 700 }; + const { settings } = await response.json(); + return { builtin: settings.autotag_builtin_instructions, saved: settings.autotag_instructions, + phone: page.viewportSize().width <= 700 }; } for (const theme of THEMES) for (const [width, height] of SIZES) { @@ -204,14 +249,15 @@ for (const theme of THEMES) for (const width of [390, 320]) { await page.locator('#workspace-main #tag-existing').selectOption('replace_after_review'); await page.locator('#workspace-main section.surface').screenshot({ path: testInfo.outputPath(`production-tagging-selects-${theme}-${width}.png`) }); const selects = page.locator('#workspace-main section.surface > .setting-row > select'); - await expect(selects).toHaveCount(2); - for (let index = 0; index < 2; index += 1) { + // D203 adds the two per-engine default selects. + await expect(selects).toHaveCount(4); + for (let index = 0; index < 4; index += 1) { const select = selects.nth(index); const values = await select.evaluate(node => [...node.options].map(option => option.value)); for (const value of values) { await select.selectOption(value); const measured = await page.evaluate(measureTagging, '#workspace-main'); - expect(measured.selectsFit, `${await select.getAttribute('id')}=${value}`).toEqual([true, true]); + expect(measured.selectsFit, `${await select.getAttribute('id')}=${value}`).toEqual([true, true, true, true]); expect(measured.pageScroll, value).toBe(false); } } @@ -225,15 +271,16 @@ test('auto-tagging comparison detects mutations', async ({ page, request }) => { const expected = mapTaggingConcept(concept, saved); const production = await productionTagging(page, 'dark'); expect(diffInventory(expected, production)).toEqual([]); - // Unmapped, the section differs only where D132, D133, D134 and D202 are recorded (and the saved template's text). + // Unmapped, the section differs only where D132, D133, D134, D202 and D203 are recorded (and the saved template's text). // (The concept has no Skipped tags block, so every path under it is one D202 difference.) - expect(diffInventory(concept, production).map(item => (item.path.startsWith('skipped') ? 'skipped' + expect(diffInventory(concept, production).map(item => (/^(skipped|instructions|field)\b/.test(item.path) ? item.path.split('.')[0] : item.path.split('.').slice(0, 3).join('.'))) .filter((path, index, all) => all.indexOf(path) === index).sort()).toEqual([ - 'actions.buttons.1', 'actions.buttons.2', 'field.textarea.value', 'operations.0.actions', 'operations.0.description', - 'operations.0.title', 'operations.0.tree', 'operations.length', 'rows.0.control', 'rows.0.text', 'rows.4.control', 'rows.4.controlBelow', - 'rows.4.flexWrap', 'rows.4.text', 'rows.4.tree', 'rows.length', 'skipped', 'tree.6', 'tree.7', 'tree.8', 'tree.9', 'tree.10', - 'tree.length'].sort()); + 'actions.buttons.1', 'actions.buttons.2', 'field', 'instructions', 'operations.0.actions', 'operations.0.description', + 'operations.0.title', 'operations.0.tree', 'operations.length', 'rows.0.control', 'rows.0.text', + ...[4, 5, 6].flatMap(row => ['control', 'controlBelow', 'flexWrap', 'text', 'tree'].map(key => `rows.${row}.${key}`)), + 'rows.length', 'selectsFit.2', 'selectsFit.3', 'selectsFit.length', 'skipped', + ...Array.from({ length: 8 }, (_, index) => `tree.${index + 6}`), 'tree.length'].sort()); const measure = () => page.evaluate(measureTagging, '#workspace-main'); const paths = async () => diffInventory(expected, await measure()).map(item => item.path); // A removed row. @@ -284,8 +331,8 @@ test('auto-tagging at 390 wraps only the engine row', async ({ page, request }) const concept = await conceptTagging(page, 'dark'); const production = await productionTagging(page, 'dark'); expect(diffInventory(mapTaggingConcept(concept, await savedTemplate(request, page)), production)).toEqual([]); - expect(production.rows.map(row => row.flexWrap)).toEqual(['wrap', 'nowrap', 'nowrap', 'nowrap', 'nowrap']); - expect(production.selectsFit).toEqual([true, true]); + expect(production.rows.map(row => row.flexWrap)).toEqual(['wrap', 'nowrap', 'nowrap', 'nowrap', 'nowrap', 'nowrap', 'nowrap']); + expect(production.selectsFit).toEqual([true, true, true, true]); }); /** Runs in the page: the open review dialog: its title, body and the concept's tokens. */ diff --git a/tests/browser/workspace-system-tagging.spec.mjs b/tests/browser/workspace-system-tagging.spec.mjs index eff83024..5205c222 100644 --- a/tests/browser/workspace-system-tagging.spec.mjs +++ b/tests/browser/workspace-system-tagging.spec.mjs @@ -112,13 +112,14 @@ test('model options are the four real engines with size and the description show await expect(status(page)).toHaveText('Unsaved changes.'); // The other rows keep the concept's copy and the saved values (D133 adds Suggestions per image). expect(await surface(page).locator(':scope > .setting-row label').allTextContents()).toEqual(['Tagging model', 'Existing tags', - 'General-tag confidence', 'Character-tag confidence', 'Suggestions per image']); + 'General-tag confidence', 'Character-tag confidence', 'Suggestions per image', 'Default for GGUF JoyCaption', + 'Default for HuggingFace JoyCaption']); await expect(page.locator('#tag-existing')).toHaveValue('skip'); await expect(page.locator('#threshold-general')).toHaveValue('0.35'); await expect(page.locator('#threshold-character')).toHaveValue('0.85'); await expect(page.locator('#tag-suggestions')).toHaveValue('10'); - const settings = await serverJson(request, SETTINGS_URL); - await expect(page.locator('#tag-template')).toHaveValue(settings.settings.autotag_template); + // D203 (#431): Saved instructions replace the template field. + await expect(page.locator('#tag-template')).toHaveCount(0); // Unload is offered only while a model is loaded (none is). await expect(actionOf(page, 'Unload')).toBeDisabled(); }); @@ -500,3 +501,74 @@ test('skipped tags preview checks library tags only without a label list, and a await expect(skippedBlock(page)).toHaveAttribute('aria-invalid', 'true'); await page.unrouteAll({ behavior: 'ignoreErrors' }); }); + +// D203 (#431): Saved instructions: Built-in read-only, New, edit, per-engine defaults, Delete resets a default, Save +// preferences and a reload; the server pairs the two keys. D205: the batch review names the engine's default. +const instructionOption = (page, name) => page.locator('#instruction-list [role="option"]').filter({ has: page.locator(`span:text-is("${name}")`) }); + +test('saved instructions: new, edit, per-engine default, delete, save and reload (D203)', async ({ page, request }) => { + await openTagging(page); + const settings = (await serverJson(request, SETTINGS_URL)).settings; + await expect(page.locator('#instructions-count')).toHaveText('1 of 20 saved · Built\u2011in is not counted.'); + await expect(page.locator('#instruction-list [role="option"] > span:first-child')).toHaveText(['Built-in', 'Describe the image']); + await expect(page.locator('#instruction-name')).toHaveValue('Built-in'); + await expect(page.locator('#instruction-text')).toHaveValue(settings.autotag_builtin_instructions); + await expect(page.locator('#instruction-text')).toHaveAttribute('readonly', ''); + await expect(page.locator('#instruction-delete')).toBeDisabled(); + await expect(page.locator('#instruction-default-gguf')).toHaveValue(settings.autotag_instructions[0].id); + // + New, then name and text it, and make it HuggingFace's default. + await page.locator('#instruction-new').click(); + await expect(page.locator('#instruction-name')).toHaveValue('New instructions'); + await expect(page.locator('#instruction-note')).toHaveText('The text must not be empty.'); + await page.locator('#instruction-name').fill('Photo realism'); + await page.locator('#instruction-text').fill('Tag camera angle and lighting.'); + await expect(page.locator('#instruction-text').locator('xpath=..').locator('label .muted')).toHaveText('30 of 4,000 characters'); + await page.locator('#instruction-default-hf').selectOption({ label: 'Photo realism' }); + await expect(instructionOption(page, 'Photo realism').locator('.chip')).toHaveText(['Default for HuggingFace']); + await expect(status(page)).toHaveText('Unsaved changes.'); + await save(page).click(); + await expect(status(page)).toHaveText('Saved to the server configuration.'); + let saved = (await serverJson(request, SETTINGS_URL)).settings; + expect(saved.autotag_instructions.map(entry => [entry.name, entry.text])).toEqual([['Describe the image', 'Describe the image'], + ['Photo realism', 'Tag camera angle and lighting.']]); + expect(saved.autotag_default_instructions.hf).toBe(saved.autotag_instructions[1].id); + // A reload shows the saved list; deleting GGUF's default resets it to Built-in and says so. + await openTagging(page); + await instructionOption(page, 'Describe the image').click(); + await page.locator('#instruction-delete').click(); + await expect(page.locator('#instruction-reset')).toHaveText('GGUF now uses Built-in.'); + await expect(page.locator('#instruction-default-gguf')).toHaveValue('builtin'); + await save(page).click(); + await expect(status(page)).toHaveText('Saved to the server configuration.'); + saved = (await serverJson(request, SETTINGS_URL)).settings; + expect(saved.autotag_instructions.map(entry => entry.name)).toEqual(['Photo realism']); + expect(saved.autotag_default_instructions.gguf).toBe('builtin'); + expect(saved.autotag_template).toBe(saved.autotag_builtin_instructions); +}); + +test('a refused instructions save names the entry and highlights the block (D203)', async ({ page }) => { + await openTagging(page); + await page.locator('#instruction-duplicate').click(); + await page.locator('#instruction-name').fill('describe the IMAGE'); + await expect(page.locator('#instruction-note')).toHaveText('The name “describe the IMAGE” is already used.'); + await save(page).click(); + await expect(status(page)).toHaveText('Saved instructions: entry 2 name “describe the IMAGE” is already used.'); + await expect(surface(page).locator('.setting-block[data-setting="tag-instructions"]')).toHaveAttribute('aria-invalid', 'true'); +}); + +test('the batch review names the language model\'s default instructions (D205)', async ({ page }) => { + await openTagging(page); + await page.locator('#tag-model').selectOption('gguf'); + await page.locator('#tag-existing').selectOption('merge'); + const answer = page.waitForResponse(item => item.url().endsWith(PREVIEW_URL)); + await reviewButton(page).click(); + await answer; + await expect(dialog(page).locator('#modal-title')).toHaveText('Review tag output images'); + const grid = await gridOf(page); + expect(grid.slice(-3)).toEqual([['Instructions', 'Describe the image'], ['Settings', 'Up to 10 tags per image'], + grid.at(-1)]); + // The batch sends the default's text. + const start = page.waitForRequest(item => item.url().endsWith('/prompt_manager/jobs/autotag')); + await dialog(page).getByRole('button', { name: 'Start tagging', exact: true }).click(); + expect((await start).postDataJSON().options).toEqual({ instructions: 'Describe the image' }); +}); diff --git a/tests/browser/workspace_tagging_fixture.py b/tests/browser/workspace_tagging_fixture.py index 26afa4eb..21acd432 100644 --- a/tests/browser/workspace_tagging_fixture.py +++ b/tests/browser/workspace_tagging_fixture.py @@ -391,10 +391,15 @@ DOWNLOAD_HOLD_LIMIT_S = 60 DEFAULTS = (0.35, 0.85, 10) +# The saved "Describe the image" entry's id (#431). +FIXTURE_INSTRUCTIONS_ID = "d35c71be0a1e" + + def _set_defaults(model, values) -> None: """Set the saved tagging defaults (``AutotagConfig``, #328); a reset also - puts back the saved model, batch policy and template (the fake model's - short instructions, which the concept comparisons show).""" + puts back the saved model, batch policy, skipped tags and instructions + (the fake model's short instructions, which the concept comparisons + show).""" from py.config import AutotagConfig general, character, limit = values @@ -404,7 +409,21 @@ def _set_defaults(model, values) -> None: if values == DEFAULTS: AutotagConfig.MODEL_TYPE = "wd14-swinv2" AutotagConfig.EXISTING = "skip" - AutotagConfig.TEMPLATE = model.default_prompt + # #431: Built-in is always the shipped text, so the fake model's short + # instructions are a saved entry, the default of both engines (the + # concept comparisons show it). + AutotagConfig.INSTRUCTIONS = ( + { + "id": FIXTURE_INSTRUCTIONS_ID, + "name": model.default_prompt, + "text": model.default_prompt, + }, + ) + AutotagConfig.DEFAULT_INSTRUCTIONS = { + "gguf": FIXTURE_INSTRUCTIONS_ID, + "hf": FIXTURE_INSTRUCTIONS_ID, + } + AutotagConfig._LEGACY_TEMPLATE = None AutotagConfig.SKIPPED_TAGS = () diff --git a/tests/fixtures/legacy_contract_snapshot.json b/tests/fixtures/legacy_contract_snapshot.json index 4fa2a827..93218b1d 100644 --- a/tests/fixtures/legacy_contract_snapshot.json +++ b/tests/fixtures/legacy_contract_snapshot.json @@ -185,9 +185,12 @@ "monitor.scheduled", "revision", "settings", + "settings.autotag_builtin_instructions", "settings.autotag_character_threshold", + "settings.autotag_default_instructions", "settings.autotag_existing", "settings.autotag_general_threshold", + "settings.autotag_instructions", "settings.autotag_model_type", "settings.autotag_skipped_tags", "settings.autotag_suggestion_limit", diff --git a/tests/js/workspace-system-instructions.test.js b/tests/js/workspace-system-instructions.test.js new file mode 100644 index 00000000..0a1a7814 --- /dev/null +++ b/tests/js/workspace-system-instructions.test.js @@ -0,0 +1,257 @@ +// System › Auto-tagging "Saved instructions" (#431, D203): Built-in read-only, New, Duplicate, Delete (a deleted default +// resets to Built-in), the caps, the editor drafts and the per-engine default selects. +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const instructionsModule = import('../../web/js/workspace/system-instructions.mjs'); +const stateModule = import('../../web/js/workspace/system-state.mjs'); +const coreModule = import('../../web/js/shared/toolkit/core.js'); + +/** A minimal DOM: elements with children, attributes, listeners (aborted by their signal) and focus. */ +function fakeDOM() { + const previous = global.document; + class Element { + constructor(tag) { + this.tagName = tag.toUpperCase(); + this.children = []; + this.attributes = {}; + this.listeners = []; + this.dataset = {}; + this.className = ''; + this.id = ''; + this.value = ''; + this.title = ''; + this.disabled = false; + this._text = ''; + } + get localName() { return this.tagName.toLowerCase(); } + get textContent() { return this._text + this.children.map(child => (typeof child === 'string' ? child : child.textContent)).join(''); } + set textContent(text) { this.children = []; this._text = String(text); } + append(...nodes) { for (const node of nodes) { if (typeof node !== 'string') node.parentNode = this; this.children.push(node); } } + replaceChildren(...nodes) { this.children = []; this._text = ''; this.append(...nodes); } + querySelectorAll(selector) { + assert.equal(selector, 'button'); + const found = []; + const walk = node => { for (const child of node.children) if (typeof child !== 'string') { if (child.tagName === 'BUTTON') found.push(child); walk(child); } }; + walk(this); + return found; + } + focus() { global.document.activeElement = this; } + setAttribute(name, value) { this.attributes[name] = String(value); } + getAttribute(name) { return Object.hasOwn(this.attributes, name) ? this.attributes[name] : null; } + removeAttribute(name) { delete this.attributes[name]; } + addEventListener(type, callback, options = {}) { + if (options.signal?.aborted) return; + const entry = { type, callback }; + this.listeners.push(entry); + options.signal?.addEventListener('abort', () => { this.listeners = this.listeners.filter(item => item !== entry); }); + } + dispatch(type, event = {}) { + return Promise.all(this.listeners.filter(item => item.type === type).map(({ callback }) => callback({ type, target: this, ...event }))); + } + } + global.document = { createElement: tag => new Element(tag), activeElement: null }; + const walk = (node, found = []) => { found.push(node); node.children.forEach(child => typeof child !== 'string' && walk(child, found)); return found; }; + return { find: (root, predicate) => walk(root).find(predicate), restore() { global.document = previous; } }; +} + + +const PHOTO = { id: '0123456789ab', name: 'Custom template', text: 'Photo tags.' }; +const ANIME = { id: '0123456789ac', name: 'Anime', text: 'Anime tags.' }; +const HOST = { autotag_builtin_instructions: 'Built-in text.', autotag_instructions: [PHOTO, ANIME], + autotag_default_instructions: { gguf: PHOTO.id, hf: PHOTO.id } }; + +async function mounted({ host = HOST } = {}) { + const { savedInstructionsBlock } = await instructionsModule; + const { createSystemDrafts } = await stateModule; + const { createScope } = await coreModule; + const dom = fakeDOM(); + const drafts = createSystemDrafts({ + readBrowser: () => ({}), readRuntime: () => ({}), readHost: async () => ({ values: host, revision: 'sha256:one' }), + writeBrowser: async () => ({}), writeHost: async () => ({ success: true, persisted: true }), writeRuntime: async () => ({}), + }); + const scope = createScope(); + let counter = 0; + const random = bytes => bytes.map(() => (counter += 1)); + const block = savedInstructionsBlock({ scope, drafts, random }); + const root = global.document.createElement('div'); + root.append(...block.nodes); + scope.cleanup(drafts.subscribe(block.repaint)); + block.repaint(); + await drafts.reloadHost(); + const byId = id => dom.find(root, node => node.id === id); + const options = () => byId('instruction-list').children.map(option => [option.children[0].textContent, + option.children[1]?.children.map(badge => badge.textContent) ?? [], option.getAttribute('aria-selected')]); + const click = id => byId(id).dispatch('click'); + // A click on an option reaches the list's one listener (as it bubbles in a browser). + const pick = name => byId('instruction-list').dispatch('click', { target: byId('instruction-list').children + .find(option => option.children[0].textContent === name).children[0] }); + return { dom, drafts, scope, block, byId, options, click, pick, root }; +} + +test('helpers: Built-in first, defaults, free names, ids and the server sentences', async () => { + const { allEntries, defaultEntry, freeName, newId, listProblem, entryNote } = await instructionsModule; + assert.deepEqual(allEntries(HOST).map(entry => entry.name), ['Built-in', 'Custom template', 'Anime']); + assert.equal(allEntries(HOST)[0].text, 'Built-in text.'); + assert.equal(defaultEntry(HOST, 'hf').text, 'Photo tags.'); + assert.equal(defaultEntry({ ...HOST, autotag_default_instructions: { gguf: 'gone', hf: 'builtin' } }, 'gguf').name, 'Built-in'); + assert.equal(freeName('New instructions', allEntries(HOST)), 'New instructions'); + assert.equal(freeName('anime', allEntries(HOST)), 'anime (2)'); + assert.equal(freeName('Anime', [...allEntries(HOST), { name: 'Anime (2)' }]), 'Anime (3)'); + let calls = 0; + const id = newId([{ id: '000000000000' }], bytes => { calls += 1; return bytes.fill(calls === 1 ? 0 : 171); }); + assert.equal(id, 'abababababab'); + assert.match(newId([]), /^[0-9a-f]{12}$/); + const list = entries => listProblem({ autotag_instructions: entries }); + assert.equal(list([PHOTO]), null); + assert.equal(list([{ ...PHOTO, name: ' ' }]), 'Saved instructions: entry 1 needs a name.'); + assert.equal(list([PHOTO, { ...ANIME, name: 'x'.repeat(61) }]), 'Saved instructions: entry 2 name must be at most 60 characters.'); + assert.equal(list([{ ...PHOTO, name: 'a\nb' }]), 'Saved instructions: entry 1 name must not contain a line break.'); + assert.equal(list([{ ...PHOTO, name: 'built-in' }]), 'Saved instructions: the name “Built-in” is reserved.'); + assert.equal(list([PHOTO, { ...ANIME, name: ' custom TEMPLATE ' }]), 'Saved instructions: entry 2 name “custom TEMPLATE” is already used.'); + assert.equal(list([{ ...PHOTO, text: ' ' }]), 'Saved instructions: entry 1 text must be non-empty text.'); + assert.equal(list([{ ...PHOTO, text: 'x'.repeat(4001) }]), 'Saved instructions: entry 1 text must be at most 4000 characters.'); + const long = { ...PHOTO, text: 'x'.repeat(4001) }; + assert.equal(entryNote(long, [long]), 'The text must be at most 4,000 characters.'); +}); + +test('the list shows Built-in read-only first, the saved entries and their default chips', async () => { + const view = await mounted(); + try { + assert.deepEqual(view.options(), [['Built-in', ['Read-only'], 'true'], + ['Custom template', ['Default for GGUF', 'Default for HuggingFace'], 'false'], ['Anime', [], 'false']]); + assert.equal(view.byId('instructions-count').textContent, '2 of 20 saved · Built‑in is not counted.'); + assert.equal(view.byId('instruction-name').value, 'Built-in'); + assert.equal(view.byId('instruction-name').readOnly, true); + assert.equal(view.byId('instruction-text').value, 'Built-in text.'); + assert.equal(view.byId('instruction-text').readOnly, true); + assert.equal(view.byId('instruction-note').textContent, 'Built-in instructions can’t be changed. Duplicate them to make your own.'); + assert.equal(view.byId('instruction-delete').disabled, true); + assert.equal(view.byId('instruction-list').getAttribute('role'), 'listbox'); + assert.equal(view.block.nodes[0].dataset.key, 'autotag_instructions'); + assert.deepEqual(view.byId('instruction-default-gguf').children.map(option => option.textContent), ['Built-in', 'Custom template', 'Anime']); + assert.equal(view.byId('instruction-default-gguf').value, PHOTO.id); + assert.equal(view.byId('instruction-default-hf').dataset.key, 'autotag_default_instructions'); + } finally { view.scope.dispose(); view.dom.restore(); } +}); + +test('editing a saved entry drafts the list; its counts and note follow', async () => { + const view = await mounted(); + try { + await view.pick('Anime'); + assert.equal(view.byId('instruction-name').readOnly, false); + assert.equal(view.byId('instruction-delete').disabled, false); + view.byId('instruction-name').value = 'Anime tags'; + await view.byId('instruction-name').dispatch('input'); + view.byId('instruction-text').value = 'Tags, anime.'; + await view.byId('instruction-text').dispatch('input'); + assert.deepEqual(view.drafts.get('host', 'autotag_instructions'), [PHOTO, { ...ANIME, name: 'Anime tags', text: 'Tags, anime.' }]); + assert.equal(view.byId('instruction-name').parentNode.children[0].children[1].textContent, '10 of 60'); + assert.equal(view.byId('instruction-text').parentNode.children[0].children[1].textContent, '12 of 4,000 characters'); + view.byId('instruction-name').value = 'custom template'; + await view.byId('instruction-name').dispatch('input'); + assert.equal(view.byId('instruction-note').textContent, 'The name “custom template” is already used.'); + // Built-in is never edited. + await view.pick('Built-in'); + view.byId('instruction-text').value = 'changed'; + await view.byId('instruction-text').dispatch('input'); + assert.equal(view.drafts.get('host', 'autotag_instructions')[0], PHOTO); + } finally { view.scope.dispose(); view.dom.restore(); } +}); + +test('New and Duplicate append an entry with a fresh id and a free name; the caps disable them', async () => { + const view = await mounted(); + try { + await view.click('instruction-new'); + const added = view.drafts.get('host', 'autotag_instructions').at(-1); + assert.match(added.id, /^[0-9a-f]{12}$/); + assert.deepEqual([added.name, added.text], ['New instructions', '']); + assert.equal(view.options().at(-1)[2], 'true'); + assert.equal(view.byId('instruction-note').textContent, 'The text must not be empty.'); + await view.pick('Built-in'); + await view.click('instruction-duplicate'); + assert.deepEqual(view.drafts.get('host', 'autotag_instructions').at(-1).name, 'Built-in copy'); + assert.equal(view.drafts.get('host', 'autotag_instructions').at(-1).text, 'Built-in text.'); + const full = await mounted({ host: { ...HOST, autotag_instructions: Array.from({ length: 20 }, + (_, i) => ({ id: i.toString(16).padStart(12, '0'), name: `n${i}`, text: 't' })) } }); + try { + assert.equal(full.byId('instruction-new').disabled, true); + assert.equal(full.byId('instruction-duplicate').disabled, true); + assert.equal(full.byId('instruction-cap').hidden, false); + await full.click('instruction-new'); + assert.equal(full.drafts.get('host', 'autotag_instructions').length, 20); + } finally { full.scope.dispose(); } + } finally { view.scope.dispose(); view.dom.restore(); } +}); + +test('deleting a default resets that engine to Built-in and says so; a select change clears the note', async () => { + const view = await mounted(); + try { + await view.pick('Custom template'); + await view.click('instruction-delete'); + assert.deepEqual(view.drafts.get('host', 'autotag_instructions'), [ANIME]); + assert.deepEqual(view.drafts.get('host', 'autotag_default_instructions'), { gguf: 'builtin', hf: 'builtin' }); + assert.equal(view.byId('instruction-reset').textContent, 'GGUF and HuggingFace now use Built-in.'); + assert.equal(view.options()[1][2], 'true', 'the next entry is selected'); + view.byId('instruction-default-hf').value = ANIME.id; + await view.byId('instruction-default-hf').dispatch('change'); + assert.deepEqual(view.drafts.get('host', 'autotag_default_instructions'), { gguf: 'builtin', hf: ANIME.id }); + assert.equal(view.byId('instruction-reset').textContent, ''); + await view.click('instruction-delete'); + assert.equal(view.byId('instruction-reset').textContent, 'HuggingFace now uses Built-in.'); + assert.deepEqual(view.options().map(([name]) => name), ['Built-in']); + } finally { view.scope.dispose(); view.dom.restore(); } +}); + +test('arrow keys move the selection; before the read everything is disabled', async () => { + const view = await mounted(); + try { + const list = view.byId('instruction-list'); + await list.dispatch('keydown', { key: 'ArrowDown' }); + assert.equal(view.byId('instruction-name').value, 'Custom template'); + await list.dispatch('keydown', { key: 'End' }); + assert.equal(view.byId('instruction-name').value, 'Anime'); + await list.dispatch('keydown', { key: 'ArrowDown' }); + assert.equal(view.byId('instruction-name').value, 'Anime'); + await list.dispatch('keydown', { key: 'Home' }); + assert.equal(view.byId('instruction-name').value, 'Built-in'); + await list.dispatch('keydown', { key: 'a' }); + assert.equal(list.getAttribute('aria-activedescendant'), 'instruction-option-builtin'); + } finally { view.scope.dispose(); view.dom.restore(); } + const unread = await mounted({ host: {} }); + try { + assert.equal(unread.byId('instruction-new').disabled, true); + assert.equal(unread.byId('instruction-default-gguf').disabled, true); + } finally { unread.scope.dispose(); unread.dom.restore(); } +}); + +test('the reset note goes once the defaults are saved; a long name is cut to fit its copy suffix', async () => { + const view = await mounted(); + try { + await view.pick('Custom template'); + await view.click('instruction-delete'); + assert.equal(view.byId('instruction-reset').textContent, 'GGUF and HuggingFace now use Built-in.'); + await view.drafts.save(); + assert.equal(view.byId('instruction-reset').textContent, ''); + await view.pick('Anime'); + view.byId('instruction-name').value = 'x'.repeat(60); + await view.byId('instruction-name').dispatch('input'); + await view.click('instruction-duplicate'); + const copy = view.drafts.get('host', 'autotag_instructions').at(-1).name; + assert.equal(copy, `${'x'.repeat(55)} copy`); + await view.click('instruction-duplicate'); + assert.equal([...view.drafts.get('host', 'autotag_instructions').at(-1).name].length <= 60, true); + } finally { view.scope.dispose(); view.dom.restore(); } +}); + +test('option clicks are one listener on the list, not one per repaint', async () => { + const view = await mounted(); + try { + const list = view.byId('instruction-list'); + const before = list.listeners.length; + for (let i = 0; i < 5; i += 1) await view.pick('Anime'); + assert.equal(list.listeners.length, before); + assert.equal(view.byId('instruction-list').children.every(option => option.listeners.length === 0), true); + assert.equal(view.byId('instruction-name').value, 'Anime'); + } finally { view.scope.dispose(); view.dom.restore(); } +}); diff --git a/tests/js/workspace-system-tagging.test.js b/tests/js/workspace-system-tagging.test.js index 7697edd7..7396fe6d 100644 --- a/tests/js/workspace-system-tagging.test.js +++ b/tests/js/workspace-system-tagging.test.js @@ -42,6 +42,8 @@ function fakeDOM() { this.children.push(node); } } + replaceChildren(...nodes) { this.children = []; this._text = ''; this.append(...nodes); } + focus() { global.document.activeElement = this; } remove() { if (this.parentNode) this.parentNode.children = this.parentNode.children.filter(child => child !== this); this.parentNode = null; } setAttribute(name, value) { this.attributes[name] = String(value); } getAttribute(name) { return Object.hasOwn(this.attributes, name) ? this.attributes[name] : null; } @@ -65,7 +67,9 @@ function fakeDOM() { } const HOST = { autotag_model_type: 'wd14-swinv2', autotag_existing: 'skip', autotag_general_threshold: 0.35, - autotag_character_threshold: 0.85, autotag_suggestion_limit: 10, autotag_template: 'Describe the image as tags.' }; + autotag_character_threshold: 0.85, autotag_suggestion_limit: 10, autotag_builtin_instructions: 'Built-in tags.', + autotag_instructions: [{ id: '0123456789ab', name: 'Describe', text: 'Describe the image as tags.' }], + autotag_default_instructions: { gguf: '0123456789ab', hf: 'builtin' } }; const entry = (label, size, extra = {}) => ({ name: label, label, size_label: size, downloaded: true, loaded: false, ...extra }); const MODELS = { success: true, @@ -124,7 +128,8 @@ test('the section keeps the concept surface, rows, labels and actions in order', for (const option of ['Skip already tagged', 'Merge with existing', 'Replace after review']) assert.ok(body.includes(option) && SOURCE.includes(`'${option}'`)); assert.ok(body.includes('

Auto-tagging

') && SOURCE.includes("'h2', 'Auto-tagging'")); assert.ok(body.includes('One tagging configuration for images and prompt-library actions. Every result can be reviewed before applying.')); - assert.ok(body.includes('LLM tag template') && SOURCE.includes("'LLM tag template'")); + // D203 (#431): Saved instructions replace the concept's template field. + assert.ok(body.includes('LLM tag template') && !SOURCE.includes("'LLM tag template'")); assert.ok(body.includes('"Preview tagging review", "tag-review", "", "ca-primary"')); // Prototype copy is never shipped. for (const sample of ['Simulate download', 'Simulate unload', 'Ollama']) assert.ok(!SOURCE.includes(sample), sample); @@ -167,27 +172,25 @@ test('controls draft their host keys; numbers as numbers, other text as typed', const { SETTING_KEYS, coerceNumber, draftValue } = await tagging; assert.deepEqual(SETTING_KEYS, { 'tag-model': 'autotag_model_type', 'tag-existing': 'autotag_existing', 'threshold-general': 'autotag_general_threshold', 'threshold-character': 'autotag_character_threshold', - 'tag-suggestions': 'autotag_suggestion_limit', 'tag-template': 'autotag_template' }); + 'tag-suggestions': 'autotag_suggestion_limit' }); assert.deepEqual(['0.4', ' 1.5 ', '15', '', 'abc', '1e-1'].map(coerceNumber), [0.4, 1.5, 15, '', 'abc', 0.1]); assert.equal(draftValue('autotag_suggestion_limit', '15'), 15); - assert.equal(draftValue('autotag_template', ' 12 '), ' 12 '); + assert.equal(draftValue('autotag_existing', ' 12 '), ' 12 '); assert.equal(draftValue('autotag_existing', undefined), ''); const { dom, drafts, scope, byId } = await mounted(); try { assert.equal(byId('tag-model').value, 'wd14-swinv2'); assert.equal(byId('threshold-general').value, '0.35'); - assert.equal(byId('tag-template').value, HOST.autotag_template); assert.deepEqual([byId('threshold-general').min, byId('threshold-general').max, byId('threshold-general').step], ['0', '1', '0.05']); assert.deepEqual([byId('tag-suggestions').min, byId('tag-suggestions').max], ['1', '100']); byId('tag-existing').value = 'merge'; await byId('tag-existing').dispatch('change'); byId('threshold-general').value = '0.4'; await byId('threshold-general').dispatch('change'); byId('tag-suggestions').value = '15'; await byId('tag-suggestions').dispatch('change'); - byId('tag-template').value = 'Tags only.'; await byId('tag-template').dispatch('input'); assert.deepEqual(drafts.changes('host'), { autotag_existing: 'merge', autotag_general_threshold: 0.4, - autotag_suggestion_limit: 15, autotag_template: 'Tags only.' }); + autotag_suggestion_limit: 15 }); // Every control names its stored key, so a refused save highlights it (D92). for (const id of Object.keys(SETTING_KEYS)) assert.equal(byId(id).dataset.key, SETTING_KEYS[id]); - assert.equal(byId('tag-template').dataset.setting, 'tag-template'); + assert.equal(byId('tag-template'), undefined); } finally { scope.dispose(); assert.equal(dom.listenerCount(), 0); dom.restore(); } }); @@ -201,7 +204,8 @@ test('opening the section reads the model status once; the select waits for it', assert.deepEqual(byId('tag-model').options.map(option => option.textContent), ['WD14 SwinV2 · ~400 MB', 'WD14 ViT · ~400 MB', 'GGUF JoyCaption · ~4.5 GB', 'HuggingFace JoyCaption · ~16 GB']); assert.deepEqual(surface.children.map(node => node.className), - ['', '', 'setting-row', 'setting-row', 'setting-row', 'setting-row', 'setting-row', 'setting-block', 'field', 'row-actions', 'operation']); + ['', '', 'setting-row', 'setting-row', 'setting-row', 'setting-row', 'setting-row', 'setting-block', 'setting-block', 'setting-row', + 'setting-row', 'small instruction-reset', 'row-actions', 'operation']); } finally { scope.dispose(); dom.restore(); } const failed = await mounted({ modelsFail: true }); try { @@ -302,7 +306,9 @@ test('Preview tagging review opens the Tag image dialog with the section’s val assert.deepEqual(taggingPreset(HOST), { model: 'wd14-swinv2', options: { general_threshold: 0.35, character_threshold: 0.85, suggestion_limit: 10 } }); assert.deepEqual(taggingPreset({ ...HOST, autotag_model_type: 'gguf' }), { model: 'gguf', - options: { instructions: HOST.autotag_template, suggestion_limit: 10 } }); + options: { instructions: 'Describe the image as tags.', suggestion_limit: 10 } }); + // D205 (#431): each language-model engine reads its own drafted default; Built-in's text comes from the server. + assert.deepEqual(taggingPreset({ ...HOST, autotag_model_type: 'hf' }).options.instructions, 'Built-in tags.'); assert.equal(draftProblem(HOST), null); assert.deepEqual(draftProblem({ ...HOST, autotag_general_threshold: 1.5 }), { key: 'autotag_general_threshold', message: 'General-tag confidence must be between 0 and 1.' }); @@ -319,15 +325,17 @@ test('Preview tagging review opens the Tag image dialog with the section’s val 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: ' ' }), - { key: 'autotag_template', message: 'LLM tag template must be non-empty text.' }); - assert.deepEqual(draftProblem({ ...HOST, autotag_model_type: 'hf', autotag_template: 'x'.repeat(4001) }), - { key: 'autotag_template', message: 'LLM tag template must be at most 4000 characters.' }); + // #431: a saved entry the server would refuse holds back a language-model run, with the server's sentence. + const entries = list => ({ ...HOST, autotag_model_type: 'hf', autotag_instructions: list }); + assert.deepEqual(draftProblem(entries([{ id: '0123456789ab', name: 'A', text: ' ' }])), + { key: 'autotag_instructions', message: 'Saved instructions: entry 1 text must be non-empty text.' }); + assert.deepEqual(draftProblem(entries([{ id: '0123456789ab', name: 'A', text: 'x'.repeat(4001) }])), + { key: 'autotag_instructions', message: 'Saved instructions: entry 1 text must be at most 4000 characters.' }); // Characters are code points, as Python's len counts them: 4000 emoji (8000 UTF-16 units) are allowed. - assert.equal(draftProblem({ ...HOST, autotag_model_type: 'hf', autotag_template: '🏜'.repeat(4000) }), null); - assert.equal(draftProblem({ ...HOST, autotag_model_type: 'hf', autotag_template: '🏜'.repeat(4001) }).key, 'autotag_template'); - // A WD14 run never reads the template, nor a language model the thresholds. - assert.equal(draftProblem({ ...HOST, autotag_template: '' }), null); + assert.equal(draftProblem(entries([{ id: '0123456789ab', name: 'A', text: '🏜'.repeat(4000) }])), null); + assert.equal(draftProblem(entries([{ id: '0123456789ab', name: 'A', text: '🏜'.repeat(4001) }])).key, 'autotag_instructions'); + // A WD14 run never reads the instructions, nor a language model the thresholds. + assert.equal(draftProblem({ ...HOST, autotag_instructions: [{ id: '0123456789ab', name: '', text: '' }] }), null); assert.equal(draftProblem({ ...HOST, autotag_model_type: 'gguf', autotag_general_threshold: 7 }), null); const none = await mounted({ image: null }); @@ -372,9 +380,12 @@ test('the batch review grid, body and Cancel-only reasons (D134, #379, CUJ 4–5 ['Settings', 'General 0.35 · Character 0.85 · up to 10 tags per image'], ['Backup', 'Create and verify before changes']]); const llm = { ...HOST, autotag_model_type: 'gguf' }; assert.deepEqual(batchGrid({ ...PLAN, model: { type: 'gguf' }, backup: { policy: 'off' }, estimate_seconds: null, - estimate_device: null, suggestion_limit: 4 }, llm).slice(-3), + estimate_device: null, suggestion_limit: 4 }, llm).slice(-4), [['Time', 'Language-model engines take longer per image. Activity shows the time left once tagging starts.'], - ['Settings', 'Template of 27 characters · up to 4 tags per image'], ['Backup', 'No safety backup (the policy is off)']]); + ['Instructions', 'Describe'], ['Settings', 'Up to 4 tags per image'], ['Backup', 'No safety backup (the policy is off)']]); + // D205 (#431): HF shows its own default by name. + assert.deepEqual(batchGrid({ ...PLAN, model: { type: 'hf' } }, { ...llm, autotag_model_type: 'hf' }).find(([term]) => term === 'Instructions'), + ['Instructions', 'Built-in']); assert.equal(batchGrid({}, HOST)[2][1], '0'); assert.equal(batchGrid({}, HOST)[6][1], 'General 0.35 · Character 0.85'); assert.equal(existingLabel('odd'), 'odd'); @@ -382,7 +393,7 @@ test('the batch review grid, body and Cancel-only reasons (D134, #379, CUJ 4–5 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.deepEqual(batchBody(llm), { model_type: 'gguf', existing: 'skip', options: { instructions: 'Describe the image as tags.' } }); assert.equal(batchCloseNote(PLAN), null); assert.equal(batchCloseNote({ ...PLAN, review_required: true }), REPLACE_NOTE); assert.equal(REPLACE_NOTE, 'Replacing tags needs a review of each image. Use Preview tagging review, or choose Skip or Merge for a batch run.'); diff --git a/tests/test_api_annotations.py b/tests/test_api_annotations.py index 769afa89..22b5ef2e 100644 --- a/tests/test_api_annotations.py +++ b/tests/test_api_annotations.py @@ -1144,8 +1144,10 @@ class TestEnvelopes(AnnotationAPITestCase): self.assertEqual( sorted(await self.json_of(resp)), [ + "default_instructions", # #431 adds the saved instructions "default_prompt", "existing", # #328 adds the saved batch policy and model + "instructions", "loaded_model_type", "model_loaded", "model_type", diff --git a/tests/test_api_autotag.py b/tests/test_api_autotag.py index d8cb6978..ae643f9f 100644 --- a/tests/test_api_autotag.py +++ b/tests/test_api_autotag.py @@ -1536,7 +1536,8 @@ class TestSavedTaggingDefaults(AutotagAPITestCase): "EXISTING", "GENERAL_THRESHOLD", "CHARACTER_THRESHOLD", - "TEMPLATE", + "INSTRUCTIONS", + "DEFAULT_INSTRUCTIONS", "SUGGESTION_LIMIT", ): self.addCleanup(setattr, AutotagConfig, attr, getattr(AutotagConfig, attr)) @@ -1557,7 +1558,11 @@ class TestSavedTaggingDefaults(AutotagAPITestCase): self.config.EXISTING = "replace_after_review" self.config.GENERAL_THRESHOLD = 0.4 self.config.CHARACTER_THRESHOLD = 0.7 - self.config.TEMPLATE = "Saved template" + # #431: the saved engine's default instructions. + self.config.INSTRUCTIONS = ( + {"id": "0123456789ab", "name": "Saved", "text": "Saved template"}, + ) + self.config.DEFAULT_INSTRUCTIONS = {"gguf": "0123456789ab", "hf": "builtin"} self.config.SUGGESTION_LIMIT = 15 data = await (await self.client.get("/prompt_manager/autotag/models")).json() self.assertEqual( @@ -1581,6 +1586,27 @@ class TestSavedTaggingDefaults(AutotagAPITestCase): "suggestion_limit": 15, }, ) + # Built-in first with every text, and each engine's default. + self.assertEqual( + data["instructions"], + [ + {"id": "builtin", "name": "Built-in", "text": DEFAULT_TAG_TEMPLATE}, + {"id": "0123456789ab", "name": "Saved", "text": "Saved template"}, + ], + ) + self.assertEqual( + data["default_instructions"], {"gguf": "0123456789ab", "hf": "builtin"} + ) + # HF reads its own default; a WD14 engine reads GGUF's. + for model_type, prompt in ( + ("hf", DEFAULT_TAG_TEMPLATE), + ("wd14-vit", "Saved template"), + ): + self.config.MODEL_TYPE = model_type + data = await ( + await self.client.get("/prompt_manager/autotag/models") + ).json() + self.assertEqual(data["default_prompt"], prompt, model_type) async def test_single_without_thresholds_uses_saved_ones(self): image = self._make_image(self.output_dir) diff --git a/tests/test_autotag_legacy.py b/tests/test_autotag_legacy.py index f8a379a7..8f5d8d3f 100644 --- a/tests/test_autotag_legacy.py +++ b/tests/test_autotag_legacy.py @@ -158,7 +158,30 @@ class TestLegacyBatch(LegacyTestCase): self.image("b.png", (2, 2, 2)) await self.relist() await self.legacy("model_type=gguf&prompt=%20") - self.assertEqual(self.tagger.calls[-1]["prompt"], AutotagConfig.TEMPLATE) + self.assertEqual( + self.tagger.calls[-1]["prompt"], + AutotagConfig.default_instructions_text("gguf"), + ) + # #431: a blank prompt falls back to the request engine's own default. + saved = (AutotagConfig.INSTRUCTIONS, AutotagConfig.DEFAULT_INSTRUCTIONS) + self.addCleanup( + lambda: ( + setattr(AutotagConfig, "INSTRUCTIONS", saved[0]), + setattr(AutotagConfig, "DEFAULT_INSTRUCTIONS", saved[1]), + ) + ) + AutotagConfig.INSTRUCTIONS = ( + {"id": "0123456789ab", "name": "Photo", "text": "Photo tags."}, + ) + AutotagConfig.DEFAULT_INSTRUCTIONS = {"gguf": "builtin", "hf": "0123456789ab"} + for model_type, expected in ( + ("hf", "Photo tags."), + ("gguf", AutotagConfig.default_instructions_text("gguf")), + ): + self.image(f"{model_type}.png", (3, 3, len(model_type))) + await self.relist() + await self.legacy(f"model_type={model_type}&prompt=%20&skip_tagged=false") + self.assertEqual(self.tagger.calls[-1]["prompt"], expected, model_type) self.assertEqual(self.tagger.custom_prompt, "legacy prompt") async def test_a_missing_backend_package_is_one_error_frame(self): diff --git a/tests/test_config.py b/tests/test_config.py index 145b339e..945aa5ab 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -442,9 +442,11 @@ class TestAutotagConfig(unittest.TestCase): "EXISTING", "GENERAL_THRESHOLD", "CHARACTER_THRESHOLD", - "TEMPLATE", "SUGGESTION_LIMIT", "SKIPPED_TAGS", + "INSTRUCTIONS", + "DEFAULT_INSTRUCTIONS", + "_LEGACY_TEMPLATE", ) } @@ -489,16 +491,13 @@ class TestAutotagConfig(unittest.TestCase): "character_threshold": 0.85, "suggestion_limit": 10, "skipped_tags": [], + "instructions": [], + "default_instructions": {"gguf": "builtin", "hf": "builtin"}, } }, ) - # The built-in template is not written; a saved one is. - self.config_class.TEMPLATE = "Saved." - self.assertEqual( - PromptManagerConfig.get_config()["autotag"]["defaults"]["template"], - "Saved.", - ) - self.config_class.TEMPLATE = config_module.DEFAULT_TAG_TEMPLATE + # #431: the old template key is written only as the GGUF default's + # mirror (test_saved_instructions covers it). with tempfile.TemporaryDirectory() as tmpdir: path = os.path.join(tmpdir, "config.json") PromptManagerConfig.update_config( @@ -526,7 +525,6 @@ class TestAutotagConfig(unittest.TestCase): "existing": "merge", "general_threshold": 0.4, "character_threshold": 0.7, - "template": "Tags only.", "suggestion_limit": 15, "skipped_tags": ("blurry", "closed eyes"), } @@ -535,7 +533,6 @@ class TestAutotagConfig(unittest.TestCase): "existing": "skip", "general_threshold": 0.35, "character_threshold": 0.85, - "template": config_module.DEFAULT_TAG_TEMPLATE, "suggestion_limit": 10, "skipped_tags": (), } @@ -569,7 +566,12 @@ class TestAutotagConfig(unittest.TestCase): k: getattr(self.config_class, attr) for k, (attr, _v, _d) in self.config_class._KEYS.items() } - self.assertEqual(loaded, dict(good, **{key: fallback[key]})) + # The old template is no attribute any more (#431): a bad + # one is only warned about and imports nothing. + expected = ( + dict(good, **{key: fallback[key]}) if key in fallback else good + ) + self.assertEqual(loaded, expected) def test_skipped_tags_load_trimmed_and_warn_once_when_damaged(self): """#431: the list is kept as typed (trimmed); a damaged list is diff --git a/tests/test_saved_instructions.py b/tests/test_saved_instructions.py new file mode 100644 index 00000000..f7f4b09f --- /dev/null +++ b/tests/test_saved_instructions.py @@ -0,0 +1,379 @@ +"""Saved instructions with a default per language-model engine (#431, PR 2). + +The entry and default validation, the in-memory migration of the old single +``autotag.defaults.template`` at load (first upgrade and re-upgrade), the +load fallbacks, and ``AutotagConfig.default_instructions_text``. +""" + +import hashlib +import json +import os +import sys +import tempfile +import unittest +from unittest.mock import MagicMock + +# config.py imports ComfyUI's server module; stand it in (as test_config does). +_mock_server = MagicMock() +_mock_server.PromptServer.instance.routes = MagicMock() +sys.modules.setdefault("server", _mock_server) + +from py.config import AutotagConfig, PromptManagerConfig # noqa: E402 +from py.tagging_defaults import ( # noqa: E402 + BUILTIN_INSTRUCTIONS_ID, + DEFAULT_TAG_TEMPLATE, + MAX_SAVED_INSTRUCTIONS, + validate_default_instructions, + validate_instructions, +) + +ATTRS = ("MODEL_TYPE", "INSTRUCTIONS", "DEFAULT_INSTRUCTIONS", "_LEGACY_TEMPLATE") + + +def entry(id_, name, text="Tags only."): + return {"id": id_, "name": name, "text": text} + + +def sha12(text): + return hashlib.sha256(text.encode("utf-8")).hexdigest()[:12] + + +class TestValidation(unittest.TestCase): + def test_entries_are_kept_with_trimmed_names(self): + self.assertEqual( + validate_instructions([entry("0123456789ab", " Photo ", "Tags.")]), + [entry("0123456789ab", "Photo", "Tags.")], + ) + self.assertEqual(validate_instructions([]), []) + + def test_every_refusal_sentence(self): + ok = entry("0123456789ab", "One") + cases = ( + ("x", "Saved instructions must be a list."), + ( + [ + entry(f"{i:012x}", f"n{i}") + for i in range(MAX_SAVED_INSTRUCTIONS + 1) + ], + "Saved instructions: at most 20 entries besides Built-in.", + ), + ([ok, "x"], "Saved instructions: entry 2 has an invalid id."), + ( + [entry("ABCDEF012345", "A")], + "Saved instructions: entry 1 has an invalid id.", + ), + ([entry("builtin", "A")], "Saved instructions: entry 1 has an invalid id."), + ( + [{"id": "0123456789ab", "name": "A"}], + "Saved instructions: entry 1 has an invalid id.", + ), + ( + [ok, entry("0123456789ab", "Two")], + "Saved instructions: entry 2 repeats the id of entry 1.", + ), + ( + [entry("0123456789ab", " ")], + "Saved instructions: entry 1 needs a name.", + ), + ( + [entry("0123456789ab", "x" * 61)], + "Saved instructions: entry 1 name must be at most 60 characters.", + ), + ( + [entry("0123456789ab", "a\nb")], + "Saved instructions: entry 1 name must not contain a line break.", + ), + ( + [ok, entry("0123456789ac", "one")], + "Saved instructions: entry 2 name “one” is already used.", + ), + ( + [entry("0123456789ab", "BUILT-IN")], + "Saved instructions: the name “Built-in” is reserved.", + ), + ( + [entry("0123456789ab", "A", " ")], + "Saved instructions: entry 1 text must be non-empty text.", + ), + ( + [entry("0123456789ab", "A", "x" * 4001)], + "Saved instructions: entry 1 text must be at most 4000 characters.", + ), + ) + for value, message in cases: + with self.subTest(message=message): + with self.assertRaises(ValueError) as caught: + validate_instructions(value) + self.assertEqual(str(caught.exception), message) + + def test_defaults_name_built_in_or_a_saved_entry(self): + saved = [entry("0123456789ab", "One")] + self.assertEqual( + validate_default_instructions( + {"gguf": "builtin", "hf": "0123456789ab"}, saved + ), + {"gguf": "builtin", "hf": "0123456789ab"}, + ) + for value, message in ( + ( + {"gguf": "ffffffffffff", "hf": "builtin"}, + "Default for GGUF JoyCaption must be one of the saved instructions.", + ), + ( + {"gguf": "builtin", "hf": "nope"}, + "Default for HuggingFace JoyCaption must be one of the saved " + "instructions.", + ), + ( + {"gguf": "builtin"}, + "Default instructions must name GGUF and HuggingFace.", + ), + ( + {"gguf": [], "hf": "builtin"}, + "Default for GGUF JoyCaption must be one of the saved instructions.", + ), + ("builtin", "Default instructions must name GGUF and HuggingFace."), + ): + with self.subTest(value=value): + with self.assertRaises(ValueError) as caught: + validate_default_instructions(value, saved) + self.assertEqual(str(caught.exception), message) + + +class ConfigTestCase(unittest.TestCase): + def setUp(self): + saved = {attr: getattr(AutotagConfig, attr) for attr in ATTRS} + self.addCleanup( + lambda: [setattr(AutotagConfig, a, v) for a, v in saved.items()] + ) + AutotagConfig.update_config({"defaults": {}}) + + def load(self, defaults, warnings=0): + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "config.json") + with open(path, "w", encoding="utf-8") as handle: + json.dump({"autotag": {"defaults": defaults}}, handle) + before = os.stat(path).st_mtime_ns, open(path, "rb").read() + if warnings: + with self.assertLogs("prompt_manager.config", level="WARNING") as logs: + PromptManagerConfig.load_from_file(path) + self.assertEqual(len(logs.records), warnings, logs.output) + self.logs = [record.getMessage() for record in logs.records] + else: + PromptManagerConfig.load_from_file(path) + # Nothing is written at load. + self.assertEqual( + (os.stat(path).st_mtime_ns, open(path, "rb").read()), before + ) + + def state(self): + return [dict(e) for e in AutotagConfig.INSTRUCTIONS], dict( + AutotagConfig.DEFAULT_INSTRUCTIONS + ) + + +class TestLoad(ConfigTestCase): + def test_a_new_install_has_built_in_only_for_both_engines(self): + self.load({}) + self.assertEqual(self.state(), ([], {"gguf": "builtin", "hf": "builtin"})) + self.assertEqual( + AutotagConfig.default_instructions_text("gguf"), DEFAULT_TAG_TEMPLATE + ) + self.assertEqual( + AutotagConfig.all_instructions()[0], + { + "id": BUILTIN_INSTRUCTIONS_ID, + "name": "Built-in", + "text": DEFAULT_TAG_TEMPLATE, + }, + ) + + def test_saved_entries_and_per_engine_defaults_load(self): + saved = [ + entry("0123456789ab", "Photo", "Photo tags."), + entry("0123456789ac", "Anime", "Anime tags."), + ] + self.load( + { + "instructions": saved, + "default_instructions": {"gguf": "0123456789ac", "hf": "0123456789ab"}, + } + ) + self.assertEqual( + self.state(), (saved, {"gguf": "0123456789ac", "hf": "0123456789ab"}) + ) + self.assertEqual(AutotagConfig.default_instructions_text("gguf"), "Anime tags.") + self.assertEqual(AutotagConfig.default_instructions_text("hf"), "Photo tags.") + with self.assertRaises(ValueError): + AutotagConfig.default_instructions_text("wd14-vit") + + def test_a_dangling_default_falls_back_to_built_in_with_a_warning(self): + saved = [entry("0123456789ab", "Photo")] + self.load( + { + "instructions": saved, + "default_instructions": {"gguf": "ffffffffffff", "hf": "0123456789ab"}, + }, + warnings=1, + ) + self.assertEqual( + self.state(), (saved, {"gguf": "builtin", "hf": "0123456789ab"}) + ) + self.assertEqual( + self.logs, + [ + "autotag.defaults.default_instructions.gguf names no saved " + "instructions; using Built-in" + ], + ) + + def test_a_damaged_list_falls_back_to_built_in_only_and_the_defaults_follow(self): + self.load( + { + "instructions": [ + entry("0123456789ab", "A"), + entry("0123456789ab", "B"), + ], + "default_instructions": {"gguf": "0123456789ab", "hf": "builtin"}, + }, + warnings=1, # one warning; the defaults follow the list silently + ) + self.assertEqual(self.state(), ([], {"gguf": "builtin", "hf": "builtin"})) + self.assertEqual( + self.logs[0], + "autotag.defaults.instructions entry 2 repeats the id of entry 1; " + "using Built-in only", + ) + + def test_damaged_defaults_fall_back_with_a_warning(self): + self.load({"default_instructions": "builtin"}, warnings=1) + self.assertEqual(self.state(), ([], {"gguf": "builtin", "hf": "builtin"})) + + +class TestMigration(ConfigTestCase): + CUSTOM = "My own tags, please." + + def test_first_upgrade_makes_custom_template_the_default_for_both(self): + self.load({"template": self.CUSTOM}) + self.assertEqual( + self.state(), + ( + [entry(sha12(self.CUSTOM), "Custom template", self.CUSTOM)], + {"gguf": sha12(self.CUSTOM), "hf": sha12(self.CUSTOM)}, + ), + ) + # Deterministic: a second load gives the same ids. + first = self.state() + self.load({"template": self.CUSTOM}) + self.assertEqual(self.state(), first) + + def test_a_template_equal_to_built_in_creates_no_entry(self): + self.load({"template": DEFAULT_TAG_TEMPLATE}) + self.assertEqual(self.state(), ([], {"gguf": "builtin", "hf": "builtin"})) + + def test_an_invalid_template_is_ignored_with_todays_warning(self): + self.load({"template": " "}, warnings=1) + self.assertEqual(self.state(), ([], {"gguf": "builtin", "hf": "builtin"})) + self.assertIn("autotag.defaults.template", self.logs[0]) + + def test_a_stored_default_is_never_overwritten_by_the_first_upgrade(self): + self.load( + { + "template": self.CUSTOM, + "default_instructions": {"gguf": "builtin", "hf": "builtin"}, + }, + warnings=0, + ) + self.assertEqual( + self.state(), + ( + [entry(sha12(self.CUSTOM), "Custom template", self.CUSTOM)], + {"gguf": "builtin", "hf": "builtin"}, + ), + ) + + def test_the_mirror_of_a_saved_default_imports_nothing(self): + saved = [entry(sha12(self.CUSTOM), "Custom template", self.CUSTOM)] + both = {"gguf": sha12(self.CUSTOM), "hf": "builtin"} + self.load( + { + "template": self.CUSTOM, + "instructions": saved, + "default_instructions": both, + } + ) + self.assertEqual(self.state(), (saved, both)) + + def test_re_upgrade_imports_an_edit_made_on_an_older_build(self): + saved = [entry(sha12(self.CUSTOM), "Custom template", "Edited since.")] + both = {"gguf": sha12(self.CUSTOM), "hf": "builtin"} + with self.assertLogs("prompt_manager.config", level="INFO") as logs: + self.load( + { + "template": self.CUSTOM, + "instructions": saved, + "default_instructions": both, + } + ) + instructions, defaults = self.state() + # The text's own hash is taken by the edited entry: the next free one. + imported = hashlib.sha256((self.CUSTOM + "\n2").encode()).hexdigest()[:12] + self.assertEqual( + instructions, saved + [entry(imported, "Custom template (2)", self.CUSTOM)] + ) + self.assertEqual(defaults, both) + self.assertIn( + "Imported the tagging template edited on an older version as " + "'Custom template (2)'", + "\n".join(logs.output), + ) + + def test_re_upgrade_into_a_full_list_imports_nothing_and_warns(self): + saved = [entry(f"{i:012x}", f"n{i}") for i in range(MAX_SAVED_INSTRUCTIONS)] + self.load({"template": self.CUSTOM, "instructions": saved}, warnings=1) + self.assertEqual(self.state()[0], saved) + self.assertIn("full", self.logs[0]) + + +class TestGetConfig(ConfigTestCase): + def test_get_config_writes_the_lists_and_the_gguf_mirror(self): + AutotagConfig.INSTRUCTIONS = (entry("0123456789ab", "Photo", "Photo tags."),) + AutotagConfig.DEFAULT_INSTRUCTIONS = {"gguf": "0123456789ab", "hf": "builtin"} + defaults = AutotagConfig.get_config()["defaults"] + self.assertEqual( + defaults["instructions"], [entry("0123456789ab", "Photo", "Photo tags.")] + ) + self.assertEqual( + defaults["default_instructions"], {"gguf": "0123456789ab", "hf": "builtin"} + ) + self.assertEqual(defaults["template"], "Photo tags.") + AutotagConfig.DEFAULT_INSTRUCTIONS = {"gguf": "builtin", "hf": "0123456789ab"} + self.assertNotIn("template", AutotagConfig.get_config()["defaults"]) + + +class TestDamagedValues(ConfigTestCase): + def test_a_non_text_default_falls_back_with_a_warning(self): + self.load({"default_instructions": {"gguf": [], "hf": "builtin"}}, warnings=1) + self.assertEqual(self.state(), ([], {"gguf": "builtin", "hf": "builtin"})) + + def test_a_damaged_list_warns_once_and_migrates_the_template_afresh(self): + custom = "My own tags, please." + self.load( + { + "template": custom, + "instructions": "broken", + "default_instructions": {"gguf": "0123456789ab", "hf": "builtin"}, + }, + warnings=1, + ) + self.assertEqual( + self.state(), + ( + [entry(sha12(custom), "Custom template", custom)], + {"gguf": sha12(custom), "hf": sha12(custom)}, + ), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_saved_instructions_routes.py b/tests/test_saved_instructions_routes.py new file mode 100644 index 00000000..0eee2e46 --- /dev/null +++ b/tests/test_saved_instructions_routes.py @@ -0,0 +1,224 @@ +"""Saved instructions through POST /settings and the legacy fallbacks (#431). + +Server-side pairing (a save carrying either key writes both, the missing one +from the running, migrated configuration), the downgrade mirror in the old +``template`` key, the refusals, GET /settings, and the legacy single route's +blank-prompt fallback per request engine. The "restart" is a reload of the +isolated config.json (``PROMPT_MANAGER_CONFIG_PATH`` in the temp tree). +""" + +import hashlib + +try: + from test_settings_sources import SourcesTestCase + from test_autotag_legacy import LegacyTestCase +except ImportError: + from tests.test_settings_sources import SourcesTestCase + from tests.test_autotag_legacy import LegacyTestCase + +from py.config import AutotagConfig, PromptManagerConfig +from py.tagging_defaults import DEFAULT_TAG_TEMPLATE + +ATTRS = ("MODEL_TYPE", "INSTRUCTIONS", "DEFAULT_INSTRUCTIONS", "_LEGACY_TEMPLATE") +SETTINGS = "/prompt_manager/settings" +CUSTOM = "My own tags, please." +CUSTOM_ID = hashlib.sha256(CUSTOM.encode()).hexdigest()[:12] + + +def entry(id_, name, text): + return {"id": id_, "name": name, "text": text} + + +class InstructionsTestCase(SourcesTestCase): + async def asyncSetUp(self): + await super().asyncSetUp() + saved = {attr: getattr(AutotagConfig, attr) for attr in ATTRS} + self.addCleanup( + lambda: [setattr(AutotagConfig, a, v) for a, v in saved.items()] + ) + + def restart(self): + PromptManagerConfig.load_from_file(self.config_path) + + async def save(self, payload, revision=True): + if revision: + payload = {"expected_revision": (await self._get())["revision"], **payload} + return await self._post(SETTINGS, payload) + + def disk(self): + return self._read_config()["autotag"]["defaults"] + + +class TestMigrationPersists(InstructionsTestCase): + async def asyncSetUp(self): + await super().asyncSetUp() + self._write_config( + {"custom": {"keep": True}, "autotag": {"defaults": {"template": CUSTOM}}} + ) + self.restart() + + async def test_get_shows_the_migrated_entry_and_writes_nothing(self): + before = self._config_bytes() + settings = (await self._get())["settings"] + self.assertEqual( + settings["autotag_instructions"], + [entry(CUSTOM_ID, "Custom template", CUSTOM)], + ) + self.assertEqual( + settings["autotag_default_instructions"], + {"gguf": CUSTOM_ID, "hf": CUSTOM_ID}, + ) + self.assertEqual(settings["autotag_template"], CUSTOM) + self.assertEqual(settings["autotag_builtin_instructions"], DEFAULT_TAG_TEMPLATE) + self.assertEqual(self._config_bytes(), before) + + async def test_a_save_of_only_the_hf_default_writes_the_migrated_list_too(self): + status, data = await self.save( + {"autotag_default_instructions": {"gguf": CUSTOM_ID, "hf": "builtin"}} + ) + self.assertEqual(status, 200, data) + disk = self.disk() + self.assertEqual( + disk["instructions"], [entry(CUSTOM_ID, "Custom template", CUSTOM)] + ) + self.assertEqual( + disk["default_instructions"], {"gguf": CUSTOM_ID, "hf": "builtin"} + ) + self.assertEqual(disk["template"], CUSTOM) + self.assertEqual(self._read_config()["custom"], {"keep": True}) + self.restart() + self.assertEqual( + (list(AutotagConfig.INSTRUCTIONS), AutotagConfig.DEFAULT_INSTRUCTIONS), + ( + [entry(CUSTOM_ID, "Custom template", CUSTOM)], + {"gguf": CUSTOM_ID, "hf": "builtin"}, + ), + ) + + async def test_gguf_built_in_deletes_the_mirror_and_keeps_custom_template(self): + status, data = await self.save( + {"autotag_default_instructions": {"gguf": "builtin", "hf": CUSTOM_ID}} + ) + self.assertEqual(status, 200, data) + self.assertNotIn("template", self.disk()) + self.restart() + self.assertEqual( + list(AutotagConfig.INSTRUCTIONS), + [entry(CUSTOM_ID, "Custom template", CUSTOM)], + ) + self.assertEqual( + AutotagConfig.DEFAULT_INSTRUCTIONS, {"gguf": "builtin", "hf": CUSTOM_ID} + ) + self.assertEqual( + AutotagConfig.default_instructions_text("gguf"), DEFAULT_TAG_TEMPLATE + ) + + async def test_a_save_of_only_the_list_keeps_the_defaults_that_still_exist(self): + photo = entry("0123456789ab", "Photo", "Photo tags.") + status, data = await self.save( + { + "autotag_instructions": [ + entry(CUSTOM_ID, "Custom template", CUSTOM), + photo, + ] + } + ) + self.assertEqual(status, 200, data) + self.assertEqual( + self.disk()["default_instructions"], {"gguf": CUSTOM_ID, "hf": CUSTOM_ID} + ) + self.assertEqual(AutotagConfig.INSTRUCTIONS[1], photo) + # A list that drops a default without moving it is refused, nothing written. + before = self._config_bytes() + status, data = await self.save({"autotag_instructions": [photo]}) + self.assertEqual(status, 400, data) + self.assertEqual( + data["error"], + "Default for GGUF JoyCaption must be one of the saved instructions.", + ) + self.assertEqual(self._config_bytes(), before) + + +class TestRefusals(InstructionsTestCase): + async def test_the_old_template_key_is_refused(self): + before = self._config_bytes() + status, data = await self.save({"autotag_template": "Tags only."}) + self.assertEqual( + (status, data["error"]), + ( + 400, + "LLM tag template was replaced by Saved instructions. Reload the page.", + ), + ) + self.assertEqual(self._config_bytes(), before) + + async def test_instruction_keys_without_a_revision_are_refused(self): + for payload in ( + {"autotag_instructions": []}, + {"autotag_default_instructions": {"gguf": "builtin", "hf": "builtin"}}, + ): + with self.subTest(payload=payload): + status, data = await self.save(payload, revision=False) + self.assertEqual( + (status, data["error"]), (400, "Reload the page and try again.") + ) + + async def test_entry_refusals_name_the_entry(self): + status, data = await self.save( + { + "autotag_instructions": [ + entry("0123456789ab", "Photo", "Photo tags."), + entry("0123456789ac", "photo", "x"), + ] + } + ) + self.assertEqual( + (status, data["error"]), + (400, "Saved instructions: entry 2 name “photo” is already used."), + ) + + async def test_a_new_entry_can_become_a_default_in_one_save(self): + photo = entry("0123456789ab", "Photo", "Photo tags.") + status, data = await self.save( + { + "autotag_instructions": [photo], + "autotag_default_instructions": { + "gguf": "0123456789ab", + "hf": "builtin", + }, + } + ) + self.assertEqual(status, 200, data) + self.assertEqual(self.disk()["template"], "Photo tags.") + settings = (await self._get())["settings"] + self.assertEqual(settings["autotag_template"], "Photo tags.") + self.assertEqual(AutotagConfig.default_instructions_text("gguf"), "Photo tags.") + self.assertEqual(AutotagConfig._LEGACY_TEMPLATE, "Photo tags.") + + +class TestLegacySingleFallback(LegacyTestCase): + async def test_a_blank_prompt_uses_the_request_engines_default(self): + saved = {attr: getattr(AutotagConfig, attr) for attr in ATTRS} + self.addCleanup( + lambda: [setattr(AutotagConfig, a, v) for a, v in saved.items()] + ) + AutotagConfig.INSTRUCTIONS = (entry("0123456789ab", "Photo", "Photo tags."),) + AutotagConfig.DEFAULT_INSTRUCTIONS = {"gguf": "builtin", "hf": "0123456789ab"} + prompt = self.prompt("a prompt") + image = self.link(prompt, self.image("a.png", (1, 2, 3), sees=["bird"])) + for model_type, expected in ( + ("hf", "Photo tags."), + ("gguf", DEFAULT_TAG_TEMPLATE), + ): + with self.subTest(model_type=model_type): + status, data = await self.single( + {"image_id": image, "model_type": model_type, "prompt": " "} + ) + self.assertEqual(status, 200, data) + self.assertEqual(self.tagger.calls[-1]["prompt"], expected) + + +if __name__ == "__main__": + import unittest + + unittest.main() diff --git a/tests/test_settings_sources.py b/tests/test_settings_sources.py index ef05eea7..40cb326f 100644 --- a/tests/test_settings_sources.py +++ b/tests/test_settings_sources.py @@ -1023,16 +1023,17 @@ class TestAutotagDefaultsSettings(SourcesTestCase): "EXISTING", "GENERAL_THRESHOLD", "CHARACTER_THRESHOLD", - "TEMPLATE", "SUGGESTION_LIMIT", "SKIPPED_TAGS", + "INSTRUCTIONS", + "DEFAULT_INSTRUCTIONS", + "_LEGACY_TEMPLATE", ) SAVED = { "autotag_model_type": "gguf", "autotag_existing": "merge", "autotag_general_threshold": 0.4, "autotag_character_threshold": 0.8, - "autotag_template": "Tags only.", "autotag_suggestion_limit": 15, } @@ -1068,9 +1069,11 @@ class TestAutotagDefaultsSettings(SourcesTestCase): "EXISTING": "merge", "GENERAL_THRESHOLD": 0.4, "CHARACTER_THRESHOLD": 0.8, - "TEMPLATE": "Tags only.", "SUGGESTION_LIMIT": 15, "SKIPPED_TAGS": (), + "INSTRUCTIONS": (), + "DEFAULT_INSTRUCTIONS": {"gguf": "builtin", "hf": "builtin"}, + "_LEGACY_TEMPLATE": None, }, ) settings = (await self._get())["settings"] @@ -1141,11 +1144,11 @@ class TestAutotagDefaultsSettings(SourcesTestCase): "replace", "Existing tags must be skip, merge or replace_after_review.", ), - ("autotag_template", " ", "LLM tag template must be non-empty text."), + # #431: the old template is refused whatever it holds. ( "autotag_template", - "x" * 4001, - "LLM tag template must be at most 4000 characters.", + "Tags only.", + "LLM tag template was replaced by Saved instructions. Reload the page.", ), ( "autotag_suggestion_limit", diff --git a/tools/vendor/frontend-coverage-baseline.json b/tools/vendor/frontend-coverage-baseline.json index 72ef55d5..dae0d0d9 100644 --- a/tools/vendor/frontend-coverage-baseline.json +++ b/tools/vendor/frontend-coverage-baseline.json @@ -629,6 +629,12 @@ "functions": 95, "statements": 98 }, + "workspace/system-instructions.mjs": { + "lines": 80, + "branches": 80, + "functions": 80, + "statements": 80 + }, "workspace/system-skipped-tags.mjs": { "lines": 80, "branches": 80, diff --git a/web/js/workspace/system-instructions.mjs b/web/js/workspace/system-instructions.mjs new file mode 100644 index 00000000..ee3e6bfc --- /dev/null +++ b/web/js/workspace/system-instructions.mjs @@ -0,0 +1,351 @@ +/** + * System › Auto-tagging "Saved instructions" (#431, D203): named language-model instructions and the default of each + * language-model engine. It replaces the concept's "LLM tag template" textarea. + * + * A `.setting-block` (`data-setting="tag-instructions"`, `data-key="autotag_instructions"`): the label, the + * description with the count ("N of 20 saved · Built\u2011in is not counted."), a `role="listbox"` list (Built-in first, + * "Read-only", then the saved entries, each with its "Default for GGUF" / "Default for HuggingFace" chips), "+ New", + * "Duplicate" and "Delete", and the editor beside the list (under it below 700 px): "Name" (60 characters, live count) + * and "Text" ("N of 4,000 characters"), both read-only for Built-in with its note. Then two `.setting-row` selects, + * "Default for GGUF JoyCaption" and "Default for HuggingFace JoyCaption" (`data-key="autotag_default_instructions"`), + * and the note a deleted default leaves ("GGUF now uses Built-in."). Host drafts: `autotag_instructions` (the saved + * entries, never Built-in) and `autotag_default_instructions`; the server pairs them on save. Built-in's text is the + * read-only `autotag_builtin_instructions` of GET /settings. Ids are 12 lowercase hex characters made here. + */ +import { createButton } from '../shared/toolkit/core.js'; +import { settingRow } from './system-controls.mjs'; + +export const INSTRUCTIONS_KEY = 'autotag_instructions'; +export const DEFAULTS_KEY = 'autotag_default_instructions'; +export const BUILTIN_TEXT_KEY = 'autotag_builtin_instructions'; +export const BUILTIN_ID = 'builtin'; +export const BUILTIN_NAME = 'Built-in'; +export const MAX_SAVED = 20; +export const MAX_NAME_CHARS = 60; +export const MAX_TEXT_CHARS = 4000; +export const ENGINES = Object.freeze([['gguf', 'GGUF', 'GGUF JoyCaption'], ['hf', 'HuggingFace', 'HuggingFace JoyCaption']]); + +export const INSTRUCTIONS_COPY = Object.freeze({ + label: 'Saved instructions', + description: 'Instructions for the language-model engines. Built-in can’t be changed; duplicate it to make your own.', + count: saved => `${saved} of ${MAX_SAVED} saved · Built\u2011in is not counted.`, + readOnly: 'Read-only', + defaultFor: short => `Default for ${short}`, + newLabel: '+ New', + duplicate: 'Duplicate', + delete: 'Delete', + cap: `At most ${MAX_SAVED} saved instructions.`, + name: 'Name', + text: 'Text', + nameCount: length => `${length} of ${MAX_NAME_CHARS}`, + textCount: length => `${length.toLocaleString('en-US')} of ${MAX_TEXT_CHARS.toLocaleString('en-US')} characters`, + builtinNote: 'Built-in instructions can’t be changed. Duplicate them to make your own.', + newName: 'New instructions', + copyName: name => `${name} copy`, + defaultRow: long => `Default for ${long}`, + defaultDescription: 'The Tag image dialog and batches start from this.', + reset: shorts => `${shorts.join(' and ')} now ${shorts.length > 1 ? 'use' : 'uses'} Built-in.`, +}); + +/** Characters as Python's `len` counts them (code points). */ +const characters = text => [...String(text ?? '')].length; +const fold = text => String(text ?? '').trim().toLowerCase(); + +/** The saved entries of the drafts (`[]` when unread or damaged). */ +export const savedEntries = values => (Array.isArray(values?.[INSTRUCTIONS_KEY]) ? values[INSTRUCTIONS_KEY] : []); + +/** Built-in (its text from GET /settings) and then the saved entries. */ +export function allEntries(values) { + const text = typeof values?.[BUILTIN_TEXT_KEY] === 'string' ? values[BUILTIN_TEXT_KEY] : ''; + return [{ id: BUILTIN_ID, name: BUILTIN_NAME, text }, ...savedEntries(values)]; +} + +/** The drafted default entry of `engine` (`gguf` or `hf`); Built-in when the id names nothing. */ +export function defaultEntry(values, engine) { + const entries = allEntries(values); + const id = values?.[DEFAULTS_KEY]?.[engine]; + return entries.find(entry => entry.id === id) ?? entries[0]; +} + +/** `base`, else `base (2)`, `base (3)`… whichever no entry uses (case-insensitive). */ +export function freeName(base, entries) { + const taken = new Set(entries.map(entry => fold(entry.name))); + if (!taken.has(fold(base))) return base; + for (let n = 2; ; n += 1) if (!taken.has(fold(`${base} (${n})`))) return `${base} (${n})`; +} + +/** " copy" (then "(2)"…), the name cut so the whole stays within the 60-character limit. */ +export function copyName(name, entries) { + for (let room = MAX_NAME_CHARS - ' copy'.length; room > 0; room -= 1) { + const base = [...String(name ?? '').trim()].slice(0, room).join('').trimEnd(); + const candidate = freeName(INSTRUCTIONS_COPY.copyName(base), entries); + if (characters(candidate) <= MAX_NAME_CHARS) return candidate; + } + return freeName(INSTRUCTIONS_COPY.copyName(BUILTIN_NAME), entries); +} + +/** A new entry id: 12 lowercase hex characters from `random` (crypto.getRandomValues), never one already used. */ +export function newId(entries, random = bytes => globalThis.crypto.getRandomValues(bytes)) { + const used = new Set(entries.map(entry => entry.id)); + for (;;) { + const id = [...random(new Uint8Array(6))].map(byte => byte.toString(16).padStart(2, '0')).join(''); + if (!used.has(id)) return id; + } +} + +/** The editor's note for a saved entry's name and text (the mock's short sentences), or ''. */ +export function entryNote(entry, entries) { + const name = String(entry.name ?? '').trim(); + if (!name) return 'Needs a name.'; + if (characters(name) > MAX_NAME_CHARS) return `The name must be at most ${MAX_NAME_CHARS} characters.`; + if (/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/u.test(name)) return 'The name must not contain a line break.'; + if (fold(name) === fold(BUILTIN_NAME)) return 'The name “Built-in” is reserved.'; + if (entries.some(other => other !== entry && other.id !== BUILTIN_ID && fold(other.name) === fold(name))) { + return `The name “${name}” is already used.`; + } + if (!String(entry.text ?? '').trim()) return 'The text must not be empty.'; + if (characters(entry.text) > MAX_TEXT_CHARS) return `The text must be at most ${MAX_TEXT_CHARS.toLocaleString('en-US')} characters.`; + return ''; +} + +/** + * The server's sentence for the first saved entry it would refuse (Built-in is not counted), else null. The Save row + * shows the server's own refusal; this one holds back Preview tagging review and the batch Review. + */ +export function listProblem(values) { + const entries = savedEntries(values); + for (const [index, entry] of entries.entries()) { + // As the server checks them: a repeated name is the later entry's problem. + const note = entryNote(entry, entries.slice(0, index + 1)); + if (!note) continue; + const n = index + 1; + if (note.startsWith('Needs')) return `Saved instructions: entry ${n} needs a name.`; + if (note.includes('reserved')) return 'Saved instructions: the name “Built-in” is reserved.'; + if (note.includes('already used')) return `Saved instructions: entry ${n} name “${String(entry.name).trim()}” is already used.`; + if (note.includes('line break')) return `Saved instructions: entry ${n} name must not contain a line break.`; + if (note.startsWith('The name')) return `Saved instructions: entry ${n} name must be at most ${MAX_NAME_CHARS} characters.`; + if (note.includes('empty')) return `Saved instructions: entry ${n} text must be non-empty text.`; + return `Saved instructions: entry ${n} text must be at most ${MAX_TEXT_CHARS} characters.`; + } + return null; +} + +function el(tag, text, className) { + const node = document.createElement(tag); + if (text !== undefined) node.textContent = text; + if (className) node.className = className; + return node; +} + +/** A `.field` with a label row (text and a muted count) and its control. */ +function field(id, label, control) { + const wrap = el('div', undefined, 'field'); + const caption = el('label'); + caption.htmlFor = id; + const count = el('span', '', 'small muted'); + caption.append(el('span', label), count); + control.id = id; + wrap.append(caption, control); + return { wrap, count }; +} + +/** + * Build the block and the two default rows; `drafts` is the System draft store. Returns `{nodes, repaint}`: System + * appends `nodes` in order and calls `repaint` whenever the drafts change. + */ +export function savedInstructionsBlock({ scope, drafts, random }) { + const block = el('div', undefined, 'setting-block'); + block.dataset.setting = 'tag-instructions'; + block.dataset.key = INSTRUCTIONS_KEY; + const text = el('div', undefined, 'setting-block-text'); + const label = el('label', INSTRUCTIONS_COPY.label); + label.id = 'instructions-label'; + const description = el('p', `${INSTRUCTIONS_COPY.description} `); + const count = el('span', ''); + count.id = 'instructions-count'; + description.append(count); + text.append(label, description); + + const layout = el('div', undefined, 'instructions-layout'); + const side = el('div', undefined, 'instructions-side'); + const list = el('div', undefined, 'instruction-list'); + list.id = 'instruction-list'; + list.setAttribute('role', 'listbox'); + list.setAttribute('aria-labelledby', 'instructions-label'); + list.tabIndex = 0; + const actions = el('div', undefined, 'row-actions instruction-actions'); + const create = createButton(INSTRUCTIONS_COPY.newLabel); + create.id = 'instruction-new'; + const duplicate = createButton(INSTRUCTIONS_COPY.duplicate); + duplicate.id = 'instruction-duplicate'; + const remove = createButton(INSTRUCTIONS_COPY.delete, 'danger'); + remove.id = 'instruction-delete'; + actions.append(create, duplicate, remove); + const cap = el('p', INSTRUCTIONS_COPY.cap, 'small'); + cap.id = 'instruction-cap'; + cap.hidden = true; + side.append(list, actions, cap); + + const editor = el('div', undefined, 'instruction-editor'); + const nameInput = el('input'); + nameInput.autocomplete = 'off'; + const name = field('instruction-name', INSTRUCTIONS_COPY.name, nameInput); + const textArea = el('textarea'); + textArea.rows = 7; + const body = field('instruction-text', INSTRUCTIONS_COPY.text, textArea); + const note = el('p', '', 'small'); + note.id = 'instruction-note'; + note.setAttribute('role', 'status'); + editor.append(name.wrap, body.wrap, note); + layout.append(side, editor); + block.append(text, layout); + + const selects = ENGINES.map(([engine, , long]) => { + const row = settingRow({ id: `instruction-default-${engine}`, label: INSTRUCTIONS_COPY.defaultRow(long), + description: INSTRUCTIONS_COPY.defaultDescription, control: { type: 'select', options: [[BUILTIN_ID, BUILTIN_NAME]] } }); + const select = row.children[1]; + select.dataset.key = DEFAULTS_KEY; + return { engine, select, row }; + }); + const reset = el('p', '', 'small instruction-reset'); + reset.id = 'instruction-reset'; + reset.setAttribute('role', 'status'); + + let selectedId = BUILTIN_ID; + const values = () => ({ [INSTRUCTIONS_KEY]: drafts.get('host', INSTRUCTIONS_KEY), + [DEFAULTS_KEY]: drafts.get('host', DEFAULTS_KEY), [BUILTIN_TEXT_KEY]: drafts.get('host', BUILTIN_TEXT_KEY) }); + const ready = () => Array.isArray(drafts.get('host', INSTRUCTIONS_KEY)) && drafts.get('host', DEFAULTS_KEY) !== undefined; + const defaults = () => ({ ...(drafts.get('host', DEFAULTS_KEY) ?? {}) }); + const setEntries = next => drafts.set('host', INSTRUCTIONS_KEY, next); + const selected = () => allEntries(values()).find(entry => entry.id === selectedId) ?? allEntries(values())[0]; + + /** Replace the selected saved entry's `key` with `value` (a new list of new objects). */ + function edit(key, value) { + if (selectedId === BUILTIN_ID) return; + setEntries(savedEntries(values()).map(entry => (entry.id === selectedId ? { ...entry, [key]: value } : entry))); + } + + function add(entry) { + const entries = savedEntries(values()); + if (!ready() || entries.length >= MAX_SAVED) return; + selectedId = entry.id; + reset.textContent = ''; + setEntries([...entries, entry]); + } + + function deleteSelected() { + const entries = savedEntries(values()); + const index = entries.findIndex(entry => entry.id === selectedId); + if (index < 0) return; + const gone = entries[index]; + const next = defaults(); + const shorts = []; + for (const [engine, short] of ENGINES) { + if (next[engine] === gone.id) { next[engine] = BUILTIN_ID; shorts.push(short); } + } + const left = entries.filter(entry => entry.id !== gone.id); + selectedId = (left[index] ?? left[index - 1])?.id ?? BUILTIN_ID; + reset.textContent = shorts.length ? INSTRUCTIONS_COPY.reset(shorts) : ''; + if (shorts.length) drafts.set('host', DEFAULTS_KEY, next); + setEntries(left); + } + + function select(id) { + selectedId = id; + repaint(); + } + + function renderList(entries, chosen) { + list.replaceChildren(...entries.map(entry => { + const option = el('div'); + option.id = `instruction-option-${entry.id}`; + option.dataset.id = entry.id; + option.setAttribute('role', 'option'); + option.setAttribute('aria-selected', String(entry.id === selectedId)); + option.append(el('span', entry.name)); + const badges = el('span', undefined, 'badges'); + if (entry.id === BUILTIN_ID) badges.append(el('span', INSTRUCTIONS_COPY.readOnly, 'chip')); + for (const [engine, short] of ENGINES) { + if (chosen[engine] === entry.id) badges.append(el('span', INSTRUCTIONS_COPY.defaultFor(short), 'chip blue')); + } + if (badges.children.length) option.append(badges); + return option; + })); + list.setAttribute('aria-activedescendant', `instruction-option-${selectedId}`); + } + + function repaint() { + // The note a deleted default left goes once the defaults are saved (or set back). + if (!Object.hasOwn(drafts.changes('host'), DEFAULTS_KEY)) reset.textContent = ''; + const loaded = ready(); + for (const control of [create, duplicate, remove, nameInput, textArea, ...selects.map(item => item.select)]) { + control.disabled = !loaded; + } + if (!loaded) return; + const current = values(); + const entries = allEntries(current); + if (!entries.some(entry => entry.id === selectedId)) selectedId = BUILTIN_ID; + const chosen = defaults(); + const saved = savedEntries(current); + renderList(entries, chosen); + const entry = selected(); + const builtin = entry.id === BUILTIN_ID; + // A focused field keeps what the user is typing. + if (document.activeElement !== nameInput) nameInput.value = entry.name; + if (document.activeElement !== textArea) textArea.value = entry.text; + nameInput.readOnly = textArea.readOnly = builtin; + name.count.textContent = builtin ? '' : INSTRUCTIONS_COPY.nameCount(characters(entry.name)); + body.count.textContent = INSTRUCTIONS_COPY.textCount(characters(entry.text)); + const problem = builtin ? '' : entryNote(entry, entries); + note.textContent = builtin ? INSTRUCTIONS_COPY.builtinNote : problem; + note.className = problem ? 'small skip-problem' : 'small'; + const full = saved.length >= MAX_SAVED; + create.disabled = duplicate.disabled = full; + remove.disabled = builtin; + cap.hidden = !full; + count.textContent = INSTRUCTIONS_COPY.count(saved.length); + for (const { engine, select: menu } of selects) { + menu.replaceChildren(...entries.map(item => { + const option = el('option', item.name); + option.value = item.id; + return option; + })); + menu.value = chosen[engine] ?? BUILTIN_ID; + } + } + + scope.listen(create, 'click', () => { + const entries = allEntries(values()); + add({ id: newId(entries, random), name: freeName(INSTRUCTIONS_COPY.newName, entries), text: '' }); + textArea.focus?.(); + }); + scope.listen(duplicate, 'click', () => { + const entries = allEntries(values()); + const source = selected(); + add({ id: newId(entries, random), name: copyName(source.name, entries), text: source.text }); + }); + scope.listen(remove, 'click', deleteSelected); + // One listener for every option, however often the list is rebuilt. + scope.listen(list, 'click', event => { + let node = event.target; + while (node && node !== list && node.getAttribute?.('role') !== 'option') node = node.parentNode; + if (node && node !== list) select(node.dataset.id); + }); + scope.listen(nameInput, 'input', () => edit('name', nameInput.value)); + scope.listen(textArea, 'input', () => edit('text', textArea.value)); + scope.listen(list, 'keydown', event => { + const ids = allEntries(values()).map(entry => entry.id); + const at = ids.indexOf(selectedId); + const next = { ArrowDown: at + 1, ArrowUp: at - 1, Home: 0, End: ids.length - 1 }[event.key]; + if (next === undefined) return; + event.preventDefault?.(); + select(ids[Math.max(0, Math.min(ids.length - 1, next))]); + }); + for (const { engine, select: menu } of selects) { + scope.listen(menu, 'change', () => { + reset.textContent = ''; + drafts.set('host', DEFAULTS_KEY, { ...defaults(), [engine]: menu.value }); + }); + } + return { nodes: [block, ...selects.map(item => item.row), reset], repaint }; +} diff --git a/web/js/workspace/system-tagging.mjs b/web/js/workspace/system-tagging.mjs index a958b22a..3201d9c4 100644 --- a/web/js/workspace/system-tagging.mjs +++ b/web/js/workspace/system-tagging.mjs @@ -10,7 +10,8 @@ * verbatim (they state the shipped defaults); host keys `autotag_existing`, `autotag_general_threshold`, * `autotag_character_threshold`. D133 adds "Suggestions per image" (`autotag_suggestion_limit`, 1–100) after them. * - D202 (#431): the "Skipped tags" block after it (system-skipped-tags.mjs, host key `autotag_skipped_tags`). - * - The `.field` "LLM tag template" (`#tag-template`, `autotag_template`). + * - D203 (#431): "Saved instructions" and the two per-engine default rows (system-instructions.mjs, host keys + * `autotag_instructions` and `autotag_default_instructions`) replace the concept's "LLM tag template" field. * - `.row-actions`: "Preview tagging review" (`ca-primary`) opens the Tag image dialog for the image selected in Images * with the section's current model and values, aria-disabled without one (the D115 sentence) or with an invalid * value (the toast names it). D132: the concept's two simulated job buttons are "Download" (or @@ -37,6 +38,8 @@ import { NO_SELECTION } from './vocabulary.mjs'; import { showWorkspaceToast } from './toast.mjs'; import { navigateTo } from './state.mjs'; import { skippedTagsBlock } from './system-skipped-tags.mjs'; +import { BUILTIN_TEXT_KEY, DEFAULTS_KEY, INSTRUCTIONS_KEY, defaultEntry, listProblem, savedInstructionsBlock } + from './system-instructions.mjs'; export const MODELS_URL = '/prompt_manager/autotag/models'; export const UNLOAD_URL = '/prompt_manager/autotag/unload'; @@ -70,10 +73,10 @@ export const SETTING_KEYS = Object.freeze({ 'threshold-general': 'autotag_general_threshold', 'threshold-character': 'autotag_character_threshold', 'tag-suggestions': 'autotag_suggestion_limit', - 'tag-template': 'autotag_template', }); +/** #431: the values the instruction readers take besides the rows' (D203, D205). */ +const INSTRUCTION_VALUES = Object.freeze([INSTRUCTIONS_KEY, DEFAULTS_KEY, BUILTIN_TEXT_KEY]); const NUMERIC = new Set(['autotag_general_threshold', 'autotag_character_threshold', 'autotag_suggestion_limit']); -const MAX_TEMPLATE_CHARS = 4000; export const STATUS_COPY = Object.freeze({ loading: 'Loading model status…', @@ -93,9 +96,6 @@ export const SETTING_PROBLEMS = Object.freeze({ 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.', - autotag_template_length: `LLM tag template must be at most ${MAX_TEMPLATE_CHARS} characters.`, }); /** Characters as Python's `len` counts them (code points), not UTF-16 units. */ export const characters = text => [...String(text ?? '')].length; @@ -193,20 +193,23 @@ 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); +/** #431 (D205): the drafted default instructions of the drafted language-model engine. */ +export const engineInstructions = values => defaultEntry(values, values.autotag_model_type === 'hf' ? 'hf' : 'gguf'); + /** * The first setting the chosen model would use that the section cannot send, as `{key, message}`, else null: WD14 - * reads both thresholds (0 to 1 in steps of 0.05, #369), 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 saved instructions (#431: the server's + * sentence for the first entry it would refuse); Generate also reads the suggestion count (`limit`). */ export function draftProblem(values, { limit = true } = {}) { const model = values.autotag_model_type; - const keys = CLASSIFIERS.has(model) ? ['autotag_general_threshold', 'autotag_character_threshold'] : ['autotag_template']; + const keys = CLASSIFIERS.has(model) ? ['autotag_general_threshold', 'autotag_character_threshold'] : [INSTRUCTIONS_KEY]; if (limit) keys.push('autotag_suggestion_limit'); for (const key of keys) { const value = values[key]; - if (key === 'autotag_template') { - if (typeof value !== 'string' || value.trim() === '') return { key, message: SETTING_PROBLEMS.autotag_template }; - if (characters(value) > MAX_TEMPLATE_CHARS) return { key, message: SETTING_PROBLEMS.autotag_template_length }; + if (key === INSTRUCTIONS_KEY) { + const message = listProblem(values); + if (message) return { key, message }; continue; } const valid = key === 'autotag_suggestion_limit' ? Number.isInteger(value) && value >= 1 && value <= 100 : unit(value); @@ -222,7 +225,7 @@ export function taggingPreset(values) { const options = CLASSIFIERS.has(model) ? { general_threshold: snapped(values.autotag_general_threshold), character_threshold: snapped(values.autotag_character_threshold) } - : { instructions: values.autotag_template }; + : { instructions: engineInstructions(values).text }; return { model, options: { ...options, suggestion_limit: values.autotag_suggestion_limit } }; } @@ -232,7 +235,7 @@ export function batchBody(values) { const options = CLASSIFIERS.has(model) ? { general_threshold: snapped(values.autotag_general_threshold), character_threshold: snapped(values.autotag_character_threshold) } - : { instructions: values.autotag_template }; + : { instructions: engineInstructions(values).text }; return { model_type: model, existing: values.autotag_existing, options }; } @@ -287,14 +290,20 @@ export function timeText(plan) { return durationText(Math.ceil(seconds / 60)) + (plan.estimate_device === 'gpu' ? TIME_ON_GPU : TIME_ON_CPU); } -/** D134: the review's rows from the server's preview and the drafts it was made with (image counts, #379). */ +/** + * D134: the review's rows from the server's preview and the drafts it was made with (image counts, #379). D205 (#431): + * a language-model engine shows its default instructions by name in an "Instructions" row, and Settings reads "Up to L + * tags per image". + */ export function batchGrid(plan, values) { const id = plan.model?.type ?? values.autotag_model_type; - let settings = CLASSIFIERS.has(id) - ? `General ${values.autotag_general_threshold} · Character ${values.autotag_character_threshold}` - : `Template of ${number(characters(values.autotag_template))} characters`; + const classifier = CLASSIFIERS.has(id); // The saved count the run uses (the preview's), not an unsaved draft. - if (Number.isSafeInteger(plan.suggestion_limit)) settings += ` · up to ${number(plan.suggestion_limit)} tags per image`; + const limit = Number.isSafeInteger(plan.suggestion_limit) ? `up to ${number(plan.suggestion_limit)} tags per image` : ''; + const settings = classifier + ? `General ${values.autotag_general_threshold} · Character ${values.autotag_character_threshold}${limit ? ` · ${limit}` : ''}` + : limit.charAt(0).toUpperCase() + limit.slice(1); + const instructions = classifier ? [] : [['Instructions', defaultEntry(values, id === 'hf' ? 'hf' : 'gguf').name]]; return [ ['Model', `${modelLabel(id, plan.model)} · ${modelSize(id, plan.model)}`], ['Existing tags', existingLabel(plan.existing ?? values.autotag_existing)], @@ -302,6 +311,7 @@ export function batchGrid(plan, values) { ['Already auto-tagged', number(plan.already_tagged)], ['Images to tag', `about ${number(plan.to_tag)}`], ['Time', timeText(plan)], + ...instructions, ['Settings', settings], ['Backup', backupText(plan.backup)], ]; @@ -416,16 +426,9 @@ export function mountTagging(container, { api, activity, scope, drafts, notify = const skipped = skippedTagsBlock({ api, scope, drafts }); surface.append(skipped.node); - const field = el('div', undefined, 'field'); - const templateLabel = el('label', 'LLM tag template'); - templateLabel.htmlFor = 'tag-template'; - const template = el('textarea'); - template.id = 'tag-template'; - template.dataset.setting = 'tag-template'; - template.dataset.key = SETTING_KEYS['tag-template']; - field.append(templateLabel, template); - surface.append(field); - controls.push({ id: 'tag-template', key: SETTING_KEYS['tag-template'], input: template, text: null }); + // D203 (#431): the saved instructions and each language-model engine's default replace the template field. + const instructions = savedInstructionsBlock({ scope, drafts }); + surface.append(...instructions.nodes); for (const { key, input } of controls) { // Text controls draft while typing (the Save row follows at once); selects and numbers on change. @@ -454,12 +457,14 @@ export function mountTagging(container, { api, activity, scope, drafts, notify = // The "could not be read" line under each row, while the server settings could not be read. const notes = new Map(); const image = selected(); - const current = () => Object.fromEntries(Object.values(SETTING_KEYS).map(key => [key, drafts.get('host', key)])); + const current = () => Object.fromEntries([...Object.values(SETTING_KEYS), ...INSTRUCTION_VALUES] + .map(key => [key, drafts.get('host', key)])); const chosen = () => drafts.get('host', 'autotag_model_type'); const known = () => MODEL_IDS.includes(chosen()); const repaint = () => { skipped.repaint(); + instructions.repaint(); const values = current(); const unread = values.autotag_model_type === undefined; const failed = drafts.hostState?.() === 'failed' && unread; diff --git a/web/js/workspace/workspace.css b/web/js/workspace/workspace.css index 67aac066..391afe06 100644 --- a/web/js/workspace/workspace.css +++ b/web/js/workspace/workspace.css @@ -513,15 +513,36 @@ pre.prompt-text { font: inherit; font-size: 12px; line-height: 1.75; margin-bloc .skip-add { max-width: none; /* D202 */ } } +/* D203 (#431): "Saved instructions": the list (Built-in first, its chips) beside the Name/Text editor, stacked under + it below 700px, and the note a deleted default leaves under the two default rows. */ +.instructions-layout { display: grid; grid-template-columns: minmax(200px, 260px) minmax(0, 1fr); gap: 18px; margin-top: 12px; /* D203 */ } +.instruction-list { border: 1px solid var(--line); border-radius: 6px; background: var(--ca-field); max-height: 300px; overflow: auto; /* D203 */ } +.instruction-list [role="option"] { display: flex; flex-direction: column; gap: 4px; padding: 9px 12px; cursor: pointer; border-bottom: 1px solid var(--line); font-size: 12px; /* D203 */ } +.instruction-list [role="option"]:last-child { border-bottom: 0; /* D203 */ } +.instruction-list [role="option"][aria-selected="true"] { background: var(--soft-blue); box-shadow: inset 3px 0 0 var(--ca-accent); /* D203 */ } +.instruction-list .badges { display: flex; flex-wrap: wrap; gap: 4px; /* D203 */ } +.instruction-actions { margin-top: 10px; flex-wrap: wrap; /* D203 */ } +.instruction-editor .field:first-child { margin-top: 0; /* D203 */ } +.instruction-editor input[readonly], .instruction-editor textarea[readonly] { color: var(--ca-muted); /* D203 */ } +.instruction-editor label { display: flex; justify-content: space-between; gap: 8px; /* D203 */ } +.instruction-reset { color: var(--warning); margin: 0 0 14px; /* D203 */ } +.instruction-reset:empty { display: none; /* D203 */ } +@media (max-width: 700px) { + .instructions-layout { grid-template-columns: 1fr; /* D203 */ } +} + /* D132: at phone width the concept's 120px cap cuts every engine label ("WD14 SwinV2 · ~400 MB"); the Tagging model select alone keeps its natural width there (its widest engine label) and, with no room beside the row text, wraps under it. Existing tags (#398) never shrinks for its row text and, past the cap, keeps its natural width beside the text ("Replace after review"), bounded by its row like the engine select. */ .setting-row #tag-existing { flex-shrink: 0; /* D132 */ } +/* D203: the default-instructions selects show their chosen name whole, as Existing tags does (#398). */ +.setting-row #instruction-default-gguf, .setting-row #instruction-default-hf { flex-shrink: 0; /* D203 */ } @media (max-width: 700px) { #workspace-main .setting-row:has(> #tag-model) { flex-wrap: wrap; /* D132 */ } #workspace-main .setting-row #tag-model { flex-shrink: 0; max-width: 100% !important; /* D132 */ } #workspace-main .setting-row #tag-existing { max-width: 100% !important; /* D132 */ } + #workspace-main .setting-row #instruction-default-gguf, #workspace-main .setting-row #instruction-default-hf { max-width: 100% !important; /* D203 */ } } /* D109: System › Diagnostics & logs' added Lines field sits in the concept's toolbar like its Level select, and a value