diff --git a/TESTING.md b/TESTING.md index 72d83e7..36e2b5e 100644 --- a/TESTING.md +++ b/TESTING.md @@ -2308,15 +2308,26 @@ validation, annotation group included. The browser fixtures clear never the metadata row. Needs review items (#249, `utils/review_capture.py`, `database/review.py`, -`tests/test_review.py`, `tests/test_review_outcome.py`): a file is queued only -when its final outcome is ambiguous and unlinked. The output scan (Activity's +`tests/test_review.py`, `tests/test_review_outcome.py`, +`tests/test_review_choosable.py`): a file is queued only when its final outcome +is ambiguous and unlinked and there is something to choose. Only scans produce +items; the monitor links new outputs and clears pending items it resolves +(maintainer rulings, 2026-10-23). The output scan (Activity's Scan for metadata and the legacy `/scan`) hands `record_ambiguous_file` a file the ingestion resolver finds `ambiguous` only when the browser-parser fallback then links nothing (status `ambiguous`) and no saved row links the file already; an ambiguous file a saved row links is reported `unchanged` ("Already linked to its prompt"), not Needs review. A file the fallback links, an exact pair the ingester stores, and `conflict` and `unresolved` files are not -queued. The queue heals itself: after each batch, the scan removes the pending +queued. Neither is an ambiguous file with nothing to choose: fewer than two of +its candidates are `resolved` with non-blank positive text (`reviewable`). With +none the file could not be traced; with exactly one, the untraced branches may +still have made the file, so that text is not certain either. Both report +`unresolved` ("could not be traced"), are never linked, and their pending, +undecided items whose own evidence offers no choice (a limited item counts as +a choice when it holds at least two candidates) leave the queue on the next +scan or monitor pass (`settle_untraced_review_items`). The maintainer's +second live check found 39 items whose 11 candidates were all "unresolved". The queue heals itself: after each batch, the scan removes the pending items of every listed file (read or skipped as known) that a saved row now links, and the monitor does the same for each ambiguous output it links (`settle_linked_review_items`; the review index revision moves). Only an @@ -2333,20 +2344,21 @@ fenced by the capture's storage epoch. Evidence is the resolver reason plus one `{sampler_id, state, reason, positive, negative, hash}` per selectable candidate (nested ambiguous stages flattened, duplicates dropped; each side `{text, storage_text, provenance}`; `hash` = SHA-256 of the canonical candidate -without it). No candidate, more than 16, or evidence over 8 MiB is a `limited` +without it). More than 16 candidates, or evidence over 8 MiB, is a `limited` item without candidates. Recording is idempotent per `(byte_id, resolver_version, evidence_hash)` and never touches an existing item or its decisions; changed bytes add a new pending item and keep the old one. The capture is released before the write; no model is loaded and no pixel is decoded. Scan statuses, counters, outcome rows and the legacy SSE events are -unchanged apart from the already-linked case above; the job's `scope` gains +unchanged apart from the already-linked and nothing-to-choose cases above; the job's `scope` gains `review_items` (this scan's files that are in the pending queue after it, new or already there: the review group's ambiguous rows, less files a capture could not read or a decision already settled), `review_added` (new items), -`review_settled` (items removed because their file is linked now) and +`review_settled` (items removed because their file is linked now), +`review_untraced` (items removed because they offer nothing to choose) and `review_errors`. The log ends with `? N images added to Needs review`, `? N images already in Needs review`, `= N images left Needs review · now -linked` and `! N images could not be added to Needs review` (failures, e.g. +linked`, `= N images left Needs review · nothing to choose` and `! N images could not be added to Needs review` (failures, e.g. capture capacity), each only when N > 0. Recording captures in the background: it never takes the store's last worker or capture slot, which stay free for an interactive Tag dialog or Apply, and waits up to 2 s for room before counting the file as not added diff --git a/database/review.py b/database/review.py index 65af8a2..40780e8 100644 --- a/database/review.py +++ b/database/review.py @@ -39,6 +39,7 @@ from .review_schema import ( METADATA_TABLE, canonical_evidence, evidence_digest, + evidence_offers_choice, ) # Only this module's table constants are interpolated into the SQL below. @@ -95,6 +96,13 @@ _PENDING_OF_LOCATOR = ( f"JOIN {ITEMS_TABLE} i ON i.byte_id=b.id " "WHERE f.locator=? AND i.state='pending'" ) +# Pending items of one file with their evidence: an untraced file's settle. +_PENDING_EVIDENCE_OF_LOCATOR = ( + f"SELECT i.id,i.evidence FROM {FILES_TABLE} f " # nosec B608 + f"JOIN {BYTES_TABLE} b ON b.file_id=f.id " + f"JOIN {ITEMS_TABLE} i ON i.byte_id=b.id " + "WHERE f.locator=? AND i.state='pending'" +) # Only an undecided item goes: a decision row (RESTRICT) is never deleted. _SETTLE_ITEM = ( f"DELETE FROM {ITEMS_TABLE} WHERE id=? AND state='pending' " # nosec B608 @@ -417,6 +425,41 @@ class ReviewOperations: conn.execute(_BUMP_REVIEW_INDEX) return removed + def settle_untraced_review_items(self, locators) -> int: + """Remove the pending, undecided items that offer nothing to choose. + + ``locators`` are files the caller found ambiguous without a choice + (fewer than two candidates carry a prompt). Of their items, one leaves + the queue only when its own stored evidence offers no choice either + (``evidence_offers_choice``) and it is still ``pending`` at revision 0 + with no decision row, the same safe delete as + ``settle_linked_review_items``. Files without a pending item cost one + indexed read and open no write transaction. Returns how many items + were removed (the review index revision moves when any was). + """ + locators = list(locators) + for locator in locators: + _validate_locator(locator) + conn = self.model.get_connection() + pending = [ + locator + for locator in locators + if conn.execute(_PENDING_OF_LOCATOR, (locator,)).fetchone() is not None + ] + if not pending: + return 0 + removed = 0 + with self._pair_transaction() as conn: + for locator in pending: + rows = conn.execute(_PENDING_EVIDENCE_OF_LOCATOR, (locator,)).fetchall() + for item_id, evidence in rows: + if evidence_offers_choice(json.loads(evidence)): + continue + removed += conn.execute(_SETTLE_ITEM, (item_id,)).rowcount + if removed: + conn.execute(_BUMP_REVIEW_INDEX) + return removed + def review_queue_page(self, *, limit: int, after: tuple | None = None) -> dict: """One page of pending items, oldest first, from one snapshot. diff --git a/database/review_schema.py b/database/review_schema.py index 8c8aaf7..0370a5e 100644 --- a/database/review_schema.py +++ b/database/review_schema.py @@ -200,6 +200,44 @@ def canonical_evidence(value: Any) -> str: return text +def choosable(candidate: Any) -> bool: + """True when a candidate offers a prompt to choose: it is ``resolved`` and + its positive side carries non-blank text. The page disables any other + candidate, so it is never something to choose. + """ + if not isinstance(candidate, dict) or candidate.get("state") != "resolved": + return False + positive = candidate.get("positive") + if not isinstance(positive, dict): + return False + text = positive.get("text") + return isinstance(text, str) and bool(text.strip()) + + +# An item is worth a decision only when at least this many candidates offer +# a prompt; with fewer there is nothing to choose between (maintainer test of +# #284/#288: 39 items whose candidates were all "unresolved"). +MIN_CHOOSABLE = 2 + + +def evidence_offers_choice(evidence: Any) -> bool: + """False when stored evidence provably offers nothing to choose. + + Unlimited evidence must list ``MIN_CHOOSABLE`` choosable candidates. A + limited item stores none, only ``candidate_total``; it may offer a choice + (more than ``MAX_CANDIDATES`` prompts) unless that total is too small. + """ + if not isinstance(evidence, dict): + return False + if evidence.get("limited"): + total = evidence.get("candidate_total") + return type(total) is int and total >= MIN_CHOOSABLE + candidates = evidence.get("candidates") + if not isinstance(candidates, list): + return False + return sum(1 for c in candidates if choosable(c)) >= MIN_CHOOSABLE + + def evidence_digest(text: str) -> str: """SHA-256 of canonical evidence text (ASCII, hence UTF-8 identical).""" return hashlib.sha256(text.encode("utf-8")).hexdigest() diff --git a/py/api/admin.py b/py/api/admin.py index 740643a..8f8c5e4 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -44,6 +44,7 @@ try: parse_page_json, ) from ...utils.prompt_pair_resolver import resolve_prompt_pair + from ...utils.review_capture import reviewable except ImportError: from database.operations import PromptDatabase from database.pair_ingestion import ingest_resolved_pair @@ -54,6 +55,7 @@ except ImportError: parse_page_json, ) from utils.prompt_pair_resolver import resolve_prompt_pair + from utils.review_capture import reviewable try: @@ -214,8 +216,9 @@ def _time_left(done, total, batches, elapsed): def new_review_counts(): """The scan's Needs review tally: ``added`` new items, ``queued`` files in the queue after this scan (new or already there), ``settled`` items - removed because their file is linked now, and ``errors``.""" - return {"added": 0, "queued": 0, "settled": 0, "errors": 0} + removed because their file is linked now, ``untraced`` items removed + because they offer nothing to choose, and ``errors``.""" + return {"added": 0, "queued": 0, "settled": 0, "untraced": 0, "errors": 0} def _review_scope(review): @@ -229,6 +232,7 @@ def _review_scope(review): "review_items": review["queued"], "review_added": review["added"], "review_settled": review["settled"], + "review_untraced": review["untraced"], "review_errors": review["errors"], } @@ -1612,7 +1616,9 @@ class AdminRoutesMixin: whose final outcome is ambiguous and unlinked (the resolver found it ambiguous and the fallback did not link it) is recorded for Needs review (best effort, counted in ``review`` when given); one a saved - row already links is reported ``unchanged`` and never queued. + row already links is reported ``unchanged`` and never queued. An + ambiguous file with nothing to choose is reported ``unresolved`` and + its pending items without a choice leave the queue. """ counts = dict.fromkeys(SCAN_COUNT_KEYS, 0) @@ -1624,10 +1630,14 @@ class AdminRoutesMixin: if self._scan_job and self._scan_job.cancelled: break counts["processed"] += 1 - ambiguous = [] + ambiguous, untraced = [], [] try: outcome = self._ingest_scanned_file( - media_file, metadata, detailed=True, ambiguous=ambiguous + media_file, + metadata, + detailed=True, + ambiguous=ambiguous, + untraced=untraced, ) except Exception as e: self.logger.error(f"Error processing {media_file.name}: {e}") @@ -1639,6 +1649,8 @@ class AdminRoutesMixin: outcome = "unchanged" # already linked: not a review case else: self._record_scanned_review(media_file, roots, review) + elif untraced and outcome == "unresolved": + self._settle_untraced_reviews(untraced, roots, review) if outcome is None: note(media_file, None) continue @@ -1673,7 +1685,7 @@ class AdminRoutesMixin: return positive.strip() or None def _ingest_scanned_file( - self, media_file, metadata, detailed=False, ambiguous=None + self, media_file, metadata, detailed=False, ambiguous=None, untraced=None ): """Enrich exact pairs; retain legacy positive-only import for absent pairs. @@ -1683,7 +1695,10 @@ class AdminRoutesMixin: 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. + ``"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``. """ if not metadata: return None @@ -1699,7 +1714,12 @@ class AdminRoutesMixin: return None _with_file_size(canonical, media_file) resolution = resolve_prompt_pair(canonical) - if resolution["state"] == "ambiguous" and ambiguous is not None: + # 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) + elif resolution["state"] == "ambiguous" and ambiguous is not None: ambiguous.append(media_file) if resolution["state"] == "resolved": result = ingest_resolved_pair( @@ -1720,9 +1740,9 @@ class AdminRoutesMixin: # Graphs without a recognized sampler keep the legacy extractor below. prompt_text = self._graph_prompt_text(metadata) if not prompt_text: - return resolution["state"] + return "unresolved" if unchoosable else resolution["state"] elif resolution["state"] == "ambiguous": - return "ambiguous" + return "unresolved" if unchoosable else "ambiguous" elif resolution["state"] == "unresolved" and not isinstance( canonical.get("parameters"), str ): diff --git a/py/api/review_recording.py b/py/api/review_recording.py index 31de6d9..26c2de2 100644 --- a/py/api/review_recording.py +++ b/py/api/review_recording.py @@ -5,13 +5,18 @@ 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 already. A file the fallback links, an exact pair the ingester stored and an -already linked file are never queued. +already linked file are never queued. Neither is an ambiguous file with +nothing to choose: fewer than two of its candidates carry a prompt +(``utils.review_capture.reviewable``); the scan reports it unresolved ("could +not be traced"). The queue heals itself: a pending item whose file is now linked (by the fallback, a later resolution or the monitor) is removed by the next scan -that lists the file or by the monitor's next pass over it. Only an -undecided item goes (``pending``, revision 0, no decision row); a decision -is never deleted or changed (``settle_linked_review_items``). +that lists the file or by the monitor's next pass over it, and so is a +pending item without a choice whose file is still ambiguous without one +(``settle_untraced_review_items``). Only an undecided item goes +(``pending``, revision 0, no decision row); a decision is never deleted or +changed (``settle_linked_review_items``). The scan records through ``_record_scanned_review`` (best effort: a failure is logged and counted); the monitor calls ``record_review_file`` from its @@ -42,6 +47,9 @@ try: except ImportError: from utils.review_capture import record_ambiguous_file +# The scan log's reason for items that leave the queue with nothing to choose. +NOTHING_TO_CHOOSE = "nothing to choose" + class ReviewRecordingMixin: """Record the scan's and the monitor's ambiguous files for Needs review.""" @@ -49,7 +57,7 @@ class ReviewRecordingMixin: def _record_scanned_review(self, media_file, roots, review=None): """Record one ambiguous, unlinked scanned file (logged and counted).""" try: - outcome = self._record_review_sync(media_file, roots) + outcome = self._record_review_sync(media_file, roots, review) except Exception: self.logger.warning( f"{media_file.name} could not be added to Needs review", @@ -65,15 +73,22 @@ class ReviewRecordingMixin: if outcome in QUEUED_OUTCOMES: review["queued"] += 1 - def _record_review_sync(self, media_file, roots): - """``record_ambiguous_file`` for a file under one of ``roots`` (blocking).""" + def _record_review_sync(self, media_file, roots, review=None): + """``record_ambiguous_file`` for a file under one of ``roots`` (blocking). + + An ``untraced`` file (its captured bytes offer nothing to choose) also + loses its pending items that offer nothing either. + """ located = self._review_location(media_file, roots) if located is None: return "skipped" root, index, relative, _spelled = located - return record_ambiguous_file( + outcome = record_ambiguous_file( self.db, self._capture_store(), Path(root), index, relative ) + if outcome == "untraced": + self._settle_untraced_reviews([media_file], roots, review) + return outcome @staticmethod def _review_location(media_file, roots, real_roots=None): @@ -129,6 +144,33 @@ class ReviewRecordingMixin: review["settled"] += settled return settled + def _settle_untraced_reviews(self, media_files, roots, review=None): + """Remove the pending items without a choice of untraced ``media_files``. + + ``media_files`` are files found ambiguous with fewer than two + candidates that carry a prompt. Blocking; best effort like the linked + settle (a failure is logged and counted in ``review["errors"]``). + Returns how many items left. + """ + locators = [] + for media_file in media_files: + located = self._review_location(media_file, roots) + if located is not None: + locators.append(os.path.normcase(located[3])) + try: + removed = self.db.settle_untraced_review_items(locators) if locators else 0 + except Exception: + self.logger.warning( + "Needs review items without a choice could not be removed", + exc_info=True, + ) + if review is not None: + review["errors"] += 1 + return 0 + if review is not None: + review["untraced"] += removed + return removed + def _fallback_links(self, metadata): """True when the scan's legacy fallback would link these records.""" return bool(metadata.get("prompt")) and bool(self._graph_prompt_text(metadata)) @@ -140,7 +182,9 @@ class ReviewRecordingMixin: current root is ``skipped``. A linked file is never queued: its pending items are removed and the answer is ``linked``. An unlinked file the scan's fallback would link is ``skipped`` (the next scan - links it); any other file is recorded when it is ambiguous. While a + links it); any other file is recorded when it is ambiguous with a + choice, and is ``untraced`` (its pending items without a choice + removed) when it is ambiguous without one. While a job runs that a ``records`` job may not run beside, nothing is written and the answer is ``deferred``. Errors propagate: the monitor logs them and keeps its link. @@ -183,6 +227,12 @@ class ReviewRecordingMixin: f"{SCAN_LOG_PREFIXES['known']} " f"{_plural(review['settled'], 'image')} left Needs review · now linked" ) + if review["untraced"]: + job.log_line( + f"{SCAN_LOG_PREFIXES['known']} " + f"{_plural(review['untraced'], 'image')} left Needs review · " + f"{NOTHING_TO_CHOOSE}" + ) if review["errors"]: job.log_line( f"{SCAN_LOG_PREFIXES['errors']} " diff --git a/tests/test_api_admin.py b/tests/test_api_admin.py index 3ae7626..5db64b8 100644 --- a/tests/test_api_admin.py +++ b/tests/test_api_admin.py @@ -2223,7 +2223,9 @@ class TestActivityJobs(AdminAPITestCase): for name in names: (self.output_dir / name).write_bytes(b"not an image") - def fake_ingest(media_file, metadata, detailed=False, ambiguous=None): + def fake_ingest( + media_file, metadata, detailed=False, ambiguous=None, untraced=None + ): key = Path(media_file).relative_to(self.output_dir).as_posix() outcome = names[key] if isinstance(outcome, Exception): diff --git a/tests/test_api_review.py b/tests/test_api_review.py index 5e872f7..cb5e5a2 100644 --- a/tests/test_api_review.py +++ b/tests/test_api_review.py @@ -23,10 +23,17 @@ try: ReviewTestCase, ambiguous_graph, digest, + unlinkable_graph, write_png, ) except ImportError: # discovered with tests/ as the top-level directory - from test_review import ReviewTestCase, ambiguous_graph, digest, write_png + from test_review import ( + ReviewTestCase, + ambiguous_graph, + digest, + unlinkable_graph, + write_png, + ) from database.review_schema import canonical_evidence, evidence_digest from py.config import GalleryConfig @@ -284,13 +291,9 @@ class TestQueueReads(ReviewRoutesTestCase): self._require_routes() self._roots() limited = self.output_dir / "limited.png" - write_png( - limited, - parameters=( - "cat\nNegative prompt: blur\nNegative prompt: noise\n" - "Steps: 20, Sampler: Euler" - ), - ) + # More than 16 candidates with a prompt: limited, still a choice. + many = [f"prompt {n}" for n in range(17)] + write_png(limited, prompt=unlinkable_graph(*many)) self.assertEqual(self._record_raw(str(limited)), "limited") self._record(self._ambiguous("two.png")) items = (await self._list())["items"] diff --git a/tests/test_pair_scan.py b/tests/test_pair_scan.py index b8d914d..1597b1f 100644 --- a/tests/test_pair_scan.py +++ b/tests/test_pair_scan.py @@ -82,7 +82,8 @@ class TestPairScan(AdminAPITestCase): "class_type": "BasicGuider", "inputs": {"width": 2048, "height": 2048}, } - self.assertEqual(self._scanned("untraceable.png", g), "ambiguous") + # Ambiguous with nothing to choose: unresolved, never a review item. + self.assertEqual(self._scanned("untraceable.png", g), "unresolved") self.assertEqual(self.api.db.list_prompt_pairs()["total"], 0) self.assertEqual(self._texts(), []) @@ -159,9 +160,10 @@ class TestPairScan(AdminAPITestCase): "\nSteps: 20, Sampler: Euler" ) } + # No candidate to choose: reported unresolved, still not guessed. self.assertEqual( self.api._ingest_scanned_file(self.output_dir / "ambiguous.png", meta), - "ambiguous", + "unresolved", ) self.assertEqual(self.api.db.get_recent_prompts()["prompts"], []) diff --git a/tests/test_review.py b/tests/test_review.py index c118cea..70776c0 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -267,10 +267,12 @@ class TestScanRecording(ReviewTestCase): with self.assertLogs("prompt_manager.api", logging.WARNING) as logs: without = run(other, broken) self.assertEqual(recorded, without) - self.assertEqual(recorded[0]["ambiguous"], 2) + # delims.png offers nothing to choose: unresolved, not queued. + self.assertEqual(recorded[0]["ambiguous"], 1) + self.assertEqual(recorded[0]["unresolved"], 1) self.assertEqual(recorded[0]["added"], 1) self.assertTrue(any("Needs review" in line for line in logs.output)) - self.assertEqual(len(self._items()), 2) + self.assertEqual(len(self._items()), 1) other.close_annotation_captures() other_db.close_all() @@ -407,7 +409,9 @@ class TestScanRecording(ReviewTestCase): self.assertEqual(items[0]["locator"], items[1]["locator"]) self.assertEqual(job["scope"]["review_items"], 1) - async def test_parameters_delimiter_ambiguity_is_limited_without_candidates(self): + async def test_many_candidates_are_limited_without_candidates(self): + """Only ``many.png`` is queued: the A1111 delimiter ambiguity offers + no candidate to choose (``tests/test_review_choosable.py``).""" self._require_module() self._roots() write_png( @@ -420,7 +424,7 @@ class TestScanRecording(ReviewTestCase): job = await self._scan() items = {Path(item["locator"]).name: item for item in self._items()} - self.assertEqual(set(items), {"delims.png", "many.png"}) + self.assertEqual(set(items), {"many.png"}) for item in items.values(): self.assertEqual((item["limited"], item["candidate_count"]), (1, 0)) evidence = json.loads(item["evidence"]) @@ -431,7 +435,7 @@ class TestScanRecording(ReviewTestCase): json.loads(items["many.png"]["evidence"])["candidate_total"], 18, ) - self.assertEqual(job["scope"]["review_items"], 2) + self.assertEqual(job["scope"]["review_items"], 1) async def test_capacity_errors_are_logged_and_counted_not_fatal(self): self._require_module() @@ -533,8 +537,11 @@ class TestRecordAmbiguousFile(ReviewTestCase): self.assertEqual(self._record("two.png"), "duplicate") self.assertEqual(self._record("one.png"), "skipped") self.assertEqual(self._record("missing.png"), "skipped") - self.assertEqual(self._record("delims.png"), "limited") - self.assertEqual(self._record("delims.png"), "duplicate") + self.assertEqual(self._record("delims.png"), "untraced") + many = [f"prompt {n}" for n in range(17)] + write_png(self.output_dir / "many.png", prompt=unlinkable_graph(*many)) + self.assertEqual(self._record("many.png"), "limited") + self.assertEqual(self._record("many.png"), "duplicate") self.assertEqual(self._review_revision(), 2) async def test_restore_between_capture_and_write_skips(self): diff --git a/tests/test_review_choosable.py b/tests/test_review_choosable.py new file mode 100644 index 0000000..de86540 --- /dev/null +++ b/tests/test_review_choosable.py @@ -0,0 +1,335 @@ +"""Needs review queues a file only when there is something to choose. + +The maintainer's live queue (#284 / #288) held 39 items whose 11 candidates +all read "unresolved", "No positive / No negative": the ingestion resolver +found several sampler branches but could trace no prompt on any of them. +An ambiguous result is reviewable only when at least two candidates carry a +resolved prompt text. With none, the file could not be traced; with exactly +one, the other branches may still have made the file, so the one text is +not certain either (the resolver never flattens unknown semantics). Both +stay unresolved and are never queued, and a rescan removes the pending, +undecided items such files already have. +""" + +import os +import unittest +from unittest.mock import patch + +try: + from tests.test_review import DELIMITERS, write_png + from tests.test_review_outcome import OutcomeTestCase +except ImportError: # discovered with tests/ as the top-level directory + from test_review import DELIMITERS, write_png + from test_review_outcome import OutcomeTestCase + +from database.review_schema import evidence_offers_choice +from utils import review_capture + + +def opaque_graph(*opaque, traced=()): + """Several KSamplers: ``opaque`` positives come from an encoder the + ingestion resolver does not know, ``traced`` ones from CLIPTextEncode. + + The first sampler is opaque, so the scan's browser-parser fallback links + nothing either. + """ + graph = { + "1": {"class_type": "CheckpointLoaderSimple", "inputs": {"ckpt_name": "m"}}, + "2": {"class_type": "CLIPTextEncode", "inputs": {"text": "bad"}}, + } + sides = [("OpaqueTextEncode", "prompt", t) for t in opaque] + sides += [("CLIPTextEncode", "text", t) for t in traced] + for index, (encoder_class, key, text) in enumerate(sides): + encoder, sampler = str(10 + 2 * index), str(11 + 2 * index) + graph[encoder] = {"class_type": encoder_class, "inputs": {key: text}} + graph[sampler] = { + "class_type": "KSampler", + "inputs": { + "model": ["1", 0], + "positive": [encoder, 0], + "negative": ["2", 0], + }, + } + return graph + + +def _always_reviewable(_resolution): + return True + + +class ChoosableTestCase(OutcomeTestCase): + def _record_pre_fix(self, name): + """An item as the pre-fix recorder stored it (no choosable test).""" + with patch.object(review_capture, "reviewable", _always_reviewable): + self._record(name) + + def _write(self, name, graph): + write_png(self.output_dir / name, prompt=graph) + return self.output_dir / name + + +class TestChoosableCandidates(unittest.TestCase): + def _resolution(self, *states): + candidates = [] + for index, state in enumerate(states): + side = {"text": f"t{index}"} if state == "resolved" else None + candidates.append( + {"sampler_id": str(index), "state": state, "positive": side} + ) + return {"state": "ambiguous", "candidates": candidates} + + def test_two_resolved_candidates_are_reviewable(self): + resolution = self._resolution("resolved", "resolved", "unresolved") + self.assertTrue(review_capture.reviewable(resolution)) + + def test_none_or_one_resolved_candidate_is_not_reviewable(self): + for states in ((), ("unresolved",) * 11, ("resolved", "unresolved")): + with self.subTest(states=states): + resolution = self._resolution(*states) + self.assertFalse(review_capture.reviewable(resolution)) + + def test_blank_or_missing_text_does_not_count(self): + resolution = self._resolution("resolved", "resolved") + resolution["candidates"][0]["positive"] = {"text": " "} + self.assertFalse(review_capture.reviewable(resolution)) + resolution["candidates"][0]["positive"] = None + self.assertFalse(review_capture.reviewable(resolution)) + + def test_nested_ambiguous_stage_counts_its_own_candidates(self): + inner = self._resolution("resolved", "resolved")["candidates"] + resolution = { + "state": "ambiguous", + "candidates": [{"state": "ambiguous", "candidates": inner}], + } + self.assertTrue(review_capture.reviewable(resolution)) + + def test_a_result_that_is_not_ambiguous_is_not_reviewable(self): + resolution = self._resolution("resolved", "resolved") + resolution["state"] = "resolved" + self.assertFalse(review_capture.reviewable(resolution)) + + +class TestStoredEvidenceOffersChoice(unittest.TestCase): + def test_unlimited_evidence_needs_two_choosable_candidates(self): + side = {"text": "cat"} + two = [{"state": "resolved", "positive": side}] * 2 + self.assertTrue(evidence_offers_choice({"candidates": two})) + one = two[:1] + [{"state": "unresolved", "positive": None}] + self.assertFalse(evidence_offers_choice({"candidates": one})) + + def test_limited_evidence_offers_a_choice_only_with_two_candidates(self): + self.assertTrue( + evidence_offers_choice({"limited": True, "candidate_total": 18}) + ) + for total in (0, 1, None, "18", True): + with self.subTest(total=total): + evidence = {"limited": True, "candidate_total": total} + self.assertFalse(evidence_offers_choice(evidence)) + + def test_malformed_evidence_offers_nothing(self): + for evidence in (None, [], {"candidates": "x"}, {}): + with self.subTest(evidence=evidence): + self.assertFalse(evidence_offers_choice(evidence)) + + +class TestNothingToChooseIsNotQueued(ChoosableTestCase): + async def test_no_candidate_with_text_could_not_be_traced(self): + self._roots() + self._write("none.png", opaque_graph("cat", "dog")) + + job = await self._scan() + + self.assertEqual(self._items(), []) + self.assertEqual(self._pending(), 0) + self.assertEqual(job["scope"]["review_items"], 0) + (row,) = job["outcomes"]["review"] + self.assertEqual(row["path"], "none.png") + self.assertIn("could not be traced", row["detail"]) + self.assertEqual(job["counters"]["review"], 1, job["counters"]) + self.assertEqual(self._capture_files(), []) + + async def test_exactly_one_candidate_with_text_stays_unresolved(self): + """One traced branch beside untraced ones is not certain: no link.""" + self._roots() + self._write("one.png", opaque_graph("cat", traced=("dog",))) + + job = await self._scan() + + self.assertEqual(self._items(), []) + (row,) = job["outcomes"]["review"] + self.assertIn("could not be traced", row["detail"]) + self.assertEqual(job["counters"].get("added", 0), 0, job["counters"]) + self.assertEqual(job["counters"].get("known", 0), 0, job["counters"]) + + async def test_two_candidates_with_text_are_queued(self): + self._roots() + self._write("two.png", opaque_graph("x", traced=("cat", "dog"))) + + job = await self._scan() + + (item,) = self._items() + self.assertEqual(os.path.basename(item["locator"]), "two.png") + self.assertEqual(item["candidate_count"], 3) + self.assertEqual(job["scope"]["review_items"], 1) + (row,) = job["outcomes"]["review"] + self.assertIn("more than one prompt could match", row["detail"]) + + async def test_parameters_delimiter_ambiguity_is_not_queued(self): + """Two A1111 negative delimiters offer no candidate to choose.""" + self._roots() + write_png(self.output_dir / "delims.png", parameters=DELIMITERS) + + job = await self._scan() + + self.assertEqual(self._items(), []) + (row,) = job["outcomes"]["review"] + self.assertIn("could not be traced", row["detail"]) + + def test_recorder_reports_untraced_and_stores_nothing(self): + self._roots() + self._write("none.png", opaque_graph("cat", "dog")) + self._write("one.png", opaque_graph("cat", traced=("dog",))) + for name in ("none.png", "one.png"): + with self.subTest(name=name): + outcome = review_capture.record_ambiguous_file( + self.api.db, self.api._captures, self.output_dir, 0, name + ) + self.assertEqual(outcome, "untraced") + self.assertEqual(self._items(), []) + self.assertEqual(self._capture_files(), []) + + def test_monitor_does_not_queue_an_untraced_output(self): + self._roots() + path = self._write("none.png", opaque_graph("cat", "dog")) + + self.assertEqual(self.api.record_review_file(str(path)), "untraced") + self.assertEqual(self._items(), []) + + +class TestUntracedItemsLeaveTheQueue(ChoosableTestCase): + async def test_rescan_removes_pending_items_without_a_choice(self): + """The maintainer's 39 items: queued before the rule, gone on rescan.""" + self._roots() + self._write("none.png", opaque_graph("cat", "dog")) + self._write("one.png", opaque_graph("cat", traced=("dog",))) + self._write("two.png", opaque_graph("x", traced=("cat", "dog"))) + for name in ("none.png", "one.png", "two.png"): + self._record_pre_fix(name) + revision = self._review_revision() + self.assertEqual(self._pending(), 3) + + job = await self._scan("all") + + (item,) = self._items() + self.assertEqual(os.path.basename(item["locator"]), "two.png") + self.assertEqual(self._pending(), 1) + self.assertGreater(self._review_revision(), revision) + self.assertEqual(job["scope"]["review_untraced"], 2) + self.assertEqual(job["scope"]["review_items"], 1) + self.assertIn( + "= 2 images left Needs review · nothing to choose", "\n".join(job["log"]) + ) + + async def test_decided_items_of_untraced_files_are_never_touched(self): + self._roots() + for name in ("sel.png", "unres.png"): + self._write(name, opaque_graph("cat", traced=(name, name + "2"))) + self._record_pre_fix(name) + items = {os.path.basename(i["locator"]): i for i in self._items()} + self._decide(items["sel.png"], "selected") + self._decide(items["unres.png"], "unresolved") + before = (self._items(), self._decisions()) + # The files now carry graphs without a choice, queued pre-fix. + for name in ("sel.png", "unres.png", "pending.png"): + self._write(name, opaque_graph("a cow", "dog")) + self._record_pre_fix(name) + self.assertEqual(self._pending(), 3) + + job = await self._scan("all") + + self.assertEqual((self._items(), self._decisions()), before) + self.assertEqual(self._pending(), 0) + self.assertEqual(job["scope"]["review_untraced"], 3) + + async def test_an_item_with_a_choice_survives_beside_an_untraced_one(self): + """Removal judges each item by its own evidence: an item that offers + a choice is not removed although the file's new bytes offer none.""" + self._roots() + self._write("f.png", opaque_graph("x", traced=("cat", "dog"))) + self._record_pre_fix("f.png") + (choosable,) = self._items() + self._write("f.png", opaque_graph("a cow", "dog")) + self._record_pre_fix("f.png") + self.assertEqual(self._pending(), 2) + + job = await self._scan("all") + + self.assertEqual([i["id"] for i in self._items()], [choosable["id"]]) + self.assertEqual(job["scope"]["review_untraced"], 1) + + async def test_limited_items_go_only_without_enough_candidates(self): + """A pre-fix delimiter item (no candidate) goes; a limited item of 18 + candidates may hold a choice and stays, though the bytes now offer + none.""" + self._roots() + write_png(self.output_dir / "delims.png", parameters=DELIMITERS) + self._record_pre_fix("delims.png") + many = [f"prompt {n}" for n in range(17)] + self._write("many.png", opaque_graph("x", traced=many)) + self._record_pre_fix("many.png") + kept = [i["id"] for i in self._items() if "many" in i["locator"]] + self._write("many.png", opaque_graph("a cow", "dog")) + + job = await self._scan("all") + + self.assertEqual([i["id"] for i in self._items()], kept) + self.assertEqual(job["scope"]["review_untraced"], 1) + + async def test_choosable_pending_item_stays(self): + self._roots() + self._write("two.png", opaque_graph("x", traced=("cat", "dog"))) + await self._scan() + (item,) = self._items() + + job = await self._scan("all") + + self.assertEqual([i["id"] for i in self._items()], [item["id"]]) + self.assertEqual(job["scope"]["review_untraced"], 0) + + async def test_a_failed_removal_is_logged_and_counted_not_fatal(self): + import logging + import sqlite3 + + self._roots() + self._write("none.png", opaque_graph("cat", "dog")) + self._record_pre_fix("none.png") + with ( + patch.object( + self.api.db, + "settle_untraced_review_items", + side_effect=sqlite3.OperationalError("database is locked"), + ), + self.assertLogs("prompt_manager.api", logging.WARNING) as logs, + ): + job = await self._scan("all") + + self.assertEqual(job["state"], "finished") + self.assertEqual(job["scope"]["review_errors"], 1) + self.assertTrue(any("could not be removed" in line for line in logs.output)) + self.assertEqual(self._pending(), 1) + await self._scan("all") + self.assertEqual(self._pending(), 0) + + def test_monitor_removes_the_pending_item_of_an_untraced_output(self): + self._roots() + path = self._write("none.png", opaque_graph("cat", "dog")) + self._record_pre_fix("none.png") + # A pre-fix item of other bytes keeps the stat shortcut out of the way. + self._write("none.png", opaque_graph("a cow", "dog")) + + self.assertEqual(self.api.record_review_file(str(path)), "untraced") + self.assertEqual(self._items(), []) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_review_outcome.py b/tests/test_review_outcome.py index 3c36658..c4952c8 100644 --- a/tests/test_review_outcome.py +++ b/tests/test_review_outcome.py @@ -184,10 +184,12 @@ class TestOnlyUnlinkedAmbiguousFilesAreQueued(OutcomeTestCase): if "could not be traced" in row["detail"] ) queued = [name for name in review if name not in unresolved_rows] - self.assertEqual(queued, ["delims.png", "u1.png", "u2.png"], job) - self.assertEqual(job["scope"]["review_items"], 3) - self.assertEqual(self._pending(), 3) - self.assertIn("? 3 images already in Needs review", "\n".join(job["log"])) + # delims.png has nothing to choose: "could not be traced". + self.assertEqual(queued, ["u1.png", "u2.png"], job) + self.assertIn("delims.png", unresolved_rows) + self.assertEqual(job["scope"]["review_items"], 2) + self.assertEqual(self._pending(), 2) + self.assertIn("? 2 images already in Needs review", "\n".join(job["log"])) class TestQueueSelfHeals(OutcomeTestCase): diff --git a/utils/review_capture.py b/utils/review_capture.py index 31b9e2b..f26a46b 100644 --- a/utils/review_capture.py +++ b/utils/review_capture.py @@ -6,15 +6,21 @@ finds ``ambiguous`` to :func:`record_ambiguous_file`. It captures the file reads the metadata again from the capture, resolves it with the default (ingestion) profile and stores only an ``ambiguous`` result, bound to the capture's byte record. The evidence a reviewer chooses from is therefore the -evidence of exactly those bytes. Nothing here links a file, loads a model or -decodes pixels; the capture is released before the database write. +evidence of exactly those bytes. An ambiguous result is stored only when it +is ``reviewable``: at least two candidates carry a resolved prompt text. With +none the file could not be traced; with exactly one, the untraced branches +may still have made the file, so that text is not certain and the file stays +unresolved (the resolver never flattens unknown semantics into a guess). +Nothing here links a file, loads a model or decodes pixels; the capture is +released before the database write. Evidence is small on purpose (startup validates every row): the resolver's reason and, per candidate sampler, ``{sampler_id, state, reason, positive, negative}`` where each side is ``{text, storage_text, provenance}``. A candidate's ``hash`` is the SHA-256 of its canonical JSON without the hash. -More than ``MAX_CANDIDATES`` candidates, none at all, or evidence over the -8 MiB bound make a ``limited`` item without candidates. +More than ``MAX_CANDIDATES`` candidates or evidence over the 8 MiB bound make +a ``limited`` item without candidates (a result without any candidate is not +``reviewable`` and is never stored). """ from __future__ import annotations @@ -28,13 +34,23 @@ from typing import Any try: from ..database.library_commands import LibraryCommandError - from ..database.review_schema import MAX_CANDIDATES, canonical_evidence + from ..database.review_schema import ( + MAX_CANDIDATES, + MIN_CHOOSABLE, + canonical_evidence, + choosable, + ) from .media_capture import CaptureCapacity, CaptureError from .prompt_inspector import MetadataReadError, read_inspection_file from .prompt_pair_resolver import RESOLVER_VERSION, resolve_prompt_pair except ImportError: # imported with the repo root on sys.path from database.library_commands import LibraryCommandError - from database.review_schema import MAX_CANDIDATES, canonical_evidence + from database.review_schema import ( + MAX_CANDIDATES, + MIN_CHOOSABLE, + canonical_evidence, + choosable, + ) from utils.media_capture import CaptureCapacity, CaptureError from utils.prompt_inspector import MetadataReadError, read_inspection_file from utils.prompt_pair_resolver import RESOLVER_VERSION, resolve_prompt_pair @@ -55,7 +71,9 @@ def record_ambiguous_file(db, store, root, root_index: int, relative: str) -> st evidence, or the file's current stat is the verified fingerprint of bytes an item cites, so it is not even copied, and that item is still pending), ``decided`` (as ``duplicate``, but the item carries a decision, - so the file is not in the queue) or ``skipped`` (the file + so the file is not in the queue), ``untraced`` (ambiguous in its captured + bytes, but not ``reviewable``: nothing to choose, nothing stored) or + ``skipped`` (the file changed, vanished, is not ambiguous in its captured bytes, or storage was restored meanwhile). ``CaptureCapacity`` and database errors propagate to the caller, which @@ -87,6 +105,8 @@ def record_ambiguous_file(db, store, root, root_index: int, relative: str) -> st resolution = captured[1] if captured is not None else None if resolution is None or resolution.get("state") != "ambiguous": return "skipped" + if not reviewable(resolution): + return "untraced" evidence, count, limited = review_evidence(resolution) try: inserted = db.record_review_item( @@ -180,6 +200,24 @@ def captured_review(store, capture) -> tuple[dict, dict] | None: return canonical, resolve_prompt_pair(canonical) +def reviewable(resolution: Any) -> bool: + """True when an ambiguous result offers a choice: ``MIN_CHOOSABLE`` of its + selectable candidates (nested ambiguous stages flattened) are choosable. + + Counted before the evidence is bounded, so a ``limited`` item (more than + ``MAX_CANDIDATES`` candidates) with prompts to choose is still queued. + """ + if not isinstance(resolution, dict) or resolution.get("state") != "ambiguous": + return False + count = 0 + for candidate in _flattened(resolution.get("candidates"), 0): + if choosable(candidate): + count += 1 + if count >= MIN_CHOOSABLE: + return True + return False + + def review_evidence(resolution: dict) -> tuple[dict, int, bool]: """``(evidence, candidate_count, limited)`` for one ambiguous result.""" base = {