diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d936c0..369db9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,29 @@ 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 diff --git a/src/lembas/__init__.py b/src/lembas/__init__.py index 508d50f..954e169 100644 --- a/src/lembas/__init__.py +++ b/src/lembas/__init__.py @@ -1,3 +1,3 @@ """LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints.""" -__version__ = "1.3.1" +__version__ = "1.3.2" diff --git a/src/lembas/web/templates/admin/model_detail.html b/src/lembas/web/templates/admin/model_detail.html index 29c9e66..291a0c0 100644 --- a/src/lembas/web/templates/admin/model_detail.html +++ b/src/lembas/web/templates/admin/model_detail.html @@ -69,6 +69,12 @@

+{# 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. #} + +

Presentation

@@ -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. #} - - - +

The vocabulary is not the same for every model, and diff --git a/tests/test_form_structure.py b/tests/test_form_structure.py new file mode 100644 index 0000000..946cb82 --- /dev/null +++ b/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. + +`

` inside `` 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"", re.I) +SUBMIT = re.compile(r"]*>", 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("= 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']*\bid="([^"]+)"', markup)) + + # Open **as a browser would**, which is the whole point. A `` 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("]*\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