Phase 3.5 cleanup: 17-item backlog burndown #5
@@ -381,7 +381,10 @@ async def regenerate_assistant_turn(
|
||||
speaker_bot.get("name", "bot") if speaker_bot is not None else "bot"
|
||||
)
|
||||
new_turn_html = render_turn_html(
|
||||
speaker_name_for_render, new_text, role="bot"
|
||||
speaker_name_for_render,
|
||||
new_text,
|
||||
role="bot",
|
||||
event_id=new_assistant_event_id,
|
||||
)
|
||||
await publish(
|
||||
chat_id,
|
||||
@@ -616,7 +619,10 @@ async def regenerate_assistant_turn(
|
||||
# Broadcast a replace event so connected tabs swap the prior
|
||||
# interjection node in-place (mirrors T73.1's primary swap).
|
||||
interject_html = render_turn_html(
|
||||
silent_witness.get("name", "bot"), interject_text, role="bot"
|
||||
silent_witness.get("name", "bot"),
|
||||
interject_text,
|
||||
role="bot",
|
||||
event_id=new_interjection_event_id,
|
||||
)
|
||||
await publish(
|
||||
chat_id,
|
||||
|
||||
@@ -74,17 +74,24 @@ def read_recent_dialogue(
|
||||
)
|
||||
rows = list(reversed(cur.fetchall()))
|
||||
out: list[dict] = []
|
||||
for _row_id, kind, payload_json in rows:
|
||||
for row_id, kind, payload_json in rows:
|
||||
p = json.loads(payload_json)
|
||||
if p.get("chat_id") != chat_id:
|
||||
continue
|
||||
if kind in ("user_turn", "user_turn_edit"):
|
||||
out.append({"speaker": "you", "text": p.get("prose", "")})
|
||||
out.append(
|
||||
{
|
||||
"speaker": "you",
|
||||
"text": p.get("prose", ""),
|
||||
"event_id": row_id,
|
||||
}
|
||||
)
|
||||
else:
|
||||
out.append(
|
||||
{
|
||||
"speaker": p.get("speaker_id", "bot"),
|
||||
"text": p.get("text", ""),
|
||||
"event_id": row_id,
|
||||
}
|
||||
)
|
||||
return out
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
<p class="muted">No turns yet. Start typing below.</p>
|
||||
{% else %}
|
||||
{% for turn in turns %}
|
||||
<div class="turn turn-{{ turn.role }}">
|
||||
<div{% if turn.event_id is not none %} id="turn-{{ turn.event_id }}"{% endif %} class="turn turn-{{ turn.role }}">
|
||||
<strong>{{ turn.speaker }}</strong>
|
||||
{{ turn.text|render_prose|safe }}
|
||||
</div>
|
||||
@@ -119,6 +119,39 @@ document.querySelector('.drawer-toggle')?.addEventListener('click', (e) => {
|
||||
}
|
||||
});
|
||||
|
||||
// T86: live-swap regenerated turns. The backend (chat/services/
|
||||
// regenerate.py) broadcasts a ``turn_html_replace`` SSE frame after
|
||||
// appending the new assistant_turn — JSON payload of shape
|
||||
// ``{data: <html>, turn_id: <new_id>, supersedes_id: <old_id>}``.
|
||||
// We replace the prior turn's DOM node in-place when we can locate
|
||||
// it by id, otherwise fall back to appending so a tab opened mid-
|
||||
// regenerate still shows the new turn. The renderer
|
||||
// (chat/web/render.py::render_turn_html) and the Jinja loop above
|
||||
// both stamp ``id="turn-<event_id>"`` on each turn DIV, so the
|
||||
// primary in-place swap path is the live one — the append fallback
|
||||
// only kicks in when a tab opened AFTER the regenerate started (no
|
||||
// prior turn DOM node to replace).
|
||||
shell.addEventListener('htmx:sseMessage', (e) => {
|
||||
if (e.detail.type !== 'turn_html_replace') return;
|
||||
let data;
|
||||
try { data = JSON.parse(e.detail.data); } catch (_) { return; }
|
||||
const html = (data && data.data) || '';
|
||||
const trimmed = html.trim();
|
||||
if (!trimmed) return;
|
||||
const oldNode = document.getElementById('turn-' + data.supersedes_id);
|
||||
if (oldNode) {
|
||||
const tmpl = document.createElement('template');
|
||||
tmpl.innerHTML = trimmed;
|
||||
const newNode = tmpl.content.firstChild;
|
||||
if (newNode) oldNode.replaceWith(newNode);
|
||||
} else {
|
||||
// Fallback: append if the prior turn isn't in the DOM (e.g. user
|
||||
// opened the tab AFTER the regenerate started, or the renderer
|
||||
// hasn't yet stamped per-turn ids — see comment above).
|
||||
timeline.insertAdjacentHTML('beforeend', trimmed);
|
||||
}
|
||||
});
|
||||
|
||||
// SSE connection lost — show a banner and unlock so the user can
|
||||
// retry. The server commits the partial as truncated when its
|
||||
// request.is_disconnected() poll trips (T19).
|
||||
|
||||
+20
-2
@@ -52,12 +52,30 @@ async def chat_detail(chat_id: str, request: Request, conn=Depends(get_conn)):
|
||||
raw_turns = _read_recent_dialogue(conn, chat_id, limit=200)
|
||||
turns: list[dict] = []
|
||||
for t in raw_turns:
|
||||
# event_id is forwarded so the Jinja loop can stamp
|
||||
# ``id="turn-<event_id>"`` on each rendered turn — the
|
||||
# ``turn_html_replace`` SSE handler in chat.html relies on this
|
||||
# id to swap a regenerated turn in-place (T86 follow-up).
|
||||
if t["speaker"] == "you":
|
||||
turns.append({"role": "you", "speaker": "you", "text": t["text"]})
|
||||
turns.append(
|
||||
{
|
||||
"role": "you",
|
||||
"speaker": "you",
|
||||
"text": t["text"],
|
||||
"event_id": t.get("event_id"),
|
||||
}
|
||||
)
|
||||
else:
|
||||
bot = get_bot(conn, t["speaker"])
|
||||
label = bot["name"] if bot else t["speaker"]
|
||||
turns.append({"role": "bot", "speaker": label, "text": t["text"]})
|
||||
turns.append(
|
||||
{
|
||||
"role": "bot",
|
||||
"speaker": label,
|
||||
"text": t["text"],
|
||||
"event_id": t.get("event_id"),
|
||||
}
|
||||
)
|
||||
|
||||
return TEMPLATES.TemplateResponse(
|
||||
request,
|
||||
|
||||
@@ -378,7 +378,12 @@ async def process_meanwhile_turn(
|
||||
"truncated": truncated,
|
||||
},
|
||||
)
|
||||
turn_html = _render_turn_html(speaker_bot["name"], text, role="bot")
|
||||
turn_html = _render_turn_html(
|
||||
speaker_bot["name"],
|
||||
text,
|
||||
role="bot",
|
||||
event_id=assistant_event_id,
|
||||
)
|
||||
await publish(chat_id, {"event": "turn_html", "data": turn_html})
|
||||
|
||||
if cancelled:
|
||||
|
||||
+15
-2
@@ -84,7 +84,13 @@ def render_prose(text: str) -> str:
|
||||
return "".join(f"<p>{p}</p>" for p in paragraphs)
|
||||
|
||||
|
||||
def render_turn_html(speaker: str, text: str, role: str = "bot") -> str:
|
||||
def render_turn_html(
|
||||
speaker: str,
|
||||
text: str,
|
||||
role: str = "bot",
|
||||
*,
|
||||
event_id: int | None = None,
|
||||
) -> str:
|
||||
"""Render a full transcript turn as ``<div class="turn …">…</div>``.
|
||||
|
||||
Used by both the SSE fragment publisher in :mod:`chat.web.turns`
|
||||
@@ -94,12 +100,19 @@ def render_turn_html(speaker: str, text: str, role: str = "bot") -> str:
|
||||
``role`` selects the CSS class (``turn-you`` vs ``turn-bot``); the
|
||||
speaker label and role name are HTML-escaped defensively even though
|
||||
they currently come from trusted server-side state.
|
||||
|
||||
``event_id`` (T86 follow-up) stamps ``id="turn-<event_id>"`` on the
|
||||
wrapper div so the chat-page ``turn_html_replace`` SSE handler can
|
||||
locate the prior turn node by id and swap it in-place. When omitted
|
||||
the id attribute is dropped so SSE-only fragments without a stable
|
||||
event id (legacy callers) still render cleanly.
|
||||
"""
|
||||
speaker_html = html.escape(speaker)
|
||||
role_html = html.escape(role)
|
||||
body_html = render_prose(text)
|
||||
id_attr = f' id="turn-{int(event_id)}"' if event_id is not None else ""
|
||||
return (
|
||||
f'<div class="turn turn-{role_html}">'
|
||||
f'<div{id_attr} class="turn turn-{role_html}">'
|
||||
f"<strong>{speaker_html}</strong>"
|
||||
f"{body_html}"
|
||||
f"</div>"
|
||||
|
||||
+17
-4
@@ -483,7 +483,11 @@ async def post_turn(
|
||||
|
||||
# 7. Append the assistant_turn with the final text. (See note above on
|
||||
# why we skip ``project`` for these transcript-only event kinds.)
|
||||
append_event(
|
||||
# Capture the returned event id so we can stamp ``id="turn-<n>"`` on
|
||||
# the SSE-emitted HTML fragment — the chat-page ``turn_html_replace``
|
||||
# handler relies on the id to swap regenerated turns in-place
|
||||
# (T86 follow-up).
|
||||
primary_assistant_event_id = append_event(
|
||||
conn,
|
||||
kind="assistant_turn",
|
||||
payload={
|
||||
@@ -583,6 +587,7 @@ async def post_turn(
|
||||
interjection_text: str | None = None
|
||||
interjection_speaker_id: str | None = None
|
||||
interjection_truncated = False
|
||||
interjection_event_id: int | None = None
|
||||
if (
|
||||
guest_bot is not None
|
||||
and not cancelled
|
||||
@@ -670,7 +675,9 @@ async def post_turn(
|
||||
|
||||
interjection_text = "".join(interject_accumulated)
|
||||
|
||||
append_event(
|
||||
# Capture the event id (T86 follow-up) so the SSE fragment
|
||||
# below carries ``id="turn-<n>"`` for in-place swap.
|
||||
interjection_event_id = append_event(
|
||||
conn,
|
||||
kind="assistant_turn",
|
||||
payload={
|
||||
@@ -925,7 +932,10 @@ async def post_turn(
|
||||
},
|
||||
)
|
||||
primary_html = _render_turn_html(
|
||||
addressee_bot["name"], primary_text, role="bot"
|
||||
addressee_bot["name"],
|
||||
primary_text,
|
||||
role="bot",
|
||||
event_id=primary_assistant_event_id,
|
||||
)
|
||||
await publish(
|
||||
chat_id, {"event": "turn_html", "data": primary_html}
|
||||
@@ -949,7 +959,10 @@ async def post_turn(
|
||||
},
|
||||
)
|
||||
interject_html = _render_turn_html(
|
||||
interject_speaker_name, interjection_text, role="bot"
|
||||
interject_speaker_name,
|
||||
interjection_text,
|
||||
role="bot",
|
||||
event_id=interjection_event_id,
|
||||
)
|
||||
await publish(
|
||||
chat_id, {"event": "turn_html", "data": interject_html}
|
||||
|
||||
@@ -85,3 +85,26 @@ def test_render_prose_mixed_full_message():
|
||||
assert '<em class="action">looks up</em>' in out
|
||||
# The apostrophe in ``she's`` is HTML-escaped to ``'``.
|
||||
assert '<span class="ooc">((she's tired))</span>' in out
|
||||
|
||||
|
||||
def test_render_turn_html_stamps_event_id_when_provided():
|
||||
"""T86 follow-up: when ``event_id`` is supplied the wrapper DIV
|
||||
carries ``id="turn-<event_id>"`` so the chat-page
|
||||
``turn_html_replace`` SSE handler can locate the prior turn DOM
|
||||
node by id and swap it in-place. Without the id the handler's
|
||||
``getElementById('turn-' + supersedes_id)`` lookup misses and
|
||||
the regenerated turn appends instead of replaces.
|
||||
"""
|
||||
out = render_turn_html("BotA", "Hello.", role="bot", event_id=42)
|
||||
assert 'id="turn-42"' in out
|
||||
# The id must sit on the wrapper DIV, not somewhere nested inside.
|
||||
assert out.startswith('<div id="turn-42" class="turn turn-bot">')
|
||||
|
||||
|
||||
def test_render_turn_html_omits_id_when_event_id_missing():
|
||||
"""Legacy callers (no ``event_id`` passed) get a clean DIV with no
|
||||
id attribute — preserves the pre-T86 fragment shape.
|
||||
"""
|
||||
out = render_turn_html("BotA", "Hello.", role="bot")
|
||||
assert "id=" not in out
|
||||
assert out.startswith('<div class="turn turn-bot">')
|
||||
|
||||
@@ -174,3 +174,74 @@ def test_chat_html_includes_stop_streaming_script(client, tmp_path):
|
||||
assert "stop-streaming" in body or "isStreaming" in body
|
||||
# Cancel route reference must be wired so the Stop button can call it.
|
||||
assert "/turns/cancel" in body
|
||||
|
||||
|
||||
def test_chat_html_has_turn_html_replace_listener(client, tmp_path):
|
||||
"""T86: the chat shell wires a JS handler for the ``turn_html_replace``
|
||||
SSE event so regenerate-driven swaps land in connected tabs without a
|
||||
page refresh.
|
||||
|
||||
This is a presence / string-check test: it verifies the handler is
|
||||
embedded in the rendered template but does NOT drive a real browser
|
||||
(no headless runner is wired into this test environment). The end-to-
|
||||
end behaviour — receiving the event over SSE and replacing the prior
|
||||
turn's DOM node — is therefore not exercised here; a manual smoke
|
||||
check or future browser-driven test would close that gap.
|
||||
"""
|
||||
_seed_chat(tmp_path / "test.db")
|
||||
response = client.get("/chats/chat_bot_a")
|
||||
assert response.status_code == 200
|
||||
body = response.text
|
||||
# The handler must be wired against the SSE event name the backend
|
||||
# publishes (chat.services.regenerate -> "turn_html_replace").
|
||||
assert "turn_html_replace" in body
|
||||
# Confirm the handler reads the JSON payload's ``supersedes_id`` so
|
||||
# it can locate the prior turn node. The exact lookup mechanism may
|
||||
# vary, but the field name is part of the contract with the backend.
|
||||
assert "supersedes_id" in body
|
||||
|
||||
|
||||
def test_rendered_turn_html_includes_event_id(client, tmp_path):
|
||||
"""T86 follow-up: the chat-detail Jinja loop stamps
|
||||
``id="turn-<event_id>"`` on every rendered turn DIV. Without this id
|
||||
the ``turn_html_replace`` SSE handler's ``getElementById`` lookup
|
||||
misses, falls through to ``insertAdjacentHTML('beforeend', …)``, and
|
||||
the regenerated turn appears APPENDED instead of swapped in-place
|
||||
(rendering the primary handler path dead code — exactly the gap the
|
||||
T86 reviewer flagged).
|
||||
|
||||
Seed a user_turn + assistant_turn, GET the chat page, and assert the
|
||||
response body carries both turns' event ids on the wrapper DIVs.
|
||||
"""
|
||||
db_path = tmp_path / "test.db"
|
||||
_seed_chat(db_path)
|
||||
with open_db(db_path) as conn:
|
||||
ut_id = append_event(
|
||||
conn,
|
||||
kind="user_turn",
|
||||
payload={
|
||||
"chat_id": "chat_bot_a",
|
||||
"prose": "hello bot",
|
||||
"segments": [],
|
||||
},
|
||||
)
|
||||
at_id = append_event(
|
||||
conn,
|
||||
kind="assistant_turn",
|
||||
payload={
|
||||
"chat_id": "chat_bot_a",
|
||||
"speaker_id": "bot_a",
|
||||
"text": "Hi there.",
|
||||
"truncated": False,
|
||||
"user_turn_id": ut_id,
|
||||
},
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
response = client.get("/chats/chat_bot_a")
|
||||
assert response.status_code == 200
|
||||
body = response.text
|
||||
# Both seeded turns must carry ``id="turn-<event_id>"`` so the SSE
|
||||
# in-place swap can find them.
|
||||
assert f'id="turn-{ut_id}"' in body
|
||||
assert f'id="turn-{at_id}"' in body
|
||||
|
||||
@@ -104,10 +104,16 @@ def test_read_recent_dialogue_returns_chronological_pairs(tmp_path):
|
||||
with open_db(db) as conn:
|
||||
out = read_recent_dialogue(conn, "chat_a", limit=10)
|
||||
|
||||
assert out == [
|
||||
{"speaker": "you", "text": "hello"},
|
||||
{"speaker": "bot_a", "text": "Original."},
|
||||
# Each entry now carries the source ``event_log.id`` as ``event_id``
|
||||
# (T86 follow-up) so the chat-detail Jinja loop can stamp
|
||||
# ``id="turn-<n>"`` on each rendered turn DIV — needed by the
|
||||
# ``turn_html_replace`` SSE handler for in-place regenerate swaps.
|
||||
speakers = [(e["speaker"], e["text"]) for e in out]
|
||||
assert speakers == [
|
||||
("you", "hello"),
|
||||
("bot_a", "Original."),
|
||||
]
|
||||
assert all("event_id" in e and isinstance(e["event_id"], int) for e in out)
|
||||
|
||||
|
||||
def test_read_recent_dialogue_filters_superseded_and_other_chats(tmp_path):
|
||||
|
||||
Reference in New Issue
Block a user