A form's handler answers its own request, and a finished reply is finished
Two regressions, one of them much older than it looked. htmx events bubble, and the composer's form declares `hx-on::after-request` so it can clear itself after sending. Six things inside that form 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 afterRequest events was reaching that 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. That has been true for as long as those controls have existed. The jobs chip did not introduce it; it polls, so it made it happen every five seconds, and that is the only reason it was ever noticed. `event.target === this` is the whole fix, and it is what the attribute always meant. Moving the chip out of the form would have left the other five. The second: `steps.for_message` marked its trailing prose step as still being written, so every finished reply ending in prose carried `msg__body--live` and blinked a caret at the reader for ever. One flag was doing two jobs -- emit the tail, and mark it live -- and a stored reply wants the first without the second. They are separate arguments now. Note what the existing test for that did: it asserted the caret was on the *right* step, through `for_message`, and passed. It never asked whether a finished reply should have one at all. It is driven through the live path now, and the stored path has its own assertion. The composer handler is driven under a DOM stub -- extract the body from the template, fire the event from a descendant and from the form -- because a source assertion can only say the guard is present, not what it does. Checked against the bug before being kept: without the guard the stub reports the text wiped and the thread scrolled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -63,6 +63,11 @@ def for_message(message: Any) -> list[Step]:
|
|||||||
thinking=(message.reasoning or "") if not message.error else "",
|
thinking=(message.reasoning or "") if not message.error else "",
|
||||||
events=list(message.tool_calls_json or []),
|
events=list(message.tool_calls_json or []),
|
||||||
marks=list(getattr(message, "steps_json", None) 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],
|
marks: list[dict],
|
||||||
since: int = 0,
|
since: int = 0,
|
||||||
include_open: bool = True,
|
include_open: bool = True,
|
||||||
|
live: bool = True,
|
||||||
) -> list[Step]:
|
) -> list[Step]:
|
||||||
"""The shared walk.
|
"""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
|
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,
|
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
|
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)))
|
steps.append(Step(index=0, kind=KIND_TOOLS, events=tuple(events)))
|
||||||
if text:
|
if text:
|
||||||
steps.append(
|
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
|
return steps
|
||||||
|
|
||||||
@@ -180,7 +194,9 @@ def _build(
|
|||||||
steps.append(Step(index=index, kind=KIND_THINKING, text=trailing_thought))
|
steps.append(Step(index=index, kind=KIND_THINKING, text=trailing_thought))
|
||||||
if trailing_text := text[text_from:]:
|
if trailing_text := text[text_from:]:
|
||||||
source = f"{carry}\n{trailing_text}" if carry else trailing_text
|
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:]:
|
if trailing_events := events[tools_from:]:
|
||||||
steps.append(Step(index=index, kind=KIND_TOOLS, events=tuple(trailing_events)))
|
steps.append(Step(index=index, kind=KIND_TOOLS, events=tuple(trailing_events)))
|
||||||
|
|
||||||
|
|||||||
@@ -48,7 +48,22 @@
|
|||||||
{% if chat %}
|
{% if chat %}
|
||||||
hx-post="/api/chats/{{ chat.id }}/messages"
|
hx-post="/api/chats/{{ chat.id }}/messages"
|
||||||
hx-target="#thread" hx-swap="beforeend"
|
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();
|
this.reset();
|
||||||
document.getElementById('attachments').replaceChildren();
|
document.getElementById('attachments').replaceChildren();
|
||||||
window.lembas.autosize(this.querySelector('textarea'));
|
window.lembas.autosize(this.querySelector('textarea'));
|
||||||
|
|||||||
@@ -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 "
|
"this fetches from inside a form targeting #thread and does not say "
|
||||||
f"where its answer goes:\n{markup}"
|
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"
|
||||||
|
)
|
||||||
|
|||||||
+56
-10
@@ -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():
|
def test_only_the_trailing_prose_is_marked_live():
|
||||||
"""`--live` draws the caret, and a caret after every paragraph that happened
|
"""`--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."""
|
to precede a tool call is not where the reply is being written.
|
||||||
built = steps.for_message(
|
|
||||||
_message(
|
Driven through the *live* path deliberately. This used to go through
|
||||||
content="before after",
|
`for_message` and pass, which is precisely how a stored reply came to blink
|
||||||
events=[{"name": "a"}],
|
a cursor for ever: the test asserted the caret was in the right place
|
||||||
marks=[{"round": 0, "thinking_to": 0, "text_to": 6, "tools_to": 1}],
|
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():
|
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)
|
first, last = (s for s in built if s.kind == "text")
|
||||||
last = next(s for s in built if s.kind == "text" and s.open)
|
|
||||||
# `code-block`, not the literal source: the fence renderer highlights, so
|
# `code-block`, not the literal source: the fence renderer highlights, so
|
||||||
# `x = 1` comes back as a run of spans.
|
# `x = 1` comes back as a run of spans.
|
||||||
assert "code-block" in first.html
|
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"])
|
generation = _generation(content=["all of it"], reasoning=["thinking"])
|
||||||
|
|
||||||
assert steps.tail(generation) == ("thinking", "all of it")
|
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),
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user