Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
54fee49810
|
@@ -16,6 +16,29 @@ for 1.0.0 have something to be assembled from.
|
|||||||
|
|
||||||
## Unreleased
|
## 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
|
## 1.3.1
|
||||||
|
|
||||||
- Fixed: **updating to 1.2.0 or later broke every page that lists models**, with
|
- 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."""
|
"""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>
|
</p>
|
||||||
</section>
|
</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 }}">
|
<form method="post" action="/admin/models/{{ model.id }}">
|
||||||
<section class="card">
|
<section class="card">
|
||||||
<h2 class="card__title">Presentation</h2>
|
<h2 class="card__title">Presentation</h2>
|
||||||
@@ -132,13 +138,28 @@
|
|||||||
pretending the model accepts nothing.
|
pretending the model accepts nothing.
|
||||||
|
|
||||||
Its own form, because this page's main form is a PUT of everything and
|
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">
|
<p class="field__hint">
|
||||||
The vocabulary is <strong>not the same for every model</strong>, and
|
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