Merge remote-tracking branch 'origin/main' into feat/249-system-appearance

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.