diff --git a/tests/js/comfy-metadata.test.js b/tests/js/comfy-metadata.test.js index 7e45d2e..bfd96f2 100644 --- a/tests/js/comfy-metadata.test.js +++ b/tests/js/comfy-metadata.test.js @@ -224,6 +224,22 @@ test("facts use resolution.sampler_id for detailer-stage nodes", () => { assert.ok(Object.values(factsFor(wide, { resolution: { state: "ambiguous", sampler_id: "d" } })).every(fact => fact.status === "unresolved"), "the stage walk is bounded by MAX_VISITS"); }); +test("a shared pair resolved across sampler branches merges facts per field (#249)", () => { + // The server resolves identical branches with no sampler named: settings the + // branches agree on are known, those they differ on (a LoRA per epoch) are ambiguous. + const shared = { resolution: { state: "resolved", sampler_id: null, reason: "Every sampler branch uses the same prompt pair" } }; + const grid = factGraph(); + grid.lora = { class_type: "CheckpointLoaderSimple", inputs: { ckpt_name: "epoch_10.safetensors" } }; + grid.other = structuredClone(grid.s); grid.other.inputs.model = ["lora", 0]; + assert.deepEqual(statusValues(factsFor(grid, shared)), { + seed: ["known", "9007199254740993"], steps: ["known", "28"], cfg: ["known", "6.5000000000000000001"], + sampler: ["known", "dpmpp_2m"], scheduler: ["known", "karras"], model: ["ambiguous", null], + }); + assert.ok(Object.values(factsFor(grid, shared)).every(fact => fact.sampler_id === null)); + // One listed sampler keeps reading that sampler alone. + assert.equal(factsFor(factGraph(), shared).model.value, "model.safetensors"); +}); + test("ambiguous results without a sampler_id merge facts across every listed stage", () => { const ambiguous = { resolution: { state: "ambiguous", sampler_id: null, reason: "Multiple sampler branches" } }; // f01b's shape: an upstream sampler and a detailer whose settings partly differ. diff --git a/tests/test_api_prompt_inspector.py b/tests/test_api_prompt_inspector.py index ae9a082..c4702a9 100644 --- a/tests/test_api_prompt_inspector.py +++ b/tests/test_api_prompt_inspector.py @@ -128,7 +128,12 @@ class TestPromptInspector(ImageAPITestCase): for expected in ("ambiguous", "unresolved"): source = graph() if expected == "ambiguous": - source["4"] = source["3"] + # Branches with different prompts (#249: identical ones resolve). + source["5"] = {"class_type": "CLIPTextEncode", "inputs": {"text": "x"}} + source["4"] = { + "class_type": "KSampler", + "inputs": {"positive": ["5", 0], "negative": ["2", 0]}, + } else: source["1"]["class_type"] = "UnknownEncoder" png( diff --git a/tests/test_identical_sampler_branches.py b/tests/test_identical_sampler_branches.py new file mode 100644 index 0000000..ca8ff01 --- /dev/null +++ b/tests/test_identical_sampler_branches.py @@ -0,0 +1,248 @@ +"""Several sampler branches that share one prompt pair resolve to that pair (#249). + +The shape is a LoRA epoch-compare grid: one sampler per LoRA epoch, every +sampler fed by the same positive ``CLIPTextEncode`` and the same +``ConditioningZeroOut`` negative, every decoded image labelled, batched and +concatenated into one grid that a single ``SaveImage`` writes. Which branch +made which tile does not matter to the prompt: every branch used the same pair. +Branches that differ stay ambiguous. +""" + +import unittest + +from utils import prompt_pair_resolver as resolver +from utils.prompt_inspector import inspect_prompt_evidence +from utils.prompt_pair_resolver import resolve_prompt_pair + +POSITIVE = "a woman holding a coffee cup, in a beanie, sitting by a window" +OTHER = "a man reading a newspaper on a park bench" +NEGATIVE = "blurry, low quality" +# Each profile with the negative it can read: inspection reads the zero-out. +PROFILES = { + "default": (resolver.DEFAULT_PROFILE, NEGATIVE), + "inspection": (resolver.INSPECTION_PROFILE, None), +} + + +def _grid(epochs: int = 4, positives: dict = None, negative: str = None) -> dict: + """A compare grid; ``positives`` maps an epoch to its own positive text. + + The negative is a ``ConditioningZeroOut`` of the positive, or with + ``negative`` an encoded text (the default profile cannot read a zero-out). + """ + graph = { + "1": { + "class_type": "UNETLoader", + "inputs": {"unet_name": "base.safetensors", "weight_dtype": "default"}, + }, + "2": { + "class_type": "CLIPLoader", + "inputs": {"clip_name": "te.safetensors", "type": "stable_diffusion"}, + }, + "3": {"class_type": "VAELoader", "inputs": {"vae_name": "vae.safetensors"}}, + "4": { + "class_type": "CLIPTextEncode", + "inputs": {"text": POSITIVE, "clip": ["2", 0]}, + }, + "5": ( + {"class_type": "ConditioningZeroOut", "inputs": {"conditioning": ["4", 0]}} + if negative is None + else { + "class_type": "CLIPTextEncode", + "inputs": {"text": negative, "clip": ["2", 0]}, + } + ), + "6": { + "class_type": "EmptyLatentImage", + "inputs": {"width": 512, "height": 512, "batch_size": 1}, + }, + } + batch = {} + for epoch in range(epochs): + base = 10 + epoch * 10 + lora, sampler, decode, label = (str(base + offset) for offset in range(4)) + positive = "4" + if positives and epoch in positives: + positive = str(base + 5) + graph[positive] = { + "class_type": "CLIPTextEncode", + "inputs": {"text": positives[epoch], "clip": ["2", 0]}, + } + graph[lora] = { + "class_type": "LoraLoaderModelOnly", + "inputs": { + "lora_name": f"subject_epoch_{epoch:02d}.safetensors", + "strength_model": 1.0, + "model": ["1", 0], + }, + } + graph[sampler] = { + "class_type": "KSampler", + "inputs": { + "seed": 42, + "steps": 8, + "cfg": 1.0, + "sampler_name": "euler", + "scheduler": "simple", + "denoise": 1.0, + "model": [lora, 0], + "positive": [positive, 0], + "negative": ["5", 0], + "latent_image": ["6", 0], + }, + } + graph[decode] = { + "class_type": "VAEDecode", + "inputs": {"samples": [sampler, 0], "vae": ["3", 0]}, + } + graph[label] = { + "class_type": "AddLabel", + "inputs": {"text": f"epoch {epoch}", "image": [decode, 0]}, + } + batch[f"images.image{epoch}"] = [label, 0] + graph["900"] = {"class_type": "BatchImagesNode", "inputs": batch} + graph["901"] = { + "class_type": "ImageConcatFromBatch", + "inputs": {"num_columns": 2, "images": ["900", 0]}, + } + graph["902"] = { + "class_type": "SaveImage", + "inputs": {"filename_prefix": "compare/grid", "images": ["901", 0]}, + } + return graph + + +def _texts(result: dict) -> tuple: + return tuple( + result[role]["text"] if isinstance(result[role], dict) else None + for role in ("positive", "negative") + ) + + +class TestIdenticalSamplerBranches(unittest.TestCase): + def test_grid_of_identical_branches_resolves_to_the_shared_pair(self): + for name, (profile, negative) in PROFILES.items(): + with self.subTest(profile=name): + result = resolve_prompt_pair( + {"prompt": _grid(negative=negative)}, profile=profile + ) + self.assertEqual(result["state"], "resolved") + self.assertEqual(_texts(result), (POSITIVE, negative or "")) + # No one branch is named: every branch made this pair. + self.assertIsNone(result["sampler_id"]) + self.assertEqual(result["reason"], resolver.IDENTICAL_BRANCHES_REASON) + # Every branch stays listed as the evidence for the shared pair. + self.assertEqual( + [item["sampler_id"] for item in result["candidates"]], + ["11", "21", "31", "41"], + ) + self.assertEqual( + {_texts(item) for item in result["candidates"]}, + {(POSITIVE, negative or "")}, + ) + self.assertEqual(result["reason_code"], None) + if negative is None: + self.assertEqual( + result["negative"]["provenance"]["text_source"], + "zeroed-conditioning", + ) + + def test_default_profile_keeps_an_unreadable_zero_out_grid_ambiguous(self): + # Ingestion cannot read ConditioningZeroOut: every branch is unresolved, + # so the grid stays ambiguous and the scan keeps its legacy fallback. + result = resolve_prompt_pair({"prompt": _grid()}) + self.assertEqual(result["state"], "ambiguous") + self.assertEqual(result["reason"], "Multiple sampler branches") + self.assertEqual( + {item["state"] for item in result["candidates"]}, {"unresolved"} + ) + + def test_single_branch_grid_still_names_its_sampler(self): + for name, (profile, negative) in PROFILES.items(): + with self.subTest(profile=name): + result = resolve_prompt_pair( + {"prompt": _grid(epochs=1, negative=negative)}, profile=profile + ) + self.assertEqual(result["state"], "resolved") + self.assertEqual(result["sampler_id"], "11") + self.assertEqual(result["reason"], "") + + def test_branches_with_different_prompts_stay_ambiguous(self): + for name, (profile, negative) in PROFILES.items(): + with self.subTest(profile=name): + source = _grid(positives={2: OTHER}, negative=negative) + result = resolve_prompt_pair({"prompt": source}, profile=profile) + self.assertEqual(result["state"], "ambiguous") + self.assertEqual(result["reason"], "Multiple sampler branches") + self.assertIsNone(result["positive"]) + self.assertEqual( + sorted(_texts(item)[0] for item in result["candidates"]), + sorted([POSITIVE] * 3 + [OTHER]), + ) + + def test_a_branch_that_cannot_be_read_keeps_the_grid_ambiguous(self): + for name, (profile, negative) in PROFILES.items(): + with self.subTest(profile=name): + source = _grid(negative=negative) + source["31"]["inputs"]["negative"] = ["800", 0] + source["800"] = {"class_type": "MysteryConditioning", "inputs": {}} + result = resolve_prompt_pair({"prompt": source}, profile=profile) + self.assertEqual(result["state"], "ambiguous") + self.assertEqual(result["reason"], "Multiple sampler branches") + + def test_identical_unreadable_branches_are_not_collapsed(self): + source = _grid(epochs=2) + source["5"] = {"class_type": "MysteryConditioning", "inputs": {}} + for name, (profile, _negative) in PROFILES.items(): + with self.subTest(profile=name): + result = resolve_prompt_pair({"prompt": source}, profile=profile) + self.assertEqual(result["state"], "ambiguous") + + def test_same_text_with_a_different_recipe_stays_ambiguous(self): + adapters = { + ("RecipeText", 0): resolver.OutputAdapter( + "text", "conditioning", compose=True + ) + } + source = _grid(epochs=2, negative=NEGATIVE) + source["21"]["inputs"]["positive"] = ["700", 0] + source["700"] = { + "class_type": "RecipeText", + "inputs": {"prepend_text": "a woman", "text": "holding a coffee cup"}, + } + alone = resolve_prompt_pair( + {"prompt": source}, output_node_id="21", adapters=adapters + ) + # Branch "10" reads the same effective text through a plain encoder. + source["4"]["inputs"]["text"] = alone["positive"]["text"] + result = resolve_prompt_pair({"prompt": source}, adapters=adapters) + self.assertEqual(result["state"], "ambiguous") + self.assertEqual( + {_texts(item) for item in result["candidates"]}, + {(alone["positive"]["text"], NEGATIVE)}, + ) + self.assertEqual( + {item["positive"]["storage_text"] for item in result["candidates"]}, + {alone["positive"]["text"], "holding a coffee cup"}, + ) + + def test_inspector_shows_the_positive_and_an_explicitly_empty_negative(self): + evidence = inspect_prompt_evidence({"prompt": _grid()}) + prompts = evidence["prompts"] + self.assertEqual(prompts["positive"]["status"], "resolved") + self.assertEqual(prompts["positive"]["text"], POSITIVE) + self.assertEqual(prompts["negative"]["status"], "resolved") + self.assertEqual(prompts["negative"]["text"], "") + self.assertEqual(prompts["negative"]["source"], "zeroed-conditioning") + self.assertEqual(evidence["resolution"]["state"], "resolved") + self.assertIsNone(evidence["resolution"]["sampler_id"]) + + def test_inspector_keeps_differing_grids_ambiguous(self): + evidence = inspect_prompt_evidence({"prompt": _grid(positives={0: OTHER})}) + self.assertEqual(evidence["resolution"]["state"], "ambiguous") + self.assertEqual(evidence["prompts"]["positive"]["status"], "ambiguous") + self.assertNotIn(POSITIVE, str(evidence["prompts"])) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_inspection_outcomes.py b/tests/test_inspection_outcomes.py index a3c3555..8e0aced 100644 --- a/tests/test_inspection_outcomes.py +++ b/tests/test_inspection_outcomes.py @@ -254,7 +254,9 @@ class TestOutcomeMessages(unittest.TestCase): def test_candidates_and_absence_keep_their_status_lines(self): graph = _runtime() graph["7"] = _encode("positive") - graph["5"] = _ksampler() + # A second sampler with its own prompt (#249: identical ones resolve). + graph["8"] = _encode("other positive") + graph["5"] = _ksampler("8") evidence = inspect_prompt_evidence({"prompt": graph}) self.assertEqual(evidence["resolution"]["state"], "ambiguous") self.assertEqual(_messages(evidence), {"positive": None, "negative": None}) diff --git a/tests/test_pair_image_linking.py b/tests/test_pair_image_linking.py index 6c87309..cba9f27 100644 --- a/tests/test_pair_image_linking.py +++ b/tests/test_pair_image_linking.py @@ -56,6 +56,14 @@ class TestPairImageLinking(unittest.TestCase): self.handler.link_image_to_prompt(self.path, {"id": self.pid}, meta) self.assertEqual(self.db.list_prompt_pairs()["total"], 0) meta = metadata() - meta["prompt"]["4"] = meta["prompt"]["3"] + # Two samplers with different positives (#249: identical ones resolve). + meta["prompt"]["5"] = { + "class_type": "CLIPTextEncode", + "inputs": {"text": "dog"}, + } + meta["prompt"]["4"] = { + "class_type": "KSampler", + "inputs": {"positive": ["5", 0], "negative": ["2", 0]}, + } self.handler.link_image_to_prompt(self.path + "2", {"id": self.pid}, meta) self.assertEqual(self.db.list_prompt_pairs()["total"], 0) diff --git a/tests/test_pair_scan.py b/tests/test_pair_scan.py index 945d19c..5a56373 100644 --- a/tests/test_pair_scan.py +++ b/tests/test_pair_scan.py @@ -44,10 +44,15 @@ class TestPairScan(AdminAPITestCase): return [p["text"] for p in self.api.db.get_recent_prompts()["prompts"]] def test_samplers_sharing_one_prompt_link_that_prompt(self): + # #249: every branch resolves to one exact pair, so the pair is ingested. g = sampled_graph() g["5"] = {**g["3"]} self.assertEqual(self._scanned("multi.png", g), "added") - self.assertEqual(self._texts(), ["cat"]) + self.assertEqual(sorted(self._texts()), ["blur", "cat"]) + pairs = self.api.db.list_prompt_pairs()["pairs"] + self.assertEqual( + [(p["positive_text"], p["negative_text"]) for p in pairs], [("cat", "blur")] + ) def test_samplers_with_different_prompts_link_the_first_sampler_prompt(self): # The prompt the inspector and the legacy panels show for this file. diff --git a/tests/test_prompt_pair_resolver.py b/tests/test_prompt_pair_resolver.py index cda53ad..f92bfe4 100644 --- a/tests/test_prompt_pair_resolver.py +++ b/tests/test_prompt_pair_resolver.py @@ -54,13 +54,30 @@ class TestPromptPairResolver(unittest.TestCase): self.assertEqual(result["positive"]["text"], "portrait") self.assertIsNone(result["negative"]) - def test_multiple_samplers_are_ambiguous_even_when_texts_match(self): + def test_multiple_samplers_with_matching_texts_resolve_to_the_shared_pair(self): source = graph() source["4"] = { "class_type": "KSamplerAdvanced", "inputs": source["3"]["inputs"], } result = resolve_prompt_pair(source) + self.assertEqual(result["state"], "resolved") + self.assertEqual( + (result["positive"]["text"], result["negative"]["text"]), + ("portrait", "blurry"), + ) + self.assertIsNone(result["sampler_id"]) + self.assertEqual(result["reason"], resolver.IDENTICAL_BRANCHES_REASON) + self.assertEqual(len(result["candidates"]), 2) + + def test_multiple_samplers_with_different_texts_are_ambiguous(self): + source = graph() + source["5"] = {"class_type": "CLIPTextEncode", "inputs": {"text": "other"}} + source["4"] = { + "class_type": "KSamplerAdvanced", + "inputs": {"positive": ["5", 0], "negative": ["2", 0]}, + } + result = resolve_prompt_pair(source) self.assertEqual(result["state"], "ambiguous") self.assertEqual(len(result["candidates"]), 2) self.assertIsNone(result["positive"]) @@ -2253,12 +2270,14 @@ class TestDetailerStages(unittest.TestCase): self.assertEqual(_sides(result), (parity.DETAILER_POS, parity.NEG)) self.assertEqual(result["sampler_id"], "50") - def test_raw_and_detailed_saves_stay_ambiguous_without_an_anchor(self): + def test_raw_and_detailed_saves_of_one_prompt_share_it_without_an_anchor(self): source = _detailed(same_prompt=True) source["14"] = {"class_type": "SaveImage", "inputs": {"images": ["8", 0]}} result = _inspect(source) - self.assertEqual(result["state"], "ambiguous") - self.assertEqual(result["reason"], "Multiple sampler branches") + # #249: both saves used one pair, so it resolves with no sampler named. + self.assertEqual(result["state"], "resolved") + self.assertIsNone(result["sampler_id"]) + self.assertEqual(result["reason"], resolver.IDENTICAL_BRANCHES_REASON) self.assertEqual( sorted(item["sampler_id"] for item in result["candidates"]), ["3", "50"] ) diff --git a/utils/prompt_pair_resolver.py b/utils/prompt_pair_resolver.py index 1e8d4d2..f64fee9 100644 --- a/utils/prompt_pair_resolver.py +++ b/utils/prompt_pair_resolver.py @@ -17,8 +17,9 @@ ComfyUI, database or configuration dependencies; its only file read is the inspection profile's node semantics table (``utils.inspection_semantics``). Two ``ResolverProfile`` values exist. ``DEFAULT_PROFILE`` (ingestion: scan, live -monitor, admin import) keeps the historical behavior exactly, including the -500-node gate, and only adds ``reason_code: None``. ``INSPECTION_PROFILE`` bounds +monitor, admin import) keeps the historical behavior, including the 500-node +gate, and only adds ``reason_code: None``; both profiles resolve several sampler +branches that share one exact pair to that pair (#249). ``INSPECTION_PROFILE`` bounds work instead of graph size: a per-candidate visit allowance, a total visit budget shared by every candidate and attribution walk, a node ceiling backstop, and bounds on built provenance and text. It also names why a result is not resolved @@ -187,6 +188,8 @@ _MAX_BRANCH_CANDIDATES = 32 _RUNTIME_REASON = "Text is decided at run time" # Uncoded outcomes the inspector explains by their exact reason. EMPTY_POSITIVE_REASON = "Positive prompt is empty" +# Several sampler branches, every one resolved to the same pair (#249). +IDENTICAL_BRANCHES_REASON = "Every sampler branch uses the same prompt pair" MULTISTAGE_UNATTRIBUTED_REASON = "Multistage sampler ancestry is ambiguous" _INCOMPATIBLE_SLOT = "Output slot has an incompatible type" # Before 3.2.3 a save patch overwrote every PromptManager node's saved ``text`` @@ -1656,8 +1659,10 @@ def resolve_prompt_pair( Accepts ``{"prompt": api_graph, "parameters": optional_a1111_text}`` or a direct API graph. An optional saver/sampler node ID anchors branch selection. - Unanchored multiple samplers are ambiguous, even when their texts are equal - (the inspection profile first drops stages that only reach previews). + Unanchored multiple samplers are ambiguous unless every branch resolves to + the same texts and recipes; then that shared pair is resolved with no + ``sampler_id`` (the inspection profile first drops stages that only reach + previews). ``profile`` selects the bounds; explicit ``max_*`` keywords override it. """ profile = _profile_with(profile, max_visits, max_depth, max_text_length) @@ -1781,14 +1786,50 @@ def _resolve( if not sampler_ids: return _no_sampler(normalized, profile) if len(sampler_ids) > 1: + candidates = [resolver.pair(node_id) for node_id in sampler_ids] + shared = _shared_pair(candidates) + if shared is not None: + return shared return _result( "ambiguous", reason="Multiple sampler branches", - candidates=[resolver.pair(node_id) for node_id in sampler_ids], + candidates=candidates, ) return resolver.pair(sampler_ids[0]) +def _recipe_key(result: Dict[str, Any]) -> tuple: + """What a reader sees of a result, plus each side's stored recipe.""" + return _outcome_key(result) + tuple( + result[role]["storage_text"] if isinstance(result[role], dict) else None + for role in ("positive", "negative") + ) + + +def _shared_pair(candidates: list) -> Optional[Dict[str, Any]]: + """The one pair every sampler branch resolved to, or None (#249). + + A compare grid runs several samplers on one positive and one negative: the + prompt is certain even though no single branch made the image. Every branch + must be resolved with identical texts and recipes; any other mix stays + ambiguous. No sampler is named, since none alone made the output; the sides + (and their provenance) are the first branch's, and every branch stays listed + in ``candidates`` as the evidence. + """ + first = candidates[0] + if first["state"] != "resolved": + return None + if len({_recipe_key(item) for item in candidates}) != 1: + return None + return _result( + "resolved", + positive=first["positive"], + negative=first["negative"], + reason=IDENTICAL_BRANCHES_REASON, + candidates=candidates, + ) + + # Bounds for walking a UI workflow's subgraph instances (R1 only). _MAX_SUBGRAPH_DEPTH = 8 _MAX_WORKFLOW_NODES = 100_000 diff --git a/web/js/comfy-metadata.js b/web/js/comfy-metadata.js index a74e2de..f216646 100644 --- a/web/js/comfy-metadata.js +++ b/web/js/comfy-metadata.js @@ -631,6 +631,8 @@ * until PR3), and unresolved or absent over several listed samplers, merge the same * way across every listed sampler and its stages. Otherwise (no resolution, or one * listed sampler) exactly one listed sampler supplies them; several leave every field ambiguous. + * A resolved pair without a named sampler (identical branches, #249) merges like + * ambiguous when several samplers are listed. */ function graphFacts(graph, source, resolution) { if (!object(graph)) return factSet("unresolved", source); @@ -650,8 +652,9 @@ } const candidates = entries.filter(([id]) => samplerEntry(graph, id)); // Review I-1: an unresolved or absent prompt does not make the run's settings - // unknown, so several listed samplers merge the same way. - if (state === "ambiguous" || ((state === "unresolved" || state === "absent") && candidates.length > 1)) { + // unknown, so several listed samplers merge the same way. So does a pair the + // server resolved across several branches without naming one (#249). + if (state === "ambiguous" || (["unresolved", "absent", "resolved"].includes(state) && candidates.length > 1)) { // The server's branches: every listed sampler, a detailer judged with its stages. const stages = new Set(); if (!candidates.length || !candidates.every(([id]) => collectStages(graph, id, new Set(), stages, { visits: 0 }))) return factSet("unresolved", source);