Fix attachments never being sent with the message
Uploading an image showed the chip and then did nothing: the file was stored but never reached the model. Two causes, both in the composer template. The chips live in #attachments, and each carries the hidden file_ids input that binds it to the message. That container sat OUTSIDE the <form>, with an `hx-include="#attachments"` on a hidden <div> inside the form meant to pull it back in. That attribute only has an effect on the element issuing the request -- on a child of it, it does nothing. So the form serialised content and nothing else, and post_message saw no file_ids at all. Fixed by putting #attachments inside the form, where the inputs are submitted because they are in the form, rather than because of an attribute that has to be wired correctly. The file input stays outside, since inside it would submit an empty file part on every message. Second: /chat preselected models[0] rather than the model a new chat would actually use. With a vision model set as the default and a non-vision one first in the admin ordering, the composer showed the wrong model, sent the wrong model, and told the user images *would* be sent when they would not. It now resolves through default_model(), the same path /start uses. Every server-side test passed throughout, because the bug was entirely in the wiring between template and browser. Added tests that serialise the rendered form the way a browser does -- every named input inside <form> -- and assert file_ids is among them and the image reaches the model as a content part. Verified they fail with the old markup restored, then pass again. Confirmed end to end against gemma4-e4b-q8: given a drawing, it replied "Left: Green Circle / Right: Orange Triangle". 220 tests, ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jaroslav Beneš
+14
-3
@@ -82,9 +82,20 @@ async def chat_index(request: Request, db: Db, user: RequiredUser, model: str =
|
||||
creating a row for a chat that may never be sent.
|
||||
"""
|
||||
context = _chat_context(db, user, None)
|
||||
preselected = next(
|
||||
(m for m in context["models"] if m.model_id == model), None
|
||||
) or (context["models"][0] if context["models"] else None)
|
||||
|
||||
# Fall back to the same choice a new chat would make -- the user's default,
|
||||
# then the instance default, then first in order. Using models[0] here
|
||||
# instead would show a model the chat is not going to use, which matters:
|
||||
# the composer decides from it whether to warn that images will be dropped.
|
||||
preselected = next((m for m in context["models"] if m.model_id == model), None)
|
||||
if preselected is None:
|
||||
chosen = chat_service.default_model(db, user)
|
||||
if chosen is not None:
|
||||
preselected = next(
|
||||
(m for m in context["models"] if m.model_id == chosen[0]), None
|
||||
)
|
||||
if preselected is None and context["models"]:
|
||||
preselected = context["models"][0]
|
||||
|
||||
return render(
|
||||
request,
|
||||
|
||||
@@ -312,16 +312,24 @@
|
||||
background: var(--bg);
|
||||
}
|
||||
.composer__inner { max-width: var(--thread-max-width); margin: 0 auto; }
|
||||
|
||||
/* A column: chips on top, then the control row. The chips are inside the form
|
||||
so their hidden file_ids inputs are submitted with the message. */
|
||||
.composer__form {
|
||||
display: flex;
|
||||
gap: var(--sp-1);
|
||||
align-items: flex-end;
|
||||
flex-direction: column;
|
||||
gap: var(--sp-2);
|
||||
padding: var(--sp-2);
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius-xl);
|
||||
background: var(--surface);
|
||||
transition: border-color var(--transition-fast), box-shadow var(--transition-fast);
|
||||
}
|
||||
.composer__row {
|
||||
display: flex;
|
||||
gap: var(--sp-1);
|
||||
align-items: flex-end;
|
||||
}
|
||||
/* Attach and send are the same size and sit on the same baseline as the last
|
||||
line of the textarea, so the control row reads as one object. */
|
||||
.composer__btn { flex: none; align-self: flex-end; border-radius: var(--radius-full); }
|
||||
@@ -386,10 +394,10 @@
|
||||
.composer { position: relative; }
|
||||
|
||||
.composer__attachments {
|
||||
margin: 0 0 var(--sp-2);
|
||||
display: flex;
|
||||
flex-wrap: wrap;
|
||||
gap: var(--sp-2);
|
||||
padding: var(--sp-1) var(--sp-1) 0;
|
||||
}
|
||||
.composer__attachments:empty { display: none; }
|
||||
|
||||
|
||||
@@ -107,15 +107,17 @@
|
||||
rejection) then appears immediately, and a large file cannot make the send
|
||||
button appear to hang. */
|
||||
function uploadFiles(fileList) {
|
||||
var form = document.getElementById("upload-form");
|
||||
var input = document.getElementById("file-input");
|
||||
var target = document.getElementById("attachments");
|
||||
if (!form || !target || !fileList || !fileList.length) return;
|
||||
if (!input || !target || !fileList || !fileList.length) return;
|
||||
|
||||
var url = input.dataset.uploadUrl;
|
||||
|
||||
Array.prototype.forEach.call(fileList, function (file) {
|
||||
var body = new FormData();
|
||||
body.append("file", file, file.name);
|
||||
|
||||
fetch(form.getAttribute("hx-post"), { method: "POST", body: body })
|
||||
fetch(url, { method: "POST", body: body, credentials: "same-origin" })
|
||||
.then(function (response) { return response.text(); })
|
||||
.then(function (html) {
|
||||
target.insertAdjacentHTML("beforeend", html);
|
||||
|
||||
@@ -8,21 +8,23 @@
|
||||
redirects, and the reply streams on arrival because the page renders the
|
||||
unfinished assistant message with its sse-connect. That is what stops an
|
||||
opened-and-abandoned chat ever being written to the database.
|
||||
|
||||
The attachment chips live INSIDE this form on purpose. Each carries a hidden
|
||||
file_ids input, and being inside the form is what gets them serialised with
|
||||
the message. Keeping them outside and reaching for hx-include does not work:
|
||||
that attribute only has an effect on the element issuing the request.
|
||||
#}
|
||||
<div class="composer" {% if can.get("files.upload") %}data-dropzone{% endif %}>
|
||||
{% if can.get("files.upload") %}
|
||||
<form id="upload-form" hx-post="/api/files{% if chat %}?chat_id={{ chat.id }}{% endif %}"
|
||||
hx-target="#attachments" hx-swap="beforeend"
|
||||
hx-encoding="multipart/form-data" hx-on::after-request="this.reset()">
|
||||
<input class="visually-hidden" type="file" name="file" id="file-input" multiple
|
||||
accept="image/*,.pdf,.txt,.md,.csv,.json,.py,.js,.ts,.rs,.go,.sh,.sql,.yaml,.yml,.toml,.log"
|
||||
onchange="window.lembas.uploadFiles(this.files); this.value = ''">
|
||||
</form>
|
||||
{# Outside the form: it is only ever read by JavaScript, and inside it would
|
||||
be submitted as an empty file part on every message. #}
|
||||
<input class="visually-hidden" type="file" id="file-input" multiple
|
||||
data-upload-url="/api/files{% if chat %}?chat_id={{ chat.id }}{% endif %}"
|
||||
accept="image/*,.pdf,.txt,.md,.csv,.json,.py,.js,.ts,.rs,.go,.sh,.sql,.yaml,.yml,.toml,.log"
|
||||
onchange="window.lembas.uploadFiles(this.files); this.value = ''">
|
||||
{% endif %}
|
||||
|
||||
<div class="composer__inner">
|
||||
<div class="composer__attachments" id="attachments"></div>
|
||||
|
||||
<form class="composer__form"
|
||||
{% if chat %}
|
||||
hx-post="/api/chats/{{ chat.id }}/messages"
|
||||
@@ -37,30 +39,31 @@
|
||||
hx-post="/api/chats/start" hx-swap="none"
|
||||
{% endif %}>
|
||||
|
||||
{# The chips live outside this form, so their hidden inputs are pulled in
|
||||
explicitly at submit time. #}
|
||||
<div hx-include="#attachments" hidden></div>
|
||||
<div class="composer__attachments" id="attachments"></div>
|
||||
|
||||
{% if not chat and current_model %}
|
||||
<input type="hidden" name="model_id" value="{{ current_model.model_id }}">
|
||||
{% endif %}
|
||||
|
||||
{% if can.get("files.upload") %}
|
||||
<button class="btn btn--icon composer__btn" type="button"
|
||||
aria-label="Attach a file" title="Attach a file"
|
||||
onclick="document.getElementById('file-input').click()">
|
||||
{{ icon("attach") }}
|
||||
</button>
|
||||
{% endif %}
|
||||
<div class="composer__row">
|
||||
{% if can.get("files.upload") %}
|
||||
<button class="btn btn--icon composer__btn" type="button"
|
||||
aria-label="Attach a file" title="Attach a file"
|
||||
onclick="document.getElementById('file-input').click()">
|
||||
{{ icon("attach") }}
|
||||
</button>
|
||||
{% endif %}
|
||||
|
||||
<textarea class="composer__input" name="content" rows="1"
|
||||
data-autosize data-max-height="320" data-composer-input
|
||||
placeholder="{% if chat %}Send a message…{% else %}Ask anything…{% endif %}"
|
||||
aria-label="Message" {{ 'autofocus' if not chat }}></textarea>
|
||||
<textarea class="composer__input" name="content" rows="1"
|
||||
data-autosize data-max-height="320" data-composer-input
|
||||
placeholder="{% if chat %}Send a message…{% else %}Ask anything…{% endif %}"
|
||||
aria-label="Message" {{ 'autofocus' if not chat }}></textarea>
|
||||
|
||||
<button class="btn btn--primary btn--icon composer__btn" type="submit"
|
||||
aria-label="Send">
|
||||
{{ icon("send") }}
|
||||
</button>
|
||||
<button class="btn btn--primary btn--icon composer__btn" type="submit"
|
||||
aria-label="Send">
|
||||
{{ icon("send") }}
|
||||
</button>
|
||||
</div>
|
||||
</form>
|
||||
|
||||
<p class="composer__hint">
|
||||
|
||||
@@ -495,3 +495,98 @@ def test_deleting_a_message_deletes_its_attachments(client: TestClient, db, chat
|
||||
|
||||
client.delete(f"/api/chats/{chat_with_model}")
|
||||
assert db.scalar(select(Attachment)) is None
|
||||
|
||||
|
||||
# --- Composer wiring ---------------------------------------------------------
|
||||
# These assert on the rendered HTML rather than on behaviour, because the bug
|
||||
# they guard against lives entirely in the template: every server-side test
|
||||
# passed while the browser silently never sent file_ids at all.
|
||||
def test_the_attachments_container_is_inside_the_composer_form(
|
||||
client: TestClient, db, chat_with_model
|
||||
):
|
||||
"""The chips carry the hidden file_ids inputs. Outside the form they are
|
||||
not serialised, and hx-include does not help -- it only has an effect on
|
||||
the element issuing the request, not on a child of it."""
|
||||
import re
|
||||
|
||||
page = client.get(f"/chat/{chat_with_model}").text
|
||||
form = re.search(r'<form class="composer__form".*?</form>', page, re.S)
|
||||
assert form, "composer form not found"
|
||||
assert 'id="attachments"' in form.group(0)
|
||||
|
||||
|
||||
def test_the_file_input_is_outside_the_composer_form(client: TestClient, db, chat_with_model):
|
||||
"""Inside, it would be submitted as an empty file part on every message."""
|
||||
import re
|
||||
|
||||
page = client.get(f"/chat/{chat_with_model}").text
|
||||
form = re.search(r'<form class="composer__form".*?</form>', page, re.S)
|
||||
assert 'id="file-input"' not in form.group(0)
|
||||
assert 'id="file-input"' in page
|
||||
|
||||
|
||||
def test_the_new_chat_composer_also_contains_the_attachments(
|
||||
client: TestClient, db, chat_with_model
|
||||
):
|
||||
import re
|
||||
|
||||
page = client.get("/chat").text
|
||||
form = re.search(r'<form class="composer__form".*?</form>', page, re.S)
|
||||
assert form and 'id="attachments"' in form.group(0)
|
||||
|
||||
|
||||
def test_the_chip_carries_a_file_ids_input(client: TestClient, db, registered):
|
||||
"""That input is the entire mechanism by which an upload reaches a message."""
|
||||
response = client.post(
|
||||
"/api/files", files={"file": ("a.txt", b"hi", "text/plain")}
|
||||
)
|
||||
assert 'name="file_ids"' in response.text
|
||||
assert 'type="hidden"' in response.text
|
||||
|
||||
|
||||
def test_a_browser_serialising_the_form_actually_sends_the_attachment(
|
||||
client: TestClient, db, chat_with_model
|
||||
):
|
||||
"""End-to-end wiring check.
|
||||
|
||||
Uploads a file, splices the returned chip into the page exactly as the
|
||||
browser does, then serialises the composer form the way a browser would --
|
||||
every named input inside <form> -- and posts that. This is the test that
|
||||
fails when the chips drift back outside the form.
|
||||
"""
|
||||
import re
|
||||
|
||||
chip = client.post(
|
||||
"/api/files", files={"file": ("proof.png", png_bytes(), "image/png")}
|
||||
).text
|
||||
attachment = db.scalar(select(Attachment))
|
||||
assert attachment.message_id is None
|
||||
|
||||
page = client.get(f"/chat/{chat_with_model}").text
|
||||
form_html = re.search(r'<form class="composer__form".*?</form>', page, re.S).group(0)
|
||||
# The chips are inserted into #attachments, which lives inside the form.
|
||||
form_html = form_html.replace(
|
||||
'<div class="composer__attachments" id="attachments"></div>',
|
||||
f'<div class="composer__attachments" id="attachments">{chip}</div>',
|
||||
)
|
||||
|
||||
fields: list[tuple[str, str]] = []
|
||||
for tag in re.findall(r"<(?:input|textarea)\b[^>]*>", form_html):
|
||||
name = re.search(r'name="([^"]+)"', tag)
|
||||
if not name:
|
||||
continue
|
||||
value = re.search(r'value="([^"]*)"', tag)
|
||||
fields.append((name.group(1), value.group(1) if value else ""))
|
||||
|
||||
assert ("file_ids", attachment.id) in fields, f"file_ids not serialised: {fields}"
|
||||
|
||||
client.post(f"/api/chats/{chat_with_model}/messages", data=dict(fields) | {"content": "look"})
|
||||
|
||||
db.refresh(attachment)
|
||||
message = db.scalar(select(Message).where(Message.role == "user"))
|
||||
assert attachment.message_id == message.id
|
||||
|
||||
chat = db.get(Chat, chat_with_model)
|
||||
content = chat_service.build_request(db, chat)["messages"][0]["content"]
|
||||
assert isinstance(content, list), "the image never reached the model"
|
||||
assert any(p.get("type") == "image_url" for p in content)
|
||||
|
||||
@@ -660,3 +660,54 @@ def test_moving_returns_to_the_filtered_view(client: TestClient, db, registered)
|
||||
follow_redirects=False,
|
||||
)
|
||||
assert response.headers["location"] == "/admin/models?filter=enabled&page=2"
|
||||
|
||||
|
||||
def test_the_new_chat_composer_preselects_the_default_model(
|
||||
client: TestClient, db, registered
|
||||
):
|
||||
"""It must match what /start would actually pick. Showing a different model
|
||||
also mis-reports whether images will be sent."""
|
||||
connection = _connection(db)
|
||||
db.add_all(
|
||||
[
|
||||
Model(connection_id=connection.id, model_id="first-in-order", position=0),
|
||||
Model(connection_id=connection.id, model_id="the-default", position=7),
|
||||
]
|
||||
)
|
||||
db.commit()
|
||||
settings_store.update(db, {"default_model": "the-default"})
|
||||
|
||||
page = client.get("/chat").text
|
||||
assert 'value="the-default"' in page
|
||||
assert '<option value="the-default"\n selected' in page or (
|
||||
'value="the-default"' in page and "selected" in page
|
||||
)
|
||||
# And the hidden field the composer submits carries it too.
|
||||
assert 'name="model_id" value="the-default"' in page
|
||||
|
||||
|
||||
def test_the_composer_respects_an_explicit_model_query(client: TestClient, db, registered):
|
||||
connection = _connection(db)
|
||||
db.add_all(
|
||||
[
|
||||
Model(connection_id=connection.id, model_id="default-one", position=0),
|
||||
Model(connection_id=connection.id, model_id="asked-for", position=1),
|
||||
]
|
||||
)
|
||||
db.commit()
|
||||
settings_store.update(db, {"default_model": "default-one"})
|
||||
|
||||
page = client.get("/chat?model=asked-for").text
|
||||
assert 'name="model_id" value="asked-for"' in page
|
||||
|
||||
|
||||
def test_the_vision_warning_follows_the_preselected_model(client: TestClient, db, registered):
|
||||
connection = _connection(db)
|
||||
db.add(
|
||||
Model(
|
||||
connection_id=connection.id, model_id="blind-model", position=0,
|
||||
capabilities_json={"vision": False},
|
||||
)
|
||||
)
|
||||
db.commit()
|
||||
assert "has no vision" in client.get("/chat").text
|
||||
|
||||
Reference in New Issue
Block a user