diff --git a/TESTING.md b/TESTING.md index 9a198fc2..d3b4d057 100644 --- a/TESTING.md +++ b/TESTING.md @@ -5342,6 +5342,75 @@ same as a file the scan could not attribute, so the file stays on disk and in th gallery as unlinked, and its favorite and tags (kept by file) are untouched. Every run prints this. Queue-linked batch images can't be proven after the fact and read `unlink`, so review before applying. Stop ComfyUI first. + +### Foreign workflows (#247): the shared per-file decision + +The output scan decides each file through `utils/file_prompt.py` +(`decide_file_prompt`, the ladder), which the monitor will share for outputs of +workflows without a PromptManager node. It does no file or database I/O. + +- **Save to library (#90).** Before anything else, a file whose PromptManager + node had Save to library off is `not_saved`, and one whose switch is wired + from a node the file cannot show is `not_saved_wired`: nothing is written, no + recorded-run fallback, no Needs review item. A resolved pair whose negative's + node is off stores the positive alone (source `opted-out-negative`). The + #90 graph walk follows only prompt-like inputs, so the ladder also walks + every input upstream of each sampler or guider positive (not negative, model, + clip, latent or image inputs; every PromptManager node of a graph without a + sampler) and checks the text later steps name: a resolved or inspected + positive whose path passes such a node (`side_opted_out`), or a legacy answer + that is such a node's text (resolved, else literal). A switch that is off + makes the file `not_saved`; one wired from a node the graph cannot read makes + it `not_saved_wired`, as #90's direct check does. +- **Order.** The default profile decides first, exactly as before (resolved + pair, A1111, sampler-less). Only a graph it leaves ambiguous or unresolved + reaches the inspector's resolve (`inspection_resolution` in + `utils/prompt_inspector.py`, the inspection profile, then the recorded saver + anchor of #386; `name_anchor=False`: the file-name anchor never decides a + link; the scan's legacy floor may still link the file, as before #247, and a + file nothing links is queued with the anchor's choice per #390), and only that positive's `storage_text` is stored (never + prepend or append text, never `recorded_texts`). Then the #170 browser parser + (`graph_prompt_text`), the legacy floor, then the recorded run's stored + prompt. The legacy answer is refused when a PromptManager-family node has a + prepend or append part and the answer is not that node's own `text` (the + parser joins the parts, so the stored text cannot be told from it). History is asked once per file and only at that step; the scan's + batch lookup stays lazy and makes one `recorded_runs` pass for the stored + prompt and the saver (`_BatchHistory`). +- **Policies.** `SCAN` keeps the legacy floor for every undecided file. + `MONITOR` waits for history inside the output folder (`history`) and stores + nothing for a file the inspector calls ambiguous; an untraceable or + malformed one still gets the legacy floor. +- **Junk.** `junk_text` refuses `blank`, `number` (the #252 `_numeric` rule), + `lora-only` (only `` tags and separators) and `model-file` (one + token ending in a model extension) at whichever step found the text and + nothing later is tried; the file reports `found`, except at the undecided + graph step, which keeps its own status (an ambiguous file is still queued). +- **One prompt per file.** `database/scanned_prompts.py` stores a positive in + one `BEGIN IMMEDIATE` transaction: a file already linked to another prompt + (either spelling, `file_link_rows`/`link_conflict`, the pair ingester's rule) + is `conflict` and nothing is written; a failed link rolls the new prompt back + (`found`); two writers of one text store one prompt. A recorded run's stored + prompt links the same way (`link_stored_prompt_or_raise`). A rescan therefore + never adds a second link to a file an older scan linked to the legacy answer; + the row reads "Kept: already linked to a different prompt". Link rows are + matched under the path and its `realpath` spelling (`file_link_rows`), so a + file the scan stored through the resolved folder is found when it arrives + through a symlinked or junction folder. The reverse order (a row stored under + the watcher's raw spelling, then a scan whose root resolves differently) is + matched only when that raw spelling equals the realpath. Every scan write + runs through `survive_library_switch`, so a library move reruns it. +- **Marks.** Every prompt ingestion creates, pair components included, carries + category `scanned`, tag `auto-scanned` and the note `Auto-scanned from + ` (`SCAN_MARKS`); a reused prompt keeps its fields. Run counts are + never touched. + +Tests: `tests/test_file_prompt.py` (one test per ladder step and junk class), +`tests/test_scanned_prompts.py` (store, conflict, rollback, concurrency with +`FirstWriterHold`, pair marks), `tests/test_pair_scan.py` `TestScanDecision`, +`tests/test_run_attribution.py` (dataset branches linked by the recorded +saver, a legacy-linked file left as it is, `_BatchHistory.recorded`) and +`tests/test_recorded_saver_anchor.py` `TestInspectionResolution`. + ## Linked prompt text (#343) A link drives `text` on every PromptManager node, and `negative_text` on the Pair, diff --git a/database/pair_ingestion.py b/database/pair_ingestion.py index 50011947..bfca31c1 100644 --- a/database/pair_ingestion.py +++ b/database/pair_ingestion.py @@ -1,7 +1,14 @@ -"""Atomic metadata enrichment for a single known generation context.""" +"""Atomic metadata enrichment for a single known generation context. + +Also the helpers both ingestion store paths share (#247): the file's link +rows and the one-prompt-per-file rule (``file_link_rows``, +``link_conflict``) and the marks a prompt created by ingestion carries +(``Marks``, ``mark_created``). +""" import os -from typing import Any, Dict +from dataclasses import dataclass +from typing import Any, Dict, List, Optional, Tuple from .models import normalize_image_path from .operations import _coerce_file_info @@ -37,16 +44,85 @@ def blank_positive(resolution: Dict[str, Any]) -> bool: return not (isinstance(text, str) and text.strip()) +@dataclass(frozen=True) +class Marks: + """What a prompt ingestion creates carries, so it can be found and removed. + + ``note`` is formatted with ``name``, the file's name. + """ + + category: str + tags: Tuple[str, ...] + note: str + + +SCAN_MARKS = Marks("scanned", ("auto-scanned",), "Auto-scanned from {name}") +RUN_MARKS = Marks("scanned", ("auto-scanned",), "Saved from a workflow run: {name}") + + +def mark_created(conn: Any, db: Any, prompt_id: int, marks: Marks, name: str) -> None: + """Apply ``marks`` to a prompt row the caller just created on ``conn``.""" + conn.execute( + "UPDATE prompts SET category = ?, notes = ? WHERE id = ?", + (marks.category, marks.note.format(name=name), prompt_id), + ) + db._sync_prompt_tags(conn, prompt_id, list(marks.tags)) + + +def file_link_rows(conn: Any, image_path: str) -> List[Any]: + """The file's link rows, each with its prompt's hash. + + Matched under the path and its resolved spelling (an output folder + reached through a symlink or junction), each normalized and raw; + deduplicated by row id. + """ + rows: Dict[Any, Any] = {} + for spelling in dict.fromkeys((image_path, _resolved(image_path))): + for row in conn.execute( + "SELECT gi.*,p.hash AS positive_hash FROM generated_images gi " + "JOIN prompts p ON p.id=gi.prompt_id " + "WHERE gi.file_path=? OR gi.image_path=?", + (normalize_image_path(spelling), spelling), + ).fetchall(): + rows.setdefault(row["id"], row) + return list(rows.values()) + + +def _resolved(path: str) -> str: + try: + return os.path.realpath(path) + except (OSError, ValueError): + return path + + +def link_conflict(rows: List[Any], prompt_hash: str) -> bool: + """One prompt per file: more than one row, or a row of another prompt.""" + return len(rows) > 1 or bool(rows and rows[0]["positive_hash"] != prompt_hash) + + def ingest_resolved_pair( - db: Any, image_path: str, metadata: Dict[str, Any], resolution: Dict[str, Any] + db: Any, + image_path: str, + metadata: Dict[str, Any], + resolution: Dict[str, Any], + *, + marks: Optional[Marks] = None, ) -> Dict[str, Any]: + """Store a resolved pair and link the file to it in one transaction. + + With ``marks`` (ingestion from files), each pair component this call + creates carries them; without (the PromptManager node path) components + are created bare, as always. + """ state = resolution.get("state", "unresolved") if state == "resolved" and blank_positive(resolution): state = "unresolved" if state != "resolved": return {"status": state, "pairs_added": 0, "links_added": 0} with db._pair_transaction() as conn: - return _ingest_resolved_pair_in(conn, db, image_path, metadata, resolution) + return _ingest_resolved_pair_in( + conn, db, image_path, metadata, resolution, marks=marks + ) def _conflict() -> Dict[str, Any]: @@ -61,6 +137,7 @@ def _ingest_resolved_pair_in( resolution: Dict[str, Any], *, provenance: Dict[str, Any] | None = None, + marks: Optional[Marks] = None, ) -> Dict[str, Any]: """``ingest_resolved_pair`` inside the caller's open ``_pair_transaction``. @@ -69,7 +146,8 @@ def _ingest_resolved_pair_in( negative is a ``conflict`` and nothing is written. The caller owns the transaction, so its own later failure rolls the link back too. ``provenance`` replaces the link's default resolver provenance (Needs - review records the user's selection there). + review records the user's selection there). ``marks`` as in + ``ingest_resolved_pair``. """ positive, negative = resolution["positive"], resolution["negative"] positive_text = positive.get("storage_text", positive["text"]) @@ -82,15 +160,8 @@ def _ingest_resolved_pair_in( "negative": negative, } encoded = dumps_strict(provenance, sort_keys=True, separators=(",", ":")) - images = conn.execute( - "SELECT gi.*,p.hash AS positive_hash FROM generated_images gi " - "JOIN prompts p ON p.id=gi.prompt_id " - "WHERE gi.file_path=? OR gi.image_path=?", - (normalized, image_path), - ).fetchall() - if len(images) > 1 or ( - images and images[0]["positive_hash"] != generate_prompt_hash(positive_text) - ): + images = file_link_rows(conn, image_path) + if link_conflict(images, generate_prompt_hash(positive_text)): return _conflict() existing = images[0] if images else None if existing: @@ -115,7 +186,12 @@ def _ingest_resolved_pair_in( generate_prompt_hash(negative_text) if negative_text.strip() else None, ), ).fetchone() - pair_id = db._save_pair(conn, positive_text, negative_text) + if marks is None: + pair_id = db._save_pair(conn, positive_text, negative_text) + else: + pair_id = _save_marked_pair( + conn, db, positive_text, negative_text, marks, os.path.basename(image_path) + ) pair = conn.execute("SELECT * FROM prompt_pairs WHERE id=?", (pair_id,)).fetchone() pairs_added = int(prior_pair is None) file_size, width, height, image_format = _coerce_file_info(metadata) @@ -197,6 +273,26 @@ def _ingest_resolved_pair_in( } +def _save_marked_pair( + conn: Any, + db: Any, + positive_text: str, + negative_text: str, + marks: Marks, + name: str, +) -> int: + """``_save_pair`` that marks each component it creates.""" + texts = [positive_text] + ([negative_text] if negative_text.strip() else []) + ids = [] + for text in texts: + prompt_id, created = db._pair_component_outcome(conn, text) + if created: + mark_created(conn, db, prompt_id, marks, name) + ids.append(prompt_id) + negative_id = ids[1] if len(ids) > 1 else None + return db._save_pair_components(conn, ids[0], negative_id)[0] + + def _document_form(conn: Any, values: Dict[str, Any]) -> Dict[str, Any]: """``values`` with the workflow and prompt graph texts stored once. diff --git a/database/run_attribution.py b/database/run_attribution.py index 50a671db..6a2c8f2c 100644 --- a/database/run_attribution.py +++ b/database/run_attribution.py @@ -164,7 +164,7 @@ def _runs_by_location(db: Any, names: List[str]) -> Optional[Dict[tuple, dict]]: return found -def _recorded_runs( +def recorded_runs( db: Any, image_paths: Iterable[str], output_root: Optional[str], @@ -201,7 +201,8 @@ def _recorded_runs( return attributed -def _named_prompt(db: Any, run_id: str) -> Optional[int]: +def named_prompt(db: Any, run_id: str) -> Optional[int]: + """The one stored prompt every positive context of ``run_id`` names, else None.""" contexts = db.get_execution_contexts(run_id).get("contexts") or [] named = { context.get("positive_prompt_id") @@ -228,11 +229,11 @@ def stored_run_prompts( paths = list(dict.fromkeys(path for path in image_paths if path)) result: Dict[str, Optional[int]] = dict.fromkeys(paths) prompts: Dict[str, Optional[int]] = {} - for path, (run_id, _nodes) in _recorded_runs( + for path, (run_id, _nodes) in recorded_runs( db, paths, output_root, path_style ).items(): if run_id not in prompts: - prompts[run_id] = _named_prompt(db, run_id) + prompts[run_id] = named_prompt(db, run_id) result[path] = prompts[run_id] return result @@ -275,7 +276,7 @@ def recorded_output( ) -> Optional[RecordedOutput]: """The recorded run facts for ``image_path``, else None (#386). - The file must be attributed to one run (``_recorded_runs``: one run + The file must be attributed to one run (``recorded_runs``: one run reported its location and it was written during that run). Read-only; it never links anything. """ @@ -286,7 +287,7 @@ def recorded_output( if own_transaction: # outputs and texts from one state of history conn.execute("BEGIN") try: - found = _recorded_runs(db, [image_path], output_root, path_style) + found = recorded_runs(db, [image_path], output_root, path_style) if image_path not in found: return None run_id, nodes = found[image_path] diff --git a/database/scanned_prompts.py b/database/scanned_prompts.py new file mode 100644 index 00000000..4cfc611d --- /dev/null +++ b/database/scanned_prompts.py @@ -0,0 +1,87 @@ +"""Link an output file to the prompt of one text, creating it when new (#247). + +The store step of ingestion from file metadata when the decision is a +positive text (``utils.file_prompt``). The prompt insert and the link run in +one ``BEGIN IMMEDIATE`` transaction: two writers never store one text twice, +a failed link leaves no new prompt behind, and a file stays linked to one +prompt (a file already linked to another prompt is a ``conflict``). +""" + +import logging +import os +from typing import Any, Dict, Optional, Tuple + +from .pair_ingestion import Marks, file_link_rows, link_conflict, mark_created + +try: + from ..utils.hashing import generate_prompt_hash +except ImportError: + from utils.hashing import generate_prompt_hash + +_LOGGER = logging.getLogger("prompt_manager.database.scanned_prompts") + + +def store_scanned_prompt_or_raise( + db: Any, path: str, canonical: Dict[str, Any], text: str, marks: Marks +) -> Tuple[str, Optional[int]]: + """``(status, prompt id)`` after linking ``path`` to ``text``'s prompt. + + ``added`` (prompt created, carrying ``marks``), ``linked`` (stored text + reused), ``unchanged`` (already linked to it) or ``conflict`` (linked to + another prompt; nothing written, id None). Database errors are raised + and roll the whole store back, so a caller can run it again. + """ + with db._pair_transaction() as conn: + rows = file_link_rows(conn, path) + if link_conflict(rows, generate_prompt_hash(text)): + return "conflict", None + if rows: + return "unchanged", rows[0]["prompt_id"] + prompt_id, created = db._pair_component_outcome(conn, text) + if created: + mark_created(conn, db, prompt_id, marks, os.path.basename(path)) + db._insert_image_link(conn, prompt_id, path, canonical) + return ("added" if created else "linked"), prompt_id + + +def store_scanned_prompt( + db: Any, path: str, canonical: Dict[str, Any], text: str, marks: Marks +) -> Tuple[str, Optional[int]]: + """The scan's form: an error is logged and reported as ``("found", None)``. + + A write that failed because Move library switched runs again on the new + library (``survive_library_switch``); the rerun re-checks the link rows. + """ + try: + return db.survive_library_switch( + lambda: store_scanned_prompt_or_raise(db, path, canonical, text, marks) + ) + except Exception as error: + _LOGGER.error( + f"Failed to store the prompt of {os.path.basename(path)}: {error}" + ) + return "found", None + + +def link_stored_prompt_or_raise( + db: Any, path: str, canonical: Dict[str, Any], prompt_id: int +) -> Optional[str]: + """Link ``path`` to the stored prompt ``prompt_id`` (recorded history, #342). + + ``linked``, ``unchanged`` (already linked to it), ``conflict`` (linked to + another prompt; nothing written) or None when the prompt is gone. One + ``BEGIN IMMEDIATE`` transaction; database errors are raised. + """ + with db._pair_transaction() as conn: + rows = file_link_rows(conn, path) + if len(rows) > 1 or (rows and rows[0]["prompt_id"] != prompt_id): + return "conflict" + if rows: + return "unchanged" + exists = conn.execute( + "SELECT 1 FROM prompts WHERE id = ?", (prompt_id,) + ).fetchone() + if exists is None: + return None + db._insert_image_link(conn, prompt_id, path, canonical) + return "linked" diff --git a/py/api/__init__.py b/py/api/__init__.py index 76fd0bb1..bd6ce9af 100644 --- a/py/api/__init__.py +++ b/py/api/__init__.py @@ -77,6 +77,7 @@ from .image_categories import ImageCategoryRoutesMixin try: from ...database.operations import PromptDatabase + from ...utils.file_prompt import parse_comfyui_prompt from ...utils.logging_config import get_logger from ...utils.video_metadata import is_video_path, read_video_metadata except ImportError: @@ -86,6 +87,7 @@ except ImportError: 0, os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) ) from database.operations import PromptDatabase + from utils.file_prompt import parse_comfyui_prompt from utils.logging_config import get_logger from utils.video_metadata import is_video_path, read_video_metadata @@ -1031,85 +1033,4 @@ class PromptManagerAPI( def _parse_comfyui_prompt(self, metadata): """Parse ComfyUI and A1111 prompt data from metadata.""" - result = { - "prompt": None, - "workflow": None, - "parameters": {}, - "positive_prompt": None, - "negative_prompt": None, - } - - # Check for A1111/Forge style parameters first. - # The chunk is one block: a possibly multi-line positive prompt, then a - # "Negative prompt:" section, then a trailing "Steps: ..." settings line. - # Taking lines[0] as the positive prompt truncates any multi-line prompt - # to one line, so split on the settings line instead. - if "parameters" in metadata: - import re as _re - - params = metadata["parameters"] - body = params - settings = "" - _sm = _re.search(r"\nSteps:\s", body) - if _sm: - settings = body[_sm.start() + 1 :].strip() - body = body[: _sm.start()] - negative = None - _nm = _re.search(r"(?im)^\s*negative prompt:\s*", body) - if _nm: - negative = body[_nm.end() :].strip() - body = body[: _nm.start()] - elif body.lower().lstrip().startswith("negative prompt:"): - negative = body.lower().lstrip()[len("negative prompt:") :].strip() - body = "" - result["positive_prompt"] = body.strip() or None - if negative: - result["negative_prompt"] = negative - # Store raw parameters too - result["parameters"]["parameters"] = params - if settings: - for _f in ("Steps", "Sampler", "Schedule type", "CFG scale", "Seed"): - _fm = _re.search( - r"(?i)(?:^|,\s*)" + _re.escape(_f) + r":\s*([^,]+)", settings - ) - if _fm: - result["parameters"][_f.lower().replace(" ", "_")] = _fm.group( - 1 - ).strip() - - # If no A1111 format found, proceed with ComfyUI parsing - if result["positive_prompt"] is None: - # Check for direct prompt field - if "prompt" in metadata: - try: - prompt_data = json.loads(metadata["prompt"]) - result["prompt"] = prompt_data - except json.JSONDecodeError: - result["prompt"] = metadata["prompt"] - - # Check for workflow - if "workflow" in metadata: - try: - workflow_data = json.loads(metadata["workflow"]) - result["workflow"] = workflow_data - except json.JSONDecodeError: - result["workflow"] = metadata["workflow"] - - # Check for other common ComfyUI fields - common_fields = [ - "positive", - "negative", - "steps", - "cfg", - "sampler", - "scheduler", - "seed", - ] - for field in common_fields: - if field in metadata: - try: - result["parameters"][field] = json.loads(metadata[field]) - except json.JSONDecodeError: - result["parameters"][field] = metadata[field] - - return result + return parse_comfyui_prompt(metadata) diff --git a/py/api/admin.py b/py/api/admin.py index e010e3fb..a7b3bac7 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -40,45 +40,51 @@ except ImportError: from utils import exit_signal try: - from ...database.pair_ingestion import blank_positive, ingest_resolved_pair + from ...database.pair_ingestion import SCAN_MARKS, ingest_resolved_pair from ...database.run_attribution import ( + RecordedOutput, host_output_directory, - stored_run_prompt, - stored_run_prompts, + named_prompt, + recorded_runs, ) - from ...utils.generation_params import ( - PLACEHOLDERS, - GenerationParamsError, - extract_generation_params, - parse_page_json, + from ...database.scanned_prompts import ( + link_stored_prompt_or_raise, + store_scanned_prompt, + ) + from ...utils.file_prompt import ( + NEVER, + NOT_SAVED, + NOT_SAVED_WIRED, + PAIR, + Policy, + decide_file_prompt, ) - from ...utils.prompt_graph import metadata_opted_out, side_opted_out - from ...utils.prompt_pair_resolver import resolve_prompt_pair, sampler_less_prompt - from ...utils.review_capture import reviewable except ImportError: - from database.pair_ingestion import blank_positive, ingest_resolved_pair + from database.pair_ingestion import SCAN_MARKS, ingest_resolved_pair from database.run_attribution import ( + RecordedOutput, host_output_directory, - stored_run_prompt, - stored_run_prompts, + named_prompt, + recorded_runs, ) - from utils.generation_params import ( - PLACEHOLDERS, - GenerationParamsError, - extract_generation_params, - parse_page_json, + from database.scanned_prompts import ( + link_stored_prompt_or_raise, + store_scanned_prompt, + ) + from utils.file_prompt import ( + NEVER, + NOT_SAVED, + NOT_SAVED_WIRED, + PAIR, + Policy, + decide_file_prompt, ) - from utils.prompt_graph import metadata_opted_out, side_opted_out - from utils.prompt_pair_resolver import resolve_prompt_pair, sampler_less_prompt - from utils.review_capture import reviewable try: from ...database.library import display_title - from ...utils.hashing import generate_prompt_hash except ImportError: from database.library import display_title - from utils.hashing import generate_prompt_hash IMAGE_EXTENSIONS = (".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".tiff") VIDEO_EXTENSIONS = (".mp4", ".avi", ".mov", ".mkv", ".webm", ".m4v", ".wmv") @@ -258,7 +264,7 @@ SCAN_DETAILS = { "skipped": "Skipped · already linked", "ambiguous": "Needs review: more than one prompt could match", "unresolved": "Needs review: the prompt could not be traced in the workflow", - "conflict": "Needs review: already linked to a different prompt", + "conflict": "Kept: already linked to a different prompt", "failed": "Error: the prompt could not be stored", "error": "Error: the file could not be processed (see the server log)", "found": "Metadata found, but no prompt text to store", @@ -325,22 +331,6 @@ def _file_size(path): return None -def _with_file_size(canonical, media_file): - """Record the file's size in ``file_info`` unless the metadata carries one. - - Stored as ``generated_images.file_size``; the "new" scan scope skips a known - file only while its size is unchanged. - """ - info = canonical.get("file_info") - info = dict(info) if isinstance(info, dict) else {} - if info.get("size") is None: - size = _file_size(media_file) - if size is not None: - info["size"] = size - if info: - canonical["file_info"] = info - - def _scan_roots(output_dirs): """(absolute root, public label, listing index), the most specific first. @@ -596,18 +586,56 @@ class _BatchHistory: """A scan batch's recorded-history lookup, made on the first file needing it. Most files link through their metadata and never ask; the first one that - does triggers one lookup for the whole batch (``_recorded_runs_for``). + does triggers one ``recorded_runs`` pass for the whole batch, from which + both the stored prompt (``get``) and the recorded saver (``recorded``) + of every file are read. A failure is logged and names nothing. """ def __init__(self, api, media_files): self._api = api - self._files = media_files - self._found = None + self._files = [str(path) for path in media_files] + self._runs = None + self._prompts = {} + + def _found(self): + if self._runs is None: + try: + self._runs = recorded_runs( + self._api.db, self._files, host_output_directory(), None + ) + except Exception: + self._api.logger.warning( + "History attribution failed for a scan batch", exc_info=True + ) + self._runs = {} + return self._runs def get(self, path): - if self._found is None: - self._found = self._api._recorded_runs_for(self._files) - return self._found.get(path) + """The stored prompt id history attributes to ``path``, else None.""" + found = self._found().get(path) + if found is None: + return None + run_id = found[0] + if run_id not in self._prompts: + try: + self._prompts[run_id] = named_prompt(self._api.db, run_id) + except Exception: + self._api.logger.warning( + "History attribution failed for a scan batch", exc_info=True + ) + self._prompts[run_id] = None + return self._prompts[run_id] + + def recorded(self, path): + """``RecordedOutput(saver, {})`` for an attributed file, else None. + + ``saver`` is set only when the run reported the file from one node. + """ + found = self._found().get(path) + if found is None: + return None + nodes = found[1] + return RecordedOutput(next(iter(nodes)) if len(nodes) == 1 else None, {}) class AdminRoutesMixin: @@ -1672,24 +1700,6 @@ class AdminRoutesMixin: note(media_file, status) return counts - @staticmethod - def _graph_prompt_text(metadata): - """The positive prompt the browser parser traces from the graphs, or None. - - Records parse as the legacy pages parsed them (NaN reads as null). - """ - records = {} - for key in ("prompt", "workflow"): - value = metadata.get(key) - records[key] = parse_page_json(value) if isinstance(value, str) else value - try: - positive = extract_generation_params(records)["positivePrompt"] - except GenerationParamsError: - return None - if not isinstance(positive, str) or positive == PLACEHOLDERS["positivePrompt"]: - return None - return positive.strip() or None - def _ingest_scanned_file( self, media_file, @@ -1700,148 +1710,81 @@ class AdminRoutesMixin: records=None, history=None, ): - """Enrich exact pairs; retain legacy positive-only import for absent pairs. + """Store the prompt the shared per-file decision names (#247). - Returns the file's scan status, or None without prompt metadata. With - ``detailed``, a stored or linked file returns a dict instead: its - ``status``, the prompt's display ``title`` and, for a resolved pair, - the pair counts. When the resolver finds the file ambiguous (whether - or not the browser parser then links it) and ``ambiguous`` is a list, - ``media_file`` is appended to it; only a file that then returns - ``"ambiguous"`` (nothing linked it) is a Needs review case. An - ambiguous result with nothing to choose (``reviewable`` is False: fewer - than two candidates carry a prompt) that nothing links returns - ``"unresolved"`` instead, and ``media_file`` goes to ``untraced``. - The parsed records of either ambiguous file go to - ``records["canonical"]`` when ``records`` is a dict. + ``utils.file_prompt.decide_file_prompt`` (scan policy) decides; this + stores the result. Returns the file's scan status, or None without + prompt metadata. With ``detailed``, a stored or linked file returns a + dict instead: its ``status``, the prompt's display ``title`` and, for + a resolved pair, the pair counts. When the resolver finds the file + ambiguous (whether or not a later step then links it) and + ``ambiguous`` is a list, ``media_file`` is appended to it; only a file + that then returns ``"ambiguous"`` (nothing linked it) is a Needs + review case. An ambiguous result with nothing to choose + (``reviewable`` is False: fewer than two candidates carry a prompt) + goes to ``untraced`` instead and returns ``"unresolved"`` when nothing + links it. The parsed records of either ambiguous file go to + ``records["canonical"]`` when ``records`` is a dict. ``history`` is + the batch's lazy recorded-history lookup. """ - if not metadata: - # No prompt metadata (--disable-metadata, bare savers): only the - # recorded run can say which prompt made it (#342) - return self._link_recorded_run(media_file, {}, detailed, history) - canonical = dict(metadata) - for key in ("prompt", "workflow"): - if isinstance(canonical.get(key), str): - try: - canonical[key] = json.loads(canonical[key]) - except (ValueError, TypeError): - # Preserve malformed metadata for conservative resolver rejection. - pass - if metadata_opted_out(canonical): - # Save to library was off for the run's prompt: no prompt, link, - # recorded-run fallback or Needs review item (#90) - return "not_saved" - if metadata_opted_out(canonical, unreadable=True): - # The switch is wired from a node the file cannot show: only the - # live run (the node saw the value) can link it, never a scan - return "not_saved_wired" - if not canonical.get("prompt") and not canonical.get("parameters"): - return self._link_recorded_run(media_file, canonical, detailed, history) - _with_file_size(canonical, media_file) - resolution = resolve_prompt_pair(canonical) - if resolution["state"] == "resolved" and side_opted_out( - canonical.get("prompt"), resolution["negative"] - ): - # Only the negative's node is off: store the positive alone (#90) - prompt_text = ( - resolution["positive"] - .get("storage_text", resolution["positive"]["text"]) - .strip() - ) - status = self._store_scanned_prompt(media_file, canonical, prompt_text) - if not detailed or status not in SCAN_TITLED_DETAILS: - return status - return { - "status": status, - "title": self._scan_title(media_file, prompt_text), - } - if blank_positive(resolution): - # Only prepend/append text: no stored prompt can own it (#342) - resolution = {**resolution, "state": "unresolved"} - # Ambiguous without a choice: unresolved unless something links it. - unchoosable = resolution["state"] == "ambiguous" and not reviewable(resolution) - if unchoosable: - if untraced is not None: - untraced.append(media_file) - if records is not None: - records["canonical"] = canonical - elif resolution["state"] == "ambiguous" and ambiguous is not None: - ambiguous.append(media_file) - if records is not None: - records["canonical"] = canonical - if resolution["state"] == "resolved": - result = ingest_resolved_pair( - self.db, str(media_file), canonical, resolution + path = str(media_file) + if history is None: # one file: history is still asked once + history = _BatchHistory(self, [media_file]) + decision = decide_file_prompt( + metadata, + file_name=Path(path).name, + file_size=functools.partial(_file_size, media_file), + in_output=False, # only the monitor policy reads it + lookup=functools.partial(history.recorded, path), + policy=Policy.SCAN, + ) + if decision.kind in (NOT_SAVED, NOT_SAVED_WIRED): + # Save to library off (#90): no write, recorded-run fallback or + # Needs review item + return decision.kind + self._note_review(media_file, decision, ambiguous, untraced, records) + if decision.kind == NEVER: + if decision.try_recorded_run: + linked = self._link_recorded_run( + media_file, decision.canonical, detailed, history + ) + if linked is not None: + return linked + return decision.unlinked_status + if decision.kind == PAIR: + # A rerun after a library move re-checks the link rows. + result = self.db.survive_library_switch( + lambda: ingest_resolved_pair( + self.db, + path, + decision.canonical, + decision.resolution, + marks=SCAN_MARKS, + ) ) if not detailed: return result["status"] - return { - **result, - "title": self._scan_title(media_file, resolution["positive"]["text"]), - } - if canonical.get("prompt") and resolution["state"] in ( - "ambiguous", - "unresolved", - ): - # The resolver could not decide: link the prompt the inspector and the - # legacy pages show for this file (the browser parser, in Python). - # Graphs without a recognized sampler keep the legacy extractor below. - prompt_text = self._graph_prompt_text(metadata) - if not prompt_text: - linked = self._link_recorded_run( - media_file, canonical, detailed, history - ) - if linked is not None: - return linked - return "unresolved" if unchoosable else resolution["state"] - elif resolution["state"] == "ambiguous": - return "unresolved" if unchoosable else "ambiguous" - elif resolution["state"] == "unresolved" and not isinstance( - canonical.get("parameters"), str - ): - return resolution["state"] - elif canonical.get("prompt"): - # No node takes a positive or negative input (a guider or wrapper - # graph): the one wired text, never a guess (#252). - found = sampler_less_prompt(canonical["prompt"]) - prompt_text = found["text"] - if prompt_text is None and isinstance(canonical.get("parameters"), str): - # A metadata saver also wrote A1111 text: its positive prompt. - parsed = self._parse_comfyui_prompt(metadata)["positive_prompt"] - prompt_text = parsed.strip() if parsed and parsed.strip() else None - if prompt_text is None: - linked = self._link_recorded_run( - media_file, canonical, detailed, history - ) - if linked is not None: - return linked - return "unresolved" if found["state"] == "ambiguous" else "found" - else: - prompt_text = self._parse_comfyui_prompt(metadata)["positive_prompt"] - if not (prompt_text and prompt_text.strip()): - return "found" - prompt_text = prompt_text.strip() - - status = self._store_scanned_prompt(media_file, canonical, prompt_text) + title_text = decision.resolution["positive"]["text"] + return {**result, "title": self._scan_title(media_file, title_text)} + status, _prompt_id = store_scanned_prompt( + self.db, path, decision.canonical, decision.text, SCAN_MARKS + ) if not detailed or status not in SCAN_TITLED_DETAILS: return status - return {"status": status, "title": self._scan_title(media_file, prompt_text)} + return {"status": status, "title": self._scan_title(media_file, decision.text)} - def _recorded_runs_for(self, media_files): - """``{path: prompt id or None}`` from recorded history for a scan batch. - - One output lookup per chunk of names (``stored_run_prompts``), so a - batch never queries history once per file. An empty dict on failure. - """ - try: - return stored_run_prompts( - self.db, [str(path) for path in media_files], host_output_directory() - ) - except Exception: - self.logger.warning( - "History attribution failed for a scan batch", exc_info=True - ) - return {} + @staticmethod + def _note_review(media_file, decision, ambiguous, untraced, records): + """Hand an ambiguous file to the batch's Needs review lists.""" + if decision.review == "untraced": + if untraced is not None: + untraced.append(media_file) + if records is not None: + records["canonical"] = decision.canonical + elif decision.review == "ambiguous" and ambiguous is not None: + ambiguous.append(media_file) + if records is not None: + records["canonical"] = decision.canonical def _link_recorded_run(self, media_file, canonical, detailed=False, history=None): """Link a file recorded history attributes to one stored prompt (#342). @@ -1850,31 +1793,25 @@ class AdminRoutesMixin: ``{"status", "title"}``), or None when history names no single stored prompt for the file (``database.run_attribution``). Never creates a prompt. Works without any prompt metadata in the file. ``history`` is - the batch's precomputed lookup; without it the file is looked up alone. + the batch's lookup (``_BatchHistory``). A file already linked to + another prompt is a ``conflict``: one prompt per file. """ - if history is not None: - prompt_id = history.get(str(media_file)) - else: - try: - prompt_id = stored_run_prompt( - self.db, str(media_file), host_output_directory() - ) - except Exception: - self.logger.warning( - f"History attribution failed for {media_file.name}", - exc_info=True, - ) - return None + if history is None: + history = _BatchHistory(self, [media_file]) + prompt_id = history.get(str(media_file)) if prompt_id is None: return None try: - image_id = self.db.link_image_to_prompt( - prompt_id, str(media_file), metadata=canonical + status = self.db.survive_library_switch( + lambda: link_stored_prompt_or_raise( + self.db, str(media_file), canonical, prompt_id + ) ) except Exception as e: self.logger.error(f"Failed to link {media_file.name} from history: {e}") return "found" - status = "linked" if image_id else "unchanged" + if status is None: + return None if not detailed or status not in SCAN_TITLED_DETAILS: return status prompt = self.db.get_prompt_by_id(prompt_id) or {} @@ -1894,35 +1831,3 @@ class AdminRoutesMixin: except Exception: self.logger.warning("Scan row title could not be read; using its text") return display_title((), text) - - def _store_scanned_prompt(self, media_file, canonical, prompt_text): - """Link ``media_file`` to ``prompt_text``'s prompt, saving it when new.""" - prompt_hash = generate_prompt_hash(prompt_text) - existing = self.db.get_prompt_by_hash(prompt_hash) - if existing: - try: - image_id = self.db.link_image_to_prompt( - existing["id"], str(media_file), metadata=canonical - ) - except Exception as e: - self.logger.error( - f"Failed to link {media_file.name} to existing prompt: {e}" - ) - return "found" - return "linked" if image_id else "unchanged" - - prompt_id = self.db.save_prompt( - prompt_text, - "scanned", - ["auto-scanned"], - None, - f"Auto-scanned from {media_file.name}", - prompt_hash, - ) - if not prompt_id: - return "found" - try: - self.db.link_image_to_prompt(prompt_id, str(media_file), metadata=canonical) - except Exception as e: - self.logger.error(f"Failed to link {media_file.name} to new prompt: {e}") - return "added" diff --git a/py/api/annotation_suggest.py b/py/api/annotation_suggest.py index befef53f..9411640b 100644 --- a/py/api/annotation_suggest.py +++ b/py/api/annotation_suggest.py @@ -86,6 +86,23 @@ def unavailable_message(model_name: str) -> str: return f"{model_name} is not downloaded. {UNAVAILABLE_HINT}" +def model_refusal(status: dict, model_type: str, model_name: str): + """Why *model_type* cannot run, from ``get_models_status()``, else None. + + Not downloaded first (``unavailable_message``), then a backend package + that cannot be imported (#424). Both are a 409 ``model-unavailable``. + """ + entry = status.get(model_type) or {} + if not entry.get("downloaded"): + return unavailable_message(model_name) + missing = entry.get("missing_packages") or [] + if missing: + from ..autotag import missing_packages_message + + return missing_packages_message(model_type, [str(name) for name in missing]) + return None + + def run_suggestions(service, capture_path: str, model_type: str, options: dict): """Blocking: load (or swap to) *model_type*, then suggest for the capture. diff --git a/py/api/annotations.py b/py/api/annotations.py index d9a27a0f..7f3ab943 100644 --- a/py/api/annotations.py +++ b/py/api/annotations.py @@ -21,9 +21,9 @@ from aiohttp import web from .annotation_suggest import ( SUGGEST_FIELDS, OptionError, + model_refusal, run_suggestions, suggestion_rows, - unavailable_message, validate_options, ) from .autotag_routes import model_busy_response @@ -595,7 +595,7 @@ class AnnotationRoutesMixin: it, including a legacy batch), never downloads (``model-unavailable``) and never writes. """ - from ..autotag import MODELS, get_autotag_service + from ..autotag import MODELS, MissingBackendPackages, get_autotag_service from ..config import MAX_SUGGESTION_LIMIT, AutotagConfig data, error = await self._annotation_body(request, SUGGEST_FIELDS) @@ -633,9 +633,10 @@ class AnnotationRoutesMixin: return error service = get_autotag_service() status = await self._run_in_executor(service.get_models_status) - if not status.get(model_type, {}).get("downloaded"): - name = MODELS.get(model_type, {}).get("name", model_type) - return _coded("model-unavailable", unavailable_message(name)) + name = MODELS.get(model_type, {}).get("name", model_type) + refusal = model_refusal(status, model_type, name) + if refusal is not None: + return _coded("model-unavailable", refusal) owner = model_gate.new_owner("image-annotations") if not model_gate.try_acquire(owner): return model_busy_response() @@ -654,6 +655,9 @@ class AnnotationRoutesMixin: options, on_skipped=functools.partial(store.release_use, token), ) + except MissingBackendPackages as exc: + # A package gone since the status read (load_model logged it). + return _coded("model-unavailable", str(exc)) except Exception: self.logger.exception("Tag suggestion failed") return _error( diff --git a/py/api/autotag_routes.py b/py/api/autotag_routes.py index 7314f0f7..edc8ca82 100644 --- a/py/api/autotag_routes.py +++ b/py/api/autotag_routes.py @@ -102,6 +102,17 @@ def size_label(size: str) -> str: return f"{match.group(1)} {match.group(2)}" if match else str(size or "") +def package_entry(flags: dict) -> dict: + """The public package fields of one status entry (#424): the pip names + that cannot be imported, the install command and note (strings or null).""" + missing = flags.get("missing_packages") or [] + text = { + key: flags[key] if isinstance(flags.get(key), str) else None + for key in ("install_command", "install_note") + } + return {"missing_packages": [str(name) for name in missing], **text} + + def public_models(status: dict, loaded_type: Optional[str]) -> dict: """Each engine model's public entry: names and sizes from ``MODELS``, the flags from ``status``; never a path (``model_path`` is dropped).""" @@ -122,6 +133,7 @@ def public_models(status: dict, loaded_type: Optional[str]) -> dict: for key in ("model_exists", "mmproj_exists"): if key in flags: entry[key] = bool(flags[key]) + entry.update(package_entry(flags)) models[model_type] = entry return models @@ -750,6 +762,7 @@ class AutotagRoutesMixin: "downloaded": bool( (status.get(model_type) or {}).get("downloaded") ), + **package_entry(status.get(model_type) or {}), }, "existing": existing, "review_required": existing == "replace_after_review", @@ -1073,8 +1086,8 @@ class AutotagRoutesMixin: return self.db.current_image_tags({locator: fingerprint}).get(locator, []) async def _suggest_single(self, target, model_type, options, base): - from ..autotag import MODELS, get_autotag_service - from .annotation_suggest import unavailable_message + from ..autotag import MODELS, MissingBackendPackages, get_autotag_service + from .annotation_suggest import model_refusal from .annotations import _pinned_suggestions, _settled, _submit try: @@ -1084,8 +1097,8 @@ class AutotagRoutesMixin: service = get_autotag_service() status = await self._run_in_executor(service.get_models_status) - if not (status.get(model_type) or {}).get("downloaded"): - message = unavailable_message(MODELS[model_type]["name"]) + message = model_refusal(status, model_type, MODELS[model_type]["name"]) + if message is not None: return self._coded_error(409, "model-unavailable", message) owner = model_gate.new_owner("legacy-single") if not model_gate.try_acquire(owner): @@ -1131,6 +1144,9 @@ class AutotagRoutesMixin: options, on_skipped=functools.partial(store.release_use, capture.token), ) + except MissingBackendPackages as exc: + # A package gone since the status read (load_model logged it). + return self._coded_error(409, "model-unavailable", str(exc)) finally: store.release(capture.token) finally: diff --git a/py/api/image_tag_jobs.py b/py/api/image_tag_jobs.py index 61c79422..5d1f9547 100644 --- a/py/api/image_tag_jobs.py +++ b/py/api/image_tag_jobs.py @@ -42,7 +42,7 @@ from .. import model_gate from .admin import _plural, _time_left from .annotation_suggest import ( OptionError, - unavailable_message, + model_refusal, validate_options, ) from .annotations import ( @@ -512,10 +512,8 @@ class ImageTagJobsMixin: ) ) label = MODELS[model_type]["label"] - if not (status.get(model_type) or {}).get("downloaded"): - message = ( - f"{label} is not downloaded. Download it in System › Auto-tagging." - ) + message = model_refusal(status, model_type, label) + if message is not None: return None, _refusal(409, "model-unavailable", message) if backup_on and short is not None: return None, _refusal(507, "insufficient-space", short_space_message(short)) @@ -792,13 +790,13 @@ class ImageTagJobsMixin: async def _prepare_tagging_model(self, job, spec: _Spec, owner): """The loaded service, or None (logged) when the job cannot run.""" - from ..autotag import MODELS, get_autotag_service + from ..autotag import MODELS, MissingBackendPackages, get_autotag_service service = get_autotag_service() status = await self._run_in_executor(service.get_models_status) - if not status.get(spec.model_type, {}).get("downloaded"): - name = MODELS.get(spec.model_type, {}).get("name", spec.model_type) - message = unavailable_message(name) + name = MODELS.get(spec.model_type, {}).get("name", spec.model_type) + message = model_refusal(status, spec.model_type, name) + if message is not None: job.update(status_line=message) job.log_line(f"! {message}") return None @@ -810,6 +808,11 @@ class ImageTagJobsMixin: spec.model_type, spec.options["use_gpu"], ) + except MissingBackendPackages as exc: + # A package gone since the status read (load_model logged it). + job.update(status_line=str(exc)) + job.log_line(f"! {exc}") + return None except Exception: self.logger.exception("The auto-tag job could not load its model") job.update(status_line=INFERENCE_FAILED) diff --git a/py/api/review_recording.py b/py/api/review_recording.py index 141cf14a..2716c77b 100644 --- a/py/api/review_recording.py +++ b/py/api/review_recording.py @@ -3,7 +3,7 @@ The rule (maintainer test of #284/#288): a file is queued only when its final outcome is ambiguous and unlinked. The strict resolver must find it ``ambiguous``, the scan's legacy fallback (the browser parser, -``_graph_prompt_text``) must not link it, and no saved row may link it +``graph_prompt_text``) must not link it, and no saved row may link it already. A file the fallback links, an exact pair the ingester stored and an already linked file are never queued. Neither is an ambiguous file with nothing to choose: fewer than two of its candidates carry a prompt diff --git a/py/autotag.py b/py/autotag.py index d002f626..727af79d 100644 --- a/py/autotag.py +++ b/py/autotag.py @@ -7,6 +7,7 @@ Refactored from standalone_tagger.py for integration with PromptManager API. import errno import gc +import importlib.util import inspect import os import sys @@ -155,6 +156,91 @@ MODELS = { }, } +# The Python modules each backend imports when it loads, with their pip names +# (#424). None of them is a PromptManager requirement: they are optional, and +# llama-cpp-python needs a build for the user's GPU. Nothing is ever installed. +BACKEND_PACKAGES: Dict[str, Tuple[Tuple[str, str], ...]] = { + "gguf": (("llama_cpp", "llama-cpp-python"),), + # The default 8-bit load needs bitsandbytes, and device_map needs accelerate. + "hf": ( + ("torch", "torch"), + ("transformers", "transformers"), + ("accelerate", "accelerate"), + ("bitsandbytes", "bitsandbytes"), + ), + "wd14-swinv2": (("onnxruntime", "onnxruntime"),), + "wd14-vit": (("onnxruntime", "onnxruntime"),), +} +_INSTALL_NOTES = { + "gguf": ( + 'For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" pip install llama-cpp-python' + ), + "wd14-swinv2": "Use onnxruntime-gpu for CUDA.", + "wd14-vit": "Use onnxruntime-gpu for CUDA.", +} +_PIP_NAMES = { + module: pip for packages in BACKEND_PACKAGES.values() for module, pip in packages +} + + +def _importable(module: str) -> bool: + """Whether *module* (a top-level name) can be imported; imports nothing.""" + if sys.modules.get(module) is not None: + return True + try: + return importlib.util.find_spec(module) is not None + except (ImportError, ValueError, OSError): + return False + + +def missing_packages(model_type: str) -> List[str]: + """The pip names of the packages *model_type* needs and cannot import.""" + return [ + pip + for module, pip in BACKEND_PACKAGES.get(model_type, ()) + if not _importable(module) + ] + + +def package_fields(model_type: str, missing: List[str]) -> Dict[str, Any]: + """The status fields of *missing*: pip names, install command and note.""" + missing = list(missing) + return { + "missing_packages": missing, + "install_command": "pip install " + " ".join(missing) if missing else None, + "install_note": _INSTALL_NOTES.get(model_type) if missing else None, + } + + +def _joined(names: List[str]) -> str: + if len(names) <= 1: + return "".join(names) + return ", ".join(names[:-1]) + " and " + names[-1] + + +def missing_packages_message(model_type: str, missing: List[str]) -> str: + """The one sentence every refusal of a model with missing packages uses.""" + label = MODELS.get(model_type, {}).get("label", model_type) + fields = package_fields(model_type, missing) + pronoun = "it" if len(missing) == 1 else "them" + message = ( + f"{label} needs {_joined(missing)}. Install {pronoun} into ComfyUI's " + f"Python: {fields['install_command']}." + ) + if fields["install_note"]: + message += " " + fields["install_note"] + return message + + +class MissingBackendPackages(RuntimeError): + """A model whose backend packages cannot be imported (#424).""" + + def __init__(self, model_type: str, missing: List[str]): + self.model_type = model_type + self.missing = list(missing) + super().__init__(missing_packages_message(model_type, self.missing)) + + # Default prompts DEFAULT_PROMPT = DEFAULT_TAG_TEMPLATE @@ -167,6 +253,10 @@ SYSTEM_PROMPT = ( ) +def _none(_model_type: str) -> List[str]: + return [] + + def _load_rgb(image_path: str) -> Image.Image: """Decode the image as RGB and close the file before inference. @@ -203,6 +293,7 @@ class AutoTagService: self, models_dir: Optional[Path] = None, model_loader: Optional[ModelLoader] = None, + package_probe: Optional[Callable[[str], List[str]]] = None, ): """Initialize the AutoTag service. @@ -213,9 +304,16 @@ class AutoTagService: ``(kind, backend)`` tuple used by ``generate_tags``. Defaults to ``_load_model``, which imports the ML backends lazily. Tests inject a fake here. + package_probe: Callable ``(model_type)`` returning the pip names + of the backend packages that cannot be imported. + Defaults to ``missing_packages`` with the default + loader; an injected loader brings its own backend. """ self.logger = get_logger("autotag.service") self._model_loader: ModelLoader = model_loader or self._load_model + if package_probe is None: + package_probe = missing_packages if model_loader is None else _none + self._package_probe = package_probe # Determine models directory if models_dir: @@ -320,6 +418,9 @@ class AutoTagService: # Get the actual path (local or cache) model_status["model_path"] = str(self._get_hf_model_path()) + model_status.update( + package_fields(model_type, self._package_probe(model_type)) + ) status[model_type] = model_status return status @@ -455,13 +556,9 @@ class AutoTagService: def _load_wd14_tagger(self, model_type: str, use_gpu: bool = True): """Load WD14 ONNX model and tag list.""" - try: - import onnxruntime as ort - except ImportError: - raise RuntimeError( - "onnxruntime is required for WD14 models. " - "Install with: pip install onnxruntime (or onnxruntime-gpu for CUDA)" - ) + # A missing onnxruntime is refused by load_model before this runs + # (#424); a ModuleNotFoundError here becomes the same refusal. + import onnxruntime as ort import csv config = MODELS[model_type] @@ -694,6 +791,7 @@ class AutoTagService: Raises: ValueError: If model_type is invalid RuntimeError: If model not downloaded or loading fails + MissingBackendPackages: If its backend packages cannot be imported """ if model_type not in MODELS: raise ValueError(f"Invalid model type: {model_type}") @@ -705,6 +803,11 @@ class AutoTagService: status = self.get_models_status() if not status[model_type]["downloaded"]: raise RuntimeError(f"Model {model_type} not downloaded") + missing = status[model_type].get("missing_packages") or [] + if missing: + refusal = MissingBackendPackages(model_type, missing) + self.logger.warning(str(refusal)) + raise refusal try: self._tagger = self._model_loader(model_type, use_gpu) @@ -712,6 +815,17 @@ class AutoTagService: self.logger.info(f"Model {model_type} loaded successfully") return True + except ModuleNotFoundError as e: + # A package find_spec found but that does not import (a broken + # install), or a dependency of one: the same refusal, no traceback. + self._tagger = None + self._current_model_type = None + module = (e.name or "").split(".")[0] or str(e) + refusal = MissingBackendPackages( + model_type, [_PIP_NAMES.get(module, module)] + ) + self.logger.warning(str(refusal)) + raise refusal from None except Exception as e: self.logger.error(f"Failed to load model {model_type}: {e}") self._tagger = None diff --git a/tests/browser/server.py b/tests/browser/server.py index eb135b43..da867594 100644 --- a/tests/browser/server.py +++ b/tests/browser/server.py @@ -76,6 +76,8 @@ class TestModel: loaded = None # Models the Tag image dialog workflows mark as not downloaded. unavailable: set = set() + # Pip names per model whose backend packages "cannot be imported" (#424). + missing_packages: dict = {} # Scored names the concept comparisons answer instead of the fixed list # (``/__test__/tagging/suggestions``); None keeps the fixed list. suggestions: list | None = None @@ -83,10 +85,13 @@ class TestModel: models_config: dict = {} def get_models_status(self) -> dict: + from py.autotag import package_fields + return { name: { "downloaded": name not in self.unavailable, "loaded": self.loaded == name, + **package_fields(name, self.missing_packages.get(name, [])), } for name in self.models_config } diff --git a/tests/browser/workspace-system-tagging.spec.mjs b/tests/browser/workspace-system-tagging.spec.mjs index 0d9fa58a..5a5a640a 100644 --- a/tests/browser/workspace-system-tagging.spec.mjs +++ b/tests/browser/workspace-system-tagging.spec.mjs @@ -123,6 +123,32 @@ test('model options are the four real engines with size and the description show await expect(actionOf(page, 'Unload')).toBeDisabled(); }); +const NEEDS_LLAMA = 'GGUF JoyCaption needs llama-cpp-python. Install it into ComfyUI\'s Python: pip install llama-cpp-python. ' + + 'For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" pip install llama-cpp-python'; + +test('a downloaded model whose backend package is missing says so and is not offered as ready (#424)', async ({ page, request }) => { + await expect(await request.post('/__test__/tagging/models', { data: { unavailable: ['wd14-vit'], + missing_packages: { gguf: ['llama-cpp-python'], 'wd14-vit': ['onnxruntime'] } } })).toBeOK(); + await openTagging(page); + const line = modelRow(page).locator('p').first(); + await page.locator('#tag-model').selectOption('gguf'); + await expect(line).toHaveText('Downloaded. Needs llama-cpp-python: pip install llama-cpp-python. ' + + 'For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" pip install llama-cpp-python'); + // Files and packages are separate: Download (again) stays offered. + await expect(actionOf(page, 'Download again')).not.toHaveAttribute('aria-disabled', 'true'); + // Review explains instead of reviewing; nothing is previewed. + await expect(reviewButton(page)).toHaveAttribute('aria-disabled', 'true'); + await reviewButton(page).dispatchEvent('click'); + await expect(toast(page)).toHaveText(NEEDS_LLAMA); + expect(page.modelCalls.filter(call => call.endsWith('/autotag/batch/preview'))).toEqual([]); + // Not downloaded reads differently and still names what it will need. + await page.locator('#tag-model').selectOption('wd14-vit'); + await expect(line).toHaveText('Not downloaded. Download size ~400 MB. Also needs onnxruntime.'); + await page.locator('#tag-model').selectOption('wd14-swinv2'); + await expect(line).toHaveText('Downloaded. Not loaded.'); + await expect(reviewButton(page)).not.toHaveAttribute('aria-disabled', 'true'); +}); + test('entering the section makes no model call besides status', async ({ page }) => { await openTagging(page); await page.locator('#workspace-main .section-nav').getByRole('button', { name: 'Integrations', exact: true }).click(); diff --git a/tests/browser/workspace-tagging-bulk.spec.mjs b/tests/browser/workspace-tagging-bulk.spec.mjs index 01bbda1d..6f3d114d 100644 --- a/tests/browser/workspace-tagging-bulk.spec.mjs +++ b/tests/browser/workspace-tagging-bulk.spec.mjs @@ -186,6 +186,26 @@ test('a model that is not downloaded keeps Start disabled and says why', async ( expect(posted).toBe(0); }); +const NEEDS_LLAMA = 'GGUF JoyCaption needs llama-cpp-python. Install it into ComfyUI\'s Python: pip install llama-cpp-python. ' + + 'For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" pip install llama-cpp-python'; + +test('a model missing its backend package keeps Start disabled and names the install command (#424)', async ({ page, request }) => { + await expect(await request.post('/__test__/tagging/models', { data: { unavailable: [], + missing_packages: { gguf: ['llama-cpp-python'] } } })).toBeOK(); + await select(page, FIRST); + await barButton(page, 'Auto-tag selected').click(); + const status = modal(page).locator('#tag-generation-status'); + const start = actionButtons(page).last(); + await modal(page).getByRole('combobox', { name: 'Tagging model', exact: true }).selectOption('gguf'); + await expect(status).toHaveText(NEEDS_LLAMA); + await expect(start).toHaveAttribute('aria-disabled', 'true'); + await modal(page).getByRole('combobox', { name: 'Tagging model', exact: true }).selectOption('wd14-vit'); + await expect(status).toHaveText(''); + await expect(start).not.toHaveAttribute('aria-disabled', 'true'); + await actionButtons(page).first().click(); + await expect(modal(page)).toHaveCount(0); +}); + test('Suggestions field prefills from the setting and caps chips at the entered count', async ({ page, request }) => { await expect(await request.post('/__test__/tagging/defaults', { data: { general: 0.5, character: 0.9, limit: 7 } })).toBeOK(); await select(page, FIRST); diff --git a/tests/browser/workspace-tagging.spec.mjs b/tests/browser/workspace-tagging.spec.mjs index eaefe7ba..0c4cf8ec 100644 --- a/tests/browser/workspace-tagging.spec.mjs +++ b/tests/browser/workspace-tagging.spec.mjs @@ -260,6 +260,18 @@ test('unavailable model shows inline status and never downloads', async ({ page, await expect(dialog(page).locator('.dialog-actions .ca-primary')).toBeFocused(); }); +test('a missing backend package is the server\'s install sentence, no traceback page (#424)', async ({ page, request }) => { + await expect(await request.post('/__test__/tagging/models', { data: { unavailable: [], + missing_packages: { 'wd14-swinv2': ['onnxruntime'] } } })).toBeOK(); + await openTag(page); + const answer = await generated(page); + expect(answer.status()).toBe(409); + expect(await answer.json()).toMatchObject({ success: false, code: 'model-unavailable' }); + await expect(status(page)).toHaveText('WD14 SwinV2 needs onnxruntime. Install it into ComfyUI\'s Python: ' + + 'pip install onnxruntime. Use onnxruntime-gpu for CUDA.'); + await expect(chips(page)).toHaveCount(0); +}); + test('video tag is manual only', async ({ page, request }) => { await openTag(page, VIDEO); const preview = dialog(page).locator('video.tag-image-preview'); diff --git a/tests/browser/workspace_tagging_fixture.py b/tests/browser/workspace_tagging_fixture.py index ab1d5240..7e2cca21 100644 --- a/tests/browser/workspace_tagging_fixture.py +++ b/tests/browser/workspace_tagging_fixture.py @@ -41,6 +41,18 @@ FILES = ( ("loose/unlinked.png", "unlinked"), ) MODELS = ("wd14-swinv2", "wd14-vit", "gguf", "hf") + + +def _valid_missing(value) -> bool: + """``{"": ["", ...]}`` with known models and string names.""" + return isinstance(value, dict) and all( + name in MODELS + and isinstance(names, list) + and all(isinstance(pip, str) and pip for pip in names) + for name, names in value.items() + ) + + # Unlinked copies enough to virtualize the Images grid (it renders all up to 120). MAX_EXTRA_IMAGES = 160 _TABLES = ( @@ -219,6 +231,7 @@ def register_tagging_commands( if not 0 <= extra <= MAX_EXTRA_IMAGES: raise web.HTTPBadRequest() model.unavailable = set() + model.missing_packages = {} model.suggestions = None model.loaded = None restore_download() @@ -237,17 +250,25 @@ def register_tagging_commands( @routes.post("/__test__/tagging/models") async def models(request: web.Request) -> web.Response: - """Mark fake models as not downloaded (``{"unavailable": [...]}``).""" + """Mark fake models as not downloaded (``{"unavailable": [...]}``) and, + optionally, as missing backend packages (``"missing_packages": + {"": ["", ...]}``, #424).""" data = await request.json() if request.can_read_body else None if ( request.query or not isinstance(data, dict) - or set(data) != {"unavailable"} + or "unavailable" not in data + or set(data) - {"unavailable", "missing_packages"} or not isinstance(data["unavailable"], list) or any(name not in MODELS for name in data["unavailable"]) + or not _valid_missing(data.get("missing_packages", {})) ): raise web.HTTPBadRequest() model.unavailable = set(data["unavailable"]) + model.missing_packages = { + name: list(names) + for name, names in data.get("missing_packages", {}).items() + } return web.json_response({"success": True}) @routes.post("/__test__/tagging/loaded") diff --git a/tests/js/workspace-images-tagging-bulk.test.js b/tests/js/workspace-images-tagging-bulk.test.js index ac6dbeff..9ffec9a1 100644 --- a/tests/js/workspace-images-tagging-bulk.test.js +++ b/tests/js/workspace-images-tagging-bulk.test.js @@ -135,6 +135,41 @@ test('a model that is not downloaded is named with the server sentence', () => { assert.throws(() => modelUnavailable({}, 'nope'), TypeError); }); +test('a downloaded model whose backend package is missing names the package and the command (#424)', () => { + const { modelUnavailable, needsPackages } = state; + const gguf = { downloaded: true, name: 'GGUF (Recommended)', label: 'GGUF JoyCaption', missing_packages: ['llama-cpp-python'], + install_command: 'pip install llama-cpp-python', + install_note: 'For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" pip install llama-cpp-python' }; + const sentence = 'GGUF JoyCaption needs llama-cpp-python. Install it into ComfyUI\'s Python: pip install llama-cpp-python. ' + + 'For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" pip install llama-cpp-python'; + assert.equal(modelUnavailable({ gguf }, 'gguf'), sentence); + // Not downloaded is named first, in its own sentence. + assert.equal(modelUnavailable({ gguf: { ...gguf, downloaded: false } }, 'gguf'), + 'GGUF (Recommended) is not downloaded. Download it in System › Auto-tagging.'); + assert.equal(modelUnavailable({ gguf: { ...gguf, missing_packages: [] } }, 'gguf'), ''); + assert.equal(needsPackages('HuggingFace JoyCaption', { missing_packages: ['accelerate', 'bitsandbytes'] }), + 'HuggingFace JoyCaption needs accelerate and bitsandbytes. Install them into ComfyUI\'s Python: pip install accelerate bitsandbytes.'); + assert.equal(needsPackages('X', { missing_packages: ['a', 'b', 'c'], install_command: 'pip install a b c' }), + 'X needs a, b and c. Install them into ComfyUI\'s Python: pip install a b c.'); + assert.equal(needsPackages('X', null), ''); + assert.equal(needsPackages('X', { missing_packages: [1, ''] }), ''); + // Without the server's label the option's own name is used. + assert.ok(modelUnavailable({ 'wd14-vit': { downloaded: true, missing_packages: ['onnxruntime'] } }, 'wd14-vit') + .startsWith('WD14 ViT needs onnxruntime.')); +}); + +test('the bulk dialog keeps Start disabled while the chosen model misses a package (#424)', async () => { + const { createTaggingOptionsController } = state; + const models = { 'wd14-vit': { downloaded: true, label: 'WD14 ViT', missing_packages: ['onnxruntime'], + install_command: 'pip install onnxruntime' } }; + const options = createTaggingOptionsController({ api: { get: async () => ({ success: true, models, model_type: 'wd14-vit' }) } }); + await options.open(); + assert.equal(options.state.model, 'wd14-vit'); + assert.equal(options.state.canStart, false); + assert.equal(options.state.unavailable, 'WD14 ViT needs onnxruntime. Install it into ComfyUI\'s Python: pip install onnxruntime.'); + options.dispose(); +}); + test('the bulk options start from the server defaults and wait for them before Start', async () => { const { createTaggingOptionsController } = state; let answer; diff --git a/tests/js/workspace-system-tagging.test.js b/tests/js/workspace-system-tagging.test.js index 95c03b4a..317b9149 100644 --- a/tests/js/workspace-system-tagging.test.js +++ b/tests/js/workspace-system-tagging.test.js @@ -482,6 +482,60 @@ test('Tag output images › Review previews the drafts, else says why; a refused } finally { scope.dispose(); dom.restore(); } }); +// #424: GGUF on disk without llama-cpp-python. +const NEEDS_LLAMA = { missing_packages: ['llama-cpp-python'], install_command: 'pip install llama-cpp-python', + install_note: 'For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" pip install llama-cpp-python' }; +const NEEDS_SENTENCE = 'GGUF JoyCaption needs llama-cpp-python. Install it into ComfyUI\'s Python: pip install llama-cpp-python. ' + + 'For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" pip install llama-cpp-python'; + +test('a model on disk whose backend package is missing never reads as ready (#424)', async () => { + const { modelDescription, readStatus, batchCloseNote, STATUS_COPY } = await tagging; + const status = readStatus({ ...MODELS, models: { ...MODELS.models, + gguf: entry('GGUF JoyCaption', '~4.5 GB', { ...NEEDS_LLAMA }), + 'wd14-vit': entry('WD14 ViT', '~400 MB', { downloaded: false, missing_packages: ['onnxruntime'], + install_command: 'pip install onnxruntime', install_note: 'Use onnxruntime-gpu for CUDA.' }), + hf: entry('HuggingFace JoyCaption', '~16 GB', { downloaded: false, model_exists: undefined, + missing_packages: ['accelerate', 'bitsandbytes'], install_command: 'pip install accelerate bitsandbytes' }) } }); + const ready = modelDescription(status, 'gguf'); + assert.notEqual(ready, STATUS_COPY.downloaded); + assert.equal(ready, 'Downloaded. Needs llama-cpp-python: pip install llama-cpp-python. ' + + 'For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" pip install llama-cpp-python'); + // Not downloaded reads differently, and still says what it will need. + assert.equal(modelDescription(status, 'wd14-vit'), 'Not downloaded. Download size ~400 MB. Also needs onnxruntime.'); + assert.equal(modelDescription(status, 'hf'), 'Not downloaded. Download size ~16 GB. Also needs accelerate and bitsandbytes.'); + const partial = readStatus({ ...MODELS, models: { ...MODELS.models, gguf: entry('GGUF JoyCaption', '~4.5 GB', + { downloaded: false, model_exists: true, mmproj_exists: false, ...NEEDS_LLAMA }) } }); + assert.equal(modelDescription(partial, 'gguf'), 'Partly downloaded. Download again to finish. Also needs llama-cpp-python.'); + // Untrusted shapes are ignored; a missing command is built from the names. + const odd = readStatus({ ...MODELS, models: { ...MODELS.models, gguf: entry('GGUF JoyCaption', '~4.5 GB', + { missing_packages: ['llama-cpp-python', 3, ''], install_command: 7, install_note: {} }) } }); + assert.equal(modelDescription(odd, 'gguf'), 'Downloaded. Needs llama-cpp-python: pip install llama-cpp-python.'); + const junk = readStatus({ ...MODELS, models: { ...MODELS.models, gguf: entry('GGUF JoyCaption', '~4.5 GB', + { missing_packages: 'llama-cpp-python' }) } }); + assert.equal(modelDescription(junk, 'gguf'), STATUS_COPY.downloaded); + // The batch review says why it cannot start, in the server's sentence. + assert.equal(batchCloseNote({ ...PLAN, model: { type: 'gguf', label: 'GGUF JoyCaption', downloaded: true, ...NEEDS_LLAMA } }), + NEEDS_SENTENCE); + assert.equal(batchCloseNote({ ...PLAN, model: { ...PLAN.model, downloaded: false, ...NEEDS_LLAMA } }), + 'WD14 SwinV2 is not downloaded. Download it first.'); +}); + +test('Review explains a missing backend package instead of reviewing (#424)', async () => { + const models = { ...MODELS, models: { ...MODELS.models, gguf: entry('GGUF JoyCaption', '~4.5 GB', { ...NEEDS_LLAMA }) } }; + const { dom, scope, button, byId, calls } = await mounted({ models }); + try { + byId('tag-model').value = 'gguf'; await byId('tag-model').dispatch('change'); + const review = button('Review'); + assert.equal(review.getAttribute('aria-disabled'), 'true'); + assert.equal(review.dataset.reason, NEEDS_SENTENCE); + await review.dispatch('click'); + assert.equal(calls.toasts.at(-1), NEEDS_SENTENCE); + assert.deepEqual(calls.post, []); + // Download stays offered: files and packages are separate. + assert.equal(button('Download again').getAttribute('aria-disabled'), null); + } finally { scope.dispose(); dom.restore(); } +}); + test('settingRefusal marks only an Auto-tagging save\'s plain 400 as invalid-setting (CUJ 2a)', async () => { const { settingRefusal } = await import('../../web/js/workspace/system.mjs'); const refusal = (extra = {}) => Object.assign(new Error('General-tag confidence must be between 0 and 1.'), diff --git a/tests/test_api_admin.py b/tests/test_api_admin.py index 120c1e93..755a5369 100644 --- a/tests/test_api_admin.py +++ b/tests/test_api_admin.py @@ -2205,24 +2205,30 @@ class TestOutputScanInternals(AdminAPITestCase): self.assertEqual(self.api._ingest_scanned_file(f, meta), "unchanged") def test_ingest_link_failures_are_tolerated(self): + """A failed link rolls the new prompt back: found, nothing left behind.""" f = Path("gen.png") meta = {"prompt": json.dumps(_wired_prompt("a dog"))} - self.api.db.link_image_to_prompt = _raise - self.assertEqual(self.api._ingest_scanned_file(f, meta), "added") - self.assertEqual(self.api._ingest_scanned_file(f, meta), "found") + self.api.db._insert_image_link = _raise + with self.assertLogs("prompt_manager.database.scanned_prompts", "ERROR"): + self.assertEqual(self.api._ingest_scanned_file(f, meta), "found") + self.assertEqual(self.api._ingest_scanned_file(f, meta), "found") + self.assertEqual(self.api.db.get_recent_prompts()["prompts"], []) def test_ingest_save_failure_counts_as_found(self): f = Path("gen.png") meta = {"prompt": json.dumps(_wired_prompt("a bird"))} - self.api.db.save_prompt = lambda *a, **k: None - self.assertEqual(self.api._ingest_scanned_file(f, meta), "found") + with patch( + "py.api.admin.store_scanned_prompt", return_value=("found", None) + ) as store: + self.assertEqual(self.api._ingest_scanned_file(f, meta), "found") + self.assertEqual(store.call_args.args[3], "a bird") def test_batch_counts_and_per_file_errors(self): meta = {"prompt": json.dumps(_wired_prompt("a fish"))} - self.api.db.get_prompt_by_hash = _raise - counts = self.api._ingest_scan_batch_sync( - [(Path("a.png"), meta), (Path("b.png"), {})] - ) + with patch("py.api.admin.store_scanned_prompt", side_effect=_raise): + counts = self.api._ingest_scan_batch_sync( + [(Path("a.png"), meta), (Path("b.png"), {})] + ) self.assertEqual( counts, { diff --git a/tests/test_api_annotation_suggest.py b/tests/test_api_annotation_suggest.py index de097d61..7d1b3539 100644 --- a/tests/test_api_annotation_suggest.py +++ b/tests/test_api_annotation_suggest.py @@ -29,6 +29,7 @@ except ImportError: model_gate = None from py import config as config_module +from py.autotag import MissingBackendPackages, package_fields AutotagConfig = getattr(config_module, "AutotagConfig", None) @@ -37,6 +38,11 @@ WAIT = 30.0 WD14 = {"general_threshold": 0.35, "character_threshold": 0.85} LLM = {"instructions": "Generate tags."} UNAVAILABLE = "Download it in System › Auto-tagging." +NEEDS_LLAMA = ( + "GGUF JoyCaption needs llama-cpp-python. Install it into ComfyUI's Python: " + 'pip install llama-cpp-python. For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" ' + "pip install llama-cpp-python" +) SCORED = [("sandstone", 0.98), ("desert", 0.96)] + [ (f"tag_{i:03d}", round(0.9 - i * 0.005, 4)) for i in range(150) @@ -59,6 +65,8 @@ class FakeTagger: "hf": {}, } self.downloaded = {name: True for name in self.models_config} + # pip names of the backend packages that cannot be imported (#424) + self.missing = {name: [] for name in self.models_config} self.loaded = None self.load_calls = [] self.unload_calls = 0 @@ -71,7 +79,11 @@ class FakeTagger: def get_models_status(self): return { - name: {"downloaded": self.downloaded[name], "model_path": None} + name: { + "downloaded": self.downloaded[name], + "model_path": None, + **package_fields(name, self.missing[name]), + } for name in self.models_config } @@ -315,6 +327,37 @@ class TestModelAvailability(SuggestTestCase): self.assertEqual(self.tagger.calls, []) self.assertIsNone(model_gate.holder()) + async def test_missing_backend_package_is_refused_before_the_model(self): + observed = await self.observed() + self.tagger.missing["gguf"] = ["llama-cpp-python"] + + status, body = await self.suggest(self.suggest_body(observed, "gguf")) + + self.assertEqual((status, body["code"]), (409, "model-unavailable")) + self.assertEqual(body["success"], False) + self.assertEqual(body["error"], NEEDS_LLAMA) + self.assertEqual(self.tagger.load_calls, []) + self.assertIsNone(model_gate.holder()) + # Not downloaded still reads as before, whatever the packages. + self.tagger.downloaded["gguf"] = False + status, body = await self.suggest(self.suggest_body(observed, "gguf")) + self.assertIn("is not downloaded", body["error"]) + + async def test_a_package_lost_after_the_check_is_the_same_409_without_a_traceback( + self, + ): + observed = await self.observed() + self.tagger.load_error = MissingBackendPackages("gguf", ["llama-cpp-python"]) + + with self.assertNoLogs(self.api.logger, level="ERROR"): + status, body = await self.suggest(self.suggest_body(observed, "gguf")) + + self.assertEqual((status, body["code"]), (409, "model-unavailable")) + self.assertEqual(body["error"], NEEDS_LLAMA) + self.assertIsNone(model_gate.holder()) + entry = self.api._captures._entries[observed["capture"]["token"]] + self.assertEqual(entry.uses, 0) + async def test_load_failure_is_a_generic_error_and_frees_everything(self): observed = await self.observed() self.tagger.load_error = RuntimeError(f"cannot load {self.output_dir}") diff --git a/tests/test_api_autotag.py b/tests/test_api_autotag.py index cfe3f47a..4b009ba8 100644 --- a/tests/test_api_autotag.py +++ b/tests/test_api_autotag.py @@ -73,6 +73,7 @@ class FakeAutotagService: self.status_calls = 0 self.status_threads = [] self.status_paths = {} + self.status_extra = {} self.suggest_calls = 0 def get_models_status(self): @@ -82,6 +83,7 @@ class FakeAutotagService: name: { "downloaded": self.downloaded, "model_path": self.status_paths.get(name), + **self.status_extra.get(name, {}), } for name in self.models_config } @@ -1111,6 +1113,24 @@ class TestModelStatus(AutotagAPITestCase): leaked = [text for text in _strings(data) if _absolute(text)] self.assertEqual(leaked, []) + async def test_models_status_names_missing_backend_packages(self): + fields = { + "missing_packages": ["llama-cpp-python"], + "install_command": "pip install llama-cpp-python", + "install_note": "For a CUDA build: ...", + } + self.service.status_extra = {"gguf": fields} + models = (await self._models())["models"] + self.assertTrue(models["gguf"]["downloaded"]) + self.assertEqual( + {key: models["gguf"][key] for key in fields}, + fields, + ) + self.assertEqual( + {key: models["wd14-vit"][key] for key in fields}, + {"missing_packages": [], "install_command": None, "install_note": None}, + ) + async def test_models_status_adds_label_size_label_and_loaded(self): self.service._loaded_type = "wd14-vit" data = await self._models() @@ -1669,6 +1689,10 @@ class TestTagOutputImagesPreview(BatchJobTestCase): "label": "WD14 ViT", "size_label": "~400 MB", "downloaded": True, + # #424: the backend packages it cannot import, none here. + "missing_packages": [], + "install_command": None, + "install_note": None, }, ) self.assertEqual(skip["existing"], "skip") diff --git a/tests/test_autotag_batch.py b/tests/test_autotag_batch.py index c6703ecb..4b63d2f9 100644 --- a/tests/test_autotag_batch.py +++ b/tests/test_autotag_batch.py @@ -553,6 +553,35 @@ class TestStartRefusals(BatchTestCase): self.assertEqual(self.api._jobs.list(), []) self.assertIsNone(model_gate.holder()) + async def test_a_missing_backend_package_refuses_the_start(self): + self.tagger.missing["gguf"] = ["llama-cpp-python"] + status, data = await self.start_batch(model_type="gguf") + self.assertEqual((status, data["code"]), (409, "model-unavailable")) + self.assertEqual(data["success"], False) + self.assertTrue( + data["error"].startswith("GGUF JoyCaption needs llama-cpp-python."), data + ) + self.assertIn("pip install llama-cpp-python", data["error"]) + self.assertEqual(self.api._jobs.list(), []) + self.assertEqual(self.tagger.load_calls, []) + self.assertIsNone(model_gate.holder()) + + async def test_a_package_lost_after_the_start_fails_the_job_without_a_traceback( + self, + ): + from py.autotag import MissingBackendPackages + + self.image("a.png", (1, 1, 1)) + self.tagger.load_error = MissingBackendPackages("gguf", ["llama-cpp-python"]) + with self.assertNoLogs(self.api.logger, level="ERROR"): + job = await self.run_batch(model_type="gguf") + self.assertEqual(job.state, "failed") + self.assertTrue( + job.status_line.startswith("GGUF JoyCaption needs"), job.status_line + ) + self.assertTrue(any("pip install llama-cpp-python" in line for line in job.log)) + self.assertIsNone(model_gate.holder()) + async def test_model_busy_when_the_gate_is_held(self): owner = model_gate.new_owner("image-annotations") self.assertTrue(model_gate.try_acquire(owner)) @@ -658,6 +687,20 @@ class TestPreview(BatchTestCase): self.assertIsNone(llm["estimate_device"]) self.assertEqual([s for s in _strings(skip) if _absolute(s)], []) + async def test_preview_names_missing_backend_packages(self): + self.image("a.png", (1, 1, 1)) + data = (await self.preview(model_type="gguf"))[1] + self.assertEqual(data["model"]["missing_packages"], []) + self.assertIsNone(data["model"]["install_command"]) + self.tagger.missing["gguf"] = ["llama-cpp-python"] + data = (await self.preview(model_type="gguf"))[1] + self.assertTrue(data["model"]["downloaded"]) + self.assertEqual(data["model"]["missing_packages"], ["llama-cpp-python"]) + self.assertEqual( + data["model"]["install_command"], "pip install llama-cpp-python" + ) + self.assertIn("DGGML_CUDA", data["model"]["install_note"]) + async def test_preview_uses_the_graphics_card_rate_when_wd14_runs_on_cuda(self): self.image("a.png", (1, 1, 1)) self.tagger.wd14_device = lambda: "gpu" diff --git a/tests/test_autotag_engine.py b/tests/test_autotag_engine.py index 7f7faa3f..ec3e468e 100644 --- a/tests/test_autotag_engine.py +++ b/tests/test_autotag_engine.py @@ -800,8 +800,12 @@ class TestDefaultLoaders(EngineTestCase): transformers, model, processor, inputs = self._fake_transformers() pixel = inputs["pixel_values"] service = AutoTagService(models_dir=self.models_dir) + # The 8-bit load also needs these two (#424); stand-ins satisfy the check. + stubs = { + name: types.ModuleType(name) for name in ("accelerate", "bitsandbytes") + } - with _patch_modules({"torch": torch, "transformers": transformers}): + with _patch_modules({"torch": torch, "transformers": transformers, **stubs}): service.load_model("hf") tags = service.generate_tags(str(self.image_path)) diff --git a/tests/test_autotag_legacy.py b/tests/test_autotag_legacy.py index 126ef670..f8a379a7 100644 --- a/tests/test_autotag_legacy.py +++ b/tests/test_autotag_legacy.py @@ -161,6 +161,39 @@ class TestLegacyBatch(LegacyTestCase): self.assertEqual(self.tagger.calls[-1]["prompt"], AutotagConfig.TEMPLATE) self.assertEqual(self.tagger.custom_prompt, "legacy prompt") + async def test_a_missing_backend_package_is_one_error_frame(self): + self.tagger.missing["gguf"] = ["llama-cpp-python"] + _, frames = await self.legacy("model_type=gguf") + self.assertEqual(len(frames), 1) + self.assertEqual(frames[0]["type"], "error") + self.assertTrue( + frames[0]["message"].startswith("GGUF JoyCaption needs llama-cpp-python.") + ) + self.assertEqual(self.tagger.load_calls, []) + + async def test_legacy_single_refuses_a_missing_backend_package(self): + from py.autotag import MissingBackendPackages + + self.image("a.png", (1, 1, 1)) + await self.relist() + self.tagger.missing["gguf"] = ["llama-cpp-python"] + status, data = await self.single( + {"path": "a.png", "root_index": 0, "model_type": "gguf"} + ) + self.assertEqual((status, data["code"]), (409, "model-unavailable")) + self.assertIn("pip install llama-cpp-python", data["error"]) + self.assertEqual(self.tagger.load_calls, []) + # Lost after the check: the same 409, no ERROR record, nothing held. + self.tagger.missing["gguf"] = [] + self.tagger.load_error = MissingBackendPackages("gguf", ["llama-cpp-python"]) + with self.assertNoLogs(self.api.logger, level="ERROR"): + status, data = await self.single( + {"path": "a.png", "root_index": 0, "model_type": "gguf"} + ) + self.assertEqual((status, data["code"]), (409, "model-unavailable")) + self.assertIn("pip install llama-cpp-python", data["error"]) + self.assertIsNone(model_gate.holder()) + async def test_each_refusal_is_one_error_frame(self): self.tagger.downloaded["wd14-vit"] = False _, frames = await self.legacy() diff --git a/tests/test_autotag_packages.py b/tests/test_autotag_packages.py new file mode 100644 index 00000000..fb910ce8 --- /dev/null +++ b/tests/test_autotag_packages.py @@ -0,0 +1,248 @@ +"""#424: each tagging backend reports the Python packages it cannot import. + +``find_spec`` is patched throughout: no ML backend is installed or imported. +""" + +import os +import sys +import types +import unittest +from unittest.mock import patch + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +try: + from test_autotag_engine import ( + EngineTestCase, + FakeGgufModel, + _patch_modules, + _touch_model_files, + ) +except ImportError: # pragma: no cover - run as a package + from tests.test_autotag_engine import ( + EngineTestCase, + FakeGgufModel, + _patch_modules, + _touch_model_files, + ) + +from py import autotag # noqa: E402 +from py.autotag import ( # noqa: E402 + BACKEND_PACKAGES, + MODELS, + AutoTagService, + MissingBackendPackages, + missing_packages, + package_fields, +) + +GGUF_CUDA = 'For a CUDA build: CMAKE_ARGS="-DGGML_CUDA=on" pip install llama-cpp-python' + + +def _find_spec_without(*absent): + """A ``find_spec`` that knows every module except *absent*.""" + + def find_spec(name, *args): + return None if name in absent else object() + + return find_spec + + +def _missing_from(*absent): + return patch.object( + autotag.importlib.util, "find_spec", side_effect=_find_spec_without(*absent) + ) + + +class TestMissingPackages(unittest.TestCase): + + def test_every_backend_names_its_modules_and_pip_names(self): + self.assertEqual(BACKEND_PACKAGES["gguf"], (("llama_cpp", "llama-cpp-python"),)) + self.assertEqual( + [pip for _module, pip in BACKEND_PACKAGES["hf"]], + ["torch", "transformers", "accelerate", "bitsandbytes"], + ) + for model in ("wd14-swinv2", "wd14-vit"): + self.assertEqual(BACKEND_PACKAGES[model], (("onnxruntime", "onnxruntime"),)) + self.assertEqual(set(BACKEND_PACKAGES), set(MODELS)) + + def test_reports_pip_names_of_modules_find_spec_cannot_find(self): + with _missing_from("llama_cpp", "accelerate", "bitsandbytes"): + self.assertEqual(missing_packages("gguf"), ["llama-cpp-python"]) + self.assertEqual(missing_packages("hf"), ["accelerate", "bitsandbytes"]) + self.assertEqual(missing_packages("wd14-vit"), []) + + def test_find_spec_errors_count_as_missing(self): + for error in (ValueError("no spec"), ImportError("broken"), OSError("io")): + with self.subTest(error=type(error).__name__): + with patch.object( + autotag.importlib.util, "find_spec", side_effect=error + ): + self.assertEqual(missing_packages("wd14-vit"), ["onnxruntime"]) + + def test_a_module_already_imported_is_present_without_a_lookup(self): + with _patch_modules({"llama_cpp": types.ModuleType("llama_cpp")}): + with patch.object( + autotag.importlib.util, "find_spec", side_effect=AssertionError + ): + self.assertEqual(missing_packages("gguf"), []) + + def test_the_check_imports_nothing(self): + before = set(sys.modules) + missing_packages("gguf") + missing_packages("hf") + missing_packages("wd14-vit") + imported = set(sys.modules) - before + heavy = {"llama_cpp", "torch", "transformers", "onnxruntime"} + self.assertFalse(imported & heavy, imported) + + def test_unknown_model_has_no_packages(self): + self.assertEqual(missing_packages("nope"), []) + + def test_package_fields_and_the_one_sentence(self): + self.assertEqual( + package_fields("gguf", []), + {"missing_packages": [], "install_command": None, "install_note": None}, + ) + fields = package_fields("gguf", ["llama-cpp-python"]) + self.assertEqual(fields["missing_packages"], ["llama-cpp-python"]) + self.assertEqual(fields["install_command"], "pip install llama-cpp-python") + self.assertEqual(fields["install_note"], GGUF_CUDA) + self.assertEqual( + package_fields("hf", ["torch", "bitsandbytes"])["install_command"], + "pip install torch bitsandbytes", + ) + self.assertIsNone(package_fields("hf", ["torch"])["install_note"]) + self.assertIn( + "onnxruntime-gpu", + package_fields("wd14-vit", ["onnxruntime"])["install_note"], + ) + error = MissingBackendPackages("gguf", ["llama-cpp-python"]) + self.assertIsInstance(error, RuntimeError) + self.assertEqual( + str(error), + "GGUF JoyCaption needs llama-cpp-python. Install it into ComfyUI's " + "Python: pip install llama-cpp-python. " + GGUF_CUDA, + ) + self.assertEqual( + str(MissingBackendPackages("hf", ["accelerate", "bitsandbytes"])), + "HuggingFace JoyCaption needs accelerate and bitsandbytes. Install them " + "into ComfyUI's Python: pip install accelerate bitsandbytes.", + ) + + +class TestStatusReportsPackages(EngineTestCase): + + def test_downloaded_keeps_its_files_only_meaning(self): + _touch_model_files(self.models_dir, "gguf") + _touch_model_files(self.models_dir, "wd14-vit") + service = AutoTagService(models_dir=self.models_dir) + + with _missing_from("llama_cpp"): + status = service.get_models_status() + + self.assertTrue(status["gguf"]["downloaded"]) + self.assertEqual(status["gguf"]["missing_packages"], ["llama-cpp-python"]) + self.assertEqual( + status["gguf"]["install_command"], "pip install llama-cpp-python" + ) + self.assertEqual(status["gguf"]["install_note"], GGUF_CUDA) + self.assertEqual(status["wd14-vit"]["missing_packages"], []) + self.assertIsNone(status["wd14-vit"]["install_command"]) + # A model not on disk still says which packages it will need. + self.assertFalse(status["wd14-swinv2"]["downloaded"]) + self.assertIn("missing_packages", status["hf"]) + + def test_an_injected_loader_brings_its_own_backend(self): + service = self._service(("gguf", FakeGgufModel("x"))) + with _missing_from("llama_cpp"): + self.assertEqual( + service.get_models_status()["gguf"]["missing_packages"], [] + ) + + def test_an_injected_probe_is_used_for_status_and_load(self): + _touch_model_files(self.models_dir, "gguf") + service = AutoTagService( + models_dir=self.models_dir, + model_loader=lambda *_: ("gguf", FakeGgufModel("x")), + package_probe=lambda model_type: ["llama-cpp-python"], + ) + self.assertEqual( + service.get_models_status()["gguf"]["missing_packages"], + ["llama-cpp-python"], + ) + with self.assertRaises(MissingBackendPackages): + service.load_model("gguf") + + +class TestLoadFailsFast(EngineTestCase): + + def test_missing_package_refuses_before_the_loader_without_an_error_log(self): + _touch_model_files(self.models_dir, "gguf") + service = AutoTagService(models_dir=self.models_dir) + calls = [] + service._model_loader = lambda *args: calls.append(args) + + with ( + _missing_from("llama_cpp"), + self.assertLogs(service.logger, level="DEBUG") as logs, + ): + with self.assertRaises(MissingBackendPackages) as ctx: + service.load_model("gguf") + + self.assertEqual(calls, []) + self.assertEqual(ctx.exception.model_type, "gguf") + self.assertEqual(ctx.exception.missing, ["llama-cpp-python"]) + self.assertIn("pip install llama-cpp-python", str(ctx.exception)) + self.assertFalse(service.is_model_loaded()) + levels = {record.levelname for record in logs.records} + self.assertNotIn("ERROR", levels) + self.assertIn("WARNING", levels) + self.assertTrue(all(record.exc_info is None for record in logs.records)) + + def test_not_downloaded_is_reported_before_packages(self): + service = AutoTagService(models_dir=self.models_dir) + with _missing_from("llama_cpp"): + with self.assertRaises(RuntimeError) as ctx: + service.load_model("gguf") + self.assertNotIsInstance(ctx.exception, MissingBackendPackages) + + def test_a_module_not_found_from_the_loader_becomes_the_same_refusal(self): + _touch_model_files(self.models_dir, "gguf") + + def loader(model_type, use_gpu): + raise ModuleNotFoundError("No module named 'llama_cpp'", name="llama_cpp") + + service = AutoTagService( + models_dir=self.models_dir, + model_loader=loader, + package_probe=lambda _model: [], + ) + with self.assertLogs(service.logger, level="DEBUG") as logs: + with self.assertRaises(MissingBackendPackages) as ctx: + service.load_model("gguf") + self.assertEqual(ctx.exception.missing, ["llama-cpp-python"]) + self.assertNotIn("ERROR", {record.levelname for record in logs.records}) + self.assertFalse(service.is_model_loaded()) + + def test_an_unknown_module_not_found_names_the_module(self): + def loader(model_type, use_gpu): + raise ModuleNotFoundError( + "No module named 'sentencepiece'", name="sentencepiece" + ) + + hf_dir = self.models_dir / MODELS["hf"]["subdir"] + hf_dir.mkdir(parents=True, exist_ok=True) + (hf_dir / "config.json").write_text("{}") + service = AutoTagService( + models_dir=self.models_dir, + model_loader=loader, + package_probe=lambda _model: [], + ) + with self.assertRaises(MissingBackendPackages) as ctx: + service.load_model("hf") + self.assertEqual(ctx.exception.missing, ["sentencepiece"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_file_prompt.py b/tests/test_file_prompt.py new file mode 100644 index 00000000..c4dadfed --- /dev/null +++ b/tests/test_file_prompt.py @@ -0,0 +1,705 @@ +"""The per-file prompt decision shared by the scan and the monitor (#247). + +One test per case, named by the ladder step of the design +(``docs/egm/247-foreign-workflows``). The ladder does no file or database +I/O: history comes in through ``lookup``. +""" + +import json +import unittest +from unittest import mock + +from database.run_attribution import RecordedOutput +from tests.test_pair_scan import flux_graph, sampled_graph +from tests.test_prompt_attribution import issue_graph, pair_evidence +from tests import test_save_to_library as save_tests +from utils import file_prompt +from utils.file_prompt import Policy, decide_file_prompt, junk_text +from utils.prompt_inspector import InspectionResult + +try: + from tests import inspection_parity as parity + from tests import saver_anchor_graphs as shapes +except ImportError: # pragma: no cover - discover -s tests puts tests/ first + import inspection_parity as parity + import saver_anchor_graphs as shapes + +SHARED_NAME = "IMG_GEN_00371_.png" +LORA = "" + + +def _decide( + metadata, + *, + policy=Policy.SCAN, + lookup=None, + in_output=True, + file_name=SHARED_NAME, + file_size=None, +): + return decide_file_prompt( + metadata, + file_name=file_name, + file_size=file_size, + in_output=in_output, + lookup=lookup, + policy=policy, + ) + + +def _graph(graph): + return {"prompt": json.dumps(graph)} + + +class _Lookup: + """A history lookup that counts its calls.""" + + def __init__(self, saver=None): + self.calls = 0 + self.found = RecordedOutput(saver, {}) if saver is not None else None + + def __call__(self): + self.calls += 1 + return self.found + + +def _inspected(state, malformed=False): + resolution = {"state": state, "positive": None, "negative": None} + return InspectionResult(resolution, malformed=malformed) + + +def _untraced_by_both(): + """A sampled graph whose positive no profile traces; the legacy parser does.""" + return mock.patch.object( + file_prompt, "inspection_resolution", return_value=_inspected("unresolved") + ) + + +def _gated(graph): + """``graph`` past the default profile's node gate, so it is unresolved.""" + graph = dict(graph) + for index in range(510): + graph[f"pad{index}"] = {"class_type": "Note", "inputs": {}} + return graph + + +def _prompt_manager_branches(prepend="PRE", append="APP"): + """Two KSampler branches, each fed by a PromptManagerText string.""" + graph = sampled_graph() + del graph["1"] + sampler = graph.pop("3") + for index, text in enumerate(("a cat", "a dog"), start=1): + base = index * 10 + graph[str(base)] = { + "class_type": "PromptManagerText", + "inputs": {"text": text, "prepend_text": prepend, "append_text": append}, + } + graph[str(base + 1)] = { + "class_type": "CLIPTextEncode", + "inputs": {"text": [str(base), 0]}, + } + graph[str(base + 2)] = { + **sampler, + "inputs": {**sampler["inputs"], "positive": [str(base + 1), 0]}, + } + return graph + + +def _numeric_first_branches(): + """Three branches; the legacy parser's answer (the first) is a number.""" + graph = sampled_graph("2048") + for index, text in ((5, "a cat"), (6, "a dog")): + graph[str(index * 10)] = { + "class_type": "CLIPTextEncode", + "inputs": {"text": text}, + } + graph[str(index * 10 + 1)] = { + **graph["3"], + "inputs": {**graph["3"]["inputs"], "positive": [str(index * 10), 0]}, + } + return graph + + +class TestStepsBeforeResolving(unittest.TestCase): + def test_step_1_no_metadata_tries_the_recorded_run(self): + for metadata in ({}, None): + decision = _decide(metadata) + self.assertEqual(decision.kind, "never") + self.assertTrue(decision.try_recorded_run) + self.assertIsNone(decision.unlinked_status) + self.assertEqual(decision.canonical, {}) + + def test_step_3_neither_prompt_nor_parameters_tries_the_recorded_run(self): + decision = _decide({"workflow": json.dumps({"nodes": []})}) + self.assertEqual(decision.kind, "never") + self.assertTrue(decision.try_recorded_run) + self.assertIsNone(decision.unlinked_status) + self.assertEqual(decision.canonical, {"workflow": {"nodes": []}}) + + def test_strings_are_parsed_and_malformed_ones_kept(self): + decision = _decide({"prompt": "{not json", "workflow": "{}"}) + self.assertEqual(decision.canonical["prompt"], "{not json") + self.assertEqual(decision.canonical["workflow"], {}) + + def test_the_file_size_is_recorded_unless_the_metadata_has_one(self): + meta = _graph(sampled_graph()) + self.assertEqual( + _decide(meta, file_size=42).canonical["file_info"], {"size": 42} + ) + meta["file_info"] = {"size": 7, "width": 3} + self.assertEqual( + _decide(meta, file_size=42).canonical["file_info"], {"size": 7, "width": 3} + ) + self.assertNotIn("file_info", _decide(_graph(sampled_graph())).canonical) + + def test_a_file_size_reader_is_called_only_for_prompt_metadata(self): + reads = [] + + def size(): + reads.append(1) + return 9 + + for metadata in ({}, {"workflow": "{}"}): + _decide(metadata, file_size=size) + self.assertEqual(reads, []) + decision = _decide(_graph(sampled_graph()), file_size=size) + self.assertEqual((reads, decision.canonical["file_info"]), ([1], {"size": 9})) + + +def _off_behind(middle, key): + """A switched-off PromptManagerText reaching the encoder through + ``middle``'s ``key`` input.""" + graph = save_tests.run_graph() + graph["1"] = { + "class_type": "PromptManagerText", + "inputs": {"text": "SECRET OFF TEXT", "save_to_library": False}, + } + graph["7"] = {"class_type": middle, "inputs": {key: ["1", 0]}} + graph["8"] = { + "class_type": "CLIPTextEncode", + "inputs": {"text": ["7", 0], "clip": ["4", 1]}, + } + graph["3"]["inputs"]["positive"] = ["8", 0] + return graph + + +class TestSaveToLibrarySteps(unittest.TestCase): + """#90: a PromptManager node with Save to library off stores nothing.""" + + def test_step_2_an_opted_out_file_is_not_saved(self): + lookup = _Lookup("9") + graph = save_tests.run_graph(save=False) + graph["5"] = {**graph["3"], "inputs": {**graph["3"]["inputs"]}} + graph["5"]["inputs"]["positive"] = ["2", 0] # ambiguous otherwise + for policy in Policy: + decision = _decide(_graph(graph), policy=policy, lookup=lookup) + self.assertEqual(decision.kind, "not_saved") + self.assertFalse(decision.try_recorded_run) + self.assertIsNone(decision.review) + self.assertEqual(lookup.calls, 0) + + def test_step_2_a_wired_switch_is_not_saved_wired(self): + graph = save_tests.run_graph(save=["9", 0]) + graph["9"] = {"class_type": "SomeCustomSwitch", "inputs": {}} + lookup = _Lookup("9") + decision = _decide(_graph(graph), lookup=lookup) + self.assertEqual(decision.kind, "not_saved_wired") + self.assertFalse(decision.try_recorded_run) + self.assertIsNone(decision.review) + self.assertEqual(lookup.calls, 0) + + def test_step_8_1_an_off_node_behind_a_pass_through_is_not_saved(self): + """The #90 graph walk does not follow ``anything`` or ``value``; the + inspector does, so its positive is checked against the switch.""" + for middle, key in (("easy showAnything", "anything"), ("MyNode", "value")): + with self.subTest(middle=middle): + graph = _off_behind(middle, key) + for policy in Policy: + decision = _decide(_graph(graph), policy=policy, lookup=_Lookup()) + self.assertEqual(decision.kind, "not_saved") + self.assertIsNone(decision.text) + + def test_step_8_4_a_legacy_answer_from_an_off_node_is_not_saved(self): + graph = _off_behind("MyNode", "value") + graph["9"] = {"class_type": "CLIPTextEncode", "inputs": {"text": "on text"}} + graph["5"] = {**graph["3"], "inputs": {**graph["3"]["inputs"]}} + graph["5"]["inputs"]["positive"] = ["9", 0] + decision = _decide(_graph(graph), lookup=_Lookup()) + self.assertEqual(decision.kind, "not_saved") + self.assertIsNone(decision.text) + + def test_step_2_an_off_node_on_any_positive_input_is_not_saved(self): + """Any input name upstream of a positive, not only prompt-like ones.""" + guider = _off_behind("MyNode", "value") + guider["3"] = { + "class_type": "BasicGuider", + "inputs": {"model": ["4", 0], "conditioning": ["8", 0]}, + } + prepended = _off_behind("MyNode", "value") + prepended["1"]["inputs"]["prepend_text"] = "pre" + pair = _off_behind("MyNode", "value") + pair["1"] = { + "class_type": "PromptManagerPair", + "inputs": {"text": "x", "negative_text": "y", "save_to_library": False}, + } + pair["2"]["inputs"]["text"] = ["1", 1] + cases = {"sampler-less guider": guider, "prepend": prepended, "pair": pair} + for name, graph in cases.items(): + with self.subTest(name=name): + lookup = _Lookup("9") + decision = _decide(_graph(graph), lookup=lookup) + self.assertEqual(decision.kind, "not_saved") + self.assertFalse(decision.try_recorded_run) + self.assertEqual(lookup.calls, 0) + + def test_step_2_a_wired_switch_behind_a_pass_through_is_not_saved_wired(self): + graph = _off_behind("MyNode", "value") + graph["1"]["inputs"]["save_to_library"] = ["20", 0] + graph["20"] = {"class_type": "SomeBoolNode", "inputs": {}} + self.assertEqual(_decide(_graph(graph)).kind, "not_saved_wired") + + def test_a_switched_on_node_behind_a_pass_through_is_stored(self): + """Regression guard: on, or no switch at all (older files).""" + for value in (True, None): + with self.subTest(value=value): + graph = _off_behind("MyNode", "value") + if value is None: + del graph["1"]["inputs"]["save_to_library"] + else: + graph["1"]["inputs"]["save_to_library"] = value + decision = _decide(_graph(graph), lookup=_Lookup()) + self.assertEqual(decision.kind, "positive") + self.assertEqual(decision.text, "SECRET OFF TEXT") + + def test_an_off_node_on_a_negative_input_name_never_blocks(self): + """The walk skips negative inputs: #90's own rule (``neg``, + ``neg_text``, ``text_neg``) and any name containing "negative".""" + for key in ( + "neg", + "neg_text", + "text_neg", + "negative_text", + "negative2", + "negativeprompt", + "NegativePrompt", + ): + with self.subTest(key=key): + graph = { + "1": { + "class_type": "PromptManagerText", + "inputs": {"text": "bad hands", "save_to_library": False}, + }, + "9": { + "class_type": "PromptManagerText", + "inputs": {"text": "a cat", "save_to_library": True}, + }, + "7": { + "class_type": "PipeEncode", + "inputs": {"pos_text": ["9", 0], key: ["1", 0]}, + }, + "4": {"class_type": "CheckpointLoaderSimple", "inputs": {}}, + "3": { + "class_type": "KSampler", + "inputs": { + "model": ["4", 0], + "positive": ["7", 0], + "negative": ["7", 1], + }, + }, + } + self.assertIsNone(file_prompt._switched_off_upstream(graph)) + self.assertNotIn( + _decide(_graph(graph)).kind, ("not_saved", "not_saved_wired") + ) + + def test_the_guards_accept_malformed_nodes(self): + graph = { + "1": {"class_type": "PromptManagerText", "inputs": None}, + "2": {"class_type": "PromptManagerText"}, + } + self.assertIsNone(file_prompt._off_text(graph, "x")) + side = {"provenance": ["not", "a", "dict"]} + with mock.patch.object(file_prompt, "side_opted_out", return_value=True): + self.assertIsNone(file_prompt._side_switch(graph, side)) + + def test_the_off_text_guard_reads_a_nodes_resolved_text(self): + graph = { + "1": { + "class_type": "PromptManagerText", + "inputs": {"text": ["30", 0], "save_to_library": False}, + }, + "30": { + "class_type": "PrimitiveStringMultiline", + "inputs": {"value": "SECRET OFF TEXT"}, + }, + "2": { + "class_type": "PromptManagerText", + "inputs": {"text": "WIRED", "save_to_library": ["9", 0]}, + }, + "9": {"class_type": "SomeBoolNode", "inputs": {}}, + } + self.assertEqual(file_prompt._off_text(graph, " SECRET OFF TEXT"), "not_saved") + self.assertEqual(file_prompt._off_text(graph, "WIRED"), "not_saved_wired") + self.assertIsNone(file_prompt._off_text(graph, "other")) + + def test_steps_5_and_7_check_the_resolved_positive_too(self): + """A resolved positive through an off node the #90 walk missed.""" + cases = { + "pair": save_tests.run_graph(save=False), + "negative off": save_tests.run_graph(save=False, negative_save=False), + } + for name, graph in cases.items(): + with ( + self.subTest(name=name), + mock.patch.object( + file_prompt, "metadata_opted_out", return_value=False + ), + ): + self.assertEqual(_decide(_graph(graph)).kind, "not_saved") + + def test_step_5_a_junk_positive_is_never_stored(self): + """Pinned: the positive alone still passes the junk gate, and a blank + positive (never storable) stores nothing either.""" + graph = save_tests.run_graph(save=True, negative_save=False) + graph["1"]["inputs"]["text"] = "2048" + decision = _decide(_graph(graph)) + self.assertEqual((decision.kind, decision.source), ("never", "junk:number")) + self.assertEqual(decision.unlinked_status, "found") + graph["1"]["inputs"]["text"] = "" + decision = _decide(_graph(graph)) + self.assertEqual((decision.kind, decision.text), ("never", None)) + + def test_step_2_a_workflow_only_opt_out_comes_before_step_3(self): + workflow = { + "nodes": [{"id": 1, "type": "PromptManager", "widgets_values": [False]}] + } + decision = _decide({"workflow": json.dumps(workflow)}) + self.assertEqual(decision.kind, "not_saved") + + def test_step_5_an_opted_out_negative_stores_the_positive_alone(self): + graph = save_tests.run_graph(save=True, negative_save=False) + decision = _decide(_graph(graph)) + self.assertEqual(decision.kind, "positive") + self.assertEqual(decision.source, "opted-out-negative") + self.assertEqual(decision.text, save_tests.POSITIVE) + + +class TestResolvedSteps(unittest.TestCase): + def test_step_6_a_blank_positive_counts_as_unresolved(self): + meta = { + "prompt": json.dumps(issue_graph()), + "comfyassets_prompt_pairs": json.dumps(pair_evidence()), + } + decision = _decide(meta, policy=Policy.MONITOR, in_output=False) + self.assertEqual(decision.kind, "never") + self.assertEqual(decision.resolution["state"], "unresolved") + + def test_step_7_a_resolved_pair(self): + lookup = _Lookup("9") + decision = _decide(_graph(sampled_graph("cat", "blur")), lookup=lookup) + self.assertEqual(decision.kind, "pair") + self.assertEqual(decision.source, "resolver") + self.assertEqual(decision.text, "cat") + self.assertEqual(decision.resolution["negative"]["text"], "blur") + self.assertIsNone(decision.review) + self.assertEqual(lookup.calls, 0, "decided before step 8: history not asked") + + def test_step_7_a_blank_negative_stays_a_pair(self): + decision = _decide(_graph(sampled_graph("cat", ""))) + self.assertEqual(decision.kind, "pair") + self.assertEqual(decision.resolution["negative"]["text"], "") + + +class TestUndecidedGraph(unittest.TestCase): + def test_step_8_1_the_inspector_alone_decides(self): + decision = _decide({"prompt": parity.build("f03")["prompt"]}) + self.assertEqual(decision.kind, "positive") + self.assertEqual(decision.source, "inspector") + self.assertEqual(decision.text, parity.POS) + self.assertEqual(decision.resolution["state"], "unresolved") + + def test_step_8_1_the_recorded_saver_names_its_branch(self): + lookup = _Lookup("23") + decision = _decide(_graph(shapes.shared_prefix()), lookup=lookup) + self.assertEqual(decision.kind, "positive") + self.assertEqual(decision.source, "recorded-run") + self.assertEqual(decision.text, shapes.BRANCH_TEXTS[1]) + self.assertEqual(decision.review, "ambiguous") + self.assertEqual(lookup.calls, 1) + + def test_step_8_1_stores_the_storage_text_without_prepend_or_append(self): + graph = shapes.shared_prefix() + graph["20"]["inputs"]["text"] = ["25", 1] + graph["25"] = { + "class_type": "PromptManagerSplit", + "inputs": { + "text": shapes.BRANCH_TEXTS[1], + "prepend_text": "trigger", + "append_text": "detail", + }, + } + decision = _decide(_graph(graph), lookup=_Lookup("23")) + self.assertEqual(decision.source, "recorded-run") + self.assertEqual(decision.text, shapes.BRANCH_TEXTS[1]) + + def test_step_8_1_a_file_name_naming_another_saver_anchors_nothing(self): + graph = shapes.branch_bench() + for index in (1, 3, 4): + graph[str(index * 10 + 3)]["inputs"]["filename_prefix"] = "bench/shared" + decision = _decide( + _graph(graph), lookup=_Lookup("43"), file_name="s45_branch_2_00001_.png" + ) + # The name contradicts history's branch 4: no recorded anchor. + self.assertEqual(decision.source, "legacy-parser") + + def test_step_8_1_the_file_name_anchor_alone_never_decides(self): + """#390: the file-name anchor never decides a link. The scan's legacy + floor may still link the file (as before #247), and a file nothing + links is queued with the anchor's choice per #390; the monitor + stores nothing.""" + name = "s45_branch_2_00001_.png" + scan = _decide(_graph(shapes.branch_bench()), lookup=_Lookup(), file_name=name) + self.assertEqual((scan.kind, scan.source), ("positive", "legacy-parser")) + self.assertEqual(scan.review, "ambiguous") + monitor = _decide( + _graph(shapes.branch_bench()), + policy=Policy.MONITOR, + lookup=_Lookup(), + file_name=name, + ) + self.assertEqual(monitor.kind, "never") + recorded = _decide( + _graph(shapes.branch_bench()), lookup=_Lookup("43"), file_name=name + ) + # The recorded saver still decides: history says branch 4, but the + # file name names branch 2, which contradicts it. + self.assertNotEqual(recorded.source, "recorded-run") + recorded = _decide(_graph(shapes.branch_bench()), lookup=_Lookup("43")) + self.assertEqual(recorded.source, "recorded-run") + self.assertEqual(recorded.text, shapes.BRANCH_TEXTS[3]) + + def test_step_8_2_the_monitor_waits_for_history_inside_the_output_folder(self): + decision = _decide(_graph(shapes.shared_prefix()), policy=Policy.MONITOR) + self.assertEqual(decision.kind, "history") + + def test_step_8_2_outside_the_output_folder_steps_8_3_and_8_4_apply(self): + decision = _decide( + _graph(shapes.shared_prefix()), policy=Policy.MONITOR, in_output=False + ) + self.assertEqual(decision.kind, "never") + with _untraced_by_both(): + decision = _decide( + _graph(_gated(sampled_graph())), policy=Policy.MONITOR, in_output=False + ) + self.assertEqual( + (decision.kind, decision.source), ("positive", "legacy-parser") + ) + + def test_step_8_3_an_ambiguous_file_is_left_unlinked_by_the_monitor(self): + lookup = _Lookup("missing") + decision = _decide( + _graph(shapes.shared_prefix()), policy=Policy.MONITOR, lookup=lookup + ) + self.assertEqual(decision.kind, "never") + self.assertEqual(decision.unlinked_status, "ambiguous") + self.assertFalse(decision.try_recorded_run) + self.assertEqual(lookup.calls, 1) + + def test_step_8_4_joined_prompt_manager_text_is_never_the_legacy_answer(self): + """The legacy parser returns prepend + text + append; the stored text + of a PromptManager node cannot be told from it.""" + decision = _decide(_graph(_prompt_manager_branches()), lookup=_Lookup()) + self.assertEqual(decision.kind, "never") + self.assertEqual(decision.unlinked_status, "ambiguous") + self.assertEqual(decision.review, "ambiguous") + self.assertTrue(decision.try_recorded_run) + # A linked part the parser does not follow: its answer is the node's + # own text, which is what the node stores (#170 parity). + linked = _prompt_manager_branches(prepend=["99", 0], append="") + decision = _decide(_graph(linked), lookup=_Lookup()) + self.assertEqual((decision.kind, decision.text), ("positive", "a cat")) + + def test_step_8_4_prompt_manager_text_without_parts_keeps_the_floor(self): + """Regression guard (passes on the base): no prepend or append.""" + graph = _prompt_manager_branches(prepend="", append=" ") + decision = _decide(_graph(graph), lookup=_Lookup()) + self.assertEqual((decision.kind, decision.text), ("positive", "a cat")) + + def test_step_8_4_an_ambiguous_file_gets_the_legacy_answer_in_the_scan(self): + decision = _decide(_graph(shapes.shared_prefix()), lookup=_Lookup()) + self.assertEqual(decision.kind, "positive") + self.assertEqual(decision.source, "legacy-parser") + self.assertEqual(decision.text, shapes.BRANCH_TEXTS[0]) + self.assertEqual(decision.review, "ambiguous") + + def test_step_8_4_an_untraceable_file_gets_the_legacy_answer_in_both(self): + for policy in Policy: + for inspected in (_inspected("unresolved"), _inspected("x", True)): + with ( + self.subTest(policy=policy, malformed=inspected.malformed), + mock.patch.object( + file_prompt, "inspection_resolution", return_value=inspected + ), + ): + decision = _decide( + _graph(_gated(sampled_graph("cat"))), + policy=policy, + lookup=_Lookup(), + ) + self.assertEqual(decision.kind, "positive") + self.assertEqual(decision.source, "legacy-parser") + self.assertEqual(decision.text, "cat") + + def test_step_8_4_nothing_traced_tries_the_recorded_run(self): + graph = sampled_graph() + graph["5"] = {**graph["3"]} + for sampler in ("3", "5"): + graph[sampler]["inputs"]["positive"] = ["9", 0] + graph["9"] = {"class_type": "BasicGuider", "inputs": {"width": 2048}} + decision = _decide(_graph(graph), lookup=_Lookup()) + self.assertEqual(decision.kind, "never") + self.assertTrue(decision.try_recorded_run) + # Ambiguous without a choice is reported unresolved. + self.assertEqual(decision.unlinked_status, "unresolved") + self.assertEqual(decision.review, "untraced") + + +class TestStepsWithoutAnUndecidedGraph(unittest.TestCase): + def test_step_9_ambiguous_parameters_only(self): + meta = { + "parameters": ( + "cat\nNegative prompt: blur\nNegative prompt: noise" + "\nSteps: 20, Sampler: Euler" + ) + } + decision = _decide(meta) + self.assertEqual(decision.kind, "never") + self.assertFalse(decision.try_recorded_run) + self.assertEqual(decision.unlinked_status, "unresolved") + + def test_step_10_unresolved_without_a1111_text(self): + decision = _decide({"prompt": {}, "parameters": 5}) + self.assertEqual(decision.kind, "never") + self.assertFalse(decision.try_recorded_run) + self.assertEqual(decision.unlinked_status, "unresolved") + + def test_step_11_sampler_less_with_one_text(self): + decision = _decide(_graph(flux_graph())) + self.assertEqual((decision.kind, decision.source), ("positive", "sampler-less")) + self.assertEqual(decision.text, "a red fox") + + def test_step_11_sampler_less_with_several_texts(self): + graph = flux_graph() + graph["50"] = { + "class_type": "Lora Loader (LoraManager)", + "inputs": {"model": ["1", 0], "clip": ["2", 0], "text": "a lora note"}, + } + graph["22"]["inputs"]["model"] = ["50", 0] + graph["6"]["inputs"]["clip"] = ["50", 1] + decision = _decide(_graph(graph)) + self.assertEqual(decision.kind, "never") + self.assertTrue(decision.try_recorded_run) + self.assertEqual(decision.unlinked_status, "unresolved") + meta = {**_graph(graph), "parameters": "a red fox\nNegative prompt: x"} + decision = _decide(meta) + self.assertEqual((decision.kind, decision.source), ("positive", "a1111")) + self.assertEqual(decision.text, "a red fox") + + def test_step_11_sampler_less_without_a_text(self): + graph = {"40": {"class_type": "easy showAnything", "inputs": {"text": "x"}}} + decision = _decide(_graph(graph)) + self.assertEqual(decision.kind, "never") + self.assertTrue(decision.try_recorded_run) + self.assertEqual(decision.unlinked_status, "found") + + def test_step_12_a1111_only(self): + decision = _decide({"parameters": "a cat"}) + self.assertEqual((decision.kind, decision.source), ("positive", "a1111")) + self.assertEqual(decision.text, "a cat") + decision = _decide({"parameters": "Negative prompt: blur"}) + self.assertEqual((decision.kind, decision.unlinked_status), ("never", "found")) + self.assertFalse(decision.try_recorded_run) + + def test_step_12_parsed_metadata_never_raises(self): + # The monitor hands parsed values: a dict prompt, non-string parameters. + decision = _decide({"prompt": {}, "parameters": "a cat"}) + self.assertEqual(decision.text, "a cat") + decision = _decide({"parameters": 5}) + self.assertEqual((decision.kind, decision.unlinked_status), ("never", "found")) + + +class TestJunkGate(unittest.TestCase): + JUNK = { + "blank": " ", + "number": "2048", + "lora-only": f"{LORA}, |", + "model-file": "models/checkpoints/Flux-Dev.SafeTensors", + } + + def test_each_junk_class_is_named(self): + for name, text in self.JUNK.items(): + with self.subTest(name=name): + self.assertEqual(junk_text(text), name) + for ending in ("ckpt", "pt", "pth", "bin", "gguf", "sft"): + self.assertEqual(junk_text(f"model.{ending}"), "model-file") + + def test_near_misses_are_not_junk(self): + for text in ( + f"a red fox {LORA}", + "a.safetensors b.safetensors", + "nan", + "a portrait saved as final.safetensors", + "4k", + "C:\\My Models\\x.safetensors", + ): + with self.subTest(text=text): + self.assertIsNone(junk_text(text)) + + def test_step_13_junk_from_any_step_is_never_stored(self): + cases = { + "resolver": _graph(sampled_graph("2048", "blur")), + "sampler-less": _graph(flux_graph(text=LORA)), + "a1111": {"parameters": "model.safetensors"}, + } + for step, meta in cases.items(): + with self.subTest(step=step): + decision = _decide(meta) + self.assertEqual(decision.kind, "never") + self.assertTrue(decision.source.startswith("junk:"), decision.source) + self.assertTrue(decision.try_recorded_run) + self.assertEqual(decision.unlinked_status, "found") + + def test_step_13_a_junk_legacy_answer_is_never_stored(self): + with _untraced_by_both(): + decision = _decide(_graph(_gated(sampled_graph(LORA))), lookup=_Lookup()) + self.assertEqual(decision.source, "junk:lora-only") + self.assertEqual(decision.kind, "never") + self.assertEqual(decision.unlinked_status, "unresolved") + + def test_step_13_junk_at_step_8_keeps_that_steps_status_and_review(self): + # An ambiguous file whose legacy answer is a number is still a + # Needs review case: its status stays "ambiguous". + decision = _decide(_graph(_numeric_first_branches()), lookup=_Lookup()) + self.assertEqual((decision.kind, decision.source), ("never", "junk:number")) + self.assertEqual(decision.unlinked_status, "ambiguous") + self.assertEqual(decision.review, "ambiguous") + self.assertTrue(decision.try_recorded_run) + + +class TestParseComfyuiPrompt(unittest.TestCase): + def test_a_parsed_prompt_and_non_string_values_are_kept(self): + parsed = file_prompt.parse_comfyui_prompt( + {"prompt": {"1": {}}, "workflow": {"nodes": []}, "steps": 20} + ) + self.assertEqual(parsed["prompt"], {"1": {}}) + self.assertEqual(parsed["workflow"], {"nodes": []}) + self.assertEqual(parsed["parameters"]["steps"], 20) + self.assertIsNone(parsed["positive_prompt"]) + + def test_non_string_parameters_are_ignored(self): + parsed = file_prompt.parse_comfyui_prompt({"parameters": {"a": 1}}) + self.assertIsNone(parsed["positive_prompt"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_image_tag_jobs.py b/tests/test_image_tag_jobs.py index f593aaf5..b555c79a 100644 --- a/tests/test_image_tag_jobs.py +++ b/tests/test_image_tag_jobs.py @@ -299,6 +299,19 @@ class TestAutoTag(ImageTagJobTestCase): self.tagger.download_model.assert_not_called() self.assertIsNone(model_gate.holder()) + async def test_a_missing_backend_package_fails_the_job_before_the_first_file(self): + self.png("a.png") + self.tagger.missing["wd14-vit"] = ["onnxruntime"] + + with self.assertNoLogs(self.api.logger, level="ERROR"): + job = await self.run_job(self.auto_body(["a.png"])) + + self.assertEqual(job.state, "failed") + self.assertEqual(job.processed, 0) + self.assertTrue(job.status_line.startswith("WD14 ViT needs onnxruntime.")) + self.assertIn("pip install onnxruntime", job.status_line) + self.assertEqual(self.tagger.load_calls, []) + async def test_model_unavailable_fails_the_job_before_the_first_file(self): self.png("a.png") self.tagger.downloaded["wd14-vit"] = False diff --git a/tests/test_ingestion_resolver_profile.py b/tests/test_ingestion_resolver_profile.py index 2b96ced0..e22613b3 100644 --- a/tests/test_ingestion_resolver_profile.py +++ b/tests/test_ingestion_resolver_profile.py @@ -1,9 +1,11 @@ -"""Ingestion keeps the default resolver profile; only inspection routes moved (D1). +"""Ingestion decides with the default resolver profile first (D1, #247 D-Q1). The live monitor and the admin scan write pairs and links to the user's -database, so their resolver semantics stay byte-for-byte today's. f03 (506 -executable nodes) separates the profiles: the default profile's 500-node gate -leaves it unresolved, while the inspection profile resolves it. +database, so the default profile decides every file it can, byte-for-byte as +before. f03 (506 executable nodes) separates the profiles: the default +profile's 500-node gate leaves it unresolved, while the inspection profile +resolves it. Only such an undecided file reaches the inspector's resolve +(``inspection_resolution``), and only its positive is stored (#247). """ import logging @@ -68,24 +70,26 @@ class TestIngestionKeepsDefaultProfile(unittest.TestCase): spy.assert_default_and_gated(self) handler.db_manager.link_image_to_prompt_or_raise.assert_called_once() - def test_admin_scan_ingestion_uses_the_default_profile(self): + def test_admin_scan_ingestion_uses_the_default_profile_first(self): from py.api import admin + from utils import file_prompt mixin = admin.AdminRoutesMixin.__new__(admin.AdminRoutesMixin) mixin.db = Mock() - mixin.db.get_prompt_by_hash.return_value = None - mixin.db.save_prompt.return_value = 7 spy = _Spy() - with patch.object(admin, "resolve_prompt_pair", spy): + with ( + patch.object(file_prompt, "resolve_prompt_pair", spy), + patch.object( + admin, "store_scanned_prompt", return_value=("added", 7) + ) as store, + ): status = mixin._ingest_scanned_file(Path("/out/f03.png"), _f03_metadata()) spy.assert_default_and_gated(self) - # The resolver leaves f03 unresolved; the scan links the prompt the - # browser parser traces, as the inspector shows it. + # The default profile leaves f03 unresolved; the inspector's resolve + # names the prompt the inspector shows, and the scan stores it. self.assertEqual(status, "added") - self.assertEqual( - mixin.db.save_prompt.call_args.args[0], "a red fox in snow, detailed fur" - ) - mixin.db.link_image_to_prompt.assert_called_once() + self.assertEqual(store.call_args.args[3], "a red fox in snow, detailed fur") + mixin.db.link_image_to_prompt.assert_not_called() if __name__ == "__main__": diff --git a/tests/test_pair_scan.py b/tests/test_pair_scan.py index 2ee70151..3f91f41d 100644 --- a/tests/test_pair_scan.py +++ b/tests/test_pair_scan.py @@ -1,9 +1,12 @@ """Metadata scanning enriches known pairs and conservatively leaves conflicts.""" import json +import sqlite3 from pathlib import Path +from unittest.mock import patch from tests.test_api_admin import AdminAPITestCase +from utils.hashing import generate_prompt_hash def graph(positive="cat", negative="blur"): @@ -449,3 +452,159 @@ class TestSamplerLessScan(AdminAPITestCase): path = self.output_dir / "saver.png" self.assertEqual(self.api._ingest_scanned_file(path, meta), "added") self.assertEqual(self._texts(), ["a red fox"]) + + +class TestScanDecision(AdminAPITestCase): + """The scan applies the shared per-file decision (#247): one prompt per + file, no junk text, marks on every prompt it creates.""" + + def _path(self, name): + path = self.output_dir / name + path.write_bytes(b"png") + return path + + def _links(self, path): + rows = self.api.db.model.get_connection().execute( + "SELECT prompt_id FROM generated_images WHERE image_path=?", (str(path),) + ) + return [row[0] for row in rows] + + def _prompt(self, text): + return self.api.db.get_prompt_by_hash(generate_prompt_hash(text)) + + def test_a_file_linked_to_another_prompt_is_a_conflict(self): + path = self._path("fox.png") + other = self.api.db.save_prompt( + "a dog", prompt_hash=generate_prompt_hash("a dog") + ) + self.api.db.link_image_to_prompt(other, str(path)) + meta = {"prompt": json.dumps(flux_graph())} + self.assertEqual(self.api._ingest_scanned_file(path, meta), "conflict") + self.assertEqual(self._links(path), [other]) + self.assertIsNone(self._prompt("a red fox")) + + def test_junk_text_is_never_stored(self): + cases = { + "lora.png": {"prompt": json.dumps(flux_graph(text=""))}, + "model.png": {"parameters": "models/flux-dev.safetensors"}, + } + for name, meta in cases.items(): + with self.subTest(name=name): + path = self._path(name) + self.assertEqual(self.api._ingest_scanned_file(path, meta), "found") + self.assertEqual(self._links(path), []) + self.assertEqual(self.api.db.get_recent_prompts()["prompts"], []) + + def test_created_prompts_carry_the_scan_marks(self): + path = self._path("pair.png") + meta = {"prompt": json.dumps(sampled_graph("cat", "blur"))} + self.assertEqual(self.api._ingest_scanned_file(path, meta), "added") + for text in ("cat", "blur"): + prompt = self._prompt(text) + self.assertEqual(prompt["category"], "scanned") + self.assertEqual(prompt["tags"], ["auto-scanned"]) + self.assertEqual(prompt["notes"], "Auto-scanned from pair.png") + + def test_outcome_of_a_positive_reports_its_title(self): + """Regression guard (passes on the base): the detailed positive row.""" + path = self._path("fox.png") + meta = {"prompt": json.dumps(flux_graph())} + first = self.api._ingest_scanned_file(path, meta, detailed=True) + self.assertEqual(first, {"status": "added", "title": "a red fox"}) + again = self.api._ingest_scanned_file(path, meta, detailed=True) + self.assertEqual(again["status"], "unchanged") + other = self._path("fox2.png") + linked = self.api._ingest_scanned_file(other, meta, detailed=True) + self.assertEqual(linked["status"], "linked") + + def test_outcome_of_a_pair_reports_its_counts(self): + """Regression guard (passes on the base): the detailed pair row.""" + path = self._path("pair.png") + meta = {"prompt": json.dumps(sampled_graph("cat", "blur"))} + result = self.api._ingest_scanned_file(path, meta, detailed=True) + self.assertEqual(result["status"], "added") + self.assertEqual((result["pairs_added"], result["links_added"]), (1, 1)) + self.assertEqual(result["title"], "cat") + + def test_outcome_without_a_store_reports_the_unlinked_status(self): + """Regression guard (passes on the base): never-stored rows.""" + path = self._path("notes.png") + meta = {"prompt": json.dumps({"40": {"class_type": "Textbox", "inputs": {}}})} + self.assertEqual(self.api._ingest_scanned_file(path, meta), "found") + self.assertIsNone(self.api._ingest_scanned_file(path, {})) + + def test_review_lists_get_the_default_profiles_ambiguous_files(self): + """Regression guard (passes on the base): Needs review inputs.""" + g = sampled_graph() + g["6"] = {"class_type": "CLIPTextEncode", "inputs": {"text": "dog"}} + g["12"] = {**g["3"], "inputs": {**g["3"]["inputs"], "positive": ["6", 0]}} + ambiguous, untraced, records = [], [], {} + path = self._path("two.png") + status = self.api._ingest_scanned_file( + path, + {"prompt": json.dumps(g)}, + ambiguous=ambiguous, + untraced=untraced, + records=records, + ) + # Linked by the legacy floor, still listed (queued only if unlinked). + self.assertEqual(status, "added") + self.assertEqual((ambiguous, untraced), ([path], [])) + self.assertEqual(records["canonical"]["prompt"], g) + + def test_an_ambiguous_file_with_a_junk_answer_is_still_queued(self): + from tests.test_file_prompt import _numeric_first_branches + + path = self._path("numbers.png") + meta = {"prompt": json.dumps(_numeric_first_branches())} + with patch.object( + type(self.api), "_record_scanned_review", autospec=True + ) as record: + counts = self.api._ingest_scan_batch_sync([(path, meta)]) + self.assertEqual(counts["ambiguous"], 1, counts) + record.assert_called_once() + self.assertEqual(self._links(path), []) + + def test_a_conflict_row_says_the_link_was_kept(self): + from py.api.admin import _scan_outcome + + _group, row, _line = _scan_outcome(self._path("c.png"), [], "conflict") + self.assertEqual(row["detail"], "Kept: already linked to a different prompt") + + def test_a_library_move_reruns_the_scan_writes(self): + from py.api import admin + + real = admin.ingest_resolved_pair + model = self.api.db.model + + def moved_once(*args, **kwargs): + if not calls: + calls.append(1) + model._generation += 1 + raise sqlite3.OperationalError("closed") + return real(*args, **kwargs) + + calls = [] + path = self._path("pair.png") + with patch.object(admin, "ingest_resolved_pair", side_effect=moved_once): + meta = {"prompt": json.dumps(sampled_graph("cat", "blur"))} + self.assertEqual(self.api._ingest_scanned_file(path, meta), "added") + self.assertEqual(calls, [1]) + + def test_an_off_node_behind_a_pass_through_stores_nothing(self): + """#90 regression guard for the scan (main stored nothing here).""" + from tests.test_save_to_library import run_graph + + g = run_graph(save=False) + g["1"]["class_type"] = "PromptManagerText" + g["7"] = {"class_type": "easy showAnything", "inputs": {"anything": ["1", 0]}} + g["8"] = { + "class_type": "CLIPTextEncode", + "inputs": {"text": ["7", 0], "clip": ["4", 1]}, + } + g["3"]["inputs"]["positive"] = ["8", 0] + path = self._path("off.png") + meta = {"prompt": json.dumps(g)} + self.assertEqual(self.api._ingest_scanned_file(path, meta), "not_saved") + self.assertEqual(self.api.db.get_recent_prompts()["prompts"], []) + self.assertEqual(self._links(path), []) diff --git a/tests/test_recorded_saver_anchor.py b/tests/test_recorded_saver_anchor.py index 3ca852c6..b8df3731 100644 --- a/tests/test_recorded_saver_anchor.py +++ b/tests/test_recorded_saver_anchor.py @@ -375,5 +375,89 @@ class TestRecordedTexts(unittest.TestCase): self.assertEqual(evidence["resolution"]["sampler_id"], sampler) +class TestInspectionResolution(unittest.TestCase): + """``inspection_resolution``: the inspector's resolve and recorded anchor, + shared with ingestion (#247), so the two cannot drift.""" + + def _result(self, graph, saver=None, *, as_text=True, name="IMG_GEN_00371_.png"): + from utils.prompt_inspector import inspection_resolution + + prompt = json.dumps(graph) if as_text else graph + return inspection_resolution({"prompt": prompt}, name, saver) + + def test_the_recorded_saver_names_its_branch_by_record(self): + result = self._result(shapes.shared_prefix(), _saver(2)) + self.assertTrue(result.by_record) + self.assertFalse(result.malformed) + self.assertEqual(result.resolution["state"], "resolved") + self.assertEqual(_positive(result.resolution), shapes.BRANCH_TEXTS[1]) + self.assertIs(result.anchored, result.resolution) + + def test_without_a_saver_nothing_is_anchored(self): + result = self._result(shapes.shared_prefix()) + self.assertEqual(result.resolution["state"], "ambiguous") + self.assertIsNone(result.anchored) + self.assertFalse(result.by_record) + + def test_a_saver_that_names_nothing_keeps_the_open_result(self): + result = self._result(shapes.shared_prefix(), "missing") + self.assertEqual(result.resolution["state"], "ambiguous") + self.assertIsNotNone(result.anchored) + self.assertFalse(result.by_record) + + def test_a_file_that_resolves_alone_is_never_anchored(self): + result = self._result( + shapes.branch_bench(), _saver(4), name="s45_branch_2_00001_.png" + ) + self.assertEqual(_positive(result.resolution), shapes.BRANCH_TEXTS[1]) + self.assertIsNone(result.anchored) + self.assertFalse(result.by_record) + + def test_parsed_metadata_resolves_as_the_file_text_does(self): + # The monitor hands parsed dicts; the scan hands the file's strings. + parsed = self._result(shapes.shared_prefix(), _saver(3), as_text=False) + text = self._result(shapes.shared_prefix(), _saver(3)) + self.assertEqual(parsed.resolution, text.resolution) + + def test_malformed_metadata_is_reported_as_malformed(self): + from utils.prompt_inspector import inspection_resolution + + result = inspection_resolution({"prompt": "{not json"}, None, None) + self.assertTrue(result.malformed) + self.assertEqual(result.resolution["state"], "unresolved") + self.assertEqual(result.resolution["reason_code"], "malformed") + + def test_metadata_without_a_role_record_is_absent(self): + from utils.prompt_inspector import inspection_resolution + + result = inspection_resolution({"workflow": "{}"}, None, "9") + self.assertFalse(result.malformed) + self.assertEqual(result.resolution["state"], "absent") + + def test_without_the_name_anchor_a_file_name_names_no_branch(self): + from utils.prompt_inspector import inspection_resolution + + graph = {"prompt": json.dumps(shapes.branch_bench())} + name = "s45_branch_2_00001_.png" + named = inspection_resolution(graph, name, None) + self.assertEqual(_positive(named.resolution), shapes.BRANCH_TEXTS[1]) + blind = inspection_resolution(graph, name, None, name_anchor=False) + self.assertEqual(blind.resolution["state"], "ambiguous") + # The recorded saver still decides, and the name still contradicts it. + recorded = inspection_resolution(graph, name, _saver(2), name_anchor=False) + self.assertTrue(recorded.by_record) + shared = {"prompt": json.dumps(shapes.shared_prefix())} + bench = shapes.branch_bench() + for index in (1, 3, 4): + bench[_saver(index)]["inputs"]["filename_prefix"] = "bench/shared" + contradicted = inspection_resolution( + {"prompt": json.dumps(bench)}, name, _saver(4), name_anchor=False + ) + self.assertFalse(contradicted.by_record) + self.assertTrue( + inspection_resolution(shared, name, _saver(4), name_anchor=False).by_record + ) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_run_attribution.py b/tests/test_run_attribution.py index 050d710a..dbc05d53 100644 --- a/tests/test_run_attribution.py +++ b/tests/test_run_attribution.py @@ -17,6 +17,8 @@ from database.run_attribution import ( from tests import test_execution_history_database as history_tests from tests.background_stops import close_handler from tests.test_api_admin import AdminAPITestCase +from tests import saver_anchor_graphs as shapes +from tests.test_pair_scan import sampled_graph from tests.test_execution_context_database import known, snapshot from tests.test_prompt_attribution import issue_graph, llm_text_graph, make_tracker from utils.execution_contexts import ( @@ -326,7 +328,7 @@ class TestRecordedOutputNode(RecordedRuns, unittest.TestCase): self.assertIsNone(recorded_output_lookup(broken)(self.image)) with ( mock.patch( - "database.run_attribution._recorded_runs", + "database.run_attribution.recorded_runs", side_effect=sqlite3.OperationalError("locked"), ), self.assertLogs("prompt_manager.run_attribution", "WARNING"), @@ -451,6 +453,86 @@ class TestScanUsesHistory(RecordedRuns, AdminAPITestCase): self.assertIsNone(self.linked_prompt(path)) self.assertEqual(db.list_prompt_pairs()["pairs"], []) + def scan_dataset(self, names, nodes): + """Two branches of the shared-prefix dataset graph, one run, then a scan.""" + paths = [self.write(name) for name in names] + self.record_run( + self.api.db, + str(self.output_dir), + [("", path.name) for path in paths], + [], + nodes=nodes, + ) + meta = {"prompt": json.dumps(shapes.shared_prefix())} + return paths, self.api._ingest_scan_batch_sync([(p, meta) for p in paths]) + + def test_the_recorded_saver_links_each_dataset_file_to_its_branch(self): + """#247: before, the legacy parser gave every branch the first prompt.""" + paths, counts = self.scan_dataset( + ["IMG_GEN_00001_.png", "IMG_GEN_00002_.png"], ["13", "23"] + ) + self.assertEqual(counts["added"], 2, counts) + for path, text in zip(paths, shapes.BRANCH_TEXTS): + prompt = self.api.db.get_prompt_by_hash(generate_prompt_hash(text)) + self.assertEqual(self.linked_prompt(path), prompt["id"]) + self.assertEqual(prompt["category"], "scanned") + + def test_a_file_the_legacy_answer_linked_is_left_as_it_is(self): + """#247 Q5: a rescan never adds a second link to the branch prompt.""" + db = self.api.db + legacy = shapes.BRANCH_TEXTS[0] + legacy_id = db.save_prompt(legacy, prompt_hash=generate_prompt_hash(legacy)) + path = self.write("IMG_GEN_00002_.png") + db.link_image_to_prompt(legacy_id, str(path)) + self.record_run(db, str(self.output_dir), [("", path.name)], [], nodes=["23"]) + meta = {"prompt": json.dumps(shapes.shared_prefix())} + self.assertEqual(self.api._ingest_scanned_file(path, meta), "conflict") + self.assertEqual(self.linked_prompt(path), legacy_id) + self.assertIsNone( + db.get_prompt_by_hash(generate_prompt_hash(shapes.BRANCH_TEXTS[1])) + ) + + def test_a_recorded_run_never_adds_a_second_link(self): + db = self.api.db + db.save_prompt(text=LLM_TEXT, prompt_hash=generate_prompt_hash(LLM_TEXT)) + other = db.save_prompt(text="a dog", prompt_hash=generate_prompt_hash("a dog")) + path = self.write("bare_00003_.png") + db.link_image_to_prompt(other, str(path)) + self.record_run(db, str(self.output_dir), [("", path.name)], [LLM_TEXT]) + self.assertEqual(self.api._ingest_scanned_file(path, {}), "conflict") + rows = db.model.get_connection().execute( + "SELECT prompt_id FROM generated_images WHERE image_path=?", (str(path),) + ) + self.assertEqual([row[0] for row in rows], [other]) + + def test_a_single_file_asks_history_once(self): + db = self.api.db + db.save_prompt(text=LLM_TEXT, prompt_hash=generate_prompt_hash(LLM_TEXT)) + path = self.write("llm_00002_.png") + self.record_run(db, str(self.output_dir), [("", path.name)], [LLM_TEXT]) + meta = {"prompt": json.dumps(llm_text_graph())} + statements = [] + conn = db.model.get_connection() + conn.set_trace_callback(statements.append) + try: + self.assertEqual(self.api._ingest_scanned_file(path, meta), "linked") + finally: + conn.set_trace_callback(None) + lookups = [s for s in statements if "FROM execution_run_outputs" in s] + self.assertEqual(len(lookups), 1) + + def test_a_file_the_recorded_saver_links_is_not_queued(self): + """A default-ambiguous file linked at the inspector's step is no + Needs review case.""" + with mock.patch.object( + type(self.api), "_record_scanned_review", autospec=True + ) as record: + _paths, counts = self.scan_dataset( + ["IMG_GEN_00003_.png", "IMG_GEN_00004_.png"], ["13", "23"] + ) + self.assertEqual(counts["added"], 2, counts) + record.assert_not_called() + class TestHostHelpers(unittest.TestCase): """The host lookups degrade to "no proof", never to an error.""" @@ -752,13 +834,17 @@ class TestScanBatchLookup(RecordedRuns, AdminAPITestCase): """M5: a scan batch asks history once per chunk of names, not per file.""" def test_a_batch_its_metadata_links_never_asks_history(self): - """R3: the batch lookup is made only when a file needs it.""" + """R3: the batch lookup is made only when a file needs it. + + Since #247 every file the default profile leaves undecided asks (the + recorded saver may name its branch), so these files resolve as pairs. + """ files = [] for n in range(3): path = self.output_dir / f"typed_{n:05}_.png" path.write_bytes(b"png") files.append(path) - meta = {"prompt": json.dumps(issue_graph(text="typed mountains"))} + meta = {"prompt": json.dumps(sampled_graph("typed mountains"))} statements = [] conn = self.api.db.model.get_connection() conn.set_trace_callback(statements.append) @@ -802,6 +888,72 @@ class TestScanBatchLookup(RecordedRuns, AdminAPITestCase): } self.assertEqual(linked, {f.name for f in files[:4]}) + def test_the_batch_saver_equals_the_files_recorded_output(self): + from py.api.admin import _BatchHistory + + db = self.api.db + names = ["one_node.png", "two_nodes.png", "two_runs.png", "unrecorded.png"] + files = [] + for name in names: + path = self.output_dir / name + path.write_bytes(b"png") + files.append(path) + root = str(self.output_dir) + self.record_run(db, root, [("", "one_node.png")], [], nodes=["13"]) + self.record_run( + db, root, [("", "two_nodes.png")] * 2, [], item=1, nodes=["13", "23"] + ) + self.record_run(db, root, [("", "two_runs.png")], [], item=2) + self.record_run(db, root, [("", "two_runs.png")], [], item=3) + history = _BatchHistory(self.api, files) + with mock.patch( + "database.run_attribution.host_output_directory", return_value=root + ): + for path in files: + with self.subTest(path=path.name): + expected = recorded_output(db, str(path), root) + found = history.recorded(str(path)) + if expected is None: + self.assertIsNone(found) + else: + self.assertEqual(found.saver_id, expected.saver_id) + self.assertEqual(history.recorded(str(files[0])).saver_id, "13") + self.assertIsNone(history.recorded(str(files[1])).saver_id) + + def test_a_failed_batch_lookup_is_logged_and_names_nothing(self): + from py.api.admin import _BatchHistory + + path = self.output_dir / "x.png" + history = _BatchHistory(self.api, [path]) + with ( + mock.patch( + "py.api.admin.recorded_runs", side_effect=sqlite3.OperationalError("x") + ), + self.assertLogs("prompt_manager", "WARNING"), + ): + self.assertIsNone(history.recorded(str(path))) + self.assertIsNone(history.get(str(path))) + + def test_a_failed_prompt_lookup_is_logged_and_names_nothing(self): + from py.api.admin import _BatchHistory + + path = self.output_dir / "one.png" + path.write_bytes(b"png") + root = str(self.output_dir) + self.record_run(self.api.db, root, [("", path.name)], [LLM_TEXT]) + history = _BatchHistory(self.api, [path]) + with ( + mock.patch( + "database.run_attribution.host_output_directory", return_value=root + ), + mock.patch( + "py.api.admin.named_prompt", side_effect=sqlite3.OperationalError("x") + ), + self.assertLogs("prompt_manager", "WARNING"), + ): + self.assertIsNone(history.get(str(path))) + self.assertEqual(history.recorded(str(path)).saver_id, "9") + class TestLinkedOutputRoot(RecordedRuns, unittest.TestCase): """R2: ComfyUI's output folder reached through a symlink or junction.""" diff --git a/tests/test_scanned_prompts.py b/tests/test_scanned_prompts.py new file mode 100644 index 00000000..c256e6c2 --- /dev/null +++ b/tests/test_scanned_prompts.py @@ -0,0 +1,274 @@ +"""Storing a file's prompt from its text: one transaction, one prompt each (#247).""" + +import os +import sqlite3 +import tempfile +import threading +import unittest +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path +from unittest import mock + +try: + from tests.open_handles import assert_closed + from tests import test_execution_history_database as history_tests +except ImportError: # discovered with tests/ as the top-level directory + from open_handles import assert_closed + import test_execution_history_database as history_tests + +from database.operations import PromptDatabase +from database.pair_ingestion import ( + RUN_MARKS, + SCAN_MARKS, + Marks, + ingest_resolved_pair, +) +from database.scanned_prompts import store_scanned_prompt, store_scanned_prompt_or_raise +from utils.hashing import generate_prompt_hash + +META = {"prompt": {"1": {"class_type": "CLIPTextEncode", "inputs": {"text": "x"}}}} + + +def resolved(positive="cat", negative="blur"): + return { + "state": "resolved", + "positive": {"text": positive, "provenance": {"source": "api"}}, + "negative": {"text": negative, "provenance": {"source": "api"}}, + } + + +class StoreCase(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.db = PromptDatabase(str(Path(self.temp.name) / "test.db")) + self.path = str(Path(self.temp.name) / "gen.png") + + def tearDown(self): + self.db.close_all() + assert_closed(self, self.temp.name) + self.temp.cleanup() + + def prompts(self): + rows = self.db.model.get_connection().execute( + "SELECT id, text FROM prompts ORDER BY id" + ) + return [tuple(row) for row in rows] + + def links(self): + rows = self.db.model.get_connection().execute( + "SELECT prompt_id, image_path FROM generated_images ORDER BY id" + ) + return [tuple(row) for row in rows] + + def save(self, text, **fields): + return self.db.save_prompt( + text, prompt_hash=generate_prompt_hash(text), **fields + ) + + +class TestStoreScannedPrompt(StoreCase): + def test_a_new_text_is_created_with_the_scan_marks_and_linked(self): + status, prompt_id = store_scanned_prompt_or_raise( + self.db, self.path, META, " a cat ", SCAN_MARKS + ) + self.assertEqual(status, "added") + prompt = self.db.get_prompt_by_id(prompt_id) + self.assertEqual(prompt["text"], "a cat") + self.assertEqual(prompt["category"], "scanned") + self.assertEqual(prompt["tags"], ["auto-scanned"]) + self.assertEqual(prompt["notes"], "Auto-scanned from gen.png") + self.assertEqual(prompt["run_count"], 0) + self.assertEqual(self.links(), [(prompt_id, self.path)]) + + def test_a_run_created_prompt_names_the_workflow_run(self): + _status, prompt_id = store_scanned_prompt_or_raise( + self.db, self.path, META, "a cat", RUN_MARKS + ) + prompt = self.db.get_prompt_by_id(prompt_id) + self.assertEqual(prompt["notes"], "Saved from a workflow run: gen.png") + self.assertEqual(prompt["tags"], ["auto-scanned"]) + + def test_a_stored_text_is_reused_by_hash_and_keeps_its_fields(self): + existing = self.save("A Cat", category="mine", notes="kept") + status, prompt_id = store_scanned_prompt_or_raise( + self.db, self.path, META, "a cat ", SCAN_MARKS + ) + self.assertEqual((status, prompt_id), ("linked", existing)) + prompt = self.db.get_prompt_by_id(existing) + self.assertEqual((prompt["category"], prompt["notes"]), ("mine", "kept")) + self.assertEqual(prompt["tags"], []) + self.assertEqual(len(self.prompts()), 1) + + def test_a_repeat_is_unchanged(self): + first = store_scanned_prompt_or_raise( + self.db, self.path, META, "a cat", SCAN_MARKS + ) + again = store_scanned_prompt_or_raise( + self.db, self.path, META, "a cat", SCAN_MARKS + ) + self.assertEqual(again, ("unchanged", first[1])) + self.assertEqual(len(self.links()), 1) + + def test_a_file_linked_to_another_prompt_is_a_conflict(self): + other = self.save("a dog") + # Linked under another spelling of the same file. + spelled = os.path.join(self.temp.name, ".", "gen.png") + self.db.link_image_to_prompt(other, spelled) + result = store_scanned_prompt_or_raise( + self.db, self.path, META, "a cat", SCAN_MARKS + ) + self.assertEqual(result, ("conflict", None)) + self.assertEqual(self.prompts(), [(other, "a dog")]) + self.assertEqual(self.links(), [(other, spelled)]) + + def test_a_failed_link_raises_and_leaves_no_prompt_behind(self): + with mock.patch.object( + self.db, "_insert_image_link", side_effect=sqlite3.OperationalError("x") + ): + with self.assertRaises(sqlite3.OperationalError): + store_scanned_prompt_or_raise( + self.db, self.path, META, "a cat", SCAN_MARKS + ) + with self.assertLogs("prompt_manager", "ERROR"): + result = store_scanned_prompt( + self.db, self.path, META, "a cat", SCAN_MARKS + ) + self.assertEqual(result, ("found", None)) + self.assertEqual(self.prompts(), []) + self.assertEqual(self.links(), []) + + def test_two_concurrent_writers_store_one_prompt_and_one_link(self): + hold = history_tests.FirstWriterHold() + outcome = self.db._pair_component_outcome + started = threading.Barrier(2) + + def held_open(*args, **kwargs): + hold() + return outcome(*args, **kwargs) + + def write(_index): + started.wait(timeout=30) + try: + return store_scanned_prompt_or_raise( + self.db, self.path, META, "a cat", SCAN_MARKS + )[0] + finally: + self.db.model.close() + + with ( + mock.patch.object( + self.db, "_pair_component_outcome", side_effect=held_open + ), + ThreadPoolExecutor(max_workers=2) as pool, + ): + statuses = sorted(pool.map(write, range(2))) + self.assertEqual(statuses, ["added", "unchanged"]) + self.assertEqual(len(self.prompts()), 1) + self.assertEqual(len(self.links()), 1) + + def test_a_library_move_reruns_the_store(self): + from database import scanned_prompts + + real = scanned_prompts.store_scanned_prompt_or_raise + calls = [] + + def moved_once(*args, **kwargs): + if not calls: + calls.append(1) + self.db.model._generation += 1 + raise sqlite3.OperationalError("closed") + return real(*args, **kwargs) + + with mock.patch.object( + scanned_prompts, "store_scanned_prompt_or_raise", side_effect=moved_once + ): + status, _id = store_scanned_prompt( + self.db, self.path, META, "a cat", SCAN_MARKS + ) + self.assertEqual((status, calls), ("added", [1])) + + def _linked_folder(self): + """``(out_real, out_link)``: a symlink to the same output folder.""" + from tests.test_api_duplicate_files import TestReview + + if not TestReview._can_symlink(self.temp.name): # pragma: no cover + self.skipTest("symlinks are not available") + real = Path(self.temp.name) / "out_real" + real.mkdir() + link = Path(self.temp.name) / "out_link" + os.symlink(str(real), str(link)) + (real / "a.png").write_bytes(b"png") + return os.path.realpath(str(real)), str(link) + + def test_a_file_linked_through_its_real_folder_is_found_through_a_link(self): + """The scan stores realpath spellings, the monitor the watcher's.""" + real, link = self._linked_folder() + other = self.save("a dog") + self.db.link_image_to_prompt(other, os.path.join(real, "a.png")) + through_link = os.path.join(link, "a.png") + self.assertEqual( + store_scanned_prompt_or_raise( + self.db, through_link, META, "a cat", SCAN_MARKS + ), + ("conflict", None), + ) + self.assertEqual( + store_scanned_prompt_or_raise( + self.db, through_link, META, "a dog", SCAN_MARKS + ), + ("unchanged", other), + ) + pair = ingest_resolved_pair(self.db, through_link, META, resolved()) + self.assertEqual(pair["status"], "conflict") + self.assertEqual(len(self.links()), 1) + + +class TestPairMarks(StoreCase): + def test_marks_reach_only_created_components(self): + kept = self.save("cat", category="mine") + result = ingest_resolved_pair( + self.db, self.path, META, resolved(), marks=RUN_MARKS + ) + self.assertEqual(result["status"], "added") + positive = self.db.get_prompt_by_id(kept) + self.assertEqual((positive["category"], positive["tags"]), ("mine", [])) + negative_id = next(pid for pid, text in self.prompts() if text == "blur") + negative = self.db.get_prompt_by_id(negative_id) + self.assertEqual(negative["category"], "scanned") + self.assertEqual(negative["tags"], ["auto-scanned"]) + self.assertEqual(negative["notes"], "Saved from a workflow run: gen.png") + + def test_without_marks_created_components_stay_bare(self): + """Regression guard (passes on the base): the node path is unchanged.""" + ingest_resolved_pair(self.db, self.path, META, resolved()) + for prompt_id, _text in self.prompts(): + prompt = self.db.get_prompt_by_id(prompt_id) + self.assertIsNone(prompt["category"]) + self.assertEqual(prompt["tags"], []) + + def test_a_blank_negative_creates_only_the_positive(self): + result = ingest_resolved_pair( + self.db, self.path, META, resolved(negative=""), marks=SCAN_MARKS + ) + self.assertEqual(result["status"], "added") + self.assertEqual([text for _id, text in self.prompts()], ["cat"]) + + def test_marks_are_a_frozen_value(self): + self.assertEqual( + SCAN_MARKS, Marks("scanned", ("auto-scanned",), SCAN_MARKS.note) + ) + with self.assertRaises(AttributeError): + SCAN_MARKS.category = "x" + + +class TestPublicRunAttribution(unittest.TestCase): + def test_recorded_runs_and_named_prompt_are_public(self): + from database import run_attribution + + self.assertTrue(callable(run_attribution.recorded_runs)) + self.assertTrue(callable(run_attribution.named_prompt)) + self.assertFalse(hasattr(run_attribution, "_recorded_runs")) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/vendor/browser-coverage.test.mjs b/tests/vendor/browser-coverage.test.mjs index 39dcde57..a0336c49 100644 --- a/tests/vendor/browser-coverage.test.mjs +++ b/tests/vendor/browser-coverage.test.mjs @@ -27,3 +27,23 @@ test('browser coverage maps nested JS/MJS and excludes unrelated and traversal p const root = fileURLToPath(new URL('../../web/js/', import.meta.url)); assert.deepEqual(report.result.map(entry => fileURLToPath(entry.url)), [join(root, 'gallery.js'), join(root, 'shared/controls/button.mjs'), join(root, 'shared/panel.js')]); }); + +// #432: every raw file repeated each script's full source (about 84% of its bytes) and the coverage merge ran out of +// memory. c8 reads the source from the mapped file on disk, so the entries keep only the URL, id and functions. +test('browser coverage writes no script source, only what c8 maps', async t => { + const directory = mkdtempSync(join(tmpdir(), 'pm-browser-coverage-')); + const previous = process.env.PM_JS_COVERAGE; + process.env.PM_JS_COVERAGE = directory; + t.after(() => { + if (previous === undefined) delete process.env.PM_JS_COVERAGE; + else process.env.PM_JS_COVERAGE = previous; + rmSync(directory, { recursive: true, force: true }); + }); + const functions = [{ functionName: 'f', isBlockCoverage: true, ranges: [{ startOffset: 0, endOffset: 9, count: 1 }] }]; + await saveCoverage({ coverage: { stopJSCoverage: async () => [{ url: 'http://127.0.0.1:8765/prompt_manager/js/gallery.js', + scriptId: '7', source: 'x'.repeat(100_000), functions }] } }, 'chromium'); + const report = JSON.parse(readFileSync(join(directory, readdirSync(directory)[0]), 'utf8')); + assert.equal(report.result.length, 1); + assert.deepEqual(Object.keys(report.result[0]).sort(), ['functions', 'scriptId', 'url']); + assert.deepEqual(report.result[0].functions, functions); +}); diff --git a/tools/vendor/browser-coverage.mjs b/tools/vendor/browser-coverage.mjs index 4aaee07e..6655096c 100644 --- a/tools/vendor/browser-coverage.mjs +++ b/tools/vendor/browser-coverage.mjs @@ -12,7 +12,10 @@ export async function startCoverage(page, browserName) { export async function saveCoverage(page, browserName) { if (!process.env.PM_JS_COVERAGE || browserName !== 'chromium') return; const entries = await page.coverage.stopJSCoverage(); - const result = mapBrowserCoverage(entries, { urlPrefix: '/prompt_manager/js/', sourceRoot: join(repositoryRoot, 'web/js') }); + // The source is left out (#432): c8 reads it from the mapped file on disk, and repeating every script's text in every + // raw file made the coverage merge run out of memory. + const result = mapBrowserCoverage(entries, { urlPrefix: '/prompt_manager/js/', sourceRoot: join(repositoryRoot, 'web/js') }) + .map(({ source: _source, ...entry }) => entry); mkdirSync(process.env.PM_JS_COVERAGE, { recursive: true }); writeFileSync(join(process.env.PM_JS_COVERAGE, `browser-${randomUUID()}.json`), JSON.stringify({ result })); } diff --git a/utils/file_prompt.py b/utils/file_prompt.py new file mode 100644 index 00000000..0061277f --- /dev/null +++ b/utils/file_prompt.py @@ -0,0 +1,634 @@ +"""The per-file prompt decision shared by the output scan and the monitor (#247). + +``decide_file_prompt`` is the ladder: which prompt one output file names, +from its metadata alone plus, when the graph cannot decide, the save node +recorded history says wrote it. It does no file or database I/O; the caller +reads the metadata, the file size and history (through ``lookup``) and +stores the ``Decision``. + +The default resolver profile decides first, as ingestion always has. Only +for a file it leaves ambiguous or unresolved does the ladder ask the +inspector's resolve (``inspection_resolution``: the inspection profile, then +the recorded saver anchor; never the file-name saver anchor, whose files go +to Needs review by the #390 ruling), then the #170 browser parser (the legacy floor). +The policies differ in one place: for a file the inspector calls ambiguous, +the scan keeps the legacy floor and the monitor stores nothing. Junk text +(``junk_text``) is never stored, whichever step found it. +""" + +import json +import re +from dataclasses import dataclass, field +from enum import Enum +from typing import Any, Callable, Mapping, Optional, Union + +from .generation_params import ( + PLACEHOLDERS, + GenerationParamsError, + extract_generation_params, + parse_page_json, +) +from .prompt_graph import ( + PROMPT_MANAGER_TYPES, + _positive_roots, + is_negative_input, + metadata_opted_out, + resolve_text, + save_setting, + side_opted_out, +) +from .prompt_inspector import inspection_resolution +from .prompt_pair_resolver import _numeric, resolve_prompt_pair, sampler_less_prompt +from .review_capture import reviewable + +try: + from ..database.pair_ingestion import blank_positive +except ImportError: + from database.pair_ingestion import blank_positive + + +class Policy(Enum): + SCAN = "scan" + MONITOR = "monitor" + + +PAIR = "pair" +POSITIVE = "positive" +HISTORY = "history" +NOT_SAVED = "not_saved" +NOT_SAVED_WIRED = "not_saved_wired" +NEVER = "never" +KINDS = (PAIR, POSITIVE, HISTORY, NOT_SAVED, NOT_SAVED_WIRED, NEVER) + +_UNDECIDED = ("ambiguous", "unresolved") + + +@dataclass(frozen=True) +class Decision: + """What to store for one file (see the design's "The ladder"). + + ``kind``: one of ``KINDS``. ``text``: the storage text of the positive. + ``canonical``: the parsed metadata stored with the link. ``resolution``: + the resolution that decided a ``pair``, else the default profile's + result. ``source``: which step named the text (log lines and the scan's + outcome detail). ``review``: ``ambiguous`` or ``untraced`` when the + default profile was ambiguous (the scan's Needs review lists). + ``try_recorded_run``: the scan tries the recorded run's stored prompt + when nothing else links. ``unlinked_status``: the scan status when + nothing links (None: no prompt metadata). + """ + + kind: str + text: Optional[str] = None + canonical: dict = field(default_factory=dict) + resolution: Optional[dict] = None + source: str = "none" + review: Optional[str] = None + try_recorded_run: bool = False + unlinked_status: Optional[str] = None + + +@dataclass(frozen=True) +class _File: + """The ladder's inputs beside the metadata.""" + + metadata: Mapping + canonical: dict + resolution: dict + review: Optional[str] + file_name: Optional[str] + in_output: bool + lookup: Optional[Callable[[], Any]] + policy: Policy + + +def decide_file_prompt( + metadata: Any, + *, + file_name: Optional[str], + file_size: Union[None, int, Callable[[], Optional[int]]], + in_output: bool, + lookup: Optional[Callable[[], Any]], + policy: Policy, +) -> Decision: + """The ladder for one file's raw ``metadata``. + + ``prompt``/``workflow`` may be JSON strings (the scan) or parsed values + (the monitor). ``lookup`` returns the file's ``RecordedOutput`` or None; + it is None when history must not be asked yet. It is called at most + once, and only for a graph the default profile leaves undecided. + ``file_size`` may be a callable: it is read only for a file with prompt + metadata whose ``file_info`` has no size. + """ + if not metadata: + return Decision(NEVER, try_recorded_run=True) + canonical = _parsed(metadata) + if metadata_opted_out(canonical): + # Save to library was off for the run's prompt: nothing is stored, + # no recorded-run fallback, no Needs review item (#90) + return Decision(NOT_SAVED, canonical=canonical) + if metadata_opted_out(canonical, unreadable=True): + # The switch is wired from a node the file cannot show: only the + # live run (the node saw the value) can link it + return Decision(NOT_SAVED_WIRED, canonical=canonical) + switched = _switched_off_upstream(canonical.get("prompt")) + if switched is not None: + # The #90 walk follows only prompt-like inputs; any input name + # upstream of a positive can carry a switched-off node's text + return Decision(switched, canonical=canonical) + if not canonical.get("prompt") and not canonical.get("parameters"): + return Decision(NEVER, canonical=canonical, try_recorded_run=True) + _with_size(canonical, file_size) + resolution = resolve_prompt_pair(canonical) + switched = ( + _side_switch(canonical.get("prompt"), resolution["positive"]) + if resolution["state"] == "resolved" + else None + ) + if switched is not None: + # The positive passes a node that is off, though the #90 graph walk + # did not reach it (steps 5 and 7) + return Decision(switched, canonical=canonical) + if resolution["state"] == "resolved" and side_opted_out( + canonical.get("prompt"), resolution["negative"] + ): + # Only the negative's node is off: the positive alone, never a pair + positive = resolution["positive"] + text = positive.get("storage_text", positive["text"]).strip() + return _gated( + Decision( + POSITIVE, + text=text, + canonical=canonical, + resolution=resolution, + source="opted-out-negative", + ), + None, + ) + if blank_positive(resolution): + # Only prepend/append text: no stored prompt can own it (#342) + resolution = {**resolution, "state": "unresolved"} + found = _File( + metadata, + canonical, + resolution, + _review(resolution), + file_name, + in_output, + lookup, + policy, + ) + return _gated(_decide(found), found) + + +def _decide(found: _File) -> Decision: + state = found.resolution["state"] + graph = found.canonical.get("prompt") + if state == "resolved": + positive = found.resolution["positive"] + text = positive.get("storage_text", positive["text"]) + return _decision(found, PAIR, text, "resolver") + if graph and state in _UNDECIDED: + return _undecided_graph(found) + if state == "ambiguous": + return _never(found, _unlinked(found)) + if state == "unresolved" and not isinstance(found.canonical.get("parameters"), str): + return _never(found, "unresolved") + if graph: + return _sampler_less(found, graph) + text = _a1111_positive(found.metadata) + if text is None: + return _never(found, "found") + return _decision(found, POSITIVE, text, "a1111") + + +def _undecided_graph(found: _File) -> Decision: + recorded = found.lookup() if found.lookup is not None else None + saver = recorded.saver_id if recorded is not None else None + inspected = inspection_resolution( + found.metadata, found.file_name, saver, name_anchor=False + ) + text = _inspected_text(inspected.resolution) + switched = ( + _side_switch(found.canonical["prompt"], inspected.resolution["positive"]) + if text is not None + else None + ) + if switched is not None: + # The inspector follows pass-throughs the #90 graph walk does not + return Decision(switched, canonical=found.canonical) + if text is not None: + source = "recorded-run" if inspected.by_record else "inspector" + return _decision(found, POSITIVE, text, source) + if found.policy is Policy.MONITOR: + if found.lookup is None and found.in_output: + return _decision(found, HISTORY, None, "none") + if not inspected.malformed and inspected.resolution["state"] == "ambiguous": + return _never(found, _unlinked(found)) + text = graph_prompt_text(found.metadata) + switched = _off_text(found.canonical["prompt"], text) if text else None + if switched is not None: + # The legacy answer is a switched-off node's own text (#90) + return Decision(switched, canonical=found.canonical) + if text is not None and _joined_text(found.canonical["prompt"], text): + # The legacy parser joins prepend + text + append: the text a + # PromptManager node stores cannot be told from it (#343). + return _never(found, _unlinked(found), try_recorded_run=True) + if text is not None: + return _decision(found, POSITIVE, text, "legacy-parser") + return _never(found, _unlinked(found), try_recorded_run=True) + + +def _sampler_less(found: _File, graph: Any) -> Decision: + # No node takes a positive or negative input (a guider or wrapper graph): + # the one wired text, never a guess (#252). + wired = sampler_less_prompt(graph) + if wired["text"] is not None: + return _decision(found, POSITIVE, wired["text"], "sampler-less") + if isinstance(found.canonical.get("parameters"), str): + # A metadata saver also wrote A1111 text: its positive prompt. + text = _a1111_positive(found.metadata) + if text is not None: + return _decision(found, POSITIVE, text, "a1111") + status = "unresolved" if wired["state"] == "ambiguous" else "found" + return _never(found, status, try_recorded_run=True) + + +_UNDECIDED_SOURCES = ("inspector", "recorded-run", "legacy-parser") + + +def _switch_kind(settings: Any) -> Optional[str]: + """``not_saved`` when a setting is off, ``not_saved_wired`` when one is + unknowable (wired from a node the graph cannot read), else None.""" + settings = list(settings) + if False in settings: + return NOT_SAVED + if None in settings: + return NOT_SAVED_WIRED + return None + + +def _prompt_manager_ids(graph: Any) -> list: + if not isinstance(graph, dict): + return [] + return [ + str(node_id) + for node_id, node in graph.items() + if isinstance(node, dict) and node.get("class_type") in PROMPT_MANAGER_TYPES + ] + + +def _side_switch(graph: Any, side: Any) -> Optional[str]: + """The switch outcome of the PromptManager nodes on a side's path.""" + if not isinstance(graph, dict) or not side_opted_out(graph, side): + return None + provenance = side.get("provenance") + path = provenance.get("path") if isinstance(provenance, dict) else None + path = path if isinstance(path, list) else [] + nodes = {str(step.get("node_id")) for step in path if isinstance(step, dict)} & set( + _prompt_manager_ids(graph) + ) + return _switch_kind(save_setting(graph, node_id) for node_id in nodes) + + +# Inputs that never carry prompt text or positive conditioning. +_NOT_TEXT_INPUTS = frozenset( + { + "model", + "clip", + "vae", + "latent_image", + "samples", + "pixels", + "image", + "images", + "mask", + "noise", + "sigmas", + "sampler", + "control_net", + "upscale_model", + } +) +_MAX_SWITCH_VISITS = 5000 + + +def _negative_name(key: Any) -> bool: + """#90's negative rule, plus any name containing "negative" + (``negative2``, ``NegativePrompt``).""" + return is_negative_input(key) or "negative" in str(key).lower() + + +def _switched_off_upstream(graph: Any) -> Optional[str]: + """The switch outcome of every PromptManager node upstream of a positive. + + Follows every input except negative and non-text ones (#90's walk follows + only prompt-like names). A graph without a recognisable sampler checks + every PromptManager node, as #90 does. + """ + if not isinstance(graph, dict): + return None + roots = _positive_roots(graph) + if not roots: + ids = _prompt_manager_ids(graph) + return _switch_kind(save_setting(graph, node_id) for node_id in ids) + seen, found = set(), set() + stack = [str(link[0]) for link in roots] + while stack and len(seen) < _MAX_SWITCH_VISITS: + node_id = stack.pop() + if node_id in seen: + continue + seen.add(node_id) + node = graph.get(node_id) + if not isinstance(node, dict): + continue + if node.get("class_type") in PROMPT_MANAGER_TYPES: + found.add(node_id) + inputs = node.get("inputs") + for key, value in (inputs.items() if isinstance(inputs, dict) else ()): + if key in _NOT_TEXT_INPUTS or _negative_name(key): + continue + if isinstance(value, (list, tuple)) and len(value) == 2: + if isinstance(value[0], (str, int)) and isinstance(value[1], int): + stack.append(str(value[0])) + return _switch_kind(save_setting(graph, node_id) for node_id in found) + + +def _off_text(graph: Any, text: str) -> Optional[str]: + """The switch outcome when ``text`` is the text (resolved, else literal) + of a PromptManager-family node whose Save to library is not on.""" + wanted = text.strip() + matched = [] + for node_id in _prompt_manager_ids(graph): + node_text = resolve_text(graph, node_id) + if node_text is None: + inputs = graph[node_id].get("inputs") + literal = inputs.get("text") if isinstance(inputs, dict) else None + node_text = literal if isinstance(literal, str) else None + if node_text is not None and node_text.strip() == wanted: + matched.append(save_setting(graph, node_id)) + return _switch_kind(matched) + + +def _joined_text(graph: Any, text: str) -> bool: + """Whether ``text`` may hold a PromptManager node's prepend or append. + + True when a PromptManager-family node has a prepend or append part and + ``text`` is not the literal ``text`` of one such node (the legacy + parser's answer when it does not follow a linked part). + """ + own = _prompt_manager_parts(graph) + return own is not None and text.strip() not in own + + +def _prompt_manager_parts(graph: Any) -> Optional[set]: + """The literal texts of PromptManager-family nodes with a prepend or + append part, or None when no such node exists.""" + if not isinstance(graph, dict): + return None + found = None + for node in graph.values(): + if not isinstance(node, dict): + continue + name, inputs = node.get("class_type"), node.get("inputs") + if not isinstance(name, str) or "promptmanager" not in name.lower(): + continue + if not isinstance(inputs, dict) or not _has_part(inputs): + continue + found = found if found is not None else set() + if isinstance(inputs.get("text"), str): + found.add(inputs["text"].strip()) + return found + + +def _has_part(inputs: dict) -> bool: + for key in ("prepend_text", "append_text"): + part = inputs.get(key) + if part is not None and not (isinstance(part, str) and not part.strip()): + return True + return False + + +def _gated(decision: Decision, found: Optional[_File]) -> Decision: + """Junk text is never stored (step 13). + + A junk answer of the undecided-graph step keeps that step's unlinked + status, so an ambiguous file is still a Needs review case; any other + step reports ``found``. + """ + if decision.kind not in (PAIR, POSITIVE): + return decision + junk = junk_text(decision.text) + if junk is None: + return decision + return Decision( + NEVER, + canonical=decision.canonical, + resolution=decision.resolution, + source=f"junk:{junk}", + review=decision.review, + try_recorded_run=True, + unlinked_status=( + _unlinked(found) + if found is not None and decision.source in _UNDECIDED_SOURCES + else "found" + ), + ) + + +def _decision(found: _File, kind: str, text: Optional[str], source: str) -> Decision: + return Decision( + kind, + text=text, + canonical=found.canonical, + resolution=found.resolution, + source=source, + review=found.review, + ) + + +def _never(found: _File, status: str, *, try_recorded_run: bool = False) -> Decision: + return Decision( + NEVER, + canonical=found.canonical, + resolution=found.resolution, + review=found.review, + try_recorded_run=try_recorded_run, + unlinked_status=status, + ) + + +def _review(resolution: dict) -> Optional[str]: + if resolution["state"] != "ambiguous": + return None + return "ambiguous" if reviewable(resolution) else "untraced" + + +def _unlinked(found: _File) -> str: + """Ambiguous without a choice is reported unresolved.""" + if found.review == "untraced": + return "unresolved" + return found.resolution["state"] + + +def _inspected_text(resolution: dict) -> Optional[str]: + """The storage text of a positive the inspector shows, else None.""" + if resolution.get("state") in _UNDECIDED: + return None + positive = resolution.get("positive") + if not isinstance(positive, dict): + return None + text = positive.get("storage_text", positive.get("text")) + return text if isinstance(text, str) and text.strip() else None + + +def _parsed(metadata: Mapping) -> dict: + canonical = dict(metadata) + for key in ("prompt", "workflow"): + if isinstance(canonical.get(key), str): + try: + canonical[key] = json.loads(canonical[key]) + except (ValueError, TypeError): + # Preserve malformed metadata for conservative resolver rejection. + pass + return canonical + + +def _with_size(canonical: dict, file_size: Any) -> None: + """Record the file's size in ``file_info`` unless the metadata carries one. + + Stored as ``generated_images.file_size``; the "new" scan scope skips a + known file only while its size is unchanged. + """ + info = canonical.get("file_info") + info = dict(info) if isinstance(info, dict) else {} + if info.get("size") is None: + size = file_size() if callable(file_size) else file_size + if size is not None: + info["size"] = size + if info: + canonical["file_info"] = info + + +def _a1111_positive(metadata: Mapping) -> Optional[str]: + text = parse_comfyui_prompt(metadata)["positive_prompt"] + return text.strip() if text and text.strip() else None + + +_LORA_TAG = re.compile(r"]*>") +_LORA_SEPARATORS = " \t\r\n\f\v,;|" +_MODEL_FILE = re.compile(r"\S*\.(?:safetensors|ckpt|pt|pth|bin|gguf|sft)", re.I) + + +def junk_text(text: Any) -> Optional[str]: + """The #252 junk class of ``text``, or None when it may be a prompt.""" + stripped = text.strip() if isinstance(text, str) else "" + if not stripped: + return "blank" + if _numeric(stripped): + return "number" + if not _LORA_TAG.sub("", stripped).strip(_LORA_SEPARATORS): + return "lora-only" + if _MODEL_FILE.fullmatch(stripped): + return "model-file" + return None + + +def graph_prompt_text(metadata: Mapping) -> Optional[str]: + """The positive prompt the browser parser traces from the graphs, or None. + + Records parse as the legacy pages parsed them (NaN reads as null). + """ + records = {} + for key in ("prompt", "workflow"): + value = metadata.get(key) + records[key] = parse_page_json(value) if isinstance(value, str) else value + try: + positive = extract_generation_params(records)["positivePrompt"] + except GenerationParamsError: + return None + if not isinstance(positive, str) or positive == PLACEHOLDERS["positivePrompt"]: + return None + return positive.strip() or None + + +_COMMON_FIELDS = ( + "positive", + "negative", + "steps", + "cfg", + "sampler", + "scheduler", + "seed", +) + + +def _json_or_raw(value: Any) -> Any: + if not isinstance(value, str): + return value + try: + return json.loads(value) + except json.JSONDecodeError: + return value + + +def parse_comfyui_prompt(metadata: Mapping) -> dict: + """Parse ComfyUI and A1111 prompt data from metadata. + + Accepts the file's strings or parsed values: a parsed ``prompt`` or + ``workflow`` is kept as it is and a non-string ``parameters`` is ignored. + """ + result = { + "prompt": None, + "workflow": None, + "parameters": {}, + "positive_prompt": None, + "negative_prompt": None, + } + params = metadata.get("parameters") + if isinstance(params, str): + _parse_a1111(params, result) + if result["positive_prompt"] is None: + for key in ("prompt", "workflow"): + if key in metadata: + result[key] = _json_or_raw(metadata[key]) + for name in _COMMON_FIELDS: + if name in metadata: + result["parameters"][name] = _json_or_raw(metadata[name]) + return result + + +_SETTINGS_FIELDS = ("Steps", "Sampler", "Schedule type", "CFG scale", "Seed") + + +def _parse_a1111(params: str, result: dict) -> None: + """A1111/Forge ``parameters``: a possibly multi-line positive, then a + "Negative prompt:" section, then a trailing "Steps: ..." settings line. + Taking the first line as the positive would truncate a multi-line prompt, + so the text is split on the settings line instead. + """ + body = params + settings = "" + found = re.search(r"\nSteps:\s", body) + if found: + settings = body[found.start() + 1 :].strip() + body = body[: found.start()] + negative = None + found = re.search(r"(?im)^\s*negative prompt:\s*", body) + if found: + negative = body[found.end() :].strip() + body = body[: found.start()] + elif body.lower().lstrip().startswith("negative prompt:"): + negative = body.lower().lstrip()[len("negative prompt:") :].strip() + body = "" + result["positive_prompt"] = body.strip() or None + if negative: + result["negative_prompt"] = negative + result["parameters"]["parameters"] = params + for name in _SETTINGS_FIELDS if settings else (): + found = re.search( + r"(?i)(?:^|,\s*)" + re.escape(name) + r":\s*([^,]+)", settings + ) + if found: + key = name.lower().replace(" ", "_") + result["parameters"][key] = found.group(1).strip() diff --git a/utils/prompt_graph.py b/utils/prompt_graph.py index b21021af..88a45c54 100644 --- a/utils/prompt_graph.py +++ b/utils/prompt_graph.py @@ -41,9 +41,14 @@ _TEXT_KEY = re.compile(r"^(text|string|prompt)(_?[a-z0-9]+)?$", re.IGNORECASE) _NEGATIVE_KEY = re.compile(r"(^|_)neg(ative)?($|_)", re.IGNORECASE) +def is_negative_input(key: Any) -> bool: + """Whether an input name carries a negative prompt (#90's one rule).""" + return isinstance(key, str) and bool(_NEGATIVE_KEY.search(key)) + + def follows_input(key: Any) -> bool: """Whether an input name carries positive conditioning or prompt text.""" - if not isinstance(key, str) or _NEGATIVE_KEY.search(key): + if not isinstance(key, str) or is_negative_input(key): return False return key == "positive" or bool( _CONDITIONING_KEY.match(key) or _TEXT_KEY.match(key) diff --git a/utils/prompt_inspector.py b/utils/prompt_inspector.py index 1daaef6c..6211f408 100644 --- a/utils/prompt_inspector.py +++ b/utils/prompt_inspector.py @@ -1,4 +1,10 @@ -"""Read prompt evidence without inference, indexing, or decoding image pixels.""" +"""Read prompt evidence without inference, indexing, or decoding image pixels. + +``inspection_resolution`` is the inspector's resolve (inspection profile, then +the recorded saver anchor, #386). Ingestion (the output scan, through +``utils.file_prompt``) calls it too, only for files the default profile +cannot decide (#247), so the inspector and ingestion cannot drift. +""" import hashlib import io @@ -571,6 +577,102 @@ def _recorded_execution(resolved: dict, texts: Mapping) -> dict | None: return {"prompts": prompts, "resolution": resolution} +_ABSENT_RESOLUTION = { + "state": "absent", + "sampler_id": None, + "reason": "No parsed prompt metadata", + "reason_code": None, + "node": None, +} +_MALFORMED_RESOLUTION = { + "state": "unresolved", + "sampler_id": None, + "reason": MALFORMED_EVIDENCE_REASON, + "reason_code": "malformed", + "node": None, +} + + +@dataclass(frozen=True) +class InspectionResult: + """The inspector's resolution of one file's metadata (#386, #247). + + ``resolution`` is the result to use. ``anchored`` is the result anchored + on the recorded saver, kept or not (None when no anchored resolve ran). + ``by_record`` says the anchored result was kept: history's saver chose + the samplers and the result names the positive. ``malformed`` marks + metadata the inspector cannot read as a graph. + """ + + resolution: dict + anchored: dict | None = None + by_record: bool = False + malformed: bool = False + + +def inspection_resolution( + metadata: Mapping, + file_name: str | None, + recorded_saver_id: str | None, + *, + profile: ResolverProfile = INSPECTION_PROFILE, + name_anchor: bool = True, +) -> InspectionResult: + """Resolve raw file metadata as the inspector does. + + Keeps only the role records, parses a string ``prompt`` and attaches the + parsed ``workflow`` only when the graph may hold a PromptManager node. + Resolves with ``profile``; when that leaves the prompt open and + ``recorded_saver_id`` is given, resolves again anchored on that saver + and keeps the anchored result only when it names the positive. + + ``name_anchor=False`` (ingestion, #247) resolves the unanchored pass + without ``file_name``, so the saver its file name names anchors nothing: + such a file goes to Needs review, never linked (#390 ruling). The + recorded-saver pass still gets ``file_name``, only to refuse a saver the + name contradicts. + """ + workflow = metadata.get("workflow") + canonical = { + key: value for key, value in metadata.items() if key in _ROLE_METADATA_KEYS + } + if not canonical: + return InspectionResult(dict(_ABSENT_RESOLUTION)) + malformed = InspectionResult(dict(_MALFORMED_RESOLUTION), malformed=True) + if "prompt" not in canonical and "parameters" not in canonical: + return malformed + if isinstance(canonical.get("prompt"), str): + try: + canonical["prompt"] = json.loads(canonical["prompt"]) + except (ValueError, RecursionError): + return malformed + workflow = ( + _parsed_workflow(workflow) if _may_hold_prompt_manager(canonical) else None + ) + if workflow is not None and "prompt" in canonical: + # Only to recognize overwritten PromptManager text (R1); never a role. + canonical["workflow"] = workflow + resolved = resolve_prompt_pair( + canonical, profile=profile, file_name=file_name if name_anchor else None + ) + if resolved["state"] not in _UNDECIDED or recorded_saver_id is None: + return InspectionResult(resolved) + anchored = resolve_prompt_pair( + canonical, + profile=profile, + file_name=file_name, + recorded_saver_id=recorded_saver_id, + ) + # Only a result whose samplers history's saver chose is about this file. + if ( + anchored.get("recorded_anchor") is True + and anchored["state"] not in _UNDECIDED + and isinstance(anchored["positive"], dict) + ): + return InspectionResult(anchored, anchored, True) + return InspectionResult(resolved, anchored) + + def inspect_prompt_evidence( metadata: dict, saved: dict | None = None, @@ -593,22 +695,13 @@ def inspect_prompt_evidence( ``execution``. Inspection routes resolve with ``INSPECTION_PROFILE``: bounded work, not - graph size. Ingestion (scan, live monitor, admin import) calls the resolver - directly with its default profile. ``resolution`` carries the closed - ``reason_code`` and, for a blocking node, ``node: {id, class_type}``. + graph size. The resolve itself is ``inspection_resolution``, which + ingestion also calls for files its default profile cannot decide (#247). + ``resolution`` carries the closed ``reason_code`` and, for a blocking + node, ``node: {id, class_type}``. """ - workflow = metadata.get("workflow") - metadata = { - key: value for key, value in metadata.items() if key in _ROLE_METADATA_KEYS - } - resolution = { - "state": "unresolved", - "sampler_id": None, - "reason": MALFORMED_EVIDENCE_REASON, - "reason_code": "malformed", - "node": None, - } - if not metadata: + roles = {key for key in metadata if key in _ROLE_METADATA_KEYS} + if not roles: result = {role: _side("absent") for role in ("positive", "negative")} if saved: context = saved.get("pair_context") @@ -621,56 +714,30 @@ def inspect_prompt_evidence( "verified against this file." ), } - return { - "prompts": result, - "resolution": { - "state": "absent", - "sampler_id": None, - "reason": "No parsed prompt metadata", - "reason_code": None, - "node": None, - }, - } - canonical = dict(metadata) - malformed = { - "prompts": _explained( - {role: _side("unresolved") for role in ("positive", "negative")}, - resolution, - ), - "resolution": resolution, - } - if "prompt" not in canonical and "parameters" not in canonical: - return malformed - if isinstance(canonical.get("prompt"), str): - try: - canonical["prompt"] = json.loads(canonical["prompt"]) - except (ValueError, RecursionError): - return malformed - workflow = ( - _parsed_workflow(workflow) if _may_hold_prompt_manager(canonical) else None + return {"prompts": result, "resolution": dict(_ABSENT_RESOLUTION)} + inspected = inspection_resolution( + metadata, file_name, recorded_saver_id, profile=profile ) - if workflow is not None and "prompt" in canonical: - # Only to recognize overwritten PromptManager text (R1); never a role. - canonical["workflow"] = workflow - resolved = resolve_prompt_pair(canonical, profile=profile, file_name=file_name) - recorded = False - if resolved["state"] in _UNDECIDED and recorded_saver_id is not None: - anchored = resolve_prompt_pair( - canonical, - profile=profile, - file_name=file_name, - recorded_saver_id=recorded_saver_id, - ) - # Only a result whose samplers history's saver chose is about this file. - if anchored.get("recorded_anchor") is True: - if anchored["state"] not in _UNDECIDED and isinstance( - anchored["positive"], dict - ): - resolved, recorded = anchored, True - elif recorded_texts: - executed = _recorded_execution(anchored, recorded_texts) - if executed is not None: - return executed + if inspected.malformed: + resolution = inspected.resolution + return { + "prompts": _explained( + {role: _side("unresolved") for role in ("positive", "negative")}, + resolution, + ), + "resolution": resolution, + } + resolved, recorded = inspected.resolution, inspected.by_record + anchored = inspected.anchored + if ( + not recorded + and recorded_texts + and anchored is not None + and anchored.get("recorded_anchor") is True + ): + executed = _recorded_execution(anchored, recorded_texts) + if executed is not None: + return executed state = resolved["state"] result = {} for role in ("positive", "negative"): diff --git a/web/js/workspace/image-tagging-state.mjs b/web/js/workspace/image-tagging-state.mjs index 18b2e880..6bb74132 100644 --- a/web/js/workspace/image-tagging-state.mjs +++ b/web/js/workspace/image-tagging-state.mjs @@ -203,17 +203,48 @@ async function readDefaults(api, signal) { } export const UNAVAILABLE_HINT = 'Download it in System › Auto-tagging.'; +const nonEmpty = value => typeof value === 'string' && value.trim() !== ''; +const joined = names => (names.length > 1 ? `${names.slice(0, -1).join(', ')} and ${names.at(-1)}` : names.join('')); + +/** The pip names a `/autotag/models` entry says its backend cannot import (#424); only non-empty strings count. */ +export const missingPackages = entry => (object(entry) && Array.isArray(entry.missing_packages) + ? entry.missing_packages.filter(nonEmpty) : []); + +/** `pip install …` for `entry`'s missing packages: the server's command, else built from the names; '' when none. */ +export function installCommand(entry) { + const missing = missingPackages(entry); + if (!missing.length) return ''; + return nonEmpty(entry.install_command) ? entry.install_command : `pip install ${missing.join(' ')}`; +} + +/** + * The server's missing-package sentence (`missing_packages_message`, #424) for `name`, else '' (nothing missing): + * " needs llama-cpp-python. Install it into ComfyUI's Python: pip install llama-cpp-python. ". + */ +export function needsPackages(name, entry) { + const missing = missingPackages(entry); + if (!missing.length) return ''; + const note = nonEmpty(entry.install_note) ? ` ${entry.install_note}` : ''; + return `${name} needs ${joined(missing)}. Install ${missing.length === 1 ? 'it' : 'them'} into ComfyUI's Python: ` + + `${installCommand(entry)}.${note}`; +} + +/** "a, b and c": the missing package names as the sentences list them. */ +export const packageList = entry => joined(missingPackages(entry)); + /** * Why `model` cannot run, from the `/autotag/models` status (the server's own `model-unavailable` sentence), else '' - * (downloaded, or the status is unknown: the job then reports a missing model itself). + * (ready, or the status is unknown: the job then reports a missing model itself). Not downloaded is named first, then + * a backend package that cannot be imported (#424). */ export function modelUnavailable(models, model) { const option = modelOf(model); requireValue(option, 'Unknown tagging model'); const status = object(models) && object(models[model]) ? models[model] : null; - if (!status || status.downloaded !== false) return ''; - const name = typeof status.name === 'string' && status.name.trim() ? status.name : option.label.split(' · ')[0]; - return `${name} is not downloaded. ${UNAVAILABLE_HINT}`; + if (!status) return ''; + const fallback = option.label.split(' · ')[0]; + if (status.downloaded === false) return `${nonEmpty(status.name) ? status.name : fallback} is not downloaded. ${UNAVAILABLE_HINT}`; + return needsPackages(nonEmpty(status.label) ? status.label : fallback.replace(' (Recommended)', ''), status); } function initialState(source, preset) { diff --git a/web/js/workspace/system-tagging.mjs b/web/js/workspace/system-tagging.mjs index d9b369fa..6a96f177 100644 --- a/web/js/workspace/system-tagging.mjs +++ b/web/js/workspace/system-tagging.mjs @@ -29,7 +29,7 @@ import { HOST_UNAVAILABLE } from './system-appearance.mjs'; import { openCloseOnlyReview, openReviewDialog } from './workspace-dialogs.mjs'; import { BACKUP_COPY, backupText, holding, reviewRefusal } from './maintenance-review.mjs'; import { busyToast, downloadDatabaseBackup } from './activity-scan.mjs'; -import { MODEL_OPTIONS } from './image-tagging-state.mjs'; +import { MODEL_OPTIONS, installCommand, missingPackages, needsPackages, packageList } from './image-tagging-state.mjs'; import { openImageTagging } from './image-tagging.mjs'; import { selectedImage } from './images.mjs'; import { NO_SELECTION } from './vocabulary.mjs'; @@ -80,6 +80,7 @@ export const STATUS_COPY = Object.freeze({ loaded: 'Downloaded and loaded.', partial: 'Partly downloaded. Download again to finish.', missing: size => `Not downloaded. Download size ${size}.`, + needs: (names, command, note) => `Downloaded. Needs ${names}: ${command}.${typeof note === 'string' && note.trim() ? ` ${note}` : ''}`, }); /** The messages that name a setting the section cannot use (the server's own wording). */ @@ -140,15 +141,21 @@ export function modelOptions(models) { /** * D132: the Tagging model description for `status` (`{phase: loading|loaded|failed, models}`) and the chosen `model`. - * GGUF with only one of its two files on disk is partly downloaded. + * GGUF with only one of its two files on disk is partly downloaded. A backend package the server cannot import (#424) + * turns "Downloaded" into "Downloaded. Needs : pip install ." and adds "Also needs ." otherwise. */ export function modelDescription(status, model) { if (status?.phase === 'loading') return STATUS_COPY.loading; const entry = status?.models?.[model]; if (status?.phase !== 'loaded' || !object(entry)) return STATUS_COPY.failed; - if (entry.downloaded === true) return entry.loaded === true ? STATUS_COPY.loaded : STATUS_COPY.downloaded; - if (partlyDownloaded(model, entry)) return STATUS_COPY.partial; - return STATUS_COPY.missing(modelSize(model, entry)); + // #424: a backend package that cannot be imported; a model on disk without it never reads as ready. + const needs = missingPackages(entry).length ? packageList(entry) : ''; + if (entry.downloaded === true) { + if (needs) return STATUS_COPY.needs(needs, installCommand(entry), entry.install_note); + return entry.loaded === true ? STATUS_COPY.loaded : STATUS_COPY.downloaded; + } + const files = partlyDownloaded(model, entry) ? STATUS_COPY.partial : STATUS_COPY.missing(modelSize(model, entry)); + return needs ? `${files} Also needs ${needs}.` : files; } /** GGUF with only one of its two files on disk (a download that did not finish). */ @@ -316,6 +323,7 @@ export function diskShortNote(disk) { export function batchCloseNote(plan) { if (plan.review_required === true) return REPLACE_NOTE; if (plan.model?.downloaded === false) return notDownloaded(modelLabel(plan.model.type, plan.model)); + if (missingPackages(plan.model).length) return needsPackages(modelLabel(plan.model.type, plan.model), plan.model); if (plan.disk?.sufficient === false) return diskShortNote(plan.disk); if (!Number.isSafeInteger(plan.images) || plan.images === 0) return NO_IMAGES; if (plan.to_tag === 0) return NOTHING_TO_TAG; @@ -484,8 +492,9 @@ export function mountTagging(container, { api, activity, scope, drafts, notify = unload.disabled = !(status.phase === 'loaded' && status.loadedType); if (review.textContent === 'Reviewing…') return; const batchProblem = known() ? draftProblem(values, { limit: false }) : unknown; - setAvailable(review, entry?.downloaded === false ? notDownloaded(modelLabel(values.autotag_model_type, entry)) - : batchProblem?.message); + const label = modelLabel(values.autotag_model_type, entry); + setAvailable(review, entry?.downloaded === false ? notDownloaded(label) + : needsPackages(label, entry) || batchProblem?.message); }; let reads = 0;