diff --git a/src/lembas/services/steps.py b/src/lembas/services/steps.py index 26a43b1..6a53054 100644 --- a/src/lembas/services/steps.py +++ b/src/lembas/services/steps.py @@ -63,6 +63,11 @@ def for_message(message: Any) -> list[Step]: thinking=(message.reasoning or "") if not message.error else "", events=list(message.tool_calls_json or []), marks=list(getattr(message, "steps_json", None) or []), + # A stored reply has a trailing step and it is not being written. Those + # are two facts and they used to be one flag: `include_open` both + # emitted the tail and marked it live, so every finished bubble ending + # in prose carried `msg__body--live` and blinked a caret for ever. + live=False, ) @@ -112,9 +117,18 @@ def _build( marks: list[dict], since: int = 0, include_open: bool = True, + live: bool = True, ) -> list[Step]: """The shared walk. + Two flags, because they are two questions and conflating them put a blinking + caret on every finished reply: + + * `include_open` -- emit the trailing step at all. `closed_from` says no, + because the step still being written is carried by its own frames. + * `live` -- mark that trailing step as still being written. Only ever true + of a running generation. A stored reply has a tail and it is finished. + 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 @@ -132,7 +146,7 @@ def _build( 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)) + Step(index=0, kind=KIND_TEXT, open=live, html=render_markdown(text)) ) return steps @@ -180,7 +194,9 @@ def _build( 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))) + steps.append( + Step(index=index, kind=KIND_TEXT, open=live, html=render_markdown(source)) + ) if trailing_events := events[tools_from:]: steps.append(Step(index=index, kind=KIND_TOOLS, events=tuple(trailing_events))) diff --git a/src/lembas/web/templates/chat/_composer.html b/src/lembas/web/templates/chat/_composer.html index 71490b9..ffc1dc5 100644 --- a/src/lembas/web/templates/chat/_composer.html +++ b/src/lembas/web/templates/chat/_composer.html @@ -48,7 +48,22 @@ {% if chat %} hx-post="/api/chats/{{ chat.id }}/messages" hx-target="#thread" hx-swap="beforeend" - hx-on::after-request="if (event.detail.successful) { + {# `event.target === this` is load-bearing, and its absence quietly + threw away typed messages for releases. + + htmx events BUBBLE. This form contains six things that make + requests -- the two scope switches, "ask me about these again", + the agent mode select, the effort select and the jobs chip -- and + every one of their `htmx:afterRequest` events reached this handler. + So changing the mode, or the effort, or toggling a tool, called + `this.reset()` on a composer somebody was typing in and dragged the + view to the bottom. The jobs chip did not introduce that; it polls, + so it made it happen every five seconds, which is what finally made + it visible. + + A form's own handler answers its own request. Anything else in here + that fetches is somebody else's business. There is a test. #} + hx-on::after-request="if (event.target === this && event.detail.successful) { this.reset(); document.getElementById('attachments').replaceChildren(); window.lembas.autosize(this.querySelector('textarea')); diff --git a/tests/test_chat.py b/tests/test_chat.py index 66dd1f3..cf536a0 100644 --- a/tests/test_chat.py +++ b/tests/test_chat.py @@ -1016,3 +1016,31 @@ def test_nothing_inside_the_composer_form_fetches_without_saying_where_it_lands( "this fetches from inside a form targeting #thread and does not say " f"where its answer goes:\n{markup}" ) + + +def test_the_composer_form_answers_only_its_own_request(): + """The other half of the bug that blanked agent chats, and the one that had + been quietly costing typed messages for far longer. + + htmx events bubble. This form contains six things that fetch -- two scope + switches, "ask me about these again", the agent mode select, the effort + select and the jobs chip -- and every one of their `htmx:afterRequest` + events reaches the form's own `hx-on::after-request`. Without the guard, + changing the mode or the effort called `this.reset()` on a composer somebody + was typing in, and dragged the view to the bottom. The chip polls, so it did + it every five seconds; that is the only reason it was ever noticed. + """ + import re + from pathlib import Path + + import lembas + + source = ( + Path(lembas.__file__).parent / "web/templates/chat/_composer.html" + ).read_text() + + handler = re.search(r'hx-on::after-request="([^"]*)"', source) + assert handler, "the composer no longer clears itself after sending" + assert "event.target === this" in handler.group(1), ( + "a descendant's request will run this handler without the guard" + ) diff --git a/tests/test_steps.py b/tests/test_steps.py index 1074c6e..19c2d9d 100644 --- a/tests/test_steps.py +++ b/tests/test_steps.py @@ -103,16 +103,20 @@ def test_thinking_is_sliced_per_round_and_the_column_stays_whole(): def test_only_the_trailing_prose_is_marked_live(): """`--live` draws the caret, and a caret after every paragraph that happened - to precede a tool call is not where the reply is being written.""" - built = steps.for_message( - _message( - content="before after", - events=[{"name": "a"}], - marks=[{"round": 0, "thinking_to": 0, "text_to": 6, "tools_to": 1}], - ) + to precede a tool call is not where the reply is being written. + + Driven through the *live* path deliberately. This used to go through + `for_message` and pass, which is precisely how a stored reply came to blink + a cursor for ever: the test asserted the caret was in the right place + without ever asking whether it should be there at all. + """ + generation = _generation( + content=["before after"], + tool_events=[{"name": "a"}], + steps=[{"round": 0, "thinking_to": 0, "text_to": 6, "tools_to": 1}], ) - assert [s.open for s in built if s.kind == "text"] == [False, True] + assert [s.open for s in _live_steps(generation) if s.kind == "text"] == [False, True] def test_a_step_with_nothing_in_it_produces_nothing(): @@ -189,8 +193,7 @@ def test_a_fence_left_open_is_closed_and_reopened_around_the_tool_call(): ) ) - first = next(s for s in built if s.kind == "text" and not s.open) - last = next(s for s in built if s.kind == "text" and s.open) + first, last = (s for s in built if s.kind == "text") # `code-block`, not the literal source: the fence renderer highlights, so # `x = 1` comes back as a run of spans. assert "code-block" in first.html @@ -300,3 +303,46 @@ def test_a_reply_with_no_marks_has_everything_in_its_tail(): generation = _generation(content=["all of it"], reasoning=["thinking"]) assert steps.tail(generation) == ("thinking", "all of it") + + +# --- Finished means finished --------------------------------------------------- +def test_a_stored_reply_marks_nothing_as_still_being_written(): + """`msg__body--live` draws the blinking caret. `include_open` used to do two + jobs -- emit the trailing step, and mark it live -- so every finished reply + ending in prose blinked a cursor at the reader for ever.""" + built = steps.for_message( + _message( + content="before after", + events=[{"name": "a"}], + marks=[{"round": 0, "thinking_to": 0, "text_to": 6, "tools_to": 1}], + ) + ) + + assert [s.open for s in built] == [False] * len(built) + + +def test_a_stored_reply_with_no_marks_marks_nothing_either(): + """The compatibility branch had its own copy of the same flag, so every row + written before the marks existed blinked too -- which is most of them.""" + built = steps.for_message(_message(content="An older answer.")) + + assert [s.open for s in built] == [False] + + +def test_a_running_reply_still_marks_its_tail(): + """The other half: the caret has to be somewhere while the reply is being + written, or there is no sign it is still going.""" + generation = _generation(content=["still going"]) + + assert [s.open for s in _live_steps(generation)] == [True] + + +def _live_steps(generation): + from lembas.services.steps import _build + + return _build( + text=generation.text, + thinking=generation.thinking, + events=list(generation.tool_events), + marks=list(generation.steps), + )