Compare commits
8 Commits
82701d3c18
...
phase-3.5
| Author | SHA1 | Date | |
|---|---|---|---|
| 74bb42397d | |||
| 3be8ed8915 | |||
| 097073ede5 | |||
| 4a2617565b | |||
| 73625c0ac4 | |||
| aea20a2c83 | |||
| 9493d24a53 | |||
| da7aa88b8e |
@@ -204,15 +204,7 @@ Phase 2.5 cleanup shipped end-to-end across 8 tasks (T68–T75). Two CLAUDE.md b
|
|||||||
|
|
||||||
### Phase 2.6 / 3 backlog
|
### Phase 2.6 / 3 backlog
|
||||||
|
|
||||||
New follow-ups discovered during Phase 2.5 execution. None are blocking; pick up at any time.
|
All items shipped — see Phase 3.5 status below.
|
||||||
|
|
||||||
- **Frontend handler for `turn_html_replace` SSE event (from T73.1 review)**: regenerate's backend broadcast lands, but no live tab swaps the regenerated turn until a JS handler is wired. The existing `turn_html` event uses HTMX `sse-swap` to append; `turn_html_replace` ships JSON with `supersedes_id` for replacement semantics. Phase 2.6 should wire the JS to swap the prior turn's DOM node in place.
|
|
||||||
- **Cancel/stop hook for in-flight regenerate streams (from T73 review)**: `post_turn` registers stream tasks in `_in_flight_tasks` so the user can stop them. Regenerate doesn't. A user clicking "Stop" mid-regenerate has no cancel hook today.
|
|
||||||
- **DRY: regenerate vs post_turn (from T73 review)**: recent-dialogue assembly and prior-edges block are duplicated between `chat/services/regenerate.py` and `chat/web/turns.py`. Extract to shared helpers analogous to `_gather_state_update_inputs`.
|
|
||||||
- **Sibling-discovery query optimization (from T73 review)**: `regenerate.py`'s sibling-assistant-turn lookup scans all non-superseded `assistant_turn` rows globally. Adding a `chat_id` predicate via JSON extraction (or a denormalized column) bounds the cost to per-chat scale.
|
|
||||||
- **`_witness_role_for` defensive coding (from T71 review)**: helper returns `"guest"` when `host_bot_id is None`, which is wrong for Phase-1 chats. Defensive: `return "host" if host_bot_id is None or speaker_bot_id == host_bot_id else "guest"`. Not exercised by current tests; harden as a precaution.
|
|
||||||
- **Confidence type tightening (from T74 review)**: `chat/services/addressee.py::AddresseeDecision.confidence` could be typed as `Literal["high","medium","low"]` for stricter validation. Currently `str` with a comment.
|
|
||||||
- **Scene-close-on-cancel UX revisit**: T74.3 pinned the existing behavior (close fires even on cancel). If real play-testing surfaces a regression, revisit.
|
|
||||||
|
|
||||||
## Phase 3 status
|
## Phase 3 status
|
||||||
|
|
||||||
@@ -249,51 +241,49 @@ Phase 3 shipped end-to-end across 19 tasks (T49–T67). Events with full lifecyc
|
|||||||
|
|
||||||
### Phase 3.5 / 4 backlog
|
### Phase 3.5 / 4 backlog
|
||||||
|
|
||||||
New follow-ups discovered during Phase 3 reviews and execution. None are blocking; pick up at any time.
|
All items shipped — see Phase 3.5 status below.
|
||||||
|
|
||||||
#### From T53 review
|
## Phase 3.5 status
|
||||||
|
|
||||||
- **`narrate_skip` `timeout_s` not piped through to `client.generate`**: parameter accepted but ignored. Fix: pass `timeout_s=timeout_s` to `client.generate(**...)`, or drop the parameter entirely if Featherless's client doesn't honor it.
|
Phase 3.5 cleanup shipped end-to-end across 12 tasks (T76–T87). Two CLAUDE.md backlogs (Phase 2.6/3, Phase 3.5/4) are now empty; deferred follow-ups discovered during execution are tracked in a new "Phase 3.6 / 4 backlog" section below. Test count grew from 315 (Phase 3) to 343 (+28 new tests).
|
||||||
|
|
||||||
#### From T57 review
|
- **Wave 1 — trivial polish (parallel)**:
|
||||||
|
- **T76** `narrate_skip` `timeout_s` plumbed through to `client.generate`.
|
||||||
|
- **T77** `AddresseeDecision.confidence` typed as `Literal["high","medium","low"]`.
|
||||||
|
- **T78** `search_memories` docstring notes SQL-side significance bias (`SIGNIFICANCE_RANK_BIAS`).
|
||||||
|
- **T79** `_witness_role_for` defensive `host_bot_id is None` handling (returns `"host"` for Phase-1 chats).
|
||||||
|
- **Wave 2 — scene_summarize polish (single)**:
|
||||||
|
- **T80** five T58 follow-ups: re-close suffix bloat guard, transcript scoping by scene, swallowed-exception logging in `detect_threads`, chat-clock `closed_at`, and three new tests covering T58 gaps (200-char truncation, `thread_updated`/`thread_closed` candidate paths, try/except fallback).
|
||||||
|
- **Wave 3 — typed exception (single)**:
|
||||||
|
- **T81** `ChatNotFoundError` replaces string-prefix sniff in skip routes; mapped to 404 (vs 400 for other `ValueError` cases).
|
||||||
|
- **Wave 4 — turn-flow wiring (single)**:
|
||||||
|
- **T82** `consume_pending_meanwhile_digests` wired into `post_turn` (closes T66 gap; meanwhile digests no longer pile up); natural-language skip dispatch now runs scene close detection first.
|
||||||
|
- **Wave 5 — regenerate polish (single)**:
|
||||||
|
- **T83** five sub-fixes — cancel/stop hook (regenerate registers stream task in `_in_flight_tasks`); DRY extraction of `read_recent_dialogue` and `gather_prior_edges` into `chat/services/turn_common.py`; chat-scoped sibling-assistant-turn lookup; lifecycle-rollback warning log on regenerate; ordering-symmetry comment between post_turn and regenerate event-detection paths.
|
||||||
|
- **Wave 6 — final polish (parallel)**:
|
||||||
|
- **T84** unified `record_turn_memory` API with `you_present` kwarg; `record_meanwhile_memory` becomes a thin wrapper.
|
||||||
|
- **T85** JSON-build audit (no findings) + meanwhile cancel route-level test.
|
||||||
|
- **T86** frontend `turn_html_replace` SSE handler + turn_id stamping on rendered HTML so the in-place swap actually works.
|
||||||
|
|
||||||
- **`search_memories` docstring should mention SQL-side significance bias**: the function docstring still describes only the Python composite re-rank; add a one-line note about `SIGNIFICANCE_RANK_BIAS`.
|
### Phase 3.6 / 4 backlog
|
||||||
|
|
||||||
#### From T58 review
|
New follow-ups discovered during Phase 3.5 reviews and execution. None are blocking; pick up at any time.
|
||||||
|
|
||||||
- **Scene close re-close suffix bloat risk**: `_build_key_quotes_suffix` reads from `memories.pov_summary`. If a scene close runs twice, the second pass would read the rewritten text plus the previous "Key quotes:" suffix and append a second one. Either guard for double-suffix or source quotes from `event_log` `assistant_turn`/`user_turn` text instead.
|
#### From T80 review
|
||||||
- **Thread detection transcript scoping**: `_read_recent_dialogue` returns chat-wide history with no `scene_id` filter (Phase 1 turns lack one). Feeding chat-wide history to `detect_threads` will misattribute threads to the closing scene when the scene boundary falls inside the last 50 turns. Scope by `scene_id` once turns carry it, or by `started_at` against scene-open timestamp.
|
|
||||||
- **Swallowed exceptions in `detect_threads` try/except**: bare `Exception` swallows programmer errors silently. Log at debug level so silent regressions are recoverable.
|
|
||||||
- **Scene close `closed_at` clock divergence**: T58 uses `datetime.now(timezone.utc).isoformat()` instead of chat-clock time. Diverges from chat-clock semantics elsewhere; revisit if event reconstructions need chat-clock ordering.
|
|
||||||
- **Test coverage gaps in T58**: no test for 200-char quote truncation; no test for `thread_updated`/`thread_closed` candidate paths; no test for the `try/except` fallback.
|
|
||||||
|
|
||||||
#### From T61 review
|
- **`read_recent_dialogue` chat-id pushdown**: helper filters `chat_id` post-fetch in Python. Could push the `json_extract(payload_json, '$.chat_id') = ?` predicate into SQL (matching T83.3's pattern) for tighter LIMIT semantics. Currently a chat-with-many-other-chats can have its 50-row LIMIT consumed by foreign rows.
|
||||||
|
- **Lifecycle warning wording in regenerate**: T83.4's warning log lists ALL lifecycle event ids that exist after the original `assistant_turn` id, not just ones produced by the superseded turn. For the typical "regenerate the most recent" flow these are identical, but if a user regenerates an OLDER turn, the warning will list intervening-turn lifecycle events that legitimately stand. Tighten warning wording to "lifecycle transitions at-or-after turn X" (operator-friendly); a code-level fix would require a schema change to add explicit back-reference from lifecycle events to their producing turn.
|
||||||
|
|
||||||
- **Regenerate doesn't roll back lifecycle transitions from superseded turn**: `event_started`/`event_completed` rows from a superseded turn remain. Phase 3.5 should add a lifecycle-undo step. Caveat: regenerate-after-completion may double-emit promotion artifacts if the new text re-completes the same event.
|
#### From T84 review
|
||||||
- **Asymmetry in event-detection ordering**: post_turn runs lifecycle BETWEEN interjection and scene-close; regenerate runs lifecycle at the END. Benign because regenerate has no scene-close path, but worth tidying.
|
|
||||||
|
|
||||||
#### From T62 review
|
- **`record_turn_memory` legacy single-bot function** still exists alongside the unified `record_turn_memory_for_present`. Could be consolidated in a follow-up.
|
||||||
|
|
||||||
- **Error-message prefix sniff for 404 vs 400 routing**: drawer skip routes use `str(exc).startswith("chat not found")` to distinguish 404 from 400. Fragile if error wording changes. Use a typed exception subclass.
|
#### From T86 fix-up
|
||||||
- **Skip command bypasses scene close detection**: a user typing "fade out, skip an hour" would skip without closing the scene. Acceptable for Phase 3 but worth noting.
|
|
||||||
|
|
||||||
#### From T63 review
|
- **Test fixtures + `tests/test_phase3_integration.py`** that seed turns directly via `append_event`+`project` may need updating once any new test asserts the rendered HTML carries the new turn ids end-to-end. Existing tests pass because they don't read the stamped attribute, but they're brittle if the contract evolves.
|
||||||
|
|
||||||
- **`participants_json` JSON injection** (FIXED in T63 but worth noting in backlog as a "double-check other JSON-string-build sites" task): T63 originally used f-string interpolation; fixed to use `json.dumps`. Audit other state modules for similar patterns.
|
#### Deferred items (carry-over)
|
||||||
|
|
||||||
#### From T64 review
|
- **Scene-close-on-cancel UX revisit** (Phase 2.5 carry-over): T74.3 pinned the existing behavior; revisit if real play-testing surfaces a regression.
|
||||||
|
- **Cross-feature canned-queue brittleness**: meanwhile-scene close test required a canned response for T65's digest call after T64+T65 merge. Future close-path additions will keep extending the queue. Consider a structured fixture builder rather than positional canned arrays. NOT addressed in Phase 3.5.
|
||||||
- **`record_meanwhile_memory` and `record_turn_memory_for_present` share private `_write_one_memory` helper**: minor DRY note; both helpers are similar enough that a unified API with a `you_present: bool` kwarg might be cleaner long-term.
|
- **Lifecycle-transition rollback in regenerate**: T83.4 added a warning log; actual rollback (with proper schema linkage from lifecycle event back to producing turn) is Phase 4 work.
|
||||||
- **Stop button cancellation for meanwhile turns**: T64 fix-up registered tasks in `_in_flight_tasks`; verify the `/turns/cancel` endpoint actually cancels meanwhile streams (the test pins registration but not the cancel-from-route path).
|
|
||||||
|
|
||||||
#### From cross-feature interactions discovered in Wave 6b merge
|
|
||||||
|
|
||||||
- **Cross-feature canned-queue brittleness**: meanwhile-scene close test required a canned response for T65's digest call after T64+T65 merge. Future close-path additions will keep extending the queue; consider a structured fixture builder rather than positional canned arrays.
|
|
||||||
|
|
||||||
#### From T66 integration tests
|
|
||||||
|
|
||||||
- **`consume_pending_meanwhile_digests` is defined but NOT wired into `post_turn`**: the helper lives in `chat/services/prompt.py` (T65) but `chat/web/turns.py` never calls it. Meanwhile digests stay pending forever in production. Phase 3.5 should call the helper after the first you-turn following a meanwhile close — probably right after the assistant_turn lands but before the next prompt assembly. Pinned by `tests/test_phase3_integration.py::test_meanwhile_close_digest_surfaces_then_consumed` which currently calls the helper directly.
|
|
||||||
|
|
||||||
#### Discovered during Phase 3 execution
|
|
||||||
|
|
||||||
- **`_witness_role_for` defensive `host_bot_id is None`** (carry-over from Phase 2.5 T71 backlog) — still pending.
|
|
||||||
|
|||||||
@@ -134,17 +134,34 @@ def record_turn_memory_for_present(
|
|||||||
chat_clock_at: str | None = None,
|
chat_clock_at: str | None = None,
|
||||||
source: str = "direct",
|
source: str = "direct",
|
||||||
significance: int = 1,
|
significance: int = 1,
|
||||||
|
you_present: bool = True,
|
||||||
) -> dict[str, tuple[int, int | None]]:
|
) -> dict[str, tuple[int, int | None]]:
|
||||||
"""Write a ``memory_written`` event for each present bot witness.
|
"""Single entry-point for per-turn memory writes (T84).
|
||||||
|
|
||||||
Host is always written. Guest is written iff ``guest_bot_id is not
|
Writes one ``memory_written`` event per present bot witness. Host is
|
||||||
None``. Witness flags are ``[you=1, host=1, guest=1]`` when a guest
|
always written. Guest is written iff ``guest_bot_id is not None``.
|
||||||
is present, ``[you=1, host=1, guest=0]`` otherwise.
|
|
||||||
|
Witness flags depend on ``you_present``:
|
||||||
|
|
||||||
|
- ``you_present=True`` (default — Phase 1/2/3 you-scenes): the user
|
||||||
|
is a witness. Mask is ``[you=1, host=1, guest=1]`` when a guest is
|
||||||
|
present, ``[you=1, host=1, guest=0]`` otherwise.
|
||||||
|
- ``you_present=False`` (Phase 3 meanwhile scenes): the user is
|
||||||
|
absent. Mask is ``[you=0, host=1, guest=1]`` for both bots. Both
|
||||||
|
``host_bot_id`` and ``guest_bot_id`` are required — a meanwhile
|
||||||
|
scene by definition has both bots, so passing ``guest_bot_id=None``
|
||||||
|
with ``you_present=False`` is a programming error and raises
|
||||||
|
:class:`ValueError`.
|
||||||
|
|
||||||
Returns a mapping ``{bot_id: (event_id, memory_id)}`` so callers can
|
Returns a mapping ``{bot_id: (event_id, memory_id)}`` so callers can
|
||||||
look up the freshly-projected memory id per owner without re-querying
|
look up the freshly-projected memory id per owner without re-querying
|
||||||
the database.
|
the database.
|
||||||
"""
|
"""
|
||||||
|
if not you_present and guest_bot_id is None:
|
||||||
|
raise ValueError("you_present=False requires guest_bot_id")
|
||||||
|
|
||||||
|
witness_you = 1 if you_present else 0
|
||||||
|
witness_host = 1
|
||||||
witness_guest = 1 if guest_bot_id is not None else 0
|
witness_guest = 1 if guest_bot_id is not None else 0
|
||||||
|
|
||||||
result: dict[str, tuple[int, int | None]] = {}
|
result: dict[str, tuple[int, int | None]] = {}
|
||||||
@@ -153,8 +170,8 @@ def record_turn_memory_for_present(
|
|||||||
owner_id=host_bot_id,
|
owner_id=host_bot_id,
|
||||||
chat_id=chat_id,
|
chat_id=chat_id,
|
||||||
narrative_text=narrative_text,
|
narrative_text=narrative_text,
|
||||||
witness_you=1,
|
witness_you=witness_you,
|
||||||
witness_host=1,
|
witness_host=witness_host,
|
||||||
witness_guest=witness_guest,
|
witness_guest=witness_guest,
|
||||||
scene_id=scene_id,
|
scene_id=scene_id,
|
||||||
chat_clock_at=chat_clock_at,
|
chat_clock_at=chat_clock_at,
|
||||||
@@ -167,8 +184,8 @@ def record_turn_memory_for_present(
|
|||||||
owner_id=guest_bot_id,
|
owner_id=guest_bot_id,
|
||||||
chat_id=chat_id,
|
chat_id=chat_id,
|
||||||
narrative_text=narrative_text,
|
narrative_text=narrative_text,
|
||||||
witness_you=1,
|
witness_you=witness_you,
|
||||||
witness_host=1,
|
witness_host=witness_host,
|
||||||
witness_guest=1,
|
witness_guest=1,
|
||||||
scene_id=scene_id,
|
scene_id=scene_id,
|
||||||
chat_clock_at=chat_clock_at,
|
chat_clock_at=chat_clock_at,
|
||||||
@@ -190,46 +207,22 @@ def record_meanwhile_memory(
|
|||||||
source: str = "direct",
|
source: str = "direct",
|
||||||
significance: int = 1,
|
significance: int = 1,
|
||||||
) -> dict[str, tuple[int, int | None]]:
|
) -> dict[str, tuple[int, int | None]]:
|
||||||
"""Write per-POV ``memory_written`` events for a meanwhile turn (T64).
|
"""Backward-compat thin wrapper for meanwhile memory writes (T64, T84).
|
||||||
|
|
||||||
A meanwhile scene runs entirely between host + guest, with "you"
|
Equivalent to calling :func:`record_turn_memory_for_present` with
|
||||||
absent. Both bots are present witnesses, so each one gets a row with
|
``you_present=False``. Kept so existing call sites in
|
||||||
witness flags ``[you=0, host=1, guest=1]`` — different from the
|
:mod:`chat.web.meanwhile` continue to work without churn. New code
|
||||||
normal-turn ``record_turn_memory_for_present`` shape, which assumes
|
should prefer the unified entry-point directly.
|
||||||
the user is always a witness (``witness_you=1``).
|
|
||||||
|
|
||||||
The ``guest_bot_id`` is required (a meanwhile scene by definition
|
|
||||||
has both bots) — callers passing ``None`` is a programming error.
|
|
||||||
|
|
||||||
Returns ``{bot_id: (event_id, memory_id)}`` mirroring
|
|
||||||
:func:`record_turn_memory_for_present` so downstream queues
|
|
||||||
(significance scoring) can pull memory ids without re-querying.
|
|
||||||
"""
|
"""
|
||||||
result: dict[str, tuple[int, int | None]] = {}
|
return record_turn_memory_for_present(
|
||||||
result[host_bot_id] = _write_one_memory(
|
|
||||||
conn,
|
conn,
|
||||||
owner_id=host_bot_id,
|
|
||||||
chat_id=chat_id,
|
chat_id=chat_id,
|
||||||
|
host_bot_id=host_bot_id,
|
||||||
|
guest_bot_id=guest_bot_id,
|
||||||
narrative_text=narrative_text,
|
narrative_text=narrative_text,
|
||||||
witness_you=0,
|
|
||||||
witness_host=1,
|
|
||||||
witness_guest=1,
|
|
||||||
scene_id=scene_id,
|
scene_id=scene_id,
|
||||||
chat_clock_at=chat_clock_at,
|
chat_clock_at=chat_clock_at,
|
||||||
source=source,
|
source=source,
|
||||||
significance=significance,
|
significance=significance,
|
||||||
|
you_present=False,
|
||||||
)
|
)
|
||||||
result[guest_bot_id] = _write_one_memory(
|
|
||||||
conn,
|
|
||||||
owner_id=guest_bot_id,
|
|
||||||
chat_id=chat_id,
|
|
||||||
narrative_text=narrative_text,
|
|
||||||
witness_you=0,
|
|
||||||
witness_host=1,
|
|
||||||
witness_guest=1,
|
|
||||||
scene_id=scene_id,
|
|
||||||
chat_clock_at=chat_clock_at,
|
|
||||||
source=source,
|
|
||||||
significance=significance,
|
|
||||||
)
|
|
||||||
return result
|
|
||||||
|
|||||||
@@ -381,7 +381,10 @@ async def regenerate_assistant_turn(
|
|||||||
speaker_bot.get("name", "bot") if speaker_bot is not None else "bot"
|
speaker_bot.get("name", "bot") if speaker_bot is not None else "bot"
|
||||||
)
|
)
|
||||||
new_turn_html = render_turn_html(
|
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(
|
await publish(
|
||||||
chat_id,
|
chat_id,
|
||||||
@@ -616,7 +619,10 @@ async def regenerate_assistant_turn(
|
|||||||
# Broadcast a replace event so connected tabs swap the prior
|
# Broadcast a replace event so connected tabs swap the prior
|
||||||
# interjection node in-place (mirrors T73.1's primary swap).
|
# interjection node in-place (mirrors T73.1's primary swap).
|
||||||
interject_html = render_turn_html(
|
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(
|
await publish(
|
||||||
chat_id,
|
chat_id,
|
||||||
|
|||||||
@@ -74,17 +74,24 @@ def read_recent_dialogue(
|
|||||||
)
|
)
|
||||||
rows = list(reversed(cur.fetchall()))
|
rows = list(reversed(cur.fetchall()))
|
||||||
out: list[dict] = []
|
out: list[dict] = []
|
||||||
for _row_id, kind, payload_json in rows:
|
for row_id, kind, payload_json in rows:
|
||||||
p = json.loads(payload_json)
|
p = json.loads(payload_json)
|
||||||
if p.get("chat_id") != chat_id:
|
if p.get("chat_id") != chat_id:
|
||||||
continue
|
continue
|
||||||
if kind in ("user_turn", "user_turn_edit"):
|
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:
|
else:
|
||||||
out.append(
|
out.append(
|
||||||
{
|
{
|
||||||
"speaker": p.get("speaker_id", "bot"),
|
"speaker": p.get("speaker_id", "bot"),
|
||||||
"text": p.get("text", ""),
|
"text": p.get("text", ""),
|
||||||
|
"event_id": row_id,
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
return out
|
return out
|
||||||
|
|||||||
@@ -17,7 +17,7 @@
|
|||||||
<p class="muted">No turns yet. Start typing below.</p>
|
<p class="muted">No turns yet. Start typing below.</p>
|
||||||
{% else %}
|
{% else %}
|
||||||
{% for turn in turns %}
|
{% 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>
|
<strong>{{ turn.speaker }}</strong>
|
||||||
{{ turn.text|render_prose|safe }}
|
{{ turn.text|render_prose|safe }}
|
||||||
</div>
|
</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
|
// SSE connection lost — show a banner and unlock so the user can
|
||||||
// retry. The server commits the partial as truncated when its
|
// retry. The server commits the partial as truncated when its
|
||||||
// request.is_disconnected() poll trips (T19).
|
// 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)
|
raw_turns = _read_recent_dialogue(conn, chat_id, limit=200)
|
||||||
turns: list[dict] = []
|
turns: list[dict] = []
|
||||||
for t in raw_turns:
|
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":
|
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:
|
else:
|
||||||
bot = get_bot(conn, t["speaker"])
|
bot = get_bot(conn, t["speaker"])
|
||||||
label = bot["name"] if bot else 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(
|
return TEMPLATES.TemplateResponse(
|
||||||
request,
|
request,
|
||||||
|
|||||||
@@ -378,7 +378,12 @@ async def process_meanwhile_turn(
|
|||||||
"truncated": truncated,
|
"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})
|
await publish(chat_id, {"event": "turn_html", "data": turn_html})
|
||||||
|
|
||||||
if cancelled:
|
if cancelled:
|
||||||
|
|||||||
+15
-2
@@ -84,7 +84,13 @@ def render_prose(text: str) -> str:
|
|||||||
return "".join(f"<p>{p}</p>" for p in paragraphs)
|
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>``.
|
"""Render a full transcript turn as ``<div class="turn …">…</div>``.
|
||||||
|
|
||||||
Used by both the SSE fragment publisher in :mod:`chat.web.turns`
|
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
|
``role`` selects the CSS class (``turn-you`` vs ``turn-bot``); the
|
||||||
speaker label and role name are HTML-escaped defensively even though
|
speaker label and role name are HTML-escaped defensively even though
|
||||||
they currently come from trusted server-side state.
|
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)
|
speaker_html = html.escape(speaker)
|
||||||
role_html = html.escape(role)
|
role_html = html.escape(role)
|
||||||
body_html = render_prose(text)
|
body_html = render_prose(text)
|
||||||
|
id_attr = f' id="turn-{int(event_id)}"' if event_id is not None else ""
|
||||||
return (
|
return (
|
||||||
f'<div class="turn turn-{role_html}">'
|
f'<div{id_attr} class="turn turn-{role_html}">'
|
||||||
f"<strong>{speaker_html}</strong>"
|
f"<strong>{speaker_html}</strong>"
|
||||||
f"{body_html}"
|
f"{body_html}"
|
||||||
f"</div>"
|
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
|
# 7. Append the assistant_turn with the final text. (See note above on
|
||||||
# why we skip ``project`` for these transcript-only event kinds.)
|
# 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,
|
conn,
|
||||||
kind="assistant_turn",
|
kind="assistant_turn",
|
||||||
payload={
|
payload={
|
||||||
@@ -583,6 +587,7 @@ async def post_turn(
|
|||||||
interjection_text: str | None = None
|
interjection_text: str | None = None
|
||||||
interjection_speaker_id: str | None = None
|
interjection_speaker_id: str | None = None
|
||||||
interjection_truncated = False
|
interjection_truncated = False
|
||||||
|
interjection_event_id: int | None = None
|
||||||
if (
|
if (
|
||||||
guest_bot is not None
|
guest_bot is not None
|
||||||
and not cancelled
|
and not cancelled
|
||||||
@@ -670,7 +675,9 @@ async def post_turn(
|
|||||||
|
|
||||||
interjection_text = "".join(interject_accumulated)
|
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,
|
conn,
|
||||||
kind="assistant_turn",
|
kind="assistant_turn",
|
||||||
payload={
|
payload={
|
||||||
@@ -925,7 +932,10 @@ async def post_turn(
|
|||||||
},
|
},
|
||||||
)
|
)
|
||||||
primary_html = _render_turn_html(
|
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(
|
await publish(
|
||||||
chat_id, {"event": "turn_html", "data": primary_html}
|
chat_id, {"event": "turn_html", "data": primary_html}
|
||||||
@@ -949,7 +959,10 @@ async def post_turn(
|
|||||||
},
|
},
|
||||||
)
|
)
|
||||||
interject_html = _render_turn_html(
|
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(
|
await publish(
|
||||||
chat_id, {"event": "turn_html", "data": interject_html}
|
chat_id, {"event": "turn_html", "data": interject_html}
|
||||||
|
|||||||
@@ -570,3 +570,173 @@ def test_meanwhile_turn_registered_in_in_flight_tasks(
|
|||||||
# Post-flight: the entry has been cleaned up so the next turn (or
|
# Post-flight: the entry has been cleaned up so the next turn (or
|
||||||
# the cancel route) doesn't see a stale task.
|
# the cancel route) doesn't see a stale task.
|
||||||
assert "chat_bot_a" not in _in_flight_tasks
|
assert "chat_bot_a" not in _in_flight_tasks
|
||||||
|
|
||||||
|
|
||||||
|
def test_meanwhile_turn_cancellation_via_route(app_state_setup, tmp_path):
|
||||||
|
"""T85.2: a cancellation that fires while a meanwhile beat is
|
||||||
|
streaming truncates the assistant_turn and skips the post-turn
|
||||||
|
memory + state-update writes — the same end-to-end shape the
|
||||||
|
/turns/cancel route produces.
|
||||||
|
|
||||||
|
Drives the cancel by hijacking ``client.stream`` to raise
|
||||||
|
CancelledError on its first iteration — the exact pattern proven
|
||||||
|
by ``test_cancelled_turn_still_closes_scene_when_user_prose_signals_close``
|
||||||
|
in ``tests/test_turn_flow.py``. This mirrors what
|
||||||
|
``cancel_turn`` does in production (``task.cancel()`` schedules a
|
||||||
|
CancelledError on the next await); doing the raise inline avoids
|
||||||
|
the TestClient-loop-reentry problem that prevents driving a second
|
||||||
|
POST mid-stream from the same synchronous test thread, while
|
||||||
|
exercising the same code path: the meanwhile streamer's
|
||||||
|
``except asyncio.CancelledError`` block at meanwhile.py:276 sets
|
||||||
|
``cancelled=True`` + ``truncated=True``, the assistant_turn lands
|
||||||
|
with the partial, and the memory/state-update branch is skipped.
|
||||||
|
|
||||||
|
The ``_in_flight_tasks`` registration that wires the cancel route
|
||||||
|
to the meanwhile streamer is independently pinned by
|
||||||
|
``test_meanwhile_turn_registered_in_in_flight_tasks`` above; this
|
||||||
|
test pins the downstream behavioural shape the registration
|
||||||
|
enables — together they cover the full Stop-button lifecycle for
|
||||||
|
meanwhile beats.
|
||||||
|
|
||||||
|
Behavioural pins:
|
||||||
|
|
||||||
|
* ``assistant_turn`` lands with ``truncated=True``,
|
||||||
|
``meanwhile_scene_id=2``, ``speaker_id="bot_a"``.
|
||||||
|
* No ``memory_written`` events fire (cancel skips per-bot writes).
|
||||||
|
* No post-turn ``edge_update`` events fire (cancel skips state updates).
|
||||||
|
* ``_in_flight_tasks`` is empty post-flight.
|
||||||
|
"""
|
||||||
|
from typing import AsyncIterator, Sequence
|
||||||
|
|
||||||
|
from chat.llm.client import Message
|
||||||
|
from chat.web.turns import _in_flight_tasks
|
||||||
|
|
||||||
|
_seed_meanwhile_chat(tmp_path / "test.db")
|
||||||
|
|
||||||
|
class _CancelOnStreamMock(MockLLMClient):
|
||||||
|
"""Yields CancelledError on first iteration of ``stream`` —
|
||||||
|
simulates ``cancel_turn`` having fired ``task.cancel()`` on the
|
||||||
|
in-flight streaming task. ``generate`` is delegated to the
|
||||||
|
canned-queue base so parse_turn still resolves cleanly.
|
||||||
|
"""
|
||||||
|
|
||||||
|
async def stream(
|
||||||
|
self, messages: Sequence[Message], *, model: str, **params
|
||||||
|
) -> AsyncIterator[str]:
|
||||||
|
raise asyncio.CancelledError
|
||||||
|
yield # pragma: no cover — keeps this an async generator.
|
||||||
|
|
||||||
|
canned_parse = json.dumps(
|
||||||
|
{"segments": [{"kind": "narration", "text": "they exchange a glance"}]}
|
||||||
|
)
|
||||||
|
# Canned queue: only parse_turn — the narrative slot is never pulled
|
||||||
|
# because stream raises before consuming it, and post-turn
|
||||||
|
# state-update is skipped by the cancel branch.
|
||||||
|
mock = _CancelOnStreamMock(canned=[canned_parse])
|
||||||
|
from chat.web.kickoff import get_llm_client
|
||||||
|
|
||||||
|
app.dependency_overrides[get_llm_client] = lambda: mock
|
||||||
|
try:
|
||||||
|
# The meanwhile controller re-raises CancelledError after the
|
||||||
|
# partial assistant_turn is recorded (meanwhile.py:387). The
|
||||||
|
# outer post_turn route has no catch for CancelledError on the
|
||||||
|
# meanwhile path (turns.py:244-254 only catches ValueError), so
|
||||||
|
# the exception propagates up through Starlette. TestClient
|
||||||
|
# surfaces that as a 500 or a propagated exception depending on
|
||||||
|
# Starlette/asyncio versions; we don't pin the response.
|
||||||
|
try:
|
||||||
|
app_state_setup.post(
|
||||||
|
"/chats/chat_bot_a/turns",
|
||||||
|
data={"prose": "they exchange a glance"},
|
||||||
|
)
|
||||||
|
except BaseException:
|
||||||
|
pass
|
||||||
|
finally:
|
||||||
|
app.dependency_overrides.clear()
|
||||||
|
|
||||||
|
with open_db(tmp_path / "test.db") as conn:
|
||||||
|
assistant_rows = conn.execute(
|
||||||
|
"SELECT payload_json FROM event_log "
|
||||||
|
"WHERE kind = 'assistant_turn' ORDER BY id"
|
||||||
|
).fetchall()
|
||||||
|
memory_count = conn.execute(
|
||||||
|
"SELECT COUNT(*) FROM event_log WHERE kind = 'memory_written'"
|
||||||
|
).fetchone()[0]
|
||||||
|
# Edge updates AFTER the assistant_turn (i.e. excluding seeded ones).
|
||||||
|
max_at_row = conn.execute(
|
||||||
|
"SELECT MAX(id) FROM event_log WHERE kind = 'assistant_turn'"
|
||||||
|
).fetchone()
|
||||||
|
max_at = max_at_row[0] if max_at_row[0] is not None else 0
|
||||||
|
post_turn_edge_updates = conn.execute(
|
||||||
|
"SELECT COUNT(*) FROM event_log "
|
||||||
|
"WHERE kind = 'edge_update' AND id > ?",
|
||||||
|
(max_at,),
|
||||||
|
).fetchone()[0]
|
||||||
|
|
||||||
|
# The cancelled assistant_turn was still recorded with truncated=True,
|
||||||
|
# carrying whatever partial text accumulated before cancel propagated
|
||||||
|
# (zero text here since the cancel hits on the first iteration).
|
||||||
|
assert len(assistant_rows) == 1
|
||||||
|
payload = json.loads(assistant_rows[0][0])
|
||||||
|
assert payload["truncated"] is True, payload
|
||||||
|
assert payload["meanwhile_scene_id"] == 2
|
||||||
|
assert payload["speaker_id"] == "bot_a"
|
||||||
|
|
||||||
|
# No per-bot memory writes — cancellation short-circuits the memory
|
||||||
|
# + state-update branch (see chat/web/meanwhile.py:308).
|
||||||
|
assert memory_count == 0
|
||||||
|
|
||||||
|
# No post-turn edge_updates — same short-circuit.
|
||||||
|
assert post_turn_edge_updates == 0
|
||||||
|
|
||||||
|
# Post-flight: registry cleared so the cancel route won't try to
|
||||||
|
# re-cancel a defunct task on a follow-up POST.
|
||||||
|
assert "chat_bot_a" not in _in_flight_tasks
|
||||||
|
|
||||||
|
|
||||||
|
def test_meanwhile_cancel_route_no_op_after_turn_completes(
|
||||||
|
app_state_setup, tmp_path
|
||||||
|
):
|
||||||
|
"""T85.2: POST ``/chats/<id>/turns/cancel`` AFTER a meanwhile turn
|
||||||
|
has fully completed is a silent 204 no-op — there is no in-flight
|
||||||
|
task to cancel, the registry is empty, and the route must not error.
|
||||||
|
|
||||||
|
Pins the cancel endpoint's robustness against the common-but-racy
|
||||||
|
sequence where the user clicks Stop just after the stream finished
|
||||||
|
(the SSE channel hasn't yet flipped the client-side ``isStreaming``
|
||||||
|
flag). This is a complement to the snapshot test: the snapshot test
|
||||||
|
pins that the registry IS populated mid-flight, this test pins that
|
||||||
|
it isn't AFTER and that the route copes gracefully.
|
||||||
|
"""
|
||||||
|
from chat.web.turns import _in_flight_tasks
|
||||||
|
|
||||||
|
_seed_meanwhile_chat(tmp_path / "test.db")
|
||||||
|
canned_parse = json.dumps(
|
||||||
|
{"segments": [{"kind": "narration", "text": "they exchange a glance"}]}
|
||||||
|
)
|
||||||
|
canned = [
|
||||||
|
canned_parse,
|
||||||
|
"BotA leans in. *quietly*",
|
||||||
|
_zero_state(),
|
||||||
|
_zero_state(),
|
||||||
|
]
|
||||||
|
mock = _override_llm(canned)
|
||||||
|
try:
|
||||||
|
response = app_state_setup.post(
|
||||||
|
"/chats/chat_bot_a/turns",
|
||||||
|
data={"prose": "they exchange a glance"},
|
||||||
|
)
|
||||||
|
assert response.status_code == 204
|
||||||
|
finally:
|
||||||
|
app.dependency_overrides.clear()
|
||||||
|
assert mock._canned == []
|
||||||
|
|
||||||
|
# Registry was cleaned up after the stream completed.
|
||||||
|
assert "chat_bot_a" not in _in_flight_tasks
|
||||||
|
|
||||||
|
# Cancel after-the-fact: 204, no error, registry stays empty.
|
||||||
|
cancel_response = app_state_setup.post(
|
||||||
|
"/chats/chat_bot_a/turns/cancel"
|
||||||
|
)
|
||||||
|
assert cancel_response.status_code == 204
|
||||||
|
assert "chat_bot_a" not in _in_flight_tasks
|
||||||
|
|||||||
@@ -444,3 +444,91 @@ def test_record_for_present_dict_keys_match(tmp_path):
|
|||||||
narrative_text="Both bots witness this.",
|
narrative_text="Both bots witness this.",
|
||||||
)
|
)
|
||||||
assert set(result_with_guest.keys()) == {"bot_a", "bot_b"}
|
assert set(result_with_guest.keys()) == {"bot_a", "bot_b"}
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# T84: unified record_turn_memory_for_present API with you_present kwarg.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_record_turn_memory_you_present_false_writes_meanwhile_witness_mask(tmp_path):
|
||||||
|
"""When ``you_present=False`` the witness mask should be
|
||||||
|
``[you=0, host=1, guest=1]`` for both bots — the meanwhile shape."""
|
||||||
|
db = tmp_path / "t.db"
|
||||||
|
apply_migrations(db)
|
||||||
|
_seed_two_bots(db)
|
||||||
|
with open_db(db) as conn:
|
||||||
|
result = record_turn_memory_for_present(
|
||||||
|
conn,
|
||||||
|
chat_id="chat_ab",
|
||||||
|
host_bot_id="bot_a",
|
||||||
|
guest_bot_id="bot_b",
|
||||||
|
narrative_text="BotA and BotB confer privately.",
|
||||||
|
scene_id=None,
|
||||||
|
chat_clock_at="2026-04-26T20:00:00+00:00",
|
||||||
|
you_present=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert set(result.keys()) == {"bot_a", "bot_b"}
|
||||||
|
|
||||||
|
rows = conn.execute(
|
||||||
|
"SELECT owner_id, witness_you, witness_host, witness_guest "
|
||||||
|
"FROM memories ORDER BY owner_id"
|
||||||
|
).fetchall()
|
||||||
|
assert len(rows) == 2
|
||||||
|
for _owner, w_you, w_host, w_guest in rows:
|
||||||
|
assert w_you == 0
|
||||||
|
assert w_host == 1
|
||||||
|
assert w_guest == 1
|
||||||
|
|
||||||
|
# Two memory_written events were appended.
|
||||||
|
cur = conn.execute(
|
||||||
|
"SELECT COUNT(*) FROM event_log WHERE kind = 'memory_written'"
|
||||||
|
)
|
||||||
|
assert cur.fetchone()[0] == 2
|
||||||
|
|
||||||
|
|
||||||
|
def test_record_turn_memory_you_present_true_default_writes_normal_witness_mask(tmp_path):
|
||||||
|
"""Default ``you_present=True`` preserves Phase 2 behaviour:
|
||||||
|
``witness_you=1`` for the host POV row."""
|
||||||
|
db = tmp_path / "t.db"
|
||||||
|
apply_migrations(db)
|
||||||
|
_seed_minimal(db)
|
||||||
|
with open_db(db) as conn:
|
||||||
|
# No explicit you_present arg — should default to True.
|
||||||
|
result = record_turn_memory_for_present(
|
||||||
|
conn,
|
||||||
|
chat_id="chat_bot_a",
|
||||||
|
host_bot_id="bot_a",
|
||||||
|
guest_bot_id=None,
|
||||||
|
narrative_text="BotA hums to herself.",
|
||||||
|
)
|
||||||
|
assert set(result.keys()) == {"bot_a"}
|
||||||
|
|
||||||
|
row = conn.execute(
|
||||||
|
"SELECT witness_you, witness_host, witness_guest "
|
||||||
|
"FROM memories WHERE owner_id = 'bot_a'"
|
||||||
|
).fetchone()
|
||||||
|
assert row is not None
|
||||||
|
w_you, w_host, w_guest = row
|
||||||
|
assert w_you == 1
|
||||||
|
assert w_host == 1
|
||||||
|
assert w_guest == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_record_turn_memory_you_present_false_requires_guest(tmp_path):
|
||||||
|
"""Calling with ``you_present=False`` and no ``guest_bot_id`` is a
|
||||||
|
programming error — meanwhile scenes always have both bots."""
|
||||||
|
db = tmp_path / "t.db"
|
||||||
|
apply_migrations(db)
|
||||||
|
_seed_minimal(db)
|
||||||
|
with open_db(db) as conn:
|
||||||
|
with pytest.raises(ValueError, match="you_present=False requires guest_bot_id"):
|
||||||
|
record_turn_memory_for_present(
|
||||||
|
conn,
|
||||||
|
chat_id="chat_bot_a",
|
||||||
|
host_bot_id="bot_a",
|
||||||
|
guest_bot_id=None,
|
||||||
|
narrative_text="invalid",
|
||||||
|
you_present=False,
|
||||||
|
)
|
||||||
|
|||||||
@@ -85,3 +85,26 @@ def test_render_prose_mixed_full_message():
|
|||||||
assert '<em class="action">looks up</em>' in out
|
assert '<em class="action">looks up</em>' in out
|
||||||
# The apostrophe in ``she's`` is HTML-escaped to ``'``.
|
# The apostrophe in ``she's`` is HTML-escaped to ``'``.
|
||||||
assert '<span class="ooc">((she's tired))</span>' in out
|
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
|
assert "stop-streaming" in body or "isStreaming" in body
|
||||||
# Cancel route reference must be wired so the Stop button can call it.
|
# Cancel route reference must be wired so the Stop button can call it.
|
||||||
assert "/turns/cancel" in body
|
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:
|
with open_db(db) as conn:
|
||||||
out = read_recent_dialogue(conn, "chat_a", limit=10)
|
out = read_recent_dialogue(conn, "chat_a", limit=10)
|
||||||
|
|
||||||
assert out == [
|
# Each entry now carries the source ``event_log.id`` as ``event_id``
|
||||||
{"speaker": "you", "text": "hello"},
|
# (T86 follow-up) so the chat-detail Jinja loop can stamp
|
||||||
{"speaker": "bot_a", "text": "Original."},
|
# ``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):
|
def test_read_recent_dialogue_filters_superseded_and_other_chats(tmp_path):
|
||||||
|
|||||||
Reference in New Issue
Block a user