From 7511a0fd3837502b2fa680c12ff18d977b432034 Mon Sep 17 00:00:00 2001 From: Vito Date: Fri, 23 Oct 2026 06:29:45 -0700 Subject: [PATCH 1/2] fix(activity): no Cancel on jobs that ended before Activity opened (#306) * fix(activity): drop Cancel job from a job that had ended before Activity opened Since #289 the Activity list removed an item's actions only when they were isConnected. On the first render the item is not in the page yet, so a job that had already ended when Activity opened kept its "Cancel job" (and a running job's progress was moved after its actions, then back). The list now asks parentNode instead. The review spec "a scan job with review items offers Review metadata in Activity" failed on this before the change (Expected 0 buttons on a finished job, Received 1) and passes after it. * test(backup): wait for the restore, not a 5 s timer, in the thread test test_database_is_usable_from_every_thread_after_restore let its worker wait 5 s for the restore. On a stalled Windows runner (py3.11, where the previous restore test also took 25 s instead of 0.3 s) the restore took longer, the wait timed out, and the worker read the old database before the restore ran. The worker now waits for the event under a 120 s bound that only a hung test reaches, reports a timeout as an error, and the main thread releases it even when the restore raises. With a restore delayed by 6 s the old test fails and the new one passes. * test(activity): no Cancel job on jobs that ended before Activity opened Since #289 a list item dropped its actions only when they were isConnected, so the first render of jobs the store already knew (Activity opened from another page) kept "Cancel job" on a finished or cancelled job and moved a running job's progress after its actions. Activity's own list read renders again a moment later and hides it, so the new spec holds that read and checks the first render: no progress or Cancel job on the ended jobs, and a running job's children in order heading, detail, progress, actions. With the parentNode fix reverted it fails on chromium and touch (Expected 0, Received 1 on the finished job); with it, it passes. --- tests/browser/workspace-activity.spec.mjs | 36 +++++++++++++++++++++++ tests/test_database_backup.py | 22 ++++++++++---- web/js/workspace/activity.mjs | 8 +++-- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/tests/browser/workspace-activity.spec.mjs b/tests/browser/workspace-activity.spec.mjs index cbd848c0..a3a16165 100644 --- a/tests/browser/workspace-activity.spec.mjs +++ b/tests/browser/workspace-activity.spec.mjs @@ -724,6 +724,42 @@ test('a held legacy job shows no Cancel in the list or its view', async ({ page, await expect(heading(page).locator('.heading-actions > button')).toHaveText(['Download report']); }); +// Since #289 an item dropped its Cancel job only when it was already in the page, so the first render of jobs the +// store already knew (Activity opened from another page) kept a Cancel job on a job that had ended, and moved a +// running job's progress after its actions. The read Activity starts on opening is held, so that first render is +// what the page shows (the read's own render would otherwise repair it a moment later). +test('jobs that ended before Activity opened show no Cancel job, a running one keeps its progress first', async ({ page, request }) => { + await page.setViewportSize({ width: 1600, height: 1100 }); + const finished = await startJob(request, { title: 'Finished scan' }); + await finish(request, finished.id); + const cancelled = await startJob(request, { title: 'Cancelled scan' }); + await finish(request, cancelled.id, 'cancelled'); + await startJob(request, { title: 'Running scan' }); + // The page's first list read has all three jobs once the running one shows in the pill. + await page.goto(`${workspace}#/images`); + await expect(pill(page)).toBeVisible(); + let hold; + const held = new Promise(resolve => { hold = resolve; }); + // Only the first read is held; any later one goes through. + await page.route(url => url.pathname === '/prompt_manager/jobs', route => { + if (!hold) return route.continue(); + hold(route); + hold = null; + }); + await activityButton(page).click(); + const route = await held; + await expect(items(page)).toHaveCount(3); + const item = title => items(page).filter({ has: page.getByRole('link', { name: new RegExp(`${title}$`) }) }); + for (const ended of ['Finished scan', 'Cancelled scan']) { + await expect(item(ended).locator('progress, .row-actions')).toHaveCount(0); + await expect(item(ended).getByRole('button', { name: 'Cancel job', exact: true })).toHaveCount(0); + } + await expect(item('Running scan').getByRole('button', { name: 'Cancel job', exact: true })).toHaveCount(1); + expect(await item('Running scan').evaluate(node => [...node.children].map(child => child.localName + (child.className ? `.${child.className}` : '')))) + .toEqual(['div.panel-heading', 'p', 'progress', 'div.row-actions']); + await route.continue(); +}); + test('busy start names the running job', async ({ page, request }) => { await page.setViewportSize({ width: 1600, height: 1100 }); // The page's timers are Playwright's, so the toast's lifetime below is run out rather than waited for. diff --git a/tests/test_database_backup.py b/tests/test_database_backup.py index deb1346f..93686971 100644 --- a/tests/test_database_backup.py +++ b/tests/test_database_backup.py @@ -24,6 +24,9 @@ from database.operations import PromptDatabase from tests.open_handles import assert_closed from utils.hashing import generate_prompt_hash +# A bound on thread hand-offs that only a hung test reaches (never a pacing timer). +THREAD_WAIT_SECONDS = 120 + def _count_prompts(path): """Row count read through a brand-new connection (closed before return).""" @@ -350,7 +353,13 @@ class TestRestoreFromFile(BackupTestCase): def worker(): self.db.search_prompts(text="old") # opens this thread's connection ready.set() - proceed.wait(5) + # Wait for the restore itself, not for a timer: a stalled Windows + # runner once took longer than 5 s to restore, and the worker then + # read the old database before the restore had run. + if not proceed.wait(THREAD_WAIT_SECONDS): + results["error"] = "the restore did not finish" + self.db.close() + return try: results["texts"] = [p["text"] for p in self.db.search_prompts()] except Exception as exc: # noqa: BLE001 - reported through results @@ -360,12 +369,15 @@ class TestRestoreFromFile(BackupTestCase): thread = threading.Thread(target=worker) thread.start() - ready.wait(5) + self.assertTrue(ready.wait(THREAD_WAIT_SECONDS)) - self.db.model.restore_from_file(src) - proceed.set() - thread.join(5) + try: + self.db.model.restore_from_file(src) + finally: + proceed.set() + thread.join(THREAD_WAIT_SECONDS) + self.assertFalse(thread.is_alive()) self.assertNotIn("error", results) self.assertEqual(results["texts"], ["restored"]) self.assertEqual([p["text"] for p in self.db.search_prompts()], ["restored"]) diff --git a/web/js/workspace/activity.mjs b/web/js/workspace/activity.mjs index eb7bc14b..f4deedee 100644 --- a/web/js/workspace/activity.mjs +++ b/web/js/workspace/activity.mjs @@ -148,18 +148,20 @@ function itemFor(job, scope) { chip.textContent = state; detail.textContent = jobDetail(next); const running = next.state === 'running'; + // parentNode, not isConnected: on the first render the item is not in the page yet, so a job that had already + // ended when Activity opened kept its Cancel job (and a running one's progress moved after its actions). if (running) { progress.value = next.progress; progress.textContent = `${next.progress}%`; progress.setAttribute('aria-label', `${next.title} progress`); - if (!progress.isConnected) item.insertBefore(progress, actions.isConnected ? actions : null); + if (!progress.parentNode) item.insertBefore(progress, actions.parentNode ? actions : null); } else { progress.remove(); } // Truth rule: a job that cannot be cancelled (a held legacy-page job) shows no Cancel job. if (running && next.cancellable) { - if (!actions.isConnected) item.append(actions); - } else if (actions.isConnected) { + if (!actions.parentNode) item.append(actions); + } else if (actions.parentNode) { // A focused Cancel job that goes away hands its focus to the item's title link, never to the page body. if (actions.contains(document.activeElement)) link.focus(); actions.remove(); From 966a25d1293c2c9b59278059452b9a27a072979c Mon Sep 17 00:00:00 2001 From: Vito Date: Fri, 23 Oct 2026 06:48:21 -0700 Subject: [PATCH 2/2] fix(scan): ingest metadata whose graph holds NaN or Infinity (#303) * fix(scan): ingest metadata whose graph holds NaN or Infinity ComfyUI writes its prompt with Python's default json.dumps, so a node whose IS_CHANGED returns float("nan") saves "is_changed": [NaN] in every PNG. The exact-pair ingestion stored the snapshots with allow_nan=False, so the scan failed those files ("Out of range float values are not JSON compliant: nan") and the monitor dropped their link. The legacy pages replaced NaN with null before JSON.parse and ingested them. - utils/json_strict.dumps_strict: strict JSON with non-finite numbers as null; identical bytes for every other value. - Pair snapshots and provenance, and the plain prompt link, store strict JSON (the legacy link wrote bare NaN into the database). - The browser-parser scan fallback parses records as the legacy pages do (parse_page_json), so a graph with NaN still traces its prompt. - The prompt images route answers strict JSON instead of regex-replacing "NaN" (which also rewrote prompt text such as "ratio: NaN"); the NaN cleaners also null Infinity. Prompt text and its hash are unchanged: only numbers become null. * fix(scan): leave the browser-parser NaN parse to the resolver parity fix The scan fallback's legacy-page parse (parse_page_json in _graph_prompt_text) is owned by the resolver parity change, which adds the same helper with the legacy pages' exact rule (NaN reads as null, Infinity still rejected). This branch keeps only the strict serialization of stored and answered JSON. --- database/operations.py | 13 +- database/pair_ingestion.py | 10 +- py/api/__init__.py | 5 +- py/api/images.py | 30 +-- tests/test_nonfinite_metadata.py | 360 +++++++++++++++++++++++++++++++ utils/json_strict.py | 42 ++++ 6 files changed, 427 insertions(+), 33 deletions(-) create mode 100644 tests/test_nonfinite_metadata.py create mode 100644 utils/json_strict.py diff --git a/database/operations.py b/database/operations.py index 8424e880..024dc867 100644 --- a/database/operations.py +++ b/database/operations.py @@ -4,6 +4,7 @@ Database operations for KikoTextEncode prompt storage and retrieval. import sqlite3 import json +import math import datetime import os from typing import Optional, List, Dict, Any, Set, Tuple @@ -26,6 +27,7 @@ from .health import HealthOperations, iter_consistency_issues try: from ..utils.logging_config import get_logger from ..utils.hashing import generate_prompt_hash + from ..utils.json_strict import dumps_strict except ImportError: import sys @@ -33,6 +35,7 @@ except ImportError: sys.path.insert(0, current_dir) from utils.logging_config import get_logger from utils.hashing import generate_prompt_hash + from utils.json_strict import dumps_strict # Subquery to fetch tags from junction table, embedded in SELECT statements @@ -1697,9 +1700,9 @@ class PromptDatabase( width, height, image_format, - json.dumps(metadata.get("workflow", {})), - json.dumps(metadata.get("prompt", {})), - json.dumps(metadata.get("parameters", {})), + dumps_strict(metadata.get("workflow", {})), + dumps_strict(metadata.get("prompt", {})), + dumps_strict(metadata.get("parameters", {})), ), ) conn.commit() @@ -1904,13 +1907,13 @@ class PromptDatabase( obj: The object to clean (dict, list, or scalar) Returns: - Cleaned object with NaN values replaced by None + Cleaned object with NaN and Infinity values replaced by None """ if isinstance(obj, dict): return {key: self._clean_nan_values(value) for key, value in obj.items()} elif isinstance(obj, list): return [self._clean_nan_values(item) for item in obj] - elif isinstance(obj, float) and str(obj) == "nan": + elif isinstance(obj, float) and not math.isfinite(obj): return None else: return obj diff --git a/database/pair_ingestion.py b/database/pair_ingestion.py index cad3641a..27781987 100644 --- a/database/pair_ingestion.py +++ b/database/pair_ingestion.py @@ -1,6 +1,5 @@ """Atomic metadata enrichment for a single known generation context.""" -import json import os from typing import Any, Dict @@ -9,8 +8,10 @@ from .operations import _coerce_file_info try: from ..utils.hashing import generate_prompt_hash + from ..utils.json_strict import dumps_strict except ImportError: from utils.hashing import generate_prompt_hash + from utils.json_strict import dumps_strict def ingest_resolved_pair( @@ -24,9 +25,7 @@ def ingest_resolved_pair( negative_text = negative.get("storage_text", negative["text"]) normalized = normalize_image_path(image_path) provenance = {"resolver_version": 1, "positive": positive, "negative": negative} - encoded = json.dumps( - provenance, sort_keys=True, separators=(",", ":"), allow_nan=False - ) + encoded = dumps_strict(provenance, sort_keys=True, separators=(",", ":")) with db._pair_transaction() as conn: images = conn.execute( "SELECT gi.*,p.hash AS positive_hash FROM generated_images gi " @@ -67,8 +66,9 @@ def ingest_resolved_pair( ).fetchone() pairs_added = int(prior_pair is None) file_size, width, height, image_format = _coerce_file_info(metadata) + # ComfyUI's NaN/Infinity are stored as null: the snapshot is strict JSON. snapshots = { - column: json.dumps(metadata[key], sort_keys=True, allow_nan=False) + column: dumps_strict(metadata[key], sort_keys=True) for column, key in ( ("workflow_data", "workflow"), ("prompt_metadata", "prompt"), diff --git a/py/api/__init__.py b/py/api/__init__.py index fbff121f..34ae5dac 100644 --- a/py/api/__init__.py +++ b/py/api/__init__.py @@ -16,6 +16,7 @@ import datetime import functools import gzip as gzip_module import json +import math import ntpath import os import re @@ -609,12 +610,12 @@ class PromptManagerAPI( return prompts def _clean_nan_recursive(self, obj): - """Recursively clean NaN values from nested data structures.""" + """Recursively replace NaN and Infinity in nested data with None.""" if isinstance(obj, dict): return {key: self._clean_nan_recursive(value) for key, value in obj.items()} elif isinstance(obj, list): return [self._clean_nan_recursive(item) for item in obj] - elif isinstance(obj, float) and str(obj) == "nan": + elif isinstance(obj, float) and not math.isfinite(obj): return None else: return obj diff --git a/py/api/images.py b/py/api/images.py index 69ad96fa..3bf0a341 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -3,6 +3,7 @@ import asyncio import base64 import binascii +import functools import hashlib import hmac import json @@ -37,6 +38,7 @@ try: from ...database.models import normalize_image_path from ...utils.card_seed import card_seed, header_params, parsed_positive from ...utils.parallel import map_parallel + from ...utils.json_strict import dumps_strict from ...utils.inspection_response import ( encode_inspection_response, library_metadata, @@ -56,6 +58,7 @@ except ImportError: from database.models import normalize_image_path from utils.card_seed import card_seed, header_params, parsed_positive from utils.parallel import map_parallel + from utils.json_strict import dumps_strict from utils.inspection_response import ( encode_inspection_response, library_metadata, @@ -982,27 +985,12 @@ class ImageRoutesMixin: # Add url and thumbnail_url so the frontend can serve them cleaned_images = self._present_images(cleaned_images) - # Additional fallback: convert to JSON string and clean NaN values manually - try: - response_data = {"success": True, "images": cleaned_images} - # Convert to JSON string - json_str = json.dumps(response_data, default=str) - - # Clean any remaining NaN values with regex - json_str = re.sub(r":\s*NaN", ": null", json_str) - json_str = re.sub(r"\[\s*NaN\s*\]", "[null]", json_str) - json_str = re.sub(r",\s*NaN\s*,", ", null,", json_str) - json_str = re.sub(r",\s*NaN\s*\]", ", null]", json_str) - json_str = re.sub(r"\[\s*NaN\s*,", "[null,", json_str) - - # Parse back to verify it's valid JSON - cleaned_data = json.loads(json_str) - - return web.json_response(cleaned_data) - except Exception as json_error: - self.logger.error(f"JSON cleaning error: {json_error}") - # Fallback to original response - return web.json_response({"success": True, "images": cleaned_images}) + # Rows an older version stored may still hold NaN/Infinity: answer + # strict JSON, and leave prompt text that spells "NaN" untouched. + return web.json_response( + {"success": True, "images": cleaned_images}, + dumps=functools.partial(dumps_strict, default=str), + ) except Exception as e: self.logger.error(f"Get prompt images error: {e}") diff --git a/tests/test_nonfinite_metadata.py b/tests/test_nonfinite_metadata.py new file mode 100644 index 00000000..ff6abcd5 --- /dev/null +++ b/tests/test_nonfinite_metadata.py @@ -0,0 +1,360 @@ +"""ComfyUI's ``NaN``/``Infinity`` metadata ingests, and is stored as strict JSON. + +ComfyUI serializes the prompt with Python's ``json.dumps``, so a node whose +``IS_CHANGED`` returns ``float("nan")`` (the "always re-run" idiom) writes +``"is_changed": [NaN]`` into every saved PNG; widgets can hold ``Infinity``. +The legacy pages replaced ``NaN`` with ``null`` before ``JSON.parse``, so such +files must ingest. Whatever is stored or answered as JSON stays strict (the +non-finite numbers become ``null``) and the prompt text and hash never change. +""" + +import json +import logging +import os +import sqlite3 +import sys +import tempfile +import time +import types +import unittest +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch + +from PIL import Image, PngImagePlugin + +from database.operations import PromptDatabase +from tests.history_waits import drain_writer +from tests.test_api_admin import AdminAPITestCase +from tests.test_api_images import ImageAPITestCase +from utils.hashing import generate_prompt_hash +from utils.image_monitor import ImageGenerationHandler + +try: + from tests.open_handles import assert_closed +except ImportError: # discovered with tests/ as the top-level directory + from open_handles import assert_closed + +# The words "NaN" and "Infinity" inside the prompt are text, never numbers. +POSITIVE = " A cat named NaN beside an Infinity pool, ratio: NaN, [NaN] " +NEGATIVE = "blurry" +NAN, INF = float("nan"), float("inf") + + +def graph(positive=POSITIVE, negative=NEGATIVE): + """An API prompt graph with non-finite numbers where ComfyUI writes them.""" + g = { + "1": {"class_type": "CLIPTextEncode", "inputs": {"text": positive}}, + "2": {"class_type": "CLIPTextEncode", "inputs": {"text": negative}}, + "3": { + "class_type": "KSampler", + "inputs": {"positive": ["1", 0], "negative": ["2", 0], "model": ["4", 0]}, + }, + "4": {"class_type": "CheckpointLoaderSimple", "inputs": {"ckpt_name": "m"}}, + "5": { + "class_type": "Image Filter", + "inputs": {"strength": INF, "floor": -INF}, + "_meta": {"title": "Image Filter"}, + "is_changed": [NAN], + }, + } + return g + + +WORKFLOW = { + "nodes": [ + {"id": 5, "type": "Image Filter", "widgets_values": [NAN, INF, -INF]}, + ], + "links": [], +} + + +def comfy_text(value): + """The record text ComfyUI writes: Python's default, non-strict JSON.""" + text = json.dumps(value) + assert "NaN" in text and "Infinity" in text + return text + + +def write_png(path, prompt_graph=None, workflow=WORKFLOW): + info = PngImagePlugin.PngInfo() + info.add_text("prompt", comfy_text(prompt_graph or graph())) + if workflow is not None: + info.add_text("workflow", comfy_text(workflow)) + path.parent.mkdir(parents=True, exist_ok=True) + with Image.new("RGB", (4, 4)) as image: + image.save(path, pnginfo=info) + return path + + +def _reject(name): + raise ValueError(f"non-strict JSON constant {name}") + + +def strict_loads(text): + return json.loads(text, parse_constant=_reject) + + +class StrictRows: + """Assertions over the rows a write left behind.""" + + def image_rows(self, db): + with db.model.get_connection() as conn: + return [ + dict(row) + for row in conn.execute( + "SELECT prompt_id, workflow_data, prompt_metadata, parameters " + "FROM generated_images ORDER BY id" + ) + ] + + def assert_strict_snapshot(self, row, *, workflow=True): + prompt = strict_loads(row["prompt_metadata"]) + self.assertEqual(prompt["5"]["is_changed"], [None]) + self.assertEqual(prompt["5"]["inputs"], {"strength": None, "floor": None}) + self.assertEqual(prompt["1"]["inputs"]["text"], POSITIVE) + stored_workflow = strict_loads(row["workflow_data"]) + if workflow: + self.assertEqual( + stored_workflow["nodes"][0]["widgets_values"], [None, None, None] + ) + if row["parameters"] is not None: + strict_loads(row["parameters"]) + + def assert_prompt_text(self, db, prompt_id): + with db.model.get_connection() as conn: + row = conn.execute( + "SELECT text, hash FROM prompts WHERE id=?", (prompt_id,) + ).fetchone() + self.assertEqual(row["text"], POSITIVE.strip()) + self.assertEqual(row["hash"], generate_prompt_hash(POSITIVE)) + + +def dumps_strict(value, **kwargs): + from utils.json_strict import dumps_strict as dumps + + return dumps(value, **kwargs) + + +class TestDumpsStrict(unittest.TestCase): + def test_finite_values_serialize_byte_identically(self): + value = {"b": [1, 2.5, "NaN", None, True], "a": {"x": "Infinity"}} + for kwargs in ({}, {"sort_keys": True, "separators": (",", ":")}): + with self.subTest(kwargs=kwargs): + self.assertEqual( + dumps_strict(value, **kwargs), + json.dumps(value, allow_nan=False, **kwargs), + ) + + def test_non_finite_numbers_become_null_and_strings_stay(self): + value = {"n": NAN, "i": [INF, -INF], "s": "NaN Infinity", "t": (NAN, 1)} + encoded = dumps_strict(value, sort_keys=True) + self.assertEqual( + strict_loads(encoded), + {"n": None, "i": [None, None], "s": "NaN Infinity", "t": [None, 1]}, + ) + + def test_other_errors_still_raise(self): + cycle = [] + cycle.append(cycle) + for value in (cycle, [NAN, cycle]): + with self.assertRaisesRegex(ValueError, "Circular"): + dumps_strict(value) + with self.assertRaises(TypeError): + dumps_strict({"x": object()}) + self.assertEqual( + dumps_strict({"x": object()}, default=lambda _: NAN), '{"x": null}' + ) + + +class TestScanIngestsNonFiniteMetadata(StrictRows, AdminAPITestCase): + def ingest(self, name, prompt_graph=None): + path = write_png(self.output_dir / name, prompt_graph) + extracted = self.api._extract_comfyui_metadata(str(path)) + return self.api._ingest_scan_batch_sync([(path, extracted)]) + + def test_resolved_pair_with_nan_is_added_with_strict_snapshots(self): + counts = self.ingest("nan.png") + self.assertEqual(counts["failed"], 0) + self.assertEqual(counts["added"], 1) + rows = self.image_rows(self.api.db) + self.assertEqual(len(rows), 1) + self.assert_strict_snapshot(rows[0]) + self.assert_prompt_text(self.api.db, rows[0]["prompt_id"]) + pairs = self.api.db.list_prompt_pairs()["pairs"] + self.assertEqual( + [(p["positive_text"], p["negative_text"]) for p in pairs], + [(POSITIVE.strip(), NEGATIVE)], + ) + # A repeated scan compares the same strict snapshot: unchanged. + self.assertEqual(self.ingest("nan.png")["unchanged"], 1) + + async def test_scan_route_reports_no_failures(self): + from py.config import GalleryConfig + + write_png(self.output_dir / "nan.png") + GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)] + resp = await self.client.request("POST", "/prompt_manager/scan") + body = await resp.text() + self.assertEqual(resp.status, 200) + self.assertIn('"type": "complete"', body) + self.assertIn('"added": 1', body) + self.assertIn('"failed": 0', body) + + +class TestMonitorIngestsNonFiniteMetadata(StrictRows, unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.db = PromptDatabase(str(Path(self.temp.name) / "db.sqlite")) + self.pid = self.db.save_prompt( + POSITIVE.strip(), prompt_hash=generate_prompt_hash(POSITIVE) + ) + self.tracker = SimpleNamespace( + pop_next_prompt=lambda: None, + get_current_prompt=lambda: None, + extend_prompt_timeout=lambda *args: None, + ) + self.handler = ImageGenerationHandler(self.db, self.tracker) + self.handler.logger = logging.getLogger("test.nonfinite.monitor") + self.handler.logger.disabled = True + + def tearDown(self): + self.db.close_all() + assert_closed(self, self.temp.name) + self.temp.cleanup() + + def test_new_image_with_nan_links_its_pair(self): + # The monitor links only images a PromptManager node produced. + g = graph() + g["1"]["class_type"] = "PromptManager" + path = write_png(Path(self.temp.name) / "out" / "nan.png", g) + self.handler.process_new_image(str(path)) + rows = self.image_rows(self.db) + self.assertEqual([row["prompt_id"] for row in rows], [self.pid]) + self.assert_strict_snapshot(rows[0]) + self.assert_prompt_text(self.db, self.pid) + pairs = self.db.list_prompt_pairs()["pairs"] + self.assertEqual(len(pairs), 1) + self.assertEqual(pairs[0]["image_count"], 1) + + def test_legacy_link_stores_strict_json(self): + # The plain prompt link (no exact pair) must not write NaN either. + path = str(Path(self.temp.name) / "plain.png") + metadata = {"prompt": graph(), "workflow": WORKFLOW, "parameters": {"x": NAN}} + self.assertTrue(self.db.link_image_to_prompt(self.pid, path, metadata)) + row = self.image_rows(self.db)[0] + self.assert_strict_snapshot(row) + self.assertEqual(strict_loads(row["parameters"]), {"x": None}) + + def test_stored_images_read_back_without_non_finite_numbers(self): + path = str(Path(self.temp.name) / "legacy.png") + self.db.link_image_to_prompt(self.pid, path, {"prompt": graph()}) + with self.db.model.get_connection() as conn: + # A row an older version wrote with Python's default json.dumps. + conn.execute( + "UPDATE generated_images SET prompt_metadata=?", (comfy_text(graph()),) + ) + conn.commit() + images = self.db.get_prompt_images(str(self.pid)) + strict_loads(json.dumps(images, default=str)) + + +class TestInspectorAnswersStrictJSON(ImageAPITestCase): + async def test_inspect_route_resolves_and_answers_strict_json(self): + write_png(self.output_dir / "nan.png") + response = await self.client.get( + "/prompt_manager/images/inspect/nan.png", params={"root": "0"} + ) + self.assertEqual(response.status, 200) + result = strict_loads(await response.text()) + self.assertEqual(result["resolution"]["state"], "resolved") + self.assertEqual( + result["prompts"]["positive"], + {"status": "resolved", "text": POSITIVE, "source": "graph-inferred"}, + ) + + async def test_prompt_images_route_answers_strict_json(self): + pid = self.api.db.save_prompt( + POSITIVE.strip(), prompt_hash=generate_prompt_hash(POSITIVE) + ) + path = str(write_png(self.output_dir / "linked.png")) + self.api.db.link_image_to_prompt(pid, path, {"prompt": graph()}) + with self.api.db.model.get_connection() as conn: + conn.execute( + "UPDATE generated_images SET prompt_metadata=?", (comfy_text(graph()),) + ) + conn.commit() + response = await self.client.get(f"/prompt_manager/prompts/{pid}/images") + self.assertEqual(response.status, 200) + result = strict_loads(await response.text()) + self.assertEqual(len(result["images"]), 1) + stored = result["images"][0]["prompt_metadata"] + self.assertEqual(stored["1"]["inputs"]["text"], POSITIVE) + self.assertEqual(stored["5"]["is_changed"], [None]) + + +class TestHistoryCapturesNonFiniteGraphs(unittest.TestCase): + """The queued graph ComfyUI hands the hook may carry float('nan').""" + + JOB = "a0c26da3-eb26-405c-b143-c662a84c3112" + + def setUp(self): + self.directory = tempfile.TemporaryDirectory() + self.addCleanup(self.directory.cleanup) + self.db = PromptDatabase(os.path.join(self.directory.name, "history.db")) + self.addCleanup(self.db.model.reset_schema_cache) + self.addCleanup(self.db.model.close_all) + + def test_queued_graph_with_nan_records_its_run_and_contexts(self): + from tests.test_execution_history_hooks import HostQueue, status + from utils.execution_context_capture import record_runtime_context + from utils.execution_history import register_history_hook + + queue = HostQueue() + registration = register_history_hook( + SimpleNamespace(prompt_queue=queue), lambda: self.db + ) + self.addCleanup(lambda: registration.dispose(3)) + self.addCleanup(registration.writer.invalidate) + deadline = time.monotonic() + 3 + while not registration.writer.status().accepting: + self.assertLess(time.monotonic(), deadline, "writer never accepted") + time.sleep(0.005) + g = { + "p": { + "class_type": "PromptManagerPair", + "inputs": {"text": POSITIVE, "negative_text": NEGATIVE}, + }, + "s": { + "class_type": "KSampler", + "inputs": {"positive": ["p", 0], "negative": ["p", 1], "cfg": INF}, + "is_changed": [NAN], + }, + "5": graph()["5"], + } + extra = {"extra_pnginfo": {"workflow": WORKFLOW}} + queue.results.append(((0, self.JOB, g, extra, ["s"]), 1)) + queue.get() + host = types.ModuleType("comfy_execution.utils") + host.get_executing_context = lambda: SimpleNamespace( + prompt_id=self.JOB, node_id="p", list_index=0 + ) + with patch.dict(sys.modules, {"comfy_execution.utils": host}): + for role, text in (("positive", POSITIVE), ("negative", NEGATIVE)): + record_runtime_context( + "PromptManagerPair", text, "", "", text, role=role + ) + queue.task_done(1, {}, status("execution_success", job=self.JOB)) + drain_writer(registration.writer, clock=time.monotonic) + with self.db.model.get_connection() as conn: + conn.row_factory = sqlite3.Row + runs = [dict(r) for r in conn.execute("SELECT * FROM execution_runs")] + self.assertEqual([run["outcome"] for run in runs], ["succeeded"]) + contexts = self.db.get_execution_contexts(runs[0]["run_id"]) + self.assertEqual(contexts["state"], "complete") + self.assertEqual(contexts["contexts"][0]["positive"]["raw"], POSITIVE) + strict_loads(json.dumps(contexts)) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/json_strict.py b/utils/json_strict.py new file mode 100644 index 00000000..7eeb8aed --- /dev/null +++ b/utils/json_strict.py @@ -0,0 +1,42 @@ +"""Strict JSON output for values that may carry ComfyUI's non-finite numbers. + +ComfyUI writes its metadata with Python's default ``json.dumps``, so a graph can +hold ``NaN``, ``Infinity`` and ``-Infinity`` (an ``IS_CHANGED`` that returns +``float("nan")`` puts ``"is_changed": [NaN]`` into every saved prompt). Those +literals are not JSON: browsers' ``JSON.parse`` and ``allow_nan=False`` reject +them. Reading follows the legacy pages (``parse_page_json`` where the browser +parser is mirrored; Python's ``json.loads`` elsewhere). Whatever PromptManager +stores or answers as JSON goes through ``dumps_strict``, which writes each +non-finite number as ``null`` (the legacy pages' own replacement for NaN) and +leaves strings, including the words ``NaN`` or ``Infinity`` inside prompt +text, untouched. +""" + +import json +from typing import Any + +__all__ = ["dumps_strict"] + + +def _null(_constant: str) -> None: + return None + + +def dumps_strict(value: Any, **kwargs: Any) -> str: + """``json.dumps(value, allow_nan=False, **kwargs)`` with non-finite numbers null. + + A value without non-finite numbers serializes byte for byte as the plain + call does. Any other error (a cycle, an unserializable object) still raises. + """ + kwargs.pop("allow_nan", None) + try: + return json.dumps(value, allow_nan=False, **kwargs) + except ValueError as error: + if "Out of range float" not in str(error): + raise + # The non-strict text spells each non-finite number as a bare literal, never + # inside a string, so parsing it back maps exactly those numbers to null. + lenient = json.dumps(value, allow_nan=True, default=kwargs.get("default")) + return json.dumps( + json.loads(lenient, parse_constant=_null), allow_nan=False, **kwargs + )