From 7b67568f2ce506d7cb60a5d7abee4a2228993bd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Bene=C5=A1?= Date: Tue, 21 Jul 2026 12:59:52 +0200 Subject: [PATCH] Fix bulk actions, create chats lazily, rework the UI Seven reported problems. **Bulk model actions 404'd.** /admin/models/{model_id} was registered before /admin/models/bulk, and FastAPI matches in registration order, so "bulk" was parsed as a model id. Moved above the parameterised route, with a comment saying why, and a regression test. **Empty chats piled up.** There is now no endpoint that creates one. "New chat" is a link to /chat, which renders a composer with no row behind it, and POST /api/chats/start writes the chat together with its first message. Opening one and walking away leaves nothing. **Pinning meant two different things.** The picker is now always in the administrator's position order; pinned models get shortcuts in the chat sidebar and nothing else. A picker whose order silently differs from the admin screen is just confusing. **Model images were missing in chat.** Assistant bubbles now show the avatar of the model that actually wrote the turn -- which is not always the model the chat is set to now -- falling back to the LLeMbas mark. The picker shows it too. **No global or per-model system prompt.** Three layers now: instance (Admin -> General), model (Admin -> Models), chat. Precedence, not concatenation: most specific wins outright. Stacking them reads well in a settings screen and badly in practice, because two layers that disagree give the model contradictory instructions and nobody can tell which is losing. The chat panel shows the inherited prompt as placeholder text so "leave empty to inherit" is not a guess. **Alignment and button sizing.** Added --control-h and friends to tokens.css; every button, input and select takes its height from them, so a mixed row is flush by construction rather than by per-instance nudging. Icon buttons are square at that height. Added .btn-row, .card__header/.card__footer and .grid so pages stop carrying inline styles, and moved every admin page onto them. **Settings needed structure.** The user settings page is now tabbed (Account / Models / Appearance / Security) using radio inputs and sibling selectors -- no JavaScript, and the browser keeps the chosen tab across a re-render. Caught while checking: the chat.css surgery had deleted the attachment, chip and dropzone rules. Restored, and there is now a check that every literal class used in a template has a CSS rule. 197 tests, ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 30 +- README.md | 6 +- src/lembas/api/admin.py | 2 + src/lembas/api/admin_models.py | 60 ++-- src/lembas/api/chats.py | 70 +++- src/lembas/api/pages.py | 47 ++- src/lembas/db/models/connection.py | 4 + src/lembas/services/chat.py | 46 ++- src/lembas/services/settings_store.py | 3 + src/lembas/web/static/css/admin.css | 220 +++++++++---- src/lembas/web/static/css/app.css | 202 ++++++++++-- src/lembas/web/static/css/chat.css | 59 +--- src/lembas/web/static/css/tokens.css | 16 +- src/lembas/web/static/js/app.js | 14 + .../web/templates/admin/_connection_row.html | 6 +- .../web/templates/admin/connections.html | 4 +- src/lembas/web/templates/admin/general.html | 17 +- src/lembas/web/templates/admin/groups.html | 12 +- src/lembas/web/templates/admin/models.html | 43 ++- src/lembas/web/templates/admin/users.html | 15 +- src/lembas/web/templates/chat/_composer.html | 84 +++++ src/lembas/web/templates/chat/_message.html | 21 +- src/lembas/web/templates/chat/index.html | 184 ++++------- .../web/templates/partials/sidebar.html | 49 +-- src/lembas/web/templates/settings.html | 303 +++++++++++------- tests/conftest.py | 37 +++ tests/test_auth.py | 4 +- tests/test_chat.py | 77 +++-- tests/test_files.py | 11 +- tests/test_permissions.py | 154 +++++++-- 30 files changed, 1264 insertions(+), 536 deletions(-) create mode 100644 src/lembas/web/templates/chat/_composer.html diff --git a/CLAUDE.md b/CLAUDE.md index 2118f17..f1a6dca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,7 +19,7 @@ lembas info # paths + counts, useful when confused lembas secret-key # generate LEMBAS_SECRET_KEY lembas create-admin # create or promote an admin -pytest # 186 tests, ~7s +pytest # 197 tests, ~7s ruff check . # lint (line length 100) python scripts/build_artwork.py # regenerate all SVG artwork python scripts/fetch_vendor.py # verify vendored JS against the lockfile @@ -35,9 +35,10 @@ redesign, not a tweak. committed under `web/static/vendor/`. 2. **Nothing loads from a CDN at runtime.** A self-hosted tool must work offline and must not report page views to a third party. -3. **No hard-coded colours outside `tokens.css`.** Every colour, space and - radius resolves through a CSS variable. That is what makes a new theme one - new block rather than an audit of every stylesheet. +3. **No hard-coded values outside `tokens.css`.** Every colour, space, radius + and control height resolves through a CSS variable. `--control-h` is why + buttons, inputs and selects line up: they all take their height from it, so + a mixed row is flush by construction rather than by nudging. 4. **Additive-only schema changes.** SQLite only, no Alembic. `init_db()` runs `db/migrations.py:sync_schema()`, which creates missing tables *and* adds missing columns by diffing the models against the database. Renames, drops @@ -190,6 +191,27 @@ null in the composer; `files.claim()` binds them, and only unclaimed rows owned by that user, so a forged id cannot pull in someone else's file. Abandoned ones are swept at startup. +**Chats are created lazily.** There is no endpoint that makes an empty chat. +"New chat" is a link to `/chat`, which renders a composer with no row behind +it; `POST /api/chats/start` writes the chat together with its first message. +That is why an opened-and-abandoned chat never appears in the sidebar. Tests +that just need a chat use the `make_chat` fixture rather than the HTTP flow. + +**Route order matters for static path segments.** FastAPI matches in +registration order, so `/admin/models/bulk` must be registered *before* +`/admin/models/{model_id}` or "bulk" is parsed as a model id and 404s. This has +already been a bug once. + +**Pinning is not ordering.** The model picker is always in the administrator's +`position` order. Pinned models get shortcuts in the chat sidebar and nothing +else -- a picker whose order differs from the admin screen is just confusing. + +**System prompts are precedence, not concatenation.** chat > model > instance, +most specific wins outright (`services/chat.py:effective_system_prompt`). +Stacking them reads well in a settings screen and badly in practice: two layers +that disagree give the model contradictory instructions and nobody can tell +which is losing. + **JSON columns need reassignment.** `user.settings_json["theme"] = x` on a plain dict is not detected. The columns use `MutableDict` (`db/types.py`), but the safe habit is `obj.field = {**obj.field, "k": v}`. diff --git a/README.md b/README.md index 95b4d92..6cefbb5 100644 --- a/README.md +++ b/README.md @@ -31,8 +31,10 @@ runtime. Clone it, `pip install -e .`, run it. **Working now** - **Chats** — streaming replies, Markdown with server-side syntax highlighting, - copy and regenerate, automatic chat titles, per-chat system prompt and - sampling settings + copy and regenerate, automatic chat titles. Chats are created when you send + the first message, so an abandoned one never clutters the sidebar +- **System prompts** — instance-wide, per-model and per-chat, with the most + specific winning outright - **Reasoning display** — thinking from reasoning models streams into its own collapsible block, labelled with how long it took, and is never replayed as context diff --git a/src/lembas/api/admin.py b/src/lembas/api/admin.py index ee97289..abbca8a 100644 --- a/src/lembas/api/admin.py +++ b/src/lembas/api/admin.py @@ -62,6 +62,7 @@ async def save_general( user: AdminUser, instance_name: str = Form("LLeMbas"), allow_signup: bool = Form(False), + system_prompt: str = Form(""), ) -> Response: """Save instance settings. @@ -73,6 +74,7 @@ async def save_general( { "instance_name": instance_name.strip()[:120] or "LLeMbas", "allow_signup": allow_signup, + "system_prompt": system_prompt.strip()[:8000], }, ) log.info("registration %s by %s", "opened" if allow_signup else "closed", user.email) diff --git a/src/lembas/api/admin_models.py b/src/lembas/api/admin_models.py index 0e0c406..a73a75e 100644 --- a/src/lembas/api/admin_models.py +++ b/src/lembas/api/admin_models.py @@ -59,12 +59,43 @@ async def models_page(request: Request, db: Db, user: AdminUser, saved: str = "" "groups": list(db.scalars(select(Group).order_by(Group.name))), "connections": list(db.scalars(select(Connection).order_by(Connection.name))), "default_model": settings_store.get(db, "default_model") or "", + "instance_prompt": settings_store.get(db, "system_prompt") or "", "capabilities": CAPABILITIES, "saved": saved, }, ) +# Registered BEFORE /{model_id}: FastAPI matches in registration order, so +# with the parameterised route first, "bulk" is captured as a model id and +# the handler 404s on a model that does not exist. +@router.post("/admin/models/bulk") +async def bulk_models( + db: Db, user: AdminUser, action: str = Form(...), model_ids: list[str] = Form(default=[]) +) -> Response: + """Enable or disable several models at once. + + A freshly refreshed connection can advertise dozens of models; turning them + off one at a time is not a reasonable way to spend an afternoon. + """ + models = list(db.scalars(select(Model).where(Model.id.in_(model_ids or [])))) + for model in models: + if action == "enable": + model.enabled = True + elif action == "disable": + model.enabled = False + elif action == "public": + model.public = True + model.groups = [] + elif action == "private": + model.public = False + db.commit() + _renumber(db) + return RedirectResponse( + f"/admin/models?saved={len(models)}+model(s)+updated.", status_code=303 + ) + + @router.post("/admin/models/{model_id}") async def update_model( db: Db, @@ -72,6 +103,7 @@ async def update_model( model_id: str, display_name: str = Form(""), description: str = Form(""), + system_prompt: str = Form(""), enabled: bool = Form(False), pinned: bool = Form(False), public: bool = Form(False), @@ -82,6 +114,7 @@ async def update_model( model.display_name = display_name.strip()[:300] model.description = description.strip()[:2000] + model.system_prompt = system_prompt.strip()[:8000] model.enabled = enabled model.pinned = pinned model.public = public @@ -165,33 +198,6 @@ async def delete_model_image(db: Db, user: AdminUser, model_id: str) -> Response return RedirectResponse("/admin/models?saved=Image+removed.", status_code=303) -@router.post("/admin/models/bulk") -async def bulk_models( - db: Db, user: AdminUser, action: str = Form(...), model_ids: list[str] = Form(default=[]) -) -> Response: - """Enable or disable several models at once. - - A freshly refreshed connection can advertise dozens of models; turning them - off one at a time is not a reasonable way to spend an afternoon. - """ - models = list(db.scalars(select(Model).where(Model.id.in_(model_ids or [])))) - for model in models: - if action == "enable": - model.enabled = True - elif action == "disable": - model.enabled = False - elif action == "public": - model.public = True - model.groups = [] - elif action == "private": - model.public = False - db.commit() - _renumber(db) - return RedirectResponse( - f"/admin/models?saved={len(models)}+model(s)+updated.", status_code=303 - ) - - # --- Serving model images ---------------------------------------------------- @router.get("/uploads/models/{filename}") async def model_image(user: RequiredUser, filename: str) -> Response: diff --git a/src/lembas/api/chats.py b/src/lembas/api/chats.py index a825183..25fbc32 100644 --- a/src/lembas/api/chats.py +++ b/src/lembas/api/chats.py @@ -42,9 +42,18 @@ def _owned_chat(db: DBSession, chat_id: str, user_id: str) -> Chat: return chat -@router.post("", dependencies=[Depends(require_permission("chat.create"))]) -async def create_chat(db: Db, user: RequiredUser, folder_id: str = Form("")) -> Response: - chosen = chat_service.default_model(db, user) +def _new_chat(db: DBSession, user: User, *, folder_id: str = "", model_id: str = "") -> Chat: + """Create a chat row, resolving which model it should use.""" + chosen = None + if model_id: + match = next( + (m for m in chat_service.available_models(db, user) if m.model_id == model_id), None + ) + if match is not None: + chosen = (match.model_id, match.connection_id) + if chosen is None: + chosen = chat_service.default_model(db, user) + chat = Chat( user_id=user.id, folder_id=folder_id or None, @@ -53,14 +62,48 @@ async def create_chat(db: Db, user: RequiredUser, folder_id: str = Form("")) -> ) db.add(chat) db.commit() + return chat + + +@router.post("/start", dependencies=[Depends(require_permission("chat.create"))]) +async def start_chat( + db: Db, + user: RequiredUser, + content: str = Form(""), + file_ids: list[str] = Form(default=[]), + folder_id: str = Form(""), + model_id: str = Form(""), +) -> Response: + """Create a chat from its first message. + + Chats are made here rather than by a "New chat" button so that an opened- + and-abandoned chat never exists: the row appears only once there is + something in it. The reply then streams the same way as any other, because + /chat/{id} renders the unfinished assistant message with its sse-connect. + """ + content = content.strip() + if not content and not file_ids: + return Response(status_code=status.HTTP_204_NO_CONTENT) + + chat = _new_chat(db, user, folder_id=folder_id, model_id=model_id) + + user_message = chat_service.create_message(db, chat, ROLE_USER, content) + if file_ids: + files_service.claim(db, ids=file_ids, user_id=user.id, message_id=user_message.id) + chat_service.create_message( + db, chat, ROLE_ASSISTANT, "", complete_=False, model_id=chat.model_id + ) - # HX-Redirect rather than a swap: a new chat is a new URL, and the address - # bar has to follow so the chat can be reloaded or bookmarked. response = Response(status_code=status.HTTP_204_NO_CONTENT) response.headers["HX-Redirect"] = f"/chat/{chat.id}" return response +# There is deliberately no route that creates an empty chat. Starting one is +# navigation to /chat (optionally ?model=...), and the row is written by +# /start when the first message is actually sent. + + @router.post("/{chat_id}/messages") async def post_message( request: Request, @@ -103,6 +146,9 @@ async def post_message( "assistant_message": assistant_message, "chat": chat, "user": user, + "models_by_id": { + m.model_id: m for m in chat_service.available_models(db, user) + }, }, ) @@ -281,6 +327,9 @@ async def _generate(chat_id: str, message_id: str) -> AsyncIterator[str]: # template shares both roles, and a missing `user` would only # blow up on whichever branch is not being exercised here. "user": db.get(User, chat.user_id), + "models_by_id": { + m.model_id: m for m in chat_service.available_models(db, None) + }, } ) title_html = templates.get_template("chat/_title_oob.html").render( @@ -432,7 +481,16 @@ async def regenerate( return templates.TemplateResponse( request, "chat/_message.html", - {"request": request, "message": message, "chat": chat, "body_html": "", "user": user}, + { + "request": request, + "message": message, + "chat": chat, + "body_html": "", + "user": user, + "models_by_id": { + m.model_id: m for m in chat_service.available_models(db, user) + }, + }, ) diff --git a/src/lembas/api/pages.py b/src/lembas/api/pages.py index bcb7c9b..3b781d2 100644 --- a/src/lembas/api/pages.py +++ b/src/lembas/api/pages.py @@ -25,15 +25,16 @@ def _chat_context(db: DBSession, user: User, chat: Chat | None) -> dict: """ models = chat_service.available_models(db, user) current = next((m for m in models if m.model_id == chat.model_id), None) if chat else None - pinned = [m for m in models if m.pinned] return { "models": models, - "pinned_models": pinned, - # Excludes the pinned ones: they already have their own optgroup, and - # listing a model twice gives the @@ -43,7 +43,7 @@

-
+
diff --git a/src/lembas/web/templates/admin/general.html b/src/lembas/web/templates/admin/general.html index d8c1d6e..a5884a0 100644 --- a/src/lembas/web/templates/admin/general.html +++ b/src/lembas/web/templates/admin/general.html @@ -26,6 +26,21 @@
+
+

Default system prompt

+

+ Applied to every chat that does not have a prompt of its own. A model's + prompt overrides this, and a chat's prompt overrides both — most specific + wins outright rather than the three being stacked together. +

+
+ + +

Leave empty to send no system prompt at all.

+
+
+

Registration @@ -69,6 +84,6 @@

- +
{% endblock %} diff --git a/src/lembas/web/templates/admin/groups.html b/src/lembas/web/templates/admin/groups.html index ad786e3..7ff3c16 100644 --- a/src/lembas/web/templates/admin/groups.html +++ b/src/lembas/web/templates/admin/groups.html @@ -40,7 +40,7 @@ {% endfor %}
{% endfor %} - +
@@ -49,7 +49,7 @@
-
+ +
+ +
-
+
Deleting a group leaves its members alone.
diff --git a/src/lembas/web/templates/admin/models.html b/src/lembas/web/templates/admin/models.html index cb9c22c..4b4234c 100644 --- a/src/lembas/web/templates/admin/models.html +++ b/src/lembas/web/templates/admin/models.html @@ -8,8 +8,9 @@ {% block admin_content %}

Every model discovered across your connections. The order here is the order - users see. Pinned models are offered first, and the default is what a new chat - starts with. + users see in the picker. Pinning does not reorder anything — + it puts a shortcut in the chat sidebar. The default is what a + new chat starts with.

{% if saved %} @@ -26,12 +27,14 @@
{% else %} - - With selected: - - - - + +
+ With selected: + + + + +
{% for model in models %} @@ -82,7 +85,7 @@ {% for model in models %}
-
+
{{ model_avatar(model, cls="model-row__avatar") }}
@@ -111,6 +114,18 @@ placeholder="What is this model good at?">{{ model.description }}
+
+ + +

+ Used by chats on this model that have no prompt of their own. + {% if instance_prompt %}Leave empty to fall back to the instance prompt. + {% else %}Leave empty to send no system prompt.{% endif %} +

+
+
Capabilities
@@ -138,7 +153,7 @@
@@ -169,12 +184,14 @@ {% endif %}
- +
+ +
-
+
+ enctype="multipart/form-data" class="btn-row"> diff --git a/src/lembas/web/templates/admin/users.html b/src/lembas/web/templates/admin/users.html index 6928aa9..43e82c9 100644 --- a/src/lembas/web/templates/admin/users.html +++ b/src/lembas/web/templates/admin/users.html @@ -16,7 +16,7 @@
{{ icon("check", "icon--sm") }} {{ saved }}
{% endif %} - + @@ -50,7 +50,9 @@ {% endfor %}
- +
+ +
@@ -61,7 +63,7 @@ {% for account in users %}
-
+
{{ account.name }} @@ -135,12 +137,11 @@
{% endif %} - +
-
-
+
+ diff --git a/src/lembas/web/templates/chat/_composer.html b/src/lembas/web/templates/chat/_composer.html new file mode 100644 index 0000000..fb419c7 --- /dev/null +++ b/src/lembas/web/templates/chat/_composer.html @@ -0,0 +1,84 @@ +{% from "_macros.html" import icon %} +{# + The composer, used both inside an existing chat and on /chat where no chat + row exists yet. + + The only difference is where it posts. With a chat, the turn is appended to + the thread in place; without one, /api/chats/start creates the chat and + 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. +#} +
+ {% if can.get("files.upload") %} + + + + {% 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 %} + + + + +
+ +

+ Enter to send, Shift+Enter for a new line. + {% if can.get("files.upload") %} + Drag files in, or paste an image. + {% if current_model and not current_model.capabilities_json.get("vision") %} + {{ current_model.label }} has no vision, so images + will not be sent — documents still will. + {% endif %} + {% endif %} +

+
+ + {% if can.get("files.upload") %} + + {% endif %} +
diff --git a/src/lembas/web/templates/chat/_message.html b/src/lembas/web/templates/chat/_message.html index 090fb46..53d52d0 100644 --- a/src/lembas/web/templates/chat/_message.html +++ b/src/lembas/web/templates/chat/_message.html @@ -1,4 +1,5 @@ -{% from "_macros.html" import icon, mark %} +{% from "_macros.html" import icon, mark, model_avatar %} +{% set speaking_model = (models_by_id | default({})).get(message.model_id) %} {# One message bubble, in either of two states. @@ -22,7 +23,14 @@ diff --git a/src/lembas/web/templates/partials/sidebar.html b/src/lembas/web/templates/partials/sidebar.html index 1454ded..87cacb9 100644 --- a/src/lembas/web/templates/partials/sidebar.html +++ b/src/lembas/web/templates/partials/sidebar.html @@ -1,24 +1,22 @@ -{% from "_macros.html" import icon, brand %} +{% from "_macros.html" import icon, brand, model_avatar %} {# - Sidebar: brand, new chat, the folder tree, then unfiled chats. + Sidebar: brand, new chat, pinned models, the folder tree, then unfiled chats. - Folders render recursively through _folder.html. Chats appear under their - folder, and any chat without one falls to the flat list at the bottom. + "New chat" is a link, not a button that creates a row. The chat is written + when the first message is sent, so opening one and walking away leaves + nothing behind. #} -
+
-
-

Permissions

-

- {% if user.is_admin %} - You are an administrator, so every permission applies. - {% else %} - What this account may do, from the instance baseline plus your groups. + {# --- Models --- #} +

+
+

Default model

+

+ What a new chat starts with. Existing chats keep their model. +

+ {% if models %} +
+
+ +
+ +
+ {% else %} +

No models are available to you yet.

+ {% endif %} +
+ + {% if models %} +
+

Available to you

+

In the order an administrator arranged them.

+
    + {% for model in models %} +
  • +
    + {{ model_avatar(model, cls="nav-item__avatar") }} +
    + {{ model.label }} + {% if model.description %} +
    {{ model.description }}
    + {% endif %} +
    +
    +
    + {% for name, on in (model.capabilities_json or {}).items() %} + {% if on %}{{ name }}{% endif %} + {% endfor %} + {% if model.pinned %}pinned{% endif %} +
    +
    • + {% endfor %} +
+
{% endif %} -

-
- {% for key, granted in can.items() %} - - {{ key }}{{ '' if granted else ' — no' }} - - {% endfor %} -
- {% if user.groups %} -

- Groups: {% for group in user.groups %}{{ group.name }}{% if not loop.last %}, {% endif %}{% endfor %} -

- {% endif %} -
+
-
-

Appearance

-
- - -
-

- Moria is the dark theme, Shire the light one. Your choice is saved - to this browser and to your account. -

-
+ {# --- Appearance --- #} +
+
+

Theme

+

+ Saved to this browser and to your account, so it follows you. +

+
+ + +
+
+
-
-

Change password

-
-
- - + {# --- Security --- #} +
+
+

Change password

+

+ Every other session is signed out when the password changes. + You stay signed in here. +

+ +
+
+ + +
+
+
+ + +

At least 8 characters.

+
+
+ + +
+
+ +
-
- - -

At least 8 characters.

-
-
- - -
- -

- Every other session is signed out when the password changes. You - stay signed in here. -

- -
-
-

Session

-
- -
-
+
+

Session

+
+ +
+
+
+
diff --git a/tests/conftest.py b/tests/conftest.py index 3be1692..5e92b54 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -83,6 +83,43 @@ def registered(client: TestClient) -> dict[str, str]: return credentials +@pytest.fixture +def make_chat(db: Session): + """Create a chat row directly, as scaffolding for other tests. + + Chats are normally created by POST /api/chats/start along with their first + exchange -- there is deliberately no endpoint that makes an empty one. Most + tests want a chat to act on, not that flow, so they get one straight from + the database rather than having to subtract an opening turn from every + assertion. The flow itself is covered in test_chat.py. + """ + from sqlalchemy import select + + from lembas.db.models import Chat, Model, User + + def _create(email: str | None = None, model_id: str | None = None) -> str: + user = ( + db.scalar(select(User).where(User.email == email)) + if email + else db.scalars(select(User).order_by(User.created_at)).first() + ) + model = ( + db.scalar(select(Model).where(Model.model_id == model_id)) + if model_id + else db.scalars(select(Model).order_by(Model.position)).first() + ) + chat = Chat( + user_id=user.id, + model_id=model.model_id if model else "", + connection_id=model.connection_id if model else None, + ) + db.add(chat) + db.commit() + return chat.id + + return _create + + @pytest.fixture def user_id(db: Session, registered: dict[str, str]) -> str: """The registered user's id. diff --git a/tests/test_auth.py b/tests/test_auth.py index 35da345..7049188 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -101,7 +101,9 @@ def test_htmx_requests_get_a_redirect_header_not_a_login_page( ): """An htmx request must never swap a login form into a fragment of the UI.""" client.post("/auth/logout", follow_redirects=False) - response = client.post("/api/chats", headers={"HX-Request": "true"}) + response = client.post( + "/api/chats/start", data={"content": "hi"}, headers={"HX-Request": "true"} + ) assert response.status_code == 204 assert response.headers["HX-Redirect"] == "/auth/login" diff --git a/tests/test_chat.py b/tests/test_chat.py index 2dbdfa1..bd71c25 100644 --- a/tests/test_chat.py +++ b/tests/test_chat.py @@ -144,22 +144,61 @@ def _add_connection(db) -> Connection: return connection -def test_new_chat_redirects_to_its_own_url(client: TestClient, db, registered): +# --- Starting a chat --------------------------------------------------------- +def test_starting_a_chat_creates_it_and_redirects(client: TestClient, db, registered): _add_connection(db) - response = client.post("/api/chats", headers={"HX-Request": "true"}) + response = client.post("/api/chats/start", data={"content": "Hello there"}) assert response.status_code == 204 assert response.headers["HX-Redirect"].startswith("/chat/") + chat = db.scalar(select(Chat)) + assert chat.model_id == "test-model" + messages = db.scalars(select(Message).order_by(Message.created_at)).all() + assert [m.role for m in messages] == ["user", "assistant"] + assert messages[0].content == "Hello there" -def test_new_chat_picks_up_the_default_model(client: TestClient, db, registered): + +def test_starting_with_nothing_creates_no_chat(client: TestClient, db, registered): + """The whole point of lazy creation: an abandoned composer leaves nothing.""" _add_connection(db) - client.post("/api/chats", headers={"HX-Request": "true"}) + assert client.post("/api/chats/start", data={"content": " "}).status_code == 204 + assert db.scalar(select(Chat)) is None + + +def test_visiting_the_chat_page_creates_nothing(client: TestClient, db, registered): + _add_connection(db) + assert client.get("/chat").status_code == 200 + assert db.scalar(select(Chat)) is None + + +def test_starting_a_chat_honours_the_requested_model(client: TestClient, db, registered): + """The pinned-model shortcuts pass ?model=, which arrives here.""" + connection = _add_connection(db) + db.add(Model(connection_id=connection.id, model_id="other-model", position=5)) + db.commit() + + client.post("/api/chats/start", data={"content": "hi", "model_id": "other-model"}) + assert db.scalar(select(Chat)).model_id == "other-model" + + +def test_starting_a_chat_ignores_a_model_you_cannot_reach(client: TestClient, db, registered): + connection = _add_connection(db) + db.add(Model(connection_id=connection.id, model_id="secret", position=5, public=False)) + db.commit() + + client.post("/auth/logout", follow_redirects=False) + client.post( + "/auth/register", + data={"name": "Sam", "email": "sam@shire.test", "password": "potatoes-po-ta-toes"}, + follow_redirects=False, + ) + client.post("/api/chats/start", data={"content": "hi", "model_id": "secret"}) assert db.scalar(select(Chat)).model_id == "test-model" -def test_posting_a_message_stores_both_turns(client: TestClient, db, registered): +def test_posting_a_message_stores_both_turns(client: TestClient, db, registered, make_chat): _add_connection(db) - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat() response = client.post(f"/api/chats/{chat_id}/messages", data={"content": "Hello there"}) assert response.status_code == 200 @@ -174,16 +213,18 @@ def test_posting_a_message_stores_both_turns(client: TestClient, db, registered) assert "sse-connect" in response.text -def test_empty_message_is_ignored(client: TestClient, db, registered): +def test_empty_message_is_ignored(client: TestClient, db, registered, make_chat): _add_connection(db) - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat() assert client.post(f"/api/chats/{chat_id}/messages", data={"content": " "}).status_code == 204 assert db.scalar(select(Message)) is None -def test_a_chat_belonging_to_someone_else_is_not_found(client: TestClient, db, registered): +def test_a_chat_belonging_to_someone_else_is_not_found( + client: TestClient, db, registered, make_chat +): _add_connection(db) - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat() client.post("/auth/logout", follow_redirects=False) client.post( @@ -195,9 +236,9 @@ def test_a_chat_belonging_to_someone_else_is_not_found(client: TestClient, db, r assert client.get(f"/chat/{chat_id}").status_code == 404 -def test_renaming_a_chat_stops_it_being_auto_titled(client: TestClient, db, registered): +def test_renaming_a_chat_stops_it_being_auto_titled(client: TestClient, db, registered, make_chat): _add_connection(db) - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat() client.patch(f"/api/chats/{chat_id}", data={"title": "My own title"}) chat = db.get(Chat, chat_id) @@ -206,9 +247,9 @@ def test_renaming_a_chat_stops_it_being_auto_titled(client: TestClient, db, regi assert chat.title_generated is True -def test_deleting_a_chat_removes_its_messages(client: TestClient, db, registered): +def test_deleting_a_chat_removes_its_messages(client: TestClient, db, registered, make_chat): _add_connection(db) - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat() client.post(f"/api/chats/{chat_id}/messages", data={"content": "Hello"}) client.delete(f"/api/chats/{chat_id}") @@ -216,13 +257,13 @@ def test_deleting_a_chat_removes_its_messages(client: TestClient, db, registered assert db.scalar(select(Message)) is None -def test_deleting_a_folder_keeps_the_chats_inside_it(client: TestClient, db, registered): +def test_deleting_a_folder_keeps_the_chats_inside_it(client: TestClient, db, registered, make_chat): """Losing a conversation to a mis-clicked folder delete is unforgivable.""" _add_connection(db) client.post("/api/folders", data={"name": "Quests"}) folder = db.scalar(select(Folder)) - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat() client.patch(f"/api/chats/{chat_id}", data={"folder_id": folder.id}) client.delete(f"/api/folders/{folder.id}") @@ -296,10 +337,10 @@ def test_system_prompt_leads_the_message_list(db, user_id): def test_streaming_reports_an_unreachable_endpoint_in_the_thread( client: TestClient, db, registered -): +, make_chat): """A failed turn must never be an unexplained blank bubble.""" _add_connection(db) # points at 127.0.0.1:1, which refuses connections - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat() client.post(f"/api/chats/{chat_id}/messages", data={"content": "Hello"}) message = db.scalar(select(Message).where(Message.role == "assistant")) diff --git a/tests/test_files.py b/tests/test_files.py index 11789db..354ad5e 100644 --- a/tests/test_files.py +++ b/tests/test_files.py @@ -83,7 +83,7 @@ def pdf_bytes(pages: list[str]) -> bytes: @pytest.fixture -def chat_with_model(client: TestClient, db, registered): +def chat_with_model(client: TestClient, db, registered, make_chat): """A chat whose model has vision turned on.""" connection = Connection(name="T", base_url="http://127.0.0.1:1", api_key_encrypted=encrypt("")) db.add(connection) @@ -96,8 +96,7 @@ def chat_with_model(client: TestClient, db, registered): ) ) db.commit() - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] - return chat_id + return make_chat() # --- Type detection and processing ------------------------------------------- @@ -326,7 +325,9 @@ def test_a_truly_empty_message_is_still_ignored(client: TestClient, db, chat_wit assert db.scalar(select(Message)) is None -def test_you_cannot_attach_someone_elses_file(client: TestClient, db, chat_with_model): +def test_you_cannot_attach_someone_elses_file( + client: TestClient, db, chat_with_model, make_chat +): """A forged id must not pull another user's file into a conversation.""" client.post("/api/files", files={"file": ("mine.txt", b"secret", "text/plain")}) stolen = db.scalar(select(Attachment)) @@ -337,7 +338,7 @@ def test_you_cannot_attach_someone_elses_file(client: TestClient, db, chat_with_ data={"name": "Sam", "email": "sam@shire.test", "password": "potatoes-po-ta-toes"}, follow_redirects=False, ) - their_chat = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + their_chat = make_chat(email="sam@shire.test") client.post( f"/api/chats/{their_chat}/messages", data={"content": "gimme", "file_ids": [stolen.id]}, diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 9dd2108..aa54f47 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -99,7 +99,7 @@ def test_a_group_can_grant_back_what_the_baseline_removed(db, plain_user): # --- Enforcement through the API --------------------------------------------- def test_creating_a_chat_is_refused_without_permission(client: TestClient, db, plain_user): settings_store.update(db, {"default_permissions": {"chat.create": False}}) - assert client.post("/api/chats").status_code == 403 + assert client.post("/api/chats/start", data={"content": "hi"}).status_code == 403 assert db.scalar(select(Chat)) is None @@ -108,19 +108,21 @@ def test_folder_routes_are_refused_without_permission(client: TestClient, db, pl assert client.post("/api/folders", data={"name": "Nope"}).status_code == 403 -def test_changing_sampling_is_refused_without_permission(client: TestClient, db, plain_user): +def test_changing_sampling_is_refused_without_permission( + client: TestClient, db, plain_user, make_chat +): _model(db, "test-model") - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat(email="sam@shire.test") response = client.patch(f"/api/chats/{chat_id}", data={"temperature": "0.9"}) assert response.status_code == 403 -def test_sampling_is_allowed_once_a_group_grants_it(client: TestClient, db, plain_user): +def test_sampling_is_allowed_once_a_group_grants_it(client: TestClient, db, plain_user, make_chat): _model(db, "test-model") db.add(Group(name="Tuners", permissions_json={"chat.params": True}, users=[plain_user])) db.commit() - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat(email="sam@shire.test") assert client.patch(f"/api/chats/{chat_id}", data={"temperature": "0.9"}).status_code == 204 chat = db.get(Chat, chat_id) @@ -134,10 +136,10 @@ def test_sampling_is_allowed_once_a_group_grants_it(client: TestClient, db, plai ) def test_out_of_range_parameters_are_dropped_not_clamped( client: TestClient, db, registered, field, value -): +, make_chat): """Silently changing what someone typed is worse than ignoring it.""" _model(db, "test-model") - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat() client.patch(f"/api/chats/{chat_id}", data={field: value}) chat = db.get(Chat, chat_id) @@ -145,9 +147,9 @@ def test_out_of_range_parameters_are_dropped_not_clamped( assert field not in (chat.params_json or {}) -def test_an_empty_parameter_clears_it(client: TestClient, db, registered): +def test_an_empty_parameter_clears_it(client: TestClient, db, registered, make_chat): _model(db, "test-model") - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat() client.patch(f"/api/chats/{chat_id}", data={"temperature": "0.7"}) client.patch(f"/api/chats/{chat_id}", data={"temperature": ""}) @@ -187,12 +189,14 @@ def test_disabled_models_are_hidden_from_everyone(db, registered): assert permissions.models_visible_to(db, admin_user) == [] -def test_switching_to_an_inaccessible_model_is_refused(client: TestClient, db, plain_user): +def test_switching_to_an_inaccessible_model_is_refused( + client: TestClient, db, plain_user, make_chat +): """The picker is not the security boundary; a crafted request must fail.""" _model(db, "open-model", public=True) _model(db, "secret-model", public=False) - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat(email="sam@shire.test") response = client.patch(f"/api/chats/{chat_id}", data={"model_id": "secret-model"}) assert response.status_code == 403 @@ -201,17 +205,21 @@ def test_switching_to_an_inaccessible_model_is_refused(client: TestClient, db, p assert chat.model_id == "open-model" -def test_model_select_permission_is_required_to_switch(client: TestClient, db, plain_user): +def test_model_select_permission_is_required_to_switch( + client: TestClient, db, plain_user, make_chat +): _model(db, "a-model", public=True) _model(db, "b-model", public=True) settings_store.update(db, {"default_permissions": {"chat.model_select": False}}) - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat(email="sam@shire.test") assert client.patch(f"/api/chats/{chat_id}", data={"model_id": "b-model"}).status_code == 403 # --- Ordering and defaults --------------------------------------------------- -def test_pinned_models_sort_first(db, registered): +def test_pinning_does_not_reorder_the_picker(db, registered): + """Pinning is a sidebar shortcut. A picker whose order silently differs from + the admin screen is just confusing.""" connection = _connection(db) db.add_all( [ @@ -222,8 +230,8 @@ def test_pinned_models_sort_first(db, registered): db.commit() admin_user = db.scalar(select(User).where(User.role == "admin")) assert [m.model_id for m in chat_service.available_models(db, admin_user)] == [ - "favourite", "ordinary", + "favourite", ] @@ -254,7 +262,7 @@ def test_instance_default_model_is_used_for_new_chats(client: TestClient, db, re db.commit() settings_store.update(db, {"default_model": "chosen"}) - client.post("/api/chats") + client.post("/api/chats/start", data={"content": "hi"}) assert db.scalar(select(Chat)).model_id == "chosen" @@ -270,7 +278,7 @@ def test_a_users_own_default_beats_the_instance_default(client: TestClient, db, settings_store.update(db, {"default_model": "instance-pick"}) client.post("/api/preferences/default-model", data={"model_id": "my-pick"}) - client.post("/api/chats") + client.post("/api/chats/start", data={"content": "hi"}) assert db.scalar(select(Chat)).model_id == "my-pick" @@ -346,7 +354,7 @@ def test_deactivating_a_user_revokes_their_sessions(client: TestClient, db, regi assert other.get("/chat", follow_redirects=False).status_code == 303 -def test_a_pinned_model_appears_once_in_the_picker(client: TestClient, db, registered): +def test_a_pinned_model_appears_once_in_the_picker(client: TestClient, db, registered, make_chat): """Two options with the same value, both selected, is not a picker.""" connection = _connection(db) db.add_all( @@ -357,7 +365,115 @@ def test_a_pinned_model_appears_once_in_the_picker(client: TestClient, db, regis ) db.commit() - chat_id = client.post("/api/chats").headers["HX-Redirect"].rsplit("/", 1)[1] + chat_id = make_chat() page = client.get(f"/chat/{chat_id}").text assert page.count('