Defaults an administrator can actually set
There were none. `workflow.DEFAULTS` was the only source, so 512x512, euler and twenty steps were what every instance got whatever card it was running on -- and 512 square on an SDXL checkpoint is precisely what the tool's own description warns produces duplicated limbs. The two ways round it were both bad: bake literals into a template where the placeholders should be, or write prose in the instructions box and hope. Three rungs now, most specific winning, with DEFAULTS staying underneath as the floor so an instance that sets nothing behaves exactly as it did and a floor improved in code still reaches everybody. An empty box is "no opinion" rather than zero, which matters: read as a number it would set every instance to zero steps, and ComfyUI refuses that in a way that looks like a broken model. The right control for each, because a text box is wrong for most of them. The samplers and schedulers were already being discovered by the Test button, stored, and read by nothing at all -- they are the pickers now. A stored value missing from the list is kept as an option anyway, or opening this page and pressing Save would silently clear a working setting. Checkpoints are chosen rather than typed, and the instance default is a rung of its own instead of "whatever happens to be first in a textarea somebody filled in some order". And batch, at last: `batch_size` was a literal 1 in the base template, so an administrator whose card can comfortably make four had no way of saying so. Deliberately not something a model may set -- one asking for six because it is unsure is the exact cost this must not invite. The tool's schema restates the defaults it quotes. Every "Default 20." in there was written when there was one set of defaults in the world; left alone, an instance drawing at 1024 would go on telling the model 512, and the model reasons from that sentence rather than ignoring it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -141,3 +141,85 @@ def test_nonsense_falls_back_instead_of_raising():
|
||||
|
||||
def test_a_seed_larger_than_comfyui_allows_is_wrapped():
|
||||
assert 0 <= wf.resolve({"prompt": "x", "seed": wf.MAX_SEED + 5})["seed"] <= wf.MAX_SEED
|
||||
|
||||
|
||||
# --- Instance defaults: the rung that did not exist -----------------------------
|
||||
def test_an_instance_default_beats_the_built_in_floor():
|
||||
"""For the whole life of this feature there were two rungs, so 512x512 and
|
||||
twenty steps were what every instance got whatever card it was running on.
|
||||
The only ways to move them were to bake literals into a template instead of
|
||||
placeholders, or to write prose in the instructions box and hope."""
|
||||
values = wf.resolve({}, settings={"default_width": 1024, "default_steps": 30})
|
||||
|
||||
assert values["width"] == 1024
|
||||
assert values["steps"] == 30
|
||||
# And everything unset still falls through.
|
||||
assert values["height"] == wf.DEFAULTS["height"]
|
||||
assert values["cfg"] == wf.DEFAULTS["cfg"]
|
||||
|
||||
|
||||
def test_a_model_still_beats_an_instance_default():
|
||||
"""Most specific wins, which is the same ladder the checkpoint and the
|
||||
workflow already follow."""
|
||||
values = wf.resolve({"width": 768}, settings={"default_width": 1024})
|
||||
assert values["width"] == 768
|
||||
|
||||
|
||||
def test_an_empty_setting_is_no_opinion_rather_than_zero():
|
||||
"""The empty string is how an administrator says "leave this alone". Read as
|
||||
a number it would set every instance to zero steps, and a zero-step
|
||||
generation is a refusal from ComfyUI that looks like a broken model."""
|
||||
values = wf.resolve({}, settings={"default_steps": "", "default_cfg": None})
|
||||
|
||||
assert values["steps"] == wf.DEFAULTS["steps"]
|
||||
assert values["cfg"] == wf.DEFAULTS["cfg"]
|
||||
|
||||
|
||||
def test_an_instance_default_is_clamped_like_any_other():
|
||||
"""A number written straight into the settings row, or stored by an earlier
|
||||
version, still has to be safe when a generation reads it."""
|
||||
values = wf.resolve({}, settings={"default_steps": 9000, "default_width": 4})
|
||||
|
||||
assert values["steps"] == wf.LIMITS["steps"][1]
|
||||
assert values["width"] == wf.LIMITS["width"][0]
|
||||
|
||||
|
||||
def test_no_settings_at_all_behaves_exactly_as_before():
|
||||
"""The whole safety of adding a rung: an instance that sets nothing is
|
||||
unchanged.
|
||||
|
||||
Everything but the seed, which is deliberately fresh on every call — two
|
||||
resolves that agreed about it would mean the retry loop produced the same
|
||||
rejected image four times over.
|
||||
"""
|
||||
without = wf.resolve({}, settings=None)
|
||||
plain = wf.resolve({})
|
||||
del without["seed"], plain["seed"]
|
||||
|
||||
assert without == plain
|
||||
|
||||
|
||||
# --- Batch ----------------------------------------------------------------------
|
||||
def test_batch_fills_from_the_instance_and_not_from_the_model():
|
||||
"""`batch_size` was a literal 1 in the base template, so an administrator
|
||||
whose card can comfortably make four at a time had no way of saying so short
|
||||
of editing JSON. It is deliberately not a tool parameter: a model asking for
|
||||
six because it is unsure is exactly the cost this must not invite."""
|
||||
assert "batch" not in wf.MODEL_SETTABLE
|
||||
assert wf.resolve({}, settings={"default_batch": 4})["batch"] == 4
|
||||
# A model that invents the key is ignored rather than refused.
|
||||
assert wf.resolve({"batch": 6}, settings={"default_batch": 2})["batch"] == 2
|
||||
|
||||
|
||||
def test_the_base_template_takes_the_batch_placeholder():
|
||||
"""A placeholder nothing references is a setting that silently does
|
||||
nothing."""
|
||||
assert "batch" in wf.placeholders_in(BASE)
|
||||
|
||||
|
||||
def test_every_placeholder_is_described():
|
||||
"""The editor's legend is built from this table, so a placeholder added
|
||||
without one appears in the list with a blank beside it."""
|
||||
for name in wf.PLACEHOLDERS:
|
||||
assert name in wf.DESCRIPTIONS, name
|
||||
assert wf.DESCRIPTIONS[name][1].strip(), name
|
||||
|
||||
Reference in New Issue
Block a user