From e9fab9d8586e89e1475037663aaeac84ec14b9dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Bene=C5=A1?= Date: Wed, 5 Aug 2026 12:19:07 +0200 Subject: [PATCH] A job that finishes reaches the page you are looking at MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three complaints, all downstream of background commands. A finished job woke the model and not the browser. `jobs.wake` writes the completion and calls `generation.ensure`, and nothing tells the page: the only stream here is per-message, opened by the `sse-connect` on an incomplete assistant bubble -- which is a bubble this page has not got, because the reply that created it began somewhere else. `_queue_frames` proves the swap works and can only ride a stream already open. So the reader sat on the chat, watched the sidebar dot light up for the chat in front of them, and had to click it or reload to see a reply that had been there for minutes. `GET /api/chats/{id}/tail?after=` and a five-second poller is the answer, polled for the reason `/unread` is: a second always-on connection per tab is a lot of machinery for something that happens a few times a day. A cursor it cannot place -- absent, from another chat, naming a row a rewind deleted -- is answered with 204 and never with the transcript, which the page still holds every bubble of. The cut is read from the row so `_inject`'s restamp moves it too, and compared in SQL, a row read back from SQLite being naive where one still in the session is aware; the `id >` tie-break is not decoration, since under a bare `>` a row sharing the cut's microsecond is skipped for ever. The cursor comes from the DOM, because the DOM is the honest answer to what the page has -- the composer's POST, the `done` frame and the last poll all move it, and a variable would have to be updated by each of them, correctly, for ever. On `htmx:configRequest` rather than `hx-vals="js:…"`: two of the three things that handler does are cancellations, which `hx-vals` cannot express. Not `article.msg:last-of-type` either -- that is per-parent, so on a compacted chat it answers with the last article inside the `
` and the poll re-appends half the conversation. It is silent while a reply streams, since that reply delivers its own bubbles in the one frame that can get the order right, and a `htmx:beforeSwap` listener drops any answer holding a bubble already on the page: the race `hx-sync` cannot reach, and a duplicate there is a second `sse-connect` for one message rather than a cosmetic one. The route clears `unread` on every tick including the 204, because `_persist` marks a reply unread whenever `followers == 0` and that is true of a job-woken reply with somebody watching it. The completion also claimed the reader had sent it. The role is load-bearing -- `_inject` sends a queued turn verbatim and `build_messages` must keep seeing a user turn -- so `Message.machine` marks the bubble instead and the request is untouched. Their initial, their name and a pencil offering to rewrite what a machine reported: the route refuses the edit too, a hidden button being a courtesy. `_completion_text` is deliberately unchanged, `tool.background` quoting its opening sentence to the model, and there is now a test holding the two together. And the panel. `.jobs__row` had no horizontal padding while `.picker__menu` has none either, so every row ran flush into the border under a header inset by --sp-3. `jobs__row--open` had been emitted since the panel shipped with no rule anywhere, so the row whose log was on screen looked like the ones that were not. The dot was keyed on `status`, and `done` is exit 0 and exit 2 alike -- green beside the row's own "Failed, exit 2" -- so `JobView.tone` answers the colour and the template goes on answering the wording, which is the half a class name cannot carry. `duration` is empty for a running job on purpose: this panel is fetched when somebody opens it and never polled, so a live figure would freeze the instant it painted. Its stamps are normalised before subtracting, a job started before a restart and finished after it having one naive and one aware. Driven under the DOM stub before committing, per the standing rule: two listeners on document.body for events dispatched at a requesting element are exactly the shape a regex cannot check. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 70 +++ PLAN.md | 14 +- src/lembas/api/chats.py | 100 ++++- src/lembas/db/models/chat.py | 10 + src/lembas/services/agent/jobs.py | 74 +++- src/lembas/services/chat.py | 2 + src/lembas/web/static/css/chat.css | 56 ++- src/lembas/web/static/js/app.js | 74 ++++ .../web/templates/chat/_jobs_panel.html | 8 +- src/lembas/web/templates/chat/_message.html | 40 +- src/lembas/web/templates/chat/index.html | 28 ++ tests/test_agent_jobs.py | 132 ++++++ tests/test_chat.py | 97 +++++ tests/test_chat_tail.py | 411 ++++++++++++++++++ 14 files changed, 1105 insertions(+), 11 deletions(-) create mode 100644 tests/test_chat_tail.py diff --git a/CLAUDE.md b/CLAUDE.md index 1c7ec2c..d3c6484 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1028,6 +1028,76 @@ nothing, and the next job started would never appear. The log tail is fetched only for an expanded row — reading every job's output on every poll would be one SSH connection per job per five seconds, for output nobody is looking at. +**The dot is coloured by outcome, and the panel is inset because the menu is +not.** `status` is `running|done|killed|lost`, and `done` is two outcomes — so +`jobs__dot--done` would have been green beside the row's own words "Failed, exit +2". `JobView.tone` answers the colour question and the template's if-chain keeps +answering the wording one, which is the half that cannot live in a class name. +`duration` is empty for a *running* job on purpose: this panel is fetched when +somebody opens it and is never polled (the chip is the thing on a timer), so a +live figure would be frozen the instant it painted. Its two stamps are normalised +before subtracting, for the reason `compaction.moment` exists — a job started +before a restart and finished after it has one naive stamp and one aware, and +subtracting them raises. `_short_duration` here is deliberately not `steps`'s: +that one takes milliseconds and tops out at minutes, and a three-hour build +through it reads `184m 12s`. And `.jobs__row` had no horizontal padding while +`.picker__menu` has none either, so every row ran flush into the border under a +header that was inset by `--sp-3`; `jobs__row--open` had been emitted by the +template since the panel shipped with no rule anywhere to render it, which is why +the row whose log was on screen looked like the ones that were not. + +**The open chat page polls for turns it has not got.** `jobs.wake` starts a reply +without any request from the browser, and there is no channel to say so: the only +stream is per-message and it is opened by the `sse-connect` on an incomplete +assistant bubble — a bubble the page does not have, because the reply that made it +began elsewhere. `_queue_frames` proves the swap works but can only ride a stream +already open, so a job finishing on an idle chat lit the sidebar dot for the chat +the reader was *looking at* and did nothing else until a reload. +`GET /api/chats/{id}/tail?after=` is the answer, polled for the reason `/unread` +is. Four things about it: + +- **A cursor it cannot place is answered with 204, never with the transcript.** + An absent `after`, one from another chat, one a rewind deleted: returning the + thread would append a second copy of every bubble the page still holds. A page + whose history was rewritten underneath it is one only a reload can reconcile, + and that is not this route's call to make with a half-typed message in the box. +- **The cut is read from the row**, so `_inject`'s restamp of the placeholder + moves it too, and the comparison is done **in SQL** — a row read back from + SQLite is naive and one still in the session is aware. The `id >` tie-breaker + is not decoration: under a bare `>` a row sharing the cut's microsecond is + skipped forever. +- **The cursor comes from the DOM** (`app.js`, on `htmx:configRequest`), because + the DOM is the honest answer to what the page holds — the composer's POST, the + `done` frame and the last poll all move it, and a variable would have to be + updated by each of them forever. Not `hx-vals="js:…"`: two of the three things + that handler does are *cancellations*, which `hx-vals` cannot express. Not + `article.msg:last-of-type` either — that is per-parent, so on a compacted chat + it answers with the last article inside the `
` rather than the newest + message, and the poll then re-appends half the conversation. +- **It is silent while a reply is streaming**, and a `htmx:beforeSwap` listener + drops any answer containing a bubble the page already has. `hx-sync` cannot + reach that race — the two requests come from different elements — and a + duplicate here is not cosmetic, it is a second `sse-connect` for one message. + +The route also clears `unread`/`unread_notified` on every tick **including the +204**: `_persist` marks a reply unread whenever `followers == 0`, which is true of +a job-woken reply with the reader watching it. The element lives outside `#thread` +(a rewind swaps that container's contents and would take the poller with it) and +outside the composer form (which would lend it `hx-target="#thread"`, the jobs +chip's old bug); `tests/test_chat_tail.py` walks the page and refuses both. + +**A background job's completion is a user turn on the wire and a machine event on +screen.** The role is load-bearing — `_inject` sends a queued turn verbatim and +`build_messages` must keep seeing a user turn — so `Message.machine` marks the +bubble instead, and nothing about the request changes. Without it the transcript +rendered a machine's report under the reader's name with their initial beside it +and a pencil offering to rewrite it, which is the application putting words in +their mouth; the route refuses the edit too, because a hidden button is a +courtesy. `_completion_text` is deliberately untouched: the `tool.background` +fragment quotes its opening sentence to the model, so rewording it would break +that instruction with nothing anywhere to notice. The body skips the `tokens` +filter — an `@` in a command line is not a mention of anybody's files. + **Reasoning effort goes out twice, and only when it is set.** There is no field that works everywhere. OpenAI and vLLM read `reasoning_effort`; llama.cpp's own documentation says other values "have no effect", its maintainer says diff --git a/PLAN.md b/PLAN.md index d8e4391..e667aca 100644 --- a/PLAN.md +++ b/PLAN.md @@ -50,6 +50,14 @@ be a different project, not a refactor. - [x] Chats created on first message, so an abandoned composer leaves nothing - [x] **Unread indicator** — a green dot and a toast when a reply lands while you were elsewhere +- [x] **A reply that started without you asking still arrives** — the open chat + page polls for turns it has not got, so a background job waking the model + appears where you are looking instead of only after a reload. Quiet while + a reply is streaming, since that reply delivers its own bubbles +- [x] **A turn nobody typed says so** — a background job's completion is a user + turn on the wire, because the request needs one, and a machine event in + the transcript: its own icon and name, no pencil, and no claim that you + sent it - [x] **Folders that carry something** — arbitrarily nested, with a name, a description, a system prompt inherited by the chats inside them, and seeds for the model, the kind and the agent target. Deleting one keeps the chats @@ -136,8 +144,10 @@ be a different project, not a refactor. and both re-point when that changes. The shell you opened and the files you left open are adopted into the chat when you send the first prompt - [x] **Background jobs are visible** — a chip in the composer row counting what - is still running, and a panel with each job's command, state, log tail and - a Stop button. Survives a restart, because the job does + is still running, and a panel with each job's command, state, log tail, + how long it took and a Stop button. The dot is coloured by outcome rather + than by status, since `done` covers exit 0 and exit 2 alike. Survives a + restart, because the job does - [x] `shell_run`, `file_read`, `file_write`, `file_list` — files over SFTP, never through a shell, because the SSH exec protocol has no argv form - [x] **Plan mode ends with a plan** you can carry out with one button, which diff --git a/src/lembas/api/chats.py b/src/lembas/api/chats.py index e8bc481..58b9a5d 100644 --- a/src/lembas/api/chats.py +++ b/src/lembas/api/chats.py @@ -12,7 +12,7 @@ from types import SimpleNamespace from fastapi import APIRouter, Depends, Form, HTTPException, Request, status from fastapi.responses import HTMLResponse, JSONResponse, Response, StreamingResponse -from sqlalchemy import func, select +from sqlalchemy import and_, func, or_, select from sqlalchemy.orm import Session as DBSession from lembas.api.deps import Db, RequiredUser, require_permission @@ -723,6 +723,91 @@ async def unread_poll(db: Db, user: RequiredUser) -> Response: return response +@router.get("/{chat_id}/tail") +async def thread_tail(db: Db, user: RequiredUser, chat_id: str, after: str = "") -> Response: + """Turns this page has not got yet, appended to the transcript it is showing. + + A reply can begin without a request from the browser: `jobs.wake` writes a + completion turn and calls `generation.ensure` when a background job finishes + on an idle chat. There is no channel to tell the page about it. The only + stream here is per-message and it is opened by the `sse-connect` on an + incomplete assistant bubble -- a bubble this page does not have, because the + reply that created it started somewhere else. `_queue_frames` proves the swap + works, but it can only ride a stream that is already open. + + So the page asks. Polled for the same reason `/unread` is: a second always-on + connection per tab is a great deal of machinery for something that happens a + few times a day. The cursor comes from the browser -- see `app.js`, which + reads the last bubble in `#thread`, the honest answer to what this page + already holds. + """ + chat = _owned_chat(db, chat_id, user.id) + + # Somebody is looking at this chat, which is what `unread` means the absence + # of. `_persist` marks a reply unread whenever `generation.followers == 0`, + # and that is true of a job-woken reply even with the reader watching it -- + # so today the toast announces a chat that is already on screen. This is + # `pages.chat_detail` said again for as long as the page stays open rather + # than once when it loads, and it is cleared whether or not anything arrived: + # the claim being made is that somebody is here. + # + # Not airtight, and not pretending to be: the sidebar polls on 10s and this + # on 5s, so this usually wins, but a badly timed tick can still raise one + # toast for the chat in front of you. + if chat.unread or chat.unread_notified: + chat.unread = False + chat.unread_notified = False + db.commit() + + # No cursor, a cursor from another chat, or one naming a row a rewind has + # since deleted. Answering with the transcript would append a second copy of + # every bubble the page still holds, and a page whose history was rewritten + # underneath it is one only a reload can reconcile -- which is not this + # route's decision to make, with a half-typed message possibly in the box. + cut = db.get(Message, after) if after else None + if cut is None or cut.chat_id != chat.id: + return Response(status_code=status.HTTP_204_NO_CONTENT) + + # The cut is read from the row rather than taken as a timestamp on the wire, + # which is what makes `_inject`'s restamp harmless: if the page's last bubble + # was the assistant placeholder and the placeholder moved, the cut moves with + # it. Compared in SQL and never in Python, for the reason `compaction.moment` + # exists -- a row read back from SQLite is naive and one still in the session + # is aware, and `>` between them raises. + # + # The id clause is not decoration. Under a bare `>` a row sharing the cut's + # microsecond is skipped forever; with it, at most the one sorting lower is. + fresh = list( + db.scalars( + select(Message) + .where( + Message.chat_id == chat.id, + or_( + Message.created_at > cut.created_at, + and_(Message.created_at == cut.created_at, Message.id > cut.id), + ), + ) + .order_by(Message.created_at, Message.id) + ) + ) + if not fresh: + # 204 and not an empty 200: htmx does not swap on a 204, where an empty + # body would still fire a swap and a settle on every open page every + # five seconds. + return Response(status_code=status.HTTP_204_NO_CONTENT) + + # Queued turns come too, unfiltered. A completion waiting behind a running + # reply is exactly what the reader wants to watch arrive, and its bubble can + # never carry `sse-connect` -- `_message.html` requires the assistant role + # for that. When the running reply ends, `_queue_frames` deletes the stale + # node out of band and re-renders it in place, so arriving early costs + # nothing. + # + # No `just_finished`: that flag is what read-aloud-automatically keys off, + # and a bubble the page merely missed must not start talking. + return HTMLResponse("".join(_render_bubble(db, chat, user, row) for row in fresh)) + + @router.post("/{chat_id}/messages") async def post_message( request: Request, @@ -1230,6 +1315,11 @@ async def edit_form( message = db.get(Message, message_id) if message is None or message.chat_id != chat.id or message.role != ROLE_USER: raise HTTPException(status.HTTP_404_NOT_FOUND, "That message no longer exists.") + # See `edit_message` for why, and for why this is not the same sentence. + if message.machine: + raise HTTPException( + status.HTTP_404_NOT_FOUND, "A background job's message cannot be edited." + ) return templates.TemplateResponse( request, @@ -1287,6 +1377,14 @@ async def edit_message( message = db.get(Message, message_id) if message is None or message.chat_id != chat.id or message.role != ROLE_USER: raise HTTPException(status.HTTP_404_NOT_FOUND, "That message no longer exists.") + # The bubble hides the pencil, but a hidden button is a courtesy and this is + # the rule: editing rewinds and re-sends under the reader's own authority, + # and what a machine reported is not theirs to rewrite. Its own sentence, + # because "no longer exists" would be false and would leave nothing to do. + if message.machine: + raise HTTPException( + status.HTTP_404_NOT_FOUND, "A background job's message cannot be edited." + ) content = content.strip() if not content and not message.attachments: diff --git a/src/lembas/db/models/chat.py b/src/lembas/db/models/chat.py index 9fbe4e3..badb928 100644 --- a/src/lembas/db/models/chat.py +++ b/src/lembas/db/models/chat.py @@ -333,6 +333,16 @@ class Message(UUIDPrimaryKey, Timestamps, Base): # of tool calls -- is the only thing that clears it. queued: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False) + # Written by the application rather than by the person whose bubble this + # would otherwise be. `agent/jobs.py:wake` is the one writer: a background + # job finishing is a new turn in the *user* role, and that role is + # load-bearing -- `_inject` sends a queued turn verbatim and `build_messages` + # has to keep seeing a user turn -- but it is not the reader speaking, and + # rendering it under their name with their initial beside it is the + # application putting words in their mouth. Nothing about the request + # changes; only the bubble does. + machine: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False) + chat: Mapped[Chat] = relationship(back_populates="messages") attachments: Mapped[list[Attachment]] = relationship( # noqa: F821 back_populates="message", diff --git a/src/lembas/services/agent/jobs.py b/src/lembas/services/agent/jobs.py index bc81b06..6098b94 100644 --- a/src/lembas/services/agent/jobs.py +++ b/src/lembas/services/agent/jobs.py @@ -42,6 +42,7 @@ import re import time import uuid from dataclasses import dataclass, field +from datetime import UTC, datetime from typing import Any from sqlalchemy import select @@ -381,6 +382,68 @@ class JobView: def running(self) -> bool: return self.status == "running" + @property + def tone(self) -> str: + """What colour this job is, which is not the question `status` answers. + + `done` is two outcomes. The row beside the dot already tells them apart + in words -- "Finished" against "Failed, exit 2" -- so a dot keyed on the + status would be green next to a sentence saying the opposite. + + The *wording* stays in the template's if-chain rather than moving here + beside the colour. Authored text belongs in the file somebody reads to + change it, and saving one branch is not worth taking five phrases out of + it; this is the half that cannot be said in a class name. + """ + if self.running: + return "running" + if self.status != "done": + return self.status # killed, lost + return "ok" if not self.exit_status else "failed" + + @property + def duration(self) -> str: + """How long it took, once it is over. Empty while it is still running. + + Empty on purpose rather than for want of an answer. This panel is + fetched when somebody opens it and is never polled -- the chip beside + the composer is what refreshes on a timer -- so a live "running for + 2m 05s" would be stale the instant it painted and stay stale until the + reader pressed something. The chip says something is still going; this + says how long the finished ones took, which is true forever. + + Both stamps are normalised before subtracting, for the reason + `compaction.moment` normalises: SQLite stores no offset, so a row read + back from disk is naive while one still in the session's identity map + keeps its tzinfo, and subtracting one from the other raises. `moment` + itself is not reused because it takes a `Message`, not a stamp. + """ + if self.running or self.started_at is None or self.finished_at is None: + return "" + seconds = (_aware(self.finished_at) - _aware(self.started_at)).total_seconds() + return _short_duration(seconds) if seconds >= 0 else "" + + +def _aware(stamp: datetime) -> datetime: + """A stamp that can be subtracted from another. See `JobView.duration`.""" + return stamp if stamp.tzinfo is not None else stamp.replace(tzinfo=UTC) + + +def _short_duration(seconds: float) -> str: + """A wall-clock span, at the precision somebody reading a log cares about. + + Deliberately not `steps._short_duration`. That one takes milliseconds, tops + out at minutes and is tuned to a label repainting beside an animating word; + a three-hour build through it reads `184m 12s`. This one is written for a + span that can be hours and is only ever rendered once it is final. + """ + total = int(seconds) + if total < 60: + return f"{total}s" + if total < 3600: + return f"{total // 60}m {total % 60:02d}s" + return f"{total // 3600}h {(total % 3600) // 60:02d}m" + def listing(db, chat_id: str) -> list[JobView]: """Every job this chat has, newest first. @@ -453,8 +516,6 @@ def clear() -> None: # chat, a transient database hiccup) still runs and is still tracked in-process; # it just will not survive a restart, which is the row's only purpose. def _persist_row(job: JobState) -> None: - from datetime import UTC, datetime - from lembas.db.models import Job from lembas.db.session import session_scope @@ -614,7 +675,14 @@ async def wake( chat = db.get(Chat, chat_id) if chat is None: return - chat_service.create_message(db, chat, ROLE_USER, content, queued=running) + # `machine` changes the bubble and nothing else: the role stays + # `user` because `_inject` sends a queued turn verbatim and the + # request must carry a user turn, and the framing the *model* + # reads is in the words `_completion_text` wrote. What it stops + # is the transcript claiming the reader typed this. + chat_service.create_message( + db, chat, ROLE_USER, content, queued=running, machine=True + ) if not running: assistant = chat_service.create_message( db, chat, ROLE_ASSISTANT, "", complete_=False, model_id=chat.model_id diff --git a/src/lembas/services/chat.py b/src/lembas/services/chat.py index f24e9fa..c4d67bb 100644 --- a/src/lembas/services/chat.py +++ b/src/lembas/services/chat.py @@ -530,6 +530,7 @@ def create_message( complete_: bool = True, model_id: str = "", queued: bool = False, + machine: bool = False, ) -> Message: message = Message( chat_id=chat.id, @@ -538,6 +539,7 @@ def create_message( complete=complete_, model_id=model_id, queued=queued, + machine=machine, ) db.add(message) db.commit() diff --git a/src/lembas/web/static/css/chat.css b/src/lembas/web/static/css/chat.css index f934ee8..90be814 100644 --- a/src/lembas/web/static/css/chat.css +++ b/src/lembas/web/static/css/chat.css @@ -740,6 +740,30 @@ .msg:focus-within .msg__actions { opacity: 1; } .msg__actions .is-copied { color: var(--success); } +/* --- A turn nobody typed --------------------------------------------------- + A background job finishing is a new turn in the user role, because that is + what the request needs it to be -- but the bubble is not the reader's, and it + must not be wearing their initial or their name. Sunken and quiet rather than + the user bubble's raised fill: this is a report, not something somebody said. + + `.msg--user.msg--machine` is a compound out of necessity, not for emphasis: + `.msg--user .msg__body--plain` above already sets `--bubble-user`, and one + class cannot beat two without matching its specificity. + + The body font stays the page's. The first lines are a sentence and only the + tail is a log, so setting the whole block in mono to suit the log makes the + sentence worse without making the log better. */ +.msg--machine .msg__gutter { background: var(--surface-active); color: var(--ink-faint); } +.msg--machine .msg__author { color: var(--ink-muted); font-weight: 500; } +.msg--user.msg--machine .msg__body--plain { + background: var(--bg-sunken); + border-inline-start: 2px solid var(--border-strong); + border-start-start-radius: var(--radius-sm); + border-end-start-radius: var(--radius-sm); + color: var(--ink-muted); + font-size: var(--text-sm); +} + /* --- A turn that is waiting to be sent ------------------------------------ Its actions do not fade in on hover like the others: they are the only way to withdraw something that has not happened yet, and a control you have to @@ -1086,8 +1110,29 @@ .picker__menu--jobs { width: min(34rem, 92vw); } -.jobs__row { padding: var(--sp-2) 0; border-bottom: 1px solid var(--border); } +/* Inset to match `.picker__group` and `.picker__lede` above them. The menu has + no padding of its own (app.css), so a row with none ran flush into the border + while the header over it sat --sp-3 in. The padding goes inside the row and + the border stays on it, so the divider is still full-bleed -- which is what + makes a stack of rows read as a list rather than as paragraphs. */ +.jobs__row { padding: var(--sp-2) var(--sp-3); border-bottom: 1px solid var(--border); } .jobs__row:last-child { border-bottom: 0; } + +/* Which row's log is on screen. An inset shadow rather than a + `border-inline-start`, which would take its 2px out of the row's width and + shift the open row's text against every closed one above it. */ +.jobs__row--open { background: var(--surface-active); box-shadow: inset 2px 0 0 var(--accent); } + +/* The whole row lights up, driven by the one thing in it you can press. The + command button spans the row's content but not its padding, so hovering the + band between two rows would otherwise light nothing. `:has()` is already how + `.interaction__option` follows its own input. + + No `:focus-visible` rule here, and that is not an omission: app.css gives + every focusable element an accent outline, and a second one written locally + is a copy that drifts. */ +.jobs__row:has(.jobs__command:hover) { background: var(--surface-hover); } + .jobs__head { display: flex; align-items: center; gap: var(--sp-2); } .jobs__dot { @@ -1097,9 +1142,15 @@ border-radius: var(--radius-full); background: var(--ink-muted); } +/* Keyed on `JobView.tone`, not on `status`: `done` is both exit 0 and exit 2. + `lost` is muted rather than red to match what the row says in words -- the + host rebooted or /tmp was cleared, so nothing failed and we simply cannot say + how it ended. */ .jobs__dot--running { background: var(--accent); } +.jobs__dot--ok { background: var(--success); } +.jobs__dot--failed { background: var(--danger); } .jobs__dot--killed { background: var(--warning); } -.jobs__dot--lost { background: var(--danger); } +.jobs__dot--lost { background: var(--ink-muted); } /* The command is the button: the row is wide, the affordance should be too. `min-width: 0` or the flex item will not shrink below its content and the @@ -1122,6 +1173,7 @@ white-space: nowrap; font-size: var(--text-xs); } +.jobs__command:hover code { color: var(--accent); } .jobs__stop { flex: none; } .jobs__meta { diff --git a/src/lembas/web/static/js/app.js b/src/lembas/web/static/js/app.js index a600627..54571d2 100644 --- a/src/lembas/web/static/js/app.js +++ b/src/lembas/web/static/js/app.js @@ -752,6 +752,80 @@ scrollThread(false); }); + /* --- The transcript tail ----------------------------------------------- + A reply can begin without a request from this page: a background job + finishing wakes the chat server-side. There is no chat-level channel to + hear about it on -- the only stream is per-message, and it is opened by a + bubble this page has not got. So `#thread-tail` polls, and this is where it + is told what the page already holds. + + The cursor is read from the DOM rather than from a variable rendered into + the page, because the DOM is the honest answer to that question. Every path + that appends a bubble moves it -- the composer's own POST, the `done` + frame's out-of-band swaps, the last poll -- and a variable would have to be + updated by each of them, correctly, forever. + + `htmx:configRequest` and not `hx-vals="js:…"`: two of the three things here + are *cancellations*, which `hx-vals` cannot express, and splitting the read + from the cancellations would put one decision in two files. (It is also the + only string htmx would ever be handed to evaluate in this project, and it + would die silently under a CSP.) */ + document.body.addEventListener("htmx:configRequest", function (event) { + var elt = (event.detail && event.detail.elt) || event.target; + if (!elt || elt.id !== "thread-tail") return; + + var thread = document.getElementById("thread"); + if (!thread) return event.preventDefault(); + + /* Quiet while this page is following a reply. That reply delivers its own + bubbles through the `done` frame, which is the only channel that can get + the *order* right -- and it is the one window in which the transcript's + order moves underneath us, since `_inject` restamps the placeholder to + sort after a prompt taken into it. Asking during it is how a page appends + a bubble it already has. + + Exact rather than approximate: a page holding an incomplete assistant + bubble always carries this attribute, which is the state machine + `_message.html` documents. */ + if (thread.querySelector("[sse-connect]")) return event.preventDefault(); + + /* Deliberately not `article.msg:last-of-type`. That is per-parent, and + `querySelector` returns the first match in document order -- so on a + compacted chat it answers with the last article inside + `
` rather than the newest message. The last of + everything matching is what "the last bubble this page holds" means. */ + var articles = thread.querySelectorAll("article.msg"); + var last = articles.length ? articles[articles.length - 1] : null; + if (!last || last.id.indexOf("msg-") !== 0) return event.preventDefault(); + + event.detail.parameters.after = last.id.slice(4); + }); + + /* Whatever the reason -- the composer's POST committing between this request + going out and its answer coming back, a `done` frame landing first -- if any + bubble in the answer is already on the page then the page has moved on since + the question was asked, and swapping would duplicate it. A duplicate here is + not cosmetic: it would carry a second `sse-connect` for one message. + + This is the race `hx-sync` cannot reach, since the two requests come from + different elements. The whole answer is dropped rather than filtered: the + next poll is five seconds away and recomputes its cursor from a DOM that has + settled, which is a correct page one tick late instead of a wrong one now. */ + document.body.addEventListener("htmx:beforeSwap", function (event) { + var elt = (event.detail && event.detail.elt) || event.target; + if (!elt || elt.id !== "thread-tail" || !event.detail) return; + + var seen = /\bid="(msg-[^"]+)"/g; + var body = event.detail.serverResponse || ""; + var match; + while ((match = seen.exec(body)) !== null) { + if (document.getElementById(match[1])) { + event.detail.shouldSwap = false; + return; + } + } + }); + /* Tokens arriving over SSE are appended outside the normal swap cycle. Narrowed to frames that land in the thread. `metrics`, `status`, `ask` and diff --git a/src/lembas/web/templates/chat/_jobs_panel.html b/src/lembas/web/templates/chat/_jobs_panel.html index dcd637d..596897b 100644 --- a/src/lembas/web/templates/chat/_jobs_panel.html +++ b/src/lembas/web/templates/chat/_jobs_panel.html @@ -22,7 +22,10 @@ {% for job in jobs %}
- {# The whole command in the title, a truncated one on screen. A command line @@ -61,6 +64,9 @@ Finished {% endif %} {% if job.started_at %}· started {{ job.started_at.strftime("%H:%M") }}{% endif %} + {# Only ever on a finished job -- see JobView.duration for why a running one + says nothing rather than saying something frozen. #} + {% if job.duration %} · took {{ job.duration }}{% endif %}

{% if open_job == job.id %} diff --git a/src/lembas/web/templates/chat/_message.html b/src/lembas/web/templates/chat/_message.html index 858b6c6..d147e7e 100644 --- a/src/lembas/web/templates/chat/_message.html +++ b/src/lembas/web/templates/chat/_message.html @@ -20,8 +20,17 @@ second concurrent reply the queue exists to prevent. #} {% set queued = (message.role == "user" and message.queued) %} +{# + Written by the application, in the user role, and not by the person whose + bubble this would otherwise be -- a background job reporting that it finished. + The wire role is deliberately unchanged (see Message.machine), so this is the + only place in the whole request the difference exists. `msg--user` stays on the + article as well, so the bubble keeps the layout it already had and + `msg--machine` only overrides what should differ. +#} +{% set machine = (message.role == "user" and message.machine) %} -
{{ (user.name or "?")[0]|upper }} {% endif %} @@ -49,6 +64,8 @@ {% if message.role == "assistant" %} {{ speaking_model.label if speaking_model else "LLeMbas" }} + {% elif machine %} + Background job {% else %} {{ user.name or "You" }} {% endif %} @@ -194,6 +211,14 @@ {% if message.stopped %}

{{ icon("x", "icon--sm") }} Stopped. This reply is cut short.

{% endif %} + {% elif machine and message.content %} + {# Jinja's own escaping, not `tokens`: that filter marks `@name` as a + reference to this reader's files and people, and an `@` inside a + command line or a log is neither. Nothing here goes through + services/markdown.py either -- the fence in the content stays a fence + on screen, which is the rule `_jobs_panel.html` states about the log + it shows for the same reason: it came off somebody else's machine. #} +
{{ message.content }}
{% elif message.content %} {# `tokens` escapes and then marks up: @mentions read as references rather than as punctuation. It must stay `pre-wrap` -- the newlines @@ -226,6 +251,13 @@ a pencil. Discard and retype is the honest affordance. #}
{{ icon("clock", "icon--sm") }} Waiting to be sent + {# Copy is here as well as on a delivered turn: a queued machine event + holds a job's output, which is the one waiting bubble somebody actually + wants to paste somewhere. The hidden source div is already below. #} + @@ -241,7 +273,11 @@ data-copy="msg-body-{{ message.id }}" aria-label="Copy message"> {{ icon("copy", "icon--sm") }} - {% if message.role == "user" %} + {# Not on a machine event: editing rewinds the transcript and starts a + reply from the edited words, so a pencil here offers to rewrite what a + machine reported and re-send it under the reader's own authority. The + route refuses it too -- this only removes the button. #} + {% if message.role == "user" and not machine %}
+ {% if chat %} + {# + Where a reply that started outside a request arrives. A background job + finishing wakes the chat server-side and there is no channel to say so: + the only stream is per-message, opened by a bubble this page has not got. + See `GET /api/chats/{id}/tail`, and `app.js` for the cursor -- which is + read from the DOM on `htmx:configRequest`, not rendered here, because the + DOM is the honest answer to what this page already holds. + + OUTSIDE `#thread`, not in it: a rewind and a compaction both swap + `chat/_thread.html` into that container with `innerHTML`, and a poller + living inside would be swapped away by the first one and never fire + again. OUTSIDE the composer's form as well, and it names its own target + regardless -- htmx inherits `hx-target`, the form carries `#thread`, and + the jobs chip has already demonstrated once what an unstated target does + to a transcript. + + `hx-sync="this:drop"` because a poll fires whether or not the last one + has come back, and two answers computed from the same cursor are two + copies of one bubble. + #} + + {% endif %} + {% include "chat/_composer.html" %} {% endif %} diff --git a/tests/test_agent_jobs.py b/tests/test_agent_jobs.py index 7c4ff91..6d10993 100644 --- a/tests/test_agent_jobs.py +++ b/tests/test_agent_jobs.py @@ -436,6 +436,138 @@ async def test_two_jobs_finishing_at_once_start_one_reply(db, user_id, registere assert len(started) == 1, "the lock made the second wake see the first's reply" +async def test_a_completion_is_marked_as_a_machine_event(db, user_id, registered, monkeypatch): + """The role stays `user` -- `_inject` sends a queued turn verbatim and + `build_messages` has to keep seeing a user turn -- and `machine` is what + stops the transcript claiming the reader typed it.""" + from lembas.services import generation as generation_service + + chat_id = _chat(db, user_id) + monkeypatch.setattr(generation_service, "running_for", lambda _c: None) + monkeypatch.setattr(generation_service, "ensure", lambda c, m: None) + + await jobs.wake(chat_id, "abc123abc123", "sleep 1", "done", 0, "hi") + + from lembas.db.models import Message + + db.expire_all() + users = [ + m for m in db.query(Message).filter(Message.chat_id == chat_id).all() if m.role == "user" + ] + assert users[0].role == "user", "the wire role is load-bearing and must not move" + assert users[0].machine is True + + +async def test_a_queued_completion_is_marked_too(db, user_id, registered, monkeypatch): + """The busy path writes the same row with `queued` set; it must not lose the + marking on the way, or a completion delivered by `_drain` arrives wearing the + reader's name.""" + from lembas.services import generation as generation_service + + chat_id = _chat(db, user_id) + monkeypatch.setattr(generation_service, "running_for", lambda _c: object()) + monkeypatch.setattr(generation_service, "ensure", lambda c, m: None) + + await jobs.wake(chat_id, "abc123abc123", "sleep 1", "done", 0, "hi") + + from lembas.db.models import Message + + db.expire_all() + users = [ + m for m in db.query(Message).filter(Message.chat_id == chat_id).all() if m.role == "user" + ] + assert users[0].queued is True + assert users[0].machine is True + + +def test_the_prompt_quotes_the_words_the_completion_actually_carries(): + """`tool.background` tells the model a completion "begins" with a particular + sentence, so that it reads one as a machine event rather than as the person + speaking. Rewording `_completion_text` breaks that instruction, in a way + nothing else here would notice -- the turn still arrives, the model just + stops being told what it is.""" + from lembas.services import prompts as prompts_service + + text = jobs._completion_text("abc123abc123", "pytest -q", "done", 0, "") + opening = "A background job you started has finished" + assert text.startswith(opening) + + fragment = next(f for f in prompts_service.BUILTIN if f.key == "tool.background") + assert opening in fragment.default + + +# --- What a job looks like in the panel ---------------------------------------- +@pytest.mark.parametrize( + ("status", "exit_status", "expected"), + [ + ("running", None, "running"), + ("done", 0, "ok"), + ("done", 2, "failed"), + ("killed", 143, "killed"), + ("lost", None, "lost"), + ], +) +def test_the_dot_tells_a_failure_from_a_success(status, exit_status, expected): + """`status` is `done` for exit 0 and exit 2 alike, and the row beside the dot + already says "Finished" or "Failed, exit 2". A dot keyed on the status would + be green next to the sentence contradicting it.""" + view = jobs.JobView(id="a", command="x", status=status, exit_status=exit_status) + assert view.tone == expected + + +def test_a_finished_job_says_how_long_it_took(): + from datetime import UTC, datetime, timedelta + + started = datetime(2026, 8, 5, 14, 0, tzinfo=UTC) + view = jobs.JobView( + id="a", + command="x", + status="done", + exit_status=0, + started_at=started, + finished_at=started + timedelta(seconds=252), + ) + assert view.duration == "4m 12s" + + +def test_a_running_job_reports_no_duration(): + """Not for want of an answer. The panel is fetched when somebody opens it and + never polled, so a live figure would be frozen the instant it painted.""" + from datetime import UTC, datetime + + view = jobs.JobView( + id="a", + command="x", + status="running", + started_at=datetime(2026, 8, 5, 14, 0, tzinfo=UTC), + ) + assert view.duration == "" + + +def test_a_job_with_no_row_reports_no_duration(): + """`_persist_row` is best-effort by design, so a job with no stamps is a real + case rather than a defensive one.""" + assert jobs.JobView(id="a", command="x", status="done", exit_status=0).duration == "" + + +def test_a_duration_survives_a_stamp_read_back_from_disk(): + """SQLite stores no offset, so a row loaded from disk comes back naive while + one still in the session's identity map keeps its tzinfo -- and a job started + before a restart and finished after it has one of each. Subtracting them + without normalising raises, and it raises in the panel, not in a test.""" + from datetime import UTC, datetime + + view = jobs.JobView( + id="a", + command="x", + status="done", + exit_status=0, + started_at=datetime(2026, 8, 5, 14, 0), # naive, as SQLite hands it back + finished_at=datetime(2026, 8, 5, 15, 6, tzinfo=UTC), + ) + assert view.duration == "1h 06m" + + # --- The watcher, end to end --------------------------------------------------- async def test_the_watcher_end_to_end(db, user_id, registered, monkeypatch, tmp_path): from lembas.services.agent import ssh as ssh_service diff --git a/tests/test_chat.py b/tests/test_chat.py index 9b1e6fc..b994f57 100644 --- a/tests/test_chat.py +++ b/tests/test_chat.py @@ -689,6 +689,103 @@ def test_cancelling_an_edit_restores_the_bubble(client: TestClient, db, register assert "edit-form" not in page +# --- A turn nobody typed ------------------------------------------------------ +def _machine_turn(db, chat_id: str, content: str = "A background job you started has finished"): + """What `jobs.wake` writes: a user-role turn the application produced.""" + from lembas.db.models import Chat + from lembas.services import chat as chat_service + + chat = db.get(Chat, chat_id) + return chat_service.create_message(db, chat, "user", content, machine=True) + + +def test_a_machine_turn_is_not_shown_as_the_readers_own( + client: TestClient, db, registered, make_chat +): + """A background job finishing is a user turn because the request needs it to + be, not because the reader said it. Rendering it under their name with their + initial beside it is the application putting words in their mouth.""" + _add_connection(db) + chat_id = make_chat() + _machine_turn(db, chat_id) + + page = client.get(f"/chat/{chat_id}").text + + assert "msg--machine" in page + assert "Background job" in page + assert "msg__initial" not in page, "no initial in the gutter for a turn nobody typed" + + +def test_a_machine_turn_offers_no_pencil(client: TestClient, db, registered, make_chat): + """Editing rewinds and re-sends under the reader's own authority, and what a + machine reported is not theirs to rewrite.""" + _add_connection(db) + chat_id = make_chat() + event = _machine_turn(db, chat_id) + + page = client.get(f"/chat/{chat_id}").text + + assert f"/messages/{event.id}/edit" not in page + + +def test_a_machine_turn_cannot_be_edited(client: TestClient, db, registered, make_chat): + """The hidden button is a courtesy; the route is the rule.""" + _add_connection(db) + chat_id = make_chat() + event = _machine_turn(db, chat_id) + + assert client.get(f"/api/chats/{chat_id}/messages/{event.id}/edit").status_code == 404 + assert ( + client.post( + f"/api/chats/{chat_id}/messages/{event.id}/edit", + data={"content": "something I would rather it had said"}, + ).status_code + == 404 + ) + + +def test_a_machine_turn_keeps_its_output(client: TestClient, db, registered, make_chat): + """The fenced log is most of why somebody reads one of these at all.""" + _add_connection(db) + chat_id = make_chat() + _machine_turn( + db, + chat_id, + "[job abc] `pytest -q`\nIt finished successfully.\n\n```\n1529 passed\n```", + ) + + page = client.get(f"/chat/{chat_id}").text + + assert "1529 passed" in page + + +def test_a_machine_turn_still_reaches_the_model_as_a_user_turn(db, registered, make_chat): + """The wire role is load-bearing: `_inject` sends a queued turn verbatim and + every template requires the first non-system message to be `user`. `machine` + changes the bubble and nothing else.""" + from lembas.db.models import Chat + from lembas.services import chat as chat_service + + chat_id = make_chat() + _machine_turn(db, chat_id, "a job finished") + + sent = chat_service.build_messages(db, db.get(Chat, chat_id)) + + assert [(m["role"], m["content"]) for m in sent] == [("user", "a job finished")] + + +def test_an_ordinary_turn_is_not_a_machine_event(db, registered, make_chat): + """Every row written before the column reads the same way, because + `sync_schema` adds a NOT NULL boolean with a literal default of 0.""" + from lembas.db.models import Chat + from lembas.services import chat as chat_service + + chat_id = make_chat() + chat = db.get(Chat, chat_id) + + assert chat_service.create_message(db, chat, "user", "hello").machine is False + + # --- Background generation --------------------------------------------------- def test_sending_launches_the_generation_immediately( client: TestClient, db, registered, make_chat diff --git a/tests/test_chat_tail.py b/tests/test_chat_tail.py new file mode 100644 index 0000000..5f93358 --- /dev/null +++ b/tests/test_chat_tail.py @@ -0,0 +1,411 @@ +"""Turns that arrive without the page having asked for them. + +A reply can begin outside a request: `jobs.wake` writes a completion turn and +calls `generation.ensure` when a background job finishes on an idle chat. The +browser has no way to hear about it -- the only stream here is per-message and +it is opened by the `sse-connect` on an incomplete assistant bubble, which is a +bubble the page does not have, because the reply that created it started +somewhere else. So the page polls, and this is that poll. + +Most of what can go wrong is a duplicate or an avalanche: a cursor the server +cannot place must never be answered with the whole transcript, because the page +still holds every one of those bubbles. +""" + +from __future__ import annotations + +from html.parser import HTMLParser + +import pytest +from fastapi.testclient import TestClient + +from lembas.db.models import ROLE_ASSISTANT, ROLE_USER, Chat, Connection, Message, Model +from lembas.services import chat as chat_service +from lembas.services.crypto import encrypt + + +@pytest.fixture +def connection(db): + connection = Connection( + name="Test", base_url="http://127.0.0.1:1", api_key_encrypted=encrypt("") + ) + db.add(connection) + db.commit() + db.add(Model(connection_id=connection.id, model_id="test-model")) + db.commit() + return connection + + +def _tail(client: TestClient, chat_id: str, after: str = ""): + return client.get(f"/api/chats/{chat_id}/tail", params={"after": after} if after else {}) + + +def _say(db, chat_id: str, role: str, content: str, **kwargs) -> Message: + chat = db.get(Chat, chat_id) + return chat_service.create_message(db, chat, role, content, **kwargs) + + +# --- The cursor ---------------------------------------------------------------- +def test_nothing_new_is_a_204(client, db, registered, connection, make_chat): + """A 204 and not an empty 200: htmx does not swap on a 204, where an empty + body would still fire a swap and a settle on every open page every five + seconds.""" + chat_id = make_chat() + last = _say(db, chat_id, ROLE_USER, "hello") + + assert _tail(client, chat_id, last.id).status_code == 204 + + +def test_a_turn_that_arrived_since_is_handed_back(client, db, registered, connection, make_chat): + chat_id = make_chat() + first = _say(db, chat_id, ROLE_USER, "hello") + _say(db, chat_id, ROLE_ASSISTANT, "and a reply") + + response = _tail(client, chat_id, first.id) + + assert response.status_code == 200 + assert "and a reply" in response.text + assert f"msg-{first.id}" not in response.text, "the cursor itself is not resent" + + +def test_no_after_returns_nothing(client, db, registered, connection, make_chat): + """An empty `#thread` sends no cursor, and answering with the transcript + would be the whole conversation appended to a page already showing it.""" + chat_id = make_chat() + _say(db, chat_id, ROLE_USER, "hello") + + response = _tail(client, chat_id) + + assert response.status_code == 204 + assert "msg-" not in response.text + + +def test_an_unknown_after_does_not_dump_the_thread( + client, db, registered, connection, make_chat +): + """A rewind in another tab deletes the row the cursor names. A page whose + history was rewritten underneath it is one only a reload can reconcile, and + that is not this route's decision to make -- there may be a half-typed + message in the box.""" + chat_id = make_chat() + _say(db, chat_id, ROLE_USER, "hello") + _say(db, chat_id, ROLE_ASSISTANT, "a reply") + + response = _tail(client, chat_id, "0" * 32) + + assert response.status_code == 204 + assert "msg-" not in response.text + + +def test_an_after_from_another_chat_returns_nothing( + client, db, registered, connection, make_chat +): + mine = make_chat() + other = make_chat() + elsewhere = _say(db, other, ROLE_USER, "in the other chat") + _say(db, mine, ROLE_USER, "here") + + response = _tail(client, mine, elsewhere.id) + + assert response.status_code == 204 + assert "msg-" not in response.text + + +def test_another_readers_chat_is_not_found(client, db, registered, connection, make_chat): + from lembas.db.models import User + from lembas.security.passwords import hash_password + + stranger = User( + email="stranger@example.com", name="Stranger", password_hash=hash_password("x" * 12) + ) + db.add(stranger) + db.commit() + theirs = Chat(user_id=stranger.id) + db.add(theirs) + db.commit() + last = _say(db, theirs.id, ROLE_USER, "private") + + assert _tail(client, theirs.id, last.id).status_code == 404 + + +# --- What comes back ----------------------------------------------------------- +def test_a_reply_that_started_outside_a_request_reaches_an_open_page( + client, db, registered, connection, make_chat +): + """The case the whole route exists for: `jobs.wake` wrote both rows and + started a generation, and the page has neither. The assistant bubble has to + arrive carrying its own `sse-connect`, because that shell is the only thing + that opens a stream.""" + chat_id = make_chat() + cursor = _say(db, chat_id, ROLE_ASSISTANT, "an earlier reply") + _say(db, chat_id, ROLE_USER, "A background job you started has finished", machine=True) + _say(db, chat_id, ROLE_ASSISTANT, "", complete_=False) + + response = _tail(client, chat_id, cursor.id) + + assert response.status_code == 200 + assert "A background job you started has finished" in response.text + assert "sse-connect" in response.text + assert "Background job" in response.text, "and not under the reader's name" + + +async def test_a_finished_job_reaches_the_page_without_a_reload( + client, db, registered, connection, make_chat, monkeypatch +): + """The complaint this was built for, end to end: `jobs.wake` on an idle chat, + then the poll the open page would have made a moment later. + + Everything between is real -- the rows wake wrote, the cursor the page would + have sent, the bubbles the route renders. Only `generation.ensure` is stubbed, + since there is no upstream to answer. + """ + from lembas.services import generation as generation_service + from lembas.services.agent import jobs + + chat_id = make_chat() + cursor = _say(db, chat_id, ROLE_ASSISTANT, "on it") + monkeypatch.setattr(generation_service, "running_for", lambda _c: None) + monkeypatch.setattr(generation_service, "ensure", lambda c, m: None) + + await jobs.wake(chat_id, "abc123abc123", "pytest -q", "done", 0, "1529 passed") + + response = _tail(client, chat_id, cursor.id) + + assert response.status_code == 200 + assert "1529 passed" in response.text, "the job's output arrived with it" + assert "Background job" in response.text + assert "msg__initial" not in response.text, "and never as something the reader sent" + assert "sse-connect" in response.text, "the reply picks itself up from here" + + +def test_a_queued_turn_is_returned_and_carries_no_streaming_shell( + client, db, registered, connection, make_chat +): + """A completion waiting behind a running reply is exactly what the reader + wants to watch arrive. It is safe to deliver early because the streaming + shell requires the assistant role, so a queued user turn can never carry one + -- and `_queue_frames` re-renders it in place when the reply ends.""" + chat_id = make_chat() + cursor = _say(db, chat_id, ROLE_USER, "do the thing") + _say(db, chat_id, ROLE_USER, "a job finished", queued=True, machine=True) + + response = _tail(client, chat_id, cursor.id) + + assert response.status_code == 200 + assert "a job finished" in response.text + assert "sse-connect" not in response.text + + +def test_the_tail_renders_the_same_partial_the_thread_does(client, db, registered, connection): + """Through `_render_bubble`, so a bubble that arrived late is the same bubble + a reload would have drawn. Four handlers already render this template and a + fifth that did its own thing is a fifth that forgets `template_flags`.""" + from pathlib import Path + + import lembas + + source = (Path(lembas.__file__).parent / "api/chats.py").read_text(encoding="utf-8") + body = source[source.index("async def thread_tail") : source.index("async def post_message")] + assert "_render_bubble" in body + + +# --- Being here counts as reading it ------------------------------------------- +def test_polling_the_page_clears_the_unread_flag(client, db, registered, connection, make_chat): + """`_persist` marks a reply unread whenever nobody is following it, which is + true of a job-woken reply even with the reader watching -- so the toast + announced the chat that was already on screen.""" + chat_id = make_chat() + last = _say(db, chat_id, ROLE_USER, "hello") + chat = db.get(Chat, chat_id) + chat.unread = True + chat.unread_notified = True + db.commit() + + _tail(client, chat_id, last.id) + + db.expire_all() + chat = db.get(Chat, chat_id) + assert chat.unread is False + assert chat.unread_notified is False + + +def test_the_flag_is_cleared_even_when_nothing_arrived( + client, db, registered, connection, make_chat +): + """The claim being made is that somebody is here, not that something came -- + and the empty answer is by far the common one.""" + chat_id = make_chat() + last = _say(db, chat_id, ROLE_USER, "hello") + chat = db.get(Chat, chat_id) + chat.unread = True + db.commit() + + assert _tail(client, chat_id, last.id).status_code == 204 + + db.expire_all() + assert db.get(Chat, chat_id).unread is False + + +# --- Where the poller sits ----------------------------------------------------- +class _Ancestry(HTMLParser): + """The open-tag stack above the element with a given id.""" + + def __init__(self, wanted: str) -> None: + super().__init__() + self.wanted = wanted + self.stack: list[tuple[str, dict[str, str]]] = [] + self.found: tuple[dict[str, str], list[tuple[str, dict[str, str]]]] | None = None + + def handle_starttag(self, tag, attrs): + got = {key: (value or "") for key, value in attrs} + if got.get("id") == self.wanted: + self.found = (got, list(self.stack)) + if tag not in ("br", "img", "input", "hr", "meta", "link", "source", "use", "path"): + self.stack.append((tag, got)) + + def handle_endtag(self, tag): + for index in range(len(self.stack) - 1, -1, -1): + if self.stack[index][0] == tag: + del self.stack[index:] + return + + +def test_the_poller_is_outside_the_thread_and_outside_the_composer_form( + client, db, registered, connection, make_chat +): + """Both failures are silent and both have happened here before. + + Inside `#thread` it would be swapped away by the first rewind or compaction, + which replace that container's contents, and never fire again. Inside the + composer's form it would inherit `hx-target="#thread"` from an ancestor -- + the jobs chip did exactly that and blanked the transcript on every tick. + """ + chat_id = make_chat() + _say(db, chat_id, ROLE_USER, "hello") + + page = client.get(f"/chat/{chat_id}").text + parser = _Ancestry("thread-tail") + parser.feed(page) + + assert parser.found is not None, "the chat page has no tail poller" + attrs, ancestors = parser.found + + ids = {got.get("id") for _tag, got in ancestors} + assert "thread" not in ids, "a rewind would swap the poller away" + assert not any(tag == "form" for tag, _got in ancestors), "it would inherit hx-target" + + assert attrs.get("hx-target") == "#thread" + assert attrs.get("hx-swap") == "beforeend" + assert attrs.get("hx-sync"), "two overlapping polls are two copies of one bubble" + + +def test_the_new_chat_screen_has_no_poller(client, db, registered, connection): + """There is no row to poll: a chat is written together with its first + message.""" + assert 'id="thread-tail"' not in client.get("/chat").text + + +def test_the_cursor_is_not_read_with_last_of_type(): + """`:last-of-type` is per-parent and `querySelector` returns the first match + in document order, so on a compacted chat it answers with the last article + inside `
` rather than the newest message -- and + the poll then asks about a message from the middle of the conversation and + re-appends everything after it. + + Matched on the selector *as written in a selector string*, not on the words: + the comment beside the code names `:last-of-type` in order to say why it is + not being used, exactly as `data-prompt`'s comment names `hx-prompt`. + """ + from pathlib import Path + + import lembas + + source = (Path(lembas.__file__).parent / "web/static/js/app.js").read_text(encoding="utf-8") + assert ':last-of-type"' not in source + assert ":last-of-type'" not in source + assert 'querySelectorAll("article.msg")' in source + + +def test_the_poller_is_quiet_while_a_reply_is_streaming(): + """That reply delivers its own bubbles through the `done` frame, which is the + only channel that gets the order right -- and it is the one window in which + the transcript's order moves underneath us, since `_inject` restamps the + placeholder.""" + from pathlib import Path + + import lembas + + source = (Path(lembas.__file__).parent / "web/static/js/app.js").read_text(encoding="utf-8") + start = source.index("htmx:configRequest") + block = source[start : source.index("htmx:beforeSwap", start)] + assert "thread-tail" in block + assert "sse-connect" in block + assert "preventDefault" in block + assert "parameters.after" in block + + +def test_a_bubble_the_page_already_has_is_never_swapped_in(): + """The race `hx-sync` cannot reach: the composer's POST committing between a + tail request going out and its answer coming back. A duplicate here is not + cosmetic -- it would carry a second `sse-connect` for one message.""" + from pathlib import Path + + import lembas + + source = (Path(lembas.__file__).parent / "web/static/js/app.js").read_text(encoding="utf-8") + block = source[source.index("htmx:beforeSwap") :] + assert "shouldSwap = false" in block + assert "getElementById" in block + + +def test_no_template_hands_htmx_a_string_to_evaluate(): + """Every `hx-vals` in this project is static JSON rendered server-side. A + `js:` one would be the only string htmx ever evaluated here, and it would die + silently under any CSP a deployment added later -- the same family as the + `hx-prompt` rule.""" + from pathlib import Path + + import lembas + + templates = Path(lembas.__file__).parent / "web/templates" + offenders = [ + path.relative_to(templates) + for path in templates.rglob("*.html") + if 'hx-vals="js:' in path.read_text(encoding="utf-8") + or "hx-vals='js:" in path.read_text(encoding="utf-8") + ] + assert not offenders, f"hx-vals with js: cannot cancel a request: {offenders}" + + +# --- Ordering ------------------------------------------------------------------ +def test_rows_sharing_a_timestamp_do_not_stall_the_poll( + client, db, registered, connection, make_chat +): + """Under a bare `created_at >` a row sharing the cursor's microsecond is + skipped forever -- returned never, passed never. The id clause is what makes + the poll make progress instead of stopping on a row it can neither hand back + nor step over.""" + chat_id = make_chat() + cursor = _say(db, chat_id, ROLE_USER, "first") + twin = _say(db, chat_id, ROLE_ASSISTANT, "same instant") + twin.created_at = cursor.created_at + db.commit() + + ordered = sorted([cursor.id, twin.id]) + response = _tail(client, chat_id, ordered[0]) + + assert response.status_code == 200 + assert f"msg-{ordered[1]}" in response.text + + +def test_the_turns_come_back_in_the_order_they_happened( + client, db, registered, connection, make_chat +): + chat_id = make_chat() + cursor = _say(db, chat_id, ROLE_USER, "first") + _say(db, chat_id, ROLE_ASSISTANT, "second") + _say(db, chat_id, ROLE_USER, "third") + + body = _tail(client, chat_id, cursor.id).text + + assert body.index("second") < body.index("third")