From b602657450b297c7441efbdf2711d357a044fac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Bene=C5=A1?= Date: Tue, 4 Aug 2026 08:30:49 +0200 Subject: [PATCH] A folder that carries something, and a way to name one A folder was a name and nothing else -- and not even that, since PATCH could rename one and nothing in the interface ever called it. It now carries a description, a system prompt, and seeds for the model, the kind and the agent target, with a settings page behind the row. The prompt is a fourth rung on the ladder, chat > folder > model > instance, and it goes above the model deliberately: a model's prompt describes the model wherever it is used, a folder's describes this piece of work whichever model is pointed at it. It is read when a reply is built rather than copied when a chat is made, so editing it reaches the chats already there, and the walk up the parents is bounded and cycle-safe because it runs on the request path. `api/pages.py` mirrors the ladder for the settings panel and had to gain the same rung -- a panel naming the wrong source is worse than one naming none, because it is believed. The seeds fill in what the request left empty and nothing it filled in: the folder says what this work usually needs, the screen in front of somebody says what they want this time. `ssh_profile_id` is a plain string rather than a foreign key, for the reason `compacted_through_id` is, so it is validated on read. Getting *into* a folder needed fixing too. `/api/chats/start` has accepted a folder_id since folders existed and nothing ever sent one, so the only route in was to make the chat elsewhere and move it. There is a New chat here on the row now, and `?folder=` on the new-chat screen. Naming is a themed dialog, and deliberately not htmx's hx-prompt: htmx calls the browser's prompt() synchronously and only then fires htmx:prompt with the answer already in hand, so intercepting the event cannot supply a different one and the grey box appears anyway. `data-prompt` follows the data-confirm-button shape instead -- swallow the click, ask, write the answer into hx-vals, click again behind a guard. JSON.stringify rather than concatenation, or a folder called `"` produces hx-vals that does not parse and the rename silently does nothing. Driven under a DOM stub, and there is a test that no template brings hx-prompt back. Co-Authored-By: Claude Opus 5 (1M context) --- src/lembas/api/chats.py | 19 ++ src/lembas/api/folders.py | 82 ++++- src/lembas/api/pages.py | 64 +++- src/lembas/db/models/chat.py | 22 ++ src/lembas/services/chat.py | 34 +- src/lembas/web/static/js/ui.js | 43 +++ src/lembas/web/templates/chat/_composer.html | 6 + src/lembas/web/templates/folders/edit.html | 163 +++++++++ .../web/templates/partials/_folder.html | 19 ++ .../web/templates/partials/sidebar.html | 8 +- tests/test_folders.py | 311 ++++++++++++++++++ tests/test_ui_js.py | 66 ++++ 12 files changed, 821 insertions(+), 16 deletions(-) create mode 100644 src/lembas/web/templates/folders/edit.html create mode 100644 tests/test_folders.py create mode 100644 tests/test_ui_js.py diff --git a/src/lembas/api/chats.py b/src/lembas/api/chats.py index ad80355..0e355cb 100644 --- a/src/lembas/api/chats.py +++ b/src/lembas/api/chats.py @@ -21,6 +21,7 @@ from lembas.db.models import ( ROLE_ASSISTANT, ROLE_USER, Chat, + Folder, Message, Model, User, @@ -95,7 +96,25 @@ def _new_chat( offered the control -- by which point the model had already answered under the wrong rules. The reasoning effort is accepted for the same reason, and wins over the model's default: an explicit choice beats an inherited one. + + A folder's own defaults fill in anything the request left empty, and nothing + it filled in. That order is the point: the folder says what this piece of + work usually needs, and the screen in front of somebody says what they want + this time. The folder's system prompt is deliberately not among them -- it + is read at request time so that editing the folder later reaches the chats + already in it. """ + folder = db.get(Folder, folder_id) if folder_id else None + if folder is not None and folder.user_id != user.id: + folder = None + if folder is not None: + model_id = model_id or folder.model_id + kind = kind or folder.kind + if kind == KIND_AGENT: + ssh_profile_id = ssh_profile_id or folder.ssh_profile_id + project_dir = project_dir or folder.project_dir + agent_mode = agent_mode or folder.agent_mode + chosen = None if model_id: match = next( diff --git a/src/lembas/api/folders.py b/src/lembas/api/folders.py index 3b8a447..ac0609e 100644 --- a/src/lembas/api/folders.py +++ b/src/lembas/api/folders.py @@ -2,11 +2,12 @@ from __future__ import annotations -from fastapi import APIRouter, Depends, Form, HTTPException, Response, status +from fastapi import APIRouter, Depends, Form, HTTPException, Request, Response, status from sqlalchemy.orm import Session as DBSession from lembas.api.deps import Db, RequiredUser, require_permission -from lembas.db.models import Folder +from lembas.db.models import KINDS, Folder +from lembas.services.agent import policy as agent_policy # Every route here manages folders, so the guard belongs on the router. router = APIRouter( @@ -47,13 +48,26 @@ def _refresh_sidebar() -> Response: return response +def _prompted(request: Request) -> str: + """What somebody typed into an `hx-prompt` dialog, if anything. + + htmx sends it as a header rather than a field, because the element carrying + the attribute may not be a form control at all. `ui.js` swaps the browser's + own prompt for the themed one and hands the answer back through the same + header, so this reads identically either way. + """ + return (request.headers.get("HX-Prompt") or "").strip() + + @router.post("") async def create_folder( + request: Request, db: Db, user: RequiredUser, - name: str = Form("New folder"), + name: str = Form(""), parent_id: str = Form(""), ) -> Response: + name = name.strip() or _prompted(request) parent = _owned_folder(db, parent_id, user.id) if parent_id else None # A cap on nesting, so a runaway client cannot build a tree deep enough to @@ -67,7 +81,7 @@ async def create_folder( db.add( Folder( user_id=user.id, - name=name.strip()[:200] or "New folder", + name=name[:200] or "New folder", parent_id=parent.id if parent else None, ) ) @@ -75,21 +89,47 @@ async def create_folder( return _refresh_sidebar() +# The settings a folder hands to chats started inside it, and how far each may +# run. A table rather than a run of `if` blocks so the save handler and the form +# cannot come to disagree about which fields exist -- the same reasoning the +# tool label table carries. +_SEEDS = { + "description": 500, + "system_prompt": 20_000, + "model_id": 300, + "ssh_profile_id": 32, + "project_dir": 1000, +} + + @router.patch("/{folder_id}") async def update_folder( + request: Request, db: Db, user: RequiredUser, folder_id: str, - name: str | None = Form(None), - parent_id: str | None = Form(None), - collapsed: bool | None = Form(None), ) -> Response: + """Rename, move, collapse, or set what this folder hands to its chats. + + Reads the raw form rather than declaring `Form(None)` parameters, because + FastAPI cannot tell an empty field from an absent one -- a submitted `x=` + arrives as None, so "clear this prompt" and "leave it alone" would be the + same request. Key presence is the distinction, which is the rule + `api/chats.py:update_chat` already follows and the reason every field here + is clearable. + """ folder = _owned_folder(db, folder_id, user.id) + form = await request.form() - if name is not None and name.strip(): - folder.name = name.strip()[:200] + # A rename can arrive from a settings form or from an `hx-prompt` button on + # the folder row; one route serves both. A blank name is ignored rather than + # stored, since a folder nobody can see the name of is one nobody can find. + name = str(form.get("name") or "").strip() or _prompted(request) + if name: + folder.name = name[:200] - if parent_id is not None: + if "parent_id" in form: + parent_id = str(form["parent_id"]).strip() new_parent = _owned_folder(db, parent_id, user.id) if parent_id else None # Reparenting a folder into its own subtree would detach that subtree # from the root and make it unreachable. @@ -103,10 +143,28 @@ async def update_folder( cursor = db.get(Folder, cursor.parent_id) if cursor.parent_id else None folder.parent_id = new_parent.id if new_parent else None - if collapsed is not None: - folder.collapsed = collapsed + if "collapsed" in form: + folder.collapsed = str(form["collapsed"]).lower() in ("1", "true", "on", "yes") + + for field, limit in _SEEDS.items(): + if field in form: + setattr(folder, field, str(form[field]).strip()[:limit]) + + # Both are vocabularies rather than free text, and both accept "" for "no + # opinion". Anything else is dropped rather than stored: a folder seeding a + # kind that is not a kind would hand every chat started in it a value that + # `_new_chat` then has to ignore anyway. + if "kind" in form: + wanted = str(form["kind"]).strip() + folder.kind = wanted if wanted in KINDS else "" + if "agent_mode" in form: + wanted = str(form["agent_mode"]).strip() + folder.agent_mode = wanted if wanted in agent_policy.MODES else "" db.commit() + # One rule for every caller: reload. A rename or a move changes the tree, + # and a save from the settings page comes back showing what was stored -- + # which is what somebody who pressed Save wants to see anyway. return _refresh_sidebar() diff --git a/src/lembas/api/pages.py b/src/lembas/api/pages.py index f3a9c89..1244ff9 100644 --- a/src/lembas/api/pages.py +++ b/src/lembas/api/pages.py @@ -354,6 +354,7 @@ async def chat_index( model: str = "", temporary: bool = False, kind: str = "", + folder: str = "", ): """A composer with no chat behind it yet. @@ -364,14 +365,35 @@ async def chat_index( 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. + `?folder=` is the same again, and is what "New chat here" on a folder row + posts: the chat is filed there, and `_new_chat` fills in whatever the + folder seeds and the screen left empty. """ context = _chat_context(db, user, None) + # Somebody else's folder id in the URL is ignored rather than refused. It + # would only ever get there by hand, and an error page holding a composer + # hostage over a bad query string helps nobody. + starting_folder = db.get(Folder, folder) if folder else None + if starting_folder is not None and starting_folder.user_id != user.id: + starting_folder = None + # A folder that fixes the kind picks the fork, unless the URL already said. + if not kind and starting_folder is not None: + kind = starting_folder.kind + # 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) + # The folder's own model, ahead of the reader's default and behind an + # explicit `?model=`. Same order `_new_chat` applies, so the picker shows + # the model the chat is actually going to be created with -- which matters, + # because the composer decides from it whether to warn about images. + if preselected is None and starting_folder is not None and starting_folder.model_id: + preselected = next( + (m for m in context["models"] if m.model_id == starting_folder.model_id), None + ) if preselected is None: chosen = chat_service.default_model(db, user) if chosen is not None: @@ -392,12 +414,45 @@ async def chat_index( "current_model": preselected, "starting_temporary": temporary, "starting_kind": kind if kind in KINDS else KIND_CHAT, + "starting_folder": starting_folder, "suggestions": suggestions_service.visible(db), **sidebar_context(db, user), }, ) +@router.get("/folders/{folder_id}") +async def folder_settings(request: Request, db: Db, user: RequiredUser, folder_id: str): + """What a folder hands to the chats started inside it. + + A page rather than a row that expands, following the admin convention: a + form per row in a tree that nests eight deep would be unusable, and the + sidebar is the one part of the application that has to stay scannable. + + Guarded by `folder.manage`, the same permission the whole folder router + carries -- editing a folder's system prompt is managing a folder, and a page + that renders for somebody whose save is going to 403 is a trap. + """ + if not permissions.has(db, user, "folder.manage"): + raise HTTPException(status.HTTP_403_FORBIDDEN, "You cannot manage folders.") + + folder = db.get(Folder, folder_id) + if folder is None or folder.user_id != user.id: + raise HTTPException(status.HTTP_404_NOT_FOUND, "That folder no longer exists.") + + return render( + request, + "folders/edit.html", + { + "folder": folder, + "chat": None, + "models": chat_service.available_models(db, user), + **_agent_context(db, user, None), + **sidebar_context(db, user), + }, + ) + + @router.get("/chat/{chat_id}") async def chat_detail(request: Request, db: Db, user: RequiredUser, chat_id: str): chat = db.get(Chat, chat_id) @@ -431,11 +486,18 @@ async def chat_detail(request: Request, db: Db, user: RequiredUser, chat_id: str # What the chat would use if its own prompt were empty, so the settings # panel can show it as placeholder text rather than leaving the user to # guess what "inherited" means. + # + # This mirrors `chat_service.effective_system_prompt` and has to keep + # mirroring it, layer for layer and in the same order -- a panel naming the + # wrong source is worse than one naming none, because it is believed. inherited, inherited_from = "", "" + folder_prompt = chat_service.folder_system_prompt(db, chat) current = next( (m for m in chat_service.available_models(db, user) if m.model_id == chat.model_id), None ) - if current is not None and (current.system_prompt or "").strip(): + if folder_prompt: + inherited, inherited_from = folder_prompt, "folder" + elif current is not None and (current.system_prompt or "").strip(): inherited, inherited_from = current.system_prompt.strip(), "model" else: instance_prompt = (settings_store.get(db, "system_prompt") or "").strip() diff --git a/src/lembas/db/models/chat.py b/src/lembas/db/models/chat.py index 359f24c..f4cff42 100644 --- a/src/lembas/db/models/chat.py +++ b/src/lembas/db/models/chat.py @@ -49,6 +49,28 @@ class Folder(UUIDPrimaryKey, Timestamps, Base): position: Mapped[int] = mapped_column(Integer, default=0, nullable=False) collapsed: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False) + # What chats started in this folder inherit. A folder is where somebody + # groups the work on one thing, so it is the natural place to say "chats + # about this use this prompt, this model, this machine" -- said once rather + # than on every new chat. + description: Mapped[str] = mapped_column(String(500), default="") + # Read at request time, never copied onto the chat: editing the folder later + # has to reach the chats already in it, which is the whole point of putting + # it here. It slots into the ladder between the chat and the model. + system_prompt: Mapped[str] = mapped_column(Text, default="") + + # Seeds, copied onto a new chat and then that chat's own. Empty means "no + # opinion", so a folder can carry a prompt without also dictating a model. + model_id: Mapped[str] = mapped_column(String(300), default="") + kind: Mapped[str] = mapped_column(String(16), default="") + # Deliberately not a ForeignKey. `migrations.py` compiles the column type + # only, so a REFERENCES clause would exist on a fresh database and not on an + # upgraded one -- the same reason `Chat.compacted_through_id` is a plain id. + # The profile may also have been deleted, so it is validated on read. + ssh_profile_id: Mapped[str] = mapped_column(String(32), default="") + project_dir: Mapped[str] = mapped_column(String(1000), default="") + agent_mode: Mapped[str] = mapped_column(String(16), default="") + children: Mapped[list[Folder]] = relationship( back_populates="parent", cascade="all, delete-orphan", diff --git a/src/lembas/services/chat.py b/src/lembas/services/chat.py index e4e49a8..fd79dcb 100644 --- a/src/lembas/services/chat.py +++ b/src/lembas/services/chat.py @@ -149,23 +149,53 @@ def message_payload(message: Message, *, vision: bool) -> dict[str, Any]: return {"role": message.role, "content": parts} +def folder_system_prompt(db: DBSession, chat: Chat) -> str: + """The nearest prompt on the chat's folder, or on a folder above it. + + Walks up rather than reading one level, because folders nest and a project's + prompt belongs on the project rather than on each sub-folder of it. The + nearest one wins, which is the same rule the ladder as a whole follows. + + Bounded and cycle-safe the way `api/folders.py:_depth_of` is. Reparenting + already refuses to build a cycle, but this runs on the request path for + every reply and a row written by something else must not be able to hang it. + """ + from lembas.db.models import Folder + + folder = chat.folder + seen: set[str] = set() + while folder is not None and folder.id not in seen: + seen.add(folder.id) + if (folder.system_prompt or "").strip(): + return folder.system_prompt.strip() + folder = db.get(Folder, folder.parent_id) if folder.parent_id else None + return "" + + def effective_system_prompt(db: DBSession, chat: Chat) -> str: """The system prompt a chat actually runs with. - Three layers, most specific wins outright: + Four layers, most specific wins outright: - chat > model > instance + chat > folder > model > instance Precedence rather than concatenation. Stacking them reads well in a settings screen and badly in practice: the moment two layers disagree the model gets contradictory instructions and nobody can tell which one is losing. With precedence, "why is it behaving like this" has one answer. + + The folder sits above the model because it is the more specific statement: + a model's prompt describes the model wherever it is used, and a folder's + describes this piece of work whichever model is pointed at it. """ from lembas.services import settings_store if chat.system_prompt.strip(): return chat.system_prompt.strip() + if inherited := folder_system_prompt(db, chat): + return inherited + model = db.scalar( select(Model).where(Model.model_id == chat.model_id).order_by(Model.position) ) diff --git a/src/lembas/web/static/js/ui.js b/src/lembas/web/static/js/ui.js index 59667d8..0dc410e 100644 --- a/src/lembas/web/static/js/ui.js +++ b/src/lembas/web/static/js/ui.js @@ -205,6 +205,49 @@ }); }, true); + /* Asking for one line of text before a request goes out: renaming a folder, + renaming a chat. + + Deliberately NOT htmx's own hx-prompt. htmx calls the browser's prompt() + synchronously and only then fires htmx:prompt with the answer already in + hand -- so intercepting the event cannot supply a different one, and the + native box appears regardless. Cancelling the event only aborts the + request. This is the data-confirm-button shape instead: swallow the click, + ask in our own dialog, write the answer where htmx will collect it, and + click again behind a guard flag. + + The answer goes into hx-vals as a normal field rather than into a header, + because every route that wants it already reads a form. htmx reads + attributes when the request is built, so setting it just before the second + click is enough. It is JSON.stringify'd, never concatenated: a folder + called `"` would otherwise produce hx-vals that does not parse, and the + request would go out with the field missing rather than with the name. */ + document.addEventListener("click", function (event) { + var el = event.target.closest("[data-prompt]"); + if (!el || el.dataset.prompted) return; + event.preventDefault(); + event.stopPropagation(); + + var field = el.dataset.promptField || "name"; + prompt({ + title: el.dataset.promptTitle, + message: el.dataset.prompt, + value: el.dataset.promptValue || "", + confirmLabel: el.dataset.promptLabel || "Save", + }).then(function (value) { + /* null is Cancel. An empty string is somebody clearing the box and + pressing Save, which is not a rename either -- the routes ignore a + blank name, so sending it would be a request that does nothing. */ + if (value === null || !String(value).trim()) return; + var values = {}; + values[field] = String(value).trim(); + el.setAttribute("hx-vals", JSON.stringify(values)); + el.dataset.prompted = "1"; + el.click(); + delete el.dataset.prompted; + }); + }, true); + /* Plain forms opt in with data-confirm, so they need no inline onsubmit. */ document.addEventListener("submit", function (event) { var form = event.target; diff --git a/src/lembas/web/templates/chat/_composer.html b/src/lembas/web/templates/chat/_composer.html index b687cd1..18736a1 100644 --- a/src/lembas/web/templates/chat/_composer.html +++ b/src/lembas/web/templates/chat/_composer.html @@ -66,6 +66,12 @@ {% if not chat and starting_temporary %} {% endif %} + {% if not chat and starting_folder %} + {# `/api/chats/start` has accepted a folder_id since folders existed and + nothing ever sent one, so the only way into a folder was to make the + chat elsewhere and move it. This is "New chat here" arriving. #} + + {% endif %} {# The text, with a mirror behind it. diff --git a/src/lembas/web/templates/folders/edit.html b/src/lembas/web/templates/folders/edit.html new file mode 100644 index 0000000..1cf356d --- /dev/null +++ b/src/lembas/web/templates/folders/edit.html @@ -0,0 +1,163 @@ +{% extends "base.html" %} +{% from "_macros.html" import icon %} +{# + What a folder hands to the chats started inside it. + + A page rather than a panel in the sidebar, following the admin convention: + compact rows, and the full form one click away. A form per folder row in a + tree that nests eight deep would be unusable, and the sidebar is the one + place in the application that has to stay scannable. + + The whole form is one PATCH at the route that already existed. Every field is + clearable, because `update_folder` reads the raw form and checks key presence + rather than declaring `Form(None)` parameters -- with those, an empty box and + an absent one are the same request. +#} + +{% block title %}{{ folder.name }} - LLeMbas{% endblock %} + +{% block head %} + + +{% endblock %} + +{% block body_attrs %} data-authenticated="true"{% endblock %} + +{% block body %} +
+ {% include "partials/sidebar.html" %} + +
+
+

+ {{ icon("folder", "icon--sm") }} + {{ folder.name }} +

+ + + {{ icon("plus", "icon--sm") }} New chat here + +
+ +
+
+
+

Name

+
+ +
+
+ + +

+ For you, not for any model. It is never sent anywhere. +

+
+
+ +
+

System prompt

+

+ Used by every chat in this folder, and by folders nested inside it, + unless the chat has a prompt of its own. Read each time a reply is + built rather than copied when a chat is made, so editing this + reaches the chats already here. +

+
+ +

+ Precedence, not concatenation: chat, then folder, then model, then + instance. The most specific one wins outright. +

+
+
+ +
+

What a new chat starts as

+

+ Seeds, copied onto a chat when it is created and its own from then + on. Anything chosen on the new-chat screen wins over these. +

+ +
+ + +
+ +
+ + +

+ A folder set to one kind shows on only that side of the sidebar's + switch, and opens the new-chat screen already on that fork. +

+
+ + {% if agent_profiles %} + {# Only meaningful for an agent chat, and shown regardless of the kind + above: somebody filling this in is on their way to setting the kind + too, and a field that appears only once another field is right is a + field people conclude is missing. #} +
+ + +
+ +
+ + +
+ + {% if agent_modes %} +
+ + +
+ {% endif %} + {% endif %} +
+ +
+ + Back +
+
+
+
+
+{% endblock %} diff --git a/src/lembas/web/templates/partials/_folder.html b/src/lembas/web/templates/partials/_folder.html index e557197..f240476 100644 --- a/src/lembas/web/templates/partials/_folder.html +++ b/src/lembas/web/templates/partials/_folder.html @@ -18,6 +18,25 @@ {{ folder.name }} + {# Start a chat already filed here, and already carrying whatever the + folder seeds. Without this the only way into a folder is to make the + chat somewhere else and move it. #} + + {{ icon("plus", "icon--sm") }} + + + + {{ icon("sliders", "icon--sm") }} + {% endif %} diff --git a/tests/test_folders.py b/tests/test_folders.py new file mode 100644 index 0000000..a465e54 --- /dev/null +++ b/tests/test_folders.py @@ -0,0 +1,311 @@ +"""What a folder carries, and what the chats inside it inherit from it.""" + +from __future__ import annotations + +from fastapi.testclient import TestClient +from sqlalchemy import select + +from lembas.db.models import Chat, Connection, Folder, Model, SshProfile, User +from lembas.services import chat as chat_service +from lembas.services import settings_store +from lembas.services.crypto import encrypt + + +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 _folder(db, name: str, parent: Folder | None = None, **fields) -> 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, **fields + ) + db.add(folder) + db.commit() + return folder + + +# --- Naming and renaming ------------------------------------------------------- +def test_a_folder_is_created_with_the_name_that_was_asked_for( + client: TestClient, db, registered +): + client.post("/api/folders", data={"name": "Isengard"}) + assert db.scalar(select(Folder)).name == "Isengard" + + +def test_a_folder_with_no_name_still_gets_one(client: TestClient, db, registered): + """The button asks first, but a request that arrives without one must not + produce a folder with a blank label nobody can click.""" + client.post("/api/folders", data={}) + assert db.scalar(select(Folder)).name == "New folder" + + +def test_renaming_a_folder(client: TestClient, db, registered): + """PATCH has been able to do this since folders existed and nothing in the + interface called it, so a folder could not be renamed at all.""" + folder = _folder(db, "Isengard") + assert client.patch(f"/api/folders/{folder.id}", data={"name": "Orthanc"}).status_code == 204 + db.refresh(folder) + assert folder.name == "Orthanc" + + +def test_a_blank_rename_is_ignored(client: TestClient, db, registered): + """A folder nobody can see the name of is one nobody can find.""" + folder = _folder(db, "Isengard") + client.patch(f"/api/folders/{folder.id}", data={"name": " "}) + db.refresh(folder) + assert folder.name == "Isengard" + + +# --- The system prompt ladder --------------------------------------------------- +def test_a_folder_prompt_reaches_a_chat_inside_it(client: TestClient, db, registered, make_chat): + _add_connection(db) + folder = _folder(db, "Errands", system_prompt="Answer in the fewest words.") + chat = db.get(Chat, make_chat()) + chat.folder_id = folder.id + db.commit() + + assert chat_service.effective_system_prompt(db, chat) == "Answer in the fewest words." + + +def test_a_nested_folder_inherits_its_parents_prompt( + client: TestClient, db, registered, make_chat +): + """A project's prompt belongs on the project, not on each sub-folder of it.""" + _add_connection(db) + top = _folder(db, "Project", system_prompt="Answer in the fewest words.") + inner = _folder(db, "Notes", parent=top) + chat = db.get(Chat, make_chat()) + chat.folder_id = inner.id + db.commit() + + assert chat_service.effective_system_prompt(db, chat) == "Answer in the fewest words." + + +def test_the_nearest_folder_prompt_wins(client: TestClient, db, registered, make_chat): + _add_connection(db) + top = _folder(db, "Project", system_prompt="From the top.") + inner = _folder(db, "Notes", parent=top, system_prompt="From the sub-folder.") + chat = db.get(Chat, make_chat()) + chat.folder_id = inner.id + db.commit() + + assert chat_service.effective_system_prompt(db, chat) == "From the sub-folder." + + +def test_the_chats_own_prompt_still_wins(client: TestClient, db, registered, make_chat): + _add_connection(db) + folder = _folder(db, "Errands", system_prompt="From the folder.") + chat = db.get(Chat, make_chat()) + chat.folder_id = folder.id + chat.system_prompt = "From the chat." + db.commit() + + assert chat_service.effective_system_prompt(db, chat) == "From the chat." + + +def test_a_folder_prompt_beats_the_models(client: TestClient, db, registered, make_chat): + """The folder is the more specific statement: a model's prompt describes the + model wherever it is used, a folder's describes this piece of work.""" + _add_connection(db) + model = db.scalar(select(Model)) + model.system_prompt = "From the model." + folder = _folder(db, "Errands", system_prompt="From the folder.") + chat = db.get(Chat, make_chat()) + chat.folder_id = folder.id + db.commit() + + assert chat_service.effective_system_prompt(db, chat) == "From the folder." + + +def test_an_empty_folder_prompt_falls_through(client: TestClient, db, registered, make_chat): + _add_connection(db) + model = db.scalar(select(Model)) + model.system_prompt = "From the model." + folder = _folder(db, "Errands") + chat = db.get(Chat, make_chat()) + chat.folder_id = folder.id + db.commit() + + assert chat_service.effective_system_prompt(db, chat) == "From the model." + + +def test_the_panel_names_the_folder_as_the_source(client: TestClient, db, registered, make_chat): + """The settings panel mirrors the ladder and has to keep mirroring it. One + naming the wrong source is worse than one naming none, because it is + believed.""" + _add_connection(db) + folder = _folder(db, "Errands", system_prompt="Answer in the fewest words.") + chat_id = make_chat() + chat = db.get(Chat, chat_id) + chat.folder_id = folder.id + db.commit() + + page = client.get(f"/chat/{chat_id}").text + assert "folder prompt" in page or "the folder" in page + + +# --- Seeds ---------------------------------------------------------------------- +def test_a_folder_seeds_a_new_chats_model(client: TestClient, db, registered): + _add_connection(db) + db.add(Model(connection_id=db.scalar(select(Connection)).id, model_id="other-model")) + db.commit() + folder = _folder(db, "Errands", model_id="other-model") + + client.post("/api/chats/start", data={"content": "Hello", "folder_id": folder.id}) + chat = db.scalar(select(Chat)) + assert chat.model_id == "other-model" + + +def test_an_explicit_choice_beats_the_folders_seed(client: TestClient, db, registered): + """The folder says what this work usually needs; the screen in front of + somebody says what they want this time.""" + _add_connection(db) + db.add(Model(connection_id=db.scalar(select(Connection)).id, model_id="other-model")) + db.commit() + folder = _folder(db, "Errands", model_id="other-model") + + client.post( + "/api/chats/start", + data={"content": "Hello", "folder_id": folder.id, "model_id": "test-model"}, + ) + assert db.scalar(select(Chat)).model_id == "test-model" + + +def test_a_folder_belonging_to_someone_else_seeds_nothing(client: TestClient, db, registered): + _add_connection(db) + other = User(name="Sam", email="sam@shire.test", password_hash="x") + db.add(other) + db.commit() + folder = Folder(user_id=other.id, name="Theirs", model_id="other-model") + db.add(folder) + db.commit() + + client.post("/api/chats/start", data={"content": "Hello", "folder_id": folder.id}) + chat = db.scalar(select(Chat)) + assert chat.model_id == "test-model" + + +def test_a_deleted_connection_on_a_folder_does_not_raise(client: TestClient, db, registered): + """`ssh_profile_id` is a plain string, not a foreign key, so it can outlive + the profile it names. It is validated on read instead.""" + _add_connection(db) + settings_store.update(db, {"enabled": True}, key=settings_store.AGENTS) + folder = _folder(db, "Errands", kind="agent", ssh_profile_id="gone") + + response = client.post( + "/api/chats/start", data={"content": "Hello", "folder_id": folder.id} + ) + assert response.status_code == 204 + # No profile means no agent chat: `_agent_target` refuses rather than + # creating one pointed at nothing. + assert db.scalar(select(Chat)).kind == "chat" + + +def test_the_seeds_are_saved_and_cleared_through_one_route(client: TestClient, db, registered): + """Every field clearable, which is what reading the raw form buys: with + `Form(None)` an empty box and an absent one are the same request.""" + folder = _folder(db, "Errands") + client.patch( + f"/api/folders/{folder.id}", + data={ + "description": "Work on the tower.", + "system_prompt": "Answer in the fewest words.", + "model_id": "test-model", + "kind": "agent", + }, + ) + db.refresh(folder) + assert folder.description == "Work on the tower." + assert folder.system_prompt == "Answer in the fewest words." + assert folder.kind == "agent" + + client.patch(f"/api/folders/{folder.id}", data={"system_prompt": "", "kind": ""}) + db.refresh(folder) + assert folder.system_prompt == "" + assert folder.kind == "" + # Untouched keys are left alone rather than blanked. + assert folder.description == "Work on the tower." + + +def test_a_kind_that_is_not_a_kind_is_dropped(client: TestClient, db, registered): + """A folder seeding a kind that is not a kind hands every chat a value + `_new_chat` then has to ignore anyway.""" + folder = _folder(db, "Errands") + client.patch(f"/api/folders/{folder.id}", data={"kind": "wizard", "agent_mode": "reckless"}) + db.refresh(folder) + assert folder.kind == "" + assert folder.agent_mode == "" + + +# --- Getting into a folder at all ----------------------------------------------- +def test_new_chat_here_files_the_chat(client: TestClient, db, registered): + """`/api/chats/start` has accepted a folder_id since folders existed and + nothing ever sent one.""" + _add_connection(db) + folder = _folder(db, "Errands") + + page = client.get(f"/chat?folder={folder.id}").text + assert f'name="folder_id" value="{folder.id}"' in page + + client.post("/api/chats/start", data={"content": "Hello", "folder_id": folder.id}) + assert db.scalar(select(Chat)).folder_id == folder.id + + +def test_a_folder_fixed_to_agent_opens_the_new_chat_screen_on_that_fork( + client: TestClient, db, registered +): + _add_connection(db) + settings_store.update(db, {"enabled": True}, key=settings_store.AGENTS) + db.add( + SshProfile( + owner_id=db.scalars(select(User).order_by(User.created_at)).first().id, + name="Box", + host="example.test", + username="root", + host_key="ssh-ed25519 AAAA", + ) + ) + db.commit() + folder = _folder(db, "Errands", kind="agent") + + page = client.get(f"/chat?folder={folder.id}").text + assert 'name="kind" value="agent"' in page + + +# --- The settings page ---------------------------------------------------------- +def test_the_settings_page_renders_what_is_stored(client: TestClient, db, registered): + _add_connection(db) + folder = _folder(db, "Errands", system_prompt="Answer in the fewest words.") + + page = client.get(f"/folders/{folder.id}").text + assert "Answer in the fewest words." in page + assert f'hx-patch="/api/folders/{folder.id}"' in page + + +def test_the_settings_page_refuses_someone_elses_folder(client: TestClient, db, registered): + other = User(name="Sam", email="sam@shire.test", password_hash="x") + db.add(other) + db.commit() + folder = Folder(user_id=other.id, name="Theirs") + db.add(folder) + db.commit() + + assert client.get(f"/folders/{folder.id}").status_code == 404 + + +def test_the_settings_form_posts_at_a_route_that_serves_patch( + client: TestClient, db, registered +): + """A control wired to a method its route does not serve fails silently -- + htmx surfaces nothing, so it looks exactly like a control that works.""" + folder = _folder(db, "Errands") + assert client.post(f"/api/folders/{folder.id}", data={"name": "x"}).status_code == 405 + assert client.patch(f"/api/folders/{folder.id}", data={"name": "x"}).status_code == 204 diff --git a/tests/test_ui_js.py b/tests/test_ui_js.py new file mode 100644 index 0000000..c7509f3 --- /dev/null +++ b/tests/test_ui_js.py @@ -0,0 +1,66 @@ +"""Dialogs and toasts, checked without a runtime. + +There is no JavaScript test runner here and hard rule 1 keeps Node out of the +project, so the behaviour is driven by hand under a DOM stub before committing. +What can be pinned in the suite are the invariants the file states about itself +-- and in particular the one that would look like an improvement to somebody +tidying up later. +""" + +from __future__ import annotations + +from pathlib import Path + +import lembas + +ROOT = Path(lembas.__file__).parent +SOURCE = (ROOT / "web/static/js/ui.js").read_text(encoding="utf-8") +TEMPLATES = ROOT / "web/templates" + + +def test_the_dialogs_never_fall_back_to_the_browsers_own(): + """`window.confirm` and `window.prompt` cannot be styled, ignore the theme + and block the tab. Putting one back is the thing this module exists to + prevent.""" + assert "window.confirm(" not in SOURCE + assert "window.prompt(" not in SOURCE + + +def test_no_template_uses_htmx_s_own_prompt(): + """htmx's hx-prompt calls the browser's prompt() *synchronously* and only + then fires htmx:prompt with the answer already in hand -- so intercepting + the event cannot supply a different one, and the native box appears + regardless. `data-prompt` exists because of that, and an hx-prompt slipped + in later would summon the grey box back with nothing to catch it. + + hx-confirm is fine and is used widely: that one fires *before*, and ui.js + intercepts it. + """ + # The attribute, not the word: the comment beside `data-prompt` names + # hx-prompt in order to say why it is not being used. + offenders = [ + path.relative_to(TEMPLATES) + for path in TEMPLATES.rglob("*.html") + if 'hx-prompt="' in path.read_text(encoding="utf-8") + ] + assert not offenders, f"hx-prompt summons window.prompt: {offenders}" + + +def test_the_prompt_answer_is_json_encoded_rather_than_concatenated(): + """A folder called `"` would otherwise produce hx-vals that does not parse, + and htmx would send the request with the field missing rather than with the + name -- a rename that silently does nothing.""" + start = SOURCE.index("[data-prompt]") + block = SOURCE[start : SOURCE.index("data-confirm", start)] + assert "JSON.stringify" in block + + +def test_every_prompt_button_names_a_field_or_takes_the_default(): + """The field name is what the route reads. A button with a field the route + does not look at posts nothing and looks exactly like one that works.""" + for path in TEMPLATES.rglob("*.html"): + text = path.read_text(encoding="utf-8") + if "data-prompt=" not in text: + continue + # Either an explicit field, or the "name" default the handler applies. + assert "data-prompt-field" in text or "/api/folders" in text, path