diff --git a/tests/js/comfy-metadata.test.js b/tests/js/comfy-metadata.test.js index 5a687c0..efe004d 100644 --- a/tests/js/comfy-metadata.test.js +++ b/tests/js/comfy-metadata.test.js @@ -202,10 +202,8 @@ test("facts use resolution.sampler_id for detailer-stage nodes", () => { seed: ["known", "42"], steps: ["ambiguous", null], cfg: ["ambiguous", null], sampler: ["known", "euler"], scheduler: ["known", "normal"], model: ["known", "model.safetensors"], }); - // Without a named sampler, two samplers stay ambiguous (today's rules, now over the samplers table). - for (const resolution of [undefined, { state: "unresolved", sampler_id: "d" }]) { - assert.ok(Object.values(factsFor(detailerFactGraph(), { resolution })).every(fact => fact.status === "ambiguous"), JSON.stringify(resolution)); - } + // Without a resolution, two samplers stay ambiguous (today's rules, now over the samplers table). + assert.ok(Object.values(factsFor(detailerFactGraph())).every(fact => fact.status === "ambiguous")); // A sampler_id the graph or the samplers table does not hold never falls back to a guess. for (const sampler_id of ["missing", "decode", "model", "__proto__"]) { for (const state of ["resolved", "ambiguous"]) { @@ -276,6 +274,50 @@ test("stage walks fail on unlisted sampler-shaped nodes like the resolver", () = } }); +test("unresolved and absent results over several stages merge facts per field", () => { + // Review I-1: the run's settings are facts even when its prompt is not, e.g. an + // upstream prompt behind an unsupported node (LoRAfromText) leaves the server unresolved. + const merged = { + seed: ["ambiguous", null], steps: ["ambiguous", null], cfg: ["known", "6.5"], + sampler: ["known", "euler"], scheduler: ["known", "normal"], model: ["known", "model.safetensors"], + }; + for (const resolution of [ + { state: "unresolved", sampler_id: "d", reason_code: "unsupported-node" }, + { state: "unresolved", sampler_id: null, reason_code: "malformed" }, + { state: "absent", sampler_id: "d" }, + ]) { + const facts = factsFor(detailerFactGraph(), { resolution }); + assert.deepEqual(statusValues(facts), merged, JSON.stringify(resolution)); + assert.ok(factFields.every(field => facts[field].sampler_id === null)); + } + // A stage missing a field leaves it unresolved; a failed walk leaves everything unresolved. + const unrecorded = detailerFactGraph(); delete unrecorded.d.inputs.scheduler; + assert.equal(factsFor(unrecorded, { resolution: { state: "unresolved", sampler_id: "d" } }).scheduler.status, "unresolved"); + const cyclic = detailerFactGraph(); cyclic.decode.inputs.samples = ["d", 0]; + assert.ok(Object.values(factsFor(cyclic, { resolution: { state: "unresolved", sampler_id: "d" } })).every(fact => fact.status === "unresolved")); + // One listed sampler keeps today's rule: its own facts, attributed to it. + const single = factsFor(factGraph(), { resolution: { state: "unresolved", sampler_id: "s" } }); + assert.ok(factFields.every(field => single[field].status === "known" && single[field].sampler_id === "s")); +}); + +test("stage walks follow only the resolver's links", () => { + // Review M-2: `["eff", -1]` is not a link to the server, so it is not walked. + for (const slot of [-1, 0.5, "0"]) { + const graph = detailerFactGraph(); + graph.eff = { class_type: "KSampler (Efficient)", inputs: { positive: ["model", 0], model: ["model", 0] } }; + graph.decode.inputs.extra = ["eff", slot]; + assert.equal(factsFor(graph, { resolution: { state: "ambiguous", sampler_id: "d" } }).seed.status, "ambiguous", String(slot)); + const shaped = detailerFactGraph(); + shaped.eff = { class_type: "Unlisted", inputs: { positive: ["model", slot], image: ["decode", 0] } }; + shaped.d.inputs.image = ["eff", 0]; + assert.equal(factsFor(shaped, { resolution: { state: "ambiguous", sampler_id: "d" } }).seed.status, "ambiguous", `shaped ${slot}`); + } + const linked = detailerFactGraph(); + linked.eff = { class_type: "KSampler (Efficient)", inputs: { positive: ["model", 0], model: ["model", 0] } }; + linked.decode.inputs.extra = ["eff", 0]; + assert.equal(factsFor(linked, { resolution: { state: "ambiguous", sampler_id: "d" } }).seed.status, "unresolved"); +}); + test("Flux2Scheduler provides steps and scheduler is its node type", () => { const graph = customFactGraph(); graph.sigmas = { class_type: "Flux2Scheduler", inputs: { steps: 4, width: 1024, height: 1024 } }; diff --git a/tests/js/inspection-semantics-sync.test.js b/tests/js/inspection-semantics-sync.test.js index 6d1cd5c..86d5808 100644 --- a/tests/js/inspection-semantics-sync.test.js +++ b/tests/js/inspection-semantics-sync.test.js @@ -58,3 +58,18 @@ test("stage walk table classes equal every class in the semantics table", () => for (const category of ["runtime", "non_file_outputs"]) semantics[category].forEach(name => classes.add(name)); assert.deepStrictEqual([...metadata._tableClasses].sort(), [...classes].sort()); }); + +// Review I-2: one source for the stage-walk input lists (Python reads them there too). +test("stage walk inputs equal semantics.stage_inputs", () => { + assert.equal(typeof semantics.stage_inputs, "object"); + assert.equal(typeof metadata._stageInputs, "object", "comfy-metadata exposes its stage inputs for this test"); + assert.deepStrictEqual(plain(metadata._stageInputs), semantics.stage_inputs); + assert.ok(Object.isFrozen(metadata._stageInputs)); +}); + +// Review M-3: JS treats every samplers entry as a candidate; the server only these strategies. +test("every samplers entry is a candidate strategy", () => { + for (const [name, entry] of Object.entries(semantics.samplers)) { + assert.ok(["ksampler", "custom-advanced", "detailer-stage"].includes(entry.strategy), name); + } +}); diff --git a/tests/test_inspection_semantics.py b/tests/test_inspection_semantics.py index 56a7ed4..583fa2c 100644 --- a/tests/test_inspection_semantics.py +++ b/tests/test_inspection_semantics.py @@ -55,6 +55,7 @@ def _minimal(): "runtime": [], "savers": {}, "non_file_outputs": [], + "stage_inputs": {"shaped": ["positive"], "with_model": ["samples"]}, "facts": { "configuration_outputs": {}, "model_outputs": {}, @@ -309,6 +310,35 @@ class SpecEntriesTest(unittest.TestCase): for name in ("PromptManager", "PromptManagerPair", "PromptManagerText"): self.assertNotIn(name, table.all_classes) + def test_stage_inputs_are_the_one_source_for_both_stage_walks(self): + # Review I-2: Python's resolver and comfy-metadata.js read these lists here. + stage_inputs = load_semantics().stage_inputs + self.assertEqual( + stage_inputs["shaped"], + ( + "latent_image", + "guider", + "positive", + "negative", + "pipe", + "basic_pipe", + "detailer_pipe", + "sampler", + "sigmas", + "noise", + "text_embeds", + "hyvid_embeds", + ), + ) + self.assertEqual( + stage_inputs["with_model"], + ("conditioning", "neg_conditioning", "samples", "latent"), + ) + # The default profile's own sampler-shape names stay a subset of the table's. + from utils.prompt_pair_resolver import _SAMPLER_LIKE_INPUTS + + self.assertLessEqual(set(_SAMPLER_LIKE_INPUTS), set(stage_inputs["shaped"])) + def test_facts_section_mirrors_the_browser_tables(self): facts = load_semantics().facts self.assertEqual( @@ -492,6 +522,13 @@ class ValidationTest(_TableCase): lambda d: d["facts"]["scalar_outputs"].update(PrimitiveInt="int"), "facts", ), + (lambda d: d.pop("stage_inputs"), "category"), + (lambda d: d["stage_inputs"].update(extra=["x"]), "stage_inputs"), + (lambda d: d["stage_inputs"].pop("with_model"), "stage_inputs"), + (lambda d: d["stage_inputs"].update(shaped=[]), "input name"), + (lambda d: d["stage_inputs"].update(shaped="pipe"), "input name"), + (lambda d: d["stage_inputs"].update(with_model=["positive"]), "both"), + (lambda d: d["stage_inputs"].update(shaped=["model"]), "model"), ( lambda d: d["facts"]["configuration_outputs"].update( X={"kind": "sigmas", "fields": ["steps"], "type_fields": []} diff --git a/tests/test_prompt_pair_resolver.py b/tests/test_prompt_pair_resolver.py index 6f5fd89..fdc5166 100644 --- a/tests/test_prompt_pair_resolver.py +++ b/tests/test_prompt_pair_resolver.py @@ -1,6 +1,7 @@ """Conservative prompt-pair resolution contracts, independent of ComfyUI.""" import collections +import json import random import subprocess # nosec B404 - fixed interpreter command, no shell import sys @@ -3124,6 +3125,37 @@ class TestSemanticsIsolation(unittest.TestCase): finally: inspection_semantics.get_semantics.cache_clear() + def test_stage_shaped_inputs_come_from_the_table(self): + # Review I-2: the stage-walk input lists live in the semantics table, so + # a key added there changes the inspection walk and never the default one. + source = _latent_from(None) + source.update( + { + "30": {"class_type": "MysteryEmbeds", "inputs": {}}, + "31": {"class_type": "MysteryStage", "inputs": {"stage_x": ["30", 0]}}, + "32": { + "class_type": "VAEDecode", + "inputs": {"samples": ["31", 0], "vae": ["4", 2]}, + }, + "23": { + "class_type": "VAEEncode", + "inputs": {"pixels": ["32", 0], "vae": ["4", 2]}, + }, + } + ) + source["3"]["inputs"]["latent_image"] = ["23", 0] + table = json.loads( + (REPO / "web" / "js" / "inspection-semantics.json").read_text("utf-8") + ) + plain_default, plain = self.resolve_with_table(json.dumps(table), source) + table["stage_inputs"]["shaped"].append("stage_x") + default, inspected = self.resolve_with_table(json.dumps(table), source) + self.assertEqual(plain["state"], "resolved") + self.assertEqual(inspected["state"], "unresolved") + self.assertEqual(inspected["reason_code"], "unsupported-node") + self.assertEqual(inspected["node"], {"id": "31", "class_type": "MysteryStage"}) + self.assertEqual(default, plain_default) + def test_corrupt_table_leaves_default_resolution_working(self): for text in ("{not json", "[" * 100_000, "{}"): with self.subTest(text=text[:10]): diff --git a/tools/vendor/frontend-coverage-baseline.json b/tools/vendor/frontend-coverage-baseline.json index 71e23ad..25d68a5 100644 --- a/tools/vendor/frontend-coverage-baseline.json +++ b/tools/vendor/frontend-coverage-baseline.json @@ -19,7 +19,7 @@ }, "comfy-metadata.js": { "lines": 97, - "branches": 91, + "branches": 92, "functions": 98, "statements": 97 }, diff --git a/utils/inspection_semantics.py b/utils/inspection_semantics.py index 45dff31..24ba0c6 100644 --- a/utils/inspection_semantics.py +++ b/utils/inspection_semantics.py @@ -67,8 +67,19 @@ _FACTS_TABLES = ( "scalar_outputs", ) _CATEGORIES = frozenset( - ("schema",) + _CLASS_CATEGORIES + _SLOT_CATEGORIES + _SET_CATEGORIES + ("facts",) + ("schema",) + + _CLASS_CATEGORIES + + _SLOT_CATEGORIES + + _SET_CATEGORIES + + ("stage_inputs", "facts") ) +# Input names that make an unlisted node a possible sampler stage in an ancestry +# walk (``shaped``: pipes, video embeds and custom-sampler parts carry a prompt +# without a ``positive`` link), and those that do so beside a linked ``model`` +# (``with_model``: XlabsSampler's ``conditioning``, video samplers' ``samples``). +# ``model`` alone (ModelSampling*) or with ``clip`` (LoraLoader) is a model patch. +# The resolver and comfy-metadata.js both read these lists. +_STAGE_INPUT_LISTS = ("shaped", "with_model") _STRING_PARAMS = MappingProxyType( { "literal": frozenset(), @@ -99,6 +110,7 @@ class Semantics: runtime: FrozenSet[str] savers: Mapping[str, Mapping[str, Any]] non_file_outputs: FrozenSet[str] + stage_inputs: Mapping[str, Tuple[str, ...]] facts: Mapping[str, Any] all_classes: FrozenSet[str] @@ -351,6 +363,21 @@ def _string_map(table: Any, where: str, allowed: Optional[FrozenSet[str]]) -> No raise SemanticsError(f"{where}[{name!r}]: unknown kind {value!r}") +def _stage_inputs(doc: dict) -> Mapping[str, Tuple[str, ...]]: + table = doc["stage_inputs"] + if not isinstance(table, dict) or set(table) != set(_STAGE_INPUT_LISTS): + raise SemanticsError( + f"stage_inputs must have exactly {list(_STAGE_INPUT_LISTS)}" + ) + for key in _STAGE_INPUT_LISTS: + _names(table[key], f"stage_inputs.{key}") + if "model" in table[key]: + raise SemanticsError(f"stage_inputs.{key}: model is a patch input") + if set(table["shaped"]) & set(table["with_model"]): + raise SemanticsError("stage_inputs: an input is listed in both lists") + return _freeze(table) + + def _facts(doc: dict) -> Mapping[str, Any]: facts = doc["facts"] if not isinstance(facts, dict) or set(facts) != set(_FACTS_TABLES): @@ -407,7 +434,12 @@ def _validate(doc: Any) -> Semantics: tables[category] = _slot_category(doc, category, owners) for category in _SET_CATEGORIES: tables[category] = _set_category(doc, category, owners) - return Semantics(facts=_facts(doc), all_classes=owners.classes(), **tables) + return Semantics( + stage_inputs=_stage_inputs(doc), + facts=_facts(doc), + all_classes=owners.classes(), + **tables, + ) def load_semantics(path: Optional[Union[str, Path]] = None) -> Semantics: diff --git a/utils/prompt_pair_resolver.py b/utils/prompt_pair_resolver.py index a9c12b1..b6bef3e 100644 --- a/utils/prompt_pair_resolver.py +++ b/utils/prompt_pair_resolver.py @@ -157,23 +157,8 @@ _UNVERIFIABLE_CODES = frozenset({"unsupported-node", "runtime-value"}) # JoinStringMulti's own ``inputcount`` range; anything else never executed. _MAX_JOIN_INPUTS = 1000 _SAMPLER_LIKE_INPUTS = ("latent_image", "guider", "positive", "negative") -# Inputs that make an unlisted node a possible sampler stage in an ancestry walk: -# pipes (easy-use, Impact), video embeds and custom-sampler parts carry a prompt -# without a ``positive`` link. ``model`` is not one (LoRA loaders are walked). -_STAGE_SHAPED_INPUTS = _SAMPLER_LIKE_INPUTS + ( - "pipe", - "basic_pipe", - "detailer_pipe", - "sampler", - "sigmas", - "noise", - "text_embeds", - "hyvid_embeds", -) -# With a linked ``model``, any of these makes an unlisted node a possible stage -# (XlabsSampler's ``conditioning``, video samplers' ``samples``). ``model`` alone -# (ModelSampling*) or with ``clip`` (LoraLoader) is a model patch. -_MODEL_STAGE_INPUTS = ("conditioning", "neg_conditioning", "samples", "latent") +# The stage-walk input lists (``stage_inputs``) live in the semantics table; the +# default profile never reads them. _NO_SAMPLER_NAMES = 3 _CLASS_NAME_LENGTH = 64 @@ -1037,10 +1022,11 @@ class _GraphResolver: """An unlisted node that may itself be a sampler stage.""" if name in self.semantics.all_classes: return False - if any(_link(inputs.get(key)) for key in _STAGE_SHAPED_INPUTS): + stage_inputs = self.semantics.stage_inputs + if any(_link(inputs.get(key)) for key in stage_inputs["shaped"]): return True return _link(inputs.get("model")) and any( - _link(inputs.get(key)) for key in _MODEL_STAGE_INPUTS + _link(inputs.get(key)) for key in stage_inputs["with_model"] ) def upstream_stages(self, origin: str, value: Any) -> list: diff --git a/web/js/comfy-metadata.js b/web/js/comfy-metadata.js index 4e5c525..f345075 100644 --- a/web/js/comfy-metadata.js +++ b/web/js/comfy-metadata.js @@ -415,7 +415,9 @@ SamplerCustomAdvanced: { facts: "custom-advanced" }, FaceDetailer: { facts: "ksampler", image_stage: "image" }, }); - // Stages merged for one ambiguous result: the resolver's candidate cap. + // Stages merged for one result: the resolver's candidate cap. Together with + // MAX_VISITS per candidate walk these are deliberately tighter than the server's + // budgets (2,000 visits, depth 64), so the JS walk always fails closed first. const MAX_STAGES = 32; // Every class the semantics table lists (the resolver's `all_classes`): a stage walk // passes these, and treats any other node shaped like a sampler as an unknown stage. @@ -431,10 +433,12 @@ "JoinStringMulti", "PromptSearchList", "PreviewImage", "PreviewAny", "SaveImageWebsocket", "Image Comparer (rgthree)", ]); - // The resolver's stage-shaped inputs (pipes, embeds, custom-sampler parts), and the - // ones that make a node with a linked `model` a stage rather than a model patch. - const STAGE_SHAPED_INPUTS = ["latent_image", "guider", "positive", "negative", "pipe", "basic_pipe", "detailer_pipe", "sampler", "sigmas", "noise", "text_embeds", "hyvid_embeds"]; - const MODEL_STAGE_INPUTS = ["conditioning", "neg_conditioning", "samples", "latent"]; + // The table's `stage_inputs` (the resolver reads the same lists): inputs that make an + // unlisted node a possible stage, and those that do so beside a linked `model`. + const STAGE_INPUTS = Object.freeze({ + shaped: ["latent_image", "guider", "positive", "negative", "pipe", "basic_pipe", "detailer_pipe", "sampler", "sigmas", "noise", "text_embeds", "hyvid_embeds"], + with_model: ["conditioning", "neg_conditioning", "samples", "latent"], + }); // Fixed output0 contracts from the pinned core node schemas, not input-name inference. const SCALAR_OUTPUTS = Object.freeze({ PrimitiveInt: "number", PrimitiveFloat: "number", @@ -505,14 +509,18 @@ const samplerEntry = (graph, id) => (Object.hasOwn(graph, id) && Object.hasOwn(SAMPLERS, graph[id].class_type) ? SAMPLERS[graph[id].class_type] : undefined); - const linked = (inputs, key) => Object.hasOwn(inputs, key) && isLink(inputs[key]); + // The resolver's `_link`: an integer or string id and a non-negative integer slot. + // Stage walks use it so they cross exactly the server's edges; the legacy `isLink` stays. + const stageLink = value => Array.isArray(value) && value.length === 2 && + (typeof value[0] === "string" || Number.isInteger(value[0])) && Number.isInteger(value[1]) && value[1] >= 0; + const linked = (inputs, key) => Object.hasOwn(inputs, key) && stageLink(inputs[key]); /** An unlisted node that may itself be a sampler stage: the walk cannot judge it. */ const stageShaped = ({ class_type, inputs }) => !TABLE_CLASSES.has(class_type) && - (STAGE_SHAPED_INPUTS.some(key => linked(inputs, key)) || (linked(inputs, "model") && MODEL_STAGE_INPUTS.some(key => linked(inputs, key)))); + (STAGE_INPUTS.shaped.some(key => linked(inputs, key)) || (linked(inputs, "model") && STAGE_INPUTS.with_model.some(key => linked(inputs, key)))); /** Samplers reached from a detailer's image input, breadth-first, stopping at listed samplers. */ function upstreamStages(graph, origin, value, budget) { - if (!isLink(value)) return null; + if (!stageLink(value)) return null; const found = [], queue = [String(value[0])], queued = new Set(queue); for (let head = 0; head < queue.length; head++) { const id = queue[head]; @@ -520,7 +528,7 @@ if (samplerEntry(graph, id)) { found.push(id); continue; } if (stageShaped(graph[id])) return null; for (const input of Object.values(graph[id].inputs)) { - const next = isLink(input) ? String(input[0]) : null; + const next = stageLink(input) ? String(input[0]) : null; if (next !== null && !queued.has(next)) { queued.add(next); queue.push(next); } } } @@ -533,6 +541,8 @@ * below this one, so a cyclic chain fails instead of recursing. False when the chain * cannot be walked within the bounds. */ + // `stages` is shared on purpose across one result's candidates: re-adding a stage is + // harmless, and MAX_STAGES caps the union, not each candidate. function collectStages(graph, id, later, stages, budget) { stages.add(id); if (stages.size > MAX_STAGES || later.size >= MAX_STAGES) return false; @@ -597,9 +607,10 @@ * Facts follow the server's resolution. Its `sampler_id` drives them only when * resolved, or for an ambiguous result naming the final stage of a multistage * chain (merged per field across the stages, independent of any selection: D5). - * Ambiguous without a named sampler (no candidates on the wire until PR3) merges - * the same way across every listed sampler and its stages. Otherwise exactly one - * listed sampler in the graph supplies them; several leave every field ambiguous. + * Ambiguous without a named sampler (no candidates on the wire 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. */ function graphFacts(graph, source, resolution) { if (!object(graph)) return factSet("unresolved", source); @@ -618,7 +629,9 @@ return mergeFacts([...stages].map(id => samplerFacts(ctx, id, source)), source); } const candidates = entries.filter(([id]) => samplerEntry(graph, id)); - if (state === "ambiguous") { + // 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)) { // 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); @@ -704,6 +717,7 @@ }); Object.defineProperty(api, "_samplerTable", { value: SAMPLERS }); Object.defineProperty(api, "_tableClasses", { value: TABLE_CLASSES }); + Object.defineProperty(api, "_stageInputs", { value: STAGE_INPUTS }); if (typeof module !== "undefined" && module.exports) module.exports = api; else root.ComfyMetadata = api; })(typeof window !== "undefined" ? window : globalThis); diff --git a/web/js/inspection-semantics.json b/web/js/inspection-semantics.json index 38102b7..d16dfdf 100644 --- a/web/js/inspection-semantics.json +++ b/web/js/inspection-semantics.json @@ -144,6 +144,23 @@ } }, "non_file_outputs": ["PreviewImage", "PreviewAny", "SaveImageWebsocket", "Image Comparer (rgthree)"], + "stage_inputs": { + "shaped": [ + "latent_image", + "guider", + "positive", + "negative", + "pipe", + "basic_pipe", + "detailer_pipe", + "sampler", + "sigmas", + "noise", + "text_embeds", + "hyvid_embeds" + ], + "with_model": ["conditioning", "neg_conditioning", "samples", "latent"] + }, "facts": { "configuration_outputs": { "RandomNoise": { "kind": "noise", "fields": ["noise_seed"] },