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