core, app: a value the queue would refuse, caught before it is queued
ComfyUI refuses the WHOLE graph over one wrong value. v4 hit that with a LoRA file that had been deleted: a feature that was switched off still stopped every generation, and the message was "Prompt outputs failed validation" over a dozen loaders. build() checked wiring and required-ness and never looked at a literal, so check() called such a pipeline queueable. A combo value that is not among the node choices, and a number outside the bounds it declares, are now refused here -- naming the slot and the input. No bounds means unknown, and unknown is not a refusal. And when the queue does refuse one, its per-node reasons were captured by run.js and never read: the status bar showed only the top-level string, because describe() read `node` and the refusal path sets `nodes`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5f0247405c
commit
66e4bce345
@@ -94,3 +94,56 @@ test("every phase says something, including ones added later", () => {
|
||||
assert.ok(text && text.trim(), `${phase} draws an empty status`);
|
||||
}
|
||||
});
|
||||
|
||||
// --- a refusal from the queue, which names the nodes ------------------------
|
||||
|
||||
test("a graph the queue refused says which node and why", () => {
|
||||
// run.js captured ComfyUI's node_errors from the start; describe() only ever
|
||||
// read the singular `node`, which is set by the WebSocket crash path and
|
||||
// never by this one. So every value refusal showed ComfyUI's top-level
|
||||
// string -- "Prompt outputs failed validation" -- over a pipeline that can
|
||||
// hold a dozen loaders, with nothing saying which to fix.
|
||||
const said = describe({
|
||||
phase: FAILED, images: [],
|
||||
error: {
|
||||
message: "Prompt outputs failed validation",
|
||||
nodes: {
|
||||
vae: {
|
||||
class_type: "FunPackVAELoader",
|
||||
errors: [{ type: "value_not_in_list", message: "Value not in list",
|
||||
details: "vae_name: 'gone.safetensors' not in []" }],
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
assert.match(said, /FunPackVAELoader/);
|
||||
assert.match(said, /vae/);
|
||||
assert.match(said, /Value not in list/);
|
||||
assert.match(said, /gone\.safetensors/);
|
||||
});
|
||||
|
||||
test("a crash during execution still names its one node", () => {
|
||||
const said = describe({
|
||||
phase: FAILED, images: [],
|
||||
error: { node: "sampler", message: "shape mismatch" },
|
||||
});
|
||||
assert.equal(said, "sampler: shape mismatch");
|
||||
});
|
||||
|
||||
test("a refusal with no node errors falls back to what it did say", () => {
|
||||
const said = describe({
|
||||
phase: FAILED, images: [],
|
||||
error: { message: "the queue could not be reached: fetch failed", nodes: {} },
|
||||
});
|
||||
assert.match(said, /could not be reached/);
|
||||
});
|
||||
|
||||
test("a node error carrying nothing readable does not swallow the message", () => {
|
||||
// An entry with an empty errors array would otherwise return "" and read as
|
||||
// a run that failed for no reason at all.
|
||||
const said = describe({
|
||||
phase: FAILED, images: [],
|
||||
error: { message: "refused", nodes: { "4": { class_type: "X", errors: [] } } },
|
||||
});
|
||||
assert.equal(said, "refused");
|
||||
});
|
||||
|
||||
+39
-4
@@ -96,6 +96,43 @@ export function createTransport({ onGenerate, onCancel } = {}) {
|
||||
return { node: bar.node, draw, say, hold, release, generate, cancel, progress, status };
|
||||
}
|
||||
|
||||
/**
|
||||
* Why a run failed, in one line, from whichever of the two shapes it arrived in.
|
||||
*
|
||||
* A crash during execution names one node -- `node`. A graph the queue refused
|
||||
* names as many as were wrong -- `nodes`, an object keyed by node id, each with
|
||||
* its own reasons. Only the first was ever read, so the whole of the second was
|
||||
* captured and thrown away: the bar showed ComfyUI's top-level string, which is
|
||||
* "Prompt outputs failed validation" and says nothing about which of a dozen
|
||||
* loaders to go and fix.
|
||||
*/
|
||||
export function failure(error) {
|
||||
if (!error) return "the run failed";
|
||||
|
||||
const first = firstNodeError(error.nodes);
|
||||
if (first) return first;
|
||||
const where = error.node ? `${error.node}: ` : "";
|
||||
return `${where}${error.message || "the run failed"}`;
|
||||
}
|
||||
|
||||
function firstNodeError(nodes) {
|
||||
if (!nodes || typeof nodes !== "object") return null;
|
||||
for (const [id, entry] of Object.entries(nodes)) {
|
||||
const reasons = entry && Array.isArray(entry.errors) ? entry.errors : [];
|
||||
for (const reason of reasons) {
|
||||
const what = reason && (reason.message || reason.type);
|
||||
if (!what) continue;
|
||||
// The class type as well as the id: a node id is a number in ComfyUI's
|
||||
// own graphs and a slot name in ours, and neither on its own is much to
|
||||
// go on when the message is "Value not in list".
|
||||
const which = entry.class_type ? `${entry.class_type} (${id})` : id;
|
||||
const detail = reason.details ? ` — ${reason.details}` : "";
|
||||
return `${which}: ${what}${detail}`;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/** One line saying where the run is. The only place these words are decided. */
|
||||
export function describe(state) {
|
||||
switch (state.phase) {
|
||||
@@ -114,10 +151,8 @@ export function describe(state) {
|
||||
return state.images.length
|
||||
? `Done — ${state.images.length} result${state.images.length === 1 ? "" : "s"}`
|
||||
: "Done, but nothing was saved";
|
||||
case FAILED: {
|
||||
const where = state.error && state.error.node ? `${state.error.node}: ` : "";
|
||||
return `${where}${(state.error && state.error.message) || "the run failed"}`;
|
||||
}
|
||||
case FAILED:
|
||||
return failure(state.error);
|
||||
case CANCELLED:
|
||||
return "Cancelled";
|
||||
default:
|
||||
|
||||
+63
-2
@@ -62,6 +62,15 @@ class Schemas:
|
||||
found = self.of(class_type)
|
||||
return list(found.get("required", [])) if found else []
|
||||
|
||||
def limits(self, class_type: str) -> Dict[str, dict]:
|
||||
"""What each input will ACCEPT: a combo's choices, a number's bounds.
|
||||
|
||||
Absent from a schema means unknown, which is checked as no limit --
|
||||
a schema that does not say is not a schema that says no.
|
||||
"""
|
||||
found = self.of(class_type)
|
||||
return dict(found.get("limits", {})) if found else {}
|
||||
|
||||
|
||||
def from_comfyui() -> Schemas:
|
||||
"""Schemas read from whatever ComfyUI has registered, FunPack's and everyone
|
||||
@@ -72,7 +81,7 @@ def from_comfyui() -> Schemas:
|
||||
if node is None:
|
||||
return None
|
||||
spec = node.INPUT_TYPES()
|
||||
inputs, required = {}, []
|
||||
inputs, required, limits = {}, [], {}
|
||||
for section in ("required", "optional"):
|
||||
for name, declared in (spec.get(section) or {}).items():
|
||||
kind, options = comfy_types.declared(declared)
|
||||
@@ -86,8 +95,19 @@ def from_comfyui() -> Schemas:
|
||||
# thing that says a slot is incomplete -- and a slot missing
|
||||
# a required input built clean and failed once queued.
|
||||
required.append(name)
|
||||
|
||||
bounds = {}
|
||||
if inputs[name] == comfy_types.COMBO:
|
||||
bounds["choices"] = (list(kind) if isinstance(kind, (list, tuple))
|
||||
else comfy_types.choices(options))
|
||||
for edge in ("min", "max"):
|
||||
if edge in options:
|
||||
bounds[edge] = options[edge]
|
||||
if bounds:
|
||||
limits[name] = bounds
|
||||
|
||||
return {"inputs": inputs, "outputs": list(node.RETURN_TYPES),
|
||||
"required": required}
|
||||
"required": required, "limits": limits}
|
||||
return Schemas(lookup)
|
||||
|
||||
|
||||
@@ -175,6 +195,7 @@ def build(slots: Sequence[dict], schemas: Optional[Schemas] = None) -> Tuple[dic
|
||||
continue
|
||||
|
||||
declared = schemas.inputs(class_type)
|
||||
limits = schemas.limits(class_type)
|
||||
inputs = {}
|
||||
for name, value in (slot.get("inputs") or {}).items():
|
||||
if name not in declared:
|
||||
@@ -200,6 +221,10 @@ def build(slots: Sequence[dict], schemas: Optional[Schemas] = None) -> Tuple[dic
|
||||
continue
|
||||
inputs[name] = [source, index]
|
||||
else:
|
||||
problem = unacceptable(name, value, limits.get(name))
|
||||
if problem:
|
||||
problems.append(f"{slot_id}.{problem}")
|
||||
continue
|
||||
inputs[name] = value
|
||||
for name in schemas.required(class_type):
|
||||
if name not in inputs:
|
||||
@@ -212,6 +237,42 @@ def build(slots: Sequence[dict], schemas: Optional[Schemas] = None) -> Tuple[dic
|
||||
return prompt, problems
|
||||
|
||||
|
||||
def unacceptable(name: str, value: Any, bounds: Optional[dict]) -> Optional[str]:
|
||||
"""Why this literal will not do, or None.
|
||||
|
||||
ComfyUI checks these too, at /prompt -- and it refuses the WHOLE graph when
|
||||
one value is wrong, which v4 hit with a LoRA file that had been deleted: a
|
||||
feature that was switched off still stopped every generation. Refusing here
|
||||
means the reason arrives naming the slot and the input, before anything is
|
||||
queued, instead of as "Prompt outputs failed validation".
|
||||
|
||||
No bounds means unknown, and unknown is not a refusal.
|
||||
"""
|
||||
if not bounds:
|
||||
return None
|
||||
|
||||
choices = bounds.get("choices")
|
||||
if choices is not None:
|
||||
# An empty list is a machine with no files of that kind, not a value
|
||||
# that was checked and rejected -- the slot being unfilled is what gets
|
||||
# reported, and saying "x is not one of: " helps nobody.
|
||||
if choices and value not in choices:
|
||||
offered = ", ".join(repr(c) for c in choices[:6])
|
||||
more = "" if len(choices) <= 6 else f", and {len(choices) - 6} more"
|
||||
return (f"{name} is {value!r}, which is not one of: {offered}{more}")
|
||||
return None
|
||||
|
||||
# Only a number has bounds, and only a number is compared to them. A bool is
|
||||
# an int in Python and would sail through a naive comparison as 0 or 1.
|
||||
if isinstance(value, bool) or not isinstance(value, (int, float)):
|
||||
return None
|
||||
if "min" in bounds and value < bounds["min"]:
|
||||
return f"{name} is {value}, below the smallest {bounds['min']} it takes"
|
||||
if "max" in bounds and value > bounds["max"]:
|
||||
return f"{name} is {value}, above the largest {bounds['max']} it takes"
|
||||
return None
|
||||
|
||||
|
||||
def cycles(prompt: Dict[str, dict]) -> List[str]:
|
||||
"""Slots that feed each other, named in the order the loop runs.
|
||||
|
||||
|
||||
@@ -240,3 +240,84 @@ def test_a_diamond_is_not_a_cycle():
|
||||
{"id": "join", "node": "TwoIn", "inputs": {"a": ["left", 0], "b": ["right", 0]}},
|
||||
], SCHEMAS)
|
||||
assert problems == []
|
||||
|
||||
|
||||
# --- a value ComfyUI would refuse -------------------------------------------
|
||||
#
|
||||
# ComfyUI checks these too, at /prompt, and it refuses the WHOLE graph when one
|
||||
# value is wrong. v4 hit exactly that with a LoRA file that had been deleted: a
|
||||
# feature that was switched OFF still stopped every generation, and the message
|
||||
# was "Prompt outputs failed validation" over a graph of a dozen loaders.
|
||||
|
||||
def _schemas_with_limits():
|
||||
return graph.Schemas(lambda ct: {
|
||||
"Loader": {
|
||||
"inputs": {"ckpt_name": "COMBO", "steps": "INT", "on": "BOOLEAN"},
|
||||
"outputs": ["MODEL"], "required": ["ckpt_name"],
|
||||
"limits": {"ckpt_name": {"choices": ["a.safetensors", "b.safetensors"]},
|
||||
"steps": {"min": 1, "max": 100}},
|
||||
},
|
||||
"Empty": {"inputs": {"pick": "COMBO"}, "outputs": [], "required": [],
|
||||
"limits": {"pick": {"choices": []}}},
|
||||
}.get(ct))
|
||||
|
||||
|
||||
def test_a_combo_value_that_is_no_longer_on_disk_is_refused_before_queueing():
|
||||
_prompt, problems = graph.build(
|
||||
[{"id": "model", "node": "Loader", "inputs": {"ckpt_name": "gone.safetensors"}}],
|
||||
_schemas_with_limits())
|
||||
assert any("gone.safetensors" in p and "not one of" in p for p in problems), problems
|
||||
assert any("model.ckpt_name" in p for p in problems), problems
|
||||
|
||||
|
||||
def test_a_combo_value_that_is_on_disk_is_accepted():
|
||||
prompt, problems = graph.build(
|
||||
[{"id": "model", "node": "Loader", "inputs": {"ckpt_name": "b.safetensors"}}],
|
||||
_schemas_with_limits())
|
||||
assert problems == []
|
||||
assert prompt["model"]["inputs"]["ckpt_name"] == "b.safetensors"
|
||||
|
||||
|
||||
def test_a_number_outside_what_the_node_takes_is_refused():
|
||||
_prompt, problems = graph.build(
|
||||
[{"id": "model", "node": "Loader",
|
||||
"inputs": {"ckpt_name": "a.safetensors", "steps": 0}}],
|
||||
_schemas_with_limits())
|
||||
assert any("below the smallest" in p for p in problems), problems
|
||||
|
||||
_prompt, problems = graph.build(
|
||||
[{"id": "model", "node": "Loader",
|
||||
"inputs": {"ckpt_name": "a.safetensors", "steps": 1000}}],
|
||||
_schemas_with_limits())
|
||||
assert any("above the largest" in p for p in problems), problems
|
||||
|
||||
|
||||
def test_a_bool_is_not_measured_against_a_number_bound():
|
||||
"""True is 1 in Python, so a naive comparison lets a checkbox be judged
|
||||
against a step count -- and quietly passes it."""
|
||||
prompt, problems = graph.build(
|
||||
[{"id": "model", "node": "Loader",
|
||||
"inputs": {"ckpt_name": "a.safetensors", "on": True}}],
|
||||
_schemas_with_limits())
|
||||
assert problems == []
|
||||
assert prompt["model"]["inputs"]["on"] is True
|
||||
|
||||
|
||||
def test_an_empty_choice_list_is_reported_as_unfilled_not_as_a_wrong_value():
|
||||
"""A machine with no files of that kind has not rejected anything. Saying
|
||||
"x is not one of: " helps nobody, and the slot being unfilled is already
|
||||
reported."""
|
||||
_prompt, problems = graph.build(
|
||||
[{"id": "e", "node": "Empty", "inputs": {"pick": "whatever"}}],
|
||||
_schemas_with_limits())
|
||||
assert not any("not one of" in p for p in problems), problems
|
||||
|
||||
|
||||
def test_a_schema_that_says_nothing_about_a_value_does_not_refuse_it():
|
||||
"""Most injected schemas carry no limits at all. Unknown is not a refusal:
|
||||
a schema that does not say is not a schema that says no."""
|
||||
schemas = graph.Schemas(lambda ct: {"inputs": {"anything": "STRING"},
|
||||
"outputs": [], "required": []})
|
||||
_prompt, problems = graph.build(
|
||||
[{"id": "x", "node": "Whatever", "inputs": {"anything": "hello"}}], schemas)
|
||||
assert problems == []
|
||||
|
||||
@@ -302,3 +302,48 @@ def test_every_dropdown_in_the_registry_is_offered_as_one(registered):
|
||||
if w["type"] == comfy_types.COMBO)
|
||||
|
||||
assert dropdowns > 100, f"only {dropdowns} dropdowns found; the sweep is not seeing them"
|
||||
|
||||
|
||||
def test_a_file_that_is_no_longer_there_is_refused_here_not_at_the_queue(server, registered):
|
||||
"""v4's failure, reproduced against real nodes.
|
||||
|
||||
A LoRA file that had been deleted made ComfyUI refuse the WHOLE prompt --
|
||||
a feature that was switched off still stopped every generation, and the
|
||||
message was "Prompt outputs failed validation" over a graph of a dozen
|
||||
loaders. ComfyUI checks this at /prompt, which is one step after our own
|
||||
check says the pipeline is queueable.
|
||||
"""
|
||||
_status, body = _request(server, "GET", "/funpack/api/pipeline")
|
||||
slots = body["slots"]
|
||||
for slot in slots:
|
||||
if slot["id"] == "model":
|
||||
slot["inputs"]["model_name"] = "deleted-yesterday.safetensors"
|
||||
|
||||
_status, checked = _request(server, "POST", "/funpack/api/pipeline",
|
||||
{"action": "check", "slots": slots})
|
||||
assert checked["queueable"] is False
|
||||
assert any("deleted-yesterday.safetensors" in problem and "not one of" in problem
|
||||
for problem in checked["incomplete"]), checked["incomplete"]
|
||||
assert checked["prompt"] is None, "a graph the queue would refuse was handed over"
|
||||
|
||||
|
||||
def test_a_value_the_node_does_offer_still_builds(server, registered):
|
||||
"""The control. Without it the test above passes on a check that refuses
|
||||
everything."""
|
||||
from core import widgets
|
||||
|
||||
_status, body = _request(server, "GET", "/funpack/api/pipeline")
|
||||
slots = body["slots"]
|
||||
described = widgets.describe("FunPackDiffusionModelLoader")
|
||||
choices = next(w for w in described["widgets"] if w["name"] == "model_name")["choices"]
|
||||
if not choices:
|
||||
pytest.skip("no model files on this machine, so there is no valid value to use")
|
||||
|
||||
for slot in slots:
|
||||
if slot["id"] == "model":
|
||||
slot["inputs"]["model_name"] = choices[0]
|
||||
|
||||
_status, checked = _request(server, "POST", "/funpack/api/pipeline",
|
||||
{"action": "check", "slots": slots})
|
||||
assert not any("not one of" in problem for problem in checked["incomplete"]), \
|
||||
checked["incomplete"]
|
||||
|
||||
Reference in New Issue
Block a user