Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
54fee49810
|
@@ -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
|
||||
|
||||
@@ -1,3 +1,3 @@
|
||||
"""LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints."""
|
||||
|
||||
__version__ = "1.3.1"
|
||||
__version__ = "1.3.2"
|
||||
|
||||
@@ -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">
|
||||
<button class="btn btn--sm" type="submit" form="detect-efforts">
|
||||
{{ icon('search', 'icon--sm') }} Detect from the endpoint
|
||||
</button>
|
||||
</form>
|
||||
|
||||
<p class="field__hint">
|
||||
The vocabulary is <strong>not the same for every model</strong>, and
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user