fix(ingestion): address the #344 review (history re-check, keys, repair)

- Images without an embedded API graph: the scan now asks recorded
  history before skipping a file with no prompt metadata, and the monitor
  gives a keyless unlinked image one history-only re-check when its run's
  outputs are recorded (a seal notification from the outputs write; at
  most 1000 waiting images, each for an hour).
- Run keys: an image is unkeyable when any positive node has no key, so a
  two-pass graph is never linked through the other pass; links follow
  ComfyUI's is_link exactly.
- A failed save marks the node's run key like an empty editor, so an
  older entry of that key can't stand in for this run.
- A placeholder is the latest entry: the image stays unlinked without
  falling through to history, the same rule as a popped placeholder.
- An index on execution_run_outputs(filename, subfolder, root_key,
  path_style) for the history lookup, added by migration and accepted by
  the output schema check (any other index is still refused).
- Repair tool: unlink removes only the link record and only while the
  image is still linked to the prompt it was judged on; every run prints
  what --apply does.
- Real-host case proves the re-run's PromptManager node was cached.

tests/comfyui/image-attribution.spec.mjs

@@ -37,7 +37,7 @@ function graph({ kind = 'PromptManagerPair', text = '', prepend = '', llm = '',
     return nodes;
 }
 
-async function saved(request, prompt) {
+async function saved(request, prompt, history = {}) {
     const id = randomUUID();
     const response = await request.post('/prompt', { data: { prompt, prompt_id: id } });
     expect(response.ok()).toBe(true);
@@ -48,6 +48,7 @@ async function saved(request, prompt) {
     }, { timeout: 15000 }).toBe(true);
     const [image] = item.outputs['70'].images;
     expect(image.type).toBe('output');
+    history.item = item;
     return image.filename;
 }
 
@@ -62,11 +63,19 @@ async function monitoring(execution, enabled) {
     expect((await response.json()).monitoring).toBe(enabled);
 }
 
+async function cacheMode(execution, mode) {
+    expect((await execution.request.post('/pm_execution_fixture/cache', { data: { mode } })).ok()).toBe(true);
+}
+
 test.beforeEach(async ({ execution }) => {
     expect((await execution.request.post('/pm_execution_fixture/metadata', { data: { disabled: false } })).ok()).toBe(true);
+    await cacheMode(execution, 'CLASSIC');  // keeps the re-run's PromptManager node cached
     await monitoring(execution, true);
 });
-test.afterEach(async ({ execution }) => { await monitoring(execution, false); });
+test.afterEach(async ({ execution }) => {
+    await monitoring(execution, false);
+    await cacheMode(execution, 'RAM_PRESSURE');
+});
 
 test('an output whose positive is only linked prepend text stays unlinked, never given an earlier prompt', async ({ execution }, testInfo) => {
     const tag = `${testInfo.project.name} ${randomUUID()}`;
@@ -89,9 +98,13 @@ test('an output whose positive is only linked prepend text stays unlinked, never
     const state = await readLinks(execution.databasePath, [orphan], [prepended]);
     expect(state).toEqual({ links: { [orphan]: null }, stored: { [prepended]: false } });
 
-    // Only the sampler seed changes: the PromptManager node may be served from the
-    // cache without running, and its unchanged upstream still proves the prompt.
+    // Only the sampler seed changes: the PromptManager node is served from the cache
+    // without running (the host says so), so no queue entry exists for this run and
+    // only the snapshot's run match can link it.
+    const rerunHistory = {};
     const rerun = await saved(execution.request, { ...llmGraph, '40': { ...llmGraph['40'],
-        inputs: { ...llmGraph['40'].inputs, seed: 2 } } });
+        inputs: { ...llmGraph['40'].inputs, seed: 2 } } }, rerunHistory);
+    const cached = rerunHistory.item.status.messages.find(([kind]) => kind === 'execution_cached');
+    expect(cached?.[1].nodes).toEqual(expect.arrayContaining(['11', '20']));
     await linkedTo(execution, rerun, llmText);
 });

tests/test_prompt_attribution.py

@@ -510,5 +510,119 @@ class TestRunKeys(unittest.TestCase):
         self.assertIsNone(prompt_node_key(chain, "0"))
 
 
+class TestReviewFindings(MonitorCase, NodeCase):
+    """Final-review findings M1-M4 (#344)."""
+
+    def test_m1_a_positive_node_without_a_key_makes_the_image_unkeyable(self):
+        graph = llm_text_graph()
+        graph["30"] = {
+            "class_type": "PromptManager",
+            "inputs": {"text": ["missing", 0], "clip": ["4", 1]},
+        }
+        graph["31"] = {
+            "class_type": "KSampler",
+            "inputs": {**graph["3"]["inputs"], "positive": ["30", 0]},
+        }
+        self.assertIsNone(prompt_node_key(graph, "30"))
+        self.assertEqual(image_run_keys({"prompt": graph}), {})
+        prompt_id = self.save(LLM_TEXT)
+        self.tracker.set_current_prompt(
+            LLM_TEXT,
+            {"prompt_id": prompt_id},
+            run_key=prompt_node_key(graph, "18:35"),
+            run_id="run-1",
+        )
+        self.assertIsNone(self.process({"prompt": graph}))
+
+    def test_m2_links_are_exactly_comfyui_is_link(self):
+        from utils.run_keys import _links
+
+        inputs = {
+            "string_slot_int": ["7", 0],
+            "string_slot_float": ["7", 0.0],
+            "string_slot_bool": ["7", True],  # bool is an int to ComfyUI too
+            "int_source": [7, 0],
+            "tuple": ("7", 0),
+            "three": ["7", 0, 1],
+            "slot_text": ["7", "0"],
+        }
+        self.assertEqual(sorted(_links(inputs)), ["7", "7", "7"])
+        widget = issue_graph()
+        widget["18:17"]["inputs"]["size"] = [512, 768]  # a value, not a link
+        self.assertIsNotNone(prompt_node_key(widget, "18:35"))
+        floated = issue_graph()
+        floated["18:35"]["inputs"]["prepend_text"] = ["18:13", 0.0]
+        changed = copy.deepcopy(floated)
+        changed["18:17"]["inputs"]["value"] = "another instruction"
+        self.assertNotEqual(
+            prompt_node_key(floated, "18:35"), prompt_node_key(changed, "18:35")
+        )
+
+    def _failing_save(self, node):
+        def explode(*args, **kwargs):
+            raise RuntimeError("database is locked")
+
+        node.db = mock.Mock(wraps=self.db)
+        for name in ("save_prompt", "save_prompt_pair", "get_prompt_by_hash"):
+            setattr(node.db, name, mock.Mock(side_effect=explode))
+        return node
+
+    def test_m3_a_failed_save_takes_over_its_run_key(self):
+        graph = llm_text_graph()
+        key = prompt_node_key(graph, "18:35")
+        older = self.save("previous run text")
+        self.tracker.set_current_prompt(
+            "previous run text", {"prompt_id": older}, run_key=key, run_id="run-1"
+        )
+        node = self._failing_save(self.node(PromptManager))
+        with mock.patch("prompt_manager_base.executing_run_id", return_value="run-2"):
+            node.encode_prompt(
+                self.clip(), "new run text", prompt=graph, unique_id="18:35"
+            )
+        latest, run_id, _count = self.tracker.keyed_snapshot()[key]
+        self.assertTrue(latest.get("placeholder"))
+        self.assertEqual(run_id, "run-2")
+        self.assertIsNone(self.process({"prompt": graph}))
+
+    def test_m3_a_failed_pair_save_takes_over_its_run_key(self):
+        graph = issue_graph(text="typed")
+        key = prompt_node_key(graph, "18:35")
+        node = self._failing_save(self.node(PromptManagerPair))
+        with self.assertLogs("test.prompt_attribution.node", "WARNING"):
+            node.encode_prompt(self.clip(), "typed", prompt=graph, unique_id="18:35")
+        self.assertTrue(self.tracker.keyed_snapshot()[key][0].get("placeholder"))
+
+    def test_m3_a_failed_text_node_save_takes_over_its_run_key(self):
+        graph = issue_graph()
+        graph["18:35"] = {
+            "class_type": "PromptManagerText",
+            "inputs": {"text": "typed"},
+        }
+        graph["40"] = {
+            "class_type": "CLIPTextEncode",
+            "inputs": {"text": ["18:35", 0], "clip": ["4", 1]},
+        }
+        graph["3"]["inputs"]["positive"] = ["40", 0]
+        node = self._failing_save(self.node(PromptManagerText))
+        with mock.patch.object(
+            node, "_track_prompt_execution", side_effect=RuntimeError("boom")
+        ):
+            node.process_text("typed", prompt=graph, unique_id="18:35")
+        key = prompt_node_key(graph, "18:35")
+        self.assertTrue(self.tracker.keyed_snapshot()[key][0].get("placeholder"))
+
+    def test_m4_a_placeholder_never_falls_through_to_history(self):
+        graph = llm_text_graph()
+        key = prompt_node_key(graph, "18:35")
+        self.tracker.mark_empty(key, "run-1", push_to_queue=False)
+        with mock.patch.object(
+            self.handler, "_history_prompt", return_value=99
+        ) as history:
+            self.assertIsNone(self.process({"prompt": graph}))  # snapshot form
+            self.tracker.mark_empty(key, "run-2", push_to_queue=True)
+            self.assertIsNone(self.process({"prompt": graph}))  # popped form
+        history.assert_not_called()
+
+
 if __name__ == "__main__":
     unittest.main()

tests/test_review_image_links_tool.py

@@ -17,6 +17,7 @@ except ImportError:  # discovered with tests/ as the top-level directory
     from open_handles import assert_closed
 
 from database.operations import PromptDatabase
+from tests.test_api_images import ImageAPITestCase
 from tests.test_prompt_attribution import issue_graph, pair_evidence
 from tools import review_image_links as tool
 from utils.hashing import generate_prompt_hash
@@ -31,7 +32,7 @@ def png(path, prompt=None, evidence=None):
     Image.new("RGB", (4, 4)).save(path, pnginfo=info)
 
 
-class ReviewToolTests(unittest.TestCase):
+class ToolFixture(unittest.TestCase):
     def setUp(self):
         self.tmp = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, self.tmp, True)
@@ -81,12 +82,14 @@ class ReviewToolTests(unittest.TestCase):
         finally:
             db.close_all()
 
+
+class ReviewToolTests(ToolFixture):
     def test_dry_run_judges_and_changes_nothing(self):
         before = self.links()
         ids = f"{self.guessed[0]}-{self.gone}"
         code, out = self.run_tool("--ids", ids, "--json")
         self.assertEqual(code, 0)
-        verdicts = {v["image_id"]: v for v in json.loads(out.split("Dry run")[0])}
+        verdicts = {v["image_id"]: v for v in json.loads(out.split("--apply, for")[0])}
         for image_id in self.guessed:
             self.assertEqual(verdicts[image_id]["verdict"], "unlink")
             self.assertEqual(verdicts[image_id]["sources"], {})
@@ -114,7 +117,7 @@ class ReviewToolTests(unittest.TestCase):
             "--ids", named, "--prompt", str(self.coffee), "--apply"
         )
         self.assertEqual(code, 0)
-        self.assertIn("Applied: 1 relinked, 2 unlinked", out)
+        self.assertIn("Applied: 1 relinked, 2 unlinked, 0 skipped", out)
         links = self.links()
         self.assertNotIn(self.guessed[0], links)
         self.assertNotIn(self.guessed[1], links)
@@ -171,5 +174,82 @@ class ReviewToolTests(unittest.TestCase):
         self.assertEqual(verdict["verdict"], "conflict")
 
 
+class ApplyGuardTests(ToolFixture):
+    """M6: unlink clears only the link, and only the link it was judged on."""
+
+    def test_unlink_skips_a_row_linked_elsewhere_since_the_review(self):
+        db = PromptDatabase(self.db_path)
+        try:
+            verdicts = tool.review(db, [self.guessed[0]], None, self.output)
+            self.assertEqual(verdicts[0]["verdict"], "unlink")
+            db.model.get_connection().execute(
+                "UPDATE generated_images SET prompt_id=? WHERE id=?",
+                (self.typed, self.guessed[0]),
+            )
+            db.model.get_connection().commit()
+            result = tool.apply(db, verdicts, [self.guessed[0]])
+        finally:
+            db.close_all()
+        self.assertEqual(result["unlinked"], 0)
+        self.assertEqual(self.links()[self.guessed[0]], self.typed)
+
+    def test_unlink_keeps_the_file_and_its_favorite(self):
+        path = os.path.join(self.output, "moss_00009_.png")
+        db = PromptDatabase(self.db_path)
+        try:
+            db.set_image_favorite("a" * 64, "moss_00009_.png")
+        finally:
+            db.close_all()
+        self.run_tool("--ids", str(self.guessed[0]), "--apply")
+        self.assertTrue(os.path.isfile(path))
+        db = PromptDatabase(self.db_path)
+        try:
+            self.assertEqual(db.get_root_favorites("a" * 64), {"moss_00009_.png"})
+            summary = db.get_image_link_summaries([path]).get(path) or {}
+        finally:
+            db.close_all()
+        self.assertNotEqual(summary.get("status"), "linked")
+
+    def test_dry_run_states_what_apply_does(self):
+        _code, out = self.run_tool("--ids", str(self.guessed[0]))
+        self.assertIn("removes only the image's link record", out)
+        self.assertIn("file stays on disk and in the gallery as unlinked", out)
+
+
+class GalleryAfterUnlinkTests(ImageAPITestCase):
+    """M6: after --apply the file is still listed, as an unlinked image."""
+
+    async def test_unlinked_file_stays_in_the_workspace_gallery(self):
+        db = self.api.db
+        prompt = db.save_prompt(
+            text="a woman holding a coffee cup",
+            prompt_hash=generate_prompt_hash("a woman holding a coffee cup"),
+        )
+        path = self.output_dir / "moss_00009_.png"
+        png(str(path), issue_graph(), pair_evidence())
+        image_id = db.link_image_to_prompt(prompt, str(path), metadata={})
+        out = io.StringIO()
+        tool.main(
+            [
+                db.model.db_path,
+                "--output-dir",
+                str(self.output_dir),
+                "--ids",
+                str(image_id),
+                "--apply",
+            ],
+            out=out,
+        )
+        self.assertIn("Applied: 0 relinked, 1 unlinked", out.getvalue())
+        response = await self.client.get(
+            "/prompt_manager/images/output", params={"mode": "workspace"}
+        )
+        self.assertEqual(response.status, 200)
+        images = {i["filename"]: i for i in (await response.json())["images"]}
+        self.assertIn("moss_00009_.png", images)
+        link = images["moss_00009_.png"].get("linked") or {}
+        self.assertNotEqual(link.get("status"), "linked")
+
+
 if __name__ == "__main__":
     unittest.main()

tests/test_run_attribution.py

@@ -198,6 +198,27 @@ class TestScanUsesHistory(RecordedRuns, AdminAPITestCase):
         self.assertEqual(self.api._ingest_scanned_file(path, meta), "linked")
         self.assertEqual(self.linked_prompt(path), prompt_id)
 
+    def test_a_metadata_less_file_links_through_its_recorded_run(self):
+        """I1: --disable-metadata outputs carry no prompt; history still proves one."""
+        db = self.api.db
+        prompt_id = db.save_prompt(
+            text=LLM_TEXT, prompt_hash=generate_prompt_hash(LLM_TEXT)
+        )
+        path = self.write("bare_00001_.png")
+        self.record_run(db, str(self.output_dir), [("", path.name)], [LLM_TEXT])
+        self.assertEqual(self.api._ingest_scanned_file(path, {}), "linked")
+        self.assertEqual(self.linked_prompt(path), prompt_id)
+        self.assertEqual(self.api._ingest_scanned_file(path, None), "unchanged")
+        detailed = self.api._ingest_scanned_file(path, {}, detailed=True)
+        self.assertEqual(detailed["status"], "unchanged")
+        self.assertTrue(detailed["title"])
+
+    def test_a_metadata_less_file_without_a_recorded_run_is_still_skipped(self):
+        path = self.write("bare_00002_.png")
+        self.assertIsNone(self.api._ingest_scanned_file(path, {}))
+        self.assertIsNone(self.api._ingest_scanned_file(path, {"workflow": {}}))
+        self.assertIsNone(self.linked_prompt(path))
+
     def test_the_issue_file_stays_unlinked_even_with_its_pair_record(self):
         """Resolved with an empty stored positive: before #342 the scan raised."""
         db = self.api.db
@@ -233,10 +254,6 @@ class TestScanUsesHistory(RecordedRuns, AdminAPITestCase):
         self.assertEqual(db.list_prompt_pairs()["pairs"], [])
 
 
-if __name__ == "__main__":
-    unittest.main()
-
-
 class TestHostHelpers(unittest.TestCase):
     """The host lookups degrade to "no proof", never to an error."""
 
@@ -301,3 +318,176 @@ class TestHostHelpers(unittest.TestCase):
                 sys.modules.pop("comfy_execution.utils", None)
             else:
                 sys.modules["comfy_execution.utils"] = original
+
+
+class TestMonitorHistoryRecheck(RecordedRuns, unittest.TestCase):
+    """I1: a keyless image gets one history-only re-check when its run is sealed."""
+
+    def setUp(self):
+        history_tests.HistoryDatabaseTests.setUp(self)
+        self.db = history_tests.HistoryDatabaseTests.database(self)
+        self.output = str(Path(self.temp.name) / "output")
+        os.makedirs(self.output)
+        self.handler = ImageGenerationHandler(self.db, make_tracker(self.db))
+        self.handler.processing_delay = 0
+        self.handler.settle_interval = 0.01
+        self.addCleanup(self.handler.shutdown)
+        self.linked = []
+        self.handler.link_image_to_prompt = lambda path, prompt, meta: (
+            self.linked.append((os.path.basename(path), prompt["id"]))
+        )
+        patches = (
+            mock.patch(
+                "utils.image_monitor.host_output_directory", return_value=self.output
+            ),
+            mock.patch.object(
+                self.handler.metadata_extractor, "extract_metadata", return_value=None
+            ),
+        )
+        for patch in patches:
+            patch.start()
+            self.addCleanup(patch.stop)
+
+    close_databases = history_tests.HistoryDatabaseTests.close_databases
+
+    def image(self, name):
+        path = os.path.join(self.output, name)
+        Path(path).write_bytes(b"png")
+        return path
+
+    def save(self, text):
+        return self.db.save_prompt(text=text, prompt_hash=generate_prompt_hash(text))
+
+    def wait_for(self, predicate, timeout=5.0):
+        deadline = time.monotonic() + timeout
+        while time.monotonic() < deadline:
+            if predicate():
+                return True
+            time.sleep(0.01)
+        return predicate()
+
+    def test_the_run_ending_after_processing_links_the_image_once(self):
+        prompt_id = self.save(LLM_TEXT)
+        path = self.image("late_00001_.png")
+        self.handler.process_new_image(path)  # the run is still executing
+        self.assertEqual(self.linked, [])
+        self.assertIn(os.path.normcase(path), self.handler.awaiting_history())
+        self.record_run(self.db, self.output, [("", "late_00001_.png")], [LLM_TEXT])
+        self.assertTrue(self.wait_for(lambda: self.linked))
+        self.assertTrue(self.wait_for(lambda: self.handler.pending_count() == 0))
+        self.assertEqual(self.linked, [("late_00001_.png", prompt_id)])
+        self.assertEqual(self.handler.awaiting_history(), [])
+        # Another run reporting the same name later is not a second re-check
+        self.record_run(
+            self.db, self.output, [("", "late_00001_.png")], [LLM_TEXT], item=1
+        )
+        self.assertTrue(self.wait_for(lambda: self.handler.pending_count() == 0))
+        self.assertEqual(len(self.linked), 1)
+
+    def test_a_run_naming_nothing_stored_leaves_the_image_unlinked(self):
+        path = self.image("empty_00001_.png")
+        self.handler.process_new_image(path)
+        self.record_run(self.db, self.output, [("", "empty_00001_.png")], [""])
+        self.assertTrue(self.wait_for(lambda: not self.handler.awaiting_history()))
+        self.assertTrue(self.wait_for(lambda: self.handler.pending_count() == 0))
+        self.assertEqual(self.linked, [])
+
+    def test_history_found_at_once_leaves_nothing_awaiting(self):
+        prompt_id = self.save(LLM_TEXT)
+        self.record_run(self.db, self.output, [("", "done_00001_.png")], [LLM_TEXT])
+        self.handler.process_new_image(self.image("done_00001_.png"))
+        self.assertEqual(self.linked, [("done_00001_.png", prompt_id)])
+        self.assertEqual(self.handler.awaiting_history(), [])
+
+    def test_keyed_images_do_not_wait_for_history(self):
+        with mock.patch.object(
+            self.handler.metadata_extractor,
+            "extract_metadata",
+            return_value={"prompt": llm_text_graph()},
+        ):
+            self.handler.process_new_image(self.image("keyed_00001_.png"))
+        self.assertEqual(self.handler.awaiting_history(), [])
+
+    def test_the_waiting_set_is_bounded_and_expires(self):
+        from utils import image_monitor
+
+        with mock.patch.object(image_monitor, "MAX_AWAITING_HISTORY", 2):
+            for n in range(3):
+                self.handler.process_new_image(self.image(f"b_{n}.png"))
+        self.assertEqual(
+            [os.path.basename(p) for p in self.handler.awaiting_history()],
+            ["b_1.png", "b_2.png"],
+        )
+        later = time.monotonic() + image_monitor.AWAIT_HISTORY_SECONDS + 1
+        with mock.patch.object(image_monitor.time, "monotonic", return_value=later):
+            self.assertEqual(self.handler.awaiting_history(), [])
+
+    def test_shutdown_forgets_waiting_images_and_a_dead_handler_is_dropped(self):
+        import gc
+
+        from database import run_attribution
+
+        self.handler.process_new_image(self.image("gone_00001_.png"))
+        self.handler.shutdown()
+        self.assertEqual(self.handler.awaiting_history(), [])
+        other = ImageGenerationHandler(self.db, make_tracker(self.db))
+        with mock.patch.object(
+            other.metadata_extractor, "extract_metadata", return_value=None
+        ):
+            other.process_new_image(self.image("gone_00002_.png"))
+        del other
+        gc.collect()
+        run_attribution.notify_outputs_sealed("run", [])  # no dead callback raises
+
+
+class TestOutputLookupIndex(unittest.TestCase):
+    """M5: the history lookup by file name uses an index, added by migration."""
+
+    setUp = history_tests.HistoryDatabaseTests.setUp
+    close_databases = history_tests.HistoryDatabaseTests.close_databases
+    database = history_tests.HistoryDatabaseTests.database
+
+    def test_index_exists_is_used_validated_and_reinstalled(self):
+        from database.execution_history_schema import (
+            history_schema_error,
+            migrate_execution_history,
+        )
+        from database.run_attribution import _RUNS_FOR_OUTPUT
+
+        db = self.database()
+        conn = db.model.get_connection()
+        name = "idx_history_read_execution_run_outputs"
+        columns = [r[2] for r in conn.execute(f"PRAGMA index_info({name})")]
+        self.assertEqual(columns, ["filename", "subfolder", "root_key", "path_style"])
+        plan = " ".join(
+            str(row[3])
+            for row in conn.execute(
+                "EXPLAIN QUERY PLAN " + _RUNS_FOR_OUTPUT, ("posix", "k", "", "f.png")
+            )
+        )
+        self.assertIn(name, plan)
+        self.assertIsNone(history_schema_error(conn, allow_missing=False))
+        conn.execute(f"DROP INDEX {name}")
+        self.assertIsNone(history_schema_error(conn, allow_missing=False))
+        migrate_execution_history(conn)
+        migrate_execution_history(conn)
+        self.assertTrue(
+            conn.execute("SELECT 1 FROM sqlite_master WHERE name=?", (name,)).fetchone()
+        )
+
+    def test_any_other_index_on_the_outputs_table_is_still_refused(self):
+        from database.execution_output_schema import output_schema_error
+
+        db = self.database()
+        conn = db.model.get_connection()
+        conn.execute("CREATE INDEX idx_rogue ON execution_run_outputs(filename)")
+        self.assertIn("unexpected", output_schema_error(conn))
+        conn.execute("DROP INDEX idx_rogue")
+        name = "idx_history_read_execution_run_outputs"
+        conn.execute(f"DROP INDEX {name}")
+        conn.execute(f"CREATE INDEX {name} ON execution_run_outputs(filename)")
+        self.assertIn("unexpected", output_schema_error(conn))
+
+
+if __name__ == "__main__":
+    unittest.main()