fix(quality): sweep the Code Quality standard findings (#558)

* fix(quality): preserve cursor iteration and option validation

Keep prefetched rows on ManagedCursor so partial loops and explicit fetches share one position, and stop existing iterators after close or retirement. Preserve chunked reads and native SQLite result lifetime on reexecution and errors.

Catch OptionError before ValueError so legacy auto-tag requests receive the actual validation error. Remove the overwritten ViewerJS title option.

Regression tests failed before the fixes for lost rows, reads after close/retirement, and misleading option errors. Focused Python checks pass; cursor checks also pass on Python 3.13 and 3.14. Refs #321.

* fix(quality): make the test doubles legal raises and hashes

The fake queue no longer raises an attribute that starts as None. The
oversized kind records hashes instead of raising from __hash__, which
also catches a hash that production code swallows. Hostile.__bool__
raises the TypeError a truth test may raise.

* fix(quality): drop the extra saveCoverage argument

* refactor(quality): remove assignments that are overwritten before use

* refactor(quality): remove the remaining overwritten test assignments

* refactor(quality): drop imports repeated inside functions

* refactor(quality): give duplicate identity keys one shape

* refactor(quality): name the unused job event loop variable

* refactor(quality): release model gates in finally, not except BaseException

* refactor(quality): end the job runners with an explicit return

* refactor(quality): use the precise unittest asserts

* refactor(quality): import unittest.mock one way in the tests

* fix(quality): say why each empty except is safe, log the broad ones

* refactor(quality): stop binding values the tests never read

* refactor(quality): remove the unused preview aliases and tag page query

* refactor(quality): remove re-exports nothing imports

* refactor(quality): parenthesize deliberate string joins in lists

* refactor(quality): remove unused JavaScript variables and imports

* refactor(quality): use the precise assert in the model status test

py/api/admin.py

@@ -176,7 +176,7 @@ def _physical_file_key(path, stat):
     volumes on Windows) falls back to the resolved, case-folded path.
     """
     if stat.st_ino:
-        return ("inode", stat.st_dev, stat.st_ino)
+        return ("inode", (stat.st_dev, stat.st_ino))
     return ("path", os.path.normcase(os.path.realpath(str(path))))
 
 
@@ -539,6 +539,7 @@ def _diagnose_output_dirs(configured_dirs, public_path=None):
 
         candidates.append(folder_paths.get_output_directory())
     except ImportError:
+        # outside ComfyUI there is no output folder to add
         pass
 
     output_dirs = []

py/api/annotation_suggest.py

@@ -13,7 +13,6 @@ import math
 from ..tagging_defaults import (  # noqa: F401 - re-exported for the route
     LLM_MODEL_TYPES as LLM_MODELS,
     MAX_INSTRUCTION_BYTES,
-    MAX_INSTRUCTION_CHARS,
     MODEL_TYPES,
     WD14_MODEL_TYPES as WD14_MODELS,
     validate_template,

py/api/autotag_routes.py

@@ -424,6 +424,7 @@ class AutotagRoutesMixin:
         title = f"Tagging model download · {label}"
         _download_titles.clear()
         _download_titles[owner] = title
+        admitted = False
         try:
             job = self._jobs.start(
                 MODEL_DOWNLOAD_JOB_KIND,
@@ -439,13 +440,13 @@ class AutotagRoutesMixin:
                     self._run_model_download, service, model_type, owner
                 ),
             )
-        except BaseException as refused:
-            # JobBusy, or anything else the registry raised: give the gate back.
-            model_gate.release(owner)
-            _download_titles.pop(owner, None)
-            if isinstance(refused, JobBusy):
-                return None, busy_response(refused)
-            raise
+            admitted = True
+        except JobBusy as refused:
+            return None, busy_response(refused)
+        finally:
+            if not admitted:  # JobBusy, or anything else: give the gate back.
+                model_gate.release(owner)
+                _download_titles.pop(owner, None)
 
         def ended(_task):
             model_gate.release(owner)
@@ -1041,12 +1042,12 @@ class AutotagRoutesMixin:
                 return self._bad_request('existing must be "skip" or "merge"')
             try:
                 options = self._single_options(data, model_type)
+            except OptionError as error:
+                return self._bad_request(str(error))
             except (ValueError, TypeError):
                 return self._bad_request(
                     "general_threshold and character_threshold must be numeric"
                 )
-            except OptionError as error:
-                return self._bad_request(str(error))
             target, error = await self._select_autotag_target(data)
             if error is not None:
                 return error

py/api/backup_restore.py

@@ -971,6 +971,7 @@ class BackupRestoreRoutesMixin:
         line = restored_line(staged.summary)
         job.log_line(line)
         job.update(progress=100, status_line=line)
+        return None
 
 
 async def _outlive_cancel(future):

py/api/duplicate_files.py

@@ -574,12 +574,13 @@ class DuplicateFilesMixin:
             return _fail_job(job, "sources-changed", SOURCES_CHANGED)
         ok, backup = await self._safety_backup_step(job, db, backup_on)
         if not ok:
-            return
+            return None
         try:
             await self._remove_duplicates(job, db, plan, backup)
         finally:
             if backup:
                 await self._prune_backup_pool(job, db)
+        return None
 
     async def _remove_duplicates(self, job, db, plan, backup):
         targets = plan.payload["targets"]

py/api/duplicate_spellings.py

@@ -27,7 +27,7 @@ def _identity(path):
     except OSError:
         return None
     if info.st_ino:
-        return ("inode", info.st_dev, info.st_ino)
+        return ("inode", (info.st_dev, info.st_ino))
     return ("path", os.path.normcase(os.path.realpath(str(path))))
 
 

py/api/image_delete.py

@@ -330,12 +330,13 @@ class ImageDeleteMixin:
             return _fail_job(job, "sources-changed", SOURCES_CHANGED)
         ok, backup = await self._safety_backup_step(job, db, backup_on)
         if not ok:
-            return
+            return None
         try:
             await self._delete_images(job, db, targets, mode, backup)
         finally:
             if backup:
                 await self._prune_backup_pool(job, db)
+        return None
 
     async def _delete_images(self, job, db, targets, mode, backup):
         job.update(total=len(targets), status_line=TITLES[mode])

py/api/image_tag_jobs.py

@@ -531,6 +531,7 @@ class ImageTagJobsMixin:
         run = AutotagRun(
             model_type, existing, options, keep_in_memory, backup_on, owner
         )
+        admitted = False
         try:
             job = self._jobs.start(
                 AUTOTAG_BATCH_JOB_KIND,
@@ -541,11 +542,12 @@ class ImageTagJobsMixin:
                 groups=AUTOTAG_GROUP_NAMES,
                 runner=functools.partial(self._run_autotag_batch, run),
             )
-        except BaseException as refused:
-            model_gate.release(owner)
-            if isinstance(refused, JobBusy):
-                return None, _busy_refusal(refused)
-            raise
+            admitted = True
+        except JobBusy as refused:
+            return None, _busy_refusal(refused)
+        finally:
+            if not admitted:
+                model_gate.release(owner)
         job.task.add_done_callback(lambda _task: model_gate.release(owner))
         return job, None
 

py/api/images.py

@@ -39,10 +39,8 @@ from .thumbnail_encode import (
     PREVIEW_HELD,
     EncoderClosed,
     PreviewWriteHeld,
-    replace_unless_held,
     run_encoder,
     sweep_stray_temps,
-    temp_sibling,
     thumbnail_frame,
     writer_settings,
     write_thumbnail,
@@ -339,11 +337,6 @@ def _thumbnail_batch_size(workers):
     return max(4, workers * 2)
 
 
-# Preview files are written by thumbnail_encode in the configured format
-# (thumbnail_settings, #317); these names stay importable from here.
-_temp_sibling = temp_sibling
-_replace_unless_held = replace_unless_held
-
 # A preview that could not be replaced (#249 S5 M6). Windows refuses to rename
 # over a file another process has open (WinError 5 or 32), such as a preview
 # the gallery is serving; the old preview stays and the file counts as an error.
@@ -396,6 +389,7 @@ def _video_placeholder_image(thumbnail_size):
             font=font,
         )
     except (OSError, AttributeError):
+        # a placeholder without its label still works
         pass
     return img
 
@@ -499,6 +493,7 @@ def _preview_names(folders, folder):
                     except OSError:
                         continue
         except OSError:
+            # an unreadable folder lists as empty; nothing there to cache
             pass
         folders[folder] = names
     return names

py/api/job_routes.py

@@ -250,7 +250,7 @@ class JobRoutesMixin:
         try:
             await response.write(_summary_event(job))
             async with contextlib.aclosing(job.events()) as events:
-                async for _payload in events:
+                async for _ in events:
                     await response.write(_summary_event(job))
             # The last event is published before the job takes its final state.
             await response.write(_summary_event(job))

py/api/library_location.py

@@ -207,6 +207,7 @@ class LibraryLocationRoutesMixin:
         try:
             size = os.path.getsize(location.db_path)
         except OSError:
+            # no library file yet: report size 0
             pass
         return {
             "path": self._library_public_path(location.db_path),
@@ -373,6 +374,7 @@ class LibraryLocationRoutesMixin:
         line += ". Logs move at the next restart."
         job.log_line(line)
         job.update(progress=100, status_line=line)
+        return None
 
     @staticmethod
     def _fail_library_move(job, code, message):

py/api/log_cleanup.py

@@ -205,3 +205,4 @@ class LogCleanupMixin:
             line += f", {failed} could not be deleted"
         job.log_line(line)
         job.update(processed=len(files), progress=100, status_line=line)
+        return None

py/api/lora_import_routes.py

@@ -436,7 +436,7 @@ class LoraImportRoutesMixin:
             job.log_line(f"{CANCELLED} {CACHE_NOTE}")
             job.update(status_line=CANCELLED)
             job.finish("cancelled")
-            return
+            return None
         except Exception:
             self.logger.exception("LoRA import staging failed")
             return _fail_job(job, "import-failed", f"{FAILED} {CACHE_NOTE}")
@@ -455,10 +455,10 @@ class LoraImportRoutesMixin:
             job.log_line(f"{CANCELLED} {CACHE_NOTE}")
             job.update(status_line=CANCELLED)
             job.finish("cancelled")
-            return
+            return None
         ok, _backup = await self._safety_backup_step(job, db, backup_on)
         if not ok:
-            return
+            return None
         # I1: the files are listed and fingerprinted again right before the
         # replace, so a file changed, removed or added while the downloads ran
         # (LoraManager refreshing its CivitAI info) aborts with nothing deleted.
@@ -466,6 +466,7 @@ class LoraImportRoutesMixin:
             return _fail_job(job, "plan-changed", f"{SOURCE_CHANGED} {CACHE_NOTE}")
         job.update(status_line="Replacing the LoRA collection", progress=95)
         await self._replace_in_job(job, db, plan, staged)
+        return None
 
     def _count_downloads(self, job, staged):
         downloaded = staged.stats.get("downloaded", 0)
@@ -518,3 +519,4 @@ class LoraImportRoutesMixin:
             staged.stats.get("download_errors", 0),
         )
         job.update(progress=100, processed=job.total, status_line=line)
+        return None

py/api/lora_integration.py

@@ -536,6 +536,7 @@ class LoraIntegrationMixin:
                 except asyncio.CancelledError:
                     interrupted = True
                 except Exception:
+                    # the await after this loop raises it for the caller
                     pass
             if interrupted:
                 raise asyncio.CancelledError

py/api/maintenance.py

@@ -673,6 +673,7 @@ class MaintenanceRoutesMixin:
         line = _changed_line(plan.operation, result["changed"], result.get("marks", 0))
         job.log_line(line)
         job.update(progress=100, status_line=f"{line} · {backup_note(backup)}")
+        return None
 
     def _maintenance_rows(self, operation, described):
         """Outcome rows for the changed records (root-relative paths only)."""

py/api/thumbnail_auto.py

@@ -533,6 +533,7 @@ class AutoThumbnailer:
             try:
                 process.kill()
             except OSError:
+                # the process has already exited
                 pass
         if worker is None:
             return True

py/api/thumbnail_encode.py

@@ -158,6 +158,7 @@ def _unlink_quietly(path):
     try:
         Path(path).unlink()
     except FileNotFoundError:
+        # already gone, which is what was wanted
         pass
     except OSError as exc:
         logger.debug(f"Could not remove temp file {Path(path).name}: {exc}")
@@ -221,6 +222,7 @@ def shutdown_encoders():
         try:
             process.kill()
         except OSError:
+            # the process has already exited
             pass
 
 
@@ -359,10 +361,12 @@ def thumbnail_frame(img, edge):
     try:
         img.seek(0)
     except (EOFError, AttributeError, ValueError):
+        # not seekable or a single frame: use the current one
         pass
     try:
         img.draft("RGB", (edge, edge))  # JPEG: decode at a reduced scale
     except (AttributeError, ValueError, OSError):
+        # draft is only a speedup; the full decode below still works
         pass
     frame = img.convert("RGB") if img.mode != "RGB" else img.copy()
     frame.thumbnail((edge, edge), Image.Resampling.LANCZOS)

py/api/workflow_storage.py

@@ -275,3 +275,4 @@ class WorkflowStorageRoutesMixin:
         job.record_counted("changed", 1)
         job.log_line(line)
         job.update(progress=100, status_line=line)
+        return None