diff --git a/src/lembas/api/pages.py b/src/lembas/api/pages.py index 3b781d2..1617c4f 100644 --- a/src/lembas/api/pages.py +++ b/src/lembas/api/pages.py @@ -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, diff --git a/src/lembas/web/static/css/chat.css b/src/lembas/web/static/css/chat.css index 126c664..0508dff 100644 --- a/src/lembas/web/static/css/chat.css +++ b/src/lembas/web/static/css/chat.css @@ -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; } diff --git a/src/lembas/web/static/js/app.js b/src/lembas/web/static/js/app.js index 1d620a1..bc7d254 100644 --- a/src/lembas/web/static/js/app.js +++ b/src/lembas/web/static/js/app.js @@ -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); diff --git a/src/lembas/web/templates/chat/_composer.html b/src/lembas/web/templates/chat/_composer.html index fb419c7..d57df01 100644 --- a/src/lembas/web/templates/chat/_composer.html +++ b/src/lembas/web/templates/chat/_composer.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. #}
{% if can.get("files.upload") %} -
- -
+ {# Outside the form: it is only ever read by JavaScript, and inside it would + be submitted as an empty file part on every message. #} + {% endif %}
-
-
- {# The chips live outside this form, so their hidden inputs are pulled in - explicitly at submit time. #} - +
+ {% if not chat and current_model %} {% endif %} - {% if can.get("files.upload") %} - - {% endif %} +
+ {% if can.get("files.upload") %} + + {% endif %} - + - + +

diff --git a/tests/test_files.py b/tests/test_files.py index 354ad5e..e4ef42d 100644 --- a/tests/test_files.py +++ b/tests/test_files.py @@ -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'

', 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'', 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'', 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 -- 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'', page, re.S).group(0) + # The chips are inserted into #attachments, which lives inside the form. + form_html = form_html.replace( + '
', + f'
{chip}
', + ) + + 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) diff --git a/tests/test_permissions.py b/tests/test_permissions.py index fa4e844..efb71fa 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -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 '