Six things that looked like they worked
None of these fails loudly and two of them correct themselves if you reload, which is why five were found by reading rather than by anybody reporting them. The reply that lost its author is the one worth knowing about: the frame that replaces a bubble when a reply lands was looking the models up as nobody, and "no user" answers "no models" rather than "all models" -- so every finished reply swapped the model's avatar for the plain mark and put the instance's name where the model's should be, until the next page load put it back. Beside it: a concurrency quota enforced on two of the six paths that start a reply, including neither of the two most used; a custom theme whose success and warning colours moved the text and left the background behind; a phone shell sized to one viewport inside a document sized to another, which is the reported scroll past the bottom of Settings; a whole conversation's Markdown rendered on every page load and read by nothing; a skip guard inert since it was written; and an endpoint nothing has ever called. The scroll fix folded five near-identical scroller rules into one, which is also where the containment they were all missing now lives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,3 @@
|
||||
"""LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints."""
|
||||
|
||||
__version__ = "1.0.3"
|
||||
__version__ = "1.0.4"
|
||||
|
||||
+43
-21
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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};")
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -25,16 +25,12 @@
|
||||
</p>
|
||||
<div class="compacted__body">
|
||||
{% for message in compacted %}
|
||||
{% with body_html = bodies.get(message.id, "") %}
|
||||
{% include "chat/_message.html" %}
|
||||
{% endwith %}
|
||||
{% include "chat/_message.html" %}
|
||||
{% endfor %}
|
||||
</div>
|
||||
</details>
|
||||
{% endif %}
|
||||
|
||||
{% for message in messages %}
|
||||
{% with body_html = bodies.get(message.id, "") %}
|
||||
{% include "chat/_message.html" %}
|
||||
{% endwith %}
|
||||
{% include "chat/_message.html" %}
|
||||
{% endfor %}
|
||||
|
||||
@@ -29,7 +29,5 @@
|
||||
{% endif %}
|
||||
|
||||
{% for message in messages %}
|
||||
{% with body_html = bodies.get(message.id, "") %}
|
||||
{% include "chat/_message.html" %}
|
||||
{% endwith %}
|
||||
{% include "chat/_message.html" %}
|
||||
{% endfor %}
|
||||
|
||||
@@ -83,9 +83,7 @@
|
||||
{% endif %}
|
||||
|
||||
{% for message in messages %}
|
||||
{% with body_html = bodies.get(message.id, "") %}
|
||||
{% include "chat/_message.html" %}
|
||||
{% endwith %}
|
||||
{% include "chat/_message.html" %}
|
||||
{% endfor %}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user