From 27b48957954edc4289e871be03af029ea726850a Mon Sep 17 00:00:00 2001 From: Vito Date: Wed, 21 Oct 2026 10:55:35 -0700 Subject: [PATCH 1/2] fix(resolver): identical sampler branches resolve to their shared prompt pair (#270) * fix(resolver): identical sampler branches resolve to their shared prompt pair A LoRA epoch-compare grid runs one sampler per epoch, every one fed by the same positive encode and the same ConditioningZeroOut negative, and saves one concatenated image. `_resolve` returned "Multiple sampler branches" (ambiguous) whenever more than one sampler was found, even though every branch used the same pair, so the workspace inspector fell back to the legacy parser and the image could not be paired. When every branch resolves to identical texts and stored recipes, the result is now that pair: state resolved, no sampler named (none alone made the output), reason "Every sampler branch uses the same prompt pair", the first branch's sides and provenance, and every branch kept in `candidates` as evidence. A branch that differs, cannot be read, or is not resolved keeps the result ambiguous. Both profiles share the rule; the default profile still cannot read ConditioningZeroOut, so the zero-out grid stays ambiguous for ingestion and keeps the scan's legacy-parser fallback. Workspace facts: a resolved result without a named sampler over several listed samplers merges facts per field, as an ambiguous one does (the grid's shared seed/steps are known, the per-epoch model is ambiguous). Tests that encoded the old behavior (identical branches => ambiguous): - test_prompt_pair_resolver: test_multiple_samplers_are_ambiguous_even_when_texts_match -> test_multiple_samplers_with_matching_texts_resolve_to_the_shared_pair (plus a new differing-texts case that stays ambiguous); test_raw_and_detailed_saves_stay_ambiguous_without_an_anchor (same prompt) -> ..._of_one_prompt_share_it_without_an_anchor (anchored cases unchanged). - test_pair_scan: two samplers sharing one prompt now ingest the pair ("cat"/"blur") instead of the positive-only fallback. - test_api_prompt_inspector, test_inspection_outcomes, test_pair_image_linking: their "ambiguous" fixtures now use samplers with different prompts. No parity vector changes (f01/f09 branches are not all resolved). * fix(inspector): never show the positive as a zeroed negative from the legacy fill When the server leaves the negative undecided, the workspace fills it from the legacy browser parser (comfy-metadata.js), which follows ConditioningZeroOut back to its input and so reads the positive text as the negative. The workspace now checks the prompt graph first: - every sampler's negative is a ConditioningZeroOut (Reroutes allowed): the negative shows the server's explicitly empty zeroed state ({status: resolved, text: "", source: zeroed-conditioning}); - a zero-out elsewhere in a negative path (one branch zeroed, another encoded; a zero-out combined with other conditioning), or a workflow-only file holding one: the negative is not filled and keeps the server's status; - graphs without a zero-out keep the legacy fill unchanged. comfy-metadata.js is untouched; the legacy pages keep their reading. * fix(images): explain why Set category has nothing to set Set category on a selection where no image is linked to a Library prompt said only "None of the selected images has a linked prompt." It now says why and what it means: "The selected images aren't in your Library yet, so there is no prompt category to set." Unit and page-workflow tests follow the copy. * fix(inspector): check only the negative the legacy parse read for a zero-out Review I1/M2/M3/M5 of the zero-out legacy-fill guard. The guard decided over every node with a `negative` input, so a graph whose read sampler encodes "blurry" lost that legacy-filled negative when another branch zeroed its own (fewer than the legacy pages: below the parity floor). - comfy-metadata.js gains `legacyNegativeSource`, which returns the graph and sampler `negative` link that `extractGenerationParams` reads from. It reuses the parser's own sampler pick (`selectSampler`, factored out of `fromPromptGraph` without changing any reading). The workspace checks only that link: a direct zero-out shows the zeroed empty negative, a zero-out further up the path leaves it unfilled, and anything else keeps the legacy fill. - Workflow-only files go through the parser's own workflow conversion, so bypassed (mode 4) and muted (mode 2) zero-outs and unconnected ones behave as the legacy reading does. A subgraph instance whose definition runs a zero-out (directly or nested) leaves the negative unfilled. - One bounded walk with an index pointer replaces a walk per negative input with queue.shift(). - An ambiguous fact that the legacy parse knows (a grid's per-branch model) is still filled, documented: the legacy pages show it and this page never shows less. Tests: the reversed mixed graph shows "blurry"; the mixed graph whose read sampler is zeroed, the zero-out-only grid and a direct workflow zero-out show the zeroed empty negative; a combined zero-out and a zero-out subgraph instance stay unfilled; bypassed and unconnected workflow zero-outs keep the legacy reading. An API-graph Reroute in front of a zero-out gives the legacy parse no negative, so the server answer stays (that expectation changed from zeroed). * fix(images): name the missing link in the Set category toast Review M4: say that no selected image is linked to a saved prompt, in the same terms as the partial-success toast and right for one image. Also review M1: a monitor test that two parallel samplers sharing one positive and one negative write that pair (the resolver collapse). --- tests/browser/workspace-images.spec.mjs | 2 +- tests/js/comfy-metadata.test.js | 16 ++ tests/js/inspection-semantics-sync.test.js | 2 +- tests/js/workspace-images-category.test.js | 2 +- tests/js/workspace-inspection-state.test.js | 130 ++++++++++ tests/test_api_prompt_inspector.py | 7 +- tests/test_identical_sampler_branches.py | 248 ++++++++++++++++++++ tests/test_inspection_outcomes.py | 4 +- tests/test_pair_image_linking.py | 25 +- tests/test_pair_scan.py | 7 +- tests/test_prompt_pair_resolver.py | 27 ++- utils/prompt_pair_resolver.py | 51 +++- web/js/comfy-metadata.js | 40 +++- web/js/workspace/images-category.mjs | 2 +- web/js/workspace/inspection-state.mjs | 61 ++++- 15 files changed, 600 insertions(+), 24 deletions(-) create mode 100644 tests/test_identical_sampler_branches.py diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index 57909a8..5a97715 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -1921,7 +1921,7 @@ test('Set category without a linked image posts nothing and opens no dialog', as await page.route(categoryRoute, route => { posted += 1; return route.continue(); }); await checkbox(cards(page).last(), 'same.png').check(); await categoryButton(page).click(); - await expect(toast(page)).toHaveText('None of the selected images has a linked prompt.'); + await expect(toast(page)).toHaveText("None of the selected images is linked to a saved prompt, so there is no prompt category to set."); await expect(page.locator('dialog[open]')).toHaveCount(0); expect(posted).toBe(0); await expect(bulkbar(page).locator('.bulkbar > strong')).toHaveText('1 selected'); 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/js/inspection-semantics-sync.test.js b/tests/js/inspection-semantics-sync.test.js index bbbbd38..bd9f094 100644 --- a/tests/js/inspection-semantics-sync.test.js +++ b/tests/js/inspection-semantics-sync.test.js @@ -30,7 +30,7 @@ test("facts guider providers are guiders in the semantics table", () => { }); test("the facts tables are not part of the legacy page contract", () => { - assert.deepStrictEqual(Object.keys(metadata).filter(key => !key.startsWith("_")).sort(), ["PLACEHOLDERS", "extractGenerationFacts", "extractGenerationParams"]); + assert.deepStrictEqual(Object.keys(metadata).filter(key => !key.startsWith("_")).sort(), ["PLACEHOLDERS", "extractGenerationFacts", "extractGenerationParams", "legacyNegativeSource"]); assert.ok(Object.isFrozen(metadata._factsTables)); }); diff --git a/tests/js/workspace-images-category.test.js b/tests/js/workspace-images-category.test.js index 641d3fa..612e46a 100644 --- a/tests/js/workspace-images-category.test.js +++ b/tests/js/workspace-images-category.test.js @@ -33,7 +33,7 @@ test('the outcome toast counts the prompts and the skipped images', async () => test('the dialog and failure copy is the plan\'s', async () => { const {CATEGORY_DIALOG, CATEGORY_NONE_LINKED, CATEGORY_FAILED} = await source; assert.deepEqual(CATEGORY_DIALOG, {title:'Categorize selected images', label:'Category', action:'Apply'}); - assert.equal(CATEGORY_NONE_LINKED, 'None of the selected images has a linked prompt.'); + assert.equal(CATEGORY_NONE_LINKED, "None of the selected images is linked to a saved prompt, so there is no prompt category to set."); assert.equal(CATEGORY_FAILED, 'Category could not be set. Try again.'); assert.ok(Object.isFrozen(CATEGORY_DIALOG)); }); diff --git a/tests/js/workspace-inspection-state.test.js b/tests/js/workspace-inspection-state.test.js index 92bc176..6fca178 100644 --- a/tests/js/workspace-inspection-state.test.js +++ b/tests/js/workspace-inspection-state.test.js @@ -538,6 +538,136 @@ test('a definite server prompt is kept even when the legacy parser reads it diff assert.equal(record.prompts.positive.text, 'a lighthouse at dusk, oil painting'); }); +// #249: a LoRA compare grid. Every sampler's negative is a ConditioningZeroOut of the +// positive encode; the legacy parser follows the zero-out back to the positive text. +function zeroedGridGraph(samplers = 3) { + const 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' } }, + 4: { class_type: 'CLIPTextEncode', inputs: { text: 'a woman holding a coffee cup', clip: ['2', 0] } }, + 5: { class_type: 'ConditioningZeroOut', inputs: { conditioning: ['4', 0] } }, + 6: { class_type: 'EmptyLatentImage', inputs: { width: 512, height: 512, batch_size: 1 } }, + }; + for (let index = 0; index < samplers; index++) { + graph[`1${index}`] = { class_type: 'LoraLoaderModelOnly', inputs: { lora_name: `epoch_${index}.safetensors`, strength_model: 1, model: ['1', 0] } }; + graph[`2${index}`] = { class_type: 'KSampler', inputs: { seed: 42, steps: 8, cfg: 1, sampler_name: 'euler', scheduler: 'simple', denoise: 1, + model: [`1${index}`, 0], positive: ['4', 0], negative: ['5', 0], latent_image: ['6', 0] } }; + } + return graph; +} + +function undecided(graph) { + const entry = image(), data = envelope(entry); + data.prompts = { positive: { status: 'ambiguous', text: null, source: null }, negative: { status: 'ambiguous', text: null, source: null } }; + data.metadata.prompt_graph = { status: 'parsed', value: graph }; + data.resolution = { state: 'ambiguous', sampler_id: null, reason: 'Multiple sampler branches' }; + return contract('toInspectionRecord')(data, entry, extractGenerationFacts); +} + +test('a zeroed negative is never filled with the positive text from the legacy parse (#249)', t => { + const parser = require('../../web/js/comfy-metadata.js'); + globalThis.ComfyMetadata = parser; + t.after(() => { delete globalThis.ComfyMetadata; }); + // The legacy pages keep their reading: the zero-out passes the positive through. + assert.equal(parser.extractGenerationParams({ prompt: zeroedGridGraph() }).negativePrompt, 'a woman holding a coffee cup'); + const record = undecided(zeroedGridGraph()); + assert.deepEqual(record.prompts.positive, { status: 'resolved', text: 'a woman holding a coffee cup', source: 'graph-inferred' }); + assert.deepEqual(record.prompts.negative, { status: 'resolved', text: '', source: 'zeroed-conditioning' }); + // Behind an API-graph Reroute the legacy parse reads no negative, so there is nothing + // to correct: the server's answer stays. + const rerouted = zeroedGridGraph(1); + rerouted[7] = { class_type: 'Reroute', inputs: { value: ['5', 0] } }; + rerouted[20].inputs.negative = ['7', 0]; + assert.equal(parser.legacyNegativeSource({ prompt: rerouted }), null); + assert.deepEqual(undecided(rerouted).prompts.negative, { status: 'ambiguous', text: null, source: null }); +}); + +test('the zero-out check follows the sampler the legacy parse read (#249)', t => { + const parser = require('../../web/js/comfy-metadata.js'); + globalThis.ComfyMetadata = parser; + t.after(() => { delete globalThis.ComfyMetadata; }); + const zeroed = { status: 'resolved', text: '', source: 'zeroed-conditioning' }; + // The legacy parse reads the first sampler (20): its negative is the zero-out. + const mixed = zeroedGridGraph(2); + mixed[8] = { class_type: 'CLIPTextEncode', inputs: { text: 'blurry', clip: ['2', 0] } }; + mixed[21].inputs.negative = ['8', 0]; + assert.deepEqual(undecided(mixed).prompts.negative, zeroed); + // Reversed: the sampler it reads encodes "blurry", which it showed before and still does. + const reversed = zeroedGridGraph(2); + reversed[8] = { class_type: 'CLIPTextEncode', inputs: { text: 'blurry', clip: ['2', 0] } }; + reversed[20].inputs.negative = ['8', 0]; + assert.equal(parser.extractGenerationParams({ prompt: reversed }).negativePrompt, 'blurry'); + assert.deepEqual(undecided(reversed).prompts.negative, { status: 'resolved', text: 'blurry', source: 'graph-inferred' }); + // The positive fills from the legacy parse in both cases. + assert.equal(undecided(mixed).prompts.positive.text, 'a woman holding a coffee cup'); + assert.equal(undecided(reversed).prompts.positive.text, 'a woman holding a coffee cup'); +}); + +test('a zero-out mixed into the read negative leaves it as the server left it (#249)', t => { + globalThis.ComfyMetadata = require('../../web/js/comfy-metadata.js'); + t.after(() => { delete globalThis.ComfyMetadata; }); + // A zero-out combined with another conditioning is not an empty negative. + const combined = zeroedGridGraph(1); + combined[8] = { class_type: 'CLIPTextEncode', inputs: { text: 'blurry', clip: ['2', 0] } }; + combined[9] = { class_type: 'ConditioningCombine', inputs: { conditioning_1: ['5', 0], conditioning_2: ['8', 0] } }; + combined[20].inputs.negative = ['9', 0]; + assert.deepEqual(undecided(combined).prompts.negative, { status: 'ambiguous', text: null, source: null }); +}); + +function workflowOnly(workflow) { + const entry = image(), data = envelope(entry); + data.prompts = { positive: { status: 'unresolved', text: null, source: null }, negative: { status: 'unresolved', text: null, source: null } }; + Object.assign(data.metadata, { raw: { workflow: JSON.stringify(workflow) }, record_formats: { workflow: 'reserialized-json' }, + prompt_graph: { status: 'absent', value: null }, workflow_graph: { status: 'parsed', value: workflow } }); + return contract('toInspectionRecord')(data, entry, extractGenerationFacts); +} + +// A UI workflow: encode 4 -> zero-out 5 -> sampler 3's negative, encode 4 -> positive. +function zeroedWorkflow({ zeroMode = 0, extraNodes = [], extraLinks = [], negativeLink = 3 } = {}) { + return { nodes: [ + { id: 4, type: 'CLIPTextEncode', inputs: [], outputs: [{ links: [1, 2] }], widgets_values: ['a woman holding a coffee cup'] }, + { id: 5, type: 'ConditioningZeroOut', mode: zeroMode, inputs: [{ name: 'conditioning', type: 'CONDITIONING', link: 1 }], outputs: [{ links: [3] }] }, + { id: 1, type: 'UNETLoader', inputs: [], outputs: [{ links: [4] }], widgets_values: ['base.safetensors', 'default'] }, + { id: 3, type: 'KSampler', inputs: [{ name: 'model', link: 4 }, { name: 'positive', link: 2 }, { name: 'negative', link: negativeLink }], widgets_values: [42, 'fixed', 8, 1, 'euler', 'simple', 1] }, + ...extraNodes, + ], links: [[1, 4, 0, 5, 0, 'CONDITIONING'], [2, 4, 0, 3, 1, 'CONDITIONING'], [3, 5, 0, 3, 2, 'CONDITIONING'], [4, 1, 0, 3, 0, 'MODEL'], ...extraLinks] }; +} + +test('workflow-only files follow the legacy reading of their zero-out (#249)', t => { + const parser = require('../../web/js/comfy-metadata.js'); + globalThis.ComfyMetadata = parser; + t.after(() => { delete globalThis.ComfyMetadata; }); + // A connected zero-out on the read sampler's negative is the zeroed empty negative. + const direct = zeroedWorkflow(); + assert.equal(parser.extractGenerationParams({ workflow: direct }).negativePrompt, 'a woman holding a coffee cup', 'the legacy parse would fill it'); + assert.deepEqual(workflowOnly(direct).prompts.negative, { status: 'resolved', text: '', source: 'zeroed-conditioning' }); + // A bypassed zero-out (mode 4) passes its conditioning through: the legacy reading is right. + const bypassed = workflowOnly(zeroedWorkflow({ zeroMode: 4 })); + assert.deepEqual(bypassed.prompts.negative, { status: 'resolved', text: 'a woman holding a coffee cup', source: 'graph-inferred' }); + // An unconnected zero-out beside a text negative changes nothing. + const unconnected = workflowOnly(zeroedWorkflow({ negativeLink: 6, + extraNodes: [{ id: 6, type: 'CLIPTextEncode', inputs: [], outputs: [{ links: [6] }], widgets_values: ['blurry'] }], + extraLinks: [[6, 6, 0, 3, 2, 'CONDITIONING']] })); + assert.deepEqual(unconnected.prompts.negative, { status: 'resolved', text: 'blurry', source: 'graph-inferred' }); +}); + +test('a negative read through a subgraph holding a zero-out is not filled (#249)', t => { + const parser = require('../../web/js/comfy-metadata.js'); + globalThis.ComfyMetadata = parser; + t.after(() => { delete globalThis.ComfyMetadata; }); + // The sampler's negative comes from a subgraph instance whose definition zeroes it. + const definition = 'aaaaaaaa-0000-4000-8000-000000000001'; + const workflow = zeroedWorkflow({ negativeLink: 7, + extraNodes: [{ id: 9, type: definition, inputs: [{ name: 'conditioning', type: 'CONDITIONING', link: 8 }], outputs: [{ links: [7] }] }], + extraLinks: [[7, 9, 0, 3, 2, 'CONDITIONING'], [8, 4, 0, 9, 0, 'CONDITIONING']] }); + workflow.definitions = { subgraphs: [{ id: definition, nodes: [{ id: 1, type: 'ConditioningZeroOut', inputs: [], outputs: [] }], links: [] }] }; + assert.equal(parser.extractGenerationParams({ workflow }).negativePrompt, 'a woman holding a coffee cup', 'the legacy parse would fill it'); + assert.deepEqual(workflowOnly(workflow).prompts.negative, { status: 'unresolved', text: null, source: null }); + // The same instance of a definition without a zero-out keeps the legacy fill. + workflow.definitions.subgraphs[0].nodes[0].type = 'Reroute'; + assert.equal(workflowOnly(workflow).prompts.negative.text, 'a woman holding a coffee cup'); +}); + test('callers without the browser global pass the legacy parser explicitly', () => { const parser = require('../../web/js/comfy-metadata.js'); const entry = image(), data = envelope(entry); 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..9c2b962 100644 --- a/tests/test_pair_image_linking.py +++ b/tests/test_pair_image_linking.py @@ -51,11 +51,34 @@ class TestPairImageLinking(unittest.TestCase): self.assertEqual(pairs[0]["image_count"], 1) self.assertEqual(pairs[0]["run_count"], 0) + def test_parallel_identical_branches_create_their_shared_pair(self): + # #249: two samplers on one positive and one negative are one certain pair. + meta = metadata() + meta["prompt"]["4"] = { + "class_type": "KSampler", + "inputs": {"positive": ["1", 0], "negative": ["2", 0]}, + } + self.handler.link_image_to_prompt(self.path, {"id": self.pid}, meta) + pairs = self.db.list_prompt_pairs()["pairs"] + self.assertEqual( + [(p["positive_text"], p["negative_text"]) for p in pairs], [("cat", "blur")] + ) + self.assertEqual(pairs[0]["image_count"], 1) + self.assertEqual(pairs[0]["run_count"], 0) + def test_stale_context_or_ambiguous_metadata_does_not_create_pair(self): meta = metadata(positive="dog") 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..c505ea7 100644 --- a/web/js/comfy-metadata.js +++ b/web/js/comfy-metadata.js @@ -212,12 +212,17 @@ return undefined; } + /** The sampler the legacy parse reads: a custom sampler's guider, else the first sampler. */ + function selectSampler(ctx, nodes) { + const custom = nodes.find(isCustomSampler); + const [, guider] = custom ? enter(ctx, custom.inputs.guider, new Set()) : []; + return { custom, sampler: isSampler(guider) ? guider : nodes.find(isSampler) }; + } + function fromPromptGraph(graph, workflow) { const ctx = createContext(graph, workflow); const nodes = Object.values(graph); - const custom = nodes.find(isCustomSampler); - const [, guider] = custom ? enter(ctx, custom.inputs.guider, new Set()) : []; - const sampler = isSampler(guider) ? guider : nodes.find(isSampler); + const { custom, sampler } = selectSampler(ctx, nodes); if (!sampler) { const loose = nodes.find((n) => n && n.inputs && /ksampler/i.test(n.class_type || "")); const inputs = (loose && loose.inputs) || {}; @@ -402,6 +407,26 @@ return merged; } + /** + * Where `extractGenerationParams` reads its negative from (#249): the graph it used + * (the API graph, else the workflow converted the same way) and the read sampler's + * `negative` link, or null when it found no negative. It changes no reading. + */ + function legacyNegativeSource(comfyData) { + const data = comfyData || {}; + const sources = []; + if (data.prompt && typeof data.prompt === "object") sources.push([data.prompt, data.workflow]); + if (data.workflow) sources.push([workflowToGraph(data.workflow), null]); + for (const [graph, workflow] of sources) { + const ctx = createContext(graph, workflow); + const { sampler } = selectSampler(ctx, Object.values(graph)); + if (sampler && hasValue(traceConditioning(ctx, sampler.inputs.negative, "negative"))) { + return { graph, link: sampler.inputs.negative }; + } + } + return null; + } + const FACT_FIELDS = ["seed", "steps", "cfg", "sampler", "scheduler", "model"]; const WORKFLOW_PASSTHROUGH = Symbol("inspection workflow passthrough"); // These tables must equal inspection-semantics.json: SAMPLERS its `samplers` section @@ -631,6 +656,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 +677,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); @@ -758,7 +786,7 @@ return factSet(metadataStatus === "available" || metadataStatus === "absent" ? "not-recorded" : "unresolved"); } - const api = { extractGenerationParams, extractGenerationFacts, PLACEHOLDERS }; + const api = { extractGenerationParams, extractGenerationFacts, legacyNegativeSource, PLACEHOLDERS }; // For the semantics sync test only; not part of the legacy page contract. Object.defineProperty(api, "_factsTables", { value: Object.freeze({ diff --git a/web/js/workspace/images-category.mjs b/web/js/workspace/images-category.mjs index c305c68..77c9085 100644 --- a/web/js/workspace/images-category.mjs +++ b/web/js/workspace/images-category.mjs @@ -5,7 +5,7 @@ import { countOf, noun } from './plural.mjs'; * filter matches the linked positive prompt's, so the action sets the category on those prompts. */ export const CATEGORY_DIALOG = Object.freeze({title:'Categorize selected images', label:'Category', action:'Apply'}); -export const CATEGORY_NONE_LINKED = 'None of the selected images has a linked prompt.'; +export const CATEGORY_NONE_LINKED = "None of the selected images is linked to a saved prompt, so there is no prompt category to set."; export const CATEGORY_FAILED = 'Category could not be set. Try again.'; // The server's limit (`validate_category`): the field stops there instead of failing on Apply. export const CATEGORY_MAX_LENGTH = 100; diff --git a/web/js/workspace/inspection-state.mjs b/web/js/workspace/inspection-state.mjs index 98fb521..1a2fdbe 100644 --- a/web/js/workspace/inspection-state.mjs +++ b/web/js/workspace/inspection-state.mjs @@ -174,6 +174,55 @@ function comfyMetadata() { return globalThis.ComfyMetadata || globalThis.window?.ComfyMetadata; } +// #249: the legacy parse follows ConditioningZeroOut back to its input, so it reads the +// positive text as the negative. Only the negative it actually read is checked: a direct +// zero-out shows the server's zeroed state; a zero-out elsewhere on +// that path, or a subgraph instance whose definition holds one, leaves the negative +// unfilled. Any other negative keeps the legacy fill, so this page never shows less. +const ZERO_OUT = 'ConditioningZeroOut'; +const ZEROED_NEGATIVE = Object.freeze({ status: 'resolved', text: '', source: 'zeroed-conditioning' }); +const MAX_NEGATIVE_VISITS = 2000; +const MODE_MUTED = 2, MODE_BYPASSED = 4; +const isLink = value => Array.isArray(value) && value.length === 2 && ['string', 'number'].includes(typeof value[0]); + +/** Subgraph definition IDs that run a zero-out, directly or through a nested instance. */ +function zeroOutSubgraphs(workflow) { + const definitions = Array.isArray(workflow?.definitions?.subgraphs) ? workflow.definitions.subgraphs.filter(object) : []; + const active = item => Array.isArray(item.nodes) ? item.nodes.filter(node => object(node) && ![MODE_MUTED, MODE_BYPASSED].includes(node.mode)) : []; + const found = new Set(); + for (let changed = true; changed;) { + changed = false; + for (const item of definitions) { + if (found.has(item.id) || !active(item).some(node => node.type === ZERO_OUT || found.has(node.type))) continue; + found.add(item.id); changed = true; + } + } + return found; +} + +/** 'zeroed', 'undecided' (do not fill), or null (the legacy fill applies). */ +function zeroOutNegative(source, workflow) { + if (!source || !object(source.graph) || !isLink(source.link)) return null; + const { graph } = source, subgraphs = zeroOutSubgraphs(workflow); + const zeroing = node => node.class_type === ZERO_OUT || subgraphs.has(node.class_type); + if (!Object.values(graph).some(node => object(node) && zeroing(node))) return null; + const node = graph[String(source.link[0])]; + if (!object(node)) return 'undecided'; + if (node.class_type === ZERO_OUT) return 'zeroed'; + // One bounded walk upstream; an index pointer keeps it linear in the nodes visited. + const queue = [node], seen = new Set(); + for (let index = 0; index < queue.length; index++) { + const current = queue[index]; + if (zeroing(current) || seen.size >= MAX_NEGATIVE_VISITS) return 'undecided'; + for (const value of object(current.inputs) ? Object.values(current.inputs) : []) { + const id = isLink(value) ? String(value[0]) : null; + if (id === null || seen.has(id) || !object(graph[id])) continue; + seen.add(id); queue.push(graph[id]); + } + } + return null; +} + function withWorkflowParams(record, parser = comfyMetadata()) { const graph = record.metadata.prompt_graph.status === 'parsed' ? record.metadata.prompt_graph.value : null; const workflow = record.metadata.workflow_graph.status === 'parsed' ? record.metadata.workflow_graph.value : null; @@ -184,14 +233,24 @@ function withWorkflowParams(record, parser = comfyMetadata()) { const found = key => typeof params[key] === 'string' || typeof params[key] === 'number' ? String(params[key]) !== '' && params[key] !== parser.PLACEHOLDERS?.[key] : false; const prompts = { ...record.prompts }; + const read = prompts.negative?.status === 'resolved' || typeof parser.legacyNegativeSource !== 'function' ? null : + parser.legacyNegativeSource({ prompt: graph, workflow }); + const zeroed = zeroOutNegative(read, workflow); for (const [role, key] of [['positive', 'positivePrompt'], ['negative', 'negativePrompt']]) { const current = prompts[role]; - if (!found(key) || current?.status === 'resolved') continue; + if (current?.status === 'resolved') continue; + if (role === 'negative' && zeroed !== null) { + if (zeroed === 'zeroed') prompts.negative = { ...ZEROED_NEGATIVE }; + continue; + } + if (!found(key)) continue; prompts[role] = { status: 'resolved', text: params[key], source: 'graph-inferred' }; } record.prompts = prompts; for (const [field, key] of LEGACY_FACTS) { // Only open answers are filled: a known value or a definite "not recorded" stays. + // An ambiguous field (e.g. a compare grid's per-branch model) is filled too, on + // purpose: the legacy pages show this value, and this page never shows less. if (!found(key) || !['unresolved', 'ambiguous'].includes(record.generation[field]?.status)) continue; record.generation[field] = { status: 'known', value: String(params[key]), source: graph ? 'api-graph' : 'workflow', evidence: 'configuration', reason: null, sampler_id: null }; } From 454ac12e1dea33a3c14a3d78c9234239dc6d021d Mon Sep 17 00:00:00 2001 From: Vito Date: Wed, 21 Oct 2026 12:36:38 -0700 Subject: [PATCH 2/2] fix(images): list sources and folders that load after Refine opened (#272) * fix(images): list sources and folders that load after Refine opened "Refine this view" copied the discovered sources and folders when it opened, so opening Filters before /gallery/subfolders answered left Source and Folder at "All sources" / "All output folders" until the dialog was reopened. On a slow CI runner this stalled the touch "collection viewer follows filtered loaded order beyond mounted cards" workflow in selectOption('0') until the 30s timeout. openRefineDialog now returns setOptions(key, options); the Images page calls it when discovery settles, and the open dialog refills Source and its Folder list, showing the value it opened with (or the one chosen since) once offered. It does nothing after the dialog closed or was replaced. * test(library): read filtered shelf titles after the cards' frame The Library concept comparison read card titles as soon as the caption changed and the card count dropped below six. Production empties the shelf when a filter applies, writes the new caption when the page answers and places the cards in the next animation frame, so on a slow CI runner the empty shelf satisfied "fewer than six" and the titles read as []. Settle one frame pair before reading, as the Images and Inspector comparisons already do. --- tests/browser/library-concept.spec.mjs | 4 ++ tests/browser/workspace-images.spec.mjs | 24 ++++++++++++ tests/js/workspace-toolbar.test.js | 50 +++++++++++++++++++++++++ web/js/workspace/images.mjs | 10 ++++- web/js/workspace/workspace-dialogs.mjs | 25 +++++++++++-- 5 files changed, 107 insertions(+), 6 deletions(-) diff --git a/tests/browser/library-concept.spec.mjs b/tests/browser/library-concept.spec.mjs index 1589ad2..afc73ae 100644 --- a/tests/browser/library-concept.spec.mjs +++ b/tests/browser/library-concept.spec.mjs @@ -158,6 +158,10 @@ async function filteredOf(page,apply,undo) { await apply(); await expect.poll(()=>captionOf(page)).not.toBe(before); await expect.poll(()=>page.locator(CARDS).count()).toBeLessThan(CONCEPT_PROMPTS); + // Production empties the shelf when the filter applies, writes the answer's caption at once and places its cards + // in the next animation frame (one paint for a viewer), so the empty shelf already counts as "fewer": read the + // titles only after that frame, as the Images and Inspector comparisons do. + await settle(page); const result={caption:await captionOf(page),titles:await titlesOf(page)}; await undo(); await expect.poll(()=>captionOf(page)).toBe(before); diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index 5a97715..2398862 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -1487,6 +1487,30 @@ test('a star press leaves the continuation status, count and caption date untouc expect(await page.evaluate(() => window.__statusMutations)).toEqual([]); }); +// Opened before the source and folder discovery answered (a slow server), Refine lists them once it does. +test('Refine opened before discovery settles lists the discovered sources and folders', async ({ page }) => { + let release, started; + const held = new Promise(resolve => { release = resolve; }), hit = new Promise(resolve => { started = resolve; }); + await page.route('**/prompt_manager/gallery/subfolders?*', async route => { started(); await held; await route.continue(); }, { times: 1 }); + await page.goto(workspace); + await expect(cards(page)).toHaveCount(2); + await hit; + await filtersButton(page).click(); + const source = refineDialog(page).getByLabel('Source', { exact: true }); + const folder = refineDialog(page).getByLabel('Folder', { exact: true }); + const values = select => select.locator('option').evaluateAll(options => options.map(option => option.value)); + expect(await values(source)).toEqual(['all']); + expect(await values(folder)).toEqual(['']); + release(); + await expect.poll(() => values(source)).toEqual(['all', '0', '1']); + expect(await values(folder)).toEqual(['', 'shared']); + await expect(source).toHaveValue('all'); + await source.selectOption('1'); + await refineDialog(page).getByRole('button', { name: 'Apply filters', exact: true }).click(); + await expect(cards(page)).toHaveCount(1); + await expect(cards(page)).toHaveAttribute('data-image-key', '1:shared/same.png'); +}); + // #249 final review m1: as in the old field row, a Source offers only its own folders. test('Refine offers only the chosen source folders', async ({ page, request }) => { await expect(await request.post('/__test__/add-nested-image')).toBeOK(); diff --git a/tests/js/workspace-toolbar.test.js b/tests/js/workspace-toolbar.test.js index bf01384..789569d 100644 --- a/tests/js/workspace-toolbar.test.js +++ b/tests/js/workspace-toolbar.test.js @@ -342,6 +342,56 @@ test('refine dialog round-trips values and clear resets them', async () => { } }); +test('Refine lists options discovered after it opened and keeps the wanted values', async () => { + const { openRefineDialog, openImportDialog, closeWorkspaceDialog } = await dialogsModule; + const dom = fakeDOM(); + const scope = (await coreModule).createScope(); + try { + let bySource = {}; + const optionsFor = source => [['', 'All output folders'], ...(bySource[source] ?? []).map(path => [path, path])]; + const fields = [{ key: 'source', options: [['all', 'All sources']] }, { key: 'folder', options: optionsFor('all'), optionsFor }, + REFINE_FIELDS[2]]; + const applied = []; + const refine = openRefineDialog({ scope, fields, values: { source: '1', folder: 'temp-only', media: 'video' }, + onApply: value => applied.push(value), onClear() {} }); + const [source, folder, media] = byId(dom, 'modal-body').children.filter(child => child.className === 'field') + .map(field => field.children[1]); + // Opened before discovery: only the cleared values. + assert.deepEqual(optionPairs(source), [['all', 'All sources']]); + assert.equal(source.value, 'all'); + assert.equal(folder.value, ''); + bySource = { all: ['portraits', 'temp-only'], 0: ['portraits'], 1: ['temp-only'] }; + refine.setOptions('source', [['all', 'All sources'], ['0', 'output'], ['1', 'archive']]); + // The values it opened with, now offered, and the chosen Source's folders. + assert.deepEqual(optionPairs(source).map(([value]) => value), ['all', '0', '1']); + assert.equal(source.value, '1'); + assert.deepEqual(optionPairs(folder), [['', 'All output folders'], ['temp-only', 'temp-only']]); + assert.equal(folder.value, 'temp-only'); + // A choice made since wins over the opening value; tags and unknown keys are ignored. + source.value = '0'; + source.dispatchEvent(new Event('change')); + refine.setOptions('source', [['all', 'All sources'], ['0', 'output'], ['1', 'archive']]); + refine.setOptions('tags', [['x', 'x']]); + assert.equal(source.value, '0'); + assert.deepEqual(optionPairs(folder).map(([value]) => value), ['', 'portraits']); + assert.equal(media.value, 'video'); + button(byId(dom, 'modal-body'), 'Apply filters').click(); + assert.deepEqual(applied, [{ source: '0', folder: '', media: 'video' }]); + // Closed: a late discovery leaves the shell alone. + refine.setOptions('source', [['all', 'All sources']]); + assert.equal(source.options.length, 3); + // Replaced by another dialog: the same. + const again = openRefineDialog({ scope, fields, values: {}, onApply() {}, onClear() {} }); + openImportDialog({ scope, onInspect() {}, onRescan() {} }); + again.setOptions('source', [['all', 'All sources'], ['9', 'late']]); + assert.equal(byId(dom, 'modal-title').textContent, 'Import & inspect'); + } finally { + closeWorkspaceDialog(); + scope.dispose(); + dom.restore(); + } +}); + test('the Folder list follows the chosen Source', async () => { const { openRefineDialog, closeWorkspaceDialog } = await dialogsModule; const dom = fakeDOM(); diff --git a/web/js/workspace/images.mjs b/web/js/workspace/images.mjs index e3ab44b..ddc1b09 100644 --- a/web/js/workspace/images.mjs +++ b/web/js/workspace/images.mjs @@ -133,6 +133,8 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage let state, geometry = { positions: [], columns: 1, height: 0, maxHeight: 0 }, galleryWidth = 0, optionsRequest, categoryRequest, recovering = false, rescanning = false; // Discovered sources (each with its own folders) and every folder, for "Refine this view" and the scope chip. let roots = [], folders = []; + // The open "Refine this view", so a discovery that settles after it opened still lists its sources and folders. + let refine = null; const rendered = new Map(); const itemIndexes = new Map(); const filters = { root: 'all', q: '', category: '', tags: [], subfolder: '', media: 'all', sort: 'newest', ...visit?.filters, ...saved, @@ -643,8 +645,8 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage /** "Refine this view": Source, Folder, Media and Tags, the filters the old field row held (D14). */ function openFilters(trigger) { if (recovering) return; - openRefineDialog({scope, trigger, fields:[ - {key:'source', options:[['all', 'All sources'], ...roots.map(root => [String(root.index), root.label])]}, + refine = openRefineDialog({scope, trigger, fields:[ + {key:'source', options:sourceOptions()}, {key:'folder', options:folderOptions('all'), optionsFor:folderOptions}, {key:'media', options:MEDIA_OPTIONS}, {key:'tags'}, @@ -654,6 +656,9 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage applyRefinement(cleared); }}); } + function sourceOptions() { + return [['all', 'All sources'], ...roots.map(root => [String(root.index), root.label])]; + } /** The chosen Source's folders, as the old field row reloaded them; every folder under All sources. */ function folderOptions(source) { const root = source === 'all' ? null : roots.find(item => String(item.index) === source); @@ -750,6 +755,7 @@ export function mountImages(main, api, parentScope, {embedded = false, fetchPage roots = data.roots.filter(root => Number.isSafeInteger(root?.index) && typeof root.label === 'string') .map(root => ({index:root.index, label:root.label, subfolders:paths(root.subfolders)})); folders = paths(data.subfolders); + refine?.setOptions('source', sourceOptions()); updateScope(); sourcesRetry = null; return true; diff --git a/web/js/workspace/workspace-dialogs.mjs b/web/js/workspace/workspace-dialogs.mjs index 78179d8..1364ec9 100644 --- a/web/js/workspace/workspace-dialogs.mjs +++ b/web/js/workspace/workspace-dialogs.mjs @@ -152,6 +152,9 @@ function choose(select, value) { * Tags; Folder may add `optionsFor(source)`, the options of one Source, so its list follows the chosen Source and a * folder that Source lacks returns to the first option. `values` holds the current value per key. Apply reports the edited values; Clear resets every field and * reports the cleared values plus `category: ''`, so the page clears its toolbar category too. Both close first. + * Returns `{setOptions(key, options)}` for options discovered after the dialog opened (a slow folder discovery): the + * select is refilled and shows the value the field opened with, or the one chosen since, once it is offered; a Folder + * that follows its Source is refilled from `optionsFor` again. It does nothing once this dialog closed or was replaced. */ export function openRefineDialog({ scope, fields, values = {}, onApply, onClear, trigger = null }) { requireScope(scope); @@ -172,16 +175,21 @@ export function openRefineDialog({ scope, fields, values = {}, onApply, onClear, choose(select, values[key]); return [{ key, control: select, cleared: item.options[0][0], optionsFor: item.optionsFor }]; }); + // Each select's wanted value: the one it opened with until the person picks another (see setOptions). + const wanted = new Map(controls.map(({ key }) => [key, values[key]])); + for (const { key, control } of controls) { + if (key !== 'tags') control.addEventListener('change', () => wanted.set(key, control.value)); + } const source = controls.find(item => item.key === 'source'); const folder = controls.find(item => item.key === 'folder' && typeof item.optionsFor === 'function'); - const followSource = () => { - const chosen = folder.control.value; + const followSource = (chosen = folder.control.value) => { fillSelect(folder.control, 'folder', folder.optionsFor(source.control.value)); choose(folder.control, chosen); }; if (source && folder) { - followSource(); - source.control.addEventListener('change', followSource); + followSource(values.folder); + // A Source that lacks the chosen folder clears it, and that cleared folder is the wanted one from then on. + source.control.addEventListener('change', () => { followSource(); wanted.set('folder', folder.control.value); }); } const read = () => Object.fromEntries(controls.map(({ key, control }) => [key, key === 'tags' ? control.value.trim() : control.value])); @@ -207,6 +215,15 @@ export function openRefineDialog({ scope, fields, values = {}, onApply, onClear, ...controls.map(({ key, control }) => field(FIELD_LABELS[key], control, `filter-${key}`)), actions, ], { scope, trigger }); + const opened = session; + function setOptions(key, options) { + const item = controls.find(candidate => candidate.key === key); + if (session !== opened || !item || key === 'tags') return; + fillSelect(item.control, key, options); + choose(item.control, wanted.get(key)); + if (source && folder && (item === source || item === folder)) followSource(wanted.get('folder')); + } + return { setOptions }; } /** One operation row; a press on its busy (aria-disabled) action is ignored and keeps the dialog open. */