Merge remote-tracking branch 'origin/main' into fix/duplicate-delete-serve

ComfyUI stamps "is_changed": [NaN] on nodes that always re-run, the
PromptManager node among them. The legacy gallery and admin pages replaced
NaN with null before JSON.parse, so they traced these files' prompts. The
scan's fallback parsed records strictly, so every NaN-stamped file the
resolver could not decide (hires-fix and refiner chains, custom latent
nodes) fell to "Needs review" although legacy linked its prompt.

parse_page_json parses as the legacy pages did, but only the NaN number
reads as null: the word inside prompt text is kept. Infinity still makes
the record unreadable, as it did in the pages.

py/api/admin.py

@@ -41,7 +41,7 @@ try:
         PLACEHOLDERS,
         GenerationParamsError,
         extract_generation_params,
-        parse_js_json,
+        parse_page_json,
     )
     from ...utils.prompt_pair_resolver import resolve_prompt_pair
 except ImportError:
@@ -51,7 +51,7 @@ except ImportError:
         PLACEHOLDERS,
         GenerationParamsError,
         extract_generation_params,
-        parse_js_json,
+        parse_page_json,
     )
     from utils.prompt_pair_resolver import resolve_prompt_pair
 
@@ -804,6 +804,9 @@ class AdminRoutesMixin:
             result = await self._run_in_executor(
                 self._delete_duplicate_images_sync, image_paths, output_dir
             )
+            if result["deleted_count"]:
+                # The cached listing would keep (and count) the deleted files.
+                self.invalidate_gallery_cache()
 
             response_data = {
                 "success": True,
@@ -1603,11 +1606,14 @@ class AdminRoutesMixin:
 
     @staticmethod
     def _graph_prompt_text(metadata):
-        """The positive prompt the browser parser traces from the graphs, or None."""
+        """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_js_json(value) if isinstance(value, str) else value
+            records[key] = parse_page_json(value) if isinstance(value, str) else value
         try:
             positive = extract_generation_params(records)["positivePrompt"]
         except GenerationParamsError:

tests/browser/workflows.spec.mjs

@@ -354,39 +354,48 @@ test('prompt deletion requires confirmation and can retry after a network failur
     }
 });
 
-test('duplicate cleanup cancels safely and deletes only the selected file after a retry', async ({ page, request }) => {
+// The duplicate scan reads the output root only, but the fixture's input root holds a different
+// shared/same.png. A bare /images/serve/<path> URL tries every gallery root in turn, so once the
+// output copy is deleted it answers 200 with the input copy: check each file in its own root.
+const serveIn = (root, path) => `/prompt_manager/images/serve/${path.split(/[\\/]/).map(encodeURIComponent).join('/')}?root=${root}`;
+const rootOf = url => new URL(url, 'http://gallery.test').searchParams.get('root');
+
+// Which copy the scan keeps depends on file times: the fixture stamps shared/same.png with the
+// wall clock, and the kernel can stamp its bulk copies a fraction of a millisecond earlier.
+// listOrder rewrites the finished scan's groups so a test can pick the order it needs.
+async function openDuplicateScan(page, request, listOrder = null) {
     await expect(await request.post('/__test__/reset', { data: { extra_images: 2 } })).toBeOK();
+    if (listOrder) {
+        await page.route('**/prompt_manager/scan_duplicates/stream', async route => {
+            const response = await route.fetch();
+            const body = (await response.text()).replace(/^data: (.*)$/gm, (line, json) => {
+                const event = JSON.parse(json);
+                if (event.type !== 'complete') return line;
+                for (const group of event.duplicates) group.images = listOrder(group.images);
+                return `data: ${JSON.stringify(event)}`;
+            });
+            await route.fulfill({ response, body });
+        });
+    }
     await page.goto('/prompt_manager/gallery');
     await expect(page.locator('#galleryGrid .image-item')).toHaveCount(4);
     const originals = await page.locator('#galleryGrid img[data-original]').evaluateAll(images => images.map(image => image.dataset.original));
+    const output = rootOf(originals.find(url => url.includes('/bulk/')));
+    const input = rootOf(originals.find(url => url.includes('/shared/same.png') && rootOf(url) !== output));
+    expect(output).not.toBeNull();
+    expect(input).not.toBeNull();
     await page.locator('#settingsBtn').click();
     await page.locator('#scanDuplicatesBtn').click();
     await expect(page.locator('#duplicatesCount')).toHaveText('1');
     const candidates = page.locator('.duplicate-checkbox');
     await expect(candidates).toHaveCount(2);
-    const selected = await candidates.first().getAttribute('data-image-path');
-    const retained = await candidates.last().getAttribute('data-image-path');
-    const serve = path => `/prompt_manager/images/serve/${path.split(/[\\/]/).map(encodeURIComponent).join('/')}`;
-    await candidates.first().check();
-    let deletes = 0;
-    page.on('request', request => { if (request.url().endsWith('/delete_duplicate_images')) deletes++; });
-    confirmNextDialog(page, false, 'permanently delete 1 duplicate image files');
-    await page.locator('#removeDuplicatesBtn').click();
-    expect(deletes).toBe(0);
-    await expect(await request.get(serve(selected))).toBeOK();
-    await page.route('**/prompt_manager/delete_duplicate_images', route => route.abort('failed'), { times: 1 });
-    confirmNextDialog(page, true, 'permanently delete 1 duplicate image files');
-    await page.locator('#removeDuplicatesBtn').click();
-    await expect(page.getByText('Failed to delete duplicate images', { exact: true })).toBeVisible();
-    await expect(page.locator('#removeDuplicatesBtn')).toBeEnabled();
-    await expect(await request.get(serve(selected))).toBeOK();
-    confirmNextDialog(page, true, 'permanently delete 1 duplicate image files');
-    const deleted = page.waitForResponse(response => response.url().endsWith('/delete_duplicate_images'));
-    await page.locator('#removeDuplicatesBtn').click();
-    expect((await (await deleted).json())).toMatchObject({ success: true, deleted_count: 1, failed_count: 0 });
-    await expect(page.locator('#duplicatesModal')).toBeHidden();
-    expect((await request.get(serve(selected))).status()).toBe(404);
-    await expect(await request.get(serve(retained))).toBeOK();
+    return { originals, output, input, candidates };
+}
+
+async function expectOnlySelectedGone(page, request, { originals, output, input }, selected, retained) {
+    expect((await request.get(serveIn(output, selected))).status()).toBe(404);
+    await expect(await request.get(serveIn(output, retained))).toBeOK();
+    await expect(await request.get(serveIn(input, 'shared/same.png'))).toBeOK();
     await page.reload();
     await expect(page.locator('#galleryGrid .image-item')).toHaveCount(3);
     const remaining = await page.locator('#galleryGrid img[data-original]').evaluateAll(images => images.map(image => image.dataset.original));
@@ -395,4 +404,47 @@ test('duplicate cleanup cancels safely and deletes only the selected file after
         expect(originals).toContain(url);
         await expect(await request.get(url)).toBeOK();
     }
+}
+
+test('duplicate cleanup cancels safely and deletes only the selected file after a retry', async ({ page, request }) => {
+    const scan = await openDuplicateScan(page, request);
+    const { output, candidates } = scan;
+    const selected = await candidates.first().getAttribute('data-image-path');
+    const retained = await candidates.last().getAttribute('data-image-path');
+    await candidates.first().check();
+    let deletes = 0;
+    page.on('request', request => { if (request.url().endsWith('/delete_duplicate_images')) deletes++; });
+    confirmNextDialog(page, false, 'permanently delete 1 duplicate image files');
+    await page.locator('#removeDuplicatesBtn').click();
+    expect(deletes).toBe(0);
+    await expect(await request.get(serveIn(output, selected))).toBeOK();
+    await page.route('**/prompt_manager/delete_duplicate_images', route => route.abort('failed'), { times: 1 });
+    confirmNextDialog(page, true, 'permanently delete 1 duplicate image files');
+    await page.locator('#removeDuplicatesBtn').click();
+    await expect(page.getByText('Failed to delete duplicate images', { exact: true })).toBeVisible();
+    await expect(page.locator('#removeDuplicatesBtn')).toBeEnabled();
+    await expect(await request.get(serveIn(output, selected))).toBeOK();
+    confirmNextDialog(page, true, 'permanently delete 1 duplicate image files');
+    const deleted = page.waitForResponse(response => response.url().endsWith('/delete_duplicate_images'));
+    await page.locator('#removeDuplicatesBtn').click();
+    expect((await (await deleted).json())).toMatchObject({ success: true, deleted_count: 1, failed_count: 0 });
+    await expect(page.locator('#duplicatesModal')).toBeHidden();
+    await expectOnlySelectedGone(page, request, scan, selected, retained);
+});
+
+test('deleting a duplicate whose path another gallery root also holds removes only the scanned copy', async ({ page, request }) => {
+    // The order CI saw: a bulk copy stamped first, so shared/same.png is the first Delete candidate.
+    const ciOrder = ['bulk/0000.png', 'shared/same.png', 'bulk/0001.png'];
+    const rank = image => ciOrder.indexOf(image.path.replace(/\\/g, '/'));
+    const scan = await openDuplicateScan(page, request, images => [...images].sort((a, b) => rank(a) - rank(b)));
+    const selected = await scan.candidates.first().getAttribute('data-image-path');
+    const retained = await scan.candidates.last().getAttribute('data-image-path');
+    expect(selected.replace(/\\/g, '/')).toBe('shared/same.png');
+    await scan.candidates.first().check();
+    confirmNextDialog(page, true, 'permanently delete 1 duplicate image files');
+    const deleted = page.waitForResponse(response => response.url().endsWith('/delete_duplicate_images'));
+    await page.locator('#removeDuplicatesBtn').click();
+    expect((await (await deleted).json())).toMatchObject({ success: true, deleted_count: 1, failed_count: 0 });
+    await expect(page.locator('#duplicatesModal')).toBeHidden();
+    await expectOnlySelectedGone(page, request, scan, selected, retained);
 });

tests/test_api_admin.py

@@ -868,6 +868,23 @@ class TestDeleteDuplicatesEndpoint(AdminAPITestCase):
         self.assertFalse(target.exists())
         self.assertTrue(thumb_dir.exists())
 
+    async def test_deleted_files_leave_the_cached_gallery_listing(self):
+        # The listing is cached for 30 s; a delete must not leave the gallery
+        # listing (and counting) a file that is gone.
+        GalleryConfig.MONITORING_DIRECTORIES = [self._root(self.output_dir)]
+        for name in ("keep.png", "gone.png"):
+            (self.output_dir / name).write_bytes(b"same bytes")
+
+        async def listed():
+            resp = await self.client.get("/prompt_manager/images/output")
+            data = await resp.json()
+            return data["total"], sorted(i["filename"] for i in data["images"])
+
+        self.assertEqual(await listed(), (2, ["gone.png", "keep.png"]))
+        resp = await self._post({"image_paths": ["gone.png"]})
+        self.assertEqual((await resp.json())["deleted_count"], 1)
+        self.assertEqual(await listed(), (1, ["keep.png"]))
+
     def test_unexpected_error_per_file_is_reported_without_path(self):
         self.api._delete_one_duplicate = lambda *a: (_ for _ in ()).throw(
             PermissionError(13, "Permission denied", os.path.join(self.tmpdir, "x.png"))

tests/test_generation_params.py

@@ -5,7 +5,10 @@ returns for the same input (tests/test_generation_params_js_parity.py checks
 that directly when Node is available).
 """
 
+import json
 import os
+import shutil
+import subprocess  # nosec B404 - runs a fixed Node one-liner
 import sys
 import unittest
 
@@ -17,6 +20,7 @@ from utils.generation_params import (  # noqa: E402
     extract_generation_params,
     extract_from_records,
     parse_js_json,
+    parse_page_json,
 )
 
 try:
@@ -219,6 +223,47 @@ class ParsesLikeJavaScript(unittest.TestCase):
             run("seed-beyond-double-precision")["seed"], 4974052037430000001.0
         )
 
+    def test_page_parse_reads_nan_numbers_as_null(self):
+        # gallery.js and admin.js replace NaN with null before JSON.parse.
+        self.assertEqual(
+            parse_page_json('{"a": NaN, "b": [NaN], "t": "NaN"}'),
+            {"a": None, "b": [None], "t": "NaN"},
+        )
+        self.assertEqual(
+            parse_page_json('{"seed": 9007199254740993}'), {"seed": 9007199254740992.0}
+        )
+        for text in ('{"a": Infinity}', "[-Infinity]", "not json", "", None):
+            with self.subTest(text=text):
+                self.assertIsNone(parse_page_json(text))
+
+    @unittest.skipUnless(shutil.which("node"), "Node is not installed")
+    def test_page_parse_equals_the_legacy_pages_for_nan_numbers(self):
+        # gallery.js/admin.js: two NaN regexes, then JSON.parse (Infinity throws).
+        texts = [
+            '{"1": {"is_changed": [NaN]}, "2": {"is_changed": NaN, "v": 1.5}}',
+            '{"a":NaN,"b":[NaN, 2]}',
+            '{"a": Infinity}',
+            '{"seed": 9007199254740993}',
+        ]
+        script = (
+            "const texts = JSON.parse(require('fs').readFileSync(0, 'utf8'));"
+            "console.log(JSON.stringify(texts.map((t) => { try { return {v: JSON.parse("
+            "t.replace(/:\\s*NaN\\b/g, ': null').replace(/\\bNaN\\b/g, 'null'))}; }"
+            " catch (e) { return null; } })));"
+        )
+        completed = subprocess.run(  # nosec B603 B607 - fixed argv, no shell
+            ["node", "-e", script],
+            input=json.dumps(texts),
+            capture_output=True,
+            text=True,
+            check=True,
+            timeout=60,
+        )
+        expected = json.loads(completed.stdout)
+        for text, page in zip(texts, expected):
+            with self.subTest(text=text):
+                self.assertEqual(parse_page_json(text), page and page["v"])
+
     def test_duplicate_keys_keep_the_last_value(self):
         self.assertEqual(parse_js_json('{"a": 1, "a": 2}'), {"a": 2})
 

tests/test_pair_scan.py

@@ -165,6 +165,122 @@ class TestPairScan(AdminAPITestCase):
         )
         self.assertEqual(self.api.db.get_recent_prompts()["prompts"], [])
 
+    def _nan_scanned(self, name, prompt_graph):
+        """Scan ``prompt_graph`` as ComfyUI writes it: ``is_changed`` is NaN."""
+        stamped = {
+            node_id: {**node, "is_changed": [float("nan")]}
+            for node_id, node in prompt_graph.items()
+        }
+        text = json.dumps(stamped)
+        self.assertIn("NaN", text)
+        return self._scanned_text(name, text)
+
+    def _scanned_text(self, name, prompt_text):
+        meta = {"prompt": prompt_text}
+        return self.api._ingest_scanned_file(self.output_dir / name, meta)
+
+    @staticmethod
+    def _hires_fix_graph(positive):
+        """A PromptManager prompt on a base sampler and a hires-fix sampler.
+
+        The base latent comes from a custom batch node and the hires pass from
+        a custom image filter: no sampler branch traces, so the scan links the
+        prompt the legacy pages show.
+        """
+        return {
+            "1": {
+                "class_type": "CheckpointLoaderSimple",
+                "inputs": {"ckpt_name": "m.safetensors"},
+            },
+            "2": {
+                "class_type": "MyTriggerWords",
+                "inputs": {"trigger_words": ["1", 2]},
+            },
+            "3": {
+                "class_type": "PromptManager",
+                "inputs": {
+                    "text": positive,
+                    "prepend_text": ["2", 0],
+                    "append_text": "",
+                    "clip": ["1", 1],
+                },
+            },
+            "4": {
+                "class_type": "CLIPTextEncode",
+                "inputs": {"text": "blur", "clip": ["1", 1]},
+            },
+            "5": {"class_type": "MyLatentBatch", "inputs": {"batch_size": 6}},
+            "6": {
+                "class_type": "KSampler",
+                "inputs": {
+                    "seed": 7,
+                    "model": ["1", 0],
+                    "positive": ["3", 0],
+                    "negative": ["4", 0],
+                    "latent_image": ["5", 0],
+                },
+            },
+            "7": {
+                "class_type": "VAEDecode",
+                "inputs": {"samples": ["6", 0], "vae": ["1", 2]},
+            },
+            "8": {"class_type": "MyImageFilter", "inputs": {"images": ["7", 0]}},
+            "9": {
+                "class_type": "VAEEncode",
+                "inputs": {"pixels": ["8", 0], "vae": ["1", 2]},
+            },
+            "14": {
+                "class_type": "KSampler",
+                "inputs": {
+                    "seed": 7,
+                    "denoise": 0.25,
+                    "model": ["1", 0],
+                    "positive": ["3", 0],
+                    "negative": ["4", 0],
+                    "latent_image": ["9", 0],
+                },
+            },
+            "15": {
+                "class_type": "VAEDecode",
+                "inputs": {"samples": ["14", 0], "vae": ["1", 2]},
+            },
+            "16": {"class_type": "SaveImage", "inputs": {"images": ["15", 0]}},
+        }
+
+    def test_hires_fix_prompt_links_with_or_without_nan_is_changed(self):
+        # Legacy parity: the legacy pages read NaN as null before parsing, so they
+        # showed this prompt; the scan left NaN-stamped files in review.
+        for name, scan in (("plain.png", None), ("stamped.png", "nan")):
+            with self.subTest(name=name):
+                g = self._hires_fix_graph("cat on a sofa")
+                if scan:
+                    status = self._nan_scanned(name, g)
+                else:
+                    status = self._scanned(name, g)
+                self.assertIn(status, ("added", "linked"))
+                self.assertEqual(self._texts(), ["cat on a sofa"])
+
+    def test_untraced_single_sampler_links_the_legacy_prompt_with_nan(self):
+        g = self._hires_fix_graph("cat on a sofa")
+        for node_id in ("7", "8", "9", "14", "15"):
+            del g[node_id]
+        g["16"]["inputs"]["images"] = ["6", 0]
+        self.assertEqual(self._nan_scanned("single.png", g), "added")
+        self.assertEqual(self._texts(), ["cat on a sofa"])
+
+    def test_nan_inside_prompt_text_is_kept(self):
+        # Only the NaN number reads as null; legacy's regex also rewrote text.
+        g = self._hires_fix_graph("NaN cat, value: NaN")
+        self.assertEqual(self._nan_scanned("text.png", g), "added")
+        self.assertEqual(self._texts(), ["NaN cat, value: NaN"])
+
+    def test_infinity_is_still_unreadable_like_the_legacy_pages(self):
+        text = json.dumps(self._hires_fix_graph("cat")).replace(
+            '"seed": 7', '"seed": Infinity'
+        )
+        self.assertIn(self._scanned_text("inf.png", text), ("ambiguous", "unresolved"))
+        self.assertEqual(self._texts(), [])
+
     def test_raw_png_workflow_is_stored_as_object_and_repeat_is_unchanged(self):
         path = self.output_dir / "workflow.png"
         workflow = {"nodes": [{"id": 1, "type": "PromptManagerPair"}]}

utils/generation_params.py

@@ -28,6 +28,7 @@ __all__ = [
     "extract_generation_params",
     "extract_from_records",
     "parse_js_json",
+    "parse_page_json",
 ]
 
 PLACEHOLDERS = MappingProxyType(
@@ -953,6 +954,30 @@ def parse_js_json(text):
         return None
 
 
+def _nan_as_null(name):
+    """The legacy pages' NaN cleaning; Infinity still throws in ``JSON.parse``."""
+    if name == "NaN":
+        return None
+    raise ValueError(f"{name} is not JSON")
+
+
+def parse_page_json(text):
+    """A metadata record as the legacy gallery and admin pages parsed it.
+
+    They replace ``NaN`` with ``null`` and then ``JSON.parse`` (ComfyUI writes
+    ``"is_changed": [NaN]`` for nodes that always re-run, PromptManager among
+    them). Only the NaN number reads as null here: the pages' regex also
+    rewrote the word inside strings, which changed prompt text. None where the
+    parse throws.
+    """
+    if not isinstance(text, str):
+        return None
+    try:
+        return json.loads(text, parse_constant=_nan_as_null, parse_int=_double_integer)
+    except (ValueError, RecursionError):
+        return None
+
+
 def extract_from_records(prompt_text, workflow_text):
     """``extract_generation_params`` for raw ``prompt``/``workflow`` record text.