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:
Jaroslav Beneš
2026-08-06 11:50:37 +02:00
parent fb54a236ae
commit 6fcb9c9892
16 changed files with 784 additions and 39 deletions
+121
View File
@@ -270,3 +270,124 @@ def test_only_an_administrator_can_reach_it(client: TestClient, db, registered):
assert client.get("/admin/images").status_code in (302, 303, 403, 404)
assert client.post("/admin/images/workflows", data={}).status_code in (302, 303, 403, 404)
# --- The defaults card ----------------------------------------------------------
def test_the_defaults_are_saved_and_clamped(client, db, registered):
"""Clamped at the save as well as on the way out. A number stored by an
earlier version, or written straight into the settings row, still has to be
safe when a generation reads it."""
from lembas.services import settings_store
client.post(
"/admin/images",
data={
"base_url": "http://comfy.test",
"api_key": "",
"timeout": "600",
"checkpoints": "sd.safetensors",
"default_steps": "9000",
"default_width": "1024",
"default_cfg": "5.5",
"default_sampler": "dpmpp_2m",
"default_batch": "4",
},
follow_redirects=False,
)
values = settings_store.images(db)
assert values["default_steps"] == 150 # the ceiling, not 9000
assert values["default_width"] == 1024
assert values["default_cfg"] == 5.5
assert values["default_sampler"] == "dpmpp_2m"
assert values["default_batch"] == 4
def test_an_empty_default_stays_empty_rather_than_becoming_zero(client, db, registered):
"""The empty string is how an administrator says "no opinion", which
`workflow.resolve` reads as "fall through to the built-in". Read as a number
it would set the instance to zero steps."""
from lembas.services import settings_store
client.post(
"/admin/images",
data={"base_url": "http://comfy.test", "api_key": "", "timeout": "600"},
follow_redirects=False,
)
values = settings_store.images(db)
assert values["default_steps"] == ""
assert values["default_cfg"] == ""
def test_saving_the_page_does_not_clear_the_discovered_lists(client, db, registered):
"""They belong to whatever ComfyUI was tested, and a save that only changed
the instructions box has no opinion about them. The sampler picker is built
from them, so clearing them would empty it."""
from lembas.services import settings_store
settings_store.update(
db, {"samplers": ["euler", "dpmpp_2m"], "schedulers": ["normal"]},
key=settings_store.IMAGES,
)
db.commit()
client.post(
"/admin/images",
data={"base_url": "http://comfy.test", "api_key": "", "timeout": "600"},
follow_redirects=False,
)
values = settings_store.images(db)
assert values["samplers"] == ["euler", "dpmpp_2m"]
def test_the_sampler_picker_offers_what_comfyui_advertised(client, db, registered):
"""Discovered, stored, and — until now — read by nothing at all. There are
forty-odd samplers and spelling one wrong is a refused workflow, so it is
picked rather than typed."""
from lembas.services import settings_store
settings_store.update(
db, {"samplers": ["euler", "dpmpp_2m_sde"], "schedulers": ["karras"]},
key=settings_store.IMAGES,
)
db.commit()
body = client.get("/admin/images").text
assert 'name="default_sampler"' in body
assert "dpmpp_2m_sde" in body
assert "karras" in body
def test_a_stored_sampler_survives_a_list_that_does_not_have_it(client, db, registered):
"""Otherwise opening the page and saving it silently clears a working
setting, because the select had no option matching the stored value."""
from lembas.services import settings_store
settings_store.update(
db, {"samplers": ["euler"], "default_sampler": "some_custom_sampler"},
key=settings_store.IMAGES,
)
db.commit()
body = client.get("/admin/images").text
assert "some_custom_sampler" in body
def test_the_workflow_editor_says_what_each_placeholder_resolves_to(client, db, registered):
"""A legend listing names answers "what may I write"; the question somebody
has in front of a workflow that came out wrong is "what happens if I leave
this out"."""
from lembas.services import settings_store
settings_store.update(db, {"default_width": 1024}, key=settings_store.IMAGES)
db.commit()
body = client.get("/admin/images/workflows/new").text
assert "ckpt_name" in body # the name that is not the placeholder's
assert "sampler_name" in body
assert "1024" in body # the resolved default, beside {{width}}
+43
View File
@@ -487,3 +487,46 @@ def test_every_parameter_says_when_to_move_it(db, user_id, configured):
"the negative prompt's one real trap: phrasing it as an instruction"
)
assert "portrait" in schema["properties"]["width"]["description"]
def test_the_schema_states_this_instance_s_defaults(db, registered):
"""Every "Default 20." in those descriptions was written when there was one
set of defaults in the world. A schema still saying "Default 512" beside an
instance that draws at 1024 is worse than saying nothing: the model reasons
from it and omits the parameter, arriving at the right behaviour for the
wrong reason or the wrong one silently."""
from lembas.services.images import tool as image_tool
schema = image_tool.schema_for(
db, {"default_width": 1024, "default_steps": 30, "default_sampler": "dpmpp_2m"}
)
properties = schema["properties"]
assert "Default 1024." in properties["width"]["description"]
assert "Default 30." in properties["steps"]["description"]
assert "Default dpmpp_2m." in properties["sampler"]["description"]
# And the rest of the sentence survives -- these say what to do *instead* of
# the default, which is most of their value.
assert "SDXL" in properties["width"]["description"]
def test_a_whole_number_default_is_not_written_as_a_decimal(db, registered):
""""Default 8.0" is text a model reasons about, and reads as a precision
somebody chose."""
from lembas.services.images import tool as image_tool
schema = image_tool.schema_for(db, {"default_cfg": 6})
assert "Default 6." in schema["properties"]["cfg"]["description"]
assert "Default 6.0" not in schema["properties"]["cfg"]["description"]
def test_the_punctuation_after_a_default_is_kept(db, registered):
"""`denoise` says "Default 1, which is what you want…" while the rest use a
full stop. A rewrite that assumed one would produce a sentence that does not
read."""
from lembas.services.images import tool as image_tool
schema = image_tool.schema_for(db, {})
assert "Default 1, which is" in schema["properties"]["denoise"]["description"]
+82
View File
@@ -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