diff --git a/CHANGELOG.md b/CHANGELOG.md index 6501bbb..6fdfd3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,44 @@ for 1.0.0 have something to be assembled from. ## Unreleased +## 1.8.0 + +- **The crowd is where you would look for it.** In 1.6.0 the only way to add a + model to a chat was the Chat settings panel — behind the ⋯ menu, inside a chat + that already existed — and the switch that turns the feature on was a card on + the Agents page. Somebody who enabled it went looking and found nothing, which + is the correct outcome of that arrangement. + + Now there is a **crowd button in the composer**, beside the attachment and + scope buttons, on both the chat screen and Messages. It carries a count when + the chat has a crowd, it lists the models you can reach, and it says what the + turn will cost before you tick anything. On the new-chat screen the choice + **rides along with the first message**, so a chat can start as a crowd rather + than having to be converted into one. + + The instance switch and its bounds have moved to their own page, **Admin → + Crowd**. + +- Fixed: **the new-chat screen was wider than a phone.** Before the first + message, the suggestion cards pushed the conversation 65px past the edge of a + 390px screen and it could be dragged sideways; after the first message it + looked right, because the cards were gone. Reported from a phone. + + Two things were true at once. The cards' grid asked for a minimum column width + it could not give up — the ordinary version of this bug — and it was *also* a + grid item, which means it carried a min-content floor that beats `width: 100%` + outright. Fixing only the first made it 27px worse. Both are fixed, on all four + grids in the stylesheets that could have it, and a test now refuses either half + of the pair on its own. + + The reason this survived four releases of narrow-width checking is worth + recording: the screenshot harness built its client without running the + application's startup, so the suggestion cards were **absent from every shot + ever taken of that screen**, and its overflow check deliberately ignored + anything inside a scrolling box — correct for a wide table in its own scroller, + blind to a box that scrolls sideways when nobody asked it to. Both are fixed, + and the harness now names the offending element and the child responsible. + ## 1.7.0 - **The interface speaks Slovak.** Pick a language under **Appearance** in your diff --git a/scripts/shoot.py b/scripts/shoot.py index f233911..9546bae 100644 --- a/scripts/shoot.py +++ b/scripts/shoot.py @@ -26,6 +26,7 @@ import shutil import subprocess import sys import tempfile +from functools import cache from pathlib import Path REPO = Path(__file__).resolve().parent.parent @@ -128,8 +129,65 @@ window.__measure = function () { return found.sort(function (a, b) { return b.over - a.over; }).slice(0, 8); } + /* --- A box that scrolls sideways when nobody asked it to ----------------- + + The blind spot that hid the suggestions bug through forty measurements. + `.suggestions` rendered 455px wide inside a 390px `.thread-scroll`, and + every check above looked straight past it: `culprits('x')` skips anything + with a scrollable ancestor -- correct for a table inside its own scroller, + wrong for the scroller itself -- and `scrollsSideways` stayed false because + `.thread-scroll` absorbed the overflow instead of the document. + + "Authored" is the distinction that makes this reportable rather than noise. + The tree's rule is that anything wide gets its OWN scroller, so a wrapper + carrying `overflow-x: auto` in a stylesheet is right. A box given only + `overflow-y: auto` scrolls sideways as well, because the other axis then + computes to `auto` -- and that is always a bug. Computed style cannot tell + those apart, both being `auto`, so the rules that say it are read off the + stylesheets -- in Python, by `authored_sideways()` below, and not from the + CSSOM here: a stylesheet loaded over `file://` is a foreign origin for + `cssRules` even with `--allow-file-access-from-files`, and every sheet + throws. That silently found *nothing authored*, which turns this check into + "every vertical scroller is a bug" -- so the list arriving empty is a hard + error rather than a clean run. */ + var sidewaysAuthors = __SIDEWAYS_AUTHORS__; + + function authoredSideways(el) { + if (el.style.overflowX || el.style.overflow) return true; + for (var i = 0; i < sidewaysAuthors.length; i++) { + try { if (el.matches(sidewaysAuthors[i])) return true; } catch (e) { /* :has() etc */ } + } + return false; + } + + var sideways = []; + document.querySelectorAll('body, body *').forEach(function (el) { + var ox = getComputedStyle(el).overflowX; + if (ox !== 'auto' && ox !== 'scroll') return; + if (el.scrollWidth <= el.clientWidth + 1) return; + if (authoredSideways(el)) return; + /* Which child is doing it. "`.thread-scroll` scrolls sideways" is not + actionable; "`.suggestions` is 455px inside its 390px" is. */ + var worst = null; + el.querySelectorAll('*').forEach(function (kid) { + var over = kid.getBoundingClientRect().width - el.clientWidth; + if (over > 1 && (!worst || over > worst.over)) { + worst = {tag: kid.tagName.toLowerCase(), + cls: (kid.className && kid.className.toString().slice(0, 50)) || '', + w: Math.round(kid.getBoundingClientRect().width), + over: Math.round(over)}; + } + }); + sideways.push({tag: el.tagName.toLowerCase(), + cls: (el.className && el.className.toString().slice(0, 50)) || '', + scrollW: el.scrollWidth, clientW: el.clientWidth, + widest: worst}); + }); + var shell = document.querySelector('.shell'); return { + sidewaysScrollers: sideways.slice(0, 8), + sidewaysCount: sideways.length, docScrollH: de.scrollHeight, innerH: window.innerHeight, docScrollW: de.scrollWidth, @@ -167,6 +225,42 @@ window.__measure = function () { """ +@cache +def authored_sideways() -> tuple[str, ...]: + """Selectors whose rules really do ask for horizontal scrolling. + + The tree's rule is that anything wide gets its own scroller, so these are + the correct ones: a table wrapper, a code block, the tab bar. Everything + else that scrolls sideways is `overflow-y: auto` dragging the other axis + along with it, which is always a bug and is what `.suggestions` did. + """ + selectors: list[str] = [] + for path in sorted((STATIC / "css").glob("*.css")): + text = re.sub(r"/\*.*?\*/", "", path.read_text(), flags=re.S) + # Innermost blocks only: `[^{}]*` cannot cross a brace, so an `@media` + # prelude never matches and the rules inside it do. + for prelude, body in re.findall(r"([^{}]*)\{([^{}]*)\}", text): + wants = False + for declaration in body.split(";"): + name, _, value = declaration.partition(":") + name, value = name.strip().lower(), value.strip().lower() + if name not in ("overflow", "overflow-x") or not value: + continue + # `overflow: hidden auto` is x then y, so the first word is ours; + # `overflow: auto` is both. + wants = wants or value.split()[0] in ("auto", "scroll") + if not wants: + continue + selectors += [ + part.strip() + for part in prelude.split(",") + if part.strip() and not part.strip().startswith("@") + ] + if not selectors: + raise SystemExit("read no horizontal-overflow rules -- the sideways check would cry wolf") + return tuple(selectors) + + def build_client(): import lembas.config as config_mod @@ -198,6 +292,17 @@ def build_client(): db.flush() for name in ("gemma4-moe", "qwen3-coder"): db.add(Model(connection_id=connection.id, model_id=name, display_name=name)) + + # 🚨 The suggestion cards are seeded by the startup hook, and `TestClient(app)` + # runs a lifespan only inside a `with` block -- so every shot of the new-chat + # screen ever taken by this script was of a page with its cards missing. That + # is how a grid 65px wider than a phone survived forty measurements. Seeded + # here rather than by entering the lifespan, which would also start the + # schedule ticker and rehydrate background jobs inside a screenshot run. + from lembas.services.suggestions import seed_defaults as seed_suggestions + + with session_scope() as db: + seed_suggestions(db) return client @@ -219,6 +324,20 @@ def rewrite(html: str, client, assets: Path) -> str: html, ) + # Anything else the *application* serves rather than mounts. Model avatars live + # under `/uploads/models/…`, which is a route behind auth -- so they cannot be + # pointed at a file on disk and have to be fetched through the client like + # `/branding.css` above. A real instance has them and a fixture does not, which + # is exactly the difference that makes a page measured here unlike the page + # somebody is looking at. + for url in sorted({*re.findall(r'\bsrc="(/(?:uploads|branding)/[^"?]+)"', html)}): + response = client.get(url) + if response.status_code != 200: + continue + name = "fetched-" + url.strip("/").replace("/", "-") + (assets / name).write_bytes(response.content) + html = html.replace(f'src="{url}"', f'src="file://{assets}/{name}"') + # Fail loudly, and only about things that decide how the page LOOKS: every # `src`, and `href` on a . An `href` on an anchor is a destination, # not an asset -- flagging those makes the guard cry wolf on every page and @@ -249,14 +368,15 @@ def rewrite(html: str, client, assets: Path) -> str: if missing: raise SystemExit(f"REWRITTEN TO NOTHING -- still an unstyled document: {missing[:5]}") - # The one-time notifications offer is a modal over the very page we came + # The one-time notifications offer is a modal over the very page we came # to measure, and it is gated on a localStorage key. Set it in the head, so # it runs before the deferred script that reads it. quiet = ( "" ) - return html.replace("", quiet + MEASURE + "", 1) + measure = MEASURE.replace("__SIDEWAYS_AUTHORS__", json.dumps(list(authored_sideways()))) + return html.replace("", quiet + measure + "", 1) def shoot(client, path: str, width: int, height: int, theme: str, outdir: Path) -> dict: @@ -391,6 +511,13 @@ def main() -> None: flags.append(f"DOC-SCROLLS({r['docScrollH']}>{r['innerH']})") if r["scrollsSideways"]: flags.append(f"SIDEWAYS({r['docScrollW']}>{r['innerW']})") + for s in r.get("sidewaysScrollers", []): + widest = s["widest"] + blame = f"<{widest['tag']}.{widest['cls']} {widest['w']}px" if widest else "" + flags.append( + f"SCROLLER-SIDEWAYS({s['tag']}.{s['cls']} " + f"{s['scrollW']}>{s['clientW']}{blame})" + ) if r["overflowCount"]: flags.append(f"overflow:{r['overflowCount']}") if r["smallCount"]: diff --git a/src/lembas/__init__.py b/src/lembas/__init__.py index 92e4d79..5acd820 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.7.0" +__version__ = "1.8.0" diff --git a/src/lembas/api/admin_agents.py b/src/lembas/api/admin_agents.py index c873cf2..c710ba5 100644 --- a/src/lembas/api/admin_agents.py +++ b/src/lembas/api/admin_agents.py @@ -66,10 +66,6 @@ async def agents_page(request: Request, db: Db, user: AdminUser, saved: bool = F # reply is allowed to set going on its own, and a nav entry for one # card would be worse than the near-miss. "subagents": settings_store.subagents(db), - # And a third group on the same page, for the same reason: a crowd is - # not an agent-chat feature either, but this is where somebody comes to - # find out what one turn is allowed to set going. - "crowd": settings_store.crowd(db), "saved": saved, }, ) @@ -115,35 +111,6 @@ async def save_subagents( return RedirectResponse("/admin/agents?saved=1", status_code=status.HTTP_303_SEE_OTHER) -@router.post("/crowd") -async def save_crowd( - db: Db, - user: AdminUser, - enabled: bool = Form(False), - max_models: int = Form(4), - max_rounds: int = Form(2), - wall_seconds: int = Form(900), - collapse_agreement: bool = Form(False), -) -> Response: - """Its own route, for the reason `save_subagents` gives above.""" - settings_store.update( - db, - { - "enabled": enabled, - # Clamped here as well as on read. Every floor is one: a zero would be - # the feature switched off wearing the switch's clothes, and that is a - # thing to answer in one place. - "max_models": min(max(max_models, 1), 8), - "max_rounds": min(max(max_rounds, 1), 5), - "wall_seconds": min(max(wall_seconds, 60), 7200), - "collapse_agreement": collapse_agreement, - }, - key=settings_store.CROWD, - ) - log.info("crowd %s by %s", "enabled" if enabled else "disabled", user.email) - return RedirectResponse("/admin/agents?saved=1", status_code=status.HTTP_303_SEE_OTHER) - - @router.post("") async def save_agents( db: Db, diff --git a/src/lembas/api/admin_crowd.py b/src/lembas/api/admin_crowd.py new file mode 100644 index 0000000..ff84f7a --- /dev/null +++ b/src/lembas/api/admin_crowd.py @@ -0,0 +1,68 @@ +"""The crowd: several models answering one turn, in any chat. + +Its own module because it is its own page, and it is its own page because as a card +on `/admin/agents` it read as an agent-chat feature. It is not one: a crowd works in +an ordinary conversation, and the owner reasonably concluded otherwise from where +the switch was sitting. +""" + +from __future__ import annotations + +import logging + +from fastapi import APIRouter, Form, Request, Response, status +from fastapi.responses import RedirectResponse + +from lembas.api.deps import AdminUser, Db +from lembas.services import settings_store +from lembas.web.templating import render + +log = logging.getLogger(__name__) + +router = APIRouter(prefix="/admin/crowd", tags=["admin-crowd"]) + + +@router.get("") +async def crowd_page(request: Request, db: Db, user: AdminUser, saved: str = ""): + """Its own page, for the reason its template records: as a card on the Agents + screen it read as an agent-chat feature, which it is not.""" + return render( + request, + "admin/crowd.html", + {"crowd": settings_store.crowd(db), "saved": saved}, + ) + + +@router.post("") +async def save_crowd( + db: Db, + user: AdminUser, + enabled: bool = Form(False), + max_models: int = Form(4), + max_rounds: int = Form(2), + wall_seconds: int = Form(900), + collapse_agreement: bool = Form(False), +) -> Response: + """One group, one form, one route. + + The bounds are clamped here as well as in `settings_store.crowd`, which is the + same belt-and-braces `save_subagents` in `admin_agents.py` uses: a value posted + past this route -- by an older page, or by hand -- still reads back sane. + """ + settings_store.update( + db, + { + "enabled": enabled, + # Every floor is one: a zero would be the feature switched off + # wearing the switch's clothes. + "max_models": min(max(max_models, 1), 8), + "max_rounds": min(max(max_rounds, 1), 5), + "wall_seconds": min(max(wall_seconds, 60), 7200), + "collapse_agreement": collapse_agreement, + }, + key=settings_store.CROWD, + ) + log.info("crowd %s by %s", "enabled" if enabled else "disabled", user.email) + return RedirectResponse("/admin/crowd?saved=1", status_code=status.HTTP_303_SEE_OTHER) + + diff --git a/src/lembas/api/chats.py b/src/lembas/api/chats.py index 2e5e71a..fedf1c4 100644 --- a/src/lembas/api/chats.py +++ b/src/lembas/api/chats.py @@ -296,6 +296,11 @@ async def start_chat( scope_on: list[str] = Form(default=[]), scope_skill_all: list[str] = Form(default=[]), scope_skill_on: list[str] = Form(default=[]), + # Who else answers, as the crowd menu stood before the first word. There is no + # chat row yet to attach members to, so the choice rides along with the message + # -- the same mechanism the scope switches above use, and the reason the control + # lives inside the composer's form rather than in the topbar. + crowd_model_ids: list[str] = Form(default=[]), ) -> Response: """Create a chat from its first message. @@ -329,6 +334,8 @@ async def start_chat( skills_off=frozenset(scope_skill_all) - frozenset(scope_skill_on), ) + _apply_crowd(db, chat, user, crowd_model_ids) + _adopt_draft(db, user, draft_id, chat) user_message = chat_service.create_message(db, chat, ROLE_USER, content) @@ -1510,6 +1517,45 @@ def _thread_context(db: DBSession, chat: Chat, user: User) -> dict: } +def _apply_crowd(db: DBSession, chat: Chat, user: User, values: list[str]) -> None: + """Replace a chat's crowd with the models named, in the order named. + + One implementation for both the composer (where the choice rides along with + the first message) and the settings panel, because two would be two places to + forget a rule -- and there are three: + + * **Checked against what this person can reach**, never against what exists. + A control checked only in the template is advisory, and a crafted request + walks past it. Same reasoning as the model branch in `update_chat`. + * **Never the chat's own model**, which would answer twice in a row. + * **Capped by `crowd.max_models`**, on the way in as well as on the way out. + + The connection is stored beside the id because `Model` is unique on the pair, + and a model offered by two connections is two rows with different capabilities. + """ + from lembas.db.models import CrowdMember + + settings = settings_store.crowd(db) + reachable = { + model.model_id: model for model in chat_service.available_models(db, user) + } + wanted: list[str] = [] + for value in values: + value = str(value).strip() + if value and value in reachable and value != chat.model_id and value not in wanted: + wanted.append(value) + wanted = wanted[: int(settings["max_models"])] + + chat.crowd = [ + CrowdMember( + model_id=model_id, + connection_id=reachable[model_id].connection_id, + position=index, + ) + for index, model_id in enumerate(wanted) + ] + + def _messages_after(db: DBSession, message: Message) -> list[Message]: """Everything later in this chat than one message. @@ -2117,39 +2163,8 @@ async def update_chat(request: Request, db: Db, user: RequiredUser, chat_id: str if "crowd_model_ids" in form: # The same shape as the bases above: one field always sent, so clearing - # every box clears the crowd. Checked against what this person can reach - # rather than against what exists, or the picker is advisory and a crafted - # request walks past it -- the reasoning the model branch carries. - from lembas.db.models import CrowdMember - - settings = settings_store.crowd(db) - reachable = { - model.model_id for model in chat_service.available_models(db, user) - } - wanted: list[str] = [] - for value in form.getlist("crowd_model_ids"): - value = str(value).strip() - # Never the chat's own model: it would answer twice in a row, which is - # nobody's idea of a second opinion. - if value and value in reachable and value != chat.model_id and value not in wanted: - wanted.append(value) - wanted = wanted[: int(settings["max_models"])] - - chat.crowd = [ - CrowdMember( - model_id=model_id, - connection_id=next( - ( - model.connection_id - for model in chat_service.available_models(db, user) - if model.model_id == model_id - ), - None, - ), - position=index, - ) - for index, model_id in enumerate(wanted) - ] + # every box clears the crowd. + _apply_crowd(db, chat, user, form.getlist("crowd_model_ids")) submitted_params = {name: form[name] for name in _PARAM_RANGES if name in form} if submitted_params: diff --git a/src/lembas/api/pages.py b/src/lembas/api/pages.py index 18287d3..61cec6f 100644 --- a/src/lembas/api/pages.py +++ b/src/lembas/api/pages.py @@ -83,7 +83,9 @@ def _chat_context(db: DBSession, user: User, chat: Chat | None) -> dict: else [] ), "attached_base_ids": [base.id for base in chat.knowledge_bases] if chat else [], - **_crowd_context(db, user, chat, models), + **_crowd_context( + db, user, chat, models, current.model_id if current is not None else "" + ), # What *this* model takes, not the three every model used to be assumed # to take. The vocabulary is per model -- gpt-oss has no `xhigh` and # Bonsai has no `high`, and sending the wrong one does not degrade, it @@ -197,12 +199,17 @@ def _scope_context(db: DBSession, user: User, chat: Chat | None) -> dict: } -def _crowd_context(db: DBSession, user: User, chat: Chat | None, models: list) -> dict: +def _crowd_context( + db: DBSession, user: User, chat: Chat | None, models: list, default_model_id: str = "" +) -> dict: """Who else could answer in this chat, and what that would cost. - Empty — and the panel then shows nothing rather than an empty control — when - the feature is off, when there is nobody else to add, or on the new-chat - screen, where there is no chat to attach anybody to yet. + Offered on the **new-chat screen as well**, where there is no chat row yet: the + choice rides along with the first message, the way the scope switches do. The + first version of this was per-chat only and therefore invisible to anybody + setting a conversation up — which is how the feature shipped switched on and + unreachable. Empty only when the feature is off or there is nobody else to add, + and then the control is absent rather than being an empty menu. The cost is spelled out because it is the thing somebody will not have thought about: a turn is `speakers x rounds x 2 - 1` replies, and on one local endpoint @@ -211,21 +218,31 @@ def _crowd_context(db: DBSession, user: User, chat: Chat | None, models: list) - from lembas.services import crowd as crowd_service settings = settings_store.crowd(db) - if chat is None or not settings["enabled"]: + if not settings["enabled"]: return {"crowd_available": [], "crowd_member_ids": [], "crowd_skipped": []} - others = [model for model in models if model.model_id != chat.model_id] - members = [ - row.model_id - for row in sorted(chat.crowd, key=lambda row: (row.position, row.model_id)) - ] + # On the new-chat screen the "own" model is whichever one the picker is + # showing, so the list excludes it for the same reason it does in a chat: + # adding it would have it answer twice in a row. + own = chat.model_id if chat is not None else default_model_id + others = [model for model in models if model.model_id != own] + members = ( + [ + row.model_id + for row in sorted(chat.crowd, key=lambda row: (row.position, row.model_id)) + ] + if chat is not None + else [] + ) reachable = {model.model_id for model in others} speakers = 1 + len([model_id for model_id in members if model_id in reachable]) rounds = int(settings["max_rounds"]) return { "crowd_available": others, "crowd_member_ids": [model_id for model_id in members if model_id in reachable], - "crowd_skipped": crowd_service.unreachable_members(db, chat, user), + "crowd_skipped": ( + crowd_service.unreachable_members(db, chat, user) if chat is not None else [] + ), # One round is out and back: everybody answers, everybody but the last is # asked whether they disagree, and the main model closes. "crowd_replies": max(1, speakers * 2 - 1), diff --git a/src/lembas/main.py b/src/lembas/main.py index ee79c6e..fe833d9 100644 --- a/src/lembas/main.py +++ b/src/lembas/main.py @@ -17,6 +17,7 @@ from lembas.api import ( admin_agents, admin_audio, admin_branding, + admin_crowd, admin_extraction, admin_images, admin_models, @@ -221,6 +222,7 @@ def create_app() -> FastAPI: app.include_router(admin_suggestions.router) app.include_router(admin_tools.router) app.include_router(admin_agents.router) + app.include_router(admin_crowd.router) app.include_router(push.router) app.include_router(branding.router) diff --git a/src/lembas/web/i18n/sk.py b/src/lembas/web/i18n/sk.py index 82fcf3b..678ab6c 100644 --- a/src/lembas/web/i18n/sk.py +++ b/src/lembas/web/i18n/sk.py @@ -1075,12 +1075,15 @@ MESSAGES.update( "potom nemôže držať kolo otvorené celé poobedie." ), "Fold away a short \"I agree\" on the way back": ( - "Zbaliť krátke „súhlasím“ na cestě späť" + "Zbaliť krátke „súhlasím“ na ceste späť" ), - "Off by default. With it on, each chat's settings panel offers the other models; a chat with none ticked behaves exactly as it always has.": ( - "Predvolene vypnuté. Po zapnutí panel nastavení každej konverzácie ponúka " - "ostatné modely; konverzácia bez zaškrtnutého modelu sa chová presne ako " - "vždy." + "Off by default. With it on, every chat's composer offers the other models; a chat with none ticked behaves exactly as it always has.": ( + "Predvolene vypnuté. Po zapnutí ponúka pole na písanie v každej " + "konverzácii ostatné modely; konverzácia bez zaškrtnutého modelu sa chová " + "presne ako vždy." + ), + "%(models)s models answer each turn, over up to %(rounds)s rounds.": ( + "Na každý ťah odpovedá %(models)s modelov, a to najviac v %(rounds)s kolách." ), "Check for due work every": "Kontrolovať splatnú prácu každých", "How often it looks": "Ako často sa pozerá", diff --git a/src/lembas/web/static/css/admin.css b/src/lembas/web/static/css/admin.css index b4bbf09..024d472 100644 --- a/src/lembas/web/static/css/admin.css +++ b/src/lembas/web/static/css/admin.css @@ -260,7 +260,9 @@ a.tabs__tab { text-decoration: none; } */ .field-row { display: grid; - grid-template-columns: repeat(auto-fit, minmax(9rem, 1fr)); + /* Both halves of the pair -- see `.grid--2` in app.css. */ + min-width: 0; + grid-template-columns: repeat(auto-fit, minmax(min(100%, 9rem), 1fr)); gap: var(--sp-3); } .field-row > .field { margin-bottom: var(--sp-4); } diff --git a/src/lembas/web/static/css/app.css b/src/lembas/web/static/css/app.css index 7680f37..0c9695a 100644 --- a/src/lembas/web/static/css/app.css +++ b/src/lembas/web/static/css/app.css @@ -384,8 +384,16 @@ input.visually-hidden[type="checkbox"] { /* Multi-column form layout, one definition. */ .grid { display: grid; gap: var(--sp-4); } -.grid--2 { grid-template-columns: repeat(auto-fit, minmax(14rem, 1fr)); } -.grid--3 { grid-template-columns: repeat(auto-fit, minmax(9rem, 1fr)); } +/* `min(100%, …)` on every auto-fit track and `min-width: 0` with it, for the + reason `.suggestions` in chat.css sets out at length. The pair is not + optional: `min(100%, …)` stops the track demanding more than the box, and + `min-width: 0` stops the *box* demanding more than its parent -- a grid or + flex item carries `min-width: auto`, which is a min-content floor, and a + floor beats `width`. A stylesheet cannot tell whether one of these grids has + been dropped into a flex parent today, so both go on every one of them. + `tests/test_narrow_grids.py` refuses a track that has only half the pair. */ +.grid--2 { min-width: 0; grid-template-columns: repeat(auto-fit, minmax(min(100%, 14rem), 1fr)); } +.grid--3 { min-width: 0; grid-template-columns: repeat(auto-fit, minmax(min(100%, 9rem), 1fr)); } /* --- Alerts --------------------------------------------------------------- */ .alert { diff --git a/src/lembas/web/static/css/chat.css b/src/lembas/web/static/css/chat.css index 419f7f0..9aa7f07 100644 --- a/src/lembas/web/static/css/chat.css +++ b/src/lembas/web/static/css/chat.css @@ -266,7 +266,27 @@ */ .suggestions { display: grid; - grid-template-columns: repeat(auto-fit, minmax(13rem, 1fr)); + /* 🚨 `min-width: 0` is what keeps this grid on the screen, and `width: 100%` + alone did not: it is a grid item of `.thread__intro`, so it carries + `min-width: auto`, which for a grid item means *a min-content floor* -- and + min-width beats width. Its min-content size is two cards side by side, so it + rendered 428px wide inside a 390px phone with `width: 100%` set and ignored. + + That floor is also why writing the track as `minmax(min(100%, 13rem), 1fr)` + -- the tree's standing rule, and right -- made it *worse* on its own, 428px + to 455px: a percentage is indefinite while the floor is being measured, so + the track fell back to a card's max-content and raised the very number that + was overflowing. The two go together. With the floor removed, `width: 100%` + finally resolves against the 366px column, `min(100%, …)` hands the track + 366px to clamp against, and `auto-fit` places one column. + + It scrolled `.thread-scroll` rather than the page, which is why a pass + looking for a document that scrolls sideways never saw it: `overflow-y: auto` + makes the other axis scrollable too. Reported on a phone, found by asking + which *element* could scroll and then reading its computed `width` against + its parent's. */ + min-width: 0; + grid-template-columns: repeat(auto-fit, minmax(min(100%, 13rem), 1fr)); gap: var(--sp-3); width: 100%; max-width: 40rem; diff --git a/src/lembas/web/templates/admin/_layout.html b/src/lembas/web/templates/admin/_layout.html index 8f3f977..16614ac 100644 --- a/src/lembas/web/templates/admin/_layout.html +++ b/src/lembas/web/templates/admin/_layout.html @@ -50,6 +50,13 @@ {{ icon("sliders", "icon--sm") }} {{ t("Models") }} + {# Its own entry rather than a card on Agents, where it started. Sitting + there made it read as an agent-chat feature -- which is what the owner + took it for, reasonably, since that is what the page is called. #} + + {{ icon("users", "icon--sm") }} + {{ t("A crowd") }} + {{ icon("speaker", "icon--sm") }} {{ t("Audio") }} diff --git a/src/lembas/web/templates/admin/agents.html b/src/lembas/web/templates/admin/agents.html index 7559fcb..c86e807 100644 --- a/src/lembas/web/templates/admin/agents.html +++ b/src/lembas/web/templates/admin/agents.html @@ -395,76 +395,6 @@ button was pressed, which is what keeps each group's save handler writing one key. #} -{# A third settings group on this page, saved by its own form -- the reason the - Helpers card gives. A crowd is not an agent-chat feature either, but this is the - page somebody opens to find out what one turn may set going. #} -
-
-

{{ t("A crowd") }}

-

- A chat can have more than one model in it. The chat's own model answers, then - each of the others in turn; then the order runs {{ t("backwards") }}, - each one asked whether it disagrees with anything; and it ends back at the - first, which either closes or sends them round again. -

- -
- {{ icon("warning", "icon--sm") }} - - One turn costs {{ t("models × rounds × 2 − 1") }} replies — four - models over two rounds is fifteen — and on a single local endpoint every - change of speaker also loads a different model. Larger crowds of smaller - models, and sometimes of bigger ones, start going round in circles: that is - what the round limit is for, and it is a limit ordinary work will reach - rather than a runaway backstop. - -
- -
- -

{{ t("Off by default. With it on, each chat's settings panel offers the other models; a chat with none ticked behaves exactly as it always has.") }}

-
- -
- - -

{{ t("Four is already eight replies a turn at one round each. More voices past that tend to repeat each other rather than add anything.") }}

-
- -
- - -

{{ t("A round is out and back. Two gives the first model one chance to change its mind after hearing the objections, which is the point of the whole thing; three is where going in circles starts.") }}

-
- -
- - -

{{ t("Across every speaker, not each. A member whose endpoint has stalled cannot then hold the round open all afternoon.") }}

-
- -
- -

{{ t("The disagreements are what a crowd is for; a column of bubbles saying nothing is what makes somebody switch it off. The text is still there behind a disclosure.") }}

-
- -
- -
-
-
-

{{ t("Helpers") }}

diff --git a/src/lembas/web/templates/admin/crowd.html b/src/lembas/web/templates/admin/crowd.html new file mode 100644 index 0000000..c634533 --- /dev/null +++ b/src/lembas/web/templates/admin/crowd.html @@ -0,0 +1,87 @@ +{% extends "admin/_layout.html" %} +{% from "_macros.html" import icon %} +{% set section = "crowd" %} + +{% block title %}A crowd - {{ brand.name }}{% endblock %} +{% block heading %}A crowd{% endblock %} + +{% block admin_content %} +{# + Its own page rather than a card on Agents, which is where it shipped in 1.6.0. + Sitting there made it read as an agent-chat feature -- the owner took it for one, + reasonably, because that is what the page is called -- and a crowd has nothing to + do with agent chats: it works in any conversation. +#} +

{{ t("Several models answering one turn, in any chat. Not an agent-chat feature: it works in an ordinary conversation, and the control is in the composer beside the tool switches.") }}

+ +{% if saved %} +
{{ icon("check", "icon--sm") }} {{ t("Settings saved.") }}
+{% endif %} + + +
+

+ A chat can have more than one model in it. The chat's own model answers, then + each of the others in turn; then the order runs {{ t("backwards") }}, + each one asked whether it disagrees with anything; and it ends back at the + first, which either closes or sends them round again. +

+ +
+ {{ icon("warning", "icon--sm") }} + + One turn costs {{ t("models × rounds × 2 − 1") }} replies — four + models over two rounds is fifteen — and on a single local endpoint every + change of speaker also loads a different model. Larger crowds of smaller + models, and sometimes of bigger ones, start going round in circles: that is + what the round limit is for, and it is a limit ordinary work will reach + rather than a runaway backstop. + +
+ +
+ +

{{ t("Off by default. With it on, every chat's composer offers the other models; a chat with none ticked behaves exactly as it always has.") }}

+
+ +
+ + +

{{ t("Four is already eight replies a turn at one round each. More voices past that tend to repeat each other rather than add anything.") }}

+
+ +
+ + +

{{ t("A round is out and back. Two gives the first model one chance to change its mind after hearing the objections, which is the point of the whole thing; three is where going in circles starts.") }}

+
+ +
+ + +

{{ t("Across every speaker, not each. A member whose endpoint has stalled cannot then hold the round open all afternoon.") }}

+
+ +
+ +

{{ t("The disagreements are what a crowd is for; a column of bubbles saying nothing is what makes somebody switch it off. The text is still there behind a disclosure.") }}

+
+ +
+ +
+
+ + +{% endblock %} diff --git a/src/lembas/web/templates/chat/_composer.html b/src/lembas/web/templates/chat/_composer.html index 444152b..bf109e4 100644 --- a/src/lembas/web/templates/chat/_composer.html +++ b/src/lembas/web/templates/chat/_composer.html @@ -293,6 +293,97 @@ {% endif %} + + {# + Who else answers. + + Beside the tool switches rather than buried in Chat settings, and + *inside this form* rather than in the topbar, for one reason each. + + The first: somebody deciding who answers is making the same kind of + choice as somebody picking the model, and the first version of this + put it only in the Chat settings panel — behind the ⋯ menu, inside a + chat that already existed. The owner enabled the feature, went + looking, and could not find it. A control nobody can find is a + feature nobody has. + + The second: on the new-chat screen there is no chat row to attach + anybody to, so the choice has to *ride along with the first message* + — which means being a field of this form. That is the same mechanism + the scope switches above use, with the same hidden-input trick, + because a browser submits only the ticked boxes and `start_chat` + needs to know which ones were not. + #} + {% if crowd_available %} +
+ + + +
+ {% endif %} {# @@ -502,6 +593,12 @@ hx-patch and not hx-post: there is no POST for a chat, only PATCH, and htmx shows nothing when a request 405s -- which is how these controls spent the first half of their lives doing nothing. #} + {% if chat and crowd_available %} + {# Empty, and a sibling of the composer's form rather than inside it. See the + crowd checkboxes above, and `#agent-mode-form` below, for why both halves + of that sentence matter. #} +
+ {% endif %} {% if chat and chat.kind == "agent" %}
{% endif %} diff --git a/tests/test_crowd_ui.py b/tests/test_crowd_ui.py index 557acbb..1b57758 100644 --- a/tests/test_crowd_ui.py +++ b/tests/test_crowd_ui.py @@ -70,6 +70,87 @@ def _members(db, chat) -> list[str]: ] +# --- Reachable where somebody would look -------------------------------------- +# +# The feature shipped in 1.6.0 switched on and unreachable: the only control was +# inside the Chat settings panel, behind the ⋯ menu, in a chat that already +# existed. The owner enabled it, went looking, and reported that there was nothing +# to find. A control nobody can find is a feature nobody has, so these assert the +# two places it has to be rather than the one place it was. +def test_the_composer_offers_the_crowd_in_a_chat(client, db): + """Beside the tool switches, where the comparable decisions are.""" + chat = _chat(db) + page = client.get(f"/chat/{chat.id}").text + assert 'name="crowd_model_ids"' in page + assert 'form="crowd-form"' in page + assert '
' in page + + +def test_the_composer_offers_the_crowd_before_the_chat_exists(client, db): + """On the new-chat screen there is no row to attach anybody to, so the choice + rides along with the first message — the mechanism the scope switches use.""" + page = client.get("/chat").text + assert 'name="crowd_model_ids"' in page + # Riding along, so no sibling form and no PATCH: the composer's own POST + # carries it. + assert '' not in page + assert 'value="second-model"' in page + + +def test_starting_a_chat_with_a_crowd_keeps_it(client, db): + """The end of that path: the first message creates the chat *and* its crowd.""" + from sqlalchemy import select as sa_select + + client.post( + "/api/chats/start", + data={ + "content": "Who is right?", + "model_id": "main-model", + "crowd_model_ids": ["", "second-model", "third-model"], + }, + follow_redirects=False, + ) + db.expire_all() + chat = db.scalars(sa_select(Chat).order_by(Chat.created_at.desc())).first() + assert [row.model_id for row in sorted(chat.crowd, key=lambda r: r.position)] == [ + "second-model", + "third-model", + ] + + +def test_starting_a_chat_refuses_a_model_the_person_cannot_reach(client, db): + """The same rule as the panel, in the same one function, so there is nowhere + for the two to disagree.""" + from sqlalchemy import select as sa_select + + group = Group(name="Wheel") + db.add(group) + restricted = db.scalar(select(Model).where(Model.model_id == "third-model")) + restricted.public = False + restricted.groups = [group] + user = _user(db) + user.role = "user" + db.commit() + + client.post( + "/api/chats/start", + data={ + "content": "Who is right?", + "model_id": "main-model", + "crowd_model_ids": ["second-model", "third-model"], + }, + follow_redirects=False, + ) + db.expire_all() + chat = db.scalars(sa_select(Chat).order_by(Chat.created_at.desc())).first() + assert [row.model_id for row in chat.crowd] == ["second-model"] + + +def test_the_composer_control_is_absent_while_the_feature_is_off(client, db): + settings_store.update(db, {"enabled": False}, key=settings_store.CROWD) + assert 'name="crowd_model_ids"' not in client.get("/chat").text + + # --- Choosing ----------------------------------------------------------------- def test_the_panel_offers_the_other_models(client, db): chat = _chat(db) diff --git a/tests/test_narrow_grids.py b/tests/test_narrow_grids.py new file mode 100644 index 0000000..fda3359 --- /dev/null +++ b/tests/test_narrow_grids.py @@ -0,0 +1,85 @@ +"""A grid that reflows can still overflow a phone, and twice it has. + +`repeat(auto-fit, minmax(13rem, 1fr))` puts two 208px cards side by side on a +390px screen: `auto-fit` decides how many columns fit, and a track whose minimum +is a fixed length never gives that minimum up. The tree's standing rule is to +write `minmax(min(100%, 13rem), 1fr)` instead, so the *track* yields rather than +the viewport. + +Half of the rule is not the rule. `.suggestions` had `width: 100%` and got the +`min(100%, …)` track, and still rendered 455px wide inside a 366px column -- it +is a grid item, so it carries `min-width: auto`, which for a grid item means a +min-content floor, and a floor beats `width`. Worse, the floor is measured while +the percentage is indefinite, so the track falls back to a card's max-content: +adding `min(100%, …)` on its own moved the overflow from 428px to 455px. + +So both halves are asserted here, on every auto-fit grid in the stylesheets. A +stylesheet cannot see whether a given grid is a flex item on some page today or +becomes one next week, and `min-width: 0` costs nothing where it is not needed. + +Found on a phone, on the one screen the screenshot harness had never actually +rendered -- `scripts/shoot.py` builds its client without a lifespan, so the +suggestion cards were absent from every shot ever taken of the new-chat screen. +That is fixed there; this file is the cheap half that runs in the suite. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import lembas + +CSS = Path(lembas.__file__).parent / "web/static/css" + +# `[^{}]*` cannot cross a brace, so an `@media` prelude never matches and the +# rules nested inside it do. +RULE = re.compile(r"([^{}]*)\{([^{}]*)\}") +COMMENT = re.compile(r"/\*.*?\*/", re.S) + + +def _auto_grids() -> list[tuple[str, str, dict[str, str]]]: + found = [] + for path in sorted(CSS.glob("*.css")): + text = COMMENT.sub("", path.read_text(encoding="utf-8")) + for prelude, body in RULE.findall(text): + if "auto-fit" not in body and "auto-fill" not in body: + continue + declarations = { + part.partition(":")[0].strip(): part.partition(":")[2].strip() + for part in body.split(";") + if ":" in part + } + found.append((path.name, prelude.strip(), declarations)) + return found + + +def test_the_stylesheets_still_have_auto_fit_grids_to_check(): + # Otherwise the two tests below pass by finding nothing, which is how a + # coverage test quietly stops covering anything. + assert len(_auto_grids()) >= 4 + + +def test_every_auto_fit_track_can_give_up_its_minimum(): + offenders = [ + f"{name} {selector}" + for name, selector, declarations in _auto_grids() + for value in [declarations.get("grid-template-columns", "")] + if "minmax(" in value and "min(100%" not in value + ] + assert not offenders, ( + "an auto-fit track with a fixed minimum overflows a phone; write " + f"minmax(min(100%, X), 1fr): {offenders}" + ) + + +def test_every_auto_fit_grid_drops_its_automatic_minimum_size(): + offenders = [ + f"{name} {selector}" + for name, selector, declarations in _auto_grids() + if declarations.get("min-width") != "0" + ] + assert not offenders, ( + "a grid item's `min-width: auto` is a min-content floor and beats " + f"`width`, so `min(100%, …)` alone does not save it: {offenders}" + )