diff --git a/CHANGELOG.md b/CHANGELOG.md index b753402..f6f9e14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,48 @@ for 1.0.0 have something to be assembled from. ## Unreleased +## 1.0.4 + +Six things that looked like they worked. Five of them were found by reading the +code rather than by anybody reporting them, which is what they have in common: +none of these fails loudly, and two of them correct themselves if you reload. + +- Fixed: **a reply lost the model's name and picture the moment it finished.** + While a reply streams it is attributed correctly; at the instant it lands, the + frame that replaces the bubble was looking the models up as nobody, and "no + user" answers "no models" rather than "all models". So a finished reply swapped + the model's avatar for the plain leaf mark, put the instance's name where the + model's should be, and grew a raw model id beside it. Reloading the page put it + all back, which is why this survived a release: it is only ever wrong until you + look away. +- Fixed: **a limit on how many replies an account may write at once could be + stepped over by pressing New chat.** It was enforced when sending into a chat + that already existed and nowhere else — not on a new chat, not on editing an + earlier message, not on sending a queued one, and not on regenerating. Four of + the six ways to start a reply ignored it, including the commonest. +- Fixed: **a custom theme's confirmations and warnings kept the built-in + theme's colour behind them.** Setting `success` or `warning` moved the text and + left the background it sits on, because the faded companion colour was derived + for three of the five settable colours. Visible on every alert and badge of + those two kinds, on the "on" state in the permissions list, and on the added + lines of every diff in an agent chat. +- Fixed: **on a phone, every page with a sidebar could be scrolled past its own + bottom into empty background.** The shell was sized to the part of the screen + you can actually see and the document around it to the part you can see with + the browser's toolbar retracted; the difference between those is real on a + phone and nil on a desktop, which is why it was never noticed on one. Reported + on Settings and true everywhere. A flick that ran off the end of a list now + stops there as well, instead of dragging the page behind it. +- Fixed: **the conversation was rendering every assistant message twice on every + page load** — once into Markdown that nothing read, and once the way it is + actually shown. The same was true of Messages, for your own turns. Nothing + looked wrong; a long conversation was simply slower to open than it needed to + be, every time, along with every rewind and every compaction. +- Fixed: a test file meant to skip itself on a machine without `setsid` never + did, because it set its marker twice and the second one replaced the first. +- Removed: an endpoint serving a message's unrendered Markdown, which nothing + had ever called — the copy button reads the page it is already on. + ## 1.0.3 Two Arch-isms in the installer, both of which only a Debian machine could find. diff --git a/src/lembas/__init__.py b/src/lembas/__init__.py index cb530f2..ca22f10 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__ = "1.0.3" +__version__ = "1.0.4" diff --git a/src/lembas/api/chats.py b/src/lembas/api/chats.py index c8a79c7..127185f 100644 --- a/src/lembas/api/chats.py +++ b/src/lembas/api/chats.py @@ -308,6 +308,12 @@ async def start_chat( if not content and not file_ids: return Response(status_code=status.HTTP_204_NO_CONTENT) + # Before `_new_chat`, not after: a refusal that has already written the row + # leaves an empty chat in the sidebar as the visible result of being told + # no. There is no chat yet to exclude from the count, and none is needed -- + # nothing can be running for a chat that does not exist. + _refuse_extra_reply(db, None, user) + chat = _new_chat( db, user, @@ -981,7 +987,7 @@ def _note_rewind(chat: Chat) -> None: chat.rewound_at = datetime.now(UTC) -def _too_many_replies(db: DBSession, chat: Chat, user: User) -> str: +def _too_many_replies(db: DBSession, chat: Chat | None, user: User) -> str: """Why this account may not start another reply right now, or "". In-process, and that is exact rather than approximate only because this @@ -998,10 +1004,13 @@ def _too_many_replies(db: DBSession, chat: Chat, user: User) -> str: row[0] for row in db.execute(select(Chat.id).where(Chat.user_id == user.id)).all() } + # `chat` is None on the new-chat path, where there is no row yet and so + # nothing to exclude -- every running reply of theirs counts. + here = chat.id if chat is not None else None running = sum( 1 for chat_id in mine - if chat_id != chat.id and generation_service.running_for(chat_id) is not None + if chat_id != here and generation_service.running_for(chat_id) is not None ) if running < ceiling: return "" @@ -1011,6 +1020,22 @@ def _too_many_replies(db: DBSession, chat: Chat, user: User) -> str: ) +def _refuse_extra_reply(db: DBSession, chat: Chat | None, user: User) -> None: + """Raise if this account is already writing as many replies as it may. + + A function rather than two lines repeated, because it is repeated five + times now. It used to be called once -- from `_send`, which serves + `post_message` and `execute_plan` -- while four other routes start a + generation: `start_chat`, `edit_message`, `send_queued_now` and + `regenerate`. So a group's `concurrent_replies` was reached by sending into + a chat that already existed and walked straight past by pressing New chat, + which is the commonest way to start a reply there is. A quota you can step + over by using the obvious button is not a quota. + """ + if busy := _too_many_replies(db, chat, user): + raise HTTPException(status.HTTP_429_TOO_MANY_REQUESTS, busy) + + def _send( request: Request, db: Db, @@ -1042,8 +1067,7 @@ def _send( # This chat's own reply does not count against it -- a second message here # is queued rather than sent, a few lines down, and that path is what the # queue is for. - if busy := _too_many_replies(db, chat, user): - raise HTTPException(status.HTTP_429_TOO_MANY_REQUESTS, busy) + _refuse_extra_reply(db, chat, user) if queued := _reply_in_flight(db, chat): waiting = db.scalar( @@ -1328,8 +1352,16 @@ async def _follow(chat_id: str, message_id: str) -> AsyncIterator[str]: # template shares both roles, and a missing `user` would only # blow up on whichever branch is not being exercised here. "user": owner, + # `owner`, never None. `models_visible_to` answers an absent + # user with [], so a None here is not "every model" but *no* + # model -- and this frame replaces the whole bubble at the + # moment a reply finishes. The template then finds no + # `speaking_model` and the finished reply swaps its avatar for + # the LLeMbas mark, its author for the instance name, and grows + # a raw model_id chip, all of which a reload silently corrects. + # That is why it went unreported for so long. "models_by_id": { - m.model_id: m for m in chat_service.available_models(db, None) + m.model_id: m for m in chat_service.available_models(db, owner) }, # This frame replaces the whole bubble, so it has to carry the # speaker button's conditions too -- and the owner's, not the @@ -1360,7 +1392,7 @@ def _render_bubble(db: DBSession, chat: Chat, owner: User | None, message: Messa "message": message, "chat": chat, "user": owner, - "models_by_id": {m.model_id: m for m in chat_service.available_models(db, None)}, + "models_by_id": {m.model_id: m for m in chat_service.available_models(db, owner)}, **audio_service.template_flags(db, owner), } ) @@ -1447,11 +1479,6 @@ def _thread_context(db: DBSession, chat: Chat, user: User) -> dict: "user": user, "messages": messages, "compacted": compacted, - "bodies": { - m.id: render_markdown(m.content) - for m in everything - if m.role == ROLE_ASSISTANT and m.content - }, "models_by_id": {m.model_id: m for m in chat_service.available_models(db, user)}, **audio_service.template_flags(db, user), } @@ -1560,6 +1587,8 @@ async def edit_message( raise HTTPException( status.HTTP_409_CONFLICT, "Wait for the current reply to finish, or stop it." ) + # `_reply_in_flight` is about *this* chat; the quota is about the account. + _refuse_extra_reply(db, chat, user) message.content = content @@ -1703,6 +1732,7 @@ async def send_queued_now( raise HTTPException( status.HTTP_409_CONFLICT, "Wait for the current reply to finish, or stop it." ) + _refuse_extra_reply(db, chat, user) message.queued = False db.commit() @@ -2123,16 +2153,6 @@ async def delete_chat(db: Db, user: RequiredUser, chat_id: str) -> Response: return response -@router.get("/{chat_id}/messages/{message_id}/raw") -async def raw_message(db: Db, user: RequiredUser, chat_id: str, message_id: str) -> HTMLResponse: - """The unrendered Markdown of a message, for the copy button.""" - _owned_chat(db, chat_id, user.id) - message = db.get(Message, message_id) - if message is None or message.chat_id != chat_id: - raise HTTPException(status.HTTP_404_NOT_FOUND, "That message no longer exists.") - return HTMLResponse(escape_text(message.content)) - - @router.post("/{chat_id}/messages/{message_id}/regenerate") async def regenerate( request: Request, @@ -2147,6 +2167,8 @@ async def regenerate( if message is None or message.chat_id != chat.id or message.role != ROLE_ASSISTANT: raise HTTPException(status.HTTP_404_NOT_FOUND, "That reply no longer exists.") + _refuse_extra_reply(db, chat, user) + message.content = "" message.error = "" message.complete = False diff --git a/src/lembas/api/messages.py b/src/lembas/api/messages.py index 990e2b1..01da782 100644 --- a/src/lembas/api/messages.py +++ b/src/lembas/api/messages.py @@ -21,7 +21,6 @@ from lembas.api.pages import _chat_context, sidebar_context from lembas.db.models import Message, Schedule from lembas.services import messages as messages_service from lembas.services import schedules as schedules_service -from lembas.services.markdown import render_markdown from lembas.services.schedule import clock from lembas.services.schedule import rule as rule_service from lembas.web.templating import render @@ -31,11 +30,6 @@ log = logging.getLogger(__name__) router = APIRouter(tags=["messages"]) -def _bodies(messages: list[Message]) -> dict[str, str]: - """Markdown rendered server-side, keyed by id, as `chat_detail` does.""" - return {m.id: render_markdown(m.content) for m in messages if m.role == "user"} - - @router.get("/messages") async def messages_page(request: Request, db: Db, user: RequiredUser): conversation = messages_service.for_user(db, user) @@ -61,7 +55,6 @@ async def messages_page(request: Request, db: Db, user: RequiredUser): "chat": conversation, "messages": live, "compacted": [], - "bodies": _bodies(live), "inherited_prompt": "", "inherited_from": "", "more_before": bool(live) and messages_service.has_more_before( @@ -109,7 +102,6 @@ async def messages_history( "messages/_history.html", { "messages": page, - "bodies": _bodies(page), "more_before": messages_service.has_more_before(db, conversation, page[0]), "oldest_id": page[0].id, # `render()` injects `user` and friends; `TemplateResponse` does diff --git a/src/lembas/services/branding.py b/src/lembas/services/branding.py index 470557a..b0d8cdf 100644 --- a/src/lembas/services/branding.py +++ b/src/lembas/services/branding.py @@ -462,7 +462,20 @@ def theme_css(theme: Theme) -> str: if not theme.tokens: return "" lines = [f" --{name}: {value};" for name, value in theme.tokens.items()] - for name, alpha in (("accent", "0.14"), ("leaf", "0.14"), ("danger", "0.14")): + # Every settable colour that has a `-soft` companion in tokens.css, not the + # three somebody stopped at. `success` and `warning` were settable and their + # softs were not derived, so a custom theme moved the text and left the + # background behind it in the base theme's hue -- an alert, a badge, a + # permission's "on" state and the `+` lines of every agent diff, each in two + # colours that were never meant to meet. Precisely the half-working failure + # this function's own docstring says it exists to prevent. + for name, alpha in ( + ("accent", "0.14"), + ("leaf", "0.14"), + ("danger", "0.14"), + ("success", "0.14"), + ("warning", "0.14"), + ): soft = _soft(theme.tokens.get(name, ""), alpha) if soft: lines.append(f" --{name}-soft: {soft};") diff --git a/src/lembas/web/static/css/admin.css b/src/lembas/web/static/css/admin.css index cd84fad..1e31391 100644 --- a/src/lembas/web/static/css/admin.css +++ b/src/lembas/web/static/css/admin.css @@ -15,14 +15,10 @@ that one, silently, while the reader was lost in the other. Under `.admin-scroll` the body is now an ordinary block and the page scrolls as one. */ -.admin-scroll, -.main > .tabs > .tabs__body { - flex: 1; - min-height: 0; - overflow-y: auto; - scrollbar-width: thin; - scrollbar-color: var(--border-strong) transparent; -} +/* What makes one of these scroll is `.scroll-region` in app.css, which both of + these selectors are listed in. Named there so the four declarations exist + once; named *here* is the reasoning above, which is about which element is + the scroller on which screen rather than about how a scroller behaves. */ .page, .admin-page { diff --git a/src/lembas/web/static/css/app.css b/src/lembas/web/static/css/app.css index d1f2e21..afbbc43 100644 --- a/src/lembas/web/static/css/app.css +++ b/src/lembas/web/static/css/app.css @@ -18,6 +18,33 @@ body { height: 100%; } +/* + A page built around the shell never scrolls its own document. + + `.shell` is `100dvh` -- the *dynamic* viewport, which is what you can actually + see -- while `html` and `body` above are `100%`, which resolves against the + initial containing block and is the *large* viewport, the one you get with the + browser's toolbar retracted. On a desktop those are the same number and this + rule does nothing. On a phone they differ by the height of the toolbar, and + the difference is a document taller than its own window: you scroll past the + bottom of the sidebar and the main column into bare background, and because + every gesture retracts or extends the toolbar the shell resizes underneath you + and it never settles. + + Reported on /settings, true of every page with a shell. `:has()` rather than a + class because the shell is what decides this, not the route -- the auth, error + and offline pages have no shell and genuinely do scroll their document, and + they must keep doing so. +*/ +html:has(body > .shell), +body:has(> .shell) { + height: 100dvh; + overflow: hidden; + /* A flick that reaches the end of an inner scroller stops there rather than + pulling the page around behind it. */ + overscroll-behavior: none; +} + /* The `hidden` attribute has to win. @@ -367,6 +394,39 @@ button, input, textarea, select { .badge--danger { background: var(--danger-soft); color: var(--danger); } .badge--warning { background: var(--warning-soft); color: var(--warning); } +/* --- Scroll regions -------------------------------------------------------- + The four declarations that make an element *the* scroller, written once. + + They were spelled out five times -- the sidebar's list, the thread, the + inspector, the canvas and (in admin.css) the tabs and admin pages -- and + agreed on three of them. The fourth, `overscroll-behavior`, was on the + sidebar alone, with a good comment explaining why it was needed there. It is + needed everywhere for the same reason: a flick that reaches the end of a + scroller chains to whatever is behind it, and behind these is the shell, + which does not scroll -- so what the gesture produces is not a scrolled page + but a rubber-band into blank background, which reads as the layout having + come loose. + + `min-height: 0` is the half that is load-bearing rather than cosmetic: a flex + child will not shrink below its content without it, so a scroller missing it + grows its parent instead of scrolling inside it. `.thread-scroll` relied on a + scroll container's automatic minimum size to get away with omitting it, which + is true and is not something the next person should have to know. */ +.scroll-region, +.sidebar__scroll, +.inspector__body, +.canvas__body, +.thread-scroll, +.admin-scroll, +.main > .tabs > .tabs__body { + flex: 1; + min-height: 0; + overflow-y: auto; + overscroll-behavior: contain; + scrollbar-width: thin; + scrollbar-color: var(--border-strong) transparent; +} + /* --- Application shell ----------------------------------------------------- */ .shell { display: flex; height: 100dvh; overflow: hidden; } @@ -412,18 +472,9 @@ button, input, textarea, select { flex: none; } +/* A `.scroll-region`; only the padding is its own. */ .sidebar__scroll { - flex: 1; - min-height: 0; - overflow-y: auto; - /* A flick past the end of the list stops there rather than chaining to - whatever is behind it. The shell is `overflow: hidden`, so what chaining - produced was not a scrolled page but a rubber-band into blank background -- - which reads as the sidebar having come loose from the layout. */ - overscroll-behavior: contain; padding: 0 var(--sp-2) var(--sp-3); - scrollbar-width: thin; - scrollbar-color: var(--border-strong) transparent; } .sidebar__footer { @@ -491,12 +542,7 @@ button, input, textarea, select { } .inspector__body { - flex: 1; - min-height: 0; - overflow-y: auto; padding: var(--sp-4); - scrollbar-width: thin; - scrollbar-color: var(--border-strong) transparent; } .inspector__heading { @@ -789,8 +835,8 @@ body.is-resizing .canvas__body { pointer-events: none; } } .canvas__body { - flex: 1; - min-height: 0; + /* Both axes, unlike every other scroll region: nothing re-wraps a source + line, so it has to be reachable sideways. */ overflow: auto; padding: var(--sp-3); } diff --git a/src/lembas/web/static/css/chat.css b/src/lembas/web/static/css/chat.css index 56a15a0..745931d 100644 --- a/src/lembas/web/static/css/chat.css +++ b/src/lembas/web/static/css/chat.css @@ -6,12 +6,11 @@ */ /* --- Thread --------------------------------------------------------------- */ +/* A `.scroll-region` (app.css); the smooth behaviour is this one's own, because + this is the scroller something is repeatedly scrolled *to* -- the newest + message, a jump back to the bottom -- and the others are not. */ .thread-scroll { - flex: 1; - overflow-y: auto; scroll-behavior: smooth; - scrollbar-width: thin; - scrollbar-color: var(--border-strong) transparent; } .thread { diff --git a/src/lembas/web/templates/chat/_thread.html b/src/lembas/web/templates/chat/_thread.html index afcad66..1fe70cf 100644 --- a/src/lembas/web/templates/chat/_thread.html +++ b/src/lembas/web/templates/chat/_thread.html @@ -25,16 +25,12 @@