A page that could not save, and said nothing

The model page has been unable to save anything below the reasoning efforts
since 1.3.0. "Save changes" did nothing at all, so the description, the system
prompt, every capability and tool switch and the whole availability card
silently would not take -- while the fields above it saved normally, which is
what made the page look as though it worked.

"Detect from the endpoint" had stopped detecting too: it submitted the page as
an ordinary save carrying only the top half of the form, so every field below
took its empty default. Pressing it would have cleared that model's description
and system prompt and switched the model off with all of its tools disabled.

One HTML rule causes both. A form inside another form is not allowed, and rather
than complaining a browser discards the inner start tag and lets the matching
end tag close the *outer* form -- so from that point down the page was in no
form, and a button in no form does nothing. The detect form is now declared
before the main one and the button reaches it by id.

Nothing in the markup reads wrong, and no test that posts to a route can see
this, because such a test supplies the fields itself. tests/test_form_structure.py
reads every template the way a browser parses it instead, including that rule,
and was checked against the old markup before being trusted: it reports the same
orphaned "Save changes" that headless Chromium did.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

CHANGELOG.md

@@ -16,6 +16,46 @@ for 1.0.0 have something to be assembled from.
 
 ## Unreleased
 
+## 1.3.2
+
+- Fixed: **the model page could not save anything below the reasoning efforts**,
+  and had not been able to since 1.3.0. "Save changes" did nothing at all — not
+  slowly, not with an error, simply nothing — so the description, the system
+  prompt, every capability and tool switch, and the whole availability card
+  (enabled, pinned, available to everyone, groups) silently would not take. The
+  fields above it, including the display name and the reasoning efforts, saved
+  normally, which is what made it look like it worked.
+
+  Worse, the **Detect from the endpoint** button had stopped detecting. It
+  submitted the page as an ordinary save instead — a save carrying only the top
+  half of the form, so everything below took its empty default: it would have
+  cleared that model's description and system prompt and switched the model off
+  with all of its tools disabled. If you pressed it, check that model's page.
+
+  The cause was one HTML rule: a form inside another form is not allowed, and
+  rather than complaining, a browser discards the inner tag and lets the closing
+  tag end the *outer* form. Everything after that point was in no form, and a
+  button in no form does nothing. Nothing in the markup looks wrong, and no test
+  that posts to a route can see it — so the fix comes with one that reads every
+  page the way a browser parses it.
+
+## 1.3.1
+
+- Fixed: **updating to 1.2.0 or later broke every page that lists models**, with
+  a 500 and nothing but the error page to show for it. The per-model reasoning
+  effort list added in 1.2.0 was the first list-shaped setting this application
+  had ever added to a table that already had rows in it, and the code that fills
+  in such a column on existing rows could not tell a list from a dictionary — so
+  it wrote the wrong kind of empty value into every model, and reading one back
+  raised rather than returning nothing.
+
+  A fresh install was never affected, which is exactly why it was not caught:
+  the column is only filled in that way on a database that already existed.
+
+  This release both stops it happening and **puts right the rows already
+  written**, on start, with nothing to run by hand. If your instance is showing
+  the error page, updating is the whole fix.
+
 ## 1.3.0
 
 - **A model's reasoning efforts can now be detected rather than known.** There

src/lembas/__init__.py

@@ -1,3 +1,3 @@
 """LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints."""
 
-__version__ = "1.3.0"
+__version__ = "1.3.2"

src/lembas/db/migrations.py

@@ -39,6 +39,29 @@ log = logging.getLogger(__name__)
 MANUAL_STEPS: list[str] = []
 
 
+def _default_shape(column: Column) -> type | None:
+    """`list` or `dict`, from the column's own Python-side default.
+
+    `default=list` and `default=dict` are how the two JSON flavours are
+    declared, and SQLAlchemy keeps the callable. Calling it is cheap and is the
+    only way to tell a MutableList column from a MutableDict one -- see the note
+    in `_literal_default`.
+    """
+    default = column.default
+    if default is None or not getattr(default, "is_callable", False):
+        return None
+    try:
+        # SQLAlchemy wraps a zero-argument callable to take a context.
+        produced = default.arg(None)
+    except Exception:  # noqa: BLE001 - a default we cannot call tells us nothing
+        return None
+    if isinstance(produced, list):
+        return list
+    if isinstance(produced, dict):
+        return dict
+    return None
+
+
 def _literal_default(column: Column) -> str | None:
     """A SQL literal to backfill an existing row's new column with.
 
@@ -63,8 +86,22 @@ def _literal_default(column: Column) -> str | None:
     if "JSON" in affinity:
         # MutableList columns must start as [] and MutableDict as {}; guessing
         # wrong makes the first read blow up rather than return empty.
-        python_type = getattr(column.type, "python_type", None)
+        #
-        return "'[]'" if python_type is list else "'{}'"
+        # 🚨 NOT `column.type.python_type`. `MutableList.as_mutable(JSON)`
+        # returns the *same* JSON type object with an event listener attached --
+        # it does not subclass or wrap it -- so the type cannot tell you which
+        # of the two it is, and `JSON.python_type` is `dict` for both. That read
+        # as "this is a dict column" for every list column, and the first one
+        # ever added by a migration (`Model.reasoning_efforts`, 1.2.0) arrived
+        # as `'{}'` on every existing row. `MutableList` refuses a dict, so the
+        # failure was not an empty list but a ValueError on *load* -- every page
+        # that lists models, 500, on an instance that had simply been updated.
+        #
+        # The Python-side default is the only honest signal: a JSONList column
+        # is declared `default=list` and a JSONDict one `default=dict`, and
+        # calling it says which. Anything that cannot be called or produces
+        # neither falls back to `{}`, which is what this always assumed.
+        return "'[]'" if _default_shape(column) is list else "'{}'"
     if "BOOL" in affinity:
         return "0"
     if any(token in affinity for token in ("INT", "FLOAT", "NUMERIC", "DECIMAL")):
@@ -190,6 +227,50 @@ def ensure_fts(engine: Engine) -> list[str]:
     return created
 
 
+def repair_json_shapes(engine: Engine) -> list[str]:
+    """Put right any JSON column backfilled with the wrong empty value.
+
+    `_literal_default` used to read the shape off `column.type.python_type`,
+    which is `dict` for a MutableList column as well as a MutableDict one -- so
+    the first list-shaped JSON column ever added by a migration arrived as
+    `'{}'` on every row that already existed. `MutableList` refuses a dict, and
+    refuses it while *loading*, so the symptom was not an empty list but a
+    `ValueError` and a 500 on every page that touched the table.
+
+    Converges, like `ensure_fts` beside it: it runs on every start, it is
+    idempotent, and on a database that was never damaged it does nothing. Only
+    the exact wrong value is rewritten -- `'{}'` in a column whose default
+    produces a list -- because `{}` cannot be a legitimate value there, while
+    anything else in that column might be somebody's data.
+    """
+    fixed: list[str] = []
+    inspector = inspect(engine)
+    known = set(inspector.get_table_names())
+
+    with engine.begin() as connection:
+        for table in Base.metadata.sorted_tables:
+            if table.name not in known:
+                continue
+            for column in table.columns:
+                if "JSON" not in column.type.__class__.__name__.upper():
+                    continue
+                if _default_shape(column) is not list:
+                    continue
+                result = connection.execute(
+                    text(
+                        f'UPDATE "{table.name}" SET "{column.name}" = \'[]\' '
+                        f'WHERE "{column.name}" = \'{{}}\''
+                    )
+                )
+                if result.rowcount:
+                    fixed.append(f"{table.name}.{column.name} ({result.rowcount} row(s))")
+                    log.warning(
+                        "repaired %s.%s on %d row(s): was '{}' in a list column",
+                        table.name, column.name, result.rowcount,
+                    )
+    return fixed
+
+
 def sync_schema(engine: Engine) -> list[str]:
     """Bring the database up to the declared schema. Returns what it changed."""
     import lembas.db.models  # noqa: F401  (registers every table on the metadata)
@@ -219,6 +300,14 @@ def sync_schema(engine: Engine) -> list[str]:
                 changes.append(f"add column {table.name}.{column.name}")
                 log.info("schema: %s", statement)
 
+    # Before the search indexes, and before anything can try to load a row:
+    # a column left holding the wrong empty value makes the ORM raise on read.
+    try:
+        for repair in repair_json_shapes(engine):
+            changes.append(f"repair {repair}")
+    except Exception:  # noqa: BLE001 - a repair that fails must not stop a start
+        log.exception("could not repair JSON column shapes")
+
     try:
         for index in ensure_fts(engine):
             changes.append(f"create search index {index}")

src/lembas/web/templates/admin/model_detail.html

@@ -69,6 +69,12 @@
   </p>
 </section>
 
+{# Empty, hidden, and outside every other form: the Detect button further down
+   is associated with it by `form="detect-efforts"`. It carries no fields on
+   purpose — detection asks the endpoint and needs nothing from this page. #}
+<form id="detect-efforts" method="post"
+      action="/admin/models/{{ model.id }}/detect-efforts" hidden></form>
+
 <form method="post" action="/admin/models/{{ model.id }}">
   <section class="card">
     <h2 class="card__title">Presentation</h2>
@@ -132,13 +138,28 @@
         pretending the model accepts nothing.
 
         Its own form, because this page's main form is a PUT of everything and
-        a detect must not carry half-edited fields with it.
+        a detect must not carry half-edited fields with it — and that form is
+        declared before the main one rather than here, with this button reaching
+        it by id.
+
+        🚨 It was written inline here, nested inside the main form, which HTML
+        does not allow. Nothing complains: the parser *drops* the inner `form`
+        start tag and then lets the matching end tag close the outer one — so
+        from this point down the page was in no form at all. "Save changes"
+        submitted nothing; the description, the system prompt, every capability
+        and the whole availability card could not be saved. And this button
+        submitted the main form's surviving half to the *save* route, where every
+        field it did not carry took its default: description cleared, system
+        prompt cleared, and the model disabled with all of its tools off.
+
+        Shipped in 1.3.0 and found in 1.3.2 by asking a browser which form each
+        control belonged to, which is the only thing that finds it — the markup
+        reads correctly, and a test posting to the route bypasses the parser
+        entirely. `tests/test_form_structure.py` is the guard.
       #}
-      <form method="post" action="/admin/models/{{ model.id }}/detect-efforts">
+      <button class="btn btn--sm" type="submit" form="detect-efforts">
-        <button class="btn btn--sm" type="submit">
+        {{ icon('search', 'icon--sm') }} Detect from the endpoint
-          {{ icon('search', 'icon--sm') }} Detect from the endpoint
+      </button>
-        </button>
-      </form>
 
       <p class="field__hint">
         The vocabulary is <strong>not the same for every model</strong>, and

tests/test_form_structure.py

@@ -0,0 +1,135 @@
+"""Where a form begins and ends, and which form a button belongs to.
+
+Every other test in this suite talks to a route. That is what let this ship: a
+POST from `TestClient` carries exactly the fields the test names, so a page whose
+fields are not in any form passes every one of them. The browser is the only
+thing that disagrees, and what it disagrees about is a parse rule.
+
+`<form>` inside `<form>` is not allowed in HTML, and the failure is silent and
+inverted: the parser **drops the inner start tag**, and the inner *end* tag then
+closes the outer form. So a nested form does not create a small form inside a big
+one -- it truncates the big one, and everything below becomes unsubmittable.
+
+That is what `admin/model_detail.html` did from 1.3.0 to 1.3.2. "Save changes"
+belonged to no form and did nothing; the description, the system prompt, all
+nineteen capability switches and the availability card could not be saved; and
+the one button that *was* inside the surviving half posted it to the save route,
+where every absent field took its `Form()` default -- clearing the description
+and the system prompt and disabling the model.
+
+The markup reads correctly at every point, which is why this is a test about
+structure rather than about wording.
+"""
+
+from __future__ import annotations
+
+import re
+from pathlib import Path
+
+import pytest
+
+TEMPLATES = Path(__file__).resolve().parents[1] / "src/lembas/web/templates"
+
+# Jinja comments are not markup. The explanation of this very bug, in
+# `model_detail.html`, contains the words it warns about.
+COMMENT = re.compile(r"\{#.*?#\}", re.S)
+TAG = re.compile(r"<form\b|</form\s*>", re.I)
+SUBMIT = re.compile(r"<button\b[^>]*>", re.I)
+
+
+def _markup(template: Path) -> str:
+    return COMMENT.sub("", template.read_text())
+
+
+def _pages() -> list[Path]:
+    return sorted(TEMPLATES.rglob("*.html"))
+
+
+def test_the_scan_finds_the_forms_it_is_meant_to_police():
+    """A blindness guard. If the tags stop being written the way this matches,
+    every assertion below passes by finding nothing -- which is exactly how the
+    bug it exists for got through its own page's tests."""
+    total = sum(len(TAG.findall(_markup(page))) for page in _pages())
+    assert total > 40, f"only {total} form tags found across the templates"
+
+
+@pytest.mark.parametrize("page", _pages(), ids=lambda p: p.name)
+def test_no_form_is_nested_inside_another(page: Path):
+    depth = 0
+    for match in TAG.finditer(_markup(page)):
+        if match.group(0).startswith("</"):
+            depth -= 1
+            assert depth >= 0, f"{page.name}: a form ends where none began"
+            continue
+        depth += 1
+        assert depth == 1, (
+            f"{page.name}: a form opens inside another at character {match.start()}. "
+            "HTML drops the inner tag and the matching end tag closes the OUTER "
+            "form, so everything below it stops being submittable. Declare the "
+            "second form outside the first and point the button at it with "
+            'form="its-id".'
+        )
+
+
+@pytest.mark.parametrize("page", _pages(), ids=lambda p: p.name)
+def test_every_submit_button_can_actually_submit_something(page: Path):
+    """A submit outside every form is inert, and looks exactly like a working one.
+
+    A button may reach its form by id instead of by containment, which is how
+    the fix to the bug above works -- so an `form="..."` is accepted, provided
+    the form it names is declared in the same template.
+    """
+    markup = _markup(page)
+    ids = set(re.findall(r'<form\b[^>]*\bid="([^"]+)"', markup))
+
+    # Open **as a browser would**, which is the whole point. A `<form>` start tag
+    # while a form is already open is a parse error and is *ignored*; the next
+    # end tag therefore closes the one that was already open. Counting nesting
+    # naively instead reports the buttons after it as still inside a form, which
+    # is precisely the wrong answer -- and the reason the first version of this
+    # test passed on the markup it was written for.
+    open_form = False
+    cursor = 0
+    orphans: list[str] = []
+
+    def check(start: int, end: int | None) -> None:
+        for button in SUBMIT.finditer(markup, start, end if end is not None else len(markup)):
+            tag = button.group(0)
+            if 'type="submit"' not in tag:
+                continue
+            named = re.search(r'\bform="([^"]+)"', tag)
+            if named is not None:
+                assert named.group(1) in ids, (
+                    f"{page.name}: a submit button names form "
+                    f"{named.group(1)!r}, which this template does not declare"
+                )
+                continue
+            if not open_form:
+                orphans.append(tag[:90])
+
+    for match in TAG.finditer(markup):
+        check(cursor, match.start())
+        cursor = match.end()
+        if match.group(0).startswith("</"):
+            open_form = False
+        elif not open_form:
+            open_form = True
+    check(cursor, None)
+
+    assert not orphans, (
+        f"{page.name}: {len(orphans)} submit button(s) belong to no form and do "
+        f"nothing when pressed: {orphans}"
+    )
+
+
+def test_the_detect_button_is_associated_with_the_detect_form():
+    """The specific fix, pinned. Not the general rule above: this says the button
+    reaches the *detection* route, which is the half the general rule cannot see.
+    Submitting the page's main form instead is what cleared a model's settings."""
+    markup = _markup(TEMPLATES / "admin/model_detail.html")
+    form = re.search(
+        r'<form\b[^>]*\bid="detect-efforts"[^>]*\baction="([^"]*)"', markup, re.S
+    )
+    assert form, "the detect form is gone; the button below it now saves the page"
+    assert form.group(1).endswith("/detect-efforts")
+    assert 'form="detect-efforts"' in markup

tests/test_migrations.py

@@ -230,3 +230,124 @@ def test_each_new_table_is_usable_after_the_upgrade(db, table):
 
     declared = {column.name for column in Base.metadata.tables[table].c}
     assert _columns(engine, table) == declared
+
+
+# --- A list-shaped JSON column added to a database that already had rows -----
+#
+# Reported as a 500 on a live instance the moment it updated:
+#
+#   ValueError: Attribute 'reasoning_efforts' does not accept objects
+#               of type <class 'dict'>
+#
+# `_literal_default` read the shape off `column.type.python_type`, and
+# `MutableList.as_mutable(JSON)` returns the *same* JSON type object with a
+# listener attached -- it does not subclass it -- so `python_type` is `dict` for
+# both flavours. Every existing row got `'{}'` in a list column, and MutableList
+# refuses a dict while *loading*, so every page that listed models raised.
+#
+# The suite never caught it because `conftest.py` builds a fresh database, where
+# the column is created from the model rather than backfilled by a migration.
+# These tests exercise the path that actually ran.
+def test_a_list_column_is_backfilled_with_a_list():
+    from lembas.db.migrations import _default_shape, _literal_default
+    from lembas.db.models import Model
+
+    columns = {c.name: c for c in Model.__table__.columns}
+    assert _default_shape(columns["reasoning_efforts"]) is list
+    assert _literal_default(columns["reasoning_efforts"]) == "'[]'"
+
+
+def test_a_dict_column_still_gets_a_dict():
+    from lembas.db.migrations import _literal_default
+    from lembas.db.models import Model
+
+    columns = {c.name: c for c in Model.__table__.columns}
+    assert _literal_default(columns["capabilities_json"]) == "'{}'"
+    assert _literal_default(columns["params_json"]) == "'{}'"
+
+
+def _seed_model(engine, **overrides):
+    """A real row, made the way the application makes one.
+
+    Built through the ORM rather than a hand-written INSERT: the table has
+    several NOT NULL columns and a test that enumerates them is a test that
+    breaks every time one is added, for reasons having nothing to do with what
+    it is checking.
+    """
+    from sqlalchemy.orm import Session
+
+    from lembas.db.models import Connection, Model
+
+    with Session(engine) as session:
+        connection = Connection(
+            name="local", base_url="http://127.0.0.1:1", api_key_encrypted=""
+        )
+        session.add(connection)
+        session.flush()
+        model = Model(connection_id=connection.id, model_id="bonsai", **overrides)
+        session.add(model)
+        session.commit()
+        return model.id
+
+
+def test_the_damage_already_written_is_repaired_on_start(tmp_path):
+    """The fix to `_literal_default` helps the next instance. This is the one
+    that helps the instance that has already updated."""
+    from sqlalchemy import create_engine, text
+
+    from lembas.db.migrations import repair_json_shapes, sync_schema
+
+    engine = create_engine(f"sqlite:///{tmp_path}/repair.db")
+    sync_schema(engine)
+    model_id = _seed_model(engine)
+
+    # Exactly what the broken backfill left behind on a row that predated the
+    # column: the wrong empty value, in a column that refuses it on load.
+    with engine.begin() as connection:
+        connection.execute(
+            text("UPDATE models SET reasoning_efforts = '{}' WHERE id = :id"),
+            {"id": model_id},
+        )
+
+    assert repair_json_shapes(engine)
+
+    with engine.begin() as connection:
+        stored = connection.execute(
+            text("SELECT reasoning_efforts FROM models WHERE id = :id"), {"id": model_id}
+        ).scalar()
+    assert stored == "[]"
+
+    # And the row loads again, which is the whole point -- the failure was a
+    # ValueError while reading, not a wrong value sitting harmlessly.
+    from sqlalchemy.orm import Session
+
+    from lembas.db.models import Model
+
+    with Session(engine) as session:
+        assert session.get(Model, model_id).reasoning_efforts == []
+
+    # Converges: a second run finds nothing left to do.
+    assert repair_json_shapes(engine) == []
+
+
+def test_the_repair_leaves_a_dict_column_alone(tmp_path):
+    """`{}` is a legitimate value in a MutableDict column and must survive."""
+    from sqlalchemy import create_engine, text
+
+    from lembas.db.migrations import repair_json_shapes, sync_schema
+
+    engine = create_engine(f"sqlite:///{tmp_path}/keep.db")
+    sync_schema(engine)
+    model_id = _seed_model(engine)
+
+    with engine.begin() as connection:
+        connection.execute(
+            text("UPDATE models SET capabilities_json = '{}' WHERE id = :id"),
+            {"id": model_id},
+        )
+    repair_json_shapes(engine)
+    with engine.begin() as connection:
+        stored = connection.execute(
+            text("SELECT capabilities_json FROM models WHERE id = :id"), {"id": model_id}
+        ).scalar()
+    assert stored == "{}"