From 9c61e40662fa5e9760a65b0155428230f988b223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Bene=C5=A1?= Date: Tue, 4 Aug 2026 08:19:17 +0200 Subject: [PATCH] Two kinds of work, and a switch to say which The sidebar rendered an agent chat and an ordinary one identically, in one list, so hours of machine work sat among a morning's questions. A switch below the pinned models now shows one kind at a time, stored on the account so it follows the reader to another browser. Three things it does that are not the obvious version: The switch is inside the fragment it swaps. Targeting only the tree would leave the two buttons showing the side you had just left -- the request works and the interface says otherwise, which is the failure this codebase keeps cataloguing. A folder can be emptied by the filter, or have been empty all along, and only the first is a reason to hide it. `shown_in` is that line: a folder somebody made a moment ago and has not filled yet stays on both sides, or it can never be found again, let alone filed into. With agent chats switched off there is no switch, and the sidebar goes back to showing everything rather than to one side of a fork nobody can move. An administrator turning the feature off would otherwise strand whoever last left the switch on Agents in an empty sidebar with no way out. The control reuses the composer's `.segmented`, which is the same choice in a different place, and the verb goes on the input rather than the wrapper. Co-Authored-By: Claude Opus 5 (1M context) --- src/lembas/api/pages.py | 72 ++++-- src/lembas/api/preferences.py | 35 +++ src/lembas/db/models/chat.py | 44 +++- src/lembas/web/static/css/chat.css | 5 + src/lembas/web/templates/chat/_composer.html | 13 +- .../web/templates/partials/_chat_link.html | 5 +- .../web/templates/partials/_folder.html | 10 +- .../web/templates/partials/_sidebar_tree.html | 76 +++++++ .../web/templates/partials/sidebar.html | 30 +-- tests/test_sidebar_split.py | 213 ++++++++++++++++++ 10 files changed, 455 insertions(+), 48 deletions(-) create mode 100644 src/lembas/web/templates/partials/_sidebar_tree.html create mode 100644 tests/test_sidebar_split.py diff --git a/src/lembas/api/pages.py b/src/lembas/api/pages.py index b86635f..f3a9c89 100644 --- a/src/lembas/api/pages.py +++ b/src/lembas/api/pages.py @@ -8,7 +8,7 @@ from sqlalchemy import select from sqlalchemy.orm import Session as DBSession from lembas.api.deps import Db, RequiredUser -from lembas.db.models import Chat, Folder, KnowledgeBase, Message, User +from lembas.db.models import KIND_CHAT, KINDS, Chat, Folder, KnowledgeBase, Message, User from lembas.security import permissions from lembas.services import audio as audio_service from lembas.services import chat as chat_service @@ -209,6 +209,18 @@ def _terminal_enabled(db: DBSession, user: User, chat: Chat | None, profile) -> return ssh_service.available() == "" +def sidebar_kind(user: User) -> str: + """Which side of the sidebar's switch this user last chose. + + One resolver, because the page, the fragment route and the switch's own + pressed state all have to agree about it. Anything unrecognised -- an older + release's value, a hand-edited row -- reads as ordinary chats rather than + showing an empty sidebar nobody can explain. + """ + chosen = (user.settings_json or {}).get("sidebar_kind") + return chosen if chosen in KINDS else KIND_CHAT + + def sidebar_context(db: DBSession, user: User) -> dict: """Folder tree plus the chats that belong to no folder. @@ -217,29 +229,50 @@ def sidebar_context(db: DBSession, user: User) -> dict: Only root folders are queried; children come through the relationship and render recursively in the template. + + Everything is narrowed to one `Chat.kind`. A folder the filter has emptied + is dropped here rather than in the template, so the "Folders" heading cannot + appear above nothing -- the same reason `visible_chats` moved off the + template in the first place. `shown_in` is what draws that line: a folder + that was empty to begin with is kept, on both sides. """ - folders = list( - db.scalars( + # With the switch absent the sidebar goes back to showing everything, rather + # than to one side of a fork nobody can move. An administrator turning agent + # chats off would otherwise strand whoever last left the switch on Agents in + # a sidebar that is empty with no way out of it. + split = permissions.has(db, user, "agent.ssh") and bool( + settings_store.agents(db).get("enabled") + ) + kind = sidebar_kind(user) if split else "" + + folders = [ + folder + for folder in db.scalars( select(Folder) .where(Folder.user_id == user.id, Folder.parent_id.is_(None)) .order_by(Folder.position, Folder.name) ) + if folder.shown_in(kind) + ] + narrowed = select(Chat).where( + Chat.user_id == user.id, + Chat.folder_id.is_(None), + Chat.archived.is_(False), + Chat.temporary.is_(False), ) + if kind: + narrowed = narrowed.where(Chat.kind == kind) unfiled = list( - db.scalars( - select(Chat) - .where( - Chat.user_id == user.id, - Chat.folder_id.is_(None), - Chat.archived.is_(False), - Chat.temporary.is_(False), - ) - .order_by(Chat.pinned.desc(), Chat.updated_at.desc()) - ) + db.scalars(narrowed.order_by(Chat.pinned.desc(), Chat.updated_at.desc())) ) return { "folders": folders, "unfiled_chats": unfiled, + "sidebar_kind": kind, + # Whether the switch is worth showing at all. A two-way switch with one + # useful side is worse than no switch: it offers a view that is empty by + # construction and cannot be made otherwise. + "sidebar_split": split, "can": permissions.resolve(db, user), } @@ -315,14 +348,22 @@ async def offline(request: Request) -> Response: @router.get("/chat") async def chat_index( - request: Request, db: Db, user: RequiredUser, model: str = "", temporary: bool = False + request: Request, + db: Db, + user: RequiredUser, + model: str = "", + temporary: bool = False, + kind: str = "", ): """A composer with no chat behind it yet. `?model=` preselects one, which is how the pinned shortcuts work without creating a row for a chat that may never be sent. `?temporary=1` is the same idea for the temporary flag: it lives in the URL rather than in - JavaScript, so it survives a reload and can be bookmarked. + JavaScript, so it survives a reload and can be bookmarked. `?kind=agent` + is how the sidebar's Agent side opens a new chat already on that side -- + a preselection like the other two, not a decision: the kind is still + chosen on the screen and still fixed only when the first message is sent. """ context = _chat_context(db, user, None) @@ -350,6 +391,7 @@ async def chat_index( **context, "current_model": preselected, "starting_temporary": temporary, + "starting_kind": kind if kind in KINDS else KIND_CHAT, "suggestions": suggestions_service.visible(db), **sidebar_context(db, user), }, diff --git a/src/lembas/api/preferences.py b/src/lembas/api/preferences.py index c268421..8465ba3 100644 --- a/src/lembas/api/preferences.py +++ b/src/lembas/api/preferences.py @@ -76,6 +76,41 @@ async def set_layout(db: Db, user: RequiredUser, widths: dict = Body(...)) -> di return {"ok": True, "layout": kept} +@router.post("/sidebar-kind") +async def set_sidebar_kind( + request: Request, db: Db, user: RequiredUser, kind: str = Form("") +) -> Response: + """Switch the sidebar between ordinary chats and agent chats. + + Saves and re-renders in one round trip, because the two cannot be allowed to + disagree: a switch that stored a choice and left the tree showing the other + side would look broken, and re-rendering without storing would lose it on the + next navigation. The tree comes back as a fragment rather than an `HX-Refresh` + -- a full reload is what `api/folders.py` does for a structural change, and it + would throw away the folder open/closed state on every flick of the switch, + which is the same thing `/api/chats/unread` avoids by swapping out of band. + + An unrecognised value is refused rather than stored: `sidebar_kind` reads it + back as "chat" anyway, so storing it would be a preference that silently + does nothing. + """ + from lembas.api.pages import sidebar_context + from lembas.db.models import KINDS + from lembas.web.templating import templates + + if kind not in KINDS: + return Response(status_code=status.HTTP_400_BAD_REQUEST) + + user.settings_json = {**(user.settings_json or {}), "sidebar_kind": kind} + db.commit() + + return templates.TemplateResponse( + request, + "partials/_sidebar_tree.html", + {"chat": None, "user": user, **sidebar_context(db, user)}, + ) + + @router.post("/default-model") async def set_default_model( db: Db, user: RequiredUser, model_id: str = Form("") diff --git a/src/lembas/db/models/chat.py b/src/lembas/db/models/chat.py index 10d00c4..359f24c 100644 --- a/src/lembas/db/models/chat.py +++ b/src/lembas/db/models/chat.py @@ -57,8 +57,7 @@ class Folder(UUIDPrimaryKey, Timestamps, Base): parent: Mapped[Folder | None] = relationship(back_populates="children", remote_side="Folder.id") chats: Mapped[list[Chat]] = relationship(back_populates="folder") - @property - def visible_chats(self) -> list[Chat]: + def visible_chats(self, kind: str = "") -> list[Chat]: """The chats in this folder that belong in the sidebar. The relationship itself stays unfiltered -- back-population needs every @@ -68,13 +67,52 @@ class Folder(UUIDPrimaryKey, Timestamps, Base): existed. The unfiled list has always filtered them (api/pages.py); the folder branch went through the relationship and filtered nothing. + `kind` narrows to one side of the sidebar's Chat/Agent switch. Empty + means both, which is what every caller outside the sidebar wants. + Ordered like the unfiled list: pinned first, then most recently touched. """ - kept = [chat for chat in self.chats if not chat.archived and not chat.temporary] + kept = [ + chat + for chat in self.chats + if not chat.archived and not chat.temporary and (not kind or chat.kind == kind) + ] kept.sort(key=lambda chat: chat.updated_at, reverse=True) kept.sort(key=lambda chat: not chat.pinned) return kept + def visible_children(self, kind: str = "") -> list[Folder]: + """Sub-folders the sidebar should show on this side of the switch. + + Here rather than in the template because Jinja's `selectattr` names a + test, it does not call a method -- so the filter would have to be spelled + out as a loop appending to a list, in a template that already includes + itself recursively. + """ + return [child for child in self.children if child.shown_in(kind)] + + def holds(self, kind: str = "") -> bool: + """Whether anything of this kind is anywhere under this folder. + + Recursive, because a folder's only matching chat may be three levels + down and judging on its own contents alone would bury it. + """ + if self.visible_chats(kind): + return True + return any(child.holds(kind) for child in self.children) + + def shown_in(self, kind: str = "") -> bool: + """Whether this folder belongs on one side of the sidebar's switch. + + Two different reasons a folder can have nothing in it, and only one of + them is a reason to hide it. A folder full of ordinary chats is noise on + the Agent side and is dropped. A folder that is empty of *everything* is + a container somebody just made and has not filled yet -- hiding that one + means it can never be found again, let alone filed into, so it shows on + both sides and says "Empty" for itself. + """ + return self.holds(kind) or not self.holds() + def __repr__(self) -> str: return f"" diff --git a/src/lembas/web/static/css/chat.css b/src/lembas/web/static/css/chat.css index ca6837d..667ef73 100644 --- a/src/lembas/web/static/css/chat.css +++ b/src/lembas/web/static/css/chat.css @@ -1248,6 +1248,11 @@ outline: 2px solid var(--accent); outline-offset: 1px; } +/* The sidebar's copy fills its column rather than sitting at its content + width: it is the heading for everything below it, not a control in a row. */ +.segmented--grow { display: flex; margin: 0 0 var(--sp-3); } +.segmented--grow .segmented__option { flex: 1; } +.segmented--grow .segmented__option span { flex: 1; justify-content: center; } /* --- A plan, and the way to carry it out ----------------------------------- */ .plan { diff --git a/src/lembas/web/templates/chat/_composer.html b/src/lembas/web/templates/chat/_composer.html index f6dafec..b687cd1 100644 --- a/src/lembas/web/templates/chat/_composer.html +++ b/src/lembas/web/templates/chat/_composer.html @@ -275,15 +275,22 @@ #} {% if not chat and agent_profiles %}
- + {# Seeded from `?kind=`, which is how the sidebar's Agent side opens + this screen already on the right fork. `ui.js` reads the checked + radio when it wires the picker, so the hidden field and the + revealed connection follow from this and nothing else. #} +
diff --git a/src/lembas/web/templates/partials/_chat_link.html b/src/lembas/web/templates/partials/_chat_link.html index 673ce79..fae2045 100644 --- a/src/lembas/web/templates/partials/_chat_link.html +++ b/src/lembas/web/templates/partials/_chat_link.html @@ -6,7 +6,10 @@
- {{ icon("chat", "icon--sm") }} + {# The kind, in the one place a person looks. Legible even with the switch + off, which is what it is for: a chat that can run commands should not look + like one that cannot. #} + {{ icon("terminal" if chat_item.kind == "agent" else "chat", "icon--sm") }} {{ chat_item.title }} {# Toggled out of band by the unread poll; see /api/chats/unread. #} {# Bound once: the loop and the "Empty" check must be looking at the same list, or a folder holding only archived chats claims to be empty while - showing them. #} - {% set listed = folder.visible_chats %} + showing them. Narrowed by the sidebar's switch, so a folder shows one + kind at a time -- a folder is free to hold both. #} + {% set listed = folder.visible_chats(sidebar_kind | default("")) %} + {% set shown = folder.visible_children(sidebar_kind | default("")) %} - {% for child in folder.children %} + {% for child in shown %} {% with folder = child %} {% include "partials/_folder.html" %} {% endwith %} @@ -45,7 +47,7 @@ {% include "partials/_chat_link.html" %} {% endfor %} - {% if not folder.children and not listed %} + {% if not shown and not listed %}

Empty

{% endif %}
diff --git a/src/lembas/web/templates/partials/_sidebar_tree.html b/src/lembas/web/templates/partials/_sidebar_tree.html new file mode 100644 index 0000000..0fd6f9c --- /dev/null +++ b/src/lembas/web/templates/partials/_sidebar_tree.html @@ -0,0 +1,76 @@ +{% from "_macros.html" import icon %} +{# + The Chat/Agent switch, the folder tree and the unfiled chats. + + Its own partial because it is rendered from two places: the sidebar on every + page, and `POST /api/preferences/sidebar-kind` when the switch is flicked. + Pinned models stay above it in `sidebar.html` -- they start a chat of either + kind and are not part of what is being switched. + + The switch is INSIDE the swapped fragment on purpose. Targeting only the tree + would leave the two buttons showing the side you just left, which is the same + class of failure as a control whose verb goes somewhere the event does not: + the request works and the interface says otherwise. Each button keeps a stable + id so htmx puts focus back on the one that was pressed. + + `sidebar_kind` is narrowed already: `sidebar_context` drops a folder holding + nothing of this kind at any depth, so the heading below cannot appear above + nothing. It is still passed down to `_folder.html`, which needs it for its own + contents and for the children it recurses into. +#} + diff --git a/src/lembas/web/templates/partials/sidebar.html b/src/lembas/web/templates/partials/sidebar.html index 2932345..e97c16d 100644 --- a/src/lembas/web/templates/partials/sidebar.html +++ b/src/lembas/web/templates/partials/sidebar.html @@ -14,8 +14,12 @@ {% if can.get("chat.create") or can.get("folder.manage") %}
{% if can.get("chat.create") %} - - {{ icon("plus", "icon--sm") }} New chat + {# Carries the side the switch is on, so "New chat" on the Agent side opens + the new-chat screen already set to an agent chat. #} + + {{ icon("plus", "icon--sm") }} + {{ "New agent chat" if sidebar_kind == "agent" else "New chat" }} {% endif %} {% if can.get("folder.manage") %} @@ -33,7 +37,7 @@ -
{% endif %} - {% if folders %} -
-
Folders
- {% for folder in folders %} - {% include "partials/_folder.html" %} - {% endfor %} -
- {% endif %} - -
-
Chats
- {% if unfiled_chats %} - {% for chat_item in unfiled_chats %} - {% include "partials/_chat_link.html" %} - {% endfor %} - {% else %} -

No chats yet. The road begins here.

- {% endif %} -
+ {% include "partials/_sidebar_tree.html" %}
diff --git a/tests/test_sidebar_split.py b/tests/test_sidebar_split.py new file mode 100644 index 0000000..67dac25 --- /dev/null +++ b/tests/test_sidebar_split.py @@ -0,0 +1,213 @@ +"""The sidebar's Chat/Agent switch: what it stores, and what it hides.""" + +from __future__ import annotations + +from fastapi.testclient import TestClient +from sqlalchemy import select + +from lembas.db.models import Chat, Connection, Folder, Model, User +from lembas.services import settings_store +from lembas.services.crypto import encrypt + + +def _enable_agents(db) -> None: + """The split only applies when agent chats are possible. + + With them off the sidebar deliberately goes back to showing everything, so + a test that did not do this would be asserting against the un-split + behaviour and passing for the wrong reason. + """ + settings_store.update(db, {"enabled": True}, key=settings_store.AGENTS) + + +def _add_connection(db) -> Connection: + connection = Connection( + name="Test", base_url="http://127.0.0.1:1", api_key_encrypted=encrypt("") + ) + db.add(connection) + db.commit() + db.add(Model(connection_id=connection.id, model_id="test-model")) + db.commit() + return connection + + +def _chat(db, *, title: str, kind: str = "chat", folder: Folder | None = None) -> Chat: + user = db.scalars(select(User).order_by(User.created_at)).first() + chat = Chat( + user_id=user.id, + title=title, + kind=kind, + model_id="test-model", + folder_id=folder.id if folder else None, + ) + db.add(chat) + db.commit() + return chat + + +def _folder(db, name: str, parent: Folder | None = None) -> Folder: + user = db.scalars(select(User).order_by(User.created_at)).first() + folder = Folder(user_id=user.id, name=name, parent_id=parent.id if parent else None) + db.add(folder) + db.commit() + return folder + + +# --- What the switch stores --------------------------------------------------- +def test_the_switch_stores_the_choice_and_returns_the_tree(client: TestClient, db, registered): + response = client.post("/api/preferences/sidebar-kind", data={"kind": "agent"}) + assert response.status_code == 200 + # The fragment, not a redirect and not a full page: the folder open/closed + # state must survive a flick of the switch. + assert 'id="sidebar-tree"' in response.text + assert "" not in response.text.lower() + + user = db.scalars(select(User).order_by(User.created_at)).first() + db.refresh(user) + assert user.settings_json["sidebar_kind"] == "agent" + + +def test_an_unknown_kind_is_refused_rather_than_stored(client: TestClient, db, registered): + """`sidebar_kind` reads anything unrecognised back as "chat", so storing it + would be a preference that silently does nothing.""" + assert client.post("/api/preferences/sidebar-kind", data={"kind": "wizard"}).status_code == 400 + + user = db.scalars(select(User).order_by(User.created_at)).first() + db.refresh(user) + assert "sidebar_kind" not in (user.settings_json or {}) + + +def test_the_switch_survives_a_page_load(client: TestClient, db, registered): + _add_connection(db) + _enable_agents(db) + client.post("/api/preferences/sidebar-kind", data={"kind": "agent"}) + page = client.get("/chat").text + assert 'id="sidebar-kind-agent"' in page + # The Agent side is what "New chat" opens on, so the fork is already picked. + assert 'href="/chat?kind=agent"' in page + + +# --- What each side shows ------------------------------------------------------ +def test_each_side_shows_only_its_own_kind(client: TestClient, db, registered): + _add_connection(db) + _enable_agents(db) + _chat(db, title="An ordinary question") + _chat(db, title="A machine errand", kind="agent") + + page = client.get("/chat").text + assert "An ordinary question" in page + assert "A machine errand" not in page + + client.post("/api/preferences/sidebar-kind", data={"kind": "agent"}) + page = client.get("/chat").text + assert "A machine errand" in page + assert "An ordinary question" not in page + + +def test_a_folder_of_the_other_kind_is_hidden(client: TestClient, db, registered): + _add_connection(db) + _enable_agents(db) + folder = _folder(db, "Errands") + _chat(db, title="A machine errand", kind="agent", folder=folder) + + # Chat side: the folder holds something, but nothing of this kind. + assert "Errands" not in client.get("/chat").text + + client.post("/api/preferences/sidebar-kind", data={"kind": "agent"}) + page = client.get("/chat").text + assert "Errands" in page + assert "A machine errand" in page + + +def test_a_folder_matching_three_levels_down_is_shown(client: TestClient, db, registered): + """Judging a folder on its own contents alone would bury it.""" + _add_connection(db) + _enable_agents(db) + top = _folder(db, "Top") + middle = _folder(db, "Middle", parent=top) + bottom = _folder(db, "Bottom", parent=middle) + _chat(db, title="A machine errand", kind="agent", folder=bottom) + + client.post("/api/preferences/sidebar-kind", data={"kind": "agent"}) + page = client.get("/chat").text + assert "Top" in page + assert "Middle" in page + assert "Bottom" in page + assert "A machine errand" in page + + +def test_an_empty_folder_shows_on_both_sides(client: TestClient, db, registered): + """Two reasons a folder can look empty, and only one is a reason to hide it. + + A folder the filter emptied is noise. A folder that was empty to begin with + is a container somebody just made -- hiding that one means it can never be + found again, let alone filed into. + """ + _add_connection(db) + _enable_agents(db) + _folder(db, "Waiting") + + assert "Waiting" in client.get("/chat").text + client.post("/api/preferences/sidebar-kind", data={"kind": "agent"}) + assert "Waiting" in client.get("/chat").text + + +def test_a_folder_holding_both_kinds_shows_the_right_half(client: TestClient, db, registered): + _add_connection(db) + _enable_agents(db) + folder = _folder(db, "Mixed") + _chat(db, title="An ordinary question", folder=folder) + _chat(db, title="A machine errand", kind="agent", folder=folder) + + page = client.get("/chat").text + assert "Mixed" in page + assert "An ordinary question" in page + assert "A machine errand" not in page + + client.post("/api/preferences/sidebar-kind", data={"kind": "agent"}) + page = client.get("/chat").text + assert "Mixed" in page + assert "A machine errand" in page + assert "An ordinary question" not in page + + +# --- Whether the switch appears at all ----------------------------------------- +def test_the_switch_is_absent_when_agent_chats_are_off(client: TestClient, db, registered): + """A two-way switch with one useful side is worse than no switch: it offers + a view that is empty by construction and cannot be made otherwise.""" + _add_connection(db) + settings_store.update(db, {"enabled": False}, key=settings_store.AGENTS) + + page = client.get("/chat").text + assert 'id="sidebar-kind-agent"' not in page + + +def test_the_switch_is_present_when_agent_chats_are_on(client: TestClient, db, registered): + _add_connection(db) + settings_store.update(db, {"enabled": True}, key=settings_store.AGENTS) + + page = client.get("/chat").text + assert 'id="sidebar-kind-agent"' in page + assert 'id="sidebar-kind-chat"' in page + + +# --- The row itself ------------------------------------------------------------ +def test_an_agent_chat_is_marked_in_the_row(client: TestClient, db, registered): + """Legible with the switch off too: a chat that can run commands should not + look like one that cannot.""" + _add_connection(db) + _enable_agents(db) + _chat(db, title="A machine errand", kind="agent") + client.post("/api/preferences/sidebar-kind", data={"kind": "agent"}) + + page = client.get("/chat").text + assert "#i-terminal" in page + + +# --- The verb goes where the event does ---------------------------------------- +def test_the_switch_posts_at_a_route_that_serves_post(client: TestClient, db, registered): + """Asserted as a refusal as well as a success. A control wired to a method + its route does not serve fails silently -- htmx surfaces nothing, so the + interface looks exactly like one that works.""" + assert client.get("/api/preferences/sidebar-kind").status_code == 405 + assert client.post("/api/preferences/sidebar-kind", data={"kind": "chat"}).status_code == 200