diff --git a/CHANGELOG.md b/CHANGELOG.md index 54e45a5..85e947a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,56 @@ for 1.0.0 have something to be assembled from. ## Unreleased +## 0.9.10 + +**The second audit pass: screens that were harder to use than they needed to +be.** Checked by rendering them in a real browser and measuring, not by reading +the CSS. + +- Fixed: **the Prompts admin page put its reference material first.** The + Variables legend and the Preview run to a screen each and sat above the tabs, + so the editor — the thing the page is for — started two screens down and every + tab switch had to move the whole page to be any use. On a short tab it could + not move far enough and left the panel stranded above a screenful of nothing. + The editor comes first now, the reference after, and the tab bar stays put: + measured, it moved 385→642px between tabs before and does not move at all now. + The tab bar also sticks to the top, so a long panel does not scroll it away. +- Fixed: **custom themes were three fixed slots.** A fresh instance opened on + fifty-seven empty colour boxes under three identical headings, and a fourth + theme could not be made at all. Now: one block per theme you have, plus one + blank to add the next, with the colours behind a disclosure — so a theme is a + name and a starting point until you ask for more. Up to twelve. The page is + half the height it was. +- Fixed: **deleting a chat left every file it held on disk.** The rows went — + the message, the attachments, the generated images — and the files they named + stayed, with nothing that would ever look at them again. Four of the five ways + a chat can end had this: the delete button, a schedule's task chat, a helper's + hidden chat, and deleting an account. There is one function that deletes a + chat now, and it removes the files first. +- Fixed, and it is what made the above invisible: **a file attached before the + chat existed never learned which chat it belonged to.** Anything picked on the + new-chat screen kept an empty `chat_id` for the rest of its life. Six things + filter on that, so for those files the model was not told they were attached, + the canvas would not open them, and the cleanup could not find them. +- **Folders can be nested, which the README has always claimed.** The route has + handled it since folders existed — cycle guard, depth limit — and the sidebar + has always drawn a tree; there was simply no control that could ask for it. + Moving a folder also respects the depth limit now, which only creating one did. +- The Proxmox container installs the **update helper by default**. A container + made thirty seconds ago to run one thing is not the shared host the plain + installer has to be careful about, and an appliance you cannot update without + a shell is one nobody updates. `INSTALL_UPDATE_HELPER=0` opts out. Docker + deliberately has no equivalent: updating a container is pulling an image, and + a helper inside one would need the Docker socket, which is root on the host. +- The starting points on the new-chat screen are four new ones, aimed at + somebody who has just stood an instance up and wants to know what is behind + it. Only a fresh install gets them; an instance that has already seeded keeps + whatever its administrator has made of the list. +- `README.md` describes what this actually is again — schedules, reports, + helpers, image generation, semantic search, quotas, sharing, branding and the + updates page were all missing, and two things listed as *planned* had shipped. + It gained sections on Docker, the Proxmox container and updating. + ## 0.9.9 **The first of five audit passes before 1.0.0** — everything that landed between diff --git a/README.md b/README.md index 5de7073..60de15f 100644 --- a/README.md +++ b/README.md @@ -99,18 +99,49 @@ runtime. Clone it, `pip install -e .`, run it. - **Model settings** — searchable, filterable list with a page per model: ordering, pinned models, an instance default and a per-user default, custom names, descriptions and images. Scales to hundreds of models +- **Things that happen because time passed** — say "every Monday at nine" and a + model can set it up itself, against the same recurrence rule the manual form + uses. A run can file a **report** you read later, send you a message, or work + on in a chat of its own. The reply says the timing back in words, which is the + one moment anybody can check that Monday was understood as Monday +- **News that finds you** — a dot in the sidebar, a count in the tab title while + you are looking elsewhere, and **web push** so a schedule firing at seven in + the morning reaches a browser that is shut. Opt-in per device +- **Helpers** — a reply can hand a self-contained piece of work to another model + that runs on its own and reports back, several at once, so research fans out + instead of queueing. A helper cannot ask questions, cannot send helpers of its + own, and on a machine runs only a fixed list of read-only commands +- **Drawing** — point it at a ComfyUI and a model can make images, against + workflow templates and defaults you set: size, steps, sampler, scheduler, + checkpoint. It reviews its own result and can try again +- **Semantic search** — pick an embedding model and library search fuses keyword + and meaning, so *"how do I get paid"* finds a document that says *"invoicing"*. + Choosing none is not a degraded mode: it is byte-for-byte the keyword search + that was always there, with nothing written and no requests made - **Users, groups & permissions** — per-group grants that union rather than - override, and model access restricted to chosen groups + override, model access restricted to chosen groups, read and write split for + notes, memory and skills, and a screen that answers *"what can this account + actually do?"* by naming where each permission came from +- **Quotas** — monthly tokens, concurrent replies, agent wall clock, images a + day, helpers a reply. Resolved by maximum across a person's groups, with zero + meaning *no limit* +- **Sharing** — hand a document, a note, a skill or a report to a group or a + person, read-only, with a *Shared with me* filter in every listing +- **Make it yours** — name, tagline, logo, favicon and launcher icons; the + Middle-earth wording is editable data; custom **themes** defined as a set of + colours rather than a stylesheet, and global CSS overrides - **Accounts** — first account becomes the administrator, argon2 password hashing, revocable server-side sessions, self-service password change, admin-managed accounts -- **Admin settings** — open or close registration from the UI, stored in the - database and effective immediately -- **Two themes** — *Moria* (dark) and *Shire* (light), switchable per user +- **Admin settings** — registration, upload and extraction limits, prompt + fragments, and an **Updates** page showing what is running, what is available + and what changed between +- **Two themes and your own** — *Moria* (dark), *Shire* (light), and as many + more as you care to define **Planned** -Image generation · OCR for scanned PDFs · semantic search in the library. +OCR for scanned PDFs · conversation branching · chat export · archived chats. See [PLAN.md](PLAN.md) for what is built, what is not, and why. @@ -322,6 +353,80 @@ lembas secret-key # generate a value for LEMBAS_SECRET_KEY lembas create-admin # create or promote an administrator ``` +## Running it somewhere + +Three ways, all in this repository. + +### Docker + +```bash +export LEMBAS_SECRET_KEY="$(lembas secret-key)" # required; there is no default +docker compose up -d +``` + +One stage, no build step, non-root. The image bakes **no secret key, no data and +no `.git`** — a key inside an image is one every copy shares, and rotating it +makes stored API keys unreadable. Data lives in a named volume on `/data`. + +`docker-compose.yml` publishes on `127.0.0.1` and expects a TLS proxy in front: +the service worker and the microphone both require HTTPS or localhost, so plain +http on a LAN address is a constraint rather than a preference. One replica, and +that is deliberate — the generation registry, the terminal sessions and the +schedule ticker are all in-process, so two would mean every schedule firing +twice. + +**Updating a container is pulling a new image**, and `/admin/updates` says so +rather than offering a button: + +```bash +docker compose pull && docker compose up -d +``` + +There is deliberately no in-container update helper. The one the other install +paths use restarts a systemd service; the equivalent here would be a process +inside the container reaching the Docker socket to replace the container it is +running in — which is root on the host, granted to anybody who can administer +the web interface. The image is the unit of deployment, and that is the whole +point of it. + +### A machine of its own + +`deploy/` holds a systemd unit, an nginx vhost, and install/update scripts. Every +template is parameterised and substituted at install time, so nothing +host-specific is committed here. See [deploy/README.md](deploy/README.md). + +`deploy/lxc-install.sh` creates an unprivileged Proxmox container and runs that +same installer inside it — a wrapper around what already works rather than a +second install path: + +```bash +CTID=140 SITE_HOST=chat.example ./deploy/lxc-install.sh +``` + +The container gets the **update helper by default**, unlike a bare +`install.sh`. The installer defaults it off because it cannot know what it is +installing onto; a container this script made thirty seconds ago to run one +thing, on a hypervisor you own, is not that host — and an appliance you cannot +update without a shell is one nobody updates. `INSTALL_UPDATE_HELPER=0` opts +out. + +### Updating + +**Admin → Updates** shows the version running, what is available on the channel +this host follows, and the commits between. `stable` is the newest `vX.Y.Z` tag; +`edge` is the branch tip, which is whatever was pushed most recently. + +The button that applies an update is **opt-in**, and that is the design: the +service runs unprivileged and cannot restart itself, so the request is a file +that a systemd `.path` unit picks up and runs as root. It carries no ref and no +channel — pressing it is always "deploy the channel this host was configured +with", never "deploy something else". Install it with +`INSTALL_UPDATE_HELPER=1`; without it the page says so and prints the command to +run by hand. + +Release notes come out of the annotated tag itself, so no forge API is involved +anywhere. + ## How it fits together ``` diff --git a/deploy/lxc-install.sh b/deploy/lxc-install.sh index dd11bb7..48ad8c2 100755 --- a/deploy/lxc-install.sh +++ b/deploy/lxc-install.sh @@ -124,10 +124,25 @@ pct exec "$CTID" -- bash -lc " # The same installer this repository ships, run inside. Everything it decides -- # the service user, the prefix, the unit, the vhost, the self-signed certificate # -- it decides there, so this script has no opinions to keep in step with it. +# +# The update helper is **on by default here**, and only here. `install.sh` +# defaults it off because it cannot know what it is installing onto: on a shared +# or long-lived host, letting anybody who can administer the web interface +# deploy as root is a decision somebody should make on purpose. A container +# created by this script thirty seconds ago is not that host -- it exists to run +# LLeMbas and nothing else, whoever ran this owns the hypervisor, and an +# appliance you cannot update without a shell is an appliance nobody updates. +# +# Set INSTALL_UPDATE_HELPER=0 to opt back out. +INSTALL_UPDATE_HELPER="${INSTALL_UPDATE_HELPER:-1}" +LEMBAS_CHANNEL="${LEMBAS_CHANNEL:-stable}" + echo "== install ==" pct exec "$CTID" -- bash -lc " set -e SITE_HOST='$SITE_HOST' LEMBAS_BRANCH='$BRANCH' REPO_URL='$REPO_URL' \ + INSTALL_UPDATE_HELPER='$INSTALL_UPDATE_HELPER' \ + LEMBAS_CHANNEL='$LEMBAS_CHANNEL' \ bash /tmp/lembas-src/deploy/install.sh " diff --git a/src/lembas/__init__.py b/src/lembas/__init__.py index e3c56de..1e526cb 100644 --- a/src/lembas/__init__.py +++ b/src/lembas/__init__.py @@ -1,3 +1,3 @@ """LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints.""" -__version__ = "0.9.9" +__version__ = "0.9.10" diff --git a/src/lembas/api/admin_branding.py b/src/lembas/api/admin_branding.py index 4865ded..a1e548b 100644 --- a/src/lembas/api/admin_branding.py +++ b/src/lembas/api/admin_branding.py @@ -30,6 +30,14 @@ router = APIRouter(prefix="/admin/customization", tags=["admin-branding"]) MAX_CUSTOM_CSS = 40_000 +# How many custom themes an instance may keep. Not a design limit -- there is +# nothing in `theme_css` that cares -- but the whole set lives in one settings +# row read into a process-level snapshot on every render, and the page offers a +# blank block whenever there is room, so *some* number has to say when to stop +# offering. Twelve is far past what anybody wants and small enough that the +# stylesheet stays a stylesheet. +MAX_THEMES = 12 + def _page(request: Request, db: Db, saved: str = "", error: str = "") -> Response: values = settings_store.get_group(db, branding_service.BRANDING) @@ -55,6 +63,7 @@ def _page(request: Request, db: Db, saved: str = "", error: str = "") -> Respons "tokens": branding_service.THEME_TOKENS, "custom_themes": [t for t in brand.themes if not t.built_in], "bases": [name for name, _, _ in branding_service.BUILT_IN], + "max_themes": MAX_THEMES, "saved": saved, "error": error, }, @@ -167,6 +176,11 @@ async def save_themes(request: Request, db: Db, user: AdminUser) -> Response: every **read**, so a theme written straight into the settings table by hand, or stored by an earlier version, still has to produce a stylesheet that parses. Validating only on save would put that guarantee in the wrong place. + + The indices need not be contiguous and are not renumbered. The page renders + one block per theme plus a blank one, so clearing an id in the middle leaves + a gap -- and a gap is simply an index with no id, which the loop already + skips. Renumbering would be work in aid of nothing. """ form = await request.form() themes = [] @@ -186,6 +200,10 @@ async def save_themes(request: Request, db: Db, user: AdminUser) -> Response: }, } ) + # Enforced here as well as in the template, because the template's job is to + # stop offering and this one's is to stop accepting -- a crafted POST is not + # the page. + themes = themes[:MAX_THEMES] _write(db, {"themes": themes}) log.info("%d custom theme(s) saved by %s", len(themes), user.email) return RedirectResponse( diff --git a/src/lembas/api/admin_users.py b/src/lembas/api/admin_users.py index add11f2..b1c11fb 100644 --- a/src/lembas/api/admin_users.py +++ b/src/lembas/api/admin_users.py @@ -16,6 +16,7 @@ from lembas.db.models import ( ROLE_ADMIN, ROLE_PENDING, ROLE_USER, + Chat, Group, Model, User, @@ -23,6 +24,7 @@ from lembas.db.models import ( from lembas.security import permissions from lembas.security.passwords import hash_password, validate_password from lembas.security.sessions import revoke_all_for_user +from lembas.services import chat as chat_service from lembas.services import settings_store, sharing from lembas.services import usage as usage_service from lembas.web.templating import render @@ -249,6 +251,11 @@ async def delete_user(db: Db, user: AdminUser, user_id: str) -> Response: # still there to be found. sharing.forget_owner(db, target.id) sharing.forget_principal(db, PRINCIPAL_USER, target.id) + # And the same shape a third time: the chats cascade, their attachment rows + # cascade, and every file those rows named stays on disk with nothing left + # that will ever look at it. Before the delete, while the rows still say + # which files they are. + chat_service.delete_chats(db, list(db.scalars(select(Chat).where(Chat.user_id == target.id)))) db.delete(target) db.commit() log.info("%s deleted account %s", user.email, email) diff --git a/src/lembas/api/chats.py b/src/lembas/api/chats.py index bf994f1..e6d98b9 100644 --- a/src/lembas/api/chats.py +++ b/src/lembas/api/chats.py @@ -2098,7 +2098,9 @@ async def delete_chat(db: Db, user: RequiredUser, chat_id: str) -> Response: # there would be nothing left to find it by and a shell would sit open on # somebody's machine until the idle timeout noticed. await terminal_service.close_chat(chat_id) - db.delete(chat) + # Not `db.delete(chat)`: that cascades to the attachment rows and leaves + # every file they name on disk forever. See `chat_service.delete_chats`. + chat_service.delete_chats(db, [chat]) db.commit() response = Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/src/lembas/api/folders.py b/src/lembas/api/folders.py index ac0609e..237ce99 100644 --- a/src/lembas/api/folders.py +++ b/src/lembas/api/folders.py @@ -3,6 +3,7 @@ from __future__ import annotations from fastapi import APIRouter, Depends, Form, HTTPException, Request, Response, status +from sqlalchemy import select from sqlalchemy.orm import Session as DBSession from lembas.api.deps import Db, RequiredUser, require_permission @@ -36,6 +37,62 @@ def _depth_of(db: DBSession, folder: Folder | None) -> int: return depth +def _descendants(db: DBSession, folder: Folder) -> set[str]: + """Every folder under this one, and this one. Bounded by MAX_DEPTH.""" + found = {folder.id} + frontier = [folder.id] + for _ in range(MAX_DEPTH + 1): + if not frontier: + break + children = list( + db.scalars(select(Folder).where(Folder.parent_id.in_(frontier))) + ) + frontier = [c.id for c in children if c.id not in found] + found.update(frontier) + return found + + +def _subtree_height(db: DBSession, folder: Folder) -> int: + """How many levels this folder's own subtree occupies, itself included. + + A move has to consider it: the constraint is on the *deepest leaf* after the + move, not on the folder being dragged. + """ + height = 1 + frontier = [folder.id] + for _ in range(MAX_DEPTH + 1): + children = list( + db.scalars(select(Folder.id).where(Folder.parent_id.in_(frontier))) + ) + if not children: + break + height += 1 + frontier = children + return height + + +def candidate_parents(db: DBSession, user_id: str, folder: Folder) -> list[Folder]: + """Folders this one could be moved into. + + Everything the person owns, minus the folder itself and its own subtree -- + which is the cycle guard in `update_folder` stated as a list rather than as + a refusal. A picker that offers a move the route will reject is a control + that looks like it works. + + Depth is checked at the route rather than filtered here: it depends on how + tall *this* folder's subtree is, and a select that silently omitted a folder + for that reason would be unexplainable from the screen. + """ + blocked = _descendants(db, folder) + return [ + candidate + for candidate in db.scalars( + select(Folder).where(Folder.user_id == user_id).order_by(Folder.name) + ) + if candidate.id not in blocked + ] + + def _refresh_sidebar() -> Response: """Tell the browser to reload so the tree re-renders. @@ -141,6 +198,17 @@ async def update_folder( "A folder cannot be moved inside itself.", ) cursor = db.get(Folder, cursor.parent_id) if cursor.parent_id else None + # And the depth cap, which `create_folder` has always applied and this + # path never did -- moving a three-deep subtree under a six-deep folder + # builds a tree nine deep, which is what MAX_DEPTH exists to keep out of + # the recursive sidebar template. It went unnoticed because nothing in + # the interface could submit `parent_id` at all until now. + subtree = _subtree_height(db, folder) + if new_parent is not None and _depth_of(db, new_parent) + subtree > MAX_DEPTH: + raise HTTPException( + status.HTTP_400_BAD_REQUEST, + f"Folders cannot be nested more than {MAX_DEPTH} deep.", + ) folder.parent_id = new_parent.id if new_parent else None if "collapsed" in form: diff --git a/src/lembas/api/pages.py b/src/lembas/api/pages.py index 23d5120..20cd27d 100644 --- a/src/lembas/api/pages.py +++ b/src/lembas/api/pages.py @@ -606,6 +606,12 @@ async def chat_index( ) +def _candidate_parents(db: DBSession, user_id: str, folder: Folder) -> list[Folder]: + from lembas.api.folders import candidate_parents + + return candidate_parents(db, user_id, folder) + + @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. @@ -631,6 +637,10 @@ async def folder_settings(request: Request, db: Db, user: RequiredUser, folder_i { "folder": folder, "chat": None, + # Imported here rather than at module scope: `api.folders` imports + # `api.deps`, which this module is a peer of, and the pair have been + # kept apart deliberately. + "parents": _candidate_parents(db, user.id, folder), "models": chat_service.available_models(db, user), **_agent_context(db, user, None), **sidebar_context(db, user), diff --git a/src/lembas/services/chat.py b/src/lembas/services/chat.py index b2b9070..5ea1cae 100644 --- a/src/lembas/services/chat.py +++ b/src/lembas/services/chat.py @@ -621,6 +621,37 @@ async def summarise_for_compaction( return raw.strip() +def delete_chats(db: DBSession, chats) -> int: + """Delete chats, and the files their attachments point at. + + **The one way to delete a chat.** `db.delete(chat)` cascades to its messages + and to its attachment *rows*, and leaves every file on disk -- a generated + image, an uploaded PDF, a photo -- with nothing that will ever look at them + again: `sweep_orphans` only considers uploads that were never attached. + + `files_service.remove_files_for_chats` was written for exactly this and was + called from one place, the temporary sweep. The delete button, a schedule's + task chat, a helper's hidden chat and deleting an account all went straight + to `db.delete`, so four of the five ways a chat can end leaked its files. + That is `sharing.forget_principal` again: a helper that exists, is correct, + and is not called on the path that needs it. + + The order matters and is why this is a function rather than a note. The + files have to be unlinked **while the rows still say which they are**, so it + happens before the delete and in the same session. + + Does not commit -- the caller decides, because some of them are deleting + other things in the same transaction. + """ + live = [chat for chat in chats if chat is not None] + if not live: + return 0 + files_service.remove_files_for_chats(db, [chat.id for chat in live]) + for chat in live: + db.delete(chat) + return len(live) + + def sweep_temporary(db: DBSession, older_than: timedelta = TEMPORARY_LIFETIME) -> int: """Delete temporary chats nobody has touched for a day. @@ -652,9 +683,7 @@ def sweep_temporary(db: DBSession, older_than: timedelta = TEMPORARY_LIFETIME) - if not stale: return 0 - files_service.remove_files_for_chats(db, [chat.id for chat in stale]) - for chat in stale: - db.delete(chat) + delete_chats(db, stale) db.commit() log.info("swept %d temporary chat(s)", len(stale)) return len(stale) diff --git a/src/lembas/services/files.py b/src/lembas/services/files.py index 242f586..acbe81b 100644 --- a/src/lembas/services/files.py +++ b/src/lembas/services/files.py @@ -29,7 +29,7 @@ from sqlalchemy import select from sqlalchemy.orm import Session as DBSession from lembas.config import settings -from lembas.db.models import KIND_DOCUMENT, KIND_IMAGE, KIND_TEXT, Attachment +from lembas.db.models import KIND_DOCUMENT, KIND_IMAGE, KIND_TEXT, Attachment, Message log = logging.getLogger(__name__) @@ -621,6 +621,19 @@ def claim(db: DBSession, *, ids: list[str], user_id: str, message_id: str) -> li Only unclaimed attachments belonging to this user are taken, so a stray or forged id cannot pull someone else's file into a conversation. + + **`chat_id` is set here, and it was not.** `POST /api/files` takes one, and + the composer sends it -- but only once a chat exists. A file picked on the + *new-chat* screen is stored before there is a chat to name, so its + `chat_id` stayed NULL for the rest of its life even after the message it + belongs to was sent. Six places filter on that column, and every one of them + was quietly wrong about those files: the harness did not name them among the + attached documents, the canvas refused to open them, and + `remove_files_for_chats` could not find them to delete -- so the temporary + sweep, the one caller it had, was removing nothing. + + Read from the message rather than passed in, so no caller can bind an + attachment to one chat and a message in another. """ if not ids: return [] @@ -634,8 +647,11 @@ def claim(db: DBSession, *, ids: list[str], user_id: str, message_id: str) -> li ) ) ) + message = db.get(Message, message_id) for attachment in pending: attachment.message_id = message_id + if message is not None: + attachment.chat_id = message.chat_id db.commit() return pending diff --git a/src/lembas/services/schedules.py b/src/lembas/services/schedules.py index f8a10b8..d836fe7 100644 --- a/src/lembas/services/schedules.py +++ b/src/lembas/services/schedules.py @@ -221,7 +221,9 @@ def delete(db: DBSession, schedule: Schedule, *, keep_chat: bool = True) -> None if keep_chat: chat.kind = "chat" else: - db.delete(chat) + from lembas.services import chat as chat_service + + chat_service.delete_chats(db, [chat]) db.delete(schedule) db.commit() diff --git a/src/lembas/services/subagent.py b/src/lembas/services/subagent.py index 8c031c1..ca8778e 100644 --- a/src/lembas/services/subagent.py +++ b/src/lembas/services/subagent.py @@ -338,9 +338,14 @@ def _cleanup(chat_id: str, *, keep: bool) -> None: return try: with session_scope() as db: + from lembas.services import chat as chat_service + chat = db.get(Chat, chat_id) if chat is not None: - db.delete(chat) + # A writing helper can generate an image or attach a file, and + # `db.delete` would leave both on disk with the row that named + # them gone. + chat_service.delete_chats(db, [chat]) except Exception: # noqa: BLE001 - tidying up is not the result log.debug("could not remove subagent chat %s", chat_id, exc_info=True) diff --git a/src/lembas/services/suggestions.py b/src/lembas/services/suggestions.py index 26cbdea..f7f15fd 100644 --- a/src/lembas/services/suggestions.py +++ b/src/lembas/services/suggestions.py @@ -27,32 +27,46 @@ MAX_DESCRIPTION = 300 MAX_PROMPT = 4000 # A card is sent the moment it is clicked, so each of these has to work cold -- -# with nothing pasted and nothing typed. They are written to ask for what they -# need, which turns the first reply into the right question rather than a guess -# at material nobody has given yet. +# with nothing pasted and nothing typed. These four are *self-contained* rather +# than question-asking: they are what somebody clicks on an instance they have +# just stood up, to find out whether the thing works and what the model behind +# it is. That is the honest first use, and it is a different job from the task +# cards that preceded them, which opened by asking for material the reader had +# not given. +# +# The name and description are read by somebody who has never seen this +# instance; the prompt is read by the model. They are allowed to differ, and +# here they do -- "Start a session" says what the card is for, and its prompt is +# the bare line that produces a clean opening turn. +# +# Only a fresh install gets these: `SEEDED_KEY` means an instance that has +# already seeded keeps whatever its administrator has since made of the list. # # No Middle-earth flavour: this is functional UI. DEFAULTS: tuple[tuple[str, str, str], ...] = ( ( - "Explain this", - "Get something confusing back in plain language.", - "I want something explained in plain language. Ask me what it is, then " - "give me one sentence summarising it, the details that actually matter, " - "and anything I should watch out for.", + "Start a session", + "Open a fresh conversation with nothing assumed.", + "Initializing a new session.", ), ( - "Draft a reply", - "Turn a message you have received into an answer you can send.", - "I need to reply to a message. Ask me what it says, what tone I want and " - "what outcome I am after, then write a draft I could send as it stands.", + "What can you do?", + "Have the model say which tools, instructions and limits it has.", + "Who are you and what are you capable of? What are your available tools? " + "What do you already know from your instructions? Do you have any set " + "personality?", ), ( - "Find the flaw", - "Have a plan argued with before you commit to it.", - "I want a plan argued with. Ask me what the plan is, then tell me what is " - "most likely to go wrong, what I am assuming without evidence, and what " - "would change your mind. Do not soften it, and do not agree just because " - "I sound confident.", + "Show me some code", + "See how the model writes, in a language of its own choosing.", + "Pick a random programming language and produce a coding example showing " + "your coding capabilities.", + ), + ( + "Tell me something", + "A fact worth knowing, from a subject picked at random.", + "Give me a random fun fact from a random topic of your choosing. " + "Available topics are science, history, sociology, theology etc.", ), ) diff --git a/src/lembas/web/static/css/admin.css b/src/lembas/web/static/css/admin.css index b49f83e..9f9af39 100644 --- a/src/lembas/web/static/css/admin.css +++ b/src/lembas/web/static/css/admin.css @@ -80,6 +80,18 @@ flex: none; overflow-x: auto; scrollbar-width: none; + /* Sticky where the *page* is the scroller, which is the admin layout. The + Tools panel is longer than a screen, so without this the bar scrolls away + and changing tab means scrolling back up to find it. Harmless on the + settings page, where `.tabs__body` scrolls underneath a bar that never + moves anyway. + + It needs the opaque `background` above it already has, or the panel would + show through. `z-index` because a panel's own cards establish stacking + contexts and would otherwise paint over it. */ + position: sticky; + top: 0; + z-index: 1; } .tabs__tab { diff --git a/src/lembas/web/templates/admin/customization.html b/src/lembas/web/templates/admin/customization.html index de3dc6d..46580b2 100644 --- a/src/lembas/web/templates/admin/customization.html +++ b/src/lembas/web/templates/admin/customization.html @@ -123,9 +123,25 @@ {# --- Themes --------------------------------------------------------------- #} {# - Three blocks, always rendered, so adding a theme needs no JavaScript: an empty - id means that block is not a theme. Saving replaces the whole list, which is - what makes removing one a matter of clearing its id. + One block per theme that exists, plus a single blank one to add the next. + + It used to be three blocks, always rendered, whether or not anybody had made a + theme -- so a fresh instance opened on fifty-seven empty colour boxes under + three identical headings, and the fourth theme was unreachable without editing + the template. The spare-block pattern is the no-JavaScript way to do "add + another": fill the blank one, save, and the page comes back with your theme + and a new blank. There is no button to press and nothing to clone, which is + why this still works with scripting off. + + The colours live in a
so a block is a heading and three fields until + you ask for them. Nineteen inputs is the right number to *offer* and the wrong + number to *show*: everything left empty inherits from the base, so most themes + set four or five. + + Saving replaces the whole list, which is what makes removing one a matter of + clearing its id -- and why the blocks may have gaps in their numbering without + anything caring. `save_themes` skips an index with no id and counts from the + keys it was actually sent. #}
@@ -138,11 +154,14 @@ are worked out from the accent, so you do not have to.

- {% for index in range(3) %} + {# The spare block is offered until the cap, so the form cannot grow without + end and the page cannot stop offering one while there is room. #} + {% set slots = custom_themes | length + (1 if custom_themes | length < max_themes else 0) %} + {% for index in range(slots) %} {% set existing = custom_themes[index] if index < custom_themes | length else none %}

- {{ existing.label if existing else "A theme of your own" }} + {{ existing.label or existing.id if existing else "Add a theme" }}

@@ -152,7 +171,11 @@ value="{{ existing.id if existing else '' }}" maxlength="24" pattern="[a-z][a-z0-9-]*" placeholder="dusk">

- Lowercase letters, digits and hyphens. Clear it to remove the theme. + {% if existing %} + Lowercase letters, digits and hyphens. Clear it to remove this theme. + {% else %} + Lowercase letters, digits and hyphens. Give it one to make a theme. + {% endif %}

@@ -172,23 +195,42 @@
-
- {% for name, description in tokens %} -
- - -

--{{ name }}

+ {# Open on a theme that has set something, so an existing override is never + hidden behind a disclosure somebody has to know to open. #} + + + Colours + {% if existing and existing.tokens %} + {{ existing.tokens | length }} set + {% else %} + all inherited + {% endif %} + +
+ {% for name, description in tokens %} +
+ + +

--{{ name }}

+
+ {% endfor %}
- {% endfor %} -
+
{% endfor %} + + {% if custom_themes | length >= max_themes %} +

+ That is {{ max_themes }} themes, which is the limit. Clear one's id and + save to make room for another. +

+ {% endif %}
diff --git a/src/lembas/web/templates/admin/prompts.html b/src/lembas/web/templates/admin/prompts.html index b13c7d7..a1caaba 100644 --- a/src/lembas/web/templates/admin/prompts.html +++ b/src/lembas/web/templates/admin/prompts.html @@ -18,6 +18,77 @@
{{ icon("check", "icon--sm") }} Prompts saved.
{% endif %} +{# + The editor first, the reference after, and that ordering is the fix rather + than a preference. + + The Variables legend and the Preview both run to a screen each, and they used + to sit *above* the tabs -- so the thing this page exists for started two + screens down. Every tab switch then had to move the viewport to be any use, + and `ui.js` did: it put the tab bar at the top of the scroller. On a short + panel the scroller cannot go that far, so the browser clamped to the maximum + and left the panel stranded above a screen of nothing, which reads as a page + that failed to load. That is the empty space; the scrolling was the cause and + not the cure. + + With the tabs near the top there is nothing to scroll past: switching panels + changes what is below the bar and leaves the bar where it is. The reference + cards keep their place in the reading order -- after the work, where you go + to look something up -- and the preview still watches the form through + `hx-include`, which does not care about document order. +#} + +
+
+ {% for key, label, fragments in groups %} + + + {% endfor %} +
+ +
+ {% for key, label, fragments in groups %} +
+ {% for fragment in fragments %} + {% with value = values[fragment.key], overridden = fragment.key in overridden %} + {% include "admin/_prompt_field.html" %} + {% endwith %} + {% endfor %} +
+ {% endfor %} +
+
+ +
+

Length

+
+ + +

+ Everything above is cut off past this. 0 means the built-in + {{ default_harness_chars }}. It is a backstop against a large skill index + or memory list quietly eating the context window, not a budget to tune. +

+
+
+ +
+ + {# + data-confirm-button, not data-confirm: this button acts on its own through + formaction, and confirming the whole form would also catch plain Save. + #} + +
+ + +

Variables

@@ -134,57 +205,6 @@ keyup changed delay:700ms from:#prompt-form">

-
-
-
- {% for key, label, fragments in groups %} - - - {% endfor %} -
- -
- {% for key, label, fragments in groups %} -
- {% for fragment in fragments %} - {% with value = values[fragment.key], overridden = fragment.key in overridden %} - {% include "admin/_prompt_field.html" %} - {% endwith %} - {% endfor %} -
- {% endfor %} -
-
- -
-

Length

-
- - -

- Everything above is cut off past this. 0 means the built-in - {{ default_harness_chars }}. It is a backstop against a large skill index - or memory list quietly eating the context window, not a budget to tune. -

-
-
- -
- - {# - data-confirm-button, not data-confirm: this button acts on its own through - formaction, and confirming the whole form would also catch plain Save. - #} - -
-
-

Tool descriptions

diff --git a/src/lembas/web/templates/folders/edit.html b/src/lembas/web/templates/folders/edit.html index a8dac2a..c0d57da 100644 --- a/src/lembas/web/templates/folders/edit.html +++ b/src/lembas/web/templates/folders/edit.html @@ -64,6 +64,33 @@ For you, not for any model. It is never sent anywhere.

+ {# + Nesting has worked at the route since folders existed -- with a cycle + guard and a depth cap -- and the sidebar template has always + recursed to draw it. Nothing anywhere submitted `parent_id`, so the + README advertised "arbitrarily nested" folders that could not be + nested. This is that control. + + The options are `candidate_parents`, which is the cycle guard stated + as a list rather than as a refusal: a picker offering a move the + route will reject is a control that looks like it works. + #} +
+ + +

+ A folder inside another inherits its system prompt where it has + none of its own. +

+
diff --git a/tests/test_admin_prompts.py b/tests/test_admin_prompts.py index 8d819e8..001b420 100644 --- a/tests/test_admin_prompts.py +++ b/tests/test_admin_prompts.py @@ -2,6 +2,8 @@ from __future__ import annotations +import pathlib + import pytest from fastapi.testclient import TestClient from sqlalchemy import select @@ -49,6 +51,39 @@ def test_the_preview_controls_are_inside_what_the_preview_includes( assert f'name="{name}"' in controls, name +def test_the_editor_comes_before_the_reference(client: TestClient, registered): + """The Variables legend and the Preview run to a screen each and used to sit + above the tabs, so the thing this page exists for started two screens down. + Every tab switch then had to move the viewport to be any use -- and on a + short panel the scroller cannot reach the tab bar, so it clamped to the + bottom and left the panel stranded above a screenful of nothing. + + Asserted as an ordering rather than by reading the CSS, because the position + is the fix: with the tabs near the top there is nothing to scroll past. + """ + page = client.get("/admin/prompts").text + tabs = page.index('class="tabs__bar"') + assert tabs < page.index("Variables") + assert tabs < page.index("Preview") + assert tabs < page.index("Tool descriptions") + + +def test_the_tab_bar_sticks_to_the_top_of_the_scroller(): + """The Tools panel is longer than a screen, so a bar that scrolls away means + changing tab is a scroll back up to find it.""" + css = ( + pathlib.Path(__file__).resolve().parents[1] + / "src/lembas/web/static/css/admin.css" + ).read_text() + bar = css.split(".tabs__bar {", 1)[1].split("}", 1)[0] + assert "position: sticky" in bar + assert "top: 0" in bar + # Opaque, or the panel shows through it; stacked, or a panel's own cards + # paint over it. + assert "background:" in bar + assert "z-index:" in bar + + def test_the_page_is_refused_to_a_plain_user(client: TestClient, plain_user): assert client.get("/admin/prompts").status_code == 403 assert client.post("/admin/prompts", data={}).status_code == 403 diff --git a/tests/test_branding.py b/tests/test_branding.py index e9377d7..206d746 100644 --- a/tests/test_branding.py +++ b/tests/test_branding.py @@ -232,6 +232,80 @@ def test_clearing_an_id_removes_the_theme(db, client, registered): assert "dusk" not in branding.snapshot().theme_ids +def test_the_page_offers_one_blank_theme_and_no_more(db, client, registered): + """It rendered three blocks whether or not anybody had made a theme, so a + fresh instance opened on fifty-seven empty colour boxes under three + identical headings -- and the fourth theme was unreachable without editing + the template. One block per theme plus a single blank one is the + no-JavaScript way to say "add another".""" + page = client.get("/admin/customization").text + assert page.count('name="theme_0_id"') == 1 + assert 'name="theme_1_id"' not in page + + _save_theme(client, theme_0_bg="#123456") + page = client.get("/admin/customization").text + # The saved one, and a fresh blank below it. + assert 'name="theme_1_id"' in page + assert 'name="theme_2_id"' not in page + + +def test_a_fourth_theme_is_reachable(db, client, registered): + """Three was a template constant, and the page is what made it a limit.""" + data = {} + for index, name in enumerate(("one", "two", "three", "four")): + data[f"theme_{index}_id"] = name + data[f"theme_{index}_base"] = "moria" + client.post("/admin/customization/themes", data=data, follow_redirects=False) + + ids = branding.snapshot().theme_ids + assert {"one", "two", "three", "four"} <= set(ids) + + +def test_the_page_stops_offering_at_the_cap(db, client, registered): + from lembas.api.admin_branding import MAX_THEMES + + data = {} + for index in range(MAX_THEMES): + data[f"theme_{index}_id"] = f"t{index}" + data[f"theme_{index}_base"] = "moria" + client.post("/admin/customization/themes", data=data, follow_redirects=False) + + page = client.get("/admin/customization").text + assert f'name="theme_{MAX_THEMES}_id"' not in page + assert "which is the limit" in page + + +def test_the_cap_is_enforced_on_save_as_well_as_offered(db, client, registered): + """The template's job is to stop offering; the route's is to stop accepting. + A crafted POST is not the page.""" + from lembas.api.admin_branding import MAX_THEMES + + data = {} + for index in range(MAX_THEMES + 5): + data[f"theme_{index}_id"] = f"t{index}" + data[f"theme_{index}_base"] = "moria" + client.post("/admin/customization/themes", data=data, follow_redirects=False) + + custom = [t for t in branding.snapshot().themes if not t.built_in] + assert len(custom) == MAX_THEMES + + +def test_a_gap_in_the_middle_does_not_disturb_the_rest(db, client, registered): + """Clearing an id leaves a hole in the numbering, and nothing renumbers.""" + data = {} + for index, name in enumerate(("one", "two", "three")): + data[f"theme_{index}_id"] = name + data[f"theme_{index}_base"] = "moria" + client.post("/admin/customization/themes", data=data, follow_redirects=False) + + data["theme_1_id"] = "" + client.post("/admin/customization/themes", data=data, follow_redirects=False) + + ids = set(branding.snapshot().theme_ids) + assert "one" in ids and "three" in ids + assert "two" not in ids + + # --- The stylesheet ------------------------------------------------------------- def test_custom_css_is_served_as_a_stylesheet(db, client, registered): """A route rather than an inline `